diff mbox

[v7,25/44,media] replace all occurrences of MEDIA_ENT_T_DEVNODE_V4L

Message ID 23e2f9440a259e1162e15dba7e6261dbc4c521c6.1440359643.git.mchehab@osg.samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mauro Carvalho Chehab Aug. 23, 2015, 8:17 p.m. UTC
Now that interfaces and entities are distinct, it makes no sense
of keeping something named as MEDIA_ENT_T_DEVNODE.

This change was done with this script:

	for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed s,MEDIA_ENT_T_DEVNODE_V4L,MEDIA_ENT_T_V4L2_VIDEO, <$i >a && mv a $i; done

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Comments

Hans Verkuil Aug. 25, 2015, 9:23 a.m. UTC | #1
On 08/23/15 22:17, Mauro Carvalho Chehab wrote:
> Now that interfaces and entities are distinct, it makes no sense
> of keeping something named as MEDIA_ENT_T_DEVNODE.
> 
> This change was done with this script:
> 
> 	for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed s,MEDIA_ENT_T_DEVNODE_V4L,MEDIA_ENT_T_V4L2_VIDEO, <$i >a && mv a $i; done
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> index 5872f8bbf774..910243d4edb8 100644
> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> @@ -183,7 +183,7 @@
>  	    <entry>Unknown device node</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_DEVNODE_V4L</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_V4L2_VIDEO</constant></entry>
>  	    <entry>V4L video, radio or vbi device node</entry>
>  	  </row>

OK, this makes no sense and that ties in with my confusion of the previous patch.

These are not device nodes, in the new scheme these are DMA entities (I know,
naming TDB) that have an associated interface.

I think a much better approach would be to add entity type(s) for such DMA
engines in patch 24, then use that new name in existing drivers and split
up the existing DEVNODE_V4L media_entity into a media_entity and a
media_intf_devnode:

The current media_entity defined in struct video_device has to be replaced
by media_intf_devnode, and the DMA entity has to be added as a new entity
to these drivers.

This reflects these two action items from our meeting:

Migration: add v4l-subdev media_interface: Laurent
Migration: add explicit DMA Engine entity: Laurent

Unless Laurent says differently I think this is something you'll have to
do given Laurent's workload.

I think doing this at this stage of the patch series is crucial, otherwise
the remaining patches really make no sense.

I'll skip reviewing patches 26-38 for now.

Regards,

	Hans

