Re: [PATCH 3/5] mfd: Add Hexagon EFI driver

[Marco Felsch <m.felsch@pengutronix.de>]

Hi Ahmad,

On 26-02-06, Ahmad Fatoum wrote:
> Hi,
> 
> On 2/5/26 4:45 PM, Marco Felsch wrote:
> > This adds the EFI core driver to communicate with the system
> > co-processor.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> Acked-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> but see below for some nits should there be a v2.
> 
> > +	max_msg_len = HGS_EFI_SEP_FRAME_PREAMBLE_SZ +
> > +		      HGS_EFI_SEP_FRAME_POSTAMBLE_SZ +
> > +		      efi->coder->sep_header_hdrsize + cmd->payload_size;
> > +	msg = p = xzalloc(max_msg_len);
> > +	if (!msg) {
> 
> xzalloc never fails.
> 
> > +		dev_err(dev, "No memory\n");
> 
> dev_err allocates memory, so this wouldn't work anyway.

Dropped the check and the print, also for the probe() function.

> > +	/* Split header from payload first for the following str-ops on buf */
> > +	payload = strstr(buf, ":");
> 
> Nitpick: strchr()
> 
> > +	if (!payload) {
> > +		dev_warn(dev, "Failed to find header delim\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	hdrlen = payload - buf;
> > +	if (hdrlen > sizeof(struct hgs_efi_sep_ascii_hdr)) {
> > +		dev_warn(dev, "Invalid header len detected\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	*payload = 0;
> > +	payload++;
> > +
> > +	/*
> > +	 * Albeit the CRC is optional and the calc as a few flaws the coder may
> > +	 * has added it. Skip the CRC check but do the msg-id check.
> > +	 */
> > +	p = strstr(buf, ",");
> 
> Nitpick: strchr()

I applied all your findings, thanks!

Regards,
  Marco

> 
> 
> Cheers,
> Ahmad
> 
> -- 
> Pengutronix e.K.                  |                             |
> Steuerwalder Str. 21              | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |