Re: [PATCH RFC v3 2/9] dt-bindings: board: Introduce board-id

2024-05-22 Thread Rob Herring (Arm)


On Wed, 22 May 2024 16:54:23 -0700, Elliot Berman wrote:
> Device manufcturers frequently ship multiple boards or SKUs under a
> single softwre package. These software packages ship multiple devicetree
> blobs and require some mechanims to pick the correct DTB for the boards
> that use the software package. This patch introduces a common language
> for adding board identifiers to devicetrees.
> 
> Signed-off-by: Elliot Berman 
> ---
>  .../devicetree/bindings/board/board-id.yaml| 71 
> ++
>  1 file changed, 71 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2.example.dtb:
 opp-table-0: opp-12:opp-microvolt-slow:0: [915000, 90, 925000, 
925000, 91, 935000] is too long
from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2.example.dtb:
 opp-table-0: opp-12:opp-microvolt-fast:0: [975000, 97, 985000, 
965000, 96, 975000] is too long
from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2.example.dtb:
 opp-table-0: Unevaluated properties are not allowed ('opp-10', 
'opp-12', 'opp-shared' were unexpected)
from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
compress: size (5) error for type uint32-matrix
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.example.dtb:
 uimage@10: compress: b'lzma\x00' is not of type 'object', 'array', 
'boolean', 'null'
from schema $id: http://devicetree.org/schemas/dt-core.yaml#
marvell,pad-type: size (11) error for type uint32-matrix
marvell,pad-type: size (3) error for type uint32-matrix
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@aa: marvell,pad-type: b'fixed-1-8v\x00' is not of type 'array'
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@aa: marvell,pad-type: b'fixed-1-8v\x00' is not of type 'array'
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@aa: Unevaluated properties are not allowed ('marvell,pad-type' was 
unexpected)
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@aa: marvell,pad-type: b'fixed-1-8v\x00' is not of type 'object', 
'array', 'boolean', 'null'
from schema $id: http://devicetree.org/schemas/dt-core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@ab: marvell,pad-type: b'sd\x00' is not of type 'array'
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@ab: marvell,pad-type: b'sd\x00' is not of type 'array'
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@ab: Unevaluated properties are not allowed ('marvell,pad-type' was 
unexpected)
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@ab: marvell,pad-type: b'sd\x00' is not of type 'object', 'array', 
'boolean', 'null'
from schema $id: http://devicetree.org/schemas/dt-core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/sc27xx-fg.example.dtb:
 battery: ocv-capacity-table-0:0: [4185000, 100, 4113000, 95, 4066000, 90, 
4022000, 85, 3983000, 80, 3949000, 75, 3917000, 70, 3889000, 65, 3864000, 60, 
3835000, 55, 3805000, 50, 3787000, 45, 3777000, 40, 3773000, 35, 377, 30, 
3765000, 25, 3752000, 20, 3724000, 15, 368, 10, 3605000, 5, 340, 0] is 
too long
from schema $id: 
http://devicetree.org/schemas/power/supply/battery.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/battery.example.dtb:
 battery: ocv-capacity-table-0:0: [4185000, 100, 4113000, 95, 4066000, 90] is 
too long
from schema $id: 
http://devicetree.org/schemas/power/supply/battery.yaml#

Re: [PATCH RFC v3 2/9] dt-bindings: board: Introduce board-id

2024-05-21 Thread Rob Herring
On Tue, May 21, 2024 at 2:25 PM Conor Dooley  wrote:
>
> On Tue, May 21, 2024 at 08:21:45PM +0100, Conor Dooley wrote:
> > On Tue, May 21, 2024 at 11:37:59AM -0700, Elliot Berman wrote:
> > > Device manufcturers frequently ship multiple boards or SKUs under a
> > > single softwre package. These software packages ship multiple devicetree
> > > blobs and require some mechanims to pick the correct DTB for the boards
> > > that use the software package.
> >
> > Okay, you've got the problem statement here, nice.
> >
> > > This patch introduces a common language
> > > for adding board identifiers to devicetrees.
> >
> > But then a completely useless remainder of the commit message.
> > I open this patch, see the regexes, say "wtf", look at the commit
> > message and there is absolutely no explanation of what these properties
> > are for. That's quite frankly just not good enough - even for an RFC.
> >
> > >
> > > Signed-off-by: Elliot Berman 
> > > ---
> > >  .../devicetree/bindings/board/board-id.yaml| 24 
> > > ++
> > >  1 file changed, 24 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/board/board-id.yaml 
> > > b/Documentation/devicetree/bindings/board/board-id.yaml
> > > new file mode 100644
> > > index ..99514aef9718
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/board/board-id.yaml
> > > @@ -0,0 +1,24 @@
> > > +# SPDX-License-Identifier: BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/board/board-id.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: board identifiers
> > > +description: Common property for board-id subnode
> >
> > s/property/properties/
> >
> > > +
> > > +maintainers:
> > > +  - Elliot Berman 
> > > +
> > > +properties:
> > > +  $nodename:
> > > +const: '/'
> > > +  board-id:
> > > +type: object
> > > +patternProperties:
> > > +  "^.*(?!_str)$":
> >
> > Does this regex even work? Take "foo_str" as an example - doesn't "^.*"
> > consume all of the string, leaving the negative lookahead with nothing
> > to object to? I didn't properly test this with an example and the dt
> > tooling, but I lazily threw it into regex101 and both the python and
> > emcascript versions agree with me. Did you test this?
> >
> > And while I am here, no underscores in property names please. And if
> > "str" means string, I suggest not saving 3 characters.
> >
> > > +$ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > +  "^.*_str$":
> > > +$ref: /schemas/types.yaml#/definitions/string-array
> >
> > Why do we even need two methods? Commit message tells me nothing and
> > there's no description at all... Why do we need regexes here, rather
> > than explicitly defined properties? Your commit message should explain
> > the justification for that and the property descriptions (as comments if
> > needs be for patternProperties) should explain why this is intended to
> > be used.
> >
> > How is anyone supposed to look at this binding and understand how it
> > should be used?
>
> Also, please do not CC private mailing lists on your postings, I do not
> want to get spammed by linaro's mailman :(

boot-architecture is not private[0]. It is where EBBR gets discussed
amongst other things. This came up in a thread there[1].

Rob

[0] https://lists.linaro.org/mailman3/lists/boot-architecture.lists.linaro.org/
[1] 
https://lists.linaro.org/archives/list/boot-architecture@lists.linaro.org/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/
___
boot-architecture mailing list -- boot-architecture@lists.linaro.org
To unsubscribe send an email to boot-architecture-le...@lists.linaro.org


Re: [PATCH RFC v3 5/9] dt-bindings: board: Document board-ids for Qualcomm devices

2024-05-21 Thread Rob Herring (Arm)


On Tue, 21 May 2024 11:38:02 -0700, Elliot Berman wrote:
> Document board identifiers for devices from Qualcomm Technologies, Inc.
> These platforms are described with two mechanisms: the hardware SoC
> registers and the "CDT" which is in a RO storage.
> 
> The hardware SoC registers describe both the SoC (e.g. SM8650, SC7180)
> as well as revision. Add qcom,soc to describe only the SoC itself and
> qcom,soc-version when the devicetree only works with a certain revision.
> 
> The CDT describes all other information about the board/platform.
> Besides the platform type (e.g. MTP, ADP, CRD), there are 3 further
> levels of versioning as well as additional fields to describe the PMIC
> and boot storage device attached. The 3 levels of versioning are a
> subtype, major, and minor version of the platform. Support describing
> just the platform type (qcom,platform), the platform type and subtype
> (qcom,platform-type), and all 4 numbers (qcom,platform-version).
> 
> Signed-off-by: Elliot Berman 
> ---
>  .../devicetree/bindings/board/qcom,board-id.yaml   | 144 
> +
>  1 file changed, 144 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:15:12: [error] 
string value is redundantly quoted with any quotes (quoted-strings)
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:74:8: [error] 
empty value in block mapping (empty-values)
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:81:8: [error] 
empty value in block mapping (empty-values)
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:88:8: [error] 
empty value in block mapping (empty-values)
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:97:8: [error] 
empty value in block mapping (empty-values)
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:103:8: [error] 
empty value in block mapping (empty-values)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml:
 allOf:2:if: None is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml:
 allOf:3:if: None is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml:
 allOf:4:if: None is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml:
 allOf:5:if: None is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml:
 allOf:6:if: None is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml:
 $id: 'http://devicetree.org/schemas/board/qcom,board-id.yaml' does not match 
'http://devicetree.org/schemas/.*\\.yaml#'
from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml:
 $schema: 'http://devicetree.org/meta-schemas/core.yaml' is not one of 
['http://devicetree.org/meta-schemas/core.yaml#', 
'http://devicetree.org/meta-schemas/base.yaml#']
from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml:
 ignoring, error in schema: allOf: 2: if
Documentation/devicetree/bindings/board/qcom,board-id.example.dts:26:56: error: 
macro "QCOM_BOARD_ID" passed 4 arguments, but takes just 3
   26 | qcom,platform-version = ,
  |^
In file included from 
Documentation/devicetree/bindings/board/qcom,board-id.example.dts:4:
./scripts/dtc/include-prefixes/dt-bindings/arm/qcom,ids.h:279: note: macro 
"QCOM_BOARD_ID" defined here
  279 | #define QCOM_BOARD_ID(a, major, minor) \
  | 
Documentation/devicetree/bindings/board/qcom,board-id.example.dts:27:56: error: 
macro "QCOM_BOARD_ID" passed 4 arguments, but takes just 3
   27 | ;
  |^
./scripts/dtc/include-prefixes/dt-bindings/arm/qcom,ids.h:279: note: macro 
"QCOM_BOARD_ID" defined here
  279 | #define QCOM_BOARD_ID(a, major, minor) \
  | 
