[PATCH v2] staging: media: davinci_vpfe: add error handling on kmalloc failure

[Ji-Hun Kim]

There is no failure checking on the param value which will be allocated
memory by kmalloc. Add a null pointer checking statement. Then goto error:
and return -ENOMEM error code when kmalloc is failed.

Signed-off-by: Ji-Hun Kim 
---
Changes since v1:
  - Return with -ENOMEM directly, instead of goto error: then return.

 drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c 
b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index 6a3434c..ffcd86d 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1280,6 +1280,9 @@ static int ipipe_s_config(struct v4l2_subdev *sd, struct 
vpfe_ipipe_config *cfg)
 
params = kmalloc(sizeof(struct ipipe_module_params),
 GFP_KERNEL);
+   if (!params)
+   return -ENOMEM;
+
to = (void *)params + module_if->param_offset;
size = module_if->param_size;
 
@@ -1323,6 +1326,9 @@ static int ipipe_g_config(struct v4l2_subdev *sd, struct 
vpfe_ipipe_config *cfg)
 
params =  kmalloc(sizeof(struct ipipe_module_params),
GFP_KERNEL);
+   if (!params)
+   return -ENOMEM;
+
from = (void *)params + module_if->param_offset;
size = module_if->param_size;
 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v2 10/21] lightnvm: Remove depends on HAS_DMA in case of platform dependency

[Madalin-cristian Bucur]

> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
> On Behalf Of Geert Uytterhoeven
> Sent: Friday, March 16, 2018 3:52 PM
> To: Christoph Hellwig ; Marek Szyprowski
> ; Robin Murphy ;
> Felipe Balbi ; Greg Kroah-Hartman
> ; James E . J . Bottomley
> ; Martin K . Petersen
> ; Andrew Morton  foundation.org>; Mark Brown ; Liam Girdwood
> ; Tejun Heo ; Herbert Xu
> ; David S . Miller ;
> Bartlomiej Zolnierkiewicz ; Stefan Richter
> ; Alan Tull ; Moritz Fischer
> ; Wolfram Sang ; Jonathan Cameron
> ; Joerg Roedel ; Matias Bjorling
> ; Jassi Brar ; Mauro Carvalho
> Chehab ; Ulf Hansson ; David
> Woodhouse ; Brian Norris
> ; Marek Vasut ;
> Cyrille Pitchen ; Boris Brezillon
> ; Richard Weinberger ;
> Kalle Valo ; Ohad Ben-Cohen ;
> Bjorn Andersson ; Eric Anholt ;
> Stefan Wahren 
> Cc: io...@lists.linux-foundation.org; linux-...@vger.kernel.org; linux-
> s...@vger.kernel.org; alsa-de...@alsa-project.org; linux-...@vger.kernel.org;
> linux-cry...@vger.kernel.org; linux-fb...@vger.kernel.org; linux1394-
> de...@lists.sourceforge.net; linux-f...@vger.kernel.org; linux-
> i...@vger.kernel.org; linux-...@vger.kernel.org; linux-bl...@vger.kernel.org;
> linux-me...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> m...@lists.infradead.org; net...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; linux-ser...@vger.kernel.org; linux-
> s...@vger.kernel.org; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org; Geert Uytterhoeven 
> Subject: [PATCH v2 10/21] lightnvm: Remove depends on HAS_DMA in case of
> platform dependency
> 
> Remove dependencies on HAS_DMA where a Kconfig symbol depends on
> another
> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
> In most cases this other symbol is an architecture or platform specific
> symbol, or PCI.
> 
> Generic symbols and drivers without platform dependencies keep their
> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
> cannot work anyway.
> 
> This simplifies the dependencies, and allows to improve compile-testing.
> 
> Notes:
>   - FSL_FMAN keeps its dependency on HAS_DMA, as it calls set_dma_ops(),
> which does not exist if HAS_DMA=n (Do we need a dummy? The use of
> set_dma_ops() in this driver is questionable),

Hi,

The set_dma_ops() is no longer required in the fsl/fman, I'll send a patch to 
remove it.

Thanks

>   - SND_SOC_LPASS_IPQ806X and SND_SOC_LPASS_PLATFORM loose their
> dependency on HAS_DMA, as they are selected from
> SND_SOC_APQ8016_SBC.
> 
> Signed-off-by: Geert Uytterhoeven 
> Reviewed-by: Mark Brown 
> Acked-by: Robin Murphy 
> ---
> v2:
>   - Add Reviewed-by, Acked-by,
>   - Drop RFC state,
>   - Split per subsystem.
> ---
>  drivers/lightnvm/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/lightnvm/Kconfig b/drivers/lightnvm/Kconfig
> index 10c08982185a572f..9c03f35d9df113c6 100644
> --- a/drivers/lightnvm/Kconfig
> +++ b/drivers/lightnvm/Kconfig
> @@ -4,7 +4,7 @@
> 
>  menuconfig NVM
>   bool "Open-Channel SSD target support"
> - depends on BLOCK && HAS_DMA && PCI
> + depends on BLOCK && PCI
>   select BLK_DEV_NVME
>   help
> Say Y here to get to enable Open-channel SSDs.
> --
> 2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: Re: [PATCH] staging: media: davinci_vpfe: add error handling on kmalloc failure

[Ji-Hun Kim]

On Fri, Mar 16, 2018 at 11:32:34AM +0300, Dan Carpenter wrote:
> On Fri, Mar 16, 2018 at 01:58:23PM +0900, Ji-Hun Kim wrote:
> > There is no failure checking on the param value which will be allocated
> > memory by kmalloc. Add a null pointer checking statement. Then goto error:
> > and return -ENOMEM error code when kmalloc is failed.
> > 
> > Signed-off-by: Ji-Hun Kim 
> > ---
> >  drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c 
> > b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> > index 6a3434c..55a922c 100644
> > --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> > +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> > @@ -1280,6 +1280,10 @@ static int ipipe_s_config(struct v4l2_subdev *sd, 
> > struct vpfe_ipipe_config *cfg)
> >  
> > params = kmalloc(sizeof(struct ipipe_module_params),
> >  GFP_KERNEL);
> > +   if (!params) {
> > +   rval = -ENOMEM;
> > +   goto error;
> ^^
> 
> What does "goto error" do, do you think?  It's not clear from the name.
> When you have an unclear goto like this it often means the error
> handling is going to be buggy.
> 
> In this case, it does nothing so a direct "return -ENOMEM;" would be
> more clear.  But the rest of the error handling is buggy.
Hi Dan,
I appreciate for your specific feedbacks. It looks more clear. And I'd
like you to see my question below. I will send the patch v2.

> 
>   1263  static int ipipe_s_config(struct v4l2_subdev *sd, struct 
> vpfe_ipipe_config *cfg)
>   1264  {
>   1265  struct vpfe_ipipe_device *ipipe = v4l2_get_subdevdata(sd);
>   1266  unsigned int i;
>   1267  int rval = 0;
>   1268  
>   1269  for (i = 0; i < ARRAY_SIZE(ipipe_modules); i++) {
>   1270  unsigned int bit = 1 << i;
>   1271  
>   1272  if (cfg->flag & bit) {
>   1273  const struct ipipe_module_if *module_if =
>   1274  _modules[i];
>   1275  struct ipipe_module_params *params;
>   1276  void __user *from = *(void * __user *)
>   1277  ((void *)cfg + 
> module_if->config_offset);
>   1278  size_t size;
>   1279  void *to;
>   1280  
>   1281  params = kmalloc(sizeof(struct 
> ipipe_module_params),
>   1282   GFP_KERNEL);
> 
> Do a direct return:
> 
>   if (!params)
>   return -ENOMEM;
> 
>   1283  to = (void *)params + module_if->param_offset;
>   1284  size = module_if->param_size;
>   1285  
>   1286  if (to && from && size) {
>   1287  if (copy_from_user(to, from, size)) {
>   1288  rval = -EFAULT;
>   1289  break;
> 
> The most recent thing we allocated is "params" so lets do a
> "goto free_params;".  We'll have to declare "params" at the start of the
> function instead inside this block.
> 
>   1290  }
>   1291  rval = module_if->set(ipipe, to);
>   1292  if (rval)
>   1293  goto error;
> 
> goto free_params again since params is still the most recent thing we
> allocated.
> 
>   1294  } else if (to && !from && size) {
>   1295  rval = module_if->set(ipipe, NULL);
>   1296  if (rval)
>   1297  goto error;
> 
> And here again goto free_params.
> 
>   1298  }
>   1299  kfree(params);
>   1300  }
>   1301  }
>   1302  error:
>   1303  return rval;
> 
> 
> Change this to:
> 
>   return 0;
Instead of returning rval, returning 0 would be fine? It looks that should
return rval in normal case.

> 
> free_params:
>   kfree(params);
>   return rval;
> 
>   1304  }
> 
> regards,
> dan carpenter
> 
> 
Thanks,
Ji-Hun
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback

[Jia-Ju Bai]

On 2018/3/19 10:52, KY Srinivasan wrote:



-Original Message-
From: Jia-Ju Bai 
Sent: Sunday, March 18, 2018 7:53 AM
To: KY Srinivasan ; Haiyang Zhang
; Stephen Hemminger
; bhelg...@google.com
Cc: de...@linuxdriverproject.org; linux-...@vger.kernel.org; linux-
ker...@vger.kernel.org; Jia-Ju Bai 
Subject: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with
GFP_KERNEL in hv_pci_onchannelcallback

hv_pci_onchannelcallback() is not called in atomic context.

The call chain ending up at hv_pci_onchannelcallback() is:
[1] hv_pci_onchannelcallback() <- hv_pci_probe()
hv_pci_probe() is only set as ".probe" in hv_driver
structure "hv_pci_drv".

This function is setup as the function to handle interrupts on the channel. 
Hence the
need for GFP_ATOMIC.



Oh, sorry for my incorrect patch.
Thanks for your reply :)


Best wishes,
Jia-Ju Bai
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] vmbus: use put_device() if device_register fail

[Michael Kelley (EOSG)]

> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org  
> On Behalf
> Of KY Srinivasan
> Sent: Sunday, March 18, 2018 8:02 PM
> To: Arvind Yadav ; Stephen Hemminger
> ; Haiyang Zhang 
> Cc: de...@linuxdriverproject.org; linux-ker...@vger.kernel.org
> Subject: RE: [PATCH] vmbus: use put_device() if device_register fail
> 
> > -Original Message-
> > From: devel  On Behalf
> > Of Arvind Yadav
> > Sent: Saturday, March 17, 2018 11:48 PM
> > To: Stephen Hemminger ; Haiyang Zhang
> > 
> > Cc: de...@linuxdriverproject.org; linux-ker...@vger.kernel.org
> > Subject: [PATCH] vmbus: use put_device() if device_register fail
> >
> > if device_register() returned an error. Always use put_device()
> > to give up the reference initialized.
> >
> > Signed-off-by: Arvind Yadav 
> > ---
> >  drivers/hv/vmbus_drv.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index bc65c4d..25da2f3 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -1358,6 +1358,7 @@ int vmbus_device_register(struct hv_device
> > *child_device_obj)
> > ret = device_register(_device_obj->device);
> > if (ret) {
> > pr_err("Unable to register child device\n");
> > +   put_device(_device_obj->device);
> 
> If the registration failed; we would not have acquired the reference on the 
> device and so
> we should not be dropping the reference in the failure path.

Looking at the code for device_register() and its constituent parts
device_initialize() and device_add(), Arvind is correct.  device_initialize()
creates the object with a reference count of 1.  device_add() does not
decrement that reference count or free the object, even if it fails.  The
comments for device_register() call this out as well.

Michael  

> 
> K. Y
> > return ret;
> > }
> >
> > --
> > 2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 13/21] mmc: Remove depends on HAS_DMA in case of platform dependency

[Ulf Hansson]

On 16 March 2018 at 14:51, Geert Uytterhoeven  wrote:
> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
> In most cases this other symbol is an architecture or platform specific
> symbol, or PCI.
>
> Generic symbols and drivers without platform dependencies keep their
> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
> cannot work anyway.
>
> This simplifies the dependencies, and allows to improve compile-testing.
>
> Signed-off-by: Geert Uytterhoeven 
> Reviewed-by: Mark Brown 
> Acked-by: Robin Murphy 

Acked-by: Ulf Hansson 

> ---
> v2:
>   - Add Reviewed-by, Acked-by,
>   - Drop RFC state,
>   - Split per subsystem.
> ---
>  drivers/mmc/host/Kconfig | 10 ++
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 620c2d90a646f387..f6d43348b4a3e5d4 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -358,7 +358,6 @@ config MMC_MESON_MX_SDIO
> tristate "Amlogic Meson6/Meson8/Meson8b SD/MMC Host Controller 
> support"
> depends on ARCH_MESON || COMPILE_TEST
> depends on COMMON_CLK
> -   depends on HAS_DMA
> depends on OF
> help
>   This selects support for the SD/MMC Host Controller on
> @@ -401,7 +400,6 @@ config MMC_OMAP
>
>  config MMC_OMAP_HS
> tristate "TI OMAP High Speed Multimedia Card Interface support"
> -   depends on HAS_DMA
> depends on ARCH_OMAP2PLUS || ARCH_KEYSTONE || COMPILE_TEST
> help
>   This selects the TI OMAP High Speed Multimedia card Interface.
> @@ -511,7 +509,6 @@ config MMC_DAVINCI
>
>  config MMC_GOLDFISH
> tristate "goldfish qemu Multimedia Card Interface support"
> -   depends on HAS_DMA
> depends on GOLDFISH || COMPILE_TEST
> help
>   This selects the Goldfish Multimedia card Interface emulation
> @@ -605,7 +602,7 @@ config MMC_SDHI
>
>  config MMC_SDHI_SYS_DMAC
> tristate "DMA for SDHI SD/SDIO controllers using SYS-DMAC"
> -   depends on MMC_SDHI && HAS_DMA
> +   depends on MMC_SDHI
> default MMC_SDHI if (SUPERH || ARM)
> help
>   This provides DMA support for SDHI SD/SDIO controllers
> @@ -615,7 +612,7 @@ config MMC_SDHI_SYS_DMAC
>  config MMC_SDHI_INTERNAL_DMAC
> tristate "DMA for SDHI SD/SDIO controllers using on-chip bus 
> mastering"
> depends on ARM64 || COMPILE_TEST
> -   depends on MMC_SDHI && HAS_DMA
> +   depends on MMC_SDHI
> default MMC_SDHI if ARM64
> help
>   This provides DMA support for SDHI SD/SDIO controllers
> @@ -688,7 +685,6 @@ config MMC_CAVIUM_THUNDERX
>
>  config MMC_DW
> tristate "Synopsys DesignWare Memory Card Interface"
> -   depends on HAS_DMA
> depends on ARC || ARM || ARM64 || MIPS || COMPILE_TEST
> help
>   This selects support for the Synopsys DesignWare Mobile Storage IP
> @@ -758,7 +754,6 @@ config MMC_DW_ZX
>
>  config MMC_SH_MMCIF
> tristate "SuperH Internal MMCIF support"
> -   depends on HAS_DMA
> depends on SUPERH || ARCH_RENESAS || COMPILE_TEST
> help
>   This selects the MMC Host Interface controller (MMCIF) found in 
> various
> @@ -878,7 +873,6 @@ config MMC_TOSHIBA_PCI
>  config MMC_BCM2835
> tristate "Broadcom BCM2835 SDHOST MMC Controller support"
> depends on ARCH_BCM2835 || COMPILE_TEST
> -   depends on HAS_DMA
> help
>   This selects the BCM2835 SDHOST MMC controller. If you have
>   a BCM2835 platform with SD or MMC devices, say Y or M here.
> --
> 2.7.4
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] vmbus: use put_device() if device_register fail

[KY Srinivasan]

> -Original Message-
> From: devel  On Behalf
> Of Arvind Yadav
> Sent: Saturday, March 17, 2018 11:48 PM
> To: Stephen Hemminger ; Haiyang Zhang
> 
> Cc: de...@linuxdriverproject.org; linux-ker...@vger.kernel.org
> Subject: [PATCH] vmbus: use put_device() if device_register fail
> 
> if device_register() returned an error. Always use put_device()
> to give up the reference initialized.
> 
> Signed-off-by: Arvind Yadav 
> ---
>  drivers/hv/vmbus_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index bc65c4d..25da2f3 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1358,6 +1358,7 @@ int vmbus_device_register(struct hv_device
> *child_device_obj)
>   ret = device_register(_device_obj->device);
>   if (ret) {
>   pr_err("Unable to register child device\n");
> + put_device(_device_obj->device);

If the registration failed; we would not have acquired the reference on the 
device and so
we should not be dropping the reference in the failure path.

K. Y
>   return ret;
>   }
> 
> --
> 2.7.4
> 
> ___
> devel mailing list
> de...@linuxdriverproject.org
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdriverd
> ev.linuxdriverproject.org%2Fmailman%2Flistinfo%2Fdriverdev-
> devel=04%7C01%7Ckys%40microsoft.com%7Caf95bfab917f4e1fa4b008
> d58c9c3b72%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63656952
> 5011478334%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> oiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-
> 1=%2FEb%2FMTY2SMh8sY1tar2c8Om2uKPUZAXIUQkrG0q07CA%3D
> eserved=0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback

[KY Srinivasan]

