[PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Marco Felsch]

This patch fixes 'commit 9b3e4207661e ("pinctrl: mcp23s08: spi: Fix regmap
debugfs entries")'. For sure the MCP23S18 device has only 1 address pin
but this will get decoded into three internally ([1], section 1.4).

[1] http://ww1.microchip.com/downloads/en/DeviceDoc/22103a.pdf

Cc: stable@vger.kernel.org
Cc: Jan Kundrát <jan.kundrat@cesnet.cz>
Fixes: 9b3e4207661e ('pinctrl: mcp23s08: spi: Fix regmap debugfs entries')
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 49 ++++++++++++++----------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 4a8a8efadefa..472746931ea8 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -794,41 +794,38 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 	switch (type) {
 #ifdef CONFIG_SPI_MASTER
 	case MCP_TYPE_S08:
-	case MCP_TYPE_S17:
-		switch (type) {
-		case MCP_TYPE_S08:
-			one_regmap_config =
-				devm_kmemdup(dev, &mcp23x08_regmap,
-					sizeof(struct regmap_config), GFP_KERNEL);
-			mcp->reg_shift = 0;
-			mcp->chip.ngpio = 8;
-			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
-					"mcp23s08.%d", raw_chip_address);
-			break;
-		case MCP_TYPE_S17:
-			one_regmap_config =
-				devm_kmemdup(dev, &mcp23x17_regmap,
-					sizeof(struct regmap_config), GFP_KERNEL);
-			mcp->reg_shift = 1;
-			mcp->chip.ngpio = 16;
-			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
-					"mcp23s17.%d", raw_chip_address);
-			break;
-		}
+		one_regmap_config = devm_kmemdup(dev, &mcp23x08_regmap,
+						 sizeof(struct regmap_config),
+						 GFP_KERNEL);
 		if (!one_regmap_config)
 			return -ENOMEM;
 
-		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d", raw_chip_address);
+		mcp->reg_shift = 0;
+		mcp->chip.ngpio = 8;
+		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s08.%d",
+						 raw_chip_address);
+		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d",
+							 raw_chip_address);
 		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
 					       one_regmap_config);
 		break;
-
+	case MCP_TYPE_S17:
 	case MCP_TYPE_S18:
-		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
-					       &mcp23x17_regmap);
+		one_regmap_config = devm_kmemdup(dev, &mcp23x17_regmap,
+						 sizeof(struct regmap_config),
+						 GFP_KERNEL);
+		if (!one_regmap_config)
+			return -ENOMEM;
+
 		mcp->reg_shift = 1;
 		mcp->chip.ngpio = 16;
-		mcp->chip.label = "mcp23s18";
+		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s%s.%d",
+						 type == MCP_TYPE_S17 ?
+						 "17" : "18", raw_chip_address);
+		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d",
+							 raw_chip_address);
+		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
+					       one_regmap_config);
 		break;
 #endif /* CONFIG_SPI_MASTER */
 
-- 
2.19.0

[PATCH 2/3] pinctrl: mcp23s08: fix irq and irqchip setup order

[Marco Felsch]

Since 'commit 02e389e63e35 ("pinctrl: mcp23s08: fix irq setup order")' the
irq request isn't the last devm_* allocation. Without a deeper look at
the irq and testing this isn't a good solution. Since this driver relies
on the devm mechanism, requesting a interrupt should be the last thing
to avoid memory corruptions during unbinding.

'Commit 02e389e63e35 ("pinctrl: mcp23s08: fix irq setup order")' fixed the
order for the interrupt-controller use case only. The
mcp23s08_irq_setup() must be split into two to fix it for the
interrupt-controller use case and to register the irq at last. So the
irq will be freed first during unbind.

Cc: stable@vger.kernel.org
Cc: Dmitry Mastykin <mastichi@gmail.com>
Cc: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Fixes: 82039d244f87 ("pinctrl: mcp23s08: add pinconf support")
Fixes: 02e389e63e35 ("pinctrl: mcp23s08: fix irq setup order")
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 472746931ea8..367b648be7c7 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -636,6 +636,14 @@ static int mcp23s08_irq_setup(struct mcp23s08 *mcp)
 		return err;
 	}
 
+	return 0;
+}
+
+static int mcp23s08_irqchip_setup(struct mcp23s08 *mcp)
+{
+	struct gpio_chip *chip = &mcp->chip;
+	int err;
+
 	err =  gpiochip_irqchip_add_nested(chip,
 					   &mcp23s08_irq_chip,
 					   0,
@@ -908,8 +916,8 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 			goto fail;
 	}
 
-	if (mcp->irq && mcp->irq_controller) {
-		ret = mcp23s08_irq_setup(mcp);
+	if (mcp->irq_controller) {
+		ret = mcp23s08_irqchip_setup(mcp);
 		if (ret)
 			goto fail;
 	}
@@ -941,6 +949,9 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 		goto fail;
 	}
 
+	if (mcp->irq)
+		ret = mcp23s08_irq_setup(mcp);
+
 fail:
 	if (ret < 0)
 		dev_dbg(dev, "can't setup chip %d, --> %d\n", addr, ret);
-- 
2.19.0

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Phil Reid]

G'day Marco,

On 25/09/2018 10:56 PM, Marco Felsch wrote:
> This patch fixes 'commit 9b3e4207661e ("pinctrl: mcp23s08: spi: Fix regmap
> debugfs entries")'. For sure the MCP23S18 device has only 1 address pin
> but this will get decoded into three internally ([1], section 1.4).
> 
> [1] http://ww1.microchip.com/downloads/en/DeviceDoc/22103a.pdf

The MCP23018 (i2c) has an address pin that's decoded to 8 addresses.
The spi MCP23s18 does not from what I can see.

reference:
TABLE 1-2: SPI PINOUT DESCRIPTION (MCP23S18)

and

> 1.4 Multi-bit Address Decoder
> The ADDR pin is used to set the slave address of the
> MCP23018 (I2C only) to allow up to eight devices on
> the bus using only a

So I think the original code is correct.


> 
> Cc: stable@vger.kernel.org
> Cc: Jan Kundrát <jan.kundrat@cesnet.cz>
> Fixes: 9b3e4207661e ('pinctrl: mcp23s08: spi: Fix regmap debugfs entries')
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>   drivers/pinctrl/pinctrl-mcp23s08.c | 49 ++++++++++++++----------------
>   1 file changed, 23 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index 4a8a8efadefa..472746931ea8 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -794,41 +794,38 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>   	switch (type) {
>   #ifdef CONFIG_SPI_MASTER
>   	case MCP_TYPE_S08:
> -	case MCP_TYPE_S17:
> -		switch (type) {
> -		case MCP_TYPE_S08:
> -			one_regmap_config =
> -				devm_kmemdup(dev, &mcp23x08_regmap,
> -					sizeof(struct regmap_config), GFP_KERNEL);
> -			mcp->reg_shift = 0;
> -			mcp->chip.ngpio = 8;
> -			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
> -					"mcp23s08.%d", raw_chip_address);
> -			break;
> -		case MCP_TYPE_S17:
> -			one_regmap_config =
> -				devm_kmemdup(dev, &mcp23x17_regmap,
> -					sizeof(struct regmap_config), GFP_KERNEL);
> -			mcp->reg_shift = 1;
> -			mcp->chip.ngpio = 16;
> -			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
> -					"mcp23s17.%d", raw_chip_address);
> -			break;
> -		}
> +		one_regmap_config = devm_kmemdup(dev, &mcp23x08_regmap,
> +						 sizeof(struct regmap_config),
> +						 GFP_KERNEL);
>   		if (!one_regmap_config)
>   			return -ENOMEM;
>   
> -		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d", raw_chip_address);
> +		mcp->reg_shift = 0;
> +		mcp->chip.ngpio = 8;
> +		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s08.%d",
> +						 raw_chip_address);
> +		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d",
> +							 raw_chip_address);
>   		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
>   					       one_regmap_config);
>   		break;
> -
> +	case MCP_TYPE_S17:
>   	case MCP_TYPE_S18:
> -		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
> -					       &mcp23x17_regmap);
> +		one_regmap_config = devm_kmemdup(dev, &mcp23x17_regmap,
> +						 sizeof(struct regmap_config),
> +						 GFP_KERNEL);
> +		if (!one_regmap_config)
> +			return -ENOMEM;
> +
>   		mcp->reg_shift = 1;
>   		mcp->chip.ngpio = 16;
> -		mcp->chip.label = "mcp23s18";
> +		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s%s.%d",
> +						 type == MCP_TYPE_S17 ?
> +						 "17" : "18", raw_chip_address);
> +		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d",
> +							 raw_chip_address);
> +		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
> +					       one_regmap_config);
>   		break;
>   #endif /* CONFIG_SPI_MASTER */
>   
> 


-- 
Regards
Phil Reid

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Jan Kundrát]

On úterý 25. září 2018 16:56:14 CEST, Marco Felsch wrote:
> This patch fixes 'commit 9b3e4207661e ("pinctrl: mcp23s08: spi: Fix regmap
> debugfs entries")'. For sure the MCP23S18 device has only 1 address pin
> but this will get decoded into three internally ([1], section 1.4).
>
> [1] http://ww1.microchip.com/downloads/en/DeviceDoc/22103a.pdf

Hi Marco, Section 1.4 only applies to I2C devices. Quoting the first 
paragraph:

> The ADDR pin is used to set the slave address of the MCP23018 (I2C only) 
> to allow up to eight devices on the bus using only a single pin. 
> Typically, this would require three pins.

Here's my summary of how it works among the whole family of these chips:

- MCP23O18 is an I2C device. Its address selection is a bit unusual, but it 
can still provide 8 different addresses. In Linux, these are separate I2C 
clients as usual.

- MCP23S18, on the other hand, is an SPI device. It does not support any 
extra addressing apart from the SPI CS pin (see page 13 of the datasheet 
you linked to). As far as I can tell, it's always just one device on a 
given SPI CS signal.

- MCP23S17 is also a SPI device, but it supports a creative/hacky 
"addressing" scheme where up to eight devices can share the same SPI CS 
signal. On Linux, this opens an ugly can of warms with several "gpiochip" 
instances within the same SPI client, but that's not relevant in the 
context of this patch.

