From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Sat, 28 Mar 2020 02:00:50 +0100 Subject: [PATCH V3] ARM: dts: stm32: Add KS8851-16MLL ethernet on FMC2 In-Reply-To: <45a730af-61bd-663b-87f0-cc38cd994b8b@st.com> References: <20200326155815.69193-1-marex@denx.de> <45a730af-61bd-663b-87f0-cc38cd994b8b@st.com> 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 On 3/27/20 9:55 AM, Patrice CHOTARD wrote: > Hi Marek Hi, [...] >> diff --git a/board/dhelectronics/dh_stm32mp1/board.c b/board/dhelectronics/dh_stm32mp1/board.c >> index b663696983..be55242799 100644 >> --- a/board/dhelectronics/dh_stm32mp1/board.c >> +++ b/board/dhelectronics/dh_stm32mp1/board.c >> @@ -376,6 +376,32 @@ static void sysconf_init(void) >> #endif >> } >> >> +static void board_init_fmc2(void) >> +{ >> +#define STM32_FMC2_BCR1 0x0 >> +#define STM32_FMC2_BTR1 0x4 >> +#define STM32_FMC2_BWTR1 0x104 >> +#define STM32_FMC2_BCR(x) ((x) * 0x8 + STM32_FMC2_BCR1) >> +#define STM32_FMC2_BTR(x) ((x) * 0x8 + STM32_FMC2_BTR1) >> +#define STM32_FMC2_BWTR(x) ((x) * 0x8 + STM32_FMC2_BWTR1) > > > All these defines can be put in ./arch/arm/mach-stm32mp/include/mach/stm32.h No, we need a driver for FMC2, that's where they should go. This is a stopgap solution. >> +#define RCC_MP_AHB6RSTCLRR 0x218 >> +#define RCC_MP_AHB6ENSETR 0x19c >> + >> + /* Set up FMC2 bus for KS8851-16MLL and X11 SRAM */ >> + writel(BIT(12), STM32_RCC_BASE + RCC_MP_AHB6RSTCLRR); >> + writel(BIT(12), STM32_RCC_BASE + RCC_MP_AHB6ENSETR); >> + > > Add a define for AHB6RSTCLRR and AHB6ENSETR BIT(12) Maybe you should fix board/stm32mp1 and arch/arm/mach-stm32mp to do the same ? :) >> + /* KS8851-16MLL */ >> + writel(0x000010db, STM32_FMC2_BASE + STM32_FMC2_BCR(1)); >> + writel(0xc0022222, STM32_FMC2_BASE + STM32_FMC2_BTR(1)); >> + /* AS7C34098 SRAM on X11 */ >> + writel(0x000010db, STM32_FMC2_BASE + STM32_FMC2_BCR(3)); >> + writel(0xc0022222, STM32_FMC2_BASE + STM32_FMC2_BTR(3)); >> + > > Avoid to put hardcoded value, it difficult to see what is done, add defines. > >> + setbits_le32(STM32_FMC2_BASE + STM32_FMC2_BCR1, BIT(31)); > > Add a define for FMC2_BCR1 BIT(31) , it will be easier to understand what is done OK