[PATCH 0/3] charger-supply for max8997_charger

[PATCH 0/3] charger-supply for max8997_charger

[Timon Baetz]

Based on the discussion from [0] add an optional DT property to retrieve
the regulator used for charging control in the max8997_charger driver.

[0] https://lore.kernel.org/lkml/20210118124505.GG4455@sirena.org.uk/


Timon Baetz (3):
  regulator: dt-bindings: Document charger-supply for max8997
  ARM: dts: exynos: Add charger supply for I9100
  power: supply: max8997_charger: Switch to new binding

 .../bindings/regulator/max8997-regulator.txt         |  1 +
 arch/arm/boot/dts/exynos4210-i9100.dts               |  2 ++
 drivers/power/supply/max8997_charger.c               | 12 ++++++++----
 3 files changed, 11 insertions(+), 4 deletions(-)

-- 
2.25.1



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

[PATCH 1/3] regulator: dt-bindings: Document charger-supply for max8997

[Timon Baetz]

Add charger-supply optional property to enable charging control.

Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
---
 .../devicetree/bindings/regulator/max8997-regulator.txt          | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/regulator/max8997-regulator.txt b/Documentation/devicetree/bindings/regulator/max8997-regulator.txt
index 6fe825b8ac1b..b53c5e2b335f 100644
--- a/Documentation/devicetree/bindings/regulator/max8997-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/max8997-regulator.txt
@@ -35,6 +35,7 @@ Optional properties:
 - interrupts: Interrupt specifiers for two interrupt sources.
   - First interrupt specifier is for 'irq1' interrupt.
   - Second interrupt specifier is for 'alert' interrupt.
+- charger-supply: regulator node for charging current.
 - max8997,pmic-buck1-uses-gpio-dvs: 'buck1' can be controlled by gpio dvs.
 - max8997,pmic-buck2-uses-gpio-dvs: 'buck2' can be controlled by gpio dvs.
 - max8997,pmic-buck5-uses-gpio-dvs: 'buck5' can be controlled by gpio dvs.
-- 
2.25.1



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

[PATCH 2/3] ARM: dts: exynos: Add charger supply for I9100

[Timon Baetz]

The regulator is used for charging control by max8997_charger driver.

Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
---
 arch/arm/boot/dts/exynos4210-i9100.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
index 304a8ee2364c..dad950daafb4 100644
--- a/arch/arm/boot/dts/exynos4210-i9100.dts
+++ b/arch/arm/boot/dts/exynos4210-i9100.dts
@@ -384,6 +384,8 @@ pmic@66 {
 		pinctrl-0 = <&max8997_irq>, <&otg_gp>, <&usb_sel>;
 		pinctrl-names = "default";
 
+		charger-supply = <&charger_reg>;
+
 		regulators {
 			vadc_reg: LDO1 {
 				regulator-name = "VADC_3.3V_C210";
-- 
2.25.1



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

[PATCH 3/3] power: supply: max8997_charger: Switch to new binding

[Timon Baetz]

Get regulator from parent device's node and extcon by name.

Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
---
 drivers/power/supply/max8997_charger.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/power/supply/max8997_charger.c b/drivers/power/supply/max8997_charger.c
index 321bd6b8ee41..625d8cc4312a 100644
--- a/drivers/power/supply/max8997_charger.c
+++ b/drivers/power/supply/max8997_charger.c
@@ -168,6 +168,7 @@ static int max8997_battery_probe(struct platform_device *pdev)
 	int ret = 0;
 	struct charger_data *charger;
 	struct max8997_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+	struct device_node *np = pdev->dev.of_node;
 	struct i2c_client *i2c = iodev->i2c;
 	struct max8997_platform_data *pdata = iodev->pdata;
 	struct power_supply_config psy_cfg = {};
@@ -237,20 +238,23 @@ static int max8997_battery_probe(struct platform_device *pdev)
 		return PTR_ERR(charger->battery);
 	}
 
+	// grab regulator from parent device's node
+	pdev->dev.of_node = iodev->dev->of_node;
 	charger->reg = devm_regulator_get_optional(&pdev->dev, "charger");
+	pdev->dev.of_node = np;
 	if (IS_ERR(charger->reg)) {
 		if (PTR_ERR(charger->reg) == -EPROBE_DEFER)
 			return -EPROBE_DEFER;
 		dev_info(&pdev->dev, "couldn't get charger regulator\n");
 	}
-	charger->edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
-	if (IS_ERR(charger->edev)) {
-		if (PTR_ERR(charger->edev) == -EPROBE_DEFER)
+	charger->edev = extcon_get_extcon_dev("max8997-muic");
+	if (IS_ERR_OR_NULL(charger->edev)) {
+		if (!charger->edev)
 			return -EPROBE_DEFER;
 		dev_info(charger->dev, "couldn't get extcon device\n");
 	}
 
-	if (!IS_ERR(charger->reg) && !IS_ERR(charger->edev)) {
+	if (!IS_ERR(charger->reg) && !IS_ERR_OR_NULL(charger->edev)) {
 		INIT_WORK(&charger->extcon_work, max8997_battery_extcon_evt_worker);
 		ret = devm_add_action(&pdev->dev, max8997_battery_extcon_evt_stop_work, charger);
 		if (ret) {
-- 
2.25.1



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

Re: [PATCH 1/3] regulator: dt-bindings: Document charger-supply for max8997

[Krzysztof Kozlowski]

On Sat, Jan 30, 2021 at 05:29:17PM +0000, Timon Baetz wrote:
> Add charger-supply optional property to enable charging control.
> 
> Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
> ---
>  .../devicetree/bindings/regulator/max8997-regulator.txt          | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

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

Re: [PATCH 3/3] power: supply: max8997_charger: Switch to new binding

[Krzysztof Kozlowski]

On Sat, Jan 30, 2021 at 05:30:14PM +0000, Timon Baetz wrote:
> Get regulator from parent device's node and extcon by name.
> 
> Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
> ---
>  drivers/power/supply/max8997_charger.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/power/supply/max8997_charger.c b/drivers/power/supply/max8997_charger.c
> index 321bd6b8ee41..625d8cc4312a 100644
> --- a/drivers/power/supply/max8997_charger.c
> +++ b/drivers/power/supply/max8997_charger.c
> @@ -168,6 +168,7 @@ static int max8997_battery_probe(struct platform_device *pdev)
>  	int ret = 0;
>  	struct charger_data *charger;
>  	struct max8997_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> +	struct device_node *np = pdev->dev.of_node;
>  	struct i2c_client *i2c = iodev->i2c;
>  	struct max8997_platform_data *pdata = iodev->pdata;
>  	struct power_supply_config psy_cfg = {};
> @@ -237,20 +238,23 @@ static int max8997_battery_probe(struct platform_device *pdev)
>  		return PTR_ERR(charger->battery);
>  	}
>  
> +	// grab regulator from parent device's node
> +	pdev->dev.of_node = iodev->dev->of_node;
>  	charger->reg = devm_regulator_get_optional(&pdev->dev, "charger");
> +	pdev->dev.of_node = np;

I think the device does not have its own node anymore. Or did I miss
something?

>  	if (IS_ERR(charger->reg)) {
>  		if (PTR_ERR(charger->reg) == -EPROBE_DEFER)
>  			return -EPROBE_DEFER;
>  		dev_info(&pdev->dev, "couldn't get charger regulator\n");
>  	}
> -	charger->edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
> -	if (IS_ERR(charger->edev)) {
> -		if (PTR_ERR(charger->edev) == -EPROBE_DEFER)
> +	charger->edev = extcon_get_extcon_dev("max8997-muic");
> +	if (IS_ERR_OR_NULL(charger->edev)) {
> +		if (!charger->edev)

Isn't NULL returned when there is simply no extcon? It's different than
deferred probe. Returning here EPROBE_DEFER might lead to infinite probe
tries (on every new device probe) instead of just failing it.

Best regards,
Krzysztof


>  			return -EPROBE_DEFER;
>  		dev_info(charger->dev, "couldn't get extcon device\n");
>  	}
>  
> -	if (!IS_ERR(charger->reg) && !IS_ERR(charger->edev)) {
> +	if (!IS_ERR(charger->reg) && !IS_ERR_OR_NULL(charger->edev)) {
>  		INIT_WORK(&charger->extcon_work, max8997_battery_extcon_evt_worker);
>  		ret = devm_add_action(&pdev->dev, max8997_battery_extcon_evt_stop_work, charger);
>  		if (ret) {
> -- 
> 2.25.1
> 
> 

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

Re: [PATCH 3/3] power: supply: max8997_charger: Switch to new binding

[Timon Baetz]

On Sun, 31 Jan 2021 18:28:40 +0100, Krzysztof Kozlowski wrote:
> On Sat, Jan 30, 2021 at 05:30:14PM +0000, Timon Baetz wrote:
> > Get regulator from parent device's node and extcon by name.
> >
> > Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
> > ---
> >  drivers/power/supply/max8997_charger.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/power/supply/max8997_charger.c b/drivers/power/supply/max8997_charger.c
> > index 321bd6b8ee41..625d8cc4312a 100644
> > --- a/drivers/power/supply/max8997_charger.c
> > +++ b/drivers/power/supply/max8997_charger.c
> > @@ -168,6 +168,7 @@ static int max8997_battery_probe(struct platform_device *pdev)
> >  	int ret = 0;
> >  	struct charger_data *charger;
> >  	struct max8997_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> > +	struct device_node *np = pdev->dev.of_node;
> >  	struct i2c_client *i2c = iodev->i2c;
> >  	struct max8997_platform_data *pdata = iodev->pdata;
> >  	struct power_supply_config psy_cfg = {};
> > @@ -237,20 +238,23 @@ static int max8997_battery_probe(struct platform_device *pdev)
> >  		return PTR_ERR(charger->battery);
> >  	}
> >
> > +	// grab regulator from parent device's node
> > +	pdev->dev.of_node = iodev->dev->of_node;
> >  	charger->reg = devm_regulator_get_optional(&pdev->dev, "charger");
> > +	pdev->dev.of_node = np;  
> 
> I think the device does not have its own node anymore. Or did I miss
> something?

The idea is to reset of_node to whatever it was before (NULL) and basically 
leave the device unchanged. Probe might run again because of deferral.

> >  	if (IS_ERR(charger->reg)) {
> >  		if (PTR_ERR(charger->reg) == -EPROBE_DEFER)
> >  			return -EPROBE_DEFER;
> >  		dev_info(&pdev->dev, "couldn't get charger regulator\n");
> >  	}
> > -	charger->edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
> > -	if (IS_ERR(charger->edev)) {
> > -		if (PTR_ERR(charger->edev) == -EPROBE_DEFER)
> > +	charger->edev = extcon_get_extcon_dev("max8997-muic");
> > +	if (IS_ERR_OR_NULL(charger->edev)) {
> > +		if (!charger->edev)  
> 
> Isn't NULL returned when there is simply no extcon? It's different than
> deferred probe. Returning here EPROBE_DEFER might lead to infinite probe
> tries (on every new device probe) instead of just failing it.

extcon_get_extcon_dev() just loops through all registered extcon devices
and compared names. It will return NULL when "max8997-muic" isn't
registered yet. extcon_get_extcon_dev() never returns EPROBE_DEFER so
checking for NULL seems to be the only way. Other drivers using that
function also do NULL check and return EPROBE_DEFER.

Thanks for reviewing,
Timon


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

Re: [PATCH 3/3] power: supply: max8997_charger: Switch to new binding

[Krzysztof Kozlowski]

On Mon, Feb 01, 2021 at 09:26:42AM +0000, Timon Baetz wrote:
> On Sun, 31 Jan 2021 18:28:40 +0100, Krzysztof Kozlowski wrote:
> > On Sat, Jan 30, 2021 at 05:30:14PM +0000, Timon Baetz wrote:
> > > Get regulator from parent device's node and extcon by name.
> > >
> > > Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
> > > ---
> > >  drivers/power/supply/max8997_charger.c | 12 ++++++++----
> > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/power/supply/max8997_charger.c b/drivers/power/supply/max8997_charger.c
> > > index 321bd6b8ee41..625d8cc4312a 100644
> > > --- a/drivers/power/supply/max8997_charger.c
> > > +++ b/drivers/power/supply/max8997_charger.c
> > > @@ -168,6 +168,7 @@ static int max8997_battery_probe(struct platform_device *pdev)
> > >  	int ret = 0;
> > >  	struct charger_data *charger;
> > >  	struct max8997_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> > > +	struct device_node *np = pdev->dev.of_node;
> > >  	struct i2c_client *i2c = iodev->i2c;
> > >  	struct max8997_platform_data *pdata = iodev->pdata;
> > >  	struct power_supply_config psy_cfg = {};
> > > @@ -237,20 +238,23 @@ static int max8997_battery_probe(struct platform_device *pdev)
> > >  		return PTR_ERR(charger->battery);
> > >  	}
> > >
> > > +	// grab regulator from parent device's node
> > > +	pdev->dev.of_node = iodev->dev->of_node;
> > >  	charger->reg = devm_regulator_get_optional(&pdev->dev, "charger");
> > > +	pdev->dev.of_node = np;  
> > 
> > I think the device does not have its own node anymore. Or did I miss
> > something?
> 
> The idea is to reset of_node to whatever it was before (NULL) and basically 
> leave the device unchanged. Probe might run again because of deferral.

Good point.

> 
> > >  	if (IS_ERR(charger->reg)) {
> > >  		if (PTR_ERR(charger->reg) == -EPROBE_DEFER)
> > >  			return -EPROBE_DEFER;
> > >  		dev_info(&pdev->dev, "couldn't get charger regulator\n");
> > >  	}
> > > -	charger->edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
> > > -	if (IS_ERR(charger->edev)) {
> > > -		if (PTR_ERR(charger->edev) == -EPROBE_DEFER)
> > > +	charger->edev = extcon_get_extcon_dev("max8997-muic");
> > > +	if (IS_ERR_OR_NULL(charger->edev)) {
> > > +		if (!charger->edev)  
> > 
> > Isn't NULL returned when there is simply no extcon? It's different than
> > deferred probe. Returning here EPROBE_DEFER might lead to infinite probe
> > tries (on every new device probe) instead of just failing it.
> 
> extcon_get_extcon_dev() just loops through all registered extcon devices
> and compared names. It will return NULL when "max8997-muic" isn't
> registered yet. extcon_get_extcon_dev() never returns EPROBE_DEFER so
> checking for NULL seems to be the only way. Other drivers using that
> function also do NULL check and return EPROBE_DEFER.

Indeed, thanks for clarification. Looks good:

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof


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

Re: (subset) [PATCH 0/3] charger-supply for max8997_charger

[Mark Brown]

On Sat, 30 Jan 2021 17:28:49 +0000, Timon Baetz wrote:
> Based on the discussion from [0] add an optional DT property to retrieve
> the regulator used for charging control in the max8997_charger driver.
> 
> [0] https://lore.kernel.org/lkml/20210118124505.GG4455@sirena.org.uk/
> 
> 
> Timon Baetz (3):
>   regulator: dt-bindings: Document charger-supply for max8997
>   ARM: dts: exynos: Add charger supply for I9100
>   power: supply: max8997_charger: Switch to new binding
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/3] regulator: dt-bindings: Document charger-supply for max8997
      commit: 41a8a027f4d3f81d83b8942ef29f84223ca35ffc

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

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

Re: [PATCH 2/3] ARM: dts: exynos: Add charger supply for I9100

[Timon Baetz]

On Sat, 30 Jan 2021 17:29:31 +0000, Timon Baetz wrote:
> The regulator is used for charging control by max8997_charger driver.
> 
> Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
> ---
>  arch/arm/boot/dts/exynos4210-i9100.dts | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
> index 304a8ee2364c..dad950daafb4 100644
> --- a/arch/arm/boot/dts/exynos4210-i9100.dts
> +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> @@ -384,6 +384,8 @@ pmic@66 {
>  		pinctrl-0 = <&max8997_irq>, <&otg_gp>, <&usb_sel>;
>  		pinctrl-names = "default";
>  
> +		charger-supply = <&charger_reg>;
> +
>  		regulators {
>  			vadc_reg: LDO1 {
>  				regulator-name = "VADC_3.3V_C210";

Now that the bindings have been accepted and integrated into linux-next,
is anything else blocking this?

Thanks,
Timon


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

Re: [PATCH 2/3] ARM: dts: exynos: Add charger supply for I9100

[Krzysztof Kozlowski]

On Thu, Feb 11, 2021 at 05:59:04AM +0000, Timon Baetz wrote:
> On Sat, 30 Jan 2021 17:29:31 +0000, Timon Baetz wrote:
> > The regulator is used for charging control by max8997_charger driver.
> > 
> > Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
> > ---
> >  arch/arm/boot/dts/exynos4210-i9100.dts | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
> > index 304a8ee2364c..dad950daafb4 100644
> > --- a/arch/arm/boot/dts/exynos4210-i9100.dts
> > +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> > @@ -384,6 +384,8 @@ pmic@66 {
> >  		pinctrl-0 = <&max8997_irq>, <&otg_gp>, <&usb_sel>;
> >  		pinctrl-names = "default";
> >  
> > +		charger-supply = <&charger_reg>;
> > +
> >  		regulators {
> >  			vadc_reg: LDO1 {
> >  				regulator-name = "VADC_3.3V_C210";
> 
> Now that the bindings have been accepted and integrated into linux-next,
> is anything else blocking this?

No, patch is fine. It's just too late in the cycle, so I'll take the
patch after merge window.

Best regards,
Krzysztof


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

Re: [PATCH 2/3] ARM: dts: exynos: Add charger supply for I9100

[Krzysztof Kozlowski]

On Sat, Jan 30, 2021 at 05:29:31PM +0000, Timon Baetz wrote:
> The regulator is used for charging control by max8997_charger driver.
> 
> Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
> ---
>  arch/arm/boot/dts/exynos4210-i9100.dts | 2 ++
>  1 file changed, 2 insertions(+)

Thanks, applied.

Best regards,
Krzysztof


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

Re: [PATCH 3/3] power: supply: max8997_charger: Switch to new binding

[Timon Baetz]

On Mon, 1 Feb 2021 19:03:35 +0100, Krzysztof Kozlowski wrote:
> On Mon, Feb 01, 2021 at 09:26:42AM +0000, Timon Baetz wrote:
> > On Sun, 31 Jan 2021 18:28:40 +0100, Krzysztof Kozlowski wrote:
> > > On Sat, Jan 30, 2021 at 05:30:14PM +0000, Timon Baetz wrote:
> > > > Get regulator from parent device's node and extcon by name.
> > > >
> > > > Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
> > > > ---
> > > >  drivers/power/supply/max8997_charger.c | 12 ++++++++----
> > > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/power/supply/max8997_charger.c b/drivers/power/supply/max8997_charger.c
> > > > index 321bd6b8ee41..625d8cc4312a 100644
> > > > --- a/drivers/power/supply/max8997_charger.c
> > > > +++ b/drivers/power/supply/max8997_charger.c
> > > > @@ -168,6 +168,7 @@ static int max8997_battery_probe(struct platform_device *pdev)
> > > >  	int ret = 0;
> > > >  	struct charger_data *charger;
> > > >  	struct max8997_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> > > > +	struct device_node *np = pdev->dev.of_node;
> > > >  	struct i2c_client *i2c = iodev->i2c;
> > > >  	struct max8997_platform_data *pdata = iodev->pdata;
> > > >  	struct power_supply_config psy_cfg = {};
> > > > @@ -237,20 +238,23 @@ static int max8997_battery_probe(struct platform_device *pdev)
> > > >  		return PTR_ERR(charger->battery);
> > > >  	}
> > > >
> > > > +	// grab regulator from parent device's node
> > > > +	pdev->dev.of_node = iodev->dev->of_node;
> > > >  	charger->reg = devm_regulator_get_optional(&pdev->dev, "charger");
> > > > +	pdev->dev.of_node = np;
> > >
> > > I think the device does not have its own node anymore. Or did I miss
> > > something?
> >
> > The idea is to reset of_node to whatever it was before (NULL) and basically
> > leave the device unchanged. Probe might run again because of deferral.
>
> Good point.
>
> >
> > > >  	if (IS_ERR(charger->reg)) {
> > > >  		if (PTR_ERR(charger->reg) == -EPROBE_DEFER)
> > > >  			return -EPROBE_DEFER;
> > > >  		dev_info(&pdev->dev, "couldn't get charger regulator\n");
> > > >  	}
> > > > -	charger->edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
> > > > -	if (IS_ERR(charger->edev)) {
> > > > -		if (PTR_ERR(charger->edev) == -EPROBE_DEFER)
> > > > +	charger->edev = extcon_get_extcon_dev("max8997-muic");
> > > > +	if (IS_ERR_OR_NULL(charger->edev)) {
> > > > +		if (!charger->edev)
> > >
> > > Isn't NULL returned when there is simply no extcon? It's different than
> > > deferred probe. Returning here EPROBE_DEFER might lead to infinite probe
> > > tries (on every new device probe) instead of just failing it.
> >
> > extcon_get_extcon_dev() just loops through all registered extcon devices
> > and compared names. It will return NULL when "max8997-muic" isn't
> > registered yet. extcon_get_extcon_dev() never returns EPROBE_DEFER so
> > checking for NULL seems to be the only way. Other drivers using that
> > function also do NULL check and return EPROBE_DEFER.
>
> Indeed, thanks for clarification. Looks good:
>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Is something blocking this from being accepted?

Timon



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

Re: [PATCH 3/3] power: supply: max8997_charger: Switch to new binding

[Timon Baetz]

On Sat, 30 Jan 2021 17:30:14 +0000, Timon Baetz wrote:
> Get regulator from parent device's node and extcon by name.
>
> Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
> ---
>  drivers/power/supply/max8997_charger.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/power/supply/max8997_charger.c b/drivers/power/supply/max8997_charger.c
> index 321bd6b8ee41..625d8cc4312a 100644
> --- a/drivers/power/supply/max8997_charger.c
> +++ b/drivers/power/supply/max8997_charger.c
> @@ -168,6 +168,7 @@ static int max8997_battery_probe(struct platform_device *pdev)
>  	int ret = 0;
>  	struct charger_data *charger;
>  	struct max8997_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> +	struct device_node *np = pdev->dev.of_node;
>  	struct i2c_client *i2c = iodev->i2c;
>  	struct max8997_platform_data *pdata = iodev->pdata;
>  	struct power_supply_config psy_cfg = {};
> @@ -237,20 +238,23 @@ static int max8997_battery_probe(struct platform_device *pdev)
>  		return PTR_ERR(charger->battery);
>  	}
>
> +	// grab regulator from parent device's node
> +	pdev->dev.of_node = iodev->dev->of_node;
>  	charger->reg = devm_regulator_get_optional(&pdev->dev, "charger");
> +	pdev->dev.of_node = np;
>  	if (IS_ERR(charger->reg)) {
>  		if (PTR_ERR(charger->reg) == -EPROBE_DEFER)
>  			return -EPROBE_DEFER;
>  		dev_info(&pdev->dev, "couldn't get charger regulator\n");
>  	}
> -	charger->edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
> -	if (IS_ERR(charger->edev)) {
> -		if (PTR_ERR(charger->edev) == -EPROBE_DEFER)
> +	charger->edev = extcon_get_extcon_dev("max8997-muic");
> +	if (IS_ERR_OR_NULL(charger->edev)) {
> +		if (!charger->edev)
>  			return -EPROBE_DEFER;
>  		dev_info(charger->dev, "couldn't get extcon device\n");
>  	}
>
> -	if (!IS_ERR(charger->reg) && !IS_ERR(charger->edev)) {
> +	if (!IS_ERR(charger->reg) && !IS_ERR_OR_NULL(charger->edev)) {
>  		INIT_WORK(&charger->extcon_work, max8997_battery_extcon_evt_worker);
>  		ret = devm_add_action(&pdev->dev, max8997_battery_extcon_evt_stop_work, charger);
>  		if (ret) {


Hello, I am not sure if I am missing something here. The other 2
patches of the series already made it into linux-next but this is still
pending. Do I have to resubmit? The patch still applies without
conflicts. Any help would be appreciated.

Thanks, Timon


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