2024-03-15 10:31:48

by Prabhakar

[permalink] [raw]
Subject: [PATCH v2 0/4] Add RIIC support for Renesas RZ/V2H SoC

From: Lad Prabhakar <[email protected]>

Hi all,

This patch series aims to add RIIC support for Renesas RZ/V2H(P) SoC.

v1->v2
- Dropped dt binding which update the comment.
- Used a const for V2H SoC instead of enum in items list
- Dropped internal review tags
- Renamed i2c read/write to riic_readb/riic_writeb
- Made riic as first parameter for riic_writeb
- Dropped family from struct riic_of_data
- Included RIIC_REG_END in enum list as flexible array member
in a struct with no named members is not allowed

Cheers,
Prabhakar

Lad Prabhakar (4):
dt-bindings: i2c: renesas,riic: Document R9A09G057 support
i2c: riic: Introduce helper functions for I2C read/write operations
i2c: riic: Pass register offsets and chip details as OF data
i2c: riic: Add support for R9A09G057 SoC

.../devicetree/bindings/i2c/renesas,riic.yaml | 19 +--
drivers/i2c/busses/i2c-riic.c | 125 +++++++++++++-----
2 files changed, 100 insertions(+), 44 deletions(-)

--
2.34.1



2024-03-15 10:31:52

by Prabhakar

[permalink] [raw]
Subject: [PATCH v2 1/4] dt-bindings: i2c: renesas,riic: Document R9A09G057 support

From: Lad Prabhakar <[email protected]>

Document support for the I2C Bus Interface (RIIC) available in the
Renesas RZ/V2H(P) (R9A09G057) SoC.

The RIIC interface in the Renesas RZ/V2H(P) differs from RZ/A in a
couple of ways:
- Register offsets for the RZ/V2H(P) SoC differ from those of the
RZ/A SoC.
- RZ/V2H register access is limited to 8-bit, whereas RZ/A supports
8/16/32-bit.
- RZ/V2H has bit differences in the slave address register.

To accommodate these differences in the existing driver, a new compatible
string "renesas,riic-r9a09g057" is added.

Signed-off-by: Lad Prabhakar <[email protected]>
---
v1->v2
- Used a const for V2H SoC instead of enum in items list
---
.../devicetree/bindings/i2c/renesas,riic.yaml | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/renesas,riic.yaml b/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
index 2291a7cd619b..91ecf17b7a81 100644
--- a/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
+++ b/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
@@ -15,14 +15,17 @@ allOf:

properties:
compatible:
- items:
- - enum:
- - renesas,riic-r7s72100 # RZ/A1H
- - renesas,riic-r7s9210 # RZ/A2M
- - renesas,riic-r9a07g043 # RZ/G2UL and RZ/Five
- - renesas,riic-r9a07g044 # RZ/G2{L,LC}
- - renesas,riic-r9a07g054 # RZ/V2L
- - const: renesas,riic-rz # RZ/A or RZ/G2L
+ oneOf:
+ - items:
+ - enum:
+ - renesas,riic-r7s72100 # RZ/A1H
+ - renesas,riic-r7s9210 # RZ/A2M
+ - renesas,riic-r9a07g043 # RZ/G2UL and RZ/Five
+ - renesas,riic-r9a07g044 # RZ/G2{L,LC}
+ - renesas,riic-r9a07g054 # RZ/V2L
+ - const: renesas,riic-rz # RZ/A or RZ/G2L
+
+ - const: renesas,riic-r9a09g057 # RZ/V2H(P)

reg:
maxItems: 1
--
2.34.1


2024-03-15 10:32:13

by Prabhakar

[permalink] [raw]
Subject: [PATCH v2 2/4] i2c: riic: Introduce helper functions for I2C read/write operations

From: Lad Prabhakar <[email protected]>

Introduce helper functions for performing I2C read and write operations
in the RIIC driver.

These helper functions lay the groundwork for adding support for the
RZ/V2H SoC. This is essential because the register offsets for the RZ/V2H
SoC differ from those of the RZ/A SoC. By abstracting the read and write
operations, we can seamlessly adapt the driver to support different SoC
variants without extensive modifications.

