All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/7] i2c-designware: move to managed functions (devm_*)
@ 2013-04-10 10:36 ` Mika Westerberg
  0 siblings, 0 replies; 13+ messages in thread
From: Mika Westerberg @ 2013-04-10 10:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, Wolfram Sang, ben-linux, Jean Delvare,
	Andy Shevchenko, Christian Ruppert, Mika Westerberg

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

This makes the error handling much more simpler than open-coding everything
and in addition makes the probe function smaller and tidier.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Changes to v1:
	- dropped redundant dev_err() after devm_ioremap_resource().

 drivers/i2c/busses/i2c-designware-platdrv.c |   73 +++++++--------------------
 1 file changed, 19 insertions(+), 54 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index e3085c4..94b3a4d 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -92,7 +92,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
-	struct resource *mem, *ioarea;
+	struct resource *mem;
 	int irq, r;
 
 	/* NOTE: driver uses the static register mapping */
@@ -108,32 +108,25 @@ static int dw_i2c_probe(struct platform_device *pdev)
 		return irq; /* -ENXIO */
 	}
 
-	ioarea = request_mem_region(mem->start, resource_size(mem),
-			pdev->name);
-	if (!ioarea) {
-		dev_err(&pdev->dev, "I2C region already claimed\n");
-		return -EBUSY;
-	}
+	dev = devm_kzalloc(&pdev->dev, sizeof(struct dw_i2c_dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
 
-	dev = kzalloc(sizeof(struct dw_i2c_dev), GFP_KERNEL);
-	if (!dev) {
-		r = -ENOMEM;
-		goto err_release_region;
-	}
+	dev->base = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(dev->base))
+		return PTR_ERR(dev->base);
 
 	init_completion(&dev->cmd_complete);
 	mutex_init(&dev->lock);
-	dev->dev = get_device(&pdev->dev);
+	dev->dev = &pdev->dev;
 	dev->irq = irq;
 	platform_set_drvdata(pdev, dev);
 
-	dev->clk = clk_get(&pdev->dev, NULL);
+	dev->clk = devm_clk_get(&pdev->dev, NULL);
 	dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
 
-	if (IS_ERR(dev->clk)) {
-		r = -ENODEV;
-		goto err_free_mem;
-	}
+	if (IS_ERR(dev->clk))
+		return PTR_ERR(dev->clk);
 	clk_prepare_enable(dev->clk);
 
 	dev->functionality =
@@ -146,13 +139,6 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
 		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
 
-	dev->base = ioremap(mem->start, resource_size(mem));
-	if (dev->base == NULL) {
-		dev_err(&pdev->dev, "failure mapping io resources\n");
-		r = -EBUSY;
-		goto err_unuse_clocks;
-	}
-
 	/* Try first if we can configure the device from ACPI */
 	r = dw_i2c_acpi_configure(pdev);
 	if (r) {
@@ -164,13 +150,14 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	}
 	r = i2c_dw_init(dev);
 	if (r)
-		goto err_iounmap;
+		return r;
 
 	i2c_dw_disable_int(dev);
-	r = request_irq(dev->irq, i2c_dw_isr, IRQF_SHARED, pdev->name, dev);
+	r = devm_request_irq(&pdev->dev, dev->irq, i2c_dw_isr, IRQF_SHARED,
+			pdev->name, dev);
 	if (r) {
 		dev_err(&pdev->dev, "failure requesting irq %i\n", dev->irq);
-		goto err_iounmap;
+		return r;
 	}
 
 	adap = &dev->adapter;
@@ -186,57 +173,35 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	r = i2c_add_numbered_adapter(adap);
 	if (r) {
 		dev_err(&pdev->dev, "failure adding adapter\n");
-		goto err_free_irq;
+		return r;
 	}
 	of_i2c_register_devices(adap);
 	acpi_i2c_register_devices(adap);
 
+	/* Increase reference counter */
+	get_device(&pdev->dev);
+
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_put(&pdev->dev);
 
 	return 0;
-
-err_free_irq:
-	free_irq(dev->irq, dev);
-err_iounmap:
-	iounmap(dev->base);
-err_unuse_clocks:
-	clk_disable_unprepare(dev->clk);
-	clk_put(dev->clk);
-	dev->clk = NULL;
-err_free_mem:
-	put_device(&pdev->dev);
-	kfree(dev);
-err_release_region:
-	release_mem_region(mem->start, resource_size(mem));
-
-	return r;
 }
 
 static int dw_i2c_remove(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
-	struct resource *mem;
 
 	pm_runtime_get_sync(&pdev->dev);
 
 	i2c_del_adapter(&dev->adapter);
 	put_device(&pdev->dev);
 
-	clk_disable_unprepare(dev->clk);
-	clk_put(dev->clk);
-	dev->clk = NULL;
-
 	i2c_dw_disable(dev);
-	free_irq(dev->irq, dev);
-	kfree(dev);
 
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	release_mem_region(mem->start, resource_size(mem));
 	return 0;
 }
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 1/7] i2c-designware: move to managed functions (devm_*)
@ 2013-04-10 10:36 ` Mika Westerberg
  0 siblings, 0 replies; 13+ messages in thread
