Hi Aswath!
See below.
On 2021-12-17 07:26, Peter Rosin wrote:
> 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().
>
And now I notice that there is no mux_state_get. There should be one,
in my opinion, at least if there is an exported mux_state_put.
So, Aswath, can you please test the patch that's coming as a reply
to this message on top of what is currently on its way into linux-next?
I.e. wait for the next linux-next to be released, or
https://gitlab.com/peda-linux/mux.git#for-next
Or, optionally, just grab the mux-state-get branch:
https://gitlab.com/peda-linux/mux.git#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)
>> {
>>
