[U-Boot] [PATCH] sysreset: add support for socfpga sysreset

Marek Vasut marex at denx.de
Fri May 3 20:39:33 UTC 2019


On 5/3/19 10:37 PM, Simon Goldschmidt wrote:
> 
> 
> On 03.05.19 22:27, Marek Vasut wrote:
>> On 5/3/19 10:21 PM, Simon Goldschmidt wrote:
>>> This moves sysreset support for socfgpa from ad-hoc code in mach-socfpga
>>> to a UCLASS_SYSRESET based dm driver.
>>>
>>> A side effect is that gen5 and a10 can now differ between cold and warm
>>> reset.
>>
>> s/differ/select/ ?
> 
> Right.
> 
>>
>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
>>> ---
>>>
>>>   arch/arm/Kconfig                              |  3 +
>>>   arch/arm/mach-socfpga/Makefile                |  1 -
>>>   .../mach-socfpga/include/mach/reset_manager.h |  1 +
>>>   arch/arm/mach-socfpga/reset_manager.c         | 41 -------------
>>>   drivers/sysreset/Kconfig                      |  6 ++
>>>   drivers/sysreset/Makefile                     |  1 +
>>>   drivers/sysreset/sysreset_socfpga.c           | 58 +++++++++++++++++++
>>>   7 files changed, 69 insertions(+), 42 deletions(-)
>>>   delete mode 100644 arch/arm/mach-socfpga/reset_manager.c
>>>   create mode 100644 drivers/sysreset/sysreset_socfpga.c
>>>
>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>> index 49f01f1ff1..656ff92bd8 100644
>>> --- a/arch/arm/Kconfig
>>> +++ b/arch/arm/Kconfig
>>> @@ -823,6 +823,7 @@ config ARCH_SOCFPGA
>>>       select OF_CONTROL
>>>       select SPL_DM_RESET if DM_RESET
>>>       select SPL_DM_SERIAL
>>> +    select SPL_DRIVERS_MISC_SUPPORT
>>
>> Why MISC ?
> 
> Because that's what includes the sysreset drivers into SPL.

That seems wrong ?

