diff mbox

[U-Boot,RFC] sunxi: support board-specific configuration options

Message ID 1440244364-11752-2-git-send-email-bernhard.nortmann@web.de
State Rejected
Delegated to: Hans de Goede
Headers show

Commit Message

Bernhard Nortmann Aug. 22, 2015, 11:52 a.m. UTC
Extend sunxi-common.h to include sunxi-boards.h - which in turn
can support multiple configurations/options/includes, based on
board-specific symbols (preprocessor definitions). These might
be supplied by the respective *_defconfig files.

Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
---

 configs/Bananapi_defconfig     |  2 +-
 include/configs/bananapi.h     | 23 +++++++++++++++++++++++
 include/configs/sunxi-boards.h |  5 +++++
 include/configs/sunxi-common.h |  1 +
 4 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 include/configs/bananapi.h
 create mode 100644 include/configs/sunxi-boards.h

Comments

Hans de Goede Aug. 22, 2015, 2:02 p.m. UTC | #1
Hi Bernhard,

On 22-08-15 13:52, Bernhard Nortmann wrote:
> Extend sunxi-common.h to include sunxi-boards.h - which in turn
> can support multiple configurations/options/includes, based on
> board-specific symbols (preprocessor definitions). These might
> be supplied by the respective *_defconfig files.
>
> Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
> ---
>
>   configs/Bananapi_defconfig     |  2 +-
>   include/configs/bananapi.h     | 23 +++++++++++++++++++++++
>   include/configs/sunxi-boards.h |  5 +++++
>   include/configs/sunxi-common.h |  1 +
>   4 files changed, 30 insertions(+), 1 deletion(-)
>   create mode 100644 include/configs/bananapi.h
>   create mode 100644 include/configs/sunxi-boards.h
>
> diff --git a/configs/Bananapi_defconfig b/configs/Bananapi_defconfig
> index 560295f..6bd0f3e 100644
> --- a/configs/Bananapi_defconfig
> +++ b/configs/Bananapi_defconfig
> @@ -7,7 +7,7 @@ CONFIG_VIDEO_COMPOSITE=y
>   CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-bananapi"
>   # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
>   CONFIG_SPL=y
> -CONFIG_SYS_EXTRA_OPTIONS="AXP209_POWER,SUNXI_GMAC,RGMII,MACPWR=SUNXI_GPH(23),AHCI"
> +CONFIG_SYS_EXTRA_OPTIONS="AXP209_POWER,SUNXI_GMAC,RGMII,MACPWR=SUNXI_GPH(23),AHCI,BOARD_BANANAPI"
>   # CONFIG_CMD_IMLS is not set
>   # CONFIG_CMD_FLASH is not set
>   # CONFIG_CMD_FPGA is not set

We want to move away from using CONFIG_SYS_EXTRA_OPTIONS, not start using
more of them.

The proper way to deal with this would be to allow defining one or more
LED gpio pins in Kconfig, like e.g. the USB?_VBUS_PIN config options
in board/sunxi/Kconfig, and then modify the generic code paths so that
these will be used when set.

This way we get led support for all boards in one go (once all the
defconfig-s are updated to set the gpio pins), and we do not end up
with board specific code.

Regards,

Hans



