[RFC PATCH 0/4] add ACPI support for syscon

[RFC PATCH 0/4] add ACPI support for syscon

[Kefeng Wang]

Lots of drivers begin to support both OF and ACPI now, but some drivers depend on syscon,
so syscon need to support ACPI firstly.

Add ACPI support for syscon, and introduce syscon_regmap_lookup_by_dev_property()
helper when the driver need to get a regmap handle from syscon, it can used in
both OF and ACPI.

Kefeng Wang (4):
  acpi: property: Introduce helper acpi_dev_get_reference_device()
  device property: Introduce helper device_get_reference_node()
  ACPI/platform: Introduce helper acpi_dev_find_plat_dev()
  mfd: syscon: add ACPI support

 drivers/acpi/acpi_platform.c | 25 ++++++++++++++++++++++
 drivers/acpi/property.c      | 23 ++++++++++++++++++++
 drivers/base/property.c      | 40 +++++++++++++++++++++++++++++++++++
 drivers/mfd/syscon.c         | 50 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h         | 13 ++++++++++++
 include/linux/mfd/syscon.h   |  8 +++++++
 include/linux/property.h     |  6 ++++++
 7 files changed, 165 insertions(+)

-- 
1.7.12.4

[RFC PATCH 0/4] add ACPI support for syscon

[Kefeng Wang]

Lots of drivers begin to support both OF and ACPI now, but some drivers depend on syscon,
so syscon need to support ACPI firstly.

Add ACPI support for syscon, and introduce syscon_regmap_lookup_by_dev_property()
helper when the driver need to get a regmap handle from syscon, it can used in
both OF and ACPI.

Kefeng Wang (4):
  acpi: property: Introduce helper acpi_dev_get_reference_device()
  device property: Introduce helper device_get_reference_node()
  ACPI/platform: Introduce helper acpi_dev_find_plat_dev()
  mfd: syscon: add ACPI support

 drivers/acpi/acpi_platform.c | 25 ++++++++++++++++++++++
 drivers/acpi/property.c      | 23 ++++++++++++++++++++
 drivers/base/property.c      | 40 +++++++++++++++++++++++++++++++++++
 drivers/mfd/syscon.c         | 50 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h         | 13 ++++++++++++
 include/linux/mfd/syscon.h   |  8 +++++++
 include/linux/property.h     |  6 ++++++
 7 files changed, 165 insertions(+)

-- 
1.7.12.4

[RFC PATCH 1/4] acpi: property: Introduce helper acpi_dev_get_reference_device()

[Kefeng Wang]

Like of_parse_phandle() helper function to read and parse a phandle property
and return a pointer to the resulting device_node, introduce helper function
acpi_dev_get_reference_device() to read and parse a device properties(used in
_DSD method) and return a pointer to the resulting acpi_device.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/acpi/property.c | 23 +++++++++++++++++++++++
 include/linux/acpi.h    |  7 +++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 88f4306..e2e7754 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -398,6 +398,29 @@ int acpi_dev_get_property(struct acpi_device *adev, const char *name,
 }
 EXPORT_SYMBOL_GPL(acpi_dev_get_property);
 
