diff mbox series

[v1] ASoC: Intel: Skylake: Switch to modern UUID API

Message ID 20190619150213.87691-1-andriy.shevchenko@linux.intel.com (mailing list archive)
State Accepted
Commit 9e0784d00e35e058353e2e7e59dd956be7519788
Headers show
Series [v1] ASoC: Intel: Skylake: Switch to modern UUID API | expand

Commit Message

Andy Shevchenko June 19, 2019, 3:02 p.m. UTC
Switch the driver to use modern UUID API, i.e. guid_t type and
accompanying functions, such as guid_equal().

Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 sound/soc/intel/skylake/skl-pcm.c       | 12 ++++++------
 sound/soc/intel/skylake/skl-sst-dsp.h   |  6 +++---
 sound/soc/intel/skylake/skl-sst-utils.c | 23 +++++++----------------
 sound/soc/intel/skylake/skl-sst.c       |  4 +---
 sound/soc/intel/skylake/skl-topology.c  | 24 ++++++++++++------------
 sound/soc/intel/skylake/skl-topology.h  |  6 +++---
 6 files changed, 32 insertions(+), 43 deletions(-)

Comments

Pierre-Louis Bossart June 19, 2019, 3:46 p.m. UTC | #1
On 6/19/19 5:02 PM, Andy Shevchenko wrote:
> Switch the driver to use modern UUID API, i.e. guid_t type and
> accompanying functions, such as guid_equal().
> 
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Looks good to me - couple of nit-picks below on unrelated indentation 
changes.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> @@ -247,7 +241,6 @@ int snd_skl_parse_uuids(struct sst_dsp *ctx, const struct firmware *fw,
>   	struct adsp_fw_hdr *adsp_hdr;
>   	struct adsp_module_entry *mod_entry;
>   	int i, num_entry, size;
> -	uuid_le *uuid_bin;
>   	const char *buf;
>   	struct skl_sst *skl = ctx->thread_context;
>   	struct uuid_module *module;
> @@ -279,8 +272,7 @@ int snd_skl_parse_uuids(struct sst_dsp *ctx, const struct firmware *fw,
>   		return -EINVAL;
>   	}
>   
> -	mod_entry = (struct adsp_module_entry *)
> -		(buf + offset + adsp_hdr->len);
> +	mod_entry = (struct adsp_module_entry *)(buf + offset + adsp_hdr->len);

this really has nothing to do with the guid change, could be a separate 
patch in a perfect world.

> diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c
> index 5951bbdf1f1a..13c636dece56 100644
> --- a/sound/soc/intel/skylake/skl-sst.c
> +++ b/sound/soc/intel/skylake/skl-sst.c
> @@ -420,11 +420,9 @@ static int skl_load_module(struct sst_dsp *ctx, u16 mod_id, u8 *guid)
>   	struct skl_module_table *module_entry = NULL;
>   	int ret = 0;
>   	char mod_name[64]; /* guid str = 32 chars + 4 hyphens */
> -	uuid_le *uuid_mod;
>   
> -	uuid_mod = (uuid_le *)guid;
>   	snprintf(mod_name, sizeof(mod_name), "%s%pUL%s",
> -				"intel/dsp_fw_", uuid_mod, ".bin");
> +					     "intel/dsp_fw_", guid, ".bin");

indentation looks off, not sure if this is a diff effect.
Andy Shevchenko June 20, 2019, 3:09 p.m. UTC | #2
On Wed, Jun 19, 2019 at 05:46:34PM +0200, Pierre-Louis Bossart wrote:
> On 6/19/19 5:02 PM, Andy Shevchenko wrote:
> > Switch the driver to use modern UUID API, i.e. guid_t type and
> > accompanying functions, such as guid_equal().
> > 
> > Cc: Liam Girdwood <lgirdwood@gmail.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > Cc: Vinod Koul <vkoul@kernel.org>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Looks good to me - couple of nit-picks below on unrelated indentation
> changes.
> 
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Thanks for review!

> >   	char mod_name[64]; /* guid str = 32 chars + 4 hyphens */
> > -	uuid_le *uuid_mod;
> > -	uuid_mod = (uuid_le *)guid;
> >   	snprintf(mod_name, sizeof(mod_name), "%s%pUL%s",
> > -				"intel/dsp_fw_", uuid_mod, ".bin");
> > +					     "intel/dsp_fw_", guid, ".bin");
> 
> indentation looks off, not sure if this is a diff effect.

Ah, this can be modified to the below (by a separate patch, since Mark applied
this one already):

	snprintf(mod_name, sizeof(mod_name), "intel/dsp_fw_%pUL.bin", guid);

What do you think?

P.S. And it will take only one line.
Pierre-Louis Bossart June 21, 2019, 3:41 a.m. UTC | #3
>>>    	char mod_name[64]; /* guid str = 32 chars + 4 hyphens */
>>> -	uuid_le *uuid_mod;
>>> -	uuid_mod = (uuid_le *)guid;
>>>    	snprintf(mod_name, sizeof(mod_name), "%s%pUL%s",
>>> -				"intel/dsp_fw_", uuid_mod, ".bin");
>>> +					     "intel/dsp_fw_", guid, ".bin");
>>
>> indentation looks off, not sure if this is a diff effect.
> 
> Ah, this can be modified to the below (by a separate patch, since Mark applied
> this one already):
> 
> 	snprintf(mod_name, sizeof(mod_name), "intel/dsp_fw_%pUL.bin", guid);
> 
> What do you think?
> 
> P.S. And it will take only one line.

Sounds good to me. Not sure why it was written this way.
Amadeusz Sławiński June 24, 2019, 8:51 a.m. UTC | #4
On Wed, 19 Jun 2019 18:02:13 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Switch the driver to use modern UUID API, i.e. guid_t type and
> accompanying functions, such as guid_equal().
> 
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  sound/soc/intel/skylake/skl-pcm.c       | 12 ++++++------
>  sound/soc/intel/skylake/skl-sst-dsp.h   |  6 +++---
>  sound/soc/intel/skylake/skl-sst-utils.c | 23 +++++++----------------
>  sound/soc/intel/skylake/skl-sst.c       |  4 +---
>  sound/soc/intel/skylake/skl-topology.c  | 24 ++++++++++++------------
>  sound/soc/intel/skylake/skl-topology.h  |  6 +++---
>  6 files changed, 32 insertions(+), 43 deletions(-)
> 
> diff --git a/sound/soc/intel/skylake/skl-pcm.c
> b/sound/soc/intel/skylake/skl-pcm.c index 8b7232d3ffee..b2b9958605d1
> 100644 --- a/sound/soc/intel/skylake/skl-pcm.c
> +++ b/sound/soc/intel/skylake/skl-pcm.c
> @@ -1310,12 +1310,12 @@ static int skl_get_module_info(struct skl
> *skl, struct skl_module_cfg *mconfig) {
>  	struct skl_sst *ctx = skl->skl_sst;
>  	struct skl_module_inst_id *pin_id;
> -	uuid_le *uuid_mod, *uuid_tplg;
> +	guid_t *uuid_mod, *uuid_tplg;
>  	struct skl_module *skl_module;
>  	struct uuid_module *module;
>  	int i, ret = -EIO;
>  
> -	uuid_mod = (uuid_le *)mconfig->guid;
> +	uuid_mod = (guid_t *)mconfig->guid;
>  
>  	if (list_empty(&ctx->uuid_list)) {
>  		dev_err(ctx->dev, "Module list is empty\n");
> @@ -1323,7 +1323,7 @@ static int skl_get_module_info(struct skl *skl,
> struct skl_module_cfg *mconfig) }
>  
>  	list_for_each_entry(module, &ctx->uuid_list, list) {
> -		if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
> +		if (guid_equal(uuid_mod, &module->uuid)) {
>  			mconfig->id.module_id = module->id;
>  			if (mconfig->module)
>  				mconfig->module->loadable =
> module->is_loadable; @@ -1340,7 +1340,7 @@ static int
> skl_get_module_info(struct skl *skl, struct skl_module_cfg *mconfig)
> for (i = 0; i < skl->nr_modules; i++) { skl_module = skl->modules[i];
>  		uuid_tplg = &skl_module->uuid;
> -		if (!uuid_le_cmp(*uuid_mod, *uuid_tplg)) {
> +		if (guid_equal(uuid_mod, uuid_tplg)) {
>  			mconfig->module = skl_module;
>  			ret = 0;
>  			break;
> @@ -1352,13 +1352,13 @@ static int skl_get_module_info(struct skl
> *skl, struct skl_module_cfg *mconfig) list_for_each_entry(module,
> &ctx->uuid_list, list) { for (i = 0; i < MAX_IN_QUEUE; i++) {
>  			pin_id = &mconfig->m_in_pin[i].id;
> -			if (!uuid_le_cmp(pin_id->mod_uuid,
> module->uuid))
> +			if (guid_equal(&pin_id->mod_uuid,
> &module->uuid)) pin_id->module_id = module->id;
>  		}
>  
>  		for (i = 0; i < MAX_OUT_QUEUE; i++) {
>  			pin_id = &mconfig->m_out_pin[i].id;
> -			if (!uuid_le_cmp(pin_id->mod_uuid,
> module->uuid))
> +			if (guid_equal(&pin_id->mod_uuid,
> &module->uuid)) pin_id->module_id = module->id;
>  		}
>  	}
> diff --git a/sound/soc/intel/skylake/skl-sst-dsp.h
> b/sound/soc/intel/skylake/skl-sst-dsp.h index
> e1d6f6719f7e..cbc7a93d56c2 100644 ---
> a/sound/soc/intel/skylake/skl-sst-dsp.h +++
> b/sound/soc/intel/skylake/skl-sst-dsp.h @@ -177,7 +177,7 @@ struct
> skl_dsp_loader_ops { #define MAX_INSTANCE_BUFF 2
>  
>  struct uuid_module {
> -	uuid_le uuid;
> +	guid_t uuid;
>  	int id;
>  	int is_loadable;
>  	int max_instance;
> @@ -241,8 +241,8 @@ void bxt_sst_dsp_cleanup(struct device *dev,
> struct skl_sst *ctx); 
>  int snd_skl_parse_uuids(struct sst_dsp *ctx, const struct firmware
> *fw, unsigned int offset, int index);
> -int skl_get_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int
> instance_id); -int skl_put_pvt_id(struct skl_sst *ctx, uuid_le
> *uuid_mod, int *pvt_id); +int skl_get_pvt_id(struct skl_sst *ctx,
> guid_t *uuid_mod, int instance_id); +int skl_put_pvt_id(struct
> skl_sst *ctx, guid_t *uuid_mod, int *pvt_id); int
> skl_get_pvt_instance_id_map(struct skl_sst *ctx, int module_id, int
> instance_id); void skl_freeup_uuid_list(struct skl_sst *ctx);
> diff --git a/sound/soc/intel/skylake/skl-sst-utils.c
> b/sound/soc/intel/skylake/skl-sst-utils.c index
> 2ae405617876..85551321c35b 100644 ---
> a/sound/soc/intel/skylake/skl-sst-utils.c +++
> b/sound/soc/intel/skylake/skl-sst-utils.c @@ -21,17 +21,11 @@
>  #include "../common/sst-dsp-priv.h"
>  #include "skl-sst-ipc.h"
>  
> -
> -#define UUID_STR_SIZE 37
>  #define DEFAULT_HASH_SHA256_LEN 32
>  
>  /* FW Extended Manifest Header id = $AE1 */
>  #define SKL_EXT_MANIFEST_HEADER_MAGIC   0x31454124
>  
> -struct UUID {
> -	u8 id[16];
> -};
> -
>  union seg_flags {
>  	u32 ul;
>  	struct {
> @@ -65,7 +59,7 @@ struct module_type {
>  struct adsp_module_entry {
>  	u32 struct_id;
>  	u8  name[8];
> -	struct UUID uuid;
> +	u8  uuid[16];

guid_t uuid;

>  	struct module_type type;
>  	u8  hash1[DEFAULT_HASH_SHA256_LEN];
>  	u32 entry_point;
> @@ -184,13 +178,13 @@ static inline int skl_pvtid_128(struct
> uuid_module *module)
>   * This generates a 128 bit private unique id for a module TYPE so
> that
>   * module instance is unique
>   */
> -int skl_get_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int
> instance_id) +int skl_get_pvt_id(struct skl_sst *ctx, guid_t
> *uuid_mod, int instance_id) {
>  	struct uuid_module *module;
>  	int pvt_id;
>  
>  	list_for_each_entry(module, &ctx->uuid_list, list) {
> -		if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
> +		if (guid_equal(uuid_mod, &module->uuid)) {
>  
>  			pvt_id = skl_pvtid_128(module);
>  			if (pvt_id >= 0) {
> @@ -214,13 +208,13 @@ EXPORT_SYMBOL_GPL(skl_get_pvt_id);
>   *
>   * This frees a 128 bit private unique id previously generated
>   */
> -int skl_put_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int
> *pvt_id) +int skl_put_pvt_id(struct skl_sst *ctx, guid_t *uuid_mod,
> int *pvt_id) {
>  	int i;
>  	struct uuid_module *module;
>  
>  	list_for_each_entry(module, &ctx->uuid_list, list) {
> -		if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
> +		if (guid_equal(uuid_mod, &module->uuid)) {
>  
>  			if (*pvt_id != 0)
>  				i = (*pvt_id) / 64;
> @@ -247,7 +241,6 @@ int snd_skl_parse_uuids(struct sst_dsp *ctx,
> const struct firmware *fw, struct adsp_fw_hdr *adsp_hdr;
>  	struct adsp_module_entry *mod_entry;
>  	int i, num_entry, size;
> -	uuid_le *uuid_bin;
>  	const char *buf;
>  	struct skl_sst *skl = ctx->thread_context;
>  	struct uuid_module *module;
> @@ -279,8 +272,7 @@ int snd_skl_parse_uuids(struct sst_dsp *ctx,
> const struct firmware *fw, return -EINVAL;
>  	}
>  
> -	mod_entry = (struct adsp_module_entry *)
> -		(buf + offset + adsp_hdr->len);
> +	mod_entry = (struct adsp_module_entry *)(buf + offset +
> adsp_hdr->len); 
>  	num_entry = adsp_hdr->num_modules;
>  
> @@ -307,8 +299,7 @@ int snd_skl_parse_uuids(struct sst_dsp *ctx,
> const struct firmware *fw, goto free_uuid_list;
>  		}
>  
> -		uuid_bin = (uuid_le *)mod_entry->uuid.id;
> -		memcpy(&module->uuid, uuid_bin,
> sizeof(module->uuid));
> +		guid_copy(&module->uuid, (guid_t *)&mod_entry->uuid);
>  
>  		module->id = (i | (index << 12));
>  		module->is_loadable = mod_entry->type.load_type;
> diff --git a/sound/soc/intel/skylake/skl-sst.c
> b/sound/soc/intel/skylake/skl-sst.c index 5951bbdf1f1a..13c636dece56
> 100644 --- a/sound/soc/intel/skylake/skl-sst.c
> +++ b/sound/soc/intel/skylake/skl-sst.c
> @@ -420,11 +420,9 @@ static int skl_load_module(struct sst_dsp *ctx,
> u16 mod_id, u8 *guid) struct skl_module_table *module_entry = NULL;
>  	int ret = 0;
>  	char mod_name[64]; /* guid str = 32 chars + 4 hyphens */
> -	uuid_le *uuid_mod;
>  
> -	uuid_mod = (uuid_le *)guid;
>  	snprintf(mod_name, sizeof(mod_name), "%s%pUL%s",
> -				"intel/dsp_fw_", uuid_mod, ".bin");
> +					     "intel/dsp_fw_", guid,
> ".bin"); 
>  	module_entry = skl_module_get_from_id(ctx, mod_id);
>  	if (module_entry == NULL) {
> diff --git a/sound/soc/intel/skylake/skl-topology.c
> b/sound/soc/intel/skylake/skl-topology.c index
> c69d999d7bf1..9fd756bcc740 100644 ---
> a/sound/soc/intel/skylake/skl-topology.c +++
> b/sound/soc/intel/skylake/skl-topology.c @@ -580,7 +580,7 @@
> skl_tplg_init_pipe_modules(struct skl *skl, struct skl_pipe *pipe)
> int ret = 0; 
>  	list_for_each_entry(w_module, &pipe->w_list, node) {
> -		uuid_le *uuid_mod;
> +		guid_t *uuid_mod;
>  		w = w_module->w;
>  		mconfig = w->priv;
>  
> @@ -588,7 +588,7 @@ skl_tplg_init_pipe_modules(struct skl *skl,
> struct skl_pipe *pipe) if (mconfig->id.module_id < 0) {
>  			dev_err(skl->skl_sst->dev,
>  					"module %pUL id not
> populated\n",
> -					(uuid_le *)mconfig->guid);
> +					(guid_t *)mconfig->guid);
>  			return -EIO;
>  		}
>  
> @@ -622,7 +622,7 @@ skl_tplg_init_pipe_modules(struct skl *skl,
> struct skl_pipe *pipe)
>  		 * FE/BE params
>  		 */
>  		skl_tplg_update_module_params(w, ctx);
> -		uuid_mod = (uuid_le *)mconfig->guid;
> +		uuid_mod = (guid_t *)mconfig->guid;
>  		mconfig->id.pvt_id = skl_get_pvt_id(ctx, uuid_mod,
>  						mconfig->id.instance_id);
>  		if (mconfig->id.pvt_id < 0)
> @@ -661,9 +661,9 @@ static int skl_tplg_unload_pipe_modules(struct
> skl_sst *ctx, struct skl_module_cfg *mconfig = NULL;
>  
>  	list_for_each_entry(w_module, &pipe->w_list, node) {
> -		uuid_le *uuid_mod;
> +		guid_t *uuid_mod;
>  		mconfig  = w_module->w->priv;
> -		uuid_mod = (uuid_le *)mconfig->guid;
> +		uuid_mod = (guid_t *)mconfig->guid;
>  
>  		if (mconfig->module->loadable &&
> ctx->dsp->fw_ops.unload_mod && mconfig->m_state > SKL_MODULE_UNINIT) {
> @@ -918,12 +918,12 @@ static int
> skl_tplg_set_module_bind_params(struct snd_soc_dapm_widget *w, return
> 0; }
>  
> -static int skl_get_module_id(struct skl_sst *ctx, uuid_le *uuid)
> +static int skl_get_module_id(struct skl_sst *ctx, guid_t *uuid)
>  {
>  	struct uuid_module *module;
>  
>  	list_for_each_entry(module, &ctx->uuid_list, list) {
> -		if (uuid_le_cmp(*uuid, module->uuid) == 0)
> +		if (guid_equal(uuid, &module->uuid))
>  			return module->id;
>  	}
>  
> @@ -2121,11 +2121,11 @@ static int skl_tplg_add_pipe(struct device
> *dev, return 0;
>  }
>  
> -static int skl_tplg_get_uuid(struct device *dev, u8 *guid,
> +static int skl_tplg_get_uuid(struct device *dev, guid_t *guid,
>  	      struct snd_soc_tplg_vendor_uuid_elem *uuid_tkn)
>  {
>  	if (uuid_tkn->token == SKL_TKN_UUID) {
> -		memcpy(guid, &uuid_tkn->uuid, 16);
> +		guid_copy(guid, (guid_t *)&uuid_tkn->uuid);
>  		return 0;
>  	}
>  
> @@ -2151,7 +2151,7 @@ static int skl_tplg_fill_pin(struct device *dev,
>  		break;
>  
>  	case SKL_TKN_UUID:
> -		ret = skl_tplg_get_uuid(dev,
> m_pin[pin_index].id.mod_uuid.b,
> +		ret = skl_tplg_get_uuid(dev,
> &m_pin[pin_index].id.mod_uuid, (struct snd_soc_tplg_vendor_uuid_elem
> *)tkn_elem); if (ret < 0)
>  			return ret;
> @@ -2667,7 +2667,7 @@ static int skl_tplg_get_tokens(struct device
> *dev, 
>  		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
>  			if (is_module_guid) {
> -				ret = skl_tplg_get_uuid(dev,
> mconfig->guid,
> +				ret = skl_tplg_get_uuid(dev, (guid_t
> *)mconfig->guid, array->uuid);
>  				is_module_guid = false;
>  			} else {
> @@ -3486,7 +3486,7 @@ static int skl_tplg_get_manifest_uuid(struct
> device *dev, 
>  	if (uuid_tkn->token == SKL_TKN_UUID) {
>  		mod = skl->modules[ref_count];
> -		memcpy(&mod->uuid, &uuid_tkn->uuid,
> sizeof(uuid_tkn->uuid));
> +		guid_copy(&mod->uuid, (guid_t *)&uuid_tkn->uuid);
>  		ref_count++;
>  	} else {
>  		dev_err(dev, "Not an UUID token tkn %d\n",
> uuid_tkn->token); diff --git a/sound/soc/intel/skylake/skl-topology.h
> b/sound/soc/intel/skylake/skl-topology.h index
> b66e3a728853..5d2047114db0 100644 ---
> a/sound/soc/intel/skylake/skl-topology.h +++
> b/sound/soc/intel/skylake/skl-topology.h @@ -215,7 +215,7 @@ struct
> skl_mod_inst_map { struct skl_uuid_inst_map {
>  	u16 inst_id;
>  	u16 reserved;
> -	uuid_le mod_uuid;
> +	guid_t mod_uuid;
>  } __packed;
>  
>  struct skl_kpb_params {
> @@ -227,7 +227,7 @@ struct skl_kpb_params {
>  };
>  
>  struct skl_module_inst_id {
> -	uuid_le mod_uuid;
> +	guid_t mod_uuid;
>  	int module_id;
>  	u32 instance_id;
>  	int pvt_id;
> @@ -360,7 +360,7 @@ struct skl_module_res {
>  };
>  
>  struct skl_module {
> -	uuid_le uuid;
> +	guid_t uuid;
>  	u8 loadable;
>  	u8 input_pin_type;
>  	u8 output_pin_type;


I would also add:

struct skl_module_cfg {
-	u8 guid[16];
+	guid_t uuid;

should get rid of few casts above. Overall I would expect no casts at
all.
Andy Shevchenko June 25, 2019, 5:15 p.m. UTC | #5
On Mon, Jun 24, 2019 at 10:51:57AM +0200, Amadeusz Sławiński wrote:
> On Wed, 19 Jun 2019 18:02:13 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

Thanks for review, though this one will be part of upstream.

> > -struct UUID {
> > -	u8 id[16];
> > -};
> > -

> >  struct adsp_module_entry {
> >  	u32 struct_id;
> >  	u8  name[8];
> > -	struct UUID uuid;
> > +	u8  uuid[16];
> 
> guid_t uuid;

This seems a part of ABI.
ABI doesn't and shouldn't know anything about kernel internal types.

If I'm mistaken, one, who knows better the area, can submit a followup.

> >  struct skl_module_inst_id {
> > -	uuid_le mod_uuid;
> > +	guid_t mod_uuid;
> >  	int module_id;
> >  	u32 instance_id;
> >  	int pvt_id;
> > @@ -360,7 +360,7 @@ struct skl_module_res {
> >  };
> >  
> >  struct skl_module {
> > -	uuid_le uuid;
> > +	guid_t uuid;
> >  	u8 loadable;
> >  	u8 input_pin_type;
> >  	u8 output_pin_type;
> 
> 
> I would also add:
> 
> struct skl_module_cfg {
> -	u8 guid[16];
> +	guid_t uuid;
> 
> should get rid of few casts above. Overall I would expect no casts at
> all.

See above.

Nevertheless, there is an idea to add some wrappers to UUID framework to cover
cases when raw buffer is copied to UUID type or back and accompanying API.

At least btrfs expects such.

I will try to keep this in mind and convert ASoC stuff to the new API when it
appears in the upstream.
diff mbox series

Patch

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 8b7232d3ffee..b2b9958605d1 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1310,12 +1310,12 @@  static int skl_get_module_info(struct skl *skl, struct skl_module_cfg *mconfig)
 {
 	struct skl_sst *ctx = skl->skl_sst;
 	struct skl_module_inst_id *pin_id;
-	uuid_le *uuid_mod, *uuid_tplg;
+	guid_t *uuid_mod, *uuid_tplg;
 	struct skl_module *skl_module;
 	struct uuid_module *module;
 	int i, ret = -EIO;
 
-	uuid_mod = (uuid_le *)mconfig->guid;
+	uuid_mod = (guid_t *)mconfig->guid;
 
 	if (list_empty(&ctx->uuid_list)) {
 		dev_err(ctx->dev, "Module list is empty\n");
@@ -1323,7 +1323,7 @@  static int skl_get_module_info(struct skl *skl, struct skl_module_cfg *mconfig)
 	}
 
 	list_for_each_entry(module, &ctx->uuid_list, list) {
-		if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
+		if (guid_equal(uuid_mod, &module->uuid)) {
 			mconfig->id.module_id = module->id;
 			if (mconfig->module)
 				mconfig->module->loadable = module->is_loadable;
@@ -1340,7 +1340,7 @@  static int skl_get_module_info(struct skl *skl, struct skl_module_cfg *mconfig)
 	for (i = 0; i < skl->nr_modules; i++) {
 		skl_module = skl->modules[i];
 		uuid_tplg = &skl_module->uuid;
-		if (!uuid_le_cmp(*uuid_mod, *uuid_tplg)) {
+		if (guid_equal(uuid_mod, uuid_tplg)) {
 			mconfig->module = skl_module;
 			ret = 0;
 			break;
@@ -1352,13 +1352,13 @@  static int skl_get_module_info(struct skl *skl, struct skl_module_cfg *mconfig)
 	list_for_each_entry(module, &ctx->uuid_list, list) {
 		for (i = 0; i < MAX_IN_QUEUE; i++) {
 			pin_id = &mconfig->m_in_pin[i].id;
-			if (!uuid_le_cmp(pin_id->mod_uuid, module->uuid))
+			if (guid_equal(&pin_id->mod_uuid, &module->uuid))
 				pin_id->module_id = module->id;
 		}
 
 		for (i = 0; i < MAX_OUT_QUEUE; i++) {
 			pin_id = &mconfig->m_out_pin[i].id;
-			if (!uuid_le_cmp(pin_id->mod_uuid, module->uuid))
+			if (guid_equal(&pin_id->mod_uuid, &module->uuid))
 				pin_id->module_id = module->id;
 		}
 	}
diff --git a/sound/soc/intel/skylake/skl-sst-dsp.h b/sound/soc/intel/skylake/skl-sst-dsp.h
index e1d6f6719f7e..cbc7a93d56c2 100644
--- a/sound/soc/intel/skylake/skl-sst-dsp.h
+++ b/sound/soc/intel/skylake/skl-sst-dsp.h
@@ -177,7 +177,7 @@  struct skl_dsp_loader_ops {
 #define MAX_INSTANCE_BUFF 2
 
 struct uuid_module {
-	uuid_le uuid;
+	guid_t uuid;
 	int id;
 	int is_loadable;
 	int max_instance;
@@ -241,8 +241,8 @@  void bxt_sst_dsp_cleanup(struct device *dev, struct skl_sst *ctx);
 
 int snd_skl_parse_uuids(struct sst_dsp *ctx, const struct firmware *fw,
 				unsigned int offset, int index);
-int skl_get_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int instance_id);
-int skl_put_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int *pvt_id);
+int skl_get_pvt_id(struct skl_sst *ctx, guid_t *uuid_mod, int instance_id);
+int skl_put_pvt_id(struct skl_sst *ctx, guid_t *uuid_mod, int *pvt_id);
 int skl_get_pvt_instance_id_map(struct skl_sst *ctx,
 				int module_id, int instance_id);
 void skl_freeup_uuid_list(struct skl_sst *ctx);
diff --git a/sound/soc/intel/skylake/skl-sst-utils.c b/sound/soc/intel/skylake/skl-sst-utils.c
index 2ae405617876..85551321c35b 100644
--- a/sound/soc/intel/skylake/skl-sst-utils.c
+++ b/sound/soc/intel/skylake/skl-sst-utils.c
@@ -21,17 +21,11 @@ 
 #include "../common/sst-dsp-priv.h"
 #include "skl-sst-ipc.h"
 
-
-#define UUID_STR_SIZE 37
 #define DEFAULT_HASH_SHA256_LEN 32
 
 /* FW Extended Manifest Header id = $AE1 */
 #define SKL_EXT_MANIFEST_HEADER_MAGIC   0x31454124
 
-struct UUID {
-	u8 id[16];
-};
-
 union seg_flags {
 	u32 ul;
 	struct {
@@ -65,7 +59,7 @@  struct module_type {
 struct adsp_module_entry {
 	u32 struct_id;
 	u8  name[8];
-	struct UUID uuid;
+	u8  uuid[16];
 	struct module_type type;
 	u8  hash1[DEFAULT_HASH_SHA256_LEN];
 	u32 entry_point;
@@ -184,13 +178,13 @@  static inline int skl_pvtid_128(struct uuid_module *module)
  * This generates a 128 bit private unique id for a module TYPE so that
  * module instance is unique
  */
-int skl_get_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int instance_id)
+int skl_get_pvt_id(struct skl_sst *ctx, guid_t *uuid_mod, int instance_id)
 {
 	struct uuid_module *module;
 	int pvt_id;
 
 	list_for_each_entry(module, &ctx->uuid_list, list) {
-		if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
+		if (guid_equal(uuid_mod, &module->uuid)) {
 
 			pvt_id = skl_pvtid_128(module);
 			if (pvt_id >= 0) {
@@ -214,13 +208,13 @@  EXPORT_SYMBOL_GPL(skl_get_pvt_id);
  *
  * This frees a 128 bit private unique id previously generated
  */
-int skl_put_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int *pvt_id)
+int skl_put_pvt_id(struct skl_sst *ctx, guid_t *uuid_mod, int *pvt_id)
 {
 	int i;
 	struct uuid_module *module;
 
 	list_for_each_entry(module, &ctx->uuid_list, list) {
-		if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
+		if (guid_equal(uuid_mod, &module->uuid)) {
 
 			if (*pvt_id != 0)
 				i = (*pvt_id) / 64;
@@ -247,7 +241,6 @@  int snd_skl_parse_uuids(struct sst_dsp *ctx, const struct firmware *fw,
 	struct adsp_fw_hdr *adsp_hdr;
 	struct adsp_module_entry *mod_entry;
 	int i, num_entry, size;
-	uuid_le *uuid_bin;
 	const char *buf;
 	struct skl_sst *skl = ctx->thread_context;
 	struct uuid_module *module;
@@ -279,8 +272,7 @@  int snd_skl_parse_uuids(struct sst_dsp *ctx, const struct firmware *fw,
 		return -EINVAL;
 	}
 
-	mod_entry = (struct adsp_module_entry *)
-		(buf + offset + adsp_hdr->len);
+	mod_entry = (struct adsp_module_entry *)(buf + offset + adsp_hdr->len);
 
 	num_entry = adsp_hdr->num_modules;
 
@@ -307,8 +299,7 @@  int snd_skl_parse_uuids(struct sst_dsp *ctx, const struct firmware *fw,
 			goto free_uuid_list;
 		}
 
-		uuid_bin = (uuid_le *)mod_entry->uuid.id;
-		memcpy(&module->uuid, uuid_bin, sizeof(module->uuid));
+		guid_copy(&module->uuid, (guid_t *)&mod_entry->uuid);
 
 		module->id = (i | (index << 12));
 		module->is_loadable = mod_entry->type.load_type;
diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c
index 5951bbdf1f1a..13c636dece56 100644
--- a/sound/soc/intel/skylake/skl-sst.c
+++ b/sound/soc/intel/skylake/skl-sst.c
@@ -420,11 +420,9 @@  static int skl_load_module(struct sst_dsp *ctx, u16 mod_id, u8 *guid)
 	struct skl_module_table *module_entry = NULL;
 	int ret = 0;
 	char mod_name[64]; /* guid str = 32 chars + 4 hyphens */
-	uuid_le *uuid_mod;
 
-	uuid_mod = (uuid_le *)guid;
 	snprintf(mod_name, sizeof(mod_name), "%s%pUL%s",
-				"intel/dsp_fw_", uuid_mod, ".bin");
+					     "intel/dsp_fw_", guid, ".bin");
 
 	module_entry = skl_module_get_from_id(ctx, mod_id);
 	if (module_entry == NULL) {
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index c69d999d7bf1..9fd756bcc740 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -580,7 +580,7 @@  skl_tplg_init_pipe_modules(struct skl *skl, struct skl_pipe *pipe)
 	int ret = 0;
 
 	list_for_each_entry(w_module, &pipe->w_list, node) {
-		uuid_le *uuid_mod;
+		guid_t *uuid_mod;
 		w = w_module->w;
 		mconfig = w->priv;
 
@@ -588,7 +588,7 @@  skl_tplg_init_pipe_modules(struct skl *skl, struct skl_pipe *pipe)
 		if (mconfig->id.module_id < 0) {
 			dev_err(skl->skl_sst->dev,
 					"module %pUL id not populated\n",
-					(uuid_le *)mconfig->guid);
+					(guid_t *)mconfig->guid);
 			return -EIO;
 		}
 
@@ -622,7 +622,7 @@  skl_tplg_init_pipe_modules(struct skl *skl, struct skl_pipe *pipe)
 		 * FE/BE params
 		 */
 		skl_tplg_update_module_params(w, ctx);
-		uuid_mod = (uuid_le *)mconfig->guid;
+		uuid_mod = (guid_t *)mconfig->guid;
 		mconfig->id.pvt_id = skl_get_pvt_id(ctx, uuid_mod,
 						mconfig->id.instance_id);
 		if (mconfig->id.pvt_id < 0)
@@ -661,9 +661,9 @@  static int skl_tplg_unload_pipe_modules(struct skl_sst *ctx,
 	struct skl_module_cfg *mconfig = NULL;
 
 	list_for_each_entry(w_module, &pipe->w_list, node) {
-		uuid_le *uuid_mod;
+		guid_t *uuid_mod;
 		mconfig  = w_module->w->priv;
-		uuid_mod = (uuid_le *)mconfig->guid;
+		uuid_mod = (guid_t *)mconfig->guid;
 
 		if (mconfig->module->loadable && ctx->dsp->fw_ops.unload_mod &&
 			mconfig->m_state > SKL_MODULE_UNINIT) {
@@ -918,12 +918,12 @@  static int skl_tplg_set_module_bind_params(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
-static int skl_get_module_id(struct skl_sst *ctx, uuid_le *uuid)
+static int skl_get_module_id(struct skl_sst *ctx, guid_t *uuid)
 {
 	struct uuid_module *module;
 
 	list_for_each_entry(module, &ctx->uuid_list, list) {
-		if (uuid_le_cmp(*uuid, module->uuid) == 0)
+		if (guid_equal(uuid, &module->uuid))
 			return module->id;
 	}
 
@@ -2121,11 +2121,11 @@  static int skl_tplg_add_pipe(struct device *dev,
 	return 0;
 }
 
-static int skl_tplg_get_uuid(struct device *dev, u8 *guid,
+static int skl_tplg_get_uuid(struct device *dev, guid_t *guid,
 	      struct snd_soc_tplg_vendor_uuid_elem *uuid_tkn)
 {
 	if (uuid_tkn->token == SKL_TKN_UUID) {
-		memcpy(guid, &uuid_tkn->uuid, 16);
+		guid_copy(guid, (guid_t *)&uuid_tkn->uuid);
 		return 0;
 	}
 
@@ -2151,7 +2151,7 @@  static int skl_tplg_fill_pin(struct device *dev,
 		break;
 
 	case SKL_TKN_UUID:
-		ret = skl_tplg_get_uuid(dev, m_pin[pin_index].id.mod_uuid.b,
+		ret = skl_tplg_get_uuid(dev, &m_pin[pin_index].id.mod_uuid,
 			(struct snd_soc_tplg_vendor_uuid_elem *)tkn_elem);
 		if (ret < 0)
 			return ret;
@@ -2667,7 +2667,7 @@  static int skl_tplg_get_tokens(struct device *dev,
 
 		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
 			if (is_module_guid) {
-				ret = skl_tplg_get_uuid(dev, mconfig->guid,
+				ret = skl_tplg_get_uuid(dev, (guid_t *)mconfig->guid,
 							array->uuid);
 				is_module_guid = false;
 			} else {
@@ -3486,7 +3486,7 @@  static int skl_tplg_get_manifest_uuid(struct device *dev,
 
 	if (uuid_tkn->token == SKL_TKN_UUID) {
 		mod = skl->modules[ref_count];
-		memcpy(&mod->uuid, &uuid_tkn->uuid, sizeof(uuid_tkn->uuid));
+		guid_copy(&mod->uuid, (guid_t *)&uuid_tkn->uuid);
 		ref_count++;
 	} else {
 		dev_err(dev, "Not an UUID token tkn %d\n", uuid_tkn->token);
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h
index b66e3a728853..5d2047114db0 100644
--- a/sound/soc/intel/skylake/skl-topology.h
+++ b/sound/soc/intel/skylake/skl-topology.h
@@ -215,7 +215,7 @@  struct skl_mod_inst_map {
 struct skl_uuid_inst_map {
 	u16 inst_id;
 	u16 reserved;
-	uuid_le mod_uuid;
+	guid_t mod_uuid;
 } __packed;
 
 struct skl_kpb_params {
@@ -227,7 +227,7 @@  struct skl_kpb_params {
 };
 
 struct skl_module_inst_id {
-	uuid_le mod_uuid;
+	guid_t mod_uuid;
 	int module_id;
 	u32 instance_id;
 	int pvt_id;
@@ -360,7 +360,7 @@  struct skl_module_res {
 };
 
 struct skl_module {
-	uuid_le uuid;
+	guid_t uuid;
 	u8 loadable;
 	u8 input_pin_type;
 	u8 output_pin_type;