Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] leds: lm3692x: Introduce LM3692x dual string driver
From: Dan Murphy @ 2017-11-15 13:06 UTC (permalink / raw)
  To: Pavel Machek
  Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, devicetree,
	linux-kernel, linux-leds
In-Reply-To: <20171115130057.GA11557@amd>

Pavel

On 11/15/2017 07:00 AM, Pavel Machek wrote:
> Hi!
> 
>>>> +	ret = regmap_write(led->regmap, LM3692X_PWM_CTRL,
>>>> +		LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ);
>>>> +	if (ret)
>>>> +		dev_err(&led->client->dev, "Failed programming PWM CTRL\n");
>>>> +
>>>> +	ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL,
>>>> +			LM3692X_BRHT_MODE_RAMP_MULTI |
>>>> +			LM3692X_BL_ADJ_POL |
>>>> +			LM3692X_RAMP_RATE_250us);
>>>> +	if (ret)
>>>> +		dev_err(&led->client->dev, "Failed programming BOOST CTRL\n");
>>>> +
>>>> +	ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00);
>>>> +	if (ret)
>>>> +		dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n");
>>>
>>> If something fails, is it wise to continue?
> 
> If something fails, is it wise to continue?
> 

Sorry missed that comment.  It is ok to continue but probably should goto out and return
an error so the probe fails.

Dan

> 									Pavel
> 


-- 
------------------
Dan Murphy

^ permalink raw reply

* [PATCH] dt-bindings: add device tree binding for Arm TrustZone CryptoCell crypto engine
From: Gilad Ben-Yossef @ 2017-11-15 13:05 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Rob Herring, Mark Rutland
  Cc: Ofir Drang, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

The Arm TrustZone CryptoCell is a hardware security engine. This patch
adds DT bindings for its Rich Execution Environment crypto engine.

A driver supporting this device is already present in the staging tree.

Signed-off-by: Gilad Ben-Yossef <gilad-6S/DczAoZh3WXxRugSxzZg@public.gmane.org>
---
 .../devicetree/bindings/crypto/arm-cryptocell.txt  | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/arm-cryptocell.txt

diff --git a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
new file mode 100644
index 0000000..ccf8a101
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
@@ -0,0 +1,22 @@
+Arm TrustZone CryptoCell cryptographic engine
+
+Required properties:
+- compatible: Should be "arm,cryptocell-712-ree".
+- reg: Base physical address of the engine and length of memory mapped region.
+- interrupts: Interrupt number for the device.
+
+Optional properties:
+- interrupt-parent: The phandle for the interrupt controller that services
+  interrupts for this device.
+- clocks: Reference to the crypto engine clock.
+- dma-coherent: Present if dma operations are coherent.
+
+Examples:
+
+       arm_cc712: arm_cc712@80000000 {
+               compatible = "arm,cryptocell-712-ree";
+               interrupt-parent = <&intc>;
+               interrupts = < 0 30 4 >;
+               reg = < 0x80000000 0x10000 >;
+
+       };
-- 
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 2/2] leds: lm3692x: Introduce LM3692x dual string driver
From: Pavel Machek @ 2017-11-15 13:00 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, devicetree,
	linux-kernel, linux-leds
In-Reply-To: <1b0c05a6-2261-85d5-a3c6-9753f7600a20@ti.com>

[-- Attachment #1: Type: text/plain, Size: 903 bytes --]

Hi!

> >> +	ret = regmap_write(led->regmap, LM3692X_PWM_CTRL,
> >> +		LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ);
> >> +	if (ret)
> >> +		dev_err(&led->client->dev, "Failed programming PWM CTRL\n");
> >> +
> >> +	ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL,
> >> +			LM3692X_BRHT_MODE_RAMP_MULTI |
> >> +			LM3692X_BL_ADJ_POL |
> >> +			LM3692X_RAMP_RATE_250us);
> >> +	if (ret)
> >> +		dev_err(&led->client->dev, "Failed programming BOOST CTRL\n");
> >> +
> >> +	ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00);
> >> +	if (ret)
> >> +		dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n");
> > 
> > If something fails, is it wise to continue?

If something fails, is it wise to continue?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* [PATCH] dt-bindings: rtc: imxdi: Improve the bindings text
From: Fabio Estevam @ 2017-11-15 12:59 UTC (permalink / raw)
  To: alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jbe-bIcnvbaLZ9MEGnE8C9+IrQ, Fabio Estevam

From: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>

Improve the bindings text by doing the following changes:

- Remove the i.MX53 reference, as the RTC on i.MX53 is a different hardware
- Add 'clocks' to the list of required properties
- Explain that the optional security violation irq is the second entry
- Use the real unit address and irq numbers for i.MX25

Signed-off-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
---
 Documentation/devicetree/bindings/rtc/imxdi-rtc.txt | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt b/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
index 323cf26..b8f3050 100644
--- a/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
@@ -1,20 +1,20 @@
 * i.MX25 Real Time Clock controller
 
-This binding supports the following chips: i.MX25, i.MX53
-
 Required properties:
 - compatible: should be: "fsl,imx25-rtc"
 - reg: physical base address of the controller and length of memory mapped
   region.
+- clocks: should contain the phandle for the rtc clock
 - interrupts: rtc alarm interrupt
 
 Optional properties:
-- interrupts: dryice security violation interrupt
+- interrupts: dryice security violation interrupt (second entry)
 
 Example:
 
-rtc@80056000 {
-	compatible = "fsl,imx53-rtc", "fsl,imx25-rtc";
-	reg = <0x80056000 2000>;
-	interrupts = <29 56>;
+rtc@53ffc000 {
+	compatible = "fsl,imx25-rtc";
+	reg = <0x53ffc000 0x4000>;
+	clocks = <&clks 81>;
+	interrupts = <25 56>;
 };
-- 
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

* [PATCH v3 4/4] iio: adc: at91-sama5d2_adc: ack DRDY irq in direct mode
From: Eugen Hristev @ 2017-11-15 12:56 UTC (permalink / raw)
  To: nicolas.ferre, alexandre.belloni, linux-iio, lars,
	linux-arm-kernel, devicetree, linux-kernel, ludovic.desroches,
	jic23
  Cc: eugen.hristev
In-Reply-To: <1510750608-8697-1-git-send-email-eugen.hristev@microchip.com>

Need to acknowledge DRDY irq in direct mode/ software
triggered mode. Otherwise, on the next conversion, overrun
flag will be raised, which is not a correct state.
This doesn't affect the functionality, but will generate
possible incorrect overrun reports.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 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 11d34a8..274cb5e 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -787,6 +787,9 @@ 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));
 
+		/* Needed to ACK the DRDY interruption */
+		at91_adc_readl(st, AT91_SAMA5D2_LCDR);
+
 		mutex_unlock(&st->lock);
 
 		iio_device_release_direct_mode(indio_dev);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 3/4] iio: adc: at91-sama5d2_adc: add support for DMA
From: Eugen Hristev @ 2017-11-15 12:56 UTC (permalink / raw)
  To: nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, lars-Qo5EllUWu/uELgA04lAiVw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA,
	jic23-DgEjT+Ai2ygdnm+yROfE0A
  Cc: eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA
In-Reply-To: <1510750608-8697-1-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>

Added support for DMA transfers. The implementation uses the user watermark
to decide whether DMA will be used or not. For watermark 1, DMA will not be
used. If watermark is bigger, DMA will be used.
Sysfs attributes are created to indicate whether the DMA is used,
with hwfifo_enabled, and the current DMA watermark is readable
in hwfifo_watermark. Minimum and maximum values are in hwfifo_watermark_min
and hwfifo_watermark_max.

Signed-off-by: Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
---
 Changes in v3:
 - Remove misleaded dev_info message when DMA was not enabled at probe
 - Rebased patch on top of the
[PATCH] iio: adc: at91-sama5d2_adc: fix probe error on missing trigger property
Which is already upstreamed in 4.14
 - Fixed the bug introduced in v2, with buffer size
 - added extra check when enabling DMA, to have hw trigger present.
This is because now, we can have the driver with software trigger only (if no
hw trigger in device tree, start as software only)

 Changes in v2:
 - No longer add last timestamp to all samples. Now, compute an interval
between samples w.r.t. start and end time of the transfer and number
of samples. Then distribute them each in the time interval.
 - Add warning for conversion overrun. This helps user identify cases
when the watermark needs adjustment : the software is too slow in reading
data from the ADC.
 - Protection around watermark is not needed, changing of the watermark
cannot be done while the buffer is enabled. When buffer is disabled, all
DMA resources are freed anyway.
 - Added validation on trigger to be used by own device
 - Best sample rate I could obtain using the low frequency clock was about
4k samples/second, with a watermark of 100. To get up to 50k samples/second
the ADC frequency must be increased to max.
 - Fixed computation of DMA buffer size
 - Addressed other comments from mailing list review. Feedback is appreciated

 drivers/iio/adc/Kconfig            |   1 +
 drivers/iio/adc/at91-sama5d2_adc.c | 453 +++++++++++++++++++++++++++++++++++--
 2 files changed, 434 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 1d13bf0..1a3a8e3 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -158,6 +158,7 @@ config AT91_SAMA5D2_ADC
 	tristate "Atmel AT91 SAMA5D2 ADC"
 	depends on ARCH_AT91 || COMPILE_TEST
 	depends on HAS_IOMEM
+	depends on HAS_DMA
 	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for Atmel SAMA5D2 ADC which is
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index a70ef7f..11d34a8 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -16,6 +16,8 @@
 
 #include <linux/bitops.h>
 #include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -100,6 +102,8 @@
 #define AT91_SAMA5D2_LCDR	0x20
 /* Interrupt Enable Register */
 #define AT91_SAMA5D2_IER	0x24
+/* Interrupt Enable Register - general overrun error */
+#define AT91_SAMA5D2_IER_GOVRE BIT(25)
 /* Interrupt Disable Register */
 #define AT91_SAMA5D2_IDR	0x28
 /* Interrupt Mask Register */
@@ -167,13 +171,19 @@
 
 /*
  * Maximum number of bytes to hold conversion from all channels
- * plus the timestamp
+ * without the timestamp.
  */
-#define AT91_BUFFER_MAX_BYTES ((AT91_SAMA5D2_SINGLE_CHAN_CNT +		\
-				AT91_SAMA5D2_DIFF_CHAN_CNT) * 2 + 8)
+#define AT91_BUFFER_MAX_CONVERSION_BYTES ((AT91_SAMA5D2_SINGLE_CHAN_CNT + \
+					 AT91_SAMA5D2_DIFF_CHAN_CNT) * 2)
+
+/* This total must also include the timestamp */
+#define AT91_BUFFER_MAX_BYTES (AT91_BUFFER_MAX_CONVERSION_BYTES + 8)
 
 #define AT91_BUFFER_MAX_HWORDS (AT91_BUFFER_MAX_BYTES / 2)
 
+#define AT91_HWFIFO_MAX_SIZE_STR	"128"
+#define AT91_HWFIFO_MAX_SIZE		128
+
 #define AT91_SAMA5D2_CHAN_SINGLE(num, addr)				\
 	{								\
 		.type = IIO_VOLTAGE,					\
@@ -228,6 +238,28 @@ struct at91_adc_trigger {
 	bool				hw_trig;
 };
 
+/**
+ * at91_adc_dma - at91-sama5d2 dma information struct
+ * @dma_chan:		the dma channel acquired
+ * @rx_buf:		dma coherent allocated area
+ * @rx_dma_buf:		dma handler for the buffer
+ * @phys_addr:		physical address of the ADC base register
+ * @buf_idx:		index inside the dma buffer where reading was last done
+ * @rx_buf_sz:		size of buffer used by DMA operation
+ * @watermark:		number of conversions to copy before DMA triggers irq
+ * @dma_ts:		hold the start timestamp of dma operation
+ */
+struct at91_adc_dma {
+	struct dma_chan			*dma_chan;
+	u8				*rx_buf;
+	dma_addr_t			rx_dma_buf;
+	phys_addr_t			phys_addr;
+	int				buf_idx;
+	int				rx_buf_sz;
+	int				watermark;
+	s64				dma_ts;
+};
+
 struct at91_adc_state {
 	void __iomem			*base;
 	int				irq;
@@ -242,6 +274,7 @@ struct at91_adc_state {
 	u32				conversion_value;
 	struct at91_adc_soc_info	soc_info;
 	wait_queue_head_t		wq_data_available;
+	struct at91_adc_dma		dma_st;
 	u16				buffer[AT91_BUFFER_MAX_HWORDS];
 	/*
 	 * lock to prevent concurrent 'single conversion' requests through
@@ -322,11 +355,17 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
 		if (state) {
 			at91_adc_writel(st, AT91_SAMA5D2_CHER,
 					BIT(chan->channel));
-			at91_adc_writel(st, AT91_SAMA5D2_IER,
-					BIT(chan->channel));
+			/* enable irq only if not using DMA */
+			if (!st->dma_st.dma_chan) {
+				at91_adc_writel(st, AT91_SAMA5D2_IER,
+						BIT(chan->channel));
+			}
 		} else {
-			at91_adc_writel(st, AT91_SAMA5D2_IDR,
-					BIT(chan->channel));
+			/* disable irq only if not using DMA */
+			if (!st->dma_st.dma_chan) {
+				at91_adc_writel(st, AT91_SAMA5D2_IDR,
+						BIT(chan->channel));
+			}
 			at91_adc_writel(st, AT91_SAMA5D2_CHDR,
 					BIT(chan->channel));
 		}
@@ -340,6 +379,10 @@ static int at91_adc_reenable_trigger(struct iio_trigger *trig)
 	struct iio_dev *indio = iio_trigger_get_drvdata(trig);
 	struct at91_adc_state *st = iio_priv(indio);
 
+	/* if we are using DMA, we must not reenable irq after each trigger */
+	if (st->dma_st.dma_chan)
+		return 0;
+
 	enable_irq(st->irq);
 
 	/* Needed to ACK the DRDY interruption */
@@ -351,6 +394,153 @@ static const struct iio_trigger_ops at91_adc_trigger_ops = {
 	.owner = THIS_MODULE,
 	.set_trigger_state = &at91_adc_configure_trigger,
 	.try_reenable = &at91_adc_reenable_trigger,
+	.validate_device = iio_trigger_validate_own_device,
+};
+
+static int at91_adc_dma_size_done(struct at91_adc_state *st)
+{
+	struct dma_tx_state state;
+	enum dma_status status;
+	int i, size;
+
+	status = dmaengine_tx_status(st->dma_st.dma_chan,
+				     st->dma_st.dma_chan->cookie,
+				     &state);
+	if (status != DMA_IN_PROGRESS)
+		return 0;
+
+	/* Transferred length is size in bytes from end of buffer */
+	i = st->dma_st.rx_buf_sz - state.residue;
+
+	/* Return available bytes */
+	if (i >= st->dma_st.buf_idx)
+		size = i - st->dma_st.buf_idx;
+	else
+		size = st->dma_st.rx_buf_sz + i - st->dma_st.buf_idx;
+	return size;
+}
+
+static void at91_dma_buffer_done(void *data)
+{
+	struct iio_dev *indio_dev = data;
+
+	iio_trigger_poll_chained(indio_dev->trig);
+}
+
+static int at91_adc_dma_start(struct iio_dev *indio_dev)
+{
+	struct at91_adc_state *st = iio_priv(indio_dev);
+	struct dma_async_tx_descriptor *desc;
+	dma_cookie_t cookie;
+	int ret;
+	u8 bit;
+
+	if (!st->dma_st.dma_chan)
+		return 0;
+
+	/* we start a new DMA, so set buffer index to start */
+	st->dma_st.buf_idx = 0;
+
+	/*
+	 * compute buffer size w.r.t. watermark and enabled channels.
+	 * scan_bytes is aligned so we need an exact size for DMA
+	 */
+	st->dma_st.rx_buf_sz = 0;
+
+	for_each_set_bit(bit, indio_dev->active_scan_mask,
+			 indio_dev->num_channels) {
+		struct iio_chan_spec const *chan = indio_dev->channels + bit;
+
+		st->dma_st.rx_buf_sz += chan->scan_type.storagebits / 8;
+	}
+	st->dma_st.rx_buf_sz *= st->dma_st.watermark;
+
+	/* Prepare a DMA cyclic transaction */
+	desc = dmaengine_prep_dma_cyclic(st->dma_st.dma_chan,
+					 st->dma_st.rx_dma_buf,
+					 st->dma_st.rx_buf_sz,
+					 st->dma_st.rx_buf_sz / 2,
+					 DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
+
+	if (!desc) {
+		dev_err(&indio_dev->dev, "cannot prepare DMA cyclic\n");
+		return -EBUSY;
+	}
+
+	desc->callback = at91_dma_buffer_done;
+	desc->callback_param = indio_dev;
+
+	cookie = dmaengine_submit(desc);
+	ret = dma_submit_error(cookie);
+	if (ret) {
+		dev_err(&indio_dev->dev, "cannot submit DMA cyclic\n");
+		dmaengine_terminate_async(st->dma_st.dma_chan);
+		return ret;
+	}
+
+	/* enable general overrun error signaling */
+	at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_GOVRE);
+	/* Issue pending DMA requests */
+	dma_async_issue_pending(st->dma_st.dma_chan);
+
+	/* consider current time as DMA start time for timestamps */
+	st->dma_st.dma_ts = iio_get_time_ns(indio_dev);
+
+	dev_dbg(&indio_dev->dev, "DMA cyclic started\n");
+
+	return 0;
+}
+
+static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
+{
+	int ret;
+
+	ret = at91_adc_dma_start(indio_dev);
+	if (ret) {
+		dev_err(&indio_dev->dev, "buffer postenable failed\n");
+		return ret;
+	}
+
+	return iio_triggered_buffer_postenable(indio_dev);
+}
+
+static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct at91_adc_state *st = iio_priv(indio_dev);
+	int ret;
+	u8 bit;
+
+	ret = iio_triggered_buffer_predisable(indio_dev);
+	if (ret < 0)
+		dev_err(&indio_dev->dev, "buffer predisable failed\n");
+
+	if (!st->dma_st.dma_chan)
+		return ret;
+
+	/* if we are using DMA we must clear registers and end DMA */
+	dmaengine_terminate_sync(st->dma_st.dma_chan);
+
+	/*
+	 * For each enabled channel we must read the last converted value
+	 * to clear EOC status and not get a possible interrupt later.
+	 * This value is being read by DMA from LCDR anyway
+	 */
+	for_each_set_bit(bit, indio_dev->active_scan_mask,
+			 indio_dev->num_channels) {
+		struct iio_chan_spec const *chan = indio_dev->channels + bit;
+
+		if (st->dma_st.dma_chan)
+			at91_adc_readl(st, chan->address);
+	}
+
+	/* read overflow register to clear possible overflow status */
+	at91_adc_readl(st, AT91_SAMA5D2_OVER);
+	return ret;
+}
+
+static const struct iio_buffer_setup_ops at91_buffer_setup_ops = {
+	.postenable = &at91_adc_buffer_postenable,
+	.predisable = &at91_adc_buffer_predisable,
 };
 
 static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio,
@@ -389,24 +579,77 @@ static int at91_adc_trigger_init(struct iio_dev *indio)
 	return 0;
 }
 
