[RFI] pinmux: pin group ownership

[RFI] pinmux: pin group ownership

[Jon Hunter]

Hello,

I have been reviewing the pinmux driver code and I stumbled upon the
following change.

e5b3b2d9ed202697a937c282f9c4d93b1e3e0848 (pinctrl: allows not to define
the get_group_pins operation)

Looking at this change, if pinmux_enable_setting() is called but
.get_group_pins() is not defined, then num_pins will be 0. If this is
the case then pin_request() is not called to allocate the pins in the
group (because no pins are defined for the group). So that makes sense.

However, I am trying to understand then, if the pinmux driver will
protect against another device attempting to use the same group for a
different function when already in-use?

For example, if you have the two functions i2c0 and uart0 mapped to pin
group A, but no pins are defined for group A, will pinmux prevent
someone attempting to configure both functions on the same group at the
same time?

I did not see anywhere that sets a usecount for a group (ie. allocates
the group) but only for a pin.

Cheers
Jon

Re: [RFI] pinmux: pin group ownership

[Linus Walleij]

On Fri, May 8, 2015 at 5:01 PM, Jon Hunter <jonathanh@nvidia.com> wrote:

> Looking at this change, if pinmux_enable_setting() is called but
> .get_group_pins() is not defined, then num_pins will be 0. If this is
> the case then pin_request() is not called to allocate the pins in the
> group (because no pins are defined for the group). So that makes sense.
>
> However, I am trying to understand then, if the pinmux driver will
> protect against another device attempting to use the same group for a
> different function when already in-use?
>
> For example, if you have the two functions i2c0 and uart0 mapped to pin
> group A, but no pins are defined for group A, will pinmux prevent
> someone attempting to configure both functions on the same group at the
> same time?
>
> I did not see anywhere that sets a usecount for a group (ie. allocates
> the group) but only for a pin.

The usecount are done on individual pins, not groups.

In the devel tree you can even set the pinmux_ops.strict to disallow
GPIOs and other functions to share a pin.

Yours,
Linus Walleij

Re: [RFI] pinmux: pin group ownership

[Jon Hunter]

Hi Linus,

On 12/05/15 12:30, Linus Walleij wrote:
> On Fri, May 8, 2015 at 5:01 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
> 
>> Looking at this change, if pinmux_enable_setting() is called but
>> .get_group_pins() is not defined, then num_pins will be 0. If this is
>> the case then pin_request() is not called to allocate the pins in the
>> group (because no pins are defined for the group). So that makes sense.
>>
>> However, I am trying to understand then, if the pinmux driver will
>> protect against another device attempting to use the same group for a
>> different function when already in-use?
>>
>> For example, if you have the two functions i2c0 and uart0 mapped to pin
>> group A, but no pins are defined for group A, will pinmux prevent
>> someone attempting to configure both functions on the same group at the
>> same time?
>>
>> I did not see anywhere that sets a usecount for a group (ie. allocates
>> the group) but only for a pin.
> 
> The usecount are done on individual pins, not groups.

Thanks for confirming. With regard to the change
e5b3b2d9ed202697a937c282f9c4d93b1e3e0848, IIUC then this allows devices
not to define any pins but just groups. Probably because from some
devices (such as berlin) pinmux'ing is always done at the group level.
If this is the case then it would appear that there is no protection
against two devices trying to use the same group. Is this correct?

> In the devel tree you can even set the pinmux_ops.strict to disallow
> GPIOs and other functions to share a pin.

Thanks, however, I don't think that this helps in this case because
pin_request() would not be called by pinmux_enable_setting() as num_pins
= 0.

Cheers
Jon

Re: [RFI] pinmux: pin group ownership

[Linus Walleij]

On Tue, May 12, 2015 at 1:53 PM, Jon Hunter <jonathanh@nvidia.com> wrote:

[Me]:
>> The usecount are done on individual pins, not groups.
>
> Thanks for confirming. With regard to the change
> e5b3b2d9ed202697a937c282f9c4d93b1e3e0848, IIUC then this allows devices
> not to define any pins but just groups. Probably because from some
> devices (such as berlin) pinmux'ing is always done at the group level.

That's correct.

> If this is the case then it would appear that there is no protection
> against two devices trying to use the same group. Is this correct?

Seems like so... darn you found a loophole.

>> In the devel tree you can even set the pinmux_ops.strict to disallow
>> GPIOs and other functions to share a pin.
>
> Thanks, however, I don't think that this helps in this case because
> pin_request() would not be called by pinmux_enable_setting() as num_pins
> = 0.

You're right.

The implementation assumes collissions happen at pin level,
not group level.

I wonder if there are enough devices out there with this problem
that we need to check for it?

If so I would opt to create a bool .group_only property
on pinmux_ops like we created .strict so that we can
avoid all pin-specific code on such pin controllers.

But I don't know it it's worth it for these few pin controllers :/

On the U300 there is only group selection, but the datasheet
helpfully defined all pins anyway so I could handle collissions
on the pin level. This is the ideal solution if possible.

Yours,
Linus Walleij

Re: [RFI] pinmux: pin group ownership

[Jon Hunter]

On 12/05/15 13:10, Linus Walleij wrote:
> On Tue, May 12, 2015 at 1:53 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
> 
> [Me]:
>>> The usecount are done on individual pins, not groups.
>>
>> Thanks for confirming. With regard to the change
>> e5b3b2d9ed202697a937c282f9c4d93b1e3e0848, IIUC then this allows devices
>> not to define any pins but just groups. Probably because from some
>> devices (such as berlin) pinmux'ing is always done at the group level.
> 
> That's correct.
> 
>> If this is the case then it would appear that there is no protection
>> against two devices trying to use the same group. Is this correct?
> 
> Seems like so... darn you found a loophole.

Ok, I was hoping I had missed something, so thanks for confirming.

>>> In the devel tree you can even set the pinmux_ops.strict to disallow
>>> GPIOs and other functions to share a pin.
>>
>> Thanks, however, I don't think that this helps in this case because
>> pin_request() would not be called by pinmux_enable_setting() as num_pins
>> = 0.
> 
> You're right.
> 
> The implementation assumes collissions happen at pin level,
> not group level.
> 
> I wonder if there are enough devices out there with this problem
> that we need to check for it?

Probably not many, indeed.

> If so I would opt to create a bool .group_only property
> on pinmux_ops like we created .strict so that we can
> avoid all pin-specific code on such pin controllers.
> 
> But I don't know it it's worth it for these few pin controllers :/
> 
> On the U300 there is only group selection, but the datasheet
> helpfully defined all pins anyway so I could handle collissions
> on the pin level. This is the ideal solution if possible.

Yes, I agree registering all pins would be ideal, however, I could see
that this may seem unnecessary for some devices.

Thanks for the info.

Cheers
Jon

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------