Hi Simon,
On 28.07.20 21:01, Simon Glass wrote:
Hi Stefan,
On Fri, 24 Jul 2020 at 04:09, Stefan Roese sr@denx.de wrote:
From: Suneel Garapati sgarapati@marvell.com
Signed-off-by: Suneel Garapati sgarapati@marvell.com
Signed-off-by: Stefan Roese sr@denx.de
Changes in v1:
Change patch subject
arch/arm/include/asm/arch-octeontx/board.h | 123 ++ arch/arm/include/asm/arch-octeontx/clock.h | 25 + .../asm/arch-octeontx/csrs/csrs-mio_emm.h | 1193 +++++++++++++++++ .../include/asm/arch-octeontx/csrs/csrs-xcv.h | 428 ++++++ arch/arm/include/asm/arch-octeontx/gpio.h | 6 + arch/arm/include/asm/arch-octeontx/smc.h | 20 + arch/arm/include/asm/arch-octeontx/soc.h | 33 + 7 files changed, 1828 insertions(+) create mode 100644 arch/arm/include/asm/arch-octeontx/board.h create mode 100644 arch/arm/include/asm/arch-octeontx/clock.h create mode 100644 arch/arm/include/asm/arch-octeontx/csrs/csrs-mio_emm.h create mode 100644 arch/arm/include/asm/arch-octeontx/csrs/csrs-xcv.h create mode 100644 arch/arm/include/asm/arch-octeontx/gpio.h create mode 100644 arch/arm/include/asm/arch-octeontx/smc.h create mode 100644 arch/arm/include/asm/arch-octeontx/soc.h
Reviewed-by: Simon Glass sjg@chromium.org
Generic thoughts to consider:
- drop extra brackets around constants - e.g. MIO_EMM_BAR_E_MIO_EMM_PF_BAR4
- use #define or enum instead of inline functions, e.g. MIO_EMM_DMA
- lower-case hex
Yes, thanks. I agree. Let me check, if I can find some time to clean this up a bit more.
I don't normally see bitfields in U-Boot. Is that a good idea?
It is not, I agree. I've been bitten by those bitfields in the other Octeon drivers before (I2C, SPI etc). Bitfields make portable driver code quite hard. You need to add separate bitfield definitions for big- endian and little-endian support. So I switched on the common drivers, and with common I mean drivers supporting MIPS Octeon and ARM Octeon TX/TX2, from using bitfields to BIT_ULL() / GENMASK_ULL etc. This works just fine with big- and little-endian systems.
But, as you probably have guessed, these header are auto-generated and changing this to BIT_ULL etc is really painful (time consuming) and also potentially error prone. So I really would like to stay with the bitfields for those structs / variables, that are solely used by the Octeon TX/TX2 (little-endian) platforms.
I hope this is okay.
Thanks, Stefan