[PATCH v2 1/2] console: provide best-effort clk_get_for_console helper

[PATCH v2 1/2] console: provide best-effort clk_get_for_console helper

[Ahmad Fatoum]

From: Ahmad Fatoum <ahmad@a3f.at>

clk_get will return -EPROBE_DEFER if clock provider hasn't yet been
probed. In a system with deep probe enabled, dependencies are probed
on demand, so a -EPROBE_DEFER return is final and the console probe
will never succeed.

CONFIG_DEBUG_LL is often used to debug this, but because the low-level
console is not interactive, it's a bit cumbersome. Improve upon this a
bit, by providing a clk_get_for_console helper that returns NULL if and
only if we are sure that a clock provider will not be probed.

In that case, the driver can skip code paths initialization code and
baud rate setting dependent on having access to the clock and still
register a console that reuses what was set up by CONFIG_DEBUG_LL.

Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
v1 -> v2:
  - restrict new mechanism to stdout console (Sascha)
---
 include/console.h   | 27 +++++++++++++++++++++++++++
 include/linux/clk.h | 21 +++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/include/console.h b/include/console.h
index d7cbdfa99207..69c0ec144b31 100644
--- a/include/console.h
+++ b/include/console.h
@@ -8,6 +8,7 @@
 #define _CONSOLE_H_
 
 #include <param.h>
+#include <linux/clk.h>
 #include <linux/list.h>
 #include <driver.h>
 #include <serdev.h>
@@ -228,4 +229,30 @@ static inline void console_ctrlc_allow(void) { }
 static inline void console_ctrlc_forbid(void) { }
 #endif
 
+/**
+ * clk_get_for_console - get clock, ignoring known unavailable clock controller
+ * @dev: device for clock "consumer"
+ * @id: clock consumer ID
+ *
+ * Return: a struct clk corresponding to the clock producer, a
+ * valid IS_ERR() condition containing errno or NULL if it could
+ * be determined that the clock producer will never be probed in
+ * absence of modules. The NULL return allows serial drivers to
+ * skip clock handling for the stdout console and rely on CONFIG_DEBUG_LL.
+ */
+static inline struct clk *clk_get_for_console(struct device *dev, const char *id)
+{
+	__always_unused unsigned baudrate;
+	struct clk *clk;
+
+	if (!IS_ENABLED(CONFIG_DEBUG_LL) || !of_device_is_stdout_path(dev, &baudrate))
+		return clk_get(dev, id);
+
+	clk = clk_get_if_available(dev, id);
+	if (clk == NULL)
+		dev_warn(dev, "couldn't get clock (ignoring)\n");
+
+	return clk;
+}
+
 #endif
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 61b4ff3127cf..f505f18a75c0 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -14,6 +14,7 @@
 #include <linux/spinlock.h>
 #include <linux/stringify.h>
 #include <linux/container_of.h>
+#include <deep-probe.h>
 #include <xfuncs.h>
 
 struct device;
@@ -982,4 +983,24 @@ static inline struct clk *clk_get_enabled(struct device *dev, const char *id)
 	return clk;
 }
 
+/**
+ * clk_get_if_available - get clock, ignoring known unavailable clock controller
+ * @dev: device for clock "consumer"
+ * @id: clock consumer ID
+ *
+ * Return: a struct clk corresponding to the clock producer, a
+ * valid IS_ERR() condition containing errno or NULL if it could
+ * be determined that the clock producer will never be probed in
+ * absence of modules.
+ */
+static inline struct clk *clk_get_if_available(struct device *dev, const char *id)
+{
+	struct clk *clk = clk_get(dev, id);
+
+	if (clk == ERR_PTR(-EPROBE_DEFER) && deep_probe_is_supported())
+		return NULL;
+
+	return clk;
+}
+
 #endif
-- 
2.39.2

[PATCH v2 2/2] serial: stm32: support probing with missing clock provider

[Ahmad Fatoum]

The STM32MP135F-DK support in barebox has bitrotted, because of changes
to the upstream device tree: The root of the clock tree is now provided
by OP-TEE over SCMI for which no support exists in barebox, so drivers
requesting clocks will defer indefinitely, including the console device.

A series adding OP-TEE SCMI support will follow, but for now, let's
ensure that console registration isn't deferred in such a case.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 drivers/serial/serial_stm32.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/serial_stm32.c b/drivers/serial/serial_stm32.c
index 4a67408f44f6..3e18a2c152af 100644
--- a/drivers/serial/serial_stm32.c
+++ b/drivers/serial/serial_stm32.c
@@ -165,7 +165,7 @@ static int stm32_serial_probe(struct device *dev)
 	stm32->stm32f4 = info->stm32f4;
 	stm32->uart_enable_bit = info->uart_enable_bit;
 
