[PATCH 0/3] Remove the need for qcom,{msm-id,board-id,pmic-id}

[PATCH 0/3] Remove the need for qcom,{msm-id,board-id,pmic-id}

[Stephen Boyd]

This patchset documents a compatible string format that encodes
all the information that was being encoded in qcom specific DT
properties in downstream msm kernels. The goal being to come
up with a format that will allow us to express the information
we want to express without requiring the use of vendor specific
properties. An updated dtbTool will be released after these new
bindings are accepted so that users can work with non-updateable
bootloaders.

This is an attempt to resolve a discussion around March of this year[1]
where the arm-soc maintainers suggested we express this information
through the board's compatible string.

[1] http://lkml.kernel.org/g/1425503602-24916-1-git-send-email-galak@codeaurora.org

Stephen Boyd (3):
  devicetree: bindings: Document qcom board compatible format
  arm64: dts: qcom: Make msm8916-mtp compatible string compliant
  arm: dts: qcom: Update ifc6540 compat for qcom boot format

 Documentation/devicetree/bindings/arm/qcom.txt | 86 ++++++++++++++++++++++++++
 arch/arm/boot/dts/qcom-apq8084-ifc6540.dts     |  2 +-
 arch/arm64/boot/dts/qcom/msm8916-mtp.dts       |  2 +-
 3 files changed, 88 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/qcom.txt

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 1/3] devicetree: bindings: Document qcom board compatible format

[Stephen Boyd]

Some qcom based bootloaders identify the dtb blob based on a set
of device properties like SoC, platform, PMIC, and revisions of
those components. In downstream kernels, these values are added
to the different component dtsi files (i.e. pmic dtsi file, SoC
dtsi file, board dtsi file, etc.) via qcom specific DT
properties. The dtb files are parsed by a program called dtbTool
that picks out these properties and creates a table of contents
binary blob with the property information and some offsets into
the concatenation of all the dtbs (termed a QCDT image).

The suggestion is to do this via the board compatible string
instead, because these qcom specific properties are never used by
the kernel. Add a document describing the format of the
compatible string that encodes all this information that's
currently encoded in the qcom,{msm-id,board-id,pmic-id}
properties in downstream devicetrees. Future bootloaders may be
updated to look at the compatible field instead of looking for
the table of contents image. For non-updateable bootloaders, a
new dtbTool program will parse the compatible string and generate
a QCDT image from it.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 Documentation/devicetree/bindings/arm/qcom.txt | 86 ++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/qcom.txt

diff --git a/Documentation/devicetree/bindings/arm/qcom.txt b/Documentation/devicetree/bindings/arm/qcom.txt
new file mode 100644
index 000000000000..ed084367182d
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/qcom.txt
@@ -0,0 +1,86 @@
+QCOM device tree bindings
+-------------------------
+
+Some qcom based bootloaders identify the dtb blob based on a set of
+device properties like SoC, platform, PMIC, and revisions of those components.
+To support this scheme, we encode this information into the board compatible
+string.
+
+Each board must specify a top-level board compatible string with the following
+format:
+
+	compatible = "qcom,<SoC>(-<soc_version>)(-<foundry_id>)-<plat_type>(/<subtype>)(-<plat_version>)(-<mb>MB)(-<panel>-panel)(-boot-<boot>)(-<pmic>(-v<pmic_version>)){0-4}"
+
+where elements in parentheses "()" are optional and elements in brackets "<>"
+are names of elements. Meaning only the 'SoC' and 'plat_type' elements are
+required.
+
+The 'SoC' element must be one of the following strings:
+
+	apq8016
+	apq8074
+	apq8084
+	apq8096
+	msm8916
+	msm8974
+	msm8996
+
+The 'plat_type' element must be one of the following strings:
+
+	cdp
+	liquid
+	dragonboard
+	mtp sbc
+
+
+The 'soc_version', 'plat_version' and 'pmic_version' elements take the form of
+v<Major>.<Minor> where the minor number may be omitted when it's zero, i.e.
+v1.0 is the same as v1. If all versions of the 'plat_version' element's match,
+then a wildcard '*' should be used, e.g. 'v*'.
+
+The 'foundry_id', 'subtype', and 'mb' elements are one or more digits from 0
+to 9.
+
+The 'panel' element must be one of the following strings:
+
+	720p
+	fWVGA
+	hd
+	qHD
+
+The 'boot' element must be one of the following strings:
+
+	emmc_sdc1
+	ufs
+
+The 'pmic' element must be one of the following strings:
+
+	pm8841
+	pm8019
+	pm8110
+	pma8084
+	pmi8962
+	pmd9635
+	pm8994
+	pmi8994
+	pm8916
+	pm8004
+	pm8909
+
+The 'pmic' element is specified in order of ascending USID. The PMIC in USID0
+goes first, and then USID2, USID4, and finally USID6. Up to four PMICs may be
+specified and no holes in the USID number space are allowed.
+
+Examples:
+
+	"qcom,msm8916-v1-cdp-pm8916-v2.1"
+
+A CDP board with an msm8916 SoC, version 1 paired with a pm8916 PMIC of version
+2.1.
+
+	"qcom,apq8074-v2.0-2-dragonboard/1-v0.1-512MB-panel-qHD-boot-emmc_sdc1-pm8941-v0.2-pm8909-v2.2-pma8084-v3-pm8110-v1"
+
+A dragonboard board v0.1 of subtype 1 with an apq8074 SoC version 2, made in
+foundry 2 with 512MB of memory and a qHD panel booting from emmc_sdc1, paired
+with a pm8941 PMIC version 0.2 at USID0, pm8909 PMIC version 2.2 at USID2,
+pma8084 version 3 at USID4 and a pm8110 version 1 at USID6.
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/3] devicetree: bindings: Document qcom board compatible format

[Andy Gross]

On Mon, Oct 26, 2015 at 02:25:10PM -0700, Stephen Boyd wrote:
> Some qcom based bootloaders identify the dtb blob based on a set
> of device properties like SoC, platform, PMIC, and revisions of
> those components. In downstream kernels, these values are added
> to the different component dtsi files (i.e. pmic dtsi file, SoC
> dtsi file, board dtsi file, etc.) via qcom specific DT
> properties. The dtb files are parsed by a program called dtbTool
> that picks out these properties and creates a table of contents
> binary blob with the property information and some offsets into
> the concatenation of all the dtbs (termed a QCDT image).
> 
> The suggestion is to do this via the board compatible string
> instead, because these qcom specific properties are never used by
> the kernel. Add a document describing the format of the
> compatible string that encodes all this information that's
> currently encoded in the qcom,{msm-id,board-id,pmic-id}
> properties in downstream devicetrees. Future bootloaders may be
> updated to look at the compatible field instead of looking for
> the table of contents image. For non-updateable bootloaders, a
> new dtbTool program will parse the compatible string and generate
> a QCDT image from it.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---

Looks workable.

Reviewed-by: Andy Gross <agross@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/3] devicetree: bindings: Document qcom board compatible format

[Rob Herring]

On Mon, Oct 26, 2015 at 02:25:10PM -0700, Stephen Boyd wrote:
> Some qcom based bootloaders identify the dtb blob based on a set
> of device properties like SoC, platform, PMIC, and revisions of
> those components. In downstream kernels, these values are added
> to the different component dtsi files (i.e. pmic dtsi file, SoC
> dtsi file, board dtsi file, etc.) via qcom specific DT
> properties. The dtb files are parsed by a program called dtbTool
> that picks out these properties and creates a table of contents
> binary blob with the property information and some offsets into
> the concatenation of all the dtbs (termed a QCDT image).

Got a pointer to what these properties look like?

> The suggestion is to do this via the board compatible string
> instead, because these qcom specific properties are never used by
> the kernel. Add a document describing the format of the
> compatible string that encodes all this information that's
> currently encoded in the qcom,{msm-id,board-id,pmic-id}
> properties in downstream devicetrees. Future bootloaders may be
> updated to look at the compatible field instead of looking for
> the table of contents image. For non-updateable bootloaders, a
> new dtbTool program will parse the compatible string and generate
> a QCDT image from it.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/arm/qcom.txt | 86 ++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/qcom.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom.txt b/Documentation/devicetree/bindings/arm/qcom.txt
> new file mode 100644
> index 000000000000..ed084367182d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/qcom.txt
> @@ -0,0 +1,86 @@
> +QCOM device tree bindings
> +-------------------------
> +
> +Some qcom based bootloaders identify the dtb blob based on a set of
> +device properties like SoC, platform, PMIC, and revisions of those components.
> +To support this scheme, we encode this information into the board compatible
> +string.

Why does all this need to be a single property?

> +Each board must specify a top-level board compatible string with the following
> +format:
> +
> +	compatible = "qcom,<SoC>(-<soc_version>)(-<foundry_id>)-<plat_type>(/<subtype>)(-<plat_version>)(-<mb>MB)(-<panel>-panel)(-boot-<boot>)(-<pmic>(-v<pmic_version>)){0-4}"
> +
> +where elements in parentheses "()" are optional and elements in brackets "<>"

[] brackets are more generally used for optional params.

> +are names of elements. Meaning only the 'SoC' and 'plat_type' elements are
> +required.
> +
> +The 'SoC' element must be one of the following strings:
> +
> +	apq8016
> +	apq8074
> +	apq8084
> +	apq8096
> +	msm8916
> +	msm8974
> +	msm8996
> +
> +The 'plat_type' element must be one of the following strings:
> +
> +	cdp
> +	liquid
> +	dragonboard
> +	mtp sbc

Platform is pretty overloaded meaning. Perhaps board_type would be more 
clear.

> +
> +The 'soc_version', 'plat_version' and 'pmic_version' elements take the form of
> +v<Major>.<Minor> where the minor number may be omitted when it's zero, i.e.
> +v1.0 is the same as v1. If all versions of the 'plat_version' element's match,
> +then a wildcard '*' should be used, e.g. 'v*'.
> +
> +The 'foundry_id', 'subtype', and 'mb' elements are one or more digits from 0
> +to 9.

Can you define what these are exactly. I gather mb is RAM size.

> +
> +The 'panel' element must be one of the following strings:
> +
> +	720p
> +	fWVGA
> +	hd
> +	qHD

How is this used?


> +The 'boot' element must be one of the following strings:
> +
> +	emmc_sdc1
> +	ufs
> +
> +The 'pmic' element must be one of the following strings:
> +
> +	pm8841
> +	pm8019
> +	pm8110
> +	pma8084
> +	pmi8962
> +	pmd9635
> +	pm8994
> +	pmi8994
> +	pm8916
> +	pm8004
> +	pm8909
> +
> +The 'pmic' element is specified in order of ascending USID. The PMIC in USID0
> +goes first, and then USID2, USID4, and finally USID6. Up to four PMICs may be
> +specified and no holes in the USID number space are allowed.

