[OpenWrt-Devel] [PATCH] bcm53xx: Add functions to get/release copies of nvram contents.

[Ian Kent]

On Tue, 2015-05-12 at 19:31 +0200, Rafał Miłecki wrote:
> On 12 May 2015 at 15:10, Hante Meuleman <meuleman at 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.

kmalloc() can fail on larger allocation requests, especially on low
memory systems, due to chunk table fragmentation and there's a limit on
size, 128k I think.

I've seen order 4 allocation failures when trying to kmalloc() 64k
chunks in the past. But I also agree that, this being done early in the
boot process, those fails shouldn't occur, but nevertheless ....

> 
> 
> > +       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 at lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel