Hi Jani, On Tue, Feb 25, 2025 at 09:52:41AM +0200, Jani Nikula wrote: > On Mon, 24 Feb 2025, Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx> wrote: > > Hi Ville, > > > > On Wed, Feb 12, 2025 at 01:19:37AM +0200, Ville Syrjala wrote: > >> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >> > >> Convert the BDW+ EU/slice fuse bits to the modern REG_BIT()/etc. > >> style. > > > > using REG_BIT() and co. doesn't alway make it more readable. In > > some of the cases below I would have preferred not to use it. > > Interesting. I read through the patch and I thought all of it was > good. Care to elaborate? yes you're right, I should have given an example, but I had already edited the e-mail and I was lazy to get it back. In any case, this is an example: - s_en = intel_uncore_read(uncore, GEN11_GT_SLICE_ENABLE) & - GEN11_GT_S_ENA_MASK; + s_en = REG_FIELD_GET(GEN11_GT_S_ENA_MASK, + intel_uncore_read(uncore, GEN11_GT_SLICE_ENABLE)); The removed line to me is clearer than the added line. I'm not saying that it's not good (otherwise I wouldn't have r-b'ed it), I'm just saying that not always using the REG_* macros makes the code clearer. For consistency with the rest of the patch is anyway fine. Thanks, Andi