| Message ID | 20201125182010.23167-5-pali@kernel.org |
|---|---|
| State | Accepted |
| Commit | 061c6d1b238aa100728b3082fc943e8f679f8e7f |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | Use just one DTS file for all Espressobin variants | expand |
On 25/11/2020 19:20, Pali Rohár wrote: > Try to initialize emmc in board_late_init() and if it fails then we know > that emmc device is not connected. > > This allows to use in U-Boot just one DTS file for all Espressobin variants > and also to correctly set fdtfile env variable for Linux kernel. > > Signed-off-by: Pali Rohár <pali@kernel.org> > Tested-by: Gérald Kerma <gerald@gk2.net> Small nit below, with that: Reviewed-by: Andre Heider <a.heider@gmail.com> > --- > board/Marvell/mvebu_armada-37xx/board.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c > index 73d69e0388..f67b04b78c 100644 > --- a/board/Marvell/mvebu_armada-37xx/board.c > +++ b/board/Marvell/mvebu_armada-37xx/board.c > @@ -8,6 +8,7 @@ > #include <env.h> > #include <i2c.h> > #include <init.h> > +#include <mmc.h> > #include <phy.h> > #include <asm/io.h> > #include <asm/arch/cpu.h> > @@ -83,6 +84,7 @@ int board_init(void) > #ifdef CONFIG_BOARD_LATE_INIT > int board_late_init(void) > { > + struct mmc *mmc_dev; > bool ddr4, emmc; > > if (env_get("fdtfile")) > @@ -95,7 +97,9 @@ int board_late_init(void) > ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS) > & A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4; > > - emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); > + /* eMMC is mmc dev num 1 */ > + mmc_dev = find_mmc_device(1); > + emmc = (mmc_dev && mmc_init(mmc_dev) == 0); I think you meant "mmc_dev && (mmc_init(mmc_dev) == 0);"? > > if (ddr4 && emmc) > env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb"); >
On Wednesday 02 December 2020 15:35:08 Andre Heider wrote: > On 25/11/2020 19:20, Pali Rohár wrote: > > Try to initialize emmc in board_late_init() and if it fails then we know > > that emmc device is not connected. > > > > This allows to use in U-Boot just one DTS file for all Espressobin variants > > and also to correctly set fdtfile env variable for Linux kernel. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > Tested-by: Gérald Kerma <gerald@gk2.net> > > Small nit below, with that: > Reviewed-by: Andre Heider <a.heider@gmail.com> > > > --- > > board/Marvell/mvebu_armada-37xx/board.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c > > index 73d69e0388..f67b04b78c 100644 > > --- a/board/Marvell/mvebu_armada-37xx/board.c > > +++ b/board/Marvell/mvebu_armada-37xx/board.c > > @@ -8,6 +8,7 @@ > > #include <env.h> > > #include <i2c.h> > > #include <init.h> > > +#include <mmc.h> > > #include <phy.h> > > #include <asm/io.h> > > #include <asm/arch/cpu.h> > > @@ -83,6 +84,7 @@ int board_init(void) > > #ifdef CONFIG_BOARD_LATE_INIT > > int board_late_init(void) > > { > > + struct mmc *mmc_dev; > > bool ddr4, emmc; > > if (env_get("fdtfile")) > > @@ -95,7 +97,9 @@ int board_late_init(void) > > ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS) > > & A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4; > > - emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); > > + /* eMMC is mmc dev num 1 */ > > + mmc_dev = find_mmc_device(1); > > + emmc = (mmc_dev && mmc_init(mmc_dev) == 0); > > I think you meant "mmc_dev && (mmc_init(mmc_dev) == 0);"? Hm... no... I usually do not put parenthesis around || and && operators. But both variants are same right? If U-Boot coding style prefers second (your) variant, I do not have a problem to change it. Stefan, do you need to change this styling? > > if (ddr4 && emmc) > > env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb"); > > >
On 03.12.20 16:56, Pali Rohár wrote: > On Wednesday 02 December 2020 15:35:08 Andre Heider wrote: >> On 25/11/2020 19:20, Pali Rohár wrote: >>> Try to initialize emmc in board_late_init() and if it fails then we know >>> that emmc device is not connected. >>> >>> This allows to use in U-Boot just one DTS file for all Espressobin variants >>> and also to correctly set fdtfile env variable for Linux kernel. >>> >>> Signed-off-by: Pali Rohár <pali@kernel.org> >>> Tested-by: Gérald Kerma <gerald@gk2.net> >> >> Small nit below, with that: >> Reviewed-by: Andre Heider <a.heider@gmail.com> >> >>> --- >>> board/Marvell/mvebu_armada-37xx/board.c | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c >>> index 73d69e0388..f67b04b78c 100644 >>> --- a/board/Marvell/mvebu_armada-37xx/board.c >>> +++ b/board/Marvell/mvebu_armada-37xx/board.c >>> @@ -8,6 +8,7 @@ >>> #include <env.h> >>> #include <i2c.h> >>> #include <init.h> >>> +#include <mmc.h> >>> #include <phy.h> >>> #include <asm/io.h> >>> #include <asm/arch/cpu.h> >>> @@ -83,6 +84,7 @@ int board_init(void) >>> #ifdef CONFIG_BOARD_LATE_INIT >>> int board_late_init(void) >>> { >>> + struct mmc *mmc_dev; >>> bool ddr4, emmc; >>> if (env_get("fdtfile")) >>> @@ -95,7 +97,9 @@ int board_late_init(void) >>> ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS) >>> & A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4; >>> - emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); >>> + /* eMMC is mmc dev num 1 */ >>> + mmc_dev = find_mmc_device(1); >>> + emmc = (mmc_dev && mmc_init(mmc_dev) == 0); >> >> I think you meant "mmc_dev && (mmc_init(mmc_dev) == 0);"? > > Hm... no... I usually do not put parenthesis around || and && operators. > > But both variants are same right? Yours still has parenthesis around the (a && b) part, which is not needed. But this is nitpicking AFAIAC. > If U-Boot coding style prefers second (your) variant, I do not have > a problem to change it. checkpatch should complain if something does not match the coding style. > Stefan, do you need to change this styling? No, not from my point of view. Thanks, Stefan
On Thursday 03 December 2020 17:30:44 Stefan Roese wrote: > On 03.12.20 16:56, Pali Rohár wrote: > > On Wednesday 02 December 2020 15:35:08 Andre Heider wrote: > > > On 25/11/2020 19:20, Pali Rohár wrote: > > > > Try to initialize emmc in board_late_init() and if it fails then we know > > > > that emmc device is not connected. > > > > > > > > This allows to use in U-Boot just one DTS file for all Espressobin variants > > > > and also to correctly set fdtfile env variable for Linux kernel. > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > Tested-by: Gérald Kerma <gerald@gk2.net> > > > > > > Small nit below, with that: > > > Reviewed-by: Andre Heider <a.heider@gmail.com> > > > > > > > --- > > > > board/Marvell/mvebu_armada-37xx/board.c | 6 +++++- > > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c > > > > index 73d69e0388..f67b04b78c 100644 > > > > --- a/board/Marvell/mvebu_armada-37xx/board.c > > > > +++ b/board/Marvell/mvebu_armada-37xx/board.c > > > > @@ -8,6 +8,7 @@ > > > > #include <env.h> > > > > #include <i2c.h> > > > > #include <init.h> > > > > +#include <mmc.h> > > > > #include <phy.h> > > > > #include <asm/io.h> > > > > #include <asm/arch/cpu.h> > > > > @@ -83,6 +84,7 @@ int board_init(void) > > > > #ifdef CONFIG_BOARD_LATE_INIT > > > > int board_late_init(void) > > > > { > > > > + struct mmc *mmc_dev; > > > > bool ddr4, emmc; > > > > if (env_get("fdtfile")) > > > > @@ -95,7 +97,9 @@ int board_late_init(void) > > > > ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS) > > > > & A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4; > > > > - emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); > > > > + /* eMMC is mmc dev num 1 */ > > > > + mmc_dev = find_mmc_device(1); > > > > + emmc = (mmc_dev && mmc_init(mmc_dev) == 0); > > > > > > I think you meant "mmc_dev && (mmc_init(mmc_dev) == 0);"? > > > > Hm... no... I usually do not put parenthesis around || and && operators. > > > > But both variants are same right? > > Yours still has parenthesis around the (a && b) part, which is not > needed. But this is nitpicking AFAIAC. > > > If U-Boot coding style prefers second (your) variant, I do not have > > a problem to change it. > > checkpatch should complain if something does not match the coding style. > > > Stefan, do you need to change this styling? > > No, not from my point of view. Ok! So based on Philipp and Andre replies, would you be sending these patches to 2021.01 release? As for this release is scheduled usage of "globalscale,espressobin-emmc" compatible string in U-Boot, these patches are removing them and previous versions do not used them. > Thanks, > Stefan
On 05.12.20 13:44, Pali Rohár wrote: > On Thursday 03 December 2020 17:30:44 Stefan Roese wrote: >> On 03.12.20 16:56, Pali Rohár wrote: >>> On Wednesday 02 December 2020 15:35:08 Andre Heider wrote: >>>> On 25/11/2020 19:20, Pali Rohár wrote: >>>>> Try to initialize emmc in board_late_init() and if it fails then we know >>>>> that emmc device is not connected. >>>>> >>>>> This allows to use in U-Boot just one DTS file for all Espressobin variants >>>>> and also to correctly set fdtfile env variable for Linux kernel. >>>>> >>>>> Signed-off-by: Pali Rohár <pali@kernel.org> >>>>> Tested-by: Gérald Kerma <gerald@gk2.net> >>>> >>>> Small nit below, with that: >>>> Reviewed-by: Andre Heider <a.heider@gmail.com> >>>> >>>>> --- >>>>> board/Marvell/mvebu_armada-37xx/board.c | 6 +++++- >>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c >>>>> index 73d69e0388..f67b04b78c 100644 >>>>> --- a/board/Marvell/mvebu_armada-37xx/board.c >>>>> +++ b/board/Marvell/mvebu_armada-37xx/board.c >>>>> @@ -8,6 +8,7 @@ >>>>> #include <env.h> >>>>> #include <i2c.h> >>>>> #include <init.h> >>>>> +#include <mmc.h> >>>>> #include <phy.h> >>>>> #include <asm/io.h> >>>>> #include <asm/arch/cpu.h> >>>>> @@ -83,6 +84,7 @@ int board_init(void) >>>>> #ifdef CONFIG_BOARD_LATE_INIT >>>>> int board_late_init(void) >>>>> { >>>>> + struct mmc *mmc_dev; >>>>> bool ddr4, emmc; >>>>> if (env_get("fdtfile")) >>>>> @@ -95,7 +97,9 @@ int board_late_init(void) >>>>> ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS) >>>>> & A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4; >>>>> - emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); >>>>> + /* eMMC is mmc dev num 1 */ >>>>> + mmc_dev = find_mmc_device(1); >>>>> + emmc = (mmc_dev && mmc_init(mmc_dev) == 0); >>>> >>>> I think you meant "mmc_dev && (mmc_init(mmc_dev) == 0);"? >>> >>> Hm... no... I usually do not put parenthesis around || and && operators. >>> >>> But both variants are same right? >> >> Yours still has parenthesis around the (a && b) part, which is not >> needed. But this is nitpicking AFAIAC. >> >>> If U-Boot coding style prefers second (your) variant, I do not have >>> a problem to change it. >> >> checkpatch should complain if something does not match the coding style. >> >>> Stefan, do you need to change this styling? >> >> No, not from my point of view. > > Ok! So based on Philipp and Andre replies, would you be sending these > patches to 2021.01 release? As for this release is scheduled usage of > "globalscale,espressobin-emmc" compatible string in U-Boot, these > patches are removing them and previous versions do not used them. Yes, I'm preparing a pull request just now. Thanks, Stefan
diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c index 73d69e0388..f67b04b78c 100644 --- a/board/Marvell/mvebu_armada-37xx/board.c +++ b/board/Marvell/mvebu_armada-37xx/board.c @@ -8,6 +8,7 @@ #include <env.h> #include <i2c.h> #include <init.h> +#include <mmc.h> #include <phy.h> #include <asm/io.h> #include <asm/arch/cpu.h> @@ -83,6 +84,7 @@ int board_init(void) #ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { + struct mmc *mmc_dev; bool ddr4, emmc; if (env_get("fdtfile")) @@ -95,7 +97,9 @@ int board_late_init(void) ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS) & A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4; - emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); + /* eMMC is mmc dev num 1 */ + mmc_dev = find_mmc_device(1); + emmc = (mmc_dev && mmc_init(mmc_dev) == 0); if (ddr4 && emmc) env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");