diff mbox series

nvmem: qfprom: increase id to support multiple devices

Message ID 20200629133204.23279-1-shawn.guo@linaro.org
State New
Headers show
Series nvmem: qfprom: increase id to support multiple devices | expand

Commit Message

Shawn Guo June 29, 2020, 1:32 p.m. UTC 
There could be multiple qfprom devices on some SoCs.  For example, on
MSM8939, qfprom@58000 holds efuse bits for Core Power Reduction (CPR),
and qfprom@5c000 holds bits for TSENS.  Registering multiple nvmem
devices with the same id results in the following failure on the second
device.

[    1.682731] sysfs: cannot create duplicate filename '/bus/nvmem/devices/qfprom0'
[    1.685889] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.107-00567-g11c887ce2838-dirty #334
[    1.693191] Hardware name: Square, Inc. T2 Devkit (DT)
[    1.701610] Call trace:
[    1.706673]  dump_backtrace+0x0/0x158
[    1.708987]  show_stack+0x14/0x20
[    1.712810]  dump_stack+0x98/0xbc
[    1.716114]  sysfs_warn_dup+0x60/0x78
[    1.719401]  sysfs_do_create_link_sd.isra.0+0xdc/0xe8
[    1.723047]  sysfs_create_link+0x20/0x40
[    1.728088]  bus_add_device+0x68/0x130
[    1.732083]  device_add+0x3f8/0x628
[    1.735639]  nvmem_register.part.4+0x150/0x348
[    1.739018]  devm_nvmem_register+0x4c/0xa8
[    1.743532]  qfprom_probe+0x94/0xb8
[    1.747615]  platform_drv_probe+0x50/0xa0
[    1.750998]  really_probe+0x1b8/0x298
[    1.755164]  driver_probe_device+0x58/0x100
[    1.758810]  __driver_attach+0xe0/0xe8
[    1.762802]  bus_for_each_dev+0x74/0xc8
[    1.766622]  driver_attach+0x20/0x28
[    1.770354]  bus_add_driver+0x1ac/0x218
[    1.774175]  driver_register+0x60/0x110
[    1.777734]  __platform_driver_register+0x40/0x48
[    1.781570]  qfprom_driver_init+0x18/0x20
[    1.786416]  do_one_initcall+0x5c/0x178
[    1.790418]  kernel_init_freeable+0x198/0x244
[    1.794062]  kernel_init+0x10/0x108
[    1.798567]  ret_from_fork+0x10/0x18
[    1.802084] qcom,qfprom: probe of 5c000.qfprom failed with error -17

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

---
 drivers/nvmem/qfprom.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.17.1

Comments

Srinivas Kandagatla June 30, 2020, 9:41 a.m. UTC | #1 
Hi Shawn,
Thankyou for the patch!


On 29/06/2020 14:32, Shawn Guo wrote:
> There could be multiple qfprom devices on some SoCs.  For example, on

> MSM8939, qfprom@58000 holds efuse bits for Core Power Reduction (CPR),

> and qfprom@5c000 holds bits for TSENS.  Registering multiple nvmem

> devices with the same id results in the following failure on the second

> device.

> 

> [    1.682731] sysfs: cannot create duplicate filename '/bus/nvmem/devices/qfprom0'

> [    1.685889] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.107-00567-g11c887ce2838-dirty #334

> [    1.693191] Hardware name: Square, Inc. T2 Devkit (DT)

> [    1.701610] Call trace:

> [    1.706673]  dump_backtrace+0x0/0x158

> [    1.708987]  show_stack+0x14/0x20

> [    1.712810]  dump_stack+0x98/0xbc

> [    1.716114]  sysfs_warn_dup+0x60/0x78

> [    1.719401]  sysfs_do_create_link_sd.isra.0+0xdc/0xe8

> [    1.723047]  sysfs_create_link+0x20/0x40

> [    1.728088]  bus_add_device+0x68/0x130

> [    1.732083]  device_add+0x3f8/0x628

> [    1.735639]  nvmem_register.part.4+0x150/0x348

> [    1.739018]  devm_nvmem_register+0x4c/0xa8

> [    1.743532]  qfprom_probe+0x94/0xb8

> [    1.747615]  platform_drv_probe+0x50/0xa0

> [    1.750998]  really_probe+0x1b8/0x298

> [    1.755164]  driver_probe_device+0x58/0x100

> [    1.758810]  __driver_attach+0xe0/0xe8

> [    1.762802]  bus_for_each_dev+0x74/0xc8

> [    1.766622]  driver_attach+0x20/0x28

> [    1.770354]  bus_add_driver+0x1ac/0x218

> [    1.774175]  driver_register+0x60/0x110

> [    1.777734]  __platform_driver_register+0x40/0x48

> [    1.781570]  qfprom_driver_init+0x18/0x20

> [    1.786416]  do_one_initcall+0x5c/0x178

> [    1.790418]  kernel_init_freeable+0x198/0x244

> [    1.794062]  kernel_init+0x10/0x108

> [    1.798567]  ret_from_fork+0x10/0x18

> [    1.802084] qcom,qfprom: probe of 5c000.qfprom failed with error -17

> 



> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

> ---

>   drivers/nvmem/qfprom.c | 2 ++

>   1 file changed, 2 insertions(+)


TBH, this functionality need to go into core driver which already has 
IDR to allocated ID's for nvmem providers.

Can you try this patch:

---------------------------->cut<---------------------------------diff 
--git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 927eb5f6003f..aa87bd6415ab 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -629,12 +629,18 @@ struct nvmem_device *nvmem_register(const struct 
nvmem_config *config)
         if (!config->no_of_node)
                 nvmem->dev.of_node = config->dev->of_node;

