Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[s.hauer@xxxxxxxxxxxxxx]

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 |