[PATCH can-next] dt-bindings: can: tcan4x5x: add missing required clock-names

[PATCH can-next] dt-bindings: can: tcan4x5x: add missing required clock-names

[Sean Nyekjaer]

tcan4x5x requires an external clock called cclk, add it here.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
 Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
index ff18cf7393550d1b7107b1233d8302203026579d..f3f3cbc03aec13e517552d2e29ecea1585de8e36 100644
--- a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
+++ b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
@@ -29,6 +29,10 @@ properties:
   clocks:
     maxItems: 1
 
+  clock-names:
+    items:
+      - const: cclk
+
   reset-gpios:
     description: Hardwired output GPIO. If not defined then software reset.
     maxItems: 1
@@ -154,6 +158,7 @@ examples:
         can@0 {
             compatible = "ti,tcan4x5x";
             reg = <0>;
+            clock-names = "cclk";
             clocks = <&can0_osc>;
             pinctrl-names = "default";
             pinctrl-0 = <&can0_pins>;
@@ -179,6 +184,7 @@ examples:
         can@0 {
             compatible = "ti,tcan4552", "ti,tcan4x5x";
             reg = <0>;
+            clock-names = "cclk";
             clocks = <&can0_osc>;
             pinctrl-names = "default";
             pinctrl-0 = <&can0_pins>;

---
base-commit: e0b741bc53c94f9ae25d4140202557a0aa51b5a0
change-id: 20241127-tcancclk-c149c0b3b050

Best regards,
-- 
Sean Nyekjaer <sean@geanix.com>

Re: [PATCH can-next] dt-bindings: can: tcan4x5x: add missing required clock-names

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1905 bytes --]

On Wed, Nov 27, 2024 at 02:40:47PM +0100, Sean Nyekjaer wrote:
> tcan4x5x requires an external clock called cclk, add it here.

That's not what this patch is doing, the clock input is already there,
so I don't know what this patch actually accomplishes? clock-names isn't
a required property, so you can't even use it in a driver.

> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
>  Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
> index ff18cf7393550d1b7107b1233d8302203026579d..f3f3cbc03aec13e517552d2e29ecea1585de8e36 100644
> --- a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
> +++ b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
> @@ -29,6 +29,10 @@ properties:
>    clocks:
>      maxItems: 1
>  
> +  clock-names:
> +    items:
> +      - const: cclk
> +
>    reset-gpios:
>      description: Hardwired output GPIO. If not defined then software reset.
>      maxItems: 1
> @@ -154,6 +158,7 @@ examples:
>          can@0 {
>              compatible = "ti,tcan4x5x";
>              reg = <0>;
> +            clock-names = "cclk";
>              clocks = <&can0_osc>;
>              pinctrl-names = "default";
>              pinctrl-0 = <&can0_pins>;
> @@ -179,6 +184,7 @@ examples:
>          can@0 {
>              compatible = "ti,tcan4552", "ti,tcan4x5x";
>              reg = <0>;
> +            clock-names = "cclk";
>              clocks = <&can0_osc>;
>              pinctrl-names = "default";
>              pinctrl-0 = <&can0_pins>;
> 
> ---
> base-commit: e0b741bc53c94f9ae25d4140202557a0aa51b5a0
> change-id: 20241127-tcancclk-c149c0b3b050
> 
> Best regards,
> -- 
> Sean Nyekjaer <sean@geanix.com>
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH can-next] dt-bindings: can: tcan4x5x: add missing required clock-names

[Sean Nyekjaer]

Hi Conor,

On Wed, Nov 27, 2024 at 03:50:30PM +0100, Conor Dooley wrote:
> On Wed, Nov 27, 2024 at 02:40:47PM +0100, Sean Nyekjaer wrote:
> > tcan4x5x requires an external clock called cclk, add it here.
> 
> That's not what this patch is doing, the clock input is already there,
> so I don't know what this patch actually accomplishes? clock-names isn't
> a required property, so you can't even use it in a driver.
> 

Thanks for asking the right questions :)

I know the clock input is there, but it looks (to me) like the driver looks for the
specific clock called cclk:
https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/m_can.c#L2299
https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/tcan4x5x-core.c#L396

Br,
/Sean

Re: [PATCH can-next] dt-bindings: can: tcan4x5x: add missing required clock-names

[Sean Nyekjaer]

On Wed, Nov 27, 2024 at 04:56:13PM +0100, Sean Nyekjaer wrote:
> Hi Conor,
> 
> On Wed, Nov 27, 2024 at 03:50:30PM +0100, Conor Dooley wrote:
> > On Wed, Nov 27, 2024 at 02:40:47PM +0100, Sean Nyekjaer wrote:
> > > tcan4x5x requires an external clock called cclk, add it here.
> > 
> > That's not what this patch is doing, the clock input is already there,
> > so I don't know what this patch actually accomplishes? clock-names isn't
> > a required property, so you can't even use it in a driver.
> > 
> 
> Thanks for asking the right questions :)
> 
> I know the clock input is there, but it looks (to me) like the driver looks for the
> specific clock called cclk:
> https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/m_can.c#L2299
> https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/tcan4x5x-core.c#L396

