[PATCH v2 0/5] I2C framework, reboot Unmatched via PMIC

[Alexandre Ghiti]

On Wed, Oct 20, 2021 at 8:17 AM Nikita Shubin <nikita.shubin at maquefel.me> wrote:
>
> Hello Alexandre!
>
> On Wed, 20 Oct 2021 06:59:25 +0200
> Alexandre Ghiti <alexandre.ghiti at canonical.com> wrote:
>
> > On Fri, Oct 15, 2021 at 3:20 PM Nikita Shubin
> > <nikita.shubin at maquefel.me> wrote:
> > >
> > > From: Nikita Shubin <n.shubin at yadro.com>
> > >
> > > This series introduce rebooting via i2c da9063 PMIC, currently on
> > > SiFive Unmatched board.
> > >
> > > "gpio-poweroff" remains with default priority of 128 - default
> > > priority, da9063 is 1 by default the least priority.
> > >
> > > da9063-reset {
> > >         compatible = "da9063-reset";
> > >         priority = <1>;
> > > };
> > >
> > > Is required to be added as a child node of PMIC.
> > >
> > > tested via Linux and U-Boot with reset extension:
> > >
> > > OpenSBI:
> > > Platform Reboot Device    : da9063-reset
> > > Platform Shutdown Device  : gpio-reset
> > >
> > > v1 -> v2:
> > > Added:
> > > lib: sbi: add priority for reset handler
> > >
> > > Renamed read/write to smbus_write/smbus_read, as actually this
> > > is not a "raw" read/write but a one with a register address
> > > provided, later a "raw" version will be need but currently i don't
> > > have anything to test it.
> > >
> > > To Xiang W:
> > > I have analized your proposal of switching to sbi_list,
> > > and switched i2c adapters array to list. Unfortunately
> > > to switch drivers we require "init" functions.
> > >
> > > to Alexander Ghiti:
> > >
> > > > No need for the struct i2c_adapter argument as it is globally
> > > > accessible.
> > >
> > > It looks like a more clean and reusable way to me, let's here more
> > > opinions
> > > > Why should the da9063 device care about the adapter configuration
> > > > here? I think It should just rely on an already
> > > > configured/initialized i2c controller.
> > >
> > > With clocks init added to sifive_i2c configure it can become usable
> > > even if nobody cared about controller initialization.
> > >
> > > > Establishing the link between the i2c device and its adapter
> > > > should somehow be implicitly done by the i2c library, IMO the
> > > > device should not care about its controller.
> > >
> > > And how do you think it will look like ? In out case it's not a
> > > separate driver/client entity but all togather for simplicity.
> > >
> > > Nikita Shubin (5):
> > >   lib: utils/reset: add priority to gpio reset
> > >   lib: utils/i2c: Add generic I2C configuration library
> > >   lib: utils/i2c: Add simple FDT based I2C framework
> > >   lib: utils/i2c: Add minimal SiFive I2C driver
> > >   lib: utils/reset: Add generic da9063 reset driver
> > >
> > >  include/sbi_utils/i2c/fdt_i2c.h    |  26 +++
> > >  include/sbi_utils/i2c/i2c.h        |  99 +++++++++++
> > >  lib/utils/i2c/fdt_i2c.c            | 111 ++++++++++++
> > >  lib/utils/i2c/fdt_i2c_sifive.c     | 271
> > > +++++++++++++++++++++++++++++ lib/utils/i2c/i2c.c                |
> > > 85 +++++++++ lib/utils/i2c/objects.mk           |  12 ++
> > >  lib/utils/reset/fdt_reset.c        |   2 +
> > >  lib/utils/reset/fdt_reset_da9063.c | 214 +++++++++++++++++++++++
> > >  lib/utils/reset/fdt_reset_gpio.c   |  18 +-
> > >  lib/utils/reset/objects.mk         |   1 +
> > >  10 files changed, 836 insertions(+), 3 deletions(-)
> > >  create mode 100644 include/sbi_utils/i2c/fdt_i2c.h
> > >  create mode 100644 include/sbi_utils/i2c/i2c.h
> > >  create mode 100644 lib/utils/i2c/fdt_i2c.c
> > >  create mode 100644 lib/utils/i2c/fdt_i2c_sifive.c
> > >  create mode 100644 lib/utils/i2c/i2c.c
> > >  create mode 100644 lib/utils/i2c/objects.mk
> > >  create mode 100644 lib/utils/reset/fdt_reset_da9063.c
> > >
> > > --
> > > 2.31.1
> > >
> >
> > I'm testing your patchset and although it seems to work from Linux, it
> > fails when used from u-boot at the first smbus_write in
> > da9063_sanity_check. Maybe it relies on some initialization done by
> > the Linux driver?
> >
>
> That's sound strange becouse i explicitly tested with u-boot SBI
> extension, well in fact i tested mostly via u-boot because it's quicker
> than booting Linux.
>
> Do you have CONFIG_SYS_I2C_OCORES enabled ?
>
> I tested on u-boot v2021.07 with Sifive Unmatched patches and SBI reset
> extension applied. May it is time for configure ;) ?
>
> Sharing my u-boot branch:
> https://github.com/maquefel/u-boot/tree/yadro/riscv/unmatched-v2021.07

That's weird, it still does not work for me from u-boot with the following:

- openSBI: https://github.com/AlexGhiti/opensbi/tree/int/alex/nikita_pmic_reset_v2
- u-boot: https://github.com/maquefel/u-boot/tree/yadro/riscv/unmatched-v2021.07

And this fails this way:

=> reset
resetting ...
da9063_system_reset: chip is not da9063 PMIC

Alex







>
>
> > Thanks,
> >
> > Alex
> >
> >
> > >
> > > --
> > > opensbi mailing list
> > > opensbi at lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/opensbi
>