Re: [PATCH 12/14] arm64: dts: renesas: r8a77965: add CAN support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <erosca@xxxxxxxxxxxxxx>
> > ---
> >  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.



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux