| Message ID | 20220830164518.1381632-4-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add RZ/N1 CAN support | expand |
Hi Biju, On Tue, Aug 30, 2022 at 6:45 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > Enable CAN{0,1} on RZ/N1D-DB board. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v2->v3: > * No change Thanks for your patch! > --- a/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts > +++ b/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts > @@ -26,6 +26,20 @@ aliases { > }; > }; > > +&can0 { > + pinctrl-0 = <&pins_can0>; > + pinctrl-names = "default"; > + > + status = "okay"; > +}; > + > +&can1 { > + pinctrl-0 = <&pins_can1>; > + pinctrl-names = "default"; > + > + status = "okay"; > +}; According to the schematics and board documentation, only a single CAN connector is present, and the CAN interface to use must be selected using the CN10/CN11 jumpers. Hence I think we need a #define and an #ifdef to configure this, or at least keep one interface disabled, and add a comment explaining why. The rest LGTM. 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
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v3 3/3] ARM: dts: r9a06g032-rzn1d400-db: Enable > CAN{0,1} > > Hi Biju, > > On Tue, Aug 30, 2022 at 6:45 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > Enable CAN{0,1} on RZ/N1D-DB board. My bad, it is RZ/N1D-DB CPU board fitted to RZ/N1-EB carrier board. Actually it enables CAN{0,1} on the carrier board. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v2->v3: > > * No change > > Thanks for your patch! > > > --- a/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts > > +++ b/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts > > @@ -26,6 +26,20 @@ aliases { > > }; > > }; > > > > +&can0 { > > + pinctrl-0 = <&pins_can0>; > > + pinctrl-names = "default"; > > + > > + status = "okay"; > > +}; > > + > > +&can1 { > > + pinctrl-0 = <&pins_can1>; > > + pinctrl-names = "default"; > > + > > + status = "okay"; > > +}; > > According to the schematics and board documentation, only a single CAN See above, RZ/N1-EB schematics has both connectors?? > connector is present, and the CAN interface to use must be selected > using the CN10/CN11 jumpers. Hence I think we need a #define and an > #ifdef to configure this, or at least keep one interface disabled, and > add a comment explaining why. Our BSP release, by default enables both the CAN interfaces(CN10/CN11) jumpers. I have a RZ/N1-EB carrier board and tested CAN loopback on these interfaces. Cheers, biju
Hi Geert, > Subject: RE: [PATCH v3 3/3] ARM: dts: r9a06g032-rzn1d400-db: Enable > CAN{0,1} > > Hi Geert, > > Thanks for the feedback. > > > Subject: Re: [PATCH v3 3/3] ARM: dts: r9a06g032-rzn1d400-db: Enable > > CAN{0,1} > > > > Hi Biju, > > > > On Tue, Aug 30, 2022 at 6:45 PM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > Enable CAN{0,1} on RZ/N1D-DB board. > > My bad, it is RZ/N1D-DB CPU board fitted to RZ/N1-EB carrier board. > Actually it enables CAN{0,1} on the carrier board. > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- > > > v2->v3: > > > * No change > > > > Thanks for your patch! > > > > > --- a/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts > > > +++ b/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts > > > @@ -26,6 +26,20 @@ aliases { > > > }; > > > }; > > > > > > +&can0 { > > > + pinctrl-0 = <&pins_can0>; > > > + pinctrl-names = "default"; > > > + > > > + status = "okay"; > > > +}; > > > + > > > +&can1 { > > > + pinctrl-0 = <&pins_can1>; > > > + pinctrl-names = "default"; > > > + > > > + status = "okay"; > > > +}; > > > > According to the schematics and board documentation, only a single CAN > > See above, RZ/N1-EB schematics has both connectors?? > > > connector is present, and the CAN interface to use must be selected > > using the CN10/CN11 jumpers. Hence I think we need a #define and an > > #ifdef to configure this, or at least keep one interface disabled, and > > add a comment explaining why. OK, To avoid confusion, will guard enabling CAN1 with #define macro as cpu schematic doesn't mention about this. Cheers, Biju > > Our BSP release, by default enables both the CAN interfaces(CN10/CN11) > jumpers. > I have a RZ/N1-EB carrier board and tested CAN loopback on these > interfaces. > > Cheers, > biju >
Hi Biju, On Thu, Sep 1, 2022 at 3:15 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: Re: [PATCH v3 3/3] ARM: dts: r9a06g032-rzn1d400-db: Enable > > CAN{0,1} > > > On Tue, Aug 30, 2022 at 6:45 PM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > Enable CAN{0,1} on RZ/N1D-DB board. > > My bad, it is RZ/N1D-DB CPU board fitted to RZ/N1-EB carrier board. > Actually it enables CAN{0,1} on the carrier board. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- a/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts > > > +++ b/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts > > > @@ -26,6 +26,20 @@ aliases { > > > }; > > > }; > > > > > > +&can0 { > > > + pinctrl-0 = <&pins_can0>; > > > + pinctrl-names = "default"; > > > + > > > + status = "okay"; > > > +}; > > > + > > > +&can1 { > > > + pinctrl-0 = <&pins_can1>; > > > + pinctrl-names = "default"; > > > + > > > + status = "okay"; > > > +}; > > > > According to the schematics and board documentation, only a single CAN > > See above, RZ/N1-EB schematics has both connectors?? AFAIU, CN10 and CN11 are not the real CAN connectors, they are headers to add jumpers to select which CAN interface to route to the real CAN connector J16. > > connector is present, and the CAN interface to use must be selected > > using the CN10/CN11 jumpers. Hence I think we need a #define and an > > #ifdef to configure this, or at least keep one interface disabled, and > > add a comment explaining why. > > Our BSP release, by default enables both the CAN interfaces(CN10/CN11) jumpers. > I have a RZ/N1-EB carrier board and tested CAN loopback on these interfaces. Yeah, you can enable loopback by wiring CN10 and CN11 appropriately ;-) But that's not intended for normal use. 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
diff --git a/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts b/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts index 4bf813335e21..49104c73eca3 100644 --- a/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts +++ b/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts @@ -26,6 +26,20 @@ aliases { }; }; +&can0 { + pinctrl-0 = <&pins_can0>; + pinctrl-names = "default"; + + status = "okay"; +}; + +&can1 { + pinctrl-0 = <&pins_can1>; + pinctrl-names = "default"; + + status = "okay"; +}; + ð_miic { status = "okay"; renesas,miic-switch-portin = <MIIC_GMAC2_PORT>; @@ -52,6 +66,18 @@ &mii_conv5 { }; &pinctrl{ + pins_can0: pins_can0 { + pinmux = <RZN1_PINMUX(162, RZN1_FUNC_CAN)>, /* CAN0_TXD */ + <RZN1_PINMUX(163, RZN1_FUNC_CAN)>; /* CAN0_RXD */ + drive-strength = <6>; + }; + + pins_can1: pins_can1 { + pinmux = <RZN1_PINMUX(109, RZN1_FUNC_CAN)>, /* CAN1_TXD */ + <RZN1_PINMUX(110, RZN1_FUNC_CAN)>; /* CAN1_RXD */ + drive-strength = <6>; + }; + pins_eth3: pins_eth3 { pinmux = <RZN1_PINMUX(36, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>, <RZN1_PINMUX(37, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
Enable CAN{0,1} on RZ/N1D-DB board. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v2->v3: * No change v1->v2: * No change --- arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts | 26 +++++++++++++++++++++ 1 file changed, 26 insertions(+)