Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-09 Thread Sinan Kaya 



On 11/9/2015 1:19 PM, Rob Herring wrote:

On Sat, Nov 07, 2015 at 11:53:00PM -0500, Sinan Kaya wrote:

This patch adds support for hidma engine. The driver
consists of two logical blocks. The DMA engine interface
and the low-level interface. The hardware only supports
memcpy/memset and this driver only support memcpy
interface. HW and driver doesn't support slave interface.

Signed-off-by: Sinan Kaya 
---
  .../devicetree/bindings/dma/qcom_hidma.txt |  18 +
  drivers/dma/qcom/Kconfig   |   9 +
  drivers/dma/qcom/Makefile  |   2 +
  drivers/dma/qcom/hidma.c   | 743 
  drivers/dma/qcom/hidma.h   | 157 
  drivers/dma/qcom/hidma_dbg.c   | 225 +
  drivers/dma/qcom/hidma_ll.c| 944 +
  7 files changed, 2098 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/dma/qcom_hidma.txt
  create mode 100644 drivers/dma/qcom/hidma.c
  create mode 100644 drivers/dma/qcom/hidma.h
  create mode 100644 drivers/dma/qcom/hidma_dbg.c
  create mode 100644 drivers/dma/qcom/hidma_ll.c

diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma.txt 
b/Documentation/devicetree/bindings/dma/qcom_hidma.txt
new file mode 100644
index 000..c9fb2d44
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/qcom_hidma.txt
@@ -0,0 +1,18 @@
+Qualcomm Technologies HIDMA Channel driver
+
+Required properties:
+- compatible: must contain "qcom,hidma"


This should be "qcom,hidma-1.0" to match the example and driver. I
would drop "qcom,hidma" altogether.


I matched it.



Rob



--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-09 Thread Rob Herring 
On Sat, Nov 07, 2015 at 11:53:00PM -0500, Sinan Kaya wrote:
> This patch adds support for hidma engine. The driver
> consists of two logical blocks. The DMA engine interface
> and the low-level interface. The hardware only supports
> memcpy/memset and this driver only support memcpy
> interface. HW and driver doesn't support slave interface.
> 
> Signed-off-by: Sinan Kaya 
> ---
>  .../devicetree/bindings/dma/qcom_hidma.txt |  18 +
>  drivers/dma/qcom/Kconfig   |   9 +
>  drivers/dma/qcom/Makefile  |   2 +
>  drivers/dma/qcom/hidma.c   | 743 
>  drivers/dma/qcom/hidma.h   | 157 
>  drivers/dma/qcom/hidma_dbg.c   | 225 +
>  drivers/dma/qcom/hidma_ll.c| 944 
> +
>  7 files changed, 2098 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/qcom_hidma.txt
>  create mode 100644 drivers/dma/qcom/hidma.c
>  create mode 100644 drivers/dma/qcom/hidma.h
>  create mode 100644 drivers/dma/qcom/hidma_dbg.c
>  create mode 100644 drivers/dma/qcom/hidma_ll.c
> 
> diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma.txt 
> b/Documentation/devicetree/bindings/dma/qcom_hidma.txt
> new file mode 100644
> index 000..c9fb2d44
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/qcom_hidma.txt
> @@ -0,0 +1,18 @@
> +Qualcomm Technologies HIDMA Channel driver
> +
> +Required properties:
> +- compatible: must contain "qcom,hidma"

This should be "qcom,hidma-1.0" to match the example and driver. I 
would drop "qcom,hidma" altogether.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-09 Thread Rob Herring 
On Sat, Nov 07, 2015 at 11:53:00PM -0500, Sinan Kaya wrote:
> This patch adds support for hidma engine. The driver
> consists of two logical blocks. The DMA engine interface
> and the low-level interface. The hardware only supports
> memcpy/memset and this driver only support memcpy
> interface. HW and driver doesn't support slave interface.
> 
> Signed-off-by: Sinan Kaya 
> ---
>  .../devicetree/bindings/dma/qcom_hidma.txt |  18 +
>  drivers/dma/qcom/Kconfig   |   9 +
>  drivers/dma/qcom/Makefile  |   2 +
>  drivers/dma/qcom/hidma.c   | 743 
>  drivers/dma/qcom/hidma.h   | 157 
>  drivers/dma/qcom/hidma_dbg.c   | 225 +
>  drivers/dma/qcom/hidma_ll.c| 944 
> +
>  7 files changed, 2098 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/qcom_hidma.txt
>  create mode 100644 drivers/dma/qcom/hidma.c
>  create mode 100644 drivers/dma/qcom/hidma.h
>  create mode 100644 drivers/dma/qcom/hidma_dbg.c
>  create mode 100644 drivers/dma/qcom/hidma_ll.c
> 
> diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma.txt 
> b/Documentation/devicetree/bindings/dma/qcom_hidma.txt
> new file mode 100644
> index 000..c9fb2d44
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/qcom_hidma.txt
> @@ -0,0 +1,18 @@
> +Qualcomm Technologies HIDMA Channel driver
> +
> +Required properties:
> +- compatible: must contain "qcom,hidma"

