Hi Ian, On 10 March 2015 at 04:30, Ian Kent <ra...@themaw.net> wrote: > Add new sprom revision 11 variables to the nvram -> sprom reader. > > Signed-off-by: Ian Kent <ra...@themaw.net>
There are few problems with bcm53xx's SPROM and I've few comments to your patch. bcm53xx's SPROM driver is modified version of mainline arch/mips/bcm74xx/sprom.c. It already differs a bit, but we can still see the differences and try to mainline them. By adding more changes to it we'll get lost and upsteaming changes will become more complex. 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. Also your changes to struct ssb_sprom may get complex to maintain, I believe you should try to upstream them. 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/ I'm really happy you worked on SPROM rev 11 properties, it would be great to get them as a patch for the bcm47xx's driver. > ++ 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. _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
