Re: [PATCH v2] nvmem: mtk-efuse: Only register socinfo device if needed cells are there

["Nícolas F. R. A. Prado" <nfraprado@collabora.com>]

On Fri, Jul 19, 2024 at 11:29:03AM +0200, AngeloGioacchino Del Regno wrote:
> Il 19/07/24 00:07, Nícolas F. R. A. Prado ha scritto:
> > On Wed, Jul 10, 2024 at 11:31:11AM +0200, AngeloGioacchino Del Regno wrote:
> > > Il 08/07/24 21:43, Nícolas F. R. A. Prado ha scritto:
> > > > Not every efuse region has cells storing SoC information. Only register
> > > > an socinfo device if the required cells are present.
> > > > 
> > > > This prevents the pointless process of creating an socinfo device,
> > > > probing it with the socinfo driver only to ultimately error out like so
> > > > 
> > > >     mtk-socinfo mtk-socinfo.0.auto: error -ENOENT: Failed to get socinfo data
> > > >     mtk-socinfo mtk-socinfo.0.auto: probe with driver mtk-socinfo failed with error -2
> > > > 
> > > > This issue is observed on the mt8183-kukui-jacuzzi-juniper-sku16
> > > > platform, which has two efuse regions, but only one of them contains the
> > > > SoC data.
> > > > 
> > > 
> > > I think that we should rather remove or disable the first eFuse region, as
> > > even though that is enabled:
> > > 
> > >   - This is the only SoC having two regions
> > >     - I'm not even sure that the region at 0x8000000 is really efuse
> > >     - Not even referenced in datasheets....
> > >   - It's unused, as in, it's not exposing any information and no declared cells
> > > 
> > > Don't misunderstand me, this is not an invalid change, but I rather prefer
> > > to resolve this by disabling that (effectively unused!) node, avoiding to
> > > add more lines to this driver that would be useless after fixing that small
> > > single thing.
> > 
> > I'm not confident that we can say that that efuse is not exposing any
> > information. Indeed there are no cells so it's not used by any other driver, but
> > the efuse contents are still exposed to userspace if CONFIG_NVMEM_SYSFS is
> > enabled.
> > 
> > I dumped it on one of the mt8183-kukui-jacuzzi-juniper-sku16 units:
> > 
> >    $ ls -l /sys/bus/nvmem/devices/
> >    total 0
> >    lrwxrwxrwx    1 root     root             0 Jul 18 21:43 mmtd0 -> ../../../devices/platform/soc/11010000.spi/spi_master/spi1/spi1.0/mtd/mtd0/mtd0
> >    lrwxrwxrwx    1 root     root             0 Jul 18 21:43 nvmem0 -> ../../../devices/platform/soc/8000000.efuse/nvmem0
> >    lrwxrwxrwx    1 root     root             0 Jul 18 21:43 nvmem1 -> ../../../devices/platform/soc/11f10000.efuse/nvmem1
> >    $ hexdump -C /sys/bus/nvmem/devices/nvmem0/nvmem
> >    00000000  88 07 00 00 00 8a 00 00  00 ca 00 00 00 00 00 00  |................|
> >    00000010
> > 
> > I power cycled the unit and ran this again and it still showed the same
> > contents. I also ran the same on a different unit of the same model and it
> > showed the same contents. Of course this doesn't prove anything, but given that
> > the contents seem to be constant across reboots and even different units, it
> > does look like it could be an efuse to me. :)
> > 
> > As to whether the contents are useful at all, or if there are
> > userspace applications making use of it I have no clue. But if in doubt,
> > shouldn't we keep it around?
> 
> (Added William-tw Lin from MediaTek to the loop)
> 
> I'll say yes only if MediaTek (please!) says that this region has useful
> information, and only if MediaTek actually tells us what those fuses are.
> 
> The reason is that sometimes when SoCs have multiple efuse regions, one contains
> uncalibrated data for factory calibration (etc etc), one contains data that derives
> from the uncalibrated regions and that is supposed to be used by the OS.
> 
> If we got the uncalibrated data that is *not* for OS usage in the MT8183 DT, then
> we can as well just remove it.
> 
> Besides, I have no concern about any userspace application using that.

No reply, so I've sent v3.

Thanks,
Nícolas