[RFC] what to do next

[Dominik Brodowski]

Once the previous patchset consisting of these patches:

http://www.brodo.de/patches/2003-10-02/pcmcia-2.6.0-test6+-change-ds-license
http://www.brodo.de/patches/2003-10-02/pcmcia-2.6.0-test6+-pcmcia_device
http://www.brodo.de/patches/2003-10-02/pcmcia-2.6.0-test6+-remove_socket_bind_t
http://www.brodo.de/patches/2003-10-02/pcmcia-2.6.0-test6+-remove_internal_driver_use_count

is merged into the 2.6.-test kernel, I'd like to do the following changes:


http://www.brodo.de/patches/2003-10-02/pcmcia-2.6.0-test6+-attach_parameter
  Add the corresponding "struct pcmcia_device" as argument to the 
  ->attach() call. Update all drivers to do so.
	[due to the size of this patch it is only available online]


http://www.brodo.de/patches/2003-10-02/pcmcia-2.6.0-test6+-new_driver_callbacks_core
  Add new callbacks for PCMCIA drivers:

	->probe():	called instead of the CS_EVENT_CARD_INSERTION
			call. As a driver can only call 
			pcmcia_register_client once a corresponding
			PCMCIA card had been inserted into a socket,
			the CARD_INSERTION event is generated upon all
			current calls to pcmcia_register_client. So it can
			be put to after the ->attach() call as well.

	->remove():	called when CS_EVENT_CARD_REMOVAL would have
			been called.

	->suspend():	called when CS_EVENT_PM_SUSPEND would have been
			called. [Note: should be integrated into the PM
			core soon].

	->resume():	called when CS_EVENT_PM_RESUME would have been
			called. [Note: should be integrated into the PM
			core soon].

	->pre_reset():	called when CS_EVENT_CARD_RESET would have been
			called.

	->post_reset():	called when CS_EVENT_RESET_PHYSICAL would have
			been called.

  ->probe() and ->attach() will be merged into one function, same with
  ->remove() and ->detach(). And maybe pre_reset() and suspend(), and 
  post_reset() and resume() can be merged as well, as only very few
  drivers do something different on suspension...


http://www.brodo.de/patches/2003-10-02/pcmcia-2.6.0-test6+-update_pcnet_cs_driver_as_example
  Update the pcnet_cs driver to reflect the core changes as an example.


http://www.brodo.de/patches/2003-10-02/pcmcia-2.6.0-test6+-move_suspend_and_reset_to_callbacks_all
  Move the suspend, resume, reset and reset_physical to the corresponding 
  callbacks for all in-kernel pcmcia drivers.
	[due to the size of this patch it is only available online]



After this next step is done, the event callbacks should be removed
completely from the remaining in-kernel pcmcia drivers. Then, I intend to
move some PCMCIA-related code from ds.c and cs.c into a new
drivers/pcmcia/pcmcia.c, which is then linked into pcmcia_core.ko. Then both
bind_req_t and client_t can be merged into struct pcmcia_device, with
client_handle_t becoming a pointer to struct pcmcia_device *p_dev.

	Dominik
-------------- next part --------------
Add new callbacks for PCMCIA drivers:

	->probe():	called instead of the CS_EVENT_CARD_INSERTION
			call. As a driver can only call 
			pcmcia_register_client once a corresponding
			PCMCIA card had been inserted into a socket,
			the CARD_INSERTION event is generated upon all
			current calls to pcmcia_register_client. So it can
			be put to after the ->attach() call as well.

	->remove():	called when CS_EVENT_CARD_REMOVAL would have
			been called.

	->suspend():	called when CS_EVENT_PM_SUSPEND would have been
			called. [Note: should be integrated into the PM
			core soon].

	->resume():	called when CS_EVENT_PM_RESUME would have been
			called. [Note: should be integrated into the PM
			core soon].

	->pre_reset():	called when CS_EVENT_CARD_RESET would have been
			called.

	->post_reset():	called when CS_EVENT_RESET_PHYSICAL would have
			been called.

->probe() and ->attach() will be merged into one function, same with
->remove() and ->detach(). And maybe pre_reset() and suspend(), and 
post_reset() and resume() can be merged as well, as only very few
drivers do something different on suspension...

diff -ruN linux-original/drivers/pcmcia/ds.c linux/drivers/pcmcia/ds.c
--- linux-original/drivers/pcmcia/ds.c	2003-10-02 23:19:14.717279728 +0200
+++ linux/drivers/pcmcia/ds.c	2003-10-02 23:21:12.561364704 +0200
@@ -246,6 +246,69 @@
     
 ======================================================================*/
 
+static void ds_call_driver_callbacks(struct pcmcia_bus_socket *s, 
+				     event_t event)
+{
+	struct pcmcia_device *p_dev, *tmp;
+	struct pcmcia_driver *p_drv;
+
+	/* TBD: properly assure that events are serialized, or at least
+	 * safe against driver and device removal. This will be done once
+	 * ds and cs are a) integrated and b) split up into core [pccard.ko], 
+	 * cardbus.ko and pcmcia.ko
+	 */
+	list_for_each_entry_safe(p_dev, tmp, &s->devices_list, socket_device_list) {
+		struct device *dev; 
+		struct device_driver *drv;
+
+		dev = get_device(&p_dev->dev);
+		if (!dev)
+			continue;
+		p_dev = to_pcmcia_dev(dev);
+
+		drv = get_driver(dev->driver);
+		if (!drv) {
+			put_device(dev);
+			continue;
+		}
+		p_drv = to_pcmcia_drv(drv);
+
+		switch(event) {
+		case CS_EVENT_CARD_INSERTION:
+			printk(KERN_DEBUG "stale CARD_INSERTION event detected. Please inform <linux at brodo.de>\n");
+			printk(KERN_DEBUG "of what you did just now with your PCMCIA card. Thanks.\n");
+			break;
+
+		case CS_EVENT_CARD_REMOVAL:
+			if (p_drv->remove)
+				p_drv->remove(p_dev);
+
+		case CS_EVENT_RESET_PHYSICAL:
+			if (p_drv->pre_reset)
+				p_drv->pre_reset(p_dev);
+			break;
+
+		case CS_EVENT_CARD_RESET:
+			if (p_drv->post_reset)
+				p_drv->post_reset(p_dev);
+			break;
+
+		case CS_EVENT_PM_SUSPEND:
+			if (p_drv->suspend)
+				p_drv->suspend(p_dev);
+			break;
+
+		case CS_EVENT_PM_RESUME:
+			if (p_drv->resume)
+				p_drv->resume(p_dev);
+			break;
+		}
+
+		put_driver(drv);
+		put_device(dev);
+	}
+}
+
 static int ds_event(event_t event, int priority,
 		    event_callback_args_t *args)
 {
@@ -253,8 +316,16 @@
 
     DEBUG(1, "ds: ds_event(0x%06x, %d, 0x%p)\n",
 	  event, priority, args->client_handle);
+
     s = args->client_data;
     
+    /* The CS core used a single linked list for the event queue,
+       which assured that the later a "client" is register, the
+       earlier it is informed of events. So call the drivers first,
+       and do our stuff later.
+    */
+    ds_call_driver_callbacks(s, event);
+    
     switch (event) {
 	
     case CS_EVENT_CARD_REMOVAL:
@@ -404,8 +475,14 @@
 		}
 	}
 
-	return 0;
+	/* and now call the real "probe" function ... */
+	if (p_drv->probe) {
+		ret = p_drv->probe(p_dev);
+		if (ret)
+			goto err_unreg;
+	}
 
+	return 0;
 
  err_unreg:
 	spin_lock_irqsave(&pcmcia_dev_list_lock, flags);
diff -ruN linux-original/include/pcmcia/ds.h linux/include/pcmcia/ds.h
--- linux-original/include/pcmcia/ds.h	2003-10-02 23:19:14.722278968 +0200
+++ linux/include/pcmcia/ds.h	2003-10-02 23:21:06.931220616 +0200
@@ -134,10 +134,20 @@
 struct pcmcia_device;
 
 struct pcmcia_driver {
-	dev_link_t		*(*attach)(struct pcmcia_device *);
-	void			(*detach)(dev_link_t *);
 	struct module		*owner;
 	struct device_driver	drv;
+
+	dev_link_t	*(*attach)	(struct pcmcia_device *);
+	void		(*detach)	(dev_link_t *);
+
+	int		(*probe)	(struct pcmcia_device *);
+	int		(*remove)	(struct pcmcia_device *);
+
+	int		(*suspend)	(struct pcmcia_device *);
+	int		(*resume)	(struct pcmcia_device *);
+
+	int		(*pre_reset)	(struct pcmcia_device *);
+	int		(*post_reset)	(struct pcmcia_device *);
 };
 
 /* driver registration */
-------------- next part --------------
Update the pcnet_cs driver to reflect the core changes as an example.

 pcnet_cs.c |  117 ++++++++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 77 insertions(+), 40 deletions(-)

diff -ruN linux-original/drivers/net/pcmcia/pcnet_cs.c linux/drivers/net/pcmcia/pcnet_cs.c
--- linux-original/drivers/net/pcmcia/pcnet_cs.c	2003-10-02 23:19:14.701282160 +0200
+++ linux/drivers/net/pcmcia/pcnet_cs.c	2003-10-02 23:25:14.765544072 +0200
@@ -297,10 +297,7 @@
     dev_list = link;
     client_reg.dev_info = &dev_info;
     client_reg.Attributes = INFO_IO_CLIENT | INFO_CARD_SHARE;
-    client_reg.EventMask =
-	CS_EVENT_CARD_INSERTION | CS_EVENT_CARD_REMOVAL |
-	CS_EVENT_RESET_PHYSICAL | CS_EVENT_CARD_RESET |
-	CS_EVENT_PM_SUSPEND | CS_EVENT_PM_RESUME;
+    client_reg.EventMask = 0;
     client_reg.event_handler = &pcnet_event;
     client_reg.Version = 0x0210;
     client_reg.event_callback_args.client_data = link;
@@ -814,53 +811,82 @@
 
 ======================================================================*/
 
-static int pcnet_event(event_t event, int priority,
-		       event_callback_args_t *args)
+static int pcnet_pre_reset(struct pcmcia_device *p_dev)
 {
-    dev_link_t *link = args->client_data;
-    pcnet_dev_t *info = link->priv;
+	dev_link_t *link = p_dev->instance;
+	pcnet_dev_t *info = link->priv;
+	if (link->state & DEV_CONFIG) {
+		if (link->open)
+			netif_device_detach(&info->dev);
+		CardServices(ReleaseConfiguration, link->handle);
+	}
+	return 0;
+}
 
-    DEBUG(2, "pcnet_event(0x%06x)\n", event);
 
-    switch (event) {
-    case CS_EVENT_CARD_REMOVAL:
-	link->state &= ~DEV_PRESENT;
+static int pcnet_post_reset(struct pcmcia_device *p_dev)
+{
+	dev_link_t *link = p_dev->instance;
+	pcnet_dev_t *info = link->priv;
 	if (link->state & DEV_CONFIG) {
-	    netif_device_detach(&info->dev);
-	    pcnet_release(link);
+		CardServices(RequestConfiguration, link->handle, &link->conf);
+		if (link->open) {
+			pcnet_reset_8390(&info->dev);
+			NS8390_init(&info->dev, 1);
+			netif_device_attach(&info->dev);
+		}
 	}
-	break;
-    case CS_EVENT_CARD_INSERTION:
+	return 0;
+}
+
+
+static int pcnet_suspend(struct pcmcia_device *p_dev)
+{
+	dev_link_t *link = p_dev->instance;
+	link->state |= DEV_SUSPEND;
+	return pcnet_pre_reset(p_dev);
+}
+
+
+static int pcnet_resume(struct pcmcia_device *p_dev)
+{
+	dev_link_t *link = p_dev->instance;
+	link->state &= ~DEV_SUSPEND;
+	return pcnet_post_reset(p_dev);
+}
+
+
+static int pcnet_probe(struct pcmcia_device *p_dev)
+{
+	dev_link_t *link = p_dev->instance;
+
 	link->state |= DEV_PRESENT | DEV_CONFIG_PENDING;
 	pcnet_config(link);
-	break;
-    case CS_EVENT_PM_SUSPEND:
-	link->state |= DEV_SUSPEND;
-	/* Fall through... */
-    case CS_EVENT_RESET_PHYSICAL:
+	return 0;
+}
+
+
+static int pcnet_remove(struct pcmcia_device *p_dev)
+{
+	dev_link_t *link = p_dev->instance;
+	pcnet_dev_t *info = link->priv;
+
+	link->state &= ~DEV_PRESENT;
 	if (link->state & DEV_CONFIG) {
-	    if (link->open)
 		netif_device_detach(&info->dev);
-	    CardServices(ReleaseConfiguration, link->handle);
-	}
-	break;
-    case CS_EVENT_PM_RESUME:
-	link->state &= ~DEV_SUSPEND;
-	/* Fall through... */
-    case CS_EVENT_CARD_RESET:
-	if (link->state & DEV_CONFIG) {
-	    CardServices(RequestConfiguration, link->handle, &link->conf);
-	    if (link->open) {
-		pcnet_reset_8390(&info->dev);
-		NS8390_init(&info->dev, 1);
-		netif_device_attach(&info->dev);
-	    }
+		pcnet_release(link);
 	}
-	break;
-    }
-    return 0;
+	return 0;
+}
+
+
+static int pcnet_event(event_t event, int priority,
+		       event_callback_args_t *args)
+{
+	return 0;
 } /* pcnet_event */
 
+
 /*======================================================================
 
     MII interface support for DL10019 and DL10022 based cards
@@ -1566,12 +1592,23 @@
 /*====================================================================*/
 
 static struct pcmcia_driver pcnet_driver = {
+	.owner		= THIS_MODULE,
+
 	.drv		= {
 		.name	= "pcnet_cs",
 	},
+
 	.attach		= pcnet_attach,
 	.detach		= pcnet_detach,
-	.owner		= THIS_MODULE,
+
+	.probe		= pcnet_probe,
+	.remove		= pcnet_remove,
+
+	.pre_reset	= pcnet_pre_reset,
+	.post_reset	= pcnet_post_reset,
+
+	.suspend	= pcnet_suspend,
+	.resume		= pcnet_resume,
 };
 
 static int __init init_pcnet_cs(void)