[U-Boot] [PATCH 1/4] i2c: i2c-cdns: Detect unsupported sequences for rev 1.0
Moritz Fischer
moritz.fischer at ettus.com
Mon Jan 2 20:35:43 CET 2017
Hi Michal,
On Mon, Jan 2, 2017 at 6:29 AM, Michal Simek <michal.simek at xilinx.com> wrote:
> +Siva: please test it.
>
> On 27.12.2016 23:46, Moritz Fischer wrote:
>> Revision 1.0 of this IP has a couple of issues, such as not supporting
>> repeated start conditions for read transfers.
>>
>> So scan through the list of i2c messages for these conditions
>> and report an error if they are attempted.
>>
>> This has been fixed for revision 1.4 of the IP, so only report the error
>> when the IP can really not do it.
>>
>> Signed-off-by: Moritz Fischer <moritz.fischer at ettus.com>
>> Cc: Heiko Schocher <hs at denx.de>
>> Cc: Michal Simek <michal.simek at xilinx.com>
>> Cc: u-boot at lists.denx.de
>> ---
>> drivers/i2c/i2c-cdns.c | 69 ++++++++++++++++++++++++++++++++++++++++----------
>> 1 file changed, 55 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/i2c/i2c-cdns.c b/drivers/i2c/i2c-cdns.c
>> index f49f60b..c69e7e8 100644
>> --- a/drivers/i2c/i2c-cdns.c
>> +++ b/drivers/i2c/i2c-cdns.c
>> @@ -67,6 +67,7 @@ struct cdns_i2c_regs {
>>
>> #define CDNS_I2C_FIFO_DEPTH 16
>> #define CDNS_I2C_TRANSFER_SIZE_MAX 255 /* Controller transfer limit */
>> +#define CDNS_I2C_BROKEN_HOLD_BIT BIT(0)
>>
>> #ifdef DEBUG
>> static void cdns_i2c_debug_status(struct cdns_i2c_regs *cdns_i2c)
>> @@ -114,6 +115,13 @@ struct i2c_cdns_bus {
>> int id;
>> unsigned int input_freq;
>> struct cdns_i2c_regs __iomem *regs; /* register base */
>> +
>
> no reason.
>
>> + int hold_flag;
>> + u32 quirks;
>> +};
>> +
>> +struct cdns_i2c_platform_data {
>> + u32 quirks;
>> };
>>
>> /* Wait for an interrupt */
>> @@ -236,18 +244,14 @@ static int cdns_i2c_probe_chip(struct udevice *bus, uint chip_addr,
>> }
>>
>> static int cdns_i2c_write_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
>> - u32 len, bool next_is_read)
>> + u32 len)
>> {
>> u8 *cur_data = data;
>>
>> struct cdns_i2c_regs *regs = i2c_bus->regs;
>>
>> - setbits_le32(®s->control, CDNS_I2C_CONTROL_CLR_FIFO |
>> - CDNS_I2C_CONTROL_HOLD);
>> + setbits_le32(®s->control, CDNS_I2C_CONTROL_CLR_FIFO);
>>
>> - /* if next is a read, we need to clear HOLD, doesn't work */
>> - if (next_is_read)
>> - clrbits_le32(®s->control, CDNS_I2C_CONTROL_HOLD);
>>
>
> two blank line after removing this code.
Whoops...
>
>> clrbits_le32(®s->control, CDNS_I2C_CONTROL_RW);
>>
>> @@ -267,7 +271,9 @@ static int cdns_i2c_write_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
>> }
>>
>> /* All done... release the bus */
>> - clrbits_le32(®s->control, CDNS_I2C_CONTROL_HOLD);
>> + if (!i2c_bus->hold_flag)
>> + clrbits_le32(®s->control, CDNS_I2C_CONTROL_HOLD);
>> +
>> /* Wait for the address and data to be sent */
>> if (!cdns_i2c_wait(regs, CDNS_I2C_INTERRUPT_COMP))
>> return -ETIMEDOUT;
>> @@ -285,7 +291,7 @@ static int cdns_i2c_read_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
>> struct cdns_i2c_regs *regs = i2c_bus->regs;
>>
>> /* Check the hardware can handle the requested bytes */
>> - if ((len < 0) || (len > CDNS_I2C_TRANSFER_SIZE_MAX))
>> + if ((len < 0))
>> return -EINVAL;
>>
>> setbits_le32(®s->control, CDNS_I2C_CONTROL_CLR_FIFO |
>> @@ -310,7 +316,8 @@ static int cdns_i2c_read_data(struct i2c_cdns_bus *i2c_bus, u32 addr, u8 *data,
>> *(cur_data++) = readl(®s->data);
>> } while (readl(®s->transfer_size) != 0);
>> /* All done... release the bus */
>> - clrbits_le32(®s->control, CDNS_I2C_CONTROL_HOLD);
>> + if (!i2c_bus->hold_flag)
>> + clrbits_le32(®s->control, CDNS_I2C_CONTROL_HOLD);
>>
>> #ifdef DEBUG
>> cdns_i2c_debug_status(regs);
>> @@ -322,19 +329,43 @@ static int cdns_i2c_xfer(struct udevice *dev, struct i2c_msg *msg,
>> int nmsgs)
>> {
>> struct i2c_cdns_bus *i2c_bus = dev_get_priv(dev);
>> - int ret;
>> + int ret, count;
>> + bool hold_quirk;
>> +
>> +
>
> ditto.
>
>> + printf("i2c_xfer: %d messages\n", nmsgs);
>
> debug?
Yeah, will fix.
>
>
>> + hold_quirk = !!(i2c_bus->quirks & CDNS_I2C_BROKEN_HOLD_BIT);
>> +
>> + if (nmsgs > 1) {
>> + /*
>> + * This controller does not give completion interrupt after a
>> + * master receive message if HOLD bit is set (repeated start),
>> + * resulting in SW timeout. Hence, if a receive message is
>> + * followed by any other message, an error is returned
>> + * indicating that this sequence is not supported.
>> + */
>> + for (count = 0; (count < nmsgs - 1) && hold_quirk; count++) {
>> + if (msg[count].flags & I2C_M_RD) {
>> + printf("Can't do repeated start after a receive message\n");
>> + return -EOPNOTSUPP;
>> + }
>> + }
>> +
>> + i2c_bus->hold_flag = 1;
>> + setbits_le32(&i2c_bus->regs->control, CDNS_I2C_CONTROL_HOLD);
>> + } else {
>> + i2c_bus->hold_flag = 0;
>> + }
>>
>> debug("i2c_xfer: %d messages\n", nmsgs);
>> for (; nmsgs > 0; nmsgs--, msg++) {
>> - bool next_is_read = nmsgs > 1 && (msg[1].flags & I2C_M_RD);
>> -
>> debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len);
>> if (msg->flags & I2C_M_RD) {
>> ret = cdns_i2c_read_data(i2c_bus, msg->addr, msg->buf,
>> msg->len);
>> } else {
>> ret = cdns_i2c_write_data(i2c_bus, msg->addr, msg->buf,
>> - msg->len, next_is_read);
>> + msg->len);
>> }
>> if (ret) {
>> debug("i2c_write: error sending\n");
>> @@ -348,11 +379,16 @@ static int cdns_i2c_xfer(struct udevice *dev, struct i2c_msg *msg,
>> static int cdns_i2c_ofdata_to_platdata(struct udevice *dev)
>> {
>> struct i2c_cdns_bus *i2c_bus = dev_get_priv(dev);
>> + struct cdns_i2c_platform_data *pdata =
>> + (struct cdns_i2c_platform_data *)dev_get_driver_data(dev);
>>
>> i2c_bus->regs = (struct cdns_i2c_regs *)dev_get_addr(dev);
>> if (!i2c_bus->regs)
>> return -ENOMEM;
>>
>> + if (pdata)
>> + i2c_bus->quirks = pdata->quirks;
>> +
>> i2c_bus->input_freq = 100000000; /* TODO hardcode input freq for now */
>>
>> return 0;
>> @@ -364,8 +400,13 @@ static const struct dm_i2c_ops cdns_i2c_ops = {
>> .set_bus_speed = cdns_i2c_set_bus_speed,
>> };
>>
>> +static const struct cdns_i2c_platform_data r1p10_i2c_def = {
>> + .quirks = CDNS_I2C_BROKEN_HOLD_BIT,
>> +};
>> +
>> static const struct udevice_id cdns_i2c_of_match[] = {
>> - { .compatible = "cdns,i2c-r1p10" },
>> + { .compatible = "cdns,i2c-r1p10", .data = (ulong)&r1p10_i2c_def },
>> + { .compatible = "cdns,i2c-r1p14" },
>
> This should be maybe v2 because you have added this in previous patch.
Yeah, the patch series needs to be rebased on top of the other patches
you already applied.
Tbh, I should've just sent all that together as single series, sorry
for the noise
Cheers,
Moritz
More information about the U-Boot
mailing list