[2/4] pinctrl: mcp23s08: spi: Fix regmap debugfs entries

Message ID de8fbb7902652b5a512f67e2ee0813571e9d2372.1516997103.git.jan.kundrat@cesnet.cz
State New
Headers show
Series
  • Improvements for MCP23Sxx chips
Related show

Commit Message

Jan Kundrát Jan. 25, 2018, 5:29 p.m.
The SPI version of this chip allows several devices to be present on the
same SPI bus via a local address. If this is in action and if the kernel
has debugfs, however, the code attempts to create duplicate entries for
the regmap's debugfs:

  mcp23s08 spi1.1: Failed to create debugfs directory

This patch simply assigns a local name matching the device logical
address to the `struct regmap_config`.

No changes are needed for MCP23S18 because that device does not support
any logical addressing. Similarly, I2C devices do not need any action,
either, because they are already different in their I2C address.

A similar problem is present for the pinctrl debugfs instance, but that
one is not addressed by this patch.

Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

Comments

Linus Walleij Feb. 7, 2018, 1:43 p.m. | #1
On Thu, Jan 25, 2018 at 6:29 PM, Jan Kundrát <jan.kundrat@cesnet.cz> wrote:

> The SPI version of this chip allows several devices to be present on the
> same SPI bus via a local address. If this is in action and if the kernel
> has debugfs, however, the code attempts to create duplicate entries for
> the regmap's debugfs:
>
>   mcp23s08 spi1.1: Failed to create debugfs directory
>
> This patch simply assigns a local name matching the device logical
> address to the `struct regmap_config`.
>
> No changes are needed for MCP23S18 because that device does not support
> any logical addressing. Similarly, I2C devices do not need any action,
> either, because they are already different in their I2C address.
>
> A similar problem is present for the pinctrl debugfs instance, but that
> one is not addressed by this patch.
>
> Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz>

This looks good and straight-forward.

Patch applied for v4.17.

Hope it doesn't clash with Phil et al's parallel developments.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Phil Reid Feb. 19, 2018, 8:57 a.m. | #2
G'day Jan / Linus,

I was just trying to submit the open drain stuff but having a problem with this patch.
See below.


On 26/01/2018 01:29, Jan Kundrát wrote:
> The SPI version of this chip allows several devices to be present on the
> same SPI bus via a local address. If this is in action and if the kernel
> has debugfs, however, the code attempts to create duplicate entries for
> the regmap's debugfs:
> 
>    mcp23s08 spi1.1: Failed to create debugfs directory
> 
> This patch simply assigns a local name matching the device logical
> address to the `struct regmap_config`.
> 
> No changes are needed for MCP23S18 because that device does not support
> any logical addressing. Similarly, I2C devices do not need any action,
> either, because they are already different in their I2C address.
> 
> A similar problem is present for the pinctrl debugfs instance, but that
> one is not addressed by this patch.
> 
> Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz>
> ---
>   drivers/pinctrl/pinctrl-mcp23s08.c | 37 ++++++++++++++++++++++++++-----------
>   1 file changed, 26 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index 186101c575e3..8dc758bacdd1 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -788,6 +788,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>   {
>   	int status, ret;
>   	bool mirror = false;
> +	struct regmap_config *one_regmap_config = NULL;
>   
>   	mutex_init(&mcp->lock);
>   
> @@ -808,22 +809,36 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>   	switch (type) {
>   #ifdef CONFIG_SPI_MASTER
>   	case MCP_TYPE_S08:
> -		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
> -					       &mcp23x08_regmap);
> -		mcp->reg_shift = 0;
> -		mcp->chip.ngpio = 8;
> -		mcp->chip.label = "mcp23s08";
> -		break;
> -
>   	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 = "mcp23s08";
> +			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 = "mcp23s17";
> +			break;
> +		}
> +		if (!one_regmap_config)
> +			return -ENOMEM;
> +
> +		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d", (addr & ~0x40) >> 1);
>   		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
> -					       &mcp23x17_regmap);
> -		mcp->reg_shift = 1;
> -		mcp->chip.ngpio = 16;
> -		mcp->chip.label = "mcp23s17";
> +					       one_regmap_config);
>   		break;
>   
>   	case MCP_TYPE_S18:
> +		if (!one_regmap_config)
> +			return -ENOMEM;

one_regmap_config is always null here.
So probe is always failing with ENOMEM. or am I missing something else?



>   		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
>   					       &mcp23x17_regmap);
>   		mcp->reg_shift = 1;
>

Patch

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 186101c575e3..8dc758bacdd1 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -788,6 +788,7 @@  static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 {
 	int status, ret;
 	bool mirror = false;
+	struct regmap_config *one_regmap_config = NULL;
 
 	mutex_init(&mcp->lock);
 
@@ -808,22 +809,36 @@  static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 	switch (type) {
 #ifdef CONFIG_SPI_MASTER
 	case MCP_TYPE_S08:
-		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
-					       &mcp23x08_regmap);
-		mcp->reg_shift = 0;
-		mcp->chip.ngpio = 8;
-		mcp->chip.label = "mcp23s08";
-		break;
-
 	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 = "mcp23s08";
+			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 = "mcp23s17";
+			break;
+		}
+		if (!one_regmap_config)
+			return -ENOMEM;
+
+		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d", (addr & ~0x40) >> 1);
 		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
-					       &mcp23x17_regmap);
-		mcp->reg_shift = 1;
-		mcp->chip.ngpio = 16;
-		mcp->chip.label = "mcp23s17";
+					       one_regmap_config);
 		break;
 
 	case MCP_TYPE_S18:
+		if (!one_regmap_config)
+			return -ENOMEM;
 		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
 					       &mcp23x17_regmap);
 		mcp->reg_shift = 1;