Re: [PATCH v6 3/3] nvmem: core: Expose cells through sysfs

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Chen-Yu,

> >  static int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> > @@ -1006,6 +1101,12 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
> >  	if (rval)
> >  		goto err_remove_cells;
> >  
> > +#ifdef CONFIG_NVMEM_SYSFS
> > +	rval = nvmem_populate_sysfs_cells(nvmem);
> > +	if (rval)
> > +		goto err_remove_cells;  
> 
> This breaks nvmem / efuse devices with multiple cells that share the
> same name. Something like this in DT:
> 
> 	efuse: efuse@11f10000 {
> 		compatible = "mediatek,mt8183-efuse",
> 			     "mediatek,efuse";
> 		reg = <0 0x11f10000 0 0x1000>;
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		thermal_calibration: calib@180 {
> 			reg = <0x180 0xc>;
> 		};
> 
> 		mipi_tx_calibration: calib@190 {
> 			reg = <0x190 0xc>;
> 		};
> 
> 		svs_calibration: calib@580 {
> 			reg = <0x580 0x64>;
> 		};
> 	};
> 
> creates three cells, all named DT, and sysfs will complain:
> 
>     sysfs: cannot create duplicate filename '/devices/platform/soc/11f10000.efuse/nvmem1/cells/calib'
>     mediatek,efuse: probe of 11f10000.efuse failed with error -17
> 
> This causes the MT8183-based Chromebooks to lose display capability,
> among other things.

Sorry for the breakage, I did not identify this case, but you're right
this is incorrectly handled currently.

> The problem lies in the nvmem DT parsing code, where the cell name is
> derived from the node name, without including the address portion.
> However I'm not sure we can change that, since it could be considered
> ABI?

I would be in favor suffixing the cell names anyway as they have not
been exposed yet to userspace at all (well, not more than a couple of
days in -next).

Thanks,
Miquèl