Re: [PATCH v4 05/41] counter: add HAS_IOPORT dependencies

From: William Breathitt Gray <william.gray@linaro.org>
To: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: "Arnd Bergmann" <arnd@arndb.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Alan Stern" <stern@rowland.harvard.edu>,
	"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@vger.kernel.org,
	linux-pci@vger.kernel.org, "Arnd Bergmann" <arnd@kernel.org>,
	linux-iio@vger.kernel.org
Subject: Re: [PATCH v4 05/41] counter: add HAS_IOPORT dependencies
Date: Fri, 19 May 2023 10:21:08 -0400	[thread overview]
Message-ID: <ZGeF1K0Yxu9lTgN2@fedora> (raw)
In-Reply-To: <abc02dc2af7563ae26bf0d0ddd927d9b4a21dda3.camel@linux.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 2071 bytes --]

On Fri, May 19, 2023 at 03:39:57PM +0200, Niklas Schnelle wrote:
> On Fri, 2023-05-19 at 15:38 +0200, Niklas Schnelle wrote:
> > On Fri, 2023-05-19 at 15:17 +0200, Niklas Schnelle wrote:
> > > On Thu, 2023-05-18 at 21:26 -0400, William Breathitt Gray wrote:
> > > > On Tue, May 16, 2023 at 01:00:01PM +0200, Niklas Schnelle wrote:
> > > > > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > > > > not being declared. We thus need to add HAS_IOPORT as dependency for
> > > > > those drivers using them.
> > > > > 
> > > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > > > > Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> > > > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > > > 
> > > > Hi Niklas,
> > > > 
> > > > The change itself is fine, but please update the description to reflect
> > > > that this is adding a depends on HAS_IOPORT_MAP rather than HAS_IOPORT,
> > > > along with the reason why it's needed (i.e. devm_ioport_map() is used).
> > > > 
> > > > Thanks,
> > > > 
> > > > William Breathitt Gray
> > > > 
> > > > 
> > > 
> > > Right, this clearly needs adjustment. I went with the following commit
> > > message for v5:
> > > 
> > > "counter: add HAS_IOPORT_MAP dependency
> > > 
> > > The 104_QUAD_8 counter driver uses devm_ioport_map() without depending
> > > on HAS_IOPORT_MAP. This causes compilation to fail on platforms such as
> > > s390 which do not support I/O port mapping. Add the missing
> > > HAS_IOPORT_MAP dependency to fix this."
> > > 
> > 
> > Just noticed this isn't entirely correct. As devm_ioport_map() has an
> > empty stub for HAS_IOPORT_MAP=n this doesn't lead to a compile error it
> > just doesn't work. Will reword to "This causes the driver to not be
> > useable on platforms ..."
> 
> s/useable/usable/

104_QUAD_8 has an explicit dependency on PC104 and X86, so I don't think
it would ever be used outside of x86 platforms. Does it still make sense
to have the HAS_IOPORT_MAP dependency in this case?

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]