On Fri, Jan 20, 2023 at 09:31:43AM +0000, Sverdlin, Alexander wrote: > Hello Sascha, > > On Fri, 2023-01-20 at 10:16 +0100, Sascha Hauer wrote: > > > + /* Enable LVDS mode */ > > > + return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2, > > > + RK3568_LVDS0_MODE_EN(1), > > > + RK3568_LVDS0_MODE_EN(1)); > > > > Isn't this the same as: > > > > regmap_write(lvds->grf, RK3568_GRF_VO_CON2, > > RK3568_LVDS0_MODE_EN(1)); > > > > Unless I am missing something I find a plain regmap_write() easier to > > read. > > the former is setting a bit in a RMW operation, the latter is a plain > write, isn't it? That's right from the view what the function itself does. Note the registers that are accessed here are a bit special. They effectively are 16bit wide, the upper 16bit contain a mask. Only the bits set in the mask are actually modified in the lower bits. See the register bit definitions: #define HIWORD_UPDATE(v, h, l) ((GENMASK(h, l) << 16) | ((v) << (l))) #define RK3568_LVDS0_SELECT(val) HIWORD_UPDATE(val, 5, 4) Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
