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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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.

> +	/* 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()


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 |