What is USID?

> +
> +Examples:
> +
> +	"qcom,msm8916-v1-cdp-pm8916-v2.1"
> +
> +A CDP board with an msm8916 SoC, version 1 paired with a pm8916 PMIC of version
> +2.1.
> +
> +	"qcom,apq8074-v2.0-2-dragonboard/1-v0.1-512MB-panel-qHD-boot-emmc_sdc1-pm8941-v0.2-pm8909-v2.2-pma8084-v3-pm8110-v1"

Which example is more common?

> +
> +A dragonboard board v0.1 of subtype 1 with an apq8074 SoC version 2, made in
> +foundry 2 with 512MB of memory and a qHD panel booting from emmc_sdc1, paired
> +with a pm8941 PMIC version 0.2 at USID0, pm8909 PMIC version 2.2 at USID2,
> +pma8084 version 3 at USID4 and a pm8110 version 1 at USID6.
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] devicetree: bindings: Document qcom board compatible format

[Stephen Boyd]

On 11/12, Rob Herring wrote:
> On Mon, Oct 26, 2015 at 02:25:10PM -0700, Stephen Boyd wrote:
> > Some qcom based bootloaders identify the dtb blob based on a set
> > of device properties like SoC, platform, PMIC, and revisions of
> > those components. In downstream kernels, these values are added
> > to the different component dtsi files (i.e. pmic dtsi file, SoC
> > dtsi file, board dtsi file, etc.) via qcom specific DT
> > properties. The dtb files are parsed by a program called dtbTool
> > that picks out these properties and creates a table of contents
> > binary blob with the property information and some offsets into
> > the concatenation of all the dtbs (termed a QCDT image).
> 
> Got a pointer to what these properties look like?

Do you mean the blob header format? You can see that described
in a text document next to the C file for dtbtool[1].

> 
> > The suggestion is to do this via the board compatible string
> > instead, because these qcom specific properties are never used by
> > the kernel. Add a document describing the format of the
> > compatible string that encodes all this information that's
> > currently encoded in the qcom,{msm-id,board-id,pmic-id}
> > properties in downstream devicetrees. Future bootloaders may be
> > updated to look at the compatible field instead of looking for
> > the table of contents image. For non-updateable bootloaders, a
> > new dtbTool program will parse the compatible string and generate
> > a QCDT image from it.
> > 
> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> > ---
> >  Documentation/devicetree/bindings/arm/qcom.txt | 86 ++++++++++++++++++++++++++
> >  1 file changed, 86 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/arm/qcom.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/qcom.txt b/Documentation/devicetree/bindings/arm/qcom.txt
> > new file mode 100644
> > index 000000000000..ed084367182d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/qcom.txt
> > @@ -0,0 +1,86 @@
> > +QCOM device tree bindings
> > +-------------------------
> > +
> > +Some qcom based bootloaders identify the dtb blob based on a set of
> > +device properties like SoC, platform, PMIC, and revisions of those components.
> > +To support this scheme, we encode this information into the board compatible
> > +string.
> 
> Why does all this need to be a single property?

Because the different vendor properties were rejected by arm-soc
maintainers and the board compatible string was suggested as the
place to put such information.

> 
> > +Each board must specify a top-level board compatible string with the following
> > +format:
> > +
> > +	compatible = "qcom,<SoC>(-<soc_version>)(-<foundry_id>)-<plat_type>(/<subtype>)(-<plat_version>)(-<mb>MB)(-<panel>-panel)(-boot-<boot>)(-<pmic>(-v<pmic_version>)){0-4}"
> > +
> > +where elements in parentheses "()" are optional and elements in brackets "<>"
> 
> [] brackets are more generally used for optional params.

Ok. I can make that change.

> 
> > +are names of elements. Meaning only the 'SoC' and 'plat_type' elements are
> > +required.
> > +
> > +The 'SoC' element must be one of the following strings:
> > +
> > +	apq8016
> > +	apq8074
> > +	apq8084
> > +	apq8096
> > +	msm8916
> > +	msm8974
> > +	msm8996
> > +
> > +The 'plat_type' element must be one of the following strings:
> > +
> > +	cdp
> > +	liquid
> > +	dragonboard
> > +	mtp sbc
> 
> Platform is pretty overloaded meaning. Perhaps board_type would be more 
> clear.

Ok.

> 
> > +
> > +The 'soc_version', 'plat_version' and 'pmic_version' elements take the form of
> > +v<Major>.<Minor> where the minor number may be omitted when it's zero, i.e.
> > +v1.0 is the same as v1. If all versions of the 'plat_version' element's match,
> > +then a wildcard '*' should be used, e.g. 'v*'.
> > +
> > +The 'foundry_id', 'subtype', and 'mb' elements are one or more digits from 0
> > +to 9.
> 
> Can you define what these are exactly. I gather mb is RAM size.

Not really, foundry_id is a number and so is subtype.

> 
> > +
> > +The 'panel' element must be one of the following strings:
> > +
> > +	720p
> > +	fWVGA
> > +	hd
> > +	qHD
> 
> How is this used?

I believe this was added so that we could have different dtbs for
devices that have different panels on them. I'm not sure this is
still used though. It could be legacy.

