Message-ID: <e6dec500c2e7532a65c12c7e8919e5b7f202ed14.camel@mediatek.com>
Date: Wed, 14 Aug 2024 06:32:33 +0000
From: William-tw Lin (林鼎崴)
	<William-tw.Lin@...iatek.com>
To: "srinivas.kandagatla@...aro.org" <srinivas.kandagatla@...aro.org>,
	"angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>
CC: "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>, "wenst@...omium.org" <wenst@...omium.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"kernel@...labora.com" <kernel@...labora.com>, "nfraprado@...labora.com"
	<nfraprado@...labora.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] nvmem: mtk-efuse: Only register socinfo device if
 needed cells are there

On Mon, 2024-08-05 at 13:37 +0200, AngeloGioacchino Del Regno wrote:
> Il 03/08/24 16:34, Nícolas F. R. A. Prado ha scritto:
> > 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.
This node contains some SoC-related information. However, this is
unrelated to the mtk-socninfo driver.
> > > 
> > > 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.
> > 
> 
> Resolved with devicetree change. Please ignore this patch.
> 
> Cheers,
> Angelo
> 

Powered by blists - more mailing lists