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 <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.
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 = <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.
Well - I certainly have no complaints about functionality which isn't
present :)
--
Regards
Kieran
>
>>
>>> + 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.
>
