[PATCH v3 06/15] board: ns3: default reset type to L3

[Simon Glass]

Hi Rayagonda,

On Wed, 8 Jul 2020 at 01:01, Rayagonda Kokatanur
<rayagonda.kokatanur at broadcom.com> wrote:
>
> Hi Simon,
>
> On Fri, Jul 3, 2020 at 6:16 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Rayagonda,
> >
> > On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur
> > <rayagonda.kokatanur at broadcom.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Fri, Jun 26, 2020 at 6:42 AM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Hi Rayagonda,
> > > >
> > > > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur
> > > > <rayagonda.kokatanur at 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 at broadcom.com>
> > > > > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty at broadcom.com>
> > > > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at 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