linux-kernel - Re: [PATCH RFC] arm64: dts: ti: k3-j721e: Use nvmem to model the efuses

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <41489c26-75ae-4810-9668-47ab170836ea@ti.com>
Date:   Fri, 3 Nov 2023 10:36:54 -0500
From:   Andrew Davis <afd@...com>
To:     Vignesh Raghavendra <vigneshr@...com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>, Nishanth Menon <nm@...com>,
        Tero Kristo <kristo@...nel.org>,
        "Vadapalli, Siddharth" <s-vadapalli@...com>,
        "Vardhan, Vibhore" <vibhore@...com>
CC:     <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC] arm64: dts: ti: k3-j721e: Use nvmem to model the
 efuses

On 10/29/23 11:25 PM, Vignesh Raghavendra wrote:
> + Siddharth for CSPW driver, + Vibhore of CPUFreq
> 
> On 25/10/23 21:13, Andrew Davis wrote:
>> The nvmem subsystem is used to model efuse areas. Currently our efuse
>> areas are described using raw syscon nodes and offsets into that. The
>> use case for this efuse area is to store the SoC Ethernet MAC address.
>> This is already such a commonly supported use case that both the nvmem
>> and networking subsystem have built-in support for it. Use that here
>> to get the following:
>>
>> 1. More standard, complete, and accurate description of this hardware
>>     efuse area.
>>
>> 2. Remove the need for this custom 'ti,syscon-efuse' property (which
>>     I'm surprised was accepted into the binding in the first place..)
>>
>> 3. Allow the parent node to not have to be a syscon which fixes a DT
>>     check warning.
>>
> 
> Sounds like a good plan to me.
> 
>> RFC for now as we don't have a simple generic compatible for memory
>> mapped nvmem areas. For now we use the 'uniphier-efuse' compatible
>> as it is essentially just a generic mmio nvmem area, which means any
>> generic area is technically "compatible".
>>
> 
> Looking at drivers/nvmem/ its seems like each SoC arch seems to have
> their own individual efuse driver?
> 

Yes, but I'm not sure I see the need for a whole new driver/compatible
for every simple MMIO-based efuse ranges. Would be nice to have a
generic one.

> For K3 SoCs (at least AM62), there are other properties in addition to
> MAC address in the eFUSE which are useful for different drivers such as
> Speed grades. Can this be extended for the same?
> 

Yes, with the "nvmem-layout" being worked on right now by the nvmem
folks I see a clear path to extend this over the rest of our efuse
area and replace the remaining "syscon" hacks we have been using.

Andrew

>> If this all is acceptable I'll split this patch into three and do
>> the same for all the other K3 devices still using this odd
>> 'ti,syscon-efuse' pattern for v1.
>>
>> Signed-off-by: Andrew Davis <afd@...com>
>> ---
>>   .../boot/dts/ti/k3-j721e-mcu-wakeup.dtsi      | 24 ++++++++++++++++---
>>   1 file changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
>> index 05d6ef127ba78..8f488d43d49df 100644
>> --- a/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
>> @@ -34,13 +34,30 @@ k3_reset: reset-controller {
>>   		};
>>   	};
>>   
>> -	mcu_conf: syscon@...00000 {
>> -		compatible = "syscon", "simple-mfd";
>> +	mcu_conf: bus@...00000 {
>> +		compatible = "simple-bus";
>>   		reg = <0x0 0x40f00000 0x0 0x20000>;
>>   		#address-cells = <1>;
>>   		#size-cells = <1>;
>>   		ranges = <0x0 0x0 0x40f00000 0x20000>;
>>   
>> +		efuse@200 {
>> +			compatible = "socionext,uniphier-efuse";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			reg = <0x200 0x40>;
>> +
>> +			nvmem-layout {
>> +				compatible = "fixed-layout";
>> +				#address-cells = <1>;
>> +				#size-cells = <1>;
>> +
>> +				mac_address: mac@0 {
>> +					reg = <0x0 0x6>;
>> +				};
>> +			};
>> +		};
>> +
>>   		phy_gmii_sel: phy@...0 {
>>   			compatible = "ti,am654-phy-gmii-sel";
>>   			reg = <0x4040 0x4>;
>> @@ -533,7 +550,8 @@ cpsw_port1: port@1 {
>>   				reg = <1>;
>>   				ti,mac-only;
>>   				label = "port1";
>> -				ti,syscon-efuse = <&mcu_conf 0x200>;
>> +				nvmem-cells = <&mac_address>;
>> +				nvmem-cell-names = "mac-address";
>>   				phys = <&phy_gmii_sel 1>;
>>   			};
>>   		};
>