[PATCH v1 13/24] arm: octeontx: Add headers for OcteonTX

[Stefan Roese]

Hi Simon,

On 28.07.20 21:01, Simon Glass wrote:
> Hi Stefan,
> 
> On Fri, 24 Jul 2020 at 04:09, Stefan Roese <sr at denx.de> wrote:
>>
>> From: Suneel Garapati <sgarapati at marvell.com>
>>
>> Signed-off-by: Suneel Garapati <sgarapati at marvell.com>
>>
>> Signed-off-by: Stefan Roese <sr at 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 at 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