| Message ID | 20201004224601.420786-1-sebastian.reichel@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | power: supply: sbs-battery: chromebook workaround for PEC | expand |
On Monday, October 5, 2020 12:46:01 AM CEST, Sebastian Reichel wrote: > Looks like the I2C tunnel implementation from Chromebook's > embedded controller does not handle PEC correctly. Fix this > by disabling PEC for batteries behind those I2C tunnels as > a workaround. > > Reported-by: "Milan P. Stanić" <mps@arvanta.net> > Reported-by: Vicente Bergas <vicencb@gmail.com> > CC: Enric Balletbo i Serra <enric.balletbo@collabora.com> > Fixes: 7222bd603dd2 ("power: supply: sbs-battery: add PEC support") > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > --- > Hi, > > This is compile-tested only, since I do not have a chromebook at > hand. Please test if this fixes your issue. Hi Sebastian, tested on rk3399-gru-kevin with 5.9.0-rc8 and now the CPU usage is 0 when idling. dmesg reports: [ 1.370249] sbs-battery 9-000b: Disabling PEC because of broken Cros-EC implementation So, Tested-by: Vicente Bergas <vicencb@gmail.com> Thanks, Vicente. > -- Sebastian > --- > drivers/power/supply/sbs-battery.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/power/supply/sbs-battery.c > b/drivers/power/supply/sbs-battery.c > index 13192cbcce71..b6a538ebb378 100644 > --- a/drivers/power/supply/sbs-battery.c > +++ b/drivers/power/supply/sbs-battery.c > @@ -279,6 +279,12 @@ static int sbs_update_presence(struct > sbs_info *chip, bool is_present) > else > client->flags &= ~I2C_CLIENT_PEC; > > + if (of_device_is_compatible(client->dev.parent->of_node, > "google,cros-ec-i2c-tunnel") > + && client->flags & I2C_CLIENT_PEC) { > + dev_info(&client->dev, "Disabling PEC because of broken > Cros-EC implementation\n"); > + client->flags &= ~I2C_CLIENT_PEC; > + } > + > dev_dbg(&client->dev, "PEC: %s\n", (client->flags & I2C_CLIENT_PEC) ? > "enabled" : "disabled"); >
Hi all, On Mon, 2020-10-05 at 19:53, Vicente Bergas wrote: > On Monday, October 5, 2020 12:46:01 AM CEST, Sebastian Reichel wrote: > > Looks like the I2C tunnel implementation from Chromebook's > > embedded controller does not handle PEC correctly. Fix this > > by disabling PEC for batteries behind those I2C tunnels as > > a workaround. > > > > Reported-by: "Milan P. Stanić" <mps@arvanta.net> > > Reported-by: Vicente Bergas <vicencb@gmail.com> > > CC: Enric Balletbo i Serra <enric.balletbo@collabora.com> > > Fixes: 7222bd603dd2 ("power: supply: sbs-battery: add PEC support") > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > > --- > > Hi, > > > > This is compile-tested only, since I do not have a chromebook at > > hand. Please test if this fixes your issue. > > Hi Sebastian, > tested on rk3399-gru-kevin with 5.9.0-rc8 and now the CPU usage is 0 when > idling. > dmesg reports: > [ 1.370249] sbs-battery 9-000b: Disabling PEC because of broken Cros-EC > implementation Also I tested on same board and same kernel version and can confirm this. > So, > Tested-by: Vicente Bergas <vicencb@gmail.com> > > Thanks, > Vicente. > > > -- Sebastian > > --- > > drivers/power/supply/sbs-battery.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/power/supply/sbs-battery.c > > b/drivers/power/supply/sbs-battery.c > > index 13192cbcce71..b6a538ebb378 100644 > > --- a/drivers/power/supply/sbs-battery.c > > +++ b/drivers/power/supply/sbs-battery.c > > @@ -279,6 +279,12 @@ static int sbs_update_presence(struct sbs_info > > *chip, bool is_present) > > else > > client->flags &= ~I2C_CLIENT_PEC; > > + if (of_device_is_compatible(client->dev.parent->of_node, > > "google,cros-ec-i2c-tunnel") > > + && client->flags & I2C_CLIENT_PEC) { > > + dev_info(&client->dev, "Disabling PEC because of broken Cros-EC > > implementation\n"); > > + client->flags &= ~I2C_CLIENT_PEC; > > + } > > + > > dev_dbg(&client->dev, "PEC: %s\n", (client->flags & I2C_CLIENT_PEC) ? > > "enabled" : "disabled"); >
On Mon, 2020-10-05 at 20:48, Milan P. Stanić wrote: > Hi all, > > On Mon, 2020-10-05 at 19:53, Vicente Bergas wrote: > > On Monday, October 5, 2020 12:46:01 AM CEST, Sebastian Reichel wrote: > > > Looks like the I2C tunnel implementation from Chromebook's > > > embedded controller does not handle PEC correctly. Fix this > > > by disabling PEC for batteries behind those I2C tunnels as > > > a workaround. > > > > > > Reported-by: "Milan P. Stanić" <mps@arvanta.net> > > > Reported-by: Vicente Bergas <vicencb@gmail.com> > > > CC: Enric Balletbo i Serra <enric.balletbo@collabora.com> > > > Fixes: 7222bd603dd2 ("power: supply: sbs-battery: add PEC support") > > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > > > --- > > > Hi, > > > > > > This is compile-tested only, since I do not have a chromebook at > > > hand. Please test if this fixes your issue. > > > > Hi Sebastian, > > tested on rk3399-gru-kevin with 5.9.0-rc8 and now the CPU usage is 0 when > > idling. > > dmesg reports: > > [ 1.370249] sbs-battery 9-000b: Disabling PEC because of broken Cros-EC > > implementation > > Also I tested on same board and same kernel version and can confirm > this. And I forgot to mention that I opened bug report. sorry. https://bugzilla.kernel.org/show_bug.cgi?id=209409 > > So, > > Tested-by: Vicente Bergas <vicencb@gmail.com> > > > > Thanks, > > Vicente. > > > > > -- Sebastian > > > --- > > > drivers/power/supply/sbs-battery.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/power/supply/sbs-battery.c > > > b/drivers/power/supply/sbs-battery.c > > > index 13192cbcce71..b6a538ebb378 100644 > > > --- a/drivers/power/supply/sbs-battery.c > > > +++ b/drivers/power/supply/sbs-battery.c > > > @@ -279,6 +279,12 @@ static int sbs_update_presence(struct sbs_info > > > *chip, bool is_present) > > > else > > > client->flags &= ~I2C_CLIENT_PEC; > > > + if (of_device_is_compatible(client->dev.parent->of_node, > > > "google,cros-ec-i2c-tunnel") > > > + && client->flags & I2C_CLIENT_PEC) { > > > + dev_info(&client->dev, "Disabling PEC because of broken Cros-EC > > > implementation\n"); > > > + client->flags &= ~I2C_CLIENT_PEC; > > > + } > > > + > > > dev_dbg(&client->dev, "PEC: %s\n", (client->flags & I2C_CLIENT_PEC) ? > > > "enabled" : "disabled"); > >
Hi, On Mon, 2020-10-05 at 00:46, Sebastian Reichel wrote: > Looks like the I2C tunnel implementation from Chromebook's > embedded controller does not handle PEC correctly. Fix this > by disabling PEC for batteries behind those I2C tunnels as > a workaround. > > Reported-by: "Milan P. Stanić" <mps@arvanta.net> > Reported-by: Vicente Bergas <vicencb@gmail.com> > CC: Enric Balletbo i Serra <enric.balletbo@collabora.com> > Fixes: 7222bd603dd2 ("power: supply: sbs-battery: add PEC support") > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > --- > Hi, > > This is compile-tested only, since I do not have a chromebook at > hand. Please test if this fixes your issue. > > -- Sebastian > --- > drivers/power/supply/sbs-battery.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c > index 13192cbcce71..b6a538ebb378 100644 > --- a/drivers/power/supply/sbs-battery.c > +++ b/drivers/power/supply/sbs-battery.c > @@ -279,6 +279,12 @@ static int sbs_update_presence(struct sbs_info *chip, bool is_present) > else > client->flags &= ~I2C_CLIENT_PEC; > > + if (of_device_is_compatible(client->dev.parent->of_node, "google,cros-ec-i2c-tunnel") > + && client->flags & I2C_CLIENT_PEC) { > + dev_info(&client->dev, "Disabling PEC because of broken Cros-EC implementation\n"); > + client->flags &= ~I2C_CLIENT_PEC; > + } > + > dev_dbg(&client->dev, "PEC: %s\n", (client->flags & I2C_CLIENT_PEC) ? > "enabled" : "disabled"); Just for info, yesterday I built kernel 5.9-rc8 from: https://gitlab.collabora.com/eballetbo/linux/-/commits/topic/chromeos/somewhat-stable-next for Acer R13 chromebook (Mediatek mt8173, Machine model: Google Elm) without above patch, and on it sbs-battery works without any problem. Maybe problem is only on rk3399 (just guessing) -- regards
Hi, On Thu, Oct 08, 2020 at 09:07:01PM +0200, Milan P. Stanić wrote: > On Mon, 2020-10-05 at 00:46, Sebastian Reichel wrote: > > Looks like the I2C tunnel implementation from Chromebook's > > embedded controller does not handle PEC correctly. Fix this > > by disabling PEC for batteries behind those I2C tunnels as > > a workaround. > > > > Reported-by: "Milan P. Stanić" <mps@arvanta.net> > > Reported-by: Vicente Bergas <vicencb@gmail.com> > > CC: Enric Balletbo i Serra <enric.balletbo@collabora.com> > > Fixes: 7222bd603dd2 ("power: supply: sbs-battery: add PEC support") > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > > --- > > Hi, > > > > This is compile-tested only, since I do not have a chromebook at > > hand. Please test if this fixes your issue. > > > > -- Sebastian > > --- > > drivers/power/supply/sbs-battery.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c > > index 13192cbcce71..b6a538ebb378 100644 > > --- a/drivers/power/supply/sbs-battery.c > > +++ b/drivers/power/supply/sbs-battery.c > > @@ -279,6 +279,12 @@ static int sbs_update_presence(struct sbs_info *chip, bool is_present) > > else > > client->flags &= ~I2C_CLIENT_PEC; > > > > + if (of_device_is_compatible(client->dev.parent->of_node, "google,cros-ec-i2c-tunnel") > > + && client->flags & I2C_CLIENT_PEC) { > > + dev_info(&client->dev, "Disabling PEC because of broken Cros-EC implementation\n"); > > + client->flags &= ~I2C_CLIENT_PEC; > > + } > > + > > dev_dbg(&client->dev, "PEC: %s\n", (client->flags & I2C_CLIENT_PEC) ? > > "enabled" : "disabled"); > > Just for info, yesterday I built kernel 5.9-rc8 from: > https://gitlab.collabora.com/eballetbo/linux/-/commits/topic/chromeos/somewhat-stable-next > for Acer R13 chromebook (Mediatek mt8173, Machine model: Google Elm) > without above patch, and on it sbs-battery works without any problem. Thanks for the info. I updated the commit message stating that not all Chromebooks are affected. > Maybe problem is only on rk3399 (just guessing) Maybe. I did not take a close look how the cros-ec-i2c-tunnel is implemented in hardware. It might also be completly unrelated to the cros-ec-i2c-tunnel and a defect battery controller instead. For now I sent above fix to Linus, so that the final 5.9 release works on all Chromebooks. It's better to do further investigation with a workaround in place :) -- Sebastian
diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c index 13192cbcce71..b6a538ebb378 100644 --- a/drivers/power/supply/sbs-battery.c +++ b/drivers/power/supply/sbs-battery.c @@ -279,6 +279,12 @@ static int sbs_update_presence(struct sbs_info *chip, bool is_present) else client->flags &= ~I2C_CLIENT_PEC; + if (of_device_is_compatible(client->dev.parent->of_node, "google,cros-ec-i2c-tunnel") + && client->flags & I2C_CLIENT_PEC) { + dev_info(&client->dev, "Disabling PEC because of broken Cros-EC implementation\n"); + client->flags &= ~I2C_CLIENT_PEC; + } + dev_dbg(&client->dev, "PEC: %s\n", (client->flags & I2C_CLIENT_PEC) ? "enabled" : "disabled");
Looks like the I2C tunnel implementation from Chromebook's embedded controller does not handle PEC correctly. Fix this by disabling PEC for batteries behind those I2C tunnels as a workaround. Reported-by: "Milan P. Stanić" <mps@arvanta.net> Reported-by: Vicente Bergas <vicencb@gmail.com> CC: Enric Balletbo i Serra <enric.balletbo@collabora.com> Fixes: 7222bd603dd2 ("power: supply: sbs-battery: add PEC support") Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> --- Hi, This is compile-tested only, since I do not have a chromebook at hand. Please test if this fixes your issue. -- Sebastian --- drivers/power/supply/sbs-battery.c | 6 ++++++ 1 file changed, 6 insertions(+)