Skip to contentSkip to conversation

xtensa: support configurable processor configurations

PATCH New Chris Zankel 3 participants 9 messages Nov 10, 2012 – Nov 13, 2012

lore
Share & tools
lore
#0 lore raw

PATCH v3 2/2 xtensa: support configurable processor configurations

Xtensa is a configurable processor architecture, which allows to define additional instructions and registers. The required variant specific information for the toolchain is delivered in an 'overlay' file, which needs to be 'untarred' to the corresponding directories after the source is installed and patched.
This patch provides support for binutils, gcc, and gdb with a very limited changes to the build scripts. These additions are only executed for the Xtensa architecture and have no effect on other architectures.

arch/Config.in.xtensa new file +30 -0
new file mode 100644
index 0000000..61c0235
@@ -0,0 +1,30 @@
+choice
+	prompt "Target ABI"
+	depends on BR2_xtensa
+	default BR2_xtensa_fsf
+config BR2_xtensa_custom
+	bool "Custom Xtensa processor configuration"
+config BR2_xtensa_fsf
+	bool "fsf - Default configuration"
+endchoice
+
+config BR2_xtensa_custom_name
+	string "Custom Xtensa processor configuration anme"
+	depends on BR2_xtensa_custom
+	default ""
+	help
+	  Name given to a custom Xtensa processor configuration.
+
+config BR2_xtensa_core_name
+	string
+	default BR2_xtensa_custom_name	if BR2_xtensa_custom
+	default ""			if BR2_xtensa_fsf
+
+config BR2_xtensa_overlay_dir
+	string "Overlay directory for custom configuration"
+	depends on BR2_xtensa_custom
+	default ""
+	help
+	  Provide a directory path that contains the overlay files
+	  for the custom configuration. The path is based on the
+	  buildroot top directory.
package/binutils/binutils.mk +9 -0
index e1aac9b..0c69cb5 100644
@@ -66,5 +66,14 @@ define BINUTILS_INSTALL_TARGET_CMDS
 endef
 endif
 
+XTENSA_CORE_NAME = $(call qstrip, $(BR2_xtensa_core_name))
+ifneq ($(XTENSA_CORE_NAME),)
+define BINUTILS_XTENSA_PRE_PATCH
+	tar xf $(BR2_xtensa_overlay_dir)/xtensa_$(XTENSA_CORE_NAME).tar \
+		-C $(@D) bfd include ld
+endef
+HOST_BINUTILS_PRE_PATCH_HOOKS += BINUTILS_XTENSA_PRE_PATCH
+endif
+
 $(eval $(autotools-package))
 $(eval $(host-autotools-package))
toolchain/gcc/gcc-uclibc-4.x.mk +4 -0
index 1a075cc..1710070 100644
@@ -251,6 +251,10 @@ $(GCC_DIR)/.unpacked: $(DL_DIR)/$(GCC_SOURCE)
 	rm -rf $(GCC_DIR)
 	$(GCC_CAT) $(DL_DIR)/$(GCC_SOURCE) | tar -C $(TOOLCHAIN_DIR) $(TAR_OPTIONS) -
 	$(call CONFIG_UPDATE,$(@D))
+ifneq ($(call qstrip, $(BR2_xtensa_core_name)),)
+	tar xf $(BR2_xtensa_overlay_dir)/xtensa_$(call qstrip,\
+		$(BR2_xtensa_core_name)).tar -C $(@D) include
+endif
 	touch $@
 
 gcc-patched: $(GCC_DIR)/.patched
toolchain/gdb/gdb.mk +4 -0
index 6b029a8..dc305a6 100644
@@ -25,6 +25,10 @@ gdb-unpacked: $(GDB_DIR)/.unpacked
 $(GDB_DIR)/.unpacked: $(DL_DIR)/$(GDB_SOURCE)
 	mkdir -p $(GDB_DIR)
 	$(GDB_CAT) $(DL_DIR)/$(GDB_SOURCE) | tar -C $(GDB_DIR) $(TAR_STRIP_COMPONENTS)=1 $(TAR_OPTIONS) -
+ifneq ($(call qstrip, $(BR2_xtensa_core_name)),)
+	tar xf $(BR2_xtensa_overlay_dir)/xtensa_$(call qstrip, \
+		$(BR2_xtensa_core_name)).tar -C $(@D) bfd include gdb
+endif
 ifneq ($(wildcard $(GDB_PATCH_DIR)),)
 	support/scripts/apply-patches.sh $(GDB_DIR) $(GDB_PATCH_DIR) \*.patch
 endif