This patch is part of the preparation process for integrating support for
the RZ/V2H SoC into the RIIC driver.

Signed-off-by: Lad Prabhakar <[email protected]>
---
v1->v2
- Renamed i2c read/write to riic_readb/riic_writeb
- Made riic as first parameter for riic_writeb
---
drivers/i2c/busses/i2c-riic.c | 56 +++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 23 deletions(-)

diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
index e43ff483c56e..5eb25ef3abc4 100644
--- a/drivers/i2c/busses/i2c-riic.c
+++ b/drivers/i2c/busses/i2c-riic.c
@@ -105,9 +105,19 @@ struct riic_irq_desc {
char *name;
};

+static inline void riic_writeb(struct riic_dev *riic, u8 offset, u8 val)
+{
+ writeb(val, riic->base + offset);
+}
+
+static inline u8 riic_readb(struct riic_dev *riic, u8 offset)
+{
+ return readb(riic->base + offset);
+}
+
static inline void riic_clear_set_bit(struct riic_dev *riic, u8 clear, u8 set, u8 reg)
{
- writeb((readb(riic->base + reg) & ~clear) | set, riic->base + reg);
+ riic_writeb(riic, reg, (riic_readb(riic, reg) & ~clear) | set);
}

static int riic_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
@@ -119,7 +129,7 @@ static int riic_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)

pm_runtime_get_sync(adap->dev.parent);

- if (readb(riic->base + RIIC_ICCR2) & ICCR2_BBSY) {
+ if (riic_readb(riic, RIIC_ICCR2) & ICCR2_BBSY) {
riic->err = -EBUSY;
goto out;
}
@@ -127,7 +137,7 @@ static int riic_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
reinit_completion(&riic->msg_done);
riic->err = 0;

- writeb(0, riic->base + RIIC_ICSR2);
+ riic_writeb(riic, RIIC_ICSR2, 0);

for (i = 0, start_bit = ICCR2_ST; i < num; i++) {
riic->bytes_left = RIIC_INIT_MSG;
@@ -135,9 +145,9 @@ static int riic_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
riic->msg = &msgs[i];
riic->is_last = (i == num - 1);

- writeb(ICIER_NAKIE | ICIER_TIE, riic->base + RIIC_ICIER);
+ riic_writeb(riic, RIIC_ICIER, ICIER_NAKIE | ICIER_TIE);

- writeb(start_bit, riic->base + RIIC_ICCR2);
+ riic_writeb(riic, RIIC_ICCR2, start_bit);

time_left = wait_for_completion_timeout(&riic->msg_done, riic->adapter.timeout);
if (time_left == 0)
@@ -191,7 +201,7 @@ static irqreturn_t riic_tdre_isr(int irq, void *data)
* value could be moved to the shadow shift register right away. So
* this must be after updates to ICIER (where we want to disable TIE)!
*/
- writeb(val, riic->base + RIIC_ICDRT);
+ riic_writeb(riic, RIIC_ICDRT, val);

return IRQ_HANDLED;
}
@@ -200,9 +210,9 @@ static irqreturn_t riic_tend_isr(int irq, void *data)
{
struct riic_dev *riic = data;

- if (readb(riic->base + RIIC_ICSR2) & ICSR2_NACKF) {
+ if (riic_readb(riic, RIIC_ICSR2) & ICSR2_NACKF) {
/* We got a NACKIE */
- readb(riic->base + RIIC_ICDRR); /* dummy read */
+ riic_readb(riic, RIIC_ICDRR); /* dummy read */
riic_clear_set_bit(riic, ICSR2_NACKF, 0, RIIC_ICSR2);
riic->err = -ENXIO;
} else if (riic->bytes_left) {
@@ -211,7 +221,7 @@ static irqreturn_t riic_tend_isr(int irq, void *data)

if (riic->is_last || riic->err) {
riic_clear_set_bit(riic, ICIER_TEIE, ICIER_SPIE, RIIC_ICIER);
- writeb(ICCR2_SP, riic->base + RIIC_ICCR2);
+ riic_writeb(riic, RIIC_ICCR2, ICCR2_SP);
} else {
/* Transfer is complete, but do not send STOP */
riic_clear_set_bit(riic, ICIER_TEIE, 0, RIIC_ICIER);
@@ -230,7 +240,7 @@ static irqreturn_t riic_rdrf_isr(int irq, void *data)

if (riic->bytes_left == RIIC_INIT_MSG) {
riic->bytes_left = riic->msg->len;
- readb(riic->base + RIIC_ICDRR); /* dummy read */
+ riic_readb(riic, RIIC_ICDRR); /* dummy read */
return IRQ_HANDLED;
}

@@ -238,7 +248,7 @@ static irqreturn_t riic_rdrf_isr(int irq, void *data)
/* STOP must come before we set ACKBT! */
if (riic->is_last) {
riic_clear_set_bit(riic, 0, ICIER_SPIE, RIIC_ICIER);
- writeb(ICCR2_SP, riic->base + RIIC_ICCR2);
+ riic_writeb(riic, RIIC_ICCR2, ICCR2_SP);
}

riic_clear_set_bit(riic, 0, ICMR3_ACKBT, RIIC_ICMR3);
@@ -248,7 +258,7 @@ static irqreturn_t riic_rdrf_isr(int irq, void *data)
}

/* Reading acks the RIE interrupt */
- *riic->buf = readb(riic->base + RIIC_ICDRR);
+ *riic->buf = riic_readb(riic, RIIC_ICDRR);
riic->buf++;
riic->bytes_left--;

@@ -260,10 +270,10 @@ static irqreturn_t riic_stop_isr(int irq, void *data)
struct riic_dev *riic = data;

/* read back registers to confirm writes have fully propagated */
- writeb(0, riic->base + RIIC_ICSR2);
- readb(riic->base + RIIC_ICSR2);
- writeb(0, riic->base + RIIC_ICIER);
- readb(riic->base + RIIC_ICIER);
+ riic_writeb(riic, RIIC_ICSR2, 0);
+ riic_readb(riic, RIIC_ICSR2);
+ riic_writeb(riic, RIIC_ICIER, 0);
+ riic_readb(riic, RIIC_ICIER);

complete(&riic->msg_done);

@@ -365,15 +375,15 @@ static int riic_init_hw(struct riic_dev *riic, struct i2c_timings *t)
t->scl_rise_ns / (1000000000 / rate), cks, brl, brh);

/* Changing the order of accessing IICRST and ICE may break things! */
- writeb(ICCR1_IICRST | ICCR1_SOWP, riic->base + RIIC_ICCR1);
+ riic_writeb(riic, RIIC_ICCR1, ICCR1_IICRST | ICCR1_SOWP);
riic_clear_set_bit(riic, 0, ICCR1_ICE, RIIC_ICCR1);

- writeb(ICMR1_CKS(cks), riic->base + RIIC_ICMR1);
- writeb(brh | ICBR_RESERVED, riic->base + RIIC_ICBRH);
- writeb(brl | ICBR_RESERVED, riic->base + RIIC_ICBRL);
+ riic_writeb(riic, RIIC_ICMR1, ICMR1_CKS(cks));
+ riic_writeb(riic, RIIC_ICBRH, brh | ICBR_RESERVED);
+ riic_writeb(riic, RIIC_ICBRL, brl | ICBR_RESERVED);

- writeb(0, riic->base + RIIC_ICSER);
- writeb(ICMR3_ACKWP | ICMR3_RDRFS, riic->base + RIIC_ICMR3);
+ riic_writeb(riic, RIIC_ICSER, 0);
+ riic_writeb(riic, RIIC_ICMR3, ICMR3_ACKWP | ICMR3_RDRFS);

riic_clear_set_bit(riic, ICCR1_IICRST, 0, RIIC_ICCR1);

@@ -481,7 +491,7 @@ static void riic_i2c_remove(struct platform_device *pdev)
struct riic_dev *riic = platform_get_drvdata(pdev);

pm_runtime_get_sync(&pdev->dev);
- writeb(0, riic->base + RIIC_ICIER);
+ riic_writeb(riic, RIIC_ICIER, 0);
pm_runtime_put(&pdev->dev);
i2c_del_adapter(&riic->adapter);
pm_runtime_disable(&pdev->dev);
--
2.34.1


2024-03-15 10:32:23

by Prabhakar

[permalink] [raw]
Subject: [PATCH v2 3/4] i2c: riic: Pass register offsets and chip details as OF data

From: Lad Prabhakar <[email protected]>

With an increasing number of SoCs reusing this driver, each with slight
variations in the RIIC IP, it becomes necessary to support passing these
details as OF data. This approach simplifies the extension of the driver
for other SoCs.

This patch lays the groundwork for adding support for the Renesas RZ/V2H
SoC.

Signed-off-by: Lad Prabhakar <[email protected]>
---
v1->v2
- Dropped family from struct riic_of_data
- Included RIIC_REG_END in enum list as flexible array member
in a struct with no named members is not allowed
---
drivers/i2c/busses/i2c-riic.c | 56 +++++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
index 5eb25ef3abc4..3ae2d5c2f85a 100644
--- a/drivers/i2c/busses/i2c-riic.c
+++ b/drivers/i2c/busses/i2c-riic.c
@@ -46,18 +46,6 @@
#include <linux/pm_runtime.h>
#include <linux/reset.h>

-#define RIIC_ICCR1 0x00
-#define RIIC_ICCR2 0x04
-#define RIIC_ICMR1 0x08
-#define RIIC_ICMR3 0x10
-#define RIIC_ICSER 0x18
-#define RIIC_ICIER 0x1c
-#define RIIC_ICSR2 0x24
-#define RIIC_ICBRL 0x34
-#define RIIC_ICBRH 0x38
-#define RIIC_ICDRT 0x3c
-#define RIIC_ICDRR 0x40
-
#define ICCR1_ICE 0x80
#define ICCR1_IICRST 0x40
#define ICCR1_SOWP 0x10
@@ -87,6 +75,25 @@

#define RIIC_INIT_MSG -1

+enum riic_reg_list {
+ RIIC_ICCR1 = 0,
+ RIIC_ICCR2,
+ RIIC_ICMR1,
+ RIIC_ICMR3,
+ RIIC_ICSER,
+ RIIC_ICIER,
+ RIIC_ICSR2,
+ RIIC_ICBRL,
+ RIIC_ICBRH,
+ RIIC_ICDRT,
+ RIIC_ICDRR,
+ RIIC_REG_END,
+};
+
+struct riic_of_data {
+ u8 regs[RIIC_REG_END];
+};
+
struct riic_dev {
void __iomem *base;
u8 *buf;
@@ -94,6 +101,7 @@ struct riic_dev {
int bytes_left;
int err;
int is_last;
+ const struct riic_of_data *info;
struct completion msg_done;
struct i2c_adapter adapter;
struct clk *clk;
@@ -107,12 +115,12 @@ struct riic_irq_desc {

static inline void riic_writeb(struct riic_dev *riic, u8 offset, u8 val)
{
- writeb(val, riic->base + offset);
+ writeb(val, riic->base + riic->info->regs[offset]);
}

static inline u8 riic_readb(struct riic_dev *riic, u8 offset)
{
- return readb(riic->base + offset);
+ return readb(riic->base + riic->info->regs[offset]);
}

static inline void riic_clear_set_bit(struct riic_dev *riic, u8 clear, u8 set, u8 reg)
@@ -453,6 +461,8 @@ static int riic_i2c_probe(struct platform_device *pdev)
}
}

+ riic->info = of_device_get_match_data(&pdev->dev);
+
adap = &riic->adapter;
i2c_set_adapdata(adap, riic);
strscpy(adap->name, "Renesas RIIC adapter", sizeof(adap->name));
@@ -497,8 +507,24 @@ static void riic_i2c_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
}

+static const struct riic_of_data riic_rz_a_info = {
+ .regs = {
+ [RIIC_ICCR1] = 0x00,
+ [RIIC_ICCR2] = 0x04,
+ [RIIC_ICMR1] = 0x08,
+ [RIIC_ICMR3] = 0x10,
+ [RIIC_ICSER] = 0x18,
+ [RIIC_ICIER] = 0x1c,
+ [RIIC_ICSR2] = 0x24,
+ [RIIC_ICBRL] = 0x34,
+ [RIIC_ICBRH] = 0x38,
+ [RIIC_ICDRT] = 0x3c,
+ [RIIC_ICDRR] = 0x40,
+ },
+};
+
static const struct of_device_id riic_i2c_dt_ids[] = {
- { .compatible = "renesas,riic-rz", },
+ { .compatible = "renesas,riic-rz", .data = &riic_rz_a_info },
{ /* Sentinel */ },
};

--
2.34.1


2024-03-15 10:32:57

by Prabhakar

[permalink] [raw]
Subject: [PATCH v2 4/4] i2c: riic: Add support for R9A09G057 SoC

From: Lad Prabhakar <[email protected]>

Extend the RIIC driver to support the RZ/V2H(P) ("R9A09G057") SoC. It
accomplishes this by appending the compatible string list and passing
the RZ/V2H-specific OF data.

Signed-off-by: Lad Prabhakar <[email protected]>
---
v1->v2
- Dropped setting family
---
drivers/i2c/busses/i2c-riic.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
index 3ae2d5c2f85a..861b244d5118 100644
--- a/drivers/i2c/busses/i2c-riic.c
+++ b/drivers/i2c/busses/i2c-riic.c
@@ -523,8 +523,25 @@ static const struct riic_of_data riic_rz_a_info = {
},
};

+static const struct riic_of_data riic_rz_v2h_info = {
+ .regs = {
+ [RIIC_ICCR1] = 0x00,
+ [RIIC_ICCR2] = 0x01,
+ [RIIC_ICMR1] = 0x02,
+ [RIIC_ICMR3] = 0x04,
+ [RIIC_ICSER] = 0x06,
+ [RIIC_ICIER] = 0x07,
+ [RIIC_ICSR2] = 0x09,
+ [RIIC_ICBRL] = 0x10,
+ [RIIC_ICBRH] = 0x11,
+ [RIIC_ICDRT] = 0x12,
+ [RIIC_ICDRR] = 0x13,
+ },
+};
+
static const struct of_device_id riic_i2c_dt_ids[] = {
{ .compatible = "renesas,riic-rz", .data = &riic_rz_a_info },
+ { .compatible = "renesas,riic-r9a09g057", .data = &riic_rz_v2h_info },
{ /* Sentinel */ },
};

--
2.34.1


2024-03-15 12:50:43

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] dt-bindings: i2c: renesas,riic: Document R9A09G057 support

On Fri, Mar 15, 2024 at 11:31 AM Prabhakar <[email protected]> wrote:
> From: Lad Prabhakar <[email protected]>
>
> Document support for the I2C Bus Interface (RIIC) available in the
> Renesas RZ/V2H(P) (R9A09G057) SoC.
>
> The RIIC interface in the Renesas RZ/V2H(P) differs from RZ/A in a
> couple of ways:
> - Register offsets for the RZ/V2H(P) SoC differ from those of the
> RZ/A SoC.
> - RZ/V2H register access is limited to 8-bit, whereas RZ/A supports
> 8/16/32-bit.
> - RZ/V2H has bit differences in the slave address register.
>
> To accommodate these differences in the existing driver, a new compatible
> string "renesas,riic-r9a09g057" is added.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> v1->v2
> - Used a const for V2H SoC instead of enum in items list

Reviewed-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2024-03-15 12:59:17

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] i2c: riic: Introduce helper functions for I2C read/write operations

Hi Prabhakar,

On Fri, Mar 15, 2024 at 11:31 AM Prabhakar <[email protected]> wrote:
> From: Lad Prabhakar <[email protected]>
>
> Introduce helper functions for performing I2C read and write operations
> in the RIIC driver.
>
> These helper functions lay the groundwork for adding support for the
> RZ/V2H SoC. This is essential because the register offsets for the RZ/V2H
> SoC differ from those of the RZ/A SoC. By abstracting the read and write
> operations, we can seamlessly adapt the driver to support different SoC
> variants without extensive modifications.
>
> This patch is part of the preparation process for integrating support for
> the RZ/V2H SoC into the RIIC driver.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> v1->v2
> - Renamed i2c read/write to riic_readb/riic_writeb
> - Made riic as first parameter for riic_writeb

Thanks for the update!

> --- a/drivers/i2c/busses/i2c-riic.c
> +++ b/drivers/i2c/busses/i2c-riic.c
> @@ -105,9 +105,19 @@ struct riic_irq_desc {
> char *name;
> };
>
> +static inline void riic_writeb(struct riic_dev *riic, u8 offset, u8 val)

Please use the same parameter order as writeb(), i.e. "val" before
"offset"), to increase uniformity. This also would make it easier
to review your changes using "git {diff,show} --color-words".

The rest LGTM.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2024-03-15 13:07:25

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] i2c: riic: Pass register offsets and chip details as OF data

On Fri, Mar 15, 2024 at 11:31 AM Prabhakar <[email protected]> wrote:
> From: Lad Prabhakar <[email protected]>
>
> With an increasing number of SoCs reusing this driver, each with slight
> variations in the RIIC IP, it becomes necessary to support passing these
> details as OF data. This approach simplifies the extension of the driver
> for other SoCs.
>
> This patch lays the groundwork for adding support for the Renesas RZ/V2H
> SoC.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> v1->v2
> - Dropped family from struct riic_of_data
> - Included RIIC_REG_END in enum list as flexible array member
> in a struct with no named members is not allowed

Reviewed-by: Geert Uytterhoeven <[email protected]>

> --- a/drivers/i2c/busses/i2c-riic.c
> +++ b/drivers/i2c/busses/i2c-riic.c
> @@ -453,6 +461,8 @@ static int riic_i2c_probe(struct platform_device *pdev)
> }
> }
>
> + riic->info = of_device_get_match_data(&pdev->dev);

This is fine, as all in-tree users use DT (Linux does not support
sh7752/sh7753, which seem to use the same RIIC variant as RZ/V2H).

> +
> adap = &riic->adapter;
> i2c_set_adapdata(adap, riic);
> strscpy(adap->name, "Renesas RIIC adapter", sizeof(adap->name));

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2024-03-15 13:12:10

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] i2c: riic: Add support for R9A09G057 SoC

