Re: [PATCH v2 12/19] ASoC: SDCA: Add UMP buffer helper functions

[Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>]

> +int sdca_ump_get_owner_host(struct device *dev,
> +			    struct regmap *function_regmap,
> +			    struct sdca_function_data *function,
> +			    struct sdca_entity *entity,
> +			    struct sdca_control *control);
> +int sdca_ump_set_owner_device(struct device *dev,
> +			      struct regmap *function_regmap,
> +			      struct sdca_function_data *function,
> +			      struct sdca_entity *entity,
> +			      struct sdca_control *control);
> +int sdca_ump_read_message(struct device *dev,
> +			  struct regmap *device_regmap,
> +			  struct regmap *function_regmap,
> +			  struct sdca_function_data *function,
> +			  struct sdca_entity *entity,
> +			  unsigned int offset_sel, unsigned int length_sel,
> +			  void **msg);
> +int sdca_ump_write_message(struct device *dev,
> +			   struct regmap *device_regmap,
> +			   struct regmap *function_regmap,
> +			   struct sdca_function_data *function,
> +			   struct sdca_entity *entity,
> +			   unsigned int offset_sel, unsigned int msg_offset,
> +			   unsigned int length_sel,
> +			   void *msg, int msg_len);

I am still missing the big picture on when/where the timeouts read from DisCo properties are used.
I looked at patch 14 and I don't see any timeouts used except for the reset.

> +/**
> + * sdca_ump_read_message - read a UMP message from the device
> + * @dev: Pointer to the struct device used for error messages.
> + * @device_regmap: Pointer to the Device register map.
> + * @function_regmap: Pointer to the regmap for the SDCA Function.
> + * @function: Pointer to the Function information.
> + * @entity: Pointer to the SDCA Entity.
> + * @offset_sel: Control Selector for the UMP Offset Control.
> + * @length_sel: Control Selector for the UMP Length Control.
> + * @msg: Pointer that will be populated with an dynamically buffer
> + * containing the UMP message. Note this needs to be freed by the
> + * caller.
> + *
> + * The caller should first call sdca_ump_get_owner_host() to ensure the host
> + * currently owns the UMP buffer, and then this function can be used to
> + * retrieve a message. It is the callers responsibility to free the
> + * message once it is finished with it. Finally sdca_ump_set_owner_device()
> + * should be called to return the buffer to the device.

Maybe I misunderstood something in the UMP spec, but I could see a different flow where the host does a set_owner_device() and later reads the buffer updated by the device once the ownership is given back to the host.

I think you're assuming that a read is always initiated by the device, but what if the host wants to read something (volume/acoustic levels, etc) without getting any notification? 

IOW, will a read only happen when the device throws an interrupt on its own? The spec doesn't seem very clear on this...