#1 lore raw

PATCH v3 2/2 xtensa: support configurable processor configurations

Quoted 17 lines · Chris Zankel

Xtensa is a configurable processor architecture, which allows to define additional instructions and registers. The required variant specific information for the toolchain is delivered in an 'overlay' file, which needs to be 'untarred' to the corresponding directories after the source is installed and patched.
This patch provides support for binutils, gcc, and gdb with a very limited changes to the build scripts. These additions are only executed for the Xtensa architecture and have no effect on other architectures.

arch/Config.in.xtensa new file +0 -0
new file mode 100644
index 0000000..61c0235

arch/Config.in.xtensa line 9

Quoted 9 lines
arch/Config.in.xtensa new file +9 -0
@@ -0,0 +1,30 @@
+choice
+	prompt "Target ABI"
+	depends on BR2_xtensa
+	default BR2_xtensa_fsf
+config BR2_xtensa_custom
+	bool "Custom Xtensa processor configuration"
+config BR2_xtensa_fsf
+	bool "fsf - Default configuration"
+endchoice

Put empty lines between the different config options.

All config names should be in capitals - the only exception is the
arch name itself (i.e. BR2_xtensa, but BR2_XTENSA_FSF).

arch/Config.in.xtensa line 12

Quoted 3 lines
arch/Config.in.xtensa new file +3 -0
+
+config BR2_xtensa_custom_name
+	string "Custom Xtensa processor configuration anme"

anme -> name

arch/Config.in.xtensa line 21

Quoted 9 lines
arch/Config.in.xtensa new file +9 -0
+	depends on BR2_xtensa_custom
+	default ""
+	help
+	  Name given to a custom Xtensa processor configuration.
+
+config BR2_xtensa_core_name
+	string
+	default BR2_xtensa_custom_name	if BR2_xtensa_custom
+	default ""			if BR2_xtensa_fsf

Minor detail: missing "depends on BR2_xtensa".

Perhaps it's better to move the condition to a
if BR2_xtensa
...
endif
around the entire file. Thomas has a patch lined up to move all the conditions outside of the Config.in.*

arch/Config.in.xtensa line 30

Quoted 9 lines
arch/Config.in.xtensa new file +9 -0
+
+config BR2_xtensa_overlay_dir
+	string "Overlay directory for custom configuration"
+	depends on BR2_xtensa_custom
+	default ""
+	help
+	  Provide a directory path that contains the overlay files
+	  for the custom configuration. The path is based on the
+	  buildroot top directory.

based on -> relative to

