Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754027AbbK3OHi (ORCPT ); Mon, 30 Nov 2015 09:07:38 -0500 Received: from mail-yk0-f178.google.com ([209.85.160.178]:33178 "EHLO mail-yk0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753150AbbK3OHg (ORCPT ); Mon, 30 Nov 2015 09:07:36 -0500 MIME-Version: 1.0 In-Reply-To: <1448542190-19498-4-git-send-email-m.szyprowski@samsung.com> References: <1448542190-19498-1-git-send-email-m.szyprowski@samsung.com> <1448542190-19498-4-git-send-email-m.szyprowski@samsung.com> Date: Mon, 30 Nov 2015 15:07:35 +0100 Message-ID: Subject: Re: [PATCH v2 3/4] ARM: amba: Move reading of periphid to amba_match() From: Ulf Hansson To: Marek Szyprowski , Tomeu Vizoso Cc: linux-samsung-soc , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , Russell King - ARM Linux , Greg Kroah-Hartman , Kukjin Kim , Krzysztof Kozlowski , Bartlomiej Zolnierkiewicz Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6155 Lines: 189 On 26 November 2015 at 13:49, Marek Szyprowski wrote: > From: Tomeu Vizoso > > Reading the periphid when the Primecell device is registered means that > the apb pclk must be available by then or the device won't be registered > at all. > > By reading the periphid in amba_match() we can return -EPROBE_DEFER if > the apb pclk isn't there yet and the device will be retried later. > > Signed-off-by: Tomeu Vizoso > Signed-off-by: Marek Szyprowski > --- > drivers/amba/bus.c | 88 ++++++++++++++++++++++++++++-------------------------- > 1 file changed, 46 insertions(+), 42 deletions(-) > > diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c > index f009936..72ebf9b 100644 > --- a/drivers/amba/bus.c > +++ b/drivers/amba/bus.c > @@ -24,6 +24,8 @@ > > #define to_amba_driver(d) container_of(d, struct amba_driver, drv) > > +static int read_periphid(struct amba_device *d, unsigned int *periphid); I would change this to: static int amba_read_periphid(struct amba_device *dev); That will actually also make the patch smaller. > + > static const struct amba_id * > amba_lookup(const struct amba_id *table, struct amba_device *dev) > { > @@ -43,11 +45,22 @@ static int amba_match(struct device *dev, struct device_driver *drv) > { > struct amba_device *pcdev = to_amba_device(dev); > struct amba_driver *pcdrv = to_amba_driver(drv); > + int ret; > > /* When driver_override is set, only bind to the matching driver */ > if (pcdev->driver_override) > return !strcmp(pcdev->driver_override, drv->name); > > + if (!pcdev->periphid) { > + ret = read_periphid(pcdev, &pcdev->periphid); > + if (ret) { > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Failed to read periphid: %d", > + ret); > + return ret; > + } > + } > + > return amba_lookup(pcdrv->id_table, pcdev) != NULL; > } > > @@ -336,44 +349,22 @@ static void amba_device_release(struct device *dev) > kfree(d); > } > > -/** > - * amba_device_add - add a previously allocated AMBA device structure > - * @dev: AMBA device allocated by amba_device_alloc > - * @parent: resource parent for this devices resources > - * > - * Claim the resource, and read the device cell ID if not already > - * initialized. Register the AMBA device with the Linux device > - * manager. > - */ > -int amba_device_add(struct amba_device *dev, struct resource *parent) > +static int read_periphid(struct amba_device *d, unsigned int *periphid) > { > u32 size; > void __iomem *tmp; > - int i, ret; > - > - WARN_ON(dev->irq[0] == (unsigned int)-1); > - WARN_ON(dev->irq[1] == (unsigned int)-1); > - > - ret = request_resource(parent, &dev->res); > - if (ret) > - goto err_out; > - > - /* Hard-coded primecell ID instead of plug-n-play */ This line now belongs in amba_match(), please move it there instead of just deleting it. > - if (dev->periphid != 0) > - goto skip_probe; > + int i, ret = 0; > > /* > * Dynamically calculate the size of the resource > * and use this for iomap > */ > - size = resource_size(&dev->res); > - tmp = ioremap(dev->res.start, size); > - if (!tmp) { > - ret = -ENOMEM; > - goto err_release; > - } > + size = resource_size(&d->res); > + tmp = ioremap(d->res.start, size); > + if (!tmp) > + return -ENOMEM; > > - ret = amba_get_enable_pclk(dev); > + ret = amba_get_enable_pclk(d); > if (ret == 0) { > u32 pid, cid; > > @@ -388,37 +379,50 @@ int amba_device_add(struct amba_device *dev, struct resource *parent) > cid |= (readl(tmp + size - 0x10 + 4 * i) & 255) << > (i * 8); > > - amba_put_disable_pclk(dev); > + amba_put_disable_pclk(d); > > if (cid == AMBA_CID || cid == CORESIGHT_CID) > - dev->periphid = pid; > + *periphid = pid; > > - if (!dev->periphid) > + if (!*periphid) > ret = -ENODEV; > } > > iounmap(tmp); > > + return ret; > +} > + > +/** > + * amba_device_add - add a previously allocated AMBA device structure > + * @dev: AMBA device allocated by amba_device_alloc > + * @parent: resource parent for this devices resources > + * > + * Claim the resource, and register the AMBA device with the Linux device > + * manager. > + */ > +int amba_device_add(struct amba_device *dev, struct resource *parent) > +{ > + int ret; > + > + WARN_ON(dev->irq[0] == (unsigned int)-1); > + WARN_ON(dev->irq[1] == (unsigned int)-1); > + > + ret = request_resource(parent, &dev->res); > if (ret) > - goto err_release; > + return ret; > > - skip_probe: > ret = device_add(&dev->dev); > if (ret) > - goto err_release; > + return ret; > > if (dev->irq[0]) > ret = device_create_file(&dev->dev, &dev_attr_irq0); > if (ret == 0 && dev->irq[1]) > ret = device_create_file(&dev->dev, &dev_attr_irq1); > - if (ret == 0) > - return ret; > - > - device_unregister(&dev->dev); > + if (ret) > + device_unregister(&dev->dev); > > - err_release: > - release_resource(&dev->res); > - err_out: > return ret; > } > EXPORT_SYMBOL_GPL(amba_device_add); > -- > 1.9.2 > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/