diff mbox series

[3/3] mtd: rawnand: denali: Do not reset the block on SoCFPGA

Message ID 20200109100754.1007705-3-marex@denx.de
State New
Headers show
Series [1/3] mtd: rawnand: denali-spl: Add missing hardware init | expand

Commit Message

Marek Vasut Jan. 9, 2020, 10:07 a.m. UTC
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 at denx.de>
Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
---
 drivers/mtd/nand/raw/denali_dt.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Masahiro Yamada Jan. 9, 2020, 11:04 a.m. UTC | #1
On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex at 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 at denx.de>
> Cc: Masahiro Yamada <yamada.masahiro at 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
>
Marek Vasut Jan. 9, 2020, 11:15 a.m. UTC | #2
On 1/9/20 12:04 PM, Masahiro Yamada wrote:
> On Thu, Jan 9, 2020 at 7:08 PM Marek Vasut <marex at 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 at denx.de>
>> Cc: Masahiro Yamada <yamada.masahiro at 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.
Masahiro Yamada Jan. 9, 2020, 12:11 p.m. UTC | #3
On Thu, Jan 9, 2020 at 8:16 PM Marek Vasut <marex at 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 at 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 at denx.de>
> >> Cc: Masahiro Yamada <yamada.masahiro at 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.
Marek Vasut Jan. 9, 2020, 2:25 p.m. UTC | #4
On 1/9/20 1:11 PM, Masahiro Yamada wrote:
> On Thu, Jan 9, 2020 at 8:16 PM Marek Vasut <marex at 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 at 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 at denx.de>
>>>> Cc: Masahiro Yamada <yamada.masahiro at 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.
Masahiro Yamada Jan. 9, 2020, 3:01 p.m. UTC | #5
On Thu, Jan 9, 2020 at 11:48 PM Marek Vasut <marex at 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 at 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 at 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 at denx.de>
> >>>> Cc: Masahiro Yamada <yamada.masahiro at 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.
Marek Vasut Jan. 10, 2020, 12:10 a.m. UTC | #6
On 1/9/20 4:01 PM, Masahiro Yamada wrote:
> On Thu, Jan 9, 2020 at 11:48 PM Marek Vasut <marex at 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 at 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 at 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 at denx.de>
>>>>>> Cc: Masahiro Yamada <yamada.masahiro at 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.
diff mbox series

Patch

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);
 }