[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d7391797-5c23-4ef9-b448-980ebe5a1d67@collabora.com>
Date: Mon, 5 Aug 2024 13:37:12 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc: William-tw Lin <william-tw.lin@...iatek.com>,
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,
Nícolas F. R. A. Prado <nfraprado@...labora.com>
Subject: Re: [PATCH v2] nvmem: mtk-efuse: Only register socinfo device if
needed cells are there
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.
>>
>> 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