Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] of: base: add support to get machine model name
From: Frank Rowand @ 2016-11-18 20:00 UTC (permalink / raw)
  To: Rob Herring, Sudeep Holla
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161118144651.275xz4gu6jaefhp7@rob-hp-laptop>

On 11/18/16 06:46, Rob Herring wrote:
> On Thu, Nov 17, 2016 at 03:32:54PM +0000, Sudeep Holla wrote:
>> Currently platforms/drivers needing to get the machine model name are
>> replicating the same snippet of code. In some case, the OF reference
>> counting is either missing or incorrect.
>>
>> This patch adds support to read the machine model name either using
>> the "model" or the "compatible" property in the device tree root node
>> to the core OF/DT code.
>>
>> This can be used to remove all the duplicate code snippets doing exactly
>> same thing later.
>>
>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>> Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
>> ---
>>  drivers/of/base.c  | 32 ++++++++++++++++++++++++++++++++
>>  include/linux/of.h |  6 ++++++
>>  2 files changed, 38 insertions(+)
>>
>> Hi Rob,
>>
>> It would be good if we can target this for v4.10, so that we have no
>> dependencies to push PATCH 2/2 in v4.11
> 
> Applied.
> 
> Rob
> 

A little fast on the trigger Rob.

-Frank
--
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 v3 1/3] dt-bindings: firmware: scm: Add MSM8996 DT bindings
From: Bjorn Andersson @ 2016-11-18 19:55 UTC (permalink / raw)
  To: Sarangdhar Joshi
  Cc: Andy Gross, David Brown, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, linux-arm-msm, linux-soc,
	devicetree, linux-arm-kernel, linux-kernel, Jordan Crouse,
	Stephen Boyd, Trilok Soni
In-Reply-To: <1479259165-1601-2-git-send-email-spjoshi@codeaurora.org>

On Tue 15 Nov 17:19 PST 2016, Sarangdhar Joshi wrote:

> Add SCM DT bindings for Qualcomm's MSM8996 platform.
> 
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Sarangdhar Joshi <spjoshi@codeaurora.org>

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

Regards,
Bjorn

> ---
>  Documentation/devicetree/bindings/firmware/qcom,scm.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> index 3b4436e..20f26fb 100644
> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> @@ -10,8 +10,10 @@ Required properties:
>   * "qcom,scm-apq8064" for APQ8064 platforms
>   * "qcom,scm-msm8660" for MSM8660 platforms
>   * "qcom,scm-msm8690" for MSM8690 platforms
> + * "qcom,scm-msm8996" for MSM8996 platforms
>   * "qcom,scm" for later processors (MSM8916, APQ8084, MSM8974, etc)
>  - clocks: One to three clocks may be required based on compatible.
> + * No clock required for "qcom,scm-msm8996"
>   * Only core clock required for "qcom,scm-apq8064", "qcom,scm-msm8660", and "qcom,scm-msm8960"
>   * Core, iface, and bus clocks required for "qcom,scm"
>  - clock-names: Must contain "core" for the core clock, "iface" for the interface
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

^ permalink raw reply

* Re: [PATCH v3 2/2] DW DMAC: add multi-block property to device tree
From: Andy Shevchenko @ 2016-11-18 19:33 UTC (permalink / raw)
  To: Eugeniy Paltsev, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	vinod.koul-ral2JQCrhuEAvxtiuMwx3w,
	dmaengine-u79uwXL29TY76Z2rM5mHXA,
	linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1479496356-27834-3-git-send-email-Eugeniy.Paltsev-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

On Fri, 2016-11-18 at 22:12 +0300, Eugeniy Paltsev wrote:
> Several versions of DW DMAC have multi block transfers hardware
> support. Hardware support of multi block transfers is disabled
> by default if we use DT to configure DMAC and software emulation
> of multi block transfers used instead.
> Add multi-block property, so it is possible to enable hardware
> multi block transfers (if present) via DT.
> 
> Switch from per device is_nollp variable to multi_block array
> to be able enable/disable multi block transfers separately per
> channel.
> 

> Update DT documentation.
> 
> Update existing platform data.

Kinda useless for commit message, but might go after --- delimiter.

> 
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/dma/snps-dma.txt | 2 ++
>  drivers/dma/dw/core.c                              | 2 +-
>  drivers/dma/dw/platform.c                          | 5 +++++
>  drivers/tty/serial/8250/8250_lpss.c                | 2 +-
>  include/linux/platform_data/dma-dw.h               | 4 ++--
>  5 files changed, 11 insertions(+), 4 deletions(-)

> --- a/Documentation/devicetree/bindings/dma/snps-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
> @@ -27,6 +27,8 @@ Optional properties:
>    that services interrupts for this device
>  - is_private: The device channels should be marked as private and not
> for by the
>    general purpose DMA channel allocator. False if not passed.
> +- multi-block: Multi block transfers supported by hardware per AHB
> master.
> +  0 (default): not supported, 1: supported.
>  
>  Example:
>  
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index c2c0a61..f2a3d06 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -1569,7 +1569,7 @@ int dw_dma_probe(struct dw_dma_chip *chip)
>  				(dwc_params >> DWC_PARAMS_MBLK_EN &
> 0x1) == 0;
>  		} else {
>  			dwc->block_size = pdata->block_size;
> -			dwc->nollp = pdata->is_nollp;
> +			dwc->nollp = pdata->multi_block[i];

You missed the point. You assign positive value to negative variable.
It's a bug. Have you tested this? How?

In case of positive property you have to update DTS. By the way, I'm
pretty sure that spare13xx boards has auto configuration enabled, though
it has to be checked with vendor (I assume you may have fast response
from them).

>  		}
>  	}
>  

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
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 v8 2/3] Documentation: Add binding document for Lattice iCE40 FPGA manager
From: Marek Vasut @ 2016-11-18 19:28 UTC (permalink / raw)
  To: Moritz Fischer, atull
  Cc: Rob Herring, Joel Holdsworth, Geert Uytterhoeven, Devicetree List,
	Linux Kernel Mailing List, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	Clifford Wolf
In-Reply-To: <CAAtXAHcORUBHH-V98cK0CABtjXYB1afhLHb_2m-akn5tMcv5vQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 11/18/2016 08:17 PM, Moritz Fischer wrote:
> On Fri, Nov 18, 2016 at 10:56 AM, atull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org> wrote:
>> On Mon, 14 Nov 2016, Rob Herring wrote:
>>
>>> On Sun, Nov 06, 2016 at 07:49:21PM -0700, Joel Holdsworth wrote:
>>>> This adds documentation of the device tree bindings of the Lattice iCE40
>>>> FPGA driver for the FPGA manager framework.
>>>>
>>>> Signed-off-by: Joel Holdsworth <joel-IJEoVVyKhCJXvIrf17iDB/XRex20P6io@public.gmane.org>
>>>> ---
>>>>  .../bindings/fpga/lattice-ice40-fpga-mgr.txt        | 21 +++++++++++++++++++++
>>>>  1 file changed, 21 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/fpga/lattice-ice40-fpga-mgr.txt
>>>
>>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>
>>
>> Acked-by: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> Acked-by: Moritz Fischer <moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org>
Acked-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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/2] DW DMAC: enable memory-to-memory transfers support
From: Andy Shevchenko @ 2016-11-18 19:27 UTC (permalink / raw)
  To: Eugeniy Paltsev, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	vinod.koul-ral2JQCrhuEAvxtiuMwx3w,
	dmaengine-u79uwXL29TY76Z2rM5mHXA,
	linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1479496356-27834-2-git-send-email-Eugeniy.Paltsev-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

On Fri, 2016-11-18 at 22:12 +0300, Eugeniy Paltsev wrote:
> All known devices, which use DT for configuration, support
> memory-to-memory transfers. So enable it by default, if we read
> configuration from DT.
> 
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

You missed the given tag(s).

> ---
>  drivers/dma/dw/platform.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
> index 5bda0eb..aa7a5c1 100644
> --- a/drivers/dma/dw/platform.c
> +++ b/drivers/dma/dw/platform.c
> @@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev)
>  	if (of_property_read_bool(np, "is_private"))
>  		pdata->is_private = true;
>  
> +	/*
> +	 * All known devices, which use DT for configuration, support
> +	 * memory-to-memory transfers. So enable it by default.
> +	 */
> +	pdata->is_memcpy = true;
> +
>  	if (!of_property_read_u32(np, "chan_allocation_order", &tmp))
>  		pdata->chan_allocation_order = (unsigned char)tmp;
>  

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
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 v3 0/2] DW DMAC: update device tree
From: Andy Shevchenko @ 2016-11-18 19:26 UTC (permalink / raw)
  To: Eugeniy Paltsev, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	vinod.koul-ral2JQCrhuEAvxtiuMwx3w,
	dmaengine-u79uwXL29TY76Z2rM5mHXA,
	linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1479496356-27834-1-git-send-email-Eugeniy.Paltsev-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

On Fri, 2016-11-18 at 22:12 +0300, Eugeniy Paltsev wrote:
> It wasn't possible to enable some features like
> memory-to-memory transfers or multi block transfers via DT.
> It is fixed by these patches.

First of all, please, give time to reviewers to comment the patches.
Usually it should be at least 24h (for the series that has been sent
first time 1 week approximately).

> 
> Changes for v3:
>  * Update existing platform data.
>    We don't need to update existing DTS because default logic 
>    wasn't change: we don't set "is_nollp" if we read 
>    configuration from DT before. And we don't set it now if
>    "multi-block" property doesn't exist in DTS.

