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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 12 Nov 2015 17:10:04 -0600
From:	Rob Herring <robh@...nel.org>
To:	Stephen Boyd <sboyd@...eaurora.org>, Arnd Bergmann <arnd@...db.de>
Cc:	Andy Gross <agross@...eaurora.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-arm-msm <linux-arm-msm@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Olof Johansson <olof@...om.net>,
	Kevin Hilman <khilman@...nel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH 1/3] devicetree: bindings: Document qcom board compatible format

On Thu, Nov 12, 2015 at 1:44 PM, Stephen Boyd <sboyd@...eaurora.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/