Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
On Thu, Jan 15, 2015 at 04:54:14PM +0200, Laurent Pinchart wrote: DT nodes should use the more specific adi,adxl345 and adi,adxl346 compatible values instead. As the ADXL346 is backward-compatible with the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345, in that order. Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com --- Documentation/devicetree/bindings/i2c/trivial-devices.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt index 9f41d05be3be..757e42510517 100644 --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt @@ -18,8 +18,7 @@ adi,adt7475 +/-1C TDM Extended Temp Range I.C adi,adt7476 +/-1C TDM Extended Temp Range I.C adi,adt7490 +/-1C TDM Extended Temp Range I.C adi,adxl345 Three-Axis Digital Accelerometer -adi,adxl346 Three-Axis Digital Accelerometer -adi,adxl34x Three-Axis Digital Accelerometer +adi,adxl346 Three-Axis Digital Accelerometer (backward-compatibility value adi,adxl345 must be listed too) I'd rather drop 346 because there is no compatible for that one anywhere. No need to resend, I can fix it here... signature.asc Description: Digital signature
Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang w...@the-dreams.de wrote:
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -18,8 +18,7 @@ adi,adt7475 +/-1C TDM Extended Temp Range I.C
adi,adt7476 +/-1C TDM Extended Temp Range I.C
adi,adt7490 +/-1C TDM Extended Temp Range I.C
adi,adxl345 Three-Axis Digital Accelerometer
-adi,adxl346 Three-Axis Digital Accelerometer
-adi,adxl34x Three-Axis Digital Accelerometer
+adi,adxl346 Three-Axis Digital Accelerometer
(backward-compatibility value adi,adxl345 must be listed too)
I'd rather drop 346 because there is no compatible for that one anywhere.
No need to resend, I can fix it here...
If you drop adi,adxl346, checkpatch will start complaining if it encounters
it in a .dts.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
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 v2 2/2] input: adxl34x: Add OF match support
On Thu, Jan 15, 2015 at 06:45:33PM +0100, Geert Uytterhoeven wrote:
On Thu, Jan 15, 2015 at 3:54 PM, Laurent Pinchart
laurent.pinchart+rene...@ideasonboard.com wrote:
The I2C subsystem can match devices without explicit OF support based on
the part of their compatible property after the comma. However, this
mechanism uses the first compatible value only. For adxl34x OF device
nodes the compatible property will contain the more specific
adi,adxl345 or adi,adxl346 value first. This prevents the device
node from being matched with the adxl34x driver.
Fix this by adding an OF match table with an adi,adxl345 compatible
entry. There's no need to add the adi,adxl346 entry as the ADXL346 is
backward-compatible with the ADXL345 with differences handled by runtime
detection of the device model.
Thanks!
Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com
Acked-by: Geert Uytterhoeven geert+rene...@glider.be
--- a/drivers/input/misc/adxl34x-i2c.c
+++ b/drivers/input/misc/adxl34x-i2c.c
+#ifdef CONFIG_OF
+static const struct of_device_id adxl34x_of_id[] = {
+ /*
+* The ADXL346 is backward-compatible with the ADXL345. Differences
are
+* handled by runtime detection of the device model, there's thus no
+* need for listing the adi,adxl346 compatible value explicitly.
+*/
+ { .compatible = adi,adxl345, },
+ /*
+* Deprecated, DT nodes should use one or more of the
device-specific
+* compatible values adi,adxl345 and adi,adxl346.
Ideally, the two comments above are moved to a real DT binding document ;-)
+*/
+ { .compatible = adi,adxl34x, },
I'd append /* deprecated */ to the line above, so git grep adxl34x
will show its deprecated status.
I still do not understand what we are trying to fix here. Why is
adi,adxl34x compatible string no good anymore? If we start using exact
models and the physical device does not match do we abort probe? What is
the problem that we are solving here?
Thanks.
--
Dmitry
--
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: mv64xxx: enable acquisition of timeout value from Device Tree
From: Marcin Wojtas m...@semihalf.com
This commit enables obtaining timeout value in miliseconds using the
timeout-ms property. If it doesn't succeed, the default value of
1000ms is set.
The Device Tree binding documentation is updated accordingly.
[gregory.clem...@free-electrons.com: convert commit log to mainline
style]
Signed-off-by: Marcin Wojtas m...@semihalf.com
Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com
---
Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt | 5 +
drivers/i2c/busses/i2c-mv64xxx.c | 9 -
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
index 5c30026921ae..d7fa740b68f1 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
@@ -25,12 +25,16 @@ default frequency is 100kHz
whenever you're using the allwinner,sun6i-a31-i2c
compatible.
+ - timeout-ms : timeout value in miliseconds. If not set the default
+ timeout is 1000ms
+
Examples:
i2c@11000 {
compatible = marvell,mv64xxx-i2c;
reg = 0x11000 0x20;
interrupts = 29;
+ timeout-ms = 1000;
clock-frequency = 10;
};
@@ -40,5 +44,6 @@ For the Armada XP:
compatible = marvell,mv78230-i2c, marvell,mv64xxx-i2c;
reg = 0x11000 0x100;
interrupts = 29;
+ timeout-ms = 1000;
clock-frequency = 10;
};
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 30059c1df2a3..a379a17c5c21 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -803,7 +803,7 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
#else
const struct of_device_id *device;
struct device_node *np = dev-of_node;
- u32 bus_freq, tclk;
+ u32 bus_freq, tclk, timeout;
int rc = 0;
if (IS_ERR(drv_data-clk)) {
@@ -832,10 +832,9 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
reset_control_deassert(drv_data-rstc);
}
- /* Its not yet defined how timeouts will be specified in device tree.
-* So hard code the value to 1 second.
-*/
- drv_data-adapter.timeout = HZ;
+ if (of_property_read_u32(np, timeout-ms, timeout))
+ timeout = 1000; /* 1000ms by default */
+ drv_data-adapter.timeout = msecs_to_jiffies(timeout);
device = of_match_device(mv64xxx_i2c_of_match_table, dev);
if (!device)
--
1.9.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 v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
On Thu, Jan 15, 2015 at 06:32:31PM +0100, Geert Uytterhoeven wrote: On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang w...@the-dreams.de wrote: --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt @@ -18,8 +18,7 @@ adi,adt7475 +/-1C TDM Extended Temp Range I.C adi,adt7476 +/-1C TDM Extended Temp Range I.C adi,adt7490 +/-1C TDM Extended Temp Range I.C adi,adxl345 Three-Axis Digital Accelerometer -adi,adxl346 Three-Axis Digital Accelerometer -adi,adxl34x Three-Axis Digital Accelerometer +adi,adxl346 Three-Axis Digital Accelerometer (backward-compatibility value adi,adxl345 must be listed too) I'd rather drop 346 because there is no compatible for that one anywhere. No need to resend, I can fix it here... If you drop adi,adxl346, checkpatch will start complaining if it encounters it in a .dts. Boah, this is annoying. That means we need an 346 entry even if it is not different from 345 (which is fine by me). If checkpatch does it this way, that means the rule of thumb is to *always* have a dedicated compatible entry? Can someone confirm this? Why did we discuss then? Now, I am confused as well... signature.asc Description: Digital signature
Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
On Thu, Jan 15, 2015 at 6:43 PM, Wolfram Sang w...@the-dreams.de wrote:
On Thu, Jan 15, 2015 at 06:32:31PM +0100, Geert Uytterhoeven wrote:
On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang w...@the-dreams.de wrote:
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -18,8 +18,7 @@ adi,adt7475 +/-1C TDM Extended Temp Range I.C
adi,adt7476 +/-1C TDM Extended Temp Range I.C
adi,adt7490 +/-1C TDM Extended Temp Range I.C
adi,adxl345 Three-Axis Digital Accelerometer
-adi,adxl346 Three-Axis Digital Accelerometer
-adi,adxl34x Three-Axis Digital Accelerometer
+adi,adxl346 Three-Axis Digital Accelerometer
(backward-compatibility value adi,adxl345 must be listed too)
I'd rather drop 346 because there is no compatible for that one anywhere.
No need to resend, I can fix it here...
If you drop adi,adxl346, checkpatch will start complaining if it encounters
it in a .dts.
Boah, this is annoying. That means we need an 346 entry even if it is
not different from 345 (which is fine by me).
To be clear: you need the entry in the documentation. It can be omitted
from the driver if it's not (yet) used for matching.
If checkpatch does it this way, that means the rule of thumb is to
*always* have a dedicated compatible entry? Can someone confirm this?
All compatible values that are in use must be documented.
Checkpatch just greps in Documentation/devicetree/bindings/.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
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 v2 2/2] input: adxl34x: Add OF match support
On Thu, Jan 15, 2015 at 04:54:15PM +0200, Laurent Pinchart wrote: The I2C subsystem can match devices without explicit OF support based on the part of their compatible property after the comma. However, this mechanism uses the first compatible value only. For adxl34x OF device nodes the compatible property will contain the more specific adi,adxl345 or adi,adxl346 value first. This prevents the device node from being matched with the adxl34x driver. Fix this by adding an OF match table with an adi,adxl345 compatible entry. There's no need to add the adi,adxl346 entry as the ADXL346 is backward-compatible with the ADXL345 with differences handled by runtime detection of the device model. Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com Reviewed-by: Wolfram Sang wsa+rene...@sang-engineering.com signature.asc Description: Digital signature
Re: [PATCH] i2c: Mark instantiated device nodes with OF_POPULATE
On Thu, Jan 15, 2015 at 08:33:18PM +0200, Pantelis Antoniou wrote:
Mark (and unmark) device nodes with the POPULATE flag as appropriate.
This is required to avoid multi probing when using I2C and device
overlays containing a mux.
This patch is also more careful with the release of the adapter device
which caused a deadlock with muxes, and does not break the build
on !OF since the node flag accessors are not defined then.
Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
---
drivers/i2c/i2c-core.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 39d25a8..1d44e3a 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1122,6 +1122,10 @@ EXPORT_SYMBOL_GPL(i2c_new_device);
*/
void i2c_unregister_device(struct i2c_client *client)
{
+#if IS_ENABLED(CONFIG_OF_DYNAMIC)
Hi Pantelis,
I thought I read a note somewhere a couple of days ago suggesting that
CONFIG_OF_DYNAMIC would go away soon. Also, of_node_clear_flag is defined
in #ifdef CONFIG_OF, and AFAICS none of the other callers set OF_POPULATED
in the context of CONFIG_OF_DYNAMIC. Given that, wouldn't it be better
to use CONFIG_OF ?
Thanks,
Guenter
+ if (client-dev.of_node)
+ of_node_clear_flag(client-dev.of_node, OF_POPULATED);
+#endif
device_unregister(client-dev);
}
EXPORT_SYMBOL_GPL(i2c_unregister_device);
@@ -1441,8 +1445,11 @@ static void of_i2c_register_devices(struct i2c_adapter
*adap)
dev_dbg(adap-dev, of_i2c: walking child nodes\n);
- for_each_available_child_of_node(adap-dev.of_node, node)
+ for_each_available_child_of_node(adap-dev.of_node, node) {
+ if (of_node_test_and_set_flag(node, OF_POPULATED))
+ continue;
of_i2c_register_device(adap, node);
+ }
}
static int of_dev_node_match(struct device *dev, void *data)
@@ -1976,6 +1983,11 @@ static int of_i2c_notify(struct notifier_block *nb,
unsigned long action,
if (adap == NULL)
return NOTIFY_OK; /* not for us */
+ if (of_node_test_and_set_flag(rd-dn, OF_POPULATED)) {
+ put_device(adap-dev);
+ return NOTIFY_OK;
+ }
+
client = of_i2c_register_device(adap, rd-dn);
put_device(adap-dev);
@@ -1986,6 +1998,10 @@ static int of_i2c_notify(struct notifier_block *nb,
unsigned long action,
}
break;
case OF_RECONFIG_CHANGE_REMOVE:
+ /* already depopulated? */
+ if (!of_node_check_flag(rd-dn, OF_POPULATED))
+ return NOTIFY_OK;
+
/* find our device by node */
client = of_find_i2c_device_by_node(rd-dn);
if (client == NULL)
--
1.7.12
--
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 v2 2/2] input: adxl34x: Add OF match support
On Thu, Jan 15, 2015 at 3:54 PM, Laurent Pinchart
laurent.pinchart+rene...@ideasonboard.com wrote:
The I2C subsystem can match devices without explicit OF support based on
the part of their compatible property after the comma. However, this
mechanism uses the first compatible value only. For adxl34x OF device
nodes the compatible property will contain the more specific
adi,adxl345 or adi,adxl346 value first. This prevents the device
node from being matched with the adxl34x driver.
Fix this by adding an OF match table with an adi,adxl345 compatible
entry. There's no need to add the adi,adxl346 entry as the ADXL346 is
backward-compatible with the ADXL345 with differences handled by runtime
detection of the device model.
Thanks!
Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com
Acked-by: Geert Uytterhoeven geert+rene...@glider.be
--- a/drivers/input/misc/adxl34x-i2c.c
+++ b/drivers/input/misc/adxl34x-i2c.c
+#ifdef CONFIG_OF
+static const struct of_device_id adxl34x_of_id[] = {
+ /*
+* The ADXL346 is backward-compatible with the ADXL345. Differences
are
+* handled by runtime detection of the device model, there's thus no
+* need for listing the adi,adxl346 compatible value explicitly.
+*/
+ { .compatible = adi,adxl345, },
+ /*
+* Deprecated, DT nodes should use one or more of the device-specific
+* compatible values adi,adxl345 and adi,adxl346.
Ideally, the two comments above are moved to a real DT binding document ;-)
+*/
+ { .compatible = adi,adxl34x, },
I'd append /* deprecated */ to the line above, so git grep adxl34x
will show its deprecated status.
+ { }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
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 v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
On Thu, Jan 15, 2015 at 06:02:09PM +0100, Wolfram Sang wrote: On Thu, Jan 15, 2015 at 04:54:14PM +0200, Laurent Pinchart wrote: DT nodes should use the more specific adi,adxl345 and adi,adxl346 compatible values instead. As the ADXL346 is backward-compatible with the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345, in that order. Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com --- Documentation/devicetree/bindings/i2c/trivial-devices.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt index 9f41d05be3be..757e42510517 100644 --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt @@ -18,8 +18,7 @@ adi,adt7475 +/-1C TDM Extended Temp Range I.C adi,adt7476+/-1C TDM Extended Temp Range I.C adi,adt7490+/-1C TDM Extended Temp Range I.C adi,adxl345Three-Axis Digital Accelerometer -adi,adxl346Three-Axis Digital Accelerometer -adi,adxl34xThree-Axis Digital Accelerometer +adi,adxl346Three-Axis Digital Accelerometer (backward-compatibility value adi,adxl345 must be listed too) I'd rather drop 346 because there is no compatible for that one anywhere. No need to resend, I can fix it here... So color me confused now. We deprecated adi,adxl34x because it is generic, but then we adding specific adi,adxl345 that covers again both devices? Why do we do this exactly? What is broken at the moment? Thanks. -- Dmitry -- 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: Mark instantiated device nodes with OF_POPULATE
On Thu, Jan 15, 2015 at 06:33:55PM +0200, Pantelis Antoniou wrote: Mark (and unmark) device nodes with the POPULATE flag as appropriate. This is required to avoid multi probing when using I2C and device overlays containing a mux. This patch is also more careful with the release of the adapter device which caused a deadlock with muxes. Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com Applied to for-current, thanks! signature.asc Description: Digital signature
Re: [PATCH] i2c: Mark instantiated device nodes with OF_POPULATE
Hi Wolfram, On Jan 15, 2015, at 19:35 , Wolfram Sang w...@the-dreams.de wrote: On Thu, Jan 15, 2015 at 06:33:55PM +0200, Pantelis Antoniou wrote: Mark (and unmark) device nodes with the POPULATE flag as appropriate. This is required to avoid multi probing when using I2C and device overlays containing a mux. This patch is also more careful with the release of the adapter device which caused a deadlock with muxes. Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com Applied to for-current, thanks! Just sent an updated patch that doesn’t break the build on !OF; some OF accessors are not defined then. Regards — Pantelis -- 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: Mark instantiated device nodes with OF_POPULATE
Mark (and unmark) device nodes with the POPULATE flag as appropriate.
This is required to avoid multi probing when using I2C and device
overlays containing a mux.
This patch is also more careful with the release of the adapter device
which caused a deadlock with muxes, and does not break the build
on !OF since the node flag accessors are not defined then.
Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
---
drivers/i2c/i2c-core.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 39d25a8..1d44e3a 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1122,6 +1122,10 @@ EXPORT_SYMBOL_GPL(i2c_new_device);
*/
void i2c_unregister_device(struct i2c_client *client)
{
+#if IS_ENABLED(CONFIG_OF_DYNAMIC)
+ if (client-dev.of_node)
+ of_node_clear_flag(client-dev.of_node, OF_POPULATED);
+#endif
device_unregister(client-dev);
}
EXPORT_SYMBOL_GPL(i2c_unregister_device);
@@ -1441,8 +1445,11 @@ static void of_i2c_register_devices(struct i2c_adapter
*adap)
dev_dbg(adap-dev, of_i2c: walking child nodes\n);
- for_each_available_child_of_node(adap-dev.of_node, node)
+ for_each_available_child_of_node(adap-dev.of_node, node) {
+ if (of_node_test_and_set_flag(node, OF_POPULATED))
+ continue;
of_i2c_register_device(adap, node);
+ }
}
static int of_dev_node_match(struct device *dev, void *data)
@@ -1976,6 +1983,11 @@ static int of_i2c_notify(struct notifier_block *nb,
unsigned long action,
if (adap == NULL)
return NOTIFY_OK; /* not for us */
+ if (of_node_test_and_set_flag(rd-dn, OF_POPULATED)) {
+ put_device(adap-dev);
+ return NOTIFY_OK;
+ }
+
client = of_i2c_register_device(adap, rd-dn);
put_device(adap-dev);
@@ -1986,6 +1998,10 @@ static int of_i2c_notify(struct notifier_block *nb,
unsigned long action,
}
break;
case OF_RECONFIG_CHANGE_REMOVE:
+ /* already depopulated? */
+ if (!of_node_check_flag(rd-dn, OF_POPULATED))
+ return NOTIFY_OK;
+
/* find our device by node */
client = of_find_i2c_device_by_node(rd-dn);
if (client == NULL)
--
1.7.12
--
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 v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
On Thu, Jan 15, 2015 at 3:54 PM, Laurent Pinchart
laurent.pinchart+rene...@ideasonboard.com wrote:
DT nodes should use the more specific adi,adxl345 and adi,adxl346
compatible values instead. As the ADXL346 is backward-compatible with
the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
in that order.
Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com
Acked-by: Geert Uytterhoeven geert+rene...@glider.be
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
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] [RFC] i2c: Don't wait for device release in i2c_del_adapter
Hi Greg, On Jan 14, 2015, at 22:41 , Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Wed, Jan 14, 2015 at 07:24:22PM +0200, Pantelis Antoniou wrote: I’ll try to dig around tomorrow and see what the real device reference counts are, but my hunch goes like this: MUX +—- ADAPTER +— DEV. Mux remove method is called, i2c_del_mux_adapter is called on all the channels of the mux, calling in turn i2c_del_adapter which hangs on completion of the dev_released. The call to device_unregister never calls the device_type callback (i2c_adapter_dev_release) because the reference count is not 1 at that point, someone else is having another reference. First of all, my head hurts. Tracking device references ain’t easy. Is there some kind of debugging method you’d recommend for this? Who has that reference? It's not sysfs, right? Or is it? How is the remove method being called, is that coming from a sysfs file? No, it’s not sysfs. Sysfs is not used at all. The remove method of the mux gets called and on it’s remove method i2c_del_mux_adapter is called, which in turn calls i2c_del_adapter() which hangs. Anyway, finally figured out what the problem was, a reference was held, and not released properly in the i2c notifier pathc. I’ll send out an updated patch later today. Having that call wait for the other release call to happen is really old, as Jean points out, from 2003. We have fixed sysfs since then to detach the files from the devices easier, we used to have some nasy reference count issues in that area. Perhaps this isn't needed anymore, It’s awfully easy to get a hang with this blocking wait. Turns a memory leak into a hard hang. I don't remember the i2c core code at all, that was a long time ago. Bummer. thanks, greg k-h Regards — Pantelis -- 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 0/2] Fix OF match for adxl34x driver
Hello, This patch set fixes OF matching for the adxl34x driver when the DT node lists a device-specific adi,adxl345 or adi,adxl346 compatible value first. The first version (see http://www.spinics.net/lists/linux-i2c/msg18107.html) added an OF match entry for the adi,adxl34x compatible string. The discussion that followed concluded that that compatible string should be deprecated and that the driver should match the device-specific strings instead. The first patch thus deprecates the adi,adxl34x compatible string by removing it the DT trivial devices list, and the second patch then adds an OF match table to the adxl34x driver. Laurent Pinchart (2): DT: i2c: Deprecate adi,adxl34x compatible string input: adxl34x: Add OF match support .../devicetree/bindings/i2c/trivial-devices.txt | 3 +-- drivers/input/misc/adxl34x-i2c.c| 21 + 2 files changed, 22 insertions(+), 2 deletions(-) -- Regards, Laurent Pinchart -- 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: Mark instantiated device nodes with OF_POPULATE
Mark (and unmark) device nodes with the POPULATE flag as appropriate.
This is required to avoid multi probing when using I2C and device
overlays containing a mux.
This patch is also more careful with the release of the adapter device
which caused a deadlock with muxes.
Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
---
drivers/i2c/i2c-core.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 39d25a8..8c32ee3 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1122,6 +1122,8 @@ EXPORT_SYMBOL_GPL(i2c_new_device);
*/
void i2c_unregister_device(struct i2c_client *client)
{
+ if (client-dev.of_node)
+ of_node_clear_flag(client-dev.of_node, OF_POPULATED);
device_unregister(client-dev);
}
EXPORT_SYMBOL_GPL(i2c_unregister_device);
@@ -1441,8 +1443,11 @@ static void of_i2c_register_devices(struct i2c_adapter
*adap)
dev_dbg(adap-dev, of_i2c: walking child nodes\n);
- for_each_available_child_of_node(adap-dev.of_node, node)
+ for_each_available_child_of_node(adap-dev.of_node, node) {
+ if (of_node_test_and_set_flag(node, OF_POPULATED))
+ continue;
of_i2c_register_device(adap, node);
+ }
}
static int of_dev_node_match(struct device *dev, void *data)
@@ -1976,6 +1981,11 @@ static int of_i2c_notify(struct notifier_block *nb,
unsigned long action,
if (adap == NULL)
return NOTIFY_OK; /* not for us */
+ if (of_node_test_and_set_flag(rd-dn, OF_POPULATED)) {
+ put_device(adap-dev);
+ return NOTIFY_OK;
+ }
+
client = of_i2c_register_device(adap, rd-dn);
put_device(adap-dev);
@@ -1986,6 +1996,10 @@ static int of_i2c_notify(struct notifier_block *nb,
unsigned long action,
}
break;
case OF_RECONFIG_CHANGE_REMOVE:
+ /* already depopulated? */
+ if (!of_node_check_flag(rd-dn, OF_POPULATED))
+ return NOTIFY_OK;
+
/* find our device by node */
client = of_find_i2c_device_by_node(rd-dn);
if (client == NULL)
--
1.7.12
--
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 2/3] i2c: iproc: Add Broadcom iProc I2C Driver
Hello,
On Thu, Jan 15, 2015 at 01:07:05PM +0100, Wolfram Sang wrote:
+ iproc_i2c-msg = msg;
Can it happen that iproc_i2c-msg still holds an uncompleted message
here or is this serialized by the core? Wolfram? Either here something
We have per-adapter locks serializing transfers, if you mean that?
ok, so in the efm32 driver the if (ddata-msgs) condition in
efm32_i2c_master_xfer can never be true, right?
+static int bcm_iproc_i2c_remove(struct platform_device *pdev)
+{
+ struct bcm_iproc_i2c_dev *iproc_i2c = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(iproc_i2c-adapter);
You need to free the irq before i2c_del_adapter.
One could also keep using devm_request_irq and disable all interrupts
sources here?
calling devm_free_irq would work or writel(0, iproc_i2c-base +
IE_OFFSET) + synchronize_irq().
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
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 v2] of: i2c: Add idle-disconnect DT property to PCA954x mux driver
Hi Wolfram, On Thursday 15 January 2015 14:19:35 Wolfram Sang wrote: On Thu, Jan 15, 2015 at 02:09:09PM +0100, Alexander Sverdlin wrote: On 15/01/15 13:32, ext Wolfram Sang wrote: On Fri, Dec 19, 2014 at 06:00:10PM +0100, Alexander Sverdlin wrote: of: i2c: Add idle-disconnect DT property to PCA954x mux driver Add idle-disconnect device tree property to PCA954x mux driver. The new property forces the multiplexer to disconnect child buses in idle state. This is used, for example, when there are several multiplexers on the same bus and the devices on the underlying buses might have same I2C addresses. Basically OK. Question to DT maintainers: idle-disconnect, i2c-mux-idle-disconnect, or is there another existing binding we could use? At the same time old (and not used in the tree) platform data binding deselect_on_exit is removed to simplify the implementation. Old binding has different (per-channel) semantics and doesn't fit well in the new concept. I'd prefer to keep it. It should be only one || more. It is not really in the way IMO. It complicates the implementation 3x times :) This is part of our discussion with Laurent: Does it? I don't want DT and platform_data to behave equally. I just want to keep being backwards compatible. So, I'd suggest: (pdata pdata-modes[num].deselect_on_exit) || idle_disconnect ? pca954x_deselect_mux : NULL); I'm not keen to brake out-of-tree code (if any), but may be it will be decided to drop this per-channel deselect_on_exit, because it's not used at least in the kernel tree... I couldn't find a user of the platform_data, at all. But removing platform_data support is a seperate patch, and deprecating platform_data is a seperate and general issue IMO. Sure, but it could be a preliminary patch on top of which to add idle- disconnect DT support :-) -- Regards, Laurent Pinchart -- 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 v2] of: i2c: Add idle-disconnect DT property to PCA954x mux driver
On Thu, Jan 15, 2015 at 6:32 AM, Wolfram Sang w...@the-dreams.de wrote: On Fri, Dec 19, 2014 at 06:00:10PM +0100, Alexander Sverdlin wrote: of: i2c: Add idle-disconnect DT property to PCA954x mux driver Add idle-disconnect device tree property to PCA954x mux driver. The new property forces the multiplexer to disconnect child buses in idle state. This is used, for example, when there are several multiplexers on the same bus and the devices on the underlying buses might have same I2C addresses. Basically OK. Question to DT maintainers: idle-disconnect, i2c-mux-idle-disconnect, or is there another existing binding we could use? Not that I'm aware of. If this is specific to i2c-mux's then I'd go with the latter. Either way, this should go in a common i2c or i2c-mux binding doc. Alternatively, couldn't the kernel do this automatically when the above conditions happen? Rob -- 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 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
DT nodes should use the more specific adi,adxl345 and adi,adxl346 compatible values instead. As the ADXL346 is backward-compatible with the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345, in that order. Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com --- Documentation/devicetree/bindings/i2c/trivial-devices.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt index 9f41d05be3be..757e42510517 100644 --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt @@ -18,8 +18,7 @@ adi,adt7475 +/-1C TDM Extended Temp Range I.C adi,adt7476+/-1C TDM Extended Temp Range I.C adi,adt7490+/-1C TDM Extended Temp Range I.C adi,adxl345Three-Axis Digital Accelerometer -adi,adxl346Three-Axis Digital Accelerometer -adi,adxl34xThree-Axis Digital Accelerometer +adi,adxl346Three-Axis Digital Accelerometer (backward-compatibility value adi,adxl345 must be listed too) at,24c08 i2c serial eeprom (24cxx) atmel,24c00i2c serial eeprom (24cxx) atmel,24c01i2c serial eeprom (24cxx) -- 2.0.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 v2 2/2] input: adxl34x: Add OF match support
The I2C subsystem can match devices without explicit OF support based on
the part of their compatible property after the comma. However, this
mechanism uses the first compatible value only. For adxl34x OF device
nodes the compatible property will contain the more specific
adi,adxl345 or adi,adxl346 value first. This prevents the device
node from being matched with the adxl34x driver.
Fix this by adding an OF match table with an adi,adxl345 compatible
entry. There's no need to add the adi,adxl346 entry as the ADXL346 is
backward-compatible with the ADXL345 with differences handled by runtime
detection of the device model.
Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com
---
drivers/input/misc/adxl34x-i2c.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/input/misc/adxl34x-i2c.c b/drivers/input/misc/adxl34x-i2c.c
index 416f47ddcc90..1e028f1f221c 100644
--- a/drivers/input/misc/adxl34x-i2c.c
+++ b/drivers/input/misc/adxl34x-i2c.c
@@ -10,6 +10,7 @@
#include linux/input.h /* BUS_I2C */
#include linux/i2c.h
#include linux/module.h
+#include linux/of.h
#include linux/types.h
#include linux/pm.h
#include adxl34x.h
@@ -137,11 +138,31 @@ static const struct i2c_device_id adxl34x_id[] = {
MODULE_DEVICE_TABLE(i2c, adxl34x_id);
+#ifdef CONFIG_OF
+static const struct of_device_id adxl34x_of_id[] = {
+ /*
+* The ADXL346 is backward-compatible with the ADXL345. Differences are
+* handled by runtime detection of the device model, there's thus no
+* need for listing the adi,adxl346 compatible value explicitly.
+*/
+ { .compatible = adi,adxl345, },
+ /*
+* Deprecated, DT nodes should use one or more of the device-specific
+* compatible values adi,adxl345 and adi,adxl346.
+*/
+ { .compatible = adi,adxl34x, },
+ { }
+};
+
+MODULE_DEVICE_TABLE(of, adxl34x_of_id);
+#endif
+
static struct i2c_driver adxl34x_driver = {
.driver = {
.name = adxl34x,
.owner = THIS_MODULE,
.pm = adxl34x_i2c_pm,
+ .of_match_table = of_match_ptr(adxl34x_of_id),
},
.probe= adxl34x_i2c_probe,
.remove = adxl34x_i2c_remove,
--
2.0.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 v2 2/2] input: adxl34x: Add OF match support
On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote: On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote: I still do not understand what we are trying to fix here. Why is adi,adxl34x compatible string no good anymore? If we start using exact models and the physical device does not match do we abort probe? What is the problem that we are solving here? Because there's no guarantee that the driver actually supports all adi,adxl34x with x = 0..9, some of which don't exist yet. That's one of the reasons. Another one is that the adxl34x driver won't match DT nodes that list the adi,adxl34x compatible value in positions other than the first. -- Regards, Laurent Pinchart -- 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: mv64xxx: enable acquisition of timeout value from Device Tree
Dear Gregory CLEMENT, On Thu, 15 Jan 2015 18:24:31 +0100, Gregory CLEMENT wrote: diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt index 5c30026921ae..d7fa740b68f1 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt +++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt @@ -25,12 +25,16 @@ default frequency is 100kHz whenever you're using the allwinner,sun6i-a31-i2c compatible. + - timeout-ms : timeout value in miliseconds. If not set the default + timeout is 1000ms I think this description lacks one information: the timeout of *what* ? Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- 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: Mark instantiated device nodes with OF_POPULATE
On Thu, Jan 15, 2015 at 09:12:36PM +0200, Pantelis Antoniou wrote:
Hi Guenter,
On Jan 15, 2015, at 20:55 , Guenter Roeck li...@roeck-us.net wrote:
On Thu, Jan 15, 2015 at 08:33:18PM +0200, Pantelis Antoniou wrote:
Mark (and unmark) device nodes with the POPULATE flag as appropriate.
This is required to avoid multi probing when using I2C and device
overlays containing a mux.
This patch is also more careful with the release of the adapter device
which caused a deadlock with muxes, and does not break the build
on !OF since the node flag accessors are not defined then.
Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
---
drivers/i2c/i2c-core.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 39d25a8..1d44e3a 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1122,6 +1122,10 @@ EXPORT_SYMBOL_GPL(i2c_new_device);
*/
void i2c_unregister_device(struct i2c_client *client)
{
+#if IS_ENABLED(CONFIG_OF_DYNAMIC)
Hi Pantelis,
I thought I read a note somewhere a couple of days ago suggesting that
CONFIG_OF_DYNAMIC would go away soon. Also, of_node_clear_flag is defined
in #ifdef CONFIG_OF, and AFAICS none of the other callers set OF_POPULATED
in the context of CONFIG_OF_DYNAMIC. Given that, wouldn't it be better
to use CONFIG_OF ?
Thanks,
Guenter
Well, I thought about it. Thing is that the notifier is under CONFIG_DYNAMIC,
and it seems it’s natural to be that way.
When we move to always enabling CONFIG_DYNAMIC the change to CONFIG_OF will be
part of the conversion.
Ok.
Thanks,
Guenter
--
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 v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
Hi Wolfram, On Thursday 15 January 2015 18:43:33 Wolfram Sang wrote: On Thu, Jan 15, 2015 at 06:32:31PM +0100, Geert Uytterhoeven wrote: On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang w...@the-dreams.de wrote: --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt @@ -18,8 +18,7 @@ adi,adt7475 +/-1C TDM Extended Temp Range I.C adi,adt7476 +/-1C TDM Extended Temp Range I.C adi,adt7490 +/-1C TDM Extended Temp Range I.C adi,adxl345 Three-Axis Digital Accelerometer -adi,adxl346 Three-Axis Digital Accelerometer -adi,adxl34x Three-Axis Digital Accelerometer +adi,adxl346 Three-Axis Digital Accelerometer (backward-compatibility value adi,adxl345 must be listed too) I'd rather drop 346 because there is no compatible for that one anywhere. No need to resend, I can fix it here... If you drop adi,adxl346, checkpatch will start complaining if it encounters it in a .dts. Boah, this is annoying. That means we need an 346 entry even if it is not different from 345 (which is fine by me). If checkpatch does it this way, that means the rule of thumb is to *always* have a dedicated compatible entry? Can someone confirm this? I believe we should register a new compatible entry for a device when the device isn't identical, from a compatibility point of view, to an already registered device. In this specific case the adxl346 offers addition features compared to the adxl345. It thus qualify for its own compatible string. Its DT nodes should list both the adi,adxl346 and adi,adxl345 compatible strings in that order, as the chip is compatible with the adxl345. On the driver side, given that we don't need to differentiate between the devices based on the compatible string (as runtime model detection is possible) we don't need to add a match entry for adi,adxl346. Why did we discuss then? Now, I am confused as well... -- Regards, Laurent Pinchart -- 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: Mark instantiated device nodes with OF_POPULATE
On Thu, Jan 15, 2015 at 12:33 PM, Pantelis Antoniou
pantelis.anton...@konsulko.com wrote:
Mark (and unmark) device nodes with the POPULATE flag as appropriate.
This is required to avoid multi probing when using I2C and device
overlays containing a mux.
This patch is also more careful with the release of the adapter device
which caused a deadlock with muxes, and does not break the build
on !OF since the node flag accessors are not defined then.
Why don't we define them then and get rid of the ifdef below.
It may be good to split this out of of.h because we don't want drivers
fiddling with these bits.
Rob
Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
---
drivers/i2c/i2c-core.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 39d25a8..1d44e3a 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1122,6 +1122,10 @@ EXPORT_SYMBOL_GPL(i2c_new_device);
*/
void i2c_unregister_device(struct i2c_client *client)
{
+#if IS_ENABLED(CONFIG_OF_DYNAMIC)
+ if (client-dev.of_node)
+ of_node_clear_flag(client-dev.of_node, OF_POPULATED);
+#endif
device_unregister(client-dev);
}
EXPORT_SYMBOL_GPL(i2c_unregister_device);
@@ -1441,8 +1445,11 @@ static void of_i2c_register_devices(struct i2c_adapter
*adap)
dev_dbg(adap-dev, of_i2c: walking child nodes\n);
- for_each_available_child_of_node(adap-dev.of_node, node)
+ for_each_available_child_of_node(adap-dev.of_node, node) {
+ if (of_node_test_and_set_flag(node, OF_POPULATED))
+ continue;
of_i2c_register_device(adap, node);
+ }
}
static int of_dev_node_match(struct device *dev, void *data)
@@ -1976,6 +1983,11 @@ static int of_i2c_notify(struct notifier_block *nb,
unsigned long action,
if (adap == NULL)
return NOTIFY_OK; /* not for us */
+ if (of_node_test_and_set_flag(rd-dn, OF_POPULATED)) {
+ put_device(adap-dev);
+ return NOTIFY_OK;
+ }
+
client = of_i2c_register_device(adap, rd-dn);
put_device(adap-dev);
@@ -1986,6 +1998,10 @@ static int of_i2c_notify(struct notifier_block *nb,
unsigned long action,
}
break;
case OF_RECONFIG_CHANGE_REMOVE:
+ /* already depopulated? */
+ if (!of_node_check_flag(rd-dn, OF_POPULATED))
+ return NOTIFY_OK;
+
/* find our device by node */
client = of_find_i2c_device_by_node(rd-dn);
if (client == NULL)
--
1.7.12
--
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 v2 2/2] input: adxl34x: Add OF match support
On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote: On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote: On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote: I still do not understand what we are trying to fix here. Why is adi,adxl34x compatible string no good anymore? If we start using exact models and the physical device does not match do we abort probe? What is the problem that we are solving here? Because there's no guarantee that the driver actually supports all adi,adxl34x with x = 0..9, some of which don't exist yet. So, what? When we encounter such devices and decide that we actually want a different driver for them (instead of enhancing the existing one) we'll give them their own compatible string. It's not like adi,adxl348 will automatically match with adi,adxl34x, is it? That's one of the reasons. Another one is that the adxl34x driver won't match DT nodes that list the adi,adxl34x compatible value in positions other than the first. Will it match anything else in the position other than 1st (i.e. if device has compatible sting like adi,adxl345-1, adi,adxl345)? Why adi,adxl34x is special? Thanks. -- Dmitry -- 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 v2 2/2] input: adxl34x: Add OF match support
On Thursday 15 January 2015 13:06:32 Dmitry Torokhov wrote: On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote: On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote: On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote: I still do not understand what we are trying to fix here. Why is adi,adxl34x compatible string no good anymore? If we start using exact models and the physical device does not match do we abort probe? What is the problem that we are solving here? Because there's no guarantee that the driver actually supports all adi,adxl34x with x = 0..9, some of which don't exist yet. So, what? When we encounter such devices and decide that we actually want a different driver for them (instead of enhancing the existing one) we'll give them their own compatible string. It's not like adi,adxl348 will automatically match with adi,adxl34x, is it? Please remember that compatible strings shouldn't be decided based on a particular operating system implementation. That's one of the reasons. Another one is that the adxl34x driver won't match DT nodes that list the adi,adxl34x compatible value in positions other than the first. Will it match anything else in the position other than 1st (i.e. if device has compatible sting like adi,adxl345-1, adi,adxl345)? Why adi,adxl34x is special? The problem on the driver side is that the automatic I2C DT compatible to device name matching implementation only tries to match with the first compatible string without looking at the other ones. The driver thus needs to be enhanced with an explicit OF match table to be able to match on DT nodes that specify adi,adxl345 or adi,adxl346 as the first compatible entry. -- Regards, Laurent Pinchart -- 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 v2 2/2] input: adxl34x: Add OF match support
On Thu, Jan 15, 2015 at 11:34:00PM +0200, Laurent Pinchart wrote: On Thursday 15 January 2015 13:06:32 Dmitry Torokhov wrote: On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote: On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote: On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote: I still do not understand what we are trying to fix here. Why is adi,adxl34x compatible string no good anymore? If we start using exact models and the physical device does not match do we abort probe? What is the problem that we are solving here? Because there's no guarantee that the driver actually supports all adi,adxl34x with x = 0..9, some of which don't exist yet. So, what? When we encounter such devices and decide that we actually want a different driver for them (instead of enhancing the existing one) we'll give them their own compatible string. It's not like adi,adxl348 will automatically match with adi,adxl34x, is it? Please remember that compatible strings shouldn't be decided based on a particular operating system implementation. In this case we can never have generic compatible strings of whatever since in 10 years there might appear an OS that handles them differently. Let's be a bit realistic here as well. That's one of the reasons. Another one is that the adxl34x driver won't match DT nodes that list the adi,adxl34x compatible value in positions other than the first. Will it match anything else in the position other than 1st (i.e. if device has compatible sting like adi,adxl345-1, adi,adxl345)? Why adi,adxl34x is special? The problem on the driver side is that the automatic I2C DT compatible to device name matching implementation only tries to match with the first compatible string without looking at the other ones. The driver thus needs to be enhanced with an explicit OF match table to be able to match on DT nodes that specify adi,adxl345 or adi,adxl346 as the first compatible entry. Why don't we enhance I2C core instead to do proper matching? -- Dmitry -- 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 v2 2/2] input: adxl34x: Add OF match support
Hello. On 01/15/2015 11:34 PM, Laurent Pinchart wrote: I still do not understand what we are trying to fix here. Why is adi,adxl34x compatible string no good anymore? If we start using exact models and the physical device does not match do we abort probe? What is the problem that we are solving here? Because there's no guarantee that the driver actually supports all adi,adxl34x with x = 0..9, some of which don't exist yet. That's one of the reasons. Another one is that the adxl34x driver won't match DT nodes that list the adi,adxl34x compatible value in positions other than the first. Let's also not forget that wildcards in the compatible prop are not really allowed. WBR, Sergei -- 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 v2 2/2] input: adxl34x: Add OF match support
Hi Dmitry, On Thursday 15 January 2015 13:50:53 Dmitry Torokhov wrote: On Thu, Jan 15, 2015 at 11:34:00PM +0200, Laurent Pinchart wrote: On Thursday 15 January 2015 13:06:32 Dmitry Torokhov wrote: On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote: On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote: On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote: I still do not understand what we are trying to fix here. Why is adi,adxl34x compatible string no good anymore? If we start using exact models and the physical device does not match do we abort probe? What is the problem that we are solving here? Because there's no guarantee that the driver actually supports all adi,adxl34x with x = 0..9, some of which don't exist yet. So, what? When we encounter such devices and decide that we actually want a different driver for them (instead of enhancing the existing one) we'll give them their own compatible string. It's not like adi,adxl348 will automatically match with adi,adxl34x, is it? Please remember that compatible strings shouldn't be decided based on a particular operating system implementation. In this case we can never have generic compatible strings of whatever since in 10 years there might appear an OS that handles them differently. Let's be a bit realistic here as well. I don't agree with you. The generic compatible strings should express compatibility with a hardware interface to the system, not compatibility with particular drivers on particular operating systems. We can thus have generic compatible strings without taking the OS into account. That's one of the reasons. Another one is that the adxl34x driver won't match DT nodes that list the adi,adxl34x compatible value in positions other than the first. Will it match anything else in the position other than 1st (i.e. if device has compatible sting like adi,adxl345-1, adi,adxl345)? Why adi,adxl34x is special? The problem on the driver side is that the automatic I2C DT compatible to device name matching implementation only tries to match with the first compatible string without looking at the other ones. The driver thus needs to be enhanced with an explicit OF match table to be able to match on DT nodes that specify adi,adxl345 or adi,adxl346 as the first compatible entry. Why don't we enhance I2C core instead to do proper matching? That would also be possible, but I believe that patch 1/2 is still the right thing to do, in which case patch 2/2 is required anyway. -- Regards, Laurent Pinchart -- 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 01/11] i2c: add quirk structure to describe adapter flaws
Hi Wolfram,
On Fri, 2015-01-09 at 18:21 +0100, Wolfram Sang wrote:
+ */
+struct i2c_adapter_quirks {
+ u64 flags;
+ int max_num_msgs;
+ u16 max_write_len;
+ u16 max_read_len;
+ u16 max_comb_write_len;
+ u16 max_comb_read_len;
+};
+
+#define I2C_ADAPTER_QUIRK_COMB_WRITE_FIRST BIT(0)
+#define I2C_ADAPTER_QUIRK_COMB_READ_SECOND BIT(1)
+#define I2C_ADAPTER_QUIRK_COMB_WRITE_THEN_READ
(I2C_ADAPTER_QUIRK_COMB_WRITE_FIRST | \
+
I2C_ADAPTER_QUIRK_COMB_READ_SECOND)
+
/*
* i2c_adapter is the structure used to identify a physical i2c bus along
* with the access algorithms necessary to access it.
@@ -472,6 +506,7 @@ struct i2c_adapter {
struct list_head userspace_clients;
struct i2c_bus_recovery_info *bus_recovery_info;
+ struct i2c_adapter_quirks *quirks;
};
#define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
I suggest to add const.
const struct i2c_adapter_quirks *quirks;
also, in i2c-core.c, should modify:
const struct i2c_adapter_quirks *q = adap-quirks;
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] i2c: Don't wait for device release in i2c_del_adapter
Hi Greg, On Jan 16, 2015, at 00:55 , Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Thu, Jan 15, 2015 at 06:25:22PM +0200, Pantelis Antoniou wrote: Hi Greg, On Jan 14, 2015, at 22:41 , Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Wed, Jan 14, 2015 at 07:24:22PM +0200, Pantelis Antoniou wrote: I’ll try to dig around tomorrow and see what the real device reference counts are, but my hunch goes like this: MUX +—- ADAPTER +— DEV. Mux remove method is called, i2c_del_mux_adapter is called on all the channels of the mux, calling in turn i2c_del_adapter which hangs on completion of the dev_released. The call to device_unregister never calls the device_type callback (i2c_adapter_dev_release) because the reference count is not 1 at that point, someone else is having another reference. First of all, my head hurts. Tracking device references ain’t easy. Is there some kind of debugging method you’d recommend for this? You can turn on debugging for kobjects and the driver core if you want to slow down your system log a bunch, but it can be helpful for trickier issues. Or just sprinkle a few printks around. Turning on debugging for kobjects is like opening the floodgates… I managed to make do with a few printks, but some kind of tracking would be helpful. I don't remember the i2c core code at all, that was a long time ago. Bummer. Do you remember code you wrote 12 year ago and haven't looked at for at least 11? Why expect others to as well? :) You’re supposed to be a super-hero maintainer god right? How am I supposed to continue living with my belief system crumbled like that? :) thanks, greg k-h Regards — Pantelis -- 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 1/2] of: Add vendor prefix 'netlogic'
From: Subhendu Sekhar Behera sbeh...@broadcom.com Add vendor name netlogic in vendor-prefixes.txt, which will be used for the Netlogic XLP and XLPII MIPS SoCs. These processors were from NetLogic Microsystems which is now part of Broadcom Corporation. Signed-off-by: Subhendu Sekhar Behera sbeh...@broadcom.com Signed-off-by: Jayachandran C jchan...@broadcom.com --- Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index b1df0ad..ad45908 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -111,6 +111,7 @@ mxicy Macronix International Co., Ltd. national National Semiconductor neonodeNeonode Inc. netgearNETGEAR +netlogic Broadcom Corporation (formerly NetLogic Microsystems) newhaven Newhaven Display International nintendo Nintendo nokia Nokia -- 1.9.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 2/2] i2c: Support for Netlogic XLP9XX/5XX I2C controller.
From: Subhendu Sekhar Behera sbeh...@broadcom.com
Add an I2C bus driver i2c-xlp9xx.c to support the I2C block in the
XLP9xx/XLP5xx MIPS SoC. Update Kconfig and Makefile to add the
CONFIG_I2C_XLP9XX option.
Also add document Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
for DT compatible string netlogic,xlp9xx-i2c.
Signed-off-by: Subhendu Sekhar Behera sbeh...@broadcom.com
Signed-off-by: Jayachandran C jchan...@broadcom.com
---
.../devicetree/bindings/i2c/i2c-xlp9xx.txt | 19 +
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile| 1 +
drivers/i2c/busses/i2c-xlp9xx.c| 447 +
4 files changed, 477 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
create mode 100644 drivers/i2c/busses/i2c-xlp9xx.c
diff --git a/Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
b/Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
new file mode 100644
index ..dca75d1
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-xlp9xx.txt
@@ -0,0 +1,19 @@
+XLP9XX/5XX I2C Bus Controller Driver
+
+Required properties:
+- compatible : Should be netlogic,xlp9xx-i2c.
+- reg: Requires register offset and length.
+- interrupts: Should contain irq number.
+- clock-frequency: I2C clock frequency required.
+
+Example:
+
+i2c0: i2c@113100 {
+ compatible = netlogic,xlp9xx-i2c;
+ #address-cells = 1;
+ #size-cells = 0;
+ reg = 0 0x113100 0x100;
+ clock-frequency = 13300;
+ interrupts = 30;
+ interrupt-parent = pic;
+};
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 31e8308..44ba68e 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -878,6 +878,16 @@ config I2C_XLR
This driver can also be built as a module. If so, the module
will be called i2c-xlr.
+config I2C_XLP9XX
+ tristate XLP9XX I2C support
+ depends on CPU_XLP
+ help
+ This driver enables support for the on-chip I2C interface of
+ the Broadcom XLP9xx/XLP5xx MIPS processors.
+
+ This driver can also be built as a module. If so, the module will
+ be called i2c-xlp9xx.
+
config I2C_RCAR
tristate Renesas R-Car I2C Controller
depends on ARCH_SHMOBILE || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 56388f6..f61a198 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_I2C_WMT) += i2c-wmt.o
obj-$(CONFIG_I2C_OCTEON) += i2c-octeon.o
obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o
obj-$(CONFIG_I2C_XLR) += i2c-xlr.o
+obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
obj-$(CONFIG_I2C_RCAR) += i2c-rcar.o
# External I2C/SMBus adapter drivers
diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
new file mode 100644
index ..8ff1137
--- /dev/null
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -0,0 +1,447 @@
+/*
+ * Copyright (c) 2003-2015 Broadcom Corporation
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed as is without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include linux/kernel.h
+#include linux/slab.h
+#include linux/init.h
+#include linux/module.h
+#include linux/errno.h
+#include linux/io.h
+#include linux/platform_device.h
+#include linux/i2c.h
+#include linux/interrupt.h
+#include linux/completion.h
+
+#define XLP9XX_I2C_DIV 0x0
+#define XLP9XX_I2C_CTRL0x1
+#define XLP9XX_I2C_CMD 0x2
+#define XLP9XX_I2C_STATUS 0x3
+#define XLP9XX_I2C_MTXFIFO 0x4
+#define XLP9XX_I2C_MRXFIFO 0x5
+#define XLP9XX_I2C_MFIFOCTRL 0x6
+#define XLP9XX_I2C_STXFIFO 0x7
+#define XLP9XX_I2C_SRXFIFO 0x8
+#define XLP9XX_I2C_SFIFOCTRL 0x9
+#define XLP9XX_I2C_SLAVEADDR 0xA
+#define XLP9XX_I2C_OWNADDR 0xB
+#define XLP9XX_I2C_FIFOWCNT0xC
+#define XLP9XX_I2C_INTEN 0xD
+#define XLP9XX_I2C_INTST 0xE
+#define XLP9XX_I2C_WAITCNT 0xF
+#define XLP9XX_I2C_TIMEOUT 0X10
+#define XLP9XX_I2C_GENCALLADDR 0x11
+
+#define XLP9XX_I2C_TIMEOUT_MS 1000
+#define XLP9XX_I2C_DEFAULT_FREQ13300UL
+
+#define XLP9XX_I2C_FIFO_SIZE 0x80U
+
+#define XLP9XX_I2C_FIFO_WCNT_MASK 0xff
+#define XLP9XX_I2C_FIFO_CTRL_MASK 0x
+
+#define XLP9XX_I2C_CTRL_MCTLEN_SHIFT 16
+#define XLP9XX_I2C_MFIFOCTRL_HITH_SHIFT8
+#define XLP9XX_I2C_MFIFOCTRL_LOTH_SHIFT0
+#define XLP9XX_I2C_SLAVEADDR_ADDR_SHIFT1
+
+#define XLP9XX_I2C_CTRL_RSTBIT(8)
+#define XLP9XX_I2C_CTRL_EN BIT(6)
+#define XLP9XX_I2C_CTRL_MASTER
Re: [PATCH] [RFC] i2c: Don't wait for device release in i2c_del_adapter
Hi Pantelis, On Thu, 15 Jan 2015 18:25:22 +0200, Pantelis Antoniou wrote: The remove method of the mux gets called and on it’s remove method i2c_del_mux_adapter is called, which in turn calls i2c_del_adapter() which hangs. Anyway, finally figured out what the problem was, a reference was held, and not released properly in the i2c notifier pathc. I’ll send out an updated patch later today. Having that call wait for the other release call to happen is really old, as Jean points out, from 2003. We have fixed sysfs since then to detach the files from the devices easier, we used to have some nasy reference count issues in that area. Perhaps this isn't needed anymore, It’s awfully easy to get a hang with this blocking wait. Turns a memory leak into a hard hang. Turns an invisible memory leak into a visible bug, giving you the opportunity to fix it. You should be grateful ;-) That being said, if this completion serves no other purpose then it can go away, I don't mind. -- Jean Delvare SUSE L3 Support -- 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: Mark instantiated device nodes with OF_POPULATE
Hi Guenter,
On Jan 15, 2015, at 20:55 , Guenter Roeck li...@roeck-us.net wrote:
On Thu, Jan 15, 2015 at 08:33:18PM +0200, Pantelis Antoniou wrote:
Mark (and unmark) device nodes with the POPULATE flag as appropriate.
This is required to avoid multi probing when using I2C and device
overlays containing a mux.
This patch is also more careful with the release of the adapter device
which caused a deadlock with muxes, and does not break the build
on !OF since the node flag accessors are not defined then.
Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
---
drivers/i2c/i2c-core.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 39d25a8..1d44e3a 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1122,6 +1122,10 @@ EXPORT_SYMBOL_GPL(i2c_new_device);
*/
void i2c_unregister_device(struct i2c_client *client)
{
+#if IS_ENABLED(CONFIG_OF_DYNAMIC)
Hi Pantelis,
I thought I read a note somewhere a couple of days ago suggesting that
CONFIG_OF_DYNAMIC would go away soon. Also, of_node_clear_flag is defined
in #ifdef CONFIG_OF, and AFAICS none of the other callers set OF_POPULATED
in the context of CONFIG_OF_DYNAMIC. Given that, wouldn't it be better
to use CONFIG_OF ?
Thanks,
Guenter
Well, I thought about it. Thing is that the notifier is under CONFIG_DYNAMIC,
and it seems it’s natural to be that way.
When we move to always enabling CONFIG_DYNAMIC the change to CONFIG_OF will be
part of the conversion.
Regards
— Pantelis
+if (client-dev.of_node)
+of_node_clear_flag(client-dev.of_node, OF_POPULATED);
+#endif
device_unregister(client-dev);
}
EXPORT_SYMBOL_GPL(i2c_unregister_device);
@@ -1441,8 +1445,11 @@ static void of_i2c_register_devices(struct
i2c_adapter *adap)
dev_dbg(adap-dev, of_i2c: walking child nodes\n);
-for_each_available_child_of_node(adap-dev.of_node, node)
+for_each_available_child_of_node(adap-dev.of_node, node) {
+if (of_node_test_and_set_flag(node, OF_POPULATED))
+continue;
of_i2c_register_device(adap, node);
+}
}
static int of_dev_node_match(struct device *dev, void *data)
@@ -1976,6 +1983,11 @@ static int of_i2c_notify(struct notifier_block *nb,
unsigned long action,
if (adap == NULL)
return NOTIFY_OK; /* not for us */
+if (of_node_test_and_set_flag(rd-dn, OF_POPULATED)) {
+put_device(adap-dev);
+return NOTIFY_OK;
+}
+
client = of_i2c_register_device(adap, rd-dn);
put_device(adap-dev);
@@ -1986,6 +1998,10 @@ static int of_i2c_notify(struct notifier_block *nb,
unsigned long action,
}
break;
case OF_RECONFIG_CHANGE_REMOVE:
+/* already depopulated? */
+if (!of_node_check_flag(rd-dn, OF_POPULATED))
+return NOTIFY_OK;
+
/* find our device by node */
client = of_find_i2c_device_by_node(rd-dn);
if (client == NULL)
--
1.7.12
--
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 V3 1/1] iio: Added Capella cm3232 ambient light sensor driver.
CM3232 is an advanced ambient light sensor with I2C protocol interface. The I2C slave address is internally hardwired as 0x10 (7-bit). Writing to configure register is byte mode, but reading ALS register requests to use word mode for 16-bit resolution. Signed-off-by: Kevin Tsai kt...@capellamicro.com --- v3: Added hw_id to als_info structure. Removed unused include files. Modified cm3232_write_als_it() to handle register update fail. Thanks comments from Daniel Baluta. v2: Removed unused CM3232_CMD_ALS_HS. Modified cm3232_als_info structure. Removed id field. Modified cm3232_chip structure. Merged CM3232_als_it_bits and CM3232_als_it_values to cm3232_it_scale. Removed mutex lock. Renamed als_raw to regs_als. Moved it to cm3232_chip structure. Modified cm3232_read_als_it() and cm3232_write_als_it() to support val2. Thanks comments from Jeremiah Mahler, Peter Meerwald, Daniel Baluta, and Joe Perches. v1: Added cm3232.c to support Capella Microsystems CM3232 Ambient Light Sensor. .../devicetree/bindings/i2c/trivial-devices.txt| 1 + MAINTAINERS| 6 + drivers/iio/light/Kconfig | 11 + drivers/iio/light/Makefile | 1 + drivers/iio/light/cm3232.c | 409 + 5 files changed, 428 insertions(+) create mode 100644 drivers/iio/light/cm3232.c diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt index 9f4e382..572a7c4 100644 --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt @@ -34,6 +34,7 @@ atmel,24c512 i2c serial eeprom (24cxx) atmel,24c1024 i2c serial eeprom (24cxx) atmel,at97sc3204t i2c trusted platform module (TPM) capella,cm32181CM32181: Ambient Light Sensor +capella,cm3232 CM3232: Ambient Light Sensor catalyst,24c32 i2c serial eeprom cirrus,cs42l51 Cirrus Logic CS42L51 audio codec dallas,ds1307 64 x 8, Serial, I2C Real-Time Clock diff --git a/MAINTAINERS b/MAINTAINERS index ddb9ac8..06a613a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2378,6 +2378,12 @@ F: security/capability.c F: security/commoncap.c F: kernel/capability.c +CAPELLA MICROSYSTEMS LIGHT SENSOR DRIVER +M: Kevin Tsai kt...@capellamicro.com +S: Maintained +F: drivers/iio/light/cm* +F: Documentation/devicetree/bindings/i2c/trivial-devices.txt + CC2520 IEEE-802.15.4 RADIO DRIVER M: Varka Bhadram varkabhad...@gmail.com L: linux-w...@vger.kernel.org diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig index 5bea821..cd5028e 100644 --- a/drivers/iio/light/Kconfig +++ b/drivers/iio/light/Kconfig @@ -48,6 +48,17 @@ config CM32181 To compile this driver as a module, choose M here: the module will be called cm32181. +config CM3232 + depends on I2C + tristate CM3232 ambient light sensor + help +Say Y here if you use cm3232. +This option enables ambient light sensor using +Capella Microsystems cm3232 device driver. + +To compile this driver as a module, choose M here: +the module will be called cm3232. + config CM36651 depends on I2C tristate CM36651 driver diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile index 47877a3..f2c8d55 100644 --- a/drivers/iio/light/Makefile +++ b/drivers/iio/light/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_ADJD_S311) += adjd_s311.o obj-$(CONFIG_AL3320A) += al3320a.o obj-$(CONFIG_APDS9300) += apds9300.o obj-$(CONFIG_CM32181) += cm32181.o +obj-$(CONFIG_CM3232) += cm3232.o obj-$(CONFIG_CM36651) += cm36651.o obj-$(CONFIG_GP2AP020A00F) += gp2ap020a00f.o obj-$(CONFIG_HID_SENSOR_ALS) += hid-sensor-als.o diff --git a/drivers/iio/light/cm3232.c b/drivers/iio/light/cm3232.c new file mode 100644 index 000..8573f98 --- /dev/null +++ b/drivers/iio/light/cm3232.c @@ -0,0 +1,409 @@ +/* + * CM3232 Ambient Light Sensor + * + * Copyright (C) 2014-2015 Capella Microsystems Inc. + * Author: Kevin Tsai kt...@capellamicro.com + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2, as published + * by the Free Software Foundation. + * + * IIO driver for CM3232 (7-bit I2C slave address 0x10). + * + */ + +#include linux/i2c.h +#include linux/module.h +#include linux/iio/iio.h +#include linux/iio/sysfs.h +#include linux/init.h + +/* Registers Address */ +#define CM3232_REG_ADDR_CMD0x00 +#define CM3232_REG_ADDR_ALS0x50 +#define CM3232_REG_ADDR_ID 0x53 + +#define CM3232_CMD_ALS_DISABLE BIT(0) + +#define CM3232_CMD_ALS_IT_SHIFT2 +#define CM3232_CMD_ALS_IT_MASK (BIT(2) | BIT(3) | BIT(4))
Re: [PATCH v4 2/3] i2c: iproc: Add Broadcom iProc I2C Driver
Hello,
On Wed, Jan 14, 2015 at 02:23:32PM -0800, Ray Jui wrote:
+#include linux/device.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/sched.h
+#include linux/i2c.h
+#include linux/interrupt.h
+#include linux/platform_device.h
+#include linux/clk.h
+#include linux/io.h
+#include linux/slab.h
+#include linux/delay.h
some of them are not needed. I tested on amd64 and efm32 and could drop
linux/device.h, linux/sched.h, linux/clk.h. (BTW, I wonder that you
don't need clk handling.)
+#define TIM_CFG_OFFSET 0x04
+#define TIME_CFG_MODE_400_SHIFT 31
Is the register name and the bit name prefix really different or is this
a typo?
+static int __wait_for_bus_idle(struct bcm_iproc_i2c_dev *iproc_i2c)
A bcm_iproc_i2c prefix would be nice here.
+static int bcm_iproc_i2c_format_addr(struct bcm_iproc_i2c_dev *iproc_i2c,
+ struct i2c_msg *msg, u8 *addr)
+{
+
+ if (msg-flags I2C_M_TEN) {
+ dev_err(iproc_i2c-device, no support for 10-bit address\n);
+ return -EINVAL;
+ }
+
+ *addr = (msg-addr 1);
You can also drop the parentheses.
+ switch (val) {
+ case M_CMD_STATUS_SUCCESS:
+ return 0;
+
+ case M_CMD_STATUS_LOST_ARB:
+ dev_err(iproc_i2c-device, lost bus arbitration\n);
+ return -EREMOTEIO;
+
+ case M_CMD_STATUS_NACK_ADDR:
+ dev_err(iproc_i2c-device, NAK addr:0x%02x\n,
+ iproc_i2c-msg-addr);
+ return -EREMOTEIO;
+
+ case M_CMD_STATUS_NACK_DATA:
+ dev_err(iproc_i2c-device, NAK data\n);
+ return -EREMOTEIO;
+
+ case M_CMD_STATUS_TIMEOUT:
+ dev_err(iproc_i2c-device, bus timeout\n);
+ return -ETIMEDOUT;
+
+ default:
+ dev_err(iproc_i2c-device, unknown error code=%d\n, val);
+ return -EREMOTEIO;
+ }
+
+ return -EREMOTEIO;
This is not reached.
+}
+
+static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
+ struct i2c_msg *msg)
+{
+ int ret, i;
+ u8 addr;
+ u32 val;
+ unsigned long time_left = msecs_to_jiffies(I2C_TIMEOUT_MESC);
+
+ if (msg-len 1 || msg-len M_TX_RX_FIFO_SIZE - 1) {
Is the 1 a hardware or a software limitation? That means your driver
doesn't support I2C_SMBUS_QUICK which is used for example by i2cdetect.
+ dev_err(iproc_i2c-device,
+ supported data length is 1 - %u bytes\n,
+ M_TX_RX_FIFO_SIZE - 1);
+ return -EINVAL;
+ }
+
+ iproc_i2c-msg = msg;
Can it happen that iproc_i2c-msg still holds an uncompleted message
here or is this serialized by the core? Wolfram? Either here something
like:
if (iproc_i2c-msg)
return -EBUSY;
and
iproc_i2c-msg = NULL;
when a transfer is completed is needed, or the respective code can be
dropped from other drivers (e.g. i2c-efm32).
On the other hand .msg is only used in bcm_iproc_i2c_check_status() to
give a diagnostic message. Maybe you can drop .msg and instead give it
as an additional parameter to bcm_iproc_i2c_check_status().
+ ret = __wait_for_bus_idle(iproc_i2c);
+ if (ret)
+ return ret;
I would still prefer to have something like:
if (bcm_iproc_i2c_bus_busy())
return -EBUSY;
instead of a tight loop here.
+ ret = bcm_iproc_i2c_format_addr(iproc_i2c, msg, addr);
+ if (ret)
+ return ret;
+
+ /* load slave address into the TX FIFO */
+ writel(addr, iproc_i2c-base + M_TX_OFFSET);
+
+ /* for a write transaction, load data into the TX FIFO */
+ if (!(msg-flags I2C_M_RD)) {
+ for (i = 0; i msg-len; i++) {
+ val = msg-buf[i];
+
+ /* mark the last byte */
+ if (i == msg-len - 1)
+ val |= 1 M_TX_WR_STATUS_SHIFT;
What happens if you don't mark this last byte? Could this be used to
support transfers bigger than the fifo size?
+ /*
+ * Enable the start busy interrupt, which will be triggered after
+ * the transaction is done, i.e., the internal start_busy bit
s/\.,/./ I think
+ * transitions from 1 to 0
s/$/./
+ */
+ writel(1 IE_M_START_BUSY_SHIFT, iproc_i2c-base + IE_OFFSET);
+
+ /*
+ * Now we can activate the transfer. For a read operation, specify the
+ * number of bytes to read
s/$/./
+ */
+ val = 1 M_CMD_START_BUSY_SHIFT;
+ if (msg-flags I2C_M_RD) {
+ val |= (M_CMD_PROTOCOL_BLK_RD M_CMD_PROTOCOL_SHIFT) |
+(msg-len M_CMD_RD_CNT_SHIFT);
+ } else {
+ val |= (M_CMD_PROTOCOL_BLK_WR M_CMD_PROTOCOL_SHIFT);
+ }
+ writel(val, iproc_i2c-base + M_CMD_OFFSET);
+
+ time_left =
Re: [PATCH v4 3/3] ARM: dts: add I2C device nodes for Broadcom Cygnus
Hello,
On Wed, Jan 14, 2015 at 02:23:33PM -0800, Ray Jui wrote:
Add I2C device nodes and its properties in bcm-cygnus.dtsi but keep
them disabled there. Individual I2C devices can be enabled in board
specific dts file when I2C slave devices are enabled in the future
s/$/./
Signed-off-by: Ray Jui r...@broadcom.com
Reviewed-by: Scott Branden sbran...@broadcom.com
---
arch/arm/boot/dts/bcm-cygnus.dtsi | 20
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi
b/arch/arm/boot/dts/bcm-cygnus.dtsi
index 5126f9e..f7d6c1d 100644
--- a/arch/arm/boot/dts/bcm-cygnus.dtsi
+++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
@@ -70,6 +70,26 @@
};
};
+ i2c0: i2c@18008000 {
+ compatible = brcm,iproc-i2c;
in patch 2 you wrote the driver is for a family of SoCs, right? Then I'd
make this:
compatible = brcm,$mysoc-iproc-i2c, brcm,iproc-i2c;
(or maybe s/$mysoc-iproc-i2c/$mysoc-i2c/).
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
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
[PATCH V4 0/2] i2c-designware: Add Intel Baytrail pmic i2c bus support
V4: - Added bus lock to i2c_dw_init() as suggested by Jarrko Nikula jarkko.nik...@linux.intel.com. Addresses infrequent hang that was seen occuring during probe. - Added label in i2c_dw_xfer() to bypass unnecessary lock release when acquire fails. Suggested by Mika. - Changed make to build driver as composite addition to i2c-designware-platform. Driver can be module or built-in following the platform driver selection. Tested as both. - Added EPROBE_DEFER return to baytrail definition of i2c_dw_eval_lock() to defer probe if iosf_mbi driver (required for acquring bus lock) is not available. V3: - Split lock support and driver into separate patches - Change module build to bool. Platforms running without this driver cannot perform critical functions such as charging. Furthermore attempts by other drivers to access the i2c bus without a lock will hang the platform. - Replaced has_hw_lock flag with acquire/release function pointers. - Replaced acquire/release ifdef code with single i2c_dw_eval_lock_support() test for cleaner (if still undesirable) compile time scalability. Future Intel platforms will however continue to use the Baytrail driver. V2: - Moved semaphore detection out of dw platform driver - Replaced function pointers with defined acquire/release functions in dw core. This helps eliminate the ifdefery in the dw platform driver. - Use new has_hw_lock flag to check if the lock exists on a given bus. - Use new pm_runtime_disabled flag to conditionally turnoff runtime pm in the dw platform driver. David E. Box (2): i2c-designware: Add i2c bus locking support i2c-designware: Add Intel Baytrail PMIC I2C bus support drivers/i2c/busses/Kconfig | 11 ++ drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-designware-baytrail.c | 160 +++ drivers/i2c/busses/i2c-designware-core.c | 26 + drivers/i2c/busses/i2c-designware-core.h | 12 ++ drivers/i2c/busses/i2c-designware-platdrv.c | 20 +++- 6 files changed, 225 insertions(+), 5 deletions(-) create mode 100644 drivers/i2c/busses/i2c-designware-baytrail.c -- 1.9.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 V4 2/2] i2c-designware: Add Intel Baytrail PMIC I2C bus support
This patch implements an I2C bus sharing mechanism between the host and platform
hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 PMIC.
On these platforms access to the PMIC must be shared with platform hardware. The
hardware unit assumes full control of the I2C bus and the host must request
access through a special semaphore. Hardware control of the bus also makes it
necessary to disable runtime pm to avoid interfering with hardware transactions.
Signed-off-by: David E. Box david.e@linux.intel.com
---
drivers/i2c/busses/Kconfig | 11 ++
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-designware-baytrail.c | 160 +++
drivers/i2c/busses/i2c-designware-core.h | 6 +
drivers/i2c/busses/i2c-designware-platdrv.c | 20 +++-
5 files changed, 193 insertions(+), 5 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-designware-baytrail.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 917c358..9a83c46 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -464,6 +464,17 @@ config I2C_DESIGNWARE_PCI
This driver can also be built as a module. If so, the module
will be called i2c-designware-pci.
+config I2C_DESIGNWARE_BAYTRAIL
+ bool Intel Baytrail I2C semaphore support
+ depends on I2C_DESIGNWARE_PLATFORM
+ select IOSF_MBI
+ help
+ This driver enables managed host access to the PMIC I2C bus on select
+ Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows
+ the host to request uninterrupted access to the PMIC's I2C bus from
+ the platform firmware controlling it. You should say Y if running on
+ a BayTrail system using the AXP288.
+
config I2C_EFM32
tristate EFM32 I2C controller
depends on ARCH_EFM32 || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 78d56c5..15b2965 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_I2C_DAVINCI) += i2c-davinci.o
obj-$(CONFIG_I2C_DESIGNWARE_CORE) += i2c-designware-core.o
obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c-designware-platform.o
i2c-designware-platform-objs := i2c-designware-platdrv.o
+i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_BAYTRAIL) +=
i2c-designware-baytrail.o
obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o
i2c-designware-pci-objs := i2c-designware-pcidrv.o
obj-$(CONFIG_I2C_EFM32)+= i2c-efm32.o
diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c
b/drivers/i2c/busses/i2c-designware-baytrail.c
new file mode 100644
index 000..5f1ff4c
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -0,0 +1,160 @@
+/*
+ * Intel BayTrail PMIC I2C bus semaphore implementaion
+ * Copyright (c) 2014, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+#include linux/module.h
+#include linux/delay.h
+#include linux/device.h
+#include linux/acpi.h
+#include linux/i2c.h
+#include linux/interrupt.h
+#include asm/iosf_mbi.h
+#include i2c-designware-core.h
+
+#define SEMAPHORE_TIMEOUT 100
+#define PUNIT_SEMAPHORE0x7
+
+static unsigned long acquired;
+
+static int get_sem(struct device *dev, u32 *sem)
+{
+ u32 reg_val;
+ int ret;
+
+ ret = iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ, PUNIT_SEMAPHORE,
+ reg_val);
+ if (ret) {
+ dev_err(dev, iosf failed to read punit semaphore\n);
+ return ret;
+ }
+
+ *sem = reg_val 0x1;
+
+ return 0;
+}
+
+static void reset_semaphore(struct device *dev)
+{
+ u32 data;
+
+ if (iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ,
+ PUNIT_SEMAPHORE, data)) {
+ dev_err(dev, iosf failed to reset punit semaphore during
read\n);
+ return;
+ }
+
+ data = data 0xfffe;
+ if (iosf_mbi_write(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_WRITE,
+PUNIT_SEMAPHORE, data))
+ dev_err(dev, iosf failed to reset punit semaphore during
write\n);
+}
+
+int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
+{
+ u32 sem = 0;
+ int ret;
+ unsigned long start, end;
+
+ if (!dev || !dev-dev)
+ return -ENODEV;
+
+ if (!dev-acquire_lock)
+ return 0;
+
+ /* host driver writes 0x2 to side band semaphore register */
+
[PATCH V4 1/2] i2c-designware: Add i2c bus locking support
Adds support for acquiring and releasing a hardware bus lock in the i2c
designware core transfer function. This is needed for i2c bus controllers
that are shared with but not controlled by the kernel.
Signed-off-by: David E. Box david.e@linux.intel.com
---
drivers/i2c/busses/i2c-designware-core.c | 26 ++
drivers/i2c/busses/i2c-designware-core.h | 6 ++
2 files changed, 32 insertions(+)
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index 3c20e4b..3d3ac4d 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -289,6 +289,15 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
u32 hcnt, lcnt;
u32 reg;
u32 sda_falling_time, scl_falling_time;
+ int ret;
+
+ if (dev-acquire_lock) {
+ ret = dev-acquire_lock(dev);
+ if (ret) {
+ dev_err(dev-dev, couldn't acquire bus ownership\n);
+ return ret;
+ }
+ }
input_clock_khz = dev-get_clk_rate_khz(dev);
@@ -302,6 +311,8 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
} else if (reg != DW_IC_COMP_TYPE_VALUE) {
dev_err(dev-dev, Unknown Synopsys component type:
0x%08x\n, reg);
+ if (dev-release_lock)
+ dev-release_lock(dev);
return -ENODEV;
}
@@ -368,6 +379,9 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
/* configure the i2c master */
dw_writel(dev, dev-master_cfg , DW_IC_CON);
+
+ if (dev-release_lock)
+ dev-release_lock(dev);
return 0;
}
EXPORT_SYMBOL_GPL(i2c_dw_init);
@@ -631,6 +645,14 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
dev-abort_source = 0;
dev-rx_outstanding = 0;
+ if (dev-acquire_lock) {
+ ret = dev-acquire_lock(dev);
+ if (ret) {
+ dev_err(dev-dev, couldn't acquire bus ownership\n);
+ goto done_nolock;
+ }
+ }
+
ret = i2c_dw_wait_bus_not_busy(dev);
if (ret 0)
goto done;
@@ -676,6 +698,10 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
ret = -EIO;
done:
+ if (dev-release_lock)
+ dev-release_lock(dev);
+
+done_nolock:
pm_runtime_mark_last_busy(dev-dev);
pm_runtime_put_autosuspend(dev-dev);
mutex_unlock(dev-lock);
diff --git a/drivers/i2c/busses/i2c-designware-core.h
b/drivers/i2c/busses/i2c-designware-core.h
index d66b6cb..a472c91 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -65,6 +65,9 @@
* @ss_lcnt: standard speed LCNT value
* @fs_hcnt: fast speed HCNT value
* @fs_lcnt: fast speed LCNT value
+ * @acquire_lock: function to acquire a hardware lock on the bus
+ * @release_lock: function to release a hardware lock on the bus
+ * @pm_runtime_disabled: true if pm runtime is disabled
*
* HCNT and LCNT parameters can be used if the platform knows more accurate
* values than the one computed based only on the input clock frequency.
@@ -105,6 +108,9 @@ struct dw_i2c_dev {
u16 ss_lcnt;
u16 fs_hcnt;
u16 fs_lcnt;
+ int (*acquire_lock)(struct dw_i2c_dev *dev);
+ void(*release_lock)(struct dw_i2c_dev *dev);
+ boolpm_runtime_disabled;
};
#define ACCESS_SWAP0x0001
--
1.9.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] input: adxl34x: Add OF match support
On Thu, Dec 18, 2014 at 02:49:28PM +0200, Laurent Pinchart wrote: Hi Wolfram, On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote: On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote: The I2C subsystem can match devices without explicit OF support based on the part of their compatible property after the comma. However, this mechanism uses the first compatible value only. For adxl34x OF device nodes the compatible property should list the more specific adi,adxl345 or adi,adxl346 value first and the adi,adxl34x fallback value second. This prevents the device node from being matched with the adxl34x driver. Fix this by adding an OF match table with an adi,adxl34x compatible entry. Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com --- drivers/input/misc/adxl34x-i2c.c | 11 +++ 1 file changed, 11 insertions(+) Another option would have been to add adxl325 and adxl326 entries to the adxl34x_id I2C match table, but it would have had the drawback of requiring a driver update for every new device. AFAIK this is even required for compatible entries, to be as specific as possible. I think this makes sense. With platform_ids, we already had the problem that pca954x was too generic and was used for both GPIO extenders and I2C muxers (IIRC). There are three compatible strings defined for the ADXL345 and ADXL346 in Documentation/devicetree/bindings/i2c/trivial-devices.txt: adi,adxl345, adi,adxl346, adi,adxl34x. Given that the last one is a fallback for the first two I don't see a need to add the specific compatible strings to the driver for now. If a new totally incompatible chip named ADXL347 comes out we will need a new driver which won't be allowed to use the adi,adxl34x compatible string. Been there, got bitten. We only found out too late, because one driver was in i2c and the other in GPIO (or LED even?), both using 953x :( An option would be to remove adi,adxl34x from Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case the driver should match explicitly on adi,adxl345 and adi,adxl346. That might clash with the DT ABI stability requirements though. I do prefer this: 1) add specific compatible values to the driver. We do those updates for new devices all the time 2) also add 34x as a compatible but mark it as deprecateed 3) delete 34x from trivial devices Everyone OK with that? Thanks, Wolfram signature.asc Description: Digital signature
Re: [PATCH v3 2/3] i2c: iproc: Add Broadcom iProc I2C Driver
+ case M_CMD_STATUS_LOST_ARB: + dev_err(dev-device, lost bus arbitration\n); I wouldn't dev_err that, only dev_dbg. I'm not sure how usual the errors for the next two cases is, maybe degrade them to dev_dbg, too? These errors are rare, and it's nice to keep them at the dev_err level so the user will be more aware. This is wrong. Arbitration lost and NACK is pretty standard stuff on an I2C bus. User doesn't need to know about it, it is just noise in the logs. Timeout is different, you can report that (although I should probably move such a message into the core). Please also use the proper errno codes defined in Documentation/i2c/fault-codes. They should be distinct enough to drop the messages. + return -EREMOTEIO; + + case M_CMD_STATUS_NACK_ADDR: + dev_err(dev-device, NAK addr:0x%02x\n, dev-msg-addr); + return -EREMOTEIO; + + case M_CMD_STATUS_NACK_DATA: + dev_err(dev-device, NAK data\n); + return -EREMOTEIO; + + case M_CMD_STATUS_TIMEOUT: + dev_err(dev-device, bus timeout\n); + return -ETIMEDOUT; + + default: + dev_err(dev-device, unknown error code=%d\n, val); + return -EREMOTEIO; + } + + return -EREMOTEIO; +} signature.asc Description: Digital signature
Re: [PATCH v4 2/3] i2c: iproc: Add Broadcom iProc I2C Driver
+ iproc_i2c-msg = msg;
Can it happen that iproc_i2c-msg still holds an uncompleted message
here or is this serialized by the core? Wolfram? Either here something
We have per-adapter locks serializing transfers, if you mean that?
+static int bcm_iproc_i2c_cfg_speed(struct bcm_iproc_i2c_dev *iproc_i2c)
+{
+ unsigned int bus_speed, speed_bit;
+ u32 val;
+ int ret = of_property_read_u32(iproc_i2c-device-of_node,
+ clock-frequency, bus_speed);
+ if (ret 0) {
+ dev_err(iproc_i2c-device,
+ missing clock-frequency property\n);
+ return -ENODEV;
Is a missing property the only situation where of_property_read_u32
returns an error? Would it be sane to default to 100 kHz?
Default of 100kHz instead of -ENODEV sounds very reasonable.
+static int bcm_iproc_i2c_remove(struct platform_device *pdev)
+{
+ struct bcm_iproc_i2c_dev *iproc_i2c = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(iproc_i2c-adapter);
You need to free the irq before i2c_del_adapter.
One could also keep using devm_request_irq and disable all interrupts
sources here?
Thanks for the reviews, Uwe!
Wolfram
signature.asc
Description: Digital signature
Re: [PATCH v2] of: i2c: Add idle-disconnect DT property to PCA954x mux driver
On Fri, Dec 19, 2014 at 06:00:10PM +0100, Alexander Sverdlin wrote: of: i2c: Add idle-disconnect DT property to PCA954x mux driver Add idle-disconnect device tree property to PCA954x mux driver. The new property forces the multiplexer to disconnect child buses in idle state. This is used, for example, when there are several multiplexers on the same bus and the devices on the underlying buses might have same I2C addresses. Basically OK. Question to DT maintainers: idle-disconnect, i2c-mux-idle-disconnect, or is there another existing binding we could use? At the same time old (and not used in the tree) platform data binding deselect_on_exit is removed to simplify the implementation. Old binding has different (per-channel) semantics and doesn't fit well in the new concept. I'd prefer to keep it. It should be only one || more. It is not really in the way IMO. Thanks, Wolfram signature.asc Description: Digital signature
Re: [PATCH v2] of: i2c: Add idle-disconnect DT property to PCA954x mux driver
On Thu, Jan 15, 2015 at 02:09:09PM +0100, Alexander Sverdlin wrote: Hi Wolfram! On 15/01/15 13:32, ext Wolfram Sang wrote: On Fri, Dec 19, 2014 at 06:00:10PM +0100, Alexander Sverdlin wrote: of: i2c: Add idle-disconnect DT property to PCA954x mux driver Add idle-disconnect device tree property to PCA954x mux driver. The new property forces the multiplexer to disconnect child buses in idle state. This is used, for example, when there are several multiplexers on the same bus and the devices on the underlying buses might have same I2C addresses. Basically OK. Question to DT maintainers: idle-disconnect, i2c-mux-idle-disconnect, or is there another existing binding we could use? At the same time old (and not used in the tree) platform data binding deselect_on_exit is removed to simplify the implementation. Old binding has different (per-channel) semantics and doesn't fit well in the new concept. I'd prefer to keep it. It should be only one || more. It is not really in the way IMO. It complicates the implementation 3x times :) This is part of our discussion with Laurent: Does it? I don't want DT and platform_data to behave equally. I just want to keep being backwards compatible. So, I'd suggest: (pdata pdata-modes[num].deselect_on_exit) || idle_disconnect ? pca954x_deselect_mux : NULL); I'm not keen to brake out-of-tree code (if any), but may be it will be decided to drop this per-channel deselect_on_exit, because it's not used at least in the kernel tree... I couldn't find a user of the platform_data, at all. But removing platform_data support is a seperate patch, and deprecating platform_data is a seperate and general issue IMO. signature.asc Description: Digital signature
Re: [PATCH] input: adxl34x: Add OF match support
On Thu, Jan 15, 2015 at 1:53 PM, Wolfram Sang w...@the-dreams.de wrote:
There are three compatible strings defined for the ADXL345 and ADXL346 in
Documentation/devicetree/bindings/i2c/trivial-devices.txt: adi,adxl345,
adi,adxl346, adi,adxl34x. Given that the last one is a fallback for the
first two I don't see a need to add the specific compatible strings to the
driver for now. If a new totally incompatible chip named ADXL347 comes out we
will need a new driver which won't be allowed to use the adi,adxl34x
compatible string.
Been there, got bitten. We only found out too late, because one driver
was in i2c and the other in GPIO (or LED even?), both using 953x :(
Yep, pca953x = leds-pca953x
An option would be to remove adi,adxl34x from
Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case the
driver should match explicitly on adi,adxl345 and adi,adxl346. That might
clash with the DT ABI stability requirements though.
I do prefer this:
1) add specific compatible values to the driver. We do those updates for
new devices all the time
2) also add 34x as a compatible but mark it as deprecateed
3) delete 34x from trivial devices
Everyone OK with that?
Sounds fine to me. Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
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 v2] of: i2c: Add idle-disconnect DT property to PCA954x mux driver
Hi Wolfram! On 15/01/15 13:32, ext Wolfram Sang wrote: On Fri, Dec 19, 2014 at 06:00:10PM +0100, Alexander Sverdlin wrote: of: i2c: Add idle-disconnect DT property to PCA954x mux driver Add idle-disconnect device tree property to PCA954x mux driver. The new property forces the multiplexer to disconnect child buses in idle state. This is used, for example, when there are several multiplexers on the same bus and the devices on the underlying buses might have same I2C addresses. Basically OK. Question to DT maintainers: idle-disconnect, i2c-mux-idle-disconnect, or is there another existing binding we could use? At the same time old (and not used in the tree) platform data binding deselect_on_exit is removed to simplify the implementation. Old binding has different (per-channel) semantics and doesn't fit well in the new concept. I'd prefer to keep it. It should be only one || more. It is not really in the way IMO. It complicates the implementation 3x times :) This is part of our discussion with Laurent: I would copy pdata-modes[chan].deselect_on_exit to data-idle_disconnect in the probe function, so you could avoiding accessing pdata here. Unfortunately, this pdata has different (per-channel) semantics. I cannot really understand, why it was done this way, but anyway it's not possible to use one global bit to represent per-channel bits without changing the behavior. I'm not keen to brake out-of-tree code (if any), but may be it will be decided to drop this per-channel deselect_on_exit, because it's not used at least in the kernel tree... I'd vote for removing deselect_on_exit from platform data, but I won't insist. -- Best regards, Alexander Sverdlin. -- 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 v2] of: i2c: Add idle-disconnect DT property to PCA954x mux driver
Hi Wolfram, On 15/01/15 14:19, ext Wolfram Sang wrote: of: i2c: Add idle-disconnect DT property to PCA954x mux driver Add idle-disconnect device tree property to PCA954x mux driver. The new property forces the multiplexer to disconnect child buses in idle state. This is used, for example, when there are several multiplexers on the same bus and the devices on the underlying buses might have same I2C addresses. Basically OK. Question to DT maintainers: idle-disconnect, i2c-mux-idle-disconnect, or is there another existing binding we could use? At the same time old (and not used in the tree) platform data binding deselect_on_exit is removed to simplify the implementation. Old binding has different (per-channel) semantics and doesn't fit well in the new concept. I'd prefer to keep it. It should be only one || more. It is not really in the way IMO. It complicates the implementation 3x times :) This is part of our discussion with Laurent: Does it? I don't want DT and platform_data to behave equally. I just want to keep being backwards compatible. So, I'd suggest: (pdata pdata-modes[num].deselect_on_exit) || idle_disconnect ? pca954x_deselect_mux : NULL); you are right, thanks for the hint :) I'll prepare v3... -- Best regards, Alexander Sverdlin. -- 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] input: adxl34x: Add OF match support
Hi Wolfram, On Thursday 15 January 2015 13:53:22 Wolfram Sang wrote: On Thu, Dec 18, 2014 at 02:49:28PM +0200, Laurent Pinchart wrote: On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote: On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote: The I2C subsystem can match devices without explicit OF support based on the part of their compatible property after the comma. However, this mechanism uses the first compatible value only. For adxl34x OF device nodes the compatible property should list the more specific adi,adxl345 or adi,adxl346 value first and the adi,adxl34x fallback value second. This prevents the device node from being matched with the adxl34x driver. Fix this by adding an OF match table with an adi,adxl34x compatible entry. Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com --- drivers/input/misc/adxl34x-i2c.c | 11 +++ 1 file changed, 11 insertions(+) Another option would have been to add adxl325 and adxl326 entries to the adxl34x_id I2C match table, but it would have had the drawback of requiring a driver update for every new device. AFAIK this is even required for compatible entries, to be as specific as possible. I think this makes sense. With platform_ids, we already had the problem that pca954x was too generic and was used for both GPIO extenders and I2C muxers (IIRC). There are three compatible strings defined for the ADXL345 and ADXL346 in Documentation/devicetree/bindings/i2c/trivial-devices.txt: adi,adxl345, adi,adxl346, adi,adxl34x. Given that the last one is a fallback for the first two I don't see a need to add the specific compatible strings to the driver for now. If a new totally incompatible chip named ADXL347 comes out we will need a new driver which won't be allowed to use the adi,adxl34x compatible string. Been there, got bitten. We only found out too late, because one driver was in i2c and the other in GPIO (or LED even?), both using 953x :( That seems like a development, review and/or merge process failure to me, I wouldn't avoid generic compatible strings for that reason only. An option would be to remove adi,adxl34x from Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case the driver should match explicitly on adi,adxl345 and adi,adxl346. That might clash with the DT ABI stability requirements though. I do prefer this: 1) add specific compatible values to the driver. We do those updates for new devices all the time Do you mean OF compatible values, or I2C match table entries ? I assume OF compatible values. As the ADXL346 is backward-compatible with the ADXL345, and as the driver doesn't support the ADXL346-specific features, how about adding only the adxl345 for now, and using compatible = adi,adxl346, adi,adxl345; for the ADXL346 ? 2) also add 34x as a compatible but mark it as deprecateed 3) delete 34x from trivial devices OK. Everyone OK with that? -- Regards, Laurent Pinchart -- 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 v2 2/2] input: adxl34x: Add OF match support
On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov
dmitry.torok...@gmail.com wrote:
I still do not understand what we are trying to fix here. Why is
adi,adxl34x compatible string no good anymore? If we start using exact
models and the physical device does not match do we abort probe? What is
the problem that we are solving here?
Because there's no guarantee that the driver actually supports all
adi,adxl34x with x = 0..9, some of which don't exist yet.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
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] input: adxl34x: Add OF match support
On Thursday 15 January 2015 16:19:19 Laurent Pinchart wrote: On Thursday 15 January 2015 13:53:22 Wolfram Sang wrote: On Thu, Dec 18, 2014 at 02:49:28PM +0200, Laurent Pinchart wrote: On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote: On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote: The I2C subsystem can match devices without explicit OF support based on the part of their compatible property after the comma. However, this mechanism uses the first compatible value only. For adxl34x OF device nodes the compatible property should list the more specific adi,adxl345 or adi,adxl346 value first and the adi,adxl34x fallback value second. This prevents the device node from being matched with the adxl34x driver. Fix this by adding an OF match table with an adi,adxl34x compatible entry. Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com --- drivers/input/misc/adxl34x-i2c.c | 11 +++ 1 file changed, 11 insertions(+) Another option would have been to add adxl325 and adxl326 entries to the adxl34x_id I2C match table, but it would have had the drawback of requiring a driver update for every new device. AFAIK this is even required for compatible entries, to be as specific as possible. I think this makes sense. With platform_ids, we already had the problem that pca954x was too generic and was used for both GPIO extenders and I2C muxers (IIRC). There are three compatible strings defined for the ADXL345 and ADXL346 in Documentation/devicetree/bindings/i2c/trivial-devices.txt: adi,adxl345, adi,adxl346, adi,adxl34x. Given that the last one is a fallback for the first two I don't see a need to add the specific compatible strings to the driver for now. If a new totally incompatible chip named ADXL347 comes out we will need a new driver which won't be allowed to use the adi,adxl34x compatible string. Been there, got bitten. We only found out too late, because one driver was in i2c and the other in GPIO (or LED even?), both using 953x :( That seems like a development, review and/or merge process failure to me, I wouldn't avoid generic compatible strings for that reason only. An option would be to remove adi,adxl34x from Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case the driver should match explicitly on adi,adxl345 and adi,adxl346. That might clash with the DT ABI stability requirements though. I do prefer this: 1) add specific compatible values to the driver. We do those updates for new devices all the time Do you mean OF compatible values, or I2C match table entries ? I assume OF compatible values. As the ADXL346 is backward-compatible with the ADXL345, and as the driver doesn't support the ADXL346-specific features, how about adding only the adxl345 for now, and using compatible = adi,adxl346, adi,adxl345; for the ADXL346 ? I spoke too fast. The driver supports ADXL346-specific features, but does so by detecting the device model at runtime. I still believe it would make sense to list both the 346 and 345 models in DT for 346 devices, as they're compatible with the 345. 2) also add 34x as a compatible but mark it as deprecateed 3) delete 34x from trivial devices OK. Everyone OK with that? -- Regards, Laurent Pinchart -- 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] input: adxl34x: Add OF match support
On Thu, Jan 15, 2015 at 3:19 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
An option would be to remove adi,adxl34x from
Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case
the driver should match explicitly on adi,adxl345 and adi,adxl346.
That might clash with the DT ABI stability requirements though.
I do prefer this:
1) add specific compatible values to the driver. We do those updates for
new devices all the time
Do you mean OF compatible values, or I2C match table entries ? I assume OF
compatible values.
As the ADXL346 is backward-compatible with the ADXL345, and as the driver
doesn't support the ADXL346-specific features, how about adding only the
adxl345 for now, and using compatible = adi,adxl346, adi,adxl345; for the
ADXL346 ?
Adxl34x is instantiated in:
arch/arm/boot/dts/sh73a0-kzm9g.dts
arch/arm/mach-shmobile/board-kzm9g.c
arch/arm/mach-shmobile/board-mackerel.c
arch/blackfin/mach-bf527/boards/tll6527m.c
arch/blackfin/mach-bf537/boards/stamp.c
arch/blackfin/mach-bf548/boards/ezkit.c
arch/blackfin/mach-bf609/boards/ezkit.c
The shmobile variants are all adxl345.
The tll6527m is also an adxl345.
For the remaining, it's not clear to me. I Googled a bit, but no luck.
So they'll have to live with adxl345, too ;-)
2) also add 34x as a compatible but mark it as deprecateed
3) delete 34x from trivial devices
OK.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
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] input: adxl34x: Add OF match support
Been there, got bitten. We only found out too late, because one driver was in i2c and the other in GPIO (or LED even?), both using 953x :( That seems like a development, review and/or merge process failure to me, I wouldn't avoid generic compatible strings for that reason only. Well, I think different here, but let's skip this discussion as it is not really needed right now... As the ADXL346 is backward-compatible with the ADXL345, and as the driver doesn't support the ADXL346-specific features, how about adding only the adxl345 for now, and using compatible = adi,adxl346, adi,adxl345; for the ADXL346 ? I spoke too fast. The driver supports ADXL346-specific features, but does so by detecting the device model at runtime. I still believe it would make sense to list both the 346 and 345 models in DT for 346 devices, as they're compatible with the 345. I agree. 2) also add 34x as a compatible but mark it as deprecateed 3) delete 34x from trivial devices OK. Yay :) signature.asc Description: Digital signature
Re: [PATCH] input: adxl34x: Add OF match support
On Thursday 15 January 2015 15:36:37 Wolfram Sang wrote: Been there, got bitten. We only found out too late, because one driver was in i2c and the other in GPIO (or LED even?), both using 953x :( That seems like a development, review and/or merge process failure to me, I wouldn't avoid generic compatible strings for that reason only. Well, I think different here, but let's skip this discussion as it is not really needed right now... As the ADXL346 is backward-compatible with the ADXL345, and as the driver doesn't support the ADXL346-specific features, how about adding only the adxl345 for now, and using compatible = adi,adxl346, adi,adxl345; for the ADXL346 ? I spoke too fast. The driver supports ADXL346-specific features, but does so by detecting the device model at runtime. I still believe it would make sense to list both the 346 and 345 models in DT for 346 devices, as they're compatible with the 345. I agree. 2) also add 34x as a compatible but mark it as deprecateed 3) delete 34x from trivial devices OK. Yay :) I'll submit patches very soon. -- Regards, Laurent Pinchart -- 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
