Re: [PATCH 2/3] drm: rcar-du: Recognize "renesas,vsps" in addition to "vsps"

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Magnus Damm <magnus.damm@gmail.com>,
	dri-devel@lists.freedesktop.org,
	linux-renesas-soc@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 2/3] drm: rcar-du: Recognize "renesas,vsps" in addition to "vsps"
Date: Thu, 7 Nov 2019 04:31:07 +0200	[thread overview]
Message-ID: <20191107023107.GL4878@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20191105183504.21447-3-geert+renesas@glider.be>

Hi Geert,

Thank you for the patch.

On Tue, Nov 05, 2019 at 07:35:03PM +0100, Geert Uytterhoeven wrote:
> The Renesas-specific "vsps" property lacks a vendor prefix.
> Add a "renesas," prefix to comply with DT best practises.
> Retain backward compatibility with old DTBs by falling back to "vsps"
> when needed.
> 
> Fixes: 6d62ef3ac30be756 ("drm: rcar-du: Expose the VSP1 compositor through KMS planes")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

and taken in my tree (with a small change below).

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index 186422ac552b2870..b14676f7a9f17501 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -544,6 +544,7 @@ static int rcar_du_properties_init(struct rcar_du_device *rcdu)
>  static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
>  {
>  	const struct device_node *np = rcdu->dev->of_node;
> +	const char *vsps_prop_name = "renesas,vsps";
>  	struct of_phandle_args args;
>  	struct {
>  		struct device_node *np;
> @@ -559,15 +560,21 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
>  	 * entry contains a pointer to the VSP DT node and a bitmask of the
>  	 * connected DU CRTCs.
>  	 */
> -	cells = of_property_count_u32_elems(np, "vsps") / rcdu->num_crtcs - 1;
> +	ret = of_property_count_u32_elems(np, vsps_prop_name);
> +	if (ret < 0) {
> +		/* Backward compatibility with old DTBs */

s/DTBs/DTBs./

> +		vsps_prop_name = "vsps";
> +		ret = of_property_count_u32_elems(np, vsps_prop_name);
> +	}
> +	cells = ret / rcdu->num_crtcs - 1;
>  	if (cells > 1)
>  		return -EINVAL;
>  
>  	for (i = 0; i < rcdu->num_crtcs; ++i) {
>  		unsigned int j;
>  
> -		ret = of_parse_phandle_with_fixed_args(np, "vsps", cells, i,
> -						       &args);
> +		ret = of_parse_phandle_with_fixed_args(np, vsps_prop_name,
> +						       cells, i, &args);
>  		if (ret < 0)
>  			goto error;
>  
> @@ -589,8 +596,8 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
>  
>  		/*
>  		 * Store the VSP pointer and pipe index in the CRTC. If the
> -		 * second cell of the 'vsps' specifier isn't present, default
> -		 * to 0 to remain compatible with older DT bindings.
> +		 * second cell of the 'renesas,vsps' specifier isn't present,
> +		 * default to 0 to remain compatible with older DT bindings.
>  		 */
>  		rcdu->crtcs[i].vsp = &rcdu->vsps[j];
>  		rcdu->crtcs[i].vsp_pipe = cells >= 1 ? args.args[0] : 0;

-- 
Regards,

Laurent Pinchart