[PATCH v6 1/6] hwrng: remove msm hw_random driver

[Vinod Koul]

This driver is for a psedo-rng so should not be added in hwrng.
Remove it so that it's replacement can be added.

Signed-off-by: Vinod Koul 
---
 drivers/char/hw_random/Kconfig   |  13 ---
 drivers/char/hw_random/Makefile  |   1 -
 drivers/char/hw_random/msm-rng.c | 183 ---
 3 files changed, 197 deletions(-)
 delete mode 100644 drivers/char/hw_random/msm-rng.c

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index c34b257d852d..dac895dc01b9 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -307,19 +307,6 @@ config HW_RANDOM_HISI
 
  If unsure, say Y.
 
-config HW_RANDOM_MSM
-   tristate "Qualcomm SoCs Random Number Generator support"
-   depends on HW_RANDOM && ARCH_QCOM
-   default HW_RANDOM
-   ---help---
- This driver provides kernel-side support for the Random Number
- Generator hardware found on Qualcomm SoCs.
-
- To compile this driver as a module, choose M here. the
- module will be called msm-rng.
-
- If unsure, say Y.
-
 config HW_RANDOM_ST
tristate "ST Microelectronics HW Random Number Generator support"
depends on HW_RANDOM && ARCH_STI
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 533e913c93d1..e35ec3ce3a20 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -29,7 +29,6 @@ obj-$(CONFIG_HW_RANDOM_POWERNV) += powernv-rng.o
 obj-$(CONFIG_HW_RANDOM_HISI)   += hisi-rng.o
 obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o
 obj-$(CONFIG_HW_RANDOM_IPROC_RNG200) += iproc-rng200.o
-obj-$(CONFIG_HW_RANDOM_MSM) += msm-rng.o
 obj-$(CONFIG_HW_RANDOM_ST) += st-rng.o
 obj-$(CONFIG_HW_RANDOM_XGENE) += xgene-rng.o
 obj-$(CONFIG_HW_RANDOM_STM32) += stm32-rng.o
