Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Makefile.inc: Add CARRIER_DIR to component discovery
The configuration of the COMe module contains an additional carriers directory, which contains the source codes for configuration for the different carrierboards into which this module is plugged. Let's take this into account in the build system to use CARRIER_DIR to select the type of board in the local Makefile.inc, as it has done for VARIANT_DIR.
Used to build an image for the Kontron COMe-mAL10 module together with the Kontron Ref.Carrier-i T10 TNI.
Change-Id: Ic6b2f8994b1293ae6f5bda8c9cc95128ba0abf7a Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M Makefile.inc 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/42609/1
diff --git a/Makefile.inc b/Makefile.inc index 86335d9..0f9d635 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -34,6 +34,7 @@ # Basic component discovery MAINBOARDDIR=$(call strip_quotes,$(CONFIG_MAINBOARD_DIR)) VARIANT_DIR:=$(call strip_quotes,$(CONFIG_VARIANT_DIR)) +CARRIER_DIR:=$(call strip_quotes,$(CONFIG_CARRIER_DIR)) COREBOOT_EXPORTS += MAINBOARDDIR VARIANT_DIR
## Final build results, which CBFSTOOL uses to create the final
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42609
to look at the new patch set (#2).
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Makefile.inc: Add CARRIER_DIR to component discovery
The configuration of the COMe module contains an additional carriers directory, which contains the source codes for configuration for the different carrierboards into which this module is plugged. Let's take this into account in the build system to use CARRIER_DIR to select the board type in the local Makefile.inc, as it has done for VARIANT_DIR.
It is used for Kontron COMe-mAL10 module together with T10 TNI carrier board [1]
[1] https://review.coreboot.org/c/coreboot/+/39133
Change-Id: Ic6b2f8994b1293ae6f5bda8c9cc95128ba0abf7a Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M Makefile.inc 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/42609/2
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42609
to look at the new patch set (#3).
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Makefile.inc: Add CARRIER_DIR to component discovery
The configuration of the COMe module contains the additional directory- carriers, which contains the source codes for configuration for the different carrierboards into which this module is plugged. Let's take this into account in the build system to use CARRIER_DIR to select the board type in the local Makefile.inc, as it has done for VARIANT_DIR.
It is used for Kontron COMe-mAL10 module together with T10 TNI carrier board [1]
[1] https://review.coreboot.org/c/coreboot/+/39133
Change-Id: Ic6b2f8994b1293ae6f5bda8c9cc95128ba0abf7a Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M Makefile.inc 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/42609/3
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42609/6/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/42609/6/Makefile.inc@38 PS6, Line 38: COREBOOT_EXPORTS += MAINBOARDDIR VARIANT_DIR add CONFIG_CARRIER_DIR here?
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42609
to look at the new patch set (#7).
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Makefile.inc: Add CARRIER_DIR to component discovery
The configuration of the COMe module contains the additional directory- carriers, which contains the source codes for configuration for the different carrierboards into which this module is plugged. Let's take this into account in the build system to use CARRIER_DIR to select the board type in the local Makefile.inc, as it has done for VARIANT_DIR.
It is used for Kontron COMe-mAL10 module together with T10 TNI carrier board - https://review.coreboot.org/c/coreboot/+/39133.
Change-Id: Ic6b2f8994b1293ae6f5bda8c9cc95128ba0abf7a Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M Makefile.inc 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/42609/7
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42609
to look at the new patch set (#8).
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Makefile.inc: Add CARRIER_DIR to component discovery
The configuration of the COMe module contains the additional directory- carriers, which contains the source codes for configuration for the different carrierboards into which this module is plugged. Let's take this into account in the build system to use CARRIER_DIR to select the board type in the local Makefile.inc, as it has done for VARIANT_DIR.
It is used for Kontron COMe-mAL10 module together with T10 TNI carrier board - https://review.coreboot.org/c/coreboot/+/39133.
Change-Id: Ic6b2f8994b1293ae6f5bda8c9cc95128ba0abf7a Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M Makefile.inc 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/42609/8
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Patch Set 8:
(1 comment)
Thanks for review
https://review.coreboot.org/c/coreboot/+/42609/6/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/42609/6/Makefile.inc@38 PS6, Line 38: COREBOOT_EXPORTS += MAINBOARDDIR VARIANT_DIR
add CONFIG_CARRIER_DIR here?
Done
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42609
to look at the new patch set (#9).
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Makefile.inc: Add CARRIER_DIR to component discovery
The configuration of the COMe module contains the additional directory- carriers, which contains the source codes for configuration for the different carrierboards into which this module is plugged. Let's take this into account in the build system to use CARRIER_DIR to select the board type in the local Makefile.inc, as it has done for VARIANT_DIR.
It is used for Kontron COMe-mAL10 module together with T10 TNI carrier board (https://review.coreboot.org/c/coreboot/+/39133).
Change-Id: Ic6b2f8994b1293ae6f5bda8c9cc95128ba0abf7a Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M Makefile.inc 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/42609/9
Hello Lance Zhao, build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Angel Pons, Werner Zeh, Andrey Petrov,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42609
to look at the new patch set (#10).
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Makefile.inc: Add CARRIER_DIR to component discovery
The configuration of the COMe module contains the additional directory- carriers, which contains the source codes for configuration for the different carrierboards into which this module is plugged. Let's take this into account in the build system to use CARRIER_DIR to select the board type in the local Makefile.inc, as it has done for VARIANT_DIR.
TEST = Build ROM image for Kontron mAL10 COMe module together with T10 TNI carrier board (https://review.coreboot.org/c/coreboot/+/39133).
Change-Id: Ic6b2f8994b1293ae6f5bda8c9cc95128ba0abf7a Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M Makefile.inc 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/42609/10
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Patch Set 10:
What's the relationship between carrier boards and modules: can a carrier board be used for modules that differ more than what we could model in variants? If so, do we need a separate category for carriers?
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Patch Set 10:
What's the relationship between carrier boards and modules: can a carrier board be used for modules that differ more than what we could model in variants?
In the case of the Kontron module, variant refers to the COMe-mAL10 or COMe-m4AL10 module type. They differ in the type of memory (DDR3L or DDR4), and maybe they differ in some chips.
https://www.kontron.com/products/iot/iot-industry-4.0/iot-ready-boards-and-m...
COMe-mAL10 (E2): up to 8 GByte DDR3-1866 (-1600) memory down (ECC for E2-versions) (get params from spd eeprom)
COMe-m4AL10 (E2): up to 16 GByte LPDDR4-2400 (-2133) memory down (non-ECC) (set UPD params in romestage)
Moreover, each module variant can use one of the carrier boards. It could be boards from the Kontron:
https://www.kontron.com/products/boards-and-standard-form-factors/com-expres... https://www.kontron.com/products/boards-and-standard-form-factors/com-expres...
other manufacturers can add their own backplane too.
Thus, we have the following tree:
mainboard -+- kontron -+- mal10 --+ | +-- variants -+--- mal10 | | | +--- m4al10 | +-- carriers -+--- i-t10-tni | +--- eval-t10 | +--- vendor-new-backplane
And maybe we should add a new separate catalog for vendors:
+-- carriers -+- kontron -+- i-t10-tni | | | +- eval-t10 | +- secunet --- some-backplane-that-they-use-together-with-mal10 | +- vendor ---- new-backplane
If so, do we need a separate category for carriers?
I think at the moment there is no need to add carrierboards as a separate category. There are very few COMe modules and carrier boards.
But, in the future it could be very handy and perhaps we should move all carrierbоrd into a separate directory in src:
src --+-- | +-- carrierboard -+- kontron -+- i-t10-tni -> in Kconfig : CARRIER_TYPE10_COME | | | +- eval-t10 | +- aaeon ----- boxer26 -> in Kconfig : CARRIER_TYPE6_COME | +- secunet --- some-backplane-that-they-use-together-with-mal10 | +- vendor ---- new-backplane
CONFIG_CARRIER_TYPE10 is set in src/mainboard/.../come-module/Kconfig
Thus, for each type 10 come module, all carrier boards with CONFIG_CARRIER_TYPE10 will be available in menuconfig.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Patch Set 10: Code-Review+2
Thanks for the detailed explanation. One thing I'm wondering: You mention 2 modules that can use these carrier boards, that are variants of one another. Are these carrier boards (in principle) usable for _very_ different modules? That would indicate the need to move carriers into its own category at some point.
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Patch Set 11:
Thanks for the detailed explanation.
And thank you for the review!
One thing I'm wondering: You mention 2 modules that can use these carrier boards, that are variants of one another. Are these carrier boards (in principle) usable for _very_ different modules?
Excuse me, but what do you mean by _very_?
COMe modules and carriers depend on the form factor type [1,2] only. We cannot use COMe mal10 module (with type10 standard) together with boxer26 with connector for type6 form factor. This can only be used for carrier board with connector for type10 (Mini) [2].
Usually, even if the mal10 board had Sky[3]/Coffee/Cannon/... Lake (not sure, but I think AMD, VIA, and others too) instead of Apollo Lake, this can also be used for any carrier boards with type10 connector (for example i-t10-tni).
However, we should carefully read the notice (*) in the manufacturer's datasheet. For example, some interfaces may not be used in a connector. Or the carrier board has power limitations for the module and we should limit this during initialization. But this is the problem of people who will be adding the code for these boards :) They need to take these specifics into account when initializing the board.
The interfaces in the connectors (A, B, C, D) are standardized and depend on the type(1,2,3,4,5,6,7,10) of module (See Types in [1]).
[1] https://en.wikipedia.org/wiki/COM_Express [2] https://en.wikipedia.org/wiki/COM_Express#/media/File:COM_Express_form_facto... [3] http://linuxgizmos.com/com-express-type-10-mini-module-runs-linux-on-skylake...
====
And some interesting things: Recently, Kontron announced Computer-On-Module standard COM HPC. https://www.arcweb.com/blog/kontron-announced-computer-module-standard-com-h... https://www.kontron.com/landingpages/com-hpc
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Patch Set 14:
However, we should carefully read the notice (*) in the manufacturer's datasheet. For example, some interfaces may not be used in a connector. Or the carrier board has power limitations for the module and we should limit this during initialization. But this is the problem of people who will be adding the code for these boards :) They need to take these specifics into account when initializing the board.
I had the idea to re-use the devicetree alias mechanism to support carriers with different modules. The modules would declare aliases with the names from the COMe standard and the carriers would be able to pick them up and add devices below these interfaces. This would automatically catch incompatibilities because of the optional/alter- native interfaces COMe allows (sconfig wouldn't be able to link them).
I guess it would be most useful for evaluation carriers, but probably not much beyond. And as I never write complete support for them, I didn't put any effort into it yet.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Patch Set 17:
Patch Set 10:
What's the relationship between carrier boards and modules: can a carrier board be used for modules that differ more than what we could model in variants?
In the case of the Kontron module, variant refers to the COMe-mAL10 or COMe-m4AL10 module type. They differ in the type of memory (DDR3L or DDR4), and maybe they differ in some chips.
[…]
Can you add part of this explanation/answer, I just left the first paragraph, to the commit message? That’d be great as documentation.
Hello Lance Zhao, build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Angel Pons, Werner Zeh, Andrey Petrov,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42609
to look at the new patch set (#18).
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Makefile.inc: Add CARRIER_DIR to component discovery
The idea is to split the “mainboard” category into “variants” and “carrierboards”, in the case when we use the COMe module together with the Carrier Board instead of a single monolithic motherboard. Previously, the “variants” category defined the type of motherboard, which has a number of differences from the base one, for example, it differed in the size or type of memory, and in the configuration of the interfaces. Thus, there is no need to create a separate directory in src/mainboard for a board that is similar in configuration to the base board. But for a COMe module, “variants” contains different variants of only this module, and the entire Carrier Board configuration is allocated to a separate category - “carrierboards”, and each of the variants can be used with one of the many boards in “carrierboards”.
For example, in the case of the Kontron mAL10 COMe module, variant refers to the COMe-mAL10 or COMe-m4AL10 module type. They differ in the type of memory (DDR3L or DDR4), and maybe they differ in some chips (see more in https://www.kontron.com/products). However, all variants contain the same type of processor/SoC.
The "carrierboards" directory can be able contain both the Kontron's Evalution carrier boards (such as Eval Carrier2 T10 and COMe Ref.Carrier-i T10 TNI) and third party vendor backplanes that are compatible with the COMe modules from “variants”.
Thus, the src/mainboard/<module-name> directory contains the common configuration code for all variants from src/mainboard/<module-name>/ variants, which can be supplemented/redefined with a configuration from src/mainboard/<module-name>/carrierboard/<vendor-carrierboard-name>.
This architectural solution will be able to systematize and simplify understanding of the code structure for COMe modules and will allow vendors to add/maintain their code in a separate directory.
This work is also the first step towards to union of all carrierboards into the global category in src/carrierboard on a par with all boards from src/mainboard.
The patch takes this into account in the build system and adds CARRIER_DIR component to use the “carrierboards” category, as it has done for VARIANT_DIR.
TEST = Build ROM image for Kontron mAL10 COMe module together with T10 TNI carrier board (https://review.coreboot.org/c/coreboot/+/39133).
Change-Id: Ic6b2f8994b1293ae6f5bda8c9cc95128ba0abf7a Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M Makefile.inc 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/42609/18
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Patch Set 18:
Patch Set 17:
Patch Set 10:
What's the relationship between carrier boards and modules: can a carrier board be used for modules that differ more than what we could model in variants?
In the case of the Kontron module, variant refers to the COMe-mAL10 or COMe-m4AL10 module type. They differ in the type of memory (DDR3L or DDR4), and maybe they differ in some chips.
[…]
Can you add part of this explanation/answer, I just left the first paragraph, to the commit message? That’d be great as documentation.
I have added more details about my idea to the commit message, please review. Thanks
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Patch Set 20: Code-Review+2
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42609 )
Change subject: Makefile.inc: Add CARRIER_DIR to component discovery ......................................................................
Makefile.inc: Add CARRIER_DIR to component discovery
The idea is to split the “mainboard” category into “variants” and “carrierboards”, in the case when we use the COMe module together with the Carrier Board instead of a single monolithic motherboard. Previously, the “variants” category defined the type of motherboard, which has a number of differences from the base one, for example, it differed in the size or type of memory, and in the configuration of the interfaces. Thus, there is no need to create a separate directory in src/mainboard for a board that is similar in configuration to the base board. But for a COMe module, “variants” contains different variants of only this module, and the entire Carrier Board configuration is allocated to a separate category - “carrierboards”, and each of the variants can be used with one of the many boards in “carrierboards”.
For example, in the case of the Kontron mAL10 COMe module, variant refers to the COMe-mAL10 or COMe-m4AL10 module type. They differ in the type of memory (DDR3L or DDR4), and maybe they differ in some chips (see more in https://www.kontron.com/products). However, all variants contain the same type of processor/SoC.
The "carrierboards" directory can be able contain both the Kontron's Evalution carrier boards (such as Eval Carrier2 T10 and COMe Ref.Carrier-i T10 TNI) and third party vendor backplanes that are compatible with the COMe modules from “variants”.
Thus, the src/mainboard/<module-name> directory contains the common configuration code for all variants from src/mainboard/<module-name>/ variants, which can be supplemented/redefined with a configuration from src/mainboard/<module-name>/carrierboard/<vendor-carrierboard-name>.
This architectural solution will be able to systematize and simplify understanding of the code structure for COMe modules and will allow vendors to add/maintain their code in a separate directory.
This work is also the first step towards to union of all carrierboards into the global category in src/carrierboard on a par with all boards from src/mainboard.
The patch takes this into account in the build system and adds CARRIER_DIR component to use the “carrierboards” category, as it has done for VARIANT_DIR.
TEST = Build ROM image for Kontron mAL10 COMe module together with T10 TNI carrier board (https://review.coreboot.org/c/coreboot/+/39133).
Change-Id: Ic6b2f8994b1293ae6f5bda8c9cc95128ba0abf7a Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42609 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Patrick Georgi pgeorgi@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M Makefile.inc 1 file changed, 2 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/Makefile.inc b/Makefile.inc index e3afa89..9273961 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -35,7 +35,8 @@ # Basic component discovery MAINBOARDDIR=$(call strip_quotes,$(CONFIG_MAINBOARD_DIR)) VARIANT_DIR:=$(call strip_quotes,$(CONFIG_VARIANT_DIR)) -COREBOOT_EXPORTS += MAINBOARDDIR VARIANT_DIR +CARRIER_DIR:=$(call strip_quotes,$(CONFIG_CARRIER_DIR)) +COREBOOT_EXPORTS += MAINBOARDDIR VARIANT_DIR CARRIER_DIR
## Final build results, which CBFSTOOL uses to create the final ## rom image file, are placed under $(objcbfs).
-
Angel Pons (Code Review) -
Maxim Polyakov (Code Review)
-
Michael Niewöhner (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)