linux-kernel - Re: [PATCH v4 1/2] char: xillybus: Move class-related functions to new xillybus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YGBspAmjcIO6agIm@kroah.com>
Date:   Sun, 28 Mar 2021 13:46:44 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Eli Billauer <eli.billauer@...il.com>
Cc:     devel@...verdev.osuosl.org, linux-usb@...r.kernel.org,
        arnd@...db.de, linux-kernel@...r.kernel.org,
        dan.carpenter@...cle.com
Subject: Re: [PATCH v4 1/2] char: xillybus: Move class-related functions to
 new xillybus_class.c

On Tue, Mar 23, 2021 at 02:05:23PM +0200, Eli Billauer wrote:
> On 22/03/21 13:11, Greg KH wrote:
> > 
> > > XILLYBUS and XILLYBUS_PCIE are currently enabled as M in several Linux
> > > distributions. Making them depend on, rather than select XILLYBUS_CLASS is
> > > likely to disable the driver in those distributions.
> > That's not an issue here, depends-on will allow those distros to also
> > enable this option.
> > 
> > But wait, why is this a separate option at all?  Shouldn't the class
> > code just be part of the "core" XILLYBUS code anyway?
> > 
> I'll try to explain the whole picture:
> 
>            XILLYBUS_CLASS
>               /        \
>              |          |
>          XILLYBUS   XILLYUSB
>       /           \
>      |             |
> XILLYBUS_PCIE  XILLYBUS_OF
> 
> XILLYBUS_CLASS is new and common to all drivers in this group.
> 
> XILLYBUS is for drivers based upon memory registers + DMA-based interfaces,
> and it's combined with XILLYBUS_PCIE and/or XILLYBUS_OF. XILLYUSB is for the
> USB variant only.
> 
> Or a more detailed, bottom-up outline:
> 
> * CONFIG_XILLYBUS_PCIE -> xillybus_pcie.c: Functions related to PCIe.
> * CONFIG_XILLYBUS_OF -> xillybus_of.c: Functions related to Xillybus as a
> peripheral on an FPGA / Processor combo chip.
> * CONFIG_XILLYBUS -> xillybus_core.c: Functions that are common to the two
> above, mainly access to the peripheral with memory-mapped registers and DMA.
> * CONFIG_XILLYUSB -> xillyusb.c: The driver for the USB variant, accesses
> the peripheral through the USB framework.
> * CONFIG_XILLYBUS_CLASS -> xillybus_class.c: The new module, which contains
> the class and API parts that would otherwise appear both in xillybus_core.c
> and xillyusb.c. Contains utility functions for the two latter.
> 
> Because XILLYBUS_CLASS is new and "N" by default, making a "depends on"
> relationship means that "make olddefconfig" silently turns off
> CONFIG_XILLYBUS and CONFIG_XILLYBUS_PCIE. That's a bug: Adding a new driver
> shouldn't change anything in the existing configuration.
> 
> That's why I had the "select XILLYBUS_CLASS" to begin with: It ensures a
> smooth transition on a kernel upgrade, by adding XILLYBUS_CLASS to kernels
> that had CONFIG_XILLYBUS enabled. Is there another way to prevent the said
> bug, without "select"?

Ok, that explains it a bit better, thanks for taking the time to explain
it.  At first glance it seems odd, but this looks better, if you include
this information in the changelog texts for your patches when you send
the next series, that will help out with the review.

thanks,

greg k-h