public inbox for linux-mmc@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: wwang <wei_wang@realsil.com.cn>
Cc: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"cjb@laptop.org" <cjb@laptop.org>, "bp@alien8.de" <bp@alien8.de>
Subject: Re: [PATCH 1/3] drivers/misc: Add realtek card reader core driver
Date: Fri, 3 Aug 2012 14:39:28 +0000	[thread overview]
Message-ID: <201208031439.28928.arnd@arndb.de> (raw)
In-Reply-To: <501B37F4.80206@realsil.com.cn>

On Friday 03 August 2012, wwang wrote:
> I got your ideas. Bus driver depending on other modules is indeed a bad
> style.
> 
> In our situation, just take pci device for example, pci-common is the
> place to detect card plugged or unplugged, so pci-common is required to
> call and probe sdmmc-pci or memstick-pci. If not using bus driver, I
> need to fulfill my own mechanism, like callback functions and other
> internal data structures, to achieve this goal.

The MFD layer provides some helpers to create "platform" devices for the
child nodes. You can attach all the data you need for those in
the "platform_data" field, including callback pointers if necessary.

> But bus driver can
> easily finish this job. I still prefer to retain bus driver, but detach
> the adapter part from the original bus driver. So the bus driver will
> not depend on other modules any more, and don't need any further
> modification if adding new high-level drivers. I will also move all the
> mmc specific code to sdmmc-pci driver. pci-common only contains the
> generic code related to pci operations. Just like the below diagram:
> 
>    sdmmc-pci      sdmmc-usb        memstick-pci      memstick-usb
>      \   \           /   \             /    \           /   |
>       |   \         /     \           /      \         /    |
>       |    \       /       \___      /        \        |    |
>       \     \ ____/         ___|____/          \_______|    |
>        \     |             /   |                       |    /
>         \____|______      /    |____________      _____|___/
>              |      \    /                  \    /     |   
>              |     pci-common             usb-common   |
>              \              \              /           /
>               \              \            /           /
>                \_____________ \          /___________/
>                              \rtsx-slot-bus/
> 
> 
> And I plan to push sdmmc-pci and sdmmc-usb to drivers/mmc/host, push
> memstick-pci and memstick-usb to drivers/memstick/host, and push
> pci-common, usb-common and rtsx-slot-bus to drivers/mfd/realtek_cr.
> 
> I would like to receive your suggestions. Thank you.

This looks ok, yes. I still don't see the value in having your own
bus_type though, and I believe using a platform device will save
you a significant chunk of code while achieving the same.

In the diagram above, the pci-common and the usb-common modules
each know exactly what their child devices are, so there is little
value creating an device-agnostic abstraction layer beneath it.

Having a module for common stuff in the place of your "rtsx-slot-bus"
is ok to handle stuff like your "ring buffer" that would be needed
by all four devices on the top. But when you introduce infrastructure
like your own bus_type, you should always consider whether that
infrastructure actually does something that is at the same time
common to all of your hardware and different from everything else.
I suspect you will find that your bus_type provide something
specific to realtek card readers and that you can just as well use
the platform bus.

	Arnd

  reply	other threads:[~2012-08-03 14:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-31  7:42 [PATCH 1/3] drivers/misc: Add realtek card reader core driver wei_wang
2012-07-31 11:23 ` Arnd Bergmann
2012-08-01  6:19   ` wwang
2012-08-01 14:31     ` Arnd Bergmann
2012-08-03  2:31       ` wwang
2012-08-03 14:39         ` Arnd Bergmann [this message]
2012-08-13 20:59       ` Maxim Levitsky

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=201208031439.28928.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=cjb@laptop.org \
    --cc=devel@linuxdriverproject.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=wei_wang@realsil.com.cn \
    /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