make[2]: *** [scripts/Makefile.lib:427: 
Documentation/devicetree/bindings/board/qcom,board-id.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: 

Re: [PATCH RFC v3 2/9] dt-bindings: board: Introduce board-id

2024-05-21 Thread Rob Herring (Arm)


On Tue, 21 May 2024 11:37:59 -0700, Elliot Berman wrote:
> Device manufcturers frequently ship multiple boards or SKUs under a
> single softwre package. These software packages ship multiple devicetree
> blobs and require some mechanims to pick the correct DTB for the boards
> that use the software package. This patch introduces a common language
> for adding board identifiers to devicetrees.
> 
> Signed-off-by: Elliot Berman 
> ---
>  .../devicetree/bindings/board/board-id.yaml| 24 
> ++
>  1 file changed, 24 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2.example.dtb:
 opp-table-0: opp-12:opp-microvolt-slow:0: [915000, 90, 925000, 
925000, 91, 935000] is too long
from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2.example.dtb:
 opp-table-0: opp-12:opp-microvolt-fast:0: [975000, 97, 985000, 
965000, 96, 975000] is too long
from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2.example.dtb:
 opp-table-0: Unevaluated properties are not allowed ('opp-10', 
'opp-12', 'opp-shared' were unexpected)
from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
compress: size (5) error for type uint32-matrix
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.example.dtb:
 uimage@10: compress: b'lzma\x00' is not of type 'object', 'array', 
'boolean', 'null'
from schema $id: http://devicetree.org/schemas/dt-core.yaml#
marvell,pad-type: size (11) error for type uint32-matrix
marvell,pad-type: size (3) error for type uint32-matrix
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@aa: marvell,pad-type: b'fixed-1-8v\x00' is not of type 'array'
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@aa: marvell,pad-type: b'fixed-1-8v\x00' is not of type 'array'
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@aa: Unevaluated properties are not allowed ('marvell,pad-type' was 
unexpected)
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@aa: marvell,pad-type: b'fixed-1-8v\x00' is not of type 'object', 
'array', 'boolean', 'null'
from schema $id: http://devicetree.org/schemas/dt-core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@ab: marvell,pad-type: b'sd\x00' is not of type 'array'
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@ab: marvell,pad-type: b'sd\x00' is not of type 'array'
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@ab: Unevaluated properties are not allowed ('marvell,pad-type' was 
unexpected)
from schema $id: 
http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb:
 mmc@ab: marvell,pad-type: b'sd\x00' is not of type 'object', 'array', 
'boolean', 'null'
from schema $id: http://devicetree.org/schemas/dt-core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/sc27xx-fg.example.dtb:
 battery: ocv-capacity-table-0:0: [4185000, 100, 4113000, 95, 4066000, 90, 
4022000, 85, 3983000, 80, 3949000, 75, 3917000, 70, 3889000, 65, 3864000, 60, 
3835000, 55, 3805000, 50, 3787000, 45, 3777000, 40, 3773000, 35, 377, 30, 
3765000, 25, 3752000, 20, 3724000, 15, 368, 10, 3605000, 5, 340, 0] is 
too long
from schema $id: 
http://devicetree.org/schemas/power/supply/battery.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/battery.example.dtb:
 battery: ocv-capacity-table-0:0: [4185000, 100, 4113000, 95, 4066000, 90] is 
too long
from schema $id: 
http://devicetree.org/schemas/power/supply/battery.yaml#

Re: OS provided DT proposal

2024-05-10 Thread Rob Herring
On Wed, May 8, 2024 at 7:19 PM Elliot Berman  wrote:
>
> On Thu, May 02, 2024 at 09:00:47AM -0500, Rob Herring wrote:
> > On Wed, May 1, 2024 at 4:18 PM Humphreys, Jonathan  
> > wrote:
> > > [1] Rather than using the device tree source filename, to have more 
> > > flexibility,
> > > one can conceive an ID or compatible string that the OS could then scan 
> > > the DTBs
> > > to find a match.
> >
> > I agree with Daniel that we should use the root node compatible for
> > this. We discussed this a while back on this list (or u-boot?). To
> > summarize, both using the filename or root node compatible were
> > proposed. Several folks (myself included) don't like making the
> > filename an ABI. However, there are some cases where the filename is
> > more unique than the root node compatible. We should fix those root
> > node compatibles in that case IMO.
>
> I think firmware-provided compatible string can cause headaches for both
> firmware and OS developers. I gave a talk about this at EOSS [1,2] and
> we've been posting some proposals [3,4] to introduce a board-id, which
> allows DTBs to have varying degrees of precision about describing what
> hardware they are applicable to.
>
> Compatible strings should be a mapping of some identifier
> registers/storage into a string. Today, bootloader has to figure out
> that mapping and I understood Jon's proposal as wanting to get firmware
> to provide the compatible string. However, the compatible string for a
> DTB could need to describe only a subset of those identifiers
> (compatible string) to get a DTB that works. This would be especially
> true for DT overlays, although there are other real and hypothetical
> situations where a DTB shouldn't/can't describe the complete set of
> identifiers. Firmware either needs to provide every possible combination
> of compatible string or knowledge needs to be baked into the OS about
> interpreting the compatible string. In simple terms, the proposal is to
> split out the identifers that are baked into the compatible string into
> separate "board-id" properties.

I don't think there is any way the OS (or OS loader) would be able
handle these properties and the logic to parse them. It all looks to
be platform specific. This could only work if the OS says to the
firmware "here's the 1000 DTB files, which one should I use? That's
quite different from the current proposals of how this would work.

Rob
___
boot-architecture mailing list -- boot-architecture@lists.linaro.org
To unsubscribe send an email to boot-architecture-le...@lists.linaro.org


Re: OS provided DT proposal

2024-05-07 Thread Rob Herring
On Mon, May 6, 2024 at 7:37 AM Ahmad Fatoum  wrote:
>
> Hello Jonathan,
> Hello Rob,
>
> Thanks for bringing this topic to discussion. I think the status quo is
> an obvious shortcoming that needs to be addressed.
>
> On 02.05.24 16:00, Rob Herring wrote:
> > On Wed, May 1, 2024 at 4:18 PM Humphreys, Jonathan  
> > wrote:
> >> Problem statement:
> >> ==
> >>
> >> Device trees are in theory a pure description of the hardware, and since 
> >> the hardware
> >> doesn't change, the device tree describing the hardware likewise never 
> >> changes.
> >> With this, a device tree could then be burned into the hardware's ROM to be
> >> queried by software for hardware discovery. In practice, though, device 
> >> trees
> >> evolve over time. They evolve for many reasons, including
> >> - support for previously unsupported hardware
> >> - device driver improvements that require additional hardware information
> >> - bug fixes
> >
> > I really would like specific cases of these where compatibility is
> > broken highlighted.
>
> Screening for backwards-compatibility of new kernels (or their bindings)
> with old DTs is not enough. When an A/B system fails to boot and does
> a fallback, you can run into the inverse situation, namely:
> An old kernel is presented with a new device tree as bootloader updates
> are often not rolled back.

I'm interested in both cases. Indeed, I think this problem is harder
than backwards compatibility. There's not much we can do if say a
platform was missing some provider (e.g. a clock controller) and then
you add one. The old OS is not going to know what to do with the clock
controller and all its 'clocks' property references as it lacks a
driver. We mitigated this somewhat in Linux to make some dependencies
optional or timeout. For example, if a platform booted without pinctrl
before, then any pinctrl added should be optional (unless the firmware
also stopped doing pin setup when it moved into the DT).

> This seems unavoidable and the solution we have for that is to ship device
> trees along with kernel updates and load both together.
>
> > The tooling and reviewing to identify these cases
> > has gotten much better.
>
> barebox has been pulling in kernel device trees for many years and it's
> a frequent cause of regressions. Here are some recent fixes
> found with $(git log --grep="^Fixes:.*dts: update"):

Thanks for the pointers. Some analysis below...

>
> * "aiodev: imx_thermal: fix breakage after device tree sync"
>   https://github.com/barebox/barebox/commit/451c25b60e

Removing a required property is something I check. Would not have
helped here as it got moved from required to deprecated in 2017.
Required and deprecated should be orthogonal. For the DTS, what should
have happened here is the 'fsl,tempmon-data" property should have been
kept with the nvmem properties added.

> * "pinctrl: stm32: Remove check for pins-are-numbered"
>   https://github.com/barebox/barebox/commit/38ff8dad11

Another case of removing a required property.

> * "ARM: dts: i.MX8MP: snps,dis-u2-freeclk-exists-quirk"
>   https://github.com/barebox/barebox/commit/db01bf84cf

Should have kept the existing property and made the new property
incremental meaning on top of it. Yet another reason for me to dislike
the dozens of quirk properties we have on this binding. We push back
on them a lot more now and instead push that quirks are implied from
SoC specific compatible strings unless they are board specific.

I don't think the binding comparison I'm working on could catch this.
For this we probably need to compare DTBs looking for removed
properties. I think dtx_diff already can do that.

> * "clk: imx8mp: add USB suspend clock"
>   https://github.com/barebox/barebox/commit/d86bbaed71

Changing the number of entries is something I check.

> * "ARM: i.MX8MN: assume USBOTG power domains to be powered"
>   https://github.com/barebox/barebox/commit/7b62fbc632

That's the problem of new providers added. I don't know about barebox
design, but I think you'd be able to handle that better than the Linux
kernel can. The biggest problem in Linux is we never know when all
drivers have been loaded or dependencies have probed. A module could
be loaded a week after boot to provide a dependency. I'd think
bootloaders generally don't have that problem.

> All of these bugs would have broken a newer Linux kernel being booted with an 
> old
> device tree.

I'm not sure we can conclude that. Depends if Linux handles the old binding.

> In practice, they didn't because normally barebox-built device trees are
> used for barebox and Linux-built device trees are shipped along 

Re: OS provided DT proposal

2024-05-02 Thread Rob Herring
On Wed, May 1, 2024 at 4:18 PM Humphreys, Jonathan  wrote:
>
>
>
> Hi all.  Several EBBR meetings ago, I introduced the need for allowing OS 
> provided device trees [1].  Please find below the proposal I am delinquent on 
> sending.
>
>
> Hopefully, we can discuss this in the next meeting.
>
>
> Thanks
>
> Jon
>
>
> [1] https://github.com/ARM-software/ebbr/wiki/EBBR-Notes-2024.02.12
>
>
>
> Problem statement:
> ==
>
> Device trees are in theory a pure description of the hardware, and since the 
> hardware
> doesn't change, the device tree describing the hardware likewise never 
> changes.
> With this, a device tree could then be burned into the hardware's ROM to be
> queried by software for hardware discovery. In practice, though, device trees
> evolve over time. They evolve for many reasons, including
> - support for previously unsupported hardware
> - device driver improvements that require additional hardware information
> - bug fixes

I really would like specific cases of these where compatibility is
broken highlighted. The tooling and reviewing to identify these cases
has gotten much better. I've been prototyping a tool which will
compare 2 versions of binding schemas and spit out incompatible
changes for example. Those aren't the only types of changes as you
point out, but if we can eliminate a whole class of issues I think the
situation would be much better.


> Linux's device tree source is maintained with the kernel source, and kernel 
> builds
> include building the device trees too. This ensures that the device tree
> matching the kernel's usage is always kept in sync. Often, embedded distros 
> will
> include the matching device tree blobs.
>
> The EBBR mandates that the device tree blob is provided by the firmware.
>
> Thus it is likely that the device tree provided by the firmware and given to 
> the
> operating system is not the matching device tree blob for that kernel. This 
> can
> cause hardware to be missing, buggy, or non-functional.
>
> Proposal:
> =
>
> A key goal of the EBBR is to define the contract between the firmware and the 
> OS
> so that the OS doesn't need to be built specifically for the hardware, and the
> firmware can boot any compliant OS. Thus, any solution that requires the OS to
> know specifics about the hardware beyond the EBBR contract would violate the
> EBBR goals. This precludes any solution where the OS, having the matching 
> DTBs,
> would pick the DTB, because this requires the OS to know what hardware it is
> being run on. Likewise, any solution where the firmware is aware of the OS
> matching DTBs would require the firmware to be aware of the particular OS it 
> is
> booting.

Nothing prevents an OS from using its own DTB already. The OS does
know what hardware it runs on because we tell it with the DTB.

> What can be known:
> - The firmware knows what board it is running on, and thus knows what device
>   tree to use. But it doesn't know what version of the device tree to use,
>   because it doesn't know what OS is being booted.
> - The OS knows what version of DTBs matches it's kernel, but does not know 
> which
>   specific device tree to use.
>
> This proposal then has the firmware choose the device tree by name, or some
> other identifier that can be used to match the device tree for the board [1]. 
> It
> has the OS-provided OS loader select the location of the matching versions of
> DTBs for it.
>
> The firmware would pass the device tree filename/id to the OS loader, instead 
> of
> the DTB itself.

If the firmware can't know which version of DTB, how can it know
whether to pass a DTB vs. an identifier? The OS might be perfectly
fine with firmware's DTB.

> The OS loader would determine the location of the matching DTBs
> based on the chosen OS to boot, load the matching DTB from that location, and
> pass to the kernel.
>
> Considerations:
> - often a DTB requires fixups. The EFI_DT_FIXUP_PROTOCOL could be utilized.
> - device tree overlays could be indicated with a scheme using the device tree 
> ID
>   passed to the OS loader
> - authenticating the DTB would be the responsibility of the OS distribution 
> and
>   handled in the same way as the kernel itself is authenticated. The OS is the
>   entity responsible for signing the DTB, as it should be.
>
> This proposal should be in addition to supporting the standard way of passing 
> in
> a firmware-provided DT, in cases where the OS doesn't provide or have a need 
> to
> provide a matching DT.

Agreed, but that contradicts what you said above unless you mean we
define 2 ways to operate with some platforms working one standard way
and other platforms working the other standard way.

> [1] Rather than using the device tree source filename, to have more 
> flexibility,
> one can conceive an ID or compatible string that the OS could then scan the 
> DTBs
> to find a match.

I agree with Daniel that we should use the root node compatible for
this. We discussed this a while 

Re: [PATCH 00/21] Qualcomm generic board support

2023-12-12 Thread Rob Herring
On Mon, Dec 11, 2023 at 11:47 PM Sumit Garg  wrote:
>
> Hi Tom,
>
> On Sun, 10 Dec 2023 at 03:33, Tom Rini  wrote:
> >
> > On Mon, Dec 04, 2023 at 11:02:57AM +0530, Sumit Garg wrote:
> >
> > [snip]
> > > But currently u-boot doesn't have a proper way to validate those DTS
> > > against DT bindings (maintained in Linux kernel). Although there are
> > > Devicetree schema tools available here [2], there isn't a versioned
> > > release package of DT bindings which one should use to validate DTS
> > > files.
> >
> > I will have more / other things to say but I want to chime in here. That
> > U-Boot cannot validate the DTS files is a bug, not a feature. I would
> > very much appreciate if someone(s) with time and skills to do so would
> > re-sync us with the kernel Kbuild again so that we can both stay in sync
> > again and have the validation targets / functionality available here
> > too.
> >
>
> Agree, the Kbuild changes to add dtbs_check was the first thing I
> implemented after importing devicetree-rebasing repo in u-boot (see
> commit [1] for details). Usage with PR [2] included:
>
> While building any u-boot target, just add make target: "dtbs_check"
> and you will see the DT schema checks being performed. Steps:
>
> $ make _defconfig
> $ make -j`nproc` dtbs_check
>
> Currently there are a lot of incompatibility warnings I have seen for
> the platforms I built. This shows how much difficult it has been to
> keep DT in sync with upstream DT bindings.

The versions in the Linux tree generally still have lots of warnings
too. It's a mountain of warnings. The warnings get amplified when
there are N boards for 1 SoC. Some platforms are more active than
others to get rid of them. Newer platforms like Apple are warning free
(or nearly so). If you want an overview of the state of platforms, I
have a CI job[1] doing just that. Look at platform-warnings.log in the
artifacts. It does some deduplicating of the warnings. Linux-next and
Linus' master are built daily.

Fixing the warnings alone will not solve potential incompatibility
issues. The schemas can and do change (and in turn the dts files). We
try to catch that in review, but the rule is that the platform must be
okay with the ABI change (recent mistake, early stages, limited users,
etc.) and the commit message must spell out 'this is an ABI change'.
That's all manual and not easily identifiable. I'm working on a
tool[2] to compare versions of the schema to identify some ABI
changes.  Though it is limited by thinking of what schema changes are
ABI changes and my ability to write python code to parse those cases.
Right now it looks for new required properties, removed properties,
and changed number of entries. I'm interested in any ideas for other
checks.

Rob

[1] https://gitlab.com/robherring/linux-dt/-/jobs
[2] https://github.com/robherring/dt-schema/tree/abi-check
___
boot-architecture mailing list -- boot-architecture@lists.linaro.org
To unsubscribe send an email to boot-architecture-le...@lists.linaro.org


Re: [PATCH 00/21] Qualcomm generic board support

2023-12-07 Thread Rob Herring
On Thu, Dec 7, 2023 at 2:08 AM ff  wrote:
>
>
>
> > Le 6 déc. 2023 à 21:42, Rob Herring  a écrit :
> >
> > On Tue, Dec 5, 2023 at 11:05 PM Sumit Garg  wrote:
> >>
> >>> On Tue, 5 Dec 2023 at 15:39, Krzysztof Kozlowski
> >>>  wrote:
> >>>
> >>> On 05/12/2023 10:45, Sumit Garg wrote:
> >>>> + U-boot custodians list
> >>>>
> >>>> On Tue, 5 Dec 2023 at 12:58, Krzysztof Kozlowski
> >>>>  wrote:
> >>>>>
> >>>>> On 05/12/2023 08:13, Sumit Garg wrote:
> >>>>>>>> @DT bindings maintainers,
> >>>>>>>>
> >>>>>>>> Given the ease of maintenance of DT bindings within Linux kernel
> >>>>>>>> source tree, I don't have a specific objection there. But can we ease
> >>>>>>>> DTS testing for firmware/bootloader projects by providing a versioned
> >>>>>>>> release package for DT bindings? Or if someone else has a better idea
> >>>>>>>> here please feel free to chime in.
> >>>>>>>
> >>>>>>> This doesn't work for you?:
> >>>>>>>
> >>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/
> >>>>>>
> >>>>>> Thanks, this is certainly a good step which I wasn't aware of. Further
> >>>>>> simplification can be done to decouple devicetree source files from DT
> >>>>>> bindings.
> >>>>>
> >>>>> Why?
> >>>>
> >>>> I suppose you are already aware that Linux DTS files are a subset of
> >>>> what could be supported by devicetree schemas. There can be
> >>>> firmware/bootloader specific properties (one example being [1]) which
> >>>> Linux kernel can simply ignore. Will you be willing to add all of
> >>>> those DT properties to Linux DTS files and maintain them?
> >>>
> >>> We already added them and we already maintain them. DTS describes the
> >>> hardware, not the OS-subset of the hardware.
> >>
> >> Let look at some numbers if your statement is justified or not for the
> >> example I gave:
> >>
> >> u-boot$ git grep -nr bootph-* arch/arm* | wc -l
> >> 4079
> >>
> >> linux$ git grep -nr bootph-* arch/arm* | wc -l
> >> 267
> >
> > I have no control over whether anyone has submitted the other 3812 
> > instances.
> >
> >> It looks like there is always going to be a catch up game regarding DT
> >> properties which either Linux kernel or u-boot or any other
> >> firmware/bootloader project don't care about.
> >
> > As long as dts files in u-boot are manually sync'ed, yes. That is the
> > problem and it doesn't matter if we have a standalone repository or
> > not.
> >
> > If you want to move in that direction, start automating what u-boot
> > imports. You need to do that for bindings if you want to run
> > validation, so why not dts files too?
> >
> >>>> However, DT bindings are something which should be common, the
> >>>> hardware description of a device should be universal. IMO, splitting
> >>>
> >>> Both DT bindings and DTS should be common. I don't see the difference.
> >>
> >> If we really care about DTS to be common then the contribution model
> >> has to change where there is a single repo hosting DT bindings and
> >> DTS. All other projects whether it is Linux kernel or u-boot or any
> >> other OS/firmware/bootloader are just consuming DTS files from that
> >> single repo.
> >
> > Really, only the kernel and u-boot matter. No, I don't mean I don't
> > care about other projects, but those are the 2 with the widest h/w
> > support by far and which have a major effort to sync copies of dts
> > files in both projects. The rest are just noise in terms of this
> > problem.
> >
> >> I suppose this is something that Linux DT maintainers
> >> have objected to in the past for ease of maintenance. I am not sure if
> >> you folks are willing to change that stance.
> >
> > The issue is no one steps up to help maintain such a repository while
> > there is lots of review and maintainer work on what goes into the
> > kernel tree. I'm happy to direct my binding review attention to
> > wherever the majority of the bindings go. 

Re: [PATCH 00/21] Qualcomm generic board support

2023-12-06 Thread Rob Herring
On Tue, Dec 5, 2023 at 11:05 PM Sumit Garg  wrote:
>
> On Tue, 5 Dec 2023 at 15:39, Krzysztof Kozlowski
>  wrote:
> >
> > On 05/12/2023 10:45, Sumit Garg wrote:
> > > + U-boot custodians list
> > >
> > > On Tue, 5 Dec 2023 at 12:58, Krzysztof Kozlowski
> > >  wrote:
> > >>
> > >> On 05/12/2023 08:13, Sumit Garg wrote:
> > > @DT bindings maintainers,
> > >
> > > Given the ease of maintenance of DT bindings within Linux kernel
> > > source tree, I don't have a specific objection there. But can we ease
> > > DTS testing for firmware/bootloader projects by providing a versioned
> > > release package for DT bindings? Or if someone else has a better idea
> > > here please feel free to chime in.
> > 
> >  This doesn't work for you?:
> > 
> >  https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/
> > >>>
> > >>> Thanks, this is certainly a good step which I wasn't aware of. Further
> > >>> simplification can be done to decouple devicetree source files from DT
> > >>> bindings.
> > >>
> > >> Why?
> > >
> > > I suppose you are already aware that Linux DTS files are a subset of
> > > what could be supported by devicetree schemas. There can be
> > > firmware/bootloader specific properties (one example being [1]) which
> > > Linux kernel can simply ignore. Will you be willing to add all of
> > > those DT properties to Linux DTS files and maintain them?
> >
> > We already added them and we already maintain them. DTS describes the
> > hardware, not the OS-subset of the hardware.
>
> Let look at some numbers if your statement is justified or not for the
> example I gave:
>
> u-boot$ git grep -nr bootph-* arch/arm* | wc -l
> 4079
>
> linux$ git grep -nr bootph-* arch/arm* | wc -l
> 267

I have no control over whether anyone has submitted the other 3812 instances.

> It looks like there is always going to be a catch up game regarding DT
> properties which either Linux kernel or u-boot or any other
> firmware/bootloader project don't care about.

As long as dts files in u-boot are manually sync'ed, yes. That is the
problem and it doesn't matter if we have a standalone repository or
not.

If you want to move in that direction, start automating what u-boot
imports. You need to do that for bindings if you want to run
validation, so why not dts files too?

> > > However, DT bindings are something which should be common, the
> > > hardware description of a device should be universal. IMO, splitting
> >
> > Both DT bindings and DTS should be common. I don't see the difference.
>
> If we really care about DTS to be common then the contribution model
> has to change where there is a single repo hosting DT bindings and
> DTS. All other projects whether it is Linux kernel or u-boot or any
> other OS/firmware/bootloader are just consuming DTS files from that
> single repo.

Really, only the kernel and u-boot matter. No, I don't mean I don't
care about other projects, but those are the 2 with the widest h/w
support by far and which have a major effort to sync copies of dts
files in both projects. The rest are just noise in terms of this
problem.

> I suppose this is something that Linux DT maintainers
> have objected to in the past for ease of maintenance. I am not sure if
> you folks are willing to change that stance.

The issue is no one steps up to help maintain such a repository while
there is lots of review and maintainer work on what goes into the
kernel tree. I'm happy to direct my binding review attention to
wherever the majority of the bindings go. But the work on the DTS side
is mostly SoC tree maintainers and sub-maintainers.

Assume for a minute we have this standalone repo. What happens next?
We start with an empty repo and then merge and move platforms 1 by 1?
How many years will that take? What do we do with platforms no one is
interested in moving? Or do we start with devicetree-rebasing instead
(That was the plan at one time) and sync u-boot back to that?

All the work needed to get u-boot and kernel dts files in sync has
virtually no dependency on a standalone repo. If the dts files aren't
already kept in sync, someone has to figure the differences and
eliminate them. Maybe some platforms are in good shape, but it is
still a manual process. Fix that part, because a standalone repo does
nothing for you until you do.

> > > DT bindings alone would ease the compliance process for u-boot drivers
> > > in quite similar manner to Linux drivers.

There's no compliance of drivers really beyond checking if compatible
strings used by drivers have a schema.

Why is extracting the bindings out a problem? SystemReady has no issue
doing just that for its compliance test.

Rob
___
boot-architecture mailing list -- boot-architecture@lists.linaro.org
To unsubscribe send an email to boot-architecture-le...@lists.linaro.org


Re: [PATCH 00/21] Qualcomm generic board support

2023-12-04 Thread Rob Herring
On Sun, Dec 3, 2023 at 11:33 PM Sumit Garg  wrote:
>
> + Linux kernel DT bindings maintainers, EBBR ML
>
> On Thu, 30 Nov 2023 at 20:05, Tom Rini  wrote:
> >
> > On Thu, Nov 30, 2023 at 01:02:25PM +0530, Sumit Garg wrote:
> > > On Wed, 29 Nov 2023 at 22:06, Neil Armstrong  
> > > wrote:
> > > >
> > > > On 29/11/2023 16:34, Caleb Connolly wrote:
> > > > >
> > > > >
> > > > > On 23/11/2023 07:04, Sumit Garg wrote:
> > > > >> On Wed, 22 Nov 2023 at 21:34, Caleb Connolly 
> > > > >>  wrote:
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >>> On 22/11/2023 14:27, Tom Rini wrote:
> > > >  On Wed, Nov 22, 2023 at 07:44:09PM +0530, Sumit Garg wrote:
> > > > > On Wed, 22 Nov 2023 at 19:31, Tom Rini  wrote:
> > > > >>
> > > > >> On Wed, Nov 22, 2023 at 11:51:29AM +0530, Sumit Garg wrote:
> > > > >>> Hi Caleb,
> > > > >>>
> > > > >>> On Tue, 21 Nov 2023 at 22:39, Caleb Connolly 
> > > > >>>  wrote:
> > > > >> [snip]
> > > >  == DT loading ==
> > > > 
> > > >  Previously, boards used the FDT blob embedded into U-Boot (via
> > > >  OF_SEPARATE). However, most Qualcomm boards run U-Boot as a 
> > > >  secondary
> > > >  bootloader, so we can instead rely on the first-stage 
> > > >  bootloader to
> > > >  populate some useful FDT properties for us (notably the 
> > > >  /memory node and
> > > >  KASLR seed) and fetch the DTB that it provides. Combined with 
> > > >  the memory
> > > >  map changes above, this let's us entirely avoid configuring 
> > > >  the memory
> > > >  map explicitly.
> > > > >>>
> > > > >>> Since with this change, we don't need to embed FDT blob in the 
> > > > >>> u-boot
> > > > >>> binary, so I was thinking if we really need to import DTs from 
> > > > >>> Linux
> > > > >>> for different platforms and then play a catchup game?
> > > > >>>
> > > > >>> For now, yes.
> > > > >>
> > > > >> But why? Is there any value added by larger u-boot specific DT (most
> > > > >> of the nodes being unused by u-boot) than what currently u-boot
> > > > >> supports? The more important part is to get alignment with Linux DT
> > > > >> bindings. If you need to have memory/reserved-memory nodes in u-boot
> > > > >> DT for generalization purposes then you should import those 
> > > > >> particular
> > > > >> nodes only.
> > > > >
> > > > > I've been thinking about and hacking on this for the last week or so,
> > > > > sorry for the delayed reply here.
> > > > >
> > > > > The value is in preventing any of the existing bindings from 
> > > > > regressing,
> > >
> > > That is actually best addressed in Linux by checking the DTS against
> > > yaml DT bindings. We don't have that testing available in u-boot and
> > > only depend on careful reviews.
> >
> > I would absolutely love for someone to make another attempt at updating
> > our kbuild infrastucture so that we can run the validation targets.
> >
>
> Given that EBBR requires [1] the platform (firmware/bootloader) and
> not OS to supply the devicetree, it becomes evident that
> firmware/bootloaders import DTS from Linux kernel (where it is
> maintained).
>
> But currently u-boot doesn't have a proper way to validate those DTS
> against DT bindings (maintained in Linux kernel). Although there are
> Devicetree schema tools available here [2], there isn't a versioned
> release package of DT bindings which one should use to validate DTS
> files.
>
> @DT bindings maintainers,
>
> Given the ease of maintenance of DT bindings within Linux kernel
> source tree, I don't have a specific objection there. But can we ease
> DTS testing for firmware/bootloader projects by providing a versioned
> release package for DT bindings? Or if someone else has a better idea
> here please feel free to chime in.

This doesn't work for you?:

https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/

Note that I would like to revamp this repo to use some modern CI job,
use git_filter_repo python module rather than git-filter-branch, and
move to devicetree.org GH, but if projects aren't relying on this
repo, I'm not motivated to do that work.

Rob
___
boot-architecture mailing list -- boot-architecture@lists.linaro.org
To unsubscribe send an email to boot-architecture-le...@lists.linaro.org


Re: [DT] Chosen node documentation

2023-11-02 Thread Rob Herring
On Thu, Nov 2, 2023 at 4:05 AM ff  wrote:
>
> Hi,
>
> As I am working on Automotive Virtual Platform Speficition in the context of 
> SOAFEE and there is a reference to « chosen » standardization in the 
> document, I am verifying all external links.
>
> The file 
> https://elixir.bootlin.com/linux/v5.19.17/source/Documentation/devicetree/bindings/chosen.txt
>  has disappeared from the Doc tree since 6.0
>
> There are no references to kaslr-seed, stdout-path or 
> linux,usable-memory-range in the whole DT, including in the current 6.6 tree. 
> There is a brief mention (probably not normative) of « chosen » is 
> usage_model.rst but that’s all.
>
> Does anyone knows if that is intentional?

Checking the commit log for the file will answer your question:

$ git log -- Documentation/devicetree/bindings/chosen.txt
commit ad6c94de2ec453d966f71654cd7dd68cafd03dc3
Author: Jason A. Donenfeld 
Date:   Tue Jun 28 17:33:54 2022 +0200

dt-bindings: chosen: remove old .txt binding

chosen.txt has been replaced by a schema in dtschema[1] and is now out
of date as well. Remove it to avoid confusion.

[1] 
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/chosen.yaml

Link: 
https://lore.kernel.org/lkml/c8dddfe6-6385-ed34-e789-9f845c8a3...@linaro.org/
Link: 
https://lore.kernel.org/lkml/CAL_Jsq+uSdk9YNbUW35yjN3q8-3FDobrxHmBpy=4rkmcfnb...@mail.gmail.com/
Signed-off-by: Jason A. Donenfeld 
    [robh: Improve commmit msg]
Signed-off-by: Rob Herring 
Link: https://lore.kernel.org/r/20220628153354.870543-1-ja...@zx2c4.com
___
boot-architecture mailing list -- boot-architecture@lists.linaro.org
To unsubscribe send an email to boot-architecture-le...@lists.linaro.org


Re: [PATCH] schemas: Add schema for firmware logs

2023-02-06 Thread Rob Herring
+boot-architecture

On Mon, Feb 6, 2023 at 3:25 PM Simon Glass  wrote:
>
> Hi Rob,
>
> On Mon, 6 Feb 2023 at 10:15, Rob Herring  wrote:
> >
> > On Sat, Feb 4, 2023 at 6:04 AM Simon Glass  wrote:
> > >
> > > Hi Peter,
> > >
> > > On Sat, 4 Feb 2023 at 02:36, Peter Robinson  wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > Does it make sense to devise something that is compatible with the
> > > > kernel's pstore [1] mechanism?
> > >
> > > Possibly...can you please be a little more specific?
> >
> > Peter is talking about the same thing I suggested on IRC.
> >
> > pstore == ramoops
>
> Oh, I only looked at the DT binding as I thought that was what you
> were talking about on irc.

The binding is called ramoops as it's for the RAM backend for pstore.

My suggestion was either using/extending ramoops or following its
design as a reserved memory region. All you would need to extend the
ramoops binding is a new property to define the size of your data.

> For pstore, isn't the point that Linux wants to save stuff to allow
> debugging or collection on reboot? What does that have to do with
> console logs from firmware? That seems like a different thing. Or are
> you suggesting that we add a pstore driver into U-Boot? It is quite a
> lot of code, including compression, etc. It might be easier for Linux
> to write the data into pstore when it starts up?

Originally ramoops was just what you described. It has grown to
multiple backends and types of records (hence the rename to pstore).
If you just add a new subsection within the pstore region, then I
think the existing kernel infrastructure will support reading it from
userspace. Maybe new types have to be explicitly supported, IDK.

U-boot being able to read pstore wouldn't be a terrible feature to
have anyways if your boot crashes before anything else is up to get
the output. Note I'd guess the ram backend doesn't do compression as
supporting slightly corrupted ram is a feature which wouldn't work.


I think any new DT binding is premature and pstore/ramoops was just a
suggestion to consider. This needs wider consideration of how to
handle all the various (boot) firmware logs. I've added the
boot-architecture list for a bit more visibility.

Rob
___
boot-architecture mailing list -- boot-architecture@lists.linaro.org
To unsubscribe send an email to boot-architecture-le...@lists.linaro.org


Re: [RFC] Proposed location to host the firmware handoff specification.

2022-07-05 Thread Rob Herring
On Tue, Jul 5, 2022 at 10:37 AM Simon Glass  wrote:
>
> Hi Rob,
>
> On Tue, 5 Jul 2022 at 09:24, Rob Herring  wrote:
> >
> > On Thu, Jun 30, 2022 at 3:24 AM Simon Glass  wrote:
> > >
> > > Hi Jose,
> > >
> > > I don't think this is correct. TF-A is a project that aims to replace
> > > U-Boot SPL (and perhaps other components) with more closed firmware,
> > > e.g. the permissive license.
> > >
> > > This spec needs to be in a neutral place, not captive of one project.
> > >
> > > Given its close relationship to device tree, I suggest 
> > > github.com/devicetree-org
> >
> > The only relationship to DT I see is DT is a payload as is ACPI. So I
> > don't think dt.org is the right place.
>
> Actually I was about to email you about this. Here's how I see it.
>
> DT is a base structure to allow self-describing data to be stored.
> This is in contrast with ACPI where there is just a header, but it is
> not possible to read the data without specific parsing code for the
> particular table types. Let's ignore ACPI for this discussion.
>
> Unfortunately DT has an overhead and is too expensive for early
> firmware use. It takes 3-4KB of code for libfdt as well as extra code
> and data to read properties, etc.
>
> Transfer List aims to bridge the gap, allowing simple C structures to
> be put into a tagged data structure. The intention is that anything
> more complex than that would use DT.
>
> So there is at least some relationship: simple stuff = Transfer list,
> complex stuff = DT.

That's a stretch IMO. Perhaps if this was a new output (DTB) format
for easier parsing, I'd agree. It's related to DT only as much as any
other data passed between 2 boot components (remember ATAGS?).

> The Transfer List spec specifies the data format for each entry type
> (the analog to the schema). The DT provides the format and schema for
> more complicated stuff.
>
> We could perhaps put it in an entirely separate repo, but to me there
> is a relationship with DT.

It seems to me that TF is the main driver and user of this, so I don't
see the issue with them hosting it at least to start as long as
there's not barriers to contributions. It's just a namespace like
devicetree-org. Personally, I'd be more concerned on what the source
format is (I assume the plan is not to commit PDFs) and what the
output generation is. GH has a lot of nice features to support that
which we've used for the DT spec and EBBR.

I'm not saying no to devicetree-org either. If the consensus is to put
it there, I really don't care so much as it takes less time to create
a new repo there than to write this email.

Rob
___
boot-architecture mailing list -- boot-architecture@lists.linaro.org
To unsubscribe send an email to boot-architecture-le...@lists.linaro.org


Re: [RFC] Proposed location to host the firmware handoff specification.

2022-07-05 Thread Rob Herring
On Thu, Jun 30, 2022 at 3:24 AM Simon Glass  wrote:
>
> Hi Jose,
>
> I don't think this is correct. TF-A is a project that aims to replace
> U-Boot SPL (and perhaps other components) with more closed firmware,
> e.g. the permissive license.
>
> This spec needs to be in a neutral place, not captive of one project.
>
> Given its close relationship to device tree, I suggest 
> github.com/devicetree-org

The only relationship to DT I see is DT is a payload as is ACPI. So I
don't think dt.org is the right place.

Rob
___
boot-architecture mailing list -- boot-architecture@lists.linaro.org
To unsubscribe send an email to boot-architecture-le...@lists.linaro.org


Re: [EXT] Re: Device Tree call: Define Secure Data Path reserved memory

2022-03-21 Thread Rob Herring
On Mon, Mar 21, 2022 at 7:37 AM Bill Mills  wrote:
>
> Everyone in the to: list,
>
> Are you all free today at 3PM UTC (11 AM US East)?

Yes, I will be there.

Rob

>
> Oliver,
>
> See the DTE home page for a description of the meeting, the Zoom link
> and a google ics file.
>
> However I just added you to the invite going forward.  Just delete after
> we are done if you do not want to continue.
>
> The meeting is tied to UK time so right now appears 1 hour later than
> normal as US has done DST and the UK will not until next week.
>
> Thanks,
> Bill
>
> On 3/7/22 11:00 AM, Olivier Masse wrote:
> > Hi Bill,
> >
> > Could you send us some pointers to join your call ?
> >
> > Sorry for this late request.
> >
> > Olivier
> >
> > On mar., 2022-02-22 at 15:00 -0500, Bill Mills wrote:
> >> Caution: EXT Email
> >>
> >> Oliver,
> >>
> >> On 2/22/22 7:36 AM, Olivier Masse wrote:
> >>> Hi All,
> >>>
> >>> Could we postpone to the next call ?
> >>>
> >>
> >> Yes we can discuss on March 7.
> >>
> >> However I think you need to define what you want to discuss.
> >>
> >> First of, there is nothing in your suggested DTS that tells Linux (or
> >> other  OS) that it should not map the memory area.  It looks to me as
> >> you should include the "no-map" property in your node.
> >>
> >> How will the Linux video/graphics drivers refer to this node?  I
> >> presume
> >> by phandle, right?
> >>
> >> Are you setting any suggested naming convention for the node name
> >> (after
> >> you fix the node name to include @address)?
> >>
> >> You are defining a compatible string in the node.  The current
> >> suggestion is not in the spec.  It could be "optee,sdp" if you get
> >> optee
> >> stakeholders to agree.  What should be the definition of "optee,sdp".
> >> I presume it means secure data path but that could mean a ton of
> >> things.
> >> Will sdp always mean DRM protected media playback or could it include
> >> more?
> >>
> >> Thanks,
> >> Bill
> >>
> >>> BR / Olivier
> >>>
> >>> On mar., 2022-02-15 at 13:46 -0600, Rob Herring wrote:
> >>>> Caution: EXT Email
> >>>>
> >>>> On Fri, Feb 11, 2022 at 8:21 AM Bill Mills  >>>>>
> >>>> wrote:
> >>>>>
> >>>>> Rob,
> >>>>>
> >>>>> Can you confirm for the DT call on Feb 21?
> >>>>
> >>>> I'm on holiday on the 21st.
> >>>>
> >>>>>
> >>>>> Oliver,
> >>>>>
> >>>>> On 2/11/22 4:54 AM, Olivier Masse wrote:
> >>>>>> Hi Bill,
> >>>>>>
> >>>>>> NXP had a discussion with Linaro about this optee os issue:
> >>>>>>
> >>>
> >>>
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FOP-TEE%2Foptee_os%2Fissues%2F5133data=04%7C01%7Colivier.masse%40nxp.com%7C64218f2199dd4d71f84008d9f63e093a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637811568813890462%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=Enk98Vs%2B1dDeoXceyiPnNhkSRksZ9YGvFN84DAQRnCI%3Dreserved=0
> >>>>>>
> >>>>>> Which is implemented by this first draft here:
> >>>>>>
> >>>
> >>>
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FOP-TEE%2Foptee_os%2Fpull%2F5149data=04%7C01%7Colivier.masse%40nxp.com%7C64218f2199dd4d71f84008d9f63e093a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637811568813890462%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=UYLsOYcqoRKyDWkAWjXeYD5l7IAYO%2Bx8YVMs8538n78%3Dreserved=0
> >>>>>>
> >>>>>> Could we be part of the next Device Tree call to discuss
> >>>>>> about
> >>>>>> adding
> >>>>>> a reserved memory in optee os embedded DT ?
> >>>>>>
> >>>>>
> >>>>> Thanks for bringing it up.
> >>>>> I have skimmed the PR threads and the discussion seems to be:
> >>>>> 1) OP-TEE internal issues
> >>>>> 2) DT standards questions and issues
> >>>>