+/**
+ * acpi_dev_get_reference_device  - return the acpi_device referenced
+ * @adev: ACPI device to get the property from.
+ * @name: Name of the property.
+ * @index: Index of the reference to return
+ *
+ * Returns referenced ACPI device pointer, or NULL if not found
+ */
+struct acpi_device *acpi_dev_get_reference_device(struct acpi_device *adev,
+						  const char *name, size_t index)
+{
+	struct acpi_reference_args args;
+	int ret;
+
+	ret = acpi_node_get_property_reference(acpi_fwnode_handle(adev), name, index, &args);
+
+	if (ret)
+		return NULL;
+
+	return args.adev;
+}
+EXPORT_SYMBOL(acpi_dev_get_reference_device);
+
 static struct acpi_device_data *acpi_device_data_of_node(struct fwnode_handle *fwnode)
 {
 	if (fwnode->type == FWNODE_ACPI) {
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 93811dc..d76a688 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -793,6 +793,8 @@ struct acpi_reference_args {
 #ifdef CONFIG_ACPI
 int acpi_dev_get_property(struct acpi_device *adev, const char *name,
 			  acpi_object_type type, const union acpi_object **obj);
+struct acpi_device *acpi_dev_get_reference_device(struct acpi_device *adev,
+						  const char *name, size_t index);
 int acpi_node_get_property_reference(struct fwnode_handle *fwnode,
 				     const char *name, size_t index,
 				     struct acpi_reference_args *args);
@@ -870,6 +872,11 @@ static inline int acpi_dev_get_property(struct acpi_device *adev,
 {
 	return -ENXIO;
 }
+static inline struct acpi_device *acpi_dev_get_reference_device(struct acpi_device *adev,
+						  const char *name, size_t index)
+{
+	return NULL;
+}
 
 static inline int acpi_node_get_property_reference(struct fwnode_handle *fwnode,
 				const char *name, const char *cells_name,
-- 
1.7.12.4

[RFC PATCH 1/4] acpi: property: Introduce helper acpi_dev_get_reference_device()

[Kefeng Wang]

Like of_parse_phandle() helper function to read and parse a phandle property
and return a pointer to the resulting device_node, introduce helper function
acpi_dev_get_reference_device() to read and parse a device properties(used in
_DSD method) and return a pointer to the resulting acpi_device.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/acpi/property.c | 23 +++++++++++++++++++++++
 include/linux/acpi.h    |  7 +++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 88f4306..e2e7754 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -398,6 +398,29 @@ int acpi_dev_get_property(struct acpi_device *adev, const char *name,
 }
 EXPORT_SYMBOL_GPL(acpi_dev_get_property);
 
+/**
+ * acpi_dev_get_reference_device  - return the acpi_device referenced
+ * @adev: ACPI device to get the property from.
+ * @name: Name of the property.
+ * @index: Index of the reference to return
+ *
+ * Returns referenced ACPI device pointer, or NULL if not found
+ */
+struct acpi_device *acpi_dev_get_reference_device(struct acpi_device *adev,
+						  const char *name, size_t index)
+{
+	struct acpi_reference_args args;
+	int ret;
+
+	ret = acpi_node_get_property_reference(acpi_fwnode_handle(adev), name, index, &args);
+
+	if (ret)
+		return NULL;
+
+	return args.adev;
+}
+EXPORT_SYMBOL(acpi_dev_get_reference_device);
+
 static struct acpi_device_data *acpi_device_data_of_node(struct fwnode_handle *fwnode)
 {
 	if (fwnode->type == FWNODE_ACPI) {
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 93811dc..d76a688 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -793,6 +793,8 @@ struct acpi_reference_args {
 #ifdef CONFIG_ACPI
 int acpi_dev_get_property(struct acpi_device *adev, const char *name,
 			  acpi_object_type type, const union acpi_object **obj);
+struct acpi_device *acpi_dev_get_reference_device(struct acpi_device *adev,
+						  const char *name, size_t index);
 int acpi_node_get_property_reference(struct fwnode_handle *fwnode,
 				     const char *name, size_t index,
 				     struct acpi_reference_args *args);
@@ -870,6 +872,11 @@ static inline int acpi_dev_get_property(struct acpi_device *adev,
 {
 	return -ENXIO;
 }
+static inline struct acpi_device *acpi_dev_get_reference_device(struct acpi_device *adev,
+						  const char *name, size_t index)
+{
+	return NULL;
+}
 
 static inline int acpi_node_get_property_reference(struct fwnode_handle *fwnode,
 				const char *name, const char *cells_name,
-- 
1.7.12.4

[RFC PATCH 2/4] device property: Introduce helper device_get_reference_node()

[Kefeng Wang]

With of_parse_phandle() and acpi_dev_get_reference_device(), we can introduce
a universal helper device_get_reference_node() to read and parse a device
property and return a pointer to the resulting firmware node.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/base/property.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/property.h |  6 ++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 1325ff2..6517140 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -568,6 +568,46 @@ struct fwnode_handle *device_get_next_child_node(struct device *dev,
 EXPORT_SYMBOL_GPL(device_get_next_child_node);
 
 /**
+ * fwnode_get_reference_node - Find the firmware node referenced
+ * @fwnode: Firmware node to get the property from.
+ * @propname: Name of the property
+ * @index: Index of the reference
+ *
+ * Returns referenced fwnode handler pointer, or an NULL if not found
+ */
+struct fwnode_handle *fwnode_get_reference_node(struct fwnode_handle *fwnode,
+					 const char *propname, int index)
+{
+	if (is_of_node(fwnode)) {
+		struct device_node *np;
+		np = of_parse_phandle(to_of_node(fwnode), propname, index);
+		return np ? &np->fwnode : NULL;
+	} else if (is_acpi_node(fwnode)) {
+		struct acpi_device *adev;
+		adev = acpi_dev_get_reference_device(to_acpi_device_node(fwnode),
+						     propname, index);
+		return adev ? acpi_fwnode_handle(adev) : NULL;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_reference_node);
+
+/**
+ * dev_get_reference_node - Find the firmware node referenced
+ * @dev: Device to get the property from.
+ * @propname: Name of the property
+ * @index: Index of the reference
+ *
+ * Returns referenced fwnode handler pointer, or an NULL if not found
+ */
+struct fwnode_handle *device_get_reference_node(struct device *dev,
+					 const char *propname, int index)
+{
+	return fwnode_get_reference_node(dev_fwnode(dev), propname, index);
+}
+EXPORT_SYMBOL_GPL(device_get_reference_node);
+
+/**
  * fwnode_handle_put - Drop reference to a device node
  * @fwnode: Pointer to the device node to drop the reference to.
  *
diff --git a/include/linux/property.h b/include/linux/property.h
index 0a3705a..d77e6a2 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -70,6 +70,12 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 int fwnode_property_match_string(struct fwnode_handle *fwnode,
 				 const char *propname, const char *string);
 
+struct fwnode_handle *fwnode_get_reference_node(struct fwnode_handle *fwnode,
+						const char *propname, int index);
+
+struct fwnode_handle *device_get_reference_node(struct device *dev,
+						const char *propname, int index);
+
 struct fwnode_handle *device_get_next_child_node(struct device *dev,
 						 struct fwnode_handle *child);
 
-- 
1.7.12.4

[RFC PATCH 2/4] device property: Introduce helper device_get_reference_node()

[Kefeng Wang]

With of_parse_phandle() and acpi_dev_get_reference_device(), we can introduce
a universal helper device_get_reference_node() to read and parse a device
property and return a pointer to the resulting firmware node.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/base/property.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/property.h |  6 ++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 1325ff2..6517140 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -568,6 +568,46 @@ struct fwnode_handle *device_get_next_child_node(struct device *dev,
 EXPORT_SYMBOL_GPL(device_get_next_child_node);
 
 /**
+ * fwnode_get_reference_node - Find the firmware node referenced
+ * @fwnode: Firmware node to get the property from.
+ * @propname: Name of the property
+ * @index: Index of the reference
+ *
+ * Returns referenced fwnode handler pointer, or an NULL if not found
+ */
+struct fwnode_handle *fwnode_get_reference_node(struct fwnode_handle *fwnode,
+					 const char *propname, int index)
+{
+	if (is_of_node(fwnode)) {
+		struct device_node *np;
+		np = of_parse_phandle(to_of_node(fwnode), propname, index);
+		return np ? &np->fwnode : NULL;
+	} else if (is_acpi_node(fwnode)) {
+		struct acpi_device *adev;
+		adev = acpi_dev_get_reference_device(to_acpi_device_node(fwnode),
+						     propname, index);
+		return adev ? acpi_fwnode_handle(adev) : NULL;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_reference_node);
+
+/**
+ * dev_get_reference_node - Find the firmware node referenced
+ * @dev: Device to get the property from.
+ * @propname: Name of the property
+ * @index: Index of the reference
+ *
+ * Returns referenced fwnode handler pointer, or an NULL if not found
+ */
+struct fwnode_handle *device_get_reference_node(struct device *dev,
+					 const char *propname, int index)
+{
+	return fwnode_get_reference_node(dev_fwnode(dev), propname, index);
+}
+EXPORT_SYMBOL_GPL(device_get_reference_node);
+
+/**
  * fwnode_handle_put - Drop reference to a device node
  * @fwnode: Pointer to the device node to drop the reference to.
  *
diff --git a/include/linux/property.h b/include/linux/property.h
index 0a3705a..d77e6a2 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -70,6 +70,12 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 int fwnode_property_match_string(struct fwnode_handle *fwnode,
 				 const char *propname, const char *string);
 
+struct fwnode_handle *fwnode_get_reference_node(struct fwnode_handle *fwnode,
+						const char *propname, int index);
+
+struct fwnode_handle *device_get_reference_node(struct device *dev,
+						const char *propname, int index);
+
 struct fwnode_handle *device_get_next_child_node(struct device *dev,
 						 struct fwnode_handle *child);
 
-- 
1.7.12.4

[RFC PATCH 3/4] ACPI/platform: Introduce helper acpi_dev_find_plat_dev()

[Kefeng Wang]

Sometimes in a device object, we refer to another device object,
mostly in _DSD method, and we need to get platform device with
the ACPI device node from the referenced device object. So introduce
the helper acpi_dev_find_plat_dev() to do this.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/acpi/acpi_platform.c | 25 +++++++++++++++++++++++++
 include/linux/acpi.h         |  6 ++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index 296b7a1..2603f9b 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -121,3 +121,28 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev)
 	return pdev;
 }
 EXPORT_SYMBOL_GPL(acpi_create_platform_device);
+
+static int acpi_dev_object_match(struct device *dev, void *data)
+{
+	return ACPI_COMPANION(dev) == data;
+}
+
+/*
+ * acpi_find_plat_device - Find the platform_device associated
+ * with an acpi device
+ * @adev: Pointer to the acpi device
+ *
+ * Returns platform_device pointer, or NULL if not found
+ */
+struct platform_device *acpi_dev_find_plat_dev(struct acpi_device *adev)
+{
+	struct device *dev;
+
+	if (!adev)
+		return NULL;
+
+	dev = bus_find_device(&platform_bus_type, NULL, adev,
+				acpi_dev_object_match);
+	return dev ? to_platform_device(dev) : NULL;
+}
+EXPORT_SYMBOL(acpi_dev_find_plat_dev);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index d76a688..16af22f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -468,6 +468,7 @@ int acpi_device_modalias(struct device *, char *, int);
 void acpi_walk_dep_device_list(acpi_handle handle);
 
 struct platform_device *acpi_create_platform_device(struct acpi_device *);
+struct platform_device *acpi_dev_find_plat_dev(struct acpi_device *adev);
 #define ACPI_PTR(_ptr)	(_ptr)
 
 #else	/* !CONFIG_ACPI */
@@ -929,6 +930,11 @@ static inline struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
 	return NULL;
 }
 
+static inline struct platform_device *acpi_dev_find_plat_dev(struct acpi_device *adev)
+{
+	return NULL;
+}
+
 #define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, validate, data, fn) \
 	static const void * __acpi_table_##name[]			\
 		__attribute__((unused))					\
-- 
1.7.12.4

[RFC PATCH 3/4] ACPI/platform: Introduce helper acpi_dev_find_plat_dev()

[Kefeng Wang]

Sometimes in a device object, we refer to another device object,
mostly in _DSD method, and we need to get platform device with
the ACPI device node from the referenced device object. So introduce
the helper acpi_dev_find_plat_dev() to do this.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/acpi/acpi_platform.c | 25 +++++++++++++++++++++++++
 include/linux/acpi.h         |  6 ++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index 296b7a1..2603f9b 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -121,3 +121,28 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev)
 	return pdev;
 }
 EXPORT_SYMBOL_GPL(acpi_create_platform_device);
+
+static int acpi_dev_object_match(struct device *dev, void *data)
+{
+	return ACPI_COMPANION(dev) == data;
+}
+
+/*
+ * acpi_find_plat_device - Find the platform_device associated
+ * with an acpi device
+ * @adev: Pointer to the acpi device
+ *
+ * Returns platform_device pointer, or NULL if not found
+ */
+struct platform_device *acpi_dev_find_plat_dev(struct acpi_device *adev)
+{
+	struct device *dev;
+
+	if (!adev)
+		return NULL;
+
+	dev = bus_find_device(&platform_bus_type, NULL, adev,
+				acpi_dev_object_match);
+	return dev ? to_platform_device(dev) : NULL;
+}
+EXPORT_SYMBOL(acpi_dev_find_plat_dev);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index d76a688..16af22f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -468,6 +468,7 @@ int acpi_device_modalias(struct device *, char *, int);
 void acpi_walk_dep_device_list(acpi_handle handle);
 
 struct platform_device *acpi_create_platform_device(struct acpi_device *);
+struct platform_device *acpi_dev_find_plat_dev(struct acpi_device *adev);
 #define ACPI_PTR(_ptr)	(_ptr)
 
 #else	/* !CONFIG_ACPI */
@@ -929,6 +930,11 @@ static inline struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
 	return NULL;
 }
 
+static inline struct platform_device *acpi_dev_find_plat_dev(struct acpi_device *adev)
+{
+	return NULL;
+}
+
 #define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, validate, data, fn) \
 	static const void * __acpi_table_##name[]			\
 		__attribute__((unused))					\
-- 
1.7.12.4

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Kefeng Wang]

This enables syscon with ACPI support.
syscon_regmap_lookup_by_dev_property() function was added. With helper
device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
in both DT and ACPI.

The device driver can obtain syscon using _DSD method in DSDT, an example
is shown below.

    Device(CTL0) {
          Name(_HID, "HISI0061")
          Name(_CRS, ResourceTemplate() {
                  Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
          })
    }

    Device(DEV0) {
          Name(_HID, "HISI00B1")
          Name(_CRS, ResourceTemplate() {
                  Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
                  Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
          })

          Name (_DSD, Package () {
              ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
              Package () {
                  Package () {"syscon",Package() {\_SB.CTL0} }
              }
          })
    }

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/mfd/syscon.c       | 50 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/syscon.h |  8 ++++++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 176bf0f..6f23418 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -12,6 +12,7 @@
  * (at your option) any later version.
  */
 
+#include <linux/acpi.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -21,6 +22,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_data/syscon.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>
 #include <linux/slab.h>
@@ -174,6 +176,47 @@ struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_phandle);
 
+struct regmap *syscon_regmap_lookup_by_dev_property(struct device *dev,
+						    const char *propname)
+{
+	struct fwnode_handle *fwnode;
+	struct regmap *regmap = NULL;
+
+	fwnode = device_get_reference_node(dev, propname, 0);
+
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+		struct device_node *syscon_np;
+		if (propname)
+			syscon_np = to_of_node(fwnode);
+		else
+			syscon_np = dev->of_node;
+		if (!syscon_np)
+			return ERR_PTR(-ENODEV);
+		regmap = syscon_node_to_regmap(syscon_np);
+		of_node_put(syscon_np);
+	} else if (IS_ENABLED(CONFIG_ACPI)) {
+		struct platform_device *pdev;
+		struct acpi_device *adev;
+		struct syscon *syscon;
+
+		adev = to_acpi_device_node(fwnode);
+		if (!adev)
+			return ERR_PTR(-ENODEV);
+
+		pdev = acpi_dev_find_plat_dev(adev);
+		if (!pdev)
+			return ERR_PTR(-ENODEV);
+
+		syscon = platform_get_drvdata(pdev);
+		if (!syscon)
+			return ERR_PTR(-ENODEV);
+
+		regmap = syscon->regmap;
+	}
+	return regmap;
+}
+EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_dev_property);
+
 static int syscon_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -216,9 +259,16 @@ static const struct platform_device_id syscon_ids[] = {
 	{ }
 };
 
+static const struct acpi_device_id syscon_acpi_match[] = {
+	{ "HISI0061", 0 }, /* better to use generic ACPI ID */
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, syscon_acpi_match);
+
 static struct platform_driver syscon_driver = {
 	.driver = {
 		.name = "syscon",
+		.acpi_match_table = ACPI_PTR(syscon_acpi_match),
 	},
 	.probe		= syscon_probe,
 	.id_table	= syscon_ids,
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index 75e543b..db79ca2 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -17,6 +17,7 @@
 
 #include <linux/err.h>
 
+struct device;
 struct device_node;
 
 #ifdef CONFIG_MFD_SYSCON
@@ -26,6 +27,8 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
 extern struct regmap *syscon_regmap_lookup_by_phandle(
 					struct device_node *np,
 					const char *property);
+extern struct regmap *syscon_regmap_lookup_by_dev_property(struct device *dev,
+						      const char *propname);
 #else
 static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
@@ -48,6 +51,11 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle(
 {
 	return ERR_PTR(-ENOSYS);
 }
+static inline struct regmap *syscon_regmap_lookup_by_dev_property(struct device *dev,
+						      const char *propname);
+{
+	return ERR_PTR(-ENOSYS);
+}
 #endif
 
 #endif /* __LINUX_MFD_SYSCON_H__ */
-- 
1.7.12.4

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Kefeng Wang]

This enables syscon with ACPI support.
syscon_regmap_lookup_by_dev_property() function was added. With helper
device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
in both DT and ACPI.

The device driver can obtain syscon using _DSD method in DSDT, an example
is shown below.

    Device(CTL0) {
          Name(_HID, "HISI0061")
          Name(_CRS, ResourceTemplate() {
                  Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
          })
    }

    Device(DEV0) {
          Name(_HID, "HISI00B1")
          Name(_CRS, ResourceTemplate() {
                  Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
                  Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
          })

          Name (_DSD, Package () {
              ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
              Package () {
                  Package () {"syscon",Package() {\_SB.CTL0} }
              }
          })
    }

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/mfd/syscon.c       | 50 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/syscon.h |  8 ++++++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 176bf0f..6f23418 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -12,6 +12,7 @@
  * (at your option) any later version.
  */
 
+#include <linux/acpi.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -21,6 +22,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_data/syscon.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>
 #include <linux/slab.h>
@@ -174,6 +176,47 @@ struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_phandle);
 
+struct regmap *syscon_regmap_lookup_by_dev_property(struct device *dev,
+						    const char *propname)
+{
+	struct fwnode_handle *fwnode;
+	struct regmap *regmap = NULL;
+
+	fwnode = device_get_reference_node(dev, propname, 0);
+
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+		struct device_node *syscon_np;
+		if (propname)
+			syscon_np = to_of_node(fwnode);
+		else
+			syscon_np = dev->of_node;
+		if (!syscon_np)
+			return ERR_PTR(-ENODEV);
+		regmap = syscon_node_to_regmap(syscon_np);
+		of_node_put(syscon_np);
+	} else if (IS_ENABLED(CONFIG_ACPI)) {
+		struct platform_device *pdev;
+		struct acpi_device *adev;
+		struct syscon *syscon;
+
+		adev = to_acpi_device_node(fwnode);
+		if (!adev)
+			return ERR_PTR(-ENODEV);
+
+		pdev = acpi_dev_find_plat_dev(adev);
+		if (!pdev)
+			return ERR_PTR(-ENODEV);
+
+		syscon = platform_get_drvdata(pdev);
+		if (!syscon)
+			return ERR_PTR(-ENODEV);
+
+		regmap = syscon->regmap;
+	}
+	return regmap;
+}
+EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_dev_property);
+
 static int syscon_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -216,9 +259,16 @@ static const struct platform_device_id syscon_ids[] = {
 	{ }
 };
 
+static const struct acpi_device_id syscon_acpi_match[] = {
+	{ "HISI0061", 0 }, /* better to use generic ACPI ID */
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, syscon_acpi_match);
+
 static struct platform_driver syscon_driver = {
 	.driver = {
 		.name = "syscon",
+		.acpi_match_table = ACPI_PTR(syscon_acpi_match),
 	},
 	.probe		= syscon_probe,
 	.id_table	= syscon_ids,
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index 75e543b..db79ca2 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -17,6 +17,7 @@
 
 #include <linux/err.h>
 
+struct device;
 struct device_node;
 
 #ifdef CONFIG_MFD_SYSCON
@@ -26,6 +27,8 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
 extern struct regmap *syscon_regmap_lookup_by_phandle(
 					struct device_node *np,
 					const char *property);
+extern struct regmap *syscon_regmap_lookup_by_dev_property(struct device *dev,
+						      const char *propname);
 #else
 static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
@@ -48,6 +51,11 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle(
 {
 	return ERR_PTR(-ENOSYS);
 }
+static inline struct regmap *syscon_regmap_lookup_by_dev_property(struct device *dev,
+						      const char *propname);
+{
+	return ERR_PTR(-ENOSYS);
+}
 #endif
 
 #endif /* __LINUX_MFD_SYSCON_H__ */
-- 
1.7.12.4

Re: [RFC PATCH 1/4] acpi: property: Introduce helper acpi_dev_get_reference_device()

[Mika Westerberg]

On Wed, Dec 02, 2015 at 05:09:25PM +0800, Kefeng Wang wrote:
> Like of_parse_phandle() helper function to read and parse a phandle property
> and return a pointer to the resulting device_node, introduce helper function
> acpi_dev_get_reference_device() to read and parse a device properties(used in
> _DSD method) and return a pointer to the resulting acpi_device.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/acpi/property.c | 23 +++++++++++++++++++++++
>  include/linux/acpi.h    |  7 +++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 88f4306..e2e7754 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -398,6 +398,29 @@ int acpi_dev_get_property(struct acpi_device *adev, const char *name,
>  }
>  EXPORT_SYMBOL_GPL(acpi_dev_get_property);
>  
> +/**
> + * acpi_dev_get_reference_device  - return the acpi_device referenced
> + * @adev: ACPI device to get the property from.
> + * @name: Name of the property.
> + * @index: Index of the reference to return
> + *
> + * Returns referenced ACPI device pointer, or NULL if not found
> + */
> +struct acpi_device *acpi_dev_get_reference_device(struct acpi_device *adev,
> +						  const char *name, size_t index)
> +{
> +	struct acpi_reference_args args;
> +	int ret;
> +
> +	ret = acpi_node_get_property_reference(acpi_fwnode_handle(adev), name, index, &args);
> +
> +	if (ret)
> +		return NULL;
> +
> +	return args.adev;
> +}
> +EXPORT_SYMBOL(acpi_dev_get_reference_device);

This wrapper looks pretty useless honestly. Why not use
acpi_node_get_property_reference() directly if you need to get the
device reference?

Also you should use EXPORT_SYMBOL_GPL() here.

[RFC PATCH 1/4] acpi: property: Introduce helper acpi_dev_get_reference_device()

[Mika Westerberg]

On Wed, Dec 02, 2015 at 05:09:25PM +0800, Kefeng Wang wrote:
> Like of_parse_phandle() helper function to read and parse a phandle property
> and return a pointer to the resulting device_node, introduce helper function
> acpi_dev_get_reference_device() to read and parse a device properties(used in
> _DSD method) and return a pointer to the resulting acpi_device.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/acpi/property.c | 23 +++++++++++++++++++++++
>  include/linux/acpi.h    |  7 +++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 88f4306..e2e7754 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -398,6 +398,29 @@ int acpi_dev_get_property(struct acpi_device *adev, const char *name,
>  }
>  EXPORT_SYMBOL_GPL(acpi_dev_get_property);
>  
> +/**
> + * acpi_dev_get_reference_device  - return the acpi_device referenced
> + * @adev: ACPI device to get the property from.
> + * @name: Name of the property.
> + * @index: Index of the reference to return
> + *
> + * Returns referenced ACPI device pointer, or NULL if not found
> + */
> +struct acpi_device *acpi_dev_get_reference_device(struct acpi_device *adev,
> +						  const char *name, size_t index)
> +{
> +	struct acpi_reference_args args;
> +	int ret;
> +
> +	ret = acpi_node_get_property_reference(acpi_fwnode_handle(adev), name, index, &args);
> +
> +	if (ret)
> +		return NULL;
> +
> +	return args.adev;
> +}
> +EXPORT_SYMBOL(acpi_dev_get_reference_device);

This wrapper looks pretty useless honestly. Why not use
acpi_node_get_property_reference() directly if you need to get the
device reference?

Also you should use EXPORT_SYMBOL_GPL() here.

Re: [RFC PATCH 4/4] mfd: syscon: add ACPI support

[Arnd Bergmann]

On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
> This enables syscon with ACPI support.
> syscon_regmap_lookup_by_dev_property() function was added. With helper
> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
> in both DT and ACPI.
> 
> The device driver can obtain syscon using _DSD method in DSDT, an example
> is shown below.
> 
>     Device(CTL0) {
>           Name(_HID, "HISI0061")
>           Name(_CRS, ResourceTemplate() {
>                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
>           })
>     }
> 
>     Device(DEV0) {
>           Name(_HID, "HISI00B1")
>           Name(_CRS, ResourceTemplate() {
>                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
>                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
>           })
> 
>           Name (_DSD, Package () {
>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>               Package () {
>                   Package () {"syscon",Package() {\_SB.CTL0} }
>               }
>           })
>     }
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

This sounds like a bad idea:

syscon is basically a hack to let us access register that the SoC designer
couldn't fit in anywhere sane. We need something like this with devicetree
because we decided not to have any interpreted bytecode to do this behind
our back.

With ACPI, the same thing is done with AML, which is actually nicer than
syscon (once you have to deal with all the problems introduced by AML).

Use that instead.

> +               pdev = acpi_dev_find_plat_dev(adev);
> +               if (!pdev)
> +                       return ERR_PTR(-ENODEV);
> +               syscon = platform_get_drvdata(pdev);

This still requires the syscon device to be probed first, which is
something we shouldn't really need any more.

A lot of syscon users depend on the regmap to be available before
device drivers are loaded, so we have changed the code to just look up
the device_node that is already present and ignore the device.

Once clps711x has been converted to DT, I think we can rip out the
syscon_regmap_lookup_by_pdevname() interface and separate the syscon
regmap handling from the device handling that we only really need
for debugfs.

> @@ -216,9 +259,16 @@ static const struct platform_device_id syscon_ids[] = {
>  	{ }
>  };
>  
> +static const struct acpi_device_id syscon_acpi_match[] = {
> +	{ "HISI0061", 0 }, /* better to use generic ACPI ID */
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, syscon_acpi_match);
> +
>  static struct platform_driver syscon_driver = {
>  	.driver = {
>  		.name = "syscon",
> +		.acpi_match_table = ACPI_PTR(syscon_acpi_match),
>  	},
>  	.probe		= syscon_probe,
>  	.id_table	= syscon_ids,

We certainly don't want to list specific devices, so the other problem
aside, we wouldn't merge it until there is an official ACPI way to
specify "random crap register areas" that does not depend on a vendor
specific ID.

	Arnd

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Arnd Bergmann]

On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
> This enables syscon with ACPI support.
> syscon_regmap_lookup_by_dev_property() function was added. With helper
> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
> in both DT and ACPI.
> 
> The device driver can obtain syscon using _DSD method in DSDT, an example
> is shown below.
> 
>     Device(CTL0) {
>           Name(_HID, "HISI0061")
>           Name(_CRS, ResourceTemplate() {
>                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
>           })
>     }
> 
>     Device(DEV0) {
>           Name(_HID, "HISI00B1")
>           Name(_CRS, ResourceTemplate() {
>                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
>                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
>           })
> 
>           Name (_DSD, Package () {
>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>               Package () {
>                   Package () {"syscon",Package() {\_SB.CTL0} }
>               }
>           })
>     }
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

This sounds like a bad idea:

syscon is basically a hack to let us access register that the SoC designer
couldn't fit in anywhere sane. We need something like this with devicetree
because we decided not to have any interpreted bytecode to do this behind
our back.

With ACPI, the same thing is done with AML, which is actually nicer than
syscon (once you have to deal with all the problems introduced by AML).

Use that instead.

> +               pdev = acpi_dev_find_plat_dev(adev);
> +               if (!pdev)
> +                       return ERR_PTR(-ENODEV);
> +               syscon = platform_get_drvdata(pdev);

This still requires the syscon device to be probed first, which is
something we shouldn't really need any more.

A lot of syscon users depend on the regmap to be available before
device drivers are loaded, so we have changed the code to just look up
the device_node that is already present and ignore the device.

Once clps711x has been converted to DT, I think we can rip out the
syscon_regmap_lookup_by_pdevname() interface and separate the syscon
regmap handling from the device handling that we only really need
for debugfs.

> @@ -216,9 +259,16 @@ static const struct platform_device_id syscon_ids[] = {
>  	{ }
>  };
>  
> +static const struct acpi_device_id syscon_acpi_match[] = {
> +	{ "HISI0061", 0 }, /* better to use generic ACPI ID */
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, syscon_acpi_match);
> +
>  static struct platform_driver syscon_driver = {
>  	.driver = {
>  		.name = "syscon",
> +		.acpi_match_table = ACPI_PTR(syscon_acpi_match),
>  	},
>  	.probe		= syscon_probe,
>  	.id_table	= syscon_ids,

We certainly don't want to list specific devices, so the other problem
aside, we wouldn't merge it until there is an official ACPI way to
specify "random crap register areas" that does not depend on a vendor
specific ID.

	Arnd

Re: [RFC PATCH 0/4] add ACPI support for syscon

[Lorenzo Pieralisi]

On Wed, Dec 02, 2015 at 05:09:24PM +0800, Kefeng Wang wrote:
> Lots of drivers begin to support both OF and ACPI now, but some drivers depend on syscon,

Define "lots of drivers" please and in the process point me at what
they are, thanks.

> so syscon need to support ACPI firstly.

I do not think so, that's exactly what I do NOT want to happen, namely
rewriting DT management in ACPI _entirely_.

The rules for using _DSD properties are described here:

https://lists.acpica.org/pipermail/dsd/2015-September/000026.html

So, first thing, tell us why you need this patchset and what's its
aim (ie using syscon to control what ?).

Thanks,
Lorenzo

> Add ACPI support for syscon, and introduce syscon_regmap_lookup_by_dev_property()
> helper when the driver need to get a regmap handle from syscon, it can used in
> both OF and ACPI.
> 
> Kefeng Wang (4):
>   acpi: property: Introduce helper acpi_dev_get_reference_device()
>   device property: Introduce helper device_get_reference_node()
>   ACPI/platform: Introduce helper acpi_dev_find_plat_dev()
>   mfd: syscon: add ACPI support
> 
>  drivers/acpi/acpi_platform.c | 25 ++++++++++++++++++++++
>  drivers/acpi/property.c      | 23 ++++++++++++++++++++
>  drivers/base/property.c      | 40 +++++++++++++++++++++++++++++++++++
>  drivers/mfd/syscon.c         | 50 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/acpi.h         | 13 ++++++++++++
>  include/linux/mfd/syscon.h   |  8 +++++++
>  include/linux/property.h     |  6 ++++++
>  7 files changed, 165 insertions(+)
> 
> -- 
> 1.7.12.4
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC PATCH 0/4] add ACPI support for syscon

[Lorenzo Pieralisi]

On Wed, Dec 02, 2015 at 05:09:24PM +0800, Kefeng Wang wrote:
> Lots of drivers begin to support both OF and ACPI now, but some drivers depend on syscon,

Define "lots of drivers" please and in the process point me at what
they are, thanks.

> so syscon need to support ACPI firstly.

I do not think so, that's exactly what I do NOT want to happen, namely
rewriting DT management in ACPI _entirely_.

The rules for using _DSD properties are described here:

https://lists.acpica.org/pipermail/dsd/2015-September/000026.html

So, first thing, tell us why you need this patchset and what's its
aim (ie using syscon to control what ?).

Thanks,
Lorenzo

> Add ACPI support for syscon, and introduce syscon_regmap_lookup_by_dev_property()
> helper when the driver need to get a regmap handle from syscon, it can used in
> both OF and ACPI.
> 
> Kefeng Wang (4):
>   acpi: property: Introduce helper acpi_dev_get_reference_device()
>   device property: Introduce helper device_get_reference_node()
>   ACPI/platform: Introduce helper acpi_dev_find_plat_dev()
>   mfd: syscon: add ACPI support
> 
>  drivers/acpi/acpi_platform.c | 25 ++++++++++++++++++++++
>  drivers/acpi/property.c      | 23 ++++++++++++++++++++
>  drivers/base/property.c      | 40 +++++++++++++++++++++++++++++++++++
>  drivers/mfd/syscon.c         | 50 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/acpi.h         | 13 ++++++++++++
>  include/linux/mfd/syscon.h   |  8 +++++++
>  include/linux/property.h     |  6 ++++++
>  7 files changed, 165 insertions(+)
> 
> -- 
> 1.7.12.4
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [RFC PATCH 4/4] mfd: syscon: add ACPI support

[Graeme Gregory]

On Wed, Dec 02, 2015 at 11:44:51AM +0100, Arnd Bergmann wrote:
> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
> > This enables syscon with ACPI support.
> > syscon_regmap_lookup_by_dev_property() function was added. With helper
> > device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
> > in both DT and ACPI.
> > 
> > The device driver can obtain syscon using _DSD method in DSDT, an example
> > is shown below.
> > 
> >     Device(CTL0) {
> >           Name(_HID, "HISI0061")
> >           Name(_CRS, ResourceTemplate() {
> >                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
> >           })
> >     }
> > 
> >     Device(DEV0) {
> >           Name(_HID, "HISI00B1")
> >           Name(_CRS, ResourceTemplate() {
> >                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
> >                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
> >           })
> > 
> >           Name (_DSD, Package () {
> >               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> >               Package () {
> >                   Package () {"syscon",Package() {\_SB.CTL0} }
> >               }
> >           })
> >     }
> > 
> > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> 
> This sounds like a bad idea:
> 
> syscon is basically a hack to let us access register that the SoC designer
> couldn't fit in anywhere sane. We need something like this with devicetree
> because we decided not to have any interpreted bytecode to do this behind
> our back.
> 
> With ACPI, the same thing is done with AML, which is actually nicer than
> syscon (once you have to deal with all the problems introduced by AML).
> 
> Use that instead.
> 

I have to agree with Arnd here, this is specifically why it was chosen
to use ACPI on machines to move all these "hacks" to AML.

This leaves your driver being generic and any "special" initialisation
can be supplied by the OEM through the ACPI tables.

Graeme

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Graeme Gregory]

On Wed, Dec 02, 2015 at 11:44:51AM +0100, Arnd Bergmann wrote:
> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
> > This enables syscon with ACPI support.
> > syscon_regmap_lookup_by_dev_property() function was added. With helper
> > device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
> > in both DT and ACPI.
> > 
> > The device driver can obtain syscon using _DSD method in DSDT, an example
> > is shown below.
> > 
> >     Device(CTL0) {
> >           Name(_HID, "HISI0061")
> >           Name(_CRS, ResourceTemplate() {
> >                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
> >           })
> >     }
> > 
> >     Device(DEV0) {
> >           Name(_HID, "HISI00B1")
> >           Name(_CRS, ResourceTemplate() {
> >                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
> >                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
> >           })
> > 
> >           Name (_DSD, Package () {
> >               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> >               Package () {
> >                   Package () {"syscon",Package() {\_SB.CTL0} }
> >               }
> >           })
> >     }
> > 
> > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> 
> This sounds like a bad idea:
> 
> syscon is basically a hack to let us access register that the SoC designer
> couldn't fit in anywhere sane. We need something like this with devicetree
> because we decided not to have any interpreted bytecode to do this behind
> our back.
> 
> With ACPI, the same thing is done with AML, which is actually nicer than
> syscon (once you have to deal with all the problems introduced by AML).
> 
> Use that instead.
> 

I have to agree with Arnd here, this is specifically why it was chosen
to use ACPI on machines to move all these "hacks" to AML.

This leaves your driver being generic and any "special" initialisation
can be supplied by the OEM through the ACPI tables.

Graeme

Re: [RFC PATCH 4/4] mfd: syscon: add ACPI support

[Kefeng Wang]

Hi Graeme, Arnd, and Lorenzo,

Firstly, we absolutely agree with the point which use AML to do some "special"
initialisation and configuration.

SAS and NIC driver were accepted by linux in hisilicon hip05 chip, and the drivers
reset the control by syscon, we want to use "_RST" method, which is introduced by
ACPI 6.0 spec in "7.3.25 _RST (Device Reset)", is it reasonable and standard for us?

But here is a scene, we can not find a suitable way to support ACPI. There is no
independent memory region in some module(the driver not upstreamed), that is,
when write and read the module's register, we must r/w by syscon. Any advice?

Thanks,
Kefeng

On 2015/12/3 18:41, Graeme Gregory wrote:
> On Wed, Dec 02, 2015 at 11:44:51AM +0100, Arnd Bergmann wrote:
>> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
>>> This enables syscon with ACPI support.
>>> syscon_regmap_lookup_by_dev_property() function was added. With helper
>>> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
>>> in both DT and ACPI.
>>>
>>> The device driver can obtain syscon using _DSD method in DSDT, an example
>>> is shown below.
>>>
>>>     Device(CTL0) {
>>>           Name(_HID, "HISI0061")
>>>           Name(_CRS, ResourceTemplate() {
>>>                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
>>>           })
>>>     }
>>>
>>>     Device(DEV0) {
>>>           Name(_HID, "HISI00B1")
>>>           Name(_CRS, ResourceTemplate() {
>>>                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
>>>                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
>>>           })
>>>
>>>           Name (_DSD, Package () {
>>>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>>               Package () {
>>>                   Package () {"syscon",Package() {\_SB.CTL0} }
>>>               }
>>>           })
>>>     }
>>>
>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>
>> This sounds like a bad idea:
>>
>> syscon is basically a hack to let us access register that the SoC designer
>> couldn't fit in anywhere sane. We need something like this with devicetree
>> because we decided not to have any interpreted bytecode to do this behind
>> our back.
>>
>> With ACPI, the same thing is done with AML, which is actually nicer than
>> syscon (once you have to deal with all the problems introduced by AML).
>>
>> Use that instead.
>>
> 
> I have to agree with Arnd here, this is specifically why it was chosen
> to use ACPI on machines to move all these "hacks" to AML.
> 
> This leaves your driver being generic and any "special" initialisation
> can be supplied by the OEM through the ACPI tables.
> 
> Graeme
> 
> 
> .
> 

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Kefeng Wang]

Hi Graeme, Arnd, and Lorenzo,

Firstly, we absolutely agree with the point which use AML to do some "special"
initialisation and configuration.

SAS and NIC driver were accepted by linux in hisilicon hip05 chip, and the drivers
reset the control by syscon, we want to use "_RST" method, which is introduced by
ACPI 6.0 spec in "7.3.25 _RST (Device Reset)", is it reasonable and standard for us?

But here is a scene, we can not find a suitable way to support ACPI. There is no
independent memory region in some module(the driver not upstreamed), that is,
when write and read the module's register, we must r/w by syscon. Any advice?

Thanks,
Kefeng

On 2015/12/3 18:41, Graeme Gregory wrote:
> On Wed, Dec 02, 2015 at 11:44:51AM +0100, Arnd Bergmann wrote:
>> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
>>> This enables syscon with ACPI support.
>>> syscon_regmap_lookup_by_dev_property() function was added. With helper
>>> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
>>> in both DT and ACPI.
>>>
>>> The device driver can obtain syscon using _DSD method in DSDT, an example
>>> is shown below.
>>>
>>>     Device(CTL0) {
>>>           Name(_HID, "HISI0061")
>>>           Name(_CRS, ResourceTemplate() {
>>>                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
>>>           })
>>>     }
>>>
>>>     Device(DEV0) {
>>>           Name(_HID, "HISI00B1")
>>>           Name(_CRS, ResourceTemplate() {
>>>                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
>>>                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
>>>           })
>>>
>>>           Name (_DSD, Package () {
>>>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>>               Package () {
>>>                   Package () {"syscon",Package() {\_SB.CTL0} }
>>>               }
>>>           })
>>>     }
>>>
>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>
>> This sounds like a bad idea:
>>
>> syscon is basically a hack to let us access register that the SoC designer
>> couldn't fit in anywhere sane. We need something like this with devicetree
>> because we decided not to have any interpreted bytecode to do this behind
>> our back.
>>
>> With ACPI, the same thing is done with AML, which is actually nicer than
>> syscon (once you have to deal with all the problems introduced by AML).
>>
>> Use that instead.
>>
> 
> I have to agree with Arnd here, this is specifically why it was chosen
> to use ACPI on machines to move all these "hacks" to AML.
> 
> This leaves your driver being generic and any "special" initialisation
> can be supplied by the OEM through the ACPI tables.
> 
> Graeme
> 
> 
> .
> 

Re: [RFC PATCH 2/4] device property: Introduce helper device_get_reference_node()

[Russell King - ARM Linux]

On Wed, Dec 02, 2015 at 05:09:26PM +0800, Kefeng Wang wrote:
> With of_parse_phandle() and acpi_dev_get_reference_device(), we can introduce
> a universal helper device_get_reference_node() to read and parse a device
> property and return a pointer to the resulting firmware node.

What's happening to make this fwnode stuff actually usable?  Whenever
I've looked at it from the DT perspective, it looks very much like a
half-baked train wreck - although the struct device_node contains a
fwnode, of_node_init() initialises it partially, and we have a way to
convert _from_ a fwnode to a device_node (to_of_node), nothing sets
the struct device fwnode pointer.

Whenever I've looked at this, it's always led me to the conclusion
that the way that fwnode stuff has currently been written, I'm
supposed to get the device_node, and then use the embedded fwnode
directly, which I'm pretty sure misses the point of fwnode (as it
means any driver code making use of this is tied to DT.)

This patch seems to confirm my suspicions that it's just wrong -
trying to use device_get_reference_node() here will fail in a DT
based setup, because (I assume) dev_fwnode(dev) returns dev->fwnode
which is NULL there.

I don't know, maybe there is an intention that fwnode should not be
used for DT?

Maybe someone who knows what the intentions are behind this fwnode stuff
can enlighten the situation, and maybe sort out this apparent oversight
which IMHO should've been spotted in the initial fwnode review?

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[RFC PATCH 2/4] device property: Introduce helper device_get_reference_node()

[Russell King - ARM Linux]

On Wed, Dec 02, 2015 at 05:09:26PM +0800, Kefeng Wang wrote:
> With of_parse_phandle() and acpi_dev_get_reference_device(), we can introduce
> a universal helper device_get_reference_node() to read and parse a device
> property and return a pointer to the resulting firmware node.

What's happening to make this fwnode stuff actually usable?  Whenever
I've looked at it from the DT perspective, it looks very much like a
half-baked train wreck - although the struct device_node contains a
fwnode, of_node_init() initialises it partially, and we have a way to
convert _from_ a fwnode to a device_node (to_of_node), nothing sets
the struct device fwnode pointer.

Whenever I've looked at this, it's always led me to the conclusion
that the way that fwnode stuff has currently been written, I'm
supposed to get the device_node, and then use the embedded fwnode
directly, which I'm pretty sure misses the point of fwnode (as it
means any driver code making use of this is tied to DT.)

This patch seems to confirm my suspicions that it's just wrong -
trying to use device_get_reference_node() here will fail in a DT
based setup, because (I assume) dev_fwnode(dev) returns dev->fwnode
which is NULL there.

I don't know, maybe there is an intention that fwnode should not be
used for DT?

Maybe someone who knows what the intentions are behind this fwnode stuff
can enlighten the situation, and maybe sort out this apparent oversight
which IMHO should've been spotted in the initial fwnode review?

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [RFC PATCH 4/4] mfd: syscon: add ACPI support

[Lorenzo Pieralisi]

On Thu, Dec 03, 2015 at 09:01:11PM +0800, Kefeng Wang wrote:
> Hi Graeme, Arnd, and Lorenzo,
> 
> Firstly, we absolutely agree with the point which use AML to do some "special"
> initialisation and configuration.

Good.

> SAS and NIC driver were accepted by linux in hisilicon hip05 chip, and the drivers
> reset the control by syscon, we want to use "_RST" method, which is introduced by
> ACPI 6.0 spec in "7.3.25 _RST (Device Reset)", is it reasonable and standard for us?

Can you point me at the drivers you are referring to please ?

> But here is a scene, we can not find a suitable way to support ACPI. There is no
> independent memory region in some module(the driver not upstreamed), that is,
> when write and read the module's register, we must r/w by syscon. Any advice?

What do you mean ? You mean that the reset control is a piece of HW
that is shared between multiple "components" ? What's your concern
about AML code driving those registers here ?

Thanks,
Lorenzo

> 
> Thanks,
> Kefeng
> 
> On 2015/12/3 18:41, Graeme Gregory wrote:
> > On Wed, Dec 02, 2015 at 11:44:51AM +0100, Arnd Bergmann wrote:
> >> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
> >>> This enables syscon with ACPI support.
> >>> syscon_regmap_lookup_by_dev_property() function was added. With helper
> >>> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
> >>> in both DT and ACPI.
> >>>
> >>> The device driver can obtain syscon using _DSD method in DSDT, an example
> >>> is shown below.
> >>>
> >>>     Device(CTL0) {
> >>>           Name(_HID, "HISI0061")
> >>>           Name(_CRS, ResourceTemplate() {
> >>>                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
> >>>           })
> >>>     }
> >>>
> >>>     Device(DEV0) {
> >>>           Name(_HID, "HISI00B1")
> >>>           Name(_CRS, ResourceTemplate() {
> >>>                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
> >>>                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
> >>>           })
> >>>
> >>>           Name (_DSD, Package () {
> >>>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> >>>               Package () {
> >>>                   Package () {"syscon",Package() {\_SB.CTL0} }
> >>>               }
> >>>           })
> >>>     }
> >>>
> >>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> >>
> >> This sounds like a bad idea:
> >>
> >> syscon is basically a hack to let us access register that the SoC designer
> >> couldn't fit in anywhere sane. We need something like this with devicetree
> >> because we decided not to have any interpreted bytecode to do this behind
> >> our back.
> >>
> >> With ACPI, the same thing is done with AML, which is actually nicer than
> >> syscon (once you have to deal with all the problems introduced by AML).
> >>
> >> Use that instead.
> >>
> > 
> > I have to agree with Arnd here, this is specifically why it was chosen
> > to use ACPI on machines to move all these "hacks" to AML.
> > 
> > This leaves your driver being generic and any "special" initialisation
> > can be supplied by the OEM through the ACPI tables.
> > 
> > Graeme
> > 
> > 
> > .
> > 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Lorenzo Pieralisi]

On Thu, Dec 03, 2015 at 09:01:11PM +0800, Kefeng Wang wrote:
> Hi Graeme, Arnd, and Lorenzo,
> 
> Firstly, we absolutely agree with the point which use AML to do some "special"
> initialisation and configuration.

Good.

> SAS and NIC driver were accepted by linux in hisilicon hip05 chip, and the drivers
> reset the control by syscon, we want to use "_RST" method, which is introduced by
> ACPI 6.0 spec in "7.3.25 _RST (Device Reset)", is it reasonable and standard for us?

Can you point me at the drivers you are referring to please ?

> But here is a scene, we can not find a suitable way to support ACPI. There is no
> independent memory region in some module(the driver not upstreamed), that is,
> when write and read the module's register, we must r/w by syscon. Any advice?

What do you mean ? You mean that the reset control is a piece of HW
that is shared between multiple "components" ? What's your concern
about AML code driving those registers here ?

Thanks,
Lorenzo

> 
> Thanks,
> Kefeng
> 
> On 2015/12/3 18:41, Graeme Gregory wrote:
> > On Wed, Dec 02, 2015 at 11:44:51AM +0100, Arnd Bergmann wrote:
> >> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
> >>> This enables syscon with ACPI support.
> >>> syscon_regmap_lookup_by_dev_property() function was added. With helper
> >>> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
> >>> in both DT and ACPI.
> >>>
> >>> The device driver can obtain syscon using _DSD method in DSDT, an example
> >>> is shown below.
> >>>
> >>>     Device(CTL0) {
> >>>           Name(_HID, "HISI0061")
> >>>           Name(_CRS, ResourceTemplate() {
> >>>                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
> >>>           })
> >>>     }
> >>>
> >>>     Device(DEV0) {
> >>>           Name(_HID, "HISI00B1")
> >>>           Name(_CRS, ResourceTemplate() {
> >>>                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
> >>>                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
> >>>           })
> >>>
> >>>           Name (_DSD, Package () {
> >>>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> >>>               Package () {
> >>>                   Package () {"syscon",Package() {\_SB.CTL0} }
> >>>               }
> >>>           })
> >>>     }
> >>>
> >>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> >>
> >> This sounds like a bad idea:
> >>
> >> syscon is basically a hack to let us access register that the SoC designer
> >> couldn't fit in anywhere sane. We need something like this with devicetree
> >> because we decided not to have any interpreted bytecode to do this behind
> >> our back.
> >>
> >> With ACPI, the same thing is done with AML, which is actually nicer than
> >> syscon (once you have to deal with all the problems introduced by AML).
> >>
> >> Use that instead.
> >>
> > 
> > I have to agree with Arnd here, this is specifically why it was chosen
> > to use ACPI on machines to move all these "hacks" to AML.
> > 
> > This leaves your driver being generic and any "special" initialisation
> > can be supplied by the OEM through the ACPI tables.
> > 
> > Graeme
> > 
> > 
> > .
> > 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [RFC PATCH 2/4] device property: Introduce helper device_get_reference_node()