See my comments in the patches.
And do not send the updated version earlier than Monday, please.

> 
> Changes for v2:
>  * I thought about is_memcpy DT property: all known devices, which 
>    use DT for configuration, support memory-to-memory transfers. 
>    So we don't need to read it from DT. So enable it by default, 
>    if we read configuration from DT.
> 
>  * Use "multi-block" instead of "hw-llp" name to be more clear.
> 
>  * Move adding DT property and adding documentation for this
>    property to one patch.
> 
> Eugeniy Paltsev (2):
>   DW DMAC: enable memory-to-memory transfers support
>   DW DMAC: add multi-block property to device tree
> 
>  Documentation/devicetree/bindings/dma/snps-dma.txt |  2 ++
>  drivers/dma/dw/core.c                              |  2 +-
>  drivers/dma/dw/platform.c                          | 11 +++++++++++
>  drivers/tty/serial/8250/8250_lpss.c                |  2 +-
>  include/linux/platform_data/dma-dw.h               |  4 ++--
>  5 files changed, 17 insertions(+), 4 deletions(-)
> 

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 1/2] of: Add vendor prefix for Lattice Semiconductor
From: Moritz Fischer @ 2016-11-18 19:17 UTC (permalink / raw)
  To: atull
  Cc: Joel Holdsworth, Devicetree List, Linux Kernel Mailing List,
	linux-spi
In-Reply-To: <alpine.DEB.2.20.1611181257340.3387@atull-VirtualBox2>

On Fri, Nov 18, 2016 at 10:59 AM, atull <atull@opensource.altera.com> wrote:
> On Sat, 29 Oct 2016, Joel Holdsworth wrote:
>
>> ---
>>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> index 1992aa9..d64a835 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> @@ -146,6 +146,7 @@ kosagi    Sutajio Ko-Usagi PTE Ltd.
>>  kyo  Kyocera Corporation
>>  lacie        LaCie
>>  lantiq       Lantiq Semiconductor
>> +lattice      Lattice Semiconductor
>>  lenovo       Lenovo Group Ltd.
>>  lg   LG Corporation
>>  linux        Linux-specific binding
>> --
>> 2.7.4
>>
>>
>
> Acked-by: Alan Tull <atull@opensource.altera.com>
Acked-by: Moritz Fischer <moritz.fischer@ettus.com>

^ permalink raw reply

* Re: [PATCH v8 2/3] Documentation: Add binding document for Lattice iCE40 FPGA manager
From: Moritz Fischer @ 2016-11-18 19:17 UTC (permalink / raw)
  To: atull
  Cc: Rob Herring, Joel Holdsworth, Geert Uytterhoeven, Devicetree List,
	Linux Kernel Mailing List, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	Marek Vašut, Clifford Wolf
In-Reply-To: <alpine.DEB.2.20.1611181255120.3387@atull-VirtualBox2>

On Fri, Nov 18, 2016 at 10:56 AM, atull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org> wrote:
> On Mon, 14 Nov 2016, Rob Herring wrote:
>
>> On Sun, Nov 06, 2016 at 07:49:21PM -0700, Joel Holdsworth wrote:
>> > This adds documentation of the device tree bindings of the Lattice iCE40
>> > FPGA driver for the FPGA manager framework.
>> >
>> > Signed-off-by: Joel Holdsworth <joel-IJEoVVyKhCJXvIrf17iDB/XRex20P6io@public.gmane.org>
>> > ---
>> >  .../bindings/fpga/lattice-ice40-fpga-mgr.txt        | 21 +++++++++++++++++++++
>> >  1 file changed, 21 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/fpga/lattice-ice40-fpga-mgr.txt
>>
>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>
>
> Acked-by: Alan Tull <atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
Acked-by: Moritz Fischer <moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v3 2/2] DW DMAC: add multi-block property to device tree
From: Eugeniy Paltsev @ 2016-11-18 19:12 UTC (permalink / raw)
  To: devicetree
  Cc: mark.rutland, linux-snps-arc, vinod.koul, linux-kernel, robh+dt,
	dmaengine, andriy.shevchenko, Eugeniy Paltsev
In-Reply-To: <1479496356-27834-1-git-send-email-Eugeniy.Paltsev@synopsys.com>

Several versions of DW DMAC have multi block transfers hardware
support. Hardware support of multi block transfers is disabled
by default if we use DT to configure DMAC and software emulation
of multi block transfers used instead.
Add multi-block property, so it is possible to enable hardware
multi block transfers (if present) via DT.

Switch from per device is_nollp variable to multi_block array
to be able enable/disable multi block transfers separately per
channel.

Update DT documentation.

Update existing platform data.

Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 Documentation/devicetree/bindings/dma/snps-dma.txt | 2 ++
 drivers/dma/dw/core.c                              | 2 +-
 drivers/dma/dw/platform.c                          | 5 +++++
 drivers/tty/serial/8250/8250_lpss.c                | 2 +-
 include/linux/platform_data/dma-dw.h               | 4 ++--
 5 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt b/Documentation/devicetree/bindings/dma/snps-dma.txt
index 0f55832..03d6d6d 100644
--- a/Documentation/devicetree/bindings/dma/snps-dma.txt
+++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
@@ -27,6 +27,8 @@ Optional properties:
   that services interrupts for this device
 - is_private: The device channels should be marked as private and not for by the
   general purpose DMA channel allocator. False if not passed.
+- multi-block: Multi block transfers supported by hardware per AHB master.
+  0 (default): not supported, 1: supported.
 
 Example:
 
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index c2c0a61..f2a3d06 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1569,7 +1569,7 @@ int dw_dma_probe(struct dw_dma_chip *chip)
 				(dwc_params >> DWC_PARAMS_MBLK_EN & 0x1) == 0;
 		} else {
 			dwc->block_size = pdata->block_size;
-			dwc->nollp = pdata->is_nollp;
+			dwc->nollp = pdata->multi_block[i];
 		}
 	}
 
diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index aa7a5c1..b262fd3 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -152,6 +152,11 @@ dw_dma_parse_dt(struct platform_device *pdev)
 			pdata->data_width[tmp] = BIT(arr[tmp] & 0x07);
 	}
 
+	if (!of_property_read_u32_array(np, "multi-block", arr, nr_masters)) {
+		for (tmp = 0; tmp < nr_masters; tmp++)
+			pdata->multi_block[tmp] = arr[tmp];
+	}
+
 	return pdata;
 }
 #else
diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
index f607946..58cbb30 100644
--- a/drivers/tty/serial/8250/8250_lpss.c
+++ b/drivers/tty/serial/8250/8250_lpss.c
@@ -157,12 +157,12 @@ static int byt_serial_setup(struct lpss8250 *lpss, struct uart_port *port)
 static const struct dw_dma_platform_data qrk_serial_dma_pdata = {
 	.nr_channels = 2,
 	.is_private = true,
-	.is_nollp = true,
 	.chan_allocation_order = CHAN_ALLOCATION_ASCENDING,
 	.chan_priority = CHAN_PRIORITY_ASCENDING,
 	.block_size = 4095,
 	.nr_masters = 1,
 	.data_width = {4},
+	.multi_block = {0},
 };
 
 static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h
index 5f0e11e..0773bb4 100644
--- a/include/linux/platform_data/dma-dw.h
+++ b/include/linux/platform_data/dma-dw.h
@@ -40,19 +40,18 @@ struct dw_dma_slave {
  * @is_private: The device channels should be marked as private and not for
  *	by the general purpose DMA channel allocator.
  * @is_memcpy: The device channels do support memory-to-memory transfers.
- * @is_nollp: The device channels does not support multi block transfers.
  * @chan_allocation_order: Allocate channels starting from 0 or 7
  * @chan_priority: Set channel priority increasing from 0 to 7 or 7 to 0.
  * @block_size: Maximum block size supported by the controller
  * @nr_masters: Number of AHB masters supported by the controller
  * @data_width: Maximum data width supported by hardware per AHB master
  *		(in bytes, power of 2)
+ * @multi_block: Multi block transfers supported by hardware per AHB master.
  */
 struct dw_dma_platform_data {
 	unsigned int	nr_channels;
 	bool		is_private;
 	bool		is_memcpy;
-	bool		is_nollp;
 #define CHAN_ALLOCATION_ASCENDING	0	/* zero to seven */
 #define CHAN_ALLOCATION_DESCENDING	1	/* seven to zero */
 	unsigned char	chan_allocation_order;
@@ -62,6 +61,7 @@ struct dw_dma_platform_data {
 	unsigned int	block_size;
 	unsigned char	nr_masters;
 	unsigned char	data_width[DW_DMA_MAX_NR_MASTERS];
+	unsigned char	multi_block[DW_DMA_MAX_NR_MASTERS];
 };
 
 #endif /* _PLATFORM_DATA_DMA_DW_H */
-- 
2.5.5

^ permalink raw reply related

* [PATCH v3 1/2] DW DMAC: enable memory-to-memory transfers support
From: Eugeniy Paltsev @ 2016-11-18 19:12 UTC (permalink / raw)
  To: devicetree
  Cc: mark.rutland, linux-snps-arc, vinod.koul, linux-kernel, robh+dt,
	dmaengine, andriy.shevchenko, Eugeniy Paltsev
In-Reply-To: <1479496356-27834-1-git-send-email-Eugeniy.Paltsev@synopsys.com>

All known devices, which use DT for configuration, support
memory-to-memory transfers. So enable it by default, if we read
configuration from DT.

Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 drivers/dma/dw/platform.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index 5bda0eb..aa7a5c1 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev)
 	if (of_property_read_bool(np, "is_private"))
 		pdata->is_private = true;
 
+	/*
+	 * All known devices, which use DT for configuration, support
+	 * memory-to-memory transfers. So enable it by default.
+	 */
+	pdata->is_memcpy = true;
+
 	if (!of_property_read_u32(np, "chan_allocation_order", &tmp))
 		pdata->chan_allocation_order = (unsigned char)tmp;
 
-- 
2.5.5

^ permalink raw reply related

* [PATCH v3 0/2] DW DMAC: update device tree
From: Eugeniy Paltsev @ 2016-11-18 19:12 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
	vinod.koul-ral2JQCrhuEAvxtiuMwx3w,
	dmaengine-u79uwXL29TY76Z2rM5mHXA,
	linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Eugeniy Paltsev

It wasn't possible to enable some features like
memory-to-memory transfers or multi block transfers via DT.
It is fixed by these patches.

Changes for v3:
 * Update existing platform data.
   We don't need to update existing DTS because default logic 
   wasn't change: we don't set "is_nollp" if we read 
   configuration from DT before. And we don't set it now if
   "multi-block" property doesn't exist in DTS.

Changes for v2:
 * I thought about is_memcpy DT property: all known devices, which 
   use DT for configuration, support memory-to-memory transfers. 
   So we don't need to read it from DT. So enable it by default, 
   if we read configuration from DT.

 * Use "multi-block" instead of "hw-llp" name to be more clear.

 * Move adding DT property and adding documentation for this
   property to one patch.

Eugeniy Paltsev (2):
  DW DMAC: enable memory-to-memory transfers support
  DW DMAC: add multi-block property to device tree

 Documentation/devicetree/bindings/dma/snps-dma.txt |  2 ++
 drivers/dma/dw/core.c                              |  2 +-
 drivers/dma/dw/platform.c                          | 11 +++++++++++
 drivers/tty/serial/8250/8250_lpss.c                |  2 +-
 include/linux/platform_data/dma-dw.h               |  4 ++--
 5 files changed, 17 insertions(+), 4 deletions(-)

-- 
2.5.5

--
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 1/2] mfd: rn5t618: Add Ricoh RC5T619 PMIC support
From: Lee Jones @ 2016-11-18 19:05 UTC (permalink / raw)
  To: Pierre-Hugues Husson
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161105161925.14910-2-phh-8tEavu1zA38@public.gmane.org>

