Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Thu, Mar 9, 2023 at 3:26 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
> > On Tue, Mar 7, 2023 at 10:13 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add
> > > > pinctrl_get_device() Mon, Mar 06, 2023 at 09:00:02AM +0000, Biju Das
> > kirjoitti:

...

> > > > > Add pinctrl_get_device() to find a device handle associated with a
> > > > > pincontrol group(i.e. by matching function name and group name for
> > > > > a device). This device handle can then be used for finding match
> > > > > for the pin output disable device that protects device against
> > > > > short circuits on the pins.
> > > >
> > > > Not sure I understand the use case. Please, create a better commit
> > message.
> > >
> > > OK, Basically pinmux_enable_setting allows exclusive access of pin to a
> > device.
> > > It won't allow multiple devices to claim a pin.

This is a confusion you brought. You got us completely lost. Please,
try again from the clean sheet.

> > Which is correct. No? Show me the schematics of the real use case for that.
> >
> > The owner of the pin is the host side. I can't imagine how the same pin is
> > shared inside the SoC. Is it broken hardware design?
>
> We are discussing usage of
>
> echo "fname gname" and you asked a question whether multiple devices can
> claim a pin at the same time
>
> and my answer is no.

> as setting->data.mux will be unique for a pin and will be claimed by
> device during commit state.
>
> Am I missing anything here??

Yes. The same fname/gname can be in many *pin control* (provider) devices.

So, it does not uniquely define the pin control device.

...

> > > > Also it is missing the explanation why there will be no collisions
> > > > when looking by the same pair of function and group name from different
> > device.
> > >
> > > setting->data.mux will be unique for a pin. So there won't be a
> > > setting->collision when
> > > looking by the same pair of function and group name from different device.
> > >
> > > > (Always imagine that you have 2+ same IP blocks on the platform
> > > > before doing any pin control core work. This will help you to design
> > > > it properly. )
> >
> > Not sure how the pair function_name group_name makes the device unique.
>
> Do you agree Device handle + function_name +  group_name make it unique.

Yes.

> For pin outdisable we are making use of this 3 combination.
> See the details.
>
>
> This patch series adds support for controlling output disable function using
> sysfs in a generic way as described below.
>
> |    A     |    |     B      |    |     C     |    |     D        |  | E |
> |user space|--->|pinctrl core|<-->|SoC pinctrl|<-->|Output disable|--|PWM|
> |          |    |            |    |           |    |              |  |   |
>
> A executes command to configure a pin group for pin output disable operation
>   echo "fname gname conf conf_val" > configure
>
> B parses the command and identifies the binding device associated with that
>   pin group
>
> C matches the binding device against the device registered by D for
>   configuration operation
>
> D matches the pin group and configure the pins for Output disable operation
>
> Both D and E are linked together by dt(i.e. PWM channel linked with Output
>  disable Port)

Sounds like an overengineered hack to achieve something that I can't
read between the lines. Why is this so complicated interface and flow
are needed to begin with?


-- 
With Best Regards,
Andy Shevchenko