diff mbox series

[v13,04/21] media: v4l2-fwnode: fix v4l2_fwnode_parse_link handling

Message ID 20200312103156.3178-5-m.felsch@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series TVP5150 Features and Fixes | expand

Commit Message

Marco Felsch March 12, 2020, 10:31 a.m. UTC
Currently the driver differentiate the port number property handling for
ACPI and DT. This is wrong as because ACPI should use the "reg" val too
[1].

[1] https://patchwork.kernel.org/patch/11421985/

Fixes: ca50c197bd96 ("[media] v4l: fwnode: Support generic fwnode for parsing standardised properties")
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sakari Ailus March 12, 2020, 10:37 a.m. UTC | #1
Hi Marco,

On Thu, Mar 12, 2020 at 11:31:39AM +0100, Marco Felsch wrote:
> Currently the driver differentiate the port number property handling for
> ACPI and DT. This is wrong as because ACPI should use the "reg" val too
> [1].
> 
> [1] https://patchwork.kernel.org/patch/11421985/
> 
> Fixes: ca50c197bd96 ("[media] v4l: fwnode: Support generic fwnode for parsing standardised properties")
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 6ece4320e1d2..d56eee9c09b8 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -560,7 +560,7 @@ EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_alloc_parse);
>  int v4l2_fwnode_parse_link(struct fwnode_handle *__fwnode,
>  			   struct v4l2_fwnode_link *link)
>  {
> -	const char *port_prop = is_of_node(__fwnode) ? "reg" : "port";
> +	const char *port_prop = "reg";
>  	struct fwnode_handle *fwnode;
>  
>  	memset(link, 0, sizeof(*link));

I'd omit this patch. With this change, the parsing is no longer compatible
with previous definition (now deprecated) that use the "port" property.

We don't really need to address this for v4l2_fwnode_parse_link(), but
rather should change the users to use other APIs. No driver supporting ACPI
uses this anyway so it's not really an issue right now.
Sakari Ailus March 12, 2020, 10:42 a.m. UTC | #2
On Thu, Mar 12, 2020 at 11:31:39AM +0100, Marco Felsch wrote:
> Currently the driver differentiate the port number property handling for
> ACPI and DT. This is wrong as because ACPI should use the "reg" val too
> [1].
> 
> [1] https://patchwork.kernel.org/patch/11421985/
> 
> Fixes: ca50c197bd96 ("[media] v4l: fwnode: Support generic fwnode for parsing standardised properties")
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Oh well, I guess this is OK; I noticed the next patch that depends on this
one. But it should be merged with the next patch so that it won't be
accidentally backported alone.

Then, please add:

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Sakari Ailus March 12, 2020, 10:43 a.m. UTC | #3
On Thu, Mar 12, 2020 at 12:42:22PM +0200, Sakari Ailus wrote:
> On Thu, Mar 12, 2020 at 11:31:39AM +0100, Marco Felsch wrote:
> > Currently the driver differentiate the port number property handling for
> > ACPI and DT. This is wrong as because ACPI should use the "reg" val too
> > [1].
> > 
> > [1] https://patchwork.kernel.org/patch/11421985/
> > 
> > Fixes: ca50c197bd96 ("[media] v4l: fwnode: Support generic fwnode for parsing standardised properties")
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> Oh well, I guess this is OK; I noticed the next patch that depends on this
> one. But it should be merged with the next patch so that it won't be
> accidentally backported alone.
> 
> Then, please add:
> 
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Forgot that no ACPI driver uses it. So _no_ changes are needed to this, and
you can add the ack.

Thanks.
Marco Felsch March 12, 2020, 11:14 a.m. UTC | #4
Hi Sakari,

On 20-03-12 12:43, Sakari Ailus wrote:
> On Thu, Mar 12, 2020 at 12:42:22PM +0200, Sakari Ailus wrote:
> > On Thu, Mar 12, 2020 at 11:31:39AM +0100, Marco Felsch wrote:
> > > Currently the driver differentiate the port number property handling for
> > > ACPI and DT. This is wrong as because ACPI should use the "reg" val too
> > > [1].
> > > 
> > > [1] https://patchwork.kernel.org/patch/11421985/
> > > 
> > > Fixes: ca50c197bd96 ("[media] v4l: fwnode: Support generic fwnode for parsing standardised properties")
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > 
> > Oh well, I guess this is OK; I noticed the next patch that depends on this
> > one. But it should be merged with the next patch so that it won't be
> > accidentally backported alone.
> > 
> > Then, please add:
> > 
> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> Forgot that no ACPI driver uses it. So _no_ changes are needed to this, and
> you can add the ack.
> 
> Thanks.

I thought about it too and in the end I splitted the patche for possible
backport reasons. Squashing shouldn't be a problem for Hans?

Regards,
  Marco
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 6ece4320e1d2..d56eee9c09b8 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -560,7 +560,7 @@  EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_alloc_parse);
 int v4l2_fwnode_parse_link(struct fwnode_handle *__fwnode,
 			   struct v4l2_fwnode_link *link)
 {
-	const char *port_prop = is_of_node(__fwnode) ? "reg" : "port";
+	const char *port_prop = "reg";
 	struct fwnode_handle *fwnode;
 
 	memset(link, 0, sizeof(*link));