Hi Sebastian, On 02.06.2020 20:01, Sebastian Reichel wrote: > On Tue, Jun 02, 2020 at 09:17:09AM +0200, Marek Szyprowski wrote: >> 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. > Ok, I pushed out a revert of those two patches. They should land in > tomorrows linux-next release. Please test it. Today's linux-next (20200603) boots fine on the Samsung Exynos-based Chromebooks. Let me know how if you need any help debugging the issues to resurrect those patches. >>> 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. > So basically userspace slows down itself massively by trying to > re-read uevent over and over when an error occurs. Does not seem > like a sensible thing to do. I will have a look at this when I find > some time. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
