[PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Andy Shevchenko]

This series includes few logical sets that bring a support of non-ACPI
platforms for Intel Skylake.

First part is a refactoring of built-in device properties support:
- keep single value inside the structure
- provide helper macros to define built-in properties
- fall back to secondary fwnode if primary has no asked property

Second is a propagating built-in device properties in platform core.

Third one is modifications to MFD code and intel-lpss.c driver in particular
to define and pass built-in properties to the individual drivers.

And last part is a fix for I2C bug found on Lenovo Yoga hardware and a first
converted user.

Built-in device properties is an alternative to platform data. It provides a
unified API that drivers can use to cover all cases at once: DT, ACPI, and
built-in properties.

With this series applied a platform data can be considered obsolete. Moreover,
built-in device properties allow to adjust the existing configuration, for
example, in cases when ACPI values are wrong on some platforms.

The series has been tested on available hardware and doesn't break current
behaviour. But we ask people who have the affected hardware to apply the series
on your side and check with Lenovo hardware.

Changelog v2:
- fix isuues found by kbuild bot (kbuild)
- append a patch to propagate device properties in polatform code (Arnd)
- update few existing and add couple of new patches due to above
- check with kmemleak

Andy Shevchenko (9):
  device property: always check for fwnode type
  device property: rename helper functions
  device property: refactor built-in properties support
  device property: keep single value inplace
  device property: improve readability of macros
  device property: return -EINVAL when property isn't found in ACPI
  device property: Fallback to secondary fwnode if primary misses the
    property
  mfd: core: propagate device properties to sub devices drivers
  mfd: intel-lpss: Pass HSUART configuration via properties

Heikki Krogerus (1):
  device property: helper macros for property entry creation

Mika Westerberg (6):
  device property: Take a copy of the property set
  driver core: platform: Add support for built-in device properties
  driver core: Do not overwrite secondary fwnode with NULL if it is set
  mfd: intel-lpss: Add support for passing device properties
  mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
  i2c: designware: Convert to use unified device property API

 drivers/acpi/property.c                     |  10 +-
 drivers/base/core.c                         |   5 +-
 drivers/base/platform.c                     |  25 ++
 drivers/base/property.c                     | 491 ++++++++++++++++++++++------
 drivers/i2c/busses/i2c-designware-platdrv.c |  50 ++-
 drivers/mfd/intel-lpss-acpi.c               |  19 +-
 drivers/mfd/intel-lpss-pci.c                |  44 ++-
 drivers/mfd/intel-lpss.c                    |  16 +-
 drivers/mfd/intel-lpss.h                    |   2 +
 drivers/mfd/mfd-core.c                      |   7 +
 include/linux/mfd/core.h                    |   5 +
 include/linux/platform_device.h             |   5 +
 include/linux/property.h                    |  99 +++++-
 13 files changed, 621 insertions(+), 157 deletions(-)

-- 
2.6.2

[PATCH v2 01/16] device property: always check for fwnode type

[Andy Shevchenko]

Currently the property accessors unconditionally fall back to built-in property
set as a last resort. Make this strict and return an error in case the type of
fwnode is unknown.

This is actually a follow up to the commit 4fa7508e9f1c (device property:
Return -ENXIO if there is no suitable FW interface).

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/property.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 1325ff2..09e488d 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -135,8 +135,9 @@ bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
 		return of_property_read_bool(to_of_node(fwnode), propname);
 	else if (is_acpi_node(fwnode))
 		return !acpi_node_prop_get(fwnode, propname, NULL);
-
-	return !!pset_prop_get(to_pset(fwnode), propname);
+	else if (is_pset(fwnode))
+		return !!pset_prop_get(to_pset(fwnode), propname);
+	return false;
 }
 EXPORT_SYMBOL_GPL(fwnode_property_present);
 
@@ -494,9 +495,10 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 	else if (is_acpi_node(fwnode))
 		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
 					   val, 1);
-
-	return pset_prop_read_array(to_pset(fwnode), propname,
-				    DEV_PROP_STRING, val, 1);
+	else if (is_pset(fwnode))
+		return pset_prop_read_array(to_pset(fwnode), propname,
+					    DEV_PROP_STRING, val, 1);
+	return -ENXIO;
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_string);
 
-- 
2.6.2

[PATCH v2 02/16] device property: rename helper functions

[Andy Shevchenko]

To be in align with the rest of fwnode types we rename the built-in property
set ones, i.e.
	is_pset() -> is_pset_node()
	to_pset() -> to_pset_node()

There is no functional change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/property.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 09e488d..2e01f3f 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -37,14 +37,14 @@ void device_add_property_set(struct device *dev, struct property_set *pset)
 }
 EXPORT_SYMBOL_GPL(device_add_property_set);
 
-static inline bool is_pset(struct fwnode_handle *fwnode)
+static inline bool is_pset_node(struct fwnode_handle *fwnode)
 {
 	return fwnode && fwnode->type == FWNODE_PDATA;
 }
 
-static inline struct property_set *to_pset(struct fwnode_handle *fwnode)
+static inline struct property_set *to_pset_node(struct fwnode_handle *fwnode)
 {
-	return is_pset(fwnode) ?
+	return is_pset_node(fwnode) ?
 		container_of(fwnode, struct property_set, fwnode) : NULL;
 }
 
@@ -135,8 +135,8 @@ bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
 		return of_property_read_bool(to_of_node(fwnode), propname);
 	else if (is_acpi_node(fwnode))
 		return !acpi_node_prop_get(fwnode, propname, NULL);
-	else if (is_pset(fwnode))
-		return !!pset_prop_get(to_pset(fwnode), propname);
+	else if (is_pset_node(fwnode))
+		return !!pset_prop_get(to_pset_node(fwnode), propname);
 	return false;
 }
 EXPORT_SYMBOL_GPL(fwnode_property_present);
@@ -323,8 +323,8 @@ EXPORT_SYMBOL_GPL(device_property_match_string);
 	else if (is_acpi_node(_fwnode_)) \
 		_ret_ = acpi_node_prop_read(_fwnode_, _propname_, _proptype_, \
 					    _val_, _nval_); \
-	else if (is_pset(_fwnode_)) \
-		_ret_ = pset_prop_read_array(to_pset(_fwnode_), _propname_, \
+	else if (is_pset_node(_fwnode_)) 						\
+		_ret_ = pset_prop_read_array(to_pset_node(_fwnode_), _propname_,	\
 					     _proptype_, _val_, _nval_); \
 	else \
 		_ret_ = -ENXIO; \
@@ -465,8 +465,8 @@ int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
 	else if (is_acpi_node(fwnode))
 		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
 					   val, nval);
-	else if (is_pset(fwnode))
-		return pset_prop_read_array(to_pset(fwnode), propname,
+	else if (is_pset_node(fwnode))
+		return pset_prop_read_array(to_pset_node(fwnode), propname,
 					    DEV_PROP_STRING, val, nval);
 	return -ENXIO;
 }
@@ -495,8 +495,8 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 	else if (is_acpi_node(fwnode))
 		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
 					   val, 1);
-	else if (is_pset(fwnode))
-		return pset_prop_read_array(to_pset(fwnode), propname,
+	else if (is_pset_node(fwnode))
+		return pset_prop_read_array(to_pset_node(fwnode), propname,
 					    DEV_PROP_STRING, val, 1);
 	return -ENXIO;
 }
-- 
2.6.2

[PATCH v2 03/16] device property: refactor built-in properties support

[Andy Shevchenko]

Instead of using the type and nval fields we will use length (in bytes) of the
value. The sanity check is done in the accessors.

The built-in property accessors are split in the same way such as device tree.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/property.c  | 150 ++++++++++++++++++++++++++++++++++-------------
 include/linux/property.h |   8 +--
 2 files changed, 113 insertions(+), 45 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 2e01f3f..86834bd 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -63,45 +63,107 @@ static struct property_entry *pset_prop_get(struct property_set *pset,
 	return NULL;
 }
 
-static int pset_prop_read_array(struct property_set *pset, const char *name,
-				enum dev_prop_type type, void *val, size_t nval)
+static void *pset_prop_find(struct property_set *pset, const char *propname,
+			    size_t length)
 {
 	struct property_entry *prop;
-	unsigned int item_size;
+	void *pointer;
 
-	prop = pset_prop_get(pset, name);
+	prop = pset_prop_get(pset, propname);
+	if (!prop)
+		return ERR_PTR(-EINVAL);
+	pointer = prop->value.raw_data;
+	if (!pointer)
+		return ERR_PTR(-ENODATA);
+	if (length > prop->length)
+		return ERR_PTR(-EOVERFLOW);
+	return pointer;
+}
+
+static int pset_prop_read_u8_array(struct property_set *pset,
+				   const char *propname,
+				   u8 *values, size_t nval)
+{
+	void *pointer;
+	size_t length = nval * sizeof(*values);
+
+	pointer = pset_prop_find(pset, propname, length);
+	if (IS_ERR(pointer))
+		return PTR_ERR(pointer);
+
+	memcpy(values, pointer, length);
+	return 0;
+}
+
+static int pset_prop_read_u16_array(struct property_set *pset,
+				    const char *propname,
+				    u16 *values, size_t nval)
+{
+	void *pointer;
+	size_t length = nval * sizeof(*values);
+
+	pointer = pset_prop_find(pset, propname, length);
+	if (IS_ERR(pointer))
+		return PTR_ERR(pointer);
+
+	memcpy(values, pointer, length);
+	return 0;
+}
+
+static int pset_prop_read_u32_array(struct property_set *pset,
+				    const char *propname,
+				    u32 *values, size_t nval)
+{
+	void *pointer;
+	size_t length = nval * sizeof(*values);
+
+	pointer = pset_prop_find(pset, propname, length);
+	if (IS_ERR(pointer))
+		return PTR_ERR(pointer);
+
+	memcpy(values, pointer, length);
+	return 0;
+}
+
+static int pset_prop_read_u64_array(struct property_set *pset,
+				    const char *propname,
+				    u64 *values, size_t nval)
+{
+	void *pointer;
+	size_t length = nval * sizeof(*values);
+
+	pointer = pset_prop_find(pset, propname, length);
+	if (IS_ERR(pointer))
+		return PTR_ERR(pointer);
+
+	memcpy(values, pointer, length);
+	return 0;
+}
+
+static int pset_prop_count_elems_of_size(struct property_set *pset,
+					 const char *propname, size_t length)
+{
+	struct property_entry *prop;
+
+	prop = pset_prop_get(pset, propname);
 	if (!prop)
-		return -ENODATA;
-
-	if (prop->type != type)
-		return -EPROTO;
-
-	if (!val)
-		return prop->nval;
-
-	if (prop->nval < nval)
-		return -EOVERFLOW;
-
-	switch (type) {
-	case DEV_PROP_U8:
-		item_size = sizeof(u8);
-		break;
-	case DEV_PROP_U16:
-		item_size = sizeof(u16);
-		break;
-	case DEV_PROP_U32:
-		item_size = sizeof(u32);
-		break;
-	case DEV_PROP_U64:
-		item_size = sizeof(u64);
-		break;
-	case DEV_PROP_STRING:
-		item_size = sizeof(const char *);
-		break;
-	default:
 		return -EINVAL;
-	}
-	memcpy(val, prop->value.raw_data, nval * item_size);
+
+	return prop->length / length;
+}
+
+static int pset_prop_read_string_array(struct property_set *pset,
+				       const char *propname,
+				       const char **strings, size_t nval)
+{
+	void *pointer;
+	size_t length = nval * sizeof(*strings);
+
+	pointer = pset_prop_find(pset, propname, length);
+	if (IS_ERR(pointer))
+		return PTR_ERR(pointer);
+
+	memcpy(strings, pointer, length);
 	return 0;
 }
 
@@ -314,6 +376,10 @@ EXPORT_SYMBOL_GPL(device_property_match_string);
 	(val) ? of_property_read_##type##_array((node), (propname), (val), (nval)) \
 	      : of_property_count_elems_of_size((node), (propname), sizeof(type))
 
+#define PSET_PROP_READ_ARRAY(node, propname, type, val, nval)				\
+	(val) ? pset_prop_read_##type##_array((node), (propname), (val), (nval))	\
+	      : pset_prop_count_elems_of_size((node), (propname), sizeof(type))
+
 #define FWNODE_PROP_READ_ARRAY(_fwnode_, _propname_, _type_, _proptype_, _val_, _nval_) \
 ({ \
 	int _ret_; \
@@ -324,8 +390,8 @@ EXPORT_SYMBOL_GPL(device_property_match_string);
 		_ret_ = acpi_node_prop_read(_fwnode_, _propname_, _proptype_, \
 					    _val_, _nval_); \
 	else if (is_pset_node(_fwnode_)) 						\
-		_ret_ = pset_prop_read_array(to_pset_node(_fwnode_), _propname_,	\
-					     _proptype_, _val_, _nval_); \
+		_ret_ = PSET_PROP_READ_ARRAY(to_pset_node(_fwnode_), _propname_,	\
+					     _type_, _val_, _nval_);			\
 	else \
 		_ret_ = -ENXIO; \
 	_ret_; \
@@ -466,8 +532,12 @@ int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
 		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
 					   val, nval);
 	else if (is_pset_node(fwnode))
-		return pset_prop_read_array(to_pset_node(fwnode), propname,
-					    DEV_PROP_STRING, val, nval);
+		return val ?
+			pset_prop_read_string_array(to_pset_node(fwnode),
+						    propname, val, nval) :
+			pset_prop_count_elems_of_size(to_pset_node(fwnode),
+						      propname,
+						      sizeof(const char *));
 	return -ENXIO;
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_string_array);
@@ -496,8 +566,8 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
 					   val, 1);
 	else if (is_pset_node(fwnode))
