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

On 10/04/2024 19:27, 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.

I don't see an obvious usecase for having multiple stub drivers for a given uclass. Maybe as a result of that, I have no idea how we'd go about choosing which stub to use if multiple were available.

I don't think it would be particularly hard to switch over if at some point in the future we have such a need, but I don't see a benefit to this added complexity now.

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

I could drop the CONFIG_FALLBACK_DRIVERS option, it's just there to make it possible to opt in/out of all stub drivers at once, but maybe this isn't very useful.

Kind Regards,

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;