On Sat, Nov 30, 2019 at 03:51:55PM +0100, Torsten Duwe wrote:

drm_dp_link_rate_to_bw_code and ...bw_code_to_link_rate simply divide by and multiply with 27000, respectively. Avoid an overflow in the u8 dpcd[0] and the multiply+divide alltogether.

This was supposed to be a 1:1 conversion of what the helpers did, but then I tried to be clever and reuse variables rather than add an intermediate one and failed to realize that dpcd[0] was u8 and would therefore overflow with the multiplication.

Personally I think it's slightly nicer to go through the multiply/divide for consistency with how these helpers are used. MAX_LINK_RATE is a bit of an exception in that the field occupies the whole register, but most other registers need to have the fields extracted properly.

In this case it doesn't really matter, so the below is probably good enough.

Good catch!

Reviewed-by: Thierry Reding treding@nvidia.com

fixes: ff1e8fb68ea06027 ("analogix-anx78xx: Avoid drm_dp_link helpers") Signed-off-by: Torsten Duwe duwe@suse.de

---

Has anybody actually tested ff1e8fb68ea06027 ? I copied that code in good faith for the anx6345 and changed a few other things simultaneously, and spent some time wondering why the panel stayed dark.

---

diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c index 41867be03751..864423f59d66 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c @@ -722,10 +722,9 @@ static int anx78xx_dp_link_training(struct anx78xx *anx78xx) if (err) return err;

• dpcd[0] = drm_dp_max_link_rate(anx78xx->dpcd);
• dpcd[0] = drm_dp_link_rate_to_bw_code(dpcd[0]); err = regmap_write(anx78xx->map[I2C_IDX_TX_P0],

• 	   SP_DP_MAIN_LINK_BW_SET_REG, dpcd[0]);
  

• 	   SP_DP_MAIN_LINK_BW_SET_REG,
  

• 	   anx78xx->dpcd[DP_MAX_LINK_RATE]);
  

  if (err) return err;