Re: [PATCH 02/14] dt-bindings: phy: qcom,qmp-usb3-dp: fix sc8280xp bindings

[Johan Hovold <johan@kernel.org>]

On Mon, Nov 14, 2022 at 04:49:37PM +0100, Krzysztof Kozlowski wrote:
> On 14/11/2022 15:18, Johan Hovold wrote:
> > On Mon, Nov 14, 2022 at 03:07:41PM +0100, Krzysztof Kozlowski wrote:
> >> On 14/11/2022 14:27, Johan Hovold wrote:
> >>> On Fri, Nov 11, 2022 at 04:17:29PM +0100, Krzysztof Kozlowski wrote:
> >>>> On 11/11/2022 10:24, Johan Hovold wrote:
> >>>>> The current QMP USB3-DP PHY bindings are based on the original MSM8996
> >>>>> binding which provided multiple PHYs per IP block and these in turn were
> >>>>> described by child nodes.
> > 
> >>>>> +  "#clock-cells":
> >>>>> +    const: 1
> >>>>> +
> >>>>> +  clock-output-names:
> >>>>> +    items:
> >>>>> +      - const: usb3_pipe
> >>>>> +      - const: dp_link
> >>>>> +      - const: dp_vco_div
> >>>>
> >>>> Why defining here fixed names? The purpose of this field is to actually
> >>>> allow customizing these - at least in most cases. If these have to be
> >>>> fixed, then driver should just instantiate these clocks with such names,
> >>>> right?
> >>>
> >>> I'm only using these names as documentation of the indexes. The driver
> >>
> >> What do you mean by documentation of indexes? You require these specific
> >> entries and do not allow anything else.
> > 
> > I'm using this property as documentation of the valid indexes that can
> > be used when referring to clocks provided by this device.
> > 
> > There are currently three and the mapping is described by the
> > 'clock-output-names' property.
> 
> That's not the purpose of this property. Drop it then. The names do not
> define the ABI and do not document it, actually. You require now that
> every DTB, if providing clock-output-names, will have exactly such names
> instead of having fixed IDs. DTBs are not for defining the ABI.

Fair enough, I'll drop it. But there doesn't seem to be a good way to
describe the indexes currently and most bindings simply ignore to do so.

So what is the preference then? Just leave things undocumented, listing
indexes in a free-text 'description', or adding a free-text reference to
a binding header file and using those define names in a free-text
'description'?

And if going with the last option, does this mean that every SoC and PHY
type needs its own header for those three clocks or so to avoid having
a common dumping ground header file where indexes will not necessarily
be 0-based and consecutive.

Johan