-	stm32->clk = clk_get(dev, NULL);
+	stm32->clk = clk_get_for_console(dev, NULL);
 	if (IS_ERR(stm32->clk)) {
 		ret = PTR_ERR(stm32->clk);
 		dev_err(dev, "Failed to get UART clock %d\n", ret);
@@ -183,7 +183,7 @@ static int stm32_serial_probe(struct device *dev)
 	cdev->putc   = stm32_serial_putc;
 	cdev->getc   = stm32_serial_getc;
 	cdev->flush  = stm32_serial_flush;
-	cdev->setbrg = stm32_serial_setbaudrate;
+	cdev->setbrg = stm32->clk ? stm32_serial_setbaudrate : NULL;
 	cdev->linux_console_name = "ttySTM";
 
 	if (dev->of_node) {
-- 
2.39.2

Re: [PATCH v2 1/2] console: provide best-effort clk_get_for_console helper

[Sascha Hauer]

On Wed, Nov 22, 2023 at 06:13:19PM +0100, Ahmad Fatoum wrote:
> From: Ahmad Fatoum <ahmad@a3f.at>
> 
> clk_get will return -EPROBE_DEFER if clock provider hasn't yet been
> probed. In a system with deep probe enabled, dependencies are probed
> on demand, so a -EPROBE_DEFER return is final and the console probe
> will never succeed.
> 
> CONFIG_DEBUG_LL is often used to debug this, but because the low-level
> console is not interactive, it's a bit cumbersome. Improve upon this a
> bit, by providing a clk_get_for_console helper that returns NULL if and
> only if we are sure that a clock provider will not be probed.
> 
> In that case, the driver can skip code paths initialization code and
> baud rate setting dependent on having access to the clock and still
> register a console that reuses what was set up by CONFIG_DEBUG_LL.
> 
> Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
> ---
> v1 -> v2:
>   - restrict new mechanism to stdout console (Sascha)
> ---
>  include/console.h   | 27 +++++++++++++++++++++++++++
>  include/linux/clk.h | 21 +++++++++++++++++++++
>  2 files changed, 48 insertions(+)

Applied, thanks

Sascha

> 
> diff --git a/include/console.h b/include/console.h
> index d7cbdfa99207..69c0ec144b31 100644
> --- a/include/console.h
> +++ b/include/console.h
> @@ -8,6 +8,7 @@
>  #define _CONSOLE_H_
>  
>  #include <param.h>
> +#include <linux/clk.h>
>  #include <linux/list.h>
>  #include <driver.h>
>  #include <serdev.h>
> @@ -228,4 +229,30 @@ static inline void console_ctrlc_allow(void) { }
>  static inline void console_ctrlc_forbid(void) { }
>  #endif
>  
> +/**
> + * clk_get_for_console - get clock, ignoring known unavailable clock controller
> + * @dev: device for clock "consumer"
> + * @id: clock consumer ID
> + *
> + * Return: a struct clk corresponding to the clock producer, a
> + * valid IS_ERR() condition containing errno or NULL if it could
> + * be determined that the clock producer will never be probed in
> + * absence of modules. The NULL return allows serial drivers to
> + * skip clock handling for the stdout console and rely on CONFIG_DEBUG_LL.
> + */
> +static inline struct clk *clk_get_for_console(struct device *dev, const char *id)
> +{
> +	__always_unused unsigned baudrate;
> +	struct clk *clk;
> +
> +	if (!IS_ENABLED(CONFIG_DEBUG_LL) || !of_device_is_stdout_path(dev, &baudrate))
> +		return clk_get(dev, id);
> +
> +	clk = clk_get_if_available(dev, id);
> +	if (clk == NULL)
> +		dev_warn(dev, "couldn't get clock (ignoring)\n");
> +
> +	return clk;
> +}
> +
>  #endif
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 61b4ff3127cf..f505f18a75c0 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -14,6 +14,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/stringify.h>
>  #include <linux/container_of.h>
> +#include <deep-probe.h>
>  #include <xfuncs.h>
>  
>  struct device;
> @@ -982,4 +983,24 @@ static inline struct clk *clk_get_enabled(struct device *dev, const char *id)
>  	return clk;
>  }
>  
> +/**
> + * clk_get_if_available - get clock, ignoring known unavailable clock controller
> + * @dev: device for clock "consumer"
> + * @id: clock consumer ID
> + *
> + * Return: a struct clk corresponding to the clock producer, a
> + * valid IS_ERR() condition containing errno or NULL if it could
> + * be determined that the clock producer will never be probed in
> + * absence of modules.
> + */
> +static inline struct clk *clk_get_if_available(struct device *dev, const char *id)
> +{
> +	struct clk *clk = clk_get(dev, id);
> +
> +	if (clk == ERR_PTR(-EPROBE_DEFER) && deep_probe_is_supported())
> +		return NULL;
> +
> +	return clk;
> +}
> +
>  #endif
> -- 
> 2.39.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |