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;
Message-ID: <8d5f62aff02f0043c8f601f24c949c5fe03e132e.camel@linux.ibm.com>
Subject: Re: [PATCH v4 35/41] usb: uhci: handle HAS_IOPORT dependencies
From:   Niklas Schnelle <schnelle@linux.ibm.com>
To:     Arnd Bergmann <arnd@arndb.de>,
        Alan Stern <stern@rowland.harvard.edu>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Bjorn Helgaas <bhelgaas@google.com>,
        Uwe =?ISO-8859-1?Q?Kleine-K=F6nig?= 
        <u.kleine-koenig@pengutronix.de>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        linux-kernel@vger.kernel.org,
        Linux-Arch <linux-arch@vger.kernel.org>,
        linux-pci@vger.kernel.org, Arnd Bergmann <arnd@kernel.org>,
        linux-usb@vger.kernel.org
Date:   Fri, 19 May 2023 13:31:12 +0200
In-Reply-To: <440855f4-897c-4597-bbe6-7c5f295f616a@app.fastmail.com>
References: <20230516110038.2413224-1-schnelle@linux.ibm.com>
         <20230516110038.2413224-36-schnelle@linux.ibm.com>
         <2023051643-overtime-unbridle-7cdd@gregkh>
         <2c03973e-0635-4dbb-a1df-bfda8cbee161@rowland.harvard.edu>
         <440855f4-897c-4597-bbe6-7c5f295f616a@app.fastmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.1 (3.48.1-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2023-05-17 at 14:17 +0200, Arnd Bergmann wrote:
> On Tue, May 16, 2023, at 22:17, Alan Stern wrote:
> > On Tue, May 16, 2023 at 06:29:56PM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, May 16, 2023 at 01:00:31PM +0200, Niklas Schnelle wrote:
> >=20
> > > I'm confused now.
> > >=20
> > > So if CONFIG_HAS_IOPORT is enabled, wonderful, all is good.
> > >=20
> > > But if it isn't, then these are just no-ops that do nothing?  So then
> > > the driver will fail to work?  Why have these stubs at all?
> > >=20
> > > Why not just not build the driver at all if this option is not enable=
d?
> >=20
> > I should add something to my previous email.  This particular section o=
f=20
> > code is protected by:
> >=20
> > #ifndef CONFIG_USB_UHCI_SUPPORT_NON_PCI_HC
> > /* Support PCI only */
> >=20
> > So it gets used only in cases where the driver supports just a PCI bus=
=20
> > -- no other sorts of non-PCI on-chip devices.  But the preceding patch=
=20
> > in this series changes the Kconfig file to say:
> >=20
> >  config USB_UHCI_HCD
> > 	tristate "UHCI HCD (most Intel and VIA) support"
> > 	depends on (USB_PCI && HAS_IOPORT) || USB_UHCI_SUPPORT_NON_PCI_HC
> >=20
> > As a result, when the configuration includes support only for PCI=20
> > controllers the driver won't get built unless HAS_IOPORT is set.  Thus=
=20
> > the no-op case (in this part of the code) can't arise.
>=20
> Indeed, that makes sense.
>=20
> > Which is a long-winded way of saying that you're right; the UHCI_IN()=
=20
> > and UHCI_OUT() wrappers aren't needed in this part of the driver.  I=
=20
> > guess Niklas put them in either for consistency with the rest of the=
=20
> > code or because it didn't occur to him that they could be omitted.  (An=
d=20
> > I didn't spot it either.)
>=20
> It's probably less confusing to leave out the PCI-only part of
> the patch then and only modify the generic portion.
>=20
>       Arnd

Yes I agree that way the UHCI_IN/OUT() macro is also only used directly
in combination with uhci_has_pci_registers(). I've done this for v5.

Thanks,
Niklas