>>>       select SPL_LIBCOMMON_SUPPORT
>>>       select SPL_LIBGENERIC_SUPPORT
>>>       select SPL_NAND_SUPPORT if SPL_NAND_DENALI
>>> @@ -833,6 +834,8 @@ config ARCH_SOCFPGA
>>>       select SUPPORT_SPL
>>>       select SYS_NS16550
>>>       select SYS_THUMB_BUILD if TARGET_SOCFPGA_GEN5 ||
>>> TARGET_SOCFPGA_ARRIA10
>>> +    select SYSRESET
>>> +    select SYSRESET_SOCFPGA
>>>       imply CMD_DM
>>>       imply CMD_MTDPARTS
>>>       imply CRC32_VERIFY
>>> diff --git a/arch/arm/mach-socfpga/Makefile
>>> b/arch/arm/mach-socfpga/Makefile
>>> index e66720447f..fc1181cb27 100644
>>> --- a/arch/arm/mach-socfpga/Makefile
>>> +++ b/arch/arm/mach-socfpga/Makefile
>>> @@ -8,7 +8,6 @@
>>>   obj-y    += board.o
>>>   obj-y    += clock_manager.o
>>>   obj-y    += misc.o
>>> -obj-y    += reset_manager.o
>>>     ifdef CONFIG_TARGET_SOCFPGA_GEN5
>>>   obj-y    += clock_manager_gen5.o
>>> diff --git a/arch/arm/mach-socfpga/include/mach/reset_manager.h
>>> b/arch/arm/mach-socfpga/include/mach/reset_manager.h
>>> index 42beaecdd6..6ad037e325 100644
>>> --- a/arch/arm/mach-socfpga/include/mach/reset_manager.h
>>> +++ b/arch/arm/mach-socfpga/include/mach/reset_manager.h
>>> @@ -11,6 +11,7 @@ void reset_cpu(ulong addr);
>>>   void socfpga_per_reset(u32 reset, int set);
>>>   void socfpga_per_reset_all(void);
>>>   +#define RSTMGR_CTRL_SWCOLDRSTREQ_LSB 0
>>
>> Separate patch
> 
> Fair enough.
> 
>>
>>>   #define RSTMGR_CTRL_SWWARMRSTREQ_LSB 1
>>>     /*
>>> diff --git a/arch/arm/mach-socfpga/reset_manager.c
>>> b/arch/arm/mach-socfpga/reset_manager.c
>>> deleted file mode 100644
>>> index e0a01ed07a..0000000000
>>> --- a/arch/arm/mach-socfpga/reset_manager.c
>>> +++ /dev/null
>>> @@ -1,41 +0,0 @@
>>> -// SPDX-License-Identifier: GPL-2.0+
>>> -/*
>>> - *  Copyright (C) 2013 Altera Corporation <www.altera.com>
>>> - */
>>> -
>>> -
>>> -#include <common.h>
>>> -#include <asm/io.h>
>>> -#include <asm/arch/reset_manager.h>
>>> -
>>> -#if defined(CONFIG_TARGET_SOCFPGA_STRATIX10)
>>> -#include <asm/arch/mailbox_s10.h>
>>> -#endif
>>> -
>>> -DECLARE_GLOBAL_DATA_PTR;
>>> -
>>> -#if !defined(CONFIG_TARGET_SOCFPGA_STRATIX10)
>>> -static const struct socfpga_reset_manager *reset_manager_base =
>>> -        (void *)SOCFPGA_RSTMGR_ADDRESS;
>>> -#endif
>>> -
>>> -/*
>>> - * Write the reset manager register to cause reset
>>> - */
>>> -void reset_cpu(ulong addr)
>>> -{
>>> -    /* request a warm reset */
>>> -#if defined(CONFIG_TARGET_SOCFPGA_STRATIX10)
>>> -    puts("Mailbox: Issuing mailbox cmd REBOOT_HPS\n");
>>> -    mbox_reset_cold();
>>> -#else
>>> -    writel(1 << RSTMGR_CTRL_SWWARMRSTREQ_LSB,
>>> -           &reset_manager_base->ctrl);
>>> -#endif
>>> -    /*
>>> -     * infinite loop here as watchdog will trigger and reset
>>> -     * the processor
>>> -     */
>>> -    while (1)
>>> -        ;
>>> -}
>>> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
>>> index 8ce3e2e207..8b6e6626d2 100644
>>> --- a/drivers/sysreset/Kconfig
>>> +++ b/drivers/sysreset/Kconfig
>>> @@ -36,6 +36,12 @@ config SYSRESET_PSCI
>>>         Enable PSCI SYSTEM_RESET function call.  To use this, PSCI
>>> firmware
>>>         must be running on your system.
>>>   +config SYSRESET_SOCFPGA
>>> +    bool "Enable support for Intel SOCFPGA family"
>>> +    depends on ARCH_SOCFPGA
>>> +    help
>>> +      This enables the system reset driver support for Intel SOCFPGA
>>> SoCs.
>>> +
>>>   config SYSRESET_TI_SCI
>>>       bool "TI System Control Interface (TI SCI) system reset driver"
>>>       depends on TI_SCI_PROTOCOL
>>> diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
>>> index b3728ac17f..0241b0132d 100644
>>> --- a/drivers/sysreset/Makefile
>>> +++ b/drivers/sysreset/Makefile
>>> @@ -11,6 +11,7 @@ obj-$(CONFIG_SYSRESET_GPIO) += sysreset_gpio.o
>>>   obj-$(CONFIG_SYSRESET_MCP83XX) += sysreset_mpc83xx.o
>>>   obj-$(CONFIG_SYSRESET_MICROBLAZE) += sysreset_microblaze.o
>>>   obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o
>>> +obj-$(CONFIG_SYSRESET_SOCFPGA) += sysreset_socfpga.o
>>>   obj-$(CONFIG_SYSRESET_TI_SCI) += sysreset-ti-sci.o
>>>   obj-$(CONFIG_SYSRESET_SYSCON) += sysreset_syscon.o
>>>   obj-$(CONFIG_SYSRESET_WATCHDOG) += sysreset_watchdog.o
>>> diff --git a/drivers/sysreset/sysreset_socfpga.c
>>> b/drivers/sysreset/sysreset_socfpga.c
>>> new file mode 100644
>>> index 0000000000..b5bbf4aad6
>>> --- /dev/null
>>> +++ b/drivers/sysreset/sysreset_socfpga.c
>>> @@ -0,0 +1,58 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2019 Pepperl+Fuchs
>>> + * Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <dm.h>
>>> +#include <errno.h>
>>> +#include <sysreset.h>
>>> +#include <asm/io.h>
>>> +#include <asm/arch/reset_manager.h>
>>> +
>>> +#if defined(CONFIG_TARGET_SOCFPGA_STRATIX10)
>>> +#include <asm/arch/mailbox_s10.h>
>>> +#else
>>> +static const struct socfpga_reset_manager *reset_manager_base =
>>> +        (void *)SOCFPGA_RSTMGR_ADDRESS;
>>
>> Shouldn't this come from DT ?
> 
> Hmm, that would indeed be better, yes. The thing is we do have a driver
> matching the dts node in question, but it supplies peripheral resets,
> not sysreset. Thinking about this, I don't know how to make 2 drivers
> match the same dts node...?

Make it one driver or something MFDish ?

>>> +#endif
>>> +
>>> +static int socfpga_sysreset_request(struct udevice *dev,
>>> +                    enum sysreset_t type)
>>> +{
>>> +#if defined(CONFIG_TARGET_SOCFPGA_STRATIX10)
>>
>> You want a different driver for S10 instead of this ifdeffery.
> 
> That might indeed be a better solution, yes.

Yes please

>>> +    puts("Mailbox: Issuing mailbox cmd REBOOT_HPS\n");
>>> +    mbox_reset_cold();
>>> +#else
>>> +    switch (type) {
>>> +    case SYSRESET_WARM:
>>> +        writel(1 << RSTMGR_CTRL_SWWARMRSTREQ_LSB,
>>> +               &reset_manager_base->ctrl);
>>> +        break;
>>> +    case SYSRESET_COLD:
>>> +        writel(1 << RSTMGR_CTRL_SWCOLDRSTREQ_LSB,
>>> +               &reset_manager_base->ctrl);
>>> +        break;
>>> +    default:
>>> +        return -EPROTONOSUPPORT;
>>> +    }
>>> +#endif
>>> +    return -EINPROGRESS;
>>> +}
>>> +
>>> +static struct sysreset_ops socfpga_sysreset = {
>>> +    .request = socfpga_sysreset_request,
>>> +};
>>> +
>>> +U_BOOT_DRIVER(sysreset_socfpga) = {
>>> +    .id    = UCLASS_SYSRESET,
>>> +    .name    = "socfpga_sysreset",
>>> +    .ops    = &socfpga_sysreset,
>>> +};
>>> +
>>> +/* The reset driver does not have a device node, so bind it here */
>>
>> Isn't that part of reset manager ?
> 
> Exactly, see above. 1 dts node, 2 UCLASSes...
> Any idea how to overcome this limit?

See above, or look at syscon_get_regmap() to export a register to a
different driver(s).

-- 
Best regards,
Marek Vasut


More information about the U-Boot mailing list