* [PATCH RFC] [media] Postpone the addition of MEDIA_IOC_G_TOPOLOGY
@ 2015-12-28 14:03 Mauro Carvalho Chehab
2015-12-28 14:15 ` Javier Martinez Canillas
2015-12-28 15:37 ` Shuah Khan
0 siblings, 2 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2015-12-28 14:03 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-api,
Hans Verkuil, Laurent Pinchart, Javier Martinez Canillas,
Sakari Ailus, Shuah Khan
There are a few discussions left with regards to this ioctl:
1) the name of the new structs will contain _v2_ on it?
2) what's the best alternative to avoid compat32 issues?
Due to that, let's postpone the addition of this new ioctl to
the next Kernel version, to give people more time to discuss it.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
Resending the patch, as the first one were sent to the wrong ML. Also, add the
relevant parties to the Cc of thiis patch.
Documentation/DocBook/media/v4l/media-ioc-g-topology.xml | 3 +++
drivers/media/media-device.c | 5 ++++-
include/uapi/linux/media.h | 6 +++++-
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/media-ioc-g-topology.xml b/Documentation/DocBook/media/v4l/media-ioc-g-topology.xml
index e0d49fa329f0..63152ab9efba 100644
--- a/Documentation/DocBook/media/v4l/media-ioc-g-topology.xml
+++ b/Documentation/DocBook/media/v4l/media-ioc-g-topology.xml
@@ -48,6 +48,9 @@
<refsect1>
<title>Description</title>
+
+ <para><emphasis role="bold">NOTE:</emphasis> This new ioctl is programmed to be added on Kernel 4.6. Its definition/arguments may change until its final version.</para>
+
<para>The typical usage of this ioctl is to call it twice.
On the first call, the structure defined at &media-v2-topology; should
be zeroed. At return, if no errors happen, this ioctl will return the
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 4d1c13de494b..900124c04e9a 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -234,6 +234,7 @@ static long media_device_setup_link(struct media_device *mdev,
return ret;
}
+#if 0 /* Let's postpone it to Kernel 4.6 */
static long __media_device_get_topology(struct media_device *mdev,
struct media_v2_topology *topo)
{
@@ -389,6 +390,7 @@ static long media_device_get_topology(struct media_device *mdev,
return 0;
}
+#endif
static long media_device_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
@@ -422,13 +424,14 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
mutex_unlock(&dev->graph_mutex);
break;
+#if 0 /* Let's postpone it to Kernel 4.6 */
case MEDIA_IOC_G_TOPOLOGY:
mutex_lock(&dev->graph_mutex);
ret = media_device_get_topology(dev,
(struct media_v2_topology __user *)arg);
mutex_unlock(&dev->graph_mutex);
break;
-
+#endif
default:
ret = -ENOIOCTLCMD;
}
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 5dbb208e5451..1e3c8cb43bd7 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -286,7 +286,7 @@ struct media_links_enum {
* later, before the adding this API upstream.
*/
-
+#if 0 /* Let's postpone it to Kernel 4.6 */
struct media_v2_entity {
__u32 id;
char name[64]; /* FIXME: move to a property? (RFC says so) */
@@ -351,6 +351,7 @@ static inline void __user *media_get_uptr(__u64 arg)
{
return (void __user *)(uintptr_t)arg;
}
+#endif
/* ioctls */
@@ -358,6 +359,9 @@ static inline void __user *media_get_uptr(__u64 arg)
#define MEDIA_IOC_ENUM_ENTITIES _IOWR('|', 0x01, struct media_entity_desc)
#define MEDIA_IOC_ENUM_LINKS _IOWR('|', 0x02, struct media_links_enum)
#define MEDIA_IOC_SETUP_LINK _IOWR('|', 0x03, struct media_link_desc)
+
+#if 0 /* Let's postpone it to Kernel 4.6 */
#define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology)
+#endif
#endif /* __LINUX_MEDIA_H */
--
2.5.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] [media] Postpone the addition of MEDIA_IOC_G_TOPOLOGY
2015-12-28 14:03 [PATCH RFC] [media] Postpone the addition of MEDIA_IOC_G_TOPOLOGY Mauro Carvalho Chehab
@ 2015-12-28 14:15 ` Javier Martinez Canillas
2015-12-28 15:37 ` Shuah Khan
1 sibling, 0 replies; 5+ messages in thread
From: Javier Martinez Canillas @ 2015-12-28 14:15 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List
Cc: Mauro Carvalho Chehab, linux-api, Hans Verkuil, Laurent Pinchart,
Sakari Ailus, Shuah Khan
Hello Mauro,
On 12/28/2015 11:03 AM, Mauro Carvalho Chehab wrote:
> There are a few discussions left with regards to this ioctl:
>
> 1) the name of the new structs will contain _v2_ on it?
> 2) what's the best alternative to avoid compat32 issues?
>
> Due to that, let's postpone the addition of this new ioctl to
> the next Kernel version, to give people more time to discuss it.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> ---
>
I agree that leaving the IOCTL out for now is the most sensible thing to
do since as you said, we didn't have time due holidays season to finish
the discussion about the struct media_v2_topology layout.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] [media] Postpone the addition of MEDIA_IOC_G_TOPOLOGY
2015-12-28 14:03 [PATCH RFC] [media] Postpone the addition of MEDIA_IOC_G_TOPOLOGY Mauro Carvalho Chehab
2015-12-28 14:15 ` Javier Martinez Canillas
@ 2015-12-28 15:37 ` Shuah Khan
2015-12-28 15:44 ` Greg KH
1 sibling, 1 reply; 5+ messages in thread
From: Shuah Khan @ 2015-12-28 15:37 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List
Cc: Mauro Carvalho Chehab, linux-api, Hans Verkuil, Laurent Pinchart,
Javier Martinez Canillas, Sakari Ailus, Shuah Khan
On 12/28/2015 07:03 AM, Mauro Carvalho Chehab wrote:
> There are a few discussions left with regards to this ioctl:
>
> 1) the name of the new structs will contain _v2_ on it?
> 2) what's the best alternative to avoid compat32 issues?
>
> Due to that, let's postpone the addition of this new ioctl to
> the next Kernel version, to give people more time to discuss it.
I thought we discussed this in our irc meeting and
arrived at a good solution for compat32 issue
My recommendation is getting this ioctl into 4.5 with
a warning that it could change. The reason for that is
that this ioctl helps with testing the media controller
v2 api. Without this API, we won't see much testing from
userspace in 4.5
Maybe I am missing something, but I do think the benefits
of having this iooctl, I hope outweighs the negatives of
exposing this ioctl to use-space if it includes a warning.
thanks,
-- Shuah
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> ---
>
> Resending the patch, as the first one were sent to the wrong ML. Also, add the
> relevant parties to the Cc of thiis patch.
>
> Documentation/DocBook/media/v4l/media-ioc-g-topology.xml | 3 +++
> drivers/media/media-device.c | 5 ++++-
> include/uapi/linux/media.h | 6 +++++-
> 3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/DocBook/media/v4l/media-ioc-g-topology.xml b/Documentation/DocBook/media/v4l/media-ioc-g-topology.xml
> index e0d49fa329f0..63152ab9efba 100644
> --- a/Documentation/DocBook/media/v4l/media-ioc-g-topology.xml
> +++ b/Documentation/DocBook/media/v4l/media-ioc-g-topology.xml
> @@ -48,6 +48,9 @@
>
> <refsect1>
> <title>Description</title>
> +
> + <para><emphasis role="bold">NOTE:</emphasis> This new ioctl is programmed to be added on Kernel 4.6. Its definition/arguments may change until its final version.</para>
> +
> <para>The typical usage of this ioctl is to call it twice.
> On the first call, the structure defined at &media-v2-topology; should
> be zeroed. At return, if no errors happen, this ioctl will return the
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 4d1c13de494b..900124c04e9a 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -234,6 +234,7 @@ static long media_device_setup_link(struct media_device *mdev,
> return ret;
> }
>
> +#if 0 /* Let's postpone it to Kernel 4.6 */
> static long __media_device_get_topology(struct media_device *mdev,
> struct media_v2_topology *topo)
> {
> @@ -389,6 +390,7 @@ static long media_device_get_topology(struct media_device *mdev,
>
> return 0;
> }
> +#endif
>
> static long media_device_ioctl(struct file *filp, unsigned int cmd,
> unsigned long arg)
> @@ -422,13 +424,14 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
> mutex_unlock(&dev->graph_mutex);
> break;
>
> +#if 0 /* Let's postpone it to Kernel 4.6 */
> case MEDIA_IOC_G_TOPOLOGY:
> mutex_lock(&dev->graph_mutex);
> ret = media_device_get_topology(dev,
> (struct media_v2_topology __user *)arg);
> mutex_unlock(&dev->graph_mutex);
> break;
> -
> +#endif
> default:
> ret = -ENOIOCTLCMD;
> }
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 5dbb208e5451..1e3c8cb43bd7 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -286,7 +286,7 @@ struct media_links_enum {
> * later, before the adding this API upstream.
> */
>
> -
> +#if 0 /* Let's postpone it to Kernel 4.6 */
> struct media_v2_entity {
> __u32 id;
> char name[64]; /* FIXME: move to a property? (RFC says so) */
> @@ -351,6 +351,7 @@ static inline void __user *media_get_uptr(__u64 arg)
> {
> return (void __user *)(uintptr_t)arg;
> }
> +#endif
>
> /* ioctls */
>
> @@ -358,6 +359,9 @@ static inline void __user *media_get_uptr(__u64 arg)
> #define MEDIA_IOC_ENUM_ENTITIES _IOWR('|', 0x01, struct media_entity_desc)
> #define MEDIA_IOC_ENUM_LINKS _IOWR('|', 0x02, struct media_links_enum)
> #define MEDIA_IOC_SETUP_LINK _IOWR('|', 0x03, struct media_link_desc)
> +
> +#if 0 /* Let's postpone it to Kernel 4.6 */
> #define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology)
> +#endif
>
> #endif /* __LINUX_MEDIA_H */
>
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] [media] Postpone the addition of MEDIA_IOC_G_TOPOLOGY
2015-12-28 15:37 ` Shuah Khan
@ 2015-12-28 15:44 ` Greg KH
2015-12-28 16:05 ` Shuah Khan
0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2015-12-28 15:44 UTC (permalink / raw)
To: Shuah Khan
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, linux-api, Hans Verkuil, Laurent Pinchart,
Javier Martinez Canillas, Sakari Ailus
On Mon, Dec 28, 2015 at 08:37:19AM -0700, Shuah Khan wrote:
> On 12/28/2015 07:03 AM, Mauro Carvalho Chehab wrote:
> > There are a few discussions left with regards to this ioctl:
> >
> > 1) the name of the new structs will contain _v2_ on it?
> > 2) what's the best alternative to avoid compat32 issues?
> >
> > Due to that, let's postpone the addition of this new ioctl to
> > the next Kernel version, to give people more time to discuss it.
>
> I thought we discussed this in our irc meeting and
> arrived at a good solution for compat32 issue
>
> My recommendation is getting this ioctl into 4.5 with
> a warning that it could change. The reason for that is
> that this ioctl helps with testing the media controller
> v2 api. Without this API, we won't see much testing from
> userspace in 4.5
People will ignore the warning, that never works :(
sorry,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] [media] Postpone the addition of MEDIA_IOC_G_TOPOLOGY
2015-12-28 15:44 ` Greg KH
@ 2015-12-28 16:05 ` Shuah Khan
0 siblings, 0 replies; 5+ messages in thread
From: Shuah Khan @ 2015-12-28 16:05 UTC (permalink / raw)
To: Greg KH
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, linux-api, Hans Verkuil, Laurent Pinchart,
Javier Martinez Canillas, Sakari Ailus, Shuah Khan
On 12/28/2015 08:44 AM, Greg KH wrote:
> On Mon, Dec 28, 2015 at 08:37:19AM -0700, Shuah Khan wrote:
>> On 12/28/2015 07:03 AM, Mauro Carvalho Chehab wrote:
>>> There are a few discussions left with regards to this ioctl:
>>>
>>> 1) the name of the new structs will contain _v2_ on it?
>>> 2) what's the best alternative to avoid compat32 issues?
>>>
>>> Due to that, let's postpone the addition of this new ioctl to
>>> the next Kernel version, to give people more time to discuss it.
>>
>> I thought we discussed this in our irc meeting and
>> arrived at a good solution for compat32 issue
>>
>> My recommendation is getting this ioctl into 4.5 with
>> a warning that it could change. The reason for that is
>> that this ioctl helps with testing the media controller
>> v2 api. Without this API, we won't see much testing from
>> userspace in 4.5
>
> People will ignore the warning, that never works :(
>
Yeah. If people do ignore warnings, it could become a
problem if we have to make changes.
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-12-28 16:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-28 14:03 [PATCH RFC] [media] Postpone the addition of MEDIA_IOC_G_TOPOLOGY Mauro Carvalho Chehab
2015-12-28 14:15 ` Javier Martinez Canillas
2015-12-28 15:37 ` Shuah Khan
2015-12-28 15:44 ` Greg KH
2015-12-28 16:05 ` Shuah Khan
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).