Hi Kevin, > > > + reg->_0b.bits.out_sel = pll->out_sel; > > > + reg->_0b.bits.kint_l = pll->kint & 0xf; > > > + reg->_0e.bits.pll_pu_byp = 0; > > > + reg->_0e.bits.pll_pu = 0; > > > + reg->_0e.bits.stopstate_sel = 1; > > > + reg->_0f.bits.det_delay = pll->det_delay; > > > + > > > + val = (u8 *)® > > > + > > > + for (i = 0; i < sizeof(reg_addr); ++i) { > > > + regmap_write(regmap, reg_addr[i], val[i]); > > > + DRM_DEBUG("%02x: %02x\n", reg_addr[i], val[i]); > > > + } > > > +} > > > > It would be great to also convert this part to a pattern without > > structures. > I will try it, but our pll registers, all not have official name from > aisc design owner. > If need to convert it, our pll regiters macro define, it can only be > named as reg_01, reg_02... IT is better to use the stupid names provided by you asic design owner, than just numbers. Add a comment explaining this is due to the asic design owner the names are stupid and not your fault. > > > + > > > + if (dsi->panel) { > > > + drm_panel_prepare(dsi->panel); > > > + drm_panel_enable(dsi->panel); > > > + } > > > > Please use the new devm_drm_of_get_bridge helper here, and use the > > bridge API instead. > We use drm_panel_init and drm_panel_add API to add panel, so here is a > panel device, not a bridge. The new way to do this is to always wrap the panel in a bridge. We will start to slowly migrate away from direct use of the panel API, and let display drivers always wrap the panles in a bridge. So please do as Maxime suggests. Sam