I know that the current code works with MCP23S17 -- we have a board with 
two chips with a shared SPI CS, but a different "address" prefix. Looking 
at the datasheet, MCP23S18 works in a different manner. I therefore don't 
think that this patch which effectively unifies handling of both ...S18 and 
...S17 is correct.

Can you please clarify what doesn't work for you, and how it should work? 
Did my patch 9b3e4207661e broke something?

With kind regards,
Jan

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Jan Kundrát]

On úterý 25. září 2018 16:56:14 CEST, Marco Felsch wrote:
> This patch fixes 'commit 9b3e4207661e ("pinctrl: mcp23s08: spi: Fix regmap
> debugfs entries")'. For sure the MCP23S18 device has only 1 address pin
> but this will get decoded into three internally ([1], section 1.4).
>
> [1] http://ww1.microchip.com/downloads/en/DeviceDoc/22103a.pdf

Hi Marco, Section 1.4 only applies to I2C devices. Quoting the first 
paragraph:

> The ADDR pin is used to set the slave address of the MCP23018 (I2C only) 
> to allow up to eight devices on the bus using only a single pin. 
> Typically, this would require three pins.

Here's my summary of how it works among the whole family of these chips:

- MCP23O18 is an I2C device. Its address selection is a bit unusual, but it 
can still provide 8 different addresses. In Linux, these are separate I2C 
clients as usual.

- MCP23S18, on the other hand, is an SPI device. It does not support any 
extra addressing apart from the SPI CS pin (see page 13 of the datasheet 
you linked to). As far as I can tell, it's always just one device on a 
given SPI CS signal.

- MCP23S17 is also a SPI device, but it supports a creative/hacky 
"addressing" scheme where up to eight devices can share the same SPI CS 
signal. On Linux, this opens an ugly can of warms with several "gpiochip" 
instances within the same SPI client, but that's not relevant in the 
context of this patch.

I know that the current code works with MCP23S17 -- we have a board with 
two chips with a shared SPI CS, but a different "address" prefix. Looking 
at the datasheet, MCP23S18 works in a different manner. I therefore don't 
think that this patch which effectively unifies handling of both ...S18 and 
...S17 is correct.

Can you please clarify what doesn't work for you, and how it should work? 
Did my patch 9b3e4207661e broke something?

With kind regards,
Jan

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Marco Felsch]

On 18-09-26 07:55, Phil Reid wrote:
> G'day Marco,
> 
> On 25/09/2018 10:56 PM, Marco Felsch wrote:
> > This patch fixes 'commit 9b3e4207661e ("pinctrl: mcp23s08: spi: Fix regmap
> > debugfs entries")'. For sure the MCP23S18 device has only 1 address pin
> > but this will get decoded into three internally ([1], section 1.4).
> > 
> > [1] http://ww1.microchip.com/downloads/en/DeviceDoc/22103a.pdf
> 
> The MCP23018 (i2c) has an address pin that's decoded to 8 addresses.
> The spi MCP23s18 does not from what I can see.
> 
> reference:
> TABLE 1-2: SPI PINOUT DESCRIPTION (MCP23S18)
> 
> and
> 
> > 1.4 Multi-bit Address Decoder
> > The ADDR pin is used to set the slave address of the
> > MCP23018 (I2C only) to allow up to eight devices on
> > the bus using only a
> 
> So I think the original code is correct.

Sorry, mixed up different RM and the commit message isn't correct too,
shame on me.

The patch should be smaller and should not do more than one at the same
time: fixing and uniform the code. Sorry for that.

Without the patch this would ever be true, since one_regmap_config is
initialised to NULL gets only modified in case of S08/S17.

...

case MCP_TYPE_S18:
	if (!one_regmap_config)
		return -ENOMEM;

...

This was the fixing part. The other one is to uniform the handling and
the debugfs entries.

Kind regards,
Marco

