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

Ahmad Fatoum
2026-02-06T13:09:52Z

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 |