All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] mtd: rawnand: denali-spl: Add missing hardware init
@ 2020-01-09 10:07 Marek Vasut
  2020-01-09 10:07 ` [PATCH 2/3] mtd: rawnand: denali: Allow operation without clock driver Marek Vasut
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Marek Vasut @ 2020-01-09 10:07 UTC (permalink / raw)
  To: u-boot

While the Denali NAND is initialized by the BootROM in SPL, there
are still a couple of settings which are missing. These can trigger
subtle corruption of the data read out of the NAND. Fill these
settings in just like they are filled in by the full Denali NAND
driver in denali_hw_init().

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
---
 drivers/mtd/nand/raw/denali_spl.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/nand/raw/denali_spl.c b/drivers/mtd/nand/raw/denali_spl.c
index dbaba3cab2..b8b29812aa 100644
--- a/drivers/mtd/nand/raw/denali_spl.c
+++ b/drivers/mtd/nand/raw/denali_spl.c
@@ -173,6 +173,13 @@ void nand_init(void)
 	page_size = readl(denali_flash_reg + DEVICE_MAIN_AREA_SIZE);
 	oob_size = readl(denali_flash_reg + DEVICE_SPARE_AREA_SIZE);
 	pages_per_block = readl(denali_flash_reg + PAGES_PER_BLOCK);
+
+	/* Do as denali_hw_init() does. */
+	writel(CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES,
+	       denali_flash_reg + SPARE_AREA_SKIP_BYTES);
+	writel(0x0F, denali_flash_reg + RB_PIN_ENABLED);
+	writel(CHIP_EN_DONT_CARE__FLAG, denali_flash_reg + CHIP_ENABLE_DONT_CARE);
+	writel(0xffff, denali_flash_reg + SPARE_AREA_MARKER);
 }
 
 int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
-- 
2.24.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] mtd: rawnand: denali: Allow operation without clock driver
  2020-01-09 10:07 [PATCH 1/3] mtd: rawnand: denali-spl: Add missing hardware init Marek Vasut
@ 2020-01-09 10:07 ` Marek Vasut
  2020-01-09 11:02   ` Masahiro Yamada
  2020-01-09 10:07 ` [PATCH 3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA Marek Vasut
  2020-01-09 11:29 ` [PATCH 1/3] mtd: rawnand: denali-spl: Add missing hardware init Masahiro Yamada
  2 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2020-01-09 10:07 UTC (permalink / raw)
  To: u-boot

The SoCFPGA Gen5 does not have a clock driver yet, let the NAND driver
work without a clock driver by falling back to the default frequencies.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
---
 drivers/mtd/nand/raw/denali_dt.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
index 0ce81324b9..2c9e249ab6 100644
--- a/drivers/mtd/nand/raw/denali_dt.c
+++ b/drivers/mtd/nand/raw/denali_dt.c
@@ -62,7 +62,6 @@ static int denali_dt_probe(struct udevice *dev)
 {
 	struct denali_nand_info *denali = dev_get_priv(dev);
 	const struct denali_dt_data *data;
-	struct clk clk, clk_x, clk_ecc;
 	struct resource res;
 	int ret;
 
@@ -87,11 +86,14 @@ static int denali_dt_probe(struct udevice *dev)
 
 	denali->host = devm_ioremap(dev, res.start, resource_size(&res));
 
+#if CONFIG_IS_ENABLED(CLK)
+	struct clk clk, clk_x, clk_ecc;
+
 	ret = clk_get_by_name(dev, "nand", &clk);
 	if (ret)
 		ret = clk_get_by_index(dev, 0, &clk);
 	if (ret)
-		return ret;
+		clk.dev = NULL;
 
 	ret = clk_get_by_name(dev, "nand_x", &clk_x);
 	if (ret)
@@ -117,10 +119,12 @@ static int denali_dt_probe(struct udevice *dev)
 			return ret;
 	}
 
-	if (clk_x.dev) {
+	if (clk.dev && clk_x.dev) {
 		denali->clk_rate = clk_get_rate(&clk);
 		denali->clk_x_rate = clk_get_rate(&clk_x);
-	} else {
+	} else
+#endif
+	{
 		/*
 		 * Hardcode the clock rates for the backward compatibility.
 		 * This works for both SOCFPGA and UniPhier.
-- 
2.24.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA
  2020-01-09 10:07 [PATCH 1/3] mtd: rawnand: denali-spl: Add missing hardware init Marek Vasut
  2020-01-09 10:07 ` [PATCH 2/3] mtd: rawnand: denali: Allow operation without clock driver Marek Vasut
@ 2020-01-09 10:07 ` Marek Vasut
  2020-01-09 11:04   ` Masahiro Yamada
  2020-01-09 11:29 ` [PATCH 1/3] mtd: rawnand: denali-spl: Add missing hardware init Masahiro Yamada
  2 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2020-01-09 10:07 UTC (permalink / raw)
  To: u-boot

Legacy kernel versions for SoCFPGA may not implement proper reset
handling. Apply the same approach as SoCFPGA reset driver, check
environment variable "socfpga_legacy_reset_compat", and if it is
set, do not reset the IP before booting Linux. This way, even the
older kernel versions can be booted by up to date U-Boot.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
---
 drivers/mtd/nand/raw/denali_dt.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
index 2c9e249ab6..d35f2a3543 100644
--- a/drivers/mtd/nand/raw/denali_dt.c
+++ b/drivers/mtd/nand/raw/denali_dt.c
@@ -148,6 +148,18 @@ static int denali_dt_remove(struct udevice *dev)
 {
 	struct denali_nand_info *denali = dev_get_priv(dev);
 
+#if CONFIG_IS_ENABLED(ARCH_SOCFPGA)
+	/*
+	 * Legacy kernel versions do not implement proper reset handling on
+	 * SoCFPGA. To let those older kernel versions work, reuse the same
+	 * approach as the SoCFPGA reset driver does -- check environment
+	 * variable socfpga_legacy_reset_compat and avoid resetting the IP
+	 * before booting the kernel if it is set to 1.
+	 */
+	if (env_get_ulong("socfpga_legacy_reset_compat", 10, 0))
+		return 0;
+#endif
+
 	return reset_release_bulk(&denali->resets);
 }
 
-- 
2.24.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] mtd: rawnand: denali: Allow operation without clock driver
  2020-01-09 10:07 ` [PATCH 2/3] mtd: rawnand: denali: Allow operation without clock driver Marek Vasut
@ 2020-01-09 11:02   ` Masahiro Yamada
  2020-01-09 11:14     ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Masahiro Yamada @ 2020-01-09 11:02 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>
> The SoCFPGA Gen5 does not have a clock driver yet, let the NAND driver
> work without a clock driver by falling back to the default frequencies.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>


Why do you need this?

denali_dt_probe() succeeds without CONFIG_CLK, doesn't it?



> ---
>  drivers/mtd/nand/raw/denali_dt.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
> index 0ce81324b9..2c9e249ab6 100644
> --- a/drivers/mtd/nand/raw/denali_dt.c
> +++ b/drivers/mtd/nand/raw/denali_dt.c
> @@ -62,7 +62,6 @@ static int denali_dt_probe(struct udevice *dev)
>  {
>         struct denali_nand_info *denali = dev_get_priv(dev);
>         const struct denali_dt_data *data;
> -       struct clk clk, clk_x, clk_ecc;
>         struct resource res;
>         int ret;
>
> @@ -87,11 +86,14 @@ static int denali_dt_probe(struct udevice *dev)
>
>         denali->host = devm_ioremap(dev, res.start, resource_size(&res));
>
> +#if CONFIG_IS_ENABLED(CLK)
> +       struct clk clk, clk_x, clk_ecc;
> +
>         ret = clk_get_by_name(dev, "nand", &clk);
>         if (ret)
>                 ret = clk_get_by_index(dev, 0, &clk);
>         if (ret)
> -               return ret;
> +               clk.dev = NULL;
>
>         ret = clk_get_by_name(dev, "nand_x", &clk_x);
>         if (ret)
> @@ -117,10 +119,12 @@ static int denali_dt_probe(struct udevice *dev)
>                         return ret;
>         }
>
> -       if (clk_x.dev) {
> +       if (clk.dev && clk_x.dev) {
>                 denali->clk_rate = clk_get_rate(&clk);
>                 denali->clk_x_rate = clk_get_rate(&clk_x);
> -       } else {
> +       } else
> +#endif
> +       {
>                 /*
>                  * Hardcode the clock rates for the backward compatibility.
>                  * This works for both SOCFPGA and UniPhier.
> --
> 2.24.1
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA
  2020-01-09 10:07 ` [PATCH 3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA Marek Vasut
@ 2020-01-09 11:04   ` Masahiro Yamada
  2020-01-09 11:15     ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Masahiro Yamada @ 2020-01-09 11:04 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>
> Legacy kernel versions for SoCFPGA may not implement proper reset
> handling. Apply the same approach as SoCFPGA reset driver, check
> environment variable "socfpga_legacy_reset_compat", and if it is
> set, do not reset the IP before booting Linux. This way, even the
> older kernel versions can be booted by up to date U-Boot.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>  drivers/mtd/nand/raw/denali_dt.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
> index 2c9e249ab6..d35f2a3543 100644
> --- a/drivers/mtd/nand/raw/denali_dt.c
> +++ b/drivers/mtd/nand/raw/denali_dt.c
> @@ -148,6 +148,18 @@ static int denali_dt_remove(struct udevice *dev)
>  {
>         struct denali_nand_info *denali = dev_get_priv(dev);
>
> +#if CONFIG_IS_ENABLED(ARCH_SOCFPGA)
> +       /*
> +        * Legacy kernel versions do not implement proper reset handling on
> +        * SoCFPGA. To let those older kernel versions work, reuse the same
> +        * approach as the SoCFPGA reset driver does -- check environment
> +        * variable socfpga_legacy_reset_compat and avoid resetting the IP
> +        * before booting the kernel if it is set to 1.
> +        */
> +       if (env_get_ulong("socfpga_legacy_reset_compat", 10, 0))
> +               return 0;
> +#endif
> +
>         return reset_release_bulk(&denali->resets);
>  }


Why don't you simply remove reset_release_bulk() ?

>
> --
> 2.24.1
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] mtd: rawnand: denali: Allow operation without clock driver
  2020-01-09 11:02   ` Masahiro Yamada
@ 2020-01-09 11:14     ` Marek Vasut
  2020-01-09 12:04       ` Masahiro Yamada
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2020-01-09 11:14 UTC (permalink / raw)
  To: u-boot

On 1/9/20 12:02 PM, Masahiro Yamada wrote:
> On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>>
>> The SoCFPGA Gen5 does not have a clock driver yet, let the NAND driver
>> work without a clock driver by falling back to the default frequencies.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> 
> 
> Why do you need this?
> 
> denali_dt_probe() succeeds without CONFIG_CLK, doesn't it?

No, if you don't have the NAND clock, this fails.
This could be reverted once there is DM/DT capable clock driver for
SoCFPGA Gen5, until then this is needed.

[...]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA
  2020-01-09 11:04   ` Masahiro Yamada
@ 2020-01-09 11:15     ` Marek Vasut
  2020-01-09 12:11       ` Masahiro Yamada
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2020-01-09 11:15 UTC (permalink / raw)
  To: u-boot

On 1/9/20 12:04 PM, Masahiro Yamada wrote:
> On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>>
>> Legacy kernel versions for SoCFPGA may not implement proper reset
>> handling. Apply the same approach as SoCFPGA reset driver, check
>> environment variable "socfpga_legacy_reset_compat", and if it is
>> set, do not reset the IP before booting Linux. This way, even the
>> older kernel versions can be booted by up to date U-Boot.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>  drivers/mtd/nand/raw/denali_dt.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
>> index 2c9e249ab6..d35f2a3543 100644
>> --- a/drivers/mtd/nand/raw/denali_dt.c
>> +++ b/drivers/mtd/nand/raw/denali_dt.c
>> @@ -148,6 +148,18 @@ static int denali_dt_remove(struct udevice *dev)
>>  {
>>         struct denali_nand_info *denali = dev_get_priv(dev);
>>
>> +#if CONFIG_IS_ENABLED(ARCH_SOCFPGA)
>> +       /*
>> +        * Legacy kernel versions do not implement proper reset handling on
>> +        * SoCFPGA. To let those older kernel versions work, reuse the same
>> +        * approach as the SoCFPGA reset driver does -- check environment
>> +        * variable socfpga_legacy_reset_compat and avoid resetting the IP
>> +        * before booting the kernel if it is set to 1.
>> +        */
>> +       if (env_get_ulong("socfpga_legacy_reset_compat", 10, 0))
>> +               return 0;
>> +#endif
>> +
>>         return reset_release_bulk(&denali->resets);
>>  }
> 
> 
> Why don't you simply remove reset_release_bulk() ?

Because once mainline has proper reset handling, this can be used only
for legacy kernel versions.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] mtd: rawnand: denali-spl: Add missing hardware init
  2020-01-09 10:07 [PATCH 1/3] mtd: rawnand: denali-spl: Add missing hardware init Marek Vasut
  2020-01-09 10:07 ` [PATCH 2/3] mtd: rawnand: denali: Allow operation without clock driver Marek Vasut
  2020-01-09 10:07 ` [PATCH 3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA Marek Vasut
@ 2020-01-09 11:29 ` Masahiro Yamada
  2020-01-09 13:18   ` Marek Vasut
  2 siblings, 1 reply; 15+ messages in thread
From: Masahiro Yamada @ 2020-01-09 11:29 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>
> While the Denali NAND is initialized by the BootROM in SPL, there
> are still a couple of settings which are missing. These can trigger
> subtle corruption of the data read out of the NAND. Fill these
> settings in just like they are filled in by the full Denali NAND
> driver in denali_hw_init().
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>  drivers/mtd/nand/raw/denali_spl.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/denali_spl.c b/drivers/mtd/nand/raw/denali_spl.c
> index dbaba3cab2..b8b29812aa 100644
> --- a/drivers/mtd/nand/raw/denali_spl.c
> +++ b/drivers/mtd/nand/raw/denali_spl.c
> @@ -173,6 +173,13 @@ void nand_init(void)
>         page_size = readl(denali_flash_reg + DEVICE_MAIN_AREA_SIZE);
>         oob_size = readl(denali_flash_reg + DEVICE_SPARE_AREA_SIZE);
>         pages_per_block = readl(denali_flash_reg + PAGES_PER_BLOCK);
> +
> +       /* Do as denali_hw_init() does. */
> +       writel(CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES,
> +              denali_flash_reg + SPARE_AREA_SKIP_BYTES);
> +       writel(0x0F, denali_flash_reg + RB_PIN_ENABLED);
> +       writel(CHIP_EN_DONT_CARE__FLAG, denali_flash_reg + CHIP_ENABLE_DONT_CARE);
> +       writel(0xffff, denali_flash_reg + SPARE_AREA_MARKER);
>  }
>

You need this because SOCFPGA SPL resets the nand controller, correct?

https://github.com/u-boot/u-boot/blob/v2020.01/arch/arm/mach-socfpga/spl_gen5.c#L89




--
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] mtd: rawnand: denali: Allow operation without clock driver
  2020-01-09 11:14     ` Marek Vasut
@ 2020-01-09 12:04       ` Masahiro Yamada
  2020-01-09 13:21         ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Masahiro Yamada @ 2020-01-09 12:04 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 9, 2020 at 8:16 PM Marek Vasut <marex@denx.de> wrote:
>
> On 1/9/20 12:02 PM, Masahiro Yamada wrote:
> > On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> The SoCFPGA Gen5 does not have a clock driver yet, let the NAND driver
> >> work without a clock driver by falling back to the default frequencies.
> >>
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> >
> >
> > Why do you need this?
> >
> > denali_dt_probe() succeeds without CONFIG_CLK, doesn't it?
>
> No, if you don't have the NAND clock, this fails.
> This could be reverted once there is DM/DT capable clock driver for
> SoCFPGA Gen5, until then this is needed.

Oh, sorry, I read the code in a wrong way.
But, this ifdef is ugly.

How about this alternative one?
http://patchwork.ozlabs.org/patch/1220337/



--
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA
  2020-01-09 11:15     ` Marek Vasut
@ 2020-01-09 12:11       ` Masahiro Yamada
  2020-01-09 14:25         ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Masahiro Yamada @ 2020-01-09 12:11 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 9, 2020 at 8:16 PM Marek Vasut <marex@denx.de> wrote:
>
> On 1/9/20 12:04 PM, Masahiro Yamada wrote:
> > On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> Legacy kernel versions for SoCFPGA may not implement proper reset
> >> handling. Apply the same approach as SoCFPGA reset driver, check
> >> environment variable "socfpga_legacy_reset_compat", and if it is
> >> set, do not reset the IP before booting Linux. This way, even the
> >> older kernel versions can be booted by up to date U-Boot.
> >>
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> >> ---
> >>  drivers/mtd/nand/raw/denali_dt.c | 12 ++++++++++++
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
> >> index 2c9e249ab6..d35f2a3543 100644
> >> --- a/drivers/mtd/nand/raw/denali_dt.c
> >> +++ b/drivers/mtd/nand/raw/denali_dt.c
> >> @@ -148,6 +148,18 @@ static int denali_dt_remove(struct udevice *dev)
> >>  {
> >>         struct denali_nand_info *denali = dev_get_priv(dev);
> >>
> >> +#if CONFIG_IS_ENABLED(ARCH_SOCFPGA)
> >> +       /*
> >> +        * Legacy kernel versions do not implement proper reset handling on
> >> +        * SoCFPGA. To let those older kernel versions work, reuse the same
> >> +        * approach as the SoCFPGA reset driver does -- check environment
> >> +        * variable socfpga_legacy_reset_compat and avoid resetting the IP
> >> +        * before booting the kernel if it is set to 1.
> >> +        */
> >> +       if (env_get_ulong("socfpga_legacy_reset_compat", 10, 0))
> >> +               return 0;
> >> +#endif
> >> +
> >>         return reset_release_bulk(&denali->resets);
> >>  }
> >
> >
> > Why don't you simply remove reset_release_bulk() ?
>
> Because once mainline has proper reset handling, this can be used only
> for legacy kernel versions.


I am suffering for the same problem for the uniphier platform.

The reset driver support for Linux will never be back-ported.
So, it will continue to be a problem for Linux v4.19, v5.4, etc.

reset_release_bulk() should go away.


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] mtd: rawnand: denali-spl: Add missing hardware init
  2020-01-09 11:29 ` [PATCH 1/3] mtd: rawnand: denali-spl: Add missing hardware init Masahiro Yamada
@ 2020-01-09 13:18   ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2020-01-09 13:18 UTC (permalink / raw)
  To: u-boot

On 1/9/20 12:29 PM, Masahiro Yamada wrote:
> On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>>
>> While the Denali NAND is initialized by the BootROM in SPL, there
>> are still a couple of settings which are missing. These can trigger
>> subtle corruption of the data read out of the NAND. Fill these
>> settings in just like they are filled in by the full Denali NAND
>> driver in denali_hw_init().
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>  drivers/mtd/nand/raw/denali_spl.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/raw/denali_spl.c b/drivers/mtd/nand/raw/denali_spl.c
>> index dbaba3cab2..b8b29812aa 100644
>> --- a/drivers/mtd/nand/raw/denali_spl.c
>> +++ b/drivers/mtd/nand/raw/denali_spl.c
>> @@ -173,6 +173,13 @@ void nand_init(void)
>>         page_size = readl(denali_flash_reg + DEVICE_MAIN_AREA_SIZE);
>>         oob_size = readl(denali_flash_reg + DEVICE_SPARE_AREA_SIZE);
>>         pages_per_block = readl(denali_flash_reg + PAGES_PER_BLOCK);
>> +
>> +       /* Do as denali_hw_init() does. */
>> +       writel(CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES,
>> +              denali_flash_reg + SPARE_AREA_SKIP_BYTES);
>> +       writel(0x0F, denali_flash_reg + RB_PIN_ENABLED);
>> +       writel(CHIP_EN_DONT_CARE__FLAG, denali_flash_reg + CHIP_ENABLE_DONT_CARE);
>> +       writel(0xffff, denali_flash_reg + SPARE_AREA_MARKER);
>>  }
>>
> 
> You need this because SOCFPGA SPL resets the nand controller, correct?
> 
> https://github.com/u-boot/u-boot/blob/v2020.01/arch/arm/mach-socfpga/spl_gen5.c#L89

Yes. The driver should init the hardware correctly and fully, which it
does not do without this patch.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] mtd: rawnand: denali: Allow operation without clock driver
  2020-01-09 12:04       ` Masahiro Yamada
@ 2020-01-09 13:21         ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2020-01-09 13:21 UTC (permalink / raw)
  To: u-boot

On 1/9/20 1:04 PM, Masahiro Yamada wrote:
> On Thu, Jan 9, 2020 at 8:16 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 1/9/20 12:02 PM, Masahiro Yamada wrote:
>>> On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> The SoCFPGA Gen5 does not have a clock driver yet, let the NAND driver
>>>> work without a clock driver by falling back to the default frequencies.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>
>>>
>>> Why do you need this?
>>>
>>> denali_dt_probe() succeeds without CONFIG_CLK, doesn't it?
>>
>> No, if you don't have the NAND clock, this fails.
>> This could be reverted once there is DM/DT capable clock driver for
>> SoCFPGA Gen5, until then this is needed.
> 
> Oh, sorry, I read the code in a wrong way.
> But, this ifdef is ugly.
> 
> How about this alternative one?
> http://patchwork.ozlabs.org/patch/1220337/

The clock code takes space for no reason in that patch, I prefer this
one to keep the size in check.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA
  2020-01-09 12:11       ` Masahiro Yamada
