Re: [PATCH v2] i2c: rk3x: fix bug that cause measured high_ns doesn't meet I2C spec
Addy, On Tue, Dec 2, 2014 at 6:37 PM, Addy Ke wrote: > high_ns calculated from the low division of CLKDIV register is the sum > of actual measured high_ns and rise_ns. The rise time which related to > external pull-up resistor can be up to the maximum rise time in I2C spec. > > In my test, if external pull-up resistor is 4.7K, rise_ns is about > 700ns. So the actual measured high_ns is about 3900ns, which is less > than 4000ns(the minimum high_ns in I2C spec). > > To fix this bug, min_low_ns should include fall time and min_high_ns > should include rise time too. > > This patch merged the patch that Doug submitted to chromium, which > can get the rise and fall times for signals from the device tree. > This allows us to more accurately calculate timings. see: > https://chromium-review.googlesource.com/#/c/232774/ > > Signed-off-by: Addy Ke > --- > Changes in v2: > - merged the patch that Doug submitted to chromium > > Documentation/devicetree/bindings/i2c/i2c-rk3x.txt | 10 > drivers/i2c/busses/i2c-rk3x.c | 55 > +++--- > 2 files changed, 47 insertions(+), 18 deletions(-) This looks good to me. Thank you for spinning. Reviewed-by: Doug Anderson -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] i2c: rk3x: fix bug that cause measured high_ns doesn't meet I2C spec
high_ns calculated from the low division of CLKDIV register is the sum
of actual measured high_ns and rise_ns. The rise time which related to
external pull-up resistor can be up to the maximum rise time in I2C spec.
In my test, if external pull-up resistor is 4.7K, rise_ns is about
700ns. So the actual measured high_ns is about 3900ns, which is less
than 4000ns(the minimum high_ns in I2C spec).
To fix this bug, min_low_ns should include fall time and min_high_ns
should include rise time too.
This patch merged the patch that Doug submitted to chromium, which
can get the rise and fall times for signals from the device tree.
This allows us to more accurately calculate timings. see:
https://chromium-review.googlesource.com/#/c/232774/
Signed-off-by: Addy Ke
---
Changes in v2:
- merged the patch that Doug submitted to chromium
Documentation/devicetree/bindings/i2c/i2c-rk3x.txt | 10
drivers/i2c/busses/i2c-rk3x.c | 55 +++---
2 files changed, 47 insertions(+), 18 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt
b/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt
index dde6c22..faf5997 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-rk3x.txt
@@ -21,6 +21,13 @@ Required on RK3066, RK3188 :
Optional properties :
- clock-frequency : SCL frequency to use (in Hz). If omitted, 100kHz is used.
+ - rise-ns : Number of nanoseconds the signal takes to rise (t(r) in i2c spec).
+If not specified this is assumed to be the max the spec allows
+(1000 ns for standard mode, 300 ns for fast mode) which might
+cause slightly slower communication.
+ - fall-ns : Number of nanoseconds the signal takes to fall (t(f) in the i2c0
+spec). If not specified this is assumed to be the max the spec
+allows (300 ns) which might cause slightly slower communication.
Example:
@@ -39,4 +46,7 @@ i2c0: i2c@2002d000 {
clock-names = "i2c";
clocks = <&cru PCLK_I2C0>;
+
+ rise-ns = <800>;
+ fall-ns = <100>;
};
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 0ee5802..025d4b5 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -102,6 +102,8 @@ struct rk3x_i2c {
/* Settings */
unsigned int scl_frequency;
+ unsigned int rise_ns;
+ unsigned int fall_ns;
/* Synchronization & notification */
spinlock_t lock;
@@ -435,6 +437,8 @@ out:
*
* @clk_rate: I2C input clock rate
* @scl_rate: Desired SCL rate
+ * @rise_ns: How many ns it takes for signals to rise.
+ * @fall_ns: How many ns it takes for signals to fall.
* @div_low: Divider output for low
* @div_high: Divider output for high
*
@@ -443,11 +447,14 @@ out:
* too high, we silently use the highest possible rate.
*/
static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long scl_rate,
+ unsigned long rise_ns, unsigned long fall_ns,
unsigned long *div_low, unsigned long *div_high)
{
+ unsigned long spec_min_low_ns, spec_min_high_ns;
+ unsigned long spec_max_data_hold_ns;
+ unsigned long spec_data_hold_buffer_ns;
+
unsigned long min_low_ns, min_high_ns;
- unsigned long max_data_hold_ns;
- unsigned long data_hold_buffer_ns;
unsigned long max_low_ns, min_total_ns;
unsigned long clk_rate_khz, scl_rate_khz;
@@ -470,28 +477,30 @@ static int rk3x_i2c_calc_divs(unsigned long clk_rate,
unsigned long scl_rate,
/*
* min_low_ns: The minimum number of ns we need to hold low
-* to meet i2c spec
+* to meet i2c spec, should include fall time.
* min_high_ns: The minimum number of ns we need to hold high
-* to meet i2c spec
+* to meet i2c spec, should include rise time.
* max_low_ns: The maximum number of ns we can hold low
-* to meet i2c spec
+* to meet i2c spec.
*
* Note: max_low_ns should be (max data hold time * 2 - buffer)
* This is because the i2c host on Rockchip holds the data line
* for half the low time.
*/
if (scl_rate <= 10) {
- min_low_ns = 4700;
- min_high_ns = 4000;
- max_data_hold_ns = 3450;
- data_hold_buffer_ns = 50;
+ spec_min_low_ns = 4700;
+ spec_min_high_ns = 4000;
+ spec_max_data_hold_ns = 3450;
+ spec_data_hold_buffer_ns = 50;
} else {
- min_low_ns = 1300;
- min_high_ns = 600;
- max_data_hold_ns = 900;
- data_hold_buffer_ns = 50;
+ spec_min_low_ns = 1300;
+ spec_min_high_ns = 600;
+
[PATCH] i2c-omap / PM: Drop CONFIG_PM_RUNTIME from i2c-omap.c
From: Rafael J. Wysocki
After commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if PM_SLEEP is
selected) PM_RUNTIME is always set if PM is set, so some #ifdef blocks
depending on CONFIG_PM_RUNTIME may be dropped now.
Do that in drivers/i2c/busses/i2c-omap.c.
Signed-off-by: Rafael J. Wysocki
---
Note: This depends on commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if
PM_SLEEP is selected) which is only in linux-next at the moment (via the
linux-pm tree).
Please let me know if it is OK to take this one into linux-pm.
---
drivers/i2c/busses/i2c-omap.c |2 --
1 file changed, 2 deletions(-)
Index: linux-pm/drivers/i2c/busses/i2c-omap.c
===
--- linux-pm.orig/drivers/i2c/busses/i2c-omap.c
+++ linux-pm/drivers/i2c/busses/i2c-omap.c
@@ -1280,7 +1280,6 @@ static int omap_i2c_remove(struct platfo
}
#ifdef CONFIG_PM
-#ifdef CONFIG_PM_RUNTIME
static int omap_i2c_runtime_suspend(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
@@ -1318,7 +1317,6 @@ static int omap_i2c_runtime_resume(struc
return 0;
}
-#endif /* CONFIG_PM_RUNTIME */
static struct dev_pm_ops omap_i2c_pm_ops = {
SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend,
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: rk3x: fix bug that cause measured high_ns doesn't meet I2C spec
Addy, On Thu, Nov 6, 2014 at 12:11 AM, Addy Ke wrote: > high_ns calculated from the low division of CLKDIV register is the sum of > actual measured high_ns and rise_ns. The rise time which related to > external pull-up resistor can be up to the maximum rise time in I2C spec. > > In my test, if external pull-up resistor is 4.7K, rise_ns is about 700ns. > So the actual measured high_ns is about 3900ns, which is less than 4000ns > (the minimum high_ns in I2C spec). It's a little unfortunate to have to make the assumption that the rise time for a board is going to be the maximum the spec allows. I think this is something that's better to let a board specify in the device tree. Allowing us to specify it also gives us a little extra slop and makes it easier to specify a "fall time" without affecting the resulting frequency. Right now your code assumes that the fall time is 0. I've prototyped what things could look like if the rise and fall times were specified in the device tree. You can see my patch (atop yours) at: https://chromium-review.googlesource.com/#/c/232774/ ...perhaps you could squash that into your patch and post up v2? -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] devicetree: bindings: Add defeature-repeated-start property for Cadence I2C
On 12/02/2014 03:15 PM, Wolfram Sang wrote: What do you do when disable repeated start? Sending STOP and START? If so, this is really something different than repeated start. By using I2C_FUNC_I2C a user expects repeated start, so if the HW does not support it, we should say so and don't try to emulate it with something different. Yes, we send stop. As said before, this is wrong. Another master could interfere between the messages when using stop+start. This is no replacement for repeated start. More importantly a lot of I2C slaves also reset their internal state machine on a stop. So e.g. if reading a register is implemented by doing start,write,repeated start,read,stop and you replace that with start,write,stop,start,read,stop you'll always read register zero instead of the register you wanted to read. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] devicetree: bindings: Add defeature-repeated-start property for Cadence I2C
> > What do you do when disable repeated start? Sending STOP and START? If > > so, this is really something different than repeated start. By using > > I2C_FUNC_I2C a user expects repeated start, so if the HW does not > > support it, we should say so and don't try to emulate it with something > > different. > > Yes, we send stop. As said before, this is wrong. Another master could interfere between the messages when using stop+start. This is no replacement for repeated start. signature.asc Description: Digital signature
Re: [PATCH 3/4] devicetree: bindings: Add defeature-repeated-start property for Cadence I2C
Hi, On Tue, Dec 2, 2014 at 6:46 PM, Wolfram Sang wrote: > >> But given the bugs, it will be useful to just disable it if the system >> doesn't >> require repeated start. > > What do you do when disable repeated start? Sending STOP and START? If > so, this is really something different than repeated start. By using > I2C_FUNC_I2C a user expects repeated start, so if the HW does not > support it, we should say so and don't try to emulate it with something > different. > Yes, we send stop. Using repeated start, when number of messages passed > 1, HOLD bit is set by the driver. This is an indication to the controller not to send a STOP. If we disable repeated start, the driver will not set HOLD bit. All the messages are sent but with START and a STOP for each of them. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding
> Thank you for references. Yes, thank you all. Really great to see this consolidation effort going on! > I'll try to review all out of i2c/busses/* registered i2c adapters, may > be there is something in common between all of them. Yay, cool! Thanks! > I'll prepare the change of the HDMI DDC support for review (will be able > to test it only on iMX6), Wolfram, please skip from your consideration > the published version of the i2c bus driver. Okay, will do. > Wolfram, by the way is I2C_CLASS_DDC adapter class in operational use or > deprecated? Class based instantiation is NOT deprecated and will stay as far as I can see. Many platform bus drivers did set the class flag needlessly, so we had a deprecation mechanism to get rid of those flags in drivers in order to speed up boot time. signature.asc Description: Digital signature
Re: [PATCH 3/4] devicetree: bindings: Add defeature-repeated-start property for Cadence I2C
> But given the bugs, it will be useful to just disable it if the system doesn't > require repeated start. What do you do when disable repeated start? Sending STOP and START? If so, this is really something different than repeated start. By using I2C_FUNC_I2C a user expects repeated start, so if the HW does not support it, we should say so and don't try to emulate it with something different. > If you think DT entry is not the way to go, do you think a CONFIG option or > something better will work? No, check at runtime if the transfer is possible on this HW. Bail out if not. > We chose a DT property because there is a good chance the user has multiple > cadence I2C controllers - one connected to a slave that needs repeated start > (like a power controller) and another that doesn't care. The user should not need to know with this level of detail if we can avoid it IMO. signature.asc Description: Digital signature
Re: [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding
Hi Andy,
thank you for joining the discussion.
On 02.12.2014 08:36, Andy Yan wrote:
> Hi Vladimir:
>
> I am working on convert imx-hdmi to dw_hdmi now:
> https://lkml.org/lkml/2014/12/1/190
> I also have a plan to use the internal HDMI I2C master under the I2c
> framework,
> and I also have a patch to do this work. So glad to see your work.
> Please also Cc me and zubair.kakak...@imgtec.com,
> maybe Zubair also have interests on your future patch.
> On 2014年12月02日 00:22, Philipp Zabel wrote:
>> Hi Vladimir,
>>
>> [Added Andy Yan to Cc:, because imx-hdmi->dw-hdmi]
>>
>> Am Montag, den 01.12.2014, 17:39 +0200 schrieb Vladimir Zapolskiy:
>>> On 01.12.2014 17:11, Philipp Zabel wrote:
Am Montag, den 01.12.2014, 16:54 +0200 schrieb Vladimir Zapolskiy:
> Hi Philipp and Shawn,
>
> On 15.11.2014 19:49, Vladimir Zapolskiy wrote:
>> Provide information about how to bind internal iMX6Q/DL HDMI DDC I2C
>> master controller. The property is set as optional one, because iMX6
>> HDMI DDC bus may be represented by one of general purpose I2C busses
>> found on SoC.
>>
>> Signed-off-by: Vladimir Zapolskiy
>> Cc: Wolfram Sang
>> Cc: Philipp Zabel
>> Cc: Shawn Guo
>> Cc: devicet...@vger.kernel.org
>> Cc: linux-me...@vger.kernel.org
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: linux-i2c@vger.kernel.org
>> ---
>> Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt | 10
>> +-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>> b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>> index 1b756cf..43c8924 100644
>> --- a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>> +++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>> @@ -10,6 +10,8 @@ Required properties:
>>- #address-cells : should be <1>
>>- #size-cells : should be <0>
>>- compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
>> + If internal HDMI DDC I2C master controller is supposed to be used,
>> + then "simple-bus" should be added to compatible value.
>>- gpr : should be <&gpr>.
>> The phandle points to the iomuxc-gpr region containing the HDMI
>> multiplexer control register.
>> @@ -22,6 +24,7 @@ Required properties:
>>
>> Optional properties:
>>- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
>> + - ddc: internal HDMI DDC I2C master controller
>>
>> example:
>>
>> @@ -32,7 +35,7 @@ example:
>> hdmi: hdmi@012 {
>> #address-cells = <1>;
>> #size-cells = <0>;
>> -compatible = "fsl,imx6q-hdmi";
>> +compatible = "fsl,imx6q-hdmi", "simple-bus";
>> reg = <0x0012 0x9000>;
>> interrupts = <0 115 0x04>;
>> gpr = <&gpr>;
>> @@ -40,6 +43,11 @@ example:
>> clock-names = "iahb", "isfr";
>> ddc-i2c-bus = <&i2c2>;
>>
>> +hdmi_ddc: ddc {
>> +compatible = "fsl,imx6q-hdmi-ddc";
>> +status = "disabled";
>> +};
>> +
>> port@0 {
>> reg = <0>;
>>
>>
> knowing in advance that I2C framework lacks a graceful support of non
> fully compliant I2C devices, do you have any objections to the proposed
> iMX HDMI DTS change?
I'm not sure about this. Have you seen "drm: Decouple EDID parsing from
I2C adapter"? I feel like in the absence of a ddc-i2c-bus property the
imx-hdmi/dw-hdmi driver should try to use the internal HDMI DDC I2C
master controller, bypassing the I2C framework altogether.
>>> My idea is exactly not to bypass the I2C framework, briefly the
>>> rationale is that
>>> * it allows to reuse I2C UAPI/tools naturally applied to the internal
>>> iMX HDMI DDC bus,
>>> * it allows to use iMX HDMI DDC bus as an additional feature-limited I2C
>>> bus on SoC (who knows, I absolutely won't be surprised, if anyone needs
>>> it on practice),
>>> * if an HDMI controller supports an external I2C bus, the integration
>>> with HDMI DDC bus driver based on I2C framework is seamless.
>>>
>>> However I agree that the selected approach may look odd, the question is
>>> if the oddness comes from the technical side or from the fact that
>>> nobody has done it before this way.
>>>
>>> I'm open to any critique, if the proposal of creating an I2C bus from
>>> HDMI DDC bus is lame, then I suppose the shared iMX HDMI DDC bus driver
>>> should be converted to something formless and internally used by
>>> imx-hdmi. The negative side-effects of such a change from my point of
>>> view are
>
Re: [PATCH 3/4] devicetree: bindings: Add defeature-repeated-start property for Cadence I2C
Hi, On Tue, Dec 2, 2014 at 6:22 PM, Wolfram Sang wrote: >> >> + - defeature-repeated-start: Include this property to defeature >> >> repeated start >> >> + This defeature is due to a few bugs in the >> >> + I2C controller. >> >> + Completion interrupt after a read/receive >> >> + operation is NOT obtained if HOLD bit is set >> >> + at that time. Because of this bug, repeated >> >> start >> >> + will only work if there are no transfers >> >> following >> >> + a read/receive transfer. >> >> + If HOLD is held for long without a transfer, >> >> + invalid read transactions are generated by the >> >> + controller due to a HW timeout related bug. >> > >> > I'm not keen on the name; it sounds like we're disabling a feature >> > rather than describing the problem (and "defeature" is not a common >> > term in this sense, "disable" would be better). >> > >> > It sounds like there are two issues with staying in the HOLD state? Lost >> > completion IRQs and a separate HW timeout bug? Or are the two related? >> > >> >> Yes, there are two issues here and they are not related. >> But a combination of both is leading to not using repeated start. >> The intention was to defeature except that it works in some scenarios >> (such as a typical write+read in that order with repeated start) >> and there are people who already use the driver with slaves that need this. > > That should not be handled using a binding. If you get a transfer (at > runtime) with criteria you don't support, return with -EOPNOTSUPP from > the master xfer routine. > I put a check in place for one failure condition that we know (will change the error code returned). But given the bugs, it will be useful to just disable it if the system doesn't require repeated start. If you think DT entry is not the way to go, do you think a CONFIG option or something better will work? We chose a DT property because there is a good chance the user has multiple cadence I2C controllers - one connected to a slave that needs repeated start (like a power controller) and another that doesn't care. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: Add parameters to sysfs-added i2c devices
On 12/01/2014 11:38 AM, Wolfram Sang wrote:
> On Mon, Nov 24, 2014 at 07:43:23AM -0600, Corey Minyard wrote:
>> Ping, I haven't heard anything on this.
> Use cases please :) What client drivers would need that and what params
> do they need?
I'm adding an IPMI SMB driver, it should be going in 3.19. These
devices have an IPMI bus address (not the I2C address), and it needs to
be specified if it varies from the default, or certain messages won't
work correctly.
It seems like this would be useful for other things; it's a little
surprising to me that no other devices need to have some parameters
passed in.
-corey
>> -corey
>>
>> On 11/14/2014 08:41 AM, miny...@acm.org wrote:
>>> From: Corey Minyard
>>>
>>> Some devices might need parameters to control their operation,
>>> add the ability to pass these parameters to the client.
>>>
>>> This also makes the parsing of sysfs-added I2C devices a little
>>> more flexible, allowing tabs and arbitrary numbers of spaces.
>>>
>>> Signed-off-by: Corey Minyard
>>> ---
>>> drivers/i2c/i2c-core.c | 46 ++
>>> include/linux/i2c.h| 3 +++
>>> 2 files changed, 37 insertions(+), 12 deletions(-)
>>>
>>> I'm adding an SMBus IPMI driver, and it needs to be able to have
>>> the IPMI bus address specified in case it's not the default.
>>>
>>> Also, this seems like it would be useful for other devices that
>>> need some sort of configuration.
>>>
>>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>>> index f43b4e1..9a2ab13 100644
>>> --- a/drivers/i2c/i2c-core.c
>>> +++ b/drivers/i2c/i2c-core.c
>>> @@ -781,7 +781,10 @@ static int i2c_device_pm_restore(struct device *dev)
>>>
>>> static void i2c_client_dev_release(struct device *dev)
>>> {
>>> - kfree(to_i2c_client(dev));
>>> + struct i2c_client *client = to_i2c_client(dev);
>>> +
>>> + kfree(client->parms);
>>> + kfree(client);
>>> }
>>>
>>> static ssize_t
>>> @@ -1059,6 +1062,13 @@ i2c_new_device(struct i2c_adapter *adap, struct
>>> i2c_board_info const *info)
>>> client->flags = info->flags;
>>> client->addr = info->addr;
>>> client->irq = info->irq;
>>> + if (info->parms) {
>>> + client->parms = kstrdup(info->parms, GFP_KERNEL);
>>> + if (!client->parms) {
>>> + dev_err(&adap->dev, "Out of memory allocating parms\n");
>>> + goto out_err_silent;
>>> + }
>>> + }
>>>
>>> strlcpy(client->name, info->type, sizeof(client->name));
>>>
>>> @@ -1207,31 +1217,43 @@ i2c_sysfs_new_device(struct device *dev, struct
>>> device_attribute *attr,
>>> struct i2c_adapter *adap = to_i2c_adapter(dev);
>>> struct i2c_board_info info;
>>> struct i2c_client *client;
>>> - char *blank, end;
>>> + char *pos, end;
>>> int res;
>>>
>>> memset(&info, 0, sizeof(struct i2c_board_info));
>>>
>>> - blank = strchr(buf, ' ');
>>> - if (!blank) {
>>> + pos = strpbrk(buf, " \t");
>>> + if (!pos) {
>>> dev_err(dev, "%s: Missing parameters\n", "new_device");
>>> return -EINVAL;
>>> }
>>> - if (blank - buf > I2C_NAME_SIZE - 1) {
>>> + if (pos - buf > I2C_NAME_SIZE - 1) {
>>> dev_err(dev, "%s: Invalid device name\n", "new_device");
>>> return -EINVAL;
>>> }
>>> - memcpy(info.type, buf, blank - buf);
>>> + memcpy(info.type, buf, pos - buf);
>>>
>>> - /* Parse remaining parameters, reject extra parameters */
>>> - res = sscanf(++blank, "%hi%c", &info.addr, &end);
>>> - if (res < 1) {
>>> + while (isspace(*pos))
>>> + pos++;
>>> +
>>> + /* Parse address, saving remaining parameters. */
>>> + res = sscanf(pos, "%hi%c", &info.addr, &end);
>>> + if (res < 1 || !isspace(end)) {
>>> dev_err(dev, "%s: Can't parse I2C address\n", "new_device");
>>> return -EINVAL;
>>> }
>>> - if (res > 1 && end != '\n') {
>>> - dev_err(dev, "%s: Extra parameters\n", "new_device");
>>> - return -EINVAL;
>>> + if (res > 1 && end != '\n') {
>>> + if (isspace(end)) {
>>> + /* Extra parms, skip the address and space. */
>>> + while (!isspace(*pos))
>>> + pos++;
>>> + while (isspace(*pos))
>>> + pos++;
>>> + info.parms = pos;
>>> + } else {
>>> + dev_err(dev, "%s: Extra parameters\n", "new_device");
>>> + return -EINVAL;
>>> + }
>>> }
>>>
>>> client = i2c_new_device(adap, &info);
>>> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
>>> index b556e0a..e40fc56 100644
>>> --- a/include/linux/i2c.h
>>> +++ b/include/linux/i2c.h
>>> @@ -222,6 +222,7 @@ struct i2c_client {
>>> char name[I2C_NAME_SIZE];
>>> struct i2c_adapter *adapter;/* the adapter we sit on*/
>>> struct device dev; /* the device structure */
>
Re: [PATCH 3/4] devicetree: bindings: Add defeature-repeated-start property for Cadence I2C
> >> + - defeature-repeated-start: Include this property to defeature repeated > >> start > >> + This defeature is due to a few bugs in the > >> + I2C controller. > >> + Completion interrupt after a read/receive > >> + operation is NOT obtained if HOLD bit is set > >> + at that time. Because of this bug, repeated > >> start > >> + will only work if there are no transfers > >> following > >> + a read/receive transfer. > >> + If HOLD is held for long without a transfer, > >> + invalid read transactions are generated by the > >> + controller due to a HW timeout related bug. > > > > I'm not keen on the name; it sounds like we're disabling a feature > > rather than describing the problem (and "defeature" is not a common > > term in this sense, "disable" would be better). > > > > It sounds like there are two issues with staying in the HOLD state? Lost > > completion IRQs and a separate HW timeout bug? Or are the two related? > > > > Yes, there are two issues here and they are not related. > But a combination of both is leading to not using repeated start. > The intention was to defeature except that it works in some scenarios > (such as a typical write+read in that order with repeated start) > and there are people who already use the driver with slaves that need this. That should not be handled using a binding. If you get a transfer (at runtime) with criteria you don't support, return with -EOPNOTSUPP from the master xfer routine. That being said, the number of broken/not-fully-compliant I2C controllers has increased a lot recent times (why can't we just use the established old ones?). Maybe we will have core support for a subset of I2C (wr+rd) in the future, but that's still ahead... signature.asc Description: Digital signature
Re: [PATCH 3/4] devicetree: bindings: Add defeature-repeated-start property for Cadence I2C
Hi Mark, On Tue, Dec 2, 2014 at 4:49 PM, Mark Rutland wrote: > On Tue, Dec 02, 2014 at 10:05:48AM +, Harini Katakam wrote: >> From: Vishnu Motghare >> >> This patch adds "defeature-repeated-start" property in i2c-cadence.txt. >> >> Signed-off-by: Vishnu Motghare >> Signed-off-by: Harini Katakam >> --- >> .../devicetree/bindings/i2c/i2c-cadence.txt| 11 +++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/i2c/i2c-cadence.txt >> b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt >> index 7cb0b56..9d417a7 100644 >> --- a/Documentation/devicetree/bindings/i2c/i2c-cadence.txt >> +++ b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt >> @@ -11,6 +11,17 @@ Required properties: >> Optional properties: >>- clock-frequency: Desired operating frequency, in Hz, of the bus. >>- clock-names: Input clock name, should be 'pclk'. >> + - defeature-repeated-start: Include this property to defeature repeated >> start >> + This defeature is due to a few bugs in the >> + I2C controller. >> + Completion interrupt after a read/receive >> + operation is NOT obtained if HOLD bit is set >> + at that time. Because of this bug, repeated start >> + will only work if there are no transfers >> following >> + a read/receive transfer. >> + If HOLD is held for long without a transfer, >> + invalid read transactions are generated by the >> + controller due to a HW timeout related bug. > > I'm not keen on the name; it sounds like we're disabling a feature > rather than describing the problem (and "defeature" is not a common > term in this sense, "disable" would be better). > > It sounds like there are two issues with staying in the HOLD state? Lost > completion IRQs and a separate HW timeout bug? Or are the two related? > Yes, there are two issues here and they are not related. But a combination of both is leading to not using repeated start. The intention was to defeature except that it works in some scenarios (such as a typical write+read in that order with repeated start) and there are people who already use the driver with slaves that need this. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] i2c: cadence: Defeature repeated start based on devicetree property
On Tue, Dec 02, 2014 at 10:05:49AM +, Harini Katakam wrote: > From: Vishnu Motghare > > This patch defeatures repeated start in the driver if > "defeature-repeated-start" property is present in the devicetree. > > This defeature is proposed due to a few bugs in the controller > - completion indication is not given to the driver at the end of > a read/receive transfer with HOLD bit set. > - Invalid read transaction are generated on the bus when HW timeout > condition occurs with HOLD bit set. > > Signed-off-by: Vishnu Motghare > Signed-off-by: Harini Katakam > --- > drivers/i2c/busses/i2c-cadence.c | 44 > +++--- > 1 file changed, 31 insertions(+), 13 deletions(-) [...] > @@ -840,6 +856,8 @@ static int cdns_i2c_probe(struct platform_device *pdev) > cdns_i2c_writereg(CDNS_I2C_CR_ACK_EN | CDNS_I2C_CR_NEA | CDNS_I2C_CR_MS, > CDNS_I2C_CR_OFFSET); > > + of_property_read_u32(pdev->dev.of_node, "defeature-repeated-start", > + &id->defeature_repeated_start); This will not work with the binding you described. You want to treat this as a bool, and use of_property_read_bool. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 0/2] i2c: omap: new fixes for driver
01 дек. 2014 г., в 23:04, Kevin Hilman написал(а): > Done. Built v3.18-rc7 + these 2 patches using omap2plus_defconfig. Boot > logs here for your amusement: Hello, Kevin! Thank you so much for doing tests. What a pity you don't have omap2430sdp board in the tests. Alexander. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] devicetree: bindings: Add defeature-repeated-start property for Cadence I2C
On Tue, Dec 02, 2014 at 10:05:48AM +, Harini Katakam wrote: > From: Vishnu Motghare > > This patch adds "defeature-repeated-start" property in i2c-cadence.txt. > > Signed-off-by: Vishnu Motghare > Signed-off-by: Harini Katakam > --- > .../devicetree/bindings/i2c/i2c-cadence.txt| 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-cadence.txt > b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt > index 7cb0b56..9d417a7 100644 > --- a/Documentation/devicetree/bindings/i2c/i2c-cadence.txt > +++ b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt > @@ -11,6 +11,17 @@ Required properties: > Optional properties: >- clock-frequency: Desired operating frequency, in Hz, of the bus. >- clock-names: Input clock name, should be 'pclk'. > + - defeature-repeated-start: Include this property to defeature repeated > start > + This defeature is due to a few bugs in the > + I2C controller. > + Completion interrupt after a read/receive > + operation is NOT obtained if HOLD bit is set > + at that time. Because of this bug, repeated start > + will only work if there are no transfers following > + a read/receive transfer. > + If HOLD is held for long without a transfer, > + invalid read transactions are generated by the > + controller due to a HW timeout related bug. I'm not keen on the name; it sounds like we're disabling a feature rather than describing the problem (and "defeature" is not a common term in this sense, "disable" would be better). It sounds like there are two issues with staying in the HOLD state? Lost completion IRQs and a separate HW timeout bug? Or are the two related? Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing
01 дек. 2014 г., в 22:58, Tony Lindgren написал(а): > I think this is a different issue than what I'm seeing. Hello, Tony! Thank you for testing! Could check i2c-omap.c from commit ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4. As all my changes comes after it[1]. So I can understand was the problem before my work. I there was no problems, then try with my first commit: 27caca9d2e01c92b26d0690f065aad093fea01c7 The problems you talk about is this? [9.675994] omap_i2c 48072000.i2c: controller timed out [ 10.704010] omap_i2c 48072000.i2c: controller timed out [ 11.734069] omap_i2c 48072000.i2c: controller timed out root@omap2430sdp:/# [ 12.823638] omap_i2c 48072000.i2c: controller timed out And how it is possible to switch from ti,omap2430-i2c to ti,omap2420-i2c? They are so different IP, from the driver point of view. They have different data bus width. Alexander. [1] alexander@ubuntu:busses$ git log --pretty=oneline --reverse ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4^..HEAD -- i2c-omap.c ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4 i2c: remove FSF address 27caca9d2e01c92b26d0690f065aad093fea01c7 i2c: omap: fix NACK and Arbitration Lost irq handling 854a59425a0b9600ee974b113aae081c873163f6 i2c: omap: cleanup register definitions 903c3859f77f9b0aace551da03267ef7a211dbc4 i2c: omap: implement workaround for handling invalid BB-bit values 80cc361f14e8fa97119afa3324c2c913915e7252 i2c: omap: don't reset controller if Arbitration Lost detected 39370ab406933efdedb425910f0a36c16667c45f i2c: omap: add notes related to i2c multimaster mode ccfc866356674cb3a61829d239c685af6e85f197 i2c: omap: fix i207 errata handling 7d168dc7ed384e50bb7bff4920b73550fd2e9fcb Merge branch 'i2c/for-3.19' into i2c/for-next 2f769d173f0e6a2e85d75fe396f18f794fc4a615 omap: i2c: don't check bus state IP rev3.3 and earlier 2b6f66d87b44aaf1f34f071e6f6430c3ccaa8812 i2c: omap: fix buffer overruns during RX/TX data processing 30c52545106785405856c7e7e40b683b79c8084a i2c: omap: show that the reason of system lockup is an unhandled ISR event -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] i2c: cadence: Defeature repeated start based on devicetree property
From: Vishnu Motghare
This patch defeatures repeated start in the driver if
"defeature-repeated-start" property is present in the devicetree.
This defeature is proposed due to a few bugs in the controller
- completion indication is not given to the driver at the end of
a read/receive transfer with HOLD bit set.
- Invalid read transaction are generated on the bus when HW timeout
condition occurs with HOLD bit set.
Signed-off-by: Vishnu Motghare
Signed-off-by: Harini Katakam
---
drivers/i2c/busses/i2c-cadence.c | 44 +++---
1 file changed, 31 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 8065205..89335f7 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -135,6 +135,7 @@
* @bus_hold_flag: Flag used in repeated start for clearing HOLD bit
* @clk: Pointer to struct clk
* @clk_rate_change_nb:Notifier block for clock rate changes
+ * @defeature_repeated_start: Flag to de feature repeated start
*/
struct cdns_i2c {
void __iomem *membase;
@@ -154,6 +155,7 @@ struct cdns_i2c {
unsigned int bus_hold_flag;
struct clk *clk;
struct notifier_block clk_rate_change_nb;
+ unsigned int defeature_repeated_start;
};
#define to_cdns_i2c(_nb) container_of(_nb, struct cdns_i2c, \
@@ -246,7 +248,7 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
}
if ((isr_status & CDNS_I2C_IXR_COMP) && !id->recv_count) {
- if (!id->bus_hold_flag)
+ if (id->defeature_repeated_start || !id->bus_hold_flag)
cdns_i2c_clear_bus_hold(id);
done_flag = 1;
}
@@ -283,7 +285,8 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
*/
done_flag = 1;
}
- if (!id->send_count && !id->bus_hold_flag)
+ if ((id->defeature_repeated_start && !id->send_count) ||
+ (!id->send_count && !id->bus_hold_flag))
cdns_i2c_clear_bus_hold(id);
status = IRQ_HANDLED;
@@ -347,10 +350,15 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
} else
cdns_i2c_writereg(id->recv_count, CDNS_I2C_XFER_SIZE_OFFSET);
/* Clear the bus hold flag if bytes to receive is less than FIFO size */
- if (!id->bus_hold_flag &&
- ((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) &&
- (id->recv_count <= CDNS_I2C_FIFO_DEPTH))
- cdns_i2c_clear_bus_hold(id);
+ if (id->defeature_repeated_start &&
+ (((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) &&
+(id->recv_count <= CDNS_I2C_FIFO_DEPTH)))
+ cdns_i2c_clear_bus_hold(id);
+ else if (!id->bus_hold_flag &&
+((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) &&
+(id->recv_count <= CDNS_I2C_FIFO_DEPTH))
+ cdns_i2c_clear_bus_hold(id);
+
/* Set the slave address in address register - triggers operation */
cdns_i2c_writereg(id->p_msg->addr & CDNS_I2C_ADDR_MASK,
CDNS_I2C_ADDR_OFFSET);
@@ -411,8 +419,10 @@ static void cdns_i2c_msend(struct cdns_i2c *id)
* Clear the bus hold flag if there is no more data
* and if it is the last message.
*/
- if (!id->bus_hold_flag && !id->send_count)
+if ((id->defeature_repeated_start && !id->send_count) ||
+(!id->send_count && !id->bus_hold_flag))
cdns_i2c_clear_bus_hold(id);
+
/* Set the slave address in address register - triggers operation. */
cdns_i2c_writereg(id->p_msg->addr & CDNS_I2C_ADDR_MASK,
CDNS_I2C_ADDR_OFFSET);
@@ -520,19 +530,25 @@ static int cdns_i2c_master_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs,
/*
* Set the flag to one when multiple messages are to be
* processed with a repeated start.
+* De feature repeated start when defeature_repeated_start flag is set
*/
- if (num > 1) {
- id->bus_hold_flag = 1;
- reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
- reg |= CDNS_I2C_CR_HOLD;
- cdns_i2c_writereg(reg, CDNS_I2C_CR_OFFSET);
+ if (!id->defeature_repeated_start && num > 1) {
+ for (count = 0; count < num-1; count++) {
+ if (msgs[count].flags & I2C_M_RD)
+ return -EINVAL;
+ }
+ id->bus_hold_flag = 1;
+ reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
+ reg |= CDNS_I2C_CR_HOLD;
+ cdns_i2c_writ
[PATCH 1/4] i2c: cadence: Handle > 252 byte transfers
The I2C controller sends a NACK to the slave when transfer size register
reaches zero, irrespective of the hold bit. So, in order to handle transfers
greater than 252 bytes, the transfer size register has to be maintained at a
value >= 1. This patch implements the same.
The interrupt status is cleared at the beginning of the isr instead of
the end, to avoid missing any interrupts - this is in sync with the new
transfer handling.
Signed-off-by: Harini Katakam
---
drivers/i2c/busses/i2c-cadence.c | 156 --
1 file changed, 81 insertions(+), 75 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 63f3f03..e54899e 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -126,6 +126,7 @@
* @suspended: Flag holding the device's PM status
* @send_count:Number of bytes still expected to send
* @recv_count:Number of bytes still expected to receive
+ * @curr_recv_count: Number of bytes to be received in current transfer
* @irq: IRQ number
* @input_clk: Input clock to I2C controller
* @i2c_clk: Maximum I2C clock speed
@@ -144,6 +145,7 @@ struct cdns_i2c {
u8 suspended;
unsigned int send_count;
unsigned int recv_count;
+ unsigned int curr_recv_count;
int irq;
unsigned long input_clk;
unsigned int i2c_clk;
@@ -180,14 +182,15 @@ static void cdns_i2c_clear_bus_hold(struct cdns_i2c *id)
*/
static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
{
- unsigned int isr_status, avail_bytes;
- unsigned int bytes_to_recv, bytes_to_send;
+ unsigned int isr_status, avail_bytes, updatetx;
+ unsigned int bytes_to_send;
struct cdns_i2c *id = ptr;
/* Signal completion only after everything is updated */
int done_flag = 0;
irqreturn_t status = IRQ_NONE;
isr_status = cdns_i2c_readreg(CDNS_I2C_ISR_OFFSET);
+ cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
/* Handling nack and arbitration lost interrupt */
if (isr_status & (CDNS_I2C_IXR_NACK | CDNS_I2C_IXR_ARB_LOST)) {
@@ -195,89 +198,91 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
status = IRQ_HANDLED;
}
- /* Handling Data interrupt */
- if ((isr_status & CDNS_I2C_IXR_DATA) &&
- (id->recv_count >= CDNS_I2C_DATA_INTR_DEPTH)) {
- /* Always read data interrupt threshold bytes */
- bytes_to_recv = CDNS_I2C_DATA_INTR_DEPTH;
- id->recv_count -= CDNS_I2C_DATA_INTR_DEPTH;
- avail_bytes = cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
-
- /*
-* if the tranfer size register value is zero, then
-* check for the remaining bytes and update the
-* transfer size register.
-*/
- if (!avail_bytes) {
- if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
- cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
- CDNS_I2C_XFER_SIZE_OFFSET);
- else
- cdns_i2c_writereg(id->recv_count,
- CDNS_I2C_XFER_SIZE_OFFSET);
- }
+ updatetx = 0;
+ if (id->recv_count > id->curr_recv_count)
+ updatetx = 1;
+
+ /* When receiving, handle data and transfer complete interrupts */
+ if (id->p_recv_buf &&
+ ((isr_status & CDNS_I2C_IXR_COMP) ||
+(isr_status & CDNS_I2C_IXR_DATA))) {
+ while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
+ CDNS_I2C_SR_RXDV) {
+ if ((id->recv_count < CDNS_I2C_FIFO_DEPTH) &&
+ !id->bus_hold_flag)
+ cdns_i2c_clear_bus_hold(id);
- /* Process the data received */
- while (bytes_to_recv--)
*(id->p_recv_buf)++ =
cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
+ id->recv_count--;
+ id->curr_recv_count--;
- if (!id->bus_hold_flag &&
- (id->recv_count <= CDNS_I2C_FIFO_DEPTH))
- cdns_i2c_clear_bus_hold(id);
+ if (updatetx &&
+ (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1))
+ break;
+ }
- status = IRQ_HANDLED;
- }
+ if (updatetx &&
+ (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1)) {
+ /* wait while fifo is full */
+ while (cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET) !=
+ (id->curr_recv_count - CDNS
[PATCH 3/4] devicetree: bindings: Add defeature-repeated-start property for Cadence I2C
From: Vishnu Motghare
This patch adds "defeature-repeated-start" property in i2c-cadence.txt.
Signed-off-by: Vishnu Motghare
Signed-off-by: Harini Katakam
---
.../devicetree/bindings/i2c/i2c-cadence.txt| 11 +++
1 file changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
index 7cb0b56..9d417a7 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
@@ -11,6 +11,17 @@ Required properties:
Optional properties:
- clock-frequency: Desired operating frequency, in Hz, of the bus.
- clock-names: Input clock name, should be 'pclk'.
+ - defeature-repeated-start: Include this property to defeature repeated start
+ This defeature is due to a few bugs in the
+ I2C controller.
+ Completion interrupt after a read/receive
+ operation is NOT obtained if HOLD bit is set
+ at that time. Because of this bug, repeated start
+ will only work if there are no transfers following
+ a read/receive transfer.
+ If HOLD is held for long without a transfer,
+ invalid read transactions are generated by the
+ controller due to a HW timeout related bug.
Example:
i2c@e0004000 {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] i2c: cadence: Set the hardware time-out register to maximum value
From: Vishnu Motghare Cadence I2C controller has bug wherein it generates invalid read transactions after time-out in master receiver mode. This driver does not use the HW timeout and this interrupt is disabled but the feature itself cannot be disabled. Hence, this patch writes the maximum value (0xFF) to this register. This is one of the workarounds to this bug and it will not avoid the issue completely but reduce the chances of error. Signed-off-by: Vishnu Motghare Signed-off-by: Harini Katakam --- drivers/i2c/busses/i2c-cadence.c |4 1 file changed, 4 insertions(+) diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c index e54899e..8065205 100644 --- a/drivers/i2c/busses/i2c-cadence.c +++ b/drivers/i2c/busses/i2c-cadence.c @@ -111,6 +111,8 @@ #define CDNS_I2C_DIVA_MAX 4 #define CDNS_I2C_DIVB_MAX 64 +#define CDNS_I2C_TIMEOUT_MAX 0xFF + #define cdns_i2c_readreg(offset) readl_relaxed(id->membase + offset) #define cdns_i2c_writereg(val, offset) writel_relaxed(val, id->membase + offset) @@ -858,6 +860,8 @@ static int cdns_i2c_probe(struct platform_device *pdev) goto err_clk_dis; } + cdns_i2c_writereg(CDNS_I2C_TIMEOUT_MAX, CDNS_I2C_TIME_OUT_OFFSET); + dev_info(&pdev->dev, "%u kHz mmio %08lx irq %d\n", id->i2c_clk / 1000, (unsigned long)r_mem->start, id->irq); -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] Cadence I2C driver fixes
This series implements workarounds for some bugs in Cadence I2C controller. - The I2C controller when configured in Master Mode generates invalid read transactions. When HOLD bit is set and HW timeout occurs, invalid read transactions are generated on the bus. This will affect repeated start conditions and large data transfer condtions where transfer_size_register has to be updated again. The transfer size register rolls over erroneously under these conditions. Note that this HW timeout cannot be disabled even if the interrupt is unused. Errata link: http://www.xilinx.com/support/answers/61664.html -The I2C controller when configured in Master Mode is the missing master completion interrupt. During repeated start condition, the driver has to set the HOLD bit for the set of transfer being done together and clear the HOLD bit just before the last transfer. But the controller does not indicate completion when a read/receive transfer is done if the HOLD bit is set. This affects all repeated start operation where a read/receive transfer is not the last transfer. Errata link: http://www.xilinx.com/support/answers/61665.html To address the above, - >252 byte transfer are done such that transfer_size never becomes zero. - timeout register value is increased (even though the driver doesn't use this). - repeated start is defeatured based on a devicetree input. This input is because repeated start is required as part of protocol for some slave devices (not just to retain control in a multi-master scenario). The driver works as expected with these devices and hence the defeature is optional in such cases. - In case user chooses not to de-feature repeated start, then a check is performed to see if there any transfer following a read/receive transfer in the set of messages using repeated start. An error is returned in such cases because if we proceed, completion interrupt is never obtained and a SW timeout will occur. Harini Katakam (1): i2c: cadence: Handle > 252 byte transfers Vishnu Motghare (3): i2c: cadence: Set the hardware time-out register to maximum value devicetree: bindings: Add defeature-repeated-start property for Cadence I2C i2c: cadence: Defeature repeated start based on devicetree property .../devicetree/bindings/i2c/i2c-cadence.txt| 11 ++ drivers/i2c/busses/i2c-cadence.c | 200 +++- 2 files changed, 125 insertions(+), 86 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/3] gpio: add support for Cypress CYUSBS234 USB-GPIO adapter
On Mon, 01 Dec 2014, Linus Walleij wrote: > On Fri, Nov 28, 2014 at 5:41 PM, Muthu Mani wrote: > > > Adds support for USB-GPIO interface of Cypress Semiconductor > > CYUSBS234 USB-Serial Bridge controller. > > > > The GPIO get/set can be done through vendor command on control endpoint > > for the configured gpios. > > > > Details about the device can be found at: > > http://www.cypress.com/?rID=84126 > > > > Signed-off-by: Muthu Mani > > Signed-off-by: Rajaram Regupathy > > --- > > Changes since v3: > > (..) > > +config GPIO_CYUSBS23X > > + tristate "CYUSBS23x GPIO support" > > + depends on MFD_CYUSBS23X && USB > > Doesn't MFD_CYUSV23X already depend on USB? Yup. > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > + > > +#include > > Why this arbitrary newlines? Add this > > #include Narcissist. =;-) [...] -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
