From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 856A7C3DA7F for ; Sun, 4 Aug 2024 11:00:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=D73264s0rVgukFWFnLR+vhWZQ06smgTwC5awVs888Oo=; b=p7fcMX9s0sp61c1jaD4Xc6XiqW tJHisoPl/6oyT5r4LcIPRW7bIdNy5SHfMFRmh8noYBas63CVL6lf1/A5/fIPhA2FWI41SRMGP0D3r j29oEIQrpGJDBt1xxCZ93epAs91lwSc8RsOcj7io5Aa+BNAOHCXRjZvfWOW6hvbZrXQWAoKiNDs3I DZRVqsotVFmJAr9cDZ28dxnxOfJ3hGXYFkjln350iwCZ1YG7MzZm3+JUrpLenWtlGY7alZbB7UTxV 3VnYNMwHY/ulztWZrRq3LlEEqk/WRCGYf5obPs+29mpP3fbBE5lPkBBZWRElwzDECuSk6/li0kPfZ V6azAScg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1saYyK-0000000D7UJ-2KjE; Sun, 04 Aug 2024 11:00:20 +0000 Received: from out-170.mta0.migadu.com ([91.218.175.170]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1saYxn-0000000D7Pp-2ttf for linux-arm-kernel@lists.infradead.org; Sun, 04 Aug 2024 10:59:50 +0000 Date: Sun, 4 Aug 2024 12:59:37 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=grimler.se; s=key1; t=1722769181; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=D73264s0rVgukFWFnLR+vhWZQ06smgTwC5awVs888Oo=; b=cbzRaOqpNdt0nkCPe1REyNFoxFAt3LxF1sfg91oYwLswUpazCyM3ajF/GDqm52fr6A49ut S2Z0/jlSgpicapb27uC9ISakign8gj+erkvowgPE/jVRKjQsRIEmVHz8vk1amegcFFTZv4 gBZpGNFFqaJgL28qqk5Tm1WWbX6b0xw= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Henrik Grimler To: Artur Weber Cc: Krzysztof Kozlowski , Chanwoo Choi , Sebastian Reichel , Rob Herring , Conor Dooley , Lee Jones , Krzysztof Kozlowski , Alim Akhtar , linux-pm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, ~postmarketos/upstreaming@lists.sr.ht, Wolfgang Wiedmeyer , Denis 'GNUtoo' Carikli , Krzysztof Kozlowski Subject: Re: [PATCH v3 00/10] power: supply: max77693: Toggle charging/OTG based on extcon status Message-ID: <20240804105937.GA20421@l14.localdomain> References: <20240729-max77693-charger-extcon-v3-0-02315a6869d4@gmail.com> <20240801062253.GA2681@l14.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240801062253.GA2681@l14.localdomain> X-Migadu-Flow: FLOW_OUT X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240804_035948_510168_03E0D94C X-CRM114-Status: GOOD ( 46.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi again Artur, On Thu, Aug 01, 2024 at 08:23:26AM +0200, Henrik Grimler wrote: > Hi Artur, > > On Mon, Jul 29, 2024 at 07:47:34PM +0200, Artur Weber wrote: > > This patchset does the following: > > > > - Add CURRENT_MAX and INPUT_CURRENT_MAX power supply properties to > > expose the "fast charge current" (maximum current from charger to > > battery) and "CHGIN input current limit" (maximum current from > > external supply to charger). > > > > - Add functions for toggling charging and OTG modes. > > > > - Add an extcon-based handler that enables charging or OTG depending > > on the cable type plugged in. The extcon device to use for cable > > detection can be specified in the device tree, and is entirely > > optional. > > > > The extcon listener implementation is inspired by the rt5033 charger > > driver (commit 8242336dc8a8 ("power: supply: rt5033_charger: Add cable > > detection and USB OTG supply")). > > Tested on exynos4412-i9305 (after applying the changes in patch 8 - 10 > to exynos4412-midas.dtsi). It works well, device correctly identifies > a usb cable connected to charger or a usb cable connected to computer, > and sets a limit of 1.8 A and 0.5 A in the two cases. > > I did notice that device does not always detect cable insertion, so I > can occassionally get two de-attach events in a row. Cable was > inserted between 428 and 462 in below log snippet: > > [ 389.458399] max77693-muic max77693-muic: external connector is attached(chg_type:0x3, prev_chg_type:0x3) > [ 389.469765] max77693-charger max77693-charger: fast charging. connector type: 6 > [ 428.151857] max77693-muic max77693-muic: external connector is detached(chg_type:0x3, prev_chg_type:0x0) > [ 428.160319] max77693-charger max77693-charger: not charging. connector type: 13 > [ 462.156048] max77693-muic max77693-muic: external connector is detached(chg_type:0x0, prev_chg_type:0x0) > [ 469.881925] max77693-muic max77693-muic: external connector is attached(chg_type:0x3, prev_chg_type:0x3) > [ 469.890049] max77693-charger max77693-charger: fast charging. connector type: 6 > > but this is probably an issue in extcon driver though rather than > charger. > > I have not tested so that MHL still works, as I do not have access to > that cable at the moment, will try it in a few days. MHL now tested on exynos4412-i9300 as well. It works, and the series fixes so that we can hotplug the cable (with a few patches to make sii9324 use extcon as well), before we had to connect cable before boot and rely on bootloader to setup everything. Thanks! > > Signed-off-by: Artur Weber Tested-by: Henrik Grimler Best regards, Henrik Grimler > > v3 no longer uses the CHARGER regulator to manage the power status, and > > that's for two reasons: > > > > - Regulator enable/disable behavior was interfering with how the power > > supply driver worked (we occasionally got "unbalanced disables" > > errors when switching charging state, despite checking for the > > regulator status with regulator_is_enabled() - the CHARGER reg would > > report as enabled despite the enable count being 0). > > This broke OTG insertion if the OTG cable was plugged in first, and > > sometimes caused warnings on unsuspend. > > > > - Changing the charging values directly in the power supply driver is > > less opaque and lets us avoid bringing in a dependency on regulators. > > > > It also splits the current limits back into two properties: > > INPUT_CURRENT_LIMIT and CONSTANT_CHARGE_CURRENT_MAX. Again, there are > > two reasons for this split: > > > > - They are two separate current controls, one for USB->charger and one > > for charger->battery, and they have different limits (0-2.1A for CC > > vs 60mA-2.58A for input). Given that the power supply core has the > > properties for both values separately, it's more logical to present > > them as such. > > > > - It's safer to keep these separate; CONSTANT_CHARGE_CURRENT_MAX is > > pretty explicitly only set *once* - at probe time with a safe value > > specified in the DT. This way, INPUT_CURRENT_LIMIT is safer to modify > > since in the event of an invalid value the CC current will hold back > > the extra current thus preventing damage to the battery. > > > > The latter is relevant as I'm working on a follow-up patchset that > > allows for controlling the charging parameters using power supply > > properties/sysfs properties rather than the CHARGER regulator. > > > > Note that the CHARGER regulator gets disabled automatically if it's > > not used, which will disable charging if it was auto-enabled by the > > extcon code. This can be worked around by re-attaching the cable, or > > more properly by removing the CHARGER regulator from DT for devices > > that use the extcon-based charger management, as has been done in the > > Galaxy Tab 3 8.0 DTSI. > > > > See v1 for old description: > > > > https://lore.kernel.org/r/20240530-max77693-charger-extcon-v1-0-dc2a9e5bdf30@gmail.com > > --- > > Changes in v3: > > - Drop uses of CHARGER regulator, manage registers directly in power > > supply driver instead > > - Link to v2: https://lore.kernel.org/r/20240715-max77693-charger-extcon-v2-0-0838ffbb18c3@gmail.com > > > > Changes in v2: > > - Changed to use monitored-battery for charge current value > > - Both current limit variables are now set by the CHARGER regulator > > - Link to v1: https://lore.kernel.org/r/20240530-max77693-charger-extcon-v1-0-dc2a9e5bdf30@gmail.com > > > > --- > > Artur Weber (10): > > dt-bindings: power: supply: max77693: Add monitored-battery property > > dt-bindings: power: supply: max77693: Add maxim,usb-connector property > > power: supply: max77693: Expose input current limit and CC current properties > > power: supply: max77693: Set charge current limits during init > > power: supply: max77693: Add USB extcon detection for enabling charging > > power: supply: max77693: Add support for detecting and enabling OTG > > power: supply: max77693: Set up charge/input current according to cable type > > ARM: dts: samsung: exynos4212-tab3: Add battery node with charge current value > > ARM: dts: samsung: exynos4212-tab3: Add USB connector node > > ARM: dts: exynos4212-tab3: Drop CHARGER regulator > > > > .../bindings/power/supply/maxim,max77693.yaml | 15 + > > arch/arm/boot/dts/samsung/exynos4212-tab3.dtsi | 22 +- > > drivers/power/supply/Kconfig | 1 + > > drivers/power/supply/max77693_charger.c | 302 ++++++++++++++++++++- > > include/linux/mfd/max77693-private.h | 12 + > > 5 files changed, 337 insertions(+), 15 deletions(-) > > --- > > base-commit: 0c3836482481200ead7b416ca80c68a29cfdaabd > > change-id: 20240525-max77693-charger-extcon-9ebb7bad83ce > > > > Best regards, > > -- > > Artur Weber > >