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

[Rafał Miłecki]

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 :)

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