Re: [U-Boot] [PATCH 1/4] dm: Add Hardware Spinlock class

Hi Benjamin,

On 13 November 2018 at 01:51, Benjamin Gaignard benjamin.gaignard@linaro.org wrote:

From: Benjamin Gaignard benjamin.gaignard@linaro.org

This is uclass for Hardware Spinlocks. It implements two mandatory operations: lock and unlock and one optional relax operation.

Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org

arch/sandbox/dts/test.dts | 4 + arch/sandbox/include/asm/state.h | 1 + configs/sandbox_defconfig | 2 + drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/hwspinlock/Kconfig | 16 ++++ drivers/hwspinlock/Makefile | 6 ++ drivers/hwspinlock/hwspinlock-uclass.c | 145 ++++++++++++++++++++++++++++++++ drivers/hwspinlock/sandbox_hwspinlock.c | 56 ++++++++++++ include/dm/uclass-id.h | 1 + include/hwspinlock.h | 140 ++++++++++++++++++++++++++++++ test/dm/Makefile | 1 + test/dm/hwspinlock.c | 40 +++++++++ 13 files changed, 415 insertions(+) create mode 100644 drivers/hwspinlock/Kconfig create mode 100644 drivers/hwspinlock/Makefile create mode 100644 drivers/hwspinlock/hwspinlock-uclass.c create mode 100644 drivers/hwspinlock/sandbox_hwspinlock.c create mode 100644 include/hwspinlock.h create mode 100644 test/dm/hwspinlock.c

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 57e0dd7663..50cd2f89e0 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -712,6 +712,10 @@ sandbox_tee { compatible = "sandbox,tee"; };

•   hwspinlock@0 {
  

•           compatible = "sandbox,hwspinlock";
  

•   };
  

};

#include "sandbox_pmic.dtsi" diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h index dcb6d5f568..a5d7c6aaf3 100644 --- a/arch/sandbox/include/asm/state.h +++ b/arch/sandbox/include/asm/state.h @@ -99,6 +99,7 @@ struct sandbox_state {

    ulong next_tag;                 /* Next address tag to allocate */
    struct list_head mapmem_head;   /* struct sandbox_mapmem_entry */

•   bool hwspinlock;                /* Hardware Spinlock status */
  

};

/* Minimum space we guarantee in the state FDT when calling read/write*/ diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 2ce336fc81..36b67be2df 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -97,6 +97,8 @@ CONFIG_BOARD=y CONFIG_BOARD_SANDBOX=y CONFIG_PM8916_GPIO=y CONFIG_SANDBOX_GPIO=y +CONFIG_DM_HWSPINLOCK=y +CONFIG_HWSPINLOCK_SANDBOX=y CONFIG_DM_I2C_COMPAT=y CONFIG_I2C_CROS_EC_TUNNEL=y CONFIG_I2C_CROS_EC_LDO=y diff --git a/drivers/Kconfig b/drivers/Kconfig index 927a2b87f6..7e6ca7cd3e 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -40,6 +40,8 @@ source "drivers/fpga/Kconfig"

source "drivers/gpio/Kconfig"

+source "drivers/hwspinlock/Kconfig"

source "drivers/i2c/Kconfig"

source "drivers/input/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index fb38b67541..0ef56fb416 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -111,4 +111,5 @@ obj-$(CONFIG_W1) += w1/ obj-$(CONFIG_W1_EEPROM) += w1-eeprom/

obj-$(CONFIG_MACH_PIC32) += ddr/microchip/ +obj-$(CONFIG_DM_HWSPINLOCK) += hwspinlock/ endif diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig new file mode 100644 index 0000000000..de367fd2a9 --- /dev/null +++ b/drivers/hwspinlock/Kconfig @@ -0,0 +1,16 @@ +menu "Hardware Spinlock Support"

+config DM_HWSPINLOCK

•   bool "Enable U-Boot hardware spinlock support"
  

•   help
  

•     This option enables U-Boot hardware spinlock support
  

+config HWSPINLOCK_SANDBOX

•   bool "Enable Hardware Spinlock support for Sandbox"
  

•   depends on SANDBOX && DM_HWSPINLOCK
  

•   help
  

•     Enable hardware spinlock support in Sandbox. This is a dummy device that
  

•     can be probed and support all the methods of HWSPINLOCK, but does not
  

•     really do anything.
  

+endmenu diff --git a/drivers/hwspinlock/Makefile b/drivers/hwspinlock/Makefile new file mode 100644 index 0000000000..2704d6814f --- /dev/null +++ b/drivers/hwspinlock/Makefile @@ -0,0 +1,6 @@ +# SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause +# +# Copyright (C) 2018, STMicroelectronics - All Rights Reserved

+obj-$(CONFIG_DM_HWSPINLOCK) += hwspinlock-uclass.o +obj-$(CONFIG_HWSPINLOCK_SANDBOX) += sandbox_hwspinlock.o diff --git a/drivers/hwspinlock/hwspinlock-uclass.c b/drivers/hwspinlock/hwspinlock-uclass.c new file mode 100644 index 0000000000..1fed24228c --- /dev/null +++ b/drivers/hwspinlock/hwspinlock-uclass.c @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause +/*

• • Copyright (C) 2018, STMicroelectronics - All Rights Reserved
• */

+#include <common.h> +#include <dm.h> +#include <errno.h> +#include <hwspinlock.h> +#include <dm/device-internal.h>

+static inline const struct hwspinlock_ops * +hwspinlock_dev_ops(struct udevice *dev) +{

•   return (const struct hwspinlock_ops *)dev->driver->ops;
  

+}

+static int hwspinlock_of_xlate_default(struct hwspinlock *hws,

•                                  struct ofnode_phandle_args *args)
  

+{

•   if (args->args_count > 1) {
  

•           debug("Invaild args_count: %d\n", args->args_count);
  

•           return -EINVAL;
  

•   }
  

•   if (args->args_count)
  

•           hws->id = args->args[0];
  

•   else
  

•           hws->id = 0;
  

•   return 0;
  

+}

+int hwspinlock_get_by_index(struct udevice *dev, int index,

•                       struct hwspinlock *hws)
  

+{

•   int ret;
  

•   struct ofnode_phandle_args args;
  

•   struct udevice *dev_hws;
  

•   const struct hwspinlock_ops *ops;
  

•   assert(hws);
  

•   hws->dev = NULL;
  

•   ret = dev_read_phandle_with_args(dev, "hwlocks", "#hwlock-cells", 1,
  

•                                    index, &args);
  

•   if (ret) {
  

•           dev_dbg(dev, "%s: dev_read_phandle_with_args: err=%d\n",
  

•                   __func__, ret);
  

•           return ret;
  

•   }
  

•   ret = uclass_get_device_by_ofnode(UCLASS_HWSPINLOCK,
  

•                                     args.node, &dev_hws);
  

•   if (ret) {
  

•           dev_dbg(dev,
  

•                   "%s: uclass_get_device_by_of_offset failed: err=%d\n",
  

•                   __func__, ret);
  

•           return ret;
  

•   }
  

•   hws->dev = dev_hws;
  

•   ops = hwspinlock_dev_ops(dev_hws);
  

•   if (ops->of_xlate)
  

•           ret = ops->of_xlate(hws, &args);
  

•   else
  

•           ret = hwspinlock_of_xlate_default(hws, &args);
  

•   if (ret)
  

•           dev_dbg(dev, "of_xlate() failed: %d\n", ret);
  

•   return ret;
  

+}

+int hwspinlock_lock_timeout(struct hwspinlock *hws, unsigned int timeout) +{

•   const struct hwspinlock_ops *ops;
  

•   ulong start;
  

•   int ret;
  

•   assert(hws);
  

•   if (!hws->dev)
  

•           return -EINVAL;
  

•   ops = hwspinlock_dev_ops(hws->dev);
  

Remove this blank line?

•   if (!ops || !ops->lock)
  

•           return -EINVAL;
  

-ENOSYS but please drop the !ops since that shouldn't happen.

•   for (start = get_timer(0); get_timer(start) < timeout;) {
  

•           ret = ops->lock(hws->dev, hws->id);
  

•           if (!ret)
  

•                   return ret;
  

•           if (ops->relax)
  

•                   ops->relax(hws->dev);
  

•   }
  

•   return -ETIME;
  

We normally use ETIMEDOUT for this.

+}

+int hwspinlock_unlock(struct hwspinlock *hws) +{

•   const struct hwspinlock_ops *ops;
  

•   assert(hws);
  

•   if (!hws->dev)
  

•           return -EINVAL;
  

•   ops = hwspinlock_dev_ops(hws->dev);
  

•   if (!ops || !ops->unlock)
  

•           return -EINVAL;
  

-ENOSYS

•   return ops->unlock(hws->dev, hws->id);
  

+}

+static int hwspinlock_post_bind(struct udevice *dev) +{ +#if defined(CONFIG_NEEDS_MANUAL_RELOC)

•   struct hwspinlock_ops *ops = device_get_ops(dev);
  

•   static int reloc_done;
  

•   if (!reloc_done) {
  

•           if (ops->lock)
  

•                   ops->lock += gd->reloc_off;
  

•           if (ops->unlock)
  

•                   ops->unlock += gd->reloc_off;
  

•           if (ops->relax)
  

•                   ops->relax += gd->reloc_off;
  

•           reloc_done++;
  

•   }
  

+#endif

•   return 0;
  

+}

+UCLASS_DRIVER(hwspinlock) = {

•   .id             = UCLASS_HWSPINLOCK,
  

•   .name           = "hwspinlock",
  

•   .post_bind      = hwspinlock_post_bind,
  

+}; diff --git a/drivers/hwspinlock/sandbox_hwspinlock.c b/drivers/hwspinlock/sandbox_hwspinlock.c new file mode 100644 index 0000000000..be920f5f99 --- /dev/null +++ b/drivers/hwspinlock/sandbox_hwspinlock.c @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause +/*

• • Copyright (C) 2018, STMicroelectronics - All Rights Reserved
• */

+#include <common.h> +#include <dm.h> +#include <hwspinlock.h> +#include <asm/state.h>

+static int sandbox_lock(struct udevice *dev, int index) +{

•   struct sandbox_state *state = state_get_current();
  

•   if (index != 0)
  

•           return -1;
  

•   if (state->hwspinlock)
  

•           return -1;
  

•   state->hwspinlock = true;
  

•   return 0;
  

+}

+static int sandbox_unlock(struct udevice *dev, int index) +{

•   struct sandbox_state *state = state_get_current();
  

•   if (index != 0)
  

•           return -1;
  

•   if (!state->hwspinlock)
  

•           return -1;
  

•   state->hwspinlock = false;
  

•   return 0;
  

+}

+static const struct hwspinlock_ops sandbox_hwspinlock_ops = {

•   .lock = sandbox_lock,
  

•   .unlock = sandbox_unlock,
  

+};

+static const struct udevice_id sandbox_hwspinlock_ids[] = {

•   { .compatible = "sandbox,hwspinlock" },
  

•   {}
  

+};

+U_BOOT_DRIVER(hwspinlock_sandbox) = {

•   .name = "hwspinlock_sandbox",
  

•   .id = UCLASS_HWSPINLOCK,
  

•   .of_match = sandbox_hwspinlock_ids,
  

•   .ops = &sandbox_hwspinlock_ops,
  

+}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 269a2c6e72..6193017432 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -40,6 +40,7 @@ enum uclass_id { UCLASS_ETH, /* Ethernet device */ UCLASS_FS_FIRMWARE_LOADER, /* Generic loader */ UCLASS_GPIO, /* Bank of general-purpose I/O pins */

•   UCLASS_HWSPINLOCK,      /* Hardware semaphores */
    UCLASS_FIRMWARE,        /* Firmware */
    UCLASS_I2C,             /* I2C bus */
    UCLASS_I2C_EEPROM,      /* I2C EEPROM device */
  

diff --git a/include/hwspinlock.h b/include/hwspinlock.h new file mode 100644 index 0000000000..4671c8fce5 --- /dev/null +++ b/include/hwspinlock.h @@ -0,0 +1,140 @@ +/* SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause */ +/*

• • Copyright (C) 2018, STMicroelectronics - All Rights Reserved
• */

+#ifndef _HWSPINLOCK_H_ +#define _HWSPINLOCK_H_

+/**

• • Implement a hwspinlock uclass.
• • Hardware spinlocks are used to perform hardware protection of
• • critical sections and synchronisation between multiprocessors.
• */

+struct udevice;

+/**

• • struct hwspinlock - A handle to (allowing control of) a single hardware
• • spinlock.
• • @dev: The device which implements the hardware spinlock.
• • @id: The hardware spinlock ID within the provider.
• */

+struct hwspinlock {

•   struct udevice *dev;
  

•   unsigned long id;
  

+};

+#if CONFIG_IS_ENABLED(DM_HWSPINLOCK)

+/**

• • hwspinlock_get_by_index - Get a hardware spinlock by integer index
• • This looks up and request a hardware spinlock. The index is relative to the
• • client device; each device is assumed to have n hardware spinlock associated
• • with it somehow, and this function finds and requests one of them.
• • @dev: The client device.
• • @index: The index of the hardware spinlock to request, within the

• •         client's list of hardware spinlock.
    

• • @hws: A pointer to a hardware spinlock struct to initialize.
• • @return 0 if OK, or a negative error code.
• */

+int hwspinlock_get_by_index(struct udevice *dev,

•                       int index, struct hwspinlock *hws);
  

+/**

• • Lock the hardware spinlock
• • @hws: A hardware spinlock struct that previously requested by

• •         hwspinlock_get_by_index
    

• • @timeout: Timeout value in msecs
• • @return: 0 if OK, -ETIME if timeout, -ve on other errors

-ETIMEDOUT

[..]

Regards, Simon