* [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17
@ 2020-08-14 10:03 Thomas Preston
2020-08-14 10:03 ` [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config Thomas Preston
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Thomas Preston @ 2020-08-14 10:03 UTC (permalink / raw)
To: linus.walleij, robh+dt, linux-gpio, devicetree, linux-kernel
Cc: thomas.preston
Hi,
I'm in the process of adding a device tree overlay for the PiFace
Digital Raspberry Pi daughter board [0]. It's an mcp23s17 SPI GPIO port
expander. In doing so, I noticed some errors with the mcp23s08 driver.
This series adds my fixups to (which builds successfully):
v5.8-13252-gcf4a66ae5e4a
But I have only tested them on (rpi-5.4.y):
v5.4.51-1078-g92834e4bb4ce
They're quite trivial and backwards compatible, although I might be
wrong about "interrupt-controller". Can someone please confirm?
Many thanks,
Thomas
[0] https://github.com/raspberrypi/linux/pull/3794
Thomas Preston (3):
pinctrl: mcp23s08: Fixup mcp23x17 regmap_config
pinctrl: mcp23s08: Remove interrupt-controller
devicetree: mcp23s08: Remove interrupt-controller
.../bindings/pinctrl/pinctrl-mcp23s08.txt | 8 -----
drivers/pinctrl/pinctrl-mcp23s08.c | 35 +++++++++----------
drivers/pinctrl/pinctrl-mcp23s08.h | 2 +-
3 files changed, 18 insertions(+), 27 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config 2020-08-14 10:03 [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 Thomas Preston @ 2020-08-14 10:03 ` Thomas Preston 2020-08-28 9:06 ` Linus Walleij 2020-08-28 10:09 ` Andy Shevchenko 2020-08-14 10:03 ` [PATCH 2/3] pinctrl: mcp23s08: Remove interrupt-controller Thomas Preston ` (2 subsequent siblings) 3 siblings, 2 replies; 12+ messages in thread From: Thomas Preston @ 2020-08-14 10:03 UTC (permalink / raw) To: linus.walleij, robh+dt, linux-gpio, devicetree, linux-kernel Cc: thomas.preston - Fix a typo where mcp23x17 configs are referred to as mcp23x16. - Fix precious range to include INTCAP{A,B}, which clear on read. - Fix precious range to include GPIOB, which clears on read. - Fix volatile range to include GPIOB, to fix debugfs registers reporting different values than `gpioget gpiochip2 {0..15}`. Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk> --- drivers/pinctrl/pinctrl-mcp23s08.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c index 42b12ea14d6b..0138638276e7 100644 --- a/drivers/pinctrl/pinctrl-mcp23s08.c +++ b/drivers/pinctrl/pinctrl-mcp23s08.c @@ -87,7 +87,7 @@ const struct regmap_config mcp23x08_regmap = { }; EXPORT_SYMBOL_GPL(mcp23x08_regmap); -static const struct reg_default mcp23x16_defaults[] = { +static const struct reg_default mcp23x17_defaults[] = { {.reg = MCP_IODIR << 1, .def = 0xffff}, {.reg = MCP_IPOL << 1, .def = 0x0000}, {.reg = MCP_GPINTEN << 1, .def = 0x0000}, @@ -98,23 +98,23 @@ static const struct reg_default mcp23x16_defaults[] = { {.reg = MCP_OLAT << 1, .def = 0x0000}, }; -static const struct regmap_range mcp23x16_volatile_range = { +static const struct regmap_range mcp23x17_volatile_range = { .range_min = MCP_INTF << 1, - .range_max = MCP_GPIO << 1, + .range_max = (MCP_GPIO << 1) + 1, }; -static const struct regmap_access_table mcp23x16_volatile_table = { - .yes_ranges = &mcp23x16_volatile_range, +static const struct regmap_access_table mcp23x17_volatile_table = { + .yes_ranges = &mcp23x17_volatile_range, .n_yes_ranges = 1, }; -static const struct regmap_range mcp23x16_precious_range = { - .range_min = MCP_GPIO << 1, - .range_max = MCP_GPIO << 1, +static const struct regmap_range mcp23x17_precious_range = { + .range_min = MCP_INTCAP << 1, + .range_max = (MCP_GPIO << 1) + 1, }; -static const struct regmap_access_table mcp23x16_precious_table = { - .yes_ranges = &mcp23x16_precious_range, +static const struct regmap_access_table mcp23x17_precious_table = { + .yes_ranges = &mcp23x17_precious_range, .n_yes_ranges = 1, }; @@ -124,10 +124,10 @@ const struct regmap_config mcp23x17_regmap = { .reg_stride = 2, .max_register = MCP_OLAT << 1, - .volatile_table = &mcp23x16_volatile_table, - .precious_table = &mcp23x16_precious_table, - .reg_defaults = mcp23x16_defaults, - .num_reg_defaults = ARRAY_SIZE(mcp23x16_defaults), + .volatile_table = &mcp23x17_volatile_table, + .precious_table = &mcp23x17_precious_table, + .reg_defaults = mcp23x17_defaults, + .num_reg_defaults = ARRAY_SIZE(mcp23x17_defaults), .cache_type = REGCACHE_FLAT, .val_format_endian = REGMAP_ENDIAN_LITTLE, }; -- 2.26.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config 2020-08-14 10:03 ` [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config Thomas Preston @ 2020-08-28 9:06 ` Linus Walleij 2020-08-28 9:28 ` Andy Shevchenko 2020-08-28 10:09 ` Andy Shevchenko 1 sibling, 1 reply; 12+ messages in thread From: Linus Walleij @ 2020-08-28 9:06 UTC (permalink / raw) To: Thomas Preston, Andy Shevchenko, Jan Kundrát, Phil Reid Cc: open list:GPIO SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On Fri, Aug 14, 2020 at 12:04 PM Thomas Preston <thomas.preston@codethink.co.uk> wrote: > - Fix a typo where mcp23x17 configs are referred to as mcp23x16. > - Fix precious range to include INTCAP{A,B}, which clear on read. > - Fix precious range to include GPIOB, which clears on read. > - Fix volatile range to include GPIOB, to fix debugfs registers > reporting different values than `gpioget gpiochip2 {0..15}`. > > Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk> Since the other two patches seem wrong, please resend this one patch, also include the people on TO: here: Andy, Phil and Jan, who all use this chip a lot. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config 2020-08-28 9:06 ` Linus Walleij @ 2020-08-28 9:28 ` Andy Shevchenko 2020-08-28 17:30 ` Thomas Preston 0 siblings, 1 reply; 12+ messages in thread From: Andy Shevchenko @ 2020-08-28 9:28 UTC (permalink / raw) To: Linus Walleij Cc: Thomas Preston, Jan Kundrát, Phil Reid, open list:GPIO SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On Fri, Aug 28, 2020 at 11:06:21AM +0200, Linus Walleij wrote: > On Fri, Aug 14, 2020 at 12:04 PM Thomas Preston > <thomas.preston@codethink.co.uk> wrote: > > > - Fix a typo where mcp23x17 configs are referred to as mcp23x16. > > - Fix precious range to include INTCAP{A,B}, which clear on read. > > - Fix precious range to include GPIOB, which clears on read. > > - Fix volatile range to include GPIOB, to fix debugfs registers > > reporting different values than `gpioget gpiochip2 {0..15}`. > > > > Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk> > > Since the other two patches seem wrong, please resend this one patch, > also include the people on TO: here: Andy, Phil and Jan, who all use > this chip a lot. And it seems it combines a lot of stuff in one patch. Can we have a split with appropriate Fixes: tags? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config 2020-08-28 9:28 ` Andy Shevchenko @ 2020-08-28 17:30 ` Thomas Preston 0 siblings, 0 replies; 12+ messages in thread From: Thomas Preston @ 2020-08-28 17:30 UTC (permalink / raw) To: Andy Shevchenko, Linus Walleij Cc: Jan Kundrát, Phil Reid, open list:GPIO SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 28/08/2020 10:28, Andy Shevchenko wrote: > On Fri, Aug 28, 2020 at 11:06:21AM +0200, Linus Walleij wrote: >> On Fri, Aug 14, 2020 at 12:04 PM Thomas Preston >> <thomas.preston@codethink.co.uk> wrote: >> >>> - Fix a typo where mcp23x17 configs are referred to as mcp23x16. >>> - Fix precious range to include INTCAP{A,B}, which clear on read. >>> - Fix precious range to include GPIOB, which clears on read. >>> - Fix volatile range to include GPIOB, to fix debugfs registers >>> reporting different values than `gpioget gpiochip2 {0..15}`. >>> >>> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk> >> >> Since the other two patches seem wrong, please resend this one patch, >> also include the people on TO: here: Andy, Phil and Jan, who all use >> this chip a lot. > > And it seems it combines a lot of stuff in one patch. Can we have a split with > appropriate Fixes: tags? > Yeah no problem, just looking at this now. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config 2020-08-14 10:03 ` [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config Thomas Preston 2020-08-28 9:06 ` Linus Walleij @ 2020-08-28 10:09 ` Andy Shevchenko 2020-08-28 19:19 ` Thomas Preston 1 sibling, 1 reply; 12+ messages in thread From: Andy Shevchenko @ 2020-08-28 10:09 UTC (permalink / raw) To: Thomas Preston Cc: Linus Walleij, Rob Herring, open list:GPIO SUBSYSTEM, devicetree, Linux Kernel Mailing List On Fri, Aug 14, 2020 at 1:35 PM Thomas Preston <thomas.preston@codethink.co.uk> wrote: > > - Fix a typo where mcp23x17 configs are referred to as mcp23x16. I'm not sure it's correct. MPC23016 is an existing I²C IO expander. > - Fix precious range to include INTCAP{A,B}, which clear on read. > - Fix precious range to include GPIOB, which clears on read. > - Fix volatile range to include GPIOB, to fix debugfs registers > reporting different values than `gpioget gpiochip2 {0..15}`. I'm wondering if you read all the datasheets before doing these changes. MPC2308 MPC23016 MPC23017 ... > -static const struct regmap_range mcp23x16_volatile_range = { > +static const struct regmap_range mcp23x17_volatile_range = { > .range_min = MCP_INTF << 1, > - .range_max = MCP_GPIO << 1, > + .range_max = (MCP_GPIO << 1) + 1, This looks weird. Usually we do a mask or a bit based mask, like (1 << x) - 1. > }; ... > +static const struct regmap_range mcp23x17_precious_range = { > + .range_min = MCP_INTCAP << 1, > + .range_max = (MCP_GPIO << 1) + 1, Ditto. > }; -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config 2020-08-28 10:09 ` Andy Shevchenko @ 2020-08-28 19:19 ` Thomas Preston 0 siblings, 0 replies; 12+ messages in thread From: Thomas Preston @ 2020-08-28 19:19 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Rob Herring, open list:GPIO SUBSYSTEM, devicetree, Linux Kernel Mailing List Hey Andy, Linus, Thanks for looking at this. On 28/08/2020 11:09, Andy Shevchenko wrote: > On Fri, Aug 14, 2020 at 1:35 PM Thomas Preston > <thomas.preston@codethink.co.uk> wrote: >> >> - Fix a typo where mcp23x17 configs are referred to as mcp23x16. > > I'm not sure it's correct. MPC23016 is an existing I²C IO expander. > The MCP23016 device is not mentioned anywhere else in this driver. The only place this string is used is in `struct regmap_config mcp23x17_regmap` (another device). It seems to me that this is a typo but I might be wrong. ~/w/linux$ git grep -h compatible drivers/pinctrl/pinctrl-mcp23s08* .compatible = "microchip,mcp23008", .compatible = "microchip,mcp23017", .compatible = "microchip,mcp23018", .compatible = "mcp,mcp23008", .compatible = "mcp,mcp23017", .compatible = "microchip,mcp23s08", .compatible = "microchip,mcp23s17", .compatible = "microchip,mcp23s18", .compatible = "mcp,mcp23s08", .compatible = "mcp,mcp23s17", Also I don't have an MC23016, so I can't test configuration for it. >> - Fix precious range to include INTCAP{A,B}, which clear on read. >> - Fix precious range to include GPIOB, which clears on read. >> - Fix volatile range to include GPIOB, to fix debugfs registers >> reporting different values than `gpioget gpiochip2 {0..15}`. > > I'm wondering if you read all the datasheets before doing these changes. > MPC2308 > MPC23016 > MPC23017 > ... > I did not! I was only changing configuration for MCP23017 devices. What have I missed? For reference, I think you are referring to [0], [1], [2]. I'm familiar with the last one. >> -static const struct regmap_range mcp23x16_volatile_range = { >> +static const struct regmap_range mcp23x17_volatile_range = { >> .range_min = MCP_INTF << 1, >> - .range_max = MCP_GPIO << 1, >> + .range_max = (MCP_GPIO << 1) + 1, > > This looks weird. Usually we do a mask or a bit based mask, like (1 << x) - 1. > I don't think these are masks, they're addresses. I believe the author has doubled the register indexing using a 1 bit shift, because the MCP23017 device is configured with sequential addresses (IOCON.BANK = 0). On page 12 of the datasheet [2] this looks like: 0x00 IODIRA, MCP_IODIR << 1 0x01 IODIRB 0x02 IPOLA, MCP_IPOL << 1 0x03 IPOLB ... 0x12 GPIOA, MCP_GPIO << 1 0x13 GPIOB This means you can read 16 bits from MCP_GPIO << 1 and get the register values for both banks, or even use this for .range_min. However, this trick doesn't work for .range_max: .range_max = MCP_GPIO << 1; /* 0x12 */ But I think it needs to be 0x13 to include GPIOB. Now that I'm looking into it, how does `mcp23x17_regmap.val_bits = 16` affect this? Perhaps `MCP_GPIO << 1` is fine after all. I will whip up a v2 and test this. I'll split the changes across patches and fix the typo last patch - in case you don't agree with me. Many thanks, Thomas [0] MCP23008 https://ww1.microchip.com/downloads/en/DeviceDoc/21919e.pdf [1] MCP23016 http://ww1.microchip.com/downloads/en/devicedoc/20090c.pdf [2] MCP23017 https://ww1.microchip.com/downloads/en/DeviceDoc/20001952C.pdf ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] pinctrl: mcp23s08: Remove interrupt-controller 2020-08-14 10:03 [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 Thomas Preston 2020-08-14 10:03 ` [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config Thomas Preston @ 2020-08-14 10:03 ` Thomas Preston 2020-08-14 10:03 ` [PATCH 3/3] devicetree: " Thomas Preston 2020-08-14 13:56 ` [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 Thomas Preston 3 siblings, 0 replies; 12+ messages in thread From: Thomas Preston @ 2020-08-14 10:03 UTC (permalink / raw) To: linus.walleij, robh+dt, linux-gpio, devicetree, linux-kernel Cc: thomas.preston The mcp23s08 device and friends are interrupt /client/ nodes, and should not reference the interrupt controller device tree property "interrupt-controller" [0]. Fix the mcp23s08 driver so that it activates interrupts when it detects the "interrupts" property instead, which is always present if we want interrupts enabled. [0] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk> --- drivers/pinctrl/pinctrl-mcp23s08.c | 7 +++---- drivers/pinctrl/pinctrl-mcp23s08.h | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c index 0138638276e7..ac8926985c28 100644 --- a/drivers/pinctrl/pinctrl-mcp23s08.c +++ b/drivers/pinctrl/pinctrl-mcp23s08.c @@ -566,9 +566,8 @@ int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev, if (ret < 0) goto fail; - mcp->irq_controller = - device_property_read_bool(dev, "interrupt-controller"); - if (mcp->irq && mcp->irq_controller) { + mcp->irq_enabled = device_property_present(dev, "interrupts"); + if (mcp->irq && mcp->irq_enabled) { mcp->irq_active_high = device_property_read_bool(dev, "microchip,irq-active-high"); @@ -601,7 +600,7 @@ int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev, goto fail; } - if (mcp->irq && mcp->irq_controller) { + if (mcp->irq && mcp->irq_enabled) { struct gpio_irq_chip *girq = &mcp->chip.irq; girq->chip = &mcp->irq_chip; diff --git a/drivers/pinctrl/pinctrl-mcp23s08.h b/drivers/pinctrl/pinctrl-mcp23s08.h index 90dc27081a3c..1aa9b11780fc 100644 --- a/drivers/pinctrl/pinctrl-mcp23s08.h +++ b/drivers/pinctrl/pinctrl-mcp23s08.h @@ -30,7 +30,7 @@ struct mcp23s08 { u16 irq_rise; u16 irq_fall; int irq; - bool irq_controller; + bool irq_enabled; int cached_gpio; /* lock protects regmap access with bypass/cache flags */ struct mutex lock; -- 2.26.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] devicetree: mcp23s08: Remove interrupt-controller 2020-08-14 10:03 [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 Thomas Preston 2020-08-14 10:03 ` [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config Thomas Preston 2020-08-14 10:03 ` [PATCH 2/3] pinctrl: mcp23s08: Remove interrupt-controller Thomas Preston @ 2020-08-14 10:03 ` Thomas Preston 2020-08-14 13:56 ` [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 Thomas Preston 3 siblings, 0 replies; 12+ messages in thread From: Thomas Preston @ 2020-08-14 10:03 UTC (permalink / raw) To: linus.walleij, robh+dt, linux-gpio, devicetree, linux-kernel Cc: thomas.preston The mcp23s08 device and friends are interrupt /client/ nodes, and should not reference the interrupt controller device tree properties "interrupt-controller" and "interrupt-cells" [0]. Remove the confusing "interrupt-controller" and "interrupt-cells" properties from the pinctrl-mcp23s08 devicetree bindings documentation. [0] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk> --- .../devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt index 8b94aa8f5971..bb1b53030552 100644 --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt @@ -43,10 +43,6 @@ Required device specific properties (only for SPI chips): - spi-max-frequency = The maximum frequency this chip is able to handle Optional properties: -- #interrupt-cells : Should be two. - - first cell is the pin number - - second cell is used to specify flags. -- interrupt-controller: Marks the device node as a interrupt controller. - drive-open-drain: Sets the ODR flag in the IOCON register. This configures the IRQ output as open drain active low. @@ -72,8 +68,6 @@ gpiom1: gpio@20 { interrupt-parent = <&gpio1>; interrupts = <17 IRQ_TYPE_LEVEL_LOW>; - interrupt-controller; - #interrupt-cells=<2>; microchip,irq-mirror; }; @@ -130,8 +124,6 @@ gpio21: gpio@21 { interrupt-parent = <&socgpio>; interrupts = <0x17 0x8>; interrupt-names = "mcp23017@21 irq"; - interrupt-controller; - #interrupt-cells = <0x2>; microchip,irq-mirror; pinctrl-names = "default"; pinctrl-0 = <&i2cgpio0irq &gpio21pullups>; -- 2.26.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 2020-08-14 10:03 [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 Thomas Preston ` (2 preceding siblings ...) 2020-08-14 10:03 ` [PATCH 3/3] devicetree: " Thomas Preston @ 2020-08-14 13:56 ` Thomas Preston 2020-08-17 19:29 ` Rob Herring 3 siblings, 1 reply; 12+ messages in thread From: Thomas Preston @ 2020-08-14 13:56 UTC (permalink / raw) To: linus.walleij, robh+dt, linux-gpio, devicetree, linux-kernel On 14/08/2020 11:03, Thomas Preston wrote: > I'm in the process of adding a device tree overlay for the PiFace > Digital Raspberry Pi daughter board [0]. It's an mcp23s17 SPI GPIO port > expander. In doing so, I noticed some errors with the mcp23s08 driver. [snip] > They're quite trivial and backwards compatible, although I might be > wrong about "interrupt-controller". Can someone please confirm? [snip] > [0] https://github.com/raspberrypi/linux/pull/3794 Actually I think I'm wrong about the interrupt-controller changes in patches 0002 and 0003. I think Patch 0001 fixups are fine still. Sorry for the noise! ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 2020-08-14 13:56 ` [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 Thomas Preston @ 2020-08-17 19:29 ` Rob Herring 2020-08-18 11:09 ` Thomas Preston 0 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2020-08-17 19:29 UTC (permalink / raw) To: Thomas Preston; +Cc: linus.walleij, linux-gpio, devicetree, linux-kernel On Fri, Aug 14, 2020 at 02:56:54PM +0100, Thomas Preston wrote: > On 14/08/2020 11:03, Thomas Preston wrote: > > I'm in the process of adding a device tree overlay for the PiFace > > Digital Raspberry Pi daughter board [0]. It's an mcp23s17 SPI GPIO port > > expander. In doing so, I noticed some errors with the mcp23s08 driver. > [snip] > > They're quite trivial and backwards compatible, although I might be > > wrong about "interrupt-controller". Can someone please confirm? > [snip] > > [0] https://github.com/raspberrypi/linux/pull/3794 > > Actually I think I'm wrong about the interrupt-controller changes in patches > 0002 and 0003. You are. Looking at the datasheet, the GPIOs have interrupt capability. GPIO controllers are typically both an interrupt client and provider. Rob ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 2020-08-17 19:29 ` Rob Herring @ 2020-08-18 11:09 ` Thomas Preston 0 siblings, 0 replies; 12+ messages in thread From: Thomas Preston @ 2020-08-18 11:09 UTC (permalink / raw) To: Rob Herring; +Cc: linus.walleij, linux-gpio, devicetree, linux-kernel On 17/08/2020 20:29, Rob Herring wrote: > On Fri, Aug 14, 2020 at 02:56:54PM +0100, Thomas Preston wrote: >> Actually I think I'm wrong about the interrupt-controller changes in patches >> 0002 and 0003. > > You are. Looking at the datasheet, the GPIOs have interrupt capability. > GPIO controllers are typically both an interrupt client and provider. Thanks for the clarification. I still think the patch 0001 is required. The precious and volatile ranges are broken. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-08-28 19:20 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-14 10:03 [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 Thomas Preston 2020-08-14 10:03 ` [PATCH 1/3] pinctrl: mcp23s08: Fixup mcp23x17 regmap_config Thomas Preston 2020-08-28 9:06 ` Linus Walleij 2020-08-28 9:28 ` Andy Shevchenko 2020-08-28 17:30 ` Thomas Preston 2020-08-28 10:09 ` Andy Shevchenko 2020-08-28 19:19 ` Thomas Preston 2020-08-14 10:03 ` [PATCH 2/3] pinctrl: mcp23s08: Remove interrupt-controller Thomas Preston 2020-08-14 10:03 ` [PATCH 3/3] devicetree: " Thomas Preston 2020-08-14 13:56 ` [PATCH 0/3] pinctrl: mcp23s08: Fixups for mcp23x17 Thomas Preston 2020-08-17 19:29 ` Rob Herring 2020-08-18 11:09 ` Thomas Preston
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).