On Fri, Mar 15, 2024 at 11:31 AM Prabhakar <[email protected]> wrote:
> From: Lad Prabhakar <[email protected]>
>
> Extend the RIIC driver to support the RZ/V2H(P) ("R9A09G057") SoC. It
> accomplishes this by appending the compatible string list and passing
> the RZ/V2H-specific OF data.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> v1->v2
> - Dropped setting family

Reviewed-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2024-03-15 13:49:59

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] i2c: riic: Introduce helper functions for I2C read/write operations

Hi Geert,

Thank you for the review.

On Fri, Mar 15, 2024 at 12:59 PM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Fri, Mar 15, 2024 at 11:31 AM Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Introduce helper functions for performing I2C read and write operations
> > in the RIIC driver.
> >
> > These helper functions lay the groundwork for adding support for the
> > RZ/V2H SoC. This is essential because the register offsets for the RZ/V2H
> > SoC differ from those of the RZ/A SoC. By abstracting the read and write
> > operations, we can seamlessly adapt the driver to support different SoC
> > variants without extensive modifications.
> >
> > This patch is part of the preparation process for integrating support for
> > the RZ/V2H SoC into the RIIC driver.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > v1->v2
> > - Renamed i2c read/write to riic_readb/riic_writeb
> > - Made riic as first parameter for riic_writeb
>
> Thanks for the update!
>
> > --- a/drivers/i2c/busses/i2c-riic.c
> > +++ b/drivers/i2c/busses/i2c-riic.c
> > @@ -105,9 +105,19 @@ struct riic_irq_desc {
> > char *name;
> > };
> >
> > +static inline void riic_writeb(struct riic_dev *riic, u8 offset, u8 val)
>
> Please use the same parameter order as writeb(), i.e. "val" before
> "offset"), to increase uniformity. This also would make it easier
> to review your changes using "git {diff,show} --color-words".
>
Agreed, I will update it in the next version.

