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: <4wmxjxcvt7un7wk5v43q3jpxqjs2jbc626mgah2fxbfuouu4q6@ptzibxe2apmx>
Date: Fri, 3 Jan 2025 07:50:43 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Wasim Nazir <quic_wasimn@...cinc.com>
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 Thu, Jan 02, 2025 at 02:37:39PM +0530, Wasim Nazir wrote:
> 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.

Krzysztof asked you to include DTSI files instead of including DTS
files. Hope this helps.

> 
> [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

-- 
With best wishes
Dmitry