* [PATCH 1/1] spi: imx: support to set watermark level via DTS
@ 2017-01-05 6:10 Jiada Wang
2017-01-05 12:48 ` Vladimir Zapolskiy
2017-01-05 17:58 ` Mark Brown
0 siblings, 2 replies; 4+ messages in thread
From: Jiada Wang @ 2017-01-05 6:10 UTC (permalink / raw)
To: broonie, robh+dt, mark.rutland
Cc: linux-spi, devicetree, linux-kernel, jiada_wang
Previously watermark level is configured to fifosize/2,
DMA mode can be used only when transfer length can be divided
by 'watermark level * bpw', which makes DMA mode not practical.
This patch adds new DTS property 'dma-wml', user can configure
DMA watermark level, by specify 'dma-wml' in corresponding ecspi
node.
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt | 2 ++
drivers/spi/spi-imx.c | 12 ++++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
index 8bc95e2..1e9345f 100644
--- a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
+++ b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
@@ -19,6 +19,7 @@ See the clock consumer binding,
- dmas: DMA specifiers for tx and rx dma. See the DMA client binding,
Documentation/devicetree/bindings/dma/dma.txt
- dma-names: DMA request names should include "tx" and "rx" if present.
+- dma-wml: Specifies DMA watermark level
Obsolete properties:
- fsl,spi-num-chipselects : Contains the number of the chipselect
@@ -35,4 +36,5 @@ ecspi@70010000 {
<&gpio3 25 0>; /* GPIO3_25 */
dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
dma-names = "rx", "tx";
+ dma-wml = <16>;
};
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index deb782f..5c0ce19 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -929,8 +929,6 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx,
if (of_machine_is_compatible("fsl,imx6dl"))
return 0;
- spi_imx->wml = spi_imx_get_fifosize(spi_imx) / 2;
-
/* Prepare for TX DMA: */
master->dma_tx = dma_request_slave_channel_reason(dev, "tx");
if (IS_ERR(master->dma_tx)) {
@@ -1155,6 +1153,7 @@ static int spi_imx_probe(struct platform_device *pdev)
struct spi_imx_data *spi_imx;
struct resource *res;
int i, ret, irq;
+ u32 wml;
if (!np && !mxc_platform_info) {
dev_err(&pdev->dev, "can't get the platform data\n");
@@ -1177,6 +1176,15 @@ static int spi_imx_probe(struct platform_device *pdev)
spi_imx->devtype_data = of_id ? of_id->data :
(struct spi_imx_devtype_data *)pdev->id_entry->driver_data;
+ if (of_property_read_u32(np, "dma-wml", &wml) == 0) {
+ if (wml > spi_imx_get_fifosize(spi_imx) || wml == 0) {
+ dev_warn(&pdev->dev, "mis-configured dma-wml\n");
+ spi_imx->wml = spi_imx_get_fifosize(spi_imx) / 2;
+ } else
+ spi_imx->wml = wml;
+ } else
+ spi_imx->wml = spi_imx_get_fifosize(spi_imx) / 2;
+
if (mxc_platform_info) {
master->num_chipselect = mxc_platform_info->num_chipselect;
master->cs_gpios = devm_kzalloc(&master->dev,
--
2.9.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] spi: imx: support to set watermark level via DTS
2017-01-05 6:10 [PATCH 1/1] spi: imx: support to set watermark level via DTS Jiada Wang
@ 2017-01-05 12:48 ` Vladimir Zapolskiy
2017-01-05 17:58 ` Mark Brown
1 sibling, 0 replies; 4+ messages in thread
From: Vladimir Zapolskiy @ 2017-01-05 12:48 UTC (permalink / raw)
To: Jiada Wang, broonie, robh+dt, mark.rutland
Cc: linux-spi, devicetree, linux-kernel
Hi Jiada,
On 01/05/2017 08:10 AM, Jiada Wang wrote:
> Previously watermark level is configured to fifosize/2,
> DMA mode can be used only when transfer length can be divided
> by 'watermark level * bpw', which makes DMA mode not practical.
>
> This patch adds new DTS property 'dma-wml', user can configure
> DMA watermark level, by specify 'dma-wml' in corresponding ecspi
> node.
>
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> ---
> Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt | 2 ++
> drivers/spi/spi-imx.c | 12 ++++++++++--
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
> index 8bc95e2..1e9345f 100644
> --- a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
> +++ b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
> @@ -19,6 +19,7 @@ See the clock consumer binding,
> - dmas: DMA specifiers for tx and rx dma. See the DMA client binding,
> Documentation/devicetree/bindings/dma/dma.txt
> - dma-names: DMA request names should include "tx" and "rx" if present.
> +- dma-wml: Specifies DMA watermark level
It is supposed to be an optional property, please add a new section.
I'm not sure about the property naming, the property may be considered
as a generic one and then it should not have 'fsl' prefix like yours one,
but then its description should be moved to bindings/spi/spi-bus.txt.
On the other hand for a kind of generic property it might be desirable
to have two properties, DMA RX and DMA TX watermark (threshold) levels.
And the property may be defined as a combined DMA RX / DMA TX watermark
specific to the Freescale ECSPI controller (however I would propose to
separate them, since the hardware allows to do it), but then please add
'fsl' prefix.
>
> Obsolete properties:
> - fsl,spi-num-chipselects : Contains the number of the chipselect
> @@ -35,4 +36,5 @@ ecspi@70010000 {
> <&gpio3 25 0>; /* GPIO3_25 */
> dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
> dma-names = "rx", "tx";
> + dma-wml = <16>;
Please use tab symbols for indentation.
> };
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index deb782f..5c0ce19 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -929,8 +929,6 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx,
> if (of_machine_is_compatible("fsl,imx6dl"))
> return 0;
>
> - spi_imx->wml = spi_imx_get_fifosize(spi_imx) / 2;
> -
> /* Prepare for TX DMA: */
> master->dma_tx = dma_request_slave_channel_reason(dev, "tx");
> if (IS_ERR(master->dma_tx)) {
> @@ -1155,6 +1153,7 @@ static int spi_imx_probe(struct platform_device *pdev)
> struct spi_imx_data *spi_imx;
> struct resource *res;
> int i, ret, irq;
> + u32 wml;
>
> if (!np && !mxc_platform_info) {
> dev_err(&pdev->dev, "can't get the platform data\n");
> @@ -1177,6 +1176,15 @@ static int spi_imx_probe(struct platform_device *pdev)
> spi_imx->devtype_data = of_id ? of_id->data :
> (struct spi_imx_devtype_data *)pdev->id_entry->driver_data;
>
> + if (of_property_read_u32(np, "dma-wml", &wml) == 0) {
> + if (wml > spi_imx_get_fifosize(spi_imx) || wml == 0) {
> + dev_warn(&pdev->dev, "mis-configured dma-wml\n");
> + spi_imx->wml = spi_imx_get_fifosize(spi_imx) / 2;
> + } else
> + spi_imx->wml = wml;
checkpatch should complain regarding missing bracers on the "else" branch.
> + } else
> + spi_imx->wml = spi_imx_get_fifosize(spi_imx) / 2;
> +
Same is here.
Since the setting depends on the availability of DMA feature, it would be
better to read the property out from spi_imx_sdma_init():
spi_imx->wml = spi_imx_get_fifosize(spi_imx) / 2;
if (!of_property_read_u32(np, "dma-wml", &wml)) {
if (!wml || wml > spi_imx_get_fifosize(spi_imx)) /* power of 2 also? */
dev_warn(&pdev->dev, "mis-configured dma-wml\n");
else
spi_imx->wml = wml;
}
> if (mxc_platform_info) {
> master->num_chipselect = mxc_platform_info->num_chipselect;
> master->cs_gpios = devm_kzalloc(&master->dev,
>
For v2 please add to cc list:
* Sascha Hauer <s.hauer@pengutronix.de>
* Shawn Guo <shawn.guo@linaro.org>
* Alexander Shiyan <shc_work@mail.ru>
--
With best wishes,
Vladimir
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] spi: imx: support to set watermark level via DTS
2017-01-05 6:10 [PATCH 1/1] spi: imx: support to set watermark level via DTS Jiada Wang
2017-01-05 12:48 ` Vladimir Zapolskiy
@ 2017-01-05 17:58 ` Mark Brown
2017-01-06 9:00 ` Jiada Wang
1 sibling, 1 reply; 4+ messages in thread
From: Mark Brown @ 2017-01-05 17:58 UTC (permalink / raw)
To: Jiada Wang; +Cc: robh+dt, mark.rutland, linux-spi, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 557 bytes --]
On Thu, Jan 05, 2017 at 03:10:15PM +0900, Jiada Wang wrote:
> Previously watermark level is configured to fifosize/2,
> DMA mode can be used only when transfer length can be divided
> by 'watermark level * bpw', which makes DMA mode not practical.
> This patch adds new DTS property 'dma-wml', user can configure
> DMA watermark level, by specify 'dma-wml' in corresponding ecspi
> node.
Doesn't this just move the problem around a bit - can we not have the
driver figure out a more sensible watermark for each transfer rather
than fixing one in the DT?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] spi: imx: support to set watermark level via DTS
2017-01-05 17:58 ` Mark Brown
@ 2017-01-06 9:00 ` Jiada Wang
0 siblings, 0 replies; 4+ messages in thread
From: Jiada Wang @ 2017-01-06 9:00 UTC (permalink / raw)
To: Mark Brown vladimir_zapolskiy@mentor.com
Cc: robh+dt, mark.rutland, linux-spi, devicetree, linux-kernel
Hello Mark and Vladimir
On 01/05/2017 09:58 AM, Mark Brown wrote:
> On Thu, Jan 05, 2017 at 03:10:15PM +0900, Jiada Wang wrote:
>
>> Previously watermark level is configured to fifosize/2,
>> DMA mode can be used only when transfer length can be divided
>> by 'watermark level * bpw', which makes DMA mode not practical.
>> This patch adds new DTS property 'dma-wml', user can configure
>> DMA watermark level, by specify 'dma-wml' in corresponding ecspi
>> node.
> Doesn't this just move the problem around a bit - can we not have the
> driver figure out a more sensible watermark for each transfer rather
> than fixing one in the DT?
I would like to submit another version of patch to adjust watermark level
based on transfer length. to use largest number (< fifosize / 2 ) which
can divide
(transfer_length / bytes_per_work).
Thanks,
Jiada
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-06 9:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-05 6:10 [PATCH 1/1] spi: imx: support to set watermark level via DTS Jiada Wang
2017-01-05 12:48 ` Vladimir Zapolskiy
2017-01-05 17:58 ` Mark Brown
2017-01-06 9:00 ` Jiada Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).