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]
Message-ID: <CAD=FV=VuGPvUY8edN+PEuZS_pO+=3WHeJ-4J5tHDAPRnXJs0QA@mail.gmail.com>
Date:   Thu, 11 Mar 2021 13:54:38 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Roja Rani Yarubandi <rojay@...eaurora.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        Akash Asthana <akashast@...eaurora.org>,
        msavaliy@....qualcomm.com
Subject: Re: [PATCH] arm64: dts: sc7280: Add qspi, qupv3_0 and qupv3_1 nodes

Hi,

On Wed, Mar 10, 2021 at 7:41 PM Roja Rani Yarubandi
<rojay@...eaurora.org> wrote:
>
> +&qspi_cs0 {
> +       pinconf {
> +               pins = "gpio15";
> +               bias-disable;
> +       };

The "pinconf" / "pinmux" subnode shouldn't be used for new SoCs. See:

http://lore.kernel.org/r/CAD=FV=UY_AFRrAY0tef5jP698LEng6oN652LcX3B4nG=aWh0gA@mail.gmail.com

...same feedback for this whole patch.

> +                       qup_spi0_default: qup-spi0-default {
> +                               pinmux {
> +                                       pins = "gpio0", "gpio1",
> +                                              "gpio2", "gpio3";
> +                                       function = "qup00";
> +                               };
> +                       };

Please split these SPI nodes as per the thread above, like:

tlmm: pinctrl@... {
  qup_spi0_data_clk: qup-spi0-data-clk {
    pins = "gpio0", "gpio1", "gpio2";
    function = "qup0";
  };

  qup_spi0_cs: qup-spi0-cs {
    pins = "gpio3";
    function = "qup0";
  };

  qup_spi0_cs_gpio: qup-spi0-cs-gpio {
    pins = "gpio3";
    function = "gpio";
  };
};


> +                       qup_uart0_default: qup-uart0-default {
> +                               pinmux {
> +                                       pins = "gpio0", "gpio1",
> +                                              "gpio2", "gpio3";
> +                                       function = "qup00";
> +                               };
> +                       };

I suspect things would actually be cleaner if we broke the uart lines
up since the boards tend to have to adjust pulls differently for each
line. With the new "no pinconf / pinmux" world it's pretty clean. It's
obviously up to Bjorn, but if it were me I'd request this in the SoC
file:

qup_uart0_cts: qup-uart0-cts {
  pins = "...";
  function = "qup00";
};

qup_uart0_rts: qup-uart0-rts {
  pins = "...";
  function = "qup00";
};

qup_uart0_rx: qup-uart0-rx {
  pins = "...";
  function = "qup00";
};

qup_uart0_tx: qup-uart0-tx {
  pins = "...";
  function = "qup00";
};

...and now when the board file wants to adjust the pulls they can just
reference each one:

/*
 * Comments about why the UART0 pulls make sense.
 * Blah blah blah.
 */

&qup_uart0_cts {
  bias-pull-down;
};

&qup_uart0_rts {
  drive-strength = <2>;
  bias-disable;
};

&qup_uart0_rx {
  bias-pull-up;
};

&qup_uart0_tx {
  drive-strength = <2>;
  bias-disable;
};


> +               qspi: spi@...c000 {

I believe the qspi node is sorted incorrectly. When I apply this to
the top of the Qualcomm tree it shows up after the "apps_smmu:
iommu@...00000" node. However:

0x088dc000 < 0x15000000

...which means it should be _before_.

-Doug

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