[U-Boot] [PATCH 2 00/22] DM: Cmd: GPIO/LED/STM32/CLK: provide command-line support for device-tree configured gpios and leds

[U-Boot] [PATCH 2 00/22] DM: Cmd: GPIO/LED/STM32/CLK: provide command-line support for device-tree configured gpios and leds

[Benjamin Tietz]

This series begins to provide device-tree support on stm32 devices,
starting with a the stack of a simple clock-driver (at least
enabling/disabling peripheral clocks), the gpio driver and leds.

As the current led command-line interface isn't aware of any device-tree
configured led, the command gets rewritten and extended for device-tree LEDs
on the way. These changes are architecture indipendent.

To accomplish these changes the led-uclass driver had to be extended, too.

Changes in v2:
* more verbose commit messages
* correct clock calculation in stm32_clk
* minor adjustments


---

Benjamin Tietz (22):
      stm32: gpio: fix otype access
      stm32: gpio_direction_output: make sure, output is set to push-pull
      stm32: gpio_get_value: always return 0 or 1
      stm32f429-discovery: config: enable status leds
      Cmd: led: provide a selector in kconfig
      DTS: stm32f429: provide device-tree files (from linux kernel)
      clock-uclass: allow disabling a peripheral clock
      STM32: clock: provide dts-accessible clock driver
      DTS: STM32f429: add gpio-banks
      STM32: gpio: group SOC-specific code to one ifdef/elif construct
      GPIO: STM32: make DTS-aware
      STM32F429-discovery: led: disable board-specific code, if DM is selected
      GPIO/LED: make more robust, if STATUS_LED isn't selected
      Cmd: LED: rewrite to prepare non-static access
      DTS: STM32F429-disco: add board leds and enable rcc
      LED: add function to retrieve a device's label
      LED: provide function to count and get all (DM-)LEDs
      cmd: LED: be aware of DTS-configured leds
      LED: provide functionality to get led status
      LED: GPIO: provide get_on() op
      LED: provide toggling interface
      Cmd: LED: make DM-leds toggle


 arch/arm/dts/Makefile                 |    2 
 arch/arm/dts/armv7-m.dtsi             |   24 ++
 arch/arm/dts/stm32429i-eval.dts       |   75 ++++++
 arch/arm/dts/stm32f429-disco.dts      |   97 ++++++++
 arch/arm/dts/stm32f429.dtsi           |  282 +++++++++++++++++++++++
 board/st/stm32f429-discovery/Makefile |    3 
 cmd/Kconfig                           |    4 
 cmd/led.c                             |  401 ++++++++++++++++++++++++---------
 drivers/clk/Kconfig                   |    4 
 drivers/clk/Makefile                  |    1 
 drivers/clk/clk-uclass.c              |   10 +
 drivers/clk/clk_stm32.c               |  112 +++++++++
 drivers/gpio/stm32_gpio.c             |  202 ++++++++++++++---
 drivers/led/led-uclass.c              |   83 +++++++
 drivers/led/led_gpio.c                |   11 +
 drivers/misc/gpio_led.c               |    4 
 drivers/misc/status_led.c             |    2 
 include/clk.h                         |   18 +
 include/configs/stm32f429-discovery.h |   14 +
 include/led.h                         |   65 +++++
 include/status_led.h                  |    4 
 21 files changed, 1277 insertions(+), 141 deletions(-)
 create mode 100644 arch/arm/dts/armv7-m.dtsi
 create mode 100644 arch/arm/dts/stm32429i-eval.dts
 create mode 100644 arch/arm/dts/stm32f429-disco.dts
 create mode 100644 arch/arm/dts/stm32f429.dtsi
 create mode 100644 drivers/clk/clk_stm32.c

--

best regards
Benjamin Tietz

[U-Boot] [PATCH v2 01/22] stm32: gpio: fix otype access

[Benjamin Tietz]

Since the GPIOx_OTYPER register has only one bit for every pin, the width and shift has to be corrected.
---
 drivers/gpio/stm32_gpio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/stm32_gpio.c b/drivers/gpio/stm32_gpio.c
index 50f86d3..2457211 100644
--- a/drivers/gpio/stm32_gpio.c
+++ b/drivers/gpio/stm32_gpio.c
@@ -66,7 +66,7 @@ int stm32_gpio_config(const struct stm32_gpio_dsc *dsc,
 	i = dsc->pin * 2;
 
 	clrsetbits_le32(&gpio_regs->moder, 0x3 << i, ctl->mode << i);
-	clrsetbits_le32(&gpio_regs->otyper, 0x3 << i, ctl->otype << i);
+	clrsetbits_le32(&gpio_regs->otyper, 0x1 << dsc->pin, ctl->otype << dsc->pin);
 	clrsetbits_le32(&gpio_regs->ospeedr, 0x3 << i, ctl->speed << i);
 	clrsetbits_le32(&gpio_regs->pupdr, 0x3 << i, ctl->pupd << i);
 

[U-Boot] [PATCH v2 02/22] stm32: gpio_direction_output: make sure, output is set to push-pull

[Benjamin Tietz]

Previously the bit for settting the pin-behaviour was left undefined, which may result in an open-drain behaviour for the affected pin.
Now it is correctly initialized, always.
---
 drivers/gpio/stm32_gpio.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/stm32_gpio.c b/drivers/gpio/stm32_gpio.c
index 2457211..d092c8f 100644
--- a/drivers/gpio/stm32_gpio.c
+++ b/drivers/gpio/stm32_gpio.c
@@ -236,6 +236,7 @@ int gpio_direction_output(unsigned gpio, int value)
 #if defined(CONFIG_STM32F4) || defined(CONFIG_STM32F7)
 	ctl.af = STM32_GPIO_AF0;
 	ctl.mode = STM32_GPIO_MODE_OUT;
+	ctl.otype = STM32_GPIO_OTYPE_PP;
 	ctl.pupd = STM32_GPIO_PUPD_NO;
 	ctl.speed = STM32_GPIO_SPEED_50M;
 #elif defined(CONFIG_STM32F1)

[U-Boot] [PATCH v2 03/22] stm32: gpio_get_value: always return 0 or 1

[Benjamin Tietz]

Some functions using the GPIO-interface depend on gpio_get_value to
return 0 or 1, while the stm32 driver returned 0 or "something not zero".
This patch corrects this behaviour.
---
 drivers/gpio/stm32_gpio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/stm32_gpio.c b/drivers/gpio/stm32_gpio.c
index d092c8f..516dfcc 100644
--- a/drivers/gpio/stm32_gpio.c
+++ b/drivers/gpio/stm32_gpio.c
@@ -263,7 +263,7 @@ int gpio_get_value(unsigned gpio)
 	dsc.port = stm32_gpio_to_port(gpio);
 	dsc.pin = stm32_gpio_to_pin(gpio);
 
-	return stm32_gpin_get(&dsc);
+	return !!stm32_gpin_get(&dsc);
 }
 
 int gpio_set_value(unsigned gpio, int value)

[U-Boot] [PATCH v2 04/22] stm32f429-discovery: config: enable status leds

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

This enables and adds configuration for status-leds on the
STM32F429 discovery board.
---
 include/configs/stm32f429-discovery.h |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/configs/stm32f429-discovery.h b/include/configs/stm32f429-discovery.h
index 8bbe580..9d275c0 100644
--- a/include/configs/stm32f429-discovery.h
+++ b/include/configs/stm32f429-discovery.h
@@ -45,6 +45,14 @@
 #define CONFIG_BOARD_SPECIFIC_LED
 #define CONFIG_RED_LED			110
 #define CONFIG_GREEN_LED		109
+#define CONFIG_GPIO_LED		1
+#define CONFIG_STATUS_LED
+#define STATUS_LED_BIT		CONFIG_RED_LED
+#define STATUS_LED_STATE	0
+#define STATUS_LED_PERIOD	0
+#define STATUS_LED_BIT1		CONFIG_GREEN_LED
+#define STATUS_LED_STATE1	0
+#define STATUS_LED_PERIOD1	0
 
 #define CONFIG_STM32_GPIO
 #define CONFIG_STM32_FLASH

[U-Boot] [PATCH v2 05/22] Cmd: led: provide a selector in kconfig

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

This patch creates an entry for selecting the led command from
kconfig configuration.
---
 cmd/Kconfig |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index d51645c..f49ff47 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -421,6 +421,10 @@ config CMD_GPIO
 	help
 	  GPIO support.
 
+config CMD_LED
+	bool "led"
+	help
+	  support for accessing and toggeling LEDs
 endmenu
 
 

[U-Boot] [PATCH v2 06/22] DTS: stm32f429: provide device-tree files (from linux kernel)

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

This patch adds the DTS configuration files needed for the
STM32F429-Discovery board, as they can be found in a current
Linux Kernel.
---
 arch/arm/dts/Makefile            |    2 
 arch/arm/dts/armv7-m.dtsi        |   24 +++++
 arch/arm/dts/stm32429i-eval.dts  |   75 +++++++++++++++
 arch/arm/dts/stm32f429-disco.dts |   75 +++++++++++++++
 arch/arm/dts/stm32f429.dtsi      |  190 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 366 insertions(+)
 create mode 100644 arch/arm/dts/armv7-m.dtsi
 create mode 100644 arch/arm/dts/stm32429i-eval.dts
 create mode 100644 arch/arm/dts/stm32f429-disco.dts
 create mode 100644 arch/arm/dts/stm32f429.dtsi

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index a827613..ff5fe03 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -103,6 +103,8 @@ dtb-$(CONFIG_AM43XX) += am437x-gp-evm.dtb am437x-sk-evm.dtb	\
 	am437x-idk-evm.dtb
 dtb-$(CONFIG_THUNDERX) += thunderx-88xx.dtb
 
+dtb-$(CONFIG_STM32F4) += stm32f429-disco.dtb
+
 dtb-$(CONFIG_ARCH_SOCFPGA) +=				\
 	socfpga_arria5_socdk.dtb			\
 	socfpga_cyclone5_mcvevk.dtb			\
diff --git a/arch/arm/dts/armv7-m.dtsi b/arch/arm/dts/armv7-m.dtsi
new file mode 100644
index 0000000..b1ad7cf
--- /dev/null
+++ b/arch/arm/dts/armv7-m.dtsi
@@ -0,0 +1,24 @@
+#include "skeleton.dtsi"
+
+/ {
+	nvic: nv-interrupt-controller  {
+		compatible = "arm,armv7m-nvic";
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		reg = <0xe000e100 0xc00>;
+	};
+
+	systick: timer at e000e010 {
+		compatible = "arm,armv7m-systick";
+		reg = <0xe000e010 0x10>;
+		status = "disabled";
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		interrupt-parent = <&nvic>;
+		ranges;
+	};
+};
diff --git a/arch/arm/dts/stm32429i-eval.dts b/arch/arm/dts/stm32429i-eval.dts
new file mode 100644
index 0000000..6964fc9
--- /dev/null
+++ b/arch/arm/dts/stm32429i-eval.dts
@@ -0,0 +1,75 @@
+/*
+ * Copyright 2015 - Maxime Coquelin <mcoquelin.stm32@gmail.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ *     You should have received a copy of the GNU General Public
+ *     License along with this file; if not, write to the Free
+ *     Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ *     MA 02110-1301 USA
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f429.dtsi"
+
+/ {
+	model = "STMicroelectronics STM32429i-EVAL board";
+	compatible = "st,stm32429i-eval", "st,stm32f429";
+
+	chosen {
+		bootargs = "root=/dev/ram rdinit=/linuxrc";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		reg = <0xc0000000 0x2000000>;
+	};
+
+	aliases {
+		serial0 = &usart1;
+	};
+};
+
+&clk_hse {
+	clock-frequency = <25000000>;
+};
+
+&usart1 {
+	status = "okay";
+};
diff --git a/arch/arm/dts/stm32f429-disco.dts b/arch/arm/dts/stm32f429-disco.dts
new file mode 100644
index 0000000..2bae81c
--- /dev/null
+++ b/arch/arm/dts/stm32f429-disco.dts
@@ -0,0 +1,75 @@
+/*
+ * Copyright 2015 - Maxime Coquelin <mcoquelin.stm32@gmail.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ *     You should have received a copy of the GNU General Public
+ *     License along with this file; if not, write to the Free
+ *     Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ *     MA 02110-1301 USA
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f429.dtsi"
+
+/ {
+	model = "STMicroelectronics STM32F429i-DISCO board";
+	compatible = "st,stm32f429i-disco", "st,stm32f429";
+
+	chosen {
+		bootargs = "root=/dev/ram";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		reg = <0x90000000 0x800000>;
+	};
+
+	aliases {
+		serial0 = &usart1;
+	};
+};
+
+&clk_hse {
+	clock-frequency = <8000000>;
+};
+
+&usart1 {
+	status = "okay";
+};
diff --git a/arch/arm/dts/stm32f429.dtsi b/arch/arm/dts/stm32f429.dtsi
new file mode 100644
index 0000000..5e1e234
--- /dev/null
+++ b/arch/arm/dts/stm32f429.dtsi
@@ -0,0 +1,190 @@
+/*
+ * Copyright 2015 - Maxime Coquelin <mcoquelin.stm32@gmail.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ *     You should have received a copy of the GNU General Public
+ *     License along with this file; if not, write to the Free
+ *     Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ *     MA 02110-1301 USA
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include "armv7-m.dtsi"
+
+/ {
+	clocks {
+		clk_hse: clk-hse {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <0>;
+		};
+	};
+
+	soc {
+		timer2: timer at 40000000 {
+			compatible = "st,stm32-timer";
+			reg = <0x40000000 0x400>;
+			interrupts = <28>;
+			clocks = <&rcc 0 128>;
+			status = "disabled";
+		};
+
+		timer3: timer at 40000400 {
+			compatible = "st,stm32-timer";
+			reg = <0x40000400 0x400>;
+			interrupts = <29>;
+			clocks = <&rcc 0 129>;
+			status = "disabled";
+		};
+
+		timer4: timer at 40000800 {
+			compatible = "st,stm32-timer";
+			reg = <0x40000800 0x400>;
+			interrupts = <30>;
+			clocks = <&rcc 0 130>;
+			status = "disabled";
+		};
+
+		timer5: timer at 40000c00 {
+			compatible = "st,stm32-timer";
+			reg = <0x40000c00 0x400>;
+			interrupts = <50>;
+			clocks = <&rcc 0 131>;
+		};
+
+		timer6: timer at 40001000 {
+			compatible = "st,stm32-timer";
+			reg = <0x40001000 0x400>;
+			interrupts = <54>;
+			clocks = <&rcc 0 132>;
+			status = "disabled";
+		};
+
+		timer7: timer at 40001400 {
+			compatible = "st,stm32-timer";
+			reg = <0x40001400 0x400>;
+			interrupts = <55>;
+			clocks = <&rcc 0 133>;
+			status = "disabled";
+		};
+
+		usart2: serial at 40004400 {
+			compatible = "st,stm32-usart", "st,stm32-uart";
+			reg = <0x40004400 0x400>;
+			interrupts = <38>;
+			clocks =  <&rcc 0 145>;
+			status = "disabled";
+		};
+
+		usart3: serial at 40004800 {
+			compatible = "st,stm32-usart", "st,stm32-uart";
+			reg = <0x40004800 0x400>;
+			interrupts = <39>;
+			clocks = <&rcc 0 146>;
+			status = "disabled";
+		};
+
+		usart4: serial at 40004c00 {
+			compatible = "st,stm32-uart";
+			reg = <0x40004c00 0x400>;
+			interrupts = <52>;
+			clocks = <&rcc 0 147>;
+			status = "disabled";
+		};
+
+		usart5: serial at 40005000 {
+			compatible = "st,stm32-uart";
+			reg = <0x40005000 0x400>;
+			interrupts = <53>;
+			clocks = <&rcc 0 148>;
+			status = "disabled";
+		};
+
+		usart7: serial at 40007800 {
+			compatible = "st,stm32-usart", "st,stm32-uart";
+			reg = <0x40007800 0x400>;
+			interrupts = <82>;
+			clocks = <&rcc 0 158>;
+			status = "disabled";
+		};
+
+		usart8: serial at 40007c00 {
+			compatible = "st,stm32-usart", "st,stm32-uart";
+			reg = <0x40007c00 0x400>;
+			interrupts = <83>;
+			clocks = <&rcc 0 159>;
+			status = "disabled";
+		};
+
+		usart1: serial at 40011000 {
+			compatible = "st,stm32-usart", "st,stm32-uart";
+			reg = <0x40011000 0x400>;
+			interrupts = <37>;
+			clocks = <&rcc 0 164>;
+			status = "disabled";
+		};
+
+		usart6: serial at 40011400 {
+			compatible = "st,stm32-usart", "st,stm32-uart";
+			reg = <0x40011400 0x400>;
+			interrupts = <71>;
+			clocks = <&rcc 0 165>;
+			status = "disabled";
+		};
+
+		rcc: rcc at 40023810 {
+			#clock-cells = <2>;
+			compatible = "st,stm32f42xx-rcc", "st,stm32-rcc";
+			reg = <0x40023800 0x400>;
+			clocks = <&clk_hse>;
+		};
+
+		rng: rng at 50060800 {
+			compatible = "st,stm32-rng";
+			reg = <0x50060800 0x400>;
+			interrupts = <80>;
+			clocks = <&rcc 0 38>;
+		};
+	};
+};
+
+&systick {
+	clocks = <&rcc 1 0>;
+	status = "okay";
+};

[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

Currently, clocks can be enabled, only. To be feature-complete - and allow
a bit of power-saving in applications - disabling a clock should be
possible, too.

This extend the API of the DM clock driver to allow a clock to be disabled.

The corresponding operation is optional for not breaking existing drivers.
---
 drivers/clk/clk-uclass.c |   10 ++++++++++
 include/clk.h            |   18 ++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index b483c1e..462f5f8 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -44,6 +44,16 @@ int clk_enable(struct udevice *dev, int periph)
 	return ops->enable(dev, periph);
 }
 
+int clk_disable(struct udevice *dev, int periph)
+{
+	struct clk_ops *ops = clk_get_ops(dev);
+
+	if (!ops->disable)
+		return -ENOSYS;
+
+	return ops->disable(dev, periph);
+}
+
 ulong clk_get_periph_rate(struct udevice *dev, int periph)
 {
 	struct clk_ops *ops = clk_get_ops(dev);
diff --git a/include/clk.h b/include/clk.h
index ca20c3d..395f813 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -43,6 +43,15 @@ struct clk_ops {
 	int (*enable)(struct udevice *dev, int periph);
 
 	/**
+	 * disable() - Disable the clock for a peripheral
+	 *
+	 * @dev:	clock provider
+	 * @periph:	Peripheral ID to enable
+	 * @return zero on success, or -ve error code
+	 */
+	int (*disable)(struct udevice *dev, int periph);
+
+	/**
 	 * get_periph_rate() - Get clock rate for a peripheral
 	 *
 	 * @dev:	Device to check (UCLASS_CLK)
@@ -90,6 +99,15 @@ ulong clk_set_rate(struct udevice *dev, ulong rate);
 int clk_enable(struct udevice *dev, int periph);
 
 /**
+ * clk_disable() - Disable the clock for a peripheral
+ *
+ * @dev:	clock provider
+ * @periph:	Peripheral ID to enable
+ * @return zero on success, or -ve error code
+ */
+int clk_disable(struct udevice *dev, int periph);
+
+/**
  * clk_get_periph_rate() - Get current clock rate for a peripheral
  *
  * @dev:	Device to check (UCLASS_CLK)

[U-Boot] [PATCH v2 08/22] STM32: clock: provide dts-accessible clock driver

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

This implements an basic clock driver for the RCC-part of STM32 MCUs.
Currently, only enabling and disabling of peripheral clocks and retrieving main frequency is implemented.
---
 drivers/clk/Kconfig     |    4 ++
 drivers/clk/Makefile    |    1 
 drivers/clk/clk_stm32.c |  112 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 117 insertions(+)
 create mode 100644 drivers/clk/clk_stm32.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 6eee8eb..ebef031 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -23,4 +23,8 @@ config SPL_CLK
 source "drivers/clk/uniphier/Kconfig"
 source "drivers/clk/exynos/Kconfig"
 
+config CLK_STM32
+	bool "Enable clock driver for STM32 devices"
+	depends on CLK
+
 endmenu
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 81fe600..e7fd05a 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_SANDBOX) += clk_sandbox.o
 obj-$(CONFIG_MACH_PIC32) += clk_pic32.o
 obj-$(CONFIG_CLK_UNIPHIER) += uniphier/
 obj-$(CONFIG_CLK_EXYNOS) += exynos/
+obj-$(CONFIG_CLK_STM32) += clk_stm32.o
diff --git a/drivers/clk/clk_stm32.c b/drivers/clk/clk_stm32.c
new file mode 100644
index 0000000..b9f6e11
--- /dev/null
+++ b/drivers/clk/clk_stm32.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (C) 2016 Benjamin Tietz <uboot@dresden.micronet24.de>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ *
+ */
+#include <common.h>
+#include <clk.h>
+#include <dm.h>
+#include <asm/io.h>
+#include <asm/arch/stm32.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct stm32_rcc_clk_priv {
+	struct stm32_rcc_regs *regs;
+};
+
+#define PLLCFGR_PLLN(reg)	(((reg)>>6)&0x1ff)
+#define PLLCFGR_PLLM(reg)	(((reg)>>0)&0x3f)
+#define PLLCFGR_PLLP(reg)	(((reg)>>16)&0x3)
+#define PLLCFGR_PLLQ(reg)	(((reg)>>24)&0x0f)
+#define PLLCFGR_HSE(reg)	(reg & (1<<22))
+
+static long stm32_rcc_get_vco_rate(struct udevice *dev, u32 *regptr) {
+	struct stm32_rcc_clk_priv *data = dev_get_priv(dev);
+	long freq = 16000000;
+	if(!(data && data->regs))
+		return -EINVAL;
+	u32 reg = readl(&data->regs->pllcfgr);
+	if(regptr) *regptr = reg;
+	if(PLLCFGR_HSE(reg)) {
+		struct udevice *pclk;
+		int ret;
+		if((ret = clk_get_by_index(dev, 0, &pclk)) < 0)
+			return ret;
+
+		freq = clk_get_rate(pclk);
+	}
+	if(freq < 0) return freq;
+	return freq * PLLCFGR_PLLN(reg) / PLLCFGR_PLLM(reg);
+}
+
+static ulong stm32_rcc_get_rate(struct udevice *dev) {
+	u32 reg = 0;
+	long freq = stm32_rcc_get_vco_rate(dev, &reg);
+	if(freq < 0) return freq;
+	return freq / ( 2 + 2 * PLLCFGR_PLLP(reg));
+}
+
+static int stm32_rcc_xxable(struct udevice *dev, int periph, int on) {
+	struct stm32_rcc_clk_priv *data = dev_get_priv(dev);
+	if(!(data && data->regs))
+		return -EINVAL;
+
+	int port = periph >> 5;
+	int bit = periph & 0x1f;
+	if(port >= 8)
+		return -ENOSYS;
+
+	u32 *enr = &data->regs->ahb1enr;
+	on ?
+		setbits_le32(&enr[port], 1<<bit):
+		clrbits_le32(&enr[port], 1<<bit);
+	return 0;
+}
+
+static int stm32_rcc_enable(struct udevice *dev, int periph) {
+	return stm32_rcc_xxable(dev, periph, 1);
+}
+
+static int stm32_rcc_disable(struct udevice *dev, int periph) {
+	return stm32_rcc_xxable(dev, periph, 0);
+}
+
+static struct clk_ops stm32_rcc_clk_ops = {
+	.get_rate = stm32_rcc_get_rate,
+	.enable = stm32_rcc_enable,
+	.disable = stm32_rcc_disable,
+};
+
+static int stm32_rcc_clk_probe(struct udevice *dev) {
+	struct stm32_rcc_clk_priv *priv = dev_get_priv(dev);
+	fdt_addr_t addr;
+	fdt_size_t size;
+
+	addr = fdtdec_get_addr_size(gd->fdt_blob, dev->of_offset, "reg", &size);
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	if(size < sizeof(*priv->regs))
+		return -EINVAL;
+
+	priv->regs = (struct stm32_rcc_regs *) addr;
+
+	return 0;
+}
+
+static const struct udevice_id stm32_rcc_clk_ids[] = {
+	{ .compatible = "st,stm32f42xx-rcc", },
+	{}
+};
+
+U_BOOT_DRIVER(stm32_rcc_clk) = {
+	.name		= "stm32_rcc_clk",
+	.id		= UCLASS_CLK,
+	.of_match	= stm32_rcc_clk_ids,
+	.flags		= DM_FLAG_PRE_RELOC,
+	.ops		= &stm32_rcc_clk_ops,
+	.probe		= stm32_rcc_clk_probe,
+	.priv_auto_alloc_size = sizeof(struct stm32_rcc_clk_priv),
+};

[U-Boot] [PATCH v2 09/22] DTS: STM32f429: add gpio-banks

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

This adds all GPIO-banks known by the STM32F4 MCU to the corresponding
.dts-file. Those not available in the the TQFP-package are currently
disabled.
---
 arch/arm/dts/stm32f429.dtsi |   92 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/arch/arm/dts/stm32f429.dtsi b/arch/arm/dts/stm32f429.dtsi
