From:   Alexander Stein <alexander.stein@ew.tq-group.com>
To:     MyungJoo Ham <myungjoo.ham@samsung.com>,
        Chanwoo Choi <cw00.choi@samsung.com>
Cc:     Alexander Stein <alexander.stein@ew.tq-group.com>,
        linux-kernel@vger.kernel.org
Subject: [PATCH 1/1] extcon: deduplicate code in extcon_set_state_sync()
Date:   Tue, 23 Nov 2021 15:53:01 +0100
Message-Id: <20211123145301.778629-1-alexander.stein@ew.tq-group.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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)
-- 
2.25.1