>  	  <row>
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> index 92e8116dc28f..88cd789cdaf7 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.c
> +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> @@ -193,7 +193,7 @@ static int xvip_pipeline_validate(struct xvip_pipeline *pipe,
>  	while ((entity = media_entity_graph_walk_next(&graph))) {
>  		struct xvip_dma *dma;
>  
> -		if (entity->type != MEDIA_ENT_T_DEVNODE_V4L)
> +		if (entity->type != MEDIA_ENT_T_V4L2_VIDEO)
>  			continue;
>  
>  		dma = to_xvip_dma(media_entity_to_video_device(entity));
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 71a1b93b0790..44b330589787 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -912,7 +912,7 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
>  	/* Part 5: Register the entity. */
>  	if (vdev->v4l2_dev->mdev &&
>  	    vdev->vfl_type != VFL_TYPE_SUBDEV) {
> -		vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L;
> +		vdev->entity.type = MEDIA_ENT_T_V4L2_VIDEO;
>  		vdev->entity.name = vdev->name;
>  		vdev->entity.info.dev.major = VIDEO_MAJOR;
>  		vdev->entity.info.dev.minor = vdev->minor;
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 83615b8fb46a..e6e1115d8215 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -535,7 +535,7 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad,
>  		return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
>  	}
>  
> -	WARN(pad->entity->type != MEDIA_ENT_T_DEVNODE_V4L,
> +	WARN(pad->entity->type != MEDIA_ENT_T_V4L2_VIDEO,
>  	     "Driver bug! Wrong media entity type 0x%08x, entity %s\n",
>  	     pad->entity->type, pad->entity->name);
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mauro Carvalho Chehab Aug. 25, 2015, 11:32 a.m. UTC | #2
Em Tue, 25 Aug 2015 11:23:24 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 08/23/15 22:17, Mauro Carvalho Chehab wrote:
> > Now that interfaces and entities are distinct, it makes no sense
> > of keeping something named as MEDIA_ENT_T_DEVNODE.
> > 
> > This change was done with this script:
> > 
> > 	for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed s,MEDIA_ENT_T_DEVNODE_V4L,MEDIA_ENT_T_V4L2_VIDEO, <$i >a && mv a $i; done
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > index 5872f8bbf774..910243d4edb8 100644
> > --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > @@ -183,7 +183,7 @@
> >  	    <entry>Unknown device node</entry>
> >  	  </row>
> >  	  <row>
> > -	    <entry><constant>MEDIA_ENT_T_DEVNODE_V4L</constant></entry>
> > +	    <entry><constant>MEDIA_ENT_T_V4L2_VIDEO</constant></entry>
> >  	    <entry>V4L video, radio or vbi device node</entry>
> >  	  </row>
> 
> OK, this makes no sense and that ties in with my confusion of the previous patch.
> 
> These are not device nodes, in the new scheme these are DMA entities (I know,
> naming TDB) that have an associated interface.

Yes. Well, DMA is a bad name. It won't cover USB devices, where the DMA
engine is outside the V4L2 drivers, nor it would work for RDS radio data,
with may not need any DMA at all on no-USB devices, as the data flows via
the I2C bus.

> I think a much better approach would be to add entity type(s) for such DMA
> engines in patch 24, then use that new name in existing drivers and split
> up the existing DEVNODE_V4L media_entity into a media_entity and a
> media_intf_devnode:

Sorry, but I didn't get. That's precisely what I did ;)

> The current media_entity defined in struct video_device has to be replaced
> by media_intf_devnode, and the DMA entity has to be added as a new entity
> to these drivers.

If I do this way, it would break bisectability. I need first to replace
the names, but keep them as entities, and then add the interfaces.

> 
> This reflects these two action items from our meeting:
> 
> Migration: add v4l-subdev media_interface: Laurent
> Migration: add explicit DMA Engine entity: Laurent
> 
> Unless Laurent says differently I think this is something you'll have to
> do given Laurent's workload.

Yes. The above action items are covered on this series.

What patch 24 does is to define the new namespace, moving the legacy
symbols kept due to backward compatibility on a separate part of the
header.

Then, patches 25-38 replace the occurrences of the deprecated names
by the new ones.

Nothing is touched at the interfaces yet, to avoid breaking bisectability.

Then, the next patches add interfaces support at the V4L side.

> I think doing this at this stage of the patch series is crucial, otherwise
> the remaining patches really make no sense.
> 
> I'll skip reviewing patches 26-38 for now.
> 
> Regards,
> 
> 	Hans
> 
> >  	  <row>
> > diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> > index 92e8116dc28f..88cd789cdaf7 100644
> > --- a/drivers/media/platform/xilinx/xilinx-dma.c
> > +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> > @@ -193,7 +193,7 @@ static int xvip_pipeline_validate(struct xvip_pipeline *pipe,
> >  	while ((entity = media_entity_graph_walk_next(&graph))) {
> >  		struct xvip_dma *dma;
> >  
> > -		if (entity->type != MEDIA_ENT_T_DEVNODE_V4L)
> > +		if (entity->type != MEDIA_ENT_T_V4L2_VIDEO)
> >  			continue;
> >  
> >  		dma = to_xvip_dma(media_entity_to_video_device(entity));
> > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> > index 71a1b93b0790..44b330589787 100644
> > --- a/drivers/media/v4l2-core/v4l2-dev.c
> > +++ b/drivers/media/v4l2-core/v4l2-dev.c
> > @@ -912,7 +912,7 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
> >  	/* Part 5: Register the entity. */
> >  	if (vdev->v4l2_dev->mdev &&
> >  	    vdev->vfl_type != VFL_TYPE_SUBDEV) {
> > -		vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L;
> > +		vdev->entity.type = MEDIA_ENT_T_V4L2_VIDEO;
> >  		vdev->entity.name = vdev->name;
> >  		vdev->entity.info.dev.major = VIDEO_MAJOR;
> >  		vdev->entity.info.dev.minor = vdev->minor;
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > index 83615b8fb46a..e6e1115d8215 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -535,7 +535,7 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad,
> >  		return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
> >  	}
> >  
> > -	WARN(pad->entity->type != MEDIA_ENT_T_DEVNODE_V4L,
> > +	WARN(pad->entity->type != MEDIA_ENT_T_V4L2_VIDEO,
> >  	     "Driver bug! Wrong media entity type 0x%08x, entity %s\n",
> >  	     pad->entity->type, pad->entity->name);
> >  
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans Verkuil Aug. 25, 2015, 1:54 p.m. UTC | #3
On 08/25/15 13:32, Mauro Carvalho Chehab wrote:
> Em Tue, 25 Aug 2015 11:23:24 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 08/23/15 22:17, Mauro Carvalho Chehab wrote:
>>> Now that interfaces and entities are distinct, it makes no sense
>>> of keeping something named as MEDIA_ENT_T_DEVNODE.
>>>
>>> This change was done with this script:
>>>
>>> 	for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed s,MEDIA_ENT_T_DEVNODE_V4L,MEDIA_ENT_T_V4L2_VIDEO, <$i >a && mv a $i; done
>>>
>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
>>> index 5872f8bbf774..910243d4edb8 100644
>>> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
>>> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
>>> @@ -183,7 +183,7 @@
>>>  	    <entry>Unknown device node</entry>
>>>  	  </row>
>>>  	  <row>
>>> -	    <entry><constant>MEDIA_ENT_T_DEVNODE_V4L</constant></entry>
>>> +	    <entry><constant>MEDIA_ENT_T_V4L2_VIDEO</constant></entry>
>>>  	    <entry>V4L video, radio or vbi device node</entry>
>>>  	  </row>
>>
>> OK, this makes no sense and that ties in with my confusion of the previous patch.
>>
>> These are not device nodes, in the new scheme these are DMA entities (I know,
>> naming TDB) that have an associated interface.
> 
> Yes. Well, DMA is a bad name. It won't cover USB devices, where the DMA
> engine is outside the V4L2 drivers, nor it would work for RDS radio data,
> with may not need any DMA at all on no-USB devices, as the data flows via
> the I2C bus.
> 
>> I think a much better approach would be to add entity type(s) for such DMA
>> engines in patch 24, then use that new name in existing drivers and split
>> up the existing DEVNODE_V4L media_entity into a media_entity and a
>> media_intf_devnode:
> 
> Sorry, but I didn't get. That's precisely what I did ;)
> 
>> The current media_entity defined in struct video_device has to be replaced
>> by media_intf_devnode, and the DMA entity has to be added as a new entity
>> to these drivers.
> 
> If I do this way, it would break bisectability. I need first to replace
> the names, but keep them as entities, and then add the interfaces.
> 
>>
>> This reflects these two action items from our meeting:
>>
>> Migration: add v4l-subdev media_interface: Laurent
>> Migration: add explicit DMA Engine entity: Laurent
>>
>> Unless Laurent says differently I think this is something you'll have to
>> do given Laurent's workload.
> 
> Yes. The above action items are covered on this series.
> 
> What patch 24 does is to define the new namespace, moving the legacy
> symbols kept due to backward compatibility on a separate part of the
> header.
> 
> Then, patches 25-38 replace the occurrences of the deprecated names
> by the new ones.
> 
> Nothing is touched at the interfaces yet, to avoid breaking bisectability.

I don't follow why that would break bisect.

> Then, the next patches add interfaces support at the V4L side.

So this is not yet included in this patch series? That would explain
my confusion. If it is, then I need to take another look on Friday.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mauro Carvalho Chehab Aug. 25, 2015, 3:12 p.m. UTC | #4
Em Tue, 25 Aug 2015 15:54:09 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 08/25/15 13:32, Mauro Carvalho Chehab wrote:
> > Em Tue, 25 Aug 2015 11:23:24 +0200
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> > 
> >> On 08/23/15 22:17, Mauro Carvalho Chehab wrote:
> >>> Now that interfaces and entities are distinct, it makes no sense
> >>> of keeping something named as MEDIA_ENT_T_DEVNODE.
> >>>
> >>> This change was done with this script:
> >>>
> >>> 	for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed s,MEDIA_ENT_T_DEVNODE_V4L,MEDIA_ENT_T_V4L2_VIDEO, <$i >a && mv a $i; done
> >>>
> >>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >>>
> >>> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> >>> index 5872f8bbf774..910243d4edb8 100644
> >>> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> >>> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> >>> @@ -183,7 +183,7 @@
> >>>  	    <entry>Unknown device node</entry>
> >>>  	  </row>
> >>>  	  <row>
> >>> -	    <entry><constant>MEDIA_ENT_T_DEVNODE_V4L</constant></entry>
> >>> +	    <entry><constant>MEDIA_ENT_T_V4L2_VIDEO</constant></entry>
> >>>  	    <entry>V4L video, radio or vbi device node</entry>
> >>>  	  </row>
> >>
> >> OK, this makes no sense and that ties in with my confusion of the previous patch.
> >>
> >> These are not device nodes, in the new scheme these are DMA entities (I know,
> >> naming TDB) that have an associated interface.
> > 
> > Yes. Well, DMA is a bad name. It won't cover USB devices, where the DMA
> > engine is outside the V4L2 drivers, nor it would work for RDS radio data,
> > with may not need any DMA at all on no-USB devices, as the data flows via
> > the I2C bus.
> > 
> >> I think a much better approach would be to add entity type(s) for such DMA
> >> engines in patch 24, then use that new name in existing drivers and split
> >> up the existing DEVNODE_V4L media_entity into a media_entity and a
> >> media_intf_devnode:
> > 
> > Sorry, but I didn't get. That's precisely what I did ;)
> > 
> >> The current media_entity defined in struct video_device has to be replaced
> >> by media_intf_devnode, and the DMA entity has to be added as a new entity
> >> to these drivers.
> > 
> > If I do this way, it would break bisectability. I need first to replace
> > the names, but keep them as entities, and then add the interfaces.
> > 
> >>
> >> This reflects these two action items from our meeting:
> >>
> >> Migration: add v4l-subdev media_interface: Laurent
> >> Migration: add explicit DMA Engine entity: Laurent
> >>
> >> Unless Laurent says differently I think this is something you'll have to
> >> do given Laurent's workload.
> > 
> > Yes. The above action items are covered on this series.
> > 
> > What patch 24 does is to define the new namespace, moving the legacy
> > symbols kept due to backward compatibility on a separate part of the
> > header.
> > 
> > Then, patches 25-38 replace the occurrences of the deprecated names
> > by the new ones.
> > 
> > Nothing is touched at the interfaces yet, to avoid breaking bisectability.
> 
> I don't follow why that would break bisect.

It won't break compilation, but it will break runtime.

I mean: if we replace the current occurrences of the
"video output data entities" [1] any userspace app that would be used to test 
somethingwill stop working.

Ok, that means that it would break bisectability for us ;)
Still, better to avoid.

[1] I don't like the "DMA" entities term, as it is too broken.
I prefer to refer to them with some other name, like I/O entities. 
However, even this name is not perfect. Those are, in reality, a
"data interface", while what we call interface is actually a 
"control interface", but calling like that would be confusing, I think.
So, I'll simply call it as "video/vbi/... output data entities".

> 
> > Then, the next patches add interfaces support at the V4L side.
> 
> So this is not yet included in this patch series? That would explain
> my confusion. If it is, then I need to take another look on Friday.

It is on the 3 patches I sent yesterday, after this patch series:
	https://patchwork.linuxtv.org/patch/31081/
	https://patchwork.linuxtv.org/patch/31082/
	https://patchwork.linuxtv.org/patch/31083/

Please notice that the above patch series is not complete, as
there's something non-trivial to be addressed on non-subdev
V4L2 interfaces: how to create the indirect links.

By indirect links, I meant to refer to the interface links that
don't control an entity directly, but via the internal hardware
control/I2C bus(es).

So, a video interface, on a PC customer's hardware controls not only
the video output data entity, but it also indirectly controls the
tuner and the analog demod. Or, on a webcam hardware, it will also
controls the sensor.

However, on platform drivers, it controls just the
"video output data entity" that is directly associated with it via 
its device node.

We need to add some support to automatically create those links,
once available, but only if the device is a PC customer's hardware.

Btw, that's another reason to postpone it: creating the interfaces
offer this additional challenge, while creating the entities are
easy, as nothing changes there.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans Verkuil Aug. 25, 2015, 3:22 p.m. UTC | #5
On 08/25/2015 05:12 PM, Mauro Carvalho Chehab wrote:
> Em Tue, 25 Aug 2015 15:54:09 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 08/25/15 13:32, Mauro Carvalho Chehab wrote:
>>> Em Tue, 25 Aug 2015 11:23:24 +0200
>>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>>
>>>> On 08/23/15 22:17, Mauro Carvalho Chehab wrote:
>>>>> Now that interfaces and entities are distinct, it makes no sense
>>>>> of keeping something named as MEDIA_ENT_T_DEVNODE.
>>>>>
>>>>> This change was done with this script:
>>>>>
>>>>> 	for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed s,MEDIA_ENT_T_DEVNODE_V4L,MEDIA_ENT_T_V4L2_VIDEO, <$i >a && mv a $i; done
>>>>>
>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>>>
>>>>> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
>>>>> index 5872f8bbf774..910243d4edb8 100644
>>>>> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
>>>>> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
>>>>> @@ -183,7 +183,7 @@
>>>>>  	    <entry>Unknown device node</entry>
>>>>>  	  </row>
>>>>>  	  <row>
>>>>> -	    <entry><constant>MEDIA_ENT_T_DEVNODE_V4L</constant></entry>
>>>>> +	    <entry><constant>MEDIA_ENT_T_V4L2_VIDEO</constant></entry>
>>>>>  	    <entry>V4L video, radio or vbi device node</entry>
>>>>>  	  </row>
>>>>
>>>> OK, this makes no sense and that ties in with my confusion of the previous patch.
>>>>
>>>> These are not device nodes, in the new scheme these are DMA entities (I know,
>>>> naming TDB) that have an associated interface.
>>>
>>> Yes. Well, DMA is a bad name. It won't cover USB devices, where the DMA
>>> engine is outside the V4L2 drivers, nor it would work for RDS radio data,
>>> with may not need any DMA at all on no-USB devices, as the data flows via
>>> the I2C bus.
>>>
>>>> I think a much better approach would be to add entity type(s) for such DMA
>>>> engines in patch 24, then use that new name in existing drivers and split
>>>> up the existing DEVNODE_V4L media_entity into a media_entity and a
>>>> media_intf_devnode:
>>>
>>> Sorry, but I didn't get. That's precisely what I did ;)
>>>
>>>> The current media_entity defined in struct video_device has to be replaced
>>>> by media_intf_devnode, and the DMA entity has to be added as a new entity
>>>> to these drivers.
>>>
>>> If I do this way, it would break bisectability. I need first to replace
>>> the names, but keep them as entities, and then add the interfaces.
>>>
>>>>
>>>> This reflects these two action items from our meeting:
>>>>
>>>> Migration: add v4l-subdev media_interface: Laurent
>>>> Migration: add explicit DMA Engine entity: Laurent
>>>>
>>>> Unless Laurent says differently I think this is something you'll have to
>>>> do given Laurent's workload.
>>>
>>> Yes. The above action items are covered on this series.
>>>
>>> What patch 24 does is to define the new namespace, moving the legacy
>>> symbols kept due to backward compatibility on a separate part of the
>>> header.
>>>
>>> Then, patches 25-38 replace the occurrences of the deprecated names
>>> by the new ones.
>>>
>>> Nothing is touched at the interfaces yet, to avoid breaking bisectability.
>>
>> I don't follow why that would break bisect.
> 
> It won't break compilation, but it will break runtime.
> 
> I mean: if we replace the current occurrences of the
> "video output data entities" [1] any userspace app that would be used to test 
> somethingwill stop working.
> 
> Ok, that means that it would break bisectability for us ;)
> Still, better to avoid.
> 
> [1] I don't like the "DMA" entities term, as it is too broken.
> I prefer to refer to them with some other name, like I/O entities. 
> However, even this name is not perfect. Those are, in reality, a
> "data interface", while what we call interface is actually a 
> "control interface", but calling like that would be confusing, I think.
> So, I'll simply call it as "video/vbi/... output data entities".
> 
>>
>>> Then, the next patches add interfaces support at the V4L side.
>>
>> So this is not yet included in this patch series? That would explain
>> my confusion. If it is, then I need to take another look on Friday.
> 
> It is on the 3 patches I sent yesterday, after this patch series:
> 	https://patchwork.linuxtv.org/patch/31081/
> 	https://patchwork.linuxtv.org/patch/31082/
> 	https://patchwork.linuxtv.org/patch/31083/
> 

