From: Marek Szyprowski <email@example.com> To: Sebastian Reichel <firstname.lastname@example.org> Cc: Rob Herring <email@example.com>, Greg Kroah-Hartman <firstname.lastname@example.org>, "Rafael J . Wysocki" <email@example.com>, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, 'Linux Samsung SOC' <firstname.lastname@example.org> Subject: Re: [PATCHv1 00/19] Improve SBS battery support Date: Tue, 2 Jun 2020 09:17:09 +0200 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <firstname.lastname@example.org> Hi Sebastian, On 01.06.2020 19:05, Sebastian Reichel wrote: > On Mon, Jun 01, 2020 at 12:40:27PM +0200, Marek Szyprowski wrote: >> On 13.05.2020 20:55, Sebastian Reichel wrote: >>> This patchset improves support for SBS compliant batteries. Due to >>> the changes, the battery now exposes 32 power supply properties and >>> (un)plugging it generates a backtrace containing the following message >>> without the first patch in this series: >>> >>> --------------------------- >>> WARNING: CPU: 0 PID: 20 at lib/kobject_uevent.c:659 add_uevent_var+0xd4/0x104 >>> add_uevent_var: too many keys >>> --------------------------- >>> >>> For references this is what an SBS battery status looks like after >>> the patch series has been applied: >>> >>> cat /sys/class/power_supply/sbs-0-000b/uevent >>> POWER_SUPPLY_NAME=sbs-0-000b >>> POWER_SUPPLY_TYPE=Battery >>> POWER_SUPPLY_STATUS=Discharging >>> POWER_SUPPLY_CAPACITY_LEVEL=Normal >>> POWER_SUPPLY_HEALTH=Good >>> POWER_SUPPLY_PRESENT=1 >>> POWER_SUPPLY_TECHNOLOGY=Li-ion >>> POWER_SUPPLY_CYCLE_COUNT=12 >>> POWER_SUPPLY_VOLTAGE_NOW=11441000 >>> POWER_SUPPLY_CURRENT_NOW=-26000 >>> POWER_SUPPLY_CURRENT_AVG=-24000 >>> POWER_SUPPLY_CAPACITY=76 >>> POWER_SUPPLY_CAPACITY_ERROR_MARGIN=1 >>> POWER_SUPPLY_TEMP=198 >>> POWER_SUPPLY_TIME_TO_EMPTY_AVG=438600 >>> POWER_SUPPLY_TIME_TO_FULL_AVG=3932100 >>> POWER_SUPPLY_SERIAL_NUMBER=0000 >>> POWER_SUPPLY_VOLTAGE_MIN_DESIGN=10800000 >>> POWER_SUPPLY_VOLTAGE_MAX_DESIGN=10800000 >>> POWER_SUPPLY_ENERGY_NOW=31090000 >>> POWER_SUPPLY_ENERGY_FULL=42450000 >>> POWER_SUPPLY_ENERGY_FULL_DESIGN=41040000 >>> POWER_SUPPLY_CHARGE_NOW=2924000 >>> POWER_SUPPLY_CHARGE_FULL=3898000 >>> POWER_SUPPLY_CHARGE_FULL_DESIGN=3800000 >>> POWER_SUPPLY_CONSTANT_CHARGE_CURRENT_MAX=3000000 >>> POWER_SUPPLY_CONSTANT_CHARGE_VOLTAGE_MAX=12300000 >>> POWER_SUPPLY_MANUFACTURE_YEAR=2017 >>> POWER_SUPPLY_MANUFACTURE_MONTH=7 >>> POWER_SUPPLY_MANUFACTURE_DAY=3 >>> POWER_SUPPLY_MANUFACTURER=UR18650A >>> POWER_SUPPLY_MODEL_NAME=GEHC >> This patch landed in linux-next dated 20200529. Sadly it causes a >> regression on Samsung Exynos-based Chromebooks (Exynos5250 Snow, >> Exynos5420 Peach-Pi and Exynos5800 Peach-Pit). System boots to >> userspace, but then, when udev populates /dev, booting hangs: >> >> [ 4.435167] VFS: Mounted root (ext4 filesystem) readonly on device >> 179:51. >> [ 4.457477] devtmpfs: mounted >> [ 4.460235] Freeing unused kernel memory: 1024K >> [ 4.464022] Run /sbin/init as init process >> INIT: version 2.88 booting >> [info] Using makefile-style concurrent boot in runlevel S. >> [ 5.102096] random: crng init done >> [....] Starting the hotplug events dispatcher: systemd-udevdstarting >> version 236 >> [ ok . >> [....] Synthesizing the initial hotplug events...[ ok done. >> [....] Waiting for /dev to be fully populated...[ 34.409914] >> TPS65090_RAILSDCDC1: disabling >> [ 34.412977] TPS65090_RAILSDCDC2: disabling >> [ 34.417021] TPS65090_RAILSDCDC3: disabling >> [ 34.423848] TPS65090_RAILSLDO1: disabling >> [ 34.429068] TPS65090_RAILSLDO2: disabling > :( > > log does not look useful either. > >> Bisect between v5.7-rc1 and next-20200529 pointed me to the first bad >> commit: [c4b12a2f3f3de670f6be5e96092a2cab0b877f1a] power: supply: >> sbs-battery: simplify read_read_string_data. > ok. I tested this on an to-be-upstreamed i.MX6 based system > and arch/arm/boot/dts/imx53-ppd.dts. I think the difference > is, that i2c-exynos5 does not expose I2C_FUNC_SMBUS_READ_BLOCK_DATA. > I hoped all systems using SBS battery support this, but now > I see I2C_FUNC_SMBUS_EMUL only supports writing block data. > Looks like I need to add another patch implementing that > using the old code with added PEC support. > > In any case that should only return -ENODEV for the property > (and uevent), but not break boot. So something fishy is going > on. > >> However reverting it in linux-next doesn't fix the issue, so the >> next commits are also relevant to this issue. > The next patch, which adds PEC support depends on the simplification > of sbs_read_string_data. The old, open coded variant will result in > PEC failure for string properties (which should not stop boot either > of course). Can you try reverting both? Indeed, reverting both (and fixing the conflict) restores proper boot. > If that helps I will revert those two instead of dropping the whole > series for this merge window. > >> Let me know how can I help debugging it. > I suspect, that this is userspace endlessly retrying reading the > battery uevent when an error is returned. Could you check this? > Should be easy to see by adding some printfs. I've added some debug messages in sbs_get_property() and it read the same properties many times. However I've noticed that if I wait long enough booting finally continues. > That would mean a faulty battery could stall complete boot without > a useful error message, which is bad and needs to be fixed. > > Sorry for the inconvience and thanks for your report, No problem, finding regressions is one of the linux-next goal. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
next prev parent reply other threads:[~2020-06-02 7:17 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-13 18:55 Sebastian Reichel 2020-05-13 18:55 ` [PATCHv1 01/19] kobject: increase allowed number of uevent variables Sebastian Reichel 2020-05-14 6:11 ` Greg Kroah-Hartman 2020-05-15 14:45 ` Emil Velikov 2020-05-13 18:55 ` [PATCHv1 02/19] power: supply: core: add capacity error margin property Sebastian Reichel 2020-05-13 18:55 ` [PATCHv1 03/19] power: supply: core: add manufacture date properties Sebastian Reichel 2020-05-15 14:47 ` Emil Velikov 2020-05-15 15:14 ` Sebastian Reichel 2020-05-15 16:01 ` Emil Velikov 2020-05-13 18:56 ` [PATCHv1 04/19] power: supply: core: add POWER_SUPPLY_HEALTH_CALIBRATION_REQUIRED Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 05/19] power: supply: sbs-battery: Add TI BQ20Z65 support Sebastian Reichel 2020-05-28 2:37 ` Rob Herring 2020-05-13 18:56 ` [PATCHv1 06/19] power: supply: sbs-battery: add POWER_SUPPLY_PROP_CAPACITY_ERROR_MARGIN support Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 07/19] power: supply: sbs-battery: simplify read_read_string_data Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 08/19] power: supply: sbs-battery: add PEC support Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 09/19] power: supply: sbs-battery: add POWER_SUPPLY_PROP_CURRENT_AVG support Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 10/19] power: supply: sbs-battery: Improve POWER_SUPPLY_PROP_TECHNOLOGY support Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 11/19] power: supply: sbs-battery: add POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT/VOLTAGE_MAX support Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 12/19] power: supply: sbs-battery: add MANUFACTURE_DATE support Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 13/19] power: supply: sbs-battery: add POWER_SUPPLY_HEALTH_CALIBRATION_REQUIRED support Sebastian Reichel 2020-05-15 15:35 ` Emil Velikov 2020-05-13 18:56 ` [PATCHv1 14/19] power: supply: sbs-battery: fix idle battery status Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 15/19] power: supply: sbs-battery: add ability to disable charger broadcasts Sebastian Reichel 2020-05-15 14:57 ` Emil Velikov 2020-05-13 18:56 ` [PATCHv1 16/19] power: supply: sbs-battery: switch from of_property_* to device_property_* Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 17/19] power: supply: sbs-battery: switch to i2c's probe_new Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 18/19] power: supply: sbs-battery: constify power-supply property array Sebastian Reichel 2020-05-13 18:56 ` [PATCHv1 19/19] dt-bindings: power: sbs-battery: Convert to yaml Sebastian Reichel 2020-05-28 2:40 ` Rob Herring 2020-05-28 22:44 ` [PATCHv1 00/19] Improve SBS battery support Sebastian Reichel 2020-05-29 16:27 ` Pavel Machek 2020-06-01 21:57 ` Sebastian Reichel [not found] ` <CGME20200601104027eucas1p2b076ee860520d709e8178c41550653f7@eucas1p2.samsung.com> 2020-06-01 10:40 ` Marek Szyprowski 2020-06-01 17:05 ` Sebastian Reichel 2020-06-02 7:17 ` Marek Szyprowski [this message] 2020-06-02 18:01 ` Sebastian Reichel 2020-06-03 18:49 ` Marek Szyprowski
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCHv1 00/19] Improve SBS battery support' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.