On Wed, 2015-05-13 at 06:34 +0200, Rafał Miłecki wrote: > On 13 May 2015 at 03:49, Ian Kent <[email protected]> wrote: > > On Tue, 2015-05-12 at 19:31 +0200, Rafał Miłecki wrote: > >> On 12 May 2015 at 15:10, Hante Meuleman <[email protected]> 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 [email protected] https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