Re: Device Tree call: Define Secure Data Path reserved memory

2022-02-15 Thread Rob Herring
On Fri, Feb 11, 2022 at 8:21 AM Bill Mills  wrote:
>
> Rob,
>
> Can you confirm for the DT call on Feb 21?

I'm on holiday on the 21st.

>
> Oliver,
>
> On 2/11/22 4:54 AM, Olivier Masse wrote:
> > Hi Bill,
> >
> > NXP had a discussion with Linaro about this optee os issue:
> > https://github.com/OP-TEE/optee_os/issues/5133
> >
> > Which is implemented by this first draft here:
> > https://github.com/OP-TEE/optee_os/pull/5149
> >
> > Could we be part of the next Device Tree call to discuss about adding
> > a reserved memory in optee os embedded DT ?
> >
>
> Thanks for bringing it up.
> I have skimmed the PR threads and the discussion seems to be:
> 1) OP-TEE internal issues
> 2) DT standards questions and issues
>
> For the DT call we need to focus on #2 above.
> We will definitely need Rob for this discussion so we need to do it when
> he can join.  (If we don't resolve the question in this email thread
> before then.)
>
> Context for all:
>
> SDP here is related to DRM protected playback of media streams.
>
> Jens comment in PR:
>  > So far we have managed to avoid defining our own bindings in OP-TEE,
>  > instead we've been able to reuse already established bindings. With
>  > this you're proposing something new. I'm not sure of the best way of
>  > doing such a thing. Are we sure there is nothing to reuse?
>  > If not: How should it be reviewed? Who should review it?
>
> DTS in PR:
> /*
>   * Copyright (c) 2021, NXP. All rights reserved.
>   *
>   * SPDX-License-Identifier: BSD-3-Clause
>   */
>
> /dts-v1/;
>
> / {
> #address-cells = <1>;
> #size-cells = <0>;
>
> reserved-memory {
> #address-cells = <1>;
> #size-cells = <1>;
>
> sdp_mem {
> compatible = "optee-sdp";
> reg = <0x3E80 0x0040>;
> };
> };
> };
>
> However it was modified after that.
>
> Oliver: please reply to this thread (on list please) with the final DTS
> you are proposing. fix DT conventions and what are you doing with the
> no-map property if anything.
>
> Is the memory above meant to be visible to NS world AND S world?  Or is
> just for secure world.
>
> Thanks,
> Bill
>
> > Best regards,
> > Olivier Masse
>
> --
> Bill Mills
> Principal Technical Consultant, Linaro
> +1-240-643-0836
> TZ: US Eastern
> Work Schedule:  Tues/Wed/Thur
> ___
> boot-architecture mailing list -- boot-architecture@lists.linaro.org
> To unsubscribe send an email to boot-architecture-le...@lists.linaro.org
___
boot-architecture mailing list -- boot-architecture@lists.linaro.org
To unsubscribe send an email to boot-architecture-le...@lists.linaro.org


Re: [v5.4 stable] arm: stm32: Regression observed on "no-map" reserved memory region

2021-04-20 Thread Rob Herring
On Tue, Apr 20, 2021 at 11:10 AM Ard Biesheuvel  wrote:
>
> On Tue, 20 Apr 2021 at 17:54, Rob Herring  wrote:
> >
> > On Tue, Apr 20, 2021 at 10:12 AM Alexandre TORGUE
> >  wrote:
> > >
> > >
> > >
> > > On 4/20/21 4:45 PM, Rob Herring wrote:
> > > > On Tue, Apr 20, 2021 at 9:03 AM Alexandre TORGUE
> > > >  wrote:
> > > >>
> > > >> Hi,
> > > >
> > > > Greg or Sasha won't know what to do with this. Not sure who follows
> > > > the stable list either. Quentin sent the patch, but is not the author.
> > > > Given the patch in question is about consistency between EFI memory
> > > > map boot and DT memory map boot, copying EFI knowledgeable folks would
> > > > help (Ard B for starters).
> > >
> > > Ok thanks for the tips. I add Ard in the loop.
> >
> > Sigh. If it was only Ard I was suggesting I would have done that
> > myself. Now everyone on the patch in question and relevant lists are
> > Cc'ed.
> >
>
> Thanks for the cc.
>
> > >
> > > Ard, let me know if other people have to be directly added or if I have
> > > to resend to another mailing list.
> > >
> > > thanks
> > > alex
> > >
> > > >
> > > >>
> > > >> Since v5.4.102 I observe a regression on stm32mp1 platform: "no-map"
> > > >> reserved-memory regions are no more "reserved" and make part of the
> > > >> kernel System RAM. This causes allocation failure for devices which try
> > > >> to take a reserved-memory region.
> > > >>
> > > >> It has been introduced by the following path:
> > > >>
> > > >> "fdt: Properly handle "no-map" field in the memory region
> > > >> [ Upstream commit 86588296acbfb1591e92ba60221e95677ecadb43 ]"
> > > >> which replace memblock_remove by memblock_mark_nomap in no-map case.
> > > >>
>
> Why was this backported? It doesn't look like a bugfix to me.

Probably because of commit 8a5a75e5e9e5 ("of/fdt: Make sure no-map
does not remove already reserved regions") which was in the same
series.

'Properly handle' implies before it was 'improperly handled', so
sounds like a fix.

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [v5.4 stable] arm: stm32: Regression observed on "no-map" reserved memory region

2021-04-20 Thread Rob Herring
On Tue, Apr 20, 2021 at 10:12 AM Alexandre TORGUE
 wrote:
>
>
>
> On 4/20/21 4:45 PM, Rob Herring wrote:
> > On Tue, Apr 20, 2021 at 9:03 AM Alexandre TORGUE
> >  wrote:
> >>
> >> Hi,
> >
> > Greg or Sasha won't know what to do with this. Not sure who follows
> > the stable list either. Quentin sent the patch, but is not the author.
> > Given the patch in question is about consistency between EFI memory
> > map boot and DT memory map boot, copying EFI knowledgeable folks would
> > help (Ard B for starters).
>
> Ok thanks for the tips. I add Ard in the loop.

Sigh. If it was only Ard I was suggesting I would have done that
myself. Now everyone on the patch in question and relevant lists are
Cc'ed.

>
> Ard, let me know if other people have to be directly added or if I have
> to resend to another mailing list.
>
> thanks
> alex
>
> >
> >>
> >> Since v5.4.102 I observe a regression on stm32mp1 platform: "no-map"
> >> reserved-memory regions are no more "reserved" and make part of the
> >> kernel System RAM. This causes allocation failure for devices which try
> >> to take a reserved-memory region.
> >>
> >> It has been introduced by the following path:
> >>
> >> "fdt: Properly handle "no-map" field in the memory region
> >> [ Upstream commit 86588296acbfb1591e92ba60221e95677ecadb43 ]"
> >> which replace memblock_remove by memblock_mark_nomap in no-map case.
> >>
> >> Reverting this patch it's fine.
> >>
> >> I add part of my DT (something is maybe wrong inside):
> >>
> >> memory@c000 {
> >>  reg = <0xc000 0x2000>;
> >> };
> >>
> >> reserved-memory {
> >>  #address-cells = <1>;
> >>  #size-cells = <1>;
> >>  ranges;
> >>
> >>  gpu_reserved: gpu@d400 {
> >>  reg = <0xd400 0x400>;
> >>  no-map;
> >>  };
> >> };
> >>
> >> Sorry if this issue has already been raised and discussed.
> >>
> >> Thanks
> >> alex
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: DT conformance and "reg" tuples

2021-01-04 Thread Rob Herring
On Thu, Dec 24, 2020 at 3:09 AM François Ozog  wrote:
>
> Hi,
>
> As I am thinking about conformance testing for SystemReady and Trusted
> Substrate, I'd like to get your feedback on the following.
>
> There are 7 values in the reg entry of interrupt-controller@21 from the
> below DT. This corresponds to 3 valid {address,size} plus a single
> {address}.

dtc should reject this checking that the size of 'reg' is a multiple
of #address-cells plus #size-cells. Note it also checks pretty much
all common bindings using '#*-cells' style.

Furthermore, dt-schema checks for 3 entries assuming that's what the
schema for "arm,gic-400" says.

> The spec does not state anything on incomplete {address,size} pairs... I
> understand that #size-cell can be zero, indicating that the reg will
> contain only {address} "tuples" and not {address,size} tuples. But that
> should be for all reg tuples, not just one.

The simple-bus schema will reject #size-cell being anything other than 1 or 2.

>
> In this case, I assume the driver will get what it wants, but from a
> certification perspective:
>
>- I would reject this DT.
>- I would document proper tuple forming in the spec (no incomplete pairs)
>
>
> Last, I would also add some "notes" in the spec about where to get the
> "#*-cells" for the reg property of a device. If you think "hardware" it is
> obvious that the information must be retrieved from the immediate parent
> and "inheritance" does not make sense. But as I Googled the topic, I have
> seen a number of discussions and wrong patches around that. So I would add
> a non normative text (properly identified as such) to describe that in the
> spec.

Patches welcome. :)

>
> Thank you for your help
>
> Cheers
>
> FF
> config-space@f000 {
> #address-cells = <0x01>;
> #size-cells = <0x01>;
> compatible = "simple-bus";
> ranges = <0x00 0x00 0xf000 0x100>;
>
> interrupt-controller@21 {
>compatible = "arm,gic-400";
>#interrupt-cells = <0x03>;
>#address-cells = <0x01>;
>#size-cells = <0x01>;
>ranges;
>interrupt-controller;
>interrupts = <0x01 0x09 0xf04>;
>reg = <0x21 0x1 0x22 0x2 0x24 0 0x2>;
>phandle = <0x01>;
>
>v2m@28 {
>compatible = "arm,gic-v2m-frame";
>msi-controller;
>reg = <0x28 0x1000>;
>arm,msi-base-spi = <0xa0>;
>arm,msi-num-spis = <0x20>;
>phandle = <0x03>;
>};
>
>
> --
> François-Frédéric Ozog | *Director Linaro Edge & Fog Computing Group*
> T: +33.67221.6485
> francois.o...@linaro.org | Skype: ffozog
> ___
> boot-architecture mailing list
> boot-architecture@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/boot-architecture
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: DT based PSA description of hardware partition

2020-12-21 Thread Rob Herring
On Fri, Dec 18, 2020 at 9:43 PM François Ozog  wrote:
>
> Hi
>
> I assume this needs to be analyzed from System Device Tree perspective:
> https://trustedfirmware-a.readthedocs.io/en/latest/components/psa-ffa-manifest-binding.html

That's not what we're reviewing upstream[1].

Rob

[1] 
https://lore.kernel.org/linux-devicetree/20201204121137.2966778-1-sudeep.ho...@arm.com/

>
> And this is to be included in the DT Technical Report.
>
> Cheers
>
> FF
>
> --
> François-Frédéric Ozog | *Director Linaro Edge & Fog Computing Group*
> T: +33.67221.6485
> francois.o...@linaro.org | Skype: ffozog
> ___
> boot-architecture mailing list
> boot-architecture@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/boot-architecture
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: Sense of soc bus? (was: [PATCH] base: soc: Export soc_device_to_device() helper)

2019-11-14 Thread Rob Herring
On Tue, Nov 12, 2019 at 4:47 AM Andreas Färber  wrote:
>
> Am 12.11.19 um 08:29 schrieb Uwe Kleine-König:
> > On Tue, Nov 12, 2019 at 06:23:47AM +0100, Greg Kroah-Hartman wrote:
> >> On Mon, Nov 11, 2019 at 09:10:41PM +0100, Andreas Färber wrote:
> >>> Am 11.11.19 um 07:40 schrieb Greg Kroah-Hartman:
>  On Mon, Nov 11, 2019 at 06:42:05AM +0100, Andreas Färber wrote:
> > Am 11.11.19 um 06:27 schrieb Greg Kroah-Hartman:
> >> On Mon, Nov 11, 2019 at 05:56:09AM +0100, Andreas Färber wrote:
> >>> Use of soc_device_to_device() in driver modules causes a build 
> >>> failure.
> >>> Given that the helper is nicely documented in include/linux/sys_soc.h,
> >>> let's export it as GPL symbol.
> >>
> >> I thought we were fixing the soc drivers to not need this.  What
> >> happened to that effort?  I thought I had patches in my tree (or
> >> someone's tree) that did some of this work already, such that this
> >> symbol isn't needed anymore.
> >
> > I do still see this function used in next-20191108 in drivers/soc/.
> >
> > I'll be happy to adjust my RFC driver if someone points me to how!
> 
>  Look at c31e73121f4c ("base: soc: Handle custom soc information sysfs
>  entries") for how you can just use the default attributes for the soc to
>  create the needed sysfs files, instead of having to do it "by hand"
>  which is racy and incorrect.
> >>>
> >>> Unrelated.
> >>>
> > Given the current struct layout, a type cast might work (but ugly).
> > Or if we stay with my current RFC driver design, we could use the
> > platform_device instead of the soc_device (which would clutter the
> > screen more than "soc soc0:") or resort to pr_info() w/o device.
> 
>  Ick, no, don't cast blindly.  What do you need the pointer for?  Is this
>  for in-tree code?
> >>>
> >>> No, an RFC patchset: https://patchwork.kernel.org/cover/11224261/
> >>>
> >>> As I indicated above, I used it for a dev_info(), which I can easily
> >>> avoid by using pr_info() instead:
> >>>
> >>> diff --git a/drivers/soc/realtek/chip.c b/drivers/soc/realtek/chip.c
> >>> index e5078c6731fd..f9380e831659 100644
> >>> --- a/drivers/soc/realtek/chip.c
> >>> +++ b/drivers/soc/realtek/chip.c
> >>> @@ -178,8 +178,7 @@ static int rtd_soc_probe(struct platform_device *pdev)
> >>>
> >>> platform_set_drvdata(pdev, soc_dev);
> >>>
> >>> -   dev_info(soc_device_to_device(soc_dev),
> >>> -   "%s %s (0x%08x) rev %s (0x%08x) detected\n",
> >>> +   pr_info("%s %s (0x%08x) rev %s (0x%08x) detected\n",
> >>> soc_dev_attr->family, soc_dev_attr->soc_id, chip_id,
> >>> soc_dev_attr->revision, chip_rev);
> >>
> >> First off, the driver should not be spitting out noise for when all goes
> >> well like this :)
> >
> > I didn't follow the discussion closely, but I think I want to object
> > here a bit. While I agree that each driver emitting some stuff to the
> > log buffer is hardly helpful, seeing the exact SoC details is indeed
> > useful at times. With my Debian kernel team member hat on, I'd say
> > keep this information. This way the SoC details make it into kernel bug
> > reports without effort on our side.
>
> Seconded. For example, RTD1295 will support LSADC only from revision B00
> on (and it's not the first time I'm seeing such things in the industry).
> So if a user complains, it will be helpful to see that information.
>
> Referencing your Amlogic review, with all due respect for its authors,
> the common framework here just lets that information evaporate into the
> deeps of sysfs. People who know can check /sys/bus/soc/devices/soc0, but
> ordinary users will at most upload dmesg output to a Bugzilla ticket.
>
> And it was precisely info-level boot output from the Amlogic GX driver
> that made me aware of this common framework and inspired me to later
> contribute such a driver elsewhere myself. That's not a bad effect. ;)
>
> So if anything, we should standardize that output and move it into
> soc_device_register(): "  [rev ] detected"
> with suitable NULL checks? (what I did above for Realtek, minus hex)
>
> The info level seems correct to me - it allows people to use a different
> log_level if they only care about warnings or errors on the console;
> it's not debug info for that driver, except in my case the accompanying
> hex numbers that I'd be happy to drop in favor of standardization.
>
> Another aspect here is that the overall amount of soc_device_register()
> users is just an estimated one third above the analysis shared before.
> In particular server platforms, be it arm64 or x86_64, that potentially
> have more than one SoC integrated in a multi-socket configuration, don't
> feed into this soc bus at all! Therefore my comment that
> dev_info()-printed "soc soc0:" is kind of useless if there's only one
> SoC on those boards. I'm not aware of any tool or a more common file
> aggregating this 

Re: [PATCH] arm64: add support for rng-seed

2019-05-14 Thread Rob Herring
On Wed, May 8, 2019 at 10:06 AM Hsin-Yi Wang  wrote:
>
> On Wed, May 8, 2019 at 10:04 PM Rob Herring  wrote:
> >
> > On Tue, May 7, 2019 at 11:08 PM Hsin-Yi Wang  wrote:
> > >
> > > On Wed, May 8, 2019 at 3:47 AM Rob Herring  wrote:
> > > >
> > > > +boot-architecture list as there was some discussion about this IIRC.
> > > >
> > > > On Mon, May 6, 2019 at 11:54 PM Hsin-Yi Wang  
> > > > wrote:
> > > > >
> > > > > Introducing a chosen node, rng-seed, which is an 64 bytes entropy
> > > > > that can be passed to kernel called very early to increase device
> > > > > randomness. Bootloader should provide this entropy and the value is
> > > > > read from /chosen/rng-seed in DT.
> > > > >
> > > > > Signed-off-by: Hsin-Yi Wang 
> > > > >
> > > > > ---
> > > > >  Documentation/devicetree/bindings/chosen.txt | 14 +
> > > >
> > > > Actually, this file has been converted to json-schema and lives
> > > > here[1]. I need to remove this one (or leave it with a reference to
> > > > the new one).
> > > >
> > > > >  arch/arm64/kernel/setup.c|  2 ++
> > > > >  drivers/of/fdt.c | 33 
> > > > > 
> > > > >  include/linux/of_fdt.h   |  1 +
> > > > >  4 files changed, 50 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/chosen.txt 
> > > > > b/Documentation/devicetree/bindings/chosen.txt
> > > > > index 45e79172a646..bfd360691650 100644
> > > > > --- a/Documentation/devicetree/bindings/chosen.txt
> > > > > +++ b/Documentation/devicetree/bindings/chosen.txt
> > > > > @@ -28,6 +28,20 @@ mode) when EFI_RNG_PROTOCOL is supported, it will 
> > > > > be overwritten by
> > > > >  the Linux EFI stub (which will populate the property itself, using
> > > > >  EFI_RNG_PROTOCOL).
> > > > >
> > > > > +rng-seed
> > > > > +---
> > > > > +
> > > > > +This property served as an entropy to add device randomness. It is 
> > > > > parsed
> > > > > +as a 64 byte value, e.g.
> > > >
> > > > Why only 64-bytes?
> > > We can also not specify size and read what bootloader can provide.
> > > >
> > > > > +
> > > > > +/ {
> > > > > +   chosen {
> > > > > +   rng-seed = <0x31951b3c 0xc9fab3a5 0xffdf1660 ...>
> > > > > +   };
> > > > > +};
> > > > > +
> > > > > +This random value should be provided by bootloader.
> > > > > +
> > > > >  stdout-path
> > > > >  ---
> > > > >
> > > > > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> > > > > index 413d566405d1..ade4261516dd 100644
> > > > > --- a/arch/arm64/kernel/setup.c
> > > > > +++ b/arch/arm64/kernel/setup.c
> > > > > @@ -292,6 +292,8 @@ void __init setup_arch(char **cmdline_p)
> > > > > early_fixmap_init();
> > > > > early_ioremap_init();
> > > > >
> > > > > +   early_init_dt_rng_seed(__fdt_pointer);
> > > > > +
> > > >
> > > > I'm trying to reduce or eliminate all these early_init_dt_* calls.
> > > >
> > > > Why is this arch specific and why can't this be done after
> > > > unflattening? It doesn't look like add_device_randomness() needs
> > > > anything early.
> > > Currently unflattening is called after setup_machine_fdt(), which
> > > called fixmap_remap_fdt() //__fixmap_remap_fdt(dt_phys, ,
> > > PAGE_KERNEL_RO), and we can't modify DT after that since it's read
> > > only. But we need to clear (eg. write 0 to it) the rng-seed after
> > > reading from DT.
> >
> > Why do you need to clear it? That wasn't necessary for kaslr-seed.
> I think it's for security purpose. If we know the random seed, it's
> more likely we can predict randomness.
> Currently on arm64, kaslr-seed will be wiped out (in
> arch/arm64/kernel/kaslr.c#get_kaslr_seed(), it's set to 0) so we can't
> read from sysfs (eg. /sys/firmware/devicetree/.../kaslr-seed)
> I'm not sure on other arch if it will be wiped out.

The difference is if I have the kaslr seed, I can calculate the kernel
base address.

In your case, you are feeding an RNG which continually has entropy
added to it. I can't see that knowing one piece of the entropy data is
a security hole. It looks more like you've just copied what what done
for kaslr-seed.

> > Why not change the mapping to RW? It would be nice if this worked on
> > more than one arch.

Still wondering on this question. Mapping it R/W would mean rng-seed
could be handled later and completely out of the arch code and so
could the zeroing of the kaslr-seed. Also, we generally assume the FDT
is modifiable for any fixups. This happens on arm32 and powerpc, but I
guess we haven't needed that yet on arm64.

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [PATCH] arm64: add support for rng-seed

2019-05-14 Thread Rob Herring
On Thu, May 9, 2019 at 11:27 PM Hsin-Yi Wang  wrote:
>
> On Wed, May 8, 2019 at 3:47 AM Rob Herring  wrote:
>
> > >  Documentation/devicetree/bindings/chosen.txt | 14 +
> >
> > Actually, this file has been converted to json-schema and lives
> > here[1]. I need to remove this one (or leave it with a reference to
> > the new one).
> >
>
> Hi Rob,
> I can't find where the new document is. Can you help point it again? Thanks.

Sorry, forgot to add that:

https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [PATCH] arm64: add support for rng-seed

2019-05-14 Thread Rob Herring
+boot-architecture list as there was some discussion about this IIRC.

On Mon, May 6, 2019 at 11:54 PM Hsin-Yi Wang  wrote:
>
> Introducing a chosen node, rng-seed, which is an 64 bytes entropy
> that can be passed to kernel called very early to increase device
> randomness. Bootloader should provide this entropy and the value is
> read from /chosen/rng-seed in DT.
>
> Signed-off-by: Hsin-Yi Wang 
>
> ---
>  Documentation/devicetree/bindings/chosen.txt | 14 +

Actually, this file has been converted to json-schema and lives
here[1]. I need to remove this one (or leave it with a reference to
the new one).

>  arch/arm64/kernel/setup.c|  2 ++
>  drivers/of/fdt.c | 33 
>  include/linux/of_fdt.h   |  1 +
>  4 files changed, 50 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/chosen.txt 
> b/Documentation/devicetree/bindings/chosen.txt
> index 45e79172a646..bfd360691650 100644
> --- a/Documentation/devicetree/bindings/chosen.txt
> +++ b/Documentation/devicetree/bindings/chosen.txt
> @@ -28,6 +28,20 @@ mode) when EFI_RNG_PROTOCOL is supported, it will be 
> overwritten by
>  the Linux EFI stub (which will populate the property itself, using
>  EFI_RNG_PROTOCOL).
>
> +rng-seed
> +---
> +
> +This property served as an entropy to add device randomness. It is parsed
> +as a 64 byte value, e.g.

Why only 64-bytes?

> +
> +/ {
> +   chosen {
> +   rng-seed = <0x31951b3c 0xc9fab3a5 0xffdf1660 ...>
> +   };
> +};
> +
> +This random value should be provided by bootloader.
> +
>  stdout-path
>  ---
>
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 413d566405d1..ade4261516dd 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -292,6 +292,8 @@ void __init setup_arch(char **cmdline_p)
> early_fixmap_init();
> early_ioremap_init();
>
> +   early_init_dt_rng_seed(__fdt_pointer);
> +

I'm trying to reduce or eliminate all these early_init_dt_* calls.

Why is this arch specific and why can't this be done after
unflattening? It doesn't look like add_device_randomness() needs
anything early.

> setup_machine_fdt(__fdt_pointer);
>
> parse_early_param();
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index de893c9616a1..74e2c0c80b91 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> @@ -1117,6 +1118,38 @@ int __init early_init_dt_scan_chosen(unsigned long 
> node, const char *uname,
> return 1;
>  }
>
> +extern void *__init __fixmap_remap_fdt(phys_addr_t dt_phys, int *size,
> +  pgprot_t prot);
> +
> +void __init early_init_dt_rng_seed(u64 dt_phys)
> +{
> +   void *fdt;
> +   int node, size, i;
> +   fdt64_t *prop;
> +   u64 rng_seed[8];
> +
> +   fdt = __fixmap_remap_fdt(dt_phys, , PAGE_KERNEL);
> +   if (!fdt)
> +   return;
> +
> +   node = fdt_path_offset(fdt, "/chosen");
> +   if (node < 0)
> +   return;
> +
> +   prop = fdt_getprop_w(fdt, node, "rng-seed", );
> +   if (!prop || size != sizeof(u64) * 8)
> +   return;
> +
> +   for (i = 0; i < 8; i++) {
> +   rng_seed[i] = fdt64_to_cpu(*(prop + i));
> +   /* clear seed so it won't be found. */
> +   *(prop + i) = 0;
> +   }
> +   add_device_randomness(rng_seed, size);
> +
> +   return;
> +}
> +
>  #ifndef MIN_MEMBLOCK_ADDR
>  #define MIN_MEMBLOCK_ADDR  __pa(PAGE_OFFSET)
>  #endif
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index a713e5d156d8..a4548dd6351e 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -71,6 +71,7 @@ extern uint32_t of_get_flat_dt_phandle(unsigned long node);
>
>  extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
>  int depth, void *data);
> +extern void early_init_dt_rng_seed(u64 dt_phys);
>  extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>  int depth, void *data);
>  extern int early_init_dt_scan_chosen_stdout(void);
> --
> 2.20.1
>
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [PATCH v2 1/2] fdt: add support for rng-seed

2019-05-13 Thread Rob Herring
On Sun, May 12, 2019 at 7:39 PM Hsin-Yi Wang  wrote:
>
> Introducing a chosen node, rng-seed, which is an entropy that can be
> passed to kernel called very early to increase initial device
> randomness. Bootloader should provide this entropy and the value is
> read from /chosen/rng-seed in DT.
>
> Signed-off-by: Hsin-Yi Wang 
> ---
> change log:
> v1->v2:
> * call function in early_init_dt_scan_chosen
> * will add doc to devicetree-org/dt-schema on github if this is accepted
> ---
>  Documentation/devicetree/bindings/chosen.txt | 14 ++
>  drivers/of/fdt.c | 11 +++
>  2 files changed, 25 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/chosen.txt 
> b/Documentation/devicetree/bindings/chosen.txt
> index 45e79172a646..fef5c82672dc 100644
> --- a/Documentation/devicetree/bindings/chosen.txt
> +++ b/Documentation/devicetree/bindings/chosen.txt
> @@ -28,6 +28,20 @@ mode) when EFI_RNG_PROTOCOL is supported, it will be 
> overwritten by
>  the Linux EFI stub (which will populate the property itself, using
>  EFI_RNG_PROTOCOL).
>
> +rng-seed
> +---
> +
> +This property served as an entropy to add device randomness. It is parsed
> +as a byte array, e.g.
> +
> +/ {
> +   chosen {
> +   rng-seed = <0x31 0x95 0x1b 0x3c 0xc9 0xfa 0xb3 ...>;
> +   };
> +};
> +
> +This random value should be provided by bootloader.
> +
>  stdout-path
>  ---
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index de893c9616a1..96ea5eba9dd5 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include   /* for COMMAND_LINE_SIZE */
>  #include 
> @@ -1079,6 +1080,7 @@ int __init early_init_dt_scan_chosen(unsigned long 
> node, const char *uname,
>  {
> int l;
> const char *p;
> +   const void *rng_seed;
>
> pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
>
> @@ -1113,6 +1115,15 @@ int __init early_init_dt_scan_chosen(unsigned long 
> node, const char *uname,
>
> pr_debug("Command line is: %s\n", (char*)data);
>
> +   rng_seed = of_get_flat_dt_prop(node, "rng-seed", );
> +   if (!rng_seed || l == 0)
> +   return 1;

This only works if this hunk stays at the end of the function. I'd
invert the if and move the next 2 functions under it.

> +
> +   /* try to clear seed so it won't be found. */
> +fdt_nop_property(initial_boot_params, node, "rng-seed");

I'd just delete the property.

Also, what about kexec? Don't you need to add a new seed?

> +
> +add_device_randomness(rng_seed, l);
> +
> /* break now */
> return 1;
>  }
> --
> 2.20.1
>


Re: [RFC v9 2/5] dt-bindings: pstore-block: new support for blkoops

2019-02-22 Thread Rob Herring
+boot-architecture list

On Tue, Feb 19, 2019 at 07:52:47PM +0800, liaoweixiong wrote:
> Create DT binding document for blkoops.
> 
> Signed-off-by: liaoweixiong 
> ---
>  .../devicetree/bindings/pstore/blkoops.txt | 53 
> ++
>  MAINTAINERS|  1 +
>  2 files changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pstore/blkoops.txt
> 
> diff --git a/Documentation/devicetree/bindings/pstore/blkoops.txt 
> b/Documentation/devicetree/bindings/pstore/blkoops.txt
> new file mode 100644
> index 000..5462915
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pstore/blkoops.txt
> @@ -0,0 +1,53 @@
> +Blkoops oops logger
> +===
> +
> +Blkoops provides a block partition for oops, excluding panics now, so they 
> can
> +be recovered after a reboot.
> +
> +Any space of block device will be used for a circular buffer of oops records.
> +These records have a configurable size, with a size of 0 indicating that they
> +should be disabled.
> +
> +At least one of "block-device" and "total_size" must be set.
> +
> +At least one of "dmesg-size" or "pmsg-size" must be set non-zero.
> +
> +Required properties:
> +
> +- compatible: must be "blkoops".
> +
> +Optional properties:
> +
> +- block-device: The block device to use. Most of the time, it is a partition 
> of
> + device. If block-device is NULL, no block device is effective
> + and the data will be lost after rebooting.
> + It accept the following variants:
> + 1)  device number in hexadecimal
> +represents itself no leading 0x, for example b302.
> + 2) /dev/ represents the device number of disk
> + 3) /dev/ represents the device number of
> +partition - device number of disk plus the partition number
> + 4) /dev/p - same as the above, that form is
> +used when disk name of partitioned disk ends on a digit.
> + 5) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing
> +the unique id of a partition if the partition table provides
> +it. The UUID may be either an EFI/GPT UUID, or refer to an
> +MSDOS partition using the format -PP, where 
> +is a zero-filled hex representation of the 32-bit
> +"NT disk signature", and PP is a zero-filled hex
> +representation of the 1-based partition number.
> + 6) PARTUUID=/PARTNROFF= to select a partition in
> +relation to a partition with a known unique id.
> + 7) : major and minor number of the device
> +separated by a colon.

No.

I didn't suggest to go look at PARTUUID to copy it into the binding, but 
rather to point out that the kernel can already mount by UUID. 
Specifying the UUID in DT is also not what I suggested. My suggestion is 
to define a known UUID so that the kernel (and bootloaders, userspace, 
the world) can just know the UUID. Just like the EFI system partition. 
Now this means you have to get it defined in the UEFI specification 
(or maybe EBBR[1]). If you want help with how to do that, the 
boot-architecture list is a good place to start.

major/minor numbers are a Linux thing, so they don't go in DT.
/dev/* is Linux thing, so it doesn't go in DT.

You can always define all these parameters as kernel command line 
options and avoid DT. That would also make this work on *all* systems, 
not just DT based systems. (Though I still believe that the partition 
should be discoverable.)

Rob

[1] https://github.com/ARM-software/ebbr


___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: Moving ARM dts files

2018-12-07 Thread Rob Herring
On Fri, Dec 7, 2018 at 9:16 AM Mark Brown  wrote:
>
> On Fri, Dec 07, 2018 at 08:57:06AM -0600, Rob Herring wrote:
> > On Thu, Dec 6, 2018 at 2:07 PM Mark Brown  wrote:
>
> > > The issues with the existing install_dtbs sounded unrelated to this.
>
> > Maybe, what are the issues? We can't change the source layout
> > transparently if dtbs_install is not being used.
>
> I thought that was the thing with adding -@ so overlays could be used?

I don't think so as that is during building, not install. Any user can
set '-@' with 'make DTC_FLAGS="-@" ...' already. The issue with that
was changing the default globally and no way to set per platform. Now
that I think about, moving the sources to subdirs may allow setting
DTC_FLAGS per subdir which may be good enough.

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: Moving ARM dts files

2018-12-06 Thread Rob Herring
On Thu, Dec 6, 2018 at 7:32 AM Andreas Färber  wrote:
>
> Am 05.12.18 um 05:17 schrieb Rob Herring:
> > On Tue, Dec 4, 2018 at 7:22 PM Andreas Färber  wrote:
> >>
> >> Rob,
> >>
> >> Am 04.12.18 um 19:36 schrieb Rob Herring:
> >>> I've put together a script to move the dts files and update the
> >>> makefiles. It doesn't handle files not following a common prefix which
> >>> isn't many and some includes within the dts files will need some fixups
> >>> by hand.
> >>>
> >>> MAINTAINERS will also need updating.
> >>>
> >>> A few questions:
> >>>
> >>> Do we want to move absolutely everything to subdirs?
> >>
> >> This refactoring is a terrible idea!
> >
> > How do you really feel?
> >
> >> While it would've been nice to have more structure from the start,
> >> bootloaders like U-Boot expect a flat structure for arm .dtb files now.
> >> If you start installing them into subdirs instead, they won't find the
> >> files anymore under the hardcoded name.
> >>
> >> Doing this only for new platforms would be much less invasive and allow
> >> to prepare bootloaders accordingly.
> >
> > That was my suggestion where this started for the new RDA platform.
> > Olof preferred to move everything and that's my desire too.
> >
> >> Alternatively, white-list which ones
> >> are safe to move around.
> >
> > I'd prefer to know which ones the distros don't want moved. That
> > should be easier to figure out. We also need that anyways in context
> > of what platforms we care about compatibility.
> >
> > Another option is dtbs_install target could flatten the installed
> > dtbs. That is the only part the distros should depend on.
>
> I'd be okay with distinguishing source vs. install location. Due to the
> issue I mention below (and more) we can't use install_dtbs for openSUSE
> and had to reimplement it, which we'd need to (and can) adjust.

What would be needed for dtbs_install to work? arm64 needs to support
a flat install? If it doesn't work for Debian or openSUSE, I'm not
sure why we have it. So I'd like to make it work.

> >> But don't just script a refactoring because it
> >> looks nicer in the source tree, without testing what side effects this
> >> can have for board/distro users of the compiled files in practice.
> >> We already had that discussion for arm64 because Debian chose to ignore
> >> the kernel-installed subdirectories and installed .dtb files into a flat
> >> directory, which collided with openSUSE sticking to the kernel choice.
> >
> > So everyone already deals with subdirs or not with arm and arm64
> > already, seems like they can deal with this. I will raise the topic on
> > the cross-distro list though.
>
> Sounds like you're twisting words... The keyword was "hardcoded" paths -
> one way or another (not "and") depending on the kernel choices being
> flat for arm, vendor subdir for arm64.
>
> >> This topic becomes even more important with EBBR: There is neither a
> >> mechanism in place to sync .dts files into U-Boot or EDK2 source trees,
> >> nor are capsule updates implemented in U-Boot for easily deploying such
> >> bootloaders with new .dts sources or paths yet.
> >
> > EBBR actually says firmware (including dtbs) goes in directories named
> > by vendor.
>
> Fine, but unrelated.

If the distros want dtbs in a flat dir and EBBR says otherwise, then
it is related.

> >> And I can assure you
> >> that just getting users to dd the right bootloader can be difficult...
> >> Since DT forward and backward compatibility is often being neglected,
> >> for example with optional properties or renamed compatibles that break
> >> booting with previous drivers, new kernel versions often need updated
> >> Device Trees to make use of new/enhanced drivers. Therefore it is
> >> unfortunately often enough a necessity to load newer kernel-based .dtb
> >> files matching the kernel (as opposed to the dream of kernel-independent
> >> hardware descriptions) when working with the latest -rc or -next kernels
> >> at least. For examples of DTs needing updates, look no further than
> >> Linaro's 96boards - in case of hikey960/EDK2 GRUB is another layer where
> >> .dtb paths may be hardcoded, ditto for arm; and Armada was an example
> >> where the upstream bindings for the network IP changed incompatibly.
> >
> > Compatibility is an issue, yes, but that really has nothing to do

Re: Moving ARM dts files

2018-12-04 Thread Rob Herring
On Tue, Dec 4, 2018 at 7:22 PM Andreas Färber  wrote:
>
> Rob,
>
> Am 04.12.18 um 19:36 schrieb Rob Herring:
> > I've put together a script to move the dts files and update the
> > makefiles. It doesn't handle files not following a common prefix which
> > isn't many and some includes within the dts files will need some fixups
> > by hand.
> >
> > MAINTAINERS will also need updating.
> >
> > A few questions:
> >
> > Do we want to move absolutely everything to subdirs?
>
> This refactoring is a terrible idea!

How do you really feel?

> While it would've been nice to have more structure from the start,
> bootloaders like U-Boot expect a flat structure for arm .dtb files now.
> If you start installing them into subdirs instead, they won't find the
> files anymore under the hardcoded name.
>
> Doing this only for new platforms would be much less invasive and allow
> to prepare bootloaders accordingly.

That was my suggestion where this started for the new RDA platform.
Olof preferred to move everything and that's my desire too.

> Alternatively, white-list which ones
> are safe to move around.

I'd prefer to know which ones the distros don't want moved. That
should be easier to figure out. We also need that anyways in context
of what platforms we care about compatibility.

Another option is dtbs_install target could flatten the installed
dtbs. That is the only part the distros should depend on.

> But don't just script a refactoring because it
> looks nicer in the source tree, without testing what side effects this
> can have for board/distro users of the compiled files in practice.
> We already had that discussion for arm64 because Debian chose to ignore
> the kernel-installed subdirectories and installed .dtb files into a flat
> directory, which collided with openSUSE sticking to the kernel choice.

So everyone already deals with subdirs or not with arm and arm64
already, seems like they can deal with this. I will raise the topic on
the cross-distro list though.

> This topic becomes even more important with EBBR: There is neither a
> mechanism in place to sync .dts files into U-Boot or EDK2 source trees,
> nor are capsule updates implemented in U-Boot for easily deploying such
> bootloaders with new .dts sources or paths yet.

EBBR actually says firmware (including dtbs) goes in directories named
by vendor.

> And I can assure you
> that just getting users to dd the right bootloader can be difficult...
> Since DT forward and backward compatibility is often being neglected,
> for example with optional properties or renamed compatibles that break
> booting with previous drivers, new kernel versions often need updated
> Device Trees to make use of new/enhanced drivers. Therefore it is
> unfortunately often enough a necessity to load newer kernel-based .dtb
> files matching the kernel (as opposed to the dream of kernel-independent
> hardware descriptions) when working with the latest -rc or -next kernels
> at least. For examples of DTs needing updates, look no further than
> Linaro's 96boards - in case of hikey960/EDK2 GRUB is another layer where
> .dtb paths may be hardcoded, ditto for arm; and Armada was an example
> where the upstream bindings for the network IP changed incompatibly.

Compatibility is an issue, yes, but that really has nothing to do with this.

> DT overlays are another topic that is not making any progress upstream
> according to the ELCE BoF, so beyond the Raspberry Pi the only known
> working way to apply them is to write a U-Boot boot.scr script, which
> can either reuse $fdtcontroladdr DT or use the filename $fdtfile or
> hardcode one, the latter two of which would break with your renaming.

DT overlays also have nothing to do with this as there aren't any in
the kernel. I'm not inclined to take any either with a flat tree.
We're already at 1800+ files.

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [Arm.ebbr-discuss] U-boot

2018-07-30 Thread Rob Herring
On Mon, Jul 30, 2018 at 8:11 AM Alexander Graf  wrote:
>
> On 07/30/2018 02:39 PM, Alexander Graf wrote:
> > On 07/30/2018 02:16 PM, David Rusling wrote:
> >> Success.   I now have a u-boot built on Arm64 that works.   Along the
> >> way I learnt various things:
> >>
> >> [1] Raspberry Pi's first stage loader generates the device tree.
> >>  Overlays are used to turn various things on (for example sound) at
> >> boot time.
> >
> > Yes. In order for those to propagate into U-Boot you will want to
> > enable CONFIG_OF_BOARD. That way the first stage generated DT gets
> > consumed by U-Boot as well. This allows you to run on CM3 or have the
> > same U-Boot binary for B and B+ for example.
>
> I forgot to mention that for this to work fully with propagation of that
> same device tree to an upstream kernel, you will also want to add the
> "upstream" overlay. Just add a line saying "dtoverlay=upstream" to
> config.txt. Then plug in any installer or image that is UEFI enabled
> (and works with DT) and it should boot.

Yuck. That's a fix-up applied to the RPi downstream dtb, right? I
always just load my own (upstream) dtb. Who is providing the upstream
overlay, and how do you do dtb updates in this case as what is
upstream evolves? I'm just concerned this is very much RPi specific
and how can EBBR define managing dtb updates/storage in a consistent
way.

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [Arm.ebbr-discuss] [RFC] uefi: Account for SetVariable() not working at runtime

2018-07-12 Thread Rob Herring
On Thu, Jul 12, 2018 at 9:37 AM Graeme Gregory
 wrote:
>
> On Thu, 12 Jul 2018 at 16:30, Udit Kumar  wrote:
> >
> > Hi Mark
> >
> > > -Original Message-
> > > From: Mark Brown [mailto:broo...@kernel.org]
> > > Sent: Thursday, July 12, 2018 8:20 PM
> > > To: Udit Kumar 
> > > Cc: Ard Biesheuvel ; Architecture Mailman List
> > > ; nd ; arm.ebbr-discuss
> > > 
> > > Subject: Re: [Arm.ebbr-discuss] [RFC] uefi: Account for SetVariable() not 
> > > working
> > > at runtime
> > >
> > > On Thu, Jul 12, 2018 at 02:19:49PM +, Udit Kumar wrote:
> > >
> > > > > > Do any existing implementations change variables from non-volatile
> > > > > > to volatile?
> > >
> > > > > The UEFI spec is explicit about which variables are volatile and 
> > > > > which are not.
> > > > > Simply relaxing non-volatile to volatile in the general case doesn't
> > > > > seem like a useful approach to me.
> > >
> > > > I believe at boot-time, UEFI specs will be followed for volatile and 
> > > > non-volatile
> > > variables.
> > > > Having this in statement EBBR, will help those platform, which cannot 
> > > > expose
> > > non-volatile variables at runtime.
> > >
> > > If nothing currently does it the chances that anything will actually cope 
> > > well
> > > seem minimal.  Like Daniel said it seems more likely to break things - if 
> > > the
> > > variables are defined as being non-volatile then the OS is unlikely to be 
> > > checking
> > > at runtime if that's the case or not unless it's explicitly written to 
> > > work with
> > > EBBR.  If an error is generated because a non-volatile variable can't be 
> > > set then
> > > that should at least fall into whatever error handling code is there to 
> > > cover
> > > normal rutime failures which has some chance of doing something sensible.
> >
> > Right,
> > There will be some breaks or say diversion from UEFI specs.
> > If we need to follow UEFI specs 'Table 10. Global Variables' on such 
> > platform
> > where we cannot write to NV storage at runtime, then in either case
> > 1/ passing OS as no-efi-runtime or
> > 2/ Returning errors/not saving to NV storage
> > is violation of UEFI spec.
> >
> > Which divergence is acceptable ?
> >
> Neither, fix the specs with proper updates to support your use-case.
>
> Or fix your platform to obey the specs you have chosen.

How do you add firmware storage to existing platforms?

> Don't write a spec to bend another spec that way leads to chaos.

Well, EBBR is such a spec. The chaos is already there and EBBR
attempts to apply some amount of order to the chaos.

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [PATCH v4 1/6] driver core: allow stopping deferred probe after init

2018-07-09 Thread Rob Herring
On Mon, Jul 9, 2018 at 9:52 AM Russell King - ARM Linux
 wrote:
>
> On Mon, Jul 09, 2018 at 09:41:48AM -0600, Rob Herring wrote:
> > Deferred probe will currently wait forever on dependent devices to probe,
> > but sometimes a driver will never exist. It's also not always critical for
> > a driver to exist. Platforms can rely on default configuration from the
> > bootloader or reset defaults for things such as pinctrl and power domains.
> > This is often the case with initial platform support until various drivers
> > get enabled. There's at least 2 scenarios where deferred probe can render
> > a platform broken. Both involve using a DT which has more devices and
> > dependencies than the kernel supports. The 1st case is a driver may be
> > disabled in the kernel config. The 2nd case is the kernel version may
> > simply not have the dependent driver. This can happen if using a newer DT
> > (provided by firmware perhaps) with a stable kernel version. Deferred
> > probe issues can be difficult to debug especially if the console has
> > dependencies or userspace fails to boot to a shell.
> >
> > There are also cases like IOMMUs where only built-in drivers are
> > supported, so deferring probe after initcalls is not needed. The IOMMU
> > subsystem implemented its own mechanism to handle this using OF_DECLARE
> > linker sections.
> >
> > This commit adds makes ending deferred probe conditional on initcalls
> > being completed or a debug timeout. Subsystems or drivers may opt-in by
> > calling driver_deferred_probe_check_init_done() instead of
> > unconditionally returning -EPROBE_DEFER. They may use additional
> > information from DT or kernel's config to decide whether to continue to
> > defer probe or not.
> >
> > The timeout mechanism is intended for debug purposes and WARNs loudly.
> > The remaining deferred probe pending list will also be dumped after the
> > timeout. Not that this timeout won't work for the console which needs
> > to be enabled before userspace starts. However, if the console's
> > dependencies are resolved, then the kernel log will be printed (as
> > opposed to no output).
>
> So what happens if we have a set of modules which use deferred probing
> in order to work?

It is opt-in by subsystem or drivers and mainly intended for
subsystems which can be optional or only support built-in drivers.
However, I don't really envision many other users other than the ones
I converted (pinctrl, iommu, pm-domains). If you look at patch 3,
you'll see it is dependent on !CONFIG_MODULES.

For the timeout, well, that's for debugging only. If you get to the
point of loading sound modules, you probably don't need the timeout.
It's for debugging not booting.

> For example, with sound stuff built as modules, and auto-loaded in
> parallel by udev, the modules get added in a random order.  The
> modules have non-udev obvious dependencies between them (resource
> dependencies) which result in deferred probing being necessary to
> bring the device up.
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


[PATCH v4 5/6] iommu: Remove IOMMU_OF_DECLARE

2018-07-09 Thread Rob Herring
Now that we use the driver core to stop deferred probe for missing
drivers, IOMMU_OF_DECLARE can be removed.

This is slightly less optimal than having a list of built-in drivers in
that we'll now defer probe twice before giving up. This shouldn't have a
significant impact on boot times as past discussions about deferred
probe have given no evidence of deferred probe having a substantial
impact.

Cc: Robin Murphy 
Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 
Cc: Rob Clark 
Cc: Heiko Stuebner 
Cc: Frank Rowand 
Cc: linux-arm-ker...@lists.infradead.org
Cc: io...@lists.linux-foundation.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-arm-...@vger.kernel.org
Cc: linux-rockc...@lists.infradead.org
Cc: devicet...@vger.kernel.org
Acked-by: Will Deacon 
Acked-by: Marek Szyprowski 
Acked-by: Joerg Roedel 
Signed-off-by: Rob Herring 
---
 drivers/iommu/arm-smmu-v3.c   |  2 --
 drivers/iommu/arm-smmu.c  |  7 ---
 drivers/iommu/exynos-iommu.c  |  2 --
 drivers/iommu/ipmmu-vmsa.c|  3 ---
 drivers/iommu/msm_iommu.c |  2 --
 drivers/iommu/of_iommu.c  | 19 +--
 drivers/iommu/qcom_iommu.c|  2 --
 drivers/iommu/rockchip-iommu.c|  2 --
 include/asm-generic/vmlinux.lds.h |  2 --
 include/linux/of_iommu.h  |  4 
 10 files changed, 1 insertion(+), 44 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 1d647104bccc..22bdabd3d8e0 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);
 
-IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
 MODULE_AUTHOR("Will Deacon ");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f7a96bcf94a6..c73cfce1ccc0 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);
 
-IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
-IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
-IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
-IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
-IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
-IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
 MODULE_AUTHOR("Will Deacon ");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 85879cfec52f..b128cb4372d3 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1390,5 +1390,3 @@ static int __init exynos_iommu_init(void)
return ret;
 }
 core_initcall(exynos_iommu_init);
-
-IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 40ae6e87cb88..f026aa16d5f1 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1108,9 +1108,6 @@ static void __exit ipmmu_exit(void)
 subsys_initcall(ipmmu_init);
 module_exit(ipmmu_exit);
 
-IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
-IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
-
 MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
 MODULE_AUTHOR("Laurent Pinchart ");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 0d3350463a3f..27377742600d 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -877,7 +877,5 @@ static void __exit msm_iommu_driver_exit(void)
 subsys_initcall(msm_iommu_driver_init);
 module_exit(msm_iommu_driver_exit);
 
-IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
-
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Stepan Moskovchenko ");
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 78ddf47dd67a..f7787e757244 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -27,9 +27,6 @@
 
 #define NO_IOMMU   1
 
-static const struct of_device_id __iommu_of_table_sentinel
-   __used __section(__iommu_of_table_end);
-
 /**
  * of_get_dma_window - Parse *dma-window property and returns 0 if found.
  *
@@ -98,19 +95,6 @@ int of_get_dma_window(struct device_node *dn, const char 
*prefix, int index,
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);
 
-static bool of_iommu_driver_present(struct device_node *np)
-{
-   /*
-* If the IOMMU still isn't ready by the time we reach init, assume
-* it never will be. We don't want to defer indefinitely, nor attempt
-* to dereference __iommu_of_table after it's been freed.
-*/
-   if (system_st

[PATCH v4 4/6] iommu: Stop deferring probe at end of initcalls

2018-07-09 Thread Rob Herring
The IOMMU subsystem has its own mechanism to not defer probe if driver
support is missing. Now that the driver core supports stopping deferring
probe if drivers aren't built-in (and probed), use the driver core
support so the IOMMU specific support can be removed.

Acked-by: Joerg Roedel 
Cc: io...@lists.linux-foundation.org
Signed-off-by: Rob Herring 
---
 drivers/iommu/of_iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5c36a8b7656a..78ddf47dd67a 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -133,7 +133,7 @@ static int of_iommu_xlate(struct device *dev,
 * a proper probe-ordering dependency mechanism in future.
 */
if (!ops)
-   return -EPROBE_DEFER;
+   return driver_deferred_probe_check_state(dev);
 
return ops->of_xlate(dev, iommu_spec);
 }
-- 
2.17.1

___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


[PATCH v4 3/6] pinctrl: Support stopping deferred probe after initcalls

2018-07-09 Thread Rob Herring
Pinctrl drivers are a common dependency which can prevent a system
booting even if the default or bootloader configured settings can work.
If a pinctrl node in DT indicates that the default pin setup can be used
with the 'pinctrl-use-default' property, then only defer probe until
initcalls are done. If the deferred probe timeout is enabled or loadable
modules are disabled, then we'll stop deferring probe regardless of the
DT property. This gives platforms the option to work without their
pinctrl driver being enabled.

Dropped the pinctrl specific deferring probe message as the driver core
can print deferred probe related messages if needed.

Reviewed-by: Linus Walleij 
Signed-off-by: Rob Herring 
---
v4:
- Add Linus' R-by.

v3:
- Drop pinctrl deferred probe msg in favor of driver core messages
- Move the handling of "pinctrl-use-default" option out of driver core
- Stop deferring probe if modules are not enabled.

 drivers/pinctrl/devicetree.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index c4aa411f5935..2969ff3162c3 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -111,17 +111,24 @@ static int dt_to_map_one_config(struct pinctrl *p,
int ret;
struct pinctrl_map *map;
unsigned num_maps;
+   bool allow_default = false;

/* Find the pin controller containing np_config */
np_pctldev = of_node_get(np_config);
for (;;) {
+   if (!allow_default)
+   allow_default = of_property_read_bool(np_pctldev,
+ 
"pinctrl-use-default");
+
np_pctldev = of_get_next_parent(np_pctldev);
if (!np_pctldev || of_node_is_root(np_pctldev)) {
-   dev_info(p->dev, "could not find pctldev for node %pOF, 
deferring probe\n",
-   np_config);
of_node_put(np_pctldev);
-   /* OK let's just assume this will appear later then */
-   return -EPROBE_DEFER;
+   ret = driver_deferred_probe_check_state(p->dev);
+   /* keep deferring if modules are enabled unless we've 
timed out */
+   if (IS_ENABLED(CONFIG_MODULES) && !allow_default && ret 
== -ENODEV)
+   ret = -EPROBE_DEFER;
+
+   return ret;
}
/* If we're creating a hog we can use the passed pctldev */
if (hog_pctldev && (np_pctldev == p->dev->of_node)) {
--
2.17.1
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


[PATCH v4 2/6] dt-bindings: pinctrl: add a 'pinctrl-use-default' property

2018-07-09 Thread Rob Herring
Pin setup may be optional in some cases such as the reset default works
or the pin setup is done by the bootloader. In these cases, it is optional
for the OS to support managing the pin controller and pin setup. In order
to support this scenario, add a property 'pinctrl-use-default' to indicate
that the pin configuration is optional.

Signed-off-by: Rob Herring 
---
 .../devicetree/bindings/pinctrl/pinctrl-bindings.txt| 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt 
b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
index ad9a36e9..cef2b5855d60 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -103,6 +103,12 @@ Optional properties:
 #pinctrl-cells:Number of pin control cells in addition to the index 
within the
pin controller device instance
 
+pinctrl-use-default: Boolean. Indicates that the OS can use the boot default
+   pin configuration. This allows using an OS that does not have a
+   driver for the pin controller. This property can be set either
+   globally for the pin controller or in child nodes for individual
+   pin group control.
+
 Pin controller devices should contain the pin configuration nodes that client
 devices reference.
 
-- 
2.17.1

___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


[PATCH v4 1/6] driver core: allow stopping deferred probe after init

2018-07-09 Thread Rob Herring
Deferred probe will currently wait forever on dependent devices to probe,
but sometimes a driver will never exist. It's also not always critical for
a driver to exist. Platforms can rely on default configuration from the
bootloader or reset defaults for things such as pinctrl and power domains.
This is often the case with initial platform support until various drivers
get enabled. There's at least 2 scenarios where deferred probe can render
a platform broken. Both involve using a DT which has more devices and
dependencies than the kernel supports. The 1st case is a driver may be
disabled in the kernel config. The 2nd case is the kernel version may
simply not have the dependent driver. This can happen if using a newer DT
(provided by firmware perhaps) with a stable kernel version. Deferred
probe issues can be difficult to debug especially if the console has
dependencies or userspace fails to boot to a shell.

There are also cases like IOMMUs where only built-in drivers are
supported, so deferring probe after initcalls is not needed. The IOMMU
subsystem implemented its own mechanism to handle this using OF_DECLARE
linker sections.

This commit adds makes ending deferred probe conditional on initcalls
being completed or a debug timeout. Subsystems or drivers may opt-in by
calling driver_deferred_probe_check_init_done() instead of
unconditionally returning -EPROBE_DEFER. They may use additional
information from DT or kernel's config to decide whether to continue to
defer probe or not.

The timeout mechanism is intended for debug purposes and WARNs loudly.
The remaining deferred probe pending list will also be dumped after the
timeout. Not that this timeout won't work for the console which needs
to be enabled before userspace starts. However, if the console's
dependencies are resolved, then the kernel log will be printed (as
opposed to no output).

Cc: Alexander Graf 
Signed-off-by: Rob Herring 
---
v4:
- Rebase on driver-core-next
- Only allow base 10 for timeout

v3:
- Merged with timeout patch.
- Clarify that deferred_probe_timeout is a debug option.
- Drop the 'optional' param. The only user was pinctrl, so it has to handle
  that functionality.
- Rename function to driver_deferred_probe_check_state
- Added kerneldoc for driver_deferred_probe_check_state
- Print a 1 line warning if stopping deferred probe after initcalls and a
  WARN on timeout.

 .../admin-guide/kernel-parameters.txt |  9 +++
 drivers/base/dd.c | 59 +++
 include/linux/device.h|  2 +
 3 files changed, 70 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index efc7aa7a0670..e83ef4648ea4 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -804,6 +804,15 @@
Defaults to the default architecture's huge page size
if not specified.

+   deferred_probe_timeout=
+   [KNL] Debugging option to set a timeout in seconds for
+   deferred probe to give up waiting on dependencies to
+   probe. Only specific dependencies (subsystems or
+   drivers) that have opted in will be ignored. A timeout 
of 0
+   will timeout at the end of initcalls. This option will 
also
+   dump out devices still on the deferred probe list after
+   retrying.
+
dhash_entries=  [KNL]
Set number of hash buckets for dentry cache.

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index e85705e84407..fb62f1be40d3 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -55,6 +55,7 @@ static LIST_HEAD(deferred_probe_pending_list);
 static LIST_HEAD(deferred_probe_active_list);
 static atomic_t deferred_trigger_count = ATOMIC_INIT(0);
 static struct dentry *deferred_devices;
+static bool initcalls_done;

 /*
  * In some cases, like suspend to RAM or hibernation, It might be reasonable
@@ -219,6 +220,51 @@ static int deferred_devs_show(struct seq_file *s, void 
*data)
 }
 DEFINE_SHOW_ATTRIBUTE(deferred_devs);

+static int deferred_probe_timeout = -1;
+static int __init deferred_probe_timeout_setup(char *str)
+{
+   deferred_probe_timeout = simple_strtol(str, NULL, 10);
+   return 1;
+}
+__setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
+
+/**
+ * driver_deferred_probe_check_state() - Check deferred probe state
+ * @dev: device to check
+ *
+ * Returns -ENODEV if init is done and all built-in drivers have had a chance
+ * to probe (i.e. initcalls are done), -ETIMEDOUT if deferred probe debug
+ * timeout has expired, or -EPROBE_DEFER if none of those conditions are met.
+ *
+ * Drivers or subsystems can opt-in to calling this function instead of 
directly
+ * returning -EPROBE_DEFER.
+ */
+int driver_deferred_probe_c

[PATCH v4 0/6] Make deferring probe forever optional

2018-07-09 Thread Rob Herring
This series came out of a discussion on the ARM boot-architecture
list[1] about DT forwards and backwards compatibility issues. There are
issues with newer DTs breaking on older, stable kernels. Some of these
are difficult to solve, but cases of optional devices not having
kernel support should be solvable.

I tested this on a RPi3 B with the pinctrl driver forced off. With this
change, the MMC/SD and UART drivers can function without the pinctrl
driver. I left the dts change out this time.

v2 and v3 of this series can be found here[2][3].

Rob

[1] https://lists.linaro.org/pipermail/boot-architecture/2018-April/000466.html
[2] https://lore.kernel.org/patchwork/project/lkml/list/?series=347413
[3] https://lore.kernel.org/patchwork/project/lkml/list/?series=357344

Rob Herring (6):
  driver core: allow stopping deferred probe after init
  dt-bindings: pinctrl: add a 'pinctrl-use-default' property
  pinctrl: Support stopping deferred probe after initcalls
  iommu: Stop deferring probe at end of initcalls
  iommu: Remove IOMMU_OF_DECLARE
  PM / Domains: Stop deferring probe at the end of initcall

 .../admin-guide/kernel-parameters.txt |  9 +++
 .../bindings/pinctrl/pinctrl-bindings.txt |  6 ++
 drivers/base/dd.c | 59 +++
 drivers/base/power/domain.c   |  2 +-
 drivers/iommu/arm-smmu-v3.c   |  2 -
 drivers/iommu/arm-smmu.c  |  7 ---
 drivers/iommu/exynos-iommu.c  |  2 -
 drivers/iommu/ipmmu-vmsa.c|  3 -
 drivers/iommu/msm_iommu.c |  2 -
 drivers/iommu/of_iommu.c  | 21 +--
 drivers/iommu/qcom_iommu.c|  2 -
 drivers/iommu/rockchip-iommu.c|  2 -
 drivers/pinctrl/devicetree.c  | 15 +++--
 include/asm-generic/vmlinux.lds.h |  2 -
 include/linux/device.h|  2 +
 include/linux/of_iommu.h  |  4 --
 16 files changed, 90 insertions(+), 50 deletions(-)

--
2.17.1
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [Arm.ebbr-discuss] [PATCH] Create a new chapter to discuss shared storage

2018-06-29 Thread Rob Herring
On Fri, Jun 29, 2018 at 11:03 AM Grant Likely  wrote:
>
> Special care is needed when storage is shared between firmware and the
> OS. Add a chapter that discusses the issues and puts down the
> requirements for using shared storage.
>
> Resolves: #19
>
> Cc: Daniel Thompson 
> Signed-off-by: Grant Likely 
> ---
>  source/chapter2-uefi.rst   |  44 --
>  source/chapter4-firmware-media.rst | 267 
> +
>  source/index.rst   |   1 +
>  3 files changed, 268 insertions(+), 44 deletions(-)
>  create mode 100644 source/chapter4-firmware-media.rst
>
> diff --git a/source/chapter2-uefi.rst b/source/chapter2-uefi.rst
> index 07426a4..05230d2 100644
> --- a/source/chapter2-uefi.rst
> +++ b/source/chapter2-uefi.rst
> @@ -56,50 +56,6 @@ System Volume Format
>  The system firmware must support all partitioning standards required
>  by the UEFI specification.
>
> -On systems where the system firmware binaries reside on the System Volume 
> then
> -the System Volume must be pre-configured with a partition table and include
> -protective partitions to reduce risk of accidental destruction of the system
> -firmware.
> -
> -All pre-installed partition tables must use GPT partitioning unless
> -some immutable feature of the platform (such as a mask programmed boot ROM)
> -makes this impossible; on such platforms MBR partitioning may be
> -used as an alternative.
> -
> -GPT partitioning
> -
> -
> -Any pre-installed partition table must strictly conform to the UEFI
> -specification and include a protective MBR authored exactly as
> -described in UEFI specification (hybrid partitioning schemes are not
> -permitted).
> -
> -Pre-installed protective partitions must have the Platform Required
> -Attribute Flag set.
> -
> -It is recommended that automatic system disk partitioning utilities
> -preserve Platform Required partitions as is, and that manual disk
> -partitioning utilities provide warnings and/or other safe guards to
> -reduce risk of accidental removal.
> -
> -MBR partitioning
> -
> -
> -Pre-installed protective partitions should have a partition type of 0xF8
> -unless some immutable feature of the platform makes this impossible.
> -
> -It is recommended that disk partitioning utilities treat such
> -partitions in the same manner as GPT partitions with the Platform
> -Required Attribute Flag set.
> -
> -It is recommended that pre-installed protective partitions that are not
> -type 0xF8 be located wholly within 1MB of the start of the disk.
> -
> -Automatic disk partitioning utilities shall not create partitions
> -within 1MB of the start of the disk. Manual disk partitioning
> -utilities should avoid recommending that partitions start within
> -1MB of the start of the disk.
> -
>  UEFI Boot Services
>  ==
>
> diff --git a/source/chapter4-firmware-media.rst 
> b/source/chapter4-firmware-media.rst
> new file mode 100644
> index 000..f8a03db
> --- /dev/null
> +++ b/source/chapter4-firmware-media.rst
> @@ -0,0 +1,267 @@
> +
> +Firmware Storage
> +
> +
> +In general, it is recommended for all platforms to store firmware images
> +and data on a dedicated storage device which is not accessed by the operating
> +system.
> +It is common practice to boot firmware out of a dedicated SPI flash,
> +while the OS is loaded from another source, whether it be SATA, SD, USB,
> +Network boot, or something else.
> +
> +However, many embedded systems have size, cost, or usage constraints that
> +make separate firmware storage unfeasible.
> +On such systems, firmware and the OS reside on the same media.
> +
> +Some shared media provides dedicate boot regions, but in many cases

dedicated

perhaps an "(e.g. eMMC)"

> +firmware needs to reside in a normal region of storage.
> +Care must be taken to ensure firmware kept in normal storage does not
> +conflict with normal usage of the media by an OS.
> +
> +* Firmware must be stored on the media in a way that does not conflict
> +  with normal partitioning and usage by the operating system.
> +* Normal operation of the OS must not interfere with firmware files
> +* Firmware needs a method to modify variable storage at runtime while the
> +  OS controls access to the device
> +
> +Partitioning of Shared Storage
> +==
> +
> +Shared storage must use GPT partitioning unless the storage media provides
> +device-native partitioning [#UFSparts]_,
> +or if an immutable property of the platform (ex. the masked boot ROM)
> +is incompatible with the SoC boot ROM).
> +MBR partitioning shall be used when the platform is incompatible with GPT.

s/when/only when/ ? Though perhaps the rest below is strong enough.

> +
> +.. warning::
> +
> +   MBR partitioning is deprecated and only included for legacy support.
> +   All new platforms are expected to use either GPT or device-native
> +   partitioning.
> +
> +   GPT partitioning supports a 

[PATCH v3 6/6] PM / Domains: Stop deferring probe at the end of initcall

2018-06-28 Thread Rob Herring
All PM domain drivers must be built-in (at least those using DT), so
there is no point deferring probe after initcalls are done. Continuing
to defer probe may prevent booting successfully even if managing PM
domains is not required. This can happen if the user failed to enable
the driver or if power-domains are added to a platform's DT, but there
is not yet a driver (e.g. a new DTB with an old kernel).

Call the driver core function driver_deferred_probe_check_init_done()
instead of just returning -EPROBE_DEFER to stop deferring probe when
initcalls are done.

Cc: "Rafael J. Wysocki" 
Cc: Kevin Hilman 
Cc: Ulf Hansson 
Cc: Pavel Machek 
Cc: Len Brown 
Cc: Greg Kroah-Hartman 
Cc: linux...@vger.kernel.org
Signed-off-by: Rob Herring 
---
v3:
  - Update to new function name

 drivers/base/power/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 4925af5c4cf0..8c12213875c6 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2253,7 +2253,7 @@ static int __genpd_dev_pm_attach(struct device *dev, 
struct device_node *np,
mutex_unlock(_list_lock);
dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
__func__, PTR_ERR(pd));
-   return -EPROBE_DEFER;
+   return driver_deferred_probe_check_state(dev);
}

dev_dbg(dev, "adding to PM domain %s\n", pd->name);
--
2.17.1
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


[PATCH v3 5/6] iommu: Remove IOMMU_OF_DECLARE

2018-06-28 Thread Rob Herring
Now that we use the driver core to stop deferred probe for missing
drivers, IOMMU_OF_DECLARE can be removed.

This is slightly less optimal than having a list of built-in drivers in
that we'll now defer probe twice before giving up. This shouldn't have a
significant impact on boot times as past discussions about deferred
probe have given no evidence of deferred probe having a substantial
impact.

Cc: Robin Murphy 
Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 
Cc: Rob Clark 
Cc: Heiko Stuebner 
Cc: Frank Rowand 
Cc: linux-arm-ker...@lists.infradead.org
Cc: io...@lists.linux-foundation.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-arm-...@vger.kernel.org
Cc: linux-rockc...@lists.infradead.org
Cc: devicet...@vger.kernel.org
Acked-by: Will Deacon 
Acked-by: Marek Szyprowski 
Acked-by: Joerg Roedel 
Signed-off-by: Rob Herring 
---
v3:
  - Also remove linker sections from vmlinux.lds.h

 drivers/iommu/arm-smmu-v3.c   |  2 --
 drivers/iommu/arm-smmu.c  |  7 ---
 drivers/iommu/exynos-iommu.c  |  2 --
 drivers/iommu/ipmmu-vmsa.c|  3 ---
 drivers/iommu/msm_iommu.c |  2 --
 drivers/iommu/of_iommu.c  | 19 +--
 drivers/iommu/qcom_iommu.c|  2 --
 drivers/iommu/rockchip-iommu.c|  2 --
 include/asm-generic/vmlinux.lds.h |  2 --
 include/linux/of_iommu.h  |  4 
 10 files changed, 1 insertion(+), 44 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 1d647104bccc..22bdabd3d8e0 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);

-IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
 MODULE_AUTHOR("Will Deacon ");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f7a96bcf94a6..c73cfce1ccc0 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);

-IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
-IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
-IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
-IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
-IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
-IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
 MODULE_AUTHOR("Will Deacon ");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 85879cfec52f..b128cb4372d3 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1390,5 +1390,3 @@ static int __init exynos_iommu_init(void)
return ret;
 }
 core_initcall(exynos_iommu_init);
