[PATCH] arm64: dts: r8a7795: Add CAN external clock support

[PATCH] arm64: dts: r8a7795: Add CAN external clock support

[Ramesh Shanmugasundaram]

Adds external CAN clock node for r8a7795. This clock can be used as
fCAN clock of CAN and CAN FD controller.

Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index a7315eb..5167a6c 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -115,6 +115,13 @@
 		clock-frequency = <0>;
 	};
 
+	/* External CAN clock - to be overridden by boards that provide it */
+	can_clk: can {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+	};
+
 	/* External SCIF clock - to be overridden by boards that provide it */
 	scif_clk: scif {
 		compatible = "fixed-clock";
-- 
1.9.1

Re: [PATCH] arm64: dts: r8a7795: Add CAN external clock support

[Geert Uytterhoeven]

Hi Ramesh,

On Fri, Feb 26, 2016 at 3:13 PM, Ramesh Shanmugasundaram
<ramesh.shanmugasundaram@bp.renesas.com> wrote:
> Adds external CAN clock node for r8a7795. This clock can be used as
> fCAN clock of CAN and CAN FD controller.
>
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> index a7315eb..5167a6c 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> @@ -115,6 +115,13 @@
>                 clock-frequency = <0>;
>         };
>
> +       /* External CAN clock - to be overridden by boards that provide it */
> +       can_clk: can {
> +               compatible = "fixed-clock";
> +               #clock-cells = <0>;
> +               clock-frequency = <0>;

As this is an optional clock, it should have

        status = "disabled";

> +       };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH] arm64: dts: r8a7795: Add CAN external clock support

[Ramesh Shanmugasundaram]

Hi Geert,

> On Fri, Feb 26, 2016 at 3:13 PM, Ramesh Shanmugasundaram
> <ramesh.shanmugasundaram@bp.renesas.com> wrote:
> > Adds external CAN clock node for r8a7795. This clock can be used as
> > fCAN clock of CAN and CAN FD controller.
> >
> > Signed-off-by: Ramesh Shanmugasundaram
> > <ramesh.shanmugasundaram@bp.renesas.com>
> > ---
> >  arch/arm64/boot/dts/renesas/r8a7795.dtsi | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> > b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> > index a7315eb..5167a6c 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> > @@ -115,6 +115,13 @@
> >                 clock-frequency = <0>;
> >         };
> >
> > +       /* External CAN clock - to be overridden by boards that provide
> it */
> > +       can_clk: can {
> > +               compatible = "fixed-clock";
> > +               #clock-cells = <0>;
> > +               clock-frequency = <0>;
> 
> As this is an optional clock, it should have
> 
>         status = "disabled";

Yes, I too thought the same but the status property is not obeyed. The "disabled" clock still shows up in clk_tree and clk_xxx apis works just fine (Is this a DT bug?). Hence left the ambiguous "status". In code I use clk_get_rate(can_clk) != 0 to see if it's enabled, which seems logical.

If you add this as a child of soc node, status property is obeyed. But all external clocks are defined outside soc (correctly) and I followed the same.

Thanks,
Ramesh

Note: tests based on next-20160127

Re: [PATCH] arm64: dts: r8a7795: Add CAN external clock support

[Geert Uytterhoeven]

Hi Ramesh,

On Fri, Feb 26, 2016 at 4:00 PM, Ramesh Shanmugasundaram
<ramesh.shanmugasundaram@bp.renesas.com> wrote:
>> On Fri, Feb 26, 2016 at 3:13 PM, Ramesh Shanmugasundaram
>> <ramesh.shanmugasundaram@bp.renesas.com> wrote:
>> > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>> > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>> > @@ -115,6 +115,13 @@
>> >                 clock-frequency = <0>;
>> >         };
>> >
>> > +       /* External CAN clock - to be overridden by boards that provide
>> it */
>> > +       can_clk: can {
>> > +               compatible = "fixed-clock";
>> > +               #clock-cells = <0>;
>> > +               clock-frequency = <0>;
>>
>> As this is an optional clock, it should have
>>
>>         status = "disabled";
>
> Yes, I too thought the same but the status property is not obeyed. The
> "disabled" clock still shows up in clk_tree and clk_xxx apis works just fine
> (Is this a DT bug?). Hence left the ambiguous "status". In code I use
> clk_get_rate(can_clk) != 0 to see if it's enabled, which seems logical.
>
> If you add this as a child of soc node, status property is obeyed. But all
> external clocks are defined outside soc (correctly) and I followed the same.

You're right. That's clearly a bug. I sent a patch to fix that.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2] arm64: dts: r8a7795: Add CAN external clock support

[Ramesh Shanmugasundaram]

Adds external CAN clock node for r8a7795. This clock can be used as
fCAN clock of CAN and CAN FD controller.

Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
---
Thanks Geert for the comments. Added status property as discussed
here (http://www.spinics.net/lists/arm-kernel/msg486931.html)
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index a7315eb..4049182 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -115,6 +115,14 @@
 		clock-frequency = <0>;
 	};
 
+	/* External CAN clock - to be overridden by boards that provide it */
+	can_clk: can {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+		status = "disabled";
+	};
+
 	/* External SCIF clock - to be overridden by boards that provide it */
 	scif_clk: scif {
 		compatible = "fixed-clock";
-- 
1.9.1

Re: [PATCH v2] arm64: dts: r8a7795: Add CAN external clock support

[Geert Uytterhoeven]

On Fri, Feb 26, 2016 at 5:38 PM, Ramesh Shanmugasundaram
<ramesh.shanmugasundaram-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org> wrote:
> Adds external CAN clock node for r8a7795. This clock can be used as
> fCAN clock of CAN and CAN FD controller.
>
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>

Acked-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] arm64: dts: r8a7795: Add CAN external clock support

[Simon Horman]

On Fri, Feb 26, 2016 at 05:54:16PM +0100, Geert Uytterhoeven wrote:
> On Fri, Feb 26, 2016 at 5:38 PM, Ramesh Shanmugasundaram
> <ramesh.shanmugasundaram@bp.renesas.com> wrote:
> > Adds external CAN clock node for r8a7795. This clock can be used as
> > fCAN clock of CAN and CAN FD controller.
> >
> > Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
> 
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, I have queued this up for v4.7.