[Rafael J. Wysocki]

On Thursday, December 03, 2015 03:28:42 PM Russell King - ARM Linux wrote:
> On Wed, Dec 02, 2015 at 05:09:26PM +0800, Kefeng Wang wrote:
> > With of_parse_phandle() and acpi_dev_get_reference_device(), we can introduce
> > a universal helper device_get_reference_node() to read and parse a device
> > property and return a pointer to the resulting firmware node.
> 
> What's happening to make this fwnode stuff actually usable?  Whenever
> I've looked at it from the DT perspective, it looks very much like a
> half-baked train wreck - although the struct device_node contains a
> fwnode, of_node_init() initialises it partially, and we have a way to
> convert _from_ a fwnode to a device_node (to_of_node), nothing sets
> the struct device fwnode pointer.
> 
> Whenever I've looked at this, it's always led me to the conclusion
> that the way that fwnode stuff has currently been written, I'm
> supposed to get the device_node, and then use the embedded fwnode
> directly, which I'm pretty sure misses the point of fwnode (as it
> means any driver code making use of this is tied to DT.)
> 
> This patch seems to confirm my suspicions that it's just wrong -
> trying to use device_get_reference_node() here will fail in a DT
> based setup, because (I assume) dev_fwnode(dev) returns dev->fwnode
> which is NULL there.
> 
> I don't know, maybe there is an intention that fwnode should not be
> used for DT?

No, there isn't.

> Maybe someone who knows what the intentions are behind this fwnode stuff
> can enlighten the situation, and maybe sort out this apparent oversight
> which IMHO should've been spotted in the initial fwnode review?

At that time there was a plan (or maybe just and intention, I'm not sure to
be honest) to introduce and accessor macro for the of_node pointer in struct
device and make the users of it access it via that macro, which then would
make it possible to switch over to using dev->fwnode in the DT case (by
changing the implementation of that macro).

That hasn't materialized as clearly visible, but still can be done AFAICS,
although finding a volunteer for that work may be hard I suppose.

Thanks,
Rafael

[RFC PATCH 2/4] device property: Introduce helper device_get_reference_node()

[Rafael J. Wysocki]

On Thursday, December 03, 2015 03:28:42 PM Russell King - ARM Linux wrote:
> On Wed, Dec 02, 2015 at 05:09:26PM +0800, Kefeng Wang wrote:
> > With of_parse_phandle() and acpi_dev_get_reference_device(), we can introduce
> > a universal helper device_get_reference_node() to read and parse a device
> > property and return a pointer to the resulting firmware node.
> 
> What's happening to make this fwnode stuff actually usable?  Whenever
> I've looked at it from the DT perspective, it looks very much like a
> half-baked train wreck - although the struct device_node contains a
> fwnode, of_node_init() initialises it partially, and we have a way to
> convert _from_ a fwnode to a device_node (to_of_node), nothing sets
> the struct device fwnode pointer.
> 
> Whenever I've looked at this, it's always led me to the conclusion
> that the way that fwnode stuff has currently been written, I'm
> supposed to get the device_node, and then use the embedded fwnode
> directly, which I'm pretty sure misses the point of fwnode (as it
> means any driver code making use of this is tied to DT.)
> 
> This patch seems to confirm my suspicions that it's just wrong -
> trying to use device_get_reference_node() here will fail in a DT
> based setup, because (I assume) dev_fwnode(dev) returns dev->fwnode
> which is NULL there.
> 
> I don't know, maybe there is an intention that fwnode should not be
> used for DT?

No, there isn't.

> Maybe someone who knows what the intentions are behind this fwnode stuff
> can enlighten the situation, and maybe sort out this apparent oversight
> which IMHO should've been spotted in the initial fwnode review?

At that time there was a plan (or maybe just and intention, I'm not sure to
be honest) to introduce and accessor macro for the of_node pointer in struct
device and make the users of it access it via that macro, which then would
make it possible to switch over to using dev->fwnode in the DT case (by
changing the implementation of that macro).

That hasn't materialized as clearly visible, but still can be done AFAICS,
although finding a volunteer for that work may be hard I suppose.

Thanks,
Rafael

Re: [RFC PATCH 2/4] device property: Introduce helper device_get_reference_node()

[Rafael J. Wysocki]

On Friday, December 04, 2015 12:29:12 AM Rafael J. Wysocki wrote:
> On Thursday, December 03, 2015 03:28:42 PM Russell King - ARM Linux wrote:
> > On Wed, Dec 02, 2015 at 05:09:26PM +0800, Kefeng Wang wrote:
> > > With of_parse_phandle() and acpi_dev_get_reference_device(), we can introduce
> > > a universal helper device_get_reference_node() to read and parse a device
> > > property and return a pointer to the resulting firmware node.
> > 
> > What's happening to make this fwnode stuff actually usable?  Whenever
> > I've looked at it from the DT perspective, it looks very much like a
> > half-baked train wreck - although the struct device_node contains a
> > fwnode, of_node_init() initialises it partially, and we have a way to
> > convert _from_ a fwnode to a device_node (to_of_node), nothing sets
> > the struct device fwnode pointer.
> > 
> > Whenever I've looked at this, it's always led me to the conclusion
> > that the way that fwnode stuff has currently been written, I'm
> > supposed to get the device_node, and then use the embedded fwnode
> > directly, which I'm pretty sure misses the point of fwnode (as it
> > means any driver code making use of this is tied to DT.)
> > 
> > This patch seems to confirm my suspicions that it's just wrong -
> > trying to use device_get_reference_node() here will fail in a DT
> > based setup, because (I assume) dev_fwnode(dev) returns dev->fwnode
> > which is NULL there.
> > 
> > I don't know, maybe there is an intention that fwnode should not be
> > used for DT?
> 
> No, there isn't.
> 
> > Maybe someone who knows what the intentions are behind this fwnode stuff
> > can enlighten the situation, and maybe sort out this apparent oversight
> > which IMHO should've been spotted in the initial fwnode review?
> 
> At that time there was a plan (or maybe just and intention, I'm not sure to
> be honest) to introduce and accessor macro for the of_node pointer in struct
> device and make the users of it access it via that macro, which then would
> make it possible to switch over to using dev->fwnode in the DT case (by
> changing the implementation of that macro).
> 
> That hasn't materialized as clearly visible, but still can be done AFAICS,
> although finding a volunteer for that work may be hard I suppose.

That said if there are real problems with this (other than aesthetics),
I actually may go off and do it, but I can't promise that to happen soon.

Thanks,
Rafael

[RFC PATCH 2/4] device property: Introduce helper device_get_reference_node()

