Quoting Doug Anderson (2020-05-05 11:45:05) > On Mon, May 4, 2020 at 10:44 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > > > Quoting Douglas Anderson (2020-05-04 21:36:31) > > > regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG, > > > CHA_DSI_LANES_MASK, val); > > > > > > + regmap_write(pdata->regmap, SN_LN_ASSIGN_REG, pdata->ln_assign); > > > + regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, LN_POLRS_MASK, > > > + pdata->ln_polrs << LN_POLRS_OFFSET); > > > + > > > /* set dsi clk frequency value */ > > > ti_sn_bridge_set_dsi_rate(pdata); > > > > > > @@ -1063,6 +1066,50 @@ static int ti_sn_setup_gpio_controller(struct ti_sn_bridge *pdata) > > > return ret; > > > } > > > > > > +static void ti_sn_bridge_parse_lanes(struct ti_sn_bridge *pdata, > > > + struct device_node *np) > > > +{ > > > + u32 lane_assignments[SN_MAX_DP_LANES] = { 0, 1, 2, 3 }; > > > + u32 lane_polarities[SN_MAX_DP_LANES] = { }; > > > + struct device_node *endpoint; > > > + u8 ln_assign = 0; > > > + u8 ln_polrs = 0; > > > > Do we need to assign to 0 to start? Seems like no? > > Yes. See usage: > > ln_assign = ln_assign << LN_ASSIGN_WIDTH | lane_assignments[i]; > ln_polrs = ln_polrs << 1 | lane_polarities[i]; > > Notably each time we shift a new bit in we base on the old value. If > you think it'll make it clearer, I can put this initialization at the > beginning of the loop. It's 2 extra lines of code but if it adds > clarity I'll do it. No it doesn't really make it any clearer.