Re: [PATCH 2/9] drm/bridge: tc358767: Simplify tc_stream_clock_calc()

[Andrzej Hajda]

On 26.02.2019 20:36, Andrey Smirnov wrote:
> Drop the use of tc_write() as well as "magicly" used "ret" and "err:"
> and replace it with a simple call to regmap_write(). No functional
> change intended.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx>
> Cc: Archit Taneja <architt@xxxxxxxxxxxxxx>
> Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
> Cc: Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: Chris Healy <cphealy@xxxxxxxxx>
> Cc: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index b0f8264a1285..86ebd49194b7 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -504,7 +504,6 @@ static int tc_pxl_pll_dis(struct tc_data *tc)
>  
>  static int tc_stream_clock_calc(struct tc_data *tc)
>  {
> -	int ret;
>  	/*
>  	 * If the Stream clock and Link Symbol clock are
>  	 * asynchronous with each other, the value of M changes over
> @@ -520,11 +519,7 @@ static int tc_stream_clock_calc(struct tc_data *tc)
>  	 * M/N = f_STRMCLK / f_LSCLK
>  	 *
>  	 */
> -	tc_write(DP0_VIDMNGEN1, 32768);
> -
> -	return 0;
> -err:
> -	return ret;
> +	return regmap_write(tc->regmap, DP0_VIDMNGEN1, 32768);


The patch looks semantically correct, but you are dropping here custom
accessor (tc_write) in favor of regmap API.

I think the best would be consistent across the whole driver: either use
only accessors, either drop them entirely and use regmap API.

And it would be good to have comment of the original authors about this
change.


Regards

Andrzej


>  }
>  
>  static int tc_aux_link_setup(struct tc_data *tc)