LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Craig Tatlor <ctatlor97@gmail.com>
Cc: linux-arm-msm@vger.kernel.org, Sebastian Reichel <sre@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	Linux PM list <linux-pm@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/3] dt-bindings: power: supply: qcom_bms: Add bindings
Date: Thu, 26 Apr 2018 13:39:50 +0200	[thread overview]
Message-ID: <CACRpkdZ4VO_9eNvJdznoqd0cdV2qCbnFjjhQOtZKgpxFES1-VQ@mail.gmail.com> (raw)
In-Reply-To: <20180407134712.23131-2-ctatlor97@gmail.com>

On Sat, Apr 7, 2018 at 3:46 PM, Craig Tatlor <ctatlor97@gmail.com> wrote:

This is very interesting bindings!

> Add bindings for the Qualcomm battery measurement system.
>
> Signed-off-by: Craig Tatlor <ctatlor97@gmail.com>

Please expand on acronyms as requested by Rob.

> +Required properties:
> +- compatible:               Should contain "qcom,pm8941-bms".
> +- reg:                      Specifies the SPMI address and length of the
> +                           controller's registers.
> +- interrupts:               OCV threshold interrupt.
> +- io-channels:              Should contain IIO channel specifier for the
> +                           ADC channel that reports battery temperature.
> +- io-channel-names:         Should contain "temp".

These are fine (and good) as bindings go.

> +- qcom,fcc-temp-legend:     An array containing the temperature, in degC,
> +                           for each column of the FCC lookup table.
> +- qcom,fcc-lut:             An array of FCC values in mah, one entry for each
> +                           temperature defined in in qcom,fcc-temp-legend.
> +- qcom,ocv-temp-legend:     An array containing the temperature, in degC,
> +                           for each column of the OCV lookup table.
> +- qcom,ocv-capacity-legend: An array containing the capacity for each
> +                           row of the OCV lookup table.
> +- qcom,ocv-lut:             An array of OCV values in mV, one entry for each
> +                           capacity defined in qcom,ocv-capacity-legend.

I wonder if these are really Qualcomm-specific.

Can't we just cut the "qcom,*" prefix from all and simply define
these lookup tables in a way (with units) that makes sense to all
kinds of battery capacities?

Maybe spell them out though and avoid the abbreviations.

Just put those in the already existing
Documentation/devicetree/bindings/power/supply/battery.txt
so others can reuse them from there.

You will see that those bindings does not abbreviate but
spells out the natural sciences definitions to it's a bliss to
read for everyone.

Yours,
Linus Walleij

  reply	other threads:[~2018-04-26 11:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-07 13:46 [PATCH 1/3] power: supply: Add support for the Qualcomm Battery Monitoring System Craig Tatlor
2018-04-07 13:46 ` [PATCH 2/3] dt-bindings: power: supply: qcom_bms: Add bindings Craig Tatlor
2018-04-26 11:39   ` Linus Walleij [this message]
2018-04-07 13:46 ` [PATCH 3/3] MAINTAINERS: Add entry for the Qualcomm BMS Craig Tatlor
2018-04-26 18:21 ` [PATCH 1/3] power: supply: Add support for the Qualcomm Battery Monitoring System Bjorn Andersson

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=CACRpkdZ4VO_9eNvJdznoqd0cdV2qCbnFjjhQOtZKgpxFES1-VQ@mail.gmail.com \
    --to=linus.walleij@linaro.org \
    --cc=akpm@linux-foundation.org \
    --cc=ctatlor97@gmail.com \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mchehab@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=robh+dt@kernel.org \
    --cc=sre@kernel.org \
    --subject='Re: [PATCH 2/3] dt-bindings: power: supply: qcom_bms: Add bindings' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).