On Tuesday 27 April 2010 05:42:10 am Greg KH wrote:
> On Mon, Apr 26, 2010 at 06:29:04PM +0200, Nicolas Schichan wrote:
> > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> > index 91fed3c..179abad 100644
> > --- a/drivers/ata/libata-core.c
> > +++ b/drivers/ata/libata-core.c
> > @@ -5660,6 +5660,10 @@ int sata_link_init_spd(struct ata_link *link)
> > return 0;
> > }
> >
> > +static void ata_port_dev_release(struct device *dev)
> > +{
> > +}
>
> {sigh}
>
> By doing the above, you have guaranteed that I will make fun of this
> code. Consider this the ridicule it deserves.
>
> (hint, read the kobject documentation for why I get to make fun of
> it...)
>
> Think about why you created an empty release function, to try to get the
> kernel to stop spitting out a message to you.
That's right, shame on me for not reading the documentation.
> Didn't you think that the
> message was there for a reason, and it was not to be worked around?
Well after reading the kobject documentation, I understand why it is
bad thing to have this function empty. Since someone may still hold a
reference on the device when I call device_unregister(), I guess the
only safe place where to kfree the struct ata_port is in the release
callback of the device.
Please find an updated patch addressing your comments below:
Signed-off-by: Nicolas Schichan <nschichan@xxxxxxxxxx>
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 91fed3c..f96d8c0 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5660,6 +5660,14 @@ int sata_link_init_spd(struct ata_link *link)
return 0;
}
+static void ata_port_dev_release(struct device *dev)
+{
+ struct ata_port *ap;
+
+ ap = container_of(dev, struct ata_port, ata_port_device);
+ kfree(ap);
+}
+
/**
* ata_port_alloc - allocate and initialize basic ATA port resources
* @host: ATA host this allocated port belongs to
@@ -5672,7 +5680,7 @@ int sata_link_init_spd(struct ata_link *link)
* LOCKING:
* Inherited from calling layer (may sleep).
*/
-struct ata_port *ata_port_alloc(struct ata_host *host)
+struct ata_port *ata_port_alloc(struct ata_host *host, int port_no)
{
struct ata_port *ap;
@@ -5690,6 +5698,20 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
ap->host = host;
ap->dev = host->dev;
ap->last_ctl = 0xFF;
+ ap->port_no = port_no;
+
+ /*
+ * register device used to add the scsi host behind this
+ * port. make it a parent of the struct device of the host.
+ */
+ ap->ata_port_device.release = ata_port_dev_release;
+ dev_set_name(&ap->ata_port_device, "port%i", ap->port_no);
+ ap->ata_port_device.parent = ap->dev;
+ if (device_register(&ap->ata_port_device) < 0) {
+ kfree(ap);
+ return NULL;
+ }
+
#if defined(ATA_VERBOSE_DEBUG)
/* turn on all debugging levels */
@@ -5741,7 +5763,11 @@ static void ata_host_release(struct device *gendev,
void *res)
kfree(ap->pmp_link);
kfree(ap->slave_link);
- kfree(ap);
+
+ /*
+ * ap will be kfree'ed in device release callback.
+ */
+ device_unregister(&ap->ata_port_device);
host->ports[i] = NULL;
}
@@ -5797,11 +5823,10 @@ struct ata_host *ata_host_alloc(struct device *dev,
int max_ports)
for (i = 0; i < max_ports; i++) {
struct ata_port *ap;
- ap = ata_port_alloc(host);
+ ap = ata_port_alloc(host, i);
if (!ap)
goto err_out;
- ap->port_no = i;
host->ports[i] = ap;
}
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index b4ee28d..1cbda75 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3210,7 +3210,7 @@ int ata_scsi_add_hosts(struct ata_host *host, struct
scsi_host_template *sht)
*/
shost->max_host_blocked = 1;
- rc = scsi_add_host(ap->scsi_host, ap->host->dev);
+ rc = scsi_add_host(ap->scsi_host, &ap->ata_port_device);
if (rc)
goto err_add;
}
@@ -3593,11 +3593,10 @@ struct ata_port *ata_sas_port_alloc(struct ata_host
*host,
{
struct ata_port *ap;
- ap = ata_port_alloc(host);
+ ap = ata_port_alloc(host, 0);
if (!ap)
return NULL;
- ap->port_no = 0;
ap->lock = shost->host_lock;
ap->pio_mask = port_info->pio_mask;
ap->mwdma_mask = port_info->mwdma_mask;
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 823e630..cd44e9e 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -112,7 +112,7 @@ extern void ata_link_init(struct ata_port *ap, struct
ata_link *link, int pmp);
extern int sata_link_init_spd(struct ata_link *link);
extern int ata_task_ioctl(struct scsi_device *scsidev, void __user *arg);
extern int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg);
-extern struct ata_port *ata_port_alloc(struct ata_host *host);
+extern struct ata_port *ata_port_alloc(struct ata_host *host, int port_no);
extern void ata_dev_enable_pm(struct ata_device *dev, enum link_pm policy);
extern void ata_lpm_schedule(struct ata_port *ap, enum link_pm);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index b0f6d97..64d94d9 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -749,6 +749,9 @@ struct ata_port {
struct ata_host *host;
struct device *dev;
+ /* used as base to create scsi host behind the port.*/
+ struct device ata_port_device;
+
void *port_task_data;
struct delayed_work port_task;
struct delayed_work hotplug_task;
--
Nicolas Schichan
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
