2024-05-23 23:17:55

by Judith Mendez

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

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 <[email protected]>
---
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
+
+ power-domains:
+ maxItems: 1
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - ti,am3352-eqep
+ then:
+ required:
+ - clock-names
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - ti,am62-eqep
+ then:
+ required:
+ - power-domains

required:
- compatible
- reg
- interrupts
- clocks
- - clock-names

additionalProperties: false

@@ -47,4 +73,21 @@ examples:
interrupts = <79>;
};

+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/soc/ti,sci_pm_domain.h>
+
+ bus {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ eqep1: counter@23210000 {
+ compatible = "ti,am62-eqep";
+ reg = <0x00 0x23210000 0x00 0x100>;
+ power-domains = <&k3_pds 60 TI_SCI_PD_EXCLUSIVE>;
+ clocks = <&k3_clks 60 0>;
+ interrupts = <GIC_SPI 117 IRQ_TYPE_EDGE_RISING>;
+ status = "disabled";
+ };
+ };
...
--
2.45.1



2024-05-24 18:38:33

by Conor Dooley

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

On Thu, May 23, 2024 at 06:15:10PM -0500, 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.

Really the clock should be named after the function it has in the IP
block - it looks like "sysoutclk" is more likely the name of the clock
routed to the IP rather than the role it has?
>
> While we are here, add an example using ti,am62-eqep compatible.
>
> Signed-off-by: Judith Mendez <[email protected]>
> ---
> 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

I'm going to ack this even though the driver makes it seem like the
devices are compatible (there's no match data etc) given the addition of
the power domain and changes in required properties.
Acked-by: Conor Dooley <[email protected]>

Cheers,
Conor.


Attachments:
(No filename) (1.72 kB)
signature.asc (235.00 B)
Download all attachments

2024-05-24 20:58:25

by David Lechner

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

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 <[email protected]>
>> ---
>> 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).

>
> What if we just add
>
> else:
> clock-names: false
>
> since there is only one clock and not worry about the name?
>
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - ti,am62-eqep
>> + then:
>> + required:
>> + - power-domains
>>
>> required:
>> - compatible
>> - reg
>> - interrupts
>> - clocks
>> - - clock-names
>>


2024-05-24 21:09:21

by David Lechner

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

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 <[email protected]>
> ---
> 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

What if we just add

else:
clock-names: false

since there is only one clock and not worry about the name?

> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - ti,am62-eqep
> + then:
> + required:
> + - power-domains
>
> required:
> - compatible
> - reg
> - interrupts
> - clocks
> - - clock-names
>

2024-05-24 21:31:38

by Judith Mendez

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

On 5/24/24 1:38 PM, Conor Dooley wrote:
> On Thu, May 23, 2024 at 06:15:10PM -0500, 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.
>
> Really the clock should be named after the function it has in the IP
> block - it looks like "sysoutclk" is more likely the name of the clock
> routed to the IP rather than the role it has?

It is the name of the clock, though id like to keep sysclkout for
backwards compatibility, even though the name is confusing.
~ Judith


>>
>> While we are here, add an example using ti,am62-eqep compatible.
>>
>> Signed-off-by: Judith Mendez <[email protected]>
>> ---
>> 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
>
> I'm going to ack this even though the driver makes it seem like the
> devices are compatible (there's no match data etc) given the addition of
> the power domain and changes in required properties.
> Acked-by: Conor Dooley <[email protected]>
>
> Cheers,
> Conor.
>


2024-05-24 21:34:22

by Judith Mendez

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

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 <[email protected]>
>> ---
>> 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.

This is fine with me. Will update for v3.

>
>> + power-domains:
>> + maxItems: 1
>> +
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - ti,am3352-eqep
>> + then:
>> + required:
>> + - clock-names
>
> What if we just add
>
> else:
> clock-names: false
>
> since there is only one clock and not worry about the name?


Yes, can do, thanks.

>
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - ti,am62-eqep
>> + then:
>> + required:
>> + - power-domains
>>
>> required:
>> - compatible
>> - reg
>> - interrupts
>> - clocks
>> - - clock-names
>>


2024-05-24 21:45:10

by Judith Mendez

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

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 <[email protected]>
>>> ---
>>> 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

>
>>
>> What if we just add
>>
>> else:
>> clock-names: false
>>
>> since there is only one clock and not worry about the name?
>>
>>> + - if:
>>> + properties:
>>> + compatible:
>>> + contains:
>>> + enum:
>>> + - ti,am62-eqep
>>> + then:
>>> + required:
>>> + - power-domains
>>>
>>> required:
>>> - compatible
>>> - reg
>>> - interrupts
>>> - clocks
>>> - - clock-names
>>>
>


2024-05-25 15:24:08

by Conor Dooley

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

On Fri, May 24, 2024 at 04:30:32PM -0500, Judith Mendez wrote:
> On 5/24/24 1:38 PM, Conor Dooley wrote:
> > On Thu, May 23, 2024 at 06:15:10PM -0500, 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.
> >
> > Really the clock should be named after the function it has in the IP
> > block - it looks like "sysoutclk" is more likely the name of the clock
> > routed to the IP rather than the role it has?
>
> It is the name of the clock, though id like to keep sysclkout for
> backwards compatibility, even though the name is confusing.

FWIW, I was not suggesting that it be renamed.

Thanks,
Conor.


Attachments:
(No filename) (0.98 kB)
signature.asc (235.00 B)
Download all attachments

2024-05-25 17:49:51

by David Lechner

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

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 <[email protected]>
>>>> ---
>>>> 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. :-)


2024-05-29 17:21:04

by Judith Mendez

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

Hi Conor,

On 5/25/24 10:23 AM, Conor Dooley wrote:
> On Fri, May 24, 2024 at 04:30:32PM -0500, Judith Mendez wrote:
>> On 5/24/24 1:38 PM, Conor Dooley wrote:
>>> On Thu, May 23, 2024 at 06:15:10PM -0500, 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.
>>>
>>> Really the clock should be named after the function it has in the IP
>>> block - it looks like "sysoutclk" is more likely the name of the clock
>>> routed to the IP rather than the role it has?
>>
>> It is the name of the clock, though id like to keep sysclkout for
>> backwards compatibility, even though the name is confusing.
>
> FWIW, I was not suggesting that it be renamed.

Understood, thanks.

>
> Thanks,
> Conor.


2024-05-29 18:56:21

by Judith Mendez

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

Hi David,

On 5/25/24 12:49 PM, David Lechner wrote:
> 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 <[email protected]>
>>>>> ---
>>>>> 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. :-)

Understood, so I will make clock-names optional for ti,am3352-eqep
compatible and not allowed for ti,am62-eqep compatible.

Thanks for your feedback (:

~ Judith
>