[PATCH v2 1/1] drivers: i2c: add brcm iproc I2C driver support

[Rayagonda Kokatanur]

On Wed, Apr 8, 2020 at 9:30 AM Heiko Schocher <hs at denx.de> wrote:
>
> Hello Rayagonda Kokatanur,
>
> Am 26.03.2020 um 11:51 schrieb Rayagonda Kokatanur:
> > Add I2C driver support for Broadcom iproc-based socs.
> >
> > Signed-off-by: Arjun Jyothi <arjun.jyothi at broadcom.com>
> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com>
> > Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli at broadcom.com>
> > Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov at broadcom.com>
> > ---
> > Changes from v1:
> >   -Address all review comments from Heiko Schocher
> >
> >   drivers/i2c/Kconfig     |   8 +
> >   drivers/i2c/Makefile    |   1 +
> >   drivers/i2c/iproc_i2c.c | 713 ++++++++++++++++++++++++++++++++++++++++
> >   drivers/i2c/iproc_i2c.h | 344 +++++++++++++++++++
> >   4 files changed, 1066 insertions(+)
> >   create mode 100644 drivers/i2c/iproc_i2c.c
> >   create mode 100644 drivers/i2c/iproc_i2c.h
> >
> > diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> > index 03d2fed341..71c827927a 100644
> > --- a/drivers/i2c/Kconfig
> > +++ b/drivers/i2c/Kconfig
> > @@ -71,6 +71,14 @@ config SYS_I2C_AT91
> >         i2c-gpio driver unless your system can cope with this limitation.
> >         Binding info: doc/device-tree-bindings/i2c/i2c-at91.txt
> >
> > +config IPROC_I2C
> > +     bool "Broadcom I2C driver"
> > +     depends on DM_I2C
> > +     help
> > +       Broadcom I2C driver.
> > +       Add support for Broadcom I2C driver.
> > +       Say yes here to to enable the Broadco I2C driver.
> > +
> >   config SYS_I2C_FSL
> >          bool "Freescale I2C bus driver"
> >          depends on DM_I2C
> > diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> > index f5a471f887..1ab7a941ca 100644
> > --- a/drivers/i2c/Makefile
> > +++ b/drivers/i2c/Makefile
> > @@ -21,6 +21,7 @@ obj-$(CONFIG_SYS_I2C_FSL) += fsl_i2c.o
> >   obj-$(CONFIG_SYS_I2C_IHS) += ihs_i2c.o
> >   obj-$(CONFIG_SYS_I2C_INTEL) += intel_i2c.o
> >   obj-$(CONFIG_SYS_I2C_IMX_LPI2C) += imx_lpi2c.o
> > +obj-$(CONFIG_IPROC_I2C) += iproc_i2c.o
>
> Sorry for the late comment ... can you please rename this config to
>
> CONFIG_SYS_I2C_IPROC

Will fix this, thank you.

>
> >   obj-$(CONFIG_SYS_I2C_KONA) += kona_i2c.o
> >   obj-$(CONFIG_SYS_I2C_LPC32XX) += lpc32xx_i2c.o
> >   obj-$(CONFIG_SYS_I2C_MESON) += meson_i2c.o
> > diff --git a/drivers/i2c/iproc_i2c.c b/drivers/i2c/iproc_i2c.c
> > new file mode 100644
> > index 0000000000..ea28c18da5
> > --- /dev/null
> > +++ b/drivers/i2c/iproc_i2c.c
> [...]
> > +/*
> > + * This function set clock frequency for SMBus block. As per hardware
> > + * engineering, the clock frequency can be changed dynamically.
> > + */
> > +static int iproc_i2c_set_clk_freq(struct iproc_i2c *bus_prvdata)
> > +{
> > +     struct iproc_i2c_regs *base = bus_prvdata->base;
> > +     unsigned int regval;
> > +
> > +     regval = iproc_i2c_reg_read(&base->timg_cfg);
> > +
> > +     switch (bus_prvdata->bus_speed) {
> > +     case I2C_SPEED_100KHZ:
> > +             regval &= ~CCB_SMB_TIMGCFG_MODE400_MASK;
> > +             break;
> > +
> > +     case I2C_SPEED_400KHZ:
> > +             regval |= CCB_SMB_TIMGCFG_MODE400_MASK;
> > +             break;
>
> Please use here the enums from:
>
> https://gitlab.denx.de/u-boot/u-boot/-/blob/master/include/i2c.h#L44
>
> and drop later your own defines of I2C_SPEED_*

I will use enums from include/i2c.h file and remove our own enums. Thank you.
>
> Thanks!
>
> Beside of this, you can add my
>
> Reviewed-by: Heiko Schocher <hs at denx.de>

Will add, thank you.

Best regards,
Rayagonda

>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de