[U-Boot] [PATCH v2 2/6] dm: core: Allow multiple drivers to bind for a single node

[Simon Glass]

Hi Philipp,

On 22 February 2017 at 13:47, Philipp Tomsich
<philipp.tomsich at theobroma-systems.com> wrote:
> Currently, driver binding stops once it encounters the first
> compatible driver that doesn't refuse to bind. However, there are
> cases where a single node will need to be handled by multiple driver
> classes. For those cases we provide a configurable option to continue
> to bind after the first driver has been found.
>
> The first use cases for this are from the DM conversion of the sunxi
> (Allwinner) architecture:
>  * pinctrl (UCLASS_PINCTRL) and gpio (UCLASS_GPIO) drivers need to
>    bind against a single node
>  * clock (UCLASS_CLK) and reset (UCLASS_RESET) drivers also need to
>    bind against a single node

Does linux work this way? Another approach would be to have a separate
MISC driver with two children, one pinctrl, one clk.

>
> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
> ---
>  drivers/core/Kconfig | 14 ++++++++++++++
>  drivers/core/lists.c | 12 +++++++++++-
>  2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> index 8749561..913101c 100644
> --- a/drivers/core/Kconfig
> +++ b/drivers/core/Kconfig
> @@ -31,6 +31,20 @@ config DM_WARN
>           This will cause dm_warn() to be compiled out - it will do nothing
>           when called.
>
> +config DM_ALLOW_MULTIPLE_DRIVERS
> +        bool "Allow multiple drivers to bind for one node"
> +       depends on DM
> +       default n

You should be able to drop this line.

> +       help
> +         The driver model in U-Boot originally did not allow multiple
> +         drivers to bind for a single device node.
> +
> +         If enabled, multiple drivers can now bind for a single node
> +         by using the same compatible string for matching: lists_bind_fdt()
> +         will assume that binding multiple drivers is desirable, if the
> +         caller does not request the pointer to the udevice structure to
> +         be returned (i.e. if devp is NULL).

Please update the function documentation in the header file.

> +
>  config DM_DEVICE_REMOVE
>         bool "Support device removal"
>         depends on DM
> diff --git a/drivers/core/lists.c b/drivers/core/lists.c
> index 23b6ba7..52efe69 100644
> --- a/drivers/core/lists.c
> +++ b/drivers/core/lists.c
> @@ -166,7 +166,11 @@ int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,
>                 dm_dbg("   - attempt to match compatible string '%s'\n",
>                        compat);
>
> -               for (entry = driver; entry != driver + n_ents; entry++) {
> +               entry = driver;
> +#if defined(CONFIG_DM_ALLOW_MULTIPLE_DRIVERS)
> +       allow_more_matches:
> +#endif
> +               for (; entry != driver + n_ents; entry++) {
>                         ret = driver_check_compatible(entry->of_match, &id,
>                                                       compat);
>                         if (!ret)
> @@ -190,6 +194,12 @@ int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,
>                         found = true;
>                         if (devp)
>                                 *devp = dev;
> +#if defined(CONFIG_DM_ALLOW_MULTIPLE_DRIVERS)

Can you make this a variable, e.g. with allow_multiple =
IS_ENABLED(DM_ALLOW_MULTIPLE_DRIVERS)? I'd prefer not to add #ifdefs
in this file.

> +                       else {
> +                               entry++;
> +                               goto allow_more_matches;

Is it possible to loop without using goto?

> +                       }
> +#endif
>                 }
>                 break;
>         }
> --
> 1.9.1
>

Regards,
Simon