Re: [PATCH 2/4] configs: at91: sam9x60: Switch to new reset driver

2023-08-16 Thread Eugen Hristev

On 8/16/23 16:42, Alexander Dahl wrote:

Hei hei,

Am Wed, Aug 16, 2023 at 03:48:25PM +0300 schrieb Eugen Hristev:


Hi Alexander,

On 8/9/23 17:16, Alexander Dahl wrote:

Since commit 61040097a9d1 ("reset: at91: Add reset driver for basic
assert/deassert operations") the compatible "microchip,sam9x60-rstc" for
the sam9x60 reset controller in sam9x60.dtsi is not handled by
CONFIG_SYSRESET_AT91 anymore, but by CONFIG_RESET_AT91 now.  This
resulted in the following error message, when trying to reset from
U-Boot shell:

  U-Boot> reset
  resetting ...
  System reset not supported on this platform
  ### ERROR ### Please RESET the board ###

Fixed with the switch to the new driver.  Tested on sam9x60-curiosity
board.  Defconfigs for sam9x60ek adapted in the same way, but without
testing.  These should be all sam9x60 boards affected in U-Boot here.


 From what I remember from the top of my head, it makes sense to use the new
reset driver, however, you should not remove the old SYSRESET driver,
because that driver handles different kind of resets on the SoC and PHYs.
Can you double check that?


Had a look into it and TBH it's very confusing to me.  I found no help
in documentation.  There are two different uclass which from the
outside seem to do the same thing.  The Kconfig help texts do not
explain what the purpose is of one over the other.  As a user this
creates very bad user experience: Do I need one or the other or both?


Maybe I can shed some light into it.
U-boot, unlike Linux, is a bit more modular in a few aspects, as in this 
RESET vs SYSRESET case.
From my understanding, RESET is used to reset the board and SYSRESET 
for general reset purposes: reset a PHY, reset something else, etc.
DT-bindings are done by Linux guys, and in Linux, we have a single 
driver for all above (it's similar with PINCTRL and GPIO, one linux 
driver, but two different u-boot UCLASS), so we have just one 
compatible, but U-boot needs to probe two drivers to achieve all the 
wanted functionality !
And we are forced to use the bindings from Linux (little rant), so just 
one U-boot driver is bound to the compatible at boot time. Then, the 
second driver, has to be manually bound by the first driver, and it does 
not have a compatible into it.


I think some of the stuff I explained(poorly) should be readable from 
this patch https://lists.denx.de/pipermail/u-boot/2022-December/501618.html


To simply answer your question, you would need both.



Then there's CONFIG_SYSRESET_CMD_RESET which implicates there are more
than one possible implementations for cmd 'reset' … but which is the
right one.  The help text does not explain.

I though sysreset is old and reset is new and assumed things get
migrated like in other subsystems.  But here it seems we have two
entangled subsystems where one hooks into the other (as reset_at91
does).

So, I can of course _just_ enable CONFIG_RESET_AT91 and hope for the
best here, keeping SYSRESET_CMD_RESET as is … but it would be very
nice if someone else could explain the what and why in help texts and
documentation!

Greets
Alex

P.S.: I would have added sysreset or reset subsystem maintainer to Cc,
but according to MAINTAINERS there is none.


Signed-off-by: Alexander Dahl 
---
   configs/sam9x60_curiosity_mmc1_defconfig | 4 ++--
   configs/sam9x60_curiosity_mmc_defconfig  | 4 ++--
   configs/sam9x60ek_mmc_defconfig  | 4 ++--
   configs/sam9x60ek_nandflash_defconfig| 4 ++--
   configs/sam9x60ek_qspiflash_defconfig| 4 ++--
   5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/configs/sam9x60_curiosity_mmc1_defconfig 
b/configs/sam9x60_curiosity_mmc1_defconfig
index 21b2cc2edd..e8781b363b 100644
--- a/configs/sam9x60_curiosity_mmc1_defconfig
+++ b/configs/sam9x60_curiosity_mmc1_defconfig
@@ -14,6 +14,7 @@ CONFIG_DM_GPIO=y
   CONFIG_DEFAULT_DEVICE_TREE="at91-sam9x60_curiosity"
   CONFIG_SYS_PROMPT="U-Boot> "
   CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_DM_RESET=y
   CONFIG_DEBUG_UART_BASE=0xf200
   CONFIG_DEBUG_UART_CLOCK=2
   CONFIG_DEBUG_UART_BOARD_INIT=y
@@ -79,11 +80,10 @@ CONFIG_PHY_MICREL=y
   CONFIG_MACB=y
   CONFIG_PINCTRL=y
   CONFIG_PINCTRL_AT91=y
+CONFIG_RESET_AT91=y
   CONFIG_DM_SERIAL=y
   CONFIG_DEBUG_UART_ANNOUNCE=y
   CONFIG_ATMEL_USART=y
-CONFIG_SYSRESET=y
-CONFIG_SYSRESET_AT91=y
   CONFIG_TIMER=y
   CONFIG_MCHP_PIT64B_TIMER=y
   CONFIG_W1=y
diff --git a/configs/sam9x60_curiosity_mmc_defconfig 
b/configs/sam9x60_curiosity_mmc_defconfig
index 269f015989..0f57588d8b 100644
--- a/configs/sam9x60_curiosity_mmc_defconfig
+++ b/configs/sam9x60_curiosity_mmc_defconfig
@@ -14,6 +14,7 @@ CONFIG_DM_GPIO=y
   CONFIG_DEFAULT_DEVICE_TREE="at91-sam9x60_curiosity"
   CONFIG_SYS_PROMPT="U-Boot> "
   CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_DM_RESET=y
   CONFIG_DEBUG_UART_BASE=0xf200
   CONFIG_DEBUG_UART_CLOCK=2
   CONFIG_DEBUG_UART_BOARD_INIT=y
@@ -79,11 +80,10 @@ CONFIG_PHY_MICREL=y
   CONFIG_MACB=y
   CONFIG_PINCTRL=y
   CONFIG_PINCTRL_AT

Re: [PATCH 2/4] configs: at91: sam9x60: Switch to new reset driver

2023-08-16 Thread Alexander Dahl
Hei hei,

Am Wed, Aug 16, 2023 at 03:48:25PM +0300 schrieb Eugen Hristev:
> 
> Hi Alexander,
> 
> On 8/9/23 17:16, Alexander Dahl wrote:
> > Since commit 61040097a9d1 ("reset: at91: Add reset driver for basic
> > assert/deassert operations") the compatible "microchip,sam9x60-rstc" for
> > the sam9x60 reset controller in sam9x60.dtsi is not handled by
> > CONFIG_SYSRESET_AT91 anymore, but by CONFIG_RESET_AT91 now.  This
> > resulted in the following error message, when trying to reset from
> > U-Boot shell:
> > 
> >  U-Boot> reset
> >  resetting ...
> >  System reset not supported on this platform
> >  ### ERROR ### Please RESET the board ###
> > 
> > Fixed with the switch to the new driver.  Tested on sam9x60-curiosity
> > board.  Defconfigs for sam9x60ek adapted in the same way, but without
> > testing.  These should be all sam9x60 boards affected in U-Boot here.
> 
> From what I remember from the top of my head, it makes sense to use the new
> reset driver, however, you should not remove the old SYSRESET driver,
> because that driver handles different kind of resets on the SoC and PHYs.
> Can you double check that?

Had a look into it and TBH it's very confusing to me.  I found no help
in documentation.  There are two different uclass which from the
outside seem to do the same thing.  The Kconfig help texts do not
explain what the purpose is of one over the other.  As a user this
creates very bad user experience: Do I need one or the other or both? 

Then there's CONFIG_SYSRESET_CMD_RESET which implicates there are more
than one possible implementations for cmd 'reset' … but which is the
right one.  The help text does not explain.

I though sysreset is old and reset is new and assumed things get
migrated like in other subsystems.  But here it seems we have two
entangled subsystems where one hooks into the other (as reset_at91
does).

So, I can of course _just_ enable CONFIG_RESET_AT91 and hope for the
best here, keeping SYSRESET_CMD_RESET as is … but it would be very
nice if someone else could explain the what and why in help texts and
documentation!

Greets
Alex

P.S.: I would have added sysreset or reset subsystem maintainer to Cc,
but according to MAINTAINERS there is none.

> > Signed-off-by: Alexander Dahl 
> > ---
> >   configs/sam9x60_curiosity_mmc1_defconfig | 4 ++--
> >   configs/sam9x60_curiosity_mmc_defconfig  | 4 ++--
> >   configs/sam9x60ek_mmc_defconfig  | 4 ++--
> >   configs/sam9x60ek_nandflash_defconfig| 4 ++--
> >   configs/sam9x60ek_qspiflash_defconfig| 4 ++--
> >   5 files changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/configs/sam9x60_curiosity_mmc1_defconfig 
> > b/configs/sam9x60_curiosity_mmc1_defconfig
> > index 21b2cc2edd..e8781b363b 100644
> > --- a/configs/sam9x60_curiosity_mmc1_defconfig
> > +++ b/configs/sam9x60_curiosity_mmc1_defconfig
> > @@ -14,6 +14,7 @@ CONFIG_DM_GPIO=y
> >   CONFIG_DEFAULT_DEVICE_TREE="at91-sam9x60_curiosity"
> >   CONFIG_SYS_PROMPT="U-Boot> "
> >   CONFIG_OF_LIBFDT_OVERLAY=y
> > +CONFIG_DM_RESET=y
> >   CONFIG_DEBUG_UART_BASE=0xf200
> >   CONFIG_DEBUG_UART_CLOCK=2
> >   CONFIG_DEBUG_UART_BOARD_INIT=y
> > @@ -79,11 +80,10 @@ CONFIG_PHY_MICREL=y
> >   CONFIG_MACB=y
> >   CONFIG_PINCTRL=y
> >   CONFIG_PINCTRL_AT91=y
> > +CONFIG_RESET_AT91=y
> >   CONFIG_DM_SERIAL=y
> >   CONFIG_DEBUG_UART_ANNOUNCE=y
> >   CONFIG_ATMEL_USART=y
> > -CONFIG_SYSRESET=y
> > -CONFIG_SYSRESET_AT91=y
> >   CONFIG_TIMER=y
> >   CONFIG_MCHP_PIT64B_TIMER=y
> >   CONFIG_W1=y
> > diff --git a/configs/sam9x60_curiosity_mmc_defconfig 
> > b/configs/sam9x60_curiosity_mmc_defconfig
> > index 269f015989..0f57588d8b 100644
> > --- a/configs/sam9x60_curiosity_mmc_defconfig
> > +++ b/configs/sam9x60_curiosity_mmc_defconfig
> > @@ -14,6 +14,7 @@ CONFIG_DM_GPIO=y
> >   CONFIG_DEFAULT_DEVICE_TREE="at91-sam9x60_curiosity"
> >   CONFIG_SYS_PROMPT="U-Boot> "
> >   CONFIG_OF_LIBFDT_OVERLAY=y
> > +CONFIG_DM_RESET=y
> >   CONFIG_DEBUG_UART_BASE=0xf200
> >   CONFIG_DEBUG_UART_CLOCK=2
> >   CONFIG_DEBUG_UART_BOARD_INIT=y
> > @@ -79,11 +80,10 @@ CONFIG_PHY_MICREL=y
> >   CONFIG_MACB=y
> >   CONFIG_PINCTRL=y
> >   CONFIG_PINCTRL_AT91=y
> > +CONFIG_RESET_AT91=y
> >   CONFIG_DM_SERIAL=y
> >   CONFIG_DEBUG_UART_ANNOUNCE=y
> >   CONFIG_ATMEL_USART=y
> > -CONFIG_SYSRESET=y
> > -CONFIG_SYSRESET_AT91=y
> >   CONFIG_TIMER=y
> >   CONFIG_MCHP_PIT64B_TIMER=y
> >   CONFIG_W1=y
> > diff --git a/configs/sam9x60ek_mmc_defconfig 
> > b/configs/sam9x60ek_mmc_defconfig
> > index 2a1399748c..446caceba0 100644
> > --- a/configs/sam9x60ek_mmc_defconfig
> > +++ b/configs/sam9x60ek_mmc_defconfig
> > @@ -15,6 +15,7 @@ CONFIG_DM_GPIO=y
> >   CONFIG_DEFAULT_DEVICE_TREE="sam9x60ek"
> >   CONFIG_SYS_PROMPT="U-Boot> "
> >   CONFIG_OF_LIBFDT_OVERLAY=y
> > +CONFIG_DM_RESET=y
> >   CONFIG_DEBUG_UART_BASE=0xf200
> >   CONFIG_DEBUG_UART_CLOCK=2
> >   CONFIG_DEBUG_UART_BOARD_INIT=y
> > @@ -87,14 +88,13 @@ CONFIG_PHY_MICREL=y
> >   CONFIG_MACB=y
> >   CON

Re: [PATCH 2/4] configs: at91: sam9x60: Switch to new reset driver

2023-08-16 Thread Eugen Hristev



Hi Alexander,

On 8/9/23 17:16, Alexander Dahl wrote:

Since commit 61040097a9d1 ("reset: at91: Add reset driver for basic
assert/deassert operations") the compatible "microchip,sam9x60-rstc" for
the sam9x60 reset controller in sam9x60.dtsi is not handled by
CONFIG_SYSRESET_AT91 anymore, but by CONFIG_RESET_AT91 now.  This
resulted in the following error message, when trying to reset from
U-Boot shell:

 U-Boot> reset
 resetting ...
 System reset not supported on this platform
 ### ERROR ### Please RESET the board ###

Fixed with the switch to the new driver.  Tested on sam9x60-curiosity
board.  Defconfigs for sam9x60ek adapted in the same way, but without
testing.  These should be all sam9x60 boards affected in U-Boot here.


From what I remember from the top of my head, it makes sense to use the 
new reset driver, however, you should not remove the old SYSRESET 
driver, because that driver handles different kind of resets on the SoC 
and PHYs. Can you double check that?




Signed-off-by: Alexander Dahl 
---
  configs/sam9x60_curiosity_mmc1_defconfig | 4 ++--
  configs/sam9x60_curiosity_mmc_defconfig  | 4 ++--
  configs/sam9x60ek_mmc_defconfig  | 4 ++--
  configs/sam9x60ek_nandflash_defconfig| 4 ++--
  configs/sam9x60ek_qspiflash_defconfig| 4 ++--
  5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/configs/sam9x60_curiosity_mmc1_defconfig 
b/configs/sam9x60_curiosity_mmc1_defconfig
index 21b2cc2edd..e8781b363b 100644
--- a/configs/sam9x60_curiosity_mmc1_defconfig
+++ b/configs/sam9x60_curiosity_mmc1_defconfig
@@ -14,6 +14,7 @@ CONFIG_DM_GPIO=y
  CONFIG_DEFAULT_DEVICE_TREE="at91-sam9x60_curiosity"
  CONFIG_SYS_PROMPT="U-Boot> "
  CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_DM_RESET=y
  CONFIG_DEBUG_UART_BASE=0xf200
  CONFIG_DEBUG_UART_CLOCK=2
  CONFIG_DEBUG_UART_BOARD_INIT=y
@@ -79,11 +80,10 @@ CONFIG_PHY_MICREL=y
  CONFIG_MACB=y
  CONFIG_PINCTRL=y
  CONFIG_PINCTRL_AT91=y
+CONFIG_RESET_AT91=y
  CONFIG_DM_SERIAL=y
  CONFIG_DEBUG_UART_ANNOUNCE=y
  CONFIG_ATMEL_USART=y
-CONFIG_SYSRESET=y
-CONFIG_SYSRESET_AT91=y
  CONFIG_TIMER=y
  CONFIG_MCHP_PIT64B_TIMER=y
  CONFIG_W1=y
diff --git a/configs/sam9x60_curiosity_mmc_defconfig 
b/configs/sam9x60_curiosity_mmc_defconfig
index 269f015989..0f57588d8b 100644
--- a/configs/sam9x60_curiosity_mmc_defconfig
+++ b/configs/sam9x60_curiosity_mmc_defconfig
@@ -14,6 +14,7 @@ CONFIG_DM_GPIO=y
  CONFIG_DEFAULT_DEVICE_TREE="at91-sam9x60_curiosity"
  CONFIG_SYS_PROMPT="U-Boot> "
  CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_DM_RESET=y
  CONFIG_DEBUG_UART_BASE=0xf200
  CONFIG_DEBUG_UART_CLOCK=2
  CONFIG_DEBUG_UART_BOARD_INIT=y
@@ -79,11 +80,10 @@ CONFIG_PHY_MICREL=y
  CONFIG_MACB=y
  CONFIG_PINCTRL=y
  CONFIG_PINCTRL_AT91=y
+CONFIG_RESET_AT91=y
  CONFIG_DM_SERIAL=y
  CONFIG_DEBUG_UART_ANNOUNCE=y
  CONFIG_ATMEL_USART=y
-CONFIG_SYSRESET=y
-CONFIG_SYSRESET_AT91=y
  CONFIG_TIMER=y
  CONFIG_MCHP_PIT64B_TIMER=y
  CONFIG_W1=y
diff --git a/configs/sam9x60ek_mmc_defconfig b/configs/sam9x60ek_mmc_defconfig
index 2a1399748c..446caceba0 100644
--- a/configs/sam9x60ek_mmc_defconfig
+++ b/configs/sam9x60ek_mmc_defconfig
@@ -15,6 +15,7 @@ CONFIG_DM_GPIO=y
  CONFIG_DEFAULT_DEVICE_TREE="sam9x60ek"
  CONFIG_SYS_PROMPT="U-Boot> "
  CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_DM_RESET=y
  CONFIG_DEBUG_UART_BASE=0xf200
  CONFIG_DEBUG_UART_CLOCK=2
  CONFIG_DEBUG_UART_BOARD_INIT=y
@@ -87,14 +88,13 @@ CONFIG_PHY_MICREL=y
  CONFIG_MACB=y
  CONFIG_PINCTRL=y
  CONFIG_PINCTRL_AT91=y
+CONFIG_RESET_AT91=y
  CONFIG_DM_SERIAL=y
  CONFIG_DEBUG_UART_ANNOUNCE=y
  CONFIG_ATMEL_USART=y
  CONFIG_SPI=y
  CONFIG_DM_SPI=y
  CONFIG_ATMEL_QSPI=y
-CONFIG_SYSRESET=y
-CONFIG_SYSRESET_AT91=y
  CONFIG_TIMER=y
  CONFIG_ATMEL_PIT_TIMER=y
  CONFIG_W1=y
diff --git a/configs/sam9x60ek_nandflash_defconfig 
b/configs/sam9x60ek_nandflash_defconfig
index c6c4686658..acaa16ee49 100644
--- a/configs/sam9x60ek_nandflash_defconfig
+++ b/configs/sam9x60ek_nandflash_defconfig
@@ -14,6 +14,7 @@ CONFIG_DM_GPIO=y
  CONFIG_DEFAULT_DEVICE_TREE="sam9x60ek"
  CONFIG_SYS_PROMPT="U-Boot> "
  CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_DM_RESET=y
  CONFIG_DEBUG_UART_BASE=0xf200
  CONFIG_DEBUG_UART_CLOCK=2
  CONFIG_DEBUG_UART_BOARD_INIT=y
@@ -89,14 +90,13 @@ CONFIG_PHY_MICREL=y
  CONFIG_MACB=y
  CONFIG_PINCTRL=y
  CONFIG_PINCTRL_AT91=y
+CONFIG_RESET_AT91=y
  CONFIG_DM_SERIAL=y
  CONFIG_DEBUG_UART_ANNOUNCE=y
  CONFIG_ATMEL_USART=y
  CONFIG_SPI=y
  CONFIG_DM_SPI=y
  CONFIG_ATMEL_QSPI=y
-CONFIG_SYSRESET=y
-CONFIG_SYSRESET_AT91=y
  CONFIG_TIMER=y
  CONFIG_ATMEL_PIT_TIMER=y
  CONFIG_W1=y
diff --git a/configs/sam9x60ek_qspiflash_defconfig 
b/configs/sam9x60ek_qspiflash_defconfig
index ef2e2db8b8..6fb79214e5 100644
--- a/configs/sam9x60ek_qspiflash_defconfig
+++ b/configs/sam9x60ek_qspiflash_defconfig
@@ -14,6 +14,7 @@ CONFIG_DM_GPIO=y
  CONFIG_DEFAULT_DEVICE_TREE="sam9x60ek"
  CONFIG_SYS_PROMPT="U-Boot> "
  CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_DM_RESET=y
  CONFIG_DEBUG_UART_BASE=0xf200
  CONFIG_D