diff mbox series

Revert "nvmem: add new config option"

Message ID	20230718084804.20139-1-zajec5@gmail.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: =?utf-8?b?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com> To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Cc: Miquel Raynal <miquel.raynal@bootlin.com>, Richard Weinberger <richard@nod.at>, Vignesh Raghavendra <vigneshr@ti.com>, Bartosz Golaszewski <bgolaszewski@baylibre.com>, Alban Bedel <albeu@free.fr>, Christian Marangi <ansuelsmth@gmail.com>, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, =?utf-8?b?UmFmYcWCIE1pxYJlY2tp?= <rafal@milecki.pl> Subject: [PATCH] Revert "nvmem: add new config option" Date: Tue, 18 Jul 2023 10:48:04 +0200 Message-Id: <20230718084804.20139-1-zajec5@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	Revert "nvmem: add new config option" \| Revert "nvmem: add new config option"

Commit Message

Rafał Miłecki July 18, 2023, 8:48 a.m. UTC

  From: Rafał Miłecki <rafal@milecki.pl>

This reverts commit 517f14d9cf3533d5ab4fded195ab6f80a92e378f.

It seems that "no_of_node" config option was added to help mtd's case.

DT nodes of MTD partitions (that are also NVMEM devices) may contain
subnodes that SHOULD NOT be treated as NVMEM fixed cells. To prevent
NVMEM core code from parsing them "no_of_node" was set to true and that
made for_each_child_of_node() in NVMEM a no-op.

With the introduction of "add_legacy_fixed_of_cells" config option
things got more explicit. MTD subsystem simply tells NVMEM when to look
for fixed cells and there is no need to hack "of_node" pointer anymore.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
Important: this is based on top of the
[PATCH V4] nvmem: add explicit config option to read old syntax fixed OF cells
---
 drivers/mtd/mtdcore.c          | 1 -
 drivers/nvmem/core.c           | 2 +-
 include/linux/nvmem-provider.h | 2 --
 3 files changed, 1 insertion(+), 4 deletions(-)

Comments

Miquel Raynal July 18, 2023, 9:19 a.m. UTC | #1

Hi Rafał,

zajec5@gmail.com wrote on Tue, 18 Jul 2023 10:48:04 +0200:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This reverts commit 517f14d9cf3533d5ab4fded195ab6f80a92e378f.
> 
> It seems that "no_of_node" config option was added to help mtd's case.
> 
> DT nodes of MTD partitions (that are also NVMEM devices) may contain
> subnodes that SHOULD NOT be treated as NVMEM fixed cells. To prevent
> NVMEM core code from parsing them "no_of_node" was set to true and that
> made for_each_child_of_node() in NVMEM a no-op.
> 
> With the introduction of "add_legacy_fixed_of_cells" config option
> things got more explicit. MTD subsystem simply tells NVMEM when to look
> for fixed cells and there is no need to hack "of_node" pointer anymore.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

kernel test robot July 18, 2023, 4 p.m. UTC | #2

