Re: [PATCH 5.10.y-cip 19/23] i2c: Add Renesas RZ/V2M controller

Pavel Machek
2023-05-24T07:51:03Z

[-- Attachment #1: Type: text/plain, Size: 1526 bytes --]

Hi!

Minor details I noticed during review (they should not prevent merge):

> commit e0ca796a151bd4de76efd71aade2b4805a95c93d upstream.
>
> Yet another i2c controller from Renesas that is found on the RZ/V2M
> (r9a09g011) SoC. It can support only 100kHz and 400KHz operation.

KHz should be kHz.

> +++ b/drivers/i2c/busses/Kconfig
> @@ -961,6 +961,16 @@ config HAVE_S3C2410_I2C
> include I2C support for any machine, kindly select this in the
> respective Kconfig file.
>
> +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.

Double space (" ").

> +#define bit_setl(addr, val) writel(readl(addr) | (val), (addr))
> +#define bit_clrl(addr, val) writel(readl(addr) & ~(val), (addr))

Normally we'd put macro names in all uppercase.

> +static int rzv2m_i2c_probe(struct platform_device *pdev)
> +{
...
> + 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);

Is it possible that bit_clrl(priv->base + IICB0CTL0, IICB0IICE); is
missing before pm_runtime_disable?

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]