[OpenWrt-Devel] [PATCH 3/6] bcm53xx: update sprom from nvram to handle rev 11

[Rafał Miłecki]

On 2 April 2015 at 11:03, Ian Kent <raven at themaw.net> wrote:
> On Thu, 2015-04-02 at 10:22 +0200, Rafał Miłecki wrote:
>> Other than that, current way of handling revisions is quite messy, I
>> guess you noticed it by yourself. I started reworking, see:
>> http://patchwork.linux-mips.org/patch/9659/
>> Again, my change if for upstream driver.
>
> I've noticed a couple of changes you've submitted.
>
> I must admit they are interesting but not what I would have come up
> with. I guess it's my lack of experience with the code, but that'll come
> in time.

That's expected. When you look at driver for the first time, it's hard
to have a nice idea for redesigning it. Unless it's a real crap :P


>> So my idea to resolve this situation is to:
>> 1) sync bcm53xx SPROM driver with mainline one, let it differ only
>> with DT specific code
>> 2) keep submitting SPROM changes to the mainline driver and just
>> backport them to bcm53xx
>> 3) clear bcm47xx's sprom.c and work on moving it to the
>> drivers/firmware/broadcom/
>
> OK, just to clarify, your recommending the canonical source is the
> current bcm47xx and the goal is to make that generic and move it to a
> common location in the source tree.
>
> So I should familiarize myself with that source too.

You got me correct.. This is the same trick I did with bcm53xx NVRAM
driver. Some time ago it was also quite different from the bcm47xx
one. So I modified bcm47xx nvram.c (by sending upstream patches) to
make is usable by bcm53xx and then backported it to bcm53xx. Now the
drivers are identical according to my knowledge.

Few minutes ago I've updated bcm53xx SPROM driver to be based on bcm47xx's one:
https://dev.openwrt.org/changeset/45230/
so we are already doing pretty well with unifying them.
You can now work with bcm53xx's variation of SPROM driver and easily
port changes to the bcm47xx's sprom.c


>> > ++              entry_count = ARRAY_SIZE(gains_info->rxgains5gmelnagaina);
>> > ++              for (j = 0; j < entry_count; j++) {
>> > ++                      snprintf(postfix, sizeof(postfix), "%i", j);
>> > ++                      snprintf(tmp, sizeof(tmp), "%i:%s", i, "rxgains5gmelnagaina");
>> > ++                      nvram_read_u8(fill, postfix, tmp,
>> > ++                                    &gains_info->rxgains5gmelnagaina[j], 0);
>> > ++              }
>>
>> You shouldn't let unexpected NVRAM content crash your driver. Don't
>> trust the entry_count, verify it with your array size.
>
> Umm ... don't have my patch handy and don't quite follow.

Just ignore my silly comment, my brain got some memory corruption :|

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