[Rafael J. Wysocki]

On Friday, December 04, 2015 12:29:12 AM Rafael J. Wysocki wrote:
> On Thursday, December 03, 2015 03:28:42 PM Russell King - ARM Linux wrote:
> > On Wed, Dec 02, 2015 at 05:09:26PM +0800, Kefeng Wang wrote:
> > > With of_parse_phandle() and acpi_dev_get_reference_device(), we can introduce
> > > a universal helper device_get_reference_node() to read and parse a device
> > > property and return a pointer to the resulting firmware node.
> > 
> > What's happening to make this fwnode stuff actually usable?  Whenever
> > I've looked at it from the DT perspective, it looks very much like a
> > half-baked train wreck - although the struct device_node contains a
> > fwnode, of_node_init() initialises it partially, and we have a way to
> > convert _from_ a fwnode to a device_node (to_of_node), nothing sets
> > the struct device fwnode pointer.
> > 
> > Whenever I've looked at this, it's always led me to the conclusion
> > that the way that fwnode stuff has currently been written, I'm
> > supposed to get the device_node, and then use the embedded fwnode
> > directly, which I'm pretty sure misses the point of fwnode (as it
> > means any driver code making use of this is tied to DT.)
> > 
> > This patch seems to confirm my suspicions that it's just wrong -
> > trying to use device_get_reference_node() here will fail in a DT
> > based setup, because (I assume) dev_fwnode(dev) returns dev->fwnode
> > which is NULL there.
> > 
> > I don't know, maybe there is an intention that fwnode should not be
> > used for DT?
> 
> No, there isn't.
> 
> > Maybe someone who knows what the intentions are behind this fwnode stuff
> > can enlighten the situation, and maybe sort out this apparent oversight
> > which IMHO should've been spotted in the initial fwnode review?
> 
> At that time there was a plan (or maybe just and intention, I'm not sure to
> be honest) to introduce and accessor macro for the of_node pointer in struct
> device and make the users of it access it via that macro, which then would
> make it possible to switch over to using dev->fwnode in the DT case (by
> changing the implementation of that macro).
> 
> That hasn't materialized as clearly visible, but still can be done AFAICS,
> although finding a volunteer for that work may be hard I suppose.

That said if there are real problems with this (other than aesthetics),
I actually may go off and do it, but I can't promise that to happen soon.

Thanks,
Rafael

Re: [RFC PATCH 4/4] mfd: syscon: add ACPI support

[Kefeng Wang]

On 2015/12/3 23:56, Lorenzo Pieralisi wrote:
> On Thu, Dec 03, 2015 at 09:01:11PM +0800, Kefeng Wang wrote:
>> Hi Graeme, Arnd, and Lorenzo,
>>
>> Firstly, we absolutely agree with the point which use AML to do some "special"
>> initialisation and configuration.
> 
> Good.
> 
>> SAS and NIC driver were accepted by linux in hisilicon hip05 chip, and the drivers
>> reset the control by syscon, we want to use "_RST" method, which is introduced by
>> ACPI 6.0 spec in "7.3.25 _RST (Device Reset)", is it reasonable and standard for us?
> 
> Can you point me at the drivers you are referring to please ?

SAS： https://lkml.org/lkml/2015/11/17/572
      [PATCH v5 19/32] scsi: hisi_sas: add v1 HW initialisation code
      static int reset_hw_v1_hw(struct hisi_hba *hisi_hba);

HNS： drivers/net/ethernet/hisilicon/hns_mdio.c
      static int hns_mdio_reset(struct mii_bus *bus)；

> 
>> But here is a scene, we can not find a suitable way to support ACPI. There is no
>> independent memory region in some module(the driver not upstreamed), that is,
>> when write and read the module's register, we must r/w by syscon. Any advice?
> 
> What do you mean ? You mean that the reset control is a piece of HW
> that is shared between multiple "components" ? What's your concern
> about AML code driving those registers here ?

This is unrelated to reset control.

I mean we have some driver(not upstreamed), like LLC(last level cache), when access the register
of llc, we need help through syscon, because the llc has no independent registers region , steps
of Read and Write register in those drivers is shown below,

 1) get the syscon base;
 2) configure and choose the module which need to be accessed;
 3) R/W the value from the syscon, that is, get/set the value from/to llc;

Every read and write the register in those drivers, we must through syscon. That is why we need
syscon to support ACPI.

Thanks,
Kefeng


> 
> Thanks,
> Lorenzo
> 
>>
>> Thanks,
>> Kefeng
>>
>> On 2015/12/3 18:41, Graeme Gregory wrote:
>>> On Wed, Dec 02, 2015 at 11:44:51AM +0100, Arnd Bergmann wrote:
>>>> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
>>>>> This enables syscon with ACPI support.
>>>>> syscon_regmap_lookup_by_dev_property() function was added. With helper
>>>>> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
>>>>> in both DT and ACPI.
>>>>>
>>>>> The device driver can obtain syscon using _DSD method in DSDT, an example
>>>>> is shown below.
>>>>>
>>>>>     Device(CTL0) {
>>>>>           Name(_HID, "HISI0061")
>>>>>           Name(_CRS, ResourceTemplate() {
>>>>>                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
>>>>>           })
>>>>>     }
>>>>>
>>>>>     Device(DEV0) {
>>>>>           Name(_HID, "HISI00B1")
>>>>>           Name(_CRS, ResourceTemplate() {
>>>>>                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
>>>>>                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
>>>>>           })
>>>>>
>>>>>           Name (_DSD, Package () {
>>>>>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>>>>               Package () {
>>>>>                   Package () {"syscon",Package() {\_SB.CTL0} }
>>>>>               }
>>>>>           })
>>>>>     }
>>>>>
>>>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>>>
>>>> This sounds like a bad idea:
>>>>
>>>> syscon is basically a hack to let us access register that the SoC designer
>>>> couldn't fit in anywhere sane. We need something like this with devicetree
>>>> because we decided not to have any interpreted bytecode to do this behind
>>>> our back.
>>>>
>>>> With ACPI, the same thing is done with AML, which is actually nicer than
>>>> syscon (once you have to deal with all the problems introduced by AML).
>>>>
>>>> Use that instead.
>>>>
>>>
>>> I have to agree with Arnd here, this is specifically why it was chosen
>>> to use ACPI on machines to move all these "hacks" to AML.
>>>
>>> This leaves your driver being generic and any "special" initialisation
>>> can be supplied by the OEM through the ACPI tables.
>>>
>>> Graeme
>>>
>>>
>>> .
>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Kefeng Wang]

On 2015/12/3 23:56, Lorenzo Pieralisi wrote:
> On Thu, Dec 03, 2015 at 09:01:11PM +0800, Kefeng Wang wrote:
>> Hi Graeme, Arnd, and Lorenzo,
>>
>> Firstly, we absolutely agree with the point which use AML to do some "special"
>> initialisation and configuration.
> 
> Good.
> 
>> SAS and NIC driver were accepted by linux in hisilicon hip05 chip, and the drivers
>> reset the control by syscon, we want to use "_RST" method, which is introduced by
>> ACPI 6.0 spec in "7.3.25 _RST (Device Reset)", is it reasonable and standard for us?
> 
> Can you point me at the drivers you are referring to please ?

SAS? https://lkml.org/lkml/2015/11/17/572
      [PATCH v5 19/32] scsi: hisi_sas: add v1 HW initialisation code
      static int reset_hw_v1_hw(struct hisi_hba *hisi_hba);

HNS? drivers/net/ethernet/hisilicon/hns_mdio.c
      static int hns_mdio_reset(struct mii_bus *bus)?

> 
>> But here is a scene, we can not find a suitable way to support ACPI. There is no
>> independent memory region in some module(the driver not upstreamed), that is,
>> when write and read the module's register, we must r/w by syscon. Any advice?
> 
> What do you mean ? You mean that the reset control is a piece of HW
> that is shared between multiple "components" ? What's your concern
> about AML code driving those registers here ?

This is unrelated to reset control.

I mean we have some driver(not upstreamed), like LLC(last level cache), when access the register
of llc, we need help through syscon, because the llc has no independent registers region , steps
of Read and Write register in those drivers is shown below,

 1) get the syscon base;
 2) configure and choose the module which need to be accessed;
 3) R/W the value from the syscon, that is, get/set the value from/to llc;

Every read and write the register in those drivers, we must through syscon. That is why we need
syscon to support ACPI.

Thanks,
Kefeng


> 
> Thanks,
> Lorenzo
> 
>>
>> Thanks,
>> Kefeng
>>
>> On 2015/12/3 18:41, Graeme Gregory wrote:
>>> On Wed, Dec 02, 2015 at 11:44:51AM +0100, Arnd Bergmann wrote:
>>>> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
>>>>> This enables syscon with ACPI support.
>>>>> syscon_regmap_lookup_by_dev_property() function was added. With helper
>>>>> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
>>>>> in both DT and ACPI.
>>>>>
>>>>> The device driver can obtain syscon using _DSD method in DSDT, an example
>>>>> is shown below.
>>>>>
>>>>>     Device(CTL0) {
>>>>>           Name(_HID, "HISI0061")
>>>>>           Name(_CRS, ResourceTemplate() {
>>>>>                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
>>>>>           })
>>>>>     }
>>>>>
>>>>>     Device(DEV0) {
>>>>>           Name(_HID, "HISI00B1")
>>>>>           Name(_CRS, ResourceTemplate() {
>>>>>                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
>>>>>                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
>>>>>           })
>>>>>
>>>>>           Name (_DSD, Package () {
>>>>>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>>>>               Package () {
>>>>>                   Package () {"syscon",Package() {\_SB.CTL0} }
>>>>>               }
>>>>>           })
>>>>>     }
>>>>>
>>>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>>>
>>>> This sounds like a bad idea:
>>>>
>>>> syscon is basically a hack to let us access register that the SoC designer
>>>> couldn't fit in anywhere sane. We need something like this with devicetree
>>>> because we decided not to have any interpreted bytecode to do this behind
>>>> our back.
>>>>
>>>> With ACPI, the same thing is done with AML, which is actually nicer than
>>>> syscon (once you have to deal with all the problems introduced by AML).
>>>>
>>>> Use that instead.
>>>>
>>>
>>> I have to agree with Arnd here, this is specifically why it was chosen
>>> to use ACPI on machines to move all these "hacks" to AML.
>>>
>>> This leaves your driver being generic and any "special" initialisation
>>> can be supplied by the OEM through the ACPI tables.
>>>
>>> Graeme
>>>
>>>
>>> .
>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 

Re: [RFC PATCH 4/4] mfd: syscon: add ACPI support

[Arnd Bergmann]

On Monday 07 December 2015 14:15:37 Kefeng Wang wrote:
> On 2015/12/3 23:56, Lorenzo Pieralisi wrote:
> > On Thu, Dec 03, 2015 at 09:01:11PM +0800, Kefeng Wang wrote:
> >> Hi Graeme, Arnd, and Lorenzo,
> >>
> >> Firstly, we absolutely agree with the point which use AML to do some "special"
> >> initialisation and configuration.
> > 
> > Good.
> > 
> >> SAS and NIC driver were accepted by linux in hisilicon hip05 chip, and the drivers
> >> reset the control by syscon, we want to use "_RST" method, which is introduced by
> >> ACPI 6.0 spec in "7.3.25 _RST (Device Reset)", is it reasonable and standard for us?
> > 
> > Can you point me at the drivers you are referring to please ?
> 
> SAS： https://lkml.org/lkml/2015/11/17/572
>       [PATCH v5 19/32] scsi: hisi_sas: add v1 HW initialisation code
>       static int reset_hw_v1_hw(struct hisi_hba *hisi_hba);
> 
> HNS： drivers/net/ethernet/hisilicon/hns_mdio.c
>       static int hns_mdio_reset(struct mii_bus *bus)；

It seems that there is some commonality in here that there is more than
one device reset in this system control unit.

I'd suggest moving this out into a proper reset driver that of course
then has to be based on syscon for DT based systems so it doesn't conflict
with the other random stuff in the syscon space, and for backwards
compatibiltiy with old kernels that only know about the syscon based reset
you have currently implemented.

