[PATCH 0/4] rockchip: Add gpio request() ops and drop PCIe reset-gpios workaround

[Mark Kettenis]

> Date: Sat, 11 May 2024 20:47:40 +0200
> From: Jonas Karlman <jonas at kwiboo.se>

Hi Jonas & Alex,

> Hi Alex,
> 
> On 2024-05-11 19:44, Alex Bee wrote:
> > Hi Jonas,
> > 
> > Am 11.05.24 um 13:28 schrieb Jonas Karlman:
> >> This series add gpio request() and pinctrl gpio_request_enable() ops so
> >> that a gpio requested pin automatically use gpio pinmux and U-Boot
> >> behaves more similar to Linux kernel.
> > 
> > I'm not sure that's a good idea.
> > While linux does it the same way, we really shouldn't expect every 
> > software/os/ … which uses DT (now or in future) to implicitly switch the 
> > pin function when using a pin as gpio. So the real fix would probably be 
> > to add the the correct pinctrl settings to the upstream DT of those 
> > boards and sync it later on (not sure those if those SoCs already using 
> > OF_UPSTREAM) and leave the -u-boot.dtsi-"hack" alone for now.

I missed Alex's mail, but OpenBSD certainly is one of the OSes that
could break if the pinctrl settings get removed.  We currently have no
code to automatically mux the gpio pins on these Rockchip SoCs. I
suppose as long as U-Boot probes the PCIe bus, the pins will already
be muxed correctly and things will continue to work.  But I think
there are certain boot scenarios where this won't happen.

I've wondered in the past what the purpose of the gpio-ranges
properties was.  I never considered that they could be used to
automatically mux the pins for GPIO since the gpio-ranges mapping
provides no indication of what the correct pin settings arefor the
GPIO pins.

> 
> I fully agree that the pinctrl for the problematic boards should be
> corrected in upstream DT, but that is a separate issue and should not
> block adding support for the request()/gpio_request_enable() ops.
> 
> While the pcie reset-gpios full board freeze that was my driving factor
> to fully implement the gpio request() ops it is not the only use case,
> using the gpio cmd on a pin that use a non-gpio pinmux is another.
> 
> Or do you see any technical issue with having the gpio request() ops
> implemented and having it ensure gpio pinmux is used on a gpio requested
> pin? Similar to how gpio/pinctrl is behaving in Linux and on some other
> platforms in U-Boot?

Well, it removes the incentive to fix the upstream DTs and would make
it harder to notice missing pinctrls in the DTs.

Cheers,

Mark

> > Alex
> >>
> >> With the gpio and pinctrl ops implemented this series also remove a PCIe
> >> reset-gpios related device lock-up workaround from board u-boot.dtsi.
> >>
> >> PX30, RK3066, RK3188, RK356x and RK3588 are the only SoCs that currently
> >> define gpio-ranges props and is affected by this series.
> >>
> >> A follow up series adding support for the pinmux status cmd will also
> >> add gpio-ranges props for remaining RK SoCs.
> >>
> >> Jonas Karlman (4):
> >>    pinctrl: rockchip: Add gpio_request_enable() ops
> >>    gpio: rockchip: Add request() ops
> >>    rockchip: rk3568-rock-3a: Drop PCIe reset-gpios workaround
> >>    rockchip: rk3568-radxa-e25: Drop PCIe reset-gpios workaround
> >>
> >>   arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi     | 12 -------
> >>   arch/arm/dts/rk3568-rock-3a-u-boot.dtsi       | 12 -------
> >>   drivers/gpio/rk_gpio.c                        | 10 ++++++
> >>   .../pinctrl/rockchip/pinctrl-rockchip-core.c  | 31 +++++++++++++++++++
> >>   4 files changed, 41 insertions(+), 24 deletions(-)
> >>
> > 
> 
>