Cheers,
Prabhakar

2024-03-15 18:35:38

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] dt-bindings: i2c: renesas,riic: Document R9A09G057 support

Hi Prabhakar,,

On Fri, Mar 15, 2024 at 1:50 PM Geert Uytterhoeven <[email protected]> wrote:
> On Fri, Mar 15, 2024 at 11:31 AM Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Document support for the I2C Bus Interface (RIIC) available in the
> > Renesas RZ/V2H(P) (R9A09G057) SoC.
> >
> > The RIIC interface in the Renesas RZ/V2H(P) differs from RZ/A in a
> > couple of ways:
> > - Register offsets for the RZ/V2H(P) SoC differ from those of the
> > RZ/A SoC.
> > - RZ/V2H register access is limited to 8-bit, whereas RZ/A supports
> > 8/16/32-bit.
> > - RZ/V2H has bit differences in the slave address register.
> >
> > To accommodate these differences in the existing driver, a new compatible
> > string "renesas,riic-r9a09g057" is added.

As it looks like there will be a v3 of this series, please drop "in
the existing driver".

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2024-03-15 19:18:24

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] dt-bindings: i2c: renesas,riic: Document R9A09G057 support

Hi Geert,

On Fri, Mar 15, 2024 at 6:35 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,,
>
> On Fri, Mar 15, 2024 at 1:50 PM Geert Uytterhoeven <[email protected]> wrote:
> > On Fri, Mar 15, 2024 at 11:31 AM Prabhakar <[email protected]> wrote:
> > > From: Lad Prabhakar <[email protected]>
> > >
> > > Document support for the I2C Bus Interface (RIIC) available in the
> > > Renesas RZ/V2H(P) (R9A09G057) SoC.
> > >
> > > The RIIC interface in the Renesas RZ/V2H(P) differs from RZ/A in a
> > > couple of ways:
> > > - Register offsets for the RZ/V2H(P) SoC differ from those of the
> > > RZ/A SoC.
> > > - RZ/V2H register access is limited to 8-bit, whereas RZ/A supports
> > > 8/16/32-bit.
> > > - RZ/V2H has bit differences in the slave address register.
> > >
> > > To accommodate these differences in the existing driver, a new compatible
> > > string "renesas,riic-r9a09g057" is added.
>
> As it looks like there will be a v3 of this series, please drop "in
> the existing driver".
>
Sure I'll drop it.

Cheers,
Prabhakar

2024-03-17 19:57:43

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] dt-bindings: i2c: renesas,riic: Document R9A09G057 support


On Fri, 15 Mar 2024 10:30:30 +0000, Prabhakar wrote:
> From: Lad Prabhakar <[email protected]>
>
> Document support for the I2C Bus Interface (RIIC) available in the
> Renesas RZ/V2H(P) (R9A09G057) SoC.
>
> The RIIC interface in the Renesas RZ/V2H(P) differs from RZ/A in a
> couple of ways:
> - Register offsets for the RZ/V2H(P) SoC differ from those of the
> RZ/A SoC.
> - RZ/V2H register access is limited to 8-bit, whereas RZ/A supports
> 8/16/32-bit.
> - RZ/V2H has bit differences in the slave address register.
>
> To accommodate these differences in the existing driver, a new compatible
> string "renesas,riic-r9a09g057" is added.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> v1->v2
> - Used a const for V2H SoC instead of enum in items list
> ---
> .../devicetree/bindings/i2c/renesas,riic.yaml | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>

Acked-by: Rob Herring <[email protected]>