drivers/i2c/busses/i2c-omap.c | 209 ++++++++++++++++------------------ 1 file changed, 101 insertions(+), 108 deletions(-)
This commit addresses the coding style issues present in i2c-omap.c,
identified by checkpatch.pl and removes unused parameters present in
two functions.
1. Coding style issues includes Macro Utilization, alignnment
correction, updating ms_sleep() < 20 to usleep_range().
2. Removed unused parameters from omap_i2c_receive_data()
and omap_i2c_transmit_data().
No functional changes have been introduced in this commit.
Signed-off-by: Dhruv Menon <dhruvmenon1104@gmail.com>
---
drivers/i2c/busses/i2c-omap.c | 209 ++++++++++++++++------------------
1 file changed, 101 insertions(+), 108 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 1d9ad25c89ae..eee1671edebd 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -78,39 +78,39 @@ enum {
};
/* I2C Interrupt Enable Register (OMAP_I2C_IE): */
-#define OMAP_I2C_IE_XDR (1 << 14) /* TX Buffer drain int enable */
-#define OMAP_I2C_IE_RDR (1 << 13) /* RX Buffer drain int enable */
-#define OMAP_I2C_IE_XRDY (1 << 4) /* TX data ready int enable */
-#define OMAP_I2C_IE_RRDY (1 << 3) /* RX data ready int enable */
-#define OMAP_I2C_IE_ARDY (1 << 2) /* Access ready int enable */
-#define OMAP_I2C_IE_NACK (1 << 1) /* No ack interrupt enable */
-#define OMAP_I2C_IE_AL (1 << 0) /* Arbitration lost int ena */
+#define OMAP_I2C_IE_XDR BIT(14) /* TX Buffer drain int enable */
+#define OMAP_I2C_IE_RDR BIT(13) /* RX Buffer drain int enable */
+#define OMAP_I2C_IE_XRDY BIT(4) /* TX data ready int enable */
+#define OMAP_I2C_IE_RRDY BIT(3) /* RX data ready int enable */
+#define OMAP_I2C_IE_ARDY BIT(2) /* Access ready int enable */
+#define OMAP_I2C_IE_NACK BIT(1) /* No ack interrupt enable */
+#define OMAP_I2C_IE_AL BIT(0) /* Arbitration lost int ena */
/* I2C Status Register (OMAP_I2C_STAT): */
-#define OMAP_I2C_STAT_XDR (1 << 14) /* TX Buffer draining */
-#define OMAP_I2C_STAT_RDR (1 << 13) /* RX Buffer draining */
-#define OMAP_I2C_STAT_BB (1 << 12) /* Bus busy */
-#define OMAP_I2C_STAT_ROVR (1 << 11) /* Receive overrun */
-#define OMAP_I2C_STAT_XUDF (1 << 10) /* Transmit underflow */
-#define OMAP_I2C_STAT_AAS (1 << 9) /* Address as slave */
-#define OMAP_I2C_STAT_BF (1 << 8) /* Bus Free */
-#define OMAP_I2C_STAT_XRDY (1 << 4) /* Transmit data ready */
-#define OMAP_I2C_STAT_RRDY (1 << 3) /* Receive data ready */
-#define OMAP_I2C_STAT_ARDY (1 << 2) /* Register access ready */
-#define OMAP_I2C_STAT_NACK (1 << 1) /* No ack interrupt enable */
-#define OMAP_I2C_STAT_AL (1 << 0) /* Arbitration lost int ena */
+#define OMAP_I2C_STAT_XDR BIT(14) /* TX Buffer draining */
+#define OMAP_I2C_STAT_RDR BIT(13) /* RX Buffer draining */
+#define OMAP_I2C_STAT_BB BIT(12) /* Bus busy */
+#define OMAP_I2C_STAT_ROVR BIT(11) /* Receive overrun */
+#define OMAP_I2C_STAT_XUDF BIT(10) /* Transmit underflow */
+#define OMAP_I2C_STAT_AAS BIT(9) /* Address as slave */
+#define OMAP_I2C_STAT_BF BIT(8) /* Bus Free */
+#define OMAP_I2C_STAT_XRDY BIT(4) /* Transmit data ready */
+#define OMAP_I2C_STAT_RRDY BIT(3) /* Receive data ready */
+#define OMAP_I2C_STAT_ARDY BIT(2) /* Register access ready */
+#define OMAP_I2C_STAT_NACK BIT(1) /* No ack interrupt enable */
+#define OMAP_I2C_STAT_AL BIT(0) /* Arbitration lost int ena */
/* I2C WE wakeup enable register */
-#define OMAP_I2C_WE_XDR_WE (1 << 14) /* TX drain wakup */
-#define OMAP_I2C_WE_RDR_WE (1 << 13) /* RX drain wakeup */
-#define OMAP_I2C_WE_AAS_WE (1 << 9) /* Address as slave wakeup*/
-#define OMAP_I2C_WE_BF_WE (1 << 8) /* Bus free wakeup */
-#define OMAP_I2C_WE_STC_WE (1 << 6) /* Start condition wakeup */
-#define OMAP_I2C_WE_GC_WE (1 << 5) /* General call wakeup */
-#define OMAP_I2C_WE_DRDY_WE (1 << 3) /* TX/RX data ready wakeup */
-#define OMAP_I2C_WE_ARDY_WE (1 << 2) /* Reg access ready wakeup */
-#define OMAP_I2C_WE_NACK_WE (1 << 1) /* No acknowledgment wakeup */
-#define OMAP_I2C_WE_AL_WE (1 << 0) /* Arbitration lost wakeup */
+#define OMAP_I2C_WE_XDR_WE BIT(14) /* TX drain wakup */
+#define OMAP_I2C_WE_RDR_WE BIT(13) /* RX drain wakeup */
+#define OMAP_I2C_WE_AAS_WE BIT(9) /* Address as slave wakeup*/
+#define OMAP_I2C_WE_BF_WE BIT(8) /* Bus free wakeup */
+#define OMAP_I2C_WE_STC_WE BIT(6) /* Start condition wakeup */
+#define OMAP_I2C_WE_GC_WE BIT(5) /* General call wakeup */
+#define OMAP_I2C_WE_DRDY_WE BIT(3) /* TX/RX data ready wakeup */
+#define OMAP_I2C_WE_ARDY_WE BIT(2) /* Reg access ready wakeup */
+#define OMAP_I2C_WE_NACK_WE BIT(1) /* No acknowledgment wakeup */
+#define OMAP_I2C_WE_AL_WE BIT(0) /* Arbitration lost wakeup */
#define OMAP_I2C_WE_ALL (OMAP_I2C_WE_XDR_WE | OMAP_I2C_WE_RDR_WE | \
OMAP_I2C_WE_AAS_WE | OMAP_I2C_WE_BF_WE | \
@@ -119,59 +119,59 @@ enum {
OMAP_I2C_WE_NACK_WE | OMAP_I2C_WE_AL_WE)
/* I2C Buffer Configuration Register (OMAP_I2C_BUF): */
-#define OMAP_I2C_BUF_RDMA_EN (1 << 15) /* RX DMA channel enable */
-#define OMAP_I2C_BUF_RXFIF_CLR (1 << 14) /* RX FIFO Clear */
-#define OMAP_I2C_BUF_XDMA_EN (1 << 7) /* TX DMA channel enable */
-#define OMAP_I2C_BUF_TXFIF_CLR (1 << 6) /* TX FIFO Clear */
+#define OMAP_I2C_BUF_RDMA_EN BIT(15) /* RX DMA channel enable */
+#define OMAP_I2C_BUF_RXFIF_CLR BIT(14) /* RX FIFO Clear */
+#define OMAP_I2C_BUF_XDMA_EN BIT(7) /* TX DMA channel enable */
+#define OMAP_I2C_BUF_TXFIF_CLR BIT(6) /* TX FIFO Clear */
/* I2C Configuration Register (OMAP_I2C_CON): */
-#define OMAP_I2C_CON_EN (1 << 15) /* I2C module enable */
-#define OMAP_I2C_CON_BE (1 << 14) /* Big endian mode */
-#define OMAP_I2C_CON_OPMODE_HS (1 << 12) /* High Speed support */
-#define OMAP_I2C_CON_STB (1 << 11) /* Start byte mode (master) */
-#define OMAP_I2C_CON_MST (1 << 10) /* Master/slave mode */
-#define OMAP_I2C_CON_TRX (1 << 9) /* TX/RX mode (master only) */
-#define OMAP_I2C_CON_XA (1 << 8) /* Expand address */
-#define OMAP_I2C_CON_RM (1 << 2) /* Repeat mode (master only) */
-#define OMAP_I2C_CON_STP (1 << 1) /* Stop cond (master only) */
-#define OMAP_I2C_CON_STT (1 << 0) /* Start condition (master) */
+#define OMAP_I2C_CON_EN BIT(15) /* I2C module enable */
+#define OMAP_I2C_CON_BE BIT(14) /* Big endian mode */
+#define OMAP_I2C_CON_OPMODE_HS BIT(12) /* High Speed support */
+#define OMAP_I2C_CON_STB BIT(11) /* Start byte mode (master) */
+#define OMAP_I2C_CON_MST BIT(10) /* Master/slave mode */
+#define OMAP_I2C_CON_TRX BIT(9) /* TX/RX mode (master only) */
+#define OMAP_I2C_CON_XA BIT(8) /* Expand address */
+#define OMAP_I2C_CON_RM BIT(2) /* Repeat mode (master only) */
+#define OMAP_I2C_CON_STP BIT(1) /* Stop cond (master only) */
+#define OMAP_I2C_CON_STT BIT(0) /* Start condition (master) */
/* I2C SCL time value when Master */
#define OMAP_I2C_SCLL_HSSCLL 8
#define OMAP_I2C_SCLH_HSSCLH 8
/* I2C System Test Register (OMAP_I2C_SYSTEST): */
-#define OMAP_I2C_SYSTEST_ST_EN (1 << 15) /* System test enable */
-#define OMAP_I2C_SYSTEST_FREE (1 << 14) /* Free running mode */
-#define OMAP_I2C_SYSTEST_TMODE_MASK (3 << 12) /* Test mode select */
-#define OMAP_I2C_SYSTEST_TMODE_SHIFT (12) /* Test mode select */
+#define OMAP_I2C_SYSTEST_ST_EN BIT(15) /* System test enable */
+#define OMAP_I2C_SYSTEST_FREE BIT(14) /* Free running mode */
+#define OMAP_I2C_SYSTEST_TMODE_MASK GENMASK(13, 12) /* Test mode select */
+#define OMAP_I2C_SYSTEST_TMODE_SHIFT (12) /* Test mode select */
/* Functional mode */
-#define OMAP_I2C_SYSTEST_SCL_I_FUNC (1 << 8) /* SCL line input value */
-#define OMAP_I2C_SYSTEST_SCL_O_FUNC (1 << 7) /* SCL line output value */
-#define OMAP_I2C_SYSTEST_SDA_I_FUNC (1 << 6) /* SDA line input value */
-#define OMAP_I2C_SYSTEST_SDA_O_FUNC (1 << 5) /* SDA line output value */
+#define OMAP_I2C_SYSTEST_SCL_I_FUNC BIT(8) /* SCL line input value */
+#define OMAP_I2C_SYSTEST_SCL_O_FUNC BIT(7) /* SCL line output value */
+#define OMAP_I2C_SYSTEST_SDA_I_FUNC BIT(6) /* SDA line input value */
+#define OMAP_I2C_SYSTEST_SDA_O_FUNC BIT(5) /* SDA line output value */
/* SDA/SCL IO mode */
-#define OMAP_I2C_SYSTEST_SCL_I (1 << 3) /* SCL line sense in */
-#define OMAP_I2C_SYSTEST_SCL_O (1 << 2) /* SCL line drive out */
-#define OMAP_I2C_SYSTEST_SDA_I (1 << 1) /* SDA line sense in */
-#define OMAP_I2C_SYSTEST_SDA_O (1 << 0) /* SDA line drive out */
+#define OMAP_I2C_SYSTEST_SCL_I BIT(3) /* SCL line sense in */
+#define OMAP_I2C_SYSTEST_SCL_O BIT(2) /* SCL line drive out */
+#define OMAP_I2C_SYSTEST_SDA_I BIT(1) /* SDA line sense in */
+#define OMAP_I2C_SYSTEST_SDA_O BIT(0) /* SDA line drive out */
/* OCP_SYSSTATUS bit definitions */
-#define SYSS_RESETDONE_MASK (1 << 0)
+#define SYSS_RESETDONE_MASK BIT(0)
/* OCP_SYSCONFIG bit definitions */
-#define SYSC_CLOCKACTIVITY_MASK (0x3 << 8)
-#define SYSC_SIDLEMODE_MASK (0x3 << 3)
-#define SYSC_ENAWAKEUP_MASK (1 << 2)
-#define SYSC_SOFTRESET_MASK (1 << 1)
-#define SYSC_AUTOIDLE_MASK (1 << 0)
+#define SYSC_CLOCKACTIVITY_MASK GENMASK(9, 8)
+#define SYSC_SIDLEMODE_MASK GENMASK(4, 3)
+#define SYSC_ENAWAKEUP_MASK BIT(2)
+#define SYSC_SOFTRESET_MASK BIT(1)
+#define SYSC_AUTOIDLE_MASK BIT(0)
#define SYSC_IDLEMODE_SMART 0x2
#define SYSC_CLOCKACTIVITY_FCLK 0x2
/* Errata definitions */
-#define I2C_OMAP_ERRATA_I207 (1 << 0)
-#define I2C_OMAP_ERRATA_I462 (1 << 1)
+#define I2C_OMAP_ERRATA_I207 BIT(0)
+#define I2C_OMAP_ERRATA_I462 BIT(1)
#define OMAP_I2C_IP_V2_INTERRUPTS_MASK 0x6FFF
@@ -277,7 +277,6 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev *omap, int reg)
static void __omap_i2c_init(struct omap_i2c_dev *omap)
{
-
omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, 0);
/* Setup clock prescaler to obtain approx 12MHz I2C module clock: */
@@ -316,22 +315,22 @@ static int omap_i2c_reset(struct omap_i2c_dev *omap)
/* Disable I2C controller before soft reset */
omap_i2c_write_reg(omap, OMAP_I2C_CON_REG,
- omap_i2c_read_reg(omap, OMAP_I2C_CON_REG) &
- ~(OMAP_I2C_CON_EN));
+ omap_i2c_read_reg(omap, OMAP_I2C_CON_REG) &
+ ~(OMAP_I2C_CON_EN));
omap_i2c_write_reg(omap, OMAP_I2C_SYSC_REG, SYSC_SOFTRESET_MASK);
/* For some reason we need to set the EN bit before the
- * reset done bit gets set. */
+ * reset done bit gets set.
+ */
timeout = jiffies + OMAP_I2C_TIMEOUT;
omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
while (!(omap_i2c_read_reg(omap, OMAP_I2C_SYSS_REG) &
SYSS_RESETDONE_MASK)) {
if (time_after(jiffies, timeout)) {
- dev_warn(omap->dev, "timeout waiting "
- "for controller reset\n");
+ dev_warn(omap->dev, "timeout waiting for controller reset\n");
return -ETIMEDOUT;
}
- msleep(1);
+ usleep_range(1000, 2000);
}
/* SYSC register is cleared by the reset; rewrite it */
@@ -396,15 +395,13 @@ static int omap_i2c_init(struct omap_i2c_dev *omap)
}
if (!(omap->flags & OMAP_I2C_FLAG_SIMPLE_CLOCK)) {
-
/*
* HSI2C controller internal clk rate should be 19.2 Mhz for
* HS and for all modes on 2430. On 34xx we can use lower rate
* to get longer filter period for better noise suppression.
* The filter is iclk (fclk for HS) period.
*/
- if (omap->speed > 400 ||
- omap->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK)
+ if (omap->speed > 400 || omap->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK)
internal_clk = 19200;
else if (omap->speed > 100)
internal_clk = 9600;
@@ -506,7 +503,7 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *omap)
while (omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG) & OMAP_I2C_STAT_BB) {
if (time_after(jiffies, timeout))
return omap_i2c_recover_bus(omap);
- msleep(1);
+ usleep_range(1000, 2000);
}
return 0;
@@ -595,7 +592,7 @@ static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *omap)
return omap_i2c_recover_bus(omap);
}
- msleep(1);
+ usleep_range(1000, 2000);
}
omap->bb_valid = 1;
@@ -616,7 +613,7 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *omap, u8 size, bool is_rx)
* then we might use draining feature to transfer the remaining bytes.
*/
- omap->threshold = clamp(size, (u8) 1, omap->fifo_size);
+ omap->threshold = clamp(size, (u8)1, omap->fifo_size);
buf = omap_i2c_read_reg(omap, OMAP_I2C_BUF_REG);
@@ -636,7 +633,7 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *omap, u8 size, bool is_rx)
omap->b_hw = 1; /* Enable hardware fixes */
/* calculate wakeup latency constraint for MPU */
- if (omap->set_mpu_wkup_lat != NULL)
+ if (omap->set_mpu_wkup_lat)
omap->latency = (1000000 * omap->threshold) /
(1000 * omap->speed / 8);
}
@@ -718,13 +715,13 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
if (omap->b_hw && stop) {
unsigned long delay = jiffies + OMAP_I2C_TIMEOUT;
u16 con = omap_i2c_read_reg(omap, OMAP_I2C_CON_REG);
+
while (con & OMAP_I2C_CON_STT) {
con = omap_i2c_read_reg(omap, OMAP_I2C_CON_REG);
/* Let the user know if i2c is in a bad state */
if (time_after(jiffies, delay)) {
- dev_err(omap->dev, "controller timed out "
- "waiting for start condition to finish\n");
+ dev_err(omap->dev, "controller timed out waiting for start condition to finish\n");
return -ETIMEDOUT;
}
cpu_relax();
@@ -782,7 +779,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
return -EIO;
}
-
/*
* Prepare controller for a transaction and call omap_i2c_xfer_msg
* to do the work during IRQ processing.
@@ -807,7 +803,7 @@ omap_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg msgs[], int num,
if (r < 0)
goto out;
- if (omap->set_mpu_wkup_lat != NULL)
+ if (omap->set_mpu_wkup_lat)
omap->set_mpu_wkup_lat(omap->dev, omap->latency);
for (i = 0; i < num; i++) {
@@ -822,7 +818,7 @@ omap_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg msgs[], int num,
omap_i2c_wait_for_bb(omap);
- if (omap->set_mpu_wkup_lat != NULL)
+ if (omap->set_mpu_wkup_lat)
omap->set_mpu_wkup_lat(omap->dev, -1);
out:
@@ -875,18 +871,15 @@ static inline void i2c_omap_errata_i207(struct omap_i2c_dev *omap, u16 stat)
if (stat & OMAP_I2C_STAT_RDR) {
/* Step 1: If RDR is set, clear it */
omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RDR);
-
/* Step 2: */
if (!(omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG)
& OMAP_I2C_STAT_BB)) {
-
/* Step 3: */
if (omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG)
& OMAP_I2C_STAT_RDR) {
omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RDR);
dev_dbg(omap->dev, "RDR when bus is busy.\n");
}
-
}
}
}
@@ -911,7 +904,7 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)
dev_err(omap->dev, "Arbitration lost\n");
omap_i2c_complete_cmd(omap, OMAP_I2C_STAT_AL);
break;
- case 0x02: /* No acknowledgement */
+ case 0x02: /* No acknowledgment */
omap_i2c_complete_cmd(omap, OMAP_I2C_STAT_NACK);
omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, OMAP_I2C_CON_STP);
break;
@@ -927,8 +920,9 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)
*omap->buf++ = w >> 8;
omap->buf_len--;
}
- } else
+ } else {
dev_err(omap->dev, "RRDY IRQ while no data requested\n");
+ }
break;
case 0x05: /* Transmit data ready */
if (omap->buf_len) {
@@ -939,8 +933,9 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)
omap->buf_len--;
}
omap_i2c_write_reg(omap, OMAP_I2C_DATA_REG, w);
- } else
+ } else {
dev_err(omap->dev, "XRDY IRQ while no data to send\n");
+ }
break;
default:
return IRQ_NONE;
@@ -995,8 +990,7 @@ static int errata_omap3_i462(struct omap_i2c_dev *omap)
return 0;
}
-static void omap_i2c_receive_data(struct omap_i2c_dev *omap, u8 num_bytes,
- bool is_rdr)
+static void omap_i2c_receive_data(struct omap_i2c_dev *omap, u8 num_bytes)
{
u16 w;
@@ -1016,8 +1010,7 @@ static void omap_i2c_receive_data(struct omap_i2c_dev *omap, u8 num_bytes,
}
}
-static int omap_i2c_transmit_data(struct omap_i2c_dev *omap, u8 num_bytes,
- bool is_xdr)
+static int omap_i2c_transmit_data(struct omap_i2c_dev *omap, u8 num_bytes)
{
u16 w;
@@ -1133,7 +1126,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
OMAP_I2C_BUFSTAT_REG) >> 8) & 0x3F;
}
- omap_i2c_receive_data(omap, num_bytes, true);
+ omap_i2c_receive_data(omap, num_bytes);
omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RDR);
continue;
}
@@ -1144,7 +1137,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
if (omap->threshold)
num_bytes = omap->threshold;
- omap_i2c_receive_data(omap, num_bytes, false);
+ omap_i2c_receive_data(omap, num_bytes);
omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RRDY);
continue;
}
@@ -1156,7 +1149,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
if (omap->fifo_size)
num_bytes = omap->buf_len;
- ret = omap_i2c_transmit_data(omap, num_bytes, true);
+ ret = omap_i2c_transmit_data(omap, num_bytes);
if (ret < 0)
break;
@@ -1171,7 +1164,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
if (omap->threshold)
num_bytes = omap->threshold;
- ret = omap_i2c_transmit_data(omap, num_bytes, false);
+ ret = omap_i2c_transmit_data(omap, num_bytes);
if (ret < 0)
break;
@@ -1266,13 +1259,13 @@ static const struct of_device_id omap_i2c_of_match[] = {
MODULE_DEVICE_TABLE(of, omap_i2c_of_match);
#endif
-#define OMAP_I2C_SCHEME(rev) ((rev & 0xc000) >> 14)
+#define OMAP_I2C_SCHEME(rev) (((rev) & 0xc000) >> 14)
-#define OMAP_I2C_REV_SCHEME_0_MAJOR(rev) (rev >> 4)
-#define OMAP_I2C_REV_SCHEME_0_MINOR(rev) (rev & 0xf)
+#define OMAP_I2C_REV_SCHEME_0_MAJOR(rev) (((rev) >> 4))
+#define OMAP_I2C_REV_SCHEME_0_MINOR(rev) (((rev) & 0xf))
-#define OMAP_I2C_REV_SCHEME_1_MAJOR(rev) ((rev & 0x0700) >> 7)
-#define OMAP_I2C_REV_SCHEME_1_MINOR(rev) (rev & 0x1f)
+#define OMAP_I2C_REV_SCHEME_1_MAJOR(rev) (((rev) & 0x0700) >> 7)
+#define OMAP_I2C_REV_SCHEME_1_MINOR(rev) (((rev) & 0x1f))
#define OMAP_I2C_SCHEME_0 0
#define OMAP_I2C_SCHEME_1 1
@@ -1383,7 +1376,7 @@ omap_i2c_probe(struct platform_device *pdev)
of_property_read_u32(node, "clock-frequency", &freq);
/* convert DT freq value in Hz into kHz for speed */
omap->speed = freq / 1000;
- } else if (pdata != NULL) {
+ } else if (pdata) {
omap->speed = pdata->clkrate;
omap->flags = pdata->flags;
omap->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat;
@@ -1434,7 +1427,7 @@ omap_i2c_probe(struct platform_device *pdev)
omap->errata = 0;
if (omap->rev >= OMAP_I2C_REV_ON_2430 &&
- omap->rev < OMAP_I2C_REV_ON_4430_PLUS)
+ omap->rev < OMAP_I2C_REV_ON_4430_PLUS)
omap->errata |= I2C_OMAP_ERRATA_I207;
if (omap->rev <= OMAP_I2C_REV_ON_3430_3530)
@@ -1459,7 +1452,7 @@ omap_i2c_probe(struct platform_device *pdev)
omap->b_hw = 1; /* Enable hardware fixes */
/* calculate wakeup latency constraint for MPU */
- if (omap->set_mpu_wkup_lat != NULL)
+ if (omap->set_mpu_wkup_lat)
omap->latency = (1000000 * omap->fifo_size) /
(1000 * omap->speed / 8);
}
@@ -1469,12 +1462,12 @@ omap_i2c_probe(struct platform_device *pdev)
if (omap->rev < OMAP_I2C_OMAP1_REV_2)
r = devm_request_irq(&pdev->dev, omap->irq, omap_i2c_omap1_isr,
- IRQF_NO_SUSPEND, pdev->name, omap);
+ IRQF_NO_SUSPEND, pdev->name, omap);
else
r = devm_request_threaded_irq(&pdev->dev, omap->irq,
- omap_i2c_isr, omap_i2c_isr_thread,
- IRQF_NO_SUSPEND | IRQF_ONESHOT,
- pdev->name, omap);
+ omap_i2c_isr, omap_i2c_isr_thread,
+ IRQF_NO_SUSPEND | IRQF_ONESHOT,
+ pdev->name, omap);
if (r) {
dev_err(omap->dev, "failure requesting irq %i\n", omap->irq);
--
2.43.0On Tue, Dec 03, 2024 at 12:22:51AM +0530, Dhruv Menon wrote:
> This commit addresses the coding style issues present in i2c-omap.c,
> identified by checkpatch.pl and removes unused parameters present in
> two functions.
>
> 1. Coding style issues includes Macro Utilization, alignnment
> correction, updating ms_sleep() < 20 to usleep_range().
> 2. Removed unused parameters from omap_i2c_receive_data()
> and omap_i2c_transmit_data().
>
> No functional changes have been introduced in this commit.
Not sure if that is correct as sleeps can be now shorter? I wouldn't
touch them unless you can show some real benefit (checkpatch.pl warning
isn't one for old driver code).
Maybe also changes should be split into separate patches for easier
review.
A.
>
> Signed-off-by: Dhruv Menon <dhruvmenon1104@gmail.com>
> ---
> drivers/i2c/busses/i2c-omap.c | 209 ++++++++++++++++------------------
> 1 file changed, 101 insertions(+), 108 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 1d9ad25c89ae..eee1671edebd 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -78,39 +78,39 @@ enum {
> };
>
> /* I2C Interrupt Enable Register (OMAP_I2C_IE): */
> -#define OMAP_I2C_IE_XDR (1 << 14) /* TX Buffer drain int enable */
> -#define OMAP_I2C_IE_RDR (1 << 13) /* RX Buffer drain int enable */
> -#define OMAP_I2C_IE_XRDY (1 << 4) /* TX data ready int enable */
> -#define OMAP_I2C_IE_RRDY (1 << 3) /* RX data ready int enable */
> -#define OMAP_I2C_IE_ARDY (1 << 2) /* Access ready int enable */
> -#define OMAP_I2C_IE_NACK (1 << 1) /* No ack interrupt enable */
> -#define OMAP_I2C_IE_AL (1 << 0) /* Arbitration lost int ena */
> +#define OMAP_I2C_IE_XDR BIT(14) /* TX Buffer drain int enable */
> +#define OMAP_I2C_IE_RDR BIT(13) /* RX Buffer drain int enable */
> +#define OMAP_I2C_IE_XRDY BIT(4) /* TX data ready int enable */
> +#define OMAP_I2C_IE_RRDY BIT(3) /* RX data ready int enable */
> +#define OMAP_I2C_IE_ARDY BIT(2) /* Access ready int enable */
> +#define OMAP_I2C_IE_NACK BIT(1) /* No ack interrupt enable */
> +#define OMAP_I2C_IE_AL BIT(0) /* Arbitration lost int ena */
>
> /* I2C Status Register (OMAP_I2C_STAT): */
> -#define OMAP_I2C_STAT_XDR (1 << 14) /* TX Buffer draining */
> -#define OMAP_I2C_STAT_RDR (1 << 13) /* RX Buffer draining */
> -#define OMAP_I2C_STAT_BB (1 << 12) /* Bus busy */
> -#define OMAP_I2C_STAT_ROVR (1 << 11) /* Receive overrun */
> -#define OMAP_I2C_STAT_XUDF (1 << 10) /* Transmit underflow */
> -#define OMAP_I2C_STAT_AAS (1 << 9) /* Address as slave */
> -#define OMAP_I2C_STAT_BF (1 << 8) /* Bus Free */
> -#define OMAP_I2C_STAT_XRDY (1 << 4) /* Transmit data ready */
> -#define OMAP_I2C_STAT_RRDY (1 << 3) /* Receive data ready */
> -#define OMAP_I2C_STAT_ARDY (1 << 2) /* Register access ready */
> -#define OMAP_I2C_STAT_NACK (1 << 1) /* No ack interrupt enable */
> -#define OMAP_I2C_STAT_AL (1 << 0) /* Arbitration lost int ena */
> +#define OMAP_I2C_STAT_XDR BIT(14) /* TX Buffer draining */
> +#define OMAP_I2C_STAT_RDR BIT(13) /* RX Buffer draining */
> +#define OMAP_I2C_STAT_BB BIT(12) /* Bus busy */
> +#define OMAP_I2C_STAT_ROVR BIT(11) /* Receive overrun */
> +#define OMAP_I2C_STAT_XUDF BIT(10) /* Transmit underflow */
> +#define OMAP_I2C_STAT_AAS BIT(9) /* Address as slave */
> +#define OMAP_I2C_STAT_BF BIT(8) /* Bus Free */
> +#define OMAP_I2C_STAT_XRDY BIT(4) /* Transmit data ready */
> +#define OMAP_I2C_STAT_RRDY BIT(3) /* Receive data ready */
> +#define OMAP_I2C_STAT_ARDY BIT(2) /* Register access ready */
> +#define OMAP_I2C_STAT_NACK BIT(1) /* No ack interrupt enable */
> +#define OMAP_I2C_STAT_AL BIT(0) /* Arbitration lost int ena */
>
> /* I2C WE wakeup enable register */
> -#define OMAP_I2C_WE_XDR_WE (1 << 14) /* TX drain wakup */
> -#define OMAP_I2C_WE_RDR_WE (1 << 13) /* RX drain wakeup */
> -#define OMAP_I2C_WE_AAS_WE (1 << 9) /* Address as slave wakeup*/
> -#define OMAP_I2C_WE_BF_WE (1 << 8) /* Bus free wakeup */
> -#define OMAP_I2C_WE_STC_WE (1 << 6) /* Start condition wakeup */
> -#define OMAP_I2C_WE_GC_WE (1 << 5) /* General call wakeup */
> -#define OMAP_I2C_WE_DRDY_WE (1 << 3) /* TX/RX data ready wakeup */
> -#define OMAP_I2C_WE_ARDY_WE (1 << 2) /* Reg access ready wakeup */
> -#define OMAP_I2C_WE_NACK_WE (1 << 1) /* No acknowledgment wakeup */
> -#define OMAP_I2C_WE_AL_WE (1 << 0) /* Arbitration lost wakeup */
> +#define OMAP_I2C_WE_XDR_WE BIT(14) /* TX drain wakup */
> +#define OMAP_I2C_WE_RDR_WE BIT(13) /* RX drain wakeup */
> +#define OMAP_I2C_WE_AAS_WE BIT(9) /* Address as slave wakeup*/
> +#define OMAP_I2C_WE_BF_WE BIT(8) /* Bus free wakeup */
> +#define OMAP_I2C_WE_STC_WE BIT(6) /* Start condition wakeup */
> +#define OMAP_I2C_WE_GC_WE BIT(5) /* General call wakeup */
> +#define OMAP_I2C_WE_DRDY_WE BIT(3) /* TX/RX data ready wakeup */
> +#define OMAP_I2C_WE_ARDY_WE BIT(2) /* Reg access ready wakeup */
> +#define OMAP_I2C_WE_NACK_WE BIT(1) /* No acknowledgment wakeup */
> +#define OMAP_I2C_WE_AL_WE BIT(0) /* Arbitration lost wakeup */
>
> #define OMAP_I2C_WE_ALL (OMAP_I2C_WE_XDR_WE | OMAP_I2C_WE_RDR_WE | \
> OMAP_I2C_WE_AAS_WE | OMAP_I2C_WE_BF_WE | \
> @@ -119,59 +119,59 @@ enum {
> OMAP_I2C_WE_NACK_WE | OMAP_I2C_WE_AL_WE)
>
> /* I2C Buffer Configuration Register (OMAP_I2C_BUF): */
> -#define OMAP_I2C_BUF_RDMA_EN (1 << 15) /* RX DMA channel enable */
> -#define OMAP_I2C_BUF_RXFIF_CLR (1 << 14) /* RX FIFO Clear */
> -#define OMAP_I2C_BUF_XDMA_EN (1 << 7) /* TX DMA channel enable */
> -#define OMAP_I2C_BUF_TXFIF_CLR (1 << 6) /* TX FIFO Clear */
> +#define OMAP_I2C_BUF_RDMA_EN BIT(15) /* RX DMA channel enable */
> +#define OMAP_I2C_BUF_RXFIF_CLR BIT(14) /* RX FIFO Clear */
> +#define OMAP_I2C_BUF_XDMA_EN BIT(7) /* TX DMA channel enable */
> +#define OMAP_I2C_BUF_TXFIF_CLR BIT(6) /* TX FIFO Clear */
>
> /* I2C Configuration Register (OMAP_I2C_CON): */
> -#define OMAP_I2C_CON_EN (1 << 15) /* I2C module enable */
> -#define OMAP_I2C_CON_BE (1 << 14) /* Big endian mode */
> -#define OMAP_I2C_CON_OPMODE_HS (1 << 12) /* High Speed support */
> -#define OMAP_I2C_CON_STB (1 << 11) /* Start byte mode (master) */
> -#define OMAP_I2C_CON_MST (1 << 10) /* Master/slave mode */
> -#define OMAP_I2C_CON_TRX (1 << 9) /* TX/RX mode (master only) */
> -#define OMAP_I2C_CON_XA (1 << 8) /* Expand address */
> -#define OMAP_I2C_CON_RM (1 << 2) /* Repeat mode (master only) */
> -#define OMAP_I2C_CON_STP (1 << 1) /* Stop cond (master only) */
> -#define OMAP_I2C_CON_STT (1 << 0) /* Start condition (master) */
> +#define OMAP_I2C_CON_EN BIT(15) /* I2C module enable */
> +#define OMAP_I2C_CON_BE BIT(14) /* Big endian mode */
> +#define OMAP_I2C_CON_OPMODE_HS BIT(12) /* High Speed support */
> +#define OMAP_I2C_CON_STB BIT(11) /* Start byte mode (master) */
> +#define OMAP_I2C_CON_MST BIT(10) /* Master/slave mode */
> +#define OMAP_I2C_CON_TRX BIT(9) /* TX/RX mode (master only) */
> +#define OMAP_I2C_CON_XA BIT(8) /* Expand address */
> +#define OMAP_I2C_CON_RM BIT(2) /* Repeat mode (master only) */
> +#define OMAP_I2C_CON_STP BIT(1) /* Stop cond (master only) */
> +#define OMAP_I2C_CON_STT BIT(0) /* Start condition (master) */
>
> /* I2C SCL time value when Master */
> #define OMAP_I2C_SCLL_HSSCLL 8
> #define OMAP_I2C_SCLH_HSSCLH 8
>
> /* I2C System Test Register (OMAP_I2C_SYSTEST): */
> -#define OMAP_I2C_SYSTEST_ST_EN (1 << 15) /* System test enable */
> -#define OMAP_I2C_SYSTEST_FREE (1 << 14) /* Free running mode */
> -#define OMAP_I2C_SYSTEST_TMODE_MASK (3 << 12) /* Test mode select */
> -#define OMAP_I2C_SYSTEST_TMODE_SHIFT (12) /* Test mode select */
> +#define OMAP_I2C_SYSTEST_ST_EN BIT(15) /* System test enable */
> +#define OMAP_I2C_SYSTEST_FREE BIT(14) /* Free running mode */
> +#define OMAP_I2C_SYSTEST_TMODE_MASK GENMASK(13, 12) /* Test mode select */
> +#define OMAP_I2C_SYSTEST_TMODE_SHIFT (12) /* Test mode select */
> /* Functional mode */
> -#define OMAP_I2C_SYSTEST_SCL_I_FUNC (1 << 8) /* SCL line input value */
> -#define OMAP_I2C_SYSTEST_SCL_O_FUNC (1 << 7) /* SCL line output value */
> -#define OMAP_I2C_SYSTEST_SDA_I_FUNC (1 << 6) /* SDA line input value */
> -#define OMAP_I2C_SYSTEST_SDA_O_FUNC (1 << 5) /* SDA line output value */
> +#define OMAP_I2C_SYSTEST_SCL_I_FUNC BIT(8) /* SCL line input value */
> +#define OMAP_I2C_SYSTEST_SCL_O_FUNC BIT(7) /* SCL line output value */
> +#define OMAP_I2C_SYSTEST_SDA_I_FUNC BIT(6) /* SDA line input value */
> +#define OMAP_I2C_SYSTEST_SDA_O_FUNC BIT(5) /* SDA line output value */
> /* SDA/SCL IO mode */
> -#define OMAP_I2C_SYSTEST_SCL_I (1 << 3) /* SCL line sense in */
> -#define OMAP_I2C_SYSTEST_SCL_O (1 << 2) /* SCL line drive out */
> -#define OMAP_I2C_SYSTEST_SDA_I (1 << 1) /* SDA line sense in */
> -#define OMAP_I2C_SYSTEST_SDA_O (1 << 0) /* SDA line drive out */
> +#define OMAP_I2C_SYSTEST_SCL_I BIT(3) /* SCL line sense in */
> +#define OMAP_I2C_SYSTEST_SCL_O BIT(2) /* SCL line drive out */
> +#define OMAP_I2C_SYSTEST_SDA_I BIT(1) /* SDA line sense in */
> +#define OMAP_I2C_SYSTEST_SDA_O BIT(0) /* SDA line drive out */
>
> /* OCP_SYSSTATUS bit definitions */
> -#define SYSS_RESETDONE_MASK (1 << 0)
> +#define SYSS_RESETDONE_MASK BIT(0)
>
> /* OCP_SYSCONFIG bit definitions */
> -#define SYSC_CLOCKACTIVITY_MASK (0x3 << 8)
> -#define SYSC_SIDLEMODE_MASK (0x3 << 3)
> -#define SYSC_ENAWAKEUP_MASK (1 << 2)
> -#define SYSC_SOFTRESET_MASK (1 << 1)
> -#define SYSC_AUTOIDLE_MASK (1 << 0)
> +#define SYSC_CLOCKACTIVITY_MASK GENMASK(9, 8)
> +#define SYSC_SIDLEMODE_MASK GENMASK(4, 3)
> +#define SYSC_ENAWAKEUP_MASK BIT(2)
> +#define SYSC_SOFTRESET_MASK BIT(1)
> +#define SYSC_AUTOIDLE_MASK BIT(0)
>
> #define SYSC_IDLEMODE_SMART 0x2
> #define SYSC_CLOCKACTIVITY_FCLK 0x2
>
> /* Errata definitions */
> -#define I2C_OMAP_ERRATA_I207 (1 << 0)
> -#define I2C_OMAP_ERRATA_I462 (1 << 1)
> +#define I2C_OMAP_ERRATA_I207 BIT(0)
> +#define I2C_OMAP_ERRATA_I462 BIT(1)
>
> #define OMAP_I2C_IP_V2_INTERRUPTS_MASK 0x6FFF
>
> @@ -277,7 +277,6 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev *omap, int reg)
>
> static void __omap_i2c_init(struct omap_i2c_dev *omap)
> {
> -
> omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, 0);
>
> /* Setup clock prescaler to obtain approx 12MHz I2C module clock: */
> @@ -316,22 +315,22 @@ static int omap_i2c_reset(struct omap_i2c_dev *omap)
>
> /* Disable I2C controller before soft reset */
> omap_i2c_write_reg(omap, OMAP_I2C_CON_REG,
> - omap_i2c_read_reg(omap, OMAP_I2C_CON_REG) &
> - ~(OMAP_I2C_CON_EN));
> + omap_i2c_read_reg(omap, OMAP_I2C_CON_REG) &
> + ~(OMAP_I2C_CON_EN));
>
> omap_i2c_write_reg(omap, OMAP_I2C_SYSC_REG, SYSC_SOFTRESET_MASK);
> /* For some reason we need to set the EN bit before the
> - * reset done bit gets set. */
> + * reset done bit gets set.
> + */
> timeout = jiffies + OMAP_I2C_TIMEOUT;
> omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
> while (!(omap_i2c_read_reg(omap, OMAP_I2C_SYSS_REG) &
> SYSS_RESETDONE_MASK)) {
> if (time_after(jiffies, timeout)) {
> - dev_warn(omap->dev, "timeout waiting "
> - "for controller reset\n");
> + dev_warn(omap->dev, "timeout waiting for controller reset\n");
> return -ETIMEDOUT;
> }
> - msleep(1);
> + usleep_range(1000, 2000);
> }
>
> /* SYSC register is cleared by the reset; rewrite it */
> @@ -396,15 +395,13 @@ static int omap_i2c_init(struct omap_i2c_dev *omap)
> }
>
> if (!(omap->flags & OMAP_I2C_FLAG_SIMPLE_CLOCK)) {
> -
> /*
> * HSI2C controller internal clk rate should be 19.2 Mhz for
> * HS and for all modes on 2430. On 34xx we can use lower rate
> * to get longer filter period for better noise suppression.
> * The filter is iclk (fclk for HS) period.
> */
> - if (omap->speed > 400 ||
> - omap->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK)
> + if (omap->speed > 400 || omap->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK)
> internal_clk = 19200;
> else if (omap->speed > 100)
> internal_clk = 9600;
> @@ -506,7 +503,7 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *omap)
> while (omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG) & OMAP_I2C_STAT_BB) {
> if (time_after(jiffies, timeout))
> return omap_i2c_recover_bus(omap);
> - msleep(1);
> + usleep_range(1000, 2000);
> }
>
> return 0;
> @@ -595,7 +592,7 @@ static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *omap)
> return omap_i2c_recover_bus(omap);
> }
>
> - msleep(1);
> + usleep_range(1000, 2000);
> }
>
> omap->bb_valid = 1;
> @@ -616,7 +613,7 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *omap, u8 size, bool is_rx)
> * then we might use draining feature to transfer the remaining bytes.
> */
>
> - omap->threshold = clamp(size, (u8) 1, omap->fifo_size);
> + omap->threshold = clamp(size, (u8)1, omap->fifo_size);
>
> buf = omap_i2c_read_reg(omap, OMAP_I2C_BUF_REG);
>
> @@ -636,7 +633,7 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *omap, u8 size, bool is_rx)
> omap->b_hw = 1; /* Enable hardware fixes */
>
> /* calculate wakeup latency constraint for MPU */
> - if (omap->set_mpu_wkup_lat != NULL)
> + if (omap->set_mpu_wkup_lat)
> omap->latency = (1000000 * omap->threshold) /
> (1000 * omap->speed / 8);
> }
> @@ -718,13 +715,13 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
> if (omap->b_hw && stop) {
> unsigned long delay = jiffies + OMAP_I2C_TIMEOUT;
> u16 con = omap_i2c_read_reg(omap, OMAP_I2C_CON_REG);
> +
> while (con & OMAP_I2C_CON_STT) {
> con = omap_i2c_read_reg(omap, OMAP_I2C_CON_REG);
>
> /* Let the user know if i2c is in a bad state */
> if (time_after(jiffies, delay)) {
> - dev_err(omap->dev, "controller timed out "
> - "waiting for start condition to finish\n");
> + dev_err(omap->dev, "controller timed out waiting for start condition to finish\n");
> return -ETIMEDOUT;
> }
> cpu_relax();
> @@ -782,7 +779,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
> return -EIO;
> }
>
> -
> /*
> * Prepare controller for a transaction and call omap_i2c_xfer_msg
> * to do the work during IRQ processing.
> @@ -807,7 +803,7 @@ omap_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg msgs[], int num,
> if (r < 0)
> goto out;
>
> - if (omap->set_mpu_wkup_lat != NULL)
> + if (omap->set_mpu_wkup_lat)
> omap->set_mpu_wkup_lat(omap->dev, omap->latency);
>
> for (i = 0; i < num; i++) {
> @@ -822,7 +818,7 @@ omap_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg msgs[], int num,
>
> omap_i2c_wait_for_bb(omap);
>
> - if (omap->set_mpu_wkup_lat != NULL)
> + if (omap->set_mpu_wkup_lat)
> omap->set_mpu_wkup_lat(omap->dev, -1);
>
> out:
> @@ -875,18 +871,15 @@ static inline void i2c_omap_errata_i207(struct omap_i2c_dev *omap, u16 stat)
> if (stat & OMAP_I2C_STAT_RDR) {
> /* Step 1: If RDR is set, clear it */
> omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RDR);
> -
> /* Step 2: */
> if (!(omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG)
> & OMAP_I2C_STAT_BB)) {
> -
> /* Step 3: */
> if (omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG)
> & OMAP_I2C_STAT_RDR) {
> omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RDR);
> dev_dbg(omap->dev, "RDR when bus is busy.\n");
> }
> -
> }
> }
> }
> @@ -911,7 +904,7 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)
> dev_err(omap->dev, "Arbitration lost\n");
> omap_i2c_complete_cmd(omap, OMAP_I2C_STAT_AL);
> break;
> - case 0x02: /* No acknowledgement */
> + case 0x02: /* No acknowledgment */
> omap_i2c_complete_cmd(omap, OMAP_I2C_STAT_NACK);
> omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, OMAP_I2C_CON_STP);
> break;
> @@ -927,8 +920,9 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)
> *omap->buf++ = w >> 8;
> omap->buf_len--;
> }
> - } else
> + } else {
> dev_err(omap->dev, "RRDY IRQ while no data requested\n");
> + }
> break;
> case 0x05: /* Transmit data ready */
> if (omap->buf_len) {
> @@ -939,8 +933,9 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)
> omap->buf_len--;
> }
> omap_i2c_write_reg(omap, OMAP_I2C_DATA_REG, w);
> - } else
> + } else {
> dev_err(omap->dev, "XRDY IRQ while no data to send\n");
> + }
> break;
> default:
> return IRQ_NONE;
> @@ -995,8 +990,7 @@ static int errata_omap3_i462(struct omap_i2c_dev *omap)
> return 0;
> }
>
> -static void omap_i2c_receive_data(struct omap_i2c_dev *omap, u8 num_bytes,
> - bool is_rdr)
> +static void omap_i2c_receive_data(struct omap_i2c_dev *omap, u8 num_bytes)
> {
> u16 w;
>
> @@ -1016,8 +1010,7 @@ static void omap_i2c_receive_data(struct omap_i2c_dev *omap, u8 num_bytes,
> }
> }
>
> -static int omap_i2c_transmit_data(struct omap_i2c_dev *omap, u8 num_bytes,
> - bool is_xdr)
> +static int omap_i2c_transmit_data(struct omap_i2c_dev *omap, u8 num_bytes)
> {
> u16 w;
>
> @@ -1133,7 +1126,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
> OMAP_I2C_BUFSTAT_REG) >> 8) & 0x3F;
> }
>
> - omap_i2c_receive_data(omap, num_bytes, true);
> + omap_i2c_receive_data(omap, num_bytes);
> omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RDR);
> continue;
> }
> @@ -1144,7 +1137,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
> if (omap->threshold)
> num_bytes = omap->threshold;
>
> - omap_i2c_receive_data(omap, num_bytes, false);
> + omap_i2c_receive_data(omap, num_bytes);
> omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RRDY);
> continue;
> }
> @@ -1156,7 +1149,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
> if (omap->fifo_size)
> num_bytes = omap->buf_len;
>
> - ret = omap_i2c_transmit_data(omap, num_bytes, true);
> + ret = omap_i2c_transmit_data(omap, num_bytes);
> if (ret < 0)
> break;
>
> @@ -1171,7 +1164,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
> if (omap->threshold)
> num_bytes = omap->threshold;
>
> - ret = omap_i2c_transmit_data(omap, num_bytes, false);
> + ret = omap_i2c_transmit_data(omap, num_bytes);
> if (ret < 0)
> break;
>
> @@ -1266,13 +1259,13 @@ static const struct of_device_id omap_i2c_of_match[] = {
> MODULE_DEVICE_TABLE(of, omap_i2c_of_match);
> #endif
>
> -#define OMAP_I2C_SCHEME(rev) ((rev & 0xc000) >> 14)
> +#define OMAP_I2C_SCHEME(rev) (((rev) & 0xc000) >> 14)
>
> -#define OMAP_I2C_REV_SCHEME_0_MAJOR(rev) (rev >> 4)
> -#define OMAP_I2C_REV_SCHEME_0_MINOR(rev) (rev & 0xf)
> +#define OMAP_I2C_REV_SCHEME_0_MAJOR(rev) (((rev) >> 4))
> +#define OMAP_I2C_REV_SCHEME_0_MINOR(rev) (((rev) & 0xf))
>
> -#define OMAP_I2C_REV_SCHEME_1_MAJOR(rev) ((rev & 0x0700) >> 7)
> -#define OMAP_I2C_REV_SCHEME_1_MINOR(rev) (rev & 0x1f)
> +#define OMAP_I2C_REV_SCHEME_1_MAJOR(rev) (((rev) & 0x0700) >> 7)
> +#define OMAP_I2C_REV_SCHEME_1_MINOR(rev) (((rev) & 0x1f))
> #define OMAP_I2C_SCHEME_0 0
> #define OMAP_I2C_SCHEME_1 1
>
> @@ -1383,7 +1376,7 @@ omap_i2c_probe(struct platform_device *pdev)
> of_property_read_u32(node, "clock-frequency", &freq);
> /* convert DT freq value in Hz into kHz for speed */
> omap->speed = freq / 1000;
> - } else if (pdata != NULL) {
> + } else if (pdata) {
> omap->speed = pdata->clkrate;
> omap->flags = pdata->flags;
> omap->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat;
> @@ -1434,7 +1427,7 @@ omap_i2c_probe(struct platform_device *pdev)
> omap->errata = 0;
>
> if (omap->rev >= OMAP_I2C_REV_ON_2430 &&
> - omap->rev < OMAP_I2C_REV_ON_4430_PLUS)
> + omap->rev < OMAP_I2C_REV_ON_4430_PLUS)
> omap->errata |= I2C_OMAP_ERRATA_I207;
>
> if (omap->rev <= OMAP_I2C_REV_ON_3430_3530)
> @@ -1459,7 +1452,7 @@ omap_i2c_probe(struct platform_device *pdev)
> omap->b_hw = 1; /* Enable hardware fixes */
>
> /* calculate wakeup latency constraint for MPU */
> - if (omap->set_mpu_wkup_lat != NULL)
> + if (omap->set_mpu_wkup_lat)
> omap->latency = (1000000 * omap->fifo_size) /
> (1000 * omap->speed / 8);
> }
> @@ -1469,12 +1462,12 @@ omap_i2c_probe(struct platform_device *pdev)
>
> if (omap->rev < OMAP_I2C_OMAP1_REV_2)
> r = devm_request_irq(&pdev->dev, omap->irq, omap_i2c_omap1_isr,
> - IRQF_NO_SUSPEND, pdev->name, omap);
> + IRQF_NO_SUSPEND, pdev->name, omap);
> else
> r = devm_request_threaded_irq(&pdev->dev, omap->irq,
> - omap_i2c_isr, omap_i2c_isr_thread,
> - IRQF_NO_SUSPEND | IRQF_ONESHOT,
> - pdev->name, omap);
> + omap_i2c_isr, omap_i2c_isr_thread,
> + IRQF_NO_SUSPEND | IRQF_ONESHOT,
> + pdev->name, omap);
>
> if (r) {
> dev_err(omap->dev, "failure requesting irq %i\n", omap->irq);
> --
> 2.43.0
>
>
Am Mon, 2 Dec 2024 22:58:17 +0200 schrieb Aaro Koskinen <aaro.koskinen@iki.fi>: > On Tue, Dec 03, 2024 at 12:22:51AM +0530, Dhruv Menon wrote: > > This commit addresses the coding style issues present in i2c-omap.c, > > identified by checkpatch.pl and removes unused parameters present in > > two functions. > > > > 1. Coding style issues includes Macro Utilization, alignnment > > correction, updating ms_sleep() < 20 to usleep_range(). > > 2. Removed unused parameters from omap_i2c_receive_data() > > and omap_i2c_transmit_data(). > > > > No functional changes have been introduced in this commit. > > Not sure if that is correct as sleeps can be now shorter? I wouldn't > touch them unless you can show some real benefit (checkpatch.pl warning > isn't one for old driver code). > The sleeps are not shorter, instead possibly longer. I do not think that is an issue, AFAIK the idea with sleep range is to bundle wakeups and reduce power consumption. > Maybe also changes should be split into separate patches for easier > review. > I would leave out omap_i2c_*data() parameter stuff until the i2c irq regressions are fixed, maybe that parameter is needed. Regards, Andreas
Hi, On Tue, Dec 03, 2024 at 08:35:47AM +0100, Andreas Kemnade wrote: > The sleeps are not shorter, instead possibly longer. I do not think > that is an issue, AFAIK the idea with sleep range is to bundle wakeups > and reduce power consumption. msleep(1) is jiffie-based and can take up to 20 ms. If you replace it with more precise usleep then polling is more frequent (more wakeups) A.
> > Not sure if that is correct as sleeps can be now shorter? I wouldn't > > touch them unless you can show some real benefit (checkpatch.pl warning > > isn't one for old driver code). > > > The sleeps are not shorter, instead possibly longer. I do not think > that is an issue, AFAIK the idea with sleep range is to bundle wakeups > and reduce power consumption. > As per timer,how-to docs present, using ms_sleep(1) will lead longer sleep time (about 20ms in HZ=100) > > Maybe also changes should be split into separate patches for easier > > review. > > > I would leave out omap_i2c_*data() parameter stuff until the i2c irq > regressions are fixed, maybe that parameter is needed. > Using git blame, the only usage of it has been for error logging. On Tue, Dec 03, 2024 at 08:35:47AM +0100, Andreas Kemnade wrote: > Am Mon, 2 Dec 2024 22:58:17 +0200 > schrieb Aaro Koskinen <aaro.koskinen@iki.fi>: > > > On Tue, Dec 03, 2024 at 12:22:51AM +0530, Dhruv Menon wrote: > > > This commit addresses the coding style issues present in i2c-omap.c, > > > identified by checkpatch.pl and removes unused parameters present in > > > two functions. > > > > > > 1. Coding style issues includes Macro Utilization, alignnment > > > correction, updating ms_sleep() < 20 to usleep_range(). > > > 2. Removed unused parameters from omap_i2c_receive_data() > > > and omap_i2c_transmit_data(). > > > > > > No functional changes have been introduced in this commit. > > > > Not sure if that is correct as sleeps can be now shorter? I wouldn't > > touch them unless you can show some real benefit (checkpatch.pl warning > > isn't one for old driver code). > > > The sleeps are not shorter, instead possibly longer. I do not think > that is an issue, AFAIK the idea with sleep range is to bundle wakeups > and reduce power consumption. > > > Maybe also changes should be split into separate patches for easier > > review. > > > I would leave out omap_i2c_*data() parameter stuff until the i2c irq > regressions are fixed, maybe that parameter is needed. > > Regards, > Andreas
Hello Aaro,
I have updated the patch as requiested, splitting it two parts,
1. [PATCH v2 1/2] i2c: omap: Cleaned up coding style
2. [PATCH v2 2/2] i2c: omap: Removed unsed parameter
I have also removed the changes regarding msleep as the adjustment
was relatively minor. The change was initially considered based
on "Timer's howto" documentation which recommends to change any
msleep(x) < 10ms to usleep_range(x*1000, x*2000) for better
precision.
Thank you for the time and review. I look forward to your feedback
Regards
Dhruv Menon
On Mon, Dec 02, 2024 at 10:58:17PM +0200, Aaro Koskinen wrote:
> On Tue, Dec 03, 2024 at 12:22:51AM +0530, Dhruv Menon wrote:
> > This commit addresses the coding style issues present in i2c-omap.c,
> > identified by checkpatch.pl and removes unused parameters present in
> > two functions.
> >
> > 1. Coding style issues includes Macro Utilization, alignnment
> > correction, updating ms_sleep() < 20 to usleep_range().
> > 2. Removed unused parameters from omap_i2c_receive_data()
> > and omap_i2c_transmit_data().
> >
> > No functional changes have been introduced in this commit.
>
> Not sure if that is correct as sleeps can be now shorter? I wouldn't
> touch them unless you can show some real benefit (checkpatch.pl warning
> isn't one for old driver code).
>
> Maybe also changes should be split into separate patches for easier
> review.
>
> A.
>
> >
> > Signed-off-by: Dhruv Menon <dhruvmenon1104@gmail.com>
> > ---
> > drivers/i2c/busses/i2c-omap.c | 209 ++++++++++++++++------------------
> > 1 file changed, 101 insertions(+), 108 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> > index 1d9ad25c89ae..eee1671edebd 100644
> > --- a/drivers/i2c/busses/i2c-omap.c
> > +++ b/drivers/i2c/busses/i2c-omap.c
> > @@ -78,39 +78,39 @@ enum {
> > };
> >
> > /* I2C Interrupt Enable Register (OMAP_I2C_IE): */
> > -#define OMAP_I2C_IE_XDR (1 << 14) /* TX Buffer drain int enable */
> > -#define OMAP_I2C_IE_RDR (1 << 13) /* RX Buffer drain int enable */
> > -#define OMAP_I2C_IE_XRDY (1 << 4) /* TX data ready int enable */
> > -#define OMAP_I2C_IE_RRDY (1 << 3) /* RX data ready int enable */
> > -#define OMAP_I2C_IE_ARDY (1 << 2) /* Access ready int enable */
> > -#define OMAP_I2C_IE_NACK (1 << 1) /* No ack interrupt enable */
> > -#define OMAP_I2C_IE_AL (1 << 0) /* Arbitration lost int ena */
> > +#define OMAP_I2C_IE_XDR BIT(14) /* TX Buffer drain int enable */
> > +#define OMAP_I2C_IE_RDR BIT(13) /* RX Buffer drain int enable */
> > +#define OMAP_I2C_IE_XRDY BIT(4) /* TX data ready int enable */
> > +#define OMAP_I2C_IE_RRDY BIT(3) /* RX data ready int enable */
> > +#define OMAP_I2C_IE_ARDY BIT(2) /* Access ready int enable */
> > +#define OMAP_I2C_IE_NACK BIT(1) /* No ack interrupt enable */
> > +#define OMAP_I2C_IE_AL BIT(0) /* Arbitration lost int ena */
> >
> > /* I2C Status Register (OMAP_I2C_STAT): */
> > -#define OMAP_I2C_STAT_XDR (1 << 14) /* TX Buffer draining */
> > -#define OMAP_I2C_STAT_RDR (1 << 13) /* RX Buffer draining */
> > -#define OMAP_I2C_STAT_BB (1 << 12) /* Bus busy */
> > -#define OMAP_I2C_STAT_ROVR (1 << 11) /* Receive overrun */
> > -#define OMAP_I2C_STAT_XUDF (1 << 10) /* Transmit underflow */
> > -#define OMAP_I2C_STAT_AAS (1 << 9) /* Address as slave */
> > -#define OMAP_I2C_STAT_BF (1 << 8) /* Bus Free */
> > -#define OMAP_I2C_STAT_XRDY (1 << 4) /* Transmit data ready */
> > -#define OMAP_I2C_STAT_RRDY (1 << 3) /* Receive data ready */
> > -#define OMAP_I2C_STAT_ARDY (1 << 2) /* Register access ready */
> > -#define OMAP_I2C_STAT_NACK (1 << 1) /* No ack interrupt enable */
> > -#define OMAP_I2C_STAT_AL (1 << 0) /* Arbitration lost int ena */
> > +#define OMAP_I2C_STAT_XDR BIT(14) /* TX Buffer draining */
> > +#define OMAP_I2C_STAT_RDR BIT(13) /* RX Buffer draining */
> > +#define OMAP_I2C_STAT_BB BIT(12) /* Bus busy */
> > +#define OMAP_I2C_STAT_ROVR BIT(11) /* Receive overrun */
> > +#define OMAP_I2C_STAT_XUDF BIT(10) /* Transmit underflow */
> > +#define OMAP_I2C_STAT_AAS BIT(9) /* Address as slave */
> > +#define OMAP_I2C_STAT_BF BIT(8) /* Bus Free */
> > +#define OMAP_I2C_STAT_XRDY BIT(4) /* Transmit data ready */
> > +#define OMAP_I2C_STAT_RRDY BIT(3) /* Receive data ready */
> > +#define OMAP_I2C_STAT_ARDY BIT(2) /* Register access ready */
> > +#define OMAP_I2C_STAT_NACK BIT(1) /* No ack interrupt enable */
> > +#define OMAP_I2C_STAT_AL BIT(0) /* Arbitration lost int ena */
> >
> > /* I2C WE wakeup enable register */
> > -#define OMAP_I2C_WE_XDR_WE (1 << 14) /* TX drain wakup */
> > -#define OMAP_I2C_WE_RDR_WE (1 << 13) /* RX drain wakeup */
> > -#define OMAP_I2C_WE_AAS_WE (1 << 9) /* Address as slave wakeup*/
> > -#define OMAP_I2C_WE_BF_WE (1 << 8) /* Bus free wakeup */
> > -#define OMAP_I2C_WE_STC_WE (1 << 6) /* Start condition wakeup */
> > -#define OMAP_I2C_WE_GC_WE (1 << 5) /* General call wakeup */
> > -#define OMAP_I2C_WE_DRDY_WE (1 << 3) /* TX/RX data ready wakeup */
> > -#define OMAP_I2C_WE_ARDY_WE (1 << 2) /* Reg access ready wakeup */
> > -#define OMAP_I2C_WE_NACK_WE (1 << 1) /* No acknowledgment wakeup */
> > -#define OMAP_I2C_WE_AL_WE (1 << 0) /* Arbitration lost wakeup */
> > +#define OMAP_I2C_WE_XDR_WE BIT(14) /* TX drain wakup */
> > +#define OMAP_I2C_WE_RDR_WE BIT(13) /* RX drain wakeup */
> > +#define OMAP_I2C_WE_AAS_WE BIT(9) /* Address as slave wakeup*/
> > +#define OMAP_I2C_WE_BF_WE BIT(8) /* Bus free wakeup */
> > +#define OMAP_I2C_WE_STC_WE BIT(6) /* Start condition wakeup */
> > +#define OMAP_I2C_WE_GC_WE BIT(5) /* General call wakeup */
> > +#define OMAP_I2C_WE_DRDY_WE BIT(3) /* TX/RX data ready wakeup */
> > +#define OMAP_I2C_WE_ARDY_WE BIT(2) /* Reg access ready wakeup */
> > +#define OMAP_I2C_WE_NACK_WE BIT(1) /* No acknowledgment wakeup */
> > +#define OMAP_I2C_WE_AL_WE BIT(0) /* Arbitration lost wakeup */
> >
> > #define OMAP_I2C_WE_ALL (OMAP_I2C_WE_XDR_WE | OMAP_I2C_WE_RDR_WE | \
> > OMAP_I2C_WE_AAS_WE | OMAP_I2C_WE_BF_WE | \
> > @@ -119,59 +119,59 @@ enum {
> > OMAP_I2C_WE_NACK_WE | OMAP_I2C_WE_AL_WE)
> >
> > /* I2C Buffer Configuration Register (OMAP_I2C_BUF): */
> > -#define OMAP_I2C_BUF_RDMA_EN (1 << 15) /* RX DMA channel enable */
> > -#define OMAP_I2C_BUF_RXFIF_CLR (1 << 14) /* RX FIFO Clear */
> > -#define OMAP_I2C_BUF_XDMA_EN (1 << 7) /* TX DMA channel enable */
> > -#define OMAP_I2C_BUF_TXFIF_CLR (1 << 6) /* TX FIFO Clear */
> > +#define OMAP_I2C_BUF_RDMA_EN BIT(15) /* RX DMA channel enable */
> > +#define OMAP_I2C_BUF_RXFIF_CLR BIT(14) /* RX FIFO Clear */
> > +#define OMAP_I2C_BUF_XDMA_EN BIT(7) /* TX DMA channel enable */
> > +#define OMAP_I2C_BUF_TXFIF_CLR BIT(6) /* TX FIFO Clear */
> >
> > /* I2C Configuration Register (OMAP_I2C_CON): */
> > -#define OMAP_I2C_CON_EN (1 << 15) /* I2C module enable */
> > -#define OMAP_I2C_CON_BE (1 << 14) /* Big endian mode */
> > -#define OMAP_I2C_CON_OPMODE_HS (1 << 12) /* High Speed support */
> > -#define OMAP_I2C_CON_STB (1 << 11) /* Start byte mode (master) */
> > -#define OMAP_I2C_CON_MST (1 << 10) /* Master/slave mode */
> > -#define OMAP_I2C_CON_TRX (1 << 9) /* TX/RX mode (master only) */
> > -#define OMAP_I2C_CON_XA (1 << 8) /* Expand address */
> > -#define OMAP_I2C_CON_RM (1 << 2) /* Repeat mode (master only) */
> > -#define OMAP_I2C_CON_STP (1 << 1) /* Stop cond (master only) */
> > -#define OMAP_I2C_CON_STT (1 << 0) /* Start condition (master) */
> > +#define OMAP_I2C_CON_EN BIT(15) /* I2C module enable */
> > +#define OMAP_I2C_CON_BE BIT(14) /* Big endian mode */
> > +#define OMAP_I2C_CON_OPMODE_HS BIT(12) /* High Speed support */
> > +#define OMAP_I2C_CON_STB BIT(11) /* Start byte mode (master) */
> > +#define OMAP_I2C_CON_MST BIT(10) /* Master/slave mode */
> > +#define OMAP_I2C_CON_TRX BIT(9) /* TX/RX mode (master only) */
> > +#define OMAP_I2C_CON_XA BIT(8) /* Expand address */
> > +#define OMAP_I2C_CON_RM BIT(2) /* Repeat mode (master only) */
> > +#define OMAP_I2C_CON_STP BIT(1) /* Stop cond (master only) */
> > +#define OMAP_I2C_CON_STT BIT(0) /* Start condition (master) */
> >
> > /* I2C SCL time value when Master */
> > #define OMAP_I2C_SCLL_HSSCLL 8
> > #define OMAP_I2C_SCLH_HSSCLH 8
> >
> > /* I2C System Test Register (OMAP_I2C_SYSTEST): */
> > -#define OMAP_I2C_SYSTEST_ST_EN (1 << 15) /* System test enable */
> > -#define OMAP_I2C_SYSTEST_FREE (1 << 14) /* Free running mode */
> > -#define OMAP_I2C_SYSTEST_TMODE_MASK (3 << 12) /* Test mode select */
> > -#define OMAP_I2C_SYSTEST_TMODE_SHIFT (12) /* Test mode select */
> > +#define OMAP_I2C_SYSTEST_ST_EN BIT(15) /* System test enable */
> > +#define OMAP_I2C_SYSTEST_FREE BIT(14) /* Free running mode */
> > +#define OMAP_I2C_SYSTEST_TMODE_MASK GENMASK(13, 12) /* Test mode select */
> > +#define OMAP_I2C_SYSTEST_TMODE_SHIFT (12) /* Test mode select */
> > /* Functional mode */
> > -#define OMAP_I2C_SYSTEST_SCL_I_FUNC (1 << 8) /* SCL line input value */
> > -#define OMAP_I2C_SYSTEST_SCL_O_FUNC (1 << 7) /* SCL line output value */
> > -#define OMAP_I2C_SYSTEST_SDA_I_FUNC (1 << 6) /* SDA line input value */
> > -#define OMAP_I2C_SYSTEST_SDA_O_FUNC (1 << 5) /* SDA line output value */
> > +#define OMAP_I2C_SYSTEST_SCL_I_FUNC BIT(8) /* SCL line input value */
> > +#define OMAP_I2C_SYSTEST_SCL_O_FUNC BIT(7) /* SCL line output value */
> > +#define OMAP_I2C_SYSTEST_SDA_I_FUNC BIT(6) /* SDA line input value */
> > +#define OMAP_I2C_SYSTEST_SDA_O_FUNC BIT(5) /* SDA line output value */
> > /* SDA/SCL IO mode */
> > -#define OMAP_I2C_SYSTEST_SCL_I (1 << 3) /* SCL line sense in */
> > -#define OMAP_I2C_SYSTEST_SCL_O (1 << 2) /* SCL line drive out */
> > -#define OMAP_I2C_SYSTEST_SDA_I (1 << 1) /* SDA line sense in */
> > -#define OMAP_I2C_SYSTEST_SDA_O (1 << 0) /* SDA line drive out */
> > +#define OMAP_I2C_SYSTEST_SCL_I BIT(3) /* SCL line sense in */
> > +#define OMAP_I2C_SYSTEST_SCL_O BIT(2) /* SCL line drive out */
> > +#define OMAP_I2C_SYSTEST_SDA_I BIT(1) /* SDA line sense in */
> > +#define OMAP_I2C_SYSTEST_SDA_O BIT(0) /* SDA line drive out */
> >
> > /* OCP_SYSSTATUS bit definitions */
> > -#define SYSS_RESETDONE_MASK (1 << 0)
> > +#define SYSS_RESETDONE_MASK BIT(0)
> >
> > /* OCP_SYSCONFIG bit definitions */
> > -#define SYSC_CLOCKACTIVITY_MASK (0x3 << 8)
> > -#define SYSC_SIDLEMODE_MASK (0x3 << 3)
> > -#define SYSC_ENAWAKEUP_MASK (1 << 2)
> > -#define SYSC_SOFTRESET_MASK (1 << 1)
> > -#define SYSC_AUTOIDLE_MASK (1 << 0)
> > +#define SYSC_CLOCKACTIVITY_MASK GENMASK(9, 8)
> > +#define SYSC_SIDLEMODE_MASK GENMASK(4, 3)
> > +#define SYSC_ENAWAKEUP_MASK BIT(2)
> > +#define SYSC_SOFTRESET_MASK BIT(1)
> > +#define SYSC_AUTOIDLE_MASK BIT(0)
> >
> > #define SYSC_IDLEMODE_SMART 0x2
> > #define SYSC_CLOCKACTIVITY_FCLK 0x2
> >
> > /* Errata definitions */
> > -#define I2C_OMAP_ERRATA_I207 (1 << 0)
> > -#define I2C_OMAP_ERRATA_I462 (1 << 1)
> > +#define I2C_OMAP_ERRATA_I207 BIT(0)
> > +#define I2C_OMAP_ERRATA_I462 BIT(1)
> >
> > #define OMAP_I2C_IP_V2_INTERRUPTS_MASK 0x6FFF
> >
> > @@ -277,7 +277,6 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev *omap, int reg)
> >
> > static void __omap_i2c_init(struct omap_i2c_dev *omap)
> > {
> > -
> > omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, 0);
> >
> > /* Setup clock prescaler to obtain approx 12MHz I2C module clock: */
> > @@ -316,22 +315,22 @@ static int omap_i2c_reset(struct omap_i2c_dev *omap)
> >
> > /* Disable I2C controller before soft reset */
> > omap_i2c_write_reg(omap, OMAP_I2C_CON_REG,
> > - omap_i2c_read_reg(omap, OMAP_I2C_CON_REG) &
> > - ~(OMAP_I2C_CON_EN));
> > + omap_i2c_read_reg(omap, OMAP_I2C_CON_REG) &
> > + ~(OMAP_I2C_CON_EN));
> >
> > omap_i2c_write_reg(omap, OMAP_I2C_SYSC_REG, SYSC_SOFTRESET_MASK);
> > /* For some reason we need to set the EN bit before the
> > - * reset done bit gets set. */
> > + * reset done bit gets set.
> > + */
> > timeout = jiffies + OMAP_I2C_TIMEOUT;
> > omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
> > while (!(omap_i2c_read_reg(omap, OMAP_I2C_SYSS_REG) &
> > SYSS_RESETDONE_MASK)) {
> > if (time_after(jiffies, timeout)) {
> > - dev_warn(omap->dev, "timeout waiting "
> > - "for controller reset\n");
> > + dev_warn(omap->dev, "timeout waiting for controller reset\n");
> > return -ETIMEDOUT;
> > }
> > - msleep(1);
> > + usleep_range(1000, 2000);
> > }
> >
> > /* SYSC register is cleared by the reset; rewrite it */
> > @@ -396,15 +395,13 @@ static int omap_i2c_init(struct omap_i2c_dev *omap)
> > }
> >
> > if (!(omap->flags & OMAP_I2C_FLAG_SIMPLE_CLOCK)) {
> > -
> > /*
> > * HSI2C controller internal clk rate should be 19.2 Mhz for
> > * HS and for all modes on 2430. On 34xx we can use lower rate
> > * to get longer filter period for better noise suppression.
> > * The filter is iclk (fclk for HS) period.
> > */
> > - if (omap->speed > 400 ||
> > - omap->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK)
> > + if (omap->speed > 400 || omap->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK)
> > internal_clk = 19200;
> > else if (omap->speed > 100)
> > internal_clk = 9600;
> > @@ -506,7 +503,7 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *omap)
> > while (omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG) & OMAP_I2C_STAT_BB) {
> > if (time_after(jiffies, timeout))
> > return omap_i2c_recover_bus(omap);
> > - msleep(1);
> > + usleep_range(1000, 2000);
> > }
> >
> > return 0;
> > @@ -595,7 +592,7 @@ static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *omap)
> > return omap_i2c_recover_bus(omap);
> > }
> >
> > - msleep(1);
> > + usleep_range(1000, 2000);
> > }
> >
> > omap->bb_valid = 1;
> > @@ -616,7 +613,7 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *omap, u8 size, bool is_rx)
> > * then we might use draining feature to transfer the remaining bytes.
> > */
> >
> > - omap->threshold = clamp(size, (u8) 1, omap->fifo_size);
> > + omap->threshold = clamp(size, (u8)1, omap->fifo_size);
> >
> > buf = omap_i2c_read_reg(omap, OMAP_I2C_BUF_REG);
> >
> > @@ -636,7 +633,7 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *omap, u8 size, bool is_rx)
> > omap->b_hw = 1; /* Enable hardware fixes */
> >
> > /* calculate wakeup latency constraint for MPU */
> > - if (omap->set_mpu_wkup_lat != NULL)
> > + if (omap->set_mpu_wkup_lat)
> > omap->latency = (1000000 * omap->threshold) /
> > (1000 * omap->speed / 8);
> > }
> > @@ -718,13 +715,13 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
> > if (omap->b_hw && stop) {
> > unsigned long delay = jiffies + OMAP_I2C_TIMEOUT;
> > u16 con = omap_i2c_read_reg(omap, OMAP_I2C_CON_REG);
> > +
> > while (con & OMAP_I2C_CON_STT) {
> > con = omap_i2c_read_reg(omap, OMAP_I2C_CON_REG);
> >
> > /* Let the user know if i2c is in a bad state */
> > if (time_after(jiffies, delay)) {
> > - dev_err(omap->dev, "controller timed out "
> > - "waiting for start condition to finish\n");
> > + dev_err(omap->dev, "controller timed out waiting for start condition to finish\n");
> > return -ETIMEDOUT;
> > }
> > cpu_relax();
> > @@ -782,7 +779,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
> > return -EIO;
> > }
> >
> > -
> > /*
> > * Prepare controller for a transaction and call omap_i2c_xfer_msg
> > * to do the work during IRQ processing.
> > @@ -807,7 +803,7 @@ omap_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg msgs[], int num,
> > if (r < 0)
> > goto out;
> >
> > - if (omap->set_mpu_wkup_lat != NULL)
> > + if (omap->set_mpu_wkup_lat)
> > omap->set_mpu_wkup_lat(omap->dev, omap->latency);
> >
> > for (i = 0; i < num; i++) {
> > @@ -822,7 +818,7 @@ omap_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg msgs[], int num,
> >
> > omap_i2c_wait_for_bb(omap);
> >
> > - if (omap->set_mpu_wkup_lat != NULL)
> > + if (omap->set_mpu_wkup_lat)
> > omap->set_mpu_wkup_lat(omap->dev, -1);
> >
> > out:
> > @@ -875,18 +871,15 @@ static inline void i2c_omap_errata_i207(struct omap_i2c_dev *omap, u16 stat)
> > if (stat & OMAP_I2C_STAT_RDR) {
> > /* Step 1: If RDR is set, clear it */
> > omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RDR);
> > -
> > /* Step 2: */
> > if (!(omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG)
> > & OMAP_I2C_STAT_BB)) {
> > -
> > /* Step 3: */
> > if (omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG)
> > & OMAP_I2C_STAT_RDR) {
> > omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RDR);
> > dev_dbg(omap->dev, "RDR when bus is busy.\n");
> > }
> > -
> > }
> > }
> > }
> > @@ -911,7 +904,7 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)
> > dev_err(omap->dev, "Arbitration lost\n");
> > omap_i2c_complete_cmd(omap, OMAP_I2C_STAT_AL);
> > break;
> > - case 0x02: /* No acknowledgement */
> > + case 0x02: /* No acknowledgment */
> > omap_i2c_complete_cmd(omap, OMAP_I2C_STAT_NACK);
> > omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, OMAP_I2C_CON_STP);
> > break;
> > @@ -927,8 +920,9 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)
> > *omap->buf++ = w >> 8;
> > omap->buf_len--;
> > }
> > - } else
> > + } else {
> > dev_err(omap->dev, "RRDY IRQ while no data requested\n");
> > + }
> > break;
> > case 0x05: /* Transmit data ready */
> > if (omap->buf_len) {
> > @@ -939,8 +933,9 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)
> > omap->buf_len--;
> > }
> > omap_i2c_write_reg(omap, OMAP_I2C_DATA_REG, w);
> > - } else
> > + } else {
> > dev_err(omap->dev, "XRDY IRQ while no data to send\n");
> > + }
> > break;
> > default:
> > return IRQ_NONE;
> > @@ -995,8 +990,7 @@ static int errata_omap3_i462(struct omap_i2c_dev *omap)
> > return 0;
> > }
> >
> > -static void omap_i2c_receive_data(struct omap_i2c_dev *omap, u8 num_bytes,
> > - bool is_rdr)
> > +static void omap_i2c_receive_data(struct omap_i2c_dev *omap, u8 num_bytes)
> > {
> > u16 w;
> >
> > @@ -1016,8 +1010,7 @@ static void omap_i2c_receive_data(struct omap_i2c_dev *omap, u8 num_bytes,
> > }
> > }
> >
> > -static int omap_i2c_transmit_data(struct omap_i2c_dev *omap, u8 num_bytes,
> > - bool is_xdr)
> > +static int omap_i2c_transmit_data(struct omap_i2c_dev *omap, u8 num_bytes)
> > {
> > u16 w;
> >
> > @@ -1133,7 +1126,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
> > OMAP_I2C_BUFSTAT_REG) >> 8) & 0x3F;
> > }
> >
> > - omap_i2c_receive_data(omap, num_bytes, true);
> > + omap_i2c_receive_data(omap, num_bytes);
> > omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RDR);
> > continue;
> > }
> > @@ -1144,7 +1137,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
> > if (omap->threshold)
> > num_bytes = omap->threshold;
> >
> > - omap_i2c_receive_data(omap, num_bytes, false);
> > + omap_i2c_receive_data(omap, num_bytes);
> > omap_i2c_ack_stat(omap, OMAP_I2C_STAT_RRDY);
> > continue;
> > }
> > @@ -1156,7 +1149,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
> > if (omap->fifo_size)
> > num_bytes = omap->buf_len;
> >
> > - ret = omap_i2c_transmit_data(omap, num_bytes, true);
> > + ret = omap_i2c_transmit_data(omap, num_bytes);
> > if (ret < 0)
> > break;
> >
> > @@ -1171,7 +1164,7 @@ static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
> > if (omap->threshold)
> > num_bytes = omap->threshold;
> >
> > - ret = omap_i2c_transmit_data(omap, num_bytes, false);
> > + ret = omap_i2c_transmit_data(omap, num_bytes);
> > if (ret < 0)
> > break;
> >
> > @@ -1266,13 +1259,13 @@ static const struct of_device_id omap_i2c_of_match[] = {
> > MODULE_DEVICE_TABLE(of, omap_i2c_of_match);
> > #endif
> >
> > -#define OMAP_I2C_SCHEME(rev) ((rev & 0xc000) >> 14)
> > +#define OMAP_I2C_SCHEME(rev) (((rev) & 0xc000) >> 14)
> >
> > -#define OMAP_I2C_REV_SCHEME_0_MAJOR(rev) (rev >> 4)
> > -#define OMAP_I2C_REV_SCHEME_0_MINOR(rev) (rev & 0xf)
> > +#define OMAP_I2C_REV_SCHEME_0_MAJOR(rev) (((rev) >> 4))
> > +#define OMAP_I2C_REV_SCHEME_0_MINOR(rev) (((rev) & 0xf))
> >
> > -#define OMAP_I2C_REV_SCHEME_1_MAJOR(rev) ((rev & 0x0700) >> 7)
> > -#define OMAP_I2C_REV_SCHEME_1_MINOR(rev) (rev & 0x1f)
> > +#define OMAP_I2C_REV_SCHEME_1_MAJOR(rev) (((rev) & 0x0700) >> 7)
> > +#define OMAP_I2C_REV_SCHEME_1_MINOR(rev) (((rev) & 0x1f))
> > #define OMAP_I2C_SCHEME_0 0
> > #define OMAP_I2C_SCHEME_1 1
> >
> > @@ -1383,7 +1376,7 @@ omap_i2c_probe(struct platform_device *pdev)
> > of_property_read_u32(node, "clock-frequency", &freq);
> > /* convert DT freq value in Hz into kHz for speed */
> > omap->speed = freq / 1000;
> > - } else if (pdata != NULL) {
> > + } else if (pdata) {
> > omap->speed = pdata->clkrate;
> > omap->flags = pdata->flags;
> > omap->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat;
> > @@ -1434,7 +1427,7 @@ omap_i2c_probe(struct platform_device *pdev)
> > omap->errata = 0;
> >
> > if (omap->rev >= OMAP_I2C_REV_ON_2430 &&
> > - omap->rev < OMAP_I2C_REV_ON_4430_PLUS)
> > + omap->rev < OMAP_I2C_REV_ON_4430_PLUS)
> > omap->errata |= I2C_OMAP_ERRATA_I207;
> >
> > if (omap->rev <= OMAP_I2C_REV_ON_3430_3530)
> > @@ -1459,7 +1452,7 @@ omap_i2c_probe(struct platform_device *pdev)
> > omap->b_hw = 1; /* Enable hardware fixes */
> >
> > /* calculate wakeup latency constraint for MPU */
> > - if (omap->set_mpu_wkup_lat != NULL)
> > + if (omap->set_mpu_wkup_lat)
> > omap->latency = (1000000 * omap->fifo_size) /
> > (1000 * omap->speed / 8);
> > }
> > @@ -1469,12 +1462,12 @@ omap_i2c_probe(struct platform_device *pdev)
> >
> > if (omap->rev < OMAP_I2C_OMAP1_REV_2)
> > r = devm_request_irq(&pdev->dev, omap->irq, omap_i2c_omap1_isr,
> > - IRQF_NO_SUSPEND, pdev->name, omap);
> > + IRQF_NO_SUSPEND, pdev->name, omap);
> > else
> > r = devm_request_threaded_irq(&pdev->dev, omap->irq,
> > - omap_i2c_isr, omap_i2c_isr_thread,
> > - IRQF_NO_SUSPEND | IRQF_ONESHOT,
> > - pdev->name, omap);
> > + omap_i2c_isr, omap_i2c_isr_thread,
> > + IRQF_NO_SUSPEND | IRQF_ONESHOT,
> > + pdev->name, omap);
> >
> > if (r) {
> > dev_err(omap->dev, "failure requesting irq %i\n", omap->irq);
> > --
> > 2.43.0
> >
> >
Just a minor nit: > Am 03.12.2024 um 08:31 schrieb Dhruv Menon <dhruvmenon1104@gmail.com>: > > Hello Aaro, > > I have updated the patch as requiested, splitting it two parts, > > 1. [PATCH v2 1/2] i2c: omap: Cleaned up coding style > 2. [PATCH v2 2/2] i2c: omap: Removed unsed parameter use "this patch will do" point of view, not "I have done" => Cleaned -> Clean And there is a typo in "unsed". > > I have also removed the changes regarding msleep as the adjustment > was relatively minor. The change was initially considered based > on "Timer's howto" documentation which recommends to change any > msleep(x) < 10ms to usleep_range(x*1000, x*2000) for better > precision. > > Thank you for the time and review. I look forward to your feedback > > Regards > Dhruv Menon >
This patch has been splitted into two parts,
1. [PATCH v2 1/2] i2c: omap: Clean up coding style
2. [PATCH v2 2/2] i2c: omap: Removed unused parameter
The patchset also removed the changes regarding msleep as the
adjustment was relatively minor which was added earlier. The
change was initially considered based on "Timer's howto"
documentation which recommends to change any msleep(x) < 10ms
to usleep_range(x*1000, x*2000) for better precision.
Thank you for the time and review. I look forward to your feedback
Regards
Dhruv Menon
On Tue, Dec 03, 2024 at 09:25:43AM +0100, H. Nikolaus Schaller wrote:
> Just a minor nit:
>
> > Am 03.12.2024 um 08:31 schrieb Dhruv Menon <dhruvmenon1104@gmail.com>:
> >
> > Hello Aaro,
> >
> > I have updated the patch as requiested, splitting it two parts,
> >
> > 1. [PATCH v2 1/2] i2c: omap: Cleaned up coding style
> > 2. [PATCH v2 2/2] i2c: omap: Removed unsed parameter
>
> use "this patch will do" point of view, not "I have done"
>
> => Cleaned -> Clean
>
> And there is a typo in "unsed".
>
> >
> > I have also removed the changes regarding msleep as the adjustment
> > was relatively minor. The change was initially considered based
> > on "Timer's howto" documentation which recommends to change any
> > msleep(x) < 10ms to usleep_range(x*1000, x*2000) for better
> > precision.
> >
> > Thank you for the time and review. I look forward to your feedback
> >
> > Regards
> > Dhruv Menon
> >
>
Am Tue, 3 Dec 2024 15:02:30 +0530 schrieb Dhruv Menon <dhruvmenon1104@gmail.com>: > This patch has been splitted into two parts, > > 1. [PATCH v2 1/2] i2c: omap: Clean up coding style > 2. [PATCH v2 2/2] i2c: omap: Removed unused parameter > > The patchset also removed the changes regarding msleep as the > adjustment was relatively minor which was added earlier. The > change was initially considered based on "Timer's howto" > documentation which recommends to change any msleep(x) < 10ms > to usleep_range(x*1000, x*2000) for better precision. > > Thank you for the time and review. I look forward to your feedback > send the output inline, see Documentation/process/submitting-patches.rst: No MIME, no links, no compression, no attachments. Just plain text Also we want imperative mood in the patches. There is also a lot of automated processing of these patch emails. Regards, Andreas
© 2016 - 2026 Red Hat, Inc.