> 
> 
> > +The 'boot' element must be one of the following strings:
> > +
> > +	emmc_sdc1
> > +	ufs
> > +
> > +The 'pmic' element must be one of the following strings:
> > +
> > +	pm8841
> > +	pm8019
> > +	pm8110
> > +	pma8084
> > +	pmi8962
> > +	pmd9635
> > +	pm8994
> > +	pmi8994
> > +	pm8916
> > +	pm8004
> > +	pm8909
> > +
> > +The 'pmic' element is specified in order of ascending USID. The PMIC in USID0
> > +goes first, and then USID2, USID4, and finally USID6. Up to four PMICs may be
> > +specified and no holes in the USID number space are allowed.
> 
> What is USID?

USID is Unique Slave IDentifier. It's an SPMI concept.

> 
> > +
> > +Examples:
> > +
> > +	"qcom,msm8916-v1-cdp-pm8916-v2.1"
> > +
> > +A CDP board with an msm8916 SoC, version 1 paired with a pm8916 PMIC of version
> > +2.1.
> > +
> > +	"qcom,apq8074-v2.0-2-dragonboard/1-v0.1-512MB-panel-qHD-boot-emmc_sdc1-pm8941-v0.2-pm8909-v2.2-pma8084-v3-pm8110-v1"
> 
> Which example is more common?
> 

The former. I tried to make up the worst case example so we could
see how large the string may become.

[1] https://www.codeaurora.org/cgit/quic/la/device/qcom/common/tree/dtbtool/dtbtool.txt?h=LA.BF64.1.2.1.c1_rb1.30

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/3] devicetree: bindings: Document qcom board compatible format

[Rob Herring]

On Thu, Nov 12, 2015 at 1:44 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 11/12, Rob Herring wrote:
>> On Mon, Oct 26, 2015 at 02:25:10PM -0700, Stephen Boyd wrote:
>> > Some qcom based bootloaders identify the dtb blob based on a set
>> > of device properties like SoC, platform, PMIC, and revisions of
>> > those components. In downstream kernels, these values are added
>> > to the different component dtsi files (i.e. pmic dtsi file, SoC
>> > dtsi file, board dtsi file, etc.) via qcom specific DT
>> > properties. The dtb files are parsed by a program called dtbTool
>> > that picks out these properties and creates a table of contents
>> > binary blob with the property information and some offsets into
>> > the concatenation of all the dtbs (termed a QCDT image).
>>
>> Got a pointer to what these properties look like?
>
> Do you mean the blob header format? You can see that described
> in a text document next to the C file for dtbtool[1].

Thanks.

>> > +Some qcom based bootloaders identify the dtb blob based on a set of
>> > +device properties like SoC, platform, PMIC, and revisions of those components.
>> > +To support this scheme, we encode this information into the board compatible
>> > +string.
>>
>> Why does all this need to be a single property?
>
> Because the different vendor properties were rejected by arm-soc
> maintainers and the board compatible string was suggested as the
> place to put such information.

I'm surprised an 80+ character compatible stream is okay. The parts
giving me heartburn here are not mentioned in the previous discussion
nor the QCDT format.

As presented previously I agree with the push back. However, we could
do standard properties for SOC and board versions rather than
something vendor specific. Then the existing kernel support for
versions could use it. We could also just make this compatible string
formatting standard for more than just QC boards.

>> > +
>> > +The 'soc_version', 'plat_version' and 'pmic_version' elements take the form of
>> > +v<Major>.<Minor> where the minor number may be omitted when it's zero, i.e.
>> > +v1.0 is the same as v1. If all versions of the 'plat_version' element's match,
>> > +then a wildcard '*' should be used, e.g. 'v*'.
>> > +
>> > +The 'foundry_id', 'subtype', and 'mb' elements are one or more digits from 0
>> > +to 9.
>>
>> Can you define what these are exactly. I gather mb is RAM size.
>
> Not really, foundry_id is a number and so is subtype.

For mb, can't the tool just parse the memory node to get ram size
rather than parsing the compatible.

>> > +
>> > +The 'panel' element must be one of the following strings:
>> > +
>> > +   720p
>> > +   fWVGA
>> > +   hd
>> > +   qHD
>>
>> How is this used?
>
> I believe this was added so that we could have different dtbs for
> devices that have different panels on them. I'm not sure this is
> still used though. It could be legacy.

Dealing with multiple panels is fairly common. I think you could use
an alias to the panel node and match using its compatible or
resolution.

>> > +The 'boot' element must be one of the following strings:
>> > +
>> > +   emmc_sdc1
>> > +   ufs
>> > +

Again, perhaps an alias would work here.

>> > +The 'pmic' element must be one of the following strings:
>> > +
>> > +   pm8841
>> > +   pm8019
>> > +   pm8110
>> > +   pma8084
>> > +   pmi8962
>> > +   pmd9635
>> > +   pm8994
>> > +   pmi8994
>> > +   pm8916
>> > +   pm8004
>> > +   pm8909
>> > +
>> > +The 'pmic' element is specified in order of ascending USID. The PMIC in USID0
>> > +goes first, and then USID2, USID4, and finally USID6. Up to four PMICs may be
>> > +specified and no holes in the USID number space are allowed.
>>
>> What is USID?
>
> USID is Unique Slave IDentifier. It's an SPMI concept.

Okay, then please say "SPMI USID" or something.

>> > +Examples:
>> > +
>> > +   "qcom,msm8916-v1-cdp-pm8916-v2.1"
>> > +
>> > +A CDP board with an msm8916 SoC, version 1 paired with a pm8916 PMIC of version
>> > +2.1.
>> > +
>> > +   "qcom,apq8074-v2.0-2-dragonboard/1-v0.1-512MB-panel-qHD-boot-emmc_sdc1-pm8941-v0.2-pm8909-v2.2-pma8084-v3-pm8110-v1"
>>
>> Which example is more common?
>>
>
> The former. I tried to make up the worst case example so we could
> see how large the string may become.

I'm generally okay with the former. It's the latter that I don't like
so much. I'd like Arnd's thoughts here given he had many of the
comments.

Rob

Re: [PATCH 1/3] devicetree: bindings: Document qcom board compatible format

[Stephen Boyd]

On 11/12, Rob Herring wrote:
> On Thu, Nov 12, 2015 at 1:44 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 11/12, Rob Herring wrote:
> >> On Mon, Oct 26, 2015 at 02:25:10PM -0700, Stephen Boyd wrote:
> 
> >> > +Some qcom based bootloaders identify the dtb blob based on a set of
> >> > +device properties like SoC, platform, PMIC, and revisions of those components.
> >> > +To support this scheme, we encode this information into the board compatible
> >> > +string.
> >>
> >> Why does all this need to be a single property?
> >
> > Because the different vendor properties were rejected by arm-soc
> > maintainers and the board compatible string was suggested as the
> > place to put such information.
> 
> I'm surprised an 80+ character compatible stream is okay. The parts
> giving me heartburn here are not mentioned in the previous discussion
> nor the QCDT format.
> 
> As presented previously I agree with the push back. However, we could
> do standard properties for SOC and board versions rather than
> something vendor specific. Then the existing kernel support for
> versions could use it. We could also just make this compatible string
> formatting standard for more than just QC boards.

Some standard properties for these things sounds good to me.
What's the existing kernel support for versions though? Is that
just compatible string matching, or something else?

> 
> >> > +
> >> > +The 'soc_version', 'plat_version' and 'pmic_version' elements take the form of
> >> > +v<Major>.<Minor> where the minor number may be omitted when it's zero, i.e.
> >> > +v1.0 is the same as v1. If all versions of the 'plat_version' element's match,
> >> > +then a wildcard '*' should be used, e.g. 'v*'.
> >> > +
> >> > +The 'foundry_id', 'subtype', and 'mb' elements are one or more digits from 0
> >> > +to 9.
> >>
> >> Can you define what these are exactly. I gather mb is RAM size.
> >
> > Not really, foundry_id is a number and so is subtype.
> 
> For mb, can't the tool just parse the memory node to get ram size
> rather than parsing the compatible.

Sure. Right now the bootloader injects the memory information
during boot. I think it should work if we already have the memory
information there. I don't have any usage of mb at the moment
though, so if you want we can drop this field until a time that
we need it.

> 
> >> > +
> >> > +The 'panel' element must be one of the following strings:
> >> > +
> >> > +   720p
> >> > +   fWVGA
> >> > +   hd
> >> > +   qHD
> >>
> >> How is this used?
> >
> > I believe this was added so that we could have different dtbs for
> > devices that have different panels on them. I'm not sure this is
> > still used though. It could be legacy.
> 
> Dealing with multiple panels is fairly common. I think you could use
> an alias to the panel node and match using its compatible or
> resolution.

So dtbtool will need to resolve the alias and then figure out
which type of panel it is from compatible? Ok. I'd rather not
write that code unless it's needed, so I hope this field is not
used either.

> 
> >> > +The 'boot' element must be one of the following strings:
> >> > +
> >> > +   emmc_sdc1
> >> > +   ufs
> >> > +
> 
> Again, perhaps an alias would work here.
> 
> >> > +The 'pmic' element must be one of the following strings:
> >> > +
> >> > +   pm8841
> >> > +   pm8019
> >> > +   pm8110
> >> > +   pma8084
> >> > +   pmi8962
> >> > +   pmd9635
> >> > +   pm8994
> >> > +   pmi8994
> >> > +   pm8916
> >> > +   pm8004
> >> > +   pm8909
> >> > +
> >> > +The 'pmic' element is specified in order of ascending USID. The PMIC in USID0
> >> > +goes first, and then USID2, USID4, and finally USID6. Up to four PMICs may be
> >> > +specified and no holes in the USID number space are allowed.
> >>
> >> What is USID?
> >
> > USID is Unique Slave IDentifier. It's an SPMI concept.
> 
> Okay, then please say "SPMI USID" or something.

Ok.

In attempts to shorten the compatible string, we could use
aliases for the USIDs too. Then it should be possible to pull out
the information from the compatible fields of the PMIC nodes to
construct the PMIC part of the string. I'd like to avoid having
to go down the path of / -> soc -> spmi controller -> usidx,y,z
and just go straight to the usid node from a phandle.

With that in mind, right now I'm using fdtget from python to grab
the compatible string and parse it with a regex. If things need
to become more complicated to start following phandles, etc. are
there some python bindings to libfdt somewhere?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/3] devicetree: bindings: Document qcom board compatible format

[Rob Herring]

