On 12 May 2015 at 15:10, Hante Meuleman <meule...@broadcom.com> wrote:
> Added two functions to the bcm47xx_nvram module to get and release copies
> of the complete nvram contents. This can be used by for example brcmfmac
> to get complete nvram contents which will after some parsing be sent to
> device.

Thanks for sending this PATCH in a friendly way. It's plain text and
seems to be well formatted, nice!

Two comments:

1) This patch does more than you say. You modify initialization and
reading. Please keep patch per change (and explain them).

2) You modify OpenWrt's copy of mainline driver code. I'm pretty sure
I explained it to you. I don't want these driver to start differ
again. I want to submit all change to mainline version and just copy
most recent version to bcm53xx. So instead of this patch against
OpenWrt's copy, please send a patch against mainline's nvram.c in the
first place.



> +char *bcm47xx_nvram_get_contents(size_t *nvram_size)
> +{
> +       int err;
> +       char *nvram;
> +
> +       if (!nvram_buf[0]) {
> +               err = nvram_init();
> +               if (err)
> +                       return NULL;
> +       }
> +
> +       *nvram_size = nvram_len - sizeof(struct nvram_header);
> +       nvram = vmalloc(*nvram_size);

I can see you switched to vmalloc. Just to confirm, I'm OK with either way.


> +       if (nvram == NULL)
> +               return NULL;
> +       memcpy(nvram, &nvram_buf[sizeof(struct nvram_header)], *nvram_size);
> +
> +       return nvram;
> +}
> +EXPORT_SYMBOL(bcm47xx_nvram_get_contents);
> +
> +void bcm47xx_nvram_release_contents(char *nvram)
> +{
> +       vfree(nvram);
> +}
> +EXPORT_SYMBOL(bcm47xx_nvram_release_contents);
> +
> +
>  MODULE_LICENSE("GPLv2");

One empty line will be enough.
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Reply via email to