On 28-08-20, 11:03, Pierre-Louis Bossart wrote: > Hi Vinod, > This change to use FIELD_PREP/GET looks good, the code is indeed a lot > clearer, but ... > > > +#define SDW_DISCO_LINK_ID(adr) FIELD_GET(SDW_DISCO_LINK_ID_MASK, addr) > > +#define SDW_VERSION(adr) FIELD_GET(SDW_VERSION_MASK, addr) > > +#define SDW_UNIQUE_ID(adr) FIELD_GET(SDW_UNIQUE_ID_MASK, addr) > > +#define SDW_MFG_ID(adr) FIELD_GET(SDW_MFG_ID_MASK, addr) > > +#define SDW_PART_ID(adr) FIELD_GET(SDW_PART_ID_MASK, addr) > > +#define SDW_CLASS_ID(adr) FIELD_GET(SDW_CLASS_ID_MASK, addr) > > ...our CI stopped on a compilation error with these macros. You will need > the patch1 attached. Aha, that looks wrong indeed, somehow my test & build for aarch64 and build for x86 didnt flag this, neither this kbuild-bot! Have fixed it up now > > Patch 9 also introduces conflicts with the multi-link code (fix in patch2), > so would you mind if we go first with the multi-link code, or defer patch9 > for now? We can fix the series required while applying.. > > Our validation for CML w/ RT700 is at: > https://github.com/thesofproject/linux/pull/2404 > > We will also test on machines that are not in the CI farm and provide > feedback. > > Thanks > -Pierre > > >From 3aba5a7229c904664dacf1843f2e925585d4bd3e Mon Sep 17 00:00:00 2001 > From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > Date: Fri, 28 Aug 2020 10:45:22 -0500 > Subject: [PATCH 1/2] fixup! soundwire: define and use addr bit masks > > s/addr/adr > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > --- > include/linux/soundwire/sdw.h | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h > index 892bf4718bc3..ebfabab63ec9 100644 > --- a/include/linux/soundwire/sdw.h > +++ b/include/linux/soundwire/sdw.h > @@ -472,12 +472,12 @@ struct sdw_slave_id { > #define SDW_PART_ID_MASK GENMASK(23, 8) > #define SDW_CLASS_ID_MASK GENMASK(7, 0) > > -#define SDW_DISCO_LINK_ID(adr) FIELD_GET(SDW_DISCO_LINK_ID_MASK, addr) > -#define SDW_VERSION(adr) FIELD_GET(SDW_VERSION_MASK, addr) > -#define SDW_UNIQUE_ID(adr) FIELD_GET(SDW_UNIQUE_ID_MASK, addr) > -#define SDW_MFG_ID(adr) FIELD_GET(SDW_MFG_ID_MASK, addr) > -#define SDW_PART_ID(adr) FIELD_GET(SDW_PART_ID_MASK, addr) > -#define SDW_CLASS_ID(adr) FIELD_GET(SDW_CLASS_ID_MASK, addr) > +#define SDW_DISCO_LINK_ID(adr) FIELD_GET(SDW_DISCO_LINK_ID_MASK, adr) > +#define SDW_VERSION(adr) FIELD_GET(SDW_VERSION_MASK, adr) > +#define SDW_UNIQUE_ID(adr) FIELD_GET(SDW_UNIQUE_ID_MASK, adr) > +#define SDW_MFG_ID(adr) FIELD_GET(SDW_MFG_ID_MASK, adr) > +#define SDW_PART_ID(adr) FIELD_GET(SDW_PART_ID_MASK, adr) > +#define SDW_CLASS_ID(adr) FIELD_GET(SDW_CLASS_ID_MASK, adr) > > /** > * struct sdw_slave_intr_status - Slave interrupt status > -- > 2.25.1 > > >From f0280ed5dbe284df628e58c5afa1e61452cd5cb8 Mon Sep 17 00:00:00 2001 > From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > Date: Fri, 28 Aug 2020 10:51:52 -0500 > Subject: [PATCH 2/2] soundwire: intel: use FIELD_PREP macro > > Follow upstream changes. > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > --- > drivers/soundwire/intel.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c > index 566c7a99a5c1..20f111ce8a7a 100644 > --- a/drivers/soundwire/intel.c > +++ b/drivers/soundwire/intel.c > @@ -381,10 +381,11 @@ static int intel_link_power_up(struct sdw_intel *sdw) > link_control = intel_readl(shim, SDW_SHIM_LCTL); > > /* only power-up enabled links */ > - spa_mask = sdw->link_res->link_mask << > - SDW_REG_SHIFT(SDW_SHIM_LCTL_SPA_MASK); > - cpa_mask = sdw->link_res->link_mask << > - SDW_REG_SHIFT(SDW_SHIM_LCTL_CPA_MASK); > + spa_mask = FIELD_PREP(SDW_SHIM_LCTL_SPA_MASK, > + sdw->link_res->link_mask); > + cpa_mask = FIELD_PREP(SDW_SHIM_LCTL_CPA_MASK, > + sdw->link_res->link_mask); > + > > link_control |= spa_mask; > > @@ -555,10 +556,11 @@ static int intel_link_power_down(struct sdw_intel *sdw) > link_control = intel_readl(shim, SDW_SHIM_LCTL); > > /* only power-down enabled links */ > - spa_mask = (~sdw->link_res->link_mask) << > - SDW_REG_SHIFT(SDW_SHIM_LCTL_SPA_MASK); > - cpa_mask = sdw->link_res->link_mask << > - SDW_REG_SHIFT(SDW_SHIM_LCTL_CPA_MASK); > + spa_mask = FIELD_PREP(SDW_SHIM_LCTL_SPA_MASK, > + ~sdw->link_res->link_mask); > + > + cpa_mask = FIELD_PREP(SDW_SHIM_LCTL_CPA_MASK, > + sdw->link_res->link_mask); > > link_control &= spa_mask; > > -- > 2.25.1 > -- ~Vinod
