[PATCH 1/9] soundwire: define and use addr bit masks

[Vinod Koul]

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 at linux.intel.com>
> 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 at linux.intel.com>
> ---
>  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 at linux.intel.com>
> 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 at linux.intel.com>
> ---
>  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