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

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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