Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 2 Nov 2021 17:44:15 +0000
From:   Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     Rob Herring <robh@kernel.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        linuxarm@huawei.com, mauro.chehab@huawei.com,
        Krzysztof =?UTF-8?B?V2lsY3p5xYRza2k=?= <kw@linux.com>,
        Binghui Wang <wangbinghui@hisilicon.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Xiaowei Song <songxiaowei@hisilicon.com>,
        linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
        Kishon Vijay Abraham I <kishon@ti.com>
Subject: Re: [PATCH v15 04/13] PCI: kirin: Add support for bridge slot DT
 schema
Message-ID: <20211102174415.58cd3d4f@sal.lan>
In-Reply-To: <20211102160612.GA612467@bhelgaas>
References: <bb391a0e0f0863b66e645048315fab1a4f63f277.1634812676.git.mchehab+huawei@kernel.org>
        <20211102160612.GA612467@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Bjorn,

Em Tue, 2 Nov 2021 11:06:12 -0500
Bjorn Helgaas <helgaas@kernel.org> escreveu:

> > +
> > +	/* Per-slot clkreq */
> > +	int		n_gpio_clkreq;
> > +	int		gpio_id_clkreq[MAX_PCI_SLOTS];
> > +	const char	*clkreq_names[MAX_PCI_SLOTS];  
> 
> I think there's been previous discussion about this, but I didn't
> follow it, so I'm just double-checking that this is what we want here.
> 
> IIUC, this (MAX_PCI_SLOTS, "hisilicon,clken-gpios") applies to an
> external PEX 8606 bridge, which seems a little strange to be
> hard-coded into the kirin driver this way.
> 
> I see that "hisilicon,clken-gpios" is optional, but what if some
> platform connects all 6 lanes?  What if there's a different bridge
> altogether?
> 
> I'll assume this is actually the way we want thing unless I hear
> otherwise.

Yes, there was past discussions about that with Rob, with regards
to how the DT would represent it, which got reflected at the code.
At the end, it was decided to just add a single property inside PCIe:


		pcie@f4000000 {
                        compatible = "hisilicon,kirin970-pcie";
...
                        hisilicon,clken-gpios = <&gpio27 3 0>, <&gpio17 0 0>,
                                                <&gpio20 6 0>;

I don't think this is a problem, as, if some day another bridge would
need a larger number of slots, it is just a matter of changing the
number at the MAX_PCI_SLOTS, as this controls only the size of the array
(and the check for array overflow when parsing the properties).

Regards,
Mauro