[PATCH v2] mfd: cros_ec_dev: Return number of bytes read with CROS_EC_DEV_IOCRDMEM

[PATCH v2] mfd: cros_ec_dev: Return number of bytes read with CROS_EC_DEV_IOCRDMEM

[Enric Balletbo i Serra]

From: Guenter Roeck <linux@roeck-us.net>

While ioctls normally return a negative error or 0 on success, this is not
the case for CROS_EC_DEV_IOCXCMD in the cros_ec driver, which returns the
number of bytes read instead. However, CROS_EC_DEV_IOCRDMEM in the same
driver does not return the number of byte read on success. This is both
inconsistent and problematic, since the amount of returned data can be
dynamic. Have it return the number of data bytes for consistency.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Hi,

This patch was send long time ago and never landed to mainline [1]. This
is a second attempt. Originally was made to apply on platform/chrome but
after the recent changes we did it should go now throught the MFD
subsystem. Without this patch userspace tools like ectool doesn't work
as expected.

Best regards,
 Enric

[1] https://lore.kernel.org/patchwork/patch/665211/

Changes in v2:
- Adapt and rebase to current mainline.

 drivers/mfd/cros_ec_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index ed809fc97df8..d275deaecb12 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -224,7 +224,7 @@ static long ec_device_ioctl_readmem(struct cros_ec_dev *ec, void __user *arg)
 	if (copy_to_user((void __user *)arg, &s_mem, sizeof(s_mem)))
 		return -EFAULT;
 
-	return 0;
+	return num;
 }
 
 static long ec_device_ioctl(struct file *filp, unsigned int cmd,
-- 
2.20.1

Re: [PATCH v2] mfd: cros_ec_dev: Return number of bytes read with CROS_EC_DEV_IOCRDMEM

[Benson Leung]

[-- Attachment #1: Type: text/plain, Size: 1978 bytes --]

On Wed, Feb 06, 2019 at 06:39:59PM +0100, Enric Balletbo i Serra wrote:
> From: Guenter Roeck <linux@roeck-us.net>
> 
> While ioctls normally return a negative error or 0 on success, this is not
> the case for CROS_EC_DEV_IOCXCMD in the cros_ec driver, which returns the
> number of bytes read instead. However, CROS_EC_DEV_IOCRDMEM in the same
> driver does not return the number of byte read on success. This is both
> inconsistent and problematic, since the amount of returned data can be
> dynamic. Have it return the number of data bytes for consistency.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Reviewed-by: Benson Leung <bleung@chromium.org>

> ---
> Hi,
> 
> This patch was send long time ago and never landed to mainline [1]. This
> is a second attempt. Originally was made to apply on platform/chrome but
> after the recent changes we did it should go now throught the MFD
> subsystem. Without this patch userspace tools like ectool doesn't work
> as expected.
> 
> Best regards,
>  Enric
> 
> [1] https://lore.kernel.org/patchwork/patch/665211/
> 
> Changes in v2:
> - Adapt and rebase to current mainline.
> 
>  drivers/mfd/cros_ec_dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index ed809fc97df8..d275deaecb12 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -224,7 +224,7 @@ static long ec_device_ioctl_readmem(struct cros_ec_dev *ec, void __user *arg)
>  	if (copy_to_user((void __user *)arg, &s_mem, sizeof(s_mem)))
>  		return -EFAULT;
>  
> -	return 0;
> +	return num;
>  }
>  
>  static long ec_device_ioctl(struct file *filp, unsigned int cmd,
> -- 
> 2.20.1
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] mfd: cros_ec_dev: Return number of bytes read with CROS_EC_DEV_IOCRDMEM

[Lee Jones]

On Wed, 06 Feb 2019, Enric Balletbo i Serra wrote:

> From: Guenter Roeck <linux@roeck-us.net>
> 
> While ioctls normally return a negative error or 0 on success, this is not
> the case for CROS_EC_DEV_IOCXCMD in the cros_ec driver, which returns the
> number of bytes read instead. However, CROS_EC_DEV_IOCRDMEM in the same
> driver does not return the number of byte read on success. This is both
> inconsistent and problematic, since the amount of returned data can be
> dynamic. Have it return the number of data bytes for consistency.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> Hi,
> 
> This patch was send long time ago and never landed to mainline [1]. This
> is a second attempt. Originally was made to apply on platform/chrome but
> after the recent changes we did it should go now throught the MFD
> subsystem. Without this patch userspace tools like ectool doesn't work
> as expected.
> 
> Best regards,
>  Enric
> 
> [1] https://lore.kernel.org/patchwork/patch/665211/
> 
> Changes in v2:
> - Adapt and rebase to current mainline.

Applied, thanks.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog