Re: [PATCH 1/2] arm64: dts: ti: k3-j721e-main: add DP & DP PHY

[Rahul T R <r-ravikumar@ti.com>]

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