> -Original Message-
> From: Jia-Ju Bai 
> Sent: Sunday, March 18, 2018 7:53 AM
> To: KY Srinivasan ; Haiyang Zhang
> ; Stephen Hemminger
> ; bhelg...@google.com
> Cc: de...@linuxdriverproject.org; linux-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Jia-Ju Bai 
> Subject: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with
> GFP_KERNEL in hv_pci_onchannelcallback
> 
> hv_pci_onchannelcallback() is not called in atomic context.
> 
> The call chain ending up at hv_pci_onchannelcallback() is:
> [1] hv_pci_onchannelcallback() <- hv_pci_probe()
> hv_pci_probe() is only set as ".probe" in hv_driver
> structure "hv_pci_drv".

This function is setup as the function to handle interrupts on the channel. 
Hence the
need for GFP_ATOMIC.

K. Y
> 
> Despite never getting called from atomic context,
> hv_pci_onchannelcallback() calls kmalloc with GFP_ATOMIC,
> which waits busily for allocation.
> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL
> to avoid busy waiting.
> 
> This is found by a static analysis tool named DCNS written by myself.
> 
> Signed-off-by: Jia-Ju Bai 
> ---
>  drivers/pci/host/pci-hyperv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 0fe3ea1..c5c8a99 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1887,7 +1887,7 @@ static void hv_pci_onchannelcallback(void
> *context)
>   struct pci_dev_incoming *dev_message;
>   struct hv_pci_dev *hpdev;
> 
> - buffer = kmalloc(bufferlen, GFP_ATOMIC);
> + buffer = kmalloc(bufferlen, GFP_KERNEL);
>   if (!buffer)
>   return;
> 
> @@ -1899,7 +1899,7 @@ static void hv_pci_onchannelcallback(void
> *context)
>   kfree(buffer);
>   /* Handle large packet */
>   bufferlen = bytes_recvd;
> - buffer = kmalloc(bytes_recvd, GFP_ATOMIC);
> + buffer = kmalloc(bytes_recvd, GFP_KERNEL);
>   if (!buffer)
>   return;
>   continue;
> --
> 1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 19/21] spi: Remove depends on HAS_DMA in case of platform dependency

[Mark Brown]

On Fri, Mar 16, 2018 at 02:51:52PM +0100, Geert Uytterhoeven wrote:
> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
> In most cases this other symbol is an architecture or platform specific
> symbol, or PCI.

Acked-by: Mark Brown 


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 01/21] ASoC: Remove depends on HAS_DMA in case of platform dependency

[Mark Brown]

On Fri, Mar 16, 2018 at 02:51:34PM +0100, Geert Uytterhoeven wrote:
> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
> In most cases this other symbol is an architecture or platform specific
> symbol, or PCI.

Acked-by: Mark Brown 

Thanks again for doing this work, it's really good to see!


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 16/21] remoteproc: Remove depends on HAS_DMA in case of platform dependency

[Bjorn Andersson]

On Fri 16 Mar 06:51 PDT 2018, Geert Uytterhoeven wrote:

> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
> In most cases this other symbol is an architecture or platform specific
> symbol, or PCI.
> 
> Generic symbols and drivers without platform dependencies keep their
> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
> cannot work anyway.
> 
> This simplifies the dependencies, and allows to improve compile-testing.
> 
> Signed-off-by: Geert Uytterhoeven 
> Reviewed-by: Mark Brown 
> Acked-by: Robin Murphy 

Acked-by: Bjorn Andersson 

Regards,
Bjorn

> ---
> v2:
>   - Add Reviewed-by, Acked-by,
>   - Drop RFC state,
>   - Split per subsystem.
> ---
>  drivers/remoteproc/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index b609e1d3654ba65f..b60d8132113de0f7 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -23,7 +23,6 @@ config IMX_REMOTEPROC
>  
>  config OMAP_REMOTEPROC
>   tristate "OMAP remoteproc support"
> - depends on HAS_DMA
>   depends on ARCH_OMAP4 || SOC_OMAP5
>   depends on OMAP_IOMMU
>   select MAILBOX
> -- 
> 2.7.4
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 2/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in new_pcichild_device

[Michael Kelley (EOSG)]

> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org  
> On Behalf
> Of Jia-Ju Bai
> Sent: Sunday, March 18, 2018 7:53 AM
> To: KY Srinivasan ; Haiyang Zhang 
> ; Stephen
> Hemminger ; bhelg...@google.com
> Cc: de...@linuxdriverproject.org; linux-...@vger.kernel.org; 
> linux-ker...@vger.kernel.org;
> Jia-Ju Bai 
> Subject: [PATCH 2/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with 
> GFP_KERNEL in
> new_pcichild_device
> 
> new_pcichild_device() is not called in atomic context.
> 
> The call chain ending up at new_pcichild_device() is:
> [1] new_pcichild_device() <- pci_devices_present_work()
> pci_devices_present_work() is only set in INIT_WORK().
> 
> Despite never getting called from atomic context,
> new_pcichild_device() calls kzalloc with GFP_ATOMIC,
> which waits busily for allocation.
> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL
> to avoid busy waiting.
> 
> This is found by a static analysis tool named DCNS written by myself.
> 
> Signed-off-by: Jia-Ju Bai 

Reviewed-by: Michael Kelley 

> ---
>  drivers/pci/host/pci-hyperv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 0fe3ea1..289e31d 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1500,7 +1500,7 @@ static struct hv_pci_dev *new_pcichild_device(struct
> hv_pcibus_device *hbus,
>   unsigned long flags;
>   int ret;
> 
> - hpdev = kzalloc(sizeof(*hpdev), GFP_ATOMIC);
> + hpdev = kzalloc(sizeof(*hpdev), GFP_KERNEL);
>   if (!hpdev)
>   return NULL;
> 
> --
> 1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback

[Michael Kelley (EOSG)]

> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org  
> On Behalf
> Of Jia-Ju Bai
> Sent: Sunday, March 18, 2018 7:53 AM
> To: KY Srinivasan ; Haiyang Zhang 
> ; Stephen
> Hemminger ; bhelg...@google.com
> Cc: de...@linuxdriverproject.org; linux-...@vger.kernel.org; 
> linux-ker...@vger.kernel.org;
> Jia-Ju Bai 
> Subject: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with 
> GFP_KERNEL in
> hv_pci_onchannelcallback
> 
> hv_pci_onchannelcallback() is not called in atomic context.
> 
> The call chain ending up at hv_pci_onchannelcallback() is:
> [1] hv_pci_onchannelcallback() <- hv_pci_probe()
> hv_pci_probe() is only set as ".probe" in hv_driver
> structure "hv_pci_drv".
> 
> Despite never getting called from atomic context,

Not true.   hv_pci_probe() registers hv_pci_onchannelcallback() as
a callback function that is invoked from the VMbus interrupt handler.
So GFP_ATOMIC is appropriate.

Michael

> hv_pci_onchannelcallback() calls kmalloc with GFP_ATOMIC,
> which waits busily for allocation.
> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL
> to avoid busy waiting.
> 
> This is found by a static analysis tool named DCNS written by myself.
> 
> Signed-off-by: Jia-Ju Bai 
> ---
>  drivers/pci/host/pci-hyperv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 0fe3ea1..c5c8a99 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1887,7 +1887,7 @@ static void hv_pci_onchannelcallback(void *context)
>   struct pci_dev_incoming *dev_message;
>   struct hv_pci_dev *hpdev;
> 
> - buffer = kmalloc(bufferlen, GFP_ATOMIC);
> + buffer = kmalloc(bufferlen, GFP_KERNEL);
>   if (!buffer)
>   return;
> 
> @@ -1899,7 +1899,7 @@ static void hv_pci_onchannelcallback(void *context)
>   kfree(buffer);
>   /* Handle large packet */
>   bufferlen = bytes_recvd;
> - buffer = kmalloc(bytes_recvd, GFP_ATOMIC);
> + buffer = kmalloc(bytes_recvd, GFP_KERNEL);
>   if (!buffer)
>   return;
>   continue;
> --
> 1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] vmbus: use put_device() if device_register fail

[Michael Kelley (EOSG)]

> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org  
> On Behalf
> Of Arvind Yadav
> Sent: Saturday, March 17, 2018 11:48 PM
> To: Stephen Hemminger ; Haiyang Zhang
> 
> Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org
> Subject: [PATCH] vmbus: use put_device() if device_register fail
> 
> if device_register() returned an error. Always use put_device()
> to give up the reference initialized.
> 
> Signed-off-by: Arvind Yadav 

Reviewed-by: Michael Kelley 

> ---
>  drivers/hv/vmbus_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index bc65c4d..25da2f3 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1358,6 +1358,7 @@ int vmbus_device_register(struct hv_device 
> *child_device_obj)
>   ret = device_register(_device_obj->device);
>   if (ret) {
>   pr_err("Unable to register child device\n");
> + put_device(_device_obj->device);
>   return ret;
>   }
> 
> --
> 2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 14/21] mtd: Remove depends on HAS_DMA in case of platform dependency

[Boris Brezillon]

Hi Geert,

On Fri, 16 Mar 2018 14:51:47 +0100
Geert Uytterhoeven  wrote:

> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
> In most cases this other symbol is an architecture or platform specific
> symbol, or PCI.
> 
> Generic symbols and drivers without platform dependencies keep their
> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
> cannot work anyway.
> 
> This simplifies the dependencies, and allows to improve compile-testing.
> 

Don't know which release you're targeting but it's likely to conflict
with the change I have in my nand/next branch. Is this a problem if I
take this patch through the mtd tree after [1] has reached Linus' tree?

Regards,

Boris

[1]https://lkml.org/lkml/2018/3/16/435

> Signed-off-by: Geert Uytterhoeven 
> Reviewed-by: Mark Brown 
> Acked-by: Robin Murphy 
> ---
> v2:
>   - Add Reviewed-by, Acked-by,
>   - Drop RFC state,
>   - Drop new dependency of MTD_NAND_MARVELL on HAS_DMA,
>   - Split per subsystem.
> ---
>  drivers/mtd/nand/Kconfig| 8 ++--
>  drivers/mtd/spi-nor/Kconfig | 2 +-
>  2 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 736ac887303c88ba..55a2f8a2fa90cd87 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -46,7 +46,7 @@ config MTD_NAND_DENALI
>  config MTD_NAND_DENALI_PCI
>  tristate "Support Denali NAND controller on Intel Moorestown"
>   select MTD_NAND_DENALI
> - depends on HAS_DMA && PCI
> + depends on PCI
>  help
>Enable the driver for NAND flash on Intel Moorestown, using the
>Denali NAND controller core.
> @@ -184,7 +184,6 @@ config MTD_NAND_S3C2410_CLKSTOP
>  config MTD_NAND_TANGO
>   tristate "NAND Flash support for Tango chips"
>   depends on ARCH_TANGO || COMPILE_TEST
> - depends on HAS_DMA
>   help
> Enables the NAND Flash controller on Tango chips.
>  
> @@ -328,7 +327,7 @@ config MTD_NAND_MARVELL
>   tristate "NAND controller support on Marvell boards"
>   depends on PXA3xx || ARCH_MMP || PLAT_ORION || ARCH_MVEBU || \
>  COMPILE_TEST
> - depends on HAS_IOMEM && HAS_DMA
> + depends on HAS_IOMEM
>   help
> This enables the NAND flash controller driver for Marvell boards,
> including:
> @@ -490,7 +489,6 @@ config MTD_NAND_SH_FLCTL
>   tristate "Support for NAND on Renesas SuperH FLCTL"
>   depends on SUPERH || COMPILE_TEST
>   depends on HAS_IOMEM
> - depends on HAS_DMA
>   help
> Several Renesas SuperH CPU has FLCTL. This option enables support
> for NAND Flash using FLCTL.
> @@ -558,7 +556,6 @@ config MTD_NAND_SUNXI
>  config MTD_NAND_HISI504
>   tristate "Support for NAND controller on Hisilicon SoC Hip04"
>   depends on ARCH_HISI || COMPILE_TEST
> - depends on HAS_DMA
>   help
> Enables support for NAND controller on Hisilicon SoC Hip04.
>  
> @@ -572,7 +569,6 @@ config MTD_NAND_QCOM
>  config MTD_NAND_MTK
>   tristate "Support for NAND controller on MTK SoCs"
>   depends on ARCH_MEDIATEK || COMPILE_TEST
> - depends on HAS_DMA
>   help
> Enables support for NAND controller on MTK SoCs.
> This controller is found on mt27xx, mt81xx, mt65xx SoCs.
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 89da88e591215db1..c493b8230a38c059 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -71,7 +71,7 @@ config SPI_FSL_QUADSPI
>  config SPI_HISI_SFC
>   tristate "Hisilicon SPI-NOR Flash Controller(SFC)"
>   depends on ARCH_HISI || COMPILE_TEST
> - depends on HAS_IOMEM && HAS_DMA
> + depends on HAS_IOMEM
>   help
> This enables support for hisilicon SPI-NOR flash controller.
>  



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase()

[Boris Brezillon]

On Mon, 12 Feb 2018 22:03:07 +0100
Boris Brezillon  wrote:

> mtd_erase() can return an error before ->fail_addr is initialized to
> MTD_FAIL_ADDR_UNKNOWN. Move this initialization at the very beginning
> of the function.

Applied the patchset after addressing Miquel's comments.

> 
> Signed-off-by: Boris Brezillon 
> ---
>  drivers/mtd/mtdcore.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index a1c94526fb88..c87859ff338b 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -953,6 +953,8 @@ EXPORT_SYMBOL_GPL(__put_mtd_device);
>   */
>  int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
>  {
> + instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
> +
>   if (!mtd->erasesize || !mtd->_erase)
>   return -ENOTSUPP;
>  
> @@ -961,7 +963,6 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info 
> *instr)
>   if (!(mtd->flags & MTD_WRITEABLE))
>   return -EROFS;
>  
> - instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
>   if (!instr->len) {
>   instr->state = MTD_ERASE_DONE;
>   mtd_erase_callback(instr);



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 10/21] lightnvm: Remove depends on HAS_DMA in case of platform dependency

[Matias Bjørling]

On 03/16/2018 02:51 PM, Geert Uytterhoeven wrote:

Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
In most cases this other symbol is an architecture or platform specific
symbol, or PCI.

Generic symbols and drivers without platform dependencies keep their
dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
cannot work anyway.

This simplifies the dependencies, and allows to improve compile-testing.

Notes:
   - FSL_FMAN keeps its dependency on HAS_DMA, as it calls set_dma_ops(),
 which does not exist if HAS_DMA=n (Do we need a dummy? The use of
 set_dma_ops() in this driver is questionable),
   - SND_SOC_LPASS_IPQ806X and SND_SOC_LPASS_PLATFORM loose their
 dependency on HAS_DMA, as they are selected from
 SND_SOC_APQ8016_SBC.

Signed-off-by: Geert Uytterhoeven 
Reviewed-by: Mark Brown 
Acked-by: Robin Murphy 
---
v2:
   - Add Reviewed-by, Acked-by,
   - Drop RFC state,
   - Split per subsystem.
---
  drivers/lightnvm/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/lightnvm/Kconfig b/drivers/lightnvm/Kconfig
index 10c08982185a572f..9c03f35d9df113c6 100644
--- a/drivers/lightnvm/Kconfig
+++ b/drivers/lightnvm/Kconfig
@@ -4,7 +4,7 @@
  
  menuconfig NVM

bool "Open-Channel SSD target support"
-   depends on BLOCK && HAS_DMA && PCI
+   depends on BLOCK && PCI
select BLK_DEV_NVME
help
  Say Y here to get to enable Open-channel SSDs.



Looks good.

Reviewed-by: Matias Bjørling 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in new_pcichild_device

[Jia-Ju Bai]

new_pcichild_device() is not called in atomic context.

The call chain ending up at new_pcichild_device() is:
[1] new_pcichild_device() <- pci_devices_present_work()
pci_devices_present_work() is only set in INIT_WORK().

Despite never getting called from atomic context,
new_pcichild_device() calls kzalloc with GFP_ATOMIC,
which waits busily for allocation.
GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL
to avoid busy waiting.

This is found by a static analysis tool named DCNS written by myself.

Signed-off-by: Jia-Ju Bai 
---
 drivers/pci/host/pci-hyperv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 0fe3ea1..289e31d 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1500,7 +1500,7 @@ static struct hv_pci_dev *new_pcichild_device(struct 
hv_pcibus_device *hbus,
unsigned long flags;
int ret;
 
-   hpdev = kzalloc(sizeof(*hpdev), GFP_ATOMIC);
+   hpdev = kzalloc(sizeof(*hpdev), GFP_KERNEL);
if (!hpdev)
return NULL;
 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback

[Jia-Ju Bai]

hv_pci_onchannelcallback() is not called in atomic context.

The call chain ending up at hv_pci_onchannelcallback() is:
[1] hv_pci_onchannelcallback() <- hv_pci_probe()
hv_pci_probe() is only set as ".probe" in hv_driver 
structure "hv_pci_drv".

Despite never getting called from atomic context, 
hv_pci_onchannelcallback() calls kmalloc with GFP_ATOMIC, 
which waits busily for allocation.
GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL 
to avoid busy waiting.

This is found by a static analysis tool named DCNS written by myself.

Signed-off-by: Jia-Ju Bai 
---
 drivers/pci/host/pci-hyperv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 0fe3ea1..c5c8a99 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1887,7 +1887,7 @@ static void hv_pci_onchannelcallback(void *context)
struct pci_dev_incoming *dev_message;
struct hv_pci_dev *hpdev;
 
-   buffer = kmalloc(bufferlen, GFP_ATOMIC);
+   buffer = kmalloc(bufferlen, GFP_KERNEL);
if (!buffer)
return;
 
@@ -1899,7 +1899,7 @@ static void hv_pci_onchannelcallback(void *context)
kfree(buffer);
/* Handle large packet */
bufferlen = bytes_recvd;
-   buffer = kmalloc(bytes_recvd, GFP_ATOMIC);
+   buffer = kmalloc(bytes_recvd, GFP_KERNEL);
if (!buffer)
return;
continue;
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 11/11] Move resolver ad2c1200 driver out of staging to mainline iio

