linux-kernel - Re: [PATCH v4 6/7] ARM: dts: Add support for emtrion emCON-MX6 series

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 4 May 2018 15:03:40 +0800
From:   Shawn Guo <shawnguo@...nel.org>
To:     Türk, Jan <Jan.Tuerk@...rion.de>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Thierry Reding <thierry.reding@...il.com>,
        David Airlie <airlied@...ux.ie>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <fabio.estevam@....com>,
        Russell King <linux@...linux.org.uk>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        LinuxArmKernelMailingListe <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v4 6/7] ARM: dts: Add support for emtrion emCON-MX6 series

On Thu, May 03, 2018 at 12:00:06PM +0200, Türk, Jan wrote:
> > > +/ {
> > > +   aliases {
> > > +           mmc0 = &usdhc3;
> > > +           mmc2 = &usdhc1;
> > > +           mmc1 = &usdhc2;
> > > +           mmc3 = &usdhc4;
> > > +           boardID = &boardID;
> >
> > Why do you need this boardID alias?
> I wanted to have a generic entry point to address the board-id on all CPU-modules with their different SoC's resulting in different devicetree paths.
> Also as it now has the generic "gpio@3a" name, there would be no other way in differencing the board Identifying GPIO-expander from an additionally
> attached one (except platform code etc.)
> With the alias every code could look up the information required over the alias path with the same piece of code.

Okay.  But no uppercase please.  Otherwise, you introduce the following
DTC warnings (with W=1 switch) we are trying to clean up.

 Warning (alias_paths): /aliases: aliases property name must include only lowercase and '-'

> >
> > > +   };

<snip>

> > > &pinctrl_nor_flash
> > > +            &pinctrl_usdhc2
> > > +            &pinctrl_spdif_out     &pinctrl_spdif_in
> > > +            &pinctrl_cpi1          &pinctrl_cpi2
> > > +           >;
> >
> > Again, please do not put consumer device specific pins in hog group.
> > Also, it's confusing to have the same pinctrl in both hog and consumer device
> > node, like pinctrl_nor_flash and pinctrl_usdhc2 here.
> 
> About pinctrl_nor_flash I fully agree, that's a mistake.
> pinctrl_usdhc2 is not connected on the avari, and therefore not enabled.
> As told before it is added to the Hog group to force the default pinmuxing without enabling the hardware itself.

If you want to do such default pinmuxing, please do it in your firmware.
We generally do not want to use hog group too much, because that makes
it difficult to find out which client devices consume which pins.

> 
> >
> > > +};

<snip>

> > > +&i2c1 {
> > > +   clock-frequency = <100000>;
> > > +   pinctrl-names = "default";
> > > +   pinctrl-0 = <&pinctrl_i2c1>;
> > > +   status = "okay";
> > > +
> > > +   rtc: rtc@68 {
> >
> > Is the label actually used?  If yes, I would suggest a more specific name like
> > ds1307.
> Really? Why should it be a generic name for "gpio" and "pmic" but not a generic name for an "rtc" chip?

I'm talking about label not node name.  That said I was suggesting
something like:

	ds1307: rtc@68

> 
> >
> > > +           compatible = "dallas,ds1307";
> > > +           reg = <0x68>;
> > > +   };

Shawn