If there are additional devices that also use the same syscon node for
reset, they should of course only use the device_reset() method.

> >> But here is a scene, we can not find a suitable way to support ACPI. There is no
> >> independent memory region in some module(the driver not upstreamed), that is,
> >> when write and read the module's register, we must r/w by syscon. Any advice?
> > 
> > What do you mean ? You mean that the reset control is a piece of HW
> > that is shared between multiple "components" ? What's your concern
> > about AML code driving those registers here ?
> 
> This is unrelated to reset control.
> 
> I mean we have some driver(not upstreamed), like LLC(last level cache), when access the register
> of llc, we need help through syscon, because the llc has no independent registers region , steps
> of Read and Write register in those drivers is shown below,
> 
>  1) get the syscon base;
>  2) configure and choose the module which need to be accessed;
>  3) R/W the value from the syscon, that is, get/set the value from/to llc;
> 
> Every read and write the register in those drivers, we must through syscon. That is why we need
> syscon to support ACPI.

last level cache is something that should go through architecture code,
it has no business in syscon anyway. What do you control with this anyway?
AFAIK, ARMv8 has architected instructions to control caches and doesn't
need to talk to a cache controller the way we used to do on v6 and older v7
based systems.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Arnd Bergmann]

On Monday 07 December 2015 14:15:37 Kefeng Wang wrote:
> On 2015/12/3 23:56, Lorenzo Pieralisi wrote:
> > On Thu, Dec 03, 2015 at 09:01:11PM +0800, Kefeng Wang wrote:
> >> Hi Graeme, Arnd, and Lorenzo,
> >>
> >> Firstly, we absolutely agree with the point which use AML to do some "special"
> >> initialisation and configuration.
> > 
> > Good.
> > 
> >> SAS and NIC driver were accepted by linux in hisilicon hip05 chip, and the drivers
> >> reset the control by syscon, we want to use "_RST" method, which is introduced by
> >> ACPI 6.0 spec in "7.3.25 _RST (Device Reset)", is it reasonable and standard for us?
> > 
> > Can you point me at the drivers you are referring to please ?
> 
> SAS? https://lkml.org/lkml/2015/11/17/572
>       [PATCH v5 19/32] scsi: hisi_sas: add v1 HW initialisation code
>       static int reset_hw_v1_hw(struct hisi_hba *hisi_hba);
> 
> HNS? drivers/net/ethernet/hisilicon/hns_mdio.c
>       static int hns_mdio_reset(struct mii_bus *bus)?

It seems that there is some commonality in here that there is more than
one device reset in this system control unit.

I'd suggest moving this out into a proper reset driver that of course
then has to be based on syscon for DT based systems so it doesn't conflict
with the other random stuff in the syscon space, and for backwards
compatibiltiy with old kernels that only know about the syscon based reset
you have currently implemented.

If there are additional devices that also use the same syscon node for
reset, they should of course only use the device_reset() method.

> >> But here is a scene, we can not find a suitable way to support ACPI. There is no
> >> independent memory region in some module(the driver not upstreamed), that is,
> >> when write and read the module's register, we must r/w by syscon. Any advice?
> > 
> > What do you mean ? You mean that the reset control is a piece of HW
> > that is shared between multiple "components" ? What's your concern
> > about AML code driving those registers here ?
> 
> This is unrelated to reset control.
> 
> I mean we have some driver(not upstreamed), like LLC(last level cache), when access the register
> of llc, we need help through syscon, because the llc has no independent registers region , steps
> of Read and Write register in those drivers is shown below,
> 
>  1) get the syscon base;
>  2) configure and choose the module which need to be accessed;
>  3) R/W the value from the syscon, that is, get/set the value from/to llc;
> 
> Every read and write the register in those drivers, we must through syscon. That is why we need
> syscon to support ACPI.

last level cache is something that should go through architecture code,
it has no business in syscon anyway. What do you control with this anyway?
AFAIK, ARMv8 has architected instructions to control caches and doesn't
need to talk to a cache controller the way we used to do on v6 and older v7
based systems.

	Arnd

Re: [RFC PATCH 4/4] mfd: syscon: add ACPI support

[Zhangfei Gao]

Hi, Arnd

On Wed, Dec 2, 2015 at 6:44 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
>> This enables syscon with ACPI support.
>> syscon_regmap_lookup_by_dev_property() function was added. With helper
>> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
>> in both DT and ACPI.
>>
>> The device driver can obtain syscon using _DSD method in DSDT, an example
>> is shown below.
>>
>>     Device(CTL0) {
>>           Name(_HID, "HISI0061")
>>           Name(_CRS, ResourceTemplate() {
>>                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
>>           })
>>     }
>>
>>     Device(DEV0) {
>>           Name(_HID, "HISI00B1")
>>           Name(_CRS, ResourceTemplate() {
>>                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
>>                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
>>           })
>>
>>           Name (_DSD, Package () {
>>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>               Package () {
>>                   Package () {"syscon",Package() {\_SB.CTL0} }
>>               }
>>           })
>>     }
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>
> This sounds like a bad idea:
>
> syscon is basically a hack to let us access register that the SoC designer
> couldn't fit in anywhere sane. We need something like this with devicetree
> because we decided not to have any interpreted bytecode to do this behind
> our back.
>
> With ACPI, the same thing is done with AML, which is actually nicer than
> syscon (once you have to deal with all the problems introduced by AML).
>
> Use that instead.
>

Would you mind clarifying AML method, still not understand.
Is is means realize the operation in uefi/bootloader and call some
interface from kernel?

The issue here is we want to access some common registers,
which we do not want to ioremap for many times in each driver.

In hisi platforms, we have many common registers shared by many components.

Thanks.


>> +               pdev = acpi_dev_find_plat_dev(adev);
>> +               if (!pdev)
>> +                       return ERR_PTR(-ENODEV);
>> +               syscon = platform_get_drvdata(pdev);
>
> This still requires the syscon device to be probed first, which is
> something we shouldn't really need any more.
>
> A lot of syscon users depend on the regmap to be available before
> device drivers are loaded, so we have changed the code to just look up
> the device_node that is already present and ignore the device.
>
> Once clps711x has been converted to DT, I think we can rip out the
> syscon_regmap_lookup_by_pdevname() interface and separate the syscon
> regmap handling from the device handling that we only really need
> for debugfs.

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Zhangfei Gao]

Hi, Arnd

On Wed, Dec 2, 2015 at 6:44 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
>> This enables syscon with ACPI support.
>> syscon_regmap_lookup_by_dev_property() function was added. With helper
>> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used
>> in both DT and ACPI.
>>
>> The device driver can obtain syscon using _DSD method in DSDT, an example
>> is shown below.
>>
>>     Device(CTL0) {
>>           Name(_HID, "HISI0061")
>>           Name(_CRS, ResourceTemplate() {
>>                   Memory32Fixed(ReadWrite, 0x80000000, 0x10000)
>>           })
>>     }
>>
>>     Device(DEV0) {
>>           Name(_HID, "HISI00B1")
>>           Name(_CRS, ResourceTemplate() {
>>                   Memory32Fixed(ReadWrite, 0x8c030000, 0x10000)
>>                   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 }
>>           })
>>
>>           Name (_DSD, Package () {
>>               ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>               Package () {
>>                   Package () {"syscon",Package() {\_SB.CTL0} }
>>               }
>>           })
>>     }
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>
> This sounds like a bad idea:
>
> syscon is basically a hack to let us access register that the SoC designer
> couldn't fit in anywhere sane. We need something like this with devicetree
> because we decided not to have any interpreted bytecode to do this behind
> our back.
>
> With ACPI, the same thing is done with AML, which is actually nicer than
> syscon (once you have to deal with all the problems introduced by AML).
>
> Use that instead.
>

Would you mind clarifying AML method, still not understand.
Is is means realize the operation in uefi/bootloader and call some
interface from kernel?

The issue here is we want to access some common registers,
which we do not want to ioremap for many times in each driver.

In hisi platforms, we have many common registers shared by many components.

Thanks.


>> +               pdev = acpi_dev_find_plat_dev(adev);
>> +               if (!pdev)
>> +                       return ERR_PTR(-ENODEV);
>> +               syscon = platform_get_drvdata(pdev);
>
> This still requires the syscon device to be probed first, which is
> something we shouldn't really need any more.
>
> A lot of syscon users depend on the regmap to be available before
> device drivers are loaded, so we have changed the code to just look up
> the device_node that is already present and ignore the device.
>
> Once clps711x has been converted to DT, I think we can rip out the
> syscon_regmap_lookup_by_pdevname() interface and separate the syscon
> regmap handling from the device handling that we only really need
> for debugfs.

Re: [RFC PATCH 4/4] mfd: syscon: add ACPI support

[Arnd Bergmann]

On Friday 11 December 2015 18:35:17 Zhangfei Gao wrote:
> On Wed, Dec 2, 2015 at 6:44 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
> >
> > This sounds like a bad idea:
> >
> > syscon is basically a hack to let us access register that the SoC designer
> > couldn't fit in anywhere sane. We need something like this with devicetree
> > because we decided not to have any interpreted bytecode to do this behind
> > our back.
> >
> > With ACPI, the same thing is done with AML, which is actually nicer than
> > syscon (once you have to deal with all the problems introduced by AML).
> >
> > Use that instead.
> >
> 
> Would you mind clarifying AML method, still not understand.
> Is is means realize the operation in uefi/bootloader and call some
> interface from kernel?
>
> The issue here is we want to access some common registers,
> which we do not want to ioremap for many times in each driver.
> 
> In hisi platforms, we have many common registers shared by many components.

Sorry, you'd have to ask someone who is more familiar with ACPI.

Basically, the idea of ACPI as I understand it is that you have a
interpreted bytecode language that is provided by the BIOS as tables
and executed in the context of the kernel. This byte code has access
to stuff like GPIO, PCI config space, MMIO registers, IPMI or the
embedded controller on a PC. This way, you can hide all the platform
specific details behind a function call from a generic device driver,
and the BIOS writer can provide the specific implementation for any
registers that are shared across components.

That's about all I know, but the ACPI specifications are available
publicly, please have a look there for more details.

	Arnd

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Arnd Bergmann]

On Friday 11 December 2015 18:35:17 Zhangfei Gao wrote:
> On Wed, Dec 2, 2015 at 6:44 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
> >
> > This sounds like a bad idea:
> >
> > syscon is basically a hack to let us access register that the SoC designer
> > couldn't fit in anywhere sane. We need something like this with devicetree
> > because we decided not to have any interpreted bytecode to do this behind
> > our back.
> >
> > With ACPI, the same thing is done with AML, which is actually nicer than
> > syscon (once you have to deal with all the problems introduced by AML).
> >
> > Use that instead.
> >
> 
> Would you mind clarifying AML method, still not understand.
> Is is means realize the operation in uefi/bootloader and call some
> interface from kernel?
>
> The issue here is we want to access some common registers,
> which we do not want to ioremap for many times in each driver.
> 
> In hisi platforms, we have many common registers shared by many components.

Sorry, you'd have to ask someone who is more familiar with ACPI.

Basically, the idea of ACPI as I understand it is that you have a
interpreted bytecode language that is provided by the BIOS as tables
and executed in the context of the kernel. This byte code has access
to stuff like GPIO, PCI config space, MMIO registers, IPMI or the
embedded controller on a PC. This way, you can hide all the platform
specific details behind a function call from a generic device driver,
and the BIOS writer can provide the specific implementation for any
registers that are shared across components.

That's about all I know, but the ACPI specifications are available
publicly, please have a look there for more details.

	Arnd

Re: [RFC PATCH 4/4] mfd: syscon: add ACPI support

[Graeme Gregory]

