Hi,
This series adds two tangent features to the Nomadik I2C controller:
- Add a new compatible to support Mobileye EyeQ5 which uses the same IP
block as Nomadik.
It has two quirks to be handled:
- The memory bus only supports 32-bit accesses. A writeb() is used
which we avoid.
- We must write a value into a shared register region (OLB, "Other
Logic Block") depending on the I2C bus speed.
- Allow xfer timeouts below one jiffy by using a workqueue and hrtimers
instead of a completion.
The situation to be addressed is:
- Many devices on the same I2C bus.
- One xfer to each device is sent at regular interval.
- One device gets stuck and does not answer.
- With long timeouts, following devices won't get their message. A
shorter timeout ensures we can still talk to the following
devices.
This clashes a bit with the current i2c_adapter timeout field that
stores a jiffies amount. We cannot rely on it and therefore we take
a value from devicetree as a µs value. If the timeout is less than a
jiffy duration, we switch from standard jiffies timeout to
hrtimers.
There is one patch targeting a hwmon dt-bindings file:
Documentation/devicetree/bindings/hwmon/lm75.yaml. The rest is touching
the I2C bus driver, its bindings and platform devicetrees.
About dependencies:
- The series is based upon v6.8-rc4.
- For testing on EyeQ5 hardware and devicetree patches, we need the
base platform series from Grégory [0].
- The last commit (adding DT phandles for resets), we need the syscon
series [1] that provides the reset controller node.
I think there are discussions to be had about:
- The handling of timeouts. Having a non-jiffy value is not driver
specific. Should this change be done at the subsystem layer? The
subsystem could even fetch the value from devicetree and auto-fill
timeout, with a default given by the driver. Not many drivers seem
to use the i2c_adapter timeout field from my quick grepping.
- The DT prop for timeout. I've picked "timeout-usecs". Some drivers
use vendor prefixes, but this is not vendor-specific and only a
software implementation detail.
- The shape of this series. Initially it was split in two. However I
brought them together as they cannot be applied independently.
Please tell me if a better approach is to be preferred.
Those are thoughts, I'm sure people will have feedback on this.
Have a nice day,
Théo Lebrun
[0]: https://lore.kernel.org/lkml/[email protected]/
[1]: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Théo Lebrun <[email protected]>
---
Théo Lebrun (13):
dt-bindings: i2c: nomadik: add timeout-usecs property bindings
dt-bindings: i2c: nomadik: add mobileye,eyeq5-i2c bindings and example
dt-bindings: hwmon: lm75: add label property
i2c: nomadik: rename private struct pointers from dev to priv
i2c: nomadik: simplify IRQ masking logic
i2c: nomadik: use bitops helpers
i2c: nomadik: support short xfer timeouts using waitqueue & hrtimer
i2c: nomadik: replace jiffies by ktime for FIFO flushing timeout
i2c: nomadik: fetch timeout-usecs property from devicetree
i2c: nomadik: support Mobileye EyeQ5 I2C controller
MIPS: mobileye: eyeq5: add 5 I2C controller nodes
MIPS: mobileye: eyeq5: add evaluation board I2C temp sensor
MIPS: mobileye: eyeq5: add resets to I2C controllers
Documentation/devicetree/bindings/hwmon/lm75.yaml | 4 +
.../devicetree/bindings/i2c/st,nomadik-i2c.yaml | 49 +-
arch/mips/boot/dts/mobileye/eyeq5-epm5.dts | 8 +
arch/mips/boot/dts/mobileye/eyeq5.dtsi | 75 +++
drivers/i2c/busses/i2c-nomadik.c | 710 ++++++++++++---------
5 files changed, 534 insertions(+), 312 deletions(-)
---
base-commit: d55aa725e32849f709b61eab3b7a50b810a71a84
change-id: 20231023-mbly-i2c-7c2fbbb1299f
Best regards,
--
Théo Lebrun <[email protected]>
IRQ_MASK and I2C_CLEAR_ALL_INTS are redundant. One masks the top three
bits off as reserved, the other one masks the reserved IRQs inside the
u32. Get rid of IRQ_MASK and only use the most restrictive mask.
Signed-off-by: Théo Lebrun <[email protected]>
---
drivers/i2c/busses/i2c-nomadik.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index cd511c884f99..80bdf7e42613 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -94,9 +94,6 @@
/* some bits in ICR are reserved */
#define I2C_CLEAR_ALL_INTS 0x131f007f
-/* first three msb bits are reserved */
-#define IRQ_MASK(mask) (mask & 0x1fffffff)
-
/* maximum threshold value */
#define MAX_I2C_FIFO_THRESHOLD 15
@@ -249,8 +246,7 @@ static int flush_i2c_fifo(struct nmk_i2c_dev *priv)
*/
static void disable_all_interrupts(struct nmk_i2c_dev *priv)
{
- u32 mask = IRQ_MASK(0);
- writel(mask, priv->virtbase + I2C_IMSCR);
+ writel(0, priv->virtbase + I2C_IMSCR);
}
/**
@@ -259,9 +255,7 @@ static void disable_all_interrupts(struct nmk_i2c_dev *priv)
*/
static void clear_all_interrupts(struct nmk_i2c_dev *priv)
{
- u32 mask;
- mask = IRQ_MASK(I2C_CLEAR_ALL_INTS);
- writel(mask, priv->virtbase + I2C_ICR);
+ writel(I2C_CLEAR_ALL_INTS, priv->virtbase + I2C_ICR);
}
/**
@@ -468,7 +462,7 @@ static int read_i2c(struct nmk_i2c_dev *priv, u16 flags)
else
irq_mask |= I2C_IT_MTDWS;
- irq_mask = I2C_CLEAR_ALL_INTS & IRQ_MASK(irq_mask);
+ irq_mask &= I2C_CLEAR_ALL_INTS;
writel(readl(priv->virtbase + I2C_IMSCR) | irq_mask,
priv->virtbase + I2C_IMSCR);
@@ -547,7 +541,7 @@ static int write_i2c(struct nmk_i2c_dev *priv, u16 flags)
else
irq_mask |= I2C_IT_MTDWS;
- irq_mask = I2C_CLEAR_ALL_INTS & IRQ_MASK(irq_mask);
+ irq_mask &= I2C_CLEAR_ALL_INTS;
writel(readl(priv->virtbase + I2C_IMSCR) | irq_mask,
priv->virtbase + I2C_IMSCR);
@@ -703,8 +697,8 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
*/
static int disable_interrupts(struct nmk_i2c_dev *priv, u32 irq)
{
- irq = IRQ_MASK(irq);
- writel(readl(priv->virtbase + I2C_IMSCR) & ~(I2C_CLEAR_ALL_INTS & irq),
+ irq &= I2C_CLEAR_ALL_INTS;
+ writel(readl(priv->virtbase + I2C_IMSCR) & ~irq,
priv->virtbase + I2C_IMSCR);
return 0;
}
--
2.43.1
Add EyeQ5 bindings to the existing Nomadik I2C dt-bindings. Add the two
EyeQ5-specific properties behind a conditional. Add an example for this
compatible.
Signed-off-by: Théo Lebrun <[email protected]>
---
.../devicetree/bindings/i2c/st,nomadik-i2c.yaml | 44 ++++++++++++++++++++--
1 file changed, 40 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
index e6b95e3765ac..eaade14b6d4c 100644
--- a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
@@ -14,9 +14,6 @@ description: The Nomadik I2C host controller began its life in the ST
maintainers:
- Linus Walleij <[email protected]>
-allOf:
- - $ref: /schemas/i2c/i2c-controller.yaml#
-
# Need a custom select here or 'arm,primecell' will match on lots of nodes
select:
properties:
@@ -24,6 +21,7 @@ select:
contains:
enum:
- st,nomadik-i2c
+ - mobileye,eyeq5-i2c
required:
- compatible
@@ -39,6 +37,10 @@ properties:
- const: stericsson,db8500-i2c
- const: st,nomadik-i2c
- const: arm,primecell
+ # The variant found on Mobileye EyeQ5
+ - items:
+ - const: mobileye,eyeq5-i2c
+ - const: arm,primecell
reg:
maxItems: 1
@@ -55,7 +57,7 @@ properties:
- items:
- const: mclk
- const: apb_pclk
- # Clock name in DB8500
+ # Clock name in DB8500 or EyeQ5
- items:
- const: i2cclk
- const: apb_pclk
@@ -83,6 +85,25 @@ required:
unevaluatedProperties: false
+allOf:
+ - $ref: /schemas/i2c/i2c-controller.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mobileye,eyeq5-i2c
+ then:
+ properties:
+ mobileye,olb:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: A phandle to the OLB syscon.
+ mobileye,id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Platform-wide controller ID (integer starting from zero).
+ required:
+ - mobileye,olb
+ - mobileye,id
+
examples:
- |
#include <dt-bindings/interrupt-controller/irq.h>
@@ -116,5 +137,20 @@ examples:
clocks = <&i2c0clk>, <&pclki2c0>;
clock-names = "mclk", "apb_pclk";
};
+ - |
+ #include <dt-bindings/interrupt-controller/mips-gic.h>
+ i2c@300000 {
+ compatible = "mobileye,eyeq5-i2c", "arm,primecell";
+ reg = <0x300000 0x1000>;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SHARED 1 IRQ_TYPE_LEVEL_HIGH>;
+ clock-frequency = <400000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clocks = <&i2c_ser_clk>, <&i2c_clk>;
+ clock-names = "i2cclk", "apb_pclk";
+ mobileye,olb = <&olb>;
+ mobileye,id = <0>;
+ };
...
--
2.43.1
Constant register bit fields are declared using hardcoded hex values;
replace them by calls to BIT() and GENMASK(). Replace custom GEN_MASK()
macro by the generic FIELD_PREP(). Replace manual bit manipulations by
the generic FIELD_GET() macro.
Signed-off-by: Théo Lebrun <[email protected]>
---
drivers/i2c/busses/i2c-nomadik.c | 150 ++++++++++++++++++++-------------------
1 file changed, 77 insertions(+), 73 deletions(-)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 80bdf7e42613..aa68ab402b10 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -9,6 +9,7 @@
* Author: Srinidhi Kasagar <[email protected]>
* Author: Sachin Verma <[email protected]>
*/
+#include <linux/bitfield.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/amba/bus.h>
@@ -42,54 +43,59 @@
#define I2C_ICR (0x038)
/* Control registers */
-#define I2C_CR_PE (0x1 << 0) /* Peripheral Enable */
-#define I2C_CR_OM (0x3 << 1) /* Operating mode */
-#define I2C_CR_SAM (0x1 << 3) /* Slave addressing mode */
-#define I2C_CR_SM (0x3 << 4) /* Speed mode */
-#define I2C_CR_SGCM (0x1 << 6) /* Slave general call mode */
-#define I2C_CR_FTX (0x1 << 7) /* Flush Transmit */
-#define I2C_CR_FRX (0x1 << 8) /* Flush Receive */
-#define I2C_CR_DMA_TX_EN (0x1 << 9) /* DMA Tx enable */
-#define I2C_CR_DMA_RX_EN (0x1 << 10) /* DMA Rx Enable */
-#define I2C_CR_DMA_SLE (0x1 << 11) /* DMA sync. logic enable */
-#define I2C_CR_LM (0x1 << 12) /* Loopback mode */
-#define I2C_CR_FON (0x3 << 13) /* Filtering on */
-#define I2C_CR_FS (0x3 << 15) /* Force stop enable */
+#define I2C_CR_PE BIT(0) /* Peripheral Enable */
+#define I2C_CR_OM GENMASK(2, 1) /* Operating mode */
+#define I2C_CR_SAM BIT(3) /* Slave addressing mode */
+#define I2C_CR_SM GENMASK(5, 4) /* Speed mode */
+#define I2C_CR_SGCM BIT(6) /* Slave general call mode */
+#define I2C_CR_FTX BIT(7) /* Flush Transmit */
+#define I2C_CR_FRX BIT(8) /* Flush Receive */
+#define I2C_CR_DMA_TX_EN BIT(9) /* DMA Tx enable */
+#define I2C_CR_DMA_RX_EN BIT(10) /* DMA Rx Enable */
+#define I2C_CR_DMA_SLE BIT(11) /* DMA sync. logic enable */
+#define I2C_CR_LM BIT(12) /* Loopback mode */
+#define I2C_CR_FON GENMASK(14, 13) /* Filtering on */
+#define I2C_CR_FS GENMASK(16, 15) /* Force stop enable */
+
+/* Slave control register (SCR) */
+#define I2C_SCR_SLSU GENMASK(31, 16) /* Slave data setup time */
/* Master controller (MCR) register */
-#define I2C_MCR_OP (0x1 << 0) /* Operation */
-#define I2C_MCR_A7 (0x7f << 1) /* 7-bit address */
-#define I2C_MCR_EA10 (0x7 << 8) /* 10-bit Extended address */
-#define I2C_MCR_SB (0x1 << 11) /* Extended address */
-#define I2C_MCR_AM (0x3 << 12) /* Address type */
-#define I2C_MCR_STOP (0x1 << 14) /* Stop condition */
-#define I2C_MCR_LENGTH (0x7ff << 15) /* Transaction length */
+#define I2C_MCR_OP BIT(0) /* Operation */
+#define I2C_MCR_A7 GENMASK(7, 1) /* 7-bit address */
+#define I2C_MCR_EA10 GENMASK(10, 8) /* 10-bit Extended address */
+#define I2C_MCR_SB BIT(11) /* Extended address */
+#define I2C_MCR_AM GENMASK(13, 12) /* Address type */
+#define I2C_MCR_STOP BIT(14) /* Stop condition */
+#define I2C_MCR_LENGTH GENMASK(25, 15) /* Transaction length */
/* Status register (SR) */
-#define I2C_SR_OP (0x3 << 0) /* Operation */
-#define I2C_SR_STATUS (0x3 << 2) /* controller status */
-#define I2C_SR_CAUSE (0x7 << 4) /* Abort cause */
-#define I2C_SR_TYPE (0x3 << 7) /* Receive type */
-#define I2C_SR_LENGTH (0x7ff << 9) /* Transfer length */
+#define I2C_SR_OP GENMASK(1, 0) /* Operation */
+#define I2C_SR_STATUS GENMASK(3, 2) /* controller status */
+#define I2C_SR_CAUSE GENMASK(6, 4) /* Abort cause */
+#define I2C_SR_TYPE GENMASK(8, 7) /* Receive type */
+#define I2C_SR_LENGTH GENMASK(19, 9) /* Transfer length */
+
+/* Baud-rate counter register (BRCR) */
+#define I2C_BRCR_BRCNT1 GENMASK(31, 16) /* Baud-rate counter 1 */
+#define I2C_BRCR_BRCNT2 GENMASK(15, 0) /* Baud-rate counter 2 */
/* Interrupt mask set/clear (IMSCR) bits */
-#define I2C_IT_TXFE (0x1 << 0)
-#define I2C_IT_TXFNE (0x1 << 1)
-#define I2C_IT_TXFF (0x1 << 2)
-#define I2C_IT_TXFOVR (0x1 << 3)
-#define I2C_IT_RXFE (0x1 << 4)
-#define I2C_IT_RXFNF (0x1 << 5)
-#define I2C_IT_RXFF (0x1 << 6)
-#define I2C_IT_RFSR (0x1 << 16)
-#define I2C_IT_RFSE (0x1 << 17)
-#define I2C_IT_WTSR (0x1 << 18)
-#define I2C_IT_MTD (0x1 << 19)
-#define I2C_IT_STD (0x1 << 20)
-#define I2C_IT_MAL (0x1 << 24)
-#define I2C_IT_BERR (0x1 << 25)
-#define I2C_IT_MTDWS (0x1 << 28)
-
-#define GEN_MASK(val, mask, sb) (((val) << (sb)) & (mask))
+#define I2C_IT_TXFE BIT(0)
+#define I2C_IT_TXFNE BIT(1)
+#define I2C_IT_TXFF BIT(2)
+#define I2C_IT_TXFOVR BIT(3)
+#define I2C_IT_RXFE BIT(4)
+#define I2C_IT_RXFNF BIT(5)
+#define I2C_IT_RXFF BIT(6)
+#define I2C_IT_RFSR BIT(16)
+#define I2C_IT_RFSE BIT(17)
+#define I2C_IT_WTSR BIT(18)
+#define I2C_IT_MTD BIT(19)
+#define I2C_IT_STD BIT(20)
+#define I2C_IT_MAL BIT(24)
+#define I2C_IT_BERR BIT(25)
+#define I2C_IT_MTDWS BIT(28)
/* some bits in ICR are reserved */
#define I2C_CLEAR_ALL_INTS 0x131f007f
@@ -284,7 +290,7 @@ static int init_hw(struct nmk_i2c_dev *priv)
}
/* enable peripheral, master mode operation */
-#define DEFAULT_I2C_REG_CR ((1 << 1) | I2C_CR_PE)
+#define DEFAULT_I2C_REG_CR (FIELD_PREP(I2C_CR_OM, 0b01) | I2C_CR_PE)
/**
* load_i2c_mcr_reg() - load the MCR register
@@ -296,41 +302,42 @@ static u32 load_i2c_mcr_reg(struct nmk_i2c_dev *priv, u16 flags)
u32 mcr = 0;
unsigned short slave_adr_3msb_bits;
- mcr |= GEN_MASK(priv->cli.slave_adr, I2C_MCR_A7, 1);
+ mcr |= FIELD_PREP(I2C_MCR_A7, priv->cli.slave_adr);
if (unlikely(flags & I2C_M_TEN)) {
/* 10-bit address transaction */
- mcr |= GEN_MASK(2, I2C_MCR_AM, 12);
+ mcr |= FIELD_PREP(I2C_MCR_AM, 2);
/*
* Get the top 3 bits.
* EA10 represents extended address in MCR. This includes
* the extension (MSB bits) of the 7 bit address loaded
* in A7
*/
- slave_adr_3msb_bits = (priv->cli.slave_adr >> 7) & 0x7;
+ slave_adr_3msb_bits = FIELD_GET(GENMASK(9, 7),
+ priv->cli.slave_adr);
- mcr |= GEN_MASK(slave_adr_3msb_bits, I2C_MCR_EA10, 8);
+ mcr |= FIELD_PREP(I2C_MCR_EA10, slave_adr_3msb_bits);
} else {
/* 7-bit address transaction */
- mcr |= GEN_MASK(1, I2C_MCR_AM, 12);
+ mcr |= FIELD_PREP(I2C_MCR_AM, 1);
}
/* start byte procedure not applied */
- mcr |= GEN_MASK(0, I2C_MCR_SB, 11);
+ mcr |= FIELD_PREP(I2C_MCR_SB, 0);
/* check the operation, master read/write? */
if (priv->cli.operation == I2C_WRITE)
- mcr |= GEN_MASK(I2C_WRITE, I2C_MCR_OP, 0);
+ mcr |= FIELD_PREP(I2C_MCR_OP, I2C_WRITE);
else
- mcr |= GEN_MASK(I2C_READ, I2C_MCR_OP, 0);
+ mcr |= FIELD_PREP(I2C_MCR_OP, I2C_READ);
/* stop or repeated start? */
if (priv->stop)
- mcr |= GEN_MASK(1, I2C_MCR_STOP, 14);
+ mcr |= FIELD_PREP(I2C_MCR_STOP, 1);
else
- mcr &= ~(GEN_MASK(1, I2C_MCR_STOP, 14));
+ mcr &= ~FIELD_PREP(I2C_MCR_STOP, 1);
- mcr |= GEN_MASK(priv->cli.count, I2C_MCR_LENGTH, 15);
+ mcr |= FIELD_PREP(I2C_MCR_LENGTH, priv->cli.count);
return mcr;
}
@@ -383,7 +390,7 @@ static void setup_i2c_controller(struct nmk_i2c_dev *priv)
slsu += 1;
dev_dbg(&priv->adev->dev, "calculated SLSU = %04x\n", slsu);
- writel(slsu << 16, priv->virtbase + I2C_SCR);
+ writel(FIELD_PREP(I2C_SCR_SLSU, slsu), priv->virtbase + I2C_SCR);
/*
* The spec says, in case of std. mode the divider is
@@ -399,8 +406,8 @@ static void setup_i2c_controller(struct nmk_i2c_dev *priv)
* plus operation. Currently we do not supprt high speed mode
* so set brcr1 to 0.
*/
- brcr1 = 0 << 16;
- brcr2 = (i2c_clk / (priv->clk_freq * div)) & 0xffff;
+ brcr1 = FIELD_PREP(I2C_BRCR_BRCNT1, 0);
+ brcr2 = FIELD_PREP(I2C_BRCR_BRCNT2, i2c_clk / (priv->clk_freq * div));
/* set the baud rate counter register */
writel((brcr1 | brcr2), priv->virtbase + I2C_BRCR);
@@ -414,12 +421,13 @@ static void setup_i2c_controller(struct nmk_i2c_dev *priv)
if (priv->sm > I2C_FREQ_MODE_FAST) {
dev_err(&priv->adev->dev,
"do not support this mode defaulting to std. mode\n");
- brcr2 = i2c_clk / (I2C_MAX_STANDARD_MODE_FREQ * 2) & 0xffff;
+ brcr2 = FIELD_PREP(I2C_BRCR_BRCNT2,
+ i2c_clk / (I2C_MAX_STANDARD_MODE_FREQ * 2));
writel((brcr1 | brcr2), priv->virtbase + I2C_BRCR);
- writel(I2C_FREQ_MODE_STANDARD << 4,
- priv->virtbase + I2C_CR);
+ writel(FIELD_PREP(I2C_CR_SM, I2C_FREQ_MODE_STANDARD),
+ priv->virtbase + I2C_CR);
}
- writel(priv->sm << 4, priv->virtbase + I2C_CR);
+ writel(FIELD_PREP(I2C_CR_SM, priv->sm), priv->virtbase + I2C_CR);
/* set the Tx and Rx FIFO threshold */
writel(priv->tft, priv->virtbase + I2C_TFTR);
@@ -583,13 +591,8 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *priv, u16 flags)
u32 cause;
i2c_sr = readl(priv->virtbase + I2C_SR);
- /*
- * Check if the controller I2C operation status
- * is set to ABORT(11b).
- */
- if (((i2c_sr >> 2) & 0x3) == 0x3) {
- /* get the abort cause */
- cause = (i2c_sr >> 4) & 0x7;
+ if (FIELD_GET(I2C_SR_STATUS, i2c_sr) == I2C_ABORT) {
+ cause = FIELD_GET(I2C_SR_CAUSE, i2c_sr);
dev_err(&priv->adev->dev, "%s\n",
cause >= ARRAY_SIZE(abort_causes) ?
"unknown reason" :
@@ -730,7 +733,7 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
misr = readl(priv->virtbase + I2C_MISR);
src = __ffs(misr);
- switch ((1 << src)) {
+ switch (BIT(src)) {
/* Transmit FIFO nearly empty interrupt */
case I2C_IT_TXFNE:
@@ -824,15 +827,16 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
* during the transaction.
*/
case I2C_IT_BERR:
+ {
+ u32 sr = readl(priv->virtbase + I2C_SR);
priv->result = -EIO;
- /* get the status */
- if (((readl(priv->virtbase + I2C_SR) >> 2) & 0x3) == I2C_ABORT)
+ if (FIELD_GET(I2C_SR_STATUS, sr) == I2C_ABORT)
init_hw(priv);
i2c_set_bit(priv->virtbase + I2C_ICR, I2C_IT_BERR);
complete(&priv->xfer_complete);
-
- break;
+ }
+ break;
/*
* Tx FIFO overrun interrupt.
--
2.43.1
Disambiguate the usage of dev as a variable name; it is usually best to
keep it reserved for struct device pointers. Avoid having multiple
names for the same struct pointer (previously: dev, nmk, nmk_i2c).
Signed-off-by: Théo Lebrun <[email protected]>
---
drivers/i2c/busses/i2c-nomadik.c | 428 +++++++++++++++++++--------------------
1 file changed, 214 insertions(+), 214 deletions(-)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index b10574d42b7a..cd511c884f99 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -206,12 +206,12 @@ static inline void i2c_clr_bit(void __iomem *reg, u32 mask)
/**
* flush_i2c_fifo() - This function flushes the I2C FIFO
- * @dev: private data of I2C Driver
+ * @priv: private data of I2C Driver
*
* This function flushes the I2C Tx and Rx FIFOs. It returns
* 0 on successful flushing of FIFO
*/
-static int flush_i2c_fifo(struct nmk_i2c_dev *dev)
+static int flush_i2c_fifo(struct nmk_i2c_dev *priv)
{
#define LOOP_ATTEMPTS 10
int i;
@@ -224,19 +224,19 @@ static int flush_i2c_fifo(struct nmk_i2c_dev *dev)
* bits, until then no one must access Tx, Rx FIFO and
* should poll on these bits waiting for the completion.
*/
- writel((I2C_CR_FTX | I2C_CR_FRX), dev->virtbase + I2C_CR);
+ writel((I2C_CR_FTX | I2C_CR_FRX), priv->virtbase + I2C_CR);
for (i = 0; i < LOOP_ATTEMPTS; i++) {
- timeout = jiffies + dev->adap.timeout;
+ timeout = jiffies + priv->adap.timeout;
while (!time_after(jiffies, timeout)) {
- if ((readl(dev->virtbase + I2C_CR) &
+ if ((readl(priv->virtbase + I2C_CR) &
(I2C_CR_FTX | I2C_CR_FRX)) == 0)
- return 0;
+ return 0;
}
}
- dev_err(&dev->adev->dev,
+ dev_err(&priv->adev->dev,
"flushing operation timed out giving up after %d attempts",
LOOP_ATTEMPTS);
@@ -245,45 +245,45 @@ static int flush_i2c_fifo(struct nmk_i2c_dev *dev)
/**
* disable_all_interrupts() - Disable all interrupts of this I2c Bus
- * @dev: private data of I2C Driver
+ * @priv: private data of I2C Driver
*/
-static void disable_all_interrupts(struct nmk_i2c_dev *dev)
+static void disable_all_interrupts(struct nmk_i2c_dev *priv)
{
u32 mask = IRQ_MASK(0);
- writel(mask, dev->virtbase + I2C_IMSCR);
+ writel(mask, priv->virtbase + I2C_IMSCR);
}
/**
* clear_all_interrupts() - Clear all interrupts of I2C Controller
- * @dev: private data of I2C Driver
+ * @priv: private data of I2C Driver
*/
-static void clear_all_interrupts(struct nmk_i2c_dev *dev)
+static void clear_all_interrupts(struct nmk_i2c_dev *priv)
{
u32 mask;
mask = IRQ_MASK(I2C_CLEAR_ALL_INTS);
- writel(mask, dev->virtbase + I2C_ICR);
+ writel(mask, priv->virtbase + I2C_ICR);
}
/**
* init_hw() - initialize the I2C hardware
- * @dev: private data of I2C Driver
+ * @priv: private data of I2C Driver
*/
-static int init_hw(struct nmk_i2c_dev *dev)
+static int init_hw(struct nmk_i2c_dev *priv)
{
int stat;
- stat = flush_i2c_fifo(dev);
+ stat = flush_i2c_fifo(priv);
if (stat)
goto exit;
/* disable the controller */
- i2c_clr_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
+ i2c_clr_bit(priv->virtbase + I2C_CR, I2C_CR_PE);
- disable_all_interrupts(dev);
+ disable_all_interrupts(priv);
- clear_all_interrupts(dev);
+ clear_all_interrupts(priv);
- dev->cli.operation = I2C_NO_OPERATION;
+ priv->cli.operation = I2C_NO_OPERATION;
exit:
return stat;
@@ -294,15 +294,15 @@ static int init_hw(struct nmk_i2c_dev *dev)
/**
* load_i2c_mcr_reg() - load the MCR register
- * @dev: private data of controller
+ * @priv: private data of controller
* @flags: message flags
*/
-static u32 load_i2c_mcr_reg(struct nmk_i2c_dev *dev, u16 flags)
+static u32 load_i2c_mcr_reg(struct nmk_i2c_dev *priv, u16 flags)
{
u32 mcr = 0;
unsigned short slave_adr_3msb_bits;
- mcr |= GEN_MASK(dev->cli.slave_adr, I2C_MCR_A7, 1);
+ mcr |= GEN_MASK(priv->cli.slave_adr, I2C_MCR_A7, 1);
if (unlikely(flags & I2C_M_TEN)) {
/* 10-bit address transaction */
@@ -313,7 +313,7 @@ static u32 load_i2c_mcr_reg(struct nmk_i2c_dev *dev, u16 flags)
* the extension (MSB bits) of the 7 bit address loaded
* in A7
*/
- slave_adr_3msb_bits = (dev->cli.slave_adr >> 7) & 0x7;
+ slave_adr_3msb_bits = (priv->cli.slave_adr >> 7) & 0x7;
mcr |= GEN_MASK(slave_adr_3msb_bits, I2C_MCR_EA10, 8);
} else {
@@ -325,40 +325,40 @@ static u32 load_i2c_mcr_reg(struct nmk_i2c_dev *dev, u16 flags)
mcr |= GEN_MASK(0, I2C_MCR_SB, 11);
/* check the operation, master read/write? */
- if (dev->cli.operation == I2C_WRITE)
+ if (priv->cli.operation == I2C_WRITE)
mcr |= GEN_MASK(I2C_WRITE, I2C_MCR_OP, 0);
else
mcr |= GEN_MASK(I2C_READ, I2C_MCR_OP, 0);
/* stop or repeated start? */
- if (dev->stop)
+ if (priv->stop)
mcr |= GEN_MASK(1, I2C_MCR_STOP, 14);
else
mcr &= ~(GEN_MASK(1, I2C_MCR_STOP, 14));
- mcr |= GEN_MASK(dev->cli.count, I2C_MCR_LENGTH, 15);
+ mcr |= GEN_MASK(priv->cli.count, I2C_MCR_LENGTH, 15);
return mcr;
}
/**
* setup_i2c_controller() - setup the controller
- * @dev: private data of controller
+ * @priv: private data of controller
*/
-static void setup_i2c_controller(struct nmk_i2c_dev *dev)
+static void setup_i2c_controller(struct nmk_i2c_dev *priv)
{
u32 brcr1, brcr2;
u32 i2c_clk, div;
u32 ns;
u16 slsu;
- writel(0x0, dev->virtbase + I2C_CR);
- writel(0x0, dev->virtbase + I2C_HSMCR);
- writel(0x0, dev->virtbase + I2C_TFTR);
- writel(0x0, dev->virtbase + I2C_RFTR);
- writel(0x0, dev->virtbase + I2C_DMAR);
+ writel(0x0, priv->virtbase + I2C_CR);
+ writel(0x0, priv->virtbase + I2C_HSMCR);
+ writel(0x0, priv->virtbase + I2C_TFTR);
+ writel(0x0, priv->virtbase + I2C_RFTR);
+ writel(0x0, priv->virtbase + I2C_DMAR);
- i2c_clk = clk_get_rate(dev->clk);
+ i2c_clk = clk_get_rate(priv->clk);
/*
* set the slsu:
@@ -373,7 +373,7 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
* slsu = cycles / (1000000000 / f) + 1
*/
ns = DIV_ROUND_UP_ULL(1000000000ULL, i2c_clk);
- switch (dev->sm) {
+ switch (priv->sm) {
case I2C_FREQ_MODE_FAST:
case I2C_FREQ_MODE_FAST_PLUS:
slsu = DIV_ROUND_UP(100, ns); /* Fast */
@@ -388,15 +388,15 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
}
slsu += 1;
- dev_dbg(&dev->adev->dev, "calculated SLSU = %04x\n", slsu);
- writel(slsu << 16, dev->virtbase + I2C_SCR);
+ dev_dbg(&priv->adev->dev, "calculated SLSU = %04x\n", slsu);
+ writel(slsu << 16, priv->virtbase + I2C_SCR);
/*
* The spec says, in case of std. mode the divider is
* 2 whereas it is 3 for fast and fastplus mode of
* operation. TODO - high speed support.
*/
- div = (dev->clk_freq > I2C_MAX_STANDARD_MODE_FREQ) ? 3 : 2;
+ div = (priv->clk_freq > I2C_MAX_STANDARD_MODE_FREQ) ? 3 : 2;
/*
* generate the mask for baud rate counters. The controller
@@ -406,10 +406,10 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
* so set brcr1 to 0.
*/
brcr1 = 0 << 16;
- brcr2 = (i2c_clk/(dev->clk_freq * div)) & 0xffff;
+ brcr2 = (i2c_clk / (priv->clk_freq * div)) & 0xffff;
/* set the baud rate counter register */
- writel((brcr1 | brcr2), dev->virtbase + I2C_BRCR);
+ writel((brcr1 | brcr2), priv->virtbase + I2C_BRCR);
/*
* set the speed mode. Currently we support
@@ -417,125 +417,124 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
* TODO - support for fast mode plus (up to 1Mb/s)
* and high speed (up to 3.4 Mb/s)
*/
- if (dev->sm > I2C_FREQ_MODE_FAST) {
- dev_err(&dev->adev->dev,
+ if (priv->sm > I2C_FREQ_MODE_FAST) {
+ dev_err(&priv->adev->dev,
"do not support this mode defaulting to std. mode\n");
brcr2 = i2c_clk / (I2C_MAX_STANDARD_MODE_FREQ * 2) & 0xffff;
- writel((brcr1 | brcr2), dev->virtbase + I2C_BRCR);
+ writel((brcr1 | brcr2), priv->virtbase + I2C_BRCR);
writel(I2C_FREQ_MODE_STANDARD << 4,
- dev->virtbase + I2C_CR);
+ priv->virtbase + I2C_CR);
}
- writel(dev->sm << 4, dev->virtbase + I2C_CR);
+ writel(priv->sm << 4, priv->virtbase + I2C_CR);
/* set the Tx and Rx FIFO threshold */
- writel(dev->tft, dev->virtbase + I2C_TFTR);
- writel(dev->rft, dev->virtbase + I2C_RFTR);
+ writel(priv->tft, priv->virtbase + I2C_TFTR);
+ writel(priv->rft, priv->virtbase + I2C_RFTR);
}
/**
* read_i2c() - Read from I2C client device
- * @dev: private data of I2C Driver
+ * @priv: private data of I2C Driver
* @flags: message flags
*
* This function reads from i2c client device when controller is in
* master mode. There is a completion timeout. If there is no transfer
* before timeout error is returned.
*/
-static int read_i2c(struct nmk_i2c_dev *dev, u16 flags)
+static int read_i2c(struct nmk_i2c_dev *priv, u16 flags)
{
int status = 0;
u32 mcr, irq_mask;
unsigned long timeout;
- mcr = load_i2c_mcr_reg(dev, flags);
- writel(mcr, dev->virtbase + I2C_MCR);
+ mcr = load_i2c_mcr_reg(priv, flags);
+ writel(mcr, priv->virtbase + I2C_MCR);
/* load the current CR value */
- writel(readl(dev->virtbase + I2C_CR) | DEFAULT_I2C_REG_CR,
- dev->virtbase + I2C_CR);
+ writel(readl(priv->virtbase + I2C_CR) | DEFAULT_I2C_REG_CR,
+ priv->virtbase + I2C_CR);
/* enable the controller */
- i2c_set_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
+ i2c_set_bit(priv->virtbase + I2C_CR, I2C_CR_PE);
- init_completion(&dev->xfer_complete);
+ init_completion(&priv->xfer_complete);
/* enable interrupts by setting the mask */
irq_mask = (I2C_IT_RXFNF | I2C_IT_RXFF |
I2C_IT_MAL | I2C_IT_BERR);
- if (dev->stop || !dev->vendor->has_mtdws)
+ if (priv->stop || !priv->vendor->has_mtdws)
irq_mask |= I2C_IT_MTD;
else
irq_mask |= I2C_IT_MTDWS;
irq_mask = I2C_CLEAR_ALL_INTS & IRQ_MASK(irq_mask);
- writel(readl(dev->virtbase + I2C_IMSCR) | irq_mask,
- dev->virtbase + I2C_IMSCR);
+ writel(readl(priv->virtbase + I2C_IMSCR) | irq_mask,
+ priv->virtbase + I2C_IMSCR);
timeout = wait_for_completion_timeout(
- &dev->xfer_complete, dev->adap.timeout);
+ &priv->xfer_complete, priv->adap.timeout);
if (timeout == 0) {
/* Controller timed out */
- dev_err(&dev->adev->dev, "read from slave 0x%x timed out\n",
- dev->cli.slave_adr);
+ dev_err(&priv->adev->dev, "read from slave 0x%x timed out\n",
+ priv->cli.slave_adr);
status = -ETIMEDOUT;
}
return status;
}
-static void fill_tx_fifo(struct nmk_i2c_dev *dev, int no_bytes)
+static void fill_tx_fifo(struct nmk_i2c_dev *priv, int no_bytes)
{
int count;
for (count = (no_bytes - 2);
(count > 0) &&
- (dev->cli.count != 0);
+ (priv->cli.count != 0);
count--) {
/* write to the Tx FIFO */
- writeb(*dev->cli.buffer,
- dev->virtbase + I2C_TFR);
- dev->cli.buffer++;
- dev->cli.count--;
- dev->cli.xfer_bytes++;
+ writeb(*priv->cli.buffer, priv->virtbase + I2C_TFR);
+ priv->cli.buffer++;
+ priv->cli.count--;
+ priv->cli.xfer_bytes++;
}
}
/**
* write_i2c() - Write data to I2C client.
- * @dev: private data of I2C Driver
+ * @priv: private data of I2C Driver
* @flags: message flags
*
* This function writes data to I2C client
*/
-static int write_i2c(struct nmk_i2c_dev *dev, u16 flags)
+static int write_i2c(struct nmk_i2c_dev *priv, u16 flags)
{
u32 status = 0;
u32 mcr, irq_mask;
unsigned long timeout;
- mcr = load_i2c_mcr_reg(dev, flags);
+ mcr = load_i2c_mcr_reg(priv, flags);
- writel(mcr, dev->virtbase + I2C_MCR);
+ writel(mcr, priv->virtbase + I2C_MCR);
/* load the current CR value */
- writel(readl(dev->virtbase + I2C_CR) | DEFAULT_I2C_REG_CR,
- dev->virtbase + I2C_CR);
+ writel(readl(priv->virtbase + I2C_CR) | DEFAULT_I2C_REG_CR,
+ priv->virtbase + I2C_CR);
/* enable the controller */
- i2c_set_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
+ i2c_set_bit(priv->virtbase + I2C_CR, I2C_CR_PE);
- init_completion(&dev->xfer_complete);
+ init_completion(&priv->xfer_complete);
/* enable interrupts by settings the masks */
irq_mask = (I2C_IT_TXFOVR | I2C_IT_MAL | I2C_IT_BERR);
/* Fill the TX FIFO with transmit data */
- fill_tx_fifo(dev, MAX_I2C_FIFO_THRESHOLD);
+ fill_tx_fifo(priv, MAX_I2C_FIFO_THRESHOLD);
- if (dev->cli.count != 0)
+ if (priv->cli.count != 0)
irq_mask |= I2C_IT_TXFNE;
/*
@@ -543,23 +542,23 @@ static int write_i2c(struct nmk_i2c_dev *dev, u16 flags)
* set the MTDWS bit (Master Transaction Done Without Stop)
* to start repeated start operation
*/
- if (dev->stop || !dev->vendor->has_mtdws)
+ if (priv->stop || !priv->vendor->has_mtdws)
irq_mask |= I2C_IT_MTD;
else
irq_mask |= I2C_IT_MTDWS;
irq_mask = I2C_CLEAR_ALL_INTS & IRQ_MASK(irq_mask);
- writel(readl(dev->virtbase + I2C_IMSCR) | irq_mask,
- dev->virtbase + I2C_IMSCR);
+ writel(readl(priv->virtbase + I2C_IMSCR) | irq_mask,
+ priv->virtbase + I2C_IMSCR);
timeout = wait_for_completion_timeout(
- &dev->xfer_complete, dev->adap.timeout);
+ &priv->xfer_complete, priv->adap.timeout);
if (timeout == 0) {
/* Controller timed out */
- dev_err(&dev->adev->dev, "write to slave 0x%x timed out\n",
- dev->cli.slave_adr);
+ dev_err(&priv->adev->dev, "write to slave 0x%x timed out\n",
+ priv->cli.slave_adr);
status = -ETIMEDOUT;
}
@@ -568,28 +567,28 @@ static int write_i2c(struct nmk_i2c_dev *dev, u16 flags)
/**
* nmk_i2c_xfer_one() - transmit a single I2C message
- * @dev: device with a message encoded into it
+ * @priv: device with a message encoded into it
* @flags: message flags
*/
-static int nmk_i2c_xfer_one(struct nmk_i2c_dev *dev, u16 flags)
+static int nmk_i2c_xfer_one(struct nmk_i2c_dev *priv, u16 flags)
{
int status;
if (flags & I2C_M_RD) {
/* read operation */
- dev->cli.operation = I2C_READ;
- status = read_i2c(dev, flags);
+ priv->cli.operation = I2C_READ;
+ status = read_i2c(priv, flags);
} else {
/* write operation */
- dev->cli.operation = I2C_WRITE;
- status = write_i2c(dev, flags);
+ priv->cli.operation = I2C_WRITE;
+ status = write_i2c(priv, flags);
}
- if (status || (dev->result)) {
+ if (status || priv->result) {
u32 i2c_sr;
u32 cause;
- i2c_sr = readl(dev->virtbase + I2C_SR);
+ i2c_sr = readl(priv->virtbase + I2C_SR);
/*
* Check if the controller I2C operation status
* is set to ABORT(11b).
@@ -597,15 +596,15 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *dev, u16 flags)
if (((i2c_sr >> 2) & 0x3) == 0x3) {
/* get the abort cause */
cause = (i2c_sr >> 4) & 0x7;
- dev_err(&dev->adev->dev, "%s\n",
+ dev_err(&priv->adev->dev, "%s\n",
cause >= ARRAY_SIZE(abort_causes) ?
"unknown reason" :
abort_causes[cause]);
}
- (void) init_hw(dev);
+ init_hw(priv);
- status = status ? status : dev->result;
+ status = status ? status : priv->result;
}
return status;
@@ -663,24 +662,24 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
{
int status = 0;
int i;
- struct nmk_i2c_dev *dev = i2c_get_adapdata(i2c_adap);
+ struct nmk_i2c_dev *priv = i2c_get_adapdata(i2c_adap);
int j;
- pm_runtime_get_sync(&dev->adev->dev);
+ pm_runtime_get_sync(&priv->adev->dev);
/* Attempt three times to send the message queue */
for (j = 0; j < 3; j++) {
/* setup the i2c controller */
- setup_i2c_controller(dev);
+ setup_i2c_controller(priv);
for (i = 0; i < num_msgs; i++) {
- dev->cli.slave_adr = msgs[i].addr;
- dev->cli.buffer = msgs[i].buf;
- dev->cli.count = msgs[i].len;
- dev->stop = (i < (num_msgs - 1)) ? 0 : 1;
- dev->result = 0;
+ priv->cli.slave_adr = msgs[i].addr;
+ priv->cli.buffer = msgs[i].buf;
+ priv->cli.count = msgs[i].len;
+ priv->stop = (i < (num_msgs - 1)) ? 0 : 1;
+ priv->result = 0;
- status = nmk_i2c_xfer_one(dev, msgs[i].flags);
+ status = nmk_i2c_xfer_one(priv, msgs[i].flags);
if (status != 0)
break;
}
@@ -688,7 +687,7 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
break;
}
- pm_runtime_put_sync(&dev->adev->dev);
+ pm_runtime_put_sync(&priv->adev->dev);
/* return the no. messages processed */
if (status)
@@ -699,14 +698,14 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
/**
* disable_interrupts() - disable the interrupts
- * @dev: private data of controller
+ * @priv: private data of controller
* @irq: interrupt number
*/
-static int disable_interrupts(struct nmk_i2c_dev *dev, u32 irq)
+static int disable_interrupts(struct nmk_i2c_dev *priv, u32 irq)
{
irq = IRQ_MASK(irq);
- writel(readl(dev->virtbase + I2C_IMSCR) & ~(I2C_CLEAR_ALL_INTS & irq),
- dev->virtbase + I2C_IMSCR);
+ writel(readl(priv->virtbase + I2C_IMSCR) & ~(I2C_CLEAR_ALL_INTS & irq),
+ priv->virtbase + I2C_IMSCR);
return 0;
}
@@ -723,17 +722,18 @@ static int disable_interrupts(struct nmk_i2c_dev *dev, u32 irq)
*/
static irqreturn_t i2c_irq_handler(int irq, void *arg)
{
- struct nmk_i2c_dev *dev = arg;
+ struct nmk_i2c_dev *priv = arg;
+ struct device *dev = &priv->adev->dev;
u32 tft, rft;
u32 count;
u32 misr, src;
/* load Tx FIFO and Rx FIFO threshold values */
- tft = readl(dev->virtbase + I2C_TFTR);
- rft = readl(dev->virtbase + I2C_RFTR);
+ tft = readl(priv->virtbase + I2C_TFTR);
+ rft = readl(priv->virtbase + I2C_RFTR);
/* read interrupt status register */
- misr = readl(dev->virtbase + I2C_MISR);
+ misr = readl(priv->virtbase + I2C_MISR);
src = __ffs(misr);
switch ((1 << src)) {
@@ -741,20 +741,20 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
/* Transmit FIFO nearly empty interrupt */
case I2C_IT_TXFNE:
{
- if (dev->cli.operation == I2C_READ) {
+ if (priv->cli.operation == I2C_READ) {
/*
* in read operation why do we care for writing?
* so disable the Transmit FIFO interrupt
*/
- disable_interrupts(dev, I2C_IT_TXFNE);
+ disable_interrupts(priv, I2C_IT_TXFNE);
} else {
- fill_tx_fifo(dev, (MAX_I2C_FIFO_THRESHOLD - tft));
+ fill_tx_fifo(priv, (MAX_I2C_FIFO_THRESHOLD - tft));
/*
* if done, close the transfer by disabling the
* corresponding TXFNE interrupt
*/
- if (dev->cli.count == 0)
- disable_interrupts(dev, I2C_IT_TXFNE);
+ if (priv->cli.count == 0)
+ disable_interrupts(priv, I2C_IT_TXFNE);
}
}
break;
@@ -768,60 +768,59 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
case I2C_IT_RXFNF:
for (count = rft; count > 0; count--) {
/* Read the Rx FIFO */
- *dev->cli.buffer = readb(dev->virtbase + I2C_RFR);
- dev->cli.buffer++;
+ *priv->cli.buffer = readb(priv->virtbase + I2C_RFR);
+ priv->cli.buffer++;
}
- dev->cli.count -= rft;
- dev->cli.xfer_bytes += rft;
+ priv->cli.count -= rft;
+ priv->cli.xfer_bytes += rft;
break;
/* Rx FIFO full */
case I2C_IT_RXFF:
for (count = MAX_I2C_FIFO_THRESHOLD; count > 0; count--) {
- *dev->cli.buffer = readb(dev->virtbase + I2C_RFR);
- dev->cli.buffer++;
+ *priv->cli.buffer = readb(priv->virtbase + I2C_RFR);
+ priv->cli.buffer++;
}
- dev->cli.count -= MAX_I2C_FIFO_THRESHOLD;
- dev->cli.xfer_bytes += MAX_I2C_FIFO_THRESHOLD;
+ priv->cli.count -= MAX_I2C_FIFO_THRESHOLD;
+ priv->cli.xfer_bytes += MAX_I2C_FIFO_THRESHOLD;
break;
/* Master Transaction Done with/without stop */
case I2C_IT_MTD:
case I2C_IT_MTDWS:
- if (dev->cli.operation == I2C_READ) {
- while (!(readl(dev->virtbase + I2C_RISR)
+ if (priv->cli.operation == I2C_READ) {
+ while (!(readl(priv->virtbase + I2C_RISR)
& I2C_IT_RXFE)) {
- if (dev->cli.count == 0)
+ if (priv->cli.count == 0)
break;
- *dev->cli.buffer =
- readb(dev->virtbase + I2C_RFR);
- dev->cli.buffer++;
- dev->cli.count--;
- dev->cli.xfer_bytes++;
+ *priv->cli.buffer =
+ readb(priv->virtbase + I2C_RFR);
+ priv->cli.buffer++;
+ priv->cli.count--;
+ priv->cli.xfer_bytes++;
}
}
- disable_all_interrupts(dev);
- clear_all_interrupts(dev);
+ disable_all_interrupts(priv);
+ clear_all_interrupts(priv);
- if (dev->cli.count) {
- dev->result = -EIO;
- dev_err(&dev->adev->dev,
- "%lu bytes still remain to be xfered\n",
- dev->cli.count);
- (void) init_hw(dev);
+ if (priv->cli.count) {
+ priv->result = -EIO;
+ dev_err(dev, "%lu bytes still remain to be xfered\n",
+ priv->cli.count);
+ init_hw(priv);
}
- complete(&dev->xfer_complete);
+ complete(&priv->xfer_complete);
break;
/* Master Arbitration lost interrupt */
case I2C_IT_MAL:
- dev->result = -EIO;
- (void) init_hw(dev);
+ priv->result = -EIO;
+ init_hw(priv);
- i2c_set_bit(dev->virtbase + I2C_ICR, I2C_IT_MAL);
- complete(&dev->xfer_complete);
+ i2c_set_bit(priv->virtbase + I2C_ICR, I2C_IT_MAL);
+ complete(&priv->xfer_complete);
break;
@@ -831,13 +830,13 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
* during the transaction.
*/
case I2C_IT_BERR:
- dev->result = -EIO;
+ priv->result = -EIO;
/* get the status */
- if (((readl(dev->virtbase + I2C_SR) >> 2) & 0x3) == I2C_ABORT)
- (void) init_hw(dev);
+ if (((readl(priv->virtbase + I2C_SR) >> 2) & 0x3) == I2C_ABORT)
+ init_hw(priv);
- i2c_set_bit(dev->virtbase + I2C_ICR, I2C_IT_BERR);
- complete(&dev->xfer_complete);
+ i2c_set_bit(priv->virtbase + I2C_ICR, I2C_IT_BERR);
+ complete(&priv->xfer_complete);
break;
@@ -847,11 +846,11 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
* the Tx FIFO is full.
*/
case I2C_IT_TXFOVR:
- dev->result = -EIO;
- (void) init_hw(dev);
+ priv->result = -EIO;
+ init_hw(priv);
- dev_err(&dev->adev->dev, "Tx Fifo Over run\n");
- complete(&dev->xfer_complete);
+ dev_err(dev, "Tx Fifo Over run\n");
+ complete(&priv->xfer_complete);
break;
@@ -863,10 +862,10 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
case I2C_IT_RFSE:
case I2C_IT_WTSR:
case I2C_IT_STD:
- dev_err(&dev->adev->dev, "unhandled Interrupt\n");
+ dev_err(dev, "unhandled Interrupt\n");
break;
default:
- dev_err(&dev->adev->dev, "spurious Interrupt..\n");
+ dev_err(dev, "spurious Interrupt..\n");
break;
}
@@ -893,9 +892,9 @@ static int nmk_i2c_resume_early(struct device *dev)
static int nmk_i2c_runtime_suspend(struct device *dev)
{
struct amba_device *adev = to_amba_device(dev);
- struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+ struct nmk_i2c_dev *priv = amba_get_drvdata(adev);
- clk_disable_unprepare(nmk_i2c->clk);
+ clk_disable_unprepare(priv->clk);
pinctrl_pm_select_idle_state(dev);
return 0;
}
@@ -903,10 +902,10 @@ static int nmk_i2c_runtime_suspend(struct device *dev)
static int nmk_i2c_runtime_resume(struct device *dev)
{
struct amba_device *adev = to_amba_device(dev);
- struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+ struct nmk_i2c_dev *priv = amba_get_drvdata(adev);
int ret;
- ret = clk_prepare_enable(nmk_i2c->clk);
+ ret = clk_prepare_enable(priv->clk);
if (ret) {
dev_err(dev, "can't prepare_enable clock\n");
return ret;
@@ -914,9 +913,9 @@ static int nmk_i2c_runtime_resume(struct device *dev)
pinctrl_pm_select_default_state(dev);
- ret = init_hw(nmk_i2c);
+ ret = init_hw(priv);
if (ret) {
- clk_disable_unprepare(nmk_i2c->clk);
+ clk_disable_unprepare(priv->clk);
pinctrl_pm_select_idle_state(dev);
}
@@ -939,107 +938,108 @@ static const struct i2c_algorithm nmk_i2c_algo = {
};
static void nmk_i2c_of_probe(struct device_node *np,
- struct nmk_i2c_dev *nmk)
+ struct nmk_i2c_dev *priv)
{
/* Default to 100 kHz if no frequency is given in the node */
- if (of_property_read_u32(np, "clock-frequency", &nmk->clk_freq))
- nmk->clk_freq = I2C_MAX_STANDARD_MODE_FREQ;
+ if (of_property_read_u32(np, "clock-frequency", &priv->clk_freq))
+ priv->clk_freq = I2C_MAX_STANDARD_MODE_FREQ;
/* This driver only supports 'standard' and 'fast' modes of operation. */
- if (nmk->clk_freq <= I2C_MAX_STANDARD_MODE_FREQ)
- nmk->sm = I2C_FREQ_MODE_STANDARD;
+ if (priv->clk_freq <= I2C_MAX_STANDARD_MODE_FREQ)
+ priv->sm = I2C_FREQ_MODE_STANDARD;
else
- nmk->sm = I2C_FREQ_MODE_FAST;
- nmk->tft = 1; /* Tx FIFO threshold */
- nmk->rft = 8; /* Rx FIFO threshold */
- nmk->timeout = 200; /* Slave response timeout(ms) */
+ priv->sm = I2C_FREQ_MODE_FAST;
+ priv->tft = 1; /* Tx FIFO threshold */
+ priv->rft = 8; /* Rx FIFO threshold */
+ priv->timeout = 200; /* Slave response timeout(ms) */
}
static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
{
int ret = 0;
+ struct nmk_i2c_dev *priv;
struct device_node *np = adev->dev.of_node;
- struct nmk_i2c_dev *dev;
+ struct device *dev = &adev->dev;
struct i2c_adapter *adap;
struct i2c_vendor_data *vendor = id->data;
u32 max_fifo_threshold = (vendor->fifodepth / 2) - 1;
- dev = devm_kzalloc(&adev->dev, sizeof(*dev), GFP_KERNEL);
- if (!dev)
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
return -ENOMEM;
- dev->vendor = vendor;
- dev->adev = adev;
- nmk_i2c_of_probe(np, dev);
+ priv->vendor = vendor;
+ priv->adev = adev;
+ nmk_i2c_of_probe(np, priv);
- if (dev->tft > max_fifo_threshold) {
- dev_warn(&adev->dev, "requested TX FIFO threshold %u, adjusted down to %u\n",
- dev->tft, max_fifo_threshold);
- dev->tft = max_fifo_threshold;
+ if (priv->tft > max_fifo_threshold) {
+ dev_warn(dev, "requested TX FIFO threshold %u, adjusted down to %u\n",
+ priv->tft, max_fifo_threshold);
+ priv->tft = max_fifo_threshold;
}
- if (dev->rft > max_fifo_threshold) {
- dev_warn(&adev->dev, "requested RX FIFO threshold %u, adjusted down to %u\n",
- dev->rft, max_fifo_threshold);
- dev->rft = max_fifo_threshold;
+ if (priv->rft > max_fifo_threshold) {
+ dev_warn(dev, "requested RX FIFO threshold %u, adjusted down to %u\n",
+ priv->rft, max_fifo_threshold);
+ priv->rft = max_fifo_threshold;
}
- amba_set_drvdata(adev, dev);
+ amba_set_drvdata(adev, priv);
- dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
- resource_size(&adev->res));
- if (!dev->virtbase)
+ priv->virtbase = devm_ioremap(dev, adev->res.start,
+ resource_size(&adev->res));
+ if (!priv->virtbase)
return -ENOMEM;
- dev->irq = adev->irq[0];
- ret = devm_request_irq(&adev->dev, dev->irq, i2c_irq_handler, 0,
- DRIVER_NAME, dev);
+ priv->irq = adev->irq[0];
+ ret = devm_request_irq(dev, priv->irq, i2c_irq_handler, 0,
+ DRIVER_NAME, priv);
if (ret)
- return dev_err_probe(&adev->dev, ret,
- "cannot claim the irq %d\n", dev->irq);
+ return dev_err_probe(dev, ret,
+ "cannot claim the irq %d\n", priv->irq);
- dev->clk = devm_clk_get_enabled(&adev->dev, NULL);
- if (IS_ERR(dev->clk))
- return dev_err_probe(&adev->dev, PTR_ERR(dev->clk),
+ priv->clk = devm_clk_get_enabled(dev, NULL);
+ if (IS_ERR(priv->clk))
+ return dev_err_probe(dev, PTR_ERR(priv->clk),
"could enable i2c clock\n");
- init_hw(dev);
+ init_hw(priv);
- adap = &dev->adap;
+ adap = &priv->adap;
adap->dev.of_node = np;
- adap->dev.parent = &adev->dev;
+ adap->dev.parent = dev;
adap->owner = THIS_MODULE;
adap->class = I2C_CLASS_DEPRECATED;
adap->algo = &nmk_i2c_algo;
- adap->timeout = msecs_to_jiffies(dev->timeout);
+ adap->timeout = msecs_to_jiffies(priv->timeout);
snprintf(adap->name, sizeof(adap->name),
"Nomadik I2C at %pR", &adev->res);
- i2c_set_adapdata(adap, dev);
+ i2c_set_adapdata(adap, priv);
- dev_info(&adev->dev,
+ dev_info(dev,
"initialize %s on virtual base %p\n",
- adap->name, dev->virtbase);
+ adap->name, priv->virtbase);
ret = i2c_add_adapter(adap);
if (ret)
return ret;
- pm_runtime_put(&adev->dev);
+ pm_runtime_put(dev);
return 0;
}
static void nmk_i2c_remove(struct amba_device *adev)
{
- struct nmk_i2c_dev *dev = amba_get_drvdata(adev);
+ struct nmk_i2c_dev *priv = amba_get_drvdata(adev);
- i2c_del_adapter(&dev->adap);
- flush_i2c_fifo(dev);
- disable_all_interrupts(dev);
- clear_all_interrupts(dev);
+ i2c_del_adapter(&priv->adap);
+ flush_i2c_fifo(priv);
+ disable_all_interrupts(priv);
+ clear_all_interrupts(priv);
/* disable the controller */
- i2c_clr_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
+ i2c_clr_bit(priv->virtbase + I2C_CR, I2C_CR_PE);
}
static struct i2c_vendor_data vendor_stn8815 = {
--
2.43.1
Replace the completion by a waitqueue for synchronization from IRQ
handler to task. For short timeouts, use hrtimers, else use timers.
Usecase: avoid blocking the I2C bus for too long when an issue occurs.
The threshold picked is one jiffy: if timeout is below that, use
hrtimers. This threshold is NOT configurable.
Implement behavior but do NOT change fetching of timeout. This means the
timeout is unchanged (200ms) and the hrtimer case will never trigger.
A waitqueue is used because it supports both desired timeout approaches.
See wait_event_timeout() and wait_event_hrtimeout(). An atomic boolean
serves as synchronization condition.
Signed-off-by: Théo Lebrun <[email protected]>
---
drivers/i2c/busses/i2c-nomadik.c | 70 +++++++++++++++++++++++++++-------------
1 file changed, 48 insertions(+), 22 deletions(-)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index aa68ab402b10..e68b8e0d7919 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -162,10 +162,11 @@ struct i2c_nmk_client {
* @clk_freq: clock frequency for the operation mode
* @tft: Tx FIFO Threshold in bytes
* @rft: Rx FIFO Threshold in bytes
- * @timeout: Slave response timeout (ms)
+ * @timeout_usecs: Slave response timeout
* @sm: speed mode
* @stop: stop condition.
- * @xfer_complete: acknowledge completion for a I2C message.
+ * @xfer_wq: xfer done wait queue.
+ * @xfer_done: xfer done boolean.
* @result: controller propogated result.
*/
struct nmk_i2c_dev {
@@ -179,10 +180,11 @@ struct nmk_i2c_dev {
u32 clk_freq;
unsigned char tft;
unsigned char rft;
- int timeout;
+ int timeout_usecs;
enum i2c_freq_mode sm;
int stop;
- struct completion xfer_complete;
+ struct wait_queue_head xfer_wq;
+ bool xfer_done;
int result;
};
@@ -434,6 +436,22 @@ static void setup_i2c_controller(struct nmk_i2c_dev *priv)
writel(priv->rft, priv->virtbase + I2C_RFTR);
}
+static bool nmk_i2c_wait_xfer_done(struct nmk_i2c_dev *priv)
+{
+ if (priv->timeout_usecs < jiffies_to_usecs(1)) {
+ unsigned long timeout_usecs = priv->timeout_usecs;
+ ktime_t timeout = ktime_set(0, timeout_usecs * NSEC_PER_USEC);
+
+ wait_event_hrtimeout(priv->xfer_wq, priv->xfer_done, timeout);
+ } else {
+ unsigned long timeout = usecs_to_jiffies(priv->timeout_usecs);
+
+ wait_event_timeout(priv->xfer_wq, priv->xfer_done, timeout);
+ }
+
+ return priv->xfer_done;
+}
+
/**
* read_i2c() - Read from I2C client device
* @priv: private data of I2C Driver
@@ -445,9 +463,9 @@ static void setup_i2c_controller(struct nmk_i2c_dev *priv)
*/
static int read_i2c(struct nmk_i2c_dev *priv, u16 flags)
{
- int status = 0;
u32 mcr, irq_mask;
- unsigned long timeout;
+ int status = 0;
+ bool xfer_done;
mcr = load_i2c_mcr_reg(priv, flags);
writel(mcr, priv->virtbase + I2C_MCR);
@@ -459,7 +477,8 @@ static int read_i2c(struct nmk_i2c_dev *priv, u16 flags)
/* enable the controller */
i2c_set_bit(priv->virtbase + I2C_CR, I2C_CR_PE);
- init_completion(&priv->xfer_complete);
+ init_waitqueue_head(&priv->xfer_wq);
+ priv->xfer_done = false;
/* enable interrupts by setting the mask */
irq_mask = (I2C_IT_RXFNF | I2C_IT_RXFF |
@@ -475,10 +494,9 @@ static int read_i2c(struct nmk_i2c_dev *priv, u16 flags)
writel(readl(priv->virtbase + I2C_IMSCR) | irq_mask,
priv->virtbase + I2C_IMSCR);
- timeout = wait_for_completion_timeout(
- &priv->xfer_complete, priv->adap.timeout);
+ xfer_done = nmk_i2c_wait_xfer_done(priv);
- if (timeout == 0) {
+ if (!xfer_done) {
/* Controller timed out */
dev_err(&priv->adev->dev, "read from slave 0x%x timed out\n",
priv->cli.slave_adr);
@@ -513,9 +531,9 @@ static void fill_tx_fifo(struct nmk_i2c_dev *priv, int no_bytes)
*/
static int write_i2c(struct nmk_i2c_dev *priv, u16 flags)
{
- u32 status = 0;
u32 mcr, irq_mask;
- unsigned long timeout;
+ u32 status = 0;
+ bool xfer_done;
mcr = load_i2c_mcr_reg(priv, flags);
@@ -528,7 +546,8 @@ static int write_i2c(struct nmk_i2c_dev *priv, u16 flags)
/* enable the controller */
i2c_set_bit(priv->virtbase + I2C_CR, I2C_CR_PE);
- init_completion(&priv->xfer_complete);
+ init_waitqueue_head(&priv->xfer_wq);
+ priv->xfer_done = false;
/* enable interrupts by settings the masks */
irq_mask = (I2C_IT_TXFOVR | I2C_IT_MAL | I2C_IT_BERR);
@@ -554,10 +573,9 @@ static int write_i2c(struct nmk_i2c_dev *priv, u16 flags)
writel(readl(priv->virtbase + I2C_IMSCR) | irq_mask,
priv->virtbase + I2C_IMSCR);
- timeout = wait_for_completion_timeout(
- &priv->xfer_complete, priv->adap.timeout);
+ xfer_done = nmk_i2c_wait_xfer_done(priv);
- if (timeout == 0) {
+ if (!xfer_done) {
/* Controller timed out */
dev_err(&priv->adev->dev, "write to slave 0x%x timed out\n",
priv->cli.slave_adr);
@@ -807,7 +825,9 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
priv->cli.count);
init_hw(priv);
}
- complete(&priv->xfer_complete);
+ priv->xfer_done = true;
+ wake_up(&priv->xfer_wq);
+
break;
@@ -817,7 +837,9 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
init_hw(priv);
i2c_set_bit(priv->virtbase + I2C_ICR, I2C_IT_MAL);
- complete(&priv->xfer_complete);
+ priv->xfer_done = true;
+ wake_up(&priv->xfer_wq);
+
break;
@@ -834,7 +856,9 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
init_hw(priv);
i2c_set_bit(priv->virtbase + I2C_ICR, I2C_IT_BERR);
- complete(&priv->xfer_complete);
+ priv->xfer_done = true;
+ wake_up(&priv->xfer_wq);
+
}
break;
@@ -848,7 +872,9 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
init_hw(priv);
dev_err(dev, "Tx Fifo Over run\n");
- complete(&priv->xfer_complete);
+ priv->xfer_done = true;
+ wake_up(&priv->xfer_wq);
+
break;
@@ -949,7 +975,7 @@ static void nmk_i2c_of_probe(struct device_node *np,
priv->sm = I2C_FREQ_MODE_FAST;
priv->tft = 1; /* Tx FIFO threshold */
priv->rft = 8; /* Rx FIFO threshold */
- priv->timeout = 200; /* Slave response timeout(ms) */
+ priv->timeout_usecs = 200 * USEC_PER_MSEC; /* Slave response timeout */
}
static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
@@ -1009,7 +1035,7 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
adap->owner = THIS_MODULE;
adap->class = I2C_CLASS_DEPRECATED;
adap->algo = &nmk_i2c_algo;
- adap->timeout = msecs_to_jiffies(priv->timeout);
+ adap->timeout = usecs_to_jiffies(priv->timeout_usecs);
snprintf(adap->name, sizeof(adap->name),
"Nomadik I2C at %pR", &adev->res);
--
2.43.1
On Thu, 15 Feb 2024 17:52:09 +0100, Théo Lebrun wrote:
> Add EyeQ5 bindings to the existing Nomadik I2C dt-bindings. Add the two
> EyeQ5-specific properties behind a conditional. Add an example for this
> compatible.
>
> Signed-off-by: Théo Lebrun <[email protected]>
> ---
> .../devicetree/bindings/i2c/st,nomadik-i2c.yaml | 44 ++++++++++++++++++++--
> 1 file changed, 40 insertions(+), 4 deletions(-)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.example.dtb: i2c@300000: 'mobileye,id', 'mobileye,olb' do not match any of the regexes: '^#.*', '^(at25|bm|devbus|dmacap|dsa|exynos|fsi[ab]|gpio-fan|gpio-key|gpio|gpmc|hdmi|i2c-gpio),.*', '^(keypad|m25p|max8952|max8997|max8998|mpmc),.*', '^(pinctrl-single|#pinctrl-single|PowerPC),.*', '^(pl022|pxa-mmc|rcar_sound|rotary-encoder|s5m8767|sdhci),.*', '^(simple-audio-card|st-plgpio|st-spics|ts),.*', '^100ask,.*', '^70mai,.*', '^8dev,.*', '^GEFanuc,.*', '^ORCL,.*', '^SUNW,.*', '^[a-zA-Z0-9#_][a-zA-Z0-9+\\-._@]{0,63}$', '^[a-zA-Z0-9+\\-._]*@[0-9a-zA-Z,]*$', '^abb,.*', '^abilis,.*', '^abracon,.*', '^abt,.*', '^acbel,.*', '^acer,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^adieng,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^aesop,.*', '^airoha,.*', '^al,.*', '^alcatel,.*', '^aldec,.*', '^alfa-network,.*', '^allegro
,.*', '^alliedvision,.*', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^anbernic,.*', '^andestech,.*', '^anvo,.*', '^aosong,.*', '^apm,.*', '^apple,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arcom,.*', '^arctic,.*', '^arcx,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^asix,.*', '^aspeed,.*', '^asrock,.*', '^asus,.*', '^atheros,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^awinic,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^baikal,.*', '^bananapi,.*', '^beacon,.*', '^beagle,.*', '^belling,.*', '^bhf,.*', '^bigtreetech,.*', '^bitmain,.*', '^blutek,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bsh,.*', '^bticino,.*', '^buff
alo,.*', '^bur,.*', '^bytedance,.*', '^calamp,.*', '^calaosystems,.*', '^calxeda,.*', '^canaan,.*', '^caninos,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cdns,.*', '^cdtech,.*', '^cellwise,.*', '^ceva,.*', '^chargebyte,.*', '^checkpoint,.*', '^chefree,.*', '^chipidea,.*', '^chipone,.*', '^chipspark,.*', '^chongzhou,.*', '^chrontel,.*', '^chrp,.*', '^chunghwa,.*', '^chuwi,.*', '^ciaa,.*', '^cirrus,.*', '^cisco,.*', '^clockwork,.*', '^cloos,.*', '^cloudengines,.*', '^cnm,.*', '^cnxt,.*', '^colorfly,.*', '^compulab,.*', '^congatec,.*', '^coolpi,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csq,.*', '^ctera,.*', '^ctu,.*', '^cubietech,.*', '^cui,.*', '^cypress,.*', '^cyx,.*', '^cznic,.*', '^dallas,.*', '^dataimage,.*', '^davicom,.*', '^dell,.*', '^delta,.*', '^densitron,.*', '^denx,.*', '^devantech,.*', '^dfi,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^dimonoff,.*', '
^diodes,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.*', '^domintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^ds,.*', '^dserve,.*', '^dynaimage,.*', '^ea,.*', '^ebang,.*', '^ebbg,.*', '^ebs-systart,.*', '^ebv,.*', '^eckelmann,.*', '^edgeble,.*', '^edimax,.*', '^edt,.*', '^ees,.*', '^eeti,.*', '^einfochips,.*', '^eink,.*', '^elan,.*', '^element14,.*', '^elgin,.*', '^elida,.*', '^elimo,.*', '^elpida,.*', '^embedfire,.*', '^embest,.*', '^emlid,.*', '^emmicro,.*', '^empire-electronix,.*', '^emtrion,.*', '^enclustra,.*', '^endless,.*', '^ene,.*', '^energymicro,.*', '^engicam,.*', '^engleder,.*', '^epcos,.*', '^epfl,.*', '^epson,.*', '^esp,.*', '^est,.*', '^ettus,.*', '^eukrea,.*', '^everest,.*', '^everspin,.*', '^evervision,.*', '^exar,.*', '^excito,.*', '^exegin,.*', '^ezchip,.*', '^facebook,.*', '^fairphone,.*', '^faraday,.*', '^fascontek,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^
freecom,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*', '^galaxycore,.*', '^gardena,.*', '^gateway,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gemei,.*', '^gemtek,.*', '^genesys,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^globalscale,.*', '^globaltop,.*', '^gmt,.*', '^goldelico,.*', '^goodix,.*', '^google,.*', '^goramo,.*', '^gplus,.*', '^grinn,.*', '^grmn,.*', '^gumstix,.*', '^gw,.*', '^hannstar,.*', '^haochuangyi,.*', '^haoyu,.*', '^hardkernel,.*', '^hechuang,.*', '^hideep,.*', '^himax,.*', '^hirschmann,.*', '^hisi,.*', '^hisilicon,.*', '^hit,.*', '^hitex,.*', '^holt,.*', '^holtek,.*', '^honestar,.*', '^honeywell,.*', '^hoperun,.*', '^hp,.*', '^hpe,.*', '^hsg,.*', '^htc,.*', '^huawei,.*', '^hugsun,.*', '^hwacom,.*', '^hxt,.*', '^hycon,.*', '^hydis,.*', '^hynitron,.*', '^hynix,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^ifi,.*', '^ilitek,.*', '^imagis,.*', '^img,.*', '^imi,.*', '^inanbo,
.*', '^incircuit,.*', '^indiedroid,.*', '^inet-tek,.*', '^infineon,.*', '^inforce,.*', '^ingenic,.*', '^ingrasys,.*', '^injoinic,.*', '^innocomm,.*', '^innolux,.*', '^inside-secure,.*', '^insignal,.*', '^inspur,.*', '^intel,.*', '^intercontrol,.*', '^invensense,.*', '^inventec,.*', '^inversepath,.*', '^iom,.*', '^irondevice,.*', '^isee,.*', '^isil,.*', '^issi,.*', '^ite,.*', '^itead,.*', '^itian,.*', '^ivo,.*', '^iwave,.*', '^jadard,.*', '^jasonic,.*', '^jdi,.*', '^jedec,.*', '^jesurun,.*', '^jethome,.*', '^jianda,.*', '^joz,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*', '^kiebackpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^kobol,.*', '^koe,.*', '^kontron,.*', '^kosagi,.*', '^kvg,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^lctech,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,
.*', '^lineartechnology,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^liteon,.*', '^litex,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^lontium,.*', '^loongmasses,.*', '^loongson,.*', '^lsi,.*', '^lunzn,.*', '^lwn,.*', '^lxa,.*', '^m5stack,.*', '^macnica,.*', '^mantix,.*', '^mapleboard,.*', '^marantec,.*', '^marvell,.*', '^maxbotix,.*', '^maxim,.*', '^maxlinear,.*', '^mbvl,.*', '^mcube,.*', '^meas,.*', '^mecer,.*', '^mediatek,.*', '^megachips,.*', '^mele,.*', '^melexis,.*', '^melfas,.*', '^mellanox,.*', '^memsensing,.*', '^memsic,.*', '^menlo,.*', '^mentor,.*', '^meraki,.*', '^merrii,.*', '^methode,.*', '^micrel,.*', '^microchip,.*', '^microcrystal,.*', '^micron,.*', '^microsoft,.*', '^microsys,.*', '^mikroe,.*', '^mikrotik,.*', '^milkv,.*', '^miniand,.*', '^minix,.*', '^miramems,.*', '^mitsubishi,.*', '^mitsumi,.*', '^mixel,.*', '^miyoo,.*', '^mntre,.*', '^modtronix,.*', '^moortec,.*', '^mosaixtech,.*', '^motorcomm,.*', '
^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mstar,.*', '^mti,.*', '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxic,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^nec,.*', '^neonode,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netronix,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^newvision,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^novatek,.*', '^novtech,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^ocs,.*', '^oct,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^oneplus,.*', '^onie,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^openailab,.*', '^opencores,.*', '^openembed,.*', '^openpandora,.*', '^openrisc,.*', '^option,.*', '^oranth,.*', '^orisetech,.*', '^ortustech,.*', '^osddisplays,.*', '^osmc,.*', '^ouya,.*', '^overkiz,.*', '^ovti,.*', '^oxsemi,.*', '^ozzmaker,.*', '^panas
onic,.*', '^parade,.*', '^parallax,.*', '^pda,.*', '^pericom,.*', '^pervasive,.*', '^phicomm,.*', '^phytec,.*', '^picochip,.*', '^pine64,.*', '^pineriver,.*', '^pixcir,.*', '^plantower,.*', '^plathome,.*', '^plda,.*', '^plx,.*', '^ply,.*', '^pni,.*', '^pocketbook,.*', '^polaroid,.*', '^polyhex,.*', '^portwell,.*', '^poslab,.*', '^pov,.*', '^powertip,.*', '^powervr,.*', '^powkiddy,.*', '^primux,.*', '^probox2,.*', '^prt,.*', '^pulsedlight,.*', '^purism,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qishenglong,.*', '^qnap,.*', '^quanta,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^remarkable,.*', '^renesas,.*', '^rervision,.*', '^revotics,.*', '^rex,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riot,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^roseapplepi,.*', '^rve,.*', '^saef,.*', '^samsung,.*', '^samtec,.*', '^sanclo
ud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^seagate,.*', '^seeed,.*', '^seirobotics,.*', '^semtech,.*', '^senseair,.*', '^sensirion,.*', '^sensortek,.*', '^sercomm,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shift,.*', '^shimafuji,.*', '^shineworld,.*', '^shiratech,.*', '^si-en,.*', '^si-linux,.*', '^siemens,.*', '^sifive,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silan,.*', '^silead,.*', '^silergy,.*', '^silex-insight,.*', '^siliconfile,.*', '^siliconmitus,.*', '^silvaco,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sinowealth,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skov,.*', '^skyworks,.*', '^smartlabs,.*', '^smi,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^sophgo,.*', '^sourceparts,.*', '^spansion,.*', '^sparkfun,.*', '^spinalhdl,.*', '^sprd,.*', '^square,.*', '^ssi,.*', '^sst,.*', '^sstar,.*', '^st,.*', '^st-ericsson,.*', '^starfive,.*'
, '^starry,.*', '^startek,.*', '^starterkit,.*', '^ste,.*', '^stericsson,.*', '^storlink,.*', '^storm,.*', '^storopack,.*', '^summit,.*', '^sunchip,.*', '^sundance,.*', '^sunplus,.*', '^supermicro,.*', '^swir,.*', '^syna,.*', '^synology,.*', '^synopsys,.*', '^tbs,.*', '^tbs-biometrics,.*', '^tcg,.*', '^tcl,.*', '^tcs,.*', '^tdo,.*', '^team-source-display,.*', '^technexion,.*', '^technologic,.*', '^techstar,.*', '^techwell,.*', '^teejet,.*', '^teltonika,.*', '^tempo,.*', '^terasic,.*', '^tesla,.*', '^tfc,.*', '^thead,.*', '^thine,.*', '^thingyjp,.*', '^thundercomm,.*', '^thwc,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^topic,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^transpeed,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^turing,.*', '^tyan,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ufispace,.*', '^ugoos,.*', '^un
iwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^usr,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^vialab,.*', '^vicor,.*', '^videostrong,.*', '^virtio,.*', '^virtual,.*', '^vishay,.*', '^visionox,.*', '^vitesse,.*', '^vivante,.*', '^vivax,.*', '^vocore,.*', '^voipac,.*', '^vot,.*', '^vxt,.*', '^wanchanglong,.*', '^wand,.*', '^waveshare,.*', '^wd,.*', '^we,.*', '^welltech,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^widora,.*', '^wiligear,.*', '^willsemi,.*', '^winbond,.*', '^wingtech,.*', '^winlink,.*', '^winstar,.*', '^wirelesstag,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^x-powers,.*', '^xen,.*', '^xes,.*', '^xiaomi,.*', '^xillybus,.*', '^xingbangda,.*', '^xinpeng,.*', '^xiphera,.*', '^xlnx,.*', '^xnano,.*', '^xunlong,.*', '^xylon,.*', '^yadro,.*', '^yamaha,.*', '^yes-optoelectronics,.*', '^yic,.*', '^yiming,.*', '^ylm,.*', '^yna,.*', '^yones-toptech,.*', '^ys,.*', '^ysoft,.*', '^zarlink,
.*', '^zealz,.*', '^zeitec,.*', '^zidoo,.*', '^zii,.*', '^zinitix,.*', '^zkmagic,.*', '^zte,.*', '^zyxel,.*', 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/vendor-prefixes.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On Thu, Feb 15, 2024 at 05:52:09PM +0100, Th?o Lebrun wrote:
> Add EyeQ5 bindings to the existing Nomadik I2C dt-bindings. Add the two
> EyeQ5-specific properties behind a conditional. Add an example for this
> compatible.
>
> Signed-off-by: Th?o Lebrun <[email protected]>
> ---
> .../devicetree/bindings/i2c/st,nomadik-i2c.yaml | 44 ++++++++++++++++++++--
> 1 file changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
> index e6b95e3765ac..eaade14b6d4c 100644
> --- a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
> @@ -14,9 +14,6 @@ description: The Nomadik I2C host controller began its life in the ST
> maintainers:
> - Linus Walleij <[email protected]>
>
> -allOf:
> - - $ref: /schemas/i2c/i2c-controller.yaml#
> -
> # Need a custom select here or 'arm,primecell' will match on lots of nodes
> select:
> properties:
> @@ -24,6 +21,7 @@ select:
> contains:
> enum:
> - st,nomadik-i2c
> + - mobileye,eyeq5-i2c
> required:
> - compatible
>
> @@ -39,6 +37,10 @@ properties:
> - const: stericsson,db8500-i2c
> - const: st,nomadik-i2c
> - const: arm,primecell
> + # The variant found on Mobileye EyeQ5
> + - items:
> + - const: mobileye,eyeq5-i2c
> + - const: arm,primecell
>
> reg:
> maxItems: 1
> @@ -55,7 +57,7 @@ properties:
> - items:
> - const: mclk
> - const: apb_pclk
> - # Clock name in DB8500
> + # Clock name in DB8500 or EyeQ5
> - items:
> - const: i2cclk
> - const: apb_pclk
> @@ -83,6 +85,25 @@ required:
>
> unevaluatedProperties: false
>
> +allOf:
> + - $ref: /schemas/i2c/i2c-controller.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mobileye,eyeq5-i2c
> + then:
> + properties:
> + mobileye,olb:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: A phandle to the OLB syscon.
Define properties at the top-level and then restrict them in if/then
schemas.
> + mobileye,id:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Platform-wide controller ID (integer starting from zero).
instance indexes are a NAK. You can use i2cN aliases if you must.
Why do you need it? To access OLB? If so, add cell args to the OLB
phandle instead.
Rob
Hello,
On Fri Feb 16, 2024 at 3:22 AM CET, Rob Herring wrote:
> On Thu, Feb 15, 2024 at 05:52:09PM +0100, Théo Lebrun wrote:
> > Add EyeQ5 bindings to the existing Nomadik I2C dt-bindings. Add the two
> > EyeQ5-specific properties behind a conditional. Add an example for this
> > compatible.
[...]
> > +allOf:
> > + - $ref: /schemas/i2c/i2c-controller.yaml#
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: mobileye,eyeq5-i2c
> > + then:
> > + properties:
> > + mobileye,olb:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description: A phandle to the OLB syscon.
>
> Define properties at the top-level and then restrict them in if/then
> schemas.
Noted, thanks.
> > + mobileye,id:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Platform-wide controller ID (integer starting from zero).
>
> instance indexes are a NAK. You can use i2cN aliases if you must.
>
> Why do you need it? To access OLB? If so, add cell args to the OLB
> phandle instead.
Why we do what we do: I2C controller must write a 2 bit value depending
on the bus speed. All I2C controllers write into the same register.
Index is used to compute the shift.
mobileye,olb is a phandle to a syscon. I'll be using i2cN aliases I
guess.
Thanks,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 16/02/2024 11:18, Théo Lebrun wrote:
>
>>> + mobileye,id:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description: Platform-wide controller ID (integer starting from zero).
>>
>> instance indexes are a NAK. You can use i2cN aliases if you must.
>>
>> Why do you need it? To access OLB? If so, add cell args to the OLB
>> phandle instead.
>
> Why we do what we do: I2C controller must write a 2 bit value depending
> on the bus speed. All I2C controllers write into the same register.
Which register? Your devices do not share IO address space.
> Index is used to compute the shift.
>
> mobileye,olb is a phandle to a syscon. I'll be using i2cN aliases I
> guess.
>
> Thanks,
>
> --
> Théo Lebrun, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best regards,
Krzysztof
Hello,
On Fri Feb 16, 2024 at 11:33 AM CET, Krzysztof Kozlowski wrote:
> On 16/02/2024 11:18, Théo Lebrun wrote:
> >
> >>> + mobileye,id:
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>> + description: Platform-wide controller ID (integer starting from zero).
> >>
> >> instance indexes are a NAK. You can use i2cN aliases if you must.
> >>
> >> Why do you need it? To access OLB? If so, add cell args to the OLB
> >> phandle instead.
> >
> > Why we do what we do: I2C controller must write a 2 bit value depending
> > on the bus speed. All I2C controllers write into the same register.
>
> Which register? Your devices do not share IO address space.
mobileye,olb is a prop with a phandle to a syscon. That syscon contains
the register we are interested in.
The Linux code side of things is in the following patch. We use
syscon_regmap_lookup_by_phandle().
[PATCH 10/13] i2c: nomadik: support Mobileye EyeQ5 I2C controller
https://lore.kernel.org/lkml/[email protected]/
Thanks,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 16/02/2024 11:40, Théo Lebrun wrote:
> Hello,
>
> On Fri Feb 16, 2024 at 11:33 AM CET, Krzysztof Kozlowski wrote:
>> On 16/02/2024 11:18, Théo Lebrun wrote:
>>>
>>>>> + mobileye,id:
>>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>>> + description: Platform-wide controller ID (integer starting from zero).
>>>>
>>>> instance indexes are a NAK. You can use i2cN aliases if you must.
>>>>
>>>> Why do you need it? To access OLB? If so, add cell args to the OLB
>>>> phandle instead.
>>>
>>> Why we do what we do: I2C controller must write a 2 bit value depending
>>> on the bus speed. All I2C controllers write into the same register.
>>
>> Which register? Your devices do not share IO address space.
>
> mobileye,olb is a prop with a phandle to a syscon. That syscon contains
> the register we are interested in.
So exactly what Rob said... I don't understand why you have chosen to go
with alias.
Best regards,
Krzysztof
Hello,
On Sat Feb 17, 2024 at 9:25 AM CET, Krzysztof Kozlowski wrote:
> On 16/02/2024 11:40, Théo Lebrun wrote:
> > On Fri Feb 16, 2024 at 11:33 AM CET, Krzysztof Kozlowski wrote:
> >> On 16/02/2024 11:18, Théo Lebrun wrote:
> >>>
> >>>>> + mobileye,id:
> >>>>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>>>> + description: Platform-wide controller ID (integer starting from zero).
> >>>>
> >>>> instance indexes are a NAK. You can use i2cN aliases if you must.
> >>>>
> >>>> Why do you need it? To access OLB? If so, add cell args to the OLB
> >>>> phandle instead.
> >>>
> >>> Why we do what we do: I2C controller must write a 2 bit value depending
> >>> on the bus speed. All I2C controllers write into the same register.
> >>
> >> Which register? Your devices do not share IO address space.
> >
> > mobileye,olb is a prop with a phandle to a syscon. That syscon contains
> > the register we are interested in.
>
> So exactly what Rob said... I don't understand why you have chosen to go
> with alias.
I had misunderstood Rob's original message. Now that I've done some
tests to use cells I get what was meant. I'd have a follow-up question.
What should the cells contain? I see two options:
- phandle + I2C controller global index (from 0 thru 4). Then Linux
(or other) driver know how to map that index to register + mask
combo. ie:
i2c2: i2c@500000 {
compatible = "mobileye,eyeq5-i2c", "arm,primecell";
reg = <0 0x500000 0x0 0x1000>;
/* ... */
mobileye,olb = <&olb 2>;
};
- phandle + register offset + mask. ie:
i2c2: i2c@500000 {
compatible = "mobileye,eyeq5-i2c", "arm,primecell";
reg = <0 0x500000 0x0 0x1000>;
/* ... */
mobileye,olb = <&olb 0xB8 0x300>; /* phandle + offset + mask */
};
I would have guessed the second approach was frown upon as DT aren't
meant to contain iomem offsets. However I'm seeing quite a few drivers
using this approach, and no driver doing the first approach. Maybe my
instinct isn't leading me the right way.
See those bindings that use the second approach. They were found because
their drivers use the syscon_regmap_lookup_by_phandle_args() function
call. I've added the file creation date to highlight recent bindings
(that hopefully are closer to the right way).
- phy/starfive,jh7110-pcie-phy.yaml 2023-06-29T15:51:12+08:00
- usb/starfive,jh7110-usb.yaml 2023-05-18T19:27:48+08:00
- net/starfive,jh7110-dwmac.yaml 2023-04-17T18:02:49+08:00
- phy/qcom,sc8280xp-qmp-pcie-phy.yaml 2022-11-05T15:59:34+01:00
- sound/snps,designware-i2s.yaml 2022-07-01T20:22:49+01:00
- pinctrl/canaan,k210-fpioa.yaml 2020-12-13T22:50:44+09:00
- media/ti,cal.yaml 2019-11-12T15:53:47+01:00
I know looking at existing drivers/bindings isn't the right way, but I
have no other frame of reference. That's why I'm asking for guidance on
this one.
Thanks,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
------------------------------------------------------------------------
On Thu, Feb 15, 2024 at 5:52 PM Théo Lebrun <[email protected]> wrote:
> - Add a new compatible to support Mobileye EyeQ5 which uses the same IP
> block as Nomadik.
Sweet! I'm amazed ST Micro licensed this "ARM PrimeCell" to Mobileye, but
it's a well tested IP and used in eg ST automotive SoC:s so it's a solid
product.
It feels worth it for all the time I have put into maintaining it, finally some
real users again! :)
Yours,
Linus Walleij
On Thu, Feb 15, 2024 at 5:52 PM Théo Lebrun <[email protected]> wrote:
> Disambiguate the usage of dev as a variable name; it is usually best to
> keep it reserved for struct device pointers. Avoid having multiple
> names for the same struct pointer (previously: dev, nmk, nmk_i2c).
>
> Signed-off-by: Théo Lebrun <[email protected]>
Fair enough, it's more readable like this.
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Thu, Feb 15, 2024 at 5:52 PM Théo Lebrun <[email protected]> wrote:
> IRQ_MASK and I2C_CLEAR_ALL_INTS are redundant. One masks the top three
> bits off as reserved, the other one masks the reserved IRQs inside the
> u32. Get rid of IRQ_MASK and only use the most restrictive mask.
>
> Signed-off-by: Théo Lebrun <[email protected]>
Yeah, more readable like this definitely.
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Thu, Feb 15, 2024 at 5:52 PM Théo Lebrun <[email protected]> wrote:
> Constant register bit fields are declared using hardcoded hex values;
> replace them by calls to BIT() and GENMASK(). Replace custom GEN_MASK()
> macro by the generic FIELD_PREP(). Replace manual bit manipulations by
> the generic FIELD_GET() macro.
>
> Signed-off-by: Théo Lebrun <[email protected]>
I'm a fan of this style.
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Thu, Feb 15, 2024 at 5:52 PM Théo Lebrun <[email protected]> wrote:
> Replace the completion by a waitqueue for synchronization from IRQ
> handler to task. For short timeouts, use hrtimers, else use timers.
> Usecase: avoid blocking the I2C bus for too long when an issue occurs.
>
> The threshold picked is one jiffy: if timeout is below that, use
> hrtimers. This threshold is NOT configurable.
>
> Implement behavior but do NOT change fetching of timeout. This means the
> timeout is unchanged (200ms) and the hrtimer case will never trigger.
>
> A waitqueue is used because it supports both desired timeout approaches.
> See wait_event_timeout() and wait_event_hrtimeout(). An atomic boolean
> serves as synchronization condition.
>
> Signed-off-by: Théo Lebrun <[email protected]>
Part of me want to go and fix completions to handle hrtimer timeouts
for submicrosecond timeouts, BUT I realized that this is a bit thick
request for a simple driver, so just a suggestion for something we could
do one day. This is fine with me.
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
Hello,
On Mon Feb 19, 2024 at 3:11 PM CET, Linus Walleij wrote:
> On Thu, Feb 15, 2024 at 5:52 PM Théo Lebrun <[email protected]> wrote:
>
> > - Add a new compatible to support Mobileye EyeQ5 which uses the same IP
> > block as Nomadik.
>
> Sweet! I'm amazed ST Micro licensed this "ARM PrimeCell" to Mobileye, but
> it's a well tested IP and used in eg ST automotive SoC:s so it's a solid
> product.
>
> It feels worth it for all the time I have put into maintaining it, finally some
> real users again! :)
Using the existing Nomadik drivers with the AMBA bus infrastructure on a
non-ARM platform and having it work as-is made our day here at Bootlin.
We are indeed grateful for your work maintaining this platform!
Regards,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
------------------------------------------------------------------------
Hello,
On Mon Feb 19, 2024 at 3:19 PM CET, Linus Walleij wrote:
> On Thu, Feb 15, 2024 at 5:52 PM Théo Lebrun <[email protected]> wrote:
>
> > Replace the completion by a waitqueue for synchronization from IRQ
> > handler to task. For short timeouts, use hrtimers, else use timers.
> > Usecase: avoid blocking the I2C bus for too long when an issue occurs.
> >
> > The threshold picked is one jiffy: if timeout is below that, use
> > hrtimers. This threshold is NOT configurable.
> >
> > Implement behavior but do NOT change fetching of timeout. This means the
> > timeout is unchanged (200ms) and the hrtimer case will never trigger.
> >
> > A waitqueue is used because it supports both desired timeout approaches.
> > See wait_event_timeout() and wait_event_hrtimeout(). An atomic boolean
> > serves as synchronization condition.
> >
> > Signed-off-by: Théo Lebrun <[email protected]>
>
> Part of me want to go and fix completions to handle hrtimer timeouts
> for submicrosecond timeouts, BUT I realized that this is a bit thick
> request for a simple driver, so just a suggestion for something we could
> do one day. This is fine with me.
> Reviewed-by: Linus Walleij <[email protected]>
Indeed having to switch to another abstraction because we desire another
timeout method is nonsensical. Completion supporting hrtimeouts would
make sense. As you said though, this is too much for a simple driver.
Thanks,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
------------------------------------------------------------------------
On 19/02/2024 14:41, Théo Lebrun wrote:
> Hello,
>
> On Sat Feb 17, 2024 at 9:25 AM CET, Krzysztof Kozlowski wrote:
>> On 16/02/2024 11:40, Théo Lebrun wrote:
>>> On Fri Feb 16, 2024 at 11:33 AM CET, Krzysztof Kozlowski wrote:
>>>> On 16/02/2024 11:18, Théo Lebrun wrote:
>>>>>
>>>>>>> + mobileye,id:
>>>>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>>>>> + description: Platform-wide controller ID (integer starting from zero).
>>>>>>
>>>>>> instance indexes are a NAK. You can use i2cN aliases if you must.
>>>>>>
>>>>>> Why do you need it? To access OLB? If so, add cell args to the OLB
>>>>>> phandle instead.
>>>>>
>>>>> Why we do what we do: I2C controller must write a 2 bit value depending
>>>>> on the bus speed. All I2C controllers write into the same register.
>>>>
>>>> Which register? Your devices do not share IO address space.
>>>
>>> mobileye,olb is a prop with a phandle to a syscon. That syscon contains
>>> the register we are interested in.
>>
>> So exactly what Rob said... I don't understand why you have chosen to go
>> with alias.
>
> I had misunderstood Rob's original message. Now that I've done some
> tests to use cells I get what was meant. I'd have a follow-up question.
> What should the cells contain? I see two options:
>
> - phandle + I2C controller global index (from 0 thru 4). Then Linux
> (or other) driver know how to map that index to register + mask
> combo. ie:
>
> i2c2: i2c@500000 {
> compatible = "mobileye,eyeq5-i2c", "arm,primecell";
> reg = <0 0x500000 0x0 0x1000>;
> /* ... */
> mobileye,olb = <&olb 2>;
> };
>
> - phandle + register offset + mask. ie:
>
> i2c2: i2c@500000 {
> compatible = "mobileye,eyeq5-i2c", "arm,primecell";
> reg = <0 0x500000 0x0 0x1000>;
> /* ... */
> mobileye,olb = <&olb 0xB8 0x300>; /* phandle + offset + mask */
> };
Whichever works for your current and possibly future needs and hardware,
because property should have one meaning. It's anyway specific to the
property. Second option is quite popular. Please design it for entire
hardware, not for this one particular case.
Best regards,
Krzysztof