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

[Mark Kettenis]

> Date: Sun, 12 May 2024 21:49:21 +0200
> From: Alex Bee <knaerzche at gmail.com>
> 
> Am 11.05.24 um 20:47 schrieb Jonas Karlman:
> > 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 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?
> No, no general ("technical") issue with adding a .request hook to the gpio
> driver. But now you are now moving the original workaround to an even more
> invisible place which does things implicitly. Maybe just don't remove the
> pinctrl from the boards u-boot-dtsi's - just replace it with
> 
> &pcie30x2m1_pins {
>      rockchip,pins =
>               <2 RK_PD4 4 &pcfg_pull_none>,
>               <2 RK_PD6 RK_FUNC_GPIO &pcfg_pull_none>,
>               <2 RK_PD5 4 &pcfg_pull_none>;
> };
> 
> Even if it would (now) work without. It, at least, documents that there are
> things left to do for the upstream DT.

Right.  See the reply I just sent to Jonas.  I think that is the right
approach.  Probably means that the upstream rk3568-pinctrl.dtsi should
grow separate nodes for when the PCIe controller is in EP or RC mode.
Or maybe the perstn configuration should be split out from the
clkreqn/waken configuration like how buttonrstn is configured.

> What you were saying in reply to Mark's email is not completely true: Not
> all pins are initialized with gpio func as default. It actually depends on
> the pin which function the bootrom sets initially. In case of RK356x's
> GPIO2_PD6 (the pin in question) it's BT656_D6M0 (func2), for instance. So,
> in fact, you are changing it's function when implicitly setting it's func
> to gpio (func0).
> 
> Alex
> 
> >
> > Regards,
> > Jonas
> >
> >> 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(-)
> >>>
>