driverdev-devel.linuxdriverproject.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Eli Billauer <eli.billauer@gmail.com>
Cc: devel@driverdev.osuosl.org, dan.carpenter@oracle.com,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	arnd@arndb.de
Subject: Re: [PATCH v4 1/2] char: xillybus: Move class-related functions to new xillybus_class.c
Date: Sun, 28 Mar 2021 13:46:44 +0200	[thread overview]
Message-ID: <YGBspAmjcIO6agIm@kroah.com> (raw)
In-Reply-To: <6059D983.5080306@gmail.com>

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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

  reply	other threads:[~2021-03-28 11:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-11  9:50 [PATCH v4 0/2] Submission of XillyUSB driver eli.billauer
2021-03-11  9:50 ` [PATCH v4 1/2] char: xillybus: Move class-related functions to new xillybus_class.c eli.billauer
2021-03-21 12:24   ` Greg KH
2021-03-22 11:02     ` Eli Billauer
2021-03-22 11:11       ` Greg KH
2021-03-23 12:05         ` Eli Billauer
2021-03-28 11:46           ` Greg KH [this message]
2021-03-21 12:24   ` Greg KH
2021-03-11  9:50 ` [PATCH v4 2/2] staging: Add driver for XillyUSB (Xillybus variant for USB) eli.billauer
2021-03-21 12:23   ` Greg KH
2021-03-22 11:01     ` Eli Billauer
2021-03-22 11:13       ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YGBspAmjcIO6agIm@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=arnd@arndb.de \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=eli.billauer@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).