On Thu, Nov 12, 2015 at 6:11 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 11/12, Rob Herring wrote:
>> On Thu, Nov 12, 2015 at 1:44 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> > On 11/12, Rob Herring wrote:
>> >> On Mon, Oct 26, 2015 at 02:25:10PM -0700, Stephen Boyd wrote:
>>
>> >> > +Some qcom based bootloaders identify the dtb blob based on a set of
>> >> > +device properties like SoC, platform, PMIC, and revisions of those components.
>> >> > +To support this scheme, we encode this information into the board compatible
>> >> > +string.
>> >>
>> >> Why does all this need to be a single property?
>> >
>> > Because the different vendor properties were rejected by arm-soc
>> > maintainers and the board compatible string was suggested as the
>> > place to put such information.
>>
>> I'm surprised an 80+ character compatible stream is okay. The parts
>> giving me heartburn here are not mentioned in the previous discussion
>> nor the QCDT format.
>>
>> As presented previously I agree with the push back. However, we could
>> do standard properties for SOC and board versions rather than
>> something vendor specific. Then the existing kernel support for
>> versions could use it. We could also just make this compatible string
>> formatting standard for more than just QC boards.
>
> Some standard properties for these things sounds good to me.
> What's the existing kernel support for versions though? Is that
> just compatible string matching, or something else?

The soc_device stuff (and arm32 cpuinfo has a h/w version). Today I
think users are pulling version info from h/w registers, but if you
don't have h/w registers, then getting it from DT would make sense.


>> For mb, can't the tool just parse the memory node to get ram size
>> rather than parsing the compatible.
>
> Sure. Right now the bootloader injects the memory information
> during boot. I think it should work if we already have the memory
> information there. I don't have any usage of mb at the moment
> though, so if you want we can drop this field until a time that
> we need it.

If the bootloader injects it, then how do you know what to put into
the compatible string?

>> >> > +The 'panel' element must be one of the following strings:
>> >> > +
>> >> > +   720p
>> >> > +   fWVGA
>> >> > +   hd
>> >> > +   qHD
>> >>
>> >> How is this used?
>> >
>> > I believe this was added so that we could have different dtbs for
>> > devices that have different panels on them. I'm not sure this is
>> > still used though. It could be legacy.
>>
>> Dealing with multiple panels is fairly common. I think you could use
>> an alias to the panel node and match using its compatible or
>> resolution.
>
> So dtbtool will need to resolve the alias and then figure out
> which type of panel it is from compatible? Ok. I'd rather not
> write that code unless it's needed, so I hope this field is not
> used either.

Certainly better to figure out if it is needed first.

>> >> > +The 'boot' element must be one of the following strings:
>> >> > +
>> >> > +   emmc_sdc1
>> >> > +   ufs
>> >> > +
>>
>> Again, perhaps an alias would work here.
>>
>> >> > +The 'pmic' element must be one of the following strings:
>> >> > +
>> >> > +   pm8841
>> >> > +   pm8019
>> >> > +   pm8110
>> >> > +   pma8084
>> >> > +   pmi8962
>> >> > +   pmd9635
>> >> > +   pm8994
>> >> > +   pmi8994
>> >> > +   pm8916
>> >> > +   pm8004
>> >> > +   pm8909
>> >> > +
>> >> > +The 'pmic' element is specified in order of ascending USID. The PMIC in USID0
>> >> > +goes first, and then USID2, USID4, and finally USID6. Up to four PMICs may be
>> >> > +specified and no holes in the USID number space are allowed.
>> >>
>> >> What is USID?
>> >
>> > USID is Unique Slave IDentifier. It's an SPMI concept.
>>
>> Okay, then please say "SPMI USID" or something.
>
> Ok.
>
> In attempts to shorten the compatible string, we could use
> aliases for the USIDs too. Then it should be possible to pull out
> the information from the compatible fields of the PMIC nodes to
> construct the PMIC part of the string. I'd like to avoid having
> to go down the path of / -> soc -> spmi controller -> usidx,y,z
> and just go straight to the usid node from a phandle.

Yes. I was willing to draw the line after the PMIC, but seems Olof is less so.

> With that in mind, right now I'm using fdtget from python to grab
> the compatible string and parse it with a regex. If things need
> to become more complicated to start following phandles, etc. are
> there some python bindings to libfdt somewhere?

Not that I'm aware of. C is the only language you need. ;)

Rob

Re: [PATCH 1/3] devicetree: bindings: Document qcom board compatible format

[Stephen Boyd]

On 11/12, Rob Herring wrote:
> On Thu, Nov 12, 2015 at 6:11 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 11/12, Rob Herring wrote:
> >> On Thu, Nov 12, 2015 at 1:44 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >> > On 11/12, Rob Herring wrote:
> >> >> On Mon, Oct 26, 2015 at 02:25:10PM -0700, Stephen Boyd wrote:
> >>
> >> >> > +Some qcom based bootloaders identify the dtb blob based on a set of
> >> >> > +device properties like SoC, platform, PMIC, and revisions of those components.
> >> >> > +To support this scheme, we encode this information into the board compatible
> >> >> > +string.
> >> >>
> >> >> Why does all this need to be a single property?
> >> >
> >> > Because the different vendor properties were rejected by arm-soc
> >> > maintainers and the board compatible string was suggested as the
> >> > place to put such information.
> >>
> >> I'm surprised an 80+ character compatible stream is okay. The parts
> >> giving me heartburn here are not mentioned in the previous discussion
> >> nor the QCDT format.
> >>
> >> As presented previously I agree with the push back. However, we could
> >> do standard properties for SOC and board versions rather than
> >> something vendor specific. Then the existing kernel support for
> >> versions could use it. We could also just make this compatible string
> >> formatting standard for more than just QC boards.
> >
> > Some standard properties for these things sounds good to me.
> > What's the existing kernel support for versions though? Is that
> > just compatible string matching, or something else?
> 
> The soc_device stuff (and arm32 cpuinfo has a h/w version). Today I
> think users are pulling version info from h/w registers, but if you
> don't have h/w registers, then getting it from DT would make sense.

Ah ok. The version here is mostly about specifying some minimum
version or greater that the dtb matches against. If we were to
actually put the real version we may need more blobs for the
different combinations of board and SoC versions. So perhaps
leaving that in compatible string is the right way to go?

> 
> 
> >> For mb, can't the tool just parse the memory node to get ram size
> >> rather than parsing the compatible.
> >
> > Sure. Right now the bootloader injects the memory information
> > during boot. I think it should work if we already have the memory
> > information there. I don't have any usage of mb at the moment
> > though, so if you want we can drop this field until a time that
> > we need it.
> 
> If the bootloader injects it, then how do you know what to put into
> the compatible string?

Presumably the bootloader finds the matching size compatible
element and then populates the memory info that just happens to
match the same size. I don't know if this is happening, but it
certainly seems possible to have the same memory size at
different starting addresses. So we could simplify that situation
by having one size in the compatible that works for either
starting address. In summary, I don't 

> 
> > With that in mind, right now I'm using fdtget from python to grab
> > the compatible string and parse it with a regex. If things need
> > to become more complicated to start following phandles, etc. are
> > there some python bindings to libfdt somewhere?
> 
> Not that I'm aware of. C is the only language you need. ;)

Urgh. I converted dtbtool to python to make it simpler and avoid
that compilation step. I guess I'll explore wrapping some calls
to the C library inside dtbtool itself for this particular
purpose. It would be nice though if I could get a python object
for the root of the unflattened dtb that could be iterated and
inspected. I'll take a look at doing that sometime.

I'm thinking the aliases would be something like this:

	usid0 = <&pmic0>;
	usid2 = <&pmic1>;
	usid4 = <&pmic2>;
	usid6 = <&pmic3>;

We could specify usid1,3,5,7 too, but they're the same as the
0,2,4,6 ones.

So far I think all that will change is dropping the mb/panel
elements and parsing the PMIC ids from compatible strings and DT
traversal. Sounds right?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/3] devicetree: bindings: Document qcom board compatible format

[Olof Johansson]

Hi,

On Mon, Oct 26, 2015 at 2:25 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> Some qcom based bootloaders identify the dtb blob based on a set
> of device properties like SoC, platform, PMIC, and revisions of
> those components. In downstream kernels, these values are added
> to the different component dtsi files (i.e. pmic dtsi file, SoC
> dtsi file, board dtsi file, etc.) via qcom specific DT
> properties. The dtb files are parsed by a program called dtbTool
> that picks out these properties and creates a table of contents
> binary blob with the property information and some offsets into
> the concatenation of all the dtbs (termed a QCDT image).
>
> The suggestion is to do this via the board compatible string
> instead, because these qcom specific properties are never used by
> the kernel. Add a document describing the format of the
> compatible string that encodes all this information that's
> currently encoded in the qcom,{msm-id,board-id,pmic-id}
> properties in downstream devicetrees. Future bootloaders may be
> updated to look at the compatible field instead of looking for
> the table of contents image. For non-updateable bootloaders, a
> new dtbTool program will parse the compatible string and generate
> a QCDT image from it.
>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/arm/qcom.txt | 86 ++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/qcom.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/qcom.txt b/Documentation/devicetree/bindings/arm/qcom.txt
> new file mode 100644
> index 000000000000..ed084367182d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/qcom.txt
> @@ -0,0 +1,86 @@
> +QCOM device tree bindings
> +-------------------------
> +
> +Some qcom based bootloaders identify the dtb blob based on a set of
> +device properties like SoC, platform, PMIC, and revisions of those components.
> +To support this scheme, we encode this information into the board compatible
> +string.
> +
> +Each board must specify a top-level board compatible string with the following
> +format:
> +
> +       compatible = "qcom,<SoC>(-<soc_version>)(-<foundry_id>)-<plat_type>(/<subtype>)(-<plat_version>)(-<mb>MB)(-<panel>-panel)(-boot-<boot>)(-<pmic>(-v<pmic_version>)){0-4}"
> +
> +where elements in parentheses "()" are optional and elements in brackets "<>"
> +are names of elements. Meaning only the 'SoC' and 'plat_type' elements are
> +required.
> +
> +The 'SoC' element must be one of the following strings:
> +
> +       apq8016
> +       apq8074
> +       apq8084
> +       apq8096
> +       msm8916
> +       msm8974
> +       msm8996
> +
> +The 'plat_type' element must be one of the following strings:
> +
> +       cdp
> +       liquid
> +       dragonboard
> +       mtp sbc
> +
> +
> +The 'soc_version', 'plat_version' and 'pmic_version' elements take the form of
> +v<Major>.<Minor> where the minor number may be omitted when it's zero, i.e.
> +v1.0 is the same as v1. If all versions of the 'plat_version' element's match,
> +then a wildcard '*' should be used, e.g. 'v*'.
> +
> +The 'foundry_id', 'subtype', and 'mb' elements are one or more digits from 0
> +to 9.
> +
> +The 'panel' element must be one of the following strings:
> +
> +       720p
> +       fWVGA
> +       hd
> +       qHD
> +
> +The 'boot' element must be one of the following strings:
> +
> +       emmc_sdc1
> +       ufs
> +
> +The 'pmic' element must be one of the following strings:
> +
> +       pm8841
> +       pm8019
> +       pm8110
> +       pma8084
> +       pmi8962
> +       pmd9635
> +       pm8994
> +       pmi8994
> +       pm8916
> +       pm8004
> +       pm8909
> +
> +The 'pmic' element is specified in order of ascending USID. The PMIC in USID0
> +goes first, and then USID2, USID4, and finally USID6. Up to four PMICs may be
> +specified and no holes in the USID number space are allowed.
> +
> +Examples:
> +
> +       "qcom,msm8916-v1-cdp-pm8916-v2.1"

This is just awkward, but this...

> +
> +A CDP board with an msm8916 SoC, version 1 paired with a pm8916 PMIC of version
> +2.1.
> +
> +       "qcom,apq8074-v2.0-2-dragonboard/1-v0.1-512MB-panel-qHD-boot-emmc_sdc1-pm8941-v0.2-pm8909-v2.2-pma8084-v3-pm8110-v1"

...this is just too much. It makes no sense to try to linearly
describe pretty much the whole hardware in the compatible string like
this when the information should be elsewhere in the DT.

If this is how it's done, why bother documenting the rest in device
tree at all? Why not just do a depth-first traversal of the DT and
create a string out of that and make that the compatible while you're
at it?

Consider this NAK:ed.


-Olof

Re: [PATCH 1/3] devicetree: bindings: Document qcom board compatible format

[Stephen Boyd]

On 11/12, Olof Johansson wrote:
> On Mon, Oct 26, 2015 at 2:25 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > +Examples:
> > +
> > +       "qcom,msm8916-v1-cdp-pm8916-v2.1"
> 
> This is just awkward, but this...
> 
> > +
> > +A CDP board with an msm8916 SoC, version 1 paired with a pm8916 PMIC of version
> > +2.1.
> > +
> > +       "qcom,apq8074-v2.0-2-dragonboard/1-v0.1-512MB-panel-qHD-boot-emmc_sdc1-pm8941-v0.2-pm8909-v2.2-pma8084-v3-pm8110-v1"
> 
> ...this is just too much. It makes no sense to try to linearly
> describe pretty much the whole hardware in the compatible string like
> this when the information should be elsewhere in the DT.
> 
> If this is how it's done, why bother documenting the rest in device
> tree at all? Why not just do a depth-first traversal of the DT and
> create a string out of that and make that the compatible while you're
> at it?

Haha. The entire device is just one big compatible string! I love
it! </sarcasm>

Seriously though, once the PMIC stuff appeared I started thinking
about some way to detect that dynamically because you're right,
it's already in DT somewhere and these huge compatible strings
are gross. Using aliases as Rob suggests should work nicely so
that we can find most of the elements with some simple tree
traversal. In the example above we would be left with
apq8074-v2.0-2-dragonboard/1-v0.1. Is that palatable?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/3] arm64: dts: qcom: Make msm8916-mtp compatible string compliant

[Stephen Boyd]

This compatible string isn't compliant with the format for
subtypes. Replace it with a compliant compatible type.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/msm8916-mtp.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916-mtp.dts b/arch/arm64/boot/dts/qcom/msm8916-mtp.dts
index fced77f0fd3a..b0a064d3806b 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-mtp.dts
@@ -17,6 +17,6 @@
 
 / {
 	model = "Qualcomm Technologies, Inc. MSM 8916 MTP";
-	compatible = "qcom,msm8916-mtp", "qcom,msm8916-mtp-smb1360",
+	compatible = "qcom,msm8916-mtp", "qcom,msm8916-mtp/1",
 			"qcom,msm8916", "qcom,mtp";
 };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 3/3] arm: dts: qcom: Update ifc6540 compat for qcom boot format

[Stephen Boyd]

The ifc6540 is an sbc (single board computer) board, so update
the compatible field accordingly.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/boot/dts/qcom-apq8084-ifc6540.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom-apq8084-ifc6540.dts b/arch/arm/boot/dts/qcom-apq8084-ifc6540.dts
index c9c2b769554f..32aaa9d45228 100644
--- a/arch/arm/boot/dts/qcom-apq8084-ifc6540.dts
+++ b/arch/arm/boot/dts/qcom-apq8084-ifc6540.dts
@@ -3,7 +3,7 @@
 
 / {
 	model = "Qualcomm APQ8084/IFC6540";
-	compatible = "qcom,apq8084-ifc6540", "qcom,apq8084";
+	compatible = "qcom,apq8084-sbc", "qcom,apq8084";
 
 	aliases {
 		serial0 = &blsp2_uart2;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 0/3] Remove the need for qcom,{msm-id,board-id,pmic-id}

[Kevin Hilman]

Hi Stephen,

Stephen Boyd <sboyd@codeaurora.org> writes:

> This patchset documents a compatible string format that encodes
> all the information that was being encoded in qcom specific DT
> properties in downstream msm kernels. The goal being to come
> up with a format that will allow us to express the information
> we want to express without requiring the use of vendor specific
> properties. An updated dtbTool will be released after these new
> bindings are accepted so that users can work with non-updateable
> bootloaders.
>
> This is an attempt to resolve a discussion around March of this year[1]
> where the arm-soc maintainers suggested we express this information
> through the board's compatible string.

Thanks for working on this.  This indeed looks like the right way to
handle this, including updating dtbTool.  Very nice.

I guess the DT maintainers should have a crack at the bindings, but
otherwise it looks good.

Kevin