Ah, OK. These are not part of this patch series, so that explains
it. I hadn't gotten around to reviewing these 3.

I'll plan reviewing these on Friday and I'll revisit the patches
I skipped in the 44 part-series with this in mind.

Regards,

	Hans


> Please notice that the above patch series is not complete, as
> there's something non-trivial to be addressed on non-subdev
> V4L2 interfaces: how to create the indirect links.
> 
> By indirect links, I meant to refer to the interface links that
> don't control an entity directly, but via the internal hardware
> control/I2C bus(es).
> 
> So, a video interface, on a PC customer's hardware controls not only
> the video output data entity, but it also indirectly controls the
> tuner and the analog demod. Or, on a webcam hardware, it will also
> controls the sensor.
> 
> However, on platform drivers, it controls just the
> "video output data entity" that is directly associated with it via 
> its device node.
> 
> We need to add some support to automatically create those links,
> once available, but only if the device is a PC customer's hardware.
> 
> Btw, that's another reason to postpone it: creating the interfaces
> offer this additional challenge, while creating the entities are
> easy, as nothing changes there.
> 
> Regards,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
index 5872f8bbf774..910243d4edb8 100644
--- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
+++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
@@ -183,7 +183,7 @@ 
 	    <entry>Unknown device node</entry>
 	  </row>
 	  <row>
-	    <entry><constant>MEDIA_ENT_T_DEVNODE_V4L</constant></entry>
+	    <entry><constant>MEDIA_ENT_T_V4L2_VIDEO</constant></entry>
 	    <entry>V4L video, radio or vbi device node</entry>
 	  </row>
 	  <row>
diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index 92e8116dc28f..88cd789cdaf7 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -193,7 +193,7 @@  static int xvip_pipeline_validate(struct xvip_pipeline *pipe,
 	while ((entity = media_entity_graph_walk_next(&graph))) {
 		struct xvip_dma *dma;
 
-		if (entity->type != MEDIA_ENT_T_DEVNODE_V4L)
+		if (entity->type != MEDIA_ENT_T_V4L2_VIDEO)
 			continue;
 
 		dma = to_xvip_dma(media_entity_to_video_device(entity));
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 71a1b93b0790..44b330589787 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -912,7 +912,7 @@  int __video_register_device(struct video_device *vdev, int type, int nr,
 	/* Part 5: Register the entity. */
 	if (vdev->v4l2_dev->mdev &&
 	    vdev->vfl_type != VFL_TYPE_SUBDEV) {
-		vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L;
+		vdev->entity.type = MEDIA_ENT_T_V4L2_VIDEO;
 		vdev->entity.name = vdev->name;
 		vdev->entity.info.dev.major = VIDEO_MAJOR;
 		vdev->entity.info.dev.minor = vdev->minor;
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 83615b8fb46a..e6e1115d8215 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -535,7 +535,7 @@  v4l2_subdev_link_validate_get_format(struct media_pad *pad,
 		return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
 	}
 
-	WARN(pad->entity->type != MEDIA_ENT_T_DEVNODE_V4L,
+	WARN(pad->entity->type != MEDIA_ENT_T_V4L2_VIDEO,
 	     "Driver bug! Wrong media entity type 0x%08x, entity %s\n",
 	     pad->entity->type, pad->entity->name);