> 
> 
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Jan Kundrát <jan.kundrat@cesnet.cz>
> > Fixes: 9b3e4207661e ('pinctrl: mcp23s08: spi: Fix regmap debugfs entries')
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >   drivers/pinctrl/pinctrl-mcp23s08.c | 49 ++++++++++++++----------------
> >   1 file changed, 23 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> > index 4a8a8efadefa..472746931ea8 100644
> > --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> > +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> > @@ -794,41 +794,38 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> >   	switch (type) {
> >   #ifdef CONFIG_SPI_MASTER
> >   	case MCP_TYPE_S08:
> > -	case MCP_TYPE_S17:
> > -		switch (type) {
> > -		case MCP_TYPE_S08:
> > -			one_regmap_config =
> > -				devm_kmemdup(dev, &mcp23x08_regmap,
> > -					sizeof(struct regmap_config), GFP_KERNEL);
> > -			mcp->reg_shift = 0;
> > -			mcp->chip.ngpio = 8;
> > -			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
> > -					"mcp23s08.%d", raw_chip_address);
> > -			break;
> > -		case MCP_TYPE_S17:
> > -			one_regmap_config =
> > -				devm_kmemdup(dev, &mcp23x17_regmap,
> > -					sizeof(struct regmap_config), GFP_KERNEL);
> > -			mcp->reg_shift = 1;
> > -			mcp->chip.ngpio = 16;
> > -			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
> > -					"mcp23s17.%d", raw_chip_address);
> > -			break;
> > -		}
> > +		one_regmap_config = devm_kmemdup(dev, &mcp23x08_regmap,
> > +						 sizeof(struct regmap_config),
> > +						 GFP_KERNEL);
> >   		if (!one_regmap_config)
> >   			return -ENOMEM;
> > -		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d", raw_chip_address);
> > +		mcp->reg_shift = 0;
> > +		mcp->chip.ngpio = 8;
> > +		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s08.%d",
> > +						 raw_chip_address);
> > +		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d",
> > +							 raw_chip_address);
> >   		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
> >   					       one_regmap_config);
> >   		break;
> > -
> > +	case MCP_TYPE_S17:
> >   	case MCP_TYPE_S18:
> > -		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
> > -					       &mcp23x17_regmap);
> > +		one_regmap_config = devm_kmemdup(dev, &mcp23x17_regmap,
> > +						 sizeof(struct regmap_config),
> > +						 GFP_KERNEL);
> > +		if (!one_regmap_config)
> > +			return -ENOMEM;
> > +
> >   		mcp->reg_shift = 1;
> >   		mcp->chip.ngpio = 16;
> > -		mcp->chip.label = "mcp23s18";
> > +		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s%s.%d",
> > +						 type == MCP_TYPE_S17 ?
> > +						 "17" : "18", raw_chip_address);
> > +		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d",
> > +							 raw_chip_address);
> > +		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
> > +					       one_regmap_config);
> >   		break;
> >   #endif /* CONFIG_SPI_MASTER */
> > 
> 
> 
> -- 
> Regards
> Phil Reid
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Marco Felsch]

On 18-09-26 07:55, Phil Reid wrote:
> G'day Marco,
> 
> On 25/09/2018 10:56 PM, Marco Felsch wrote:
> > This patch fixes 'commit 9b3e4207661e ("pinctrl: mcp23s08: spi: Fix regmap
> > debugfs entries")'. For sure the MCP23S18 device has only 1 address pin
> > but this will get decoded into three internally ([1], section 1.4).
> > 
> > [1] http://ww1.microchip.com/downloads/en/DeviceDoc/22103a.pdf
> 
> The MCP23018 (i2c) has an address pin that's decoded to 8 addresses.
> The spi MCP23s18 does not from what I can see.
> 
> reference:
> TABLE 1-2: SPI PINOUT DESCRIPTION (MCP23S18)
> 
> and
> 
> > 1.4 Multi-bit Address Decoder
> > The ADDR pin is used to set the slave address of the
> > MCP23018 (I2C only) to allow up to eight devices on
> > the bus using only a
> 
> So I think the original code is correct.

Sorry, mixed up different RM and the commit message isn't correct too,
shame on me.

The patch should be smaller and should not do more than one at the same
time: fixing and uniform the code. Sorry for that.

Without the patch this would ever be true, since one_regmap_config is
initialised to NULL gets only modified in case of S08/S17.

...

case MCP_TYPE_S18:
	if (!one_regmap_config)
		return -ENOMEM;

...

This was the fixing part. The other one is to uniform the handling and
the debugfs entries.

Kind regards,
Marco

> 
> 
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Jan Kundr�t <jan.kundrat@cesnet.cz>
> > Fixes: 9b3e4207661e ('pinctrl: mcp23s08: spi: Fix regmap debugfs entries')
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >   drivers/pinctrl/pinctrl-mcp23s08.c | 49 ++++++++++++++----------------
> >   1 file changed, 23 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> > index 4a8a8efadefa..472746931ea8 100644
> > --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> > +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> > @@ -794,41 +794,38 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> >   	switch (type) {
> >   #ifdef CONFIG_SPI_MASTER
> >   	case MCP_TYPE_S08:
> > -	case MCP_TYPE_S17:
> > -		switch (type) {
> > -		case MCP_TYPE_S08:
> > -			one_regmap_config =
> > -				devm_kmemdup(dev, &mcp23x08_regmap,
> > -					sizeof(struct regmap_config), GFP_KERNEL);
> > -			mcp->reg_shift = 0;
> > -			mcp->chip.ngpio = 8;
> > -			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
> > -					"mcp23s08.%d", raw_chip_address);
> > -			break;
> > -		case MCP_TYPE_S17:
> > -			one_regmap_config =
> > -				devm_kmemdup(dev, &mcp23x17_regmap,
> > -					sizeof(struct regmap_config), GFP_KERNEL);
> > -			mcp->reg_shift = 1;
> > -			mcp->chip.ngpio = 16;
> > -			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
> > -					"mcp23s17.%d", raw_chip_address);
> > -			break;
> > -		}
> > +		one_regmap_config = devm_kmemdup(dev, &mcp23x08_regmap,
> > +						 sizeof(struct regmap_config),
> > +						 GFP_KERNEL);
> >   		if (!one_regmap_config)
> >   			return -ENOMEM;
> > -		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d", raw_chip_address);
> > +		mcp->reg_shift = 0;
> > +		mcp->chip.ngpio = 8;
> > +		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s08.%d",
> > +						 raw_chip_address);
> > +		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d",
> > +							 raw_chip_address);
> >   		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
> >   					       one_regmap_config);
> >   		break;
> > -
> > +	case MCP_TYPE_S17:
> >   	case MCP_TYPE_S18:
> > -		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
> > -					       &mcp23x17_regmap);
> > +		one_regmap_config = devm_kmemdup(dev, &mcp23x17_regmap,
> > +						 sizeof(struct regmap_config),
> > +						 GFP_KERNEL);
> > +		if (!one_regmap_config)
> > +			return -ENOMEM;
> > +
> >   		mcp->reg_shift = 1;
> >   		mcp->chip.ngpio = 16;
> > -		mcp->chip.label = "mcp23s18";
> > +		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s%s.%d",
> > +						 type == MCP_TYPE_S17 ?
> > +						 "17" : "18", raw_chip_address);
> > +		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d",
> > +							 raw_chip_address);
> > +		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
> > +					       one_regmap_config);
> >   		break;
> >   #endif /* CONFIG_SPI_MASTER */
> > 
> 
> 
> -- 
> Regards
> Phil Reid
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Marco Felsch]

