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

Wed Aug 16 17:02:26 CEST 2023

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 <ada at thorsis.com>
>>> ---
>>>    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=0xfffff200
>>>    CONFIG_DEBUG_UART_CLOCK=200000000
>>>    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=0xfffff200
>>>    CONFIG_DEBUG_UART_CLOCK=200000000
>>>    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=0xfffff200
>>>    CONFIG_DEBUG_UART_CLOCK=200000000
>>>    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=0xfffff200
>>>    CONFIG_DEBUG_UART_CLOCK=200000000
>>>    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=0xfffff200
>>>    CONFIG_DEBUG_UART_CLOCK=200000000
>>>    CONFIG_DEBUG_UART_BOARD_INIT=y
>>> @@ -88,14 +89,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
>>