@ 2020-01-09 14:25         ` Marek Vasut
  2020-01-09 15:01           ` Masahiro Yamada
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2020-01-09 14:25 UTC (permalink / raw)
  To: u-boot

On 1/9/20 1:11 PM, Masahiro Yamada wrote:
> On Thu, Jan 9, 2020 at 8:16 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 1/9/20 12:04 PM, Masahiro Yamada wrote:
>>> On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> Legacy kernel versions for SoCFPGA may not implement proper reset
>>>> handling. Apply the same approach as SoCFPGA reset driver, check
>>>> environment variable "socfpga_legacy_reset_compat", and if it is
>>>> set, do not reset the IP before booting Linux. This way, even the
>>>> older kernel versions can be booted by up to date U-Boot.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>> ---
>>>>  drivers/mtd/nand/raw/denali_dt.c | 12 ++++++++++++
>>>>  1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
>>>> index 2c9e249ab6..d35f2a3543 100644
>>>> --- a/drivers/mtd/nand/raw/denali_dt.c
>>>> +++ b/drivers/mtd/nand/raw/denali_dt.c
>>>> @@ -148,6 +148,18 @@ static int denali_dt_remove(struct udevice *dev)
>>>>  {
>>>>         struct denali_nand_info *denali = dev_get_priv(dev);
>>>>
>>>> +#if CONFIG_IS_ENABLED(ARCH_SOCFPGA)
>>>> +       /*
>>>> +        * Legacy kernel versions do not implement proper reset handling on
>>>> +        * SoCFPGA. To let those older kernel versions work, reuse the same
>>>> +        * approach as the SoCFPGA reset driver does -- check environment
>>>> +        * variable socfpga_legacy_reset_compat and avoid resetting the IP
>>>> +        * before booting the kernel if it is set to 1.
>>>> +        */
>>>> +       if (env_get_ulong("socfpga_legacy_reset_compat", 10, 0))
>>>> +               return 0;
>>>> +#endif
>>>> +
>>>>         return reset_release_bulk(&denali->resets);
>>>>  }
>>>
>>>
>>> Why don't you simply remove reset_release_bulk() ?
>>
>> Because once mainline has proper reset handling, this can be used only
>> for legacy kernel versions.
> 
> 
> I am suffering for the same problem for the uniphier platform.