Hi Rafał,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mtd/mtd/next]
[also build test WARNING on mtd/mtd/fixes linus/master v6.5-rc2 next-20230718]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Rafa-Mi-ecki/Revert-nvmem-add-new-config-option/20230718-170441
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next
patch link:    https://lore.kernel.org/r/20230718084804.20139-1-zajec5%40gmail.com
patch subject: [PATCH] Revert "nvmem: add new config option"
config: i386-randconfig-i002-20230718 (https://download.01.org/0day-ci/archive/20230718/202307182316.AVNL1wNs-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230718/202307182316.AVNL1wNs-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307182316.AVNL1wNs-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/mtd/mtdcore.c: In function 'mtd_nvmem_add':
>> drivers/mtd/mtdcore.c:542:29: warning: unused variable 'node' [-Wunused-variable]
     542 |         struct device_node *node = mtd_get_of_node(mtd);
         |                             ^~~~


vim +/node +542 drivers/mtd/mtdcore.c

c4dfa25ab307a2 Alban Bedel        2018-11-13  539  
c4dfa25ab307a2 Alban Bedel        2018-11-13  540  static int mtd_nvmem_add(struct mtd_info *mtd)
c4dfa25ab307a2 Alban Bedel        2018-11-13  541  {
658c4448bbbf02 Christian Marangi  2021-03-12 @542  	struct device_node *node = mtd_get_of_node(mtd);
c4dfa25ab307a2 Alban Bedel        2018-11-13  543  	struct nvmem_config config = {};
c4dfa25ab307a2 Alban Bedel        2018-11-13  544  
75f32f4b9d5263 Miquel Raynal      2023-03-07  545  	config.id = NVMEM_DEVID_NONE;
c4dfa25ab307a2 Alban Bedel        2018-11-13  546  	config.dev = &mtd->dev;
7b01b7239d0dc9 Ricardo Ribalda    2020-04-30  547  	config.name = dev_name(&mtd->dev);
c4dfa25ab307a2 Alban Bedel        2018-11-13  548  	config.owner = THIS_MODULE;
c4dfa25ab307a2 Alban Bedel        2018-11-13  549  	config.reg_read = mtd_nvmem_reg_read;
c4dfa25ab307a2 Alban Bedel        2018-11-13  550  	config.size = mtd->size;
c4dfa25ab307a2 Alban Bedel        2018-11-13  551  	config.word_size = 1;
c4dfa25ab307a2 Alban Bedel        2018-11-13  552  	config.stride = 1;
c4dfa25ab307a2 Alban Bedel        2018-11-13  553  	config.read_only = true;
c4dfa25ab307a2 Alban Bedel        2018-11-13  554  	config.root_only = true;
6c7621890995d0 Christophe Kerello 2022-02-20  555  	config.ignore_wp = true;
c4dfa25ab307a2 Alban Bedel        2018-11-13  556  	config.priv = mtd;
c4dfa25ab307a2 Alban Bedel        2018-11-13  557  
c4dfa25ab307a2 Alban Bedel        2018-11-13  558  	mtd->nvmem = nvmem_register(&config);
c4dfa25ab307a2 Alban Bedel        2018-11-13  559  	if (IS_ERR(mtd->nvmem)) {
c4dfa25ab307a2 Alban Bedel        2018-11-13  560  		/* Just ignore if there is no NVMEM support in the kernel */
5cab06156aade1 Miquel Raynal      2023-03-07  561  		if (PTR_ERR(mtd->nvmem) == -EOPNOTSUPP)
c4dfa25ab307a2 Alban Bedel        2018-11-13  562  			mtd->nvmem = NULL;
5cab06156aade1 Miquel Raynal      2023-03-07  563  		else
5cab06156aade1 Miquel Raynal      2023-03-07  564  			return dev_err_probe(&mtd->dev, PTR_ERR(mtd->nvmem),
5cab06156aade1 Miquel Raynal      2023-03-07  565  					     "Failed to register NVMEM device\n");
c4dfa25ab307a2 Alban Bedel        2018-11-13  566  	}
c4dfa25ab307a2 Alban Bedel        2018-11-13  567  
c4dfa25ab307a2 Alban Bedel        2018-11-13  568  	return 0;
c4dfa25ab307a2 Alban Bedel        2018-11-13  569  }
c4dfa25ab307a2 Alban Bedel        2018-11-13  570

Rafał Miłecki July 18, 2023, 4:59 p.m. UTC | #3

On 2023-07-18 18:00, kernel test robot wrote:
>    drivers/mtd/mtdcore.c: In function 'mtd_nvmem_add':
>>> drivers/mtd/mtdcore.c:542:29: warning: unused variable 'node' 
>>> [-Wunused-variable]
>      542 |         struct device_node *node = mtd_get_of_node(mtd);
>          |                             ^~~~

My patch I mentioned:
[PATCH V4] nvmem: add explicit config option to read old syntax fixed OF 
cells

adds following line:
config.add_legacy_fixed_of_cells = of_device_is_compatible(node, 
"nvmem-cells");

So this is a false kernel bot warning (but I understand kernel bot
couldn't understand it needs my other patch).

Rafał Miłecki Oct. 4, 2023, 8:45 a.m. UTC | #4

Srini,

On 26.08.2023 22:15, Rafał Miłecki wrote:
> On 18.07.2023 10:48, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> This reverts commit 517f14d9cf3533d5ab4fded195ab6f80a92e378f.
>>
>> It seems that "no_of_node" config option was added to help mtd's case.
>>
>> DT nodes of MTD partitions (that are also NVMEM devices) may contain
>> subnodes that SHOULD NOT be treated as NVMEM fixed cells. To prevent
>> NVMEM core code from parsing them "no_of_node" was set to true and that
>> made for_each_child_of_node() in NVMEM a no-op.
>>
>> With the introduction of "add_legacy_fixed_of_cells" config option
>> things got more explicit. MTD subsystem simply tells NVMEM when to look
>> for fixed cells and there is no need to hack "of_node" pointer anymore.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> Important: this is based on top of the
>> [PATCH V4] nvmem: add explicit config option to read old syntax fixed OF cells
> 
> I see you skipped those two patches for 6.6.
> 
> Can you queue them for 6.7, please?

Did you have a chance to look at this one?

>> ---
>>   drivers/mtd/mtdcore.c          | 1 -
>>   drivers/nvmem/core.c           | 2 +-
>>   include/linux/nvmem-provider.h | 2 --
>>   3 files changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
>> index 9db8d7853639..3d781ffb8c32 100644
>> --- a/drivers/mtd/mtdcore.c
>> +++ b/drivers/mtd/mtdcore.c
>> @@ -554,7 +554,6 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
>>       config.read_only = true;
>>       config.root_only = true;
>>       config.ignore_wp = true;
>> -    config.no_of_node = !of_device_is_compatible(node, "nvmem-cells");
>>       config.priv = mtd;
>>       mtd->nvmem = nvmem_register(&config);
>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>> index 58d8919e6682..a0c9153cda28 100644
>> --- a/drivers/nvmem/core.c
>> +++ b/drivers/nvmem/core.c
>> @@ -1027,7 +1027,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>>       nvmem->nkeepout = config->nkeepout;
>>       if (config->of_node)
>>           nvmem->dev.of_node = config->of_node;
>> -    else if (!config->no_of_node)
>> +    else
>>           nvmem->dev.of_node = config->dev->of_node;
>>       switch (config->id) {
>> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
>> index 1b81adebdb8b..e3930835235b 100644
>> --- a/include/linux/nvmem-provider.h
>> +++ b/include/linux/nvmem-provider.h
>> @@ -89,7 +89,6 @@ struct nvmem_cell_info {
>>    * @read_only:    Device is read-only.
>>    * @root_only:    Device is accessibly to root only.
>>    * @of_node:    If given, this will be used instead of the parent's of_node.
>> - * @no_of_node:    Device should not use the parent's of_node even if it's !NULL.
>>    * @reg_read:    Callback to read data.
>>    * @reg_write:    Callback to write data.
>>    * @size:    Device size.
>> @@ -122,7 +121,6 @@ struct nvmem_config {
>>       bool            ignore_wp;
>>       struct nvmem_layout    *layout;
>>       struct device_node    *of_node;
>> -    bool            no_of_node;
>>       nvmem_reg_read_t    reg_read;
>>       nvmem_reg_write_t    reg_write;
>>       int    size;
>

Srinivas Kandagatla Oct. 7, 2023, 10:17 a.m. UTC | #5

On Tue, 18 Jul 2023 10:48:04 +0200, Rafał Miłecki wrote:
> This reverts commit 517f14d9cf3533d5ab4fded195ab6f80a92e378f.
> 
> It seems that "no_of_node" config option was added to help mtd's case.
> 
> DT nodes of MTD partitions (that are also NVMEM devices) may contain
> subnodes that SHOULD NOT be treated as NVMEM fixed cells. To prevent
> NVMEM core code from parsing them "no_of_node" was set to true and that
> made for_each_child_of_node() in NVMEM a no-op.
> 
> [...]

Applied, thanks!

[1/1] Revert "nvmem: add new config option"
      commit: bb519262f1a9b4c37046be5a3851ab1681d7158a

Best regards,

diff mbox series

Patch

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 9db8d7853639..3d781ffb8c32 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -554,7 +554,6 @@  static int mtd_nvmem_add(struct mtd_info *mtd)
 	config.read_only = true;
 	config.root_only = true;
 	config.ignore_wp = true;
-	config.no_of_node = !of_device_is_compatible(node, "nvmem-cells");
 	config.priv = mtd;
 
 	mtd->nvmem = nvmem_register(&config);
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 58d8919e6682..a0c9153cda28 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -1027,7 +1027,7 @@  struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	nvmem->nkeepout = config->nkeepout;
 	if (config->of_node)
 		nvmem->dev.of_node = config->of_node;
-	else if (!config->no_of_node)
+	else
 		nvmem->dev.of_node = config->dev->of_node;
 
 	switch (config->id) {
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 1b81adebdb8b..e3930835235b 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -89,7 +89,6 @@  struct nvmem_cell_info {
  * @read_only:	Device is read-only.
  * @root_only:	Device is accessibly to root only.
  * @of_node:	If given, this will be used instead of the parent's of_node.
- * @no_of_node:	Device should not use the parent's of_node even if it's !NULL.
  * @reg_read:	Callback to read data.
  * @reg_write:	Callback to write data.
  * @size:	Device size.
@@ -122,7 +121,6 @@  struct nvmem_config {
 	bool			ignore_wp;
 	struct nvmem_layout	*layout;
 	struct device_node	*of_node;
-	bool			no_of_node;
 	nvmem_reg_read_t	reg_read;
 	nvmem_reg_write_t	reg_write;
 	int	size;