Linux-OMAP Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 2/5] ARM: dts: add battery phandle to cpcap_charger
@ 2021-01-17 21:45 Carl Philipp Klemm
  2021-03-24 11:54 ` Tony Lindgren
  0 siblings, 1 reply; 7+ messages in thread
From: Carl Philipp Klemm @ 2021-01-17 21:45 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: devicetree, linux-omap, Arthur Demchenkov, Tony Lindgren,
	Merlijn Wajer, Pavel Machek

This is required for 
	power: supply: cpcap-charger: get the battery inserted 
		infomation from cpcap-battery

Signed-off-by: Carl Philipp Klemm <philipp@uvos.xyz>
---
 arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
index 08a7d3ce383f..842eaa7b89e5 100644
--- a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
+++ b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
@@ -61,6 +61,7 @@ &cpcap_adc 2 &cpcap_adc 5
 			io-channel-names = "battdetb", "battp",
 					   "vbus", "chg_isense",
 					   "batti";
+			battery = <&cpcap_battery>;
 		};
 
 		cpcap_regulator: regulator {
-- 
2.29.2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] ARM: dts: add battery phandle to cpcap_charger
  2021-01-17 21:45 [PATCH 2/5] ARM: dts: add battery phandle to cpcap_charger Carl Philipp Klemm
@ 2021-03-24 11:54 ` Tony Lindgren
  2021-03-24 15:42   ` Sebastian Reichel
  0 siblings, 1 reply; 7+ messages in thread
From: Tony Lindgren @ 2021-03-24 11:54 UTC (permalink / raw)
  To: Carl Philipp Klemm
  Cc: Sebastian Reichel, devicetree, linux-omap, Arthur Demchenkov,
	Merlijn Wajer, Pavel Machek

* Carl Philipp Klemm <philipp@uvos.xyz> [210117 23:45]:
> --- a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
> +++ b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
> @@ -61,6 +61,7 @@ &cpcap_adc 2 &cpcap_adc 5
>  			io-channel-names = "battdetb", "battp",
>  					   "vbus", "chg_isense",
>  					   "batti";
> +			battery = <&cpcap_battery>;
>  		};

This seems like good standard stuff to have, picking up this patch into
omap-for-v5.13/dt thanks.

Tony

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] ARM: dts: add battery phandle to cpcap_charger
  2021-03-24 11:54 ` Tony Lindgren
@ 2021-03-24 15:42   ` Sebastian Reichel
  2021-03-24 16:29     ` Tony Lindgren
       [not found]     ` <C2FC7740-006A-430F-AA29-67572473D18B@goldelico.com>
  0 siblings, 2 replies; 7+ messages in thread
From: Sebastian Reichel @ 2021-03-24 15:42 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Carl Philipp Klemm, devicetree, linux-omap, Arthur Demchenkov,
	Merlijn Wajer, Pavel Machek


[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

Hi,

On Wed, Mar 24, 2021 at 01:54:02PM +0200, Tony Lindgren wrote:
> * Carl Philipp Klemm <philipp@uvos.xyz> [210117 23:45]:
> > --- a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
> > +++ b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
> > @@ -61,6 +61,7 @@ &cpcap_adc 2 &cpcap_adc 5
> >  			io-channel-names = "battdetb", "battp",
> >  					   "vbus", "chg_isense",
> >  					   "batti";
> > +			battery = <&cpcap_battery>;
> >  		};
> 
> This seems like good standard stuff to have, picking up this patch into
> omap-for-v5.13/dt thanks.
> 
> Tony

This was btw. one of the patches that finally made me convert all
the binding files to YAML. This patch will now result in warning
being printed when you run 'make dtbs_check', since the binding has
not been updated.

I did not yet have time to review the patchset properly, so I have
not yet replied (partely, because of being busy with the YAML
stuff).

I think the patch is also wrong, since the information is already
described in DT - just the other way around: The battery references
the charger. This provides all required information to the kernel
for a 1:1 link.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] ARM: dts: add battery phandle to cpcap_charger
  2021-03-24 15:42   ` Sebastian Reichel
@ 2021-03-24 16:29     ` Tony Lindgren
       [not found]     ` <C2FC7740-006A-430F-AA29-67572473D18B@goldelico.com>
  1 sibling, 0 replies; 7+ messages in thread
From: Tony Lindgren @ 2021-03-24 16:29 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Carl Philipp Klemm, devicetree, linux-omap, Arthur Demchenkov,
	Merlijn Wajer, Pavel Machek

Hi,

* Sebastian Reichel <sre@kernel.org> [210324 15:43]:
> On Wed, Mar 24, 2021 at 01:54:02PM +0200, Tony Lindgren wrote:
> > * Carl Philipp Klemm <philipp@uvos.xyz> [210117 23:45]:
> > > --- a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
> > > +++ b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
> > > @@ -61,6 +61,7 @@ &cpcap_adc 2 &cpcap_adc 5
> > >  			io-channel-names = "battdetb", "battp",
> > >  					   "vbus", "chg_isense",
> > >  					   "batti";
> > > +			battery = <&cpcap_battery>;
> > >  		};
> > 
> > This seems like good standard stuff to have, picking up this patch into
> > omap-for-v5.13/dt thanks.
> 
> This was btw. one of the patches that finally made me convert all
> the binding files to YAML. This patch will now result in warning
> being printed when you run 'make dtbs_check', since the binding has
> not been updated.
> 
> I did not yet have time to review the patchset properly, so I have
> not yet replied (partely, because of being busy with the YAML
> stuff).
> 
> I think the patch is also wrong, since the information is already
> described in DT - just the other way around: The battery references
> the charger. This provides all required information to the kernel
> for a 1:1 link.

OK, I will drop this patch.

Regards,

Tony

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] ARM: dts: add battery phandle to cpcap_charger
       [not found]     ` <C2FC7740-006A-430F-AA29-67572473D18B@goldelico.com>
@ 2021-03-24 23:19       ` Sebastian Reichel
  0 siblings, 0 replies; 7+ messages in thread
From: Sebastian Reichel @ 2021-03-24 23:19 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Tony Lindgren, Carl Philipp Klemm, devicetree, linux-omap,
	Arthur Demchenkov, Merlijn Wajer, Pavel Machek


[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

Hi,

On Wed, Mar 24, 2021 at 05:21:52PM +0100, H. Nikolaus Schaller wrote:
> > Am 24.03.2021 um 16:42 schrieb Sebastian Reichel <sre@kernel.org>:
> > I think the patch is also wrong, since the information is already
> > described in DT - just the other way around: The battery references
> > the charger.
> 
> Just curious for other devices to be properly defined:
> 
> Does a battery have its own driver?
> Can it be addressed (through I2C or similar mechanisms)?

Linux power-supply subsystem expects chargers and batteries
with battery being smart battery. I guess this has been
designed following the ACPI specs. On embedded devices this
usually means battery = fuel gauge.

> Is it closer to the processor (being the root node of DTS) or
> farther away than the chargers?

It depends? :) There are systems with smart batteries reachable
via I2C and gpio-charger device, which cannot be controlled and
just having an enable gpio (and thus being in the root node).
OTOH there are systems, that lack a proper fuel gauge and have
advanced I2C chargers.

> My observations is that usually chargers have drivers and need to
> reference battery information to adapt their behaviour.

I believe you are talking about chemistry information e.t.c.
that's available from the simple-battery node?

> So IMHO it would be more natural to have a charger reference the
> battery.

Direction was not something I came up with. I took over the
subsystem years ago when this was already in place. It's a
core thing in the subsystem and definetly cannot be changed
anymore:

Documentation/devicetree/bindings/power/supply/power-supply.yaml

In general sometimes the battery needs charger info and sometimes
charger needs fuel gauge info. The thing is, that this does not
mean we need phandles in both directions. One phandle is enough
to have the required information, everything else can be handled
by kernel frameworks. That's preferred, since it does not create
ABI.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] ARM: dts: add battery phandle to cpcap_charger
  2021-03-24 16:08 carl
@ 2021-03-24 23:35 ` Sebastian Reichel
  0 siblings, 0 replies; 7+ messages in thread
From: Sebastian Reichel @ 2021-03-24 23:35 UTC (permalink / raw)
  To: carl
  Cc: Pavel Machek, Arthur Demchenkov, Merlijn Wajer,
	Carl Philipp Klemm, devicetree, Tony Lindgren, linux-omap


[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]

Hi,

On Wed, Mar 24, 2021 at 05:08:58PM +0100, carl@uvos.xyz wrote:
> > I think the patch is also wrong, since the information is already 
> > described in DT - just the other way around: The battery references 
> > the charger. This provides all required information to the kernel 
> > for a 1:1 link. 
> 
> I added this so that cpcap_charger may become aware of the battery
> insertion state by querying the battery driver.

> Would you thus recommend that instead of adding this phandle i
> should amend the series such that cpcap_charger walks the tree
> looking for a cpcap_battery compatible node and then determines if
> the charger phandle points to itself? Is there some recommended
> way performing this reverse search?

I was thinking of creating a new core function to loop over all
supplied batteries of a power_supply (using psy->supplied_from),
but in this specific case it might be enough to just use
power_supply_get_by_name(). As I said, I have not yet properly
reviewed the full patchset.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] ARM: dts: add battery phandle to cpcap_charger
@ 2021-03-24 16:08 carl
  2021-03-24 23:35 ` Sebastian Reichel
  0 siblings, 1 reply; 7+ messages in thread
From: carl @ 2021-03-24 16:08 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Pavel Machek, Arthur Demchenkov, Merlijn Wajer,
	Carl Philipp Klemm, devicetree, Tony Lindgren, linux-omap

> I think the patch is also wrong, since the information is already 
> described in DT - just the other way around: The battery references 
> the charger. This provides all required information to the kernel 
> for a 1:1 link. 
>
> -- Sebastian 

I added this so that cpcap_charger may become aware of the battery insertion state by querying the battery driver.
Would you thus recommend that instead of adding this phandle i should amend the series such that cpcap_charger walks the tree looking for a cpcap_battery compatible node and then determines if the charger phandle points to itself? Is there some recommended way performing this reverse search?

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-17 21:45 [PATCH 2/5] ARM: dts: add battery phandle to cpcap_charger Carl Philipp Klemm
2021-03-24 11:54 ` Tony Lindgren
2021-03-24 15:42   ` Sebastian Reichel
2021-03-24 16:29     ` Tony Lindgren
     [not found]     ` <C2FC7740-006A-430F-AA29-67572473D18B@goldelico.com>
2021-03-24 23:19       ` Sebastian Reichel
2021-03-24 16:08 carl
2021-03-24 23:35 ` Sebastian Reichel

Linux-OMAP Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-omap/0 linux-omap/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-omap linux-omap/ https://lore.kernel.org/linux-omap \
		linux-omap@vger.kernel.org
	public-inbox-index linux-omap

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-omap


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git