Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230601163335.6zw4ojbqxz2ws6vx@skbuf> <ZHjaq+TDW/RFcoxW@bhelgaas>
 <20230601221532.2rfcda4sg5nl7pzp@skbuf> <dc430271-8511-e6e4-041b-ede197e7665d@loongson.cn>
 <7a7f78ae-7fd8-b68d-691c-609a38ab3161@loongson.cn> <20230602101628.jkgq3cmwccgsfb4c@skbuf>
 <87f2b231-2e16-e7b8-963b-fc86c407bc96@loongson.cn> <20230604085500.ioaos3ydehvqq24i@skbuf>
In-Reply-To: <20230604085500.ioaos3ydehvqq24i@skbuf>
From:   Rob Herring <robh@kernel.org>
Date:   Fri, 16 Jun 2023 11:57:43 -0600
Message-ID: <CAL_JsqLsVYiPLx2kcHkDQ4t=hQVCR7NHziDwi9cCFUFhx48Qow@mail.gmail.com>
Subject: Re: [PATCH pci] PCI: don't skip probing entire device if first fn OF
 node has status = "disabled"
To:     Vladimir Oltean <vladimir.oltean@nxp.com>
Cc:     Jianmin Lv <lvjianmin@loongson.cn>,
        Liu Peibao <liupeibao@loongson.cn>,
        Bjorn Helgaas <helgaas@kernel.org>, linux-pci@vger.kernel.org,
        netdev@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Michael Walle <michael@walle.cc>, linux-kernel@vger.kernel.org,
        Binbin Zhou <zhoubinbin@loongson.cn>,
        Huacai Chen <chenhuacai@loongson.cn>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sun, Jun 4, 2023 at 2:55=E2=80=AFAM Vladimir Oltean <vladimir.oltean@nxp=
.com> wrote:
>

Sorry, just now seeing this as I've been out the last month.

> On Sat, Jun 03, 2023 at 10:35:50AM +0800, Jianmin Lv wrote:
> > > How about 3. handle of_device_is_available() in the probe function of
> > > the "loongson, pci-gmac" driver? Would that not work?
> > >
> > This way does work only for the specified device. There are other devic=
es,
> > such as HDA, I2S, etc, which have shared pins. Then we have to add
> > of_device_is_available() checking to those drivers one by one. And we a=
re
> > not sure if there are other devices in new generation chips in future. =
So
> > I'm afraid that the way you mentioned is not suitable for us.

If we decided that disabled devices should probe, then that is exactly
what will have to be done. The restriction (of shared pins) is in the
devices and is potentially per device, so it makes more sense for the
device's drivers to handle than the host bridge IMO. (Assuming the
core doesn't handle a per device property.)


> Got it, so you have more on-chip PCIe devices than the ones listed in
> loongson64-2k1000.dtsi, and you don't want to describe them in the
> device tree just to put status =3D "disabled" for those devices/functions
> that you don't want Linux to use - although you could, and it wouldn't
> be that hard or have unintended side effects.
>
> Though you need to admit, in case you had an on-chip multi-function PCIe
> device like the NXP ENETC, and you wanted Linux to not use function 0,
> the strategy you're suggesting here that is acceptable for Loongson
> would not have worked.
>
> I believe we need a bit of coordination from PCIe and device tree
> maintainers, to suggest what would be the encouraged best practices and
> ways to solve this regression for the ENETC.

I think we need to define what behavior is correct for 'status =3D
"disabled"'. For almost everywhere in DT, it is equivalent to the
device is not present. A not present device doesn't probe. There are
unfortunately cases where status got ignored/forgotten and PCI was one
of those. PCI is a bit different since there are 2 sources of
information about a device being present. The intent with PCI is DT
overrides what's discovered. For example, 'vendor-id' overrides what's
read from the h/w.

I think we can fix making the status per function simply by making
'match_driver' be set based on the status. This would move the check
later to just before probing. That would not work for a case where
accessing the config registers is a problem. It doesn't sound like
that's a problem for Loongson based on the above response, but their
original solution did prevent that. This change would also mean the
PCI quirks would run. Perhaps the func0 memory clearing you need could
be run as a quirk instead?

Rob