Hi,
[+cc John Crispin]
On Sat, Jan 21, 2023 at 2:45 PM Arınç ÜNAL <arinc.u...@arinc9.com> wrote:
>
> On 21.01.2023 10:56, Sergio Paracuellos wrote:
> > Hi,
> >
> > On Sat, Jan 21, 2023 at 7:03 AM Arınç ÜNAL <arinc.u...@arinc9.com> wrote:
> >>
> >> Pins from 22 to 33 are on the rgmii2 pin group. They don't function as
> >> GPIO by default. Requesting a gpio by either from devicetree or `echo
> >> 203 > /sys/class/gpio/export` won't change anything. You have to claim
> >> the pin group as gpio on the devicetree.
> >
> > Yes, you have to claim the pin group as gpio on the device tree to
> > make this work. Ralink has the concept of "GPIO mode" but actually is
> > just an electrical configuration for a certain device. So if the mode
> > (function) is not requested as a real GPIO nothing is going to work.
> > So in your board's dts file you have to add something like the
> > following with the groups you want to claim as real gpio function:
> >
> > #include "mt7621.dtsi"
> > ...
> >
> > &state_default {
> > gpio {
> > groups = "jtag", "uart3", "wdt";
> > function = "gpio";
> > };
> > };
> >
> >>
> >> Quoting my response from [0]:
> >>
> >>> state_default is there to explicitly set the function of a pin group to
> >>> gpio, this is done because the bootloader may have set the function of a
> >>> pin group to something else before booting OpenWrt which would render the
> >>> pins of that group uncontrollable for general purpose aka GPIO.
> >>>
> >>> Actually I think @arinc9 did some work around that.
> >>>
> >>> Not yet, I plan to modify the gpio_request_enable pinmux operation to set
> >>> the pin group as gpio when there's a gpio request for a pin in that pin
> >>> group. gpio_request_enable pinmux operation can only set the function of
> >>> an individual pin currently. Since ralink pinctrl driver can only set the
> >>> function of a group of pins, the operation currently cannot be used.
> >>>
> >>> If we make it work, any GPIO defined on devicetree or exported from
> >>> userspace will automatically have the function of the pin group it's in
> >>> set to gpio, completely getting rid of the need for explicitly defining
> >>> functions of certain pin groups on the devicetree.
> >>
> >> Of course when I said "I plan to modify this code" I actually meant I
> >> was going to talk this through with Sergio but I never had the
> >> opportunity to do so. I guess this thread is a good place to start
> >> talking about this.
> >>
> >> I had this case on a user:
> >>
> >> They got an LED wired to wdt pin. GPIO is already exported on the DT.
> >> However their LED just won't work.
> >>
> >> It turns out the bootloader sets the wdt pin's function to something
> >> other than gpio. And when OpenWrt boots, the pinctrl driver makes no
> >> changes to the pin's function.
> >
> > Bootloader always sets its own configuration for the pinctrl. The
> > linux pinctrl driver sets every single group default mode [0] as it is
> > in the Mediatek's Mt7621 datasheet.
> >
> >>
> >> So we had to specifically claim that pin as gpio to make the LED work.
> >> Now there is already a solution for this which is the
> >> gpio_request_enable pinmux operation but it's not supposed to be used on
> >> pinctrl drivers that cannot control pins individually.
> >>
> >> Sergio, you think we can somehow make this pinmux operation mux a pin
> >> group as gpio instead of a single pin?
> >
> > I am not an expert in pinmux drivers but I think there are strong
> > reasons why only a single pin is allowed to be requested.
> >
> > See kernel doc about this here: [1]
> >
> >>
> >> Or introduce a new pinmux operation that can do this?
> >
> > I think you should send an email to kernel gpio / pinctrl kernel mail
> > list to get feedback from Bart and Linus as gpio and pin control
> > maintainers to properly understand the way to go but I don't really
> > understand what is the problem requesting the group as gpio in the
> > device tree like any other single platform is doing and seems to be
> > the correct way to go. Maybe I am missing something :)
>
> From what I understand, a gpio is requested by exporting a gpio by
> either from devicetree or `echo 203 > /sys/class/gpio/export`.
/sys/class/gpio is marked as deprecated [0] since kernel version 4.8,
please, avoid using it. Use libgpiod instead.
>
> Now, the pinctrl driver must somehow know that the pin which translates
> to the GPIO number needs to function as gpio.
>
> Doing this manually on DT bindings is an option but it's not very
> viable. I believe this is why the gpio_request_enable operation was made
> for pinctrl drivers to implement. Now a pin can be made to function as
> GPIO from userspace dynamically instead of hardcoding it on the devicetree.
Yes, 'gpio_request_enable()' is thought to request gpio on the desired pin.
>
> Boards with pinouts, like Raspberrypi, Bananapi, etc. benefit this the
> most. Because it'd be extremely hard to hardcode every pin with pinouts
> on the devicetree for each different device.
>
> For example, my Unielec U7621 board uses the rgmii2 pins for ethernet
> but at the same time it's got pinouts for them. If the pinmux operation
> worked, I could just export the gpio number and the pin would function
> as gpio. When I'm done, I could just unexport and the pin group would go
> back to function as an rgmii bus.
>
> I believe this is already the case with pinctrl drivers that can control
> pins individually. There's no state-default node on DT where some pins
> are hardcoded to function as gpio.
>
> MediaTek Moore Pinctrl driver which can control pins individually
> implements gpio_request_enable.
>
> https://github.com/torvalds/linux/blob/master/drivers/pinctrl/mediatek/pinctrl-moore.c#L77
>
> gpio_request_enable is also there on the Ralink Pinctrl driver but I
> don't think it does anything.
>
> https://github.com/torvalds/linux/blob/master/drivers/pinctrl/ralink/pinctrl-ralink.c#L161
AFAICS, the Ralink driver sets gpio mode for a group of pins using
set_mux operation [1] so when the
gpio_request_enable() operation is called a check for that pin is set
as gpio is performed. Nothing else.
Maybe John Crispin who is the writer of this driver can explain a bit
more about this.
[0]: https://www.kernel.org/doc/Documentation/gpio/sysfs.txt
[1]:
https://github.com/torvalds/linux/blob/master/drivers/pinctrl/ralink/pinctrl-ralink.c#L117
Best regards,
Sergio Paracuellos
>
> Arınç
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
