drivers: syscon: Add support for multiple regions

1. Add support for multiple syscon entries (as a side effect, this also
   fixed syscon.c implementations which weren't being linked to their
   syscon.h counterparts).
2. Add support for different width registers in syscon.
3. Add tests for syscon

Signed-off-by: Yuval Peress <[email protected]>

Author: Yuval Peress

CODEOWNERS

@@ -194,7 +194,7 @@
 /drivers/bbram/*                          @yperess @sjg20 @jackrosenthal
 /drivers/bluetooth/                       @joerchan @jhedberg @Vudentz
 /drivers/cache/                           @carlocaione
-/drivers/syscon/                          @carlocaione
+/drivers/syscon/                          @carlocaione @yperess
 /drivers/can/                             @alexanderwachter
 /drivers/can/*mcp2515*                    @karstenkoenig
 /drivers/can/*rcar*                       @julien-massot

drivers/syscon/Kconfig

@@ -30,10 +30,9 @@ config SYSCON_GENERIC
 	help
 	  Enable generic SYSCON (System Controller) driver
 
-config SYSCON_GENERIC_INIT_PRIORITY_DEVICE
-	int "SYSCON (System Controller) generic init device priority"
+config SYSCON_INIT_PRIORITY
+	int "SYSCON (System Controller) driver init priority"
 	default 50
-	depends on SYSCON_GENERIC
 	help
 	  This option controls the priority of the syscon device
 	  initialization. Higher priority ensures that the device is

drivers/syscon/syscon.c

@@ -12,106 +12,136 @@
 
 #include <drivers/syscon.h>
 
-static const struct device *syscon_dev;
+#include "syscon_common.h"
 
 struct syscon_generic_config {
 	DEVICE_MMIO_ROM;
-};
-
-static struct syscon_generic_config syscon_generic_config_0 = {
-	DEVICE_MMIO_ROM_INIT(DT_DRV_INST(0)),
+	uint8_t reg_width;
 };
 
 struct syscon_generic_data {
 	DEVICE_MMIO_RAM;
 	size_t size;
 };
 
-static struct syscon_generic_data syscon_generic_data_0;
-
-uintptr_t syscon_generic_get_base(void)
+static int syscon_generic_get_base(const struct device *dev, uintptr_t *addr)
 {
-	if (!syscon_dev) {
+	if (!dev) {
 		return -ENODEV;
 	}
 
-	return DEVICE_MMIO_GET(syscon_dev);
-}
-
-static int sanitize_reg(uint16_t *reg, size_t reg_size)
-{
-	/* Avoid unaligned readings */
-	*reg = ROUND_DOWN(*reg, sizeof(uint32_t));
-
-	/* Check for out-of-bounds readings */
-	if (*reg >= reg_size) {
-		return -EINVAL;
-	}
-
+	*addr = DEVICE_MMIO_GET(dev);
 	return 0;
 }
 
