| Message ID | 20220624101736.27217-3-phil.edworthy@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | i2c: Add new driver for Renesas RZ/V2M controller | expand |
On Fri, Jun 24, 2022 at 12:17 PM Phil Edworthy <phil.edworthy@renesas.com> wrote: > > Yet another i2c controller from Renesas that is found on the RZ/V2M > (r9a09g011) SoC. It can support only 100kHz and 400KHz operation. I see nothing wrong with this, just one suggestion for a cleanup: > +#ifdef CONFIG_PM_SLEEP > +static int rzv2m_i2c_suspend(struct device *dev) ...> +static const struct dev_pm_ops rzv2m_i2c_pm_ops = { > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(rzv2m_i2c_suspend, rzv2m_i2c_resume) > +}; > + > +#define DEV_PM_OPS (&rzv2m_i2c_pm_ops) > +#else > +#define DEV_PM_OPS NULL > +#endif /* CONFIG_PM_SLEEP */ Remove the #ifdef here, and use the new NOIRQ_SYSTEM_SLEEP_PM_OPS() in place of SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(). Arnd
Hi Arnd, On Fri, Jun 24, 2022 at 1:27 PM Arnd Bergmann <arnd@arndb.de> wrote: > On Fri, Jun 24, 2022 at 12:17 PM Phil Edworthy > <phil.edworthy@renesas.com> wrote: > > > > Yet another i2c controller from Renesas that is found on the RZ/V2M > > (r9a09g011) SoC. It can support only 100kHz and 400KHz operation. > > I see nothing wrong with this, just one suggestion for a cleanup: > > > +#ifdef CONFIG_PM_SLEEP > > +static int rzv2m_i2c_suspend(struct device *dev) > ...> +static const struct dev_pm_ops rzv2m_i2c_pm_ops = { > > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(rzv2m_i2c_suspend, rzv2m_i2c_resume) > > +}; > > + > > +#define DEV_PM_OPS (&rzv2m_i2c_pm_ops) > > +#else > > +#define DEV_PM_OPS NULL > > +#endif /* CONFIG_PM_SLEEP */ > > Remove the #ifdef here, and use the new NOIRQ_SYSTEM_SLEEP_PM_OPS() > in place of SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(). Cool, TIL! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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
Hi Arnd, On 24 June 2022 12:27 Arnd Bergmann wrote: > On Fri, Jun 24, 2022 at 12:17 PM Phil Edworthy wrote: > > > > Yet another i2c controller from Renesas that is found on the RZ/V2M > > (r9a09g011) SoC. It can support only 100kHz and 400KHz operation. > > I see nothing wrong with this, just one suggestion for a cleanup: > > > +#ifdef CONFIG_PM_SLEEP > > +static int rzv2m_i2c_suspend(struct device *dev) > ...> +static const struct dev_pm_ops rzv2m_i2c_pm_ops = { > > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(rzv2m_i2c_suspend, > rzv2m_i2c_resume) > > +}; > > + > > +#define DEV_PM_OPS (&rzv2m_i2c_pm_ops) > > +#else > > +#define DEV_PM_OPS NULL > > +#endif /* CONFIG_PM_SLEEP */ > > Remove the #ifdef here, and use the new NOIRQ_SYSTEM_SLEEP_PM_OPS() > in place of SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(). Will do! Thanks Phil
On Fri, Jun 24, 2022 at 01:48:58PM +0200, Geert Uytterhoeven wrote: > On Fri, Jun 24, 2022 at 1:27 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Fri, Jun 24, 2022 at 12:17 PM Phil Edworthy > > <phil.edworthy@renesas.com> wrote: ... > > Remove the #ifdef here, and use the new NOIRQ_SYSTEM_SLEEP_PM_OPS() > > in place of SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(). > > Cool, TIL! There are also pm_ptr() and pm_sleep_ptr() macros (need to be used when assign dev_pm_ops).
Hi Andy, On Tue, Jun 28, 2022 at 12:17 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Fri, Jun 24, 2022 at 01:48:58PM +0200, Geert Uytterhoeven wrote: > > On Fri, Jun 24, 2022 at 1:27 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > On Fri, Jun 24, 2022 at 12:17 PM Phil Edworthy > > > <phil.edworthy@renesas.com> wrote: > > ... > > > > Remove the #ifdef here, and use the new NOIRQ_SYSTEM_SLEEP_PM_OPS() > > > in place of SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(). > > > > Cool, TIL! > > There are also pm_ptr() and pm_sleep_ptr() macros (need to be used when assign > dev_pm_ops). Indeed, I have used the latter in a patch I posted yesterday ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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
Hi Andy, On 28 June 2022 11:17 Andy Shevchenko wrote: > On Fri, Jun 24, 2022 at 01:48:58PM +0200, Geert Uytterhoeven wrote: > > On Fri, Jun 24, 2022 at 1:27 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > On Fri, Jun 24, 2022 at 12:17 PM Phil Edworthy > > > <phil.edworthy@renesas.com> wrote: > > ... > > > > Remove the #ifdef here, and use the new NOIRQ_SYSTEM_SLEEP_PM_OPS() > > > in place of SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(). > > > > Cool, TIL! > > There are also pm_ptr() and pm_sleep_ptr() macros (need to be used when > assign > dev_pm_ops). I noticed these when looking at NOIRQ_SYSTEM_SLEEP_PM_OPS, but thanks for pointing them out anyway. Phil
Hi Phil, On Fri, Jun 24, 2022 at 12:18 PM Phil Edworthy <phil.edworthy@renesas.com> wrote: > Yet another i2c controller from Renesas that is found on the RZ/V2M > (r9a09g011) SoC. It can support only 100kHz and 400KHz operation. > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > --- /dev/null > +++ b/drivers/i2c/busses/i2c-rzv2m.c > +enum bcr_index { > + RZV2M_I2C_100K = 0, > + RZV2M_I2C_400K, > +}; > + > +struct bitrate_config { > + unsigned int percent_low; > + unsigned int min_hold_time_ns; > +}; > + > +static const struct bitrate_config bitrate_configs[] = { > + {47, 3450}, > + {52, 900}, These are indexed by bcr_index, so perhaps .[RZV2M_I2C_100K] = { 47, 3450 }, ... ? > +}; > +/* Calculate IICB0WL and IICB0WH */ > +static int rzv2m_i2c_clock_calculate(struct device *dev, > + struct rzv2m_i2c_priv *priv) > +{ > + const struct bitrate_config *config; > + unsigned int hold_time_ns; > + unsigned int total_pclks; > + unsigned int trf_pclks; > + unsigned long pclk_hz; > + struct i2c_timings t; > + u32 trf_ns; > + > + i2c_parse_fw_timings(dev, &t, true); > + > + pclk_hz = clk_get_rate(priv->clk); > + total_pclks = pclk_hz / t.bus_freq_hz; > + > + trf_ns = t.scl_rise_ns + t.scl_fall_ns; > + trf_pclks = (u64)pclk_hz * trf_ns / NSEC_PER_SEC; This is an open-coded 64-by-ul division, which may cause link failures when compile-tested on 32-bit. Please use mul_u64_u32_div() instead. > + > + /* Config setting */ > + switch (t.bus_freq_hz) { > + case I2C_MAX_FAST_MODE_FREQ: > + priv->bus_mode = RZV2M_I2C_400K; > + break; > + case I2C_MAX_STANDARD_MODE_FREQ: > + priv->bus_mode = RZV2M_I2C_100K; > + break; > + default: > + dev_err(dev, "transfer speed is invalid\n"); > + return -EINVAL; > + } > + config = &bitrate_configs[priv->bus_mode]; > + > + /* IICB0WL = (percent_low / Transfer clock) x PCLK */ > + priv->iicb0wl = total_pclks * config->percent_low / 100; > + if (priv->iicb0wl > 0x3ff) > + return -EINVAL; > + > + /* IICB0WH = ((percent_high / Transfer clock) x PCLK) - (tR + tF) */ > + priv->iicb0wh = total_pclks - priv->iicb0wl - trf_pclks; > + if (priv->iicb0wh > 0x3ff) > + return -EINVAL; > + > + /* > + * Data hold time must be less than 0.9us in fast mode and > + * 3.45us in standard mode. > + * Data hold time = IICB0WL[9:2] / PCLK > + */ > + hold_time_ns = (u64)(priv->iicb0wl >> 2) * NSEC_PER_SEC / pclk_hz; div64_ul() > + if (hold_time_ns > config->min_hold_time_ns) { > + dev_err(dev, "data hold time %dns is over %dns\n", > + hold_time_ns, config->min_hold_time_ns); > + return -EINVAL; > + } > + > + return 0; > +} > +static int rzv2m_i2c_write_with_ACK(struct rzv2m_i2c_priv *priv, u32 data) rzv2m_i2c_write_with_ack > +{ > + unsigned long time_left; > + > + reinit_completion(&priv->msg_tia_done); > + > + writel(data, priv->base + IICB0DAT); > + > + time_left = wait_for_completion_timeout(&priv->msg_tia_done, > + priv->adap.timeout); > + if (!time_left) > + return -ETIMEDOUT; > + > + /* Confirm ACK */ > + if ((readl(priv->base + IICB0STR0) & IICB0SSAC) != IICB0SSAC) > + return -ENXIO; > + > + return 0; > +} > + > +static int rzv2m_i2c_read_with_ACK(struct rzv2m_i2c_priv *priv, u8 *data, rzv2m_i2c_read_with_ack > + bool last) > +{ > + unsigned long time_left; > + u32 data_tmp; > + > + reinit_completion(&priv->msg_tia_done); > + > + /* Interrupt request timing : 8th clock */ > + bit_clrl(priv->base + IICB0CTL0, IICB0SLWT); > + > + /* Exit the wait state */ > + writel(IICB0WRET, priv->base + IICB0TRG); > + > + /* Wait for transaction */ > + time_left = wait_for_completion_timeout(&priv->msg_tia_done, > + priv->adap.timeout); > + if (!time_left) > + return -ETIMEDOUT; > + > + if (!last) { > + /* Read data */ > + data_tmp = readl(priv->base + IICB0DAT); > + } else { > + /* Disable ACK */ > + bit_clrl(priv->base + IICB0CTL0, IICB0SLAC); > + > + /* Read data*/ > + data_tmp = readl(priv->base + IICB0DAT); > + > + /* Interrupt request timing : 9th clock */ > + bit_setl(priv->base + IICB0CTL0, IICB0SLWT); > + > + /* Exit the wait state */ > + writel(IICB0WRET, priv->base + IICB0TRG); > + > + /* Wait for transaction */ > + time_left = wait_for_completion_timeout(&priv->msg_tia_done, > + priv->adap.timeout); > + if (!time_left) > + return -ETIMEDOUT; > + > + /* Enable ACK */ > + bit_setl(priv->base + IICB0CTL0, IICB0SLAC); > + } > + > + *data = (u8)(data_tmp & 0xff); No need to cast or mask. > + > + return 0; > +} > + > +static int rzv2m_i2c_send(struct rzv2m_i2c_priv *priv, struct i2c_msg *msg, > + int *count) unsigned int *count > +{ > + int i, ret = 0; unsigned int i > + > + for (i = 0; i < msg->len; i++) { > + ret = rzv2m_i2c_write_with_ACK(priv, msg->buf[i]); > + if (ret < 0) > + break; return ret? > + } > + *count = i; > + > + return ret; > +} > + > +static int rzv2m_i2c_receive(struct rzv2m_i2c_priv *priv, struct i2c_msg *msg, > + int *count) unsigned int *count > +{ > + int i, ret = 0; unsigned int i > + > + for (i = 0; i < msg->len; i++) { > + ret = rzv2m_i2c_read_with_ACK(priv, &msg->buf[i], > + ((msg->len - 1) == i)); > + if (ret < 0) > + break; return ret? > + } > + *count = i; > + > + return ret; > +} > + > +static int rzv2m_i2c_send_address(struct rzv2m_i2c_priv *priv, > + struct i2c_msg *msg, int read) No need to pass read, as you have access to the full msg, and 10-bit addressing is rare? > +{ > + u32 addr; > + int ret; > + > + if (msg->flags & I2C_M_TEN) { > + /* 10-bit address > + * addr_1: 5'b11110 | addr[9:8] | (R/nW) > + * addr_2: addr[7:0] > + */ > + addr = 0xf0 | ((msg->addr >> 7) & 0x06); > + addr |= read; > + /* Send 1st address(extend code) */ > + ret = rzv2m_i2c_write_with_ACK(priv, addr); > + if (ret == 0) { > + /* Send 2nd address */ > + ret = rzv2m_i2c_write_with_ACK(priv, msg->addr & 0xff); > + } > + } else { > + /* 7-bit address */ > + addr = i2c_8bit_addr_from_msg(msg); > + ret = rzv2m_i2c_write_with_ACK(priv, addr); > + } > + > + return ret; > +} > +static int rzv2m_i2c_master_xfer1(struct rzv2m_i2c_priv *priv, > + struct i2c_msg *msg, int stop) > +{ > + int count = 0; unsigned int > + int ret, read = !!(msg->flags & I2C_M_RD); > + > + /* Send start condition */ > + writel(IICB0STT, priv->base + IICB0TRG); > + > + ret = rzv2m_i2c_send_address(priv, msg, read); > + Please drop this blank line. > + if (ret == 0) { > + if (read) > + ret = rzv2m_i2c_receive(priv, msg, &count); > + else > + ret = rzv2m_i2c_send(priv, msg, &count); > + > + if ((ret == 0) && stop) > + ret = rzv2m_i2c_stop_condition(priv); > + } > + > + if (ret == -ENXIO) > + rzv2m_i2c_stop_condition(priv); > + else if (ret < 0) > + rzv2m_i2c_init(priv); > + else > + ret = count; > + > + return ret; > +} > + > +static int rzv2m_i2c_master_xfer(struct i2c_adapter *adap, > + struct i2c_msg *msgs, int num) > +{ > + struct rzv2m_i2c_priv *priv = i2c_get_adapdata(adap); > + struct device *dev = rzv2m_i2c_priv_to_dev(priv); > + int ret, i; unsigned int i > + > + pm_runtime_get_sync(dev); Please use pm_runtime_resume_and_get() in new code (and check its return code?). > + > + if (readl(priv->base + IICB0STR0) & IICB0SSBS) { > + ret = -EAGAIN; > + goto out; > + } > + > + /* I2C main transfer */ > + for (i = 0; i < num; i++) { > + ret = rzv2m_i2c_master_xfer1(priv, &msgs[i], (i == (num - 1))); > + if (ret < 0) > + goto out; > + } > + ret = num; > + > +out: > + pm_runtime_put(dev); > + > + return ret; > +} > +static struct i2c_algorithm rzv2m_i2c_algo = { > + .master_xfer = rzv2m_i2c_master_xfer, > + .functionality = rzv2m_i2c_func, No .(un)reg_slave()? ;-) The hardware seems to support it. > +}; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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
Hi Geert, On 28 June 2022 14:42 Geert Uytterhoeven wrote: > On Fri, Jun 24, 2022 at 12:18 PM Phil Edworthy wrote: > > Yet another i2c controller from Renesas that is found on the RZ/V2M > > (r9a09g011) SoC. It can support only 100kHz and 400KHz operation. > > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > Thanks for your patch! Thanks for your review! > > --- /dev/null > > +++ b/drivers/i2c/busses/i2c-rzv2m.c > > > +enum bcr_index { > > + RZV2M_I2C_100K = 0, > > + RZV2M_I2C_400K, > > +}; > > + > > +struct bitrate_config { > > + unsigned int percent_low; > > + unsigned int min_hold_time_ns; > > +}; > > + > > +static const struct bitrate_config bitrate_configs[] = { > > + {47, 3450}, > > + {52, 900}, > > These are indexed by bcr_index, so perhaps > > .[RZV2M_I2C_100K] = { 47, 3450 }, > ... > > ? Ok > > +}; > > > +/* Calculate IICB0WL and IICB0WH */ > > +static int rzv2m_i2c_clock_calculate(struct device *dev, > > + struct rzv2m_i2c_priv *priv) > > +{ > > + const struct bitrate_config *config; > > + unsigned int hold_time_ns; > > + unsigned int total_pclks; > > + unsigned int trf_pclks; > > + unsigned long pclk_hz; > > + struct i2c_timings t; > > + u32 trf_ns; > > + > > + i2c_parse_fw_timings(dev, &t, true); > > + > > + pclk_hz = clk_get_rate(priv->clk); > > + total_pclks = pclk_hz / t.bus_freq_hz; > > + > > + trf_ns = t.scl_rise_ns + t.scl_fall_ns; > > + trf_pclks = (u64)pclk_hz * trf_ns / NSEC_PER_SEC; > > This is an open-coded 64-by-ul division, which may cause link failures > when compile-tested on 32-bit. Please use mul_u64_u32_div() instead. Sure > > + > > + /* Config setting */ > > + switch (t.bus_freq_hz) { > > + case I2C_MAX_FAST_MODE_FREQ: > > + priv->bus_mode = RZV2M_I2C_400K; > > + break; > > + case I2C_MAX_STANDARD_MODE_FREQ: > > + priv->bus_mode = RZV2M_I2C_100K; > > + break; > > + default: > > + dev_err(dev, "transfer speed is invalid\n"); > > + return -EINVAL; > > + } > > + config = &bitrate_configs[priv->bus_mode]; > > + > > + /* IICB0WL = (percent_low / Transfer clock) x PCLK */ > > + priv->iicb0wl = total_pclks * config->percent_low / 100; > > + if (priv->iicb0wl > 0x3ff) > > + return -EINVAL; > > + > > + /* IICB0WH = ((percent_high / Transfer clock) x PCLK) - (tR + > tF) */ > > + priv->iicb0wh = total_pclks - priv->iicb0wl - trf_pclks; > > + if (priv->iicb0wh > 0x3ff) > > + return -EINVAL; > > + > > + /* > > + * Data hold time must be less than 0.9us in fast mode and > > + * 3.45us in standard mode. > > + * Data hold time = IICB0WL[9:2] / PCLK > > + */ > > + hold_time_ns = (u64)(priv->iicb0wl >> 2) * NSEC_PER_SEC / > pclk_hz; > > div64_ul() Ok > > + if (hold_time_ns > config->min_hold_time_ns) { > > + dev_err(dev, "data hold time %dns is over %dns\n", > > + hold_time_ns, config->min_hold_time_ns); > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > > +static int rzv2m_i2c_write_with_ACK(struct rzv2m_i2c_priv *priv, u32 > data) > > rzv2m_i2c_write_with_ack Ok <snip> > > + *data = (u8)(data_tmp & 0xff); > > No need to cast or mask. Ok > > + > > + return 0; > > +} > > + > > +static int rzv2m_i2c_send(struct rzv2m_i2c_priv *priv, struct i2c_msg > *msg, > > + int *count) > > unsigned int *count Ok > > +{ > > + int i, ret = 0; > > unsigned int i Ok > > + > > + for (i = 0; i < msg->len; i++) { > > + ret = rzv2m_i2c_write_with_ACK(priv, msg->buf[i]); > > + if (ret < 0) > > + break; > > return ret? Yes, makes sense as *count is only used if ret is >= 0 > > + } > > + *count = i; > > + > > + return ret; > > +} > > + > > +static int rzv2m_i2c_receive(struct rzv2m_i2c_priv *priv, struct > i2c_msg *msg, > > + int *count) > > unsigned int *count Ok > > +{ > > + int i, ret = 0; > > unsigned int i Ok > > + > > + for (i = 0; i < msg->len; i++) { > > + ret = rzv2m_i2c_read_with_ACK(priv, &msg->buf[i], > > + ((msg->len - 1) == i)); > > + if (ret < 0) > > + break; > > return ret? Ok > > + } > > + *count = i; > > + > > + return ret; > > +} > > + > > +static int rzv2m_i2c_send_address(struct rzv2m_i2c_priv *priv, > > + struct i2c_msg *msg, int read) > > No need to pass read, as you have access to the full msg, and 10-bit > addressing is rare? Ok, makes sense > > +{ > > + u32 addr; > > + int ret; > > + > > + if (msg->flags & I2C_M_TEN) { > > + /* 10-bit address > > + * addr_1: 5'b11110 | addr[9:8] | (R/nW) > > + * addr_2: addr[7:0] > > + */ > > + addr = 0xf0 | ((msg->addr >> 7) & 0x06); > > + addr |= read; > > + /* Send 1st address(extend code) */ > > + ret = rzv2m_i2c_write_with_ACK(priv, addr); > > + if (ret == 0) { > > + /* Send 2nd address */ > > + ret = rzv2m_i2c_write_with_ACK(priv, msg->addr & > 0xff); > > + } > > + } else { > > + /* 7-bit address */ > > + addr = i2c_8bit_addr_from_msg(msg); > > + ret = rzv2m_i2c_write_with_ACK(priv, addr); > > + } > > + > > + return ret; > > +} > > > +static int rzv2m_i2c_master_xfer1(struct rzv2m_i2c_priv *priv, > > + struct i2c_msg *msg, int stop) > > +{ > > + int count = 0; > > unsigned int Ok > > + int ret, read = !!(msg->flags & I2C_M_RD); > > + > > + /* Send start condition */ > > + writel(IICB0STT, priv->base + IICB0TRG); > > + > > + ret = rzv2m_i2c_send_address(priv, msg, read); > > + > > Please drop this blank line. Ok > > + if (ret == 0) { > > + if (read) > > + ret = rzv2m_i2c_receive(priv, msg, &count); > > + else > > + ret = rzv2m_i2c_send(priv, msg, &count); > > + > > + if ((ret == 0) && stop) > > + ret = rzv2m_i2c_stop_condition(priv); > > + } > > + > > + if (ret == -ENXIO) > > + rzv2m_i2c_stop_condition(priv); > > + else if (ret < 0) > > + rzv2m_i2c_init(priv); > > + else > > + ret = count; > > + > > + return ret; > > +} > > + > > +static int rzv2m_i2c_master_xfer(struct i2c_adapter *adap, > > + struct i2c_msg *msgs, int num) > > +{ > > + struct rzv2m_i2c_priv *priv = i2c_get_adapdata(adap); > > + struct device *dev = rzv2m_i2c_priv_to_dev(priv); > > + int ret, i; > > unsigned int i Ok > > + > > + pm_runtime_get_sync(dev); > > Please use pm_runtime_resume_and_get() in new code > (and check its return code?). Ok > > + > > + if (readl(priv->base + IICB0STR0) & IICB0SSBS) { > > + ret = -EAGAIN; > > + goto out; > > + } > > + > > + /* I2C main transfer */ > > + for (i = 0; i < num; i++) { > > + ret = rzv2m_i2c_master_xfer1(priv, &msgs[i], (i == (num > - 1))); > > + if (ret < 0) > > + goto out; > > + } > > + ret = num; > > + > > +out: > > + pm_runtime_put(dev); I notice that a lot of i2c drivers use pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend() here. I think they make sense here as well. > > + > > + return ret; > > +} > > > +static struct i2c_algorithm rzv2m_i2c_algo = { > > + .master_xfer = rzv2m_i2c_master_xfer, > > + .functionality = rzv2m_i2c_func, > > No .(un)reg_slave()? ;-) > The hardware seems to support it. You are right that the HW supports it, though I haven’t tested it! I currently don't have something I can test it with either. Thanks! Phil
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index b1d7069dd377..9e3f9eb1ea3c 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -984,6 +984,16 @@ config I2C_RK3X This driver can also be built as a module. If so, the module will be called i2c-rk3x. +config I2C_RZV2M + tristate "Renesas RZ/V2M adapter" + depends on ARCH_RENESAS || COMPILE_TEST + help + If you say yes to this option, support will be included for the + Renesas RZ/V2M I2C interface. + + This driver can also be built as a module. If so, the module + will be called i2c-rzv2m. + config I2C_S3C2410 tristate "S3C/Exynos I2C Driver" depends on ARCH_EXYNOS || ARCH_S3C24XX || ARCH_S3C64XX || \ diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index b0a10e5d9ee9..7792ffc591f0 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -101,6 +101,7 @@ obj-$(CONFIG_I2C_QCOM_GENI) += i2c-qcom-geni.o obj-$(CONFIG_I2C_QUP) += i2c-qup.o obj-$(CONFIG_I2C_RIIC) += i2c-riic.o obj-$(CONFIG_I2C_RK3X) += i2c-rk3x.o +obj-$(CONFIG_I2C_RZV2M) += i2c-rzv2m.o obj-$(CONFIG_I2C_S3C2410) += i2c-s3c2410.o obj-$(CONFIG_I2C_SH7760) += i2c-sh7760.o obj-$(CONFIG_I2C_SH_MOBILE) += i2c-sh_mobile.o diff --git a/drivers/i2c/busses/i2c-rzv2m.c b/drivers/i2c/busses/i2c-rzv2m.c new file mode 100644 index 000000000000..209171f4ff43 --- /dev/null +++ b/drivers/i2c/busses/i2c-rzv2m.c @@ -0,0 +1,530 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for the Renesas RZ/V2M I2C unit + * + * Copyright (C) 2016-2022 Renesas Electronics Corporation + */ + +#include <linux/clk.h> +#include <linux/err.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/iopoll.h> +#include <linux/i2c.h> +#include <linux/jiffies.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/reset.h> + +/* Register offsets */ +#define IICB0DAT 0x00 /* Data Register */ +#define IICB0CTL0 0x08 /* Control Register 0 */ +#define IICB0TRG 0x0C /* Trigger Register */ +#define IICB0STR0 0x10 /* Status Register 0 */ +#define IICB0CTL1 0x20 /* Control Register 1 */ +#define IICB0WL 0x24 /* Low Level Width Setting Reg */ +#define IICB0WH 0x28 /* How Level Width Setting Reg */ + +/* IICB0CTL0 */ +#define IICB0IICE BIT(7) /* I2C Enable */ +#define IICB0SLWT BIT(1) /* Interrupt Request Timing */ +#define IICB0SLAC BIT(0) /* Acknowledge */ + +/* IICB0TRG */ +#define IICB0WRET BIT(2) /* Quit Wait Trigger */ +#define IICB0STT BIT(1) /* Create Start Condition Trigger */ +#define IICB0SPT BIT(0) /* Create Stop Condition Trigger */ + +/* IICB0STR0 */ +#define IICB0SSAC BIT(8) /* Ack Flag */ +#define IICB0SSBS BIT(6) /* Bus Flag */ +#define IICB0SSSP BIT(4) /* Stop Condition Flag */ + +/* IICB0CTL1 */ +#define IICB0MDSC BIT(7) /* Bus Mode */ +#define IICB0SLSE BIT(1) /* Start condition output */ + +#define rzv2m_i2c_priv_to_dev(p) ((p)->adap.dev.parent) + +#define bit_setl(addr, val) writel(readl(addr) | (val), (addr)) +#define bit_clrl(addr, val) writel(readl(addr) & ~(val), (addr)) + +struct rzv2m_i2c_priv { + void __iomem *base; + struct i2c_adapter adap; + struct clk *clk; + int bus_mode; + struct completion msg_tia_done; + u32 iicb0wl; + u32 iicb0wh; +}; + +enum bcr_index { + RZV2M_I2C_100K = 0, + RZV2M_I2C_400K, +}; + +struct bitrate_config { + unsigned int percent_low; + unsigned int min_hold_time_ns; +}; + +static const struct bitrate_config bitrate_configs[] = { + {47, 3450}, + {52, 900}, +}; + +static irqreturn_t rzv2m_i2c_tia_irq_handler(int this_irq, void *dev_id) +{ + struct rzv2m_i2c_priv *priv = dev_id; + + complete(&priv->msg_tia_done); + + return IRQ_HANDLED; +} + +/* Calculate IICB0WL and IICB0WH */ +static int rzv2m_i2c_clock_calculate(struct device *dev, + struct rzv2m_i2c_priv *priv) +{ + const struct bitrate_config *config; + unsigned int hold_time_ns; + unsigned int total_pclks; + unsigned int trf_pclks; + unsigned long pclk_hz; + struct i2c_timings t; + u32 trf_ns; + + i2c_parse_fw_timings(dev, &t, true); + + pclk_hz = clk_get_rate(priv->clk); + total_pclks = pclk_hz / t.bus_freq_hz; + + trf_ns = t.scl_rise_ns + t.scl_fall_ns; + trf_pclks = (u64)pclk_hz * trf_ns / NSEC_PER_SEC; + + /* Config setting */ + switch (t.bus_freq_hz) { + case I2C_MAX_FAST_MODE_FREQ: + priv->bus_mode = RZV2M_I2C_400K; + break; + case I2C_MAX_STANDARD_MODE_FREQ: + priv->bus_mode = RZV2M_I2C_100K; + break; + default: + dev_err(dev, "transfer speed is invalid\n"); + return -EINVAL; + } + config = &bitrate_configs[priv->bus_mode]; + + /* IICB0WL = (percent_low / Transfer clock) x PCLK */ + priv->iicb0wl = total_pclks * config->percent_low / 100; + if (priv->iicb0wl > 0x3ff) + return -EINVAL; + + /* IICB0WH = ((percent_high / Transfer clock) x PCLK) - (tR + tF) */ + priv->iicb0wh = total_pclks - priv->iicb0wl - trf_pclks; + if (priv->iicb0wh > 0x3ff) + return -EINVAL; + + /* + * Data hold time must be less than 0.9us in fast mode and + * 3.45us in standard mode. + * Data hold time = IICB0WL[9:2] / PCLK + */ + hold_time_ns = (u64)(priv->iicb0wl >> 2) * NSEC_PER_SEC / pclk_hz; + if (hold_time_ns > config->min_hold_time_ns) { + dev_err(dev, "data hold time %dns is over %dns\n", + hold_time_ns, config->min_hold_time_ns); + return -EINVAL; + } + + return 0; +} + +static void rzv2m_i2c_init(struct rzv2m_i2c_priv *priv) +{ + u32 i2c_ctl0; + u32 i2c_ctl1; + + /* i2c disable */ + writel(0, priv->base + IICB0CTL0); + + /* IICB0CTL1 setting */ + i2c_ctl1 = IICB0SLSE; + if (priv->bus_mode == RZV2M_I2C_400K) + i2c_ctl1 |= IICB0MDSC; + writel(i2c_ctl1, priv->base + IICB0CTL1); + + /* IICB0WL IICB0WH setting */ + writel(priv->iicb0wl, priv->base + IICB0WL); + writel(priv->iicb0wh, priv->base + IICB0WH); + + /* i2c enable after setting */ + i2c_ctl0 = IICB0SLWT | IICB0SLAC | IICB0IICE; + writel(i2c_ctl0, priv->base + IICB0CTL0); +} + +static int rzv2m_i2c_write_with_ACK(struct rzv2m_i2c_priv *priv, u32 data) +{ + unsigned long time_left; + + reinit_completion(&priv->msg_tia_done); + + writel(data, priv->base + IICB0DAT); + + time_left = wait_for_completion_timeout(&priv->msg_tia_done, + priv->adap.timeout); + if (!time_left) + return -ETIMEDOUT; + + /* Confirm ACK */ + if ((readl(priv->base + IICB0STR0) & IICB0SSAC) != IICB0SSAC) + return -ENXIO; + + return 0; +} + +static int rzv2m_i2c_read_with_ACK(struct rzv2m_i2c_priv *priv, u8 *data, + bool last) +{ + unsigned long time_left; + u32 data_tmp; + + reinit_completion(&priv->msg_tia_done); + + /* Interrupt request timing : 8th clock */ + bit_clrl(priv->base + IICB0CTL0, IICB0SLWT); + + /* Exit the wait state */ + writel(IICB0WRET, priv->base + IICB0TRG); + + /* Wait for transaction */ + time_left = wait_for_completion_timeout(&priv->msg_tia_done, + priv->adap.timeout); + if (!time_left) + return -ETIMEDOUT; + + if (!last) { + /* Read data */ + data_tmp = readl(priv->base + IICB0DAT); + } else { + /* Disable ACK */ + bit_clrl(priv->base + IICB0CTL0, IICB0SLAC); + + /* Read data*/ + data_tmp = readl(priv->base + IICB0DAT); + + /* Interrupt request timing : 9th clock */ + bit_setl(priv->base + IICB0CTL0, IICB0SLWT); + + /* Exit the wait state */ + writel(IICB0WRET, priv->base + IICB0TRG); + + /* Wait for transaction */ + time_left = wait_for_completion_timeout(&priv->msg_tia_done, + priv->adap.timeout); + if (!time_left) + return -ETIMEDOUT; + + /* Enable ACK */ + bit_setl(priv->base + IICB0CTL0, IICB0SLAC); + } + + *data = (u8)(data_tmp & 0xff); + + return 0; +} + +static int rzv2m_i2c_send(struct rzv2m_i2c_priv *priv, struct i2c_msg *msg, + int *count) +{ + int i, ret = 0; + + for (i = 0; i < msg->len; i++) { + ret = rzv2m_i2c_write_with_ACK(priv, msg->buf[i]); + if (ret < 0) + break; + } + *count = i; + + return ret; +} + +static int rzv2m_i2c_receive(struct rzv2m_i2c_priv *priv, struct i2c_msg *msg, + int *count) +{ + int i, ret = 0; + + for (i = 0; i < msg->len; i++) { + ret = rzv2m_i2c_read_with_ACK(priv, &msg->buf[i], + ((msg->len - 1) == i)); + if (ret < 0) + break; + } + *count = i; + + return ret; +} + +static int rzv2m_i2c_send_address(struct rzv2m_i2c_priv *priv, + struct i2c_msg *msg, int read) +{ + u32 addr; + int ret; + + if (msg->flags & I2C_M_TEN) { + /* 10-bit address + * addr_1: 5'b11110 | addr[9:8] | (R/nW) + * addr_2: addr[7:0] + */ + addr = 0xf0 | ((msg->addr >> 7) & 0x06); + addr |= read; + /* Send 1st address(extend code) */ + ret = rzv2m_i2c_write_with_ACK(priv, addr); + if (ret == 0) { + /* Send 2nd address */ + ret = rzv2m_i2c_write_with_ACK(priv, msg->addr & 0xff); + } + } else { + /* 7-bit address */ + addr = i2c_8bit_addr_from_msg(msg); + ret = rzv2m_i2c_write_with_ACK(priv, addr); + } + + return ret; +} + +static int rzv2m_i2c_stop_condition(struct rzv2m_i2c_priv *priv) +{ + u32 value; + + /* Send stop condition */ + writel(IICB0SPT, priv->base + IICB0TRG); + return readl_poll_timeout(priv->base + IICB0STR0, + value, value & IICB0SSSP, + 100, jiffies_to_usecs(priv->adap.timeout)); +} + +static int rzv2m_i2c_master_xfer1(struct rzv2m_i2c_priv *priv, + struct i2c_msg *msg, int stop) +{ + int count = 0; + int ret, read = !!(msg->flags & I2C_M_RD); + + /* Send start condition */ + writel(IICB0STT, priv->base + IICB0TRG); + + ret = rzv2m_i2c_send_address(priv, msg, read); + + if (ret == 0) { + if (read) + ret = rzv2m_i2c_receive(priv, msg, &count); + else + ret = rzv2m_i2c_send(priv, msg, &count); + + if ((ret == 0) && stop) + ret = rzv2m_i2c_stop_condition(priv); + } + + if (ret == -ENXIO) + rzv2m_i2c_stop_condition(priv); + else if (ret < 0) + rzv2m_i2c_init(priv); + else + ret = count; + + return ret; +} + +static int rzv2m_i2c_master_xfer(struct i2c_adapter *adap, + struct i2c_msg *msgs, int num) +{ + struct rzv2m_i2c_priv *priv = i2c_get_adapdata(adap); + struct device *dev = rzv2m_i2c_priv_to_dev(priv); + int ret, i; + + pm_runtime_get_sync(dev); + + if (readl(priv->base + IICB0STR0) & IICB0SSBS) { + ret = -EAGAIN; + goto out; + } + + /* I2C main transfer */ + for (i = 0; i < num; i++) { + ret = rzv2m_i2c_master_xfer1(priv, &msgs[i], (i == (num - 1))); + if (ret < 0) + goto out; + } + ret = num; + +out: + pm_runtime_put(dev); + + return ret; +} + +static u32 rzv2m_i2c_func(struct i2c_adapter *adap) +{ + return I2C_FUNC_I2C | (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK) | + I2C_FUNC_10BIT_ADDR; +} + +static const struct i2c_adapter_quirks rzv2m_i2c_quirks = { + .flags = I2C_AQ_NO_ZERO_LEN, +}; + +static struct i2c_algorithm rzv2m_i2c_algo = { + .master_xfer = rzv2m_i2c_master_xfer, + .functionality = rzv2m_i2c_func, +}; + +static const struct of_device_id rzv2m_i2c_ids[] = { + { .compatible = "renesas,rzv2m-i2c", }, + { } +}; +MODULE_DEVICE_TABLE(of, rzv2m_i2c_ids); + +static int rzv2m_i2c_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct rzv2m_i2c_priv *priv; + struct reset_control *rstc; + struct i2c_adapter *adap; + struct resource *res; + int irq, ret; + + priv = devm_kzalloc(dev, sizeof(struct rzv2m_i2c_priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); + if (IS_ERR(priv->base)) + return PTR_ERR(priv->base); + + priv->clk = devm_clk_get(dev, NULL); + if (IS_ERR(priv->clk)) { + dev_err_probe(dev, PTR_ERR(priv->clk), "Can't get clock\n"); + return PTR_ERR(priv->clk); + } + + rstc = devm_reset_control_get(dev, NULL); + if (IS_ERR(rstc)) { + dev_err_probe(dev, PTR_ERR(rstc), "Missing reset ctrl\n"); + return PTR_ERR(rstc); + } + /* + * The reset also affects other HW that is not under the control + * of Linux. Therefore, all we can do is deassert the reset. + */ + reset_control_deassert(rstc); + + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return irq; + + ret = devm_request_irq(dev, irq, rzv2m_i2c_tia_irq_handler, 0, + dev_name(dev), priv); + if (ret < 0) { + dev_err_probe(dev, ret, "Unable to request irq %d\n", irq); + return ret; + } + + adap = &priv->adap; + adap->nr = pdev->id; + adap->algo = &rzv2m_i2c_algo; + adap->quirks = &rzv2m_i2c_quirks; + adap->class = I2C_CLASS_DEPRECATED; + adap->dev.parent = dev; + adap->dev.of_node = dev->of_node; + adap->owner = THIS_MODULE; + i2c_set_adapdata(adap, priv); + strscpy(adap->name, pdev->name, sizeof(adap->name)); + init_completion(&priv->msg_tia_done); + + ret = rzv2m_i2c_clock_calculate(dev, priv); + if (ret < 0) + return ret; + + pm_runtime_enable(dev); + + pm_runtime_get_sync(dev); + rzv2m_i2c_init(priv); + pm_runtime_put(dev); + + platform_set_drvdata(pdev, priv); + + ret = i2c_add_numbered_adapter(adap); + if (ret < 0) + pm_runtime_disable(dev); + + return ret; +} + +static int rzv2m_i2c_remove(struct platform_device *pdev) +{ + struct rzv2m_i2c_priv *priv = platform_get_drvdata(pdev); + struct device *dev = rzv2m_i2c_priv_to_dev(priv); + + i2c_del_adapter(&priv->adap); + bit_clrl(priv->base + IICB0CTL0, IICB0IICE); + pm_runtime_disable(dev); + + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int rzv2m_i2c_suspend(struct device *dev) +{ + struct rzv2m_i2c_priv *priv = dev_get_drvdata(dev); + + pm_runtime_get_sync(dev); + bit_clrl(priv->base + IICB0CTL0, IICB0IICE); + pm_runtime_put(dev); + + return 0; +} + +static int rzv2m_i2c_resume(struct device *dev) +{ + struct rzv2m_i2c_priv *priv = dev_get_drvdata(dev); + int ret; + + ret = rzv2m_i2c_clock_calculate(dev, priv); + if (ret < 0) + return ret; + + pm_runtime_get_sync(dev); + rzv2m_i2c_init(priv); + pm_runtime_put(dev); + + return 0; +} + +static const struct dev_pm_ops rzv2m_i2c_pm_ops = { + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(rzv2m_i2c_suspend, rzv2m_i2c_resume) +}; + +#define DEV_PM_OPS (&rzv2m_i2c_pm_ops) +#else +#define DEV_PM_OPS NULL +#endif /* CONFIG_PM_SLEEP */ + +static struct platform_driver rzv2m_i2c_driver = { + .driver = { + .name = "rzv2m-i2c", + .pm = DEV_PM_OPS, + .of_match_table = rzv2m_i2c_ids, + }, + .probe = rzv2m_i2c_probe, + .remove = rzv2m_i2c_remove, +}; +module_platform_driver(rzv2m_i2c_driver); + +MODULE_DESCRIPTION("RZ/V2M I2C bus driver"); +MODULE_AUTHOR("Renesas Electronics Corporation"); +MODULE_LICENSE("GPL");