Re: [PATCH] i2c: tegra: Fix ifdefs for suspend mode
On Mon, Jul 30, 2012 at 07:03:07PM +0530, Laxman Dewangan wrote:
CONFIG_PM covers runtime only PM.
I am a bit confused by this description. CONFIG_PM_RUNTIME covers
runtime PM, no?
Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
---
This patch will remove the compilation warning if CONFIG_PM_SLEEP
is not enabled.
Maybe it would help to see the warning?
drivers/i2c/busses/i2c-tegra.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 66eb53f..9a08c57 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -712,7 +712,7 @@ static int __devexit tegra_i2c_remove(struct
platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
static int tegra_i2c_suspend(struct device *dev)
{
struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
--
1.7.1.1
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 1/2] i2c: imx: Use dev_info to indicate that i2c driver was succesfully registered
On Wed, Aug 01, 2012 at 05:38:14PM -0300, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com It is useful information in dmesg to have a message indicating that the i2c driver was succesfully registered, so promote it to dev_info. Signed-off-by: Fabio Estevam fabio.este...@freescale.com Both applied to -next, thanks! -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 1/1 v4] i2c: Add default configuration into the Nomadik I2C driver.
On Wed, Aug 08, 2012 at 02:11:59PM +0100, Lee Jones wrote: From: Lee Jones lee.jo...@linaro.org Date: Tue, 7 Aug 2012 12:27:24 +0100 Subject: [PATCH 1/1 v4] i2c/nomadik: Add default configuration into the Nomadik I2C driver Actually, unlike Linus said, I prefer i2c: nomadik:; minor nit, though. At this moment in time there is only one known configuration for the Nomadik I2C driver. By not holding that configuration in the driver adds some unnecessary overhead in platform code. The configuration as already been removed from platform code, this patch checks for any ^^ has ( I fixed it ) over-riding configurations. If there aren't any, the default is used. Cc: linux-i2c@vger.kernel.org Acked-by: srinidhi kasagar srinidhi.kasa...@stericsson.com Acked-by: Linus Walleij linus.wall...@linaro.org Signed-off-by: Lee Jones lee.jo...@linaro.org Applied to -current, thanks! I added a note from Linus that it is needed because of boot regressions. That is also useful in commit messages. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] I2C: OMAP: xfer: fix runtime PM get/put balance on error
On Mon, Aug 06, 2012 at 05:28:44PM -0700, Kevin Hilman wrote: Hi Wolfram, Kevin Hilman khil...@ti.com writes: In omap_i2c_xfer(), ensure pm_runtime_put() is called, even on failure. Without this, after a failed xfer, the runtime PM usecount will have been incremented, but not decremented causing the usecount to never reach zero after a failure. This keeps the device always runtime PM enabled which keeps the enclosing power domain active, and prevents full-chip retention/off from happening during idle. Cc: Shubhrajyoti D shubhrajy...@ti.com Signed-off-by: Kevin Hilman khil...@ti.com --- This patch applies to current i2c-embedded/for-next branch This one is needed for v3.6. Can you queue this up as a fix for v3.6-rc? Done, applied to -current, thanks! -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v7] ARM: davinci: i2c: add OF support
On Tue, Aug 07, 2012 at 04:35:16PM +0200, Heiko Schocher wrote: Hello, On 30.07.2012 09:21, Heiko Schocher wrote: add of support for the davinci i2c driver. Signed-off-by: Heiko Schocherh...@denx.de Signed-off-by: Sekhar Norinsek...@ti.com Cc: davinci-linux-open-sou...@linux.davincidsp.com Cc: linux-arm-ker...@lists.infradead.org Cc: devicetree-disc...@lists.ozlabs.org Cc: linux-i2c@vger.kernel.org Cc: Ben Dooksben-li...@fluff.org Cc: Wolfram Sangw.s...@pengutronix.de Cc: Grant Likelygrant.lik...@secretlab.ca Cc: Sekhar Norinsek...@ti.com Cc: Wolfgang Denkw...@denx.de Cc: Sylwester Nawrockis.nawro...@samsung.com --- ping ... any comments? Applied to -next with indentation fixes to the binding description. Thanks! -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] i2c-i801: Add device tree support
On Wed, Aug 08, 2012 at 09:06:47AM +0200, Thierry Reding wrote: This commit adds support for probing slave devices parsed from the device tree. Signed-off-by: Thierry Reding thierry.red...@avionic-design.de How are the devicetree nodes of the slaves connected to this PCI device which should not have a node itself? --- drivers/i2c/busses/i2c-i801.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 6f00785..b5311cf 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -80,6 +80,7 @@ #include linux/slab.h #include linux/wait.h #include linux/err.h +#include linux/of_i2c.h #if defined CONFIG_I2C_MUX || defined CONFIG_I2C_MUX_MODULE #include linux/gpio.h @@ -1238,6 +1239,7 @@ static int __devinit i801_probe(struct pci_dev *dev, goto exit_free_irq; } + of_i2c_register_devices(priv-adapter); i801_probe_optional_slaves(priv); /* We ignore errors - multiplexing is optional */ i801_add_mux(priv); -- 1.7.11.4 -- 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 -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH RESEND 1/2] i2c: pnx: Fix bit definitions
On Wed, Aug 08, 2012 at 09:42:31AM +0200, Roland Stigge wrote: The I2C Control Register bits RFDAIE and RFFIE were mixed up. In addition to this fix, this patch adds the missing bit DRSIE for completeness. Signed-off-by: Roland Stigge sti...@antcom.de --- Applies to v3.6-rc1 This patch for i2c-pnx affects PNX4008 and LPC32xx (and LPC31xx, not yet in mainline). Can you please test and double-check the manuals of PNX4008 and LPC31xx? I only found this via the manual of LPC32xx but assume it's the same for the others, also. Thanks in advance! Kevin, since the other manuals seem to be not easily available, can you please check? Thanks, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH RESEND 2/2] i2c: pnx: Fix read transactions of = 2 bytes
On Wed, Aug 08, 2012 at 09:42:32AM +0200, Roland Stigge wrote: On transactions with n=2 bytes, the controller actually wrongly clocks in n+1 bytes. This is caused by the (wrong) assumption that RFE in the Status Register is 1 iff there is no byte already ordered (via a dummy TX byte). This lead to the implementation of synchronized byte ordering, e.g.: Dummy-TX - RX - Dummy-TX - RX - ... But since RFE actually stays high after some Dummy-TX, it rather looks like: Dummy-TX - Dummy-TX - RX - Dummy-TX - RX - (RX) The last RX byte is clocked in by the bus controller, but ignored by the kernel when filling the userspace buffer. This patch fixes the issue by asking for RX via Dummy-TX asynchronously. Introducing a separate counter for TX bytes. Signed-off-by: Roland Stigge sti...@antcom.de --- Applies to v3.6-rc1 This patch for i2c-pnx affects PNX4008 and LPC32xx (and LPC31xx, not yet in mainline). Can you please test? Thanks in advance! I assume you checked this on LPC32xx? I'll wait a bit more but tend to take the patch anyhow, even if there is no explicit ACK from PNX. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 1/6] i2c: omap: Prevent NULL pointer dereference in remove
On Thu, Aug 09, 2012 at 07:07:42PM +0530, Shubhrajyoti D wrote: Prevent the NULL pointer access by moving the platform_set_drvdata function after the access of the pdev. [ 654.961761] Unable to handle kernel NULL pointer dereference at virtual address 0070 [ 654.970611] pgd = df254000 [ 654.973480] [0070] *pgd=9f1da831, *pte=, *ppte= [ 654.980163] Internal error: Oops: 17 [#1] SMP ARM [ 654.985076] Modules linked in: [ 654.988281] CPU: 1Not tainted (3.6.0-rc1-00031-ge547de1-dirty #339) [ 654.995330] PC is at omap_i2c_runtime_resume+0x8/0x148 [ 655.000732] LR is at omap_i2c_runtime_resume+0x8/0x148 Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com --- drivers/i2c/busses/i2c-omap.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index c8e3886..0c593d4 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1217,8 +1217,6 @@ static int __devexit omap_i2c_remove(struct platform_device *pdev) struct omap_i2c_dev *dev = platform_get_drvdata(pdev); int ret; - platform_set_drvdata(pdev, NULL); - i2c_del_adapter(dev-adapter); ret = pm_runtime_get_sync(pdev-dev); if (IS_ERR_VALUE(ret)) @@ -1227,6 +1225,8 @@ static int __devexit omap_i2c_remove(struct platform_device *pdev) omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); pm_runtime_put(pdev-dev); pm_runtime_disable(pdev-dev); + platform_set_drvdata(pdev, NULL); + return 0; } I think this patch is correct, because drvdata is used in the PM code of the driver and thus cleared too early. As such, this is a bugfix and should be not based on the big cleanup since it should go into this rc series. I will pick it as soon as the comments on the other patches are answered. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] i2c-i801: Add device tree support
On Sat, Aug 18, 2012 at 11:48:46AM +0200, Wolfram Sang wrote: On Wed, Aug 08, 2012 at 09:06:47AM +0200, Thierry Reding wrote: This commit adds support for probing slave devices parsed from the device tree. Signed-off-by: Thierry Reding thierry.red...@avionic-design.de How are the devicetree nodes of the slaves connected to this PCI device which should not have a node itself? The matching is done by pci_set_of_node() and pci_set_bus_of_node(). The latter will eventually call pcibios_get_phb_of_node(). What makes you say that PCI devices shouldn't have nodes themselves? Thierry pgp5Ueibuccvm.pgp Description: PGP signature
Re: [PATCH] i2c: tegra: Fix ifdefs for suspend mode
On Sat, Aug 18, 2012 at 09:07:32AM +0200, Wolfram Sang wrote: On Mon, Jul 30, 2012 at 07:03:07PM +0530, Laxman Dewangan wrote: CONFIG_PM covers runtime only PM. I am a bit confused by this description. CONFIG_PM_RUNTIME covers runtime PM, no? The issue is that CONFIG_PM doesn't actually enable any of the PM callbacks, it only allows you to enable CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME. This means that if you use CONFIG_PM to protect your system sleep callbacks they may end up unreferenced if only runtime PM is enabled. Personally I'd rather just uncondtionally enable PM rather than faff around with all these ifdefs... signature.asc Description: Digital signature
Re: [PATCH] i2c: tegra: Fix ifdefs for suspend mode
On Sat, Aug 18, 2012 at 01:01:38PM +0100, Mark Brown wrote: On Sat, Aug 18, 2012 at 09:07:32AM +0200, Wolfram Sang wrote: On Mon, Jul 30, 2012 at 07:03:07PM +0530, Laxman Dewangan wrote: CONFIG_PM covers runtime only PM. I am a bit confused by this description. CONFIG_PM_RUNTIME covers runtime PM, no? The issue is that CONFIG_PM doesn't actually enable any of the PM callbacks, it only allows you to enable CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME. This means that if you use CONFIG_PM to protect your system sleep callbacks they may end up unreferenced if only runtime PM is enabled. Thanks for the explanation! Laxman, could you update the description to reflect that? -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] i2c-i801: Add device tree support
On Sat, Aug 18, 2012 at 01:17:45PM +0200, Thierry Reding wrote: On Sat, Aug 18, 2012 at 11:48:46AM +0200, Wolfram Sang wrote: On Wed, Aug 08, 2012 at 09:06:47AM +0200, Thierry Reding wrote: This commit adds support for probing slave devices parsed from the device tree. Signed-off-by: Thierry Reding thierry.red...@avionic-design.de How are the devicetree nodes of the slaves connected to this PCI device which should not have a node itself? The matching is done by pci_set_of_node() and pci_set_bus_of_node(). The latter will eventually call pcibios_get_phb_of_node(). What makes you say that PCI devices shouldn't have nodes themselves? I thought to recall that busses which can be probed at runtime should not have nodes but should be probed. Maybe that was wrong. Can you point me to a dts-file with an example? -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] i2c-i801: Add device tree support
On Sat, Aug 18, 2012 at 02:16:28PM +0200, Wolfram Sang wrote: On Sat, Aug 18, 2012 at 01:17:45PM +0200, Thierry Reding wrote: On Sat, Aug 18, 2012 at 11:48:46AM +0200, Wolfram Sang wrote: On Wed, Aug 08, 2012 at 09:06:47AM +0200, Thierry Reding wrote: This commit adds support for probing slave devices parsed from the device tree. Signed-off-by: Thierry Reding thierry.red...@avionic-design.de How are the devicetree nodes of the slaves connected to this PCI device which should not have a node itself? The matching is done by pci_set_of_node() and pci_set_bus_of_node(). The latter will eventually call pcibios_get_phb_of_node(). What makes you say that PCI devices shouldn't have nodes themselves? I thought to recall that busses which can be probed at runtime should not have nodes but should be probed. Maybe that was wrong. Can you point me to a dts-file with an example? That approach doesn't work for I2C controllers, though because you couldn't attach an slaves to them. The same goes for SPI controllers or any other dumb busses that can't be probed. arch/x86/platform/ce4100/falconfalls.dts is an example for an X86 platform where PCI devices have associated device tree nodes. Thierry pgpkZfwfyEWNk.pgp Description: PGP signature
[PATCH V2] i2c: tegra: protect suspend/resume callbacks with CONFIG_PM_SLEEP
The CONFIG_PM doesn't actually enable any of the PM callbacks, it
only allows to enable CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME.
This means if CONFIG_PM is used to protect system sleep callbacks
then it may end up unreferenced if only runtime PM is enabled.
Hence protecting sleep callbacks with CONFIG_PM_SLEEP.
Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
---
This patch will remove the compilation warning if CONFIG_PM_SLEEP
is not enabled.
Change from V1 -V2:
provide more details in description as per Mark's response.
drivers/i2c/busses/i2c-tegra.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 66eb53f..9a08c57 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -712,7 +712,7 @@ static int __devexit tegra_i2c_remove(struct
platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
static int tegra_i2c_suspend(struct device *dev)
{
struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
--
1.7.1.1
--
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 REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20
On Sat, Aug 18, 2012 at 12:32:34AM +0530, Laxman Dewangan wrote:
Tegra20 i2c controller does not support the continue transfer
which implements the I2C_M_NOSTART functionality of i2c
protocol mangling.
Removing the I2C_M_NOSTART functionality for Tegra20.
Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
---
This is rebased on linux-next-20120816.
drivers/i2c/busses/i2c-tegra.c | 73 +++
1 files changed, 58 insertions(+), 15 deletions(-)
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 7149625..554f713 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -27,6 +27,7 @@
#include linux/slab.h
#include linux/i2c-tegra.h
#include linux/of_i2c.h
+#include linux/of_device.h
#include linux/module.h
#include asm/unaligned.h
@@ -114,6 +115,15 @@ enum msg_end_type {
};
/**
+ * struct tegra_i2c_hw_feature : Different HW support on Tegra
+ * @has_continue_xfer_support: Continue transfer supports.
+ */
+
+struct tegra_i2c_hw_feature {
+ bool has_continue_xfer_support;
+};
+
+/**
* struct tegra_i2c_dev - per device i2c context
* @dev: device reference for power management
* @adapter: core i2c layer adapter information
@@ -148,6 +158,7 @@ struct tegra_i2c_dev {
int msg_read;
unsigned long bus_clk_rate;
bool is_suspended;
+ bool has_continue_xfer_support;
I wonder if it makes sense to carry a pointer here to the
tegra_i2c_hw_feature in use instead of copying all entries by hand,
since they might get more and more.
};
static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned long
reg)
@@ -563,7 +574,17 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap,
struct i2c_msg msgs[],
if (i2c_dev-is_suspended)
return -EBUSY;
- clk_prepare_enable(i2c_dev-div_clk);
+ /* Support I2C_M_NOSTART only if HW support continue xfer. */
+ for (i = 0; i num - 1; i++) {
+ if ((msgs[i + 1].flags I2C_M_NOSTART)
+ !i2c_dev-has_continue_xfer_support) {
+ dev_err(i2c_dev-dev,
+ mesg %d have illegal flag\n, i + 1);
+ return -EINVAL;
+ }
+ }
Drivers are requested to explicitly check for features of the I2C bus
(like M_NOSTART) before using them, so I'd skip this extra check.
+
+ clk_prepare_enable(i2c_dev-clk);
From a glimpse, this change looks unrelated at least. Even wrong, no?
for (i = 0; i num; i++) {
enum msg_end_type end_type = MSG_END_STOP;
if (i (num - 1)) {
@@ -582,8 +603,13 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap,
struct i2c_msg msgs[],
static u32 tegra_i2c_func(struct i2c_adapter *adap)
{
- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR |
- I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_NOSTART;
+ struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
+ u32 ret = I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR |
+ I2C_FUNC_PROTOCOL_MANGLING;
+
+ if (i2c_dev-has_continue_xfer_support)
+ ret |= I2C_FUNC_NOSTART;
+ return ret;
}
static const struct i2c_algorithm tegra_i2c_algo = {
@@ -591,6 +617,25 @@ static const struct i2c_algorithm tegra_i2c_algo = {
.functionality = tegra_i2c_func,
};
+static struct tegra_i2c_hw_feature tegra20_i2c_hw = {
+ .has_continue_xfer_support = false,
+};
+
+static struct tegra_i2c_hw_feature tegra30_i2c_hw = {
+ .has_continue_xfer_support = true,
+};
+
+#if defined(CONFIG_OF)
+/* Match table for of_platform binding */
+static const struct of_device_id tegra_i2c_of_match[] __devinitconst = {
+ { .compatible = nvidia,tegra30-i2c, .data = tegra30_i2c_hw, },
+ { .compatible = nvidia,tegra20-i2c, .data = tegra20_i2c_hw, },
+ { .compatible = nvidia,tegra20-i2c-dvc, .data = tegra20_i2c_hw, },
+ {},
+};
+MODULE_DEVICE_TABLE(of, tegra_i2c_of_match);
+#endif
+
static int __devinit tegra_i2c_probe(struct platform_device *pdev)
{
struct tegra_i2c_dev *i2c_dev;
@@ -600,6 +645,7 @@ static int __devinit tegra_i2c_probe(struct
platform_device *pdev)
struct clk *fast_clk;
const unsigned int *prop;
void __iomem *base;
+ const struct tegra_i2c_hw_feature *hw = tegra20_i2c_hw;
int irq;
int ret = 0;
@@ -659,11 +705,18 @@ static int __devinit tegra_i2c_probe(struct
platform_device *pdev)
i2c_dev-bus_clk_rate = be32_to_cpup(prop);
}
- if (pdev-dev.of_node)
+ if (pdev-dev.of_node) {
+ const struct of_device_id *match;
+ match = of_match_device(of_match_ptr(tegra_i2c_of_match),
+ pdev-dev);
+ hw = match-data;
Re: [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20
Thanks for review.
On Saturday 18 August 2012 06:17 PM, Wolfram Sang wrote:
* PGP Signed by an unknown key
On Sat, Aug 18, 2012 at 12:32:34AM +0530, Laxman Dewangan wrote:
+ bool has_continue_xfer_support;
I wonder if it makes sense to carry a pointer here to the
tegra_i2c_hw_feature in use instead of copying all entries by hand,
since they might get more and more.
Ok, we can store the hw pointer in device structure. I will send the new
patch.
};
static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned long
reg)
@@ -563,7 +574,17 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct
i2c_msg msgs[],
if (i2c_dev-is_suspended)
return -EBUSY;
- clk_prepare_enable(i2c_dev-div_clk);
+ /* Support I2C_M_NOSTART only if HW support continue xfer. */
+ for (i = 0; i num - 1; i++) {
+ if ((msgs[i + 1].flags I2C_M_NOSTART)
+ !i2c_dev-has_continue_xfer_support) {
+ dev_err(i2c_dev-dev,
+ mesg %d have illegal flag\n, i + 1);
+ return -EINVAL;
+ }
+ }
Drivers are requested to explicitly check for features of the I2C bus
(like M_NOSTART) before using them, so I'd skip this extra check.
Ok, I kept this as part of flag checking so illegal flag should not be
passed. I will remove this on next version patch.
+
+ clk_prepare_enable(i2c_dev-clk);
From a glimpse, this change looks unrelated at least. Even wrong, no?
It was already there, just before above check. Due to insertion of
check, this code shifted, otherwise it is not a new code.
--
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 2/6] i2c-xiic: Fix a possible NULL pointer access
On Sat, Aug 18, 2012 at 3:47 PM, Wolfram Sang w.s...@pengutronix.de wrote: On Thu, Aug 09, 2012 at 07:07:43PM +0530, Shubhrajyoti D wrote: platform_get_resource uses pdev so move the function platform_set_drvdata(pdev, NULL) after the get_resource. Yes agree. This descriptions sounds as you'd assume pdev is set to NULL here? Careful, platform_set_drvdata() does not clear pdev, but driver_data of pdev-dev! So, I guess the rest of this series is bogus? It would be worth checking if other drivers have the same pattern as the omap driver where the fix was valid for other reasons. Are you interested in doing that (perfectly fine to say no). Yes I can take that , lest someone with the platform/ test set-up can take that up. Regards, Wolfram -- 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/6] i2c: omap: Prevent NULL pointer dereference in remove
On Sat, Aug 18, 2012 at 3:39 PM, Wolfram Sang w.s...@pengutronix.de wrote: On Thu, Aug 09, 2012 at 07:07:42PM +0530, Shubhrajyoti D wrote: Prevent the NULL pointer access by moving the platform_set_drvdata function after the access of the pdev. [ 654.961761] Unable to handle kernel NULL pointer dereference at virtual address 0070 [ 654.970611] pgd = df254000 [ 654.973480] [0070] *pgd=9f1da831, *pte=, *ppte= [ 654.980163] Internal error: Oops: 17 [#1] SMP ARM [ 654.985076] Modules linked in: [ 654.988281] CPU: 1Not tainted (3.6.0-rc1-00031-ge547de1-dirty #339) [ 654.995330] PC is at omap_i2c_runtime_resume+0x8/0x148 [ 655.000732] LR is at omap_i2c_runtime_resume+0x8/0x148 Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com --- drivers/i2c/busses/i2c-omap.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index c8e3886..0c593d4 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1217,8 +1217,6 @@ static int __devexit omap_i2c_remove(struct platform_device *pdev) struct omap_i2c_dev *dev = platform_get_drvdata(pdev); int ret; - platform_set_drvdata(pdev, NULL); - i2c_del_adapter(dev-adapter); ret = pm_runtime_get_sync(pdev-dev); if (IS_ERR_VALUE(ret)) @@ -1227,6 +1225,8 @@ static int __devexit omap_i2c_remove(struct platform_device *pdev) omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); pm_runtime_put(pdev-dev); pm_runtime_disable(pdev-dev); + platform_set_drvdata(pdev, NULL); + return 0; } I think this patch is correct, because drvdata is used in the PM code of the driver and thus cleared too early. As such, this is a bugfix and should be not based on the big cleanup since it should go into this rc series. I will pick it as soon as the comments on the other patches are answered. BTW it was pointed out that the The platform_device object will be destroyed. Do you agree with http://www.spinics.net/lists/linux-omap/msg75553.html -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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 2/6] i2c-xiic: Fix a possible NULL pointer access
On Sat, Aug 18, 2012 at 6:52 PM, Shubhrajyoti Datta omaplinuxker...@gmail.com wrote: On Sat, Aug 18, 2012 at 3:47 PM, Wolfram Sang w.s...@pengutronix.de wrote: On Thu, Aug 09, 2012 at 07:07:43PM +0530, Shubhrajyoti D wrote: platform_get_resource uses pdev so move the function platform_set_drvdata(pdev, NULL) after the get_resource. Yes agree. This descriptions sounds as you'd assume pdev is set to NULL here? Careful, platform_set_drvdata() does not clear pdev, but driver_data of pdev-dev! So, I guess the rest of this series is bogus? It would be worth checking if other drivers have the same pattern as the omap driver where the fix was valid for other reasons. Are you interested in doing that (perfectly fine to say no). Yes I can take that , lest someone with the platform/ test set-up can take that up. By the way davinci and designware seem to have the issue however having a closer look even the probe err handling looks wrong. Regards, Wolfram -- 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 REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20
- clk_prepare_enable(i2c_dev-div_clk);
+ /* Support I2C_M_NOSTART only if HW support continue xfer. */
+ for (i = 0; i num - 1; i++) {
+ if ((msgs[i + 1].flags I2C_M_NOSTART)
+ !i2c_dev-has_continue_xfer_support) {
+ dev_err(i2c_dev-dev,
+ mesg %d have illegal flag\n, i + 1);
+ return -EINVAL;
+ }
+ }
Drivers are requested to explicitly check for features of the I2C bus
(like M_NOSTART) before using them, so I'd skip this extra check.
Ok, I kept this as part of flag checking so illegal flag should not
be passed. I will remove this on next version patch.
+
+ clk_prepare_enable(i2c_dev-clk);
From a glimpse, this change looks unrelated at least. Even wrong, no?
It was already there, just before above check. Due to insertion of
check, this code shifted, otherwise it is not a new code.
It used to be -div_clk and now it is -clk?
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 1/6] i2c: omap: Prevent NULL pointer dereference in remove
BTW it was pointed out that the The platform_device object will be destroyed. Do you agree with http://www.spinics.net/lists/linux-omap/msg75553.html Yes. I was actually wondering if that would be possible but wanted to check that somewhen later. Nice that it has already been done :) -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH V2] i2c: tegra: protect suspend/resume callbacks with CONFIG_PM_SLEEP
On Sat, Aug 18, 2012 at 05:49:58PM +0530, Laxman Dewangan wrote: The CONFIG_PM doesn't actually enable any of the PM callbacks, it only allows to enable CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME. This means if CONFIG_PM is used to protect system sleep callbacks then it may end up unreferenced if only runtime PM is enabled. Hence protecting sleep callbacks with CONFIG_PM_SLEEP. Signed-off-by: Laxman Dewangan ldewan...@nvidia.com Applied to -current, thanks! -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH REBASE 2/2] i2c: tegra: dynamically control fast clk
On Sat, Aug 18, 2012 at 12:32:35AM +0530, Laxman Dewangan wrote:
Tegra I2C driver enables the fast clock during initialization
and does not disable till driver removed.
Enable this clock before transfer and disable after transfer done.
Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
Except that this patch is affected from the flaw of the previous patch,
it looks good to me.
- clk_prepare_enable(i2c_dev-clk);
+ tegra_i2c_clock_enable(i2c_dev);
This should be div_clk.
for (i = 0; i num; i++) {
enum msg_end_type end_type = MSG_END_STOP;
if (i (num - 1)) {
@@ -597,7 +621,7 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap,
struct i2c_msg msgs[],
if (ret)
break;
}
- clk_disable_unprepare(i2c_dev-div_clk);
+ tegra_i2c_clock_disable(i2c_dev);
return ret ?: i;
If that is fixed, you can add
Reviewed-by: Wolfram Sang w.s...@pengutronix.de
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH] i2c-i801: Add device tree support
I thought to recall that busses which can be probed at runtime should not have nodes but should be probed. Maybe that was wrong. Can you point me to a dts-file with an example? That approach doesn't work for I2C controllers, though because you couldn't attach an slaves to them. The same goes for SPI controllers or any other dumb busses that can't be probed. I understand the problem. I was wondering how it has been solved. Thanks for the pointer, I got the idea now. arch/x86/platform/ce4100/falconfalls.dts is an example for an X86 platform where PCI devices have associated device tree nodes. Do you intend to send a dts file mainline which needs the patch you sent? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
[PATCH V2 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20
Tegra20 i2c controller does not support the continue transfer
which implements the I2C_M_NOSTART functionality of i2c
protocol mangling.
Removing the I2C_M_NOSTART functionality support for Tegra20.
Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
---
Changes from V1:
- make the hw feature structure as part of tegra device structure in
place of copying the member.
- Remove check for extra M_NOSTART flag if it is not supported in xfer function.
Please note that this should go on tegra common clock source tree to avoid
conflicts.
drivers/i2c/busses/i2c-tegra.c | 61 ++-
1 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 7149625..2f74236 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -27,6 +27,7 @@
#include linux/slab.h
#include linux/i2c-tegra.h
#include linux/of_i2c.h
+#include linux/of_device.h
#include linux/module.h
#include asm/unaligned.h
@@ -114,8 +115,18 @@ enum msg_end_type {
};
/**
+ * struct tegra_i2c_hw_feature : Different HW support on Tegra
+ * @has_continue_xfer_support: Continue transfer supports.
+ */
+
+struct tegra_i2c_hw_feature {
+ bool has_continue_xfer_support;
+};
+
+/**
* struct tegra_i2c_dev- per device i2c context
* @dev: device reference for power management
+ * @hw: Tegra i2c hw feature.
* @adapter: core i2c layer adapter information
* @div_clk: clock reference for div clock of i2c controller.
* @fast_clk: clock reference for fast clock of i2c controller.
@@ -133,6 +144,7 @@ enum msg_end_type {
*/
struct tegra_i2c_dev {
struct device *dev;
+ const struct tegra_i2c_hw_feature *hw;
struct i2c_adapter adapter;
struct clk *div_clk;
struct clk *fast_clk;
@@ -582,8 +594,13 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct
i2c_msg msgs[],
static u32 tegra_i2c_func(struct i2c_adapter *adap)
{
- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR |
- I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_NOSTART;
+ struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
+ u32 ret = I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR |
+ I2C_FUNC_PROTOCOL_MANGLING;
+
+ if (i2c_dev-hw-has_continue_xfer_support)
+ ret |= I2C_FUNC_NOSTART;
+ return ret;
}
static const struct i2c_algorithm tegra_i2c_algo = {
@@ -591,6 +608,25 @@ static const struct i2c_algorithm tegra_i2c_algo = {
.functionality = tegra_i2c_func,
};
+static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
+ .has_continue_xfer_support = false,
+};
+
+static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
+ .has_continue_xfer_support = true,
+};
+
+#if defined(CONFIG_OF)
+/* Match table for of_platform binding */
+static const struct of_device_id tegra_i2c_of_match[] __devinitconst = {
+ { .compatible = nvidia,tegra30-i2c, .data = tegra30_i2c_hw, },
+ { .compatible = nvidia,tegra20-i2c, .data = tegra20_i2c_hw, },
+ { .compatible = nvidia,tegra20-i2c-dvc, .data = tegra20_i2c_hw, },
+ {},
+};
+MODULE_DEVICE_TABLE(of, tegra_i2c_of_match);
+#endif
+
static int __devinit tegra_i2c_probe(struct platform_device *pdev)
{
struct tegra_i2c_dev *i2c_dev;
@@ -659,11 +695,18 @@ static int __devinit tegra_i2c_probe(struct
platform_device *pdev)
i2c_dev-bus_clk_rate = be32_to_cpup(prop);
}
- if (pdev-dev.of_node)
+ i2c_dev-hw = tegra20_i2c_hw;
+
+ if (pdev-dev.of_node) {
+ const struct of_device_id *match;
+ match = of_match_device(of_match_ptr(tegra_i2c_of_match),
+ pdev-dev);
+ i2c_dev-hw = match-data;
i2c_dev-is_dvc = of_device_is_compatible(pdev-dev.of_node,
nvidia,tegra20-i2c-dvc);
- else if (pdev-id == 3)
+ } else if (pdev-id == 3) {
i2c_dev-is_dvc = 1;
+ }
init_completion(i2c_dev-msg_complete);
platform_set_drvdata(pdev, i2c_dev);
@@ -751,16 +794,6 @@ static SIMPLE_DEV_PM_OPS(tegra_i2c_pm, tegra_i2c_suspend,
tegra_i2c_resume);
#define TEGRA_I2C_PM NULL
#endif
-#if defined(CONFIG_OF)
-/* Match table for of_platform binding */
-static const struct of_device_id tegra_i2c_of_match[] __devinitconst = {
- { .compatible = nvidia,tegra20-i2c, },
- { .compatible = nvidia,tegra20-i2c-dvc, },
- {},
-};
-MODULE_DEVICE_TABLE(of, tegra_i2c_of_match);
-#endif
-
static struct platform_driver tegra_i2c_driver = {
.probe = tegra_i2c_probe,
.remove = __devexit_p(tegra_i2c_remove),
--
1.7.1.1
--
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
[PATCH V2 2/2] i2c: tegra: dynamically control fast clk
Tegra I2C driver enables the fast clock during initialization
and does not disable till driver removed.
Enable this clock before transfer and disable after transfer done.
Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
Reviewed-by: Wolfram Sang w.s...@pengutronix.de
---
Changes from V1:
- Rebased again on patch 1.
drivers/i2c/busses/i2c-tegra.c | 35 ---
1 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 2f74236..20c35fa 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -363,12 +363,36 @@ static void tegra_dvc_init(struct tegra_i2c_dev *i2c_dev)
dvc_writel(i2c_dev, val, DVC_CTRL_REG1);
}
+static inline int tegra_i2c_clock_enable(struct tegra_i2c_dev *i2c_dev)
+{
+ int ret;
+ ret = clk_prepare_enable(i2c_dev-fast_clk);
+ if (ret 0) {
+ dev_err(i2c_dev-dev,
+ Enabling fast clk failed, err %d\n, ret);
+ return ret;
+ }
+ ret = clk_prepare_enable(i2c_dev-div_clk);
+ if (ret 0) {
+ dev_err(i2c_dev-dev,
+ Enabling div clk failed, err %d\n, ret);
+ clk_disable_unprepare(i2c_dev-fast_clk);
+ }
+ return ret;
+}
+
+static inline void tegra_i2c_clock_disable(struct tegra_i2c_dev *i2c_dev)
+{
+ clk_disable_unprepare(i2c_dev-div_clk);
+ clk_disable_unprepare(i2c_dev-fast_clk);
+}
+
static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
{
u32 val;
int err = 0;
- clk_prepare_enable(i2c_dev-div_clk);
+ tegra_i2c_clock_enable(i2c_dev);
tegra_periph_reset_assert(i2c_dev-div_clk);
udelay(2);
@@ -399,7 +423,7 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
if (tegra_i2c_flush_fifos(i2c_dev))
err = -ETIMEDOUT;
- clk_disable_unprepare(i2c_dev-div_clk);
+ tegra_i2c_clock_disable(i2c_dev);
if (i2c_dev-irq_disabled) {
i2c_dev-irq_disabled = 0;
@@ -575,7 +599,7 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct
i2c_msg msgs[],
if (i2c_dev-is_suspended)
return -EBUSY;
- clk_prepare_enable(i2c_dev-div_clk);
+ tegra_i2c_clock_enable(i2c_dev);
for (i = 0; i num; i++) {
enum msg_end_type end_type = MSG_END_STOP;
if (i (num - 1)) {
@@ -588,7 +612,7 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct
i2c_msg msgs[],
if (ret)
break;
}
- clk_disable_unprepare(i2c_dev-div_clk);
+ tegra_i2c_clock_disable(i2c_dev);
return ret ?: i;
}
@@ -724,8 +748,6 @@ static int __devinit tegra_i2c_probe(struct platform_device
*pdev)
return ret;
}
- clk_prepare_enable(i2c_dev-fast_clk);
-
i2c_set_adapdata(i2c_dev-adapter, i2c_dev);
i2c_dev-adapter.owner = THIS_MODULE;
i2c_dev-adapter.class = I2C_CLASS_HWMON;
@@ -739,7 +761,6 @@ static int __devinit tegra_i2c_probe(struct platform_device
*pdev)
ret = i2c_add_numbered_adapter(i2c_dev-adapter);
if (ret) {
dev_err(pdev-dev, Failed to add I2C adapter\n);
- clk_disable_unprepare(i2c_dev-fast_clk);
return ret;
}
--
1.7.1.1
--
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] i2c: mxs: Indicate that i2c driver was succesfully registered
From: Fabio Estevam fabio.este...@freescale.com It is useful information in dmesg to have a message indicating that the i2c driver was succesfully registered. Signed-off-by: Fabio Estevam fabio.este...@freescale.com -- drivers/i2c/busses/i2c-mxs.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c index 088c5c1..619cbc8 100644 --- a/drivers/i2c/busses/i2c-mxs.c +++ b/drivers/i2c/busses/i2c-mxs.c @@ -447,6 +447,8 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev) of_i2c_register_devices(adap); + dev_info(dev, MXS I2C adapter registered\n); + return 0; } -- 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
