Re: [PATCH 0/7] Second batch of cleanups for cros_ec
Hi Javier, Am 28.07.2014 14:19, schrieb Javier Martinez Canillas: The patches in this series are authored by different people (all on cc) and consist of the following: Andrew Bresticker (3): mfd: cros_ec: stop calling -cmd_xfer() directly mfd: cros_ec: move locking into cros_ec_cmd_xfer mfd: cros_ec: wait for completion of commands that return IN_PROGRESS Derek Basehore (1): i2c: i2c-cros-ec-tunnel: Set retries to 3 Doug Anderson (1): mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC Todd Broch (2): mfd: cros_ec: Instantiate sub-devices from device tree Input: cros_ec_keyb: Optimize ghosting algorithm. When you submit other people's patches, I believe you still need to sign them off as submitter. In particular when you rebased them. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- 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 6/7] mfd: cros_ec: Instantiate sub-devices from device tree
Hi Javier, Am 28.07.2014 14:19, schrieb Javier Martinez Canillas: From: Todd Broch tbr...@chromium.org If the EC device tree node has sub-nodes, try to instantiate them as MFD sub-devices. We can configure the EC features provided by the board. Signed-off-by: Todd Broch tbr...@chromium.org This is provoking the blunt question: What sub-devices is it good for? I.e., do you have a matching DT patchset that adds such devices? In particular I'm wondering whether that would help with the tps65090 on Spring? It's a power-regulator sub-node of cros-ec in 3.8. Thanks, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- 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 0/7] Second batch of cleanups for cros_ec
Hi Andreas, On 07/29/2014 02:27 PM, Andreas Färber wrote: Hi Javier, Am 28.07.2014 14:19, schrieb Javier Martinez Canillas: The patches in this series are authored by different people (all on cc) and consist of the following: Andrew Bresticker (3): mfd: cros_ec: stop calling -cmd_xfer() directly mfd: cros_ec: move locking into cros_ec_cmd_xfer mfd: cros_ec: wait for completion of commands that return IN_PROGRESS Derek Basehore (1): i2c: i2c-cros-ec-tunnel: Set retries to 3 Doug Anderson (1): mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC Todd Broch (2): mfd: cros_ec: Instantiate sub-devices from device tree Input: cros_ec_keyb: Optimize ghosting algorithm. When you submit other people's patches, I believe you still need to sign them off as submitter. In particular when you rebased them. You are right. I thought that given that I only rebased the patches and removed changes for files that are not yet in mainline I shouldn't add my s-o-b. But looking at the Developer's Certificate of Origin 1.1 rule (b) it seems that I should had added my s-o-b as well. Regards, Andreas Thanks a lot and best regards, Javier -- 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 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm.
Am 28.07.2014 14:19, schrieb Javier Martinez Canillas: From: Todd Broch tbr...@chromium.org Previous algorithm was a bit conservative and complicating with respect to identifying key ghosting. This CL uses the bitops hamming weight function (hweight8) to count the number of matching rows for colM colN. If that number is 1 ghosting is present. Additionally it removes NULL keys and our one virtual keypress KEY_BATTERY from consideration as these inputs are never physical keypresses. Signed-off-by: Todd Broch tbr...@chromium.org Reviewed-by: Vincent Palatin vpala...@chromium.org Reviewed-by: Luigi Semenzato semenz...@chromium.org This seems to fix my Ctrl+O problems on Spring reported a while ago, Tested-by: Andreas Färber afaer...@suse.de Thanks, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- 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 6/7] mfd: cros_ec: Instantiate sub-devices from device tree
Hello Andreas, On 07/29/2014 02:34 PM, Andreas Färber wrote: Hi Javier, Am 28.07.2014 14:19, schrieb Javier Martinez Canillas: From: Todd Broch tbr...@chromium.org If the EC device tree node has sub-nodes, try to instantiate them as MFD sub-devices. We can configure the EC features provided by the board. Signed-off-by: Todd Broch tbr...@chromium.org This is provoking the blunt question: What sub-devices is it good for? I.e., do you have a matching DT patchset that adds such devices? For Peach Pit and Pi, the matching DTS changes is [0]. But answering your question it is to instantiate the subdevices that can be child nodes of either cros-ec-spi or cros-ec-i2c. So for the devices that are directly connected to the EC Cortex-M through i2c. In particular I'm wondering whether that would help with the tps65090 on Spring? It's a power-regulator sub-node of cros-ec in 3.8. Spring is a little more complicated since the EC in Spring don't have the full EC_CMD_I2C_PASSTHRU. So the downstream Chrome OS 3.8 kernel has a forked tps65090 driver (drivers/regulator/cros_ec-tps65090.c) that talks directly with the cros_ec MFD driver, you can get more info from [1] in the About Spring section. Thanks, Andreas Best regards, Javier [0]: https://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/commit/?h=for-nextid=8060098bbb564d27a287057a93d4fe3bfd266290 [1]: http://code.google.com/p/chromium/issues/detail?id=391797 -- 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: i2c-davinci.c: CPU FREQ causes lock up due to xfr_complete
Grygorii,
Unfortunately I don't have access to a booting mainline kernel to try
and work on this. Its also currently a bit over my head.
cheers,
-Jonathan
On Tue, Jul 29, 2014 at 1:30 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Hi Jon,
On 07/29/2014 06:53 PM, Jon Cormier wrote:
A slimmer patch suggested by Grygorii Strashko grygorii.stras...@ti.com
Ok. The problem seems to be deeper than at first look.
You have sequence (in Mainline kernel):
cpufreq:
|- notify CPUFREQ_PRECHANGE
|- i2c-davinci will lock put I2C in reset
|- cpufreq_driver-target_index
|- davinci_target()
|- pdata-set_voltage() - It will try to use I2C to set new voltage,
while I2C is in reset or locked! Bug!
|- notify CPUFREQ_POSTCHANGE
|- i2c-davinci will re-enable I2C and adjust I2C clock
I see few possible ways to solve it:
1) use CLK notifier instead of CPUfreq notifiers
2) do smth similar to 61c7cff8 i2c: S3C24XX I2C frequency scaling support
+ 9bcd04bf i2c: s3c2410: grab adapter lock while changing i2c clock
3) update I2C clock in CPUFREQ_POSTCHANGE - may be unsafe
Regards,
-grygorii
Author: Cormier, Jonathan jcorm...@criticallink.com
Date: Tue Jul 29 11:50:04 2014 -0400
i2c: davinci: Change xfr_complete completion to use i2c_lock_adapter
There are several problems with the use of a completion for this task:
1. If no I2C transfer has occurred, a cpufreq transition will block
forever.
2. Once an I2C transfer has occurred the cpufreq transition will
never block since the completion is never reinitialized.
3. Even if you do reinitialize the completion for every I2C
transfer, (1) is not solved and there is still a race condition where
the cpufreq transition could start just before an I2C transfer starts
and the I2C transfer occurs during the cpufreq transition.
Author: Grygorii Strashko grygorii.stras...@ti.com
Author: Cormier, Jonathan jcorm...@criticallink.com
Signed-off-by: Cormier, Jonathan jcorm...@criticallink.com
diff --git a/drivers/i2c/busses/i2c-davinci.c
b/drivers/i2c/busses/i2c-davinci.c
index a76d85f..f8e7b7f 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -111,7 +111,6 @@ struct davinci_i2c_dev {
u8terminate;
struct i2c_adapteradapter;
#ifdef CONFIG_CPU_FREQ
-struct completionxfr_complete;
struct notifier_blockfreq_transition;
#endif
};
@@ -452,10 +451,6 @@ i2c_davinci_xfer(struct i2c_adapter *adap, struct
i2c_msg msgs[], int num)
return ret;
}
-#ifdef CONFIG_CPU_FREQ
-complete(dev-xfr_complete);
-#endif
-
return num;
}
@@ -596,11 +591,12 @@ static int i2c_davinci_cpufreq_transition(struct
notifier_block *nb,
dev = container_of(nb, struct davinci_i2c_dev, freq_transition);
if (val == CPUFREQ_PRECHANGE) {
-wait_for_completion(dev-xfr_complete);
+i2c_lock_adapter(dev-adapter);
davinci_i2c_reset_ctrl(dev, 0);
} else if (val == CPUFREQ_POSTCHANGE) {
i2c_davinci_calc_clk_dividers(dev);
davinci_i2c_reset_ctrl(dev, 1);
+i2c_unlock_adapter(dev-adapter);
}
return 0;
@@ -669,9 +665,7 @@ static int davinci_i2c_probe(struct platform_device
*pdev)
}
init_completion(dev-cmd_complete);
-#ifdef CONFIG_CPU_FREQ
-init_completion(dev-xfr_complete);
-#endif
+
dev-dev = get_device(pdev-dev);
dev-irq = irq-start;
platform_set_drvdata(pdev, dev);
On Tue, Jul 29, 2014 at 11:36 AM, Jon Cormier jcorm...@criticallink.com
wrote:
Okay here is my attempt.
Author: Cormier, Jonathan jcorm...@criticallink.com
Date: Tue Jul 29 11:26:22 2014 -0400
i2c: davinci: Change xfr_complete completion to a mutex
There are several problems with the use of a completion for this task:
1. If no I2C transfer has occurred, a cpufreq transition will block
forever.
2. Once an I2C transfer has occurred the cpufreq transition will
never block since the completion is never reinitialized.
3. Even if you do reinitialize the completion for every I2C
transfer, (1) is not solved and there is still a race condition where
the cpufreq transition could start just before an I2C transfer starts
and the I2C transfer occurs during the cpufreq transition.
Signed-off-by: Cormier, Jonathan jcorm...@criticallink.com
diff --git a/drivers/i2c/busses/i2c-davinci.c
b/drivers/i2c/busses/i2c-davinci.c
index a76d85f..9eac1c1 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -111,7 +111,7 @@ struct davinci_i2c_dev {
u8terminate;
struct i2c_adapteradapter;
#ifdef CONFIG_CPU_FREQ
-struct completionxfr_complete;
+struct mutexxfr_lock;
struct notifier_blockfreq_transition;
#endif
};
@@ -438,10 +438,14 @@
