Currently we are using an explicit udev rule to trigger loading of the
mmc-block module when an MMC or SD card is detected:
SUBSYSTEM=="mmc", RUN+="/sbin/modprobe -Qba mmc-block"
It makes much more sense for the mmc bus driver and the mmc-block module to
share MODALIAS information so that they are linked automatically.
Add MODALIAS attributes to the uevents as generated, and add the
corresponding MODULE_ALIAS marks to the mmc-block module so that it is
automatically loaded by udev. I have followed the example set by the
SCSI subsystem in pushing out numeric typed aliases:
mmc:t-0xNN
Where NN is the MMC type as defined in include/linux/mmc/card.h.
Numeric types are used to allow automatic handling of new unknown types
in the core. The matches are open on the right to allow additional
fields to be added as required in the future.
Signed-off-by: Andy Whitcroft <[email protected]>
---
drivers/mmc/card/block.c | 3 +++
drivers/mmc/core/bus.c | 5 +++++
include/linux/mmc/card.h | 10 +++++++---
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 45b1f43..5d1c5ad 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -41,6 +41,9 @@
#include "queue.h"
+MODULE_ALIAS_MMC_DEVICE(MMC_TYPE_MMC);
+MODULE_ALIAS_MMC_DEVICE(MMC_TYPE_SD);
+
/*
* max 8 partitions per card
*/
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index f210a8e..35ec7c3 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -84,6 +84,11 @@ mmc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
}
retval = add_uevent_var(env, "MMC_NAME=%s", mmc_card_name(card));
+ if (retval)
+ return retval;
+
+ retval = add_uevent_var(env, "MODALIAS=" MMC_DEVICE_MODALIAS_FMT,
+ card->type);
return retval;
}
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 403aa50..97a461e 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -86,9 +86,9 @@ struct mmc_card {
struct device dev; /* the device */
unsigned int rca; /* relative card address of device */
unsigned int type; /* card type */
-#define MMC_TYPE_MMC 0 /* MMC card */
-#define MMC_TYPE_SD 1 /* SD card */
-#define MMC_TYPE_SDIO 2 /* SDIO card */
+#define MMC_TYPE_MMC 0x00 /* MMC card */
+#define MMC_TYPE_SD 0x01 /* SD card */
+#define MMC_TYPE_SDIO 0x02 /* SDIO card */
unsigned int state; /* (our) card state */
#define MMC_STATE_PRESENT (1<<0) /* present in sysfs */
#define MMC_STATE_READONLY (1<<1) /* card is read-only */
@@ -150,4 +150,8 @@ struct mmc_driver {
extern int mmc_register_driver(struct mmc_driver *);
extern void mmc_unregister_driver(struct mmc_driver *);
+#define MODULE_ALIAS_MMC_DEVICE(type) \
+ MODULE_ALIAS("mmc:t-" __stringify(type) "*")
+#define MMC_DEVICE_MODALIAS_FMT "mmc:t-0x%02x"
+
#endif
--
1.6.0.4.911.gc990
On Tue, 6 Jan 2009 18:56:38 +0000
Andy Whitcroft <[email protected]> wrote:
> Currently we are using an explicit udev rule to trigger loading of the
> mmc-block module when an MMC or SD card is detected:
>
> SUBSYSTEM=="mmc", RUN+="/sbin/modprobe -Qba mmc-block"
>
> It makes much more sense for the mmc bus driver and the mmc-block module to
> share MODALIAS information so that they are linked automatically.
>
First, I'd just like to state that I agree that the current situation
is far from optimal. Things should "just work".
That said, the MMC/SD stuff is a bit of a mess (the entire ecosystem,
not just the Linux implementation). The classical method of matching
devices to drivers isn't a perfect fit here. There is simply no nice
and proper layering.
So right now I think that udev (or equivalent system) is the cleanest
solution. If nothing else, it means we don't have to create a kernel
ABI that might be problematic to maintain in the future. Feel free to
convince me I'm wrong though. :)
> Add MODALIAS attributes to the uevents as generated, and add the
> corresponding MODULE_ALIAS marks to the mmc-block module so that it is
> automatically loaded by udev. I have followed the example set by the
> SCSI subsystem in pushing out numeric typed aliases:
>
> mmc:t-0xNN
>
> Where NN is the MMC type as defined in include/linux/mmc/card.h.
Currently the only type that isn't matched is MMC_TYPE_SDIO. But that
will probably change once we support combo cards. At that point, all
types will result in mmc_block being loaded and we're basically back to
always loading mmc_block on card insertion.
There is also the fact that type isn't the primary thing that mmc_block
uses to determine if it can use the card. The more interesting thing is
the command classes. But since those aren't defined for pure SDIO
cards, it's not something we can sensibly use for MODALIAS.
In essence, the crappy design of MMC and SD means there is no decent
identifier for the card at this level.
Your patch leaves room for adding things in the future, but it
doesn't seem right adding an ABI with fields that we already know are
pretty useless at identifying devices.
Is the udev method causing practical problems, or is this a matter of
trying to do things the proper way just for the sake of it?
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org
WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.
On Mon, Jan 12, 2009 at 04:05:19PM +0100, Pierre Ossman wrote:
> On Tue, 6 Jan 2009 18:56:38 +0000
> Andy Whitcroft <[email protected]> wrote:
>
> > Currently we are using an explicit udev rule to trigger loading of the
> > mmc-block module when an MMC or SD card is detected:
> >
> > SUBSYSTEM=="mmc", RUN+="/sbin/modprobe -Qba mmc-block"
> >
> > It makes much more sense for the mmc bus driver and the mmc-block module to
> > share MODALIAS information so that they are linked automatically.
> >
>
> First, I'd just like to state that I agree that the current situation
> is far from optimal. Things should "just work".
>
> That said, the MMC/SD stuff is a bit of a mess (the entire ecosystem,
> not just the Linux implementation). The classical method of matching
> devices to drivers isn't a perfect fit here. There is simply no nice
> and proper layering.
>
> So right now I think that udev (or equivalent system) is the cleanest
> solution. If nothing else, it means we don't have to create a kernel
> ABI that might be problematic to maintain in the future. Feel free to
> convince me I'm wrong though. :)
The problem is that the kernel has an internal dependancy for using these
cards on the mmc-block driver but that is not expressed by the kernel.
This means that we have to maintain an external dependancy in the udev
package. Unfortuanatly that is is not versioned with the kernel so
that adds a per distro maintenance burden ensuring udev is in step with
the kernel.
> > Add MODALIAS attributes to the uevents as generated, and add the
> > corresponding MODULE_ALIAS marks to the mmc-block module so that it is
> > automatically loaded by udev. I have followed the example set by the
> > SCSI subsystem in pushing out numeric typed aliases:
> >
> > mmc:t-0xNN
> >
> > Where NN is the MMC type as defined in include/linux/mmc/card.h.
>
> Currently the only type that isn't matched is MMC_TYPE_SDIO. But that
> will probably change once we support combo cards. At that point, all
> types will result in mmc_block being loaded and we're basically back to
> always loading mmc_block on card insertion.
>
> There is also the fact that type isn't the primary thing that mmc_block
> uses to determine if it can use the card. The more interesting thing is
> the command classes. But since those aren't defined for pure SDIO
> cards, it's not something we can sensibly use for MODALIAS.
>
> In essence, the crappy design of MMC and SD means there is no decent
> identifier for the card at this level.
Yeah I can see how we might end up still loading the driver for all SDIO
cards once we support them correctly; possibly even for ones with no
storage if we are unable to tell from the underlying device. The good
thing about expressing it this way is that should the kernel find out how
to do it we can modify the aliases and prevent the module loading on all
distros automatically.
> Your patch leaves room for adding things in the future, but it
> doesn't seem right adding an ABI with fields that we already know are
> pretty useless at identifying devices.
>
> Is the udev method causing practical problems, or is this a matter of
> trying to do things the proper way just for the sake of it?
It is about reducing external dependancies to simplify life for the
distro packagers.
Would it make more sense to simply use mmc: with no data as the MODALIAS
tag here. And make mmc-block have mmc:* as an alias. That way the
actual (useless) numeric data is not exposed but we still will load the
mmc-block module for everything exactly as we do with the udev rule.
This should still make things work from a distro point of view and not
expose anything which might then get relied on. If we do it so extra
data can still be added later we won't prevent sane data being exposed
should we ever have any.
If you are happier with that I can re-spin the patches that way?
-apw
On Thu, 15 Jan 2009 15:00:03 +0000
Andy Whitcroft <[email protected]> wrote:
>
> Would it make more sense to simply use mmc: with no data as the MODALIAS
> tag here. And make mmc-block have mmc:* as an alias. That way the
> actual (useless) numeric data is not exposed but we still will load the
> mmc-block module for everything exactly as we do with the udev rule.
> This should still make things work from a distro point of view and not
> expose anything which might then get relied on. If we do it so extra
> data can still be added later we won't prevent sane data being exposed
> should we ever have any.
>
> If you are happier with that I can re-spin the patches that way?
>
Well, as long as we're on the track of temporary hack, we might as well
just export "mmc_block" as the modalias. Or would there be any
side-effects to that?
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org
WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.
On Sat, Jan 24, 2009 at 18:56, Pierre Ossman <[email protected]> wrote:
> On Thu, 15 Jan 2009 15:00:03 +0000
> Andy Whitcroft <[email protected]> wrote:
>
>>
>> Would it make more sense to simply use mmc: with no data as the MODALIAS
>> tag here. And make mmc-block have mmc:* as an alias. That way the
>> actual (useless) numeric data is not exposed but we still will load the
>> mmc-block module for everything exactly as we do with the udev rule.
>> This should still make things work from a distro point of view and not
>> expose anything which might then get relied on. If we do it so extra
>> data can still be added later we won't prevent sane data being exposed
>> should we ever have any.
>>
>> If you are happier with that I can re-spin the patches that way?
>>
>
> Well, as long as we're on the track of temporary hack, we might as well
> just export "mmc_block" as the modalias. Or would there be any
> side-effects to that?
The common format is to prefix with "<subsystem>:". Something like
"mmc:block" sounds fine to me.
Thanks,
Kay
On Sun, 25 Jan 2009 00:45:46 +0100
Kay Sievers <[email protected]> wrote:
> On Sat, Jan 24, 2009 at 18:56, Pierre Ossman <[email protected]> wrote:
> >
> > Well, as long as we're on the track of temporary hack, we might as well
> > just export "mmc_block" as the modalias. Or would there be any
> > side-effects to that?
>
> The common format is to prefix with "<subsystem>:". Something like
> "mmc:block" sounds fine to me.
>
My point was to have the kernel explicitly ask for the module it wants
as there is no decent device to driver mapping scheme.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org
WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.
On Sun, Jan 25, 2009 at 16:48, Pierre Ossman <[email protected]> wrote:
> On Sun, 25 Jan 2009 00:45:46 +0100
> Kay Sievers <[email protected]> wrote:
>
>> On Sat, Jan 24, 2009 at 18:56, Pierre Ossman <[email protected]> wrote:
>> >
>> > Well, as long as we're on the track of temporary hack, we might as well
>> > just export "mmc_block" as the modalias. Or would there be any
>> > side-effects to that?
>>
>> The common format is to prefix with "<subsystem>:". Something like
>> "mmc:block" sounds fine to me.
>>
>
> My point was to have the kernel explicitly ask for the module it wants
> as there is no decent device to driver mapping scheme.
Yep, which is what we do not want. Aliases are "aliases", and not
"module names". We need to add a matching alias to the module then.
Direct module names can not properly defined/blacklisted in userspace,
and we would need to work around that.
Every modalias should be
"<subsystem>:<whatever-name-fits-for-the-subsystem>" to plug properly
into the autoloading infrastructure. We rather have no modalias at
all, then a kernel module name there.
Thanks,
Kay
On Sun, Jan 25, 2009 at 05:00:11PM +0100, Kay Sievers wrote:
> On Sun, Jan 25, 2009 at 16:48, Pierre Ossman <[email protected]> wrote:
> > On Sun, 25 Jan 2009 00:45:46 +0100
> > Kay Sievers <[email protected]> wrote:
> >
> >> On Sat, Jan 24, 2009 at 18:56, Pierre Ossman <[email protected]> wrote:
> >> >
> >> > Well, as long as we're on the track of temporary hack, we might as well
> >> > just export "mmc_block" as the modalias. Or would there be any
> >> > side-effects to that?
> >>
> >> The common format is to prefix with "<subsystem>:". Something like
> >> "mmc:block" sounds fine to me.
> >>
> >
> > My point was to have the kernel explicitly ask for the module it wants
> > as there is no decent device to driver mapping scheme.
>
> Yep, which is what we do not want. Aliases are "aliases", and not
> "module names". We need to add a matching alias to the module then.
> Direct module names can not properly defined/blacklisted in userspace,
> and we would need to work around that.
> Every modalias should be
> "<subsystem>:<whatever-name-fits-for-the-subsystem>" to plug properly
> into the autoloading infrastructure. We rather have no modalias at
> all, then a kernel module name there.
To summarise, using a modalias pairing is good for two reasons: kernel
dependancies are explicit in the kernel and things will just work;
and standard blacklisting of the module will now work. Module alias
triggers cannot be actual module names else the latter will not work.
But we also do not want to expose the current type number information
to prevent it becoming part of the ABI. The aliases should be of the
form <subsystem>:<hints>.
I note that we already are exposing the type as text and the card name
in the ABI already exposed as MMC_TYPE and MMC_NAME. How about I switch
the alias to the textual types, as those are already in the ABI. Using
the usb model something like this:
mmc:tMMC
mmc:tSD
mmc:tSDIO
mmc:tUNKNOWN
This with a view to it being extended in the same manner, say we wanted
to expose the name here too (which is also in the ABI, I am not
proposing to add it):
mmc:tMMCn<name>
Would that satisfy everyone? I will respin the patches which will be
somewhat simpler as a result.
-apw
[Here is an updated patch which only uses information already exported in
the ABI.]
Currently we are using an explicit udev rule to trigger loading of the
mmc-block module when an MMC or SD card is detected:
SUBSYSTEM=="mmc", RUN+="/sbin/modprobe -Qba mmc-block"
It makes much more sense for the mmc bus driver and the mmc-block module to
share MODALIAS information so that they are linked automatically.
Add MODALIAS attributes to the uevents as generated, and add the
corresponding MODULE_ALIAS marks to the mmc-block module so that it is
automatically loaded by udev. Following discussion it was decided to
expose the textual types, and so I have followed the example set by
the dmi: aliases for handling text. The aliases are of the following
form:
mmc:t<type>:
For example:
mmc:tMMC:
The entry ends with a separator (:) in order to allow simple differentiation
between common prefixes such as SD and SDIO in MODULE_ALIAS declarations.
Signed-off-by: Andy Whitcroft <[email protected]>
---
drivers/mmc/card/block.c | 3 +++
drivers/mmc/core/bus.c | 5 +++++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 3d067c3..9799273 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -41,6 +41,9 @@
#include "queue.h"
+MODULE_ALIAS("mmc:tMMC:*");
+MODULE_ALIAS("mmc:tSD:*");
+
/*
* max 8 partitions per card
*/
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index f210a8e..7b34c00 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -84,6 +84,11 @@ mmc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
}
retval = add_uevent_var(env, "MMC_NAME=%s", mmc_card_name(card));
+ if (retval)
+ return retval;
+
+ retval = add_uevent_var(env, "MODALIAS=mmc:t%s:",
+ (type) ? type : "UNKNOWN");
return retval;
}
--
1.6.1.258.g7ff14.dirty
On Sun, 25 Jan 2009 17:00:11 +0100
Kay Sievers <[email protected]> wrote:
> On Sun, Jan 25, 2009 at 16:48, Pierre Ossman <[email protected]> wrote:
> >
> > My point was to have the kernel explicitly ask for the module it wants
> > as there is no decent device to driver mapping scheme.
>
> Yep, which is what we do not want. Aliases are "aliases", and not
> "module names". We need to add a matching alias to the module then.
> Direct module names can not properly defined/blacklisted in userspace,
> and we would need to work around that.
> Every modalias should be
> "<subsystem>:<whatever-name-fits-for-the-subsystem>" to plug properly
> into the autoloading infrastructure. We rather have no modalias at
> all, then a kernel module name there.
>
The thing is that asking for a module is the only thing we can do here.
We can dress it up and give it some special coding to not cause
problems, but the code will always be "ask userspace to load
mmc_block", even if we replace "mmc_block" with "mmc:foobargazonk".
Given that, do you have any preferences for a solution? If we cannot
simply have "mmc_block", then I'm leaning to "mmc:block" for now. The
contents of the aliases is just an opaque string as far as userspace is
concerned, right?
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org
WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.
On Tue, Jan 27, 2009 at 20:14, Pierre Ossman <[email protected]> wrote:
> On Sun, 25 Jan 2009 17:00:11 +0100
> Kay Sievers <[email protected]> wrote:
>
>> On Sun, Jan 25, 2009 at 16:48, Pierre Ossman <[email protected]> wrote:
>> >
>> > My point was to have the kernel explicitly ask for the module it wants
>> > as there is no decent device to driver mapping scheme.
>>
>> Yep, which is what we do not want. Aliases are "aliases", and not
>> "module names". We need to add a matching alias to the module then.
>> Direct module names can not properly defined/blacklisted in userspace,
>> and we would need to work around that.
>> Every modalias should be
>> "<subsystem>:<whatever-name-fits-for-the-subsystem>" to plug properly
>> into the autoloading infrastructure. We rather have no modalias at
>> all, then a kernel module name there.
>>
>
> The thing is that asking for a module is the only thing we can do here.
> We can dress it up and give it some special coding to not cause
> problems, but the code will always be "ask userspace to load
> mmc_block", even if we replace "mmc_block" with "mmc:foobargazonk".
>
> Given that, do you have any preferences for a solution? If we cannot
> simply have "mmc_block", then I'm leaning to "mmc:block" for now. The
> contents of the aliases is just an opaque string as far as userspace is
> concerned, right?
Right, it does not matter, we just need to be able to fnmatch()
modalias and the string in the module.
We prefer every modalias to be prefixed with the subsystem, to be able
to hook userspace configs into the alias processing. The string itself
is completely up to the subsystem to decide whatever fits. If there
could ever be several types of strings per susbsystem, it is usual to
prefix the string with some character, but some subsystems know that
this is not needed, and just put a plain single string there.
Thanks,
Kay
On Tue, Jan 27, 2009 at 08:26:26PM +0100, Kay Sievers wrote:
> On Tue, Jan 27, 2009 at 20:14, Pierre Ossman <[email protected]> wrote:
> > On Sun, 25 Jan 2009 17:00:11 +0100
> > Kay Sievers <[email protected]> wrote:
> >
> >> On Sun, Jan 25, 2009 at 16:48, Pierre Ossman <[email protected]> wrote:
> >> >
> >> > My point was to have the kernel explicitly ask for the module it wants
> >> > as there is no decent device to driver mapping scheme.
> >>
> >> Yep, which is what we do not want. Aliases are "aliases", and not
> >> "module names". We need to add a matching alias to the module then.
> >> Direct module names can not properly defined/blacklisted in userspace,
> >> and we would need to work around that.
> >> Every modalias should be
> >> "<subsystem>:<whatever-name-fits-for-the-subsystem>" to plug properly
> >> into the autoloading infrastructure. We rather have no modalias at
> >> all, then a kernel module name there.
> >>
> >
> > The thing is that asking for a module is the only thing we can do here.
> > We can dress it up and give it some special coding to not cause
> > problems, but the code will always be "ask userspace to load
> > mmc_block", even if we replace "mmc_block" with "mmc:foobargazonk".
> >
> > Given that, do you have any preferences for a solution? If we cannot
> > simply have "mmc_block", then I'm leaning to "mmc:block" for now. The
> > contents of the aliases is just an opaque string as far as userspace is
> > concerned, right?
>
> Right, it does not matter, we just need to be able to fnmatch()
> modalias and the string in the module.
>
> We prefer every modalias to be prefixed with the subsystem, to be able
> to hook userspace configs into the alias processing. The string itself
> is completely up to the subsystem to decide whatever fits. If there
> could ever be several types of strings per susbsystem, it is usual to
> prefix the string with some character, but some subsystems know that
> this is not needed, and just put a plain single string there.
As the contents as opaque we can put whatever we want after the mmc:,
the last patch I did simply used the mmc type string as a key in case it
were useful. Right now it is as we do only load the block for the SD
and MMC cases. But later it would be for all once the SDIO card
support, supports memory.
So asre we happy with the last textual patch or should I refactor back
to mmc:block.
-apw
On Wed, 28 Jan 2009 15:06:39 +0000
Andy Whitcroft <[email protected]> wrote:
>
> So asre we happy with the last textual patch or should I refactor back
> to mmc:block.
>
I prefer "block" and a comment near the code so that noone
misinterprets the system as a normal device to driver mapping.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org
WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.
On Mon, 23 Feb 2009 12:38:41 +0000
Andy Whitcroft <[email protected]> wrote:
> [Pierre, here is the updated version. Got lost somewhere after testing
> and before sending out.]
>
> Currently we are using an explicit udev rule to trigger loading of the
> mmc-block module when an MMC or SD card is detected:
>
> SUBSYSTEM=="mmc", RUN+="/sbin/modprobe -Qba mmc-block"
>
> It makes much more sense for the mmc bus driver and the mmc-block module to
> share MODALIAS information so that they are linked automatically.
>
> There is no real information of use in the MMC system at the current time.
> All devices inserted require us to load the mmc-block device. Until such
> time as useful parameters exist simply reflect the module linkage via
> the module alias below:
>
> mmc:block
>
> Signed-off-by: Andy Whitcroft <[email protected]>
> ---
Queued, thanks.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org
WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.