> Date: Sun, 12 May 2024 21:49:21 +0200 > From: Alex Bee <knaerz...@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(-) > >>> >