Re: [PATCH v2 2/3] scsi: ufs: Modulize ufs-bsg

From: Greg KH
Date: Mon Dec 16 2019 - 13:22:15 EST


On Mon, Dec 16, 2019 at 09:22:21AM -0800, Bart Van Assche wrote:
> On 12/15/19 8:36 PM, cang@xxxxxxxxxxxxxx wrote:
> > On 2019-12-16 05:49, Bart Van Assche wrote:
> > > On 2019-12-11 22:37, Bjorn Andersson wrote:
> > > > It's the asymmetry that I don't like.
> > > >
> > > > Perhaps if you instead make ufshcd platform_device_register_data() the
> > > > bsg device you would solve the probe ordering, the remove will be
> > > > symmetric and module autoloading will work as well (although then you
> > > > need a MODULE_ALIAS of platform:device-name).
> > >
> > > Hi Bjorn,
> > >
> > > From Documentation/driver-api/driver-model/platform.rst:
> > > "Platform devices are devices that typically appear as autonomous
> > > entities in the system. This includes legacy port-based devices and
> > > host bridges to peripheral buses, and most controllers integrated
> > > into system-on-chip platforms.  What they usually have in common
> > > is direct addressing from a CPU bus.  Rarely, a platform_device will
> > > be connected through a segment of some other kind of bus; but its
> > > registers will still be directly addressable."
> > >
> > > Do you agree that the above description is not a good match for the
> > > ufs-bsg kernel module?
> >
> > I missed this one.
> > How about making it a plain device and add it from ufs driver?
>
> Hi Can,
>
> Since the ufs_bsg kernel module already creates one device node under
> /dev/bsg for each UFS host I don't think that we need to create any
> additional device nodes for ufs-bsg devices. My proposal is to modify the
> original patch 2/3 from this series as follows:
> * Use module_init() instead of late_initcall_sync().
> * Remove the ufshcd_get_hba_list_lock() and
> ufshcd_put_hba_list_unlock() functions.
> * Implement a notification mechanism in the UFS core that invokes a
> callback function after an UFS host has been created and also after an
> UFS host has been removed.

You want to be a bus and have a device on it.

> * Register for these notifications from inside the ufs-bsg driver.

You want to be a bus.

> * During registration for notifications, invoke the UFS host creation
> callback function for all known UFS hosts.

You want to be a bus.

> * If the UFS core is unloaded, invoke the UFS host removal callback
> function for all known UFS hosts.

Again, a bus would do all of this for you "for free", right?

Take a look at the crazy "virtual bus" code that the RDMA people are
proposing if you want to try to use something like that, or just take
200 lines of code and be your own bus and devices that hang off of it.
That sounds exactly what you are looking for here.

> I think there are several examples of similar notification mechanisms in the
> Linux kernel, e.g. the probe and remove callback functions in struct
> pci_driver.

Look, a bus! :)

thanks,

greg k-h