diff --git a/drivers/char/hw_random/msm-rng.c b/drivers/char/hw_random/msm-rng.c
deleted file mode 100644
index 841fee845ec9..
--- a/drivers/char/hw_random/msm-rng.c
+++ /dev/null
@@ -1,183 +0,0 @@
-/*
- * Copyright (c) 2011-2013, 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 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-/* Device specific register offsets */
-#define PRNG_DATA_OUT  0x
-#define PRNG_STATUS0x0004
-#define PRNG_LFSR_CFG  0x0100
-#define PRNG_CONFIG0x0104
-
-/* Device specific register masks and config values */
-#define PRNG_LFSR_CFG_MASK 0x
-#define PRNG_LFSR_CFG_CLOCKS   0x
-#define PRNG_CONFIG_HW_ENABLE  BIT(1)
-#define PRNG_STATUS_DATA_AVAIL BIT(0)
-
-#define MAX_HW_FIFO_DEPTH  16
-#define MAX_HW_FIFO_SIZE   (MAX_HW_FIFO_DEPTH * 4)
-#define WORD_SZ4
-
-struct msm_rng {
-   void __iomem *base;
-   struct clk *clk;
-   struct hwrng hwrng;
-};
-
-#define to_msm_rng(p)  container_of(p, struct msm_rng, hwrng)
-
-static int msm_rng_enable(struct hwrng *hwrng, int enable)
-{
-   struct msm_rng *rng = to_msm_rng(hwrng);
-   u32 val;
-   int ret;
-
-   ret = clk_prepare_enable(rng->clk);
-   if (ret)
-   return ret;
-
-   if (enable) {
-   /* Enable PRNG only if it is not already enabled */
-   val = readl_relaxed(rng->base + PRNG_CONFIG);
-   if (val & PRNG_CONFIG_HW_ENABLE)
-   goto already_enabled;
-
-   val = readl_relaxed(rng->base + PRNG_LFSR_CFG);
-   val &= ~PRNG_LFSR_CFG_MASK;
-   val |= PRNG_LFSR_CFG_CLOCKS;
-   writel(val, rng->base + PRNG_LFSR_CFG);
-
-   val = readl_relaxed(rng->base + PRNG_CONFIG);
-   val |= PRNG_CONFIG_HW_ENABLE;
-   writel(val, rng->base + PRNG_CONFIG);
-   } else {
-   val = readl_relaxed(rng->base + PRNG_CONFIG);
-   val &= ~PRNG_CONFIG_HW_ENABLE;
-   writel(val, rng->base + PRNG_CONFIG);
-   }
-
-already_enabled:
-   clk_disable_unprepare(rng->clk);
-   return 0;
-}
-
-static int msm_rng_read(struct hwrng *hwrng, void *data, size_t max, bool wait)
-{
-   struct msm_rng *rng = to_msm_rng(hwrng);
-   size_t currsize = 0;
-   u32 *retdata = data;
-   size_t maxsize;
-   int ret;
-   u32 val;
-
-   /* calculate max size bytes to transfer back to caller */
-   maxsize = min_t(size_t, MAX_HW_FIFO_SIZE, max);
-
-   ret 

[PATCH v6 4/6] dt-bindings: crypto: Add new compatible qcom,prng-ee

[Vinod Koul]

Later qcom chips support v2 of the prng, which exposes an EE
(Execution Environment) for OS to use so add new compatible
qcom,prng-ee for this.

Signed-off-by: Vinod Koul 
---
 Documentation/devicetree/bindings/crypto/qcom,prng.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/crypto/qcom,prng.txt 
b/Documentation/devicetree/bindings/crypto/qcom,prng.txt
index 8e5853c2879b..7ee0e9eac973 100644
--- a/Documentation/devicetree/bindings/crypto/qcom,prng.txt
+++ b/Documentation/devicetree/bindings/crypto/qcom,prng.txt
@@ -2,7 +2,9 @@ Qualcomm MSM pseudo random number generator.
 
 Required properties:
 
-- compatible  : should be "qcom,prng"
+- compatible  : should be "qcom,prng" for 8916 etc
+  : should be "qcom,prng-ee" for 8996 and later using EE
+   (Execution Environment) slice of prng
 - reg : specifies base physical address and size of the registers map
 - clocks  : phandle to clock-controller plus clock-specifier pair
 - clock-names : "core" clocks all registers, FIFO and circuits in PRNG IP block
-- 
2.14.4

[PATCH v6 5/6] crypto: qcom: Add support for prng-ee

[Vinod Koul]

Qcom 8996 and later chips features multiple Execution Environments
(EE) and secure world is typically responsible for configuring the
prng.

Add driver data for qcom,prng as 0 and qcom,prng-ee as 1 and use
that to skip initialization routine.

Signed-off-by: Vinod Koul 
---
 drivers/crypto/qcom-rng.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
index e921382099d7..80544326e310 100644
--- a/drivers/crypto/qcom-rng.c
+++ b/drivers/crypto/qcom-rng.c
@@ -28,6 +28,7 @@ struct qcom_rng {
struct mutex lock;
void __iomem *base;
struct clk *clk;
+   unsigned int skip_init;
 };
 
 struct qcom_rng_ctx {
@@ -128,7 +129,10 @@ static int qcom_rng_init(struct crypto_tfm *tfm)
 
ctx->rng = qcom_rng_dev;
 
-   return qcom_rng_enable(ctx->rng);
+   if (!ctx->rng->skip_init)
+   return qcom_rng_enable(ctx->rng);
+
+   return 0;
 }
 
 static struct rng_alg qcom_rng_alg = {
@@ -168,6 +172,8 @@ static int qcom_rng_probe(struct platform_device *pdev)
if (IS_ERR(rng->clk))
return PTR_ERR(rng->clk);
 
+   rng->skip_init = (unsigned long)device_get_match_data(>dev);
+
qcom_rng_dev = rng;
ret = crypto_register_rng(_rng_alg);
if (ret) {
@@ -188,7 +194,8 @@ static int qcom_rng_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_rng_of_match[] = {
-   { .compatible = "qcom,prng" },
+   { .compatible = "qcom,prng", .data = (void *)0},
+   { .compatible = "qcom,prng-ee", .data = (void *)1},
{}
 };
 MODULE_DEVICE_TABLE(of, qcom_rng_of_match);
-- 
2.14.4

[PATCH v6 3/6] crypto: Add Qcom prng driver

[Vinod Koul]

This ports the Qcom prng from older hw_random driver.

No change of functionality and move from hw_random to crypto
APIs is done.

Reviewed-by: Linus Walleij 
Signed-off-by: Vinod Koul 
---
 drivers/crypto/Kconfig|  11 +++
 drivers/crypto/Makefile   |   1 +
 drivers/crypto/qcom-rng.c | 208 ++
 3 files changed, 220 insertions(+)
 create mode 100644 drivers/crypto/qcom-rng.c

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 43cccf6aff61..4156f9e8e397 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -585,6 +585,17 @@ config CRYPTO_DEV_QCE
  hardware. To compile this driver as a module, choose M here. The
  module will be called qcrypto.
 
+config CRYPTO_DEV_QCOM_RNG
+   tristate "Qualcomm Random Number Generator Driver"
+   depends on ARCH_QCOM || COMPILE_TEST
+   select CRYPTO_RNG
+   help
+ This driver provides support for the Random Number
+ Generator hardware found on Qualcomm SoCs.
+
+ To compile this driver as a module, choose M here. The
+  module will be called qcom-rng. If unsure, say N.
+
 config CRYPTO_DEV_VMX
bool "Support for VMX cryptographic acceleration instructions"
depends on PPC64 && VSX
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 7ae87b4f6c8d..3602875c4f80 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_CRYPTO_DEV_PICOXCELL) += picoxcell_crypto.o
 obj-$(CONFIG_CRYPTO_DEV_PPC4XX) += amcc/
 obj-$(CONFIG_CRYPTO_DEV_QAT) += qat/
 obj-$(CONFIG_CRYPTO_DEV_QCE) += qce/
+obj-$(CONFIG_CRYPTO_DEV_QCOM_RNG) += qcom-rng.o
 obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/
 obj-$(CONFIG_CRYPTO_DEV_S5P) += s5p-sss.o
 obj-$(CONFIG_CRYPTO_DEV_SAHARA) += sahara.o
diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
new file mode 100644
index ..e921382099d7
--- /dev/null
+++ b/drivers/crypto/qcom-rng.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2017-18 Linaro Limited
+//
+// Based on msm-rng.c and downstream driver
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Device specific register offsets */
+#define PRNG_DATA_OUT  0x
+#define PRNG_STATUS0x0004
+#define PRNG_LFSR_CFG  0x0100
+#define PRNG_CONFIG0x0104
+
+/* Device specific register masks and config values */
+#define PRNG_LFSR_CFG_MASK 0x
+#define PRNG_LFSR_CFG_CLOCKS   0x
+#define PRNG_CONFIG_HW_ENABLE  BIT(1)
+#define PRNG_STATUS_DATA_AVAIL BIT(0)
+
+#define WORD_SZ4
+
+struct qcom_rng {
+   struct mutex lock;
+   void __iomem *base;
+   struct clk *clk;
+};
+
+struct qcom_rng_ctx {
+   struct qcom_rng *rng;
+};
+
+static struct qcom_rng *qcom_rng_dev;
+
+static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max)
+{
+   unsigned int currsize = 0;
+   u32 val;
+
+   /* read random data from hardware */
+   do {
+   val = readl_relaxed(rng->base + PRNG_STATUS);
+   if (!(val & PRNG_STATUS_DATA_AVAIL))
+   break;
+
+   val = readl_relaxed(rng->base + PRNG_DATA_OUT);
+   if (!val)
+   break;
+
+   if ((max - currsize) >= WORD_SZ) {
+   memcpy(data, , WORD_SZ);
+   data += WORD_SZ;
+   currsize += WORD_SZ;
+   } else {
+   /* copy only remaining bytes */
+   memcpy(data, , max - currsize);
+   break;
+   }
+   } while (currsize < max);
+
+   return currsize;
+}
+
+static int qcom_rng_generate(struct crypto_rng *tfm,
+const u8 *src, unsigned int slen,
+u8 *dstn, unsigned int dlen)
+{
+   struct qcom_rng_ctx *ctx = crypto_rng_ctx(tfm);
+   struct qcom_rng *rng = ctx->rng;
+   int ret;
+
+   ret = clk_prepare_enable(rng->clk);
+   if (ret)
+   return ret;
+
+   mutex_lock(>lock);
+
+   ret = qcom_rng_read(rng, dstn, dlen);
+
+   mutex_unlock(>lock);
+   clk_disable_unprepare(rng->clk);
+
+   return 0;
+}
+
+static int qcom_rng_seed(struct crypto_rng *tfm, const u8 *seed,
+unsigned int slen)
+{
+   return 0;
+}
+
+static int qcom_rng_enable(struct qcom_rng *rng)
+{
+   u32 val;
+   int ret;
+
+   ret = clk_prepare_enable(rng->clk);
+   if (ret)
+   return ret;
+
+   /* Enable PRNG only if it is not already enabled */
+   val = readl_relaxed(rng->base + PRNG_CONFIG);
+   if (val & PRNG_CONFIG_HW_ENABLE)
+   goto already_enabled;
+
+   val = readl_relaxed(rng->base + PRNG_LFSR_CFG);
+   val &= ~PRNG

[PATCH v6 6/6] crypto: qcom: Add ACPI support

[Vinod Koul]

From: Timur Tabi 

Add support for probing on ACPI systems, with ACPI HID QCOM8160.

On ACPI systems, clocks are always enabled, the PRNG should
already be enabled, and the register region is read-only.
The driver only verifies that the hardware is already
enabled never tries to disable or configure it.

Signed-off-by: Timur Tabi 
Tested-by: Jeffrey Hugo 
[port to crypto API]
Signed-off-by: Vinod Koul 
---
 drivers/crypto/qcom-rng.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
index 80544326e310..e54249ccc009 100644
--- a/drivers/crypto/qcom-rng.c
+++ b/drivers/crypto/qcom-rng.c
@@ -4,6 +4,7 @@
 // Based on msm-rng.c and downstream driver
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -168,9 +169,13 @@ static int qcom_rng_probe(struct platform_device *pdev)
if (IS_ERR(rng->base))
return PTR_ERR(rng->base);
 
-   rng->clk = devm_clk_get(>dev, "core");
-   if (IS_ERR(rng->clk))
-   return PTR_ERR(rng->clk);
+   /* ACPI systems have clk already on, so skip clk_get */
+   if (!has_acpi_companion(>dev)) {
+   rng->clk = devm_clk_get(>dev, "core");
+   if (IS_ERR(rng->clk))
+   return PTR_ERR(rng->clk);
+   }
+
 
rng->skip_init = (unsigned long)device_get_match_data(>dev);
 
@@ -193,6 +198,14 @@ static int qcom_rng_remove(struct platform_device *pdev)
return 0;
 }
 
+#if IS_ENABLED(CONFIG_ACPI)
+static const struct acpi_device_id qcom_rng_acpi_match[] = {
+   { .id = "QCOM8160", .driver_data = 1 },
+   {}
+};
+MODULE_DEVICE_TABLE(acpi, qcom_rng_acpi_match);
+#endif
+
 static const struct of_device_id qcom_rng_of_match[] = {
{ .compatible = "qcom,prng", .data = (void *)0},
{ .compatible = "qcom,prng-ee", .data = (void *)1},
@@ -206,6 +219,7 @@ static struct platform_driver qcom_rng_driver = {
.driver = {
.name = KBUILD_MODNAME,
.of_match_table = of_match_ptr(qcom_rng_of_match),
+   .acpi_match_table = ACPI_PTR(qcom_rng_acpi_match),
}
 };
 module_platform_driver(qcom_rng_driver);
-- 
2.14.4

[PATCH v6 0/6] crypto: Add Qcom PRNG support

[Vinod Koul]

This series removes the hwrng qcom driver and replaces it with crypto qcom
driver and then adds support for Execution Environment (EE) found in v2
version of the hardware and ACPI support for these

Changes in v6:
 - Fix a typo in kconfig. Remove of_device.h and add of.h header
 - Add review and tested tags

Changes in v5:
 - Update ACPI check and use generic driver data API

Changes in v4:
 - Use memcpy for data copy
 - Fix trailing bytes copy
 - Fix ACPI ID table name

Timur Tabi (1):
  crypto: qcom: Add ACPI support

Vinod Koul (5):
  hwrng: remove msm hw_random driver
  dt-bindings: crypto: Move prng binding to crypto
  crypto: Add Qcom prng driver
  dt-bindings: crypto: Add new compatible qcom,prng-ee
  crypto: qcom: Add support for prng-ee

 .../bindings/{rng => crypto}/qcom,prng.txt |   4 +-
 drivers/char/hw_random/Kconfig |  13 --
 drivers/char/hw_random/Makefile|   1 -
 drivers/char/hw_random/msm-rng.c   | 183 
 drivers/crypto/Kconfig |  11 +
 drivers/crypto/Makefile|   1 +
 drivers/crypto/qcom-rng.c  | 229 +
 7 files changed, 244 insertions(+), 198 deletions(-)
 rename Documentation/devicetree/bindings/{rng => crypto}/qcom,prng.txt (73%)
 delete mode 100644 drivers/char/hw_random/msm-rng.c
 create mode 100644 drivers/crypto/qcom-rng.c

-- 
2.14.4

[PATCH v6 2/6] dt-bindings: crypto: Move prng binding to crypto

[Vinod Koul]

Now that we are adding new driver for prng in crypto, move the
binding as well.

Signed-off-by: Vinod Koul 
---
 Documentation/devicetree/bindings/{rng => crypto}/qcom,prng.txt | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/{rng => crypto}/qcom,prng.txt (100%)

diff --git a/Documentation/devicetree/bindings/rng/qcom,prng.txt 
b/Documentation/devicetree/bindings/crypto/qcom,prng.txt
similarity index 100%
rename from Documentation/devicetree/bindings/rng/qcom,prng.txt
rename to Documentation/devicetree/bindings/crypto/qcom,prng.txt
-- 
2.14.4

Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding

[Vinod Koul]

On Fri, Nov 10, 2017 at 10:44:30AM -0600, Kim Phillips wrote:
> On Fri, 10 Nov 2017 08:02:01 +
> Radu Andrei Alexe  wrote:
> 
> > On 11/9/2017 6:34 PM, Kim Phillips wrote:
> > > On Thu, 9 Nov 2017 11:54:13 +
> > > Radu Andrei Alexe  wrote:
> > >> The next patch version will create the platform device dynamically at
> > >> run time.
> > > 
> > > Why create a new device when that h/w already has one?
> > > 
> > > Why doesn't the existing crypto driver register dma capabilities with
> > > the dma driver subsystem?
> > >
> > I can think of two reasons:
> > 
> > 1. The code that this driver introduces has nothing to do with crypto 
> > and everything to do with dma.
> 
> I would think that at least a crypto "null" algorithm implementation
> would share code.
> 
> > Placing the code in the same directory as 
> > the caam subsystem would only create confusion for the reader of an 
> > already complex driver.
> 
> this different directory argument seems to be identical to your 2 below:
> 
> > 2. I wanted this driver to be tracked by the dma engine team. They have 
> > the right expertise to provide adequate feedback. If all the code was in 
> > the crypto directory they wouldn't know about this driver or any 
> > subsequent changes to it.
> 
> dma subsystem bits could still be put in the dma area if deemed
> necessary but I don't think it is: I see
> drivers/crypto/ccp/ccp-dmaengine.c calls dma_async_device_register for
> example.

which is a shame as it was sneaked past the dmaengine community!!

This is the *only* example and there and other examples where people use
dmaengine:

$ grep -rl dmaengine_prep* drivers/crypto/* |uniq
drivers/crypto/atmel-aes.c
drivers/crypto/atmel-sha.c
drivers/crypto/atmel-tdes.c
drivers/crypto/img-hash.c
drivers/crypto/omap-aes.c
drivers/crypto/omap-des.c
drivers/crypto/omap-sham.c
drivers/crypto/qce/dma.c
drivers/crypto/stm32/stm32-hash.c
drivers/crypto/ux500/cryp/cryp_core.c
drivers/crypto/ux500/hash/hash_core.c



> 
> I also don't see how that complicates things much further.
> 
> What is the rationale for using the crypto h/w as a dma engine anyway?
> Are there supporting performance figures?

that is a very good question, perf does matter. Given that we have many
folks using it, I think it would help, but yes nothing better than numbers
speak for themselves.

-- 
~Vinod

Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding

[Vinod Koul]

On Fri, Nov 10, 2017 at 08:02:01AM +, Radu Andrei Alexe wrote:
> On 11/9/2017 6:34 PM, Kim Phillips wrote:
> > On Thu, 9 Nov 2017 11:54:13 +
> > Radu Andrei Alexe  wrote:
> > 
> >> On 10/30/2017 4:24 PM, Kim Phillips wrote:
> >>> On Mon, 30 Oct 2017 15:46:51 +0200
> >>> Horia Geantă  wrote:
> >>>
>  +=
>  +CAAM DMA Node
>  +
>  +Child node of the crypto node that enables the use of the DMA 
>  capabilities
>  +of the CAAM by a stand-alone driver. The only required property is 
>  the
>  +"compatible" property. All the other properties are determined from
>  +the job rings on which the CAAM DMA driver depends (ex: the number 
>  of
>  +dma-channels is equal to the number of defined job rings).
>  +
>  +  - compatible
>  +  Usage: required
>  +  Value type: 
>  +  Definition: Must include "fsl,sec-v4.0-dma"
>  +
>  +EXAMPLE
>  +  caam-dma {
>  +compatible = "fsl,sec-v5.4-dma",
>  + "fsl,sec-v5.0-dma",
>  + "fsl,sec-v4.0-dma";
>  +  }
> >>>
> >>> If this isn't describing an aspect of the hardware, then what is it
> >>> doing in the device tree?
> >>>
> >>> Kim
> >>>
> >>
> >> Because the caam_dma driver is a platform driver I needed to create a
> >> platform device to activate the driver. My thought was that it was
> >> simpler to implement it using device tree.
> >> The next patch version will create the platform device dynamically at
> >> run time.
> > 
> > Why create a new device when that h/w already has one?
> > 
> > Why doesn't the existing crypto driver register dma capabilities with
> > the dma driver subsystem?
> > 
> > Kim
> > 
> 
> 
> I can think of two reasons:
> 
> 1. The code that this driver introduces has nothing to do with crypto 
> and everything to do with dma. Placing the code in the same directory as 
> the caam subsystem would only create confusion for the reader of an 
> already complex driver.
> 
> 2. I wanted this driver to be tracked by the dma engine team. They have 
> the right expertise to provide adequate feedback. If all the code was in 
> the crypto directory they wouldn't know about this driver or any 
> subsequent changes to it.

These are very good reasons.

We already have one DMA implementation drivers/crypto/ccp/ccp-dmaengine.c
which was sneaked past us and put into kernel with proper review!

On the other hand we have *bunch* of dmaengine users in crypto subsystem
which use dmaengine drivers and APIs and are good citizens. It allows folks
to share code with other usages of dmaengine and the usual arguments for
common code and frameworks...

If there are enough users we can add up a crypto-dmaengine lib which
programs dma controller for crypto users and avoid open coding for everyone.
for example sound-dmaengine layers does that...

HTH
-- 
~Vinod

Re: [PATCH RESEND 4/4] dma: caam: add dma memcpy driver

[Vinod Koul]

On Wed, Nov 08, 2017 at 02:36:31PM +, Radu Andrei Alexe wrote:
> On 10/31/2017 2:01 PM, Vinod Koul wrote:
> > On Mon, Oct 30, 2017 at 03:46:54PM +0200, Horia Geantă wrote:
> >> +/*
> >> + * caam support for SG DMA
> >> + *
> >> + * Copyright 2016 Freescale Semiconductor, Inc
> >> + * Copyright 2017 NXP
> >> + */
> > 
> > that is interesting, no license text?
> > 
> 
> Thanks for the catch. The next patch will contain the full license text.

or as is the "new" practice, you may used SPDX tags

> >> +#include "../crypto/caam/regs.h"
> >> +#include "../crypto/caam/jr.h"
> >> +#include "../crypto/caam/error.h"
> >> +#include "../crypto/caam/intern.h"
> >> +#include "../crypto/caam/desc_constr.h"
> > 
> > ah that puts very hard assumptions on locations of the subsystems. Please do
> > not do that and move the required stuff into common header location in
> > include/
> > 
> 
> Unfortunately this is not possible. The functionality used by CAAM DMA 
> from the CAAM subsystem should not be exported to be used by other 
> modules. It is because this driver is so tightly coupled with the CAAM 
> driver(s) that it needs access to it's 'internal' functionality (that 
> should not be otherwise shared with anyone).

Which other driver would be interested in your CAM implementation except
you. Realistically speaking none, so it is perfectly fine to do so in a
header at a right place!

> >> +struct caam_dma_sh_desc {
> > 
> > sh?
> > 
> 
> It means "shared". It is obvious to anyone who is familiar with the CAAM 
> system.

Unfortunately I am not. As a patch writer you have the onus to explain it to
people who live outside the CAAM world

> >> +static struct dma_device *dma_dev;
> >> +static struct caam_dma_sh_desc *dma_sh_desc;
> >> +static LIST_HEAD(dma_ctx_list);
> > 
> > why do you need so many globals?
> > 
> 
> How many globals are too many? I can group them in a common structure. 
> But I'm not sure how would that help.

I prefer none. Make sure the struct instance is not global and allocated in
your driver probe routine.


> >> +static dma_cookie_t caam_dma_tx_submit(struct dma_async_tx_descriptor *tx)
> >> +{
> >> +  struct caam_dma_edesc *edesc = NULL;
> >> +  struct caam_dma_ctx *ctx = NULL;
> >> +  dma_cookie_t cookie;
> >> +
> >> +  edesc = container_of(tx, struct caam_dma_edesc, async_tx);
> >> +  ctx = container_of(tx->chan, struct caam_dma_ctx, chan);
> >> +
> >> +  spin_lock_bh(>edesc_lock);
> > 
> > why _bh, i didnt see any irqs or tasklets here which is actually odd, so
> > what is going on
> > 
> 
> The tasklet is hidden inside the JR driver from the CAAM subsystem (see 
> drivers/crypto/caam/jr.c). The function caam_dma_done(), which is called 
> from the JR tasklet handler, also uses the spin_lock. I need to disable 
> bottom half to make sure that I don't run into a deadlock when I'm 
> preempted.

This would be the right time to explain why. I understand that your subsystem
has tightly coupled DMA but that doesn't really mean the DMA controller should
not have its own IRQ and bh. This makes things harder to review and follow.

> >> +  cookie = dma_cookie_assign(tx);
> >> +  list_add_tail(>node, >submit_q);
> >> +
> >> +  spin_unlock_bh(>edesc_lock);
> >> +
> >> +  return cookie;
> >> +}
> > 
> > we have a virtual channel wrapper where we do the same stuff as above, so
> > consider reusing that
> > 
> 
> Some of the functionality that the virtual channel might provide cannot 
> be extracted because it is embedded into the JR driver (e.g. the 
> tasklet). Therefore the use of the virtual channel is inefficient at 
> best if not even impossible.

Which itself is problematic in first place and no explanation provided on
why. Yes you have a special hardware, so does every second person!

> >> +static void caam_dma_issue_pending(struct dma_chan *chan)
> >> +{
> >> +  struct caam_dma_ctx *ctx = container_of(chan, struct caam_dma_ctx,
> >> +  chan);
> >> +  struct caam_dma_edesc *edesc, *_edesc;
> >> +
> >> +  spin_lock_bh(>edesc_lock);
> >> +  list_for_each_entry_safe(edesc, _edesc, >submit_q, node) {
> >> +  if (caam_jr_enqueue(ctx->jrdev, edesc->jd,
> >> +  caam_dma_done, edesc) < 0)
> > 
> > what does the caam_jr_enqueue() do?
>

Re: [PATCH RESEND 4/4] dma: caam: add dma memcpy driver

[Vinod Koul]

On Mon, Oct 30, 2017 at 03:46:54PM +0200, Horia Geantă wrote:

> @@ -600,6 +600,23 @@ config ZX_DMA
>   help
> Support the DMA engine for ZTE ZX family platform devices.
>  
> +config CRYPTO_DEV_FSL_CAAM_DMA

File is sorted alphabetically

> + tristate "CAAM DMA engine support"
> + depends on CRYPTO_DEV_FSL_CAAM_JR
> + default y

why should it be default?

> --- /dev/null
> +++ b/drivers/dma/caam_dma.c
> @@ -0,0 +1,444 @@
> +/*
> + * caam support for SG DMA
> + *
> + * Copyright 2016 Freescale Semiconductor, Inc
> + * Copyright 2017 NXP
> + */

that is interesting, no license text?

> +
> +#include 
> +#include 
> +#include 
> +#include 

why do you need this

> +#include 
> +#include 

i didn't see any debugfs code, why do you need this

alphabetical sort pls
> +
> +#include 
> +#include "dmaengine.h"
> +
> +#include "../crypto/caam/regs.h"
> +#include "../crypto/caam/jr.h"
> +#include "../crypto/caam/error.h"
> +#include "../crypto/caam/intern.h"
> +#include "../crypto/caam/desc_constr.h"

ah that puts very hard assumptions on locations of the subsystems. Please do
not do that and move the required stuff into common header location in
include/

> +/* This is max chunk size of a DMA transfer. If a buffer is larger than this
> + * value it is internally broken into chunks of max CAAM_DMA_CHUNK_SIZE bytes
> + * and for each chunk a DMA transfer request is issued.
> + * This value is the largest number on 16 bits that is a multiple of 256 
> bytes
> + * (the largest configurable CAAM DMA burst size).
> + */

Kernel comments style we follow is:

/*
 * this is for
 * multi line comment
 */

Pls fix this in the file

> +#define CAAM_DMA_CHUNK_SIZE  65280
> +
> +struct caam_dma_sh_desc {

sh?

> +struct caam_dma_ctx {
> + struct dma_chan chan;
> + struct list_head node;
> + struct device *jrdev;
> + struct list_head submit_q;

call it pending

> +static struct dma_device *dma_dev;
> +static struct caam_dma_sh_desc *dma_sh_desc;
> +static LIST_HEAD(dma_ctx_list);

why do you need so many globals?

> +static dma_cookie_t caam_dma_tx_submit(struct dma_async_tx_descriptor *tx)
> +{
> + struct caam_dma_edesc *edesc = NULL;
> + struct caam_dma_ctx *ctx = NULL;
> + dma_cookie_t cookie;
> +
> + edesc = container_of(tx, struct caam_dma_edesc, async_tx);
> + ctx = container_of(tx->chan, struct caam_dma_ctx, chan);
> +
> + spin_lock_bh(>edesc_lock);

why _bh, i didnt see any irqs or tasklets here which is actually odd, so
what is going on

> +
> + cookie = dma_cookie_assign(tx);
> + list_add_tail(>node, >submit_q);
> +
> + spin_unlock_bh(>edesc_lock);
> +
> + return cookie;
> +}

we have a virtual channel wrapper where we do the same stuff as above, so
consider reusing that

> +static void caam_dma_memcpy_init_job_desc(struct caam_dma_edesc *edesc)
> +{
> + u32 *jd = edesc->jd;
> + u32 *sh_desc = dma_sh_desc->desc;
> + dma_addr_t desc_dma = dma_sh_desc->desc_dma;
> +
> + /* init the job descriptor */
> + init_job_desc_shared(jd, desc_dma, desc_len(sh_desc), HDR_REVERSE);
> +
> + /* set SEQIN PTR */
> + append_seq_in_ptr(jd, edesc->src_dma, edesc->src_len, 0);
> +
> + /* set SEQOUT PTR */
> + append_seq_out_ptr(jd, edesc->dst_dma, edesc->dst_len, 0);
> +
> +#ifdef DEBUG
> + print_hex_dump(KERN_ERR, "caam dma desc@" __stringify(__LINE__) ": ",
> +DUMP_PREFIX_ADDRESS, 16, 4, jd, desc_bytes(jd), 1);

ah this make you compile kernel. Consider the dynamic printk helpers for
printing

> +/* This function can be called in an interrupt context */
> +static void caam_dma_issue_pending(struct dma_chan *chan)
> +{
> + struct caam_dma_ctx *ctx = container_of(chan, struct caam_dma_ctx,
> + chan);
> + struct caam_dma_edesc *edesc, *_edesc;
> +
> + spin_lock_bh(>edesc_lock);
> + list_for_each_entry_safe(edesc, _edesc, >submit_q, node) {
> + if (caam_jr_enqueue(ctx->jrdev, edesc->jd,
> + caam_dma_done, edesc) < 0)

what does the caam_jr_enqueue() do?

> +static int caam_dma_jr_chan_bind(void)
> +{
> + struct device *jrdev;
> + struct caam_dma_ctx *ctx;
> + int bonds = 0;
> + int i;
> +
> + for (i = 0; i < caam_jr_driver_probed(); i++) {
> + jrdev = caam_jridx_alloc(i);
> + if (IS_ERR(jrdev)) {
> + pr_err("job ring device %d allocation failed\n", i);
> + continue;
> + }
> +
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx) {
> + caam_jr_free(jrdev);
> + continue;

you want to continue?

> + dma_dev->dev = dev;
> + dma_dev->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
> + dma_cap_set(DMA_MEMCPY, dma_dev->cap_mask);
> + dma_cap_set(DMA_PRIVATE, dma_dev->cap_mask);
> + dma_dev->device_tx_status = 

Re: [PATCH 0/4] add CAAM DMA memcpy driver

[Vinod Koul]

On Fri, Oct 27, 2017 at 12:20:32PM +, Horia Geantă wrote:
> On 10/27/2017 2:36 PM, Koul, Vinod wrote:
> >> On 10/26/2017 1:01 PM, Radu Alexe wrote:
> >>> This patch-set introduces a new DMA memcpy driver based on the DMA
> >>> capabilities of the CAAM crypto engine. Because of this dependency the
> >>> included commits target various parts of the kernel tree.
> >> I don't see the patches on any of the mail lists.
> >> If they are moderated, I would expect this to be mentioned in the
> >> MAINTAINERS file.
> >>
> >>>
> >>> Patch 1.
> >>> Since the CAAM DMA driver is a platform driver it is enabled by a new node
> >>> in the device tree. This commit adds documentation for the device tree
> >>> bindings.
> >>>
> >>> Patch 2.
> >>> This patch adds the "caam-dma" node in the fsl-ls1012a.dtsi file.
> >>>
> >>> Patch 3.
> >>> This commit adds various capabilities in the JR driver of the CAAM that is
> >>> used by the CAAM DMA driver.
> >>>
> >>> Patch 4.
> >>> Adds the CAAM DMA memcpy driver.
> >>>
> >>> Patch 1 and 3 should be merged by the crypto maintainers, patch 2 by
> >>> devicetree maintainers and patch 4 by the DMA maintainers.
> >> I would go with all the patches through the dmaengine tree.
> > 
> > Good call, but no one CCed dmaengine folks on this series!
> > 
> I see dmaeng...@vger.kernel.org Cc-ed on both cover letter and all four
> patches.
> It might be that the sender (Radu) is not subscribed and messages are
> awaiting moderators' approval.

I think vger lists are open, maybe sender has some issues with mail as they
are still not in any archives..

-- 
~Vinod

Re: [PATCH v8 0/4] Broadcom SBA RAID support

[Vinod Koul]

On Mon, May 15, 2017 at 10:34:51AM +0530, Anup Patel wrote:
> The Broadcom SBA RAID is a stream-based device which provides
> RAID5/6 offload.
> 
> It requires a SoC specific ring manager (such as Broadcom FlexRM
> ring manager) to provide ring-based programming interface. Due to
> this, the Broadcom SBA RAID driver (mailbox client) implements
> DMA device having one DMA channel using a set of mailbox channels
> provided by Broadcom SoC specific ring manager driver (mailbox
> controller).
> 
> The Broadcom SBA RAID hardware requires PQ disk position instead
> of PQ disk coefficient. To address this, we have added raid_gflog
> table which will help driver to convert PQ disk coefficient to PQ
> disk position.

Applied, now

-- 
~Vinod

Re: [PATCH v6 0/4] Broadcom SBA RAID support

[Vinod Koul]

On Wed, May 03, 2017 at 09:15:20AM +0530, Anup Patel wrote:
> Hi Vinod,
> 
> The Broadcom FlexRM patchset have been
> merged in v4.11.
> 
> I think you now can take this patchset in next
> merge window. Right??

Sure, please rebase and resend after -rc1 is out

-- 
~Vinod

Re: [PATCH v6 0/4] Broadcom SBA RAID support

[Vinod Koul]

On Wed, Mar 29, 2017 at 11:35:43AM +0530, Anup Patel wrote:
> On Tue, Mar 21, 2017 at 2:48 PM, Vinod Koul <vinod.k...@intel.com> wrote:
> > On Tue, Mar 21, 2017 at 02:17:21PM +0530, Anup Patel wrote:
> >> On Tue, Mar 21, 2017 at 2:00 PM, Vinod Koul <vinod.k...@intel.com> wrote:
> >> > On Mon, Mar 06, 2017 at 03:13:24PM +0530, Anup Patel wrote:
> >> >> The Broadcom SBA RAID is a stream-based device which provides
> >> >> RAID5/6 offload.
> >> >>
> >> >> It requires a SoC specific ring manager (such as Broadcom FlexRM
> >> >> ring manager) to provide ring-based programming interface. Due to
> >> >> this, the Broadcom SBA RAID driver (mailbox client) implements
> >> >> DMA device having one DMA channel using a set of mailbox channels
> >> >> provided by Broadcom SoC specific ring manager driver (mailbox
> >> >> controller).
> >> >>
> >> >> The Broadcom SBA RAID hardware requires PQ disk position instead
> >> >> of PQ disk coefficient. To address this, we have added raid_gflog
> >> >> table which will help driver to convert PQ disk coefficient to PQ
> >> >> disk position.
> >> >>
> >> >> This patchset is based on Linux-4.11-rc1 and depends on patchset
> >> >> "[PATCH v5 0/2] Broadcom FlexRM ring manager support"
> >> >
> >> > Okay I applied and was about to push when I noticed this :(
> >> >
> >> > So what is the status of this..?
> >>
> >> PATCH2 is Acked but PATCH1 is under-review. Currently, its
> >> v6 of that patchset.
> >>
> >> The only dependency on that patchset is the changes in
> >> brcm-message.h which are required by this BCM-SBA-RAID
> >> driver.
> >>
> >> @Jassi,
> >> Can you please have a look at PATCH v6?
> >
> > And I would need an immutable branch/tag once merged. I am going to keep
> > this series pending till then.
> 
> The Broadcom FlexRM patchset is pickedup by Jassi and
> can be found in mailbox-for-next branch of
> git://git.linaro.org/landing-teams/working/fujitsu/integration
> 
> Both patchset (Broadcom FlexRM patchset and this one) are
> also available in sba-raid-v7 branch of
> https://github.com/Broadcom/arm64-linux.git

Jassi,

Can you provide an immutable branch/tag please for this, latter is
preferred.

Btw didn't find your tree in MAINTAINERS..

> 
> Regards,
> Anup

-- 
~Vinod

Re: [PATCH v6 0/4] Broadcom SBA RAID support

[Vinod Koul]

On Tue, Mar 21, 2017 at 02:17:21PM +0530, Anup Patel wrote:
> On Tue, Mar 21, 2017 at 2:00 PM, Vinod Koul <vinod.k...@intel.com> wrote:
> > On Mon, Mar 06, 2017 at 03:13:24PM +0530, Anup Patel wrote:
> >> The Broadcom SBA RAID is a stream-based device which provides
> >> RAID5/6 offload.
> >>
> >> It requires a SoC specific ring manager (such as Broadcom FlexRM
> >> ring manager) to provide ring-based programming interface. Due to
> >> this, the Broadcom SBA RAID driver (mailbox client) implements
> >> DMA device having one DMA channel using a set of mailbox channels
> >> provided by Broadcom SoC specific ring manager driver (mailbox
> >> controller).
> >>
> >> The Broadcom SBA RAID hardware requires PQ disk position instead
> >> of PQ disk coefficient. To address this, we have added raid_gflog
> >> table which will help driver to convert PQ disk coefficient to PQ
> >> disk position.
> >>
> >> This patchset is based on Linux-4.11-rc1 and depends on patchset
> >> "[PATCH v5 0/2] Broadcom FlexRM ring manager support"
> >
> > Okay I applied and was about to push when I noticed this :(
> >
> > So what is the status of this..?
> 
> PATCH2 is Acked but PATCH1 is under-review. Currently, its
> v6 of that patchset.
> 
> The only dependency on that patchset is the changes in
> brcm-message.h which are required by this BCM-SBA-RAID
> driver.
> 
> @Jassi,
> Can you please have a look at PATCH v6?

And I would need an immutable branch/tag once merged. I am going to keep
this series pending till then.

-- 
~Vinod

Re: [PATCH v2 2/5] async_tx: Handle DMA devices having support for fewer PQ coefficients

[Vinod Koul]

On Tue, Feb 07, 2017 at 02:32:15PM +0530, Anup Patel wrote:
> On Tue, Feb 7, 2017 at 1:57 PM, Dan Williams  wrote:
> > On Tue, Feb 7, 2017 at 12:16 AM, Anup Patel  wrote:
> >> The DMAENGINE framework assumes that if PQ offload is supported by a
> >> DMA device then all 256 PQ coefficients are supported. This assumption
> >> does not hold anymore because we now have BCM-SBA-RAID offload engine
> >> which supports PQ offload with limited number of PQ coefficients.
> >>
> >> This patch extends async_tx APIs to handle DMA devices with support
> >> for fewer PQ coefficients.
> >>
> >> Signed-off-by: Anup Patel 
> >> Reviewed-by: Scott Branden 
> >
> > I don't like this approach. Define an interface for md to query the
> > offload engine once at the beginning of time. We should not be adding
> > any new extensions to async_tx.
> 
> Even if we do capability checks in Linux MD, we still need a way
> for DMAENGINE drivers to advertise number of PQ coefficients
> handled by the HW.

If the question is only for advertising caps, then why not do as done
for dma_get_slave_caps(). you can add dma_get_pq_caps() so that clients (md)
in this case would know the HW capability.

> I agree capability checks should be done once in Linux MD but I don't
> see why this has to be part of BCM-SBA-RAID driver patches. We need
> separate patchsets to address limitations of async_tx framework.
> 
> Regards,
> Anup

-- 
~Vinod

Re: [PATCH 5/6] dmaengine: Add Broadcom SBA RAID driver

[Vinod Koul]

On Mon, Feb 06, 2017 at 05:31:15PM +0530, Anup Patel wrote:

> >> +
> >> +/* SBA C_MDATA helper macros */
> >> +#define SBA_C_MDATA_LOAD_VAL(__bnum0)((__bnum0) & 0x3)
> >> +#define SBA_C_MDATA_WRITE_VAL(__bnum0)   ((__bnum0) & 0x3)
> >> +#define SBA_C_MDATA_XOR_VAL(__bnum1, __bnum0)\
> >> + ({  u32 __v = ((__bnum0) & 0x3);\
> >> + __v |= ((__bnum1) & 0x3) << 2;  \
> >> + __v;\
> >> + })
> >> +#define SBA_C_MDATA_PQ_VAL(__dnum, __bnum1, __bnum0) \
> >> + ({  u32 __v = ((__bnum0) & 0x3);\
> >> + __v |= ((__bnum1) & 0x3) << 2;  \
> >> + __v |= ((__dnum) & 0x1f) << 5;  \
> >> + __v;\
> >> + })
> >
> > ah why are we usig complex macros, why can't these be simple functions..
> 
> "static inline functions" seemed too complicated here because most of
> these macros are two lines of c-code.

and thats where I have an issue with this. Macros for simple things is fine
but not for couple of line of logic!

> 
> Do you still insist on using "static inline functions"?

Yes

> 
> >
> >> +#define SBA_C_MDATA_LS(__c_mdata_val)((__c_mdata_val) & 0xff)
> >> +#define SBA_C_MDATA_MS(__c_mdata_val)(((__c_mdata_val) >> 8) & 
> >> 0x3)
> >> +
> >> +/* Driver helper macros */
> >> +#define to_sba_request(tx)   \
> >> + container_of(tx, struct sba_request, tx)
> >> +#define to_sba_device(dchan) \
> >> + container_of(dchan, struct sba_device, dma_chan)
> >> +
> >> +enum sba_request_state {
> >> + SBA_REQUEST_STATE_FREE = 1,
> >> + SBA_REQUEST_STATE_ALLOCED = 2,
> >> + SBA_REQUEST_STATE_PENDING = 3,
> >> + SBA_REQUEST_STATE_ACTIVE = 4,
> >> + SBA_REQUEST_STATE_COMPLETED = 5,
> >> + SBA_REQUEST_STATE_ABORTED = 6,
> >
> > whats up with a very funny indentation setting, we use 8 chars.
> >
> > Please re-read the Documentation/process/coding-style.rst
> 
> I have double checked this enum. The indentation is fine
> and as-per coding style. Am I missing anything else?

Somehow the initial indent doesnt seem to be 8 chars to me.

> >> +static enum dma_status sba_tx_status(struct dma_chan *dchan,
> >> +  dma_cookie_t cookie,
> >> +  struct dma_tx_state *txstate)
> >> +{
> >> + int mchan_idx;
> >> + enum dma_status ret;
> >> + struct sba_device *sba = to_sba_device(dchan);
> >> +
> >> + ret = dma_cookie_status(dchan, cookie, txstate);
> >> + if (ret == DMA_COMPLETE)
> >> + return ret;
> >> +
> >> + for (mchan_idx = 0; mchan_idx < sba->mchans_count; mchan_idx++)
> >> + mbox_client_peek_data(sba->mchans[mchan_idx]);
> >
> > what is this achieving?
> 
> The mbox_client_peek_data() is a hint to mailbox controller driver
> to check for available messages.
> 
> This gives good performance improvement when some DMA client
> code is polling using tx_status() callback.

Then why do it before and then check status.

-- 
~Vinod

Re: [PATCH 5/6] dmaengine: Add Broadcom SBA RAID driver

[Vinod Koul]

On Thu, Feb 02, 2017 at 10:17:15AM +0530, Anup Patel wrote:
> +config BCM_SBA_RAID
> +tristate "Broadcom SBA RAID engine support"
> +depends on (ARM64 && MAILBOX && RAID6_PQ) || COMPILE_TEST
> +select DMA_ENGINE
> +select DMA_ENGINE_RAID
> + select ASYNC_TX_ENABLE_CHANNEL_SWITCH
> + default ARCH_BCM_IPROC

whats with the funny alignement?

> +/* SBA command related defines */
> +#define SBA_TYPE_SHIFT   48
> +#define SBA_TYPE_MASK0x3
> +#define SBA_TYPE_A   0x0
> +#define SBA_TYPE_B   0x2
> +#define SBA_TYPE_C   0x3
> +#define SBA_USER_DEF_SHIFT   32
> +#define SBA_USER_DEF_MASK0x
> +#define SBA_R_MDATA_SHIFT24
> +#define SBA_R_MDATA_MASK 0xff
> +#define SBA_C_MDATA_MS_SHIFT 18
> +#define SBA_C_MDATA_MS_MASK  0x3
> +#define SBA_INT_SHIFT17
> +#define SBA_INT_MASK 0x1
> +#define SBA_RESP_SHIFT   16
> +#define SBA_RESP_MASK0x1
> +#define SBA_C_MDATA_SHIFT8
> +#define SBA_C_MDATA_MASK 0xff
> +#define SBA_CMD_SHIFT0
> +#define SBA_CMD_MASK 0xf
> +#define SBA_CMD_ZERO_ALL_BUFFERS 0x8
> +#define SBA_CMD_LOAD_BUFFER  0x9
> +#define SBA_CMD_XOR  0xa
> +#define SBA_CMD_GALOIS_XOR   0xb
> +#define SBA_CMD_ZERO_BUFFER  0x4
> +#define SBA_CMD_WRITE_BUFFER 0xc

Try using BIT and GENMAST for hardware descriptions

> +
> +/* SBA C_MDATA helper macros */
> +#define SBA_C_MDATA_LOAD_VAL(__bnum0)((__bnum0) & 0x3)
> +#define SBA_C_MDATA_WRITE_VAL(__bnum0)   ((__bnum0) & 0x3)
> +#define SBA_C_MDATA_XOR_VAL(__bnum1, __bnum0)\
> + ({  u32 __v = ((__bnum0) & 0x3);\
> + __v |= ((__bnum1) & 0x3) << 2;  \
> + __v;\
> + })
> +#define SBA_C_MDATA_PQ_VAL(__dnum, __bnum1, __bnum0) \
> + ({  u32 __v = ((__bnum0) & 0x3);\
> + __v |= ((__bnum1) & 0x3) << 2;  \
> + __v |= ((__dnum) & 0x1f) << 5;  \
> + __v;\
> + })

ah why are we usig complex macros, why can't these be simple functions..

> +#define SBA_C_MDATA_LS(__c_mdata_val)((__c_mdata_val) & 0xff)
> +#define SBA_C_MDATA_MS(__c_mdata_val)(((__c_mdata_val) >> 8) & 0x3)
> +
> +/* Driver helper macros */
> +#define to_sba_request(tx)   \
> + container_of(tx, struct sba_request, tx)
> +#define to_sba_device(dchan) \
> + container_of(dchan, struct sba_device, dma_chan)
> +
> +enum sba_request_state {
> + SBA_REQUEST_STATE_FREE = 1,
> + SBA_REQUEST_STATE_ALLOCED = 2,
> + SBA_REQUEST_STATE_PENDING = 3,
> + SBA_REQUEST_STATE_ACTIVE = 4,
> + SBA_REQUEST_STATE_COMPLETED = 5,
> + SBA_REQUEST_STATE_ABORTED = 6,

whats up with a very funny indentation setting, we use 8 chars.

Please re-read the Documentation/process/coding-style.rst

> +static int sba_alloc_chan_resources(struct dma_chan *dchan)
> +{
> + /*
> +  * We only have one channel so we have pre-alloced
> +  * channel resources. Over here we just return number
> +  * of free request.
> +  */
> + return sba_free_request_count(to_sba_device(dchan));
> +}

essentially you are not doing much, so you can skip it. Its an optional
call.

> +static void sba_free_chan_resources(struct dma_chan *dchan)
> +{
> + /*
> +  * Channel resources are pre-alloced so we just free-up
> +  * whatever we can so that we can re-use pre-alloced
> +  * channel resources next time.
> +  */
> + sba_cleanup_inflight_requests(to_sba_device(dchan));

well this one checks for pending requests as well, which shouldn't be there
when freeing a channel, something seems not quite right here..

> +static int sba_send_mbox_request(struct sba_device *sba,
> +  struct sba_request *req)
> +{
> + int mchans_idx, ret = 0;
> +
> + /* Select mailbox channel in round-robin fashion */
> + mchans_idx = atomic_inc_return(>mchans_current);
> + mchans_idx = mchans_idx % sba->mchans_count;
> +
> + /* Send batch message for the request */
> + req->bmsg.batch.msgs_queued = 0;
> + ret = 

Re: provide DMA services in drivers/crypto

[Vinod Koul]

On Wed, Oct 19, 2016 at 01:57:45PM +, Tudor-Dan Ambarus wrote:

Please wrap your mails within 80 chars, I have reflown below for reabability.

> Hi, Herbert,
> 
> CAAM has the ability to provide DMA services. This is helpful for
> platforms that don't have a dedicated DMA hardware block.
> 
> In this particular example, caam being a crypto accelerator at its
> fundamentals, where is recommended to add a caam-dma engine
> implementation, in drivers/dma or in drivers/crypto? Are the any
> rules/guidelines?

If the dma controller is internal to crypto, then it might be okay to be
inside the crypto driver. But if it is shared or common controller then a
dmaengine implementation would make sense..

> I noticed that CCP already registered DMA services, the driver being held
> in drivers/crypto.

I think drivers/crypto/ccp/ccp-dmaengine.c is wrong example, it should have
been in dmaengine directory. Not sure why it was addded in crypto :(

Thanks
-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] Use dma_pool_zalloc

[Vinod Koul]

On Fri, Apr 29, 2016 at 10:09:07PM +0200, Julia Lawall wrote:
> Dma_pool_zalloc combines dma_pool_alloc and memset 0.  The semantic patch
> that makes this transformation is as follows: (http://coccinelle.lip6.fr/)

Applied all dmaengine patches

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto/async_pq: use __free_page() instead of put_page()

[Vinod Koul]

On Tue, Mar 01, 2016 at 10:54:50PM +0900, Joonsoo Kim wrote:
> 2016-03-01 3:04 GMT+09:00 Dan Williams :
> > On Mon, Feb 29, 2016 at 1:33 AM, Arnd Bergmann  wrote:
> >> The addition of tracepoints to the page reference tracking had an
> >> unfortunate side-effect in at least one driver that calls put_page
> >> from its exit function, resulting in a link error:
> >>
> >> `.exit.text' referenced in section `__jump_table' of crypto/built-in.o: 
> >> defined in discarded section `.exit.text' of crypto/built-in.o
> >>
> >> From a cursory look at that this driver, it seems that it may be
> >> doing the wrong thing here anyway, as the page gets allocated
> >> using 'alloc_page()', and should be freed using '__free_page()'
> >> rather than 'put_page()'.
> >>
> >> With this patch, I no longer get any other build errors from the
> >> page_ref patch, so hopefully we can assume that it's always wrong
> >> to call any of those functions from __exit code, and that no other
> >> driver does it.
> >>
> >> Fixes: 0f80830dd044 ("mm/page_ref: add tracepoint to track down page 
> >> reference manipulation")
> >> Signed-off-by: Arnd Bergmann 
> >
> > Acked-by: Dan Williams 
> >
> > Vinod, will you take this one?
> 
> Problematic patch ("mm/page_ref: ~~~") is not yet merged one. It is on mmotm
> and this fix should go together with it or before it. I think that
> handling this fix by
> Andrew is easier to all.

Okay fine by me.

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/13] dmaengine: Introduce dma_request_slave_channel_compat_reason()

[Vinod Koul]

On Wed, Nov 18, 2015 at 04:51:54PM +0100, Arnd Bergmann wrote:
> On Wednesday 18 November 2015 17:43:04 Andy Shevchenko wrote:
> > >
> > > I assume that the sst-firmware.c case is a mistake, it should just use a
> > > plain DMA_SLAVE and not DMA_MEMCPY.
> > 
> > Other way around.
> > 
> 
> Ok, I see. In that case I guess it also shouldn't call
> dmaengine_slave_config(), right? I don't think that's valid
> on a MEMCPY channel.

Yes it is not valid. In this case the dma driver should invoke a generic memcpy
and not need slave parameters, knowing the hw and reason for this (firmware
download to DSP memory), this doesn't qualify for slave case.
In fact filter function doesn't need a channel, any channel in this
controller will be good

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 3/4] Crypto: Add support for APM X-Gene SoC CRC32C h/w accelerator driver

[Vinod Koul]

On Thu, Aug 20, 2015 at 12:31:44PM +0530, Rameshwar Sahu wrote:
 Hi Vinod,
 
 On Thu, Aug 20, 2015 at 11:18 AM, Vinod Koul vinod.k...@intel.com wrote:
  On Thu, Jul 30, 2015 at 05:41:07PM +0530, Rameshwar Prasad Sahu wrote:
  + nents = sg_nents(req-src);
  + sg_count = dma_map_sg(dev, req-src, nents, DMA_TO_DEVICE);
  + if (!sg_count) {
  + dev_err(dev, Failed to map src sg);
  + return -ENOMEM;
  mapping error shouldn't be no mem error
 Okay, I guess then -EIO will be fine here??

yes better

-- 
~Vinod
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 1/4] dmaengine: Add support for new feature CRC32C

[Vinod Koul]

On Thu, Aug 20, 2015 at 11:59:07AM +0530, Rameshwar Sahu wrote:
 Hi Vinod,
 
 On Thu, Aug 20, 2015 at 10:56 AM, Vinod Koul vinod.k...@intel.com wrote:
  On Thu, Jul 30, 2015 at 05:41:05PM +0530, Rameshwar Prasad Sahu wrote:
  This patch adds support for new feature CRC32C calculation in
  dmaengine framework.
 
  Looks okay can you please update Documentation also
 
 Thanks, I will update Documentation.

Also add a wrapper for new API

-- 
~Vinod

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/4] dmaengine: xgene-dma: Add support for CRC32C calculation via DMA engine

[Vinod Koul]

On Thu, Aug 20, 2015 at 12:23:50PM +0530, Rameshwar Sahu wrote:
 Hi Vinod,
 
 On Thu, Aug 20, 2015 at 11:10 AM, Vinod Koul vinod.k...@intel.com wrote:
  On Thu, Jul 30, 2015 at 05:41:06PM +0530, Rameshwar Prasad Sahu wrote:
  + /* Invalidate unused source address field */
  + for (; i  4; i++)
  + xgene_dma_invalidate_buffer(xgene_dma_lookup_ext8(desc2, i));
  +
  + /* Check whether requested buffer processed */
  + if (nbytes) {
  + chan_err(chan, Src count crossed maximum limit\n);
  + return -EINVAL;
  no cleanup ?
 Here not required, cleanup I am doing in parent function from where
 this function is getting called in case of failure.
 
  +struct dma_async_tx_descriptor *xgene_dma_prep_flyby(
  + struct xgene_dma_chan *chan, struct scatterlist *src_sg,
  + size_t len, u32 seed, u8 *result, unsigned long flags, u8 opcode)
  please fix style here
 
 Could you explain me What kind of coding style you would like here ??

See CodingStyle Chapter 2

-- 
~Vinod

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/4] dmaengine: xgene-dma: Add support for CRC32C calculation via DMA engine

[Vinod Koul]

On Thu, Jul 30, 2015 at 05:41:06PM +0530, Rameshwar Prasad Sahu wrote:
 + /* Invalidate unused source address field */
 + for (; i  4; i++)
 + xgene_dma_invalidate_buffer(xgene_dma_lookup_ext8(desc2, i));
 +
 + /* Check whether requested buffer processed */
 + if (nbytes) {
 + chan_err(chan, Src count crossed maximum limit\n);
 + return -EINVAL;
no cleanup ?

 +struct dma_async_tx_descriptor *xgene_dma_prep_flyby(
 + struct xgene_dma_chan *chan, struct scatterlist *src_sg,
 + size_t len, u32 seed, u8 *result, unsigned long flags, u8 opcode)
please fix style here

 +struct dma_async_tx_descriptor *xgene_dma_prep_crc32c(
 + struct dma_chan *dchan, struct scatterlist *src_sg,
 + size_t len, u32 seed, u8 *result, unsigned long flags)
here too

 @@ -1309,8 +1512,13 @@ static void xgene_dma_setup_ring(struct xgene_dma_ring 
 *ring)
 ring-pdma-csr_ring + XGENE_DMA_RING_ID);
 
   /* Set DMA ring buffer */
 - iowrite32(XGENE_DMA_RING_ID_BUF_SETUP(ring-num),
 -   ring-pdma-csr_ring + XGENE_DMA_RING_ID_BUF);
 + ring_id_buf = XGENE_DMA_RING_ID_BUF_SETUP(ring-num);
 +
 + if (ring-is_bufpool)
 + ring_id_buf |= XGENE_DMA_RING_IS_BUFPOOL;
 +
 + iowrite32(ring_id_buf, ring-pdma-csr_ring +
 +   XGENE_DMA_RING_ID_BUF);
pls fix style here

-- 
~Vinod
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 3/4] Crypto: Add support for APM X-Gene SoC CRC32C h/w accelerator driver

[Vinod Koul]

On Thu, Jul 30, 2015 at 05:41:07PM +0530, Rameshwar Prasad Sahu wrote:
 + nents = sg_nents(req-src);
 + sg_count = dma_map_sg(dev, req-src, nents, DMA_TO_DEVICE);
 + if (!sg_count) {
 + dev_err(dev, Failed to map src sg);
 + return -ENOMEM;
mapping error shouldn't be no mem error

 + }
 +
 + if (sg_count  XGENE_DMA_MAX_FLYBY_SRC_CNT) {
 + dev_err(dev, Unsupported src sg len\n);
would be worth printing length

 + goto err;
 + }
 +
 + flags = DMA_CTRL_ACK;
why ACK?

 +
 + tx = dchan-device-device_prep_dma_crc32c(dchan, req-src,
 +req-nbytes,
 +reqctx-seed,
 +req-result,
 +flags);
We should add helper for this

-- 
~Vinod

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/13] dmaengine: Introduce dma_request_slave_channel_compat_reason()

[Vinod Koul]

On Mon, Jun 22, 2015 at 02:31:00PM +0300, Peter Ujfalusi wrote:
 On 06/12/2015 03:58 PM, Vinod Koul wrote:
  Sorry this slipped thru
 
 I was away for a week anyways ;)
 
  Thinking about it again, I think we should coverge to two APIs and mark the
  legacy depracuated and look to convert folks and phase that out
 
 Currently, w/o this series we have these APIs:
 /* to be used with DT/ACPI */
 dma_request_slave_channel(dev, name)  /* NULL on failure */
 dma_request_slave_channel_reason(dev, name)   /* error code on failure */
 
 /* Legacy mode only - no DT/ACPI lookup */
 dma_request_channel(mask, fn, fn_param) /* NULL on failure */
 
 /* to be used with DT/ACPI or legacy boot */
 dma_request_slave_channel_compat(mask, fn, fn_param, dev, name)   /* NULL 
 on
 failure */
 
 To request _any_ channel to be used for memcpy one has to use
 dma_request_channel(mask, NULL, NULL);
 
 If I did not missed something.
I dont think so :)

 As we need different types of parameters for DT/ACPI and legacy (non DT/ACPI
 lookup) and the good API names are already taken, we might need to settle:
 
 /* to be used with DT/ACPI */
 dma_request_slave_channel(dev, name) /* error code on failure */
 - Convert users to check IS_ERR_OR_NULL() instead against NULL
 - Mark dma_request_slave_channel_reason() deprecated and convert the current 
 users
 
 /* to be used with DT/ACPI or legacy boot */
 dma_request_slave_channel_compat(mask, fn, fn_param, dev, name) /* error code
 on failure */
 - Convert users to check IS_ERR_OR_NULL() instead against NULL
 - Do not try legacy mode if either OF or ACPI failed because of real error
Should we keep the filter fn and an API for this, I am still not too sure
about that part. Anyway users should be on DT/ACPI. if someone wants filter
then let them use dma_request_channel()

 
 /* Legacy mode only - no DT/ACPI lookup */
 dma_request_channel_legacy(mask, fn, fn_param) /* error code on failure */
 - convert users of dma_request_channel()
 - mark dma_request_channel() deprecated
Why should we create a new API, how about marking dma_request_channel() as
legacy and generic memcpy API and let other users be migrated?
 
 /* to be used to get a channel for memcpy for example */
 dma_request_any_channel(mask) /* error code on failure */
 - Convert current dma_request_channel(mask, NULL, NULL) users
 I know, any of the other function could be prepared to handle this when
 parameters are missing, but it is a bit cleaner to have separate API for this.
Though it has merits but adds another API. We cna have internal
_dma_request_xxx API where parameters are missing and clean but to users
single API might be a better idea
 
 It would be nice to find another name for the
 dma_request_slave_channel_compat() so with the new name we could have chance
 to rearrange the parameters: (dev, name, mask, fn, fn_param)
 
 We would end up with the following APIs, all returning with error code on 
 failure:
 dma_request_slave_channel(dev, name);
 dma_request_channel_legacy(mask, fn, fn_param);
 dma_request_slave_channel_compat(mask, fn, fn_param, dev, name);
 dma_request_any_channel(mask);
This is good idea but still we end up with 4 APIs. Why not just converge to
two API, one legacy + memcpy + filer fn and one untimate API for slave?

Internally we may have 4 APIs for cleaner handling...

Thoughts... ??

-- 
~Vinod
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/13] dmaengine: Introduce dma_request_slave_channel_compat_reason()

[Vinod Koul]

On Thu, Jun 04, 2015 at 06:58:06PM +0300, Peter Ujfalusi wrote:
 Vinod,
 
 On 06/02/2015 03:55 PM, Vinod Koul wrote:
  On Fri, May 29, 2015 at 05:32:50PM +0300, Peter Ujfalusi wrote:
  On 05/29/2015 01:18 PM, Vinod Koul wrote:
  On Fri, May 29, 2015 at 11:42:27AM +0200, Geert Uytterhoeven wrote:
  On Fri, May 29, 2015 at 11:33 AM, Vinod Koul vinod.k...@intel.com 
  wrote:
  On Tue, May 26, 2015 at 04:25:57PM +0300, Peter Ujfalusi wrote:
  dma_request_slave_channel_compat() 'eats' up the returned error codes 
  which
  prevents drivers using the compat call to be able to do deferred 
  probing.
 
  The new wrapper is identical in functionality but it will return with 
  error
  code in case of failure and will pass the -EPROBE_DEFER to the caller 
  in
  case dma_request_slave_channel_reason() returned with it.
  This is okay but am worried about one more warpper, how about fixing
  dma_request_slave_channel_compat()
 
  Then all callers of dma_request_slave_channel_compat() have to be
  modified to handle ERR_PTR first.
 
  The same is true for (the existing) dma_request_slave_channel_reason()
  vs. dma_request_slave_channel().
  Good point, looking again, I think we should rather fix
  dma_request_slave_channel_reason() as it was expected to return err code 
  and
  add new users. Anyway users of this API do expect the reason...
 
  Hrm, they are for different use.dma_request_slave_channel()/_reason() is 
  for
  drivers only working via DT or ACPI while
  dma_request_slave_channel_compat()/_reason() is for drivers expected to 
  run in
  DT/ACPI or legacy mode as well.
 
  I added the dma_request_slave_channel_compat_reason() because OMAP/daVinci
  drivers are using this to request channels - they need to support DT and
  legacy mode.
  I think we should hide these things behind the API and do this behind the
  hood for ACPI/DT systems.
  
  Also it makes sense to use right API and mark rest as depricated
 
 So to convert the dma_request_slave_channel_compat() and not to create _reason
 variant?
 
 Or to have single API to request channel? The problem with that is that we
 need different parameters for legacy and DT for example.
Sorry this slipped thru

Thinking about it again, I think we should coverge to two APIs and mark the
legacy depracuated and look to convert folks and phase that out


-- 
~Vinod
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/13] dmaengine: Introduce dma_request_slave_channel_compat_reason()

[Vinod Koul]

On Fri, May 29, 2015 at 05:32:50PM +0300, Peter Ujfalusi wrote:
 On 05/29/2015 01:18 PM, Vinod Koul wrote:
  On Fri, May 29, 2015 at 11:42:27AM +0200, Geert Uytterhoeven wrote:
  On Fri, May 29, 2015 at 11:33 AM, Vinod Koul vinod.k...@intel.com wrote:
  On Tue, May 26, 2015 at 04:25:57PM +0300, Peter Ujfalusi wrote:
  dma_request_slave_channel_compat() 'eats' up the returned error codes 
  which
  prevents drivers using the compat call to be able to do deferred probing.
 
  The new wrapper is identical in functionality but it will return with 
  error
  code in case of failure and will pass the -EPROBE_DEFER to the caller in
  case dma_request_slave_channel_reason() returned with it.
  This is okay but am worried about one more warpper, how about fixing
  dma_request_slave_channel_compat()
 
  Then all callers of dma_request_slave_channel_compat() have to be
  modified to handle ERR_PTR first.
 
  The same is true for (the existing) dma_request_slave_channel_reason()
  vs. dma_request_slave_channel().
  Good point, looking again, I think we should rather fix
  dma_request_slave_channel_reason() as it was expected to return err code and
  add new users. Anyway users of this API do expect the reason...
 
 Hrm, they are for different use.dma_request_slave_channel()/_reason() is for
 drivers only working via DT or ACPI while
 dma_request_slave_channel_compat()/_reason() is for drivers expected to run in
 DT/ACPI or legacy mode as well.
 
 I added the dma_request_slave_channel_compat_reason() because OMAP/daVinci
 drivers are using this to request channels - they need to support DT and
 legacy mode.
I think we should hide these things behind the API and do this behind the
hood for ACPI/DT systems.

Also it makes sense to use right API and mark rest as depricated
 
 But it is doable to do this for both the non _compat and _compat version:
 1. change all users to check IS_ERR_OR_NULL(chan)
  return the PTR_ERR if not NULL, or do whatever the driver was doing in case
 of chan == NULL.
 2. change the non _compat and _compat versions to do the same as the _reason
 variants, #define the _reason ones to the non _reason names
 3. Rename the _reason use to non _reason function in drivers
 4. Remove the #defines for the _reason functions
 5. Change the IS_ERR_OR_NULL(chan) to IS_ERR(chan) in all drivers
 The result:
 Both dma_request_slave_channel() and dma_request_slave_channel_compat() will
 return ERR_PTR in case of failure or in success they will return the pinter to
 chan.
 
 Is this what you were asking?
 It is a bit broader than what this series was doing: taking care of
 OMAP/daVinci drivers for deferred probing regarding to dmaengine ;)
Yes but it would make sense right? I know it is a larger work but then we
wouldn't want another dma_request_slave_xxx API, at some point we have stop
it exapnding, perhpas now :)

Yes I am all ears to stage this work and not do transition gardually..

-- 
~Vinod
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/13] dmaengine: Introduce dma_request_slave_channel_compat_reason()

[Vinod Koul]

On Tue, May 26, 2015 at 04:25:57PM +0300, Peter Ujfalusi wrote:
 dma_request_slave_channel_compat() 'eats' up the returned error codes which
 prevents drivers using the compat call to be able to do deferred probing.
 
 The new wrapper is identical in functionality but it will return with error
 code in case of failure and will pass the -EPROBE_DEFER to the caller in
 case dma_request_slave_channel_reason() returned with it.
This is okay but am worried about one more warpper, how about fixing
dma_request_slave_channel_compat()


-- 
~Vinod
 
 Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com
 ---
  include/linux/dmaengine.h | 22 ++
  1 file changed, 22 insertions(+)
 
 diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
 index abf63ceabef9..6c777394835c 100644
 --- a/include/linux/dmaengine.h
 +++ b/include/linux/dmaengine.h
 @@ -1120,4 +1120,26 @@ static inline struct dma_chan
  
   return __dma_request_channel(mask, fn, fn_param);
  }
 +
 +#define dma_request_slave_channel_compat_reason(mask, x, y, dev, name) \
 + __dma_request_slave_channel_compat_reason((mask), x, y, dev, name)
 +
 +static inline struct dma_chan
 +*__dma_request_slave_channel_compat_reason(const dma_cap_mask_t *mask,
 +   dma_filter_fn fn, void *fn_param,
 +   struct device *dev, char *name)
 +{
 + struct dma_chan *chan;
 +
 + chan = dma_request_slave_channel_reason(dev, name);
 + /* Try via legacy API if not requesting for deferred probing */
 + if (IS_ERR(chan)  PTR_ERR(chan) != -EPROBE_DEFER)
 + chan = __dma_request_channel(mask, fn, fn_param);
 +
 + if (!chan)
 + chan = ERR_PTR(-ENODEV);
 +
 + return chan;
 +}
 +
  #endif /* DMAENGINE_H */
 -- 
 2.3.5
 

-- 
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/13] dmaengine: Introduce dma_request_slave_channel_compat_reason()

[Vinod Koul]

On Fri, May 29, 2015 at 11:42:27AM +0200, Geert Uytterhoeven wrote:
 On Fri, May 29, 2015 at 11:33 AM, Vinod Koul vinod.k...@intel.com wrote:
  On Tue, May 26, 2015 at 04:25:57PM +0300, Peter Ujfalusi wrote:
  dma_request_slave_channel_compat() 'eats' up the returned error codes which
  prevents drivers using the compat call to be able to do deferred probing.
 
  The new wrapper is identical in functionality but it will return with error
  code in case of failure and will pass the -EPROBE_DEFER to the caller in
  case dma_request_slave_channel_reason() returned with it.
  This is okay but am worried about one more warpper, how about fixing
  dma_request_slave_channel_compat()
 
 Then all callers of dma_request_slave_channel_compat() have to be
 modified to handle ERR_PTR first.
 
 The same is true for (the existing) dma_request_slave_channel_reason()
 vs. dma_request_slave_channel().
Good point, looking again, I think we should rather fix
dma_request_slave_channel_reason() as it was expected to return err code and
add new users. Anyway users of this API do expect the reason...

-- 
~Vinod

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] powerpc: add explicit OF includes for ppc4xx

[Vinod Koul]

On Sun, Nov 10, 2013 at 11:35:43PM -0600, Rob Herring wrote:
 From: Rob Herring rob.herr...@calxeda.com
 
 Commit b5b4bb3f6a11f9 (of: only include prom.h on sparc) removed implicit
 includes of of_*.h headers by powerpc's prom.h. Some PPC4xx components
 were missed in initial clean-up patch, so add the necessary includes
 to fix ppc4xx builds.
 
 Signed-off-by: Rob Herring rob.herr...@calxeda.com
 Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
 Cc: Paul Mackerras pau...@samba.org
 Cc: Tejun Heo t...@kernel.org
 Cc: Matt Mackall m...@selenic.com
 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams dan.j.willi...@intel.com
 Cc: linuxppc-...@lists.ozlabs.org
 Cc: linux-...@vger.kernel.org
 Cc: linux-crypto@vger.kernel.org
 ---
 I intend to send this patch to Linus with the rest of the DT clean-up
 series.
 
 Rob
 
  arch/powerpc/sysdev/ppc4xx_ocm.c | 1 +
  arch/powerpc/sysdev/xilinx_intc.c| 1 +
  drivers/ata/sata_dwc_460ex.c | 2 ++
  drivers/char/hw_random/ppc4xx-rng.c  | 1 +
  drivers/crypto/amcc/crypto4xx_core.c | 3 +++
  drivers/dma/ppc4xx/adma.c| 2 ++
For this:

Acked-by Vinod Koul vinod.k...@intel.com

--
~Vinod
  6 files changed, 10 insertions(+)
 
 diff --git a/arch/powerpc/sysdev/ppc4xx_ocm.c 
 b/arch/powerpc/sysdev/ppc4xx_ocm.c
 index 1b15f93..b7c4345 100644
 --- a/arch/powerpc/sysdev/ppc4xx_ocm.c
 +++ b/arch/powerpc/sysdev/ppc4xx_ocm.c
 @@ -26,6 +26,7 @@
  #include linux/kernel.h
  #include linux/dma-mapping.h
  #include linux/of.h
 +#include linux/of_address.h
  #include asm/rheap.h
  #include asm/ppc4xx_ocm.h
  #include linux/slab.h
 diff --git a/arch/powerpc/sysdev/xilinx_intc.c 
 b/arch/powerpc/sysdev/xilinx_intc.c
 index f4fdc94..83f943a 100644
 --- a/arch/powerpc/sysdev/xilinx_intc.c
 +++ b/arch/powerpc/sysdev/xilinx_intc.c
 @@ -24,6 +24,7 @@
  #include linux/irq.h
  #include linux/of.h
  #include linux/of_address.h
 +#include linux/of_irq.h
  #include asm/io.h
  #include asm/processor.h
  #include asm/i8259.h
 diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
 index 2e39173..523524b 100644
 --- a/drivers/ata/sata_dwc_460ex.c
 +++ b/drivers/ata/sata_dwc_460ex.c
 @@ -31,6 +31,8 @@
  #include linux/module.h
  #include linux/init.h
  #include linux/device.h
 +#include linux/of_address.h
 +#include linux/of_irq.h
  #include linux/of_platform.h
  #include linux/platform_device.h
  #include linux/libata.h
 diff --git a/drivers/char/hw_random/ppc4xx-rng.c 
 b/drivers/char/hw_random/ppc4xx-rng.c
 index 732c330..521f76b 100644
 --- a/drivers/char/hw_random/ppc4xx-rng.c
 +++ b/drivers/char/hw_random/ppc4xx-rng.c
 @@ -13,6 +13,7 @@
  #include linux/platform_device.h
  #include linux/hw_random.h
  #include linux/delay.h
 +#include linux/of_address.h
  #include linux/of_platform.h
  #include asm/io.h
  
 diff --git a/drivers/crypto/amcc/crypto4xx_core.c 
 b/drivers/crypto/amcc/crypto4xx_core.c
 index f88e3d8..efaf630 100644
 --- a/drivers/crypto/amcc/crypto4xx_core.c
 +++ b/drivers/crypto/amcc/crypto4xx_core.c
 @@ -27,6 +27,9 @@
  #include linux/dma-mapping.h
  #include linux/platform_device.h
  #include linux/init.h
 +#include linux/module.h
 +#include linux/of_address.h
 +#include linux/of_irq.h
  #include linux/of_platform.h
  #include linux/slab.h
  #include asm/dcr.h
 diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
 index 370ff82..e24b5ef 100644
 --- a/drivers/dma/ppc4xx/adma.c
 +++ b/drivers/dma/ppc4xx/adma.c
 @@ -42,6 +42,8 @@
  #include linux/uaccess.h
  #include linux/proc_fs.h
  #include linux/of.h
 +#include linux/of_address.h
 +#include linux/of_irq.h
  #include linux/of_platform.h
  #include asm/dcr.h
  #include asm/dcr-regs.h
 -- 
 1.8.1.2
 

-- 
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH 23/51] DMA-API: dma: pl08x: add dma_set_mask_and_coherent() call

[Vinod Koul]

On Thu, Sep 19, 2013 at 10:48:01PM +0100, Russell King wrote:
 The DMA API requires drivers to call the appropriate dma_set_mask()
 functions before doing any DMA mapping.  Add this required call to
 the AMBA PL08x driver.
 
 Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
Acked-by: Vinod Koul vinod.k...@intel.com

~Vinod
 ---
  drivers/dma/amba-pl08x.c |5 +
  1 files changed, 5 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
 index fce46c5..e51a983 100644
 --- a/drivers/dma/amba-pl08x.c
 +++ b/drivers/dma/amba-pl08x.c
 @@ -2055,6 +2055,11 @@ static int pl08x_probe(struct amba_device *adev, const 
 struct amba_id *id)
   if (ret)
   return ret;
  
 + /* Ensure that we can do DMA */
 + ret = dma_set_mask_and_coherent(adev-dev, DMA_BIT_MASK(32));
 + if (ret)
 + goto out_no_pl08x;
 +
   /* Create the driver state holder */
   pl08x = kzalloc(sizeof(*pl08x), GFP_KERNEL);
   if (!pl08x) {
 -- 
 1.7.4.4
 
 ___
 Alsa-devel mailing list
 alsa-de...@alsa-project.org
 http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH 43/51] DMA-API: dma: edma.c: no need to explicitly initialize DMA masks

[Vinod Koul]

On Fri, Sep 20, 2013 at 12:15:39AM +0100, Russell King wrote:
 register_platform_device_full() can setup the DMA mask provided the
 appropriate member is set in struct platform_device_info.  So lets
 make that be the case.  This avoids a direct reference to the DMA
 masks by this driver.
 
 Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
Acked-by: Vinod Koul vinod.k...@intel.com

This also brings me question that should we force the driver to use the
dma_set_mask_and_coherent() API or they have below flexiblity too?

~Vinod

 ---
  drivers/dma/edma.c |6 ++
  1 files changed, 2 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
 index ff50ff4..7f9fe30 100644
 --- a/drivers/dma/edma.c
 +++ b/drivers/dma/edma.c
 @@ -702,11 +702,13 @@ static struct platform_device *pdev0, *pdev1;
  static const struct platform_device_info edma_dev_info0 = {
   .name = edma-dma-engine,
   .id = 0,
 + .dma_mask = DMA_BIT_MASK(32),
  };
  
  static const struct platform_device_info edma_dev_info1 = {
   .name = edma-dma-engine,
   .id = 1,
 + .dma_mask = DMA_BIT_MASK(32),
  };


  
  static int edma_init(void)
 @@ -720,8 +722,6 @@ static int edma_init(void)
   ret = PTR_ERR(pdev0);
   goto out;
   }
 - pdev0-dev.dma_mask = pdev0-dev.coherent_dma_mask;
 - pdev0-dev.coherent_dma_mask = DMA_BIT_MASK(32);
   }
  
   if (EDMA_CTLRS == 2) {
 @@ -731,8 +731,6 @@ static int edma_init(void)
   platform_device_unregister(pdev0);
   ret = PTR_ERR(pdev1);
   }
 - pdev1-dev.dma_mask = pdev1-dev.coherent_dma_mask;
 - pdev1-dev.coherent_dma_mask = DMA_BIT_MASK(32);
   }
  
  out:
 -- 
 1.7.4.4
 
 ___
 Alsa-devel mailing list
 alsa-de...@alsa-project.org
 http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH 24/51] DMA-API: dma: pl330: add dma_set_mask_and_coherent() call

[Vinod Koul]

On Sat, Sep 21, 2013 at 09:00:00PM +0100, Russell King - ARM Linux wrote:
 On Fri, Sep 20, 2013 at 07:26:27PM +0200, Heiko Stübner wrote:
  Am Donnerstag, 19. September 2013, 23:49:01 schrieb Russell King:
   The DMA API requires drivers to call the appropriate dma_set_mask()
   functions before doing any DMA mapping.  Add this required call to
   the AMBA PL08x driver.
  ^--- copy and paste error - should of course be PL330
 
 Fixed, thanks.
with fixed changelog...

Acked-by: Vinod Koul vinod.k...@intel.com

~Vinod

-- 
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 35/39] dmaengine: ste_dma40_ll: Replace meaningless register set with comment

[Vinod Koul]

On Thu, May 30, 2013 at 11:04:23AM +0200, Linus Walleij wrote:
 On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:
 
  Unsure of the author's intentions, rather than just removing the nop,
  we're replacing it with a comment containing the possible intention
  of the statement OR:ing with 0.
 
  Cc: Vinod Koul vinod.k...@intel.com
  Cc: Dan Williams d...@fb.com
  Cc: Per Forlin per.for...@stericsson.com
  Cc: Rabin Vincent ra...@rab.in
  Signed-off-by: Lee Jones lee.jo...@linaro.org
 
 Tentatively applied. Missing Vinod's ACK.
Acked-by: Vinod Koul vinod.k...@intel.com

--
~Vinod
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 39/39] dmaengine: ste_dma40: Fetch disabled channels from DT

[Vinod Koul]

On Wed, May 15, 2013 at 10:52:02AM +0100, Lee Jones wrote:
 Some platforms have channels which are not available for normal use.
 This information is currently passed though platform data in internal
 BSP kernels. Once those platforms land, they'll need to configure them
 appropriately, so we may as well add the infrastructure.
 
 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org
Acked-by: Vinod Koul vinod.k...@intel.com

 ---
  Documentation/devicetree/bindings/dma/ste-dma40.txt |2 ++
  drivers/dma/ste_dma40.c |   17 -
  2 files changed, 18 insertions(+), 1 deletion(-)
 
 diff --git a/Documentation/devicetree/bindings/dma/ste-dma40.txt 
 b/Documentation/devicetree/bindings/dma/ste-dma40.txt
 index aa272d8..bea5b73 100644
 --- a/Documentation/devicetree/bindings/dma/ste-dma40.txt
 +++ b/Documentation/devicetree/bindings/dma/ste-dma40.txt
 @@ -11,6 +11,7 @@ Required properties:
  Optional properties:
  - dma-channels: Number of channels supported by hardware - if not present
   the driver will attempt to obtain the information from H/W
 +- disabled-channels: Channels which can not be used
  
  Example:
  
 @@ -23,6 +24,7 @@ Example:
  
   #dma-cells = 2;
   memcpy-channels  = 56 57 58 59 60;
 + disabled-channels  = 12;
   dma-channels = 8;
   };
  
 diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
 index 4e528dd..ffac822 100644
 --- a/drivers/dma/ste_dma40.c
 +++ b/drivers/dma/ste_dma40.c
 @@ -3482,7 +3482,7 @@ static int __init d40_of_probe(struct platform_device 
 *pdev,
  struct device_node *np)
  {
   struct stedma40_platform_data *pdata;
 - int num_phy = 0, num_memcpy = 0;
 + int num_phy = 0, num_memcpy = 0, num_disabled = 0;
   const const __be32 *list;
  
   pdata = devm_kzalloc(pdev-dev,
 @@ -3511,6 +3511,21 @@ static int __init d40_of_probe(struct platform_device 
 *pdev,
  dma40_memcpy_channels,
  num_memcpy);
  
 + list = of_get_property(np, disabled-channels, num_disabled);
 + num_disabled /= sizeof(*list);
 +
 + if (num_disabled  STEDMA40_MAX_PHYS || num_disabled  0) {
 + d40_err(pdev-dev,
 + Invalid number of disabled channels specified (%d)\n,
 + num_disabled);
 + return -EINVAL;
 + }
 +
 + of_property_read_u32_array(np, disabled-channels,
 +pdata-disabled_channels,
 +num_disabled);
 + pdata-disabled_channels[num_disabled] = -1;
 +
   pdev-dev.platform_data = pdata;
  
   return 0;
 -- 
 1.7.10.4
 
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 34/39] dmaengine: ste_dma40: Convert data_width from register bit format to value

[Vinod Koul]

On Wed, May 15, 2013 at 10:51:57AM +0100, Lee Jones wrote:
 When a DMA client requests and configures a DMA channel, it requests
 data_width in Bytes. The DMA40 driver then swiftly converts it over to
 the necessary register bit value. Unfortunately, for any subsequent
 calculations we have to shift '1' by the bit pattern (1  data_width)
 times to make any sense of it.
 
 This patch flips the semantics on its head and only converts the value
 to its respective register bit pattern when writing to registers. This
 way we can use the true data_width (in Bytes) value.
 
 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---
  
 @@ -2804,14 +2781,24 @@ static int d40_set_runtime_config(struct dma_chan 
 *chan,
   src_maxburst = dst_maxburst * dst_addr_width / src_addr_width;
   }
  
 + /* Only valid widths are; 1, 2, 4 and 8. */
 + if (src_addr_width = DMA_SLAVE_BUSWIDTH_UNDEFINED ||
 + src_addr_width   DMA_SLAVE_BUSWIDTH_8_BYTES   ||
 + dst_addr_width = DMA_SLAVE_BUSWIDTH_UNDEFINED ||
 + dst_addr_width   DMA_SLAVE_BUSWIDTH_8_BYTES   ||
 + ((src_addr_width  1)  (src_addr_width  1)) ||
 + ((dst_addr_width  1)  (dst_addr_width  1)))
 + return -EINVAL;
how about a simple macro to check above..

 +
 + cfg-src_info.data_width = src_addr_width;
 + cfg-dst_info.data_width = dst_addr_width;
 +
   ret = dma40_config_to_halfchannel(d40c, cfg-src_info,
 -   src_addr_width,
 src_maxburst);
   if (ret)
   return ret;
  
   ret = dma40_config_to_halfchannel(d40c, cfg-dst_info,
 -   dst_addr_width,
 dst_maxburst);
   if (ret)
   return ret;
 diff --git a/drivers/dma/ste_dma40_ll.c b/drivers/dma/ste_dma40_ll.c
 index 5ddd724..a035dfe 100644
 --- a/drivers/dma/ste_dma40_ll.c
 +++ b/drivers/dma/ste_dma40_ll.c
 @@ -10,6 +10,18 @@
  
  #include ste_dma40_ll.h
  
 +u8 d40_width_to_bits(enum dma_slave_buswidth width)
 +{
 + if (width == DMA_SLAVE_BUSWIDTH_1_BYTE)
 + return STEDMA40_ESIZE_8_BIT;
 + else if (width == DMA_SLAVE_BUSWIDTH_2_BYTES)
 + return STEDMA40_ESIZE_16_BIT;
 + else if (width == DMA_SLAVE_BUSWIDTH_8_BYTES)
 + return STEDMA40_ESIZE_64_BIT;
 + else
 + return STEDMA40_ESIZE_32_BIT;
 +}
 +
Switch looks better for this and how about
return fls(width);

as your defines are 0...3 and dmaengine define 1,2,..8 for same thing
then you can also get rid of STEDMA40_XXX_WIDTH macros!


 @@ -70,13 +70,6 @@ enum stedma40_flow_ctrl {
   STEDMA40_FLOW_CTRL,
  };
  
 -enum stedma40_periph_data_width {
 - STEDMA40_BYTE_WIDTH = STEDMA40_ESIZE_8_BIT,
 - STEDMA40_HALFWORD_WIDTH = STEDMA40_ESIZE_16_BIT,
 - STEDMA40_WORD_WIDTH = STEDMA40_ESIZE_32_BIT,
 - STEDMA40_DOUBLEWORD_WIDTH = STEDMA40_ESIZE_64_BIT
 -};
nice

--
~Vinod
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 38/39] dmaengine: ste_dma40: Fetch the number of physical channels from DT

[Vinod Koul]

On Wed, May 15, 2013 at 10:52:01AM +0100, Lee Jones wrote:
 Some platforms insist on obscure physical channel availability. This
 information is currently passed though platform data in internal BSP
 kernels. Once those platforms land, they'll need to configure them
 appropriately, so we may as well add the infrastructure.
 
 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---
Acked-by: Vinod Koul vinod.k...@intel.com

  drivers/dma/ste_dma40.c |7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
 index ae462d3..4e528dd 100644
 --- a/drivers/dma/ste_dma40.c
 +++ b/drivers/dma/ste_dma40.c
 @@ -3482,7 +3482,7 @@ static int __init d40_of_probe(struct platform_device 
 *pdev,
  struct device_node *np)
  {
   struct stedma40_platform_data *pdata;
 - int num_memcpy = 0;
 + int num_phy = 0, num_memcpy = 0;
   const const __be32 *list;
  
   pdata = devm_kzalloc(pdev-dev,
 @@ -3491,6 +3491,11 @@ static int __init d40_of_probe(struct platform_device 
 *pdev,
   if (!pdata)
   return -ENOMEM;
  
 + /* If absent this value will be obtained from h/w. */
 + of_property_read_u32(np, dma-channels, num_phy);
 + if (num_phy  0)
 + pdata-num_of_phy_chans = num_phy;
 +
   list = of_get_property(np, memcpy-channels, num_memcpy);
   num_memcpy /= sizeof(*list);
  
 -- 
 1.7.10.4
 
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/39] dmaengine: ste_dma40: Separate Logical Global Interrupt Mask (GIM) unmasking

[Vinod Koul]

On Wed, May 15, 2013 at 06:29:51PM +0200, Linus Walleij wrote:
 On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:
 
  During the initial setup of a logical channel, it is necessary to unmask
  the GIM in order to receive generated terminal count and error interrupts.
  We're separating out this required code so it will be possible to move
  the remaining code in d40_phy_cfg(), which is mostly runtime configuration
  into the runtime_config() routine.
 
  Cc: Vinod Koul vinod.k...@intel.com
  Cc: Dan Williams d...@fb.com
  Cc: Per Forlin per.for...@stericsson.com
  Cc: Rabin Vincent ra...@rab.in
  Acked-by: Arnd Bergmann a...@arndb.de
  Signed-off-by: Lee Jones lee.jo...@linaro.org
 
 Tentatively applied to my ux500-dma40 branch.
 
 This lacks an ACK from Vinod...
Acked-by: Vinod Koul vinod.k...@intel.com
 
 I cannot get any of this stack of patches up to ARM SoC
 before I have Vinod's ACK on all hitting drivers/dma/*
Wip ... :)

 
 Yours,
 Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/39] dmaengine: ste_dma40: Remove unnecessary call to d40_phy_cfg()

[Vinod Koul]

On Wed, May 15, 2013 at 10:51:25AM +0100, Lee Jones wrote:
 All configuration left in d40_phy_cfg() is runtime configurable and
 there is already a call into it from d40_runtime_config(), so let's
 rely on that.
 
 Acked-by: Vinod Koul vnod.k...@intel.com
That needs up update!

 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---
  drivers/dma/ste_dma40.c|   14 +++---
  drivers/dma/ste_dma40_ll.c |  101 
 +---
  drivers/dma/ste_dma40_ll.h |3 +-
  3 files changed, 58 insertions(+), 60 deletions(-)
 
 diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
 index 759293e..b7fe46b 100644
 --- a/drivers/dma/ste_dma40.c
 +++ b/drivers/dma/ste_dma40.c
 @@ -2043,6 +2043,14 @@ static int d40_config_memcpy(struct d40_chan *d40c)
   } else if (dma_has_cap(DMA_MEMCPY, cap) 
  dma_has_cap(DMA_SLAVE, cap)) {
   d40c-dma_cfg = dma40_memcpy_conf_phy;
 +
 + /* Generate interrrupt at end of transfer or relink. */
 + d40c-dst_def_cfg |= BIT(D40_SREG_CFG_TIM_POS);
 +
 + /* Generate interrupt on error. */
 + d40c-src_def_cfg |= BIT(D40_SREG_CFG_EIM_POS);
 + d40c-dst_def_cfg |= BIT(D40_SREG_CFG_EIM_POS);
 +
   } else {
   chan_err(d40c, No memcpy\n);
   return -EINVAL;
 @@ -2496,9 +2504,6 @@ static int d40_alloc_chan_resources(struct dma_chan 
 *chan)
   }
  
   pm_runtime_get_sync(d40c-base-dev);
 - /* Fill in basic CFG register values */
 - d40_phy_cfg(d40c-dma_cfg, d40c-src_def_cfg,
 - d40c-dst_def_cfg, chan_is_logical(d40c));
  
   d40_set_prio_realtime(d40c);
  
 @@ -2862,8 +2867,7 @@ static int d40_set_runtime_config(struct dma_chan *chan,
   if (chan_is_logical(d40c))
   d40_log_cfg(cfg, d40c-log_def.lcsp1, d40c-log_def.lcsp3);
   else
 - d40_phy_cfg(cfg, d40c-src_def_cfg,
 - d40c-dst_def_cfg, false);
 + d40_phy_cfg(cfg, d40c-src_def_cfg, d40c-dst_def_cfg);
  
   /* These settings will take precedence later */
   d40c-runtime_addr = config_addr;
 diff --git a/drivers/dma/ste_dma40_ll.c b/drivers/dma/ste_dma40_ll.c
 index 435a223..ab5a2a7 100644
 --- a/drivers/dma/ste_dma40_ll.c
 +++ b/drivers/dma/ste_dma40_ll.c
 @@ -50,63 +50,58 @@ void d40_log_cfg(struct stedma40_chan_cfg *cfg,
  
  }
  
 -/* Sets up SRC and DST CFG register for both logical and physical channels */
 -void d40_phy_cfg(struct stedma40_chan_cfg *cfg,
 -  u32 *src_cfg, u32 *dst_cfg, bool is_log)
 +void d40_phy_cfg(struct stedma40_chan_cfg *cfg, u32 *src_cfg, u32 *dst_cfg)
  {
   u32 src = 0;
   u32 dst = 0;
  
 - if (!is_log) {
 - /* Physical channel */
 - if ((cfg-dir ==  STEDMA40_PERIPH_TO_MEM) ||
 - (cfg-dir == STEDMA40_PERIPH_TO_PERIPH)) {
 - /* Set master port to 1 */
 - src |= 1  D40_SREG_CFG_MST_POS;
 - src |= D40_TYPE_TO_EVENT(cfg-dev_type);
 -
 - if (cfg-src_info.flow_ctrl == STEDMA40_NO_FLOW_CTRL)
 - src |= 1  D40_SREG_CFG_PHY_TM_POS;
 - else
 - src |= 3  D40_SREG_CFG_PHY_TM_POS;
 - }
 - if ((cfg-dir ==  STEDMA40_MEM_TO_PERIPH) ||
 - (cfg-dir == STEDMA40_PERIPH_TO_PERIPH)) {
 - /* Set master port to 1 */
 - dst |= 1  D40_SREG_CFG_MST_POS;
 - dst |= D40_TYPE_TO_EVENT(cfg-dev_type);
 -
 - if (cfg-dst_info.flow_ctrl == STEDMA40_NO_FLOW_CTRL)
 - dst |= 1  D40_SREG_CFG_PHY_TM_POS;
 - else
 - dst |= 3  D40_SREG_CFG_PHY_TM_POS;
 - }
 - /* Interrupt on end of transfer for destination */
 - dst |= 1  D40_SREG_CFG_TIM_POS;
 -
 - /* Generate interrupt on error */
 - src |= 1  D40_SREG_CFG_EIM_POS;
 - dst |= 1  D40_SREG_CFG_EIM_POS;
 -
 - /* PSIZE */
 - if (cfg-src_info.psize != STEDMA40_PSIZE_PHY_1) {
 - src |= 1  D40_SREG_CFG_PHY_PEN_POS;
 - src |= cfg-src_info.psize  D40_SREG_CFG_PSIZE_POS;
 - }
 - if (cfg-dst_info.psize != STEDMA40_PSIZE_PHY_1) {
 - dst |= 1  D40_SREG_CFG_PHY_PEN_POS;
 - dst |= cfg-dst_info.psize  D40_SREG_CFG_PSIZE_POS;
 - }
 -
 - /* Element size */
 - src |= cfg-src_info.data_width  D40_SREG_CFG_ESIZE_POS;
 - dst |= cfg-dst_info.data_width  D40_SREG_CFG_ESIZE_POS;
 -
 - /* Set the priority bit to high for the physical channel */
 - if (cfg-high_priority) {
 - src |= 1  D40_SREG_CFG_PRI_POS;
 - dst

Re: [PATCH 03/39] dmaengine: ste_dma40: Don't configure runtime configurable setup during allocate

[Vinod Koul]

On Wed, May 15, 2013 at 10:51:26AM +0100, Lee Jones wrote:
 Using the dmaengine API, allocating and configuring a channel are two
 separate actions. Here we're removing logical channel configuration from
 the channel allocating routines.
 
 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---
Acked-by: Vinod Koul vinod.k...@intel.com

--
~Vinod
  drivers/dma/ste_dma40.c |6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
 index b7fe46b..ba84df8 100644
 --- a/drivers/dma/ste_dma40.c
 +++ b/drivers/dma/ste_dma40.c
 @@ -2040,6 +2040,9 @@ static int d40_config_memcpy(struct d40_chan *d40c)
   d40c-dma_cfg = dma40_memcpy_conf_log;
   d40c-dma_cfg.dev_type = 
 dma40_memcpy_channels[d40c-chan.chan_id];
  
 + d40_log_cfg(d40c-dma_cfg,
 + d40c-log_def.lcsp1, d40c-log_def.lcsp3);
 +
   } else if (dma_has_cap(DMA_MEMCPY, cap) 
  dma_has_cap(DMA_SLAVE, cap)) {
   d40c-dma_cfg = dma40_memcpy_conf_phy;
 @@ -2508,9 +2511,6 @@ static int d40_alloc_chan_resources(struct dma_chan 
 *chan)
   d40_set_prio_realtime(d40c);
  
   if (chan_is_logical(d40c)) {
 - d40_log_cfg(d40c-dma_cfg,
 - d40c-log_def.lcsp1, d40c-log_def.lcsp3);
 -
   if (d40c-dma_cfg.dir == STEDMA40_PERIPH_TO_MEM)
   d40c-lcpa = d40c-base-lcpa_base +
   d40c-dma_cfg.dev_type * D40_LCPA_CHAN_SIZE;
 -- 
 1.7.10.4
 
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/39] dmaengine: ste_dma40: Remove redundant address fetching function

[Vinod Koul]

On Wed, May 15, 2013 at 10:51:31AM +0100, Lee Jones wrote:
 Addresses are now stored in local data structures and are easy to
 obtain, thus a specialist function used to fetch them is now surplus
 to requirement.
 
 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---
Acked-by: Vinod Koul vinod.k...@intel.com

--
~Vinod
  drivers/dma/ste_dma40.c |   18 --
  1 file changed, 18 deletions(-)
 
 diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
 index 57a127e..6ed7757 100644
 --- a/drivers/dma/ste_dma40.c
 +++ b/drivers/dma/ste_dma40.c
 @@ -2267,24 +2267,6 @@ err:
   return NULL;
  }
  
 -static dma_addr_t
 -d40_get_dev_addr(struct d40_chan *chan, enum dma_transfer_direction 
 direction)
 -{
 - struct stedma40_platform_data *plat = chan-base-plat_data;
 - struct stedma40_chan_cfg *cfg = chan-dma_cfg;
 - dma_addr_t addr = 0;
 -
 - if (chan-runtime_addr)
 - return chan-runtime_addr;
 -
 - if (direction == DMA_DEV_TO_MEM)
 - addr = plat-dev_rx[cfg-dev_type];
 - else if (direction == DMA_MEM_TO_DEV)
 - addr = plat-dev_tx[cfg-dev_type];
 -
 - return addr;
 -}
 -
  static struct dma_async_tx_descriptor *
  d40_prep_sg(struct dma_chan *dchan, struct scatterlist *sg_src,
   struct scatterlist *sg_dst, unsigned int sg_len,
 -- 
 1.7.10.4
 
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/39] dmaengine: ste_dma40: Correct copy/paste error

[Vinod Koul]

On Wed, May 15, 2013 at 10:51:33AM +0100, Lee Jones wrote:
 'struct stedma40_half_channel_info's header comment says that it's
 called 'struct stedma40_chan_cfg'. Let's straighten that out.
 
 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---
Acked-by: Vinod Koul vinod.k...@intel.com

--
~Vinod
  include/linux/platform_data/dma-ste-dma40.h |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/include/linux/platform_data/dma-ste-dma40.h 
 b/include/linux/platform_data/dma-ste-dma40.h
 index af0064e..288dc24 100644
 --- a/include/linux/platform_data/dma-ste-dma40.h
 +++ b/include/linux/platform_data/dma-ste-dma40.h
 @@ -86,7 +86,7 @@ enum stedma40_xfer_dir {
  
  
  /**
 - * struct stedma40_chan_cfg - dst/src channel configuration
 + * struct stedma40_half_channel_info - dst/src channel configuration
   *
   * @big_endian: true if the src/dst should be read as big endian
   * @data_width: Data width of the src/dst hardware
 -- 
 1.7.10.4
 
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 31/39] dmaengine: ste_dma40: Replace ST-E's home-brew DMA direction defs with generic ones

[Vinod Koul]

On Thu, May 16, 2013 at 08:06:38AM +0100, Lee Jones wrote:
 On Thu, 16 May 2013, Vinod Koul wrote:
 
  On Wed, May 15, 2013 at 10:51:54AM +0100, Lee Jones wrote:
   STEDMA40_*_TO_* direction definitions are identical in all but name to
   the pre-defined generic DMA_*_TO_* ones. Let's make things easy by not
   duplicating such things.
   
   Cc: Vinod Koul vinod.k...@intel.com
   Cc: Dan Williams d...@fb.com
   Cc: Per Forlin per.for...@stericsson.com
   Cc: Rabin Vincent ra...@rab.in
   Signed-off-by: Lee Jones lee.jo...@linaro.org
  Nice :)
 
 :)
 
  1) I dont see the STE macro getting removed, why do we need it
 
 They are removed later in the set, once their use has been removed
 from platform code and all the other drivers.
 
  2) last i checked the direction values had a bit idfference b/w what you are
  using and what dmaengine defines, so hopefully that is taken care of..
 
 Yes, no problem.
 
 place Ack here ;)
Acked-by: Vinod Koul vinod.k...@intel.com

--
~Vinod
 
 -- 
 Lee Jones
 Linaro ST-Ericsson 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 linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/39] dmaengine: ste_dma40: Remove unnecessary call to d40_phy_cfg()

[Vinod Koul]

On Thu, May 16, 2013 at 08:25:57AM +0100, Lee Jones wrote:
 On Thu, 16 May 2013, Vinod Koul wrote:
 
  On Wed, May 15, 2013 at 10:51:25AM +0100, Lee Jones wrote:
   All configuration left in d40_phy_cfg() is runtime configurable and
   there is already a call into it from d40_runtime_config(), so let's
   rely on that.
   
   Acked-by: Vinod Koul vnod.k...@intel.com
  That needs up update!
 
 Ah, where did I get that from that?
 
 Was that my mistake, or was this in the MAINTAINERS file?
Certainly not in MAINTAINERS file :)

--
~Vinod
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 36/39] dmaengine: ste_dma40: Allow memcpy channels to be configured from DT

[Vinod Koul]

On Wed, May 15, 2013 at 10:51:59AM +0100, Lee Jones wrote:
 At this moment in time the memcpy channels which can be used by the D40
 are fixed, as each supported platform in Mainline uses the same ones.
 However, platforms do exist which don't follow this convention, so
 these will need to be tailored. Fortunately, these platforms will be DT
 only, so this change has very little impact on platform data.
 
 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org
Acked-by: Vinod Koul vinod.k...@intel.com

 ---
  .../devicetree/bindings/dma/ste-dma40.txt  |2 +
  drivers/dma/ste_dma40.c|   40 
 
  include/linux/platform_data/dma-ste-dma40.h|2 +
  3 files changed, 36 insertions(+), 8 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/dma/ste-dma40.txt 
 b/Documentation/devicetree/bindings/dma/ste-dma40.txt
 index 2679a87..aa272d8 100644
 --- a/Documentation/devicetree/bindings/dma/ste-dma40.txt
 +++ b/Documentation/devicetree/bindings/dma/ste-dma40.txt
 @@ -6,6 +6,7 @@ Required properties:
  - reg-names: Names of the above areas to use during resource look-up
  - interrupt: Should contain the DMAC interrupt number
  - #dma-cells: must be 3
 +- memcpy-channels: Channels to be used for memcpy
  
  Optional properties:
  - dma-channels: Number of channels supported by hardware - if not present
 @@ -21,6 +22,7 @@ Example:
   interrupts = 0 25 0x4;
  
   #dma-cells = 2;
 + memcpy-channels  = 56 57 58 59 60;
   dma-channels = 8;
   };
  
 diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
 index 76c255f..ae462d3 100644
 --- a/drivers/dma/ste_dma40.c
 +++ b/drivers/dma/ste_dma40.c
 @@ -58,6 +58,8 @@
  #define D40_ALLOC_PHYBIT(30)
  #define D40_ALLOC_LOG_FREE   0
  
 +#define D40_MEMCPY_MAX_CHANS 8
 +
  /* Reserved event lines for memcpy only. */
  #define DB8500_DMA_MEMCPY_EV_0   51
  #define DB8500_DMA_MEMCPY_EV_1   56
 @@ -522,6 +524,8 @@ struct d40_gen_dmac {
   * @phy_start: Physical memory start of the DMA registers.
   * @phy_size: Size of the DMA register map.
   * @irq: The IRQ number.
 + * @num_memcpy_chans: The number of channels used for memcpy (mem-to-mem
 + * transfers).
   * @num_phy_chans: The number of physical channels. Read from HW. This
   * is the number of available channels for this driver, not counting Secure
   * mode allocated physical channels.
 @@ -565,6 +569,7 @@ struct d40_base {
   phys_addr_t   phy_start;
   resource_size_t   phy_size;
   int   irq;
 + int   num_memcpy_chans;
   int   num_phy_chans;
   int   num_log_chans;
   struct device_dma_parameters  dma_parms;
 @@ -2938,7 +2943,7 @@ static int __init d40_dmaengine_init(struct d40_base 
 *base,
   }
  
   d40_chan_init(base, base-dma_memcpy, base-log_chans,
 -   base-num_log_chans, ARRAY_SIZE(dma40_memcpy_channels));
 +   base-num_log_chans, base-num_memcpy_chans);
  
   dma_cap_zero(base-dma_memcpy.cap_mask);
   dma_cap_set(DMA_MEMCPY, base-dma_memcpy.cap_mask);
 @@ -3139,6 +3144,7 @@ static struct d40_base * __init 
 d40_hw_detect_init(struct platform_device *pdev)
   struct d40_base *base = NULL;
   int num_log_chans = 0;
   int num_phy_chans;
 + int num_memcpy_chans;
   int clk_ret = -EINVAL;
   int i;
   u32 pid;
 @@ -3209,6 +3215,12 @@ static struct d40_base * __init 
 d40_hw_detect_init(struct platform_device *pdev)
   else
   num_phy_chans = 4 * (readl(virtbase + D40_DREG_ICFG)  0x7) + 4;
  
 + /* The number of channels used for memcpy */
 + if (plat_data-num_of_memcpy_chans)
 + num_memcpy_chans = plat_data-num_of_memcpy_chans;
 + else
 + num_memcpy_chans = ARRAY_SIZE(dma40_memcpy_channels);
 +
   num_log_chans = num_phy_chans * D40_MAX_LOG_CHAN_PER_PHY;
  
   dev_info(pdev-dev,
 @@ -3216,7 +3228,7 @@ static struct d40_base * __init 
 d40_hw_detect_init(struct platform_device *pdev)
rev, res-start, num_phy_chans, num_log_chans);
  
   base = kzalloc(ALIGN(sizeof(struct d40_base), 4) +
 -(num_phy_chans + num_log_chans + 
 ARRAY_SIZE(dma40_memcpy_channels)) *
 +(num_phy_chans + num_log_chans + num_memcpy_chans) *
  sizeof(struct d40_chan), GFP_KERNEL);
  
   if (base == NULL) {
 @@ -3226,6 +3238,7 @@ static struct d40_base * __init 
 d40_hw_detect_init(struct platform_device *pdev)
  
   base-rev = rev;
   base-clk = clk;
 + base-num_memcpy_chans = num_memcpy_chans;
   base

Re: [PATCH 33/39] dmaengine: ste_dma40_ll: Use the BIT macro to replace ugly '(1 x)'s

[Vinod Koul]

On Wed, May 15, 2013 at 10:51:56AM +0100, Lee Jones wrote:
 The aim is to make the code that little more readable.
 
 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org
Acked-by: Vinod Koul vinod.k...@intel.com

Hopefully all the BIT conversion in the driver are done in this

--
~Vinod
 ---
  drivers/dma/ste_dma40_ll.c |   44 
 ++--
  1 file changed, 22 insertions(+), 22 deletions(-)
 
 diff --git a/drivers/dma/ste_dma40_ll.c b/drivers/dma/ste_dma40_ll.c
 index 121c0ce..5ddd724 100644
 --- a/drivers/dma/ste_dma40_ll.c
 +++ b/drivers/dma/ste_dma40_ll.c
 @@ -20,28 +20,28 @@ void d40_log_cfg(struct stedma40_chan_cfg *cfg,
   /* src is mem? - increase address pos */
   if (cfg-dir ==  DMA_MEM_TO_DEV ||
   cfg-dir ==  DMA_MEM_TO_MEM)
 - l1 |= 1  D40_MEM_LCSP1_SCFG_INCR_POS;
 + l1 |= BIT(D40_MEM_LCSP1_SCFG_INCR_POS);
  
   /* dst is mem? - increase address pos */
   if (cfg-dir ==  DMA_DEV_TO_MEM ||
   cfg-dir ==  DMA_MEM_TO_MEM)
 - l3 |= 1  D40_MEM_LCSP3_DCFG_INCR_POS;
 + l3 |= BIT(D40_MEM_LCSP3_DCFG_INCR_POS);
  
   /* src is hw? - master port 1 */
   if (cfg-dir ==  DMA_DEV_TO_MEM ||
   cfg-dir ==  DMA_DEV_TO_DEV)
 - l1 |= 1  D40_MEM_LCSP1_SCFG_MST_POS;
 + l1 |= BIT(D40_MEM_LCSP1_SCFG_MST_POS);
  
   /* dst is hw? - master port 1 */
   if (cfg-dir ==  DMA_MEM_TO_DEV ||
   cfg-dir ==  DMA_DEV_TO_DEV)
 - l3 |= 1  D40_MEM_LCSP3_DCFG_MST_POS;
 + l3 |= BIT(D40_MEM_LCSP3_DCFG_MST_POS);
  
 - l3 |= 1  D40_MEM_LCSP3_DCFG_EIM_POS;
 + l3 |= BIT(D40_MEM_LCSP3_DCFG_EIM_POS);
   l3 |= cfg-dst_info.psize  D40_MEM_LCSP3_DCFG_PSIZE_POS;
   l3 |= cfg-dst_info.data_width  D40_MEM_LCSP3_DCFG_ESIZE_POS;
  
 - l1 |= 1  D40_MEM_LCSP1_SCFG_EIM_POS;
 + l1 |= BIT(D40_MEM_LCSP1_SCFG_EIM_POS);
   l1 |= cfg-src_info.psize  D40_MEM_LCSP1_SCFG_PSIZE_POS;
   l1 |= cfg-src_info.data_width  D40_MEM_LCSP1_SCFG_ESIZE_POS;
  
 @@ -58,39 +58,39 @@ void d40_phy_cfg(struct stedma40_chan_cfg *cfg, u32 
 *src_cfg, u32 *dst_cfg)
   if ((cfg-dir == DMA_DEV_TO_MEM) ||
   (cfg-dir == DMA_DEV_TO_DEV)) {
   /* Set master port to 1 */
 - src |= 1  D40_SREG_CFG_MST_POS;
 + src |= BIT(D40_SREG_CFG_MST_POS);
   src |= D40_TYPE_TO_EVENT(cfg-dev_type);
  
   if (cfg-src_info.flow_ctrl == STEDMA40_NO_FLOW_CTRL)
 - src |= 1  D40_SREG_CFG_PHY_TM_POS;
 + src |= BIT(D40_SREG_CFG_PHY_TM_POS);
   else
   src |= 3  D40_SREG_CFG_PHY_TM_POS;
   }
   if ((cfg-dir == DMA_MEM_TO_DEV) ||
   (cfg-dir == DMA_DEV_TO_DEV)) {
   /* Set master port to 1 */
 - dst |= 1  D40_SREG_CFG_MST_POS;
 + dst |= BIT(D40_SREG_CFG_MST_POS);
   dst |= D40_TYPE_TO_EVENT(cfg-dev_type);
  
   if (cfg-dst_info.flow_ctrl == STEDMA40_NO_FLOW_CTRL)
 - dst |= 1  D40_SREG_CFG_PHY_TM_POS;
 + dst |= BIT(D40_SREG_CFG_PHY_TM_POS);
   else
   dst |= 3  D40_SREG_CFG_PHY_TM_POS;
   }
   /* Interrupt on end of transfer for destination */
 - dst |= 1  D40_SREG_CFG_TIM_POS;
 + dst |= BIT(D40_SREG_CFG_TIM_POS);
  
   /* Generate interrupt on error */
 - src |= 1  D40_SREG_CFG_EIM_POS;
 - dst |= 1  D40_SREG_CFG_EIM_POS;
 + src |= BIT(D40_SREG_CFG_EIM_POS);
 + dst |= BIT(D40_SREG_CFG_EIM_POS);
  
   /* PSIZE */
   if (cfg-src_info.psize != STEDMA40_PSIZE_PHY_1) {
 - src |= 1  D40_SREG_CFG_PHY_PEN_POS;
 + src |= BIT(D40_SREG_CFG_PHY_PEN_POS);
   src |= cfg-src_info.psize  D40_SREG_CFG_PSIZE_POS;
   }
   if (cfg-dst_info.psize != STEDMA40_PSIZE_PHY_1) {
 - dst |= 1  D40_SREG_CFG_PHY_PEN_POS;
 + dst |= BIT(D40_SREG_CFG_PHY_PEN_POS);
   dst |= cfg-dst_info.psize  D40_SREG_CFG_PSIZE_POS;
   }
  
 @@ -100,14 +100,14 @@ void d40_phy_cfg(struct stedma40_chan_cfg *cfg, u32 
 *src_cfg, u32 *dst_cfg)
  
   /* Set the priority bit to high for the physical channel */
   if (cfg-high_priority) {
 - src |= 1  D40_SREG_CFG_PRI_POS;
 - dst |= 1  D40_SREG_CFG_PRI_POS;
 + src |= BIT(D40_SREG_CFG_PRI_POS);
 + dst |= BIT(D40_SREG_CFG_PRI_POS);
   }
  
   if (cfg-src_info.big_endian)
 - src |= 1  D40_SREG_CFG_LBE_POS;
 + src |= BIT(D40_SREG_CFG_LBE_POS);
   if (cfg-dst_info.big_endian)
 - dst |= 1  D40_SREG_CFG_LBE_POS;
 + dst |= BIT(D40_SREG_CFG_LBE_POS);
  
   *src_cfg = src;
   *dst_cfg = dst;
 @@ -157,15 +157,15 @@ static int

Re: [PATCH 31/39] dmaengine: ste_dma40: Replace ST-E's home-brew DMA direction defs with generic ones

[Vinod Koul]

On Wed, May 15, 2013 at 10:51:54AM +0100, Lee Jones wrote:
 STEDMA40_*_TO_* direction definitions are identical in all but name to
 the pre-defined generic DMA_*_TO_* ones. Let's make things easy by not
 duplicating such things.
 
 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org
Nice :)

1) I dont see the STE macro getting removed, why do we need it
2) last i checked the direction values had a bit idfference b/w what you are
using and what dmaengine defines, so hopefully that is taken care of..

--
~Vinod
 ---
  drivers/dma/ste_dma40.c|   56 
 ++--
  drivers/dma/ste_dma40_ll.c |   24 +--
  2 files changed, 40 insertions(+), 40 deletions(-)
 
 diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
 index 08bc58a..483da16 100644
 --- a/drivers/dma/ste_dma40.c
 +++ b/drivers/dma/ste_dma40.c
 @@ -78,7 +78,7 @@ static int dma40_memcpy_channels[] = {
  /* Default configuration for physcial memcpy */
  struct stedma40_chan_cfg dma40_memcpy_conf_phy = {
   .mode = STEDMA40_MODE_PHYSICAL,
 - .dir = STEDMA40_MEM_TO_MEM,
 + .dir = DMA_MEM_TO_MEM,
  
   .src_info.data_width = STEDMA40_BYTE_WIDTH,
   .src_info.psize = STEDMA40_PSIZE_PHY_1,
 @@ -92,7 +92,7 @@ struct stedma40_chan_cfg dma40_memcpy_conf_phy = {
  /* Default configuration for logical memcpy */
  struct stedma40_chan_cfg dma40_memcpy_conf_log = {
   .mode = STEDMA40_MODE_LOGICAL,
 - .dir = STEDMA40_MEM_TO_MEM,
 + .dir = DMA_MEM_TO_MEM,
  
   .src_info.data_width = STEDMA40_BYTE_WIDTH,
   .src_info.psize = STEDMA40_PSIZE_LOG_1,
 @@ -843,7 +843,7 @@ static void d40_log_lli_to_lcxa(struct d40_chan *chan, 
 struct d40_desc *desc)
* that uses linked lists.
*/
   if (!(chan-phy_chan-use_soft_lli 
 - chan-dma_cfg.dir == STEDMA40_PERIPH_TO_MEM))
 + chan-dma_cfg.dir == DMA_DEV_TO_MEM))
   curr_lcla = d40_lcla_alloc_one(chan, desc);
  
   first_lcla = curr_lcla;
 @@ -1311,12 +1311,12 @@ static void d40_config_set_event(struct d40_chan 
 *d40c,
   u32 event = D40_TYPE_TO_EVENT(d40c-dma_cfg.dev_type);
  
   /* Enable event line connected to device (or memcpy) */
 - if ((d40c-dma_cfg.dir ==  STEDMA40_PERIPH_TO_MEM) ||
 - (d40c-dma_cfg.dir == STEDMA40_PERIPH_TO_PERIPH))
 + if ((d40c-dma_cfg.dir == DMA_DEV_TO_MEM) ||
 + (d40c-dma_cfg.dir == DMA_DEV_TO_DEV))
   __d40_config_set_event(d40c, event_type, event,
  D40_CHAN_REG_SSLNK);
  
 - if (d40c-dma_cfg.dir !=  STEDMA40_PERIPH_TO_MEM)
 + if (d40c-dma_cfg.dir !=  DMA_DEV_TO_MEM)
   __d40_config_set_event(d40c, event_type, event,
  D40_CHAN_REG_SDLNK);
  }
 @@ -1774,7 +1774,7 @@ static int d40_validate_conf(struct d40_chan *d40c,
   res = -EINVAL;
   }
  
 - if (conf-dir == STEDMA40_PERIPH_TO_PERIPH) {
 + if (conf-dir == DMA_DEV_TO_DEV) {
   /*
* DMAC HW supports it. Will be added to this driver,
* in case any dma client requires it.
 @@ -1905,11 +1905,11 @@ static int d40_allocate_channel(struct d40_chan 
 *d40c, bool *first_phy_user)
   phys = d40c-base-phy_res;
   num_phy_chans = d40c-base-num_phy_chans;
  
 - if (d40c-dma_cfg.dir == STEDMA40_PERIPH_TO_MEM) {
 + if (d40c-dma_cfg.dir == DMA_DEV_TO_MEM) {
   log_num = 2 * dev_type;
   is_src = true;
 - } else if (d40c-dma_cfg.dir == STEDMA40_MEM_TO_PERIPH ||
 -d40c-dma_cfg.dir == STEDMA40_MEM_TO_MEM) {
 + } else if (d40c-dma_cfg.dir == DMA_MEM_TO_DEV ||
 +d40c-dma_cfg.dir == DMA_MEM_TO_MEM) {
   /* dst event lines are used for logical memcpy */
   log_num = 2 * dev_type + 1;
   is_src = false;
 @@ -1920,7 +1920,7 @@ static int d40_allocate_channel(struct d40_chan *d40c, 
 bool *first_phy_user)
   event_line = D40_TYPE_TO_EVENT(dev_type);
  
   if (!is_log) {
 - if (d40c-dma_cfg.dir == STEDMA40_MEM_TO_MEM) {
 + if (d40c-dma_cfg.dir == DMA_MEM_TO_MEM) {
   /* Find physical half channel */
   if (d40c-dma_cfg.use_fixed_channel) {
   i = d40c-dma_cfg.phy_channel;
 @@ -2068,10 +2068,10 @@ static int d40_free_dma(struct d40_chan *d40c)
   return -EINVAL;
   }
  
 - if (d40c-dma_cfg.dir == STEDMA40_MEM_TO_PERIPH ||
 - d40c-dma_cfg.dir == STEDMA40_MEM_TO_MEM)
 + if (d40c-dma_cfg.dir == DMA_MEM_TO_DEV ||
 + d40c-dma_cfg.dir == DMA_MEM_TO_MEM)
   is_src = false;
 - else if (d40c-dma_cfg.dir == STEDMA40_PERIPH_TO_MEM)
 + else if (d40c-dma_cfg.dir

Re: [PATCH 35/39] dmaengine: ste_dma40_ll: Replace meaningless register set with comment

[Vinod Koul]

On Wed, May 15, 2013 at 10:51:58AM +0100, Lee Jones wrote:
 Unsure of the author's intentions, rather than just removing the nop,
 we're replacing it with a comment containing the possible intention
 of the statement OR:ing with 0.
Would be worthwhile to check w/ Linus W on this (or check whom to blame)

--
~Vinod
 
 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---
  drivers/dma/ste_dma40_ll.c |6 --
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/dma/ste_dma40_ll.c b/drivers/dma/ste_dma40_ll.c
 index a035dfe..27b818d 100644
 --- a/drivers/dma/ste_dma40_ll.c
 +++ b/drivers/dma/ste_dma40_ll.c
 @@ -182,8 +182,10 @@ static int d40_phy_fill_lli(struct d40_phy_lli *lli,
   else
   lli-reg_cfg = ~BIT(D40_SREG_CFG_TIM_POS);
  
 - /* Post link */
 - lli-reg_lnk |= 0  D40_SREG_LNK_PHY_PRE_POS;
 + /*
 +  * Post link - D40_SREG_LNK_PHY_PRE_POS = 0
 +  * Relink happens after transfer completion.
 +  */
  
   return 0;
  }
 -- 
 1.7.10.4
 
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] crypto: ux500/cryp - Set DMA configuration though dma_slave_config()

[Vinod Koul]

On Fri, Apr 26, 2013 at 10:41:23AM +0100, Russell King - ARM Linux wrote:
 On Fri, Apr 26, 2013 at 01:46:46PM +0530, Vinod Koul wrote:
  On Fri, Apr 26, 2013 at 10:28:39AM +0200, Linus Walleij wrote:
   On Thu, Apr 25, 2013 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote:
   
The dma engine driver must know the address in its dma space, while the
slave driver has it available in physical space. These two are often the
same, but there is no generic way to convert between the two, especially
if the dma engine resides behind an IOMMU.
   
The best assumption we can make is that the dma engine driver knows
how to convert between the two. Interestingly the documentation for
dma_slave_config talks about physical address, while the structure
itself uses a dma_addr_t. Linus Walleij introduced the structure in
c156d0a5b0 DMAENGINE: generic slave channel control v3, so I assume
he can shed some light on what he was thinking. I assume the 
documentation
is right but the structure is not and should be converted to use
phys_add_t or resource_size_t.
   
   OK I could cook a patch for that, but I think I need some input from
   Vinod and/or Russell on this.
  the dma_slave_config is physical address that should be passed directly to 
  the
  controller. Obviosuly it should phys_addr_t :)
 
 What you've just said is actually confusing.
 
 physical address is normally the term used to describe the addresses
 seen to the RAM.  phys_addr_t describes this.  This is not necessarily
 what needs to be programmed into the DMA controller.
Yes that would be true when you have MMU
 
 For RAM addresses, they must be mapped via the DMA API - and this gives
 you a dma_addr_t.
 
 DMA address is the address to be programmed into a DMA controller to
 access a particular address in RAM or device, and has type dma_addr_t.
 When you're programming a DMA controller to access a device, you are
 clearly telling it the address on the _DMA controller's bus_ to access
 that register, which may or may not be the same as the physical address.
 
 There are platforms in existence where phys_addr_t can be 32-bit but
 dma_addr_t can be 64-bit.  Getting this stuff wrong can cause problems.
Sure, thanks for pointing, so we wont do this change.

--
~Vinod
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] crypto: ux500/cryp - Set DMA configuration though dma_slave_config()

[Vinod Koul]

On Fri, Apr 26, 2013 at 10:28:39AM +0200, Linus Walleij wrote:
 On Thu, Apr 25, 2013 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote:
 
  The dma engine driver must know the address in its dma space, while the
  slave driver has it available in physical space. These two are often the
  same, but there is no generic way to convert between the two, especially
  if the dma engine resides behind an IOMMU.
 
  The best assumption we can make is that the dma engine driver knows
  how to convert between the two. Interestingly the documentation for
  dma_slave_config talks about physical address, while the structure
  itself uses a dma_addr_t. Linus Walleij introduced the structure in
  c156d0a5b0 DMAENGINE: generic slave channel control v3, so I assume
  he can shed some light on what he was thinking. I assume the documentation
  is right but the structure is not and should be converted to use
  phys_add_t or resource_size_t.
 
 OK I could cook a patch for that, but I think I need some input from
 Vinod and/or Russell on this.
the dma_slave_config is physical address that should be passed directly to the
controller. Obviosuly it should phys_addr_t :)

The mapping  unmapping of dma buffer (memcpy and memory buffer in this txn) is
required to be performed by the client driver. The dmanegine or dmaengine driver
will not do that for you...

This is true for slave usage and not for async case.
 
 It does make sense to me that if anything knows how to map a physical
 address into the DMA address space it'll be the DMA engine.
 
 However this rings a bell that there may be a possible relation to
 DMA-API, since that API syncs memory buffers to the DMA
 address space if there is some MMU inbetween the DMA and the
 (ordinary, non-device) memory.
 
 So if we think one step ahead, assuming the DMAC is actually behind
 an MMU making it see the device in some other address than the
 physical (bus) space, where would the address be resolved?
 
 Yours,
 Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html