> diff --git a/include/configs/bananapi.h b/include/configs/bananapi.h
> new file mode 100644
> index 0000000..f14a6b4
> --- /dev/null
> +++ b/include/configs/bananapi.h
> @@ -0,0 +1,23 @@
> +/*
> + * bananapi.h
> + * board-specific options for Banana Pi
> + */
> +
> +/* Note that this example currently (v2015.10-rc2) requires
> + * patching U-Boot support of GPIO LEDs.
> + * see http://lists.denx.de/pipermail/u-boot/2015-August/224713.html
> + */
> +
> +#define CONFIG_STATUS_LED
> +#define CONFIG_BOARD_SPECIFIC_LED
> +#define CONFIG_GPIO_LED
> +#define CONFIG_GPIO_LED_STUBS
> +#define CONFIG_CMD_LED
> +
> +#define GREEN_LED_GPIO		248 /* = PH24 */
> +
> +#define STATUS_LED_BIT		GREEN_LED_GPIO
> +#define STATUS_LED_STATE	STATUS_LED_ON
> +#define STATUS_LED_PERIOD	(CONFIG_SYS_HZ / 2)
> +
> +#define STATUS_LED_GREEN	GREEN_LED_GPIO
> diff --git a/include/configs/sunxi-boards.h b/include/configs/sunxi-boards.h
> new file mode 100644
> index 0000000..b2b7c59
> --- /dev/null
> +++ b/include/configs/sunxi-boards.h
> @@ -0,0 +1,5 @@
> +/* support specific (sunxi) board configurations */
> +
> +#ifdef CONFIG_BOARD_BANANAPI
> +#include <configs/bananapi.h>
> +#endif
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index 1abf73c..424c2d4 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -40,6 +40,7 @@
>   #endif
>
>   #include <asm/arch/cpu.h>	/* get chip and board defs */
> +#include <configs/sunxi-boards.h>	/* board-specific config */
>
>   #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_DM_SERIAL)
>   # define CONFIG_DW_SERIAL
>
Bernhard Nortmann Aug. 24, 2015, 9:18 a.m. UTC | #2
Hi Hans!

I agree that picking user-defined LEDs as an example might not have been the
best choice. Stuff like that should probably go into more 'generic' 
frameworks,
e.g. a place to deal with those would be to define them in the device 
tree and
have a proper driver handling them.

Unfortunately I can't think of another prominent/convincing example for this
kind of tweaking... (maybe board-specific hardware quirks?)

IIRC, I started the whole thing while trying to incorporate some 'personal'
config changes (like support for the "led" command, a modified "bootcmd"
and netconsole activation). The difficulty I encountered with that was that
while the build system seems to accept certain options, it would happily
ignore other CONFIG_* settings from the *_defconfig (anything not in
Kconfig?), which led me to take the ".h include" route.

Many other boards seem to use a similar approach, often with quite
minimal .h files (e.g. include/configs/xilinx-ppc440.h or rpi.h)
I understand that this might be "old" U-Boot configuration style,
and thus deprecated - that's certainly a valid objection.

Regards, B. Nortmann


Am 22.08.2015 16:02, schrieb Hans de Goede:
>
> We want to move away from using CONFIG_SYS_EXTRA_OPTIONS, not start using
> more of them.
>
> The proper way to deal with this would be to allow defining one or more
> LED gpio pins in Kconfig, like e.g. the USB?_VBUS_PIN config options
> in board/sunxi/Kconfig, and then modify the generic code paths so that
> these will be used when set.
>
> This way we get led support for all boards in one go (once all the
> defconfig-s are updated to set the gpio pins), and we do not end up
> with board specific code.
>
> Regards,
>
> Hans
Hans de Goede Aug. 24, 2015, 9:22 a.m. UTC | #3
Hi,

On 24-08-15 11:18, Bernhard Nortmann wrote:
> Hi Hans!
>
> I agree that picking user-defined LEDs as an example might not have been the
> best choice. Stuff like that should probably go into more 'generic' frameworks,
> e.g. a place to deal with those would be to define them in the device tree and
> have a proper driver handling them.
>
> Unfortunately I can't think of another prominent/convincing example for this
> kind of tweaking... (maybe board-specific hardware quirks?)
>
> IIRC, I started the whole thing while trying to incorporate some 'personal'
> config changes (like support for the "led" command, a modified "bootcmd"
> and netconsole activation). The difficulty I encountered with that was that
> while the build system seems to accept certain options, it would happily
> ignore other CONFIG_* settings from the *_defconfig (anything not in
> Kconfig?), which led me to take the ".h include" route.
>
> Many other boards seem to use a similar approach, often with quite
> minimal .h files (e.g. include/configs/xilinx-ppc440.h or rpi.h)
> I understand that this might be "old" U-Boot configuration style,
> and thus deprecated - that's certainly a valid objection.

