lkml.org 
[lkml]   [2023]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 05/41] counter: add HAS_IOPORT dependencies
    On Mon, May 22, 2023 at 12:42:15PM +0200, Niklas Schnelle wrote:
    > On Fri, 2023-05-19 at 10:21 -0400, William Breathitt Gray wrote:
    > > 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
    >
    > Well, yes and no, you're right that it doesn't really cause compile
    > issues despite the "|| COMPILE_TEST" albeit the code could never work.
    > Still, I'd add the dependency. At the very least it serves as
    > documentation and maybe in the future someone will want to remove those
    > empty stubs for HAS_IOPORT_MAP=n.
    >
    > Thanks
    > Niklas

    Sure, that reasoning makes sense to me too, so let's go with the
    explicit depends afterall.

    By the way, I noticed two other modules that call devm_ioport_map() but
    seem to be missing the HAS_IOPORT_MAP depends lines: the
    drivers/iio/addac/stx104.c and drivers/iio/dac/cio-dac.c drivers. Do
    these need respective patches as well?

    As an aside, I haven't been following the previous patchsets closely so
    forgive me if this has already been discussed in another thread: why
    doesn't X86 automatically select HAS_IOPORT? Are there x86 platforms
    that do not support ioport?

    William Breathitt Gray
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2023-05-22 13:23    [W:3.656 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site