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;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-431-g1d6a3ebb56-fm-20230511.001-g1d6a3ebb
Mime-Version: 1.0
Message-Id: <4e291030-99d9-4b8b-9389-9b8f2560b8e8@app.fastmail.com>
In-Reply-To: <2023051643-overtime-unbridle-7cdd@gregkh>
References: <20230516110038.2413224-1-schnelle@linux.ibm.com>
 <20230516110038.2413224-36-schnelle@linux.ibm.com>
 <2023051643-overtime-unbridle-7cdd@gregkh>
Date:   Tue, 16 May 2023 18:44:34 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
        "Niklas Schnelle" <schnelle@linux.ibm.com>
Cc:     "Alan Stern" <stern@rowland.harvard.edu>,
        "Bjorn Helgaas" <bhelgaas@google.com>,
        =?UTF-8?Q?Uwe_Kleine-K=C3=B6nig?= <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
Subject: Re: [PATCH v4 35/41] usb: uhci: handle HAS_IOPORT dependencies
Content-Type: text/plain
Precedence: bulk

On Tue, May 16, 2023, at 18:29, Greg Kroah-Hartman wrote:
> On Tue, May 16, 2023 at 01:00:31PM +0200, Niklas Schnelle wrote:

>>  #ifndef CONFIG_USB_UHCI_SUPPORT_NON_PCI_HC
>>  /* Support PCI only */
>>  static inline u32 uhci_readl(const struct uhci_hcd *uhci, int reg)
>>  {
>> -	return inl(uhci->io_addr + reg);
>> +	return UHCI_IN(inl(uhci->io_addr + reg));
>>  }
>>  
>>  static inline void uhci_writel(const struct uhci_hcd *uhci, u32 val, int reg)
>>  {
>> -	outl(val, uhci->io_addr + reg);
>> +	UHCI_OUT(outl(val, uhci->io_addr + reg));
>
> I'm confused now.
>
> So if CONFIG_HAS_IOPORT is enabled, wonderful, all is good.
>
> 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?
>
> Why not just not build the driver at all if this option is not enabled?

If I remember correctly, the problem here is the lack of
abstractions in the uhci driver, it instead supports all
combinations of on-chip non-PCI devices using readb()/writeb()
and PCI devices using inb()/outb() in a shared codebase.

A particularly tricky combination is a kernel that supports on-chip
UHCI as well as CONFIG_USB_PCI (for EHCI/XHCI) but does not support
I/O ports because of platform limitations. The trick is to come up
with a set of changes that doesn't have to rewrite the entire logic
but also doesn't add an obscene number of #ifdef checks.

That said, there is a minor problem with the empty definition

+#define UHCI_OUT(x)

I think this should be "do { } while (0)" to avoid warnings
about empty if/else blocks.

    Arnd