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;
Message-ID: <1543313169.2507.39.camel@pengutronix.de>
Subject: Re: [PATCH 3/3] PCI: imx: Add support for i.MX8MQ
From:   Lucas Stach <l.stach@pengutronix.de>
To:     Andrey Smirnov <andrew.smirnov@gmail.com>,
        Leonard Crestez <leonard.crestez@nxp.com>
Cc:     Richard Zhu <hongxing.zhu@nxp.com>, linux-imx@nxp.com,
        Chris Healy <cphealy@gmail.com>,
        Dong Aisheng <aisheng.dong@nxp.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        Fabio Estevam <fabio.estevam@nxp.com>,
        Mark Rutland <mark.rutland@arm.com>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        Bjorn Helgaas <bhelgaas@google.com>, linux-pci@vger.kernel.org
Date:   Tue, 27 Nov 2018 11:06:09 +0100
In-Reply-To: <CAHQ1cqH6LT4GsxUchrmVG1m3fqawgyUjxdHvh=SzxhBiQOd8eg@mail.gmail.com>
References: <20181117181225.10737-1-andrew.smirnov@gmail.com>
         <20181117181225.10737-4-andrew.smirnov@gmail.com>
         <ce2ac1a880ce2a7a39f0e42b5059676daa748e52.camel@nxp.com>
         <CAHQ1cqH6LT4GsxUchrmVG1m3fqawgyUjxdHvh=SzxhBiQOd8eg@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Andrey,

Am Montag, den 26.11.2018, 10:24 -0800 schrieb Andrey Smirnov:
> On Tue, Nov 20, 2018 at 2:49 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
> > 
> > On Sat, 2018-11-17 at 10:12 -0800, Andrey Smirnov wrote:
> > > @@ -921,7 +1004,28 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > > -     case IMX7D:
> > > +     case IMX8MQ:
> > > +             if (of_property_read_u32(node, "fsl,iomux-gpr1x",
> > > +                                      &imx6_pcie->gpr1x)) {
> > > +                     dev_err(dev, "Failed to get GPR1x address\n");
> > > +                     return -EINVAL;
> > > +             }
> > 
> > This is for distinguishing multiple controllers on the SOC but other
> > registers and bits might differ. Isn't it preferable to have a property
> > for controller id instead of adding many registers to DT?
> > 
> 
> I liked encoding necessary info in DT directly slightly better than
> encoding abstract ID and then decoding it further in the driver code.
> OTOH, I am not really attached to that path. Lucas, can you comment on
> this please?

Yes, after rereading the patch with this in mind I agree that having
the GPR offset on DT directly is IMO the better approach than an
abstract ID.

One other thing I noticed is that we probably need some property to
encode if the clock is supplied by an external clkgen, or if the clock
is provided by the i.MX. Hardcoding this in the driver will lead to DT
backward compatibility headaches later on if someone decides to build a
board with the clock provided from the i.MX.

Regards,
Lucas