I don't find this explanation sufficient. First of all, it should
mention that that directory should contain tar files. Second, it would help if you could explain where the tar files are supposed to come from (I guess they're generated by some Xtensa tool).

Regards,
Arnout

[snip]

Signature · 6 lines
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#2 lore raw

PATCH v3 2/2 xtensa: support configurable processor configurations

Hi Arnout,

Quoted 7 lines · Arnout Vandecappelle

arch/Config.in.xtensa line 9

Quoted 5 lines · Chris Zankel
arch/Config.in.xtensa new file +5 -0
+config BR2_xtensa_custom
+	bool "Custom Xtensa processor configuration"
+config BR2_xtensa_fsf
+	bool "fsf - Default configuration"
+endchoice

Put empty lines between the different config options.

Done.

Quoted 2 lines

All config names should be in capitals - the only exception is the
arch name itself (i.e. BR2_xtensa, but BR2_XTENSA_FSF).

To give you some background, 'fsf' describes the default configuration used in the various toolchain packages (gcc, buildroot, etc.). The idea is to append the name of the processor configuration (for example, dc232b), so to follow other architectures, I think it would be best to keep lower-case for processor names, and upper case for other options.

For the case above, it would look like this:

config BR_XTENSA_CUSTOM
...

config BR_xtensa_fsf
...

config BR_xtensa_dc232b
...

etc.

I actually think that this makes a nice distinction between processor names and buildroot configuration options.

Quoted 4 lines

arch/Config.in.xtensa line 12

Quoted 3 lines
arch/Config.in.xtensa new file +3 -0
+
+config BR2_xtensa_custom_name
+	string "Custom Xtensa processor configuration anme"

anme -> name

Done

Quoted 11 lines

arch/Config.in.xtensa line 21

Quoted 10 lines
arch/Config.in.xtensa new file +9 -0
+	depends on BR2_xtensa_custom
+	default ""
+	help
+	  Name given to a custom Xtensa processor configuration.
+
+config BR2_xtensa_core_name
+	string
+	default BR2_xtensa_custom_name	if BR2_xtensa_custom
+	default ""			if BR2_xtensa_fsf

Minor detail: missing "depends on BR2_xtensa".

Done.

Quoted 6 lines

Perhaps it's better to move the condition to a
if BR2_xtensa
...
endif
around the entire file. Thomas has a patch lined up to move all the conditions outside of the Config.in.*

It seems Xtensa would be the only one that uses that scheme, so maybe wait for the generic patch from Thomas?

Quoted 11 lines

arch/Config.in.xtensa line 30

Quoted 9 lines
arch/Config.in.xtensa new file +9 -0
+
+config BR2_xtensa_overlay_dir
+	string "Overlay directory for custom configuration"
+	depends on BR2_xtensa_custom
+	default ""
+	help
+	  Provide a directory path that contains the overlay files
+	  for the custom configuration. The path is based on the
+	  buildroot top directory.

based on -> relative to

Quoted 4 lines

I don't find this explanation sufficient. First of all, it should
mention that that directory should contain tar files. Second, it would help if you could explain where the tar files are supposed to come from (I guess they're generated by some Xtensa tool).

Made some additions, and will send a new set of patches in a bit...

-Chris

#3 lore raw

PATCH v3 2/2 xtensa: support configurable processor configurations

Quoted 27 lines · Chris Zankel

Hi Arnout,

Quoted 7 lines · Arnout Vandecappelle

arch/Config.in.xtensa line 9

Quoted 5 lines · Chris Zankel
arch/Config.in.xtensa new file +5 -0
+config BR2_xtensa_custom
+	bool "Custom Xtensa processor configuration"
+config BR2_xtensa_fsf
+	bool "fsf - Default configuration"
+endchoice

Put empty lines between the different config options.

Done.

Quoted 2 lines

All config names should be in capitals - the only exception is the
arch name itself (i.e. BR2_xtensa, but BR2_XTENSA_FSF).

To give you some background, 'fsf' describes the default configuration used in the various toolchain packages (gcc, buildroot, etc.). The idea is to append the name of the processor configuration (for example, dc232b), so to follow other architectures, I think it would be best to keep lower-case for processor names, and upper case for other options.

For the case above, it would look like this:

config BR_XTENSA_CUSTOM
...

config BR_xtensa_fsf
...

Quoted 7 lines

config BR_xtensa_dc232b
...

etc.

I actually think that this makes a nice distinction between processor names and buildroot configuration options.

ACK good point.

Quoted 19 lines
Quoted 6 lines

arch/Config.in.xtensa line 12

Quoted 5 lines
arch/Config.in.xtensa new file +3 -0
+
+config BR2_xtensa_custom_name
+	string "Custom Xtensa processor configuration anme"

anme -> name

Done

Quoted 11 lines

arch/Config.in.xtensa line 21

Quoted 10 lines
arch/Config.in.xtensa new file +9 -0
+	depends on BR2_xtensa_custom
+	default ""
+	help
+	  Name given to a custom Xtensa processor configuration.
+
+config BR2_xtensa_core_name
+	string
+	default BR2_xtensa_custom_name	if BR2_xtensa_custom
+	default ""			if BR2_xtensa_fsf

Minor detail: missing "depends on BR2_xtensa".

Done.

Quoted 8 lines
Quoted 6 lines

Perhaps it's better to move the condition to a
if BR2_xtensa
...
endif
around the entire file. Thomas has a patch lined up to move all the conditions outside of the Config.in.*

It seems Xtensa would be the only one that uses that scheme, so maybe wait for the generic patch from Thomas?

I don't think so. Then it means more work for Thomas to refresh his
patch. There will be a merge conflict anyway between the two, but with the condition outside the file the conflict will be small.

That said, Peter hasn't merged Thomas's patch even though I acked
it, so maybe he has some fundamental objection to the principle. Or maybe it's just because I didn't test it.

Bottom line: this issue won't stop my ack.

[snip]

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#4 lore raw

PATCH v3 2/2 xtensa: support configurable processor configurations

Quoted 9 lines · Arnout Vandecappelle

Perhaps it's better to move the condition to a
if BR2_xtensa
...
endif
around the entire file. Thomas has a patch lined up to move all the conditions outside of the Config.in.*

Quoted 2 lines

It seems Xtensa would be the only one that uses that scheme, so maybe wait for the generic patch from Thomas?

Quoted 7 lines

I don't think so. Then it means more work for Thomas to refresh his
patch. There will be a merge conflict anyway between the two, but with the condition outside the file the conflict will be small.

That said, Peter hasn't merged Thomas's patch even though I acked
it, so maybe he has some fundamental objection to the principle. Or maybe it's just because I didn't test it.

I guess I don't understand what that patch does, i.e. where is the 'if...endif'? Could you maybe point to the patch?

Thanks,
-Chris

#5 lore raw

PATCH v3 2/2 xtensa: support configurable processor configurations

Quoted 14 lines · Chris Zankel
Quoted 13 lines · Arnout Vandecappelle

Perhaps it's better to move the condition to a
if BR2_xtensa
...
endif
around the entire file. Thomas has a patch lined up to move all the conditions outside of the Config.in.*

Quoted 2 lines

It seems Xtensa would be the only one that uses that scheme, so maybe wait for the generic patch from Thomas?

I don't think so. Then it means more work for Thomas to refresh his
patch. There will be a merge conflict anyway between the two, but with the condition outside the file the conflict will be small.

Quoted 6 lines
Quoted 3 lines

That said, Peter hasn't merged Thomas's patch even though I acked
it, so maybe he has some fundamental objection to the principle. Or maybe it's just because I didn't test it.

I guess I don't understand what that patch does, i.e. where is the 'if...endif'? Could you maybe point to the patch?

I'm talking about http://patchwork.ozlabs.org/patch/196916/
In arch/Config.in, it introduces the following construct:

if BR2_xtensa
source "arch/Config.in.xtensa"
endif

This removes the need for adding 'depend on BR2_xtensa' in every config
option in that file.

It's true that _now_ xtensa would be the only one using that scheme, but the
idea is that if you do that for xtensa now, it will be easier to merge in Thomas's patch later (or vice versa, it will be easier to rebase your patch once Thomas's patch gets merged).

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#6 lore raw

PATCH v3 2/2 xtensa: support configurable processor configurations

Hi Arnout,

Sorry, I got your email after I sent out version 4 of the patch, which doesn't include the change you mentioned (somehow, fetchmail was stuck :-( So, please let me know if you want me to send out another version that includes that change. I don't want to confuse Thomas with all those versions...

Thanks,
-Chris

Quoted 16 lines · Arnout Vandecappelle
Quoted 2 lines

I guess I don't understand what that patch does, i.e. where is the 'if...endif'? Could you maybe point to the patch?

I'm talking about http://patchwork.ozlabs.org/patch/196916/
In arch/Config.in, it introduces the following construct:

if BR2_xtensa
source "arch/Config.in.xtensa"
endif

This removes the need for adding 'depend on BR2_xtensa' in every config
option in that file.

It's true that _now_ xtensa would be the only one using that scheme, but the
idea is that if you do that for xtensa now, it will be easier to merge in Thomas's patch later (or vice versa, it will be easier to rebase your patch once Thomas's patch gets merged).

