RE: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Biju Das <biju.das.jz@xxxxxxxxxxxxxx>]

Hi Krzysztof Kozlowski,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
> generator bindings
> 
> On 08/03/2023 19:55, Biju Das wrote:
> >
> > Renesas 8T49N241 has 4 outputs, 1 integral and 3 fractional dividers.
> > The 8T49N241 accepts up to two differential or single-ended input
> > clocks and a fundamental-mode crystal input. The internal PLL can lock
> > to either of the input reference clocks or to the crystal to behave as
> > a frequency synthesizer.
> >
> > Signed-off-by: Alex Helms <alexander.helms.jy@xxxxxxxxxxx>
> > Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
> 
> Ah, indeed, fine then.
> 
> >>
> >>>>
> >>>>> +
> >>>>> +  renesas,clock-flags:
> >>>>> +    description: Flags used in common clock frame work for
> configuring
> >>>>> +      clk outputs. See include/linux/clk-provider.h
> >>>>
> >>>> These are not bindings, so why do you non-bindings constants as
> bindings?
> >>>> They can change anytime. Choose one:
> >>>> 1. Drop entire property,
> >>>> 2. Make it a proper binding property, so an ABI and explain why
> >>>> this is DT specific. None of clock providers have to do it, so you
> >>>> need here good explanation.
> >>>
> >>> I will choose 2 and will explain as user should be allowed to
> >>> configure the output clock from clk generator, so that it has
> >>> flexibility for
> >>> 1) changing the rates (propagate rate change up one level)
> >>> 2) fixed always
> >>> 3) don't gate the ouput clk at all.
> >>
> >> User's choice is task for user-space, so not a good explanation for DT.
> >
> > I don't think clock generator HW has any business with user space.
> >
> > It is clk generator HW specific. Clk generator is vital component
> > which provides clocks to the system.
> 
> Every clock controller is vital...
> 
> > We are providing some hardware feature which is exposed as dt
> > properties.
> >
> > Like clock output is fixed rate clock or dynamic rate clock/
> 
> OK, I wait then for proper description which will explain and justify this.

Here it is, Please let me know is it ok?

renesas,output-clock-fixed-rate-mode:
    type: boolean
    description:
      In output clock fixed rate mode, the output clock frequency is always
      fixed and the hardware will use the values from the OTP or full register
	map initialized during boot.
      If not given, the output clock rate is not fixed.
    maxItems: 6

Cheers,
Biju