From: Mika Westerberg @ 2013-04-10 10:36 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang,
	ben-linux-elnMNo+KYs3YtjvyW6yDsg, Jean Delvare, Andy Shevchenko,
	Christian Ruppert, Mika Westerberg

From: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

This makes the error handling much more simpler than open-coding everything
and in addition makes the probe function smaller and tidier.

Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
Changes to v1:
	- dropped redundant dev_err() after devm_ioremap_resource().

 drivers/i2c/busses/i2c-designware-platdrv.c |   73 +++++++--------------------
 1 file changed, 19 insertions(+), 54 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index e3085c4..94b3a4d 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -92,7 +92,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
-	struct resource *mem, *ioarea;
+	struct resource *mem;
 	int irq, r;
 
 	/* NOTE: driver uses the static register mapping */
@@ -108,32 +108,25 @@ static int dw_i2c_probe(struct platform_device *pdev)
 		return irq; /* -ENXIO */
 	}
 
-	ioarea = request_mem_region(mem->start, resource_size(mem),
-			pdev->name);
-	if (!ioarea) {
-		dev_err(&pdev->dev, "I2C region already claimed\n");
-		return -EBUSY;
-	}
+	dev = devm_kzalloc(&pdev->dev, sizeof(struct dw_i2c_dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
 
-	dev = kzalloc(sizeof(struct dw_i2c_dev), GFP_KERNEL);
-	if (!dev) {
-		r = -ENOMEM;
-		goto err_release_region;
-	}
+	dev->base = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(dev->base))
+		return PTR_ERR(dev->base);
 
 	init_completion(&dev->cmd_complete);
 	mutex_init(&dev->lock);
-	dev->dev = get_device(&pdev->dev);
+	dev->dev = &pdev->dev;
 	dev->irq = irq;
 	platform_set_drvdata(pdev, dev);
 
-	dev->clk = clk_get(&pdev->dev, NULL);
+	dev->clk = devm_clk_get(&pdev->dev, NULL);
 	dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
 
-	if (IS_ERR(dev->clk)) {
-		r = -ENODEV;
-		goto err_free_mem;
-	}
+	if (IS_ERR(dev->clk))
+		return PTR_ERR(dev->clk);
 	clk_prepare_enable(dev->clk);
 
 	dev->functionality =
@@ -146,13 +139,6 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
 		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
 
-	dev->base = ioremap(mem->start, resource_size(mem));
-	if (dev->base == NULL) {
-		dev_err(&pdev->dev, "failure mapping io resources\n");
-		r = -EBUSY;
-		goto err_unuse_clocks;
-	}
-
 	/* Try first if we can configure the device from ACPI */
 	r = dw_i2c_acpi_configure(pdev);
 	if (r) {
@@ -164,13 +150,14 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	}
 	r = i2c_dw_init(dev);
 	if (r)
-		goto err_iounmap;
+		return r;
 
 	i2c_dw_disable_int(dev);
-	r = request_irq(dev->irq, i2c_dw_isr, IRQF_SHARED, pdev->name, dev);
+	r = devm_request_irq(&pdev->dev, dev->irq, i2c_dw_isr, IRQF_SHARED,
+			pdev->name, dev);
 	if (r) {
 		dev_err(&pdev->dev, "failure requesting irq %i\n", dev->irq);
-		goto err_iounmap;
+		return r;
 	}
 
 	adap = &dev->adapter;
@@ -186,57 +173,35 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	r = i2c_add_numbered_adapter(adap);
 	if (r) {
 		dev_err(&pdev->dev, "failure adding adapter\n");
-		goto err_free_irq;
+		return r;
 	}
 	of_i2c_register_devices(adap);
 	acpi_i2c_register_devices(adap);
 
+	/* Increase reference counter */
+	get_device(&pdev->dev);
+
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_put(&pdev->dev);
 
 	return 0;
-
-err_free_irq:
-	free_irq(dev->irq, dev);
-err_iounmap:
-	iounmap(dev->base);
-err_unuse_clocks:
-	clk_disable_unprepare(dev->clk);
-	clk_put(dev->clk);
-	dev->clk = NULL;
-err_free_mem:
-	put_device(&pdev->dev);
-	kfree(dev);
-err_release_region:
-	release_mem_region(mem->start, resource_size(mem));
-
-	return r;
 }
 
 static int dw_i2c_remove(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
-	struct resource *mem;
 
 	pm_runtime_get_sync(&pdev->dev);
 
 	i2c_del_adapter(&dev->adapter);
 	put_device(&pdev->dev);
 
-	clk_disable_unprepare(dev->clk);
-	clk_put(dev->clk);
-	dev->clk = NULL;
-
 	i2c_dw_disable(dev);
-	free_irq(dev->irq, dev);
-	kfree(dev);
 
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	release_mem_region(mem->start, resource_size(mem));
 	return 0;
 }
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/7] i2c-designware-pci: use dev_err() instead of printk()
@ 2013-04-10 10:36   ` Mika Westerberg
  0 siblings, 0 replies; 13+ messages in thread
From: Mika Westerberg @ 2013-04-10 10:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, Wolfram Sang, ben-linux, Jean Delvare,
	Andy Shevchenko, Christian Ruppert, Mika Westerberg

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

With dev_err() we can get the device instance printed as well and is pretty
much standard to use dev_* macros in the drivers anyway. In addition
correct the indentation of probe() arguments.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 7c5e383..eed149d 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -208,7 +208,7 @@ static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
 }
 
 static int i2c_dw_pci_probe(struct pci_dev *pdev,
-const struct pci_device_id *id)
+			    const struct pci_device_id *id)
 {
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
@@ -218,7 +218,7 @@ const struct pci_device_id *id)
 	struct  dw_pci_controller *controller;
 
 	if (id->driver_data >= ARRAY_SIZE(dw_pci_controllers)) {
-		printk(KERN_ERR "dw_i2c_pci_probe: invalid driver data %ld\n",
+		dev_err(&pdev->dev, "%s: invalid driver data %ld\n", __func__,
 			id->driver_data);
 		return -EINVAL;
 	}
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/7] i2c-designware-pci: use dev_err() instead of printk()
@ 2013-04-10 10:36   ` Mika Westerberg
  0 siblings, 0 replies; 13+ messages in thread
From: Mika Westerberg @ 2013-04-10 10:36 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang,
	ben-linux-elnMNo+KYs3YtjvyW6yDsg, Jean Delvare, Andy Shevchenko,
	Christian Ruppert, Mika Westerberg

From: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

With dev_err() we can get the device instance printed as well and is pretty
much standard to use dev_* macros in the drivers anyway. In addition
correct the indentation of probe() arguments.

Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 7c5e383..eed149d 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -208,7 +208,7 @@ static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
 }
 
 static int i2c_dw_pci_probe(struct pci_dev *pdev,
-const struct pci_device_id *id)
+			    const struct pci_device_id *id)
 {
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
@@ -218,7 +218,7 @@ const struct pci_device_id *id)
 	struct  dw_pci_controller *controller;
 
 	if (id->driver_data >= ARRAY_SIZE(dw_pci_controllers)) {
-		printk(KERN_ERR "dw_i2c_pci_probe: invalid driver data %ld\n",
+		dev_err(&pdev->dev, "%s: invalid driver data %ld\n", __func__,
 			id->driver_data);
 		return -EINVAL;
 	}
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/7] i2c-designware-pci: use managed functions pcim_* and devm_*
  2013-04-10 10:36 ` Mika Westerberg
  (?)
  (?)
@ 2013-04-10 10:36 ` Mika Westerberg
  -1 siblings, 0 replies; 13+ messages in thread
From: Mika Westerberg @ 2013-04-10 10:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, Wolfram Sang, ben-linux, Jean Delvare,
	Andy Shevchenko, Christian Ruppert, Mika Westerberg

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

This makes the error handling much more simpler than open-coding everything
and in addition makes the probe function smaller an tidier.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c |   68 +++++++---------------------
 1 file changed, 17 insertions(+), 51 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index eed149d..aacb64e 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -212,8 +212,6 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 {
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
-	unsigned long start, len;
-	void __iomem *base;
 	int r;
 	struct  dw_pci_controller *controller;
 
@@ -225,51 +223,30 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 
 	controller = &dw_pci_controllers[id->driver_data];
 
-	r = pci_enable_device(pdev);
+	r = pcim_enable_device(pdev);
 	if (r) {
 		dev_err(&pdev->dev, "Failed to enable I2C PCI device (%d)\n",
 			r);
-		goto exit;
+		return r;
 	}
 
-	/* Determine the address of the I2C area */
-	start = pci_resource_start(pdev, 0);
-	len = pci_resource_len(pdev, 0);
-	if (!start || len == 0) {
-		dev_err(&pdev->dev, "base address not set\n");
-		r = -ENODEV;
-		goto exit;
-	}
-
-	r = pci_request_region(pdev, 0, DRIVER_NAME);
+	r = pcim_iomap_regions(pdev, 1 << 0, pci_name(pdev));
 	if (r) {
-		dev_err(&pdev->dev, "failed to request I2C region "
-			"0x%lx-0x%lx\n", start,
-			(unsigned long)pci_resource_end(pdev, 0));
-		goto exit;
-	}
-
-	base = ioremap_nocache(start, len);
-	if (!base) {
 		dev_err(&pdev->dev, "I/O memory remapping failed\n");
-		r = -ENOMEM;
-		goto err_release_region;
+		return r;
 	}
 
-
-	dev = kzalloc(sizeof(struct dw_i2c_dev), GFP_KERNEL);
-	if (!dev) {
-		r = -ENOMEM;
-		goto err_release_region;
-	}
+	dev = devm_kzalloc(&pdev->dev, sizeof(struct dw_i2c_dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
 
 	init_completion(&dev->cmd_complete);
 	mutex_init(&dev->lock);
 	dev->clk = NULL;
 	dev->controller = controller;
 	dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
-	dev->base = base;
-	dev->dev = get_device(&pdev->dev);
+	dev->base = pcim_iomap_table(pdev)[0];
+	dev->dev = &pdev->dev;
 	dev->functionality =
 		I2C_FUNC_I2C |
 		I2C_FUNC_SMBUS_BYTE |
@@ -284,7 +261,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	dev->rx_fifo_depth = controller->rx_fifo_depth;
 	r = i2c_dw_init(dev);
 	if (r)
-		goto err_iounmap;
+		return r;
 
 	adap = &dev->adapter;
 	i2c_set_adapdata(adap, dev);
@@ -296,10 +273,11 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	snprintf(adap->name, sizeof(adap->name), "i2c-designware-pci-%d",
 		adap->nr);
 
-	r = request_irq(pdev->irq, i2c_dw_isr, IRQF_SHARED, adap->name, dev);
+	r = devm_request_irq(&pdev->dev, pdev->irq, i2c_dw_isr, IRQF_SHARED,
+			adap->name, dev);
 	if (r) {
 		dev_err(&pdev->dev, "failure requesting irq %i\n", dev->irq);
-		goto err_iounmap;
+		return r;
 	}
 
 	i2c_dw_disable_int(dev);
@@ -307,24 +285,16 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	r = i2c_add_numbered_adapter(adap);
 	if (r) {
 		dev_err(&pdev->dev, "failure adding adapter\n");
-		goto err_free_irq;
+		return r;
 	}
 
+	/* Increase reference counter */
+	get_device(&pdev->dev);
+
 	pm_runtime_put_noidle(&pdev->dev);
 	pm_runtime_allow(&pdev->dev);
 
 	return 0;
-
-err_free_irq:
-	free_irq(pdev->irq, dev);
-err_iounmap:
-	iounmap(dev->base);
-	put_device(&pdev->dev);
-	kfree(dev);
-err_release_region:
-	pci_release_region(pdev, 0);
-exit:
-	return r;
 }
 
 static void i2c_dw_pci_remove(struct pci_dev *pdev)
@@ -337,10 +307,6 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev)
 
 	i2c_del_adapter(&dev->adapter);
 	put_device(&pdev->dev);
-
-	free_irq(dev->irq, dev);
-	kfree(dev);
-	pci_release_region(pdev, 0);
 }
 
 /* work with hotplug and coldplug */
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/7] i2c-designware: use dynamic adapter numbering on Lynxpoint
  2013-04-10 10:36 ` Mika Westerberg
                   ` (2 preceding siblings ...)
  (?)
@ 2013-04-10 10:36 ` Mika Westerberg
  -1 siblings, 0 replies; 13+ messages in thread
From: Mika Westerberg @ 2013-04-10 10:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, Wolfram Sang, ben-linux, Jean Delvare,
	Andy Shevchenko, Christian Ruppert, Mika Westerberg

It is not good idea to mix static and dynamic I2C adapter numbering. In
this particular case on Lynxpoint we had graphics I2C adapter which took
the first numbers preventing the designware I2C driver from using the
adapter numbers it preferred.

Since Lynxpoint support was just introduced and there is no hardware available
outside Intel we can fix this by switching to use dynamic adapter numbering
instead of static.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Changes to v1:
	- Updated commit message to mention that this change should not
	  cause regressions as there are no real users for Lynxpoint
	  I2C controller driver yet.

 drivers/i2c/busses/i2c-designware-platdrv.c |    9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 94b3a4d..0735ccf 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -56,20 +56,11 @@ static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
 static int dw_i2c_acpi_configure(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
-	struct acpi_device *adev;
-	int busno, ret;
 
 	if (!ACPI_HANDLE(&pdev->dev))
 		return -ENODEV;
 
-	ret = acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev);
-	if (ret)
-		return -ENODEV;
-
 	dev->adapter.nr = -1;
-	if (adev->pnp.unique_id && !kstrtoint(adev->pnp.unique_id, 0, &busno))
-		dev->adapter.nr = busno;
-
 	dev->tx_fifo_depth = 32;
 	dev->rx_fifo_depth = 32;
 	return 0;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 5/7] i2c-designware: enable/disable the controller properly
  2013-04-10 10:36 ` Mika Westerberg
                   ` (3 preceding siblings ...)
  (?)
@ 2013-04-10 10:36 ` Mika Westerberg
  -1 siblings, 0 replies; 13+ messages in thread
From: Mika Westerberg @ 2013-04-10 10:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, Wolfram Sang, ben-linux, Jean Delvare,
	Andy Shevchenko, Christian Ruppert, Mika Westerberg

The correct way to disable or enable the controller is to wait until the
DW_IC_ENABLE_STATUS register bit matches the bit we program into DW_IC_ENABLE
register. This procedure is described in the DesignWare I2C databook.

By doing this we can be sure that the controller is in correct state once
the function returns.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Changes to v1:
	- Added comment about why we sleep 25us after each iteration.
	- } while (timeout--); instead of } while (timeout-- > 0);

 drivers/i2c/busses/i2c-designware-core.c |   34 ++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 94fd818..5cc4ebf 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -68,6 +68,7 @@
 #define DW_IC_TXFLR		0x74
 #define DW_IC_RXFLR		0x78
 #define DW_IC_TX_ABRT_SOURCE	0x80
+#define DW_IC_ENABLE_STATUS	0x9c
 #define DW_IC_COMP_PARAM_1	0xf4
 #define DW_IC_COMP_TYPE		0xfc
 #define DW_IC_COMP_TYPE_VALUE	0x44570140
@@ -248,6 +249,27 @@ static u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
 	return ((ic_clk * (tLOW + tf) + 5000) / 10000) - 1 + offset;
 }
 
+static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
+{
+	int timeout = 100;
+
+	do {
+		dw_writel(dev, enable, DW_IC_ENABLE);
+		if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == enable)
+			return;
+
+		/*
+		 * Wait 10 times the signaling period of the highest I2C
+		 * transfer supported by the driver (for 400KHz this is
+		 * 25us) as described in the DesignWare I2C databook.
+		 */
+		usleep_range(25, 250);
+	} while (timeout--);
+
+	dev_warn(dev->dev, "timeout in %sabling adapter\n",
+		 enable ? "en" : "dis");
+}
+
 /**
  * i2c_dw_init() - initialize the designware i2c master hardware
  * @dev: device private data
@@ -278,7 +300,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
 	}
 
 	/* Disable the adapter */
-	dw_writel(dev, 0, DW_IC_ENABLE);
+	__i2c_dw_enable(dev, false);
 
 	/* set standard and fast speed deviders for high/low periods */
 
@@ -345,7 +367,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 	u32 ic_con;
 
 	/* Disable the adapter */
-	dw_writel(dev, 0, DW_IC_ENABLE);
+	__i2c_dw_enable(dev, false);
 
 	/* set the slave (target) address */
 	dw_writel(dev, msgs[dev->msg_write_idx].addr, DW_IC_TAR);
@@ -359,7 +381,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 	dw_writel(dev, ic_con, DW_IC_CON);
 
 	/* Enable the adapter */
-	dw_writel(dev, 1, DW_IC_ENABLE);
+	__i2c_dw_enable(dev, true);
 
 	/* Enable interrupts */
 	dw_writel(dev, DW_IC_INTR_DEFAULT_MASK, DW_IC_INTR_MASK);
@@ -565,7 +587,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	/* no error */
 	if (likely(!dev->cmd_err)) {
 		/* Disable the adapter */
-		dw_writel(dev, 0, DW_IC_ENABLE);
+		__i2c_dw_enable(dev, false);
 		ret = num;
 		goto done;
 	}
@@ -700,7 +722,7 @@ EXPORT_SYMBOL_GPL(i2c_dw_isr);
 void i2c_dw_enable(struct dw_i2c_dev *dev)
 {
        /* Enable the adapter */
-	dw_writel(dev, 1, DW_IC_ENABLE);
+	__i2c_dw_enable(dev, true);
 }
 EXPORT_SYMBOL_GPL(i2c_dw_enable);
 
@@ -713,7 +735,7 @@ EXPORT_SYMBOL_GPL(i2c_dw_is_enabled);
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
 	/* Disable controller */
-	dw_writel(dev, 0, DW_IC_ENABLE);
+	__i2c_dw_enable(dev, false);
 
 	/* Disable all interupts */
 	dw_writel(dev, 0, DW_IC_INTR_MASK);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 6/7] i2c-designware: use usleep_range() in the busy-loop
@ 2013-04-10 10:36   ` Mika Westerberg
  0 siblings, 0 replies; 13+ messages in thread
From: Mika Westerberg @ 2013-04-10 10:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, Wolfram Sang, ben-linux, Jean Delvare,
	Andy Shevchenko, Christian Ruppert, Mika Westerberg

This is not an atomic context so there is no need to use mdelay() but
instead use usleep_range().

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 5cc4ebf..7c10c5b 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -355,7 +355,7 @@ static int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
 			return -ETIMEDOUT;
 		}
 		timeout--;
-		mdelay(1);
+		usleep_range(1000, 1100);
 	}
 
 	return 0;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 6/7] i2c-designware: use usleep_range() in the busy-loop
@ 2013-04-10 10:36   ` Mika Westerberg
  0 siblings, 0 replies; 13+ messages in thread
From: Mika Westerberg @ 2013-04-10 10:36 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang,
	ben-linux-elnMNo+KYs3YtjvyW6yDsg, Jean Delvare, Andy Shevchenko,
	Christian Ruppert, Mika Westerberg

This is not an atomic context so there is no need to use mdelay() but
instead use usleep_range().

Signed-off-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 5cc4ebf..7c10c5b 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -355,7 +355,7 @@ static int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
 			return -ETIMEDOUT;
 		}
 		timeout--;
-		mdelay(1);
+		usleep_range(1000, 1100);
 	}
 
 	return 0;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 7/7] i2c-designware: switch to use runtime PM autosuspend
  2013-04-10 10:36 ` Mika Westerberg
                   ` (5 preceding siblings ...)
  (?)
@ 2013-04-10 10:36 ` Mika Westerberg
       [not found]   ` <CAM=Q2cvp3U=-SDdWwjZQScSgesUD4Oq4_Om=sE7JNPko-HtVoQ@mail.gmail.com>
  -1 siblings, 1 reply; 13+ messages in thread
From: Mika Westerberg @ 2013-04-10 10:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-i2c, Wolfram Sang, ben-linux, Jean Delvare,
	Andy Shevchenko, Christian Ruppert, Mika Westerberg

Using autosuspend helps to reduce the resume latency in situations where
another I2C message is going to be started soon. For example with HID over
I2C touch panels we get several messages in a short period of time while
the touch panel is in use.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-core.c    |    3 ++-
 drivers/i2c/busses/i2c-designware-pcidrv.c  |    3 ++-
 drivers/i2c/busses/i2c-designware-platdrv.c |    3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 7c10c5b..21fbb34 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -600,7 +600,8 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	ret = -EIO;
 
 done:
-	pm_runtime_put(dev->dev);
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 	mutex_unlock(&dev->lock);
 
 	return ret;
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index aacb64e..c8797e2 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -291,7 +291,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	/* Increase reference counter */
 	get_device(&pdev->dev);
 
-	pm_runtime_put_noidle(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
+	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_allow(&pdev->dev);
 
 	return 0;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 0735ccf..b47c586 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -172,9 +172,10 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	/* Increase reference counter */
 	get_device(&pdev->dev);
 
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
+	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
-	pm_runtime_put(&pdev->dev);
 
 	return 0;
 }
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 7/7] i2c-designware: switch to use runtime PM autosuspend
       [not found]   ` <CAM=Q2cvp3U=-SDdWwjZQScSgesUD4Oq4_Om=sE7JNPko-HtVoQ@mail.gmail.com>
@ 2013-04-10 11:56     ` Mika Westerberg
  0 siblings, 0 replies; 13+ messages in thread
From: Mika Westerberg @ 2013-04-10 11:56 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: Linux Kernel Mailing List, Linux-I2C, Wolfram Sang, Ben Dooks,
	Jean Delvare, Andy Shevchenko, Christian Ruppert

On Wed, Apr 10, 2013 at 05:13:57PM +0530, Shubhrajyoti Datta wrote:
> Hi Mika,
> 
> 
> 
> On Wed, Apr 10, 2013 at 4:06 PM, Mika Westerberg <
> mika.westerberg@linux.intel.com> wrote:
> 
> > Using autosuspend helps to reduce the resume latency in situations where
> > another I2C message is going to be started soon. For example with HID over
> > I2C touch panels we get several messages in a short period of time while
> > the touch panel is in use.
> >
> Also the time to autosuspend could  be a macro just in case someone wants
> to tweak it.
> 
> Just a suggestion.

Well, you can always do that via sysfs /sys/.../power/autosuspend_delay_ms
(or whatever the name of the file is).

> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/i2c/busses/i2c-designware-core.c    |    3 ++-
> >  drivers/i2c/busses/i2c-designware-pcidrv.c  |    3 ++-
> >  drivers/i2c/busses/i2c-designware-platdrv.c |    3 ++-
> >  3 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-designware-core.c
> > b/drivers/i2c/busses/i2c-designware-core.c
> > index 7c10c5b..21fbb34 100644
> > --- a/drivers/i2c/busses/i2c-designware-core.c
> > +++ b/drivers/i2c/busses/i2c-designware-core.c
> > @@ -600,7 +600,8 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg
> > msgs[], int num)
> >         ret = -EIO;
> >
> >  done:
> > -       pm_runtime_put(dev->dev);
> > +       pm_runtime_mark_last_busy(dev->dev);
> > +       pm_runtime_put_autosuspend(dev->dev);
> >         mutex_unlock(&dev->lock);
> >
> >         return ret;
> > diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
> > b/drivers/i2c/busses/i2c-designware-pcidrv.c
> > index aacb64e..c8797e2 100644
> > --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> > @@ -291,7 +291,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
> >         /* Increase reference counter */
> >         get_device(&pdev->dev);
> >
> > -       pm_runtime_put_noidle(&pdev->dev);
> > +       pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
> > +       pm_runtime_use_autosuspend(&pdev->dev);
> >         pm_runtime_allow(&pdev->dev);
> >
> 
> Could you explain this?

Hmm, I'm not sure what you mean. In the above we enable autosuspend and set
the delay. Am I missing something?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/7] i2c-designware: move to managed functions (devm_*)
@ 2013-04-15 16:20   ` Wolfram Sang
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2013-04-15 16:20 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: linux-kernel, linux-i2c, ben-linux, Jean Delvare,
	Andy Shevchenko, Christian Ruppert

On Wed, Apr 10, 2013 at 01:36:36PM +0300, Mika Westerberg wrote:
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> This makes the error handling much more simpler than open-coding everything
> and in addition makes the probe function smaller and tidier.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Whole series applied to for-next, thanks!


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/7] i2c-designware: move to managed functions (devm_*)
@ 2013-04-15 16:20   ` Wolfram Sang
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2013-04-15 16:20 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	ben-linux-elnMNo+KYs3YtjvyW6yDsg, Jean Delvare, Andy Shevchenko,
	Christian Ruppert

On Wed, Apr 10, 2013 at 01:36:36PM +0300, Mika Westerberg wrote:
> From: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> This makes the error handling much more simpler than open-coding everything
> and in addition makes the probe function smaller and tidier.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Whole series applied to for-next, thanks!

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-04-15 16:20 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-10 10:36 [PATCH v2 1/7] i2c-designware: move to managed functions (devm_*) Mika Westerberg
2013-04-10 10:36 ` Mika Westerberg
2013-04-10 10:36 ` [PATCH v2 2/7] i2c-designware-pci: use dev_err() instead of printk() Mika Westerberg
2013-04-10 10:36   ` Mika Westerberg
2013-04-10 10:36 ` [PATCH v2 3/7] i2c-designware-pci: use managed functions pcim_* and devm_* Mika Westerberg
2013-04-10 10:36 ` [PATCH v2 4/7] i2c-designware: use dynamic adapter numbering on Lynxpoint Mika Westerberg
2013-04-10 10:36 ` [PATCH v2 5/7] i2c-designware: enable/disable the controller properly Mika Westerberg
2013-04-10 10:36 ` [PATCH v2 6/7] i2c-designware: use usleep_range() in the busy-loop Mika Westerberg
2013-04-10 10:36   ` Mika Westerberg
2013-04-10 10:36 ` [PATCH v2 7/7] i2c-designware: switch to use runtime PM autosuspend Mika Westerberg
     [not found]   ` <CAM=Q2cvp3U=-SDdWwjZQScSgesUD4Oq4_Om=sE7JNPko-HtVoQ@mail.gmail.com>
2013-04-10 11:56     ` Mika Westerberg
2013-04-15 16:20 ` [PATCH v2 1/7] i2c-designware: move to managed functions (devm_*) Wolfram Sang
2013-04-15 16:20   ` Wolfram Sang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.