On Wed, 2022-06-08 at 16:45 +0800, CK Hu wrote: > Hi, Rex: > > On Mon, 2022-05-23 at 12:47 +0200, Guillaume Ranquet wrote: > > From: Markus Schneider-Pargmann <msp@xxxxxxxxxxxx> > > > > This patch adds a DisplayPort driver for the Mediatek mt8195 SoC. > > > > It supports the mt8195, the embedded DisplayPort units. It offers > > DisplayPort 1.4 with up to 4 lanes. > > > > The driver creates a child device for the phy. The child device > > will > > never exist without the parent being active. As they are sharing a > > register range, the parent passes a regmap pointer to the child so > > that > > both can work with the same register range. The phy driver sets > > device > > data that is read by the parent to get the phy device that can be > > used > > to control the phy properties. > > > > This driver is based on an initial version by > > Jason-JH.Lin <jason-jh.lin@xxxxxxxxxxxx>. > > > > Signed-off-by: Markus Schneider-Pargmann <msp@xxxxxxxxxxxx> > > Signed-off-by: Guillaume Ranquet <granquet@xxxxxxxxxxxx> > > --- > > [snip] > > > + > > +static bool mtk_dp_set_swing_pre_emphasis(struct mtk_dp *mtk_dp, > > int > > lane_num, > > + int swing_val, int > > preemphasis) > > The return value is never processed, so let this function to be void. > > Regards, > CK > Hello CK, ok, I will drop this. Actually, I change "mtk_dp_write", "mtk_dp_update_bits" and "mtk_dp_bulk_16bit_write" to return void. I don't think we need to handle the issue that we failed to set registers. If we failed to set register, it's because hw is not enable. Therefore, I drop this and we can reduce many lines of codes. BRs, Bo-Chen > > +{ > > + int ret; > > + > > + u32 lane_shift = lane_num * DP_TX1_VOLT_SWING_SHIFT; > > + > > + if (lane_num < 0 || lane_num > 3) > > lane_num < 0 would not happen. lane_num > 3 only if device tree max > lane is wrong. So I would like to checkout max lane when parsing > device > tree instead of checking here. > > + return false; > > + > > + dev_dbg(mtk_dp->dev, > > + "link training swing_val= 0x%x, preemphasis = 0x%x\n", > > + swing_val, preemphasis); > > + > > + ret = mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_SWING_EMP, > > + swing_val << (DP_TX0_VOLT_SWING_SHIFT > > + lane_shift), > > + DP_TX0_VOLT_SWING_MASK << lane_shift); > > + if (ret) > > + return ret; > > + ret = mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_SWING_EMP, > > + preemphasis << (DP_TX0_PRE_EMPH_SHIFT > > + lane_shift), > > + DP_TX0_PRE_EMPH_MASK << lane_shift); > > + > > + return !ret; > > +} > > + > >