From:   Armin Wolf <W_Armin@gmx.de>
To:     rafael@kernel.org, lenb@kernel.org
Cc:     linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 4/4] ACPI: SBS: Fix handling of Smart Battery Selectors
Date:   Sat, 25 Feb 2023 09:04:58 +0100
Message-Id: <20230225080458.1342359-5-W_Armin@gmx.de>
In-Reply-To: <20230225080458.1342359-1-W_Armin@gmx.de>
References: <20230225080458.1342359-1-W_Armin@gmx.de>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:YkRaH1XJPhs=;4hmhe8YZBtQQPoxtSkBWjWo3gb+
 4kHbhaTEwcR4ZA8DeiCZnZb5nzpAM4uMPrrEN0+4kxKXzA4Z8K/AHKRmP406obwwew7GUPOsA
 WIifAuWREaZsEr4F0lWWMAtIm6D/Hxm0ta6wacRWeofnLfuOqPwtppyVMKAuhX82QyzY2q5cO
 uRWSAjJFFHiPJNCrE5oYjtPfZkm5ieKo0cNQrmwzRetT4V5i+cWMDgoI9697KGtmIGU/Mu+9h
 8wZsmz/e/uMmnxzaol18mynTh1XtV7gPaDxzK+KH4VNbrJUOF9K7GQFhq/qNVi4oBk4zqzTf8
 E79KPqizKKKb9dJNOaMA4gCErFgsSFKhbCzrwq+QDJi143i+jVdQycxVx45djmtB50Vo81sIU
 S+jPcfn6iViiyD2ZQ5iobu7J4JFzC3VeIj4fHr40NdmxmvlCl3AmTme2+1Q6APtOocenaK+gR
 9x8nTqpnd+E1eQNnaPIjtgEh8VYc0Ev4i2aKazy/PlRfUGYaiu8ci0LZfh9ghTPfqLnRh0cO0
 jQJ2oLXamKDRNdq+G2vPxAF9cLQM3UvCYVklac5l4ARYtSwL6NhIjv8mO3vGOTPlQnuW2voR/
 sEVewAOMqsRRPgp1m7R6cHv1AdCbGzPYOtVQemq8nkwlSiF37Qfq+H7U0e7GbnPTJ/1sTQn4Y
 zLIr5RLB0EXMP9qSmchtFqJOpdkkJ2HkhDm0VcNAbeeYcTeCaCum/P4nQyxHGNxIeLLLru/7G
 0DewxKlAjP9XscQ9ZftRa6dsc7kJeRW06dBCroH6Fb68BB0jjTcrRJfpXs+DapOU4iOoWPlCY
 JYQh4+t05CqH83CK2KIObGgXRON/ZqhdVPHwdJyukKcgiGXO5RJp4yzVd0dDxXVbc/lF3QcS4
 OaP+uQEegNhU46fEoU+nu8i4bDL7SRY1l8vRrTlIXgCoZJT9QTDuRDfbm3GFgAY6EgoNakzmL
 LcYgGNLeaHZA2Xj8FV5S6oFpwss=
Precedence: bulk

The "Smart Battery Selector" standard says that when writing
SelectorState (0x1), the nibbles which should not be modified
need to be masked with 0xff. This is necessary since in contrast
to a "Smart Battery Manager", the last three nibbles are writable.

Failing to do so might trigger the following cycle:
1. Host accidentally changes power source of the system (3rd nibble)
   when selecting a battery.
2. Power source is invalid, Selector changes to another power source.
3. Selector notifies host that it changed the power source.
4. Host re-reads some batteries.
5. goto 1 for each re-read battery.

This loop might also be entered when a battery which is not present
is selected for SMBus access. In the end some workqueues fill up,
which causes the system to lockup upon suspend/shutdown.

Fix this by correctly masking the value to be written, and avoid
selecting batteries which are absent.

Tested on a Acer Travelmate 4002WLMi.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
=2D--
 drivers/acpi/sbs.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
index e90752d4f488..94e3c000df2e 100644
=2D-- a/drivers/acpi/sbs.c
+++ b/drivers/acpi/sbs.c
@@ -473,23 +473,32 @@ static const struct device_attribute alarm_attr =3D =
{
    ----------------------------------------------------------------------=
---- */
 static int acpi_battery_read(struct acpi_battery *battery)
 {
-	int result =3D 0, saved_present =3D battery->present;
+	int result, saved_present =3D battery->present;
 	u16 state;

 	if (battery->sbs->manager_present) {
 		result =3D acpi_smbus_read(battery->sbs->hc, SMBUS_READ_WORD,
 				ACPI_SBS_MANAGER, 0x01, (u8 *)&state);
-		if (!result)
-			battery->present =3D state & (1 << battery->id);
-		state &=3D 0x0fff;
+		if (result)
+			return result;
+
+		battery->present =3D state & (1 << battery->id);
+		if (!battery->present)
+			return 0;
+
+		/* Masking necessary for Smart Battery Selectors */
+		state =3D 0x0fff;
 		state |=3D 1 << (battery->id + 12);
 		acpi_smbus_write(battery->sbs->hc, SMBUS_WRITE_WORD,
 				  ACPI_SBS_MANAGER, 0x01, (u8 *)&state, 2);
-	} else if (battery->id =3D=3D 0)
-		battery->present =3D 1;
-
-	if (result || !battery->present)
-		return result;
+	} else {
+		if (battery->id =3D=3D 0) {
+			battery->present =3D 1;
+		} else {
+			if (!battery->present)
+				return 0;
+		}
+	}

 	if (saved_present !=3D battery->present) {
 		battery->update_time =3D 0;
=2D-
2.30.2