[U-Boot] [PATCH 1/4] i2c: UniPhier: add driver for UniPhier i2c controller

[Masahiro Yamada]

Simon, Heiko,


If you have no objection to this series,
I will apply it to u-boot-uniphier.

Is this OK?


Some replies below to Simon's comment.



On Tue, 23 Dec 2014 12:54:53 -0700
Simon Glass <sjg at chromium.org> wrote:


> > +
> > +#define IOBUS_FREQ     100000000
> > +
> > +struct uniphier_i2c_dev {
> > +       void __iomem *base;             /* register base */
> 
> U-Boot normally uses a struct for register access.


IMHO, we can play it by ear.

I notice two disadvantages of struct strategy.

[1] we have to intert dummy registers where no register exist.

For example, there is no register at offset address 0x8
of fi2c controller.

struct uniphier_fi2c_regs {
     u32  cr;
     u32  dttx;
     u32  __dummy;   <-- necessary
     u32  slad;
        ...

[2] It is difficult to describe alias register name for the same address.

For example,

#define I2C_DTTX	0x04		/* send FIFO */
#define I2C_DTRX	0x04		/* receive FIFO */

DTTX is write-only register.
DTRX is read-only register.

They are assigned to the same address.






> > +       unsigned long input_clk;        /* master clock (Hz) */
> > +       unsigned long wait_us;          /* wait for every byte transfer (us) */
> > +};
> > +
> > +static int uniphier_i2c_probe(struct udevice *dev)
> > +{
> > +       fdt_addr_t addr;
> > +       fdt_size_t size;
> > +       struct uniphier_i2c_dev *priv = dev_get_priv(dev);
> > +
> > +       addr = fdtdec_get_addr_size(gd->fdt_blob, dev->of_offset, "reg", &size);
> > +
> > +       priv->base = map_sysmem(addr, size);
> > +
> > +       if (!priv->base)
> > +               return -ENOMEM;
> > +
> > +       priv->input_clk = IOBUS_FREQ;
> > +
> > +       /* deassert reset */
> > +       writel(0x3, priv->base + I2C_BRST);
> > +
> > +       return 0;
> > +}
> > +
> > +static int uniphier_i2c_remove(struct udevice *dev)
> > +{
> > +       struct uniphier_i2c_dev *priv = dev_get_priv(dev);
> > +
> > +       unmap_sysmem(priv->base);
> > +
> > +       return 0;
> > +}
> > +
> > +static int uniphier_i2c_child_pre_probe(struct udevice *dev)
> > +{
> > +       struct dm_i2c_chip *i2c_chip = dev_get_parentdata(dev);
> > +
> > +       if (dev->of_offset == -1)
> > +               return 0;
> > +       return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset,
> > +                                          i2c_chip);
> > +}
> > +
> > +static int send_and_recv_byte(struct uniphier_i2c_dev *dev, u32 dtrm)
> > +{
> > +       writel(dtrm, dev->base + I2C_DTRM);
> > +
> > +       /*
> > +        * U-Boot does not have a good support of interrupt.
> > +        * Wait for a while.
> > +        */
> > +       udelay(dev->wait_us);
> 
> It might be possible to check a 'ready' bit in the hardware, but I
> suppose this works and is simple for what sounds like old hardware.


Unfortunately, there is no 'ready' bit in this I2C controller.
This hardware expects intrruption is checked.

It is true that we can check the GIC register of ARM processor,
but I think it is too much for this driver.


> > +static int uniphier_i2c_set_bus_speed(struct udevice *bus, unsigned int speed)
> > +{
> > +       struct uniphier_i2c_dev *priv = dev_get_priv(bus);
> > +
> > +       /* max supported frequency is 400 kHz */
> > +       if (speed > 400000)
> > +               return -EINVAL;
> > +
> > +       /* bus reset: make sure the bus is idle when change the freqency */
> > +       writel(0x1, priv->base + I2C_BRST);
> > +
> > +       writel((priv->input_clk / speed / 2 << 16) | (priv->input_clk / speed),
> > +              priv->base + I2C_CLK);
> > +
> > +       writel(0x3, priv->base + I2C_BRST);
> > +
> > +       priv->wait_us = 20000000 / speed;
> 
> How is that calculated? Perhaps have a comment?

OK, I will add some comments.
No precise theory for this line.  Rough calculation.



Best Regards
Masahiro Yamada