Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <1552467452-538-1-git-send-email-hongxing.zhu@nxp.com>
In-Reply-To: <1552467452-538-1-git-send-email-hongxing.zhu@nxp.com>
From:   Andrey Smirnov <andrew.smirnov@gmail.com>
Date:   Thu, 14 Mar 2019 19:26:18 -0700
Message-ID: <CAHQ1cqFK36Qh8BW-0Nd+rZra2m1=et7aAWBwD-GkLU4VwvOAWg@mail.gmail.com>
Subject: Re: [RFC 1/2] dt-bindings: imx6q-pcie: Add support for i.MX8QM/QXP PCIe
To:     Richard Zhu <hongxing.zhu@nxp.com>
Cc:     "bhelgaas@google.com" <bhelgaas@google.com>,
        "lorenzo.pieralisi@arm.com" <lorenzo.pieralisi@arm.com>,
        "l.stach@pengutronix.de" <l.stach@pengutronix.de>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Mar 13, 2019 at 2:15 AM Richard Zhu <hongxing.zhu@nxp.com> wrote:
>
> Add codes needed to support i.MX8QM/QXP PCIe.
> - HSIO(High Speed IO) subsystem is new defined on i.MX8QM/QXP.
>   The PCIe and SATA modules are contained in the HSIO subsystem. There
>   are two PCIe, one SATA controllers and three mixed lane PHYs on
>   i.MX8QM. There are three use cases of the HSIO subsystem on i.MX8QM.
>   1. PCIea 2 lanes and one SATA AHCI port.
>   2. PCIea 1 lane, PCIeb 1 lane and one SATA AHCI port.
>   3. PCIea 2 lanes, PCIeb 1 lane.
>   i.MX8QXP only has PCIeb controller and one lane PHY.
>   Use the hsio-cfg property to specify the different modes.
> - The HSIO address map as viewed from system level is as shown below.
>   address [31:24]    Local address    Target    Address Size
>   5F                 0                HSIO      16MB
>   60-6F              40-4F            HSIO      256MB
>   70-7F              80-8F            HSIO      256MB
>   The property local-addr is required to specify it.
> - Both external OSC and internal PLL can be used as PCIe reference
>   clock, use the ext_osc property to distinguish them.
> - clock request GPIO for controlling the PCI reference clock request
>   signal. And should be configure OD when L1SS maybe enabled later.
> - One more power domain HSIO_GPIO and clock PCIE_PER are required by
>   i.MX8QM/QXP PCIe.
>   Add these specific properties to enable i.MX8QM/QXP PCIe.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt      | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index a7f5f5a..f7586c9 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -10,6 +10,8 @@ Required properties:
>         - "fsl,imx6qp-pcie"
>         - "fsl,imx7d-pcie"
>         - "fsl,imx8mq-pcie"
> +       - "fsl,imx8qm-pcie"
> +       - "fsl,imx8qxp-pcie"
>  - reg: base address and length of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> @@ -38,6 +40,10 @@ Optional properties:
>    The regulator will be enabled when initializing the PCIe host and
>    disabled either as part of the init process or when shutting down the
>    host.
> +- clkreq-gpio: Should specify the GPIO for controlling the PCI reference clock
> +  request signal.
> +- ext_osc: External OSC is used as PCIe reference clock or not. 0: Internal
> +  PLL. 1: External OSC.

Forgot to mention one thing in my very first reply, so I'll mention it
here. I think figuring out the way to add support for external vs
internal reference bus clock (that "ext_osc" binding above) is going
to be a whole other discussion. It might be easier/more expedient to
focus on just one particular use-case first (say external oscillator
only), get it all done and accepted and then return to this particular
feature in a separate series/discussion. However, that's just how I'd
approach this, so, please don't feel compelled to do it that way just
because I suggested it.

Thanks,
Andrey Smirnov