-		return pset_prop_read_array(to_pset_node(fwnode), propname,
-					    DEV_PROP_STRING, val, 1);
+		return pset_prop_read_string_array(to_pset_node(fwnode),
+						   propname, val, 1);
 	return -ENXIO;
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_string);
diff --git a/include/linux/property.h b/include/linux/property.h
index 0a3705a..c29460a 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -144,14 +144,12 @@ static inline int fwnode_property_read_u64(struct fwnode_handle *fwnode,
 /**
  * struct property_entry - "Built-in" device property representation.
  * @name: Name of the property.
- * @type: Type of the property.
- * @nval: Number of items of type @type making up the value.
- * @value: Value of the property (an array of @nval items of type @type).
+ * @length: Length of data making up the value.
+ * @value: Value of the property (an array of items of the given type).
  */
 struct property_entry {
 	const char *name;
-	enum dev_prop_type type;
-	size_t nval;
+	size_t length;
 	union {
 		void *raw_data;
 		u8 *u8_data;
-- 
2.6.2

[PATCH v2 04/16] device property: keep single value inplace

[Andy Shevchenko]

We may save a lot of lines of code and space by keeping single values inside
the struct property_entry. Refactor the implementation to do so.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/property.c  | 33 ++++++++++++++++++++++++++++++---
 include/linux/property.h | 31 +++++++++++++++++++++++--------
 2 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 86834bd..ad3cb09 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -72,7 +72,10 @@ static void *pset_prop_find(struct property_set *pset, const char *propname,
 	prop = pset_prop_get(pset, propname);
 	if (!prop)
 		return ERR_PTR(-EINVAL);
-	pointer = prop->value.raw_data;
+	if (prop->is_array)
+		pointer = prop->pointer.raw_data;
+	else
+		pointer = &prop->value.raw_data;
 	if (!pointer)
 		return ERR_PTR(-ENODATA);
 	if (length > prop->length)
@@ -167,6 +170,31 @@ static int pset_prop_read_string_array(struct property_set *pset,
 	return 0;
 }
 
+static int pset_prop_read_string(struct property_set *pset,
+				 const char *propname, const char **strings)
+{
+	struct property_entry *prop;
+	const char **pointer;
+
+	prop = pset_prop_get(pset, propname);
+	if (!prop)
+		return -EINVAL;
+	if (!prop->is_string)
+		return -EILSEQ;
+	if (prop->is_array) {
+		pointer = prop->pointer.str;
+		if (!pointer)
+			return -ENODATA;
+	} else {
+		pointer = &prop->value.str;
+		if (*pointer && strnlen(*pointer, prop->length) >= prop->length)
+			return -EILSEQ;
+	}
+
+	*strings = *pointer;
+	return 0;
+}
+
 static inline struct fwnode_handle *dev_fwnode(struct device *dev)
 {
 	return IS_ENABLED(CONFIG_OF) && dev->of_node ?
@@ -566,8 +594,7 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
 					   val, 1);
 	else if (is_pset_node(fwnode))
-		return pset_prop_read_string_array(to_pset_node(fwnode),
-						   propname, val, 1);
+		return pset_prop_read_string(to_pset_node(fwnode), propname, val);
 	return -ENXIO;
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_string);
diff --git a/include/linux/property.h b/include/linux/property.h
index c29460a..69a8a08 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -145,19 +145,34 @@ static inline int fwnode_property_read_u64(struct fwnode_handle *fwnode,
  * struct property_entry - "Built-in" device property representation.
  * @name: Name of the property.
  * @length: Length of data making up the value.
- * @value: Value of the property (an array of items of the given type).
+ * @is_array: True when the property is an array.
+ * @is_string: True when property is a string.
+ * @pointer: Pointer to the property (an array of items of the given type).
+ * @value: Value of the property (when it is a single item of the given type).
  */
 struct property_entry {
 	const char *name;
 	size_t length;
+	bool is_array;
+	bool is_string;
 	union {
-		void *raw_data;
-		u8 *u8_data;
-		u16 *u16_data;
-		u32 *u32_data;
-		u64 *u64_data;
-		const char **str;
-	} value;
+		union {
+			void *raw_data;
+			u8 *u8_data;
+			u16 *u16_data;
+			u32 *u32_data;
+			u64 *u64_data;
+			const char **str;
+		} pointer;
+		union {
+			unsigned long long raw_data;
+			u8 u8_data;
+			u16 u16_data;
+			u32 u32_data;
+			u64 u64_data;
+			const char *str;
+		} value;
+	};
 };
 
 /**
-- 
2.6.2

[PATCH v2 05/16] device property: helper macros for property entry creation

[Andy Shevchenko]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Marcos for easier creation of build-in property entries.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/property.h | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/include/linux/property.h b/include/linux/property.h
index 69a8a08..e4f29d8 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -175,6 +175,61 @@ struct property_entry {
 	};
 };
 
+#define PROPERTY_ENTRY_INTEGER_ARRAY(_name_, _type_, _val_)	\
+{								\
+	.name = _name_,						\
+	.length = ARRAY_SIZE(_val_) * sizeof(_type_),		\
+	.is_array = true,					\
+	.pointer._type_##_data = _val_,				\
+}
+
+#define PROPERTY_ENTRY_U8_ARRAY(_name_, _val_)			\
+	PROPERTY_ENTRY_INTEGER_ARRAY(_name_, u8, _val_)
+#define PROPERTY_ENTRY_U16_ARRAY(_name_, _val_)			\
+	PROPERTY_ENTRY_INTEGER_ARRAY(_name_, u16, _val_)
+#define PROPERTY_ENTRY_U32_ARRAY(_name_, _val_)			\
+	PROPERTY_ENTRY_INTEGER_ARRAY(_name_, u32, _val_)
+#define PROPERTY_ENTRY_U64_ARRAY(_name_, _val_)			\
+	PROPERTY_ENTRY_INTEGER_ARRAY(_name_, u64, _val_)
+
+#define PROPERTY_ENTRY_STRING_ARRAY(_name_, _val_)		\
+{								\
+	.name = _name_,						\
+	.length = ARRAY_SIZE(_val_) * sizeof(const char *),	\
+	.is_array = true,					\
+	.is_string = true,					\
+	.pointer.str = _val_,					\
+}
+
+#define PROPERTY_ENTRY_INTEGER(_name_, _type_, _val_)	\
+{							\
+	.name = _name_,					\
+	.length = sizeof(_type_),			\
+	.value._type_##_data = _val_,			\
+}
+
+#define PROPERTY_ENTRY_U8(_name_, _val_)		\
+	PROPERTY_ENTRY_INTEGER(_name_, u8, _val_)
+#define PROPERTY_ENTRY_U16(_name_, _val_)		\
+	PROPERTY_ENTRY_INTEGER(_name_, u16, _val_)
+#define PROPERTY_ENTRY_U32(_name_, _val_)		\
+	PROPERTY_ENTRY_INTEGER(_name_, u32, _val_)
+#define PROPERTY_ENTRY_U64(_name_, _val_)		\
+	PROPERTY_ENTRY_INTEGER(_name_, u64, _val_)
+
+#define PROPERTY_ENTRY_STRING(_name_, _val_)		\
+{							\
+	.name = _name_,					\
+	.length = sizeof(_val_),			\
+	.is_string = true,				\
+	.value.str = _val_,				\
+}
+
+#define PROPERTY_ENTRY_BOOL(_name_)		\
+{						\
+	.name = _name_,				\
+}
+
 /**
  * struct property_set - Collection of "built-in" device properties.
  * @fwnode: Handle to be pointed to by the fwnode field of struct device.
-- 
2.6.2

[PATCH v2 06/16] device property: improve readability of macros

[Andy Shevchenko]

There is no functional change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/property.c  | 28 ++++++++++++++--------------
 include/linux/property.h |  4 ++--
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index ad3cb09..a3538cb 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -400,29 +400,29 @@ int device_property_match_string(struct device *dev, const char *propname,
 }
 EXPORT_SYMBOL_GPL(device_property_match_string);
 
-#define OF_DEV_PROP_READ_ARRAY(node, propname, type, val, nval) \
-	(val) ? of_property_read_##type##_array((node), (propname), (val), (nval)) \
+#define OF_DEV_PROP_READ_ARRAY(node, propname, type, val, nval)				\
+	(val) ? of_property_read_##type##_array((node), (propname), (val), (nval))	\
 	      : of_property_count_elems_of_size((node), (propname), sizeof(type))
 
 #define PSET_PROP_READ_ARRAY(node, propname, type, val, nval)				\
 	(val) ? pset_prop_read_##type##_array((node), (propname), (val), (nval))	\
 	      : pset_prop_count_elems_of_size((node), (propname), sizeof(type))
 
-#define FWNODE_PROP_READ_ARRAY(_fwnode_, _propname_, _type_, _proptype_, _val_, _nval_) \
-({ \
-	int _ret_; \
-	if (is_of_node(_fwnode_)) \
-		_ret_ = OF_DEV_PROP_READ_ARRAY(to_of_node(_fwnode_), _propname_, \
-					       _type_, _val_, _nval_); \
-	else if (is_acpi_node(_fwnode_)) \
-		_ret_ = acpi_node_prop_read(_fwnode_, _propname_, _proptype_, \
-					    _val_, _nval_); \
+#define FWNODE_PROP_READ_ARRAY(_fwnode_, _propname_, _type_, _proptype_, _val_, _nval_)	\
+({											\
+	int _ret_;									\
+	if (is_of_node(_fwnode_))							\
+		_ret_ = OF_DEV_PROP_READ_ARRAY(to_of_node(_fwnode_), _propname_,	\
+					       _type_, _val_, _nval_);			\
+	else if (is_acpi_node(_fwnode_))						\
+		_ret_ = acpi_node_prop_read(_fwnode_, _propname_, _proptype_,		\
+					    _val_, _nval_);				\
 	else if (is_pset_node(_fwnode_)) 						\
 		_ret_ = PSET_PROP_READ_ARRAY(to_pset_node(_fwnode_), _propname_,	\
 					     _type_, _val_, _nval_);			\
-	else \
-		_ret_ = -ENXIO; \
-	_ret_; \
+	else										\
+		_ret_ = -ENXIO;								\
+	_ret_;										\
 })
 
 /**
diff --git a/include/linux/property.h b/include/linux/property.h
index e4f29d8..d1cf208 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -73,8 +73,8 @@ int fwnode_property_match_string(struct fwnode_handle *fwnode,
 struct fwnode_handle *device_get_next_child_node(struct device *dev,
 						 struct fwnode_handle *child);
 
-#define device_for_each_child_node(dev, child) \
-	for (child = device_get_next_child_node(dev, NULL); child; \
+#define device_for_each_child_node(dev, child)				\
+	for (child = device_get_next_child_node(dev, NULL); child;	\
 	     child = device_get_next_child_node(dev, child))
 
 void fwnode_handle_put(struct fwnode_handle *fwnode);
-- 
2.6.2

[PATCH v2 07/16] device property: return -EINVAL when property isn't found in ACPI

[Andy Shevchenko]

Change return code to be in align with OF and built-in device properties error
codes. In particular -EINVAL means property is not found.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/property.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 88f4306..2aee416 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -346,7 +346,7 @@ void acpi_free_properties(struct acpi_device *adev)
  *
  * Return: %0 if property with @name has been found (success),
  *         %-EINVAL if the arguments are invalid,
- *         %-ENODATA if the property doesn't exist,
+ *         %-EINVAL if the property doesn't exist,
  *         %-EPROTO if the property value type doesn't match @type.
  */
 static int acpi_data_get_property(struct acpi_device_data *data,
@@ -360,7 +360,7 @@ static int acpi_data_get_property(struct acpi_device_data *data,
 		return -EINVAL;
 
 	if (!data->pointer || !data->properties)
-		return -ENODATA;
+		return -EINVAL;
 
 	properties = data->properties;
 	for (i = 0; i < properties->package.count; i++) {
@@ -375,13 +375,13 @@ static int acpi_data_get_property(struct acpi_device_data *data,
 		if (!strcmp(name, propname->string.pointer)) {
 			if (type != ACPI_TYPE_ANY && propvalue->type != type)
 				return -EPROTO;
-			else if (obj)
+			if (obj)
 				*obj = propvalue;
 
 			return 0;
 		}
 	}
-	return -ENODATA;
+	return -EINVAL;
 }
 
 /**
@@ -439,7 +439,7 @@ int acpi_node_prop_get(struct fwnode_handle *fwnode, const char *propname,
  *
  * Return: %0 if array property (package) with @name has been found (success),
  *         %-EINVAL if the arguments are invalid,
- *         %-ENODATA if the property doesn't exist,
+ *         %-EINVAL if the property doesn't exist,
  *         %-EPROTO if the property is not a package or the type of its elements
  *           doesn't match @type.
  */
-- 
2.6.2

[PATCH v2 08/16] device property: Fallback to secondary fwnode if primary misses the property

[Andy Shevchenko]

The struct fwnode has notion of secondary fwnode. This is supposed to used
as fallback if the primary firmware interface (DT, ACPI) does not have the
property in question.

However, the current implementation never checks the secondary node which
prevents one to add default "built-in" properties to devices.

This patch adds fallback to the secondary fwnode if the primary fwnode
returns that the property does not exists.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/property.c | 109 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 78 insertions(+), 31 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index a3538cb..ebcbe34 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -214,12 +214,8 @@ bool device_property_present(struct device *dev, const char *propname)
 }
 EXPORT_SYMBOL_GPL(device_property_present);
 
-/**
- * fwnode_property_present - check if a property of a firmware node is present
- * @fwnode: Firmware node whose property to check
- * @propname: Name of the property
- */
-bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
+static bool __fwnode_property_present(struct fwnode_handle *fwnode,
+				      const char *propname)
 {
 	if (is_of_node(fwnode))
 		return of_property_read_bool(to_of_node(fwnode), propname);
@@ -229,6 +225,21 @@ bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
 		return !!pset_prop_get(to_pset_node(fwnode), propname);
 	return false;
 }
+
+/**
+ * fwnode_property_present - check if a property of a firmware node is present
+ * @fwnode: Firmware node whose property to check
+ * @propname: Name of the property
+ */
+bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
+{
+	bool ret;
+
+	ret = __fwnode_property_present(fwnode, propname);
+	if (ret == false && fwnode->secondary)
+		ret = __fwnode_property_present(fwnode->secondary, propname);
+	return ret;
+}
 EXPORT_SYMBOL_GPL(fwnode_property_present);
 
 /**
@@ -408,7 +419,7 @@ EXPORT_SYMBOL_GPL(device_property_match_string);
 	(val) ? pset_prop_read_##type##_array((node), (propname), (val), (nval))	\
 	      : pset_prop_count_elems_of_size((node), (propname), sizeof(type))
 
-#define FWNODE_PROP_READ_ARRAY(_fwnode_, _propname_, _type_, _proptype_, _val_, _nval_)	\
+#define FWNODE_PROP_READ(_fwnode_, _propname_, _type_, _proptype_, _val_, _nval_)	\
 ({											\
 	int _ret_;									\
 	if (is_of_node(_fwnode_))							\
@@ -425,6 +436,17 @@ EXPORT_SYMBOL_GPL(device_property_match_string);
 	_ret_;										\
 })
 
+#define FWNODE_PROP_READ_ARRAY(_fwnode_, _propname_, _type_, _proptype_, _val_, _nval_)	\
+({											\
+	int _ret_;									\
+	_ret_ = FWNODE_PROP_READ(_fwnode_, _propname_, _type_, _proptype_,		\
+				 _val_, _nval_);					\
+	if (_ret_ == -EINVAL && _fwnode_->secondary)					\
+		_ret_ = FWNODE_PROP_READ(_fwnode_->secondary, _propname_, _type_,	\
+				_proptype_, _val_, _nval_);				\
+	_ret_;										\
+})
+
 /**
  * fwnode_property_read_u8_array - return a u8 array property of firmware node
  * @fwnode: Firmware node to get the property of
@@ -529,6 +551,41 @@ int fwnode_property_read_u64_array(struct fwnode_handle *fwnode,
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_u64_array);
 
+static int __fwnode_property_read_string_array(struct fwnode_handle *fwnode,
+					       const char *propname,
+					       const char **val, size_t nval)
+{
+	if (is_of_node(fwnode))
+		return val ?
+			of_property_read_string_array(to_of_node(fwnode),
+						      propname, val, nval) :
+			of_property_count_strings(to_of_node(fwnode), propname);
+	else if (is_acpi_node(fwnode))
+		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
+					   val, nval);
+	else if (is_pset_node(fwnode))
+		return val ?
+			pset_prop_read_string_array(to_pset_node(fwnode),
+						    propname, val, nval) :
+			pset_prop_count_elems_of_size(to_pset_node(fwnode),
+						      propname,
+						      sizeof(const char *));
+	return -ENXIO;
+}
+
+static int __fwnode_property_read_string(struct fwnode_handle *fwnode,
+					 const char *propname, const char **val)
+{
+	if (is_of_node(fwnode))
+		return of_property_read_string(to_of_node(fwnode), propname, val);
+	else if (is_acpi_node(fwnode))
+		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
+					   val, 1);
+	else if (is_pset_node(fwnode))
+		return pset_prop_read_string(to_pset_node(fwnode), propname, val);
+	return -ENXIO;
+}
+
 /**
  * fwnode_property_read_string_array - return string array property of a node
  * @fwnode: Firmware node to get the property of
@@ -551,22 +608,13 @@ int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
 				      const char *propname, const char **val,
 				      size_t nval)
 {
-	if (is_of_node(fwnode))
-		return val ?
-			of_property_read_string_array(to_of_node(fwnode),
-						      propname, val, nval) :
-			of_property_count_strings(to_of_node(fwnode), propname);
-	else if (is_acpi_node(fwnode))
-		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
-					   val, nval);
-	else if (is_pset_node(fwnode))
-		return val ?
-			pset_prop_read_string_array(to_pset_node(fwnode),
-						    propname, val, nval) :
-			pset_prop_count_elems_of_size(to_pset_node(fwnode),
-						      propname,
-						      sizeof(const char *));
-	return -ENXIO;
+	int ret;
+
+	ret = __fwnode_property_read_string_array(fwnode, propname, val, nval);
+	if (ret == -EINVAL && fwnode->secondary)
+		ret = __fwnode_property_read_string_array(fwnode->secondary,
+							  propname, val, nval);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_string_array);
 
@@ -588,14 +636,13 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_string_array);
 int fwnode_property_read_string(struct fwnode_handle *fwnode,
 				const char *propname, const char **val)
 {
-	if (is_of_node(fwnode))
-		return of_property_read_string(to_of_node(fwnode), propname, val);
-	else if (is_acpi_node(fwnode))
-		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
-					   val, 1);
-	else if (is_pset_node(fwnode))
-		return pset_prop_read_string(to_pset_node(fwnode), propname, val);
-	return -ENXIO;
+	int ret;
+
+	ret = __fwnode_property_read_string(fwnode, propname, val);
+	if (ret == -EINVAL && fwnode->secondary)
+		ret = __fwnode_property_read_string(fwnode->secondary,
+						    propname, val);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_string);
 
-- 
2.6.2

[PATCH v2 09/16] device property: Take a copy of the property set

[Andy Shevchenko]

From: Mika Westerberg <mika.westerberg@linux.intel.com>

It is convenient if the property set associated with the device secondary
firmware node is a copy of the original. This allows passing property set
from a stack for example for devices created dynamically. This also ties
the property set lifetime to the associated device.

Because of that we provide new function device_remove_property_set() that
is used to disassociate and release memory allocated for the property set.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/property.c  | 191 ++++++++++++++++++++++++++++++++++++++++++-----
 include/linux/property.h |   3 +-
 2 files changed, 175 insertions(+), 19 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index ebcbe34..0b22c8a 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -19,24 +19,6 @@
 #include <linux/etherdevice.h>
 #include <linux/phy.h>
 
-/**
- * device_add_property_set - Add a collection of properties to a device object.
- * @dev: Device to add properties to.
- * @pset: Collection of properties to add.
- *
- * Associate a collection of device properties represented by @pset with @dev
- * as its secondary firmware node.
- */
-void device_add_property_set(struct device *dev, struct property_set *pset)
-{
-	if (!pset)
-		return;
-
-	pset->fwnode.type = FWNODE_PDATA;
-	set_secondary_fwnode(dev, &pset->fwnode);
-}
-EXPORT_SYMBOL_GPL(device_add_property_set);
-
 static inline bool is_pset_node(struct fwnode_handle *fwnode)
 {
 	return fwnode && fwnode->type == FWNODE_PDATA;
@@ -693,6 +675,179 @@ out:
 EXPORT_SYMBOL_GPL(fwnode_property_match_string);
 
 /**
+ * pset_free_set - releases memory allocated for copied property set
+ * @pset: Property set to release
+ *
+ * Function takes previously copied property set and releases all the
+ * memory allocated to it.
+ */
+static void pset_free_set(struct property_set *pset)
+{
+	const struct property_entry *prop;
+	size_t i, nval;
+
+	if (!pset)
+		return;
+
+	for (prop = pset->properties; prop->name; prop++) {
+		if (prop->is_array) {
+			if (prop->is_string && prop->pointer.str) {
+				nval = prop->length / sizeof(const char *);
+				for (i = 0; i < nval; i++)
+					kfree(prop->pointer.str[i]);
+			}
+			kfree(prop->pointer.raw_data);
+		} else if (prop->is_string) {
+			kfree(prop->value.str);
+		}
+		kfree(prop->name);
+	}
+
+	kfree(pset->properties);
+	kfree(pset);
+}
+
+static int pset_copy_entry(struct property_entry *dst,
+			   const struct property_entry *src)
+{
+	const char **d, **s;
+	size_t i, nval;
+
+	dst->name = kstrdup(src->name, GFP_KERNEL);
+	if (!dst->name)
+		return -ENOMEM;
+
+	if (src->is_array) {
+		if (src->is_string) {
+			nval = src->length / sizeof(const char *);
+			dst->pointer.str = kcalloc(nval, sizeof(const char *),
+						   GFP_KERNEL);
+			if (!dst->pointer.str)
+				return -ENOMEM;
+
+			d = dst->pointer.str;
+			s = src->pointer.str;
+			for (i = 0; i < nval; i++) {
+				d[i] = kstrdup(s[i], GFP_KERNEL);
+				if (!d[i] && s[i])
+					return -ENOMEM;
+			}
+		} else {
+			dst->pointer.raw_data = kmemdup(src->pointer.raw_data,
+							src->length, GFP_KERNEL);
+			if (!dst->pointer.raw_data)
+				return -ENOMEM;
+		}
+	} else if (src->is_string) {
+		dst->value.str = kstrdup(src->value.str, GFP_KERNEL);
+		if (!dst->value.str && src->value.str)
+			return -ENOMEM;
+	} else {
+		dst->value.raw_data = src->value.raw_data;
+	}
+
+	dst->length = src->length;
+	dst->is_array = src->is_array;
+	dst->is_string = src->is_string;
+
+	return 0;
+}
+
+/**
+ * pset_copy_set - copies property set
+ * @pset: Property set to copy
+ *
+ * This function takes a deep copy of the given property set and returns
+ * pointer to the copy. Call device_free_property_set() to free resources
+ * allocated in this function.
+ *
+ * Return: Pointer to the new property set or error pointer.
+ */
+static struct property_set *pset_copy_set(const struct property_set *pset)
+{
+	const struct property_entry *entry;
+	struct property_set *p;
+	size_t i, n = 0;
+
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return ERR_PTR(-ENOMEM);
+
+	while (pset->properties[n].name)
+		n++;
+
+	p->properties = kcalloc(n + 1, sizeof(*entry), GFP_KERNEL);
+	if (!p->properties) {
+		kfree(p);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	for (i = 0; i < n; i++) {
+		int ret = pset_copy_entry(&p->properties[i],
+					  &pset->properties[i]);
+		if (ret) {
+			pset_free_set(p);
+			return ERR_PTR(ret);
+		}
+	}
+
+	return p;
+}
+
+/**
+ * device_remove_property_set - Remove properties from a device object.
+ * @dev: Device whose properties to remove.
+ *
+ * The function removes properties previously associated to the device
+ * secondary firmware node with device_add_property_set(). Memory allocated
+ * to the properties will also be released.
+ */
+void device_remove_property_set(struct device *dev)
+{
+	struct fwnode_handle *fwnode;
+
+	fwnode = dev_fwnode(dev);
+	if (!fwnode)
+		return;
+	/*
+	 * Pick either primary or secondary node depending which one holds
+	 * the pset. If there is no real firmware node (ACPI/DT) primary
+	 * will hold the pset.
+	 */
+	if (!is_pset_node(fwnode))
+		fwnode = fwnode->secondary;
+	if (!IS_ERR(fwnode) && is_pset_node(fwnode))
+		pset_free_set(to_pset_node(fwnode));
+	set_secondary_fwnode(dev, NULL);
+}
+EXPORT_SYMBOL_GPL(device_remove_property_set);
+
+/**
+ * device_add_property_set - Add a collection of properties to a device object.
+ * @dev: Device to add properties to.
+ * @pset: Collection of properties to add.
+ *
+ * Associate a collection of device properties represented by @pset with @dev
+ * as its secondary firmware node. The function takes a copy of @pset.
+ */
+int device_add_property_set(struct device *dev, const struct property_set *pset)
+{
+	struct property_set *p;
+
+	if (!pset)
+		return -EINVAL;
+
+	p = pset_copy_set(pset);
+	if (IS_ERR(p))
+		return PTR_ERR(p);
+
+	p->fwnode.type = FWNODE_PDATA;
+	set_secondary_fwnode(dev, &p->fwnode);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(device_add_property_set);
+
+/**
  * device_get_next_child_node - Return the next child node handle for a device
  * @dev: Device to find the next child node for.
  * @child: Handle to one of the device's child nodes or a null handle.
diff --git a/include/linux/property.h b/include/linux/property.h
index d1cf208..3a8c7d7 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -240,7 +240,8 @@ struct property_set {
 	struct property_entry *properties;
 };
 
-void device_add_property_set(struct device *dev, struct property_set *pset);
+int device_add_property_set(struct device *dev, const struct property_set *pset);
+void device_remove_property_set(struct device *dev);
 
 bool device_dma_supported(struct device *dev);
 
-- 
2.6.2

[PATCH v2 10/16] driver core: platform: Add support for built-in device properties

[Andy Shevchenko]

From: Mika Westerberg <mika.westerberg@linux.intel.com>

Make it possible to pass built-in device properties to platform device
drivers. This is useful if the system does not have any firmware interface
like Device Tree or ACPI which provides these.

Properties associated with the platform device will be automatically
released when the corresponding device is removed.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/platform.c         | 25 +++++++++++++++++++++++++
 include/linux/platform_device.h |  5 +++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 1dd6d3b..d77ed0c 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -26,6 +26,7 @@
 #include <linux/acpi.h>
 #include <linux/clk/clk-conf.h>
 #include <linux/limits.h>
+#include <linux/property.h>
 
 #include "base.h"
 #include "power/power.h"
@@ -299,6 +300,22 @@ int platform_device_add_data(struct platform_device *pdev, const void *data,
 EXPORT_SYMBOL_GPL(platform_device_add_data);
 
 /**
+ * platform_device_add_properties - add built-in properties to a platform device
+ * @pdev: platform device to add properties to
+ * @pset: properties to add
+ *
+ * The function will take deep copy of the properties in @pset and attach
+ * the copy to the platform device. The memory associated with properties
+ * will be freed when the platform device is released.
+ */
+int platform_device_add_properties(struct platform_device *pdev,
+				   const struct property_set *pset)
+{
+	return device_add_property_set(&pdev->dev, pset);
+}
+EXPORT_SYMBOL_GPL(platform_device_add_properties);
+
+/**
  * platform_device_add - add a platform device to device hierarchy
  * @pdev: platform device we're adding
  *
@@ -409,6 +426,8 @@ void platform_device_del(struct platform_device *pdev)
 			if (r->parent)
 				release_resource(r);
 		}
+
+		device_remove_property_set(&pdev->dev);
 	}
 }
 EXPORT_SYMBOL_GPL(platform_device_del);
@@ -487,6 +506,12 @@ struct platform_device *platform_device_register_full(
 	if (ret)
 		goto err;
 
+	if (pdevinfo->pset) {
+		ret = platform_device_add_properties(pdev, pdevinfo->pset);
+		if (ret)
+			goto err;
+	}
+
 	ret = platform_device_add(pdev);
 	if (ret) {
 err:
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index dc777be..dba40b1 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -18,6 +18,7 @@
 #define PLATFORM_DEVID_AUTO	(-2)
 
 struct mfd_cell;
+struct property_set;
 
 struct platform_device {
 	const char	*name;
@@ -70,6 +71,8 @@ struct platform_device_info {
 		const void *data;
 		size_t size_data;
 		u64 dma_mask;
+
+		const struct property_set *pset;
 };
 extern struct platform_device *platform_device_register_full(
 		const struct platform_device_info *pdevinfo);
@@ -167,6 +170,8 @@ extern int platform_device_add_resources(struct platform_device *pdev,
 					 unsigned int num);
 extern int platform_device_add_data(struct platform_device *pdev,
 				    const void *data, size_t size);
+extern int platform_device_add_properties(struct platform_device *pdev,
+					  const struct property_set *pset);
 extern int platform_device_add(struct platform_device *pdev);
 extern void platform_device_del(struct platform_device *pdev);
 extern void platform_device_put(struct platform_device *pdev);
-- 
2.6.2

[PATCH v2 11/16] driver core: Do not overwrite secondary fwnode with NULL if it is set

[Andy Shevchenko]

From: Mika Westerberg <mika.westerberg@linux.intel.com>

If multiple devices share single firmware node like it is case with MFD
devices, the same firmware node (ACPI) is assigned to all of them. The
function also modifies the shared firmware node in order to preserve
secondary firmware node of the device in question.

If the new device which is sharing the firmware node does not have
secondary node it will be NULL which will be assigned to the secondary node
of the shared firmware node losing all built-in properties.

Prevent this by setting the secondary firmware node only if the replacement
is non-NULL.

Print also warning if someone tries to overwrite secondary node that has
already been assigned.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index b7d56c5..0a8bdad 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2261,7 +2261,10 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
 		if (fwnode_is_primary(fn))
 			fn = fn->secondary;
 
-		fwnode->secondary = fn;
+		if (fn) {
+			WARN_ON(fwnode->secondary);
+			fwnode->secondary = fn;
+		}
 		dev->fwnode = fwnode;
 	} else {
 		dev->fwnode = fwnode_is_primary(dev->fwnode) ?
-- 
2.6.2

[PATCH v2 12/16] mfd: core: propagate device properties to sub devices drivers

[Andy Shevchenko]

In the similar way like we do for the platform data we propagate the device
properties. For example, in case of Intel LPSS drivers we may provide a
specific property to tell the actual device driver an additional information
such as platform name.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/mfd-core.c   | 7 +++++++
 include/linux/mfd/core.h | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 60b60dc..88bd1b1 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/acpi.h>
+#include <linux/property.h>
 #include <linux/mfd/core.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
@@ -192,6 +193,12 @@ static int mfd_add_device(struct device *parent, int id,
 			goto fail_alias;
 	}
 
+	if (cell->pset) {
+		ret = platform_device_add_properties(pdev, cell->pset);
+		if (ret)
+			goto fail_alias;
+	}
+
 	ret = mfd_platform_add_cell(pdev, cell, usage_count);
 	if (ret)
 		goto fail_alias;
diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index 27dac3f..bc6f7e0 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -17,6 +17,7 @@
 #include <linux/platform_device.h>
 
 struct irq_domain;
+struct property_set;
 
 /* Matches ACPI PNP id, either _HID or _CID, or ACPI _ADR */
 struct mfd_cell_acpi_match {
@@ -44,6 +45,10 @@ struct mfd_cell {
 	/* platform data passed to the sub devices drivers */
 	void			*platform_data;
 	size_t			pdata_size;
+
+	/* device properties passed to the sub devices drivers */
+	const struct property_set *pset;
+
 	/*
 	 * Device Tree compatible string
 	 * See: Documentation/devicetree/usage-model.txt Chapter 2.2 for details
-- 
2.6.2

[PATCH v2 13/16] mfd: intel-lpss: Add support for passing device properties

[Andy Shevchenko]

From: Mika Westerberg <mika.westerberg@linux.intel.com>

If the boot firmware does not support ACPI we need a way to pass device
configuration information to the drivers. The unified device properties API
already supports passing platform data via properties so let's take
advantage of that and allow probe drivers to pass set of properties to the
host controller driver.

In order to do that we need to be able to modify the MFD cell corresponding
the host controller, so make the core driver to take copy of the cell
instead of using it directly. Then we can assign info->pset to the
resulting copy of a cell and let the MFD core to assign that to the
resulting device.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel-lpss.c | 16 ++++++++++++----
 drivers/mfd/intel-lpss.h |  2 ++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
index 6255513..1743788 100644
--- a/drivers/mfd/intel-lpss.c
+++ b/drivers/mfd/intel-lpss.c
@@ -24,6 +24,7 @@
 #include <linux/mfd/core.h>
 #include <linux/pm_qos.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/seq_file.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 
@@ -72,7 +73,7 @@ struct intel_lpss {
 	enum intel_lpss_dev_type type;
 	struct clk *clk;
 	struct clk_lookup *clock;
-	const struct mfd_cell *cell;
+	struct mfd_cell *cell;
 	struct device *dev;
 	void __iomem *priv;
 	int devid;
@@ -217,6 +218,7 @@ static void intel_lpss_ltr_hide(struct intel_lpss *lpss)
 
 static int intel_lpss_assign_devs(struct intel_lpss *lpss)
 {
+	const struct mfd_cell *cell;
 	unsigned int type;
 
 	type = lpss->caps & LPSS_PRIV_CAPS_TYPE_MASK;
@@ -224,18 +226,22 @@ static int intel_lpss_assign_devs(struct intel_lpss *lpss)
 
 	switch (type) {
 	case LPSS_DEV_I2C:
-		lpss->cell = &intel_lpss_i2c_cell;
+		cell = &intel_lpss_i2c_cell;
 		break;
 	case LPSS_DEV_UART:
-		lpss->cell = &intel_lpss_uart_cell;
+		cell = &intel_lpss_uart_cell;
 		break;
 	case LPSS_DEV_SPI:
-		lpss->cell = &intel_lpss_spi_cell;
+		cell = &intel_lpss_spi_cell;
 		break;
 	default:
 		return -ENODEV;
 	}
 
+	lpss->cell = devm_kmemdup(lpss->dev, cell, sizeof(*cell), GFP_KERNEL);
+	if (!lpss->cell)
+		return -ENOMEM;
+
 	lpss->type = type;
 
 	return 0;
@@ -401,6 +407,8 @@ int intel_lpss_probe(struct device *dev,
 	if (ret)
 		return ret;
 
+	lpss->cell->pset = info->pset;
+
 	intel_lpss_init_dev(lpss);
 
 	lpss->devid = ida_simple_get(&intel_lpss_devid_ida, 0, 0, GFP_KERNEL);
diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
index 2c7f8d7..0dcea9e 100644
--- a/drivers/mfd/intel-lpss.h
+++ b/drivers/mfd/intel-lpss.h
@@ -16,12 +16,14 @@
 
 struct device;
 struct resource;
+struct property_set;
 
 struct intel_lpss_platform_info {
 	struct resource *mem;
 	int irq;
 	unsigned long clk_rate;
 	const char *clk_con_id;
+	struct property_set *pset;
 };
 
 int intel_lpss_probe(struct device *dev,
-- 
2.6.2

[PATCH v2 14/16] mfd: intel-lpss: Pass SDA hold time to I2C host controller driver

[Andy Shevchenko]

From: Mika Westerberg <mika.westerberg@linux.intel.com>

Intel Skylake the LPSS I2C pad circuit has internal delays that require
programming non-zero SDA hold time for the I2C host controller. If this is
not done communication to slave devices may fail with arbitration lost
errors like the one seen below taken from Lenovo Yoga 900:

  i2c_hid i2c-SYNA2B29:00: Fetching the HID descriptor
  i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=20 00
  i2c_designware i2c_designware.1: i2c_dw_handle_tx_abort: lost arbitration

To fix this we follow what the Windows driver is doing and pass the default
SDA hold time of 230 ns to all Intel Skylake host controllers. This still
allows the platform to override these values by passing special ACPI
methods SSCN and FMCN.

Reported-by: Kevin Fenzi <kevin@scrye.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel-lpss-acpi.c | 19 +++++++++++++++++--
 drivers/mfd/intel-lpss-pci.c  | 31 +++++++++++++++++++++++--------
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/intel-lpss-acpi.c b/drivers/mfd/intel-lpss-acpi.c
index b6fd904..06f00d6 100644
--- a/drivers/mfd/intel-lpss-acpi.c
+++ b/drivers/mfd/intel-lpss-acpi.c
@@ -18,6 +18,7 @@
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 
 #include "intel-lpss.h"
 
@@ -25,6 +26,20 @@ static const struct intel_lpss_platform_info spt_info = {
 	.clk_rate = 120000000,
 };
 
+static struct property_entry spt_i2c_properties[] = {
+	PROPERTY_ENTRY_U32("i2c-sda-hold-time-ns", 230),
+	{ },
+};
+
+static struct property_set spt_i2c_pset = {
+	.properties = spt_i2c_properties,
+};
+
+static const struct intel_lpss_platform_info spt_i2c_info = {
+	.clk_rate = 120000000,
+	.pset = &spt_i2c_pset,
+};
+
 static const struct intel_lpss_platform_info bxt_info = {
 	.clk_rate = 100000000,
 };
@@ -35,8 +50,8 @@ static const struct intel_lpss_platform_info bxt_i2c_info = {
 
 static const struct acpi_device_id intel_lpss_acpi_ids[] = {
 	/* SPT */
-	{ "INT3446", (kernel_ulong_t)&spt_info },
-	{ "INT3447", (kernel_ulong_t)&spt_info },
+	{ "INT3446", (kernel_ulong_t)&spt_i2c_info },
+	{ "INT3447", (kernel_ulong_t)&spt_i2c_info },
 	/* BXT */
 	{ "80860AAC", (kernel_ulong_t)&bxt_i2c_info },
 	{ "80860ABC", (kernel_ulong_t)&bxt_info },
diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
index 5bfdfcc..a677480 100644
--- a/drivers/mfd/intel-lpss-pci.c
+++ b/drivers/mfd/intel-lpss-pci.c
@@ -17,6 +17,7 @@
 #include <linux/pci.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 
 #include "intel-lpss.h"
 
@@ -65,6 +66,20 @@ static const struct intel_lpss_platform_info spt_info = {
 	.clk_rate = 120000000,
 };
 
+static struct property_entry spt_i2c_properties[] = {
+	PROPERTY_ENTRY_U32("i2c-sda-hold-time-ns", 230),
+	{ },
+};
+
+static struct property_set spt_i2c_pset = {
+	.properties = spt_i2c_properties,
+};
+
+static const struct intel_lpss_platform_info spt_i2c_info = {
+	.clk_rate = 120000000,
+	.pset = &spt_i2c_pset,
+};
+
 static const struct intel_lpss_platform_info spt_uart_info = {
 	.clk_rate = 120000000,
 	.clk_con_id = "baudclk",
@@ -121,20 +136,20 @@ static const struct pci_device_id intel_lpss_pci_ids[] = {
 	{ PCI_VDEVICE(INTEL, 0x9d28), (kernel_ulong_t)&spt_uart_info },
 	{ PCI_VDEVICE(INTEL, 0x9d29), (kernel_ulong_t)&spt_info },
 	{ PCI_VDEVICE(INTEL, 0x9d2a), (kernel_ulong_t)&spt_info },
-	{ PCI_VDEVICE(INTEL, 0x9d60), (kernel_ulong_t)&spt_info },
-	{ PCI_VDEVICE(INTEL, 0x9d61), (kernel_ulong_t)&spt_info },
-	{ PCI_VDEVICE(INTEL, 0x9d62), (kernel_ulong_t)&spt_info },
-	{ PCI_VDEVICE(INTEL, 0x9d63), (kernel_ulong_t)&spt_info },
-	{ PCI_VDEVICE(INTEL, 0x9d64), (kernel_ulong_t)&spt_info },
-	{ PCI_VDEVICE(INTEL, 0x9d65), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0x9d60), (kernel_ulong_t)&spt_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0x9d61), (kernel_ulong_t)&spt_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0x9d62), (kernel_ulong_t)&spt_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0x9d63), (kernel_ulong_t)&spt_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0x9d64), (kernel_ulong_t)&spt_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0x9d65), (kernel_ulong_t)&spt_i2c_info },
 	{ PCI_VDEVICE(INTEL, 0x9d66), (kernel_ulong_t)&spt_uart_info },
 	/* SPT-H */
 	{ PCI_VDEVICE(INTEL, 0xa127), (kernel_ulong_t)&spt_uart_info },
 	{ PCI_VDEVICE(INTEL, 0xa128), (kernel_ulong_t)&spt_uart_info },
 	{ PCI_VDEVICE(INTEL, 0xa129), (kernel_ulong_t)&spt_info },
 	{ PCI_VDEVICE(INTEL, 0xa12a), (kernel_ulong_t)&spt_info },
-	{ PCI_VDEVICE(INTEL, 0xa160), (kernel_ulong_t)&spt_info },
-	{ PCI_VDEVICE(INTEL, 0xa161), (kernel_ulong_t)&spt_info },
+	{ PCI_VDEVICE(INTEL, 0xa160), (kernel_ulong_t)&spt_i2c_info },
+	{ PCI_VDEVICE(INTEL, 0xa161), (kernel_ulong_t)&spt_i2c_info },
 	{ PCI_VDEVICE(INTEL, 0xa166), (kernel_ulong_t)&spt_uart_info },
 	{ }
 };
-- 
2.6.2

[PATCH v2 15/16] mfd: intel-lpss: Pass HSUART configuration via properties

[Andy Shevchenko]

The HS-UART host controller driver needs to know certain properties like
width of the register set if it cannot get that information from ACPI or
DT. In order to support non-ACPI systems we pass this information to the
driver via device properties.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel-lpss-pci.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
index a677480..a7136c7 100644
--- a/drivers/mfd/intel-lpss-pci.c
+++ b/drivers/mfd/intel-lpss-pci.c
@@ -80,9 +80,21 @@ static const struct intel_lpss_platform_info spt_i2c_info = {
 	.pset = &spt_i2c_pset,
 };
 
+static struct property_entry uart_properties[] = {
+	PROPERTY_ENTRY_U32("reg-io-width", 4),
+	PROPERTY_ENTRY_U32("reg-shift", 2),
+	PROPERTY_ENTRY_BOOL("snps,uart-16550-compatible"),
+	{ },
+};
+
+static struct property_set uart_pset = {
+	.properties = uart_properties,
+};
+
 static const struct intel_lpss_platform_info spt_uart_info = {
 	.clk_rate = 120000000,
 	.clk_con_id = "baudclk",
+	.pset = &uart_pset,
 };
 
 static const struct intel_lpss_platform_info bxt_info = {
@@ -92,6 +104,7 @@ static const struct intel_lpss_platform_info bxt_info = {
 static const struct intel_lpss_platform_info bxt_uart_info = {
 	.clk_rate = 100000000,
 	.clk_con_id = "baudclk",
+	.pset = &uart_pset,
 };
 
 static const struct intel_lpss_platform_info bxt_i2c_info = {
-- 
2.6.2

[PATCH v2 16/16] i2c: designware: Convert to use unified device property API

[Andy Shevchenko]

From: Mika Westerberg <mika.westerberg@linux.intel.com>

With ACPI _DSD (introduced in ACPI v5.1) it is now possible to pass device
configuration information from ACPI in addition to DT. In order to support
this, convert the driver to use the unified device property accessors
instead of DT specific.

Change to ordering a bit so that we first try platform data and if that's
not available look from device properties. ACPI *CNT methods are then used
as last resort to override everything else.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 50 +++++++++++++----------------
 1 file changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 809579e..06061b5 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -36,6 +36,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
@@ -129,10 +130,10 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
 
 static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
+	struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
 	struct resource *mem;
-	struct dw_i2c_platform_data *pdata;
 	int irq, r;
 	u32 clk_freq, ht = 0;
 
@@ -156,33 +157,28 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	/* fast mode by default because of legacy reasons */
 	clk_freq = 400000;
 
-	if (has_acpi_companion(&pdev->dev)) {
-		dw_i2c_acpi_configure(pdev);
-	} else if (pdev->dev.of_node) {
-		of_property_read_u32(pdev->dev.of_node,
-					"i2c-sda-hold-time-ns", &ht);
-
-		of_property_read_u32(pdev->dev.of_node,
-				     "i2c-sda-falling-time-ns",
-				     &dev->sda_falling_time);
-		of_property_read_u32(pdev->dev.of_node,
-				     "i2c-scl-falling-time-ns",
-				     &dev->scl_falling_time);
-
-		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
-				     &clk_freq);
-
-		/* Only standard mode at 100kHz and fast mode at 400kHz
-		 * are supported.
-		 */
-		if (clk_freq != 100000 && clk_freq != 400000) {
-			dev_err(&pdev->dev, "Only 100kHz and 400kHz supported");
-			return -EINVAL;
-		}
+	if (pdata) {
+		clk_freq = pdata->i2c_scl_freq;
 	} else {
-		pdata = dev_get_platdata(&pdev->dev);
-		if (pdata)
-			clk_freq = pdata->i2c_scl_freq;
+		device_property_read_u32(&pdev->dev, "i2c-sda-hold-time-ns",
+					 &ht);
+		device_property_read_u32(&pdev->dev, "i2c-sda-falling-time-ns",
+					 &dev->sda_falling_time);
+		device_property_read_u32(&pdev->dev, "i2c-scl-falling-time-ns",
+					 &dev->scl_falling_time);
+		device_property_read_u32(&pdev->dev, "clock-frequency",
+					 &clk_freq);
+	}
+
+	if (has_acpi_companion(&pdev->dev))
+		dw_i2c_acpi_configure(pdev);
+
+	/*
+	 * Only standard mode at 100kHz and fast mode at 400kHz are supported.
+	 */
+	if (clk_freq != 100000 && clk_freq != 400000) {
+		dev_err(&pdev->dev, "Only 100kHz and 400kHz supported");
+		return -EINVAL;
 	}
 
 	r = i2c_dw_eval_lock_support(dev);
-- 
2.6.2

Re: [PATCH v2 16/16] i2c: designware: Convert to use unified device property API

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 3870 bytes --]

On Mon, Nov 30, 2015 at 05:11:44PM +0200, Andy Shevchenko wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> With ACPI _DSD (introduced in ACPI v5.1) it is now possible to pass device
> configuration information from ACPI in addition to DT. In order to support
> this, convert the driver to use the unified device property accessors
> instead of DT specific.
> 
> Change to ordering a bit so that we first try platform data and if that's
> not available look from device properties. ACPI *CNT methods are then used
> as last resort to override everything else.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

What is the bug fix here described in the cover letter?

And shall this go via I2C or via the rest of the series?

> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c | 50 +++++++++++++----------------
>  1 file changed, 23 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 809579e..06061b5 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -36,6 +36,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/property.h>
>  #include <linux/io.h>
>  #include <linux/slab.h>
>  #include <linux/acpi.h>
> @@ -129,10 +130,10 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
>  
>  static int dw_i2c_plat_probe(struct platform_device *pdev)
>  {
> +	struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
>  	struct dw_i2c_dev *dev;
>  	struct i2c_adapter *adap;
>  	struct resource *mem;
> -	struct dw_i2c_platform_data *pdata;
>  	int irq, r;
>  	u32 clk_freq, ht = 0;
>  
> @@ -156,33 +157,28 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
>  	/* fast mode by default because of legacy reasons */
>  	clk_freq = 400000;
>  
> -	if (has_acpi_companion(&pdev->dev)) {
> -		dw_i2c_acpi_configure(pdev);
> -	} else if (pdev->dev.of_node) {
> -		of_property_read_u32(pdev->dev.of_node,
> -					"i2c-sda-hold-time-ns", &ht);
> -
> -		of_property_read_u32(pdev->dev.of_node,
> -				     "i2c-sda-falling-time-ns",
> -				     &dev->sda_falling_time);
> -		of_property_read_u32(pdev->dev.of_node,
> -				     "i2c-scl-falling-time-ns",
> -				     &dev->scl_falling_time);
> -
> -		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> -				     &clk_freq);
> -
> -		/* Only standard mode at 100kHz and fast mode at 400kHz
> -		 * are supported.
> -		 */
> -		if (clk_freq != 100000 && clk_freq != 400000) {
> -			dev_err(&pdev->dev, "Only 100kHz and 400kHz supported");
> -			return -EINVAL;
> -		}
> +	if (pdata) {
> +		clk_freq = pdata->i2c_scl_freq;
>  	} else {
> -		pdata = dev_get_platdata(&pdev->dev);
> -		if (pdata)
> -			clk_freq = pdata->i2c_scl_freq;
> +		device_property_read_u32(&pdev->dev, "i2c-sda-hold-time-ns",
> +					 &ht);
> +		device_property_read_u32(&pdev->dev, "i2c-sda-falling-time-ns",
> +					 &dev->sda_falling_time);
> +		device_property_read_u32(&pdev->dev, "i2c-scl-falling-time-ns",
> +					 &dev->scl_falling_time);
> +		device_property_read_u32(&pdev->dev, "clock-frequency",
> +					 &clk_freq);
> +	}
> +
> +	if (has_acpi_companion(&pdev->dev))
> +		dw_i2c_acpi_configure(pdev);
> +
> +	/*
> +	 * Only standard mode at 100kHz and fast mode at 400kHz are supported.
> +	 */
> +	if (clk_freq != 100000 && clk_freq != 400000) {
> +		dev_err(&pdev->dev, "Only 100kHz and 400kHz supported");
> +		return -EINVAL;
>  	}
>  
>  	r = i2c_dw_eval_lock_support(dev);
> -- 
> 2.6.2
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 16/16] i2c: designware: Convert to use unified device property API

[Mika Westerberg]

On Mon, Nov 30, 2015 at 08:58:58PM +0100, Wolfram Sang wrote:
> On Mon, Nov 30, 2015 at 05:11:44PM +0200, Andy Shevchenko wrote:
> > From: Mika Westerberg <mika.westerberg@linux.intel.com>
> > 
> > With ACPI _DSD (introduced in ACPI v5.1) it is now possible to pass device
> > configuration information from ACPI in addition to DT. In order to support
> > this, convert the driver to use the unified device property accessors
> > instead of DT specific.
> > 
> > Change to ordering a bit so that we first try platform data and if that's
> > not available look from device properties. ACPI *CNT methods are then used
> > as last resort to override everything else.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> 
> What is the bug fix here described in the cover letter?

The bug fix is actually in patch [14/16] "mfd: intel-lpss: Pass SDA hold
time to I2C host controller driver".

> And shall this go via I2C or via the rest of the series?

Either way works. This should compile and work fine without the rest but
of course fix for the issue with Lenovo Yoga 900 toucpad still requires
all the patches.

Re: [PATCH v2 16/16] i2c: designware: Convert to use unified device property API

[Andy Shevchenko]

On Mon, 2015-11-30 at 20:58 +0100, Wolfram Sang wrote:
> On Mon, Nov 30, 2015 at 05:11:44PM +0200, Andy Shevchenko wrote:
> > From: Mika Westerberg <mika.westerberg@linux.intel.com>
> > 
> > With ACPI _DSD (introduced in ACPI v5.1) it is now possible to pass
> > device
> > configuration information from ACPI in addition to DT. In order to
> > support
> > this, convert the driver to use the unified device property
> > accessors
> > instead of DT specific.
> > 
> > Change to ordering a bit so that we first try platform data and if
> > that's
> > not available look from device properties. ACPI *CNT methods are
> > then used
> > as last resort to override everything else.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> 
> What is the bug fix here described in the cover letter?

The cover letter mentioned 'last part' which I refer to as patches 14,
15 (though this is for UART), and 16.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 16/16] i2c: designware: Convert to use unified device property API

[Rafael J. Wysocki]

On Tuesday, December 01, 2015 12:33:51 PM Andy Shevchenko wrote:
> On Mon, 2015-11-30 at 20:58 +0100, Wolfram Sang wrote:
> > On Mon, Nov 30, 2015 at 05:11:44PM +0200, Andy Shevchenko wrote:
> > > From: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > 
> > > With ACPI _DSD (introduced in ACPI v5.1) it is now possible to pass
> > > device
> > > configuration information from ACPI in addition to DT. In order to
> > > support
> > > this, convert the driver to use the unified device property
> > > accessors
> > > instead of DT specific.
> > > 
> > > Change to ordering a bit so that we first try platform data and if
> > > that's
> > > not available look from device properties. ACPI *CNT methods are
> > > then used
> > > as last resort to override everything else.
> > > 
> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> > 
> > What is the bug fix here described in the cover letter?
> 
> The cover letter mentioned 'last part' which I refer to as patches 14,
> 15 (though this is for UART), and 16.

Hmm.

So may I assume that patches [1-13/16] are for me and the rest is to be applied
by the other respective maintainers?

That should be easiest logistically IMHO.

Thanks,
Rafael

Re: [PATCH v2 16/16] i2c: designware: Convert to use unified device property API

[Andy Shevchenko]

On Wed, 2015-12-02 at 02:28 +0100, Rafael J. Wysocki wrote:
> On Tuesday, December 01, 2015 12:33:51 PM Andy Shevchenko wrote:
> > On Mon, 2015-11-30 at 20:58 +0100, Wolfram Sang wrote:
> > > On Mon, Nov 30, 2015 at 05:11:44PM +0200, Andy Shevchenko wrote:



> > > What is the bug fix here described in the cover letter?
> > 
> > The cover letter mentioned 'last part' which I refer to as patches
> > 14,
> > 15 (though this is for UART), and 16.
> 
> Hmm.
> 
> So may I assume that patches [1-13/16] are for me and the rest is to
> be applied
> by the other respective maintainers?
> 
> That should be easiest logistically IMHO.

Have no objections.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 16/16] i2c: designware: Convert to use unified device property API

[Mika Westerberg]

On Wed, Dec 02, 2015 at 11:23:40AM +0200, Andy Shevchenko wrote:
> On Wed, 2015-12-02 at 02:28 +0100, Rafael J. Wysocki wrote:
> > On Tuesday, December 01, 2015 12:33:51 PM Andy Shevchenko wrote:
> > > On Mon, 2015-11-30 at 20:58 +0100, Wolfram Sang wrote:
> > > > On Mon, Nov 30, 2015 at 05:11:44PM +0200, Andy Shevchenko wrote:
> 
> 
> 
> > > > What is the bug fix here described in the cover letter?
> > > 
> > > The cover letter mentioned 'last part' which I refer to as patches
> > > 14,
> > > 15 (though this is for UART), and 16.
> > 
> > Hmm.
> > 
> > So may I assume that patches [1-13/16] are for me and the rest is to
> > be applied
> > by the other respective maintainers?
> > 
> > That should be easiest logistically IMHO.
> 
> Have no objections.

Unfortunately the patches (except this one) depend on each other so they
cannot be applied separately.

Re: [PATCH v2 16/16] i2c: designware: Convert to use unified device property API

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]

On Wed, Dec 02, 2015 at 11:33:41AM +0200, Mika Westerberg wrote:
> On Wed, Dec 02, 2015 at 11:23:40AM +0200, Andy Shevchenko wrote:
> > On Wed, 2015-12-02 at 02:28 +0100, Rafael J. Wysocki wrote:
> > > On Tuesday, December 01, 2015 12:33:51 PM Andy Shevchenko wrote:
> > > > On Mon, 2015-11-30 at 20:58 +0100, Wolfram Sang wrote:
> > > > > On Mon, Nov 30, 2015 at 05:11:44PM +0200, Andy Shevchenko wrote:
> > 
> > 
> > 
> > > > > What is the bug fix here described in the cover letter?
> > > > 
> > > > The cover letter mentioned 'last part' which I refer to as patches
> > > > 14,
> > > > 15 (though this is for UART), and 16.
> > > 
> > > Hmm.
> > > 
> > > So may I assume that patches [1-13/16] are for me and the rest is to
> > > be applied
> > > by the other respective maintainers?
> > > 
> > > That should be easiest logistically IMHO.
> > 
> > Have no objections.
> 
> Unfortunately the patches (except this one) depend on each other so they
> cannot be applied separately.

So, why not let all of them go in in one go?

For this patch:

Acked-by: Wolfram Sang <wsa@the-dreams.de>


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Rafael J. Wysocki]

On Monday, November 30, 2015 05:11:28 PM Andy Shevchenko wrote:
> This series includes few logical sets that bring a support of non-ACPI
> platforms for Intel Skylake.
> 
> First part is a refactoring of built-in device properties support:
> - keep single value inside the structure
> - provide helper macros to define built-in properties
> - fall back to secondary fwnode if primary has no asked property
> 
> Second is a propagating built-in device properties in platform core.
> 
> Third one is modifications to MFD code and intel-lpss.c driver in particular
> to define and pass built-in properties to the individual drivers.
> 
> And last part is a fix for I2C bug found on Lenovo Yoga hardware and a first
> converted user.
> 
> Built-in device properties is an alternative to platform data. It provides a
> unified API that drivers can use to cover all cases at once: DT, ACPI, and
> built-in properties.
> 
> With this series applied a platform data can be considered obsolete. Moreover,
> built-in device properties allow to adjust the existing configuration, for
> example, in cases when ACPI values are wrong on some platforms.
> 
> The series has been tested on available hardware and doesn't break current
> behaviour. But we ask people who have the affected hardware to apply the series
> on your side and check with Lenovo hardware.
> 
> Changelog v2:
> - fix isuues found by kbuild bot (kbuild)
> - append a patch to propagate device properties in polatform code (Arnd)
> - update few existing and add couple of new patches due to above
> - check with kmemleak
> 
> Andy Shevchenko (9):
>   device property: always check for fwnode type
>   device property: rename helper functions
>   device property: refactor built-in properties support
>   device property: keep single value inplace
>   device property: improve readability of macros
>   device property: return -EINVAL when property isn't found in ACPI
>   device property: Fallback to secondary fwnode if primary misses the
>     property
>   mfd: core: propagate device properties to sub devices drivers
>   mfd: intel-lpss: Pass HSUART configuration via properties
> 
> Heikki Krogerus (1):
>   device property: helper macros for property entry creation
> 
> Mika Westerberg (6):
>   device property: Take a copy of the property set
>   driver core: platform: Add support for built-in device properties
>   driver core: Do not overwrite secondary fwnode with NULL if it is set
>   mfd: intel-lpss: Add support for passing device properties
>   mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
>   i2c: designware: Convert to use unified device property API

I'm going to queue up this series for v4.5.

If there are any problems with it or objections from anyone, please let me know.

Thanks,
Rafael

Re: [PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Laura Abbott]

On 12/06/2015 05:44 PM, Rafael J. Wysocki wrote:
> On Monday, November 30, 2015 05:11:28 PM Andy Shevchenko wrote:
>> This series includes few logical sets that bring a support of non-ACPI
>> platforms for Intel Skylake.
>>
>> First part is a refactoring of built-in device properties support:
>> - keep single value inside the structure
>> - provide helper macros to define built-in properties
>> - fall back to secondary fwnode if primary has no asked property
>>
>> Second is a propagating built-in device properties in platform core.
>>
>> Third one is modifications to MFD code and intel-lpss.c driver in particular
>> to define and pass built-in properties to the individual drivers.
>>
>> And last part is a fix for I2C bug found on Lenovo Yoga hardware and a first
>> converted user.
>>
>> Built-in device properties is an alternative to platform data. It provides a
>> unified API that drivers can use to cover all cases at once: DT, ACPI, and
>> built-in properties.
>>
>> With this series applied a platform data can be considered obsolete. Moreover,
>> built-in device properties allow to adjust the existing configuration, for
>> example, in cases when ACPI values are wrong on some platforms.
>>
>> The series has been tested on available hardware and doesn't break current
>> behaviour. But we ask people who have the affected hardware to apply the series
>> on your side and check with Lenovo hardware.
>>
>> Changelog v2:
>> - fix isuues found by kbuild bot (kbuild)
>> - append a patch to propagate device properties in polatform code (Arnd)
>> - update few existing and add couple of new patches due to above
>> - check with kmemleak
>>
>> Andy Shevchenko (9):
>>    device property: always check for fwnode type
>>    device property: rename helper functions
>>    device property: refactor built-in properties support
>>    device property: keep single value inplace
>>    device property: improve readability of macros
>>    device property: return -EINVAL when property isn't found in ACPI
>>    device property: Fallback to secondary fwnode if primary misses the
>>      property
>>    mfd: core: propagate device properties to sub devices drivers
>>    mfd: intel-lpss: Pass HSUART configuration via properties
>>
>> Heikki Krogerus (1):
>>    device property: helper macros for property entry creation
>>
>> Mika Westerberg (6):
>>    device property: Take a copy of the property set
>>    driver core: platform: Add support for built-in device properties
>>    driver core: Do not overwrite secondary fwnode with NULL if it is set
>>    mfd: intel-lpss: Add support for passing device properties
>>    mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
>>    i2c: designware: Convert to use unified device property API
>
> I'm going to queue up this series for v4.5.
>
> If there are any problems with it or objections from anyone, please let me know.
>


Raising an old thread, I pulled this series into Fedora rawhide and
while it worked for Lenovo Yoga we received a report that it caused
a regression on the Dell Inspiron 7559 (see
https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c27) . I haven't
asked the reporter about bisecting to see which patch broke it.
Were there any known follow up patches?

Thanks,
Laura

Re: [PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Rafael J. Wysocki]

On Tuesday, January 05, 2016 09:57:35 AM Laura Abbott wrote:
> On 12/06/2015 05:44 PM, Rafael J. Wysocki wrote:
> > On Monday, November 30, 2015 05:11:28 PM Andy Shevchenko wrote:
> >> This series includes few logical sets that bring a support of non-ACPI
> >> platforms for Intel Skylake.
> >>
> >> First part is a refactoring of built-in device properties support:
> >> - keep single value inside the structure
> >> - provide helper macros to define built-in properties
> >> - fall back to secondary fwnode if primary has no asked property
> >>
> >> Second is a propagating built-in device properties in platform core.
> >>
> >> Third one is modifications to MFD code and intel-lpss.c driver in particular
> >> to define and pass built-in properties to the individual drivers.
> >>
> >> And last part is a fix for I2C bug found on Lenovo Yoga hardware and a first
> >> converted user.
> >>
> >> Built-in device properties is an alternative to platform data. It provides a
> >> unified API that drivers can use to cover all cases at once: DT, ACPI, and
> >> built-in properties.
> >>
> >> With this series applied a platform data can be considered obsolete. Moreover,
> >> built-in device properties allow to adjust the existing configuration, for
> >> example, in cases when ACPI values are wrong on some platforms.
> >>
> >> The series has been tested on available hardware and doesn't break current
> >> behaviour. But we ask people who have the affected hardware to apply the series
> >> on your side and check with Lenovo hardware.
> >>
> >> Changelog v2:
> >> - fix isuues found by kbuild bot (kbuild)
> >> - append a patch to propagate device properties in polatform code (Arnd)
> >> - update few existing and add couple of new patches due to above
> >> - check with kmemleak
> >>
> >> Andy Shevchenko (9):
> >>    device property: always check for fwnode type
> >>    device property: rename helper functions
> >>    device property: refactor built-in properties support
> >>    device property: keep single value inplace
> >>    device property: improve readability of macros
> >>    device property: return -EINVAL when property isn't found in ACPI
> >>    device property: Fallback to secondary fwnode if primary misses the
> >>      property
> >>    mfd: core: propagate device properties to sub devices drivers
> >>    mfd: intel-lpss: Pass HSUART configuration via properties
> >>
> >> Heikki Krogerus (1):
> >>    device property: helper macros for property entry creation
> >>
> >> Mika Westerberg (6):
> >>    device property: Take a copy of the property set
> >>    driver core: platform: Add support for built-in device properties
> >>    driver core: Do not overwrite secondary fwnode with NULL if it is set
> >>    mfd: intel-lpss: Add support for passing device properties
> >>    mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
> >>    i2c: designware: Convert to use unified device property API
> >
> > I'm going to queue up this series for v4.5.
> >
> > If there are any problems with it or objections from anyone, please let me know.
> >
> 
> 
> Raising an old thread, I pulled this series into Fedora rawhide and
> while it worked for Lenovo Yoga we received a report that it caused
> a regression on the Dell Inspiron 7559 (see
> https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c27) . I haven't
> asked the reporter about bisecting to see which patch broke it.
> Were there any known follow up patches?

There were a few.

All of them are in my linux-next branch if you can try this one.

Alternatively, I can expose a branch with these to you to test.

Thanks,
Rafael

Re: [PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Laura Abbott]

On 01/05/2016 03:59 PM, Rafael J. Wysocki wrote:
> On Tuesday, January 05, 2016 09:57:35 AM Laura Abbott wrote:
>> On 12/06/2015 05:44 PM, Rafael J. Wysocki wrote:
>>> On Monday, November 30, 2015 05:11:28 PM Andy Shevchenko wrote:
>>>> This series includes few logical sets that bring a support of non-ACPI
>>>> platforms for Intel Skylake.
>>>>
>>>> First part is a refactoring of built-in device properties support:
>>>> - keep single value inside the structure
>>>> - provide helper macros to define built-in properties
>>>> - fall back to secondary fwnode if primary has no asked property
>>>>
>>>> Second is a propagating built-in device properties in platform core.
>>>>
>>>> Third one is modifications to MFD code and intel-lpss.c driver in particular
>>>> to define and pass built-in properties to the individual drivers.
>>>>
>>>> And last part is a fix for I2C bug found on Lenovo Yoga hardware and a first
>>>> converted user.
>>>>
>>>> Built-in device properties is an alternative to platform data. It provides a
>>>> unified API that drivers can use to cover all cases at once: DT, ACPI, and
>>>> built-in properties.
>>>>
>>>> With this series applied a platform data can be considered obsolete. Moreover,
>>>> built-in device properties allow to adjust the existing configuration, for
>>>> example, in cases when ACPI values are wrong on some platforms.
>>>>
>>>> The series has been tested on available hardware and doesn't break current
>>>> behaviour. But we ask people who have the affected hardware to apply the series
>>>> on your side and check with Lenovo hardware.
>>>>
>>>> Changelog v2:
>>>> - fix isuues found by kbuild bot (kbuild)
>>>> - append a patch to propagate device properties in polatform code (Arnd)
>>>> - update few existing and add couple of new patches due to above
>>>> - check with kmemleak
>>>>
>>>> Andy Shevchenko (9):
>>>>     device property: always check for fwnode type
>>>>     device property: rename helper functions
>>>>     device property: refactor built-in properties support
>>>>     device property: keep single value inplace
>>>>     device property: improve readability of macros
>>>>     device property: return -EINVAL when property isn't found in ACPI
>>>>     device property: Fallback to secondary fwnode if primary misses the
>>>>       property
>>>>     mfd: core: propagate device properties to sub devices drivers
>>>>     mfd: intel-lpss: Pass HSUART configuration via properties
>>>>
>>>> Heikki Krogerus (1):
>>>>     device property: helper macros for property entry creation
>>>>
>>>> Mika Westerberg (6):
>>>>     device property: Take a copy of the property set
>>>>     driver core: platform: Add support for built-in device properties
>>>>     driver core: Do not overwrite secondary fwnode with NULL if it is set
>>>>     mfd: intel-lpss: Add support for passing device properties
>>>>     mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
>>>>     i2c: designware: Convert to use unified device property API
>>>
>>> I'm going to queue up this series for v4.5.
>>>
>>> If there are any problems with it or objections from anyone, please let me know.
>>>
>>
>>
>> Raising an old thread, I pulled this series into Fedora rawhide and
>> while it worked for Lenovo Yoga we received a report that it caused
>> a regression on the Dell Inspiron 7559 (see
>> https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c27) . I haven't
>> asked the reporter about bisecting to see which patch broke it.
>> Were there any known follow up patches?
>
> There were a few.
>
> All of them are in my linux-next branch if you can try this one.
>
> Alternatively, I can expose a branch with these to you to test.
>
> Thanks,
> Rafael
>

I picked up all the patches from the device-properties merge but the
problem still shows up. Are there others I should pick up? Hardware
details about the touchpad are at https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c34

Thanks,
Laura

Re: [PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Mika Westerberg]

On Wed, Jan 06, 2016 at 08:19:49AM -0800, Laura Abbott wrote:
> I picked up all the patches from the device-properties merge but the
> problem still shows up. Are there others I should pick up? Hardware
> details about the touchpad are at https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c34

Can you get full dmesg of the failure and acpidump as well? I did not
find those from the bug report.

Also dmesg of non-failure case would be nice.

Re: [PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Rafael J. Wysocki]

On Wednesday, January 06, 2016 08:19:49 AM Laura Abbott wrote:
> On 01/05/2016 03:59 PM, Rafael J. Wysocki wrote:
> > On Tuesday, January 05, 2016 09:57:35 AM Laura Abbott wrote:
> >> On 12/06/2015 05:44 PM, Rafael J. Wysocki wrote:
> >>> On Monday, November 30, 2015 05:11:28 PM Andy Shevchenko wrote:
> >>>> This series includes few logical sets that bring a support of non-ACPI
> >>>> platforms for Intel Skylake.
> >>>>
> >>>> First part is a refactoring of built-in device properties support:
> >>>> - keep single value inside the structure
> >>>> - provide helper macros to define built-in properties
> >>>> - fall back to secondary fwnode if primary has no asked property
> >>>>
> >>>> Second is a propagating built-in device properties in platform core.
> >>>>
> >>>> Third one is modifications to MFD code and intel-lpss.c driver in particular
> >>>> to define and pass built-in properties to the individual drivers.
> >>>>
> >>>> And last part is a fix for I2C bug found on Lenovo Yoga hardware and a first
> >>>> converted user.
> >>>>
> >>>> Built-in device properties is an alternative to platform data. It provides a
> >>>> unified API that drivers can use to cover all cases at once: DT, ACPI, and
> >>>> built-in properties.
> >>>>
> >>>> With this series applied a platform data can be considered obsolete. Moreover,
> >>>> built-in device properties allow to adjust the existing configuration, for
> >>>> example, in cases when ACPI values are wrong on some platforms.
> >>>>
> >>>> The series has been tested on available hardware and doesn't break current
> >>>> behaviour. But we ask people who have the affected hardware to apply the series
> >>>> on your side and check with Lenovo hardware.
> >>>>
> >>>> Changelog v2:
> >>>> - fix isuues found by kbuild bot (kbuild)
> >>>> - append a patch to propagate device properties in polatform code (Arnd)
> >>>> - update few existing and add couple of new patches due to above
> >>>> - check with kmemleak
> >>>>
> >>>> Andy Shevchenko (9):
> >>>>     device property: always check for fwnode type
> >>>>     device property: rename helper functions
> >>>>     device property: refactor built-in properties support
> >>>>     device property: keep single value inplace
> >>>>     device property: improve readability of macros
> >>>>     device property: return -EINVAL when property isn't found in ACPI
> >>>>     device property: Fallback to secondary fwnode if primary misses the
> >>>>       property
> >>>>     mfd: core: propagate device properties to sub devices drivers
> >>>>     mfd: intel-lpss: Pass HSUART configuration via properties
> >>>>
> >>>> Heikki Krogerus (1):
> >>>>     device property: helper macros for property entry creation
> >>>>
> >>>> Mika Westerberg (6):
> >>>>     device property: Take a copy of the property set
> >>>>     driver core: platform: Add support for built-in device properties
> >>>>     driver core: Do not overwrite secondary fwnode with NULL if it is set
> >>>>     mfd: intel-lpss: Add support for passing device properties
> >>>>     mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
> >>>>     i2c: designware: Convert to use unified device property API
> >>>
> >>> I'm going to queue up this series for v4.5.
> >>>
> >>> If there are any problems with it or objections from anyone, please let me know.
> >>>
> >>
> >>
> >> Raising an old thread, I pulled this series into Fedora rawhide and
> >> while it worked for Lenovo Yoga we received a report that it caused
> >> a regression on the Dell Inspiron 7559 (see
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c27) . I haven't
> >> asked the reporter about bisecting to see which patch broke it.
> >> Were there any known follow up patches?
> >
> > There were a few.
> >
> > All of them are in my linux-next branch if you can try this one.
> >
> > Alternatively, I can expose a branch with these to you to test.
> >
> 
> I picked up all the patches from the device-properties merge but the
> problem still shows up. Are there others I should pick up? Hardware
> details about the touchpad are at https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c34

Well, in that case can you please ask the reporter to bisect?

Rafael

Re: [PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Laura Abbott]

On 01/07/2016 04:03 PM, Rafael J. Wysocki wrote:
> On Wednesday, January 06, 2016 08:19:49 AM Laura Abbott wrote:
>> On 01/05/2016 03:59 PM, Rafael J. Wysocki wrote:
>>> On Tuesday, January 05, 2016 09:57:35 AM Laura Abbott wrote:
>>>> On 12/06/2015 05:44 PM, Rafael J. Wysocki wrote:
>>>>> On Monday, November 30, 2015 05:11:28 PM Andy Shevchenko wrote:
>>>>>> This series includes few logical sets that bring a support of non-ACPI
>>>>>> platforms for Intel Skylake.
>>>>>>
>>>>>> First part is a refactoring of built-in device properties support:
>>>>>> - keep single value inside the structure
>>>>>> - provide helper macros to define built-in properties
>>>>>> - fall back to secondary fwnode if primary has no asked property
>>>>>>
>>>>>> Second is a propagating built-in device properties in platform core.
>>>>>>
>>>>>> Third one is modifications to MFD code and intel-lpss.c driver in particular
>>>>>> to define and pass built-in properties to the individual drivers.
>>>>>>
>>>>>> And last part is a fix for I2C bug found on Lenovo Yoga hardware and a first
>>>>>> converted user.
>>>>>>
>>>>>> Built-in device properties is an alternative to platform data. It provides a
>>>>>> unified API that drivers can use to cover all cases at once: DT, ACPI, and
>>>>>> built-in properties.
>>>>>>
>>>>>> With this series applied a platform data can be considered obsolete. Moreover,
>>>>>> built-in device properties allow to adjust the existing configuration, for
>>>>>> example, in cases when ACPI values are wrong on some platforms.
>>>>>>
>>>>>> The series has been tested on available hardware and doesn't break current
>>>>>> behaviour. But we ask people who have the affected hardware to apply the series
>>>>>> on your side and check with Lenovo hardware.
>>>>>>
>>>>>> Changelog v2:
>>>>>> - fix isuues found by kbuild bot (kbuild)
>>>>>> - append a patch to propagate device properties in polatform code (Arnd)
>>>>>> - update few existing and add couple of new patches due to above
>>>>>> - check with kmemleak
>>>>>>
>>>>>> Andy Shevchenko (9):
>>>>>>      device property: always check for fwnode type
>>>>>>      device property: rename helper functions
>>>>>>      device property: refactor built-in properties support
>>>>>>      device property: keep single value inplace
>>>>>>      device property: improve readability of macros
>>>>>>      device property: return -EINVAL when property isn't found in ACPI
>>>>>>      device property: Fallback to secondary fwnode if primary misses the
>>>>>>        property
>>>>>>      mfd: core: propagate device properties to sub devices drivers
>>>>>>      mfd: intel-lpss: Pass HSUART configuration via properties
>>>>>>
>>>>>> Heikki Krogerus (1):
>>>>>>      device property: helper macros for property entry creation
>>>>>>
>>>>>> Mika Westerberg (6):
>>>>>>      device property: Take a copy of the property set
>>>>>>      driver core: platform: Add support for built-in device properties
>>>>>>      driver core: Do not overwrite secondary fwnode with NULL if it is set
>>>>>>      mfd: intel-lpss: Add support for passing device properties
>>>>>>      mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
>>>>>>      i2c: designware: Convert to use unified device property API
>>>>>
>>>>> I'm going to queue up this series for v4.5.
>>>>>
>>>>> If there are any problems with it or objections from anyone, please let me know.
>>>>>
>>>>
>>>>
>>>> Raising an old thread, I pulled this series into Fedora rawhide and
>>>> while it worked for Lenovo Yoga we received a report that it caused
>>>> a regression on the Dell Inspiron 7559 (see
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c27) . I haven't
>>>> asked the reporter about bisecting to see which patch broke it.
>>>> Were there any known follow up patches?
>>>
>>> There were a few.
>>>
>>> All of them are in my linux-next branch if you can try this one.
>>>
>>> Alternatively, I can expose a branch with these to you to test.
>>>
>>
>> I picked up all the patches from the device-properties merge but the
>> problem still shows up. Are there others I should pick up? Hardware
>> details about the touchpad are at https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c34
>
> Well, in that case can you please ask the reporter to bisect?
>
> Rafael
>

I'll see if I can help the reporter bisect although it looks like Hans
has an idea about what might be failing.

Thanks,
Laura

Re: [PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Hans de Goede]

Hi,

On 08-01-16 18:08, Laura Abbott wrote:
> On 01/07/2016 04:03 PM, Rafael J. Wysocki wrote:
>> On Wednesday, January 06, 2016 08:19:49 AM Laura Abbott wrote:
>>> On 01/05/2016 03:59 PM, Rafael J. Wysocki wrote:
>>>> On Tuesday, January 05, 2016 09:57:35 AM Laura Abbott wrote:
>>>>> On 12/06/2015 05:44 PM, Rafael J. Wysocki wrote:
>>>>>> On Monday, November 30, 2015 05:11:28 PM Andy Shevchenko wrote:
>>>>>>> This series includes few logical sets that bring a support of non-ACPI
>>>>>>> platforms for Intel Skylake.
>>>>>>>
>>>>>>> First part is a refactoring of built-in device properties support:
>>>>>>> - keep single value inside the structure
>>>>>>> - provide helper macros to define built-in properties
>>>>>>> - fall back to secondary fwnode if primary has no asked property
>>>>>>>
>>>>>>> Second is a propagating built-in device properties in platform core.
>>>>>>>
>>>>>>> Third one is modifications to MFD code and intel-lpss.c driver in particular
>>>>>>> to define and pass built-in properties to the individual drivers.
>>>>>>>
>>>>>>> And last part is a fix for I2C bug found on Lenovo Yoga hardware and a first
>>>>>>> converted user.
>>>>>>>
>>>>>>> Built-in device properties is an alternative to platform data. It provides a
>>>>>>> unified API that drivers can use to cover all cases at once: DT, ACPI, and
>>>>>>> built-in properties.
>>>>>>>
>>>>>>> With this series applied a platform data can be considered obsolete. Moreover,
>>>>>>> built-in device properties allow to adjust the existing configuration, for
>>>>>>> example, in cases when ACPI values are wrong on some platforms.
>>>>>>>
>>>>>>> The series has been tested on available hardware and doesn't break current
>>>>>>> behaviour. But we ask people who have the affected hardware to apply the series
>>>>>>> on your side and check with Lenovo hardware.
>>>>>>>
>>>>>>> Changelog v2:
>>>>>>> - fix isuues found by kbuild bot (kbuild)
>>>>>>> - append a patch to propagate device properties in polatform code (Arnd)
>>>>>>> - update few existing and add couple of new patches due to above
>>>>>>> - check with kmemleak
>>>>>>>
>>>>>>> Andy Shevchenko (9):
>>>>>>>      device property: always check for fwnode type
>>>>>>>      device property: rename helper functions
>>>>>>>      device property: refactor built-in properties support
>>>>>>>      device property: keep single value inplace
>>>>>>>      device property: improve readability of macros
>>>>>>>      device property: return -EINVAL when property isn't found in ACPI
>>>>>>>      device property: Fallback to secondary fwnode if primary misses the
>>>>>>>        property
>>>>>>>      mfd: core: propagate device properties to sub devices drivers
>>>>>>>      mfd: intel-lpss: Pass HSUART configuration via properties
>>>>>>>
>>>>>>> Heikki Krogerus (1):
>>>>>>>      device property: helper macros for property entry creation
>>>>>>>
>>>>>>> Mika Westerberg (6):
>>>>>>>      device property: Take a copy of the property set
>>>>>>>      driver core: platform: Add support for built-in device properties
>>>>>>>      driver core: Do not overwrite secondary fwnode with NULL if it is set
>>>>>>>      mfd: intel-lpss: Add support for passing device properties
>>>>>>>      mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
>>>>>>>      i2c: designware: Convert to use unified device property API
>>>>>>
>>>>>> I'm going to queue up this series for v4.5.
>>>>>>
>>>>>> If there are any problems with it or objections from anyone, please let me know.
>>>>>>
>>>>>
>>>>>
>>>>> Raising an old thread, I pulled this series into Fedora rawhide and
>>>>> while it worked for Lenovo Yoga we received a report that it caused
>>>>> a regression on the Dell Inspiron 7559 (see
>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c27) . I haven't
>>>>> asked the reporter about bisecting to see which patch broke it.
>>>>> Were there any known follow up patches?
>>>>
>>>> There were a few.
>>>>
>>>> All of them are in my linux-next branch if you can try this one.
>>>>
>>>> Alternatively, I can expose a branch with these to you to test.
>>>>
>>>
>>> I picked up all the patches from the device-properties merge but the
>>> problem still shows up. Are there others I should pick up? Hardware
>>> details about the touchpad are at https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c34
>>
>> Well, in that case can you please ask the reporter to bisect?
>>
>> Rafael
>>
>
> I'll see if I can help the reporter bisect although it looks like Hans
> has an idea about what might be failing.

Nope, sorry if I left the impression that I have an idea where things are failing.

Benjamin Tissoires latest reply in this thread (or was it in bugzilla ?) however
seems to point in the right direction, as with this patch-set the i2c controller
on the laptop in question starts working (as intended by this patch-set) and then
i2c-hid tries to talk to the touchpad, but fails too. However it gets far enough
for the touchpad to drop of the ps/2 bus.

So what seems to be happening is that this is a dual protocol / bus (ps2 / i2c)
touchpad (not that uncommon actually) and now that we've the i2c bus working due
to this patchset, we start talking i2c to the touchpad, kicking it from ps2 mode
to i2c mode, but our i2c driver then fails to work properly with the touchpad
leaving it non functional.

Regards,

Hans

Re: [PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Laura Abbott]

On 01/08/2016 11:22 AM, Hans de Goede wrote:
> Hi,
>
> On 08-01-16 18:08, Laura Abbott wrote:
>> On 01/07/2016 04:03 PM, Rafael J. Wysocki wrote:
>>> On Wednesday, January 06, 2016 08:19:49 AM Laura Abbott wrote:
>>>> On 01/05/2016 03:59 PM, Rafael J. Wysocki wrote:
>>>>> On Tuesday, January 05, 2016 09:57:35 AM Laura Abbott wrote:
>>>>>> On 12/06/2015 05:44 PM, Rafael J. Wysocki wrote:
>>>>>>> On Monday, November 30, 2015 05:11:28 PM Andy Shevchenko wrote:
>>>>>>>> This series includes few logical sets that bring a support of non-ACPI
>>>>>>>> platforms for Intel Skylake.
>>>>>>>>
>>>>>>>> First part is a refactoring of built-in device properties support:
>>>>>>>> - keep single value inside the structure
>>>>>>>> - provide helper macros to define built-in properties
>>>>>>>> - fall back to secondary fwnode if primary has no asked property
>>>>>>>>
>>>>>>>> Second is a propagating built-in device properties in platform core.
>>>>>>>>
>>>>>>>> Third one is modifications to MFD code and intel-lpss.c driver in particular
>>>>>>>> to define and pass built-in properties to the individual drivers.
>>>>>>>>
>>>>>>>> And last part is a fix for I2C bug found on Lenovo Yoga hardware and a first
>>>>>>>> converted user.
>>>>>>>>
>>>>>>>> Built-in device properties is an alternative to platform data. It provides a
>>>>>>>> unified API that drivers can use to cover all cases at once: DT, ACPI, and
>>>>>>>> built-in properties.
>>>>>>>>
>>>>>>>> With this series applied a platform data can be considered obsolete. Moreover,
>>>>>>>> built-in device properties allow to adjust the existing configuration, for
>>>>>>>> example, in cases when ACPI values are wrong on some platforms.
>>>>>>>>
>>>>>>>> The series has been tested on available hardware and doesn't break current
>>>>>>>> behaviour. But we ask people who have the affected hardware to apply the series
>>>>>>>> on your side and check with Lenovo hardware.
>>>>>>>>
>>>>>>>> Changelog v2:
>>>>>>>> - fix isuues found by kbuild bot (kbuild)
>>>>>>>> - append a patch to propagate device properties in polatform code (Arnd)
>>>>>>>> - update few existing and add couple of new patches due to above
>>>>>>>> - check with kmemleak
>>>>>>>>
>>>>>>>> Andy Shevchenko (9):
>>>>>>>>      device property: always check for fwnode type
>>>>>>>>      device property: rename helper functions
>>>>>>>>      device property: refactor built-in properties support
>>>>>>>>      device property: keep single value inplace
>>>>>>>>      device property: improve readability of macros
>>>>>>>>      device property: return -EINVAL when property isn't found in ACPI
>>>>>>>>      device property: Fallback to secondary fwnode if primary misses the
>>>>>>>>        property
>>>>>>>>      mfd: core: propagate device properties to sub devices drivers
>>>>>>>>      mfd: intel-lpss: Pass HSUART configuration via properties
>>>>>>>>
>>>>>>>> Heikki Krogerus (1):
>>>>>>>>      device property: helper macros for property entry creation
>>>>>>>>
>>>>>>>> Mika Westerberg (6):
>>>>>>>>      device property: Take a copy of the property set
>>>>>>>>      driver core: platform: Add support for built-in device properties
>>>>>>>>      driver core: Do not overwrite secondary fwnode with NULL if it is set
>>>>>>>>      mfd: intel-lpss: Add support for passing device properties
>>>>>>>>      mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
>>>>>>>>      i2c: designware: Convert to use unified device property API
>>>>>>>
>>>>>>> I'm going to queue up this series for v4.5.
>>>>>>>
>>>>>>> If there are any problems with it or objections from anyone, please let me know.
>>>>>>>
>>>>>>
>>>>>>
>>>>>> Raising an old thread, I pulled this series into Fedora rawhide and
>>>>>> while it worked for Lenovo Yoga we received a report that it caused
>>>>>> a regression on the Dell Inspiron 7559 (see
>>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c27) . I haven't
>>>>>> asked the reporter about bisecting to see which patch broke it.
>>>>>> Were there any known follow up patches?
>>>>>
>>>>> There were a few.
>>>>>
>>>>> All of them are in my linux-next branch if you can try this one.
>>>>>
>>>>> Alternatively, I can expose a branch with these to you to test.
>>>>>
>>>>
>>>> I picked up all the patches from the device-properties merge but the
>>>> problem still shows up. Are there others I should pick up? Hardware
>>>> details about the touchpad are at https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c34
>>>
>>> Well, in that case can you please ask the reporter to bisect?
>>>
>>> Rafael
>>>
>>
>> I'll see if I can help the reporter bisect although it looks like Hans
>> has an idea about what might be failing.
>
> Nope, sorry if I left the impression that I have an idea where things are failing.
>
> Benjamin Tissoires latest reply in this thread (or was it in bugzilla ?) however
> seems to point in the right direction, as with this patch-set the i2c controller
> on the laptop in question starts working (as intended by this patch-set) and then
> i2c-hid tries to talk to the touchpad, but fails too. However it gets far enough
> for the touchpad to drop of the ps/2 bus.
>
> So what seems to be happening is that this is a dual protocol / bus (ps2 / i2c)
> touchpad (not that uncommon actually) and now that we've the i2c bus working due
> to this patchset, we start talking i2c to the touchpad, kicking it from ps2 mode
> to i2c mode, but our i2c driver then fails to work properly with the touchpad
> leaving it non functional.
>
> Regards,
>
> Hans

Thanks for clarifying. I'm going to see if I can help the reporter bisect.
It may take a few days.

Thanks,
Laura

Re: [PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Benjamin Tissoires]

On Jan 11 2016 or thereabouts, Laura Abbott wrote:
> On 01/08/2016 11:22 AM, Hans de Goede wrote:
> >Hi,
> >
> >On 08-01-16 18:08, Laura Abbott wrote:
> >>On 01/07/2016 04:03 PM, Rafael J. Wysocki wrote:
> >>>On Wednesday, January 06, 2016 08:19:49 AM Laura Abbott wrote:
> >>>>On 01/05/2016 03:59 PM, Rafael J. Wysocki wrote:
> >>>>>On Tuesday, January 05, 2016 09:57:35 AM Laura Abbott wrote:
> >>>>>>On 12/06/2015 05:44 PM, Rafael J. Wysocki wrote:
> >>>>>>>On Monday, November 30, 2015 05:11:28 PM Andy Shevchenko wrote:
> >>>>>>>>This series includes few logical sets that bring a support of non-ACPI
> >>>>>>>>platforms for Intel Skylake.
> >>>>>>>>
> >>>>>>>>First part is a refactoring of built-in device properties support:
> >>>>>>>>- keep single value inside the structure
> >>>>>>>>- provide helper macros to define built-in properties
> >>>>>>>>- fall back to secondary fwnode if primary has no asked property
> >>>>>>>>
> >>>>>>>>Second is a propagating built-in device properties in platform core.
> >>>>>>>>
> >>>>>>>>Third one is modifications to MFD code and intel-lpss.c driver in particular
> >>>>>>>>to define and pass built-in properties to the individual drivers.
> >>>>>>>>
> >>>>>>>>And last part is a fix for I2C bug found on Lenovo Yoga hardware and a first
> >>>>>>>>converted user.
> >>>>>>>>
> >>>>>>>>Built-in device properties is an alternative to platform data. It provides a
> >>>>>>>>unified API that drivers can use to cover all cases at once: DT, ACPI, and
> >>>>>>>>built-in properties.
> >>>>>>>>
> >>>>>>>>With this series applied a platform data can be considered obsolete. Moreover,
> >>>>>>>>built-in device properties allow to adjust the existing configuration, for
> >>>>>>>>example, in cases when ACPI values are wrong on some platforms.
> >>>>>>>>
> >>>>>>>>The series has been tested on available hardware and doesn't break current
> >>>>>>>>behaviour. But we ask people who have the affected hardware to apply the series
> >>>>>>>>on your side and check with Lenovo hardware.
> >>>>>>>>
> >>>>>>>>Changelog v2:
> >>>>>>>>- fix isuues found by kbuild bot (kbuild)
> >>>>>>>>- append a patch to propagate device properties in polatform code (Arnd)
> >>>>>>>>- update few existing and add couple of new patches due to above
> >>>>>>>>- check with kmemleak
> >>>>>>>>
> >>>>>>>>Andy Shevchenko (9):
> >>>>>>>>     device property: always check for fwnode type
> >>>>>>>>     device property: rename helper functions
> >>>>>>>>     device property: refactor built-in properties support
> >>>>>>>>     device property: keep single value inplace
> >>>>>>>>     device property: improve readability of macros
> >>>>>>>>     device property: return -EINVAL when property isn't found in ACPI
> >>>>>>>>     device property: Fallback to secondary fwnode if primary misses the
> >>>>>>>>       property
> >>>>>>>>     mfd: core: propagate device properties to sub devices drivers
> >>>>>>>>     mfd: intel-lpss: Pass HSUART configuration via properties
> >>>>>>>>
> >>>>>>>>Heikki Krogerus (1):
> >>>>>>>>     device property: helper macros for property entry creation
> >>>>>>>>
> >>>>>>>>Mika Westerberg (6):
> >>>>>>>>     device property: Take a copy of the property set
> >>>>>>>>     driver core: platform: Add support for built-in device properties
> >>>>>>>>     driver core: Do not overwrite secondary fwnode with NULL if it is set
> >>>>>>>>     mfd: intel-lpss: Add support for passing device properties
> >>>>>>>>     mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
> >>>>>>>>     i2c: designware: Convert to use unified device property API
> >>>>>>>
> >>>>>>>I'm going to queue up this series for v4.5.
> >>>>>>>
> >>>>>>>If there are any problems with it or objections from anyone, please let me know.
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>Raising an old thread, I pulled this series into Fedora rawhide and
> >>>>>>while it worked for Lenovo Yoga we received a report that it caused
> >>>>>>a regression on the Dell Inspiron 7559 (see
> >>>>>>https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c27) . I haven't
> >>>>>>asked the reporter about bisecting to see which patch broke it.
> >>>>>>Were there any known follow up patches?
> >>>>>
> >>>>>There were a few.
> >>>>>
> >>>>>All of them are in my linux-next branch if you can try this one.
> >>>>>
> >>>>>Alternatively, I can expose a branch with these to you to test.
> >>>>>
> >>>>
> >>>>I picked up all the patches from the device-properties merge but the
> >>>>problem still shows up. Are there others I should pick up? Hardware
> >>>>details about the touchpad are at https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c34
> >>>
> >>>Well, in that case can you please ask the reporter to bisect?
> >>>
> >>>Rafael
> >>>
> >>
> >>I'll see if I can help the reporter bisect although it looks like Hans
> >>has an idea about what might be failing.
> >
> >Nope, sorry if I left the impression that I have an idea where things are failing.
> >
> >Benjamin Tissoires latest reply in this thread (or was it in bugzilla ?) however
> >seems to point in the right direction, as with this patch-set the i2c controller
> >on the laptop in question starts working (as intended by this patch-set) and then
> >i2c-hid tries to talk to the touchpad, but fails too. However it gets far enough
> >for the touchpad to drop of the ps/2 bus.
> >
> >So what seems to be happening is that this is a dual protocol / bus (ps2 / i2c)
> >touchpad (not that uncommon actually) and now that we've the i2c bus working due
> >to this patchset, we start talking i2c to the touchpad, kicking it from ps2 mode
> >to i2c mode, but our i2c driver then fails to work properly with the touchpad
> >leaving it non functional.
> >
> >Regards,
> >
> >Hans
> 
> Thanks for clarifying. I'm going to see if I can help the reporter bisect.
> It may take a few days.
> 

Hi,

as Hans said, I am pretty sure that the regression is in fact a
different bug in hid-multitouch or in i2c-hid which will not be shown in
the bisect. Before those patches, we were not able to make his
i2c controller working, and the magic within his touchpad made it work
through ps/2. Starting from this commit, the i2c controller now works,
which switches the touchpad in the i2c (enhanced) mode.
It is unfortunate that his touchpad doesn't work over i2c, but a bisect
should not give any information more than this patch series switches his
touchpad from ps/2 to i2c.

To sum up, I'll handle the i2c not working bug when he opened the new
bug, and there should be no need for a bisect (unless there are other
issues besides his touchpad not working).

Cheers,
Benjamin

Re: [PATCH v2 14/16] mfd: intel-lpss: Pass SDA hold time to I2C host controller driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1299 bytes --]

On Mon, Nov 30, 2015 at 05:11:42PM +0200, Andy Shevchenko wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Intel Skylake the LPSS I2C pad circuit has internal delays that require
> programming non-zero SDA hold time for the I2C host controller. If this is
> not done communication to slave devices may fail with arbitration lost
> errors like the one seen below taken from Lenovo Yoga 900:
> 
>   i2c_hid i2c-SYNA2B29:00: Fetching the HID descriptor
>   i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=20 00
>   i2c_designware i2c_designware.1: i2c_dw_handle_tx_abort: lost arbitration
> 
> To fix this we follow what the Windows driver is doing and pass the default
> SDA hold time of 230 ns to all Intel Skylake host controllers. This still
> allows the platform to override these values by passing special ACPI
> methods SSCN and FMCN.
> 
> Reported-by: Kevin Fenzi <kevin@scrye.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

There has been a request to get this into stable:

https://bugzilla.kernel.org/show_bug.cgi?id=108581

Since we probably don't want to backport the whole device_property
series, is there a simpler fix to backport for stable?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 14/16] mfd: intel-lpss: Pass SDA hold time to I2C host controller driver

[Mika Westerberg]

On Mon, Feb 08, 2016 at 11:09:43AM +0100, Wolfram Sang wrote:
> On Mon, Nov 30, 2015 at 05:11:42PM +0200, Andy Shevchenko wrote:
> > From: Mika Westerberg <mika.westerberg@linux.intel.com>
> > 
> > Intel Skylake the LPSS I2C pad circuit has internal delays that require
> > programming non-zero SDA hold time for the I2C host controller. If this is
> > not done communication to slave devices may fail with arbitration lost
> > errors like the one seen below taken from Lenovo Yoga 900:
> > 
> >   i2c_hid i2c-SYNA2B29:00: Fetching the HID descriptor
> >   i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=20 00
> >   i2c_designware i2c_designware.1: i2c_dw_handle_tx_abort: lost arbitration
> > 
> > To fix this we follow what the Windows driver is doing and pass the default
> > SDA hold time of 230 ns to all Intel Skylake host controllers. This still
> > allows the platform to override these values by passing special ACPI
> > methods SSCN and FMCN.
> > 
> > Reported-by: Kevin Fenzi <kevin@scrye.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> There has been a request to get this into stable:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=108581
> 
> Since we probably don't want to backport the whole device_property
> series, is there a simpler fix to backport for stable?

Basically all it needs is to have:

	dev->sda_hold_time = 28