[David Veenstra]

Signed-off-by: David Veenstra 
---
 drivers/iio/Kconfig   |  1 +
 drivers/iio/Makefile  |  1 +
 drivers/iio/resolver/Kconfig  | 17 +
 drivers/iio/resolver/Makefile |  5 +
 drivers/{staging => }/iio/resolver/ad2s1200.c |  0
 drivers/staging/iio/resolver/Kconfig  | 12 
 drivers/staging/iio/resolver/Makefile |  1 -
 7 files changed, 24 insertions(+), 13 deletions(-)
 create mode 100644 drivers/iio/resolver/Kconfig
 create mode 100644 drivers/iio/resolver/Makefile
 rename drivers/{staging => }/iio/resolver/ad2s1200.c (100%)

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index b3c8c6ef0dff..4bec3ccbf4a1 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -92,6 +92,7 @@ source "drivers/iio/potentiometer/Kconfig"
 source "drivers/iio/potentiostat/Kconfig"
 source "drivers/iio/pressure/Kconfig"
 source "drivers/iio/proximity/Kconfig"
+source "drivers/iio/resolver/Kconfig"
 source "drivers/iio/temperature/Kconfig"
 
 endif # IIO
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index b16b2e9ddc40..1865361b8714 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -35,5 +35,6 @@ obj-y += potentiometer/
 obj-y += potentiostat/
 obj-y += pressure/
 obj-y += proximity/
+obj-y += resolver/
 obj-y += temperature/
 obj-y += trigger/
diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
new file mode 100644
index ..2ced9f22aa70
--- /dev/null
+++ b/drivers/iio/resolver/Kconfig
@@ -0,0 +1,17 @@
+#
+# Resolver/Synchro drivers
+#
+menu "Resolver to digital converters"
+
+config AD2S1200
+   tristate "Analog Devices ad2s1200/ad2s1205 driver"
+   depends on SPI
+   depends on GPIOLIB || COMPILE_TEST
+   help
+ Say yes here to build support for Analog Devices spi resolver
+ to digital converters, ad2s1200 and ad2s1205, provides direct access
+ via sysfs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad2s1200.
+endmenu
diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
new file mode 100644
index ..4e1dccae07e7
--- /dev/null
+++ b/drivers/iio/resolver/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for Resolver/Synchro drivers
+#
+
+obj-$(CONFIG_AD2S1200) += ad2s1200.o
diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/iio/resolver/ad2s1200.c
similarity index 100%
rename from drivers/staging/iio/resolver/ad2s1200.c
rename to drivers/iio/resolver/ad2s1200.c
diff --git a/drivers/staging/iio/resolver/Kconfig 
b/drivers/staging/iio/resolver/Kconfig
index 1c7e2860d6b7..6a469ee6101f 100644
--- a/drivers/staging/iio/resolver/Kconfig
+++ b/drivers/staging/iio/resolver/Kconfig
@@ -13,18 +13,6 @@ config AD2S90
  To compile this driver as a module, choose M here: the
  module will be called ad2s90.
 
-config AD2S1200
-   tristate "Analog Devices ad2s1200/ad2s1205 driver"
-   depends on SPI
-   depends on GPIOLIB || COMPILE_TEST
-   help
- Say yes here to build support for Analog Devices spi resolver
- to digital converters, ad2s1200 and ad2s1205, provides direct access
- via sysfs.
-
- To compile this driver as a module, choose M here: the
- module will be called ad2s1200.
-
 config AD2S1210
tristate "Analog Devices ad2s1210 driver"
depends on SPI
diff --git a/drivers/staging/iio/resolver/Makefile 
b/drivers/staging/iio/resolver/Makefile
index 14375e444ebf..8d901dc7500b 100644
--- a/drivers/staging/iio/resolver/Makefile
+++ b/drivers/staging/iio/resolver/Makefile
@@ -3,5 +3,4 @@
 #
 
 obj-$(CONFIG_AD2S90) += ad2s90.o
-obj-$(CONFIG_AD2S1200) += ad2s1200.o
 obj-$(CONFIG_AD2S1210) += ad2s1210.o
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 09/11] staging: iio: ad2s1200: Add scaling factor for IIO_ANGL_VEL channel

[David Veenstra]

The sysfs iio ABI states that a unit of radians per second is
expected, but the 12-bit angular velocity register has rps as its unit.
So a fractional scaling factor of approximately 2 * Pi is added to the
angular velocity channel.

Signed-off-by: David Veenstra 
---
 drivers/staging/iio/resolver/ad2s1200.c | 82 ++---
 1 file changed, 56 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/staging/iio/resolver/ad2s1200.c
index 7ee1d9f76dfb..4b97a106012c 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -61,40 +61,69 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
int ret = 0;
u16 vel;
 
-   mutex_lock(>lock);
-   gpiod_set_value(st->sample, 0);
+   switch (m) {
+   case IIO_CHAN_INFO_SCALE:
+   switch (chan->type) {
+   case IIO_ANGL_VEL:
+   /*
+* 2 * Pi is almost equal to
+* 2 * 103993 / 33102 = 103993 / (33102 / 2)
+* This fraction is based on OEIS series of
+* of nominator/denominator of convergents to Pi
+* (A002485 and A002486).
+*
+* iio_convert_raw_to_processed_unlocked uses integer
+* division. This will cause at most 5% error
+* (for very small values). But for 99.5% of the values
+* it will cause less that 1% error.
+*/
+   *val = 103993;
+   *val2 = 33102 / 2;
+   return IIO_VAL_FRACTIONAL;
+   default:
+   return -EINVAL;
+   }
+   break;
+   case IIO_CHAN_INFO_RAW:
+   mutex_lock(>lock);
+   gpiod_set_value(st->sample, 0);
+
+   /* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
+   udelay(1);
+   gpiod_set_value(st->sample, 1);
+   gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+
+   ret = spi_read(st->sdev, st->rx, 2);
+   if (ret < 0) {
+   mutex_unlock(>lock);
+   return ret;
+   }
 
-   /* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
-   udelay(1);
-   gpiod_set_value(st->sample, 1);
-   gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+   vel = be16_to_cpup((__be16 *)st->rx);
+   switch (chan->type) {
+   case IIO_ANGL:
+   *val = vel >> 4;
+   ret = IIO_VAL_INT;
+   break;
+   case IIO_ANGL_VEL:
+   *val = sign_extend32((s16)vel >> 4, 11);
+   ret = IIO_VAL_INT;
+   break;
+   default:
+   ret = -EINVAL;
+   break;
+   }
 
-   ret = spi_read(st->sdev, st->rx, 2);
-   if (ret < 0) {
+   /* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
+   udelay(1);
mutex_unlock(>lock);
-   return ret;
-   }
 
-   vel = be16_to_cpup((__be16 *)st->rx);
-   switch (chan->type) {
-   case IIO_ANGL:
-   *val = vel >> 4;
-   ret = IIO_VAL_INT;
-   break;
-   case IIO_ANGL_VEL:
-   *val = sign_extend32((s16)vel >> 4, 11);
-   ret = IIO_VAL_INT;
-   break;
+   return ret;
default:
-   ret = -EINVAL;
break;
}
 
-   /* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
-   udelay(1);
-   mutex_unlock(>lock);
-
-   return ret;
+   return -EINVAL;
 }
 
 static const struct iio_chan_spec ad2s1200_channels[] = {
@@ -108,6 +137,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
.indexed = 1,
.channel = 0,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+   .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
}
 };
 
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 08/11] staging: iio: ad2s1200: Replace legacy gpio ABI with modern ABI

[David Veenstra]

The legacy, integer based gpio ABI is replaced with the descriptor
based ABI.

For compatibility, it is first tried to use the platform data to
request the gpio's. Otherwise, it looks for the "sample" and "rdvel"
gpio function.

Signed-off-by: David Veenstra 
---
 drivers/staging/iio/resolver/ad2s1200.c | 51 -
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/staging/iio/resolver/ad2s1200.c
index 6ce9ca13094a..7ee1d9f76dfb 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -45,8 +46,8 @@
 struct ad2s1200_state {
struct mutex lock;
struct spi_device *sdev;
-   int sample;
-   int rdvel;
+   struct gpio_desc *sample;
+   struct gpio_desc *rdvel;
u8 rx[2] cacheline_aligned;
 };
 
@@ -61,12 +62,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
u16 vel;
 
mutex_lock(>lock);
-   gpio_set_value(st->sample, 0);
+   gpiod_set_value(st->sample, 0);
 
/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
udelay(1);
-   gpio_set_value(st->sample, 1);
-   gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+   gpiod_set_value(st->sample, 1);
+   gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
 
ret = spi_read(st->sdev, st->rx, 2);
if (ret < 0) {
@@ -124,13 +125,18 @@ static int ad2s1200_probe(struct spi_device *spi)
 
dev = >dev;
 
-   for (pn = 0; pn < AD2S1200_PN; pn++) {
-   ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
-   DRV_NAME);
-   if (ret) {
-   dev_err(dev, "request gpio pin %d failed\n",
-   pins[pn]);
-   return ret;
+   if (pins) {
+   for (pn = 0; pn < AD2S1200_PN; pn++) {
+   ret = devm_gpio_request_one(dev, pins[pn],
+   GPIOF_DIR_OUT,
+   DRV_NAME);
+   if (ret) {
+   dev_err(dev,
+   "Failed to claim gpio %d\n: err=%d",
+   pins[pn],
+   ret);
+   return ret;
+   }
}
}
 
@@ -142,8 +148,25 @@ static int ad2s1200_probe(struct spi_device *spi)
st = iio_priv(indio_dev);
mutex_init(>lock);
st->sdev = spi;
-   st->sample = pins[0];
-   st->rdvel = pins[1];
+
+   if (pins) {
+   st->sample = gpio_to_desc(pins[0]);
+   st->rdvel = gpio_to_desc(pins[1]);
+   } else {
+   st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
+   if (IS_ERR(st->sample)) {
+   dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
+   PTR_ERR(st->sample));
+   return PTR_ERR(st->sample);
+   }
+
+   st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
+   if (IS_ERR(st->rdvel)) {
+   dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
+   PTR_ERR(st->rdvel));
+   return PTR_ERR(st->rdvel);
+   }
+   }
 
indio_dev->dev.parent = dev;
indio_dev->info = _info;
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 07/11] staging: iio: ad2s1200: Ensure udelay(1) in all necessary code paths

[David Veenstra]

After a successful spi transaction, a udelay(1) is needed.
This doesn't happen for the default case of the switch statement
in ad2s1200_read_raw. This patch makes sure that it does.

Signed-off-by: David Veenstra 
---
 drivers/staging/iio/resolver/ad2s1200.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/staging/iio/resolver/ad2s1200.c
index e0e7c88368ed..6ce9ca13094a 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -78,20 +78,22 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
switch (chan->type) {
case IIO_ANGL:
*val = vel >> 4;
+   ret = IIO_VAL_INT;
break;
case IIO_ANGL_VEL:
*val = sign_extend32((s16)vel >> 4, 11);
+   ret = IIO_VAL_INT;
break;
default:
-   mutex_unlock(>lock);
-   return -EINVAL;
+   ret = -EINVAL;
+   break;
}
 
/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
udelay(1);
mutex_unlock(>lock);
 
-   return IIO_VAL_INT;
+   return ret;
 }
 
 static const struct iio_chan_spec ad2s1200_channels[] = {
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 06/11] staging: iio: ad2s1200: Improve readability with be16_to_cpup

[David Veenstra]

The manual states that the data is contained in the upper 12 bits
of the 16 bits read by spi. The code that extracts these 12 bits
is correct for both be and le machines, but this is not clear
from a first glance.

To improve readability the relevant expressions are replaced
with equivalent expressions that use be16_to_cpup.

Signed-off-by: David Veenstra 
---
 drivers/staging/iio/resolver/ad2s1200.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/staging/iio/resolver/ad2s1200.c
index eceb86e952de..e0e7c88368ed 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -58,7 +58,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 {
struct ad2s1200_state *st = iio_priv(indio_dev);
int ret = 0;
-   s16 vel;
+   u16 vel;
 
mutex_lock(>lock);
gpio_set_value(st->sample, 0);
@@ -74,14 +74,13 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
return ret;
}
 
+   vel = be16_to_cpup((__be16 *)st->rx);
switch (chan->type) {
case IIO_ANGL:
-   *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
+   *val = vel >> 4;
break;
case IIO_ANGL_VEL:
-   vel = (((s16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-   vel = sign_extend32(vel, 11);
-   *val = vel;
+   *val = sign_extend32((s16)vel >> 4, 11);
break;
default:
mutex_unlock(>lock);
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 04/11] staging: iio: ad2s1200: Add kernel docs to driver state

[David Veenstra]

Add missing kernel docs to the ad2s1200 driver state.

Signed-off-by: David Veenstra 
---
 drivers/staging/iio/resolver/ad2s1200.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/staging/iio/resolver/ad2s1200.c
index 20df16b7852b..00efba598671 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -34,6 +34,14 @@
 /* clock period in nano second */
 #define AD2S1200_TSCLK (10 / AD2S1200_HZ)
 
+/**
+ * struct ad2s1200_state - driver instance specific data
+ * @lock:  protect driver state
+ * @sdev:  spi device
+ * @sample:GPIO pin SAMPLE
+ * @rdvel: GPIO pin RDVEL
+ * @rx:buffer for spi transfers
+ */
 struct ad2s1200_state {
struct mutex lock;
struct spi_device *sdev;
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 05/11] staging: iio: ad2s1200: Introduce variable for repeated value

[David Veenstra]

Add variable to hold >dev in ad2s1200_probe. This value is repeatedly
used in ad2s1200_probe.

Signed-off-by: David Veenstra 
---
 drivers/staging/iio/resolver/ad2s1200.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/staging/iio/resolver/ad2s1200.c
index 00efba598671..eceb86e952de 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -118,19 +118,22 @@ static int ad2s1200_probe(struct spi_device *spi)
unsigned short *pins = spi->dev.platform_data;
struct ad2s1200_state *st;
struct iio_dev *indio_dev;
+   struct device *dev;
int pn, ret = 0;
 
+   dev = >dev;
+
for (pn = 0; pn < AD2S1200_PN; pn++) {
-   ret = devm_gpio_request_one(>dev, pins[pn], GPIOF_DIR_OUT,
+   ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
DRV_NAME);
if (ret) {
-   dev_err(>dev, "request gpio pin %d failed\n",
+   dev_err(dev, "request gpio pin %d failed\n",
pins[pn]);
return ret;
}
}
 
-   indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
+   indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
 
@@ -141,14 +144,14 @@ static int ad2s1200_probe(struct spi_device *spi)
st->sample = pins[0];
st->rdvel = pins[1];
 
-   indio_dev->dev.parent = >dev;
+   indio_dev->dev.parent = dev;
indio_dev->info = _info;
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->channels = ad2s1200_channels;
indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
indio_dev->name = spi_get_device_id(spi)->name;
 
-   ret = devm_iio_device_register(>dev, indio_dev);
+   ret = devm_iio_device_register(dev, indio_dev);
if (ret)
return ret;
 
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 03/11] staging: iio: ad2s1200: Add blank lines

[David Veenstra]

Add blank lines to improve readability.

Signed-off-by: David Veenstra 
---
 drivers/staging/iio/resolver/ad2s1200.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/staging/iio/resolver/ad2s1200.c
index 94d0a66532fd..20df16b7852b 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -9,6 +9,7 @@
  * published by the Free Software Foundation.
  *
  */
+
 #include 
 #include 
 #include 
@@ -53,10 +54,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 
mutex_lock(>lock);
gpio_set_value(st->sample, 0);
+
/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
udelay(1);
gpio_set_value(st->sample, 1);
gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+
ret = spi_read(st->sdev, st->rx, 2);
if (ret < 0) {
mutex_unlock(>lock);
@@ -76,9 +79,11 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
mutex_unlock(>lock);
return -EINVAL;
}
+
/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
udelay(1);
mutex_unlock(>lock);
+
return IIO_VAL_INT;
 }
 
@@ -116,9 +121,11 @@ static int ad2s1200_probe(struct spi_device *spi)
return ret;
}
}
+
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
+
spi_set_drvdata(spi, indio_dev);
st = iio_priv(indio_dev);
mutex_init(>lock);
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 02/11] staging: iio: ad2s1200: Reverse Christmas tree order

[David Veenstra]

Reorders the variable declarations to prefer a reverse Christmas tree
order to improve readability.

Signed-off-by: David Veenstra 
---
 drivers/staging/iio/resolver/ad2s1200.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/staging/iio/resolver/ad2s1200.c
index 7a5b2d1927d6..94d0a66532fd 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -47,9 +47,9 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 int *val2,
 long m)
 {
+   struct ad2s1200_state *st = iio_priv(indio_dev);
int ret = 0;
s16 vel;
-   struct ad2s1200_state *st = iio_priv(indio_dev);
 
mutex_lock(>lock);
gpio_set_value(st->sample, 0);
@@ -102,10 +102,10 @@ static const struct iio_info ad2s1200_info = {
 
 static int ad2s1200_probe(struct spi_device *spi)
 {
+   unsigned short *pins = spi->dev.platform_data;
struct ad2s1200_state *st;
struct iio_dev *indio_dev;
int pn, ret = 0;
-   unsigned short *pins = spi->dev.platform_data;
 
for (pn = 0; pn < AD2S1200_PN; pn++) {
ret = devm_gpio_request_one(>dev, pins[pn], GPIOF_DIR_OUT,
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/11] staging: iio: ad2s1200: Sort includes alphabetically

[David Veenstra]

This patches sorts all the includes in alphabetic order.

Signed-off-by: David Veenstra 
---
 drivers/staging/iio/resolver/ad2s1200.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/staging/iio/resolver/ad2s1200.c
index aa62c64e9bc4..7a5b2d1927d6 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -9,16 +9,16 @@
  * published by the Free Software Foundation.
  *
  */
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include 
 #include 
+#include 
 #include 
 #include 
-#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 #include 
 #include 
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 00/11] staging: iio: ad2s1200: Driver clean up

[David Veenstra]

Please forget the previous 3 emails, I forgot the message-id and
reply-to business.

This patch series is meant to move the ad2s1200 driver to mainline. In short
the following is done:

1. Clean up of minor code style issues
2. Fix minor bugs
3. Replace legacy GPIO ABI with modern ABI
4. Change the channel definitions of angular position and angular
   velocity to match the sysfs IIO ABI.

The channel for angular position had to be changed to another type
as there is no definition for IIO_ANGL in sysfs IIO ABI. There are other
types that model some kind of rotation, the candidates are:
- in_rot_from_north_*
- in_rot_quaternion
- in_incli

The in_rot_from_north_*_raw family is a rotation relevative to the
geographical north, or the magnetic north. These semantics are not a
good fit for the angular position of a resolver. The
in_rot_quaternion_raw als does not seem like a good choice, as it is
overkill for a rotation over a single and fixed axis. This is also
likely to be a cubersome format for users of the ABI interface. Finally,
there is in_incli which seems to be meant for any kind of inclination,
given in degrees. In my opinion in_incli is the best choice, as
it has a conveniant format and has semantics similar to that of angular
position.

David Veenstra (11):
  staging: iio: ad2s1200: Sort includes alphabetically
  staging: iio: ad2s1200: Reverse Christmas tree order
  staging: iio: ad2s1200: Add blank lines
  staging: iio: ad2s1200: Add kernel docs to driver state
  staging: iio: ad2s1200: Introduce variable for repeated value
  staging: iio: ad2s1200: Improve readability with be16_to_cpup
  staging: iio: ad2s1200: Ensure udelay(1) in all necessary code paths
  staging: iio: ad2s1200: Replace legacy gpio ABI with modern ABI
  staging: iio: ad2s1200: Add scaling factor for IIO_ANGL_VEL channel
  staging: iio: ad2s1200: Replace angle channel with inclination channel
  Move resolver ad2c1200 driver out of staging to mainline iio

 drivers/iio/Kconfig   |   1 +
 drivers/iio/Makefile  |   1 +
 drivers/iio/resolver/Kconfig  |  17 +++
 drivers/iio/resolver/Makefile |   5 +
 drivers/{staging => }/iio/resolver/ad2s1200.c | 177 ++
 drivers/staging/iio/resolver/Kconfig  |  12 --
 drivers/staging/iio/resolver/Makefile |   1 -
 7 files changed, 151 insertions(+), 63 deletions(-)
 create mode 100644 drivers/iio/resolver/Kconfig
 create mode 100644 drivers/iio/resolver/Makefile
 rename drivers/{staging => }/iio/resolver/ad2s1200.c (50%)

-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 02/11] staging: iio: ad2s1200: Reverse Christmas tree order

[David Veenstra]

Reorders the variable declarations to prefer a reverse Christmas tree
order to improve readability.

Signed-off-by: David Veenstra 
---
 drivers/staging/iio/resolver/ad2s1200.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/staging/iio/resolver/ad2s1200.c
index 7a5b2d1927d6..94d0a66532fd 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -47,9 +47,9 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 int *val2,
 long m)
 {
+   struct ad2s1200_state *st = iio_priv(indio_dev);
int ret = 0;
s16 vel;
-   struct ad2s1200_state *st = iio_priv(indio_dev);
 
mutex_lock(>lock);
gpio_set_value(st->sample, 0);
@@ -102,10 +102,10 @@ static const struct iio_info ad2s1200_info = {
 
 static int ad2s1200_probe(struct spi_device *spi)
 {
+   unsigned short *pins = spi->dev.platform_data;
struct ad2s1200_state *st;
struct iio_dev *indio_dev;
int pn, ret = 0;
-   unsigned short *pins = spi->dev.platform_data;
 
for (pn = 0; pn < AD2S1200_PN; pn++) {
ret = devm_gpio_request_one(>dev, pins[pn], GPIOF_DIR_OUT,
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/11] staging: iio: ad2s1200: Sort includes alphabetically

[David Veenstra]

This patches sorts all the includes in alphabetic order.

Signed-off-by: David Veenstra 
---
 drivers/staging/iio/resolver/ad2s1200.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c 
b/drivers/staging/iio/resolver/ad2s1200.c
index aa62c64e9bc4..7a5b2d1927d6 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -9,16 +9,16 @@
  * published by the Free Software Foundation.
  *
  */
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include 
 #include 
+#include 
 #include 
 #include 
-#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 #include 
 #include 
-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 00/11] staging: iio: ad2s1200: Driver clean up

[David Veenstra]

This patch series is meant clean the ad2s1200 driver, and move it
to mainline. In short the following is done:
1. Clean up of minor code style issues
2. Fix minor bugs
3. Replace legacy GPIO ABI with modern ABI
4. Change the channel definitions of angular position and angular
   velocity to match the sysfs IIO ABI.

The channel for angular position had to be changed to another type
as there is no definition for IIO_ANGL in sysfs IIO ABI. There are other
types that model some kind of rotation, the candidates are:
- in_rot_from_north_*
- in_rot_quaternion
- in_incli

The in_rot_from_north_*_raw family is a rotation relevative to the
geographical north, or the magnetic north. These semantics are not a
good fit for the angular position of a resolver. The
in_rot_quaternion_raw als does not seem like a good choice, as it is
overkill for a rotation over a single and fixed axis. This is also
likely to be a cubersome format for users of the ABI interface. Finally,
there is in_incli which seems to be meant for any kind of inclination,
given in degrees. In my opinion in_incli is the best choice, as
it has a conveniant format and has semantics similar to that of angular
position.

David Veenstra (11):
  staging: iio: ad2s1200: Sort includes alphabetically
  staging: iio: ad2s1200: Reverse Christmas tree order
  staging: iio: ad2s1200: Add blank lines
  staging: iio: ad2s1200: Add kernel docs to driver state
  staging: iio: ad2s1200: Introduce variable for repeated value
  staging: iio: ad2s1200: Improve readability with be16_to_cpup
  staging: iio: ad2s1200: Ensure udelay(1) in all necessary code paths
  staging: iio: ad2s1200: Replace legacy gpio ABI with modern ABI
  staging: iio: ad2s1200: Add scaling factor for IIO_ANGL_VEL channel
  staging: iio: ad2s1200: Replace angle channel with inclination channel
  Move resolver ad2c1200 driver out of staging to mainline iio

 drivers/iio/Kconfig   |   1 +
 drivers/iio/Makefile  |   1 +
 drivers/iio/resolver/Kconfig  |  17 +++
 drivers/iio/resolver/Makefile |   5 +
 drivers/{staging => }/iio/resolver/ad2s1200.c | 177 ++
 drivers/staging/iio/resolver/Kconfig  |  12 --
 drivers/staging/iio/resolver/Makefile |   1 -
 7 files changed, 151 insertions(+), 63 deletions(-)
 create mode 100644 drivers/iio/resolver/Kconfig
 create mode 100644 drivers/iio/resolver/Makefile
 rename drivers/{staging => }/iio/resolver/ad2s1200.c (50%)

-- 
2.16.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[Jonathan Cameron]

On Sat, 17 Mar 2018 23:11:45 -0700
John Syne  wrote:

> Hi Jonathan,
Hi John and All,

I'd love to get some additional input on this from anyone interested.
There are a lot of weird and wonderful derived quantities in an energy
meter and it seems we need to make some fundamental changes to support
them - including potentially a userspace breaking change to the event
code description.

> 
> Here is the complete list of registers for the ADE7878 which I copied from 
> the data sheet. I added a column “IIO Attribute” which I hope follows your 
> IIO ABI. Please make any changes you feel are incorrect. BTW, there are 
> several registers that cannot be generalized and are used purely for chip 
> configuration. I think we should add a new naming convention, namely 
> {register}_{}. Also, I see in the sys_bus_iio doc
> in_accel_x_peak_raw
> 
> so shouldn’t the phase be represented as follows:
> 
> in_current_A_scale
I'm still confused.  What does A represent here?  I assumed that was a wild
card for the channel number before but clearly not.

Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
I guess they sort of look like axis, and sort of like independent channels.
So could be indexed or done via modifiers depending on how you look at it.

Hmm. With neutral in there as well I guess we need to make them
modifiers (but might change my mind later ;)

Particularly as we are using the the modifier for RMS under the previous
plan... It appears we should treat that instead like we did for peak
and do it as an additional info mask element.  The problem with doing that
on a continuous measurement is that we can't treat it as a channel to
be output through the buffered interface.

So again we have run out of space. It's increasingly looking like we need
room for another field in the events - to cleanly represent computed values.

Hmm. What is the current usage? - it's been a while so I had to go
look in the header.

0-15 Channel (lots of channels)
31-16 Channel 2  (36 modifiers - lots of channels)
47-32 Channel Type (31 used so far - this looks most likely to run out of
space in the long run so leave this one alone).
54-48 Event Direction (4 used)
55 Differential  (1: channel 2 as differential pair 0: as a modifier)
63-56 Event Type (6 used)

So I think we can pinch bit 53 as another flag to indicate we have
a computed value or possibly bit 63 as event types are few and
far between as well.

Probably reasonable to assume we never have 16 bits worth
of channels and computed channels at the same time?
Hence I think we can steal bits off the top of Channel.
How many do we need?  Not sure unfortunately but feels like
8 should be plenty.

The other element of this is we add a new field to iio_chan_spec
to contain 'derived_type' or something like that which has
rms and sum squared etc. Over time we can move some of those
from the modifiers and free up a few entires there.
So modifier might be "X and Y and Z" with a derived_type of 
sum_squared to give existing sum_squared_x_y_z but no
rush on that.

Anyhow so now we have an extra element to play that will result
in a different channel.

Whilst here we should think about any other mods needed to
that event structure.  It is a little unfortunate that this
will be a breaking change for any old userspace code playing
with new drivers but it can't be helped as we didn't have
reserved values in the original definition (oops).

At somepoint we may need to add the 'shared by derived_value'
info mask but I think we can ignore that for now (seems
moderately unlikely to have anything in it!)
> 
> But for now, I followed your instructions from your reply.
> 
> After finalizing this one, I will work on the ADE9000, which as way more 
> registers ;-)
> 
> Once we can agree on the register naming, I will update the ADE7854 driver 
> for Rodrigo, which will go a long way to getting it out of staging.
I'll edit to fit with new scheme and insert indexes which I think would be
preferred though optional under the ABI as we only have one of each type/
> 
> Address   RegisterIIO Attribute   R/W Bit Length  Bit 
> Length During CommunicationsTypeDefault Value   Description
> 0x4380AIGAIN  in_current0_phaseA_scaleR/W 24  32 ZPSE 
> S   0x00Phase A current gain adjust.
A, B, C, N aren't obvious to the lay reader so I suggest we burn a few 
characters and stick phase in for ABC and just have neutral for
the neutral one. Not sure about capitalization or not though.

> 0x4381AVGAIN  in_voltage0_phaseA_scaleR/W 24  32 ZPSE 
> S   0x00Phase A voltage gain adjust.
> 0x4382BIGAIN  in_current0_phaseB_scaleR/W 24  32 ZPSE 
> S   0x00Phase B current gain adjust.
> 0x4383BVGAIN  in_voltage0_phaseB_scaleR/W 24  32 ZPSE 
> S   0x00Phase B voltage gain adjust.
> 0x4384CIGAIN  

Re: [PATCH v2] Staging: iio: adis16209: Move adis16209 driver out of staging

[Jonathan Cameron]

On Sun, 18 Mar 2018 15:18:44 +0530
Shreeya Patel  wrote:

> On 10 March 2018 21:27:31 GMT+05:30, Jonathan Cameron 
> 
> Hi Jonathan
> 
> >On Sat, 10 Mar 2018 15:50:23 +0530
> >Shreeya Patel  wrote:
> >  
> >> Move the adis16209 driver out of staging directory and merge to the
> >> mainline IIO subsystem.
> >> 
> >> Signed-off-by: Shreeya Patel   
> >As this has a clear dependency on the previous patch, please put them
> >in the same series for the next version.  That way I won't miss one!
> >
> >This also doesn't actually seem to have all the patches in place.
> >The sign extend one is definitely missing for some reason.
> >
> >One question on the ABI choice of X for the rotation axis.
> >I think the logical choice is actually Z but would like to know what
> >you and others think.
> >
> >All existing users of IIO_ROT (outside staging) have been magnetometer
> >where we don't have an axis, but rather a magnetic reference frame or
> >a quaternion output which includes all the axes.
> >
> >Jonathan
> >  
> >> ---
> >> 
> >> Changes in v2
> >>   -Re-send the patch after having some cleanups in the
> >> file included in this patch.
> >> 
> >>  drivers/iio/accel/Kconfig |  12 ++
> >>  drivers/iio/accel/Makefile|   1 +
> >>  drivers/iio/accel/adis16209.c | 329  
> >++  
> >>  drivers/staging/iio/accel/Kconfig |  12 --
> >>  drivers/staging/iio/accel/Makefile|   1 -
> >>  drivers/staging/iio/accel/adis16209.c | 329  
> >--  
> >>  6 files changed, 342 insertions(+), 342 deletions(-)
> >>  create mode 100644 drivers/iio/accel/adis16209.c
> >>  delete mode 100644 drivers/staging/iio/accel/adis16209.c
> >> 
> >> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> >> index c6d9517..f95f43c 100644
> >> --- a/drivers/iio/accel/Kconfig
> >> +++ b/drivers/iio/accel/Kconfig
> >> @@ -5,6 +5,18 @@
> >>  
> >>  menu "Accelerometers"
> >>  
> >> +config ADIS16209
> >> +tristate "Analog Devices ADIS16209 Dual-Axis Digital  
> >Inclinometer and Accelerometer"  
> >> +depends on SPI
> >> +select IIO_ADIS_LIB
> >> +select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
> >> +help
> >> +  Say Y here to build support for Analog Devices adis16209  
> >dual-axis digital inclinometer  
> >> +  and accelerometer.
> >> +
> >> +  To compile this driver as a module, say M here: the module  
> >will be  
> >> +  called adis16209.
> >> +
> >>  config ADXL345
> >>tristate
> >>  
> >> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> >> index 368aedb..40861b9 100644
> >> --- a/drivers/iio/accel/Makefile
> >> +++ b/drivers/iio/accel/Makefile
> >> @@ -4,6 +4,7 @@
> >>  #
> >>  
> >>  # When adding new entries keep the list in alphabetical order
> >> +obj-$(CONFIG_ADIS16209) += adis16209.o
> >>  obj-$(CONFIG_ADXL345) += adxl345_core.o
> >>  obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
> >>  obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> >> diff --git a/drivers/iio/accel/adis16209.c  
> >b/drivers/iio/accel/adis16209.c  
> >> new file mode 100644
> >> index 000..ed2e89f
> >> --- /dev/null
> >> +++ b/drivers/iio/accel/adis16209.c
> >> @@ -0,0 +1,329 @@
> >> +/*
> >> + * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer
> >> + *
> >> + * Copyright 2010 Analog Devices Inc.
> >> + *
> >> + * Licensed under the GPL-2 or later.
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#define ADIS16209_STARTUP_DELAY_MS220
> >> +#define ADIS16209_FLASH_CNT_REG   0x00
> >> +
> >> +/* Data Output Register Definitions */
> >> +#define ADIS16209_SUPPLY_OUT_REG  0x02
> >> +#define ADIS16209_XACCL_OUT_REG   0x04
> >> +#define ADIS16209_YACCL_OUT_REG   0x06
> >> +/* Output, auxiliary ADC input */
> >> +#define ADIS16209_AUX_ADC_REG 0x08
> >> +/* Output, temperature */
> >> +#define ADIS16209_TEMP_OUT_REG0x0A
> >> +/* Output, +/- 90 degrees X-axis inclination */
> >> +#define ADIS16209_XINCL_OUT_REG   0x0C
> >> +#define ADIS16209_YINCL_OUT_REG   0x0E
> >> +/* Output, +/-180 vertical rotational position */
> >> +#define ADIS16209_ROT_OUT_REG 0x10
> >> +
> >> +/*
> >> + * Calibration Register Definitions.
> >> + * Acceleration, inclination or rotation offset null.
> >> + */
> >> +#define ADIS16209_XACCL_NULL_REG  0x12
> >> +#define ADIS16209_YACCL_NULL_REG  0x14
> >> +#define ADIS16209_XINCL_NULL_REG  0x16
> >> +#define ADIS16209_YINCL_NULL_REG  0x18
> >> +#define ADIS16209_ROT_NULL_REG0x1A
> >> +
> >> +/* Alarm Register Definitions */
> >> +#define ADIS16209_ALM_MAG1_REG

Re: [PATCH v2 8/8] staging:iio:ade7854: Remove read_reg_* duplications

[Jonathan Cameron]

On Fri, 16 Mar 2018 19:50:29 -0300
Rodrigo Siqueira  wrote:

> The original code had a read function per data size; after updates, all
> read functions tasks were centralized in a single function, but the old
> signature was kept to maintain the module working without problems. This
> patch removes a set of duplications associated with read_reg_*, and
> update the areas that calling the old interface by the new one. During
> the update for use a single function, some errors handlings were updated
> based on the John Syne patches.
> 
> Signed-off-by: Rodrigo Siqueira 
> Signed-off-by: John Syne 
The meaning of John's signed-off-by here needs clarifying..

Also fix up the common on error handling as I believe you now do that in
an earlier patch.

Thanks and looking forward to v3.  This is a nice little bit of cleanup.
These weird devices with variable register sizes are always somewhat of
a pain to deal with.

Jonathan

> ---
>  drivers/staging/iio/meter/ade7854-i2c.c | 37 +--
>  drivers/staging/iio/meter/ade7854-spi.c | 39 
> ++---
>  drivers/staging/iio/meter/ade7854.c | 18 +++
>  drivers/staging/iio/meter/ade7854.h |  7 +++---
>  4 files changed, 15 insertions(+), 86 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index 20db8eedb84a..162c171a8851 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -110,38 +110,6 @@ static int ade7854_i2c_read_reg(struct device *dev,
>   return ret;
>  }
>  
> -static int ade7854_i2c_read_reg_8(struct device *dev,
> -   u16 reg_address,
> -   u8 *val)
> -{
> - return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val,
> - DATA_SIZE_8_BITS);
> -}
> -
> -static int ade7854_i2c_read_reg_16(struct device *dev,
> -u16 reg_address,
> -u16 *val)
> -{
> - return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val,
> - DATA_SIZE_16_BITS);
> -}
> -
> -static int ade7854_i2c_read_reg_24(struct device *dev,
> -u16 reg_address,
> -u32 *val)
> -{
> - return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val,
> - DATA_SIZE_24_BITS);
> -}
> -
> -static int ade7854_i2c_read_reg_32(struct device *dev,
> -u16 reg_address,
> -u32 *val)
> -{
> - return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val,
> - DATA_SIZE_32_BITS);
> -}
> -
>  static int ade7854_i2c_probe(struct i2c_client *client,
>const struct i2c_device_id *id)
>  {
> @@ -153,10 +121,7 @@ static int ade7854_i2c_probe(struct i2c_client *client,
>   return -ENOMEM;
>   st = iio_priv(indio_dev);
>   i2c_set_clientdata(client, indio_dev);
> - st->read_reg_8 = ade7854_i2c_read_reg_8;
> - st->read_reg_16 = ade7854_i2c_read_reg_16;
> - st->read_reg_24 = ade7854_i2c_read_reg_24;
> - st->read_reg_32 = ade7854_i2c_read_reg_32;
> + st->read_reg = ade7854_i2c_read_reg;
>   st->write_reg = ade7854_i2c_write_reg;
>   st->i2c = client;
>   st->irq = client->irq;
> diff --git a/drivers/staging/iio/meter/ade7854-spi.c 
> b/drivers/staging/iio/meter/ade7854-spi.c
> index 964d6c6e76d1..66b8a8767a26 100644
> --- a/drivers/staging/iio/meter/ade7854-spi.c
> +++ b/drivers/staging/iio/meter/ade7854-spi.c
> @@ -94,7 +94,7 @@ static int ade7854_spi_read_reg(struct device *dev,
>   st->tx[2] = reg_address & 0xFF;
>  
>   ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> - if (ret) {
> + if (ret < 0) {
>   dev_err(>spi->dev, "problem when reading register 0x%02X",
>   reg_address);
>   goto unlock;
> @@ -120,38 +120,6 @@ static int ade7854_spi_read_reg(struct device *dev,
>   return ret;
>  }
>  
> -static int ade7854_spi_read_reg_8(struct device *dev,
> -   u16 reg_address,
> -   u8 *val)
> -{
> - return ade7854_spi_read_reg(dev, reg_address, (u32 *)val,
> - DATA_SIZE_8_BITS);
> -}
> -
> -static int ade7854_spi_read_reg_16(struct device *dev,
> -u16 reg_address,
> -u16 *val)
> -{
> - return ade7854_spi_read_reg(dev, reg_address, (u32 *)val,
> - DATA_SIZE_16_BITS);
> -}
> -
> -static int ade7854_spi_read_reg_24(struct device *dev,
> -u16 reg_address,
> -u32 *val)
> -{
> - return 

Re: [PATCH v2 6/8] staging:iio:ade7854: Rework I2C read function

[Jonathan Cameron]

On Fri, 16 Mar 2018 19:49:59 -0300
Rodrigo Siqueira  wrote:

> The read operation for the I2C function has many duplications that can
> be generalized into a single function. This patch reworks the read
> operation for I2C to centralizes all similar code in a single function.
> Part of the rework includes a proper error handling and a fix on the
> i2c_master_recv for 32 bits as pointed by John Syne patches.
This seems likely to have been covered by the earlier patch. Please update
the description.

Otherwise same define comment but beyond that looks good.

Jonathan

> 
> It is possible to remove all the old interface to use the new one,
> however, for keeping the things simple and working this patch maintain
> legacy interface.
> 
> Signed-off-by: Rodrigo Siqueira 
> Signed-off-by: John Syne 
> ---
>  drivers/staging/iio/meter/ade7854-i2c.c | 110 
> 
>  1 file changed, 41 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index e95147a1bac1..20db8eedb84a 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -65,9 +65,10 @@ static int ade7854_i2c_write_reg(struct device *dev,
>   return ret < 0 ? ret : 0;
>  }
>  
> -static int ade7854_i2c_read_reg_8(struct device *dev,
> -   u16 reg_address,
> -   u8 *val)
> +static int ade7854_i2c_read_reg(struct device *dev,
> + u16 reg_address,
> + u32 *val,
> + int bytes)
>  {
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>   struct ade7854_state *st = iio_priv(indio_dev);
> @@ -79,95 +80,66 @@ static int ade7854_i2c_read_reg_8(struct device *dev,
>  
>   ret = i2c_master_send(st->i2c, st->tx, 2);
>   if (ret < 0)
> - goto out;
> + goto unlock;
>  
> - ret = i2c_master_recv(st->i2c, st->rx, 1);
> + ret = i2c_master_recv(st->i2c, st->rx, bytes);
>   if (ret < 0)
> - goto out;
> + goto unlock;
> +
> + switch (bytes) {
> + case DATA_SIZE_8_BITS:
> + *val = st->rx[0];
> + break;
> + case DATA_SIZE_16_BITS:
> + *val = (st->rx[0] << 8) | st->rx[1];
> + break;
> + case DATA_SIZE_24_BITS:
> + *val = (st->rx[0] << 16) | (st->rx[1] << 8) | st->rx[2];
> + break;
> + case DATA_SIZE_32_BITS:
> + *val = (st->rx[0] << 24) | (st->rx[1] << 16) |
> + (st->rx[2] << 8) | st->rx[3];
> + break;
> + default:
> + ret = -EINVAL;
> + goto unlock;
> + }
>  
> - *val = st->rx[0];
> -out:
> +unlock:
>   mutex_unlock(>buf_lock);
>   return ret;
>  }
>  
> +static int ade7854_i2c_read_reg_8(struct device *dev,
> +   u16 reg_address,
> +   u8 *val)
> +{
> + return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val,
> + DATA_SIZE_8_BITS);
> +}
> +
>  static int ade7854_i2c_read_reg_16(struct device *dev,
>  u16 reg_address,
>  u16 *val)
>  {
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - int ret;
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = reg_address & 0xFF;
> -
> - ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret < 0)
> - goto out;
> -
> - ret = i2c_master_recv(st->i2c, st->rx, 2);
> - if (ret < 0)
> - goto out;
> -
> - *val = (st->rx[0] << 8) | st->rx[1];
> -out:
> - mutex_unlock(>buf_lock);
> - return ret;
> + return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val,
> + DATA_SIZE_16_BITS);
>  }
>  
>  static int ade7854_i2c_read_reg_24(struct device *dev,
>  u16 reg_address,
>  u32 *val)
>  {
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - int ret;
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = reg_address & 0xFF;
> -
> - ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret < 0)
> - goto out;
> -
> - ret = i2c_master_recv(st->i2c, st->rx, 3);
> - if (ret < 0)
> - goto out;
> -
> - *val = (st->rx[0] << 16) | (st->rx[1] << 8) | st->rx[2];
> -out:
> - mutex_unlock(>buf_lock);
> - return ret;
> + return ade7854_i2c_read_reg(dev, reg_address, (u32 *)val,
> + DATA_SIZE_24_BITS);
>  }
>  
>  static int 

Re: [PATCH v2 5/8] staging:iio:ade7854: Replace many functions for one function

[Jonathan Cameron]

On Fri, 16 Mar 2018 19:49:40 -0300
Rodrigo Siqueira  wrote:

> This patch removes code duplications related to the write_reg_*
> functions and centralizes them in a single function. Also, it eliminates
> the legacy functions and replaces them by a unique signature that is
> used by SPI and I2C.
> 
> Signed-off-by: Rodrigo Siqueira 
Makes sense and a nice reduction in lines of code.

Will pick up once the precursors are ready.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/meter/ade7854-i2c.c | 33 
> +
>  drivers/staging/iio/meter/ade7854-spi.c | 33 
> +
>  drivers/staging/iio/meter/ade7854.c | 12 ++--
>  drivers/staging/iio/meter/ade7854.h |  9 -
>  4 files changed, 12 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index 1daad42b1e92..e95147a1bac1 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -65,34 +65,6 @@ static int ade7854_i2c_write_reg(struct device *dev,
>   return ret < 0 ? ret : 0;
>  }
>  
> -static int ade7854_i2c_write_reg_8(struct device *dev,
> -u16 reg_address,
> -u8 val)
> -{
> - return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS);
> -}
> -
> -static int ade7854_i2c_write_reg_16(struct device *dev,
> - u16 reg_address,
> - u16 val)
> -{
> - return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS);
> -}
> -
> -static int ade7854_i2c_write_reg_24(struct device *dev,
> - u16 reg_address,
> - u32 val)
> -{
> - return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_24_BITS);
> -}
> -
> -static int ade7854_i2c_write_reg_32(struct device *dev,
> - u16 reg_address,
> - u32 val)
> -{
> - return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_32_BITS);
> -}
> -
>  static int ade7854_i2c_read_reg_8(struct device *dev,
> u16 reg_address,
> u8 *val)
> @@ -213,10 +185,7 @@ static int ade7854_i2c_probe(struct i2c_client *client,
>   st->read_reg_16 = ade7854_i2c_read_reg_16;
>   st->read_reg_24 = ade7854_i2c_read_reg_24;
>   st->read_reg_32 = ade7854_i2c_read_reg_32;
> - st->write_reg_8 = ade7854_i2c_write_reg_8;
> - st->write_reg_16 = ade7854_i2c_write_reg_16;
> - st->write_reg_24 = ade7854_i2c_write_reg_24;
> - st->write_reg_32 = ade7854_i2c_write_reg_32;
> + st->write_reg = ade7854_i2c_write_reg;
>   st->i2c = client;
>   st->irq = client->irq;
>  
> diff --git a/drivers/staging/iio/meter/ade7854-spi.c 
> b/drivers/staging/iio/meter/ade7854-spi.c
> index f21dc24194fb..1eba3044b86f 100644
> --- a/drivers/staging/iio/meter/ade7854-spi.c
> +++ b/drivers/staging/iio/meter/ade7854-spi.c
> @@ -67,34 +67,6 @@ static int ade7854_spi_write_reg(struct device *dev,
>   return ret;
>  }
>  
> -static int ade7854_spi_write_reg_8(struct device *dev,
> -u16 reg_address,
> -u8 val)
> -{
> - return ade7854_spi_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS);
> -}
> -
> -static int ade7854_spi_write_reg_16(struct device *dev,
> - u16 reg_address,
> - u16 val)
> -{
> - return ade7854_spi_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS);
> -}
> -
> -static int ade7854_spi_write_reg_24(struct device *dev,
> - u16 reg_address,
> - u32 val)
> -{
> - return ade7854_spi_write_reg(dev, reg_address, val, DATA_SIZE_24_BITS);
> -}
> -
> -static int ade7854_spi_write_reg_32(struct device *dev,
> - u16 reg_address,
> - u32 val)
> -{
> - return ade7854_spi_write_reg(dev, reg_address, val, DATA_SIZE_32_BITS);
> -}
> -
>  static int ade7854_spi_read_reg_8(struct device *dev,
> u16 reg_address,
> u8 *val)
> @@ -260,10 +232,7 @@ static int ade7854_spi_probe(struct spi_device *spi)
>   st->read_reg_16 = ade7854_spi_read_reg_16;
>   st->read_reg_24 = ade7854_spi_read_reg_24;
>   st->read_reg_32 = ade7854_spi_read_reg_32;
> - st->write_reg_8 = ade7854_spi_write_reg_8;
> - st->write_reg_16 = ade7854_spi_write_reg_16;
> - st->write_reg_24 = ade7854_spi_write_reg_24;
> - st->write_reg_32 = ade7854_spi_write_reg_32;
> + st->write_reg = ade7854_spi_write_reg;
>   st->irq = spi->irq;
>   st->spi = spi;
>  
> diff --git 

