Messages in this thread | | | Date | Fri, 7 Jan 2022 08:10:34 +0100 | Subject | Re: [PATCH 4/6] mux: add missing mux_state_get | From | Peter Rosin <> |
| |
Hi!
On 2022-01-06 15:41, Greg Kroah-Hartman wrote: > On Mon, Jan 03, 2022 at 06:26:21PM +0100, Peter Rosin wrote: >> Hi! >> >> On 2022-01-03 13:42, Greg Kroah-Hartman wrote: >>> On Sun, Jan 02, 2022 at 11:38:36PM +0100, Peter Rosin wrote: >>>> From: Peter Rosin <peda@axentia.se> >>>> >>>> And implement devm_mux_state_get in terms of the new function. >>>> >>>> Tested-by: Aswath Govindraju <a-govindraju@ti.com> >>>> Signed-off-by: Peter Rosin <peda@axentia.se> >>>> --- >>>> drivers/mux/core.c | 41 ++++++++++++++++++++++++++---------- >>>> include/linux/mux/consumer.h | 1 + >>>> 2 files changed, 31 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/drivers/mux/core.c b/drivers/mux/core.c >>>> index 7d38e7c0c02e..90073ce01539 100644 >>>> --- a/drivers/mux/core.c >>>> +++ b/drivers/mux/core.c >>>> @@ -673,6 +673,33 @@ struct mux_control *devm_mux_control_get(struct device *dev, >>>> } >>>> EXPORT_SYMBOL_GPL(devm_mux_control_get); >>>> >>>> +/** >>>> + * mux_state_get() - Get the mux-state for a device. >>>> + * @dev: The device that needs a mux-state. >>>> + * @mux_name: The name identifying the mux-state. >>>> + * >>>> + * Return: A pointer to the mux-state, or an ERR_PTR with a negative errno. >>>> + */ >>>> +struct mux_state *mux_state_get(struct device *dev, const char *mux_name) >>>> +{ >>>> + struct mux_state *mstate; >>>> + >>>> + mstate = kzalloc(sizeof(*mstate), GFP_KERNEL); >>>> + if (!mstate) >>>> + return ERR_PTR(-ENOMEM); >>>> + >>>> + mstate->mux = mux_get(dev, mux_name, &mstate->state); >>> >>> will this build? I haven't applied it but mux_get() in my tree right >>> now is defined as: >>> static inline void mux_get(struct gsm_mux *gsm) >> >> Yes it builds. As mentioned in the cover letter, the patches have been >> in -next for a couple of weeks. The static definition you are pointing >> at is from n_gsm.c (which does not seem to be #included by any other >> file). This definition of mux_get is again static and in a .c file >> (which is not #included by anything). Surely not a conflict? > > If it's static, no, it's fine, but I don't see it in this commit either? > > I'm confused now,
Apparently :-)
The static drivers/mux/core.c:mux_get() is not in your tree because it was introduced in patch 3/6. That patch refactored the existing mux_control_get() into a new static helper function mux_get() with two wrappers -- the old mux_control_get() that preserves the preexisting interface and the new devm_mux_state_get(). mux_control_get() was always in turn wrapped by devm_mux_control_get(), while patch 3/6 failed to add a similar double wrapping with an intermediate mux_state_get(). Instead it wrapped mux_get() directly.
I didn't notice that mux_state_get() was missing until after a couple of rounds of review with Aswath, and didn't want go for another round when it was me who had made a mistake, and instead just fixed it with a commit of my own.
Maybe you thought "the new function" that this commit message speaks about was mux_get() (which was new in 3/6, but no longer "new" here in 4/6), when in fact it refers to mux_state_get()?
> can you resend the remaining changes and I will review > them again?
On it.
Cheers, Peter
| |