Linux I2C development
 help / color / mirror / Atom feed
* [PATCH 2/2] i2c: pca9541: Export OF device ID table as module aliases
From: Javier Martinez Canillas @ 2017-01-16 13:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Javier Martinez Canillas, linux-i2c, Guenter Roeck, Peter Rosin,
	Wolfram Sang
In-Reply-To: <1484574895-952-1-git-send-email-javier@osg.samsung.com>

The I2C core always reports a MODALIAS of the form i2c:<foo> even if the
device was registered via OF, this means that exporting the OF device ID
table device aliases in the module is not needed. But in order to change
how the core reports modaliases to user-space, it's better to export it.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/i2c/muxes/i2c-mux-pca9541.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index 4ea7e691afc7..77840f7845a1 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -90,6 +90,7 @@ static const struct of_device_id pca9541_of_match[] = {
 	{ .compatible = "nxp,pca9541" },
 	{}
 };
+MODULE_DEVICE_TABLE(of, pca9541_of_match);
 #endif
 
 /*
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/2] i2c: mux: pca954x: Export OF device ID table as module aliases
From: Javier Martinez Canillas @ 2017-01-16 13:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Javier Martinez Canillas, Peter Rosin, Wolfram Sang, linux-i2c

The I2C core always reports a MODALIAS of the form i2c:<foo> even if the
device was registered via OF, this means that exporting the OF device ID
table device aliases in the module is not needed. But in order to change
how the core reports modaliases to user-space, it's better to export it.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/i2c/muxes/i2c-mux-pca954x.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index dd18b9ccb1f4..a0232e079545 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -148,6 +148,7 @@ static const struct of_device_id pca954x_of_match[] = {
 	{ .compatible = "nxp,pca9548", .data = &chips[pca_9548] },
 	{}
 };
+MODULE_DEVICE_TABLE(of, pca954x_of_match);
 #endif
 
 /* Write to mux register. Don't use i2c_transfer()/i2c_smbus_xfer()
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2] i2c: i2c-cadence: Initialize configuration before probing devices
From: Mike Looijmans @ 2017-01-16 14:49 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-arm-kernel, linux-kernel, wsa, soren.brinkmann,
	michal.simek, Mike Looijmans

The cadence I2C driver calls cdns_i2c_writereg(..) to setup a workaround
in the controller, but did so after calling i2c_add_adapter() which starts
probing devices on the bus. Change the order so that the configuration is
completely finished before using the adapter.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
v2: Only move the initialization to before i2c_add_adapter, don't change the
    logging output.
 drivers/i2c/busses/i2c-cadence.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 6869712..45d6771 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -962,10 +962,6 @@ static int cdns_i2c_probe(struct platform_device *pdev)
 		goto err_clk_dis;
 	}
 
-	ret = i2c_add_adapter(&id->adap);
-	if (ret < 0)
-		goto err_clk_dis;
-
 	/*
 	 * Cadence I2C controller has a bug wherein it generates
 	 * invalid read transaction after HW timeout in master receiver mode.
@@ -975,6 +971,10 @@ static int cdns_i2c_probe(struct platform_device *pdev)
 	 */
 	cdns_i2c_writereg(CDNS_I2C_TIMEOUT_MAX, CDNS_I2C_TIME_OUT_OFFSET);
 
+	ret = i2c_add_adapter(&id->adap);
+	if (ret < 0)
+		goto err_clk_dis;
+
 	dev_info(&pdev->dev, "%u kHz mmio %08lx irq %d\n",
 		 id->i2c_clk / 1000, (unsigned long)r_mem->start, id->irq);
 
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] i2c: core: helper function to detect slave mode
From: Vladimir Zapolskiy @ 2017-01-16 23:14 UTC (permalink / raw)
  To: Luis Oliveira, Andy Shevchenko, Andy Shevchenko
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Jarkko Nikula,
	Mika Westerberg, linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w, Joao Pinto,
	CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <7fdf5d3c-d0ea-ec45-6b18-4573fff6dd11-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

Hello Luis,

