Messages in this thread | | | Subject | Re: [PATCH 1/1] extcon: deduplicate code in extcon_set_state_sync() | From | Chanwoo Choi <> | Date | Thu, 16 Dec 2021 12:05:18 +0900 |
| |
On 11/23/21 11:53 PM, Alexander Stein wrote: > Finding the cable index and checking for changed status is also done > in extcon_set_state(). So calling extcon_set_state_sync() will do these > checks twice. Remove them and use these checks from extcon_set_state(). > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > I noticed this duplicated code while debugging an extcon related issue. > I do not know if it is allowed in the kernel to assume some behavior in > EXPORT_SYMBOL* functions, but as the two functions mentioned are in the > same source file it should be ok. > In the case it is not okay to assume some behaviour, > extcon_set_state_sync() is missing a check for !edev, like > extcon_set_state() does. > > drivers/extcon/extcon.c | 14 +------------- > 1 file changed, 1 insertion(+), 13 deletions(-) > > diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c > index e7a9561a826d..a09e704fd0fa 100644 > --- a/drivers/extcon/extcon.c > +++ b/drivers/extcon/extcon.c > @@ -576,19 +576,7 @@ EXPORT_SYMBOL_GPL(extcon_set_state); > */ > int extcon_set_state_sync(struct extcon_dev *edev, unsigned int id, bool state) > { > - int ret, index; > - unsigned long flags; > - > - index = find_cable_index_by_id(edev, id); > - if (index < 0) > - return index; > - > - /* Check whether the external connector's state is changed. */ > - spin_lock_irqsave(&edev->lock, flags); > - ret = is_extcon_changed(edev, index, state); > - spin_unlock_irqrestore(&edev->lock, flags); > - if (!ret) > - return 0; > + int ret; > > ret = extcon_set_state(edev, id, state); > if (ret < 0) >
Applied it. Thanks.
-- Best Regards, Chanwoo Choi Samsung Electronics
| |