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 <[email protected]>
> > ---
> > 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@e6c30000 {
> > + compatible = "renesas,can-r8a77965",
> > + "renesas,rcar-gen3-can";
> > + reg = <0 0xe6c30000 0 0x1000>;
> > + interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&cpg CPG_MOD 916>,
> > + <&cpg CPG_CORE R8A77965_CLK_CANFD>,
> > + <&can_clk>;
> > + clock-names = "clkp1", "clkp2", "can_clk";
> > + assigned-clocks = <&cpg CPG_CORE R8A77965_CLK_CANFD>;
> > + assigned-clock-rates = <40000000>;
>
> 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 =
<40000000>;
arch/arm64/boot/dts/renesas/r8a7795.dtsi- assigned-clock-rates =
<40000000>;
arch/arm64/boot/dts/renesas/r8a7796.dtsi- assigned-clock-rates =
<40000000>;
arch/arm64/boot/dts/renesas/r8a7796.dtsi- assigned-clock-rates =
<40000000>;
arch/arm64/boot/dts/renesas/r8a77995.dtsi- assigned-clock-rates =
<40000000>;
arch/arm64/boot/dts/renesas/r8a77995.dtsi- assigned-clock-rates =
<40000000>;
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 = <&sysc R8A77965_PD_ALWAYS_ON>;
> > + resets = <&cpg 916>;
> > + status = "disabled";
> > + };
> > +
> > + can1: can@e6c38000 {
> > + compatible = "renesas,can-r8a77965",
> > + "renesas,rcar-gen3-can";
> > + reg = <0 0xe6c38000 0 0x1000>;
> > + interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&cpg CPG_MOD 915>,
> > + <&cpg CPG_CORE R8A77965_CLK_CANFD>,
> > + <&can_clk>;
> > + clock-names = "clkp1", "clkp2", "can_clk";
> > + assigned-clocks = <&cpg CPG_CORE R8A77965_CLK_CANFD>;
> > + assigned-clock-rates = <40000000>;
>
> Same here of course.
>
>
> > + power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
> > + resets = <&cpg 915>;
> > + status = "disabled";
> > + };
> > +
> > pwm0: pwm@e6e30000 {
> > compatible = "renesas,pwm-r8a77965", "renesas,pwm-rcar";
> > reg = <0 0xe6e30000 0 8>;
> >
>
Thanks,
Eugeniu.