#7 lore raw

PATCH v3 2/2 xtensa: support configurable processor configurations

Chris,

Quoted 5 lines · Chris Zankel

Sorry, I got your email after I sent out version 4 of the patch, which doesn't include the change you mentioned (somehow, fetchmail was stuck :-( So, please let me know if you want me to send out another version that includes that change. I don't want to confuse Thomas with all those versions...

You can keep your patch as is wrt to Config.in. What I propose to do is that I merge your Xtensa support in one of my branch together with my reorganization of the architecture definitions and I send a pull-request for the whole thing to Peter.

Would that be OK for you?

Thanks,

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#8 lore raw

PATCH v3 2/2 xtensa: support configurable processor configurations

Hi Thomas,

Quoted 9 lines · Thomas Petazzoni

Chris,

Quoted 5 lines · Chris Zankel

Sorry, I got your email after I sent out version 4 of the patch, which doesn't include the change you mentioned (somehow, fetchmail was stuck :-( So, please let me know if you want me to send out another version that includes that change. I don't want to confuse Thomas with all those versions...

Quoted 6 lines

You can keep your patch as is wrt to Config.in. What I propose to do is that I merge your Xtensa support in one of my branch together with my reorganization of the architecture definitions and I send a pull-request for the whole thing to Peter.

Would that be OK for you?

That would be great! Could you please use v4 of the two patches as they include changes suggested by Arnout.

Thanks,
-Chris