index 5e1e234..0d032fb 100644
--- a/arch/arm/dts/stm32f429.dtsi
+++ b/arch/arm/dts/stm32f429.dtsi
@@ -57,6 +57,98 @@
 	};
 
 	soc {
+		gpioA:	gpio at 40020000 {
+			#gpio-cells = <1>;
+			gpio-controller;
+			compatible = "st,stm32-gpio";
+			reg = <0x40020000 0x400>;
+			clocks = <&rcc 0 0>;
+		};
+
+		gpioB:	gpio at 40020400 {
+			#gpio-cells = <1>;
+			gpio-controller;
+			compatible = "st,stm32-gpio";
+			reg = <0x40020400 0x400>;
+			clocks = <&rcc 0 1>;
+		};
+
+		gpioC:	gpio at 40020800 {
+			#gpio-cells = <1>;
+			gpio-controller;
+			compatible = "st,stm32-gpio";
+			reg = <0x40020800 0x400>;
+			clocks = <&rcc 0 2>;
+		};
+
+		gpioD:	gpio at 40020c00 {
+			#gpio-cells = <1>;
+			gpio-controller;
+			compatible = "st,stm32-gpio";
+			reg = <0x40020c00 0x400>;
+			clocks = <&rcc 0 3>;
+		};
+
+		gpioE:	gpio at 40021000 {
+			#gpio-cells = <1>;
+			gpio-controller;
+			compatible = "st,stm32-gpio";
+			reg = <0x40021000 0x400>;
+			clocks = <&rcc 0 4>;
+		};
+
+		gpioF:	gpio at 40021400 {
+			#gpio-cells = <1>;
+			gpio-controller;
+			compatible = "st,stm32-gpio";
+			reg = <0x40021400 0x400>;
+			clocks = <&rcc 0 5>;
+		};
+
+		gpioG:	gpio at 40021800 {
+			#gpio-cells = <1>;
+			gpio-controller;
+			compatible = "st,stm32-gpio";
+			reg = <0x40021800 0x400>;
+			clocks = <&rcc 0 6>;
+		};
+
+		gpioH:	gpio at 40021c00 {
+			#gpio-cells = <1>;
+			gpio-count = <2>;
+			gpio-controller;
+			compatible = "st,stm32-gpio";
+			reg = <0x40021c00 0x400>;
+			clocks = <&rcc 0 7>;
+		};
+
+		gpioI:	gpio at 40022000 {
+			#gpio-cells = <1>;
+			gpio-controller;
+			compatible = "st,stm32-gpio";
+			reg = <0x40022000 0x400>;
+			clocks = <&rcc 0 8>;
+			status = "disabled";
+		};
+
+		gpioJ:	gpio at 40022400 {
+			#gpio-cells = <1>;
+			gpio-controller;
+			compatible = "st,stm32-gpio";
+			reg = <0x40022400 0x400>;
+			clocks = <&rcc 0 9>;
+			status = "disabled";
+		};
+
+		gpioK:	gpio at 40022800 {
+			#gpio-cells = <1>;
+			gpio-controller;
+			compatible = "st,stm32-gpio";
+			reg = <0x40022800 0x400>;
+			clocks = <&rcc 0 10>;
+			status = "disabled";
+		};
+
 		timer2: timer at 40000000 {
 			compatible = "st,stm32-timer";
 			reg = <0x40000000 0x400>;

[U-Boot] [PATCH v2 10/22] STM32: gpio: group SOC-specific code to one ifdef/elif construct

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

In the STM32 gpio driver, multiple family-dependent sections existed.
All but one just statically filled a datastructure. The following patch
creates these structure const-static within a single family-dependent
section.

This seperation improves the maintainability for further rework on the
driver.
---
 drivers/gpio/stm32_gpio.c |   66 +++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/drivers/gpio/stm32_gpio.c b/drivers/gpio/stm32_gpio.c
index 516dfcc..be662c3 100644
--- a/drivers/gpio/stm32_gpio.c
+++ b/drivers/gpio/stm32_gpio.c
@@ -74,6 +74,23 @@ int stm32_gpio_config(const struct stm32_gpio_dsc *dsc,
 out:
 	return rv;
 }
+
+static struct stm32_gpio_ctl ctl_in = {
+	.af = STM32_GPIO_AF0,
+	.mode = STM32_GPIO_MODE_IN,
+	.otype = STM32_GPIO_OTYPE_PP,
+	.pupd = STM32_GPIO_PUPD_NO,
+	.speed = STM32_GPIO_SPEED_50M,
+};
+
+static struct stm32_gpio_ctl ctl_out = {
+	.af = STM32_GPIO_AF0,
+	.mode = STM32_GPIO_MODE_OUT,
+	.otype = STM32_GPIO_OTYPE_PP,
+	.pupd = STM32_GPIO_PUPD_NO,
+	.speed = STM32_GPIO_SPEED_50M,
+};
+
 #elif defined(CONFIG_STM32F1)
 static const unsigned long io_base[] = {
 	STM32_GPIOA_BASE, STM32_GPIOB_BASE, STM32_GPIOC_BASE,
@@ -146,6 +163,21 @@ int stm32_gpio_config(const struct stm32_gpio_dsc *dsc,
 out:
 	return rv;
 }
+
+static struct stm32_gpio_ctl ctl_in = {
+	.mode = STM32_GPIO_MODE_IN,
+	.icnf = STM32_GPIO_ICNF_IN_FLT,
+	.ocnf = STM32_GPIO_OCNF_GP_PP,	/* ignored for input */
+	.pupd = STM32_GPIO_PUPD_UP,	/* ignored for floating */
+};
+
+static struct stm32_gpio_ctl ctl_out = {
+	.mode = STM32_GPIO_MODE_OUT_50M,
+	.ocnf = STM32_GPIO_OCNF_GP_PP,
+	.icnf = STM32_GPIO_ICNF_IN_FLT,	/* ignored for output */
+	.pupd = STM32_GPIO_PUPD_UP,	/* ignored for output */
+};
+
 #else
 #error STM32 family not supported
 #endif
@@ -203,52 +235,22 @@ int gpio_free(unsigned gpio)
 int gpio_direction_input(unsigned gpio)
 {
 	struct stm32_gpio_dsc dsc;
-	struct stm32_gpio_ctl ctl;
 
 	dsc.port = stm32_gpio_to_port(gpio);
 	dsc.pin = stm32_gpio_to_pin(gpio);
-#if defined(CONFIG_STM32F4) || defined(CONFIG_STM32F7)
-	ctl.af = STM32_GPIO_AF0;
-	ctl.mode = STM32_GPIO_MODE_IN;
-	ctl.otype = STM32_GPIO_OTYPE_PP;
-	ctl.pupd = STM32_GPIO_PUPD_NO;
-	ctl.speed = STM32_GPIO_SPEED_50M;
-#elif defined(CONFIG_STM32F1)
-	ctl.mode = STM32_GPIO_MODE_IN;
-	ctl.icnf = STM32_GPIO_ICNF_IN_FLT;
-	ctl.ocnf = STM32_GPIO_OCNF_GP_PP;	/* ignored for input */
-	ctl.pupd = STM32_GPIO_PUPD_UP;		/* ignored for floating */
-#else
-#error STM32 family not supported
-#endif
 
-	return stm32_gpio_config(&dsc, &ctl);
+	return stm32_gpio_config(&dsc, &ctl_in);
 }
 
 int gpio_direction_output(unsigned gpio, int value)
 {
 	struct stm32_gpio_dsc dsc;
-	struct stm32_gpio_ctl ctl;
 	int res;
 
 	dsc.port = stm32_gpio_to_port(gpio);
 	dsc.pin = stm32_gpio_to_pin(gpio);
-#if defined(CONFIG_STM32F4) || defined(CONFIG_STM32F7)
-	ctl.af = STM32_GPIO_AF0;
-	ctl.mode = STM32_GPIO_MODE_OUT;
-	ctl.otype = STM32_GPIO_OTYPE_PP;
-	ctl.pupd = STM32_GPIO_PUPD_NO;
-	ctl.speed = STM32_GPIO_SPEED_50M;
-#elif defined(CONFIG_STM32F1)
-	ctl.mode = STM32_GPIO_MODE_OUT_50M;
-	ctl.ocnf = STM32_GPIO_OCNF_GP_PP;
-	ctl.icnf = STM32_GPIO_ICNF_IN_FLT;	/* ignored for output */
-	ctl.pupd = STM32_GPIO_PUPD_UP;		/* ignored for output */
-#else
-#error STM32 family not supported
-#endif
 
-	res = stm32_gpio_config(&dsc, &ctl);
+	res = stm32_gpio_config(&dsc, &ctl_out);
 	if (res < 0)
 		goto out;
 	res = stm32_gpout_set(&dsc, value);

[U-Boot] [PATCH v2 11/22] GPIO: STM32: make DTS-aware

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

If CONFIG_DM_GPIO is enabled, the STM32 SOC GPIO driver will switch it's behaviour
from implementing the gpio_*() interface directly to provide a gpio-device.
---
 drivers/gpio/stm32_gpio.c |  133 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)

diff --git a/drivers/gpio/stm32_gpio.c b/drivers/gpio/stm32_gpio.c
index be662c3..844b3e1 100644
--- a/drivers/gpio/stm32_gpio.c
+++ b/drivers/gpio/stm32_gpio.c
@@ -220,6 +220,138 @@ out:
 	return rv;
 }
 
+#ifdef CONFIG_DM_GPIO
+#include <asm-generic/gpio.h>
+#include <dm/device.h>
+#include <clk.h>
+#include <dt-bindings/gpio/gpio.h>
+
+struct stm32_gpio_priv {
+	int portnum;
+	u32 requested;
+};
+
+static int stm32_gpio_request(struct udevice *dev, unsigned offset, const char *label)
+{
+	struct stm32_gpio_priv *priv = dev_get_priv(dev);
+	if(priv->requested & (1<<offset))
+		return -EBUSY;
+	if(!priv->requested) {
+		struct udevice *clk = NULL;
+		int clk_id;
+		if((clk_id = clk_get_by_index(dev, 0, &clk)) >= 0)
+			clk_enable(clk, clk_id);
+	}
+	priv->requested |= 1<<offset;
+	return 0;
+}
+
+static int stm32_gpio_free(struct udevice *dev, unsigned offset)
+{
+	struct stm32_gpio_priv *priv = dev_get_priv(dev);
+	priv->requested &= ~(1<<offset);
+	if(!priv->requested) {
+		struct udevice *clk = NULL;
+		int clk_id;
+		if((clk_id = clk_get_by_index(dev, 0, &clk)) >= 0)
+			clk_disable(clk, clk_id);
+	}
+	return 0;
+}
+
+static int stm32_gpio_direction_input(struct udevice *dev, unsigned offset)
+{
+	struct stm32_gpio_priv *priv = dev_get_priv(dev);
+	struct stm32_gpio_dsc dsc = {
+		.port = priv->portnum,
+		.pin = offset,
+	};
+	return stm32_gpio_config(&dsc, &ctl_in);
+}
+
+static int stm32_gpio_direction_output(struct udevice *dev, unsigned offset, int value)
+{
+	struct stm32_gpio_priv *priv = dev_get_priv(dev);
+	struct stm32_gpio_dsc dsc = {
+		.port = priv->portnum,
+		.pin = offset,
+	};
+	int ret = stm32_gpio_config(&dsc, &ctl_out);
+	if(ret >= 0)
+		ret = stm32_gpout_set(&dsc, value);
+	return ret;
+}
+
+static int stm32_gpio_get_value(struct udevice *dev, unsigned offset)
+{
+	struct stm32_gpio_priv *priv = dev_get_priv(dev);
+	struct stm32_gpio_dsc dsc = {
+		.port = priv->portnum,
+		.pin = offset,
+	};
+	return !!stm32_gpin_get(&dsc);
+}
+
+static int stm32_gpio_set_value(struct udevice *dev, unsigned offset, int value)
+{
+	struct stm32_gpio_priv *priv = dev_get_priv(dev);
+	struct stm32_gpio_dsc dsc = {
+		.port = priv->portnum,
+		.pin = offset,
+	};
+	return stm32_gpout_set(&dsc, value);
+}
+
+static struct dm_gpio_ops stm32_gpio_ops = {
+	.request = stm32_gpio_request,
+	.free = stm32_gpio_free,
+	.direction_input = stm32_gpio_direction_input,
+	.direction_output = stm32_gpio_direction_output,
+	.get_value = stm32_gpio_get_value,
+	.set_value = stm32_gpio_set_value,
+};
+
+static int stm32_gpio_probe(struct udevice *dev)
+{
+	struct stm32_gpio_priv *priv = dev_get_priv(dev);
+	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+	fdt_addr_t addr;
+	fdt_size_t size;
+	int i;
+	priv->requested = 0;
+
+	addr = fdtdec_get_addr_size(gd->fdt_blob, dev->of_offset, "reg", &size);
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	if(size < sizeof(struct stm32_gpio_ctl))
+		return -EINVAL;
+
+	for(i=0; i < sizeof(io_base)/sizeof(io_base[0]); i++) {
+		if(io_base[i] != addr)
+			continue;
+		priv->portnum = i;
+		uc_priv->bank_name = dev->name;
+		uc_priv->gpio_count = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "gpio-count", 16);
+		return 0;
+	}
+	return -ENXIO;
+}
+
+static const struct udevice_id stm32_gpio_ids[] = {
+	{ .compatible = "st,stm32-gpio" },
+	{ }
+};
+
+U_BOOT_DRIVER(stm32_gpio) = {
+	.name		= "stm32_gpio",
+	.id		= UCLASS_GPIO,
+	.ops		= &stm32_gpio_ops,
+	.probe		= stm32_gpio_probe,
+	.priv_auto_alloc_size = sizeof(struct stm32_gpio_priv),
+	.of_match	= stm32_gpio_ids,
+};
+#else
 /* Common GPIO API */
 
 int gpio_request(unsigned gpio, const char *label)
@@ -277,3 +409,4 @@ int gpio_set_value(unsigned gpio, int value)
 
 	return stm32_gpout_set(&dsc, value);
 }
+#endif

[U-Boot] [PATCH v2 12/22] STM32F429-discovery: led: disable board-specific code, if DM is selected

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

If the device-tree gpio selection is selected, the hardcoded board-specific
led-initialization will fail.
This will disable that code, if CONFIG_DM_GPIO is enabled.
---
 board/st/stm32f429-discovery/Makefile |    3 +++
 include/configs/stm32f429-discovery.h |   10 +++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/board/st/stm32f429-discovery/Makefile b/board/st/stm32f429-discovery/Makefile
index d94059d..34ee711 100644
--- a/board/st/stm32f429-discovery/Makefile
+++ b/board/st/stm32f429-discovery/Makefile
@@ -9,4 +9,7 @@
 #
 
 obj-y	:= stm32f429-discovery.o
+
+ifndef CONFIG_DM_GPIO
 obj-y	+= led.o
+endif
diff --git a/include/configs/stm32f429-discovery.h b/include/configs/stm32f429-discovery.h
index 9d275c0..04066ff 100644
--- a/include/configs/stm32f429-discovery.h
+++ b/include/configs/stm32f429-discovery.h
@@ -42,17 +42,21 @@
 #define CONFIG_ENV_SECT_SIZE		(128 << 10)
 #define CONFIG_ENV_SIZE			(8 << 10)
 
-#define CONFIG_BOARD_SPECIFIC_LED
+#define CONFIG_GPIO_LED		1
+#ifndef CONFIG_DM_GPIO
 #define CONFIG_RED_LED			110
 #define CONFIG_GREEN_LED		109
-#define CONFIG_GPIO_LED		1
-#define CONFIG_STATUS_LED
+#ifdef CONFIG_STATUS_LED
+#define CONFIG_BOARD_SPECIFIC_LED
+#define STATUS_LED_RED	CONFIG_RED_LED
 #define STATUS_LED_BIT		CONFIG_RED_LED
 #define STATUS_LED_STATE	0
 #define STATUS_LED_PERIOD	0
 #define STATUS_LED_BIT1		CONFIG_GREEN_LED
 #define STATUS_LED_STATE1	0
 #define STATUS_LED_PERIOD1	0
+#endif
+#endif
 
 #define CONFIG_STM32_GPIO
 #define CONFIG_STM32_FLASH

[U-Boot] [PATCH v2 13/22] GPIO/LED: make more robust, if STATUS_LED isn't selected

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

If STATUS_LED isn't selected, neither led_id_t is selected, nor STATUS_LED_ON.
The following will set led_id_t to a reasonable default and define STATUS_LED_ON for that case.
---
 drivers/misc/gpio_led.c   |    4 ++++
 drivers/misc/status_led.c |    2 ++
 include/status_led.h      |    4 ++++
 3 files changed, 10 insertions(+)

diff --git a/drivers/misc/gpio_led.c b/drivers/misc/gpio_led.c
index 164c30d..9c0ea13 100644
--- a/drivers/misc/gpio_led.c
+++ b/drivers/misc/gpio_led.c
@@ -13,6 +13,10 @@
 #define CONFIG_GPIO_LED_INVERTED_TABLE {}
 #endif
 
+#ifndef STATUS_LED_ON
+#define STATUS_LED_ON 2
+#endif
+
 static led_id_t gpio_led_inv[] = CONFIG_GPIO_LED_INVERTED_TABLE;
 
 static int gpio_led_gpio_value(led_id_t mask, int state)
