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

Ian Kent raven at themaw.net
Wed May 13 02:24:00 EDT 2015


On Wed, 2015-05-13 at 06:34 +0200, Rafał Miłecki wrote:
> On 13 May 2015 at 03:49, Ian Kent <raven at themaw.net> wrote:
> > 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 ....
> 
> I was thinking about vmalloc vs. returning nvram_buf address :)

Ha, right, still I believe in not using a data structure owned by
someone else when you don't "really" know what might be happening with
it along the way.

Ian
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel


More information about the openwrt-devel mailing list