All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Timon Baetz <timon.baetz@protonmail.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	MyungJoo Ham <myungjoo.ham@samsung.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Lee Jones <lee.jones@linaro.org>,
	Sebastian Reichel <sre@kernel.org>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, linux-pm@vger.kernel.org,
	~postmarketos/upstreaming@lists.sr.ht
Subject: Re: [PATCH v3 4/7] power: supply: max8997_charger: Set CHARGER current limit
Date: Wed, 23 Dec 2020 09:27:18 +0100	[thread overview]
Message-ID: <20201223082718.GA5472@kozik-lap> (raw)
In-Reply-To: <20201223090114.750664cd.timon.baetz@protonmail.com>

On Wed, Dec 23, 2020 at 08:09:55AM +0000, Timon Baetz wrote:
> On Tue, 22 Dec 2020 09:40:04 +0100, Krzysztof Kozlowski wrote:

(...)
> > >  	.name		= "max8997_pmic",
> > >  	.type		= POWER_SUPPLY_TYPE_BATTERY,
> > > @@ -170,6 +237,33 @@ static int max8997_battery_probe(struct platform_device *pdev)
> > >  		return PTR_ERR(charger->battery);
> > >  	}
> > >
> > > +	charger->reg = devm_regulator_get(&pdev->dev, "charger");  
> > 
> > The code looks good but isn't it breaking all existing platforms?
> 
> So there is 2 other DTS in the kernel sources that are using MAX8997
> pmic:
>  - Insignal Origen evaluation board
>  - Samsung Trats
> Non of them have charging regulators.

But still the power supply was probing on them (if not the error you
mentioned).  Now, the charger will fail.

> Also probing of the charger has been failing for long time because of 
> https://lore.kernel.org/lkml/20201109194251.562203-2-timon.baetz@protonmail.com/
> but that seems to land in 5.11.

That's a good argument supporting introduced breakage. Use it in commit
message. Don't hide such information.

> That being said, I guess I could make extcon and charger-supply
> optional if you prefer.

Since at least two boards will loose now power supply, I don't think you
have a choice.

Best regards,
Krzysztof


WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Timon Baetz <timon.baetz@protonmail.com>
Cc: devicetree@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Mark Brown <broonie@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Sebastian Reichel <sre@kernel.org>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	MyungJoo Ham <myungjoo.ham@samsung.com>,
	~postmarketos/upstreaming@lists.sr.ht,
	Lee Jones <lee.jones@linaro.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 4/7] power: supply: max8997_charger: Set CHARGER current limit
Date: Wed, 23 Dec 2020 09:27:18 +0100	[thread overview]
Message-ID: <20201223082718.GA5472@kozik-lap> (raw)
In-Reply-To: <20201223090114.750664cd.timon.baetz@protonmail.com>

On Wed, Dec 23, 2020 at 08:09:55AM +0000, Timon Baetz wrote:
> On Tue, 22 Dec 2020 09:40:04 +0100, Krzysztof Kozlowski wrote:

(...)
> > >  	.name		= "max8997_pmic",
> > >  	.type		= POWER_SUPPLY_TYPE_BATTERY,
> > > @@ -170,6 +237,33 @@ static int max8997_battery_probe(struct platform_device *pdev)
> > >  		return PTR_ERR(charger->battery);
> > >  	}
> > >
> > > +	charger->reg = devm_regulator_get(&pdev->dev, "charger");  
> > 
> > The code looks good but isn't it breaking all existing platforms?
> 
> So there is 2 other DTS in the kernel sources that are using MAX8997
> pmic:
>  - Insignal Origen evaluation board
>  - Samsung Trats
> Non of them have charging regulators.

But still the power supply was probing on them (if not the error you
mentioned).  Now, the charger will fail.

> Also probing of the charger has been failing for long time because of 
> https://lore.kernel.org/lkml/20201109194251.562203-2-timon.baetz@protonmail.com/
> but that seems to land in 5.11.

That's a good argument supporting introduced breakage. Use it in commit
message. Don't hide such information.

> That being said, I guess I could make extcon and charger-supply
> optional if you prefer.

Since at least two boards will loose now power supply, I don't think you
have a choice.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-12-23  8:28 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-22  7:31 [PATCH v3 1/7] extcon: max8997: Add CHGINS and CHGRM interrupt handling Timon Baetz
2020-12-22  7:31 ` Timon Baetz
2020-12-22  7:31 ` [PATCH v3 2/7] regulator: dt-bindings: Document max8997-pmic nodes Timon Baetz
2020-12-22  7:31   ` Timon Baetz
2020-12-22  8:37   ` Krzysztof Kozlowski
2020-12-22  8:37     ` Krzysztof Kozlowski
2020-12-22  7:31 ` [PATCH v3 3/7] mfd: max8997: Add of_compatible to extcon and charger mfd_cell Timon Baetz
2020-12-22  7:31   ` Timon Baetz
2020-12-22  8:37   ` Krzysztof Kozlowski
2020-12-22  8:37     ` Krzysztof Kozlowski
2020-12-22  9:55   ` Lee Jones
2020-12-22  9:55     ` Lee Jones
2021-02-11 21:11     ` Timon Baetz
2021-02-11 21:11       ` Timon Baetz
2021-02-12  9:37       ` Lee Jones
2021-02-12  9:37         ` Lee Jones
2020-12-22  7:31 ` [PATCH v3 4/7] power: supply: max8997_charger: Set CHARGER current limit Timon Baetz
2020-12-22  7:31   ` Timon Baetz
2020-12-22  8:40   ` Krzysztof Kozlowski
2020-12-22  8:40     ` Krzysztof Kozlowski
2020-12-23  8:09     ` Timon Baetz
2020-12-23  8:09       ` Timon Baetz
2020-12-23  8:27       ` Krzysztof Kozlowski [this message]
2020-12-23  8:27         ` Krzysztof Kozlowski
2020-12-22  7:31 ` [PATCH v3 5/7] ARM: dts: exynos: Added muic and charger nodes for i9100 Timon Baetz
2020-12-22  7:31   ` Timon Baetz
2020-12-22  7:31 ` [PATCH v3 6/7] ARM: dts: exynos: Fix charging regulator voltage and current " Timon Baetz
2020-12-22  7:31   ` Timon Baetz
2020-12-22  7:32 ` [PATCH v3 7/7] ARM: dts: exynos: Add top-off charging regulator node " Timon Baetz
2020-12-22  7:32   ` Timon Baetz

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 \
    --in-reply-to=20201223082718.GA5472@kozik-lap \
    --to=krzk@kernel.org \
    --cc=broonie@kernel.org \
    --cc=cw00.choi@samsung.com \
    --cc=devicetree@vger.kernel.org \
    --cc=lee.jones@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=myungjoo.ham@samsung.com \
    --cc=robh+dt@kernel.org \
    --cc=sre@kernel.org \
    --cc=timon.baetz@protonmail.com \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.