On Sat, 05 Nov 2016, Pierre-Hugues Husson wrote:

> The Ricoh RN5T567 is from the same family as the Ricoh RN5T618 is,
> the differences are:
> 
> + DCDC4/DCDC5
> + LDO7-10
> + Slightly different output voltage/currents
> + 32kHz Output
> + RTC
> + USB Charger detection
> 
> Signed-off-by: Pierre-Hugues Husson <phh-8tEavu1zA38@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mfd/rn5t618.txt | 16 ++++++++++------
>  drivers/mfd/Kconfig                               |  3 ++-
>  drivers/mfd/rn5t618.c                             |  1 +
>  include/linux/mfd/rn5t618.h                       |  9 +++++++++
>  4 files changed, 22 insertions(+), 7 deletions(-)

Applied, thanks.

> diff --git a/Documentation/devicetree/bindings/mfd/rn5t618.txt b/Documentation/devicetree/bindings/mfd/rn5t618.txt
> index 9e6770b..65c2326 100644
> --- a/Documentation/devicetree/bindings/mfd/rn5t618.txt
> +++ b/Documentation/devicetree/bindings/mfd/rn5t618.txt
> @@ -1,21 +1,25 @@
>  * Ricoh RN5T567/RN5T618 PMIC
>  
> -Ricoh RN5T567/RN5T618 is a power management IC family which integrates
> -3 to 4 step-down DCDC converters, 7 low-dropout regulators, GPIOs and
> -a watchdog timer. The RN5T618 provides additionally a Li-ion battery
> -charger, fuel gauge and an ADC. It can be controlled through an I2C
> -interface.
> +Ricoh RN5T567/RN5T618/RC5T619 is a power management IC family which
> +integrates 3 to 5 step-down DCDC converters, 7 to 10 low-dropout regulators,
> +GPIOs, and a watchdog timer. It can be controlled through an I2C interface.
> +The RN5T618/RC5T619 provides additionally a Li-ion battery charger,
> +fuel gauge, and an ADC.
> +The RC5T619 additionnally includes USB charger detection and an RTC.
>  
>  Required properties:
>   - compatible: must be one of
>  		"ricoh,rn5t567"
>  		"ricoh,rn5t618"
> +		"ricoh,rc5t619"
>   - reg: the I2C slave address of the device
>  
>  Sub-nodes:
>   - regulators: the node is required if the regulator functionality is
>     needed. The valid regulator names are: DCDC1, DCDC2, DCDC3, DCDC4
> -   (RN5T567), LDO1, LDO2, LDO3, LDO4, LDO5, LDORTC1 and LDORTC2.
> +   (RN5T567/RC5T619), LDO1, LDO2, LDO3, LDO4, LDO5, LDO6, LDO7, LDO8,
> +   LDO9, LDO10, LDORTC1 and LDORTC2.
> +   LDO7-10 are specific to RC5T619.
>     The common bindings for each individual regulator can be found in:
>     Documentation/devicetree/bindings/regulator/regulator.txt
>  
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index c6df644..65d0a65 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -881,7 +881,8 @@ config MFD_RN5T618
>  	select MFD_CORE
>  	select REGMAP_I2C
>  	help
> -	  Say yes here to add support for the Ricoh RN5T567 or R5T618 PMIC.
> +	  Say yes here to add support for the Ricoh RN5T567,
> +          RN5T618, RC5T619 PMIC.
>  	  This driver provides common support for accessing the device,
>  	  additional drivers must be enabled in order to use the
>  	  functionality of the device.
> diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
> index 9d377d6..0b5a2a1 100644
> --- a/drivers/mfd/rn5t618.c
> +++ b/drivers/mfd/rn5t618.c
> @@ -87,6 +87,7 @@ static int rn5t618_restart(struct notifier_block *this,
>  static const struct of_device_id rn5t618_of_match[] = {
>  	{ .compatible = "ricoh,rn5t567", .data = (void *)RN5T567 },
>  	{ .compatible = "ricoh,rn5t618", .data = (void *)RN5T618 },
> +	{ .compatible = "ricoh,rc5t619", .data = (void *)RC5T619 },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, rn5t618_of_match);
> diff --git a/include/linux/mfd/rn5t618.h b/include/linux/mfd/rn5t618.h
> index cadc654..e5a6cde 100644
> --- a/include/linux/mfd/rn5t618.h
> +++ b/include/linux/mfd/rn5t618.h
> @@ -58,10 +58,13 @@
>  #define RN5T618_DC3CTL2			0x31
>  #define RN5T618_DC4CTL			0x32
>  #define RN5T618_DC4CTL2			0x33
> +#define RN5T618_DC5CTL			0x34
> +#define RN5T618_DC5CTL2			0x35
>  #define RN5T618_DC1DAC			0x36
>  #define RN5T618_DC2DAC			0x37
>  #define RN5T618_DC3DAC			0x38
>  #define RN5T618_DC4DAC			0x39
> +#define RN5T618_DC5DAC			0x3a
>  #define RN5T618_DC1DAC_SLP		0x3b
>  #define RN5T618_DC2DAC_SLP		0x3c
>  #define RN5T618_DC3DAC_SLP		0x3d
> @@ -77,6 +80,11 @@
>  #define RN5T618_LDO3DAC			0x4e
>  #define RN5T618_LDO4DAC			0x4f
>  #define RN5T618_LDO5DAC			0x50
> +#define RN5T618_LDO6DAC			0x51
> +#define RN5T618_LDO7DAC			0x52
> +#define RN5T618_LDO8DAC			0x53
> +#define RN5T618_LDO9DAC			0x54
> +#define RN5T618_LDO10DAC		0x55
>  #define RN5T618_LDORTCDAC		0x56
>  #define RN5T618_LDORTC2DAC		0x57
>  #define RN5T618_LDO1DAC_SLP		0x58
> @@ -231,6 +239,7 @@ enum {
>  enum {
>  	RN5T567 = 0,
>  	RN5T618,
> +	RC5T619,
>  };
>  
>  struct rn5t618 {

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 1/2] of: Add vendor prefix for Lattice Semiconductor
From: atull @ 2016-11-18 18:59 UTC (permalink / raw)
  To: Joel Holdsworth; +Cc: moritz.fischer, devicetree, linux-kernel, linux-spi
In-Reply-To: <1477776746-3678-1-git-send-email-joel@airwebreathe.org.uk>

On Sat, 29 Oct 2016, Joel Holdsworth wrote:

> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 1992aa9..d64a835 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -146,6 +146,7 @@ kosagi	Sutajio Ko-Usagi PTE Ltd.
>  kyo	Kyocera Corporation
>  lacie	LaCie
>  lantiq	Lantiq Semiconductor
> +lattice	Lattice Semiconductor
>  lenovo	Lenovo Group Ltd.
>  lg	LG Corporation
>  linux	Linux-specific binding
> -- 
> 2.7.4
> 
> 

Acked-by: Alan Tull <atull@opensource.altera.com>

^ permalink raw reply

* Re: [PATCHv0 1/1] fbdev: add Intel FPGA FRAME BUFFER driver
From: Rob Herring @ 2016-11-18 18:56 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: Ong, Hean Loong, Tomi Valkeinen,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20161118141547.465c431e-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>

On Fri, Nov 18, 2016 at 8:15 AM, One Thousand Gnomes
<gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org> wrote:
>> AIUI, we're not taking new FB drivers. This should be a DRM driver
>> instead.
>
> Yes - clone one of the dumb DRM drivers, or if you've got any little bits
> of acceleration (even rolling the display) then it's possibly worth
> accelerating for text mode.
>
>> > +- max-width: The width of the framebuffer in pixels.
>> > +- max-height: The height of the framebuffer in pixels.
>> > +- bits-per-color: only "8" is currently supported
>>
>> These are not h/w properties.
>
> How are the max ones not hardware properties ?

Because the way they are used is setting the mode, not some check of
the max when the mode is set. If this is synthesized for only one
size, then that would be different, but we have bindings for modes.

Rob
--
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 v8 2/3] Documentation: Add binding document for Lattice iCE40 FPGA manager
From: atull @ 2016-11-18 18:56 UTC (permalink / raw)
  To: Rob Herring
  Cc: Joel Holdsworth, moritz.fischer, geert, devicetree, linux-kernel,
	linux-spi, marex, clifford
In-Reply-To: <20161114161130.2arzmaev6ajsw4qt@rob-hp-laptop>

On Mon, 14 Nov 2016, Rob Herring wrote:

> On Sun, Nov 06, 2016 at 07:49:21PM -0700, Joel Holdsworth wrote:
> > This adds documentation of the device tree bindings of the Lattice iCE40
> > FPGA driver for the FPGA manager framework.
> > 
> > Signed-off-by: Joel Holdsworth <joel@airwebreathe.org.uk>
> > ---
> >  .../bindings/fpga/lattice-ice40-fpga-mgr.txt        | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/fpga/lattice-ice40-fpga-mgr.txt
> 
> Acked-by: Rob Herring <robh@kernel.org>
> 

Acked-by: Alan Tull <atull@opensource.altera.com>

^ permalink raw reply

* Re: [PATCH v2 2/6] mfd: dt: Add bindings for the Aspeed SoC Display Controller (GFX)
From: Lee Jones @ 2016-11-18 18:47 UTC (permalink / raw)
  To: Andrew Jeffery, arnd
  Cc: Linus Walleij, Joel Stanley, Mark Rutland, Rob Herring,
	linux-gpio, linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <1478097481-14895-3-git-send-email-andrew@aj.id.au>

On Thu, 03 Nov 2016, Andrew Jeffery wrote:

> The Aspeed SoC Display Controller is presented as a syscon device to
> arbitrate access by display and pinmux drivers. Video pinmux
> configuration on fifth generation SoCs depends on bits in both the
> System Control Unit and the Display Controller.
> 
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>  Documentation/devicetree/bindings/mfd/aspeed-gfx.txt | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/aspeed-gfx.txt

Same here.

> diff --git a/Documentation/devicetree/bindings/mfd/aspeed-gfx.txt b/Documentation/devicetree/bindings/mfd/aspeed-gfx.txt
> new file mode 100644
> index 000000000000..aea5370efd97
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/aspeed-gfx.txt
> @@ -0,0 +1,17 @@
> +* Device tree bindings for Aspeed SoC Display Controller (GFX)
> +
> +The Aspeed SoC Display Controller primarily does as its name suggests, but also
> +participates in pinmux requests on the g5 SoCs. It is therefore considered a
> +syscon device.
> +
> +Required properties:
> +- compatible:		"aspeed,ast2500-gfx", "syscon"
> +- reg:			contains offset/length value of the GFX memory
> +			region.
> +
> +Example:
> +
> +gfx: display@1e6e6000 {
> +	compatible = "aspeed,ast2500-gfx", "syscon";
> +	reg = <0x1e6e6000 0x1000>;
> +};

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH v2 3/6] mfd: dt: Add bindings for the Aspeed LPC Host Controller (LPCHC)
From: Lee Jones @ 2016-11-18 18:45 UTC (permalink / raw)
  To: Andrew Jeffery, arnd-r2nGTMty4D4
  Cc: Linus Walleij, Joel Stanley, Mark Rutland, Rob Herring,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161118184437.GD19884-Re9dqnLqz4GzQB+pC5nmwQ@public.gmane.org>