On 01/16/2017 12:32 PM, Luis Oliveira wrote:
> On 12-Jan-17 17:01, Andy Shevchenko wrote:
>> On Sat, 2017-01-07 at 03:24 +0200, Vladimir Zapolskiy wrote:
>>> On 01/07/2017 02:19 AM, Andy Shevchenko wrote:
>>>> On Sat, Jan 7, 2017 at 1:43 AM, Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
>>>> wrote:
>>>>> On 01/07/2017 12:45 AM, Andy Shevchenko wrote:
>>
>>>>>> +             }
>>>>>>>> +     } else if (IS_BUILTIN(CONFIG_ACPI) &&
>>>>>>>> ACPI_HANDLE(dev)) {
>>>>>>>> +             dev_dbg(dev, "ACPI slave is not supported
>>>>>>>> yet\n");
>>>>>>>> +     }
>>>>>>>
>>>>>>> If so, then it might be better to drop else-if stub for now.
>>>>>>
>>>>>> Please, don't.
>>>>>>
>>>>>
>>>>> Why do you ask for this stub to be added?
>>>>
>>>> 1. Exactly the reason you asked above. Here is the code which has
>>>> built differently on different platforms. x86 usually is not using
>>>> CONFIG_OF, ARM doesn't ACPI (versus ARM64). Check GPIO library for
>>>> existing examples.
>>>
>>> From the context by the stub I mean dev_dbg() in
>>> i2c_slave_mode_detect()
>>> function, I don't see a connection to GPIO library, please clarify.
>>
>> I agree that is not good proof for using IS_ENABLED/IS_BUILTIN macro.
> 
> I can prepare a V3 and remove it if that's the decision.
> 

from my review comments you may find that your v2 change contains a bug
plus it misses a minor but desirable code optimization. You may get more
review comments then, for example it is not obvious that on a platform
with both CONFIG_ACPI and CONFIG_OF enabled there should be an exclusive
selection of only one of two possible branches as in your code etc.

The discussed IS_BUILTIN() and dev_dbg() code chunks are other ones, for
both of them you may find my review comments and Andy's responses, it's
up to you as the author to make any updates or keep the code as is,
taking into account any expressed concerns and concerns about concerns
the maintainer will make a decision.

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [Patch V1] i2c: imx-lpi2c: add VLLS mode support
From: Pandy Gao @ 2017-01-17  6:11 UTC (permalink / raw)
  To: Vladimir Zapolskiy, wsa@the-dreams.de,
	wsa-dev@sang-engineering.com
  Cc: linux-i2c@vger.kernel.org, Frank Li, Andy Duan
In-Reply-To: <fee8ed68-7280-175c-c33e-8aafe9f94e7c@mleia.com>

 From: Vladimir Zapolskiy <mailto:vz@mleia.com>  Sent: Saturday, January 07, 2017 5:20 AM
> To: Pandy Gao <pandy.gao@nxp.com>; wsa@the-dreams.de; wsa-
> dev@sang-engineering.com
> Cc: linux-i2c@vger.kernel.org; Frank Li <frank.li@nxp.com>; Andy Duan
> <fugang.duan@nxp.com>
> Subject: Re: [Patch V1] i2c: imx-lpi2c: add VLLS mode support
> 
> Hi,
> 
> On 01/04/2017 11:20 AM, Gao Pan wrote:
> > When system enters VLLS mode, module power is turned off. As a
> result,
> > all registers are reset to HW default value. After exiting VLLS mode,
> > registers are still in default mode. As a result, the pinctrl settings
> > are incorrect, which will affect the module function.
> >
> > The patch recovers the pinctrl setting when exit VLLS mode.
> >
> > Signed-off-by: Gao Pan <pandy.gao@nxp.com>
> > ---
> >  drivers/i2c/busses/i2c-imx-lpi2c.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > index c62b7cd..e528c1d1 100644
> > --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > @@ -636,12 +636,34 @@ static int lpi2c_imx_remove(struct
> platform_device *pdev)
> >  	return 0;
> >  }
> >
> > +#ifdef CONFIG_PM_SLEEP
> > +static int lpi2c_imx_suspend(struct device *dev) {
> > +	pinctrl_pm_select_sleep_state(dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static int lpi2c_imx_resume(struct device *dev) {
> > +	pinctrl_pm_select_default_state(dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(imx_lpi2c_pm, lpi2c_imx_suspend,
> lpi2c_imx_resume);
> > +#define IMX_LPI2C_PM	(&imx_lpi2c_pm)
> > +#else
> > +#define IMX_LPI2C_PM	NULL
> 
> Please drop #else branch and IMX_LPI2C_PM completely.

Thanks,  will change it in next version. 

> > +#endif
> > +
> >  static struct platform_driver lpi2c_imx_driver = {
> >  	.probe = lpi2c_imx_probe,
> >  	.remove = lpi2c_imx_remove,
> >  	.driver = {
> >  		.name = DRIVER_NAME,
> >  		.of_match_table = lpi2c_imx_of_match,
> > +		.pm = IMX_LPI2C_PM,
> 
> Here it should be:
> 
> 		.pm = &imx_lpi2c_pm,

Thanks, will change it in next version.  As the above "#else branch" is removed, the following condition is necessary.
#ifdef CONFIG_PM_SLEEP
                .pm = &imx_lpi2c_pm,
#endif

Best Regards
Pandy

^ permalink raw reply

* Re: [PATCH] i2c: exynos5: fix arbitration lost handling
From: Andi Shyti @ 2017-01-17  6:47 UTC (permalink / raw)
  To: Andrzej Hajda
  Cc: linux-i2c, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
	Wolfram Sang, Krzysztof Kozlowski, Javier Martinez Canillas,
	linux-samsung-soc
In-Reply-To: <1483618013-10721-1-git-send-email-a.hajda@samsung.com>

Hi Andrzej,

On Thu, Jan 05, 2017 at 01:06:53PM +0100, Andrzej Hajda wrote:
> In case of arbitration lost adequate interrupt sometimes is not signaled. As
> a result transfer timeouts and is not retried, as it should. To avoid such
> cases code is added to check transaction status in case of every interrupt.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Hi,
> 
> I am not I2C specialist, this patch is a result of debugging issues
> with SiI8620 MHL driver on TM2 device. I guess the main problem is
> with SiI8620 chip, but exynos5 i2c is also guilty - with i2c-gpio driver
> it works correctly - there is only one arbitration lost at 1st transaction
> (which is properly handled - transfer is repeated succesfully). In case
> of exynos5 there are much more arbitration losts. Lowering bus speed
> decreases frequency of losts but do not eliminate them.
> Any help/hint, what could cause such issues?

I'm having the same issue on TM2 and your patch fixes it. I'm
not sure, though, that's the right fix.

In any case I will look into it.

Andi

^ permalink raw reply

* [Patch V2] i2c: imx-lpi2c: add VLLS mode support
From: Gao Pan @ 2017-01-17  7:09 UTC (permalink / raw)
  To: wsa, wsa-dev, vz; +Cc: linux-i2c, pandy.gao, frank.li

When system enters VLLS mode, module power is turned off. As a result,
all registers are reset to HW default value. After exiting VLLS mode,
registers are still in default mode. As a result, the pinctrl settings
are incorrect, which will affect the module function.

The patch recovers the pinctrl setting when exit VLLS mode.

Signed-off-by: Gao Pan <pandy.gao@nxp.com>
---
V2:
 -drop "#else branch" and IMX_LPI2C_PM completely
 -pm = &imx_lpi2c_pm

 drivers/i2c/busses/i2c-imx-lpi2c.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index c62b7cd..dbde95d 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -636,12 +636,33 @@ static int lpi2c_imx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int lpi2c_imx_suspend(struct device *dev)
+{
+	pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
+}
+
+static int lpi2c_imx_resume(struct device *dev)
+{
+	pinctrl_pm_select_default_state(dev);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(imx_lpi2c_pm, lpi2c_imx_suspend, lpi2c_imx_resume);
+#endif
+
 static struct platform_driver lpi2c_imx_driver = {
 	.probe = lpi2c_imx_probe,
 	.remove = lpi2c_imx_remove,
 	.driver = {
 		.name = DRIVER_NAME,
 		.of_match_table = lpi2c_imx_of_match,
+#ifdef CONFIG_PM_SLEEP
+		.pm = &imx_lpi2c_pm,
+#endif
 	},
 };
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v5 0/5]  i2c: mux: pca954x: Add interrupt controller support
From: Phil Reid @ 2017-01-17  8:00 UTC (permalink / raw)
  To: peda-koto5C5qi+TLoDKTGw+V6w, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Various muxes can aggregate multiple interrupts from each i2c bus.
All of the muxes with interrupt support combine the active low irq lines
using an internal 'and' function and generate a combined active low
output. The muxes do provide the ability to read a control register to
determine which irq is active. By making the mux an irq controller isr
latenct can potentially be reduced by reading the status register and 
then only calling the registered isr on that bus segment.

In addition an additional enable mask is added to work around devices
that assert irq immediately before being setup by disabling the irq
from the mux until all devices are registered.

Changes from v4:
- p4: Change definition of irq_mask_enable to an array.
- p4: Removed acks due to change requested by Peter
- p5: Parse array of enables. Currently only supports 1 chip
      But dt specification will allow expansion to handle
      multple irq consume chips to be registered on a bus segment
- p5: Fix up logic related to enabling and disable irq's.
      Use a flag to indicate when irq has been enabled.

Changes from v3:
- p3: Add spin lock to irq mask / unmask.
- p4: Add Rob's ack.

Changes from v2:
- p1: Added Acked-by
- p5: fixup 2 typos

Changes from v1:
- Update for new ACPI table
- Fix typo in documentation
- Fix typo in function names
- Fix typo in irq name
- Added spaces around '+' / '='
- Change goto label names
- Change property name from i2c-mux-irq-mask-en to nxp,irq-mask-enable
- Change variable name irq_mask_en to irq_mask_enable
- Add commentt about irq_mask_enable
- Added Acked-By's

Phil Reid (5):
  i2c: mux: pca954x: Add missing pca9542 definition to chip_desc
  dt: bindings: i2c-mux-pca954x: Add documentation for interrupt
    controller
  i2c: mux: pca954x: Add interrupt controller support
  dt: bindings: i2c-mux-pca954x: Add documentation for
    nxp,irq-mask-enable
  i2c: mux: pca954x: Add irq-mask-enable to delay enabling irqs

 .../devicetree/bindings/i2c/i2c-mux-pca954x.txt    |  17 +-
 drivers/i2c/muxes/i2c-mux-pca954x.c                | 177 ++++++++++++++++++++-
 2 files changed, 189 insertions(+), 5 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v5 4/5] dt: bindings: i2c-mux-pca954x: Add documentation for nxp,irq-mask-enable
From: Phil Reid @ 2017-01-17  8:00 UTC (permalink / raw)
  To: peda-koto5C5qi+TLoDKTGw+V6w, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484640029-22870-1-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>

Unfortunately some hardware device will assert their irq line immediately
on power on and provide no mechanism to mask the irq. As the i2c muxes
provide no method to mask irq line this provides a work around by keeping
the parent irq masked until enough device drivers have loaded to service
all pending interrupts.

For example the the ltc1760 assert its SMBALERT irq immediately on power
on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
device is registered irq are enabled and fire continuously as the second
device driver has not yet loaded. Setting this parameter to <1 1> will
delay the irq being enabled until both devices are ready.

Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
 Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
index aa09704..ac71be6 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
@@ -19,6 +19,8 @@ Optional Properties:
   - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
     children in idle state. This is necessary for example, if there are several
     multiplexers on the bus and the devices behind them use same I2C addresses.
+  - nxp,irq-mask-enable: array; Defines the minimum number of chips that must
+    register an irq for each channel before the parent irq line in enabled.
   - interrupt-parent: Phandle for the interrupt controller that services
     interrupts for this device.
   - interrupts: Interrupt mapping for IRQ.
@@ -36,6 +38,7 @@ Example:
 		#size-cells = <0>;
 		reg = <0x74>;
 
+		nxp,irq-mask-enable = <0 0 0 0 1 0 0 0>;
 		interrupt-parent = <&ipic>;
 		interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
 		interrupt-controller;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v5 5/5] i2c: mux: pca954x: Add irq-mask-enable to delay enabling irqs
From: Phil Reid @ 2017-01-17  8:00 UTC (permalink / raw)
  To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
In-Reply-To: <1484640029-22870-1-git-send-email-preid@electromag.com.au>

Unfortunately some hardware device will assert their irq line immediately
on power on and provide no mechanism to mask the irq. As the i2c muxes
provide no method to mask irq line this provides a work around by keeping
the parent irq masked until enough device drivers have loaded to service
all pending interrupts.

For example the the ltc1760 assert its SMBALERT irq immediately on power
on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
device is registered irq are enabled and fire continuously as the second
device driver has not yet loaded. Setting this parameter to <1 1> will
delay the irq being enabled until both devices are ready.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index f55da88..012b2ef 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -76,6 +76,19 @@ struct chip_desc {
 	} muxtype;
 };
 
+/*
+ * irq_mask_enable: Provides a mechanism to work around hardware that asserts
+ * their irq immediately on power on. It allows the enabling of the irq to be
+ * delayed until the corresponding bits in the the irq_mask are set thru
+ * irq_unmask.
+ * For example the ltc1760 assert its SMBALERT irq immediately on power on.
+ * With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
+ * device is registered irq are enabled and fire continuously as the second
+ * device driver has not yet loaded. Setting this parameter to 0x3 while
+ * delay the irq being enabled until both devices are ready.
+ * This workaround will not work if two devices share an interrupt on the
+ * same bus segment.
+ */
 struct pca954x {
 	const struct chip_desc *chip;
 
@@ -84,7 +97,9 @@ struct pca954x {
 	struct i2c_client *client;
 
 	struct irq_domain *irq;
+	unsigned int irq_mask_enable;
 	unsigned int irq_mask;
+	bool irq_enabled;
 	spinlock_t lock;
 };
 
@@ -266,8 +281,10 @@ static void pca954x_irq_mask(struct irq_data *idata)
 	spin_lock_irqsave(&data->lock, flags);
 
 	data->irq_mask &= ~BIT(pos);
-	if (!data->irq_mask)
+	if (data->irq_enabled && !data->irq_mask) {
 		disable_irq(data->client->irq);
+		data->irq_enabled = false;
+	}
 
 	spin_unlock_irqrestore(&data->lock, flags);
 }
@@ -275,14 +292,18 @@ static void pca954x_irq_mask(struct irq_data *idata)
 static void pca954x_irq_unmask(struct irq_data *idata)
 {
 	struct pca954x *data = irq_data_get_irq_chip_data(idata);
+	unsigned int mask_enable = data->irq_mask_enable;
 	unsigned int pos = idata->hwirq;
 	unsigned long flags;
 
 	spin_lock_irqsave(&data->lock, flags);
 
-	if (!data->irq_mask)
-		enable_irq(data->client->irq);
 	data->irq_mask |= BIT(pos);
+	if (!data->irq_enabled
+	    && (data->irq_mask & mask_enable) == mask_enable) {
+		enable_irq(data->client->irq);
+		data->irq_enabled = true;
+	}
 
 	spin_unlock_irqrestore(&data->lock, flags);
 }
@@ -305,6 +326,7 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 {
 	struct pca954x *data = i2c_mux_priv(muxc);
 	struct i2c_client *client = data->client;
+	u32 irq_mask_enable[PCA954X_MAX_NCHANS] = { 0 };
 	int c, err, irq;
 
 	if (!data->chip->has_irq || client->irq <= 0)
@@ -312,6 +334,9 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 
 	spin_lock_init(&data->lock);
 
+	of_property_read_u32_array(client->dev.of_node, "nxp,irq-mask-enable",
+		irq_mask_enable, data->chip->nchans);
+
 	data->irq = irq_domain_add_linear(client->dev.of_node,
 					  data->chip->nchans,
 					  &irq_domain_simple_ops, data);
@@ -319,6 +344,8 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 		return -ENODEV;
 
 	for (c = 0; c < data->chip->nchans; c++) {
+		data->irq_mask_enable |= irq_mask_enable[c] ? BIT(c) : 0;
+		WARN_ON(irq_mask_enable[c] > 1);
 		irq = irq_create_mapping(data->irq, c);
 		irq_set_chip_data(irq, data);
 		irq_set_chip_and_handler(irq, &pca954x_irq_chip,
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v5 3/5] i2c: mux: pca954x: Add interrupt controller support
From: Phil Reid @ 2017-01-17  8:00 UTC (permalink / raw)
  To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
In-Reply-To: <1484640029-22870-1-git-send-email-preid@electromag.com.au>

Various muxes can aggregate multiple interrupts from each i2c bus.
All of the muxes with interrupt support combine the active low irq lines
using an internal 'and' function and generate a combined active low
output. The muxes do provide the ability to read a control register to
determine which irq is active. By making the mux an irq controller isr
latency can potentially be reduced by reading the status register and
then only calling the registered isr on that bus segment.

As there is no irq masking on the mux irq are disabled until irq_unmask is
called at least once.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 141 +++++++++++++++++++++++++++++++++++-
 1 file changed, 139 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index bbf088e..f55da88 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -41,14 +41,20 @@
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
 #include <linux/i2c/pca954x.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_irq.h>
 #include <linux/pm.h>
 #include <linux/slab.h>
+#include <linux/spinlock.h>
 
 #define PCA954X_MAX_NCHANS 8
 
+#define PCA954X_IRQ_OFFSET 4
+
 enum pca_type {
 	pca_9540,
 	pca_9542,
@@ -63,6 +69,7 @@ enum pca_type {
 struct chip_desc {
 	u8 nchans;
 	u8 enable;	/* used for muxes only */
+	u8 has_irq;
 	enum muxtype {
 		pca954x_ismux = 0,
 		pca954x_isswi
@@ -75,6 +82,10 @@ struct pca954x {
 	u8 last_chan;		/* last register value */
 	u8 deselect;
 	struct i2c_client *client;
+
+	struct irq_domain *irq;
+	unsigned int irq_mask;
+	spinlock_t lock;
 };
 
 /* Provide specs for the PCA954x types we know about */
@@ -87,19 +98,23 @@ struct pca954x {
 	[pca_9542] = {
 		.nchans = 2,
 		.enable = 0x4,
+		.has_irq = 1,
 		.muxtype = pca954x_ismux,
 	},
 	[pca_9543] = {
 		.nchans = 2,
+		.has_irq = 1,
 		.muxtype = pca954x_isswi,
 	},
 	[pca_9544] = {
 		.nchans = 4,
 		.enable = 0x4,
+		.has_irq = 1,
 		.muxtype = pca954x_ismux,
 	},
 	[pca_9545] = {
 		.nchans = 4,
+		.has_irq = 1,
 		.muxtype = pca954x_isswi,
 	},
 	[pca_9547] = {
@@ -222,6 +237,114 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
 	return pca954x_reg_write(muxc->parent, client, data->last_chan);
 }
 
+static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
+{
+	struct pca954x *data = dev_id;
+	unsigned int child_irq;
+	int ret, i, handled;
+
+	ret = i2c_smbus_read_byte(data->client);
+	if (ret < 0)
+		return IRQ_NONE;
+
+	for (i = 0; i < data->chip->nchans; i++) {
+		if (ret & BIT(PCA954X_IRQ_OFFSET + i)) {
+			child_irq = irq_linear_revmap(data->irq, i);
+			handle_nested_irq(child_irq);
+			handled++;
+		}
+	}
+	return handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static void pca954x_irq_mask(struct irq_data *idata)
+{
+	struct pca954x *data = irq_data_get_irq_chip_data(idata);
+	unsigned int pos = idata->hwirq;
+	unsigned long flags;
+
+	spin_lock_irqsave(&data->lock, flags);
+
+	data->irq_mask &= ~BIT(pos);
+	if (!data->irq_mask)
+		disable_irq(data->client->irq);
+
+	spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static void pca954x_irq_unmask(struct irq_data *idata)
+{
+	struct pca954x *data = irq_data_get_irq_chip_data(idata);
+	unsigned int pos = idata->hwirq;
+	unsigned long flags;
+
+	spin_lock_irqsave(&data->lock, flags);
+
+	if (!data->irq_mask)
+		enable_irq(data->client->irq);
+	data->irq_mask |= BIT(pos);
+
+	spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
+{
+	if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+	return 0;
+}
+
+static struct irq_chip pca954x_irq_chip = {
+	.name = "i2c-mux-pca954x",
+	.irq_mask = pca954x_irq_mask,
+	.irq_unmask = pca954x_irq_unmask,
+	.irq_set_type = pca954x_irq_set_type,
+};
+
+static int pca954x_irq_setup(struct i2c_mux_core *muxc)
+{
+	struct pca954x *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
+	int c, err, irq;
+
+	if (!data->chip->has_irq || client->irq <= 0)
+		return 0;
+
+	spin_lock_init(&data->lock);
+
+	data->irq = irq_domain_add_linear(client->dev.of_node,
+					  data->chip->nchans,
+					  &irq_domain_simple_ops, data);
+	if (!data->irq)
+		return -ENODEV;
+
+	for (c = 0; c < data->chip->nchans; c++) {
+		irq = irq_create_mapping(data->irq, c);
+		irq_set_chip_data(irq, data);
+		irq_set_chip_and_handler(irq, &pca954x_irq_chip,
+			handle_simple_irq);
+	}
+
+	err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
+					pca954x_irq_handler,
+					IRQF_ONESHOT | IRQF_SHARED,
+					"pca954x", data);
+	if (err)
+		goto err_req_irq;
+
+	disable_irq(data->client->irq);
+
+	return 0;
+err_req_irq:
+	for (c = 0; c < data->chip->nchans; c++) {
+		irq = irq_find_mapping(data->irq, c);
+		irq_dispose_mapping(irq);
+	}
+	irq_domain_remove(data->irq);
+
+	return err;
+}
+
 /*
  * I2C init/probing/exit functions
  */
@@ -286,6 +409,10 @@ static int pca954x_probe(struct i2c_client *client,
 	idle_disconnect_dt = of_node &&
 		of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
 
+	ret = pca954x_irq_setup(muxc);
+	if (ret)
+		goto fail_del_adapters;
+
 	/* Now create an adapter for each channel */
 	for (num = 0; num < data->chip->nchans; num++) {
 		bool idle_disconnect_pd = false;
@@ -311,7 +438,7 @@ static int pca954x_probe(struct i2c_client *client,
 			dev_err(&client->dev,
 				"failed to register multiplexed adapter"
 				" %d as bus %d\n", num, force);
-			goto virt_reg_failed;
+			goto fail_del_adapters;
 		}
 	}
 
@@ -322,7 +449,7 @@ static int pca954x_probe(struct i2c_client *client,
 
 	return 0;
 
-virt_reg_failed:
+fail_del_adapters:
 	i2c_mux_del_adapters(muxc);
 	return ret;
 }
@@ -330,6 +457,16 @@ static int pca954x_probe(struct i2c_client *client,
 static int pca954x_remove(struct i2c_client *client)
 {
 	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+	struct pca954x *data = i2c_mux_priv(muxc);
+	int c, irq;
+
+	if (data->irq) {
+		for (c = 0; c < data->chip->nchans; c++) {
+			irq = irq_find_mapping(data->irq, c);
+			irq_dispose_mapping(irq);
+		}
+		irq_domain_remove(data->irq);
+	}
 
 	i2c_mux_del_adapters(muxc);
 	return 0;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v5 2/5] dt: bindings: i2c-mux-pca954x: Add documentation for interrupt controller
From: Phil Reid @ 2017-01-17  8:00 UTC (permalink / raw)
  To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
In-Reply-To: <1484640029-22870-1-git-send-email-preid@electromag.com.au>

Various muxes can aggregate multiple irq lines and provide a control
register to determine the active line. Add bindings for interrupt
controller support.

Acked-by: Peter Rosin <peda@axentia.se>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
index cf53d5f..aa09704 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
@@ -19,7 +19,14 @@ Optional Properties:
   - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
     children in idle state. This is necessary for example, if there are several
     multiplexers on the bus and the devices behind them use same I2C addresses.
-
+  - interrupt-parent: Phandle for the interrupt controller that services
+    interrupts for this device.
+  - interrupts: Interrupt mapping for IRQ.
+  - interrupt-controller: Marks the device node as an interrupt controller.
+  - #interrupt-cells : Should be two.
+    - first cell is the pin number
+    - second cell is used to specify flags.
+    See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
 
 Example:
 
@@ -29,6 +36,11 @@ Example:
 		#size-cells = <0>;
 		reg = <0x74>;
 
+		interrupt-parent = <&ipic>;
+		interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+
 		i2c@2 {
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v5 1/5] i2c: mux: pca954x: Add missing pca9542 definition to chip_desc
From: Phil Reid @ 2017-01-17  8:00 UTC (permalink / raw)
  To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
In-Reply-To: <1484640029-22870-1-git-send-email-preid@electromag.com.au>

The spec for the pca954x was missing. This chip is the same as the pca9540
except that it has interrupt lines. While the i2c_device_id table mapped
the pca9542 to the pca9540 definition the compatible table did not. In
preparation for irq support add the pca9542 definition.

Acked-by: Peter Rosin <peda@axentia.se>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index dd18b9c..bbf088e 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -84,6 +84,11 @@ struct pca954x {
 		.enable = 0x4,
 		.muxtype = pca954x_ismux,
 	},
+	[pca_9542] = {
+		.nchans = 2,
+		.enable = 0x4,
+		.muxtype = pca954x_ismux,
+	},
 	[pca_9543] = {
 		.nchans = 2,
 		.muxtype = pca954x_isswi,
@@ -110,7 +115,7 @@ struct pca954x {
 
 static const struct i2c_device_id pca954x_id[] = {
 	{ "pca9540", pca_9540 },
-	{ "pca9542", pca_9540 },
+	{ "pca9542", pca_9542 },
 	{ "pca9543", pca_9543 },
 	{ "pca9544", pca_9544 },
 	{ "pca9545", pca_9545 },
@@ -124,7 +129,7 @@ struct pca954x {
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id pca954x_acpi_ids[] = {
 	{ .id = "PCA9540", .driver_data = pca_9540 },
-	{ .id = "PCA9542", .driver_data = pca_9540 },
+	{ .id = "PCA9542", .driver_data = pca_9542 },
 	{ .id = "PCA9543", .driver_data = pca_9543 },
 	{ .id = "PCA9544", .driver_data = pca_9544 },
 	{ .id = "PCA9545", .driver_data = pca_9545 },
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH v4 5/5] i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs
From: Phil Reid @ 2017-01-17  8:07 UTC (permalink / raw)
  To: Peter Rosin, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <b2e0ba39-2a1b-9c67-082d-d7d043ecc220-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

On 16/01/2017 20:08, Peter Rosin wrote:

>> @@ -280,9 +294,12 @@ static void pca954x_irq_unmask(struct irq_data *idata)
>>
>>  	spin_lock_irqsave(&data->lock, flags);
>>
>> -	if (!data->irq_mask)
>> +	if (!data->irq_mask_enable && !data->irq_mask)
>>  		enable_irq(data->client->irq);
>>  	data->irq_mask |= BIT(pos);
>> +	if (data->irq_mask_enable &&
>> +		(data->irq_mask & data->irq_mask) == data->irq_mask_enable)
>
> Hmm, I see that some apparently incompetent person :-) already acked this,
> but the (data->irq_mask & data->irq_mask) part doesn't make sense at all.
>
>> +		enable_irq(data->client->irq);
>>
>
> Hmm2, if you have a problematic device (like the ltc1760) on mux segment 0
> and sane devices on other segments I'd be inclined to specify irq-mask-enable
> as 0x1. But then this is possible:
>
> 1. ltc1760 registers its irq
> 2. enable_irq(data->client->irq) is called because irq_mask_enable is "fulfilled"
> 3. a sane irq register an irq on some other segment
> 4. enable_irq(...) is called again (which the code appears to try to avoid)
>
> As I read the code, there will be problems with specifying irq-mask-enable
> whenever there are more than one irq-client on a mux segment.
>
> So, I'm removing my ack until the above is resolved...
>

G'day Peter,

Yes that is obviously wrong.
I think I've got it right this time in v5.
Also implemented your suggest on the array for the dt binding.
But stuck with the bitfield implementation for the moment.
Hopefully that's ok.

-- 
Regards
Phil Reid

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 4/5] dt: bindings: i2c-mux-pca954x: Add documentation for nxp,irq-mask-enable
From: Peter Rosin @ 2017-01-17  8:57 UTC (permalink / raw)
  To: Phil Reid, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484640029-22870-5-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>

On 2017-01-17 09:00, Phil Reid wrote:
> Unfortunately some hardware device will assert their irq line immediately
> on power on and provide no mechanism to mask the irq. As the i2c muxes
> provide no method to mask irq line this provides a work around by keeping
> the parent irq masked until enough device drivers have loaded to service
> all pending interrupts.
> 
> For example the the ltc1760 assert its SMBALERT irq immediately on power
> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
> device is registered irq are enabled and fire continuously as the second
> device driver has not yet loaded. Setting this parameter to <1 1> will
> delay the irq being enabled until both devices are ready.

Hang on, does this suggestion I made make any sense at all? Maybe it does,
but does the pca954x driver even get notified of any but the first irq client
that unmasks interrupts on a mux segment? How can it count the number of
active irq clients if not?

I'm truly sorry for the trouble I'm causing by not just saying how it should
be done from the start, but I feel like I've been thrown in at the deep end
when it comes to interrupt controllers...

Cheers,
peda

> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> index aa09704..ac71be6 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> @@ -19,6 +19,8 @@ Optional Properties:
>    - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
>      children in idle state. This is necessary for example, if there are several
>      multiplexers on the bus and the devices behind them use same I2C addresses.
> +  - nxp,irq-mask-enable: array; Defines the minimum number of chips that must
> +    register an irq for each channel before the parent irq line in enabled.
>    - interrupt-parent: Phandle for the interrupt controller that services
>      interrupts for this device.
>    - interrupts: Interrupt mapping for IRQ.
> @@ -36,6 +38,7 @@ Example:
>  		#size-cells = <0>;
>  		reg = <0x74>;
>  
> +		nxp,irq-mask-enable = <0 0 0 0 1 0 0 0>;
>  		interrupt-parent = <&ipic>;
>  		interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
>  		interrupt-controller;
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [Patch V2] i2c: imx-lpi2c: add VLLS mode support
From: Vladimir Zapolskiy @ 2017-01-17  9:10 UTC (permalink / raw)
  To: Gao Pan, wsa, wsa-dev; +Cc: linux-i2c, frank.li
In-Reply-To: <1484636967-23599-1-git-send-email-pandy.gao@nxp.com>

Hello Pandy,

On 01/17/2017 09:09 AM, Gao Pan wrote:
> When system enters VLLS mode, module power is turned off. As a result,
> all registers are reset to HW default value. After exiting VLLS mode,
> registers are still in default mode. As a result, the pinctrl settings
> are incorrect, which will affect the module function.
> 
> The patch recovers the pinctrl setting when exit VLLS mode.
> 
> Signed-off-by: Gao Pan <pandy.gao@nxp.com>
> ---
> V2:
>  -drop "#else branch" and IMX_LPI2C_PM completely
>  -pm = &imx_lpi2c_pm
> 
>  drivers/i2c/busses/i2c-imx-lpi2c.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index c62b7cd..dbde95d 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -636,12 +636,33 @@ static int lpi2c_imx_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_PM_SLEEP
> +static int lpi2c_imx_suspend(struct device *dev)
> +{
> +	pinctrl_pm_select_sleep_state(dev);
> +
> +	return 0;
> +}
> +
> +static int lpi2c_imx_resume(struct device *dev)
> +{
> +	pinctrl_pm_select_default_state(dev);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(imx_lpi2c_pm, lpi2c_imx_suspend, lpi2c_imx_resume);
> +#endif

in case if I didn't manage to explain what is desirable, please change
the order of two lines above, you may find that commonly SIMPLE_DEV_PM_OPS()
macro is used outside #ifdef CONFIG_PM_SLEEP, due to the feature that
CONFIG_PM_SLEEP token is incorporated into SIMPLE_DEV_PM_OPS() internals.

> +
>  static struct platform_driver lpi2c_imx_driver = {
>  	.probe = lpi2c_imx_probe,
>  	.remove = lpi2c_imx_remove,
>  	.driver = {
>  		.name = DRIVER_NAME,
>  		.of_match_table = lpi2c_imx_of_match,
> +#ifdef CONFIG_PM_SLEEP
> +		.pm = &imx_lpi2c_pm,
> +#endif

Then this #ifdef won't be needed here.

>  	},
>  };
>  
> 

--
With best wishes,
Vladimir

^ permalink raw reply

* Re: [PATCH v5 4/5] dt: bindings: i2c-mux-pca954x: Add documentation for nxp,irq-mask-enable
From: Phil Reid @ 2017-01-17  9:28 UTC (permalink / raw)
  To: Peter Rosin, wsa, robh+dt, mark.rutland, linux-i2c, devicetree
In-Reply-To: <9190ec70-eda4-5e44-a85a-211945c30eeb@axentia.se>

On 17/01/2017 16:57, Peter Rosin wrote:
> On 2017-01-17 09:00, Phil Reid wrote:
>> Unfortunately some hardware device will assert their irq line immediately
>> on power on and provide no mechanism to mask the irq. As the i2c muxes
>> provide no method to mask irq line this provides a work around by keeping
>> the parent irq masked until enough device drivers have loaded to service
>> all pending interrupts.
>>
>> For example the the ltc1760 assert its SMBALERT irq immediately on power
>> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
>> device is registered irq are enabled and fire continuously as the second
>> device driver has not yet loaded. Setting this parameter to <1 1> will
>> delay the irq being enabled until both devices are ready.
>
G'day Peter,


> Hang on, does this suggestion I made make any sense at all? Maybe it does,
> but does the pca954x driver even get notified of any but the first irq client
> that unmasks interrupts on a mux segment? How can it count the number of
> active irq clients if not?

Good question.

So what I did to test is setup my 2 ltc1760s to use the same irq on the pca954x.
Using the latest patch series.

Adding a log message into the irq_unmask function got the following.
	dev_err(&data->client->dev, "irq_unmask %d %x %d", pos, data->irq_mask, data->irq_enabled);

dmesg | grep irq_unmask
[    4.392098] pca954x 4-0070: irq_unmask 0 1 1


But Looks like both got registered ok to the same irq.
cat /proc/interrupts
161:          0          0  i2c-mux-pca954x   0 Level     15-000a, 16-000a

So from this testing, it doesn't look like it gets called multiple times.
So back to the bitmask for the dt do you think.
I think the interrupt enablelogic is correct now.

>
> I'm truly sorry for the trouble I'm causing by not just saying how it should
> be done from the start, but I feel like I've been thrown in at the deep end
> when it comes to interrupt controllers...

No problem. I'm learning a couple things as we go.
Should help me out on other drivers :)


-- 
Regards
Phil Reid

^ permalink raw reply

* Re: [PATCH v5 4/5] dt: bindings: i2c-mux-pca954x: Add documentation for nxp,irq-mask-enable
From: Peter Rosin @ 2017-01-17  9:43 UTC (permalink / raw)
  To: Phil Reid, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <aeab2bbe-e243-98f6-b07f-76e4dda62fd6-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>

On 2017-01-17 10:28, Phil Reid wrote:
> On 17/01/2017 16:57, Peter Rosin wrote:
>> On 2017-01-17 09:00, Phil Reid wrote:
>>> Unfortunately some hardware device will assert their irq line immediately
>>> on power on and provide no mechanism to mask the irq. As the i2c muxes
>>> provide no method to mask irq line this provides a work around by keeping
>>> the parent irq masked until enough device drivers have loaded to service
>>> all pending interrupts.
>>>
>>> For example the the ltc1760 assert its SMBALERT irq immediately on power
>>> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
>>> device is registered irq are enabled and fire continuously as the second
>>> device driver has not yet loaded. Setting this parameter to <1 1> will
>>> delay the irq being enabled until both devices are ready.
>>
> G'day Peter,
> 
> 
>> Hang on, does this suggestion I made make any sense at all? Maybe it does,
>> but does the pca954x driver even get notified of any but the first irq client
>> that unmasks interrupts on a mux segment? How can it count the number of
>> active irq clients if not?
> 
> Good question.
> 
> So what I did to test is setup my 2 ltc1760s to use the same irq on the pca954x.
> Using the latest patch series.
> 
> Adding a log message into the irq_unmask function got the following.
> 	dev_err(&data->client->dev, "irq_unmask %d %x %d", pos, data->irq_mask, data->irq_enabled);
> 
> dmesg | grep irq_unmask
> [    4.392098] pca954x 4-0070: irq_unmask 0 1 1
> 
> 
> But Looks like both got registered ok to the same irq.
> cat /proc/interrupts
> 161:          0          0  i2c-mux-pca954x   0 Level     15-000a, 16-000a
> 
> So from this testing, it doesn't look like it gets called multiple times.

As I suspected, thanks for verifying!

> So back to the bitmask for the dt do you think.

Looking at kernel/irq/chip.c:irq_enable (and struct irq_chip docs), I get the
feeling that if you provide the irq_enable operation (and maybe irq_disable too?),
that might get us more info?

> I think the interrupt enablelogic is correct now.
> 
>>
>> I'm truly sorry for the trouble I'm causing by not just saying how it should
>> be done from the start, but I feel like I've been thrown in at the deep end
>> when it comes to interrupt controllers...
> 
> No problem. I'm learning a couple things as we go.
> Should help me out on other drivers :)

Yes, I'm also picking up a few bits here and there...

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [Patch V2] i2c: imx-lpi2c: add VLLS mode support
From: Pandy Gao @ 2017-01-17 10:14 UTC (permalink / raw)
  To: Vladimir Zapolskiy, wsa@the-dreams.de,
	wsa-dev@sang-engineering.com
  Cc: linux-i2c@vger.kernel.org, Frank Li
In-Reply-To: <848799b4-a364-3791-0727-414f82308e25@mleia.com>

 From: Vladimir Zapolskiy <mailto:vz@mleia.com>  Sent: Tuesday, January 17, 2017 5:10 PM
> To: Pandy Gao <pandy.gao@nxp.com>; wsa@the-dreams.de; wsa-
> dev@sang-engineering.com
> Cc: linux-i2c@vger.kernel.org; Frank Li <frank.li@nxp.com>
> Subject: Re: [Patch V2] i2c: imx-lpi2c: add VLLS mode support
> 
> Hello Pandy,
> 
> On 01/17/2017 09:09 AM, Gao Pan wrote:
> > When system enters VLLS mode, module power is turned off. As a
> result,
> > all registers are reset to HW default value. After exiting VLLS mode,
> > registers are still in default mode. As a result, the pinctrl settings
> > are incorrect, which will affect the module function.
> >
> > The patch recovers the pinctrl setting when exit VLLS mode.
> >
> > Signed-off-by: Gao Pan <pandy.gao@nxp.com>
> > ---
> > V2:
> >  -drop "#else branch" and IMX_LPI2C_PM completely  -pm =
> &imx_lpi2c_pm
> >
> >  drivers/i2c/busses/i2c-imx-lpi2c.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > index c62b7cd..dbde95d 100644
> > --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > @@ -636,12 +636,33 @@ static int lpi2c_imx_remove(struct
> platform_device *pdev)
> >  	return 0;
> >  }
> >
> > +#ifdef CONFIG_PM_SLEEP
> > +static int lpi2c_imx_suspend(struct device *dev) {
> > +	pinctrl_pm_select_sleep_state(dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static int lpi2c_imx_resume(struct device *dev) {
> > +	pinctrl_pm_select_default_state(dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(imx_lpi2c_pm, lpi2c_imx_suspend,
> > +lpi2c_imx_resume); #endif
> 
> in case if I didn't manage to explain what is desirable, please change
> the order of two lines above, you may find that commonly
> SIMPLE_DEV_PM_OPS() macro is used outside #ifdef CONFIG_PM_SLEEP,
> due to the feature that CONFIG_PM_SLEEP token is incorporated into
> SIMPLE_DEV_PM_OPS() internals.
> 
> > +
> >  static struct platform_driver lpi2c_imx_driver = {
> >  	.probe = lpi2c_imx_probe,
> >  	.remove = lpi2c_imx_remove,
> >  	.driver = {
> >  		.name = DRIVER_NAME,
> >  		.of_match_table = lpi2c_imx_of_match,
> > +#ifdef CONFIG_PM_SLEEP
> > +		.pm = &imx_lpi2c_pm,
> > +#endif
> 
> Then this #ifdef won't be needed here.

Yes, you are right. Will change it in next version. Thanks

Best Regards
Gao Pan

^ permalink raw reply

* Re: [PATCH v5 4/5] dt: bindings: i2c-mux-pca954x: Add documentation for nxp,irq-mask-enable
From: Peter Rosin @ 2017-01-17 10:14 UTC (permalink / raw)
  To: Phil Reid, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <96f3cc0e-9c73-4af4-f072-5a1aeceb67af-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

On 2017-01-17 10:43, Peter Rosin wrote:
> On 2017-01-17 10:28, Phil Reid wrote:
>> On 17/01/2017 16:57, Peter Rosin wrote:
>>> On 2017-01-17 09:00, Phil Reid wrote:
>>>> Unfortunately some hardware device will assert their irq line immediately
>>>> on power on and provide no mechanism to mask the irq. As the i2c muxes
>>>> provide no method to mask irq line this provides a work around by keeping
>>>> the parent irq masked until enough device drivers have loaded to service
>>>> all pending interrupts.
>>>>
>>>> For example the the ltc1760 assert its SMBALERT irq immediately on power
>>>> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
>>>> device is registered irq are enabled and fire continuously as the second
>>>> device driver has not yet loaded. Setting this parameter to <1 1> will
>>>> delay the irq being enabled until both devices are ready.
>>>
>> G'day Peter,
>>
>>
>>> Hang on, does this suggestion I made make any sense at all? Maybe it does,
>>> but does the pca954x driver even get notified of any but the first irq client
>>> that unmasks interrupts on a mux segment? How can it count the number of
>>> active irq clients if not?
>>
>> Good question.
>>
>> So what I did to test is setup my 2 ltc1760s to use the same irq on the pca954x.
>> Using the latest patch series.
>>
>> Adding a log message into the irq_unmask function got the following.
>> 	dev_err(&data->client->dev, "irq_unmask %d %x %d", pos, data->irq_mask, data->irq_enabled);
>>
>> dmesg | grep irq_unmask
>> [    4.392098] pca954x 4-0070: irq_unmask 0 1 1
>>
>>
>> But Looks like both got registered ok to the same irq.
>> cat /proc/interrupts
>> 161:          0          0  i2c-mux-pca954x   0 Level     15-000a, 16-000a
>>
>> So from this testing, it doesn't look like it gets called multiple times.
> 
> As I suspected, thanks for verifying!
> 
>> So back to the bitmask for the dt do you think.
> 
> Looking at kernel/irq/chip.c:irq_enable (and struct irq_chip docs), I get the
> feeling that if you provide the irq_enable operation (and maybe irq_disable too?),
> that might get us more info?

No, I no longer think that. I think we need to get at the irq descriptor "depth".
But that feels like poking at the wrong level. Crap.

And to answer the question if I think we should go back to a dt bitmask, then
no I do not think that. The array describes what we want to do, it's the linux
implementation that gives us difficulties. Agreed?

>> I think the interrupt enablelogic is correct now.
>>
>>>
>>> I'm truly sorry for the trouble I'm causing by not just saying how it should
>>> be done from the start, but I feel like I've been thrown in at the deep end
>>> when it comes to interrupt controllers...
>>
>> No problem. I'm learning a couple things as we go.
>> Should help me out on other drivers :)
> 
> Yes, I'm also picking up a few bits here and there...
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger
From: Andreas Klinger @ 2017-01-17 10:28 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, ktsai-GubuWUlQtMwciDkP5Hr2oA,
	wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, trivial-DgEjT+Ai2ygdnm+yROfE0A,
	mranostay-Re5JQEeQqe8AvxtiuMwx3w,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d05cce62-4362-b249-f49f-58eed2316ddb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Hi Jonathan,

[...]

> >> For the range, it's an interesting one.  Again the term range could
> >> mean too many things within the wider ABI. We need to make it more
> >> specific.
> >>
> >> Actually reading the datasheet, I think this is fundamentally about the
> >> maximum sampling frequency rather than directly about the range.
> >> The only reason you'd reduce the range is to speed that up. It doesn't
> >> improve the resolution, the device simply answers quicker.
> >>
> >> So I'd support this as sampling_frequency.  You could then use
> >> the the iio_info_mask_*_available and relevant callback to provide
> >> info on what it then restricts the possible output values to
> >> (rather than controlling it directly).
> >>
> > 
> > By changing the range one cannot influence the sampling frequency directly. I
> > have seen on the oszilloscope that the telegrams arrive almost at the same time
> > with different settings of range and the same gain.
> > 
> > Only if the gain is also adjusted the sensor works faster and a higher frequency
> > can be used. But the gain is also used to adjust the sensitivity of the sensor. 
> That's rather weird and not what the datasheet suggests. Ah well.
> > 
> > What about calling it "sensor_domain" or "sensor_max_range"?
> hmm. Not sure - propose that with appropriate Docs and we can think more on it.
> > 
> > 

I made a mistake in the earlier mentioned measurement which i have to correct:

The telegrams arrive faster with a smaller range and the same gain. 

But we cannot use the sensor at a higher frequency if we are not adjusting the
gain of the sensor as well. This is because of echos of the former cycle which
are still around. And this setting of the gain depends on the surrounding where
the sensor is used. So in the driver we cannot estimate it.

Therefore i'll send out the next version with the attribute name
"sensor_max_range" together with documentation as proposal.

Andreas

-- 

^ permalink raw reply

* [Patch V3] i2c: imx-lpi2c: add VLLS mode support
From: Gao Pan @ 2017-01-17 10:20 UTC (permalink / raw)
  To: wsa, wsa-dev, vz; +Cc: linux-i2c, pandy.gao, frank.li

When system enters VLLS mode, module power is turned off. As a result,
all registers are reset to HW default value. After exiting VLLS mode,
registers are still in default mode. As a result, the pinctrl settings
are incorrect, which will affect the module function.

The patch recovers the pinctrl setting when exit VLLS mode.

Signed-off-by: Gao Pan <pandy.gao@nxp.com>
---
V2:
 -drop "#else branch" and IMX_LPI2C_PM completely
 -pm = &imx_lpi2c_pm

V3:
 -remove unnecessary #ifdef

 drivers/i2c/busses/i2c-imx-lpi2c.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index c62b7cd..5a85c49 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -636,12 +636,31 @@ static int lpi2c_imx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int lpi2c_imx_suspend(struct device *dev)
+{
+	pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
+}
+
+static int lpi2c_imx_resume(struct device *dev)
+{
+	pinctrl_pm_select_default_state(dev);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(imx_lpi2c_pm, lpi2c_imx_suspend, lpi2c_imx_resume);
+
 static struct platform_driver lpi2c_imx_driver = {
 	.probe = lpi2c_imx_probe,
 	.remove = lpi2c_imx_remove,
 	.driver = {
 		.name = DRIVER_NAME,
 		.of_match_table = lpi2c_imx_of_match,
+		.pm = &imx_lpi2c_pm,
 	},
 };
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v3 0/3] iio: distance: srf08: add IIO driver for us ranger
From: Andreas Klinger @ 2017-01-17 13:48 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel, ktsai,
	wsa, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	trivial, mranostay, linux-i2c, devicetree
  Cc: ak

This patch series adds IIO driver support for srf08 ultrasonic ranger
devices.

The first patch add a trivial device tree binding for the device together
with a new vendor devantech.

The second patch is the IIO driver which in turn is using I2C to talk to
the device.

The third patch documents the added userspace ABI

Documentation about the sensor can be found here:

http://www.robot-electronics.co.uk/htm/srf08tech.html

Changes in v3:
Thanks to the reviews of Jonathan, Rob and Lars.
Some changes to the driver were made:

* Patch 2: "iio: distance: srf08: add IIO driver for us ranger"
  - defaults values for sensitivity (gain) and max. range are set in
    probe()
  - while waiting for ultrasonic echo first wait as long as the physically
    measurement need as a function of the max. range and then wait in 
    smaller steps until the response can be read.
  - attributes are now called sensor_sensitivity instead of gain and
    sensor_max_range instead of range_mm
  - device-name is now "srf08"
  - config menu is now calles "Proximity and distance sensors" instead of a
    separate menu

* Patch 3: "iio: distance: srf08: add driver ABI documentation"
  - newly created to document the driver specific interfaces

Changes in v2:
Lots of updates thanks to Peters really fast review within 30 minutes 
after first submission of the driver.

* Patch 2: "iio: distance: srf08: add IIO driver for us ranger"
  - alphabetic order in Makefile
  - use of u8 while accessing registers 
  - avoid endianness problems with 16 bit values
  - missing return value checks
  - some explaining documentation added

Andreas Klinger (3):
  iio: distance: srf08: add trivial DT binding
  iio: distance: srf08: add IIO driver for us ranger
  iio: distance: srf08: add driver ABI documentation

 .../ABI/testing/sysfs-bus-iio-distance-srf08       |  27 ++
 .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/proximity/Kconfig                      |  13 +-
 drivers/iio/proximity/Makefile                     |   1 +
 drivers/iio/proximity/srf08.c                      | 405 +++++++++++++++++++++
 6 files changed, 447 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-distance-srf08
 create mode 100644 drivers/iio/proximity/srf08.c

-- 
2.1.4


-- 

^ permalink raw reply

* [PATCH v3 1/3] iio: distance: srf08: add trivial DT binding
From: Andreas Klinger @ 2017-01-17 13:49 UTC (permalink / raw)
  To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, ktsai-GubuWUlQtMwciDkP5Hr2oA,
	wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, trivial-DgEjT+Ai2ygdnm+yROfE0A,
	mranostay-Re5JQEeQqe8AvxtiuMwx3w,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: ak-n176/SwNRljddJNmlsFzeA

Add DT binding for devantech,srf08
Add vendor devantech to vendor list

Signed-off-by: Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 +
 Documentation/devicetree/bindings/vendor-prefixes.txt     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 539874490492..86c6930c3c91 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -36,6 +36,7 @@ dallas,ds1775		Tiny Digital Thermometer and Thermostat
 dallas,ds3232		Extremely Accurate I²C RTC with Integrated Crystal and SRAM
 dallas,ds4510		CPU Supervisor with Nonvolatile Memory and Programmable I/O
 dallas,ds75		Digital Thermometer and Thermostat
+devantech,srf08		Devantech SRF08 ultrasonic ranger
 dlg,da9053		DA9053: flexible system level PMIC with multicore support
 dlg,da9063		DA9063: system PMIC for quad-core application processors
 epson,rx8010		I2C-BUS INTERFACE REAL TIME CLOCK MODULE
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 4696bb5c2198..80325e602403 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -65,6 +65,7 @@ dallas	Maxim Integrated Products (formerly Dallas Semiconductor)
 davicom	DAVICOM Semiconductor, Inc.
 delta	Delta Electronics, Inc.
 denx	Denx Software Engineering
+devantech	Devantech, Ltd.
 digi	Digi International Inc.
 digilent	Diglent, Inc.
 dlg	Dialog Semiconductor
-- 
2.1.4


-- 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 2/3] iio: distance: srf08: add IIO driver for us ranger
From: Andreas Klinger @ 2017-01-17 13:49 UTC (permalink / raw)
  To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, ktsai-GubuWUlQtMwciDkP5Hr2oA,
	wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, trivial-DgEjT+Ai2ygdnm+yROfE0A,
	mranostay-Re5JQEeQqe8AvxtiuMwx3w,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: ak-n176/SwNRljddJNmlsFzeA

This is the IIO driver for devantech srf08 ultrasonic ranger which can be
used to measure the distances to an object.

The sensor supports I2C with some registers.

Supported Features include:
 - read the distance in ranging mode in centimeters
 - output of the driver is directly the read value
 - together with the scale the driver delivers the distance in meters
 - only the first echo of the nearest object is delivered
 - set sensitivity as analog value in the range of 0-31  means setting 
   gain register on device
 - set range registers; userspace enters max. range in millimeters in 
   43 mm steps

Features not supported by this driver:
 - ranging mode in inches or in microseconds
 - ANN mode
 - change I2C address through this driver
 - light sensor

The driver was added in the directory "proximity" of the iio subsystem and
the menu in den config is now called "Proximity and distance sensors"

Signed-off-by: Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
---
 drivers/iio/proximity/Kconfig  |  13 +-
 drivers/iio/proximity/Makefile |   1 +
 drivers/iio/proximity/srf08.c  | 405 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 418 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iio/proximity/srf08.c

diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index ef4c73db5b53..ab96cb7a0054 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -18,7 +18,7 @@ config AS3935
 
 endmenu
 
-menu "Proximity sensors"
+menu "Proximity and distance sensors"
 
 config LIDAR_LITE_V2
 	tristate "PulsedLight LIDAR sensor"
@@ -45,4 +45,15 @@ config SX9500
 	  To compile this driver as a module, choose M here: the
 	  module will be called sx9500.
 
+config SRF08
+	tristate "Devantech SRF08 ultrasonic ranger sensor"
+	depends on I2C
+	help
+	  Say Y here to build a driver for Devantech SRF08 ultrasonic
+	  ranger sensor. This driver can be used to measure the distance
+	  of objects.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called srf08.
+
 endmenu
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index 9aadd9a8ee99..e914c2a5dd49 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -5,4 +5,5 @@
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_AS3935)		+= as3935.o
 obj-$(CONFIG_LIDAR_LITE_V2)	+= pulsedlight-lidar-lite-v2.o
+obj-$(CONFIG_SRF08)		+= srf08.o
 obj-$(CONFIG_SX9500)		+= sx9500.o
diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
new file mode 100644
index 000000000000..faab6cf3f1a2
--- /dev/null
+++ b/drivers/iio/proximity/srf08.c
@@ -0,0 +1,405 @@
+/*
+ * srf08.c - Support for Devantech SRF08 ultrasonic ranger
+ *
+ * Copyright (c) 2016 Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License.  See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * For details about the device see:
+ * http://www.robot-electronics.co.uk/htm/srf08tech.html
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/bitops.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+/* registers of SRF08 device */
+#define SRF08_WRITE_COMMAND	0x00	/* Command Register */
+#define SRF08_WRITE_MAX_GAIN	0x01	/* Max Gain Register: 0 .. 31 */
+#define SRF08_WRITE_RANGE	0x02	/* Range Register: 0 .. 255 */
+#define SRF08_READ_SW_REVISION	0x00	/* Software Revision */
+#define SRF08_READ_LIGHT	0x01	/* Light Sensor during last echo */
+#define SRF08_READ_ECHO_1_HIGH	0x02	/* Range of first echo received */
+#define SRF08_READ_ECHO_1_LOW	0x03	/* Range of first echo received */
+
+#define SRF08_CMD_RANGING_CM	0x51	/* Ranging Mode - Result in cm */
+
+#define SRF08_DEFAULT_GAIN	1025	/* default analogue value of Gain */
+#define SRF08_DEFAULT_RANGE	6020	/* default value of Range in mm */
+
+struct srf08_data {
+	struct i2c_client	*client;
+	int			sensitivity;		/* Gain */
+	int			range_mm;		/* max. Range in mm */
+	struct mutex		lock;
+};
+
+/*
+ * in the documentation one can read about the "Gain" of the device
+ * which is used here for amplifying the signal and filtering out unwanted
+ * ones.
+ * But with ADC's this term is already used differently and that's why it
+ * is called "Sensitivity" here.
+ */
+static const int srf08_sensitivity[] = {
+	 94,  97, 100, 103, 107, 110, 114, 118,
+	123, 128, 133, 139, 145, 152, 159, 168,
+	177, 187, 199, 212, 227, 245, 265, 288,
+	317, 352, 395, 450, 524, 626, 777, 1025 };
+
+static int srf08_read_ranging(struct srf08_data *data)
+{
+	struct i2c_client *client = data->client;
+	int ret, i;
+	int waittime;
+
+	mutex_lock(&data->lock);
+
+	ret = i2c_smbus_write_byte_data(data->client,
+			SRF08_WRITE_COMMAND, SRF08_CMD_RANGING_CM);
+	if (ret < 0) {
+		dev_err(&client->dev, "write command - err: %d\n", ret);
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+
+	/*
+	 * we read here until a correct version number shows up as
+	 * suggested by the documentation
+	 *
+	 * with an ultrasonic speed of 343 m/s and a roundtrip of it
+	 * sleep the expected duration and try to read from the device
+	 * if nothing useful is read try it in a shorter grid
+	 *
+	 * polling for not more than 20 ms should be enough
+	 */
+	waittime = 1 + data->range_mm / 172;
+	msleep(waittime);
+	for (i = 0; i < 4; i++) {
+		ret = i2c_smbus_read_byte_data(data->client,
+						SRF08_READ_SW_REVISION);
+
+		/* check if a valid version number is read */
+		if (ret < 255 && ret > 0)
+			break;
+		msleep(5);
+	}
+
+	if (ret >= 255 || ret <= 0) {
+		dev_err(&client->dev, "device not ready\n");
+		mutex_unlock(&data->lock);
+		return -EIO;
+	}
+
+	ret = i2c_smbus_read_word_swapped(data->client,
+						SRF08_READ_ECHO_1_HIGH);
+	if (ret < 0) {
+		dev_err(&client->dev, "cannot read distance: ret=%d\n", ret);
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int srf08_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *channel, int *val,
+			    int *val2, long mask)
+{
+	struct srf08_data *data = iio_priv(indio_dev);
+	int ret;
+
+	if (channel->type != IIO_DISTANCE)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = srf08_read_ranging(data);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/* 1 LSB is 1 cm */
+		*val = 0;
+		*val2 = 10000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t srf08_show_range_mm_available(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int i, len = 0;
+
+	for (i = 0; i < 256; i++)
+		len += scnprintf(buf + len, PAGE_SIZE - len,
+							"%d ", (i + 1) * 43);
+
+	buf[len - 1] = '\n';
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(sensor_max_range_available, S_IRUGO,
+				srf08_show_range_mm_available, NULL, 0);
+
+static ssize_t srf08_show_range_mm(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct srf08_data *data = iio_priv(indio_dev);
+
+	return sprintf(buf, "%d\n", data->range_mm);
+}
+
+/*
+ * set the range of the sensor to an even multiple of 43 mm
+ * which corresponds to 1 LSB in the register
+ *
+ * register value    corresponding range
+ *         0x00             43 mm
+ *         0x01             86 mm
+ *         0x02            129 mm
+ *         ...
+ *         0xFF          11008 mm
+ */
+static ssize_t srf08_write_range_mm(struct srf08_data *data, unsigned int val)
+{
+	int ret;
+	struct i2c_client *client = data->client;
+	unsigned int mod;
+	u8 regval;
+
+	ret = val / 43 - 1;
+	mod = val % 43;
+
+	if (mod || (ret < 0) || (ret > 255))
+		return -EINVAL;
+
+	regval = ret;
+
+	mutex_lock(&data->lock);
+
+	ret = i2c_smbus_write_byte_data(client, SRF08_WRITE_RANGE, regval);
+	if (ret < 0) {
+		dev_err(&client->dev, "write_range - err: %d\n", ret);
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+
+	data->range_mm = val;
+
+	mutex_unlock(&data->lock);
+
+	return 0;
+}
+
+static ssize_t srf08_store_range_mm(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct srf08_data *data = iio_priv(indio_dev);
+	int ret;
+	unsigned int val;
+
+	ret = kstrtouint(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	ret = srf08_write_range_mm(data, val);
+	if (ret < 0)
+		return ret;
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(sensor_max_range, S_IRUGO | S_IWUSR,
+			srf08_show_range_mm, srf08_store_range_mm, 0);
+
+static ssize_t srf08_show_sensitivity_available(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int i, len = 0;
+
+	for (i = 0; i < ARRAY_SIZE(srf08_sensitivity); i++)
+		len += sprintf(buf + len, "%d ", srf08_sensitivity[i]);
+
+	len += sprintf(buf + len, "\n");
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(sensor_sensitivity_available, S_IRUGO,
+				srf08_show_sensitivity_available, NULL, 0);
+
+static ssize_t srf08_show_sensitivity(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct srf08_data *data = iio_priv(indio_dev);
+	int len;
+
+	len = sprintf(buf, "%d\n", data->sensitivity);
+
+	return len;
+}
+
+static ssize_t srf08_write_sensitivity(struct srf08_data *data,
+							unsigned int val)
+{
+	struct i2c_client *client = data->client;
+	int ret, i;
+	u8 regval;
+
+	for (i = 0; i < ARRAY_SIZE(srf08_sensitivity); i++)
+		if (val == srf08_sensitivity[i]) {
+			regval = i;
+			break;
+		}
+
+	if (i >= ARRAY_SIZE(srf08_sensitivity))
+		return -EINVAL;
+
+	mutex_lock(&data->lock);
+
+	ret = i2c_smbus_write_byte_data(client,
+						SRF08_WRITE_MAX_GAIN, regval);
+	if (ret < 0) {
+		dev_err(&client->dev, "write_sensitivity - err: %d\n", ret);
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+
+	data->sensitivity = val;
+
+	mutex_unlock(&data->lock);
+
+	return 0;
+}
+
+static ssize_t srf08_store_sensitivity(struct device *dev,
+						struct device_attribute *attr,
+						const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct srf08_data *data = iio_priv(indio_dev);
+	int ret;
+	unsigned int val;
+
+	ret = kstrtouint(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	ret = srf08_write_sensitivity(data, val);
+	if (ret < 0)
+		return ret;
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(sensor_sensitivity, S_IRUGO | S_IWUSR,
+			srf08_show_sensitivity, srf08_store_sensitivity, 0);
+
+static struct attribute *srf08_attributes[] = {
+	&iio_dev_attr_sensor_max_range.dev_attr.attr,
+	&iio_dev_attr_sensor_max_range_available.dev_attr.attr,
+	&iio_dev_attr_sensor_sensitivity.dev_attr.attr,
+	&iio_dev_attr_sensor_sensitivity_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group srf08_attribute_group = {
+	.attrs = srf08_attributes,
+};
+
+static const struct iio_chan_spec srf08_channels[] = {
+	{
+		.type = IIO_DISTANCE,
+		.info_mask_separate =
+				BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+static const struct iio_info srf08_info = {
+	.read_raw = srf08_read_raw,
+	.attrs = &srf08_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
+static int srf08_probe(struct i2c_client *client,
+					 const struct i2c_device_id *id)
+{
+	struct iio_dev *indio_dev;
+	struct srf08_data *data;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter,
+					I2C_FUNC_SMBUS_READ_BYTE_DATA |
+					I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+					I2C_FUNC_SMBUS_READ_WORD_DATA))
+		return -ENODEV;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	indio_dev->name = "srf08";
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &srf08_info;
+	indio_dev->channels = srf08_channels;
+	indio_dev->num_channels = ARRAY_SIZE(srf08_channels);
+
+	mutex_init(&data->lock);
+
+	/*
+	 * set default values of device here
+	 * these register values cannot be read from the hardware
+	 * therefore set driver specific default values
+	 */
+	ret = srf08_write_sensitivity(data, SRF08_DEFAULT_GAIN);
+	if (ret < 0)
+		return ret;
+
+	ret = srf08_write_range_mm(data, SRF08_DEFAULT_RANGE);
+	if (ret < 0)
+		return ret;
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id srf08_id[] = {
+	{ "srf08", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, srf08_id);
+
+static struct i2c_driver srf08_driver = {
+	.driver = {
+		.name	= "srf08",
+	},
+	.probe = srf08_probe,
+	.id_table = srf08_id,
+};
+module_i2c_driver(srf08_driver);
+
+MODULE_AUTHOR("Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>");
+MODULE_DESCRIPTION("Devantech SRF08 ultrasonic ranger driver");
+MODULE_LICENSE("GPL");
-- 
2.1.4


-- 

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox