Re: [PATCH v2 2/8] dt-bindings: counter: Add new ti,am62-eqep compatible

[David Lechner]

On 5/24/24 4:44 PM, Judith Mendez wrote:
> On 5/24/24 3:57 PM, David Lechner wrote:
>> On 5/24/24 3:50 PM, David Lechner wrote:
>>> On 5/23/24 6:15 PM, Judith Mendez wrote:
>>>> Add new compatible ti,am62-eqep for TI K3 devices. If a device
>>>> uses this compatible, require power-domains property.
>>>>
>>>> Since there is only one functional and interface clock for eqep,
>>>> clock-names is not really required. The clock-name also changed
>>>> for TI K3 SoCs so make clock-names optional for the new compatible
>>>> since there is only one clock that is routed to the IP.
>>>>
>>>> While we are here, add an example using ti,am62-eqep compatible.
>>>>
>>>> Signed-off-by: Judith Mendez <jm@xxxxxx>
>>>> ---
>>>> Changes since v1:
>>>> - Fix eqep binding for new compatible, require
>>>>   power-domains for new compatible
>>>> ---
>>>>   .../devicetree/bindings/counter/ti-eqep.yaml  | 53 +++++++++++++++++--
>>>>   1 file changed, 48 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/counter/ti-eqep.yaml b/Documentation/devicetree/bindings/counter/ti-eqep.yaml
>>>> index 85f1ff83afe72..c4bb0231f166a 100644
>>>> --- a/Documentation/devicetree/bindings/counter/ti-eqep.yaml
>>>> +++ b/Documentation/devicetree/bindings/counter/ti-eqep.yaml
>>>> @@ -11,7 +11,9 @@ maintainers:
>>>>     properties:
>>>>     compatible:
>>>> -    const: ti,am3352-eqep
>>>> +    enum:
>>>> +      - ti,am3352-eqep
>>>> +      - ti,am62-eqep
>>>>       reg:
>>>>       maxItems: 1
>>>> @@ -21,19 +23,43 @@ properties:
>>>>       maxItems: 1
>>>>       clocks:
>>>> -    description: The clock that determines the SYSCLKOUT rate for the eQEP
>>>> -      peripheral.
>>>> +    description: The functional and interface clock that determines the clock
>>>> +      rate for the eQEP peripheral.
>>>>       maxItems: 1
>>>>       clock-names:
>>>> -    const: sysclkout
>>>> +    enum:
>>>> +      - sysclkout
>>>> +      - fck
>>>> +
>>>
>>> If we are making this optional for ti,am62-eqep, why add a new name?
>>>
>>> Also, we could change the description to say that sysclockout is not a
>>> great name but is required for backwards compatibility.
>>>
>>>> +  power-domains:
>>>> +    maxItems: 1
>>>> +
>>>> +allOf:
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - ti,am3352-eqep
>>>> +    then:
>>>> +      required:
>>>> +        - clock-names
>>
>> I just looked at the Linux driver for this and the clock name is
>> not used in the driver. So we could probably just deprecate the
>> clock-names property here and not make it required for
>> ti,am3352-eqep (and not allowed for any new compatibles as
>> suggested below).
> 
> We could do this, although I was under the impression that we should
> not drop DT properties just because the linux driver isn't using it,
> that is why I went with keeping clock-names around for am335x compatible
> and making it optional for am62x compatible.
> 
> But if it is all the same, we could drop the the DT property.
> 
> ~ Judith
> 

I wasn't suggesting to remove clock-names from the bindings, just
deprecate that property in this binding and not use it with any
new compatibles.

In the AM62x technical reference manual, it looks like it calls
the functional and interface clock FICLK rather than FCK. So
I'm just suggesting maybe it just easier to not give it a name
rather than try to get the right name? No name will work with
any future SoCs as well. :-)