Re: [PATCH v2 5/7] arm64: dts: renesas: r8a77965: Add CAN{0,1} placeholder nodes

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

 



Hi Simon, hi Geert,

On Mon, Aug 27, 2018 at 02:44:47PM +0200, Simon Horman wrote:
> On Thu, Aug 23, 2018 at 10:52:09AM +0200, Geert Uytterhoeven wrote:
> > On Fri, Aug 17, 2018 at 3:53 PM Kieran Bingham
> > <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> wrote:
> > > On 12/08/18 14:31, Eugeniu Rosca wrote:
> > > > According to R-Car Gen3 HW manual rev1.00, R-Car M3-N has two CAN
> > > > interfaces, similar to H3, M3-W and other SoCs from the same family.
> > > >
> > > > Add CAN placeholder nodes to avoid below DTC errors:
> > > > 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
> > > >
> > > > These errors occur *after* the addition of r8a77965-m3nulcb-kf.dts.
> > > > Fix them beforehand.
> > > >
> > > > 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>
> > >
> > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> > >
> > >
> > > > ---
> > > > Changes in v2:
> > > >  - [Kieran Bingham] Improved commit description:
> > > >    - Referenced the newer HW manual rev1.00 instead of rev0.55E.
> > > >    - Kept the "true story" behind the patch. Just made it more clear.
> > > >  - [Geert Uytterhoeven] Replaced CAN0 and CAN1 nodes with placeholders
> > > >    (no CAN testing was done to validate the DTS configuration).
> > > > ---
> > > >  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 16 ++++++++++++++++
> > > >  1 file changed, 16 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > > > index 486aecacb22a..4da479d3c226 100644
> > > > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > > > @@ -656,6 +656,22 @@
> > > >                       status = "disabled";
> > > >               };
> > > >
> > > > +             can0: can@e6c30000 {
> > > > +                     compatible = "renesas,can-r8a77965",
> > > > +                                  "renesas,rcar-gen3-can";
> > > > +                     reg = <0 0xe6c30000 0 0x1000>;
> > > > +                     /* placeholder */
> > > > +                     status = "disabled";
> > > > +             };
> > >
> > > This is probably more detail than is needed for a placeholder, but it
> > > looks correct so I think this is fine.
> > 
> > Indeed. Adding the "compatible" properties means they're no longer
> > placeholders, and will be probed by the driver, possibly leading to
> > undefined behavior.
> > 
> > Hence please limit the placeholders to the absolute required minimum,
> > and thus drop the "compatible" and "status" properties.
> 
> Agreed, I will update the patch accordingly.

My understanding is that Simon will drop the compatible strings and no
action is needed from my end. Let me know otherwise.

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