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

Ghennadi Procopciuc
2024-02-26T06:29:41Z

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