This series first perform enhancements in the way interrupt are handled
and cleanup in messages.
Then it adds support for the stm32mp25 which differs in that
it only has a single irq line for both event/error and has a
different handling of the FastModePlus.
Support is then enabled within the stm32mp25 related device-trees.
Changelog:
v3: - addition of 2 commits dealing with readl_relaxed(I2C_ISR) in
isr handler and a second one to use dev_err_probe during probe
- correction of SOB in commit
v2: - correct st,stm32-i2c.yaml. Use if then else scheme to indicate
number of interrupts / interrupt-names depending on the
compatible while keeping the description within the common part
- correct 2 maybe-uninitialized warnings
* ret in stm32f7_i2c_write_fm_plus_bits
* irq_error in stm32f7_i2c_probe, move the platform_get_irq
within the same if block as devm_request_threaded_irq
Alain Volmat (9):
i2c: stm32f7: use dev_err_probe upon calls of devm_request_irq
i2c: stm32f7: perform most of irq job in threaded handler
i2c: stm32f7: simplify status messages in case of errors
dt-bindings: i2c: document st,stm32mp25-i2c compatible
i2c: stm32f7: perform I2C_ISR read once at beginning of event isr
i2c: stm32f7: add support for stm32mp25 soc
arm64: dts: st: add all 8 i2c nodes on stm32mp251
arm64: dts: st: add i2c2/i2c8 pins for stm32mp25
arm64: dts: st: add i2c2 / i2c8 properties on stm32mp257f-ev1
.../devicetree/bindings/i2c/st,stm32-i2c.yaml | 28 ++
arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi | 36 ++
arch/arm64/boot/dts/st/stm32mp251.dtsi | 96 +++++
arch/arm64/boot/dts/st/stm32mp257f-ev1.dts | 20 +
drivers/i2c/busses/i2c-stm32f7.c | 342 +++++++++---------
5 files changed, 358 insertions(+), 164 deletions(-)
--
2.25.1
Convert error handling upon calls of devm_request_irq functions during
the probe of the driver.
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/i2c/busses/i2c-stm32f7.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 983509936727..d99f152ac727 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -2199,19 +2199,13 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
stm32f7_i2c_isr_event_thread,
IRQF_ONESHOT,
pdev->name, i2c_dev);
- if (ret) {
- dev_err(&pdev->dev, "Failed to request irq event %i\n",
- irq_event);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "Failed to request irq event\n");
ret = devm_request_irq(&pdev->dev, irq_error, stm32f7_i2c_isr_error, 0,
pdev->name, i2c_dev);
- if (ret) {
- dev_err(&pdev->dev, "Failed to request irq error %i\n",
- irq_error);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "Failed to request irq error\n");
setup = of_device_get_match_data(&pdev->dev);
if (!setup) {
--
2.25.1
The irq handling is currently split between the irq handler
and the threaded irq handler. Some of the handling (such as
dma related stuffs) done within the irq handler might sleep or
take some time leading to issues if the kernel is built with
realtime constraints. In order to fix that, perform an overall
rework to perform most of the job within the threaded handler
and only keep fifo access in the non threaded handler.
Signed-off-by: Alain Volmat <[email protected]>
Reviewed-by: Andi Shyti <[email protected]>
---
drivers/i2c/busses/i2c-stm32f7.c | 126 ++++++++++++++-----------------
1 file changed, 56 insertions(+), 70 deletions(-)
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index d99f152ac727..dde70e7ec722 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1497,17 +1497,11 @@ static irqreturn_t stm32f7_i2c_slave_isr_event(struct stm32f7_i2c_dev *i2c_dev)
static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
{
struct stm32f7_i2c_dev *i2c_dev = data;
- struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
- struct stm32_i2c_dma *dma = i2c_dev->dma;
- void __iomem *base = i2c_dev->base;
- u32 status, mask;
- int ret = IRQ_HANDLED;
+ u32 status;
- /* Check if the interrupt if for a slave device */
- if (!i2c_dev->master_mode) {
- ret = stm32f7_i2c_slave_isr_event(i2c_dev);
- return ret;
- }
+ /* Check if the interrupt is for a slave device */
+ if (!i2c_dev->master_mode)
+ return IRQ_WAKE_THREAD;
status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
@@ -1519,6 +1513,29 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
if (status & STM32F7_I2C_ISR_RXNE)
stm32f7_i2c_read_rx_data(i2c_dev);
+ /* Wake up the thread if other flags are raised */
+ if (status &
+ (STM32F7_I2C_ISR_NACKF | STM32F7_I2C_ISR_STOPF |
+ STM32F7_I2C_ISR_TC | STM32F7_I2C_ISR_TCR))
+ return IRQ_WAKE_THREAD;
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t stm32f7_i2c_isr_event_thread(int irq, void *data)
+{
+ struct stm32f7_i2c_dev *i2c_dev = data;
+ struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+ struct stm32_i2c_dma *dma = i2c_dev->dma;
+ void __iomem *base = i2c_dev->base;
+ u32 status, mask;
+ int ret;
+
+ if (!i2c_dev->master_mode)
+ return stm32f7_i2c_slave_isr_event(i2c_dev);
+
+ status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
+
/* NACK received */
if (status & STM32F7_I2C_ISR_NACKF) {
dev_dbg(i2c_dev->dev, "<%s>: Receive NACK (addr %x)\n",
@@ -1531,33 +1548,28 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
f7_msg->result = -ENXIO;
}
- /* STOP detection flag */
- if (status & STM32F7_I2C_ISR_STOPF) {
- /* Disable interrupts */
- if (stm32f7_i2c_is_slave_registered(i2c_dev))
- mask = STM32F7_I2C_XFER_IRQ_MASK;
+ if (status & STM32F7_I2C_ISR_TCR) {
+ if (f7_msg->smbus)
+ stm32f7_i2c_smbus_reload(i2c_dev);
else
- mask = STM32F7_I2C_ALL_IRQ_MASK;
- stm32f7_i2c_disable_irq(i2c_dev, mask);
-
- /* Clear STOP flag */
- writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
-
- if (i2c_dev->use_dma && !f7_msg->result) {
- ret = IRQ_WAKE_THREAD;
- } else {
- i2c_dev->master_mode = false;
- complete(&i2c_dev->complete);
- }
+ stm32f7_i2c_reload(i2c_dev);
}
/* Transfer complete */
if (status & STM32F7_I2C_ISR_TC) {
+ /* Wait for dma transfer completion before sending next message */
+ if (i2c_dev->use_dma && !f7_msg->result) {
+ ret = wait_for_completion_timeout(&i2c_dev->dma->dma_complete, HZ);
+ if (!ret) {
+ dev_dbg(i2c_dev->dev, "<%s>: Timed out\n", __func__);
+ stm32f7_i2c_disable_dma_req(i2c_dev);
+ dmaengine_terminate_async(dma->chan_using);
+ f7_msg->result = -ETIMEDOUT;
+ }
+ }
if (f7_msg->stop) {
mask = STM32F7_I2C_CR2_STOP;
stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
- } else if (i2c_dev->use_dma && !f7_msg->result) {
- ret = IRQ_WAKE_THREAD;
} else if (f7_msg->smbus) {
stm32f7_i2c_smbus_rep_start(i2c_dev);
} else {
@@ -1567,47 +1579,18 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
}
}
- if (status & STM32F7_I2C_ISR_TCR) {
- if (f7_msg->smbus)
- stm32f7_i2c_smbus_reload(i2c_dev);
+ /* STOP detection flag */
+ if (status & STM32F7_I2C_ISR_STOPF) {
+ /* Disable interrupts */
+ if (stm32f7_i2c_is_slave_registered(i2c_dev))
+ mask = STM32F7_I2C_XFER_IRQ_MASK;
else
- stm32f7_i2c_reload(i2c_dev);
- }
-
- return ret;
-}
-
-static irqreturn_t stm32f7_i2c_isr_event_thread(int irq, void *data)
-{
- struct stm32f7_i2c_dev *i2c_dev = data;
- struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
- struct stm32_i2c_dma *dma = i2c_dev->dma;
- u32 status;
- int ret;
-
- /*
- * Wait for dma transfer completion before sending next message or
- * notity the end of xfer to the client
- */
- ret = wait_for_completion_timeout(&i2c_dev->dma->dma_complete, HZ);
- if (!ret) {
- dev_dbg(i2c_dev->dev, "<%s>: Timed out\n", __func__);
- stm32f7_i2c_disable_dma_req(i2c_dev);
- dmaengine_terminate_async(dma->chan_using);
- f7_msg->result = -ETIMEDOUT;
- }
+ mask = STM32F7_I2C_ALL_IRQ_MASK;
+ stm32f7_i2c_disable_irq(i2c_dev, mask);
- status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
+ /* Clear STOP flag */
+ writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
- if (status & STM32F7_I2C_ISR_TC) {
- if (f7_msg->smbus) {
- stm32f7_i2c_smbus_rep_start(i2c_dev);
- } else {
- i2c_dev->msg_id++;
- i2c_dev->msg++;
- stm32f7_i2c_xfer_msg(i2c_dev, i2c_dev->msg);
- }
- } else {
i2c_dev->master_mode = false;
complete(&i2c_dev->complete);
}
@@ -1615,7 +1598,7 @@ static irqreturn_t stm32f7_i2c_isr_event_thread(int irq, void *data)
return IRQ_HANDLED;
}
-static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
+static irqreturn_t stm32f7_i2c_isr_error_thread(int irq, void *data)
{
struct stm32f7_i2c_dev *i2c_dev = data;
struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
@@ -2202,8 +2185,11 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
if (ret)
return dev_err_probe(&pdev->dev, ret, "Failed to request irq event\n");
- ret = devm_request_irq(&pdev->dev, irq_error, stm32f7_i2c_isr_error, 0,
- pdev->name, i2c_dev);
+ ret = devm_request_threaded_irq(&pdev->dev, irq_error,
+ NULL,
+ stm32f7_i2c_isr_error_thread,
+ IRQF_ONESHOT,
+ pdev->name, i2c_dev);
if (ret)
return dev_err_probe(&pdev->dev, ret, "Failed to request irq error\n");
--
2.25.1
Avoid usage of __func__ when reporting an error message
since dev_err/dev_dbg are already providing enough details
to identify the source of the message.
Signed-off-by: Alain Volmat <[email protected]>
Reviewed-by: Andi Shyti <[email protected]>
---
drivers/i2c/busses/i2c-stm32f7.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index dde70e7ec722..15100fed0a3f 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1602,6 +1602,7 @@ static irqreturn_t stm32f7_i2c_isr_error_thread(int irq, void *data)
{
struct stm32f7_i2c_dev *i2c_dev = data;
struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+ u16 addr = f7_msg->addr;
void __iomem *base = i2c_dev->base;
struct device *dev = i2c_dev->dev;
struct stm32_i2c_dma *dma = i2c_dev->dma;
@@ -1611,8 +1612,7 @@ static irqreturn_t stm32f7_i2c_isr_error_thread(int irq, void *data)
/* Bus error */
if (status & STM32F7_I2C_ISR_BERR) {
- dev_err(dev, "<%s>: Bus error accessing addr 0x%x\n",
- __func__, f7_msg->addr);
+ dev_err(dev, "Bus error accessing addr 0x%x\n", addr);
writel_relaxed(STM32F7_I2C_ICR_BERRCF, base + STM32F7_I2C_ICR);
stm32f7_i2c_release_bus(&i2c_dev->adap);
f7_msg->result = -EIO;
@@ -1620,21 +1620,19 @@ static irqreturn_t stm32f7_i2c_isr_error_thread(int irq, void *data)
/* Arbitration loss */
if (status & STM32F7_I2C_ISR_ARLO) {
- dev_dbg(dev, "<%s>: Arbitration loss accessing addr 0x%x\n",
- __func__, f7_msg->addr);
+ dev_dbg(dev, "Arbitration loss accessing addr 0x%x\n", addr);
writel_relaxed(STM32F7_I2C_ICR_ARLOCF, base + STM32F7_I2C_ICR);
f7_msg->result = -EAGAIN;
}
if (status & STM32F7_I2C_ISR_PECERR) {
- dev_err(dev, "<%s>: PEC error in reception accessing addr 0x%x\n",
- __func__, f7_msg->addr);
+ dev_err(dev, "PEC error in reception accessing addr 0x%x\n", addr);
writel_relaxed(STM32F7_I2C_ICR_PECCF, base + STM32F7_I2C_ICR);
f7_msg->result = -EINVAL;
}
if (status & STM32F7_I2C_ISR_ALERT) {
- dev_dbg(dev, "<%s>: SMBus alert received\n", __func__);
+ dev_dbg(dev, "SMBus alert received\n");
writel_relaxed(STM32F7_I2C_ICR_ALERTCF, base + STM32F7_I2C_ICR);
i2c_handle_smbus_alert(i2c_dev->alert->ara);
return IRQ_HANDLED;
--
2.25.1
Add a new compatible st,stm32mp25-i2c for the STM32MP25 series which
has only one interrupt line for both events and errors and differs in
term of handling of FastModePlus.
Signed-off-by: Alain Volmat <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
---
.../devicetree/bindings/i2c/st,stm32-i2c.yaml | 28 +++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
index 94b75d9f66cd..1b31b87c1800 100644
--- a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml
@@ -19,6 +19,7 @@ allOf:
- st,stm32f7-i2c
- st,stm32mp13-i2c
- st,stm32mp15-i2c
+ - st,stm32mp25-i2c
then:
properties:
i2c-scl-rising-time-ns:
@@ -41,6 +42,30 @@ allOf:
clock-frequency:
enum: [100000, 400000]
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - st,stm32f4-i2c
+ - st,stm32f7-i2c
+ - st,stm32mp13-i2c
+ - st,stm32mp15-i2c
+ then:
+ properties:
+ interrupts:
+ minItems: 2
+
+ interrupt-names:
+ minItems: 2
+ else:
+ properties:
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ maxItems: 1
+
properties:
compatible:
enum:
@@ -48,6 +73,7 @@ properties:
- st,stm32f7-i2c
- st,stm32mp13-i2c
- st,stm32mp15-i2c
+ - st,stm32mp25-i2c
reg:
maxItems: 1
@@ -56,11 +82,13 @@ properties:
items:
- description: interrupt ID for I2C event
- description: interrupt ID for I2C error
+ minItems: 1
interrupt-names:
items:
- const: event
- const: error
+ minItems: 1
resets:
maxItems: 1
--
2.25.1
Move readl_relaxed of I2C_ISR register at beginning of event isr
so that it done once for both master & slave handling.
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/i2c/busses/i2c-stm32f7.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 15100fed0a3f..f7abf6d3a7b9 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1419,15 +1419,13 @@ static bool stm32f7_i2c_is_slave_busy(struct stm32f7_i2c_dev *i2c_dev)
return i == busy;
}
-static irqreturn_t stm32f7_i2c_slave_isr_event(struct stm32f7_i2c_dev *i2c_dev)
+static irqreturn_t stm32f7_i2c_slave_isr_event(struct stm32f7_i2c_dev *i2c_dev, u32 status)
{
void __iomem *base = i2c_dev->base;
- u32 cr2, status, mask;
+ u32 cr2, mask;
u8 val;
int ret;
- status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
-
/* Slave transmitter mode */
if (status & STM32F7_I2C_ISR_TXIS) {
i2c_slave_event(i2c_dev->slave_running,
@@ -1531,11 +1529,11 @@ static irqreturn_t stm32f7_i2c_isr_event_thread(int irq, void *data)
u32 status, mask;
int ret;
- if (!i2c_dev->master_mode)
- return stm32f7_i2c_slave_isr_event(i2c_dev);
-
status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
+ if (!i2c_dev->master_mode)
+ return stm32f7_i2c_slave_isr_event(i2c_dev, status);
+
/* NACK received */
if (status & STM32F7_I2C_ISR_NACKF) {
dev_dbg(i2c_dev->dev, "<%s>: Receive NACK (addr %x)\n",
--
2.25.1
The stm32mp25 has only a single interrupt line used for both
events and errors. In order to cope with that, reorganise the
error handling code so that it can be called either from the
common handler (used in case of SoC having only a single IT line)
and the error handler for others.
The CR1 register also embeds a new FMP bit, necessary when running
at Fast Mode Plus frequency. This bit should be used instead of
the SYSCFG bit used on other platforms.
Add a new compatible to distinguish between the SoCs and two
boolean within the setup structure in order to know if the
platform has a single/multiple IT lines and if the FMP bit
within CR1 is available or not.
Signed-off-by: Valentin Caron <[email protected]>
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/i2c/busses/i2c-stm32f7.c | 214 ++++++++++++++++++-------------
1 file changed, 126 insertions(+), 88 deletions(-)
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index f7abf6d3a7b9..01210452216b 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -50,6 +50,7 @@
#define STM32F7_I2C_TXDR 0x28
/* STM32F7 I2C control 1 */
+#define STM32_I2C_CR1_FMP BIT(24)
#define STM32F7_I2C_CR1_PECEN BIT(23)
#define STM32F7_I2C_CR1_ALERTEN BIT(22)
#define STM32F7_I2C_CR1_SMBHEN BIT(20)
@@ -226,6 +227,8 @@ struct stm32f7_i2c_spec {
* @rise_time: Rise time (ns)
* @fall_time: Fall time (ns)
* @fmp_clr_offset: Fast Mode Plus clear register offset from set register
+ * @single_it_line: Only a single IT line is used for both events/errors
+ * @fmp_cr1_bit: Fast Mode Plus control is done via a bit in CR1
*/
struct stm32f7_i2c_setup {
u32 speed_freq;
@@ -233,6 +236,8 @@ struct stm32f7_i2c_setup {
u32 rise_time;
u32 fall_time;
u32 fmp_clr_offset;
+ bool single_it_line;
+ bool fmp_cr1_bit;
};
/**
@@ -418,6 +423,13 @@ static const struct stm32f7_i2c_setup stm32mp13_setup = {
.fmp_clr_offset = 0x4,
};
+static const struct stm32f7_i2c_setup stm32mp25_setup = {
+ .rise_time = STM32F7_I2C_RISE_TIME_DEFAULT,
+ .fall_time = STM32F7_I2C_FALL_TIME_DEFAULT,
+ .single_it_line = true,
+ .fmp_cr1_bit = true,
+};
+
static inline void stm32f7_i2c_set_bits(void __iomem *reg, u32 mask)
{
writel_relaxed(readl_relaxed(reg) | mask, reg);
@@ -1492,17 +1504,81 @@ static irqreturn_t stm32f7_i2c_slave_isr_event(struct stm32f7_i2c_dev *i2c_dev,
return IRQ_HANDLED;
}
+static irqreturn_t stm32f7_i2c_handle_isr_errs(struct stm32f7_i2c_dev *i2c_dev, u32 status)
+{
+ struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+ u16 addr = f7_msg->addr;
+ void __iomem *base = i2c_dev->base;
+ struct device *dev = i2c_dev->dev;
+ struct stm32_i2c_dma *dma = i2c_dev->dma;
+
+ /* Bus error */
+ if (status & STM32F7_I2C_ISR_BERR) {
+ dev_err(dev, "Bus error accessing addr 0x%x\n", addr);
+ writel_relaxed(STM32F7_I2C_ICR_BERRCF, base + STM32F7_I2C_ICR);
+ stm32f7_i2c_release_bus(&i2c_dev->adap);
+ f7_msg->result = -EIO;
+ }
+
+ /* Arbitration loss */
+ if (status & STM32F7_I2C_ISR_ARLO) {
+ dev_dbg(dev, "Arbitration loss accessing addr 0x%x\n", addr);
+ writel_relaxed(STM32F7_I2C_ICR_ARLOCF, base + STM32F7_I2C_ICR);
+ f7_msg->result = -EAGAIN;
+ }
+
+ if (status & STM32F7_I2C_ISR_PECERR) {
+ dev_err(dev, "PEC error in reception accessing addr 0x%x\n", addr);
+ writel_relaxed(STM32F7_I2C_ICR_PECCF, base + STM32F7_I2C_ICR);
+ f7_msg->result = -EINVAL;
+ }
+
+ if (status & STM32F7_I2C_ISR_ALERT) {
+ dev_dbg(dev, "SMBus alert received\n");
+ writel_relaxed(STM32F7_I2C_ICR_ALERTCF, base + STM32F7_I2C_ICR);
+ i2c_handle_smbus_alert(i2c_dev->alert->ara);
+ return IRQ_HANDLED;
+ }
+
+ if (!i2c_dev->slave_running) {
+ u32 mask;
+ /* Disable interrupts */
+ if (stm32f7_i2c_is_slave_registered(i2c_dev))
+ mask = STM32F7_I2C_XFER_IRQ_MASK;
+ else
+ mask = STM32F7_I2C_ALL_IRQ_MASK;
+ stm32f7_i2c_disable_irq(i2c_dev, mask);
+ }
+
+ /* Disable dma */
+ if (i2c_dev->use_dma) {
+ stm32f7_i2c_disable_dma_req(i2c_dev);
+ dmaengine_terminate_async(dma->chan_using);
+ }
+
+ i2c_dev->master_mode = false;
+ complete(&i2c_dev->complete);
+
+ return IRQ_HANDLED;
+}
+
+#define STM32F7_ERR_EVENTS (STM32F7_I2C_ISR_BERR | STM32F7_I2C_ISR_ARLO |\
+ STM32F7_I2C_ISR_PECERR | STM32F7_I2C_ISR_ALERT)
static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
{
struct stm32f7_i2c_dev *i2c_dev = data;
u32 status;
- /* Check if the interrupt is for a slave device */
- if (!i2c_dev->master_mode)
- return IRQ_WAKE_THREAD;
-
status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
+ /*
+ * Check if the interrupt is for a slave device or related
+ * to errors flags (in case of single it line mode)
+ */
+ if (!i2c_dev->master_mode ||
+ (i2c_dev->setup.single_it_line && (status & STM32F7_ERR_EVENTS)))
+ return IRQ_WAKE_THREAD;
+
/* Tx empty */
if (status & STM32F7_I2C_ISR_TXIS)
stm32f7_i2c_write_tx_data(i2c_dev);
@@ -1534,6 +1610,10 @@ static irqreturn_t stm32f7_i2c_isr_event_thread(int irq, void *data)
if (!i2c_dev->master_mode)
return stm32f7_i2c_slave_isr_event(i2c_dev, status);
+ /* Handle errors in case of this handler is used for events/errors */
+ if (i2c_dev->setup.single_it_line && (status & STM32F7_ERR_EVENTS))
+ return stm32f7_i2c_handle_isr_errs(i2c_dev, status);
+
/* NACK received */
if (status & STM32F7_I2C_ISR_NACKF) {
dev_dbg(i2c_dev->dev, "<%s>: Receive NACK (addr %x)\n",
@@ -1599,63 +1679,11 @@ static irqreturn_t stm32f7_i2c_isr_event_thread(int irq, void *data)
static irqreturn_t stm32f7_i2c_isr_error_thread(int irq, void *data)
{
struct stm32f7_i2c_dev *i2c_dev = data;
- struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
- u16 addr = f7_msg->addr;
- void __iomem *base = i2c_dev->base;
- struct device *dev = i2c_dev->dev;
- struct stm32_i2c_dma *dma = i2c_dev->dma;
u32 status;
status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
- /* Bus error */
- if (status & STM32F7_I2C_ISR_BERR) {
- dev_err(dev, "Bus error accessing addr 0x%x\n", addr);
- writel_relaxed(STM32F7_I2C_ICR_BERRCF, base + STM32F7_I2C_ICR);
- stm32f7_i2c_release_bus(&i2c_dev->adap);
- f7_msg->result = -EIO;
- }
-
- /* Arbitration loss */
- if (status & STM32F7_I2C_ISR_ARLO) {
- dev_dbg(dev, "Arbitration loss accessing addr 0x%x\n", addr);
- writel_relaxed(STM32F7_I2C_ICR_ARLOCF, base + STM32F7_I2C_ICR);
- f7_msg->result = -EAGAIN;
- }
-
- if (status & STM32F7_I2C_ISR_PECERR) {
- dev_err(dev, "PEC error in reception accessing addr 0x%x\n", addr);
- writel_relaxed(STM32F7_I2C_ICR_PECCF, base + STM32F7_I2C_ICR);
- f7_msg->result = -EINVAL;
- }
-
- if (status & STM32F7_I2C_ISR_ALERT) {
- dev_dbg(dev, "SMBus alert received\n");
- writel_relaxed(STM32F7_I2C_ICR_ALERTCF, base + STM32F7_I2C_ICR);
- i2c_handle_smbus_alert(i2c_dev->alert->ara);
- return IRQ_HANDLED;
- }
-
- if (!i2c_dev->slave_running) {
- u32 mask;
- /* Disable interrupts */
- if (stm32f7_i2c_is_slave_registered(i2c_dev))
- mask = STM32F7_I2C_XFER_IRQ_MASK;
- else
- mask = STM32F7_I2C_ALL_IRQ_MASK;
- stm32f7_i2c_disable_irq(i2c_dev, mask);
- }
-
- /* Disable dma */
- if (i2c_dev->use_dma) {
- stm32f7_i2c_disable_dma_req(i2c_dev);
- dmaengine_terminate_async(dma->chan_using);
- }
-
- i2c_dev->master_mode = false;
- complete(&i2c_dev->complete);
-
- return IRQ_HANDLED;
+ return stm32f7_i2c_handle_isr_errs(i2c_dev, status);
}
static int stm32f7_i2c_wait_polling(struct stm32f7_i2c_dev *i2c_dev)
@@ -1991,23 +2019,27 @@ static int stm32f7_i2c_unreg_slave(struct i2c_client *slave)
static int stm32f7_i2c_write_fm_plus_bits(struct stm32f7_i2c_dev *i2c_dev,
bool enable)
{
- int ret;
+ int ret = 0;
if (i2c_dev->bus_rate <= I2C_MAX_FAST_MODE_FREQ ||
- IS_ERR_OR_NULL(i2c_dev->regmap))
+ (!i2c_dev->setup.fmp_cr1_bit && IS_ERR_OR_NULL(i2c_dev->regmap)))
/* Optional */
return 0;
- if (i2c_dev->fmp_sreg == i2c_dev->fmp_creg)
- ret = regmap_update_bits(i2c_dev->regmap,
- i2c_dev->fmp_sreg,
- i2c_dev->fmp_mask,
- enable ? i2c_dev->fmp_mask : 0);
- else
- ret = regmap_write(i2c_dev->regmap,
- enable ? i2c_dev->fmp_sreg :
- i2c_dev->fmp_creg,
- i2c_dev->fmp_mask);
+ if (i2c_dev->setup.fmp_cr1_bit) {
+ if (enable)
+ stm32f7_i2c_set_bits(i2c_dev->base + STM32F7_I2C_CR1, STM32_I2C_CR1_FMP);
+ else
+ stm32f7_i2c_clr_bits(i2c_dev->base + STM32F7_I2C_CR1, STM32_I2C_CR1_FMP);
+ } else {
+ if (i2c_dev->fmp_sreg == i2c_dev->fmp_creg)
+ ret = regmap_update_bits(i2c_dev->regmap, i2c_dev->fmp_sreg,
+ i2c_dev->fmp_mask, enable ? i2c_dev->fmp_mask : 0);
+ else
+ ret = regmap_write(i2c_dev->regmap,
+ enable ? i2c_dev->fmp_sreg : i2c_dev->fmp_creg,
+ i2c_dev->fmp_mask);
+ }
return ret;
}
@@ -2141,6 +2173,13 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
if (!i2c_dev)
return -ENOMEM;
+ setup = of_device_get_match_data(&pdev->dev);
+ if (!setup) {
+ dev_err(&pdev->dev, "Can't get device data\n");
+ return -ENODEV;
+ }
+ i2c_dev->setup = *setup;
+
i2c_dev->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
if (IS_ERR(i2c_dev->base))
return PTR_ERR(i2c_dev->base);
@@ -2150,10 +2189,6 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
if (irq_event < 0)
return irq_event;
- irq_error = platform_get_irq(pdev, 1);
- if (irq_error < 0)
- return irq_error;
-
i2c_dev->wakeup_src = of_property_read_bool(pdev->dev.of_node,
"wakeup-source");
@@ -2181,20 +2216,19 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
if (ret)
return dev_err_probe(&pdev->dev, ret, "Failed to request irq event\n");
- ret = devm_request_threaded_irq(&pdev->dev, irq_error,
- NULL,
- stm32f7_i2c_isr_error_thread,
- IRQF_ONESHOT,
- pdev->name, i2c_dev);
- if (ret)
- return dev_err_probe(&pdev->dev, ret, "Failed to request irq error\n");
+ if (!i2c_dev->setup.single_it_line) {
+ irq_error = platform_get_irq(pdev, 1);
+ if (irq_error < 0)
+ return irq_error;
- setup = of_device_get_match_data(&pdev->dev);
- if (!setup) {
- dev_err(&pdev->dev, "Can't get device data\n");
- return -ENODEV;
+ ret = devm_request_threaded_irq(&pdev->dev, irq_error,
+ NULL,
+ stm32f7_i2c_isr_error_thread,
+ IRQF_ONESHOT,
+ pdev->name, i2c_dev);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "Failed to request irq error\n");
}
- i2c_dev->setup = *setup;
ret = stm32f7_i2c_setup_timing(i2c_dev, &i2c_dev->setup);
if (ret)
@@ -2202,9 +2236,12 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
/* Setup Fast mode plus if necessary */
if (i2c_dev->bus_rate > I2C_MAX_FAST_MODE_FREQ) {
- ret = stm32f7_i2c_setup_fm_plus_bits(pdev, i2c_dev);
- if (ret)
- return ret;
+ if (!i2c_dev->setup.fmp_cr1_bit) {
+ ret = stm32f7_i2c_setup_fm_plus_bits(pdev, i2c_dev);
+ if (ret)
+ return ret;
+ }
+
ret = stm32f7_i2c_write_fm_plus_bits(i2c_dev, true);
if (ret)
return ret;
@@ -2483,6 +2520,7 @@ static const struct of_device_id stm32f7_i2c_match[] = {
{ .compatible = "st,stm32f7-i2c", .data = &stm32f7_setup},
{ .compatible = "st,stm32mp15-i2c", .data = &stm32mp15_setup},
{ .compatible = "st,stm32mp13-i2c", .data = &stm32mp13_setup},
+ { .compatible = "st,stm32mp25-i2c", .data = &stm32mp25_setup},
{},
};
MODULE_DEVICE_TABLE(of, stm32f7_i2c_match);
--
2.25.1
Add the 8 nodes for all i2c instances available on the stm32mp251.
Signed-off-by: Alain Volmat <[email protected]>
---
arch/arm64/boot/dts/st/stm32mp251.dtsi | 96 ++++++++++++++++++++++++++
1 file changed, 96 insertions(+)
diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
index dfbdb3a773e4..93bc8a8908ce 100644
--- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
@@ -116,6 +116,102 @@ usart2: serial@400e0000 {
status = "disabled";
};
+ i2c1: i2c@40120000 {
+ compatible = "st,stm32mp25-i2c";
+ reg = <0x40120000 0x400>;
+ interrupt-names = "event";
+ interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&rcc CK_KER_I2C1>;
+ resets = <&rcc I2C1_R>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c2: i2c@40130000 {
+ compatible = "st,stm32mp25-i2c";
+ reg = <0x40130000 0x400>;
+ interrupt-names = "event";
+ interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&rcc CK_KER_I2C2>;
+ resets = <&rcc I2C2_R>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c3: i2c@40140000 {
+ compatible = "st,stm32mp25-i2c";
+ reg = <0x40140000 0x400>;
+ interrupt-names = "event";
+ interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&rcc CK_KER_I2C3>;
+ resets = <&rcc I2C3_R>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c4: i2c@40150000 {
+ compatible = "st,stm32mp25-i2c";
+ reg = <0x40150000 0x400>;
+ interrupt-names = "event";
+ interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&rcc CK_KER_I2C4>;
+ resets = <&rcc I2C4_R>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c5: i2c@40160000 {
+ compatible = "st,stm32mp25-i2c";
+ reg = <0x40160000 0x400>;
+ interrupt-names = "event";
+ interrupts = <GIC_SPI 181 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&rcc CK_KER_I2C5>;
+ resets = <&rcc I2C5_R>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c6: i2c@40170000 {
+ compatible = "st,stm32mp25-i2c";
+ reg = <0x40170000 0x400>;
+ interrupt-names = "event";
+ interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&rcc CK_KER_I2C6>;
+ resets = <&rcc I2C6_R>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c7: i2c@40180000 {
+ compatible = "st,stm32mp25-i2c";
+ reg = <0x40180000 0x400>;
+ interrupt-names = "event";
+ interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&rcc CK_KER_I2C7>;
+ resets = <&rcc I2C7_R>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ i2c8: i2c@46040000 {
+ compatible = "st,stm32mp25-i2c";
+ reg = <0x46040000 0x400>;
+ interrupt-names = "event";
+ interrupts = <GIC_SPI 212 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&rcc CK_KER_I2C8>;
+ resets = <&rcc I2C8_R>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
sdmmc1: mmc@48220000 {
compatible = "st,stm32mp25-sdmmc2", "arm,pl18x", "arm,primecell";
arm,primecell-periphid = <0x00353180>;
--
2.25.1
Add the i2c2 and i2c8 pins used on STM32MP257F-EV1 board.
Signed-off-by: Alain Volmat <[email protected]>
---
arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi | 36 +++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi b/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi
index 66791a974f8f..4194807606ce 100644
--- a/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi
@@ -6,6 +6,23 @@
#include <dt-bindings/pinctrl/stm32-pinfunc.h>
&pinctrl {
+ i2c2_pins_a: i2c2-0 {
+ pins {
+ pinmux = <STM32_PINMUX('B', 5, AF9)>, /* I2C2_SCL */
+ <STM32_PINMUX('B', 4, AF9)>; /* I2C2_SDA */
+ bias-disable;
+ drive-open-drain;
+ slew-rate = <0>;
+ };
+ };
+
+ i2c2_sleep_pins_a: i2c2-sleep-0 {
+ pins {
+ pinmux = <STM32_PINMUX('B', 5, ANALOG)>, /* I2C2_SCL */
+ <STM32_PINMUX('B', 4, ANALOG)>; /* I2C2_SDA */
+ };
+ };
+
sdmmc1_b4_pins_a: sdmmc1-b4-0 {
pins1 {
pinmux = <STM32_PINMUX('E', 4, AF10)>, /* SDMMC1_D0 */
@@ -90,3 +107,22 @@ pins {
};
};
};
+
+&pinctrl_z {
+ i2c8_pins_a: i2c8-0 {
+ pins {
+ pinmux = <STM32_PINMUX('Z', 4, AF8)>, /* I2C8_SCL */
+ <STM32_PINMUX('Z', 3, AF8)>; /* I2C8_SDA */
+ bias-disable;
+ drive-open-drain;
+ slew-rate = <0>;
+ };
+ };
+
+ i2c8_sleep_pins_a: i2c8-sleep-0 {
+ pins {
+ pinmux = <STM32_PINMUX('Z', 4, ANALOG)>, /* I2C8_SCL */
+ <STM32_PINMUX('Z', 3, ANALOG)>; /* I2C8_SDA */
+ };
+ };
+};
--
2.25.1
Add properties for i2c2 and i2c8 available on the stm32mp257f-ev1.
i2c2 is enabled since several devices are attached to it while
i2c8 is kept disabled since only used via the gpio expansion connector.
Signed-off-by: Alain Volmat <[email protected]>
---
arch/arm64/boot/dts/st/stm32mp257f-ev1.dts | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts
index b2d3afb15758..0ea8e69bfb3d 100644
--- a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts
+++ b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts
@@ -55,6 +55,26 @@ &arm_wdt {
status = "okay";
};
+&i2c2 {
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&i2c2_pins_a>;
+ pinctrl-1 = <&i2c2_sleep_pins_a>;
+ i2c-scl-rising-time-ns = <100>;
+ i2c-scl-falling-time-ns = <13>;
+ clock-frequency = <400000>;
+ status = "okay";
+};
+
+&i2c8 {
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&i2c8_pins_a>;
+ pinctrl-1 = <&i2c8_sleep_pins_a>;
+ i2c-scl-rising-time-ns = <57>;
+ i2c-scl-falling-time-ns = <7>;
+ clock-frequency = <400000>;
+ status = "disabled";
+};
+
&sdmmc1 {
pinctrl-names = "default", "opendrain", "sleep";
pinctrl-0 = <&sdmmc1_b4_pins_a>;
--
2.25.1
On Fri, Dec 15, 2023 at 06:06:05PM +0100, Alain Volmat wrote:
> Convert error handling upon calls of devm_request_irq functions during
> the probe of the driver.
>
> Signed-off-by: Alain Volmat <[email protected]>
Applied to for-next, thanks!
On Fri, Dec 15, 2023 at 06:06:06PM +0100, Alain Volmat wrote:
> The irq handling is currently split between the irq handler
> and the threaded irq handler. Some of the handling (such as
> dma related stuffs) done within the irq handler might sleep or
> take some time leading to issues if the kernel is built with
> realtime constraints. In order to fix that, perform an overall
> rework to perform most of the job within the threaded handler
> and only keep fifo access in the non threaded handler.
>
> Signed-off-by: Alain Volmat <[email protected]>
> Reviewed-by: Andi Shyti <[email protected]>
Applied to for-next, thanks!
On Fri, Dec 15, 2023 at 06:06:08PM +0100, Alain Volmat wrote:
> Add a new compatible st,stm32mp25-i2c for the STM32MP25 series which
> has only one interrupt line for both events and errors and differs in
> term of handling of FastModePlus.
>
> Signed-off-by: Alain Volmat <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>
Applied to for-next, thanks!
On Fri, Dec 15, 2023 at 06:06:09PM +0100, Alain Volmat wrote:
> Move readl_relaxed of I2C_ISR register at beginning of event isr
> so that it done once for both master & slave handling.
>
> Signed-off-by: Alain Volmat <[email protected]>
Applied to for-next, thanks!
On Fri, Dec 15, 2023 at 06:06:10PM +0100, Alain Volmat wrote:
> The stm32mp25 has only a single interrupt line used for both
> events and errors. In order to cope with that, reorganise the
> error handling code so that it can be called either from the
> common handler (used in case of SoC having only a single IT line)
> and the error handler for others.
> The CR1 register also embeds a new FMP bit, necessary when running
> at Fast Mode Plus frequency. This bit should be used instead of
> the SYSCFG bit used on other platforms.
> Add a new compatible to distinguish between the SoCs and two
> boolean within the setup structure in order to know if the
> platform has a single/multiple IT lines and if the FMP bit
> within CR1 is available or not.
>
> Signed-off-by: Valentin Caron <[email protected]>
> Signed-off-by: Alain Volmat <[email protected]>
Applied to for-next, thanks!
On Fri, Dec 15, 2023 at 06:06:07PM +0100, Alain Volmat wrote:
> Avoid usage of __func__ when reporting an error message
> since dev_err/dev_dbg are already providing enough details
> to identify the source of the message.
>
> Signed-off-by: Alain Volmat <[email protected]>
> Reviewed-by: Andi Shyti <[email protected]>
Applied to for-next, thanks!
Hi Alain
On 12/15/23 18:06, Alain Volmat wrote:
> This series first perform enhancements in the way interrupt are handled
> and cleanup in messages.
> Then it adds support for the stm32mp25 which differs in that
> it only has a single irq line for both event/error and has a
> different handling of the FastModePlus.
> Support is then enabled within the stm32mp25 related device-trees.
>
> Changelog:
> v3: - addition of 2 commits dealing with readl_relaxed(I2C_ISR) in
> isr handler and a second one to use dev_err_probe during probe
> - correction of SOB in commit
>
> v2: - correct st,stm32-i2c.yaml. Use if then else scheme to indicate
> number of interrupts / interrupt-names depending on the
> compatible while keeping the description within the common part
>
> - correct 2 maybe-uninitialized warnings
> * ret in stm32f7_i2c_write_fm_plus_bits
> * irq_error in stm32f7_i2c_probe, move the platform_get_irq
> within the same if block as devm_request_threaded_irq
>
> Alain Volmat (9):
> i2c: stm32f7: use dev_err_probe upon calls of devm_request_irq
> i2c: stm32f7: perform most of irq job in threaded handler
> i2c: stm32f7: simplify status messages in case of errors
> dt-bindings: i2c: document st,stm32mp25-i2c compatible
> i2c: stm32f7: perform I2C_ISR read once at beginning of event isr
> i2c: stm32f7: add support for stm32mp25 soc
> arm64: dts: st: add all 8 i2c nodes on stm32mp251
> arm64: dts: st: add i2c2/i2c8 pins for stm32mp25
> arm64: dts: st: add i2c2 / i2c8 properties on stm32mp257f-ev1
>
> .../devicetree/bindings/i2c/st,stm32-i2c.yaml | 28 ++
> arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi | 36 ++
> arch/arm64/boot/dts/st/stm32mp251.dtsi | 96 +++++
> arch/arm64/boot/dts/st/stm32mp257f-ev1.dts | 20 +
> drivers/i2c/busses/i2c-stm32f7.c | 342 +++++++++---------
> 5 files changed, 358 insertions(+), 164 deletions(-)
>
DT patches applied on stm32-next. Not that I added acces-controller
bindings for all I2C nodes.
Thanks!
Alex