Hello,
I am currently trying to bring up Linux 6.1-rcX on a PXA255 board, using a
devicetree. One problem I encountered is that the PXA GPIO driver fails to
probe because it uses the same MMIO register range as the pinctrl driver:
[ 0.666169] pxa25x-pinctrl 40e00054.pinctrl: initialized pxa2xx pinctrl driver
[ 0.694407] pxa-gpio 40e00000.gpio: can't request region for resource [mem 0x40e00000-0x40e0ffff]
[ 0.695050] pxa-gpio: probe of 40e00000.gpio failed with error -16
Before I try to fix this myself: Is GPIO on PXA25x currently expected to
work and when has it last been seen working?
What would be a good way to fix this?
I've considered calling into pinctrl-pxa25x directly from pxa-gpio and
passing in the GPIO driver's register mapping, rather than mapping the
registers again in pinctrl-pxa25x. This seems like it should address the
kernel's requirement that resources shouldn't overlap, but some
additional locking between GPIO driver and pinctrl driver might be
neccessary to keep register accesses consistent/atomic.
Best regards,
Jonathan
On Wed, Dec 07, 2022 at 12:25:53PM +0100, Jonathan Neusch?fer wrote:
> Hello,
>
> I am currently trying to bring up Linux 6.1-rcX on a PXA255 board, using a
> devicetree. One problem I encountered is that the PXA GPIO driver fails to
> probe because it uses the same MMIO register range as the pinctrl driver:
>
> [ 0.666169] pxa25x-pinctrl 40e00054.pinctrl: initialized pxa2xx pinctrl driver
> [ 0.694407] pxa-gpio 40e00000.gpio: can't request region for resource [mem 0x40e00000-0x40e0ffff]
> [ 0.695050] pxa-gpio: probe of 40e00000.gpio failed with error -16
>
> Before I try to fix this myself: Is GPIO on PXA25x currently expected to
> work and when has it last been seen working?
>
> What would be a good way to fix this?
How are the registers arranged? Is 0x40e00000-0x40e0ffff simply too
large, and making it smaller would fix the issue? Or are the registers
interleaved? It is possible to request a region in a non-exclusive
manor. Or is it more than interleaving, individual registers need to
be shared between the two drivers? If so, you have the locking issues
you mentioned.
Andrew
On Wed, Dec 07, 2022 at 04:28:07PM +0100, Andrew Lunn wrote:
> On Wed, Dec 07, 2022 at 12:25:53PM +0100, Jonathan Neusch?fer wrote:
> > Hello,
> >
> > I am currently trying to bring up Linux 6.1-rcX on a PXA255 board, using a
> > devicetree. One problem I encountered is that the PXA GPIO driver fails to
> > probe because it uses the same MMIO register range as the pinctrl driver:
> >
> > [ 0.666169] pxa25x-pinctrl 40e00054.pinctrl: initialized pxa2xx pinctrl driver
> > [ 0.694407] pxa-gpio 40e00000.gpio: can't request region for resource [mem 0x40e00000-0x40e0ffff]
> > [ 0.695050] pxa-gpio: probe of 40e00000.gpio failed with error -16
> >
> > Before I try to fix this myself: Is GPIO on PXA25x currently expected to
> > work and when has it last been seen working?
> >
> > What would be a good way to fix this?
>
> How are the registers arranged?
As documented in drivers/gpio/gpio-pxa.c - it'll be easier for you to
look there rather than for me to explain it - but suffice it to say
that the pinctrl registers are amongst the GPIO registers.
> Is 0x40e00000-0x40e0ffff simply too
> large, and making it smaller would fix the issue? Or are the registers
> interleaved?
They're interleaved. Looking at the .dtsi file for PXA25x, it seems
that the pinctrl claims just the addresses that it needs, but the GPIO
controller has no reg property in the .dtsi, so I'm not sure what fills
that information in.
DT could describe the region for PXA25x, which is just:
0x40e00000 - 0x40e00054
Later PXA have more banks, so would require additional resources to be
listed.
However, first, we need to know what provides the iomem resource for
this:
gpio: gpio@40e00000 {
compatible = "intel,pxa25x-gpio";
gpio-ranges = <&pinctrl 0 0 84>;
clocks = <&clks CLK_NONE>;
};
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Wed, Dec 07, 2022 at 04:28:07PM +0100, Andrew Lunn wrote:
> On Wed, Dec 07, 2022 at 12:25:53PM +0100, Jonathan Neuschäfer wrote:
> > Hello,
> >
> > I am currently trying to bring up Linux 6.1-rcX on a PXA255 board, using a
> > devicetree. One problem I encountered is that the PXA GPIO driver fails to
> > probe because it uses the same MMIO register range as the pinctrl driver:
> >
> > [ 0.666169] pxa25x-pinctrl 40e00054.pinctrl: initialized pxa2xx pinctrl driver
> > [ 0.694407] pxa-gpio 40e00000.gpio: can't request region for resource [mem 0x40e00000-0x40e0ffff]
> > [ 0.695050] pxa-gpio: probe of 40e00000.gpio failed with error -16
> >
> > Before I try to fix this myself: Is GPIO on PXA25x currently expected to
> > work and when has it last been seen working?
> >
> > What would be a good way to fix this?
>
> How are the registers arranged? Is 0x40e00000-0x40e0ffff simply too
> large, and making it smaller would fix the issue? Or are the registers
> interleaved? It is possible to request a region in a non-exclusive
> manor. Or is it more than interleaving, individual registers need to
> be shared between the two drivers? If so, you have the locking issues
> you mentioned.
>
> Andrew
(I wrote this before I saw the other reply, so there are probably
duplicate information)
They overlap. pxa25x.dtsi declares:
pinctrl: pinctrl@40e00000 {
reg = <0x40e00054 0x20>, // base_af[0]
<0x40e0000c 0xc>, // base_dir[0]
<0x40e0010c 4>, // base_dir[3]
<0x40f00020 0x10>; // base_sleep[0]
compatible = "marvell,pxa25x-pinctrl";
};
(comments mine, based on pinctrl-pxxa25x.c)
The GPIO driver mentions these registers (for PXA25x):
GPLR GPDR GPSR GPCR GRER GFER GEDR
BANK 0 - 0x0000 0x000C 0x0018 0x0024 0x0030 0x003C 0x0048
BANK 1 - 0x0004 0x0010 0x001C 0x0028 0x0034 0x0040 0x004C
BANK 2 - 0x0008 0x0014 0x0020 0x002C 0x0038 0x0044 0x0050
So, there are overlaps in GPDR (0x0c-0x14, GPIO pin direction register).
The register at 0x10c seems bogus for PXA25x: gpio-pxa.c mentions it as
part of bank 3, which only exists in PXA27x or later.
Jonathan
On Wed, Dec 07, 2022 at 03:41:11PM +0000, Russell King (Oracle) wrote:
> On Wed, Dec 07, 2022 at 04:28:07PM +0100, Andrew Lunn wrote:
[...]
> > How are the registers arranged?
>
> As documented in drivers/gpio/gpio-pxa.c - it'll be easier for you to
> look there rather than for me to explain it - but suffice it to say
> that the pinctrl registers are amongst the GPIO registers.
>
> > Is 0x40e00000-0x40e0ffff simply too
> > large, and making it smaller would fix the issue? Or are the registers
> > interleaved?
>
> They're interleaved. Looking at the .dtsi file for PXA25x, it seems
> that the pinctrl claims just the addresses that it needs, but the GPIO
> controller has no reg property in the .dtsi, so I'm not sure what fills
> that information in.
The GPIO reg property is in pxa2xx.dtsi.
>
> DT could describe the region for PXA25x, which is just:
>
> 0x40e00000 - 0x40e00054
>
> Later PXA have more banks, so would require additional resources to be
> listed.
>
> However, first, we need to know what provides the iomem resource for
> this:
>
> gpio: gpio@40e00000 {
> compatible = "intel,pxa25x-gpio";
> gpio-ranges = <&pinctrl 0 0 84>;
> clocks = <&clks CLK_NONE>;
> };
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Jonathan
On Wed, Dec 07, 2022 at 05:44:58PM +0100, Jonathan Neusch?fer wrote:
> On Wed, Dec 07, 2022 at 03:41:11PM +0000, Russell King (Oracle) wrote:
> > On Wed, Dec 07, 2022 at 04:28:07PM +0100, Andrew Lunn wrote:
> [...]
> > > How are the registers arranged?
> >
> > As documented in drivers/gpio/gpio-pxa.c - it'll be easier for you to
> > look there rather than for me to explain it - but suffice it to say
> > that the pinctrl registers are amongst the GPIO registers.
> >
> > > Is 0x40e00000-0x40e0ffff simply too
> > > large, and making it smaller would fix the issue? Or are the registers
> > > interleaved?
> >
> > They're interleaved. Looking at the .dtsi file for PXA25x, it seems
> > that the pinctrl claims just the addresses that it needs, but the GPIO
> > controller has no reg property in the .dtsi, so I'm not sure what fills
> > that information in.
>
> The GPIO reg property is in pxa2xx.dtsi.
Looks to me like pxa25x should override the reg property with a smaller
range (0x54) and pxa27x probably should have used a second set of
entries in reg the subsequent group of 3 gpio blocks at offset 0x100.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Wed, Dec 07, 2022 at 06:27:52PM +0000, Russell King (Oracle) wrote:
> On Wed, Dec 07, 2022 at 05:44:58PM +0100, Jonathan Neuschäfer wrote:
> > On Wed, Dec 07, 2022 at 03:41:11PM +0000, Russell King (Oracle) wrote:
> > > On Wed, Dec 07, 2022 at 04:28:07PM +0100, Andrew Lunn wrote:
> > [...]
> > > > How are the registers arranged?
> > >
> > > As documented in drivers/gpio/gpio-pxa.c - it'll be easier for you to
> > > look there rather than for me to explain it - but suffice it to say
> > > that the pinctrl registers are amongst the GPIO registers.
> > >
> > > > Is 0x40e00000-0x40e0ffff simply too
> > > > large, and making it smaller would fix the issue? Or are the registers
> > > > interleaved?
> > >
> > > They're interleaved. Looking at the .dtsi file for PXA25x, it seems
> > > that the pinctrl claims just the addresses that it needs, but the GPIO
> > > controller has no reg property in the .dtsi, so I'm not sure what fills
> > > that information in.
> >
> > The GPIO reg property is in pxa2xx.dtsi.
>
> Looks to me like pxa25x should override the reg property with a smaller
> range (0x54) and pxa27x probably should have used a second set of
> entries in reg the subsequent group of 3 gpio blocks at offset 0x100.
Without further code changes this wouldn't be sufficient, because the
pinctrl driver also touches the GPIO direction registers at offset
0x0c-0x14.
Jonathan
Jonathan Neuschäfer <[email protected]> writes:
> Without further code changes this wouldn't be sufficient,
> because the
> pinctrl driver also touches the GPIO direction registers at
> offset
> 0x0c-0x14.
Historically, this problem was adressed by a "hack" if I remember
correctly, as the
registers do overlap (on pxa27x at least, but I think on pxa25x as
well) :
- GAFR (alternate functions) and GPDR (input or output) are
"pinctrl"
- GPSR, GPCR, GPLR are "gpio" registers
- the memory map (physical) at 0x40e0 0000 is as Jonathan wrote :
- 3 u32 GPLR (gpio)
- 3 u32 GPDR (pinctrl)
- 3 u32 GPSR (gpio)
- 3 u32 GPCR (gpio)
The "hack" was that one driver was mapping the area _without_
claiming it (this
is part from memory, I didn't check in the code today). The gpio
was probably
the claiming one, while the pinctrl was the "only using one".
As of today, I should have a look what was changed, but I'm pretty
sure in all
pxa2xx architecture there is an overlap, as for these ancient
platforms the
pinctrl wasn't yet separated from the gpio IC.
The only was out so far I can see from my head would be to declare
multiple
very small IO ranges :
- pinctrl pxa25x : 0x40e0 000c (12 bytes), 0x40e0 0054 (32 bytes)
- pinctrl pxa25x : 0x40e0 000c (12 bytes), 0x40e0 0054 (32 bytes),
0x40e0 010c (4 bytes)
- gpio : from 0x40e0 0000 to 0x40e0 0070 included, without the
pinctrl ones
As a general guide, in gpio-pxa.c, you have the table in
[1]. Think GPDR and GAFR as
pinctrl, and all the other ones as gpio. Ah and yes, the GAFR ones
are missing in this
table.
One last think : in a pre device-tree world, when we didn't had
yet the pxa pinctrl
driver, the gpio driver was directly playing with the GPDR
registers, fun old times.
Cheers.
--
Robert
[1]
/*
* We handle the GPIOs by banks, each bank covers up to 32 GPIOs
with
* one set of registers. The register offsets are organized below:
*
* GPLR GPDR GPSR GPCR GRER GFER GEDR
* BANK 0 - 0x0000 0x000C 0x0018 0x0024 0x0030 0x003C 0x0048
* BANK 1 - 0x0004 0x0010 0x001C 0x0028 0x0034 0x0040 0x004C
* BANK 2 - 0x0008 0x0014 0x0020 0x002C 0x0038 0x0044 0x0050
*
* BANK 3 - 0x0100 0x010C 0x0118 0x0124 0x0130 0x013C 0x0148
* BANK 4 - 0x0104 0x0110 0x011C 0x0128 0x0134 0x0140 0x014C
* BANK 5 - 0x0108 0x0114 0x0120 0x012C 0x0138 0x0144 0x0150
*
* BANK 6 - 0x0200 0x020C 0x0218 0x0224 0x0230 0x023C 0x0248
*
* NOTE:
* BANK 3 is only available on PXA27x and later processors.
* BANK 4 and 5 are only available on PXA935, PXA1928
* BANK 6 is only available on PXA1928
*/
On Thu, Dec 08, 2022 at 07:46:26PM +0100, Robert Jarzmik wrote:
>
> Jonathan Neusch?fer <[email protected]> writes:
>
> > Without further code changes this wouldn't be sufficient, because the
> > pinctrl driver also touches the GPIO direction registers at offset
> > 0x0c-0x14.
>
> Historically, this problem was adressed by a "hack" if I remember correctly,
> as the
> registers do overlap (on pxa27x at least, but I think on pxa25x as well) :
> - GAFR (alternate functions) and GPDR (input or output) are "pinctrl"
> - GPSR, GPCR, GPLR are "gpio" registers
> - the memory map (physical) at 0x40e0 0000 is as Jonathan wrote :
> - 3 u32 GPLR (gpio)
> - 3 u32 GPDR (pinctrl)
> - 3 u32 GPSR (gpio)
> - 3 u32 GPCR (gpio)
>
> The "hack" was that one driver was mapping the area _without_ claiming it
> (this
> is part from memory, I didn't check in the code today). The gpio was
> probably
> the claiming one, while the pinctrl was the "only using one".
>
> As of today, I should have a look what was changed, but I'm pretty sure in
> all
> pxa2xx architecture there is an overlap, as for these ancient platforms the
> pinctrl wasn't yet separated from the gpio IC.
>
> The only was out so far I can see from my head would be to declare multiple
> very small IO ranges :
> - pinctrl pxa25x : 0x40e0 000c (12 bytes), 0x40e0 0054 (32 bytes)
> - pinctrl pxa25x : 0x40e0 000c (12 bytes), 0x40e0 0054 (32 bytes), 0x40e0
> 010c (4 bytes)
> - gpio : from 0x40e0 0000 to 0x40e0 0070 included, without the pinctrl ones
>
> As a general guide, in gpio-pxa.c, you have the table in [1]. Think GPDR and
> GAFR as
> pinctrl, and all the other ones as gpio. Ah and yes, the GAFR ones are
> missing in this
> table.
>
> One last think : in a pre device-tree world, when we didn't had yet the pxa
> pinctrl
> driver, the gpio driver was directly playing with the GPDR registers, fun
> old times.
From a quick look, I think this commit is to blame:
542c25b7a209 drivers: gpio: pxa: use devm_platform_ioremap_resource()
Someone "helpfully" making this change:
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- return -EINVAL;
- gpio_reg_base = devm_ioremap(&pdev->dev, res->start,
- resource_size(res));
+
+ gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
which introduces request_mem_region() to the PXA GPIO driver, resulting
in this resource clash.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
"Russell King (Oracle)" <[email protected]> writes:
> From a quick look, I think this commit is to blame:
>
> 542c25b7a209 drivers: gpio: pxa: use
> devm_platform_ioremap_resource()
>
> Someone "helpfully" making this change:
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res)
> - return -EINVAL;
> - gpio_reg_base = devm_ioremap(&pdev->dev, res->start,
> - resource_size(res));
> +
> + gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
>
> which introduces request_mem_region() to the PXA GPIO driver,
> resulting
> in this resource clash.
You're right.
Now, as for Jonathan, there are 2 options :
- first one : revert the patch Russell pointed out
- second one : a lot of work, detailed below :
Given that Arnd is removing legacy platformdata code, that will
only leave the
device-tree one, which works only with the pinctrl driver enabled.
That gives the opportunity to drop the use of GPDR from the
gpio-pxa driver,
and now we can map with separate iomem ressources pinctrl-pxa and
gpio-pxa.
Therefore, if Jonathan has the will to make a patch, I would :
- keep the patch identfied by Russell (ie. don't revert it)
- amend the device-tree descriptions for pxa
- pxa2xx.dtsi
...
gpio: gpio@40e00000 {
...
reg = <0x40e00000 0xc>, <0x40e00018 0x3c>,
<0x40e00100 4>, <0x40e00118 0x34>;
- amend the gpio_pxa.c driver, to map the 4 regions (and not only
one as before)
- the pinctrl-pxa25.c and its device-tree are already mapping the
holes in the
previous list
- check that I didn't mess up the 4 iomem regions, they should
overlap with
"pinctrl: pinctrl@40e00000".
Well it's up to Jonathan to see which version he prefers to
choose, the simple
one or the tedious one.
Cheers.
--
Robert
Thank you both for you help!
On Thu, Dec 08, 2022 at 09:19:12PM +0100, Robert Jarzmik wrote:
>
> "Russell King (Oracle)" <[email protected]> writes:
> > From a quick look, I think this commit is to blame:
> >
> > 542c25b7a209 drivers: gpio: pxa: use devm_platform_ioremap_resource()
> >
> > Someone "helpfully" making this change:
> >
> > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > - if (!res)
> > - return -EINVAL;
> > - gpio_reg_base = devm_ioremap(&pdev->dev, res->start,
> > - resource_size(res));
> > +
> > + gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
> >
> > which introduces request_mem_region() to the PXA GPIO driver, resulting
> > in this resource clash.
> You're right.
>
> Now, as for Jonathan, there are 2 options :
> - first one : revert the patch Russell pointed out
I'll try that first.
> - second one : a lot of work, detailed below :
>
> Given that Arnd is removing legacy platformdata code, that will only leave
> the
> device-tree one, which works only with the pinctrl driver enabled.
> That gives the opportunity to drop the use of GPDR from the gpio-pxa driver,
> and now we can map with separate iomem ressources pinctrl-pxa and gpio-pxa.
>
> Therefore, if Jonathan has the will to make a patch, I would :
> - keep the patch identfied by Russell (ie. don't revert it)
> - amend the device-tree descriptions for pxa
> - pxa2xx.dtsi
> ...
> gpio: gpio@40e00000 {
> ...
> reg = <0x40e00000 0xc>, <0x40e00018 0x3c>, <0x40e00100 4>, <0x40e00118
> 0x34>;
> - amend the gpio_pxa.c driver, to map the 4 regions (and not only one as
> before)
> - the pinctrl-pxa25.c and its device-tree are already mapping the holes in
> the
> previous list
> - check that I didn't mess up the 4 iomem regions, they should overlap with
> "pinctrl: pinctrl@40e00000".
And I might try this as well and see if I hit any additional obstacles.
I'll probably get around to it in january.
Thanks again,
Jonathan
On Thu, Dec 08, 2022 at 09:19:12PM +0100, Robert Jarzmik wrote:
>
> "Russell King (Oracle)" <[email protected]> writes:
> > From a quick look, I think this commit is to blame:
> >
> > 542c25b7a209 drivers: gpio: pxa: use devm_platform_ioremap_resource()
> >
> > Someone "helpfully" making this change:
> >
> > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > - if (!res)
> > - return -EINVAL;
> > - gpio_reg_base = devm_ioremap(&pdev->dev, res->start,
> > - resource_size(res));
> > +
> > + gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
> >
> > which introduces request_mem_region() to the PXA GPIO driver, resulting
> > in this resource clash.
> You're right.
>
> Now, as for Jonathan, there are 2 options :
> - first one : revert the patch Russell pointed out
> - second one : a lot of work, detailed below :
>
> Given that Arnd is removing legacy platformdata code, that will only leave
> the
> device-tree one, which works only with the pinctrl driver enabled.
> That gives the opportunity to drop the use of GPDR from the gpio-pxa driver,
> and now we can map with separate iomem ressources pinctrl-pxa and gpio-pxa.
Does the GPIO driver talk to the pinctrl driver to switch GPIOs between
input and output mode? If it does, that's fine. If not, that can cause
regressions, as GPIOs may need to be switched between input and output
mode at runtime (e.g. for I2C gpio-based bitbang).
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
"Russell King (Oracle)" <[email protected]> writes:
> Does the GPIO driver talk to the pinctrl driver to switch GPIOs
> between
> input and output mode? If it does, that's fine. If not, that can
> cause
> regressions, as GPIOs may need to be switched between input and
> output
> mode at runtime (e.g. for I2C gpio-based bitbang).
It does.
Basically it tries pinctrl first, and falls back to direct
registers
access. Here is a small extract of the gpio direction manipulation
in
gpio-pxa.c :
if (pxa_gpio_has_pinctrl()) {
ret = pinctrl_gpio_direction_output(chip->base +
offset);
if (ret)
return ret;
}
spin_lock_irqsave(&gpio_lock, flags);
tmp = readl_relaxed(base + GPDR_OFFSET);
... blablabal we set the bit, and write back the register.
Cheers.
--
Robert