On Fri, 11 Dec 2015, at 10:51 AM, Arnd Bergmann wrote:
> On Friday 11 December 2015 18:35:17 Zhangfei Gao wrote:
> > On Wed, Dec 2, 2015 at 6:44 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
> > >
> > > This sounds like a bad idea:
> > >
> > > syscon is basically a hack to let us access register that the SoC designer
> > > couldn't fit in anywhere sane. We need something like this with devicetree
> > > because we decided not to have any interpreted bytecode to do this behind
> > > our back.
> > >
> > > With ACPI, the same thing is done with AML, which is actually nicer than
> > > syscon (once you have to deal with all the problems introduced by AML).
> > >
> > > Use that instead.
> > >
> > 
> > Would you mind clarifying AML method, still not understand.
> > Is is means realize the operation in uefi/bootloader and call some
> > interface from kernel?
> >
> > The issue here is we want to access some common registers,
> > which we do not want to ioremap for many times in each driver.
> > 
> > In hisi platforms, we have many common registers shared by many components.
> 
> Sorry, you'd have to ask someone who is more familiar with ACPI.
> 
> Basically, the idea of ACPI as I understand it is that you have a
> interpreted bytecode language that is provided by the BIOS as tables
> and executed in the context of the kernel. This byte code has access
> to stuff like GPIO, PCI config space, MMIO registers, IPMI or the
> embedded controller on a PC. This way, you can hide all the platform
> specific details behind a function call from a generic device driver,
> and the BIOS writer can provide the specific implementation for any
> registers that are shared across components.
> 
> That's about all I know, but the ACPI specifications are available
> publicly, please have a look there for more details.
> 

Basically in the DSDT you would create a common accessor method for the
register/registers in question. This function takes care of the register
in the multiple user cases.

each _RST() method would then call this function to set the bits they
required.

So in effect it is no different to what you would do in Linux, just done
in AML instead.

Graeme

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Graeme Gregory]

On Fri, 11 Dec 2015, at 10:51 AM, Arnd Bergmann wrote:
> On Friday 11 December 2015 18:35:17 Zhangfei Gao wrote:
> > On Wed, Dec 2, 2015 at 6:44 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
> > >
> > > This sounds like a bad idea:
> > >
> > > syscon is basically a hack to let us access register that the SoC designer
> > > couldn't fit in anywhere sane. We need something like this with devicetree
> > > because we decided not to have any interpreted bytecode to do this behind
> > > our back.
> > >
> > > With ACPI, the same thing is done with AML, which is actually nicer than
> > > syscon (once you have to deal with all the problems introduced by AML).
> > >
> > > Use that instead.
> > >
> > 
> > Would you mind clarifying AML method, still not understand.
> > Is is means realize the operation in uefi/bootloader and call some
> > interface from kernel?
> >
> > The issue here is we want to access some common registers,
> > which we do not want to ioremap for many times in each driver.
> > 
> > In hisi platforms, we have many common registers shared by many components.
> 
> Sorry, you'd have to ask someone who is more familiar with ACPI.
> 
> Basically, the idea of ACPI as I understand it is that you have a
> interpreted bytecode language that is provided by the BIOS as tables
> and executed in the context of the kernel. This byte code has access
> to stuff like GPIO, PCI config space, MMIO registers, IPMI or the
> embedded controller on a PC. This way, you can hide all the platform
> specific details behind a function call from a generic device driver,
> and the BIOS writer can provide the specific implementation for any
> registers that are shared across components.
> 
> That's about all I know, but the ACPI specifications are available
> publicly, please have a look there for more details.
> 

Basically in the DSDT you would create a common accessor method for the
register/registers in question. This function takes care of the register
in the multiple user cases.

each _RST() method would then call this function to set the bits they
required.

So in effect it is no different to what you would do in Linux, just done
in AML instead.

Graeme

Re: [RFC PATCH 4/4] mfd: syscon: add ACPI support

[Hanjun Guo]

On 12/11/2015 06:51 PM, Arnd Bergmann wrote:
> On Friday 11 December 2015 18:35:17 Zhangfei Gao wrote:
>> On Wed, Dec 2, 2015 at 6:44 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
>>>
>>> This sounds like a bad idea:
>>>
>>> syscon is basically a hack to let us access register that the SoC designer
>>> couldn't fit in anywhere sane. We need something like this with devicetree
>>> because we decided not to have any interpreted bytecode to do this behind
>>> our back.
>>>
>>> With ACPI, the same thing is done with AML, which is actually nicer than
>>> syscon (once you have to deal with all the problems introduced by AML).
>>>
>>> Use that instead.
>>>
>>
>> Would you mind clarifying AML method, still not understand.
>> Is is means realize the operation in uefi/bootloader and call some
>> interface from kernel?
>>
>> The issue here is we want to access some common registers,
>> which we do not want to ioremap for many times in each driver.
>>
>> In hisi platforms, we have many common registers shared by many components.
>
> Sorry, you'd have to ask someone who is more familiar with ACPI.
>
> Basically, the idea of ACPI as I understand it is that you have a
> interpreted bytecode language that is provided by the BIOS as tables
> and executed in the context of the kernel. This byte code has access
> to stuff like GPIO, PCI config space, MMIO registers, IPMI or the
> embedded controller on a PC. This way, you can hide all the platform
> specific details behind a function call from a generic device driver,
> and the BIOS writer can provide the specific implementation for any
> registers that are shared across components.
>
> That's about all I know, but the ACPI specifications are available
> publicly, please have a look there for more details.

Thanks Arnd rise this up, I agree we need to abstract the syscon
inside UEFI.

For ACPI 6.0, there are method such as _ON, _OFF and _RST (reset)
to the device's power resources, I think it can be extended to
syscon to reset the device with access syscon in ACPI method,
those method will be implemented in UEFI which the kernel driver
don't need to care about the detail.

We are going to for that implementation for D02, and if problem
we meet, we will go back for this discussion.

Thanks
Hanjun

[RFC PATCH 4/4] mfd: syscon: add ACPI support

[Hanjun Guo]

On 12/11/2015 06:51 PM, Arnd Bergmann wrote:
> On Friday 11 December 2015 18:35:17 Zhangfei Gao wrote:
>> On Wed, Dec 2, 2015 at 6:44 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote:
>>>
>>> This sounds like a bad idea:
>>>
>>> syscon is basically a hack to let us access register that the SoC designer
>>> couldn't fit in anywhere sane. We need something like this with devicetree
>>> because we decided not to have any interpreted bytecode to do this behind
>>> our back.
>>>
>>> With ACPI, the same thing is done with AML, which is actually nicer than
>>> syscon (once you have to deal with all the problems introduced by AML).
>>>
>>> Use that instead.
>>>
>>
>> Would you mind clarifying AML method, still not understand.
>> Is is means realize the operation in uefi/bootloader and call some
>> interface from kernel?
>>
>> The issue here is we want to access some common registers,
>> which we do not want to ioremap for many times in each driver.
>>
>> In hisi platforms, we have many common registers shared by many components.
>
> Sorry, you'd have to ask someone who is more familiar with ACPI.
>
> Basically, the idea of ACPI as I understand it is that you have a
> interpreted bytecode language that is provided by the BIOS as tables
> and executed in the context of the kernel. This byte code has access
> to stuff like GPIO, PCI config space, MMIO registers, IPMI or the
> embedded controller on a PC. This way, you can hide all the platform
> specific details behind a function call from a generic device driver,
> and the BIOS writer can provide the specific implementation for any
> registers that are shared across components.
>
> That's about all I know, but the ACPI specifications are available
> publicly, please have a look there for more details.

Thanks Arnd rise this up, I agree we need to abstract the syscon
inside UEFI.

For ACPI 6.0, there are method such as _ON, _OFF and _RST (reset)
to the device's power resources, I think it can be extended to
syscon to reset the device with access syscon in ACPI method,
those method will be implemented in UEFI which the kernel driver
don't need to care about the detail.

We are going to for that implementation for D02, and if problem
we meet, we will go back for this discussion.

Thanks
Hanjun

Re: [RFC PATCH 2/4] device property: Introduce helper device_get_reference_node()

[Lorenzo Pieralisi]

Hi Russell,

On Thu, Dec 03, 2015 at 03:28:42PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 02, 2015 at 05:09:26PM +0800, Kefeng Wang wrote:
> > With of_parse_phandle() and acpi_dev_get_reference_device(), we can introduce
> > a universal helper device_get_reference_node() to read and parse a device
> > property and return a pointer to the resulting firmware node.
> 
> What's happening to make this fwnode stuff actually usable?  Whenever
> I've looked at it from the DT perspective, it looks very much like a
> half-baked train wreck - although the struct device_node contains a
> fwnode, of_node_init() initialises it partially, and we have a way to
> convert _from_ a fwnode to a device_node (to_of_node), nothing sets
> the struct device fwnode pointer.

I might have missed why that pointer is left to NULL in DT, it is
not that complicated to set it up and may actually simplify things
in the process.

Rafael certainly knows better than I do, it could have been done in
commit:

9017f25254e4 ("driver core: Implement device property accessors through
fwnode ones")

except that as you noticed the dev_fwnode has to be special cased since
the OF core does not set the fwnode pointer (there must be a reason
why, if there is not a patch to implement that is simple).

On DT the fwnode_handle is allocated in the struct device_node that
contains it but I still do not see why the fwnode pointer in struct
device can't be made to point at it.

> Whenever I've looked at this, it's always led me to the conclusion
> that the way that fwnode stuff has currently been written, I'm
> supposed to get the device_node, and then use the embedded fwnode
> directly, which I'm pretty sure misses the point of fwnode (as it
> means any driver code making use of this is tied to DT.)
> 
> This patch seems to confirm my suspicions that it's just wrong -
> trying to use device_get_reference_node() here will fail in a DT
> based setup, because (I assume) dev_fwnode(dev) returns dev->fwnode
> which is NULL there.

No, dev_fwnode is special cased for OF (have a look at it in
/drivers/base/property.c), I share your question on why it has to
be so, actually this would make dev_fwnode() even simpler.

> I don't know, maybe there is an intention that fwnode should not be
> used for DT?

That's the question we need to get answered :), it might well be that
we can simply update the OF populate code to initialize the pointer
in the struct device.

Thanks,
Lorenzo

[RFC PATCH 2/4] device property: Introduce helper device_get_reference_node()

[Lorenzo Pieralisi]

Hi Russell,

On Thu, Dec 03, 2015 at 03:28:42PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 02, 2015 at 05:09:26PM +0800, Kefeng Wang wrote:
> > With of_parse_phandle() and acpi_dev_get_reference_device(), we can introduce
> > a universal helper device_get_reference_node() to read and parse a device
> > property and return a pointer to the resulting firmware node.
> 
> What's happening to make this fwnode stuff actually usable?  Whenever
> I've looked at it from the DT perspective, it looks very much like a
> half-baked train wreck - although the struct device_node contains a
> fwnode, of_node_init() initialises it partially, and we have a way to
> convert _from_ a fwnode to a device_node (to_of_node), nothing sets
> the struct device fwnode pointer.

I might have missed why that pointer is left to NULL in DT, it is
not that complicated to set it up and may actually simplify things
in the process.

Rafael certainly knows better than I do, it could have been done in
commit:

9017f25254e4 ("driver core: Implement device property accessors through
fwnode ones")

except that as you noticed the dev_fwnode has to be special cased since
the OF core does not set the fwnode pointer (there must be a reason
why, if there is not a patch to implement that is simple).

On DT the fwnode_handle is allocated in the struct device_node that
contains it but I still do not see why the fwnode pointer in struct
device can't be made to point at it.

> Whenever I've looked at this, it's always led me to the conclusion
> that the way that fwnode stuff has currently been written, I'm
> supposed to get the device_node, and then use the embedded fwnode
> directly, which I'm pretty sure misses the point of fwnode (as it
> means any driver code making use of this is tied to DT.)
> 
> This patch seems to confirm my suspicions that it's just wrong -
> trying to use device_get_reference_node() here will fail in a DT
> based setup, because (I assume) dev_fwnode(dev) returns dev->fwnode
> which is NULL there.

No, dev_fwnode is special cased for OF (have a look at it in
/drivers/base/property.c), I share your question on why it has to
be so, actually this would make dev_fwnode() even simpler.

> I don't know, maybe there is an intention that fwnode should not be
> used for DT?

That's the question we need to get answered :), it might well be that
we can simply update the OF populate code to initialize the pointer
in the struct device.

Thanks,
Lorenzo