diff mbox series

[1/2] dm: core: add support for fallback drivers

Message ID 20240410-b4-stub-drivers-v1-1-6935bd2c07d1@linaro.org
State New
Headers show
Series dm: add support for stubbing optional devices | expand

Commit Message

Caleb Connolly April 10, 2024, 5:06 p.m. UTC
Introduce support for a uclass to provide a fallback/stub driver which
can be used when no device is found for a given node. This might be
useful for handling non-essential clock controllers like the RPMh on
Qualcomm platforms, or during early bringup to get UART output before a
real clock driver has been created.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/core/Kconfig  | 10 ++++++++++
 drivers/core/uclass.c | 24 +++++++++++++++++++++++-
 include/dm/uclass.h   |  3 +++
 3 files changed, 36 insertions(+), 1 deletion(-)

Comments

Heinrich Schuchardt April 10, 2024, 5:27 p.m. UTC | #1
Am 10. April 2024 19:06:57 MESZ schrieb Caleb Connolly <caleb.connolly@linaro.org>:
>Introduce support for a uclass to provide a fallback/stub driver which
>can be used when no device is found for a given node. This might be
>useful for handling non-essential clock controllers like the RPMh on
>Qualcomm platforms, or during early bringup to get UART output before a
>real clock driver has been created.
>
>Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>---
> drivers/core/Kconfig  | 10 ++++++++++
> drivers/core/uclass.c | 24 +++++++++++++++++++++++-
> include/dm/uclass.h   |  3 +++
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
>index 1081d61fcf01..09075b9b7a15 100644
>--- a/drivers/core/Kconfig
>+++ b/drivers/core/Kconfig
>@@ -466,5 +466,15 @@ config BOUNCE_BUFFER
> 
> 	  A second possible use of bounce buffers is their ability to
> 	  provide aligned buffers for DMA operations.
> 
>+menuconfig FALLBACK_DRIVERS

Wouldn't it be preferable to mark individual drivers as fallback drivers in their declaration?

This would allow alternative fallback drivers and would not require any definitions at uclass level.

Just by building a fallback driver you would enable the fallback behavior for its uclass.

Best regards

Heinrich

