Skip to contentSkip to conversation

i2c: npcm: Bug fixes timeout, spurious interrupts

PATCH Applied Tyrone Ting 3 participants 18 messages May 25, 2022 – Jun 9, 2022

Subsystems (3)
Device tree bindings I2C subsystem host drivers Arm/nuvoton npcm architecture
lore
Share & tools
lore

Tyrone Ting

author
#0 lore raw

v6 cover i2c: npcm: Bug fixes timeout, spurious interrupts

From: Tyrone Ting <[email protected]>

This patchset includes the following fixes:

- Add dt-bindings description for NPCM845.
- Bug fix for timeout calculation.
- Better handling of spurious interrupts.
- Fix for event type in slave mode.
- Removal of own slave addresses [2:10].
- Support for next gen BMC (NPCM845).

The NPCM I2C driver is tested on NPCM750 and NPCM845 evaluation boards.

Addressed comments from:
- Wolfram Sang : https://lkml.org/lkml/2022/5/21/51

Changes since version 5:
- Shorten npcm_i2caddr array since the number of supported i2c slave addresses is reduced.
- Create a new patch from the original PATCH v5 08/10 to modify the slave transaction logic.

Changes since version 4:
- Remove quotes around ref handle to nuvoton,sys-mgr in i2c binding document.
- Keep the "longer line first" order.
- Correct the SoB chain.
- Modify the if statement in one line and add new line characters.
- Modify the commit message format in the patch Remove own slave addresses 2:10. Correct the if statement in one line and shorten the debug messages.
- Create a new patch to capitalize the one-line comment in the driver.

Changes since version 3:
- Correct the const format in if condition in i2c binding document.
- Add the oops message statement and register information in register access width patch.
- Add the occurring rate of the i2c spurious interrupt issue and more details in driver's behavior to overcome this issue.
- Address Andy's comments in the patch to support NPCM845.

Changes since version 2:
- Keep old code as fallback, if getting nuvoton,sys-mgr property fails.
- Fix the error reported by running 'make DT_CHECKER_FLAGS=-m dt_binding_check'.
- Make nuvoton,sys-mgr required for nuvoton,npcm845-i2c.
- Correct the patch's subject about changing the way of getting GCR regmap and add the description about keeping old code as fallback if getting nuvoton,sys-mgr property fails.
- Correct the patch title and description about removing the unused variable clk_regmap.
- Use the data field directly instead of the macros since macros are not constants anymore in this patch.

Changes since version 1:
- Add nuvoton,sys-mgr property in NPCM devicetree.
- Describe the commit message in imperative mood.
- Modify the description in i2c binding document to cover NPCM series.
- Add new property in i2c binding document.
- Create a new patch for client address calculation.
- Create a new patch for updating gcr property name.
- Create a new patch for removing unused clock node.
- Explain EOB in the commit description.
- Create a new patch for correcting NPCM register access width.
- Remove some comment since the corresponding logic no longer exists.
- Remove fixes tag while the patch adds an additional feature.
- Use devicetree data field to support NPCM845.

Tali Perry (2):
i2c: npcm: Remove own slave addresses 2:10
i2c: npcm: Correct slave role behavior

Tyrone Ting (3):
dt-bindings: i2c: npcm: support NPCM845
i2c: npcm: Support NPCM845
i2c: npcm: Capitalize the one-line comment

 .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml     |  25 ++-
 drivers/i2c/busses/Kconfig                    |   8 +-
 drivers/i2c/busses/Makefile                   |   2 +-
 drivers/i2c/busses/i2c-npcm7xx.c              | 176 ++++++++++--------
 4 files changed, 124 insertions(+), 87 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH v6 1/5 dt-bindings: i2c: npcm: support NPCM845

From: Tyrone Ting <[email protected]>

Add compatible and nuvoton,sys-mgr description for NPCM i2c module.

Signed-off-by Tyrone Ting <[email protected]>
Reviewed-by Krzysztof Kozlowski <[email protected]>
Reviewed-by Rob Herring <[email protected]>
---
 .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml     | 25 +++++++++++++++----
 1 file changed, 20 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml +20 -5
