Re: [PATCH] media: Convert to using %pOF instead of full_name

[Rob Herring]

On Thu, Jul 20, 2017 at 6:15 AM, Mauro Carvalho Chehab
 wrote:
> Em Wed, 19 Jul 2017 11:02:01 -0500
> Rob Herring  escreveu:
>
>> On Wed, Jul 19, 2017 at 4:41 AM, Sylwester Nawrocki
>>  wrote:
>> > On 07/18/2017 11:43 PM, Rob Herring wrote:
>> >> Now that we have a custom printf format specifier, convert users of
>> >> full_name to use %pOF instead. This is preparation to remove storing
>> >> of the full path string for each node.
>> >>
>> >> Signed-off-by: Rob Herring 
>> >
>> >> ---
>> >>   drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  3 +-
>> >>   drivers/media/i2c/s5k5baf.c|  7 ++--
>> >>   drivers/media/platform/am437x/am437x-vpfe.c|  4 +-
>> >>   drivers/media/platform/atmel/atmel-isc.c   |  4 +-
>> >>   drivers/media/platform/davinci/vpif_capture.c  | 16 
>> >>   drivers/media/platform/exynos4-is/fimc-is.c|  8 ++--
>> >>   drivers/media/platform/exynos4-is/fimc-lite.c  |  3 +-
>> >>   drivers/media/platform/exynos4-is/media-dev.c  |  8 ++--
>> >>   drivers/media/platform/exynos4-is/mipi-csis.c  |  4 +-
>> >>   drivers/media/platform/mtk-mdp/mtk_mdp_comp.c  |  6 +--
>> >>   drivers/media/platform/mtk-mdp/mtk_mdp_core.c  |  8 ++--
>> >>   drivers/media/platform/omap3isp/isp.c  |  8 ++--
>> >>   drivers/media/platform/pxa_camera.c|  2 +-
>> >>   drivers/media/platform/rcar-vin/rcar-core.c|  4 +-
>> >>   drivers/media/platform/soc_camera/soc_camera.c |  6 +--
>> >>   drivers/media/platform/xilinx/xilinx-vipp.c| 52 
>> >> +-
>> >>   drivers/media/v4l2-core/v4l2-async.c   |  4 +-
>> >>   drivers/media/v4l2-core/v4l2-clk.c |  3 +-
>> >>   include/media/v4l2-clk.h   |  4 +-
>> >>   19 files changed, 71 insertions(+), 83 deletions(-)
>> >
>> >> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c 
>> >> b/drivers/media/platform/xilinx/xilinx-vipp.c
>> >> index ac4704388920..9233ad0b1b6b 100644
>> >> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
>> >> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
>> >
>> >> @@ -144,9 +144,8 @@ static int xvip_graph_build_one(struct 
>> >> xvip_composite_device *xdev,
>> >>   remote = ent->entity;
>> >>
>> >>   if (link.remote_port >= remote->num_pads) {
>> >> - dev_err(xdev->dev, "invalid port number %u on %s\n",
>> >> - link.remote_port,
>> >> - to_of_node(link.remote_node)->full_name);
>> >> + dev_err(xdev->dev, "invalid port number %u on 
>> >> %pOF\n",
>> >> + link.remote_port, link.remote_node);
>> >
>> > Shouldn't there be to_of_node(link.remote_node) instead of 
>> > link.remote_node ?
>>
>> Humm, yes. I thought I fixed that.
>
> After such fix, I'm OK with this patch.

I'll send a new version.

I think I'll send a revert of the referenced commit. It won't apply
cleanly, but at least it will capture the change in behavior and why
it was wrong.

> Are you planning to apply it on your tree, or via the media one?
>
> I guess it is probably better to apply via media, in order to avoid
> conflicts with other changes.

Yes, you can take it.

Rob

Re: [PATCH] media: Convert to using %pOF instead of full_name

[Lad, Prabhakar]

On Tue, Jul 18, 2017 at 10:43 PM, Rob Herring  wrote:
> Now that we have a custom printf format specifier, convert users of
> full_name to use %pOF instead. This is preparation to remove storing
> of the full path string for each node.
>
> Signed-off-by: Rob Herring 
> Cc: Kyungmin Park 
> Cc: Andrzej Hajda 
> Cc: Mauro Carvalho Chehab 
> Cc: "Lad, Prabhakar" 
> Cc: Songjun Wu 
> Cc: Sylwester Nawrocki 
> Cc: Kukjin Kim 
> Cc: Krzysztof Kozlowski 
> Cc: Javier Martinez Canillas 
> Cc: Minghsiu Tsai 
> Cc: Houlong Wei 
> Cc: Andrew-CT Chen 
> Cc: Matthias Brugger 
> Cc: Laurent Pinchart 
> Cc: "Niklas Söderlund" 
> Cc: Guennadi Liakhovetski 
> Cc: Hyun Kwon 
> Cc: Michal Simek 
> Cc: "Sören Brinkmann" 
> Cc: linux-me...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
>  drivers/media/platform/am437x/am437x-vpfe.c|  4 +-
>  drivers/media/platform/davinci/vpif_capture.c  | 16 

For above:

Acked-by: Lad, Prabhakar 

Cheers,
--Prabhakar Lad

Re: [PATCH] media: Convert to using %pOF instead of full_name

[Mauro Carvalho Chehab]

Em Wed, 19 Jul 2017 11:02:01 -0500
Rob Herring  escreveu:

> On Wed, Jul 19, 2017 at 4:41 AM, Sylwester Nawrocki
>  wrote:
> > On 07/18/2017 11:43 PM, Rob Herring wrote:  
> >> Now that we have a custom printf format specifier, convert users of
> >> full_name to use %pOF instead. This is preparation to remove storing
> >> of the full path string for each node.
> >>
> >> Signed-off-by: Rob Herring   
> >  
> >> ---
> >>   drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  3 +-
> >>   drivers/media/i2c/s5k5baf.c|  7 ++--
> >>   drivers/media/platform/am437x/am437x-vpfe.c|  4 +-
> >>   drivers/media/platform/atmel/atmel-isc.c   |  4 +-
> >>   drivers/media/platform/davinci/vpif_capture.c  | 16 
> >>   drivers/media/platform/exynos4-is/fimc-is.c|  8 ++--
> >>   drivers/media/platform/exynos4-is/fimc-lite.c  |  3 +-
> >>   drivers/media/platform/exynos4-is/media-dev.c  |  8 ++--
> >>   drivers/media/platform/exynos4-is/mipi-csis.c  |  4 +-
> >>   drivers/media/platform/mtk-mdp/mtk_mdp_comp.c  |  6 +--
> >>   drivers/media/platform/mtk-mdp/mtk_mdp_core.c  |  8 ++--
> >>   drivers/media/platform/omap3isp/isp.c  |  8 ++--
> >>   drivers/media/platform/pxa_camera.c|  2 +-
> >>   drivers/media/platform/rcar-vin/rcar-core.c|  4 +-
> >>   drivers/media/platform/soc_camera/soc_camera.c |  6 +--
> >>   drivers/media/platform/xilinx/xilinx-vipp.c| 52 
> >> +-
> >>   drivers/media/v4l2-core/v4l2-async.c   |  4 +-
> >>   drivers/media/v4l2-core/v4l2-clk.c |  3 +-
> >>   include/media/v4l2-clk.h   |  4 +-
> >>   19 files changed, 71 insertions(+), 83 deletions(-)  
> >  
> >> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c 
> >> b/drivers/media/platform/xilinx/xilinx-vipp.c
> >> index ac4704388920..9233ad0b1b6b 100644
> >> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> >> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c  
> >  
> >> @@ -144,9 +144,8 @@ static int xvip_graph_build_one(struct 
> >> xvip_composite_device *xdev,
> >>   remote = ent->entity;
> >>
> >>   if (link.remote_port >= remote->num_pads) {
> >> - dev_err(xdev->dev, "invalid port number %u on %s\n",
> >> - link.remote_port,
> >> - to_of_node(link.remote_node)->full_name);
> >> + dev_err(xdev->dev, "invalid port number %u on 
> >> %pOF\n",
> >> + link.remote_port, link.remote_node);  
> >
> > Shouldn't there be to_of_node(link.remote_node) instead of link.remote_node 
> > ?  
> 
> Humm, yes. I thought I fixed that.

After such fix, I'm OK with this patch.

Are you planning to apply it on your tree, or via the media one?

I guess it is probably better to apply via media, in order to avoid
conflicts with other changes.

Thanks,
Mauro

Re: [PATCH] media: Convert to using %pOF instead of full_name

[Sylwester Nawrocki]

On 07/19/2017 06:02 PM, Rob Herring wrote:

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index 851f128eba22..0a385d1ff28c 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -47,9 +47,7 @@ static bool match_fwnode(struct v4l2_subdev *sd, struct 
v4l2_async_subdev *asd)
   if (!is_of_node(sd->fwnode) || !is_of_node(asd->match.fwnode.fwnode))
   return sd->fwnode == asd->match.fwnode.fwnode;

- return !of_node_cmp(of_node_full_name(to_of_node(sd->fwnode)),
- of_node_full_name(
- to_of_node(asd->match.fwnode.fwnode)));
+ return to_of_node(sd->fwnode) == to_of_node(asd->match.fwnode.fwnode);

>>

I'm afraid this will not work, please see commit d2180e0cf77dc7a7049671d5d57d
"[media] v4l: async: make v4l2 coexist with devicetree nodes in a dt overlay"

>

Maybe I'm missing something, but how does that work exactly? Before
the overlay is applied, the remote endpoint node (and its parent)
can't be resolved. In the commit example, the endpoint in the
media_bridge would also have to be part of the overlay. If you apply
and un-apply the overlay, then the of_node (and fw_node) in the
overlay is once again invalid. IOW, you should be in the same state as
before the overlay was applied. The node is still around because of
paranoia that actually freeing nodes would break things. It seems this
paranoia is real, so i think we need to do something to prevent this
from spreading.

Furthermore, it does not appear that any media driver supports
overlays and we have no general way to apply them in mainline yet
(other than an in kernel API). So really this scenario is not one we
have to support yet.


Indeed, the motivation of the above mentioned commit was some out of
tree driver. I don't know was the exact use case, assuming that the
endpoint in the bridge node was also part of the overlay the bridge
driver must have not been rescanning device tree after overlay un-apply
and apply. Currently there is no other way to do this than to unbind
and bind. So the bridge driver must have been referencing an already
invalid node as you point out.

I haven't been following DT overlays very closely, as Frank explains
your change seems to be actually an improvement of current code.

--
Thanks,
Sylwester

Re: [PATCH] media: Convert to using %pOF instead of full_name

[Frank Rowand]

On 07/19/17 09:02, Rob Herring wrote:
> On Wed, Jul 19, 2017 at 4:41 AM, Sylwester Nawrocki
>  wrote:
>> On 07/18/2017 11:43 PM, Rob Herring wrote:
>>> Now that we have a custom printf format specifier, convert users of
>>> full_name to use %pOF instead. This is preparation to remove storing
>>> of the full path string for each node.
>>>
>>> Signed-off-by: Rob Herring 
>>
>>> ---

< snip >

>>> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
>>> b/drivers/media/v4l2-core/v4l2-async.c
>>> index 851f128eba22..0a385d1ff28c 100644
>>> --- a/drivers/media/v4l2-core/v4l2-async.c
>>> +++ b/drivers/media/v4l2-core/v4l2-async.c
>>> @@ -47,9 +47,7 @@ static bool match_fwnode(struct v4l2_subdev *sd, struct 
>>> v4l2_async_subdev *asd)
>>>   if (!is_of_node(sd->fwnode) || !is_of_node(asd->match.fwnode.fwnode))
>>>   return sd->fwnode == asd->match.fwnode.fwnode;
>>>
>>> - return !of_node_cmp(of_node_full_name(to_of_node(sd->fwnode)),
>>> - of_node_full_name(
>>> - to_of_node(asd->match.fwnode.fwnode)));
>>> + return to_of_node(sd->fwnode) == to_of_node(asd->match.fwnode.fwnode);
>>
>> I'm afraid this will not work, please see commit d2180e0cf77dc7a7049671d5d57d
>> "[media] v4l: async: make v4l2 coexist with devicetree nodes in a dt overlay"

Commit d2180e0cf77dc7a7049671d5d57d seems to have a fundamental
misunderstanding of overlays, if I understand the implications of that
commit.

When an overlay (1) is removed, all uses and references to the nodes and
properties in that overlay are no longer valid.  Any driver that uses any
information from the overlay _must_ stop using any data from the overlay.
Any driver that is bound to a new node in the overlay _must_ unbind.  Any
driver that became bound to a pre-existing node that was modified by the
overlay (became bound after the overlay was applied) _must_ adjust itself
to account for any changes to that node when the overlay is removed.  One
way to do this is to unbind when notified that the overlay is about to
be removed, then to re-bind after the overlay is completely removed.

If an overlay (2) is subsequently applied, a node with the same
full_name as from overlay (1) may exist.  There is no guarantee
that overlay (1) and overlay (2) are the same overlay, even if
that node has the same full_name in both cases.


> Maybe I'm missing something, but how does that work exactly? Before
> the overlay is applied, the remote endpoint node (and its parent)
> can't be resolved. In the commit example, the endpoint in the
> media_bridge would also have to be part of the overlay. If you apply
> and un-apply the overlay, then the of_node (and fw_node) in the
> overlay is once again invalid. IOW, you should be in the same state as
> before the overlay was applied. The node is still around because of
> paranoia that actually freeing nodes would break things. It seems this
> paranoia is real, so i think we need to do something to prevent this
> from spreading.

My understanding is that nodes from an un-applied overlay will be
freed if the expanded device tree that was used to create it has
its reference counts drop to zero.  But I wouldn't count on me
remembering this correctly without actually walking through all
the code.  And as far as I know, there is no example of this
in the mainline tree.


> Furthermore, it does not appear that any media driver supports
> overlays and we have no general way to apply them in mainline yet
> (other than an in kernel API). So really this scenario is not one we
> have to support yet.
> 
> Rob
> 

Re: [PATCH] media: Convert to using %pOF instead of full_name

[Rob Herring]

On Wed, Jul 19, 2017 at 4:41 AM, Sylwester Nawrocki
 wrote:
> On 07/18/2017 11:43 PM, Rob Herring wrote:
>> Now that we have a custom printf format specifier, convert users of
>> full_name to use %pOF instead. This is preparation to remove storing
>> of the full path string for each node.
>>
>> Signed-off-by: Rob Herring 
>
>> ---
>>   drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  3 +-
>>   drivers/media/i2c/s5k5baf.c|  7 ++--
>>   drivers/media/platform/am437x/am437x-vpfe.c|  4 +-
>>   drivers/media/platform/atmel/atmel-isc.c   |  4 +-
>>   drivers/media/platform/davinci/vpif_capture.c  | 16 
>>   drivers/media/platform/exynos4-is/fimc-is.c|  8 ++--
>>   drivers/media/platform/exynos4-is/fimc-lite.c  |  3 +-
>>   drivers/media/platform/exynos4-is/media-dev.c  |  8 ++--
>>   drivers/media/platform/exynos4-is/mipi-csis.c  |  4 +-
>>   drivers/media/platform/mtk-mdp/mtk_mdp_comp.c  |  6 +--
>>   drivers/media/platform/mtk-mdp/mtk_mdp_core.c  |  8 ++--
>>   drivers/media/platform/omap3isp/isp.c  |  8 ++--
>>   drivers/media/platform/pxa_camera.c|  2 +-
>>   drivers/media/platform/rcar-vin/rcar-core.c|  4 +-
>>   drivers/media/platform/soc_camera/soc_camera.c |  6 +--
>>   drivers/media/platform/xilinx/xilinx-vipp.c| 52 
>> +-
>>   drivers/media/v4l2-core/v4l2-async.c   |  4 +-
>>   drivers/media/v4l2-core/v4l2-clk.c |  3 +-
>>   include/media/v4l2-clk.h   |  4 +-
>>   19 files changed, 71 insertions(+), 83 deletions(-)
>
>> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c 
>> b/drivers/media/platform/xilinx/xilinx-vipp.c
>> index ac4704388920..9233ad0b1b6b 100644
>> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
>> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
>
>> @@ -144,9 +144,8 @@ static int xvip_graph_build_one(struct 
>> xvip_composite_device *xdev,
>>   remote = ent->entity;
>>
>>   if (link.remote_port >= remote->num_pads) {
>> - dev_err(xdev->dev, "invalid port number %u on %s\n",
>> - link.remote_port,
>> - to_of_node(link.remote_node)->full_name);
>> + dev_err(xdev->dev, "invalid port number %u on %pOF\n",
>> + link.remote_port, link.remote_node);
>
> Shouldn't there be to_of_node(link.remote_node) instead of link.remote_node ?

Humm, yes. I thought I fixed that.

>
>>   v4l2_fwnode_put_link();
>>   ret = -EINVAL;
>>   break;
>
>> @@ -242,17 +241,17 @@ static int xvip_graph_build_dma(struct 
>> xvip_composite_device *xdev)
>>   ent = xvip_graph_find_entity(xdev,
>>to_of_node(link.remote_node));
>>   if (ent == NULL) {
>> - dev_err(xdev->dev, "no entity found for %s\n",
>> - to_of_node(link.remote_node)->full_name);
>> + dev_err(xdev->dev, "no entity found for %pOF\n",
>> + to_of_node(link.remote_node));
>>   v4l2_fwnode_put_link();
>>   ret = -ENODEV;
>>   break;
>>   }
>
>> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
>> b/drivers/media/v4l2-core/v4l2-async.c
>> index 851f128eba22..0a385d1ff28c 100644
>> --- a/drivers/media/v4l2-core/v4l2-async.c
>> +++ b/drivers/media/v4l2-core/v4l2-async.c
>> @@ -47,9 +47,7 @@ static bool match_fwnode(struct v4l2_subdev *sd, struct 
>> v4l2_async_subdev *asd)
>>   if (!is_of_node(sd->fwnode) || !is_of_node(asd->match.fwnode.fwnode))
>>   return sd->fwnode == asd->match.fwnode.fwnode;
>>
>> - return !of_node_cmp(of_node_full_name(to_of_node(sd->fwnode)),
>> - of_node_full_name(
>> - to_of_node(asd->match.fwnode.fwnode)));
>> + return to_of_node(sd->fwnode) == to_of_node(asd->match.fwnode.fwnode);
>
> I'm afraid this will not work, please see commit d2180e0cf77dc7a7049671d5d57d
> "[media] v4l: async: make v4l2 coexist with devicetree nodes in a dt overlay"

Maybe I'm missing something, but how does that work exactly? Before
the overlay is applied, the remote endpoint node (and its parent)
can't be resolved. In the commit example, the endpoint in the
media_bridge would also have to be part of the overlay. If you apply
and un-apply the overlay, then the of_node (and fw_node) in the
overlay is once again invalid. IOW, you should be in the same state as
before the overlay was applied. The node is still around because of
paranoia that actually freeing nodes would break things. It seems this
paranoia is real, so i think we need to do something to prevent this
from spreading.

Furthermore, it does not appear that any media driver supports
overlays and we have no 

Re: [PATCH] media: Convert to using %pOF instead of full_name

[Nicolas Ferre]

On 18/07/2017 at 23:43, Rob Herring wrote:
> Now that we have a custom printf format specifier, convert users of
> full_name to use %pOF instead. This is preparation to remove storing
> of the full path string for each node.
> 
> Signed-off-by: Rob Herring 
> Cc: Kyungmin Park 
> Cc: Andrzej Hajda 
> Cc: Mauro Carvalho Chehab 
> Cc: "Lad, Prabhakar" 
> Cc: Songjun Wu 
> Cc: Sylwester Nawrocki 
> Cc: Kukjin Kim 
> Cc: Krzysztof Kozlowski 
> Cc: Javier Martinez Canillas 
> Cc: Minghsiu Tsai 
> Cc: Houlong Wei 
> Cc: Andrew-CT Chen 
> Cc: Matthias Brugger 
> Cc: Laurent Pinchart 
> Cc: "Niklas Söderlund" 
> Cc: Guennadi Liakhovetski 
> Cc: Hyun Kwon 
> Cc: Michal Simek 
> Cc: "Sören Brinkmann" 
> Cc: linux-me...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
>  drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  3 +-
>  drivers/media/i2c/s5k5baf.c|  7 ++--
>  drivers/media/platform/am437x/am437x-vpfe.c|  4 +-
>  drivers/media/platform/atmel/atmel-isc.c   |  4 +-

Instead of Songjun, here is my:
Acked-by: Nicolas Ferre 


>  drivers/media/platform/davinci/vpif_capture.c  | 16 
>  drivers/media/platform/exynos4-is/fimc-is.c|  8 ++--
>  drivers/media/platform/exynos4-is/fimc-lite.c  |  3 +-
>  drivers/media/platform/exynos4-is/media-dev.c  |  8 ++--
>  drivers/media/platform/exynos4-is/mipi-csis.c  |  4 +-
>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c  |  6 +--
>  drivers/media/platform/mtk-mdp/mtk_mdp_core.c  |  8 ++--
>  drivers/media/platform/omap3isp/isp.c  |  8 ++--
>  drivers/media/platform/pxa_camera.c|  2 +-
>  drivers/media/platform/rcar-vin/rcar-core.c|  4 +-
>  drivers/media/platform/soc_camera/soc_camera.c |  6 +--
>  drivers/media/platform/xilinx/xilinx-vipp.c| 52 
> +-
>  drivers/media/v4l2-core/v4l2-async.c   |  4 +-
>  drivers/media/v4l2-core/v4l2-clk.c |  3 +-
>  include/media/v4l2-clk.h   |  4 +-
>  19 files changed, 71 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c 
> b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> index f434fb2ee6fc..cdc4f2392ef9 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> @@ -1635,8 +1635,7 @@ static int s5c73m3_get_platform_data(struct s5c73m3 
> *state)
> 
>   node_ep = of_graph_get_next_endpoint(node, NULL);
>   if (!node_ep) {
> - dev_warn(dev, "no endpoint defined for node: %s\n",
> - node->full_name);
> + dev_warn(dev, "no endpoint defined for node: %pOF\n", node);
>   return 0;
>   }
> 
> diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
> index 962051b9939d..9c22fc963901 100644
> --- a/drivers/media/i2c/s5k5baf.c
> +++ b/drivers/media/i2c/s5k5baf.c
> @@ -1863,8 +1863,7 @@ static int s5k5baf_parse_device_node(struct s5k5baf 
> *state, struct device *dev)
> 
>   node_ep = of_graph_get_next_endpoint(node, NULL);
>   if (!node_ep) {
> - dev_err(dev, "no endpoint defined at node %s\n",
> - node->full_name);
> + dev_err(dev, "no endpoint defined at node %pOF\n", node);
>   return -EINVAL;
>   }
> 
> @@ -1882,8 +1881,8 @@ static int s5k5baf_parse_device_node(struct s5k5baf 
> *state, struct device *dev)
>   case V4L2_MBUS_PARALLEL:
>   break;
>   default:
> - dev_err(dev, "unsupported bus in endpoint defined at node %s\n",
> - node->full_name);
> + dev_err(dev, "unsupported bus in endpoint defined at node 
> %pOF\n",
> + node);
>   return -EINVAL;
>   }
> 
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
> b/drivers/media/platform/am437x/am437x-vpfe.c
> index 466aba8b0e00..dfcc484cab89 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -2490,8 +2490,8 @@ vpfe_get_pdata(struct platform_device *pdev)
> 
>   rem = of_graph_get_remote_port_parent(endpoint);
>   if (!rem) {
> - dev_err(>dev, "Remote device at %s not found\n",
> - endpoint->full_name);
> + 

Re: [PATCH] media: Convert to using %pOF instead of full_name

[Matthias Brugger]

On 07/18/2017 11:43 PM, Rob Herring wrote:

Now that we have a custom printf format specifier, convert users of
full_name to use %pOF instead. This is preparation to remove storing
of the full path string for each node.

Signed-off-by: Rob Herring 
Cc: Kyungmin Park 
Cc: Andrzej Hajda 
Cc: Mauro Carvalho Chehab 
Cc: "Lad, Prabhakar" 
Cc: Songjun Wu 
Cc: Sylwester Nawrocki 
Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 
Cc: Javier Martinez Canillas 
Cc: Minghsiu Tsai 
Cc: Houlong Wei 
Cc: Andrew-CT Chen 
Cc: Matthias Brugger 
Cc: Laurent Pinchart 
Cc: "Niklas Söderlund" 
Cc: Guennadi Liakhovetski 
Cc: Hyun Kwon 
Cc: Michal Simek 
Cc: "Sören Brinkmann" 
Cc: linux-me...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-media...@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
---
  drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  3 +-
  drivers/media/i2c/s5k5baf.c|  7 ++--
  drivers/media/platform/am437x/am437x-vpfe.c|  4 +-
  drivers/media/platform/atmel/atmel-isc.c   |  4 +-
  drivers/media/platform/davinci/vpif_capture.c  | 16 
  drivers/media/platform/exynos4-is/fimc-is.c|  8 ++--
  drivers/media/platform/exynos4-is/fimc-lite.c  |  3 +-
  drivers/media/platform/exynos4-is/media-dev.c  |  8 ++--
  drivers/media/platform/exynos4-is/mipi-csis.c  |  4 +-
  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c  |  6 +--
  drivers/media/platform/mtk-mdp/mtk_mdp_core.c  |  8 ++--


For mediatek parts:

Reviewed-by: Matthias Brugger 

Re: [PATCH] media: Convert to using %pOF instead of full_name

[Sylwester Nawrocki]

On 07/18/2017 11:43 PM, Rob Herring wrote:
> Now that we have a custom printf format specifier, convert users of
> full_name to use %pOF instead. This is preparation to remove storing
> of the full path string for each node.
> 
> Signed-off-by: Rob Herring 

> ---
>   drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  3 +-
>   drivers/media/i2c/s5k5baf.c|  7 ++--
>   drivers/media/platform/am437x/am437x-vpfe.c|  4 +-
>   drivers/media/platform/atmel/atmel-isc.c   |  4 +-
>   drivers/media/platform/davinci/vpif_capture.c  | 16 
>   drivers/media/platform/exynos4-is/fimc-is.c|  8 ++--
>   drivers/media/platform/exynos4-is/fimc-lite.c  |  3 +-
>   drivers/media/platform/exynos4-is/media-dev.c  |  8 ++--
>   drivers/media/platform/exynos4-is/mipi-csis.c  |  4 +-
>   drivers/media/platform/mtk-mdp/mtk_mdp_comp.c  |  6 +--
>   drivers/media/platform/mtk-mdp/mtk_mdp_core.c  |  8 ++--
>   drivers/media/platform/omap3isp/isp.c  |  8 ++--
>   drivers/media/platform/pxa_camera.c|  2 +-
>   drivers/media/platform/rcar-vin/rcar-core.c|  4 +-
>   drivers/media/platform/soc_camera/soc_camera.c |  6 +--
>   drivers/media/platform/xilinx/xilinx-vipp.c| 52 
> +-
>   drivers/media/v4l2-core/v4l2-async.c   |  4 +-
>   drivers/media/v4l2-core/v4l2-clk.c |  3 +-
>   include/media/v4l2-clk.h   |  4 +-
>   19 files changed, 71 insertions(+), 83 deletions(-)

> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c 
> b/drivers/media/platform/xilinx/xilinx-vipp.c
> index ac4704388920..9233ad0b1b6b 100644
> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c

> @@ -144,9 +144,8 @@ static int xvip_graph_build_one(struct 
> xvip_composite_device *xdev,
>   remote = ent->entity;
> 
>   if (link.remote_port >= remote->num_pads) {
> - dev_err(xdev->dev, "invalid port number %u on %s\n",
> - link.remote_port,
> - to_of_node(link.remote_node)->full_name);
> + dev_err(xdev->dev, "invalid port number %u on %pOF\n",
> + link.remote_port, link.remote_node);

Shouldn't there be to_of_node(link.remote_node) instead of link.remote_node ?

>   v4l2_fwnode_put_link();
>   ret = -EINVAL;
>   break;

> @@ -242,17 +241,17 @@ static int xvip_graph_build_dma(struct 
> xvip_composite_device *xdev)
>   ent = xvip_graph_find_entity(xdev,
>to_of_node(link.remote_node));
>   if (ent == NULL) {
> - dev_err(xdev->dev, "no entity found for %s\n",
> - to_of_node(link.remote_node)->full_name);
> + dev_err(xdev->dev, "no entity found for %pOF\n",
> + to_of_node(link.remote_node));
>   v4l2_fwnode_put_link();
>   ret = -ENODEV;
>   break;
>   }

> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 851f128eba22..0a385d1ff28c 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -47,9 +47,7 @@ static bool match_fwnode(struct v4l2_subdev *sd, struct 
> v4l2_async_subdev *asd)
>   if (!is_of_node(sd->fwnode) || !is_of_node(asd->match.fwnode.fwnode))
>   return sd->fwnode == asd->match.fwnode.fwnode;
> 
> - return !of_node_cmp(of_node_full_name(to_of_node(sd->fwnode)),
> - of_node_full_name(
> - to_of_node(asd->match.fwnode.fwnode)));
> + return to_of_node(sd->fwnode) == to_of_node(asd->match.fwnode.fwnode);

I'm afraid this will not work, please see commit d2180e0cf77dc7a7049671d5d57d
"[media] v4l: async: make v4l2 coexist with devicetree nodes in a dt overlay"

--
Regards,
Sylwester

Re: [PATCH] media: Convert to using %pOF instead of full_name

[Laurent Pinchart]

Hi Rob,

Thank you for the patch.

On Tuesday 18 Jul 2017 16:43:13 Rob Herring wrote:
> Now that we have a custom printf format specifier, convert users of
> full_name to use %pOF instead. This is preparation to remove storing
> of the full path string for each node.
> 
> Signed-off-by: Rob Herring 
> Cc: Kyungmin Park 
> Cc: Andrzej Hajda 
> Cc: Mauro Carvalho Chehab 
> Cc: "Lad, Prabhakar" 
> Cc: Songjun Wu 
> Cc: Sylwester Nawrocki 
> Cc: Kukjin Kim 
> Cc: Krzysztof Kozlowski 
> Cc: Javier Martinez Canillas 
> Cc: Minghsiu Tsai 
> Cc: Houlong Wei 
> Cc: Andrew-CT Chen 
> Cc: Matthias Brugger 
> Cc: Laurent Pinchart 
> Cc: "Niklas Söderlund" 
> Cc: Guennadi Liakhovetski 
> Cc: Hyun Kwon 
> Cc: Michal Simek 
> Cc: "Sören Brinkmann" 
> Cc: linux-me...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
>  drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  3 +-
>  drivers/media/i2c/s5k5baf.c|  7 ++--
>  drivers/media/platform/am437x/am437x-vpfe.c|  4 +-
>  drivers/media/platform/atmel/atmel-isc.c   |  4 +-
>  drivers/media/platform/davinci/vpif_capture.c  | 16 
>  drivers/media/platform/exynos4-is/fimc-is.c|  8 ++--
>  drivers/media/platform/exynos4-is/fimc-lite.c  |  3 +-
>  drivers/media/platform/exynos4-is/media-dev.c  |  8 ++--
>  drivers/media/platform/exynos4-is/mipi-csis.c  |  4 +-
>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c  |  6 +--
>  drivers/media/platform/mtk-mdp/mtk_mdp_core.c  |  8 ++--
>  drivers/media/platform/omap3isp/isp.c  |  8 ++--
>  drivers/media/platform/pxa_camera.c|  2 +-
>  drivers/media/platform/rcar-vin/rcar-core.c|  4 +-
>  drivers/media/platform/soc_camera/soc_camera.c |  6 +--
>  drivers/media/platform/xilinx/xilinx-vipp.c| 52 +++---
>  drivers/media/v4l2-core/v4l2-async.c   |  4 +-
>  drivers/media/v4l2-core/v4l2-clk.c |  3 +-
>  include/media/v4l2-clk.h   |  4 +-
>  19 files changed, 71 insertions(+), 83 deletions(-)

For omap3isp and xilinx,

Acked-by: Laurent Pinchart 

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: Convert to using %pOF instead of full_name

[Niklas Söderlund]

On 2017-07-18 16:43:13 -0500, Rob Herring wrote:
> Now that we have a custom printf format specifier, convert users of
> full_name to use %pOF instead. This is preparation to remove storing
> of the full path string for each node.
> 
> Signed-off-by: Rob Herring 
> Cc: Kyungmin Park 
> Cc: Andrzej Hajda 
> Cc: Mauro Carvalho Chehab 
> Cc: "Lad, Prabhakar" 
> Cc: Songjun Wu 
> Cc: Sylwester Nawrocki 
> Cc: Kukjin Kim 
> Cc: Krzysztof Kozlowski 
> Cc: Javier Martinez Canillas 
> Cc: Minghsiu Tsai 
> Cc: Houlong Wei 
> Cc: Andrew-CT Chen 
> Cc: Matthias Brugger 
> Cc: Laurent Pinchart 
> Cc: "Niklas Söderlund" 
> Cc: Guennadi Liakhovetski 
> Cc: Hyun Kwon 
> Cc: Michal Simek 
> Cc: "Sören Brinkmann" 
> Cc: linux-me...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-renesas-soc@vger.kernel.org

For rcar-vin/rcar-core.c:
Acked-by: Niklas Söderlund 

> ---
>  drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  3 +-
>  drivers/media/i2c/s5k5baf.c|  7 ++--
>  drivers/media/platform/am437x/am437x-vpfe.c|  4 +-
>  drivers/media/platform/atmel/atmel-isc.c   |  4 +-
>  drivers/media/platform/davinci/vpif_capture.c  | 16 
>  drivers/media/platform/exynos4-is/fimc-is.c|  8 ++--
>  drivers/media/platform/exynos4-is/fimc-lite.c  |  3 +-
>  drivers/media/platform/exynos4-is/media-dev.c  |  8 ++--
>  drivers/media/platform/exynos4-is/mipi-csis.c  |  4 +-
>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c  |  6 +--
>  drivers/media/platform/mtk-mdp/mtk_mdp_core.c  |  8 ++--
>  drivers/media/platform/omap3isp/isp.c  |  8 ++--
>  drivers/media/platform/pxa_camera.c|  2 +-
>  drivers/media/platform/rcar-vin/rcar-core.c|  4 +-
>  drivers/media/platform/soc_camera/soc_camera.c |  6 +--
>  drivers/media/platform/xilinx/xilinx-vipp.c| 52 
> +-
>  drivers/media/v4l2-core/v4l2-async.c   |  4 +-
>  drivers/media/v4l2-core/v4l2-clk.c |  3 +-
>  include/media/v4l2-clk.h   |  4 +-
>  19 files changed, 71 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c 
> b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> index f434fb2ee6fc..cdc4f2392ef9 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> @@ -1635,8 +1635,7 @@ static int s5c73m3_get_platform_data(struct s5c73m3 
> *state)
> 
>   node_ep = of_graph_get_next_endpoint(node, NULL);
>   if (!node_ep) {
> - dev_warn(dev, "no endpoint defined for node: %s\n",
> - node->full_name);
> + dev_warn(dev, "no endpoint defined for node: %pOF\n", node);
>   return 0;
>   }
> 
> diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
> index 962051b9939d..9c22fc963901 100644
> --- a/drivers/media/i2c/s5k5baf.c
> +++ b/drivers/media/i2c/s5k5baf.c
> @@ -1863,8 +1863,7 @@ static int s5k5baf_parse_device_node(struct s5k5baf 
> *state, struct device *dev)
> 
>   node_ep = of_graph_get_next_endpoint(node, NULL);
>   if (!node_ep) {
> - dev_err(dev, "no endpoint defined at node %s\n",
> - node->full_name);
> + dev_err(dev, "no endpoint defined at node %pOF\n", node);
>   return -EINVAL;
>   }
> 
> @@ -1882,8 +1881,8 @@ static int s5k5baf_parse_device_node(struct s5k5baf 
> *state, struct device *dev)
>   case V4L2_MBUS_PARALLEL:
>   break;
>   default:
> - dev_err(dev, "unsupported bus in endpoint defined at node %s\n",
> - node->full_name);
> + dev_err(dev, "unsupported bus in endpoint defined at node 
> %pOF\n",
> + node);
>   return -EINVAL;
>   }
> 
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
> b/drivers/media/platform/am437x/am437x-vpfe.c
> index 466aba8b0e00..dfcc484cab89 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -2490,8 +2490,8 @@ vpfe_get_pdata(struct platform_device *pdev)
> 
>   rem = of_graph_get_remote_port_parent(endpoint);
>   if (!rem) {
> - dev_err(>dev, "Remote device at %s not found\n",
> - endpoint->full_name);
> +