>+	bool "Enable per-uclass fallback drivers"
>+	depends on DM
>+	help
>+	  If a driver requests a resource (like a clock) from a node which
>+	  isn't bound to a driver, the driver model will look for a fallback
>+	  driver to "stub" the resource. These stubs usually do nothing and
>+	  are therefore only suitable in instances where the resource is not
>+	  required.
>+
> endmenu
>diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
>index e46d5717aa62..91d3a48d77b8 100644
>--- a/drivers/core/uclass.c
>+++ b/drivers/core/uclass.c
>@@ -378,8 +378,26 @@ int uclass_find_device_by_of_offset(enum uclass_id id, int node,
> 
> 	return -ENODEV;
> }
> 
>+static int uclass_bind_fallback(struct uclass *uc, ofnode node, struct udevice **devp)
>+{
>+	struct driver *drv;
>+
>+	log(LOGC_DM, LOGL_ERR, "   - binding fallback '%s' driver '%s'\n",
>+	    uc->uc_drv->name, uc->uc_drv->fallback_drv_name);
>+
>+	drv = lists_driver_lookup_name(uc->uc_drv->fallback_drv_name);
>+	if (!drv) {
>+		log(LOGC_DM, LOGL_DEBUG, "   - Can't find stub driver '%s' for uclass '%s'\n",
>+		    uc->uc_drv->fallback_drv_name, uc->uc_drv->name);
>+		return -ENOENT;
>+	}
>+
>+	return device_bind_with_driver_data(gd->dm_root, drv,
>+					  ofnode_get_name(node), 0, node, devp);
>+}
>+
> int uclass_find_device_by_ofnode(enum uclass_id id, ofnode node,
> 				 struct udevice **devp)
> {
> 	struct uclass *uc;
>@@ -401,9 +419,13 @@ int uclass_find_device_by_ofnode(enum uclass_id id, ofnode node,
> 			*devp = dev;
> 			goto done;
> 		}
> 	}
>-	ret = -ENODEV;
>+
>+	if (CONFIG_IS_ENABLED(FALLBACK_DRIVERS) && uc->uc_drv->fallback_drv_name)
>+		ret = uclass_bind_fallback(uc, node, devp);
>+	else
>+		ret = -ENODEV;
> 
> done:
> 	log(LOGC_DM, LOGL_DEBUG, "   - result for %s: %s (ret=%d)\n",
> 	    ofnode_get_name(node), *devp ? (*devp)->name : "(none)", ret);
>diff --git a/include/dm/uclass.h b/include/dm/uclass.h
>index 456eef7f2f31..b99e36485bc5 100644
>--- a/include/dm/uclass.h
>+++ b/include/dm/uclass.h
>@@ -67,8 +67,10 @@ struct udevice;
>  * @child_pre_probe: Called before a child in this uclass is probed
>  * @child_post_probe: Called after a child in this uclass is probed
>  * @init: Called to set up the uclass
>  * @destroy: Called to destroy the uclass
>+ * @stub_drv_name: Name of a stub driver to use for devices that are not
>+ * supported by any other driver.
>  * @priv_auto: If non-zero this is the size of the private data
>  * to be allocated in the uclass's ->priv pointer. If zero, then the uclass
>  * driver is responsible for allocating any data required.
>  * @per_device_auto: Each device can hold private data owned
>@@ -98,8 +100,9 @@ struct uclass_driver {
> 	int (*child_pre_probe)(struct udevice *dev);
> 	int (*child_post_probe)(struct udevice *dev);
> 	int (*init)(struct uclass *class);
> 	int (*destroy)(struct uclass *class);
>+	const char *fallback_drv_name;
> 	int priv_auto;
> 	int per_device_auto;
> 	int per_device_plat_auto;
> 	int per_child_auto;
>
Tom Rini April 10, 2024, 7:44 p.m. UTC | #2
On Wed, Apr 10, 2024 at 07:27:17PM +0200, Heinrich Schuchardt wrote:
> 
> 
> Am 10. April 2024 19:06:57 MESZ schrieb Caleb Connolly <caleb.connolly@linaro.org>:
> >Introduce support for a uclass to provide a fallback/stub driver which
> >can be used when no device is found for a given node. This might be
> >useful for handling non-essential clock controllers like the RPMh on
> >Qualcomm platforms, or during early bringup to get UART output before a
> >real clock driver has been created.
> >
> >Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> >---
> > drivers/core/Kconfig  | 10 ++++++++++
> > drivers/core/uclass.c | 24 +++++++++++++++++++++++-
> > include/dm/uclass.h   |  3 +++
> > 3 files changed, 36 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> >index 1081d61fcf01..09075b9b7a15 100644
> >--- a/drivers/core/Kconfig
> >+++ b/drivers/core/Kconfig
> >@@ -466,5 +466,15 @@ config BOUNCE_BUFFER
> > 
> > 	  A second possible use of bounce buffers is their ability to
> > 	  provide aligned buffers for DMA operations.
> > 
> >+menuconfig FALLBACK_DRIVERS
> 
> Wouldn't it be preferable to mark individual drivers as fallback drivers in their declaration?
> 
> This would allow alternative fallback drivers and would not require any definitions at uclass level.
> 
> Just by building a fallback driver you would enable the fallback behavior for its uclass.

I think some of this is addressed in the cover letter. My concern /
questions come down to I think just a matter of naming. Both "fallback"
and "stub" are used at times. As a concept, we have some areas where we
need a no-op driver because whereas the DT describes a relationship in
the hardware, here in U-Boot we can just accept things as configured. To
me "fallback" implies more functionality of the driver when it's really
just a generic no-op driver to fill in the dependencies within the tree.
Can we rename things a bit along those lines?
Sean Anderson April 11, 2024, 2:37 a.m. UTC | #3
On 4/10/24 15:44, Tom Rini wrote:
> On Wed, Apr 10, 2024 at 07:27:17PM +0200, Heinrich Schuchardt wrote:
>>
>>
>> Am 10. April 2024 19:06:57 MESZ schrieb Caleb Connolly <caleb.connolly@linaro.org>:
>>> Introduce support for a uclass to provide a fallback/stub driver which
>>> can be used when no device is found for a given node. This might be
>>> useful for handling non-essential clock controllers like the RPMh on
>>> Qualcomm platforms, or during early bringup to get UART output before a
>>> real clock driver has been created.
>>>
>>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>>> ---
>>> drivers/core/Kconfig  | 10 ++++++++++
>>> drivers/core/uclass.c | 24 +++++++++++++++++++++++-
>>> include/dm/uclass.h   |  3 +++
>>> 3 files changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
>>> index 1081d61fcf01..09075b9b7a15 100644
>>> --- a/drivers/core/Kconfig
>>> +++ b/drivers/core/Kconfig
>>> @@ -466,5 +466,15 @@ config BOUNCE_BUFFER
>>>
>>> 	  A second possible use of bounce buffers is their ability to
>>> 	  provide aligned buffers for DMA operations.
>>>
>>> +menuconfig FALLBACK_DRIVERS
>>
>> Wouldn't it be preferable to mark individual drivers as fallback drivers in their declaration?
>>
>> This would allow alternative fallback drivers and would not require any definitions at uclass level.
>>
>> Just by building a fallback driver you would enable the fallback behavior for its uclass.
> 
> I think some of this is addressed in the cover letter. My concern /
> questions come down to I think just a matter of naming. Both "fallback"
> and "stub" are used at times. As a concept, we have some areas where we
> need a no-op driver because whereas the DT describes a relationship in
> the hardware, here in U-Boot we can just accept things as configured. To
> me "fallback" implies more functionality of the driver when it's really
> just a generic no-op driver to fill in the dependencies within the tree.
> Can we rename things a bit along those lines?
> 

I would rather just have a stub driver with compatibles for all clocks we want
it to match. I think having a generic fallback driver could cause issues in the
future if we need to switch over to using a real driver.

--Sean
diff mbox series

Patch

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 1081d61fcf01..09075b9b7a15 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -466,5 +466,15 @@  config BOUNCE_BUFFER
 
 	  A second possible use of bounce buffers is their ability to
 	  provide aligned buffers for DMA operations.
 
+menuconfig FALLBACK_DRIVERS
+	bool "Enable per-uclass fallback drivers"
+	depends on DM
+	help
+	  If a driver requests a resource (like a clock) from a node which
+	  isn't bound to a driver, the driver model will look for a fallback
+	  driver to "stub" the resource. These stubs usually do nothing and
+	  are therefore only suitable in instances where the resource is not
+	  required.
+
 endmenu
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index e46d5717aa62..91d3a48d77b8 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -378,8 +378,26 @@  int uclass_find_device_by_of_offset(enum uclass_id id, int node,
 
 	return -ENODEV;
 }
 
+static int uclass_bind_fallback(struct uclass *uc, ofnode node, struct udevice **devp)
+{
+	struct driver *drv;
+
+	log(LOGC_DM, LOGL_ERR, "   - binding fallback '%s' driver '%s'\n",
+	    uc->uc_drv->name, uc->uc_drv->fallback_drv_name);
+
+	drv = lists_driver_lookup_name(uc->uc_drv->fallback_drv_name);
+	if (!drv) {
+		log(LOGC_DM, LOGL_DEBUG, "   - Can't find stub driver '%s' for uclass '%s'\n",
+		    uc->uc_drv->fallback_drv_name, uc->uc_drv->name);
+		return -ENOENT;
+	}
+
+	return device_bind_with_driver_data(gd->dm_root, drv,
+					  ofnode_get_name(node), 0, node, devp);
+}
+
 int uclass_find_device_by_ofnode(enum uclass_id id, ofnode node,
 				 struct udevice **devp)
 {
 	struct uclass *uc;
@@ -401,9 +419,13 @@  int uclass_find_device_by_ofnode(enum uclass_id id, ofnode node,
 			*devp = dev;
 			goto done;
 		}
 	}
-	ret = -ENODEV;
+
+	if (CONFIG_IS_ENABLED(FALLBACK_DRIVERS) && uc->uc_drv->fallback_drv_name)
+		ret = uclass_bind_fallback(uc, node, devp);
+	else
+		ret = -ENODEV;
 
 done:
 	log(LOGC_DM, LOGL_DEBUG, "   - result for %s: %s (ret=%d)\n",
 	    ofnode_get_name(node), *devp ? (*devp)->name : "(none)", ret);
diff --git a/include/dm/uclass.h b/include/dm/uclass.h
index 456eef7f2f31..b99e36485bc5 100644
--- a/include/dm/uclass.h
+++ b/include/dm/uclass.h
@@ -67,8 +67,10 @@  struct udevice;
  * @child_pre_probe: Called before a child in this uclass is probed
  * @child_post_probe: Called after a child in this uclass is probed
  * @init: Called to set up the uclass
  * @destroy: Called to destroy the uclass
+ * @stub_drv_name: Name of a stub driver to use for devices that are not
+ * supported by any other driver.
  * @priv_auto: If non-zero this is the size of the private data
  * to be allocated in the uclass's ->priv pointer. If zero, then the uclass
  * driver is responsible for allocating any data required.
  * @per_device_auto: Each device can hold private data owned
@@ -98,8 +100,9 @@  struct uclass_driver {
 	int (*child_pre_probe)(struct udevice *dev);
 	int (*child_post_probe)(struct udevice *dev);
 	int (*init)(struct uclass *class);
 	int (*destroy)(struct uclass *class);
+	const char *fallback_drv_name;
 	int priv_auto;
 	int per_device_auto;
 	int per_device_plat_auto;
 	int per_child_auto;