index 128444942aec..09d2591e1fa3 100644
@@ -7,17 +7,18 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 title: nuvoton NPCM7XX I2C Controller Device Tree Bindings
 
 description: |
-  The NPCM750x includes sixteen I2C bus controllers. All Controllers support
-  both master and slave mode. Each controller can switch between master and slave
-  at run time (i.e. IPMB mode). Each controller has two 16 byte HW FIFO for TX and
-  RX.
+  I2C bus controllers of the NPCM series support both master and
+  slave mode. Each controller can switch between master and slave at run time
+  (i.e. IPMB mode). HW FIFO for TX and RX are supported.
 
 maintainers:
   - Tali Perry <[email protected]>
 
 properties:
   compatible:
-    const: nuvoton,npcm750-i2c
+    enum:
+      - nuvoton,npcm750-i2c
+      - nuvoton,npcm845-i2c
 
   reg:
     maxItems: 1

@@ -36,6 +37,10 @@ properties:
     default: 100000
     enum: [100000, 400000, 1000000]
 
+  nuvoton,sys-mgr:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: The phandle of system manager register node.
+
 required:
   - compatible
   - reg

@@ -44,6 +49,15 @@ required:
 
 allOf:
   - $ref: /schemas/i2c/i2c-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: nuvoton,npcm845-i2c
+
+    then:
+      required:
+        - nuvoton,sys-mgr
 
 unevaluatedProperties: false
 

@@ -57,6 +71,7 @@ examples:
         clock-frequency = <100000>;
         interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
         compatible = "nuvoton,npcm750-i2c";
+        nuvoton,sys-mgr = <&gcr>;
     };
 
 ...
Signature · 1 line
2.17.1
author
#3 lore raw

Re: [PATCH v6 1/5] dt-bindings: i2c: npcm: support NPCM845

Hi Wolfram:

Thank you for your review and comments.

Wolfram Sang <[email protected]> 於 2022年6月9日 週四 上午4:02寫道：

Quoted 12 lines · Tyrone Ting
Quoted 7 lines

From: Tyrone Ting <[email protected]>

Add compatible and nuvoton,sys-mgr description for NPCM i2c module.

Signed-off-by Tyrone Ting <[email protected]>
Reviewed-by Krzysztof Kozlowski <[email protected]>
Reviewed-by Rob Herring <[email protected]>

Applied to for-next, thanks!

Best Regards,
Tyrone

author
#4 lore raw

PATCH v6 2/5 i2c: npcm: Remove own slave addresses 2:10

In reply to Tyrone Ting

From: Tali Perry <[email protected]>

NPCM can support up to 10 own slave addresses. In practice, only one address is actually being used. In order to access addresses 2 and above, need to switch register banks. The switch needs spinlock.
To avoid using spinlock for this useless feature removed support of SA >=
2. Also fix returned slave event enum.

Remove some comment since the bank selection is not required. The bank selection is not required since the supported slave addresses are reduced.

Fixes 56a1485b102e ("i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver")
Signed-off-by Tali Perry <[email protected]>
Signed-off-by Tyrone Ting <[email protected]>
---
 drivers/i2c/busses/i2c-npcm7xx.c | 41 +++++++++++++-------------------
 1 file changed, 16 insertions(+), 25 deletions(-)
drivers/i2c/busses/i2c-npcm7xx.c +16 -25
index 5960ccde6574..b5cc83e51029 100644
@@ -123,11 +123,11 @@ enum i2c_addr {
  * Since the addr regs are sprinkled all over the address space,
  * use this array to get the address or each register.
  */
-#define I2C_NUM_OWN_ADDR 10
+#define I2C_NUM_OWN_ADDR 2
+#define I2C_NUM_OWN_ADDR_SUPPORTED 2
+
 static const int npcm_i2caddr[I2C_NUM_OWN_ADDR] = {
-	NPCM_I2CADDR1, NPCM_I2CADDR2, NPCM_I2CADDR3, NPCM_I2CADDR4,
-	NPCM_I2CADDR5, NPCM_I2CADDR6, NPCM_I2CADDR7, NPCM_I2CADDR8,
-	NPCM_I2CADDR9, NPCM_I2CADDR10,
+	NPCM_I2CADDR1, NPCM_I2CADDR2,
 };
 #endif
 

@@ -392,14 +392,10 @@ static void npcm_i2c_disable(struct npcm_i2c *bus)
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	int i;
 
-	/* select bank 0 for I2C addresses */
-	npcm_i2c_select_bank(bus, I2C_BANK_0);
-
 	/* Slave addresses removal */
-	for (i = I2C_SLAVE_ADDR1; i < I2C_NUM_OWN_ADDR; i++)
+	for (i = I2C_SLAVE_ADDR1; i < I2C_NUM_OWN_ADDR_SUPPORTED; i++)
 		iowrite8(0, bus->reg + npcm_i2caddr[i]);
 
-	npcm_i2c_select_bank(bus, I2C_BANK_1);
 #endif
 	/* Disable module */
 	i2cctl2 = ioread8(bus->reg + NPCM_I2CCTL2);

@@ -604,8 +600,7 @@ static int npcm_i2c_slave_enable(struct npcm_i2c *bus, enum i2c_addr addr_type,
 			i2cctl1 &= ~NPCM_I2CCTL1_GCMEN;
 		iowrite8(i2cctl1, bus->reg + NPCM_I2CCTL1);
 		return 0;
-	}
-	if (addr_type == I2C_ARP_ADDR) {
+	} else if (addr_type == I2C_ARP_ADDR) {
 		i2cctl3 = ioread8(bus->reg + NPCM_I2CCTL3);
 		if (enable)
 			i2cctl3 |= I2CCTL3_ARPMEN;

@@ -614,16 +609,16 @@ static int npcm_i2c_slave_enable(struct npcm_i2c *bus, enum i2c_addr addr_type,
 		iowrite8(i2cctl3, bus->reg + NPCM_I2CCTL3);
 		return 0;
 	}
+	if (addr_type > I2C_SLAVE_ADDR2 && addr_type <= I2C_SLAVE_ADDR10)
+		dev_err(bus->dev, "try to enable more than 2 SA not supported\n");
+
 	if (addr_type >= I2C_ARP_ADDR)
 		return -EFAULT;
-	/* select bank 0 for address 3 to 10 */
-	if (addr_type > I2C_SLAVE_ADDR2)
-		npcm_i2c_select_bank(bus, I2C_BANK_0);
+
 	/* Set and enable the address */
 	iowrite8(sa_reg, bus->reg + npcm_i2caddr[addr_type]);
 	npcm_i2c_slave_int_enable(bus, enable);
-	if (addr_type > I2C_SLAVE_ADDR2)
-		npcm_i2c_select_bank(bus, I2C_BANK_1);
+
 	return 0;
 }
 #endif

@@ -846,15 +841,11 @@ static u8 npcm_i2c_get_slave_addr(struct npcm_i2c *bus, enum i2c_addr addr_type)
 {
 	u8 slave_add;
 
-	/* select bank 0 for address 3 to 10 */
-	if (addr_type > I2C_SLAVE_ADDR2)
-		npcm_i2c_select_bank(bus, I2C_BANK_0);
+	if (addr_type > I2C_SLAVE_ADDR2 && addr_type <= I2C_SLAVE_ADDR10)
+		dev_err(bus->dev, "get slave: try to use more than 2 SA not supported\n");
 
 	slave_add = ioread8(bus->reg + npcm_i2caddr[(int)addr_type]);
 
-	if (addr_type > I2C_SLAVE_ADDR2)
-		npcm_i2c_select_bank(bus, I2C_BANK_1);
-
 	return slave_add;
 }
 

@@ -864,12 +855,12 @@ static int npcm_i2c_remove_slave_addr(struct npcm_i2c *bus, u8 slave_add)
 
 	/* Set the enable bit */
 	slave_add |= 0x80;
-	npcm_i2c_select_bank(bus, I2C_BANK_0);
-	for (i = I2C_SLAVE_ADDR1; i < I2C_NUM_OWN_ADDR; i++) {
+
+	for (i = I2C_SLAVE_ADDR1; i < I2C_NUM_OWN_ADDR_SUPPORTED; i++) {
 		if (ioread8(bus->reg + npcm_i2caddr[i]) == slave_add)
 			iowrite8(0, bus->reg + npcm_i2caddr[i]);
 	}
-	npcm_i2c_select_bank(bus, I2C_BANK_1);
+
 	return 0;
 }
Signature · 1 line
2.17.1
Applied
#5 lore raw

Re: [PATCH v6 2/5] i2c: npcm: Remove own slave addresses 2:10

Quoted 8 lines · Tyrone Ting

From: Tali Perry <[email protected]>

NPCM can support up to 10 own slave addresses. In practice, only one address is actually being used. In order to access addresses 2 and above, need to switch register banks. The switch needs spinlock.
To avoid using spinlock for this useless feature removed support of SA >=
2. Also fix returned slave event enum.

Quoted 6 lines

Remove some comment since the bank selection is not required. The bank selection is not required since the supported slave addresses are reduced.

Fixes 56a1485b102e ("i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver")
Signed-off-by Tali Perry <[email protected]>
Signed-off-by Tyrone Ting <[email protected]>

Applied to for-next, thanks!

author
#6 lore raw

Re: [PATCH v6 2/5] i2c: npcm: Remove own slave addresses 2:10

Hi Wolfram:

Thank you for your review and comments.

Wolfram Sang <[email protected]> 於 2022年6月9日 週四 上午4:02寫道：

Quoted 10 lines · Tyrone Ting
Quoted 8 lines

From: Tali Perry <[email protected]>

NPCM can support up to 10 own slave addresses. In practice, only one address is actually being used. In order to access addresses 2 and above, need to switch register banks. The switch needs spinlock.
To avoid using spinlock for this useless feature removed support of SA >=
2. Also fix returned slave event enum.

Quoted 9 lines
Quoted 6 lines

Remove some comment since the bank selection is not required. The bank selection is not required since the supported slave addresses are reduced.

Fixes 56a1485b102e ("i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver")
Signed-off-by Tali Perry <[email protected]>
Signed-off-by Tyrone Ting <[email protected]>

Applied to for-next, thanks!

Best Regards,
Tyrone

author
#7 lore raw

PATCH v6 3/5 i2c: npcm: Correct slave role behavior

In reply to Tyrone Ting

From: Tali Perry <[email protected]>

Correct the slave transaction logic to be compatible with the generic slave backend driver.

Fixes 56a1485b102e ("i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver")
Signed-off-by Tali Perry <[email protected]>
Signed-off-by Tyrone Ting <[email protected]>
---
 drivers/i2c/busses/i2c-npcm7xx.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/i2c/busses/i2c-npcm7xx.c +6 -3
index b5cc83e51029..b4d218c6c8fb 100644
@@ -915,11 +915,15 @@ static int npcm_i2c_slave_get_wr_buf(struct npcm_i2c *bus)
 	for (i = 0; i < I2C_HW_FIFO_SIZE; i++) {
 		if (bus->slv_wr_size >= I2C_HW_FIFO_SIZE)
 			break;
-		i2c_slave_event(bus->slave, I2C_SLAVE_READ_REQUESTED, &value);
+		if (bus->state == I2C_SLAVE_MATCH) {
+			i2c_slave_event(bus->slave, I2C_SLAVE_READ_REQUESTED, &value);
+			bus->state = I2C_OPER_STARTED;
+		} else {
+			i2c_slave_event(bus->slave, I2C_SLAVE_READ_PROCESSED, &value);
+		}
 		ind = (bus->slv_wr_ind + bus->slv_wr_size) % I2C_HW_FIFO_SIZE;
 		bus->slv_wr_buf[ind] = value;
 		bus->slv_wr_size++;
-		i2c_slave_event(bus->slave, I2C_SLAVE_READ_PROCESSED, &value);
 	}
 	return I2C_HW_FIFO_SIZE - ret;
 }

@@ -967,7 +971,6 @@ static void npcm_i2c_slave_xmit(struct npcm_i2c *bus, u16 nwrite,
 	if (nwrite == 0)
 		return;
 
-	bus->state = I2C_OPER_STARTED;
 	bus->operation = I2C_WRITE_OPER;
 
 	/* get the next buffer */
Signature · 1 line
2.17.1
author
#9 lore raw

Re: [PATCH v6 3/5] i2c: npcm: Correct slave role behavior

Hi Wolfram:

Your previous suggestion about this commit is highly appreciated.

Thank you.

Wolfram Sang <[email protected]> 於 2022年6月9日 週四 上午4:03寫道：

Quoted 13 lines · Tyrone Ting
Quoted 8 lines

From: Tali Perry <[email protected]>

Correct the slave transaction logic to be compatible with the generic slave backend driver.

Fixes 56a1485b102e ("i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver")
Signed-off-by Tali Perry <[email protected]>
Signed-off-by Tyrone Ting <[email protected]>

Much easier to review this way! Applied to for-next, thank you!

Best Regards,
Tyrone

author
#10 lore raw

PATCH v6 4/5 i2c: npcm: Support NPCM845

In reply to Tyrone Ting

From: Tyrone Ting <[email protected]>

Add NPCM8XX I2C support.
The NPCM8XX uses a similar i2c module as NPCM7XX. The internal HW FIFO is larger in NPCM8XX.

Signed-off-by Tyrone Ting <[email protected]>
Acked-by Tomer Maimon <[email protected]>
---
 drivers/i2c/busses/Kconfig       |   8 +--
 drivers/i2c/busses/Makefile      |   2 +-
 drivers/i2c/busses/i2c-npcm7xx.c | 114 +++++++++++++++++++------------
 3 files changed, 76 insertions(+), 48 deletions(-)
drivers/i2c/busses/Kconfig +4 -4
index a1bae59208e3..b1d7069dd377 100644
@@ -838,13 +838,13 @@ config I2C_NOMADIK
 	  I2C interface from ST-Ericsson's Nomadik and Ux500 architectures,
 	  as well as the STA2X11 PCIe I/O HUB.
 
-config I2C_NPCM7XX
+config I2C_NPCM
 	tristate "Nuvoton I2C Controller"
-	depends on ARCH_NPCM7XX || COMPILE_TEST
+	depends on ARCH_NPCM || COMPILE_TEST
 	help
 	  If you say yes to this option, support will be included for the
-	  Nuvoton I2C controller, which is available on the NPCM7xx BMC
-	  controller.
+	  Nuvoton I2C controller, which is available on the NPCM BMC
+	  controllers.
 	  Driver can also support slave mode (select I2C_SLAVE).
 
 config I2C_OCORES
drivers/i2c/busses/Makefile +1 -1
index 479f60e4ee3d..b0a10e5d9ee9 100644
@@ -84,7 +84,7 @@ obj-$(CONFIG_I2C_MT7621)	+= i2c-mt7621.o
 obj-$(CONFIG_I2C_MV64XXX)	+= i2c-mv64xxx.o
 obj-$(CONFIG_I2C_MXS)		+= i2c-mxs.o
 obj-$(CONFIG_I2C_NOMADIK)	+= i2c-nomadik.o
-obj-$(CONFIG_I2C_NPCM7XX)	+= i2c-npcm7xx.o
+obj-$(CONFIG_I2C_NPCM)		+= i2c-npcm7xx.o
 obj-$(CONFIG_I2C_OCORES)	+= i2c-ocores.o
 obj-$(CONFIG_I2C_OMAP)		+= i2c-omap.o
 obj-$(CONFIG_I2C_OWL)		+= i2c-owl.o
drivers/i2c/busses/i2c-npcm7xx.c +71 -43

304 lines not on this page · raw message

Signature · 1 line
2.17.1
author
#12 lore raw

Re: [PATCH v6 4/5] i2c: npcm: Support NPCM845

Hi Wolfram:

Thank you for your review and comments.

Wolfram Sang <[email protected]> 於 2022年6月9日 週四 上午4:03寫道：

Quoted 13 lines · Tyrone Ting
Quoted 8 lines

From: Tyrone Ting <[email protected]>

Add NPCM8XX I2C support.
The NPCM8XX uses a similar i2c module as NPCM7XX. The internal HW FIFO is larger in NPCM8XX.

Signed-off-by Tyrone Ting <[email protected]>
Acked-by Tomer Maimon <[email protected]>

Applied to for-next, thanks!

Best Regards,
Tyrone

author
#13 lore raw

PATCH v6 5/5 i2c: npcm: Capitalize the one-line comment

In reply to Tyrone Ting

From: Tyrone Ting <[email protected]>

Make the one-line comments capital in the driver to get the comment style consistent.

Fixes 56a1485b102e ("i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver")
Signed-off-by Tyrone Ting <[email protected]>
---
 drivers/i2c/busses/i2c-npcm7xx.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/i2c/busses/i2c-npcm7xx.c +6 -6
index 86fd942ea648..7d863edc14e4 100644
@@ -680,7 +680,7 @@ static void npcm_i2c_reset(struct npcm_i2c *bus)
 	}
 #endif
 
-	/* clear status bits for spurious interrupts */
+	/* Clear status bits for spurious interrupts */
 	npcm_i2c_clear_master_status(bus);
 
 	bus->state = I2C_IDLE;

@@ -1252,7 +1252,7 @@ static irqreturn_t npcm_i2c_int_slave_handler(struct npcm_i2c *bus)
 	} /* SDAST */
 
 	/*
-	 * if irq is not one of the above, make sure EOB is disabled and all
+	 * If irq is not one of the above, make sure EOB is disabled and all
 	 * status bits are cleared.
 	 */
 	if (ret == IRQ_NONE) {

@@ -1506,7 +1506,7 @@ static void npcm_i2c_irq_handle_nack(struct npcm_i2c *bus)
 		npcm_i2c_clear_master_status(bus);
 		readx_poll_timeout_atomic(ioread8, bus->reg + NPCM_I2CCST, val,
 					  !(val & NPCM_I2CCST_BUSY), 10, 200);
-		/* verify no status bits are still set after bus is released */
+		/* Verify no status bits are still set after bus is released */
 		npcm_i2c_clear_master_status(bus);
 	}
 	bus->state = I2C_IDLE;

@@ -1974,7 +1974,7 @@ static int npcm_i2c_init_module(struct npcm_i2c *bus, enum i2c_mode mode,
 
 	npcm_i2c_reset(bus);
 
-	/* check HW is OK: SDA and SCL should be high at this point. */
+	/* Check HW is OK: SDA and SCL should be high at this point. */
 	if ((npcm_i2c_get_SDA(&bus->adap) == 0) || (npcm_i2c_get_SCL(&bus->adap) == 0)) {
 		dev_err(bus->dev, "I2C%d init fail: lines are low\n", bus->num);
 		dev_err(bus->dev, "SDA=%d SCL=%d\n", npcm_i2c_get_SDA(&bus->adap),

@@ -2034,7 +2034,7 @@ static irqreturn_t npcm_i2c_bus_irq(int irq, void *dev_id)
 			return IRQ_HANDLED;
 	}
 #endif
-	/* clear status bits for spurious interrupts */
+	/* Clear status bits for spurious interrupts */
 	npcm_i2c_clear_master_status(bus);
 
 	return IRQ_HANDLED;

@@ -2216,7 +2216,7 @@ static int npcm_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 		 (bus->data->rxf_ctl_last_pec & ioread8(bus->reg + NPCM_I2CRXF_CTL)))
 		npcm_i2c_reset(bus);
 
-	/* after any xfer, successful or not, stall and EOB must be disabled */
+	/* After any xfer, successful or not, stall and EOB must be disabled */
 	npcm_i2c_stall_after_start(bus, false);
 	npcm_i2c_eob_int(bus, false);
Signature · 1 line
2.17.1
Applied
#14 lore raw

Re: [PATCH v6 5/5] i2c: npcm: Capitalize the one-line comment

Quoted 7 lines · Tyrone Ting

From: Tyrone Ting <[email protected]>

Make the one-line comments capital in the driver to get the comment style consistent.

Fixes 56a1485b102e ("i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver")
Signed-off-by Tyrone Ting <[email protected]>

Applied to for-next, thanks!

author
#15 lore raw

Re: [PATCH v6 5/5] i2c: npcm: Capitalize the one-line comment

Hi Wolfram:

Thank you for your review and comments for this whole patch set.

Wolfram Sang <[email protected]> 於 2022年6月9日 週四 上午4:04寫道：

Quoted 12 lines · Tyrone Ting
Quoted 7 lines

From: Tyrone Ting <[email protected]>

Make the one-line comments capital in the driver to get the comment style consistent.

Fixes 56a1485b102e ("i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver")
Signed-off-by Tyrone Ting <[email protected]>

Applied to for-next, thanks!

Best Regards,
Tyrone

Reviewed-by on the v6 series
#16 lore raw
In reply to Tyrone Ting
Quoted 4 lines · Tyrone Ting

From: Tyrone Ting <[email protected]>

This patchset includes the following fixes:

Quoted 8 lines

- Add dt-bindings description for NPCM845.
- Bug fix for timeout calculation.
- Better handling of spurious interrupts.
- Fix for event type in slave mode.
- Removal of own slave addresses [2:10].
- Support for next gen BMC (NPCM845).

The NPCM I2C driver is tested on NPCM750 and NPCM845 evaluation boards.

LGTM,

Reviewed-by Andy Shevchenko <[email protected]>
Quoted 73 lines

Addressed comments from:
- Wolfram Sang : https://lkml.org/lkml/2022/5/21/51

Changes since version 5:
- Shorten npcm_i2caddr array since the number of supported i2c slave addresses is reduced.
- Create a new patch from the original PATCH v5 08/10 to modify the slave transaction logic.

Changes since version 4:
- Remove quotes around ref handle to nuvoton,sys-mgr in i2c binding document.
- Keep the "longer line first" order.
- Correct the SoB chain.
- Modify the if statement in one line and add new line characters.
- Modify the commit message format in the patch Remove own slave addresses 2:10. Correct the if statement in one line and shorten the debug messages.
- Create a new patch to capitalize the one-line comment in the driver.

Changes since version 3:
- Correct the const format in if condition in i2c binding document.
- Add the oops message statement and register information in register access width patch.
- Add the occurring rate of the i2c spurious interrupt issue and more details in driver's behavior to overcome this issue.
- Address Andy's comments in the patch to support NPCM845.

Changes since version 2:
- Keep old code as fallback, if getting nuvoton,sys-mgr property fails.
- Fix the error reported by running 'make DT_CHECKER_FLAGS=-m dt_binding_check'.
- Make nuvoton,sys-mgr required for nuvoton,npcm845-i2c.
- Correct the patch's subject about changing the way of getting GCR regmap and add the description about keeping old code as fallback if getting nuvoton,sys-mgr property fails.
- Correct the patch title and description about removing the unused variable clk_regmap.
- Use the data field directly instead of the macros since macros are not constants anymore in this patch.

Changes since version 1:
- Add nuvoton,sys-mgr property in NPCM devicetree.
- Describe the commit message in imperative mood.
- Modify the description in i2c binding document to cover NPCM series.
- Add new property in i2c binding document.
- Create a new patch for client address calculation.
- Create a new patch for updating gcr property name.
- Create a new patch for removing unused clock node.
- Explain EOB in the commit description.
- Create a new patch for correcting NPCM register access width.
- Remove some comment since the corresponding logic no longer exists.
- Remove fixes tag while the patch adds an additional feature.
- Use devicetree data field to support NPCM845.

Tali Perry (2):
i2c: npcm: Remove own slave addresses 2:10
i2c: npcm: Correct slave role behavior

Tyrone Ting (3):
dt-bindings: i2c: npcm: support NPCM845
i2c: npcm: Support NPCM845
i2c: npcm: Capitalize the one-line comment

 .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml     |  25 ++-
 drivers/i2c/busses/Kconfig                    |   8 +-
 drivers/i2c/busses/Makefile                   |   2 +-
 drivers/i2c/busses/i2c-npcm7xx.c              | 176 ++++++++++--------
 4 files changed, 124 insertions(+), 87 deletions(-)
Signature · 1 line
2.17.1
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#17 lore raw

Hi Andy:

Thank you for your review.

Much appreciated.

Andy Shevchenko <[email protected]> 於 2022年5月26日 週四 上午1:32寫道：

Quoted 88 lines · Tyrone Ting
Quoted 4 lines

From: Tyrone Ting <[email protected]>

This patchset includes the following fixes:

Quoted 8 lines

- Add dt-bindings description for NPCM845.
- Bug fix for timeout calculation.
- Better handling of spurious interrupts.
- Fix for event type in slave mode.
- Removal of own slave addresses [2:10].
- Support for next gen BMC (NPCM845).

The NPCM I2C driver is tested on NPCM750 and NPCM845 evaluation boards.

LGTM,

Reviewed-by Andy Shevchenko <[email protected]>
Quoted 70 lines

Addressed comments from:
- Wolfram Sang : https://lkml.org/lkml/2022/5/21/51

Changes since version 5:
- Shorten npcm_i2caddr array since the number of supported i2c slave addresses is reduced.
- Create a new patch from the original PATCH v5 08/10 to modify the slave transaction logic.

Changes since version 4:
- Remove quotes around ref handle to nuvoton,sys-mgr in i2c binding document.
- Keep the "longer line first" order.
- Correct the SoB chain.
- Modify the if statement in one line and add new line characters.
- Modify the commit message format in the patch Remove own slave addresses 2:10. Correct the if statement in one line and shorten the debug messages.
- Create a new patch to capitalize the one-line comment in the driver.

Changes since version 3:
- Correct the const format in if condition in i2c binding document.
- Add the oops message statement and register information in register access width patch.
- Add the occurring rate of the i2c spurious interrupt issue and more details in driver's behavior to overcome this issue.
- Address Andy's comments in the patch to support NPCM845.

Changes since version 2:
- Keep old code as fallback, if getting nuvoton,sys-mgr property fails.
- Fix the error reported by running 'make DT_CHECKER_FLAGS=-m dt_binding_check'.
- Make nuvoton,sys-mgr required for nuvoton,npcm845-i2c.
- Correct the patch's subject about changing the way of getting GCR regmap and add the description about keeping old code as fallback if getting nuvoton,sys-mgr property fails.
- Correct the patch title and description about removing the unused variable clk_regmap.
- Use the data field directly instead of the macros since macros are not constants anymore in this patch.

Changes since version 1:
- Add nuvoton,sys-mgr property in NPCM devicetree.
- Describe the commit message in imperative mood.
- Modify the description in i2c binding document to cover NPCM series.
- Add new property in i2c binding document.
- Create a new patch for client address calculation.
- Create a new patch for updating gcr property name.
- Create a new patch for removing unused clock node.
- Explain EOB in the commit description.
- Create a new patch for correcting NPCM register access width.
- Remove some comment since the corresponding logic no longer exists.
- Remove fixes tag while the patch adds an additional feature.
- Use devicetree data field to support NPCM845.

Tali Perry (2):
i2c: npcm: Remove own slave addresses 2:10
i2c: npcm: Correct slave role behavior

Tyrone Ting (3):
dt-bindings: i2c: npcm: support NPCM845
i2c: npcm: Support NPCM845
i2c: npcm: Capitalize the one-line comment

 .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml     |  25 ++-
 drivers/i2c/busses/Kconfig                    |   8 +-
 drivers/i2c/busses/Makefile                   |   2 +-
 drivers/i2c/busses/i2c-npcm7xx.c              | 176 ++++++++++--------
 4 files changed, 124 insertions(+), 87 deletions(-)
Quoted 9 lines
Quoted 3 lines

--
2.17.1

--
With Best Regards,
Andy Shevchenko

Best Regards,
Tyrone