* [PATCH 1/1] lib: utils: identify supported GPIO reset methods
@ 2021-09-28 11:42 Heinrich Schuchardt
2021-09-29 4:16 ` Anup Patel
0 siblings, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2021-09-28 11:42 UTC (permalink / raw)
To: opensbi
The GPIO reset driver supports reset and poweroff. But not all boards
support both. gpio_system_reset_check() must detect this situation.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
lib/utils/reset/fdt_reset_gpio.c | 44 +++++++++++++++-----------------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/lib/utils/reset/fdt_reset_gpio.c b/lib/utils/reset/fdt_reset_gpio.c
index 30127f5..77e4d0e 100644
--- a/lib/utils/reset/fdt_reset_gpio.c
+++ b/lib/utils/reset/fdt_reset_gpio.c
@@ -35,21 +35,9 @@ static struct gpio_reset restart = {
.inactive_delay = 100
};
-static int gpio_system_reset_check(u32 type, u32 reason)
-{
- switch (type) {
- case SBI_SRST_RESET_TYPE_SHUTDOWN:
- case SBI_SRST_RESET_TYPE_COLD_REBOOT:
- case SBI_SRST_RESET_TYPE_WARM_REBOOT:
- return 1;
- }
-
- return 0;
-}
-
-static void gpio_system_reset(u32 type, u32 reason)
+static struct gpio_reset *gpio_get_reset_settings(u32 type)
{
- struct gpio_reset *reset = NULL;
+ struct gpio_reset *reset;
switch (type) {
case SBI_SRST_RESET_TYPE_SHUTDOWN:
@@ -59,14 +47,26 @@ static void gpio_system_reset(u32 type, u32 reason)
case SBI_SRST_RESET_TYPE_WARM_REBOOT:
reset = &restart;
break;
+ default:
+ reset = NULL;
}
- if (reset) {
- if (!reset->pin.chip) {
- sbi_printf("%s: gpio pin not available\n", __func__);
- goto skip_reset;
- }
+ if (reset && !reset->pin.chip)
+ reset = NULL;
+
+ return reset;
+}
+
+static int gpio_system_reset_check(u32 type, u32 reason)
+{
+ return !!gpio_get_reset_settings(type);
+}
+
+static void gpio_system_reset(u32 type, u32 reason)
+{
+ struct gpio_reset *reset = gpio_get_reset_settings(type);
+ if (reset) {
/* drive it active, also inactive->active edge */
gpio_direction_output(&reset->pin, 1);
sbi_timer_mdelay(reset->active_delay);
@@ -77,11 +77,9 @@ static void gpio_system_reset(u32 type, u32 reason)
/* drive it active, also inactive->active edge */
gpio_set(&reset->pin, 1);
-
-skip_reset:
- /* hang !!! */
- sbi_hart_hang();
}
+ /* hang !!! */
+ sbi_hart_hang();
}
static struct sbi_system_reset_device gpio_reset = {
--
2.32.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 1/1] lib: utils: identify supported GPIO reset methods
2021-09-28 11:42 [PATCH 1/1] lib: utils: identify supported GPIO reset methods Heinrich Schuchardt
@ 2021-09-29 4:16 ` Anup Patel
2021-09-29 7:01 ` Heinrich Schuchardt
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Anup Patel @ 2021-09-29 4:16 UTC (permalink / raw)
To: opensbi
+Nikita
On Tue, Sep 28, 2021 at 5:13 PM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> The GPIO reset driver supports reset and poweroff. But not all boards
> support both. gpio_system_reset_check() must detect this situation.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Looks good to me.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
I had mentioned on the PMIC reset series that we need to improve
the sbi_system.h device registration such that reset drivers can
register a reset device for a range of reset types. This will allow
separate reset drivers (e.g. PMIC+GPIO) for SiFive Unmatched.
Also, reset_check() callback will not be required anymore.
Regards,
Anup
> ---
> lib/utils/reset/fdt_reset_gpio.c | 44 +++++++++++++++-----------------
> 1 file changed, 21 insertions(+), 23 deletions(-)
>
> diff --git a/lib/utils/reset/fdt_reset_gpio.c b/lib/utils/reset/fdt_reset_gpio.c
> index 30127f5..77e4d0e 100644
> --- a/lib/utils/reset/fdt_reset_gpio.c
> +++ b/lib/utils/reset/fdt_reset_gpio.c
> @@ -35,21 +35,9 @@ static struct gpio_reset restart = {
> .inactive_delay = 100
> };
>
> -static int gpio_system_reset_check(u32 type, u32 reason)
> -{
> - switch (type) {
> - case SBI_SRST_RESET_TYPE_SHUTDOWN:
> - case SBI_SRST_RESET_TYPE_COLD_REBOOT:
> - case SBI_SRST_RESET_TYPE_WARM_REBOOT:
> - return 1;
> - }
> -
> - return 0;
> -}
> -
> -static void gpio_system_reset(u32 type, u32 reason)
> +static struct gpio_reset *gpio_get_reset_settings(u32 type)
> {
> - struct gpio_reset *reset = NULL;
> + struct gpio_reset *reset;
>
> switch (type) {
> case SBI_SRST_RESET_TYPE_SHUTDOWN:
> @@ -59,14 +47,26 @@ static void gpio_system_reset(u32 type, u32 reason)
> case SBI_SRST_RESET_TYPE_WARM_REBOOT:
> reset = &restart;
> break;
> + default:
> + reset = NULL;
> }
>
> - if (reset) {
> - if (!reset->pin.chip) {
> - sbi_printf("%s: gpio pin not available\n", __func__);
> - goto skip_reset;
> - }
> + if (reset && !reset->pin.chip)
> + reset = NULL;
> +
> + return reset;
> +}
> +
> +static int gpio_system_reset_check(u32 type, u32 reason)
> +{
> + return !!gpio_get_reset_settings(type);
> +}
> +
> +static void gpio_system_reset(u32 type, u32 reason)
> +{
> + struct gpio_reset *reset = gpio_get_reset_settings(type);
>
> + if (reset) {
> /* drive it active, also inactive->active edge */
> gpio_direction_output(&reset->pin, 1);
> sbi_timer_mdelay(reset->active_delay);
> @@ -77,11 +77,9 @@ static void gpio_system_reset(u32 type, u32 reason)
>
> /* drive it active, also inactive->active edge */
> gpio_set(&reset->pin, 1);
> -
> -skip_reset:
> - /* hang !!! */
> - sbi_hart_hang();
> }
> + /* hang !!! */
> + sbi_hart_hang();
> }
>
> static struct sbi_system_reset_device gpio_reset = {
> --
> 2.32.0
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/1] lib: utils: identify supported GPIO reset methods
2021-09-29 4:16 ` Anup Patel
@ 2021-09-29 7:01 ` Heinrich Schuchardt
2021-09-29 12:38 ` Anup Patel
2021-09-29 9:14 ` Nikita Shubin
2021-10-01 4:58 ` Anup Patel
2 siblings, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2021-09-29 7:01 UTC (permalink / raw)
To: opensbi
On 9/29/21 6:16 AM, Anup Patel wrote:
> +Nikita
>
> On Tue, Sep 28, 2021 at 5:13 PM Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> The GPIO reset driver supports reset and poweroff. But not all boards
>> support both. gpio_system_reset_check() must detect this situation.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>
> Looks good to me.
>
> Reviewed-by: Anup Patel <anup.patel@wdc.com>
>
> I had mentioned on the PMIC reset series that we need to improve
> the sbi_system.h device registration such that reset drivers can
> register a reset device for a range of reset types. This will allow
> separate reset drivers (e.g. PMIC+GPIO) for SiFive Unmatched.
> Also, reset_check() callback will not be required anymore.
Hello Anup,
thanks for reviewing.
Currently we keep a list of drivers in a static array. As we are
acquiring more and drivers I would like to move to linker generated
lists as Linux and U-Boot use the for enumerating drivers.
So a driver could look like:
int drv1_check(unsigned int type, unsigned int reason) {...}
void drv1_reset(unsigned int type, unsigned int reason) {...}
SRST_DRIVER(drv1) = {
.name = "driver_1",
.check = drv1_check,
.reset = drv1_reset,
};
The linker will create a list of all SRST_DRIVER() instances over which
we can iterate in our code.
Does this make sense to you?
Example code is available in
https://github.com/xypron/linker_generated_lists
Best regards
Heinrich
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/1] lib: utils: identify supported GPIO reset methods
2021-09-29 7:01 ` Heinrich Schuchardt
@ 2021-09-29 12:38 ` Anup Patel
2021-09-30 10:29 ` Heinrich Schuchardt
0 siblings, 1 reply; 9+ messages in thread
From: Anup Patel @ 2021-09-29 12:38 UTC (permalink / raw)
To: opensbi
On Wed, Sep 29, 2021 at 12:31 PM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
>
>
> On 9/29/21 6:16 AM, Anup Patel wrote:
> > +Nikita
> >
> > On Tue, Sep 28, 2021 at 5:13 PM Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> The GPIO reset driver supports reset and poweroff. But not all boards
> >> support both. gpio_system_reset_check() must detect this situation.
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >
> > Looks good to me.
> >
> > Reviewed-by: Anup Patel <anup.patel@wdc.com>
> >
> > I had mentioned on the PMIC reset series that we need to improve
> > the sbi_system.h device registration such that reset drivers can
> > register a reset device for a range of reset types. This will allow
> > separate reset drivers (e.g. PMIC+GPIO) for SiFive Unmatched.
> > Also, reset_check() callback will not be required anymore.
>
> Hello Anup,
>
> thanks for reviewing.
>
> Currently we keep a list of drivers in a static array. As we are
> acquiring more and drivers I would like to move to linker generated
> lists as Linux and U-Boot use the for enumerating drivers.
>
> So a driver could look like:
>
> int drv1_check(unsigned int type, unsigned int reason) {...}
>
> void drv1_reset(unsigned int type, unsigned int reason) {...}
>
> SRST_DRIVER(drv1) = {
> .name = "driver_1",
> .check = drv1_check,
> .reset = drv1_reset,
> };
>
> The linker will create a list of all SRST_DRIVER() instances over which
> we can iterate in our code.
>
> Does this make sense to you?
>
> Example code is available in
> https://github.com/xypron/linker_generated_lists
I certainly like the Linker list approach. In fact, I have
implemented something similar in-past for Xvisor built-in
modules.
We can go ahead with Linker list approach with
following considerations:
1) The generic SBI library (i.e lib/sbi and include/sbi/
directories) should not use linker list because we have
couple of projects (e.g. Microchip HSS and EDK2)
which directly link with the OpenSBI generic library
instead of using OpenSBI firmwares.
2) This should only impact FDT based drivers in
SBI utils (i.e. lib/utils and include/sbi_utils directories).
Basically, all FDT based driver frameworks under
lib/utils can happily move to the Linker list approach
so that we don't have to maintain various xyz_drivers[]
arrays across different FDT based driver frameworks.
Agree ??
Regards,
Anup
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/1] lib: utils: identify supported GPIO reset methods
2021-09-29 12:38 ` Anup Patel
@ 2021-09-30 10:29 ` Heinrich Schuchardt
2021-09-30 13:13 ` Anup Patel
0 siblings, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2021-09-30 10:29 UTC (permalink / raw)
To: opensbi
On 9/29/21 14:38, Anup Patel wrote:
> On Wed, Sep 29, 2021 at 12:31 PM Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>>
>>
>> On 9/29/21 6:16 AM, Anup Patel wrote:
>>> +Nikita
>>>
>>> On Tue, Sep 28, 2021 at 5:13 PM Heinrich Schuchardt
>>> <heinrich.schuchardt@canonical.com> wrote:
>>>>
>>>> The GPIO reset driver supports reset and poweroff. But not all boards
>>>> support both. gpio_system_reset_check() must detect this situation.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>>
>>> Looks good to me.
>>>
>>> Reviewed-by: Anup Patel <anup.patel@wdc.com>
>>>
>>> I had mentioned on the PMIC reset series that we need to improve
>>> the sbi_system.h device registration such that reset drivers can
>>> register a reset device for a range of reset types. This will allow
>>> separate reset drivers (e.g. PMIC+GPIO) for SiFive Unmatched.
>>> Also, reset_check() callback will not be required anymore.
>>
>> Hello Anup,
>>
>> thanks for reviewing.
>>
>> Currently we keep a list of drivers in a static array. As we are
>> acquiring more and drivers I would like to move to linker generated
>> lists as Linux and U-Boot use the for enumerating drivers.
>>
>> So a driver could look like:
>>
>> int drv1_check(unsigned int type, unsigned int reason) {...}
>>
>> void drv1_reset(unsigned int type, unsigned int reason) {...}
>>
>> SRST_DRIVER(drv1) = {
>> .name = "driver_1",
>> .check = drv1_check,
>> .reset = drv1_reset,
>> };
>>
>> The linker will create a list of all SRST_DRIVER() instances over which
>> we can iterate in our code.
>>
>> Does this make sense to you?
>>
>> Example code is available in
>> https://github.com/xypron/linker_generated_lists
>
> I certainly like the Linker list approach. In fact, I have
> implemented something similar in-past for Xvisor built-in
> modules.
>
> We can go ahead with Linker list approach with
> following considerations:
> 1) The generic SBI library (i.e lib/sbi and include/sbi/
> directories) should not use linker list because we have
> couple of projects (e.g. Microchip HSS and EDK2)
> which directly link with the OpenSBI generic library
> instead of using OpenSBI firmwares.
The platform specification requires:
"the UEFI ResetSystem() service must be implemented via the SBI System
Reset Extension."
Is this compatible with linking part of SBI as a library but excluding
reset drivers?
> 2) This should only impact FDT based drivers in
> SBI utils (i.e. lib/utils and include/sbi_utils directories).
>
> Basically, all FDT based driver frameworks under
> lib/utils can happily move to the Linker list approach
> so that we don't have to maintain various xyz_drivers[]
> arrays across different FDT based driver frameworks.
>
> Agree ??
While trying to implement the linker list for OpenSBI I ran into some
problem with the linker:
https://sourceware.org/bugzilla/show_bug.cgi?id=28398
So I would suggest we start by converting the interface using an array.
Best regards
Heinrich
>
> Regards,
> Anup
>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/1] lib: utils: identify supported GPIO reset methods
2021-09-30 10:29 ` Heinrich Schuchardt
@ 2021-09-30 13:13 ` Anup Patel
0 siblings, 0 replies; 9+ messages in thread
From: Anup Patel @ 2021-09-30 13:13 UTC (permalink / raw)
To: opensbi
On Thu, Sep 30, 2021 at 3:59 PM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 9/29/21 14:38, Anup Patel wrote:
> > On Wed, Sep 29, 2021 at 12:31 PM Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >>
> >>
> >> On 9/29/21 6:16 AM, Anup Patel wrote:
> >>> +Nikita
> >>>
> >>> On Tue, Sep 28, 2021 at 5:13 PM Heinrich Schuchardt
> >>> <heinrich.schuchardt@canonical.com> wrote:
> >>>>
> >>>> The GPIO reset driver supports reset and poweroff. But not all boards
> >>>> support both. gpio_system_reset_check() must detect this situation.
> >>>>
> >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >>>
> >>> Looks good to me.
> >>>
> >>> Reviewed-by: Anup Patel <anup.patel@wdc.com>
> >>>
> >>> I had mentioned on the PMIC reset series that we need to improve
> >>> the sbi_system.h device registration such that reset drivers can
> >>> register a reset device for a range of reset types. This will allow
> >>> separate reset drivers (e.g. PMIC+GPIO) for SiFive Unmatched.
> >>> Also, reset_check() callback will not be required anymore.
> >>
> >> Hello Anup,
> >>
> >> thanks for reviewing.
> >>
> >> Currently we keep a list of drivers in a static array. As we are
> >> acquiring more and drivers I would like to move to linker generated
> >> lists as Linux and U-Boot use the for enumerating drivers.
> >>
> >> So a driver could look like:
> >>
> >> int drv1_check(unsigned int type, unsigned int reason) {...}
> >>
> >> void drv1_reset(unsigned int type, unsigned int reason) {...}
> >>
> >> SRST_DRIVER(drv1) = {
> >> .name = "driver_1",
> >> .check = drv1_check,
> >> .reset = drv1_reset,
> >> };
> >>
> >> The linker will create a list of all SRST_DRIVER() instances over which
> >> we can iterate in our code.
> >>
> >> Does this make sense to you?
> >>
> >> Example code is available in
> >> https://github.com/xypron/linker_generated_lists
> >
> > I certainly like the Linker list approach. In fact, I have
> > implemented something similar in-past for Xvisor built-in
> > modules.
> >
> > We can go ahead with Linker list approach with
> > following considerations:
> > 1) The generic SBI library (i.e lib/sbi and include/sbi/
> > directories) should not use linker list because we have
> > couple of projects (e.g. Microchip HSS and EDK2)
> > which directly link with the OpenSBI generic library
> > instead of using OpenSBI firmwares.
>
> The platform specification requires:
>
> "the UEFI ResetSystem() service must be implemented via the SBI System
> Reset Extension."
>
> Is this compatible with linking part of SBI as a library but excluding
> reset drivers?
For Microchip HSS, I had added a compile-time option to
use external OpenSBI firmware so they have a way to
stay compliant.
For EDK2, we have two problems:
1) It's boot-flow assumes to start from M-mode so we
can't use it inside Guest/VM (VS-mode)
2) the UEFI ResetSystem() compliance with platform
spec (which you mentioned)
Changing EDK2 boot-flow to use external OpenSBI
firmware is more involved. Someone need to get this
done soon.
>
> > 2) This should only impact FDT based drivers in
> > SBI utils (i.e. lib/utils and include/sbi_utils directories).
> >
> > Basically, all FDT based driver frameworks under
> > lib/utils can happily move to the Linker list approach
> > so that we don't have to maintain various xyz_drivers[]
> > arrays across different FDT based driver frameworks.
> >
> > Agree ??
>
> While trying to implement the linker list for OpenSBI I ran into some
> problem with the linker:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=28398
That's unfortunate.
>
> So I would suggest we start by converting the interface using an array.
How about we write a script which:
1) Takes list of C files as input
2) It will scan for a macro in each C file and output
an array into a generated C file
3) The generated C file is compiled and linked with
OpenSBI
4) Each driver framework will have it's own generated
C file
OR
Do you have some other ideas?
Regards,
Anup
>
> Best regards
>
> Heinrich
>
> >
> > Regards,
> > Anup
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] lib: utils: identify supported GPIO reset methods
2021-09-29 4:16 ` Anup Patel
2021-09-29 7:01 ` Heinrich Schuchardt
@ 2021-09-29 9:14 ` Nikita Shubin
2021-09-29 12:03 ` Heinrich Schuchardt
2021-10-01 4:58 ` Anup Patel
2 siblings, 1 reply; 9+ messages in thread
From: Nikita Shubin @ 2021-09-29 9:14 UTC (permalink / raw)
To: opensbi
On Wed, 29 Sep 2021 09:46:55 +0530
Anup Patel <anup@brainfault.org> wrote:
> +Nikita
>
> On Tue, Sep 28, 2021 at 5:13 PM Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > The GPIO reset driver supports reset and poweroff. But not all
> > boards support both. gpio_system_reset_check() must detect this
> > situation.
> >
> > Signed-off-by: Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com>
>
> Looks good to me.
>
> Reviewed-by: Anup Patel <anup.patel@wdc.com>
>
> I had mentioned on the PMIC reset series that we need to improve
> the sbi_system.h device registration such that reset drivers can
> register a reset device for a range of reset types. This will allow
> separate reset drivers (e.g. PMIC+GPIO) for SiFive Unmatched.
> Also, reset_check() callback will not be required anymore.
>
> Regards,
> Anup
Looks good to me, however
> > + /* hang !!! */
> > + sbi_hart_hang();
> > }
Heinrich do we really need to hang here ?
Despite the gpio_system_reset should never reach the end of function,
i remember you talked something about returning from function back to
the sbi_system_reset.
Yours,
Nikita Shubin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] lib: utils: identify supported GPIO reset methods
2021-09-29 9:14 ` Nikita Shubin
@ 2021-09-29 12:03 ` Heinrich Schuchardt
0 siblings, 0 replies; 9+ messages in thread
From: Heinrich Schuchardt @ 2021-09-29 12:03 UTC (permalink / raw)
To: opensbi
On 9/29/21 11:14, Nikita Shubin wrote:
> On Wed, 29 Sep 2021 09:46:55 +0530
> Anup Patel <anup@brainfault.org> wrote:
>
>> +Nikita
>>
>> On Tue, Sep 28, 2021 at 5:13 PM Heinrich Schuchardt
>> <heinrich.schuchardt@canonical.com> wrote:
>>>
>>> The GPIO reset driver supports reset and poweroff. But not all
>>> boards support both. gpio_system_reset_check() must detect this
>>> situation.
>>>
>>> Signed-off-by: Heinrich Schuchardt
>>> <heinrich.schuchardt@canonical.com>
>>
>> Looks good to me.
>>
>> Reviewed-by: Anup Patel <anup.patel@wdc.com>
>>
>> I had mentioned on the PMIC reset series that we need to improve
>> the sbi_system.h device registration such that reset drivers can
>> register a reset device for a range of reset types. This will allow
>> separate reset drivers (e.g. PMIC+GPIO) for SiFive Unmatched.
>> Also, reset_check() callback will not be required anymore.
>>
>> Regards,
>> Anup
>
> Looks good to me, however
>
>>> + /* hang !!! */
>>> + sbi_hart_hang();
>>> }
>
> Heinrich do we really need to hang here ?
>
> Despite the gpio_system_reset should never reach the end of function,
> i remember you talked something about returning from function back to
> the sbi_system_reset.
Thanks for reviewing.
As we are calling the check function before invoking the reset function
this hang will only be running until the power supply is drained.
A calling function is defined as __no_return. Once we correct the whole
reset driver framework as proposed by Anup we have to remove the
sbi_hart_hang() here and in other drivers. We then need a small wait
here and shall return SBI_EFAIL in the most improbable case.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] lib: utils: identify supported GPIO reset methods
2021-09-29 4:16 ` Anup Patel
2021-09-29 7:01 ` Heinrich Schuchardt
2021-09-29 9:14 ` Nikita Shubin
@ 2021-10-01 4:58 ` Anup Patel
2 siblings, 0 replies; 9+ messages in thread
From: Anup Patel @ 2021-10-01 4:58 UTC (permalink / raw)
To: opensbi
On Wed, Sep 29, 2021 at 9:46 AM Anup Patel <anup@brainfault.org> wrote:
>
> +Nikita
>
> On Tue, Sep 28, 2021 at 5:13 PM Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > The GPIO reset driver supports reset and poweroff. But not all boards
> > support both. gpio_system_reset_check() must detect this situation.
> >
> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>
> Looks good to me.
>
> Reviewed-by: Anup Patel <anup.patel@wdc.com>
Applied this patch to the riscv/opensbi repo.
Thanks,
Anup
>
> I had mentioned on the PMIC reset series that we need to improve
> the sbi_system.h device registration such that reset drivers can
> register a reset device for a range of reset types. This will allow
> separate reset drivers (e.g. PMIC+GPIO) for SiFive Unmatched.
> Also, reset_check() callback will not be required anymore.
>
> Regards,
> Anup
>
> > ---
> > lib/utils/reset/fdt_reset_gpio.c | 44 +++++++++++++++-----------------
> > 1 file changed, 21 insertions(+), 23 deletions(-)
> >
> > diff --git a/lib/utils/reset/fdt_reset_gpio.c b/lib/utils/reset/fdt_reset_gpio.c
> > index 30127f5..77e4d0e 100644
> > --- a/lib/utils/reset/fdt_reset_gpio.c
> > +++ b/lib/utils/reset/fdt_reset_gpio.c
> > @@ -35,21 +35,9 @@ static struct gpio_reset restart = {
> > .inactive_delay = 100
> > };
> >
> > -static int gpio_system_reset_check(u32 type, u32 reason)
> > -{
> > - switch (type) {
> > - case SBI_SRST_RESET_TYPE_SHUTDOWN:
> > - case SBI_SRST_RESET_TYPE_COLD_REBOOT:
> > - case SBI_SRST_RESET_TYPE_WARM_REBOOT:
> > - return 1;
> > - }
> > -
> > - return 0;
> > -}
> > -
> > -static void gpio_system_reset(u32 type, u32 reason)
> > +static struct gpio_reset *gpio_get_reset_settings(u32 type)
> > {
> > - struct gpio_reset *reset = NULL;
> > + struct gpio_reset *reset;
> >
> > switch (type) {
> > case SBI_SRST_RESET_TYPE_SHUTDOWN:
> > @@ -59,14 +47,26 @@ static void gpio_system_reset(u32 type, u32 reason)
> > case SBI_SRST_RESET_TYPE_WARM_REBOOT:
> > reset = &restart;
> > break;
> > + default:
> > + reset = NULL;
> > }
> >
> > - if (reset) {
> > - if (!reset->pin.chip) {
> > - sbi_printf("%s: gpio pin not available\n", __func__);
> > - goto skip_reset;
> > - }
> > + if (reset && !reset->pin.chip)
> > + reset = NULL;
> > +
> > + return reset;
> > +}
> > +
> > +static int gpio_system_reset_check(u32 type, u32 reason)
> > +{
> > + return !!gpio_get_reset_settings(type);
> > +}
> > +
> > +static void gpio_system_reset(u32 type, u32 reason)
> > +{
> > + struct gpio_reset *reset = gpio_get_reset_settings(type);
> >
> > + if (reset) {
> > /* drive it active, also inactive->active edge */
> > gpio_direction_output(&reset->pin, 1);
> > sbi_timer_mdelay(reset->active_delay);
> > @@ -77,11 +77,9 @@ static void gpio_system_reset(u32 type, u32 reason)
> >
> > /* drive it active, also inactive->active edge */
> > gpio_set(&reset->pin, 1);
> > -
> > -skip_reset:
> > - /* hang !!! */
> > - sbi_hart_hang();
> > }
> > + /* hang !!! */
> > + sbi_hart_hang();
> > }
> >
> > static struct sbi_system_reset_device gpio_reset = {
> > --
> > 2.32.0
> >
> >
> > --
> > opensbi mailing list
> > opensbi at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-10-01 4:58 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-28 11:42 [PATCH 1/1] lib: utils: identify supported GPIO reset methods Heinrich Schuchardt
2021-09-29 4:16 ` Anup Patel
2021-09-29 7:01 ` Heinrich Schuchardt
2021-09-29 12:38 ` Anup Patel
2021-09-30 10:29 ` Heinrich Schuchardt
2021-09-30 13:13 ` Anup Patel
2021-09-29 9:14 ` Nikita Shubin
2021-09-29 12:03 ` Heinrich Schuchardt
2021-10-01 4:58 ` Anup Patel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox