Re: [PATCH 15/16] dt-bindings: phy: cdns,dphy: make clocks optional

From: Pratyush Yadav <p.yadav@ti.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Vinod Koul <vkoul@kernel.org>,
	Peter Ujfalusi <peter.ujfalusi@gmail.com>,
	Maxime Ripard <mripard@kernel.org>,
	Benoit Parrot <bparrot@ti.com>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Alexandre Courbot <acourbot@chromium.org>,
	Stanimir Varbanov <stanimir.varbanov@linaro.org>,
	Helen Koike <helen.koike@collabora.com>,
	Michael Tretter <m.tretter@pengutronix.de>,
	Peter Chen <peter.chen@nxp.com>,
	Chunfeng Yun <chunfeng.yun@mediatek.com>,
	<linux-media@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-phy@lists.infradead.org>,
	<dmaengine@vger.kernel.org>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Subject: Re: [PATCH 15/16] dt-bindings: phy: cdns,dphy: make clocks optional
Date: Wed, 7 Apr 2021 00:18:30 +0530	[thread overview]
Message-ID: <20210406184828.ftxrrnrrvbprrlom@ti.com> (raw)
In-Reply-To: <YGbyfg4hs/yLsqw0@pendragon.ideasonboard.com>

On 02/04/21 01:31PM, Laurent Pinchart wrote:
> Hi Pratyush,
> 
> Thank you for the patch.
> 
> On Tue, Mar 30, 2021 at 11:03:47PM +0530, Pratyush Yadav wrote:
> > The clocks are not used by the DPHY when used in Rx mode so make them
> > optional.
> 
> Isn't there a main functional clock (DPHY_RX_MAIN_CLK in the J721E TRM)
> that is needed in RX mode ?

That clock is different from the clocks being used in this binding. The 
"psm" clock is for the PMA state machine (the internal state machine for 
the DPHY). The divider for this clock should be set such that the 
resultant clock is as close to 1 MHz as possible. This can be done 
either by programming the register value or by setting the correct value 
on the psm_clock_freq pin. On J721E the pin already has the correct 
value so there is no need for setting it via the register.

The other clock is "pll_ref" which is used to set the input clock 
divider. Setting this divider is part of the DPHY TX programming 
sequence but is not part of the RX programming sequence. I'm not sure 
what exactly the divider does but I think it is supposed to divide the 
clock from the input stream to the TX DPHY to make sure the internal 
state machine is running at the correct speed. Anyway, it is not needed 
on the RX side because for that there is another register used (see 
cdns_dphy_rx_get_band_ctrl() in patch 4).

The DPHY_RX_MAIN_CLK does eventually get divided into the PSM clock but 
it is not used directly.

> 
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > ---
> >  Documentation/devicetree/bindings/phy/cdns,dphy.yaml | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/cdns,dphy.yaml b/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
> > index d1bbf96a8250..0807ba68284d 100644
> > --- a/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
> > @@ -33,8 +33,6 @@ properties:
> >  required:
> >    - compatible
> >    - reg
> > -  - clocks
> > -  - clock-names
> >    - "#phy-cells"
> >  
> >  additionalProperties: false
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.