RE: [PATCH 1/2] ARM: dts: r9a06g032: Add CAN{0,1} nodes

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

 



Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 1/2] ARM: dts: r9a06g032: Add CAN{0,1} nodes
> 
> Hi Biju,
> 
> On Fri, Jul 1, 2022 at 6:23 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > Add CAN{0,1} nodes to R9A06G032 (RZ/N1) SoC DTSI.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm/boot/dts/r9a06g032.dtsi
> > +++ b/arch/arm/boot/dts/r9a06g032.dtsi
> > @@ -423,6 +423,24 @@ gic: interrupt-controller@44101000 {
> >                         interrupts =
> >                                 <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(2) |
> IRQ_TYPE_LEVEL_HIGH)>;
> >                 };
> > +
> > +               can0: can@52104000 {
> > +                       compatible = "nxp,sja1000";
> 
> Is this block 100% compatible to the nxp,sja1000 block, or do we need an
> SoC-specific compatible value?

You are right, We need Renesas SoC specific compatible value to accommodate the below features

1) No clock divider register (CDR) support.
2) No HW loopback(HW doesn't see tx messages on rx).
3) We can use the clk to get the rate  

> 
> > +                       reg = <0x52104000 0x800>;
> > +                       reg-io-width = <4>;
> > +                       interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
> > +                       clocks = <&sysctrl R9A06G032_HCLK_CAN0>;
> 
> According to the (old) bindings, the clock rate is specified using the
> non-standard "nxp,external-clock-frequency property" (seems like both
> bindings and driver can use some overhaul), and defaults to 16 MHz.
> According to the RZ/N1S documentation, the CAN clock is 48 MHz?

Same for RZ/N1D. May be using SoC specific compatible as suggested above, we can use the get rate to get the clock similar to [1]?? Or set default of 48MHz for Renesas SoC?? 

[1] https://github.com/renesas-rz/rzn1_linux/commit/88817d783658bdf6cccfc1a6e8ad414ad7a7c177

Cheers,
Biju

> 
> > +                       status = "disabled";
> > +               };
> > +
> > +               can1: can@52105000 {
> > +                       compatible = "nxp,sja1000";
> > +                       reg = <0x52105000 0x800>;
> > +                       reg-io-width = <4>;
> > +                       interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
> > +                       clocks = <&sysctrl R9A06G032_HCLK_CAN1>;
> > +                       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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux