Re: [PATCH v3 06/15] board: ns3: default reset type to L3
Hi Simon,
On Wed, Jul 8, 2020 at 8:31 PM Simon Glass wrote:
>
> Hi Rayagonda,
>
> On Wed, 8 Jul 2020 at 01:01, Rayagonda Kokatanur
> wrote:
> >
> > Hi Simon,
> >
> > On Fri, Jul 3, 2020 at 6:16 AM Simon Glass wrote:
> > >
> > > Hi Rayagonda,
> > >
> > > On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur
> > > wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Fri, Jun 26, 2020 at 6:42 AM Simon Glass wrote:
> > > > >
> > > > > Hi Rayagonda,
> > > > >
> > > > > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur
> > > > > wrote:
> > > > > >
> > > > > > Default "reset" from u-boot to L3 reset.
> > > > >
> > > > > U-Boot
> > > >
> > > > Thank you, will fix this.
> > > > >
> > > > > > "reset" command with argument will trigger L1 reset.
> > > > > >
> > > > > > Signed-off-by: Rajesh Ravi
> > > > > > Signed-off-by: Bharat Kumar Reddy Gooty
> > > > > > Signed-off-by: Rayagonda Kokatanur
> > > > > >
> > > > > > ---
> > > > > > board/broadcom/bcmns3/ns3.c | 20 ++--
> > > > > > 1 file changed, 18 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/board/broadcom/bcmns3/ns3.c
> > > > > > b/board/broadcom/bcmns3/ns3.c
> > > > > > index 5e644bd466..1221f26ddc 100644
> > > > > > --- a/board/broadcom/bcmns3/ns3.c
> > > > > > +++ b/board/broadcom/bcmns3/ns3.c
> > > > > > @@ -68,7 +68,23 @@ int dram_init_banksize(void)
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > -void reset_cpu(ulong addr)
> > > > > > +void reset_cpu(ulong level)
> > > > > > {
> > > > >
> > > > > This should be in a sysreset driver. It supports different reset
> > > > > types.
> > > >
> > > > I checked the sysreset driver and found a generic/common psci driver -
> > > > drivers/sysreset/sysreset_psci.c.
> > > > We could use this common driver in our platform.
> > > >
> > > > Right now this common driver uses the same command or function_id for
> > > > both WARM and COLD reset.
> > > > But in our case we should use different commands for WARM and COLD
> > > > reset.
> > > >
> > > > I am planning to add one kconfig option (USE_FN64_CMD) through which
> > > > we can select different commands or the same command for COLD reset.
> > > >
> > > > Something like this,
> > > >
> > > > static int psci_sysreset_request(struct udevice *dev, enum sysreset_t
> > > > type)
> > > > {
> > > > unsigned long function_id;
> > > >
> > > > switch (type) {
> > > > case SYSRESET_WARM:
> > > > function_id = PSCI_0_2_FN_SYSTEM_RESET;
> > > > break;
> > > >
> > > > case SYSRESET_COLD:
> > > > if (CONFIG_IS_ENABLED(USE_FN64_CMD))
> > > > function_id = PSCI_0_2_FN64_SYSTEM_RESET;
> > > > else
> > > > function_id = PSCI_0_2_FN_SYSTEM_RESET;
> > > > break;
> > > >
> > > > case SYSRESET_POWER_OFF:
> > > > function_id = PSCI_0_2_FN_SYSTEM_OFF;
> > > > break;
> > > >
> > > > default:
> > > > return -ENOSYS;
> > > > }
> > > >
> > > > invoke_psci_fn(function_id, 0, 0, 0);
> > > >
> > > > return -EINPROGRESS;
> > > > }
> > > >
> > > > This way any platform can define/select USE_FN64_CMD if they need
> > > > different commands for WARM reset.
> > > > Please let me know about this approach.
> > >
> > > It seems OK, but better would be to use the device tree to determine
> > > the command (e.g. compatible string or a property).
> >
> > I was checking linux psci drivers (drivers/firmware/psci/psci.c), they
> > are handling SYSTEM_RESET and SYSTEM_RESET2 without dt ie they read
> > psci capability, if it supports SYSTEM_RESET2 then they use it else
> > they use SYSTEM_RESET only.
> > I think, in U-Boot also we should handle in the same way.
> > This is a little bit more work, hence I am planning to handle this as
> > a separate patch set only.
> > So request you to allow this patch as it is.
> >
> > Please let me know.
>
> That's fine with me. So for now can we drop the Kconfig and just
> support SYSTEM_RESET?
For now I will keep this patch as it is, won't use UCLASS_SYSRESET.
Later I will push patch to,
-- use UCLASS_SYSRESET
-- Implement SYSTEM_RESET and SYSTEM_RESET2 as per linux.
Best regards,
Rayagonda
>
> Regards,
> Simon
Re: [PATCH v3 06/15] board: ns3: default reset type to L3
Hi Rayagonda,
On Wed, 8 Jul 2020 at 01:01, Rayagonda Kokatanur
wrote:
>
> Hi Simon,
>
> On Fri, Jul 3, 2020 at 6:16 AM Simon Glass wrote:
> >
> > Hi Rayagonda,
> >
> > On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur
> > wrote:
> > >
> > > Hi Simon,
> > >
> > > On Fri, Jun 26, 2020 at 6:42 AM Simon Glass wrote:
> > > >
> > > > Hi Rayagonda,
> > > >
> > > > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur
> > > > wrote:
> > > > >
> > > > > Default "reset" from u-boot to L3 reset.
> > > >
> > > > U-Boot
> > >
> > > Thank you, will fix this.
> > > >
> > > > > "reset" command with argument will trigger L1 reset.
> > > > >
> > > > > Signed-off-by: Rajesh Ravi
> > > > > Signed-off-by: Bharat Kumar Reddy Gooty
> > > > > Signed-off-by: Rayagonda Kokatanur
> > > > > ---
> > > > > board/broadcom/bcmns3/ns3.c | 20 ++--
> > > > > 1 file changed, 18 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
> > > > > index 5e644bd466..1221f26ddc 100644
> > > > > --- a/board/broadcom/bcmns3/ns3.c
> > > > > +++ b/board/broadcom/bcmns3/ns3.c
> > > > > @@ -68,7 +68,23 @@ int dram_init_banksize(void)
> > > > > return 0;
> > > > > }
> > > > >
> > > > > -void reset_cpu(ulong addr)
> > > > > +void reset_cpu(ulong level)
> > > > > {
> > > >
> > > > This should be in a sysreset driver. It supports different reset types.
> > >
> > > I checked the sysreset driver and found a generic/common psci driver -
> > > drivers/sysreset/sysreset_psci.c.
> > > We could use this common driver in our platform.
> > >
> > > Right now this common driver uses the same command or function_id for
> > > both WARM and COLD reset.
> > > But in our case we should use different commands for WARM and COLD reset.
> > >
> > > I am planning to add one kconfig option (USE_FN64_CMD) through which
> > > we can select different commands or the same command for COLD reset.
> > >
> > > Something like this,
> > >
> > > static int psci_sysreset_request(struct udevice *dev, enum sysreset_t
> > > type)
> > > {
> > > unsigned long function_id;
> > >
> > > switch (type) {
> > > case SYSRESET_WARM:
> > > function_id = PSCI_0_2_FN_SYSTEM_RESET;
> > > break;
> > >
> > > case SYSRESET_COLD:
> > > if (CONFIG_IS_ENABLED(USE_FN64_CMD))
> > > function_id = PSCI_0_2_FN64_SYSTEM_RESET;
> > > else
> > > function_id = PSCI_0_2_FN_SYSTEM_RESET;
> > > break;
> > >
> > > case SYSRESET_POWER_OFF:
> > > function_id = PSCI_0_2_FN_SYSTEM_OFF;
> > > break;
> > >
> > > default:
> > > return -ENOSYS;
> > > }
> > >
> > > invoke_psci_fn(function_id, 0, 0, 0);
> > >
> > > return -EINPROGRESS;
> > > }
> > >
> > > This way any platform can define/select USE_FN64_CMD if they need
> > > different commands for WARM reset.
> > > Please let me know about this approach.
> >
> > It seems OK, but better would be to use the device tree to determine
> > the command (e.g. compatible string or a property).
>
> I was checking linux psci drivers (drivers/firmware/psci/psci.c), they
> are handling SYSTEM_RESET and SYSTEM_RESET2 without dt ie they read
> psci capability, if it supports SYSTEM_RESET2 then they use it else
> they use SYSTEM_RESET only.
> I think, in U-Boot also we should handle in the same way.
> This is a little bit more work, hence I am planning to handle this as
> a separate patch set only.
> So request you to allow this patch as it is.
>
> Please let me know.
That's fine with me. So for now can we drop the Kconfig and just
support SYSTEM_RESET?
Regards,
Simon
Re: [PATCH v3 06/15] board: ns3: default reset type to L3
Hi Simon,
On Fri, Jul 3, 2020 at 6:16 AM Simon Glass wrote:
>
> Hi Rayagonda,
>
> On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur
> wrote:
> >
> > Hi Simon,
> >
> > On Fri, Jun 26, 2020 at 6:42 AM Simon Glass wrote:
> > >
> > > Hi Rayagonda,
> > >
> > > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur
> > > wrote:
> > > >
> > > > Default "reset" from u-boot to L3 reset.
> > >
> > > U-Boot
> >
> > Thank you, will fix this.
> > >
> > > > "reset" command with argument will trigger L1 reset.
> > > >
> > > > Signed-off-by: Rajesh Ravi
> > > > Signed-off-by: Bharat Kumar Reddy Gooty
> > > > Signed-off-by: Rayagonda Kokatanur
> > > > ---
> > > > board/broadcom/bcmns3/ns3.c | 20 ++--
> > > > 1 file changed, 18 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
> > > > index 5e644bd466..1221f26ddc 100644
> > > > --- a/board/broadcom/bcmns3/ns3.c
> > > > +++ b/board/broadcom/bcmns3/ns3.c
> > > > @@ -68,7 +68,23 @@ int dram_init_banksize(void)
> > > > return 0;
> > > > }
> > > >
> > > > -void reset_cpu(ulong addr)
> > > > +void reset_cpu(ulong level)
> > > > {
> > >
> > > This should be in a sysreset driver. It supports different reset types.
> >
> > I checked the sysreset driver and found a generic/common psci driver -
> > drivers/sysreset/sysreset_psci.c.
> > We could use this common driver in our platform.
> >
> > Right now this common driver uses the same command or function_id for
> > both WARM and COLD reset.
> > But in our case we should use different commands for WARM and COLD reset.
> >
> > I am planning to add one kconfig option (USE_FN64_CMD) through which
> > we can select different commands or the same command for COLD reset.
> >
> > Something like this,
> >
> > static int psci_sysreset_request(struct udevice *dev, enum sysreset_t type)
> > {
> > unsigned long function_id;
> >
> > switch (type) {
> > case SYSRESET_WARM:
> > function_id = PSCI_0_2_FN_SYSTEM_RESET;
> > break;
> >
> > case SYSRESET_COLD:
> > if (CONFIG_IS_ENABLED(USE_FN64_CMD))
> > function_id = PSCI_0_2_FN64_SYSTEM_RESET;
> > else
> > function_id = PSCI_0_2_FN_SYSTEM_RESET;
> > break;
> >
> > case SYSRESET_POWER_OFF:
> > function_id = PSCI_0_2_FN_SYSTEM_OFF;
> > break;
> >
> > default:
> > return -ENOSYS;
> > }
> >
> > invoke_psci_fn(function_id, 0, 0, 0);
> >
> > return -EINPROGRESS;
> > }
> >
> > This way any platform can define/select USE_FN64_CMD if they need
> > different commands for WARM reset.
> > Please let me know about this approach.
>
> It seems OK, but better would be to use the device tree to determine
> the command (e.g. compatible string or a property).
I was checking linux psci drivers (drivers/firmware/psci/psci.c), they
are handling SYSTEM_RESET and SYSTEM_RESET2 without dt ie they read
psci capability, if it supports SYSTEM_RESET2 then they use it else
they use SYSTEM_RESET only.
I think, in U-Boot also we should handle in the same way.
This is a little bit more work, hence I am planning to handle this as
a separate patch set only.
So request you to allow this patch as it is.
Please let me know.
Best regards,
Rayagonda
>
> Regards,
> Simon
Re: [PATCH v3 06/15] board: ns3: default reset type to L3
Hi Rayagonda,
On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur
wrote:
>
> Hi Simon,
>
> On Fri, Jun 26, 2020 at 6:42 AM Simon Glass wrote:
> >
> > Hi Rayagonda,
> >
> > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur
> > wrote:
> > >
> > > Default "reset" from u-boot to L3 reset.
> >
> > U-Boot
>
> Thank you, will fix this.
> >
> > > "reset" command with argument will trigger L1 reset.
> > >
> > > Signed-off-by: Rajesh Ravi
> > > Signed-off-by: Bharat Kumar Reddy Gooty
> > > Signed-off-by: Rayagonda Kokatanur
> > > ---
> > > board/broadcom/bcmns3/ns3.c | 20 ++--
> > > 1 file changed, 18 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
> > > index 5e644bd466..1221f26ddc 100644
> > > --- a/board/broadcom/bcmns3/ns3.c
> > > +++ b/board/broadcom/bcmns3/ns3.c
> > > @@ -68,7 +68,23 @@ int dram_init_banksize(void)
> > > return 0;
> > > }
> > >
> > > -void reset_cpu(ulong addr)
> > > +void reset_cpu(ulong level)
> > > {
> >
> > This should be in a sysreset driver. It supports different reset types.
>
> I checked the sysreset driver and found a generic/common psci driver -
> drivers/sysreset/sysreset_psci.c.
> We could use this common driver in our platform.
>
> Right now this common driver uses the same command or function_id for
> both WARM and COLD reset.
> But in our case we should use different commands for WARM and COLD reset.
>
> I am planning to add one kconfig option (USE_FN64_CMD) through which
> we can select different commands or the same command for COLD reset.
>
> Something like this,
>
> static int psci_sysreset_request(struct udevice *dev, enum sysreset_t type)
> {
> unsigned long function_id;
>
> switch (type) {
> case SYSRESET_WARM:
> function_id = PSCI_0_2_FN_SYSTEM_RESET;
> break;
>
> case SYSRESET_COLD:
> if (CONFIG_IS_ENABLED(USE_FN64_CMD))
> function_id = PSCI_0_2_FN64_SYSTEM_RESET;
> else
> function_id = PSCI_0_2_FN_SYSTEM_RESET;
> break;
>
> case SYSRESET_POWER_OFF:
> function_id = PSCI_0_2_FN_SYSTEM_OFF;
> break;
>
> default:
> return -ENOSYS;
> }
>
> invoke_psci_fn(function_id, 0, 0, 0);
>
> return -EINPROGRESS;
> }
>
> This way any platform can define/select USE_FN64_CMD if they need
> different commands for WARM reset.
> Please let me know about this approach.
It seems OK, but better would be to use the device tree to determine
the command (e.g. compatible string or a property).
Regards,
Simon
Re: [PATCH v3 06/15] board: ns3: default reset type to L3
Hi Simon,
On Fri, Jun 26, 2020 at 6:42 AM Simon Glass wrote:
>
> Hi Rayagonda,
>
> On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur
> wrote:
> >
> > Default "reset" from u-boot to L3 reset.
>
> U-Boot
Thank you, will fix this.
>
> > "reset" command with argument will trigger L1 reset.
> >
> > Signed-off-by: Rajesh Ravi
> > Signed-off-by: Bharat Kumar Reddy Gooty
> > Signed-off-by: Rayagonda Kokatanur
> > ---
> > board/broadcom/bcmns3/ns3.c | 20 ++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
> > index 5e644bd466..1221f26ddc 100644
> > --- a/board/broadcom/bcmns3/ns3.c
> > +++ b/board/broadcom/bcmns3/ns3.c
> > @@ -68,7 +68,23 @@ int dram_init_banksize(void)
> > return 0;
> > }
> >
> > -void reset_cpu(ulong addr)
> > +void reset_cpu(ulong level)
> > {
>
> This should be in a sysreset driver. It supports different reset types.
I checked the sysreset driver and found a generic/common psci driver -
drivers/sysreset/sysreset_psci.c.
We could use this common driver in our platform.
Right now this common driver uses the same command or function_id for
both WARM and COLD reset.
But in our case we should use different commands for WARM and COLD reset.
I am planning to add one kconfig option (USE_FN64_CMD) through which
we can select different commands or the same command for COLD reset.
Something like this,
static int psci_sysreset_request(struct udevice *dev, enum sysreset_t type)
{
unsigned long function_id;
switch (type) {
case SYSRESET_WARM:
function_id = PSCI_0_2_FN_SYSTEM_RESET;
break;
case SYSRESET_COLD:
if (CONFIG_IS_ENABLED(USE_FN64_CMD))
function_id = PSCI_0_2_FN64_SYSTEM_RESET;
else
function_id = PSCI_0_2_FN_SYSTEM_RESET;
break;
case SYSRESET_POWER_OFF:
function_id = PSCI_0_2_FN_SYSTEM_OFF;
break;
default:
return -ENOSYS;
}
invoke_psci_fn(function_id, 0, 0, 0);
return -EINPROGRESS;
}
This way any platform can define/select USE_FN64_CMD if they need
different commands for WARM reset.
Please let me know about this approach.
Best regards,
Rayagonda
>
> > - psci_system_reset();
> > +#define L3_RESET 30
> > + u32 reset_level, strap_val;
> > +
> > + /* Default reset type is L3 reset */
> > + if (!level) {
> > + /*
> > +* Encoding: u-boot reset command expects decimal argument
> > +* strap val = 1st decimal digit;reset level = 2nd decimal
> > digit
> > +*/
> > + strap_val = L3_RESET % 10;
> > + level = L3_RESET / 10;
> > + reset_level = level % 10;
> > + psci_system_reset2(reset_level, strap_val);
> > + } else {
> > + /* U-boot cmd "reset" with any arg will trigger L1 reset */
> > + psci_system_reset();
> > + }
> > }
> > --
> > 2.17.1
> >
>
> Regards,
> Simon
Re: [PATCH v3 06/15] board: ns3: default reset type to L3
Hi Rayagonda,
On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur
wrote:
>
> Default "reset" from u-boot to L3 reset.
U-Boot
> "reset" command with argument will trigger L1 reset.
>
> Signed-off-by: Rajesh Ravi
> Signed-off-by: Bharat Kumar Reddy Gooty
> Signed-off-by: Rayagonda Kokatanur
> ---
> board/broadcom/bcmns3/ns3.c | 20 ++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
> index 5e644bd466..1221f26ddc 100644
> --- a/board/broadcom/bcmns3/ns3.c
> +++ b/board/broadcom/bcmns3/ns3.c
> @@ -68,7 +68,23 @@ int dram_init_banksize(void)
> return 0;
> }
>
> -void reset_cpu(ulong addr)
> +void reset_cpu(ulong level)
> {
This should be in a sysreset driver. It supports different reset types.
> - psci_system_reset();
> +#define L3_RESET 30
> + u32 reset_level, strap_val;
> +
> + /* Default reset type is L3 reset */
> + if (!level) {
> + /*
> +* Encoding: u-boot reset command expects decimal argument
> +* strap val = 1st decimal digit;reset level = 2nd decimal
> digit
> +*/
> + strap_val = L3_RESET % 10;
> + level = L3_RESET / 10;
> + reset_level = level % 10;
> + psci_system_reset2(reset_level, strap_val);
> + } else {
> + /* U-boot cmd "reset" with any arg will trigger L1 reset */
> + psci_system_reset();
> + }
> }
> --
> 2.17.1
>
Regards,
Simon
[PATCH v3 06/15] board: ns3: default reset type to L3
Default "reset" from u-boot to L3 reset.
"reset" command with argument will trigger L1 reset.
Signed-off-by: Rajesh Ravi
Signed-off-by: Bharat Kumar Reddy Gooty
Signed-off-by: Rayagonda Kokatanur
---
board/broadcom/bcmns3/ns3.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
index 5e644bd466..1221f26ddc 100644
--- a/board/broadcom/bcmns3/ns3.c
+++ b/board/broadcom/bcmns3/ns3.c
@@ -68,7 +68,23 @@ int dram_init_banksize(void)
return 0;
}
-void reset_cpu(ulong addr)
+void reset_cpu(ulong level)
{
- psci_system_reset();
+#define L3_RESET 30
+ u32 reset_level, strap_val;
+
+ /* Default reset type is L3 reset */
+ if (!level) {
+ /*
+* Encoding: u-boot reset command expects decimal argument
+* strap val = 1st decimal digit;reset level = 2nd decimal digit
+*/
+ strap_val = L3_RESET % 10;
+ level = L3_RESET / 10;
+ reset_level = level % 10;
+ psci_system_reset2(reset_level, strap_val);
+ } else {
+ /* U-boot cmd "reset" with any arg will trigger L1 reset */
+ psci_system_reset();
+ }
}
--
2.17.1