-static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
+static void at91_adc_trigger_handler_nodma(struct iio_dev *indio_dev,
+					   struct iio_poll_func *pf)
 {
-	struct iio_poll_func *pf = p;
-	struct iio_dev *indio = pf->indio_dev;
-	struct at91_adc_state *st = iio_priv(indio);
+	struct at91_adc_state *st = iio_priv(indio_dev);
 	int i = 0;
 	u8 bit;
 
-	for_each_set_bit(bit, indio->active_scan_mask, indio->num_channels) {
-		struct iio_chan_spec const *chan = indio->channels + bit;
+	for_each_set_bit(bit, indio_dev->active_scan_mask,
+			 indio_dev->num_channels) {
+		struct iio_chan_spec const *chan = indio_dev->channels + bit;
 
 		st->buffer[i] = at91_adc_readl(st, chan->address);
 		i++;
 	}
+	iio_push_to_buffers_with_timestamp(indio_dev, st->buffer,
+					   pf->timestamp);
+}
+
+static void at91_adc_trigger_handler_dma(struct iio_dev *indio_dev)
+{
+	struct at91_adc_state *st = iio_priv(indio_dev);
+	int transferred_len = at91_adc_dma_size_done(st);
+	s64 ns = iio_get_time_ns(indio_dev);
+	s64 interval;
+	int sample_index = 0, sample_count, sample_size;
+
+	u32 status = at91_adc_readl(st, AT91_SAMA5D2_ISR);
+	/* if we reached this point, we cannot sample faster */
+	if (status & AT91_SAMA5D2_IER_GOVRE)
+		pr_info_ratelimited("%s: conversion overrun detected\n",
+				    indio_dev->name);
+
+	sample_size = div_s64(st->dma_st.rx_buf_sz, st->dma_st.watermark);
+
+	sample_count = div_s64(transferred_len, sample_size);
+
+	/*
+	 * interval between samples is total time since last transfer handling
+	 * divided by the number of samples (total size divided by sample size)
+	 */
+	interval = div_s64((ns - st->dma_st.dma_ts), sample_count);
+
+	while (transferred_len >= sample_size) {
+		iio_push_to_buffers_with_timestamp(indio_dev,
+				(st->dma_st.rx_buf + st->dma_st.buf_idx),
+				(st->dma_st.dma_ts + interval * sample_index));
+		/* adjust remaining length */
+		transferred_len -= sample_size;
+		/* adjust buffer index */
+		st->dma_st.buf_idx += sample_size;
+		/* in case of reaching end of buffer, reset index */
+		if (st->dma_st.buf_idx >= st->dma_st.rx_buf_sz)
+			st->dma_st.buf_idx = 0;
+		sample_index++;
+	}
+	/* adjust saved time for next transfer handling */
+	st->dma_st.dma_ts = iio_get_time_ns(indio_dev);
+}
+
+static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct at91_adc_state *st = iio_priv(indio_dev);
 
-	iio_push_to_buffers_with_timestamp(indio, st->buffer, pf->timestamp);
+	if (st->dma_st.dma_chan)
+		at91_adc_trigger_handler_dma(indio_dev);
+	else
+		at91_adc_trigger_handler_nodma(indio_dev, pf);
 
-	iio_trigger_notify_done(indio->trig);
+	iio_trigger_notify_done(indio_dev->trig);
 
 	return IRQ_HANDLED;
 }
@@ -415,7 +658,7 @@ static int at91_adc_buffer_init(struct iio_dev *indio)
 {
 	return devm_iio_triggered_buffer_setup(&indio->dev, indio,
 			&iio_pollfunc_store_time,
-			&at91_adc_trigger_handler, NULL);
+			&at91_adc_trigger_handler, &at91_buffer_setup_ops);
 }
 
 static unsigned at91_adc_startup_time(unsigned startup_time_min,
@@ -486,10 +729,13 @@ static irqreturn_t at91_adc_interrupt(int irq, void *private)
 	if (!(status & imr))
 		return IRQ_NONE;
 
-	if (iio_buffer_enabled(indio)) {
+	if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) {
 		disable_irq_nosync(irq);
 		iio_trigger_poll(indio->trig);
-	} else {
+	} else if (iio_buffer_enabled(indio) && st->dma_st.dma_chan) {
+		disable_irq_nosync(irq);
+		WARN(true, "Unexpected irq occurred\n");
+	} else if (!iio_buffer_enabled(indio)) {
 		st->conversion_value = at91_adc_readl(st, st->chan->address);
 		st->conversion_done = true;
 		wake_up_interruptible(&st->wq_data_available);
@@ -511,7 +757,6 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev,
 		ret = iio_device_claim_direct_mode(indio_dev);
 		if (ret)
 			return ret;
-
 		mutex_lock(&st->lock);
 
 		st->chan = chan;
@@ -581,9 +826,123 @@ static int at91_adc_write_raw(struct iio_dev *indio_dev,
 	return 0;
 }
 
+static void at91_adc_dma_init(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct at91_adc_state *st = iio_priv(indio_dev);
+	struct dma_slave_config config = {0};
+	/*
+	 * We make the buffer double the size of the fifo,
+	 * such that DMA uses one half of the buffer (full fifo size)
+	 * and the software uses the other half to read/write.
+	 */
+	unsigned int pages = DIV_ROUND_UP(AT91_HWFIFO_MAX_SIZE *
+					  AT91_BUFFER_MAX_CONVERSION_BYTES * 2,
+					  PAGE_SIZE);
+
+	if (st->dma_st.dma_chan)
+		return;
+
+	st->dma_st.dma_chan = dma_request_slave_channel(&pdev->dev, "rx");
+
+	if (!st->dma_st.dma_chan)  {
+		dev_info(&pdev->dev, "can't get DMA channel\n");
+		goto dma_exit;
+	}
+
+	st->dma_st.rx_buf = dma_alloc_coherent(st->dma_st.dma_chan->device->dev,
+					       pages * PAGE_SIZE,
+					       &st->dma_st.rx_dma_buf,
+					       GFP_KERNEL);
+	if (!st->dma_st.rx_buf) {
+		dev_info(&pdev->dev, "can't allocate coherent DMA area\n");
+		goto dma_chan_disable;
+	}
+
+	/* Configure DMA channel to read data register */
+	config.direction = DMA_DEV_TO_MEM;
+	config.src_addr = (phys_addr_t)(st->dma_st.phys_addr
+			  + AT91_SAMA5D2_LCDR);
+	config.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+	config.src_maxburst = 1;
+	config.dst_maxburst = 1;
+
+	if (dmaengine_slave_config(st->dma_st.dma_chan, &config)) {
+		dev_info(&pdev->dev, "can't configure DMA slave\n");
+		goto dma_free_area;
+	}
+
+	dev_info(&pdev->dev, "using %s for rx DMA transfers\n",
+		 dma_chan_name(st->dma_st.dma_chan));
+
+	return;
+
+dma_free_area:
+	dma_free_coherent(st->dma_st.dma_chan->device->dev, pages * PAGE_SIZE,
+			  st->dma_st.rx_buf, st->dma_st.rx_dma_buf);
+dma_chan_disable:
+	dma_release_channel(st->dma_st.dma_chan);
+	st->dma_st.dma_chan = 0;
+dma_exit:
+	dev_info(&pdev->dev, "continuing without DMA support\n");
+}
+
+static void at91_adc_dma_disable(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct at91_adc_state *st = iio_priv(indio_dev);
+	unsigned int pages = DIV_ROUND_UP(AT91_HWFIFO_MAX_SIZE *
+					  AT91_BUFFER_MAX_CONVERSION_BYTES * 2,
+					  PAGE_SIZE);
+
+	/* if we are not using DMA, just return */
+	if (!st->dma_st.dma_chan)
+		return;
+
+	/* wait for all transactions to be terminated first*/
+	dmaengine_terminate_sync(st->dma_st.dma_chan);
+
+	dma_free_coherent(st->dma_st.dma_chan->device->dev, pages * PAGE_SIZE,
+			  st->dma_st.rx_buf, st->dma_st.rx_dma_buf);
+	dma_release_channel(st->dma_st.dma_chan);
+	st->dma_st.dma_chan = 0;
+
+	dev_info(&pdev->dev, "continuing without DMA support\n");
+}
+
+static int at91_adc_set_watermark(struct iio_dev *indio_dev, unsigned int val)
+{
+	struct at91_adc_state *st = iio_priv(indio_dev);
+
+	if (val > AT91_HWFIFO_MAX_SIZE)
+		return -EINVAL;
+
+	if (!st->selected_trig->hw_trig) {
+		dev_dbg(&indio_dev->dev, "we need hw trigger for DMA\n");
+		return 0;
+	}
+
+	dev_dbg(&indio_dev->dev, "new watermark is %u\n", val);
+	st->dma_st.watermark = val;
+
+	/*
+	 * The logic here is: if we have watermark 1, it means we do
+	 * each conversion with it's own IRQ, thus we don't need DMA.
+	 * If the watermark is higher, we do DMA to do all the transfers in bulk
+	 */
+
+	if (val == 1)
+		at91_adc_dma_disable(to_platform_device(&indio_dev->dev));
+	else if (val > 1)
+		at91_adc_dma_init(to_platform_device(&indio_dev->dev));
+
+	return 0;
+}
+
 static const struct iio_info at91_adc_info = {
 	.read_raw = &at91_adc_read_raw,
 	.write_raw = &at91_adc_write_raw,
+	.hwfifo_set_watermark = &at91_adc_set_watermark,
 	.driver_module = THIS_MODULE,
 };
 
@@ -601,6 +960,42 @@ static void at91_adc_hw_init(struct at91_adc_state *st)
 	at91_adc_setup_samp_freq(st, st->soc_info.min_sample_rate);
 }
 
+static ssize_t at91_adc_get_fifo_state(struct device *dev,
+				       struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev =
+			platform_get_drvdata(to_platform_device(dev));
+	struct at91_adc_state *st = iio_priv(indio_dev);
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n", !!st->dma_st.dma_chan);
+}
+
+static ssize_t at91_adc_get_watermark(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev =
+			platform_get_drvdata(to_platform_device(dev));
+	struct at91_adc_state *st = iio_priv(indio_dev);
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n", st->dma_st.watermark);
+}
+
+static IIO_DEVICE_ATTR(hwfifo_enabled, 0444,
+		       at91_adc_get_fifo_state, NULL, 0);
+static IIO_DEVICE_ATTR(hwfifo_watermark, 0444,
+		       at91_adc_get_watermark, NULL, 0);
+
+static IIO_CONST_ATTR(hwfifo_watermark_min, "2");
+static IIO_CONST_ATTR(hwfifo_watermark_max, AT91_HWFIFO_MAX_SIZE_STR);
+
+static const struct attribute *at91_adc_fifo_attributes[] = {
+	&iio_const_attr_hwfifo_watermark_min.dev_attr.attr,
+	&iio_const_attr_hwfifo_watermark_max.dev_attr.attr,
+	&iio_dev_attr_hwfifo_watermark.dev_attr.attr,
+	&iio_dev_attr_hwfifo_enabled.dev_attr.attr,
+	NULL,
+};
+
 static int at91_adc_probe(struct platform_device *pdev)
 {
 	struct iio_dev *indio_dev;
@@ -676,6 +1071,9 @@ static int at91_adc_probe(struct platform_device *pdev)
 	if (!res)
 		return -EINVAL;
 
+	/* if we plan to use DMA, we need the physical address of the regs */
+	st->dma_st.phys_addr = res->start;
+
 	st->base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(st->base))
 		return PTR_ERR(st->base);
@@ -739,11 +1137,22 @@ static int at91_adc_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "couldn't setup the triggers.\n");
 			goto per_clk_disable_unprepare;
 		}
+		/*
+		 * Initially the iio buffer has a length of 2 and
+		 * a watermark of 1
+		 */
+		st->dma_st.watermark = 1;
+
+		iio_buffer_set_attrs(indio_dev->buffer,
+				     at91_adc_fifo_attributes);
 	}
 
+	if (dma_coerce_mask_and_coherent(&indio_dev->dev, DMA_BIT_MASK(32)))
+		dev_info(&pdev->dev, "cannot set DMA mask to 32-bit\n");
+
 	ret = iio_device_register(indio_dev);
 	if (ret < 0)
-		goto per_clk_disable_unprepare;
+		goto dma_disable;
 
 	if (st->selected_trig->hw_trig)
 		dev_info(&pdev->dev, "setting up trigger as %s\n",
@@ -754,6 +1163,8 @@ static int at91_adc_probe(struct platform_device *pdev)
 
 	return 0;
 
+dma_disable:
+	at91_adc_dma_disable(pdev);
 per_clk_disable_unprepare:
 	clk_disable_unprepare(st->per_clk);
 vref_disable:
@@ -770,6 +1181,8 @@ static int at91_adc_remove(struct platform_device *pdev)
 
 	iio_device_unregister(indio_dev);
 
+	at91_adc_dma_disable(pdev);
+
 	clk_disable_unprepare(st->per_clk);
 
 	regulator_disable(st->vref);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 2/4] ARM: dts: at91: sama5d2: added dma property for ADC device
From: Eugen Hristev @ 2017-11-15 12:56 UTC (permalink / raw)
  To: nicolas.ferre, alexandre.belloni, linux-iio, lars,
	linux-arm-kernel, devicetree, linux-kernel, ludovic.desroches,
	jic23
  Cc: eugen.hristev
In-Reply-To: <1510750608-8697-1-git-send-email-eugen.hristev@microchip.com>

Added DMA property for ADC device

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 arch/arm/boot/dts/sama5d2.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index 38d2216..ca8bf13 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -1430,6 +1430,8 @@
 				atmel,min-sample-rate-hz = <200000>;
 				atmel,max-sample-rate-hz = <20000000>;
 				atmel,startup-time-ms = <4>;
+				dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1) | AT91_XDMAC_DT_PERID(25))>;
+				dma-names = "rx";
 				status = "disabled";
 			};
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 1/4] dt-bindings: iio: at91-sama5d2_adc: add optional dma property
From: Eugen Hristev @ 2017-11-15 12:56 UTC (permalink / raw)
  To: nicolas.ferre, alexandre.belloni, linux-iio, lars,
	linux-arm-kernel, devicetree, linux-kernel, ludovic.desroches,
	jic23
  Cc: eugen.hristev
In-Reply-To: <1510750608-8697-1-git-send-email-eugen.hristev@microchip.com>

Added property for DMA configuration of the device.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 Changes in v3:
None, but we discussed on the ML about whether we should have "dma-names"
present in the binding even if it's only one.
The helpers in the kernel to retrieve the channel info rely on the
presence of this property, so I am resending the patch based on this.
If another solution is better, please advise and I can try it and
resend the patch.

 Documentation/devicetree/bindings/iio/adc/at91-sama5d2_adc.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/at91-sama5d2_adc.txt b/Documentation/devicetree/bindings/iio/adc/at91-sama5d2_adc.txt
index 552e7a8..6469a4c 100644
--- a/Documentation/devicetree/bindings/iio/adc/at91-sama5d2_adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/at91-sama5d2_adc.txt
@@ -17,6 +17,11 @@ Required properties:
   This property uses the IRQ edge types values: IRQ_TYPE_EDGE_RISING ,
   IRQ_TYPE_EDGE_FALLING or IRQ_TYPE_EDGE_BOTH
 
+Optional properties:
+  - dmas: Phandle to dma channel for the ADC.
+  - dma-names: Must be "rx" when dmas property is being used.
+  See ../../dma/dma.txt for details.
+
 Example:
 
 adc: adc@fc030000 {
@@ -31,4 +36,6 @@ adc: adc@fc030000 {
 	vddana-supply = <&vdd_3v3_lp_reg>;
 	vref-supply = <&vdd_3v3_lp_reg>;
 	atmel,trigger-edge-type = <IRQ_TYPE_EDGE_BOTH>;
+	dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1) | AT91_XDMAC_DT_PERID(25))>;
+	dma-names = "rx";
 }
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 0/4] iio: adc: at91-sama5d2_adc: add DMA support
From: Eugen Hristev @ 2017-11-15 12:56 UTC (permalink / raw)
  To: nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, lars-Qo5EllUWu/uELgA04lAiVw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA,
	jic23-DgEjT+Ai2ygdnm+yROfE0A
  Cc: eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA

This patch implements the DMA support for the ADC in sama5d2 SoC.
After discussing on the mailing list, this approach is based on triggered
kfifo buffer, with DMA support added on top of it.
Thus, the trigger is enabled by the buffer. The ADC itself will not have
an IRQ enabled if using DMA. With DMA, the channels are enabled, and DMA
controller is configured to read from the data ready registers.
When DMA starts, the trigger will start the conversion (external trigger
configured), then the registers will have the conversion data ready, the ADC
will trigger the DMA controller to read from the registers, the DMA will
copy the data into the software buffer, and trigger the DMA IRQ.
In the bottom half, the trigger polled and the data from the DMA buffer is
pushed to buffer.

The DMA will use a cyclic buffer to write to one half, and the software can
read from the other half. The DMA operation doesn't stop until the buffer is
disabled.
The DMA coherent area is allocated when DMA is initially started, and
deallocated only if the watermark is changed to 1 (no more DMA usage). The
coherent area is large enough to cope with maximum fifo size for all possible
channels enabled.
The fifo size is set to 128 conversions by default in the driver.

The implementation uses the user watermark  to decide whether DMA will be
used or not. For watermark 1, DMA will not be used. If watermark is greater,
DMA will be used.
Sysfs attributes are created to indicate whether the DMA is used,
with hwfifo_enabled, and the current DMA watermark is readable
in hwfifo_watermark. Minimum and maximum values are in hwfifo_watermark_min
and hwfifo_watermark_max.

Devicetree binding added for dma as well.
Binding adds both dmas and dma-names to keep compatibility with the drivers and
current helpers that rely on the properties being present

Modified devicetree for sama5d2 SoC to add connected DMA channel.

To demonstrate the capabilities of the DMA support at best sampling rate, the
frequency for the ADC needs to be modified to maximum, from sysfs attribute.

A different commit addresses the issue of not clearing the DRDY irq in direct
mode. This is required to avoid erroneous overrun warnings.

  Changes in v3:
 - Removed commit to change sample rate to maximum as this is configurable
from sysfs
 - Remove misleaded dev_info message when DMA was not enabled at probe
 - Rebased patch on top of the
[PATCH] iio: adc: at91-sama5d2_adc: fix probe error on missing trigger property
Which is already upstreamed in 4.14
 - Fixed the bug introduced in v2, with buffer size
 - added extra check when enabling DMA, to have hw trigger present.
This is because now, we can have the driver with software trigger only (if no
hw trigger in device tree, start as software only)

  Changes in v2:
 - No longer add last timestamp to all samples. Now, compute an interval
between samples w.r.t. start and end time of the transfer and number
of samples. Then distribute them each in the time interval.
 - Add warning for conversion overrun. This helps user identify cases
when the watermark needs adjustment : the software is too slow in reading
data from the ADC.
 - Protection around watermark is not needed, changing of the watermark
cannot be done while the buffer is enabled. When buffer is disabled, all
DMA resources are freed anyway.
 - Added validation on trigger to be used by own device
 - Best sample rate I could obtain using the low frequency clock was about
4k samples/second, with a watermark of 100. To get up to 50k samples/second
the ADC frequency must be increased to max.
 - Addressed other comments from mailing list review. Feedback is appreciated
 - Added [PATCH v2 4/5] iio: adc: at91-sama5d2_adc: ack DRDY irq in direct
 - Added [PATCH v2 5/5] iio: adc: at91-sama5d2_adc: use max sample rate


Eugen Hristev (4):
  dt-bindings: iio: at91-sama5d2_adc: add optional dma property
  ARM: dts: at91: sama5d2: added dma property for ADC device
  iio: adc: at91-sama5d2_adc: add support for DMA
  iio: adc: at91-sama5d2_adc: ack DRDY irq in direct mode

 .../bindings/iio/adc/at91-sama5d2_adc.txt          |   7 +
 arch/arm/boot/dts/sama5d2.dtsi                     |   2 +
 drivers/iio/adc/Kconfig                            |   1 +
 drivers/iio/adc/at91-sama5d2_adc.c                 | 456 ++++++++++++++++++++-
 4 files changed, 446 insertions(+), 20 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH] of/pci: Fix theoretical NULL dereference
From: Robin Murphy @ 2017-11-15 12:50 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	frowand.list-Re5JQEeQqe8AvxtiuMwx3w
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

In the (relatively mechanical) process of adapting the RID-mapping code
to put the resulting ID in an output argument rather than the funtion
return value, we ended up with the debug print using the argument
pointer rather than the local value, which potentially defeats the
earlier NULL check.

Fixes: 987068fcbdb7: "of/irq: Break out msi-map lookup (again)"
Reported-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
---
 drivers/of/of_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index e9ec931f5b9a..a7b1cb6c2f65 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -374,7 +374,7 @@ int of_pci_map_rid(struct device_node *np, u32 rid,
 
 		pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
 			np, map_name, map_mask, rid_base, out_base,
-			rid_len, rid, *id_out);
+			rid_len, rid, masked_rid - rid_base + out_base);
 		return 0;
 	}
 
-- 
2.13.4.dirty

--
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 2/2] leds: lm3692x: Introduce LM3692x dual string driver
From: Dan Murphy @ 2017-11-15 12:47 UTC (permalink / raw)
  To: Pavel Machek
  Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, devicetree,
	linux-kernel, linux-leds
In-Reply-To: <20171115104526.GC456@amd>

Pavel

Thanks

On 11/15/2017 04:45 AM, Pavel Machek wrote:
> On Mon 2017-11-13 14:50:52, Dan Murphy wrote:
>> Introducing the LM3692x Dual-String white LED driver.
>>
>> Data sheet is located
>> http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>  drivers/leds/leds-lm3692x.c | 380 ++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 380 insertions(+)
>>  create mode 100644 drivers/leds/leds-lm3692x.c
>>
>> diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
>> new file mode 100644
>> index 000000000000..beb753abc7b0
>> --- /dev/null
>> +++ b/drivers/leds/leds-lm3692x.c
>> @@ -0,0 +1,380 @@
>> +/*
>> + * TI lm3692x LED Driver
>> + *
>> + * Copyright (C) 2017 Texas Instruments
>> + *
>> + * Author: Dan Murphy <dmurphy@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2 as published by the Free Software Foundation.
>> + *
>> + */
> 
> I'd put data sheet link here.

OK

> 
>> +/**
>> + * struct lm3692x_led -
>> + * @lock - Lock for reading/writing the device
>> + * @client - Pointer to the I2C client
>> + * @led_dev - led class device pointer
>> + * @regmap - Devices register map
>> + * @enable_gpio - VDDIO/EN gpio to enable communication interface
>> + * @regulator - LED supply regulator pointer
>> + * @label - LED label
>> + **/
> 
> Are you sure this is linuxdoc? Would */ at the end be enough?

I will fix this

> 
>> +static int lm3692x_fault_check(struct lm3692x_led *led)
>> +{
>> +	int ret, fault;
>> +	unsigned int read_buf;
>> +
>> +	ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf);
>> +	if (ret)
>> +		goto out;
> 
> Just return here.

OK

> 
>> +	/*
>> +	 * For glitch free operation, the following data should
>> +	 * only be written while device enable bit is 0
>> +	 * Per Section 7.5.14 of the data sheet
>> +	 */
> 
> "per section 7.5.14 of the data sheet."

OK

> 
>> +	ret = regmap_write(led->regmap, LM3692X_PWM_CTRL,
>> +		LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ);
>> +	if (ret)
>> +		dev_err(&led->client->dev, "Failed programming PWM CTRL\n");
>> +
>> +	ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL,
>> +			LM3692X_BRHT_MODE_RAMP_MULTI |
>> +			LM3692X_BL_ADJ_POL |
>> +			LM3692X_RAMP_RATE_250us);
>> +	if (ret)
>> +		dev_err(&led->client->dev, "Failed programming BOOST CTRL\n");
>> +
>> +	ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00);
>> +	if (ret)
>> +		dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n");
> 
> If something fails, is it wise to continue?
> 
> Thanks,
> 									Pavel
> 


-- 
------------------
Dan Murphy

^ permalink raw reply

* [bug report] of/irq: Break out msi-map lookup (again)
From: Dan Carpenter @ 2017-11-15 12:23 UTC (permalink / raw)
  To: robin.murphy-5wv7dgnIgG8; +Cc: devicetree-u79uwXL29TY76Z2rM5mHXA

Hello Robin Murphy,

The patch 987068fcbdb7: "of/irq: Break out msi-map lookup (again)"
from Sep 12, 2016, leads to the following static checker warning:

	drivers/of/of_pci.c:375 of_pci_map_rid()
	error: we previously assumed 'id_out' could be null (see line 372)

drivers/of/of_pci.c
   371  
   372                  if (id_out)
                            ^^^^^^
Check for NULL

   373                          *id_out = masked_rid - rid_base + out_base;
   374  
   375                  pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
   376                          np, map_name, map_mask, rid_base, out_base,
   377                          rid_len, rid, *id_out);
                                               ^^^^^^
Unchecked dereference

   378                  return 0;

regards,
dan carpenter
--
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 2/2] leds: lm3692x: Introduce LM3692x dual string driver
From: Pavel Machek @ 2017-11-15 10:45 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, devicetree,
	linux-kernel, linux-leds
In-Reply-To: <20171113205052.25884-2-dmurphy@ti.com>

[-- Attachment #1: Type: text/plain, Size: 2716 bytes --]

On Mon 2017-11-13 14:50:52, Dan Murphy wrote:
> Introducing the LM3692x Dual-String white LED driver.
> 
> Data sheet is located
> http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/leds/leds-lm3692x.c | 380 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 380 insertions(+)
>  create mode 100644 drivers/leds/leds-lm3692x.c
> 
> diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
> new file mode 100644
> index 000000000000..beb753abc7b0
> --- /dev/null
> +++ b/drivers/leds/leds-lm3692x.c
> @@ -0,0 +1,380 @@
> +/*
> + * TI lm3692x LED Driver
> + *
> + * Copyright (C) 2017 Texas Instruments
> + *
> + * Author: Dan Murphy <dmurphy@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + */

I'd put data sheet link here.

> +/**
> + * struct lm3692x_led -
> + * @lock - Lock for reading/writing the device
> + * @client - Pointer to the I2C client
> + * @led_dev - led class device pointer
> + * @regmap - Devices register map
> + * @enable_gpio - VDDIO/EN gpio to enable communication interface
> + * @regulator - LED supply regulator pointer
> + * @label - LED label
> + **/

Are you sure this is linuxdoc? Would */ at the end be enough?

> +static int lm3692x_fault_check(struct lm3692x_led *led)
> +{
> +	int ret, fault;
> +	unsigned int read_buf;
> +
> +	ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf);
> +	if (ret)
> +		goto out;

Just return here.

> +	/*
> +	 * For glitch free operation, the following data should
> +	 * only be written while device enable bit is 0
> +	 * Per Section 7.5.14 of the data sheet
> +	 */

"per section 7.5.14 of the data sheet."

> +	ret = regmap_write(led->regmap, LM3692X_PWM_CTRL,
> +		LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ);
> +	if (ret)
> +		dev_err(&led->client->dev, "Failed programming PWM CTRL\n");
> +
> +	ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL,
> +			LM3692X_BRHT_MODE_RAMP_MULTI |
> +			LM3692X_BL_ADJ_POL |
> +			LM3692X_RAMP_RATE_250us);
> +	if (ret)
> +		dev_err(&led->client->dev, "Failed programming BOOST CTRL\n");
> +
> +	ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00);
> +	if (ret)
> +		dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n");

If something fails, is it wise to continue?

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver
From: Pavel Machek @ 2017-11-15 10:35 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	rpurdie-Fm38FmjxZ/leoWH0uzbU5w,
	jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171113205052.25884-1-dmurphy-l0cyMroinI0@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]

On Mon 2017-11-13 14:50:51, Dan Murphy wrote:
> This adds the devicetree bindings for the LM3692x
> I2C LED string driver.
> 
> Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>

Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> new file mode 100644
> index 000000000000..cfef67bd4100
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> @@ -0,0 +1,28 @@
> +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver
> +
> +The LM36922 is an ultra-compact, highly efficient,
> +two string white-LED driver designed for LCD display
> +backlighting.
> +
> +Required properties:
> +	- compatible:
> +		"ti,lm3692x"
> +	- reg -  I2C slave address
> +
> +Optional properties:
> +	- label - Used for naming LEDs
> +	- enable-gpio - gpio pin to enable/disable the device.
> +	- supply - "vled" - LED supply
> +
> +Example:
> +
> +leds: leds@6 {
> +	compatible = "ti,lm3692x";
> +	reg = <0x36>;
> +	label = "backlight_cluster";
> +	enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> +	vled-supply = <&vbatt>;
> +}
> +
> +For more product information please see the link below:
> +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* RE: [PATCH] usb: dwc3: Enable the USB snooping
From: Felipe Balbi @ 2017-11-15 10:22 UTC (permalink / raw)
  To: Ran Wang
  Cc: Greg Kroah-Hartman, open list:DESIGNWARE USB3 DRD IP DRIVER,
	open list, Jerry Huang, Rajesh Bhagat, Leo Li, Rob Herring,
	devicetree@vger.kernel.org
In-Reply-To: <VI1PR04MB1504776EF3D4D8C374F0C069F1290-mr6QIVyDiCF1me2Lu1FvAc9NdZoXdze2vxpqHgZTriW3zl9H0oFU5g@public.gmane.org>


Hi,

Ran Wang <ran.wang_1-3arQi8VN3Tc@public.gmane.org> writes:
>> Ran Wang <ran.wang_1-3arQi8VN3Tc@public.gmane.org> writes:
>> > Add support for USB3 snooping by asserting bits in register
>> > DWC3_GSBUSCFG0 for data and descriptor.
>> 
>> we know *how* to enable a feature :-) It's always the same, you fiddle with
>> some registers and it works. What you failed to tell us is:
>> 
>> a) WHY do you need this?
>> b) WHY do we need another DT property for this?
>> c) WHAT does this mean for PCI devices?
>
> So far I cannot have the answer for you, will get you back after some discussion
> with my colleagues.

IOW, you have no idea why you need this, right? We're not patching
things for the sake of patching things. We need to understand what these
changes mean to the HW before we send out a patch publicly.

Remember that the moment a patch like this is accepted, it has the
potential of changing behavior for *ALL* users.

>> > +	}
>> > +
>> > +	dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, cfg);
>> 
>> this will *always* read and write GSBUSCFG0 even for those platforms which
>> don't need to change anything on this register. You should just bail out early
>> if !dwc->dma_coherent
>> 
>> Also, I think dma_coherent is likely not the best name for this property.
>> 
>> Another question is: Why wasn't this setup properly during coreConsultant
>> instantiation of the RTL? Do you have devices on the market already that
>> need this or is this some early FPGA model or test-only ASIC?
>
> Yes, you are right. Actually I thought that all dwc3 IP  will have this register, and
> it can be controlled by DTS property. 

they all *have* the register, however, it's sort of expected that RTL
engineer will setup good defaults when instantiating the RTL using SNPS'
coreConsultant tool.

Does your platform work without this patch?

>> > +
>> >  /* Global Debug Queue/FIFO Space Available Register */
>> >  #define DWC3_GDBGFIFOSPACE_NUM(n)	((n) & 0x1f)
>> >  #define DWC3_GDBGFIFOSPACE_TYPE(n)	(((n) << 5) & 0x1e0)
>> > @@ -859,6 +867,7 @@ struct dwc3_scratchpad_array {
>> >   * 	3	- Reserved
>> >   * @imod_interval: set the interrupt moderation interval in 250ns
>> >   *                 increments or 0 to disable.
>> > + * @dma_coherent: set if enable dma-coherent.
>> 
>> you're not enabling dma coherency, you're enabling cache snooping. And
>> this property should describe that. Also, keep in mind that different devices
>> may want different cache types for each of those fields, so your property
>> would have to be a lot more complex. Something like:
>> 
>> 	snps,cache-type = <foobar "cacheable">, <baz "cacheable">, ...
>> 
>> Then driver would have to parse this properly to setup GSBUSCFG0.
>
> Got it, learn a lot, need more time to digest and test, thanks for
> your patiently explanation.

no problem, please figure out the answers to my previous questions,
without which I can't accept your patch.

>> In any
>> case, I still want to know why do you really need this? What's the reason?
>> What happens if you don't fix GSBUSCFG0? What's the value you have there
>> by default? Why isn't that default good enough?
>
> So far the Layerscape SoC (such as LS1088A) has enabled this feature and I
> have tested it. Once we add dma-coherent on DTS without this Patch, dwc3
> will fail on device enumeration as below:
> [   15.124031] xhci-hcd xhci-hcd.0.auto: Error while assigning device slot ID
> [   15.130912] xhci-hcd xhci-hcd.0.auto: Max number of devices this xHCI host supports is 127.
> [   15.139268] usb usb1-port1: couldn't allocate usb_device

okay, so without these changes, your host doesn't work. What is the
default value on your platform without these changes? (revert patch,
boot platform, let it fail, get register output from our regdump in debugfs)

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

^ permalink raw reply

* Re: [PATCH v2 3/5] misc: Add w2sg0004 (gps receiver) power control driver
From: kbuild test robot @ 2017-11-15 10:03 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, Rob Herring, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Russell King,
	H. Nikolaus Schaller, Thierry Reding, Jonathan Cameron,
	Maxime Ripard, Jarkko Sakkinen, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf
In-Reply-To: <1dd7e2ccf3d0361c0bb947bacbc2c00066e528c4.1510520397.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 3034 bytes --]

Hi Nikolaus,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v4.14 next-20171115]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/H-Nikolaus-Schaller/misc-new-serdev-based-drivers-for-Wi2Wi-w2sg00x4-GPS-module/20171115-115158
config: tile-allmodconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=tile 

All warnings (new ones prefixed by >>):

   drivers/misc/w2sg0004.c: In function 'w2sg_uart_receive_buf':
>> drivers/misc/w2sg0004.c:156:3: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t' [-Wformat]
   drivers/misc/w2sg0004.c:161:4: warning: format '%d' expects argument of type 'int', but argument 2 has type 'size_t' [-Wformat]
   drivers/misc/w2sg0004.c: At top level:
   drivers/misc/w2sg0004.c:487:12: warning: 'w2sg_suspend' defined but not used [-Wunused-function]
   drivers/misc/w2sg0004.c:523:12: warning: 'w2sg_resume' defined but not used [-Wunused-function]

vim +156 drivers/misc/w2sg0004.c

   125	
   126	static int w2sg_uart_receive_buf(struct serdev_device *serdev,
   127					const unsigned char *rxdata,
   128					size_t count)
   129	{
   130		struct w2sg_data *data =
   131			(struct w2sg_data *) serdev_device_get_drvdata(serdev);
   132	
   133		if (!data->requested && !data->is_on) {
   134			/*
   135			 * we have received characters while the w2sg
   136			 * should have been be turned off
   137			 */
   138			data->discard_count += count;
   139			if ((data->state == W2SG_IDLE) &&
   140			    time_after(jiffies,
   141			    data->last_toggle + data->backoff)) {
   142				/* Should be off by now, time to toggle again */
   143				pr_debug("w2sg00x4 has sent %d characters data although it should be off!\n",
   144					data->discard_count);
   145	
   146				data->discard_count = 0;
   147	
   148				data->is_on = true;
   149				data->backoff *= 2;
   150				if (!data->suspended)
   151					schedule_delayed_work(&data->work, 0);
   152			}
   153		} else if (data->open_count > 0) {
   154			int n;
   155	
 > 156			pr_debug("w2sg00x4: push %d chars to tty port\n", count);
   157	
   158			/* pass to user-space */
   159			n = tty_insert_flip_string(&data->port, rxdata, count);
   160			if (n != count)
   161				pr_err("w2sg00x4: did loose %d characters\n", count - n);
   162			tty_flip_buffer_push(&data->port);
   163			return n;
   164		}
   165	
   166		/* assume we have processed everything */
   167		return count;
   168	}
   169	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50385 bytes --]

^ permalink raw reply

* RE: [PATCH] usb: dwc3: Enable the USB snooping
From: Ran Wang @ 2017-11-15  9:33 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Greg Kroah-Hartman, open list:DESIGNWARE USB3 DRD IP DRIVER,
	open list, Jerry Huang, Rajesh Bhagat, Leo Li, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <87ineb9b5v.fsf-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Hi Balbi,

> -----Original Message-----
> From: Felipe Balbi [mailto:balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> Sent: Wednesday, November 15, 2017 4:52 PM
> To: Ran Wang <ran.wang_1-3arQi8VN3Tc@public.gmane.org>
> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>; open
> list:DESIGNWARE USB3 DRD IP DRIVER <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; open
> list <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; Jerry Huang <jerry.huang-3arQi8VN3Tc@public.gmane.org>;
> Rajesh Bhagat <rajesh.bhagat-3arQi8VN3Tc@public.gmane.org>; Leo Li <leoyang.li-3arQi8VN3Tc@public.gmane.org>; Ran
> Wang <ran.wang_1-3arQi8VN3Tc@public.gmane.org>; Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>;
> devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH] usb: dwc3: Enable the USB snooping
> 
> 
> Hi,
> 
> Ran Wang <ran.wang_1-3arQi8VN3Tc@public.gmane.org> writes:
> > Add support for USB3 snooping by asserting bits in register
> > DWC3_GSBUSCFG0 for data and descriptor.
> 
> we know *how* to enable a feature :-) It's always the same, you fiddle with
> some registers and it works. What you failed to tell us is:
> 
> a) WHY do you need this?
> b) WHY do we need another DT property for this?
> c) WHAT does this mean for PCI devices?

So far I cannot have the answer for you, will get you back after some discussion
with my colleagues.

> > Signed-off-by: Changming Huang <jerry.huang-3arQi8VN3Tc@public.gmane.org>
> > Signed-off-by: Rajesh Bhagat <rajesh.bhagat-3arQi8VN3Tc@public.gmane.org>
> > Signed-off-by: Ran Wang <ran.wang_1-3arQi8VN3Tc@public.gmane.org>
> > ---
> >  drivers/usb/dwc3/core.c | 24 ++++++++++++++++++++++++
> > drivers/usb/dwc3/core.h | 10 ++++++++++
> >  2 files changed, 34 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
> > 07832509584f..ffc078ab4a1c 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -236,6 +236,26 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> >  	return -ETIMEDOUT;
> >  }
> >
> > +/*
> > + * dwc3_enable_snooping - Enable snooping feature
> > + * @dwc3: Pointer to our controller context structure  */ static void
> > +dwc3_enable_snooping(struct dwc3 *dwc) {
> > +	u32 cfg;
> > +
> > +	cfg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);
> > +	if (dwc->dma_coherent) {
> > +		cfg &= ~DWC3_GSBUSCFG0_SNP_MASK;
> > +		cfg |= (AXI3_CACHE_TYPE_SNP <<
> DWC3_GSBUSCFG0_DATARD_SHIFT) |
> > +			(AXI3_CACHE_TYPE_SNP <<
> DWC3_GSBUSCFG0_DESCRD_SHIFT) |
> > +			(AXI3_CACHE_TYPE_SNP <<
> DWC3_GSBUSCFG0_DATAWR_SHIFT) |
> > +			(AXI3_CACHE_TYPE_SNP <<
> DWC3_GSBUSCFG0_DESCWR_SHIFT);
> 
> This "value << shift" looks super clumsy. I would rather have something akin
> to:
> 
> 	cfg |= DWC3_GSBUSCFG0_DATARD_CACHEABLE |
>         	DWC3_GSBUSCFG0_DESCRD_CACHEABLE ...
> 
> and so on.

Got it. 

> > +	}
> > +
> > +	dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, cfg);
> 
> this will *always* read and write GSBUSCFG0 even for those platforms which
> don't need to change anything on this register. You should just bail out early
> if !dwc->dma_coherent
> 
> Also, I think dma_coherent is likely not the best name for this property.
> 
> Another question is: Why wasn't this setup properly during coreConsultant
> instantiation of the RTL? Do you have devices on the market already that
> need this or is this some early FPGA model or test-only ASIC?

Yes, you are right. Actually I thought that all dwc3 IP  will have this register, and
it can be controlled by DTS property. 

> > @@ -776,6 +796,8 @@ static int dwc3_core_init(struct dwc3 *dwc)
> >  	/* Adjust Frame Length */
> >  	dwc3_frame_length_adjustment(dwc);
> >
> > +	dwc3_enable_snooping(dwc);
> > +
> >  	usb_phy_set_suspend(dwc->usb2_phy, 0);
> >  	usb_phy_set_suspend(dwc->usb3_phy, 0);
> >  	ret = phy_power_on(dwc->usb2_generic_phy);
> > @@ -1021,6 +1043,8 @@ static void dwc3_get_properties(struct dwc3
> *dwc)
> >  				&hird_threshold);
> >  	dwc->usb3_lpm_capable = device_property_read_bool(dev,
> >  				"snps,usb3_lpm_capable");
> > +	dwc->dma_coherent = device_property_read_bool(dev,
> > +				"dma-coherent");
> >
> >  	dwc->disable_scramble_quirk = device_property_read_bool(dev,
> >  				"snps,disable_scramble_quirk");
> > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index
> > 4a4a4c98508c..6e6a66650e53 100644
> > --- a/drivers/usb/dwc3/core.h
> > +++ b/drivers/usb/dwc3/core.h
> > @@ -153,6 +153,14 @@
> >
> >  /* Bit fields */
> >
> > +/* Global SoC Bus Configuration Register 0 */
> > +#define AXI3_CACHE_TYPE_SNP		0x2 /* cacheable */
> > +#define DWC3_GSBUSCFG0_DATARD_SHIFT	28
> > +#define DWC3_GSBUSCFG0_DESCRD_SHIFT	24
> > +#define DWC3_GSBUSCFG0_DATAWR_SHIFT	20
> > +#define DWC3_GSBUSCFG0_DESCWR_SHIFT	16
> > +#define DWC3_GSBUSCFG0_SNP_MASK		0xffff0000
> 
> 
> 
> > +
> >  /* Global Debug Queue/FIFO Space Available Register */
> >  #define DWC3_GDBGFIFOSPACE_NUM(n)	((n) & 0x1f)
> >  #define DWC3_GDBGFIFOSPACE_TYPE(n)	(((n) << 5) & 0x1e0)
> > @@ -859,6 +867,7 @@ struct dwc3_scratchpad_array {
> >   * 	3	- Reserved
> >   * @imod_interval: set the interrupt moderation interval in 250ns
> >   *                 increments or 0 to disable.
> > + * @dma_coherent: set if enable dma-coherent.
> 
> you're not enabling dma coherency, you're enabling cache snooping. And
> this property should describe that. Also, keep in mind that different devices
> may want different cache types for each of those fields, so your property
> would have to be a lot more complex. Something like:
> 
> 	snps,cache-type = <foobar "cacheable">, <baz "cacheable">, ...
> 
> Then driver would have to parse this properly to setup GSBUSCFG0.

Got it, learn a lot, need more time to digest and test, thanks for your patiently explanation.

> In any
> case, I still want to know why do you really need this? What's the reason?
> What happens if you don't fix GSBUSCFG0? What's the value you have there
> by default? Why isn't that default good enough?

So far the Layerscape SoC (such as LS1088A) has enabled this feature and I
have tested it. Once we add dma-coherent on DTS without this Patch, dwc3
will fail on device enumeration as below:
[   15.124031] xhci-hcd xhci-hcd.0.auto: Error while assigning device slot ID
[   15.130912] xhci-hcd xhci-hcd.0.auto: Max number of devices this xHCI host supports is 127.
[   15.139268] usb usb1-port1: couldn't allocate usb_device

> 
> ps: since you're fiddling with DT, you should also include devicetree@vger

OK

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

^ permalink raw reply

* Re: [PATCH v2 3/5] misc: Add w2sg0004 (gps receiver) power control driver
From: kbuild test robot @ 2017-11-15  9:16 UTC (permalink / raw)
  Cc: kbuild-all, Rob Herring, Mark Rutland, Benoît Cousson,
	Tony Lindgren, Russell King, H. Nikolaus Schaller, Thierry Reding,
	Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen, devicetree,
	linux-kernel, linux-omap, letux-kernel, kernel
In-Reply-To: <1dd7e2ccf3d0361c0bb947bacbc2c00066e528c4.1510520397.git.hns@goldelico.com>

[-- Attachment #1: Type: text/plain, Size: 7687 bytes --]

Hi Nikolaus,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v4.14]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/H-Nikolaus-Schaller/misc-new-serdev-based-drivers-for-Wi2Wi-w2sg00x4-GPS-module/20171115-115158
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:13:0,
                    from include/linux/delay.h:21,
                    from drivers/misc/w2sg0004.c:25:
   drivers/misc/w2sg0004.c: In function 'w2sg_uart_receive_buf':
>> drivers/misc/w2sg0004.c:156:12: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=]
      pr_debug("w2sg00x4: push %d chars to tty port\n", count);
               ^
   include/linux/printk.h:285:21: note: in definition of macro 'pr_fmt'
    #define pr_fmt(fmt) fmt
                        ^~~
   include/linux/printk.h:333:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
>> drivers/misc/w2sg0004.c:156:3: note: in expansion of macro 'pr_debug'
      pr_debug("w2sg00x4: push %d chars to tty port\n", count);
      ^~~~~~~~
   In file included from include/linux/printk.h:6:0,
                    from include/linux/kernel.h:13,
                    from include/linux/delay.h:21,
                    from drivers/misc/w2sg0004.c:25:
   include/linux/kern_levels.h:4:18: warning: format '%d' expects argument of type 'int', but argument 2 has type 'size_t {aka long unsigned int}' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/kern_levels.h:10:18: note: in expansion of macro 'KERN_SOH'
    #define KERN_ERR KERN_SOH "3" /* error conditions */
                     ^~~~~~~~
   include/linux/printk.h:301:9: note: in expansion of macro 'KERN_ERR'
     printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
            ^~~~~~~~
>> drivers/misc/w2sg0004.c:161:4: note: in expansion of macro 'pr_err'
       pr_err("w2sg00x4: did loose %d characters\n", count - n);
       ^~~~~~

vim +156 drivers/misc/w2sg0004.c

  > 25	#include <linux/delay.h>
    26	#include <linux/err.h>
    27	#include <linux/interrupt.h>
    28	#include <linux/irq.h>
    29	#include <linux/module.h>
    30	#include <linux/of.h>
    31	#include <linux/of_irq.h>
    32	#include <linux/of_gpio.h>
    33	#include <linux/platform_device.h>
    34	#include <linux/rfkill.h>
    35	#include <linux/serdev.h>
    36	#include <linux/sched.h>
    37	#include <linux/slab.h>
    38	#include <linux/tty.h>
    39	#include <linux/tty_flip.h>
    40	#include <linux/w2sg0004.h>
    41	#include <linux/workqueue.h>
    42	
    43	/*
    44	 * There seems to be restrictions on how quickly we can toggle the
    45	 * on/off line.  data sheets says "two rtc ticks", whatever that means.
    46	 * If we do it too soon it doesn't work.
    47	 * So we have a state machine which uses the common work queue to ensure
    48	 * clean transitions.
    49	 * When a change is requested we record that request and only act on it
    50	 * once the previous change has completed.
    51	 * A change involves a 10ms low pulse, and a 990ms raised level, so only
    52	 * one change per second.
    53	 */
    54	
    55	enum w2sg_state {
    56		W2SG_IDLE,	/* is not changing state */
    57		W2SG_PULSE,	/* activate on/off impulse */
    58		W2SG_NOPULSE	/* deactivate on/off impulse */
    59	};
    60	
    61	struct w2sg_data {
    62		struct		rfkill *rf_kill;
    63		struct		regulator *lna_regulator;
    64		int		lna_blocked;	/* rfkill block gps active */
    65		int		lna_is_off;	/* LNA is currently off */
    66		int		is_on;		/* current state (0/1) */
    67		unsigned long	last_toggle;
    68		unsigned long	backoff;	/* time to wait since last_toggle */
    69		int		on_off_gpio;	/* the on-off gpio number */
    70		struct		serdev_device *uart;	/* uart connected to the chip */
    71		struct		tty_driver *tty_drv;	/* this is the user space tty */
    72		struct		device *dev;	/* from tty_port_register_device() */
    73		struct		tty_port port;
    74		int		open_count;	/* how often we were opened */
    75		enum		w2sg_state state;
    76		int		requested;	/* requested state (0/1) */
    77		int		suspended;
    78		struct delayed_work work;
    79		int		discard_count;
    80	};
    81	
    82	static struct w2sg_data *w2sg_by_minor[1];
    83	
    84	static int w2sg_set_lna_power(struct w2sg_data *data)
    85	{
    86		int ret = 0;
    87		int off = data->suspended || !data->requested || data->lna_blocked;
    88	
    89		pr_debug("%s: %s\n", __func__, off ? "off" : "on");
    90	
    91		if (off != data->lna_is_off) {
    92			data->lna_is_off = off;
    93			if (!IS_ERR_OR_NULL(data->lna_regulator)) {
    94				if (off)
    95					regulator_disable(data->lna_regulator);
    96				else
    97					ret = regulator_enable(data->lna_regulator);
    98			}
    99		}
   100	
   101		return ret;
   102	}
   103	
   104	static void w2sg_set_power(void *pdata, int val)
   105	{
   106		struct w2sg_data *data = (struct w2sg_data *) pdata;
   107	
   108		pr_debug("%s to state=%d (requested=%d)\n", __func__, val, data->requested);
   109	
   110		if (val && !data->requested) {
   111			data->requested = true;
   112		} else if (!val && data->requested) {
   113			data->backoff = HZ;
   114			data->requested = false;
   115		} else
   116			return;
   117	
   118		pr_debug("w2sg00x4 scheduled for %d\n", data->requested);
   119	
   120		if (!data->suspended)
   121			schedule_delayed_work(&data->work, 0);
   122	}
   123	
   124	/* called each time data is received by the UART (i.e. sent by the w2sg0004) */
   125	
   126	static int w2sg_uart_receive_buf(struct serdev_device *serdev,
   127					const unsigned char *rxdata,
   128					size_t count)
   129	{
   130		struct w2sg_data *data =
   131			(struct w2sg_data *) serdev_device_get_drvdata(serdev);
   132	
   133		if (!data->requested && !data->is_on) {
   134			/*
   135			 * we have received characters while the w2sg
   136			 * should have been be turned off
   137			 */
   138			data->discard_count += count;
   139			if ((data->state == W2SG_IDLE) &&
   140			    time_after(jiffies,
   141			    data->last_toggle + data->backoff)) {
   142				/* Should be off by now, time to toggle again */
   143				pr_debug("w2sg00x4 has sent %d characters data although it should be off!\n",
   144					data->discard_count);
   145	
   146				data->discard_count = 0;
   147	
   148				data->is_on = true;
   149				data->backoff *= 2;
   150				if (!data->suspended)
   151					schedule_delayed_work(&data->work, 0);
   152			}
   153		} else if (data->open_count > 0) {
   154			int n;
   155	
 > 156			pr_debug("w2sg00x4: push %d chars to tty port\n", count);
   157	
   158			/* pass to user-space */
   159			n = tty_insert_flip_string(&data->port, rxdata, count);
   160			if (n != count)
 > 161				pr_err("w2sg00x4: did loose %d characters\n", count - n);
   162			tty_flip_buffer_push(&data->port);
   163			return n;
   164		}
   165	
   166		/* assume we have processed everything */
   167		return count;
   168	}
   169	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61726 bytes --]

^ permalink raw reply

* Re: [PATCH] usb: dwc3: Enable the USB snooping
From: Felipe Balbi @ 2017-11-15  8:52 UTC (permalink / raw)
  Cc: Greg Kroah-Hartman, open list:DESIGNWARE USB3 DRD IP DRIVER,
	open list, Changming Huang, Rajesh Bhagat, Li Yang, Ran Wang,
	Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171115060459.45375-1-ran.wang_1-3arQi8VN3Tc@public.gmane.org>


Hi,

Ran Wang <ran.wang_1-3arQi8VN3Tc@public.gmane.org> writes:
> Add support for USB3 snooping by asserting bits
> in register DWC3_GSBUSCFG0 for data and descriptor.

we know *how* to enable a feature :-) It's always the same, you fiddle
with some registers and it works. What you failed to tell us is:

a) WHY do you need this?
b) WHY do we need another DT property for this?
c) WHAT does this mean for PCI devices?

> Signed-off-by: Changming Huang <jerry.huang-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Ran Wang <ran.wang_1-3arQi8VN3Tc@public.gmane.org>
> ---
>  drivers/usb/dwc3/core.c | 24 ++++++++++++++++++++++++
>  drivers/usb/dwc3/core.h | 10 ++++++++++
>  2 files changed, 34 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 07832509584f..ffc078ab4a1c 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -236,6 +236,26 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
>  	return -ETIMEDOUT;
>  }
>  
> +/*
> + * dwc3_enable_snooping - Enable snooping feature
> + * @dwc3: Pointer to our controller context structure
> + */
> +static void dwc3_enable_snooping(struct dwc3 *dwc)
> +{
> +	u32 cfg;
> +
> +	cfg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);
> +	if (dwc->dma_coherent) {
> +		cfg &= ~DWC3_GSBUSCFG0_SNP_MASK;
> +		cfg |= (AXI3_CACHE_TYPE_SNP << DWC3_GSBUSCFG0_DATARD_SHIFT) |
> +			(AXI3_CACHE_TYPE_SNP << DWC3_GSBUSCFG0_DESCRD_SHIFT) |
> +			(AXI3_CACHE_TYPE_SNP << DWC3_GSBUSCFG0_DATAWR_SHIFT) |
> +			(AXI3_CACHE_TYPE_SNP << DWC3_GSBUSCFG0_DESCWR_SHIFT);

This "value << shift" looks super clumsy. I would rather have something
akin to:

	cfg |= DWC3_GSBUSCFG0_DATARD_CACHEABLE |
        	DWC3_GSBUSCFG0_DESCRD_CACHEABLE ...

and so on.

> +	}
> +
> +	dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, cfg);

this will *always* read and write GSBUSCFG0 even for those platforms
which don't need to change anything on this register. You should just
bail out early if !dwc->dma_coherent

Also, I think dma_coherent is likely not the best name for this property.

Another question is: Why wasn't this setup properly during
coreConsultant instantiation of the RTL? Do you have devices on the
market already that need this or is this some early FPGA model or
test-only ASIC?

> @@ -776,6 +796,8 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  	/* Adjust Frame Length */
>  	dwc3_frame_length_adjustment(dwc);
>  
> +	dwc3_enable_snooping(dwc);
> +
>  	usb_phy_set_suspend(dwc->usb2_phy, 0);
>  	usb_phy_set_suspend(dwc->usb3_phy, 0);
>  	ret = phy_power_on(dwc->usb2_generic_phy);
> @@ -1021,6 +1043,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>  				&hird_threshold);
>  	dwc->usb3_lpm_capable = device_property_read_bool(dev,
>  				"snps,usb3_lpm_capable");
> +	dwc->dma_coherent = device_property_read_bool(dev,
> +				"dma-coherent");
>  
>  	dwc->disable_scramble_quirk = device_property_read_bool(dev,
>  				"snps,disable_scramble_quirk");
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 4a4a4c98508c..6e6a66650e53 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -153,6 +153,14 @@
>  
>  /* Bit fields */
>  
> +/* Global SoC Bus Configuration Register 0 */
> +#define AXI3_CACHE_TYPE_SNP		0x2 /* cacheable */
> +#define DWC3_GSBUSCFG0_DATARD_SHIFT	28
> +#define DWC3_GSBUSCFG0_DESCRD_SHIFT	24
> +#define DWC3_GSBUSCFG0_DATAWR_SHIFT	20
> +#define DWC3_GSBUSCFG0_DESCWR_SHIFT	16
> +#define DWC3_GSBUSCFG0_SNP_MASK		0xffff0000



> +
>  /* Global Debug Queue/FIFO Space Available Register */
>  #define DWC3_GDBGFIFOSPACE_NUM(n)	((n) & 0x1f)
>  #define DWC3_GDBGFIFOSPACE_TYPE(n)	(((n) << 5) & 0x1e0)
> @@ -859,6 +867,7 @@ struct dwc3_scratchpad_array {
>   * 	3	- Reserved
>   * @imod_interval: set the interrupt moderation interval in 250ns
>   *                 increments or 0 to disable.
> + * @dma_coherent: set if enable dma-coherent.

you're not enabling dma coherency, you're enabling cache snooping. And
this property should describe that. Also, keep in mind that different
devices may want different cache types for each of those fields, so your
property would have to be a lot more complex. Something like:

	snps,cache-type = <foobar "cacheable">, <baz "cacheable">, ...

Then driver would have to parse this properly to setup GSBUSCFG0. In any
case, I still want to know why do you really need this? What's the
reason? What happens if you don't fix GSBUSCFG0? What's the value you
have there by default? Why isn't that default good enough?

ps: since you're fiddling with DT, you should also include
devicetree@vger

-- 
balbi
--
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 v6 2/2] watchdog: Add Spreadtrum watchdog driver
From: Eric Long @ 2017-11-15  7:48 UTC (permalink / raw)
  To: Guenter Roeck, eric.long-lxIno14LUO0EEoCn2XhGlw
  Cc: Wim Van Sebroeck, Rob Herring, Mark Rutland,
	baolin.wang-QSEj5FYQhm4dnm+yROfE0A,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171110210032.GB4591-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

Hi,

Thanks for Guenter's review and detail comments.
Please help to apply this patch if there is no any other comments.

Best regards,
Eric Long

