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

[Biju Das <biju.das.jz@xxxxxxxxxxxxxx>]

Hi Andy,

> Subject: RE: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
> 
> Hi Andy,
> 
> Thanks for the explanation.
> 
> > Subject: Re: [PATCH v6 01/13] pinctrl: core: Add pinctrl_get_device()
> >
> > 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.
> 
> I agree.
> 
> I have used the code used for [1] getting *pin control* devices associated
> with function name and group name.
> 
> [1]
> cat /sys/kernel/debug/pinctrl/pinctrl-handles
> 
> if output of [1], can return multiple devices for a given fname/gname, then
> I am wrong.
> Please correct me if that is the case.
> 
> So I was under the impression that [2], it will fail if multiple devices try
> to acquire a pin.
> 
> [2]
> https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/pinmux.c#L132


I have done the below modifications in my dt and confirm that, pinctrl framework
doesn't allow 2 devices to claim pins  with a given function name and group name
at same time.

dt-changes:
&gpt {
+               gpt6-pins {
+                       pinmux = <RZG2L_PORT_PINMUX(44, 0, 5)>, /* GTIOC6A */
+                                <RZG2L_PORT_PINMUX(44, 1, 5)>; /* GTIOC6B */
+               };
+
+               gpt7-pins {
+                       pinmux = <RZG2L_PORT_PINMUX(44, 2, 5)>, /* GTIOC7A */
+                                <RZG2L_PORT_PINMUX(44, 3, 5)>; /* GTIOC7B */
+               };
};

&mtu {

+               gpt6-pins {
+                       pinmux = <RZG2L_PORT_PINMUX(44, 0, 4)>, /* MTIOC3A */
+                                <RZG2L_PORT_PINMUX(44, 1, 4)>, /* MTIOC3B */
+                                <RZG2L_PORT_PINMUX(44, 2, 4)>, /* MTIOC3C */
+                                <RZG2L_PORT_PINMUX(44, 3, 4)>; /* MTIOC3D */
                };
};

Initially MTU device is claiming the pins P44_0 and P44_1.

root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/pinctrl-handles
	device: 10001200.timer current state: default
  state: default
    type: MUX_GROUP controller pinctrl-rzg2l group: mtu3_zphase_clk (3) function: mtu3_zphase_clk (3)
    type: MUX_GROUP controller pinctrl-rzg2l group: mtu3_clk (4) function: mtu3_clk (4)
    type: MUX_GROUP controller pinctrl-rzg2l group: gpt6-pins (5) function: gpt6-pins (5)

When It tried to load gpt, I get the below erro,
[   15.024714] pinctrl-rzg2l 11030000.pinctrl: pin P44_0 already requested by 10001200.timer; cannot claim for 10048000.pwm
         Starting Update UTMP about System Runlevel Changes...
[   15.044828] pinctrl-rzg2l 11030000.pinctrl: pin-352 (10048000.pwm) status -22
[   15.056515] pinctrl-rzg2l 11030000.pinctrl: could not request pin 352 (P44_0) from group gpt6-pins  on device pinctrl-rzg2l
[   15.070224] pwm-rzg2l-gpt 10048000.pwm: Error applying setting, reverse things back

After unloading mtu modules and loading gpt modules, gpt module is claims the pin.

root@smarc-rzg2l:~# rmmod rz_mtu3_cnt
root@smarc-rzg2l:~# rmmod pwm_rz_mtu3
root@smarc-rzg2l:~# cd /sys/bus/platform/drivers/rz-mtu3/
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# echo 10001200.timer > unbind
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# rmmod rzg2l_poeg
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# rmmod pwm_rzg2l_gpt
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# cat /sys/kernel/debug/pinctrl/pinctrl-handles | grep gpt
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3#

root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# modprobe rzg2l_poeg
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# modprobe pwm_rzg2l_gpt
root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# cat /sys/kernel/debug/pinctrl/pinctrl-handles | grep gpt
    type: MUX_GROUP controller pinctrl-rzg2l group: gpt6-pins (5) function: gpt6-pins (5)
    type: MUX_GROUP controller pinctrl-rzg2l group: gpt7-pins (11) function: gpt7-pins (11)

root@smarc-rzg2l:/sys/bus/platform/drivers/rz-mtu3# cat /sys/kernel/debug/pinctrl/pinctrl-handles
device: 10048000.pwm current state: default
  state: default
    type: MUX_GROUP controller pinctrl-rzg2l group: gpt6-pins (5) function: gpt6-pins (5)
    type: MUX_GROUP controller pinctrl-rzg2l group: gpt7-pins (11) function: gpt7-pins (11)


Since I have used same function name/ group name it gets a unique selector(5) and the functionality for
gpt is not working as the selector meant for MTU.

But if I replace "gpt6-pins" -> "mtu-pins" in MTU node, then the functionality works as expected as
the selector is different for mtu-pins and gpt-pins eventhough both uses same pins.

So I believe current implementation is based on /sys/kernel/debug/pinctrl/pinctrl-handles is
good unless I miss something here. Please correct me,if you think my observation is wrong.

Cheers,
Biju