[PATCH] MMC - CONFIG_HOTPLUG support

[PATCH] MMC - CONFIG_HOTPLUG support

[Anderson Briglia]

[-- Attachment #1: Type: text/plain, Size: 139 bytes --]

Hi all,

This patch improves the CONFIG_HOTPLUG support on the mmc core driver.
Kernel version: 2.6.15-rc4-omap1

BR,

--
Anderson Briglia

[-- Attachment #2: mmc_config_hotplug.diff --]
[-- Type: text/x-patch, Size: 948 bytes --]

Signed-off-by: Anderson Briglia <anderson.briglia@indt.org.br>

Index: linux-2.6.15-rc4/drivers/mmc/mmc_sysfs.c
===================================================================
--- linux-2.6.15-rc4.orig/drivers/mmc/mmc_sysfs.c	2005-12-15 17:00:52.000000000 -0400
+++ linux-2.6.15-rc4/drivers/mmc/mmc_sysfs.c	2005-12-15 17:54:07.000000000 -0400
@@ -145,6 +145,8 @@ static int mmc_bus_match(struct device *
 	return !mmc_card_bad(card);
 }
 
+#ifdef CONFIG_HOTPLUG
+
 static int
 mmc_bus_hotplug(struct device *dev, char **envp, int num_envp, char *buf,
 		int buf_size)
@@ -180,6 +182,17 @@ mmc_bus_hotplug(struct device *dev, char
 	return 0;
 }
 
+#else
+
+static int
+mmc_bus_hotplug(struct device *dev, char **envp, int num_envp, char *buf,
+		int buf_size)
+{
+	return -ENODEV;
+}
+
+#endif  /* CONFIG_HOTPLUG */
+
 static int mmc_bus_suspend(struct device *dev, pm_message_t state)
 {
 	struct mmc_driver *drv = to_mmc_driver(dev->driver);

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] MMC - CONFIG_HOTPLUG support

[Russell King - ARM Linux]

On Fri, Dec 16, 2005 at 03:50:13PM -0400, Anderson Briglia wrote:
> This patch improves the CONFIG_HOTPLUG support on the mmc core driver.
> Kernel version: 2.6.15-rc4-omap1

Please explain how this "improves" the support.

RE: [PATCH] MMC - CONFIG_HOTPLUG support

[Anderson.Briglia]

> This patch improves the CONFIG_HOTPLUG support on the mmc core driver.
> Kernel version: 2.6.15-rc4-omap1

Please explain how this "improves" the support.

As we can see on usb core driver, there's a compiler directive that bypass the hotplug function if the CONFIG_HOTPLUG is not selected. I think I could add this "support" on the MMC driver. Just a optimization.
Please, correct me if I'm wrong.

BR,

Anderson Briglia

Re: [PATCH] MMC - CONFIG_HOTPLUG support

[Russell King - ARM Linux]

On Mon, Dec 19, 2005 at 09:52:59AM -0300, Anderson.Briglia@indt.org.br wrote:
> > > This patch improves the CONFIG_HOTPLUG support on the mmc core driver.
> > > Kernel version: 2.6.15-rc4-omap1
> > 
> > Please explain how this "improves" the support.
> 
> As we can see on usb core driver, there's a compiler directive that
> bypass the hotplug function if the CONFIG_HOTPLUG is not selected.
> I think I could add this "support" on the MMC driver. Just a
> optimization.

Just because some other subsystem does something does not make it
right for others to do the same.  I'd like you to explain:

1. what additional support this patch gives us.
2. what the nature of the optimisation is.

> Please, correct me if I'm wrong.

I don't know if you're wrong because I don't understand the reasoning
behind the change.

Re: [PATCH] MMC - CONFIG_HOTPLUG support

[Juha Yrjölä]

On Mon, 2005-12-19 at 13:04 +0000, Russell King - ARM Linux wrote:

> > As we can see on usb core driver, there's a compiler directive that
> > bypass the hotplug function if the CONFIG_HOTPLUG is not selected.
> > I think I could add this "support" on the MMC driver. Just a
> > optimization.
> 
> Just because some other subsystem does something does not make it
> right for others to do the same.  I'd like you to explain:
> 
> 1. what additional support this patch gives us.
> 2. what the nature of the optimisation is.

With the patch the kernel text segment becomes a wee bit smaller if
certain criteria is met; that much should be obvious.

It's a trivial patch, true, but quite a valid one.  If the MMC hotplug
function is not to be called from anywhere, no reason to waste space in
compiling it in.

Cheers,
Juha

RE: [PATCH] MMC - CONFIG_HOTPLUG support

[Anderson.Briglia]

On Mon, Dec 19, 2005 at 09:52:59AM -0300, Anderson.Briglia@indt.org.br wrote:
> > > > This patch improves the CONFIG_HOTPLUG support on the mmc core driver.
> > > > Kernel version: 2.6.15-rc4-omap1
> > > 
> > > Please explain how this "improves" the support.
> > 
> > As we can see on usb core driver, there's a compiler directive that
> > bypass the hotplug function if the CONFIG_HOTPLUG is not selected.
> > I think I could add this "support" on the MMC driver. Just a
> > optimization.

> Just because some other subsystem does something does not make it
> right for others to do the same.

IMHO, the USB hotplug system is working well for usb block devices.

> I'd like you to explain:

> 1. what additional support this patch gives us.

I'm working on MMC hotplug system. This patch is a inicial work to provide a minimal kernel configuration hotplug support (and prevent future bugs). 
It's necessary more interaction between the MMC core and the MMC block driver. When a MMC card is inserted/removed the MMC block driver should be informed and adds/removes the device information from the kernel. I'm trying to implement this interaction.

> 2. what the nature of the optimisation is.

AFAIK, compiler directives are a guiding for what will/won't be compiled. IMHO, if the kernel hotplug support is not selected, doesn't make sense that the mmc_bus_hotplug function be compiled. When the MMC core and MMC block interaction will be implemented the mmc_bus_hotplug function should not be compiled providing that hotplug support.

> > Please, correct me if I'm wrong.

> I don't know if you're wrong because I don't understand the reasoning
> behind the change.

Sorry if I didn't explain before. If I'm wrong, let me know.

BR,

Anderson Briglia

Re: [PATCH] MMC - CONFIG_HOTPLUG support

[Russell King - ARM Linux]

On Mon, Dec 19, 2005 at 01:02:36PM -0300, Anderson.Briglia@indt.org.br wrote:
> On Mon, Dec 19, 2005 at 09:52:59AM -0300, Anderson.Briglia@indt.org.br wrote:
> > I'd like you to explain:
> > 1. what additional support this patch gives us.
> 
> I'm working on MMC hotplug system. This patch is a inicial work to provide
> a minimal kernel configuration hotplug support (and prevent future bugs). 

Either you have hotplug support or you don't.  There is no half-way
house implemented in any kernel subsystem.

> It's necessary more interaction between the MMC core and the MMC block
> driver. When a MMC card is inserted/removed the MMC block driver should
> be informed and adds/removes the device information from the kernel.
> I'm trying to implement this interaction.

This is already implemented.  The MMC block driver will be called via
mmc_blk_probe() when a card is inserted, and via mmc_blk_remove()
when it is removed.

> > 2. what the nature of the optimisation is.
> 
> AFAIK, compiler directives are a guiding for what will/won't be compiled.
> IMHO, if the kernel hotplug support is not selected, doesn't make sense
> that the mmc_bus_hotplug function be compiled. When the MMC core and
> MMC block interaction will be implemented the mmc_bus_hotplug function
> should not be compiled providing that hotplug support.

Right.  You could eliminate the whole function and define mmc_bus_hotplug
to NULL when CONFIG_HOTPLUG is unset, which will eliminate even more
bytes.

RE: [PATCH] MMC - CONFIG_HOTPLUG support

[Anderson.Briglia]

> > > I'd like you to explain:
> > > 1. what additional support this patch gives us.
> > 
> > I'm working on MMC hotplug system. This patch is a inicial work to provide
> > a minimal kernel configuration hotplug support (and prevent future bugs). 

> Either you have hotplug support or you don't.  There is no half-way
> house implemented in any kernel subsystem.

Ok.

> > It's necessary more interaction between the MMC core and the MMC block
> > driver. When a MMC card is inserted/removed the MMC block driver should
> > be informed and adds/removes the device information from the kernel.
> > I'm trying to implement this interaction.

> This is already implemented.  The MMC block driver will be called via
> mmc_blk_probe() when a card is inserted, and via mmc_blk_remove()
> when it is removed.

Yes, it's right. But if the MMC block driver is compiled as a module, it's not loaded automatically by the MMC core driver. Another problem is a bug on mmc omap driver: if a card is released, the sysfs and /dev entries are not removed (I'm using udev).

> > > 2. what the nature of the optimisation is.
> > 
> > AFAIK, compiler directives are a guiding for what will/won't be compiled.
> > IMHO, if the kernel hotplug support is not selected, doesn't make sense
> > that the mmc_bus_hotplug function be compiled. When the MMC core and
> > MMC block interaction will be implemented the mmc_bus_hotplug function
> > should not be compiled providing that hotplug support.

> Right.  You could eliminate the whole function and define mmc_bus_hotplug
> to NULL when CONFIG_HOTPLUG is unset, which will eliminate even more
> bytes.

Ok.

Anderson Briglia

Re: [PATCH] MMC - CONFIG_HOTPLUG support

[Russell King - ARM Linux]

On Mon, Dec 26, 2005 at 03:46:58PM -0300, Anderson.Briglia@indt.org.br wrote:
> > This is already implemented.  The MMC block driver will be called via
> > mmc_blk_probe() when a card is inserted, and via mmc_blk_remove()
> > when it is removed.
> 
> Yes, it's right. But if the MMC block driver is compiled as a module,
> it's not loaded automatically by the MMC core driver.

That's not a kernel problem - the userspace tools (module-utils) do not
know how to interpret the MMC hotplug information to determine which
modules need loading.

However, it's quite likely that even if they did, we'll need to change
the way that the MMC drivers are bound to the MMC devices - it's possible
that a MMC device could be driven by more than one driver.

> Another problem is a bug on mmc omap driver: if a card is released,
> the sysfs and /dev entries are not removed (I'm using udev).

Which sysfs entries?  Could be that the omap driver doesn't detect
card removals correctly - or is asking the MMC subsystem to re-detect
cards before the card has actually disconnected from the bus.