-       if (config->id == -1 && config->name) {
+       switch (config->id) {
+       case NVMEM_DEVID_NONE:
                 dev_set_name(&nvmem->dev, "%s", config->name);
-       } else {
+               break;
+       case NVMEM_DEVID_AUTO:
+               dev_set_name(&nvmem->dev, "%s%d", config->name, nvmem->id);
+               break;
+       default:
                 dev_set_name(&nvmem->dev, "%s%d",
                              config->name ? : "nvmem",
                              config->name ? config->id : nvmem->id);
+               break;
         }

         nvmem->read_only = device_property_present(config->dev, 
"read-only") ||
diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
index 8a91717600be..8b425f8d847d 100644
--- a/drivers/nvmem/qfprom.c
+++ b/drivers/nvmem/qfprom.c
@@ -31,6 +31,7 @@ static struct nvmem_config econfig = {
         .name = "qfprom",
         .stride = 1,
         .word_size = 1,
+       .id = NVMEM_DEVID_AUTO,
         .reg_read = qfprom_reg_read,
  };

diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 6d6f8e5d24c9..06409a6c40bc 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -27,6 +27,9 @@ enum nvmem_type {
         NVMEM_TYPE_BATTERY_BACKED,
  };

+#define NVMEM_DEVID_NONE       (-1)
+#define NVMEM_DEVID_AUTO       (-2)
+
  /**
   * struct nvmem_config - NVMEM device configuration
   *
---------------------------->cut<---------------------------------

Thanks,
srini
> 

> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c

> index 8a91717600be..ceb837e861d8 100644

> --- a/drivers/nvmem/qfprom.c

> +++ b/drivers/nvmem/qfprom.c

> @@ -40,6 +40,7 @@ static int qfprom_probe(struct platform_device *pdev)

>   	struct resource *res;

>   	struct nvmem_device *nvmem;

>   	struct qfprom_priv *priv;

> +	static int id;

>   

>   	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);

>   	if (!priv)

> @@ -52,6 +53,7 @@ static int qfprom_probe(struct platform_device *pdev)

>   

>   	econfig.size = resource_size(res);

>   	econfig.dev = dev;

> +	econfig.id = id++;

>   	econfig.priv = priv;

>   

>   	nvmem = devm_nvmem_register(dev, &econfig);

>
Shawn Guo June 30, 2020, 11:18 a.m. UTC | #2 
Hi Srini,

On Tue, Jun 30, 2020 at 10:41:24AM +0100, Srinivas Kandagatla wrote:
> TBH, this functionality need to go into core driver which already has IDR to

> allocated ID's for nvmem providers.

> 

> Can you try this patch:


The patch doesn't apply to v5.8-rc, so I manually applied it for
testing. It works like a charm.

Tested-by: Shawn Guo <shawn.guo@linaro.org>


Please copy me when you post it, so that I can pick it up for my
project.  Thanks!

Shawn

> 

> ---------------------------->cut<---------------------------------diff --git

> a/drivers/nvmem/core.c b/drivers/nvmem/core.c

> index 927eb5f6003f..aa87bd6415ab 100644

> --- a/drivers/nvmem/core.c

> +++ b/drivers/nvmem/core.c

> @@ -629,12 +629,18 @@ struct nvmem_device *nvmem_register(const struct

> nvmem_config *config)

>         if (!config->no_of_node)

>                 nvmem->dev.of_node = config->dev->of_node;

> 

> -       if (config->id == -1 && config->name) {

> +       switch (config->id) {

> +       case NVMEM_DEVID_NONE:

>                 dev_set_name(&nvmem->dev, "%s", config->name);

> -       } else {

> +               break;

> +       case NVMEM_DEVID_AUTO:

> +               dev_set_name(&nvmem->dev, "%s%d", config->name, nvmem->id);

> +               break;

> +       default:

>                 dev_set_name(&nvmem->dev, "%s%d",

>                              config->name ? : "nvmem",

>                              config->name ? config->id : nvmem->id);

> +               break;

>         }

> 

>         nvmem->read_only = device_property_present(config->dev, "read-only")

> ||

> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c

> index 8a91717600be..8b425f8d847d 100644

> --- a/drivers/nvmem/qfprom.c

> +++ b/drivers/nvmem/qfprom.c

> @@ -31,6 +31,7 @@ static struct nvmem_config econfig = {

>         .name = "qfprom",

>         .stride = 1,

>         .word_size = 1,

> +       .id = NVMEM_DEVID_AUTO,

>         .reg_read = qfprom_reg_read,

>  };

> 

> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h

> index 6d6f8e5d24c9..06409a6c40bc 100644

> --- a/include/linux/nvmem-provider.h

> +++ b/include/linux/nvmem-provider.h

> @@ -27,6 +27,9 @@ enum nvmem_type {

>         NVMEM_TYPE_BATTERY_BACKED,

>  };

> 

> +#define NVMEM_DEVID_NONE       (-1)

> +#define NVMEM_DEVID_AUTO       (-2)

> +

>  /**

>   * struct nvmem_config - NVMEM device configuration

>   *

> ---------------------------->cut<---------------------------------
diff mbox series

Patch

diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
index 8a91717600be..ceb837e861d8 100644
--- a/drivers/nvmem/qfprom.c
+++ b/drivers/nvmem/qfprom.c
@@ -40,6 +40,7 @@  static int qfprom_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct nvmem_device *nvmem;
 	struct qfprom_priv *priv;
+	static int id;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -52,6 +53,7 @@  static int qfprom_probe(struct platform_device *pdev)
 
 	econfig.size = resource_size(res);
 	econfig.dev = dev;
+	econfig.id = id++;
 	econfig.priv = priv;
 
 	nvmem = devm_nvmem_register(dev, &econfig);