-
-IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 40ae6e87cb88..f026aa16d5f1 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1108,9 +1108,6 @@ static void __exit ipmmu_exit(void)
 subsys_initcall(ipmmu_init);
 module_exit(ipmmu_exit);

-IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
-IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
-
 MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
 MODULE_AUTHOR("Laurent Pinchart ");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 0d3350463a3f..27377742600d 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -877,7 +877,5 @@ static void __exit msm_iommu_driver_exit(void)
 subsys_initcall(msm_iommu_driver_init);
 module_exit(msm_iommu_driver_exit);

-IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
-
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Stepan Moskovchenko ");
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 78ddf47dd67a..f7787e757244 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -27,9 +27,6 @@

 #define NO_IOMMU   1

-static const struct of_device_id __iommu_of_table_sentinel
-   __used __section(__iommu_of_table_end);
-
 /**
  * of_get_dma_window - Parse *dma-window property and returns 0 if found.
  *
@@ -98,19 +95,6 @@ int of_get_dma_window(struct device_node *dn, const char 
*prefix, int index,
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);

-static bool of_iommu_driver_present(struct device_node *np)
-{
-   /*
-* If the IOMMU still isn't ready by the time we reach init, assume
-* it never will be. We don't want to defer indefinitely, nor attempt
-* to dereference __iommu_of_

[PATCH v3 4/6] iommu: Stop deferring probe at end of initcalls

2018-06-28 Thread Rob Herring
The IOMMU subsystem has its own mechanism to not defer probe if driver
support is missing. Now that the driver core supports stopping deferring
probe if drivers aren't built-in (and probed), use the driver core
support so the IOMMU specific support can be removed.

Acked-by: Joerg Roedel 
Cc: io...@lists.linux-foundation.org
Signed-off-by: Rob Herring 
---
v3:
  - Update to new function name

 drivers/iommu/of_iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5c36a8b7656a..78ddf47dd67a 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -133,7 +133,7 @@ static int of_iommu_xlate(struct device *dev,
 * a proper probe-ordering dependency mechanism in future.
 */
if (!ops)
-   return -EPROBE_DEFER;
+   return driver_deferred_probe_check_state(dev);

return ops->of_xlate(dev, iommu_spec);
 }
--
2.17.1
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


[PATCH v3 3/6] pinctrl: Support stopping deferred probe after initcalls

2018-06-28 Thread Rob Herring
Pinctrl drivers are a common dependency which can prevent a system
booting even if the default or bootloader configured settings can work.
If a pinctrl node in DT indicates that the default pin setup can be used
with the 'pinctrl-use-default' property, then only defer probe until
initcalls are done. If the deferred probe timeout is enabled or loadable
modules are disabled, then we'll stop deferring probe regardless of the
DT property. This gives platforms the option to work without their
pinctrl driver being enabled.

Dropped the pinctrl specific deferring probe message as the driver core
can print deferred probe related messages if needed.

Signed-off-by: Rob Herring 
---
v3:
- Drop pinctrl deferred probe msg in favor of driver core messages
- Move the handling of "pinctrl-use-default" option out of driver core
- Stop deferring probe if modules are not enabled.

Linus, I reworked this a bit, so didn't add your ack.

 drivers/pinctrl/devicetree.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index c4aa411f5935..2969ff3162c3 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -111,17 +111,24 @@ static int dt_to_map_one_config(struct pinctrl *p,
int ret;
struct pinctrl_map *map;
unsigned num_maps;
+   bool allow_default = false;

/* Find the pin controller containing np_config */
np_pctldev = of_node_get(np_config);
for (;;) {
+   if (!allow_default)
+   allow_default = of_property_read_bool(np_pctldev,
+ 
"pinctrl-use-default");
+
np_pctldev = of_get_next_parent(np_pctldev);
if (!np_pctldev || of_node_is_root(np_pctldev)) {
-   dev_info(p->dev, "could not find pctldev for node %pOF, 
deferring probe\n",
-   np_config);
of_node_put(np_pctldev);
-   /* OK let's just assume this will appear later then */
-   return -EPROBE_DEFER;
+   ret = driver_deferred_probe_check_state(p->dev);
+   /* keep deferring if modules are enabled unless we've 
timed out */
+   if (IS_ENABLED(CONFIG_MODULES) && !allow_default && ret 
== -ENODEV)
+   ret = -EPROBE_DEFER;
+
+   return ret;
}
/* If we're creating a hog we can use the passed pctldev */
if (hog_pctldev && (np_pctldev == p->dev->of_node)) {
--
2.17.1
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


[PATCH v3 2/6] dt-bindings: pinctrl: add a 'pinctrl-use-default' property

2018-06-28 Thread Rob Herring
Pin setup may be optional in some cases such as the reset default works
or the pin setup is done by the bootloader. In these cases, it is optional
for the OS to support managing the pin controller and pin setup. In order
to support this scenario, add a property 'pinctrl-use-default' to indicate
that the pin configuration is optional.

Signed-off-by: Rob Herring 
---
 .../devicetree/bindings/pinctrl/pinctrl-bindings.txt| 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt 
b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
index ad9a36e9..cef2b5855d60 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -103,6 +103,12 @@ Optional properties:
 #pinctrl-cells:Number of pin control cells in addition to the index 
within the
pin controller device instance
 
+pinctrl-use-default: Boolean. Indicates that the OS can use the boot default
+   pin configuration. This allows using an OS that does not have a
+   driver for the pin controller. This property can be set either
+   globally for the pin controller or in child nodes for individual
+   pin group control.
+
 Pin controller devices should contain the pin configuration nodes that client
 devices reference.
 
-- 
2.17.1

___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


[PATCH v3 1/6] driver core: allow stopping deferred probe after init

2018-06-28 Thread Rob Herring
Deferred probe will currently wait forever on dependent devices to probe,
but sometimes a driver will never exist. It's also not always critical for
a driver to exist. Platforms can rely on default configuration from the
bootloader or reset defaults for things such as pinctrl and power domains.
This is often the case with initial platform support until various drivers
get enabled. There's at least 2 scenarios where deferred probe can render
a platform broken. Both involve using a DT which has more devices and
dependencies than the kernel supports. The 1st case is a driver may be
disabled in the kernel config. The 2nd case is the kernel version may
simply not have the dependent driver. This can happen if using a newer DT
(provided by firmware perhaps) with a stable kernel version. Deferred
probe issues can be difficult to debug especially if the console has
dependencies or userspace fails to boot to a shell.

There are also cases like IOMMUs where only built-in drivers are
supported, so deferring probe after initcalls is not needed. The IOMMU
subsystem implemented its own mechanism to handle this using OF_DECLARE
linker sections.

This commit adds makes ending deferred probe conditional on initcalls
being completed or a debug timeout. Subsystems or drivers may opt-in by
calling driver_deferred_probe_check_init_done() instead of
unconditionally returning -EPROBE_DEFER. They may use additional
information from DT or kernel's config to decide whether to continue to
defer probe or not.

The timeout mechanism is intended for debug purposes and WARNs loudly.
The remaining deferred probe pending list will also be dumped after the
timeout. Not that this timeout won't work for the console which needs
to be enabled before userspace starts. However, if the console's
dependencies are resolved, then the kernel log will be printed (as
opposed to no output).

Cc: Alexander Graf 
Signed-off-by: Rob Herring 
---
v3:
- Merged with timeout patch.
- Clarify that deferred_probe_timeout is a debug option.
- Drop the 'optional' param. The only user was pinctrl, so it has to handle
  that functionality.
- Rename function to driver_deferred_probe_check_state
- Added kerneldoc for driver_deferred_probe_check_state
- Print a 1 line warning if stopping deferred probe after initcalls and a
  WARN on timeout.

 .../admin-guide/kernel-parameters.txt |  9 +++
 drivers/base/dd.c | 57 +++
 include/linux/device.h|  2 +
 3 files changed, 68 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index efc7aa7a0670..e83ef4648ea4 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -804,6 +804,15 @@
Defaults to the default architecture's huge page size
if not specified.

+   deferred_probe_timeout=
+   [KNL] Debugging option to set a timeout in seconds for
+   deferred probe to give up waiting on dependencies to
+   probe. Only specific dependencies (subsystems or
+   drivers) that have opted in will be ignored. A timeout 
of 0
+   will timeout at the end of initcalls. This option will 
also
+   dump out devices still on the deferred probe list after
+   retrying.
+
dhash_entries=  [KNL]
Set number of hash buckets for dentry cache.

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 1435d7281c66..f0bc73a71a25 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -224,6 +224,51 @@ void device_unblock_probing(void)
driver_deferred_probe_trigger();
 }

+static int deferred_probe_timeout = -1;
+static int __init deferred_probe_timeout_setup(char *str)
+{
+   deferred_probe_timeout = simple_strtol(str, NULL, 0);
+   return 1;
+}
+__setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
+
+/**
+ * driver_deferred_probe_check_state() - Check deferred probe state
+ * @dev: device to check
+ *
+ * Returns -ENODEV if init is done and all built-in drivers have had a chance
+ * to probe (i.e. initcalls are done), -ETIMEDOUT if deferred probe debug
+ * timeout has expired, or -EPROBE_DEFER if none of those conditions are met.
+ *
+ * Drivers or subsystems can opt-in to calling this function instead of 
directly
+ * returning -EPROBE_DEFER.
+ */
+int driver_deferred_probe_check_state(struct device *dev)
+{
+   if (initcalls_done) {
+   if (!deferred_probe_timeout) {
+   dev_WARN(dev, "deferred probe timeout, ignoring 
dependency");
+   return -ETIMEDOUT;
+   }
+   dev_warn(dev, "ignoring dependency for device, assuming no 
driver");
+   return -ENODEV;
+   }
+   retu

[PATCH v3 0/6] Make deferring probe forever optional

2018-06-28 Thread Rob Herring
This series came out of a discussion on the ARM boot-architecture
list[1] about DT forwards and backwards compatibility issues. There are
issues with newer DTs breaking on older, stable kernels. Some of these
are difficult to solve, but cases of optional devices not having
kernel support should be solvable.

I tested this on a RPi3 B with the pinctrl driver forced off. With this
change, the MMC/SD and UART drivers can function without the pinctrl
driver. I left the dts change out this time.

v2 of this series can be found here[2].

Rob

[1] https://lists.linaro.org/pipermail/boot-architecture/2018-April/000466.html
[2] https://lore.kernel.org/patchwork/project/lkml/list/?series=347413

Rob Herring (6):
  driver core: allow stopping deferred probe after init
  dt-bindings: pinctrl: add a 'pinctrl-use-default' property
  pinctrl: Support stopping deferred probe after initcalls
  iommu: Stop deferring probe at end of initcalls
  iommu: Remove IOMMU_OF_DECLARE
  PM / Domains: Stop deferring probe at the end of initcall

 .../admin-guide/kernel-parameters.txt |  9 +++
 .../bindings/pinctrl/pinctrl-bindings.txt |  6 ++
 drivers/base/dd.c | 57 +++
 drivers/base/power/domain.c   |  2 +-
 drivers/iommu/arm-smmu-v3.c   |  2 -
 drivers/iommu/arm-smmu.c  |  7 ---
 drivers/iommu/exynos-iommu.c  |  2 -
 drivers/iommu/ipmmu-vmsa.c|  3 -
 drivers/iommu/msm_iommu.c |  2 -
 drivers/iommu/of_iommu.c  | 21 +--
 drivers/iommu/qcom_iommu.c|  2 -
 drivers/iommu/rockchip-iommu.c|  2 -
 drivers/pinctrl/devicetree.c  | 15 +++--
 include/asm-generic/vmlinux.lds.h |  2 -
 include/linux/device.h|  2 +
 include/linux/of_iommu.h  |  4 --
 16 files changed, 88 insertions(+), 50 deletions(-)

--
2.17.1
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: Notes on "requiring" separate storage

2018-05-31 Thread Rob Herring
On Thu, May 31, 2018 at 5:08 PM, William Mills  wrote:
> Hello,
>
> My notes on "requiring" (or strongly recommending) separate storage.
>
> A couple of times we have gone around the issue of "is it reasonable to
> require new platforms have separate storage for firmware".
>
> Pros:
> P1) No disk partition plan etc
> P2) Runtime var storage is much easier
>
> Cons:
> C1) Cost
> C2) Boot speed
> C3) Board becomes "state full" and brick-able
> C4) Need Buy-in from board makers
>
> I hope we understand the pros so I won't go deep on those.
>
> WRT C1 & C2: Cost & boot speed
>
> Pocket Beagle ($25) has a 2Kx8 I2C EEPROM built into the SIP (system in
> package).  This allows a single binary SPL to boot from uSD on multiple
> boards (PocketBeagle BBW, BBB, BBBlue, TI EVM, etc) as the EEPROM has
> board ID info.
>
> I looked for other low cost boards with dedicated firmware storage.
> PINE A64-LTS ($32) and SOPINE ($30) have 16MB of SPI flash.
> The original PINE A64 ($15) has no on board storage.
>
> Are there other boards I should look at?
>
> What would this look like if we put:
> * SPL & vars only into SPI flash
> * SPL & U-boot & vars into SPI flash
> * SPL/U-boot or EDK2 + vars into QSPI
>
> I assume 64KB for SPL, <500KB for SPL+Uboot, and 1MB for EDK2.

How much do we need for variable storage? That will be the next
problem if we have any firmware storage. IIRC, UEFI variables can be
up to 32 or 64K each. We'd probably need to restrict this some if we
have limited flash.

If sizes in sysfs are accurate, then this is what I get on my laptop:

$ du --apparent-size -sh /sys/firmware/efi/efivars/
55K /sys/firmware/efi/efivars/

I'd imagine we can get by with quite a bit less.

>
> QSPI:
> 1MB $0.67
> 2MB $0.70
> 16MB $1.62  (PINE A64-LTS)
> 133MHz == 66MB/s
> U-boot, 500KB in 7ms
> EDK2, 1MB in 14ms
>
> SPI:
> 128KB $0.28
> 1MB   $0.33
> 104Mhz == 13MB/s
> SPL only, 64KB in 5ms
> U-boot, 500KB in 38ms
>
>
> I2C:
> ID only 256x8   $0.09
> BBB etc 4Kx8$0.13
> 32bytes @100KHz = 2.5ms
>
> Note that on PocketBeagle the I2C cost is already baked into the SIP so
> you can't subtract this cost from the cost of the SPI flash.  This is a
> legacy issue but does so that buy-in takes time to filter down.
>
> Would a future SIP include an SPI flash?  If you do then you need to fix
> how big.
>
> WRT C3: State-full and brickable
>
> As long as the SOC supports multiple bootmodes and the board adds a
> button or switch to select a recovery mode, this can be handled.
> BeagleBone black boots from eMMC (which is brickable) but holding a
> button at power up causes it to ignore everything in eMMC and boot form
> uSD.  The uSD in this recovery mode need not be EBBR compliant (but I
> would like to allow it to be.)
>
> PocketBeagle like board would need to add a jumper or similar.
>
> C4: Board maker buy-in
>
> Probably the biggest barrier.  Must show value of EBBR on legacy boards
> before people see the value.  But if legacy works OK then why change?

Can we crowdfund free SPI flashes for board makers. ;)

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: Issue#9 Document hardware need (if any)

2018-05-30 Thread Rob Herring
On Wed, May 30, 2018 at 2:07 AM, Udit Kumar  wrote:
>
>
>> -Original Message-----
>> From: Rob Herring [mailto:robherri...@gmail.com]
>> Sent: Tuesday, May 29, 2018 8:32 PM
>> To: Udit Kumar 
>> Cc: Architecture Mailman List ;
>> n...@arm.com; arm.ebbr-disc...@arm.com
>> Subject: Re: Issue#9 Document hardware need (if any)
>>
>> On Fri, May 25, 2018 at 8:47 PM, Udit Kumar  wrote:
>> >
>> >
>> >> -Original Message-
>> >> From: Rob Herring [mailto:robherri...@gmail.com]
>> >> Sent: Friday, May 25, 2018 8:18 PM
>> >> To: Udit Kumar 
>> >> Cc: Architecture Mailman List ;
>> >> n...@arm.com; arm.ebbr-disc...@arm.com
>> >> Subject: Re: Issue#9 Document hardware need (if any)
>> >>
>> >> On Fri, May 25, 2018 at 1:44 AM, Udit Kumar  wrote:
>> >> > Hi
>> >> > At present, I don't see any specific hardware requirement for EBBR
>> >> > except
>> >> ARMv8  CPU. Current document covers it very well.
>> >>
>> >> You have to have block storage. Perhaps UEFI implies that. Boards
>> >> like the CHIP only have raw NAND and a USB connector by default. So is USB
>> MS enough?
>> >
>> > Yes but such need is not must to have EBBR running.
>>
>> I don't follow.
>
> I meant, what hardware features a SOC must have to run EBBR.
> This include IPs/CPU architecture etc.
> NAND/USB could be optional, this is not must for EBBR.

EBBR is about what the distros need/want. Raw NAND is never going to
be supported by distros (in their installers, you can always manually
craft images for NAND). USB is maybe too specific (though SD card is
really the only other choice), but there needs to be either some
removable media or network boot (or perhaps we can say both) for an OS
installer and then there must be a block device to install to. That's
one usecase. The 2nd is that you create an OS image offline and put
the image on either a USB MS device or SD card. So that is just a
subset of the first (unless the first only supports net boot).

>> >> I think being explicit with h/w requirements implied by UEFI would be
>> >> a good thing. If I'm designing a board, I don't want to have to sort
>> >> thru UEFI specs to distill down a bullet list of h/w reqs.
>> >
>> > I like to cover here, all on/off chip components could be IP/peripherals 
>> > needed
>> for EBBR.
>> > Like if USB is present, minimum version of xchi or echi needs to be 
>> > supported
>> by hardware.
>>
>> One look at XHCI or EHCI drivers and the variations across SoCs will tell 
>> you that
>> just specifying those specs is pointless. But it is probably worth saying 
>> something
>> about USB. Perhaps saying USB host
>> port(s) (more than 1?) required and the firmware must support booting from
>> USB.
>
> I put USB as an example.
> Please refer SBSA, which mandate the version of ECHI/XCHI