-int syscon_generic_read_reg(uint16_t reg, uint32_t *val)
+static int syscon_generic_read_reg(const struct device *dev, uint16_t reg, uint32_t *val)
 {
+	const struct syscon_generic_config *config;
 	struct syscon_generic_data *data;
 	uintptr_t base_address;
 
-	if (!syscon_dev) {
+	if (!dev) {
 		return -ENODEV;
 	}
 
-	data = syscon_dev->data;
+	data = dev->data;
+	config = dev->config;
 
 	if (!val) {
 		return -EINVAL;
 	}
 
-	if (sanitize_reg(&reg, data->size)) {
+	if (syscon_sanitize_reg(&reg, data->size, config->reg_width)) {
 		return -EINVAL;
 	}
 
-	base_address = DEVICE_MMIO_GET(syscon_dev);
-
-	*val = sys_read32(base_address + reg);
+	base_address = DEVICE_MMIO_GET(dev);
+
+	switch (config->reg_width) {
+	case 1:
+		*val = sys_read8(base_address + reg);
+		break;
+	case 2:
+		*val = sys_read16(base_address + reg);
+		break;
+	case 4:
+		*val = sys_read32(base_address + reg);
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	return 0;
 }
 
-int syscon_generic_write_reg(uint16_t reg, uint32_t val)
+static int syscon_generic_write_reg(const struct device *dev, uint16_t reg, uint32_t val)
 {
+	const struct syscon_generic_config *config;
 	struct syscon_generic_data *data;
 	uintptr_t base_address;
 
-	if (!syscon_dev) {
+	if (!dev) {
 		return -ENODEV;
 	}
 
-	data = syscon_dev->data;
+	data = dev->data;
+	config = dev->config;
 
-	if (sanitize_reg(&reg, data->size)) {
+	if (syscon_sanitize_reg(&reg, data->size, config->reg_width)) {
 		return -EINVAL;
 	}
 
-	base_address = DEVICE_MMIO_GET(syscon_dev);
-
-	sys_write32(val, (base_address + reg));
+	base_address = DEVICE_MMIO_GET(dev);
+
+	switch (config->reg_width) {
+	case 1:
+		sys_write8(val, (base_address + reg));
+		break;
+	case 2:
+		sys_write16(val, (base_address + reg));
+		break;
+	case 4:
+		sys_write32(val, (base_address + reg));
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	return 0;
 }
 
-int syscon_generic_init(const struct device *dev)
+static int syscon_generic_get_size(const struct device *dev, size_t *size)
 {
 	struct syscon_generic_data *data = dev->data;
 
-	syscon_dev = dev;
+	*size = data->size;
+	return 0;
+}
 
-	DEVICE_MMIO_MAP(syscon_dev, K_MEM_CACHE_NONE);
+static const struct syscon_driver_api syscon_generic_driver_api = {
+	.read = syscon_generic_read_reg,
+	.write = syscon_generic_write_reg,
+	.get_base = syscon_generic_get_base,
+	.get_size = syscon_generic_get_size,
+};
 
-	data->size = DT_REG_SIZE(DT_DRV_INST(0));
+static int syscon_generic_init(const struct device *dev)
+{
+	DEVICE_MMIO_MAP(dev, K_MEM_CACHE_NONE);
 
 	return 0;
 }
 
-DEVICE_DT_INST_DEFINE(0, syscon_generic_init, NULL, &syscon_generic_data_0,
-		&syscon_generic_config_0, PRE_KERNEL_1,
-		CONFIG_SYSCON_GENERIC_INIT_PRIORITY_DEVICE, NULL);
+#define SYSCON_INIT(inst)                                                                          \
+	static const struct syscon_generic_config syscon_generic_config_##inst = {                 \
+		DEVICE_MMIO_ROM_INIT(DT_DRV_INST(inst)),                                           \
+		.reg_width = DT_INST_PROP_OR(inst, reg_io_width, 4),                               \
+	};                                                                                         \
+	static struct syscon_generic_data syscon_generic_data_##inst = {                           \
+		.size = DT_REG_SIZE(DT_DRV_INST(inst)),                                            \
+	};                                                                                         \
+	DEVICE_DT_INST_DEFINE(inst, syscon_generic_init, NULL, &syscon_generic_data_##inst,        \
+			      &syscon_generic_config_##inst, PRE_KERNEL_1,                         \
+			      CONFIG_SYSCON_INIT_PRIORITY, &syscon_generic_driver_api);
+
+DT_INST_FOREACH_STATUS_OKAY(SYSCON_INIT);

drivers/syscon/syscon_common.h

@@ -0,0 +1,41 @@
+/*
+ * Copyright 2021 Google LLC.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef DRIVERS_SYSCON_SYSCON_COMMON_H_
+#define DRIVERS_SYSCON_SYSCON_COMMON_H_
+
+#include <sys/util.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @brief Align and check register address
+ *
+ * @param reg Pointer to the register address in question.
+ * @param reg_size The size of the syscon register region.
+ * @param reg_width The width of a single register (in bytes).
+ * @return 0 if the register read is valid.
+ * @return -EINVAL is the read is invalid.
+ */
+static inline int syscon_sanitize_reg(uint16_t *reg, size_t reg_size, uint8_t reg_width)
+{
+	/* Avoid unaligned readings */
+	*reg = ROUND_DOWN(*reg, reg_width);
+
+	/* Check for out-of-bounds readings */
+	if (*reg >= reg_size) {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* DRIVERS_SYSCON_SYSCON_COMMON_H_ */

dts/bindings/syscon/syscon.yaml

@@ -10,3 +10,7 @@ include: base.yaml
 properties:
     reg:
       required: true
+    reg-io-width:
+      type: int
+      required: false
+      description: The width of the registers in the syscon region in bytes. Default is 4 bytes.

include/drivers/syscon.h

@@ -27,37 +27,128 @@ extern "C" {
 #endif
 
 /**
- * @brief Get the syscon base address
+ * API template to get the base address of the syscon region.
  *
- * This function returns the syscon base address
+ * @see syscon_get_base
+ */
+typedef int (*syscon_api_get_base)(const struct device *dev, uintptr_t *addr);
+
+/**
+ * API template to read a single register.
  *
- * @return 0 on error, the base address on success
+ * @see syscon_read_reg
  */
-uintptr_t syscon_get_base(void);
+typedef int (*syscon_api_read_reg)(const struct device *dev, uint16_t reg, uint32_t *val);
+
+/**
+ * API template to write a single register.
+ *
+ * @see syscon_write_reg
+ */
+typedef int (*syscon_api_write_reg)(const struct device *dev, uint16_t reg, uint32_t val);
+
+/**
+ * API template to get the size of the syscon register.
+ *
+ * @see syscon_get_size
+ */
+typedef int (*syscon_api_get_size)(const struct device *dev, size_t *size);
+
+/**
+ * @brief System Control (syscon) register driver API
+ */
+__subsystem struct syscon_driver_api {
+	syscon_api_read_reg read;
+	syscon_api_write_reg write;
+	syscon_api_get_base get_base;
+	syscon_api_get_size get_size;
+};
+
+/**
+ * @brief Get the syscon base address
+ *
+ * @param dev The device to get the register size for.
+ * @param addr Where to write the base address.
+ * @return 0 When addr was written to.
+ */
+__syscall int syscon_get_base(const struct device *dev, uintptr_t *addr);
+
+static inline int z_impl_syscon_get_base(const struct device *dev, uintptr_t *addr)
+{
+	const struct syscon_driver_api *api = (const struct syscon_driver_api *)dev->api;
+
+	if (api == NULL) {
+		return -ENOTSUP;
+	}
+
+	return api->get_base(dev, addr);
+}
+
 
 /**
  * @brief Read from syscon register
  *
  * This function reads from a specific register in the syscon area
  *
+ * @param dev The device to get the register size for.
  * @param reg The register offset
  * @param val The returned value read from the syscon register
  *
  * @return 0 on success, negative on error
  */
-int syscon_read_reg(uint16_t reg, uint32_t *val);
+__syscall int syscon_read_reg(const struct device *dev, uint16_t reg, uint32_t *val);
+
+static inline int z_impl_syscon_read_reg(const struct device *dev, uint16_t reg, uint32_t *val)
+{
+	const struct syscon_driver_api *api = (const struct syscon_driver_api *)dev->api;
+
+	if (api == NULL) {
+		return -ENOTSUP;
+	}
+
+	return api->read(dev, reg, val);
+}
+
 
 /**
  * @brief Write to syscon register
  *
  * This function writes to a specific register in the syscon area
  *
+ * @param dev The device to get the register size for.
  * @param reg The register offset
  * @param val The value to be written in the register
  *
  * @return 0 on success, negative on error
  */
-int syscon_write_reg(uint16_t reg, uint32_t val);
+__syscall int syscon_write_reg(const struct device *dev, uint16_t reg, uint32_t val);
+
+static inline int z_impl_syscon_write_reg(const struct device *dev, uint16_t reg, uint32_t val)
+{
+	const struct syscon_driver_api *api = (const struct syscon_driver_api *)dev->api;
+
+	if (api == NULL) {
+		return -ENOTSUP;
+	}
+
+	return api->write(dev, reg, val);
+}
+
+/**
+ * Get the size of the syscon register in bytes.
+ *
+ * @param dev The device to get the register size for.
+ * @param size Pointer to write the size to.
+ * @return 0 for success.
+ */
+__syscall int syscon_get_size(const struct device *dev, size_t *size);
+
+static inline int z_impl_syscon_get_size(const struct device *dev, size_t *size)
+{
+	const struct syscon_driver_api *api = (const struct syscon_driver_api *)dev->api;
+
+	return api->get_size(dev, size);
+}
 
 /**
  * @}
@@ -67,4 +158,6 @@ int syscon_write_reg(uint16_t reg, uint32_t val);
 }
 #endif
 
-#endif	/* ZEPHYR_INCLUDE_DRIVERS_SYSCON_H_ */
+#include <syscalls/syscon.h>
+
+#endif /* ZEPHYR_INCLUDE_DRIVERS_SYSCON_H_ */