linux-kernel - Re: [PATCH v2] ARM: dts: imx: Add Y Soft IOTA Draco, Hydra and Ursa boards

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:   Tue, 8 Jan 2019 11:40:51 +0000
From:   Vokáč Michal <Michal.Vokac@...ft.com>
To:     Fabio Estevam <festevam@...il.com>
CC:     Rob Herring <robh+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Andrew Lunn <andrew@...n.ch>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Fabio Estevam <fabio.estevam@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2] ARM: dts: imx: Add Y Soft IOTA Draco, Hydra and Ursa
 boards

On 28.12.2018 19:20, Fabio Estevam wrote:
> On Tue, Dec 18, 2018 at 12:42 PM Vokáč Michal <Michal.Vokac@...ft.com> wrote:
> 
>> +&fec {
>> +       pinctrl-names = "default";
>> +       pinctrl-0 = <&pinctrl_enet>;
>> +       phy-mode = "rgmii-id";
>> +       phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_HIGH>;
> 
> Are you sure this is active high?
> 
> The FEC driver ignores the polarity of phy-reset-gpios and it assumes
> the GPIO reset as active low.
> 
> If it really is active high, then you would need to pass the
> "phy-reset-active-high" property.

Good catch! Originally it was phy-reset-gpios = <&gpio1 25 0> which is
active high as well. And as you say, the polarity is ignored. Later on
I decided to change all the GPIO specifiers to use defines instead of
numbers. I just swapped 0 for GPIO_ACTIVE_HIGH without using brain cells.

I will use GPIO_ACTIVE_LOW there to be correct even though it is ignored.

>> +/ {
>> +       model = "Y Soft IOTA Draco i.MX6Solo board";
>> +       compatible = "ysoft,imx6dl-yapp4-draco", "fsl,imx6dl";
>> +
>> +       cpus {
>> +               /delete-node/ cpu@1;
> 
> Is this really needed? We don't usually do this on imx6solo dts board files.
> 
> Only one CPU will be brought up online on imx6solo anyway.

OK, it is really not necessary. I will remove it.

>> +       };
>> +
>> +       memory@...00000 {
> 
> Need to pass "device_type = "memory";"

OK.

>> +/ {
>> +       model = "Y Soft IOTA Hydra i.MX6DualLite board";
>> +       compatible = "ysoft,imx6dl-yapp4-hydra", "fsl,imx6dl";
>> +
>> +       memory@...00000 {
> 
> Need to pass "device_type = "memory";"

OK.

>> +       memory@...00000 {
> 
> Need to pass "device_type = "memory";"

OK.

Thank you for the review Fabio.
All the best,
Michal