Oh I really need to get my head around the dt jargon :)
Yes I'll add the clock-names to the required list for v2!

Br,
/Sean

Re: [PATCH can-next] dt-bindings: can: tcan4x5x: add missing required clock-names

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

On Wed, Nov 27, 2024 at 05:10:31PM +0100, Sean Nyekjaer wrote:
> On Wed, Nov 27, 2024 at 04:56:13PM +0100, Sean Nyekjaer wrote:
> > Hi Conor,
> > 
> > On Wed, Nov 27, 2024 at 03:50:30PM +0100, Conor Dooley wrote:
> > > On Wed, Nov 27, 2024 at 02:40:47PM +0100, Sean Nyekjaer wrote:
> > > > tcan4x5x requires an external clock called cclk, add it here.
> > > 
> > > That's not what this patch is doing, the clock input is already there,
> > > so I don't know what this patch actually accomplishes? clock-names isn't
> > > a required property, so you can't even use it in a driver.
> > > 
> > 
> > Thanks for asking the right questions :)
> > 
> > I know the clock input is there, but it looks (to me) like the driver looks for the
> > specific clock called cclk:
> > https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/m_can.c#L2299
> > https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/tcan4x5x-core.c#L396
> 
> Oh I really need to get my head around the dt jargon :)
> Yes I'll add the clock-names to the required list for v2!

btw, where even is ti,tcan4x5x.yaml? I was gonna paste the fixes tag you
should be using but I couldn't find the file in linux-next.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH can-next] dt-bindings: can: tcan4x5x: add missing required clock-names

[Sean Nyekjaer]

On Wed, Nov 27, 2024 at 04:18:59PM +0100, Conor Dooley wrote:
> On Wed, Nov 27, 2024 at 05:10:31PM +0100, Sean Nyekjaer wrote:
> > On Wed, Nov 27, 2024 at 04:56:13PM +0100, Sean Nyekjaer wrote:
> > > Hi Conor,
> > > 
> > > On Wed, Nov 27, 2024 at 03:50:30PM +0100, Conor Dooley wrote:
> > > > On Wed, Nov 27, 2024 at 02:40:47PM +0100, Sean Nyekjaer wrote:
> > > > > tcan4x5x requires an external clock called cclk, add it here.
> > > > 
> > > > That's not what this patch is doing, the clock input is already there,
> > > > so I don't know what this patch actually accomplishes? clock-names isn't
> > > > a required property, so you can't even use it in a driver.
> > > > 
> > > 
> > > Thanks for asking the right questions :)
> > > 
> > > I know the clock input is there, but it looks (to me) like the driver looks for the
> > > specific clock called cclk:
> > > https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/m_can.c#L2299
> > > https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/tcan4x5x-core.c#L396
> > 
> > Oh I really need to get my head around the dt jargon :)
> > Yes I'll add the clock-names to the required list for v2!
> 
> btw, where even is ti,tcan4x5x.yaml? I was gonna paste the fixes tag you
> should be using but I couldn't find the file in linux-next.

It's here:
https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/commit/?h=testing&id=77400284f54b9a1f6b6127c08cb935fc05e5c3d2

Do you think the fixes tag is needed?

Fixes: 77400284f54b ("dt-bindings: can: convert tcan4x5x.txt to DT schema")

/Sean

Re: [PATCH can-next] dt-bindings: can: tcan4x5x: add missing required clock-names

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1937 bytes --]

