* Re: [PATCH 1/3] mtd: spi-nor: add optional DMA-safe bounce buffer for data transfer
From: Trent Piepho @ 2017-12-29 18:03 UTC (permalink / raw)
To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org,
broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
cyrille.pitchen-yU5RGvR974pGWvitb5QawA@public.gmane.org,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
vigneshr-l0cyMroinI0@public.gmane.org,
boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org,
richard-/L3Ra7n9ekc@public.gmane.org,
marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org,
radu.pirea-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org,
robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <08be8b42-732a-bf28-40c4-f46bf9d71c80-l0cyMroinI0@public.gmane.org>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2328 bytes --]
On Fri, 2017-12-29 at 15:46 +0530, Vignesh R wrote:
> On Friday 29 December 2017 12:24 AM, Trent Piepho wrote:
> >
> > > Vignesh has suggested to call virt_addr_valid() instead.
> > > I think Boris has also told me about this function.
> > > So it might be the right solution. What do you think about their proposal?
> >
> > Not sure what exactly the differences are between these methods. The
> > fact that each of the many existing DMA fixes uses slightly different
> > code to detect what is unsafe speaks to the difficulty of this problem!
>
> My understanding based on Documentation/DMA-API-HOWTO.txt and
> Documentation/arm/memory.txt is that
> virt_addr_valid() will guarantee that address is in range of
> PAGE_OFFSET to high_memory-1 (Kernel direct-mapped RAM region) which is
> address range of buffers that are DMA'able.
There's code in gpmi-nand.c that does:
/* first try to map the upper buffer directly */
if (virt_addr_valid(this->upper_buf) &&
!object_is_on_stack(this->upper_buf)) {
sg_init_one(sgl, this->upper_buf, this->upper_len);
So whoever wrote that thought that stack objects needed an additional
test beyond virt_addr_valid. But it does appear to be far more common
to depend on just virt_addr_valid, so perhaps the code in gpmi-nand is
in error.
> > virt_addr_valid() is already used by spi-ti-qspi. spi core uses for
> > the buffer map helper, but that code path is for buffers which are NOT
> > vmalloc or highmem, but are still not virt_addr_valid() for some other
> > reason.
> >
>
> if (vmalloced_buf || kmap_buf) {
> /* Handle vmalloc'd or kmap'd buffers */
> ...
This stuff does get DMAed. So I have to wonder, if spi.c thinks it can
use DMA with vmalloc or highmem, couldn't spi-not do the same instead
of the bounce buffer?
> } else if (virt_addr_valid(buf)) {
> /* Handle kmalloc'd and such buffers */
> ...
> } else {
> /* Error if none of the above */
So what is this case here for? It's some class that does not have a
valid virtual address and yet is not vmalloc or highmem.
> return -EINVAL;
> }
> N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±²˜¢žØ^n‡r¡ö¦zË\x1aëh™¨èÚ&¢îý»\x05ËÛÔØï¦v¬Îf\x1dp)¹¹br šê+€Ê+zf£¢·hšˆ§~††Ûiÿûàz¹\x1e®w¥¢¸?™¨èÚ&¢)ߢ^[f
^ permalink raw reply
* Re: [RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq
From: Tony Lindgren @ 2017-12-29 17:57 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel, bhelgaas, linux-pm, shawn.lin, briannorris, rjw,
dianders, devicetree, linux-pci, Rob Herring, Mark Rutland
In-Reply-To: <20171226020806.32710-2-jeffy.chen@rock-chips.com>
* Jeffy Chen <jeffy.chen@rock-chips.com> [171226 02:11]:
> We are going to handle PCIe WAKE# pin for PCI devices in the pci core,
> so add definitions of the optional PCIe WAKE# pin for PCI devices.
>
> Also add an definition of the optional PCI interrupt pin for PCI
> devices to distinguish it from the PCIe WAKE# pin.
> --- a/Documentation/devicetree/bindings/pci/pci.txt
> +++ b/Documentation/devicetree/bindings/pci/pci.txt
> @@ -24,3 +24,13 @@ driver implementation may support the following properties:
> unsupported link speed, for instance, trying to do training for
> unsupported link speed, etc. Must be '4' for gen4, '3' for gen3, '2'
> for gen2, and '1' for gen1. Any other values are invalid.
> +
> +PCI devices may support the following properties:
This should say PCI ports instead of PCI devices.
> +- interrupts: Interrupt specifier for each name in interrupt-names.
> +- interrupt-names:
> + May contain "wakeup" for PCIe WAKE# interrupt and "pci" for PCI interrupt.
> + The PCI devices may optionally include an 'interrupts' property that
> + represents the legacy PCI interrupt. And when we try to specify the PCIe
> + WAKE# pin, a corresponding 'interrupt-names' property is required to
> + distinguish them.
> --
> 2.11.0
>
>
^ permalink raw reply
* Re: [RFC PATCH v12 5/5] arm64: dts: rockchip: Move PCIe WAKE# irq to pcie port for Gru
From: Tony Lindgren @ 2017-12-29 17:55 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
shawn.lin-TNX95d0MmH7DzftRWevZcw,
briannorris-F7+t8E8rja9g9hUCZPvPmw, rjw-LthD3rsA81gm4RdzfppkhA,
dianders-F7+t8E8rja9g9hUCZPvPmw, Matthias Kaehlcke,
Heiko Stuebner, devicetree-u79uwXL29TY76Z2rM5mHXA, Klaus Goger,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Will Deacon,
Mark Rutland, Catalin Marinas
In-Reply-To: <20171226023646.17722-6-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
* Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> [171226 02:41]:
> Currently we are handling PCIe WAKE# irq in mrvl wifi driver.
>
> Move it to rockchip pcie port since we are going to handle it in the
> pci core.
Yes in the PCIe case, the pcie port node is the right place for
the wakeirq instead of the child the mvl_wifi node. So one
question further down below to verify this..
> Also avoid this irq been considered as the PCI interrupt pin in the
> of_irq_parse_pci().
The above paragraph needs a bit more clarification to be
readable :)
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> @@ -719,15 +719,16 @@ ap_i2c_audio: &i2c8 {
> #size-cells = <2>;
> ranges;
>
> + interrupts-extended = <&pcie0 1>, <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
> + interrupt-names = "pci", "wakeup";
> + pinctrl-names = "default";
> + pinctrl-0 = <&wlan_host_wake_l>;
> + wakeup-source;
> +
> mvl_wifi: wifi@0,0 {
> compatible = "pci1b4b,2b42";
> reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
> 0x83010000 0x0 0x00100000 0x0 0x00100000>;
> - interrupt-parent = <&gpio0>;
> - interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
> - pinctrl-names = "default";
> - pinctrl-0 = <&wlan_host_wake_l>;
> - wakeup-source;
> };
> };
> };
So the above modifies pcie@0,0 node. And that node describes
the particular PCIe port that the WLAN is connected to instead
of describing the whole PCIe controller device, right?
If so, then yeah it's totally where the wakeirq should be
defined for a PCIe device in the dts file :)
Regards,
Tony
--
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 03/14] dt-bindings: iio: add binding support for iio trigger provider/consumer
From: Jonathan Cameron @ 2017-12-29 17:24 UTC (permalink / raw)
To: Rob Herring
Cc: Eugen Hristev, nicolas.ferre, ludovic.desroches,
alexandre.belloni, linux-iio, linux-arm-kernel, devicetree,
linux-kernel, linux-input, dmitry.torokhov
In-Reply-To: <20171226223500.dxbp26bsx2ojbicr@rob-hp-laptop>
On Tue, 26 Dec 2017 16:35:00 -0600
Rob Herring <robh@kernel.org> wrote:
> On Fri, Dec 22, 2017 at 05:07:10PM +0200, Eugen Hristev wrote:
> > Add bindings for producer/consumer for iio triggers.
> >
> > Similar with iio channels, the iio triggers can be connected between drivers:
> > one driver will be a producer by registering iio triggers, and another driver
> > will connect as a consumer.
> >
> > Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
In cases where the connectivity is entirely known to the various drivers
(battery chargers integrated in SoCs that use ADC channels for example) we
have always kept the map in driver.
I'm not yet entirely clear if we can do this here. Might make sense if
we can...
> > ---
> > .../devicetree/bindings/iio/iio-bindings.txt | 52 +++++++++++++++++++++-
> > 1 file changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > index 68d6f8c..d861f0df 100644
> > --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > @@ -11,6 +11,10 @@ value of a #io-channel-cells property in the IIO provider node.
> >
> > [1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
> >
> > +Moreover, the provider can have a set of triggers that can be attached to
> > +from the consumer drivers.
> > +
> > +
> > ==IIO providers==
> >
> > Required properties:
> > @@ -18,6 +22,11 @@ Required properties:
> > with a single IIO output and 1 for nodes with multiple
> > IIO outputs.
> >
> > +Optional properties:
> > +#io-trigger-cells: Number of cells for the IIO trigger specifier. Typically 0
> > + for nodes with a single IIO trigger and 1 for nodes with
> > + multiple IIO triggers.
> > +
> > Example for a simple configuration with no trigger:
> >
> > adc: voltage-sensor@35 {
> > @@ -26,7 +35,7 @@ Example for a simple configuration with no trigger:
> > #io-channel-cells = <1>;
> > };
> >
> > -Example for a configuration with trigger:
> > +Example for a configuration with channels provided by trigger:
> >
> > adc@35 {
> > compatible = "some-vendor,some-adc";
> > @@ -42,6 +51,17 @@ Example for a configuration with trigger:
> > };
> > };
> >
> > +Example for a configuration for a trigger provider:
> > +
> > + adc: sensor-with-trigger@35 {
> > + compatible = "some-vendor,some-adc";
> > + reg = <0x35>;
> > + #io-channel-cells = <1>;
> > + #io-trigger-cells = <1>;
> > + /* other properties */
> > + };
> > +
> > +
> > ==IIO consumers==
> >
> > Required properties:
> > @@ -61,16 +81,38 @@ io-channel-ranges:
> > IIO channels from this node. Useful for bus nodes to provide
> > and IIO channel to their children.
> >
> > +io-triggers: List of phandle and IIO specifier pairs, one pair
> > + for each trigger input to the device. Note: if the
> > + IIO trigger provider specifies '0' for #io-trigger-cells,
> > + then only the phandle portion of the pair will appear.
> > +
> > +io-trigger-names:
> > + List of IIO trigger input name strings sorted in the same
> > + order as the io-triggers property. Consumers drivers
> > + will use io-trigger-names to match IIO trigger input names
> > + with IIO specifiers.
> > +
> > +io-trigger-ranges:
> > + Empty property indicating that child nodes can inherit named
> > + IIO triggers from this node. Useful for bus nodes to provide
> > + IIO triggers to their children.
>
> I think it would be better to be explicit in the child nodes. What's the
> use you had in mind?
>
> Rob
> --
> 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 07/14] iio: triggers: on pollfunc attach, complete iio_dev if NULL
From: Jonathan Cameron @ 2017-12-29 17:23 UTC (permalink / raw)
To: Eugen Hristev
Cc: nicolas.ferre, ludovic.desroches, alexandre.belloni, linux-iio,
linux-arm-kernel, devicetree, linux-kernel, linux-input,
dmitry.torokhov
In-Reply-To: <1513955241-10985-8-git-send-email-eugen.hristev@microchip.com>
On Fri, 22 Dec 2017 17:07:14 +0200
Eugen Hristev <eugen.hristev@microchip.com> wrote:
> When attaching a pollfunc to a trigger, if the pollfunc does not
> have an associated iio_dev pointer, just use the private data
> iio_dev pointer from the trigger to fill in the poll func required
> iio_dev reference.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
I'm yet to be convinced this is necessary rather than using a callback
buffer. It's also decidedly unsafe as there is no particular reason
in general to assume the private data is an iio_dev.
> ---
> drivers/iio/industrialio-trigger.c | 9 +++++++++
> include/linux/iio/trigger_consumer.h | 2 ++
> 2 files changed, 11 insertions(+)
>
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 8565c92..ab180bd 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -272,6 +272,15 @@ int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> bool notinuse
> = bitmap_empty(trig->pool, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
>
> + /*
> + * If we did not get a iio_dev in the poll func, attempt to
> + * obtain the trigger's owner's device struct
> + */
> + if (!pf->indio_dev)
> + pf->indio_dev = iio_trigger_get_drvdata(trig);
That isn't always valid. Triggers don't always have a iio_dev associated
with them at all.
> + if (!pf->indio_dev)
> + return -EINVAL;
> +
> /* Prevent the module from being removed whilst attached to a trigger */
> __module_get(pf->indio_dev->driver_module);
>
> diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
> index aeefcdb..36e2a02 100644
> --- a/include/linux/iio/trigger_consumer.h
> +++ b/include/linux/iio/trigger_consumer.h
> @@ -63,6 +63,8 @@ int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
> /*
> * Two functions for the uncommon case when we need to attach or detach
> * a specific pollfunc to and from a trigger
> + * If the pollfunc has a NULL iio_dev pointer, it will be filled from the
> + * trigger struct.
> */
> int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> struct iio_poll_func *pf);
^ permalink raw reply
* Re: [PATCH 09/14] iio: inkern: triggers: create helpers for OF trigger retrieval
From: Jonathan Cameron @ 2017-12-29 17:20 UTC (permalink / raw)
To: Eugen Hristev
Cc: nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA,
alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1513955241-10985-10-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
On Fri, 22 Dec 2017 17:07:16 +0200
Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org> wrote:
> Create helper API to get trigger information from OF regarding
> trigger producer/consumer for iio triggers.
> The functions will search for matching trigger by name, similar
> with channel retrieval
>
> Signed-off-by: Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
This makes sense once we make the touchscreen driver truely generic.
If it wasn't generic this could be rolled up as data within that driver.
A few small comments inline.
> ---
> drivers/iio/inkern.c | 91 ++++++++++++++++++++++++++++++++++++
> include/linux/iio/trigger_consumer.h | 1 +
> 2 files changed, 92 insertions(+)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 069defc..58bd18d 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -14,9 +14,13 @@
>
> #include <linux/iio/iio.h>
> #include "iio_core.h"
> +#include "iio_core_trigger.h"
> #include <linux/iio/machine.h>
> #include <linux/iio/driver.h>
> #include <linux/iio/consumer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +
Don't add white space here.
>
> struct iio_map_internal {
> struct iio_dev *indio_dev;
> @@ -262,6 +266,39 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
> return ERR_PTR(ret);
> }
>
> +#ifdef CONFIG_IIO_TRIGGER
Hmm. Generally frowned upon to have ifdef blocks inline within c files.
Perhaps worth pulling these out to inkern-trigger.c and using Kconfig magic
to build if relevant - not worth splitting the header though.
> +
> +static struct iio_trigger *of_iio_trigger_get(struct device_node *np, int index)
> +{
> + struct device *idev;
> + struct iio_dev *indio_dev;
> + int err;
> + struct of_phandle_args iiospec;
> + struct iio_trigger *trig;
> +
> + err = of_parse_phandle_with_args(np, "io-triggers",
> + "#io-trigger-cells",
> + index, &iiospec);
> + if (err)
> + return ERR_PTR(err);
> +
> + idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
> + iio_dev_node_match);
> + of_node_put(iiospec.np);
> + if (!idev)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> + indio_dev = dev_to_iio_dev(idev);
> +
> + trig = iio_trigger_find_from_device(indio_dev, iiospec.args[0]);
> +
> + if (!trig)
> + return ERR_PTR(-ENODEV);
> +
> + return trig;
> +}
> +#endif /* CONFIG_IIO_TRIGGER */
> +
> #else /* CONFIG_OF */
>
> static inline struct iio_channel *
> @@ -275,6 +312,12 @@ static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
> return NULL;
> }
>
> +static inline struct iio_trigger *of_iio_trigger_get(struct device_node *np,
> + int index)
> +{
> + return NULL;
> +}
> +
> #endif /* CONFIG_OF */
>
> static struct iio_channel *iio_channel_get_sys(const char *name,
> @@ -927,3 +970,51 @@ ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
> chan->channel, buf, len);
> }
> EXPORT_SYMBOL_GPL(iio_write_channel_ext_info);
> +
> +#ifdef CONFIG_IIO_TRIGGER
> +struct iio_trigger *iio_trigger_find(struct device *dev, char *name)
> +{
> + struct device_node *np = dev->of_node;
> + struct iio_trigger *trig = NULL;
> +
> + /* Walk up the tree of devices looking for a matching iio trigger */
> + while (np) {
> + int index = 0;
> +
> + /*
> + * For named iio triggers, first look up the name in the
> + * "io-trigger-names" property. If it cannot be found, the
> + * index will be an error code, and of_iio_trigger_get()
> + * will fail.
> + */
> + if (name)
> + index = of_property_match_string(np, "io-trigger-names",
> + name);
> + trig = of_iio_trigger_get(np, index);
> + if (!IS_ERR(trig) || PTR_ERR(trig) == -EPROBE_DEFER) {
> + break;
> + } else if (name && index >= 0) {
> + pr_err("ERROR: could not get IIO trigger %pOF:%s(%i)\n",
> + np, name ? name : "", index);
> + return trig;
> + }
> +
> + /*
> + * No matching IIO trigger found on this node.
> + * If the parent node has a "io-trigger-ranges" property,
> + * then we can try one of its channels.
> + */
> + np = np->parent;
> + if (np && !of_get_property(np, "io-trigger-ranges", NULL))
> + return trig;
> + }
> +
> + if (!trig || (IS_ERR(trig) && PTR_ERR(trig) != -EPROBE_DEFER))
> + dev_dbg(dev, "error retrieving trigger information\n");
> + else
> + dev_dbg(dev, "trigger found: %s\n", name);
> +
> + return trig;
> +}
> +EXPORT_SYMBOL_GPL(iio_trigger_find);
> +#endif /* CONFIG_IIO_TRIGGER */
> diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
> index 13be595..b2fc485 100644
> --- a/include/linux/iio/trigger_consumer.h
> +++ b/include/linux/iio/trigger_consumer.h
> @@ -62,6 +62,7 @@ irqreturn_t iio_pollfunc_store_time(int irq, void *p);
>
> void iio_trigger_notify_done(struct iio_trigger *trig);
>
> +struct iio_trigger *iio_trigger_find(struct device *dev, char *name);
> /*
> * Two functions for common case where all that happens is a pollfunc
> * is attached and detached from a trigger
--
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: [RFC PATCH v11 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF
From: Tony Lindgren @ 2017-12-29 17:16 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: JeffyChen, Linux Kernel Mailing List, Bjorn Helgaas, Linux PM,
Shawn Lin, Brian Norris, Doug Anderson,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux PCI,
Rob Herring, Frank Rowand
In-Reply-To: <CAJZ5v0hSMqwitCvfi7D+sknuO0YFr5F-kdkV-cSoVp30Cmdaeg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
* Rafael J. Wysocki <rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [171228 17:46]:
>
> To be precise, it is not quite possible to figure out which WAKE#
> triggered, if they are sharing the line, without looking into the
> config spaces of the devices below the switch. The switch is not
> expected to do that AFAICS. It only generates a PME message meaning
> "wakeup is being signaled somewhere below" and the PME driver that
> handles the Root Port receiving it should look at the PME Status bits
> of the devices below the switch (the pme.c driver does that IIRC or at
> least it should do that ;-)).
>
> Still, the handling of WAKE# doesn't need to cover this case AFAICS.
OK makes sense now.
Regards,
Tony
--
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 13/14] input: touchscreen: sama5d2_rts: SAMA5D2 Resistive touchscreen driver
From: Jonathan Cameron @ 2017-12-29 17:16 UTC (permalink / raw)
To: Eugen Hristev
Cc: nicolas.ferre, ludovic.desroches, alexandre.belloni, linux-iio,
linux-arm-kernel, devicetree, linux-kernel, linux-input,
dmitry.torokhov
In-Reply-To: <1513955241-10985-14-git-send-email-eugen.hristev@microchip.com>
On Fri, 22 Dec 2017 17:07:20 +0200
Eugen Hristev <eugen.hristev@microchip.com> wrote:
> This is the implementation of the Microchip SAMA5D2 SOC resistive
> touchscreen driver.
> The driver registers an input device and connects to the give IIO device
> from devicetree. It requires an IIO trigger (acting as a consumer) and
> three IIO channels : one for X position, one for Y position and one
> for pressure.
> It the reports the values to the input subsystem.
>
> Some parts of this driver are based on the initial original work by
> Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
I've suggested some difference in implementation, but this is very nearly
a generic resistive touch screen driver which is rather nice.
It does use somewhat magic trigger which is effectively a filtered
periodic trigger (only fires when touch has occurred) which would not
be particularly hard to implement in other resistive touch screen ADCs.
I'm not totally sure that is a strong requirement though - any periodic
trigger would work.
Anyhow, the big stuff to my mind is whether we can use the buffer_cb
code to do this. I originally wrote that for an accelerometer to input
bridge driver (which I never got around to finishing upstreaming). It's
existing usecases are rather esoteric but it should work here I think.
Jonathan
> ---
> drivers/input/touchscreen/Kconfig | 13 ++
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/sama5d2_rts.c | 287 ++++++++++++++++++++++++++++++++
> 3 files changed, 301 insertions(+)
> create mode 100644 drivers/input/touchscreen/sama5d2_rts.c
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 64b30fe..db8f541 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -126,6 +126,19 @@ config TOUCHSCREEN_ATMEL_MXT_T37
> Say Y here if you want support to output data from the T37
> Diagnostic Data object using a V4L device.
>
> +config TOUCHSCREEN_SAMA5D2
> + tristate "Microchip SAMA5D2 resistive touchscreen support"
> + depends on ARCH_AT91
> + depends on AT91_SAMA5D2_ADC
> + help
> + Say Y here if you have 4-wire touchscreen connected
> + to ADC Controller on your SAMA5D2 Microchip SoC.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called sama5d2_rts.
> +
> config TOUCHSCREEN_AUO_PIXCIR
> tristate "AUO in-cell touchscreen using Pixcir ICs"
> depends on I2C
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 850c156..9a2772e 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_TOUCHSCREEN_AD7879_SPI) += ad7879-spi.o
> obj-$(CONFIG_TOUCHSCREEN_ADS7846) += ads7846.o
> obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
> obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
> +obj-$(CONFIG_TOUCHSCREEN_SAMA5D2) += sama5d2_rts.o
> obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
> obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
> obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o
> diff --git a/drivers/input/touchscreen/sama5d2_rts.c b/drivers/input/touchscreen/sama5d2_rts.c
> new file mode 100644
> index 0000000..e2ae413
> --- /dev/null
> +++ b/drivers/input/touchscreen/sama5d2_rts.c
> @@ -0,0 +1,287 @@
> +/*
> + * Microchip resistive touchscreen (RTS) driver for SAMA5D2.
> + *
> + * Copyright (C) 2017 Microchip Technology,
> + * Author: Eugen Hristev <eugen.hristev@microchip.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +#include <linux/input.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +#define DRIVER_NAME "sama5d2_rts"
> +#define MAX_POS_MASK GENMASK(11, 0)
> +#define AT91_RTS_DEFAULT_PRESSURE_THRESHOLD 10000
> +
> +/**
> + * at91_rts - at91 resistive touchscreen information struct
> + * @input: the input device structure that we register
> + * @chan_x: X channel to IIO device to get position on X axis
> + * @chan_y: Y channel to IIO device to get position on Y axis
> + * @chan_pressure: pressure channel to IIO device to get pressure
> + * @trig: trigger to IIO device to register to for polling
> + * @rts_pf: pollfunc for the trigger to be called by IIO dev
> + * @pressure_threshold: number representing the threshold for the pressure
> + * @adc_connected: to know if adc device is connected
> + * @workq: to defer computations to this work queue for reporting
> + */
> +struct at91_rts {
> + struct input_dev *input;
> + struct iio_channel *chan_x, *chan_y, *chan_pressure;
> + struct iio_trigger *trig;
> + struct iio_poll_func *rts_pf;
> + u32 pressure_threshold;
> + bool adc_connected;
> + struct work_struct workq;
> +};
> +
> +static irqreturn_t at91_rts_trigger_handler(int irq, void *p)
> +{
> + struct at91_rts *st = ((struct iio_poll_func *)p)->p;
> +
> + schedule_work(&st->workq);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void at91_rts_workq_handler(struct work_struct *workq)
> +{
> + struct at91_rts *st = container_of(workq, struct at91_rts, workq);
> + unsigned int x, y, press;
> + int ret;
> +
If we were instead to use a callback buffer this would all be
in the ADC driver as a simple 3 element channel scan.
> + /* read the channels, if all good, report touch */
> + ret = iio_read_channel_raw(st->chan_x, &x);
> + if (ret < 0)
> + goto at91_rts_workq_handler_end_touch;
> +
> + ret = iio_read_channel_raw(st->chan_y, &y);
> + if (ret < 0)
> + goto at91_rts_workq_handler_end_touch;
> +
> + ret = iio_read_channel_raw(st->chan_pressure, &press);
> + if (ret < 0)
> + goto at91_rts_workq_handler_end_touch;
> +
At this point our callback would be called and provided the above data.
> + /* if pressure too low, don't report */
> + if (press > st->pressure_threshold)
> + goto at91_rts_workq_handler_exit;
> +
> + input_report_abs(st->input, ABS_X, x);
> + input_report_abs(st->input, ABS_Y, y);
> + input_report_abs(st->input, ABS_PRESSURE, press);
> + input_report_key(st->input, BTN_TOUCH, 1);
> + input_sync(st->input);
> +
This would be back in the callback buffer code once the
callback has run.
Has the interesting side effect of making this code effectively generic.
It no longer has any ties to the at91 at all that I can spot.
Just a selection of channels and a request for a particular trigger -
both of which could come from device tree.
> + iio_trigger_notify_done(st->trig);
> + return;
> +
> +at91_rts_workq_handler_end_touch:
> + /* report end of touch */
> + input_report_key(st->input, BTN_TOUCH, 0);
> + input_sync(st->input);
> +at91_rts_workq_handler_exit:
> + iio_trigger_notify_done(st->trig);
> +}
> +
> +static int at91_rts_open(struct input_dev *dev)
> +{
> + int ret;
> + struct at91_rts *st = input_get_drvdata(dev);
> +
> + /* avoid multiple initialization in case touchscreen is opened again */
> + if (st->adc_connected)
> + return 0;
> +
> + /*
> + * First, look for the channels. It is possible that the ADC device
> + * did not probe yet, but we already probed, so we returning probe defer
> + * doesn't make much sense.
Quite - so why not do the obvious and request these in the probe and hold
them until remove? Then if they aren't ready defer the probe. This driver
is useless until the ADC is there so why let it successfully probe before
that point?
> + */
> + st->chan_x = iio_channel_get(dev->dev.parent, "x");
> + if (IS_ERR_OR_NULL(st->chan_x)) {
> + dev_err(dev->dev.parent, "cannot get X channel from ADC");
> + ret = PTR_ERR(st->chan_x);
> + goto at91_rts_open_free_chan;
> + }
> +
> + st->chan_y = iio_channel_get(dev->dev.parent, "y");
> + if (IS_ERR_OR_NULL(st->chan_y)) {
> + dev_err(dev->dev.parent, "cannot get Y channel from ADC");
> + ret = PTR_ERR(st->chan_y);
> + goto at91_rts_open_free_chan;
> + }
> +
> + st->chan_pressure = iio_channel_get(dev->dev.parent, "pressure");
> + if (IS_ERR_OR_NULL(st->chan_pressure)) {
> + dev_err(dev->dev.parent, "cannot get pressure channel from ADC");
> + ret = PTR_ERR(st->chan_pressure);
> + goto at91_rts_open_free_chan;
> + }
> +
> + /* look for the trigger in device tree */
> + st->trig = iio_trigger_find(dev->dev.parent, NULL);
> + if (IS_ERR_OR_NULL(st->trig)) {
> + dev_err(dev->dev.parent, "cannot get trigger from ADC");
> + ret = PTR_ERR(st->trig)
This also feels like it should be retrieved during the probe and we should
defer if that fails. Can't do anything useful without it!
> + goto at91_rts_open_free_chan;
> + }
> +
> + /* allocate a pollfunc for the trigger */
> + st->rts_pf = iio_alloc_pollfunc(at91_rts_trigger_handler, NULL,
> + IRQF_ONESHOT, NULL,
> + dev->dev.parent->of_node->name);
> + if (!st->rts_pf) {
> + ret = -ENOMEM;
> + dev_err(dev->dev.parent, "cannot allocate trigger pollfunc");
> + goto at91_rts_open_free_chan;
> + }
> +
> + iio_pollfunc_set_private_data(st->rts_pf, st);
> +
> + /*
> + * Attach the pollfunc to the trigger. This will also call the
> + * configure function to enable the trigger
> + */
> + ret = iio_trigger_attach_poll_func(st->trig, st->rts_pf);
> + if (ret)
> + goto at91_rts_open_dealloc_pf;
Ah. Now I think I see why you needed the poll function rather than
doing this with a callback buffer.
I'd rather see a consumer interface that requests the whole ADC runs on
a particular trigger.
> +
> + dev_dbg(dev->dev.parent, "channels found, attached to trigger");
> +
> + st->adc_connected = true;
> + return 0;
> +
> +at91_rts_open_dealloc_pf:
> + iio_dealloc_pollfunc(st->rts_pf);
> +at91_rts_open_free_chan:
> + if (!IS_ERR_OR_NULL(st->chan_x))
> + iio_channel_release(st->chan_x);
> + if (!IS_ERR_OR_NULL(st->chan_y))
> + iio_channel_release(st->chan_y);
> + if (!IS_ERR_OR_NULL(st->chan_pressure))
> + iio_channel_release(st->chan_pressure);
> + /*
> + * Avoid keeping old values in channel pointers. in case some channel
> + * failed and we reopen them, and now fail, we will have invalid values
> + * to release. So write them as NULL now.
> + */
> + st->chan_x = NULL;
> + st->chan_y = NULL;
> + st->chan_pressure = NULL;
> + return ret;
> +}
> +
> +static void at91_rts_close(struct input_dev *dev)
> +{
> + struct at91_rts *st = input_get_drvdata(dev);
> +
> + if (!st->adc_connected)
> + return;
> +
> + iio_trigger_detach_poll_func(st->trig, st->rts_pf);
> + iio_dealloc_pollfunc(st->rts_pf);
> +
> + if (!IS_ERR_OR_NULL(st->chan_x))
> + iio_channel_release(st->chan_x);
> + if (!IS_ERR_OR_NULL(st->chan_y))
> + iio_channel_release(st->chan_y);
> + if (!IS_ERR_OR_NULL(st->chan_pressure))
> + iio_channel_release(st->chan_pressure);
> +
> + st->adc_connected = false;
> +}
> +
> +static int at91_rts_probe(struct platform_device *pdev)
> +{
> + int ret;
> + struct at91_rts *st;
> + struct input_dev *input;
> + struct device *dev = &pdev->dev;
> + struct device_node *node = dev->of_node;
> +
> + st = devm_kzalloc(dev, sizeof(struct at91_rts), GFP_KERNEL);
> + if (!st)
> + return -ENOMEM;
> + st->adc_connected = false;
> +
> + INIT_WORK(&st->workq, at91_rts_workq_handler);
> +
> + input = devm_input_allocate_device(dev);
> + if (!input) {
> + dev_err(dev, "failed to allocate input device\n");
> + return -ENOMEM;
> + }
> +
> + ret = of_property_read_u32(node, "microchip,pressure-threshold",
> + &st->pressure_threshold);
> + if (ret < 0) {
> + dev_dbg(dev, "can't get touchscreen pressure threshold property.\n");
> + st->pressure_threshold = AT91_RTS_DEFAULT_PRESSURE_THRESHOLD;
> + }
> +
> + input->name = DRIVER_NAME;
> + input->id.bustype = BUS_HOST;
> + input->dev.parent = &pdev->dev;
> + input->open = at91_rts_open;
> + input->close = at91_rts_close;
> +
> + input_set_abs_params(input, ABS_X, 0, MAX_POS_MASK - 1, 0, 0);
> + input_set_abs_params(input, ABS_Y, 0, MAX_POS_MASK, 0, 0);
> + input_set_abs_params(input, ABS_PRESSURE, 0, 0xffffff, 0, 0);
> +
> + input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> + input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> +
> + st->input = input;
> + input_set_drvdata(input, st);
> +
> + ret = input_register_device(input);
> + if (ret) {
> + dev_err(dev, "failed to register input device: %d", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, st);
> +
> + dev_info(dev, "probed successfully\n");
Not useful. There are many ways to find this out without looking at dmesg.
Please remove.
> + return 0;
> +}
> +
> +static int at91_rts_remove(struct platform_device *pdev)
> +{
> + struct at91_rts *st = platform_get_drvdata(pdev);
> +
> + input_unregister_device(st->input);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id at91_rts_of_match[] = {
> + {
> + .compatible = "microchip,sama5d2-resistive-touch",
> + }, {
> + /* sentinel */
> + },
> +};
> +MODULE_DEVICE_TABLE(of, at91_rts_of_match);
> +
> +static struct platform_driver atmel_rts_driver = {
> + .probe = at91_rts_probe,
> + .remove = at91_rts_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = of_match_ptr(at91_rts_of_match),
> + },
> +};
> +
> +module_platform_driver(atmel_rts_driver);
> +
> +MODULE_AUTHOR("Eugen Hristev <eugen.hristev@microchip.com>");
> +MODULE_DESCRIPTION("Microchip SAMA5D2 Resistive Touch Driver");
> +MODULE_LICENSE("GPL v2");
^ permalink raw reply
* Re: [RFC PATCH v11 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF
From: Tony Lindgren @ 2017-12-29 17:15 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, JeffyChen, Linux Kernel Mailing List,
Bjorn Helgaas, Linux PM, Shawn Lin, Brian Norris, Doug Anderson,
devicetree@vger.kernel.org, Linux PCI, Rob Herring, Frank Rowand
In-Reply-To: <6120485.xubBpvge6h@aspire.rjw.lan>
* Rafael J. Wysocki <rjw@rjwysocki.net> [171228 17:33]:
> On Thursday, December 28, 2017 5:51:34 PM CET Tony Lindgren wrote:
> >
> > Well Brian had a concern where we would have to implement PM runtime
> > for all device drivers for PCI devices.
>
> Why would we?
Seems at least I was a bit confused. In the PCIe case the WAKE# is
owned by the PCIe slot, not the child PCIe device. So you're right,
there should be no need for the child PCIe device drivers to
implement runtime PM.
I was thinking the wakeirq case with WLAN on SDIO bus. Some WLAN
devices can have a hardwired OOB wakeirq wired to a GPIO controller.
In that case the wakeirq is owned by the child device driver
(WLAN controller) and not by the SDIO slot. I was earlier
thinking this is the same as the "Figure 5-4" case 1, but it's
not.
So in the PCIe WAKE# case for device tree, we must have the
wakeirq property for the PCIe slot for the struct device managing
that slot, and not for the child device driver. I think it's
already this way in the most recent set of patches, I need to
look again.
> > So isn't my option 1 above similar to the PCIe spec "Figure 5-4"
> > case 2?
>
> No, it isn't, because in that case there is no practical difference
> between WAKE# and an in-band PME message sent by the device (Beacon)
> from the software perspective.
>
> WAKE# causes the switch to send a PME message upstream and that is
> handled by the Root Complex through the standard mechanism already
> supported by our existing PME driver (drivers/pci/pcie/pme.c).
OK. So if "Figure 5-4" case 2 is already handled then and we need
to just deal with "Figure 5-4" case 1 :)
> > Yeah. FYI, for the dedicated wakeirq cases I have, we need to keep
> > them masked during runtime to avoid tons of interrupts as they
> > are often wired to the RX pins.
>
> OK
>
> BTW, enable_irq_wake() should take care of the sharing, shouldn't it?
That can be used to tell us which device has wakeirq enabled for
wake-up events, but only for resume not runtiem PM. We still have the
shared IRQ problem to deal with. And the PCIe subsystem still needs
to go through the child devices.
> But the WAKE# thing is not just for waking up the system from sleep states,
> it is for runtime PM's wakeup signaling too.
Yes my test cases have it working for runtime PM and for waking
up system from suspend.
> > > > Currently nothing happens with wakeirqs if there's no struct
> > > > wakeup_source. On device_wakeup_enable() we call device_wakeup_attach()
> > > > that just copies dev->power.wakeirq to ws->wakeirq. And when struct
> > > > wake_source is freed the device should be active and wakeirq
> > > > disabled. Or are you seeing other issues here?
> > >
> > > I'm suspicious about one thing, but I need to look deeper into the code. :-)
>
> So we are fine except for the race and we need the wakeirq field in wakeup
> sources to automatically arm the wakeup IRQs during suspend.
OK.
> If I'm not mistaken, we only need something like the patch below (untested).
Seems like it should fix the race, I'll do some testing next week.
Regards,
Tony
> ---
> drivers/base/power/wakeirq.c | 9 ++++-----
> drivers/base/power/wakeup.c | 2 +-
> 2 files changed, 5 insertions(+), 6 deletions(-)
>
> Index: linux-pm/drivers/base/power/wakeirq.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/wakeirq.c
> +++ linux-pm/drivers/base/power/wakeirq.c
> @@ -33,7 +33,6 @@ static int dev_pm_attach_wake_irq(struct
> struct wake_irq *wirq)
> {
> unsigned long flags;
> - int err;
>
> if (!dev || !wirq)
> return -EINVAL;
> @@ -45,12 +44,12 @@ static int dev_pm_attach_wake_irq(struct
> return -EEXIST;
> }
>
> - err = device_wakeup_attach_irq(dev, wirq);
> - if (!err)
> - dev->power.wakeirq = wirq;
> + dev->power.wakeirq = wirq;
> + if (dev->power.wakeup)
> + device_wakeup_attach_irq(dev, wirq);
>
> spin_unlock_irqrestore(&dev->power.lock, flags);
> - return err;
> + return 0;
> }
>
> /**
> Index: linux-pm/drivers/base/power/wakeup.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/wakeup.c
> +++ linux-pm/drivers/base/power/wakeup.c
> @@ -303,7 +303,7 @@ int device_wakeup_attach_irq(struct devi
> }
>
> if (ws->wakeirq)
> - return -EEXIST;
> + dev_err(dev, "Leftover wakeup IRQ found, overriding\n");
>
> ws->wakeirq = wakeirq;
> return 0;
>
^ permalink raw reply
* Re: [PATCH 12/14] iio: adc: at91-sama5d2_adc: support for position and pressure channels
From: Jonathan Cameron @ 2017-12-29 17:02 UTC (permalink / raw)
To: Eugen Hristev
Cc: nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA,
alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1513955241-10985-13-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
On Fri, 22 Dec 2017 17:07:19 +0200
Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org> wrote:
> The ADC IP supports position and pressure measurements for a touchpad
> connected on channels 0,1,2,3 for a 4-wire touchscreen with pressure
> measurement support.
> Using the inkern API, a driver can request a trigger and read the
> channel values from the ADC.
> The implementation provides a trigger named "touch" which can be
> connected to a consumer driver.
> Once a driver connects and attaches a pollfunc to this trigger, the
> configure trigger callback is called, and then the ADC driver will
> initialize pad measurement.
> First step is to enable touchscreen 4wire support and enable
> pen detect IRQ.
> Once a pen is detected, a periodic trigger is setup to trigger every
> 2 ms (e.g.) and sample the resistive touchscreen values. The trigger poll
> is called, and the consumer driver is then woke up, and it can read the
> respective channels for the values : X, and Y for position and pressure
> channel.
> Because only one trigger can be active in hardware in the same time,
> while touching the pad, the ADC will block any attempt to use the
> triggered buffer. Same, conversions using the software trigger are also
> impossible (since the periodic trigger is setup).
> If some driver wants to attach while the trigger is in use, it will
> also fail.
> Once the pen is not detected anymore, the trigger is free for use (hardware
> or software trigger, with or without DMA).
> Channels 0,1,2 and 3 are unavailable if a touchscreen is enabled.
>
> Some parts of this patch are based on initial original work by
> Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
>
OK, so comments inline.
What I'm missing currently though is an explanation of why the slightly
more standard arrangement of using a callback buffer doesn't work here.
The only addition I think you need to do that is to allow a consumer to
request a particular trigger. I also think some of the other provisions
could be handled using standard features and slightly reducing the flexibility.
I don't know for example if it's useful to allow other channels to be
read when touch is not in progress or not.
So restrictions:
1. Touch screen channels can only be read when touch is enabled.
- use the available_scan_masks to control this. Or the callback that lets
you do the same dynamically.
2. You need to push these channels to your consumer driver.
- register a callback buffer rather than jumping through the hoops to
insert your own pollfunc. That will call a function in your
consumer, providing the data from the 3 channels directly.
3. You need to make sure it is using the right driver. For that you
will I think need a new interface.
Various other comments inline. I may well be missing something as this is
a fair bit of complex code to read - if so then next version should have
a clear cover letter describing why this more standard approach can't be
used.
> Signed-off-by: Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
> ---
> drivers/iio/adc/at91-sama5d2_adc.c | 455 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 446 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 9610393..79eb197 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -102,14 +102,26 @@
> #define AT91_SAMA5D2_LCDR 0x20
> /* Interrupt Enable Register */
> #define AT91_SAMA5D2_IER 0x24
> +/* Interrupt Enable Register - TS X measurement ready */
> +#define AT91_SAMA5D2_IER_XRDY BIT(20)
> +/* Interrupt Enable Register - TS Y measurement ready */
> +#define AT91_SAMA5D2_IER_YRDY BIT(21)
> +/* Interrupt Enable Register - TS pressure measurement ready */
> +#define AT91_SAMA5D2_IER_PRDY BIT(22)
> /* Interrupt Enable Register - general overrun error */
> #define AT91_SAMA5D2_IER_GOVRE BIT(25)
> +/* Interrupt Enable Register - Pen detect */
> +#define AT91_SAMA5D2_IER_PEN BIT(29)
> +/* Interrupt Enable Register - No pen detect */
> +#define AT91_SAMA5D2_IER_NOPEN BIT(30)
> /* Interrupt Disable Register */
> #define AT91_SAMA5D2_IDR 0x28
> /* Interrupt Mask Register */
> #define AT91_SAMA5D2_IMR 0x2c
> /* Interrupt Status Register */
> #define AT91_SAMA5D2_ISR 0x30
> +/* Interrupt Status Register - Pen touching sense status */
> +#define AT91_SAMA5D2_ISR_PENS BIT(31)
> /* Last Channel Trigger Mode Register */
> #define AT91_SAMA5D2_LCTMR 0x34
> /* Last Channel Compare Window Register */
> @@ -131,8 +143,37 @@
> #define AT91_SAMA5D2_CDR0 0x50
> /* Analog Control Register */
> #define AT91_SAMA5D2_ACR 0x94
> +/* Analog Control Register - Pen detect sensitivity mask */
> +#define AT91_SAMA5D2_ACR_PENDETSENS_MASK GENMASK(0, 1)
> /* Touchscreen Mode Register */
> #define AT91_SAMA5D2_TSMR 0xb0
> +/* Touchscreen Mode Register - No touch mode */
> +#define AT91_SAMA5D2_TSMR_TSMODE_NONE 0
> +/* Touchscreen Mode Register - 4 wire screen, no pressure measurement */
> +#define AT91_SAMA5D2_TSMR_TSMODE_4WIRE_NO_PRESS 1
> +/* Touchscreen Mode Register - 4 wire screen, pressure measurement */
> +#define AT91_SAMA5D2_TSMR_TSMODE_4WIRE_PRESS 2
> +/* Touchscreen Mode Register - 5 wire screen */
> +#define AT91_SAMA5D2_TSMR_TSMODE_5WIRE 3
> +/* Touchscreen Mode Register - Average samples mask */
> +#define AT91_SAMA5D2_TSMR_TSAV_MASK (3 << 4)
> +/* Touchscreen Mode Register - Average samples */
> +#define AT91_SAMA5D2_TSMR_TSAV(x) ((x) << 4)
> +/* Touchscreen Mode Register - Touch/trigger frequency ratio mask */
> +#define AT91_SAMA5D2_TSMR_TSFREQ_MASK (0xf << 8)
> +/* Touchscreen Mode Register - Touch/trigger freqency ratio */
> +#define AT91_SAMA5D2_TSMR_TSFREQ(x) ((x) << 8)
> +/* Touchscreen Mode Register - Pen Debounce Time mask */
> +#define AT91_SAMA5D2_TSMR_PENDBC_MASK (0xf << 28)
> +/* Touchscreen Mode Register - Pen Debounce Time */
> +#define AT91_SAMA5D2_TSMR_PENDBC(x) ((x) << 28)
> +/* Touchscreen Mode Register - No DMA for touch measurements */
> +#define AT91_SAMA5D2_TSMR_NOTSDMA BIT(22)
> +/* Touchscreen Mode Register - Disable pen detection */
> +#define AT91_SAMA5D2_TSMR_PENDET_DIS (0 << 24)
> +/* Touchscreen Mode Register - Enable pen detection */
> +#define AT91_SAMA5D2_TSMR_PENDET_ENA BIT(24)
> +
> /* Touchscreen X Position Register */
> #define AT91_SAMA5D2_XPOSR 0xb4
> /* Touchscreen Y Position Register */
> @@ -151,7 +192,12 @@
> #define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_FALL 2
> /* Trigger Mode external trigger any edge */
> #define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_ANY 3
> -
> +/* Trigger Mode internal periodic */
> +#define AT91_SAMA5D2_TRGR_TRGMOD_PERIODIC 5
> +/* Trigger Mode - trigger period mask */
> +#define AT91_SAMA5D2_TRGR_TRGPER_MASK (0xffff << 16)
> +/* Trigger Mode - trigger period */
> +#define AT91_SAMA5D2_TRGR_TRGPER(x) ((x) << 16)
> /* Correction Select Register */
> #define AT91_SAMA5D2_COSR 0xd0
> /* Correction Value Register */
> @@ -169,6 +215,21 @@
> #define AT91_SAMA5D2_SINGLE_CHAN_CNT 12
> #define AT91_SAMA5D2_DIFF_CHAN_CNT 6
>
> +#define AT91_SAMA5D2_TIMESTAMP_CHAN_IDX (AT91_SAMA5D2_SINGLE_CHAN_CNT + \
> + AT91_SAMA5D2_DIFF_CHAN_CNT + 1)
> +
> +#define AT91_SAMA5D2_TOUCH_X_CHAN_IDX (AT91_SAMA5D2_TIMESTAMP_CHAN_IDX + 1)
> +#define AT91_SAMA5D2_TOUCH_Y_CHAN_IDX (AT91_SAMA5D2_TOUCH_X_CHAN_IDX + 1)
> +#define AT91_SAMA5D2_TOUCH_P_CHAN_IDX (AT91_SAMA5D2_TOUCH_Y_CHAN_IDX + 1)
> +
> +#define TOUCH_SAMPLE_PERIOD_US 2000 /* 2ms */
These all need the AT91_SAMA5D2 prefix.
> +#define TOUCH_PEN_DETECT_DEBOUNCE_US 200
> +
> +#define XYZ_MASK GENMASK(11, 0)
> +
> +#define MAX_POS_BITS 12
> +
> +#define AT91_ADC_TOUCH_TRIG_SHORTNAME "touch"
> /*
> * Maximum number of bytes to hold conversion from all channels
> * without the timestamp.
> @@ -222,6 +283,37 @@
> .indexed = 1, \
> }
>
> +#define AT91_SAMA5D2_CHAN_TOUCH(num, name, mod) \
> + { \
> + .type = IIO_POSITION, \
> + .modified = 1, \
> + .channel = num, \
> + .channel2 = mod, \
> + .scan_index = num, \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + }, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
> + .datasheet_name = name, \
> + }
> +#define AT91_SAMA5D2_CHAN_PRESSURE(num, name) \
> + { \
> + .type = IIO_PRESSURE, \
> + .channel = num, \
> + .scan_index = num, \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + }, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
> + .datasheet_name = name, \
> + }
> +
> #define at91_adc_readl(st, reg) readl_relaxed(st->base + reg)
> #define at91_adc_writel(st, reg, val) writel_relaxed(val, st->base + reg)
>
> @@ -239,6 +331,20 @@ struct at91_adc_trigger {
> };
>
> /**
> + * at91_adc_touch - at91-sama5d2 touchscreen information struct
> + * @trig: hold the start timestamp of dma operation
> + * @sample_period_val: the value for periodic trigger interval
> + * @touching: is the pen touching the screen or not
> + * @x_pos: temporary placeholder for pressure computation
> + */
> +struct at91_adc_touch {
> + struct iio_trigger *trig;
> + u16 sample_period_val;
> + bool touching;
> + u32 x_pos;
> +};
> +
> +/**
> * at91_adc_dma - at91-sama5d2 dma information struct
> * @dma_chan: the dma channel acquired
> * @rx_buf: dma coherent allocated area
> @@ -267,18 +373,22 @@ struct at91_adc_state {
> struct regulator *reg;
> struct regulator *vref;
> int vref_uv;
> + unsigned int current_sample_rate;
> struct iio_trigger *trig;
> const struct at91_adc_trigger *selected_trig;
> const struct iio_chan_spec *chan;
> bool conversion_done;
> u32 conversion_value;
> + bool touch_requested;
> struct at91_adc_soc_info soc_info;
> wait_queue_head_t wq_data_available;
> struct at91_adc_dma dma_st;
> + struct at91_adc_touch touch_st;
> u16 buffer[AT91_BUFFER_MAX_HWORDS];
> /*
> * lock to prevent concurrent 'single conversion' requests through
> - * sysfs.
> + * sysfs. Also protects when enabling or disabling touchscreen
> + * producer mode and checking if this mode is enabled or not.
> */
> struct mutex lock;
> };
> @@ -310,6 +420,7 @@ static const struct at91_adc_trigger at91_adc_trigger_list[] = {
> },
> };
>
> +/* channel order is not subject to change. inkern consumers rely on this */
> static const struct iio_chan_spec at91_adc_channels[] = {
> AT91_SAMA5D2_CHAN_SINGLE(0, 0x50),
> AT91_SAMA5D2_CHAN_SINGLE(1, 0x54),
> @@ -329,10 +440,103 @@ static const struct iio_chan_spec at91_adc_channels[] = {
> AT91_SAMA5D2_CHAN_DIFF(6, 7, 0x68),
> AT91_SAMA5D2_CHAN_DIFF(8, 9, 0x70),
> AT91_SAMA5D2_CHAN_DIFF(10, 11, 0x78),
> - IIO_CHAN_SOFT_TIMESTAMP(AT91_SAMA5D2_SINGLE_CHAN_CNT
> - + AT91_SAMA5D2_DIFF_CHAN_CNT + 1),
> + IIO_CHAN_SOFT_TIMESTAMP(AT91_SAMA5D2_TIMESTAMP_CHAN_IDX),
> + AT91_SAMA5D2_CHAN_TOUCH(AT91_SAMA5D2_TOUCH_X_CHAN_IDX, "x", IIO_MOD_X),
> + AT91_SAMA5D2_CHAN_TOUCH(AT91_SAMA5D2_TOUCH_Y_CHAN_IDX, "y", IIO_MOD_Y),
> + AT91_SAMA5D2_CHAN_PRESSURE(AT91_SAMA5D2_TOUCH_P_CHAN_IDX, "pressure"),
> };
>
> +static int at91_adc_configure_touch(struct at91_adc_state *st, bool state)
> +{
> + u32 clk_khz = st->current_sample_rate / 1000;
> + int i = 0;
> + u16 pendbc;
> + u32 tsmr, acr;
> +
> + if (!state) {
> + /* disabling touch IRQs and setting mode to no touch enabled */
> + at91_adc_writel(st, AT91_SAMA5D2_IDR,
> + AT91_SAMA5D2_IER_PEN | AT91_SAMA5D2_IER_NOPEN);
> + at91_adc_writel(st, AT91_SAMA5D2_TSMR, 0);
> + return 0;
> + }
> + /*
> + * debounce time is in microseconds, we need it in milliseconds to
> + * multiply with kilohertz, so, divide by 1000, but after the multiply.
> + * round up to make sure pendbc is at least 1
> + */
> + pendbc = round_up(TOUCH_PEN_DETECT_DEBOUNCE_US * clk_khz / 1000, 1);
> +
> + /* get the required exponent */
> + while (pendbc >> i++)
> + ;
This is related to the first 0? There are cleaner ways of doing this
with ffs and friends.
> +
> + pendbc = i;
> +
> + tsmr = AT91_SAMA5D2_TSMR_TSMODE_4WIRE_PRESS;
> +
> + tsmr |= AT91_SAMA5D2_TSMR_TSAV(1) & AT91_SAMA5D2_TSMR_TSAV_MASK;
> + tsmr |= AT91_SAMA5D2_TSMR_PENDBC(pendbc) &
> + AT91_SAMA5D2_TSMR_PENDBC_MASK;
> + tsmr |= AT91_SAMA5D2_TSMR_NOTSDMA;
> + tsmr |= AT91_SAMA5D2_TSMR_PENDET_ENA;
> + tsmr |= AT91_SAMA5D2_TSMR_TSFREQ(1) & AT91_SAMA5D2_TSMR_TSFREQ_MASK;
> +
> + at91_adc_writel(st, AT91_SAMA5D2_TSMR, tsmr);
> +
> + acr = at91_adc_readl(st, AT91_SAMA5D2_ACR);
> + acr &= ~AT91_SAMA5D2_ACR_PENDETSENS_MASK;
> + acr |= 0x02 & AT91_SAMA5D2_ACR_PENDETSENS_MASK;
> + at91_adc_writel(st, AT91_SAMA5D2_ACR, acr);
> +
> + /* Sample Period Time = (TRGPER + 1) / ADCClock */
> + st->touch_st.sample_period_val = round_up((TOUCH_SAMPLE_PERIOD_US *
> + clk_khz / 1000) - 1, 1);
> + /* enable pen detect IRQ */
> + at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_PEN);
> +
> + return 0;
> +}
> +
> +static int at91_adc_touch_trigger_validate_device(struct iio_trigger *trig,
> + struct iio_dev *indio_dev)
> +{
> + /* the touch trigger cannot be used with a buffer */
> + return -EBUSY;
> +}
> +
> +static int at91_adc_configure_touch_trigger(struct iio_trigger *trig,
> + bool state)
> +{
> + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> + struct at91_adc_state *st = iio_priv(indio_dev);
> + int ret = 0;
> +
> + /*
> + * If we configure this with the IRQ enabled, the pen detected IRQ
> + * might fire before we finish setting all up, and the IRQ handler
> + * might misbehave. Better to reenable the IRQ after we are done
> + */
> + disable_irq_nosync(st->irq);
> +
> + mutex_lock(&st->lock);
> + if (state) {
> + ret = iio_buffer_enabled(indio_dev);
> + if (ret) {
> + dev_dbg(&indio_dev->dev, "trigger is currently in use\n");
> + ret = -EBUSY;
> + goto configure_touch_unlock_exit;
> + }
> + }
> + at91_adc_configure_touch(st, state);
> + st->touch_requested = state;
> +
> +configure_touch_unlock_exit:
> + enable_irq(st->irq);
> + mutex_unlock(&st->lock);
> + return ret;
> +}
> +
> static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
> {
> struct iio_dev *indio = iio_trigger_get_drvdata(trig);
> @@ -390,12 +594,27 @@ static int at91_adc_reenable_trigger(struct iio_trigger *trig)
> return 0;
> }
>
> +static int at91_adc_reenable_touch_trigger(struct iio_trigger *trig)
> +{
> + struct iio_dev *indio = iio_trigger_get_drvdata(trig);
> + struct at91_adc_state *st = iio_priv(indio);
> +
> + enable_irq(st->irq);
> +
> + return 0;
> +}
> static const struct iio_trigger_ops at91_adc_trigger_ops = {
> .set_trigger_state = &at91_adc_configure_trigger,
> .try_reenable = &at91_adc_reenable_trigger,
> .validate_device = iio_trigger_validate_own_device,
> };
>
> +static const struct iio_trigger_ops at91_adc_touch_trigger_ops = {
> + .set_trigger_state = &at91_adc_configure_touch_trigger,
> + .try_reenable = &at91_adc_reenable_touch_trigger,
> + .validate_device = &at91_adc_touch_trigger_validate_device,
> +};
> +
> static int at91_adc_dma_size_done(struct at91_adc_state *st)
> {
> struct dma_tx_state state;
> @@ -490,6 +709,23 @@ static int at91_adc_dma_start(struct iio_dev *indio_dev)
> return 0;
> }
>
> +static int at91_adc_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct at91_adc_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + /* have to make sure nobody is requesting the trigger right now */
This needs some more explanation as I don't totally follow what this
is designed to protect against.
Realistically a device is only useful if it has one trigger at a time
feeding a valid set of channels to however many consumers (whether
in the driver or not).
> + mutex_lock(&st->lock);
> + ret = st->touch_requested;
> + mutex_unlock(&st->lock);
> +
> + /*
> + * if the trigger is used by the touchscreen,
> + * we must return an error
> + */
> + return ret ? -EBUSY : 0;
> +}
> +
> static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
> {
> int ret;
> @@ -538,6 +774,7 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
> }
>
> static const struct iio_buffer_setup_ops at91_buffer_setup_ops = {
> + .preenable = &at91_adc_buffer_preenable,
> .postenable = &at91_adc_buffer_postenable,
> .predisable = &at91_adc_buffer_predisable,
> };
> @@ -555,7 +792,11 @@ static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio,
>
> trig->dev.parent = indio->dev.parent;
> iio_trigger_set_drvdata(trig, indio);
> - trig->ops = &at91_adc_trigger_ops;
> +
> + if (strcmp(trigger_name, AT91_ADC_TOUCH_TRIG_SHORTNAME))
Pass this is as a parameter to the function and avoid the strcmp nastiness.
> + trig->ops = &at91_adc_trigger_ops;
> + else
> + trig->ops = &at91_adc_touch_trigger_ops;
>
> ret = devm_iio_trigger_register(&indio->dev, trig);
> if (ret)
> @@ -571,7 +812,16 @@ static int at91_adc_trigger_init(struct iio_dev *indio)
> st->trig = at91_adc_allocate_trigger(indio, st->selected_trig->name);
> if (IS_ERR(st->trig)) {
> dev_err(&indio->dev,
> - "could not allocate trigger\n");
> + "could not allocate trigger %s\n",
> + st->selected_trig->name);
> + return PTR_ERR(st->trig);
> + }
> +
> + st->touch_st.trig = at91_adc_allocate_trigger(indio,
> + AT91_ADC_TOUCH_TRIG_SHORTNAME);
> + if (IS_ERR(st->trig)) {
> + dev_err(&indio->dev, "could not allocate trigger"
> + AT91_ADC_TOUCH_TRIG_SHORTNAME "\n");
> return PTR_ERR(st->trig);
> }
>
> @@ -703,6 +953,8 @@ static void at91_adc_setup_samp_freq(struct at91_adc_state *st, unsigned freq)
>
> dev_dbg(&indio_dev->dev, "freq: %u, startup: %u, prescal: %u\n",
> freq, startup, prescal);
> +
> + st->current_sample_rate = freq;
> }
>
> static unsigned at91_adc_get_sample_freq(struct at91_adc_state *st)
> @@ -718,23 +970,77 @@ static unsigned at91_adc_get_sample_freq(struct at91_adc_state *st)
> return f_adc;
> }
>
> +static irqreturn_t at91_adc_pen_detect_interrupt(struct at91_adc_state *st)
> +{
> + at91_adc_writel(st, AT91_SAMA5D2_IDR, AT91_SAMA5D2_IER_PEN);
> + at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_NOPEN |
> + AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
> + AT91_SAMA5D2_IER_PRDY);
> + at91_adc_writel(st, AT91_SAMA5D2_TRGR,
> + AT91_SAMA5D2_TRGR_TRGMOD_PERIODIC |
> + AT91_SAMA5D2_TRGR_TRGPER(st->touch_st.sample_period_val));
> + st->touch_st.touching = true;
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t at91_adc_no_pen_detect_interrupt(struct at91_adc_state *st)
> +{
> + at91_adc_writel(st, AT91_SAMA5D2_TRGR, 0);
> + at91_adc_writel(st, AT91_SAMA5D2_IDR, AT91_SAMA5D2_IER_NOPEN |
> + AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
> + AT91_SAMA5D2_IER_PRDY);
> + st->touch_st.touching = false;
Hmm. I think we are unfortunately racing here. There is nothing preventing
this running concurrently with the read_raw calls that check the same variable.
If this is fine (because we will always get valid data anyway (if stale)
then a comment is needed to explain that.
> +
> + disable_irq_nosync(st->irq);
> + iio_trigger_poll(st->touch_st.trig);
Comment to explain why a poll here is fine, but not on the pen on would be
good (I can guess but better to state it!)
> +
> + at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_PEN);
> +
> + return IRQ_HANDLED;
> +}
> +
> static irqreturn_t at91_adc_interrupt(int irq, void *private)
> {
> struct iio_dev *indio = private;
> struct at91_adc_state *st = iio_priv(indio);
> u32 status = at91_adc_readl(st, AT91_SAMA5D2_ISR);
> u32 imr = at91_adc_readl(st, AT91_SAMA5D2_IMR);
> + u32 rdy_mask = AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
> + AT91_SAMA5D2_IER_PRDY;
>
> if (!(status & imr))
> return IRQ_NONE;
>
> - if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) {
> + if (st->touch_requested && (status & AT91_SAMA5D2_IER_PEN)) {
> + /* pen detected IRQ */
> + return at91_adc_pen_detect_interrupt(st);
> + } else if (st->touch_requested && (status & AT91_SAMA5D2_IER_NOPEN)) {
> + /* nopen detected IRQ */
> + return at91_adc_no_pen_detect_interrupt(st);
> + } else if (st->touch_requested && (status & AT91_SAMA5D2_ISR_PENS) &&
> + ((status & rdy_mask) == rdy_mask)) {
> + /* periodic trigger IRQ - during pen sense */
> + disable_irq_nosync(irq);
> + iio_trigger_poll(st->touch_st.trig);
> + } else if ((st->touch_requested && (status & AT91_SAMA5D2_ISR_PENS))) {
> + /*
> + * touching, but the measurements are not ready yet.
> + * read and ignore.
> + */
> + status = at91_adc_readl(st, AT91_SAMA5D2_XPOSR);
> + status = at91_adc_readl(st, AT91_SAMA5D2_YPOSR);
> + status = at91_adc_readl(st, AT91_SAMA5D2_PRESSR);
> + } else if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) {
> + /* buffered trigger without DMA */
> disable_irq_nosync(irq);
> iio_trigger_poll(indio->trig);
> } else if (iio_buffer_enabled(indio) && st->dma_st.dma_chan) {
> + /* buffered trigger with DMA - should not happen */
> disable_irq_nosync(irq);
> WARN(true, "Unexpected irq occurred\n");
> } else if (!iio_buffer_enabled(indio)) {
> + /* software requested conversion */
> st->conversion_value = at91_adc_readl(st, st->chan->address);
> st->conversion_done = true;
> wake_up_interruptible(&st->wq_data_available);
> @@ -742,6 +1048,96 @@ static irqreturn_t at91_adc_interrupt(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> +static u32 at91_adc_touch_x_pos(struct at91_adc_state *st)
> +{
> + u32 xscale, val;
> + u32 x, xpos;
> +
> + /* x position = (x / xscale) * max, max = 2^MAX_POS_BITS - 1 */
> + val = at91_adc_readl(st, AT91_SAMA5D2_XPOSR);
> + if (!val)
> + dev_dbg(&iio_priv_to_dev(st)->dev, "x_pos is 0\n");
> +
> + xpos = val & XYZ_MASK;
> + x = (xpos << MAX_POS_BITS) - xpos;
> + xscale = (val >> 16) & XYZ_MASK;
> + if (xscale == 0) {
> + dev_err(&iio_priv_to_dev(st)->dev, "xscale is 0\n");
> + return 0;
> + }
> + x /= xscale;
> + st->touch_st.x_pos = x;
> +
> + return x;
> +}
> +
> +static u32 at91_adc_touch_y_pos(struct at91_adc_state *st)
> +{
> + u32 yscale, val;
> + u32 y, ypos;
> +
> + /* y position = (y / yscale) * max, max = 2^MAX_POS_BITS - 1 */
> + val = at91_adc_readl(st, AT91_SAMA5D2_YPOSR);
> + ypos = val & XYZ_MASK;
> + y = (ypos << MAX_POS_BITS) - ypos;
> + yscale = (val >> 16) & XYZ_MASK;
> +
> + if (yscale == 0)
> + return 0;
> +
> + y /= yscale;
> +
> + return y;
> +}
> +
> +static u32 at91_adc_touch_pressure(struct at91_adc_state *st)
> +{
> + u32 val, z1, z2;
> + u32 pres;
> + u32 rxp = 1;
> + u32 factor = 1000;
> +
> + /* calculate the pressure */
> + val = at91_adc_readl(st, AT91_SAMA5D2_PRESSR);
> + z1 = val & XYZ_MASK;
XYZ_MASK seems oddly named given what this seems to be doing...
> + z2 = (val >> 16) & XYZ_MASK;
> +
> + if (z1 != 0)
> + pres = rxp * (st->touch_st.x_pos * factor / 1024) *
> + (z2 * factor / z1 - factor) /
> + factor;
> + else
> + pres = 0xFFFFFFFF; /* no pen contact */
> +
> + return pres;
> +}
> +
> +static int at91_adc_read_position(struct at91_adc_state *st, int chan, int *val)
> +{
> + if (!st->touch_st.touching)
> + return -ENODATA;
> + if (chan == AT91_SAMA5D2_TOUCH_X_CHAN_IDX)
> + *val = at91_adc_touch_x_pos(st);
> + else if (chan == AT91_SAMA5D2_TOUCH_Y_CHAN_IDX)
> + *val = at91_adc_touch_y_pos(st);
> + else
> + return -ENODATA;
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int at91_adc_read_pressure(struct at91_adc_state *st, int chan, int *val)
> +{
> + if (!st->touch_st.touching)
> + return -ENODATA;
> + if (chan == AT91_SAMA5D2_TOUCH_P_CHAN_IDX)
General code flow simpler if you check error first
if (chan != AT91_SAMA5D2_TOUCH_P_CHAN_IDX)
return -ENODATA;
*val =...
> + *val = at91_adc_touch_pressure(st);
> + else
> + return -ENODATA;
> +
> + return IIO_VAL_INT;
> +}
> +
> static int at91_adc_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val, int *val2, long mask)
> @@ -752,11 +1148,38 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> + mutex_lock(&st->lock);
> +
> + if (chan->type == IIO_POSITION) {
Switch or else if as only one is true at a time.
Hmm. So you allow sysfs reads of these channels if touch in progress.
Do we actually have a use for this? Seems we could have simpler code
by just not providing direct reads for them if not.
> + ret = at91_adc_read_position(st, chan->channel, val);
> + mutex_unlock(&st->lock);
> + return ret;
> + }
> + if (chan->type == IIO_PRESSURE) {
> + ret = at91_adc_read_pressure(st, chan->channel, val);
> + mutex_unlock(&st->lock);
> + return ret;
> + }
> + /* if we using touch, channels 0, 1, 2, 3 are unavailable */
> + if (st->touch_requested && chan->channel <= 3) {
> + mutex_unlock(&st->lock);
> + return -EBUSY;
> + }
> + /*
> + * if we have the periodic trigger set up, we can't use
> + * software trigger either.
> + */
> + if (st->touch_st.touching) {
> + mutex_unlock(&st->lock);
> + return -ENODATA;
> + }
> +
> /* we cannot use software trigger if hw trigger enabled */
> ret = iio_device_claim_direct_mode(indio_dev);
> - if (ret)
> + if (ret) {
> + mutex_unlock(&st->lock);
> return ret;
> - mutex_lock(&st->lock);
> + }
>
> st->chan = chan;
>
> @@ -785,6 +1208,11 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev,
>
> at91_adc_writel(st, AT91_SAMA5D2_IDR, BIT(chan->channel));
> at91_adc_writel(st, AT91_SAMA5D2_CHDR, BIT(chan->channel));
> + /*
> + * It is possible that after this conversion, we reuse these
> + * channels for the touchscreen. So, reset the COR now.
> + */
> + at91_adc_writel(st, AT91_SAMA5D2_COR, 0);
>
> /* Needed to ACK the DRDY interruption */
> at91_adc_readl(st, AT91_SAMA5D2_LCDR);
> @@ -1180,6 +1608,10 @@ static int at91_adc_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct at91_adc_state *st = iio_priv(indio_dev);
>
> + mutex_lock(&st->lock);
> + devm_iio_trigger_unregister(&indio_dev->dev, st->touch_st.trig);
As before this needs detailed explanation. It should not be necessary.
> + mutex_unlock(&st->lock);
> +
> if (st->selected_trig->hw_trig)
> devm_iio_trigger_unregister(&indio_dev->dev, st->trig);
>
> @@ -1245,6 +1677,11 @@ static __maybe_unused int at91_adc_resume(struct device *dev)
> if (iio_buffer_enabled(indio_dev))
> at91_adc_configure_trigger(st->trig, true);
>
> + mutex_lock(&st->lock);
> + if (st->touch_requested)
> + at91_adc_configure_touch_trigger(st->touch_st.trig, true);
> + mutex_unlock(&st->lock);
> +
> return 0;
>
> vref_disable_resume:
^ permalink raw reply
* Re: [PATCH 11/14] iio: adc: at91-sama5d2_adc: optimize scan index for diff channels
From: Jonathan Cameron @ 2017-12-29 16:24 UTC (permalink / raw)
To: Eugen Hristev
Cc: nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA,
alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1513955241-10985-12-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
On Fri, 22 Dec 2017 17:07:18 +0200
Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org> wrote:
> Optimize the scan index for the differential channels. Before, it
> was single channel count + index of the first single channel
> number of the differential pair. (e.g. 11+0, +2, +4, etc.)
> Divide that number by two (since it's always even), and add it up
> as a scan index to have consecutive numbered channels in the
> index.
Why? This is odd as it stands, but that isn't a strong enough reason
to fix it.
This is making a userspace ABI change. We need a very strong
argument for why it is necessary and also why existing userspace
won't care.
>
> Signed-off-by: Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
> ---
> drivers/iio/adc/at91-sama5d2_adc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 7b9febc..9610393 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -209,7 +209,7 @@
> .channel = num, \
> .channel2 = num2, \
> .address = addr, \
> - .scan_index = num + AT91_SAMA5D2_SINGLE_CHAN_CNT, \
> + .scan_index = (num >> 1) + AT91_SAMA5D2_SINGLE_CHAN_CNT,\
> .scan_type = { \
> .sign = 's', \
> .realbits = 12, \
^ permalink raw reply
* Re: [PATCH 10/14] iio: adc: at91-sama5d2_adc: force trigger removal on module remove
From: Jonathan Cameron @ 2017-12-29 16:22 UTC (permalink / raw)
To: Eugen Hristev
Cc: nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA,
alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1513955241-10985-11-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
On Fri, 22 Dec 2017 17:07:17 +0200
Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org> wrote:
> On module remove, if we do not call trigger remove, the trigger
> stays in the subsystem, and on further module insert, we will have
> multiple triggers, and the old one is not usable.
> Have to call the remove function on module remove to solve this.
>
> Signed-off-by: Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
This needs more explanation. I can't see why the managed removal
isn't sufficient. On removal the dev should have gone away taking
the trigger with it.
If it isn't then it looks like a straight forward bug that needs fixing.
Jonathan
> ---
> drivers/iio/adc/at91-sama5d2_adc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 4eff835..7b9febc 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -1180,6 +1180,9 @@ static int at91_adc_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct at91_adc_state *st = iio_priv(indio_dev);
>
> + if (st->selected_trig->hw_trig)
> + devm_iio_trigger_unregister(&indio_dev->dev, st->trig);
> +
> iio_device_unregister(indio_dev);
>
> at91_adc_dma_disable(pdev);
^ permalink raw reply
* Re: [PATCH 02/14] iio: Add channel for Position
From: Jonathan Cameron @ 2017-12-29 16:09 UTC (permalink / raw)
To: Eugen Hristev
Cc: nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA,
alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1513955241-10985-3-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
On Fri, 22 Dec 2017 17:07:09 +0200
Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org> wrote:
> Add new channel type for position on a pad.
>
> These type of analog sensor represents the position of a pen
> on a touchpad, and is represented as a voltage, which can be
> converted to a position on X and Y axis on the pad.
>
> The channel can then be consumed by a touchscreen driver or
> read as-is for a raw indication of the touchpen on a touchpad.
>
> Signed-off-by: Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
> ---
> Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
> drivers/iio/industrialio-core.c | 1 +
> include/uapi/linux/iio/types.h | 1 +
> tools/iio/iio_event_monitor.c | 2 ++
> 4 files changed, 15 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index a478740..d2b9e2f 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -190,6 +190,17 @@ Description:
> but should match other such assignments on device).
> Units after application of scale and offset are m/s^2.
>
> +What: /sys/bus/iio/devices/iio:deviceX/in_position_x_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_position_y_raw
> +KernelVersion: 4.16
> +Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description:
> + Position in direction x or y on a pad (may be arbitrarily
> + assigned but should match other such assignments on device).
> + Units after application of scale and offset are millipercents
> + from the pad's size in both directions. Should be calibrated by
> + the consumer.
Hmm. The units are an issues as to be consistent with the existing ABI position
should be in meters. Perhaps the trick is to do similar to we have done for
relative humidity and call this in_positionrelative_x_raw etc.
That leaves position open for absolute position devices (who knows what)
in the future.
> +
> What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 2e8e36f..a4fa49b 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -85,6 +85,7 @@ static const char * const iio_chan_type_name_spec[] = {
> [IIO_COUNT] = "count",
> [IIO_INDEX] = "index",
> [IIO_GRAVITY] = "gravity",
> + [IIO_POSITION] = "position",
> };
>
> static const char * const iio_modifier_names[] = {
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 4213cdf..35e17da 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -44,6 +44,7 @@ enum iio_chan_type {
> IIO_COUNT,
> IIO_INDEX,
> IIO_GRAVITY,
> + IIO_POSITION,
> };
>
> enum iio_modifier {
> diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
> index b61245e..0c2b317 100644
> --- a/tools/iio/iio_event_monitor.c
> +++ b/tools/iio/iio_event_monitor.c
> @@ -58,6 +58,7 @@ static const char * const iio_chan_type_name_spec[] = {
> [IIO_PH] = "ph",
> [IIO_UVINDEX] = "uvindex",
> [IIO_GRAVITY] = "gravity",
> + [IIO_POSITION] = "position",
> };
>
> static const char * const iio_ev_type_text[] = {
> @@ -151,6 +152,7 @@ static bool event_is_known(struct iio_event_data *event)
> case IIO_PH:
> case IIO_UVINDEX:
> case IIO_GRAVITY:
> + case IIO_POSITION:
> break;
> default:
> return false;
^ permalink raw reply
* [PATCH v12 1/3] Documentation: Add device tree binding for Goldfish PIC driver
From: Aleksandar Markovic @ 2017-12-29 15:41 UTC (permalink / raw)
To: linux-mips-6z/3iImG2C8G8FEW9MqTrA
Cc: Miodrag Dinic, Goran Ferenc, Aleksandar Markovic, David S. Miller,
devicetree-u79uwXL29TY76Z2rM5mHXA, Douglas Leung,
Greg Kroah-Hartman, James Hogan, Jason Cooper,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Marc Zyngier, Mark Rutland,
Mauro Carvalho Chehab, Paul Burton, Petar Jovanovic,
Raghu Gandham, Randy Dunlap, Rob Herring, Thomas Gleixner
In-Reply-To: <1514562138-13774-1-git-send-email-aleksandar.markovic-FblTVreYubkAvxtiuMwx3w@public.gmane.org>
From: Miodrag Dinic <miodrag.dinic-8NJIiSa5LzA@public.gmane.org>
Add documentation for DT binding of Goldfish PIC driver. The compatible
string used by OS for binding the driver is "google,goldfish-pic".
Signed-off-by: Miodrag Dinic <miodrag.dinic-8NJIiSa5LzA@public.gmane.org>
Signed-off-by: Goran Ferenc <goran.ferenc-8NJIiSa5LzA@public.gmane.org>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic-8NJIiSa5LzA@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
.../interrupt-controller/google,goldfish-pic.txt | 30 ++++++++++++++++++++++
MAINTAINERS | 5 ++++
2 files changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
diff --git a/Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt b/Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
new file mode 100644
index 0000000..35f7527
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
@@ -0,0 +1,30 @@
+Android Goldfish PIC
+
+Android Goldfish programmable interrupt device used by Android
+emulator.
+
+Required properties:
+
+- compatible : should contain "google,goldfish-pic"
+- reg : <registers mapping>
+- interrupts : <interrupt mapping>
+
+Example for mips when used in cascade mode:
+
+ cpuintc {
+ #interrupt-cells = <0x1>;
+ #address-cells = <0>;
+ interrupt-controller;
+ compatible = "mti,cpu-interrupt-controller";
+ };
+
+ interrupt-controller@1f000000 {
+ compatible = "google,goldfish-pic";
+ reg = <0x1f000000 0x1000>;
+
+ interrupt-controller;
+ #interrupt-cells = <0x1>;
+
+ interrupt-parent = <&cpuintc>;
+ interrupts = <0x2>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index a6e86e2..7152b90 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -867,6 +867,11 @@ S: Supported
F: drivers/android/
F: drivers/staging/android/
+ANDROID GOLDFISH PIC DRIVER
+M: Miodrag Dinic <miodrag.dinic-8NJIiSa5LzA@public.gmane.org>
+S: Supported
+F: Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
+
ANDROID GOLDFISH RTC DRIVER
M: Miodrag Dinic <miodrag.dinic-8NJIiSa5LzA@public.gmane.org>
S: Supported
--
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 related
* Re: [PATCH v4 06/15] clk: Add Ingenic jz4770 CGU driver
From: Paul Cercueil @ 2017-12-29 15:02 UTC (permalink / raw)
To: Philippe Ombredanne
Cc: Ralf Baechle, Rob Herring, Michael Turquette, Mark Rutland,
Stephen Boyd, Maarten ter Huurne,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
Linux MIPS, linux-clk
In-Reply-To: <CAOFm3uEbEx+HTsOANchqiReuDBa96O0Y-V-igLGWPF-OxQ2jtw@mail.gmail.com>
Dear Mr. Ombredanne,
Le ven. 29 déc. 2017 à 13:55, Philippe Ombredanne
<pombredanne@nexb.com> a écrit :
> Dear Mr Crapouillou-Cercueil-Sir,
>
> On Thu, Dec 28, 2017 at 2:56 PM, Paul Cercueil <paul@crapouillou.net>
> wrote:
>> Add support for the clocks provided by the CGU in the Ingenic JZ4770
>> SoC.
>
> <snip>
>
>> --- /dev/null
>> +++ b/drivers/clk/ingenic/jz4770-cgu.c
>> @@ -0,0 +1,487 @@
>> +/*
>> + * JZ4770 SoC CGU driver
>> + *
>> + * Copyright 2017, Paul Cercueil <paul@crapouillou.net>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> modify
>> + * it under the terms of the GNU General Public License version 2
>> or later
>> + * as published by the Free Software Foundation.
>> + */
>
> Do you mind using a simpler one-line SPDX identifier instead of this
> fine but clearly crapouillish legalese boilerplate? Unless you are
> trying to turn the kernel in a legal compendium, of course ;)
>
> This is documented in Thomas doc patches. This would apply to your
> entire patch set.
> Thank you for your kind consideration!
Sure, I wasn't aware about that new SPDX identifier thing. I'll do it
in V5.
Regards,
-Paul
^ permalink raw reply
* Re: [PATCH 4/6] clk: ingenic: Add JZ47xx TCU clocks driver
From: Paul Cercueil @ 2017-12-29 14:02 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Daniel Lezcano,
Lee Jones
Cc: linux-clk, linux-kernel, devicetree
In-Reply-To: <20171229125942.27305-5-paul@crapouillou.net>
[...]
> + tcu->clocks.clk_num = nb_clks;
> + tcu->clocks.clks = kcalloc(nb_clocks, sizeof(struct clk *),
> GFP_KERNEL);
> + if (!tcu->clocks.clks) {
> + pr_err("%s: cannot allocate memory\n", __func__);
> + goto err_free_tcu;
> + }
Facepalm. A quick edit from kzalloc to kcalloc and I managed to break
it.
It should be nb_clks not nb_clocks. I'll fix that in the V2.
-Paul
^ permalink raw reply
* Re: [PATCH v2 1/5] ARM: sun9i: Support SMP on A80 with Multi-Cluster Power Management (MCPM)
From: Philippe Ombredanne @ 2017-12-29 13:47 UTC (permalink / raw)
To: Mylène Josserand, Chen-Yu Tsai
Cc: Maxime Ripard, Russell King, Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, LKML,
clabbe.montjoie, Thomas Petazzoni, quentin.schulz
In-Reply-To: <20171229105506.24851-2-mylene.josserand@free-electrons.com>
Mylène, Chen-Yu,
On Fri, Dec 29, 2017 at 11:55 AM, Mylène Josserand
<mylene.josserand@free-electrons.com> wrote:
> From: Chen-Yu Tsai <wens@csie.org>
>
> The A80 is a big.LITTLE SoC with 1 cluster of 4 Cortex-A7s and
> 1 cluster of 4 Cortex-A15s.
<snip>
> --- /dev/null
> +++ b/arch/arm/mach-sunxi/mcpm.c
> @@ -0,0 +1,391 @@
> +/*
> + * Copyright (c) 2015 Chen-Yu Tsai
> + *
> + * Chen-Yu Tsai <wens@csie.org>
> + *
> + * arch/arm/mach-sunxi/mcpm.c
> + *
> + * Based on arch/arm/mach-exynos/mcpm-exynos.c and Allwinner code
> + *
> + * 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.
> + */
Would you mind using a concise SPDX tag instead, as documented by
Thomas doc patches?
Thank you for your kind consideration!
--
Cordially
Philippe Ombredanne
^ permalink raw reply
* [PATCH 6/6] MAINTAINERS: Add myself as maintainer for Ingenic TCU drivers
From: Paul Cercueil @ 2017-12-29 12:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Daniel Lezcano,
Lee Jones
Cc: linux-clk, linux-kernel, devicetree, Paul Cercueil
In-Reply-To: <20171229125942.27305-1-paul@crapouillou.net>
Add myself as maintainer for the ingenic-tcu-intc interrupt controller
driver, the ingenic-tcu-clocks clock driver, and the ingenic-tcu
clocksource driver.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index a6e86e20761e..1f274bc2bbf7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6908,6 +6908,15 @@ L: linux-mtd@lists.infradead.org
S: Maintained
F: drivers/mtd/nand/jz4780_*
+INGENIC JZ47xx TCU drivers
+M: Paul Cercueil <paul@crapouillou.net>
+S: Maintained
+F: drivers/clk/ingenic/tcu.c
+F: drivers/irqchip/irq-ingenic-tcu.c
+F: drivers/clocksource/timer-ingenic.c
+F: include/linux/mfd/syscon/ingenic-tcu.h
+F: include/dt-bindings/clock/ingenic,tcu.h
+
INOTIFY
M: Jan Kara <jack@suse.cz>
R: Amir Goldstein <amir73il@gmail.com>
--
2.11.0
^ permalink raw reply related
* [PATCH 5/6] clocksource: Add a new timer-ingenic driver
From: Paul Cercueil @ 2017-12-29 12:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Daniel Lezcano,
Lee Jones
Cc: linux-clk, linux-kernel, devicetree, Paul Cercueil
In-Reply-To: <20171229125942.27305-1-paul@crapouillou.net>
This driver will use the TCU (Timer Counter Unit) present on the Ingenic
JZ47xx SoCs to provide the kernel with a clocksource and timers.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
.../devicetree/bindings/timer/ingenic,tcu.txt | 35 +++
drivers/clocksource/Kconfig | 8 +
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-ingenic.c | 259 +++++++++++++++++++++
4 files changed, 303 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/ingenic,tcu.txt
create mode 100644 drivers/clocksource/timer-ingenic.c
diff --git a/Documentation/devicetree/bindings/timer/ingenic,tcu.txt b/Documentation/devicetree/bindings/timer/ingenic,tcu.txt
new file mode 100644
index 000000000000..e4944972ea53
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/ingenic,tcu.txt
@@ -0,0 +1,35 @@
+Ingenic JZ47xx SoCs Timer/Counter Unit driver
+---------------------------------------------
+
+Required properties:
+
+- compatible : should be "ingenic,<socname>-tcu". Valid strings are:
+ * ingenic,jz4740-tcu
+ * ingenic,jz4770-tcu
+ * ingenic,jz4780-tcu
+- interrupt-parent : phandle of the TCU interrupt controller.
+- interrupts : Specifies the interrupts the controller is connected to.
+- clocks : List of phandle & clock specifiers for the TCU clocks.
+- clock-names : List of name strings for the TCU clocks.
+- ingenic,channels: a list of TCU channels to be used as timers.
+
+Example:
+
+/ {
+ regmap {
+ compatible = "simple-mfd", "syscon";
+ reg = <0x10002000 0x1000>;
+
+ tcu: timer {
+ compatible = "ingenic,jz4740-tcu";
+
+ clocks = <&tcu 0>, <&tcu 1>;
+ clock-names = "timer0", "timer1";
+
+ interrupt-parent = <&tcu>;
+ interrupts = <0>, <1>;
+
+ ingenic,channels = <0>, <1>;
+ };
+ };
+};
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index c729a88007d0..7b6dedf0347d 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -583,4 +583,12 @@ config CLKSRC_ST_LPC
Enable this option to use the Low Power controller timer
as clocksource.
+config INGENIC_TIMER
+ bool "Clocksource/timer using the TCU in Ingenic JZ SoCs"
+ depends on MACH_INGENIC || COMPILE_TEST
+ select CLKSRC_OF
+ default y
+ help
+ Support for the timer/counter unit of the Ingenic JZ SoCs.
+
endmenu
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 72711f1491e3..607c7de07d02 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -73,5 +73,6 @@ obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o
obj-$(CONFIG_H8300_TMR8) += h8300_timer8.o
obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o
obj-$(CONFIG_H8300_TPU) += h8300_tpu.o
+obj-$(CONFIG_INGENIC_TIMER) += timer-ingenic.o
obj-$(CONFIG_CLKSRC_ST_LPC) += clksrc_st_lpc.o
obj-$(CONFIG_X86_NUMACHIP) += numachip.o
diff --git a/drivers/clocksource/timer-ingenic.c b/drivers/clocksource/timer-ingenic.c
new file mode 100644
index 000000000000..2ec922e5fc0b
--- /dev/null
+++ b/drivers/clocksource/timer-ingenic.c
@@ -0,0 +1,259 @@
+/*
+ * Copyright (C) 2016 Paul Cercueil <paul@crapouillou.net>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mfd/syscon/ingenic-tcu.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define NUM_CHANNELS 8
+
+struct ingenic_tcu;
+
+struct ingenic_tcu_channel {
+ unsigned int idx;
+ struct clk *clk;
+};
+
+struct ingenic_tcu {
+ struct ingenic_tcu_channel channels[NUM_CHANNELS];
+ unsigned long requested;
+ struct regmap *map;
+};
+
+struct ingenic_clock_event_device {
+ struct clock_event_device cevt;
+ struct ingenic_tcu_channel *channel;
+ char name[32];
+};
+
+#define ingenic_cevt(_evt) \
+ container_of(_evt, struct ingenic_clock_event_device, cevt)
+
+static inline struct ingenic_tcu *to_ingenic_tcu(struct ingenic_tcu_channel *ch)
+{
+ return container_of(ch, struct ingenic_tcu, channels[ch->idx]);
+}
+
+static int ingenic_tcu_cevt_set_state_shutdown(struct clock_event_device *evt)
+{
+ struct ingenic_clock_event_device *jzcevt = ingenic_cevt(evt);
+ struct ingenic_tcu_channel *channel = jzcevt->channel;
+ struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
+ unsigned int idx = channel->idx;
+
+ regmap_write(tcu->map, REG_TECR, BIT(idx));
+ return 0;
+}
+
+static int ingenic_tcu_cevt_set_next(unsigned long next,
+ struct clock_event_device *evt)
+{
+ struct ingenic_clock_event_device *jzcevt = ingenic_cevt(evt);
+ struct ingenic_tcu_channel *channel = jzcevt->channel;
+ struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
+ unsigned int idx = channel->idx;
+
+ if (next > 0xffff)
+ return -EINVAL;
+
+ regmap_write(tcu->map, REG_TDFRc(idx), (unsigned int) next);
+ regmap_write(tcu->map, REG_TCNTc(idx), 0);
+ regmap_write(tcu->map, REG_TESR, BIT(idx));
+
+ return 0;
+}
+
+static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id)
+{
+ struct clock_event_device *cevt = dev_id;
+ struct ingenic_clock_event_device *jzcevt = ingenic_cevt(cevt);
+ struct ingenic_tcu_channel *channel = jzcevt->channel;
+ struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
+ unsigned int idx = channel->idx;
+
+ regmap_write(tcu->map, REG_TECR, BIT(idx));
+
+ if (cevt->event_handler)
+ cevt->event_handler(cevt);
+
+ return IRQ_HANDLED;
+}
+
+static int __init ingenic_tcu_req_channel(struct ingenic_tcu_channel *channel)
+{
+ struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
+ char buf[16];
+ int err;
+
+ if (test_and_set_bit(channel->idx, &tcu->requested))
+ return -EBUSY;
+
+ snprintf(buf, sizeof(buf), "timer%u", channel->idx);
+ channel->clk = clk_get(NULL, buf);
+ if (IS_ERR(channel->clk)) {
+ err = PTR_ERR(channel->clk);
+ goto out_release;
+ }
+
+ err = clk_prepare_enable(channel->clk);
+ if (err)
+ goto out_clk_put;
+
+ return 0;
+
+out_clk_put:
+ clk_put(channel->clk);
+out_release:
+ clear_bit(channel->idx, &tcu->requested);
+ return err;
+}
+
+static int __init ingenic_tcu_reset_channel(struct device_node *np,
+ struct ingenic_tcu_channel *channel)
+{
+ struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
+
+ return regmap_update_bits(tcu->map, REG_TCSRc(channel->idx),
+ 0xffff & ~TCSR_RESERVED_BITS, 0);
+}
+
+static void __init ingenic_tcu_free_channel(struct ingenic_tcu_channel *channel)
+{
+ struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
+
+ clk_disable_unprepare(channel->clk);
+ clk_put(channel->clk);
+ clear_bit(channel->idx, &tcu->requested);
+}
+
+static const char * const ingenic_tcu_timer_names[] = {
+ "TCU0", "TCU1", "TCU2", "TCU3", "TCU4", "TCU5", "TCU6", "TCU7",
+};
+
+static int __init ingenic_tcu_setup_cevt(struct device_node *np,
+ struct ingenic_tcu *tcu, unsigned int idx)
+{
+ struct ingenic_tcu_channel *channel = &tcu->channels[idx];
+ struct ingenic_clock_event_device *jzcevt;
+ unsigned long rate;
+ int err, virq;
+
+ err = ingenic_tcu_req_channel(channel);
+ if (err)
+ return err;
+
+ err = ingenic_tcu_reset_channel(np, channel);
+ if (err)
+ goto err_out_free_channel;
+
+ rate = clk_get_rate(channel->clk);
+ if (!rate) {
+ err = -EINVAL;
+ goto err_out_free_channel;
+ }
+
+ jzcevt = kzalloc(sizeof(*jzcevt), GFP_KERNEL);
+ if (!jzcevt) {
+ err = -ENOMEM;
+ goto err_out_free_channel;
+ }
+
+ virq = irq_of_parse_and_map(np, idx);
+ if (!virq) {
+ err = -EINVAL;
+ goto err_out_kfree_jzcevt;
+ }
+
+ err = request_irq(virq, ingenic_tcu_cevt_cb, IRQF_TIMER,
+ ingenic_tcu_timer_names[idx], &jzcevt->cevt);
+ if (err)
+ goto err_out_irq_dispose_mapping;
+
+ jzcevt->channel = channel;
+ snprintf(jzcevt->name, sizeof(jzcevt->name), "ingenic-tcu-chan%u",
+ channel->idx);
+
+ jzcevt->cevt.cpumask = cpumask_of(smp_processor_id());
+ jzcevt->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
+ jzcevt->cevt.name = jzcevt->name;
+ jzcevt->cevt.rating = 200;
+ jzcevt->cevt.set_state_shutdown = ingenic_tcu_cevt_set_state_shutdown;
+ jzcevt->cevt.set_next_event = ingenic_tcu_cevt_set_next;
+
+ clockevents_config_and_register(&jzcevt->cevt, rate, 10, (1 << 16) - 1);
+
+ return 0;
+
+err_out_irq_dispose_mapping:
+ irq_dispose_mapping(virq);
+err_out_kfree_jzcevt:
+ kfree(jzcevt);
+err_out_free_channel:
+ ingenic_tcu_free_channel(channel);
+ return err;
+}
+
+static int __init ingenic_tcu_init(struct device_node *np)
+{
+ struct ingenic_tcu *tcu;
+ unsigned int i;
+ int err, num_timers;
+
+ num_timers = of_property_count_elems_of_size(np, "ingenic,channels", 4);
+ if (num_timers < 0) {
+ pr_err("timer-ingenic: Unable to read DTS node");
+ return num_timers;
+ }
+
+ tcu = kzalloc(sizeof(*tcu), GFP_KERNEL);
+ if (!tcu)
+ return -ENOMEM;
+
+ tcu->map = syscon_node_to_regmap(np->parent);
+ if (IS_ERR(tcu->map)) {
+ err = PTR_ERR(tcu->map);
+ kfree(tcu);
+ return err;
+ }
+
+ for (i = 0; i < NUM_CHANNELS; i++)
+ tcu->channels[i].idx = i;
+
+ for (i = 0; i < (unsigned int) num_timers; i++) {
+ u32 channel;
+
+ of_property_read_u32_index(np, "ingenic,channels", i, &channel);
+
+ if (channel > NUM_CHANNELS) {
+ pr_warn("timer-ingenic: requested TCU channel %u does not exist\n",
+ channel);
+ continue;
+ }
+
+ err = ingenic_tcu_setup_cevt(np, tcu, channel);
+ if (err) {
+ pr_warn("timer-ingenic: Unable to init TCU channel %u: %i",
+ channel, err);
+ continue;
+ }
+ }
+
+ return 0;
+}
+
+CLOCKSOURCE_OF_DECLARE(jz4740_tcu, "ingenic,jz4740-tcu", ingenic_tcu_init);
+CLOCKSOURCE_OF_DECLARE(jz4770_tcu, "ingenic,jz4770-tcu", ingenic_tcu_init);
+CLOCKSOURCE_OF_DECLARE(jz4780_tcu, "ingenic,jz4780-tcu", ingenic_tcu_init);
--
2.11.0
^ permalink raw reply related
* [PATCH 4/6] clk: ingenic: Add JZ47xx TCU clocks driver
From: Paul Cercueil @ 2017-12-29 12:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Daniel Lezcano,
Lee Jones
Cc: linux-clk, linux-kernel, devicetree, Paul Cercueil
In-Reply-To: <20171229125942.27305-1-paul@crapouillou.net>
The TCU (Timer Counter Unit) of the Ingenic JZ47xx SoCs features 8
channels, each one having its own clock, that can be started and
stopped, reparented, and reclocked.
This driver only modifies the bits of the registers of the TCU that are
related to clocks control. It provides one clock per TCU channel (plus
one for the watchdog and one for the OS timer) that can be used by other
drivers.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
.../bindings/clock/ingenic,tcu-clocks.txt | 35 +++
drivers/clk/ingenic/Makefile | 2 +-
drivers/clk/ingenic/tcu.c | 341 +++++++++++++++++++++
3 files changed, 377 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/clock/ingenic,tcu-clocks.txt
create mode 100644 drivers/clk/ingenic/tcu.c
diff --git a/Documentation/devicetree/bindings/clock/ingenic,tcu-clocks.txt b/Documentation/devicetree/bindings/clock/ingenic,tcu-clocks.txt
new file mode 100644
index 000000000000..ed1468a6aa27
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ingenic,tcu-clocks.txt
@@ -0,0 +1,35 @@
+Ingenic SoC TCU binding
+
+The TCU is the Timer/Counter Unit present in all Ingenic SoCs. It features 8
+channels, each one having its own clock, that can be started and stopped,
+reparented, and reclocked.
+
+Required properties:
+- compatible : One of:
+ * ingenic,jz4740-tcu-clocks,
+ * ingenic,jz4770-tcu-clocks,
+ * ingenic,jz4780-tcu-clocks.
+- clocks : List of phandle & clock specifiers for clocks external to the TCU.
+ The "pclk", "rtc" and "ext" clocks should be provided.
+- clock-names : List of name strings for the external clocks.
+- #clock-cells: Should be 1.
+ Clock consumers specify this argument to identify a clock. The valid values
+ may be found in <dt-bindings/clock/ingenic,tcu.h>.
+
+Example:
+
+/ {
+ regmap {
+ compatible = "simple-mfd", "syscon";
+ reg = <0x10002000 0x1000>;
+
+ tcu: clocks {
+ compatible = "ingenic,jz4740-tcu-clocks";
+
+ clocks = <&ext>, <&rtc>, <&pclk>;
+ clock-names = "ext", "rtc", "pclk";
+
+ #clock-cells = <1>;
+ };
+ };
+};
diff --git a/drivers/clk/ingenic/Makefile b/drivers/clk/ingenic/Makefile
index cd47b0664c2b..e373118a3726 100644
--- a/drivers/clk/ingenic/Makefile
+++ b/drivers/clk/ingenic/Makefile
@@ -1,3 +1,3 @@
-obj-y += cgu.o
+obj-y += cgu.o tcu.o
obj-$(CONFIG_MACH_JZ4740) += jz4740-cgu.o
obj-$(CONFIG_MACH_JZ4780) += jz4780-cgu.o
diff --git a/drivers/clk/ingenic/tcu.c b/drivers/clk/ingenic/tcu.c
new file mode 100644
index 000000000000..a1b189984d17
--- /dev/null
+++ b/drivers/clk/ingenic/tcu.c
@@ -0,0 +1,341 @@
+/*
+ * Ingenic JZ47xx SoC TCU clocks driver
+ *
+ * Copyright (C) 2016 Paul Cercueil <paul@crapouillou.net>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mfd/syscon/ingenic-tcu.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/spinlock.h>
+
+#include <dt-bindings/clock/ingenic,tcu.h>
+
+enum ingenic_version {
+ ID_JZ4740,
+ ID_JZ4770,
+ ID_JZ4780,
+};
+
+struct ingenic_tcu {
+ struct device_node *np;
+ struct regmap *map;
+
+ struct clk_onecell_data clocks;
+};
+
+struct ingenic_tcu_clk_info {
+ struct clk_init_data init_data;
+ u8 gate_bit;
+ u8 tcsr_reg;
+};
+
+struct ingenic_tcu_clk {
+ struct clk_hw hw;
+
+ struct ingenic_tcu *tcu;
+ const struct ingenic_tcu_clk_info *info;
+
+ unsigned int idx;
+};
+
+#define to_tcu_clk(_hw) container_of(_hw, struct ingenic_tcu_clk, hw)
+
+static int ingenic_tcu_enable(struct clk_hw *hw)
+{
+ struct ingenic_tcu_clk *tcu_clk = to_tcu_clk(hw);
+ const struct ingenic_tcu_clk_info *info = tcu_clk->info;
+ struct ingenic_tcu *tcu = tcu_clk->tcu;
+
+ regmap_write(tcu->map, REG_TSCR, BIT(info->gate_bit));
+ return 0;
+}
+
+static void ingenic_tcu_disable(struct clk_hw *hw)
+{
+ struct ingenic_tcu_clk *tcu_clk = to_tcu_clk(hw);
+ struct ingenic_tcu *tcu = tcu_clk->tcu;
+ const struct ingenic_tcu_clk_info *info = tcu_clk->info;
+
+ regmap_write(tcu->map, REG_TSSR, BIT(info->gate_bit));
+}
+
+static int ingenic_tcu_is_enabled(struct clk_hw *hw)
+{
+ struct ingenic_tcu_clk *tcu_clk = to_tcu_clk(hw);
+ struct ingenic_tcu *tcu = tcu_clk->tcu;
+ const struct ingenic_tcu_clk_info *info = tcu_clk->info;
+ unsigned int value;
+
+ regmap_read(tcu->map, REG_TSR, &value);
+
+ return !(value & BIT(info->gate_bit));
+}
+
+static u8 ingenic_tcu_get_parent(struct clk_hw *hw)
+{
+ struct ingenic_tcu_clk *tcu_clk = to_tcu_clk(hw);
+ struct ingenic_tcu *tcu = tcu_clk->tcu;
+ const struct ingenic_tcu_clk_info *info = tcu_clk->info;
+ unsigned int val = 0;
+ int ret;
+
+ ret = regmap_read(tcu->map, info->tcsr_reg, &val);
+ WARN_ONCE(ret < 0, "Unable to read TCSR %i", tcu_clk->idx);
+
+ return (u8) ffs(val & TCSR_PARENT_CLOCK_MASK) - 1;
+}
+
+static int ingenic_tcu_set_parent(struct clk_hw *hw, u8 idx)
+{
+ struct ingenic_tcu_clk *tcu_clk = to_tcu_clk(hw);
+ struct ingenic_tcu *tcu = tcu_clk->tcu;
+ const struct ingenic_tcu_clk_info *info = tcu_clk->info;
+ int ret;
+
+ /*
+ * Our clock provider has the CLK_SET_PARENT_GATE flag set, so we know
+ * that the clk is in unprepared state. To be able to access TCSR
+ * we must ungate the clock supply and we gate it again when done.
+ */
+
+ regmap_write(tcu->map, REG_TSCR, BIT(info->gate_bit));
+
+ ret = regmap_update_bits(tcu->map, info->tcsr_reg,
+ TCSR_PARENT_CLOCK_MASK, BIT(idx));
+ WARN_ONCE(ret < 0, "Unable to update TCSR %i", tcu_clk->idx);
+
+ regmap_write(tcu->map, REG_TSSR, BIT(info->gate_bit));
+
+ return 0;
+}
+
+static unsigned long ingenic_tcu_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct ingenic_tcu_clk *tcu_clk = to_tcu_clk(hw);
+ struct ingenic_tcu *tcu = tcu_clk->tcu;
+ const struct ingenic_tcu_clk_info *info = tcu_clk->info;
+ unsigned int prescale;
+ int ret;
+
+ ret = regmap_read(tcu->map, info->tcsr_reg, &prescale);
+ WARN_ONCE(ret < 0, "Unable to read TCSR %i", tcu_clk->idx);
+
+ prescale = (prescale & TCSR_PRESCALE_MASK) >> TCSR_PRESCALE_LSB;
+
+ return parent_rate >> (prescale * 2);
+}
+
+static long ingenic_tcu_round_rate(struct clk_hw *hw, unsigned long req_rate,
+ unsigned long *parent_rate)
+{
+ long rate = (long) *parent_rate;
+ unsigned int shift;
+
+ if (req_rate > rate)
+ return -EINVAL;
+
+ for (shift = 0; shift < 10; shift += 2)
+ if ((rate >> shift) <= req_rate)
+ return rate >> shift;
+
+ return rate >> 10;
+}
+
+static int ingenic_tcu_set_rate(struct clk_hw *hw, unsigned long req_rate,
+ unsigned long parent_rate)
+{
+ struct ingenic_tcu_clk *tcu_clk = to_tcu_clk(hw);
+ const struct ingenic_tcu_clk_info *info = tcu_clk->info;
+ struct ingenic_tcu *tcu = tcu_clk->tcu;
+ u8 prescale = (ffs(parent_rate / req_rate) / 2) << TCSR_PRESCALE_LSB;
+ int ret;
+
+ /*
+ * Our clock provider has the CLK_SET_RATE_GATE flag set, so we know
+ * that the clk is in unprepared state. To be able to access TCSR
+ * we must ungate the clock supply and we gate it again when done.
+ */
+
+ regmap_write(tcu->map, REG_TSCR, BIT(info->gate_bit));
+
+ ret = regmap_update_bits(tcu->map, info->tcsr_reg,
+ TCSR_PRESCALE_MASK, prescale);
+ WARN_ONCE(ret < 0, "Unable to update TCSR %i", tcu_clk->idx);
+
+ regmap_write(tcu->map, REG_TSSR, BIT(info->gate_bit));
+
+ return 0;
+}
+
+static const struct clk_ops ingenic_tcu_clk_ops = {
+ .get_parent = ingenic_tcu_get_parent,
+ .set_parent = ingenic_tcu_set_parent,
+
+ .recalc_rate = ingenic_tcu_recalc_rate,
+ .round_rate = ingenic_tcu_round_rate,
+ .set_rate = ingenic_tcu_set_rate,
+
+ .enable = ingenic_tcu_enable,
+ .disable = ingenic_tcu_disable,
+ .is_enabled = ingenic_tcu_is_enabled,
+};
+
+static const char * const ingenic_tcu_timer_parents[] = {
+ "pclk",
+ "rtc",
+ "ext",
+};
+
+static const struct ingenic_tcu_clk_info ingenic_tcu_clk_info[] = {
+#define DEF_TIMER(_name, _gate_bit, _tcsr) \
+ { \
+ .init_data = { \
+ .name = _name, \
+ .parent_names = ingenic_tcu_timer_parents, \
+ .num_parents = ARRAY_SIZE(ingenic_tcu_timer_parents),\
+ .ops = &ingenic_tcu_clk_ops, \
+ .flags = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE,\
+ }, \
+ .gate_bit = _gate_bit, \
+ .tcsr_reg = _tcsr, \
+ }
+ [JZ4740_CLK_TIMER0] = DEF_TIMER("timer0", 0, REG_TCSRc(0)),
+ [JZ4740_CLK_TIMER1] = DEF_TIMER("timer1", 1, REG_TCSRc(1)),
+ [JZ4740_CLK_TIMER2] = DEF_TIMER("timer2", 2, REG_TCSRc(2)),
+ [JZ4740_CLK_TIMER3] = DEF_TIMER("timer3", 3, REG_TCSRc(3)),
+ [JZ4740_CLK_TIMER4] = DEF_TIMER("timer4", 4, REG_TCSRc(4)),
+ [JZ4740_CLK_TIMER5] = DEF_TIMER("timer5", 5, REG_TCSRc(5)),
+ [JZ4740_CLK_TIMER6] = DEF_TIMER("timer6", 6, REG_TCSRc(6)),
+ [JZ4740_CLK_TIMER7] = DEF_TIMER("timer7", 7, REG_TCSRc(7)),
+ [JZ4740_CLK_WDT] = DEF_TIMER("wdt", 16, REG_WDT_TCSR),
+ [JZ4770_CLK_OST] = DEF_TIMER("ost", 15, REG_OST_TCSR),
+#undef DEF_TIMER
+};
+
+static int ingenic_tcu_register_clock(struct ingenic_tcu *tcu, unsigned int idx,
+ const struct ingenic_tcu_clk_info *info)
+{
+ struct ingenic_tcu_clk *tcu_clk;
+ struct clk *clk;
+ int err;
+
+ tcu_clk = kzalloc(sizeof(*tcu_clk), GFP_KERNEL);
+ if (!tcu_clk)
+ return -ENOMEM;
+
+ tcu_clk->hw.init = &info->init_data;
+ tcu_clk->idx = idx;
+ tcu_clk->info = info;
+ tcu_clk->tcu = tcu;
+
+ /* Set EXT as the default parent clock */
+ ingenic_tcu_set_parent(&tcu_clk->hw, 2);
+
+ ingenic_tcu_disable(&tcu_clk->hw);
+
+ clk = clk_register(NULL, &tcu_clk->hw);
+ if (IS_ERR(clk)) {
+ err = PTR_ERR(clk);
+ goto err_free_tcu_clk;
+ }
+
+ err = clk_register_clkdev(clk, info->init_data.name, NULL);
+ if (err)
+ goto err_clk_unregister;
+
+ tcu->clocks.clks[idx] = clk;
+ return 0;
+
+err_clk_unregister:
+ clk_unregister(clk);
+err_free_tcu_clk:
+ kfree(tcu_clk);
+ return err;
+}
+
+static void __init ingenic_tcu_init(struct device_node *np,
+ enum ingenic_version id)
+{
+ struct ingenic_tcu *tcu;
+ size_t i, nb_clks;
+ int ret = -ENOMEM;
+
+ if (id >= ID_JZ4770)
+ nb_clks = (JZ4770_CLK_LAST - JZ4740_CLK_TIMER0) + 1;
+ else
+ nb_clks = (JZ4740_CLK_LAST - JZ4740_CLK_TIMER0) + 1;
+
+ tcu = kzalloc(sizeof(*tcu), GFP_KERNEL);
+ if (!tcu) {
+ pr_err("%s: cannot allocate memory\n", __func__);
+ return;
+ }
+
+ tcu->map = syscon_node_to_regmap(np->parent);
+ if (IS_ERR(tcu->map)) {
+ pr_err("%s: failed to map TCU registers\n", __func__);
+ goto err_free_tcu;
+ }
+
+ tcu->clocks.clk_num = nb_clks;
+ tcu->clocks.clks = kcalloc(nb_clocks, sizeof(struct clk *), GFP_KERNEL);
+ if (!tcu->clocks.clks) {
+ pr_err("%s: cannot allocate memory\n", __func__);
+ goto err_free_tcu;
+ }
+
+ for (i = 0; i < nb_clks; i++) {
+ ret = ingenic_tcu_register_clock(tcu, i,
+ &ingenic_tcu_clk_info[JZ4740_CLK_TIMER0 + i]);
+ if (ret) {
+ pr_err("%s: cannot register clocks\n", __func__);
+ goto err_unregister;
+ }
+ }
+
+ ret = of_clk_add_provider(np, of_clk_src_onecell_get, &tcu->clocks);
+ if (ret) {
+ pr_err("%s: cannot add OF clock provider\n", __func__);
+ goto err_unregister;
+ }
+
+ return;
+
+err_unregister:
+ for (i = 0; i < tcu->clocks.clk_num; i++)
+ if (tcu->clocks.clks[i])
+ clk_unregister(tcu->clocks.clks[i]);
+ kfree(tcu->clocks.clks);
+err_free_tcu:
+ kfree(tcu);
+}
+
+static void __init jz4740_tcu_init(struct device_node *np)
+{
+ ingenic_tcu_init(np, ID_JZ4740);
+}
+CLK_OF_DECLARE(ingenic_tcu, "ingenic,jz4740-tcu-clocks", jz4740_tcu_init);
+
+static void __init jz4770_tcu_init(struct device_node *np)
+{
+ ingenic_tcu_init(np, ID_JZ4770);
+}
+CLK_OF_DECLARE(jz4770_tcu, "ingenic,jz4770-tcu-clocks", jz4770_tcu_init);
+
+static void __init jz4780_tcu_init(struct device_node *np)
+{
+ ingenic_tcu_init(np, ID_JZ4780);
+}
+CLK_OF_DECLARE(jz4780_tcu, "ingenic,jz4780-tcu-clocks", jz4780_tcu_init);
--
2.11.0
^ permalink raw reply related
* [PATCH 3/6] irqchip: Add the ingenic-tcu-intc driver
From: Paul Cercueil @ 2017-12-29 12:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Daniel Lezcano,
Lee Jones
Cc: linux-clk, linux-kernel, devicetree, Paul Cercueil
In-Reply-To: <20171229125942.27305-1-paul@crapouillou.net>
This simple driver handles the IRQ chip of the TCU
(Timer Counter Unit) of the JZ47xx Ingenic SoCs.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
.../bindings/interrupt-controller/ingenic,tcu.txt | 32 +++++
drivers/irqchip/Kconfig | 5 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-ingenic-tcu.c | 156 +++++++++++++++++++++
4 files changed, 194 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ingenic,tcu.txt
create mode 100644 drivers/irqchip/irq-ingenic-tcu.c
diff --git a/Documentation/devicetree/bindings/interrupt-controller/ingenic,tcu.txt b/Documentation/devicetree/bindings/interrupt-controller/ingenic,tcu.txt
new file mode 100644
index 000000000000..a3e6318f8461
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/ingenic,tcu.txt
@@ -0,0 +1,32 @@
+Ingenic SoCs Timer/Counter Unit Interrupt Controller
+
+Required properties:
+
+- compatible : should be "ingenic,<socname>-tcu-intc". Valid strings are:
+ * ingenic,jz4740-tcu-intc
+ * ingenic,jz4770-tcu-intc
+ * ingenic,jz4780-tcu-intc
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt source. The value shall be 1.
+- interrupt-parent : phandle of the interrupt controller.
+- interrupts : Specifies the interrupt the controller is connected to.
+
+Example:
+
+/ {
+ regmap {
+ compatible = "simple-mfd", "syscon";
+ reg = <0x10002000 0x1000>;
+
+ tcu: interrupt-controller {
+ compatible = "ingenic,jz4740-tcu-intc";
+
+ interrupt-controller;
+ #interrupt-cells = <1>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <15>;
+ };
+ };
+};
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index c70476b34a53..d3d0ce7c554a 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -267,6 +267,11 @@ config INGENIC_IRQ
depends on MACH_INGENIC
default y
+config INGENIC_TCU_IRQ
+ bool
+ depends on MACH_INGENIC || COMPILE_TEST
+ default y
+
config RENESAS_H8300H_INTC
bool
select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index d2df34a54d38..b8070ee21d8a 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_RENESAS_H8300H_INTC) += irq-renesas-h8300h.o
obj-$(CONFIG_RENESAS_H8S_INTC) += irq-renesas-h8s.o
obj-$(CONFIG_ARCH_SA1100) += irq-sa11x0.o
obj-$(CONFIG_INGENIC_IRQ) += irq-ingenic.o
+obj-$(CONFIG_INGENIC_TCU_IRQ) += irq-ingenic-tcu.o
obj-$(CONFIG_IMX_GPCV2) += irq-imx-gpcv2.o
obj-$(CONFIG_PIC32_EVIC) += irq-pic32-evic.o
obj-$(CONFIG_MVEBU_GICP) += irq-mvebu-gicp.o
diff --git a/drivers/irqchip/irq-ingenic-tcu.c b/drivers/irqchip/irq-ingenic-tcu.c
new file mode 100644
index 000000000000..56630349d253
--- /dev/null
+++ b/drivers/irqchip/irq-ingenic-tcu.c
@@ -0,0 +1,156 @@
+/*
+ * JZ4740 SoC TCU IRQ driver
+ *
+ * Copyright (C) 2016 Paul Cercueil <paul@crapouillou.net>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/bitops.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/syscon/ingenic-tcu.h>
+
+static void ingenic_tcu_intc_cascade(struct irq_desc *desc)
+{
+ struct irq_chip *irq_chip = irq_data_get_irq_chip(&desc->irq_data);
+ struct irq_domain *domain = irq_desc_get_handler_data(desc);
+ struct irq_chip_generic *gc = irq_get_domain_generic_chip(domain, 0);
+ struct regmap *map = gc->private;
+ uint32_t irq_reg, irq_mask;
+ unsigned int i;
+
+ regmap_read(map, REG_TFR, &irq_reg);
+ regmap_read(map, REG_TMR, &irq_mask);
+
+ chained_irq_enter(irq_chip, desc);
+
+ irq_reg &= ~irq_mask;
+
+ for (i = 0; i < 32; i++) {
+ if (irq_reg & BIT(i))
+ generic_handle_irq(irq_linear_revmap(domain, i));
+ }
+
+ chained_irq_exit(irq_chip, desc);
+}
+
+static void ingenic_tcu_gc_unmask_enable_reg(struct irq_data *d)
+{
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
+ struct regmap *map = gc->private;
+ u32 mask = d->mask;
+
+ irq_gc_lock(gc);
+ regmap_write(map, ct->regs.ack, mask);
+ regmap_write(map, ct->regs.enable, mask);
+ *ct->mask_cache |= mask;
+ irq_gc_unlock(gc);
+}
+
+static void ingenic_tcu_gc_mask_disable_reg(struct irq_data *d)
+{
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
+ struct regmap *map = gc->private;
+ u32 mask = d->mask;
+
+ irq_gc_lock(gc);
+ regmap_write(map, ct->regs.disable, mask);
+ *ct->mask_cache &= ~mask;
+ irq_gc_unlock(gc);
+}
+
+static void ingenic_tcu_gc_mask_disable_reg_and_ack(struct irq_data *d)
+{
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
+ struct regmap *map = gc->private;
+ u32 mask = d->mask;
+
+ irq_gc_lock(gc);
+ regmap_write(map, ct->regs.ack, mask);
+ regmap_write(map, ct->regs.disable, mask);
+ irq_gc_unlock(gc);
+}
+
+static int __init ingenic_tcu_intc_of_init(struct device_node *node,
+ struct device_node *parent)
+{
+ struct irq_domain *domain;
+ struct irq_chip_generic *gc;
+ struct irq_chip_type *ct;
+ int err, i, num_parent_irqs;
+ unsigned int parent_irqs[3];
+ struct regmap *map;
+
+ num_parent_irqs = of_property_count_elems_of_size(
+ node, "interrupts", 4);
+ if (num_parent_irqs < 0 || num_parent_irqs > ARRAY_SIZE(parent_irqs))
+ return -EINVAL;
+
+ map = syscon_node_to_regmap(node->parent);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ domain = irq_domain_add_linear(node, 32, &irq_generic_chip_ops, NULL);
+ if (!domain)
+ return -ENOMEM;
+
+ err = irq_alloc_domain_generic_chips(domain, 32, 1, "TCU",
+ handle_level_irq, 0, IRQ_NOPROBE | IRQ_LEVEL, 0);
+ if (err)
+ goto out_domain_remove;
+
+ gc = irq_get_domain_generic_chip(domain, 0);
+ ct = gc->chip_types;
+
+ gc->wake_enabled = IRQ_MSK(32);
+ gc->private = map;
+
+ ct->regs.disable = REG_TMSR;
+ ct->regs.enable = REG_TMCR;
+ ct->regs.ack = REG_TFCR;
+ ct->chip.irq_unmask = ingenic_tcu_gc_unmask_enable_reg;
+ ct->chip.irq_mask = ingenic_tcu_gc_mask_disable_reg;
+ ct->chip.irq_mask_ack = ingenic_tcu_gc_mask_disable_reg_and_ack;
+ ct->chip.flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SKIP_SET_WAKE;
+
+ /* Mask all IRQs by default */
+ regmap_write(map, REG_TMSR, IRQ_MSK(32));
+
+ for (i = 0; i < num_parent_irqs; i++) {
+ parent_irqs[i] = irq_of_parse_and_map(node, i);
+ if (!parent_irqs[i]) {
+ err = -EINVAL;
+ goto out_unmap_irqs;
+ }
+
+ irq_set_chained_handler_and_data(parent_irqs[i],
+ ingenic_tcu_intc_cascade, domain);
+ }
+
+ return 0;
+
+out_unmap_irqs:
+ for (; i > 0; i--)
+ irq_dispose_mapping(parent_irqs[i - 1]);
+out_domain_remove:
+ irq_domain_remove(domain);
+ return err;
+}
+IRQCHIP_DECLARE(jz4740_tcu_intc, "ingenic,jz4740-tcu-intc",
+ ingenic_tcu_intc_of_init);
+IRQCHIP_DECLARE(jz4770_tcu_intc, "ingenic,jz4770-tcu-intc",
+ ingenic_tcu_intc_of_init);
+IRQCHIP_DECLARE(jz4780_tcu_intc, "ingenic,jz4780-tcu-intc",
+ ingenic_tcu_intc_of_init);
--
2.11.0
^ permalink raw reply related
* [PATCH 2/6] dt-bindings: ingenic: Add DT bindings for TCU clocks
From: Paul Cercueil @ 2017-12-29 12:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Daniel Lezcano,
Lee Jones
Cc: linux-clk, linux-kernel, devicetree, Paul Cercueil
In-Reply-To: <20171229125942.27305-1-paul@crapouillou.net>
This header provides clock numbers for the ingenic,tcu
DT binding.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
include/dt-bindings/clock/ingenic,tcu.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 include/dt-bindings/clock/ingenic,tcu.h
diff --git a/include/dt-bindings/clock/ingenic,tcu.h b/include/dt-bindings/clock/ingenic,tcu.h
new file mode 100644
index 000000000000..3d2a6552fcf1
--- /dev/null
+++ b/include/dt-bindings/clock/ingenic,tcu.h
@@ -0,0 +1,22 @@
+/*
+ * This header provides clock numbers for the ingenic,tcu DT binding.
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_INGENIC_TCU_H__
+#define __DT_BINDINGS_CLOCK_INGENIC_TCU_H__
+
+#define JZ4740_CLK_TIMER0 0
+#define JZ4740_CLK_TIMER1 1
+#define JZ4740_CLK_TIMER2 2
+#define JZ4740_CLK_TIMER3 3
+#define JZ4740_CLK_TIMER4 4
+#define JZ4740_CLK_TIMER5 5
+#define JZ4740_CLK_TIMER6 6
+#define JZ4740_CLK_TIMER7 7
+#define JZ4740_CLK_WDT 8
+#define JZ4740_CLK_LAST JZ4740_CLK_WDT
+
+#define JZ4770_CLK_OST 9
+#define JZ4770_CLK_LAST JZ4770_CLK_OST
+
+#endif /* __DT_BINDINGS_CLOCK_INGENIC_TCU_H__ */
--
2.11.0
^ permalink raw reply related
* [PATCH 1/6] mfd: syscon: Add ingenic-tcu.h header
From: Paul Cercueil @ 2017-12-29 12:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Daniel Lezcano,
Lee Jones
Cc: linux-clk, linux-kernel, devicetree, Paul Cercueil
In-Reply-To: <20171229125942.27305-1-paul@crapouillou.net>
This header contains macros for the registers that are present in the
regmap shared by all the drivers related to the TCU (Timer Counter Unit)
of the Ingenic JZ47xx SoCs.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
include/linux/mfd/syscon/ingenic-tcu.h | 53 ++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100644 include/linux/mfd/syscon/ingenic-tcu.h
diff --git a/include/linux/mfd/syscon/ingenic-tcu.h b/include/linux/mfd/syscon/ingenic-tcu.h
new file mode 100644
index 000000000000..5952fdd60005
--- /dev/null
+++ b/include/linux/mfd/syscon/ingenic-tcu.h
@@ -0,0 +1,53 @@
+#ifndef __LINUX_CLK_INGENIC_TCU_H_
+#define __LINUX_CLK_INGENIC_TCU_H_
+
+#include <linux/bitops.h>
+#include <linux/regmap.h>
+
+enum ingenic_tcu_reg {
+ REG_WDT_TDR = 0x00,
+ REG_WDT_TCER = 0x04,
+ REG_WDT_TCNT = 0x08,
+ REG_WDT_TCSR = 0x0c,
+ REG_TER = 0x10,
+ REG_TESR = 0x14,
+ REG_TECR = 0x18,
+ REG_TSR = 0x1c,
+ REG_TFR = 0x20,
+ REG_TFSR = 0x24,
+ REG_TFCR = 0x28,
+ REG_TSSR = 0x2c,
+ REG_TMR = 0x30,
+ REG_TMSR = 0x34,
+ REG_TMCR = 0x38,
+ REG_TSCR = 0x3c,
+ REG_TDFR0 = 0x40,
+ REG_TDHR0 = 0x44,
+ REG_TCNT0 = 0x48,
+ REG_TCSR0 = 0x4c,
+ REG_OST_DR = 0xe0,
+ REG_OST_CNTL = 0xe4,
+ REG_OST_CNTH = 0xe8,
+ REG_OST_TCSR = 0xec,
+ REG_TSTR = 0xf0,
+ REG_TSTSR = 0xf4,
+ REG_TSTCR = 0xf8,
+ REG_OST_CNTHBUF = 0xfc,
+};
+
+#define TCSR_RESERVED_BITS 0x3f
+#define TCSR_PARENT_CLOCK_MASK 0x07
+#define TCSR_PRESCALE_LSB 3
+#define TCSR_PRESCALE_MASK 0x38
+
+#define TCSR_PWM_SD BIT(9) /* 0: Shutdown abruptly 1: gracefully */
+#define TCSR_PWM_INITL_HIGH BIT(8) /* Sets the initial output level */
+#define TCSR_PWM_EN BIT(7) /* PWM pin output enable */
+
+#define CHANNEL_STRIDE 0x10
+#define REG_TDFRc(c) (REG_TDFR0 + ((c) * CHANNEL_STRIDE))
+#define REG_TDHRc(c) (REG_TDHR0 + ((c) * CHANNEL_STRIDE))
+#define REG_TCNTc(c) (REG_TCNT0 + ((c) * CHANNEL_STRIDE))
+#define REG_TCSRc(c) (REG_TCSR0 + ((c) * CHANNEL_STRIDE))
+
+#endif /* __LINUX_CLK_INGENIC_TCU_H_ */
--
2.11.0
^ permalink raw reply related
* [PATCH 0/6] Ingenic JZ47xx TCU drivers
From: Paul Cercueil @ 2017-12-29 12:59 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Daniel Lezcano,
Lee Jones
Cc: linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Hi,
This patch set introduces three new drivers, which together control the
TCU (Timer/Counter Unit) of Ingenic JZ47xx SoCs.
The ingenic-tcu-intc driver will demultiplex the TCU IRQ interrupts; the
ingenic-tcu-clocks driver will handle the TCU clocks (enable/disable,
reparenting, reclocking); and the ingenic-tcu driver provides a
clocksource and timers to the system.
The purpose of this patch series is to eventually completely remove the
non-standard platform and board code that handles timers for the Ingenic
SoCs. Before this can be achieved, the watchdog and PWM drivers (already
upstream) will be modified to use the same regmap as the rest of the TCU
drivers.
Thanks,
-Paul
--
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 06/15] clk: Add Ingenic jz4770 CGU driver
From: Philippe Ombredanne @ 2017-12-29 12:55 UTC (permalink / raw)
To: Paul Cercueil
Cc: Ralf Baechle, Rob Herring, Michael Turquette, Mark Rutland,
Stephen Boyd, Maarten ter Huurne,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
Linux MIPS, linux-clk-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171228135634.30000-7-paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org>
Dear Mr Crapouillou-Cercueil-Sir,
On Thu, Dec 28, 2017 at 2:56 PM, Paul Cercueil <paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org> wrote:
> Add support for the clocks provided by the CGU in the Ingenic JZ4770
> SoC.
<snip>
> --- /dev/null
> +++ b/drivers/clk/ingenic/jz4770-cgu.c
> @@ -0,0 +1,487 @@
> +/*
> + * JZ4770 SoC CGU driver
> + *
> + * Copyright 2017, Paul Cercueil <paul-icTtO2rgO2OTuSrc4Mpeew@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 or later
> + * as published by the Free Software Foundation.
> + */
Do you mind using a simpler one-line SPDX identifier instead of this
fine but clearly crapouillish legalese boilerplate? Unless you are
trying to turn the kernel in a legal compendium, of course ;)
This is documented in Thomas doc patches. This would apply to your
entire patch set.
Thank you for your kind consideration!
--
Cordially
Philippe Ombredanne
--
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
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