diff mbox

[RFC,01/11] i2c: add quirk structure to describe adapter flaws

Message ID 1420824103-24169-2-git-send-email-wsa@the-dreams.de
State RFC
Headers show

Commit Message

Wolfram Sang Jan. 9, 2015, 5:21 p.m. UTC
The number of I2C adapters which are not fully I2C compatible is rising,
sadly. Drivers usually do handle the flaws, still the user receives only
some errno for a transfer which normally can be expected to work. This
patch introduces a formal description of flaws. One advantage is that
the core can check before the actual transfer if the messages could be
transferred at all. This is done in the next patch. Another advantage is
that we can pass this information to the user so the restrictions are
exactly known and further actions can be based on that. This will be
done later after some stabilization period for this description.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
 include/linux/i2c.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Comments

Eddie Huang Jan. 16, 2015, 5:50 a.m. UTC | #1
Hi Wolfram,

On Fri, 2015-01-09 at 18:21 +0100, Wolfram Sang wrote:
>  
> + */
> +struct i2c_adapter_quirks {
> +	u64 flags;
> +	int max_num_msgs;
> +	u16 max_write_len;
> +	u16 max_read_len;
> +	u16 max_comb_write_len;
> +	u16 max_comb_read_len;
> +};
> +
> +#define I2C_ADAPTER_QUIRK_COMB_WRITE_FIRST	BIT(0)
> +#define I2C_ADAPTER_QUIRK_COMB_READ_SECOND	BIT(1)
> +#define I2C_ADAPTER_QUIRK_COMB_WRITE_THEN_READ	(I2C_ADAPTER_QUIRK_COMB_WRITE_FIRST | \
> +						I2C_ADAPTER_QUIRK_COMB_READ_SECOND)
> +
>  /*
>   * i2c_adapter is the structure used to identify a physical i2c bus along
>   * with the access algorithms necessary to access it.
> @@ -472,6 +506,7 @@ struct i2c_adapter {
>  	struct list_head userspace_clients;
>  
>  	struct i2c_bus_recovery_info *bus_recovery_info;
> +	struct i2c_adapter_quirks *quirks;
>  };
>  #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
>  

I suggest to add const.
	const struct i2c_adapter_quirks *quirks;

also, in i2c-core.c, should modify:
	const struct i2c_adapter_quirks *q = adap->quirks;



--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yingjoe Chen Jan. 16, 2015, 8:18 a.m. UTC | #2
On Fri, 2015-01-09 at 18:21 +0100, Wolfram Sang wrote:
> The number of I2C adapters which are not fully I2C compatible is rising,
> sadly. Drivers usually do handle the flaws, still the user receives only
> some errno for a transfer which normally can be expected to work. This
> patch introduces a formal description of flaws. One advantage is that
> the core can check before the actual transfer if the messages could be
> transferred at all. This is done in the next patch. Another advantage is
> that we can pass this information to the user so the restrictions are
> exactly known and further actions can be based on that. This will be
> done later after some stabilization period for this description.

Hi Wolfram,

This can describe the behavior of our current upstream driver[1], which
only support combine write-then-read.

After checking with Xudong & HW guys, it seems our HW can do more. 
On MT8135, it can support at most 2 messages, no matter read or write,
with the limitation that the length of the second message must <=
31bytes.

So this RFC is enough for our driver, but it would be better if we could
also support other case.

Joe.C

[1]:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305468.html



--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Jan. 19, 2015, 3 p.m. UTC | #3
Hi,

> This can describe the behavior of our current upstream driver[1], which
> only support combine write-then-read.
> 
> After checking with Xudong & HW guys, it seems our HW can do more. 
> On MT8135, it can support at most 2 messages, no matter read or write,
> with the limitation that the length of the second message must <=
> 31bytes.
> 
> So this RFC is enough for our driver, but it would be better if we could
> also support other case.

Hmm, I think we can convert max_comb_{read|write}_len to
max_comb_{1st|2nd}_msg_len or similar.

I'll check but it will probably not before next week.

Thanks for the input!
Wolfram Sang Jan. 19, 2015, 3:05 p.m. UTC | #4
> > +	struct i2c_adapter_quirks *quirks;
> >  };
> >  #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
> >  
> 
> I suggest to add const.
> 	const struct i2c_adapter_quirks *quirks;
> 
> also, in i2c-core.c, should modify:
> 	const struct i2c_adapter_quirks *q = adap->quirks;

Thanks, I'll think about it.
Wolfram Sang Feb. 24, 2015, 4:04 p.m. UTC | #5
On Mon, Jan 19, 2015 at 04:05:15PM +0100, Wolfram Sang wrote:
> 
> > > +	struct i2c_adapter_quirks *quirks;
> > >  };
> > >  #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
> > >  
> > 
> > I suggest to add const.
> > 	const struct i2c_adapter_quirks *quirks;
> > 
> > also, in i2c-core.c, should modify:
> > 	const struct i2c_adapter_quirks *q = adap->quirks;
> 
> Thanks, I'll think about it.

And added it...
diff mbox

Patch

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index e3a1721c8354..f8a642713706 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -447,6 +447,40 @@  int i2c_recover_bus(struct i2c_adapter *adap);
 int i2c_generic_gpio_recovery(struct i2c_adapter *adap);
 int i2c_generic_scl_recovery(struct i2c_adapter *adap);
 
+/**
+ * struct i2c_adapter_quirks - describe flaws of an i2c adapter
+ * @flags: see I2C_ADAPTER_QUIRK_* for possible flags
+ * @max_num_msgs: maximum number of messages per transfer
+ * @max_write_len: maximum length of a write message
+ * @max_read_len: maximum length of a read message
+ * @max_comb_write_len: maximum length of a write in a combined message
+ * @max_comb_read_len: maximum length of a read in a combined message
+ *
+ * Note about combined messages: Some I2C controllers can only send one
+ * message per transfer, plus something called combined message or
+ * write-then-read. This is a (usually) small write message followed by
+ * a read message and barely enough to access register based slaves like
+ * EEPROMs. There is a flag to support this mode. It implies max_num_msg = 2
+ * and does the length checks with max_comb_*_len because combined message mode
+ * usually has its own limitations. Read/write flags of the messages are also
+ * checked to be proper. Because of HW implementation, some controllers can
+ * actually do write-then-anything. To support that, write-then-read has been
+ * broken out into write-first and read-second.
+ */
+struct i2c_adapter_quirks {
+	u64 flags;
+	int max_num_msgs;
+	u16 max_write_len;
+	u16 max_read_len;
+	u16 max_comb_write_len;
+	u16 max_comb_read_len;
+};
+
+#define I2C_ADAPTER_QUIRK_COMB_WRITE_FIRST	BIT(0)
+#define I2C_ADAPTER_QUIRK_COMB_READ_SECOND	BIT(1)
+#define I2C_ADAPTER_QUIRK_COMB_WRITE_THEN_READ	(I2C_ADAPTER_QUIRK_COMB_WRITE_FIRST | \
+						I2C_ADAPTER_QUIRK_COMB_READ_SECOND)
+
 /*
  * i2c_adapter is the structure used to identify a physical i2c bus along
  * with the access algorithms necessary to access it.
@@ -472,6 +506,7 @@  struct i2c_adapter {
 	struct list_head userspace_clients;
 
 	struct i2c_bus_recovery_info *bus_recovery_info;
+	struct i2c_adapter_quirks *quirks;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)