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.

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(-)

Reply via email to