This should be "qcom,hidma-1.0" to match the example and driver. I 
would drop "qcom,hidma" altogether.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-09 Thread Sinan Kaya 



On 11/9/2015 1:19 PM, Rob Herring wrote:

On Sat, Nov 07, 2015 at 11:53:00PM -0500, Sinan Kaya wrote:

This patch adds support for hidma engine. The driver
consists of two logical blocks. The DMA engine interface
and the low-level interface. The hardware only supports
memcpy/memset and this driver only support memcpy
interface. HW and driver doesn't support slave interface.

Signed-off-by: Sinan Kaya 
---
  .../devicetree/bindings/dma/qcom_hidma.txt |  18 +
  drivers/dma/qcom/Kconfig   |   9 +
  drivers/dma/qcom/Makefile  |   2 +
  drivers/dma/qcom/hidma.c   | 743 
  drivers/dma/qcom/hidma.h   | 157 
  drivers/dma/qcom/hidma_dbg.c   | 225 +
  drivers/dma/qcom/hidma_ll.c| 944 +
  7 files changed, 2098 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/dma/qcom_hidma.txt
  create mode 100644 drivers/dma/qcom/hidma.c
  create mode 100644 drivers/dma/qcom/hidma.h
  create mode 100644 drivers/dma/qcom/hidma_dbg.c
  create mode 100644 drivers/dma/qcom/hidma_ll.c

diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma.txt 
b/Documentation/devicetree/bindings/dma/qcom_hidma.txt
new file mode 100644
index 000..c9fb2d44
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/qcom_hidma.txt
@@ -0,0 +1,18 @@
+Qualcomm Technologies HIDMA Channel driver
+
+Required properties:
+- compatible: must contain "qcom,hidma"


This should be "qcom,hidma-1.0" to match the example and driver. I
would drop "qcom,hidma" altogether.


I matched it.



Rob



--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread Sinan Kaya 



On 11/8/2015 2:13 PM, kbuild test robot wrote:

Hi Sinan,

[auto build test WARNING on: robh/for-next]
[also build test WARNING on: v4.3 next-20151106]

url:
https://github.com/0day-ci/linux/commits/Sinan-Kaya/ma-add-Qualcomm-Technologies-HIDMA-driver/20151108-125824
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux for-next
config: mn10300-allyesconfig (attached as .config)
reproduce:
 wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
 chmod +x ~/bin/make.cross
 # save the attached .config to linux build tree
 make.cross ARCH=mn10300

All warnings (new ones prefixed by >>):

In file included from include/linux/printk.h:277:0,
 from include/linux/kernel.h:13,
 from include/linux/list.h:8,
 from include/linux/kobject.h:20,
 from include/linux/device.h:17,
 from include/linux/dmaengine.h:20,
 from drivers/dma/qcom/hidma.c:45:
drivers/dma/qcom/hidma.c: In function 'hidma_prep_dma_memcpy':
include/linux/dynamic_debug.h:64:16: warning: format '%zu' expects argument 
of type 'size_t', but argument 7 has type 'unsigned int' [-Wformat=]
  static struct _ddebug  __aligned(8)   \
^
include/linux/dynamic_debug.h:84:2: note: in expansion of macro 
'DEFINE_DYNAMIC_DEBUG_METADATA'
  DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);  \
  ^
include/linux/device.h:1171:2: note: in expansion of macro 'dynamic_dev_dbg'
  dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
  ^

