linux-kernel - Re: [PATCH 1/3] drivers/misc: Add realtek card reader core driver

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <201208031439.28928.arnd@arndb.de>
Date:	Fri, 3 Aug 2012 14:39:28 +0000
From:	Arnd Bergmann <arnd@...db.de>
To:	wwang <wei_wang@...lsil.com.cn>
Cc:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
	"cjb@...top.org" <cjb@...top.org>, "bp@...en8.de" <bp@...en8.de>
Subject: Re: [PATCH 1/3] drivers/misc: Add realtek card reader core driver

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/