On 18-09-26 09:39, Jan Kundrát wrote:
> On úterý 25. září 2018 16:56:14 CEST, Marco Felsch wrote:
> > This patch fixes 'commit 9b3e4207661e ("pinctrl: mcp23s08: spi: Fix regmap
> > debugfs entries")'. For sure the MCP23S18 device has only 1 address pin
> > but this will get decoded into three internally ([1], section 1.4).
> > 
> > [1] http://ww1.microchip.com/downloads/en/DeviceDoc/22103a.pdf
> 
> Hi Marco, Section 1.4 only applies to I2C devices. Quoting the first
> paragraph:
> 
> > The ADDR pin is used to set the slave address of the MCP23018 (I2C only)
> > to allow up to eight devices on the bus using only a single pin.
> > Typically, this would require three pins.

Hi Jan,

sorry I mixed up different versions of the MCP RMs.

> 
> Here's my summary of how it works among the whole family of these chips:
> 
> - MCP23O18 is an I2C device. Its address selection is a bit unusual, but it
> can still provide 8 different addresses. In Linux, these are separate I2C
> clients as usual.
> 
> - MCP23S18, on the other hand, is an SPI device. It does not support any
> extra addressing apart from the SPI CS pin (see page 13 of the datasheet you
> linked to). As far as I can tell, it's always just one device on a given SPI
> CS signal.
> 
> - MCP23S17 is also a SPI device, but it supports a creative/hacky
> "addressing" scheme where up to eight devices can share the same SPI CS
> signal. On Linux, this opens an ugly can of warms with several "gpiochip"
> instances within the same SPI client, but that's not relevant in the context
> of this patch.
> 
> I know that the current code works with MCP23S17 -- we have a board with two
> chips with a shared SPI CS, but a different "address" prefix. Looking at the
> datasheet, MCP23S18 works in a different manner. I therefore don't think
> that this patch which effectively unifies handling of both ...S18 and ...S17
> is correct.
> 
> Can you please clarify what doesn't work for you, and how it should work?
> Did my patch 9b3e4207661e broke something?

Sorry as I mentioned to Phil I mixed up different things: fixing and
uniforming. The fixing patch should only remove this:

8<---

case MCP_TYPE_S18:
-	if (!one_regmap_config)
-		return -ENOMEM;

8<---

Or is there a corner point I didn't saw?

Kind Regards,
Marco

> 
> With kind regards,
> Jan
> 

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Phil Reid]

On 26/09/2018 5:42 PM, Marco Felsch wrote:
> On 18-09-26 07:55, Phil Reid wrote:
>> G'day Marco,
>>
>> On 25/09/2018 10:56 PM, Marco Felsch wrote:
>>> This patch fixes 'commit 9b3e4207661e ("pinctrl: mcp23s08: spi: Fix regmap
>>> debugfs entries")'. For sure the MCP23S18 device has only 1 address pin
>>> but this will get decoded into three internally ([1], section 1.4).
>>>
>>> [1]http://ww1.microchip.com/downloads/en/DeviceDoc/22103a.pdf
>> The MCP23018 (i2c) has an address pin that's decoded to 8 addresses.
>> The spi MCP23s18 does not from what I can see.
>>
>> reference:
>> TABLE 1-2: SPI PINOUT DESCRIPTION (MCP23S18)
>>
>> and
>>
>>> 1.4 Multi-bit Address Decoder
>>> The ADDR pin is used to set the slave address of the
>>> MCP23018 (I2C only) to allow up to eight devices on
>>> the bus using only a
>> So I think the original code is correct.
> Sorry, mixed up different RM and the commit message isn't correct too,
> shame on me.
> 
> The patch should be smaller and should not do more than one at the same
> time: fixing and uniform the code. Sorry for that.
> 
> Without the patch this would ever be true, since one_regmap_config is
> initialised to NULL gets only modified in case of S08/S17.
> 
> ...
> 
> case MCP_TYPE_S18:
> 	if (!one_regmap_config)
> 		return -ENOMEM;
> 
> ...
> 
> This was the fixing part. The other one is to uniform the handling and
> the debugfs entries.
G'day Marco,

Sorry I still don't follow.
I've got a mcp23s18 in one of my systems and debugfs seems to work fine.

one_regmap_config is only used for the S08/S17 to fiddle with the name / label



-- 
Regards
Phil Reid

ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au

3 The Avenue, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: preid@electromag.com.au

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Marco Felsch]

Hi Phil,

On 18-09-27 09:45, Phil Reid wrote:
> On 26/09/2018 5:42 PM, Marco Felsch wrote:
> > On 18-09-26 07:55, Phil Reid wrote:
> > > G'day Marco,
> > > 
> > > On 25/09/2018 10:56 PM, Marco Felsch wrote:
> > > > This patch fixes 'commit 9b3e4207661e ("pinctrl: mcp23s08: spi: Fix regmap
> > > > debugfs entries")'. For sure the MCP23S18 device has only 1 address pin
> > > > but this will get decoded into three internally ([1], section 1.4).
> > > > 
> > > > [1]http://ww1.microchip.com/downloads/en/DeviceDoc/22103a.pdf
> > > The MCP23018 (i2c) has an address pin that's decoded to 8 addresses.
> > > The spi MCP23s18 does not from what I can see.
> > > 
> > > reference:
> > > TABLE 1-2: SPI PINOUT DESCRIPTION (MCP23S18)
> > > 
> > > and
> > > 
> > > > 1.4 Multi-bit Address Decoder
> > > > The ADDR pin is used to set the slave address of the
> > > > MCP23018 (I2C only) to allow up to eight devices on
> > > > the bus using only a
> > > So I think the original code is correct.
> > Sorry, mixed up different RM and the commit message isn't correct too,
> > shame on me.
> > 
> > The patch should be smaller and should not do more than one at the same
> > time: fixing and uniform the code. Sorry for that.
> > 
> > Without the patch this would ever be true, since one_regmap_config is
> > initialised to NULL gets only modified in case of S08/S17.
> > 
> > ...
> > 
> > case MCP_TYPE_S18:
> > 	if (!one_regmap_config)
> > 		return -ENOMEM;
> > 
> > ...
> > 
> > This was the fixing part. The other one is to uniform the handling and
> > the debugfs entries.
> G'day Marco,
> 
> Sorry I still don't follow.
> I've got a mcp23s18 in one of my systems and debugfs seems to work fine.
> 
> one_regmap_config is only used for the S08/S17 to fiddle with the name / label

I know, but if I read the code correctly it shouldn't work, because in
your case one_regmap_config is never modified, as you told. But it is
initialized to NULL. This confuses me a bit.

This tested should not be made for the MCP_TYPE_S18 case.

Kind regards,
Marco

> 
> 
> 
> -- 
> Regards
> Phil Reid
> 
> ElectroMagnetic Imaging Technology Pty Ltd
> Development of Geophysical Instrumentation & Software
> www.electromag.com.au
> 
> 3 The Avenue, Midland WA 6056, AUSTRALIA
> Ph: +61 8 9250 8100
> Fax: +61 8 9250 7100
> Email: preid@electromag.com.au
> 

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Jan Kundrát]

On čtvrtek 27. září 2018 10:55:11 CEST, Marco Felsch wrote:
> Hi Phil,
>
> On 18-09-27 09:45, Phil Reid wrote:
>> On 26/09/2018 5:42 PM, Marco Felsch wrote:
>>> On 18-09-26 07:55, Phil Reid wrote: ...
>> G'day Marco,
>> 
>> Sorry I still don't follow.
>> I've got a mcp23s18 in one of my systems and debugfs seems to work fine.
>> 
>> one_regmap_config is only used for the S08/S17 to fiddle with 
>> the name / label
>
> I know, but if I read the code correctly it shouldn't work, because in
> your case one_regmap_config is never modified, as you told. But it is
> initialized to NULL. This confuses me a bit.
>
> This tested should not be made for the MCP_TYPE_S18 case.

Ah, right. This means that the stable tree(s) need this follow-up patch 
from February, too:

  https://patchwork.ozlabs.org/patch/875045/

Thanks for catching this.

With kind regards,
Jan

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Jan Kundrát]

On čtvrtek 27. září 2018 10:55:11 CEST, Marco Felsch wrote:
> Hi Phil,
>
> On 18-09-27 09:45, Phil Reid wrote:
>> On 26/09/2018 5:42 PM, Marco Felsch wrote:
>>> On 18-09-26 07:55, Phil Reid wrote: ...
>> G'day Marco,
>> 
>> Sorry I still don't follow.
>> I've got a mcp23s18 in one of my systems and debugfs seems to work fine.
>> 
>> one_regmap_config is only used for the S08/S17 to fiddle with 
>> the name / label
>
> I know, but if I read the code correctly it shouldn't work, because in
> your case one_regmap_config is never modified, as you told. But it is
> initialized to NULL. This confuses me a bit.
>
> This tested should not be made for the MCP_TYPE_S18 case.

