| Message ID | 20200610104120.30668-7-rayagonda.kokatanur@broadcom.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | add initial support for broadcom NS3 soc | expand |
Hi Rayagonda, On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> wrote: > > Default "reset" from u-boot to L3 reset. U-Boot > "reset" command with argument will trigger L1 reset. > > Signed-off-by: Rajesh Ravi <rajesh.ravi@broadcom.com> > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > --- > 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
Hi Simon, On Fri, Jun 26, 2020 at 6:42 AM Simon Glass <sjg@chromium.org> wrote: > > Hi Rayagonda, > > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur > <rayagonda.kokatanur@broadcom.com> 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 <rajesh.ravi@broadcom.com> > > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > > --- > > 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
Hi Rayagonda, On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> wrote: > > Hi Simon, > > On Fri, Jun 26, 2020 at 6:42 AM Simon Glass <sjg@chromium.org> wrote: > > > > Hi Rayagonda, > > > > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur > > <rayagonda.kokatanur@broadcom.com> 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 <rajesh.ravi@broadcom.com> > > > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > > > --- > > > 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
Hi Simon, On Fri, Jul 3, 2020 at 6:16 AM Simon Glass <sjg@chromium.org> wrote: > > Hi Rayagonda, > > On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur > <rayagonda.kokatanur@broadcom.com> wrote: > > > > Hi Simon, > > > > On Fri, Jun 26, 2020 at 6:42 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi Rayagonda, > > > > > > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur > > > <rayagonda.kokatanur@broadcom.com> 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 <rajesh.ravi@broadcom.com> > > > > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > > > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > > > > --- > > > > 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
Hi Rayagonda, On Wed, 8 Jul 2020 at 01:01, Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> wrote: > > Hi Simon, > > On Fri, Jul 3, 2020 at 6:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > Hi Rayagonda, > > > > On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur > > <rayagonda.kokatanur@broadcom.com> wrote: > > > > > > Hi Simon, > > > > > > On Fri, Jun 26, 2020 at 6:42 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > Hi Rayagonda, > > > > > > > > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur > > > > <rayagonda.kokatanur@broadcom.com> 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 <rajesh.ravi@broadcom.com> > > > > > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > > > > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > > > > > --- > > > > > 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
Hi Simon, On Wed, Jul 8, 2020 at 8:31 PM Simon Glass <sjg@chromium.org> wrote: > > Hi Rayagonda, > > On Wed, 8 Jul 2020 at 01:01, Rayagonda Kokatanur > <rayagonda.kokatanur@broadcom.com> wrote: > > > > Hi Simon, > > > > On Fri, Jul 3, 2020 at 6:16 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi Rayagonda, > > > > > > On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur > > > <rayagonda.kokatanur@broadcom.com> wrote: > > > > > > > > Hi Simon, > > > > > > > > On Fri, Jun 26, 2020 at 6:42 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > Hi Rayagonda, > > > > > > > > > > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur > > > > > <rayagonda.kokatanur@broadcom.com> 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 <rajesh.ravi@broadcom.com> > > > > > > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > > > > > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > > > > > > --- > > > > > > 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
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(); + } }