Re: [PATCH v2 4/8] staging:iio:ade7854: Rework SPI write function

[Jonathan Cameron]

On Fri, 16 Mar 2018 19:49:24 -0300
Rodrigo Siqueira  wrote:

> The write operation using SPI has a many code duplications (similar to
> I2C) and four different interfaces per data size. This patch introduces
> a single function that centralizes the main task related to SPI.
> 
> Signed-off-by: Rodrigo Siqueira 
A good change, but same comment on the defines as for the i2c case.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/meter/ade7854-spi.c | 108 
> 
>  1 file changed, 41 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-spi.c 
> b/drivers/staging/iio/meter/ade7854-spi.c
> index 4419b8f06197..f21dc24194fb 100644
> --- a/drivers/staging/iio/meter/ade7854-spi.c
> +++ b/drivers/staging/iio/meter/ade7854-spi.c
> @@ -15,9 +15,10 @@
>  #include 
>  #include "ade7854.h"
>  
> -static int ade7854_spi_write_reg_8(struct device *dev,
> -u16 reg_address,
> -u8 val)
> +static int ade7854_spi_write_reg(struct device *dev,
> +  u16 reg_address,
> +  u32 val,
> +  int bytes)
>  {
>   int ret;
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> @@ -32,93 +33,66 @@ static int ade7854_spi_write_reg_8(struct device *dev,
>   st->tx[0] = ADE7854_WRITE_REG;
>   st->tx[1] = (reg_address >> 8) & 0xFF;
>   st->tx[2] = reg_address & 0xFF;
> - st->tx[3] = val & 0xFF;
> + switch (bytes) {
> + case DATA_SIZE_8_BITS:
> + st->tx[3] = val & 0xFF;
> + break;
> + case DATA_SIZE_16_BITS:
> + xfer.len = 5;
> + st->tx[3] = (val >> 8) & 0xFF;
> + st->tx[4] = val & 0xFF;
> + break;
> + case DATA_SIZE_24_BITS:
> + xfer.len = 6;
> + st->tx[3] = (val >> 16) & 0xFF;
> + st->tx[4] = (val >> 8) & 0xFF;
> + st->tx[5] = val & 0xFF;
> + break;
> + case DATA_SIZE_32_BITS:
> + xfer.len = 7;
> + st->tx[3] = (val >> 24) & 0xFF;
> + st->tx[4] = (val >> 16) & 0xFF;
> + st->tx[5] = (val >> 8) & 0xFF;
> + st->tx[6] = val & 0xFF;
> + break;
> + default:
> + ret = -EINVAL;
> + goto unlock;
> + }
>  
>   ret = spi_sync_transfer(st->spi, , 1);
> +unlock:
>   mutex_unlock(>buf_lock);
>  
>   return ret;
>  }
>  
> +static int ade7854_spi_write_reg_8(struct device *dev,
> +u16 reg_address,
> +u8 val)
> +{
> + return ade7854_spi_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS);
> +}
> +
>  static int ade7854_spi_write_reg_16(struct device *dev,
>   u16 reg_address,
>   u16 val)
>  {
> - int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - struct spi_transfer xfer = {
> - .tx_buf = st->tx,
> - .bits_per_word = 8,
> - .len = 5,
> - };
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = ADE7854_WRITE_REG;
> - st->tx[1] = (reg_address >> 8) & 0xFF;
> - st->tx[2] = reg_address & 0xFF;
> - st->tx[3] = (val >> 8) & 0xFF;
> - st->tx[4] = val & 0xFF;
> -
> - ret = spi_sync_transfer(st->spi, , 1);
> - mutex_unlock(>buf_lock);
> -
> - return ret;
> + return ade7854_spi_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS);
>  }
>  
>  static int ade7854_spi_write_reg_24(struct device *dev,
>   u16 reg_address,
>   u32 val)
>  {
> - int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - struct spi_transfer xfer = {
> - .tx_buf = st->tx,
> - .bits_per_word = 8,
> - .len = 6,
> - };
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = ADE7854_WRITE_REG;
> - st->tx[1] = (reg_address >> 8) & 0xFF;
> - st->tx[2] = reg_address & 0xFF;
> - st->tx[3] = (val >> 16) & 0xFF;
> - st->tx[4] = (val >> 8) & 0xFF;
> - st->tx[5] = val & 0xFF;
> -
> - ret = spi_sync_transfer(st->spi, , 1);
> - mutex_unlock(>buf_lock);
> -
> - return ret;
> + return ade7854_spi_write_reg(dev, reg_address, val, DATA_SIZE_24_BITS);
>  }
>  
>  static int ade7854_spi_write_reg_32(struct device *dev,
>   u16 reg_address,
>   u32 val)
>  {
> - int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> - struct spi_transfer xfer = {
> - .tx_buf = st->tx,
> - .bits_per_word = 8,
> - .len 

Re: [PATCH v2 3/8] staging:iio:ade7854: Rework I2C write function

[Jonathan Cameron]

On Fri, 16 Mar 2018 19:49:08 -0300
Rodrigo Siqueira  wrote:

> The write operation using I2C has many code duplications and four
> different interfaces per data size. This patch introduces a single
> function that centralizes the main tasks.
> 
> The central function inserted by this patch can easily replace all the
> four functions related to the data size. However, this patch does not
> remove any code signature for keeping the meter module work and make
> easier to review this patch.
> 
> Signed-off-by: Rodrigo Siqueira 
> ---
>  drivers/staging/iio/meter/ade7854-i2c.c | 96 
> -
>  drivers/staging/iio/meter/ade7854.h |  7 +++
>  2 files changed, 53 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index 37c957482493..1daad42b1e92 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -15,86 +15,82 @@
>  #include 
>  #include "ade7854.h"
>  
> -static int ade7854_i2c_write_reg_8(struct device *dev,
> -u16 reg_address,
> -u8 val)
> +static int ade7854_i2c_write_reg(struct device *dev,
> +  u16 reg_address,
> +  u32 val,
> +  int bytes)
>  {
>   int ret;
> + int count;
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>   struct ade7854_state *st = iio_priv(indio_dev);
>  
>   mutex_lock(>buf_lock);
>   st->tx[0] = (reg_address >> 8) & 0xFF;
>   st->tx[1] = reg_address & 0xFF;
There are a lot of endian conversions in here, but as some aren't
standard sizes (24 bits) and others are unaligned which makes things
messy, perhaps we are better doing it 'long hand' like this.

> - st->tx[2] = val;
>  
> - ret = i2c_master_send(st->i2c, st->tx, 3);
> + switch (bytes) {
> + case DATA_SIZE_8_BITS:

The defines really don't help here.  These are real numbers
so have them as such.

> + st->tx[2] = val & 0xFF;
> + count = 3;
> + break;
> + case DATA_SIZE_16_BITS:
> + st->tx[2] = (val >> 8) & 0xFF;
> + st->tx[3] = val & 0xFF;
> + count = 4;
> + break;
> + case DATA_SIZE_24_BITS:
> + st->tx[2] = (val >> 16) & 0xFF;
> + st->tx[3] = (val >> 8) & 0xFF;
> + st->tx[4] = val & 0xFF;
> + count = 5;
> + break;
> + case DATA_SIZE_32_BITS:
> + st->tx[2] = (val >> 24) & 0xFF;
> + st->tx[3] = (val >> 16) & 0xFF;
> + st->tx[4] = (val >> 8) & 0xFF;
> + st->tx[5] = val & 0xFF;
> + count = 6;
> + break;
> + default:
> + ret = -EINVAL;
> + goto unlock;
> + }
> +
> + ret = i2c_master_send(st->i2c, st->tx, count);
> +
> +unlock:
>   mutex_unlock(>buf_lock);
>  
>   return ret < 0 ? ret : 0;
>  }
>  
> +static int ade7854_i2c_write_reg_8(struct device *dev,
> +u16 reg_address,
> +u8 val)
> +{
> + return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS);
> +}
> +
>  static int ade7854_i2c_write_reg_16(struct device *dev,
>   u16 reg_address,
>   u16 val)
>  {
> - int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = reg_address & 0xFF;
> - st->tx[2] = (val >> 8) & 0xFF;
> - st->tx[3] = val & 0xFF;
> -
> - ret = i2c_master_send(st->i2c, st->tx, 4);
> - mutex_unlock(>buf_lock);
> -
> - return ret < 0 ? ret : 0;
> + return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS);
>  }
>  
>  static int ade7854_i2c_write_reg_24(struct device *dev,
>   u16 reg_address,
>   u32 val)
>  {
> - int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
> -
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = reg_address & 0xFF;
> - st->tx[2] = (val >> 16) & 0xFF;
> - st->tx[3] = (val >> 8) & 0xFF;
> - st->tx[4] = val & 0xFF;
> -
> - ret = i2c_master_send(st->i2c, st->tx, 5);
> - mutex_unlock(>buf_lock);
> -
> - return ret < 0 ? ret : 0;
> + return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_24_BITS);
>  }
>  
>  static int ade7854_i2c_write_reg_32(struct device *dev,
>   u16 reg_address,
>   u32 val)
>  {
> - int ret;
> - struct 

Re: [PATCH v2 2/8] staging:iio:ade7854: Fix the wrong number of bits to read

[Jonathan Cameron]

On Fri, 16 Mar 2018 19:48:51 -0300
Rodrigo Siqueira  wrote:

> The function ade7854_i2c_read_reg_32() have to invoke the
> i2c_master_recv() for read 32 bits values, however, the counter is set
> to 3 which means 24 bits. This patch fixes the wrong size of 24 bits, to
> 32 bits. Finally, this patch is based on John Syne patches.
> 
> Signed-off-by: Rodrigo Siqueira 
> Signed-off-by: John Syne 
Same issue with clarifying the authorship.  Also for these fixes
ideally include a 'fixes' tag to the original commit. It makes
life easy for the scripts the stable maintainers use to work
out when things should apply.

Here I would imagine it is the original commit that added the driver,
though we may have had enough changes over the years to make these
hard to apply.

Jonathan

> ---
>  drivers/staging/iio/meter/ade7854-i2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index 4437f1e33261..37c957482493 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -191,7 +191,7 @@ static int ade7854_i2c_read_reg_32(struct device *dev,
>   if (ret < 0)
>   goto out;
>  
> - ret = i2c_master_recv(st->i2c, st->rx, 3);
> + ret = i2c_master_recv(st->i2c, st->rx, 4);
>   if (ret < 0)
>   goto out;
>  

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] Staging: iio: adis16209: Move adis16209 driver out of staging

[Shreeya Patel]

On 10 March 2018 21:27:31 GMT+05:30, Jonathan Cameron 

Hi Jonathan

>On Sat, 10 Mar 2018 15:50:23 +0530
>Shreeya Patel  wrote:
>
>> Move the adis16209 driver out of staging directory and merge to the
>> mainline IIO subsystem.
>> 
>> Signed-off-by: Shreeya Patel 
>As this has a clear dependency on the previous patch, please put them
>in the same series for the next version.  That way I won't miss one!
>
>This also doesn't actually seem to have all the patches in place.
>The sign extend one is definitely missing for some reason.
>
>One question on the ABI choice of X for the rotation axis.
>I think the logical choice is actually Z but would like to know what
>you and others think.
>
>All existing users of IIO_ROT (outside staging) have been magnetometer
>where we don't have an axis, but rather a magnetic reference frame or
>a quaternion output which includes all the axes.
>
>Jonathan
>
>> ---
>> 
>> Changes in v2
>>   -Re-send the patch after having some cleanups in the
>> file included in this patch.
>> 
>>  drivers/iio/accel/Kconfig |  12 ++
>>  drivers/iio/accel/Makefile|   1 +
>>  drivers/iio/accel/adis16209.c | 329
>++
>>  drivers/staging/iio/accel/Kconfig |  12 --
>>  drivers/staging/iio/accel/Makefile|   1 -
>>  drivers/staging/iio/accel/adis16209.c | 329
>--
>>  6 files changed, 342 insertions(+), 342 deletions(-)
>>  create mode 100644 drivers/iio/accel/adis16209.c
>>  delete mode 100644 drivers/staging/iio/accel/adis16209.c
>> 
>> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>> index c6d9517..f95f43c 100644
>> --- a/drivers/iio/accel/Kconfig
>> +++ b/drivers/iio/accel/Kconfig
>> @@ -5,6 +5,18 @@
>>  
>>  menu "Accelerometers"
>>  
>> +config ADIS16209
>> +tristate "Analog Devices ADIS16209 Dual-Axis Digital
>Inclinometer and Accelerometer"
>> +depends on SPI
>> +select IIO_ADIS_LIB
>> +select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
>> +help
>> +  Say Y here to build support for Analog Devices adis16209
>dual-axis digital inclinometer
>> +  and accelerometer.
>> +
>> +  To compile this driver as a module, say M here: the module
>will be
>> +  called adis16209.
>> +
>>  config ADXL345
>>  tristate
>>  
>> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
>> index 368aedb..40861b9 100644
>> --- a/drivers/iio/accel/Makefile
>> +++ b/drivers/iio/accel/Makefile
>> @@ -4,6 +4,7 @@
>>  #
>>  
>>  # When adding new entries keep the list in alphabetical order
>> +obj-$(CONFIG_ADIS16209) += adis16209.o
>>  obj-$(CONFIG_ADXL345) += adxl345_core.o
>>  obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
>>  obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
>> diff --git a/drivers/iio/accel/adis16209.c
>b/drivers/iio/accel/adis16209.c
>> new file mode 100644
>> index 000..ed2e89f
>> --- /dev/null
>> +++ b/drivers/iio/accel/adis16209.c
>> @@ -0,0 +1,329 @@
>> +/*
>> + * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer
>> + *
>> + * Copyright 2010 Analog Devices Inc.
>> + *
>> + * Licensed under the GPL-2 or later.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define ADIS16209_STARTUP_DELAY_MS  220
>> +#define ADIS16209_FLASH_CNT_REG 0x00
>> +
>> +/* Data Output Register Definitions */
>> +#define ADIS16209_SUPPLY_OUT_REG0x02
>> +#define ADIS16209_XACCL_OUT_REG 0x04
>> +#define ADIS16209_YACCL_OUT_REG 0x06
>> +/* Output, auxiliary ADC input */
>> +#define ADIS16209_AUX_ADC_REG   0x08
>> +/* Output, temperature */
>> +#define ADIS16209_TEMP_OUT_REG  0x0A
>> +/* Output, +/- 90 degrees X-axis inclination */
>> +#define ADIS16209_XINCL_OUT_REG 0x0C
>> +#define ADIS16209_YINCL_OUT_REG 0x0E
>> +/* Output, +/-180 vertical rotational position */
>> +#define ADIS16209_ROT_OUT_REG   0x10
>> +
>> +/*
>> + * Calibration Register Definitions.
>> + * Acceleration, inclination or rotation offset null.
>> + */
>> +#define ADIS16209_XACCL_NULL_REG0x12
>> +#define ADIS16209_YACCL_NULL_REG0x14
>> +#define ADIS16209_XINCL_NULL_REG0x16
>> +#define ADIS16209_YINCL_NULL_REG0x18
>> +#define ADIS16209_ROT_NULL_REG  0x1A
>> +
>> +/* Alarm Register Definitions */
>> +#define ADIS16209_ALM_MAG1_REG  0x20
>> +#define ADIS16209_ALM_MAG2_REG  0x22
>> +#define ADIS16209_ALM_SMPL1_REG 0x24
>> +#define ADIS16209_ALM_SMPL2_REG 0x26
>> +#define ADIS16209_ALM_CTRL_REG  0x28

I see that these alarm registers are not being used anywhere in the driver and 
yet we have it's declaration. 
Is it the left out work that will be done in 

Re: [PATCH v2 1/8] staging:iio:ade7854: Fix error handling on read/write

[Jonathan Cameron]

On Fri, 16 Mar 2018 19:48:33 -0300
Rodrigo Siqueira  wrote:

> The original code does not correctly handle the error related to I2C
> read and write. This patch fixes the error handling related to all
> read/write functions for I2C. This patch is an adaptation of the John
> Syne patches.
> 
> Signed-off-by: Rodrigo Siqueira 
> Signed-off-by: John Syne 
Hi Rodrigo,

I'm not sure what the chain of authorship was here.  If this is fundamentally
John's original patch he should still be the author and his sign off should be
first.  You then sign off afterwards to indicate that you 'handled' the patch
and believe the work to be John's (you are trusting his sign off).  This
is 'fun' legal stuff - read the docs on developers certificate of origin.

If the patch has changed 'enough' (where that is a fuzzy definition)
then you should as you have here take the authorship, but John's sign off is
no longer true (it's a different patch).  If John has reviewed the code
it is fine to have a reviewed-by or acked-by from John there to reflect
that.

Anyhow, please clarify the situation as I shouldn't take a patch where
I'm applying my sign-off without knowing the origins etc.

> ---
>  drivers/staging/iio/meter/ade7854-i2c.c | 24 
>  drivers/staging/iio/meter/ade7854.c | 10 +-
>  2 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index 317e4f0d8176..4437f1e33261 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -31,7 +31,7 @@ static int ade7854_i2c_write_reg_8(struct device *dev,
>   ret = i2c_master_send(st->i2c, st->tx, 3);
>   mutex_unlock(>buf_lock);
>  
> - return ret;
> + return ret < 0 ? ret : 0;
>  }
>  
>  static int ade7854_i2c_write_reg_16(struct device *dev,
> @@ -51,7 +51,7 @@ static int ade7854_i2c_write_reg_16(struct device *dev,
>   ret = i2c_master_send(st->i2c, st->tx, 4);
>   mutex_unlock(>buf_lock);
>  
> - return ret;
> + return ret < 0 ? ret : 0;
>  }
>  
>  static int ade7854_i2c_write_reg_24(struct device *dev,
> @@ -72,7 +72,7 @@ static int ade7854_i2c_write_reg_24(struct device *dev,
>   ret = i2c_master_send(st->i2c, st->tx, 5);
>   mutex_unlock(>buf_lock);
>  
> - return ret;
> + return ret < 0 ? ret : 0;
>  }
>  
>  static int ade7854_i2c_write_reg_32(struct device *dev,
> @@ -94,7 +94,7 @@ static int ade7854_i2c_write_reg_32(struct device *dev,
>   ret = i2c_master_send(st->i2c, st->tx, 6);
>   mutex_unlock(>buf_lock);
>  
> - return ret;
> + return ret < 0 ? ret : 0;
>  }
So for write cases you are flattening to 0 for good and < 0 for bad.
good.
>  
>  static int ade7854_i2c_read_reg_8(struct device *dev,
> @@ -110,11 +110,11 @@ static int ade7854_i2c_read_reg_8(struct device *dev,
>   st->tx[1] = reg_address & 0xFF;
>  
>   ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   ret = i2c_master_recv(st->i2c, st->rx, 1);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   *val = st->rx[0];
But in read cases you are returning the number of bytes read...
Given these functions can know the 'right' answer to that why not check
it here and do the same as for writes in return 0 for good and < 0 for
bad?
> @@ -136,11 +136,11 @@ static int ade7854_i2c_read_reg_16(struct device *dev,
>   st->tx[1] = reg_address & 0xFF;
>  
>   ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   ret = i2c_master_recv(st->i2c, st->rx, 2);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   *val = (st->rx[0] << 8) | st->rx[1];
> @@ -162,11 +162,11 @@ static int ade7854_i2c_read_reg_24(struct device *dev,
>   st->tx[1] = reg_address & 0xFF;
>  
>   ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   ret = i2c_master_recv(st->i2c, st->rx, 3);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   *val = (st->rx[0] << 16) | (st->rx[1] << 8) | st->rx[2];
> @@ -188,11 +188,11 @@ static int ade7854_i2c_read_reg_32(struct device *dev,
>   st->tx[1] = reg_address & 0xFF;
>  
>   ret = i2c_master_send(st->i2c, st->tx, 2);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   ret = i2c_master_recv(st->i2c, st->rx, 3);
> - if (ret)
> + if (ret < 0)
>   goto out;
>  
>   *val = (st->rx[0] << 24) | (st->rx[1] << 16) |
> diff --git a/drivers/staging/iio/meter/ade7854.c 
> b/drivers/staging/iio/meter/ade7854.c
> index 90d07cdca4b8..0193ae3aae29 100644
> --- a/drivers/staging/iio/meter/ade7854.c
> +++ b/drivers/staging/iio/meter/ade7854.c
> @@ -33,7 +33,7 @@ 

Re: [PATCH v2 9/9] Staging: iio: accel: adis16201: Move adis16201 driver out of staging

[Jonathan Cameron]

On Sat, 17 Mar 2018 01:36:26 +0530
Himanshu Jha  wrote:

> Move the adis16201 driver out of staging directory and merge to the
> mainline IIO subsystem.
> 
> Signed-off-by: Himanshu Jha 
Hi Himanshu,

You have made great progress on this, but this final posting of a patch moving
a driver out of staging always throws up a few more things as we take a closer
look than happens when there is still lots to do.

It is a bit like the fact most newly posted drivers pick up detailed review
comments a few versions in that weren't raised before.

Anyhow nothing major here and it did make me wonder how many unused headers
we have in drivers not in staging due to cleanups making them irrelevant.

Anyhow, a few more things inline.  I 'nearly' went with applying the
move anyway, but it is nice to take this opportunity to tidy up just that little
bit more.

Thanks,

Jonathan

> ---
> v2:
>-no change in this patch.
> 
>  drivers/iio/accel/Kconfig |  12 ++
>  drivers/iio/accel/Makefile|   1 +
>  drivers/iio/accel/adis16201.c | 327 
> ++
>  drivers/staging/iio/accel/Kconfig |  12 --
>  drivers/staging/iio/accel/Makefile|   1 -
>  drivers/staging/iio/accel/adis16201.c | 327 
> --
>  6 files changed, 340 insertions(+), 340 deletions(-)
>  create mode 100644 drivers/iio/accel/adis16201.c
>  delete mode 100644 drivers/staging/iio/accel/adis16201.c
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index c6d9517..9416c6f 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -5,6 +5,18 @@
>  
>  menu "Accelerometers"
>  
> +config ADIS16201
> +tristate "Analog Devices ADIS16201 Dual-Axis Digital Inclinometer 
> and Accelerometer"
> +depends on SPI
> +select IIO_ADIS_LIB
> +select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
> +help
> +  Say Y here to build support for Analog Devices adis16201 dual-axis
> +  digital inclinometer and accelerometer.
> +
> +  To compile this driver as a module, say M here: the module will
> +  be called adis16201.
> +
>  config ADXL345
>   tristate
>  
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 368aedb..7832ec9 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -4,6 +4,7 @@
>  #
>  
>  # When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_ADIS16201) += adis16201.o
>  obj-$(CONFIG_ADXL345) += adxl345_core.o
>  obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
>  obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
> new file mode 100644
> index 000..e439cf7
> --- /dev/null
> +++ b/drivers/iio/accel/adis16201.c
> @@ -0,0 +1,327 @@
> +/*
> + * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
> + *
> + * Copyright 2010 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include 
Not used in this file. The delay is done by the adis helper modules
not here.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
Seeing this sysfs include made me wonder if we are including this in other 
places
where it isn't used.  Anyhow, not used here as we have no attrs not created by
the iio core.

> +#include 
The adis core is doing all our buffer stuff for us as well, so this include
isn't needed here either.

> +#include 
> +
> +#define ADIS16201_STARTUP_DELAY_MS   220
> +#define ADIS16201_FLASH_CNT  0x00
> +
> +/* Data Output Register Information */
> +#define ADIS16201_SUPPLY_OUT_REG 0x02
> +#define ADIS16201_XACCL_OUT_REG  0x04
> +#define ADIS16201_YACCL_OUT_REG  0x06
> +#define ADIS16201_AUX_ADC_REG0x08
> +#define ADIS16201_TEMP_OUT_REG   0x0A
> +#define ADIS16201_XINCL_OUT_REG  0x0C
> +#define ADIS16201_YINCL_OUT_REG  0x0E
> +
> +/* Calibration Register Definition */
> +#define ADIS16201_XACCL_OFFS_REG 0x10
> +#define ADIS16201_YACCL_OFFS_REG 0x12
> +#define ADIS16201_XACCL_SCALE_REG0x14
> +#define ADIS16201_YACCL_SCALE_REG0x16
> +#define ADIS16201_XINCL_OFFS_REG 0x18
> +#define ADIS16201_YINCL_OFFS_REG 0x1A
> +#define ADIS16201_XINCL_SCALE_REG0x1C
> +#define ADIS16201_YINCL_SCALE_REG0x1E
> +
> +/* Alarm Register Definition */
> +#define ADIS16201_ALM_MAG1_REG   0x20
> +#define ADIS16201_ALM_MAG2_REG   0x22
> +#define ADIS16201_ALM_SMPL1_REG 

Re: [PATCH v2 8/9] Staging: iio: accel: adis16201: Adjust argument to match open parentheses

[Jonathan Cameron]

On Sat, 17 Mar 2018 01:36:25 +0530
Himanshu Jha  wrote:

> In adis16201_read_raw() adjust an argument to match an open parentheses
> using tabs and spaces.
> 
> Signed-off-by: Himanshu Jha 
Applied, thanks

Jonathan

> ---
> v2:
>-aligned perfectly to match open parentheses.
> 
>  drivers/staging/iio/accel/adis16201.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 00e944e..e439cf7 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -115,7 +115,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
>   switch (mask) {
>   case IIO_CHAN_INFO_RAW:
>   return adis_single_conversion(indio_dev, chan,
> - ADIS16201_ERROR_ACTIVE, val);
> +   ADIS16201_ERROR_ACTIVE, val);
>   case IIO_CHAN_INFO_SCALE:
>   switch (chan->type) {
>   case IIO_VOLTAGE:

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 7/9] Staging: iio: accel: adis16201: Prefer reverse christmas tree ordering

[Jonathan Cameron]

On Sat, 17 Mar 2018 01:36:24 +0530
Himanshu Jha  wrote:

> Prefer reverse christmas tree ordering of declarations to improve
> readability.
> 
> Signed-off-by: Himanshu Jha 
As ever, this sort of change is only worth doing if you are working on the
relevant code anyway.  You are, so good to tidy up! (I put that here to
avoid getting lots of people feeling they 'have' to make this sort of
change).

Applied

Thanks,

Jonathan
> ---
> v2:
>-no change in this patch.
> 
>  drivers/staging/iio/accel/adis16201.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 298bf90..00e944e 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -259,9 +259,9 @@ static const struct adis_data adis16201_data = {
>  
>  static int adis16201_probe(struct spi_device *spi)
>  {
> - int ret;
> - struct adis *st;
>   struct iio_dev *indio_dev;
> + struct adis *st;
> + int ret;
>  
>   indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
>   if (!indio_dev)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 6/9] Staging: iio: accel: adis16201: Use sign_extend32 function

[Jonathan Cameron]

On Sat, 17 Mar 2018 01:36:23 +0530
Himanshu Jha  wrote:

> Use sign_extned32() for 32 bit sign extending rather than hard coding.
> 
> Signed-off-by: Himanshu Jha 

Great, applied.

Thanks,

Jonathan

> ---
> v2:
>-no change in this patch.
> 
>  drivers/staging/iio/accel/adis16201.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 97150ea..298bf90 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -173,9 +173,8 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
>   ret = adis_read_reg_16(st, addr, );
>   if (ret)
>   return ret;
> - val16 &= (1 << bits) - 1;
> - val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
> - *val = val16;
> +
> + *val = sign_extend32(val16, bits - 1);
>   return IIO_VAL_INT;
>   }
>  

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 5/9] Staging: iio: accel: adis16201: Add comments about units in read_raw()

[Jonathan Cameron]

On Sat, 17 Mar 2018 01:36:22 +0530
Himanshu Jha  wrote:

> Clarify the conversion and formation of resultant data in the
> adis16201_read_raw() with sufficient comments and remove the unnecessary
> comments.
> 
> Signed-off-by: Himanshu Jha 
It is a little illogical to have comments for all but one of the channels, but
given we don't normally comment them unless they are 'odd', this is fine.

Applied, thanks

Jonathan

> ---
> v2:
>-clarify voltage base conversions.
> 
>  drivers/staging/iio/accel/adis16201.c | 27 ++-
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 8de3f27..97150ea 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -120,31 +120,43 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
>   switch (chan->type) {
>   case IIO_VOLTAGE:
>   if (chan->channel == 0) {
> + /* Voltage base units are mV hence 1.22 mV */
>   *val = 1;
> - *val2 = 22; /* 1.22 mV */
> + *val2 = 22;
>   } else {
> + /* Voltage base units are mV hence 0.61 mV */
>   *val = 0;
> - *val2 = 61; /* 0.610 mV */
> + *val2 = 61;
>   }
>   return IIO_VAL_INT_PLUS_MICRO;
>   case IIO_TEMP:
> - *val = -470; /* 0.47 C */
> + *val = -470;
>   *val2 = 0;
>   return IIO_VAL_INT_PLUS_MICRO;
>   case IIO_ACCEL:
> + /*
> +  * IIO base unit for sensitivity of accelerometer
> +  * is milli g.
> +  * 1 LSB represents 0.244 mg.
> +  */
>   *val = 0;
> - *val2 = IIO_G_TO_M_S_2(462400); /* 0.4624 mg */
> + *val2 = IIO_G_TO_M_S_2(462400);
>   return IIO_VAL_INT_PLUS_NANO;
>   case IIO_INCLI:
>   *val = 0;
> - *val2 = 10; /* 0.1 degree */
> + *val2 = 10;
>   return IIO_VAL_INT_PLUS_MICRO;
>   default:
>   return -EINVAL;
>   }
>   break;
>   case IIO_CHAN_INFO_OFFSET:
> - *val = 25000 / -470 - 1278; /* 25 C = 1278 */
> + /*
> +  * The raw ADC value is 1278 when the temperature
> +  * is 25 degrees and the scale factor per milli
> +  * degree celcius is -470.
> +  */
> + *val = 25000 / -470 - 1278;
>   return IIO_VAL_INT;
>   case IIO_CHAN_INFO_CALIBBIAS:
>   switch (chan->type) {
> @@ -252,13 +264,11 @@ static int adis16201_probe(struct spi_device *spi)
>   struct adis *st;
>   struct iio_dev *indio_dev;
>  
> - /* setup the industrialio driver allocated elements */
>   indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
>   if (!indio_dev)
>   return -ENOMEM;
>  
>   st = iio_priv(indio_dev);
> - /* this is only used for removal purposes */
>   spi_set_drvdata(spi, indio_dev);
>  
>   indio_dev->name = spi->dev.driver->name;
> @@ -277,7 +287,6 @@ static int adis16201_probe(struct spi_device *spi)
>   if (ret)
>   return ret;
>  
> - /* Get the device into a sane initial state */
>   ret = adis_initial_startup(st);
>   if (ret)
>   goto error_cleanup_buffer_trigger;

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 4/9] Staging: iio: accel: adis16201: Group register definitions

[Jonathan Cameron]

On Sat, 17 Mar 2018 01:36:21 +0530
Himanshu Jha  wrote:

> Group register definitions with its register field bits to improve
> readability and easy identification. A small comment is also added to
> denote the purpose/functionality of the grouped register definitions.
> 
> Signed-off-by: Himanshu Jha 
There was one small issue in here that I fixed up.

Applied, thanks.

Jonathan

> ---
> v2:
>-reordered patch series.
> 
>  drivers/staging/iio/accel/adis16201.c | 138 
> +-
>  1 file changed, 54 insertions(+), 84 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 0c63cd0..8de3f27 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -20,99 +20,69 @@
>  #include 
>  #include 
>  
> -#define ADIS16201_STARTUP_DELAY_MS   220
> -
> -#define ADIS16201_FLASH_CNT  0x00
> -
> -#define ADIS16201_SUPPLY_OUT_REG 0x02
> -
> -#define ADIS16201_XACCL_OUT_REG  0x04
> -
> -#define ADIS16201_YACCL_OUT_REG  0x06
> -
> -#define ADIS16201_AUX_ADC_REG0x08
> -
> -#define ADIS16201_TEMP_OUT_REG   0x0A
> -
> -#define ADIS16201_XINCL_OUT_REG  0x0C
> -
> -#define ADIS16201_YINCL_OUT_REG  0x0E
> -
> -#define ADIS16201_XACCL_OFFS_REG 0x10
> -
> -#define ADIS16201_YACCL_OFFS_REG 0x12
> -
> -#define ADIS16201_XACCL_SCALE_REG0x14
> -
> -#define ADIS16201_YACCL_SCALE_REG0x16
> -
> -#define ADIS16201_XINCL_OFFS_REG 0x18
> -
> -#define ADIS16201_YINCL_OFFS_REG 0x1A
> -
> -#define ADIS16201_XINCL_SCALE_REG0x1C
> -
> -#define ADIS16201_YINCL_SCALE_REG0x1E
> -
> -#define ADIS16201_ALM_MAG1_REG   0x20
> -
> -#define ADIS16201_ALM_MAG2_REG   0x22
> -
> -#define ADIS16201_ALM_SMPL1_REG  0x24
> -
> -#define ADIS16201_ALM_SMPL2_REG  0x26
> -
> -#define ADIS16201_ALM_CTRL_REG   0x28
> -
> -#define ADIS16201_AUX_DAC_REG0x30
> -
> -#define ADIS16201_GPIO_CTRL_REG  0x32
> -
> -#define ADIS16201_MSC_CTRL_REG   0x34
> -
> -#define ADIS16201_SMPL_PRD_REG   0x36
> -
> +#define ADIS16201_STARTUP_DELAY_MS   220
> +#define ADIS16201_FLASH_CNT  0x00
> +
> +/* Data Output Register Information */
> +#define ADIS16201_SUPPLY_OUT_REG 0x02
> +#define ADIS16201_XACCL_OUT_REG  0x04
> +#define ADIS16201_YACCL_OUT_REG  0x06
> +#define ADIS16201_AUX_ADC_REG0x08
> +#define ADIS16201_TEMP_OUT_REG   0x0A
> +#define ADIS16201_XINCL_OUT_REG  0x0C
> +#define ADIS16201_YINCL_OUT_REG  0x0E
> +
> +/* Calibration Register Definition */
> +#define ADIS16201_XACCL_OFFS_REG 0x10
> +#define ADIS16201_YACCL_OFFS_REG 0x12
> +#define ADIS16201_XACCL_SCALE_REG0x14
> +#define ADIS16201_YACCL_SCALE_REG0x16
> +#define ADIS16201_XINCL_OFFS_REG 0x18
> +#define ADIS16201_YINCL_OFFS_REG 0x1A
> +#define ADIS16201_XINCL_SCALE_REG0x1C
> +#define ADIS16201_YINCL_SCALE_REG0x1E
> +
> +/* Alarm Register Definition */
> +#define ADIS16201_ALM_MAG1_REG   0x20
> +#define ADIS16201_ALM_MAG2_REG   0x22
> +#define ADIS16201_ALM_SMPL1_REG  0x24
> +#define ADIS16201_ALM_SMPL2_REG  0x26
> +#define ADIS16201_ALM_CTRL_REG   0x28
> +
> +#define ADIS16201_AUX_DAC_REG0x30
> +#define ADIS16201_GPIO_CTRL_REG  0x32
> +#define ADIS16201_SMPL_PRD_REG   0x36
>  /* Operation, filter configuration */
> -#define ADIS16201_AVG_CNT_REG0x38
> -
> -#define ADIS16201_SLP_CNT_REG0x3A
> -
> -#define ADIS16201_DIAG_STAT_REG  0x3C
> -
> -#define ADIS16201_GLOB_CMD_REG   0x3E
> -
> -
> -#define ADIS16201_MSC_CTRL_SELF_TEST_EN  BIT(8)
> +#define ADIS16201_AVG_CNT_REG0x38
> +#define ADIS16201_SLP_CNT_REG0x3A
>  
> +/* Miscellaneous Control Register Definition */
> +#define ADIS16201_MSC_CTRL_REG   0x34
> +#define  ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
In this one case you have spaces when it should be a tab before BIT(8).
I'll fix it.

>  /* Data-ready enable: 1 = enabled, 0 = disabled */
> -#define ADIS16201_MSC_CTRL_DATA_RDY_EN   BIT(2)
> -
> +#define  ADIS16201_MSC_CTRL_DATA_RDY_EN  BIT(2)
>  /* Data-ready polarity: 1 = active high, 0 = active low */
> -#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH  BIT(1)
> -
> +#define  

Re: [PATCH v2 3/9] Staging: iio: accel: adis16201: Add _REG suffix to reisters

[Jonathan Cameron]

On Sat, 17 Mar 2018 01:36:20 +0530
Himanshu Jha  wrote:

> Add a _REG suffix to distinguish between registers and the register bit
> fileds.
> 
> Signed-off-by: Himanshu Jha 
Other than the typo in the patch title, this looks good (I fixed that up when
applying).  Applied.

Jonathan
> ---
> v2:
>-reordered patch series.
> 
>  drivers/staging/iio/accel/adis16201.c | 84 
> +--
>  1 file changed, 42 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 6c06c0d..0c63cd0 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -24,62 +24,62 @@
>  
>  #define ADIS16201_FLASH_CNT  0x00
>  
> -#define ADIS16201_SUPPLY_OUT 0x02
> +#define ADIS16201_SUPPLY_OUT_REG 0x02
>  
> -#define ADIS16201_XACCL_OUT  0x04
> +#define ADIS16201_XACCL_OUT_REG  0x04
>  
> -#define ADIS16201_YACCL_OUT  0x06
> +#define ADIS16201_YACCL_OUT_REG  0x06
>  
> -#define ADIS16201_AUX_ADC0x08
> +#define ADIS16201_AUX_ADC_REG0x08
>  
> -#define ADIS16201_TEMP_OUT   0x0A
> +#define ADIS16201_TEMP_OUT_REG   0x0A
>  
> -#define ADIS16201_XINCL_OUT  0x0C
> +#define ADIS16201_XINCL_OUT_REG  0x0C
>  
> -#define ADIS16201_YINCL_OUT  0x0E
> +#define ADIS16201_YINCL_OUT_REG  0x0E
>  
> -#define ADIS16201_XACCL_OFFS 0x10
> +#define ADIS16201_XACCL_OFFS_REG 0x10
>  
> -#define ADIS16201_YACCL_OFFS 0x12
> +#define ADIS16201_YACCL_OFFS_REG 0x12
>  
> -#define ADIS16201_XACCL_SCALE0x14
> +#define ADIS16201_XACCL_SCALE_REG0x14
>  
> -#define ADIS16201_YACCL_SCALE0x16
> +#define ADIS16201_YACCL_SCALE_REG0x16
>  
> -#define ADIS16201_XINCL_OFFS 0x18
> +#define ADIS16201_XINCL_OFFS_REG 0x18
>  
> -#define ADIS16201_YINCL_OFFS 0x1A
> +#define ADIS16201_YINCL_OFFS_REG 0x1A
>  
> -#define ADIS16201_XINCL_SCALE0x1C
> +#define ADIS16201_XINCL_SCALE_REG0x1C
>  
> -#define ADIS16201_YINCL_SCALE0x1E
> +#define ADIS16201_YINCL_SCALE_REG0x1E
>  
> -#define ADIS16201_ALM_MAG1   0x20
> +#define ADIS16201_ALM_MAG1_REG   0x20
>  
> -#define ADIS16201_ALM_MAG2   0x22
> +#define ADIS16201_ALM_MAG2_REG   0x22
>  
> -#define ADIS16201_ALM_SMPL1  0x24
> +#define ADIS16201_ALM_SMPL1_REG  0x24
>  
> -#define ADIS16201_ALM_SMPL2  0x26
> +#define ADIS16201_ALM_SMPL2_REG  0x26
>  
> -#define ADIS16201_ALM_CTRL   0x28
> +#define ADIS16201_ALM_CTRL_REG   0x28
>  
> -#define ADIS16201_AUX_DAC0x30
> +#define ADIS16201_AUX_DAC_REG0x30
>  
> -#define ADIS16201_GPIO_CTRL  0x32
> +#define ADIS16201_GPIO_CTRL_REG  0x32
>  
> -#define ADIS16201_MSC_CTRL   0x34
> +#define ADIS16201_MSC_CTRL_REG   0x34
>  
> -#define ADIS16201_SMPL_PRD   0x36
> +#define ADIS16201_SMPL_PRD_REG   0x36
>  
>  /* Operation, filter configuration */
> -#define ADIS16201_AVG_CNT0x38
> +#define ADIS16201_AVG_CNT_REG0x38
>  
> -#define ADIS16201_SLP_CNT0x3A
> +#define ADIS16201_SLP_CNT_REG0x3A
>  
> -#define ADIS16201_DIAG_STAT  0x3C
> +#define ADIS16201_DIAG_STAT_REG  0x3C
>  
> -#define ADIS16201_GLOB_CMD   0x3E
> +#define ADIS16201_GLOB_CMD_REG   0x3E
>  
>  
>  #define ADIS16201_MSC_CTRL_SELF_TEST_EN  BIT(8)
> @@ -125,10 +125,10 @@ enum adis16201_scan {
>  };
>  
>  static const u8 adis16201_addresses[] = {
> - [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS,
> - [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS,
> - [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS,
> - [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS,
> + [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
> + [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
> + [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
> + [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
>  };
>  
>  static int adis16201_read_raw(struct iio_dev *indio_dev,
> @@ -232,16 +232,16 @@ static int adis16201_write_raw(struct iio_dev 
> *indio_dev,
>  }
>  
>  static const struct iio_chan_spec adis16201_channels[] = {
> - ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT, ADIS16201_SCAN_SUPPLY, 0, 12),
> - ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT, ADIS16201_SCAN_TEMP, 0, 12),
> - ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT, ADIS16201_SCAN_ACC_X,
> + ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 
> 12),
> + ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
> + ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
>   BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> - ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT, ADIS16201_SCAN_ACC_Y,
> + ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
>   BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> - ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC, 

Re: [PATCH v2 2/9] Staging: iio: accel: adis16201: Remove unnecessary comments

[Jonathan Cameron]

On Sat, 17 Mar 2018 01:36:19 +0530
Himanshu Jha  wrote:

> Remove few unnecessary comments since the macro definitions clearly
> justify their purpose.
> 
> Signed-off-by: Himanshu Jha 
Seems like a good balance between enough information and too much noise
so applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> v2:
>-reodered patch series.
> 
>  drivers/staging/iio/accel/adis16201.c | 36 
> ---
>  1 file changed, 36 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 767ebf0..6c06c0d 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -22,96 +22,66 @@
>  
>  #define ADIS16201_STARTUP_DELAY_MS   220
>  
> -/* Flash memory write count */
>  #define ADIS16201_FLASH_CNT  0x00
>  
> -/* Output, power supply */
>  #define ADIS16201_SUPPLY_OUT 0x02
>  
> -/* Output, x-axis accelerometer */
>  #define ADIS16201_XACCL_OUT  0x04
>  
> -/* Output, y-axis accelerometer */
>  #define ADIS16201_YACCL_OUT  0x06
>  
> -/* Output, auxiliary ADC input */
>  #define ADIS16201_AUX_ADC0x08
>  
> -/* Output, temperature */
>  #define ADIS16201_TEMP_OUT   0x0A
>  
> -/* Output, x-axis inclination */
>  #define ADIS16201_XINCL_OUT  0x0C
>  
> -/* Output, y-axis inclination */
>  #define ADIS16201_YINCL_OUT  0x0E
>  
> -/* Calibration, x-axis acceleration offset */
>  #define ADIS16201_XACCL_OFFS 0x10
>  
> -/* Calibration, y-axis acceleration offset */
>  #define ADIS16201_YACCL_OFFS 0x12
>  
> -/* x-axis acceleration scale factor */
>  #define ADIS16201_XACCL_SCALE0x14
>  
> -/* y-axis acceleration scale factor */
>  #define ADIS16201_YACCL_SCALE0x16
>  
> -/* Calibration, x-axis inclination offset */
>  #define ADIS16201_XINCL_OFFS 0x18
>  
> -/* Calibration, y-axis inclination offset */
>  #define ADIS16201_YINCL_OFFS 0x1A
>  
> -/* x-axis inclination scale factor */
>  #define ADIS16201_XINCL_SCALE0x1C
>  
> -/* y-axis inclination scale factor */
>  #define ADIS16201_YINCL_SCALE0x1E
>  
> -/* Alarm 1 amplitude threshold */
>  #define ADIS16201_ALM_MAG1   0x20
>  
> -/* Alarm 2 amplitude threshold */
>  #define ADIS16201_ALM_MAG2   0x22
>  
> -/* Alarm 1, sample period */
>  #define ADIS16201_ALM_SMPL1  0x24
>  
> -/* Alarm 2, sample period */
>  #define ADIS16201_ALM_SMPL2  0x26
>  
> -/* Alarm control */
>  #define ADIS16201_ALM_CTRL   0x28
>  
> -/* Auxiliary DAC data */
>  #define ADIS16201_AUX_DAC0x30
>  
> -/* General-purpose digital input/output control */
>  #define ADIS16201_GPIO_CTRL  0x32
>  
> -/* Miscellaneous control */
>  #define ADIS16201_MSC_CTRL   0x34
>  
> -/* Internal sample period (rate) control */
>  #define ADIS16201_SMPL_PRD   0x36
>  
>  /* Operation, filter configuration */
>  #define ADIS16201_AVG_CNT0x38
>  
> -/* Operation, sleep mode control */
>  #define ADIS16201_SLP_CNT0x3A
>  
> -/* Diagnostics, system status register */
>  #define ADIS16201_DIAG_STAT  0x3C
>  
> -/* Operation, system command register */
>  #define ADIS16201_GLOB_CMD   0x3E
>  
> -/* MSC_CTRL */
>  
> -/* Self-test enable */
>  #define ADIS16201_MSC_CTRL_SELF_TEST_EN  BIT(8)
>  
>  /* Data-ready enable: 1 = enabled, 0 = disabled */
> @@ -123,18 +93,13 @@
>  /* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
>  #define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
>  
> -/* DIAG_STAT */
>  
> -/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
>  #define ADIS16201_DIAG_STAT_ALARM2BIT(9)
>  
> -/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
>  #define ADIS16201_DIAG_STAT_ALARM1BIT(8)
>  
> -/* SPI communications failure */
>  #define ADIS16201_DIAG_STAT_SPI_FAIL_BIT   3
>  
> -/* Flash update failure */
>  #define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT  2
>  
>  /* Power supply above 3.625 V */
> @@ -143,7 +108,6 @@
>  /* Power supply below 3.15 V */
>  #define ADIS16201_DIAG_STAT_POWER_LOW_BIT  0
>  
> -/* GLOB_CMD */
>  
>  #define ADIS16201_GLOB_CMD_SW_RESET  BIT(7)
>  #define ADIS16201_GLOB_CMD_FACTORY_RESET BIT(1)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/9] Staging: iio: accel: adis16201: Rename few macro definitions

[Jonathan Cameron]

On Sat, 17 Mar 2018 01:36:18 +0530
Himanshu Jha  wrote:

> Rename the macro definitions with suitable names specifying their
> purpose.
> 
> * ADIS16201_STARTUP_DELAY_MS: Remove the comment specifying the delay in
>   microseconds and rename it with addtition of _MS suffix.
> 
> * ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH: Rename the macro to make it
>   similar to other misc control registers which denotes the data ready
>   polarity.
> 
> * ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT: Rename to denote it is a
>   failure bit.
> 
> * ADIS16201_GLOB_CMD_FACTORY_RESET: Remove ambiguous _CAL suffix and add
>   _RESET suffix instead to denote factory reset command.
> 
> Signed-off-by: Himanshu Jha 
Nicely described and well put together patch.

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
> v2:
>-explained every change in the process of renaming macros. 
> 
>  drivers/staging/iio/accel/adis16201.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16201.c 
> b/drivers/staging/iio/accel/adis16201.c
> index 0fae8aa..767ebf0 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -20,7 +20,7 @@
>  #include 
>  #include 
>  
> -#define ADIS16201_STARTUP_DELAY  220 /* ms */
> +#define ADIS16201_STARTUP_DELAY_MS   220
>  
>  /* Flash memory write count */
>  #define ADIS16201_FLASH_CNT  0x00
> @@ -118,7 +118,7 @@
>  #define ADIS16201_MSC_CTRL_DATA_RDY_EN   BIT(2)
>  
>  /* Data-ready polarity: 1 = active high, 0 = active low */
> -#define ADIS16201_MSC_CTRL_ACTIVE_HIGH   BIT(1)
> +#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH  BIT(1)
>  
>  /* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
>  #define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
> @@ -135,7 +135,7 @@
>  #define ADIS16201_DIAG_STAT_SPI_FAIL_BIT   3
>  
>  /* Flash update failure */
> -#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT  2
> +#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT  2
>  
>  /* Power supply above 3.625 V */
>  #define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
> @@ -146,7 +146,7 @@
>  /* GLOB_CMD */
>  
>  #define ADIS16201_GLOB_CMD_SW_RESET  BIT(7)
> -#define ADIS16201_GLOB_CMD_FACTORY_CAL   BIT(1)
> +#define ADIS16201_GLOB_CMD_FACTORY_RESET BIT(1)
>  
>  #define ADIS16201_ERROR_ACTIVE  BIT(14)
>  
> @@ -290,7 +290,7 @@ static const struct iio_info adis16201_info = {
>  
>  static const char * const adis16201_status_error_msgs[] = {
>   [ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
> - [ADIS16201_DIAG_STAT_FLASH_UPT_BIT] = "Flash update failed",
> + [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
>   [ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
>   [ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
>  };
> @@ -303,11 +303,11 @@ static const struct adis_data adis16201_data = {
>  
>   .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
>   .self_test_no_autoclear = true,
> - .startup_delay = ADIS16201_STARTUP_DELAY,
> + .startup_delay = ADIS16201_STARTUP_DELAY_MS,
>  
>   .status_error_msgs = adis16201_status_error_msgs,
>   .status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> - BIT(ADIS16201_DIAG_STAT_FLASH_UPT_BIT) |
> + BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
>   BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
>   BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
>  };

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] vmbus: use put_device() if device_register fail

[Arvind Yadav]

if device_register() returned an error. Always use put_device()
to give up the reference initialized.

Signed-off-by: Arvind Yadav 
---
 drivers/hv/vmbus_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index bc65c4d..25da2f3 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1358,6 +1358,7 @@ int vmbus_device_register(struct hv_device 
*child_device_obj)
ret = device_register(_device_obj->device);
if (ret) {
pr_err("Unable to register child device\n");
+   put_device(_device_obj->device);
return ret;
}
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR

[John Syne]

Hi Jonathan,

Here is the complete list of registers for the ADE7878 which I copied from the 
data sheet. I added a column “IIO Attribute” which I hope follows your IIO ABI. 
Please make any changes you feel are incorrect. BTW, there are several 
registers that cannot be generalized and are used purely for chip 
configuration. I think we should add a new naming convention, namely 
{register}_{}. Also, I see in the sys_bus_iio doc
in_accel_x_peak_raw

so shouldn’t the phase be represented as follows:

in_current_A_scale

But for now, I followed your instructions from your reply.

After finalizing this one, I will work on the ADE9000, which as way more 
registers ;-)

Once we can agree on the register naming, I will update the ADE7854 driver for 
Rodrigo, which will go a long way to getting it out of staging.

Address RegisterIIO Attribute   R/W Bit Length  Bit Length 
During CommunicationsTypeDefault Value   Description
0x4380  AIGAIN  in_currentA_scale   R/W 24  32 ZPSE S   
0x00Phase A current gain adjust.
0x4381  AVGAIN  in_voltageA_scale   R/W 24  32 ZPSE S   
0x00Phase A voltage gain adjust.
0x4382  BIGAIN  in_currentB_scale   R/W 24  32 ZPSE S   
0x00Phase B current gain adjust.
0x4383  BVGAIN  in_voltageB_scale   R/W 24  32 ZPSE S   
0x00Phase B voltage gain adjust.
0x4384  CIGAIN  in_currentC_scale   R/W 24  32 ZPSE S   
0x00Phase C current gain adjust.
0x4385  CVGAIN  in_voltageC_scale   R/W 24  32 ZPSE S   
0x00Phase C voltage gain adjust.
0x4386  NIGAIN  in_currentN_scale   R/W 24  32 ZPSE S   
0x00Neutral current gain adjust (ADE7868 and ADE7878 only).
0x4387  AIRMSOS in_currentA_rms_offset  R/W 24  32 ZPSE S   
0x00Phase A current rms offset.
0x4388  AVRMSOS in_voltageA_rms_offset  R/W 24  32 ZPSE S   
0x00Phase A voltage rms offset.
0x4389  BIRMSOS in_currentB_rms_offset  R/W 24  32 ZPSE S   
0x00Phase B current rms offset.
0x438A  BVRMSOS in_voltageB_rms_offset  R/W 24  32 ZPSE S   
0x00Phase B voltage rms offset.
0x438B  CIRMSOS in_currentC_rms_offset  R/W 24  32 ZPSE S   
0x00Phase C current rms offset.
0x438C  CVRMSOS in_voltageC_rms_offset  R/W 24  32 ZPSE S   
0x00Phase C voltage rms offset.
0x438D  NIRMSOS in_currentN_rms_offset  R/W 24  32 ZPSE S   
0x00Neutral current rms offset (ADE7868 and ADE7878 only).
0x438E  AVAGAIN in_powerapparentA_scale R/W 24  32 ZPSE S   
0x00Phase A apparent power gain adjust.
0x438F  BVAGAIN in_powerapparentB_scale R/W 24  32 ZPSE S   
0x00Phase B apparent power gain adjust.
0x4390  CVAGAIN in_powerapparentC_scale R/W 24  32 ZPSE S   
0x00Phase C apparent power gain adjust.
0x4391  AWGAIN  in_powerA_scale R/W 24  32 ZPSE S   0x00
Phase A total active power gain adjust.
0x4392  AWATTOS in_powerA_offsetR/W 24  32 ZPSE S   
0x00Phase A total active power offset adjust.
0x4393  BWGAIN  in_powerB_scale R/W 24  32 ZPSE S   0x00
Phase B total active power gain adjust.
0x4394  BWATTOS in_powerB_offsetR/W 24  32 ZPSE S   
0x00Phase B total active power offset adjust.
0x4395  CWGAIN  in_powerC_scale R/W 24  32 ZPSE S   0x00
Phase C total active power gain adjust.
0x4396  CWATTOS in_powerC_offsetR/W 24  32 ZPSE S   
0x00Phase C total active power offset adjust.
0x4397  AVARGAINin_powerreactiveA_scale R/W 24  32 ZPSE S   
0x00Phase A total reactive power gain adjust (ADE7858, ADE7868, and 
ADE7878).
0x4398  AVAROS  in_powerreactiveA_offsetR/W 24  32 ZPSE S   
0x00Phase A total reactive power offset adjust (ADE7858, ADE7868, 
and ADE7878).
0x4399  BVARGAINin_powerreactiveB_scale R/W 24  32 ZPSE S   
0x00Phase B total reactive power gain adjust (ADE7858, ADE7868, and 
ADE7878).
0x439A  BVAROS  in_powerreactiveB_offsetR/W 24  32 ZPSE S   
0x00Phase B total reactive power offset adjust (ADE7858, ADE7868, 
and ADE7878).
0x439B  CVARGAINin_powerreactiveC_scale R/W 24  32 ZPSE S   
0x00Phase C total reactive power gain adjust (ADE7858, ADE7868, and 
ADE7878).
0x439C  CVAROS  in_powerreactiveC_offsetR/W 24  32 ZPSE S   
0x00Phase C total reactive power offset adjust (ADE7858, ADE7868, 
and ADE7878).
0x439D  AFWGAIN in_powerA_fundamental_scale R/W 24  32 ZPSE S   
0x00Phase A fundamental active power gain adjust. Location reserved 
for