[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f52d33ec-a003-48f8-b74d-080b9b29eb67@notapiano>
Date: Sat, 3 Aug 2024 10:34:55 -0400
From: Nícolas F. R. A. Prado <nfraprado@...labora.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: William-tw Lin <william-tw.lin@...iatek.com>,
Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
Matthias Brugger <matthias.bgg@...il.com>,
Chen-Yu Tsai <wenst@...omium.org>, kernel@...labora.com,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v2] nvmem: mtk-efuse: Only register socinfo device if
needed cells are there
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
Powered by blists - more mailing lists