On Fri, Nov 10, 2017 at 01:00:32PM -0800, Guenter Roeck wrote:
> On Mon, Nov 06, 2017 at 10:46:28AM +0800, Eric Long wrote:
> > This patch adds the watchdog driver for Spreadtrum SC9860 platform.
> > 
> > Signed-off-by: Eric Long <eric.long-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
> 
> Reviewed-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> 
> > ---
> > Changes since v5:
> >  - Modify the "irq" type as int type.
> >  - Delete unused api sprd_wdt_is_running().
> > 
> > Changes since v4:
> >  - Remove sprd_wdt_remove().
> >  - Add devm_add_action() for sprd_wdt_disable().
> > 
> > Changes since v3:
> >  - Update Kconfig SPRD_WATCHDOG help messages.
> >  - Correct the wrong spell words.
> >  - Rename SPRD_WDT_CNT_HIGH_VALUE as SPRD_WDT_CNT_HIGH_SHIFT.
> >  - Remove unused macor.
> >  - Update sprd_wdt_set_pretimeout() api.
> >  - Add wdt->wdd.timeout default value.
> >  - Use devm_watchdog_register_device() to register wdt device.
> >  - If module does not support NOWAYOUT, disable wdt when remove this driver.
> >  - Call sprd_wdt_disable() every wdt suspend.
> > 
> > Changes since v2:
> >  - Rename all the macors, add SPRD tag at the head of the macro names.
> >  - Rename SPRD_WDT_CLK as SPRD_WTC_CNT_STEP.
> >  - Remove the code which check timeout value at the wrong place.
> >  - Add min/max timeout value limit.
> >  - Remove set WDOG_HW_RUNNING status at sprd_wdt_enable().
> >  - Add timeout/pretimeout judgment when set them.
> >  - Support WATCHDOG_NOWAYOUT status.
> > 
> > Changes since v1:
> >  - Use pretimeout instead of own implementation.
> >  - Fix timeout loop when loading timeout values.
> >  - use the infrastructure to read and set "timeout-sec" property.
> >  - Add conditions when start or stop watchdog.
> >  - Change the position of enabling watchdog.
> >  - Other optimization.
> > ---
> >  drivers/watchdog/Kconfig    |   8 +
> >  drivers/watchdog/Makefile   |   1 +
> >  drivers/watchdog/sprd_wdt.c | 399 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 408 insertions(+)
> >  create mode 100644 drivers/watchdog/sprd_wdt.c
> > 
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index c722cbf..3367a8c 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -787,6 +787,14 @@ config UNIPHIER_WATCHDOG
> >  	  To compile this driver as a module, choose M here: the
> >  	  module will be called uniphier_wdt.
> >  
> > +config SPRD_WATCHDOG
> > +	tristate "Spreadtrum watchdog support"
> > +	depends on ARCH_SPRD || COMPILE_TEST
> > +	select WATCHDOG_CORE
> > +	help
> > +	  Say Y here to include watchdog timer supported
> > +	  by Spreadtrum system.
> > +
> >  # AVR32 Architecture
> >  
> >  config AT32AP700X_WDT
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index 56adf9f..187cca2 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -87,6 +87,7 @@ obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
> >  obj-$(CONFIG_ZX2967_WATCHDOG) += zx2967_wdt.o
> >  obj-$(CONFIG_STM32_WATCHDOG) += stm32_iwdg.o
> >  obj-$(CONFIG_UNIPHIER_WATCHDOG) += uniphier_wdt.o
> > +obj-$(CONFIG_SPRD_WATCHDOG) += sprd_wdt.o
> >  
> >  # AVR32 Architecture
> >  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> > diff --git a/drivers/watchdog/sprd_wdt.c b/drivers/watchdog/sprd_wdt.c
> > new file mode 100644
> > index 0000000..a8b280f
> > --- /dev/null
> > +++ b/drivers/watchdog/sprd_wdt.c
> > @@ -0,0 +1,399 @@
> > +/*
> > + * Spreadtrum watchdog driver
> > + * Copyright (C) 2017 Spreadtrum - http://www.spreadtrum.com
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * General Public License for more details.
> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/clk.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/watchdog.h>
> > +
> > +#define SPRD_WDT_LOAD_LOW		0x0
> > +#define SPRD_WDT_LOAD_HIGH		0x4
> > +#define SPRD_WDT_CTRL			0x8
> > +#define SPRD_WDT_INT_CLR		0xc
> > +#define SPRD_WDT_INT_RAW		0x10
> > +#define SPRD_WDT_INT_MSK		0x14
> > +#define SPRD_WDT_CNT_LOW		0x18
> > +#define SPRD_WDT_CNT_HIGH		0x1c
> > +#define SPRD_WDT_LOCK			0x20
> > +#define SPRD_WDT_IRQ_LOAD_LOW		0x2c
> > +#define SPRD_WDT_IRQ_LOAD_HIGH		0x30
> > +
> > +/* WDT_CTRL */
> > +#define SPRD_WDT_INT_EN_BIT		BIT(0)
> > +#define SPRD_WDT_CNT_EN_BIT		BIT(1)
> > +#define SPRD_WDT_NEW_VER_EN		BIT(2)
> > +#define SPRD_WDT_RST_EN_BIT		BIT(3)
> > +
> > +/* WDT_INT_CLR */
> > +#define SPRD_WDT_INT_CLEAR_BIT		BIT(0)
> > +#define SPRD_WDT_RST_CLEAR_BIT		BIT(3)
> > +
> > +/* WDT_INT_RAW */
> > +#define SPRD_WDT_INT_RAW_BIT		BIT(0)
> > +#define SPRD_WDT_RST_RAW_BIT		BIT(3)
> > +#define SPRD_WDT_LD_BUSY_BIT		BIT(4)
> > +
> > +/* 1s equal to 32768 counter steps */
> > +#define SPRD_WDT_CNT_STEP		32768
> > +
> > +#define SPRD_WDT_UNLOCK_KEY		0xe551
> > +#define SPRD_WDT_MIN_TIMEOUT		3
> > +#define SPRD_WDT_MAX_TIMEOUT		60
> > +
> > +#define SPRD_WDT_CNT_HIGH_SHIFT		16
> > +#define SPRD_WDT_LOW_VALUE_MASK		GENMASK(15, 0)
> > +#define SPRD_WDT_LOAD_TIMEOUT		1000
> > +
> > +struct sprd_wdt {
> > +	void __iomem *base;
> > +	struct watchdog_device wdd;
> > +	struct clk *enable;
> > +	struct clk *rtc_enable;
> > +	int irq;
> > +};
> > +
> > +static inline struct sprd_wdt *to_sprd_wdt(struct watchdog_device *wdd)
> > +{
> > +	return container_of(wdd, struct sprd_wdt, wdd);
> > +}
> > +
> > +static inline void sprd_wdt_lock(void __iomem *addr)
> > +{
> > +	writel_relaxed(0x0, addr + SPRD_WDT_LOCK);
> > +}
> > +
> > +static inline void sprd_wdt_unlock(void __iomem *addr)
> > +{
> > +	writel_relaxed(SPRD_WDT_UNLOCK_KEY, addr + SPRD_WDT_LOCK);
> > +}
> > +
> > +static irqreturn_t sprd_wdt_isr(int irq, void *dev_id)
> > +{
> > +	struct sprd_wdt *wdt = (struct sprd_wdt *)dev_id;
> > +
> > +	sprd_wdt_unlock(wdt->base);
> > +	writel_relaxed(SPRD_WDT_INT_CLEAR_BIT, wdt->base + SPRD_WDT_INT_CLR);
> > +	sprd_wdt_lock(wdt->base);
> > +	watchdog_notify_pretimeout(&wdt->wdd);
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static u32 sprd_wdt_get_cnt_value(struct sprd_wdt *wdt)
> > +{
> > +	u32 val;
> > +
> > +	val = readl_relaxed(wdt->base + SPRD_WDT_CNT_HIGH) <<
> > +		SPRD_WDT_CNT_HIGH_SHIFT;
> > +	val |= readl_relaxed(wdt->base + SPRD_WDT_CNT_LOW) &
> > +		SPRD_WDT_LOW_VALUE_MASK;
> > +
> > +	return val;
> > +}
> > +
> > +static int sprd_wdt_load_value(struct sprd_wdt *wdt, u32 timeout,
> > +			       u32 pretimeout)
> > +{
> > +	u32 val, delay_cnt = 0;
> > +	u32 tmr_step = timeout * SPRD_WDT_CNT_STEP;
> > +	u32 prtmr_step = pretimeout * SPRD_WDT_CNT_STEP;
> > +
> > +	sprd_wdt_unlock(wdt->base);
> > +	writel_relaxed((tmr_step >> SPRD_WDT_CNT_HIGH_SHIFT) &
> > +		      SPRD_WDT_LOW_VALUE_MASK, wdt->base + SPRD_WDT_LOAD_HIGH);
> > +	writel_relaxed((tmr_step & SPRD_WDT_LOW_VALUE_MASK),
> > +		       wdt->base + SPRD_WDT_LOAD_LOW);
> > +	writel_relaxed((prtmr_step >> SPRD_WDT_CNT_HIGH_SHIFT) &
> > +			SPRD_WDT_LOW_VALUE_MASK,
> > +		       wdt->base + SPRD_WDT_IRQ_LOAD_HIGH);
> > +	writel_relaxed(prtmr_step & SPRD_WDT_LOW_VALUE_MASK,
> > +		       wdt->base + SPRD_WDT_IRQ_LOAD_LOW);
> > +	sprd_wdt_lock(wdt->base);
> > +
> > +	/*
> > +	 * Waiting the load value operation done,
> > +	 * it needs two or three RTC clock cycles.
> > +	 */
> > +	do {
> > +		val = readl_relaxed(wdt->base + SPRD_WDT_INT_RAW);
> > +		if (!(val & SPRD_WDT_LD_BUSY_BIT))
> > +			break;
> > +
> > +		cpu_relax();
> > +	} while (delay_cnt++ < SPRD_WDT_LOAD_TIMEOUT);
> > +
> > +	if (delay_cnt >= SPRD_WDT_LOAD_TIMEOUT)
> > +		return -EBUSY;
> > +	return 0;
> > +}
> > +
> > +static int sprd_wdt_enable(struct sprd_wdt *wdt)
> > +{
> > +	u32 val;
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(wdt->enable);
> > +	if (ret)
> > +		return ret;
> > +	ret = clk_prepare_enable(wdt->rtc_enable);
> > +	if (ret)
> > +		return ret;
> > +
> > +	sprd_wdt_unlock(wdt->base);
> > +	val = readl_relaxed(wdt->base + SPRD_WDT_CTRL);
> > +	val |= SPRD_WDT_NEW_VER_EN;
> > +	writel_relaxed(val, wdt->base + SPRD_WDT_CTRL);
> > +	sprd_wdt_lock(wdt->base);
> > +	return 0;
> > +}
> > +
> > +static void sprd_wdt_disable(void *_data)
> > +{
> > +	struct sprd_wdt *wdt = _data;
> > +
> > +	sprd_wdt_unlock(wdt->base);
> > +	writel_relaxed(0x0, wdt->base + SPRD_WDT_CTRL);
> > +	sprd_wdt_lock(wdt->base);
> > +
> > +	clk_disable_unprepare(wdt->rtc_enable);
> > +	clk_disable_unprepare(wdt->enable);
> > +}
> > +
> > +static int sprd_wdt_start(struct watchdog_device *wdd)
> > +{
> > +	struct sprd_wdt *wdt = to_sprd_wdt(wdd);
> > +	u32 val;
> > +	int ret;
> > +
> > +	ret = sprd_wdt_load_value(wdt, wdd->timeout, wdd->pretimeout);
> > +	if (ret)
> > +		return ret;
> > +
> > +	sprd_wdt_unlock(wdt->base);
> > +	val = readl_relaxed(wdt->base + SPRD_WDT_CTRL);
> > +	val |= SPRD_WDT_CNT_EN_BIT | SPRD_WDT_INT_EN_BIT | SPRD_WDT_RST_EN_BIT;
> > +	writel_relaxed(val, wdt->base + SPRD_WDT_CTRL);
> > +	sprd_wdt_lock(wdt->base);
> > +	set_bit(WDOG_HW_RUNNING, &wdd->status);
> > +
> > +	return 0;
> > +}
> > +
> > +static int sprd_wdt_stop(struct watchdog_device *wdd)
> > +{
> > +	struct sprd_wdt *wdt = to_sprd_wdt(wdd);
> > +	u32 val;
> > +
> > +	sprd_wdt_unlock(wdt->base);
> > +	val = readl_relaxed(wdt->base + SPRD_WDT_CTRL);
> > +	val &= ~(SPRD_WDT_CNT_EN_BIT | SPRD_WDT_RST_EN_BIT |
> > +		SPRD_WDT_INT_EN_BIT);
> > +	writel_relaxed(val, wdt->base + SPRD_WDT_CTRL);
> > +	sprd_wdt_lock(wdt->base);
> > +	return 0;
> > +}
> > +
> > +static int sprd_wdt_set_timeout(struct watchdog_device *wdd,
> > +				u32 timeout)
> > +{
> > +	struct sprd_wdt *wdt = to_sprd_wdt(wdd);
> > +
> > +	if (timeout == wdd->timeout)
> > +		return 0;
> > +
> > +	wdd->timeout = timeout;
> > +
> > +	return sprd_wdt_load_value(wdt, timeout, wdd->pretimeout);
> > +}
> > +
> > +static int sprd_wdt_set_pretimeout(struct watchdog_device *wdd,
> > +				   u32 new_pretimeout)
> > +{
> > +	struct sprd_wdt *wdt = to_sprd_wdt(wdd);
> > +
> > +	if (new_pretimeout < wdd->min_timeout)
> > +		return -EINVAL;
> > +
> > +	wdd->pretimeout = new_pretimeout;
> > +
> > +	return sprd_wdt_load_value(wdt, wdd->timeout, new_pretimeout);
> > +}
> > +
> > +static u32 sprd_wdt_get_timeleft(struct watchdog_device *wdd)
> > +{
> > +	struct sprd_wdt *wdt = to_sprd_wdt(wdd);
> > +	u32 val;
> > +
> > +	val = sprd_wdt_get_cnt_value(wdt);
> > +	val = val / SPRD_WDT_CNT_STEP;
> > +
> > +	return val;
> > +}
> > +
> > +static const struct watchdog_ops sprd_wdt_ops = {
> > +	.owner = THIS_MODULE,
> > +	.start = sprd_wdt_start,
> > +	.stop = sprd_wdt_stop,
> > +	.set_timeout = sprd_wdt_set_timeout,
> > +	.set_pretimeout = sprd_wdt_set_pretimeout,
> > +	.get_timeleft = sprd_wdt_get_timeleft,
> > +};
> > +
> > +static const struct watchdog_info sprd_wdt_info = {
> > +	.options = WDIOF_SETTIMEOUT |
> > +		   WDIOF_PRETIMEOUT |
> > +		   WDIOF_MAGICCLOSE |
> > +		   WDIOF_KEEPALIVEPING,
> > +	.identity = "Spreadtrum Watchdog Timer",
> > +};
> > +
> > +static int sprd_wdt_probe(struct platform_device *pdev)
> > +{
> > +	struct resource *wdt_res;
> > +	struct sprd_wdt *wdt;
> > +	int ret;
> > +
> > +	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> > +	if (!wdt)
> > +		return -ENOMEM;
> > +
> > +	wdt_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	wdt->base = devm_ioremap_resource(&pdev->dev, wdt_res);
> > +	if (IS_ERR(wdt->base)) {
> > +		dev_err(&pdev->dev, "failed to map memory resource\n");
> > +		return PTR_ERR(wdt->base);
> > +	}
> > +
> > +	wdt->enable = devm_clk_get(&pdev->dev, "enable");
> > +	if (IS_ERR(wdt->enable)) {
> > +		dev_err(&pdev->dev, "can't get the enable clock\n");
> > +		return PTR_ERR(wdt->enable);
> > +	}
> > +
> > +	wdt->rtc_enable = devm_clk_get(&pdev->dev, "rtc_enable");
> > +	if (IS_ERR(wdt->rtc_enable)) {
> > +		dev_err(&pdev->dev, "can't get the rtc enable clock\n");
> > +		return PTR_ERR(wdt->rtc_enable);
> > +	}
> > +
> > +	wdt->irq = platform_get_irq(pdev, 0);
> > +	if (wdt->irq < 0) {
> > +		dev_err(&pdev->dev, "failed to get IRQ resource\n");
> > +		return wdt->irq;
> > +	}
> > +
> > +	ret = devm_request_irq(&pdev->dev, wdt->irq, sprd_wdt_isr,
> > +			       IRQF_NO_SUSPEND, "sprd-wdt", (void *)wdt);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "failed to register irq\n");
> > +		return ret;
> > +	}
> > +
> > +	wdt->wdd.info = &sprd_wdt_info;
> > +	wdt->wdd.ops = &sprd_wdt_ops;
> > +	wdt->wdd.parent = &pdev->dev;
> > +	wdt->wdd.min_timeout = SPRD_WDT_MIN_TIMEOUT;
> > +	wdt->wdd.max_timeout = SPRD_WDT_MAX_TIMEOUT;
> > +	wdt->wdd.timeout = SPRD_WDT_MAX_TIMEOUT;
> > +
> > +	ret = sprd_wdt_enable(wdt);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "failed to enable wdt\n");
> > +		return ret;
> > +	}
> > +	ret = devm_add_action(&pdev->dev, sprd_wdt_disable, wdt);
> > +	if (ret) {
> > +		sprd_wdt_disable(wdt);
> > +		dev_err(&pdev->dev, "Failed to add wdt disable action\n");
> > +		return ret;
> > +	}
> > +
> > +	watchdog_set_nowayout(&wdt->wdd, WATCHDOG_NOWAYOUT);
> > +	watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev);
> > +
> > +	ret = devm_watchdog_register_device(&pdev->dev, &wdt->wdd);
> > +	if (ret) {
> > +		sprd_wdt_disable(wdt);
> > +		dev_err(&pdev->dev, "failed to register watchdog\n");
> > +		return ret;
> > +	}
> > +	platform_set_drvdata(pdev, wdt);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused sprd_wdt_pm_suspend(struct device *dev)
> > +{
> > +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> > +	struct sprd_wdt *wdt = dev_get_drvdata(dev);
> > +
> > +	if (watchdog_active(wdd))
> > +		sprd_wdt_stop(&wdt->wdd);
> > +	sprd_wdt_disable(wdt);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused sprd_wdt_pm_resume(struct device *dev)
> > +{
> > +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> > +	struct sprd_wdt *wdt = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = sprd_wdt_enable(wdt);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (watchdog_active(wdd)) {
> > +		ret = sprd_wdt_start(&wdt->wdd);
> > +		if (ret) {
> > +			sprd_wdt_disable(wdt);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct dev_pm_ops sprd_wdt_pm_ops = {
> > +	SET_SYSTEM_SLEEP_PM_OPS(sprd_wdt_pm_suspend,
> > +				sprd_wdt_pm_resume)
> > +};
> > +
> > +static const struct of_device_id sprd_wdt_match_table[] = {
> > +	{ .compatible = "sprd,sp9860-wdt", },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, sprd_wdt_match_table);
> > +
> > +static struct platform_driver sprd_watchdog_driver = {
> > +	.probe	= sprd_wdt_probe,
> > +	.driver	= {
> > +		.name = "sprd-wdt",
> > +		.of_match_table = sprd_wdt_match_table,
> > +		.pm = &sprd_wdt_pm_ops,
> > +	},
> > +};
> > +module_platform_driver(sprd_watchdog_driver);
> > +
> > +MODULE_AUTHOR("Eric Long <eric.long-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>");
> > +MODULE_DESCRIPTION("Spreadtrum Watchdog Timer Controller Driver");
> > +MODULE_LICENSE("GPL v2");
> > -- 
> > 1.9.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" 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 v3 1/3] leds: core: Introduce generic pattern interface
From: Greg KH @ 2017-11-15  7:36 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, linux-kernel,
	linux-leds, linux-arm-msm, Rob Herring, Mark Rutland, devicetree,
	Fenglin Wu
In-Reply-To: <20171115071345.24331-2-bjorn.andersson@linaro.org>

On Tue, Nov 14, 2017 at 11:13:43PM -0800, Bjorn Andersson wrote:
> Some LED controllers have support for autonomously controlling
> brightness over time, according to some preprogrammed pattern or
> function.
> 
> This adds a new optional operator that LED class drivers can implement
> if they support such functionality as well as a new device attribute to
> configure the pattern for a given LED.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v2:
> - None
> 
> Changes since v1:
> - New patch, based on discussions following v1
> 
>  Documentation/ABI/testing/sysfs-class-led |  20 ++++
>  drivers/leds/led-class.c                  | 150 ++++++++++++++++++++++++++++++
>  include/linux/leds.h                      |  21 +++++
>  3 files changed, 191 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led
> index 5f67f7ab277b..74a7f5b1f89b 100644
> --- a/Documentation/ABI/testing/sysfs-class-led
> +++ b/Documentation/ABI/testing/sysfs-class-led
> @@ -61,3 +61,23 @@ Description:
>  		gpio and backlight triggers. In case of the backlight trigger,
>  		it is useful when driving a LED which is intended to indicate
>  		a device in a standby like state.
> +
> +What:		/sys/class/leds/<led>/pattern
> +Date:		July 2017

That was many months ago :)

> +KernelVersion:	4.14

And that kernel version is long since released :)

thanks,

greg k-h

^ permalink raw reply

* [PATCH v3 3/3] DT: leds: Add Qualcomm Light Pulse Generator binding
From: Bjorn Andersson @ 2017-11-15  7:13 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek, Rob Herring,
	Mark Rutland
  Cc: linux-kernel, linux-leds, linux-arm-msm, devicetree, Fenglin Wu
In-Reply-To: <20171115071345.24331-1-bjorn.andersson@linaro.org>

This adds the binding document describing the three hardware blocks
related to the Light Pulse Generator found in a wide range of Qualcomm
PMICs.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- Squashed all things into one node
- Removed quirks from the binding, compatible implies number of channels, their
  configuration etc.
- Binding describes LEDs connected as child nodes
- Support describing multi-channel LEDs
- Change style of the binding document, to match other LED bindings

Changes since v1:
- Dropped custom pattern properties
- Renamed cell-index to qcom,lpg-channel to clarify its purpose

 .../devicetree/bindings/leds/leds-qcom-lpg.txt     | 66 ++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt
new file mode 100644
index 000000000000..9cee6f9f543c
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt
@@ -0,0 +1,66 @@
+Binding for Qualcomm Light Pulse Generator
+
+The Qualcomm Light Pulse Generator consists of three different hardware blocks;
+a ramp generator with lookup table, the light pulse generator and a three
+channel current sink. These blocks are found in a wide range of Qualcomm PMICs.
+
+Required properties:
+- compatible: one of:
+	      "qcom,pm8916-pwm",
+	      "qcom,pm8941-lpg",
+	      "qcom,pm8994-lpg",
+	      "qcom,pmi8994-lpg",
+	      "qcom,pmi8998-lpg",
+
+Optional properties:
+- qcom,power-source: power-source used to drive the output, as defined in the
+		     datasheet. Should be specified if the TRILED block is
+		     present
+- qcom,dtest: configures the output into an internal test line of the
+	      pmic. Specified by a list of u32 pairs, one pair per channel,
+	      where each pair denotes the test line to drive and the second
+	      configures how the value should be outputed, as defined in the
+	      datasheet
+- #pwm-cells: should be 2, see ../pwm/pwm.txt
+
+LED subnodes:
+A set of subnodes can be used to specify LEDs connected to the LPG. Channels
+not associated with a LED are available as pwm channels, see ../pwm/pwm.txt.
+
+Required properties:
+- led-sources: list of channels associated with this LED, starting at 1 for the
+	       first LPG channel
+
+Optional properties:
+- label: see Documentation/devicetree/bindings/leds/common.txt
+- default-state: see Documentation/devicetree/bindings/leds/common.txt
+- linux,default-trigger: see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+The following example defines a RGB LED attached to the PM8941.
+
+&spmi_bus {
+	pm8941@1 {
+		lpg {
+			compatible = "qcom,pm8941-lpg";
+			qcom,power-source = <1>;
+
+			rgb {
+				led-sources = <7 6 5>;
+			};
+		};
+	};
+};
+
+The following example defines the single PWM channel of the PM8916, which can
+be muxed by the MPP4 as a current sink.
+
+&spmi_bus {
+	pm8916@1 {
+		pm8916_pwm: pwm {
+			compatible = "qcom,pm8916-pwm";
+
+			#pwm-cells = <2>;
+		};
+	};
+};
-- 
2.15.0

^ permalink raw reply related

* [PATCH v3 2/3] leds: Add driver for Qualcomm LPG
From: Bjorn Andersson @ 2017-11-15  7:13 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek
  Cc: linux-kernel, linux-leds, linux-arm-msm, Rob Herring,
	Mark Rutland, devicetree, Fenglin Wu
In-Reply-To: <20171115071345.24331-1-bjorn.andersson@linaro.org>

The Light Pulse Generator (LPG) is a PWM-block found in a wide range of
PMICs from Qualcomm. It can operate on fixed parameters or based on a
lookup-table, altering the duty cycle over time - which provides the
means for e.g. hardware assisted transitions of LED brightness.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Changes since v2:
- Squash all components into one driver
- Track PWM channels and "logical" LEDs separately
- Support multiple channels to be bound to a single LED
- Per-PMIC compatible, to deal with minor differences (e.g. value to enable
  9bit resolution for PWM)
- TRILED enablement is done atomically for all channels associated with a LED
- LUT sequencer start is done atomically for all channels associated with a LED
- Support PM8916 (PWM only), PM8941, PM8994 and PMI8998 introduced (PMI8994
  still works...)

The multiple channels per LED is currently implemented by assigning the same
pattern and same brightness to all channels. This allows the RGB LED to show
various brighness of white and do patterns in shades of white. But it's
implemented in a way that as we figure out how to expose multi-color LEDs
through the LED framework this new information could easily be applied to the
right channel, and we would have the ability to control the channels
individually.

Changes since v1:
- Remove custom DT properties for patterns
- Extract pattern interface into the LED core

 drivers/leds/Kconfig         |    7 +
 drivers/leds/Makefile        |    1 +
 drivers/leds/leds-qcom-lpg.c | 1232 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1240 insertions(+)
 create mode 100644 drivers/leds/leds-qcom-lpg.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 52ea34e337cd..ccc3aa4b2474 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -651,6 +651,13 @@ config LEDS_POWERNV
 	  To compile this driver as a module, choose 'm' here: the module
 	  will be called leds-powernv.
 
+config LEDS_QCOM_LPG
+	tristate "LED support for Qualcomm LPG"
+	depends on LEDS_CLASS
+	help
+	  This option enables support for the Light Pulse Generator found in a
+	  wide variety of Qualcomm PMICs.
+
 config LEDS_SYSCON
 	bool "LED support for LEDs on system controllers"
 	depends on LEDS_CLASS=y
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 35980450db9b..2d5149ca429d 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_LEDS_MAX77693)		+= leds-max77693.o
 obj-$(CONFIG_LEDS_MAX8997)		+= leds-max8997.o
 obj-$(CONFIG_LEDS_LM355x)		+= leds-lm355x.o
 obj-$(CONFIG_LEDS_BLINKM)		+= leds-blinkm.o
+obj-$(CONFIG_LEDS_QCOM_LPG)		+= leds-qcom-lpg.o
 obj-$(CONFIG_LEDS_SYSCON)		+= leds-syscon.o
 obj-$(CONFIG_LEDS_MENF21BMC)		+= leds-menf21bmc.o
 obj-$(CONFIG_LEDS_KTD2692)		+= leds-ktd2692.o
diff --git a/drivers/leds/leds-qcom-lpg.c b/drivers/leds/leds-qcom-lpg.c
new file mode 100644
index 000000000000..481e940d7e04
--- /dev/null
+++ b/drivers/leds/leds-qcom-lpg.c
@@ -0,0 +1,1232 @@
+/*
+ * Copyright (c) 2017 Linaro Ltd
+ * Copyright (c) 2010-2012, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define LPG_PATTERN_CONFIG_REG	0x40
+#define LPG_SIZE_CLK_REG	0x41
+#define LPG_PREDIV_CLK_REG	0x42
+#define PWM_TYPE_CONFIG_REG	0x43
+#define PWM_VALUE_REG		0x44
+#define PWM_ENABLE_CONTROL_REG	0x46
+#define PWM_SYNC_REG		0x47
+#define LPG_RAMP_DURATION_REG	0x50
+#define LPG_HI_PAUSE_REG	0x52
+#define LPG_LO_PAUSE_REG	0x54
+#define LPG_HI_IDX_REG		0x56
+#define LPG_LO_IDX_REG		0x57
+#define PWM_SEC_ACCESS_REG	0xd0
+#define PWM_DTEST_REG(x)	(0xe2 + (x) - 1)
+
+#define TRI_LED_SRC_SEL		0x45
+#define TRI_LED_EN_CTL		0x46
+#define TRI_LED_ATC_CTL		0x47
+
+#define LPG_LUT_REG(x)		(0x40 + (x) * 2)
+#define RAMP_CONTROL_REG	0xc8
+
+struct lpg_channel;
+struct lpg_data;
+
+/**
+ * struct lpg - LPG device context
+ * @dev:	struct device for LPG device
+ * @map:	regmap for register access
+ * @pwm:	PWM-chip object, if operating in PWM mode
+ * @pwm_9bit_mask: bitmask for enabling 9bit pwm
+ * @lut_base:	base address of the LUT block (optional)
+ * @lut_size:	number of entries in the LUT block
+ * @lut_bitmap:	allocation bitmap for LUT entries
+ * @triled_base: base address of the TRILED block (optional)
+ * @triled_src:	power-source for the TRILED
+ * @channels:	list of PWM channels
+ * @num_channels: number of @channels
+ */
+struct lpg {
+	struct device *dev;
+	struct regmap *map;
+
+	struct pwm_chip pwm;
+
+	const struct lpg_data *data;
+
+	u32 lut_base;
+	u32 lut_size;
+	unsigned long *lut_bitmap;
+
+	u32 triled_base;
+	u32 triled_src;
+
+	struct lpg_channel *channels;
+	unsigned int num_channels;
+};
+
+/**
+ * struct lpg_channel - per channel data
+ * @lpg:	reference to parent lpg
+ * @base:	base address of the PWM channel
+ * @triled_mask: mask in TRILED to enable this channel
+ * @lut_mask:	mask in LUT to start pattern generator for this channel
+ * @in_use:	channel is exposed to LED framework
+ * @dtest_line:	DTEST line for output, or 0 if disabled
+ * @dtest_value: DTEST line configuration
+ * @pwm_value:	duty (in microseconds) of the generated pulses, overriden by LUT
+ * @enabled:	output enabled?
+ * @period_us:	period (in microseconds) of the generated pulses
+ * @pwm_size:	resolution of the @pwm_value, 6 or 9 bits
+ * @clk:	base frequency of the clock generator
+ * @pre_div:	divider of @clk
+ * @pre_div_exp: exponential divider of @clk
+ * @ramp_enabled: duty cycle is driven by iterating over lookup table
+ * @ramp_ping_pong: reverse through pattern, rather than wrapping to start
+ * @ramp_oneshot: perform only a single pass over the pattern
+ * @ramp_reverse: iterate over pattern backwards
+ * @ramp_duration_ms: length (in milliseconds) of one pattern run
+ * @ramp_lo_pause_ms: pause (in milliseconds) before iterating over pattern
+ * @ramp_hi_pause_ms: pause (in milliseconds) after iterating over pattern
+ * @pattern_lo_idx: start index of associated pattern
+ * @pattern_hi_idx: last index of associated pattern
+ */
+struct lpg_channel {
+	struct lpg *lpg;
+
+	u32 base;
+	unsigned int triled_mask;
+	unsigned int lut_mask;
+
+	bool in_use;
+
+	u32 dtest_line;
+	u32 dtest_value;
+
+	u16 pwm_value;
+	bool enabled;
+
+	unsigned int period_us;
+	unsigned int pwm_size;
+	unsigned int clk;
+	unsigned int pre_div;
+	unsigned int pre_div_exp;
+
+	bool ramp_enabled;
+	bool ramp_ping_pong;
+	bool ramp_oneshot;
+	bool ramp_reverse;
+	unsigned long ramp_duration_ms;
+	unsigned long ramp_lo_pause_ms;
+	unsigned long ramp_hi_pause_ms;
+
+	unsigned int pattern_lo_idx;
+	unsigned int pattern_hi_idx;
+};
+
+/**
+ * struct lpg_led - logical LED object
+ * @lpg:		lpg context reference
+ * @cdev:		LED class device
+ * @num_channels:	number of @channels
+ * @channels:		list of channels associated with the LED
+ */
+struct lpg_led {
+	struct lpg *lpg;
+
+	struct led_classdev cdev;
+
+	unsigned int num_channels;
+	struct lpg_channel *channels[];
+};
+
+/**
+ * struct lpg_channel_data - per channel initialization data
+ * @base:		base address for PWM channel registers
+ * @triled_mask:	bitmask for controlling this channel in TRILED
+ */
+struct lpg_channel_data {
+	unsigned int base;
+	u8 triled_mask;
+};
+
+/**
+ * struct lpg_data - initialization data
+ * @lut_base:		base address of LUT block
+ * @lut_size:		number of entries in LUT
+ * @triled_base:	base address of TRILED
+ * @pwm_9bit_mask:	bitmask for switching from 6bit to 9bit pwm
+ * @num_channels:	number of channels in LPG
+ * @channels:		list of channel initialization data
+ */
+struct lpg_data {
+	unsigned int lut_base;
+	unsigned int lut_size;
+	unsigned int triled_base;
+	unsigned int pwm_9bit_mask;
+	int num_channels;
+	struct lpg_channel_data *channels;
+};
+
+static int triled_set(struct lpg *lpg, unsigned int mask, bool enable)
+{
+	/* Skip if we don't have a triled block */
+	if (!lpg->triled_base)
+		return 0;
+
+	return regmap_update_bits(lpg->map, lpg->triled_base + TRI_LED_EN_CTL,
+				  mask, enable ? mask : 0);
+}
+
+static int lpg_lut_store(struct lpg *lpg, const u16 *values, size_t len,
+			 unsigned int *lo_idx, unsigned int *hi_idx)
+{
+	unsigned int idx;
+	u8 val[2];
+	int i;
+
+	/* Hardware does not behave when LO_IDX == HI_IDX */
+	if (len == 1)
+		return -EINVAL;
+
+	idx = bitmap_find_next_zero_area(lpg->lut_bitmap, lpg->lut_size,
+					 0, len, 0);
+	if (idx >= lpg->lut_size)
+		return -ENOMEM;
+
+	for (i = 0; i < len; i++) {
+		val[0] = values[i] & 0xff;
+		val[1] = values[i] >> 8;
+
+		regmap_bulk_write(lpg->map,
+				  lpg->lut_base + LPG_LUT_REG(idx + i), val, 2);
+	}
+
+	bitmap_set(lpg->lut_bitmap, idx, len);
+
+	*lo_idx = idx;
+	*hi_idx = idx + len - 1;
+
+	return 0;
+}
+
+static u16 *lpg_lut_read(struct lpg *lpg, unsigned int lo_idx,
+			 unsigned int hi_idx, size_t *len)
+{
+	u16 *values;
+	u8 val[2];
+	int ret;
+	int i;
+
+	*len = hi_idx - lo_idx + 1;
+
+	values = kcalloc(*len, sizeof(u16), GFP_KERNEL);
+	if (!values)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < *len; i++) {
+		ret = regmap_bulk_read(lpg->map,
+				       lpg->lut_base + LPG_LUT_REG(lo_idx + i),
+				       &val, 2);
+		if (ret < 0) {
+			kfree(values);
+			return ERR_PTR(ret);
+		}
+
+		values[i] = val[0] | val[1] << 8;
+	}
+
+	return values;
+}
+
+static void lpg_lut_free(struct lpg *lpg, unsigned int lo_idx, unsigned int hi_idx)
+{
+	int len;
+
+	if (lo_idx == hi_idx)
+		return;
+
+	len = hi_idx - lo_idx + 1;
+	bitmap_clear(lpg->lut_bitmap, lo_idx, len);
+}
+
+static int lpg_lut_sync(struct lpg *lpg, unsigned int mask)
+{
+	return regmap_update_bits(lpg->map, lpg->lut_base + RAMP_CONTROL_REG,
+				  mask, 0xff);
+}
+
+#define NUM_PWM_PREDIV	4
+#define NUM_PWM_CLK	3
+#define NUM_EXP		7
+
+static const unsigned int lpg_clk_table[NUM_PWM_PREDIV][NUM_PWM_CLK] = {
+	{
+		1 * (NSEC_PER_SEC / 1024),
+		1 * (NSEC_PER_SEC / 32768),
+		1 * (NSEC_PER_SEC / 19200000),
+	},
+	{
+		3 * (NSEC_PER_SEC / 1024),
+		3 * (NSEC_PER_SEC / 32768),
+		3 * (NSEC_PER_SEC / 19200000),
+	},
+	{
+		5 * (NSEC_PER_SEC / 1024),
+		5 * (NSEC_PER_SEC / 32768),
+		5 * (NSEC_PER_SEC / 19200000),
+	},
+	{
+		6 * (NSEC_PER_SEC / 1024),
+		6 * (NSEC_PER_SEC / 32768),
+		6 * (NSEC_PER_SEC / 19200000),
+	},
+};
+
+/*
+ * PWM Frequency = Clock Frequency / (N * T)
+ *      or
+ * PWM Period = Clock Period * (N * T)
+ *      where
+ * N = 2^9 or 2^6 for 9-bit or 6-bit PWM size
+ * T = Pre-divide * 2^m, where m = 0..7 (exponent)
+ *
+ * This is the formula to figure out m for the best pre-divide and clock:
+ * (PWM Period / N) = (Pre-divide * Clock Period) * 2^m
+ */
+static void lpg_calc_freq(struct lpg_channel *chan, unsigned int period_us)
+{
+	int             n, m, clk, div;
+	int             best_m, best_div, best_clk;
+	unsigned int    last_err, cur_err, min_err;
+	unsigned int    tmp_p, period_n;
+
+	if (period_us == chan->period_us)
+		return;
+
+	/* PWM Period / N */
+	if (period_us < ((unsigned int)(-1) / NSEC_PER_USEC)) {
+		period_n = (period_us * NSEC_PER_USEC) >> 6;
+		n = 6;
+	} else {
+		period_n = (period_us >> 9) * NSEC_PER_USEC;
+		n = 9;
+	}
+
+	min_err = last_err = (unsigned int)(-1);
+	best_m = 0;
+	best_clk = 0;
+	best_div = 0;
+	for (clk = 0; clk < NUM_PWM_CLK; clk++) {
+		for (div = 0; div < NUM_PWM_PREDIV; div++) {
+			/* period_n = (PWM Period / N) */
+			/* tmp_p = (Pre-divide * Clock Period) * 2^m */
+			tmp_p = lpg_clk_table[div][clk];
+			for (m = 0; m <= NUM_EXP; m++) {
+				if (period_n > tmp_p)
+					cur_err = period_n - tmp_p;
+				else
+					cur_err = tmp_p - period_n;
+
+				if (cur_err < min_err) {
+					min_err = cur_err;
+					best_m = m;
+					best_clk = clk;
+					best_div = div;
+				}
+
+				if (m && cur_err > last_err)
+					/* Break for bigger cur_err */
+					break;
+
+				last_err = cur_err;
+				tmp_p <<= 1;
+			}
+		}
+	}
+
+	/* Use higher resolution */
+	if (best_m >= 3 && n == 6) {
+		n += 3;
+		best_m -= 3;
+	}
+
+	chan->clk = best_clk;
+	chan->pre_div = best_div;
+	chan->pre_div_exp = best_m;
+	chan->pwm_size = n;
+
+	chan->period_us = period_us;
+}
+
+static void lpg_calc_duty(struct lpg_channel *chan, unsigned int duty_us)
+{
+	unsigned long max = (1 << chan->pwm_size) - 1;
+	unsigned long val;
+
+	/* Figure out pwm_value with overflow handling */
+	if (duty_us < 1 << (sizeof(val) * 8 - chan->pwm_size))
+		val = (duty_us << chan->pwm_size) / chan->period_us;
+	else
+		val = duty_us / (chan->period_us >> chan->pwm_size);
+
+	if (val > max)
+		val = max;
+
+	chan->pwm_value = val;
+}
+
+static void lpg_apply_freq(struct lpg_channel *chan)
+{
+	unsigned long val;
+	struct lpg *lpg = chan->lpg;
+
+	if (!chan->enabled)
+		return;
+
+	/* Clock register values are off-by-one from lpg_clk_table */
+	val = chan->clk + 1;
+
+	if (chan->pwm_size == 9)
+		val |= lpg->data->pwm_9bit_mask;
+
+	regmap_write(lpg->map, chan->base + LPG_SIZE_CLK_REG, val);
+
+	val = chan->pre_div << 5 | chan->pre_div_exp;
+	regmap_write(lpg->map, chan->base + LPG_PREDIV_CLK_REG, val);
+}
+
+#define LPG_ENABLE_GLITCH_REMOVAL	BIT(5)
+
+static void lpg_enable_glitch(struct lpg_channel *chan)
+{
+	struct lpg *lpg = chan->lpg;
+
+	regmap_update_bits(lpg->map, chan->base + PWM_TYPE_CONFIG_REG,
+			   LPG_ENABLE_GLITCH_REMOVAL, 0);
+}
+
+static void lpg_disable_glitch(struct lpg_channel *chan)
+{
+	struct lpg *lpg = chan->lpg;
+
+	regmap_update_bits(lpg->map, chan->base + PWM_TYPE_CONFIG_REG,
+			   LPG_ENABLE_GLITCH_REMOVAL,
+			   LPG_ENABLE_GLITCH_REMOVAL);
+}
+
+static void lpg_apply_pwm_value(struct lpg_channel *chan)
+{
+	u8 val[] = { chan->pwm_value & 0xff, chan->pwm_value >> 8 };
+	struct lpg *lpg = chan->lpg;
+
+	if (!chan->enabled)
+		return;
+
+	regmap_bulk_write(lpg->map, chan->base + PWM_VALUE_REG, val, 2);
+}
+
+#define LPG_PATTERN_CONFIG_LO_TO_HI	BIT(4)
+#define LPG_PATTERN_CONFIG_REPEAT	BIT(3)
+#define LPG_PATTERN_CONFIG_TOGGLE	BIT(2)
+#define LPG_PATTERN_CONFIG_PAUSE_HI	BIT(1)
+#define LPG_PATTERN_CONFIG_PAUSE_LO	BIT(0)
+
+static void lpg_apply_lut_control(struct lpg_channel *chan)
+{
+	struct lpg *lpg = chan->lpg;
+	unsigned int hi_pause;
+	unsigned int lo_pause;
+	unsigned int step;
+	unsigned int conf = 0;
+	unsigned int lo_idx = chan->pattern_lo_idx;
+	unsigned int hi_idx = chan->pattern_hi_idx;
+	int pattern_len;
+
+	if (!chan->ramp_enabled || chan->pattern_lo_idx == chan->pattern_hi_idx)
+		return;
+
+	pattern_len = hi_idx - lo_idx + 1;
+
+	step = DIV_ROUND_UP(chan->ramp_duration_ms, pattern_len);
+	hi_pause = DIV_ROUND_UP(chan->ramp_hi_pause_ms, step);
+	lo_pause = DIV_ROUND_UP(chan->ramp_lo_pause_ms, step);
+
+	if (!chan->ramp_reverse)
+		conf |= LPG_PATTERN_CONFIG_LO_TO_HI;
+	if (!chan->ramp_oneshot)
+		conf |= LPG_PATTERN_CONFIG_REPEAT;
+	if (chan->ramp_ping_pong)
+		conf |= LPG_PATTERN_CONFIG_TOGGLE;
+	if (chan->ramp_hi_pause_ms)
+		conf |= LPG_PATTERN_CONFIG_PAUSE_HI;
+	if (chan->ramp_lo_pause_ms)
+		conf |= LPG_PATTERN_CONFIG_PAUSE_LO;
+
+	regmap_write(lpg->map, chan->base + LPG_PATTERN_CONFIG_REG, conf);
+	regmap_write(lpg->map, chan->base + LPG_HI_IDX_REG, hi_idx);
+	regmap_write(lpg->map, chan->base + LPG_LO_IDX_REG, lo_idx);
+
+	regmap_write(lpg->map, chan->base + LPG_RAMP_DURATION_REG, step);
+	regmap_write(lpg->map, chan->base + LPG_HI_PAUSE_REG, hi_pause);
+	regmap_write(lpg->map, chan->base + LPG_LO_PAUSE_REG, lo_pause);
+}
+
+#define LPG_ENABLE_CONTROL_OUTPUT		BIT(7)
+#define LPG_ENABLE_CONTROL_BUFFER_TRISTATE	BIT(5)
+#define LPG_ENABLE_CONTROL_SRC_PWM		BIT(2)
+#define LPG_ENABLE_CONTROL_RAMP_GEN		BIT(1)
+
+static void lpg_apply_control(struct lpg_channel *chan)
+{
+	unsigned int ctrl;
+	struct lpg *lpg = chan->lpg;
+
+	ctrl = LPG_ENABLE_CONTROL_BUFFER_TRISTATE;
+
+	if (chan->enabled)
+		ctrl |= LPG_ENABLE_CONTROL_OUTPUT;
+
+	if (chan->pattern_lo_idx != chan->pattern_hi_idx)
+		ctrl |= LPG_ENABLE_CONTROL_RAMP_GEN;
+	else
+		ctrl |= LPG_ENABLE_CONTROL_SRC_PWM;
+
+	regmap_write(lpg->map, chan->base + PWM_ENABLE_CONTROL_REG, ctrl);
+
+	/*
+	 * Due to LPG hardware bug, in the PWM mode, having enabled PWM,
+	 * We have to write PWM values one more time.
+	 */
+	if (chan->enabled)
+		lpg_apply_pwm_value(chan);
+}
+
+#define LPG_SYNC_PWM	BIT(0)
+
+static void lpg_apply_sync(struct lpg_channel *chan)
+{
+	struct lpg *lpg = chan->lpg;
+
+	regmap_write(lpg->map, chan->base + PWM_SYNC_REG, LPG_SYNC_PWM);
+}
+
+static void lpg_apply_dtest(struct lpg_channel *chan)
+{
+	struct lpg *lpg = chan->lpg;
+
+	if (!chan->dtest_line)
+		return;
+
+	regmap_write(lpg->map, chan->base + PWM_SEC_ACCESS_REG, 0xa5);
+	regmap_write(lpg->map, chan->base + PWM_DTEST_REG(chan->dtest_line),
+		     chan->dtest_value);
+}
+
+static void lpg_apply(struct lpg_channel *chan)
+{
+	lpg_disable_glitch(chan);
+	lpg_apply_freq(chan);
+	lpg_apply_pwm_value(chan);
+	lpg_apply_control(chan);
+	lpg_apply_sync(chan);
+	lpg_apply_lut_control(chan);
+	lpg_enable_glitch(chan);
+}
+
+static void lpg_brightness_set(struct led_classdev *cdev,
+			      enum led_brightness value)
+{
+	struct lpg_led *led = container_of(cdev, struct lpg_led, cdev);
+	struct lpg_channel *chan;
+	struct lpg *lpg = led->lpg;
+	unsigned int duty_us;
+	unsigned int triled_mask = 0;
+	unsigned int lut_mask = 0;
+	int i;
+
+	for (i = 0; i < led->num_channels; i++) {
+		chan = led->channels[i];
+
+		if (value == LED_OFF) {
+			chan->enabled = false;
+			chan->ramp_enabled = false;
+		} else if (chan->pattern_lo_idx != chan->pattern_hi_idx) {
+			lpg_calc_freq(chan, NSEC_PER_USEC);
+
+			chan->enabled = true;
+			chan->ramp_enabled = true;
+
+			lut_mask |= chan->lut_mask;
+			triled_mask |= chan->triled_mask;
+		} else {
+			lpg_calc_freq(chan, NSEC_PER_USEC);
+
+			duty_us = value * chan->period_us / cdev->max_brightness;
+			lpg_calc_duty(chan, duty_us);
+			chan->enabled = true;
+			chan->ramp_enabled = false;
+
+			triled_mask |= chan->triled_mask;
+		}
+
+		lpg_apply(chan);
+	}
+
+	/* Toggle triled lines */
+	if (triled_mask)
+		triled_set(lpg, triled_mask, chan->enabled);
+
+	/* Trigger start of ramp generator(s) */
+	if (lut_mask)
+		lpg_lut_sync(lpg, lut_mask);
+}
+
+static enum led_brightness lpg_brightness_get(struct led_classdev *cdev)
+{
+	struct lpg_led *led = container_of(cdev, struct lpg_led, cdev);
+	struct lpg_channel *chan = led->channels[0];
+	unsigned long max = (1 << chan->pwm_size) - 1;
+
+	if (!chan->enabled)
+		return LED_OFF;
+	else if (chan->pattern_lo_idx != chan->pattern_hi_idx)
+		return LED_FULL;
+	else
+		return chan->pwm_value * cdev->max_brightness / max;
+}
+
+static int lpg_blink_set(struct led_classdev *cdev,
+			 unsigned long *delay_on, unsigned long *delay_off)
+{
+	struct lpg_led *led = container_of(cdev, struct lpg_led, cdev);
+	struct lpg_channel *chan = led->channels[0];
+	unsigned int period_us;
+	unsigned int duty_us;
+
+	if (!*delay_on && !*delay_off) {
+		*delay_on = 500;
+		*delay_off = 500;
+	}
+
+	duty_us = *delay_on * USEC_PER_MSEC;
+	period_us = (*delay_on + *delay_off) * USEC_PER_MSEC;
+
+	lpg_calc_freq(chan, period_us);
+	lpg_calc_duty(chan, duty_us);
+
+	chan->enabled = true;
+	chan->ramp_enabled = false;
+
+	lpg_apply(chan);
+
+	return 0;
+}
+
+#define interpolate(x1, y1, x2, y2, x) \
+	((y1) + ((y2) - (y1)) * ((x) - (x1)) / ((x2) - (x1)))
+
+static int lpg_pattern_set(struct led_classdev *led_cdev,
+			   struct led_pattern *led_pattern, int len,
+			   bool repeat)
+{
+	struct lpg_led *led = container_of(led_cdev, struct lpg_led, cdev);
+	struct lpg_channel *chan = led->channels[0];
+	struct lpg *lpg = led->lpg;
+	unsigned int duration = 0;
+	unsigned int min_delta = (unsigned int)-1;
+	unsigned int hi_pause;
+	unsigned int lo_pause = 0;
+	unsigned int lo_idx;
+	unsigned int hi_idx;
+	unsigned int max = (1 << chan->pwm_size) - 1;
+	bool ping_pong = true;
+	int brightness_a;
+	int brightness_b;
+	u16 *pattern;
+	int src_idx;
+	int dst_idx;
+	int step_t;
+	int time_a;
+	int time_b;
+	int value;
+	int steps;
+	int ret = 0;
+	int i;
+
+	/*
+	 * The led_pattern specifies brightness values, potentially distributed
+	 * unevenly over the duration of the pattern. The LPG only support
+	 * evenly distributed values, so we interpolate new values from the
+	 * led_pattern.
+	 */
+
+	/* Sum the duration over the inner delta_ts and the tail is hi_pause */
+	for (src_idx = 0; src_idx < len - 1; src_idx++)
+		duration += led_pattern[src_idx].delta_t;
+	hi_pause = led_pattern[src_idx].delta_t;
+
+	for (src_idx = 0; src_idx < len; src_idx++) {
+		min_delta = min_t(unsigned int, min_delta,
+				  led_pattern[src_idx].delta_t);
+	}
+
+	steps = duration / min_delta + 1;
+	pattern = kcalloc(steps, sizeof(*pattern), GFP_KERNEL);
+	if (!pattern)
+		return -ENOMEM;
+
+	time_a = 0;
+	for (src_idx = 0, dst_idx = 0; dst_idx < steps; dst_idx++) {
+		/* The timestamp of this evenly distributed data point */
+		step_t = dst_idx * min_delta;
+
+		/*
+		 * Find time_a - time_b interval from source pattern that spans
+		 * step_t
+		 */
+		while (time_a + led_pattern[src_idx].delta_t < step_t) {
+			if (src_idx >= len - 1)
+				break;
+			time_a += led_pattern[++src_idx].delta_t;
+		}
+
+		if (src_idx < len - 1) {
+			time_b = time_a + led_pattern[src_idx].delta_t;
+
+			brightness_a = led_pattern[src_idx].brightness;
+			brightness_b = led_pattern[src_idx + 1].brightness;
+
+			/* Interpolate over the source pattern segment */
+			value = interpolate(time_a, brightness_a, time_b,
+					    brightness_b, step_t);
+		} else {
+			value = led_pattern[src_idx].brightness;
+		}
+
+		/* Scale calculated value to the hardware brightness value */
+		pattern[dst_idx] = value * max / led_cdev->max_brightness;
+	}
+
+	/* Detect palindromes and use "ping pong" to reduce LUT usage */
+	for (dst_idx = 0; dst_idx < steps / 2; dst_idx++) {
+		if (pattern[dst_idx] != pattern[len - dst_idx - 1]) {
+			ping_pong = false;
+			break;
+		}
+	}
+	if (ping_pong) {
+		steps = (steps + 1) / 2;
+
+		/*
+		 * When ping_pong is set the hi_pause will happen in the middle
+		 * of the pattern, so we need to use lo_pause to delay between
+		 * the loops.
+		 */
+		if (repeat)
+			lo_pause = hi_pause;
+
+		hi_pause = 0;
+	}
+
+	ret = lpg_lut_store(lpg, pattern, steps, &lo_idx, &hi_idx);
+	if (ret < 0)
+		goto out;
+
+	chan = led->channels[0];
+
+	lpg_lut_free(lpg, chan->pattern_lo_idx, chan->pattern_hi_idx);
+
+	/* Update settings on each associated channel */
+	for (i = 0; i < led->num_channels; i++) {
+		chan = led->channels[i];
+
+		chan->ramp_duration_ms = duration;
+		chan->ramp_ping_pong = ping_pong;
+		chan->ramp_oneshot = !repeat;
+
+		chan->pattern_lo_idx = lo_idx;
+		chan->pattern_hi_idx = hi_idx;
+	}
+
+out:
+	kfree(pattern);
+
+	return ret;
+}
+
+static int lpg_pattern_clear(struct led_classdev *cdev)
+{
+	struct lpg_led *led = container_of(cdev, struct lpg_led, cdev);
+	struct lpg_channel *chan;
+	struct lpg *lpg = led->lpg;
+	int i;
+
+	chan = led->channels[0];
+
+	lpg_lut_free(lpg, chan->pattern_lo_idx, chan->pattern_hi_idx);
+
+	for (i = 0; i < led->num_channels; i++) {
+		chan = led->channels[i];
+		chan->pattern_lo_idx = 0;
+		chan->pattern_hi_idx = 0;
+	}
+
+	return 0;
+}
+
+static struct led_pattern *lpg_pattern_get(struct led_classdev *cdev,
+					   size_t *len, bool *repeat)
+{
+	struct led_pattern *led_pattern;
+	struct lpg_led *led = container_of(cdev, struct lpg_led, cdev);
+	struct lpg *lpg = led->lpg;
+	struct lpg_channel *chan = led->channels[0];
+	unsigned int delta_t;
+	unsigned int max = (1 << chan->pwm_size) - 1;
+	size_t all_steps;
+	size_t steps;
+	u16 *pattern;
+	size_t i;
+	u16 val;
+
+	pattern = lpg_lut_read(lpg, chan->pattern_lo_idx, chan->pattern_hi_idx,
+			       &steps);
+	if (IS_ERR_OR_NULL(pattern))
+		return ERR_CAST(pattern);
+
+	all_steps = chan->ramp_ping_pong ? steps * 2 - 1 : steps;
+
+	delta_t = (chan->ramp_duration_ms + chan->ramp_hi_pause_ms) / all_steps;
+
+	led_pattern = kcalloc(all_steps, sizeof(*pattern), GFP_KERNEL);
+	if (!led_pattern) {
+		led_pattern = ERR_PTR(-ENOMEM);
+		goto out;
+	}
+
+	for (i = 0; i < all_steps; i++) {
+		if (i < steps)
+			val = pattern[i];
+		else
+			val = pattern[steps - i];
+
+		led_pattern[i].delta_t = delta_t;
+		led_pattern[i].brightness = val * cdev->max_brightness / max;
+	}
+
+	*len = all_steps;
+	*repeat = !chan->ramp_oneshot;
+
+out:
+	kfree(pattern);
+	return led_pattern;
+}
+
+static int lpg_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct lpg *lpg = container_of(chip, struct lpg, pwm);
+	struct lpg_channel *chan = &lpg->channels[pwm->hwpwm];
+
+	return chan->in_use ? -EBUSY : 0;
+}
+
+static int lpg_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			 struct pwm_state *state)
+{
+	struct lpg *lpg = container_of(chip, struct lpg, pwm);
+	struct lpg_channel *chan = &lpg->channels[pwm->hwpwm];
+
+	lpg_calc_freq(chan, state->period / NSEC_PER_USEC);
+	lpg_calc_duty(chan, state->duty_cycle / NSEC_PER_USEC);
+	chan->enabled = state->enabled;
+
+	lpg_apply(chan);
+
+	triled_set(lpg, chan->triled_mask, chan->enabled);
+
+	state->polarity = PWM_POLARITY_NORMAL;
+	state->period = chan->period_us * NSEC_PER_USEC;
+
+	return 0;
+}
+
+static const struct pwm_ops lpg_pwm_ops = {
+	.request = lpg_pwm_request,
+	.apply = lpg_pwm_apply,
+	.owner = THIS_MODULE,
+};
+
+static int lpg_add_pwm(struct lpg *lpg)
+{
+	int ret;
+
+	lpg->pwm.base = -1;
+	lpg->pwm.dev = lpg->dev;
+	lpg->pwm.npwm = lpg->num_channels;
+	lpg->pwm.ops = &lpg_pwm_ops;
+
+	ret = pwmchip_add(&lpg->pwm);
+	if (ret)
+		dev_err(lpg->dev, "failed to add PWM chip: ret %d\n", ret);
+
+	return ret;
+}
+
+static int lpg_add_led(struct lpg *lpg, struct device_node *np)
+{
+	struct lpg_led *led;
+	const char *state;
+	int sources;
+	int size;
+	u32 chan;
+	int ret;
+	int i;
+
+	sources = of_property_count_u32_elems(np, "led-sources");
+	if (sources <= 0) {
+		dev_err(lpg->dev, "invalid led-sources of %s\n",
+			np->name);
+		return -EINVAL;
+	}
+
+	size = sizeof(*led) + sources * sizeof(struct lpg_channel*);
+	led = devm_kzalloc(lpg->dev, size, GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	led->lpg = lpg;
+	led->num_channels = sources;
+
+	for (i = 0; i < sources; i++) {
+		ret = of_property_read_u32_index(np, "led-sources",
+						 i, &chan);
+		if (ret || !chan || chan > lpg->num_channels) {
+			dev_err(lpg->dev,
+				"invalid led-sources of %s\n",
+				np->name);
+			return -EINVAL;
+		}
+
+		led->channels[i] = &lpg->channels[chan - 1];
+
+		led->channels[i]->in_use = true;
+	}
+
+	/* Use label else node name */
+	led->cdev.name = of_get_property(np, "label", NULL) ? : np->name;
+	led->cdev.default_trigger = of_get_property(np, "linux,default-trigger", NULL);
+	led->cdev.brightness_set = lpg_brightness_set;
+	led->cdev.brightness_get = lpg_brightness_get;
+	led->cdev.blink_set = lpg_blink_set;
+	led->cdev.max_brightness = 255;
+
+	/* Register pattern accessors only if we have a LUT block */
+	if (lpg->lut_base) {
+		led->cdev.pattern_set = lpg_pattern_set;
+		led->cdev.pattern_clear = lpg_pattern_clear;
+		led->cdev.pattern_get = lpg_pattern_get;
+	}
+
+	if (!of_property_read_string(np, "default-state", &state) &&
+	    !strcmp(state, "on"))
+		led->cdev.brightness = LED_FULL;
+	else
+		led->cdev.brightness = LED_OFF;
+
+	lpg_brightness_set(&led->cdev, led->cdev.brightness);
+
+	ret = devm_led_classdev_register(lpg->dev, &led->cdev);
+	if (ret)
+		dev_err(lpg->dev, "unable to register %s\n", led->cdev.name);
+
+	return ret;
+}
+
+static int lpg_init_channels(struct lpg *lpg)
+{
+	const struct lpg_data *data = lpg->data;
+	int i;
+
+	lpg->num_channels = data->num_channels;
+	lpg->channels = devm_kcalloc(lpg->dev, data->num_channels,
+				     sizeof(struct lpg_channel), GFP_KERNEL);
+	if (!lpg->channels)
+		return -ENOMEM;
+
+	for (i = 0; i < data->num_channels; i++) {
+		lpg->channels[i].lpg = lpg;
+		lpg->channels[i].base = data->channels[i].base;
+		lpg->channels[i].triled_mask = data->channels[i].triled_mask;
+		lpg->channels[i].lut_mask = BIT(i);
+	}
+
+	return 0;
+}
+
+static int lpg_init_triled(struct lpg *lpg)
+{
+	struct device_node *np = lpg->dev->of_node;
+	int ret;
+
+	/* Skip initialization if we don't have a triled block */
+	if (!lpg->data->triled_base)
+		return 0;
+
+	lpg->triled_base = lpg->data->triled_base;
+
+	ret = of_property_read_u32(np, "qcom,power-source", &lpg->triled_src);
+	if (ret || lpg->triled_src == 2 || lpg->triled_src > 3) {
+		dev_err(lpg->dev, "invalid power source\n");
+		return -EINVAL;
+	}
+
+	/* Disable automatic trickle charge LED */
+	regmap_write(lpg->map, lpg->triled_base + TRI_LED_ATC_CTL, 0);
+
+	/* Configure power source */
+	regmap_write(lpg->map, lpg->triled_base + TRI_LED_SRC_SEL,
+		     lpg->triled_src);
+
+	/* Default all outputs to off */
+	regmap_write(lpg->map, lpg->triled_base + TRI_LED_EN_CTL, 0);
+
+	return 0;
+}
+
+static int lpg_init_lut(struct lpg *lpg)
+{
+	const struct lpg_data *data = lpg->data;
+	size_t bitmap_size;
+
+	if (!data->lut_base)
+		return 0;
+
+	lpg->lut_base = data->lut_base;
+	lpg->lut_size = data->lut_size;
+
+	bitmap_size = BITS_TO_LONGS(lpg->lut_size) / sizeof(unsigned long);
+	lpg->lut_bitmap = devm_kzalloc(lpg->dev, bitmap_size, GFP_KERNEL);
+
+	return lpg->lut_bitmap ? 0 : -ENOMEM;
+}
+
+static int lpg_parse_dtest(struct lpg *lpg)
+{
+	struct lpg_channel *chan;
+	struct device_node *np = lpg->dev->of_node;
+	int count;
+	int ret;
+	int i;
+
+	count = of_property_count_u32_elems(np, "qcom,dtest");
+	if (count == -EINVAL) {
+		return 0;
+	} else if (count < 0 || count != lpg->data->num_channels * 2) {
+		ret = count;
+		goto err_malformed;
+	}
+
+	for (i = 0; i < lpg->data->num_channels; i++) {
+		chan = &lpg->channels[i];
+
+		ret = of_property_read_u32_index(np, "qcom,dtest", i * 2,
+						 &chan->dtest_line);
+		if (ret)
+			goto err_malformed;
+
+		ret = of_property_read_u32_index(np, "qcom,dtest", i * 2 + 1,
+						 &chan->dtest_value);
+		if (ret)
+			goto err_malformed;
+	}
+
+	return 0;
+
+err_malformed:
+	dev_err(lpg->dev, "malformed qcom,dtest\n");
+	return ret;
+}
+
+static int lpg_probe(struct platform_device *pdev)
+{
+	struct device_node *np;
+	struct lpg *lpg;
+	int ret;
+	int i;
+
+	lpg = devm_kzalloc(&pdev->dev, sizeof(*lpg), GFP_KERNEL);
+	if (!lpg)
+		return -ENOMEM;
+
+	lpg->data = of_device_get_match_data(&pdev->dev);
+	if (!lpg->data)
+		return -EINVAL;
+
+	lpg->dev = &pdev->dev;
+
+	lpg->map = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!lpg->map) {
+		dev_err(&pdev->dev, "parent regmap unavailable\n");
+		return -ENXIO;
+	}
+
+	ret = lpg_init_channels(lpg);
+	if (ret < 0)
+		return ret;
+
+	ret = lpg_init_triled(lpg);
+	if (ret < 0)
+		return ret;
+
+	ret = lpg_init_lut(lpg);
+	if (ret < 0)
+		return ret;
+
+	ret = lpg_parse_dtest(lpg);
+	if (ret < 0)
+		return ret;
+
+	for_each_available_child_of_node(pdev->dev.of_node, np) {
+		ret = lpg_add_led(lpg, np);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < lpg->num_channels; i++)
+		lpg_apply_dtest(&lpg->channels[i]);
+
+	ret = lpg_add_pwm(lpg);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, lpg);
+
+	return 0;
+}
+
+static int lpg_remove(struct platform_device *pdev)
+{
+	struct lpg *lpg = platform_get_drvdata(pdev);
+
+	pwmchip_remove(&lpg->pwm);
+
+	return 0;
+}
+
+static const struct lpg_data pm8916_pwm_data = {
+	.pwm_9bit_mask = BIT(2),
+
+	.num_channels = 1,
+	.channels = (struct lpg_channel_data[]) {
+		{ .base = 0xbc00 },
+	},
+};
+
+static const struct lpg_data pm8941_lpg_data = {
+	.lut_base = 0xb000,
+	.lut_size = 64,
+
+	.triled_base = 0xd000,
+
+	.pwm_9bit_mask = 3 << 4,
+
+	.num_channels = 8,
+	.channels = (struct lpg_channel_data[]) {
+		{ .base = 0xb100 },
+		{ .base = 0xb200 },
+		{ .base = 0xb300 },
+		{ .base = 0xb400 },
+		{ .base = 0xb500, .triled_mask = BIT(5) },
+		{ .base = 0xb600, .triled_mask = BIT(6) },
+		{ .base = 0xb700, .triled_mask = BIT(7) },
+		{ .base = 0xb800 },
+	},
+};
+
+static const struct lpg_data pm8994_lpg_data = {
+	.lut_base = 0xb000,
+	.lut_size = 64,
+
+	.pwm_9bit_mask = 3 << 4,
+
+	.num_channels = 6,
+	.channels = (struct lpg_channel_data[]) {
+		{ .base = 0xb100 },
+		{ .base = 0xb200 },
+		{ .base = 0xb300 },
+		{ .base = 0xb400 },
+		{ .base = 0xb500 },
+		{ .base = 0xb600 },
+	},
+};
+
+static const struct lpg_data pmi8994_lpg_data = {
+	.lut_base = 0xb000,
+	.lut_size = 24,
+
+	.triled_base = 0xd000,
+
+	.pwm_9bit_mask = BIT(4),
+
+	.num_channels = 4,
+	.channels = (struct lpg_channel_data[]) {
+		{ .base = 0xb100, .triled_mask = BIT(5) },
+		{ .base = 0xb200, .triled_mask = BIT(6) },
+		{ .base = 0xb300, .triled_mask = BIT(7) },
+		{ .base = 0xb400 },
+	},
+};
+
+static const struct lpg_data pmi8998_lpg_data = {
+	.lut_base = 0xb000,
+	.lut_size = 49,
+
+	.pwm_9bit_mask = BIT(4),
+
+	.num_channels = 6,
+	.channels = (struct lpg_channel_data[]) {
+		{ .base = 0xb100 },
+		{ .base = 0xb200 },
+		{ .base = 0xb300, .triled_mask = BIT(5) },
+		{ .base = 0xb400, .triled_mask = BIT(6) },
+		{ .base = 0xb500, .triled_mask = BIT(7) },
+		{ .base = 0xb600 },
+	},
+};
+
+static const struct of_device_id lpg_of_table[] = {
+	{ .compatible = "qcom,pm8916-pwm", .data = &pm8916_pwm_data },
+	{ .compatible = "qcom,pm8941-lpg", .data = &pm8941_lpg_data },
+	{ .compatible = "qcom,pm8994-lpg", .data = &pm8994_lpg_data },
+	{ .compatible = "qcom,pmi8994-lpg", .data = &pmi8994_lpg_data },
+	{ .compatible = "qcom,pmi8998-lpg", .data = &pmi8998_lpg_data },
+	{},
+};
+MODULE_DEVICE_TABLE(of, lpg_of_table);
+
+static struct platform_driver lpg_driver = {
+	.probe = lpg_probe,
+	.remove = lpg_remove,
+	.driver = {
+		.name = "qcom-spmi-lpg",
+		.of_match_table = lpg_of_table,
+	},
+};
+module_platform_driver(lpg_driver);
+
+MODULE_DESCRIPTION("Qualcomm TRI LED driver");
+MODULE_LICENSE("GPL v2");
-- 
2.15.0