drivers/dma/qcom/hidma.c:391:2: note: in expansion of macro 'dev_dbg'

  dev_dbg(mdma->ddev.dev,
  ^

vim +/dev_dbg +391 drivers/dma/qcom/hidma.c

375 
376 mchan->allocated = 0;
377 spin_unlock_irqrestore(>lock, irqflags);
378 dev_dbg(mdma->ddev.dev, "freed channel for %u\n", 
mchan->dma_sig);
379 }
380 
381 
382 static struct dma_async_tx_descriptor *
383 hidma_prep_dma_memcpy(struct dma_chan *dmach, dma_addr_t dma_dest,
384 dma_addr_t dma_src, size_t len, unsigned long 
flags)
385 {
386 struct hidma_chan *mchan = to_hidma_chan(dmach);
387 struct hidma_desc *mdesc = NULL;
388 struct hidma_dev *mdma = mchan->dmadev;
389 unsigned long irqflags;
390 
  > 391  dev_dbg(mdma->ddev.dev,
392 "memcpy: chan:%p dest:%pad src:%pad len:%zu\n", mchan,
393 _dest, _src, len);
394 


What am I missing?

len is size_t. This page says use %zu for size_t.

https://www.kernel.org/doc/Documentation/printk-formats.txt




395 /* Get free descriptor */
396 spin_lock_irqsave(>lock, irqflags);
397 if (!list_empty(>free)) {
398 mdesc = list_first_entry(>free, struct 
hidma_desc,
399 node);

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





--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread Sinan Kaya 



On 11/8/2015 3:47 PM, Andy Shevchenko wrote:

On Sun, Nov 8, 2015 at 6:53 AM, Sinan Kaya  wrote:

This patch adds support for hidma engine. The driver
consists of two logical blocks. The DMA engine interface
and the low-level interface. The hardware only supports
memcpy/memset and this driver only support memcpy
interface. HW and driver doesn't support slave interface.


Make lines a bit longer.



OK


+   pm_runtime_set_autosuspend_delay(>dev, AUTOSUSPEND_TIMEOUT);
+   pm_runtime_use_autosuspend(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
+   trca_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);



+   if (!trca_resource) {
+   rc = -ENODEV;
+   goto bailout;
+   }


Why did you ignore my comment about this block?
Remove that condition entirely.


Removed these four lines above.


+
+   trca = devm_ioremap_resource(>dev, trca_resource);
+   if (IS_ERR(trca)) {
+   rc = -ENOMEM;
+   goto bailout;
+   }
+
+   evca_resource = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+   if (!evca_resource) {
+   rc = -ENODEV;
+   goto bailout;
+   }


Ditto.



done



+uninit:
+   hidma_debug_uninit(dmadev);
+   hidma_ll_uninit(dmadev->lldev);
+dmafree:
+   if (dmadev)
+   hidma_free(dmadev);
+bailout:
+   pm_runtime_disable(>dev);
+   pm_runtime_put_sync_suspend(>dev);


Are you sure this is appropriate sequence?

I think

pm_runtime_put();
pm_runtime_disable();


corrected, reordered and used pm_runtime_put_sync() instead.


will do the job.


+   return rc;
+}
+
+static int hidma_remove(struct platform_device *pdev)
+{
+   struct hidma_dev *dmadev = platform_get_drvdata(pdev);
+
+   dev_dbg(>dev, "removing\n");


Useless message.


Removed.


+   pm_runtime_get_sync(dmadev->ddev.dev);
+
+   dma_async_device_unregister(>ddev);
+   hidma_debug_uninit(dmadev);
+   hidma_ll_uninit(dmadev->lldev);
+   hidma_free(dmadev);
+
+   dev_info(>dev, "HI-DMA engine removed\n");
+   pm_runtime_put_sync_suspend(>dev);
+   pm_runtime_disable(>dev);
+
+   return 0;
+}




--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread Andy Shevchenko 
On Sun, Nov 8, 2015 at 11:51 PM, Sinan Kaya  wrote:
>
>
> On 11/8/2015 3:47 PM, Andy Shevchenko wrote:
>>>
>>> +   trca_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> >+   if (!trca_resource) {
>>> >+   rc = -ENODEV;
>>> >+   goto bailout;
>>> >+   }
>>
>> Why did you ignore my comment about this block?
>> Remove that condition entirely.
>>
>>> >+
>>> >+   trca = devm_ioremap_resource(>dev, trca_resource);
>>> >+   if (IS_ERR(trca)) {
>>> >+   rc = -ENOMEM;
>>> >+   goto bailout;
>>> >+   }
>
>
> Sorry, I didn't quite get your comment. I thought you wanted to see
> platform_get_resource and devm_ioremap_resource together.
>
> Which one do you want me to remove?

At the end you would have something like

res = platform_get_resource();
addr = devm_ioremap_resources();
if (!addr) {
…
}

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread Sinan Kaya 



On 11/8/2015 3:47 PM, Andy Shevchenko wrote:

+   trca_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>+   if (!trca_resource) {
>+   rc = -ENODEV;
>+   goto bailout;
>+   }

Why did you ignore my comment about this block?
Remove that condition entirely.


>+
>+   trca = devm_ioremap_resource(>dev, trca_resource);
>+   if (IS_ERR(trca)) {
>+   rc = -ENOMEM;
>+   goto bailout;
>+   }


Sorry, I didn't quite get your comment. I thought you wanted to see 
platform_get_resource and devm_ioremap_resource together.


Which one do you want me to remove?

--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread Andy Shevchenko 
On Sun, Nov 8, 2015 at 6:53 AM, Sinan Kaya  wrote:
> This patch adds support for hidma engine. The driver
> consists of two logical blocks. The DMA engine interface
> and the low-level interface. The hardware only supports
> memcpy/memset and this driver only support memcpy
> interface. HW and driver doesn't support slave interface.

Make lines a bit longer.

> +/*
> + * Qualcomm Technologies HIDMA DMA engine interface
> + *
> + * Copyright (c) 2015, 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.
> + */
> +
> +/*
> + * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
> + * Copyright (C) Semihalf 2009
> + * Copyright (C) Ilya Yanok, Emcraft Systems 2010
> + * Copyright (C) Alexander Popov, Promcontroller 2014
> + *
> + * Written by Piotr Ziecik . Hardware description
> + * (defines, structures and comments) was taken from MPC5121 DMA driver
> + * written by Hongjun Chen .
> + *
> + * Approved as OSADL project by a majority of OSADL members and funded
> + * by OSADL membership fees in 2009;  for details see www.osadl.org.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + * 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.
> + *
> + * The full GNU General Public License is included in this distribution in 
> the
> + * file called COPYING.
> + */
> +
> +/* Linux Foundation elects GPLv2 license only. */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "../dmaengine.h"
> +#include "hidma.h"
> +
> +/*
> + * Default idle time is 2 seconds. This parameter can
> + * be overridden by changing the following
> + * /sys/bus/platform/devices/QCOM8061:/power/autosuspend_delay_ms
> + * during kernel boot.
> + */
> +#define AUTOSUSPEND_TIMEOUT2000
> +#define ERR_INFO_SW0xFF
> +#define ERR_CODE_UNEXPECTED_TERMINATE  0x0
> +
> +static inline
> +struct hidma_dev *to_hidma_dev(struct dma_device *dmadev)
> +{
> +   return container_of(dmadev, struct hidma_dev, ddev);
> +}
> +
> +static inline
> +struct hidma_dev *to_hidma_dev_from_lldev(struct hidma_lldev **_lldevp)
> +{
> +   return container_of(_lldevp, struct hidma_dev, lldev);
> +}
> +
> +static inline
> +struct hidma_chan *to_hidma_chan(struct dma_chan *dmach)
> +{
> +   return container_of(dmach, struct hidma_chan, chan);
> +}
> +
> +static inline struct hidma_desc *
> +to_hidma_desc(struct dma_async_tx_descriptor *t)
> +{
> +   return container_of(t, struct hidma_desc, desc);
> +}
> +
> +static void hidma_free(struct hidma_dev *dmadev)
> +{
> +   dev_dbg(dmadev->ddev.dev, "free dmadev\n");
> +   INIT_LIST_HEAD(>ddev.channels);
> +}
> +
> +static unsigned int nr_desc_prm;
> +module_param(nr_desc_prm, uint, 0644);
> +MODULE_PARM_DESC(nr_desc_prm,
> +"number of descriptors (default: 0)");
> +
> +#define MAX_HIDMA_CHANNELS 64
> +static int event_channel_idx[MAX_HIDMA_CHANNELS] = {
> +   [0 ... (MAX_HIDMA_CHANNELS - 1)] = -1};
> +static unsigned int num_event_channel_idx;
> +module_param_array_named(event_channel_idx, event_channel_idx, int,
> +   _event_channel_idx, 0644);
> +MODULE_PARM_DESC(event_channel_idx,
> +   "event channel index array for the notifications");
> +static atomic_t channel_ref_count;
> +
> +/* process completed descriptors */
> +static void hidma_process_completed(struct hidma_dev *mdma)
> +{
> +   dma_cookie_t last_cookie = 0;
> +   struct hidma_chan *mchan;
> +   struct hidma_desc *mdesc;
> +   struct dma_async_tx_descriptor *desc;
> +   unsigned long irqflags;
> +   struct list_head list;
> +   struct dma_chan *dmach = NULL;

Redundant assignment.

> +
> +   list_for_each_entry(dmach, >ddev.channels,
> +   device_node) {
> +   mchan = to_hidma_chan(dmach);
> +   INIT_LIST_HEAD();
> +
> +   /* Get all completed

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread kbuild test robot 
Hi Sinan,

[auto build test WARNING on: robh/for-next]
[also build test WARNING on: v4.3 next-20151106]

url:
https://github.com/0day-ci/linux/commits/Sinan-Kaya/ma-add-Qualcomm-Technologies-HIDMA-driver/20151108-125824
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux for-next
config: mn10300-allyesconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mn10300 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:277:0,
from include/linux/kernel.h:13,
from include/linux/list.h:8,
from include/linux/kobject.h:20,
from include/linux/device.h:17,
from include/linux/dmaengine.h:20,
from drivers/dma/qcom/hidma.c:45:
   drivers/dma/qcom/hidma.c: In function 'hidma_prep_dma_memcpy':
   include/linux/dynamic_debug.h:64:16: warning: format '%zu' expects argument 
of type 'size_t', but argument 7 has type 'unsigned int' [-Wformat=]
 static struct _ddebug  __aligned(8)   \
   ^
   include/linux/dynamic_debug.h:84:2: note: in expansion of macro 
'DEFINE_DYNAMIC_DEBUG_METADATA'
 DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);  \
 ^
   include/linux/device.h:1171:2: note: in expansion of macro 'dynamic_dev_dbg'
 dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
 ^
>> drivers/dma/qcom/hidma.c:391:2: note: in expansion of macro 'dev_dbg'
 dev_dbg(mdma->ddev.dev,
 ^

vim +/dev_dbg +391 drivers/dma/qcom/hidma.c

   375  
   376  mchan->allocated = 0;
   377  spin_unlock_irqrestore(>lock, irqflags);
   378  dev_dbg(mdma->ddev.dev, "freed channel for %u\n", 
mchan->dma_sig);
   379  }
   380  
   381  
   382  static struct dma_async_tx_descriptor *
   383  hidma_prep_dma_memcpy(struct dma_chan *dmach, dma_addr_t dma_dest,
   384  dma_addr_t dma_src, size_t len, unsigned long 
flags)
   385  {
   386  struct hidma_chan *mchan = to_hidma_chan(dmach);
   387  struct hidma_desc *mdesc = NULL;
   388  struct hidma_dev *mdma = mchan->dmadev;
   389  unsigned long irqflags;
   390  
 > 391  dev_dbg(mdma->ddev.dev,
   392  "memcpy: chan:%p dest:%pad src:%pad len:%zu\n", mchan,
   393  _dest, _src, len);
   394  
   395  /* Get free descriptor */
   396  spin_lock_irqsave(>lock, irqflags);
   397  if (!list_empty(>free)) {
   398  mdesc = list_first_entry(>free, struct 
hidma_desc,
   399  node);

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


.config.gz
Description: Binary data

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread kbuild test robot 
Hi Sinan,

[auto build test WARNING on: robh/for-next]
[also build test WARNING on: v4.3 next-20151106]

url:
https://github.com/0day-ci/linux/commits/Sinan-Kaya/ma-add-Qualcomm-Technologies-HIDMA-driver/20151108-125824
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux for-next
config: mn10300-allyesconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mn10300 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:277:0,
from include/linux/kernel.h:13,
from include/linux/list.h:8,
from include/linux/kobject.h:20,
from include/linux/device.h:17,
from include/linux/dmaengine.h:20,
from drivers/dma/qcom/hidma.c:45:
   drivers/dma/qcom/hidma.c: In function 'hidma_prep_dma_memcpy':
   include/linux/dynamic_debug.h:64:16: warning: format '%zu' expects argument 
of type 'size_t', but argument 7 has type 'unsigned int' [-Wformat=]
 static struct _ddebug  __aligned(8)   \
   ^
   include/linux/dynamic_debug.h:84:2: note: in expansion of macro 
'DEFINE_DYNAMIC_DEBUG_METADATA'
 DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);  \
 ^
   include/linux/device.h:1171:2: note: in expansion of macro 'dynamic_dev_dbg'
 dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
 ^
>> drivers/dma/qcom/hidma.c:391:2: note: in expansion of macro 'dev_dbg'
 dev_dbg(mdma->ddev.dev,
 ^

vim +/dev_dbg +391 drivers/dma/qcom/hidma.c

   375  
   376  mchan->allocated = 0;
   377  spin_unlock_irqrestore(>lock, irqflags);
   378  dev_dbg(mdma->ddev.dev, "freed channel for %u\n", 
mchan->dma_sig);
   379  }
   380  
   381  
   382  static struct dma_async_tx_descriptor *
   383  hidma_prep_dma_memcpy(struct dma_chan *dmach, dma_addr_t dma_dest,
   384  dma_addr_t dma_src, size_t len, unsigned long 
flags)
   385  {
   386  struct hidma_chan *mchan = to_hidma_chan(dmach);
   387  struct hidma_desc *mdesc = NULL;
   388  struct hidma_dev *mdma = mchan->dmadev;
   389  unsigned long irqflags;
   390  
 > 391  dev_dbg(mdma->ddev.dev,
   392  "memcpy: chan:%p dest:%pad src:%pad len:%zu\n", mchan,
   393  _dest, _src, len);
   394  
   395  /* Get free descriptor */
   396  spin_lock_irqsave(>lock, irqflags);
   397  if (!list_empty(>free)) {
   398  mdesc = list_first_entry(>free, struct 
hidma_desc,
   399  node);

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


.config.gz
Description: Binary data

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread Andy Shevchenko 
On Sun, Nov 8, 2015 at 6:53 AM, Sinan Kaya  wrote:
> This patch adds support for hidma engine. The driver
> consists of two logical blocks. The DMA engine interface
> and the low-level interface. The hardware only supports
> memcpy/memset and this driver only support memcpy
> interface. HW and driver doesn't support slave interface.

Make lines a bit longer.

> +/*
> + * Qualcomm Technologies HIDMA DMA engine interface
> + *
> + * Copyright (c) 2015, 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.
> + */
> +
> +/*
> + * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
> + * Copyright (C) Semihalf 2009
> + * Copyright (C) Ilya Yanok, Emcraft Systems 2010
> + * Copyright (C) Alexander Popov, Promcontroller 2014
> + *
> + * Written by Piotr Ziecik . Hardware description
> + * (defines, structures and comments) was taken from MPC5121 DMA driver
> + * written by Hongjun Chen .
> + *
> + * Approved as OSADL project by a majority of OSADL members and funded
> + * by OSADL membership fees in 2009;  for details see www.osadl.org.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + * 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.
> + *
> + * The full GNU General Public License is included in this distribution in 
> the
> + * file called COPYING.
> + */
> +
> +/* Linux Foundation elects GPLv2 license only. */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "../dmaengine.h"
> +#include "hidma.h"
> +
> +/*
> + * Default idle time is 2 seconds. This parameter can
> + * be overridden by changing the following
> + * /sys/bus/platform/devices/QCOM8061:/power/autosuspend_delay_ms
> + * during kernel boot.
> + */
> +#define AUTOSUSPEND_TIMEOUT2000
> +#define ERR_INFO_SW0xFF
> +#define ERR_CODE_UNEXPECTED_TERMINATE  0x0
> +
> +static inline
> +struct hidma_dev *to_hidma_dev(struct dma_device *dmadev)
> +{
> +   return container_of(dmadev, struct hidma_dev, ddev);
> +}
> +
> +static inline
> +struct hidma_dev *to_hidma_dev_from_lldev(struct hidma_lldev **_lldevp)
> +{
> +   return container_of(_lldevp, struct hidma_dev, lldev);
> +}
> +
> +static inline
> +struct hidma_chan *to_hidma_chan(struct dma_chan *dmach)
> +{
> +   return container_of(dmach, struct hidma_chan, chan);
> +}
> +
> +static inline struct hidma_desc *
> +to_hidma_desc(struct dma_async_tx_descriptor *t)
> +{
> +   return container_of(t, struct hidma_desc, desc);
> +}
> +
> +static void hidma_free(struct hidma_dev *dmadev)
> +{
> +   dev_dbg(dmadev->ddev.dev, "free dmadev\n");
> +   INIT_LIST_HEAD(>ddev.channels);
> +}
> +
> +static unsigned int nr_desc_prm;
> +module_param(nr_desc_prm, uint, 0644);
> +MODULE_PARM_DESC(nr_desc_prm,
> +"number of descriptors (default: 0)");
> +
> +#define MAX_HIDMA_CHANNELS 64
> +static int event_channel_idx[MAX_HIDMA_CHANNELS] = {
> +   [0 ... (MAX_HIDMA_CHANNELS - 1)] = -1};
> +static unsigned int num_event_channel_idx;
> +module_param_array_named(event_channel_idx, event_channel_idx, int,
> +   _event_channel_idx, 0644);
> +MODULE_PARM_DESC(event_channel_idx,
> +   "event channel index array for the notifications");
> +static atomic_t channel_ref_count;
> +
> +/* process completed descriptors */
> +static void hidma_process_completed(struct hidma_dev *mdma)
> +{
> +   dma_cookie_t last_cookie = 0;
> +   struct hidma_chan *mchan;
> +   struct hidma_desc *mdesc;
> +   struct dma_async_tx_descriptor *desc;
> +   unsigned long irqflags;
> +   struct list_head list;
> +   struct dma_chan *dmach = NULL;

Redundant assignment.

> +
> +   list_for_each_entry(dmach, >ddev.channels,
> +   device_node) {
> +   mchan = to_hidma_chan(dmach);
> +

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread Sinan Kaya 



On 11/8/2015 3:47 PM, Andy Shevchenko wrote:

+   trca_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>+   if (!trca_resource) {
>+   rc = -ENODEV;
>+   goto bailout;
>+   }

Why did you ignore my comment about this block?
Remove that condition entirely.


>+
>+   trca = devm_ioremap_resource(>dev, trca_resource);
>+   if (IS_ERR(trca)) {
>+   rc = -ENOMEM;
>+   goto bailout;
>+   }


Sorry, I didn't quite get your comment. I thought you wanted to see 
platform_get_resource and devm_ioremap_resource together.


Which one do you want me to remove?

--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread Andy Shevchenko 
On Sun, Nov 8, 2015 at 11:51 PM, Sinan Kaya  wrote:
>
>
> On 11/8/2015 3:47 PM, Andy Shevchenko wrote:
>>>
>>> +   trca_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> >+   if (!trca_resource) {
>>> >+   rc = -ENODEV;
>>> >+   goto bailout;
>>> >+   }
>>
>> Why did you ignore my comment about this block?
>> Remove that condition entirely.
>>
>>> >+
>>> >+   trca = devm_ioremap_resource(>dev, trca_resource);
>>> >+   if (IS_ERR(trca)) {
>>> >+   rc = -ENOMEM;
>>> >+   goto bailout;
>>> >+   }
>
>
> Sorry, I didn't quite get your comment. I thought you wanted to see
> platform_get_resource and devm_ioremap_resource together.
>
> Which one do you want me to remove?

At the end you would have something like

res = platform_get_resource();
addr = devm_ioremap_resources();
if (!addr) {
…
}

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread Sinan Kaya 



On 11/8/2015 2:13 PM, kbuild test robot wrote:

Hi Sinan,

[auto build test WARNING on: robh/for-next]
[also build test WARNING on: v4.3 next-20151106]

url:
https://github.com/0day-ci/linux/commits/Sinan-Kaya/ma-add-Qualcomm-Technologies-HIDMA-driver/20151108-125824
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux for-next
config: mn10300-allyesconfig (attached as .config)
reproduce:
 wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
 chmod +x ~/bin/make.cross
 # save the attached .config to linux build tree
 make.cross ARCH=mn10300

All warnings (new ones prefixed by >>):

In file included from include/linux/printk.h:277:0,
 from include/linux/kernel.h:13,
 from include/linux/list.h:8,
 from include/linux/kobject.h:20,
 from include/linux/device.h:17,
 from include/linux/dmaengine.h:20,
 from drivers/dma/qcom/hidma.c:45:
drivers/dma/qcom/hidma.c: In function 'hidma_prep_dma_memcpy':
include/linux/dynamic_debug.h:64:16: warning: format '%zu' expects argument 
of type 'size_t', but argument 7 has type 'unsigned int' [-Wformat=]
  static struct _ddebug  __aligned(8)   \
^
include/linux/dynamic_debug.h:84:2: note: in expansion of macro 
'DEFINE_DYNAMIC_DEBUG_METADATA'
  DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);  \
  ^
include/linux/device.h:1171:2: note: in expansion of macro 'dynamic_dev_dbg'
  dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
  ^

drivers/dma/qcom/hidma.c:391:2: note: in expansion of macro 'dev_dbg'

  dev_dbg(mdma->ddev.dev,
  ^

vim +/dev_dbg +391 drivers/dma/qcom/hidma.c

375 
376 mchan->allocated = 0;
377 spin_unlock_irqrestore(>lock, irqflags);
378 dev_dbg(mdma->ddev.dev, "freed channel for %u\n", 
mchan->dma_sig);
379 }
380 
381 
382 static struct dma_async_tx_descriptor *
383 hidma_prep_dma_memcpy(struct dma_chan *dmach, dma_addr_t dma_dest,
384 dma_addr_t dma_src, size_t len, unsigned long 
flags)
385 {
386 struct hidma_chan *mchan = to_hidma_chan(dmach);
387 struct hidma_desc *mdesc = NULL;
388 struct hidma_dev *mdma = mchan->dmadev;
389 unsigned long irqflags;
390 
  > 391  dev_dbg(mdma->ddev.dev,
392 "memcpy: chan:%p dest:%pad src:%pad len:%zu\n", mchan,
393 _dest, _src, len);
394 


What am I missing?

len is size_t. This page says use %zu for size_t.

https://www.kernel.org/doc/Documentation/printk-formats.txt




395 /* Get free descriptor */
396 spin_lock_irqsave(>lock, irqflags);
397 if (!list_empty(>free)) {
398 mdesc = list_first_entry(>free, struct 
hidma_desc,
399 node);

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





--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

2015-11-08 Thread Sinan Kaya 



On 11/8/2015 3:47 PM, Andy Shevchenko wrote:

On Sun, Nov 8, 2015 at 6:53 AM, Sinan Kaya  wrote:

This patch adds support for hidma engine. The driver
consists of two logical blocks. The DMA engine interface
and the low-level interface. The hardware only supports
memcpy/memset and this driver only support memcpy
interface. HW and driver doesn't support slave interface.


Make lines a bit longer.



OK


+   pm_runtime_set_autosuspend_delay(>dev, AUTOSUSPEND_TIMEOUT);
+   pm_runtime_use_autosuspend(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
+   trca_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);



+   if (!trca_resource) {
+   rc = -ENODEV;
+   goto bailout;
+   }


Why did you ignore my comment about this block?
Remove that condition entirely.


Removed these four lines above.


+
+   trca = devm_ioremap_resource(>dev, trca_resource);
+   if (IS_ERR(trca)) {
+   rc = -ENOMEM;
+   goto bailout;
+   }
+
+   evca_resource = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+   if (!evca_resource) {
+   rc = -ENODEV;
+   goto bailout;
+   }


Ditto.



done



+uninit:
+   hidma_debug_uninit(dmadev);
+   hidma_ll_uninit(dmadev->lldev);
+dmafree:
+   if (dmadev)
+   hidma_free(dmadev);
+bailout:
+   pm_runtime_disable(>dev);
+   pm_runtime_put_sync_suspend(>dev);


Are you sure this is appropriate sequence?

I think

pm_runtime_put();
pm_runtime_disable();


corrected, reordered and used pm_runtime_put_sync() instead.


will do the job.


+   return rc;
+}
+
+static int hidma_remove(struct platform_device *pdev)
+{
+   struct hidma_dev *dmadev = platform_get_drvdata(pdev);
+
+   dev_dbg(>dev, "removing\n");


Useless message.


Removed.


+   pm_runtime_get_sync(dmadev->ddev.dev);
+
+   dma_async_device_unregister(>ddev);
+   hidma_debug_uninit(dmadev);
+   hidma_ll_uninit(dmadev->lldev);
+   hidma_free(dmadev);
+
+   dev_info(>dev, "HI-DMA engine removed\n");
+   pm_runtime_put_sync_suspend(>dev);
+   pm_runtime_disable(>dev);
+
+   return 0;
+}




--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/