On 1/15/2011 4:27 AM, dirk.brandewie@xxxxxxxxx wrote:
> diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> index 746b4bb..e8b354b 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -580,3 +583,31 @@ tx_aborted:
> }
> EXPORT_SYMBOL(i2c_dw_isr);
>
> +void i2c_dw_enable(struct dw_i2c_dev *dev)
> +{
> + /* Enable the adapter */
> + writel(1, dev->base + DW_IC_ENABLE);
> +}
> +EXPORT_SYMBOL(i2c_dw_enable);
Having enable/disable wrapper functions is good and promissing, as
that helps us to avoid direct references to the IC_ENABLE register
from -pci.c and -plat.c.
> +void i2c_dw_disable(struct dw_i2c_dev *i2c)
> +{
> + /* Disable controller */
> + writel(0, i2c->base + DW_IC_ENABLE);
> +
> + /* Disable all interupts */
> + writel(0x0000, i2c->base + DW_IC_INTR_MASK);
> +
> + /* Clear all interrupts */
> + readl(i2c->base + DW_IC_CLR_INTR);
> + readl(i2c->base + DW_IC_CLR_STOP_DET);
> + readl(i2c->base + DW_IC_CLR_START_DET);
> + readl(i2c->base + DW_IC_CLR_ACTIVITY);
> + readl(i2c->base + DW_IC_CLR_TX_ABRT);
> + readl(i2c->base + DW_IC_CLR_RX_OVER);
> + readl(i2c->base + DW_IC_CLR_RX_UNDER);
> + readl(i2c->base + DW_IC_CLR_TX_OVER);
> + readl(i2c->base + DW_IC_CLR_RX_DONE);
> + readl(i2c->base + DW_IC_CLR_GEN_CALL);
> +}
> +EXPORT_SYMBOL(i2c_dw_disable);
Note that when you read the IC_CLR_INTR register, all interrupt status
bits asserted at that time will be cleared, so no need to read
remaining the IC_CLR_STOP_DET..IC_CLER_GEN_CALL registers.
By the way, I have one patch regarding safely shutdown the controller.
Will send a patch to Dirk later.
> diff --git a/drivers/i2c/busses/i2c-designware-pci.c b/drivers/i2c/busses/i2c-designware-pci.c
> index 1d57761..bea467e 100644
> --- a/drivers/i2c/busses/i2c-designware-pci.c
> +++ b/drivers/i2c/busses/i2c-designware-pci.c
[...]
> +static int i2c_dw_pci_resume(struct pci_dev *pdev)
> +{
> + struct dw_i2c_dev *i2c = pci_get_drvdata(pdev);
> + int err;
> + u32 enabled;
> +
> + enabled = dw_readl(i2c, DW_IC_ENABLE);
> + if (enabled)
> + return 0;
Hmm, we still have a direct reference to IC_ENABLE here.
> @@ -219,27 +300,28 @@ const struct pci_device_id *id)
> adap = &dev->adapter;
> i2c_set_adapdata(adap, dev);
> adap->owner = THIS_MODULE;
> - adap->class = I2C_CLASS_HWMON;
> - strlcpy(adap->name, "Synopsys DesignWare I2C adapter",
> - sizeof(adap->name));
> + adap->class = 0;
> adap->algo = &i2c_dw_algo;
> adap->dev.parent = &pdev->dev;
> -
> adap->nr = controller->bus_num;
> + snprintf(adap->name, sizeof(adap->name), "i2c-dw-pci-%d",
> + adap->nr);
>
> - writel(0, dev->base + DW_IC_INTR_MASK); /* disable IRQ */
> r = request_irq(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;
> }
>
> + dw_readl(dev, DW_IC_CLR_INTR);
> + dw_writel(dev, 0, DW_IC_INTR_MASK); /* disable IRQ */
Replacing with i2c_dw_disable()?
> r = i2c_add_numbered_adapter(adap);
> if (r) {
> dev_err(&pdev->dev, "failure adding adapter\n");
> goto err_free_irq;
> }
>
> + pm_runtime_enable(&pdev->dev);
> return 0;
>
> err_free_irq:
--
Shinya Kuribayashi
Renesas Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