^ permalink raw reply related

* [PATCH v3 1/3] leds: core: Introduce generic pattern interface
From: Bjorn Andersson @ 2017-11-15  7:13 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Fenglin Wu
In-Reply-To: <20171115071345.24331-1-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Some LED controllers have support for autonomously controlling
brightness over time, according to some preprogrammed pattern or
function.

This adds a new optional operator that LED class drivers can implement
if they support such functionality as well as a new device attribute to
configure the pattern for a given LED.

Signed-off-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---

Changes since v2:
- None

Changes since v1:
- New patch, based on discussions following v1

 Documentation/ABI/testing/sysfs-class-led |  20 ++++
 drivers/leds/led-class.c                  | 150 ++++++++++++++++++++++++++++++
 include/linux/leds.h                      |  21 +++++
 3 files changed, 191 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led
index 5f67f7ab277b..74a7f5b1f89b 100644
--- a/Documentation/ABI/testing/sysfs-class-led
+++ b/Documentation/ABI/testing/sysfs-class-led
@@ -61,3 +61,23 @@ Description:
 		gpio and backlight triggers. In case of the backlight trigger,
 		it is useful when driving a LED which is intended to indicate
 		a device in a standby like state.
+
+What:		/sys/class/leds/<led>/pattern
+Date:		July 2017
+KernelVersion:	4.14
+Description:
+		Specify a pattern for the LED, for LED hardware that support
+		altering the brightness as a function of time.
+
+		The pattern is given by a series of tuples, of brightness and
+		duration (ms). The LED is expected to traverse the series and
+		each brightness value for the specified duration.
+
+		Additionally a repeat marker ":|" can be appended to the
+		series, which should cause the pattern to be repeated
+		endlessly.
+
+		As LED hardware might have different capabilities and precision
+		the requested pattern might be slighly adjusted by the driver
+		and the resulting pattern of such operation should be returned
+		when this file is read.
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index b0e2d55acbd6..bd630e2ae967 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -74,6 +74,154 @@ static ssize_t max_brightness_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(max_brightness);
 
