* [PATCH 2/2] PCI: rockchip: Add quirk to disable RC's ASPM L0s
From: Shawn Lin @ 2016-11-13 3:06 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Rob Herring, linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Wenrui Li,
Brian Norris, Jeffy Chen, devicetree-u79uwXL29TY76Z2rM5mHXA,
Shawn Lin
In-Reply-To: <1479006374-192306-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Rockchip's RC outputs 100MHz reference clock but there are
two methods for PHY to generate it.
(1)One of them is to use system PLL to generate 100MHz clock and
the PHY will relock it and filter signal noise then outputs the
reference clock.
(2)Another way is to share Soc's 24MHZ crystal oscillator with
PHY and force PHY's DLL to generate 100MHz internally.
When using case(2), the exit from L0s doesn't work fine occasionally
due to the broken design of RC receiver's logical circuit. So even if
we use extended-synch, it still fails for PHY to relock the bits from
FTS sometimes. This will hang the system.
Maybe we could argue that why not use case(1) to avoid it? The reason
is that as we could see the reference clock is derived from system PLL
and the path from it to PHY isn't so clean which means there are some
noise introduced by power-domain and other buses can't be filterd out
by PHY and we could see noise from the frequency spectrum by oscilloscope.
This makes the TX compatibility test a little difficult to pass the spec.
So case(1) and case(2) are both used indeed now. If using case(2), we
should disable RC's L0s support, and that is why we need this property to
indicate this quirk.
Also after checking quirk.c, I noticed there is already a quirk for
disabling L0s unconditionally, quirk_disable_aspm_l0s. But obviously we
shouldn't do that as mentioned above that case(1) could still works fine
with L0s.
Reported-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Cc: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
Documentation/devicetree/bindings/pci/rockchip-pcie.txt | 2 ++
drivers/pci/host/pcie-rockchip.c | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
index ba67b39..cfa44a7 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
@@ -42,6 +42,8 @@ Required properties:
Optional Property:
- ep-gpios: contain the entry for pre-reset gpio
- num-lanes: number of lanes to use
+- quirk,aspm-no-l0s: RC won't support ASPM L0s. This property is needed if
+ using 24MHz OSC for RC's PHY.
- vpcie3v3-supply: The phandle to the 3.3v regulator to use for PCIe.
- vpcie1v8-supply: The phandle to the 1.8v regulator to use for PCIe.
- vpcie0v9-supply: The phandle to the 0.9v regulator to use for PCIe.
diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 1dba698..e0d8055 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -140,6 +140,8 @@
#define PCIE_RC_CONFIG_DCR_CSPL_SHIFT 18
#define PCIE_RC_CONFIG_DCR_CSPL_LIMIT 0xff
#define PCIE_RC_CONFIG_DCR_CPLS_SHIFT 26
+#define PCIE_RC_CONFIG_LINK_CAP (PCIE_RC_CONFIG_BASE + 0xcc)
+#define PCIE_RC_CONFIG_LINK_CAP_L0S BIT(10)
#define PCIE_RC_CONFIG_LCS (PCIE_RC_CONFIG_BASE + 0xd0)
#define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
#define PCIE_RC_CONFIG_THP_CAP (PCIE_RC_CONFIG_BASE + 0x274)
@@ -607,6 +609,13 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
status &= ~PCIE_RC_CONFIG_THP_CAP_NEXT_MASK;
rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_THP_CAP);
+ /* Clear L0s from RC's link cap */
+ if (of_property_read_bool(node, "quirk,apsm-no-l0s")) {
+ status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LINK_CAP);
+ status &= ~PCIE_RC_CONFIG_LINK_CAP_L0S;
+ rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LINK_CAP);
+ }
+
rockchip_pcie_write(rockchip, 0x0, PCIE_RC_BAR_CONF);
rockchip_pcie_write(rockchip,
--
1.9.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 1/2] PCI: rockchip: cleanup bit definition for PCIE_RC_CONFIG_LCS
From: Shawn Lin @ 2016-11-13 3:06 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Rob Herring, linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Wenrui Li,
Brian Norris, Jeffy Chen, devicetree-u79uwXL29TY76Z2rM5mHXA,
Shawn Lin
PCIE_RC_CONFIG_LCS contains control and status bits specific
to the PCIe link. The layout for this register looks the same
as the existed PCI_EXP_LNKCTL and PCI_EXP_LNKSTA. So let's
reuse them.
Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
drivers/pci/host/pcie-rockchip.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 7f238af..1dba698 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -141,12 +141,6 @@
#define PCIE_RC_CONFIG_DCR_CSPL_LIMIT 0xff
#define PCIE_RC_CONFIG_DCR_CPLS_SHIFT 26
#define PCIE_RC_CONFIG_LCS (PCIE_RC_CONFIG_BASE + 0xd0)
-#define PCIE_RC_CONFIG_LCS_RETRAIN_LINK BIT(5)
-#define PCIE_RC_CONFIG_LCS_CCC BIT(6)
-#define PCIE_RC_CONFIG_LCS_LBMIE BIT(10)
-#define PCIE_RC_CONFIG_LCS_LABIE BIT(11)
-#define PCIE_RC_CONFIG_LCS_LBMS BIT(30)
-#define PCIE_RC_CONFIG_LCS_LAMS BIT(31)
#define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
#define PCIE_RC_CONFIG_THP_CAP (PCIE_RC_CONFIG_BASE + 0x274)
#define PCIE_RC_CONFIG_THP_CAP_NEXT_MASK GENMASK(31, 20)
@@ -229,7 +223,7 @@ static void rockchip_pcie_enable_bw_int(struct rockchip_pcie *rockchip)
u32 status;
status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
- status |= (PCIE_RC_CONFIG_LCS_LBMIE | PCIE_RC_CONFIG_LCS_LABIE);
+ status |= (PCI_EXP_LNKCTL_LBMIE | PCI_EXP_LNKCTL_LABIE);
rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
}
@@ -238,7 +232,7 @@ static void rockchip_pcie_clr_bw_int(struct rockchip_pcie *rockchip)
u32 status;
status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
- status |= (PCIE_RC_CONFIG_LCS_LBMS | PCIE_RC_CONFIG_LCS_LAMS);
+ status |= (PCI_EXP_LNKSTA_LBMS | PCI_EXP_LNKSTA_LABS) << 16;
rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
}
@@ -540,7 +534,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
/* Set RC's clock architecture as common clock */
status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
- status |= PCIE_RC_CONFIG_LCS_CCC;
+ status |= PCI_EXP_LNKCTL_CCC;
rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
/* Enable Gen1 training */
@@ -575,7 +569,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
* gen1 finished.
*/
status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
- status |= PCIE_RC_CONFIG_LCS_RETRAIN_LINK;
+ status |= PCI_EXP_LNKCTL_RL;
rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
timeout = jiffies + msecs_to_jiffies(500);
--
1.9.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
* Re: [PATCH v2 2/2] of: changesets: Introduce changeset helper methods
From: Frank Rowand @ 2016-11-13 2:15 UTC (permalink / raw)
To: Hans de Goede, Rob Herring, Pantelis Antoniou
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161104144241.18002-2-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On 11/04/16 07:42, Hans de Goede wrote:
> From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>
> Changesets are very powerful, but the lack of a helper API
> makes using them cumbersome. Introduce a simple copy based
> API that makes things considerably easier.
>
> To wit, adding a property using the raw API.
>
> struct property *prop;
> prop = kzalloc(sizeof(*prop)), GFP_KERNEL);
> prop->name = kstrdup("compatible");
> prop->value = kstrdup("foo,bar");
> prop->length = strlen(prop->value) + 1;
> of_changeset_add_property(ocs, np, prop);
>
> while using the helper API
>
> of_changeset_add_property_string(ocs, np, "compatible",
> "foo,bar");
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v2 (hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
> -Address review comments from:
> https://www.spinics.net/lists/kernel/msg2252845.html
That points to the May 9 version 1 patches from Pantelis (as expected),
but containing 4, not 2, patches. Patch 1/4 was applied. Patch 4/4
seems to have disappeared?
Pantelis then sent a version 2 set of the patches on May 16.
Your version is a modification of the May 9 patches (as would be expected
of a version 2). It is confusing to have two different version 2 patch
sets. I don't have any brilliant ideas on how this patch set could have
been named differently to avoid that confusion.
The point of this little side-track is simply to note the existence of two
different version 2 series so I won't be confused when I revisit this
thread in the future.
> -Simplify (and fix) __of_changeset_add_update_property_copy OOM handling
> -Remove (by manual inlining) these 2 static helpers:
> __of_changeset_add_update_property_u32
> __of_changeset_add_update_property_bool
> -Remove the following exported helper method:
> of_changeset_node_move_to
Not all comments were addressed.
There are some other changes made that are not noted in the changelog.
I am still reading through the patches. I will reply again either with
a reviewed-by or specific comments when I finish.
-Frank
> ---
> drivers/of/dynamic.c | 428 +++++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/of.h | 135 ++++++++++++++++
> 2 files changed, 563 insertions(+)
< snip >
--
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 v7 4/9] drivers:input:tsc2007: add iio interface to read external ADC input and temperature
From: H. Nikolaus Schaller @ 2016-11-12 19:22 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Sebastian Reichel, Dmitry Torokhov, Mark Rutland,
Benoît Cousson, Tony Lindgren, Russell King, Arnd Bergmann,
Michael Welling, Mika Penttilä, Javier Martinez Canillas,
Igor Grinberg, Andrew F. Davis, Mark Brown, linux-input,
devicetree, linux-kernel, linux-omap, letux-kernel, linux-iio,
kernel
In-Reply-To: <b4d79a26-4353-9860-a884-49c751bbe617@kernel.org>
> Am 12.11.2016 um 15:19 schrieb Jonathan Cameron <jic23@kernel.org>:
>
> On 12/11/16 14:04, Jonathan Cameron wrote:
>> On 11/11/16 19:02, H. Nikolaus Schaller wrote:
>>> The tsc2007 chip not only has a resistive touch screen controller but
>>> also an external AUX adc imput which can be used for an ambient
>>> light sensor, battery voltage monitoring or any general purpose.
>>>
>>> Additionally it can measure the chip temperature.
>>>
>>> This extension provides an iio interface for these adc channels.
>>>
>>> Since it is not wasting much resources and is very straightforward,
>>> we simply provide all other adc channels as optional iio interfaces
>>> as weel. This can be used for debugging or special applications.
>>>
>>> This patch also splits the tsc2007 driver in several source files:
>>> tsc2007.h -- constants, structs and stubs
>>> tsc2007_core.c -- functional parts of the original driver
>>> tsc2007_iio.c -- the optional iio stuff
>>>
>>> Makefile magic allows to conditionally link the iio
>>> stuff if CONFIG_IIO=y in a way that it works with
>>> CONFIG_TOUCHSCREEN_TSC2007=m.
>>>
>>
>> I ran a quick build test and it blows up in a number of ways related to
>> the #ifdef CONFIG_IIO.
>>
>> I'm not entirely sure why but
>> #if IS_ENABLED(CONFIG_IIO) works fine.
> Ah, I'm being sleepy today. It's because I'm building IIO as a module
> and the symbol defined would therefore be CONFIG_IIO_MODULE.
>
> The IS_ENABLED macro takes care of both cases.
Ah, yes. I wasn't aware that CONFIG_IIO is tristate at all.
We have it as "y" and therefore didn't notice.
I will collect this fix for V8.
>>
>> Otherwise looks good to me
>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> Reviewed-by: Jonathan Cameron <jic23@kernel.org>
Will add this as well.
Thanks for the detailled review!
BR,
Nikolaus
^ permalink raw reply
* Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator
From: Jonathan Cameron @ 2016-11-12 17:29 UTC (permalink / raw)
To: Peter Rosin, Thomas Gleixner
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
Mark Rutland, Daniel Baluta, Slawomir Stepien,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <9fa01d3d-74ba-a048-52bf-4df959153354-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 11/11/16 11:37, Peter Rosin wrote:
> On 2016-11-09 16:06, Thomas Gleixner wrote:
>> On Wed, 9 Nov 2016, Peter Rosin wrote:
>>> On 2016-11-08 22:47, Thomas Gleixner wrote:
>>>> I don't think you need extra race handling with that, but I might be wrong
>>>> as usual.
>>>
>>> There's obviously no way to determine which of the timeout or the
>>> interrupt that happens first without some race handling, so I don't
>>> know what you mean? If the timeout happens first, there is also a
>>> need to handle late hits from the irq that might come in during the
>>> preparation for the next step in the binary search. It gets messy
>>> quickly compared to the simplicity of the current implementation.
>>
>> Gah, forgot about that timeout thingy. Fair enough.
>>
>> Feel free to add an
>>
>> Acked-by: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>
> Thanks for looking!
Applied to the togreg branch of iio.git and pushed out
as testing for the autobuilders to play with.
Excellent patch set.
Thanks,
Jonathan
>
> Cheers,
> Peter
>
--
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 v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator
From: Jonathan Cameron @ 2016-11-12 17:27 UTC (permalink / raw)
To: Peter Rosin, Thomas Gleixner
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
Mark Rutland, Daniel Baluta, Slawomir Stepien,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d8237286-d2ff-3146-d718-f54d05b0101d-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 08/11/16 17:03, Peter Rosin wrote:
> On 2016-11-08 16:59, Thomas Gleixner wrote:
>> On Tue, 8 Nov 2016, Peter Rosin wrote:
>>> +/*
>>> + * The envelope_detector_comp_latch function works together with the compare
>>> + * interrupt service routine below (envelope_detector_comp_isr) as a latch
>>> + * (one-bit memory) for if the interrupt has triggered since last calling
>>> + * this function.
>>> + * The ..._comp_isr function disables the interrupt so that the cpu does not
>>> + * need to service a possible interrupt flood from the comparator when no-one
>>> + * cares anyway, and this ..._comp_latch function reenables them again if
>>> + * needed.
>>> + */
>>> +static int envelope_detector_comp_latch(struct envelope *env)
>>> +{
>>> + int comp;
>>> +
>>> + spin_lock_irq(&env->comp_lock);
>>> + comp = env->comp;
>>> + env->comp = 0;
>>> + spin_unlock_irq(&env->comp_lock);
>>> +
>>> + if (!comp)
>>> + return 0;
>>> +
>>> + /*
>>> + * The irq was disabled, and is reenabled just now.
>>> + * But there might have been a pending irq that
>>> + * happened while the irq was disabled that fires
>>> + * just as the irq is reenabled. That is not what
>>> + * is desired.
>>> + */
>>> + enable_irq(env->comp_irq);
>>> +
>>> + /* So, synchronize this possibly pending irq... */
>>> + synchronize_irq(env->comp_irq);
>>> +
>>> + /* ...and redo the whole dance. */
>>> + spin_lock_irq(&env->comp_lock);
>>> + comp = env->comp;
>>> + env->comp = 0;
>>> + spin_unlock_irq(&env->comp_lock);
>>> +
>>> + if (comp)
>>> + enable_irq(env->comp_irq);
>>
>> So you need that whole dance including the delayed work because you cannot
>> call iio_write_channel_raw() from hard interrupt context, right?
>
> It's not the "cannot call from hard irq context" that made me do that, it's...
>
>> So you might just register a threaded interrupt handler, which should make
>> this whole thing way simpler.
>>
>> devm_request_threaded_irq(dev, irq, NULL, your_isr, IRQF_ONESHOT, ...);
>>
>> The core will mask the interrupt line until the threaded handler is
>> finished. The threaded handler is invoked with preemption enabled, so you
>> can sleep there as long as you want. So you can do everything in your
>> handler and the above dance is just not required.
>
> ...that I couldn't work out how to reenable a oneshot irq once it had fired,
> short of freeing the irq and requesting it again. That seemed entirely
> bogus, the driver shouldn't risk losing a resource like that so I don't know
> what I didn't see? Or maybe it was that I had a hard time resolving the race
> between the irq and the timeout in a nice way. I honestly don't remember
> why exactly I abandoned oneshot irqs, but this enable/sync/enable dance
> was much nicer than what I came up with for the oneshot irq solution I
> originally worked on.
>
> Or maybe I had problems with the possibly pending irq also when using a
> oneshot irq, but didn't realize it? That was something I discovered quite
> late in the process, some time after moving away from oneshot irqs. Are
> pending irqs cleared when requesting (or reenabling, however that is done)
> a oneshot irq?
>
> Anyway, I do not want the interrupt to be serviced when no one is interested,
> since I'm afraid that nasty input might generate a flood of interrupts that
> might disturb other things that the cpu is doing. Which means that I need
> to enable/disable the interrupt as needed.
>
> However, what *I* thought Jonathan wanted input on was the part where the
> interrupt edge/level is flipped when requesting "inverted" signals in
> envelope_store_invert(). That could perhaps be seen as unorthodox and in
> need of more eyes?
Nope, as far as I can recall it was precisely this dance that was
I wanted Thomas to comment on :) The inverted bit isn't as novel as
this ;)
Anyhow, thread ended up with a good conclusion so I'm happy.
Jonathan
>
> Cheers,
> Peter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" 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 v4 7/8] dt-bindings: iio: document envelope-detector bindings
From: Jonathan Cameron @ 2016-11-12 17:25 UTC (permalink / raw)
To: Peter Rosin, linux-kernel
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Rob Herring, Mark Rutland, Daniel Baluta, Slawomir Stepien,
Thomas Gleixner, linux-iio, devicetree
In-Reply-To: <1478606339-31253-8-git-send-email-peda@axentia.se>
On 08/11/16 11:58, Peter Rosin wrote:
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Applied.
> ---
> .../bindings/iio/adc/envelope-detector.txt | 54 ++++++++++++++++++++++
> MAINTAINERS | 6 +++
> 2 files changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
> new file mode 100644
> index 000000000000..27544bdd4478
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
> @@ -0,0 +1,54 @@
> +Bindings for ADC envelope detector using a DAC and a comparator
> +
> +The DAC is used to find the peak level of an alternating voltage input
> +signal by a binary search using the output of a comparator wired to
> +an interrupt pin. Like so:
> + _
> + | \
> + input +------>-------|+ \
> + | \
> + .-------. | }---.
> + | | | / |
> + | dac|-->--|- / |
> + | | |_/ |
> + | | |
> + | | |
> + | irq|------<-------'
> + | |
> + '-------'
> +
> +Required properties:
> +- compatible: Should be "axentia,tse850-envelope-detector"
> +- io-channels: Channel node of the dac to be used for comparator input.
> +- io-channel-names: Should be "dac".
> +- interrupt specification for one client interrupt,
> + see ../../interrupt-controller/interrupts.txt for details.
> +- interrupt-names: Should be "comp".
> +
> +Example:
> +
> + &i2c {
> + dpot: mcp4651-104@28 {
> + compatible = "microchip,mcp4651-104";
> + reg = <0x28>;
> + #io-channel-cells = <1>;
> + };
> + };
> +
> + dac: dac {
> + compatible = "dpot-dac";
> + vref-supply = <®_3v3>;
> + io-channels = <&dpot 0>;
> + io-channel-names = "dpot";
> + #io-channel-cells = <1>;
> + };
> +
> + envelope-detector {
> + compatible = "axentia,tse850-envelope-detector";
> + io-channels = <&dac 0>;
> + io-channel-names = "dac";
> +
> + interrupt-parent = <&gpio>;
> + interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> + interrupt-names = "comp";
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 583c6c93b6f3..0e13066ca3a2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6132,6 +6132,12 @@ F: Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
> F: Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
> F: drivers/iio/dac/dpot-dac.c
>
> +IIO ENVELOPE DETECTOR
> +M: Peter Rosin <peda@axentia.se>
> +L: linux-iio@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
> +
> IIO SUBSYSTEM AND DRIVERS
> M: Jonathan Cameron <jic23@kernel.org>
> R: Hartmut Knaack <knaack.h@gmx.de>
>
^ permalink raw reply
* Re: [PATCH v4 6/8] iio: dpot-dac: DAC driver based on a digital potentiometer
From: Jonathan Cameron @ 2016-11-12 17:24 UTC (permalink / raw)
To: Peter Rosin, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Rob Herring, Mark Rutland, Daniel Baluta, Slawomir Stepien,
Thomas Gleixner, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478606339-31253-7-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 08/11/16 11:58, Peter Rosin wrote:
> It is assumed that the dpot is used as a voltage divider between the
> current dpot wiper setting and the maximum resistance of the dpot. The
> divided voltage is provided by a vref regulator.
>
> .------.
> .-----------. | |
> | vref |--' .---.
> | regulator |--. | |
> '-----------' | | d |
> | | p |
> | | o | wiper
> | | t |<---------+
> | | |
> | '---' dac output voltage
> | |
> '------+------------+
>
> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Applied to the togreg branch of iio.git - shortly to be pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> .../ABI/testing/sysfs-bus-iio-dac-dpot-dac | 8 +
> MAINTAINERS | 2 +
> drivers/iio/dac/Kconfig | 10 +
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/dpot-dac.c | 266 +++++++++++++++++++++
> 5 files changed, 287 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
> create mode 100644 drivers/iio/dac/dpot-dac.c
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac b/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
> new file mode 100644
> index 000000000000..580e93f373f6
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
> @@ -0,0 +1,8 @@
> +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw_available
> +Date: October 2016
> +KernelVersion: 4.9
> +Contact: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
> +Description:
> + The range of available values represented as the minimum value,
> + the step and the maximum value, all enclosed in square brackets.
> + Example: [0 1 256]
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4ff9c12b1313..583c6c93b6f3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6128,7 +6128,9 @@ IIO DIGITAL POTENTIOMETER DAC
> M: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
> L: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> S: Maintained
> +F: Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
> F: Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
> +F: drivers/iio/dac/dpot-dac.c
>
> IIO SUBSYSTEM AND DRIVERS
> M: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 120b24478469..d3084028905b 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -200,6 +200,16 @@ config AD8801
> To compile this driver as a module choose M here: the module will be called
> ad8801.
>
> +config DPOT_DAC
> + tristate "DAC emulation using a DPOT"
> + depends on OF
> + help
> + Say yes here to build support for DAC emulation using a digital
> + potentiometer.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called dpot-dac.
> +
> config LPC18XX_DAC
> tristate "NXP LPC18xx DAC driver"
> depends on ARCH_LPC18XX || COMPILE_TEST
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 27642bbf75f2..f01bf4a99867 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_AD5686) += ad5686.o
> obj-$(CONFIG_AD7303) += ad7303.o
> obj-$(CONFIG_AD8801) += ad8801.o
> obj-$(CONFIG_CIO_DAC) += cio-dac.o
> +obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
> obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
> obj-$(CONFIG_M62332) += m62332.o
> obj-$(CONFIG_MAX517) += max517.o
> diff --git a/drivers/iio/dac/dpot-dac.c b/drivers/iio/dac/dpot-dac.c
> new file mode 100644
> index 000000000000..eb1f0509a3ec
> --- /dev/null
> +++ b/drivers/iio/dac/dpot-dac.c
> @@ -0,0 +1,266 @@
> +/*
> + * IIO DAC emulation driver using a digital potentiometer
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * Author: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
> + *
> + * 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.
> + */
> +
> +/*
> + * It is assumed that the dpot is used as a voltage divider between the
> + * current dpot wiper setting and the maximum resistance of the dpot. The
> + * divided voltage is provided by a vref regulator.
> + *
> + * .------.
> + * .-----------. | |
> + * | vref |--' .---.
> + * | regulator |--. | |
> + * '-----------' | | d |
> + * | | p |
> + * | | o | wiper
> + * | | t |<---------+
> + * | | |
> + * | '---' dac output voltage
> + * | |
> + * '------+------------+
> + */
> +
> +#include <linux/err.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +
> +struct dpot_dac {
> + struct regulator *vref;
> + struct iio_channel *dpot;
> + u32 max_ohms;
> +};
> +
> +static const struct iio_chan_spec dpot_dac_iio_channel = {
> + .type = IIO_VOLTAGE,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
> + | BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW),
> + .output = 1,
> + .indexed = 1,
> +};
> +
> +static int dpot_dac_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct dpot_dac *dac = iio_priv(indio_dev);
> + int ret;
> + unsigned long long tmp;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return iio_read_channel_raw(dac->dpot, val);
> +
> + case IIO_CHAN_INFO_SCALE:
> + ret = iio_read_channel_scale(dac->dpot, val, val2);
> + switch (ret) {
> + case IIO_VAL_FRACTIONAL_LOG2:
> + tmp = *val * 1000000000LL;
> + do_div(tmp, dac->max_ohms);
> + tmp *= regulator_get_voltage(dac->vref) / 1000;
> + do_div(tmp, 1000000000LL);
> + *val = tmp;
> + return ret;
> + case IIO_VAL_INT:
> + /*
> + * Convert integer scale to fractional scale by
> + * setting the denominator (val2) to one...
> + */
> + *val2 = 1;
> + ret = IIO_VAL_FRACTIONAL;
> + /* ...and fall through. */
> + case IIO_VAL_FRACTIONAL:
> + *val *= regulator_get_voltage(dac->vref) / 1000;
> + *val2 *= dac->max_ohms;
> + break;
> + }
> +
> + return ret;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int dpot_dac_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long mask)
> +{
> + struct dpot_dac *dac = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + *type = IIO_VAL_INT;
> + return iio_read_avail_channel_raw(dac->dpot, vals, length);
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int dpot_dac_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct dpot_dac *dac = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return iio_write_channel_raw(dac->dpot, val);
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct iio_info dpot_dac_info = {
> + .read_raw = dpot_dac_read_raw,
> + .read_avail = dpot_dac_read_avail,
> + .write_raw = dpot_dac_write_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static int dpot_dac_channel_max_ohms(struct iio_dev *indio_dev)
> +{
> + struct device *dev = &indio_dev->dev;
> + struct dpot_dac *dac = iio_priv(indio_dev);
> + unsigned long long tmp;
> + int ret;
> + int val;
> + int val2;
> + int max;
> +
> + ret = iio_read_max_channel_raw(dac->dpot, &max);
> + if (ret < 0) {
> + dev_err(dev, "dpot does not indicate its raw maximum value\n");
> + return ret;
> + }
> +
> + switch (iio_read_channel_scale(dac->dpot, &val, &val2)) {
> + case IIO_VAL_INT:
> + return max * val;
> + case IIO_VAL_FRACTIONAL:
> + tmp = (unsigned long long)max * val;
> + do_div(tmp, val2);
> + return tmp;
> + case IIO_VAL_FRACTIONAL_LOG2:
> + tmp = val * 1000000000LL * max >> val2;
> + do_div(tmp, 1000000000LL);
> + return tmp;
> + default:
> + dev_err(dev, "dpot has a scale that is too weird\n");
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int dpot_dac_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct iio_dev *indio_dev;
> + struct dpot_dac *dac;
> + enum iio_chan_type type;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*dac));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, indio_dev);
> + dac = iio_priv(indio_dev);
> +
> + indio_dev->name = dev_name(dev);
> + indio_dev->dev.parent = dev;
> + indio_dev->info = &dpot_dac_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = &dpot_dac_iio_channel;
> + indio_dev->num_channels = 1;
> +
> + dac->vref = devm_regulator_get(dev, "vref");
> + if (IS_ERR(dac->vref)) {
> + if (PTR_ERR(dac->dpot) != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "failed to get vref regulator\n");
> + return PTR_ERR(dac->vref);
> + }
> +
> + dac->dpot = devm_iio_channel_get(dev, "dpot");
> + if (IS_ERR(dac->dpot)) {
> + if (PTR_ERR(dac->dpot) != -EPROBE_DEFER)
> + dev_err(dev, "failed to get dpot input channel\n");
> + return PTR_ERR(dac->dpot);
> + }
> +
> + ret = iio_get_channel_type(dac->dpot, &type);
> + if (ret < 0)
> + return ret;
> +
> + if (type != IIO_RESISTANCE) {
> + dev_err(dev, "dpot is of the wrong type\n");
> + return -EINVAL;
> + }
> +
> + ret = dpot_dac_channel_max_ohms(indio_dev);
> + if (ret < 0)
> + return ret;
> + dac->max_ohms = ret;
> +
> + ret = regulator_enable(dac->vref);
> + if (ret) {
> + dev_err(dev, "failed to enable the vref regulator\n");
> + return ret;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(dev, "failed to register iio device\n");
> + goto disable_reg;
> + }
> +
> + return 0;
> +
> +disable_reg:
> + regulator_disable(dac->vref);
> + return ret;
> +}
> +
> +static int dpot_dac_remove(struct platform_device *pdev)
> +{
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct dpot_dac *dac = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
> + regulator_disable(dac->vref);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id dpot_dac_match[] = {
> + { .compatible = "dpot-dac" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, dpot_dac_match);
> +
> +static struct platform_driver dpot_dac_driver = {
> + .probe = dpot_dac_probe,
> + .remove = dpot_dac_remove,
> + .driver = {
> + .name = "iio-dpot-dac",
> + .of_match_table = dpot_dac_match,
> + },
> +};
> +module_platform_driver(dpot_dac_driver);
> +
> +MODULE_DESCRIPTION("DAC emulation driver using a digital potentiometer");
> +MODULE_AUTHOR("Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply
* Re: [PATCH v4 4/8] dt-bindings: add axentia to vendor-prefixes
From: Jonathan Cameron @ 2016-11-12 17:23 UTC (permalink / raw)
To: Peter Rosin, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Rob Herring, Mark Rutland, Daniel Baluta, Slawomir Stepien,
Thomas Gleixner, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478606339-31253-5-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 08/11/16 11:58, Peter Rosin wrote:
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Applied.
> ---
> 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 f0a48ea78659..a437120a7eee 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -39,6 +39,7 @@ auo AU Optronics Corporation
> auvidea Auvidea GmbH
> avago Avago Technologies
> avic Shanghai AVIC Optoelectronics Co., Ltd.
> +axentia Axentia Technologies AB
> axis Axis Communications AB
> boe BOE Technology Group Co., Ltd.
> bosch Bosch Sensortec GmbH
>
--
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 v4 3/8] iio: mcp4531: provide range of available raw values
From: Jonathan Cameron @ 2016-11-12 17:22 UTC (permalink / raw)
To: Peter Rosin, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Rob Herring, Mark Rutland, Daniel Baluta, Slawomir Stepien,
Thomas Gleixner, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478606339-31253-4-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 08/11/16 11:58, Peter Rosin wrote:
> Example:
>
> $ cat '/sys/bus/iio/devices/iio:device0/out_resistance_raw_available'
> [0 1 256]
>
> Meaning: min 0, step 1 and max 256.
>
> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Applied.
> ---
> .../testing/sysfs-bus-iio-potentiometer-mcp4531 | 8 ++
> MAINTAINERS | 1 +
> drivers/iio/potentiometer/mcp4531.c | 104 ++++++++++++---------
> 3 files changed, 71 insertions(+), 42 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531 b/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
> new file mode 100644
> index 000000000000..2a91fbe394fc
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
> @@ -0,0 +1,8 @@
> +What: /sys/bus/iio/devices/iio:deviceX/out_resistance_raw_available
> +Date: October 2016
> +KernelVersion: 4.9
> +Contact: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
> +Description:
> + The range of available values represented as the minimum value,
> + the step and the maximum value, all enclosed in square brackets.
> + Example: [0 1 256]
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 411e3b87b8c2..2409e1d79cb6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7707,6 +7707,7 @@ MCP4531 MICROCHIP DIGITAL POTENTIOMETER DRIVER
> M: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
> L: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> S: Maintained
> +F: Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
> F: drivers/iio/potentiometer/mcp4531.c
>
> MEASUREMENT COMPUTING CIO-DAC IIO DRIVER
> diff --git a/drivers/iio/potentiometer/mcp4531.c b/drivers/iio/potentiometer/mcp4531.c
> index 13b6ae2fcf7b..0d1bcf89ae17 100644
> --- a/drivers/iio/potentiometer/mcp4531.c
> +++ b/drivers/iio/potentiometer/mcp4531.c
> @@ -38,7 +38,7 @@
>
> struct mcp4531_cfg {
> int wipers;
> - int max_pos;
> + int avail[3];
> int kohms;
> };
>
> @@ -78,38 +78,38 @@ enum mcp4531_type {
> };
>
> static const struct mcp4531_cfg mcp4531_cfg[] = {
> - [MCP453x_502] = { .wipers = 1, .max_pos = 128, .kohms = 5, },
> - [MCP453x_103] = { .wipers = 1, .max_pos = 128, .kohms = 10, },
> - [MCP453x_503] = { .wipers = 1, .max_pos = 128, .kohms = 50, },
> - [MCP453x_104] = { .wipers = 1, .max_pos = 128, .kohms = 100, },
> - [MCP454x_502] = { .wipers = 1, .max_pos = 128, .kohms = 5, },
> - [MCP454x_103] = { .wipers = 1, .max_pos = 128, .kohms = 10, },
> - [MCP454x_503] = { .wipers = 1, .max_pos = 128, .kohms = 50, },
> - [MCP454x_104] = { .wipers = 1, .max_pos = 128, .kohms = 100, },
> - [MCP455x_502] = { .wipers = 1, .max_pos = 256, .kohms = 5, },
> - [MCP455x_103] = { .wipers = 1, .max_pos = 256, .kohms = 10, },
> - [MCP455x_503] = { .wipers = 1, .max_pos = 256, .kohms = 50, },
> - [MCP455x_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
> - [MCP456x_502] = { .wipers = 1, .max_pos = 256, .kohms = 5, },
> - [MCP456x_103] = { .wipers = 1, .max_pos = 256, .kohms = 10, },
> - [MCP456x_503] = { .wipers = 1, .max_pos = 256, .kohms = 50, },
> - [MCP456x_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
> - [MCP463x_502] = { .wipers = 2, .max_pos = 128, .kohms = 5, },
> - [MCP463x_103] = { .wipers = 2, .max_pos = 128, .kohms = 10, },
> - [MCP463x_503] = { .wipers = 2, .max_pos = 128, .kohms = 50, },
> - [MCP463x_104] = { .wipers = 2, .max_pos = 128, .kohms = 100, },
> - [MCP464x_502] = { .wipers = 2, .max_pos = 128, .kohms = 5, },
> - [MCP464x_103] = { .wipers = 2, .max_pos = 128, .kohms = 10, },
> - [MCP464x_503] = { .wipers = 2, .max_pos = 128, .kohms = 50, },
> - [MCP464x_104] = { .wipers = 2, .max_pos = 128, .kohms = 100, },
> - [MCP465x_502] = { .wipers = 2, .max_pos = 256, .kohms = 5, },
> - [MCP465x_103] = { .wipers = 2, .max_pos = 256, .kohms = 10, },
> - [MCP465x_503] = { .wipers = 2, .max_pos = 256, .kohms = 50, },
> - [MCP465x_104] = { .wipers = 2, .max_pos = 256, .kohms = 100, },
> - [MCP466x_502] = { .wipers = 2, .max_pos = 256, .kohms = 5, },
> - [MCP466x_103] = { .wipers = 2, .max_pos = 256, .kohms = 10, },
> - [MCP466x_503] = { .wipers = 2, .max_pos = 256, .kohms = 50, },
> - [MCP466x_104] = { .wipers = 2, .max_pos = 256, .kohms = 100, },
> + [MCP453x_502] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 5, },
> + [MCP453x_103] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 10, },
> + [MCP453x_503] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 50, },
> + [MCP453x_104] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 100, },
> + [MCP454x_502] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 5, },
> + [MCP454x_103] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 10, },
> + [MCP454x_503] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 50, },
> + [MCP454x_104] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 100, },
> + [MCP455x_502] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 5, },
> + [MCP455x_103] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 10, },
> + [MCP455x_503] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 50, },
> + [MCP455x_104] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 100, },
> + [MCP456x_502] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 5, },
> + [MCP456x_103] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 10, },
> + [MCP456x_503] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 50, },
> + [MCP456x_104] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 100, },
> + [MCP463x_502] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 5, },
> + [MCP463x_103] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 10, },
> + [MCP463x_503] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 50, },
> + [MCP463x_104] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 100, },
> + [MCP464x_502] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 5, },
> + [MCP464x_103] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 10, },
> + [MCP464x_503] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 50, },
> + [MCP464x_104] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 100, },
> + [MCP465x_502] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 5, },
> + [MCP465x_103] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 10, },
> + [MCP465x_503] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 50, },
> + [MCP465x_104] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 100, },
> + [MCP466x_502] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 5, },
> + [MCP466x_103] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 10, },
> + [MCP466x_503] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 50, },
> + [MCP466x_104] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 100, },
> };
>
> #define MCP4531_WRITE (0 << 2)
> @@ -124,13 +124,14 @@ struct mcp4531_data {
> const struct mcp4531_cfg *cfg;
> };
>
> -#define MCP4531_CHANNEL(ch) { \
> - .type = IIO_RESISTANCE, \
> - .indexed = 1, \
> - .output = 1, \
> - .channel = (ch), \
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +#define MCP4531_CHANNEL(ch) { \
> + .type = IIO_RESISTANCE, \
> + .indexed = 1, \
> + .output = 1, \
> + .channel = (ch), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_RAW), \
> }
>
> static const struct iio_chan_spec mcp4531_channels[] = {
> @@ -156,13 +157,31 @@ static int mcp4531_read_raw(struct iio_dev *indio_dev,
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> *val = 1000 * data->cfg->kohms;
> - *val2 = data->cfg->max_pos;
> + *val2 = data->cfg->avail[2];
> return IIO_VAL_FRACTIONAL;
> }
>
> return -EINVAL;
> }
>
> +static int mcp4531_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long mask)
> +{
> + struct mcp4531_data *data = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + *length = ARRAY_SIZE(data->cfg->avail);
> + *vals = data->cfg->avail;
> + *type = IIO_VAL_INT;
> + return IIO_AVAIL_RANGE;
> + }
> +
> + return -EINVAL;
> +}
> +
> static int mcp4531_write_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int val, int val2, long mask)
> @@ -172,7 +191,7 @@ static int mcp4531_write_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> - if (val > data->cfg->max_pos || val < 0)
> + if (val > data->cfg->avail[2] || val < 0)
> return -EINVAL;
> break;
> default:
> @@ -186,6 +205,7 @@ static int mcp4531_write_raw(struct iio_dev *indio_dev,
>
> static const struct iio_info mcp4531_info = {
> .read_raw = mcp4531_read_raw,
> + .read_avail = mcp4531_read_avail,
> .write_raw = mcp4531_write_raw,
> .driver_module = THIS_MODULE,
> };
>
^ permalink raw reply
* Re: [PATCH v4 2/8] iio: inkern: add helpers to query available values from channels
From: Jonathan Cameron @ 2016-11-12 17:17 UTC (permalink / raw)
To: Peter Rosin, linux-kernel
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Rob Herring, Mark Rutland, Daniel Baluta, Slawomir Stepien,
Thomas Gleixner, linux-iio, devicetree
In-Reply-To: <1478606339-31253-3-git-send-email-peda@axentia.se>
On 08/11/16 11:58, Peter Rosin wrote:
> Specifically a helper for reading the available maximum raw value of a
> channel and a helper for forwarding read_avail requests for raw values
> from one iio driver to an iio channel that is consumed.
>
> These rather specific helpers are in turn built with generic helpers
> making it easy to build more helpers for available values as needed.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Applied.
> ---
> drivers/iio/inkern.c | 104 +++++++++++++++++++++++++++++++++++++++++++
> include/linux/iio/consumer.h | 28 ++++++++++++
> include/linux/iio/iio.h | 17 +++++++
> 3 files changed, 149 insertions(+)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index c4757e6367e7..cfca17ba2535 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -703,6 +703,110 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val, int *val2)
> }
> EXPORT_SYMBOL_GPL(iio_read_channel_scale);
>
> +static int iio_channel_read_avail(struct iio_channel *chan,
> + const int **vals, int *type, int *length,
> + enum iio_chan_info_enum info)
> +{
> + if (!iio_channel_has_available(chan->channel, info))
> + return -EINVAL;
> +
> + return chan->indio_dev->info->read_avail(chan->indio_dev, chan->channel,
> + vals, type, length, info);
> +}
> +
> +int iio_read_avail_channel_raw(struct iio_channel *chan,
> + const int **vals, int *length)
> +{
> + int ret;
> + int type;
> +
> + mutex_lock(&chan->indio_dev->info_exist_lock);
> + if (!chan->indio_dev->info) {
> + ret = -ENODEV;
> + goto err_unlock;
> + }
> +
> + ret = iio_channel_read_avail(chan,
> + vals, &type, length, IIO_CHAN_INFO_RAW);
> +err_unlock:
> + mutex_unlock(&chan->indio_dev->info_exist_lock);
> +
> + if (ret >= 0 && type != IIO_VAL_INT) {
> + /* raw values are assumed to be IIO_VAL_INT */
> + ret = -EINVAL;
> + goto err_unlock;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_read_avail_channel_raw);
> +
> +static int iio_channel_read_max(struct iio_channel *chan,
> + int *val, int *val2, int *type,
> + enum iio_chan_info_enum info)
> +{
> + int unused;
> + const int *vals;
> + int length;
> + int ret;
> +
> + if (!val2)
> + val2 = &unused;
> +
> + ret = iio_channel_read_avail(chan, &vals, type, &length, info);
> + switch (ret) {
> + case IIO_AVAIL_RANGE:
> + switch (*type) {
> + case IIO_VAL_INT:
> + *val = vals[2];
> + break;
> + default:
> + *val = vals[4];
> + *val2 = vals[5];
> + }
> + return 0;
> +
> + case IIO_AVAIL_LIST:
> + if (length <= 0)
> + return -EINVAL;
> + switch (*type) {
> + case IIO_VAL_INT:
> + *val = vals[--length];
> + while (length) {
> + if (vals[--length] > *val)
> + *val = vals[length];
> + }
> + break;
> + default:
> + /* FIXME: learn about max for other iio values */
> + return -EINVAL;
> + }
> + return 0;
> +
> + default:
> + return ret;
> + }
> +}
> +
> +int iio_read_max_channel_raw(struct iio_channel *chan, int *val)
> +{
> + int ret;
> + int type;
> +
> + mutex_lock(&chan->indio_dev->info_exist_lock);
> + if (!chan->indio_dev->info) {
> + ret = -ENODEV;
> + goto err_unlock;
> + }
> +
> + ret = iio_channel_read_max(chan, val, NULL, &type, IIO_CHAN_INFO_RAW);
> +err_unlock:
> + mutex_unlock(&chan->indio_dev->info_exist_lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_read_max_channel_raw);
> +
> int iio_get_channel_type(struct iio_channel *chan, enum iio_chan_type *type)
> {
> int ret = 0;
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index 9edccfba1ffb..9a4f336d8b4a 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -226,6 +226,34 @@ int iio_read_channel_processed(struct iio_channel *chan, int *val);
> int iio_write_channel_raw(struct iio_channel *chan, int val);
>
> /**
> + * iio_read_max_channel_raw() - read maximum available raw value from a given
> + * channel, i.e. the maximum possible value.
> + * @chan: The channel being queried.
> + * @val: Value read back.
> + *
> + * Note raw reads from iio channels are in adc counts and hence
> + * scale will need to be applied if standard units are required.
> + */
> +int iio_read_max_channel_raw(struct iio_channel *chan, int *val);
> +
> +/**
> + * iio_read_avail_channel_raw() - read available raw values from a given channel
> + * @chan: The channel being queried.
> + * @vals: Available values read back.
> + * @length: Number of entries in vals.
> + *
> + * Returns an error code, IIO_AVAIL_RANGE or IIO_AVAIL_LIST.
> + *
> + * For ranges, three vals are always returned; min, step and max.
> + * For lists, all the possible values are enumerated.
> + *
> + * Note raw available values from iio channels are in adc counts and
> + * hence scale will need to be applied if standard units are required.
> + */
> +int iio_read_avail_channel_raw(struct iio_channel *chan,
> + const int **vals, int *length);
> +
> +/**
> * iio_get_channel_type() - get the type of a channel
> * @channel: The channel being queried.
> * @type: The type of the channel.
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 45b781084a4b..f9be8836ef23 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -315,6 +315,23 @@ static inline bool iio_channel_has_info(const struct iio_chan_spec *chan,
> (chan->info_mask_shared_by_all & BIT(type));
> }
>
> +/**
> + * iio_channel_has_available() - Checks if a channel has an available attribute
> + * @chan: The channel to be queried
> + * @type: Type of the available attribute to be checked
> + *
> + * Returns true if the channel supports reporting available values for the
> + * given attribute type, false otherwise.
> + */
> +static inline bool iio_channel_has_available(const struct iio_chan_spec *chan,
> + enum iio_chan_info_enum type)
> +{
> + return (chan->info_mask_separate_available & BIT(type)) |
> + (chan->info_mask_shared_by_type_available & BIT(type)) |
> + (chan->info_mask_shared_by_dir_available & BIT(type)) |
> + (chan->info_mask_shared_by_all_available & BIT(type));
> +}
> +
> #define IIO_CHAN_SOFT_TIMESTAMP(_si) { \
> .type = IIO_TIMESTAMP, \
> .channel = -1, \
>
^ permalink raw reply
* Re: [PATCH v4 1/8] iio:core: add a callback to allow drivers to provide _available attributes
From: Jonathan Cameron @ 2016-11-12 17:17 UTC (permalink / raw)
To: Peter Rosin, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Rob Herring, Mark Rutland, Daniel Baluta, Slawomir Stepien,
Thomas Gleixner, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478606339-31253-2-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 08/11/16 11:58, Peter Rosin wrote:
> From: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> A large number of attributes can only take a limited range of values.
> Currently in IIO this is handled by directly registering additional
> *_available attributes thus providing this information to userspace.
>
> It is desirable to provide this information via the core for much the same
> reason this was done for the actual channel information attributes in the
> first place. If it isn't there, then it can only really be accessed from
> userspace. Other in kernel IIO consumers have no access to what valid
> parameters are.
>
> Two forms are currently supported:
> * list of values in one particular IIO_VAL_* format.
> e.g. 1.300000 1.500000 1.730000
> * range specification with a step size:
> e.g. [1.000000 0.500000 2.500000]
> equivalent to 1.000000 1.5000000 2.000000 2.500000
>
> An addition set of masks are used to allow different sharing rules for the
> *_available attributes generated.
>
> This allows for example:
>
> in_accel_x_offset
> in_accel_y_offset
> in_accel_offset_available.
>
> We could have gone with having a specification for each and every
> info_mask element but that would have meant changing the existing userspace
> ABI. This approach does not.
>
> Signed-off-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> [forward ported, added some docs and fixed buffer overflows /peda]
> Acked-by: Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Applied to the togreg branch of iio.git.
Lars, if you get a chance I'd still appreciate any feedback you have
in the ABI in particular. We still have a window to modify it but that
will be running out fast. If you are happy with it then great!
Jonathan
> ---
> drivers/iio/industrialio-core.c | 259 +++++++++++++++++++++++++++++++++++-----
> include/linux/iio/iio.h | 29 +++++
> include/linux/iio/types.h | 5 +
> 3 files changed, 260 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index fc340ed3dca1..b35c61a31fa6 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -575,66 +575,82 @@ int of_iio_read_mount_matrix(const struct device *dev,
> #endif
> EXPORT_SYMBOL(of_iio_read_mount_matrix);
>
> -/**
> - * iio_format_value() - Formats a IIO value into its string representation
> - * @buf: The buffer to which the formatted value gets written
> - * @type: One of the IIO_VAL_... constants. This decides how the val
> - * and val2 parameters are formatted.
> - * @size: Number of IIO value entries contained in vals
> - * @vals: Pointer to the values, exact meaning depends on the
> - * type parameter.
> - *
> - * Return: 0 by default, a negative number on failure or the
> - * total number of characters written for a type that belongs
> - * to the IIO_VAL_... constant.
> - */
> -ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
> +static ssize_t __iio_format_value(char *buf, size_t len, unsigned int type,
> + int size, const int *vals)
> {
> unsigned long long tmp;
> + int tmp0, tmp1;
> bool scale_db = false;
>
> switch (type) {
> case IIO_VAL_INT:
> - return sprintf(buf, "%d\n", vals[0]);
> + return snprintf(buf, len, "%d", vals[0]);
> case IIO_VAL_INT_PLUS_MICRO_DB:
> scale_db = true;
> case IIO_VAL_INT_PLUS_MICRO:
> if (vals[1] < 0)
> - return sprintf(buf, "-%d.%06u%s\n", abs(vals[0]),
> - -vals[1], scale_db ? " dB" : "");
> + return snprintf(buf, len, "-%d.%06u%s", abs(vals[0]),
> + -vals[1], scale_db ? " dB" : "");
> else
> - return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1],
> - scale_db ? " dB" : "");
> + return snprintf(buf, len, "%d.%06u%s", vals[0], vals[1],
> + scale_db ? " dB" : "");
> case IIO_VAL_INT_PLUS_NANO:
> if (vals[1] < 0)
> - return sprintf(buf, "-%d.%09u\n", abs(vals[0]),
> - -vals[1]);
> + return snprintf(buf, len, "-%d.%09u", abs(vals[0]),
> + -vals[1]);
> else
> - return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
> + return snprintf(buf, len, "%d.%09u", vals[0], vals[1]);
> case IIO_VAL_FRACTIONAL:
> tmp = div_s64((s64)vals[0] * 1000000000LL, vals[1]);
> - vals[0] = (int)div_s64_rem(tmp, 1000000000, &vals[1]);
> - return sprintf(buf, "%d.%09u\n", vals[0], abs(vals[1]));
> + tmp1 = vals[1];
> + tmp0 = (int)div_s64_rem(tmp, 1000000000, &tmp1);
> + return snprintf(buf, len, "%d.%09u", tmp0, abs(tmp1));
> case IIO_VAL_FRACTIONAL_LOG2:
> tmp = (s64)vals[0] * 1000000000LL >> vals[1];
> - vals[1] = do_div(tmp, 1000000000LL);
> - vals[0] = tmp;
> - return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
> + tmp1 = do_div(tmp, 1000000000LL);
> + tmp0 = tmp;
> + return snprintf(buf, len, "%d.%09u", tmp0, tmp1);
> case IIO_VAL_INT_MULTIPLE:
> {
> int i;
> - int len = 0;
> + int l = 0;
>
> - for (i = 0; i < size; ++i)
> - len += snprintf(&buf[len], PAGE_SIZE - len, "%d ",
> - vals[i]);
> - len += snprintf(&buf[len], PAGE_SIZE - len, "\n");
> - return len;
> + for (i = 0; i < size; ++i) {
> + l += snprintf(&buf[l], len - l, "%d ", vals[i]);
> + if (l >= len)
> + break;
> + }
> + return l;
> }
> default:
> return 0;
> }
> }
> +
> +/**
> + * iio_format_value() - Formats a IIO value into its string representation
> + * @buf: The buffer to which the formatted value gets written
> + * which is assumed to be big enough (i.e. PAGE_SIZE).
> + * @type: One of the IIO_VAL_... constants. This decides how the val
> + * and val2 parameters are formatted.
> + * @size: Number of IIO value entries contained in vals
> + * @vals: Pointer to the values, exact meaning depends on the
> + * type parameter.
> + *
> + * Return: 0 by default, a negative number on failure or the
> + * total number of characters written for a type that belongs
> + * to the IIO_VAL_... constant.
> + */
> +ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
> +{
> + ssize_t len;
> +
> + len = __iio_format_value(buf, PAGE_SIZE, type, size, vals);
> + if (len >= PAGE_SIZE - 1)
> + return -EFBIG;
> +
> + return len + sprintf(buf + len, "\n");
> +}
> EXPORT_SYMBOL_GPL(iio_format_value);
>
> static ssize_t iio_read_channel_info(struct device *dev,
> @@ -662,6 +678,119 @@ static ssize_t iio_read_channel_info(struct device *dev,
> return iio_format_value(buf, ret, val_len, vals);
> }
>
> +static ssize_t iio_format_avail_list(char *buf, const int *vals,
> + int type, int length)
> +{
> + int i;
> + ssize_t len = 0;
> +
> + switch (type) {
> + case IIO_VAL_INT:
> + for (i = 0; i < length; i++) {
> + len += __iio_format_value(buf + len, PAGE_SIZE - len,
> + type, 1, &vals[i]);
> + if (len >= PAGE_SIZE)
> + return -EFBIG;
> + if (i < length - 1)
> + len += snprintf(buf + len, PAGE_SIZE - len,
> + " ");
> + else
> + len += snprintf(buf + len, PAGE_SIZE - len,
> + "\n");
> + if (len >= PAGE_SIZE)
> + return -EFBIG;
> + }
> + break;
> + default:
> + for (i = 0; i < length / 2; i++) {
> + len += __iio_format_value(buf + len, PAGE_SIZE - len,
> + type, 2, &vals[i * 2]);
> + if (len >= PAGE_SIZE)
> + return -EFBIG;
> + if (i < length / 2 - 1)
> + len += snprintf(buf + len, PAGE_SIZE - len,
> + " ");
> + else
> + len += snprintf(buf + len, PAGE_SIZE - len,
> + "\n");
> + if (len >= PAGE_SIZE)
> + return -EFBIG;
> + }
> + };
> +
> + return len;
> +}
> +
> +static ssize_t iio_format_avail_range(char *buf, const int *vals, int type)
> +{
> + int i;
> + ssize_t len;
> +
> + len = snprintf(buf, PAGE_SIZE, "[");
> + switch (type) {
> + case IIO_VAL_INT:
> + for (i = 0; i < 3; i++) {
> + len += __iio_format_value(buf + len, PAGE_SIZE - len,
> + type, 1, &vals[i]);
> + if (len >= PAGE_SIZE)
> + return -EFBIG;
> + if (i < 2)
> + len += snprintf(buf + len, PAGE_SIZE - len,
> + " ");
> + else
> + len += snprintf(buf + len, PAGE_SIZE - len,
> + "]\n");
> + if (len >= PAGE_SIZE)
> + return -EFBIG;
> + }
> + break;
> + default:
> + for (i = 0; i < 3; i++) {
> + len += __iio_format_value(buf + len, PAGE_SIZE - len,
> + type, 2, &vals[i * 2]);
> + if (len >= PAGE_SIZE)
> + return -EFBIG;
> + if (i < 2)
> + len += snprintf(buf + len, PAGE_SIZE - len,
> + " ");
> + else
> + len += snprintf(buf + len, PAGE_SIZE - len,
> + "]\n");
> + if (len >= PAGE_SIZE)
> + return -EFBIG;
> + }
> + };
> +
> + return len;
> +}
> +
> +static ssize_t iio_read_channel_info_avail(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> + const int *vals;
> + int ret;
> + int length;
> + int type;
> +
> + ret = indio_dev->info->read_avail(indio_dev, this_attr->c,
> + &vals, &type, &length,
> + this_attr->address);
> +
> + if (ret < 0)
> + return ret;
> + switch (ret) {
> + case IIO_AVAIL_LIST:
> + return iio_format_avail_list(buf, vals, type, length);
> + case IIO_AVAIL_RANGE:
> + return iio_format_avail_range(buf, vals, type);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> /**
> * iio_str_to_fixpoint() - Parse a fixed-point number from a string
> * @str: The string to parse
> @@ -978,6 +1107,40 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
> return attrcount;
> }
>
> +static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + enum iio_shared_by shared_by,
> + const long *infomask)
> +{
> + int i, ret, attrcount = 0;
> + char *avail_postfix;
> +
> + for_each_set_bit(i, infomask, sizeof(infomask) * 8) {
> + avail_postfix = kasprintf(GFP_KERNEL,
> + "%s_available",
> + iio_chan_info_postfix[i]);
> + if (!avail_postfix)
> + return -ENOMEM;
> +
> + ret = __iio_add_chan_devattr(avail_postfix,
> + chan,
> + &iio_read_channel_info_avail,
> + NULL,
> + i,
> + shared_by,
> + &indio_dev->dev,
> + &indio_dev->channel_attr_list);
> + kfree(avail_postfix);
> + if ((ret == -EBUSY) && (shared_by != IIO_SEPARATE))
> + continue;
> + else if (ret < 0)
> + return ret;
> + attrcount++;
> + }
> +
> + return attrcount;
> +}
> +
> static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan)
> {
> @@ -993,6 +1156,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> return ret;
> attrcount += ret;
>
> + ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
> + IIO_SEPARATE,
> + &chan->
> + info_mask_separate_available);
> + if (ret < 0)
> + return ret;
> + attrcount += ret;
> +
> ret = iio_device_add_info_mask_type(indio_dev, chan,
> IIO_SHARED_BY_TYPE,
> &chan->info_mask_shared_by_type);
> @@ -1000,6 +1171,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> return ret;
> attrcount += ret;
>
> + ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
> + IIO_SHARED_BY_TYPE,
> + &chan->
> + info_mask_shared_by_type_available);
> + if (ret < 0)
> + return ret;
> + attrcount += ret;
> +
> ret = iio_device_add_info_mask_type(indio_dev, chan,
> IIO_SHARED_BY_DIR,
> &chan->info_mask_shared_by_dir);
> @@ -1007,6 +1186,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> return ret;
> attrcount += ret;
>
> + ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
> + IIO_SHARED_BY_DIR,
> + &chan->info_mask_shared_by_dir_available);
> + if (ret < 0)
> + return ret;
> + attrcount += ret;
> +
> ret = iio_device_add_info_mask_type(indio_dev, chan,
> IIO_SHARED_BY_ALL,
> &chan->info_mask_shared_by_all);
> @@ -1014,6 +1200,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> return ret;
> attrcount += ret;
>
> + ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
> + IIO_SHARED_BY_ALL,
> + &chan->info_mask_shared_by_all_available);
> + if (ret < 0)
> + return ret;
> + attrcount += ret;
> +
> if (chan->ext_info) {
> unsigned int i = 0;
> for (ext_info = chan->ext_info; ext_info->name; ext_info++) {
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index b4a0679e4a49..45b781084a4b 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -225,12 +225,22 @@ struct iio_event_spec {
> * endianness: little or big endian
> * @info_mask_separate: What information is to be exported that is specific to
> * this channel.
> + * @info_mask_separate_available: What availability information is to be
> + * exported that is specific to this channel.
> * @info_mask_shared_by_type: What information is to be exported that is shared
> * by all channels of the same type.
> + * @info_mask_shared_by_type_available: What availability information is to be
> + * exported that is shared by all channels of the same
> + * type.
> * @info_mask_shared_by_dir: What information is to be exported that is shared
> * by all channels of the same direction.
> + * @info_mask_shared_by_dir_available: What availability information is to be
> + * exported that is shared by all channels of the same
> + * direction.
> * @info_mask_shared_by_all: What information is to be exported that is shared
> * by all channels.
> + * @info_mask_shared_by_all_available: What availability information is to be
> + * exported that is shared by all channels.
> * @event_spec: Array of events which should be registered for this
> * channel.
> * @num_event_specs: Size of the event_spec array.
> @@ -269,9 +279,13 @@ struct iio_chan_spec {
> enum iio_endian endianness;
> } scan_type;
> long info_mask_separate;
> + long info_mask_separate_available;
> long info_mask_shared_by_type;
> + long info_mask_shared_by_type_available;
> long info_mask_shared_by_dir;
> + long info_mask_shared_by_dir_available;
> long info_mask_shared_by_all;
> + long info_mask_shared_by_all_available;
> const struct iio_event_spec *event_spec;
> unsigned int num_event_specs;
> const struct iio_chan_spec_ext_info *ext_info;
> @@ -349,6 +363,14 @@ struct iio_dev;
> * max_len specifies maximum number of elements
> * vals pointer can contain. val_len is used to return
> * length of valid elements in vals.
> + * @read_avail: function to return the available values from the device.
> + * mask specifies which value. Note 0 means the available
> + * values for the channel in question. Return value
> + * specifies if a IIO_AVAIL_LIST or a IIO_AVAIL_RANGE is
> + * returned in vals. The type of the vals are returned in
> + * type and the number of vals is returned in length. For
> + * ranges, there are always three vals returned; min, step
> + * and max. For lists, all possible values are enumerated.
> * @write_raw: function to write a value to the device.
> * Parameters are the same as for read_raw.
> * @write_raw_get_fmt: callback function to query the expected
> @@ -397,6 +419,13 @@ struct iio_info {
> int *val_len,
> long mask);
>
> + int (*read_avail)(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals,
> + int *type,
> + int *length,
> + long mask);
> +
> int (*write_raw)(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int val,
> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
> index 32b579525004..2aa7b6384d64 100644
> --- a/include/linux/iio/types.h
> +++ b/include/linux/iio/types.h
> @@ -29,4 +29,9 @@ enum iio_event_info {
> #define IIO_VAL_FRACTIONAL 10
> #define IIO_VAL_FRACTIONAL_LOG2 11
>
> +enum iio_available_type {
> + IIO_AVAIL_LIST,
> + IIO_AVAIL_RANGE,
> +};
> +
> #endif /* _IIO_TYPES_H_ */
>
--
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 v4 0/8] IIO wrapper drivers, dpot-dac and envelope-detector
From: Jonathan Cameron @ 2016-11-12 17:15 UTC (permalink / raw)
To: Peter Rosin, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Rob Herring, Mark Rutland, Daniel Baluta, Slawomir Stepien,
Thomas Gleixner, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478606339-31253-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 08/11/16 11:58, Peter Rosin wrote:
> Hi!
>
> This is a respin with a handful of nitpicks fixed from v3. No major
> changes. And a couple of acks was added too, thanks! I also added
> Thomas Gleixner as Cc, since Jonathan was hoping for some comments
> on the somewhat odd interrupt handling in patch 8/8 (but perhaps
> plumbers-week isn't the best week to hope for that).
>
>
> These two drivers share the fact that they wrap another iio channel,
> and I use the first in combination with the second, which is why I'm
> submitting them as a pair.
>
> The first driver is a simple wrapper converting an iio dpot into an
> iio dac. It only changes the unit and scale. It also does not add any
> fancy iio buffer support that I don't need. I suppose that can be
> added. By someone else :-)
I'll probably get to it sooner or later if no one else jumps on it.
>
> The second driver (the envelope detector) is more involved. It also
> explains why I need the dpot-dac driver. I wanted the envelope
> detector to be generic and work with any dac, but I had a dpot...
>
> But before those two new drivers, there is some infrastructure added
> to provide available values for a channel.
>
> One thing I still don't like is that the irq needs to be changed for cases
> where it is only ever interesting with the 'invert' variant, and it may not
> work to set up the irq the wrong way first. For the TSE-850, this does not
> matter, since we have a mux on the envelope detector input and can make use
> of both 'invert' and 'normal' for different signals (we could have gotten
> by with only 'invert' since the only signals we are measuring that are
> 'normal' are also DC signals and can thus be detected both from below and
> from above), but it is nice to have it both ways. The only way out of this
> is a devicetree thing. I suppose it will have to be added by whomever needs
> it whenever that is...
>
> I also wonder if the "new" *_available ABI should perhaps be documented
> for all variants directly in sysfs-bus-iio instead of doing it in a driver
> specific maner that I did? But that can be fixed later by someone more
> capable than me :-)
You doubt yourself too much ;) Some one with fewer inhibitions you mean!
Anyhow, just thought I'd add that I like this series very much.
It's a nice interesting use of the infrastructures. Good to see people
are getting more adventurous all the time.
Also always nice when someone else picks up a patch I dropped years ago
and does the remaining hard work to get it in ;)
There is a limited window left obviously if we want to adjust that ABI
so shout soon or it'll be there for ever *muhahahaha*
Jonathan
>
> v3 -> v4
> - gained acks from Rob for the three bindings patches
> - gained ack from Daniel for the core _available patch (1/8)
> - dropped the type argument from iio_read_avail_channel_raw(), since
> raw values are assumed to be of type IIO_VAL_INT anyway
> - add some more words about what iio_read_avail_channel_raw() does
> - rebased onto v4.9-rc4
>
> dpot-dac:
> - adjust to changed signature of iio_read_avail_channel_raw()
> - one instance of s/the channels supports/the channel supports/
> - drop surplus s64 cast in dpot_dac_channel_max_ohms()
>
> envelope-detector
> - the envelope-detector module is called envelope-detector
>
>
> v2 -> v3
> - add some missing @foo comments in iio.h in the initial forward ported patch
> - killed some buffer overflow problems in the initial forward ported patch
> - add inkern.c helpers for the new available attributes to avoid viral
> boilerplating in the future (also add support for finding max of
> IIO_AVAIL_LISTs of IIO_VAL_INTs)
> - add ABI docs for new ABI of mcp4531 (out_resistance_raw_available)
> and an example in the commit message
>
> dpot-dac:
> - two more counts of s/assumed the that the/assumed that the/ *blush*
> - add ABI docs for new ABI (out_voltageY_raw_available)
>
> envelope detector:
> - move device attributes 'compare_interval_ms' and 'invert' to extended
> channel attributes (out_altvoltage0_compare_interval and
> out_altvoltage0_invert) as they really are about the channel and not
> the device
> - kill "dpot-dac,max-ohms = <100000>;" in the devicetree example
>
>
> v1 -> v2
> - provide out_resistanceX_raw_available channel attribute in mcp4531 dpot
>
> dpot-dac:
> - change Vref to vref
> - the module will be called dpot-dac (in Kconfig help)
> - removed a 'the'
> - removed (s64) cast
> - make the channel indexed, makes libiio find the channel (tested with 0.5)
> - add a comment on how integer scale is converted to fractional scale
> and clarify the code a bit
> - dig out max-ohms by looking at scale and maximum available raw value
> from the dpot channel and drop the 'dpot-dac,max-ohms' devicetree property
> - provide out_voltageX_raw_available channel attribute
>
> envelope-detector:
> - change compatible from envelope-detector to axentia,tse850-envelope-detector
> - remove envelope-detector,invert and envelope-detector,comp-interval-ms from
> devicetree and add them as iio device attributes instead
> - make the channel indexed, makes libiio find the channel (tested with 0.5)
> - reorder struct envelope to better indicate what is covered by read_lock
> - add comment on interaction between envelope_detector_comp_latch (renamed
> from envelope_detector_latch) and envelope_detector_comp_isr (renamed
> from envelope_detector_isr)
> - fixup a problem in envelope_detector_comp_latch where interrupts pending
> from when the interrupt has been disabled interferes with expected
> operation
> - slight rewrite of the initial high/low assignments
> - use a better name when requesting the interrupt
> - dig out dac_max by looking at scale and maximum available raw value
> from the dac channel and drop the 'envelope-detector,dac-max' devicetree
> property
>
> Cheers,
> Peter
>
> Jonathan Cameron (1):
> iio:core: add a callback to allow drivers to provide _available
> attributes
>
> Peter Rosin (7):
> iio: inkern: add helpers to query available values from channels
> iio: mcp4531: provide range of available raw values
> dt-bindings: add axentia to vendor-prefixes
> dt-bindings: iio: document dpot-dac bindings
> iio: dpot-dac: DAC driver based on a digital potentiometer
> dt-bindings: iio: document envelope-detector bindings
> iio: envelope-detector: ADC driver based on a DAC and a comparator
>
> .../testing/sysfs-bus-iio-adc-envelope-detector | 36 ++
> .../ABI/testing/sysfs-bus-iio-dac-dpot-dac | 8 +
> .../testing/sysfs-bus-iio-potentiometer-mcp4531 | 8 +
> .../bindings/iio/adc/envelope-detector.txt | 54 +++
> .../devicetree/bindings/iio/dac/dpot-dac.txt | 41 ++
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> MAINTAINERS | 17 +
> drivers/iio/adc/Kconfig | 10 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/envelope-detector.c | 422 +++++++++++++++++++++
> drivers/iio/dac/Kconfig | 10 +
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/dpot-dac.c | 266 +++++++++++++
> drivers/iio/industrialio-core.c | 259 +++++++++++--
> drivers/iio/inkern.c | 104 +++++
> drivers/iio/potentiometer/mcp4531.c | 104 +++--
> include/linux/iio/consumer.h | 28 ++
> include/linux/iio/iio.h | 46 +++
> include/linux/iio/types.h | 5 +
> 19 files changed, 1346 insertions(+), 75 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
> create mode 100644 Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
> create mode 100644 Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
> create mode 100644 drivers/iio/adc/envelope-detector.c
> create mode 100644 drivers/iio/dac/dpot-dac.c
>
--
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 3/6] iio: adc: Add support for STM32 ADC
From: Jonathan Cameron @ 2016-11-12 17:08 UTC (permalink / raw)
To: Fabrice Gasnier, linux-iio, linux-arm-kernel, devicetree,
linux-kernel
Cc: lee.jones, linux, robh+dt, mark.rutland, mcoquelin.stm32,
alexandre.torgue, lars, knaack.h, pmeerw
In-Reply-To: <1478794738-28933-4-git-send-email-fabrice.gasnier@st.com>
On 10/11/16 16:18, Fabrice Gasnier wrote:
> This patch adds support for STMicroelectronics STM32 MCU's analog to
> digital converter.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Nice and clean - few minor things inline.
Jonathan
> ---
> drivers/iio/adc/Kconfig | 10 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/stm32-adc.c | 525 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 536 insertions(+)
> create mode 100644 drivers/iio/adc/stm32-adc.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 7edcf32..61ba674 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -419,6 +419,16 @@ config ROCKCHIP_SARADC
> To compile this driver as a module, choose M here: the
> module will be called rockchip_saradc.
>
> +config STM32_ADC
> + tristate "STMicroelectronics STM32 adc"
> + depends on MFD_STM32_ADC
> + help
> + Say yes here to build support for STMicroelectronics stm32 Analog
> + to Digital Converter (ADC).
> +
> + This driver can also be built as a module. If so, the module
> + will be called stm32-adc.
> +
> config STX104
> tristate "Apex Embedded Systems STX104 driver"
> depends on X86 && ISA_BUS_API
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 7a40c04..df7a221 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -41,6 +41,7 @@ obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
> obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
> obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
> obj-$(CONFIG_STX104) += stx104.o
> +obj-$(CONFIG_STM32_ADC) += stm32-adc.o
> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
> obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> new file mode 100644
> index 0000000..2be5fee
> --- /dev/null
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -0,0 +1,525 @@
> +/*
> + * This file is part of STM32 ADC driver
> + *
> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
> + *
> + * License type: GPLv2
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/mfd/stm32-adc-core.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +
> +/* STM32F4 - Registers for each ADC instance */
Not really sure the X adds anything in these, but doesn't do much harm
I guess ;)
> +#define STM32F4_ADCX_SR 0x00
> +#define STM32F4_ADCX_CR1 0x04
> +#define STM32F4_ADCX_CR2 0x08
> +#define STM32F4_ADCX_SMPR1 0x0C
> +#define STM32F4_ADCX_SMPR2 0x10
> +#define STM32F4_ADCX_HTR 0x24
> +#define STM32F4_ADCX_LTR 0x28
> +#define STM32F4_ADCX_SQR1 0x2C
> +#define STM32F4_ADCX_SQR2 0x30
> +#define STM32F4_ADCX_SQR3 0x34
> +#define STM32F4_ADCX_JSQR 0x38
> +#define STM32F4_ADCX_JDR1 0x3C
> +#define STM32F4_ADCX_JDR2 0x40
> +#define STM32F4_ADCX_JDR3 0x44
> +#define STM32F4_ADCX_JDR4 0x48
> +#define STM32F4_ADCX_DR 0x4C
> +
> +/* STM32F4_ADCX_SR - bit fields */
> +#define STM32F4_OVR BIT(5)
> +#define STM32F4_STRT BIT(4)
> +#define STM32F4_EOC BIT(1)
> +
> +/* STM32F4_ADCX_CR1 - bit fields */
> +#define STM32F4_OVRIE BIT(26)
> +#define STM32F4_SCAN BIT(8)
> +#define STM32F4_EOCIE BIT(5)
> +
> +/* STM32F4_ADCX_CR2 - bit fields */
> +#define STM32F4_SWSTART BIT(30)
> +#define STM32F4_EXTEN_MASK GENMASK(29, 28)
> +#define STM32F4_EOCS BIT(10)
> +#define STM32F4_ADON BIT(0)
> +
> +/* STM32F4_ADCX_SQR1 - bit fields */
> +#define STM32F4_L_SHIFT 20
> +#define STM32F4_L_MASK GENMASK(23, 20)
> +
> +/* STM32F4_ADCX_SQR3 - bit fields */
> +#define STM32F4_SQ1_SHIFT 0
> +#define STM32F4_SQ1_MASK GENMASK(4, 0)
> +
> +#define STM32_ADC_MAX_SQ 16 /* SQ1..SQ16 */
> +#define STM32_ADC_TIMEOUT_US 100000
> +#define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
> +
> +/**
> + * struct stm32_adc - private data of each ADC IIO instance
> + * @common: reference to ADC block common data
> + * @offset: ADC instance register offset in ADC block
> + * @completion: end of single conversion completion
> + * @buffer: data buffer
> + * @clk: optional adc clock, for this adc instance
> + * @irq: interrupt for this adc instance
> + * @lock: spinlock
> + */
> +struct stm32_adc {
> + struct stm32_adc_common *common;
> + u32 offset;
> + struct completion completion;
> + u16 *buffer;
> + struct clk *clk;
> + int irq;
> + spinlock_t lock; /* interrupt lock */
> +};
> +
> +/**
> + * struct stm32_adc_chan_spec - specification of stm32 adc channel
> + * @type: IIO channel type
> + * @channel: channel number (single ended)
> + * @name: channel name (single ended)
> + */
> +struct stm32_adc_chan_spec {
> + enum iio_chan_type type;
> + int channel;
> + const char *name;
> +};
> +
> +/* Input definitions common for all STM32F4 instances */
> +static const struct stm32_adc_chan_spec stm32f4_adc123_channels[] = {
> + { IIO_VOLTAGE, 0, "in0" },
> + { IIO_VOLTAGE, 1, "in1" },
> + { IIO_VOLTAGE, 2, "in2" },
> + { IIO_VOLTAGE, 3, "in3" },
> + { IIO_VOLTAGE, 4, "in4" },
> + { IIO_VOLTAGE, 5, "in5" },
> + { IIO_VOLTAGE, 6, "in6" },
> + { IIO_VOLTAGE, 7, "in7" },
> + { IIO_VOLTAGE, 8, "in8" },
> + { IIO_VOLTAGE, 9, "in9" },
> + { IIO_VOLTAGE, 10, "in10" },
> + { IIO_VOLTAGE, 11, "in11" },
> + { IIO_VOLTAGE, 12, "in12" },
> + { IIO_VOLTAGE, 13, "in13" },
> + { IIO_VOLTAGE, 14, "in14" },
> + { IIO_VOLTAGE, 15, "in15" },
> +};
> +
> +/**
> + * STM32 ADC registers access routines
> + * @adc: stm32 adc instance
> + * @reg: reg offset in adc instance
> + *
> + * Note: All instances share same base, with 0x0, 0x100 or 0x200 offset resp.
> + * for adc1, adc2 and adc3.
> + */
> +static u32 stm32_adc_readl(struct stm32_adc *adc, u32 reg)
> +{
> + return readl_relaxed(adc->common->base + adc->offset + reg);
> +}
> +
> +static u32 stm32_adc_readw(struct stm32_adc *adc, u32 reg)
> +{
> + return readw_relaxed(adc->common->base + adc->offset + reg);
> +}
> +
> +static void stm32_adc_writel(struct stm32_adc *adc, u32 reg, u32 val)
> +{
> + writel_relaxed(val, adc->common->base + adc->offset + reg);
> +}
> +
> +static void stm32_adc_set_bits(struct stm32_adc *adc, u32 reg, u32 bits)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&adc->lock, flags);
> + stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) | bits);
> + spin_unlock_irqrestore(&adc->lock, flags);
> +}
> +
> +static void stm32_adc_clr_bits(struct stm32_adc *adc, u32 reg, u32 bits)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&adc->lock, flags);
> + stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) & ~bits);
> + spin_unlock_irqrestore(&adc->lock, flags);
> +}
> +
> +/**
> + * stm32_adc_conv_irq_enable() - Enable end of conversion interrupt
> + * @adc: stm32 adc instance
> + */
> +static void stm32_adc_conv_irq_enable(struct stm32_adc *adc)
> +{
> + stm32_adc_set_bits(adc, STM32F4_ADCX_CR1, STM32F4_EOCIE);
> +};
> +
> +/**
> + * stm32_adc_conv_irq_disable() - Disable end of conversion interrupt
> + * @adc: stm32 adc instance
> + */
> +static void stm32_adc_conv_irq_disable(struct stm32_adc *adc)
> +{
> + stm32_adc_clr_bits(adc, STM32F4_ADCX_CR1, STM32F4_EOCIE);
> +}
> +
> +/**
> + * stm32_adc_start_conv() - Start conversions for regular channels.
> + * @adc: stm32 adc instance
> + */
> +static void stm32_adc_start_conv(struct stm32_adc *adc)
> +{
> + stm32_adc_set_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
> + stm32_adc_set_bits(adc, STM32F4_ADCX_CR2, STM32F4_EOCS | STM32F4_ADON);
> +
> + /* Wait for Power-up time (tSTAB from datasheet) */
> + usleep_range(2, 3);
> +
> + /* Software start ? (e.g. trigger detection disabled ?) */
> + if (!(stm32_adc_readl(adc, STM32F4_ADCX_CR2) & STM32F4_EXTEN_MASK))
> + stm32_adc_set_bits(adc, STM32F4_ADCX_CR2, STM32F4_SWSTART);
> +}
> +
> +static void stm32_adc_stop_conv(struct stm32_adc *adc)
> +{
> + stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
> + stm32_adc_clr_bits(adc, STM32F4_ADCX_SR, STM32F4_STRT);
> +
> + stm32_adc_clr_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
> + stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_ADON);
> +}
> +
> +/**
> + * stm32_adc_single_conv() - Performs a single conversion
> + * @indio_dev: IIO device
> + * @chan: IIO channel
> + * @res: conversion result
> + *
> + * The function performs a single conversion on a given channel:
> + * - Program sequencer with one channel (e.g. in SQ1 with len = 1)
> + * - Use SW trigger
> + * - Start conversion, then wait for interrupt completion.
> + */
> +static int stm32_adc_single_conv(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + int *res)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> + long timeout;
> + u32 val;
> + u16 result;
> + int ret;
> +
> + reinit_completion(&adc->completion);
> +
> + adc->buffer = &result;
> +
> + /* Program chan number in regular sequence */
> + val = stm32_adc_readl(adc, STM32F4_ADCX_SQR3);
> + val &= ~STM32F4_SQ1_MASK;
> + val |= chan->channel << STM32F4_SQ1_SHIFT;
> + stm32_adc_writel(adc, STM32F4_ADCX_SQR3, val);
> +
> + /* Set regular sequence len (0 for 1 conversion) */
> + stm32_adc_clr_bits(adc, STM32F4_ADCX_SQR1, STM32F4_L_MASK);
> +
> + /* Trigger detection disabled (conversion can be launched in SW) */
> + stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
> +
> + stm32_adc_conv_irq_enable(adc);
> +
> + stm32_adc_start_conv(adc);
> +
> + timeout = wait_for_completion_interruptible_timeout(
> + &adc->completion, STM32_ADC_TIMEOUT);
> + if (timeout == 0) {
> + dev_warn(&indio_dev->dev, "Conversion timed out!\n");
> + ret = -ETIMEDOUT;
> + } else if (timeout < 0) {
> + dev_warn(&indio_dev->dev, "Interrupted conversion!\n");
> + ret = -EINTR;
> + } else {
> + *res = result;
> + ret = IIO_VAL_INT;
> + }
> +
> + stm32_adc_stop_conv(adc);
> +
> + stm32_adc_conv_irq_disable(adc);
> +
> + return ret;
> +}
> +
> +static int stm32_adc_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> + int ret = -EINVAL;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> + if (chan->type == IIO_VOLTAGE)
> + ret = stm32_adc_single_conv(indio_dev, chan, val);
> + else
> + ret = -EINVAL;
> + iio_device_release_direct_mode(indio_dev);
return directly here. Basically always preferred to return directly if
there is not cleanup to be done.
> + break;
> + case IIO_CHAN_INFO_SCALE:
> + *val = adc->common->vref_mv;
> + *val2 = chan->scan_type.realbits;
> + ret = IIO_VAL_FRACTIONAL_LOG2;
return directly here.
> + break;
> + default:
return -EINVAL here.
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static irqreturn_t stm32_adc_isr(int irq, void *data)
> +{
> + struct stm32_adc *adc = data;
> + u32 status = stm32_adc_readl(adc, STM32F4_ADCX_SR);
> + irqreturn_t ret = IRQ_NONE;
> +
> + if (status & STM32F4_EOC) {
> + *adc->buffer = stm32_adc_readw(adc, STM32F4_ADCX_DR);
> + complete(&adc->completion);
> + ret = IRQ_HANDLED;
Slightly tidier to return IRQ_HANDLED here and directly return
IRQ_NONE below.
> + }
> +
> + return ret;
> +}
> +
> +static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
> + const struct of_phandle_args *iiospec)
> +{
> + int i;
> +
> + for (i = 0; i < indio_dev->num_channels; i++)
> + if (indio_dev->channels[i].channel == iiospec->args[0])
> + return i;
> +
> + return -EINVAL;
> +}
> +
> +/**
> + * stm32_adc_debugfs_reg_access - read or write register value
> + *
> + * To read a value from an ADC register:
> + * echo [ADC reg offset] > direct_reg_access
> + * cat direct_reg_access
> + *
> + * To write a value in a ADC register:
> + * echo [ADC_reg_offset] [value] > direct_reg_access
> + */
> +static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
> + unsigned reg, unsigned writeval,
> + unsigned *readval)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> +
> + if (!readval)
> + stm32_adc_writel(adc, reg, writeval);
> + else
> + *readval = stm32_adc_readl(adc, reg);
> +
> + return 0;
> +}
> +
> +static const struct iio_info stm32_adc_iio_info = {
> + .read_raw = stm32_adc_read_raw,
> + .debugfs_reg_access = stm32_adc_debugfs_reg_access,
> + .of_xlate = stm32_adc_of_xlate,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> + struct iio_chan_spec *chan,
> + const struct stm32_adc_chan_spec *channel,
> + int scan_index)
> +{
> + chan->type = channel->type;
> + chan->channel = channel->channel;
> + chan->datasheet_name = channel->name;
> + chan->extend_name = channel->name;
Don't set extend_name. That name doesn't add sufficient information to
make it worth adding custom ABI to the userspace interface.
> + chan->scan_index = scan_index;
> + chan->indexed = 1;
> + chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> + chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
> + chan->scan_type.sign = 'u';
> + chan->scan_type.realbits = 12;
> + chan->scan_type.storagebits = 16;
> +}
> +
> +static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> +{
> + struct device_node *node = indio_dev->dev.of_node;
> + struct property *prop;
> + const __be32 *cur;
> + struct iio_chan_spec *channels;
> + int scan_index = 0, num_channels;
> + u32 val;
> +
> + num_channels = of_property_count_u32_elems(node, "st,adc-channels");
> + if (num_channels < 0 ||
> + num_channels >= ARRAY_SIZE(stm32f4_adc123_channels)) {
> + dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
> + return num_channels < 0 ? num_channels : -EINVAL;
> + }
> +
> + channels = devm_kcalloc(&indio_dev->dev, num_channels,
> + sizeof(struct iio_chan_spec), GFP_KERNEL);
> + if (!channels)
> + return -ENOMEM;
> +
> + of_property_for_each_u32(node, "st,adc-channels", prop, cur, val) {
> + if (val >= ARRAY_SIZE(stm32f4_adc123_channels)) {
> + dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
> + return -EINVAL;
> + }
> + stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> + &stm32f4_adc123_channels[val],
> + scan_index);
> + scan_index++;
> + }
> +
> + indio_dev->num_channels = scan_index;
> + indio_dev->channels = channels;
> +
> + return 0;
> +}
> +
> +static int stm32_adc_probe(struct platform_device *pdev)
> +{
> + struct iio_dev *indio_dev;
> + struct stm32_adc *adc;
> + int ret;
> +
> + if (!pdev->dev.of_node)
> + return -ENODEV;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*adc));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + adc = iio_priv(indio_dev);
> + adc->common = dev_get_drvdata(pdev->dev.parent);
> + spin_lock_init(&adc->lock);
> + init_completion(&adc->completion);
> +
> + indio_dev->name = dev_name(&pdev->dev);
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->dev.of_node = pdev->dev.of_node;
> + indio_dev->info = &stm32_adc_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + platform_set_drvdata(pdev, adc);
> +
> + ret = of_property_read_u32(pdev->dev.of_node, "reg", &adc->offset);
> + if (ret != 0) {
> + dev_err(&pdev->dev, "missing reg property\n");
> + return -EINVAL;
> + }
> +
> + adc->irq = platform_get_irq(pdev, 0);
> + if (adc->irq < 0) {
> + dev_err(&pdev->dev, "failed to get irq\n");
> + return adc->irq;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, adc->irq, stm32_adc_isr,
> + 0, pdev->name, adc);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to request IRQ\n");
> + return ret;
> + }
> +
> + adc->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(adc->clk)) {
Could it concievably be deferred? Would be happier if this explicitly
checked for -ENODEV or whatever gets returned when not clock has
been specified.
> + adc->clk = NULL;
> + dev_dbg(&pdev->dev, "No child clk found\n");
> + } else {
> + ret = clk_prepare_enable(adc->clk);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "clk enable failed\n");
> + return ret;
> + }
> + }
> +
> + ret = stm32_adc_chan_of_init(indio_dev);
> + if (ret < 0)
> + goto err_clk_disable;
> +
> + ret = devm_iio_device_register(&pdev->dev, indio_dev);
This use of devm registration is going to cause a race in the remove.
The userspace interface will not be removed until after the remove
function has run. That disables the clock thus leaving us a window
where we could try and access the device with no clock enabled.
Basic rule of thumb is that use of devm must not effect the ordering
of unrolling what you do in probe when it comes to remove.
(which more or less means that you can't use devm_iio_device_register
unless you have no remove at all).
> + if (ret) {
> + dev_err(&pdev->dev, "iio dev register failed\n");
> + goto err_clk_disable;
> + }
> +
> + return 0;
> +
> +err_clk_disable:
> + if (adc->clk)
> + clk_disable_unprepare(adc->clk);
> +
> + return ret;
> +}
> +
> +static int stm32_adc_remove(struct platform_device *pdev)
> +{
> + struct stm32_adc *adc = platform_get_drvdata(pdev);
> +
> + if (adc->clk)
> + clk_disable_unprepare(adc->clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id stm32_adc_of_match[] = {
> + { .compatible = "st,stm32f4-adc" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, stm32_adc_of_match);
> +
> +static struct platform_driver stm32_adc_driver = {
> + .probe = stm32_adc_probe,
> + .remove = stm32_adc_remove,
> + .driver = {
> + .name = "stm32-adc",
> + .of_match_table = stm32_adc_of_match,
> + },
> +};
> +module_platform_driver(stm32_adc_driver);
> +
> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC IIO driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:stm32-adc");
>
^ permalink raw reply
* Re: [PATCH v2 2/6] mfd: stm32-adc: Add support for stm32 ADC
From: Jonathan Cameron @ 2016-11-12 17:08 UTC (permalink / raw)
To: Fabrice Gasnier, linux-iio, linux-arm-kernel, devicetree,
linux-kernel
Cc: lee.jones, linux, robh+dt, mark.rutland, mcoquelin.stm32,
alexandre.torgue, lars, knaack.h, pmeerw
In-Reply-To: <1478794738-28933-3-git-send-email-fabrice.gasnier@st.com>
On 10/11/16 16:18, Fabrice Gasnier wrote:
> Add core driver for STMicroelectronics STM32 ADC (Analog to Digital
> Converter). STM32 ADC can be composed of up to 3 ADCs with shared
> resources like clock prescaler, common interrupt line and analog
> reference voltage.
> This core driver basically manages shared resources.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Looks good to me (other than the build issue obviously ;)
The fun bit will be trying to keep the whole thing this clean as you
add the more 'interesting' functionality. *fingers crossed*
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
> drivers/mfd/Kconfig | 14 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/stm32-adc-core.c | 301 +++++++++++++++++++++++++++++++++++++
> include/linux/mfd/stm32-adc-core.h | 52 +++++++
> 4 files changed, 368 insertions(+)
> create mode 100644 drivers/mfd/stm32-adc-core.c
> create mode 100644 include/linux/mfd/stm32-adc-core.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index c6df644..2580cee 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1152,6 +1152,20 @@ config MFD_PALMAS
> If you say yes here you get support for the Palmas
> series of PMIC chips from Texas Instruments.
>
> +config MFD_STM32_ADC
> + tristate "STMicroelectronics STM32 adc"
> + depends on ARCH_STM32 || COMPILE_TEST
> + depends on OF
> + select MFD_CORE
> + select REGULATOR
> + select REGULATOR_FIXED_VOLTAGE
> + help
> + Select this option to enable the core driver for STMicroelectronics
> + STM32 analog-to-digital converter (ADC).
> +
> + This driver can also be built as a module. If so, the module
> + will be called stm32-adc-core.
> +
> config TPS6105X
> tristate "TI TPS61050/61052 Boost Converters"
> depends on I2C
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 9834e66..4571506 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -185,6 +185,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o
> obj-$(CONFIG_MFD_INTEL_LPSS_ACPI) += intel-lpss-acpi.o
> obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o
> obj-$(CONFIG_MFD_PALMAS) += palmas.o
> +obj-$(CONFIG_MFD_STM32_ADC) += stm32-adc-core.o
> obj-$(CONFIG_MFD_VIPERBOARD) += viperboard.o
> obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
> obj-$(CONFIG_MFD_RK808) += rk808.o
> diff --git a/drivers/mfd/stm32-adc-core.c b/drivers/mfd/stm32-adc-core.c
> new file mode 100644
> index 0000000..bcf52fb
> --- /dev/null
> +++ b/drivers/mfd/stm32-adc-core.c
> @@ -0,0 +1,301 @@
> +/*
> + * This file is part of STM32 ADC driver
> + *
> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
> + *
> + * Inspired from: fsl-imx25-tsadc
> + *
> + * License type: GPLv2
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdesc.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mfd/stm32-adc-core.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +/* STM32F4 - common registers for all ADC instances: 1, 2 & 3 */
> +#define STM32F4_ADC_CSR (STM32_ADCX_COMN_OFFSET + 0x00)
> +#define STM32F4_ADC_CCR (STM32_ADCX_COMN_OFFSET + 0x04)
> +
> +/* STM32F4_ADC_CSR - bit fields */
> +#define STM32F4_EOC3 BIT(17)
> +#define STM32F4_EOC2 BIT(9)
> +#define STM32F4_EOC1 BIT(1)
> +
> +/* STM32F4_ADC_CCR - bit fields */
> +#define STM32F4_ADC_ADCPRE_SHIFT 16
> +#define STM32F4_ADC_ADCPRE_MASK GENMASK(17, 16)
> +
> +/* STM32 F4 maximum analog clock rate (from datasheet) */
> +#define STM32F4_ADC_MAX_CLK_RATE 36000000
> +
> +/**
> + * struct stm32_adc_priv - stm32 ADC core private data
> + * @irq: irq for ADC block
> + * @domain: irq domain reference
> + * @aclk: clock reference for the analog circuitry
> + * @vref: regulator reference
> + * @common: common data for all ADC instances
> + */
> +struct stm32_adc_priv {
> + int irq;
> + struct irq_domain *domain;
> + struct clk *aclk;
> + struct regulator *vref;
> + struct stm32_adc_common common;
> +};
> +
> +static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
> +{
> + return container_of(com, struct stm32_adc_priv, common);
> +}
> +
> +/* STM32F4 ADC internal common clock prescaler division ratios */
> +static int stm32f4_pclk_div[] = {2, 4, 6, 8};
> +
> +/**
> + * stm32f4_adc_clk_sel() - Select stm32f4 ADC common clock prescaler
> + * @priv: stm32 ADC core private data
> + * Select clock prescaler used for analog conversions, before using ADC.
> + */
> +static int stm32f4_adc_clk_sel(struct platform_device *pdev,
> + struct stm32_adc_priv *priv)
> +{
> + unsigned long rate;
> + u32 val;
> + int i;
> +
> + rate = clk_get_rate(priv->aclk);
> + for (i = 0; i < ARRAY_SIZE(stm32f4_pclk_div); i++) {
> + if ((rate / stm32f4_pclk_div[i]) <= STM32F4_ADC_MAX_CLK_RATE)
> + break;
> + }
> + if (i >= ARRAY_SIZE(stm32f4_pclk_div))
> + return -EINVAL;
> +
> + val = readl_relaxed(priv->common.base + STM32F4_ADC_CCR);
> + val &= ~STM32F4_ADC_ADCPRE_MASK;
> + val |= i << STM32F4_ADC_ADCPRE_SHIFT;
> + writel_relaxed(val, priv->common.base + STM32F4_ADC_CCR);
> +
> + dev_dbg(&pdev->dev, "Using analog clock source at %ld kHz\n",
> + rate / (stm32f4_pclk_div[i] * 1000));
> +
> + return 0;
> +}
> +
> +/* ADC common interrupt for all instances */
> +static void stm32_adc_irq_handler(struct irq_desc *desc)
> +{
> + struct stm32_adc_priv *priv = irq_desc_get_handler_data(desc);
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + u32 status;
> +
> + chained_irq_enter(chip, desc);
> + status = readl_relaxed(priv->common.base + STM32F4_ADC_CSR);
> +
> + if (status & STM32F4_EOC1)
> + generic_handle_irq(irq_find_mapping(priv->domain, 0));
> +
> + if (status & STM32F4_EOC2)
> + generic_handle_irq(irq_find_mapping(priv->domain, 1));
> +
> + if (status & STM32F4_EOC3)
> + generic_handle_irq(irq_find_mapping(priv->domain, 2));
> +
> + chained_irq_exit(chip, desc);
> +};
> +
> +static int stm32_adc_domain_map(struct irq_domain *d, unsigned int irq,
> + irq_hw_number_t hwirq)
> +{
> + irq_set_chip_data(irq, d->host_data);
> + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_level_irq);
> +
> + return 0;
> +}
> +
> +static void stm32_adc_domain_unmap(struct irq_domain *d, unsigned int irq)
> +{
> + irq_set_chip_and_handler(irq, NULL, NULL);
> + irq_set_chip_data(irq, NULL);
> +}
> +
> +static const struct irq_domain_ops stm32_adc_domain_ops = {
> + .map = stm32_adc_domain_map,
> + .unmap = stm32_adc_domain_unmap,
> + .xlate = irq_domain_xlate_onecell,
> +};
> +
> +static int stm32_adc_irq_probe(struct platform_device *pdev,
> + struct stm32_adc_priv *priv)
> +{
> + struct device_node *np = pdev->dev.of_node;
> +
> + priv->irq = platform_get_irq(pdev, 0);
> + if (priv->irq < 0) {
> + dev_err(&pdev->dev, "failed to get irq\n");
> + return priv->irq;
> + }
> +
> + priv->domain = irq_domain_add_simple(np, STM32_ADC_MAX_ADCS, 0,
> + &stm32_adc_domain_ops,
> + priv);
> + if (!priv->domain) {
> + dev_err(&pdev->dev, "Failed to add irq domain\n");
> + return -ENOMEM;
> + }
> +
> + irq_set_chained_handler(priv->irq, stm32_adc_irq_handler);
> + irq_set_handler_data(priv->irq, priv);
> +
> + return 0;
> +}
> +
> +static void stm32_adc_irq_remove(struct platform_device *pdev,
> + struct stm32_adc_priv *priv)
> +{
> + int hwirq;
> +
> + for (hwirq = 0; hwirq < STM32_ADC_MAX_ADCS; hwirq++)
> + irq_dispose_mapping(irq_find_mapping(priv->domain, hwirq));
> + irq_domain_remove(priv->domain);
> + irq_set_chained_handler(priv->irq, NULL);
> +}
> +
> +static int stm32_adc_probe(struct platform_device *pdev)
> +{
> + struct stm32_adc_priv *priv;
> + struct device_node *np = pdev->dev.of_node;
> + struct resource *res;
> + int ret;
> +
> + if (!pdev->dev.of_node)
> + return -ENODEV;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + priv->common.base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(priv->common.base))
> + return PTR_ERR(priv->common.base);
> +
> + priv->vref = devm_regulator_get(&pdev->dev, "vref");
> + if (IS_ERR(priv->vref)) {
> + ret = PTR_ERR(priv->vref);
> + dev_err(&pdev->dev, "vref get failed, %d\n", ret);
> + return ret;
> + }
> +
> + ret = regulator_enable(priv->vref);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "vref enable failed\n");
> + return ret;
> + }
> +
> + ret = regulator_get_voltage(priv->vref);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret);
> + goto err_regulator_disable;
> + }
> + priv->common.vref_mv = ret / 1000;
> + dev_dbg(&pdev->dev, "vref+=%dmV\n", priv->common.vref_mv);
> +
> + priv->aclk = devm_clk_get(&pdev->dev, "adc");
> + if (IS_ERR(priv->aclk)) {
> + ret = PTR_ERR(priv->aclk);
> + dev_err(&pdev->dev, "Can't get 'adc' clock\n");
> + goto err_regulator_disable;
> + }
> +
> + ret = clk_prepare_enable(priv->aclk);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "adc clk enable failed\n");
> + goto err_regulator_disable;
> + }
> +
> + ret = stm32f4_adc_clk_sel(pdev, priv);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "adc clk selection failed\n");
> + goto err_clk_disable;
> + }
> +
> + ret = stm32_adc_irq_probe(pdev, priv);
> + if (ret < 0)
> + goto err_clk_disable;
> +
> + platform_set_drvdata(pdev, &priv->common);
> +
> + ret = of_platform_populate(np, NULL, NULL, &pdev->dev);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to populate DT children\n");
> + goto err_irq_remove;
> + }
> +
> + return 0;
> +
> +err_irq_remove:
> + stm32_adc_irq_remove(pdev, priv);
> +
> +err_clk_disable:
> + clk_disable_unprepare(priv->aclk);
> +
> +err_regulator_disable:
> + regulator_disable(priv->vref);
> +
> + return ret;
> +}
> +
> +static int stm32_adc_remove(struct platform_device *pdev)
> +{
> + struct stm32_adc_common *common = platform_get_drvdata(pdev);
> + struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
> +
> + of_platform_depopulate(&pdev->dev);
> + stm32_adc_irq_remove(pdev, priv);
> + clk_disable_unprepare(priv->aclk);
> + regulator_disable(priv->vref);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id stm32_adc_of_match[] = {
> + { .compatible = "st,stm32f4-adc-core" },
> +};
> +MODULE_DEVICE_TABLE(of, stm32_adc_of_match);
> +
> +static struct platform_driver stm32_adc_driver = {
> + .probe = stm32_adc_probe,
> + .remove = stm32_adc_remove,
> + .driver = {
> + .name = "stm32-adc-core",
> + .of_match_table = stm32_adc_of_match,
> + },
> +};
> +module_platform_driver(stm32_adc_driver);
> +
> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC MFD driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:stm32-adc-core");
> diff --git a/include/linux/mfd/stm32-adc-core.h b/include/linux/mfd/stm32-adc-core.h
> new file mode 100644
> index 0000000..081fa5f
> --- /dev/null
> +++ b/include/linux/mfd/stm32-adc-core.h
> @@ -0,0 +1,52 @@
> +/*
> + * This file is part of STM32 ADC driver
> + *
> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
> + *
> + * License type: GPLv2
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __STM32_ADC_H
> +#define __STM32_ADC_H
> +
> +/*
> + * STM32 - ADC global register map
> + * ________________________________________________________
> + * | Offset | Register |
> + * --------------------------------------------------------
> + * | 0x000 | Master ADC1 |
> + * --------------------------------------------------------
> + * | 0x100 | Slave ADC2 |
> + * --------------------------------------------------------
> + * | 0x200 | Slave ADC3 |
> + * --------------------------------------------------------
> + * | 0x300 | Master & Slave common regs |
> + * --------------------------------------------------------
> + */
> +#define STM32_ADC_MAX_ADCS 3
> +#define STM32_ADCX_COMN_OFFSET 0x300
> +
> +/**
> + * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
> + * @base: control registers base cpu addr
> + * @vref_mv: vref voltage (mv)
> + */
> +struct stm32_adc_common {
> + void __iomem *base;
> + int vref_mv;
> +};
> +
> +#endif
>
^ permalink raw reply
* Re: [PATCH 0/6] Add basic support for support for Rockchip RK1108 SOC
From: 陈豪 @ 2016-11-12 16:02 UTC (permalink / raw)
To: Andy Yan
Cc: Heiko Stuebner, Linus Walleij, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Shawn Lin, linux-clk-u79uwXL29TY76Z2rM5mHXA,
briannorris-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
mturquette-rdvid1DuHRBWk0Htik3J/w, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
elaine.zhang-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <1478175975-11779-1-git-send-email-andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
patches 1-5 on rk1108-cvr-v10 board.
Tested-by: Jacob Chen <jacob2.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-11-03 20:26 GMT+08:00 Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>:
>
> RK1108 is embedded with an ARM Cortex-A7 single core and a DSP core.
> It is designed for varies application scenario such as car DVR, sports
> DV, secure camera and UAV camera.
> This patch series add basic support for it, which can boot a board with
> initramfs into shell.
> More new feathers will come soon.
>
>
> Andy Yan (4):
> pinctrl: rockchip: add support for rk1108
> ARM: dts: add basic support for Rockchip RK1108 SOC
> ARM: add low level debug uart for rk1108
> ARM: dts: rockchip: add rockchip RK1108 Evaluation board
>
> Shawn Lin (2):
> dt-bindings: rockchip-dw-mshc: add RK1108 dw-mshc description
> clk: rockchip: add clock controller for rk1108
>
> Documentation/devicetree/bindings/arm/rockchip.txt | 3 +
> .../devicetree/bindings/mmc/rockchip-dw-mshc.txt | 1 +
> arch/arm/Kconfig.debug | 30 ++
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/rk1108-evb.dts | 69 +++
> arch/arm/boot/dts/rk1108.dtsi | 420 +++++++++++++++++++
> arch/arm/mach-rockchip/rockchip.c | 1 +
> drivers/clk/rockchip/Makefile | 1 +
> drivers/clk/rockchip/clk-rk1108.c | 463 +++++++++++++++++++++
> drivers/clk/rockchip/clk.h | 14 +
> drivers/pinctrl/pinctrl-rockchip.c | 27 +-
> include/dt-bindings/clock/rk1108-cru.h | 308 ++++++++++++++
> 12 files changed, 1337 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm/boot/dts/rk1108-evb.dts
> create mode 100644 arch/arm/boot/dts/rk1108.dtsi
> create mode 100644 drivers/clk/rockchip/clk-rk1108.c
> create mode 100644 include/dt-bindings/clock/rk1108-cru.h
>
> --
> 2.7.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
* Re: [PATCH v2] iio: document bindings for mounting matrices
From: Jonathan Cameron @ 2016-11-12 15:13 UTC (permalink / raw)
To: Linus Walleij, linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: Gregor Boirie, Sebastian Reichel, Samu Onkalo,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478616340-13670-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On 08/11/16 14:45, Linus Walleij wrote:
> The mounting matrix for sensors was introduced in
> commit dfc57732ad38 ("iio:core: mounting matrix support")
>
> However the device tree bindings are very terse and since this is
> a widely applicable property, we need a proper binding for it
> that the other bindings can reference. This will also be useful
> for other operating systems and sensor engineering at large.
>
> I think all 3D sensors should support it, the current situation
> is probably that the mounting information is confined in magic
> userspace components rather than using the mounting matrix, which
> is not good for portability and reuse.
Getting there and much easier to follow than previous version.
Kind of need to avoid glossing over the world magnetic field frame of
reference complexities though (that one is always a PITA to explain).
Jonathan
>
> Cc: Gregor Boirie <gregor.boirie-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org>
> Cc: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Samu Onkalo <samu.onkalo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> ChangeLog v1->v2:
> - Repeat linear algebra and actually get the rotation matrix right.
> - Reference the Wikipedia article on the subject.
> - Fix up text.
> ---
> .../devicetree/bindings/iio/mount-matrix.txt | 108 +++++++++++++++++++++
> 1 file changed, 108 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/mount-matrix.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/mount-matrix.txt b/Documentation/devicetree/bindings/iio/mount-matrix.txt
> new file mode 100644
> index 000000000000..a3714727f739
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/mount-matrix.txt
> @@ -0,0 +1,108 @@
> +Mounting matrix
> +
> +The mounting matrix is a device tree property used to orient any IIO device
> +that produce three-dimensional data in relation to the world where it is
"devices that produce" or "device that produces"
> +deployed.
> +
> +The purpose of the mounting matrix is to translate the sensor frame of
> +reference into the device frame of reference using a translation matrix as
> +defined in linear algebra.
> +
> +The typical usecase is that where a component has an internal representation
> +of the (x,y,z) triplets, such as different registers to read these coordinates,
> +and thus implying that the component should be mounted in a certain orientation
> +relative to some specific device frame of reference.
> +
> +For example a device with some kind of screen, where the user is supposed to
> +interact with the environment using an accelerometer, gyroscope or magnetometer
> +mounted on the same chassis as this screen, will likely take the screen as
> +reference to (x,y,z) orientation, with (x,y) corresponding to these axes on the
> +screen and (z) being depth, the axis perpendicular to the screen.
> +
> +For a screen you probably want (x) coordinates to go from negative on the left
> +to positive on the right and (z) depth to be negative under the screen and
> +positive in front of it, toward the face of the user.
> +
> +A sensor can be mounted in any angle along the axes relative to the frame of
> +reference. This means that the sensor may be flipped upside-down, left-right,
> +or tilted at any angle relative to the frame of reference.
> +
> +Another frame of reference is how the device with its sensor relates to the
> +external world, the environment where the device is deployed. Usually the data
> +from the sensor is used to figure out how the device is oriented with respect
> +to this world. When using the mounting matrix, the sensor and device orientation
> +becomes identical and we can focus on the data as it relates to the surrounding
> +world.
I'm not sure this last sentence is as clear as it might be... Perhaps
When the transformation represented by the mounting matrix has been applied to
sensor data, it may be treated as if it were in the device frame of reference
allowing us to focus on the data as it relates to the surrounding world.
> +
> +Device-to-world examples for some three-dimensional sensor types:
> +
> +- Accelerometers have their world frame of reference toward the center of
Stationary accelerometers (in earth frame)
> + gravity, usually to the core of the planet. A reading of the (x,y,z) values
> + from the sensor will give a projection of the gravity vector through the
> + device relative to the center of the planet, i.e. relative to its surface at
> + this point. Up and down in the world relative to the device frame of
> + reference can thus be determined. and users would likely expect a value of
> + 9.81 m/s^2 upwards along the (z) axis, i.e. out of the screen when the device
> + is held with its screen flat on the planets surface and 0 on the other axes,
> + as the gravity vector is projected 1:1 onto the sensors (z)-axis.
> +
> +- Magnetometers (compasses) have their world frame of reference relative to the
> + geomagnetic field. The system orientation vis-a-vis the world is defined with
> + respect to the local earth geomagnetic reference frame where (y) is in the
> + ground plane and positive towards magnetic North, (x) is in the ground plane,
> + perpendicular to the North axis and positive towards the East and (z) is
> + perpendicular to the ground plane and positive upwards.
Sadly it's not that simple for magnetometers due to the inclination angle...
Stand on the pole and the field points down making the concept of 'upwards'
interesing to define ;)
Some devices use an accelerometer to try and separate the two directions to
get what I think you are describing.
> +
> +- Gyroscopes detects the movement relative the device itself. The angular
> + velocity is defined as orthogonal to the plane of rotation, so if you put the
> + device on a flat surface and spin it around the z axis (such as rotating a
> + device with a screen lying flat on a table), you should get a negative value
> + along the (z) axis if rotated clockwise, and a positive value if rotated
> + counter-clockwise according to the right-hand rule.
> +
> +So unless the sensor is ideally mounted, we need a means to indicate the
> +relative orientation of any given sensor of this type with respect to the
> +frame of reference.
> +
> +To achieve this, use the device tree property "mount-matrix" for the sensor.
> +This supplies a 3x3 rotation matrix in the strict linear algebraic sense,
> +to orient the senor axes relative to a desired point of reference. This means
> +the resulting values from the sensor, after scaling to proper units, should be
> +multiplied by this matrix to give the proper vectors values in three-dimensional
> +space, relative to the device or world point of reference.
> +
> +For more information, consult:
> +https://en.wikipedia.org/wiki/Rotation_matrix
> +
> +The mounting matrix has the layout:
> +
> + (x0, y0, z0)
> + (x1, y1, z1)
> + (x2, y2, z3)
> +
> +And it is represented as an array of strings containing the real values for
> +producing the transformation matrix. The real values use a decimal point and
> +a minus (-) to indicate a negative value.
> +
> +Examples:
> +
> +Identity matrix (nothing happens to the coordinates, which means the device was
> +mechanically mounted in an ideal way and we need no transformation):
> +
> +mount-matrix = "1", "0", "0",
> + "0", "1", "0",
> + "0", "0", "1";
> +
> +The sensor is mounted 30 degrees (Pi/6 radians) tilted along the X axis, so we
> +compensate by performing a -30 degrees rotation around the X axis:
> +
> +mount-matrix = "1", "0", "0",
> + "0", "0.866", "0.5",
> + "0", "-0.5", "0.866";
> +
> +The sensor is flipped 180 degrees (Pi radians) around the Z axis, i.e. mounted
> +upside-down:
> +
> +mount-matrix = "0.998", "0.054", "0",
> + "-0.054", "0.998", "0",
> + "0", "0", "1";
>
^ permalink raw reply
* Re: [PATCH v4 2/3] clocksource: update "fn" at CLOCKSOURCE_OF_DECLARE() of nps400 timer
From: Noam Camus @ 2016-11-12 14:54 UTC (permalink / raw)
To: Daniel Lezcano
Cc: robh+dt@kernel.org, mark.rutland@arm.com, tglx@linutronix.de,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20161111232639.GA1619@mai>
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Saturday, November 12, 2016 1:26 AM
>
...
>>
>> - nps_timer_rate = clk_get_rate(clk);
>> -
>> - ret = clocksource_mmio_init(nps_msu_reg_low_addr, "EZnps-tick",
>> - nps_timer_rate, 301, 32, nps_clksrc_read);
>> + ret = clocksource_mmio_init(nps_msu_reg_low_addr, "nps-tick",
>> + nps_timer1_freq, 301, 32, nps_clksrc_read);
>301 ?
This is historically when this driver reside by ARC driver and I wish to override it.
Now that we have DT control I will restore back the legacy 300 value.
All your comments will be applied in V5
-Noam
[-- Attachment #2: Type: text/html, Size: 2124 bytes --]
^ permalink raw reply
* Re: [PATCH v7 4/9] drivers:input:tsc2007: add iio interface to read external ADC input and temperature
From: Jonathan Cameron @ 2016-11-12 14:19 UTC (permalink / raw)
To: H. Nikolaus Schaller, Sebastian Reichel, Dmitry Torokhov,
Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
Arnd Bergmann, Michael Welling, Mika Penttilä,
Javier Martinez Canillas, Igor Grinberg, Andrew F. Davis,
Mark Brown
Cc: linux-input, devicetree, linux-kernel, linux-omap, letux-kernel,
linux-iio, kernel
In-Reply-To: <fc901399-0bde-d936-b9e2-4c591549eb55@kernel.org>
On 12/11/16 14:04, Jonathan Cameron wrote:
> On 11/11/16 19:02, H. Nikolaus Schaller wrote:
>> The tsc2007 chip not only has a resistive touch screen controller but
>> also an external AUX adc imput which can be used for an ambient
>> light sensor, battery voltage monitoring or any general purpose.
>>
>> Additionally it can measure the chip temperature.
>>
>> This extension provides an iio interface for these adc channels.
>>
>> Since it is not wasting much resources and is very straightforward,
>> we simply provide all other adc channels as optional iio interfaces
>> as weel. This can be used for debugging or special applications.
>>
>> This patch also splits the tsc2007 driver in several source files:
>> tsc2007.h -- constants, structs and stubs
>> tsc2007_core.c -- functional parts of the original driver
>> tsc2007_iio.c -- the optional iio stuff
>>
>> Makefile magic allows to conditionally link the iio
>> stuff if CONFIG_IIO=y in a way that it works with
>> CONFIG_TOUCHSCREEN_TSC2007=m.
>>
>
> I ran a quick build test and it blows up in a number of ways related to
> the #ifdef CONFIG_IIO.
>
> I'm not entirely sure why but
> #if IS_ENABLED(CONFIG_IIO) works fine.
Ah, I'm being sleepy today. It's because I'm building IIO as a module
and the symbol defined would therefore be CONFIG_IIO_MODULE.
The IS_ENABLED macro takes care of both cases.
>
> Otherwise looks good to me
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> Reviewed-by: Jonathan Cameron <jic23@kernel.org>
>> ---
>> drivers/input/touchscreen/Makefile | 2 +
>> drivers/input/touchscreen/tsc2007.h | 118 ++++++++++++++++
>> .../touchscreen/{tsc2007.c => tsc2007_core.c} | 119 ++++++----------
>> drivers/input/touchscreen/tsc2007_iio.c | 152 +++++++++++++++++++++
>> 4 files changed, 310 insertions(+), 81 deletions(-)
>> create mode 100644 drivers/input/touchscreen/tsc2007.h
>> rename drivers/input/touchscreen/{tsc2007.c => tsc2007_core.c} (85%)
>> create mode 100644 drivers/input/touchscreen/tsc2007_iio.c
>>
>> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
>> index 81b8645..d932e2d 100644
>> --- a/drivers/input/touchscreen/Makefile
>> +++ b/drivers/input/touchscreen/Makefile
>> @@ -80,6 +80,8 @@ obj-$(CONFIG_TOUCHSCREEN_TSC_SERIO) += tsc40.o
>> obj-$(CONFIG_TOUCHSCREEN_TSC200X_CORE) += tsc200x-core.o
>> obj-$(CONFIG_TOUCHSCREEN_TSC2004) += tsc2004.o
>> obj-$(CONFIG_TOUCHSCREEN_TSC2005) += tsc2005.o
>> +tsc2007-y := tsc2007_core.o
>> +tsc2007-$(CONFIG_IIO) += tsc2007_iio.o
>> obj-$(CONFIG_TOUCHSCREEN_TSC2007) += tsc2007.o
>> obj-$(CONFIG_TOUCHSCREEN_UCB1400) += ucb1400_ts.o
>> obj-$(CONFIG_TOUCHSCREEN_WACOM_W8001) += wacom_w8001.o
>> diff --git a/drivers/input/touchscreen/tsc2007.h b/drivers/input/touchscreen/tsc2007.h
>> new file mode 100644
>> index 0000000..5049bf8
>> --- /dev/null
>> +++ b/drivers/input/touchscreen/tsc2007.h
>> @@ -0,0 +1,118 @@
>> +/*
>> + * drivers/input/touchscreen/tsc2007.h
>> + *
>> + * Copyright (c) 2008 MtekVision Co., Ltd.
>> + * Kwangwoo Lee <kwlee@mtekvision.com>
>> + *
>> + * Using code from:
>> + * - ads7846.c
>> + * Copyright (c) 2005 David Brownell
>> + * Copyright (c) 2006 Nokia Corporation
>> + * - corgi_ts.c
>> + * Copyright (C) 2004-2005 Richard Purdie
>> + * - omap_ts.[hc], ads7846.h, ts_osk.c
>> + * Copyright (C) 2002 MontaVista Software
>> + * Copyright (C) 2004 Texas Instruments
>> + * Copyright (C) 2005 Dirk Behme
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/input/touchscreen.h>
>> +
>> +#define TSC2007_MEASURE_TEMP0 (0x0 << 4)
>> +#define TSC2007_MEASURE_AUX (0x2 << 4)
>> +#define TSC2007_MEASURE_TEMP1 (0x4 << 4)
>> +#define TSC2007_ACTIVATE_XN (0x8 << 4)
>> +#define TSC2007_ACTIVATE_YN (0x9 << 4)
>> +#define TSC2007_ACTIVATE_YP_XN (0xa << 4)
>> +#define TSC2007_SETUP (0xb << 4)
>> +#define TSC2007_MEASURE_X (0xc << 4)
>> +#define TSC2007_MEASURE_Y (0xd << 4)
>> +#define TSC2007_MEASURE_Z1 (0xe << 4)
>> +#define TSC2007_MEASURE_Z2 (0xf << 4)
>> +
>> +#define TSC2007_POWER_OFF_IRQ_EN (0x0 << 2)
>> +#define TSC2007_ADC_ON_IRQ_DIS0 (0x1 << 2)
>> +#define TSC2007_ADC_OFF_IRQ_EN (0x2 << 2)
>> +#define TSC2007_ADC_ON_IRQ_DIS1 (0x3 << 2)
>> +
>> +#define TSC2007_12BIT (0x0 << 1)
>> +#define TSC2007_8BIT (0x1 << 1)
>> +
>> +#define MAX_12BIT ((1 << 12) - 1)
>> +
>> +#define ADC_ON_12BIT (TSC2007_12BIT | TSC2007_ADC_ON_IRQ_DIS0)
>> +
>> +#define READ_Y (ADC_ON_12BIT | TSC2007_MEASURE_Y)
>> +#define READ_Z1 (ADC_ON_12BIT | TSC2007_MEASURE_Z1)
>> +#define READ_Z2 (ADC_ON_12BIT | TSC2007_MEASURE_Z2)
>> +#define READ_X (ADC_ON_12BIT | TSC2007_MEASURE_X)
>> +#define PWRDOWN (TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
>> +
>> +struct ts_event {
>> + u16 x;
>> + u16 y;
>> + u16 z1, z2;
>> +};
>> +
>> +struct tsc2007 {
>> + struct input_dev *input;
>> + char phys[32];
>> +
>> + struct i2c_client *client;
>> +
>> + u16 model;
>> + u16 x_plate_ohms;
>> +
>> + struct touchscreen_properties prop;
>> +
>> + bool report_resistance;
>> + u16 min_x;
>> + u16 min_y;
>> + u16 max_x;
>> + u16 max_y;
>> + u16 max_rt;
>> + unsigned long poll_period; /* in jiffies */
>> + int fuzzx;
>> + int fuzzy;
>> + int fuzzz;
>> +
>> + unsigned int gpio;
>> + int irq;
>> +
>> + wait_queue_head_t wait;
>> + bool stopped;
>> + bool pendown;
>> +
>> + int (*get_pendown_state)(struct device *);
>> + void (*clear_penirq)(void);
>> +
>> + struct mutex mlock;
>> + struct iio_dev *iio_dev; /* optional */
>> +};
>> +
>> +int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd);
>> +u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
>> + struct ts_event *tc);
>> +bool tsc2007_is_pen_down(struct tsc2007 *ts);
>> +
>> +#ifdef CONFIG_IIO
>> +
>> +/* defined in tsc2007_iio.c */
>> +int tsc2007_iio_configure(struct tsc2007 *ts);
>> +void tsc2007_iio_unconfigure(struct tsc2007 *ts);
>> +
>> +#else /* CONFIG_IIO */
>> +
>> +static inline int tsc2007_iio_configure(struct tsc2007 *ts)
>> +{
>> + return 0;
>> +}
>> +static inline void tsc2007_iio_unconfigure(struct tsc2007 *ts)
>> +{
>> +}
>> +
>> +#endif /* CONFIG_IIO */
>> diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007_core.c
>> similarity index 85%
>> rename from drivers/input/touchscreen/tsc2007.c
>> rename to drivers/input/touchscreen/tsc2007_core.c
>> index 5e3c4bf..72775ae 100644
>> --- a/drivers/input/touchscreen/tsc2007.c
>> +++ b/drivers/input/touchscreen/tsc2007_core.c
>> @@ -27,80 +27,10 @@
>> #include <linux/i2c.h>
>> #include <linux/i2c/tsc2007.h>
>> #include <linux/of_device.h>
>> -#include <linux/of.h>
>> #include <linux/of_gpio.h>
>> -#include <linux/input/touchscreen.h>
>> -
>> -#define TSC2007_MEASURE_TEMP0 (0x0 << 4)
>> -#define TSC2007_MEASURE_AUX (0x2 << 4)
>> -#define TSC2007_MEASURE_TEMP1 (0x4 << 4)
>> -#define TSC2007_ACTIVATE_XN (0x8 << 4)
>> -#define TSC2007_ACTIVATE_YN (0x9 << 4)
>> -#define TSC2007_ACTIVATE_YP_XN (0xa << 4)
>> -#define TSC2007_SETUP (0xb << 4)
>> -#define TSC2007_MEASURE_X (0xc << 4)
>> -#define TSC2007_MEASURE_Y (0xd << 4)
>> -#define TSC2007_MEASURE_Z1 (0xe << 4)
>> -#define TSC2007_MEASURE_Z2 (0xf << 4)
>> -
>> -#define TSC2007_POWER_OFF_IRQ_EN (0x0 << 2)
>> -#define TSC2007_ADC_ON_IRQ_DIS0 (0x1 << 2)
>> -#define TSC2007_ADC_OFF_IRQ_EN (0x2 << 2)
>> -#define TSC2007_ADC_ON_IRQ_DIS1 (0x3 << 2)
>> -
>> -#define TSC2007_12BIT (0x0 << 1)
>> -#define TSC2007_8BIT (0x1 << 1)
>> -
>> -#define MAX_12BIT ((1 << 12) - 1)
>> -
>> -#define ADC_ON_12BIT (TSC2007_12BIT | TSC2007_ADC_ON_IRQ_DIS0)
>> -
>> -#define READ_Y (ADC_ON_12BIT | TSC2007_MEASURE_Y)
>> -#define READ_Z1 (ADC_ON_12BIT | TSC2007_MEASURE_Z1)
>> -#define READ_Z2 (ADC_ON_12BIT | TSC2007_MEASURE_Z2)
>> -#define READ_X (ADC_ON_12BIT | TSC2007_MEASURE_X)
>> -#define PWRDOWN (TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
>> -
>> -struct ts_event {
>> - u16 x;
>> - u16 y;
>> - u16 z1, z2;
>> -};
>> -
>> -struct tsc2007 {
>> - struct input_dev *input;
>> - char phys[32];
>> -
>> - struct i2c_client *client;
>> -
>> - u16 model;
>> - u16 x_plate_ohms;
>> -
>> - struct touchscreen_properties prop;
>> -
>> - bool report_resistance;
>> - u16 min_x;
>> - u16 min_y;
>> - u16 max_x;
>> - u16 max_y;
>> - u16 max_rt;
>> - unsigned long poll_period; /* in jiffies */
>> - int fuzzx;
>> - int fuzzy;
>> - int fuzzz;
>> +#include "tsc2007.h"
>>
>> - unsigned gpio;
>> - int irq;
>> -
>> - wait_queue_head_t wait;
>> - bool stopped;
>> - bool pendown;
>> -
>> - int (*get_pendown_state)(struct device *);
>> - void (*clear_penirq)(void);
>> -};
>> -
>> -static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
>> +int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
>> {
>> s32 data;
>> u16 val;
>> @@ -121,6 +51,7 @@ static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
>>
>> return val;
>> }
>> +EXPORT_SYMBOL(tsc2007_xfer);
>>
>> static void tsc2007_read_values(struct tsc2007 *tsc, struct ts_event *tc)
>> {
>> @@ -138,7 +69,7 @@ static void tsc2007_read_values(struct tsc2007 *tsc, struct ts_event *tc)
>> tsc2007_xfer(tsc, PWRDOWN);
>> }
>>
>> -static u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
>> +u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
>> struct ts_event *tc)
>> {
>> u32 rt = 0;
>> @@ -158,8 +89,9 @@ static u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
>>
>> return rt;
>> }
>> +EXPORT_SYMBOL(tsc2007_calculate_resistance);
>>
>> -static bool tsc2007_is_pen_down(struct tsc2007 *ts)
>> +bool tsc2007_is_pen_down(struct tsc2007 *ts)
>> {
>> /*
>> * NOTE: We can't rely on the pressure to determine the pen down
>> @@ -180,6 +112,7 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
>>
>> return ts->get_pendown_state(&ts->client->dev);
>> }
>> +EXPORT_SYMBOL(tsc2007_is_pen_down);
>>
>> static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
>> {
>> @@ -192,7 +125,10 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
>> while (!ts->stopped && tsc2007_is_pen_down(ts)) {
>>
>> /* pen is down, continue with the measurement */
>> +
>> + mutex_lock(&ts->mlock);
>> tsc2007_read_values(ts, &tc);
>> + mutex_unlock(&ts->mlock);
>>
>> rt = tsc2007_calculate_resistance(ts, &tc);
>>
>> @@ -450,7 +386,8 @@ static void tsc2007_call_exit_platform_hw(void *data)
>> static int tsc2007_probe(struct i2c_client *client,
>> const struct i2c_device_id *id)
>> {
>> - const struct tsc2007_platform_data *pdata = dev_get_platdata(&client->dev);
>> + const struct tsc2007_platform_data *pdata =
>> + dev_get_platdata(&client->dev);
>> struct tsc2007 *ts;
>> struct input_dev *input_dev;
>> int err;
>> @@ -472,7 +409,13 @@ static int tsc2007_probe(struct i2c_client *client,
>> ts->client = client;
>> ts->irq = client->irq;
>> ts->input = input_dev;
>> +
>> + err = tsc2007_iio_configure(ts);
>> + if (err < 0)
>> + return err;
>> +
>> init_waitqueue_head(&ts->wait);
>> + mutex_init(&ts->mlock);
>>
>> snprintf(ts->phys, sizeof(ts->phys),
>> "%s/input0", dev_name(&client->dev));
>> @@ -494,7 +437,7 @@ static int tsc2007_probe(struct i2c_client *client,
>> if (pdata) {
>> err = tsc2007_probe_pdev(client, ts, pdata, id);
>> if (err)
>> - return err;
>> + goto probe_err;
>> input_set_abs_params(input_dev, ABS_X, 0, ts->max_x-ts->min_x,
>> ts->fuzzx, 0);
>> input_set_abs_params(input_dev, ABS_Y, 0, ts->max_y-ts->min_y,
>> @@ -504,7 +447,7 @@ static int tsc2007_probe(struct i2c_client *client,
>> } else {
>> err = tsc2007_probe_dt(client, ts);
>> if (err)
>> - return err;
>> + goto probe_err;
>> }
>>
>> if (pdata) {
>> @@ -516,7 +459,7 @@ static int tsc2007_probe(struct i2c_client *client,
>> dev_err(&client->dev,
>> "Failed to register exit_platform_hw action, %d\n",
>> err);
>> - return err;
>> + goto probe_err;
>> }
>> }
>>
>> @@ -533,7 +476,7 @@ static int tsc2007_probe(struct i2c_client *client,
>> if (err) {
>> dev_err(&client->dev, "Failed to request irq %d: %d\n",
>> ts->irq, err);
>> - return err;
>> + goto probe_err;
>> }
>>
>> tsc2007_stop(ts);
>> @@ -543,17 +486,30 @@ static int tsc2007_probe(struct i2c_client *client,
>> if (err < 0) {
>> dev_err(&client->dev,
>> "Failed to setup chip: %d\n", err);
>> - return err; /* usually, chip does not respond */
>> + goto probe_err; /* chip does not respond */
>> }
>>
>> err = input_register_device(input_dev);
>> if (err) {
>> dev_err(&client->dev,
>> "Failed to register input device: %d\n", err);
>> - return err;
>> + goto probe_err;
>> }
>>
>> return 0;
>> +
>> +probe_err:
>> + tsc2007_iio_unconfigure(ts);
>> + return err;
>> +}
>> +
>> +static int tsc2007_remove(struct i2c_client *client)
>> +{
>> + struct tsc2007 *ts = i2c_get_clientdata(client);
>> +
>> + tsc2007_iio_unconfigure(ts);
>> + input_unregister_device(ts->input);
>> + return 0;
>> }
>>
>> static const struct i2c_device_id tsc2007_idtable[] = {
>> @@ -578,6 +534,7 @@ static struct i2c_driver tsc2007_driver = {
>> },
>> .id_table = tsc2007_idtable,
>> .probe = tsc2007_probe,
>> + .remove = tsc2007_remove,
>> };
>>
>> module_i2c_driver(tsc2007_driver);
>> diff --git a/drivers/input/touchscreen/tsc2007_iio.c b/drivers/input/touchscreen/tsc2007_iio.c
>> new file mode 100644
>> index 0000000..e492bba
>> --- /dev/null
>> +++ b/drivers/input/touchscreen/tsc2007_iio.c
>> @@ -0,0 +1,152 @@
>> +/*
>> + * drivers/input/touchscreen/tsc2007_iio.c
>> + *
>> + * Copyright (c) 2016 Golden Delicious Comp. GmbH&Co. KG
>> + * Nikolaus Schaller <hns@goldelico.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.
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/iio/iio.h>
>> +#include "tsc2007.h"
>> +
>> +struct tsc2007_iio {
>> + struct tsc2007 *ts;
>> +};
>> +
>> +#define TSC2007_CHAN_IIO(_chan, _name, _type, _chan_info) \
>> +{ \
>> + .datasheet_name = _name, \
>> + .type = _type, \
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>> + BIT(_chan_info), \
>> + .indexed = 1, \
>> + .channel = _chan, \
>> +}
>> +
>> +static const struct iio_chan_spec tsc2007_iio_channel[] = {
>> + TSC2007_CHAN_IIO(0, "x", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>> + TSC2007_CHAN_IIO(1, "y", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>> + TSC2007_CHAN_IIO(2, "z1", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>> + TSC2007_CHAN_IIO(3, "z2", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>> + TSC2007_CHAN_IIO(4, "adc", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>> + TSC2007_CHAN_IIO(5, "rt", IIO_VOLTAGE, IIO_CHAN_INFO_RAW), /* Ohms? */
>> + TSC2007_CHAN_IIO(6, "pen", IIO_PRESSURE, IIO_CHAN_INFO_RAW),
>> + TSC2007_CHAN_IIO(7, "temp0", IIO_TEMP, IIO_CHAN_INFO_RAW),
>> + TSC2007_CHAN_IIO(8, "temp1", IIO_TEMP, IIO_CHAN_INFO_RAW),
>> +};
>> +
>> +static int tsc2007_read_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan, int *val, int *val2, long mask)
>> +{
>> + struct tsc2007_iio *iio = iio_priv(indio_dev);
>> + struct tsc2007 *tsc = iio->ts;
>> + int adc_chan = chan->channel;
>> + int ret = 0;
>> +
>> + if (adc_chan >= ARRAY_SIZE(tsc2007_iio_channel))
>> + return -EINVAL;
>> +
>> + if (mask != IIO_CHAN_INFO_RAW)
>> + return -EINVAL;
>> +
>> + mutex_lock(&tsc->mlock);
>> +
>> + switch (chan->channel) {
>> + case 0:
>> + *val = tsc2007_xfer(tsc, READ_X);
>> + break;
>> + case 1:
>> + *val = tsc2007_xfer(tsc, READ_Y);
>> + break;
>> + case 2:
>> + *val = tsc2007_xfer(tsc, READ_Z1);
>> + break;
>> + case 3:
>> + *val = tsc2007_xfer(tsc, READ_Z2);
>> + break;
>> + case 4:
>> + *val = tsc2007_xfer(tsc, (ADC_ON_12BIT | TSC2007_MEASURE_AUX));
>> + break;
>> + case 5: {
>> + struct ts_event tc;
>> +
>> + tc.x = tsc2007_xfer(tsc, READ_X);
>> + tc.z1 = tsc2007_xfer(tsc, READ_Z1);
>> + tc.z2 = tsc2007_xfer(tsc, READ_Z2);
>> + *val = tsc2007_calculate_resistance(tsc, &tc);
>> + break;
>> + }
>> + case 6:
>> + *val = tsc2007_is_pen_down(tsc);
>> + break;
>> + case 7:
>> + *val = tsc2007_xfer(tsc,
>> + (ADC_ON_12BIT | TSC2007_MEASURE_TEMP0));
>> + break;
>> + case 8:
>> + *val = tsc2007_xfer(tsc,
>> + (ADC_ON_12BIT | TSC2007_MEASURE_TEMP1));
>> + break;
>> + }
>> +
>> + /* Prepare for next touch reading - power down ADC, enable PENIRQ */
>> + tsc2007_xfer(tsc, PWRDOWN);
>> +
>> + mutex_unlock(&tsc->mlock);
>> +
>> + ret = IIO_VAL_INT;
>> +
>> + return ret;
>> +}
>> +
>> +static const struct iio_info tsc2007_iio_info = {
>> + .read_raw = tsc2007_read_raw,
>> + .driver_module = THIS_MODULE,
>> +};
>> +
>> +int tsc2007_iio_configure(struct tsc2007 *ts)
>> +{
>> + int err;
>> + struct iio_dev *indio_dev;
>> + struct tsc2007_iio *iio;
>> +
>> + indio_dev = devm_iio_device_alloc(&ts->client->dev,
>> + sizeof(struct tsc2007_iio));
>> + if (!indio_dev) {
>> + dev_err(&ts->client->dev, "iio_device_alloc failed\n");
>> + return -ENOMEM;
>> + }
>> +
>> + iio = iio_priv(indio_dev);
>> + iio->ts = ts;
>> + ts->iio_dev = (void *) indio_dev;
>> +
>> + indio_dev->name = "tsc2007";
>> + indio_dev->dev.parent = &ts->client->dev;
>> + indio_dev->info = &tsc2007_iio_info;
>> + indio_dev->modes = INDIO_DIRECT_MODE;
>> + indio_dev->channels = tsc2007_iio_channel;
>> + indio_dev->num_channels = ARRAY_SIZE(tsc2007_iio_channel);
>> +
>> + err = iio_device_register(indio_dev);
>> + if (err < 0) {
>> + dev_err(&ts->client->dev, "iio_device_register() failed: %d\n",
>> + err);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(tsc2007_iio_configure);
>> +
>> +void tsc2007_iio_unconfigure(struct tsc2007 *ts)
>> +{
>> + struct iio_dev *indio_dev = ts->iio_dev;
>> +
>> + iio_device_unregister(indio_dev);
>> +}
>> +EXPORT_SYMBOL(tsc2007_iio_unconfigure);
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v7 4/9] drivers:input:tsc2007: add iio interface to read external ADC input and temperature
From: Jonathan Cameron @ 2016-11-12 14:04 UTC (permalink / raw)
To: H. Nikolaus Schaller, Sebastian Reichel, Dmitry Torokhov,
Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
Arnd Bergmann, Michael Welling, Mika Penttilä,
Javier Martinez Canillas, Igor Grinberg, Andrew F. Davis,
Mark Brown
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
letux-kernel-S0jZdbWzriLCfDggNXIi3w,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf
In-Reply-To: <6c64bb0331ad05840a2886bef6c3cfa64c866cb6.1478890925.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
On 11/11/16 19:02, H. Nikolaus Schaller wrote:
> The tsc2007 chip not only has a resistive touch screen controller but
> also an external AUX adc imput which can be used for an ambient
> light sensor, battery voltage monitoring or any general purpose.
>
> Additionally it can measure the chip temperature.
>
> This extension provides an iio interface for these adc channels.
>
> Since it is not wasting much resources and is very straightforward,
> we simply provide all other adc channels as optional iio interfaces
> as weel. This can be used for debugging or special applications.
>
> This patch also splits the tsc2007 driver in several source files:
> tsc2007.h -- constants, structs and stubs
> tsc2007_core.c -- functional parts of the original driver
> tsc2007_iio.c -- the optional iio stuff
>
> Makefile magic allows to conditionally link the iio
> stuff if CONFIG_IIO=y in a way that it works with
> CONFIG_TOUCHSCREEN_TSC2007=m.
>
I ran a quick build test and it blows up in a number of ways related to
the #ifdef CONFIG_IIO.
I'm not entirely sure why but
#if IS_ENABLED(CONFIG_IIO) works fine.
Otherwise looks good to me
> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
Reviewed-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> drivers/input/touchscreen/Makefile | 2 +
> drivers/input/touchscreen/tsc2007.h | 118 ++++++++++++++++
> .../touchscreen/{tsc2007.c => tsc2007_core.c} | 119 ++++++----------
> drivers/input/touchscreen/tsc2007_iio.c | 152 +++++++++++++++++++++
> 4 files changed, 310 insertions(+), 81 deletions(-)
> create mode 100644 drivers/input/touchscreen/tsc2007.h
> rename drivers/input/touchscreen/{tsc2007.c => tsc2007_core.c} (85%)
> create mode 100644 drivers/input/touchscreen/tsc2007_iio.c
>
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 81b8645..d932e2d 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -80,6 +80,8 @@ obj-$(CONFIG_TOUCHSCREEN_TSC_SERIO) += tsc40.o
> obj-$(CONFIG_TOUCHSCREEN_TSC200X_CORE) += tsc200x-core.o
> obj-$(CONFIG_TOUCHSCREEN_TSC2004) += tsc2004.o
> obj-$(CONFIG_TOUCHSCREEN_TSC2005) += tsc2005.o
> +tsc2007-y := tsc2007_core.o
> +tsc2007-$(CONFIG_IIO) += tsc2007_iio.o
> obj-$(CONFIG_TOUCHSCREEN_TSC2007) += tsc2007.o
> obj-$(CONFIG_TOUCHSCREEN_UCB1400) += ucb1400_ts.o
> obj-$(CONFIG_TOUCHSCREEN_WACOM_W8001) += wacom_w8001.o
> diff --git a/drivers/input/touchscreen/tsc2007.h b/drivers/input/touchscreen/tsc2007.h
> new file mode 100644
> index 0000000..5049bf8
> --- /dev/null
> +++ b/drivers/input/touchscreen/tsc2007.h
> @@ -0,0 +1,118 @@
> +/*
> + * drivers/input/touchscreen/tsc2007.h
> + *
> + * Copyright (c) 2008 MtekVision Co., Ltd.
> + * Kwangwoo Lee <kwlee-ec7hoAtq5SbSUeElwK9/Pw@public.gmane.org>
> + *
> + * Using code from:
> + * - ads7846.c
> + * Copyright (c) 2005 David Brownell
> + * Copyright (c) 2006 Nokia Corporation
> + * - corgi_ts.c
> + * Copyright (C) 2004-2005 Richard Purdie
> + * - omap_ts.[hc], ads7846.h, ts_osk.c
> + * Copyright (C) 2002 MontaVista Software
> + * Copyright (C) 2004 Texas Instruments
> + * Copyright (C) 2005 Dirk Behme
> + *
> + * 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.
> + */
> +
> +#include <linux/input/touchscreen.h>
> +
> +#define TSC2007_MEASURE_TEMP0 (0x0 << 4)
> +#define TSC2007_MEASURE_AUX (0x2 << 4)
> +#define TSC2007_MEASURE_TEMP1 (0x4 << 4)
> +#define TSC2007_ACTIVATE_XN (0x8 << 4)
> +#define TSC2007_ACTIVATE_YN (0x9 << 4)
> +#define TSC2007_ACTIVATE_YP_XN (0xa << 4)
> +#define TSC2007_SETUP (0xb << 4)
> +#define TSC2007_MEASURE_X (0xc << 4)
> +#define TSC2007_MEASURE_Y (0xd << 4)
> +#define TSC2007_MEASURE_Z1 (0xe << 4)
> +#define TSC2007_MEASURE_Z2 (0xf << 4)
> +
> +#define TSC2007_POWER_OFF_IRQ_EN (0x0 << 2)
> +#define TSC2007_ADC_ON_IRQ_DIS0 (0x1 << 2)
> +#define TSC2007_ADC_OFF_IRQ_EN (0x2 << 2)
> +#define TSC2007_ADC_ON_IRQ_DIS1 (0x3 << 2)
> +
> +#define TSC2007_12BIT (0x0 << 1)
> +#define TSC2007_8BIT (0x1 << 1)
> +
> +#define MAX_12BIT ((1 << 12) - 1)
> +
> +#define ADC_ON_12BIT (TSC2007_12BIT | TSC2007_ADC_ON_IRQ_DIS0)
> +
> +#define READ_Y (ADC_ON_12BIT | TSC2007_MEASURE_Y)
> +#define READ_Z1 (ADC_ON_12BIT | TSC2007_MEASURE_Z1)
> +#define READ_Z2 (ADC_ON_12BIT | TSC2007_MEASURE_Z2)
> +#define READ_X (ADC_ON_12BIT | TSC2007_MEASURE_X)
> +#define PWRDOWN (TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
> +
> +struct ts_event {
> + u16 x;
> + u16 y;
> + u16 z1, z2;
> +};
> +
> +struct tsc2007 {
> + struct input_dev *input;
> + char phys[32];
> +
> + struct i2c_client *client;
> +
> + u16 model;
> + u16 x_plate_ohms;
> +
> + struct touchscreen_properties prop;
> +
> + bool report_resistance;
> + u16 min_x;
> + u16 min_y;
> + u16 max_x;
> + u16 max_y;
> + u16 max_rt;
> + unsigned long poll_period; /* in jiffies */
> + int fuzzx;
> + int fuzzy;
> + int fuzzz;
> +
> + unsigned int gpio;
> + int irq;
> +
> + wait_queue_head_t wait;
> + bool stopped;
> + bool pendown;
> +
> + int (*get_pendown_state)(struct device *);
> + void (*clear_penirq)(void);
> +
> + struct mutex mlock;
> + struct iio_dev *iio_dev; /* optional */
> +};
> +
> +int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd);
> +u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
> + struct ts_event *tc);
> +bool tsc2007_is_pen_down(struct tsc2007 *ts);
> +
> +#ifdef CONFIG_IIO
> +
> +/* defined in tsc2007_iio.c */
> +int tsc2007_iio_configure(struct tsc2007 *ts);
> +void tsc2007_iio_unconfigure(struct tsc2007 *ts);
> +
> +#else /* CONFIG_IIO */
> +
> +static inline int tsc2007_iio_configure(struct tsc2007 *ts)
> +{
> + return 0;
> +}
> +static inline void tsc2007_iio_unconfigure(struct tsc2007 *ts)
> +{
> +}
> +
> +#endif /* CONFIG_IIO */
> diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007_core.c
> similarity index 85%
> rename from drivers/input/touchscreen/tsc2007.c
> rename to drivers/input/touchscreen/tsc2007_core.c
> index 5e3c4bf..72775ae 100644
> --- a/drivers/input/touchscreen/tsc2007.c
> +++ b/drivers/input/touchscreen/tsc2007_core.c
> @@ -27,80 +27,10 @@
> #include <linux/i2c.h>
> #include <linux/i2c/tsc2007.h>
> #include <linux/of_device.h>
> -#include <linux/of.h>
> #include <linux/of_gpio.h>
> -#include <linux/input/touchscreen.h>
> -
> -#define TSC2007_MEASURE_TEMP0 (0x0 << 4)
> -#define TSC2007_MEASURE_AUX (0x2 << 4)
> -#define TSC2007_MEASURE_TEMP1 (0x4 << 4)
> -#define TSC2007_ACTIVATE_XN (0x8 << 4)
> -#define TSC2007_ACTIVATE_YN (0x9 << 4)
> -#define TSC2007_ACTIVATE_YP_XN (0xa << 4)
> -#define TSC2007_SETUP (0xb << 4)
> -#define TSC2007_MEASURE_X (0xc << 4)
> -#define TSC2007_MEASURE_Y (0xd << 4)
> -#define TSC2007_MEASURE_Z1 (0xe << 4)
> -#define TSC2007_MEASURE_Z2 (0xf << 4)
> -
> -#define TSC2007_POWER_OFF_IRQ_EN (0x0 << 2)
> -#define TSC2007_ADC_ON_IRQ_DIS0 (0x1 << 2)
> -#define TSC2007_ADC_OFF_IRQ_EN (0x2 << 2)
> -#define TSC2007_ADC_ON_IRQ_DIS1 (0x3 << 2)
> -
> -#define TSC2007_12BIT (0x0 << 1)
> -#define TSC2007_8BIT (0x1 << 1)
> -
> -#define MAX_12BIT ((1 << 12) - 1)
> -
> -#define ADC_ON_12BIT (TSC2007_12BIT | TSC2007_ADC_ON_IRQ_DIS0)
> -
> -#define READ_Y (ADC_ON_12BIT | TSC2007_MEASURE_Y)
> -#define READ_Z1 (ADC_ON_12BIT | TSC2007_MEASURE_Z1)
> -#define READ_Z2 (ADC_ON_12BIT | TSC2007_MEASURE_Z2)
> -#define READ_X (ADC_ON_12BIT | TSC2007_MEASURE_X)
> -#define PWRDOWN (TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
> -
> -struct ts_event {
> - u16 x;
> - u16 y;
> - u16 z1, z2;
> -};
> -
> -struct tsc2007 {
> - struct input_dev *input;
> - char phys[32];
> -
> - struct i2c_client *client;
> -
> - u16 model;
> - u16 x_plate_ohms;
> -
> - struct touchscreen_properties prop;
> -
> - bool report_resistance;
> - u16 min_x;
> - u16 min_y;
> - u16 max_x;
> - u16 max_y;
> - u16 max_rt;
> - unsigned long poll_period; /* in jiffies */
> - int fuzzx;
> - int fuzzy;
> - int fuzzz;
> +#include "tsc2007.h"
>
> - unsigned gpio;
> - int irq;
> -
> - wait_queue_head_t wait;
> - bool stopped;
> - bool pendown;
> -
> - int (*get_pendown_state)(struct device *);
> - void (*clear_penirq)(void);
> -};
> -
> -static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
> +int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
> {
> s32 data;
> u16 val;
> @@ -121,6 +51,7 @@ static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
>
> return val;
> }
> +EXPORT_SYMBOL(tsc2007_xfer);
>
> static void tsc2007_read_values(struct tsc2007 *tsc, struct ts_event *tc)
> {
> @@ -138,7 +69,7 @@ static void tsc2007_read_values(struct tsc2007 *tsc, struct ts_event *tc)
> tsc2007_xfer(tsc, PWRDOWN);
> }
>
> -static u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
> +u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
> struct ts_event *tc)
> {
> u32 rt = 0;
> @@ -158,8 +89,9 @@ static u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
>
> return rt;
> }
> +EXPORT_SYMBOL(tsc2007_calculate_resistance);
>
> -static bool tsc2007_is_pen_down(struct tsc2007 *ts)
> +bool tsc2007_is_pen_down(struct tsc2007 *ts)
> {
> /*
> * NOTE: We can't rely on the pressure to determine the pen down
> @@ -180,6 +112,7 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
>
> return ts->get_pendown_state(&ts->client->dev);
> }
> +EXPORT_SYMBOL(tsc2007_is_pen_down);
>
> static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
> {
> @@ -192,7 +125,10 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
> while (!ts->stopped && tsc2007_is_pen_down(ts)) {
>
> /* pen is down, continue with the measurement */
> +
> + mutex_lock(&ts->mlock);
> tsc2007_read_values(ts, &tc);
> + mutex_unlock(&ts->mlock);
>
> rt = tsc2007_calculate_resistance(ts, &tc);
>
> @@ -450,7 +386,8 @@ static void tsc2007_call_exit_platform_hw(void *data)
> static int tsc2007_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> - const struct tsc2007_platform_data *pdata = dev_get_platdata(&client->dev);
> + const struct tsc2007_platform_data *pdata =
> + dev_get_platdata(&client->dev);
> struct tsc2007 *ts;
> struct input_dev *input_dev;
> int err;
> @@ -472,7 +409,13 @@ static int tsc2007_probe(struct i2c_client *client,
> ts->client = client;
> ts->irq = client->irq;
> ts->input = input_dev;
> +
> + err = tsc2007_iio_configure(ts);
> + if (err < 0)
> + return err;
> +
> init_waitqueue_head(&ts->wait);
> + mutex_init(&ts->mlock);
>
> snprintf(ts->phys, sizeof(ts->phys),
> "%s/input0", dev_name(&client->dev));
> @@ -494,7 +437,7 @@ static int tsc2007_probe(struct i2c_client *client,
> if (pdata) {
> err = tsc2007_probe_pdev(client, ts, pdata, id);
> if (err)
> - return err;
> + goto probe_err;
> input_set_abs_params(input_dev, ABS_X, 0, ts->max_x-ts->min_x,
> ts->fuzzx, 0);
> input_set_abs_params(input_dev, ABS_Y, 0, ts->max_y-ts->min_y,
> @@ -504,7 +447,7 @@ static int tsc2007_probe(struct i2c_client *client,
> } else {
> err = tsc2007_probe_dt(client, ts);
> if (err)
> - return err;
> + goto probe_err;
> }
>
> if (pdata) {
> @@ -516,7 +459,7 @@ static int tsc2007_probe(struct i2c_client *client,
> dev_err(&client->dev,
> "Failed to register exit_platform_hw action, %d\n",
> err);
> - return err;
> + goto probe_err;
> }
> }
>
> @@ -533,7 +476,7 @@ static int tsc2007_probe(struct i2c_client *client,
> if (err) {
> dev_err(&client->dev, "Failed to request irq %d: %d\n",
> ts->irq, err);
> - return err;
> + goto probe_err;
> }
>
> tsc2007_stop(ts);
> @@ -543,17 +486,30 @@ static int tsc2007_probe(struct i2c_client *client,
> if (err < 0) {
> dev_err(&client->dev,
> "Failed to setup chip: %d\n", err);
> - return err; /* usually, chip does not respond */
> + goto probe_err; /* chip does not respond */
> }
>
> err = input_register_device(input_dev);
> if (err) {
> dev_err(&client->dev,
> "Failed to register input device: %d\n", err);
> - return err;
> + goto probe_err;
> }
>
> return 0;
> +
> +probe_err:
> + tsc2007_iio_unconfigure(ts);
> + return err;
> +}
> +
> +static int tsc2007_remove(struct i2c_client *client)
> +{
> + struct tsc2007 *ts = i2c_get_clientdata(client);
> +
> + tsc2007_iio_unconfigure(ts);
> + input_unregister_device(ts->input);
> + return 0;
> }
>
> static const struct i2c_device_id tsc2007_idtable[] = {
> @@ -578,6 +534,7 @@ static struct i2c_driver tsc2007_driver = {
> },
> .id_table = tsc2007_idtable,
> .probe = tsc2007_probe,
> + .remove = tsc2007_remove,
> };
>
> module_i2c_driver(tsc2007_driver);
> diff --git a/drivers/input/touchscreen/tsc2007_iio.c b/drivers/input/touchscreen/tsc2007_iio.c
> new file mode 100644
> index 0000000..e492bba
> --- /dev/null
> +++ b/drivers/input/touchscreen/tsc2007_iio.c
> @@ -0,0 +1,152 @@
> +/*
> + * drivers/input/touchscreen/tsc2007_iio.c
> + *
> + * Copyright (c) 2016 Golden Delicious Comp. GmbH&Co. KG
> + * Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
> + *
> + * 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.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include "tsc2007.h"
> +
> +struct tsc2007_iio {
> + struct tsc2007 *ts;
> +};
> +
> +#define TSC2007_CHAN_IIO(_chan, _name, _type, _chan_info) \
> +{ \
> + .datasheet_name = _name, \
> + .type = _type, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(_chan_info), \
> + .indexed = 1, \
> + .channel = _chan, \
> +}
> +
> +static const struct iio_chan_spec tsc2007_iio_channel[] = {
> + TSC2007_CHAN_IIO(0, "x", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
> + TSC2007_CHAN_IIO(1, "y", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
> + TSC2007_CHAN_IIO(2, "z1", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
> + TSC2007_CHAN_IIO(3, "z2", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
> + TSC2007_CHAN_IIO(4, "adc", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
> + TSC2007_CHAN_IIO(5, "rt", IIO_VOLTAGE, IIO_CHAN_INFO_RAW), /* Ohms? */
> + TSC2007_CHAN_IIO(6, "pen", IIO_PRESSURE, IIO_CHAN_INFO_RAW),
> + TSC2007_CHAN_IIO(7, "temp0", IIO_TEMP, IIO_CHAN_INFO_RAW),
> + TSC2007_CHAN_IIO(8, "temp1", IIO_TEMP, IIO_CHAN_INFO_RAW),
> +};
> +
> +static int tsc2007_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val, int *val2, long mask)
> +{
> + struct tsc2007_iio *iio = iio_priv(indio_dev);
> + struct tsc2007 *tsc = iio->ts;
> + int adc_chan = chan->channel;
> + int ret = 0;
> +
> + if (adc_chan >= ARRAY_SIZE(tsc2007_iio_channel))
> + return -EINVAL;
> +
> + if (mask != IIO_CHAN_INFO_RAW)
> + return -EINVAL;
> +
> + mutex_lock(&tsc->mlock);
> +
> + switch (chan->channel) {
> + case 0:
> + *val = tsc2007_xfer(tsc, READ_X);
> + break;
> + case 1:
> + *val = tsc2007_xfer(tsc, READ_Y);
> + break;
> + case 2:
> + *val = tsc2007_xfer(tsc, READ_Z1);
> + break;
> + case 3:
> + *val = tsc2007_xfer(tsc, READ_Z2);
> + break;
> + case 4:
> + *val = tsc2007_xfer(tsc, (ADC_ON_12BIT | TSC2007_MEASURE_AUX));
> + break;
> + case 5: {
> + struct ts_event tc;
> +
> + tc.x = tsc2007_xfer(tsc, READ_X);
> + tc.z1 = tsc2007_xfer(tsc, READ_Z1);
> + tc.z2 = tsc2007_xfer(tsc, READ_Z2);
> + *val = tsc2007_calculate_resistance(tsc, &tc);
> + break;
> + }
> + case 6:
> + *val = tsc2007_is_pen_down(tsc);
> + break;
> + case 7:
> + *val = tsc2007_xfer(tsc,
> + (ADC_ON_12BIT | TSC2007_MEASURE_TEMP0));
> + break;
> + case 8:
> + *val = tsc2007_xfer(tsc,
> + (ADC_ON_12BIT | TSC2007_MEASURE_TEMP1));
> + break;
> + }
> +
> + /* Prepare for next touch reading - power down ADC, enable PENIRQ */
> + tsc2007_xfer(tsc, PWRDOWN);
> +
> + mutex_unlock(&tsc->mlock);
> +
> + ret = IIO_VAL_INT;
> +
> + return ret;
> +}
> +
> +static const struct iio_info tsc2007_iio_info = {
> + .read_raw = tsc2007_read_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +int tsc2007_iio_configure(struct tsc2007 *ts)
> +{
> + int err;
> + struct iio_dev *indio_dev;
> + struct tsc2007_iio *iio;
> +
> + indio_dev = devm_iio_device_alloc(&ts->client->dev,
> + sizeof(struct tsc2007_iio));
> + if (!indio_dev) {
> + dev_err(&ts->client->dev, "iio_device_alloc failed\n");
> + return -ENOMEM;
> + }
> +
> + iio = iio_priv(indio_dev);
> + iio->ts = ts;
> + ts->iio_dev = (void *) indio_dev;
> +
> + indio_dev->name = "tsc2007";
> + indio_dev->dev.parent = &ts->client->dev;
> + indio_dev->info = &tsc2007_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = tsc2007_iio_channel;
> + indio_dev->num_channels = ARRAY_SIZE(tsc2007_iio_channel);
> +
> + err = iio_device_register(indio_dev);
> + if (err < 0) {
> + dev_err(&ts->client->dev, "iio_device_register() failed: %d\n",
> + err);
> + return err;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(tsc2007_iio_configure);
> +
> +void tsc2007_iio_unconfigure(struct tsc2007 *ts)
> +{
> + struct iio_dev *indio_dev = ts->iio_dev;
> +
> + iio_device_unregister(indio_dev);
> +}
> +EXPORT_SYMBOL(tsc2007_iio_unconfigure);
>
^ permalink raw reply
* Re: [PATCH 2/2] Documentation: DT: Add bmi160 imu binding
From: Jonathan Cameron @ 2016-11-12 13:15 UTC (permalink / raw)
To: Marcin Niestroj
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Daniel Baluta, Gregor Boirie, Sanchayan Maity, Rob Herring,
Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d07474fc-e6c9-d529-6a06-0549f3dfad52-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
On 09/11/16 15:18, Marcin Niestroj wrote:
> On 06.11.2016 13:41, Jonathan Cameron wrote:
>> On 03/11/16 11:25, Marcin Niestroj wrote:
>>> This adds documentation for Bosch BMI160 Inertial Measurement Unit
>>> device-tree bindings.
>>>
>>> Signed-off-by: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
>> Unless I missed it in the previous patch we should also have of tables
>> added to the i2c and spi files (which is why the various tests haven't
>> been screaming at me that this device doesn't have documented bindings).
>
> Ok, I will add them.
>
>>
>> Otherwise, the use of interrupt names to indicate which pin on the chip
>> is a little unusual (if you cribbed this from somewhere I've forgotten
>> about then do say so!), so will want a devicetree bindings maintainer
>> input on this.
>
> I have used interrupt names similar as in other driver. Please see
> [1] for it's DT documentation and [2] for implementation.
> [1] Documentation/devicetree/bindings/iio/accel/mma8452.txt
> [2] drivers/iio/accel/mma8452.c
Fair enough. Just put the INT1 option in the docs as requested and this
should be fine.
Jonathan
>
>>
>> Thanks,
>>
>> Jonathan
>>> ---
>>> .../devicetree/bindings/iio/imu/bmi160.txt | 34 ++++++++++++++++++++++
>>> 1 file changed, 34 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.txt b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
>>> new file mode 100644
>>> index 0000000..b02ef3e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
>>> @@ -0,0 +1,34 @@
>>> +Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
>>> +and externally connectable Magnetometer
>>> +
>>> +https://www.bosch-sensortec.com/bst/products/all_products/bmi160
>>> +
>>> +Required properties:
>>> + - compatible : should be "bosch,bmi160"
>>> + - reg : the I2C address or SPI chip select number of the sensor
>>> + - spi-max-frequency : set maximum clock frequency (only for SPI)
>>> +
>>> +Optional properties:
>>> + - interrupt-parent : should be the phandle of the interrupt controller
>>> + - interrupts : interrupt mapping for GPIO IRQ, must be IRQ_TYPE_LEVEL_LOW
>>> + - interrupt-names : set to "INT2" if using INT2 pin
>>> +
>>> +Examples:
>>> +
>>> +bmi160@68 {
>>> + compatible = "bosch,bmi160";
>>> + reg = <0x68>;
>>> +
>>> + interrupt-parent = <&gpio4>;
>>> + interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
>>> +};
>>> +
>>> +bmi160@0 {
>>> + compatible = "bosch,bmi160";
>>> + reg = <0>;
>>> + spi-max-frequency = <10000000>;
>>> +
>>> + interrupt-parent = <&gpio2>;
>>> + interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
>>> + interrupt-names = "INT2";
>>> +};
>>>
>>
>
^ permalink raw reply
* Re: [PATCH 1/2] iio: bmi160: Support hardware fifo
From: Jonathan Cameron @ 2016-11-12 13:13 UTC (permalink / raw)
To: Marcin Niestroj
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Daniel Baluta, Gregor Boirie, Sanchayan Maity, Rob Herring,
Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1d50ebf1-2f14-2e3f-9b8e-b34a7ebca4c5-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
On 09/11/16 14:52, Marcin Niestroj wrote:
> Hi,
> Thanks for review. I agree with all of the comments and will fix these in next patch version. Below is just a comment on hwfifo_* sysfs access.
>
> On 06.11.2016 13:35, Jonathan Cameron wrote:
>> On 03/11/16 11:25, Marcin Niestroj wrote:
>>> This patch was developed primarily based on bmc150_accel hardware fifo
>>> implementation.
>>>
>>> IRQ handler was added, which for now is responsible only for handling
>>> watermark interrupts. The BMI160 chip has two interrupt outputs. By
>>> default INT is considered to be connected. If INT2 is used instead, the
>>> interrupt-names device-tree property can be used to specify that.
>>>
>>> Signed-off-by: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
>> I agree with Peter that there should have been a few precursor patches
>> to this one doing various cleanups and reworking bits that you have
>> in here. Would have made it easier to review (always a good thing :)
>>
>> In general the resulting code looks good to me. A few little
>> additional comments inline from me. Mostly about small code ordering things
>> and function rename suggestions that would make the code more 'obviously'
>> correct.
>>
>> Thanks,
>>
>> Jonathan
>>> ---
>>> drivers/iio/imu/bmi160/bmi160.h | 3 +-
>>> drivers/iio/imu/bmi160/bmi160_core.c | 633 +++++++++++++++++++++++++++++++++--
>>> drivers/iio/imu/bmi160/bmi160_i2c.c | 7 +-
>>> drivers/iio/imu/bmi160/bmi160_spi.c | 3 +-
>>> 4 files changed, 618 insertions(+), 28 deletions(-)
>
> <snip>
>
>>> +
>>> +static IIO_CONST_ATTR(hwfifo_watermark_min, "1");
>>> +static IIO_CONST_ATTR(hwfifo_watermark_max,
>>> + __stringify(BMI160_FIFO_LENGTH));
>>> +static IIO_DEVICE_ATTR(hwfifo_enabled, S_IRUGO,
>>> + bmi160_get_fifo_state, NULL, 0);
>>> +static IIO_DEVICE_ATTR(hwfifo_watermark, S_IRUGO,
>>> + bmi160_get_fifo_watermark, NULL, 0);
>>> +
>>> +static const struct attribute *bmi160_fifo_attributes[] = {
>>> + &iio_const_attr_hwfifo_watermark_min.dev_attr.attr,
>>> + &iio_const_attr_hwfifo_watermark_max.dev_attr.attr,
>>> + &iio_dev_attr_hwfifo_watermark.dev_attr.attr,
>>> + &iio_dev_attr_hwfifo_enabled.dev_attr.attr,
>> There are enough of these drivers now that sometimes soon we should
>> revisit the question of pulling these into the core. Can certainly
>> concieve of downstream consumer devices (in particular the iio_input
>> bridge when that finally resurfaces - my fault) wanting to be able to
>> manipulate or at least have visibilty of these.
>
> One more thing to consider is setting hwfifo_watermark to other value
> than "userspace" watermark. It would be nice to set hwfifo_watermark
> to a *safe* value to be able to get all data from hardware to kfifo.
> By safe I mean that the chance of hardware fifo overflow will be
> small. On the other hand there might be no reason to have such small
> watermark for userspace application, so we can save scheduler
> cycles.
>
This may be worth doing, but I worry we'll end up with too many knobs
around this feature. Ultimately there is no point in setting the hardware
fifo as smaller than the userspace one if the userspace one is small enough.
What counts as a safe level is going to be very hardware / load dependant.
We 'could' try and put this in the devicetree so that a particular setup
could set it at a level that is 'reasonable'. It's kind of a hardware
feature, but will also depend on software constraints (like whether the
bus controller driver can support dma for example).
Non obvious unfortunately.
Jonathan
> <snip>
>
--
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 3/3] ARM64: dts: meson-gxbb: add the USB reset also to the second USB PHY
From: Martin Blumenstingl @ 2016-11-12 13:13 UTC (permalink / raw)
To: p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, kishon-l0cyMroinI0,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Martin Blumenstingl
In-Reply-To: <20161112131305.26088-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
When the USB PHY driver was introduced the reset framework did not
have support for triggering a reset pulse for shared resets. On GXBB
however there is only one reset line for both PHYs (meaning we have a
shared reset line). With the latest changes to the reset framework and
the corresponding updates to the phy-meson8b-usb2 driver we can now pass
the reset to the second PHY as well.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 719bf50..a548663 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -87,6 +87,7 @@
compatible = "amlogic,meson-gxbb-usb2-phy";
#phy-cells = <0>;
reg = <0x0 0xc0000020 0x0 0x20>;
+ resets = <&reset RESET_USB_OTG>;
clocks = <&clkc CLKID_USB>, <&clkc CLKID_USB1>;
clock-names = "usb_general", "usb";
status = "disabled";
--
2.10.2
--
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 2/3] phy: meson8b-usb2: request a shared reset line
From: Martin Blumenstingl @ 2016-11-12 13:13 UTC (permalink / raw)
To: p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, kishon-l0cyMroinI0,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Martin Blumenstingl
In-Reply-To: <20161112131305.26088-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Both PHYs are sharing one reset line. With recent improvements to the
reset framework we can now also use reset_control_reset with shared
resets.
This allows us to drop some workarounds where the reset was only
specified for one PHY but not the other, to make sure that the reset it
only executed once (as the reset framework was not able to use
reset_control_reset with shared reset lines).
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
drivers/phy/phy-meson8b-usb2.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/phy/phy-meson8b-usb2.c b/drivers/phy/phy-meson8b-usb2.c
index 73bf632..f1ee96a 100644
--- a/drivers/phy/phy-meson8b-usb2.c
+++ b/drivers/phy/phy-meson8b-usb2.c
@@ -237,8 +237,7 @@ static int phy_meson8b_usb2_probe(struct platform_device *pdev)
if (IS_ERR(priv->clk_usb))
return PTR_ERR(priv->clk_usb);
- priv->reset = devm_reset_control_get_optional_exclusive(&pdev->dev,
- NULL);
+ priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
if (PTR_ERR(priv->reset) == -EPROBE_DEFER)
return PTR_ERR(priv->reset);
--
2.10.2
--
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 1/3] reset: allow using reset_control_reset with shared reset
From: Martin Blumenstingl @ 2016-11-12 13:13 UTC (permalink / raw)
To: p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, kishon-l0cyMroinI0,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Martin Blumenstingl
In-Reply-To: <20161112131305.26088-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Some SoCs (for example Amlogic GXBB) implement a reset controller which
only supports a reset pulse (triggered via reset_control_reset). At the
same time multiple devices (in case of the Amlogic GXBB SoC both USB
PHYs) are sharing the same reset line.
This patch allows using reset_control_reset also for shared resets.
There are limitations though:
reset_control_reset can only be used if reset_control_assert was not
used yet.
reset_control_assert can only be used if reset_control_reset was not
used yet.
For shared resets the reset is only triggered once for the lifetime of
the reset_control instance (the reset can be triggered again if all
consumers of that specific reset_control are gone, as the reset
framework will free the reset_control instance in that case).
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
drivers/reset/core.c | 43 +++++++++++++++++++++++++++++++++++++------
1 file changed, 37 insertions(+), 6 deletions(-)
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index b8ae1db..10368ed 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -32,6 +32,9 @@ static LIST_HEAD(reset_controller_list);
* @refcnt: Number of gets of this reset_control
* @shared: Is this a shared (1), or an exclusive (0) reset_control?
* @deassert_cnt: Number of times this reset line has been deasserted
+ * @triggered_count: Number of times this reset line has been reset. Currently
+ * only used for shared resets, which means that the value
+ * will be either 0 or 1.
*/
struct reset_control {
struct reset_controller_dev *rcdev;
@@ -40,6 +43,7 @@ struct reset_control {
unsigned int refcnt;
int shared;
atomic_t deassert_count;
+ atomic_t triggered_count;
};
/**
@@ -134,18 +138,35 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register);
* reset_control_reset - reset the controlled device
* @rstc: reset controller
*
- * Calling this on a shared reset controller is an error.
+ * On a shared reset line the actual reset pulse is only triggered once for the
+ * lifetime of the reset_control instance: for all but the first caller this is
+ * a no-op.
+ * Consumers must not use reset_control_(de)assert on shared reset lines when
+ * reset_control_reset has been used.
*/
int reset_control_reset(struct reset_control *rstc)
{
- if (WARN_ON(IS_ERR_OR_NULL(rstc)) ||
- WARN_ON(rstc->shared))
+ int ret;
+
+ if (WARN_ON(IS_ERR_OR_NULL(rstc)))
return -EINVAL;
- if (rstc->rcdev->ops->reset)
- return rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
+ if (!rstc->rcdev->ops->reset)
+ return -ENOTSUPP;
- return -ENOTSUPP;
+ if (rstc->shared) {
+ if (WARN_ON(atomic_read(&rstc->deassert_count) != 0))
+ return -EINVAL;
+
+ if (atomic_inc_return(&rstc->triggered_count) != 1)
+ return 0;
+ }
+
+ ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
+ if (rstc->shared && !ret)
+ atomic_dec(&rstc->triggered_count);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(reset_control_reset);
@@ -159,6 +180,8 @@ EXPORT_SYMBOL_GPL(reset_control_reset);
*
* For shared reset controls a driver cannot expect the hw's registers and
* internal state to be reset, but must be prepared for this to happen.
+ * Consumers must not use reset_control_reset on shared reset lines when
+ * reset_control_(de)assert has been used.
*/
int reset_control_assert(struct reset_control *rstc)
{
@@ -169,6 +192,9 @@ int reset_control_assert(struct reset_control *rstc)
return -ENOTSUPP;
if (rstc->shared) {
+ if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
+ return -EINVAL;
+
if (WARN_ON(atomic_read(&rstc->deassert_count) == 0))
return -EINVAL;
@@ -185,6 +211,8 @@ EXPORT_SYMBOL_GPL(reset_control_assert);
* @rstc: reset controller
*
* After calling this function, the reset is guaranteed to be deasserted.
+ * Consumers must not use reset_control_reset on shared reset lines when
+ * reset_control_(de)assert has been used.
*/
int reset_control_deassert(struct reset_control *rstc)
{
@@ -195,6 +223,9 @@ int reset_control_deassert(struct reset_control *rstc)
return -ENOTSUPP;
if (rstc->shared) {
+ if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
+ return -EINVAL;
+
if (atomic_inc_return(&rstc->deassert_count) != 1)
return 0;
}
--
2.10.2
--
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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox