[PATCH 1/8] dm: soc: Introduce UCLASS_SOC for SOC ID and attribute matching

[Simon Glass <sjg@chromium.org>]

On Mon, 29 Jun 2020 at 22:38, Dave Gerlach <d-gerlach@ti.com> wrote:
>
> Introduce UCLASS_SOC to be used for SOC identification and attribute
> matching based on the SoC ID info. This allows drivers to be provided
> for SoCs to retrieve SoC identifying information and also for matching
> device attributes for selecting SoC specific data.
>
> This is useful for other device drivers that may need different
> parameters or quirks enabled depending on the specific device variant in
> use.
>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
>  drivers/soc/Kconfig      |   9 +++
>  drivers/soc/Makefile     |   1 +
>  drivers/soc/soc-uclass.c | 102 ++++++++++++++++++++++++++++++
>  include/dm/uclass-id.h   |   1 +
>  include/soc.h            | 132 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 245 insertions(+)
>  create mode 100644 drivers/soc/soc-uclass.c
>  create mode 100644 include/soc.h

Reviewed-by: Simon Glass <sjg@chromium.org>

As Tom says, docs please but looks great otherwise.

Nits below. Do you think it might make sense to have an
integer-encoded version? Maybe it would be more hassle than it is
worth.

>
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> index 7b4e4d613088..e715dfd01712 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -1,5 +1,14 @@
>  menu "SOC (System On Chip) specific Drivers"
>
> +config SOC_DEVICE
> +       bool "Enable SoC Device ID drivers using Driver Model"
> +       help
> +         This allows drivers to be provided for SoCs to help in identifying
> +         the SoC in use and matching SoC attributes for selecting SoC
> +         specific data. This is useful for other device drivers that may
> +         need different parameters or quirks enabled depending on the
> +         specific device variant in use.
> +
>  source "drivers/soc/ti/Kconfig"
>
>  endmenu
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index ce253b7aa886..1c09a8465670 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -3,3 +3,4 @@
>  # Makefile for the U-Boot SOC specific device drivers.
>
>  obj-$(CONFIG_SOC_TI) += ti/
> +obj-$(CONFIG_SOC_DEVICE) += soc-uclass.o
> diff --git a/drivers/soc/soc-uclass.c b/drivers/soc/soc-uclass.c
> new file mode 100644
> index 000000000000..22f89514ed7d
> --- /dev/null
> +++ b/drivers/soc/soc-uclass.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2020 - Texas Instruments Incorporated - http://www.ti.com/
> + *     Dave Gerlach <d-gerlach@ti.com>
> + */
> +
> +#include <common.h>
> +#include <soc.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <dm/lists.h>
> +#include <dm/root.h>
> +
> +int soc_get(struct udevice **devp)
> +{
> +       return uclass_first_device_err(UCLASS_SOC, devp);
> +}
> +
> +int soc_get_machine(struct udevice *dev, char *buf, int size)
> +{
> +       struct soc_ops *ops = soc_get_ops(dev);
> +
> +       if (!ops->get_machine)
> +               return -ENOSYS;
> +
> +       return ops->get_machine(dev, buf, size);
> +}
> +
> +int soc_get_family(struct udevice *dev, char *buf, int size)
> +{
> +       struct soc_ops *ops = soc_get_ops(dev);
> +
> +       if (!ops->get_family)
> +               return -ENOSYS;
> +
> +       return ops->get_family(dev, buf, size);
> +}
> +
> +int soc_get_revision(struct udevice *dev, char *buf, int size)
> +{
> +       struct soc_ops *ops = soc_get_ops(dev);
> +
> +       if (!ops->get_revision)
> +               return -ENOSYS;
> +
> +       return ops->get_revision(dev, buf, size);
> +}
> +
> +const struct soc_device_attribute *
> +soc_device_match(const struct soc_device_attribute *matches)
> +{
> +       bool match;
> +       struct udevice *soc;
> +       char str[SOC_MAX_STR_SIZE];
> +
> +       if (!matches)
> +               return NULL;
> +
> +       if (soc_get(&soc))
> +               return NULL;
> +
> +       while (1) {
> +               if (!(matches->machine || matches->family ||
> +                     matches->revision))
> +                       break;
> +
> +               match = true;
> +
> +               if (matches->machine) {
> +                       if (!soc_get_machine(soc, str, SOC_MAX_STR_SIZE)) {
> +                               if (strcmp(matches->machine, str))
> +                                       match = false;
> +                       }
> +               }
> +
> +               if (matches->family) {
> +                       if (!soc_get_family(soc, str, SOC_MAX_STR_SIZE)) {
> +                               if (strcmp(matches->family, str))
> +                                       match = false;
> +                       }
> +               }
> +
> +               if (matches->revision) {
> +                       if (!soc_get_revision(soc, str, SOC_MAX_STR_SIZE)) {
> +                               if (strcmp(matches->revision, str))
> +                                       match = false;
> +                       }
> +               }
> +
> +               if (match)
> +                       return matches;
> +
> +               matches++;
> +       }
> +
> +       return NULL;
> +}
> +
> +UCLASS_DRIVER(soc) = {
> +       .id             = UCLASS_SOC,
> +       .name           = "soc",
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 7837d459f18c..690a8ed4df49 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -97,6 +97,7 @@ enum uclass_id {
>         UCLASS_SERIAL,          /* Serial UART */
>         UCLASS_SIMPLE_BUS,      /* Bus with child devices */
>         UCLASS_SMEM,            /* Shared memory interface */
> +       UCLASS_SOC,             /* SOC Device */
>         UCLASS_SOUND,           /* Playing simple sounds */
>         UCLASS_SPI,             /* SPI bus */
>         UCLASS_SPI_FLASH,       /* SPI flash */
> diff --git a/include/soc.h b/include/soc.h
> new file mode 100644
> index 000000000000..98e3cc000198
> --- /dev/null
> +++ b/include/soc.h
> @@ -0,0 +1,132 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * (C) Copyright 2020 - Texas Instruments Incorporated - http://www.ti.com/
> + *     Dave Gerlach <d-gerlach@ti.com>
> + */
> +
> +#ifndef __SOC_H
> +#define __SOC_H
> +
> +#define SOC_MAX_STR_SIZE       128
> +
> +struct soc_device_attribute {

Could this be soc_attr or maybe soc_dev_attr? This is too long.

> +       const char *machine;
> +       const char *family;
> +       const char *revision;
> +       const void *data;

Please can you comment this struct with what each thing means and an
example for your SoC.

> +};
> +
> +#ifdef CONFIG_SOC_DEVICE

Please drop the #ifdef as there is no harm in defining this.

> +struct soc_ops {
> +       /**
> +        * get_machine() - Get machine name of an SOC
> +        *
> +        * @dev:        Device to check (UCLASS_SOC)
> +        * @buf:        Buffer to place string
> +        * @size:       Size of string space
> +        * @return 0 if OK, -ENOSPC if buffer is too small, other -ve on error
> +        */
> +       int (*get_machine)(struct udevice *dev, char *buf, int size);
> +
> +       /**
> +        * get_revision() - Get revision name of a SOC
> +        *
> +        * @dev:        Device to check (UCLASS_SOC)
> +        * @buf:        Buffer to place string
> +        * @size:       Size of string space
> +        * @return 0 if OK, -ENOSPC if buffer is too small, other -ve on error
> +        */
> +       int (*get_revision)(struct udevice *dev, char *buf, int size);
> +
> +       /**
> +        * get_family() - Get family name of an SOC
> +        *
> +        * @dev:        Device to check (UCLASS_SOC)
> +        * @buf:        Buffer to place string
> +        * @size:       Size of string space
> +        * @return 0 if OK, -ENOSPC if buffer is too small, other -ve on error
> +        */
> +       int (*get_family)(struct udevice *dev, char *buf, int size);
> +};
> +
> +#define soc_get_ops(dev)        ((struct soc_ops *)(dev)->driver->ops)
> +
> +/**
> + * soc_get() - Return the soc device for the soc in use.
> + * @devp: Pointer to structure to receive the soc device.
> + *
> + * Since there can only be at most one SOC instance, the API can supply a
> + * function that returns the unique device.
> + *
> + * Return: 0 if OK, -ve on error.
> + */
> +int soc_get(struct udevice **devp);
> +
> +/**
> + * soc_get_machine() - Get machine name of an SOC
> + * @dev:       Device to check (UCLASS_SOC)
> + * @buf:       Buffer to place string
> + * @size:      Size of string space
> + *
> + * Return: 0 if OK, -ENOSPC if buffer is too small, other -ve on error
> + */
> +int soc_get_machine(struct udevice *dev, char *buf, int size);
> +
> +/**
> + * soc_get_revision() - Get revision name of an SOC
> + * @dev:       Device to check (UCLASS_SOC)
> + * @buf:       Buffer to place string
> + * @size:      Size of string space
> + *
> + * Return: 0 if OK, -ENOSPC if buffer is too small, other -ve on error
> + */
> +int soc_get_revision(struct udevice *dev, char *buf, int size);
> +
> +/**
> + * soc_get_family() - Get family name of an SOC
> + * @dev:       Device to check (UCLASS_SOC)
> + * @buf:       Buffer to place string
> + * @size:      Size of string space
> + *
> + * Return: 0 if OK, -ENOSPC if buffer is too small, other -ve on error
> + */
> +int soc_get_family(struct udevice *dev, char *buf, int size);
> +
> +/**
> + * soc_device_match() - Return match from an array of soc_device_attribute
> + * @matches:   Array with any combination of family, revision or machine set
> + *
> + * Return: Pointer to struct from matches array with set attributes matching
> + *        those provided by the soc device.

or NULL ?

> + */
> +const struct soc_device_attribute *
> +soc_device_match(const struct soc_device_attribute *matches);
> +
> +#else
> +static inline int soc_get(struct udevice **devp)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline int soc_get_machine(struct udevice *dev, char *buf, int size)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline int soc_get_revision(struct udevice *dev, char *buf, int size)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline int soc_get_family(struct udevice *dev, char *buf, int size)
> +{
> +       return -ENOSYS;
> +}
> +
> +static inline const struct soc_device_attribute *
> +soc_device_match(const struct soc_device_attribute *matches)
> +{
> +       return NULL;
> +}
> +#endif
> +#endif /* _SOC_H */
> --
> 2.20.1
>

Regards,
Simon