+static ssize_t pattern_show(struct device *dev,
+			    struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_pattern *pattern;
+	size_t offset = 0;
+	size_t count;
+	bool repeat;
+	size_t i;
+	int n;
+
+	if (!led_cdev->pattern_get)
+		return -EOPNOTSUPP;
+
+	pattern = led_cdev->pattern_get(led_cdev, &count, &repeat);
+	if (IS_ERR_OR_NULL(pattern))
+		return PTR_ERR(pattern);
+
+	for (i = 0; i < count; i++) {
+		n = snprintf(buf + offset, PAGE_SIZE - offset, "%d %d",
+			     pattern[i].brightness, pattern[i].delta_t);
+
+		if (offset + n >= PAGE_SIZE)
+			goto err_nospc;
+
+		offset += n;
+
+		if (i < count - 1)
+			buf[offset++] = ' ';
+	}
+
+	if (repeat) {
+		if (offset + 4 >= PAGE_SIZE)
+			goto err_nospc;
+
+		memcpy(buf + offset, " :|", 3);
+		offset += 3;
+	}
+
+	if (offset + 1 >= PAGE_SIZE)
+		goto err_nospc;
+
+	buf[offset++] = '\n';
+
+	kfree(pattern);
+	return offset;
+
+err_nospc:
+	kfree(pattern);
+	return -ENOSPC;
+}
+
+static ssize_t pattern_store(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_pattern *pattern = NULL;
+	unsigned long val;
+	char *sbegin;
+	char *elem;
+	char *s;
+	int len = 0;
+	int ret = 0;
+	bool odd = true;
+	bool repeat = false;
+
+	s = sbegin = kstrndup(buf, size, GFP_KERNEL);
+	if (!s)
+		return -ENOMEM;
+
+	/* Trim trailing newline */
+	s[strcspn(s, "\n")] = '\0';
+
+	/* If the remaining string is empty, clear the pattern */
+	if (!s[0]) {
+		ret = led_cdev->pattern_clear(led_cdev);
+		goto out;
+	}
+
+	pattern = kcalloc(size, sizeof(*pattern), GFP_KERNEL);
+	if (!pattern) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* Parse out the brightness & delta_t touples and check for repeat */
+	while ((elem = strsep(&s, " ")) != NULL) {
+		if (!strcmp(elem, ":|")) {
+			repeat = true;
+			break;
+		}
+
+		ret = kstrtoul(elem, 10, &val);
+		if (ret)
+			goto out;
+
+		if (odd) {
+			pattern[len].brightness = val;
+		} else {
+			/* Ensure we don't have any delta_t == 0 */
+			if (!val) {
+				ret = -EINVAL;
+				goto out;
+			}
+
+			pattern[len].delta_t = val;
+			len++;
+		}
+
+		odd = !odd;
+	}
+
+	/*
+	 * Fail if we didn't find any data points or last data point was partial
+	 */
+	if (!len || !odd) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = led_cdev->pattern_set(led_cdev, pattern, len, repeat);
+
+out:
+	kfree(pattern);
+	kfree(sbegin);
+	return ret < 0 ? ret : size;
+}
+
+static DEVICE_ATTR_RW(pattern);
+
+static umode_t led_class_attrs_mode(struct kobject *kobj,
+				    struct attribute *attr,
+				    int index)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+
+	if (attr == &dev_attr_brightness.attr)
+		return attr->mode;
+	if (attr == &dev_attr_max_brightness.attr)
+		return attr->mode;
+	if (attr == &dev_attr_pattern.attr && led_cdev->pattern_set)
+		return attr->mode;
+
+	return 0;
+}
+
 #ifdef CONFIG_LEDS_TRIGGERS
 static DEVICE_ATTR(trigger, 0644, led_trigger_show, led_trigger_store);
 static struct attribute *led_trigger_attrs[] = {
@@ -88,11 +236,13 @@ static const struct attribute_group led_trigger_group = {
 static struct attribute *led_class_attrs[] = {
 	&dev_attr_brightness.attr,
 	&dev_attr_max_brightness.attr,
+	&dev_attr_pattern.attr,
 	NULL,
 };
 
 static const struct attribute_group led_group = {
 	.attrs = led_class_attrs,
+	.is_visible = led_class_attrs_mode,
 };
 
 static const struct attribute_group *led_groups[] = {
diff --git a/include/linux/leds.h b/include/linux/leds.h
index bf6db4fe895b..584c79ff5bb5 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -33,6 +33,8 @@ enum led_brightness {
 	LED_FULL	= 255,
 };
 
+struct led_pattern;
+
 struct led_classdev {
 	const char		*name;
 	enum led_brightness	 brightness;
@@ -88,6 +90,15 @@ struct led_classdev {
 				     unsigned long *delay_on,
 				     unsigned long *delay_off);
 
+	int		(*pattern_set)(struct led_classdev *led_cdev,
+				       struct led_pattern *pattern, int len,
+				       bool repeat);
+
+	int		(*pattern_clear)(struct led_classdev *led_cdev);
+
+	struct led_pattern *(*pattern_get)(struct led_classdev *led_cdev,
+					   size_t *len, bool *repeat);
+
 	struct device		*dev;
 	const struct attribute_group	**groups;
 
@@ -446,4 +457,14 @@ static inline void led_classdev_notify_brightness_hw_changed(
 	struct led_classdev *led_cdev, enum led_brightness brightness) { }
 #endif
 
+/**
+ * struct led_pattern - brigheness value in a pattern
+ * @delta_t:	delay until next entry, in milliseconds
+ * @brightness:	brightness at time = 0
+ */
+struct led_pattern {
+	int delta_t;
+	int brightness;
+};
+
 #endif		/* __LINUX_LEDS_H_INCLUDED */
-- 
2.15.0

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

^ permalink raw reply related

* [PATCH v3 0/3] Qualcomm Light Pulse Generator
From: Bjorn Andersson @ 2017-11-15  7:13 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Fenglin Wu

This series introduces a generic pattern interface in the LED class and
a driver for the Qualcomm Light Pulse Generator.

Bjorn Andersson (3):
  leds: core: Introduce generic pattern interface
  leds: Add driver for Qualcomm LPG
  DT: leds: Add Qualcomm Light Pulse Generator binding

 Documentation/ABI/testing/sysfs-class-led          |   20 +
 .../devicetree/bindings/leds/leds-qcom-lpg.txt     |   66 ++
 drivers/leds/Kconfig                               |    7 +
 drivers/leds/Makefile                              |    1 +
 drivers/leds/led-class.c                           |  150 +++
 drivers/leds/leds-qcom-lpg.c                       | 1232 ++++++++++++++++++++
 include/linux/leds.h                               |   21 +
 7 files changed, 1497 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt
 create mode 100644 drivers/leds/leds-qcom-lpg.c

-- 
2.15.0

--
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


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