in dw_i2c_acpi_configure(). However, we cannot just unconditionally set
it as it will break existing non-SKL systems so there should be some way
to check if this is SKL or not. Perhaps we could look for DMI entries of
the affected machines?

BTW, this is not a regression in a sense as SKL I2C never worked before
this series.

Re: [PATCH v2 14/16] mfd: intel-lpss: Pass SDA hold time to I2C host controller driver

[Andy Shevchenko]

On Mon, Feb 8, 2016 at 12:29 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Mon, Feb 08, 2016 at 11:09:43AM +0100, Wolfram Sang wrote:
>> On Mon, Nov 30, 2015 at 05:11:42PM +0200, Andy Shevchenko wrote:

>> There has been a request to get this into stable:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=108581
>>
>> Since we probably don't want to backport the whole device_property
>> series, is there a simpler fix to backport for stable?
>
> Basically all it needs is to have:
>
>         dev->sda_hold_time = 28
>
> in dw_i2c_acpi_configure(). However, we cannot just unconditionally set
> it as it will break existing non-SKL systems so there should be some way
> to check if this is SKL or not. Perhaps we could look for DMI entries of
> the affected machines?
>
> BTW, this is not a regression in a sense as SKL I2C never worked before
> this series.

I could add that "backporting" something which is drastically
different to the original mainstream may probably prevent a smooth
backporting the fixes come later if any. Thus, I would try to port as
much closer as we can to the existing upstream code.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 14/16] mfd: intel-lpss: Pass SDA hold time to I2C host controller driver

[Lee Jones]

On Mon, 30 Nov 2015, Andy Shevchenko wrote:

> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Intel Skylake the LPSS I2C pad circuit has internal delays that require
> programming non-zero SDA hold time for the I2C host controller. If this is
> not done communication to slave devices may fail with arbitration lost
> errors like the one seen below taken from Lenovo Yoga 900:
> 
>   i2c_hid i2c-SYNA2B29:00: Fetching the HID descriptor
>   i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=20 00
>   i2c_designware i2c_designware.1: i2c_dw_handle_tx_abort: lost arbitration
> 
> To fix this we follow what the Windows driver is doing and pass the default
> SDA hold time of 230 ns to all Intel Skylake host controllers. This still
> allows the platform to override these values by passing special ACPI
> methods SSCN and FMCN.
> 
> Reported-by: Kevin Fenzi <kevin@scrye.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel-lpss-acpi.c | 19 +++++++++++++++++--
>  drivers/mfd/intel-lpss-pci.c  | 31 +++++++++++++++++++++++--------
>  2 files changed, 40 insertions(+), 10 deletions(-)

Seems fine in principle.

Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/drivers/mfd/intel-lpss-acpi.c b/drivers/mfd/intel-lpss-acpi.c
> index b6fd904..06f00d6 100644
> --- a/drivers/mfd/intel-lpss-acpi.c
> +++ b/drivers/mfd/intel-lpss-acpi.c
> @@ -18,6 +18,7 @@
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
>  
>  #include "intel-lpss.h"
>  
> @@ -25,6 +26,20 @@ static const struct intel_lpss_platform_info spt_info = {
>  	.clk_rate = 120000000,
>  };
>  
> +static struct property_entry spt_i2c_properties[] = {
> +	PROPERTY_ENTRY_U32("i2c-sda-hold-time-ns", 230),
> +	{ },
> +};
> +
> +static struct property_set spt_i2c_pset = {
> +	.properties = spt_i2c_properties,
> +};
> +
> +static const struct intel_lpss_platform_info spt_i2c_info = {
> +	.clk_rate = 120000000,
> +	.pset = &spt_i2c_pset,
> +};
> +
>  static const struct intel_lpss_platform_info bxt_info = {
>  	.clk_rate = 100000000,
>  };
> @@ -35,8 +50,8 @@ static const struct intel_lpss_platform_info bxt_i2c_info = {
>  
>  static const struct acpi_device_id intel_lpss_acpi_ids[] = {
>  	/* SPT */
> -	{ "INT3446", (kernel_ulong_t)&spt_info },
> -	{ "INT3447", (kernel_ulong_t)&spt_info },
> +	{ "INT3446", (kernel_ulong_t)&spt_i2c_info },
> +	{ "INT3447", (kernel_ulong_t)&spt_i2c_info },
>  	/* BXT */
>  	{ "80860AAC", (kernel_ulong_t)&bxt_i2c_info },
>  	{ "80860ABC", (kernel_ulong_t)&bxt_info },
> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> index 5bfdfcc..a677480 100644
> --- a/drivers/mfd/intel-lpss-pci.c
> +++ b/drivers/mfd/intel-lpss-pci.c
> @@ -17,6 +17,7 @@
>  #include <linux/pci.h>
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/property.h>
>  
>  #include "intel-lpss.h"
>  
> @@ -65,6 +66,20 @@ static const struct intel_lpss_platform_info spt_info = {
>  	.clk_rate = 120000000,
>  };
>  
> +static struct property_entry spt_i2c_properties[] = {
> +	PROPERTY_ENTRY_U32("i2c-sda-hold-time-ns", 230),
> +	{ },
> +};
> +
> +static struct property_set spt_i2c_pset = {
> +	.properties = spt_i2c_properties,
> +};
> +
> +static const struct intel_lpss_platform_info spt_i2c_info = {
> +	.clk_rate = 120000000,
> +	.pset = &spt_i2c_pset,
> +};
> +
>  static const struct intel_lpss_platform_info spt_uart_info = {
>  	.clk_rate = 120000000,
>  	.clk_con_id = "baudclk",
> @@ -121,20 +136,20 @@ static const struct pci_device_id intel_lpss_pci_ids[] = {
>  	{ PCI_VDEVICE(INTEL, 0x9d28), (kernel_ulong_t)&spt_uart_info },
>  	{ PCI_VDEVICE(INTEL, 0x9d29), (kernel_ulong_t)&spt_info },
>  	{ PCI_VDEVICE(INTEL, 0x9d2a), (kernel_ulong_t)&spt_info },
> -	{ PCI_VDEVICE(INTEL, 0x9d60), (kernel_ulong_t)&spt_info },
> -	{ PCI_VDEVICE(INTEL, 0x9d61), (kernel_ulong_t)&spt_info },
> -	{ PCI_VDEVICE(INTEL, 0x9d62), (kernel_ulong_t)&spt_info },
> -	{ PCI_VDEVICE(INTEL, 0x9d63), (kernel_ulong_t)&spt_info },
> -	{ PCI_VDEVICE(INTEL, 0x9d64), (kernel_ulong_t)&spt_info },
> -	{ PCI_VDEVICE(INTEL, 0x9d65), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d60), (kernel_ulong_t)&spt_i2c_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d61), (kernel_ulong_t)&spt_i2c_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d62), (kernel_ulong_t)&spt_i2c_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d63), (kernel_ulong_t)&spt_i2c_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d64), (kernel_ulong_t)&spt_i2c_info },
> +	{ PCI_VDEVICE(INTEL, 0x9d65), (kernel_ulong_t)&spt_i2c_info },
>  	{ PCI_VDEVICE(INTEL, 0x9d66), (kernel_ulong_t)&spt_uart_info },
>  	/* SPT-H */
>  	{ PCI_VDEVICE(INTEL, 0xa127), (kernel_ulong_t)&spt_uart_info },
>  	{ PCI_VDEVICE(INTEL, 0xa128), (kernel_ulong_t)&spt_uart_info },
>  	{ PCI_VDEVICE(INTEL, 0xa129), (kernel_ulong_t)&spt_info },
>  	{ PCI_VDEVICE(INTEL, 0xa12a), (kernel_ulong_t)&spt_info },
> -	{ PCI_VDEVICE(INTEL, 0xa160), (kernel_ulong_t)&spt_info },
> -	{ PCI_VDEVICE(INTEL, 0xa161), (kernel_ulong_t)&spt_info },
> +	{ PCI_VDEVICE(INTEL, 0xa160), (kernel_ulong_t)&spt_i2c_info },
> +	{ PCI_VDEVICE(INTEL, 0xa161), (kernel_ulong_t)&spt_i2c_info },
>  	{ PCI_VDEVICE(INTEL, 0xa166), (kernel_ulong_t)&spt_uart_info },
>  	{ }
>  };

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog