GPIOs not correctly exported via sysfs on ATSAMA5D2

GPIOs not correctly exported via sysfs on ATSAMA5D2

[Romain Izard]

Hello,

While experimenting with a new chip, I connected it on the SDIO
interface on my board based on a SAMA5D2 SoC. For a first step, I need
to drive the pins on the SDIO bus at a given level to program this new
chip. To do so, I tried to control the GPIO lines manually by unbinding
the SDHCI controller, and using /sys/class/gpio/export to control the
pins, with the following code:

echo a0000000.sdio-host > /sys/bus/platform/drivers/sdhci-at91/unbind
echo 4 > /sys/class/gpio/export
echo low > /sys/class/gpio/PA4/direction

Unfortunately, the state of the pin does not change and it remains
driven to 1. I checked the configuration register with devmem2, and it
appeared that the selected function remains the SDIO function even after
calling export.

The issue does not appear when I use a GPIO in a driver with an explicit
pinctrl configuration in the device tree, which explains why I did not
see it until now.

The kernel version used is Linux 5.4.22

Is this a user error from my part, or is there something missing in the
AT91 PIO4 pinctrl driver ?

Best regards,
-- 
Romain Izard

GPIOs not correctly exported via sysfs on ATSAMA5D2

[Romain Izard]

Hello,

While experimenting with a new chip, I connected it on the SDIO
interface on my board based on a SAMA5D2 SoC. For a first step, I need
to drive the pins on the SDIO bus at a given level to program this new
chip. To do so, I tried to control the GPIO lines manually by unbinding
the SDHCI controller, and using /sys/class/gpio/export to control the
pins, with the following code:

echo a0000000.sdio-host > /sys/bus/platform/drivers/sdhci-at91/unbind
echo 4 > /sys/class/gpio/export
echo low > /sys/class/gpio/PA4/direction

Unfortunately, the state of the pin does not change and it remains
driven to 1. I checked the configuration register with devmem2, and it
appeared that the selected function remains the SDIO function even after
calling export.

The issue does not appear when I use a GPIO in a driver with an explicit
pinctrl configuration in the device tree, which explains why I did not
see it until now.

The kernel version used is Linux 5.4.22

Is this a user error from my part, or is there something missing in the
AT91 PIO4 pinctrl driver ?

Best regards,
-- 
Romain Izard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Ludovic Desroches]

Hello Romain,

On Fri, Feb 28, 2020 at 11:58:21AM +0100, Romain Izard wrote:
> 
> Hello,
> 
> While experimenting with a new chip, I connected it on the SDIO
> interface on my board based on a SAMA5D2 SoC. For a first step, I need
> to drive the pins on the SDIO bus at a given level to program this new
> chip. To do so, I tried to control the GPIO lines manually by unbinding
> the SDHCI controller, and using /sys/class/gpio/export to control the
> pins, with the following code:
> 
> echo a0000000.sdio-host > /sys/bus/platform/drivers/sdhci-at91/unbind
> echo 4 > /sys/class/gpio/export
> echo low > /sys/class/gpio/PA4/direction
> 
> Unfortunately, the state of the pin does not change and it remains
> driven to 1. I checked the configuration register with devmem2, and it
> appeared that the selected function remains the SDIO function even after
> calling export.
> 
> The issue does not appear when I use a GPIO in a driver with an explicit
> pinctrl configuration in the device tree, which explains why I did not
> see it until now.
> 
> The kernel version used is Linux 5.4.22
> 
> Is this a user error from my part, or is there something missing in the
> AT91 PIO4 pinctrl driver ?

This is a known issue.

The AT91 PIO4 pinctrl driver doesn't implement gpio_request_enable()
contrary to the AT91 PIO pinctrl driver. If we implement it, then you
would be able to change the pin muxing and configuration from the sysfs.
The issue is nothing prevent you do this and so to possibly break a
device.

There is the strict pinmux_ops property which prevents from this
situation. The side effect is that we must not declare a pinmux/conf for
a GPIO so all the DT files have to been updated. That's not a big deal,
the problem is, at that time, the GPIO subsystem didn't allow to set the
bias for instance. It may have changed but not sure it covers all the
possible configurations we have from the pinmuxing subsystem.

Regards

Ludovic

> 
> Best regards,
> --
> Romain Izard

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Ludovic Desroches]

Hello Romain,

On Fri, Feb 28, 2020 at 11:58:21AM +0100, Romain Izard wrote:
> 
> Hello,
> 
> While experimenting with a new chip, I connected it on the SDIO
> interface on my board based on a SAMA5D2 SoC. For a first step, I need
> to drive the pins on the SDIO bus at a given level to program this new
> chip. To do so, I tried to control the GPIO lines manually by unbinding
> the SDHCI controller, and using /sys/class/gpio/export to control the
> pins, with the following code:
> 
> echo a0000000.sdio-host > /sys/bus/platform/drivers/sdhci-at91/unbind
> echo 4 > /sys/class/gpio/export
> echo low > /sys/class/gpio/PA4/direction
> 
> Unfortunately, the state of the pin does not change and it remains
> driven to 1. I checked the configuration register with devmem2, and it
> appeared that the selected function remains the SDIO function even after
> calling export.
> 
> The issue does not appear when I use a GPIO in a driver with an explicit
> pinctrl configuration in the device tree, which explains why I did not
> see it until now.
> 
> The kernel version used is Linux 5.4.22
> 
> Is this a user error from my part, or is there something missing in the
> AT91 PIO4 pinctrl driver ?

This is a known issue.

The AT91 PIO4 pinctrl driver doesn't implement gpio_request_enable()
contrary to the AT91 PIO pinctrl driver. If we implement it, then you
would be able to change the pin muxing and configuration from the sysfs.
The issue is nothing prevent you do this and so to possibly break a
device.

There is the strict pinmux_ops property which prevents from this
situation. The side effect is that we must not declare a pinmux/conf for
a GPIO so all the DT files have to been updated. That's not a big deal,
the problem is, at that time, the GPIO subsystem didn't allow to set the
bias for instance. It may have changed but not sure it covers all the
possible configurations we have from the pinmuxing subsystem.

Regards

Ludovic

> 
> Best regards,
> --
> Romain Izard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Russell King - ARM Linux admin]

On Fri, Feb 28, 2020 at 01:39:10PM +0100, Ludovic Desroches wrote:
> Hello Romain,
> 
> On Fri, Feb 28, 2020 at 11:58:21AM +0100, Romain Izard wrote:
> > 
> > Hello,
> > 
> > While experimenting with a new chip, I connected it on the SDIO
> > interface on my board based on a SAMA5D2 SoC. For a first step, I need
> > to drive the pins on the SDIO bus at a given level to program this new
> > chip. To do so, I tried to control the GPIO lines manually by unbinding
> > the SDHCI controller, and using /sys/class/gpio/export to control the
> > pins, with the following code:
> > 
> > echo a0000000.sdio-host > /sys/bus/platform/drivers/sdhci-at91/unbind
> > echo 4 > /sys/class/gpio/export
> > echo low > /sys/class/gpio/PA4/direction
> > 
> > Unfortunately, the state of the pin does not change and it remains
> > driven to 1. I checked the configuration register with devmem2, and it
> > appeared that the selected function remains the SDIO function even after
> > calling export.
> > 
> > The issue does not appear when I use a GPIO in a driver with an explicit
> > pinctrl configuration in the device tree, which explains why I did not
> > see it until now.
> > 
> > The kernel version used is Linux 5.4.22
> > 
> > Is this a user error from my part, or is there something missing in the
> > AT91 PIO4 pinctrl driver ?
> 
> This is a known issue.
> 
> The AT91 PIO4 pinctrl driver doesn't implement gpio_request_enable()
> contrary to the AT91 PIO pinctrl driver. If we implement it, then you
> would be able to change the pin muxing and configuration from the sysfs.
> The issue is nothing prevent you do this and so to possibly break a
> device.
> 
> There is the strict pinmux_ops property which prevents from this
> situation. The side effect is that we must not declare a pinmux/conf for
> a GPIO so all the DT files have to been updated. That's not a big deal,
> the problem is, at that time, the GPIO subsystem didn't allow to set the
> bias for instance. It may have changed but not sure it covers all the
> possible configurations we have from the pinmuxing subsystem.

There is also the problem of I2C bus recovery, where an I2C driver
may wish to claim the GPIOs for the I2C bus, but keep the I2C bus
connected to the I2C controller except when performing recovery.

I tripped over problems with that on a Marvell SoC, someone else has
recently reported exactly the same problem as a regression compared
to previous kernels for another SoC in the last day.

The assumption that if a GPIO is claimed, we want the pin to be in
GPIO mode is not universally true, a point that has been missed and
is now coming back to bite.  From what Linus said when we discussed
it, it's very difficult to fix in the GPIO/pinctrl layers.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Russell King - ARM Linux admin]

On Fri, Feb 28, 2020 at 01:39:10PM +0100, Ludovic Desroches wrote:
> Hello Romain,
> 
> On Fri, Feb 28, 2020 at 11:58:21AM +0100, Romain Izard wrote:
> > 
> > Hello,
> > 
> > While experimenting with a new chip, I connected it on the SDIO
> > interface on my board based on a SAMA5D2 SoC. For a first step, I need
> > to drive the pins on the SDIO bus at a given level to program this new
> > chip. To do so, I tried to control the GPIO lines manually by unbinding
> > the SDHCI controller, and using /sys/class/gpio/export to control the
> > pins, with the following code:
> > 
> > echo a0000000.sdio-host > /sys/bus/platform/drivers/sdhci-at91/unbind
> > echo 4 > /sys/class/gpio/export
> > echo low > /sys/class/gpio/PA4/direction
> > 
> > Unfortunately, the state of the pin does not change and it remains
> > driven to 1. I checked the configuration register with devmem2, and it
> > appeared that the selected function remains the SDIO function even after
> > calling export.
> > 
> > The issue does not appear when I use a GPIO in a driver with an explicit
> > pinctrl configuration in the device tree, which explains why I did not
> > see it until now.
> > 
> > The kernel version used is Linux 5.4.22
> > 
> > Is this a user error from my part, or is there something missing in the
> > AT91 PIO4 pinctrl driver ?
> 
> This is a known issue.
> 
> The AT91 PIO4 pinctrl driver doesn't implement gpio_request_enable()
> contrary to the AT91 PIO pinctrl driver. If we implement it, then you
> would be able to change the pin muxing and configuration from the sysfs.
> The issue is nothing prevent you do this and so to possibly break a
> device.
> 
> There is the strict pinmux_ops property which prevents from this
> situation. The side effect is that we must not declare a pinmux/conf for
> a GPIO so all the DT files have to been updated. That's not a big deal,
> the problem is, at that time, the GPIO subsystem didn't allow to set the
> bias for instance. It may have changed but not sure it covers all the
> possible configurations we have from the pinmuxing subsystem.

There is also the problem of I2C bus recovery, where an I2C driver
may wish to claim the GPIOs for the I2C bus, but keep the I2C bus
connected to the I2C controller except when performing recovery.

I tripped over problems with that on a Marvell SoC, someone else has
recently reported exactly the same problem as a regression compared
to previous kernels for another SoC in the last day.

The assumption that if a GPIO is claimed, we want the pin to be in
GPIO mode is not universally true, a point that has been missed and
is now coming back to bite.  From what Linus said when we discussed
it, it's very difficult to fix in the GPIO/pinctrl layers.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Linus Walleij]

On Fri, Feb 28, 2020 at 1:39 PM Ludovic Desroches
<ludovic.desroches@microchip.com> wrote:

> The AT91 PIO4 pinctrl driver doesn't implement gpio_request_enable()
> contrary to the AT91 PIO pinctrl driver. If we implement it, then you
> would be able to change the pin muxing and configuration from the sysfs.
> The issue is nothing prevent you do this and so to possibly break a
> device.
>
> There is the strict pinmux_ops property which prevents from this
> situation. The side effect is that we must not declare a pinmux/conf for
> a GPIO so all the DT files have to been updated. That's not a big deal,
> the problem is, at that time, the GPIO subsystem didn't allow to set the
> bias for instance. It may have changed but not sure it covers all the
> possible configurations we have from the pinmuxing subsystem.

Yes and Russell also points very clearly to the root of the problem:

- Some pin multiplexers are non-strict meaning a line can be used
  for say GPIO and something else (such as SDIO) at the same time.
  Usually this is an observation from electronics, such that the GPIO
  input register will always report the logical state on the line whether
  it is in "gpio mode" or something else, making it possible to snoop
  the line, or, as in the I2C case, it is perfectly fine to, without glitches,
  pull the line out of other use cases and into the GPIO realm and do
  GPIO things such as bus recovery actively driving the line despite
  it being connected in the mux to SDIO or I2C at the same time.

- At the same time some system designers and driver authors are
  driven by the ambition to create a system that is simple, has no
  exceptions and has a clear separation of concerns, making it
  impossible for the user - even a kernel developer or real savvy
  hacker playing around with the deprecated sysfs ABI - to shoot
  themselves in the foot. These are ambitious and admirable people
  that want to protect their users and make it easy to do the right
  thing. So they sort everything out and flag their pin multiplexers
  as "strict", allowing only one use case at the time: this device is
  either I2C or SDIO.

Personally I am not flagging any of my drivers strict because I'm
just a pragmatist and don't use the system architect type of reasoning
much in what I do. But it is admittedly a matter of taste. The flag
"strict" tells you something about the driver author's values and
ambition.

If we get to cases where the user is getting hurt by the strictness
rather than enjoying the protection it provides, such as being unable
to do I2C recovery, it may be reasonable to drop the strict setting
and just fail the ambition to separate the concerns, as clearly the
system architecture wasn't perfect in the first place, and trying to
be strict in the mux is going to be a bit like polishing dirt.

This case however looks a bit like tinkering, bring-up and hacking,
and as such it might be expected that the user hack around a bit
and recompile the kernel and device tree etc?

Yours,
Linus Walleij

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Linus Walleij]

On Fri, Feb 28, 2020 at 1:39 PM Ludovic Desroches
<ludovic.desroches@microchip.com> wrote:

> The AT91 PIO4 pinctrl driver doesn't implement gpio_request_enable()
> contrary to the AT91 PIO pinctrl driver. If we implement it, then you
> would be able to change the pin muxing and configuration from the sysfs.
> The issue is nothing prevent you do this and so to possibly break a
> device.
>
> There is the strict pinmux_ops property which prevents from this
> situation. The side effect is that we must not declare a pinmux/conf for
> a GPIO so all the DT files have to been updated. That's not a big deal,
> the problem is, at that time, the GPIO subsystem didn't allow to set the
> bias for instance. It may have changed but not sure it covers all the
> possible configurations we have from the pinmuxing subsystem.

Yes and Russell also points very clearly to the root of the problem:

- Some pin multiplexers are non-strict meaning a line can be used
  for say GPIO and something else (such as SDIO) at the same time.
  Usually this is an observation from electronics, such that the GPIO
  input register will always report the logical state on the line whether
  it is in "gpio mode" or something else, making it possible to snoop
  the line, or, as in the I2C case, it is perfectly fine to, without glitches,
  pull the line out of other use cases and into the GPIO realm and do
  GPIO things such as bus recovery actively driving the line despite
  it being connected in the mux to SDIO or I2C at the same time.

- At the same time some system designers and driver authors are
  driven by the ambition to create a system that is simple, has no
  exceptions and has a clear separation of concerns, making it
  impossible for the user - even a kernel developer or real savvy
  hacker playing around with the deprecated sysfs ABI - to shoot
  themselves in the foot. These are ambitious and admirable people
  that want to protect their users and make it easy to do the right
  thing. So they sort everything out and flag their pin multiplexers
  as "strict", allowing only one use case at the time: this device is
  either I2C or SDIO.

Personally I am not flagging any of my drivers strict because I'm
just a pragmatist and don't use the system architect type of reasoning
much in what I do. But it is admittedly a matter of taste. The flag
"strict" tells you something about the driver author's values and
ambition.

If we get to cases where the user is getting hurt by the strictness
rather than enjoying the protection it provides, such as being unable
to do I2C recovery, it may be reasonable to drop the strict setting
and just fail the ambition to separate the concerns, as clearly the
system architecture wasn't perfect in the first place, and trying to
be strict in the mux is going to be a bit like polishing dirt.

This case however looks a bit like tinkering, bring-up and hacking,
and as such it might be expected that the user hack around a bit
and recompile the kernel and device tree etc?

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Uwe Kleine-König]

On Fri, Feb 28, 2020 at 11:58:21AM +0100, Romain Izard wrote:
> Hello,
> 
> While experimenting with a new chip, I connected it on the SDIO
> interface on my board based on a SAMA5D2 SoC. For a first step, I need
> to drive the pins on the SDIO bus at a given level to program this new
> chip. To do so, I tried to control the GPIO lines manually by unbinding
> the SDHCI controller, and using /sys/class/gpio/export to control the
> pins, with the following code:
> 
> echo a0000000.sdio-host > /sys/bus/platform/drivers/sdhci-at91/unbind
> echo 4 > /sys/class/gpio/export
> echo low > /sys/class/gpio/PA4/direction
> 
> Unfortunately, the state of the pin does not change and it remains
> driven to 1. I checked the configuration register with devmem2, and it
> appeared that the selected function remains the SDIO function even after
> calling export.
> 
> The issue does not appear when I use a GPIO in a driver with an explicit
> pinctrl configuration in the device tree, which explains why I did not
> see it until now.
> 
> The kernel version used is Linux 5.4.22
> 
> Is this a user error from my part, or is there something missing in the
> AT91 PIO4 pinctrl driver ?

IMHO this is all as expected. There are gpio controllers that
automatically mux the matching pin, but you must not expect that.

My personal opinion on this is, that the downside of this automatic is
worse than its benefits:

 - It's not universal, as there are SoCs that don't have a single pin
   for a given GPIO, so you cannot reliably implement it for all
   controllers.

 - Sometimes it is useful to make use of a GPIO and a dedicated function
   on the same pin in a driver (e.g. an i2c driver might need to switch
   to gpio to do a bus recovery). The automatic pinmuxing then has
   strange side effect because you have to remux the pins after
   requesting the GPIOs even if you didn't drive the pins as GPIO and
   there is a short time where the pin function isn't the dedicated
   one.

So to me this is too magic and less explicit and so bad.

Just my 0.02€
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Uwe Kleine-König]

On Fri, Feb 28, 2020 at 11:58:21AM +0100, Romain Izard wrote:
> Hello,
> 
> While experimenting with a new chip, I connected it on the SDIO
> interface on my board based on a SAMA5D2 SoC. For a first step, I need
> to drive the pins on the SDIO bus at a given level to program this new
> chip. To do so, I tried to control the GPIO lines manually by unbinding
> the SDHCI controller, and using /sys/class/gpio/export to control the
> pins, with the following code:
> 
> echo a0000000.sdio-host > /sys/bus/platform/drivers/sdhci-at91/unbind
> echo 4 > /sys/class/gpio/export
> echo low > /sys/class/gpio/PA4/direction
> 
> Unfortunately, the state of the pin does not change and it remains
> driven to 1. I checked the configuration register with devmem2, and it
> appeared that the selected function remains the SDIO function even after
> calling export.
> 
> The issue does not appear when I use a GPIO in a driver with an explicit
> pinctrl configuration in the device tree, which explains why I did not
> see it until now.
> 
> The kernel version used is Linux 5.4.22
> 
> Is this a user error from my part, or is there something missing in the
> AT91 PIO4 pinctrl driver ?

IMHO this is all as expected. There are gpio controllers that
automatically mux the matching pin, but you must not expect that.

My personal opinion on this is, that the downside of this automatic is
worse than its benefits:

 - It's not universal, as there are SoCs that don't have a single pin
   for a given GPIO, so you cannot reliably implement it for all
   controllers.

 - Sometimes it is useful to make use of a GPIO and a dedicated function
   on the same pin in a driver (e.g. an i2c driver might need to switch
   to gpio to do a bus recovery). The automatic pinmuxing then has
   strange side effect because you have to remux the pins after
   requesting the GPIOs even if you didn't drive the pins as GPIO and
   there is a short time where the pin function isn't the dedicated
   one.

So to me this is too magic and less explicit and so bad.

Just my 0.02€
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Uwe Kleine-König]

On Fri, Feb 28, 2020 at 02:36:11PM +0100, Linus Walleij wrote:
> On Fri, Feb 28, 2020 at 1:39 PM Ludovic Desroches
> <ludovic.desroches@microchip.com> wrote:
> 
> > The AT91 PIO4 pinctrl driver doesn't implement gpio_request_enable()
> > contrary to the AT91 PIO pinctrl driver. If we implement it, then you
> > would be able to change the pin muxing and configuration from the sysfs.
> > The issue is nothing prevent you do this and so to possibly break a
> > device.
> >
> > There is the strict pinmux_ops property which prevents from this
> > situation. The side effect is that we must not declare a pinmux/conf for
> > a GPIO so all the DT files have to been updated. That's not a big deal,
> > the problem is, at that time, the GPIO subsystem didn't allow to set the
> > bias for instance. It may have changed but not sure it covers all the
> > possible configurations we have from the pinmuxing subsystem.
> 
> Yes and Russell also points very clearly to the root of the problem:
> 
> - Some pin multiplexers are non-strict meaning a line can be used
>   for say GPIO and something else (such as SDIO) at the same time.
>   Usually this is an observation from electronics, such that the GPIO
>   input register will always report the logical state on the line whether
>   it is in "gpio mode" or something else, making it possible to snoop
>   the line, or, as in the I2C case, it is perfectly fine to, without glitches,
>   pull the line out of other use cases and into the GPIO realm and do
>   GPIO things such as bus recovery actively driving the line despite
>   it being connected in the mux to SDIO or I2C at the same time.
> 
> - At the same time some system designers and driver authors are
>   driven by the ambition to create a system that is simple, has no
>   exceptions and has a clear separation of concerns, making it
>   impossible for the user - even a kernel developer or real savvy
>   hacker playing around with the deprecated sysfs ABI - to shoot
>   themselves in the foot. These are ambitious and admirable people
>   that want to protect their users and make it easy to do the right
>   thing. So they sort everything out and flag their pin multiplexers
>   as "strict", allowing only one use case at the time: this device is
>   either I2C or SDIO.
> 
> Personally I am not flagging any of my drivers strict because I'm
> just a pragmatist and don't use the system architect type of reasoning
> much in what I do. But it is admittedly a matter of taste. The flag
> "strict" tells you something about the driver author's values and
> ambition.

This points out another problem: Drivers behave differently in an area
where there is no technical need. So people are surprised which is a bad
thing. For me giving a gpio/pinctrl driver maintainer the freedom to
label the device as "strict" or not is similar to putting policy in the
kernel which is frowed for.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Uwe Kleine-König]

On Fri, Feb 28, 2020 at 02:36:11PM +0100, Linus Walleij wrote:
> On Fri, Feb 28, 2020 at 1:39 PM Ludovic Desroches
> <ludovic.desroches@microchip.com> wrote:
> 
> > The AT91 PIO4 pinctrl driver doesn't implement gpio_request_enable()
> > contrary to the AT91 PIO pinctrl driver. If we implement it, then you
> > would be able to change the pin muxing and configuration from the sysfs.
> > The issue is nothing prevent you do this and so to possibly break a
> > device.
> >
> > There is the strict pinmux_ops property which prevents from this
> > situation. The side effect is that we must not declare a pinmux/conf for
> > a GPIO so all the DT files have to been updated. That's not a big deal,
> > the problem is, at that time, the GPIO subsystem didn't allow to set the
> > bias for instance. It may have changed but not sure it covers all the
> > possible configurations we have from the pinmuxing subsystem.
> 
> Yes and Russell also points very clearly to the root of the problem:
> 
> - Some pin multiplexers are non-strict meaning a line can be used
>   for say GPIO and something else (such as SDIO) at the same time.
>   Usually this is an observation from electronics, such that the GPIO
>   input register will always report the logical state on the line whether
>   it is in "gpio mode" or something else, making it possible to snoop
>   the line, or, as in the I2C case, it is perfectly fine to, without glitches,
>   pull the line out of other use cases and into the GPIO realm and do
>   GPIO things such as bus recovery actively driving the line despite
>   it being connected in the mux to SDIO or I2C at the same time.
> 
> - At the same time some system designers and driver authors are
>   driven by the ambition to create a system that is simple, has no
>   exceptions and has a clear separation of concerns, making it
>   impossible for the user - even a kernel developer or real savvy
>   hacker playing around with the deprecated sysfs ABI - to shoot
>   themselves in the foot. These are ambitious and admirable people
>   that want to protect their users and make it easy to do the right
>   thing. So they sort everything out and flag their pin multiplexers
>   as "strict", allowing only one use case at the time: this device is
>   either I2C or SDIO.
> 
> Personally I am not flagging any of my drivers strict because I'm
> just a pragmatist and don't use the system architect type of reasoning
> much in what I do. But it is admittedly a matter of taste. The flag
> "strict" tells you something about the driver author's values and
> ambition.

This points out another problem: Drivers behave differently in an area
where there is no technical need. So people are surprised which is a bad
thing. For me giving a gpio/pinctrl driver maintainer the freedom to
label the device as "strict" or not is similar to putting policy in the
kernel which is frowed for.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Russell King - ARM Linux admin]

On Mon, Mar 02, 2020 at 09:39:59AM +0100, Uwe Kleine-König wrote:
> On Fri, Feb 28, 2020 at 11:58:21AM +0100, Romain Izard wrote:
> > Hello,
> > 
> > While experimenting with a new chip, I connected it on the SDIO
> > interface on my board based on a SAMA5D2 SoC. For a first step, I need
> > to drive the pins on the SDIO bus at a given level to program this new
> > chip. To do so, I tried to control the GPIO lines manually by unbinding
> > the SDHCI controller, and using /sys/class/gpio/export to control the
> > pins, with the following code:
> > 
> > echo a0000000.sdio-host > /sys/bus/platform/drivers/sdhci-at91/unbind
> > echo 4 > /sys/class/gpio/export
> > echo low > /sys/class/gpio/PA4/direction
> > 
> > Unfortunately, the state of the pin does not change and it remains
> > driven to 1. I checked the configuration register with devmem2, and it
> > appeared that the selected function remains the SDIO function even after
> > calling export.
> > 
> > The issue does not appear when I use a GPIO in a driver with an explicit
> > pinctrl configuration in the device tree, which explains why I did not
> > see it until now.
> > 
> > The kernel version used is Linux 5.4.22
> > 
> > Is this a user error from my part, or is there something missing in the
> > AT91 PIO4 pinctrl driver ?
> 
> IMHO this is all as expected. There are gpio controllers that
> automatically mux the matching pin, but you must not expect that.
> 
> My personal opinion on this is, that the downside of this automatic is
> worse than its benefits:
> 
>  - It's not universal, as there are SoCs that don't have a single pin
>    for a given GPIO, so you cannot reliably implement it for all
>    controllers.
> 
>  - Sometimes it is useful to make use of a GPIO and a dedicated function
>    on the same pin in a driver (e.g. an i2c driver might need to switch
>    to gpio to do a bus recovery). The automatic pinmuxing then has
>    strange side effect because you have to remux the pins after
>    requesting the GPIOs even if you didn't drive the pins as GPIO and
>    there is a short time where the pin function isn't the dedicated
>    one.

It's worse than that for the i2c driver.  The pins are muxed to the i2c
function when the driver binds.  When the i2c driver claims the GPIOs
corresponding with those pins, they get switched to GPIO mode behind
the back of pinctrl.  You then have to _explicitly_ switch pinctrl to
GPIO mode and back to I2C mode to get them back to I2C mode.

Merely requesting I2C mode after claiming the GPIOs doesn't do anything
- the pinctrl layer thinks they're still in I2C mode, and ignores the
call.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Russell King - ARM Linux admin]

On Mon, Mar 02, 2020 at 09:39:59AM +0100, Uwe Kleine-König wrote:
> On Fri, Feb 28, 2020 at 11:58:21AM +0100, Romain Izard wrote:
> > Hello,
> > 
> > While experimenting with a new chip, I connected it on the SDIO
> > interface on my board based on a SAMA5D2 SoC. For a first step, I need
> > to drive the pins on the SDIO bus at a given level to program this new
> > chip. To do so, I tried to control the GPIO lines manually by unbinding
> > the SDHCI controller, and using /sys/class/gpio/export to control the
> > pins, with the following code:
> > 
> > echo a0000000.sdio-host > /sys/bus/platform/drivers/sdhci-at91/unbind
> > echo 4 > /sys/class/gpio/export
> > echo low > /sys/class/gpio/PA4/direction
> > 
> > Unfortunately, the state of the pin does not change and it remains
> > driven to 1. I checked the configuration register with devmem2, and it
> > appeared that the selected function remains the SDIO function even after
> > calling export.
> > 
> > The issue does not appear when I use a GPIO in a driver with an explicit
> > pinctrl configuration in the device tree, which explains why I did not
> > see it until now.
> > 
> > The kernel version used is Linux 5.4.22
> > 
> > Is this a user error from my part, or is there something missing in the
> > AT91 PIO4 pinctrl driver ?
> 
> IMHO this is all as expected. There are gpio controllers that
> automatically mux the matching pin, but you must not expect that.
> 
> My personal opinion on this is, that the downside of this automatic is
> worse than its benefits:
> 
>  - It's not universal, as there are SoCs that don't have a single pin
>    for a given GPIO, so you cannot reliably implement it for all
>    controllers.
> 
>  - Sometimes it is useful to make use of a GPIO and a dedicated function
>    on the same pin in a driver (e.g. an i2c driver might need to switch
>    to gpio to do a bus recovery). The automatic pinmuxing then has
>    strange side effect because you have to remux the pins after
>    requesting the GPIOs even if you didn't drive the pins as GPIO and
>    there is a short time where the pin function isn't the dedicated
>    one.

It's worse than that for the i2c driver.  The pins are muxed to the i2c
function when the driver binds.  When the i2c driver claims the GPIOs
corresponding with those pins, they get switched to GPIO mode behind
the back of pinctrl.  You then have to _explicitly_ switch pinctrl to
GPIO mode and back to I2C mode to get them back to I2C mode.

Merely requesting I2C mode after claiming the GPIOs doesn't do anything
- the pinctrl layer thinks they're still in I2C mode, and ignores the
call.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Linus Walleij]

On Mon, Mar 2, 2020 at 10:36 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
> On Mon, Mar 02, 2020 at 09:39:59AM +0100, Uwe Kleine-König wrote:

> >  - Sometimes it is useful to make use of a GPIO and a dedicated function
> >    on the same pin in a driver (e.g. an i2c driver might need to switch
> >    to gpio to do a bus recovery). The automatic pinmuxing then has
> >    strange side effect because you have to remux the pins after
> >    requesting the GPIOs even if you didn't drive the pins as GPIO and
> >    there is a short time where the pin function isn't the dedicated
> >    one.
>
> It's worse than that for the i2c driver.  The pins are muxed to the i2c
> function when the driver binds.  When the i2c driver claims the GPIOs
> corresponding with those pins, they get switched to GPIO mode behind
> the back of pinctrl.  You then have to _explicitly_ switch pinctrl to
> GPIO mode and back to I2C mode to get them back to I2C mode.

That's especially annoying. I would consider adding a specific
consumer flag for GPIOs used this way, in additon to
GPIOD_ASIS, something like GPIOD_ASIS_NOMUX
(thinking of better names).

Since the calling site knows about this usecase we can
open code the semantics for this specifically.

Yours,
Linus Walleij

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Linus Walleij]

On Mon, Mar 2, 2020 at 10:36 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
> On Mon, Mar 02, 2020 at 09:39:59AM +0100, Uwe Kleine-König wrote:

> >  - Sometimes it is useful to make use of a GPIO and a dedicated function
> >    on the same pin in a driver (e.g. an i2c driver might need to switch
> >    to gpio to do a bus recovery). The automatic pinmuxing then has
> >    strange side effect because you have to remux the pins after
> >    requesting the GPIOs even if you didn't drive the pins as GPIO and
> >    there is a short time where the pin function isn't the dedicated
> >    one.
>
> It's worse than that for the i2c driver.  The pins are muxed to the i2c
> function when the driver binds.  When the i2c driver claims the GPIOs
> corresponding with those pins, they get switched to GPIO mode behind
> the back of pinctrl.  You then have to _explicitly_ switch pinctrl to
> GPIO mode and back to I2C mode to get them back to I2C mode.

That's especially annoying. I would consider adding a specific
consumer flag for GPIOs used this way, in additon to
GPIOD_ASIS, something like GPIOD_ASIS_NOMUX
(thinking of better names).

Since the calling site knows about this usecase we can
open code the semantics for this specifically.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Russell King - ARM Linux admin]

On Mon, Mar 02, 2020 at 05:34:19PM +0100, Linus Walleij wrote:
> On Mon, Mar 2, 2020 at 10:36 AM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> > On Mon, Mar 02, 2020 at 09:39:59AM +0100, Uwe Kleine-König wrote:
> 
> > >  - Sometimes it is useful to make use of a GPIO and a dedicated function
> > >    on the same pin in a driver (e.g. an i2c driver might need to switch
> > >    to gpio to do a bus recovery). The automatic pinmuxing then has
> > >    strange side effect because you have to remux the pins after
> > >    requesting the GPIOs even if you didn't drive the pins as GPIO and
> > >    there is a short time where the pin function isn't the dedicated
> > >    one.
> >
> > It's worse than that for the i2c driver.  The pins are muxed to the i2c
> > function when the driver binds.  When the i2c driver claims the GPIOs
> > corresponding with those pins, they get switched to GPIO mode behind
> > the back of pinctrl.  You then have to _explicitly_ switch pinctrl to
> > GPIO mode and back to I2C mode to get them back to I2C mode.
> 
> That's especially annoying. I would consider adding a specific
> consumer flag for GPIOs used this way, in additon to
> GPIOD_ASIS, something like GPIOD_ASIS_NOMUX
> (thinking of better names).

It's very annoying, and I believe something I did point out in my email
about it when I discovered it towards the end of last year.

Having a way to avoid the muxing would be a very good idea, as there
are cases where we really should not be taking the I2C pins away from
the controller during driver initialisation.  In the case of i2c-pxa,
when the pins are taken away, the controller sees the disconnected SCL
and SDA lines go low, and it can assume that the bus is busy as a
result, or worse see it as a START condition if SDA goes low while SCL
is high.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Russell King - ARM Linux admin]

On Mon, Mar 02, 2020 at 05:34:19PM +0100, Linus Walleij wrote:
> On Mon, Mar 2, 2020 at 10:36 AM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> > On Mon, Mar 02, 2020 at 09:39:59AM +0100, Uwe Kleine-König wrote:
> 
> > >  - Sometimes it is useful to make use of a GPIO and a dedicated function
> > >    on the same pin in a driver (e.g. an i2c driver might need to switch
> > >    to gpio to do a bus recovery). The automatic pinmuxing then has
> > >    strange side effect because you have to remux the pins after
> > >    requesting the GPIOs even if you didn't drive the pins as GPIO and
> > >    there is a short time where the pin function isn't the dedicated
> > >    one.
> >
> > It's worse than that for the i2c driver.  The pins are muxed to the i2c
> > function when the driver binds.  When the i2c driver claims the GPIOs
> > corresponding with those pins, they get switched to GPIO mode behind
> > the back of pinctrl.  You then have to _explicitly_ switch pinctrl to
> > GPIO mode and back to I2C mode to get them back to I2C mode.
> 
> That's especially annoying. I would consider adding a specific
> consumer flag for GPIOs used this way, in additon to
> GPIOD_ASIS, something like GPIOD_ASIS_NOMUX
> (thinking of better names).

It's very annoying, and I believe something I did point out in my email
about it when I discovered it towards the end of last year.

Having a way to avoid the muxing would be a very good idea, as there
are cases where we really should not be taking the I2C pins away from
the controller during driver initialisation.  In the case of i2c-pxa,
when the pins are taken away, the controller sees the disconnected SCL
and SDA lines go low, and it can assume that the bus is busy as a
result, or worse see it as a START condition if SDA goes low while SCL
is high.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Uwe Kleine-König]

Hello,

On Mon, Mar 02, 2020 at 05:34:19PM +0100, Linus Walleij wrote:
> On Mon, Mar 2, 2020 at 10:36 AM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> > On Mon, Mar 02, 2020 at 09:39:59AM +0100, Uwe Kleine-König wrote:
> 
> > >  - Sometimes it is useful to make use of a GPIO and a dedicated function
> > >    on the same pin in a driver (e.g. an i2c driver might need to switch
> > >    to gpio to do a bus recovery). The automatic pinmuxing then has
> > >    strange side effect because you have to remux the pins after
> > >    requesting the GPIOs even if you didn't drive the pins as GPIO and
> > >    there is a short time where the pin function isn't the dedicated
> > >    one.
> >
> > It's worse than that for the i2c driver.  The pins are muxed to the i2c
> > function when the driver binds.  When the i2c driver claims the GPIOs
> > corresponding with those pins, they get switched to GPIO mode behind
> > the back of pinctrl.  You then have to _explicitly_ switch pinctrl to
> > GPIO mode and back to I2C mode to get them back to I2C mode.
> 
> That's especially annoying. I would consider adding a specific
> consumer flag for GPIOs used this way, in additon to
> GPIOD_ASIS, something like GPIOD_ASIS_NOMUX
> (thinking of better names).

This is not only relevant for GPIOD_ASIS. GPIOs for recovery are
registered (in the case of i2c-imx) with:

        rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
	rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN);

so we'd need a _NOMUX variant for each gpiod_flags value.

Also if a board makes use of i2c, the corresponding pins shouldn't be
muxed to GPIO if userspace requests the GPIO via gpioctl or sysfs.
(IMHO i2c isn't special here, this should apply to all used pins,
shouldn't it?)

> Since the calling site knows about this usecase we can
> open code the semantics for this specifically.

Since the calling site doesn't know if the pin is used otherwise, it
should default to _NOMUX?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Uwe Kleine-König]

Hello,

On Mon, Mar 02, 2020 at 05:34:19PM +0100, Linus Walleij wrote:
> On Mon, Mar 2, 2020 at 10:36 AM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> > On Mon, Mar 02, 2020 at 09:39:59AM +0100, Uwe Kleine-König wrote:
> 
> > >  - Sometimes it is useful to make use of a GPIO and a dedicated function
> > >    on the same pin in a driver (e.g. an i2c driver might need to switch
> > >    to gpio to do a bus recovery). The automatic pinmuxing then has
> > >    strange side effect because you have to remux the pins after
> > >    requesting the GPIOs even if you didn't drive the pins as GPIO and
> > >    there is a short time where the pin function isn't the dedicated
> > >    one.
> >
> > It's worse than that for the i2c driver.  The pins are muxed to the i2c
> > function when the driver binds.  When the i2c driver claims the GPIOs
> > corresponding with those pins, they get switched to GPIO mode behind
> > the back of pinctrl.  You then have to _explicitly_ switch pinctrl to
> > GPIO mode and back to I2C mode to get them back to I2C mode.
> 
> That's especially annoying. I would consider adding a specific
> consumer flag for GPIOs used this way, in additon to
> GPIOD_ASIS, something like GPIOD_ASIS_NOMUX
> (thinking of better names).

This is not only relevant for GPIOD_ASIS. GPIOs for recovery are
registered (in the case of i2c-imx) with:

        rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
	rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN);

so we'd need a _NOMUX variant for each gpiod_flags value.

Also if a board makes use of i2c, the corresponding pins shouldn't be
muxed to GPIO if userspace requests the GPIO via gpioctl or sysfs.
(IMHO i2c isn't special here, this should apply to all used pins,
shouldn't it?)

> Since the calling site knows about this usecase we can
> open code the semantics for this specifically.

Since the calling site doesn't know if the pin is used otherwise, it
should default to _NOMUX?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Linus Walleij]

On Mon, Mar 2, 2020 at 10:30 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Mon, Mar 02, 2020 at 05:34:19PM +0100, Linus Walleij wrote:

> > That's especially annoying. I would consider adding a specific
> > consumer flag for GPIOs used this way, in additon to
> > GPIOD_ASIS, something like GPIOD_ASIS_NOMUX
> > (thinking of better names).
>
> This is not only relevant for GPIOD_ASIS. GPIOs for recovery are
> registered (in the case of i2c-imx) with:
>
>         rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
>         rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN);
>
> so we'd need a _NOMUX variant for each gpiod_flags value.

Oh we don't need a special variant for everything. In the end
it is just a bitfield, sorry if I was unclear. :(

#define GPIOD_FLAGS_BIT_NOMUX    BIT(5)

rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN |
GPIOD_FLAGS_BIT_NOMUX);

or
GPIOD_OUT_HIGH_OPEN_DRAIN | GPIOD_FLAGS_BIT_NOMUX
is fine.

It is just convenient helpers after all.

> Also if a board makes use of i2c, the corresponding pins shouldn't be
> muxed to GPIO if userspace requests the GPIO via gpioctl or sysfs.
> (IMHO i2c isn't special here, this should apply to all used pins,
> shouldn't it?)

This is another reason why people tag their pin controllers
as strict, and in that case this works.

For those that are not strict, userspace can intercept
the same pins and wreac havoc. Using GPIO from userspace
is indeed in many cases a good gun to shoot oneself in
the foot with.

The solution is not perfect, improvements can be made.

> > Since the calling site knows about this usecase we can
> > open code the semantics for this specifically.
>
> Since the calling site doesn't know if the pin is used otherwise, it
> should default to _NOMUX?

Sorry I'm not following?

I was thinking that if the I2C driver knows it will be doing this
imperative I2C recovery by shaking the pins from the GPIO
side of things, it will request the pins with the flag
GPIOD_FLAGS_BIT_NOMUX and then the GPIO and
pin control subsystems knows: "aha they wanna use these
behind the back of pin control so let's not enforce stuff,
just give them a handle to the line and make sure nobody
else goes and use these lines".

Yours,
Linus Walleij

Re: GPIOs not correctly exported via sysfs on ATSAMA5D2

[Linus Walleij]

On Mon, Mar 2, 2020 at 10:30 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Mon, Mar 02, 2020 at 05:34:19PM +0100, Linus Walleij wrote:

> > That's especially annoying. I would consider adding a specific
> > consumer flag for GPIOs used this way, in additon to
> > GPIOD_ASIS, something like GPIOD_ASIS_NOMUX
> > (thinking of better names).
>
> This is not only relevant for GPIOD_ASIS. GPIOs for recovery are
> registered (in the case of i2c-imx) with:
>
>         rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
>         rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN);
>
> so we'd need a _NOMUX variant for each gpiod_flags value.

Oh we don't need a special variant for everything. In the end
it is just a bitfield, sorry if I was unclear. :(

#define GPIOD_FLAGS_BIT_NOMUX    BIT(5)

rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN |
GPIOD_FLAGS_BIT_NOMUX);

or
GPIOD_OUT_HIGH_OPEN_DRAIN | GPIOD_FLAGS_BIT_NOMUX
is fine.

It is just convenient helpers after all.

> Also if a board makes use of i2c, the corresponding pins shouldn't be
> muxed to GPIO if userspace requests the GPIO via gpioctl or sysfs.
> (IMHO i2c isn't special here, this should apply to all used pins,
> shouldn't it?)

This is another reason why people tag their pin controllers
as strict, and in that case this works.

For those that are not strict, userspace can intercept
the same pins and wreac havoc. Using GPIO from userspace
is indeed in many cases a good gun to shoot oneself in
the foot with.

The solution is not perfect, improvements can be made.

> > Since the calling site knows about this usecase we can
> > open code the semantics for this specifically.
>
> Since the calling site doesn't know if the pin is used otherwise, it
> should default to _NOMUX?

Sorry I'm not following?

I was thinking that if the I2C driver knows it will be doing this
imperative I2C recovery by shaking the pins from the GPIO
side of things, it will request the pins with the flag
GPIOD_FLAGS_BIT_NOMUX and then the GPIO and
pin control subsystems knows: "aha they wanna use these
behind the back of pin control so let's not enforce stuff,
just give them a handle to the line and make sure nobody
else goes and use these lines".

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel