Re: [PATCH v2 2/2] arm64: dts: s32g: add uSDHC node

[Ghennadi Procopciuc <ghennadi.procopciuc@oss.nxp.com>]

On 2/24/24 12:44, Chester Lin wrote:
> Hi Ghennadi,
> 
> On Sat, Feb 24, 2024 at 04:22:30PM +0800, Chester Lin wrote:
>> Hi Ghennadi,

Hi Chester,
>>
>> On Mon, Jan 22, 2024 at 04:06:01PM +0200, Ghennadi Procopciuc wrote:
>>> From: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
>>>
>>> Add the uSDHC node for the boards that are based on S32G SoCs.
>>>
>>> Signed-off-by: Ciprian Costea <ciprianmarian.costea@nxp.com>
>>> Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
>>> ---
>>>  arch/arm64/boot/dts/freescale/s32g2.dtsi        | 10 ++++++++++
>>>  arch/arm64/boot/dts/freescale/s32g274a-evb.dts  |  6 +++++-
>>>  arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts |  6 +++++-
>>>  3 files changed, 20 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
>>> index ef1a1d61f2ba..fc19ae2e8d3b 100644
>>> --- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
>>> +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
>>> @@ -138,6 +138,16 @@ uart2: serial@402bc000 {
>>>  			status = "disabled";
>>>  		};
>>>  
>>> +		usdhc0: mmc@402f0000 {
>>> +			compatible = "nxp,s32g2-usdhc";
>>> +			reg = <0x402f0000 0x1000>;
>>> +			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>>> +			clocks = <&clks 32>, <&clks 31>, <&clks 33>;
>>
>> Same as I have mentioned in [PATCH v2 1/2], could we have fixed dt-bindings to
>> replace with these raw clock id values (32, 31, 33)?
>>
> 
> Just found the previous review discussion in v1:
> https://lore.kernel.org/all/f54d947c-58dc-498f-8871-b472f97be4a8@oss.nxp.com/
> 
Indeed, I switched to raw clocks instead of placing them into a binding
header after receiving this feedback on v1.

> What I'm worried is that, could these raw clock IDs be rearranged in the
> downstream TF-A? If so it would cause ABI inconsistency and clock issues
> since the kernel is not aware of any raw ID changes in downstream TF-A.

These clock IDs will become immutable in the downstream version of TF-A
once the patches get merged. This will prevent any unfortunate events
when the Kernel and TF-A are not in sync with regard to SCMI clock IDs.

Best regards,
Ghennadi
> 
> Chester
> 
>>> +			clock-names = "ipg", "ahb", "per";
>>> +			bus-width = <8>;
>>> +			status = "disabled";
>>> +		};
>>> +
>>>  		gic: interrupt-controller@50800000 {
>>>  			compatible = "arm,gic-v3";
>>>  			reg = <0x50800000 0x10000>,
>>> diff --git a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
>>> index 9118d8d2ee01..00070c949e2a 100644
>>> --- a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
>>> +++ b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
>>> @@ -1,7 +1,7 @@
>>>  // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>>>  /*
>>>   * Copyright (c) 2021 SUSE LLC
>>> - * Copyright (c) 2019-2021 NXP
>>> + * Copyright 2019-2021, 2024 NXP
>>>   */
>>>  
>>>  /dts-v1/;
>>> @@ -32,3 +32,7 @@ memory@80000000 {
>>>  &uart0 {
>>>  	status = "okay";
>>>  };
>>> +
>>> +&usdhc0 {
>>> +	status = "okay";
>>> +};
>>> diff --git a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
>>> index e05ee854cdf5..b3fc12899cae 100644
>>> --- a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
>>> +++ b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
>>> @@ -1,7 +1,7 @@
>>>  // SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>>>  /*
>>>   * Copyright (c) 2021 SUSE LLC
>>> - * Copyright (c) 2019-2021 NXP
>>> + * Copyright 2019-2021, 2024 NXP
>>>   */
>>>  
>>>  /dts-v1/;
>>> @@ -38,3 +38,7 @@ &uart0 {
>>>  &uart1 {
>>>  	status = "okay";
>>>  };
>>> +
>>> +&usdhc0 {
>>> +	status = "okay";
>>> +};
>>> -- 
>>> 2.43.0
>>>

-- 
Regards,
Ghennadi