Re: [PATCH v2 0/2] MUX: Add support for mux-states

[Peter Rosin <peda@axentia.se>]

Hi again.

On 2021-12-17 06:57, Peter Rosin wrote:
> Hi Aswath!
> 
> On 2021-12-02 13:40, Aswath Govindraju wrote:
>> The following series of patches add support for reading mux
>> state from the device tree.
>>
>> changes since v1:
>> - Made grammer corrections and added more information
>>   on the usage for mux-states and mux-controls
>>
>> Aswath Govindraju (2):
>>   dt-bindings: mux: Document mux-states property
>>   mux: Add support for reading mux state from consumer DT node
>>
>>  .../devicetree/bindings/mux/gpio-mux.yaml     |  11 +-
>>  .../devicetree/bindings/mux/mux-consumer.yaml |  21 ++
>>  .../bindings/mux/mux-controller.yaml          |  26 ++-
>>  drivers/mux/core.c                            | 219 ++++++++++++++++--
>>  include/linux/mux/consumer.h                  |  19 +-
>>  5 files changed, 271 insertions(+), 25 deletions(-)
>>
> 
> I finally found some time to have a last look at this (fingers crossed) and
> pushed it out for testing in linux-next.
> 
> I did end up squashing in these trivial changes in patch 2/2, I hope that's
> fine with you. Just holler if not...
> 
> Cheers,
> Peter
> 
> diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
> index 148e19381b79..5018403fe82f 100644
> --- a/Documentation/driver-api/driver-model/devres.rst
> +++ b/Documentation/driver-api/driver-model/devres.rst
> @@ -368,6 +368,7 @@ MUX
>    devm_mux_chip_alloc()
>    devm_mux_chip_register()
>    devm_mux_control_get()
> +  devm_mux_state_get()
>  
>  NET
>    devm_alloc_etherdev()
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index 7355c5ad41f7..bf448069ca85 100644
> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
> @@ -30,12 +30,13 @@
>  #define MUX_CACHE_UNKNOWN MUX_IDLE_AS_IS
>  
>  /**
> - * struct mux_state -	Represents a mux controller specific to a given device
> - * @mux:		Pointer to a mux controller
> - * @state		State of the mux to be set
> + * struct mux_state -	Represents a mux controller state specific to a given
> + *			consumer.
> + * @mux:		Pointer to a mux controller.
> + * @state		State of the mux to be selected.
>   *
> - * This structure is specific to a device that acquires it and has information
> - * specific to the device.
> + * This structure is specific to the consumer that acquires it and has
> + * information specific to that consumer.
>   */
>  struct mux_state {
>  	struct mux_control *mux;
> @@ -354,7 +355,8 @@ static void mux_control_delay(struct mux_control *mux, unsigned int delay_us)
>   * On successfully selecting the mux-control state, it will be locked until
>   * there is a call to mux_control_deselect(). If the mux-control is already
>   * selected when mux_control_select() is called, the caller will be blocked
> - * until mux_control_deselect() is called (by someone else).
> + * until mux_control_deselect() or mux_state_deselect() is called (by someone
> + * else).
>   *
>   * Therefore, make sure to call mux_control_deselect() when the operation is
>   * complete and the mux-control is free for others to use, but do not call
> @@ -384,16 +386,15 @@ int mux_control_select_delay(struct mux_control *mux, unsigned int state,
>  EXPORT_SYMBOL_GPL(mux_control_select_delay);
>  
>  /**
> - * mux_state_select_delay() - Select mux-state
> - * @mstate: The mux-state to select
> - * @delay_us: The time to delay (in microseconds) if the mux control
> - *            changes state on select
> + * mux_state_select_delay() - Select the given multiplexer state.
> + * @mstate: The mux-state to select.
> + * @delay_us: The time to delay (in microseconds) if the mux state is changed.
>   *
> - * On successfully selecting the mux-state, it will be locked until
> - * there is a call to mux_state_deselect() or mux_control_deselect().
> - * If the mux-control is already selected when mux_state_select() is
> - * called, the caller will be blocked until mux_state_deselect() or
> - * mux_control_deselect() is called (by someone else).
> + * On successfully selecting the mux-state, its mux-control will be locked
> + * until there is a call to mux_state_deselect(). If the mux-control is already
> + * selected when mux_state_select() is called, the caller will be blocked
> + * until mux_state_deselect() or mux_control_deselect() is called (by someone
> + * else).
>   *
>   * Therefore, make sure to call mux_state_deselect() when the operation is
>   * complete and the mux-control is free for others to use, but do not call
> @@ -415,7 +416,7 @@ EXPORT_SYMBOL_GPL(mux_state_select_delay);
>   * @delay_us: The time to delay (in microseconds) if the mux state is changed.
>   *
>   * On successfully selecting the mux-control state, it will be locked until
> - * mux_control_deselect() or mux_state_deselect() called.
> + * mux_control_deselect() called.
>   *
>   * Therefore, make sure to call mux_control_deselect() when the operation is
>   * complete and the mux-control is free for others to use, but do not call
> @@ -444,12 +445,12 @@ int mux_control_try_select_delay(struct mux_control *mux, unsigned int state,
>  EXPORT_SYMBOL_GPL(mux_control_try_select_delay);
>  
>  /**
> - * mux_state_try_select_delay() - Try to select the mux-state.
> - * @mstate: The mux-state to select
> + * mux_state_try_select_delay() - Try to select the given multiplexer state.
> + * @mstate: The mux-state to select.
>   * @delay_us: The time to delay (in microseconds) if the mux state is changed.
>   *
> - * On successfully selecting the mux-state, it will be locked until
> - * mux_state_deselect() or mux_control_deselect() is called.
> + * On successfully selecting the mux-state, its mux-control will be locked
> + * until mux_state_deselect() is called.
>   *
>   * Therefore, make sure to call mux_state_deselect() when the operation is
>   * complete and the mux-control is free for others to use, but do not call
> @@ -642,26 +643,6 @@ static void devm_mux_control_release(struct device *dev, void *res)
>  	mux_control_put(mux);
>  }
>  
> -/**
> - * mux_state_put() - Put away the mux-state for good.
> - * @mstate: The mux-state to put away.
> - *
> - * mux_control_put() reverses the effects of mux_control_get().
> - */
> -void mux_state_put(struct mux_state *mstate)
> -{
> -	mux_control_put(mstate->mux);
> -	kfree(mstate);
> -}
> -EXPORT_SYMBOL_GPL(mux_state_put);
> -
> -static void devm_mux_state_release(struct device *dev, void *res)
> -{
> -	struct mux_state *mstate = *(struct mux_state **)res;
> -
> -	mux_state_put(mstate);
> -}
> -
>  /**
>   * devm_mux_control_get() - Get the mux-control for a device, with resource
>   *			    management.
> @@ -692,6 +673,26 @@ struct mux_control *devm_mux_control_get(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(devm_mux_control_get);
>  
> +/**
> + * mux_state_put() - Put away the mux-state for good.
> + * @mstate: The mux-state to put away.
> + *
> + * mux_control_put() reverses the effects of mux_control_get().

And, a couple of minutes later, I squashed in this on top:

- * mux_control_put() reverses the effects of mux_control_get().
+ * mux_state_put() reverses the effects of mux_state_get().

Cheers,
Peter

> + */
> +void mux_state_put(struct mux_state *mstate)
> +{
> +	mux_control_put(mstate->mux);
> +	kfree(mstate);
> +}
> +EXPORT_SYMBOL_GPL(mux_state_put);
> +
> +static void devm_mux_state_release(struct device *dev, void *res)
> +{
> +	struct mux_state *mstate = *(struct mux_state **)res;
> +
> +	mux_state_put(mstate);
> +}
> +
>  /**
>   * devm_mux_state_get() - Get the mux-state for a device, with resource
>   *			  management.
> diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
> index bf5abf062c21..babf2a744056 100644
> --- a/include/linux/mux/consumer.h
> +++ b/include/linux/mux/consumer.h
> @@ -27,6 +27,7 @@ int __must_check mux_control_try_select_delay(struct mux_control *mux,
>  					      unsigned int delay_us);
>  int __must_check mux_state_try_select_delay(struct mux_state *mstate,
>  					    unsigned int delay_us);
> +
>  static inline int __must_check mux_control_select(struct mux_control *mux,
>  						  unsigned int state)
>  {
> @@ -37,6 +38,7 @@ static inline int __must_check mux_state_select(struct mux_state *mstate)
>  {
>  	return mux_state_select_delay(mstate, 0);
>  }
> +
>  static inline int __must_check mux_control_try_select(struct mux_control *mux,
>  						      unsigned int state)
>  {
>