[Sending Arnd this time!]

> Arnd,
> 
> Do you have a preference?
> 
> > The Aspeed LPC Host Controller is presented as a syscon device to
> > arbitrate access by LPC and pinmux drivers. LPC pinmux configuration on
> > fifth generation SoCs depends on bits in both the System Control Unit
> > and the LPC Host Controller.
> > 
> > Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt | 17 +++++++++++++++++
> 
> Create a new directory in bindings/mfd called 'syscon'.
> 
> Or perhaps 'bindings/syscon'.
> 
> >  1 file changed, 17 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> > new file mode 100644
> > index 000000000000..792651488c3d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> > @@ -0,0 +1,17 @@
> > +* Device tree bindings for the Aspeed LPC Host Controller (LPCHC)
> > +
> > +The LPCHC registers configure LPC behaviour between the BMC and the host
> > +system. The LPCHC also participates in pinmux requests on g5 SoCs and is
> > +therefore considered a syscon device.
> > +
> > +Required properties:
> > +- compatible:		"aspeed,ast2500-lpchc", "syscon"
> > +- reg:			contains offset/length value of the LPCHC memory
> > +			region.
> 
> Why not just use a single tab, then you don't have to linewrap?
> 
> > +Example:
> > +
> > +lpchc: lpchc@1e7890a0 {
> > +	compatible = "aspeed,ast2500-lpchc", "syscon";
> > +	reg = <0x1e7890a0 0xc4>;
> > +};
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 3/6] mfd: dt: Add bindings for the Aspeed LPC Host Controller (LPCHC)
From: Lee Jones @ 2016-11-18 18:44 UTC (permalink / raw)
  To: Andrew Jeffery
  Cc: Linus Walleij, Joel Stanley, Mark Rutland, Rob Herring,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478097481-14895-4-git-send-email-andrew-zrmu5oMJ5Fs@public.gmane.org>

Arnd,

Do you have a preference?

> The Aspeed LPC Host Controller is presented as a syscon device to
> arbitrate access by LPC and pinmux drivers. LPC pinmux configuration on
> fifth generation SoCs depends on bits in both the System Control Unit
> and the LPC Host Controller.
> 
> Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt | 17 +++++++++++++++++

Create a new directory in bindings/mfd called 'syscon'.

Or perhaps 'bindings/syscon'.

>  1 file changed, 17 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> new file mode 100644
> index 000000000000..792651488c3d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> @@ -0,0 +1,17 @@
> +* Device tree bindings for the Aspeed LPC Host Controller (LPCHC)
> +
> +The LPCHC registers configure LPC behaviour between the BMC and the host
> +system. The LPCHC also participates in pinmux requests on g5 SoCs and is
> +therefore considered a syscon device.
> +
> +Required properties:
> +- compatible:		"aspeed,ast2500-lpchc", "syscon"
> +- reg:			contains offset/length value of the LPCHC memory
> +			region.

Why not just use a single tab, then you don't have to linewrap?

> +Example:
> +
> +lpchc: lpchc@1e7890a0 {
> +	compatible = "aspeed,ast2500-lpchc", "syscon";
> +	reg = <0x1e7890a0 0xc4>;
> +};

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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 1/2] backlight: arcxcnn: add support for ArticSand devices
From: Lee Jones @ 2016-11-18 18:40 UTC (permalink / raw)
  To: Olimpiu Dejeu
  Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	jingoohan1-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1478792647-5813-1-git-send-email-olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>

I'll give this a quick once over, but Jingoo will also need to review.

On Thu, 10 Nov 2016, Olimpiu Dejeu wrote:

> Resubmition of arcxcnn backlight driver addressing the naming convention
>  concerns raised by Rob H.

This is not a commit log.  It's a changelog which lives at <####>.

> Signed-off-by: Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> 
> ---

<####>

>  drivers/video/backlight/Kconfig      |   7 +
>  drivers/video/backlight/Makefile     |   1 +
>  drivers/video/backlight/arcxcnn_bl.c | 541 +++++++++++++++++++++++++++++++++++
>  include/linux/i2c/arcxcnn.h          |  67 +++++
>  4 files changed, 616 insertions(+)
>  create mode 100644 drivers/video/backlight/arcxcnn_bl.c
>  create mode 100644 include/linux/i2c/arcxcnn.h
> 
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 5ffa4b4..4e1d2ad 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -460,6 +460,13 @@ config BACKLIGHT_BD6107
>  	help
>  	  If you have a Rohm BD6107 say Y to enable the backlight driver.
>  
> +config BACKLIGHT_ARCXCNN
> +	tristate "Backlight driver for the Arctic Sands ARCxCnnnn family"
> +	depends on I2C
> +	help
> +	  If you have an ARCxCnnnn family backlight say Y to enable
> +	  the backlight driver.
> +
>  endif # BACKLIGHT_CLASS_DEVICE
>  
>  endif # BACKLIGHT_LCD_SUPPORT
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 16ec534..8905129 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -55,3 +55,4 @@ obj-$(CONFIG_BACKLIGHT_SKY81452)	+= sky81452-backlight.o
>  obj-$(CONFIG_BACKLIGHT_TOSA)		+= tosa_bl.o
>  obj-$(CONFIG_BACKLIGHT_TPS65217)	+= tps65217_bl.o
>  obj-$(CONFIG_BACKLIGHT_WM831X)		+= wm831x_bl.o
> +obj-$(CONFIG_BACKLIGHT_ARCXCNN) 	+= arcxcnn_bl.o
> diff --git a/drivers/video/backlight/arcxcnn_bl.c b/drivers/video/backlight/arcxcnn_bl.c
> new file mode 100644
> index 0000000..1dad680
> --- /dev/null
> +++ b/drivers/video/backlight/arcxcnn_bl.c
> @@ -0,0 +1,541 @@
> +/*
> + * Backlight driver for ArcticSand ARC_X_C_0N_0N Devices
> + *
> + * Copyright 2016 ArcticSand, Inc.
> + *
> + * Licensed under the GPL-2 or later.

As far as I'm aware, this does not constitude as a proper GPL licence
statement.

> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/backlight.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/pwm.h>
> +#include <linux/regulator/consumer.h>

Alphabetical.

> +#include "linux/i2c/arcxcnn.h"
> +
> +#define ARCXCNN_CMD		(0x00)  /* Command Register */
> +#define ARCXCNN_CMD_STDBY	(0x80)	/* I2C Standby */
> +#define ARCXCNN_CMD_RESET	(0x40)	/* Reset */
> +#define ARCXCNN_CMD_BOOST	(0x10)	/* Boost */
> +#define ARCXCNN_CMD_OVP_MASK	(0x0C)	/* --- Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_XXV	(0x0C)	/* <rsvrd> Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_20V	(0x08)	/* 20v Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_24V	(0x04)	/* 24v Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_31V	(0x00)	/* 31.4v Over Voltage Threshold */
> +#define ARCXCNN_CMD_EXT_COMP	(0x01)	/* part (0) or full (1) external comp */
> +
> +#define ARCXCNN_CONFIG	(0x01)  /* Configuration */
> +#define ARCXCNN_STATUS1	(0x02)  /* Status 1 */
> +#define ARCXCNN_STATUS2	(0x03)  /* Status 2 */
> +#define ARCXCNN_FADECTRL	(0x04)  /* Fading Control */
> +#define ARCXCNN_ILED_CONFIG	(0x05)  /* ILED Configuration */
> +
> +#define ARCXCNN_LEDEN		(0x06)  /* LED Enable Register */
> +#define ARCXCNN_LEDEN_ISETEXT	(0x80)	/* Full-scale current set externally */
> +#define ARCXCNN_LEDEN_MASK	(0x3F)	/* LED string enables */
> +#define ARCXCNN_LEDEN_LED1	(0x01)
> +#define ARCXCNN_LEDEN_LED2	(0x02)
> +#define ARCXCNN_LEDEN_LED3	(0x04)
> +#define ARCXCNN_LEDEN_LED4	(0x08)
> +#define ARCXCNN_LEDEN_LED5	(0x10)
> +#define ARCXCNN_LEDEN_LED6	(0x20)
> +
> +#define ARCXCNN_WLED_ISET_LSB	(0x07)  /* LED ISET LSB (in upper nibble) */
> +#define ARCXCNN_WLED_ISET_MSB	(0x08)  /* LED ISET MSB (8 bits) */
> +
> +#define ARCXCNN_DIMFREQ		(0x09)
> +#define ARCXCNN_COMP_CONFIG	(0x0A)
> +#define ARCXCNN_FILT_CONFIG	(0x0B)
> +#define ARCXCNN_IMAXTUNE	(0x0C)

No need for simple defines to be place inside parenthesis.

> +#define DEFAULT_BL_NAME		"arctic_bl"

Don't do this.  Just use the name in the place you need to use it.

> +#define MAX_BRIGHTNESS		4095
> +
> +static int s_no_reset_on_remove;
> +module_param_named(noreset, s_no_reset_on_remove, int, 0644);
> +MODULE_PARM_DESC(noreset, "No reset on module removal");
> +
> +static int s_ibright = 60;
> +module_param_named(ibright, s_ibright, int, 0644);
> +MODULE_PARM_DESC(ibright, "Initial brightness (when no plat data)");
> +
> +static int s_iledstr = 0x3F;
> +module_param_named(iledstr, s_iledstr, int, 0644);
> +MODULE_PARM_DESC(iledstr, "Initial LED String (when no plat data)");
> +
> +static int s_retries = 2; /* 1 == only one try */
> +module_param_named(retries, s_retries, int, 0644);
> +MODULE_PARM_DESC(retries, "I2C retries attempted");
> +
> +enum arcxcnn_brightness_ctrl_mode {
> +	PWM_BASED = 1,
> +	REGISTER_BASED,
> +};
> +
> +struct arcxcnn;

Why?

> +struct arcxcnn {
> +	char chipname[64];
> +	enum arcxcnn_chip_id chip_id;
> +	enum arcxcnn_brightness_ctrl_mode mode;
> +	struct i2c_client *client;
> +	struct backlight_device *bl;
> +	struct device *dev;

You only need one of 'bl', 'dev' and 'client'.

> +	struct arcxcnn_platform_data *pdata;

pdata should not be stored in ddata.

> +	struct pwm_device *pwm;
> +	struct regulator *supply;	/* regulator for VDD input */
> +};

Document using kerneldoc.

> +static int arcxcnn_write_byte(struct arcxcnn *lp, u8 reg, u8 data)
> +{
> +	s32 ret = -1;
> +	int att;
> +
> +	for (att = 0; att < s_retries; att++) {

This is non-standard.

Why would it fail on first attempt and succeed on the next?

> +		ret = i2c_smbus_write_byte_data(lp->client, reg, data);
> +		if (ret >= 0)
> +			return 0;
> +	}
> +	return ret;
> +}
> +
> +static u8 arcxcnn_read_byte(struct arcxcnn *lp, u8 reg)
> +{
> +	int val;
> +	int att;
> +
> +	for (att = 0; att < s_retries; att++) {
> +		val = i2c_smbus_read_byte_data(lp->client, reg);
> +		if (val >= 0)
> +			return (u8)val;
> +	}
> +	return 0;
> +}
> +
> +static int arcxcnn_update_bit(struct arcxcnn *lp, u8 reg, u8 mask, u8 data)
> +{
> +	int ret, att;
> +	u8 tmp;
> +
> +	for (att = 0, ret = -1; att < s_retries; att++) {
> +		ret = i2c_smbus_read_byte_data(lp->client, reg);
> +		if (ret >= 0)
> +			break;
> +	}
> +	if (ret < 0) {
> +		dev_err(lp->dev, "failed to read 0x%.2x\n", reg);
> +		return ret;
> +	}
> +
> +	tmp = (u8)ret;
> +	tmp &= ~mask;
> +	tmp |= data & mask;
> +
> +	return arcxcnn_write_byte(lp, reg, tmp);
> +}

I'd really rather you didn't role your own read/write/update API!

> +static int arcxcnn_set_brightness(struct arcxcnn *lp, u32 brightness)
> +{
> +	int ret;
> +	u8 val;
> +
> +	val = (brightness & 0xF) << 4;

Define the shift value.

> +	ret = arcxcnn_write_byte(lp, ARCXCNN_WLED_ISET_LSB, val);
> +	if (ret < 0)
> +		return ret;
> +	val = (brightness >> 4);
> +	ret = arcxcnn_write_byte(lp, ARCXCNN_WLED_ISET_MSB, val);

'\n' here.

> +	return ret;
> +}
> +
> +static int arcxcnn_bl_update_status(struct backlight_device *bl)
> +{
> +	struct arcxcnn *lp = bl_get_data(bl);
> +	u32 brightness = bl->props.brightness;
> +
> +	if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> +		brightness = 0;
> +
> +	/* set brightness */
> +	if (lp->mode == PWM_BASED)
> +		; /* via pwm */

Replace this with a comment.

> +	else if (lp->mode == REGISTER_BASED)
> +		arcxcnn_set_brightness(lp, brightness);
> +
> +	/* set power-on/off/save modes */
> +	if (bl->props.power == 0)
> +		/* take out of standby */
> +		arcxcnn_update_bit(lp, ARCXCNN_CMD, ARCXCNN_CMD_STDBY, 0);
> +	else
> +		/* 1-3 == power save, 4 = off
> +		 * place in low-power standby mode
> +		 */

This is not a valid multi-line kernel comment.

See: Documentation/CodingStyle

> +		arcxcnn_update_bit(lp, ARCXCNN_CMD,
> +				ARCXCNN_CMD_STDBY, ARCXCNN_CMD_STDBY);
> +	return 0;
> +}
> +
> +static const struct backlight_ops arcxcnn_bl_ops = {
> +	.options = BL_CORE_SUSPENDRESUME,
> +	.update_status = arcxcnn_bl_update_status,
> +};
> +
> +static int arcxcnn_backlight_register(struct arcxcnn *lp)
> +{
> +	struct backlight_device *bl;

This variable seems pretty redundant.

> +	struct backlight_properties props;

Dynamically request memory for this.

> +	struct arcxcnn_platform_data *pdata = lp->pdata;
> +	const char *name = pdata->name ? : DEFAULT_BL_NAME;

Why do you need to set a name different from the device name?

> +	memset(&props, 0, sizeof(props));
> +	props.type = BACKLIGHT_PLATFORM;
> +	props.max_brightness = MAX_BRIGHTNESS;
> +
> +	if (pdata->initial_brightness > props.max_brightness)
> +		pdata->initial_brightness = props.max_brightness;
> +
> +	props.brightness = pdata->initial_brightness;
> +
> +	bl = devm_backlight_device_register(lp->dev, name, lp->dev, lp,
> +				       &arcxcnn_bl_ops, &props);
> +	if (IS_ERR(bl))
> +		return PTR_ERR(bl);
> +
> +	lp->bl = bl;
> +
> +	return 0;
> +}
> +
> +static ssize_t arcxcnn_get_chip_id(struct device *dev,

This should be arcxcnn_chip_id_show()

> +		struct device_attribute *attr, char *buf)
> +{
> +	struct arcxcnn *lp = dev_get_drvdata(dev);
> +
> +	return scnprintf(buf, PAGE_SIZE, "%s\n", lp->chipname);
> +}
> +
> +static ssize_t arcxcnn_get_led_str(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct arcxcnn *lp = dev_get_drvdata(dev);
> +
> +	return scnprintf(buf, PAGE_SIZE, "%02X\n", lp->pdata->led_str);

This is not pdata.

What is led_str anyway?  It looks like an int, not a string.

> +}
> +
> +static ssize_t arcxcnn_set_led_str(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t len)
> +{

arcxcnn_led_str_store()

> +	struct arcxcnn *lp = dev_get_drvdata(dev);
> +	unsigned long ledstr;
> +
> +	if (kstrtoul(buf, 0, &ledstr))
> +		return 0;
> +
> +	if (ledstr != lp->pdata->led_str) {
> +		/* don't allow 0 for ledstr, use power to turn all off */
> +		if (ledstr == 0)
> +			return 0;

Shouldn't you be return -EINVAL if 0 is not valid?

> +		lp->pdata->led_str = ledstr & 0x3F;
> +		arcxcnn_update_bit(lp, ARCXCNN_LEDEN,
> +			ARCXCNN_LEDEN_MASK, lp->pdata->led_str);

What are you lining up against here?

> +	}

'\n'

> +	return len;
> +}
> +
> +static ssize_t arcxcnn_get_bl_ctl_mode(struct device *dev,
> +	     struct device_attribute *attr, char *buf)
> +{

arcxcnn_bl_ctl_mode_show()

> +	struct arcxcnn *lp = dev_get_drvdata(dev);
> +	char *strmode = NULL;

Why are you pre-initialising here?

> +	if (lp->mode == PWM_BASED)
> +		strmode = "pwm based";
> +	else if (lp->mode == REGISTER_BASED)
> +		strmode = "register based";

Remove "based" and place it in scnprintf instead.

> +	return scnprintf(buf, PAGE_SIZE, "%s\n", strmode);
> +}
> +
> +static DEVICE_ATTR(chip_id, 0444, arcxcnn_get_chip_id, NULL);
> +static DEVICE_ATTR(led_str, 0664, arcxcnn_get_led_str, arcxcnn_set_led_str);
> +static DEVICE_ATTR(bl_ctl_mode, 0444, arcxcnn_get_bl_ctl_mode, NULL);
> +
> +static struct attribute *arcxcnn_attributes[] = {
> +	&dev_attr_chip_id.attr,
> +	&dev_attr_led_str.attr,
> +	&dev_attr_bl_ctl_mode.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group arcxcnn_attr_group = {
> +	.attrs = arcxcnn_attributes,
> +};
> +
> +#ifdef CONFIG_OF

No ifdefery please.

Use if (IS_ENABLED(CONFIG_OF)) before calling arcxcnn_parse_dt()
instead.

> +static int arcxcnn_parse_dt(struct arcxcnn *lp)
> +{
> +	struct device *dev = lp->dev;
> +	struct device_node *node = dev->of_node;
> +	u32 prog_val, num_entry, sources[6];
> +	int ret;
> +
> +	if (!node) {
> +		dev_err(dev, "no platform data.\n");

That's not what this means is it?

> +		return -EINVAL;
> +	}

'\n'

> +	lp->pdata->led_config_0_set = false;
> +	lp->pdata->led_config_1_set = false;
> +	lp->pdata->dim_freq_set = false;
> +	lp->pdata->comp_config_set = false;
> +	lp->pdata->filter_config_set = false;
> +	lp->pdata->trim_config_set = false;

These are already 0.

And you need to take these 'out' of pdata.

Create a config struct to place them all in and add it to ddata.

> +	ret = of_property_read_string(node, "label", &lp->pdata->name);
> +	if (ret < 0)
> +		lp->pdata->name = NULL;
> +
> +	ret = of_property_read_u32(node, "default-brightness", &prog_val);
> +	if (ret < 0)
> +		prog_val = s_ibright;
> +	lp->pdata->initial_brightness = prog_val;
> +	if (lp->pdata->initial_brightness > MAX_BRIGHTNESS)
> +		lp->pdata->initial_brightness = MAX_BRIGHTNESS;
> +
> +	ret = of_property_read_u32(node, "arc,led-config-0", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->led_config_0 = (u8)prog_val;
> +		lp->pdata->led_config_0_set = true;
> +	}
> +	ret = of_property_read_u32(node, "arc,led-config-1", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->led_config_1 = (u8)prog_val;
> +		lp->pdata->led_config_1_set = true;
> +	}
> +	ret = of_property_read_u32(node, "arc,dim-freq", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->dim_freq = (u8)prog_val;
> +		lp->pdata->dim_freq_set = true;
> +	}
> +	ret = of_property_read_u32(node, "arc,comp-config", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->comp_config = (u8)prog_val;
> +		lp->pdata->comp_config_set = true;
> +	}
> +	ret = of_property_read_u32(node, "arc,filter-config", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->filter_config = (u8)prog_val;
> +		lp->pdata->filter_config_set = true;
> +	}
> +	ret = of_property_read_u32(node, "arc,trim-config", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->trim_config = (u8)prog_val;
> +		lp->pdata->trim_config_set = true;
> +	}

You need to get these signed off by the DT folk.

> +	ret = of_property_count_u32_elems(node, "led-sources");
> +	if (ret < 0)
> +		lp->pdata->led_str = 0x3F;

What is 3F?  Please define it.

> +	else {
> +		num_entry = ret;
> +		if (num_entry > 6)
> +			num_entry = 6;

What is 6?  No magic numbers.

> +		ret = of_property_read_u32_array(node, "led-sources", sources,
> +					num_entry);
> +		if (ret < 0) {
> +			dev_err(dev, "led-sources node is invalid.\n");
> +			return -EINVAL;
> +		}
> +
> +		lp->pdata->led_str = 0;
> +		while (num_entry > 0)
> +			lp->pdata->led_str |= (1 << sources[--num_entry]);

This is not good code.  Please make it easier to read.

> +	}
> +	return 0;
> +}
> +#else
> +static int arcxcnn_parse_dt(struct arcxcnn *lp)
> +{
> +	return -EINVAL;
> +}
> +#endif
> +
> +static int arcxcnn_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> +{
> +	struct arcxcnn *lp;
> +	int ret;
> +	u8 regval;
> +	u16 chipid;
> +
> +	if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> +		return -EIO;
> +
> +	lp = devm_kzalloc(&cl->dev, sizeof(*lp), GFP_KERNEL);
> +	if (!lp)
> +		return -ENOMEM;
> +
> +	lp->client = cl;
> +	lp->dev = &cl->dev;

You don't need both of these.

> +	lp->chip_id = id->driver_data;

What are you going to use this for?

> +	lp->pdata = dev_get_platdata(&cl->dev);
> +
> +	if (!lp->pdata) {
> +		lp->pdata = devm_kzalloc(lp->dev,
> +				sizeof(*lp->pdata), GFP_KERNEL);
> +		if (!lp->pdata)
> +			return -ENOMEM;
> +
> +		/* no platform data, parse the device-tree for info.  if there
> +		 * is no device tree entry, we are being told we exist because
> +		 * user-land said so, so make up the info we need
> +		 */

Not a valid multi-line comment.

> +		ret = arcxcnn_parse_dt(lp);
> +		if (ret < 0) {
> +			/* no device tree, use defaults based on module params
> +			 */

Not a valid single-line comment.

> +			lp->pdata->led_config_0_set = false;
> +			lp->pdata->led_config_1_set = false;
> +			lp->pdata->dim_freq_set = false;
> +			lp->pdata->comp_config_set = false;
> +			lp->pdata->filter_config_set = false;
> +			lp->pdata->trim_config_set = false;
> +			lp->pdata->name = NULL;

These are already 0.

> +			lp->pdata->initial_brightness = s_ibright;
> +			lp->pdata->led_str = s_iledstr;
> +		}
> +	}
> +
> +	if (lp->pdata->dim_freq_set)
> +		lp->mode = PWM_BASED;
> +	else
> +		lp->mode = REGISTER_BASED;
> +
> +	i2c_set_clientdata(cl, lp);
> +
> +	/* read device ID */
> +	regval = arcxcnn_read_byte(lp, 0x1E);
> +	chipid = regval;
> +	chipid <<= 8;
> +	regval = arcxcnn_read_byte(lp, 0x1F);
> +	chipid |= regval;

Define all magic numbers.

> +	/* make sure it belongs to this driver
> +	 * TODO - handle specific ids
> +	 */
> +	if (chipid != 0x02A5) {
> +		#if 1

What on earth is this?

> +		dev_info(&cl->dev, "Chip Id is %04X\n", chipid);
> +		#else
> +		dev_err(&cl->dev, "%04X is not ARC2C\n", chipid);
> +		return -ENODEV;
> +		#endif
> +	}
> +	/* reset the device */
> +	arcxcnn_write_byte(lp, ARCXCNN_CMD, ARCXCNN_CMD_RESET);
> +
> +	/* set initial brightness */
> +	arcxcnn_set_brightness(lp, lp->pdata->initial_brightness);
> +
> +	/* if fadectrl set in DT, set the value directly, else leave default */
> +	if (lp->pdata->led_config_0_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_FADECTRL,
> +			lp->pdata->led_config_0);
> +
> +	/* if iled config set in DT, set the value, else internal mode */

Spelling.

> +	if (lp->pdata->led_config_1_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_ILED_CONFIG,
> +			lp->pdata->led_config_1);
> +	else
> +		arcxcnn_write_byte(lp, ARCXCNN_ILED_CONFIG, 0x57);

More magic numbers.

> +	/* other misc DT settings */
> +	if (lp->pdata->dim_freq_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_FADECTRL, lp->pdata->dim_freq);
> +	if (lp->pdata->comp_config_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_COMP_CONFIG,
> +			lp->pdata->comp_config);
> +	if (lp->pdata->filter_config_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_FILT_CONFIG,
> +			lp->pdata->filter_config);
> +	if (lp->pdata->trim_config_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_IMAXTUNE,
> +			lp->pdata->trim_config);

What are the default values of these registers?

If 0, then just write them without the checks.

> +	/* set initial LED Strings */

What is LED strings?

> +	arcxcnn_update_bit(lp, ARCXCNN_LEDEN,
> +		ARCXCNN_LEDEN_MASK, lp->pdata->led_str);
> +
> +	snprintf(lp->chipname, sizeof(lp->chipname),
> +		"%s-%04X", id->name, chipid);
> +
> +	ret = arcxcnn_backlight_register(lp);
> +	if (ret) {
> +		dev_err(lp->dev,
> +			"failed to register backlight. err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = sysfs_create_group(&lp->dev->kobj, &arcxcnn_attr_group);
> +	if (ret) {
> +		dev_err(lp->dev, "failed to register sysfs. err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	backlight_update_status(lp->bl);

'\n'

> +	return 0;
> +}
> +
> +static int arcxcnn_remove(struct i2c_client *cl)
> +{
> +	struct arcxcnn *lp = i2c_get_clientdata(cl);
> +
> +	if (!s_no_reset_on_remove) {
> +		/* disable all strings */
> +		arcxcnn_write_byte(lp, ARCXCNN_LEDEN, 0x00);
> +		/* reset the device */
> +		arcxcnn_write_byte(lp, ARCXCNN_CMD, ARCXCNN_CMD_RESET);
> +	}

'\n'

> +	lp->bl->props.brightness = 0;

'\n'

> +	backlight_update_status(lp->bl);

'\n'

> +	if (lp->supply)
> +		regulator_disable(lp->supply);

'\n'

> +	sysfs_remove_group(&lp->dev->kobj, &arcxcnn_attr_group);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id arcxcnn_dt_ids[] = {
> +	{ .compatible = "arc,arc2c0608" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, arcxcnn_dt_ids);
> +
> +/* Note that the device/chip ID is not fixed in silicon so
> + * auto-probing of these devices on the bus is most likely
> + * not possible, use device tree to set i2c bus address

Then why supply it here at all?

> + */
> +static const struct i2c_device_id arcxcnn_ids[] = {
> +	{"arc2c0608", ARC2C0608},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, arcxcnn_ids);
> +
> +static struct i2c_driver arcxcnn_driver = {
> +	.driver = {
> +		   .name = "arcxcnn_bl",
> +		   .of_match_table = of_match_ptr(arcxcnn_dt_ids),
> +		   },

Tabbing.

> +	.probe = arcxcnn_probe,
> +	.remove = arcxcnn_remove,
> +	.id_table = arcxcnn_ids,
> +};
> +

Superfluous '\n'

> +module_i2c_driver(arcxcnn_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Brian Dodge <bdodge09-1ViLX0X+lBJBDgjK7y7TUQ@public.gmane.org>");

Who?

> +MODULE_DESCRIPTION("ARCXCNN Backlight driver");
> diff --git a/include/linux/i2c/arcxcnn.h b/include/linux/i2c/arcxcnn.h
> new file mode 100644
> index 0000000..1c681dd
> --- /dev/null
> +++ b/include/linux/i2c/arcxcnn.h
> @@ -0,0 +1,67 @@
> +/*
> + * Backlight driver for ArcticSand ARC2C0608 Backlight Devices
> + *
> + * Copyright 2016 ArcticSand, Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#ifndef _ARCXCNN_H
> +#define _ARCXCNN_H
> +
> +enum arcxcnn_chip_id {
> +	ARC2C0608
> +};
> +
> +enum arcxcnn_brightness_source {
> +	ARCXCNN_PWM_ONLY,
> +	ARCXCNN_I2C_ONLY = 2,
> +};
> +
> +#define ARCXCNN_MAX_PROGENTRIES	48	/* max a/v pairs for custom */
> +
> +/**
> + * struct arcxcnn_platform_data
> + * @name : Backlight driver name. If it is not defined, default name is set.
> + * @initial_brightness : initial value of backlight brightness
> + * @led_str	 : initial LED string enables, upper bit is global on/off
> + * @led_config_0 : fading speed (period between intensity steps)
> + * @led_config_1 : misc settings, see datasheet
> + * @dim_freq	 : pwm dimming frequency if in pwm mode
> + * @comp_config	 : misc config, see datasheet
> + * @filter_config: RC/PWM filter config, see datasheet
> + * @trim_config	 : full scale current trim, see datasheet
> + * @led_config_0_set	: the value in led_config_0 is valid
> + * @led_config_1_set	: the value in led_config_1 is valid
> + * @dim_freq_set	: the value in dim_freq is valid
> + * @comp_config_set	: the value in comp_config is valid
> + * @filter_config_set	: the value in filter_config is valid
> + * @trim_config_set	: the value in trim_config is valid

Tab out the ':'s.

This is only platform data if it's set by the platform.

Otherwise it's just a config or bitfields.

> + * the _set flags are used to indicate that the value was explicitly set
> + * in the device tree or platform data. settings not set are left as default
> + * power-on default values of the chip except for led_str and led_config_1
> + * which are set by the driver (led_str is specified indirectly in the
> + * device tree via "led-sources")
> + */
> +struct arcxcnn_platform_data {
> +	const char *name;
> +	u16 initial_brightness;
> +	u8	led_str;
> +
> +	u8	led_config_0;
> +	u8	led_config_1;
> +	u8	dim_freq;
> +	u8	comp_config;
> +	u8	filter_config;
> +	u8	trim_config;
> +
> +	bool	led_config_0_set;
> +	bool	led_config_1_set;
> +	bool	dim_freq_set;
> +	bool	comp_config_set;
> +	bool	filter_config_set;
> +	bool	trim_config_set;
> +};
> +
> +#endif

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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 1/3] remoteproc: qcom: Encapsulate pvt data structure for q6v56 hexagon.
From: Bjorn Andersson @ 2016-11-18 18:40 UTC (permalink / raw)
  To: Avaneesh Kumar Dwivedi
  Cc: Rob Herring, Ohad Ben-Cohen, Mark Rutland,
	open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <a29bce91-1300-0728-51dd-7d009d9e056b-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

On Wed 16 Nov 06:02 PST 2016, Avaneesh Kumar Dwivedi wrote:

> 
> 
> On 11/11/2016 2:00 AM, Rob Herring wrote:
> >On Fri, Nov 04, 2016 at 07:30:54PM +0530, Avaneesh Kumar Dwivedi wrote:
> >>Encapsulate resources specific to each version of hexagon chip to
> >>device node to avoid conditional check for manipulation of those
> >>resources in driver code.
> >>
> >>Signed-off-by: Avaneesh Kumar Dwivedi <akdwived-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> >>---
> >>  .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |   1 +
> >>  drivers/remoteproc/qcom_q6v5_pil.c                 | 137 ++++++++++++++++++---
> >>  2 files changed, 120 insertions(+), 18 deletions(-)
> >>
> >>diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> >>index 57cb49e..cbc165c 100644
> >>--- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> >>+++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> >>@@ -8,6 +8,7 @@ on the Qualcomm Hexagon core.
> >>  	Value type: <string>
> >>  	Definition: must be one of:
> >>  		    "qcom,q6v5-pil"
> >>+		"qcom,q6v56-pil"
> >Perhaps some explanation in the commit message about what these magic
> >numbers mean?
> 
>     "v56" represent class of hexagon chip, which again is differentiated
> based on version number. Two
>     different MSM SOC may use same class of hexagon chip. example is as
> below.
> 
>     msm8974  q6v5 version  5.0.0
>     msm8916  q6v5 version  5.1.1

But looking at the Qualcomm tree I think I got 8916 wrong, it seems that
it should be "q6v56" and your patches indicates that the 8996 has a
q6v55 - which doesn't make sense to me and in some places there's
comments indicating it's version 6.

I asked you about this but I can't find an answer in any of your
replies.

Regards,
Bjorn
--
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 1/3] dt-bindings: Add Macnica Americas vendor prefix
From: Rob Herring @ 2016-11-18 18:03 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: Dinh Nguyen, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Dinh Nguyen, Mark Rutland,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CADhT+wcesJRSf0q+_BDEmtsMTAO2LcAPkqjsRW2mNc1znMp4qg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Nov 18, 2016 at 10:25 AM, Dinh Nguyen <dinh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Rob,
>
> On Tue, Nov 1, 2016 at 10:36 AM, Dinh Nguyen <dinguyen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> Add a vendor prefix for the Macnica company.
>> http://http://www.macnica.com
>>
>> Signed-off-by: Dinh Nguyen <dinguyen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> index f0a48ea..81674f2 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> @@ -158,6 +158,7 @@ lg  LG Corporation
>>  linux  Linux-specific binding
>>  lltc   Linear Technology Corporation
>>  lsi    LSI Corp. (LSI Logic)
>> +macnica        Macnica Americas
>>  marvell        Marvell Technology Group Ltd.
>>  maxim  Maxim Integrated Products
>>  meas   Measurement Specialties
>> --
>> 2.8.3
>
> Just a gentle ping.

Sorry, didn't send anything out, but the series is already applied if
you look at -next or PW.

Rob
--
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 1/2] DW DMAC: enable memory-to-memory transfers support
From: Andy Shevchenko @ 2016-11-18 17:46 UTC (permalink / raw)
  To: Eugeniy Paltsev, devicetree
  Cc: mark.rutland, vinod.koul, linux-kernel, robh+dt, dmaengine,
	linux-snps-arc
In-Reply-To: <1479487989-24543-2-git-send-email-Eugeniy.Paltsev@synopsys.com>

On Fri, 2016-11-18 at 19:53 +0300, Eugeniy Paltsev wrote:
> All known devices, which use DT for configuration, support
> memory-to-memory transfers. So enable it by default, if we read
> configuration from DT.
> 

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  drivers/dma/dw/platform.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
> index 5bda0eb..aa7a5c1 100644
> --- a/drivers/dma/dw/platform.c
> +++ b/drivers/dma/dw/platform.c
> @@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev)
>  	if (of_property_read_bool(np, "is_private"))
>  		pdata->is_private = true;
>  
> +	/*
> +	 * All known devices, which use DT for configuration, support
> +	 * memory-to-memory transfers. So enable it by default.
> +	 */
> +	pdata->is_memcpy = true;
> +
>  	if (!of_property_read_u32(np, "chan_allocation_order", &tmp))
>  		pdata->chan_allocation_order = (unsigned char)tmp;
>  

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

^ permalink raw reply

* Re: [PATCH v2 2/2] DW DMAC: add multi-block property to device tree
From: Andy Shevchenko @ 2016-11-18 17:43 UTC (permalink / raw)
  To: Eugeniy Paltsev, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	vinod.koul-ral2JQCrhuEAvxtiuMwx3w,
	dmaengine-u79uwXL29TY76Z2rM5mHXA,
	linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1479487989-24543-3-git-send-email-Eugeniy.Paltsev-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

On Fri, 2016-11-18 at 19:53 +0300, Eugeniy Paltsev wrote:
> Several versions of DW DMAC have multi block transfers hardware
> support. Hardware support of multi block transfers is disabled
> by default if we use DT to configure DMAC and software emulation
> of multi block transfers used instead.
> Add multi-block property, so it is possible to enable hardware
> multi block transfers (if present) via DT.
> 
> Switch from per device is_nollp variable to multi_block array
> to be able enable/disable multi block transfers separately per
> channel.
> 
> Update DT documentation.
> 
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/dma/snps-dma.txt | 2 ++
>  drivers/dma/dw/core.c                              | 2 +-
>  drivers/dma/dw/platform.c                          | 5 +++++
>  include/linux/platform_data/dma-dw.h               | 4 ++--
>  4 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt
> b/Documentation/devicetree/bindings/dma/snps-dma.txt
> index 0f55832..03d6d6d 100644
> --- a/Documentation/devicetree/bindings/dma/snps-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
> @@ -27,6 +27,8 @@ Optional properties:
>    that services interrupts for this device
>  - is_private: The device channels should be marked as private and not
> for by the
>    general purpose DMA channel allocator. False if not passed.
> +- multi-block: Multi block transfers supported by hardware per AHB
> master.
> +  0 (default): not supported, 1: supported.

Since default is "not supported" you have to update users accordingly.
(Check platform data and existing DTS).
 
>  Example:
>  
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index c2c0a61..f2a3d06 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -1569,7 +1569,7 @@ int dw_dma_probe(struct dw_dma_chip *chip)
>  				(dwc_params >> DWC_PARAMS_MBLK_EN &
> 0x1) == 0;
>  		} else {
>  			dwc->block_size = pdata->block_size;

> -			dwc->nollp = pdata->is_nollp;
> +			dwc->nollp = pdata->multi_block[i];

This inverts the default logic.

>  		}
>  	}
>  
> diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
> index aa7a5c1..b262fd3 100644
> --- a/drivers/dma/dw/platform.c
> +++ b/drivers/dma/dw/platform.c
> @@ -152,6 +152,11 @@ dw_dma_parse_dt(struct platform_device *pdev)
>  			pdata->data_width[tmp] = BIT(arr[tmp] &
> 0x07);
>  	}
>  
> +	if (!of_property_read_u32_array(np, "multi-block", arr,
> nr_masters)) {
> +		for (tmp = 0; tmp < nr_masters; tmp++)
> +			pdata->multi_block[tmp] = arr[tmp];
> +	}
> +
>  	return pdata;
>  }
>  #else
> diff --git a/include/linux/platform_data/dma-dw.h
> b/include/linux/platform_data/dma-dw.h
> index 5f0e11e..0773bb4 100644
> --- a/include/linux/platform_data/dma-dw.h
> +++ b/include/linux/platform_data/dma-dw.h
> @@ -40,19 +40,18 @@ struct dw_dma_slave {
>   * @is_private: The device channels should be marked as private and
> not for
>   *	by the general purpose DMA channel allocator.
>   * @is_memcpy: The device channels do support memory-to-memory
> transfers.
> - * @is_nollp: The device channels does not support multi block
> transfers.
>   * @chan_allocation_order: Allocate channels starting from 0 or 7
>   * @chan_priority: Set channel priority increasing from 0 to 7 or 7
> to 0.
>   * @block_size: Maximum block size supported by the controller
>   * @nr_masters: Number of AHB masters supported by the controller
>   * @data_width: Maximum data width supported by hardware per AHB
> master
>   *		(in bytes, power of 2)
> + * @multi_block: Multi block transfers supported by hardware per AHB
> master.
>   */
>  struct dw_dma_platform_data {
>  	unsigned int	nr_channels;
>  	bool		is_private;
>  	bool		is_memcpy;
> -	bool		is_nollp;
>  #define CHAN_ALLOCATION_ASCENDING	0	/* zero to seven */
>  #define CHAN_ALLOCATION_DESCENDING	1	/* seven to zero
> */
>  	unsigned char	chan_allocation_order;
> @@ -62,6 +61,7 @@ struct dw_dma_platform_data {
>  	unsigned int	block_size;
>  	unsigned char	nr_masters;
>  	unsigned char	data_width[DW_DMA_MAX_NR_MASTERS];
> +	unsigned char	multi_block[DW_DMA_MAX_NR_MASTERS];
>  };
>  
>  #endif /* _PLATFORM_DATA_DMA_DW_H */

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 0/3] driver: Add DT support for DA8xx
From: Bin Liu @ 2016-11-18 17:37 UTC (permalink / raw)
  To: Alexandre Bailon
  Cc: khilman-rdvid1DuHRBWk0Htik3J/w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	nsekhar-l0cyMroinI0
In-Reply-To: <1479293545-7516-1-git-send-email-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

On Wed, Nov 16, 2016 at 11:52:22AM +0100, Alexandre Bailon wrote:
> Changes in v2:
> * Remove unrelated changes in patch 3
> * Rename the device node in patch 4
> 
> Changes in v3:
> * Fix few mistakes in DT binding sample
> * Only build the device table if DT is enabled
> 
> Change in v4:
> * Fix a nit
> 
> Changes in v5:
> * Nothing. Resent the v4 in two seppaated series: one for platform and one for
>   driver.
> 
> Petr Kulhavy (3):
>   dt/bindings: Add binding for the DA8xx MUSB driver
>   usb: musb: core: added helper function for parsing DT
>   usb: musb: da8xx: Add DT support for the DA8xx driver

Applied. Thanks.
-Bin.

> 
>  .../devicetree/bindings/usb/da8xx-usb.txt          | 43 ++++++++++++++++++++
>  drivers/usb/musb/da8xx.c                           | 46 ++++++++++++++++++++++
>  drivers/usb/musb/musb_core.c                       | 19 +++++++++
>  drivers/usb/musb/musb_core.h                       |  6 +++
>  4 files changed, 114 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt
> 
> -- 
> 2.7.3
> 
--
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/3] remoteproc: qcom: add Venus video core firmware loader driver
From: Bjorn Andersson @ 2016-11-18 17:23 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Stanimir Varbanov, Ohad Ben-Cohen, Andy Gross, Rob Herring,
	Mark Rutland, Srinivas Kandagatla, linux-remoteproc, linux-kernel,
	linux-arm-msm, linux-soc, devicetree
In-Reply-To: <20161114191641.GH5177@codeaurora.org>

On Mon 14 Nov 11:16 PST 2016, Stephen Boyd wrote:

> On 11/07, Stanimir Varbanov wrote:
> > +#include <linux/module.h>
> > +#include <linux/of_reserved_mem.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/qcom_scm.h>
> > +#include <linux/remoteproc.h>
> > +
> > +#include "qcom_mdt_loader.h"
> > +#include "remoteproc_internal.h"
> > +
> > +#define VENUS_CRASH_REASON_SMEM		425
> 
> This is unused. Is there going to be some common smem API to get
> the crash reason?

There are a couple of these operations that are good candidates to be
shared between all Qualcomm remoteproc drivers. I just haven't spent the
time to figure out how to properly splice it.

For the crash reason specifically, there are three operations done in
the other drivers; read-and-null-terminate, then this is outputed to the
log, also indicating what type of event we had. And finally the
remoteproc core is informed about the crash, with a event-type passed
and we get another line in the log based on that, i.e. there's room for
a few improvements in this area.

Regards,
Bjorn

^ 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