Re: [PATCH] dt/bindings: fwu-mdata-mtd: drop changes outside FWU
On 11/04/2023 01:21, jaswinder.si...@linaro.org wrote: > From: Jassi Brar > > Any requirement of FWU should not require changes to bindings > of other subsystems. For example, for mtd-backed storage we > can do without requiring 'fixed-partitions' children to also > carry 'uuid', a property which is non-standard and not in the > bindings. > > There exists no code yet, so we can change the fwu-mtd bindings > to contain all properties within the fwu-mdata node. > > Signed-off-by: Jassi Brar > --- > > Hi Rob, Hi Krzysztof, > > I was suggested, and I agree, it would be a good idea to get your blessings > for the location and meta-data (fwu-mdata) bindings for the FWU. > > The FWU images can be located in GPT partitions or MTD backed storage. > The basic bindings for fwu-mdata has already been merged in uboot (ideally > they > too should have had your review). Now I am trying to fully support MTD backed > storage and hence looking for your review. The proposed bindings are totally > self-contained and don't require changes to any other subsystem. > > Thanks. I think we do not review U-Boot bindings usually, except these put in the Linux kernel. There were few targeting U-Boot specifically (e.g. Documentation/devicetree/bindings/mtd/partitions/u-boot.yaml and Documentation/devicetree/bindings/nvmem/u-boot,env.yaml) so if you want our blessing, the bindings should be done in Linux kernel repo. I am pretty sure that reviewing other project bindings would be too much of work for me. Best regards, Krzysztof
Re: [PATCH 1/3] dt-bindings: misc: esm: Add ESM support for TI K3 devices
On 14/04/2023 12:52, Neha Malcom Francis wrote: > Document the binding for TI K3 ESM (Error Signaling Module) block. > > Signed-off-by: Neha Malcom Francis > --- > .../devicetree/bindings/misc/esm-k3.yaml | 54 +++ > 1 file changed, 54 insertions(+) > create mode 100644 Documentation/devicetree/bindings/misc/esm-k3.yaml > > diff --git a/Documentation/devicetree/bindings/misc/esm-k3.yaml > b/Documentation/devicetree/bindings/misc/esm-k3.yaml > new file mode 100644 > index ..5e637add3b0e > --- /dev/null > +++ b/Documentation/devicetree/bindings/misc/esm-k3.yaml Filename matching compatible. Missing vendor prefix and device name. > @@ -0,0 +1,54 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright (C) 2022 Texas Instruments Incorporated > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/misc/esm-k3.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Texas Instruments K3 ESM Binding Drop: Binding > + > +maintainers: > + - Neha Malcom Francis > + > +description: | > + The ESM (Error Signaling Module) is an IP block on TI K3 devices > + that allows handling of safety events somewhat similar to what interrupt > + controller would do. The safety signals have their separate paths within > + the SoC, and they are handld by the ESM, which routes them to the proper typo: handled > + destination, which can be system reset, interrupt controller, etc. In the > + simplest configuration the signals are just routed to reset the SoC. There is no proper bindings directory for ESM? Misc is discouraged. > + > +properties: > + compatible: > +const: ti,j721e-esm > + > + reg: > +items: > + - description: physical address and length of the registers which > + contain revision and debug features Drop useless "physical address and length of the registers which". reg cannot be anything else. > + - description: physical address and length of the registers which > + indicate strapping options > + > + ti,esm-pins: > +$ref: /schemas/types.yaml#/definitions/uint32-array > +description: | Do not need '|' unless you need to preserve formatting. > + integer array of ESM event IDs to route to external event pin which can > + be used to reset the SoC. The array can have an arbitrary amount of > event > + IDs listed on it. What is ESM event ID? The property name suggests pins... > +minItems: 1 > +maxItems: 255 > + > +additionalProperties: false > + > +required: > + - compatible > + - reg > + - ti,esm-pins > + > +examples: > + - | > +main_esm: esm@70 { Drop label. > +compatible = "ti,j721e-esm"; > +reg = <0x0 0x70 0x0 0x1000>; > +ti,esm-pins = <344>, <345>; > +}; Best regards, Krzysztof
Re: [PATCH 1/3] dt-bindings: misc: esm: Add ESM support for TI K3 devices
On 17/04/2023 10:56, Neha Malcom Francis wrote: > Hi Krzysztof > > On 14/04/23 17:10, Krzysztof Kozlowski wrote: >> On 14/04/2023 12:52, Neha Malcom Francis wrote: >>> Document the binding for TI K3 ESM (Error Signaling Module) block. >>> >>> Signed-off-by: Neha Malcom Francis >>> --- >>> .../devicetree/bindings/misc/esm-k3.yaml | 54 +++ >>> 1 file changed, 54 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/misc/esm-k3.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/misc/esm-k3.yaml >>> b/Documentation/devicetree/bindings/misc/esm-k3.yaml >>> new file mode 100644 >>> index ..5e637add3b0e >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/misc/esm-k3.yaml >> >> Filename matching compatible. Missing vendor prefix and device name. >> >>> @@ -0,0 +1,54 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +# Copyright (C) 2022 Texas Instruments Incorporated >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/misc/esm-k3.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Texas Instruments K3 ESM Binding >> >> Drop: Binding >> >>> + >>> +maintainers: >>> + - Neha Malcom Francis >>> + >>> +description: | >>> + The ESM (Error Signaling Module) is an IP block on TI K3 devices >>> + that allows handling of safety events somewhat similar to what interrupt >>> + controller would do. The safety signals have their separate paths within >>> + the SoC, and they are handld by the ESM, which routes them to the proper >> >> typo: handled >> >>> + destination, which can be system reset, interrupt controller, etc. In the >>> + simplest configuration the signals are just routed to reset the SoC. >> >> There is no proper bindings directory for ESM? Misc is discouraged. >> > > There is no other directory I see fit for a block like ESM; it could > either remain in misc/ or maybe create a directory error/ for all error > signaling and correction mechanisms? I see misc/xlnx,sd-fec.txt that > could also go in error/ > > What do you think is fit? I don't know. Maybe it is something like hwmon? Or maybe along with xlnx,sd-fec, tmr-inject and tmr-manager should be moved to some "fault" directory for all fault-management-and-handling hardware? Best regards, Krzysztof
Re: [PATCH] dt-bindings: mtd: Add a schema for binman
On 21/09/2023 20:45, Simon Glass wrote: > Binman[1] is a tool for creating firmware images. It allows you to > combine various binaries and place them in an output file. > > Binman uses a DT schema to describe an image, in enough detail that > it can be automatically built from component parts, disassembled, > replaced, listed, etc. > > Images are typically stored in flash, which is why this binding is > targeted at mtd. Previous discussion is at [2] [3]. > > [1] https://u-boot.readthedocs.io/en/stable/develop/package/binman.html > [2] https://lore.kernel.org/u-boot/20230821180220.2724080-3-...@chromium.org/ > [3] https://www.spinics.net/lists/devicetree/msg626149.html > > Signed-off-by: Simon Glass > --- > > .../bindings/mtd/partitions/binman.yaml | 50 +++ > .../bindings/mtd/partitions/binman/entry.yaml | 61 +++ > .../bindings/mtd/partitions/partitions.yaml | 1 + > MAINTAINERS | 5 ++ > 4 files changed, 117 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/binman.yaml > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/binman/entry.yaml > > diff --git a/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > new file mode 100644 > index 00..c792d5a37b700a > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml > @@ -0,0 +1,50 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright 2023 Google LLC > + > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mtd/partitions/binman.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Binman firmware layout > + > +maintainers: > + - Simon Glass > + > +description: | > + The binman node provides a layout for firmware, used when packaging > firmware > + from multiple projects. For now it just supports a very simple set of > + features, as a starting point for discussion. > + > + Documentation for Binman is available at: > + > + https://u-boot.readthedocs.io/en/latest/develop/package/binman.html > + > + with the current image-description format at: > + > + > https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#image-description-format > + > +properties: > + compatible: > +const: u-boot,binman > + > +required: > + - compatible > + > +additionalProperties: false > + > +examples: > + - | > +firmware { > + binman { > +compatible = "u-boot,binman"; > + > +u-boot { It does not look like you tested the bindings, at least after quick look. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Maybe you need to update your dtschema and yamllint. > + size = <0xa>; > +}; > + > +atf-bl31 { > + offset = <0x10>; > +}; > + }; > +}; > diff --git > a/Documentation/devicetree/bindings/mtd/partitions/binman/entry.yaml > b/Documentation/devicetree/bindings/mtd/partitions/binman/entry.yaml > new file mode 100644 > index 00..8003eb4f1a994f > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/partitions/binman/entry.yaml > @@ -0,0 +1,61 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright 2023 Google LLC > + > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mtd/partitions/binman/entry.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Binman entry > + > +maintainers: > + - Simon Glass > + > +description: | > + The entry node specifies a single entry in the firmware. > + > + Entries have a specific type, such as "u-boot" or "atf-bl31". If the type > + is missing, the name is used as the type. > + > + Note: This definition is intended to be hierarchical, so that entries can > + appear in other entries. Schema for that is TBD. > + > +properties: > + $nodename: > +pattern: "^[-a-z]+(-[0-9]+)?$" Why do you need this? > + > + type: > +$ref: /schemas/types.yaml#/definitions/string > + > + offset: > +$ref: /schemas/types.yaml#/definitions/uint32 > +description: | > + Provides the offset of this entry from the start of its parent section. > + If this is omitted, Binman will determine this by packing the enclosing > + section according to alignment rules, etc. > + > + size: > +$ref: /schemas/types.yaml#/definitions/uint32 > +description: | > + Provides the size of this entry in bytes. If this is omitted, Binman > will > + use the content size, along with any alignment information, to > determine > + the size of the entry. > + Best regards, Krzysztof
Re: [PATCH] dt/bindings: fwu-mdata-mtd: drop changes outside FWU
On 03/05/2023 16:37, Ilias Apalodimas wrote: > Hi Krzysztof, > > On Tue, Apr 11, 2023 at 07:38:11AM +0200, Krzysztof Kozlowski wrote: >> On 11/04/2023 01:21, jaswinder.si...@linaro.org wrote: >>> From: Jassi Brar >>> >>> Any requirement of FWU should not require changes to bindings >>> of other subsystems. For example, for mtd-backed storage we >>> can do without requiring 'fixed-partitions' children to also >>> carry 'uuid', a property which is non-standard and not in the >>> bindings. >>> >>> There exists no code yet, so we can change the fwu-mtd bindings >>> to contain all properties within the fwu-mdata node. >>> >>> Signed-off-by: Jassi Brar >>> --- >>> >>> Hi Rob, Hi Krzysztof, >>> >>> I was suggested, and I agree, it would be a good idea to get your >>> blessings >>> for the location and meta-data (fwu-mdata) bindings for the FWU. >>> >>> The FWU images can be located in GPT partitions or MTD backed storage. >>> The basic bindings for fwu-mdata has already been merged in uboot (ideally >>> they >>> too should have had your review). Now I am trying to fully support MTD >>> backed >>> storage and hence looking for your review. The proposed bindings are totally >>> self-contained and don't require changes to any other subsystem. >>> >>> Thanks. >> >> I think we do not review U-Boot bindings usually, except these put in >> the Linux kernel. There were few targeting U-Boot specifically (e.g. >> Documentation/devicetree/bindings/mtd/partitions/u-boot.yaml and >> Documentation/devicetree/bindings/nvmem/u-boot,env.yaml) so if you want >> our blessing, the bindings should be done in Linux kernel repo. >> >> I am pretty sure that reviewing other project bindings would be too much >> of work for me. > > Sure that makes sense. But an answer here of whether the bindings make > sense to the DT maintainers or not would help to move forward. I am not a DT maintainer of other systems, components etc. Answering anything for these other systems and components means nothing. I will take no responsibility of whatever I say because I will bear no costs of it. :) IOW, to me you can make any invalid binding inside U-Boot and it will not matter for the Linux kernel. It will of course matter to U-Boot in many aspects. > > These bindings are trying to define a standardized interface for A/B > *firmware* > updates [0] which is not what traditionally goes into a device tree. OTOH we > already have some U-Boot specific bindings as you already mentioned. As we > move forward we need to be very precise on what is allowed or not on the DT > since it's now tested and verified on SystemReady certifications. > IOW if > we add those binding in U-Boot only, we would need to strip them before > handing the DT to linux, otherwise certification would fail. Which you can. Or propose to add the bindings to the Linux kernel and to the Linux kernel DTS, which then will get our review. > If you do > think that having them in the kernel repo makes sense, it would help > standardizing other boot loaders (at least it would standardize where that > metadata lives) if they want to implement something similar. I cannot speak for Rob, but that's the only way I can make a review. I cannot review or try to maintain all possible projects in the world and their bindings. How would this even work in practice? > > Just keep in mind we would need a schema per storage medium. IOW this tries > to > standardize devices which keep the firmware binary in an mtd. There's also > another biding which describes firmware files on a GPT [1]. > > [0] https://developer.arm.com/documentation/den0118/a > [1] > https://source.denx.de/u-boot/u-boot/-/blob/master/doc/device-tree-bindings/firmware/fwu-mdata-gpt.yaml Best regards, Krzysztof
Re: [PATCH 1/1] arm64: dts: ti: k3-j721s2: Add reserved status in msmc
On 03/05/2023 16:51, Nishanth Menon wrote: > On 20:17-20230503, Udit Kumar wrote: >> Mark atf, l3-cache and tifs node as reserved. > > why? (I am not reading the cover-letter for a 1 patch) And you should not have to. :) The commit msg should explain why it is useful. Best regards, Krzysztof
Re: [PATCH] dt/bindings: fwu-mdata-mtd: drop changes outside FWU
On 03/05/2023 18:26, Tom Rini wrote: I think we do not review U-Boot bindings usually, except these put in the Linux kernel. There were few targeting U-Boot specifically (e.g. Documentation/devicetree/bindings/mtd/partitions/u-boot.yaml and Documentation/devicetree/bindings/nvmem/u-boot,env.yaml) so if you want our blessing, the bindings should be done in Linux kernel repo. I am pretty sure that reviewing other project bindings would be too much of work for me. >>> >>> Sure that makes sense. But an answer here of whether the bindings make >>> sense to the DT maintainers or not would help to move forward. >> >> I am not a DT maintainer of other systems, components etc. Answering >> anything for these other systems and components means nothing. I will >> take no responsibility of whatever I say because I will bear no costs of >> it. :) IOW, to me you can make any invalid binding inside U-Boot and it >> will not matter for the Linux kernel. It will of course matter to U-Boot >> in many aspects. >> >>> >>> These bindings are trying to define a standardized interface for A/B >>> *firmware* >>> updates [0] which is not what traditionally goes into a device tree. OTOH >>> we >>> already have some U-Boot specific bindings as you already mentioned. As we >>> move forward we need to be very precise on what is allowed or not on the DT >>> since it's now tested and verified on SystemReady certifications. >>> IOW if >>> we add those binding in U-Boot only, we would need to strip them before >>> handing the DT to linux, otherwise certification would fail. >> >> Which you can. >> >> Or propose to add the bindings to the Linux kernel and to the Linux >> kernel DTS, which then will get our review. >> >>> If you do >>> think that having them in the kernel repo makes sense, it would help >>> standardizing other boot loaders (at least it would standardize where that >>> metadata lives) if they want to implement something similar. >> >> I cannot speak for Rob, but that's the only way I can make a review. I >> cannot review or try to maintain all possible projects in the world and >> their bindings. How would this even work in practice? >> >>> >>> Just keep in mind we would need a schema per storage medium. IOW this >>> tries to >>> standardize devices which keep the firmware binary in an mtd. There's also >>> another biding which describes firmware files on a GPT [1]. >>> >>> [0] https://developer.arm.com/documentation/den0118/a >>> [1] >>> https://source.denx.de/u-boot/u-boot/-/blob/master/doc/device-tree-bindings/firmware/fwu-mdata-gpt.yaml > > This is one of the bindings that we need to upstream to > https://github.com/devicetree-org/dt-schema/ Sure, this works as well. Best regards, Krzysztof
Re: [PATCH 00/21] Qualcomm generic board support
On 04/12/2023 14:24, Sumit Garg wrote: > On Mon, 4 Dec 2023 at 16:30, Daniel Thompson > wrote: >> >> On Mon, Dec 04, 2023 at 11:02:57AM +0530, 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: >>> 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. >> >> The kernel is regularly released in multiple forms (including git >> tags and tarball). Why isn't the kernel itself sufficient to be a >> versioned release of the DT bindings directory? >> > > The Linux kernel may come in various forms (upstream vs stable vs > vendor). It's difficult to decide from where the DT bindings should > come from. Should they come from upstream or should they come from the > kernel which is actually booted onto a particular device? > > IOW, as of now which kernel version should u-boot pick up for DT > validation checks? The same version as in case of release from separate DT bindings repo. > > If we can have a separate release cadence for DT bindings then the > platform (firmware/bootloader) can attest the DTB against that. Later > one should be able to boot any kernel with the DTB provided by > platform. Separate releases of DT bindings change nothing here - you are going to ask the same question: "which release shall I take"? So the answer could be the same for both: take latest mainline kernel release. You really don't need separate repo just to know which release to take. Best regards, Krzysztof
Re: [PATCH 00/21] Qualcomm generic board support
On 04/12/2023 15:38, ff wrote: > > >> Le 4 déc. 2023 à 12:00, Daniel Thompson a écrit >> : >> >> On Mon, Dec 04, 2023 at 11:02:57AM +0530, 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: >>> 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. >> >> The kernel is regularly released in multiple forms (including git >> tags and tarball). Why isn't the kernel itself sufficient to be a >> versioned release of the DT bindings directory? >> > The Linux kernel may not see all devices. For instance it could see a PCI > port while the firmware sees a SERDES that is configured to match the board > implementation and touting. The firmware shall be responsible to, statically > or dynamically make those things available to the kernel. > An OS distribution (not necessarily Linux) should not embedded all possible > combinations and know all derived boards. Which is nothing related to the discussion here: bindings. The bindings *MUST* cover PCI port and serdes. P.S. Please wrap your replies to match mailing list style / netiquette. Best regards, Krzysztof
Re: [PATCH 00/21] Qualcomm generic board support
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? > AFAIK, DT bindings should be forwards and backwards > compatible. The same with DTS. > So if you pick up DTS or DTB from any project tree > (upstream kernel or stable kernel or u-boot) then DT schema validation > would ensure that corresponding DTS or DTB doesn't regress the DT > bindings. And why is this argument to decouple DTS from bindings? > > Ideally, it should be more user/CI friendly if DT bindings can be > easily installed alongside devicetree schema tools [1]. > > [1] https://github.com/devicetree-org/dt-schema Does it mean you will work on this? Best regards, Krzysztof
Re: [PATCH 00/21] Qualcomm generic board support
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. > > 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. > DT bindings alone would ease the compliance process for u-boot drivers > in quite similar manner to Linux drivers. > > [1] > https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/bootph.yaml > >> >>> AFAIK, DT bindings should be forwards and backwards >>> compatible. >> >> The same with DTS. >> >>> So if you pick up DTS or DTB from any project tree >>> (upstream kernel or stable kernel or u-boot) then DT schema validation >>> would ensure that corresponding DTS or DTB doesn't regress the DT >>> bindings. >> >> And why is this argument to decouple DTS from bindings? >> > > See above. It's not really explained there. You can pick up DTS from any project and validate it against the repo Rob mentioned or against kernel. Whether DTS is in that repo or not, does not matter for your validation. > >>> >>> Ideally, it should be more user/CI friendly if DT bindings can be >>> easily installed alongside devicetree schema tools [1]. >>> >>> [1] https://github.com/devicetree-org/dt-schema >> >> Does it mean you will work on this? > > I am happy to collaboratively work with DT bindings maintainers and > the u-boot community once we can reach a consensus on the above. > Basically the main motive here is to validate DTS files present in the > u-boot tree and be able to reliably pass them to whichever Linux > kernel version you are trying to boot. IOW, make Linux distros to > reliably boot with devicetree supplied by u-boot. So the answer is "no" on the proposal you mentioned before. Sure, maybe someone will pick it up. Best regards, Krzysztof
Re: [PATCH 0/8] An effort to bring DT bindings compliance within U-boot
On 14/12/2023 20:48, Rob Herring wrote: >> >> I think some of the important questions to ask are, how often / likely >> are the breakages to occur? It seems like these days it's either: >> - U-Boot had an early version of the binding and we already state we >> don't support backwards compatibility here. It should be on the >> maintainer to be proactive in this case. >> - It's a "the DT was wrong about the hardware, sorry not sorry it's an >> incompatible DTS change now". This too is hopefully the kind of thing >> that at least board maintainers will be more actively aware of needing >> to deal with in U-Boot, if it's really a problem. > > A common issue in the kernel is with forward compatibility when > platforms add new resources from a new provider. Then the kernel > expects a driver for the provider and waits for the dependency. Of > course, older kernels don't have that provider driver and so the > dependency is never met. Not sure if u-boot will have similar issues? > At least you should/could know if the provider driver exists or not. > (The kernel doesn't because modules.) If some U-Boot platform decides to aggressively sync with kernel DTS, then probably the kernel subarch maintainer should be aware of it. Mentioned forward compatibility issue is a result of assumption that there are no out-of-tree upstream consumers of our DTS. I am certainly not aware of any such consumer for Samsung. If someone told me (me wearing Samsung hat), hey U-Boot now cares, I would start caring as well. The usual argument of contributors wanting to break the backward and forward compatibility, is "there is no other OS depending on this" (recent discussion with Johan about order of interrupts: https://lore.kernel.org/all/zwcpzpx-et-xh...@hovoldconsulting.com/ ). You need to tell the maintainers of that platform, that now they have other OS/firmware/bootloader depending on DTS compatibility. Best regards, Krzysztof
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On 22/12/2023 07:12, Sumit Garg wrote: > Changes in v2: > -- > - Patch #1: excluded gitab CI config check and added commit description. > - Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/ > - Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/ > - Patch #5: s/U-boot/U-Boot/ > - Patch #6 and #7: Picked up review tags > > Prerequisite > > > This patch series requires devicetree-rebasing git repo to be added as a > subtree to the main U-Boot repo via: > > $ git subtree add --prefix devicetree-rebasing \ > > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git > \ > v6.6-dts --squash > > Background > -- > > This effort started while I was reviewing patch series corresponding to > Qcom platforms [1] which was about to import modified devicetree source > files from Linux kernel. I suppose keeping devicetree files sync with > Linux kernel without any DT bindings schema validation has been a pain > for U-Boot SoC/platform maintainers. There has been past discussions > about a single DT repo but that hasn't come up and Linux kernel remained > the place where DT source files as well as bindings are placed and > maintained. Thanks for doing this. I really suggest to store information that kernel DTS is directly re-used, thus DTS backward and forward compatibility matters, also in Linux kernel sources. The point is that sub-arch maintainers should be aware of it. I don't think that as DT maintainers we can efficiently keep an eye on it. Maybe create a subsystem profile and include it to maintainer entries of such affected platforms? Best regards, Krzysztof Best regards, Krzysztof
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On 22/12/2023 14:43, Sumit Garg wrote: > On Fri, 22 Dec 2023 at 13:48, Krzysztof Kozlowski > wrote: >> >> On 22/12/2023 07:12, Sumit Garg wrote: >>> Changes in v2: >>> -- >>> - Patch #1: excluded gitab CI config check and added commit description. >>> - Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/ >>> - Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/ >>> - Patch #5: s/U-boot/U-Boot/ >>> - Patch #6 and #7: Picked up review tags >>> >>> Prerequisite >>> >>> >>> This patch series requires devicetree-rebasing git repo to be added as a >>> subtree to the main U-Boot repo via: >>> >>> $ git subtree add --prefix devicetree-rebasing \ >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git >>> \ >>> v6.6-dts --squash >>> >>> Background >>> -- >>> >>> This effort started while I was reviewing patch series corresponding to >>> Qcom platforms [1] which was about to import modified devicetree source >>> files from Linux kernel. I suppose keeping devicetree files sync with >>> Linux kernel without any DT bindings schema validation has been a pain >>> for U-Boot SoC/platform maintainers. There has been past discussions >>> about a single DT repo but that hasn't come up and Linux kernel remained >>> the place where DT source files as well as bindings are placed and >>> maintained. >> >> Thanks for doing this. >> >> I really suggest to store information that kernel DTS is directly >> re-used, thus DTS backward and forward compatibility matters, also in >> Linux kernel sources. The point is that sub-arch maintainers should be >> aware of it. I don't think that as DT maintainers we can efficiently >> keep an eye on it. Maybe create a subsystem profile and include it to >> maintainer entries of such affected platforms? >> > > From U-Boot point of view, currently we have the config option: > "CONFIG_OF_UPSTREAM=y" per platform which means directly re-use of > kernel DTS. So U-Boot sub-arch maintainers should be aware of > platforms which get converted to re-use kernel DTS. I was speaking about kernel. > > I suppose we have to relay information to kernel sub-arch maintainers > who aren't the same as maintaining U-Boot counterparts. How about > adding U-Boot ML to CC for whichever DT change gets submitted in the And every other project? Just setup lei filters. > kernel? Otherwise adding U-Boot sub-arch maintainers as reviewers for > corresponding kernel DT changes works too if that's acceptable. You just entirely ignored my proposal without addressing it... ok let it be. No, CC-ing U-boot maintainers changes nothing because as I said, I want kernel maintainers and contributors to be aware. Best regards, Krzysztof
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On 22/12/2023 16:45, Conor Dooley wrote: >>> >>> I suppose we have to relay information to kernel sub-arch maintainers >>> who aren't the same as maintaining U-Boot counterparts. How about >>> adding U-Boot ML to CC for whichever DT change gets submitted in the >> >> And every other project? Just setup lei filters. >> >>> kernel? Otherwise adding U-Boot sub-arch maintainers as reviewers for >>> corresponding kernel DT changes works too if that's acceptable. >> >> You just entirely ignored my proposal without addressing it... ok let it >> be. No, CC-ing U-boot maintainers changes nothing because as I said, I >> want kernel maintainers and contributors to be aware. > > I don't think that adding U-Boot platform maintainers as reviewers for > the platforms in the kernel is a terrible idea. Certainly kernel > platform maintainers for which U-Boot platform maintainers are added to > the MAINTAINERS entry will be aware. That said, something like your The point is it does not solve my concern here. I did not express problem that U-Boot maintainers are not aware. They can easily be aware by setting simple lei filters. The problem I want to solve is the kernel maintainers to be aware. Best regards, Krzysztof
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On 22/12/2023 16:46, Tom Rini wrote: > On Fri, Dec 22, 2023 at 04:38:01PM +0100, Krzysztof Kozlowski wrote: >> On 22/12/2023 14:43, Sumit Garg wrote: >>> On Fri, 22 Dec 2023 at 13:48, Krzysztof Kozlowski >>> wrote: >>>> >>>> On 22/12/2023 07:12, Sumit Garg wrote: >>>>> Changes in v2: >>>>> -- >>>>> - Patch #1: excluded gitab CI config check and added commit description. >>>>> - Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/ >>>>> - Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/ >>>>> - Patch #5: s/U-boot/U-Boot/ >>>>> - Patch #6 and #7: Picked up review tags >>>>> >>>>> Prerequisite >>>>> >>>>> >>>>> This patch series requires devicetree-rebasing git repo to be added as a >>>>> subtree to the main U-Boot repo via: >>>>> >>>>> $ git subtree add --prefix devicetree-rebasing \ >>>>> >>>>> git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git >>>>> \ >>>>> v6.6-dts --squash >>>>> >>>>> Background >>>>> -- >>>>> >>>>> This effort started while I was reviewing patch series corresponding to >>>>> Qcom platforms [1] which was about to import modified devicetree source >>>>> files from Linux kernel. I suppose keeping devicetree files sync with >>>>> Linux kernel without any DT bindings schema validation has been a pain >>>>> for U-Boot SoC/platform maintainers. There has been past discussions >>>>> about a single DT repo but that hasn't come up and Linux kernel remained >>>>> the place where DT source files as well as bindings are placed and >>>>> maintained. >>>> >>>> Thanks for doing this. >>>> >>>> I really suggest to store information that kernel DTS is directly >>>> re-used, thus DTS backward and forward compatibility matters, also in >>>> Linux kernel sources. The point is that sub-arch maintainers should be >>>> aware of it. I don't think that as DT maintainers we can efficiently >>>> keep an eye on it. Maybe create a subsystem profile and include it to >>>> maintainer entries of such affected platforms? >>>> >>> >>> From U-Boot point of view, currently we have the config option: >>> "CONFIG_OF_UPSTREAM=y" per platform which means directly re-use of >>> kernel DTS. So U-Boot sub-arch maintainers should be aware of >>> platforms which get converted to re-use kernel DTS. >> >> I was speaking about kernel. >> >>> >>> I suppose we have to relay information to kernel sub-arch maintainers >>> who aren't the same as maintaining U-Boot counterparts. How about >>> adding U-Boot ML to CC for whichever DT change gets submitted in the >> >> And every other project? Just setup lei filters. >> >>> kernel? Otherwise adding U-Boot sub-arch maintainers as reviewers for >>> corresponding kernel DT changes works too if that's acceptable. >> >> You just entirely ignored my proposal without addressing it... ok let it >> be. No, CC-ing U-boot maintainers changes nothing because as I said, I >> want kernel maintainers and contributors to be aware. > > Maybe an underlying question is, what kernel maintainers aren't aware, > but should have been already? Then we can figure out how to address None of them is aware. > that. For example, with your Samsung hat on you weren't aware that > exynos 4/5/7 DTS files are cared about by U-Boot, but are now aware. Hm, I am still not aware of this. I mean, you wrote it above, but it is the first time I see using directly usptream DTS for U-Boot on Samsung platforms. Did anyone test it actually? I certainly did not. I think this patchset did not remove U-Boot-tree Samsung DTS, did it? Best regards, Krzysztof
Re: [PATCH RFC] dt-bindings: nvmem: u-boot, env: add any-name MAC cells compatible
On 18/12/2023 23:02, Rafał Miłecki wrote: > On 14.12.2023 22:27, Simon Glass wrote: >> On Thu, 14 Dec 2023 at 08:36, Rafał Miłecki wrote: >>> >>> From: Rafał Miłecki >>> >>> So far we had a property for "ethaddr" NVMEM cell containing base >>> Ethernet MAC address. The problem is vendors often pick non-standard >>> names for storing MAC(s) (other than "ethaddr"). A few names were >>> noticed over years: >>> 1. "wanaddr" (Edimax, ELECOM, EnGenius, I-O DATA, Sitecom) >>> 2. "et1macaddr" (ASUS) >>> 3. "eth1addr" (Buffalo) >>> 4. "athaddr" (EnGenius) >>> 5. "baseMAC" (Netgear) >>> 6. "mac" (Netgear) >>> 7. "mac_addr" (Moxa) >>> and more ("HW_LAN_MAC", "HW_WAN_MAC", "INIC_MAC_ADDR", "LAN_MAC_ADDR", >>> "RADIOADDR0", "RADIOADDR1", "WAN_MAC_ADDR", "lan1_mac_addr", "wanmac", >>> "wmac1", "wmac2"). >>> >>> It doesn't make sense to add property for every possible MAC cell name. >>> Instead allow specifying cells with "mac" compatible. >>> >>> Signed-off-by: Rafał Miłecki >>> --- >>> List of devices and their U-Boot MAC variables: >>> alphanetworks,asl56026) wanmac >>> asus,rt-ac65p) et1macaddr >>> asus,rt-ac85p) et1macaddr >>> belkin,f9k1109v1) HW_WAN_MAC + HW_LAN_MAC >>> buffalo,ls220de) eth1addr >>> buffalo,ls421de) eth1addr >>> checkpoint,l-50) lan1_mac_addr >>> dovado,tiny-ac) INIC_MAC_ADDR >>> dovado,tiny-ac) LAN_MAC_ADDR + WAN_MAC_ADDR >>> edimax,ra21s) wanaddr >>> edimax,rg21s) wanaddr >>> elecom,wrc-2533ghbk-i) wanaddr >>> elecom,wrc-2533ghbk2-t) wanaddr >>> engenius,ecb1200) athaddr >>> engenius,ecb1750) athaddr >>> engenius,epg5000) wanaddr >>> engenius,epg600) wanaddr >>> engenius,esr1200) wanaddr >>> engenius,esr1750) wanaddr >>> engenius,esr600) wanaddr >>> engenius,esr600h) wanaddr >>> engenius,esr900) wanaddr >>> enterasys,ws-ap3705i) RADIOADDR0 + RADIOADDR1 >>> iodata,wn-ac1167dgr) wanaddr >>> iodata,wn-ac1167gr) wanaddr >>> iodata,wn-ac1600dgr) wanaddr >>> iodata,wn-ac1600dgr2) wanaddr >>> iodata,wn-ac733gr3) wanaddr >>> iodata,wn-ag300dgr) wanaddr >>> iodata,wnpr2600g) wanaddr >>> moxa,awk-1137c) mac_addr >>> netgear,wax220) mac >>> netgear,wndap620) baseMAC >>> netgear,wndap660) baseMAC >>> ocedo,panda) wmac1 + wmac2 >>> sitecom,wlr-7100) wanaddr >>> sitecom,wlr-8100) wanaddr >>> >>> .../devicetree/bindings/nvmem/u-boot,env.yaml | 33 +++ >>> 1 file changed, 33 insertions(+) >>> >> >> Are these upstream U-Boots, or just vendor forks? > > I guess most of those devices don't have upstream U-Boot support. Please We do not document properties used in all possible projects, like vendor forks. Only upstream U-Boot matters. > note that while upstreaming vendors changes would be great in most cases > it doesn't happen. Most vendors sadly don't care and most end users > don't have enough time for that. In practice we often stick with vendor > provided bootloader to flash and boot self build Linux system (like > OpenWrt). > > I'm not sure if/how does it help with this PATCH but please note that > upstream U-Boot code also supports few extra variables. > > There is generic eth_env_get_enetaddr_by_index() that supports variables > like "<%s><%d>addr" and "<%s>addr". Right now it's used only for > "eth<%d>addr" and "ethaddr" so that mostly limits us to "ethaddr", > "eth1addr", "eth2addr" and "eth3addr". > > From some rare cases: there are also "usbnet_devaddr" and "wolpassword". > > So given that U-Boot oficially supports at least 6 env variables for > MAC and there are many used with custom U-Boots and firmwares this > binding would help a lot. Please limit this to upstream U-Boot. Drop all custom and firmware ones. Then, just fix upstream U-Boot to have only one property... Best regards, Krzysztof
Re: [PATCH v2 0/8] An effort to bring DT bindings compliance within U-Boot
On 26/12/2023 08:53, Sumit Garg wrote: >> >> The problem I want to solve is the kernel maintainers to be aware. >> > > Although Tom has already expressed in the other thread that U-Boot has > been a long time user of upstream DT but if we want to make it more > formal from an enforcement point of view then I liked Conor's idea. If What did I write in my first email? "Maybe create a subsystem profile and include it to maintainer entries of such affected platforms?" You really keep ignoring my emails, so I'll stop responding here. Best regards, Krzysztof
Re: [PATCH] arm64: xilinx: Move address/size-cells to proper locations
On 10/01/2024 14:35, Michal Simek wrote: > Move cells to board dtsi files from generic zynqmp.dtsi. Changes are > related to qspi, spi, nand, i2c and ethernet nodes. > > All errors are generated when dtbs are compiled with W=1. > I don't see any errors on some other platforms, like Samsung. Isn't the actual problem that you do not disable the nodes (I2C, SPI etc) in DTSI? Best regards, Krzysztof
Re: [PATCH] arm64: xilinx: Move address/size-cells to proper locations
On 11/01/2024 08:09, Michal Simek wrote: > > > On 1/10/24 22:27, Krzysztof Kozlowski wrote: >> On 10/01/2024 14:35, Michal Simek wrote: >>> Move cells to board dtsi files from generic zynqmp.dtsi. Changes are >>> related to qspi, spi, nand, i2c and ethernet nodes. >>> >>> All errors are generated when dtbs are compiled with W=1. >>> >> >> I don't see any errors on some other platforms, like Samsung. Isn't the >> actual problem that you do not disable the nodes (I2C, SPI etc) in DTSI? > > On i2c node. Bus is present on the board but it can end in a connector or > device > which we don't have OS/bootloader drivers for. But we have drivers using i2c > tools or u-boot i2c probe. It means that transition should happen. > On i2c interesting is that W=1 is not able to report issues when you have i2c > mux described like this > > i2c@0 { > #address-cells = <1>; > #size-cells = <0>; > reg = <0>; > /* HPC0_IIC */ I understand and it is quite common, but it does not explain the case. Your bus should still be disabled in DTSI and enabled in DTS for these cases. And how exactly do you solve the warning for above case? > }; > > and it doesn't report that cells shouldn't be there. > > SPI is pretty much the same story with using spidev. > > Ethernet with mdio. I have converted all phy description to use mdio node > because it provides ability to have separate reset for the whole mdio bus and > then every phy can also have own reset itself that's why using this type of > description is better from flexibility point of view. > > qspi/nand - we have driver for that devices all the time but it doesn't make > sense to have some nodes follow some rules and others not. Best regards, Krzysztof
Re: [PATCH] arm64: xilinx: Move address/size-cells to proper locations
On 11/01/2024 09:10, Michal Simek wrote: > > > On 1/11/24 08:56, Krzysztof Kozlowski wrote: >> On 11/01/2024 08:09, Michal Simek wrote: >>> >>> >>> On 1/10/24 22:27, Krzysztof Kozlowski wrote: >>>> On 10/01/2024 14:35, Michal Simek wrote: >>>>> Move cells to board dtsi files from generic zynqmp.dtsi. Changes are >>>>> related to qspi, spi, nand, i2c and ethernet nodes. >>>>> >>>>> All errors are generated when dtbs are compiled with W=1. >>>>> >>>> >>>> I don't see any errors on some other platforms, like Samsung. Isn't the >>>> actual problem that you do not disable the nodes (I2C, SPI etc) in DTSI? >>> >>> On i2c node. Bus is present on the board but it can end in a connector or >>> device >>> which we don't have OS/bootloader drivers for. But we have drivers using i2c >>> tools or u-boot i2c probe. It means that transition should happen. >>> On i2c interesting is that W=1 is not able to report issues when you have >>> i2c >>> mux described like this >>> >>> i2c@0 { >>> #address-cells = <1>; >>> #size-cells = <0>; >>> reg = <0>; >>> /* HPC0_IIC */ >> >> I understand and it is quite common, but it does not explain the case. >> Your bus should still be disabled in DTSI and enabled in DTS for these >> cases. >> >> And how exactly do you solve the warning for above case? > > I had address/size-cells in dtsi (entire SoC) and then just enabled the whole > i2c bus(for above reason) in board dts file without any childs there. > Then W=1 reported that there are address/size-cells without child. > > Another way how to solve it would be to simply delete address/size cells in > these cases. But your device without children sill have address/size cells, doesn't it? Or does it mean you did not add it to such cases? Anyway, I have exactly the same case exynos5800-peach-pi.dts with i2c_2 and no W=1 warnings. I really do not think your solution is correct. Also, address/size cells are properties of the SoC, so rather DTSI. Best regards, Krzysztof
Re: [PATCH] arm64: xilinx: Move address/size-cells to proper locations
On 11/01/2024 09:48, Michal Simek wrote: > > > On 1/11/24 09:18, Krzysztof Kozlowski wrote: >> On 11/01/2024 09:10, Michal Simek wrote: >>> >>> >>> On 1/11/24 08:56, Krzysztof Kozlowski wrote: >>>> On 11/01/2024 08:09, Michal Simek wrote: >>>>> >>>>> >>>>> On 1/10/24 22:27, Krzysztof Kozlowski wrote: >>>>>> On 10/01/2024 14:35, Michal Simek wrote: >>>>>>> Move cells to board dtsi files from generic zynqmp.dtsi. Changes are >>>>>>> related to qspi, spi, nand, i2c and ethernet nodes. >>>>>>> >>>>>>> All errors are generated when dtbs are compiled with W=1. >>>>>>> >>>>>> >>>>>> I don't see any errors on some other platforms, like Samsung. Isn't the >>>>>> actual problem that you do not disable the nodes (I2C, SPI etc) in DTSI? >>>>> >>>>> On i2c node. Bus is present on the board but it can end in a connector or >>>>> device >>>>> which we don't have OS/bootloader drivers for. But we have drivers using >>>>> i2c >>>>> tools or u-boot i2c probe. It means that transition should happen. >>>>> On i2c interesting is that W=1 is not able to report issues when you have >>>>> i2c >>>>> mux described like this >>>>> >>>>>i2c@0 { >>>>>#address-cells = <1>; >>>>>#size-cells = <0>; >>>>>reg = <0>; >>>>>/* HPC0_IIC */ >>>> >>>> I understand and it is quite common, but it does not explain the case. >>>> Your bus should still be disabled in DTSI and enabled in DTS for these >>>> cases. >>>> >>>> And how exactly do you solve the warning for above case? >>> >>> I had address/size-cells in dtsi (entire SoC) and then just enabled the >>> whole >>> i2c bus(for above reason) in board dts file without any childs there. >>> Then W=1 reported that there are address/size-cells without child. >>> >>> Another way how to solve it would be to simply delete address/size cells in >>> these cases. >> >> But your device without children sill have address/size cells, doesn't >> it? Or does it mean you did not add it to such cases? >> >> Anyway, I have exactly the same case exynos5800-peach-pi.dts with i2c_2 >> and no W=1 warnings. I really do not think your solution is correct. >> >> Also, address/size cells are properties of the SoC, so rather DTSI. > > Let me take a look at i2c case again. For gems it should be clear that I need > to > remove it because when there is mdio node I am getting errors like this. > > arch/arm64/boot/dts/xilinx/zynqmp.dtsi:627.27-641.5: Warning > (avoid_unnecessary_addr_size): /axi/ethernet@ff0e: unnecessary > #address-cells/#size-cells without "ranges" or child "reg" property Yes, which also synchronizes your DTS with kernel: See: eb2f7ff7de56 ("arm64: xilinx: Remove address/size-cells from gem nodes") BTW, Instead of working on U-Boot clone of DTS, I would suggest spending effort on re-using in kernel DTS (see Sumit's work using rebased-DTS tree). Best regards, Krzysztof
Re: [PATCH 2/8] dts: qcom: import device trees and bindings for SA8155P-ADP
On 29/02/2024 15:21, Volodymyr Babchuk wrote: > Add device tree bindings and actual device trees for SM8150 SoC and > SA8155P-ADP board. Those files were imported from Linux kernel 6.8-rc5 > as is. In this configuration they will not work with U-Boot for couple > reasons: > Aren't you now duplicating dts/upstream/qcom? https://lore.kernel.org/all/20240222093607.3085545-1-sumit.g...@linaro.org/ Best regards, Krzysztof
Re: [PATCH 01/10] mach-snapdragon: Add support for IPQ9574
On 26/02/2024 11:07, Varadarajan Narayanan wrote: > Signed-off-by: Varadarajan Narayanan > --- > > arch/arm/dts/Makefile |2 + > arch/arm/dts/ipq9574-default.dts | 167 +++ > arch/arm/dts/ipq9574-rdp433-mht-phy.dts | 208 +++ > arch/arm/dts/ipq9574.dtsi | 771 ++ > .../include/mach/sysmap-ipq9574.h | 252 > arch/arm/mach-snapdragon/init_ipq9574.c | 81 + > board/qualcomm/ipq9574/Kconfig| 15 + > board/qualcomm/ipq9574/Makefile |4 + > board/qualcomm/ipq9574/board_init.c | 326 > board/qualcomm/ipq9574/ipq9574.c | 170 +++ > board/qualcomm/ipq9574/ipq9574.h | 75 + > board/qualcomm/ipq9574/u-boot-x32.lds | 250 > board/qualcomm/ipq9574/u-boot-x64.lds | 188 +++ > drivers/clk/qcom/clock-ipq9574.c | 1320 + > drivers/pinctrl/qcom/pinctrl-ipq9574.c| 77 + > include/configs/ipq9574.h | 111 ++ > include/dt-bindings/clock/gcc-ipq9574.h | 156 ++ > include/dt-bindings/net/qti-ipqsoc.h | 20 + > include/dt-bindings/pinctrl/pinctrl-ipqsoc.h | 19 + > include/dt-bindings/reset/ipq9574-reset.h | 54 + > 20 files changed, 4266 insertions(+) > create mode 100644 arch/arm/dts/ipq9574-default.dts > create mode 100644 arch/arm/dts/ipq9574-rdp433-mht-phy.dts > create mode 100644 arch/arm/dts/ipq9574.dtsi > create mode 100644 arch/arm/mach-snapdragon/include/mach/sysmap-ipq9574.h > create mode 100644 arch/arm/mach-snapdragon/init_ipq9574.c > create mode 100644 board/qualcomm/ipq9574/Kconfig > create mode 100644 board/qualcomm/ipq9574/Makefile > create mode 100644 board/qualcomm/ipq9574/board_init.c > create mode 100644 board/qualcomm/ipq9574/ipq9574.c > create mode 100644 board/qualcomm/ipq9574/ipq9574.h > create mode 100644 board/qualcomm/ipq9574/u-boot-x32.lds > create mode 100644 board/qualcomm/ipq9574/u-boot-x64.lds > create mode 100644 drivers/clk/qcom/clock-ipq9574.c > create mode 100644 drivers/pinctrl/qcom/pinctrl-ipq9574.c > create mode 100644 include/configs/ipq9574.h > create mode 100644 include/dt-bindings/clock/gcc-ipq9574.h > create mode 100644 include/dt-bindings/net/qti-ipqsoc.h > create mode 100644 include/dt-bindings/pinctrl/pinctrl-ipqsoc.h > create mode 100644 include/dt-bindings/reset/ipq9574-reset.h > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > index d9725030d5..8931dfa2aa 100644 > --- a/arch/arm/dts/Makefile > +++ b/arch/arm/dts/Makefile > @@ -1523,6 +1523,8 @@ dtb-$(CONFIG_ARCH_QEMU) += qemu-arm.dtb qemu-arm64.dtb > dtb-$(CONFIG_TARGET_CORSTONE1000) += corstone1000-mps3.dtb \ > corstone1000-fvp.dtb > > +dtb-$(CONFIG_TARGET_IPQ9574) += ipq9574-rdp433-mht-phy.dtb > + > include $(srctree)/scripts/Makefile.dts > > targets += $(dtb-y) > diff --git a/arch/arm/dts/ipq9574-default.dts > b/arch/arm/dts/ipq9574-default.dts > new file mode 100644 > index 00..501c9492df > --- /dev/null > +++ b/arch/arm/dts/ipq9574-default.dts > @@ -0,0 +1,167 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +/dts-v1/; > + > +#include "ipq9574.dtsi" > + > +/ { > + config_name = "config-default"; > + > + aliases { > + console = &blsp1_uart2_console; > + uart2 = &blsp1_uart3_additional; > + sdhci = &mmc; > + }; > + > + soc: soc { > + tlmm: pinctrl@100 { > + > + sdhci_pinmux: mmc { > + pinconfig; > + emmc_data { No, please use upstream DTS. You imported here a lot of vendor junk. There is no way this will pass any System Ready tests if you hand over this DTB to Linux. Plus really, that's ugly DTS to look at. I am not a maintainer of DTS in U-Boot, so up to the folks here, but I really recommend to NAK such DTS. It just re-adds all the issues we fixed in upstream kernel! I suggest using dts/upstream/qcom, but if you cannot then please import kernel DTS. Best regards, Krzysztof
Re: [PATCH 1/2] samsung: arndale: remove board_mmc_init function
On Tue, Jan 12, 2021 at 03:30:53PM +0900, Jaehoon Chung wrote: > Remove board_mmc_init function. > It will be probed with driver-model. > > Signed-off-by: Jaehoon Chung > --- > arch/arm/mach-exynos/include/mach/dwmmc.h | 2 -- > board/samsung/arndale/arndale.c | 13 - > 2 files changed, 15 deletions(-) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 1/4] treewide: configs: get rid of unused CONFIG_DEFAULT_CONSOLE
On Thu, Sep 17, 2020 at 08:52:00AM +0200, Andre Heider wrote: > These are all unused. > > Signed-off-by: Andre Heider > --- > This sets completely removes CONFIG_DEFAULT_CONSOLE from the tree. > Only compile time tested. > > include/configs/arndale.h | 2 -- > include/configs/espresso7420.h | 3 --- > include/configs/origen.h | 5 - > include/configs/peach-pi.h | 3 --- > include/configs/peach-pit.h| 3 --- > include/configs/smdk5250.h | 2 -- > include/configs/smdk5420.h | 5 - > include/configs/smdkv310.h | 2 -- > include/configs/snow.h | 2 -- > include/configs/spring.h | 2 -- > 10 files changed, 29 deletions(-) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 2/4] treewide: configs: fold CONFIG_DEFAULT_CONSOLE
On Thu, Sep 17, 2020 at 08:52:01AM +0200, Andre Heider wrote: > In prepartion to remove CONFIG_DEFAULT_CONSOLE, fold the current users. > > Signed-off-by: Andre Heider > --- > include/configs/odroid.h| 7 +-- > include/configs/odroid_xu3.h| 6 +- > include/configs/s5p_goni.h | 8 +--- > include/configs/s5pc210_universal.h | 7 +-- > include/configs/trats.h | 5 + > include/configs/trats2.h| 7 +-- > 6 files changed, 6 insertions(+), 34 deletions(-) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 4/4] configs: smdkv310: get rid of unused EXYNOS4_DEFAULT_UART_OFFSET
On Thu, Sep 17, 2020 at 08:52:03AM +0200, Andre Heider wrote: > Unused. > > Signed-off-by: Andre Heider > --- > include/configs/smdkv310.h | 3 --- > 1 file changed, 3 deletions(-) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH] net: zynq_gem: Add support for new compatible str with xlnx prefix
On 09/12/2022 16:19, Michal Simek wrote: > cdns prefix was deprecated and replaced by xlnx one. > > Signed-off-by: Michal Simek > --- > > Link: > https://lore.kernel.org/r/20220726070802.26579-1-krzysztof.kozlow...@linaro.org Thanks. Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
[U-Boot] [RFT 0/8] exynos: Fix reboot on Odroid HC1
Hi, Odroid HC1 does not reboot properly (at least from SD card but I do not expect difference on eMMC), if LDO4/VDD_ADC was turned off by Linux kernel. This condition is so far always, because Linux kernel did not enable ADC on ODroid HC1, therefore the VDD_ADC regulator was turned off as unused. The issue is in detection of revision which later is used to load proper DTB. The revision is obtained by ADC read of a voltage depending on VDD_ADC. Therefore: 1. VDD_ADC has to be turned on (but board detection happens before power is initialized), 2. Turning VDD_ADC should wait with ramp delay, 3. Reading the value from ADC should wait for it to stabilize. I must admit I did not test it on other boards because latest U-Boot does not boot from SD card. Commends and testing are welcomed. Best regards, Krzysztof Krzysztof Kozlowski (8): exynos: Redo detection of revision when all resources are ready exynos: Wait till ADC stabilizes before checking Odroid HC1 revision adc: exynos-adc: Fix wrong bit operation used to stop the ADC regulator: Add support for ramp delay power: regulator: s2mps11: Fix step for LDO27 and LDO35 power: regulator: s2mps11: Add enable delay arm: dts: exynos: Add supply for ADC block to Odroid XU3 family arm: dts: exynos: Add ramp delay property to LDO regulators to Odroid XU3 family arch/arm/dts/exynos5422-odroidxu3.dts | 20 + board/samsung/common/board.c| 19 - board/samsung/common/exynos5-dt-types.c | 34 +++- drivers/adc/exynos-adc.c| 2 +- drivers/power/regulator/regulator-uclass.c | 45 - drivers/power/regulator/s2mps11_regulator.c | 13 +- include/power/regulator.h | 2 + 7 files changed, 129 insertions(+), 6 deletions(-) -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [RFT 1/8] exynos: Redo detection of revision when all resources are ready
Detection of board type is done early - before power setup. In case of Odroid XU3/XU4/HC1 family, the detection is done using ADC which is supplied by LDO4/VDD_ADC regulator. This regulator could be turned off (e.g. by kernel before reboot); If ADC is used early, the regulators are not yet available and the detection won't work. Try to detect the revision again, once power is brought up. This is necessary to fix the detection of Odroid HC1 after reboot, if kernel turned off the LDO4 regulator. Otherwise the board is not detected Signed-off-by: Krzysztof Kozlowski --- board/samsung/common/board.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c index 6fd26a3a9198..1e2dabe68d11 100644 --- a/board/samsung/common/board.c +++ b/board/samsung/common/board.c @@ -147,6 +147,11 @@ int board_early_init_f(void) { int err; #ifdef CONFIG_BOARD_TYPES + /* +* It is done early so power might not be set up yet. In such case +* specific revision detection with ADC might not work and need to me +* redone later. +*/ set_board_type(); #endif err = board_uart_init(); @@ -166,9 +171,21 @@ int board_early_init_f(void) #if defined(CONFIG_POWER) || defined(CONFIG_DM_PMIC) int power_init_board(void) { + int ret; + set_ps_hold_ctrl(); - return exynos_power_init(); + ret = exynos_power_init(); + +#ifdef CONFIG_BOARD_TYPES + /* +* Since power is on, redo the board detection (external peripherals +* are on). +*/ + set_board_type(); +#endif + + return ret; } #endif -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [RFT 3/8] adc: exynos-adc: Fix wrong bit operation used to stop the ADC
When stopping the ADC_V2_CON1_STC_EN should be cleared. Signed-off-by: Krzysztof Kozlowski --- drivers/adc/exynos-adc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/adc/exynos-adc.c b/drivers/adc/exynos-adc.c index d33e3d632afc..12c49fc8cefb 100644 --- a/drivers/adc/exynos-adc.c +++ b/drivers/adc/exynos-adc.c @@ -62,7 +62,7 @@ int exynos_adc_stop(struct udevice *dev) /* Stop conversion */ cfg = readl(®s->con1); - cfg |= ~ADC_V2_CON1_STC_EN; + cfg &= ~ADC_V2_CON1_STC_EN; writel(cfg, ®s->con1); -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [RFT 2/8] exynos: Wait till ADC stabilizes before checking Odroid HC1 revision
Fix detection of Odroid HC1 (Exynos5422) after reboot if kernel disabled the LDO4/VDD_ADC regulator. The LDO4 supplies both ADC block and the ADC input AIN9. Voltage on AIN9 will rise slowly, so be patient and wait for it to stabilize. First reads on Odroid HC1 return 305, 1207, 1297 and finally 1308 (reference value is 1309). Signed-off-by: Krzysztof Kozlowski --- board/samsung/common/exynos5-dt-types.c | 34 - 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/board/samsung/common/exynos5-dt-types.c b/board/samsung/common/exynos5-dt-types.c index 7a86e9187768..ba8584f1a5d8 100644 --- a/board/samsung/common/exynos5-dt-types.c +++ b/board/samsung/common/exynos5-dt-types.c @@ -57,12 +57,44 @@ static unsigned int odroid_get_rev(void) return 0; } +/* + * Read ADC at least twice and check the resuls. If regulator providing voltage + * on to measured point was just turned on, first reads might require time + * to stabilize. + */ +static int odroid_get_adc_val(unsigned int *adcval) +{ + unsigned int adcval_prev = 0; + int ret, retries = 20; + + ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, + &adcval_prev); + if (ret) + return ret; + + while (retries--) { + mdelay(5); + + ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, + adcval); + if (ret) + return ret; + + if ((100 * abs(*adcval - adcval_prev) / adcval_prev) < 3) + return ret; + + adcval_prev = *adcval; + } + + return ret; +} + static int odroid_get_board_type(void) { unsigned int adcval; int ret, i; - ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, &adcval); + ret = odroid_get_adc_val(&adcval); if (ret) goto rev_default; -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [RFT 7/8] arm: dts: exynos: Add supply for ADC block to Odroid XU3 family
The ADC block requires VDD supply to be on so provide one. Signed-off-by: Krzysztof Kozlowski --- arch/arm/dts/exynos5422-odroidxu3.dts | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/dts/exynos5422-odroidxu3.dts b/arch/arm/dts/exynos5422-odroidxu3.dts index e859dd1b981a..9dfae90667cf 100644 --- a/arch/arm/dts/exynos5422-odroidxu3.dts +++ b/arch/arm/dts/exynos5422-odroidxu3.dts @@ -32,6 +32,7 @@ adc@12D1 { u-boot,dm-pre-reloc; + vdd-supply = <&ldo4_reg>; status = "okay"; }; -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [RFT 6/8] power: regulator: s2mps11: Add enable delay
According to datasheet, the output on LDO regulators will start appearing after 10-15 us. Signed-off-by: Krzysztof Kozlowski --- drivers/power/regulator/s2mps11_regulator.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/power/regulator/s2mps11_regulator.c b/drivers/power/regulator/s2mps11_regulator.c index 723d27f67c9a..1f1581852ee2 100644 --- a/drivers/power/regulator/s2mps11_regulator.c +++ b/drivers/power/regulator/s2mps11_regulator.c @@ -551,7 +551,14 @@ static int ldo_get_enable(struct udevice *dev) static int ldo_set_enable(struct udevice *dev, bool enable) { - return s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); + int ret; + + ret = s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); + + /* Wait the "enable delay" for voltage to start to rise */ + udelay(15); + + return ret; } static int ldo_get_mode(struct udevice *dev) -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [RFT 5/8] power: regulator: s2mps11: Fix step for LDO27 and LDO35
LDO27 and LDO35 have 25 mV step, not 50 mV. Signed-off-by: Krzysztof Kozlowski --- drivers/power/regulator/s2mps11_regulator.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/power/regulator/s2mps11_regulator.c b/drivers/power/regulator/s2mps11_regulator.c index ced504eb1476..723d27f67c9a 100644 --- a/drivers/power/regulator/s2mps11_regulator.c +++ b/drivers/power/regulator/s2mps11_regulator.c @@ -346,6 +346,8 @@ static int s2mps11_ldo_hex2volt(int ldo, int hex) case 11: case 22: case 23: + case 27: + case 35: uV = hex * S2MPS11_LDO_STEP + S2MPS11_LDO_UV_MIN; break; default: @@ -366,6 +368,8 @@ static int s2mps11_ldo_volt2hex(int ldo, int uV) case 11: case 22: case 23: + case 27: + case 35: hex = (uV - S2MPS11_LDO_UV_MIN) / S2MPS11_LDO_STEP; break; default: -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [RFT 4/8] regulator: Add support for ramp delay
Changing voltage and enabling regulator might require delays so the regulator stabilizes at expected level. Add support for "regulator-ramp-delay" binding which can introduce required time to both enabling the regulator and to changing the voltage. Signed-off-by: Krzysztof Kozlowski --- drivers/power/regulator/regulator-uclass.c | 45 +- include/power/regulator.h | 2 + 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 39e46279d533..4119f244c74b 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -35,10 +35,22 @@ int regulator_get_value(struct udevice *dev) return ops->get_value(dev); } +static void regulator_set_value_delay(struct udevice *dev, int old_uV, + int new_uV, unsigned int ramp_delay) +{ + int delay = DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay); + + debug("regulator %s: delay %u us (%d uV -> %d uV)\n", dev->name, + delay, old_uV, new_uV); + + udelay(delay); +} + int regulator_set_value(struct udevice *dev, int uV) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); struct dm_regulator_uclass_platdata *uc_pdata; + int ret, old_uV = uV; uc_pdata = dev_get_uclass_platdata(dev); if (uc_pdata->min_uV != -ENODATA && uV < uc_pdata->min_uV) @@ -49,7 +61,18 @@ int regulator_set_value(struct udevice *dev, int uV) if (!ops || !ops->set_value) return -ENOSYS; - return ops->set_value(dev, uV); + if (uc_pdata->ramp_delay) + old_uV = regulator_get_value(dev); + + ret = ops->set_value(dev, uV); + + if (!ret) { + if (uc_pdata->ramp_delay && old_uV > 0) + regulator_set_value_delay(dev, old_uV, uV, + uc_pdata->ramp_delay); + } + + return ret; } /* @@ -107,6 +130,7 @@ int regulator_set_enable(struct udevice *dev, bool enable) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); struct dm_regulator_uclass_platdata *uc_pdata; + int ret, old_enable = 0; if (!ops || !ops->set_enable) return -ENOSYS; @@ -115,7 +139,22 @@ int regulator_set_enable(struct udevice *dev, bool enable) if (!enable && uc_pdata->always_on) return 0; - return ops->set_enable(dev, enable); + if (uc_pdata->ramp_delay) + old_enable = regulator_get_enable(dev); + + ret = ops->set_enable(dev, enable); + if (!ret) { + if (uc_pdata->ramp_delay && !old_enable) { + int uV = regulator_get_value(dev); + + if (uV > 0) { + regulator_set_value_delay(dev, 0, uV, + uc_pdata->ramp_delay); + } + } + } + + return ret; } int regulator_get_mode(struct udevice *dev) @@ -324,6 +363,8 @@ static int regulator_pre_probe(struct udevice *dev) -ENODATA); uc_pdata->always_on = dev_read_bool(dev, "regulator-always-on"); uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on"); + uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay", + 0); /* Those values are optional (-ENODATA if unset) */ if ((uc_pdata->min_uV != -ENODATA) && diff --git a/include/power/regulator.h b/include/power/regulator.h index 5318ab3acece..c13fa1f336e5 100644 --- a/include/power/regulator.h +++ b/include/power/regulator.h @@ -149,6 +149,7 @@ enum regulator_flag { * @max_uA*- maximum amperage (micro Amps) * @always_on* - bool type, true or false * @boot_on* - bool type, true or false + * @ramp_delay - Time to settle down after voltage change (unit: uV/us) * TODO(s...@chromium.org): Consider putting the above two into @flags * @flags: - flags value (see REGULATOR_FLAG_...) * @name** - fdt regulator name - should be taken from the device tree @@ -169,6 +170,7 @@ struct dm_regulator_uclass_platdata { int max_uV; int min_uA; int max_uA; + unsigned int ramp_delay; bool always_on; bool boot_on; const char *name; -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [RFT 8/8] arm: dts: exynos: Add ramp delay property to LDO regulators to Odroid XU3 family
Add startup time to LDO regulators of S2MPS11 PMIC on Odroid XU3/XU4/HC1 family of boards to be sure the voltage is proper before relying on the regulator. The datasheet for all the S2MPS1x family is inconsistent here and does not specify unambiguously the value of ramp delay for LDO. It mentions 30 mV/us in one timing diagram but then omits it completely in LDO regulator characteristics table (it is specified for bucks). However the vendor kernels for Galaxy S5 and Odroid XU3 use values of 12 mV/us or 24 mV/us. Without the ramp delay value the consumers do not wait for voltage settle after changing it. Although the proper value of ramp delay for LDOs is unknown, it seems safer to use at least some value from reference kernel than to leave it unset. Signed-off-by: Krzysztof Kozlowski --- arch/arm/dts/exynos5422-odroidxu3.dts | 19 +++ 1 file changed, 19 insertions(+) diff --git a/arch/arm/dts/exynos5422-odroidxu3.dts b/arch/arm/dts/exynos5422-odroidxu3.dts index 9dfae90667cf..04ecc404f907 100644 --- a/arch/arm/dts/exynos5422-odroidxu3.dts +++ b/arch/arm/dts/exynos5422-odroidxu3.dts @@ -45,6 +45,7 @@ regulator-name = "vdd_ldo1"; regulator-min-microvolt = <100>; regulator-max-microvolt = <100>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -52,18 +53,21 @@ regulator-name = "vddq_mmc0"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; }; ldo4_reg: LDO4 { regulator-name = "vdd_adc"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; }; ldo5_reg: LDO5 { regulator-name = "vdd_ldo5"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -71,6 +75,7 @@ regulator-name = "vdd_ldo6"; regulator-min-microvolt = <100>; regulator-max-microvolt = <100>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -78,6 +83,7 @@ regulator-name = "vdd_ldo7"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -85,6 +91,7 @@ regulator-name = "vdd_ldo8"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -92,6 +99,7 @@ regulator-name = "vdd_ldo9"; regulator-min-microvolt = <300>; regulator-max-microvolt = <300>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -99,6 +107,7 @@ regulator-name = "vdd_ldo10"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; regulator-always-on;
Re: [U-Boot] [RFT 1/8] exynos: Redo detection of revision when all resources are ready
On Mon, 11 Feb 2019 at 08:20, Lukasz Majewski wrote: > > Hi Krzysztof, > > > Detection of board type is done early - before power setup. In case > > of Odroid XU3/XU4/HC1 family, the detection is done using ADC which > > is supplied by LDO4/VDD_ADC regulator. This regulator could be turned > > off (e.g. by kernel before reboot); If ADC is used early, the > > regulators are not yet available and the detection won't work. > > > > Try to detect the revision again, once power is brought up. > > > > This is necessary to fix the detection of Odroid HC1 after reboot, if > > kernel turned off the LDO4 regulator. Otherwise the board is not > > detected > > But such approach seems not to be the optimal one (as we perform > detection twice - with default LDO4 enabled after power on and after > soft reset). > > I would expect to enable the LDO4 regulator in the early code (I2C > would be probably necessary) and then read ADC value properly once. > > (I also guess that the "work-by-chance" approach is caused by default > settings of PMIC after power on). So basically you want to move the board detection after the exynos_power_init()... maybe it is possible. The other way is to split set_board_type() into OF part (for compatible and main board difference) and revision detection (requiring ADC). Maybe it is possible, but isn't it used before? > As fair as I remember, TI is able to read the EEPROM via I2C in the > very early u-boot (MLO to be precise) code and then make the decision > regarding the platform. > > Maybe it would be possible to do the same with Samsung? > > And another thought - if the set_board_type() can be called latter and > it works - why cannot we move it to this latter point and execute > exactly once? It is the same as previous idea... or I do not see the difference... Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFT 4/8] regulator: Add support for ramp delay
On Sun, 10 Feb 2019 at 10:49, Simon Glass wrote: > > Hi Krzysztof, > > On Sat, 9 Feb 2019 at 16:54, Krzysztof Kozlowski wrote: > > > > Changing voltage and enabling regulator might require delays so the > > regulator stabilizes at expected level. > > > > Add support for "regulator-ramp-delay" binding which can introduce > > required time to both enabling the regulator and to changing the > > voltage. > > Is this binding used in Linux? Can you please add binding > documentation for this? Yes, these are bindings from the kernel. I will add them. > > > > > Signed-off-by: Krzysztof Kozlowski > > --- > > drivers/power/regulator/regulator-uclass.c | 45 +- > > include/power/regulator.h | 2 + > > 2 files changed, 45 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/power/regulator/regulator-uclass.c > > b/drivers/power/regulator/regulator-uclass.c > > index 39e46279d533..4119f244c74b 100644 > > --- a/drivers/power/regulator/regulator-uclass.c > > +++ b/drivers/power/regulator/regulator-uclass.c > > @@ -35,10 +35,22 @@ int regulator_get_value(struct udevice *dev) > > return ops->get_value(dev); > > } > > > > +static void regulator_set_value_delay(struct udevice *dev, int old_uV, > > + int new_uV, unsigned int ramp_delay) > > +{ > > + int delay = DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay); > > So the ramp delay is microseconds per (microvolt delta)? The ramp delay is microvolt per microseconds. int delay = uv / (uV/uS) = uS > > > + > > + debug("regulator %s: delay %u us (%d uV -> %d uV)\n", dev->name, > > + delay, old_uV, new_uV); > > + > > + udelay(delay); > > +} > > + > > int regulator_set_value(struct udevice *dev, int uV) > > { > > const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); > > struct dm_regulator_uclass_platdata *uc_pdata; > > + int ret, old_uV = uV; > > > > uc_pdata = dev_get_uclass_platdata(dev); > > if (uc_pdata->min_uV != -ENODATA && uV < uc_pdata->min_uV) > > @@ -49,7 +61,18 @@ int regulator_set_value(struct udevice *dev, int uV) > > if (!ops || !ops->set_value) > > return -ENOSYS; > > > > - return ops->set_value(dev, uV); > > + if (uc_pdata->ramp_delay) > > + old_uV = regulator_get_value(dev); > > + > > + ret = ops->set_value(dev, uV); > > + > > + if (!ret) { > > + if (uc_pdata->ramp_delay && old_uV > 0) > > + regulator_set_value_delay(dev, old_uV, uV, > > + uc_pdata->ramp_delay); > > + } > > + > > + return ret; > > } > > > > /* > > @@ -107,6 +130,7 @@ int regulator_set_enable(struct udevice *dev, bool > > enable) > > { > > const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); > > struct dm_regulator_uclass_platdata *uc_pdata; > > + int ret, old_enable = 0; > > > > if (!ops || !ops->set_enable) > > return -ENOSYS; > > @@ -115,7 +139,22 @@ int regulator_set_enable(struct udevice *dev, bool > > enable) > > if (!enable && uc_pdata->always_on) > > return 0; > > > > - return ops->set_enable(dev, enable); > > + if (uc_pdata->ramp_delay) > > + old_enable = regulator_get_enable(dev); > > + > > + ret = ops->set_enable(dev, enable); > > + if (!ret) { > > + if (uc_pdata->ramp_delay && !old_enable) { > > + int uV = regulator_get_value(dev); > > + > > + if (uV > 0) { > > + regulator_set_value_delay(dev, 0, uV, > > + > > uc_pdata->ramp_delay); > > + } > > How come there is a delay when enabling it as well as when setting the > voltage? Enabling a regulator also requires the time, which might be sum of: 1. Initial enable delay (not included here), 2. Rising of voltage delay (ramp delay) from 0 to expected. In Linux kernel this is separate property regulator-enable-ramp-delay. Here I reused the ramp delay to make it simpler. However indeed there is no point to wait with ramp delay if the regulat
Re: [U-Boot] [RFT 1/8] exynos: Redo detection of revision when all resources are ready
On Mon, 11 Feb 2019 at 09:14, Lukasz Majewski wrote: > > Hi Krzysztof, > > > On Mon, 11 Feb 2019 at 08:20, Lukasz Majewski wrote: > > > > > > Hi Krzysztof, > > > > > > > Detection of board type is done early - before power setup. In > > > > case of Odroid XU3/XU4/HC1 family, the detection is done using > > > > ADC which is supplied by LDO4/VDD_ADC regulator. This regulator > > > > could be turned off (e.g. by kernel before reboot); If ADC is > > > > used early, the regulators are not yet available and the > > > > detection won't work. > > > > > > > > Try to detect the revision again, once power is brought up. > > > > > > > > This is necessary to fix the detection of Odroid HC1 after > > > > reboot, if kernel turned off the LDO4 regulator. Otherwise the > > > > board is not detected > > > > > > But such approach seems not to be the optimal one (as we perform > > > detection twice - with default LDO4 enabled after power on and after > > > soft reset). > > > > > > I would expect to enable the LDO4 regulator in the early code (I2C > > > would be probably necessary) and then read ADC value properly once. > > > > > > (I also guess that the "work-by-chance" approach is caused by > > > default settings of PMIC after power on). > > > > So basically you want to move the board detection after the > > exynos_power_init()... maybe it is possible. The other way is to split > > set_board_type() into OF part (for compatible and main board > > difference) and revision detection (requiring ADC). Maybe it is > > possible, but isn't it used before? > > I do want to avoid making the detection twice; > > First time when we have the LDO4 enabled by default and the second time > when it may happen that we do a soft reset from Linux (which disabled > LDO4). This I understand but isn't the board type used BEFORE the power init? Power init cannot be moved early as it depends on having proper resources (as I wrote in commit msg)... so only board detection can be moved later. But if setting up resources (e.g. regulators) requires board type then it is circular dependency... so I asked - isn't board type used before power init? Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFT 6/8] power: regulator: s2mps11: Add enable delay
On Mon, 11 Feb 2019 at 08:11, Lukasz Majewski wrote: > > Hi Krzysztof, > > > According to datasheet, the output on LDO regulators will start > > appearing after 10-15 us. > > > > Signed-off-by: Krzysztof Kozlowski > > --- > > drivers/power/regulator/s2mps11_regulator.c | 9 - > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/power/regulator/s2mps11_regulator.c > > b/drivers/power/regulator/s2mps11_regulator.c index > > 723d27f67c9a..1f1581852ee2 100644 --- > > a/drivers/power/regulator/s2mps11_regulator.c +++ > > b/drivers/power/regulator/s2mps11_regulator.c @@ -551,7 +551,14 @@ > > static int ldo_get_enable(struct udevice *dev) > > static int ldo_set_enable(struct udevice *dev, bool enable) > > { > > - return s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); > > + int ret; > > + > > + ret = s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); > > + > > + /* Wait the "enable delay" for voltage to start to rise */ > > + udelay(15); > > I assume, that this value is the same as in the Linux driver? No, Linux drivers does not do it. It should... but we never implemented it there. Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] ARM: Samsung: Add Exynos5422-based Odroid HC1 support
On Fri, Nov 03, 2017 at 09:30:30AM +0100, Marek Szyprowski wrote: > Odroid HC1 board is based on Odroid XU4 board, but it has no HDMI, > no eMMC, no build-in USB3.0 hub, no extension port pins, and no GPIO > button. USB3.0 ports are used for build-in JMicron USB to SATA bridge > and Gigabit R8152 ethernet chips. HC1 uses only passive cooling. > > This patch also updates Odroid's ADCmax array and reduces ADC tolerance > to 1% to ensure that XU4 and HC1 revisions are properly detected. > > I've tested this with XU3, XU3-lite, XU4 and HC1 boards. In case of my test > boards I got following values from ADC register: 372, 370, 1281 and 1313. > > Signed-off-by: Marek Szyprowski > --- > board/samsung/common/exynos5-dt-types.c | 27 --- > board/samsung/common/exynos5-dt.c | 4 ++-- > configs/odroid-xu3_defconfig| 2 +- > include/samsung/exynos5-dt-types.h | 2 ++ > 4 files changed, 25 insertions(+), 10 deletions(-) > Tested on Odroid HC1 with tftpboot: Tested-by: Krzysztof Kozlowski Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: exynos: odroid: Fix the confict scripaddr extra env setting
On Thu, 30 May 2019 at 09:11, Anand Moon wrote: > > Hi Krzysztof, > > On Wed, 29 May 2019 at 14:15, Krzysztof Kozlowski wrote: > > > > On Fri, 24 May 2019 at 10:51, Anand Moon wrote: > > > > > > Fix the confict of scriptaddr address with ramdisk_addr_r used > > > in EXTRA_ENV_SETTINGS. > > > > > > Signed-off-by: Anand Moon > > > > My comment from previous patch stays valid: > > "... but there is no conflict in the first place. These addresses are > > not used in the same time." > > > > The patch does not harm but it is not correctly explained. There is no > > issue so it is not a fix. > > > > Best regards, > > Krzysztof > > Most of the time we use custom boot.scr for loading zImage and dtb, > but some how on ARCH Linux it tried to load from this EXTRA_ENV script. > > Ok it's not a FIX, but still we need have different load address for > script (boot.scr) to loaded. I am not that sure that you need the different address. I guess the boot.scr is loaded first and parsed/sourced entirely. Then additional commands are executed (which are already in env), so ramdisk can be loaded under the same address. However there is no problem of changing the address... just reflect it in commit message. Best regards, Krzysztof > How about change the subject and commit message to below. > > "update the scriptaddr address to load from different address used in > EXTRA_ENV_SETTINGS." > > Best Regards > -Anand ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] arndale: fix unknown status
On Thu, 14 Mar 2019 at 05:10, Minkyu Kang wrote: > > On 14/03/2019 09:44, Minkyu Kang wrote: > > set status to Maintained > > > > Signed-off-by: Minkyu Kang > > Cc: Krzysztof Kozlowski > > --- > > board/samsung/arndale/MAINTAINERS | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/board/samsung/arndale/MAINTAINERS > > b/board/samsung/arndale/MAINTAINERS > > index 98ccaa4..aa64c7a 100644 > > --- a/board/samsung/arndale/MAINTAINERS > > +++ b/board/samsung/arndale/MAINTAINERS > > @@ -1,6 +1,6 @@ > > ARNDALE BOARD > > M: Krzysztof Kozlowski > > -S: Odd Fixes > > +S: Maintained > > F: board/samsung/arndale/ > > F: include/configs/arndale.h > > F: configs/arndale_defconfig > > > > applied to u-boot-samsung. Hi, In the commit changing the maintainer I changed on purpose to Odd Fixes. Because the board is not actively maintained. It is not "unknown status", it is perfectly valid status. Why changing it to Maintained? Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] arndale: fix unknown status
On Thu, 14 Mar 2019 at 09:15, Minkyu Kang wrote: > > Hi, > > On 14/03/2019 16:38, Krzysztof Kozlowski wrote: > > On Thu, 14 Mar 2019 at 05:10, Minkyu Kang wrote: > >> > >> On 14/03/2019 09:44, Minkyu Kang wrote: > >>> set status to Maintained > >>> > >>> Signed-off-by: Minkyu Kang > >>> Cc: Krzysztof Kozlowski > >>> --- > >>> board/samsung/arndale/MAINTAINERS | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/board/samsung/arndale/MAINTAINERS > >>> b/board/samsung/arndale/MAINTAINERS > >>> index 98ccaa4..aa64c7a 100644 > >>> --- a/board/samsung/arndale/MAINTAINERS > >>> +++ b/board/samsung/arndale/MAINTAINERS > >>> @@ -1,6 +1,6 @@ > >>> ARNDALE BOARD > >>> M: Krzysztof Kozlowski > >>> -S: Odd Fixes > >>> +S: Maintained > >>> F: board/samsung/arndale/ > >>> F: include/configs/arndale.h > >>> F: configs/arndale_defconfig > >>> > >> > >> applied to u-boot-samsung. > > > > Hi, > > > > In the commit changing the maintainer I changed on purpose to Odd > > Fixes. Because the board is not actively maintained. It is not > > "unknown status", it is perfectly valid status. Why changing it to > > Maintained? > > > > $ ./tools/genboardscfg.py -f > WARNING: Odd Fixes: unknown status for 'arndale' > > If you don't want to maintain then, it would be set to Orphan? I think then the genboardscfg.py should be updated because "Odd fixer" is a valid entry from MAINTAINERS (although not popular). "Odd Fixes: It has a maintainer but they don't have time to do much other than throw the odd patch in. See below.." The board is pretty close to being orphaned but not yet. Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] please pull u-boot-samsung master
On Thu, 14 Mar 2019 at 01:32, Tom Rini wrote: > > On Tue, Mar 12, 2019 at 05:23:37PM +0900, Minkyu Kang wrote: > > > Hi Tom, > > > > The following changes since commit e8e3f2d2d48f97b2c79b698eccedce8f4f880993: > > > > Merge branch '2019-03-08-master-imports' (2019-03-08 18:04:13 -0500) > > > > are available in the git repository at: > > > > > > git://git.denx.de/u-boot-samsung master > > > > for you to fetch changes up to 0fd36730396a86e5513b45221af60889e6028654: > > > > ARM: Odroid XU3: Enable driver I2C support for OdroidXU3 (2019-03-12 > > 15:56:45 +0900) > > > > NAK: > $ ./tools/genboardscfg.py -f > WARNING: Odd Fixes: unknown status for 'arndale' Hi Tom, That's the valid entry from MAINTAINERS so one of them should be updated (either remove Odd fixes or accept it). Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] MAINTAINERS: Remove unsupported statuses - Odd Fixes and Obsolete
The MAINTAINERS file was copied from Linux Kernel along with all its statuses of maintainership. However tools/genboardscfg.py accepts only Maintained, Supported and Orphan. Remove then the Odd Fixes and Obsolete from MAINTAINERS file to avoid confusion. Signed-off-by: Krzysztof Kozlowski --- MAINTAINERS | 5 - 1 file changed, 5 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 29449ffed633..66622df4eca7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12,13 +12,8 @@ Descriptions of section entries: S: Status, one of the following: Supported: Someone is actually paid to look after this. Maintained: Someone actually looks after it. - Odd Fixes: It has a maintainer but they don't have time to do - much other than throw the odd patch in. See below.. Orphan: No current maintainer [but maybe you could take the role as you write your new code]. - Obsolete:Old code. Something tagged obsolete generally means - it has been replaced by a better system and you - should be using that. F: Files and directories with wildcard patterns. A trailing slash includes all files and subdirectory files. F: drivers/net/all files in and below drivers/net -- 2.7.4 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] arndale: fix unknown status
On Thu, 14 Mar 2019 at 20:24, Tom Rini wrote: > I'm taking your patch to the MAINTAINERS file now. That said, generally > "odd fixes" are what's required of board maintainers, once the port is > in. However, as we push forward on moving to various frameworks that > should make life easier overall, over the long term, changes are needed > that may be more than just an occasional fix. Looking at arndale right > now for example: >arm: w+ arndale > +(arndale) = WARNING == > +(arndale) This board uses CONFIG_DM_I2C_COMPAT. Please remove > +(arndale) (possibly in a subsequent patch in your series) > +(arndale) before sending patches to the mailing list. > +(arndale) > > So, do you have time to look into that build time warning? Thanks! Sure, let me take a look. Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/2] arm: exynos: arndale: Remove unused CONFIG_DM_I2C_COMPAT
The CONFIG_DM_I2C_COMPAT was introduced in include/configs/exynos5-common.h in commit 189d80166b31 ("exynos5: enable dm i2c") and then it propagated up to configs/arndale_defconfig. However since beginning the Arndale board (Exynos5250) was not using I2C. In fact, the Arndale board is not configuring its PMIC (S5M8767) which uses I2C bus. This setting can be thus safely removed to fix build warning: This board uses CONFIG_DM_I2C_COMPAT. Please remove (possibly in a subsequent patch in your series) before sending patches to the mailing list. Signed-off-by: Krzysztof Kozlowski --- Not tested on Arndale board. Testing is welcomed. --- configs/arndale_defconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/configs/arndale_defconfig b/configs/arndale_defconfig index 24422645cbac..2f218e8bb64c 100644 --- a/configs/arndale_defconfig +++ b/configs/arndale_defconfig @@ -24,7 +24,6 @@ CONFIG_CMD_SOUND=y CONFIG_CMD_EXT4_WRITE=y CONFIG_DEFAULT_DEVICE_TREE="exynos5250-arndale" CONFIG_ENV_IS_IN_MMC=y -CONFIG_DM_I2C_COMPAT=y CONFIG_MMC_DW=y CONFIG_MMC_SDHCI=y CONFIG_MMC_SDHCI_S5P=y -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 2/2] arm: exynos: arndale: Remove unused CONFIG_POWER and CONFIG_POWER_I2C
The CONFIG_POWER and CONFIG_POWER_I2C were introduced in include/configs/exynos5-common.h in commit 19bd3aaa5991 ("exynos5: fix build break by adding CONFIG_POWER") and then it propagated up to include/configs/arndale.h. However before that commit, there was no build break at all on Arndale and SMDK5250 boards. It seems the commit fixed nothing and just added unused defines. In fact, the Arndale board is not configuring its PMIC (S5M8767) which uses I2C bus. Signed-off-by: Krzysztof Kozlowski --- Not tested on Arndale board. Testing is welcomed. --- include/configs/arndale.h | 8 1 file changed, 8 deletions(-) diff --git a/include/configs/arndale.h b/include/configs/arndale.h index 06b02ce90a94..3d0ce471a42c 100644 --- a/include/configs/arndale.h +++ b/include/configs/arndale.h @@ -32,10 +32,6 @@ #define CONFIG_SYS_INIT_SP_ADDRCONFIG_IRAM_STACK -/* PMIC */ -#define CONFIG_POWER -#define CONFIG_POWER_I2C - #define CONFIG_PREBOOT #define CONFIG_S5P_PA_SYSRAM 0x0202 @@ -44,8 +40,4 @@ /* The PERIPHBASE in the CBAR register is wrong on the Arndale, so override it */ #define CONFIG_ARM_GIC_BASE_ADDRESS0x1048 -/* Power */ -#define CONFIG_POWER -#define CONFIG_POWER_I2C - #endif /* __CONFIG_H */ -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Odroid U3 - Upgrade to latest u-boot kernel load fails.
On Mon, 18 Mar 2019 at 13:32, Anand Moon wrote: > > Hi Krzysztof / Marek, > > After I update the latest u-boot on my Odroud U3+ it fails to load the > kernel it hangs. > I am using Archlinux on Odroid U3. > > U-Boot 2019.04-rc3-00131-g8303467e80d-dirty (Mar 18 2019 - 12:12:23 +) > > CPU: Exynos4412 @ 1 GHz > Model: Odroid based on Exynos4412 > Type: u3 > DRAM: 2 GiB > LDO20@VDDQ_EMMC_1.8V: set 180 uV; enabling > LDO22@VDDQ_EMMC_2.8V: set 280 uV; enabling > LDO21@TFLASH_2.8V: set 280 uV; enabling > MMC: SAMSUNG SDHCI: 1, EXYNOS DWMMC: 0 > Loading Environment from MMC... Card did not respond to voltage select! > *** Warning - No block device, using default environment > > Net: No ethernet found. > Hit any key to stop autoboot: 0 > switch to partitions #0, OK > mmc1 is current device > Scanning mmc 1:1... > Found U-Boot script /boot/boot.scr > 769 bytes read in 6 ms (125 KiB/s) > ## Executing script at 4200 > 6901856 bytes read in 253 ms (26 MiB/s) > 53078 bytes read in 19 ms (2.7 MiB/s) > 6590950 bytes read in 240 ms (26.2 MiB/s) > Kernel image @ 0x4100 [ 0x00 - 0x695060 ] > ## Flattened Device Tree blob at 4080 >Booting using the fdt blob at 0x4080 >Loading Ramdisk to 4f9b6000, end 41e6 ... OK >Loading Device Tree to 4f9a6000, end 4f9b5f55 ... OK > > Starting kernel ... Can you attach your boot.init file (the source of boot.scr)? By the size of DTB you can see that different DTB is loaded. I assume you tried to boot the same kernel, then it could mean that boardname was used instead of board_name to choose DTB. Suspicious is that kernel size also differs... so maybe you booted something else? As usual debugging practice, reduce number of unknown factors. Do not change kernel and U-Boot at the same time. Also, you can try reverting commit e6b1467081d3 ("arm: exynos: Remove duplicated "boardname" env setting") and see if it helps. Or just try to bisect around changes coming from u-boot samsung tree. Try booting v2019.01 and paste the results as well. Best regards, Krzysztof > > Old u-boot boots fine see the logs below. > -- > U-Boot 2018.01-1 (Feb 13 2018 - 02:19:55 +) Arch Linux ARM > > CPU: Exynos4412 @ 1 GHz > Model: Odroid based on Exynos4412 > Board: Odroid based on Exynos4412 > Type: u3 > DRAM: 2 GiB > LDO20@VDDQ_EMMC_1.8V: set 180 uV; enabling > LDO22@VDDQ_EMMC_2.8V: set 280 uV; enabling > LDO21@TFLASH_2.8V: set 280 uV; enabling > MMC: sdhci@1253 - probe failed: -19 > > *** Warning - bad CRC, using default environment > > Net: No ethernet found. > Hit any key to stop autoboot: 0 > no mmc device at slot 1 > switch to partitions #0, OK > mmc0(part 0) is current device > Scanning mmc 0:1... > Found U-Boot script /boot/boot.scr > 770 bytes read in 31 ms (23.4 KiB/s) > ## Executing script at 5000 > 6625416 bytes read in 267 ms (23.7 MiB/s) > 72531 bytes read in 483 ms (146.5 KiB/s) > 6590888 bytes read in 256 ms (24.6 MiB/s) > Kernel image @ 0x4200 [ 0x00 - 0x651888 ] > ## Flattened Device Tree blob at 4300 >Booting using the fdt blob at 0x4300 >Loading Ramdisk to 4f9b6000, end 41a8 ... OK >Loading Device Tree to 4f9a1000, end 4f9b5b52 ... OK > > Starting kernel ... > > [0.00] Booting Linux on physical CPU 0xa00 > [0.00] Linux version 5.0.1-1-ARCH (builduser@leming) (gcc > version 8.2.1 20181127 (GCC)) #1 SMP PREEMPT Tue Mar 12 04:09:35 UTC > 2019 > [0.00] CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7), cr=10c5387d > [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing > instruction cache > [0.00] OF: fdt: Machine model: Hardkernel ODROID-U3 board > based on Exynos4412 > [0.00] Memory policy: Data cache writealloc > [0.00] Reserved memory: created DMA memory pool at 0xbf70, > size 8 MiB > [0.00] OF: reserved mem: initialized node region_mfc_right, > compatible id shared-dma-pool > [0.00] Reserved memory: created DMA memory pool at 0xbd30, > size 36 MiB > > Please could you share the details on how to debug this issue or how to > resolve. > > Best Regards > -Anand ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Odroid U3 - Upgrade to latest u-boot kernel load fails.
On Mon, 18 Mar 2019 at 18:49, Anand Moon wrote: > > Hi Krzysztof, > > On Mon, 18 Mar 2019 at 18:20, Krzysztof Kozlowski wrote: > > > > On Mon, 18 Mar 2019 at 13:32, Anand Moon wrote: > > > > > > Hi Krzysztof / Marek, > > > > > > After I update the latest u-boot on my Odroud U3+ it fails to load the > > > kernel it hangs. > > > I am using Archlinux on Odroid U3. > > > > > > U-Boot 2019.04-rc3-00131-g8303467e80d-dirty (Mar 18 2019 - 12:12:23 +) > > > > > > CPU: Exynos4412 @ 1 GHz > > > Model: Odroid based on Exynos4412 > > > Type: u3 > > > DRAM: 2 GiB > > > LDO20@VDDQ_EMMC_1.8V: set 180 uV; enabling > > > LDO22@VDDQ_EMMC_2.8V: set 280 uV; enabling > > > LDO21@TFLASH_2.8V: set 280 uV; enabling > > > MMC: SAMSUNG SDHCI: 1, EXYNOS DWMMC: 0 > > > Loading Environment from MMC... Card did not respond to voltage select! > > > *** Warning - No block device, using default environment > > > > > > Net: No ethernet found. > > > Hit any key to stop autoboot: 0 > > > switch to partitions #0, OK > > > mmc1 is current device > > > Scanning mmc 1:1... > > > Found U-Boot script /boot/boot.scr > > > 769 bytes read in 6 ms (125 KiB/s) > > > ## Executing script at 4200 > > > 6901856 bytes read in 253 ms (26 MiB/s) > > > 53078 bytes read in 19 ms (2.7 MiB/s) > > > 6590950 bytes read in 240 ms (26.2 MiB/s) > > > Kernel image @ 0x4100 [ 0x00 - 0x695060 ] > > > ## Flattened Device Tree blob at 4080 > > >Booting using the fdt blob at 0x4080 > > >Loading Ramdisk to 4f9b6000, end 41e6 ... OK > > >Loading Device Tree to 4f9a6000, end 4f9b5f55 ... OK > > > > > > Starting kernel ... > > > > Can you attach your boot.init file (the source of boot.scr)? > > > > By the size of DTB you can see that different DTB is loaded. I assume > > you tried to boot the same kernel, then it could mean that boardname > > was used instead of board_name to choose DTB. Suspicious is that > > kernel size also differs... so maybe you booted something else? As > > usual debugging practice, reduce number of unknown factors. Do not > > change kernel and U-Boot at the same time. > > > > Also, you can try reverting commit e6b1467081d3 ("arm: exynos: Remove > > duplicated "boardname" env setting") and see if it helps. Or just try > > to bisect around changes coming from u-boot samsung tree. Try booting > > v2019.01 and paste the results as well. > > > > Best regards, > > Krzysztof > > > > I have tested with pre-compiled image and the cross compiled kernel image > both failed to load the kernel. > > No revert of the commit e6b1467081d3 did not help. > > Yes it seem strange that that load address is changes some how in the u-boot > env > > printenv setting of the latest u-boot U-Boot > 2019.04-rc3-00131-g8303467e80d-dirty > [0] https://pastebin.com/1Hgc5xxC (newu-boot.txt) > > printenv setting for the old u-boot. U-Boot 2018.01-1 > [1] https://pastebin.com/wD6zK6eG(oldu-boot.txt) > > Attach is the boot.txt (boot.scr), also the u-boot env old and u-boot-env new And does 2019.01 work? Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC 1/6] odroid: exynos: USB initialization on the U3/X2
On Mon, 1 Apr 2019 at 13:52, Anand Moon wrote: > > From: Tobias Jakobi > > Rename board_usb_init() to exynos_usb_init() and call it > early in the Exynos EHCI driver when probing. > > This kind of works. After a 'usb start; usb stop; usb start' > cycle the attached devices are recognized. > > Add small delay between gpio_direction_output to stable > initialization of usb gpio pins. > > Signed-off-by: Tobias Jakobi > Signed-off-by: Anand Moon > --- > Reoder the exynos_usb_init so that "usb start" command initialization > correcly. > --- > --- > board/samsung/odroid/odroid.c | 14 +- > drivers/usb/host/ehci-exynos.c | 7 +++ > 2 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c > index 3e594fd850..79d14ead01 100644 > --- a/board/samsung/odroid/odroid.c > +++ b/board/samsung/odroid/odroid.c > @@ -468,8 +468,6 @@ struct dwc2_plat_otg_data s5pc210_otg_data = { > }; > #endif > > -#if defined(CONFIG_USB_GADGET) || defined(CONFIG_CMD_USB) > - > static void set_usb3503_ref_clk(void) > { > #ifdef CONFIG_BOARD_TYPES > @@ -490,9 +488,8 @@ static void set_usb3503_ref_clk(void) > #endif /* CONFIG_BOARD_TYPES */ > } > > -int board_usb_init(int index, enum usb_init_type init) > +int exynos_usb_init(void) > { > -#ifdef CONFIG_CMD_USB > struct udevice *dev; > int ret; > > @@ -501,6 +498,7 @@ int board_usb_init(int index, enum usb_init_type init) > /* Disconnect, Reset, Connect */ > gpio_direction_output(EXYNOS4X12_GPIO_X34, 0); > gpio_direction_output(EXYNOS4X12_GPIO_X35, 0); > + sdelay(20); This should be a separate patch with its own explanation. > gpio_direction_output(EXYNOS4X12_GPIO_X35, 1); > gpio_direction_output(EXYNOS4X12_GPIO_X34, 1); > > @@ -530,7 +528,13 @@ int board_usb_init(int index, enum usb_init_type init) > pr_err("Regulator %s value setting error: %d\n", dev->name, > ret); > return ret; > } > -#endif > + > + return 0; > +} > + > +#ifdef CONFIG_USB_GADGET > +int board_usb_init(int index, enum usb_init_type init) > +{ > debug("USB_udc_probe\n"); > return dwc2_udc_probe(&s5pc210_otg_data); > } > diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c > index fabc662eb6..b0f7bd4936 100644 > --- a/drivers/usb/host/ehci-exynos.c > +++ b/drivers/usb/host/ehci-exynos.c > @@ -31,6 +31,8 @@ struct exynos_ehci_platdata { > struct gpio_desc vbus_gpio; > }; > > +extern int exynos_usb_init(void); > + > /** > * Contains pointers to register base addresses > * for the usb controller. > @@ -152,6 +154,11 @@ static void exynos4412_setup_usb_phy(struct > exynos4412_usb_phy *usb) > setbits_le32(&usb->usbphyrstcon, (RSTCON_HOSTPHY_SWRST | > RSTCON_SWRST)); > udelay(10); > clrbits_le32(&usb->usbphyrstcon, (RSTCON_HOSTPHY_SWRST | > RSTCON_SWRST)); > + > + /* > +* "usb start" initialize the usb driver > +*/ > + exynos_usb_init(); It should be something more generic like CONFIG_SYS_USB_OHCI_BOARD_INIT which calls board_usb_init()... but it still will be calling board code from the driver. Why do you need this in the first place? Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC 2/6] odroid: exynos: usb clean up for U3/X2
On Mon, 1 Apr 2019 at 13:52, Anand Moon wrote: > > Add board_usb_cleanup routine to cleanup after > de-registering it usb devices. Also fixed the > compilation error for other board. Fix for build error should be in separate commit. Please also quote the error you are fixing... because the code compiles fine in my case. > Signed-off-by: Anand Moon > --- > board/samsung/common/board.c | 4 ++-- > board/samsung/odroid/odroid.c | 5 + > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c > index 9adbd1e2cf..c74aca9b0a 100644 > --- a/board/samsung/common/board.c > +++ b/board/samsung/common/board.c > @@ -351,10 +351,10 @@ void reset_misc(void) > } > } > > +#ifdef CONFIG_USB_DWC3 > int board_usb_cleanup(int index, enum usb_init_type init) > { > -#ifdef CONFIG_USB_DWC3 > dwc3_uboot_exit(index); > -#endif > return 0; > } > +#endif > diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c > index 79d14ead01..547ae698cf 100644 > --- a/board/samsung/odroid/odroid.c > +++ b/board/samsung/odroid/odroid.c > @@ -538,4 +538,9 @@ int board_usb_init(int index, enum usb_init_type init) > debug("USB_udc_probe\n"); > return dwc2_udc_probe(&s5pc210_otg_data); > } > + > +int board_usb_cleanup(int index, enum usb_init_type init) > +{ > + return s5pc210_phy_control(index); Why you pass index of USB controller as argument "int on"? Index != on... Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC 3/6] usb: exynos: add init_after_reset for usb reset
On Mon, 1 Apr 2019 at 13:53, Anand Moon wrote: > > Some host controllers need addidional re-initialization Please run spell-check. > after ehci_reset() so we add .init_after_reset callback > which is requires to reinit the phy after controller reset. s/requires/required/ but you do not re-init the phy. The exynos_usb_init() performs the reset of usb3503 USB hub! > > Signed-off-by: Anand Moon > --- > drivers/usb/host/ehci-exynos.c | 20 +++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c > index b0f7bd4936..e6a542e092 100644 > --- a/drivers/usb/host/ehci-exynos.c > +++ b/drivers/usb/host/ehci-exynos.c > @@ -143,6 +143,23 @@ static void exynos5_setup_usb_phy(struct exynos_usb_phy > *usb) > EHCICTRL_ENAINCR16); > } > > +static int ehci_exynos_init_after_reset(struct ehci_ctrl *ehcntl) > +{ > + if (cpu_is_exynos4()) { > + if (proid_is_exynos4412()) { No need for double indentation. > + /* > +* "usb reset" cmd: restart re-initialize the usb > driver Just "reinitialize", not restart reinitialize. Best regards, Krzysztof > +*/ > + exynos_usb_init(); > + } > + } > + return 0; > +} > + > +static const struct ehci_ops exynos_ehci_ops = { > + .init_after_reset = ehci_exynos_init_after_reset, > +}; > + > static void exynos4412_setup_usb_phy(struct exynos4412_usb_phy *usb) > { > writel(CLK_24MHZ, &usb->usbphyclk); > @@ -234,7 +251,8 @@ static int ehci_usb_probe(struct udevice *dev) > hcor = (struct ehci_hcor *)((uint32_t)ctx->hcd + > HC_LENGTH(ehci_readl(&ctx->hcd->cr_capbase))); > > - return ehci_register(dev, ctx->hcd, hcor, NULL, 0, USB_INIT_HOST); > + return ehci_register(dev, ctx->hcd, hcor, &exynos_ehci_ops, > + 0, USB_INIT_HOST); > } > > static int ehci_usb_remove(struct udevice *dev) > -- > 2.21.0 > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC 4/6] configs: exynos: Add new CONFIG_SYS_ODROID_USB config option
On Mon, 1 Apr 2019 at 13:53, Anand Moon wrote: > > Add new CONFIG_SYS_ODROID_USB flag to avoid compliation > error on other development boards. > > Fix below compilation error: > Error: You must add new CONFIG options using Kconfig > The following new ad-hoc CONFIG options were detected: > CONFIG_SYS_ODROID_USB There is no ad-hoc option "SYS_ODROID_USB" so it cannot cause build error. This is something wrong... Are you sure that you are compiling master branch? Best regards, Krzysztof > > Signed-off-by: Anand Moon > --- > board/samsung/odroid/Kconfig | 3 +++ > drivers/usb/host/ehci-exynos.c | 6 ++ > include/configs/odroid.h | 1 + > 3 files changed, 10 insertions(+) > > diff --git a/board/samsung/odroid/Kconfig b/board/samsung/odroid/Kconfig > index 8b52a0d589..c5fbffabad 100644 > --- a/board/samsung/odroid/Kconfig > +++ b/board/samsung/odroid/Kconfig > @@ -9,4 +9,7 @@ config SYS_VENDOR > config SYS_CONFIG_NAME > default "odroid" > > +config SYS_ODROID_USB > + bool "Exynos4412 Odroid USB" > + > endif > diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c > index e6a542e092..3f62eba486 100644 > --- a/drivers/usb/host/ehci-exynos.c > +++ b/drivers/usb/host/ehci-exynos.c > @@ -31,7 +31,9 @@ struct exynos_ehci_platdata { > struct gpio_desc vbus_gpio; > }; > > +#ifdef CONFIG_SYS_ODROID_USB > extern int exynos_usb_init(void); > +#endif > > /** > * Contains pointers to register base addresses > @@ -145,6 +147,7 @@ static void exynos5_setup_usb_phy(struct exynos_usb_phy > *usb) > > static int ehci_exynos_init_after_reset(struct ehci_ctrl *ehcntl) > { > +#ifdef CONFIG_SYS_ODROID_USB > if (cpu_is_exynos4()) { > if (proid_is_exynos4412()) { > /* > @@ -153,6 +156,7 @@ static int ehci_exynos_init_after_reset(struct ehci_ctrl > *ehcntl) > exynos_usb_init(); > } > } > +#endif > return 0; > } > > @@ -172,10 +176,12 @@ static void exynos4412_setup_usb_phy(struct > exynos4412_usb_phy *usb) > udelay(10); > clrbits_le32(&usb->usbphyrstcon, (RSTCON_HOSTPHY_SWRST | > RSTCON_SWRST)); > > +#ifdef CONFIG_SYS_ODROID_USB > /* > * "usb start" initialize the usb driver > */ > exynos_usb_init(); > +#endif > } > > static void setup_usb_phy(struct exynos_usb_phy *usb) > diff --git a/include/configs/odroid.h b/include/configs/odroid.h > index 9f2d43e3fa..d8d30c0f62 100644 > --- a/include/configs/odroid.h > +++ b/include/configs/odroid.h > @@ -32,6 +32,7 @@ > #define CONFIG_SYS_MEMTEST_START CONFIG_SYS_SDRAM_BASE > #define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_SDRAM_BASE + 0x5E0) > #define CONFIG_SYS_LOAD_ADDR (CONFIG_SYS_SDRAM_BASE + 0x3E0) > +#define CONFIG_SYS_ODROID_USB > > #include > > -- > 2.21.0 > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC 5/6] arm: exynos: odroid: fix the confict scripaddr extra env setting
On Mon, 1 Apr 2019 at 13:53, Anand Moon wrote: > > fix the confict of scriptaddr address with ramdisk_addr_r Start with capital letter, so s/fix/Fix. End with a full-stop. > also add missing pxefile_addr_r u-boot extras env setting. s/also/Also/ ... but there is no conflict in the first place. These addresses are not used in the same time. Best regards, Krzysztof > > Signed-off-by: Anand Moon > --- > include/configs/odroid.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/configs/odroid.h b/include/configs/odroid.h > index d8d30c0f62..64819cf81c 100644 > --- a/include/configs/odroid.h > +++ b/include/configs/odroid.h > @@ -169,9 +169,10 @@ > "consoleoff=set console console=ram; save; reset\0" \ > "initrdname=uInitrd\0" \ > "ramdisk_addr_r=0x4200\0" \ > - "scriptaddr=0x4200\0" \ > "fdt_addr_r=0x4080\0" \ > "kernel_addr_r=0x4100\0" \ > + "scriptaddr=0x5000\0" \ > + "pxefile_addr_r=0x5100\0" \ > BOOTENV > > /* GPT */ > -- > 2.21.0 > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC 6/6] arm: exynos: add usbnet_devaddr setting to env
On Mon, 1 Apr 2019 at 13:53, Anand Moon wrote: > > Add usbnet_devaddr mac address to extra env setting > to avoid failure of ethernet driver while usb start. > > Odroid # usb start > starting USB... > USB0: USB EHCI 1.00 > scanning bus 0 for devices... 3 USB Device(s) found >scanning usb for storage devices... 0 Storage Device(s) found >scanning usb for ethernet devices... > Error: sms0 address not set. > Warning: failed to set MAC address > 1 Ethernet Device(s) found > > Signed-off-by: Anand Moon > --- > include/configs/odroid.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/configs/odroid.h b/include/configs/odroid.h > index 64819cf81c..b0402e8d49 100644 > --- a/include/configs/odroid.h > +++ b/include/configs/odroid.h > @@ -33,6 +33,7 @@ > #define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_SDRAM_BASE + 0x5E0) > #define CONFIG_SYS_LOAD_ADDR (CONFIG_SYS_SDRAM_BASE + 0x3E0) > #define CONFIG_SYS_ODROID_USB > +#define CONFIG_USBNET_DEV_ADDR "02:DE:AD:BE:EF:FF" > > #include > > @@ -173,6 +174,7 @@ > "kernel_addr_r=0x4100\0" \ > "scriptaddr=0x5000\0" \ > "pxefile_addr_r=0x5100\0" \ > + "usbethaddr=" __stringify(CONFIG_USBNET_DEV_ADDR) "\0" \ No. Either the user should set it or manufacturer (e.g. in EEPROM). Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC 2/6] odroid: exynos: usb clean up for U3/X2
On Mon, 1 Apr 2019 at 17:57, Anand Moon wrote: > > Hi Krzysztof, > > On Mon, 1 Apr 2019 at 18:21, Krzysztof Kozlowski wrote: > > > > On Mon, 1 Apr 2019 at 13:52, Anand Moon wrote: > > > > > > Add board_usb_cleanup routine to cleanup after > > > de-registering it usb devices. Also fixed the > > > compilation error for other board. > > > > Fix for build error should be in separate commit. Please also quote > > the error you are fixing... because the code compiles fine in my case. > > > > Compilation error happens on other target boards. like Odroid-XU3 OK, please quote it and fix the error in separate commit. > > > > Signed-off-by: Anand Moon > > > --- > > > board/samsung/common/board.c | 4 ++-- > > > board/samsung/odroid/odroid.c | 5 + > > > 2 files changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c > > > index 9adbd1e2cf..c74aca9b0a 100644 > > > --- a/board/samsung/common/board.c > > > +++ b/board/samsung/common/board.c > > > @@ -351,10 +351,10 @@ void reset_misc(void) > > > } > > > } > > > > > > +#ifdef CONFIG_USB_DWC3 > > > int board_usb_cleanup(int index, enum usb_init_type init) > > > { > > > -#ifdef CONFIG_USB_DWC3 > > > dwc3_uboot_exit(index); > > > -#endif > > > return 0; > > > } > > > +#endif > > > diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c > > > index 79d14ead01..547ae698cf 100644 > > > --- a/board/samsung/odroid/odroid.c > > > +++ b/board/samsung/odroid/odroid.c > > > @@ -538,4 +538,9 @@ int board_usb_init(int index, enum usb_init_type init) > > > debug("USB_udc_probe\n"); > > > return dwc2_udc_probe(&s5pc210_otg_data); > > > } > > > + > > > +int board_usb_cleanup(int index, enum usb_init_type init) > > > +{ > > > + return s5pc210_phy_control(index); > > > > Why you pass index of USB controller as argument "int on"? Index != on... > > > > Generally board_usb_cleanup do cleanup the index of the board_usb_init, > so I use this naturally. I will check and update this if needed. But s5pc210_phy_control not. Your comment does not really address my concerns... Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC 3/6] usb: exynos: add init_after_reset for usb reset
On Mon, 1 Apr 2019 at 18:05, Anand Moon wrote: > > Hi Krzysztof, > > On Mon, 1 Apr 2019 at 18:25, Krzysztof Kozlowski wrote: > > > > On Mon, 1 Apr 2019 at 13:53, Anand Moon wrote: > > > > > > Some host controllers need addidional re-initialization > > > > Please run spell-check. > > > > > after ehci_reset() so we add .init_after_reset callback > > > which is requires to reinit the phy after controller reset. > > > > s/requires/required/ > > I did run checkpatch before on this, It did not spotted and error or warning. > > > but you do not re-init the phy. The exynos_usb_init() performs > > the reset of usb3503 USB hub! > > > > Yes that is needed as we do not get the usb back after "usb reset" command. Then please update the commit message to describe what exactly you are doing and what you want to achieve. Someone might think that you are doing initialization in init-after-reset... but you want to perform different reset after reset of ehci... and while writing it maybe you will notice that it is a hack and probably not the best way to do it. Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC 4/6] configs: exynos: Add new CONFIG_SYS_ODROID_USB config option
On Mon, 1 Apr 2019 at 17:50, Anand Moon wrote: > > Hi Krzysztof, > > On Mon, 1 Apr 2019 at 18:27, Krzysztof Kozlowski wrote: > > > > On Mon, 1 Apr 2019 at 13:53, Anand Moon wrote: > > > > > > Add new CONFIG_SYS_ODROID_USB flag to avoid compliation > > > error on other development boards. > > > > > > Fix below compilation error: > > > Error: You must add new CONFIG options using Kconfig > > > The following new ad-hoc CONFIG options were detected: > > > CONFIG_SYS_ODROID_USB > > > > There is no ad-hoc option "SYS_ODROID_USB" so it cannot cause build error. > > > > This is something wrong... Are you sure that you are compiling master > > branch? > > > > Best regards, > > Krzysztof > > > > [snip] > > CONFIG_SYS_ODROID_USB is adder to fix compilation error other development > board. > so this code is specific to the Odroid U3 and Odroid X2 boards. > > I have compiled both XU3 and U3 build with no problem. You quoted the error message saying that ad-hoc option CONFIG_SYS_ODROID_USB was added. But such ad-hoc option does not exist. Now you say that you add this to fix build problem... I really do not understand. So let's simplify it - please tell me how I can reproduce the problem and what is expected. Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC 4/6] configs: exynos: Add new CONFIG_SYS_ODROID_USB config option
On Mon, 1 Apr 2019 at 19:17, Anand Moon wrote: > > Hi Krzysztof, > > On Mon, 1 Apr 2019 at 21:46, Krzysztof Kozlowski wrote: > > > > On Mon, 1 Apr 2019 at 17:50, Anand Moon wrote: > > > > > > Hi Krzysztof, > > > > > > On Mon, 1 Apr 2019 at 18:27, Krzysztof Kozlowski wrote: > > > > > > > > On Mon, 1 Apr 2019 at 13:53, Anand Moon wrote: > > > > > > > > > > Add new CONFIG_SYS_ODROID_USB flag to avoid compliation > > > > > error on other development boards. > > > > > > > > > > Fix below compilation error: > > > > > Error: You must add new CONFIG options using Kconfig > > > > > The following new ad-hoc CONFIG options were detected: > > > > > CONFIG_SYS_ODROID_USB > > > > > > > > There is no ad-hoc option "SYS_ODROID_USB" so it cannot cause build > > > > error. > > > > > > > > This is something wrong... Are you sure that you are compiling master > > > > branch? > > > > > > > > Best regards, > > > > Krzysztof > > > > > > > > > > [snip] > > > > > > CONFIG_SYS_ODROID_USB is adder to fix compilation error other development > > > board. > > > so this code is specific to the Odroid U3 and Odroid X2 boards. > > > > > > I have compiled both XU3 and U3 build with no problem. > > > > You quoted the error message saying that ad-hoc option > > CONFIG_SYS_ODROID_USB was added. But such ad-hoc option does not > > exist. Now you say that you add this to fix build problem... I really > > do not understand. So let's simplify it - please tell me how I can > > reproduce the problem and what is expected. > > > > Best regards, > > Krzysztof > > This CONFIG_SYS_ODROID_USB options is more like and compile time flag > defined in odroid.h and not on any other boards, so this flags add > guard to the code. > > At the end of u-boot compilation it would prompt that new config > option is added > please add this to some Kconfig so that it could be selected via make > menuconfig > > See the link below, why I chose this options. > > [0] https://github.com/Xilinx/u-boot-xlnx/blob/master/doc/README.kconfig Can you provide answer to this: "So let's simplify it - please tell me how I can reproduce the problem and what is expected." ? Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] doc: Fix outdated ohci board hook documentation
The ohci driver calls board_usb_init(), not usb_board_init(). Signed-off-by: Krzysztof Kozlowski --- doc/README.generic_usb_ohci | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/README.generic_usb_ohci b/doc/README.generic_usb_ohci index 61300c35758a..65b0896c7fd2 100644 --- a/doc/README.generic_usb_ohci +++ b/doc/README.generic_usb_ohci @@ -13,7 +13,7 @@ Configuration options CONFIG_SYS_USB_OHCI_BOARD_INIT: call the board dependant hooks: - - extern int usb_board_init(void); + - extern int board_usb_init(void); - extern int usb_board_stop(void); - extern int usb_cpu_init_fail(void); -- 2.7.4 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC 4/6] configs: exynos: Add new CONFIG_SYS_ODROID_USB config option
On Tue, 2 Apr 2019 at 09:52, Anand Moon wrote: > > Hi Krzysztof, > > On Tue, 2 Apr 2019 at 12:32, Krzysztof Kozlowski wrote: > > > > On Mon, 1 Apr 2019 at 19:17, Anand Moon wrote: > > > > > > Hi Krzysztof, > > > > > > On Mon, 1 Apr 2019 at 21:46, Krzysztof Kozlowski wrote: > > > > > > > > On Mon, 1 Apr 2019 at 17:50, Anand Moon wrote: > > > > > > > > > > Hi Krzysztof, > > > > > > > > > > On Mon, 1 Apr 2019 at 18:27, Krzysztof Kozlowski > > > > > wrote: > > > > > > > > > > > > On Mon, 1 Apr 2019 at 13:53, Anand Moon > > > > > > wrote: > > > > > > > > > > > > > > Add new CONFIG_SYS_ODROID_USB flag to avoid compliation > > > > > > > error on other development boards. > > > > > > > > > > > > > > Fix below compilation error: > > > > > > > Error: You must add new CONFIG options using Kconfig > > > > > > > The following new ad-hoc CONFIG options were detected: > > > > > > > CONFIG_SYS_ODROID_USB > > > > > > > > > > > > There is no ad-hoc option "SYS_ODROID_USB" so it cannot cause build > > > > > > error. > > > > > > > > > > > > This is something wrong... Are you sure that you are compiling > > > > > > master branch? > > > > > > > > > > > > Best regards, > > > > > > Krzysztof > > > > > > > > > > > > > > > > [snip] > > > > > > > > > > CONFIG_SYS_ODROID_USB is adder to fix compilation error other > > > > > development board. > > > > > so this code is specific to the Odroid U3 and Odroid X2 boards. > > > > > > > > > > I have compiled both XU3 and U3 build with no problem. > > > > > > > > You quoted the error message saying that ad-hoc option > > > > CONFIG_SYS_ODROID_USB was added. But such ad-hoc option does not > > > > exist. Now you say that you add this to fix build problem... I really > > > > do not understand. So let's simplify it - please tell me how I can > > > > reproduce the problem and what is expected. > > > > > > > > Best regards, > > > > Krzysztof > > > > > > This CONFIG_SYS_ODROID_USB options is more like and compile time flag > > > defined in odroid.h and not on any other boards, so this flags add > > > guard to the code. > > > > > > At the end of u-boot compilation it would prompt that new config > > > option is added > > > please add this to some Kconfig so that it could be selected via make > > > menuconfig > > > > > > See the link below, why I chose this options. > > > > > > [0] https://github.com/Xilinx/u-boot-xlnx/blob/master/doc/README.kconfig > > > > Can you provide answer to this: > > "So let's simplify it - please tell me how I can reproduce the problem > > and what is expected." > > ? > > > > Since I would like to keep the code specific to Odroid U3 and Odroid X2. > I have introduce CONFIG_SYS_ODROID_USB this flag in "include/configs/odroid.h" So you added it? And broke build? And then add new commit to fix it? > If your remove the entry from board/samsung/odroid/Kconfig you will > hit above error > at the end of the build process. In board/samsung/odroid/Kconfig there is no such option like CONFIG_SYS_ODROID_USB. It looks like this commit is not fixing anything in existing U-Boot sources, which you claim in commit message. If you break the building in one commit, do not fix it in second commit. Just do not break it at first place. Best regards, Krzysztof > At the end of the compilation it suggest to add this entry into Kconfig option > so that this flag could be select and build flag in boards/config file. > > OBJCOPY u-boot.srec > OBJCOPY u-boot-nodtb.bin > CAT u-boot-dtb.bin > COPYu-boot.bin > SYM u-boot.sym > CFGCHK u-boot.cfg > Error: You must add new CONFIG options using Kconfig > The following new ad-hoc CONFIG options were detected: > CONFIG_SYS_ODROID_USB > > Please add these via Kconfig instead. Find a suitable Kconfig > file and add a 'config' or 'menuconfig' option. > make: *** [Makefile:1003: all] Error 1 > > This flag is mostly used as a guard to avoid compilation error on other > boards. > > I could not find any other option to fix this issue. > If possible plz let me know. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 1/3] arm: exynos: arndale: Remove unused CONFIG_POWER and CONFIG_POWER_I2C
The CONFIG_POWER and CONFIG_POWER_I2C were introduced in include/configs/exynos5-common.h in commit 19bd3aaa5991 ("exynos5: fix build break by adding CONFIG_POWER") and then it propagated up to include/configs/arndale.h. However before that commit, there was no build break at all on Arndale and SMDK5250 boards. It seems the commit fixed nothing and just added unused defines. In fact, the Arndale board is not configuring its PMIC (S5M8767) which uses I2C bus. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Simon Glass --- Not tested on Arndale board. Testing is welcomed. Changes since v1: 1. Add Simon's tag. 2. Reorder patches - first remove CONFIG_POWER_I2C, then CONFIG_DM_I2C_COMPAT. --- include/configs/arndale.h | 8 1 file changed, 8 deletions(-) diff --git a/include/configs/arndale.h b/include/configs/arndale.h index dd321c4748d0..841f3616482b 100644 --- a/include/configs/arndale.h +++ b/include/configs/arndale.h @@ -29,10 +29,6 @@ #define CONFIG_SYS_INIT_SP_ADDRCONFIG_IRAM_STACK -/* PMIC */ -#define CONFIG_POWER -#define CONFIG_POWER_I2C - #define CONFIG_PREBOOT #define CONFIG_S5P_PA_SYSRAM 0x0202 @@ -41,8 +37,4 @@ /* The PERIPHBASE in the CBAR register is wrong on the Arndale, so override it */ #define CONFIG_ARM_GIC_BASE_ADDRESS0x1048 -/* Power */ -#define CONFIG_POWER -#define CONFIG_POWER_I2C - #endif /* __CONFIG_H */ -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 3/3] configs: arndale: Use appropriate driver for Asix AX88760
Arndale board has an Asix AX88760 USB 2.0 Hub and Fast Ethernet combo. The appropriate driver for it is USB_ETHER_ASIX. The mistake probably came from misinterpretation of commit e9954b867ce0 ("usb: eth: add ASIX AX88179 DRIVER") which was tested on RECS5250 COM module. This module indeed has Exynos5250 and some similarities with Arndale 5250 board but the USB/Ethernet chip used there is apparently different. Fixes: f58ad98a621c ("usb: net: migrate USB Ethernet adapters to Kconfig") Signed-off-by: Krzysztof Kozlowski Reviewed-by: Lukasz Majewski --- Not tested. Changes since v1: 1. Add Lukasz's tag. --- configs/arndale_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configs/arndale_defconfig b/configs/arndale_defconfig index e90d670f6813..37ad6accc396 100644 --- a/configs/arndale_defconfig +++ b/configs/arndale_defconfig @@ -42,4 +42,4 @@ CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_HOST_ETHER=y -CONFIG_USB_ETHER_ASIX88179=y +CONFIG_USB_ETHER_ASIX=y -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 2/3] arm: exynos: arndale: Remove unused CONFIG_DM_I2C_COMPAT
The CONFIG_DM_I2C_COMPAT was introduced in include/configs/exynos5-common.h in commit 189d80166b31 ("exynos5: enable dm i2c") and then it propagated up to configs/arndale_defconfig. However since beginning the Arndale board (Exynos5250) was not using I2C. In fact, the Arndale board is not configuring its PMIC (S5M8767) which uses I2C bus. This setting can be thus safely removed to fix build warning: This board uses CONFIG_DM_I2C_COMPAT. Please remove (possibly in a subsequent patch in your series) before sending patches to the mailing list. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Simon Glass --- Not tested on Arndale board. Testing is welcomed. Changes since v1: 1. Add Simon's tag. 2. Reorder patches - first remove CONFIG_POWER_I2C, then CONFIG_DM_I2C_COMPAT. --- configs/arndale_defconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/configs/arndale_defconfig b/configs/arndale_defconfig index 9727d28c1241..e90d670f6813 100644 --- a/configs/arndale_defconfig +++ b/configs/arndale_defconfig @@ -25,7 +25,6 @@ CONFIG_CMD_SOUND=y CONFIG_CMD_EXT4_WRITE=y CONFIG_DEFAULT_DEVICE_TREE="exynos5250-arndale" CONFIG_ENV_IS_IN_MMC=y -CONFIG_DM_I2C_COMPAT=y CONFIG_SUPPORT_EMMC_BOOT=y CONFIG_MMC_DW=y CONFIG_MMC_SDHCI=y -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: exynos: odroid: Fix the confict scripaddr extra env setting
On Fri, 24 May 2019 at 10:51, Anand Moon wrote: > > Fix the confict of scriptaddr address with ramdisk_addr_r used > in EXTRA_ENV_SETTINGS. > > Signed-off-by: Anand Moon My comment from previous patch stays valid: "... but there is no conflict in the first place. These addresses are not used in the same time." The patch does not harm but it is not correctly explained. There is no issue so it is not a fix. Best regards, Krzysztof > > --- > Prevoius patch: > > [0] https://marc.info/?l=u-boot&m=155411969503169&w=2 > > changes from prevoius changes: > drop: "pxefile_addr_r=0x5100\0" \ > > U-Boot 2019.07-rc2-00199-g40920bdecc4-dirty (May 24 2019 - 06:39:42 +) > > CPU: Exynos4412 @ 1 GHz > Model: Odroid based on Exynos4412 > Type: u3 > DRAM: 2 GiB > LDO20@VDDQ_EMMC_1.8V: set 180 uV; enabling > LDO22@VDDQ_EMMC_2.8V: set 280 uV; enabling > LDO21@TFLASH_2.8V: set 280 uV; enabling > MMC: SAMSUNG SDHCI: 1, EXYNOS DWMMC: 0 > Loading Environment from MMC... Card did not respond to voltage select! > *** Warning - No block device, using default environment > > Net: No ethernet found. > Hit any key to stop autoboot: 0 > switch to partitions #0, OK > mmc1 is current device > Scanning mmc 1:1... > Found U-Boot script /boot/boot.scr > 775 bytes read in 5 ms (151.4 KiB/s) > 6688712 bytes read in 229 ms (27.9 MiB/s) > 72645 bytes read in 41 ms (1.7 MiB/s) > 6611360 bytes read in 227 ms (27.8 MiB/s) > Kernel image @ 0x4100 [ 0x00 - 0x660fc8 ] >Booting using the fdt blob at 0x4080 >Loading Ramdisk to 4f9b1000, end 41a0 ... OK >Loading Device Tree to 4f99c000, end 4f9b0bc4 ... OK > --- > include/configs/odroid.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/configs/odroid.h b/include/configs/odroid.h > index 9f2d43e3fa3..04bed6b0160 100644 > --- a/include/configs/odroid.h > +++ b/include/configs/odroid.h > @@ -168,7 +168,7 @@ > "consoleoff=set console console=ram; save; reset\0" \ > "initrdname=uInitrd\0" \ > "ramdisk_addr_r=0x4200\0" \ > - "scriptaddr=0x4200\0" \ > + "scriptaddr=0x5000\0" \ > "fdt_addr_r=0x4080\0" \ > "kernel_addr_r=0x4100\0" \ > BOOTENV > -- > 2.21.0 > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 1/9] adc: exynos-adc: Fix wrong bit operation used to stop the ADC
When stopping the ADC_V2_CON1_STC_EN should be cleared. Signed-off-by: Krzysztof Kozlowski --- drivers/adc/exynos-adc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/adc/exynos-adc.c b/drivers/adc/exynos-adc.c index d33e3d632afc..12c49fc8cefb 100644 --- a/drivers/adc/exynos-adc.c +++ b/drivers/adc/exynos-adc.c @@ -62,7 +62,7 @@ int exynos_adc_stop(struct udevice *dev) /* Stop conversion */ cfg = readl(®s->con1); - cfg |= ~ADC_V2_CON1_STC_EN; + cfg &= ~ADC_V2_CON1_STC_EN; writel(cfg, ®s->con1); -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 0/9] arm: exynos: Fix reboot on Odroid HC1
Hi, Changes since v1 1. Move fixes to beginning of patchset. 2. Patch 3: Rework the idea - split revision detection to be executed later. 3. Patch 4: New patch. 4. Patch 6: Apply Simon's comments. 5. Patch 6: Do not delay when changing voltage if regulator is disabled. 6. Patch 6: Do not delay when disabling the regulator. Description === Odroid HC1 does not reboot properly (at least from SD card but I do not expect difference on eMMC), if LDO4/VDD_ADC was turned off by Linux kernel. This condition happens so far always, because Linux kernel did not enable ADC on Odroid HC1, therefore the VDD_ADC regulator was turned off as unused. The issue is in detection of revision which later is used to load proper DTB. The revision is obtained by ADC read of a voltage depending on VDD_ADC. Therefore: 1. VDD_ADC has to be turned on (but board detection happens before power is initialized), 2. Turning VDD_ADC should wait with ramp delay, 3. Reading the value from ADC should wait for it to stabilize. Tested on Odroid XU3-Lite and Odroid HC1. Commends and testing are welcomed. Best regards, Krzysztof Krzysztof Kozlowski (9): adc: exynos-adc: Fix wrong bit operation used to stop the ADC power: regulator: s2mps11: Fix step for LDO27 and LDO35 arm: exynos: Detect revision later, when all resources are ready arm: exynos: odroid-xu3: Display info late to have proper type arm: exynos: Wait till ADC stabilizes before checking Odroid HC1 revision regulator: Add support for ramp delay power: regulator: s2mps11: Add enable delay arm: dts: exynos: Add supply for ADC block to Odroid XU3 family arm: dts: exynos: Add ramp delay property to LDO regulators to Odroid XU3 family arch/arm/dts/exynos5422-odroidxu3.dts | 20 +++ board/samsung/common/board.c | 15 - board/samsung/common/exynos5-dt-types.c | 58 +-- board/samsung/odroid/odroid.c | 8 +++ configs/odroid-xu3_defconfig | 2 + .../regulator/regulator.txt | 2 + drivers/adc/exynos-adc.c | 2 +- drivers/power/regulator/regulator-uclass.c| 47 ++- drivers/power/regulator/s2mps11_regulator.c | 13 - include/power/regulator.h | 2 + include/samsung/misc.h| 1 + 11 files changed, 160 insertions(+), 10 deletions(-) -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 3/9] arm: exynos: Detect revision later, when all resources are ready
Detection of board revision is done early - before power setup. In case of Odroid XU3/XU4/HC1 family, the detection is done using ADC which is supplied by LDO4/VDD_ADC regulator. This regulator could be turned off (e.g. by kernel before reboot). If ADC is used early, the regulators are not yet available and the detection won't work. Split the revision detection out of set_board_type() into separate function called later - either when displaying board info or during misc_init_r (whatever succeeds first). The idea is that set_board_type() will be called early so its method of detection are limited to flattened device tree (exynos5-dt-types.c for Exynos5) or GPIO (odroid.c for Exynos4412). The newly added set_board_revision() can be called only later, when resources like regulator are available. This is necessary to fix the detection of Odroid HC1 after reboot, if kernel turned off the LDO4 regulator. Signed-off-by: Krzysztof Kozlowski --- board/samsung/common/board.c| 13 - board/samsung/common/exynos5-dt-types.c | 20 +--- board/samsung/odroid/odroid.c | 8 include/samsung/misc.h | 1 + 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c index 96228a86a117..52a2764a1919 100644 --- a/board/samsung/common/board.c +++ b/board/samsung/common/board.c @@ -253,7 +253,12 @@ int board_eth_init(bd_t *bis) int checkboard(void) { if (IS_ENABLED(CONFIG_BOARD_TYPES)) { - const char *board_info = get_board_type(); + const char *board_info; + + /* Printing type requires having revision */ + set_board_revision(); + + board_info = get_board_type(); if (board_info) printf("Type: %s\n", board_info); @@ -287,6 +292,12 @@ int board_late_init(void) #ifdef CONFIG_MISC_INIT_R int misc_init_r(void) { + /* +* At this point regulators should be available so do full +* revision detection +*/ + set_board_revision(); + #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG set_board_info(); #endif diff --git a/board/samsung/common/exynos5-dt-types.c b/board/samsung/common/exynos5-dt-types.c index 7a86e9187768..af711e727a78 100644 --- a/board/samsung/common/exynos5-dt-types.c +++ b/board/samsung/common/exynos5-dt-types.c @@ -192,8 +192,11 @@ const char *get_board_type(void) /** * set_board_type() - set board type in gd->board_type. - * As default type set EXYNOS5_BOARD_GENERIC, if detect Odroid, - * then set its proper type. + * As default type set EXYNOS5_BOARD_GENERIC. If Odroid is detected, + * set its proper type based on device tree. + * + * This might be called early when some more specific ways to detect revision + * are not yet available. */ void set_board_type(void) { @@ -211,8 +214,19 @@ void set_board_type(void) gd->board_type = of_match->data; break; } +} + +/** + * set_board_revision() - set detailed board type in gd->board_type. + * Should be called when resources (e.g. regulators) are available + * so ADC can be used to detect the specific revision of a board. + */ +void set_board_revision(void) +{ + /* Do not detect revision twice */ + if (gd->board_type == EXYNOS5_BOARD_GENERIC) + return; - /* If Odroid, then check its revision */ if (board_is_odroidxu3()) gd->board_type = odroid_get_board_type(); } diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c index 552333fe869d..4be8cc9826c3 100644 --- a/board/samsung/odroid/odroid.c +++ b/board/samsung/odroid/odroid.c @@ -54,6 +54,14 @@ void set_board_type(void) gd->board_type = ODROID_TYPE_U3; } +void set_board_revision(void) +{ + /* +* Revision already set by set_board_type() because it can be +* executed early. +*/ +} + const char *get_board_type(void) { const char *board_type[] = {"u3", "x2"}; diff --git a/include/samsung/misc.h b/include/samsung/misc.h index 017560c25662..4ff28a1df0e8 100644 --- a/include/samsung/misc.h +++ b/include/samsung/misc.h @@ -33,6 +33,7 @@ char *get_dfu_alt_system(char *interface, char *devstr); char *get_dfu_alt_boot(char *interface, char *devstr); #endif void set_board_type(void); +void set_board_revision(void); const char *get_board_type(void); #endif /* __SAMSUNG_MISC_COMMON_H__ */ -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 2/9] power: regulator: s2mps11: Fix step for LDO27 and LDO35
LDO27 and LDO35 have 25 mV step, not 50 mV. Signed-off-by: Krzysztof Kozlowski --- drivers/power/regulator/s2mps11_regulator.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/power/regulator/s2mps11_regulator.c b/drivers/power/regulator/s2mps11_regulator.c index ced504eb1476..723d27f67c9a 100644 --- a/drivers/power/regulator/s2mps11_regulator.c +++ b/drivers/power/regulator/s2mps11_regulator.c @@ -346,6 +346,8 @@ static int s2mps11_ldo_hex2volt(int ldo, int hex) case 11: case 22: case 23: + case 27: + case 35: uV = hex * S2MPS11_LDO_STEP + S2MPS11_LDO_UV_MIN; break; default: @@ -366,6 +368,8 @@ static int s2mps11_ldo_volt2hex(int ldo, int uV) case 11: case 22: case 23: + case 27: + case 35: hex = (uV - S2MPS11_LDO_UV_MIN) / S2MPS11_LDO_STEP; break; default: -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 5/9] arm: exynos: Wait till ADC stabilizes before checking Odroid HC1 revision
Fix detection of Odroid HC1 (Exynos5422) after reboot if kernel disabled the LDO4/VDD_ADC regulator. The LDO4 supplies both ADC block and the ADC input AIN9. Voltage on AIN9 will rise slowly, so be patient and wait for it to stabilize. First reads on Odroid HC1 return 305, 1207, 1297 and finally 1308 (reference value is 1309). Signed-off-by: Krzysztof Kozlowski --- board/samsung/common/exynos5-dt-types.c | 38 - 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/board/samsung/common/exynos5-dt-types.c b/board/samsung/common/exynos5-dt-types.c index af711e727a78..8aed64183837 100644 --- a/board/samsung/common/exynos5-dt-types.c +++ b/board/samsung/common/exynos5-dt-types.c @@ -57,12 +57,48 @@ static unsigned int odroid_get_rev(void) return 0; } +/* + * Read ADC at least twice and check the resuls. If regulator providing voltage + * on to measured point was just turned on, first reads might require time + * to stabilize. + */ +static int odroid_get_adc_val(unsigned int *adcval) +{ + unsigned int adcval_prev = 0; + int ret, retries = 20; + + ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, + &adcval_prev); + if (ret) + return ret; + + while (retries--) { + mdelay(5); + + ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, + adcval); + if (ret) + return ret; + + /* +* If difference between ADC reads is less than 3%, +* accept the result +*/ + if ((100 * abs(*adcval - adcval_prev) / adcval_prev) < 3) + return ret; + + adcval_prev = *adcval; + } + + return ret; +} + static int odroid_get_board_type(void) { unsigned int adcval; int ret, i; - ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, &adcval); + ret = odroid_get_adc_val(&adcval); if (ret) goto rev_default; -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 4/9] arm: exynos: odroid-xu3: Display info late to have proper type
From: Krzysztof Kozlowski Printing the "Type" of board requires proper detection of revision which can happen only late because regulators are needed. Signed-off-by: Krzysztof Kozlowski --- board/samsung/common/board.c | 2 +- configs/odroid-xu3_defconfig | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c index 52a2764a1919..632732288e73 100644 --- a/board/samsung/common/board.c +++ b/board/samsung/common/board.c @@ -249,7 +249,7 @@ int board_eth_init(bd_t *bis) return 0; } -#ifdef CONFIG_DISPLAY_BOARDINFO +#if defined(CONFIG_DISPLAY_BOARDINFO) || defined(CONFIG_DISPLAY_BOARDINFO_LATE) int checkboard(void) { if (IS_ENABLED(CONFIG_BOARD_TYPES)) { diff --git a/configs/odroid-xu3_defconfig b/configs/odroid-xu3_defconfig index f6f05b294833..57aca5e015fc 100644 --- a/configs/odroid-xu3_defconfig +++ b/configs/odroid-xu3_defconfig @@ -11,6 +11,8 @@ CONFIG_FIT=y CONFIG_FIT_BEST_MATCH=y CONFIG_SILENT_CONSOLE=y CONFIG_CONSOLE_MUX=y +# CONFIG_DISPLAY_BOARDINFO is not set +CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_MISC_INIT_R=y CONFIG_SYS_PROMPT="ODROID-XU3 # " CONFIG_CMD_THOR_DOWNLOAD=y -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 6/9] regulator: Add support for ramp delay
Changing voltage and enabling regulator might require delays so the regulator stabilizes at expected level. Add support for "regulator-ramp-delay" binding which can introduce required time to both enabling the regulator and to changing the voltage. Signed-off-by: Krzysztof Kozlowski --- .../regulator/regulator.txt | 2 + drivers/power/regulator/regulator-uclass.c| 47 ++- include/power/regulator.h | 2 + 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/doc/device-tree-bindings/regulator/regulator.txt b/doc/device-tree-bindings/regulator/regulator.txt index 65b69c427899..4ba642b7c77f 100644 --- a/doc/device-tree-bindings/regulator/regulator.txt +++ b/doc/device-tree-bindings/regulator/regulator.txt @@ -35,6 +35,7 @@ Optional properties: - regulator-max-microamp: a maximum allowed Current value - regulator-always-on: regulator should never be disabled - regulator-boot-on: enabled by bootloader/firmware +- regulator-ramp-delay: ramp delay for regulator (in uV/us) Note The "regulator-name" constraint is used for setting the device's uclass @@ -60,4 +61,5 @@ ldo0 { regulator-max-microamp = <10>; regulator-always-on; regulator-boot-on; + regulator-ramp-delay = <12000>; }; diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 6f355b969a6d..363c6e6441fa 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -35,10 +35,22 @@ int regulator_get_value(struct udevice *dev) return ops->get_value(dev); } +static void regulator_set_value_delay(struct udevice *dev, int old_uV, + int new_uV, unsigned int ramp_delay) +{ + int delay = DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay); + + debug("regulator %s: delay %u us (%d uV -> %d uV)\n", dev->name, delay, + old_uV, new_uV); + + udelay(delay); +} + int regulator_set_value(struct udevice *dev, int uV) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); struct dm_regulator_uclass_platdata *uc_pdata; + int ret, old_uV = uV, is_enabled = 0; uc_pdata = dev_get_uclass_platdata(dev); if (uc_pdata->min_uV != -ENODATA && uV < uc_pdata->min_uV) @@ -49,7 +61,20 @@ int regulator_set_value(struct udevice *dev, int uV) if (!ops || !ops->set_value) return -ENOSYS; - return ops->set_value(dev, uV); + if (uc_pdata->ramp_delay) { + is_enabled = regulator_get_enable(dev); + old_uV = regulator_get_value(dev); + } + + ret = ops->set_value(dev, uV); + + if (!ret) { + if (uc_pdata->ramp_delay && old_uV > 0 && is_enabled) + regulator_set_value_delay(dev, old_uV, uV, + uc_pdata->ramp_delay); + } + + return ret; } /* @@ -107,6 +132,7 @@ int regulator_set_enable(struct udevice *dev, bool enable) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); struct dm_regulator_uclass_platdata *uc_pdata; + int ret, old_enable = 0; if (!ops || !ops->set_enable) return -ENOSYS; @@ -115,7 +141,22 @@ int regulator_set_enable(struct udevice *dev, bool enable) if (!enable && uc_pdata->always_on) return -EACCES; - return ops->set_enable(dev, enable); + if (uc_pdata->ramp_delay) + old_enable = regulator_get_enable(dev); + + ret = ops->set_enable(dev, enable); + if (!ret) { + if (uc_pdata->ramp_delay && !old_enable && enable) { + int uV = regulator_get_value(dev); + + if (uV > 0) { + regulator_set_value_delay(dev, 0, uV, + uc_pdata->ramp_delay); + } + } + } + + return ret; } int regulator_set_enable_if_allowed(struct udevice *dev, bool enable) @@ -335,6 +376,8 @@ static int regulator_pre_probe(struct udevice *dev) -ENODATA); uc_pdata->always_on = dev_read_bool(dev, "regulator-always-on"); uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on"); + uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay", + 0); /* Those values are optional (-ENODATA if unset) */ if ((uc_pdata->min_uV != -ENODATA) && diff --git a/include/power/regulator.h b/include/power/regulator.h index 314160a894b7..6c6e2cd4f996 100644 --- a/incl
[U-Boot] [PATCH v2 7/9] power: regulator: s2mps11: Add enable delay
According to datasheet, the output on LDO regulators will start appearing after 10-15 us. Signed-off-by: Krzysztof Kozlowski --- drivers/power/regulator/s2mps11_regulator.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/power/regulator/s2mps11_regulator.c b/drivers/power/regulator/s2mps11_regulator.c index 723d27f67c9a..1f1581852ee2 100644 --- a/drivers/power/regulator/s2mps11_regulator.c +++ b/drivers/power/regulator/s2mps11_regulator.c @@ -551,7 +551,14 @@ static int ldo_get_enable(struct udevice *dev) static int ldo_set_enable(struct udevice *dev, bool enable) { - return s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); + int ret; + + ret = s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); + + /* Wait the "enable delay" for voltage to start to rise */ + udelay(15); + + return ret; } static int ldo_get_mode(struct udevice *dev) -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 9/9] arm: dts: exynos: Add ramp delay property to LDO regulators to Odroid XU3 family
Add startup time to LDO regulators of S2MPS11 PMIC on Odroid XU3/XU4/HC1 family of boards to be sure the voltage is proper before relying on the regulator. The datasheet for all the S2MPS1x family is inconsistent here and does not specify unambiguously the value of ramp delay for LDO. It mentions 30 mV/us in one timing diagram but then omits it completely in LDO regulator characteristics table (it is specified for bucks). However the vendor kernels for Galaxy S5 and Odroid XU3 use values of 12 mV/us or 24 mV/us. Without the ramp delay value the consumers do not wait for voltage settle after changing it. Although the proper value of ramp delay for LDOs is unknown, it seems safer to use at least some value from reference kernel than to leave it unset. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Lukasz Majewski --- arch/arm/dts/exynos5422-odroidxu3.dts | 19 +++ 1 file changed, 19 insertions(+) diff --git a/arch/arm/dts/exynos5422-odroidxu3.dts b/arch/arm/dts/exynos5422-odroidxu3.dts index 9dfae90667cf..04ecc404f907 100644 --- a/arch/arm/dts/exynos5422-odroidxu3.dts +++ b/arch/arm/dts/exynos5422-odroidxu3.dts @@ -45,6 +45,7 @@ regulator-name = "vdd_ldo1"; regulator-min-microvolt = <100>; regulator-max-microvolt = <100>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -52,18 +53,21 @@ regulator-name = "vddq_mmc0"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; }; ldo4_reg: LDO4 { regulator-name = "vdd_adc"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; }; ldo5_reg: LDO5 { regulator-name = "vdd_ldo5"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -71,6 +75,7 @@ regulator-name = "vdd_ldo6"; regulator-min-microvolt = <100>; regulator-max-microvolt = <100>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -78,6 +83,7 @@ regulator-name = "vdd_ldo7"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -85,6 +91,7 @@ regulator-name = "vdd_ldo8"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -92,6 +99,7 @@ regulator-name = "vdd_ldo9"; regulator-min-microvolt = <300>; regulator-max-microvolt = <300>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -99,6 +107,7 @@ regulator-name = "vdd_ldo10"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>;
[U-Boot] [PATCH v2 8/9] arm: dts: exynos: Add supply for ADC block to Odroid XU3 family
The ADC block requires VDD supply to be on so provide one. Signed-off-by: Krzysztof Kozlowski --- arch/arm/dts/exynos5422-odroidxu3.dts | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/dts/exynos5422-odroidxu3.dts b/arch/arm/dts/exynos5422-odroidxu3.dts index e859dd1b981a..9dfae90667cf 100644 --- a/arch/arm/dts/exynos5422-odroidxu3.dts +++ b/arch/arm/dts/exynos5422-odroidxu3.dts @@ -32,6 +32,7 @@ adc@12D1 { u-boot,dm-pre-reloc; + vdd-supply = <&ldo4_reg>; status = "okay"; }; -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 4/9] arm: exynos: odroid-xu3: Display info late to have proper type
On Wed, 13 Feb 2019 at 17:49, Krzysztof Kozlowski wrote: > > From: Krzysztof Kozlowski > > Printing the "Type" of board requires proper detection of revision which > can happen only late because regulators are needed. > > Signed-off-by: Krzysztof Kozlowski I should sent it from my @kernel.org account. I'll fix it in v3 but let me wait for some more comments/review. Best regards, Krzysztof > --- > board/samsung/common/board.c | 2 +- > configs/odroid-xu3_defconfig | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c > index 52a2764a1919..632732288e73 100644 > --- a/board/samsung/common/board.c > +++ b/board/samsung/common/board.c > @@ -249,7 +249,7 @@ int board_eth_init(bd_t *bis) > return 0; > } > > -#ifdef CONFIG_DISPLAY_BOARDINFO > +#if defined(CONFIG_DISPLAY_BOARDINFO) || > defined(CONFIG_DISPLAY_BOARDINFO_LATE) > int checkboard(void) > { > if (IS_ENABLED(CONFIG_BOARD_TYPES)) { > diff --git a/configs/odroid-xu3_defconfig b/configs/odroid-xu3_defconfig > index f6f05b294833..57aca5e015fc 100644 > --- a/configs/odroid-xu3_defconfig > +++ b/configs/odroid-xu3_defconfig > @@ -11,6 +11,8 @@ CONFIG_FIT=y > CONFIG_FIT_BEST_MATCH=y > CONFIG_SILENT_CONSOLE=y > CONFIG_CONSOLE_MUX=y > +# CONFIG_DISPLAY_BOARDINFO is not set > +CONFIG_DISPLAY_BOARDINFO_LATE=y > CONFIG_MISC_INIT_R=y > CONFIG_SYS_PROMPT="ODROID-XU3 # " > CONFIG_CMD_THOR_DOWNLOAD=y > -- > 2.17.1 > > ___ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 5/9] arm: exynos: Wait till ADC stabilizes before checking Odroid HC1 revision
On Fri, 15 Feb 2019 at 08:16, Lukasz Majewski wrote: > > On Wed, 13 Feb 2019 17:46:44 +0100 > Krzysztof Kozlowski wrote: > > > Fix detection of Odroid HC1 (Exynos5422) after reboot if kernel > > disabled the LDO4/VDD_ADC regulator. > > > > The LDO4 supplies both ADC block and the ADC input AIN9. Voltage on > > AIN9 will rise slowly, so be patient and wait for it to stabilize. > > > > First reads on Odroid HC1 return 305, 1207, 1297 and finally 1308 > > (reference value is 1309). > > > > Signed-off-by: Krzysztof Kozlowski > > --- > > board/samsung/common/exynos5-dt-types.c | 38 > > - 1 file changed, 37 insertions(+), 1 > > deletion(-) > > > > diff --git a/board/samsung/common/exynos5-dt-types.c > > b/board/samsung/common/exynos5-dt-types.c index > > af711e727a78..8aed64183837 100644 --- > > a/board/samsung/common/exynos5-dt-types.c +++ > > b/board/samsung/common/exynos5-dt-types.c @@ -57,12 +57,48 @@ static > > unsigned int odroid_get_rev(void) return 0; > > } > > > > +/* > > + * Read ADC at least twice and check the resuls. If regulator > > providing voltage > > + * on to measured point was just turned on, first reads might > > require time > > + * to stabilize. > > + */ > > +static int odroid_get_adc_val(unsigned int *adcval) > > +{ > > + unsigned int adcval_prev = 0; > > + int ret, retries = 20; > > + > > + ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, > > + &adcval_prev); > > + if (ret) > > + return ret; > > + > > + while (retries--) { > > + mdelay(5); > > + > > + ret = adc_channel_single_shot("adc", > > CONFIG_ODROID_REV_AIN, > > + adcval); > > + if (ret) > > + return ret; > > + > > + /* > > + * If difference between ADC reads is less than 3%, > > + * accept the result > > + */ > > + if ((100 * abs(*adcval - adcval_prev) / adcval_prev) > > < 3) > > + return ret; > > + > > + adcval_prev = *adcval; > > + } > > Is there in the documentation any required time to wait before reading > the ADC value? No, I think this delay is not SoC specific. The ADC already has proper delay/conversion rounds. The only thing it misses is to wait for 25 cycles of ADC PCLK after SWRESET but I found that adding it does not affect anything. To my understanding this is delay is purely some charging or slow ramp rate (although measuring point is on simple voltage divider...). > If yes then maybe get_timer() based approach shall be used (if > get_timer() is available in this context)? > > Please see for example drivers/net/fec_mxc.c for how timeouts are > handled there. I can take a look. First read of ADC might be very early so maybe before times... but I will check if these could be used. Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 7/9] power: regulator: s2mps11: Add enable delay
On Fri, 15 Feb 2019 at 08:04, Lukasz Majewski wrote: > > On Wed, 13 Feb 2019 17:46:46 +0100 > Krzysztof Kozlowski wrote: > > > According to datasheet, the output on LDO regulators will start > > appearing after 10-15 us. > > > > Signed-off-by: Krzysztof Kozlowski > > --- > > drivers/power/regulator/s2mps11_regulator.c | 9 - > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/power/regulator/s2mps11_regulator.c > > b/drivers/power/regulator/s2mps11_regulator.c index > > 723d27f67c9a..1f1581852ee2 100644 --- > > a/drivers/power/regulator/s2mps11_regulator.c +++ > > b/drivers/power/regulator/s2mps11_regulator.c @@ -551,7 +551,14 @@ > > static int ldo_get_enable(struct udevice *dev) > > static int ldo_set_enable(struct udevice *dev, bool enable) > > { > > - return s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); > > + int ret; > > + > > + ret = s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); > > + > > + /* Wait the "enable delay" for voltage to start to rise */ > > + udelay(15); > > Isn't the enable delay provided/read from dts? > Or is it too early to have dtb parsed? We could read it from DTB... but I would need to add new property just for that. I can... just more commits for simple stuff :) > The udelay(15) seems a bit "magic" value (or is it specified in the > PMIC manual?). Yeah, it is magic value mentioned in PMIC manual (actually - 10-15 us). It is the same as ramp delay - PMIC specific value. Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 7/9] power: regulator: s2mps11: Add enable delay
On Fri, Feb 15, 2019 at 06:11:34PM +0100, Simon Glass wrote: > Hi Krzysztof, > > On Wed, 13 Feb 2019 at 17:47, Krzysztof Kozlowski wrote: > > > > According to datasheet, the output on LDO regulators will start > > appearing after 10-15 us. > > > > Signed-off-by: Krzysztof Kozlowski > > --- > > drivers/power/regulator/s2mps11_regulator.c | 9 - > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/power/regulator/s2mps11_regulator.c > > b/drivers/power/regulator/s2mps11_regulator.c > > index 723d27f67c9a..1f1581852ee2 100644 > > --- a/drivers/power/regulator/s2mps11_regulator.c > > +++ b/drivers/power/regulator/s2mps11_regulator.c > > @@ -551,7 +551,14 @@ static int ldo_get_enable(struct udevice *dev) > > > > static int ldo_set_enable(struct udevice *dev, bool enable) > > { > > - return s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); > > + int ret; > > + > > + ret = s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); > > How about: > > if (ret) > return ret; > Sure, good idea, thanks! Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 5/9] arm: exynos: Wait till ADC stabilizes before checking Odroid HC1 revision
On Fri, Feb 15, 2019 at 08:15:45AM +0100, Lukasz Majewski wrote: > On Wed, 13 Feb 2019 17:46:44 +0100 > Krzysztof Kozlowski wrote: > > > Fix detection of Odroid HC1 (Exynos5422) after reboot if kernel > > disabled the LDO4/VDD_ADC regulator. > > > > The LDO4 supplies both ADC block and the ADC input AIN9. Voltage on > > AIN9 will rise slowly, so be patient and wait for it to stabilize. > > > > First reads on Odroid HC1 return 305, 1207, 1297 and finally 1308 > > (reference value is 1309). > > > > Signed-off-by: Krzysztof Kozlowski > > --- > > board/samsung/common/exynos5-dt-types.c | 38 > > - 1 file changed, 37 insertions(+), 1 > > deletion(-) > > > > diff --git a/board/samsung/common/exynos5-dt-types.c > > b/board/samsung/common/exynos5-dt-types.c index > > af711e727a78..8aed64183837 100644 --- > > a/board/samsung/common/exynos5-dt-types.c +++ > > b/board/samsung/common/exynos5-dt-types.c @@ -57,12 +57,48 @@ static > > unsigned int odroid_get_rev(void) return 0; > > } > > > > +/* > > + * Read ADC at least twice and check the resuls. If regulator > > providing voltage > > + * on to measured point was just turned on, first reads might > > require time > > + * to stabilize. > > + */ > > +static int odroid_get_adc_val(unsigned int *adcval) > > +{ > > + unsigned int adcval_prev = 0; > > + int ret, retries = 20; > > + > > + ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, > > + &adcval_prev); > > + if (ret) > > + return ret; > > + > > + while (retries--) { > > + mdelay(5); > > + > > + ret = adc_channel_single_shot("adc", > > CONFIG_ODROID_REV_AIN, > > + adcval); > > + if (ret) > > + return ret; > > + > > + /* > > +* If difference between ADC reads is less than 3%, > > +* accept the result > > +*/ > > + if ((100 * abs(*adcval - adcval_prev) / adcval_prev) > > < 3) > > + return ret; > > + > > + adcval_prev = *adcval; > > + } > > Is there in the documentation any required time to wait before reading > the ADC value? > > If yes then maybe get_timer() based approach shall be used (if > get_timer() is available in this context)? > > Please see for example drivers/net/fec_mxc.c for how timeouts are > handled there. I must admit that I do not see benefit of timers... 1. Make code slightly more complicated (instead of simple retries and mdelay()). 2. Introduce no delay by itself so the ADC reads happen one after another. Probing ADC value fast does not work with my approach of waiting till the values get closer to each other... With timer-based approach, without delay I got: ADC = 660 ADC = 887 ADC = 1031 ADC = 1125 ADC = 1186 ADC = 1226 ADC = 1253 return - because the difference is too small (<3 %). I could narrow my threshold to 1%... but then: ADC = 651 ADC = 881 ADC = 1027 ADC = 1122 ADC = 1184 ADC = 1225 ADC = 1253 ADC = 1271 ADC = 1284 ADC = 1292 I prefer simpler method with delay. Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1
Hi, Changes since v2 1. Add Lukasz review tags. 2. Patch 7: Return on error, as suggested by Simon. 2. Patch 3: Use IS_ENABLED() to run revision detection only once - either during late display board or misc_init_r. Changes since v1 1. Move fixes to beginning of patchset. 2. Patch 3: Rework the idea - split revision detection to be executed later. 3. Patch 4: New patch. 4. Patch 6: Apply Simon's comments. 5. Patch 6: Do not delay when changing voltage if regulator is disabled. 6. Patch 6: Do not delay when disabling the regulator. Description === Odroid HC1 does not reboot properly (at least from SD card but I do not expect difference on eMMC), if LDO4/VDD_ADC was turned off by Linux kernel. This condition happens so far always, because Linux kernel did not enable ADC on Odroid HC1, therefore the VDD_ADC regulator was turned off as unused. The issue is in detection of revision which later is used to load proper DTB. The revision is obtained by ADC read of a voltage depending on VDD_ADC. Therefore: 1. VDD_ADC has to be turned on (but board detection happens before power is initialized), 2. Turning VDD_ADC should wait with ramp delay, 3. Reading the value from ADC should wait for it to stabilize. Tested on Odroid XU3-Lite and Odroid HC1. Commends and testing are welcomed. Best regards, Krzysztof Krzysztof Kozlowski (9): adc: exynos-adc: Fix wrong bit operation used to stop the ADC power: regulator: s2mps11: Fix step for LDO27 and LDO35 arm: exynos: Detect revision later, when all resources are ready arm: exynos: odroid-xu3: Display info late to have proper type arm: exynos: Wait till ADC stabilizes before checking Odroid HC1 revision regulator: Add support for ramp delay power: regulator: s2mps11: Add enable delay arm: dts: exynos: Add supply for ADC block to Odroid XU3 family arm: dts: exynos: Add ramp delay property to LDO regulators to Odroid XU3 family arch/arm/dts/exynos5422-odroidxu3.dts | 20 +++ board/samsung/common/board.c | 24 - board/samsung/common/exynos5-dt-types.c | 54 +-- board/samsung/odroid/odroid.c | 8 +++ configs/odroid-xu3_defconfig | 2 + .../regulator/regulator.txt | 2 + drivers/adc/exynos-adc.c | 2 +- drivers/power/regulator/regulator-uclass.c| 47 +++- drivers/power/regulator/s2mps11_regulator.c | 15 +- include/power/regulator.h | 2 + include/samsung/misc.h| 1 + 11 files changed, 167 insertions(+), 10 deletions(-) -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 1/9] adc: exynos-adc: Fix wrong bit operation used to stop the ADC
When stopping the ADC_V2_CON1_STC_EN should be cleared. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Lukasz Majewski --- drivers/adc/exynos-adc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/adc/exynos-adc.c b/drivers/adc/exynos-adc.c index d33e3d632afc..12c49fc8cefb 100644 --- a/drivers/adc/exynos-adc.c +++ b/drivers/adc/exynos-adc.c @@ -62,7 +62,7 @@ int exynos_adc_stop(struct udevice *dev) /* Stop conversion */ cfg = readl(®s->con1); - cfg |= ~ADC_V2_CON1_STC_EN; + cfg &= ~ADC_V2_CON1_STC_EN; writel(cfg, ®s->con1); -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 4/9] arm: exynos: odroid-xu3: Display info late to have proper type
Printing the "Type" of board requires proper detection of revision which can happen only late because regulators are needed. Signed-off-by: Krzysztof Kozlowski --- board/samsung/common/board.c | 2 +- configs/odroid-xu3_defconfig | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c index ef2204742e1d..4ffbd7254205 100644 --- a/board/samsung/common/board.c +++ b/board/samsung/common/board.c @@ -249,7 +249,7 @@ int board_eth_init(bd_t *bis) return 0; } -#ifdef CONFIG_DISPLAY_BOARDINFO +#if defined(CONFIG_DISPLAY_BOARDINFO) || defined(CONFIG_DISPLAY_BOARDINFO_LATE) int checkboard(void) { if (IS_ENABLED(CONFIG_BOARD_TYPES)) { diff --git a/configs/odroid-xu3_defconfig b/configs/odroid-xu3_defconfig index f6f05b294833..57aca5e015fc 100644 --- a/configs/odroid-xu3_defconfig +++ b/configs/odroid-xu3_defconfig @@ -11,6 +11,8 @@ CONFIG_FIT=y CONFIG_FIT_BEST_MATCH=y CONFIG_SILENT_CONSOLE=y CONFIG_CONSOLE_MUX=y +# CONFIG_DISPLAY_BOARDINFO is not set +CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_MISC_INIT_R=y CONFIG_SYS_PROMPT="ODROID-XU3 # " CONFIG_CMD_THOR_DOWNLOAD=y -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 2/9] power: regulator: s2mps11: Fix step for LDO27 and LDO35
LDO27 and LDO35 have 25 mV step, not 50 mV. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Lukasz Majewski --- drivers/power/regulator/s2mps11_regulator.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/power/regulator/s2mps11_regulator.c b/drivers/power/regulator/s2mps11_regulator.c index ced504eb1476..723d27f67c9a 100644 --- a/drivers/power/regulator/s2mps11_regulator.c +++ b/drivers/power/regulator/s2mps11_regulator.c @@ -346,6 +346,8 @@ static int s2mps11_ldo_hex2volt(int ldo, int hex) case 11: case 22: case 23: + case 27: + case 35: uV = hex * S2MPS11_LDO_STEP + S2MPS11_LDO_UV_MIN; break; default: @@ -366,6 +368,8 @@ static int s2mps11_ldo_volt2hex(int ldo, int uV) case 11: case 22: case 23: + case 27: + case 35: hex = (uV - S2MPS11_LDO_UV_MIN) / S2MPS11_LDO_STEP; break; default: -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 5/9] arm: exynos: Wait till ADC stabilizes before checking Odroid HC1 revision
Fix detection of Odroid HC1 (Exynos5422) after reboot if kernel disabled the LDO4/VDD_ADC regulator. The LDO4 supplies both ADC block and the ADC input AIN9. Voltage on AIN9 will rise slowly, so be patient and wait for it to stabilize. First reads on Odroid HC1 return 305, 1207, 1297 and finally 1308 (reference value is 1309). Signed-off-by: Krzysztof Kozlowski --- board/samsung/common/exynos5-dt-types.c | 38 - 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/board/samsung/common/exynos5-dt-types.c b/board/samsung/common/exynos5-dt-types.c index 7c1271d6547a..516c32923e44 100644 --- a/board/samsung/common/exynos5-dt-types.c +++ b/board/samsung/common/exynos5-dt-types.c @@ -57,12 +57,48 @@ static unsigned int odroid_get_rev(void) return 0; } +/* + * Read ADC at least twice and check the resuls. If regulator providing voltage + * on to measured point was just turned on, first reads might require time + * to stabilize. + */ +static int odroid_get_adc_val(unsigned int *adcval) +{ + unsigned int adcval_prev = 0; + int ret, retries = 20; + + ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, + &adcval_prev); + if (ret) + return ret; + + while (retries--) { + mdelay(5); + + ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, + adcval); + if (ret) + return ret; + + /* +* If difference between ADC reads is less than 3%, +* accept the result +*/ + if ((100 * abs(*adcval - adcval_prev) / adcval_prev) < 3) + return ret; + + adcval_prev = *adcval; + } + + return ret; +} + static int odroid_get_board_type(void) { unsigned int adcval; int ret, i; - ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, &adcval); + ret = odroid_get_adc_val(&adcval); if (ret) goto rev_default; -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay
Changing voltage and enabling regulator might require delays so the regulator stabilizes at expected level. Add support for "regulator-ramp-delay" binding which can introduce required time to both enabling the regulator and to changing the voltage. Signed-off-by: Krzysztof Kozlowski --- .../regulator/regulator.txt | 2 + drivers/power/regulator/regulator-uclass.c| 47 ++- include/power/regulator.h | 2 + 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/doc/device-tree-bindings/regulator/regulator.txt b/doc/device-tree-bindings/regulator/regulator.txt index 65b69c427899..4ba642b7c77f 100644 --- a/doc/device-tree-bindings/regulator/regulator.txt +++ b/doc/device-tree-bindings/regulator/regulator.txt @@ -35,6 +35,7 @@ Optional properties: - regulator-max-microamp: a maximum allowed Current value - regulator-always-on: regulator should never be disabled - regulator-boot-on: enabled by bootloader/firmware +- regulator-ramp-delay: ramp delay for regulator (in uV/us) Note The "regulator-name" constraint is used for setting the device's uclass @@ -60,4 +61,5 @@ ldo0 { regulator-max-microamp = <10>; regulator-always-on; regulator-boot-on; + regulator-ramp-delay = <12000>; }; diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 6f355b969a6d..363c6e6441fa 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -35,10 +35,22 @@ int regulator_get_value(struct udevice *dev) return ops->get_value(dev); } +static void regulator_set_value_delay(struct udevice *dev, int old_uV, + int new_uV, unsigned int ramp_delay) +{ + int delay = DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay); + + debug("regulator %s: delay %u us (%d uV -> %d uV)\n", dev->name, delay, + old_uV, new_uV); + + udelay(delay); +} + int regulator_set_value(struct udevice *dev, int uV) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); struct dm_regulator_uclass_platdata *uc_pdata; + int ret, old_uV = uV, is_enabled = 0; uc_pdata = dev_get_uclass_platdata(dev); if (uc_pdata->min_uV != -ENODATA && uV < uc_pdata->min_uV) @@ -49,7 +61,20 @@ int regulator_set_value(struct udevice *dev, int uV) if (!ops || !ops->set_value) return -ENOSYS; - return ops->set_value(dev, uV); + if (uc_pdata->ramp_delay) { + is_enabled = regulator_get_enable(dev); + old_uV = regulator_get_value(dev); + } + + ret = ops->set_value(dev, uV); + + if (!ret) { + if (uc_pdata->ramp_delay && old_uV > 0 && is_enabled) + regulator_set_value_delay(dev, old_uV, uV, + uc_pdata->ramp_delay); + } + + return ret; } /* @@ -107,6 +132,7 @@ int regulator_set_enable(struct udevice *dev, bool enable) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); struct dm_regulator_uclass_platdata *uc_pdata; + int ret, old_enable = 0; if (!ops || !ops->set_enable) return -ENOSYS; @@ -115,7 +141,22 @@ int regulator_set_enable(struct udevice *dev, bool enable) if (!enable && uc_pdata->always_on) return -EACCES; - return ops->set_enable(dev, enable); + if (uc_pdata->ramp_delay) + old_enable = regulator_get_enable(dev); + + ret = ops->set_enable(dev, enable); + if (!ret) { + if (uc_pdata->ramp_delay && !old_enable && enable) { + int uV = regulator_get_value(dev); + + if (uV > 0) { + regulator_set_value_delay(dev, 0, uV, + uc_pdata->ramp_delay); + } + } + } + + return ret; } int regulator_set_enable_if_allowed(struct udevice *dev, bool enable) @@ -335,6 +376,8 @@ static int regulator_pre_probe(struct udevice *dev) -ENODATA); uc_pdata->always_on = dev_read_bool(dev, "regulator-always-on"); uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on"); + uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay", + 0); /* Those values are optional (-ENODATA if unset) */ if ((uc_pdata->min_uV != -ENODATA) && diff --git a/include/power/regulator.h b/include/power/regulator.h index 314160a894b7..6c6e2cd4f996 100644 --- a/incl
[U-Boot] [PATCH v3 9/9] arm: dts: exynos: Add ramp delay property to LDO regulators to Odroid XU3 family
Add startup time to LDO regulators of S2MPS11 PMIC on Odroid XU3/XU4/HC1 family of boards to be sure the voltage is proper before relying on the regulator. The datasheet for all the S2MPS1x family is inconsistent here and does not specify unambiguously the value of ramp delay for LDO. It mentions 30 mV/us in one timing diagram but then omits it completely in LDO regulator characteristics table (it is specified for bucks). However the vendor kernels for Galaxy S5 and Odroid XU3 use values of 12 mV/us or 24 mV/us. Without the ramp delay value the consumers do not wait for voltage settle after changing it. Although the proper value of ramp delay for LDOs is unknown, it seems safer to use at least some value from reference kernel than to leave it unset. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Lukasz Majewski --- arch/arm/dts/exynos5422-odroidxu3.dts | 19 +++ 1 file changed, 19 insertions(+) diff --git a/arch/arm/dts/exynos5422-odroidxu3.dts b/arch/arm/dts/exynos5422-odroidxu3.dts index 9dfae90667cf..04ecc404f907 100644 --- a/arch/arm/dts/exynos5422-odroidxu3.dts +++ b/arch/arm/dts/exynos5422-odroidxu3.dts @@ -45,6 +45,7 @@ regulator-name = "vdd_ldo1"; regulator-min-microvolt = <100>; regulator-max-microvolt = <100>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -52,18 +53,21 @@ regulator-name = "vddq_mmc0"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; }; ldo4_reg: LDO4 { regulator-name = "vdd_adc"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; }; ldo5_reg: LDO5 { regulator-name = "vdd_ldo5"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -71,6 +75,7 @@ regulator-name = "vdd_ldo6"; regulator-min-microvolt = <100>; regulator-max-microvolt = <100>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -78,6 +83,7 @@ regulator-name = "vdd_ldo7"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -85,6 +91,7 @@ regulator-name = "vdd_ldo8"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -92,6 +99,7 @@ regulator-name = "vdd_ldo9"; regulator-min-microvolt = <300>; regulator-max-microvolt = <300>; + regulator-ramp-delay = <12000>; regulator-always-on; }; @@ -99,6 +107,7 @@ regulator-name = "vdd_ldo10"; regulator-min-microvolt = <180>; regulator-max-microvolt = <180>; + regulator-ramp-delay = <12000>;
[U-Boot] [PATCH v3 3/9] arm: exynos: Detect revision later, when all resources are ready
Detection of board revision is done early - before power setup. In case of Odroid XU3/XU4/HC1 family, the detection is done using ADC which is supplied by LDO4/VDD_ADC regulator. This regulator could be turned off (e.g. by kernel before reboot). If ADC is used early, the regulators are not yet available and the detection won't work. Split the revision detection out of set_board_type() into separate function called later - either when displaying board info (in late mode) or during misc_init_r. The idea is that set_board_type() will be called early so its method of detection are limited to flattened device tree (exynos5-dt-types.c for Exynos5) or GPIO (odroid.c for Exynos4412). The newly added set_board_revision() can be called only later, when resources like regulator are available. This is necessary to fix the detection of Odroid HC1 after reboot, if kernel turned off the LDO4 regulator. Signed-off-by: Krzysztof Kozlowski --- board/samsung/common/board.c| 22 +- board/samsung/common/exynos5-dt-types.c | 16 +--- board/samsung/odroid/odroid.c | 8 include/samsung/misc.h | 1 + 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c index 96228a86a117..ef2204742e1d 100644 --- a/board/samsung/common/board.c +++ b/board/samsung/common/board.c @@ -253,7 +253,18 @@ int board_eth_init(bd_t *bis) int checkboard(void) { if (IS_ENABLED(CONFIG_BOARD_TYPES)) { - const char *board_info = get_board_type(); + const char *board_info; + + if (IS_ENABLED(CONFIG_DISPLAY_BOARDINFO_LATE)) { + /* +* Printing type requires having revision, although +* this will succeed only if done late. +* Otherwise revision will be set in misc_init_r(). +*/ + set_board_revision(); + } + + board_info = get_board_type(); if (board_info) printf("Type: %s\n", board_info); @@ -287,6 +298,15 @@ int board_late_init(void) #ifdef CONFIG_MISC_INIT_R int misc_init_r(void) { + if (!IS_ENABLED(CONFIG_DISPLAY_BOARDINFO_LATE)) { + /* +* If revision was not set by late display boardinfo, +* set it here. At this point regulators should be already +* available. +*/ + set_board_revision(); + } + #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG set_board_info(); #endif diff --git a/board/samsung/common/exynos5-dt-types.c b/board/samsung/common/exynos5-dt-types.c index 7a86e9187768..7c1271d6547a 100644 --- a/board/samsung/common/exynos5-dt-types.c +++ b/board/samsung/common/exynos5-dt-types.c @@ -192,8 +192,11 @@ const char *get_board_type(void) /** * set_board_type() - set board type in gd->board_type. - * As default type set EXYNOS5_BOARD_GENERIC, if detect Odroid, - * then set its proper type. + * As default type set EXYNOS5_BOARD_GENERIC. If Odroid is detected, + * set its proper type based on device tree. + * + * This might be called early when some more specific ways to detect revision + * are not yet available. */ void set_board_type(void) { @@ -211,8 +214,15 @@ void set_board_type(void) gd->board_type = of_match->data; break; } +} - /* If Odroid, then check its revision */ +/** + * set_board_revision() - set detailed board type in gd->board_type. + * Should be called when resources (e.g. regulators) are available + * so ADC can be used to detect the specific revision of a board. + */ +void set_board_revision(void) +{ if (board_is_odroidxu3()) gd->board_type = odroid_get_board_type(); } diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c index 552333fe869d..4be8cc9826c3 100644 --- a/board/samsung/odroid/odroid.c +++ b/board/samsung/odroid/odroid.c @@ -54,6 +54,14 @@ void set_board_type(void) gd->board_type = ODROID_TYPE_U3; } +void set_board_revision(void) +{ + /* +* Revision already set by set_board_type() because it can be +* executed early. +*/ +} + const char *get_board_type(void) { const char *board_type[] = {"u3", "x2"}; diff --git a/include/samsung/misc.h b/include/samsung/misc.h index 017560c25662..4ff28a1df0e8 100644 --- a/include/samsung/misc.h +++ b/include/samsung/misc.h @@ -33,6 +33,7 @@ char *get_dfu_alt_system(char *interface, char *devstr); char *get_dfu_alt_boot(char *interface, char *devstr); #endif void set_board_type(void); +void set_board_revision(void); const char *get_board_type(void); #endif /* __SAMSUNG_MISC_COMMON_H__ */ -- 2.17.1 _
[U-Boot] [PATCH v3 8/9] arm: dts: exynos: Add supply for ADC block to Odroid XU3 family
The ADC block requires VDD supply to be on so provide one. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Lukasz Majewski --- arch/arm/dts/exynos5422-odroidxu3.dts | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/dts/exynos5422-odroidxu3.dts b/arch/arm/dts/exynos5422-odroidxu3.dts index e859dd1b981a..9dfae90667cf 100644 --- a/arch/arm/dts/exynos5422-odroidxu3.dts +++ b/arch/arm/dts/exynos5422-odroidxu3.dts @@ -32,6 +32,7 @@ adc@12D1 { u-boot,dm-pre-reloc; + vdd-supply = <&ldo4_reg>; status = "okay"; }; -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 7/9] power: regulator: s2mps11: Add enable delay
According to datasheet, the output on LDO regulators will start appearing after 10-15 us. Signed-off-by: Krzysztof Kozlowski --- drivers/power/regulator/s2mps11_regulator.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/power/regulator/s2mps11_regulator.c b/drivers/power/regulator/s2mps11_regulator.c index 723d27f67c9a..67d1f9689de3 100644 --- a/drivers/power/regulator/s2mps11_regulator.c +++ b/drivers/power/regulator/s2mps11_regulator.c @@ -551,7 +551,16 @@ static int ldo_get_enable(struct udevice *dev) static int ldo_set_enable(struct udevice *dev, bool enable) { - return s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); + int ret; + + ret = s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable); + if (ret) + return ret; + + /* Wait the "enable delay" for voltage to start to rise */ + udelay(15); + + return 0; } static int ldo_get_mode(struct udevice *dev) -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay
On Mon, 18 Feb 2019 at 15:03, Torsten Duwe wrote: > > On Sat, Feb 16, 2019 at 10:45:45AM +0100, Krzysztof Kozlowski wrote: > > Changing voltage and enabling regulator might require delays so the > > regulator stabilizes at expected level. > > > > Add support for "regulator-ramp-delay" binding which can introduce > > required time to both enabling the regulator and to changing the > > voltage. > > I'm surprised that such a thing doesn't exist already. > > > Signed-off-by: Krzysztof Kozlowski > > > --- a/doc/device-tree-bindings/regulator/regulator.txt > > +++ b/doc/device-tree-bindings/regulator/regulator.txt > > @@ -35,6 +35,7 @@ Optional properties: > > - regulator-max-microamp: a maximum allowed Current value > > - regulator-always-on: regulator should never be disabled > > - regulator-boot-on: enabled by bootloader/firmware > > +- regulator-ramp-delay: ramp delay for regulator (in uV/us) > > I guess you mean s/V, not V/s; at least the code suggests so. uV/uS. It is correct in the code: int delay = DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay); delay = (uV - uV) / (uV / uS) = uS > But my main point is: is the required delay always a linear > function of the voltage jump? Depending on the dampening and > load on the rail this could be an overshoot and settle, no? > > So I suggest to make that an array with 2 elements: a fixed part > and a time per voltage change. Does that make sense? Just to make it clear - then we do not follow Linux kernel DT bindings. The voltage change might have exponential characteristic and/or have additional fixed delay time (see patch 7 here). We could re-use some properties from Linux bindings for that purpose: https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/regulator/regulator.txt#L19 https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/regulator/regulator.txt#L24 Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay
On Mon, 18 Feb 2019 at 16:27, Torsten Duwe wrote: > > > But my main point is: is the required delay always a linear > > > function of the voltage jump? Depending on the dampening and > > > load on the rail this could be an overshoot and settle, no? > > > > > > So I suggest to make that an array with 2 elements: a fixed part > > > and a time per voltage change. Does that make sense? > > > > Just to make it clear - then we do not follow Linux kernel DT bindings. > > The voltage change might have exponential characteristic and/or have > > additional fixed delay time (see patch 7 here). We could re-use some > > properties from Linux bindings for that purpose: > > https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/regulator/regulator.txt#L19 > > https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/regulator/regulator.txt#L24 > > I see. But then "static void regulator_set_value_delay(...)" should either > at least have a "ramp" somewhere in its name or it should discover the device > properties on its own, in order to be able to handle regulator-settling-time* > and regulator-enable-ramp-delay as well in the future. (i.e. pass it uc_pdata > instead of uc_pdata->ramp_delay and also let it handle the condition). Makes sense, so let me add the ramp keyword. I will also mention in comment that other delays are not yet handled. Thanks for feedback! Best regards, Krzysztof ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] arm: dts: exynos: Adjust whitespace around status property
Just add spaces around '=' sign for clarity. Signed-off-by: Krzysztof Kozlowski --- arch/arm/dts/exynos5422-odroidxu3.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/dts/exynos5422-odroidxu3.dts b/arch/arm/dts/exynos5422-odroidxu3.dts index e859dd1b981a..e0ad927dbf18 100644 --- a/arch/arm/dts/exynos5422-odroidxu3.dts +++ b/arch/arm/dts/exynos5422-odroidxu3.dts @@ -256,7 +256,7 @@ }; serial@12C2 { - status="okay"; + status = "okay"; }; mmc@1220 { -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [RFT] configs: arndale: Use appropriate driver for Asix AX88760
Arndale board has an Asix AX88760 USB 2.0 Hub and Fast Ethernet combo. The appropriate driver for it is USB_ETHER_ASIX. The mistake probably came from misinterpretation of commit e9954b867ce0 ("usb: eth: add ASIX AX88179 DRIVER") which was tested on RECS5250 COM module. This module indeed has Exynos5250 and some similarities with Arndale 5250 board but the USB/Ethernet chip used there is apparently different. Fixes: f58ad98a621c ("usb: net: migrate USB Ethernet adapters to Kconfig") Signed-off-by: Krzysztof Kozlowski --- Not tested. --- configs/arndale_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configs/arndale_defconfig b/configs/arndale_defconfig index 24422645cbac..b1f20b915889 100644 --- a/configs/arndale_defconfig +++ b/configs/arndale_defconfig @@ -41,4 +41,4 @@ CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_HOST_ETHER=y -CONFIG_USB_ETHER_ASIX88179=y +CONFIG_USB_ETHER_ASIX=y -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 2/4] configs: odroid_xu3: Use consistent syntax for #include
When including other header from configs, use consistent <> syntax. Signed-off-by: Krzysztof Kozlowski --- include/configs/odroid_xu3.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/configs/odroid_xu3.h b/include/configs/odroid_xu3.h index f178549a7223..7f4cff186151 100644 --- a/include/configs/odroid_xu3.h +++ b/include/configs/odroid_xu3.h @@ -7,7 +7,7 @@ #ifndef __CONFIG_ODROID_XU3_H #define __CONFIG_ODROID_XU3_H -#include "exynos5420-common.h" +#include #include #define CONFIG_BOARD_COMMON -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 3/4] configs: odroid_xu3: Unify indentation
File mixed space and tab indentation. Unify it. Signed-off-by: Krzysztof Kozlowski --- include/configs/odroid_xu3.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/configs/odroid_xu3.h b/include/configs/odroid_xu3.h index 7f4cff186151..5e765a2b2b02 100644 --- a/include/configs/odroid_xu3.h +++ b/include/configs/odroid_xu3.h @@ -18,7 +18,7 @@ #define TZPC_BASE_OFFSET 0x1 -#define SDRAM_BANK_SIZE(256UL << 20UL) /* 256 MB */ +#define SDRAM_BANK_SIZE(256UL << 20UL) /* 256 MB */ /* Reserve the last 22 MiB for the secure firmware */ #define CONFIG_SYS_MEM_TOP_HIDE(22UL << 20UL) #define CONFIG_TZSW_RESERVED_DRAM_SIZE CONFIG_SYS_MEM_TOP_HIDE @@ -28,7 +28,7 @@ #define CONFIG_ENV_SIZE(SZ_1K * 16) #define CONFIG_ENV_OFFSET (SZ_1K * 3136) /* ~3 MiB offset */ -#define CONFIG_SYS_INIT_SP_ADDR(CONFIG_SYS_LOAD_ADDR - 0x100) +#define CONFIG_SYS_INIT_SP_ADDR(CONFIG_SYS_LOAD_ADDR - 0x100) #define CONFIG_DEFAULT_CONSOLE "console=ttySAC2,115200n8\0" @@ -38,7 +38,7 @@ /* DFU */ #define CONFIG_SYS_DFU_DATA_BUF_SIZE SZ_32M #define DFU_DEFAULT_POLL_TIMEOUT 300 -#define DFU_MANIFEST_POLL_TIMEOUT 25000 +#define DFU_MANIFEST_POLL_TIMEOUT 25000 /* THOR */ #define CONFIG_G_DNL_THOR_VENDOR_NUM CONFIG_USB_GADGET_VENDOR_NUM @@ -85,11 +85,11 @@ #define CONFIG_SET_DFU_ALT_BUF_LEN (SZ_1K) /* Set soc_rev, soc_id, board_rev, boardname, fdtfile */ -#define CONFIG_ODROID_REV_AIN 9 +#define CONFIG_ODROID_REV_AIN 9 #define CONFIG_REVISION_TAG #undef CONFIG_SYS_BOARD -#define CONFIG_SYS_BOARD "odroid" +#define CONFIG_SYS_BOARD "odroid" /* Define new extra env settings, including DFU settings */ #undef CONFIG_EXTRA_ENV_SETTINGS -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot