Re: [PATCH] drm/bridge: ti-sn65dsi83: Handle dsi_lanes == 0 as invalid

From: Andrzej Hajda <andrzej.hajda@intel.com>
To: Marek Vasut <marex@denx.de>, <dri-devel@lists.freedesktop.org>
Cc: Neil Armstrong <narmstrong@baylibre.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	robert.foss@linaro.org, Maxime Ripard <maxime@cerno.tech>,
	Sam Ravnborg <sam@ravnborg.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH] drm/bridge: ti-sn65dsi83: Handle dsi_lanes == 0 as invalid
Date: Thu, 19 May 2022 09:43:36 +0200	[thread overview]
Message-ID: <e4eeb60e-c694-cdf1-7ef8-e6180de20b1b@intel.com> (raw)
In-Reply-To: <20220518233844.248504-1-marex@denx.de>

On 19.05.2022 01:38, Marek Vasut wrote:
> Handle empty data-lanes = < >; property, which translates to
> dsi_lanes = 0 as invalid.
> 
> Fixes: ceb515ba29ba6 ("drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver")
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Maxime Ripard <maxime@cerno.tech>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Robert Foss <robert.foss@linaro.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>

I wonder if you could not make common helper to get number of lanes with 
checks included, quite trivial, but maybe worth to standardize it.

Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

Regards
Andrzej
> ---
>   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index d64d4385188dd..dc65f424e7f3c 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -585,7 +585,7 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
>   	ctx->host_node = of_graph_get_remote_port_parent(endpoint);
>   	of_node_put(endpoint);
>   
> -	if (ctx->dsi_lanes < 0 || ctx->dsi_lanes > 4) {
> +	if (ctx->dsi_lanes <= 0 || ctx->dsi_lanes > 4) {
>   		ret = -EINVAL;
>   		goto err_put_node;
>   	}