Re: [PATCH v3 01/12] dt-bindings: clock: Add RaspberryPi RP1 clock bindings

Stephen Boyd
2024-11-15T23:00:37Z

Quoting Andrea della Porta (2024-11-15 03:31:45)
> On 10:16 Thu 31 Oct , Andrea della Porta wrote:
> > On 08:23 Tue 29 Oct , Krzysztof Kozlowski wrote:
> > > > + '#clock-cells':
> > > > + description:
> > > > + The index in the assigned-clocks is mapped to the output clock as per
> > > > + definitions in include/dt-bindings/clock/raspberrypi,rp1-clocks.h.
> > >
> > > You still describe how current driver matches assigned-clocks to your
> > > output clocks. That's not the property of clock-cells and that's not how
> > > assigned-clocks work.
> >
> > This description is taken by another upstream binding, please see
> > Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> >
> > Its purpose is to let the user know how clock-cell number specified
> > in assigned-clocks is mapped to the clock provided by this generator.
> > Since some of these clocks are shared among peripherals, their frequency
> > cannot be set by consumers, so it's the provider itself (i.e. the clock
> > device described with this binding) that should take care of them.
> > The renesas example has assigned-clocks specified though, please see below.
> >
> > >
> > > There are no assigned clocks in your DTS, so this is really irrelevant
> > > (or not correct, choose).
> >
> > In the first revision of this patchset (please see [1] and following messages)
> > I had the assigned-clocks setup in the example while trying to explain their
> > purpose, but Conor said those didn't seem to be relevant, hence I dropped them.
> > Maybe I had to be more incisive on that.
> > So, I'd be inclined to retain the description as it is and reintroduce some
> > assigned-clocks in the example as in the renesas one, would it be ok for you?
>
> Since I'm on the verge of producing a new patchset revision, may I kindly ask
> some comments on this? Is it ok for you?
>

Everyone knows how #clock-cells works. It shouldn't need a description
about how it works. It should just point at the header file with the
numbers if anything.