Right, introducing new CONFIG_FOO_BAR defines without having them defined
through a Kconfig file is not really something we want to do. In general
for any new option we will need a new Kconfig entry.

And you're also right that you can only set CONFIG_FOO_BAR defines through
the defconfig if they are defined in a Kconfig file.

Note please also ask yourself "do we really need a new option for this?"
before adding new options. e.g. enabling extra commands typically is something
which we likely will want to do for all sunxi boards, rather then just
one or two, since if it is useful on one board it is likely useful on all
the others too, and post SPL we are not really space constrained.

Regards,

Hans



>
> Regards, B. Nortmann
>
>
> Am 22.08.2015 16:02, schrieb Hans de Goede:
>>
>> We want to move away from using CONFIG_SYS_EXTRA_OPTIONS, not start using
>> more of them.
>>
>> The proper way to deal with this would be to allow defining one or more
>> LED gpio pins in Kconfig, like e.g. the USB?_VBUS_PIN config options
>> in board/sunxi/Kconfig, and then modify the generic code paths so that
>> these will be used when set.
>>
>> This way we get led support for all boards in one go (once all the
>> defconfig-s are updated to set the gpio pins), and we do not end up
>> with board specific code.
>>
>> Regards,
>>
>> Hans
>
diff mbox

Patch

diff --git a/configs/Bananapi_defconfig b/configs/Bananapi_defconfig
index 560295f..6bd0f3e 100644
--- a/configs/Bananapi_defconfig
+++ b/configs/Bananapi_defconfig
@@ -7,7 +7,7 @@  CONFIG_VIDEO_COMPOSITE=y
 CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-bananapi"
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
 CONFIG_SPL=y
-CONFIG_SYS_EXTRA_OPTIONS="AXP209_POWER,SUNXI_GMAC,RGMII,MACPWR=SUNXI_GPH(23),AHCI"
+CONFIG_SYS_EXTRA_OPTIONS="AXP209_POWER,SUNXI_GMAC,RGMII,MACPWR=SUNXI_GPH(23),AHCI,BOARD_BANANAPI"
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_FPGA is not set
diff --git a/include/configs/bananapi.h b/include/configs/bananapi.h
new file mode 100644
index 0000000..f14a6b4
--- /dev/null
+++ b/include/configs/bananapi.h
@@ -0,0 +1,23 @@ 
+/*
+ * bananapi.h
+ * board-specific options for Banana Pi
+ */
+
+/* Note that this example currently (v2015.10-rc2) requires
+ * patching U-Boot support of GPIO LEDs.
+ * see http://lists.denx.de/pipermail/u-boot/2015-August/224713.html
+ */
+
+#define CONFIG_STATUS_LED
+#define CONFIG_BOARD_SPECIFIC_LED
+#define CONFIG_GPIO_LED
+#define CONFIG_GPIO_LED_STUBS
+#define CONFIG_CMD_LED
+
+#define GREEN_LED_GPIO		248 /* = PH24 */
+
+#define STATUS_LED_BIT		GREEN_LED_GPIO
+#define STATUS_LED_STATE	STATUS_LED_ON
+#define STATUS_LED_PERIOD	(CONFIG_SYS_HZ / 2)
+
+#define STATUS_LED_GREEN	GREEN_LED_GPIO
diff --git a/include/configs/sunxi-boards.h b/include/configs/sunxi-boards.h
new file mode 100644
index 0000000..b2b7c59
--- /dev/null
+++ b/include/configs/sunxi-boards.h
@@ -0,0 +1,5 @@ 
+/* support specific (sunxi) board configurations */
+
+#ifdef CONFIG_BOARD_BANANAPI
+#include <configs/bananapi.h>
+#endif
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 1abf73c..424c2d4 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -40,6 +40,7 @@ 
 #endif
 
 #include <asm/arch/cpu.h>	/* get chip and board defs */
+#include <configs/sunxi-boards.h>	/* board-specific config */
 
 #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_DM_SERIAL)
 # define CONFIG_DW_SERIAL