linux-kernel - Re: [PATCH v5 5/6] arm64: dts: qcom: Add support for QCS9075 Ride & Ride-r3

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] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z3ZXWxoBtMNPJ9kk@hu-wasimn-hyd.qualcomm.com>
Date: Thu, 2 Jan 2025 14:37:39 +0530
From: Wasim Nazir <quic_wasimn@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio
	<konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, <linux-arm-msm@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <kernel@...cinc.com>
Subject: Re: [PATCH v5 5/6] arm64: dts: qcom: Add support for QCS9075 Ride &
 Ride-r3

On Mon, Dec 30, 2024 at 05:45:39PM +0200, Dmitry Baryshkov wrote:
> On Sun, Dec 29, 2024 at 08:53:31PM +0530, Wasim Nazir wrote:
> > Add device tree support for QCS9075 Ride & Ride-r3 boards.
> > 
> > QCS9075 lacks the safety monitoring features of Safety-Island (SAIL)
> > subsystem which is available in QCS9100, and it affects thermal
> > management.
> > 
> > Also, between ride and ride-r3 ethernet phy is different.
> > Ride uses 1G ethernet phy while ride-r3 uses 2.5G ethernet phy.
> 
> Your board files duplicate sa8775p-ride-r3.dts and sa8775p-ride.dts, but
> include them. Existing qcs9100-ride-r3.dts and qcs9100-ride-r3.dts just
> include corresponding SA8775P files.
> 
> This is not ideal for the following reasons:
> - The approach is not uniform (between QCS9100 and QCS9075), which might
>   lead to mistakes.
> - The approach ends up duplicating DT code unnecessarily, which can lead
>   to issues being patches in the one board file, but not in the other
>   file.
> 
> If there are any reasons why you want to follow this approach, they must
> be a part of the commit message.
> 

Hi Dmitry,

Initially, we included the DTS [1] file to avoid duplication. However,
based on Krzysztof's previous suggestion [2], we change to this format.

Please let us know how to proceed further on this.

[1] https://lore.kernel.org/all/20241119174954.1219002-6-quic_wasimn@quicinc.com/
[2] https://lore.kernel.org/all/8cf9edc0-a0cb-4fd0-b10e-2138784dfba3@kernel.org/

> > 
> > Signed-off-by: Wasim Nazir <quic_wasimn@...cinc.com>
> > ---
> >  arch/arm64/boot/dts/qcom/Makefile            |  2 +
> >  arch/arm64/boot/dts/qcom/qcs9075-ride-r3.dts | 46 ++++++++++++++++++++
> >  arch/arm64/boot/dts/qcom/qcs9075-ride.dts    | 46 ++++++++++++++++++++
> >  3 files changed, 94 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/qcom/qcs9075-ride-r3.dts
> >  create mode 100644 arch/arm64/boot/dts/qcom/qcs9075-ride.dts
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> > index 78613a1bd34a..41cb2bbd3472 100644
> > --- a/arch/arm64/boot/dts/qcom/Makefile
> > +++ b/arch/arm64/boot/dts/qcom/Makefile
> > @@ -118,6 +118,8 @@ dtb-$(CONFIG_ARCH_QCOM)	+= qcs6490-rb3gen2.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qcs8300-ride.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qcs8550-aim300-aiot.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qcs9075-rb8.dtb
> > +dtb-$(CONFIG_ARCH_QCOM)	+= qcs9075-ride.dtb
> > +dtb-$(CONFIG_ARCH_QCOM)	+= qcs9075-ride-r3.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
> > diff --git a/arch/arm64/boot/dts/qcom/qcs9075-ride-r3.dts b/arch/arm64/boot/dts/qcom/qcs9075-ride-r3.dts
> > new file mode 100644
> > index 000000000000..d9a8956d3a76
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/qcs9075-ride-r3.dts
> > @@ -0,0 +1,46 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> > + */
> > +/dts-v1/;
> > +
> > +#include "sa8775p-ride.dtsi"
> > +
> > +/ {
> > +	model = "Qualcomm Technologies, Inc. QCS9075 Ride Rev3";
> > +	compatible = "qcom,qcs9075-ride-r3", "qcom,qcs9075", "qcom,sa8775p";
> > +};
> > +
> > +&ethernet0 {
> > +	phy-mode = "2500base-x";
> > +};
> > +
> > +&ethernet1 {
> > +	phy-mode = "2500base-x";
> > +};
> > +
> > +&mdio {
> > +	compatible = "snps,dwmac-mdio";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	sgmii_phy0: phy@8 {
> > +		compatible = "ethernet-phy-id31c3.1c33";
> > +		reg = <0x8>;
> > +		device_type = "ethernet-phy";
> > +		interrupts-extended = <&tlmm 7 IRQ_TYPE_EDGE_FALLING>;
> > +		reset-gpios = <&pmm8654au_2_gpios 8 GPIO_ACTIVE_LOW>;
> > +		reset-assert-us = <11000>;
> > +		reset-deassert-us = <70000>;
> > +	};
> > +
> > +	sgmii_phy1: phy@0 {
> > +		compatible = "ethernet-phy-id31c3.1c33";
> > +		reg = <0x0>;
> > +		device_type = "ethernet-phy";
> > +		interrupts-extended = <&tlmm 26 IRQ_TYPE_EDGE_FALLING>;
> > +		reset-gpios = <&pmm8654au_2_gpios 9 GPIO_ACTIVE_LOW>;
> > +		reset-assert-us = <11000>;
> > +		reset-deassert-us = <70000>;
> > +	};
> > +};
> > diff --git a/arch/arm64/boot/dts/qcom/qcs9075-ride.dts b/arch/arm64/boot/dts/qcom/qcs9075-ride.dts
> > new file mode 100644
> > index 000000000000..3b524359a72d
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/qcs9075-ride.dts
> > @@ -0,0 +1,46 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> > + */
> > +/dts-v1/;
> > +
> > +#include "sa8775p-ride.dtsi"
> > +
> > +/ {
> > +	model = "Qualcomm Technologies, Inc. QCS9075 Ride";
> > +	compatible = "qcom,qcs9075-ride", "qcom,qcs9075", "qcom,sa8775p";
> > +};
> > +
> > +&ethernet0 {
> > +	phy-mode = "sgmii";
> > +};
> > +
> > +&ethernet1 {
> > +	phy-mode = "sgmii";
> > +};
> > +
> > +&mdio {
> > +	compatible = "snps,dwmac-mdio";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	sgmii_phy0: phy@8 {
> > +		compatible = "ethernet-phy-id0141.0dd4";
> > +		reg = <0x8>;
> > +		device_type = "ethernet-phy";
> > +		interrupts-extended = <&tlmm 7 IRQ_TYPE_EDGE_FALLING>;
> > +		reset-gpios = <&pmm8654au_2_gpios 8 GPIO_ACTIVE_LOW>;
> > +		reset-assert-us = <11000>;
> > +		reset-deassert-us = <70000>;
> > +	};
> > +
> > +	sgmii_phy1: phy@a {
> > +		compatible = "ethernet-phy-id0141.0dd4";
> > +		reg = <0xa>;
> > +		device_type = "ethernet-phy";
> > +		interrupts-extended = <&tlmm 26 IRQ_TYPE_EDGE_FALLING>;
> > +		reset-gpios = <&pmm8654au_2_gpios 9 GPIO_ACTIVE_LOW>;
> > +		reset-assert-us = <11000>;
> > +		reset-deassert-us = <70000>;
> > +	};
> > +};
> > --
> > 2.47.0
> > 
> 
> -- 
> With best wishes
> Dmitry


Thanks & Regards,
Wasim