Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 31 Mar 2022 19:45:15 +0530
From:   Rahul T R <r-ravikumar@ti.com>
To:     Kishon Vijay Abraham I <kishon@ti.com>
CC:     <nm@ti.com>, <vigneshr@ti.com>, <kristo@kernel.org>,
        <robh+dt@kernel.org>, <krzysztof.kozlowski@canonical.com>,
        <linux-arm-kernel@lists.infradead.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <tomi.valkeinen@ideasonboard.com>,
        <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH 1/2] arm64: dts: ti: k3-j721e-main: add DP & DP PHY
Message-ID: <20220331141514.kr2synsaodqd6w2c@uda0490373>
References: <20220222163230.1566-1-r-ravikumar@ti.com>
 <20220222163230.1566-2-r-ravikumar@ti.com>
 <069c1751-41a9-e47c-96d7-8ed83e1fe3f9@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <069c1751-41a9-e47c-96d7-8ed83e1fe3f9@ti.com>
User-Agent: NeoMutt/20171215
Precedence: bulk

On 09:47-20220228, Kishon Vijay Abraham I wrote:
> Hi Rahul,
> 
> On 22/02/22 10:02 pm, Rahul T R wrote:
> > From: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > 
> > Add DT nodes for DisplayPort and DisplayPort PHY. The DP is Cadence MHDP
> > 8546 and the PHY is a Cadence Torrent PHY with TI WIZ wrapper.
> > 
> > A slight irregularity in the bindings is the DPTX PHY register block,
> > which is in the MHDP IP, but is needed and mapped by the PHY.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Signed-off-by: Rahul T R <r-ravikumar@ti.com>
> > ---
> >  arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 102 ++++++++++++++++++++++
> >  1 file changed, 102 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> > index 599861259a30..9e2b212100bb 100644
> > --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> > +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> > @@ -786,6 +786,82 @@
> >  		#size-cells = <2>;
> >  	};
> >  
> > +	serdes_wiz4: wiz@5050000 {
> > +		compatible = "ti,j721e-wiz-10g";
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		power-domains = <&k3_pds 297 TI_SCI_PD_EXCLUSIVE>;
> > +		clocks = <&k3_clks 297 1>, <&k3_clks 297 9>, <&cmn_refclk>;
> > +		clock-names = "fck", "core_ref_clk", "ext_ref_clk";
> > +		assigned-clocks = <&k3_clks 297 9>;
> > +		assigned-clock-parents = <&k3_clks 297 10>;
> > +		assigned-clock-rates = <19200000>;
> > +		num-lanes = <4>;
> > +		#reset-cells = <1>;
> > +		ranges = <0x5050000 0x0 0x5050000 0x10000>,
> > +			<0xa030a00 0x0 0xa030a00 0x40>;
> > +
> > +		wiz4_pll0_refclk: pll0-refclk {
> > +			clocks = <&k3_clks 297 9>, <&cmn_refclk>;
> > +			clock-output-names = "wiz4_pll0_refclk";
> > +			#clock-cells = <0>;
> > +			assigned-clocks = <&wiz4_pll0_refclk>;
> > +			assigned-clock-parents = <&k3_clks 297 9>;
> > +		};
> > +
> > +		wiz4_pll1_refclk: pll1-refclk {
> > +			clocks = <&k3_clks 297 9>, <&cmn_refclk>;
> > +			clock-output-names = "wiz4_pll1_refclk";
> > +			#clock-cells = <0>;
> > +			assigned-clocks = <&wiz4_pll1_refclk>;
> > +			assigned-clock-parents = <&k3_clks 297 9>;
> > +		};
> > +
> > +		wiz4_refclk_dig: refclk-dig {
> > +			clocks = <&k3_clks 297 9>, <&cmn_refclk>;
> > +			clock-output-names = "wiz4_refclk_dig";
> > +			#clock-cells = <0>;
> > +			assigned-clocks = <&wiz4_refclk_dig>;
> > +			assigned-clock-parents = <&k3_clks 297 9>;
> > +		};
> > +
> > +		wiz4_cmn_refclk_dig_div: cmn-refclk-dig-div {
> > +			clocks = <&wiz4_refclk_dig>;
> > +			#clock-cells = <0>;
> > +		};
> > +
> > +		wiz4_cmn_refclk1_dig_div: cmn-refclk1-dig-div {
> > +			clocks = <&wiz4_pll1_refclk>;
> > +			#clock-cells = <0>;
> > +		};
> 
> I'd prefer we deprecate creating clock sub-nodes for new platform additions and
> use a similar approach as that used for AM64 (use phandle with a parameter to
> refer clocks). Please refer how this was created for AM64
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/ti/k3-am64-main.dtsi#n795
> 
> Since this is the same SERDES IP as used in AM64, you should be able to use the
> same DT binding as that used in AM64. But if you'd like to adapt
> "ti,j721e-wiz-10g" to use phandle with parameter in the WIZ driver (while making
> sure the existing sub-node based binding is not broken), that is highly welcomed.
>

Thanks Kishon,

Will address this in the respin

Regards
Rahul T R
 
> > +
> > +		serdes4: serdes@5050000 {
> > +			/*
> > +			 * Note: we also map DPTX PHY registers as the Torrent
> > +			 * needs to manage those.
> > +			 */
> > +			compatible = "ti,j721e-serdes-10g";
> > +			reg = <0x5050000 0x10000>,
> > +			      <0xa030a00 0x40>; /* DPTX PHY */
> > +			reg-names = "torrent_phy", "dptx_phy";
> > +
> > +			resets = <&serdes_wiz4 0>;
> > +			reset-names = "torrent_reset";
> > +			clocks = <&wiz4_pll0_refclk>;
> > +			clock-names = "refclk";
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +			torrent_phy_dp: phy@0 {
> > +				reg = <0>;
> > +				resets = <&serdes_wiz4 1>;
> > +				cdns,phy-type = <PHY_TYPE_DP>;
> > +				cdns,num-lanes = <4>;
> > +				cdns,max-bit-rate = <5400>;
> > +				#phy-cells = <0>;
> > +			};
> 
> The link sub-nodes should be in the board DTS file.
> 
> Thanks,
> Kishon