Ah, right. This means that the stable tree(s) need this follow-up patch 
from February, too:

  https://patchwork.ozlabs.org/patch/875045/

Thanks for catching this.

With kind regards,
Jan

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Marco Felsch]

On 18-09-27 12:54, Jan Kundrát wrote:
> On čtvrtek 27. září 2018 10:55:11 CEST, Marco Felsch wrote:
> > Hi Phil,
> > 
> > On 18-09-27 09:45, Phil Reid wrote:
> > > On 26/09/2018 5:42 PM, Marco Felsch wrote:
> > > > On 18-09-26 07:55, Phil Reid wrote: ...
> > > G'day Marco,
> > > 
> > > Sorry I still don't follow.
> > > I've got a mcp23s18 in one of my systems and debugfs seems to work fine.
> > > 
> > > one_regmap_config is only used for the S08/S17 to fiddle with the
> > > name / label
> > 
> > I know, but if I read the code correctly it shouldn't work, because in
> > your case one_regmap_config is never modified, as you told. But it is
> > initialized to NULL. This confuses me a bit.
> > 
> > This tested should not be made for the MCP_TYPE_S18 case.

Hi Jan,

> 
> Ah, right. This means that the stable tree(s) need this follow-up patch from
> February, too:
> 
>  https://patchwork.ozlabs.org/patch/875045/
> 
> Thanks for catching this.

Okay, so we can drop this patch too. Can you give me some feedback for
patch "pinctrl: mcp23s08: fix irq and irqchip setup order"?

Kind regards,
Marco

> 
> With kind regards,
> Jan
> 
> 

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Phil Reid]

On 27/09/2018 6:54 PM, Jan Kundrát wrote:
> On čtvrtek 27. září 2018 10:55:11 CEST, Marco Felsch wrote:
>> Hi Phil,
>>
>> On 18-09-27 09:45, Phil Reid wrote:
>>> On 26/09/2018 5:42 PM, Marco Felsch wrote:
>>>> On 18-09-26 07:55, Phil Reid wrote: ...
>>> G'day Marco,
>>>
>>> Sorry I still don't follow.
>>> I've got a mcp23s18 in one of my systems and debugfs seems to work fine.
>>>
>>> one_regmap_config is only used for the S08/S17 to fiddle with the name / label
>>
>> I know, but if I read the code correctly it shouldn't work, because in
>> your case one_regmap_config is never modified, as you told. But it is
>> initialized to NULL. This confuses me a bit.
>>
>> This tested should not be made for the MCP_TYPE_S18 case.
> 
> Ah, right. This means that the stable tree(s) need this follow-up patch from February, too:
> 
>   https://patchwork.ozlabs.org/patch/875045/
> 
> Thanks for catching this.
> 
I'd forgotten about that patch.


-- 
Regards
Phil Reid

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Jan Kundrát]

On čtvrtek 27. září 2018 14:31:31 CEST, Marco Felsch wrote:
> Can you give me some feedback for
> patch "pinctrl: mcp23s08: fix irq and irqchip setup order"?

I don't understand kernel's IRQ subsystem, so I cannot review correctness 
of that one, sorry.

Do you want me to test it (just the default path, I cannot easily trigger 
the error-handling one)?

Cheers,
Jan

Re: [PATCH 1/3] pinctrl: mcp23s08: fix debugfs entry for mcp23s18 device

[Jan Kundrát]

On čtvrtek 27. září 2018 14:31:31 CEST, Marco Felsch wrote:
> Can you give me some feedback for
> patch "pinctrl: mcp23s08: fix irq and irqchip setup order"?

I don't understand kernel's IRQ subsystem, so I cannot review correctness 
of that one, sorry.

Do you want me to test it (just the default path, I cannot easily trigger 
the error-handling one)?

Cheers,
Jan

Re: [PATCH 2/3] pinctrl: mcp23s08: fix irq and irqchip setup order

[Phil Reid]

G'day Marco,


On 25/09/2018 10:56 PM, Marco Felsch wrote:
> Since 'commit 02e389e63e35 ("pinctrl: mcp23s08: fix irq setup order")' the
> irq request isn't the last devm_* allocation. Without a deeper look at
> the irq and testing this isn't a good solution. Since this driver relies
> on the devm mechanism, requesting a interrupt should be the last thing
> to avoid memory corruptions during unbinding.
> 
> 'Commit 02e389e63e35 ("pinctrl: mcp23s08: fix irq setup order")' fixed the
> order for the interrupt-controller use case only. The
> mcp23s08_irq_setup() must be split into two to fix it for the
> interrupt-controller use case and to register the irq at last. So the
> irq will be freed first during unbind.
> 

I'm no expert on the irq's, but after a bit of reading the patch makes sense to me.
I've got one question below.

> Cc: stable@vger.kernel.org
> Cc: Dmitry Mastykin <mastichi@gmail.com>
> Cc: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> Fixes: 82039d244f87 ("pinctrl: mcp23s08: add pinconf support")
> Fixes: 02e389e63e35 ("pinctrl: mcp23s08: fix irq setup order")
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>   drivers/pinctrl/pinctrl-mcp23s08.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index 472746931ea8..367b648be7c7 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -636,6 +636,14 @@ static int mcp23s08_irq_setup(struct mcp23s08 *mcp)
>   		return err;
>   	}
>   
> +	return 0;
> +}
> +
> +static int mcp23s08_irqchip_setup(struct mcp23s08 *mcp)
> +{
> +	struct gpio_chip *chip = &mcp->chip;
> +	int err;
> +
>   	err =  gpiochip_irqchip_add_nested(chip,
>   					   &mcp23s08_irq_chip,
>   					   0,
> @@ -908,8 +916,8 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>   			goto fail;
>   	}
>   
> -	if (mcp->irq && mcp->irq_controller) {
> -		ret = mcp23s08_irq_setup(mcp);
> +	if (mcp->irq_controller) {
> +		ret = mcp23s08_irqchip_setup(mcp);
The condition check changes, which may make sense to someone more knowledgeable.,
but gpiochip_set_nested_irqchip in mcp23s08_irqchip_setup references mcp->irq as well
so I'm a little confused.
>   		if (ret)
>   			goto fail;
>   	}
> @@ -941,6 +949,9 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>   		goto fail;
>   	}
>   
> +	if (mcp->irq)
> +		ret = mcp23s08_irq_setup(mcp);
> +
Is there any point if it's not a irq_controller?


>   fail:
>   	if (ret < 0)
>   		dev_dbg(dev, "can't setup chip %d, --> %d\n", addr, ret);
> 

Otherwise LGTM.

-- 
Regards
Phil Reid

Re: [PATCH 2/3] pinctrl: mcp23s08: fix irq and irqchip setup order

[Marco Felsch]

Hi Phil,

On 18-09-27 21:05, Phil Reid wrote:
> G'day Marco,
> 
> 
> On 25/09/2018 10:56 PM, Marco Felsch wrote:
> > Since 'commit 02e389e63e35 ("pinctrl: mcp23s08: fix irq setup order")' the
> > irq request isn't the last devm_* allocation. Without a deeper look at
> > the irq and testing this isn't a good solution. Since this driver relies
> > on the devm mechanism, requesting a interrupt should be the last thing
> > to avoid memory corruptions during unbinding.
> > 
> > 'Commit 02e389e63e35 ("pinctrl: mcp23s08: fix irq setup order")' fixed the
> > order for the interrupt-controller use case only. The
> > mcp23s08_irq_setup() must be split into two to fix it for the
> > interrupt-controller use case and to register the irq at last. So the
> > irq will be freed first during unbind.
> > 
> 
> I'm no expert on the irq's, but after a bit of reading the patch makes sense to me.
> I've got one question below.

Thanks for review.

> 
> > Cc: stable@vger.kernel.org
> > Cc: Dmitry Mastykin <mastichi@gmail.com>
> > Cc: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > Fixes: 82039d244f87 ("pinctrl: mcp23s08: add pinconf support")
> > Fixes: 02e389e63e35 ("pinctrl: mcp23s08: fix irq setup order")
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >   drivers/pinctrl/pinctrl-mcp23s08.c | 15 +++++++++++++--
> >   1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> > index 472746931ea8..367b648be7c7 100644
> > --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> > +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> > @@ -636,6 +636,14 @@ static int mcp23s08_irq_setup(struct mcp23s08 *mcp)
> >   		return err;
> >   	}
> > +	return 0;
> > +}
> > +
> > +static int mcp23s08_irqchip_setup(struct mcp23s08 *mcp)
> > +{
> > +	struct gpio_chip *chip = &mcp->chip;
> > +	int err;
> > +
> >   	err =  gpiochip_irqchip_add_nested(chip,
> >   					   &mcp23s08_irq_chip,
> >   					   0,
> > @@ -908,8 +916,8 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> >   			goto fail;
> >   	}
> > -	if (mcp->irq && mcp->irq_controller) {
> > -		ret = mcp23s08_irq_setup(mcp);
> > +	if (mcp->irq_controller) {
> > +		ret = mcp23s08_irqchip_setup(mcp);
> The condition check changes, which may make sense to someone more knowledgeable.,
> but gpiochip_set_nested_irqchip in mcp23s08_irqchip_setup references mcp->irq as well
> so I'm a little confused.

You're right, it make no sense to setup a irqchip without having a
upstream irq. I will fix this for a v2.

> >   		if (ret)
> >   			goto fail;
> >   	}
> > @@ -941,6 +949,9 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> >   		goto fail;
> >   	}
> > +	if (mcp->irq)
> > +		ret = mcp23s08_irq_setup(mcp);
> > +
> Is there any point if it's not a irq_controller?

AFAK, if it is a irqchip/irq_controller it can be used as a irq-parent for
other devices. The irq itself can be setup without irq_controller must
be set. E.g. Inform the System about a changed gpio state.

Kind regards,
Marco
 
> 
> >   fail:
> >   	if (ret < 0)
> >   		dev_dbg(dev, "can't setup chip %d, --> %d\n", addr, ret);
> > 
> 
> Otherwise LGTM.
> 
> -- 
> Regards
> Phil Reid
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |