| Message ID | 20220806013403.1344-1-william.zhang@broadcom.com |
|---|---|
| Headers | show |
| Series | arm: bcmbca: add basic support for more Broadcom BCA SoCs | expand |
On 8/5/2022 6:33 PM, William Zhang wrote: > Broadcom BCA (Broadband Carrier Access origin) chipset family includes > DSL, PON and WLAN access point and gateway SoC. Now that the ARCH_BCMBCA > architecture and its first SoC BCM47622 are supported in u-boot 2022.07, > this patch series add the basic support for following BCA chips under > ARCH_BCMBCA: BCM4908, BCM4912, BCM63146 and BCM6813. > > This patch series applies on top of the my previous patch [1]. > > [1] https://lists.denx.de/pipermail/u-boot/2022-August/491060.html Looks good to me, thanks William! On the mmu_table.c implementation maybe just a few nits: - should not we do an early parsing of the memory node for the given board(s) to ensure that we map no more than the amount of available DRAM? - the exact same file is currently being re-used, so it would make sense to make it a common object - you could create a memory mapping for the AXI bus region right away to avoid forgetting about it later if you start bringing up drivers that make use of that peripheral region Thanks!
Hi Florian, Thanks for the review! Please see my comments inline blow, On 08/08/2022 08:29 PM, Florian Fainelli wrote: > > > On 8/5/2022 6:33 PM, William Zhang wrote: >> Broadcom BCA (Broadband Carrier Access origin) chipset family includes >> DSL, PON and WLAN access point and gateway SoC. Now that the ARCH_BCMBCA >> architecture and its first SoC BCM47622 are supported in u-boot 2022.07, >> this patch series add the basic support for following BCA chips under >> ARCH_BCMBCA: BCM4908, BCM4912, BCM63146 and BCM6813. >> >> This patch series applies on top of the my previous patch [1]. >> >> [1] https://lists.denx.de/pipermail/u-boot/2022-August/491060.html > > Looks good to me, thanks William! On the mmu_table.c implementation > maybe just a few nits: > > - should not we do an early parsing of the memory node for the given > board(s) to ensure that we map no more than the amount of available DRAM? > Yes there will be a patch after all these SoC patches to set the ddr size during the dram_init based on the actually memory size. > - the exact same file is currently being re-used, so it would make sense > to make it a common object For these initial soc support patches, I just include the ddr and periph block and they happen to be the same range. But different SoC has different ip block address as we add more more blocks/drivers late. To avoid many ifdef, I would prefer to have one file per chip. > > - you could create a memory mapping for the AXI bus region right away to > avoid forgetting about it later if you start bringing up drivers that > make use of that peripheral region > Yeah I could but we won't forget either because system will crash if we miss that entry in the mmu table. IMHO it is better to limit the access than opening a wide range that we don't use. We can catch invalid/unintended access by only allowing the regions that we need to access. > Thanks!