As Grant said, SBSA has a different focus. IMO, it has the same issue
I mentioned though. Just look at the mess the SBSA uart is.

> should be supported. Also this specify other IPs too like Timer, UART, GiC etc
>
> IMO, such strict hardware requirement for EBBR will not be useful.

Agreed. We'd be kidding ourselves that we have any say in the SoC design.

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [PATCH v2 1/8] driver core: make deferring probe after init optional

2018-05-29 Thread Rob Herring
On Tue, May 29, 2018 at 12:12 AM, Frank Rowand  wrote:
> On 05/24/18 11:18, Mark Brown wrote:
>> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>>
>>> Subsystems or drivers may opt-in to this behavior by calling
>>> driver_deferred_probe_check_init_done() instead of just returning
>>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>>> config to decide whether to continue to defer probe or not.
>>
>> Should userspace have some involvement in this decision?  It knows if
>> it's got any intention of loading modules for example.  Kernel config
>> checks might be good enough, though it's going to be a pain to work out
>> if the relevant driver is built as a module for example.
>>
>
> A parallel issue is that loading an overlay could provide the resource
> that will allow the deferred probe to complete.  (That is, once we
> finish implementing the run time overlays feature.)

I'd like to see an actual example where that could happen. I agree you
could craft it, but would it really be a valid partitioning? For
example, SoC pinctrl, iommu, or power domains defined in an overlay
would not be something valid to apply during kernel boot or after boot
(though putting those into overlays is exactly what Alex wants to do).

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [PATCH v2 1/8] driver core: make deferring probe after init optional

2018-05-25 Thread Rob Herring
On Fri, May 25, 2018 at 7:20 AM, Robin Murphy <robin.mur...@arm.com> wrote:
> On 24/05/18 21:57, Rob Herring wrote:
>>
>> On Thu, May 24, 2018 at 2:00 PM, Greg Kroah-Hartman
>> <gre...@linuxfoundation.org> wrote:
>>>
>>> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>>>>
>>>> Deferred probe will currently wait forever on dependent devices to
>>>> probe,
>>>> but sometimes a driver will never exist. It's also not always critical
>>>> for
>>>> a driver to exist. Platforms can rely on default configuration from the
>>>> bootloader or reset defaults for things such as pinctrl and power
>>>> domains.
>>>> This is often the case with initial platform support until various
>>>> drivers
>>>> get enabled. There's at least 2 scenarios where deferred probe can
>>>> render
>>>> a platform broken. Both involve using a DT which has more devices and
>>>> dependencies than the kernel supports. The 1st case is a driver may be
>>>> disabled in the kernel config. The 2nd case is the kernel version may
>>>> simply not have the dependent driver. This can happen if using a newer
>>>> DT
>>>> (provided by firmware perhaps) with a stable kernel version.
>>>>
>>>> Subsystems or drivers may opt-in to this behavior by calling
>>>> driver_deferred_probe_check_init_done() instead of just returning
>>>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>>>> config to decide whether to continue to defer probe or not.
>>>>
>>>> Cc: Alexander Graf <ag...@suse.de>
>>>> Signed-off-by: Rob Herring <r...@kernel.org>
>>>> ---
>>>>   drivers/base/dd.c  | 17 +
>>>>   include/linux/device.h |  2 ++
>>>>   2 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>>>> index c9f54089429b..d6034718da6f 100644
>>>> --- a/drivers/base/dd.c
>>>> +++ b/drivers/base/dd.c
>>>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>>>driver_deferred_probe_trigger();
>>>>   }
>>>>
>>>> +int driver_deferred_probe_check_init_done(struct device *dev, bool
>>>> optional)
>>>> +{
>>>> + if (optional && initcalls_done) {
>>>
>>>
>>> Wait, what's the "optional" mess here?
>>
>>
>> My intent was that subsystems just always call this function and never
>> return EPROBE_DEFER themselves. Then the driver core can make
>> decisions as to what to do (such as the timeout added in the next
>> patch). Or it can print common error/debug messages. So optional is a
>> hint to allow subsystems per device control.
>
>
> Maybe just driver_defer_probe() might be a more descriptive name? To me,
> calling "foo_check_x()" with a parameter that says "I don't actually care
> about x" is the really unintuitive bit.

All the other (though static or internal to driver core) functions are
prefixed driver_deferred_probe_* so I was trying to remain consistent
there. You're right though, with the timeout it's not just whether
initcalls are done. It's really "get the return value depending on the
core's deferred probe state". So perhaps one of these:

driver_deferred_probe_get_return_val()
driver_deferred_probe_handle_return()

The other option would be a more straight-forward functions that just
returns a bool on whether to continue deferring and leave the return
code handling to the caller:

if (driver_deferred_probe_enabled_for_builtin(dev))
  return -EPROBE_DEFER;
else
  return -ENODEV;

The pinctrl case would look like this:

builtin_only = of_property_read_bool(np_pctldev, "pinctrl-use-default");
if (builtin_only && driver_deferred_probe_enabled_for_builtin(dev))
  return -EPROBE_DEFER;
else if (!builtin_only && driver_deferred_probe_enabled(dev))
  return -EPROBE_DEFER;
else
  return -ENODEV;

I still prefer the former, picking the bike shed color is easier with
the latter.

>>>
>>> The caller knows this value, so why do you need to even pass it in here?
>>
>>
>> Because regardless of the value, we always stop deferring when/if we
>> hit the timeout and the caller doesn't know about the timeout. If we
>> get rid of it, we'd need functions for both init done and for deferred
>> timeout.
>>
>>> And bool values that are not obvious are horrid.  I had to go look this
>>> up when rea

Re: [PATCH v2 1/8] driver core: make deferring probe after init optional

2018-05-24 Thread Rob Herring
On Thu, May 24, 2018 at 1:18 PM, Mark Brown <broo...@kernel.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>
> Should userspace have some involvement in this decision?  It knows if
> it's got any intention of loading modules for example.  Kernel config
> checks might be good enough, though it's going to be a pain to work out
> if the relevant driver is built as a module for example.

I looked into whether we could hook into uevents in some way. If we
knew when all the coldplug events had been handled, that would be
sufficient. But it doesn't look to me like we can tell when that
happens with the uevent netlink socket. I think about the only thing
we can tell is if userspace has opened a socket. I'm not all that
familiar with how the whole sequence works, so other opinions on this
would be helpful.

Also, for this to work with serial consoles, we have to make the
decision before we get to userspace. I couldn't get systemd to create
serial gettys either if we deferred later. There's some dependence on
/dev/console, but I didn't get to the bottom of it.

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


[PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE

2018-05-24 Thread Rob Herring
Now that we use the driver core to stop deferred probe for missing
drivers, IOMMU_OF_DECLARE can be removed.

This is slightly less optimal than having a list of built-in drivers in
that we'll now defer probe twice before giving up. This shouldn't have a
significant impact on boot times as past discussions about deferred
probe have given no evidence of deferred probe having a substantial
impact.

Cc: Will Deacon <will.dea...@arm.com>
Cc: Robin Murphy <robin.mur...@arm.com>
Cc: Joerg Roedel <j...@8bytes.org>
Cc: Marek Szyprowski <m.szyprow...@samsung.com>
Cc: Kukjin Kim <kg...@kernel.org>
Cc: Krzysztof Kozlowski <k...@kernel.org>
Cc: Rob Clark <robdcl...@gmail.com>
Cc: Heiko Stuebner <he...@sntech.de>
Cc: Frank Rowand <frowand.l...@gmail.com>
Cc: linux-arm-ker...@lists.infradead.org
Cc: io...@lists.linux-foundation.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-arm-...@vger.kernel.org
Cc: linux-rockc...@lists.infradead.org
Cc: devicet...@vger.kernel.org
Signed-off-by: Rob Herring <r...@kernel.org>
---
 drivers/iommu/arm-smmu-v3.c|  2 --
 drivers/iommu/arm-smmu.c   |  7 ---
 drivers/iommu/exynos-iommu.c   |  2 --
 drivers/iommu/ipmmu-vmsa.c |  3 ---
 drivers/iommu/msm_iommu.c  |  2 --
 drivers/iommu/of_iommu.c   | 19 +--
 drivers/iommu/qcom_iommu.c |  2 --
 drivers/iommu/rockchip-iommu.c |  2 --
 include/linux/of_iommu.h   |  4 
 9 files changed, 1 insertion(+), 42 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 1d647104bccc..22bdabd3d8e0 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);
 
-IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
 MODULE_AUTHOR("Will Deacon <will.dea...@arm.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 69e7c60792a8..9dd7cbaa3b0c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
 };
 module_platform_driver(arm_smmu_driver);
 
-IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
-IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
-IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
-IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
-IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
-IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
-
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
 MODULE_AUTHOR("Will Deacon <will.dea...@arm.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 85879cfec52f..b128cb4372d3 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1390,5 +1390,3 @@ static int __init exynos_iommu_init(void)
return ret;
 }
 core_initcall(exynos_iommu_init);
-
-IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 40ae6e87cb88..f026aa16d5f1 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1108,9 +1108,6 @@ static void __exit ipmmu_exit(void)
 subsys_initcall(ipmmu_init);
 module_exit(ipmmu_exit);
 
-IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
-IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
-
 MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
 MODULE_AUTHOR("Laurent Pinchart <laurent.pinch...@ideasonboard.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 0d3350463a3f..27377742600d 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -877,7 +877,5 @@ static void __exit msm_iommu_driver_exit(void)
 subsys_initcall(msm_iommu_driver_init);
 module_exit(msm_iommu_driver_exit);
 
-IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
-
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Stepan Moskovchenko <step...@codeaurora.org>");
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 2aac8387717c..1904ccf9fc4e 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -27,9 +27,6 @@
 
 #define NO_IOMMU   1
 
-static const struct of_device_id __iommu_of_table_sentinel
-   __used __section(__iommu_of_table_end);
-
 /**
  * of_get_dma_window - Parse *dma-window property and returns 0 if found.
  *
@@ -98,19 +95,6 @@ int of_get_dma_window(struct device_node *dn, const char 
*prefix, int index,
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);
 
-static bool of_iommu_drive

Re: [PATCH 2/2] Add PDF output

2018-05-16 Thread Rob Herring
On Mon, May 14, 2018 at 6:34 PM, Grant Likely  wrote:
> Use Sphinx-doc to generate PDF output from the source text.  With Sphinx
> installed, a PDF version of the document can be generated by typing:
> 'make latexpdf'
>
> Signed-off-by: Grant Likely 
> ---
>  .gitignore  |   1 +
>  Makefile|  20 +++
>  README.rst  |  56 +++
>  source/conf.py  | 163 
> 
>  source/ebbr.rst |  22 ++--
>  5 files changed, 259 insertions(+), 3 deletions(-)
>  create mode 100644 .gitignore
>  create mode 100644 Makefile
>  create mode 100644 source/conf.py
>
> diff --git a/.gitignore b/.gitignore
> new file mode 100644
> index 000..796b96d
> --- /dev/null
> +++ b/.gitignore
> @@ -0,0 +1 @@
> +/build
> diff --git a/Makefile b/Makefile
> new file mode 100644
> index 000..91bb4be
> --- /dev/null
> +++ b/Makefile
> @@ -0,0 +1,20 @@
> +# Minimal makefile for Sphinx documentation

SPDX tags here and other new files?

> +#
> +
> +# You can set these variables from the command line.
> +SPHINXOPTS=
> +SPHINXBUILD   = sphinx-build
> +SPHINXPROJ= EBBR
> +SOURCEDIR = source
> +BUILDDIR  = build
> +
> +# Put it first so that "make" without argument is like "make help".
> +help:
> +   @$(SPHINXBUILD) -M help "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) 
> $(O)
> +
> +.PHONY: help Makefile
> +
> +# Catch-all target: route all unknown targets to Sphinx using the new
> +# "make mode" option.  $(O) is meant as a shortcut for $(SPHINXOPTS).
> +%: Makefile
> +   @$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
> \ No newline at end of file

^^^

> diff --git a/README.rst b/README.rst
> index 1babcf4..57fac8f 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -13,6 +13,62 @@ expected in September 2018. You can find the current draft 
> text in this
>  repository, but be aware that everything in the draft text is subject to
>  change before an official v1.0 release is published.
>
> +Build Instructions
> +==
> +
> +Requirements
> +
> +
> +* Sphinx version 1.5 or later: http://sphinx-doc.org/contents.html

Why? DT spec only needs 1.2.3 (or its documentation is out of date).

> +* LaTeX (and pdflatex, and various LaTeX packages)
> +
> +On Debian and Ubuntu
> +
> +::
> +
> +  # apt-get install python-sphinx texlive texlive-latex-extra 
> libalgorithm-diff-perl \
> +texlive-humanities texlive-generic-recommended 
> texlive-generic-extra
> +
> +If the version of python-sphinx installed is too old, then an additional

Specifically, this is not needed on ubuntu 17.10 and later.

> +new version can be installed with the Python package installer::
> +
> +  $ apt-get install python-pip
> +  $ pip install --user --upgrade Sphinx
> +  $ export SPHINXBUILD=~/.local/bin/sphinx-build
> +
> +Export SPHINXBUILD (see above) if Sphinx was installed with pip --user, then 
> follow Make commands below
> +
> +On Mac OS X
> +^^^
> +
> +* Install MacTeX_
> +* Install pip if you do not have it::
> +
> +  $ sudo easy_install pip
> +
> +* Install Sphinx::
> +
> +  $ pip install --user --upgrade Sphinx
> +
> +.. _MacTeX: http://tug.org/mactex
> +
> +Make Targets
> +
> +
> +To generate PDF::
> +
> +  $ make latexpdf
> +
> +To generate hierarchy of HTML pages::
> +
> +  $ make html
> +
> +To generate a single HTML page::
> +
> +  $ make singlehtml
> +
> +Output goes in `./build` subdirectory.
> +
>  License
>  ===
>
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [RFC PATCH] driver core: make deferring probe forever optional

2018-05-07 Thread Rob Herring
On Mon, May 7, 2018 at 1:31 PM, Bjorn Andersson
<bjorn.anders...@linaro.org> wrote:
> On Tue 01 May 14:31 PDT 2018, Rob Herring wrote:
>
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>
> But how do you know if this is the case?

Because the platform worked before adding the dependency in the dts.

>> This is often the case with initial platform support until various drivers
>> get enabled.
>
> Can you please name platform that has enough support for Alexander to
> care about backwards and forwards compatibility but lacks a pinctrl
> driver.

Alex will have to answer that. I do agree pinctrl drivers shouldn't be
that hard because it is all just translating a bunch of pin data into
one-time (mostly) register writes, so it shouldn't take that long to
implement support. OTOH, maybe a pinctrl driver is low priority
because nothing needs it yet. Either a given board works with the
defaults and only some new board needs to change things or you don't
need pinctrl until low power modes are implemented. However, power
domains have the same problem and it can take years for those to get
proper support.

Alex proposed declaring dts files stable and then enforcing
compatibility after that point. If anyone believes that will work,
then please send a patch marking all the platforms in the kernel tree
that are stable.

>> There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config.
>
> I agree that there is a chance that you _might_ get some parts of the
> system working by relying on the boot loader configuration, but
> misconfiguration issues applies to any other fundamental providers, such
> as clocks, regulators, power domains and gpios as well.

If it is only a chance, then perhaps we shouldn't allow things
upstream without proper drivers for all these things. That will only
give users the wrong impression.

>> The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>
> As above, this is in no way limited to pinctrl drivers.

Yes, I wasn't trying to imply that with this patch. I was just
starting with 1 example. IOMMUs (which essentially is already doing
what this patch does) and power domains are the main other 2. Clocks
is an obvious one too, but from the discussion I referenced that
problem is a bit different as platforms change from dummy fixed-clocks
to a real clock controller driver. That will need a different
solution.

>> Unfortunately, this change breaks with modules as we have no way of
>> knowing when modules are done loading. One possibility is to make this
>> opt in or out based on compatible strings rather than at a subsystem level.
>> Ideally this information could be extracted automatically somehow. OTOH,
>> maybe the lists are pretty small. There's only a handful of subsystems
>> that can be optional, and then only so many drivers in those that can be
>> modules (at least for pinctrl, many drivers are built-in only).
>>
>
> On the Qualcomm platform most drivers are tristate and on most platforms
> there are size restrictions in the proprietary boot loader preventing us
> from boot the kernel after switching all these frameworks from tristate
> to bool (which feels like a more appropriate solution than your hack).

BTW, QCom platforms are almost the only ones with pinctrl drivers as
modules. They are also happen to be PIA to configure correctly for a
board.

However, I would like a solution that works with modules. It would be
nice to know when userspace finished processing all the coldplug
uevents. That would be sufficient to support modules. I researched
that a bit and it doesn't seem the kernel can tell when that has
happened.

>
>> Cc: Alexander Graf <ag...@suse.de>
>> Signed-off-by: Rob Herring <r...@kernel.org>
>> ---
>> This patch came out of a discussion on the ARM boot-architecture
>> list[1] about DT forwards and backwards compatibility issues. There are
>> issues with newer DTs breaking on older, stable kernels. Some of these
>> are difficult to solve, but cases of optional devices not having
>> kernel support should be solvable.
>>
>
> There are two cases here:
> 1) DT contains compatibles that isn't supported by the kernel. In this
> case the

Re: [Arm.ebbr-discuss] Booting from eMMC without the boot protocol

2018-05-07 Thread Rob Herring
On Mon, May 7, 2018 at 12:17 PM, William Mills <wmi...@ti.com> wrote:
>
>
> On 05/07/2018 11:49 AM, Rob Herring wrote:
>> On Fri, May 4, 2018 at 1:41 PM, William Mills <wmi...@ti.com> wrote:
>>>
>>> On 05/04/2018 01:03 PM, Andreas Färber wrote:
>>>> Am 04.05.2018 um 18:50 schrieb Alexander Graf:
>>>>> On 05/04/2018 06:20 PM, William Mills wrote:
>>>>>> On 05/04/2018 11:45 AM, Alexander Graf wrote:
>>>>>> I am missing something.  If there is only MBR and no GPT then there is
>>>>>> no GUID type for EFI Partition.  So how does the firmware find the "EFI
>>>>>> Partition" and the default /efi/boot/boot*.efi file?  Does it just use
>>>>>> the partition with boot flag?
>>>>>
>>>>> There is a special partition ID for the ESP (0xEF), but U-Boot currently
>>>>> just searches on every partition that's marked bootable.
>>>>
>>>> ... and falls back to the first partition if none on the device is
>>>> marked bootable.
>>>>
>>>
>>> Thanks guys, that makes sense.
>>>
>>> So the running summary in my head looks like this:
>>>
>>> Case #1: Platforms that have separate storage for firmware and OS
>>>
>>> OS storage is simple std GPT with no funny MBR stuff.
>>> One image may work on many boards.
>>>
>>> Case #2: Platforms where firmware and OS share single storage
>>>
>>> Storage uses either GPT or MBR (but only one) based on board need.
>>> Image is dedicated for one machine type (or closely related set).
>>> If raw mode is needed use MBR with 2MB reserved space
>>> OS should not touch reserved space in MBR
>>> If using GPT, firmware should place needed files in either:
>>> * Vendor registered dir in EFI partition
>>> * GUID identified partitions w/ attribute bit 0 set
>>> (can fallback to using names if GUID not found)
>>
>> This is for all the other stuff vendors stick in custom partitions if
>> we're lucky or some fixed offset if we're not, right?
>>
>
> Yes.  Today people seem to be using a GPT partition for each of these
> low level objects and seem to be using the partition name to identify
> them.  They are using very generic names like "boot".
>
> This means that the disk image will only work with one SOC or perhaps
> even one board.
>
> If people are going to use raw GPT partitions I think they should at
> least use a GUID GPT partition type to give certainty.  Falling back to
> a name is OK and will help the casual user.
>
> Bit 0 of the GPT attributes table is listed as "Platform required".
> Do current installers leave these partitions alone?

I'll leave that to one of the OS folks.

> However, i would prefer N files in an EFI dir rather than tons of
> special partitions.  See below.

Yes, I certainly agree. I'd extend that to bootloader specific files
that are just containers of files and config: uImage, FIT image,
Android boot image, Android dtbo image

>>> Should we consider a future case #3?
>>> Case #3: New Platforms where firmware and OS share storage
>>>
>>> Storage uses std GPT with no exceptions.
>>> Image may work with N preknown boards that are unrelated.
>>> Firmware files are all in a vendor specified dir in the EFI partition
>>
>> Firmware includes the bootrom and the loaded firmware (UEFI or ATF)
>> itself. This would mean the bootrom has to read a FAT filesystem. I'm
>> assuming you didn't mean that, so we'd still have to allow for
>> partitions. Perhaps we tighten it up with specific GUIDs for specific
>> firmware. That would make firmware updates easier and generically
>> implementable.
>>
>
> I did actually mean that.  Is your doubt based on code size / complexity
> or legal reasons?

Both.

It's certainly not that hard or much code size to do FAT read-only
filesystem support, but it's more that I think vendors would be
reluctant to add support. I get push back just on using filesystems in
2nd stage bootloaders on Android. I would love to see bootrom's more
standardized, but I'm doubtful.

> TI has put FAT in its ROMs for years (granted w/o long name support.)
> Sure early versions of that code had some quicks but the complexity is
> manageable.

That's good to know. Is that opensource?

> My understanding is that Microsoft has a special grant of its IP for EFI
> partitions so I would think people would be comfortable at least for the
> EFI FAT partition. ( IANAL ... )

After some further reading on the details, I agree. In any case, I
think all the patents on long filenames have expired now anyway.

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: DT handling, [Ref Linux-Efi]

2018-05-03 Thread Rob Herring
On Thu, May 3, 2018 at 11:11 AM, Rob Herring <r...@kernel.org> wrote:
> On Thu, May 3, 2018 at 9:29 AM, Alexander Graf <ag...@suse.de> wrote:
>> On 04/30/2018 08:36 PM, Rob Herring wrote:
>>>
>>> On Fri, Apr 27, 2018 at 4:39 PM, Alexander Graf <ag...@suse.de> wrote:
>>>>
>>>> Hi Rob,
>>>>
>>>> On 27.04.18 18:40, Rob Herring wrote:
>>>>>
>>>>> On Fri, Apr 27, 2018 at 2:47 AM, Alexander Graf <ag...@suse.de> wrote:
>>>>>>
>>>>>>
>>>>>> On 27.04.18 08:24, Udit Kumar wrote:
>>>>>>>
>>>>>>> Hi
>>>>>>> There is bit of discussion on linux-efi too , to handle DT update
>>>>>>>
>>>>>>> I guess some members of this forum are active there too.
>>>>>>>
>>>>>>> https://www.spinics.net/lists/linux-efi/msg13700.html
>>>>>>>
>>>>>>> To summaries
>>>>>>> 1/ Ownership of DTB
>>>>>>> IMO should be firmware and we should retain this
>>>>>>> ownership in EBBR as well, Any objections/thoughts ?
>>>>>>
>>>>>> I fully agree. On top of that we need to make clear that backward and
>>>>>> forward compatibility are not optional.
>>>>>>
>>>>>> For that I think we may need to actually give people workable solutions
>>>>>> to create device trees that are compatible with multiple levels of
>>>>>> kernel support. The main areas I'm aware of that keep breaking are:
>>>>>>
>>>>>>* fine grained interrupt controller support
>>>>>
>>>>> Do you have an example of that? The only thing I can think of is
>>>>> people switching interrupts from the GIC to an always-on, low-power
>>>>> mode custom interrupt controller.
>>>>
>>>> The last time I've seen that breakage was:
>>>>
>>>>
>>>>
>>>> https://github.com/raspberrypi/linux/blob/rpi-4.14.y/arch/arm/boot/dts/bcm270x.dtsi#L158
>>>>
>>>> which indeed does switch interrupts from the GIC to an interrupt muxer
>>>> behind the GIC.
>>>>
>>>> The problem is that once support for that lands upstream, you will have
>>>> very little option but to break backwards compatibility today.
>>>
>>> This one is unfortunate. It could have been handled better. An
>>> interrupt-map property in the aux ctrlr could have mapped the
>>> interrupts to GIC without any aux driver. Then when the aux driver
>>> lands, it just needs to remove the interrupt-map (on boot).
>>
>>
>> To do that you would've needed to know that you need the change in the first
>> place ;).
>
> Actually, I think we can still solve this. Add the interrupt-map now.
> Then when the aux driver lands it has to fixup the interrupt-map.

Scrap that...

> I think I actually hit the problem when testing my deferred probe
> patch. I saw a 30 sec delay in the console output when the pl011
> driver probed and using the pl011 as the console (they really made a
> mess of the serial console on RPi 3).
>
>>> Alternatively, I skimmed thru some discussions of the issue, but I'm
>>> not clear why the devices behind the aux controller can't all just
>>> treat their interrupts as shared. But that would be a simple change to
>>> the drivers' irq handlers, so I'm probably missing something. If that
>>> worked, then the DT would never need to change.

Fix posted[1]. :) In fact the IRQ handlers are shared already, there's
just a bug in the handler.

Rob

[1] https://patchwork.kernel.org/patch/10378841/
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: DT handling, [Ref Linux-Efi]

2018-05-03 Thread Rob Herring
On Thu, May 3, 2018 at 9:29 AM, Alexander Graf <ag...@suse.de> wrote:
> On 04/30/2018 08:36 PM, Rob Herring wrote:
>>
>> On Fri, Apr 27, 2018 at 4:39 PM, Alexander Graf <ag...@suse.de> wrote:
>>>
>>> Hi Rob,
>>>
>>> On 27.04.18 18:40, Rob Herring wrote:
>>>>
>>>> On Fri, Apr 27, 2018 at 2:47 AM, Alexander Graf <ag...@suse.de> wrote:
>>>>>
>>>>>
>>>>> On 27.04.18 08:24, Udit Kumar wrote:
>>>>>>
>>>>>> Hi
>>>>>> There is bit of discussion on linux-efi too , to handle DT update
>>>>>>
>>>>>> I guess some members of this forum are active there too.
>>>>>>
>>>>>> https://www.spinics.net/lists/linux-efi/msg13700.html
>>>>>>
>>>>>> To summaries
>>>>>> 1/ Ownership of DTB
>>>>>> IMO should be firmware and we should retain this
>>>>>> ownership in EBBR as well, Any objections/thoughts ?
>>>>>
>>>>> I fully agree. On top of that we need to make clear that backward and
>>>>> forward compatibility are not optional.
>>>>>
>>>>> For that I think we may need to actually give people workable solutions
>>>>> to create device trees that are compatible with multiple levels of
>>>>> kernel support. The main areas I'm aware of that keep breaking are:
>>>>>
>>>>>* fine grained interrupt controller support
>>>>
>>>> Do you have an example of that? The only thing I can think of is
>>>> people switching interrupts from the GIC to an always-on, low-power
>>>> mode custom interrupt controller.
>>>
>>> The last time I've seen that breakage was:
>>>
>>>
>>>
>>> https://github.com/raspberrypi/linux/blob/rpi-4.14.y/arch/arm/boot/dts/bcm270x.dtsi#L158
>>>
>>> which indeed does switch interrupts from the GIC to an interrupt muxer
>>> behind the GIC.
>>>
>>> The problem is that once support for that lands upstream, you will have
>>> very little option but to break backwards compatibility today.
>>
>> This one is unfortunate. It could have been handled better. An
>> interrupt-map property in the aux ctrlr could have mapped the
>> interrupts to GIC without any aux driver. Then when the aux driver
>> lands, it just needs to remove the interrupt-map (on boot).
>
>
> To do that you would've needed to know that you need the change in the first
> place ;).

Actually, I think we can still solve this. Add the interrupt-map now.
Then when the aux driver lands it has to fixup the interrupt-map.

I think I actually hit the problem when testing my deferred probe
patch. I saw a 30 sec delay in the console output when the pl011
driver probed and using the pl011 as the console (they really made a
mess of the serial console on RPi 3).

>> Alternatively, I skimmed thru some discussions of the issue, but I'm
>> not clear why the devices behind the aux controller can't all just
>> treat their interrupts as shared. But that would be a simple change to
>> the drivers' irq handlers, so I'm probably missing something. If that
>> worked, then the DT would never need to change.
>>
>> I guess whether this could have been handled better depends if folks
>> knowingly ignored the issue or this was found after upstreaming
>> support. The latter case may be unavoidable, but maybe we can make it
>
>
> I think most of these cases are the latter.
>
>> rare enough we only need overlays in some exceptions. Whether we try
>> to be stricter and do better up front or have some overlay based
>> solution, either one is going to require folks be aware of the issues
>> and effort to avoid them.
>
>
> Yes, but with the overlay approach we can temper it up after the fact :).

I was assuming you did not want to be the one to find all the issues
and fix them. You do want upstream to do a better job and avoid some
of the issues to begin with, right?

> I personally think what we need to do is have a flag day event. I think
> ideally that would be a move of the dts file out of the Linux tree into a
> common git repository. Once it's there, we'd need scripts that ensure
> backward compatibility all the way back to whatever was current at the flag
> day.

Well, we wouldn't move over platforms that don't have pinctrl, clocks,
etc. because they aren't stable and many of your problems go away. You
would also still have the same problem that the DT declared stable at
the flag day is newer than the kern

Re: [Arm.ebbr-discuss] DT handling, [Ref Linux-Efi]

2018-05-03 Thread Rob Herring
On Thu, May 3, 2018 at 5:13 AM, Daniel Thompson
 wrote:
> On Wed, May 02, 2018 at 05:39:02PM -0400, Tom Rini wrote:
>> On Wed, May 02, 2018 at 05:12:03AM +, Chang, Abner (HPS SW/FW 
>> Technologist) wrote:
>>
>> > > -Original Message-
>> > > From: Udit Kumar [mailto:udit.ku...@nxp.com]
>> > > Sent: Wednesday, May 02, 2018 12:26 PM
>> > > To: Chang, Abner (HPS SW/FW Technologist) ;
>> > > Alexander Graf ; William Mills 
>> > > Cc: boot-architecture@lists.linaro.org; n...@arm.com; Rod Dorris
>> > > ; arm.ebbr-disc...@arm.com
>> > > Subject: RE: DT handling, [Ref Linux-Efi]
>> > >
>> > > > We probably don't need to provide a genetic DT driver in UEFI U-Boot,
>> > > > instead, we will have to mention how SoC/platform vendors publish
>> > > > DTB/DTBO in EBBR spec.
>> > > > For example,
>> > > > The EFI_CONFIGURATION_TABLE in EFI System table could be used to
>> > > > publish the pointer to DTB and DTBO. Declare two GUIDs in EBBR, one
>> > > > for DTB another for DTBO. OS boot loader is responsible to merge
>> > > > DTB/DTBO according DTB/DTBO discovered in EFI Configuration Table. To
>> > > > read DT from EFI variable into memory, memory map to DT in EEPROM or
>> > > > other mechanisms is platform implementation. No matter which approach,
>> > > > DT producer has to create configuration table in EFI system table
>> > > > follow the data structure defined in EBBR.
>> > > > Another way instead of using GUID in configuration table is to publish
>> > > > DTB/DTBO in EFI device path, this way is more UEFI oriented IMO.
>> > > > However, we have to defined corresponding device path node in UEFI
>> > > > spec for DT. Similar to using system configuration table. DT producer
>> > > > has to install EFI device path for either DTB or DTBO. Then OS boot
>> > > > loaders locate those EFI device paths of DTB and DTBO and merge it.
>> > >
>> > > We are adding a requirement on OS boot loaders to merge it.
>> > > IMO, merging should be done by firmware itself.
>> > > In case, we want to host multiple distribution at same time, then this 
>> > > is likely
>> > > to go with OS boot loaders
>> >
>> > That is fine to merge DT by firmware, we still can standardize how
>> > UBoot merges DT in EBBR. For example, SoC and other platform UBoot
>> > drivers produce their DT or DTO in their own drivers. UBoot provides a
>> > centralized EFI DT driver to collect DT/DTO from either EFI system
>> > configuration table or EFI device path. Then this centralized EFI DT
>> > driver produces the pointer to point to final DT in EFI system
>> > configuration table. OS boot loader cab just check EFI system
>> > configuration table to retrieve DT, something like this.
>>
>> I think I need to step in here to clarify something.  U-Boot drivers
>> don't produce a DT and while it's possible, it's generally[1] not done,
>> that U-Boot uses _the_ device tree that we pass along to the next stage
>> (we've likely filtered things out for space and added a few specific
>> things of our own).
>>
>> IMHO, what EBBR should cover is saying that firmware may apply overlays
>> because we know there's N valid use cases of taking a base and fixing it
>> up in both big and small ways.  Then firmware will pass along to the
>> next stage (EFI application such as GRUB or *BSD loader or a Linux
>> Kernel or ...).
>
> Which bits of this discussion target level 0 and which target a later
> level 1?
>
> Personally I'm not sure there is enough prior art w.r.t. device tree
> overlay for anything much related to overlays to target level 0.
>
> In fact if we take the view that EBBR defines a contract between the
> system firmware and the OS then arguably DT update is also out of scope
> unless we are defining runtime services by which the OS can update the
> DT. Again not something I think is ready for level 0.

By "DT update" here, did you mean replacing the base DT or still
talking about overlays?

The contract is not just what the firmware provides to the OS, but how
the OS interacts with and configures the firmware. A user telling the
firmware what overlays to apply would be within that scope IMO.

But yes, I also agree probably not a level 0 thing.

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [RFC PATCH] driver core: make deferring probe forever optional

2018-05-02 Thread Rob Herring
On Wed, May 2, 2018 at 6:40 AM, Robin Murphy <robin.mur...@arm.com> wrote:
> On 01/05/18 22:31, Rob Herring wrote:
>>
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Unfortunately, this change breaks with modules as we have no way of
>> knowing when modules are done loading. One possibility is to make this
>> opt in or out based on compatible strings rather than at a subsystem
>> level.
>> Ideally this information could be extracted automatically somehow. OTOH,
>> maybe the lists are pretty small. There's only a handful of subsystems
>> that can be optional, and then only so many drivers in those that can be
>> modules (at least for pinctrl, many drivers are built-in only).
>
>
> Ooh, this is exactly what I wanted for of_iommu_xlate(), and would be much
> nicer than the current bodge using system_state that I ended up with. The
> context there is very similar - if the device has a parent IOMMU then we
> want to wait for that to probe first if possible, but with a deadline such
> that if it doesn't show up then we can go ahead and make progress without it
> (on the assumption that DMA ops can fall back to CMA). The modules problem
> doesn't currently apply to IOMMU drivers either, although we do use a
> special of_device_id table for detecting built-in drivers via
> OF_IOMMU_DECLARE() to avoid deferring at all when we know it would be
> pointless - a more generic solution for that could certainly be useful too.

Ah, so you kept the IOMMU_OF_DECLARE() but it does nothing but define
a table. We already have the driver match table which should pretty
much be the same data, so it would be better if we could use that if
possible. If we used MODULE_DEVICE_TABLE somehow, we could avoid
modifying lots of drivers. Though many built-in only drivers omit
that. The other problem is it would become a large set of tables to
search thru because it is global. That would probably end up slower
than just deferring. So we need something like
_DEVICE_TABLE() to have per subsystem tables. Then this
function in this patch would need to be told which table to use.
However, this is all really just an optimization to avoid deferring at
all and could be addressed later. Is there any data on how much time
you save avoiding deferring? This has come up in the past and I don't
think it is much.

I've also been thinking about if we could use MODULE_DEVICE_TABLE to
provide a list compatible strings from modules as a whitelist of
devices to keep deferring probe on. That would require building
modules to build the kernel which I don't think would work. I think my
conclusion is that the cases we care about may be short enough to just
manually maintain such a list.

>
> I agree with Greg that the naming here is a bit tricky - FWIW my initial
> thought would be something like driver_defer_until_init(), which is still
> clunky but at least imperative.

Yeah, I didn't give it much thought as I expected more comments on the
general idea... Maybe my poor naming is a good diversion. :)

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: DT handling, [Ref Linux-Efi]

2018-04-27 Thread Rob Herring
On Fri, Apr 27, 2018 at 2:47 AM, Alexander Graf  wrote:
>
>
> On 27.04.18 08:24, Udit Kumar wrote:
>> Hi
>> There is bit of discussion on linux-efi too , to handle DT update
>>
>> I guess some members of this forum are active there too.
>>
>> https://www.spinics.net/lists/linux-efi/msg13700.html
>>
>> To summaries
>> 1/ Ownership of DTB
>> IMO should be firmware and we should retain this
>> ownership in EBBR as well, Any objections/thoughts ?
>
> I fully agree. On top of that we need to make clear that backward and
> forward compatibility are not optional.
>
> For that I think we may need to actually give people workable solutions
> to create device trees that are compatible with multiple levels of
> kernel support. The main areas I'm aware of that keep breaking are:
>
>   * fine grained interrupt controller support

Do you have an example of that? The only thing I can think of is
people switching interrupts from the GIC to an always-on, low-power
mode custom interrupt controller.

>   * clock support

Are there cases other than going from fixed, fake clocks to a real
clock controller node. I'm inclined to stop allowing people to do
that. A better way this could be done is just provide a clock
controller driver with a bunch of fixed clocks. Then the switch from
the dumb driver to the real driver is just a kernel change.

>   * power domain support

Example?

>   * pinctrl support

This would be the firmware initially does all the pin setup, then you
move it to DT and drop the setup from the firmware? Otherwise I don't
understand the problem in this case. We'd start with no pinctrl and
then add it to the DT. Why wouldn't the kernel just ignore it?

> Every time a device tree changes in any of the above, that usually ends
> up in backwards incompatibility.

TBC, you're talking about new dtb with old kernels. We've mainly cared
about old dtbs and new kernels. So first we should agree the former is
important too. I do, because simply you wouldn't want a BIOS update to
make your PC stop booting your already installed OS.

I'd like to solve this with policy and good practice before we try to
apply technical solutions on top of a mess.

> My idea to solve that would be to basically create a device tree that
> has self-contained overlays that only trigger when certain feature
> strings are available. That way the base device tree could for example
> contain fixed clocks, but an overlay can get applied when the clock
> driver is enabled in the kernel configuration. That overlay would then
> enable the kernel to drive clocks.

The number of combinations that would create makes me run away. Then
we're going to combine that with all the other ways people want to use
overlays.

> Further down, we could even extend dtc with annotations that indicate
> "this property should only be exposed when feature string X is
> available" to not force people to write overlays inside the device tree.

How would that work with clocks where you are changing:

clocks = <_fixed_clk>;

to:

clocks = <_clock_ctrlr 123>;

>
>>
>> Update
>> 1/ Updating whole device tree from OS
>> [Capsule update can be used ]
>
> I think the device tree should be part of firmware. If you need to
> update it, update your firmware (or a firmware specific method, not
> specified by EBBR).
>
>> 2/ Just modifying the device tree DTBO
>
> Yes, dtbo support in the boot chain definitely makes sense.
>
>> My preferred way to handle DTBO in firmware will be
>> https://source.android.com/devices/architecture/dto/multiple
>> See picture Runtime DTO implementation for multiple DTs

The Android way of handling overlays is very much rooted in how the
Android ecosystem works.

We should probably have wider discussion and decision on to what
extent does EBBR address/care about/work with Android? On the one
hand, I don't think Android requires anything that's specifically
incompatible with EBBR if some wants to follow EBBR and use Android.
OTOH, we can't define any requirements for Android in EBBR. Google
will define things to the extent they want and vendors will follow
that only to the extent they have to.

>> To store this information in partition, options we have
>> 1/ Run time variables
>
> You mean EFI variables? We could certainly have a driver in firmware
> that reads certain EFI variables to apply dtbos from.
>
>> 2/ Some driver in Linux writing to DTBO partition
>
> What is a DTBO partition?

The Android way. Everything can be solved with another partition. :)

Rob
___
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [PROPOSAL] ARM/FDT: passing multiple binaries to a kernel

2013-09-13 Thread Rob Herring
On Fri, Sep 13, 2013 at 5:13 AM, Grant Likely grant.lik...@secretlab.ca wrote:
 On Wed, Sep 4, 2013 at 5:41 PM, Rob Herring robherri...@gmail.com wrote:
 For u-boot Andre has proposed some syntactic sugar over the fdt
 command to make boot.scr more trivial to use. We would of course need to
 implement support for using it in the relevant distro tools (but they
 tend to be very distro/machine specific already, e.g. Debian's
 flash-kernel)

 And being machine specific is a PITA. flash-kernel is certainly not
 something we want to expand on. There is not much love for boot.scr
 either. There is work to address what are not really machine
 differences, but largely vendor u-boot differences:

 http://www.mail-archive.com/u-boot@lists.denx.de/msg119025.html

 One option for u-boot which already supports syslinux style menu files
 is to adopt the syslinux multiboot parsing support:

 http://www.syslinux.org/wiki/index.php/Doc/mboot

 Even building it into U-Boot is problematic because it leaves older
 machines out in the cold. Leif's port of Grub to U-Boot is far more
 interesting since the distro can now be in control of the code that
 loads the images and jumps into the kernel/hypervisor.

Considering there is no distro support for grub on ARM yet, it may be
more interesting in the long run, but it is not for the short term. So
there needs to be something that is supportable on both u-boot and
grub (or any other bootloader).


 We need to back-up and consider what this looks like in the end for
 all the pieces and get input from folks on grub, UEFI, and armv8. The
 UEFI answer may be this is a grub problem. For armv8, this proposal
 does match up well as the kernel boot interface for v8 is DT. Despite
 some claims, ACPI will not completely replace DT because of this.

 Yes, for UEFI it is absolutely an OS loader problem. UEFI provides an
 API and runtime environment. Grub is in general moving towards a boot
 menu system and a tool for loading images. Actual booting however
 should be done by a separate OS loader application. For Linux, this
 will be an in-kernel UEFI Stub. For Xen I would recommend taking the
 Linux EFI stub code and doing the same thing. There really isn't a
 need for a multiboot spec when you can rely on a runtime execution
 environment for setting things up exactly as you want them.

You've lost me as well. How do you see the flow working with UEFI for
a user running bare metal OS, installing Xen, and rebooting running
Xen.

Rob

___
boot-architecture mailing list
boot-architecture@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/boot-architecture


Re: [PROPOSAL] ARM/FDT: passing multiple binaries to a kernel

2013-09-03 Thread Rob Herring
On Tue, Sep 3, 2013 at 10:53 AM, Andre Przywara
andre.przyw...@linaro.org wrote:
 Hi,

 a normal Linux kernel currently supports reading the start and end address
 of a single binary blob via the FDT's /chosen node.
 This will be interpreted as the location of an initial RAM disk.

 The Xen hypervisor itself is a kernel, but needs up to _two_ binaries for
 proper operation: a Dom0 Linux kernel and it's associated initrd.
 On x86 this is solved via the multiboot protocol used by the Grub
 bootloader, which supports to pass an arbitrary number of binary modules to
 any kernel.

 Since in the ARM world we have the versatile device tree, we don't need to
 implement the mulitboot protocol.

But surely there would be some advantage of reuse by using the
multi-boot protocol since Xen, grub, and OS tools already support it
for x86.

 So I'd like to propose a new binding which denotes binary modules a kernel
 can use at it's own discretion.
 The need is triggered by the Xen hypervisor (which already uses a very
 similar scheme), but the approach is deliberately chosen to be as generic as
 possible to allow future uses (like passing firmware blobs for devices or
 the like).
 Credits for this go to Ian Campbell, who started something very similar [1]
 for the Xen hypervisor. The intention of this proposal is to make this
 generic and publicly documented.

Can you describe how you see the boot flow working starting with OS
installer writes kernel, initrd, xen and ??? to disk. How does the
bootloader know what to load? The OS may not have access to the dtb,
so this has to be described to the bootloader as well.


 Looking forward to any comments!

 Thanks,
 Andre.

 [1]
 http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/arm/device-tree/booting.txt;h=94cd3f18a4e1317a35e1255bf5c6e1e091001d1a;hb=HEAD
 
 * Multiple boot modules device tree bindings

 Boot loaders wanting to pass multiple additional binaries to a kernel shall
 add a node module for each binary blob under the /chosen node with the
 following properties:

 - compatible:
 compatible = boot,module;
   A bootloader may add names to more specifically describe the module,
   e.g. Xen may use xen,dom0-kernel or xen,dom0-ramdisk.
   If possible a kernel should be able to use modules even without a
   descriptive naming, by enumerating them in order and using hard-coded
   meanings for each module (e.g. first is kernel, second is initrd).

 - reg: specifies the base physical address and size of a region in
   memory where the bootloader loaded the respective binary data to.

 - bootargs:
   An optional property describing arguments to use for this module.
   Could be a command line or configuration data.

 Example:
 /chosen {
 #size-cells = 0x1;
 #address-cells = 0x1;
 module@0 {
 compatible = xen,linux-zimage, xen,multiboot-module,
 boot,module;
 reg = 0x8000 0x003dcff8;
 bootargs = console=hvc0 earlyprintk ro root=/dev/sda1 nosmp;
 };
 module@1 {
 compatible = xen,linux-initrd, xen,multiboot-module,
 boot,module;
 reg = 0x0800 0x00123456;
 };

This has to be created and parsed typically in FDT format by early
boot code, and I worry about the complexity this has. Being future
proof and extensible is good, but we could meet today's needs with
something simple like this:

bootargs = xen args --- linux args;
xen,linux-image = start size;

So, is having a more generic solution really needed?

Rob

___
boot-architecture mailing list
boot-architecture@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/boot-architecture