On Wed, Nov 27, 2024 at 07:13:27PM +0100, Sean Nyekjaer wrote:
> On Wed, Nov 27, 2024 at 04:18:59PM +0100, Conor Dooley wrote:
> > On Wed, Nov 27, 2024 at 05:10:31PM +0100, Sean Nyekjaer wrote:
> > > On Wed, Nov 27, 2024 at 04:56:13PM +0100, Sean Nyekjaer wrote:
> > > > Hi Conor,
> > > > 
> > > > On Wed, Nov 27, 2024 at 03:50:30PM +0100, Conor Dooley wrote:
> > > > > On Wed, Nov 27, 2024 at 02:40:47PM +0100, Sean Nyekjaer wrote:
> > > > > > tcan4x5x requires an external clock called cclk, add it here.
> > > > > 
> > > > > That's not what this patch is doing, the clock input is already there,
> > > > > so I don't know what this patch actually accomplishes? clock-names isn't
> > > > > a required property, so you can't even use it in a driver.
> > > > > 
> > > > 
> > > > Thanks for asking the right questions :)
> > > > 
> > > > I know the clock input is there, but it looks (to me) like the driver looks for the
> > > > specific clock called cclk:
> > > > https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/m_can.c#L2299
> > > > https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/tcan4x5x-core.c#L396
> > > 
> > > Oh I really need to get my head around the dt jargon :)
> > > Yes I'll add the clock-names to the required list for v2!
> > 
> > btw, where even is ti,tcan4x5x.yaml? I was gonna paste the fixes tag you
> > should be using but I couldn't find the file in linux-next.
> 
> It's here:
> https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/commit/?h=testing&id=77400284f54b9a1f6b6127c08cb935fc05e5c3d2
> 
> Do you think the fixes tag is needed?
> 
> Fixes: 77400284f54b ("dt-bindings: can: convert tcan4x5x.txt to DT schema")

Ideally it'd get squashed if it isn't even in next, but ye if you made
the clock required on this platform in the conversion then you should've
made clock-names required too since the driver uses it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH can-next] dt-bindings: can: tcan4x5x: add missing required clock-names

[Sean Nyekjaer]

On Wed, Nov 27, 2024 at 08:00:51PM +0100, Conor Dooley wrote:
> On Wed, Nov 27, 2024 at 07:13:27PM +0100, Sean Nyekjaer wrote:
> > On Wed, Nov 27, 2024 at 04:18:59PM +0100, Conor Dooley wrote:
> > > On Wed, Nov 27, 2024 at 05:10:31PM +0100, Sean Nyekjaer wrote:
> > > > On Wed, Nov 27, 2024 at 04:56:13PM +0100, Sean Nyekjaer wrote:
> > > > > Hi Conor,
> > > > > 
> > > > > On Wed, Nov 27, 2024 at 03:50:30PM +0100, Conor Dooley wrote:
> > > > > > On Wed, Nov 27, 2024 at 02:40:47PM +0100, Sean Nyekjaer wrote:
> > > > > > > tcan4x5x requires an external clock called cclk, add it here.
> > > > > > 
> > > > > > That's not what this patch is doing, the clock input is already there,
> > > > > > so I don't know what this patch actually accomplishes? clock-names isn't
> > > > > > a required property, so you can't even use it in a driver.
> > > > > > 
> > > > > 
> > > > > Thanks for asking the right questions :)
> > > > > 
> > > > > I know the clock input is there, but it looks (to me) like the driver looks for the
> > > > > specific clock called cclk:
> > > > > https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/m_can.c#L2299
> > > > > https://elixir.bootlin.com/linux/v6.12/source/drivers/net/can/m_can/tcan4x5x-core.c#L396
> > > > 
> > > > Oh I really need to get my head around the dt jargon :)
> > > > Yes I'll add the clock-names to the required list for v2!
> > > 
> > > btw, where even is ti,tcan4x5x.yaml? I was gonna paste the fixes tag you
> > > should be using but I couldn't find the file in linux-next.
> > 
> > It's here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/commit/?h=testing&id=77400284f54b9a1f6b6127c08cb935fc05e5c3d2
> > 
> > Do you think the fixes tag is needed?
> > 
> > Fixes: 77400284f54b ("dt-bindings: can: convert tcan4x5x.txt to DT schema")
> 
> Ideally it'd get squashed if it isn't even in next, but ye if you made
> the clock required on this platform in the conversion then you should've
> made clock-names required too since the driver uses it.

"dt-bindings: can: convert tcan4x5x.txt to DT schema" did the
conversion no more or less.
The original txt file fails to mention the clock required,
therefore IMHO this patch should be as a seperate patch :)

I'll submit v2 and then it's upto you folks...

Thanks,
/Sean