diff --git a/drivers/misc/status_led.c b/drivers/misc/status_led.c
index 31e8831..052ecf0 100644
--- a/drivers/misc/status_led.c
+++ b/drivers/misc/status_led.c
@@ -27,11 +27,13 @@ typedef struct {
 } led_dev_t;
 
 led_dev_t led_dev[] = {
+#if defined(STATUS_LED_BIT)
     {	STATUS_LED_BIT,
 	STATUS_LED_STATE,
 	STATUS_LED_PERIOD,
 	0,
     },
+#endif
 #if defined(STATUS_LED_BIT1)
     {	STATUS_LED_BIT1,
 	STATUS_LED_STATE1,
diff --git a/include/status_led.h b/include/status_led.h
index 396ea88..c3795a7 100644
--- a/include/status_led.h
+++ b/include/status_led.h
@@ -103,6 +103,10 @@ void __led_blink(led_id_t mask, int freq);
 # include <asm/status_led.h>
 #endif
 
+#else
+
+typedef unsigned long led_id_t;
+
 #endif	/* CONFIG_STATUS_LED	*/
 
 /*

[U-Boot] [PATCH v2 14/22] Cmd: LED: rewrite to prepare non-static access

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

Previously, all knwon LED were hold in a single array and computed at compile-time.
With the new variant, if all LEDs are known, these will also be computed at compile-time.
Using functions will allow additional dynamic (eg. DM-based) LED allocation.

Apart from that, the led-command becomes independent from the STATUS_LED setting.
---
 cmd/led.c |  340 ++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 233 insertions(+), 107 deletions(-)

diff --git a/cmd/led.c b/cmd/led.c
index b0f1a61..5295a6e 100644
--- a/cmd/led.c
+++ b/cmd/led.c
@@ -2,6 +2,9 @@
  * (C) Copyright 2010
  * Jason Kridner <jkridner@beagleboard.org>
  *
+ * (C) Copyright 2016
+ * Benjamin Tietz <uboot@dresden.micronet24.de>
+ *
  * Based on cmd_led.c patch from:
  * http://www.mail-archive.com/u-boot at lists.denx.de/msg06873.html
  * (C) Copyright 2008
@@ -15,53 +18,241 @@
 #include <command.h>
 #include <status_led.h>
 
+enum led_cmd { LED_ON, LED_OFF, LED_TOGGLE, LED_BLINK, LED_LIST, LED_MAX_OP };
+
+struct led_tbl_s;
+
+typedef void (* led_op)(const struct led_tbl_s *, enum led_cmd, char *arg);
+
 struct led_tbl_s {
-	char		*string;	/* String for use in the command */
+	const char	*string;	/* String for use in the command */
 	led_id_t	mask;		/* Mask used for calling __led_set() */
-	void		(*off)(void);	/* Optional function for turning LED off */
-	void		(*on)(void);	/* Optional function for turning LED on */
-	void		(*toggle)(void);/* Optional function for toggling LED */
+	led_op op[LED_MAX_OP];		/* functions for handling LED commands */
 };
 
 typedef struct led_tbl_s led_tbl_t;
 
-static const led_tbl_t led_commands[] = {
+static int call_led(const char *name, enum led_cmd cmd, char *arg);
+
+static void _led_list_name(const led_tbl_t *led, enum led_cmd cmd, char *arg) {
+	printf("%s\n", led->string);
+}
+
+#define LED_MASK_FUNC(fnc)	((led_op) fnc)
+#define LED_TEST_RET(name, ret)	if(strcmp(name, ret.string) == 0) return &ret;
+#define LED_TBL_COLOURED(clr)	static const led_tbl_t _led_##clr = { \
+	.string = #clr, \
+	.op = { \
+		[LED_ON] = LED_MASK_FUNC(clr ## _led_on), \
+		[LED_OFF] = LED_MASK_FUNC(clr ## _led_off), \
+		[LED_LIST] = _led_list_name, \
+	}, \
+}
+
+#ifdef STATUS_LED_GREEN
+LED_TBL_COLOURED(green);
+#define LED_NAME_GREEN		_led_green.string
+#define LED_TEST_GREEN(name)	LED_TEST_RET(name, _led_green)
+#else
+#define LED_NAME_GREEN		NULL
+#define LED_TEST_GREEN(name)	while(0)
+#endif
+
+#ifdef STATUS_LED_YELLOW
+LED_TBL_COLOURED(yellow);
+#define LED_NAME_YELLOW		_led_yellow.string
+#define LED_TEST_YELLOW(name)	LED_TEST_RET(name, _led_yellow)
+#else
+#define LED_NAME_YELLOW		NULL
+#define LED_TEST_YELLOW(name)	while(0)
+#endif
+
+#ifdef STATUS_LED_RED
+LED_TBL_COLOURED(red);
+#define LED_NAME_RED		_led_red.string
+#define LED_TEST_RED(name)	LED_TEST_RET(name, _led_red)
+#else
+#define LED_NAME_RED		NULL
+#define LED_TEST_RED(name)	while(0)
+#endif
+
+#ifdef STATUS_LED_BLUE
+LED_TBL_COLOURED(blue);
+#define LED_NAME_BLUE		_led_blue.string
+#define LED_TEST_BLUE(name)	LED_TEST_RET(name, _led_blue)
+#else
+#define LED_NAME_BLUE		NULL
+#define LED_TEST_BLUE(name)	while(0)
+#endif
+
 #ifdef CONFIG_BOARD_SPECIFIC_LED
+/*
+ * LED drivers providing a blinking LED functionality, like the
+ * PCA9551, can override this empty weak function
+ */
+void __weak __led_blink(led_id_t mask, int freq)
+{
+}
+
+static void _led_status_onoff(const led_tbl_t *led, enum led_cmd cmd, char *arg) {
+	__led_set(led->mask, cmd != LED_ON ? STATUS_LED_OFF : STATUS_LED_ON);
+}
+static void _led_status_toggle(const led_tbl_t *led, enum led_cmd cmd, char *arg) {
+	__led_toggle(led->mask);
+}
+static void _led_status_blink(const led_tbl_t *led, enum led_cmd cmd, char *freq) {
+	if (!freq)
+		return;
+
+	__led_blink(led->mask, simple_strtoul(freq, NULL, 10));
+}
+
+#define LED_TBL_STATUS(name, _mask)	static const led_tbl_t _led_##name = { \
+	.string = #name, \
+	.mask = _mask, \
+	.op = { \
+		[LED_ON] = _led_status_onoff, \
+		[LED_OFF] = _led_status_onoff, \
+		[LED_TOGGLE] = _led_status_toggle, \
+		[LED_BLINK] = _led_status_blink, \
+		[LED_LIST] = _led_list_name, \
+	}, \
+}
+
 #ifdef STATUS_LED_BIT
-	{ "0", STATUS_LED_BIT, NULL, NULL, NULL },
+LED_TBL_STATUS(0, STATUS_LED_BIT);
+#define LED_NAME_0		_led_0.string
+#define LED_TEST_0(name)	LED_TEST_RET(name, _led_0)
 #endif
+
 #ifdef STATUS_LED_BIT1
-	{ "1", STATUS_LED_BIT1, NULL, NULL, NULL },
+LED_TBL_STATUS(1, STATUS_LED_BIT1);
+#define LED_NAME_1		_led_1.string
+#define LED_TEST_1(name)	LED_TEST_RET(name, _led_1)
 #endif
+
 #ifdef STATUS_LED_BIT2
-	{ "2", STATUS_LED_BIT2, NULL, NULL, NULL },
+LED_TBL_STATUS(2, STATUS_LED_BIT2);
+#define LED_NAME_2		_led_2.string
+#define LED_TEST_2(name)	LED_TEST_RET(name, _led_2)
 #endif
+
 #ifdef STATUS_LED_BIT3
-	{ "3", STATUS_LED_BIT3, NULL, NULL, NULL },
+LED_TBL_STATUS(3, STATUS_LED_BIT3);
+#define LED_NAME_3		_led_3.string
+#define LED_TEST_3(name)	LED_TEST_RET(name, _led_3)
 #endif
+
 #ifdef STATUS_LED_BIT4
-	{ "4", STATUS_LED_BIT4, NULL, NULL, NULL },
+LED_TBL_STATUS(4, STATUS_LED_BIT4);
+#define LED_NAME_4		_led_4.string
+#define LED_TEST_4(name)	LED_TEST_RET(name, _led_4)
 #endif
+
 #ifdef STATUS_LED_BIT5
-	{ "5", STATUS_LED_BIT5, NULL, NULL, NULL },
+LED_TBL_STATUS(5, STATUS_LED_BIT5);
+#define LED_NAME_5		_led_5.string
+#define LED_TEST_5(name)	LED_TEST_RET(name, _led_5)
 #endif
+#endif  /* CONFIG_BOARD_SPECIFIC_LED */
+
+#ifndef LED_NAME_0
+#define LED_NAME_0		NULL
+#define LED_TEST_0(name)	while(0)
 #endif
-#ifdef STATUS_LED_GREEN
-	{ "green", STATUS_LED_GREEN, green_led_off, green_led_on, NULL },
+
+#ifndef LED_NAME_1
+#define LED_NAME_1		NULL
+#define LED_TEST_1(name)	while(0)
 #endif
-#ifdef STATUS_LED_YELLOW
-	{ "yellow", STATUS_LED_YELLOW, yellow_led_off, yellow_led_on, NULL },
+
+#ifndef LED_NAME_2
+#define LED_NAME_2		NULL
+#define LED_TEST_2(name)	while(0)
 #endif
-#ifdef STATUS_LED_RED
-	{ "red", STATUS_LED_RED, red_led_off, red_led_on, NULL },
+
+#ifndef LED_NAME_3
+#define LED_NAME_3		NULL
+#define LED_TEST_3(name)	while(0)
 #endif
-#ifdef STATUS_LED_BLUE
-	{ "blue", STATUS_LED_BLUE, blue_led_off, blue_led_on, NULL },
+
+#ifndef LED_NAME_4
+#define LED_NAME_4		NULL
+#define LED_TEST_4(name)	while(0)
 #endif
-	{ NULL, 0, NULL, NULL, NULL }
+
+#ifndef LED_NAME_5
+#define LED_NAME_5		NULL
+#define LED_TEST_5(name)	while(0)
+#endif
+
+static int _led_count(void) {
+	int i = 0;
+	if(LED_NAME_GREEN) i++;
+	if(LED_NAME_RED) i++;
+	if(LED_NAME_BLUE) i++;
+	if(LED_NAME_YELLOW) i++;
+	if(LED_NAME_0) i++;
+	if(LED_NAME_1) i++;
+	if(LED_NAME_2) i++;
+	if(LED_NAME_3) i++;
+	if(LED_NAME_4) i++;
+	if(LED_NAME_5) i++;
+	return i;
+}
+
+#define TEST_ADD_NAME(name, tbl, size) do { if((name) && ((size) > 0)) { *tbl++ = name; size--; } } while(0)
+static int _led_list(const char **tbl, int size) {
+	int init_size = size;
+	TEST_ADD_NAME(LED_NAME_GREEN, tbl, size);
+	TEST_ADD_NAME(LED_NAME_RED, tbl, size);
+	TEST_ADD_NAME(LED_NAME_BLUE, tbl, size);
+	TEST_ADD_NAME(LED_NAME_YELLOW, tbl, size);
+	TEST_ADD_NAME(LED_NAME_0, tbl, size);
+	TEST_ADD_NAME(LED_NAME_1, tbl, size);
+	TEST_ADD_NAME(LED_NAME_2, tbl, size);
+	TEST_ADD_NAME(LED_NAME_3, tbl, size);
+	TEST_ADD_NAME(LED_NAME_4, tbl, size);
+	TEST_ADD_NAME(LED_NAME_5, tbl, size);
+	return init_size - size;
+}
+
+static void _led_all_cmd(const led_tbl_t *led, enum led_cmd cmd, char *arg) {
+	int cnt = _led_count();
+	const char *leds[cnt];
+	int i;
+
+	_led_list(leds, cnt);
+	for(i=0; i < cnt; i++)
+		call_led(leds[i], cmd, arg);
+}
+
+static const led_tbl_t _led_all = {
+	.string = "all",
+	.op = {
+		[LED_ON] = _led_all_cmd,
+		[LED_OFF] = _led_all_cmd,
+		[LED_TOGGLE] = _led_all_cmd,
+		[LED_BLINK] = _led_all_cmd,
+		[LED_LIST] = _led_all_cmd,
+	},
 };
+#define LED_TEST_ALL(name)	LED_TEST_RET(name, _led_all)
 
-enum led_cmd { LED_ON, LED_OFF, LED_TOGGLE, LED_BLINK };
+static const led_tbl_t *get_led(const char *name) {
+	LED_TEST_ALL(name);
+	LED_TEST_GREEN(name);
+	LED_TEST_YELLOW(name);
+	LED_TEST_RED(name);
+	LED_TEST_BLUE(name);
+	LED_TEST_0(name);
+	LED_TEST_1(name);
+	LED_TEST_2(name);
+	LED_TEST_3(name);
+	LED_TEST_4(name);
+	LED_TEST_5(name);
+	return NULL;
+}
 
 enum led_cmd get_led_cmd(char *var)
 {
@@ -73,23 +264,29 @@ enum led_cmd get_led_cmd(char *var)
 		return LED_TOGGLE;
 	if (strcmp(var, "blink") == 0)
 		return LED_BLINK;
+	if (strcmp(var, "list") == 0)
+		return LED_LIST;
 
 	return -1;
 }
 
-/*
- * LED drivers providing a blinking LED functionality, like the
- * PCA9551, can override this empty weak function
- */
-void __weak __led_blink(led_id_t mask, int freq)
+static int call_led(const char *name, enum led_cmd cmd, char *arg)
 {
+	const led_tbl_t *led = get_led(name);
+	if(!led)
+		return CMD_RET_USAGE;
+
+	if(cmd >= LED_MAX_OP)
+		return CMD_RET_USAGE;
+	if(led->op[cmd])
+		led->op[cmd](led, cmd, arg);
+	return 0;
+
 }
 
 int do_led (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	int i, match = 0;
 	enum led_cmd cmd;
-	int freq;
 
 	/* Validate arguments */
 	if ((argc < 3) || (argc > 4))
@@ -100,87 +297,16 @@ int do_led (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return CMD_RET_USAGE;
 	}
 
-	for (i = 0; led_commands[i].string; i++) {
-		if ((strcmp("all", argv[1]) == 0) ||
-		    (strcmp(led_commands[i].string, argv[1]) == 0)) {
-			match = 1;
-			switch (cmd) {
-			case LED_ON:
-				if (led_commands[i].on)
-					led_commands[i].on();
-				else
-					__led_set(led_commands[i].mask,
-							  STATUS_LED_ON);
-				break;
-			case LED_OFF:
-				if (led_commands[i].off)
-					led_commands[i].off();
-				else
-					__led_set(led_commands[i].mask,
-							  STATUS_LED_OFF);
-				break;
-			case LED_TOGGLE:
-				if (led_commands[i].toggle)
-					led_commands[i].toggle();
-				else
-					__led_toggle(led_commands[i].mask);
-				break;
-			case LED_BLINK:
-				if (argc != 4)
-					return CMD_RET_USAGE;
-
-				freq = simple_strtoul(argv[3], NULL, 10);
-				__led_blink(led_commands[i].mask, freq);
-			}
-			/* Need to set only 1 led if led_name wasn't 'all' */
-			if (strcmp("all", argv[1]) != 0)
-				break;
-		}
-	}
-
-	/* If we ran out of matches, print Usage */
-	if (!match) {
-		return CMD_RET_USAGE;
-	}
-
-	return 0;
+	return call_led(argv[1], cmd, argc != 4 ? NULL : argv[3]);
 }
 
 U_BOOT_CMD(
 	led, 4, 1, do_led,
-	"["
-#ifdef CONFIG_BOARD_SPECIFIC_LED
-#ifdef STATUS_LED_BIT
-	"0|"
-#endif
-#ifdef STATUS_LED_BIT1
-	"1|"
-#endif
-#ifdef STATUS_LED_BIT2
-	"2|"
-#endif
-#ifdef STATUS_LED_BIT3
-	"3|"
-#endif
-#ifdef STATUS_LED_BIT4
-	"4|"
-#endif
-#ifdef STATUS_LED_BIT5
-	"5|"
-#endif
-#endif
-#ifdef STATUS_LED_GREEN
-	"green|"
-#endif
-#ifdef STATUS_LED_YELLOW
-	"yellow|"
-#endif
-#ifdef STATUS_LED_RED
-	"red|"
-#endif
-#ifdef STATUS_LED_BLUE
-	"blue|"
-#endif
-	"all] [on|off|toggle|blink] [blink-freq in ms]",
-	"[led_name] [on|off|toggle|blink] sets or clears led(s)"
+	"[led_name|all] [on|off|toggle|blink|list] [blink-freq in ms]",
+	"if 'all' is given as led-name, all known leds will be affected by the command\n"
+	"[on] sets an led\n"
+	"[off] clears an led\n"
+	"[toggle] inverts an led\n"
+	"[blink freq] let an led blink, if supported by the controller\n"
+	"[list] shows the name of known led\n"
 );

[U-Boot] [PATCH v2 15/22] DTS: STM32F429-disco: add board leds and enable rcc

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

The following adds the configuration of the user-leds to the device tree for STM32F429 discovery board.
---
 arch/arm/dts/stm32f429-disco.dts |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/dts/stm32f429-disco.dts b/arch/arm/dts/stm32f429-disco.dts
index 2bae81c..7159f82 100644
--- a/arch/arm/dts/stm32f429-disco.dts
+++ b/arch/arm/dts/stm32f429-disco.dts
@@ -64,6 +64,20 @@
 	aliases {
 		serial0 = &usart1;
 	};
+
+	leds {
+		compatible = "gpio-leds";
+		user1 {
+			label = "user:green";
+			gpios = <&gpioG 13 0>;
+		};
+
+		user2 {
+			label = "user:red";
+			gpios = <&gpioG 14 0>;
+		};
+	};
+
 };
 
 &clk_hse {
@@ -73,3 +87,11 @@
 &usart1 {
 	status = "okay";
 };
+
+&rcc {
+	status = "okay";
+};
+
+&gpioG {
+	status = "okay";
+};

[U-Boot] [PATCH v2 16/22] LED: add function to retrieve a device's label

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

This extends the API of the led-driver to retrieve the label of and led-device.
---
 drivers/led/led-uclass.c |    6 ++++++
 include/led.h            |    8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c
index 784ac87..f5fbbcb 100644
--- a/drivers/led/led-uclass.c
+++ b/drivers/led/led-uclass.c
@@ -32,6 +32,12 @@ int led_get_by_label(const char *label, struct udevice **devp)
 	return -ENODEV;
 }
 
+const char *led_get_label(struct udevice *dev)
+{
+	struct led_uclass_plat *uc_plat = dev_get_uclass_platdata(dev);
+	return uc_plat->label;
+}
+
 int led_set_on(struct udevice *dev, int on)
 {
 	struct led_ops *ops = led_get_ops(dev);
diff --git a/include/led.h b/include/led.h
index b929d0c..32e0dec 100644
--- a/include/led.h
+++ b/include/led.h
@@ -40,6 +40,14 @@ struct led_ops {
 int led_get_by_label(const char *label, struct udevice **devp);
 
 /**
+ * led_get_label() - get the label of a given led-device
+ *
+ * @dev:	led device
+ * @return the name of the led
+ */
+const char *led_get_label(struct udevice *dev);
+
+/**
  * led_set_on() - set the state of an LED
  *
  * @dev:	LED device to change

[U-Boot] [PATCH v2 17/22] LED: provide function to count and get all (DM-)LEDs

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

This extends the API of the LED-driver to count the number
of configured LEDs and retrieving an array of them.
---
 drivers/led/led-uclass.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++
 include/led.h            |   18 ++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c
index f5fbbcb..ce519d0 100644
--- a/drivers/led/led-uclass.c
+++ b/drivers/led/led-uclass.c
@@ -32,6 +32,59 @@ int led_get_by_label(const char *label, struct udevice **devp)
 	return -ENODEV;
 }
 
+int led_count(void)
+{
+	int cnt = 0;
+	struct udevice *dev;
+	struct uclass *uc;
+	int ret;
+
+	ret = uclass_get(UCLASS_LED, &uc);
+	if (ret)
+		return ret;
+
+	uclass_foreach_dev(dev, uc) {
+		struct led_uclass_plat *uc_plat = dev_get_uclass_platdata(dev);
+
+		/* Ignore the top-level LED node */
+		if (uc_plat->label)
+			cnt++;
+	}
+
+	return cnt;
+}
+
+int led_get_all(struct udevice **devarr, int maxret)
+{
+	int cnt = 0;
+	struct udevice *dev;
+	struct uclass *uc;
+	int ret;
+
+	ret = uclass_get(UCLASS_LED, &uc);
+	if (ret)
+		return ret;
+
+	if(maxret <= cnt)
+		return cnt;
+
+	uclass_foreach_dev(dev, uc) {
+		struct led_uclass_plat *uc_plat = dev_get_uclass_platdata(dev);
+
+		/* Ignore the top-level LED node */
+		if (uc_plat->label) {
+			ret = uclass_get_device_tail(dev, 0, &devarr[cnt]);
+			if(ret)
+				return ret;
+			cnt++;
+			if(cnt >= maxret)
+				return cnt;
+		}
+	}
+
+	return cnt;
+}
+
 const char *led_get_label(struct udevice *dev)
 {
 	struct led_uclass_plat *uc_plat = dev_get_uclass_platdata(dev);
diff --git a/include/led.h b/include/led.h
index 32e0dec..4b1bd56 100644
--- a/include/led.h
+++ b/include/led.h
@@ -48,6 +48,24 @@ int led_get_by_label(const char *label, struct udevice **devp);
 const char *led_get_label(struct udevice *dev);
 
 /**
+ * led_count() - retrieve the number of DM-configured LEDs
+ *
+ * @return the number of leds found
+ */
+int led_count(void);
+
+/**
+ * led_get_all() - retrieve an array of DM-configured LEDs
+ *
+ * Should return at most led_count() leds.
+ *
+ * @devarr:	pointer to an array of led-device, to be filled.
+ * @maxitm:	pre-allocated size of the array
+ * @return number of leds filled into @devarr
+ */
+int led_get_all(struct udevice **devarr, int maxitm);
+
+/**
  * led_set_on() - set the state of an LED
  *
  * @dev:	LED device to change

[U-Boot] [PATCH v2 18/22] cmd: LED: be aware of DTS-configured leds

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

Make the LED command aware of device-tree configured LEDs, using the new
LED API calls.
---
 cmd/led.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/cmd/led.c b/cmd/led.c
index 5295a6e..7632074 100644
--- a/cmd/led.c
+++ b/cmd/led.c
@@ -186,6 +186,54 @@ LED_TBL_STATUS(5, STATUS_LED_BIT5);
 #define LED_TEST_5(name)	while(0)
 #endif
 
+#ifdef CONFIG_LED
+#include <led.h>
+
+static void _led_dm_onoff(const led_tbl_t *led, enum led_cmd cmd, char *arg)
+{
+	struct udevice *dev = NULL;
+	if(led_get_by_label(led->string, &dev))
+		return;
+	if(dev)
+		led_set_on(dev, cmd == LED_ON);
+}
+
+// this isn't const, as the string will be replaced by the current led's label, always.
+static led_tbl_t _led_dm = {
+	.op = {
+		[LED_ON] = _led_dm_onoff,
+		[LED_OFF] = _led_dm_onoff,
+		[LED_LIST] = _led_list_name,
+	},
+};
+
+static const led_tbl_t *_led_dm_get(const char *name) {
+	struct udevice *dev = NULL;
+	if(led_get_by_label(name, &dev))
+		return NULL;
+	if(!dev)
+		return NULL;
+	_led_dm.string = name;
+	return &_led_dm;
+}
+
+#define LED_DM_COUNT()	led_count()
+static int led_dm_list(const char **tbl, int size) {
+	struct udevice *devs[size];
+	int cnt = led_get_all(devs, size);
+	int i;
+	for(i = 0; i < cnt; i++)
+		tbl[i] = led_get_label(devs[i]);
+	return cnt;
+}
+#define LED_TEST_DM(name) return _led_dm_get(name)
+
+#else
+#define LED_TEST_DM(name)	return NULL
+#define LED_DM_COUNT()		0
+#define led_dm_list(tbl, size)	0
+#endif
+
 static int _led_count(void) {
 	int i = 0;
 	if(LED_NAME_GREEN) i++;
@@ -198,6 +246,7 @@ static int _led_count(void) {
 	if(LED_NAME_3) i++;
 	if(LED_NAME_4) i++;
 	if(LED_NAME_5) i++;
+	i += LED_DM_COUNT();
 	return i;
 }
 
@@ -214,6 +263,7 @@ static int _led_list(const char **tbl, int size) {
 	TEST_ADD_NAME(LED_NAME_3, tbl, size);
 	TEST_ADD_NAME(LED_NAME_4, tbl, size);
 	TEST_ADD_NAME(LED_NAME_5, tbl, size);
+	size -= led_dm_list(tbl, size);
 	return init_size - size;
 }
 
@@ -251,7 +301,8 @@ static const led_tbl_t *get_led(const char *name) {
 	LED_TEST_3(name);
 	LED_TEST_4(name);
 	LED_TEST_5(name);
-	return NULL;
+	// must be last, returns always
+	LED_TEST_DM(name);
 }
 
 enum led_cmd get_led_cmd(char *var)

[U-Boot] [PATCH v2 19/22] LED: provide functionality to get led status

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

This extends the LED-API for retrieving the current state a LED is in.
This needs corresponding support by the underlying device, but provides a way to check, wether an LED is on or off.
---
 drivers/led/led-uclass.c |   10 ++++++++++
 include/led.h            |   18 ++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c
index ce519d0..7d7dec3 100644
--- a/drivers/led/led-uclass.c
+++ b/drivers/led/led-uclass.c
@@ -101,6 +101,16 @@ int led_set_on(struct udevice *dev, int on)
 	return ops->set_on(dev, on);
 }
 
+int led_get_on(struct udevice *dev)
+{
+	struct led_ops *ops = led_get_ops(dev);
+
+	if (!ops->get_on)
+		return -ENOSYS;
+
+	return ops->get_on(dev);
+}
+
 UCLASS_DRIVER(led) = {
 	.id		= UCLASS_LED,
 	.name		= "led",
diff --git a/include/led.h b/include/led.h
index 4b1bd56..4709013 100644
--- a/include/led.h
+++ b/include/led.h
@@ -26,6 +26,16 @@ struct led_ops {
 	 * @return 0 if OK, -ve on error
 	 */
 	int (*set_on)(struct udevice *dev, int on);
+
+	/**
+	 * get_on() - get the current state of an LED
+	 *
+	 * function is optional.
+	 *
+	 * @dev:	LED device to check
+	 * @return 0 if OFF, 1 if ON, -ve on error
+	 */
+	int (*get_on)(struct udevice *dev);
 };
 
 #define led_get_ops(dev)	((struct led_ops *)(dev)->driver->ops)
@@ -74,4 +84,12 @@ int led_get_all(struct udevice **devarr, int maxitm);
  */
 int led_set_on(struct udevice *dev, int on);
 
+/**
+ * led_get_on() - get the current state of an LED
+ *
+ * @dev:	LED device to check
+ * @return 0 if OFF, 1 if ON, -ve on error
+ */
+int led_get_on(struct udevice *dev);
+
 #endif

[U-Boot] [PATCH v2 20/22] LED: GPIO: provide get_on() op

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

Add the code needed for retrieving the led-status to the generic led-gpio driver.
---
 drivers/led/led_gpio.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/led/led_gpio.c b/drivers/led/led_gpio.c
index cb6e996..80e0f72 100644
--- a/drivers/led/led_gpio.c
+++ b/drivers/led/led_gpio.c
@@ -28,6 +28,16 @@ static int gpio_led_set_on(struct udevice *dev, int on)
 	return dm_gpio_set_value(&priv->gpio, on);
 }
 
+static int gpio_led_get_on(struct udevice *dev)
+{
+	struct led_gpio_priv *priv = dev_get_priv(dev);
+
+	if (!dm_gpio_is_valid(&priv->gpio))
+		return -EREMOTEIO;
+
+	return dm_gpio_get_value(&priv->gpio);
+}
+
 static int led_gpio_probe(struct udevice *dev)
 {
 	struct led_uclass_plat *uc_plat = dev_get_uclass_platdata(dev);
@@ -88,6 +98,7 @@ static int led_gpio_bind(struct udevice *parent)
 
 static const struct led_ops gpio_led_ops = {
 	.set_on		= gpio_led_set_on,
+	.get_on		= gpio_led_get_on,
 };
 
 static const struct udevice_id led_gpio_ids[] = {

[U-Boot] [PATCH v2 21/22] LED: provide toggling interface

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

Extend the LED API to support toggling an LED. If the device provides a
direct way for this, it can implement the corresponding op.

If not supported directly by the driver, the led will be toggled by calling
led_set_on(!led_get_on()), transparently.
---
 drivers/led/led-uclass.c |   14 ++++++++++++++
 include/led.h            |   21 +++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c
index 7d7dec3..22d2264 100644
--- a/drivers/led/led-uclass.c
+++ b/drivers/led/led-uclass.c
@@ -111,6 +111,20 @@ int led_get_on(struct udevice *dev)
 	return ops->get_on(dev);
 }
 
+int led_toggle(struct udevice *dev)
+{
+	struct led_ops *ops = led_get_ops(dev);
+	int on;
+
+	if (ops->toggle)
+		return ops->toggle(dev);
+
+	on = led_get_on(dev);
+	if(on < 0)
+		return -ENOSYS;
+	return led_set_on(dev, !on);
+}
+
 UCLASS_DRIVER(led) = {
 	.id		= UCLASS_LED,
 	.name		= "led",
diff --git a/include/led.h b/include/led.h
index 4709013..f70db00 100644
--- a/include/led.h
+++ b/include/led.h
@@ -36,6 +36,17 @@ struct led_ops {
 	 * @return 0 if OFF, 1 if ON, -ve on error
 	 */
 	int (*get_on)(struct udevice *dev);
+
+	/**
+	 * toggle() - toggle the state of an LED
+	 *
+	 * will turn off the LED if it's on and vice versa.
+	 * Optional. If not given, a fallback using get_on/set_on is provided.
+	 *
+	 * @dev:	LED device to change
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*toggle)(struct udevice *dev);
 };
 
 #define led_get_ops(dev)	((struct led_ops *)(dev)->driver->ops)
@@ -92,4 +103,14 @@ int led_set_on(struct udevice *dev, int on);
  */
 int led_get_on(struct udevice *dev);
 
+/**
+ * led_toggle() - toggle the state of an LED
+ *
+ * will turn off the LED if it's on and vice versa.
+ *
+ * @dev:	LED device to change
+ * @return 0 if OK, -ve on error
+ */
+int led_toggle(struct udevice *dev);
+
 #endif

[U-Boot] [PATCH v2 22/22] Cmd: LED: make DM-leds toggle

[Benjamin Tietz]

From: Benjamin Tietz <benjamin@micronet24.de>

Support toggling of LEDs in the led command for device-tree configured LEDs.
---
 cmd/led.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/cmd/led.c b/cmd/led.c
index 7632074..217ec5a 100644
--- a/cmd/led.c
+++ b/cmd/led.c
@@ -198,11 +198,21 @@ static void _led_dm_onoff(const led_tbl_t *led, enum led_cmd cmd, char *arg)
 		led_set_on(dev, cmd == LED_ON);
 }
 
+static void _led_dm_toggle(const led_tbl_t *led, enum led_cmd cmd, char *arg)
+{
+	struct udevice *dev = NULL;
+	if(led_get_by_label(led->string, &dev))
+		return;
+	if(dev)
+		led_toggle(dev);
+}
+
 // this isn't const, as the string will be replaced by the current led's label, always.
 static led_tbl_t _led_dm = {
 	.op = {
 		[LED_ON] = _led_dm_onoff,
 		[LED_OFF] = _led_dm_onoff,
+		[LED_TOGGLE] = _led_dm_toggle,
 		[LED_LIST] = _led_list_name,
 	},
 };

[U-Boot] [PATCH 2 00/22] DM: Cmd: GPIO/LED/STM32/CLK: provide command-line support for device-tree configured gpios and leds

[Vikas MANOCHA]

Hi Benjamin,

Please keep all the involved developers in the "To" of the e-mail & resend the patchset for review comments (checkout scripts/get_maintainer.pl).
Also separate the generic stuff (dts/led) from platform specific in another patchset.

Cheers,
Vikas

> -----Original Message-----
> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Benjamin
> Tietz
> Sent: Monday, June 20, 2016 11:26 AM
> To: u-boot at lists.denx.de
> Subject: [U-Boot] [PATCH 2 00/22] DM: Cmd: GPIO/LED/STM32/CLK: provide
> command-line support for device-tree configured gpios and leds
> 
> This series begins to provide device-tree support on stm32 devices, starting
> with a the stack of a simple clock-driver (at least enabling/disabling
> peripheral clocks), the gpio driver and leds.
> 
> As the current led command-line interface isn't aware of any device-tree
> configured led, the command gets rewritten and extended for device-tree
> LEDs on the way. These changes are architecture indipendent.
> 
> To accomplish these changes the led-uclass driver had to be extended, too.
> 
> Changes in v2:
> * more verbose commit messages
> * correct clock calculation in stm32_clk
> * minor adjustments
> 
> 
> ---
> 
> Benjamin Tietz (22):
>       stm32: gpio: fix otype access
>       stm32: gpio_direction_output: make sure, output is set to push-pull
>       stm32: gpio_get_value: always return 0 or 1
>       stm32f429-discovery: config: enable status leds
>       Cmd: led: provide a selector in kconfig
>       DTS: stm32f429: provide device-tree files (from linux kernel)
>       clock-uclass: allow disabling a peripheral clock
>       STM32: clock: provide dts-accessible clock driver
>       DTS: STM32f429: add gpio-banks
>       STM32: gpio: group SOC-specific code to one ifdef/elif construct
>       GPIO: STM32: make DTS-aware
>       STM32F429-discovery: led: disable board-specific code, if DM is selected
>       GPIO/LED: make more robust, if STATUS_LED isn't selected
>       Cmd: LED: rewrite to prepare non-static access
>       DTS: STM32F429-disco: add board leds and enable rcc
>       LED: add function to retrieve a device's label
>       LED: provide function to count and get all (DM-)LEDs
>       cmd: LED: be aware of DTS-configured leds
>       LED: provide functionality to get led status
>       LED: GPIO: provide get_on() op
>       LED: provide toggling interface
>       Cmd: LED: make DM-leds toggle
> 
> 
>  arch/arm/dts/Makefile                 |    2
>  arch/arm/dts/armv7-m.dtsi             |   24 ++
>  arch/arm/dts/stm32429i-eval.dts       |   75 ++++++
>  arch/arm/dts/stm32f429-disco.dts      |   97 ++++++++
>  arch/arm/dts/stm32f429.dtsi           |  282 +++++++++++++++++++++++
>  board/st/stm32f429-discovery/Makefile |    3
>  cmd/Kconfig                           |    4
>  cmd/led.c                             |  401 ++++++++++++++++++++++++---------
>  drivers/clk/Kconfig                   |    4
>  drivers/clk/Makefile                  |    1
>  drivers/clk/clk-uclass.c              |   10 +
>  drivers/clk/clk_stm32.c               |  112 +++++++++
>  drivers/gpio/stm32_gpio.c             |  202 ++++++++++++++---
>  drivers/led/led-uclass.c              |   83 +++++++
>  drivers/led/led_gpio.c                |   11 +
>  drivers/misc/gpio_led.c               |    4
>  drivers/misc/status_led.c             |    2
>  include/clk.h                         |   18 +
>  include/configs/stm32f429-discovery.h |   14 +
>  include/led.h                         |   65 +++++
>  include/status_led.h                  |    4
>  21 files changed, 1277 insertions(+), 141 deletions(-)  create mode 100644
> arch/arm/dts/armv7-m.dtsi  create mode 100644 arch/arm/dts/stm32429i-
> eval.dts  create mode 100644 arch/arm/dts/stm32f429-disco.dts  create
> mode 100644 arch/arm/dts/stm32f429.dtsi  create mode 100644
> drivers/clk/clk_stm32.c
> 
> --
> 
> best regards
> Benjamin Tietz
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Simon Glass]

+Stephen

Hi Benjamin,

On 20 June 2016 at 12:26, Benjamin Tietz <uboot@dresden.micronet24.de> wrote:
> From: Benjamin Tietz <benjamin@micronet24.de>
>
> Currently, clocks can be enabled, only. To be feature-complete - and allow
> a bit of power-saving in applications - disabling a clock should be
> possible, too.
>
> This extend the API of the DM clock driver to allow a clock to be disabled.
>
> The corresponding operation is optional for not breaking existing drivers.

What does this mean? I can't see how it is optional.

Also please can you rebase to mainline and resend, as the API has
changed (sorry).

> ---
>  drivers/clk/clk-uclass.c |   10 ++++++++++
>  include/clk.h            |   18 ++++++++++++++++++
>  2 files changed, 28 insertions(+)
>
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index b483c1e..462f5f8 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -44,6 +44,16 @@ int clk_enable(struct udevice *dev, int periph)
>         return ops->enable(dev, periph);
>  }
>
> +int clk_disable(struct udevice *dev, int periph)
> +{
> +       struct clk_ops *ops = clk_get_ops(dev);
> +
> +       if (!ops->disable)
> +               return -ENOSYS;
> +
> +       return ops->disable(dev, periph);
> +}
> +
>  ulong clk_get_periph_rate(struct udevice *dev, int periph)
>  {
>         struct clk_ops *ops = clk_get_ops(dev);
> diff --git a/include/clk.h b/include/clk.h
> index ca20c3d..395f813 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -43,6 +43,15 @@ struct clk_ops {
>         int (*enable)(struct udevice *dev, int periph);
>
>         /**
> +        * disable() - Disable the clock for a peripheral
> +        *
> +        * @dev:        clock provider
> +        * @periph:     Peripheral ID to enable

disable

> +        * @return zero on success, or -ve error code
> +        */
> +       int (*disable)(struct udevice *dev, int periph);
> +
> +       /**
>          * get_periph_rate() - Get clock rate for a peripheral
>          *
>          * @dev:        Device to check (UCLASS_CLK)
> @@ -90,6 +99,15 @@ ulong clk_set_rate(struct udevice *dev, ulong rate);
>  int clk_enable(struct udevice *dev, int periph);
>
>  /**
> + * clk_disable() - Disable the clock for a peripheral
> + *
> + * @dev:       clock provider
> + * @periph:    Peripheral ID to enable
> + * @return zero on success, or -ve error code
> + */
> +int clk_disable(struct udevice *dev, int periph);
> +
> +/**
>   * clk_get_periph_rate() - Get current clock rate for a peripheral
>   *
>   * @dev:       Device to check (UCLASS_CLK)
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Stephen Warren]

On 07/12/2016 10:02 AM, Simon Glass wrote:
> +Stephen
>
> Hi Benjamin,
>
> On 20 June 2016 at 12:26, Benjamin Tietz <uboot@dresden.micronet24.de> wrote:
>> From: Benjamin Tietz <benjamin@micronet24.de>
>>
>> Currently, clocks can be enabled, only. To be feature-complete - and allow
>> a bit of power-saving in applications - disabling a clock should be
>> possible, too.
>>
>> This extend the API of the DM clock driver to allow a clock to be disabled.
>>
>> The corresponding operation is optional for not breaking existing drivers.
>
> What does this mean? I can't see how it is optional.
>
> Also please can you rebase to mainline and resend, as the API has
> changed (sorry).

include/clk.h already contains a clk_disable() function. I suspect you 
can just go ahead and implement it and ignore this patch?

[U-Boot] [PATCH 2 00/22] DM: Cmd: GPIO/LED/STM32/CLK: provide command-line support for device-tree configured gpios and leds

[Vikas MANOCHA]

Hi Benjamin,

Please let us know if you are working on this patchset.
I just saw another patchset but not using device tree & clock framework but would like to have DT and clock framework in place for stm32.

Cheers,
Vikas

> -----Original Message-----
> From: Vikas MANOCHA
> Sent: Friday, July 01, 2016 4:35 PM
> To: 'Benjamin Tietz' <uboot@dresden.micronet24.de>; u-
> boot at lists.denx.de
> Subject: RE: [U-Boot] [PATCH 2 00/22] DM: Cmd: GPIO/LED/STM32/CLK:
> provide command-line support for device-tree configured gpios and leds
> 
> Hi Benjamin,
> 
> Please keep all the involved developers in the "To" of the e-mail & resend
> the patchset for review comments (checkout scripts/get_maintainer.pl).
> Also separate the generic stuff (dts/led) from platform specific in another
> patchset.
> 
> Cheers,
> Vikas
> 
> > -----Original Message-----
> > From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of
> > Benjamin Tietz
> > Sent: Monday, June 20, 2016 11:26 AM
> > To: u-boot at lists.denx.de
> > Subject: [U-Boot] [PATCH 2 00/22] DM: Cmd: GPIO/LED/STM32/CLK:
> provide
> > command-line support for device-tree configured gpios and leds
> >
> > This series begins to provide device-tree support on stm32 devices,
> > starting with a the stack of a simple clock-driver (at least
> > enabling/disabling peripheral clocks), the gpio driver and leds.
> >
> > As the current led command-line interface isn't aware of any
> > device-tree configured led, the command gets rewritten and extended
> > for device-tree LEDs on the way. These changes are architecture
> indipendent.
> >
> > To accomplish these changes the led-uclass driver had to be extended, too.
> >
> > Changes in v2:
> > * more verbose commit messages
> > * correct clock calculation in stm32_clk
> > * minor adjustments
> >
> >
> > ---
> >
> > Benjamin Tietz (22):
> >       stm32: gpio: fix otype access
> >       stm32: gpio_direction_output: make sure, output is set to push-pull
> >       stm32: gpio_get_value: always return 0 or 1
> >       stm32f429-discovery: config: enable status leds
> >       Cmd: led: provide a selector in kconfig
> >       DTS: stm32f429: provide device-tree files (from linux kernel)
> >       clock-uclass: allow disabling a peripheral clock
> >       STM32: clock: provide dts-accessible clock driver
> >       DTS: STM32f429: add gpio-banks
> >       STM32: gpio: group SOC-specific code to one ifdef/elif construct
> >       GPIO: STM32: make DTS-aware
> >       STM32F429-discovery: led: disable board-specific code, if DM is
> selected
> >       GPIO/LED: make more robust, if STATUS_LED isn't selected
> >       Cmd: LED: rewrite to prepare non-static access
> >       DTS: STM32F429-disco: add board leds and enable rcc
> >       LED: add function to retrieve a device's label
> >       LED: provide function to count and get all (DM-)LEDs
> >       cmd: LED: be aware of DTS-configured leds
> >       LED: provide functionality to get led status
> >       LED: GPIO: provide get_on() op
> >       LED: provide toggling interface
> >       Cmd: LED: make DM-leds toggle
> >
> >
> >  arch/arm/dts/Makefile                 |    2
> >  arch/arm/dts/armv7-m.dtsi             |   24 ++
> >  arch/arm/dts/stm32429i-eval.dts       |   75 ++++++
> >  arch/arm/dts/stm32f429-disco.dts      |   97 ++++++++
> >  arch/arm/dts/stm32f429.dtsi           |  282 +++++++++++++++++++++++
> >  board/st/stm32f429-discovery/Makefile |    3
> >  cmd/Kconfig                           |    4
> >  cmd/led.c                             |  401 ++++++++++++++++++++++++---------
> >  drivers/clk/Kconfig                   |    4
> >  drivers/clk/Makefile                  |    1
> >  drivers/clk/clk-uclass.c              |   10 +
> >  drivers/clk/clk_stm32.c               |  112 +++++++++
> >  drivers/gpio/stm32_gpio.c             |  202 ++++++++++++++---
> >  drivers/led/led-uclass.c              |   83 +++++++
> >  drivers/led/led_gpio.c                |   11 +
> >  drivers/misc/gpio_led.c               |    4
> >  drivers/misc/status_led.c             |    2
> >  include/clk.h                         |   18 +
> >  include/configs/stm32f429-discovery.h |   14 +
> >  include/led.h                         |   65 +++++
> >  include/status_led.h                  |    4
> >  21 files changed, 1277 insertions(+), 141 deletions(-)  create mode
> > 100644 arch/arm/dts/armv7-m.dtsi  create mode 100644
> > arch/arm/dts/stm32429i- eval.dts  create mode 100644
> > arch/arm/dts/stm32f429-disco.dts  create mode 100644
> > arch/arm/dts/stm32f429.dtsi  create mode 100644
> > drivers/clk/clk_stm32.c
> >
> > --
> >
> > best regards
> > Benjamin Tietz
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2 00/22] DM: Cmd: GPIO/LED/STM32/CLK: provide command-line support for device-tree configured gpios and leds

[Benjamin Tietz]

Hi Vikas,

I was an holiday and am still catching up. Apart from that I'm still
working on this patchset and integrating the comments and API-changes.

What I'm currently missing is to setup the frequencies directly from
clk_stm32.c. When this is done, I'll send an update.

regards
Benjamin

On Thu, Jul 21, 2016 at 12:32:22AM +0200, Vikas MANOCHA wrote:
> Hi Benjamin,
> 
> Please let us know if you are working on this patchset.
> I just saw another patchset but not using device tree & clock framework but would like to have DT and clock framework in place for stm32.
> 
> Cheers,
> Vikas
> 
> > -----Original Message-----
> > From: Vikas MANOCHA
> > Sent: Friday, July 01, 2016 4:35 PM
> > To: 'Benjamin Tietz' <uboot@dresden.micronet24.de>; u-
> > boot at lists.denx.de
> > Subject: RE: [U-Boot] [PATCH 2 00/22] DM: Cmd: GPIO/LED/STM32/CLK:
> > provide command-line support for device-tree configured gpios and leds
> > 
> > Hi Benjamin,
> > 
> > Please keep all the involved developers in the "To" of the e-mail & resend
> > the patchset for review comments (checkout scripts/get_maintainer.pl).
> > Also separate the generic stuff (dts/led) from platform specific in another
> > patchset.
> > 
> > Cheers,
> > Vikas
> > 
> > > -----Original Message-----
> > > From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of
> > > Benjamin Tietz
> > > Sent: Monday, June 20, 2016 11:26 AM
> > > To: u-boot at lists.denx.de
> > > Subject: [U-Boot] [PATCH 2 00/22] DM: Cmd: GPIO/LED/STM32/CLK:
> > provide
> > > command-line support for device-tree configured gpios and leds
> > >
> > > This series begins to provide device-tree support on stm32 devices,
> > > starting with a the stack of a simple clock-driver (at least
> > > enabling/disabling peripheral clocks), the gpio driver and leds.
> > >
> > > As the current led command-line interface isn't aware of any
> > > device-tree configured led, the command gets rewritten and extended
> > > for device-tree LEDs on the way. These changes are architecture
> > indipendent.
> > >
> > > To accomplish these changes the led-uclass driver had to be extended, too.
> > >
> > > Changes in v2:
> > > * more verbose commit messages
> > > * correct clock calculation in stm32_clk
> > > * minor adjustments
> > >
> > >
> > > ---
> > >
> > > Benjamin Tietz (22):
> > >       stm32: gpio: fix otype access
> > >       stm32: gpio_direction_output: make sure, output is set to push-pull
> > >       stm32: gpio_get_value: always return 0 or 1
> > >       stm32f429-discovery: config: enable status leds
> > >       Cmd: led: provide a selector in kconfig
> > >       DTS: stm32f429: provide device-tree files (from linux kernel)
> > >       clock-uclass: allow disabling a peripheral clock
> > >       STM32: clock: provide dts-accessible clock driver
> > >       DTS: STM32f429: add gpio-banks
> > >       STM32: gpio: group SOC-specific code to one ifdef/elif construct
> > >       GPIO: STM32: make DTS-aware
> > >       STM32F429-discovery: led: disable board-specific code, if DM is
> > selected
> > >       GPIO/LED: make more robust, if STATUS_LED isn't selected
> > >       Cmd: LED: rewrite to prepare non-static access
> > >       DTS: STM32F429-disco: add board leds and enable rcc
> > >       LED: add function to retrieve a device's label
> > >       LED: provide function to count and get all (DM-)LEDs
> > >       cmd: LED: be aware of DTS-configured leds
> > >       LED: provide functionality to get led status
> > >       LED: GPIO: provide get_on() op
> > >       LED: provide toggling interface
> > >       Cmd: LED: make DM-leds toggle
> > >
> > >
> > >  arch/arm/dts/Makefile                 |    2
> > >  arch/arm/dts/armv7-m.dtsi             |   24 ++
> > >  arch/arm/dts/stm32429i-eval.dts       |   75 ++++++
> > >  arch/arm/dts/stm32f429-disco.dts      |   97 ++++++++
> > >  arch/arm/dts/stm32f429.dtsi           |  282 +++++++++++++++++++++++
> > >  board/st/stm32f429-discovery/Makefile |    3
> > >  cmd/Kconfig                           |    4
> > >  cmd/led.c                             |  401 ++++++++++++++++++++++++---------
> > >  drivers/clk/Kconfig                   |    4
> > >  drivers/clk/Makefile                  |    1
> > >  drivers/clk/clk-uclass.c              |   10 +
> > >  drivers/clk/clk_stm32.c               |  112 +++++++++
> > >  drivers/gpio/stm32_gpio.c             |  202 ++++++++++++++---
> > >  drivers/led/led-uclass.c              |   83 +++++++
> > >  drivers/led/led_gpio.c                |   11 +
> > >  drivers/misc/gpio_led.c               |    4
> > >  drivers/misc/status_led.c             |    2
> > >  include/clk.h                         |   18 +
> > >  include/configs/stm32f429-discovery.h |   14 +
> > >  include/led.h                         |   65 +++++
> > >  include/status_led.h                  |    4
> > >  21 files changed, 1277 insertions(+), 141 deletions(-)  create mode
> > > 100644 arch/arm/dts/armv7-m.dtsi  create mode 100644
> > > arch/arm/dts/stm32429i- eval.dts  create mode 100644
> > > arch/arm/dts/stm32f429-disco.dts  create mode 100644
> > > arch/arm/dts/stm32f429.dtsi  create mode 100644
> > > drivers/clk/clk_stm32.c
> > >
> > > --
> > >
> > > best regards
> > > Benjamin Tietz
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > > http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Benjamin Tietz]

Hi Simon,

On Tue, Jul 12, 2016 at 10:02:43AM -0600, Simon Glass wrote:
> +Stephen
> 
> Hi Benjamin,
> 
> On 20 June 2016 at 12:26, Benjamin Tietz <uboot@dresden.micronet24.de> wrote:
> > From: Benjamin Tietz <benjamin@micronet24.de>
> >
> > Currently, clocks can be enabled, only. To be feature-complete - and allow
> > a bit of power-saving in applications - disabling a clock should be
> > possible, too.
> >
> > This extend the API of the DM clock driver to allow a clock to be disabled.
> >
> > The corresponding operation is optional for not breaking existing drivers.
> 
> What does this mean? I can't see how it is optional.

What was meant was:
If a driver can't handling disabling of peripherals, the request will be
ignored silently - I think, the expected behaviour.

> 
> Also please can you rebase to mainline and resend, as the API has
> changed (sorry).
... including a clk_disable.
So at least this patch can be dropped.

regards
Benjamin

[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Benjamin Tietz]

Hello Vikas, hello Simon,

the new clk-API leaves me with a problem. Previously there was a
seperate way to access the clock-device itself (using clk_[gs]et_rate) and
the peripherals connected (clk_[gs]et_periph_rate). The former case now isn't
available no more. But the system clock in STM32 doesn't have a separate ID.
According to the device-tree the kernel doesn't care about that - or
uses special logic.

Possible solutions:

a) Using a magic ID. Unfortunately, the peripheral used in the current
device-tree are 0-based (and 0 is already in use), so this number isn't
available. Moving the IDs around would break compatibility to the linux
kernel.

Negative numbers look like errors.

Using a special high number looks unintuitive. And often result in
additional work-arounds in the future.

b) moving the sysclock and PLL-stuff in a seperate driver. That driver
should be found in the device-tree, too.
The device-tree should represent the hardware. Because of that, the
source-clock of the STM32 RCC device (the clocking subsystem), should
be either the external quartz or one of the internal sources, not a
pll-device. Apart from that, the kernel in its current configuration
probably is using this information to compute is current clock-speed.

c) extending the struct clk, which looks clumsy, too.

Any ideas?

regards
Benjamin

[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Benjamin Tietz]

Hello Vikas, hello Simon,

sleeping a night over this issue, I've came along with the following
concept:

The peripheral clocks are grouped by peripheral bus, that they are
connected to. Each bus is driven by one or more set of registers,
allowing up to 32 clocks to be selected. Having more than 8 registers
would require massive modifications anyway. So it can be expected to
have at most 256 clocks.

As these domains partly have special clock-domains, partly rely on their
bus-clock, I would like to interprete the id as a bitfield. At least the
least significant byte represents a simple counter over the clocks -
with all other bits set to zero representing real clocks.

Setting the LSB of the third byte (using the mask 1<<16) might represent
the "master clocks" - starting with the individual port clocks continued
by the specialized clock domains.

Using 1<<17 than could represent the individual PLLs directly.

regards
Benjamin

On Thu, Jul 28, 2016@09:28:09PM +0200, Benjamin Tietz wrote:
> Hello Vikas, hello Simon,
> 
> the new clk-API leaves me with a problem. Previously there was a
> seperate way to access the clock-device itself (using clk_[gs]et_rate) and
> the peripherals connected (clk_[gs]et_periph_rate). The former case now isn't
> available no more. But the system clock in STM32 doesn't have a separate ID.
> According to the device-tree the kernel doesn't care about that - or
> uses special logic.
> 
> Possible solutions:
> 
> a) Using a magic ID. Unfortunately, the peripheral used in the current
> device-tree are 0-based (and 0 is already in use), so this number isn't
> available. Moving the IDs around would break compatibility to the linux
> kernel.
> 
> Negative numbers look like errors.
> 
> Using a special high number looks unintuitive. And often result in
> additional work-arounds in the future.
> 
> b) moving the sysclock and PLL-stuff in a seperate driver. That driver
> should be found in the device-tree, too.
> The device-tree should represent the hardware. Because of that, the
> source-clock of the STM32 RCC device (the clocking subsystem), should
> be either the external quartz or one of the internal sources, not a
> pll-device. Apart from that, the kernel in its current configuration
> probably is using this information to compute is current clock-speed.
> 
> c) extending the struct clk, which looks clumsy, too.
> 
> Any ideas?
> 
> regards
> Benjamin

[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Stephen Warren]

On 07/28/2016 01:28 PM, Benjamin Tietz wrote:
> Hello Vikas, hello Simon,
>
> the new clk-API leaves me with a problem. Previously there was a
> seperate way to access the clock-device itself (using clk_[gs]et_rate) and
> the peripherals connected (clk_[gs]et_periph_rate). The former case now isn't
> available no more. But the system clock in STM32 doesn't have a separate ID.
> According to the device-tree the kernel doesn't care about that - or
> uses special logic.

I don't understand the issue here. The device tree model is that every 
clock has some provider (a node/phandle) and some ID (a sequence of 
integer values). There's no such thing as "the clock-device itself".

The kernel is consistent with that model; each client executes 
clk_get(), which for a DT-based system parses the 
phandle+clock_specifier and returns a clock handle, and then the client 
just uses that handle. That's exactly how U-Boot works too.

Perhaps you can show the portion of DT that's causing the issue?

Is the issue that there are clocks that your code needs to use that 
haven't yet been assigned a clock ID (clock specifier value) for use in 
DT (i.e. you have SoC-specific code that's calling clk_get() and the 
mapping isn't via DT)? If so, you simply need to assign one and 
everything will work fine. High numbers work fine for this.

> Possible solutions:
>
> a) Using a magic ID. Unfortunately, the peripheral used in the current
> device-tree are 0-based (and 0 is already in use), so this number isn't
> available. Moving the IDs around would break compatibility to the linux
> kernel.
>
> Negative numbers look like errors.
>
> Using a special high number looks unintuitive. And often result in
> additional work-arounds in the future.

What specific issues are you thinking of? I haven't experienced any when 
assigning IDs on Tegra, and I haven't observed anyone having issues 
doing this on any platform within the Linux kernel, where the exact same 
thing would be required.

[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Benjamin Tietz]

Hello Stephen,

On Fri, Jul 29, 2016 at 10:04:56AM -0600, Stephen Warren wrote:
> On 07/28/2016 01:28 PM, Benjamin Tietz wrote:
> >Hello Vikas, hello Simon,
> >
> >the new clk-API leaves me with a problem. Previously there was a
> >seperate way to access the clock-device itself (using clk_[gs]et_rate) and
> >the peripherals connected (clk_[gs]et_periph_rate). The former case now isn't
> >available no more. But the system clock in STM32 doesn't have a separate ID.
> >According to the device-tree the kernel doesn't care about that - or
> >uses special logic.
> 
> I don't understand the issue here. The device tree model is that every clock
> has some provider (a node/phandle) and some ID (a sequence of integer
> values). There's no such thing as "the clock-device itself".

For the current STM32 DT exactly that is the problem. The phandle is
available and the set of IDs is defined. There is - at least at the
moment - no ID defined for the system clock itself, but only for the
derived clocks for the individual peripherals.

The existing peripherals IDs even start to count at 0, so the "intuitive"
solution to use that ID, doesn't work either.

> 
> The kernel is consistent with that model; each client executes clk_get(),
> which for a DT-based system parses the phandle+clock_specifier and returns a
> clock handle, and then the client just uses that handle. That's exactly how
> U-Boot works too.

I must admit, that I haven't had an in-depth look on the STM32 clocking
kernel sources yet. From other architectures I've seen, the system clock
is either accessed by a certain ID, based on the underlying hardware -
which isn't available on STM32 - or assumed to be "just there". On a
first glance, the kernel STM32 driver seems to fall into the second category.

> 
> Perhaps you can show the portion of DT that's causing the issue?

It is the not existing potion of the DT ;)

> 
> Is the issue that there are clocks that your code needs to use that haven't
> yet been assigned a clock ID (clock specifier value) for use in DT (i.e. you
> have SoC-specific code that's calling clk_get() and the mapping isn't via
> DT)? If so, you simply need to assign one and everything will work fine.
> High numbers work fine for this.
> 
> >Possible solutions:
> >
> >a) Using a magic ID. Unfortunately, the peripheral used in the current
> >device-tree are 0-based (and 0 is already in use), so this number isn't
> >available. Moving the IDs around would break compatibility to the linux
> >kernel.
> >
> >Negative numbers look like errors.
> >
> >Using a special high number looks unintuitive. And often result in
> >additional work-arounds in the future.
> 
> What specific issues are you thinking of? I haven't experienced any when
> assigning IDs on Tegra, and I haven't observed anyone having issues doing
> this on any platform within the Linux kernel, where the exact same thing
> would be required.

I'm no friend of magic numbers. Experience had shown, that - especially
those introduced as a work-around - generate complicate problems at a
later point of time.
The first thing I can think of is to run into a hardware, having more
peripheral clocks than expected. Nevertheless, in a follow-up mail I
made a suggestion interpreting part of the ID as a bit-field - with
enough room for bigger hardware.

regards
Benjamin

[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Stephen Warren]

On 07/29/2016 11:26 AM, Benjamin Tietz wrote:
> Hello Stephen,
>
> On Fri, Jul 29, 2016 at 10:04:56AM -0600, Stephen Warren wrote:
>> On 07/28/2016 01:28 PM, Benjamin Tietz wrote:
>>> Hello Vikas, hello Simon,
>>>
>>> the new clk-API leaves me with a problem. Previously there was a
>>> seperate way to access the clock-device itself (using clk_[gs]et_rate) and
>>> the peripherals connected (clk_[gs]et_periph_rate). The former case now isn't
>>> available no more. But the system clock in STM32 doesn't have a separate ID.
>>> According to the device-tree the kernel doesn't care about that - or
>>> uses special logic.
>>
>> I don't understand the issue here. The device tree model is that every clock
>> has some provider (a node/phandle) and some ID (a sequence of integer
>> values). There's no such thing as "the clock-device itself".
>
> For the current STM32 DT exactly that is the problem. The phandle is
> available and the set of IDs is defined. There is - at least at the
> moment - no ID defined for the system clock itself, but only for the
> derived clocks for the individual peripherals.
>
> The existing peripherals IDs even start to count at 0, so the "intuitive"
> solution to use that ID, doesn't work either.
>
>>
>> The kernel is consistent with that model; each client executes clk_get(),
>> which for a DT-based system parses the phandle+clock_specifier and returns a
>> clock handle, and then the client just uses that handle. That's exactly how
>> U-Boot works too.
>
> I must admit, that I haven't had an in-depth look on the STM32 clocking
> kernel sources yet. From other architectures I've seen, the system clock
> is either accessed by a certain ID, based on the underlying hardware -
> which isn't available on STM32 - or assumed to be "just there". On a
> first glance, the kernel STM32 driver seems to fall into the second category.
>
>>
>> Perhaps you can show the portion of DT that's causing the issue?
>
> It is the not existing potion of the DT ;)
>
>>
>> Is the issue that there are clocks that your code needs to use that haven't
>> yet been assigned a clock ID (clock specifier value) for use in DT (i.e. you
>> have SoC-specific code that's calling clk_get() and the mapping isn't via
>> DT)? If so, you simply need to assign one and everything will work fine.
>> High numbers work fine for this.
>>
>>> Possible solutions:
>>>
>>> a) Using a magic ID. Unfortunately, the peripheral used in the current
>>> device-tree are 0-based (and 0 is already in use), so this number isn't
>>> available. Moving the IDs around would break compatibility to the linux
>>> kernel.
>>>
>>> Negative numbers look like errors.
>>>
>>> Using a special high number looks unintuitive. And often result in
>>> additional work-arounds in the future.
>>
>> What specific issues are you thinking of? I haven't experienced any when
>> assigning IDs on Tegra, and I haven't observed anyone having issues doing
>> this on any platform within the Linux kernel, where the exact same thing
>> would be required.
>
> I'm no friend of magic numbers. Experience had shown, that - especially
> those introduced as a work-around - generate complicate problems at a
> later point of time.

Well, the numbers aren't magic. For DT to work in any way at all, the 
numbering used by DT properties must match the numbering implemented by 
the clock driver code. The only way to do that is to use a shared header 
files that defines what those IDs are. That header is included by both 
the DT compiler and the C compiler, so there's no chance of them getting 
out of sync. In the U-Boot source code, look at 
include/dt-bindings/clock/tegra20-car.h for example.

> The first thing I can think of is to run into a hardware, having more
> peripheral clocks than expected. Nevertheless, in a follow-up mail I
> made a suggestion interpreting part of the ID as a bit-field - with
> enough room for bigger hardware.

Every piece of different HW requires a different DT binding (or at least 
a different compatible value; compatible values that represent similar 
HW can share a binding definition file). Each compatible value defines a 
separate set of clock IDs, and hence there's potentially a separate 
header file for each HW (although of course where different HW just 
happens to use the same set of IDs, they can use the same header file to 
define them). As such, it's easy to assign different "virtual" IDs for 
each different HW. For example, there's a separate clock ID header for 
Tegra20, Tegra30, etc. That's because all of (a) there are more clocks 
on Tegra30 thus requiring some additional IDs to be defined for it (b) 
Tegra30 probably moved some clock IDs around in HW thus requiring some 
clock IDs to be changed (c) virtual IDs had to be changed to leave space 
for the new clocks.

Note that the clock IDs defined by the DT binding should match HW where 
it makes sense. For example on Tegra20, there are 96 clocks whose IDs 
correspond exactly to bit positions in a nice regular set of HW 
registers. However there are many clocks (mainly SoC clock inputs, PLLs, 
etc.) that aren't covered by that main register set, and hence have IDs 
assigned that are entirely arbitrary.

With DT, if you have any clocks that don't have an obvious HW-defined 
ID, you have no choice but to assign them some arbitrary/chosen ID, and 
have a mapping table that converts DT IDs into information about those 
clocks; their HW identity, their type (PLL, peripheral, ...), register 
address, ...

[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Benjamin Tietz]

Hello Stephen,

On Fri, Jul 29, 2016 at 12:02:02PM -0600, Stephen Warren wrote:
> On 07/29/2016 11:26 AM, Benjamin Tietz wrote:
> >Hello Stephen,

[snip]

> >>>Using a special high number looks unintuitive. And often result in
> >>>additional work-arounds in the future.
> >>
> >>What specific issues are you thinking of? I haven't experienced any when
> >>assigning IDs on Tegra, and I haven't observed anyone having issues doing
> >>this on any platform within the Linux kernel, where the exact same thing
> >>would be required.
> >
> >I'm no friend of magic numbers. Experience had shown, that - especially
> >those introduced as a work-around - generate complicate problems at a
> >later point of time.
> 
> Well, the numbers aren't magic. For DT to work in any way at all, the
> numbering used by DT properties must match the numbering implemented by the
> clock driver code. The only way to do that is to use a shared header files
> that defines what those IDs are. That header is included by both the DT
> compiler and the C compiler, so there's no chance of them getting out of
> sync. In the U-Boot source code, look at
> include/dt-bindings/clock/tegra20-car.h for example.

Just because they are defined in a single place doesn't make that
numbers less magic. They are choosen just for a specific software
component. And U-Boot isn't the only user of device-trees. See: Linux
Kernel.

Syncing files of magic values between different projects is a pain and
should be reduced to a bare minimum. As part of that, choosing those
values should be avoided. If they are choosen once, modification isn't
really possible anymore. If one must be choosen, this should be done
carefully.

> 
> >The first thing I can think of is to run into a hardware, having more
> >peripheral clocks than expected. Nevertheless, in a follow-up mail I
> >made a suggestion interpreting part of the ID as a bit-field - with
> >enough room for bigger hardware.
> 
> Every piece of different HW requires a different DT binding (or at least a
> different compatible value; compatible values that represent similar HW can
> share a binding definition file). Each compatible value defines a separate
> set of clock IDs, and hence there's potentially a separate header file for
> each HW (although of course where different HW just happens to use the same
> set of IDs, they can use the same header file to define them). As such, it's
> easy to assign different "virtual" IDs for each different HW. For example,
> there's a separate clock ID header for Tegra20, Tegra30, etc. That's because
> all of (a) there are more clocks on Tegra30 thus requiring some additional
> IDs to be defined for it (b) Tegra30 probably moved some clock IDs around in
> HW thus requiring some clock IDs to be changed (c) virtual IDs had to be
> changed to leave space for the new clocks.
> 
> Note that the clock IDs defined by the DT binding should match HW where it
> makes sense. For example on Tegra20, there are 96 clocks whose IDs
> correspond exactly to bit positions in a nice regular set of HW registers.
> However there are many clocks (mainly SoC clock inputs, PLLs, etc.) that
> aren't covered by that main register set, and hence have IDs assigned that
> are entirely arbitrary.
> 
> With DT, if you have any clocks that don't have an obvious HW-defined ID,
> you have no choice but to assign them some arbitrary/chosen ID, and have a
> mapping table that converts DT IDs into information about those clocks;
> their HW identity, their type (PLL, peripheral, ...), register address, ...

The device-tree was introduced as an additional abstraction layer
between the hardware in use and the software running on this hardware,
esp. the bootloader and the OS kernel. One goal was to have a single
firmware running on different boards, based on the device-tree flashed
on that board. This should improve the ways to reuse code written once.

If the hardware changes there must be some way to represent this change
in software - either by having a new driver ( involving dubled code ),
or by a compatibility layer ( adding additional complexity ) or
sometimes by a reasonable decision on the design or some simple IDs in
the beginning. For the stm32-clk driver the currently implemented logic
does need some kind of decision. Please don't interprete this as
criticism on your work, but as I am, as everyone else, not perfect
and there is no obvious solution I wrote down my complaints, asking for
some comment or hint.

To discuss a solution and not a problem, I would prefer to take my mail
from this morning at 07:22 as base.
In any case, have a nice weekend.

regards
Benjamin

[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Simon Glass]

Hi Benjamin,

On 29 July 2016 at 12:34, Benjamin Tietz <uboot@dresden.micronet24.de> wrote:
> Hello Stephen,
>
> On Fri, Jul 29, 2016 at 12:02:02PM -0600, Stephen Warren wrote:
>> On 07/29/2016 11:26 AM, Benjamin Tietz wrote:
>> >Hello Stephen,
>
> [snip]
>
>> >>>Using a special high number looks unintuitive. And often result in
>> >>>additional work-arounds in the future.
>> >>
>> >>What specific issues are you thinking of? I haven't experienced any when
>> >>assigning IDs on Tegra, and I haven't observed anyone having issues doing
>> >>this on any platform within the Linux kernel, where the exact same thing
>> >>would be required.
>> >
>> >I'm no friend of magic numbers. Experience had shown, that - especially
>> >those introduced as a work-around - generate complicate problems at a
>> >later point of time.
>>
>> Well, the numbers aren't magic. For DT to work in any way at all, the
>> numbering used by DT properties must match the numbering implemented by the
>> clock driver code. The only way to do that is to use a shared header files
>> that defines what those IDs are. That header is included by both the DT
>> compiler and the C compiler, so there's no chance of them getting out of
>> sync. In the U-Boot source code, look at
>> include/dt-bindings/clock/tegra20-car.h for example.
>
> Just because they are defined in a single place doesn't make that
> numbers less magic. They are choosen just for a specific software
> component. And U-Boot isn't the only user of device-trees. See: Linux
> Kernel.
>
> Syncing files of magic values between different projects is a pain and
> should be reduced to a bare minimum. As part of that, choosing those
> values should be avoided. If they are choosen once, modification isn't
> really possible anymore. If one must be choosen, this should be done
> carefully.

Note though that the numbers are part of the device-tree binding for
an SoC. They are actually fixed in value. So the shared header file
(between C code and DT) is pretty-much essential. This is the way it
works at present.

>
>>
>> >The first thing I can think of is to run into a hardware, having more
>> >peripheral clocks than expected. Nevertheless, in a follow-up mail I
>> >made a suggestion interpreting part of the ID as a bit-field - with
>> >enough room for bigger hardware.
>>
>> Every piece of different HW requires a different DT binding (or at least a
>> different compatible value; compatible values that represent similar HW can
>> share a binding definition file). Each compatible value defines a separate
>> set of clock IDs, and hence there's potentially a separate header file for
>> each HW (although of course where different HW just happens to use the same
>> set of IDs, they can use the same header file to define them). As such, it's
>> easy to assign different "virtual" IDs for each different HW. For example,
>> there's a separate clock ID header for Tegra20, Tegra30, etc. That's because
>> all of (a) there are more clocks on Tegra30 thus requiring some additional
>> IDs to be defined for it (b) Tegra30 probably moved some clock IDs around in
>> HW thus requiring some clock IDs to be changed (c) virtual IDs had to be
>> changed to leave space for the new clocks.
>>
>> Note that the clock IDs defined by the DT binding should match HW where it
>> makes sense. For example on Tegra20, there are 96 clocks whose IDs
>> correspond exactly to bit positions in a nice regular set of HW registers.
>> However there are many clocks (mainly SoC clock inputs, PLLs, etc.) that
>> aren't covered by that main register set, and hence have IDs assigned that
>> are entirely arbitrary.
>>
>> With DT, if you have any clocks that don't have an obvious HW-defined ID,
>> you have no choice but to assign them some arbitrary/chosen ID, and have a
>> mapping table that converts DT IDs into information about those clocks;
>> their HW identity, their type (PLL, peripheral, ...), register address, ...
>
> The device-tree was introduced as an additional abstraction layer
> between the hardware in use and the software running on this hardware,
> esp. the bootloader and the OS kernel. One goal was to have a single
> firmware running on different boards, based on the device-tree flashed
> on that board. This should improve the ways to reuse code written once.
>
> If the hardware changes there must be some way to represent this change
> in software - either by having a new driver ( involving dubled code ),
> or by a compatibility layer ( adding additional complexity ) or
> sometimes by a reasonable decision on the design or some simple IDs in
> the beginning. For the stm32-clk driver the currently implemented logic
> does need some kind of decision. Please don't interprete this as
> criticism on your work, but as I am, as everyone else, not perfect
> and there is no obvious solution I wrote down my complaints, asking for
> some comment or hint.
>
> To discuss a solution and not a problem, I would prefer to take my mail
> from this morning at 07:22 as base.
> In any case, have a nice weekend.

As to your problem, can you not have separate clock drivers? For the
ones that don't have multiple clocks, they can ignore the id. For the
others, they can use the ID from the device tree.

Regards,
Simon