From patchwork Tue Apr 27 13:18:13 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Schichan X-Patchwork-Id: 51077 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 0E09FB7D5B for ; Tue, 27 Apr 2010 23:18:26 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755531Ab0D0NSY (ORCPT ); Tue, 27 Apr 2010 09:18:24 -0400 Received: from smtp4-g21.free.fr ([212.27.42.4]:42483 "EHLO smtp4-g21.free.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755433Ab0D0NSX (ORCPT ); Tue, 27 Apr 2010 09:18:23 -0400 Received: from smtp4-g21.free.fr (localhost [127.0.0.1]) by smtp4-g21.free.fr (Postfix) with ESMTP id 922C34C8152; Tue, 27 Apr 2010 15:18:15 +0200 (CEST) Received: from bobafett.staff.proxad.net (bobafett.staff.proxad.net [213.228.1.121]) by smtp4-g21.free.fr (Postfix) with ESMTP id BC97A4C814B; Tue, 27 Apr 2010 15:18:13 +0200 (CEST) Received: from daria.localnet (unknown [172.18.3.120]) by bobafett.staff.proxad.net (Postfix) with ESMTP id B9799180467; Tue, 27 Apr 2010 15:18:13 +0200 (CEST) From: Nicolas Schichan Organization: Freebox To: Greg KH Subject: Re: [RFC] add port information for ATA devices in sysfs Date: Tue, 27 Apr 2010 15:18:13 +0200 User-Agent: KMail/1.12.2 (Linux/2.6.31-20-generic; KDE/4.3.2; x86_64; ; ) Cc: Jeff Garzik , linux-ide@vger.kernel.org, Maxime Bizon References: <201004261829.04687.nschichan@freebox.fr> <20100427034210.GB4759@kroah.com> In-Reply-To: <20100427034210.GB4759@kroah.com> MIME-Version: 1.0 Message-Id: <201004271518.13611.nschichan@freebox.fr> Sender: linux-ide-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ide@vger.kernel.org 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 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;