Hi Jacopo,

Thanks for your patch.

On 2017-06-19 19:04:42 +0200, Jacopo Mondi wrote:
> Support parsing digital input on configurable port id and reg
> properties. Also make the function return -ENOENT when no subdevice is
> found.
> This change is needed to support parsing digital input on Gen3.
> 
> Signed-off-by: Jacopo Mondi <[email protected]>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index 175f138..ef61bcc 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -511,7 +511,9 @@ static int rvin_digital_notify_bound(struct 
> v4l2_async_notifier *notifier,
>       return 0;
>  }
>  
> -static int rvin_digital_graph_parse(struct rvin_dev *vin)
> +static int rvin_digital_graph_parse(struct rvin_dev *vin,
> +                                 unsigned int port,
> +                                 unsigned int reg)
>  {
>       struct device_node *ep, *np;
>       int ret;
> @@ -519,13 +521,9 @@ static int rvin_digital_graph_parse(struct rvin_dev *vin)
>       vin->digital.asd.match.fwnode.fwnode = NULL;
>       vin->digital.subdev = NULL;
>  
> -     /*
> -      * Port 0 id 0 is local digital input, try to get it.
> -      * Not all instances can or will have this, that is OK
> -      */
> -     ep = of_graph_get_endpoint_by_regs(vin->dev->of_node, 0, 0);
> +     ep = of_graph_get_endpoint_by_regs(vin->dev->of_node, port, reg);

I don't think it's necessary to supply the port and reg from the caller.  
If the DT proposed in rcar_vin.txt is used.

>       if (!ep)
> -             return 0;
> +             return -ENOENT;

I'm not saying this is wrong, but why do you change this? I can't see a 
clear advantage in doing so. And if do it I think it should be done in a 
separate patch explaining why.

>  
>       np = of_graph_get_remote_port_parent(ep);
>       if (!np) {
> @@ -555,11 +553,15 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
>       if (ret)
>               return ret;
>  
> -     ret = rvin_digital_graph_parse(vin);
> -     if (ret)
> +     /*
> +      * Port 0 id 0 is local digital input, try to get it.
> +      * Not all instances can or will have this, that is OK
> +      */
> +     ret = rvin_digital_graph_parse(vin, 0, 0);
> +     if (ret && ret != -ENOENT)
>               return ret;
>  
> -     if (!vin->digital.asd.match.fwnode.fwnode) {
> +     if (ret == -ENOENT) {
>               vin_dbg(vin, "No digital subdevice found\n");
>               return -ENODEV;
>       }
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Reply via email to