Re: [PATCH 1/2] dt-bindings: clock: fixed-factor: Add TI AM62 SoC OLDI clock

[Stephen Boyd]

Quoting Tomi Valkeinen (2023-01-17 01:40:24)
> On 16/01/2023 11:51, Aradhya Bhatia wrote:
> > Hi Stephen,
> > 
> > Thanks for taking a look at the patch.
> > 
> > On 12-Jan-23 01:14, Stephen Boyd wrote:
> >> Quoting Aradhya Bhatia (2022-12-26 01:57:44)
> >>> Add "ti,k3-am62-oldi-clk-div" to the fixed factor clock compatible enum
> >>> list.
> >>>
> >>> "ti,k3-am62-oldi-clk-div" is a fixed-factor clock that helps the TI
> >>> display subsystem request a pixel clock for itself and a corresponding
> >>> serial clock for its OLDI Transmitters. The serial clock is 7 times the
> >>> pixel clock. This clock needs the clock set rate request to be
> >>> propagated to the parent clock provider.
> >>>
> >>> Signed-off-by: Aradhya Bhatia <a-bhatia1@xxxxxx>
> >>> ---
> >>>   Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml | 1 +
> >>>   1 file changed, 1 insertion(+)
> >>>
> >>> diff --git 
> >>> a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml 
> >>> b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml
> >>> index 8f71ab300470..0696237530f7 100644
> >>> --- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml
> >>> +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml
> >>> @@ -14,6 +14,7 @@ properties:
> >>>     compatible:
> >>>       enum:
> >>>         - fixed-factor-clock
> >>> +      - ti,k3-am62-oldi-clk-div
> >>
> >> I don't see this compatible anywhere in the kernel tree. Is there a
> >> patch that adds a node using this? I wonder why the display subsystem
> >> can't add this fixed factor clk directly in the driver. Does the OLDI
> >> Transmitter send a clk to the display subsystem?
> >>
> >> I'm asking all these questions because we got rid of vendor compatibles
> >> here in hopes of simplifying the logic. Maybe the problem can be
> >> approached differently, but I don't know all the details.
> > 
> > 
> > +--------+                       +------------------+
> > |        |                       |                  |
> > |  PLL   +---+----+------------->| OLDI Transmitter |
> > |        |   |    |              |                  |
> > +--------+   |    |              +------------------+
> >               |    |
> >               |    |              +------------------+
> >               |    |              |                  |
> >               |    +------------->| OLDI Transmitter |
> >               |                   |                  |
> >               |                   +------------------+
> >               |
> >               |                   +------------------+
> >               |   +----------+    |                  |
> >               |   |    /7    |    |      Display     |
> >               +-->|   Clock  +--->| Sub-System (DSS) |
> >                   |    Div   |    |                  |
> >                   +----------+    +------------------+
> > 
> > This is how the the clock architecture for DSS looks like.
> > 
> > The clock divider is not a part of DSS, but outside it.

The divider is fixed as well? And presumably inside the SoC?

> > 
> > The clock request flow is initiated by the DSS driver because it has the
> > required timing parameter information. It requests a certain pixel
> > frequency. But the frequency required by the OLDI TXes is 7 times
> > that pixel frequency.
> > 
> > (Just for clarification, in some cases, the OLDI TX does require only
> > 3.5 times the pixel frequency, but in those situations there is another
> > divider in-front of OLDI TX that gets activated with a signal and
> > divides the incoming frequency by 2, thereby requiring the PLL to still
> > generate a 7x frequency.)
> > 
> > Hence, the idea is that the clock divider is able to propagate the set
> > rate request back to PLL, asking for a frequency 7 times more than the
> > DSS's asking rate.

Got it. Can the PLL driver provide a pll_div_7 clk that is used for the
DSS pixel clk?

> > 
> > If this is something less than ideal and should not go up, then I can
> > implement a new clock device with a separate but similar clock driver.
> > 
> > Let me know what you think!
> 
> As a clarification I would also add to the above that on other TI SoCs 
> with DSS, and also for the second video port on AM62, the clock 
> framework provides DSS a clock using the pclk frequency.
> 

Are you saying that adding a fixed div-7 clk in the DSS driver is wrong?