linux-kernel - Re: [PATCH 9/9] drm/bridge: tc358767: Drop tc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190304123918.GK6325@pendragon.ideasonboard.com>
Date:   Mon, 4 Mar 2019 14:39:18 +0200
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Andrey Smirnov <andrew.smirnov@...il.com>
Cc:     dri-devel@...ts.freedesktop.org,
        Archit Taneja <architt@...eaurora.org>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Chris Healy <cphealy@...il.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 9/9] drm/bridge: tc358767: Drop tc_read() macro

Hi Andrey,

Thank you for the patch.

On Tue, Feb 26, 2019 at 11:36:09AM -0800, Andrey Smirnov wrote:
> There's only one place where tc_read() is used, so it doesn't save us
> much. Drop it. No functional change intended.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@...il.com>
> Cc: Archit Taneja <architt@...eaurora.org>
> Cc: Andrzej Hajda <a.hajda@...sung.com>
> Cc: Laurent Pinchart <Laurent.pinchart@...asonboard.com>
> Cc: Chris Healy <cphealy@...il.com>
> Cc: Lucas Stach <l.stach@...gutronix.de>
> Cc: dri-devel@...ts.freedesktop.org
> Cc: linux-kernel@...r.kernel.org
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 239b3aaa255d..3c574f1569aa 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -240,12 +240,6 @@ static inline struct tc_data *connector_to_tc(struct drm_connector *c)
>  		if (ret)					\
>  			goto err;				\
>  	} while (0)
> -#define tc_read(reg, var)					\
> -	do {							\
> -		ret = regmap_read(tc->regmap, reg, var);	\
> -		if (ret)					\
> -			goto err;				\
> -	} while (0)

While I really like removing the goto from the macro, I think we should
either have accessors for both read and write, or remove them
completely. How about just dropping the goto in this patch, and decide
separately whether to keep accessors or remove them ?

>  static inline int tc_poll_timeout(struct regmap *map, unsigned int addr,
>  				  unsigned int cond_mask,
> @@ -337,8 +331,13 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
>  	if (request == DP_AUX_I2C_READ || request == DP_AUX_NATIVE_READ) {
>  		/* Read data */
>  		while (i < size) {
> -			if ((i % 4) == 0)
> -				tc_read(DP0_AUXRDATA(i >> 2), &tmp);
> +			if ((i % 4) == 0) {
> +				ret = regmap_read(tc->regmap,
> +						  DP0_AUXRDATA(i >> 2),
> +						  &tmp);
> +				if (ret)
> +					goto err;

You can return ret directly here.

> +			}
>  			buf[i] = tmp & 0xff;
>  			tmp = tmp >> 8;
>  			i++;

-- 
Regards,

Laurent Pinchart