Re: [PATCH 12/14] arm64: dts: renesas: r8a77965: add CAN support
Hi Eugeniu,
On 06/08/18 21:14, Eugeniu Rosca wrote:
> Hi Kieran,
>
> On Mon, Aug 06, 2018 at 12:11:22PM +0100, Kieran Bingham wrote:
>> Hi Eugeniu,
>>
>> On 05/08/18 00:11, Eugeniu Rosca wrote:
>>> According to R-Car Gen3 HW manual rev0.55E, R-Car M3-N has two CAN
>>
>> rev 0.55E sounds like rather an old version of this document. Do you
>> have access to the later rev1.00 release?
>
> Thanks for this feedback. I was able to find the newer version.
>
>>
>> (Not an issue for this patch itself, I can confirm that revision 1.00
>> still confirms M3-N CAN support)
>>
>>> interfaces, similar to H3, M3-W and other SoCs from the same family.
>>>
>>> Add CAN nodes to avoid below r8a77965-ulcb-kf.dtb build failure:
>>> Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:19.1-6 Label or path can0
>>> not found
>>> Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:25.1-6 Label or path can1
>>> not found
>>
>> Again, this is somewhat referencing the future, as (in patch sequence)
>> the file "r8a77965-ulcb-kf.dts" does not yet exist, so does not really
>> need to be mentioned here.
>
> Will remove the "r8a77965-ulcb-kf.dtb" line from commit description.
>
>>
>>
>>> CAN support is inspired from below commits:
>>> - v4.7 commit 308b7e4ba62e ("arm64: dts: r8a7795: Add CAN support")
>>> - v4.11 commit 909c16252415 ("arm64: dts: r8a7796: Add CAN support")
>>> - v4.12 commit bec0948e810f ("arm64: dts: r8a7796: Add reset control
>>> properties")
>>>
>>> Signed-off-by: Eugeniu Rosca
>>> ---
>>> arch/arm64/boot/dts/renesas/r8a77965.dtsi | 32
>>>
>>> 1 file changed, 32 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>>> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>>> index 486aecacb22a..cb8f8573d9ef 100644
>>> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>>> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>>> @@ -656,6 +656,38 @@
>>> status = "disabled";
>>> };
>>>
>>> + can0: can@e6c3 {
>>> + compatible = "renesas,can-r8a77965",
>>> +"renesas,rcar-gen3-can";
>>> + reg = <0 0xe6c3 0 0x1000>;
>>> + interrupts = ;
>>> + clocks = < CPG_MOD 916>,
>>> + < CPG_CORE R8A77965_CLK_CANFD>,
>>> + <_clk>;
>>> + clock-names = "clkp1", "clkp2", "can_clk";
>>> + assigned-clocks = < CPG_CORE R8A77965_CLK_CANFD>;
>>> + assigned-clock-rates = <4000>;
>>
>> This doesn't look right. Sections 52A.2 has a note stating:
>>
>> CANFD? must be set as follows.
>> R-Car H3, R-Car M3-W, R-Car M3-N, R-Car V3M, R-Car V3H, R-Car E3: 80 (MHz)
>>
>> R-Car D3: 40 (MHz)
>>
>>
>> Could you verify / check in case this value should be 80MHz?
>
> Are you sure section "52A. CAN-FD" is the right one for describing the
> CAN (non-FD) nodes? For non-FD CAN there is another chapter called
> "52. Controller Area Network Interface (CAN interface)". Since the
> latter doesn't point out any differences between M3-W and M3-N, I
> re-used the M3-W (r8A7796.dtsi) configuration.
My apologies - It looks like I had got the wrong section.
>
> FWIW, r8a7795 (H3), r8a7796 (M3) and r8a77995 (D3) all currently
> (v4.18-rc8) use the same "assigned-clock-rates" value for can0
> and can1 nodes:
>
> $ git grep -E -A 10 "can[01]:" -- arch/arm64/boot/dts/renesas | grep
> assigned-clock-rates
> arch/arm64/boot/dts/renesas/r8a7795.dtsi- assigned-clock-rates =
> <4000>;
> arch/arm64/boot/dts/renesas/r8a7795.dtsi- assigned-clock-rates =
> <4000>;
> arch/arm64/boot/dts/renesas/r8a7796.dtsi- assigned-clock-rates =
> <4000>;
> arch/arm64/boot/dts/renesas/r8a7796.dtsi- assigned-clock-rates =
> <4000>;
> arch/arm64/boot/dts/renesas/r8a77995.dtsi-assigned-clock-rates =
> <4000>;
> arch/arm64/boot/dts/renesas/r8a77995.dtsi-assigned-clock-rates =
> <4000>;
>
> Anyway, given that this patch only intended to avoid the "make dtbs"
> failure and given that any CAN tests are out of scope, I will just leave
> a placeholder for can0 and can1 nodes, as suggested by Geert.
Well - I certainly have no complaints about functionality which isn't
present :)
--
Regards
Kieran
>
>>
>>> + power-domains = < R8A77965_PD_ALWAYS_ON>;
>>> + resets = < 916>;
>>> + status = "disabled";
>>> + };
>>> +
>>> + can1: can@e6c38000 {
>>> + compatible = "renesas,can-r8a77965",
>>> +"renesas,rcar-gen3-can";
>>> + reg = <0 0xe6c38000 0 0x1000>;
>>> + interrupts = ;
>>> + clocks = < CPG_MOD 915>,
>>> + < CPG_CORE R8A77965_CLK_CANFD>,
>>> + <_clk>;
>>> + clock-names = "clkp1", "clkp2", "can_clk";
Re: [PATCH 12/14] arm64: dts: renesas: r8a77965: add CAN support
Hi Geert,
On Mon, Aug 06, 2018 at 05:15:37PM +0200, Geert Uytterhoeven wrote:
> Hi Eugeniu,
>
> On Sun, Aug 5, 2018 at 1:14 AM Eugeniu Rosca wrote:
> > According to R-Car Gen3 HW manual rev0.55E, R-Car M3-N has two CAN
> > interfaces, similar to H3, M3-W and other SoCs from the same family.
> >
> > Add CAN nodes to avoid below r8a77965-ulcb-kf.dtb build failure:
> > Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:19.1-6 Label or path can0
> > not found
> > Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:25.1-6 Label or path can1
> > not found
> >
> > CAN support is inspired from below commits:
> > - v4.7 commit 308b7e4ba62e ("arm64: dts: r8a7795: Add CAN support")
> > - v4.11 commit 909c16252415 ("arm64: dts: r8a7796: Add CAN support")
> > - v4.12 commit bec0948e810f ("arm64: dts: r8a7796: Add reset control
> > properties")
> >
> > Signed-off-by: Eugeniu Rosca
>
> Thanks for your patch!
>
> Have you actually tested CAN operation, or were you just fixing the
> build failure?
> In case of the latter, please just add minimal placeholders, like were present
> for other device nodes in early versions of r8a77965.dtsi.
Same as replied to Kieran, I will add some skeleton nodes for can0 and
can1, just to avoid DTC errors. It was not my goal to do CAN bring-up.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@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
Best regards,
Eugeniu.
Re: [PATCH 12/14] arm64: dts: renesas: r8a77965: add CAN support
Hi Kieran,
On Mon, Aug 06, 2018 at 12:11:22PM +0100, Kieran Bingham wrote:
> Hi Eugeniu,
>
> On 05/08/18 00:11, Eugeniu Rosca wrote:
> > According to R-Car Gen3 HW manual rev0.55E, R-Car M3-N has two CAN
>
> rev 0.55E sounds like rather an old version of this document. Do you
> have access to the later rev1.00 release?
Thanks for this feedback. I was able to find the newer version.
>
> (Not an issue for this patch itself, I can confirm that revision 1.00
> still confirms M3-N CAN support)
>
> > interfaces, similar to H3, M3-W and other SoCs from the same family.
> >
> > Add CAN nodes to avoid below r8a77965-ulcb-kf.dtb build failure:
> > Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:19.1-6 Label or path can0
> > not found
> > Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:25.1-6 Label or path can1
> > not found
>
> Again, this is somewhat referencing the future, as (in patch sequence)
> the file "r8a77965-ulcb-kf.dts" does not yet exist, so does not really
> need to be mentioned here.
Will remove the "r8a77965-ulcb-kf.dtb" line from commit description.
>
>
> > CAN support is inspired from below commits:
> > - v4.7 commit 308b7e4ba62e ("arm64: dts: r8a7795: Add CAN support")
> > - v4.11 commit 909c16252415 ("arm64: dts: r8a7796: Add CAN support")
> > - v4.12 commit bec0948e810f ("arm64: dts: r8a7796: Add reset control
> > properties")
> >
> > Signed-off-by: Eugeniu Rosca
> > ---
> > arch/arm64/boot/dts/renesas/r8a77965.dtsi | 32
> >
> > 1 file changed, 32 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > index 486aecacb22a..cb8f8573d9ef 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > @@ -656,6 +656,38 @@
> > status = "disabled";
> > };
> >
> > + can0: can@e6c3 {
> > + compatible = "renesas,can-r8a77965",
> > +"renesas,rcar-gen3-can";
> > + reg = <0 0xe6c3 0 0x1000>;
> > + interrupts = ;
> > + clocks = < CPG_MOD 916>,
> > + < CPG_CORE R8A77965_CLK_CANFD>,
> > + <_clk>;
> > + clock-names = "clkp1", "clkp2", "can_clk";
> > + assigned-clocks = < CPG_CORE R8A77965_CLK_CANFD>;
> > + assigned-clock-rates = <4000>;
>
> This doesn't look right. Sections 52A.2 has a note stating:
>
> CANFD? must be set as follows.
> R-Car H3, R-Car M3-W, R-Car M3-N, R-Car V3M, R-Car V3H, R-Car E3: 80 (MHz)
>
> R-Car D3: 40 (MHz)
>
>
> Could you verify / check in case this value should be 80MHz?
Are you sure section "52A. CAN-FD" is the right one for describing the
CAN (non-FD) nodes? For non-FD CAN there is another chapter called
"52. Controller Area Network Interface (CAN interface)". Since the
latter doesn't point out any differences between M3-W and M3-N, I
re-used the M3-W (r8A7796.dtsi) configuration.
FWIW, r8a7795 (H3), r8a7796 (M3) and r8a77995 (D3) all currently
(v4.18-rc8) use the same "assigned-clock-rates" value for can0
and can1 nodes:
$ git grep -E -A 10 "can[01]:" -- arch/arm64/boot/dts/renesas | grep
assigned-clock-rates
arch/arm64/boot/dts/renesas/r8a7795.dtsi- assigned-clock-rates =
<4000>;
arch/arm64/boot/dts/renesas/r8a7795.dtsi- assigned-clock-rates =
<4000>;
arch/arm64/boot/dts/renesas/r8a7796.dtsi- assigned-clock-rates =
<4000>;
arch/arm64/boot/dts/renesas/r8a7796.dtsi- assigned-clock-rates =
<4000>;
arch/arm64/boot/dts/renesas/r8a77995.dtsi- assigned-clock-rates =
<4000>;
arch/arm64/boot/dts/renesas/r8a77995.dtsi- assigned-clock-rates =
<4000>;
Anyway, given that this patch only intended to avoid the "make dtbs"
failure and given that any CAN tests are out of scope, I will just leave
a placeholder for can0 and can1 nodes, as suggested by Geert.
>
> > + power-domains = < R8A77965_PD_ALWAYS_ON>;
> > + resets = < 916>;
> > + status = "disabled";
> > + };
> > +
> > + can1: can@e6c38000 {
> > + compatible = "renesas,can-r8a77965",
> > +"renesas,rcar-gen3-can";
> > + reg = <0 0xe6c38000 0 0x1000>;
> > + interrupts = ;
> > + clocks = < CPG_MOD 915>,
> > + < CPG_CORE R8A77965_CLK_CANFD>,
> > + <_clk>;
> > + clock-names = "clkp1", "clkp2", "can_clk";
> > + assigned-clocks = < CPG_CORE R8A77965_CLK_CANFD>;
> > + assigned-clock-rates = <4000>;
>
> Same here of course.
>
>
> > + power-domains = < R8A77965_PD_ALWAYS_ON>;
> > + resets = < 915>;
> > +
Re: [PATCH 12/14] arm64: dts: renesas: r8a77965: add CAN support
Hi Eugeniu,
On Sun, Aug 5, 2018 at 1:14 AM Eugeniu Rosca wrote:
> According to R-Car Gen3 HW manual rev0.55E, R-Car M3-N has two CAN
> interfaces, similar to H3, M3-W and other SoCs from the same family.
>
> Add CAN nodes to avoid below r8a77965-ulcb-kf.dtb build failure:
> Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:19.1-6 Label or path can0 not
> found
> Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:25.1-6 Label or path can1 not
> found
>
> CAN support is inspired from below commits:
> - v4.7 commit 308b7e4ba62e ("arm64: dts: r8a7795: Add CAN support")
> - v4.11 commit 909c16252415 ("arm64: dts: r8a7796: Add CAN support")
> - v4.12 commit bec0948e810f ("arm64: dts: r8a7796: Add reset control
> properties")
>
> Signed-off-by: Eugeniu Rosca
Thanks for your patch!
Have you actually tested CAN operation, or were you just fixing the
build failure?
In case of the latter, please just add minimal placeholders, like were present
for other device nodes in early versions of r8a77965.dtsi.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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 12/14] arm64: dts: renesas: r8a77965: add CAN support
Hi Eugeniu,
On 05/08/18 00:11, Eugeniu Rosca wrote:
> According to R-Car Gen3 HW manual rev0.55E, R-Car M3-N has two CAN
rev 0.55E sounds like rather an old version of this document. Do you
have access to the later rev1.00 release?
(Not an issue for this patch itself, I can confirm that revision 1.00
still confirms M3-N CAN support)
> interfaces, similar to H3, M3-W and other SoCs from the same family.
>
> Add CAN nodes to avoid below r8a77965-ulcb-kf.dtb build failure:
> Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:19.1-6 Label or path can0 not
> found
> Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:25.1-6 Label or path can1 not
> found
Again, this is somewhat referencing the future, as (in patch sequence)
the file "r8a77965-ulcb-kf.dts" does not yet exist, so does not really
need to be mentioned here.
> CAN support is inspired from below commits:
> - v4.7 commit 308b7e4ba62e ("arm64: dts: r8a7795: Add CAN support")
> - v4.11 commit 909c16252415 ("arm64: dts: r8a7796: Add CAN support")
> - v4.12 commit bec0948e810f ("arm64: dts: r8a7796: Add reset control
> properties")
>
> Signed-off-by: Eugeniu Rosca
> ---
> arch/arm64/boot/dts/renesas/r8a77965.dtsi | 32
>
> 1 file changed, 32 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> index 486aecacb22a..cb8f8573d9ef 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -656,6 +656,38 @@
> status = "disabled";
> };
>
> + can0: can@e6c3 {
> + compatible = "renesas,can-r8a77965",
> + "renesas,rcar-gen3-can";
> + reg = <0 0xe6c3 0 0x1000>;
> + interrupts = ;
> + clocks = < CPG_MOD 916>,
> +< CPG_CORE R8A77965_CLK_CANFD>,
> +<_clk>;
> + clock-names = "clkp1", "clkp2", "can_clk";
> + assigned-clocks = < CPG_CORE R8A77965_CLK_CANFD>;
> + assigned-clock-rates = <4000>;
This doesn't look right. Sections 52A.2 has a note stating:
CANFD? must be set as follows.
R-Car H3, R-Car M3-W, R-Car M3-N, R-Car V3M, R-Car V3H, R-Car E3: 80 (MHz)
R-Car D3: 40 (MHz)
Could you verify / check in case this value should be 80MHz?
> + power-domains = < R8A77965_PD_ALWAYS_ON>;
> + resets = < 916>;
> + status = "disabled";
> + };
> +
> + can1: can@e6c38000 {
> + compatible = "renesas,can-r8a77965",
> + "renesas,rcar-gen3-can";
> + reg = <0 0xe6c38000 0 0x1000>;
> + interrupts = ;
> + clocks = < CPG_MOD 915>,
> +< CPG_CORE R8A77965_CLK_CANFD>,
> +<_clk>;
> + clock-names = "clkp1", "clkp2", "can_clk";
> + assigned-clocks = < CPG_CORE R8A77965_CLK_CANFD>;
> + assigned-clock-rates = <4000>;
Same here of course.
> + power-domains = < R8A77965_PD_ALWAYS_ON>;
> + resets = < 915>;
> + status = "disabled";
> + };
> +
> pwm0: pwm@e6e3 {
> compatible = "renesas,pwm-r8a77965", "renesas,pwm-rcar";
> reg = <0 0xe6e3 0 8>;
>
[PATCH 12/14] arm64: dts: renesas: r8a77965: add CAN support
According to R-Car Gen3 HW manual rev0.55E, R-Car M3-N has two CAN
interfaces, similar to H3, M3-W and other SoCs from the same family.
Add CAN nodes to avoid below r8a77965-ulcb-kf.dtb build failure:
Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:19.1-6 Label or path can0 not
found
Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:25.1-6 Label or path can1 not
found
CAN support is inspired from below commits:
- v4.7 commit 308b7e4ba62e ("arm64: dts: r8a7795: Add CAN support")
- v4.11 commit 909c16252415 ("arm64: dts: r8a7796: Add CAN support")
- v4.12 commit bec0948e810f ("arm64: dts: r8a7796: Add reset control
properties")
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/r8a77965.dtsi | 32
1 file changed, 32 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 486aecacb22a..cb8f8573d9ef 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -656,6 +656,38 @@
status = "disabled";
};
+ can0: can@e6c3 {
+ compatible = "renesas,can-r8a77965",
+"renesas,rcar-gen3-can";
+ reg = <0 0xe6c3 0 0x1000>;
+ interrupts = ;
+ clocks = < CPG_MOD 916>,
+ < CPG_CORE R8A77965_CLK_CANFD>,
+ <_clk>;
+ clock-names = "clkp1", "clkp2", "can_clk";
+ assigned-clocks = < CPG_CORE R8A77965_CLK_CANFD>;
+ assigned-clock-rates = <4000>;
+ power-domains = < R8A77965_PD_ALWAYS_ON>;
+ resets = < 916>;
+ status = "disabled";
+ };
+
+ can1: can@e6c38000 {
+ compatible = "renesas,can-r8a77965",
+"renesas,rcar-gen3-can";
+ reg = <0 0xe6c38000 0 0x1000>;
+ interrupts = ;
+ clocks = < CPG_MOD 915>,
+ < CPG_CORE R8A77965_CLK_CANFD>,
+ <_clk>;
+ clock-names = "clkp1", "clkp2", "can_clk";
+ assigned-clocks = < CPG_CORE R8A77965_CLK_CANFD>;
+ assigned-clock-rates = <4000>;
+ power-domains = < R8A77965_PD_ALWAYS_ON>;
+ resets = < 915>;
+ status = "disabled";
+ };
+
pwm0: pwm@e6e3 {
compatible = "renesas,pwm-r8a77965", "renesas,pwm-rcar";
reg = <0 0xe6e3 0 8>;
--
2.18.0
