Hello Dan,
See my reply inline.
At 2025-09-02 14:35:26, "Dan Carpenter" <dan.carpen...@linaro.org> wrote:
>Hello Andy Yan,
>
>Commit 86eecc3a9c2e ("drm/bridge: synopsys: Add DW DPTX Controller
>support library") from Aug 22, 2025 (linux-next), leads to the
>following Smatch static checker warning:
>
> drivers/gpu/drm/bridge/synopsys/dw-dp.c:730 dw_dp_voltage_max()
> warn: bitwise AND is zero '0x3 & 0x18'
Thanks for catching this, I sent a patch try to fix this issue here[0]:
>
>drivers/gpu/drm/bridge/synopsys/dw-dp.c
> 728 static u8 dw_dp_voltage_max(u8 preemph)
> 729 {
>--> 730 switch (preemph & DP_TRAIN_PRE_EMPHASIS_MASK) {
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>This mask will always be zero (DP_TRAIN_PRE_EMPH_LEVEL_0).
>
> 731 case DP_TRAIN_PRE_EMPH_LEVEL_0:
> 732 return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> 733 case DP_TRAIN_PRE_EMPH_LEVEL_1:
> 734 return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> 735 case DP_TRAIN_PRE_EMPH_LEVEL_2:
> 736 return DP_TRAIN_VOLTAGE_SWING_LEVEL_1;
> 737 case DP_TRAIN_PRE_EMPH_LEVEL_3:
> 738 default:
> 739 return DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
> 740 }
> 741 }
>
>The problem is the inconsistent >> 3 shifting. Here is how the
>caller looks like:
>
> 755 p = drm_dp_get_adjust_request_pre_emphasis(status, i);
>
>p is a 0x3 << 3 mask
>
> 756 p >>= DP_TRAIN_PRE_EMPHASIS_SHIFT;
>
>We shift it >> 3 for convenience
>
> 757
> 758 if (v != adj->voltage_swing[i] || p !=
> adj->pre_emphasis[i])
> 759 changed = true;
> 760
> 761 if (p >= (DP_TRAIN_PRE_EMPH_LEVEL_3 >>
> DP_TRAIN_PRE_EMPHASIS_SHIFT)) {
> 762 adj->pre_emphasis[i] =
> DP_TRAIN_PRE_EMPH_LEVEL_3 >>
> 763
> DP_TRAIN_PRE_EMPHASIS_SHIFT;
> 764 adj->pre_max_reached[i] = true;
> 765 } else {
> 766 adj->pre_emphasis[i] = p;
> 767 adj->pre_max_reached[i] = false;
> 768 }
> 769
> 770 v = min(v, dw_dp_voltage_max(p));
> ^
>But the dw_dp_voltage_max() function expects the unshifted value.
>
>There is another similar warning but it's probably deliberate.
Yes, this is deliberate, we will set DW_DP_SDP_HORIZONTAL_INTERVAL when we add
support for audio in the
future. So I think we can let it as it is now.
>drivers/gpu/drm/bridge/synopsys/dw-dp.c:1072 dw_dp_send_sdp() warn: bitwise
>AND is zero '0x1 & 0x2'
>
>drivers/gpu/drm/bridge/synopsys/dw-dp.c
> 1066
> 1067 if (sdp->flags & DW_DP_SDP_VERTICAL_INTERVAL)
> 1068 regmap_update_bits(dp->regmap, DW_DP_SDP_VERTICAL_CTRL,
> 1069 EN_VERTICAL_SDP << nr,
> 1070 EN_VERTICAL_SDP << nr);
> 1071
> 1072 if (sdp->flags & DW_DP_SDP_HORIZONTAL_INTERVAL)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>DW_DP_SDP_HORIZONTAL_INTERVAL is never used.
>
> 1073 regmap_update_bits(dp->regmap,
> DW_DP_SDP_HORIZONTAL_CTRL,
> 1074 EN_HORIZONTAL_SDP << nr,
> 1075 EN_HORIZONTAL_SDP << nr);
> 1076
>
[0]https://lore.kernel.org/dri-devel/20250902112922.684581-1-andys...@163.com/T/#u
>regards,
>dan carpenter
