| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20180626170125.GB5241@Mani-XPS-13-9360>
Date: Tue, 26 Jun 2018 22:31:25 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: Wolfram Sang <wsa@...-dreams.de>, Rob Herring <robh+dt@...nel.org>,
Andreas Färber <afaerber@...e.de>,
Linus Walleij <linus.walleij@...aro.org>,
linux-i2c <linux-i2c@...r.kernel.org>,
刘炜 <liuwei@...ions-semi.com>,
mp-cs@...ions-semi.com, 96boards@...obotics.com,
devicetree <devicetree@...r.kernel.org>,
Daniel Thompson <daniel.thompson@...aro.org>,
amit.kucheria@...aro.org,
linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
hzhang@...obotics.com, bdong@...obotics.com,
Mani Sadhasivam <manivannanece23@...il.com>,
Thomas Liau <thomas.liau@...ions-semi.com>,
jeff.chen@...ions-semi.com
Subject: Re: [PATCH 5/5] i2c: Add Actions Semi OWL family S900 I2C driver
Hi Andy,
Thanks for the review!
On Mon, Jun 25, 2018 at 12:38:50PM +0300, Andy Shevchenko wrote:
> On Sat, Jun 23, 2018 at 7:11 PM, Manivannan Sadhasivam
> <manivannan.sadhasivam@...aro.org> wrote:
> > Add Actions Semi OWL family S900 I2C driver.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > ---
> > drivers/i2c/busses/Kconfig | 7 +
> > drivers/i2c/busses/Makefile | 1 +
> > drivers/i2c/busses/i2c-owl.c | 459 +++++++++++++++++++++++++++++++++++
> > 3 files changed, 467 insertions(+)
> > create mode 100644 drivers/i2c/busses/i2c-owl.c
> >
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index 4f8df2ec87b1..2062da17e33b 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -762,6 +762,13 @@ config I2C_OMAP
> > Like OMAP1510/1610/1710/5912 and OMAP242x.
> > For details see http://www.ti.com/omap.
> >
> > +config I2C_OWL
> > + tristate "OWL I2C Controller"
> > + depends on ARCH_ACTIONS || COMPILE_TEST
> > + help
> > + Say Y here if you want to use the I2C bus controller on
> > + the Actions Semi OWL SoCs.
> > +
> > config I2C_PASEMI
> > tristate "PA Semi SMBus interface"
> > depends on PPC_PASEMI && PCI
> > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> > index 5a869144a0c5..b71618f77880 100644
> > --- a/drivers/i2c/busses/Makefile
> > +++ b/drivers/i2c/busses/Makefile
> > @@ -76,6 +76,7 @@ obj-$(CONFIG_I2C_MXS) += i2c-mxs.o
> > obj-$(CONFIG_I2C_NOMADIK) += i2c-nomadik.o
> > obj-$(CONFIG_I2C_OCORES) += i2c-ocores.o
> > obj-$(CONFIG_I2C_OMAP) += i2c-omap.o
> > +obj-$(CONFIG_I2C_OWL) += i2c-owl.o
> > obj-$(CONFIG_I2C_PASEMI) += i2c-pasemi.o
> > obj-$(CONFIG_I2C_PCA_PLATFORM) += i2c-pca-platform.o
> > obj-$(CONFIG_I2C_PMCMSP) += i2c-pmcmsp.o
> > diff --git a/drivers/i2c/busses/i2c-owl.c b/drivers/i2c/busses/i2c-owl.c
> > new file mode 100644
> > index 000000000000..53100ddfb3cc
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-owl.c
> > @@ -0,0 +1,459 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * OWL SoC's I2C driver
> > + *
> > + * Copyright (c) 2014 Actions Semi Inc.
> > + * Author: David Liu <liuwei@...ions-semi.com>
> > + *
> > + * Copyright (c) 2018 Linaro Ltd.
> > + * Author: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/i2c.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/io.h>
> > +
> > +/* I2C registers */
> > +#define OWL_I2C_REG_CTL (0x0000)
> > +#define OWL_I2C_REG_CLKDIV (0x0004)
> > +#define OWL_I2C_REG_STAT (0x0008)
> > +#define OWL_I2C_REG_ADDR (0x000C)
> > +#define OWL_I2C_REG_TXDAT (0x0010)
> > +#define OWL_I2C_REG_RXDAT (0x0014)
> > +#define OWL_I2C_REG_CMD (0x0018)
> > +#define OWL_I2C_REG_FIFOCTL (0x001C)
> > +#define OWL_I2C_REG_FIFOSTAT (0x0020)
> > +#define OWL_I2C_REG_DATCNT (0x0024)
> > +#define OWL_I2C_REG_RCNT (0x0028)
>
> I don't understand why these have parents?
>
I'm not sure what you mean here! Can you please elaborate?
> > +
> > +/* I2Cx_CTL Bit Mask */
> > +#define OWL_I2C_CTL_RB BIT(1)
> > +#define OWL_I2C_CTL_GBCC(x) (((x) & 0x3) << 2)
> > +#define OWL_I2C_CTL_GBCC_NONE OWL_I2C_CTL_GBCC(0)
> > +#define OWL_I2C_CTL_GBCC_START OWL_I2C_CTL_GBCC(1)
> > +#define OWL_I2C_CTL_GBCC_STOP OWL_I2C_CTL_GBCC(2)
> > +#define OWL_I2C_CTL_GBCC_RSTART OWL_I2C_CTL_GBCC(3)
> > +#define OWL_I2C_CTL_IRQE BIT(5)
> > +#define OWL_I2C_CTL_EN BIT(7)
> > +#define OWL_I2C_CTL_AE BIT(8)
> > +#define OWL_I2C_CTL_SHSM BIT(10)
> > +
> > +#define OWL_I2C_DIV_FACTOR(x) ((x) & 0xff)
> > +
> > +/* I2Cx_STAT Bit Mask */
> > +#define OWL_I2C_STAT_RACK BIT(0)
> > +#define OWL_I2C_STAT_BEB BIT(1)
> > +#define OWL_I2C_STAT_IRQP BIT(2)
> > +#define OWL_I2C_STAT_LAB BIT(3)
> > +#define OWL_I2C_STAT_STPD BIT(4)
> > +#define OWL_I2C_STAT_STAD BIT(5)
> > +#define OWL_I2C_STAT_BBB BIT(6)
> > +#define OWL_I2C_STAT_TCB BIT(7)
> > +#define OWL_I2C_STAT_LBST BIT(8)
> > +#define OWL_I2C_STAT_SAMB BIT(9)
> > +#define OWL_I2C_STAT_SRGC BIT(10)
> > +
> > +/* I2Cx_CMD Bit Mask */
> > +#define OWL_I2C_CMD_SBE BIT(0)
> > +#define OWL_I2C_CMD_RBE BIT(4)
> > +#define OWL_I2C_CMD_DE BIT(8)
> > +#define OWL_I2C_CMD_NS BIT(9)
> > +#define OWL_I2C_CMD_SE BIT(10)
> > +#define OWL_I2C_CMD_MSS BIT(11)
> > +#define OWL_I2C_CMD_WRS BIT(12)
> > +#define OWL_I2C_CMD_SECL BIT(15)
> > +
> > +#define OWL_I2C_CMD_AS(x) (((x) & 0x7) << 1)
> > +#define OWL_I2C_CMD_SAS(x) (((x) & 0x7) << 5)
> > +
> > +/* I2Cx_FIFOCTL Bit Mask */
> > +#define OWL_I2C_FIFOCTL_NIB BIT(0)
> > +#define OWL_I2C_FIFOCTL_RFR BIT(1)
> > +#define OWL_I2C_FIFOCTL_TFR BIT(2)
> > +
> > +/* I2Cc_FIFOSTAT Bit Mask */
> > +#define OWL_I2C_FIFOSTAT_RNB BIT(1)
> > +#define OWL_I2C_FIFOSTAT_RFE BIT(2)
> > +#define OWL_I2C_FIFOSTAT_TFF BIT(5)
> > +#define OWL_I2C_FIFOSTAT_TFD GENMASK(23, 16)
> > +#define OWL_I2C_FIFOSTAT_RFD GENMASK(15, 8)
> > +
>
> > +/* I2C bus timeout */
> > +#define OWL_I2C_TIMEOUT (msecs_to_jiffies(4 * 1000))
>
> Ditto.
>
Same as above
> > +
> > +#define OWL_I2C_DEFAULT_SPEED 100000
> > +#define OWL_I2C_MAX_SPEED 400000
> > +
> > +struct owl_i2c_dev {
> > + struct i2c_adapter adap;
> > + struct i2c_msg *msg;
> > + struct completion msg_complete;
> > + struct clk *clk;
> > + void __iomem *base;
> > + unsigned long clk_rate;
> > + u32 bus_freq;
> > + u32 msg_ptr;
> > +};
> > +
> > +static void owl_i2c_update_reg(void __iomem *base, unsigned int val, bool state)
> > +{
> > + unsigned int regval;
> > +
> > + regval = readl(base);
> > +
> > + if (state)
> > + regval |= val;
> > + else
> > + regval &= ~val;
> > +
> > + writel(regval, base);
> > +}
> > +
> > +static void owl_i2c_reset(struct owl_i2c_dev *i2c_dev)
> > +{
> > + unsigned int val;
> > +
> > + owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
> > + OWL_I2C_CTL_EN, false);
> > + mdelay(1);
> > + owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
> > + OWL_I2C_CTL_EN, true);
> > +
> > + /* Reset FIFO */
> > + owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_FIFOCTL,
> > + OWL_I2C_FIFOCTL_RFR | OWL_I2C_FIFOCTL_TFR,
> > + true);
> > +
> > + /* Wait until FIFO reset complete */
> > + do {
> > + val = readl(i2c_dev->base + OWL_I2C_REG_FIFOCTL);
> > + if (!(val & (OWL_I2C_FIFOCTL_RFR | OWL_I2C_FIFOCTL_TFR)))
> > + break;
> > + } while (1);
>
> No way. Get rid of infinite loop.
>
Okay. Can I change it to max of 50 retries with 1ms delay?
> > +
> > + /* Clear status registers */
> > + writel(0, i2c_dev->base + OWL_I2C_REG_STAT);
> > +}
> > +
> > +static void owl_i2c_set_freq(struct owl_i2c_dev *i2c_dev)
> > +{
> > + unsigned int val;
> > +
> > + val = (i2c_dev->clk_rate + i2c_dev->bus_freq * 16 - 1) /
> > + (i2c_dev->bus_freq * 16);
> > +
> > + /* Set clock divider factor */
> > + writel(OWL_I2C_DIV_FACTOR(val), i2c_dev->base + OWL_I2C_REG_CLKDIV);
> > +}
> > +
> > +static void owl_i2c_hw_init(struct owl_i2c_dev *i2c_dev)
> > +{
> > + /* Reset I2C controller */
> > + owl_i2c_reset(i2c_dev);
> > +
> > + /* Set bus frequency */
> > + owl_i2c_set_freq(i2c_dev);
> > +}
> > +
> > +static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
> > +{
> > + struct owl_i2c_dev *i2c_dev = _dev;
> > + struct i2c_msg *msg = i2c_dev->msg;
> > + unsigned int stat, fifostat;
> > +
> > + fifostat = readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT);
> > + if (fifostat & OWL_I2C_FIFOSTAT_RNB) {
>
> > + dev_warn(&i2c_dev->adap.dev, "received NACK from device");
>
> And if it happens hundreds times in a row?
>
Should I change it to dev_dbg?
> > + owl_i2c_reset(i2c_dev);
> > + goto stop;
> > + }
> > +
> > + stat = readl(i2c_dev->base + OWL_I2C_REG_STAT);
> > + if (stat & OWL_I2C_STAT_BEB) {
>
> > + dev_warn(&i2c_dev->adap.dev, "bus error");
>
> Ditto.
>
Same as above?
> > + owl_i2c_reset(i2c_dev);
> > + goto stop;
> > + }
> > +
> > + /* Handle FIFO read */
> > + if (msg->flags & I2C_M_RD) {
> > + while ((readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT) &
> > + OWL_I2C_FIFOSTAT_RFE) &&
> > + (i2c_dev->msg_ptr < msg->len)) {
> > + msg->buf[i2c_dev->msg_ptr++] = readl(i2c_dev->base +
> > + OWL_I2C_REG_RXDAT);
> > + }
> > + } else {
> > + /* Handle the remaining bytes which were not sent */
> > + while (!(readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT) &
> > + OWL_I2C_FIFOSTAT_TFF) &&
> > + i2c_dev->msg_ptr < msg->len) {
> > + writel(msg->buf[i2c_dev->msg_ptr++], i2c_dev->base +
> > + OWL_I2C_REG_TXDAT);
> > + }
> > + }
> > +
> > +stop:
> > + /* Clear pending interrupts */
> > + owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_STAT,
> > + OWL_I2C_STAT_IRQP, true);
> > +
> > + complete_all(&i2c_dev->msg_complete);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static u32 owl_i2c_func(struct i2c_adapter *adap)
> > +{
> > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> > +}
> > +
> > +static int owl_i2c_check_bus_busy(struct i2c_adapter *adap)
> > +{
> > + struct owl_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
> > + unsigned long timeout;
> > + unsigned int val;
> > +
> > + timeout = jiffies + OWL_I2C_TIMEOUT;
>
> > + while (1) {
>
> Red flag!
>
> Use
>
> do {
> ...
> } while (time_after(...));
>
> instead.
>
Okay, will change it.
> > + val = readl(i2c_dev->base + OWL_I2C_REG_STAT);
> > +
> > + /* Check for Arbitration lost */
> > + if (val & OWL_I2C_STAT_LAB) {
> > + val &= ~OWL_I2C_STAT_LAB;
> > + writel(val, i2c_dev->base + OWL_I2C_REG_STAT);
> > + return -EAGAIN;
> > + }
> > +
> > + /* Check for Bus busy */
> > + if (!(val & OWL_I2C_STAT_BBB))
> > + break;
> > +
> > + if (time_after(jiffies, timeout)) {
> > + dev_err(&adap->dev, "Bus busy timeout");
> > + return -ETIMEDOUT;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int owl_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> > + int num)
> > +{
> > + struct owl_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
> > + struct i2c_msg *msg;
> > + unsigned long time_left;
> > + unsigned int i2c_cmd;
> > + unsigned int addr;
> > + int ret = 0, idx;
> > +
> > + owl_i2c_hw_init(i2c_dev);
> > +
> > + ret = owl_i2c_check_bus_busy(adap);
> > + if (ret)
> > + return ret;
> > +
> > + reinit_completion(&i2c_dev->msg_complete);
> > +
> > + /* Enable I2C controller interrupt */
> > + owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
> > + OWL_I2C_CTL_IRQE, true);
> > +
> > + /*
> > + * Select: FIFO enable, Master mode, Stop enable, Data count enable,
> > + * Send start bit
> > + */
> > + i2c_cmd = (OWL_I2C_CMD_SECL | OWL_I2C_CMD_MSS | OWL_I2C_CMD_SE
> > + | OWL_I2C_CMD_NS | OWL_I2C_CMD_DE | OWL_I2C_CMD_SBE);
> > +
> > + addr = (msgs[0].addr & 0x7f) << 1;
> > +
> > + /* Handle repeated start condition */
> > + if (num > 1) {
> > + /* Set internal address length and enable repeated start */
> > + i2c_cmd |= (OWL_I2C_CMD_AS(msgs[0].len + 1)
> > + | OWL_I2C_CMD_SAS(1) | OWL_I2C_CMD_RBE);
> > +
> > + /* Write slave address */
> > + writel(addr, i2c_dev->base + OWL_I2C_REG_TXDAT);
> > +
> > + /* Write internal register address */
> > + for (idx = 0; idx < msgs[0].len; idx++)
> > + writel(msgs[0].buf[idx], i2c_dev->base +
> > + OWL_I2C_REG_TXDAT);
> > +
> > + msg = &msgs[1];
> > + } else {
> > + /* Set address length */
> > + i2c_cmd |= OWL_I2C_CMD_AS(1);
> > + msg = &msgs[0];
> > + }
> > +
> > + i2c_dev->msg = msg;
> > + i2c_dev->msg_ptr = 0;
> > +
> > + /* Set data count for the message */
> > + writel(msg->len, i2c_dev->base + OWL_I2C_REG_DATCNT);
> > +
>
> > + if (msg->flags & I2C_M_RD) {
> > + writel((addr | 1), i2c_dev->base + OWL_I2C_REG_TXDAT);
> > + } else {
> > + writel(addr, i2c_dev->base + OWL_I2C_REG_TXDAT);
>
> Don't we have a helper to construct 8-bit address?
>
Okay, will use i2c_8bit_addr_from_msg for constructing the address
> > +
> > + /* Write data to FIFO */
> > + for (idx = 0; idx < msg->len; idx++) {
> > + /* Check for FIFO full */
> > + if (readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT)
> > + & OWL_I2C_FIFOSTAT_TFF)
> > + break;
> > +
> > + writel(msg->buf[idx],
> > + i2c_dev->base + OWL_I2C_REG_TXDAT);
> > + }
> > +
> > + i2c_dev->msg_ptr = idx;
> > + }
> > +
> > + /* Ingore the NACK if needed */
> > + if (msg->flags & I2C_M_IGNORE_NAK)
> > + owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_FIFOCTL,
> > + OWL_I2C_FIFOCTL_NIB, true);
> > + else
> > + owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_FIFOCTL,
> > + OWL_I2C_FIFOCTL_NIB, false);
> > +
> > + /* Start the transfer */
> > + writel(i2c_cmd, i2c_dev->base + OWL_I2C_REG_CMD);
> > +
> > + time_left = wait_for_completion_timeout(&i2c_dev->msg_complete,
> > + adap->timeout);
> > + if (time_left == 0) {
> > + dev_err(&adap->dev, "Transaction timed out");
> > + /* Send stop condition and release the bus */
> > + owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
> > + OWL_I2C_CTL_GBCC_STOP | OWL_I2C_CTL_RB, true);
> > + ret = -ETIMEDOUT;
> > + }
> > +
> > + /* Disable I2C controller */
> > + owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
> > + OWL_I2C_CTL_EN, false);
> > +
> > + return i2c_dev->msg_ptr;
> > +}
> > +
> > +static const struct i2c_algorithm owl_i2c_algorithm = {
> > + .master_xfer = owl_i2c_master_xfer,
> > + .functionality = owl_i2c_func
> > +};
> > +
> > +static const struct i2c_adapter_quirks owl_i2c_quirks = {
> > + .flags = I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST,
> > + .max_read_len = 240,
> > + .max_write_len = 240,
> > + .max_comb_1st_msg_len = 6,
> > + .max_comb_2nd_msg_len = 240
> > +};
> > +
> > +static int owl_i2c_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct owl_i2c_dev *i2c_dev;
> > + struct resource *res;
> > + int ret, irq;
> > +
> > + i2c_dev = devm_kzalloc(dev, sizeof(*i2c_dev), GFP_KERNEL);
> > + if (!i2c_dev)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + i2c_dev->base = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(i2c_dev->base))
> > + return PTR_ERR(i2c_dev->base);
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0) {
> > + dev_err(dev, "failed to get IRQ number\n");
> > + return irq;
> > + }
> > +
> > + if (of_property_read_u32(dev->of_node, "clock-frequency",
> > + &i2c_dev->bus_freq))
> > + i2c_dev->bus_freq = OWL_I2C_DEFAULT_SPEED;
> > +
> > + /* We support only frequencies of 100k and 400k for now */
> > + if (i2c_dev->bus_freq != OWL_I2C_DEFAULT_SPEED &&
> > + i2c_dev->bus_freq > OWL_I2C_MAX_SPEED) {
> > + dev_err(dev, "invalid clock-frequency %d\n", i2c_dev->bus_freq);
> > + return -EINVAL;
> > + }
> > +
> > + i2c_dev->clk = devm_clk_get(dev, NULL);
> > + if (IS_ERR(i2c_dev->clk)) {
> > + dev_err(dev, "failed to get clock\n");
> > + return PTR_ERR(i2c_dev->clk);
> > + }
> > +
> > + ret = clk_prepare_enable(i2c_dev->clk);
> > + if (ret)
> > + return ret;
> > +
> > + i2c_dev->clk_rate = clk_get_rate(i2c_dev->clk);
> > + if (!i2c_dev->clk_rate) {
> > + dev_err(dev, "input clock rate should not be zero\n");
> > + ret = -EINVAL;
> > + goto disable_clk;
> > + }
> > +
> > + init_completion(&i2c_dev->msg_complete);
> > + i2c_dev->adap.owner = THIS_MODULE;
> > + i2c_dev->adap.algo = &owl_i2c_algorithm;
> > + i2c_dev->adap.timeout = OWL_I2C_TIMEOUT;
> > + i2c_dev->adap.quirks = &owl_i2c_quirks;
> > + i2c_dev->adap.dev.parent = dev;
> > + i2c_dev->adap.dev.of_node = dev->of_node;
> > + snprintf(i2c_dev->adap.name, sizeof(i2c_dev->adap.name),
> > + "%s", "OWL I2C adapter");
> > + i2c_set_adapdata(&i2c_dev->adap, i2c_dev);
> > +
> > + platform_set_drvdata(pdev, i2c_dev);
> > +
> > + ret = devm_request_irq(dev, irq, owl_i2c_interrupt, 0, pdev->name,
> > + i2c_dev);
> > + if (ret) {
> > + dev_err(dev, "failed to request irq %d\n", irq);
> > + goto disable_clk;
> > + }
> > +
>
> > + ret = i2c_add_adapter(&i2c_dev->adap);
> > +disable_clk:
> > + if (ret)
> > + clk_disable_unprepare(i2c_dev->clk);
> > +
> > + return ret;
>
>
> Can't we go with more usual patter, i.e.
>
> ret = ...;
> if (ret)
> goto err_handling;
>
> return 0;
>
> err_handling:
> ...
> return ret;
>
> ?
>
Ack.
Thanks,
Mani
> > +}
> > +
> > +static const struct of_device_id owl_i2c_of_match[] = {
> > + {.compatible = "actions,s900-i2c"},
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, owl_i2c_of_match);
> > +
> > +static struct platform_driver owl_i2c_driver = {
> > + .probe = owl_i2c_probe,
> > + .driver = {
> > + .name = "owl-i2c",
> > + .of_match_table = of_match_ptr(owl_i2c_of_match),
> > + },
> > +};
> > +module_platform_driver(owl_i2c_driver);
> > +
> > +MODULE_AUTHOR("David Liu <liuwei@...ions-semi.com>");
> > +MODULE_AUTHOR("Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>");
> > +MODULE_DESCRIPTION("Actions Semi OWL SoCs I2C driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.17.1
> >
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
Powered by blists - more mailing lists