| Message ID | 00bab05a-5857-33de-5e3b-09b9c531d776@chocky.org |
|---|---|
| State | Not Applicable |
| Delegated to: | Petr Štetiar |
| Headers | show |
| Series | gpio-mt7621 offset fix for 5.10 kernel series | expand |
The sender domain has a DMARC Reject/Quarantine policy which disallows sending mailing list messages using the original "From" header. To mitigate this problem, the original message has been wrapped automatically by the mailing list software. Hello Peter, On Tue, Oct 18, 2022 at 9:34 PM Peter Naulls <peter@chocky.org> wrote: > > > Looks like there was some code loss when the driver came from an earlier kernel > series. Without this, my MT7621 board starts its GPIO offsets at 416 (why that > number, I don't know): You should also explain the problem with this behavior (if there's any). Upstream kernel doc [0] mentions: * @base: identifies the first GPIO number handled by this chip; * or, if negative during registration, requests dynamic ID allocation. * DEPRECATION: providing anything non-negative and nailing the base * offset of GPIO chips is deprecated. Please pass -1 as base to * let gpiolib select the chip base in all possible cases. We want to * get rid of the static GPIO number space in the long run. I never used it but my understanding is that the recommended way for accessing GPIOs from userspace (in case that's what you need) should be done through libgpiod. Best regards, Martin [0] https://elixir.bootlin.com/linux/v4.14.295/source/include/linux/gpio/driver.h#L48
On 10/18/22 15:55, Martin Blumenstingl wrote: > Hello Peter, > > On Tue, Oct 18, 2022 at 9:34 PM Peter Naulls <peter@chocky.org> wrote: >> >> >> Looks like there was some code loss when the driver came from an earlier kernel >> series. Without this, my MT7621 board starts its GPIO offsets at 416 (why that >> number, I don't know): > You should also explain the problem with this behavior (if there's any). > > Upstream kernel doc [0] mentions: > * @base: identifies the first GPIO number handled by this chip; > * or, if negative during registration, requests dynamic ID allocation. > * DEPRECATION: providing anything non-negative and nailing the base > * offset of GPIO chips is deprecated. Please pass -1 as base to > * let gpiolib select the chip base in all possible cases. We want to > * get rid of the static GPIO number space in the long run. > > I never used it but my understanding is that the recommended way for > accessing GPIOs from userspace (in case that's what you need) should > be done through libgpiod. Thanks for pointing me at this. Of course, I don't keep tabs on all the kernel development. I will say the following though: It looks a bit odd - the 416 is arbitrary at best, but I'm sure it comes from some calculation. All/most of the other ramips use the ramips GPIO driver instead, which does specify the base, although that's in the the DTS; there's no facility for that in the mt7621 setup at present. I ended up using named GPIO mappings set up in the DTS, which appears to be OK. I was chasing some additional GPIO issues vs what I had working on an earlier kernel series - it didn't immediately help, but it was an obvious inconsistency.
Hi Peter and Martin, > On 18 Oct 2022, at 22:02, Peter Naulls <peter@chocky.org> wrote: > > On 10/18/22 15:55, Martin Blumenstingl wrote: >> [...] my understanding is that the recommended way for >> accessing GPIOs from userspace (in case that's what you need) should >> be done through libgpiod. > > Thanks for pointing me at this. Of course, I don't keep tabs on all the > kernel development. I did investigate this in June on this list [1] and in the openwrt forum [2] and tried to ask some questions about the right way to handle this, but apparently it did not echo for some reason back then. > I will say the following though: > > It looks a bit odd - the 416 is arbitrary at best, but I'm sure it comes > from some calculation. The MT76xx GPIO has 3 banks with 32 GPIOs each, and the driver initializes them in the order 0,1,2. The automatic lecagcy GPIO number assignment works by allocating numbers down from 512 in chunks of 32. So bank 0 gets 512-32 = 480 as base, bank1 gets 448 and bank2 gets 416. But GPIOs *within* the chunks are counted upwards, so you'll get a very unintuitive zigzag mapping when comparing with the pin names in the datasheet. > All/most of the other ramips use the ramips GPIO driver instead, which > does specify the base, although that's in the the DTS; there's no > facility for that in the mt7621 setup at present. Just not any more - the mt7621 had this too. I currently patch it back into 22.03's gpio-mt7621.c for my builds and set base in the DTS, see [3] I can follow the rationale to get rid of legacy GPIOs, but in the context of experimenting platforms, where GPIOs are a thing to work with in user space, there's just no real replacement yet (see details in [1],[2]). So IMHO, at least the "base" of property should stay for a while. Best Regards Lukas [1] http://lists.openwrt.org/pipermail/openwrt-devel/2022-June/038912.html [2] https://forum.openwrt.org/t/state-of-userland-access-to-gpio-based-hardware/130505 [3] https://plan44.ch/downloads/v22.03-metapatch-readd-of-base-to-gpio-mt7621.patch
On 10/18/22 17:10, Lukas Zeller wrote: . > > Just not any more - the mt7621 had this too. I currently patch it back into > 22.03's gpio-mt7621.c for my builds and set base in the DTS, see [3] > > I can follow the rationale to get rid of legacy GPIOs, but in the context > of experimenting platforms, where GPIOs are a thing to work with in > user space, there's just no real replacement yet (see details in [1],[2]). Yes, I see. I have a mix of C and scripted GPIO access in my setup, and certainly I can move to libgpiod for that - or just just access them as files with named GPIOs as setup per the DTS. I do see the GPIO shell examples in the OpenWrt wiki, but the code needs more work to deal with multiple banks, and it just makes figuring out the GPIO number to use more clunky without any good cause. Now, the numbered GPIOs really are just for debug in my system, the actual code will use the named ones, but still. Is the long-term intent for shell scripting to instead use the libgpiod tools? https://openwrt.org/docs/techref/hardware/port.gpio
On Tue, Oct 18, 2022 at 3:50 PM Peter Naulls <peter@chocky.org> wrote: > > On 10/18/22 17:10, Lukas Zeller wrote: > . > > > > Just not any more - the mt7621 had this too. I currently patch it back into > > 22.03's gpio-mt7621.c for my builds and set base in the DTS, see [3] > > > > I can follow the rationale to get rid of legacy GPIOs, but in the context > > of experimenting platforms, where GPIOs are a thing to work with in > > user space, there's just no real replacement yet (see details in [1],[2]). > > Yes, I see. > > I have a mix of C and scripted GPIO access in my setup, and certainly I can > move to libgpiod for that - or just just access them as files with > named GPIOs as setup per the DTS. Let's CC Sergio, who upstreamed this driver. > > I do see the GPIO shell examples in the OpenWrt wiki, but the code needs > more work to deal with multiple banks, and it just makes figuring out > the GPIO number to use more clunky without any good cause. > > Now, the numbered GPIOs really are just for debug in my system, the > actual code will use the named ones, but still. > > Is the long-term intent for shell scripting to instead use the libgpiod > tools? > > https://openwrt.org/docs/techref/hardware/port.gpio > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
The sender domain has a DMARC Reject/Quarantine policy which disallows sending mailing list messages using the original "From" header. To mitigate this problem, the original message has been wrapped automatically by the mailing list software. On Wed, Oct 19, 2022 at 1:10 AM Rosen Penev <rosenp@gmail.com> wrote: > > On Tue, Oct 18, 2022 at 3:50 PM Peter Naulls <peter@chocky.org> wrote: > > > > On 10/18/22 17:10, Lukas Zeller wrote: > > . > > > > > > Just not any more - the mt7621 had this too. I currently patch it back into > > > 22.03's gpio-mt7621.c for my builds and set base in the DTS, see [3] > > > > > > I can follow the rationale to get rid of legacy GPIOs, but in the context > > > of experimenting platforms, where GPIOs are a thing to work with in > > > user space, there's just no real replacement yet (see details in [1],[2]). > > > > Yes, I see. > > > > I have a mix of C and scripted GPIO access in my setup, and certainly I can > > move to libgpiod for that - or just just access them as files with > > named GPIOs as setup per the DTS. > Let's CC Sergio, who upstreamed this driver. For kernel developers, setting base in GPIOs is a no go. You have to let the kernel to assign its numbers so you can handle different GPIO layouts with multiple chips. This is the reason we have 'gpio-line-names' property so you can set up names for your pins and use it together with actual user space tools libgpiod and gpiod. Any other gpio user space library is considered deprecated in these days. Best regards, Sergio Paracuellos > > > > I do see the GPIO shell examples in the OpenWrt wiki, but the code needs > > more work to deal with multiple banks, and it just makes figuring out > > the GPIO number to use more clunky without any good cause. > > > > Now, the numbered GPIOs really are just for debug in my system, the > > actual code will use the named ones, but still. > > > > Is the long-term intent for shell scripting to instead use the libgpiod > > tools? > > > > https://openwrt.org/docs/techref/hardware/port.gpio > > > > _______________________________________________ > > openwrt-devel mailing list > > openwrt-devel@lists.openwrt.org > > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On Wed, Oct 19, 2022 at 6:02 AM Sergio Paracuellos <sergio.paracuellos@gmail.com> wrote: > > On Wed, Oct 19, 2022 at 1:10 AM Rosen Penev <rosenp@gmail.com> wrote: > > > > On Tue, Oct 18, 2022 at 3:50 PM Peter Naulls <peter@chocky.org> wrote: > > > > > > On 10/18/22 17:10, Lukas Zeller wrote: > > > . > > > > > > > > Just not any more - the mt7621 had this too. I currently patch it back into > > > > 22.03's gpio-mt7621.c for my builds and set base in the DTS, see [3] > > > > > > > > I can follow the rationale to get rid of legacy GPIOs, but in the context > > > > of experimenting platforms, where GPIOs are a thing to work with in > > > > user space, there's just no real replacement yet (see details in [1],[2]). > > > > > > Yes, I see. > > > > > > I have a mix of C and scripted GPIO access in my setup, and certainly I can > > > move to libgpiod for that - or just just access them as files with > > > named GPIOs as setup per the DTS. > > Let's CC Sergio, who upstreamed this driver. > > For kernel developers, setting base in GPIOs is a no go. You have to > let the kernel to assign its numbers so you can handle different GPIO > layouts with multiple chips. > This is the reason we have 'gpio-line-names' property so you can set > up names for your pins and use it together with actual user space > tools libgpiod and gpiod. Any other gpio user space library is > considered deprecated in these days. See Linus comments about base in review: https://marc.info/?l=linux-gpio&m=152845919029921&w=1 Thanks, Sergio Paracuellos > > Best regards, > Sergio Paracuellos > > > > > > > I do see the GPIO shell examples in the OpenWrt wiki, but the code needs > > > more work to deal with multiple banks, and it just makes figuring out > > > the GPIO number to use more clunky without any good cause. > > > > > > Now, the numbered GPIOs really are just for debug in my system, the > > > actual code will use the named ones, but still. > > > > > > Is the long-term intent for shell scripting to instead use the libgpiod > > > tools? > > > > > > https://openwrt.org/docs/techref/hardware/port.gpio > > > > > > _______________________________________________ > > > openwrt-devel mailing list > > > openwrt-devel@lists.openwrt.org > > > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Sergio Paracuellos via openwrt-devel <openwrt-devel@lists.openwrt.org> writes: > This is the reason we have 'gpio-line-names' property so you can set > up names for your pins and use it together with actual user space > tools libgpiod and gpiod. Any other gpio user space library is > considered deprecated in these days. Interesting. This property doesn't seem to be used much in OpenWrt if my grep foo is good. It should probably be added at least to every system where we want to access GPIOs from userspace? "git grep ucidef_add_gpio_switch" shows lots of good candidates... For reference, there kernel docs describes 'gpio-line-names' as Optionally, a GPIO controller may have a "gpio-line-names" property. This is an array of strings defining the names of the GPIO lines going out of the GPIO controller. This name should be the most meaningful producer name for the system, such as a rail name indicating the usage. Package names such as pin name are discouraged: such lines have opaque names (since they are by definition generic purpose) and such names are usually not very helpful. For example "MMC-CD", "Red LED Vdd" and "ethernet reset" are reasonable line names as they describe what the line is used for. "GPIO0" is not a good name to give to a GPIO line. Placeholders are discouraged: rather use the "" (blank string) if the use of the GPIO line is undefined in your design. The names are assigned starting from line offset 0 from left to right from the passed array. An incomplete array (where the number of passed named are less than ngpios) will still be used up until the last provided valid line index. Bjørn
--- a/drivers/gpio/gpio-mt7621.c 2022-10-18 15:03:42.596454871 -0400 +++ b/drivers/gpio/gpio-mt7621.c 2022-10-18 13:51:23.628305673 -0400 @@ -234,6 +234,7 @@ return ret; } + rg->chip.base = rg->bank * MTK_BANK_WIDTH; rg->chip.of_gpio_n_cells = 2; rg->chip.of_xlate = mediatek_gpio_xlate; rg->chip.label = devm_kasprintf(dev, GFP_KERNEL, "%s-bank%d",