linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] staging: android: remove ION_IOC_SYNC
@ 2016-12-16 22:42 Matthew Smith
  2016-12-16 22:42 ` [PATCH 2/3] staging: android: remove ION_IOC_CUSTOM Matthew Smith
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Matthew Smith @ 2016-12-16 22:42 UTC (permalink / raw)
  To: greg; +Cc: arve, riandrews, linux-kernel, devel, Matthew Smith

Remove definition of ION_IOC_CUSTOM from ion.h.
Remove usage from compat_ion.c and ion-ioctl.c.
Remove item from TODO file.

Signed-off-by: Matthew Smith <matthew.s3h@gmail.com>
---
 drivers/staging/android/TODO             |  3 ---
 drivers/staging/android/ion/compat_ion.c |  1 -
 drivers/staging/android/ion/ion-ioctl.c  |  6 ------
 drivers/staging/android/uapi/ion.h       | 10 ----------
 4 files changed, 20 deletions(-)

diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 8f3ac37..bcf736c 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -7,9 +7,6 @@ TODO:
 
 
 ion/
- - Remove ION_IOC_SYNC: Flushing for devices should be purely a kernel internal
-   interface on top of dma-buf. flush_for_device needs to be added to dma-buf
-   first.
  - Remove ION_IOC_CUSTOM: Atm used for cache flushing for cpu access in some
    vendor trees. Should be replaced with an ioctl on the dma-buf to expose the
    begin/end_cpu_access hooks to userspace.
diff --git a/drivers/staging/android/ion/compat_ion.c b/drivers/staging/android/ion/compat_ion.c
index 9a978d2..b892d3a 100644
--- a/drivers/staging/android/ion/compat_ion.c
+++ b/drivers/staging/android/ion/compat_ion.c
@@ -186,7 +186,6 @@ long compat_ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	case ION_IOC_SHARE:
 	case ION_IOC_MAP:
 	case ION_IOC_IMPORT:
-	case ION_IOC_SYNC:
 		return filp->f_op->unlocked_ioctl(filp, cmd,
 						(unsigned long)compat_ptr(arg));
 	default:
diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c
index 7e7431d..aab086c 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -51,7 +51,6 @@ static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
 static unsigned int ion_ioctl_dir(unsigned int cmd)
 {
 	switch (cmd) {
-	case ION_IOC_SYNC:
 	case ION_IOC_FREE:
 	case ION_IOC_CUSTOM:
 		return _IOC_WRITE;
@@ -146,11 +145,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			data.handle.handle = handle->id;
 		break;
 	}
-	case ION_IOC_SYNC:
-	{
-		ret = ion_sync_for_device(client, data.fd.fd);
-		break;
-	}
 	case ION_IOC_CUSTOM:
 	{
 		if (!dev->custom_ioctl)
diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h
index 14cd873..c3a87a5 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -207,16 +207,6 @@ struct ion_heap_query {
 #define ION_IOC_IMPORT		_IOWR(ION_IOC_MAGIC, 5, struct ion_fd_data)
 
 /**
- * DOC: ION_IOC_SYNC - syncs a shared file descriptors to memory
- *
- * Deprecated in favor of using the dma_buf api's correctly (syncing
- * will happen automatically when the buffer is mapped to a device).
- * If necessary should be used after touching a cached buffer from the cpu,
- * this will make the buffer in memory coherent.
- */
-#define ION_IOC_SYNC		_IOWR(ION_IOC_MAGIC, 7, struct ion_fd_data)
-
-/**
  * DOC: ION_IOC_CUSTOM - call architecture specific ion ioctl
  *
  * Takes the argument of the architecture specific ioctl to call and
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/3] staging: android: remove ION_IOC_CUSTOM
  2016-12-16 22:42 [PATCH 1/3] staging: android: remove ION_IOC_SYNC Matthew Smith
@ 2016-12-16 22:42 ` Matthew Smith
  2016-12-16 22:42 ` [PATCH 3/3] staging: android: remove compat_get_ion_custom_data Matthew Smith
  2016-12-16 23:05 ` [PATCH 1/3] staging: android: remove ION_IOC_SYNC Laura Abbott
  2 siblings, 0 replies; 5+ messages in thread
From: Matthew Smith @ 2016-12-16 22:42 UTC (permalink / raw)
  To: greg; +Cc: arve, riandrews, linux-kernel, devel, Matthew Smith

Remove definition of ION_IOC_CUSTOM from ion.h.
Remove usage from compat_ion.c and ion-ioctl.c.
Remove item from TOOD file.

Signed-off-by: Matthew Smith <matthew.s3h@gmail.com>
---
 drivers/staging/android/TODO             |  3 ---
 drivers/staging/android/ion/compat_ion.c | 17 -----------------
 drivers/staging/android/ion/ion-ioctl.c  |  9 ---------
 drivers/staging/android/uapi/ion.h       |  8 --------
 4 files changed, 37 deletions(-)

diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index bcf736c..10f5ef5 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -7,9 +7,6 @@ TODO:
 
 
 ion/
- - Remove ION_IOC_CUSTOM: Atm used for cache flushing for cpu access in some
-   vendor trees. Should be replaced with an ioctl on the dma-buf to expose the
-   begin/end_cpu_access hooks to userspace.
  - Clarify the tricks ion plays with explicitly managing coherency behind the
    dma api's back (this is absolutely needed for high-perf gpu drivers): Add an
    explicit coherency management mode to flush_for_device to be used by drivers
diff --git a/drivers/staging/android/ion/compat_ion.c b/drivers/staging/android/ion/compat_ion.c
index b892d3a..8e6377b 100644
--- a/drivers/staging/android/ion/compat_ion.c
+++ b/drivers/staging/android/ion/compat_ion.c
@@ -166,23 +166,6 @@ long compat_ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		return filp->f_op->unlocked_ioctl(filp, ION_IOC_FREE,
 							(unsigned long)data);
 	}
-	case COMPAT_ION_IOC_CUSTOM: {
-		struct compat_ion_custom_data __user *data32;
-		struct ion_custom_data __user *data;
-		int err;
-
-		data32 = compat_ptr(arg);
-		data = compat_alloc_user_space(sizeof(*data));
-		if (!data)
-			return -EFAULT;
-
-		err = compat_get_ion_custom_data(data32, data);
-		if (err)
-			return err;
-
-		return filp->f_op->unlocked_ioctl(filp, ION_IOC_CUSTOM,
-							(unsigned long)data);
-	}
 	case ION_IOC_SHARE:
 	case ION_IOC_MAP:
 	case ION_IOC_IMPORT:
diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c
index aab086c..038e910 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -52,7 +52,6 @@ static unsigned int ion_ioctl_dir(unsigned int cmd)
 {
 	switch (cmd) {
 	case ION_IOC_FREE:
-	case ION_IOC_CUSTOM:
 		return _IOC_WRITE;
 	default:
 		return _IOC_DIR(cmd);
@@ -145,14 +144,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			data.handle.handle = handle->id;
 		break;
 	}
-	case ION_IOC_CUSTOM:
-	{
-		if (!dev->custom_ioctl)
-			return -ENOTTY;
-		ret = dev->custom_ioctl(client, data.custom.cmd,
-						data.custom.arg);
-		break;
-	}
 	case ION_IOC_HEAP_QUERY:
 		ret = ion_query_heaps(client, &data.query);
 		break;
diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h
index c3a87a5..c44406f 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -207,14 +207,6 @@ struct ion_heap_query {
 #define ION_IOC_IMPORT		_IOWR(ION_IOC_MAGIC, 5, struct ion_fd_data)
 
 /**
- * DOC: ION_IOC_CUSTOM - call architecture specific ion ioctl
- *
- * Takes the argument of the architecture specific ioctl to call and
- * passes appropriate userdata for that ioctl
- */
-#define ION_IOC_CUSTOM		_IOWR(ION_IOC_MAGIC, 6, struct ion_custom_data)
-
-/**
  * DOC: ION_IOC_HEAP_QUERY - information about available heaps
  *
  * Takes an ion_heap_query structure and populates information about
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 3/3] staging: android: remove compat_get_ion_custom_data
  2016-12-16 22:42 [PATCH 1/3] staging: android: remove ION_IOC_SYNC Matthew Smith
  2016-12-16 22:42 ` [PATCH 2/3] staging: android: remove ION_IOC_CUSTOM Matthew Smith
@ 2016-12-16 22:42 ` Matthew Smith
  2016-12-16 23:05 ` [PATCH 1/3] staging: android: remove ION_IOC_SYNC Laura Abbott
  2 siblings, 0 replies; 5+ messages in thread
From: Matthew Smith @ 2016-12-16 22:42 UTC (permalink / raw)
  To: greg; +Cc: arve, riandrews, linux-kernel, devel, Matthew Smith

After removing the ION_IOC_CUSTOM case, compat_get_ion_custom_data is no
longer required.

Signed-off-by: Matthew Smith <matthew.s3h@gmail.com>
---
 drivers/staging/android/ion/compat_ion.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/staging/android/ion/compat_ion.c b/drivers/staging/android/ion/compat_ion.c
index 8e6377b..a177641 100644
--- a/drivers/staging/android/ion/compat_ion.c
+++ b/drivers/staging/android/ion/compat_ion.c
@@ -105,22 +105,6 @@ static int compat_put_ion_allocation_data(
 	return err;
 }
 
-static int compat_get_ion_custom_data(
-			struct compat_ion_custom_data __user *data32,
-			struct ion_custom_data __user *data)
-{
-	compat_uint_t cmd;
-	compat_ulong_t arg;
-	int err;
-
-	err = get_user(cmd, &data32->cmd);
-	err |= put_user(cmd, &data->cmd);
-	err |= get_user(arg, &data32->arg);
-	err |= put_user(arg, &data->arg);
-
-	return err;
-};
-
 long compat_ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	long ret;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] staging: android: remove ION_IOC_SYNC
  2016-12-16 22:42 [PATCH 1/3] staging: android: remove ION_IOC_SYNC Matthew Smith
  2016-12-16 22:42 ` [PATCH 2/3] staging: android: remove ION_IOC_CUSTOM Matthew Smith
  2016-12-16 22:42 ` [PATCH 3/3] staging: android: remove compat_get_ion_custom_data Matthew Smith
@ 2016-12-16 23:05 ` Laura Abbott
  2016-12-16 23:27   ` Matthew Smith
  2 siblings, 1 reply; 5+ messages in thread
From: Laura Abbott @ 2016-12-16 23:05 UTC (permalink / raw)
  To: Matthew Smith, greg; +Cc: devel, arve, riandrews, linux-kernel

On 12/16/2016 02:42 PM, Matthew Smith wrote:
> Remove definition of ION_IOC_CUSTOM from ion.h.
> Remove usage from compat_ion.c and ion-ioctl.c.
> Remove item from TODO file.

The 'remove' from that TODO is more than just removing the code.
There's also an implicit 'replace it with something else'. The
work to do this is still ongoing (see some of my latest patches).
NAK for all 3 of these patches right now.

The patches themselves looked structurally good. For your commit
message next time, explain why the code was being changed, not
just what was changed.

Thanks,
Laura

> 
> Signed-off-by: Matthew Smith <matthew.s3h@gmail.com>
> ---
>  drivers/staging/android/TODO             |  3 ---
>  drivers/staging/android/ion/compat_ion.c |  1 -
>  drivers/staging/android/ion/ion-ioctl.c  |  6 ------
>  drivers/staging/android/uapi/ion.h       | 10 ----------
>  4 files changed, 20 deletions(-)
> 
> diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
> index 8f3ac37..bcf736c 100644
> --- a/drivers/staging/android/TODO
> +++ b/drivers/staging/android/TODO
> @@ -7,9 +7,6 @@ TODO:
>  
>  
>  ion/
> - - Remove ION_IOC_SYNC: Flushing for devices should be purely a kernel internal
> -   interface on top of dma-buf. flush_for_device needs to be added to dma-buf
> -   first.
>   - Remove ION_IOC_CUSTOM: Atm used for cache flushing for cpu access in some
>     vendor trees. Should be replaced with an ioctl on the dma-buf to expose the
>     begin/end_cpu_access hooks to userspace.
> diff --git a/drivers/staging/android/ion/compat_ion.c b/drivers/staging/android/ion/compat_ion.c
> index 9a978d2..b892d3a 100644
> --- a/drivers/staging/android/ion/compat_ion.c
> +++ b/drivers/staging/android/ion/compat_ion.c
> @@ -186,7 +186,6 @@ long compat_ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  	case ION_IOC_SHARE:
>  	case ION_IOC_MAP:
>  	case ION_IOC_IMPORT:
> -	case ION_IOC_SYNC:
>  		return filp->f_op->unlocked_ioctl(filp, cmd,
>  						(unsigned long)compat_ptr(arg));
>  	default:
> diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c
> index 7e7431d..aab086c 100644
> --- a/drivers/staging/android/ion/ion-ioctl.c
> +++ b/drivers/staging/android/ion/ion-ioctl.c
> @@ -51,7 +51,6 @@ static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
>  static unsigned int ion_ioctl_dir(unsigned int cmd)
>  {
>  	switch (cmd) {
> -	case ION_IOC_SYNC:
>  	case ION_IOC_FREE:
>  	case ION_IOC_CUSTOM:
>  		return _IOC_WRITE;
> @@ -146,11 +145,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  			data.handle.handle = handle->id;
>  		break;
>  	}
> -	case ION_IOC_SYNC:
> -	{
> -		ret = ion_sync_for_device(client, data.fd.fd);
> -		break;
> -	}
>  	case ION_IOC_CUSTOM:
>  	{
>  		if (!dev->custom_ioctl)
> diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h
> index 14cd873..c3a87a5 100644
> --- a/drivers/staging/android/uapi/ion.h
> +++ b/drivers/staging/android/uapi/ion.h
> @@ -207,16 +207,6 @@ struct ion_heap_query {
>  #define ION_IOC_IMPORT		_IOWR(ION_IOC_MAGIC, 5, struct ion_fd_data)
>  
>  /**
> - * DOC: ION_IOC_SYNC - syncs a shared file descriptors to memory
> - *
> - * Deprecated in favor of using the dma_buf api's correctly (syncing
> - * will happen automatically when the buffer is mapped to a device).
> - * If necessary should be used after touching a cached buffer from the cpu,
> - * this will make the buffer in memory coherent.
> - */
> -#define ION_IOC_SYNC		_IOWR(ION_IOC_MAGIC, 7, struct ion_fd_data)
> -
> -/**
>   * DOC: ION_IOC_CUSTOM - call architecture specific ion ioctl
>   *
>   * Takes the argument of the architecture specific ioctl to call and
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] staging: android: remove ION_IOC_SYNC
  2016-12-16 23:05 ` [PATCH 1/3] staging: android: remove ION_IOC_SYNC Laura Abbott
@ 2016-12-16 23:27   ` Matthew Smith
  0 siblings, 0 replies; 5+ messages in thread
From: Matthew Smith @ 2016-12-16 23:27 UTC (permalink / raw)
  To: Laura Abbott; +Cc: greg, devel, arve, riandrews, linux-kernel

On Fri, Dec 16, 2016 at 03:05:54PM -0800, Laura Abbott wrote:
> The 'remove' from that TODO is more than just removing the code.
> There's also an implicit 'replace it with something else'. The
> work to do this is still ongoing (see some of my latest patches).
> NAK for all 3 of these patches right now.

Ah that makes a bit more sense.

> The patches themselves looked structurally good. For your commit
> message next time, explain why the code was being changed, not
> just what was changed.

Thank you for the advice, I'll remember that for next time :)

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-16 23:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-16 22:42 [PATCH 1/3] staging: android: remove ION_IOC_SYNC Matthew Smith
2016-12-16 22:42 ` [PATCH 2/3] staging: android: remove ION_IOC_CUSTOM Matthew Smith
2016-12-16 22:42 ` [PATCH 3/3] staging: android: remove compat_get_ion_custom_data Matthew Smith
2016-12-16 23:05 ` [PATCH 1/3] staging: android: remove ION_IOC_SYNC Laura Abbott
2016-12-16 23:27   ` Matthew Smith

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).