Re: [PATCH v4 1/2] char: xillybus: Move class-related functions to new xillybus_class.c

From: Greg KH
Date: Sun Mar 28 2021 - 07:49:20 EST


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