The Denali NAND patches for Linux are currently stuck on your review too.

> The reset driver support for Linux will never be back-ported.
> So, it will continue to be a problem for Linux v4.19, v5.4, etc.
> 
> reset_release_bulk() should go away.

Once the Linux patches are upstream, we can just reset the NAND
controller without problems. And that's the right thing to do, sometimes
you don't want NAND to be available in Linux, and then it should be kept
in reset.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA
  2020-01-09 14:25         ` Marek Vasut
@ 2020-01-09 15:01           ` Masahiro Yamada
  2020-01-10  0:10             ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Masahiro Yamada @ 2020-01-09 15:01 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 9, 2020 at 11:48 PM Marek Vasut <marex@denx.de> wrote:
>
> On 1/9/20 1:11 PM, Masahiro Yamada wrote:
> > On Thu, Jan 9, 2020 at 8:16 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 1/9/20 12:04 PM, Masahiro Yamada wrote:
> >>> On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
> >>>>
> >>>> Legacy kernel versions for SoCFPGA may not implement proper reset
> >>>> handling. Apply the same approach as SoCFPGA reset driver, check
> >>>> environment variable "socfpga_legacy_reset_compat", and if it is
> >>>> set, do not reset the IP before booting Linux. This way, even the
> >>>> older kernel versions can be booted by up to date U-Boot.
> >>>>
> >>>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> >>>> ---
> >>>>  drivers/mtd/nand/raw/denali_dt.c | 12 ++++++++++++
> >>>>  1 file changed, 12 insertions(+)
> >>>>
> >>>> diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
> >>>> index 2c9e249ab6..d35f2a3543 100644
> >>>> --- a/drivers/mtd/nand/raw/denali_dt.c
> >>>> +++ b/drivers/mtd/nand/raw/denali_dt.c
> >>>> @@ -148,6 +148,18 @@ static int denali_dt_remove(struct udevice *dev)
> >>>>  {
> >>>>         struct denali_nand_info *denali = dev_get_priv(dev);
> >>>>
> >>>> +#if CONFIG_IS_ENABLED(ARCH_SOCFPGA)
> >>>> +       /*
> >>>> +        * Legacy kernel versions do not implement proper reset handling on
> >>>> +        * SoCFPGA. To let those older kernel versions work, reuse the same
> >>>> +        * approach as the SoCFPGA reset driver does -- check environment
> >>>> +        * variable socfpga_legacy_reset_compat and avoid resetting the IP
> >>>> +        * before booting the kernel if it is set to 1.
> >>>> +        */
> >>>> +       if (env_get_ulong("socfpga_legacy_reset_compat", 10, 0))
> >>>> +               return 0;
> >>>> +#endif
> >>>> +
> >>>>         return reset_release_bulk(&denali->resets);
> >>>>  }
> >>>
> >>>
> >>> Why don't you simply remove reset_release_bulk() ?
> >>
> >> Because once mainline has proper reset handling, this can be used only
> >> for legacy kernel versions.
> >
> >
> > I am suffering for the same problem for the uniphier platform.
>
> The Denali NAND patches for Linux are currently stuck on your review too.

Huh?

Which patch in Linux is stuck on my review?



> > The reset driver support for Linux will never be back-ported.
> > So, it will continue to be a problem for Linux v4.19, v5.4, etc.
> >
> > reset_release_bulk() should go away.
>
> Once the Linux patches are upstream, we can just reset the NAND
> controller without problems. And that's the right thing to do, sometimes
> you don't want NAND to be available in Linux, and then it should be kept
> in reset.


The NAND reset in Linux will be available in the future version
(if my patch is applied).

The stable kernels are maintained in 6 years.

https://www.kernel.org/category/releases.html

The EOF of Linux 5.4 is set at 2021.12, but
following the recent rules, it will be extended
to 2026 or so.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA
  2020-01-09 15:01           ` Masahiro Yamada
@ 2020-01-10  0:10             ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2020-01-10  0:10 UTC (permalink / raw)
  To: u-boot

On 1/9/20 4:01 PM, Masahiro Yamada wrote:
> On Thu, Jan 9, 2020 at 11:48 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 1/9/20 1:11 PM, Masahiro Yamada wrote:
>>> On Thu, Jan 9, 2020 at 8:16 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 1/9/20 12:04 PM, Masahiro Yamada wrote:
>>>>> On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>>>>>>
>>>>>> Legacy kernel versions for SoCFPGA may not implement proper reset
>>>>>> handling. Apply the same approach as SoCFPGA reset driver, check
>>>>>> environment variable "socfpga_legacy_reset_compat", and if it is
>>>>>> set, do not reset the IP before booting Linux. This way, even the
>>>>>> older kernel versions can be booted by up to date U-Boot.
>>>>>>
>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>>>> ---
>>>>>>  drivers/mtd/nand/raw/denali_dt.c | 12 ++++++++++++
>>>>>>  1 file changed, 12 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
>>>>>> index 2c9e249ab6..d35f2a3543 100644
>>>>>> --- a/drivers/mtd/nand/raw/denali_dt.c
>>>>>> +++ b/drivers/mtd/nand/raw/denali_dt.c
>>>>>> @@ -148,6 +148,18 @@ static int denali_dt_remove(struct udevice *dev)
>>>>>>  {
>>>>>>         struct denali_nand_info *denali = dev_get_priv(dev);
>>>>>>
>>>>>> +#if CONFIG_IS_ENABLED(ARCH_SOCFPGA)
>>>>>> +       /*
>>>>>> +        * Legacy kernel versions do not implement proper reset handling on
>>>>>> +        * SoCFPGA. To let those older kernel versions work, reuse the same
>>>>>> +        * approach as the SoCFPGA reset driver does -- check environment
>>>>>> +        * variable socfpga_legacy_reset_compat and avoid resetting the IP
>>>>>> +        * before booting the kernel if it is set to 1.
>>>>>> +        */
>>>>>> +       if (env_get_ulong("socfpga_legacy_reset_compat", 10, 0))
>>>>>> +               return 0;
>>>>>> +#endif
>>>>>> +
>>>>>>         return reset_release_bulk(&denali->resets);
>>>>>>  }
>>>>>
>>>>>
>>>>> Why don't you simply remove reset_release_bulk() ?
>>>>
>>>> Because once mainline has proper reset handling, this can be used only
>>>> for legacy kernel versions.
>>>
>>>
>>> I am suffering for the same problem for the uniphier platform.
>>
>> The Denali NAND patches for Linux are currently stuck on your review too.
> 
> Huh?
> 
> Which patch in Linux is stuck on my review?

[PATCH V2] mtd: rawnand: denali_dt: Add support for configuring
SPARE_AREA_SKIP_BYTES

>>> The reset driver support for Linux will never be back-ported.
>>> So, it will continue to be a problem for Linux v4.19, v5.4, etc.
>>>
>>> reset_release_bulk() should go away.
>>
>> Once the Linux patches are upstream, we can just reset the NAND
>> controller without problems. And that's the right thing to do, sometimes
>> you don't want NAND to be available in Linux, and then it should be kept
>> in reset.
> 
> 
> The NAND reset in Linux will be available in the future version
> (if my patch is applied).

Which patch ?

> The stable kernels are maintained in 6 years.
> 
> https://www.kernel.org/category/releases.html

How is this relevant ?

> The EOF of Linux 5.4 is set at 2021.12, but
> following the recent rules, it will be extended
> to 2026 or so.

So, for all legacy kernel versions, enable this backward compatibility
thingie.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-01-10  0:10 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-09 10:07 [PATCH 1/3] mtd: rawnand: denali-spl: Add missing hardware init Marek Vasut
2020-01-09 10:07 ` [PATCH 2/3] mtd: rawnand: denali: Allow operation without clock driver Marek Vasut
2020-01-09 11:02   ` Masahiro Yamada
2020-01-09 11:14     ` Marek Vasut
2020-01-09 12:04       ` Masahiro Yamada
2020-01-09 13:21         ` Marek Vasut
2020-01-09 10:07 ` [PATCH 3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA Marek Vasut
2020-01-09 11:04   ` Masahiro Yamada
2020-01-09 11:15     ` Marek Vasut
2020-01-09 12:11       ` Masahiro Yamada
2020-01-09 14:25         ` Marek Vasut
2020-01-09 15:01           ` Masahiro Yamada
2020-01-10  0:10             ` Marek Vasut
2020-01-09 11:29 ` [PATCH 1/3] mtd: rawnand: denali-spl: Add missing hardware init Masahiro Yamada
2020-01-09 13:18   ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.