* [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness
@ 2021-04-14 22:33 Chris Packham
2021-04-14 22:33 ` [PATCH v4 1/6] i2c: mpc: Interrupt driven transfer Chris Packham
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: Chris Packham @ 2021-04-14 22:33 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Andy Shevchenko, linux-i2c, linux-kernel, Chris Packham
I've tested on T2081 and P2041 based systems with a number of i2c and smbus
devices.
I've included some clean ups provided by Andy Shevchenko to make applying the
series easier.
Andy Shevchenko (4):
i2c: mpc: Use devm_clk_get_optional()
i2c: mpc: Remove CONFIG_PM_SLEEP ifdeffery
i2c: mpc: Use device_get_match_data() helper
i2c: mpc: Drop duplicate message from devm_platform_ioremap_resource()
Chris Packham (2):
i2c: mpc: Interrupt driven transfer
i2c: mpc: Update license and copyright
drivers/i2c/busses/i2c-mpc.c | 492 +++++++++++++++++++----------------
1 file changed, 262 insertions(+), 230 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v4 1/6] i2c: mpc: Interrupt driven transfer 2021-04-14 22:33 [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness Chris Packham @ 2021-04-14 22:33 ` Chris Packham 2021-04-14 22:33 ` [PATCH v4 2/6] i2c: mpc: Update license and copyright Chris Packham ` (5 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Chris Packham @ 2021-04-14 22:33 UTC (permalink / raw) To: Wolfram Sang; +Cc: Andy Shevchenko, linux-i2c, linux-kernel, Chris Packham The fsl-i2c controller will generate an interrupt after every byte transferred. Make use of this interrupt to drive a state machine which allows the next part of a transfer to happen as soon as the interrupt is received. This is particularly helpful with SMBUS devices like the LM81 which will timeout if we take too long between bytes in a transfer. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Notes: Changes in v4: - Split license/copyright change to separate patch - Remove MPC_I2C_ACTION_INVALID - make action_str const - Remove __func__ from dev_dbg output - Use Tx ACK/Rx ACK consistently in comments and messages - Use spin_lock() instead of spin_lock_irqsave() in hardirq context - Typo fix arbiritration -> arbitration - Make mpc_i2c_wait_for_completion return an error and use that to set i2c->rc - Minor style fixes in mpc_i2c_isr and mpc_i2c_wait_for_completion - I haven't deduplicated the read/write debug in mpc_i2c_do_action() as doing so seems to make things overly complex Changes in v3: - use WARN/WARN_ON instead of BUG/BUG_ON Changes in v2: - add static_assert for state debug strings - remove superfluous space drivers/i2c/busses/i2c-mpc.c | 426 ++++++++++++++++++++--------------- 1 file changed, 239 insertions(+), 187 deletions(-) diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c index 3c8bcdfff7e7..0a80fafbe6c6 100644 --- a/drivers/i2c/busses/i2c-mpc.c +++ b/drivers/i2c/busses/i2c-mpc.c @@ -58,11 +58,35 @@ #define CSR_MIF 0x02 #define CSR_RXAK 0x01 +enum mpc_i2c_action { + MPC_I2C_ACTION_START = 1, + MPC_I2C_ACTION_RESTART, + MPC_I2C_ACTION_READ_BEGIN, + MPC_I2C_ACTION_READ_BYTE, + MPC_I2C_ACTION_WRITE, + MPC_I2C_ACTION_STOP, + + __MPC_I2C_ACTION_CNT +}; + +static const char * const action_str[] = { + "invalid", + "start", + "restart", + "read begin", + "read", + "write", + "stop", +}; + +static_assert(ARRAY_SIZE(action_str) == __MPC_I2C_ACTION_CNT); + struct mpc_i2c { struct device *dev; void __iomem *base; u32 interrupt; - wait_queue_head_t queue; + wait_queue_head_t waitq; + spinlock_t lock; struct i2c_adapter adap; int irq; u32 real_clk; @@ -70,6 +94,16 @@ struct mpc_i2c { u8 fdr, dfsrr; #endif struct clk *clk_per; + u32 cntl_bits; + enum mpc_i2c_action action; + struct i2c_msg *msgs; + int num_msgs; + int curr_msg; + u32 byte_posn; + u32 block; + int rc; + int expect_rxack; + }; struct mpc_i2c_divider { @@ -86,19 +120,6 @@ static inline void writeccr(struct mpc_i2c *i2c, u32 x) writeb(x, i2c->base + MPC_I2C_CR); } -static irqreturn_t mpc_i2c_isr(int irq, void *dev_id) -{ - struct mpc_i2c *i2c = dev_id; - if (readb(i2c->base + MPC_I2C_SR) & CSR_MIF) { - /* Read again to allow register to stabilise */ - i2c->interrupt = readb(i2c->base + MPC_I2C_SR); - writeb(0, i2c->base + MPC_I2C_SR); - wake_up(&i2c->queue); - return IRQ_HANDLED; - } - return IRQ_NONE; -} - /* Sometimes 9th clock pulse isn't generated, and slave doesn't release * the bus, because it wants to send ACK. * Following sequence of enabling/disabling and sending start/stop generates @@ -121,45 +142,6 @@ static void mpc_i2c_fixup(struct mpc_i2c *i2c) } } -static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing) -{ - u32 cmd_err; - int result; - - result = wait_event_timeout(i2c->queue, - (i2c->interrupt & CSR_MIF), timeout); - - if (unlikely(!(i2c->interrupt & CSR_MIF))) { - dev_dbg(i2c->dev, "wait timeout\n"); - writeccr(i2c, 0); - result = -ETIMEDOUT; - } - - cmd_err = i2c->interrupt; - i2c->interrupt = 0; - - if (result < 0) - return result; - - if (!(cmd_err & CSR_MCF)) { - dev_dbg(i2c->dev, "unfinished\n"); - return -EIO; - } - - if (cmd_err & CSR_MAL) { - dev_dbg(i2c->dev, "MAL\n"); - return -EAGAIN; - } - - if (writing && (cmd_err & CSR_RXAK)) { - dev_dbg(i2c->dev, "No RXAK\n"); - /* generate stop */ - writeccr(i2c, CCR_MEN); - return -ENXIO; - } - return 0; -} - #if defined(CONFIG_PPC_MPC52xx) || defined(CONFIG_PPC_MPC512x) static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = { {20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23}, @@ -434,168 +416,209 @@ static void mpc_i2c_setup_8xxx(struct device_node *node, } #endif /* CONFIG_FSL_SOC */ -static void mpc_i2c_start(struct mpc_i2c *i2c) +static void mpc_i2c_finish(struct mpc_i2c *i2c, int rc) { - /* Clear arbitration */ - writeb(0, i2c->base + MPC_I2C_SR); - /* Start with MEN */ - writeccr(i2c, CCR_MEN); + i2c->rc = rc; + i2c->block = 0; + i2c->cntl_bits = CCR_MEN; + writeccr(i2c, i2c->cntl_bits); + wake_up(&i2c->waitq); } -static void mpc_i2c_stop(struct mpc_i2c *i2c) +static void mpc_i2c_do_action(struct mpc_i2c *i2c) { - writeccr(i2c, CCR_MEN); -} + struct i2c_msg *msg = &i2c->msgs[i2c->curr_msg]; + int dir = 0; + int recv_len = 0; + u8 byte; + + dev_dbg(i2c->dev, "action = %s\n", action_str[i2c->action]); + + i2c->cntl_bits &= ~(CCR_RSTA | CCR_MTX | CCR_TXAK); + + if (msg->flags & I2C_M_RD) + dir = 1; + if (msg->flags & I2C_M_RECV_LEN) + recv_len = 1; + + switch (i2c->action) { + case MPC_I2C_ACTION_RESTART: + i2c->cntl_bits |= CCR_RSTA; + fallthrough; + + case MPC_I2C_ACTION_START: + i2c->cntl_bits |= CCR_MSTA | CCR_MTX; + writeccr(i2c, i2c->cntl_bits); + writeb((msg->addr << 1) | dir, i2c->base + MPC_I2C_DR); + i2c->expect_rxack = 1; + i2c->action = dir ? MPC_I2C_ACTION_READ_BEGIN : MPC_I2C_ACTION_WRITE; + break; + + case MPC_I2C_ACTION_READ_BEGIN: + if (msg->len) { + if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN)) + i2c->cntl_bits |= CCR_TXAK; + + writeccr(i2c, i2c->cntl_bits); + /* Dummy read */ + readb(i2c->base + MPC_I2C_DR); + } + i2c->action = MPC_I2C_ACTION_READ_BYTE; + break; + + case MPC_I2C_ACTION_READ_BYTE: + if (i2c->byte_posn || !recv_len) { + /* Generate Tx ACK on next to last byte */ + if (i2c->byte_posn == msg->len - 2) + i2c->cntl_bits |= CCR_TXAK; + /* Do not generate stop on last byte */ + if (i2c->byte_posn == msg->len - 1) + i2c->cntl_bits |= CCR_MTX; -static int mpc_write(struct mpc_i2c *i2c, int target, - const u8 *data, int length, int restart) -{ - int i, result; - unsigned timeout = i2c->adap.timeout; - u32 flags = restart ? CCR_RSTA : 0; + writeccr(i2c, i2c->cntl_bits); + } - /* Start as master */ - writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags); - /* Write target byte */ - writeb((target << 1), i2c->base + MPC_I2C_DR); + byte = readb(i2c->base + MPC_I2C_DR); - result = i2c_wait(i2c, timeout, 1); - if (result < 0) - return result; + if (i2c->byte_posn == 0 && recv_len) { + if (byte == 0 || byte > I2C_SMBUS_BLOCK_MAX) { + mpc_i2c_finish(i2c, -EPROTO); + return; + } + msg->len += byte; + /* + * For block reads, generate Tx ACK here if data length + * is 1 byte (total length is 2 bytes). + */ + if (msg->len == 2) { + i2c->cntl_bits |= CCR_TXAK; + writeccr(i2c, i2c->cntl_bits); + } + } + + dev_dbg(i2c->dev, "%s %02x\n", action_str[i2c->action], byte); + msg->buf[i2c->byte_posn++] = byte; + break; - for (i = 0; i < length; i++) { - /* Write data byte */ - writeb(data[i], i2c->base + MPC_I2C_DR); + case MPC_I2C_ACTION_WRITE: + dev_dbg(i2c->dev, "%s %02x\n", action_str[i2c->action], + msg->buf[i2c->byte_posn]); + writeb(msg->buf[i2c->byte_posn++], i2c->base + MPC_I2C_DR); + i2c->expect_rxack = 1; + break; - result = i2c_wait(i2c, timeout, 1); - if (result < 0) - return result; + case MPC_I2C_ACTION_STOP: + mpc_i2c_finish(i2c, 0); + break; + + default: + WARN(1, "Unexpected action %d\n", i2c->action); + break; } - return 0; + if (msg->len == i2c->byte_posn) { + i2c->curr_msg++; + i2c->byte_posn = 0; + + if (i2c->curr_msg == i2c->num_msgs) { + i2c->action = MPC_I2C_ACTION_STOP; + /* + * We don't get another interrupt on read so + * finish the transfer now + */ + if (dir) + mpc_i2c_finish(i2c, 0); + } else { + i2c->action = MPC_I2C_ACTION_RESTART; + } + } } -static int mpc_read(struct mpc_i2c *i2c, int target, - u8 *data, int length, int restart, bool recv_len) +static void mpc_i2c_do_intr(struct mpc_i2c *i2c, u8 status) { - unsigned timeout = i2c->adap.timeout; - int i, result; - u32 flags = restart ? CCR_RSTA : 0; + spin_lock(&i2c->lock); - /* Switch to read - restart */ - writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags); - /* Write target address byte - this time with the read flag set */ - writeb((target << 1) | 1, i2c->base + MPC_I2C_DR); + if (!(status & CSR_MCF)) { + dev_dbg(i2c->dev, "unfinished\n"); + mpc_i2c_finish(i2c, -EIO); + goto out; + } - result = i2c_wait(i2c, timeout, 1); - if (result < 0) - return result; + if (status & CSR_MAL) { + dev_dbg(i2c->dev, "arbitration lost\n"); + mpc_i2c_finish(i2c, -EAGAIN); + goto out; + } - if (length) { - if (length == 1 && !recv_len) - writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_TXAK); - else - writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA); - /* Dummy read */ - readb(i2c->base + MPC_I2C_DR); + if (i2c->expect_rxack && (status & CSR_RXAK)) { + dev_dbg(i2c->dev, "no Rx ACK\n"); + mpc_i2c_finish(i2c, -ENXIO); + goto out; } + i2c->expect_rxack = 0; - for (i = 0; i < length; i++) { - u8 byte; - - result = i2c_wait(i2c, timeout, 0); - if (result < 0) - return result; - - /* - * For block reads, we have to know the total length (1st byte) - * before we can determine if we are done. - */ - if (i || !recv_len) { - /* Generate txack on next to last byte */ - if (i == length - 2) - writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA - | CCR_TXAK); - /* Do not generate stop on last byte */ - if (i == length - 1) - writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA - | CCR_MTX); - } + mpc_i2c_do_action(i2c); - byte = readb(i2c->base + MPC_I2C_DR); +out: + spin_unlock(&i2c->lock); +} - /* - * Adjust length if first received byte is length. - * The length is 1 length byte plus actually data length - */ - if (i == 0 && recv_len) { - if (byte == 0 || byte > I2C_SMBUS_BLOCK_MAX) - return -EPROTO; - length += byte; - /* - * For block reads, generate txack here if data length - * is 1 byte (total length is 2 bytes). - */ - if (length == 2) - writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA - | CCR_TXAK); - } - data[i] = byte; +static irqreturn_t mpc_i2c_isr(int irq, void *dev_id) +{ + struct mpc_i2c *i2c = dev_id; + u8 status; + + status = readb(i2c->base + MPC_I2C_SR); + if (status & CSR_MIF) { + writeb(0, i2c->base + MPC_I2C_SR); + mpc_i2c_do_intr(i2c, status); + return IRQ_HANDLED; } + return IRQ_NONE; +} - return length; +static int mpc_i2c_wait_for_completion(struct mpc_i2c *i2c) +{ + long time_left; + + time_left = wait_event_timeout(i2c->waitq, !i2c->block, i2c->adap.timeout); + if (!time_left) + return -ETIMEDOUT; + if (time_left < 0) + return time_left; + + return 0; } -static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) +static int mpc_i2c_execute_msg(struct mpc_i2c *i2c) { - struct i2c_msg *pmsg; - int i; - int ret = 0; - unsigned long orig_jiffies = jiffies; - struct mpc_i2c *i2c = i2c_get_adapdata(adap); + unsigned long orig_jiffies; + unsigned long flags; + int ret; - mpc_i2c_start(i2c); + spin_lock_irqsave(&i2c->lock, flags); - /* Allow bus up to 1s to become not busy */ - while (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) { - if (signal_pending(current)) { - dev_dbg(i2c->dev, "Interrupted\n"); - writeccr(i2c, 0); - return -EINTR; - } - if (time_after(jiffies, orig_jiffies + HZ)) { - u8 status = readb(i2c->base + MPC_I2C_SR); + i2c->curr_msg = 0; + i2c->rc = 0; + i2c->byte_posn = 0; + i2c->block = 1; + i2c->action = MPC_I2C_ACTION_START; - dev_dbg(i2c->dev, "timeout\n"); - if ((status & (CSR_MCF | CSR_MBB | CSR_RXAK)) != 0) { - writeb(status & ~CSR_MAL, - i2c->base + MPC_I2C_SR); - i2c_recover_bus(&i2c->adap); - } - return -EIO; - } - schedule(); - } + i2c->cntl_bits = CCR_MEN | CCR_MIEN; + writeb(0, i2c->base + MPC_I2C_SR); + writeccr(i2c, i2c->cntl_bits); + + mpc_i2c_do_action(i2c); + + spin_unlock_irqrestore(&i2c->lock, flags); + + ret = mpc_i2c_wait_for_completion(i2c); + if (ret) + i2c->rc = ret; + + if (i2c->rc == -EIO || i2c->rc == -EAGAIN || i2c->rc == -ETIMEDOUT) + i2c_recover_bus(&i2c->adap); - for (i = 0; ret >= 0 && i < num; i++) { - pmsg = &msgs[i]; - dev_dbg(i2c->dev, - "Doing %s %d bytes to 0x%02x - %d of %d messages\n", - pmsg->flags & I2C_M_RD ? "read" : "write", - pmsg->len, pmsg->addr, i + 1, num); - if (pmsg->flags & I2C_M_RD) { - bool recv_len = pmsg->flags & I2C_M_RECV_LEN; - - ret = mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i, - recv_len); - if (recv_len && ret > 0) - pmsg->len = ret; - } else { - ret = - mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len, i); - } - } - mpc_i2c_stop(i2c); /* Initiate STOP */ orig_jiffies = jiffies; /* Wait until STOP is seen, allow up to 1 s */ while (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) { @@ -612,7 +635,35 @@ static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) } cond_resched(); } - return (ret < 0) ? ret : num; + + return i2c->rc; +} + +static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) +{ + int rc, ret = num; + struct mpc_i2c *i2c = i2c_get_adapdata(adap); + int i; + + dev_dbg(i2c->dev, "num = %d\n", num); + for (i = 0; i < num; i++) + dev_dbg(i2c->dev, " addr = %02x, flags = %02x, len = %d, %*ph\n", + msgs[i].addr, msgs[i].flags, msgs[i].len, + msgs[i].flags & I2C_M_RD ? 0 : msgs[i].len, + msgs[i].buf); + + WARN_ON(i2c->msgs != NULL); + i2c->msgs = msgs; + i2c->num_msgs = num; + + rc = mpc_i2c_execute_msg(i2c); + if (rc < 0) + ret = rc; + + i2c->num_msgs = 0; + i2c->msgs = NULL; + + return ret; } static u32 mpc_functionality(struct i2c_adapter *adap) @@ -667,7 +718,8 @@ static int fsl_i2c_probe(struct platform_device *op) i2c->dev = &op->dev; /* for debug and error output */ - init_waitqueue_head(&i2c->queue); + init_waitqueue_head(&i2c->waitq); + spin_lock_init(&i2c->lock); i2c->base = devm_platform_ioremap_resource(op, 0); if (IS_ERR(i2c->base)) { -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 2/6] i2c: mpc: Update license and copyright 2021-04-14 22:33 [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness Chris Packham 2021-04-14 22:33 ` [PATCH v4 1/6] i2c: mpc: Interrupt driven transfer Chris Packham @ 2021-04-14 22:33 ` Chris Packham 2021-04-14 22:33 ` [PATCH v4 3/6] i2c: mpc: Use devm_clk_get_optional() Chris Packham ` (4 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Chris Packham @ 2021-04-14 22:33 UTC (permalink / raw) To: Wolfram Sang; +Cc: Andy Shevchenko, linux-i2c, linux-kernel, Chris Packham Use SPDX-License-Identifier and add copyright for Allied Telesis because of the reasonably large rewrite in the preceding patch. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Notes: Changes in v4: - New, split out from "i2c: mpc: Interrupt driven transfer" drivers/i2c/busses/i2c-mpc.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c index 0a80fafbe6c6..37244138875a 100644 --- a/drivers/i2c/busses/i2c-mpc.c +++ b/drivers/i2c/busses/i2c-mpc.c @@ -1,16 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0 /* - * (C) Copyright 2003-2004 - * Humboldt Solutions Ltd, adrian@humboldt.co.uk. - * This is a combined i2c adapter and algorithm driver for the * MPC107/Tsi107 PowerPC northbridge and processors that include * the same I2C unit (8240, 8245, 85xx). * - * Release 0.8 - * - * This file is licensed under the terms of the GNU General Public - * License version 2. This program is licensed "as is" without any - * warranty of any kind, whether express or implied. + * Copyright (C) 2003-2004 Humboldt Solutions Ltd, adrian@humboldt.co.uk + * Copyright (C) 2021 Allied Telesis Labs */ #include <linux/kernel.h> -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 3/6] i2c: mpc: Use devm_clk_get_optional() 2021-04-14 22:33 [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness Chris Packham 2021-04-14 22:33 ` [PATCH v4 1/6] i2c: mpc: Interrupt driven transfer Chris Packham 2021-04-14 22:33 ` [PATCH v4 2/6] i2c: mpc: Update license and copyright Chris Packham @ 2021-04-14 22:33 ` Chris Packham 2021-04-14 22:33 ` [PATCH v4 4/6] i2c: mpc: Remove CONFIG_PM_SLEEP ifdeffery Chris Packham ` (3 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Chris Packham @ 2021-04-14 22:33 UTC (permalink / raw) To: Wolfram Sang Cc: Andy Shevchenko, linux-i2c, linux-kernel, Andy Shevchenko, Chris Packham From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> The peripheral clock is optional and we may get an -EPROBE_DEFER error code which would not be propagated correctly, fix this by using devm_clk_get_optional(). Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- drivers/i2c/busses/i2c-mpc.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c index 37244138875a..d2209c04f67a 100644 --- a/drivers/i2c/busses/i2c-mpc.c +++ b/drivers/i2c/busses/i2c-mpc.c @@ -737,17 +737,18 @@ static int fsl_i2c_probe(struct platform_device *op) * enable clock for the I2C peripheral (non fatal), * keep a reference upon successful allocation */ - clk = devm_clk_get(&op->dev, NULL); - if (!IS_ERR(clk)) { - err = clk_prepare_enable(clk); - if (err) { - dev_err(&op->dev, "failed to enable clock\n"); - return err; - } else { - i2c->clk_per = clk; - } + clk = devm_clk_get_optional(&op->dev, NULL); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + err = clk_prepare_enable(clk); + if (err) { + dev_err(&op->dev, "failed to enable clock\n"); + return err; } + i2c->clk_per = clk; + if (of_property_read_bool(op->dev.of_node, "fsl,preserve-clocking")) { clock = MPC_I2C_CLOCK_PRESERVE; } else { @@ -791,8 +792,7 @@ static int fsl_i2c_probe(struct platform_device *op) return 0; fail_add: - if (i2c->clk_per) - clk_disable_unprepare(i2c->clk_per); + clk_disable_unprepare(i2c->clk_per); return result; }; @@ -803,8 +803,7 @@ static int fsl_i2c_remove(struct platform_device *op) i2c_del_adapter(&i2c->adap); - if (i2c->clk_per) - clk_disable_unprepare(i2c->clk_per); + clk_disable_unprepare(i2c->clk_per); return 0; }; -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 4/6] i2c: mpc: Remove CONFIG_PM_SLEEP ifdeffery 2021-04-14 22:33 [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness Chris Packham ` (2 preceding siblings ...) 2021-04-14 22:33 ` [PATCH v4 3/6] i2c: mpc: Use devm_clk_get_optional() Chris Packham @ 2021-04-14 22:33 ` Chris Packham 2021-04-14 22:33 ` [PATCH v4 5/6] i2c: mpc: Use device_get_match_data() helper Chris Packham ` (2 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Chris Packham @ 2021-04-14 22:33 UTC (permalink / raw) To: Wolfram Sang Cc: Andy Shevchenko, linux-i2c, linux-kernel, Andy Shevchenko, Chris Packham From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Use __maybe_unused for the suspend()/resume() hooks and get rid of the CONFIG_PM_SLEEP ifdeffery to improve the code. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Notes: Changes in v4: - Fix compile error due to MPC_I2C_PM_OPS not being defined drivers/i2c/busses/i2c-mpc.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c index d2209c04f67a..0865f7ac80bd 100644 --- a/drivers/i2c/busses/i2c-mpc.c +++ b/drivers/i2c/busses/i2c-mpc.c @@ -85,9 +85,7 @@ struct mpc_i2c { struct i2c_adapter adap; int irq; u32 real_clk; -#ifdef CONFIG_PM_SLEEP u8 fdr, dfsrr; -#endif struct clk *clk_per; u32 cntl_bits; enum mpc_i2c_action action; @@ -808,8 +806,7 @@ static int fsl_i2c_remove(struct platform_device *op) return 0; }; -#ifdef CONFIG_PM_SLEEP -static int mpc_i2c_suspend(struct device *dev) +static int __maybe_unused mpc_i2c_suspend(struct device *dev) { struct mpc_i2c *i2c = dev_get_drvdata(dev); @@ -819,7 +816,7 @@ static int mpc_i2c_suspend(struct device *dev) return 0; } -static int mpc_i2c_resume(struct device *dev) +static int __maybe_unused mpc_i2c_resume(struct device *dev) { struct mpc_i2c *i2c = dev_get_drvdata(dev); @@ -828,12 +825,7 @@ static int mpc_i2c_resume(struct device *dev) return 0; } - static SIMPLE_DEV_PM_OPS(mpc_i2c_pm_ops, mpc_i2c_suspend, mpc_i2c_resume); -#define MPC_I2C_PM_OPS (&mpc_i2c_pm_ops) -#else -#define MPC_I2C_PM_OPS NULL -#endif static const struct mpc_i2c_data mpc_i2c_data_512x = { .setup = mpc_i2c_setup_512x, @@ -876,7 +868,7 @@ static struct platform_driver mpc_i2c_driver = { .driver = { .name = DRV_NAME, .of_match_table = mpc_i2c_of_match, - .pm = MPC_I2C_PM_OPS, + .pm = &mpc_i2c_pm_ops, }, }; -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 5/6] i2c: mpc: Use device_get_match_data() helper 2021-04-14 22:33 [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness Chris Packham ` (3 preceding siblings ...) 2021-04-14 22:33 ` [PATCH v4 4/6] i2c: mpc: Remove CONFIG_PM_SLEEP ifdeffery Chris Packham @ 2021-04-14 22:33 ` Chris Packham 2021-04-14 22:33 ` [PATCH v4 6/6] i2c: mpc: Drop duplicate message from devm_platform_ioremap_resource() Chris Packham 2021-04-15 20:09 ` [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness Wolfram Sang 6 siblings, 0 replies; 8+ messages in thread From: Chris Packham @ 2021-04-14 22:33 UTC (permalink / raw) To: Wolfram Sang Cc: Andy Shevchenko, linux-i2c, linux-kernel, Andy Shevchenko, Chris Packham From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Use the device_get_match_data() helper instead of open coding. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- drivers/i2c/busses/i2c-mpc.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c index 0865f7ac80bd..7a9abeeb6da0 100644 --- a/drivers/i2c/busses/i2c-mpc.c +++ b/drivers/i2c/busses/i2c-mpc.c @@ -14,6 +14,7 @@ #include <linux/of_address.h> #include <linux/of_irq.h> #include <linux/of_platform.h> +#include <linux/property.h> #include <linux/slab.h> #include <linux/clk.h> @@ -689,10 +690,9 @@ static struct i2c_bus_recovery_info fsl_i2c_recovery_info = { .recover_bus = fsl_i2c_bus_recovery, }; -static const struct of_device_id mpc_i2c_of_match[]; static int fsl_i2c_probe(struct platform_device *op) { - const struct of_device_id *match; + const struct mpc_i2c_data *data; struct mpc_i2c *i2c; const u32 *prop; u32 clock = MPC_I2C_CLOCK_LEGACY; @@ -701,10 +701,6 @@ static int fsl_i2c_probe(struct platform_device *op) struct clk *clk; int err; - match = of_match_device(mpc_i2c_of_match, &op->dev); - if (!match) - return -EINVAL; - i2c = devm_kzalloc(&op->dev, sizeof(*i2c), GFP_KERNEL); if (!i2c) return -ENOMEM; @@ -756,8 +752,8 @@ static int fsl_i2c_probe(struct platform_device *op) clock = *prop; } - if (match->data) { - const struct mpc_i2c_data *data = match->data; + data = device_get_match_data(&op->dev); + if (data) { data->setup(op->dev.of_node, i2c, clock); } else { /* Backwards compatibility */ -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 6/6] i2c: mpc: Drop duplicate message from devm_platform_ioremap_resource() 2021-04-14 22:33 [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness Chris Packham ` (4 preceding siblings ...) 2021-04-14 22:33 ` [PATCH v4 5/6] i2c: mpc: Use device_get_match_data() helper Chris Packham @ 2021-04-14 22:33 ` Chris Packham 2021-04-15 20:09 ` [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness Wolfram Sang 6 siblings, 0 replies; 8+ messages in thread From: Chris Packham @ 2021-04-14 22:33 UTC (permalink / raw) To: Wolfram Sang Cc: Andy Shevchenko, linux-i2c, linux-kernel, Andy Shevchenko, Chris Packham From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> devm_platform_ioremap_resource() prints a message in case of error. Drop custom one. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- drivers/i2c/busses/i2c-mpc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c index 7a9abeeb6da0..30d9e89a3db2 100644 --- a/drivers/i2c/busses/i2c-mpc.c +++ b/drivers/i2c/busses/i2c-mpc.c @@ -711,10 +711,8 @@ static int fsl_i2c_probe(struct platform_device *op) spin_lock_init(&i2c->lock); i2c->base = devm_platform_ioremap_resource(op, 0); - if (IS_ERR(i2c->base)) { - dev_err(i2c->dev, "failed to map controller\n"); + if (IS_ERR(i2c->base)) return PTR_ERR(i2c->base); - } i2c->irq = platform_get_irq(op, 0); if (i2c->irq < 0) -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness 2021-04-14 22:33 [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness Chris Packham ` (5 preceding siblings ...) 2021-04-14 22:33 ` [PATCH v4 6/6] i2c: mpc: Drop duplicate message from devm_platform_ioremap_resource() Chris Packham @ 2021-04-15 20:09 ` Wolfram Sang 6 siblings, 0 replies; 8+ messages in thread From: Wolfram Sang @ 2021-04-15 20:09 UTC (permalink / raw) To: Chris Packham; +Cc: Andy Shevchenko, linux-i2c, linux-kernel [-- Attachment #1: Type: text/plain, Size: 292 bytes --] On Thu, Apr 15, 2021 at 10:33:19AM +1200, Chris Packham wrote: > I've tested on T2081 and P2041 based systems with a number of i2c and smbus > devices. > > I've included some clean ups provided by Andy Shevchenko to make applying the > series easier. Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-04-15 20:09 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-14 22:33 [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness Chris Packham 2021-04-14 22:33 ` [PATCH v4 1/6] i2c: mpc: Interrupt driven transfer Chris Packham 2021-04-14 22:33 ` [PATCH v4 2/6] i2c: mpc: Update license and copyright Chris Packham 2021-04-14 22:33 ` [PATCH v4 3/6] i2c: mpc: Use devm_clk_get_optional() Chris Packham 2021-04-14 22:33 ` [PATCH v4 4/6] i2c: mpc: Remove CONFIG_PM_SLEEP ifdeffery Chris Packham 2021-04-14 22:33 ` [PATCH v4 5/6] i2c: mpc: Use device_get_match_data() helper Chris Packham 2021-04-14 22:33 ` [PATCH v4 6/6] i2c: mpc: Drop duplicate message from devm_platform_ioremap_resource() Chris Packham 2021-04-15 20:09 ` [PATCH v4 0/6] i2c: mpc: Refactor to improve responsiveness Wolfram Sang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).