linux-kernel - Re: [PATCH v2 1/1] i2c: designware: set pinctrl recovery information from device pinctrl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y5s7ofYGaec8+zIA@smile.fi.intel.com>
Date:   Thu, 15 Dec 2022 17:22:09 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     "Hawa, Hanna" <hhhawa@...zon.com>, Wolfram Sang <wsa@...nel.org>,
        jarkko.nikula@...ux.intel.com, mika.westerberg@...ux.intel.com,
        jsd@...ihalf.com, linux-i2c@...r.kernel.org,
        linux-kernel@...r.kernel.org, dwmw@...zon.co.uk, benh@...zon.com,
        ronenk@...zon.com, talel@...zon.com, jonnyc@...zon.com,
        hanochu@...zon.com, farbere@...zon.com, itamark@...zon.com
Subject: Re: [PATCH v2 1/1] i2c: designware: set pinctrl recovery information
 from device pinctrl

On Thu, Dec 15, 2022 at 03:06:13PM +0100, Linus Walleij wrote:
> On Thu, Dec 15, 2022 at 11:28 AM Andy Shevchenko
> <andriy.shevchenko@...ux.intel.com> wrote:
> > On Thu, Dec 15, 2022 at 10:15:35AM +0200, Hawa, Hanna wrote:
> > > On 12/14/2022 6:09 PM, Andy Shevchenko wrote:

...

> > > > > +     if (dev->dev->pins && dev->dev->pins->p)
> > > > > +             rinfo->pinctrl = dev->dev->pins->p;
> > > > Hmm... I don't see how this field is being used.
> > > > Can you elaborate?
> > >
> > > This field is used in i2c_generic_scl_recovery(), if it's not NULL then the
> > > flow will set the state to GPIO before running the recovery mechanism.
> > >         if (bri->pinctrl)
> > >                 pinctrl_select_state(bri->pinctrl, bri->pins_gpio);
> >
> > OK, but why that function doesn't use the dev->pins->p if it's defined?
> > (As a fallback when rinfo->pinctrl is NULL.)
> 
> I don't understand the context of these things so can't say much
> about it.

Main question here is that, is it possible that GPIOs that back up I²C bus are
on the different pin control device that the bus pins themselves?

And while writing above I think it may be the case if we use additional
GPIO pins over the I²C bus for purpose of recovery. In such case the pin control
device can be different.

At the same time, the fallback option might still work, in case the pinctrl not
overridden and I²C bus has backed GPIO function (on SCL/SDA).

...

> > > I saw that that the change failed in complication for SPARC architecture, as
> > > the pins field is wraparound with CONFIG_PINCTRL in device struct. I though
> > > on two options to solve the compilation error, first by adding wraparound of
> > > CONFIG_PINCTRL when accessing the pins field. And the second option is to
> > > add get function in pinctrl/devinfo.h file, which return the pins field, or
> > > NULL in case the PINCTRL is not defined. Which option you think we can go
> > > with?
> >
> > Getter with a stub sounds better to me, so you won't access some device core
> > fields.
> >
> > Linus, what do you think about all these (including previous paragraph)?
> 
> A getter may be a good solution, it depends, it can also be pushed
> somewhere local in the designware i2c driver can it not?

Yeah, but my point in the above paragraph that it uses the generic recovery
mechanism which may (or may not?) utilise the same pin control as I²C bus
sitting on).

> I am thinking that the rest of the code that is using that field is
> certainly not going to work without pinctrl either.

-- 
With Best Regards,
Andy Shevchenko