From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Fri, 31 Jul 2020 12:44:01 -0600 Subject: [PATCH v1 13/24] arm: octeontx: Add headers for OcteonTX In-Reply-To: References: <20200724100856.1482324-1-sr@denx.de> <20200724100856.1482324-14-sr@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de HI Stefan, On Fri, 31 Jul 2020 at 08:21, Stefan Roese wrote: > > Hi Simon, > > On 28.07.20 21:01, Simon Glass wrote: > > Hi Stefan, > > > > On Fri, 24 Jul 2020 at 04:09, Stefan Roese wrote: > >> > >> From: Suneel Garapati > >> > >> Signed-off-by: Suneel Garapati > >> > >> Signed-off-by: Stefan Roese > >> --- > >> > >> 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 > > > > 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. > Seems OK to me. Regards, Simon