Skip to contentSkip to conversation

FAILED: patch "[PATCH] iio: adc: ad7124: fix DT configuration parsing" failed to apply to 6.6-stable tree

PATCH New [email protected] 2 participants 10 messages Sep 9, 2024 – Sep 12, 2024 2 messages not in the archive

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 6.6-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
git checkout FETCH_HEAD
git cherry-pick -x 61cbfb5368dd50ed0d65ce21d305aa923581db2b
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024090914-province-underdone-1eea@gregkh' --subject-prefix 'PATCH 6.6.y' HEAD^..

Possible dependencies:

61cbfb5368dd ("iio: adc: ad7124: fix DT configuration parsing")
a6eaf02b8274 ("iio: adc: ad7124: Switch from of specific to fwnode based property handling")

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 61cbfb5368dd50ed0d65ce21d305aa923581db2b Mon Sep 17 00:00:00 2001
From: Dumitru Ceclan <[email protected]>
Date: Tue, 6 Aug 2024 11:51:33 +0300
Subject: [PATCH] iio: adc: ad7124: fix DT configuration parsing

The cfg pointer is set before reading the channel number that the configuration should point to. This causes configurations to be shifted by one channel.
For example setting bipolar to the first channel defined in the DT will cause bipolar mode to be active on the second defined channel.

Fix by moving the cfg pointer setting after reading the channel number.

Fixes 7b8d045e497a ("iio: adc: ad7124: allow more than 8 channels")
drivers/iio/adc/ad7124.c +1 -2
index afb5f4d741e6..108e9ccab1ef 100644
@@ -844,8 +844,6 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
 	st->channels = channels;
 
 	device_for_each_child_node_scoped(dev, child) {
-		cfg = &st->channels[channel].cfg;
-
 		ret = fwnode_property_read_u32(child, "reg", &channel);
 		if (ret)
 			return ret;
@@ -863,6 +861,7 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
 		st->channels[channel].ain = AD7124_CHANNEL_AINP(ain[0]) |
 						  AD7124_CHANNEL_AINM(ain[1]);
 
+		cfg = &st->channels[channel].cfg;
 		cfg->bipolar = fwnode_property_read_bool(child, "bipolar");
 
 		ret = fwnode_property_read_u32(child, "adi,reference-select", &tmp);
#1 lore raw

PATCH 1/1 iio: adc: ad7124: fix DT configuration parsing

From: Dumitru Ceclan <[email protected]>

From: Dumitru Ceclan <[email protected]>

[ Upstream commit 61cbfb5368dd50ed0d65ce21d305aa923581db2b ]

The cfg pointer is set before reading the channel number that the configuration should point to. This causes configurations to be shifted by one channel.
For example setting bipolar to the first channel defined in the DT will cause bipolar mode to be active on the second defined channel.

Fix by moving the cfg pointer setting after reading the channel number.

---
 drivers/iio/adc/ad7124.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/iio/adc/ad7124.c +1 -2
index e7b1d517d3de..089398a7664a 100644
@@ -837,8 +837,6 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
 	st->channels = channels;
 
 	device_for_each_child_node_scoped(dev, child) {
-		cfg = &st->channels[channel].cfg;
-
 		ret = fwnode_property_read_u32(child, "reg", &channel);
 		if (ret)
 			return ret;
@@ -856,6 +854,7 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
 		st->channels[channel].ain = AD7124_CHANNEL_AINP(ain[0]) |
 						  AD7124_CHANNEL_AINM(ain[1]);
 
+		cfg = &st->channels[channel].cfg;
 		cfg->bipolar = fwnode_property_read_bool(child, "bipolar");
 
 		ret = fwnode_property_read_u32(child, "adi,reference-select", &tmp);
Signature · 1 line
2.45.2
#2 lore raw

PATCH 1/2 iio: adc: ad7124: Switch from of specific to fwnode based property handling

In reply to [email protected]

From:Jonathan Cameron <[email protected]>

[ Upstream commit a6eaf02b82744b424b9b2c74847282deb2c6f77b ]

Using the generic firmware data access functions from property.h provides a number of advantages:
1) Works with different firmware types.
2) Doesn't provide a 'bad' example for new IIO drivers.
3) Lets us use the new _scoped() loops with automatic reference count cleanup for fwnode_handle

---
 drivers/iio/adc/ad7124.c | 55 +++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 32 deletions(-)
drivers/iio/adc/ad7124.c +23 -32
index b9b206fcd748..e7b1d517d3de 100644
@@ -14,7 +14,8 @@
 #include <linux/kernel.h>
 #include <linux/kfifo.h>
 #include <linux/module.h>
-#include <linux/of.h>
+#include <linux/mod_devicetable.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 
@@ -807,22 +808,19 @@ static int ad7124_check_chip_id(struct ad7124_state *st)
 	return 0;
 }
 
-static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
-					  struct device_node *np)
+static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
+				       struct device *dev)
 {
 	struct ad7124_state *st = iio_priv(indio_dev);
 	struct ad7124_channel_config *cfg;
 	struct ad7124_channel *channels;
-	struct device_node *child;
 	struct iio_chan_spec *chan;
 	unsigned int ain[2], channel = 0, tmp;
 	int ret;
 
-	st->num_channels = of_get_available_child_count(np);
-	if (!st->num_channels) {
-		dev_err(indio_dev->dev.parent, "no channel children\n");
-		return -ENODEV;
-	}
+	st->num_channels = device_get_child_node_count(dev);
+	if (!st->num_channels)
+		return dev_err_probe(dev, -ENODEV, "no channel children\n");
 
 	chan = devm_kcalloc(indio_dev->dev.parent, st->num_channels,
 			    sizeof(*chan), GFP_KERNEL);
@@ -838,39 +836,38 @@ static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
 	indio_dev->num_channels = st->num_channels;
 	st->channels = channels;
 
-	for_each_available_child_of_node(np, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		cfg = &st->channels[channel].cfg;
 
-		ret = of_property_read_u32(child, "reg", &channel);
+		ret = fwnode_property_read_u32(child, "reg", &channel);
 		if (ret)
-			goto err;
+			return ret;
 
-		if (channel >= indio_dev->num_channels) {
-			dev_err(indio_dev->dev.parent,
+		if (channel >= indio_dev->num_channels)
+			return dev_err_probe(dev, -EINVAL,
 				"Channel index >= number of channels\n");
-			ret = -EINVAL;
-			goto err;
-		}
 
-		ret = of_property_read_u32_array(child, "diff-channels",
-						 ain, 2);
+		ret = fwnode_property_read_u32_array(child, "diff-channels",
+						     ain, 2);
 		if (ret)
-			goto err;
+			return ret;
 
 		st->channels[channel].nr = channel;
 		st->channels[channel].ain = AD7124_CHANNEL_AINP(ain[0]) |
 						  AD7124_CHANNEL_AINM(ain[1]);
 
-		cfg->bipolar = of_property_read_bool(child, "bipolar");
+		cfg->bipolar = fwnode_property_read_bool(child, "bipolar");
 
-		ret = of_property_read_u32(child, "adi,reference-select", &tmp);
+		ret = fwnode_property_read_u32(child, "adi,reference-select", &tmp);
 		if (ret)
 			cfg->refsel = AD7124_INT_REF;
 		else
 			cfg->refsel = tmp;
 
-		cfg->buf_positive = of_property_read_bool(child, "adi,buffered-positive");
-		cfg->buf_negative = of_property_read_bool(child, "adi,buffered-negative");
+		cfg->buf_positive =
+			fwnode_property_read_bool(child, "adi,buffered-positive");
+		cfg->buf_negative =
+			fwnode_property_read_bool(child, "adi,buffered-negative");
 
 		chan[channel] = ad7124_channel_template;
 		chan[channel].address = channel;
@@ -880,10 +877,6 @@ static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
 	}
 
 	return 0;
-err:
-	of_node_put(child);
-
-	return ret;
 }
 
 static int ad7124_setup(struct ad7124_state *st)
@@ -943,9 +936,7 @@ static int ad7124_probe(struct spi_device *spi)
 	struct iio_dev *indio_dev;
 	int i, ret;
 
-	info = of_device_get_match_data(&spi->dev);
-	if (!info)
-		info = (void *)spi_get_device_id(spi)->driver_data;
+	info = spi_get_device_match_data(spi);
 	if (!info)
 		return -ENODEV;
 
@@ -965,7 +956,7 @@ static int ad7124_probe(struct spi_device *spi)
 	if (ret < 0)
 		return ret;
 
-	ret = ad7124_of_parse_channel_config(indio_dev, spi->dev.of_node);
+	ret = ad7124_parse_channel_config(indio_dev, &spi->dev);
 	if (ret < 0)
 		return ret;
 
Signature · 1 line
2.45.2

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw

PATCH 2/2 iio: adc: ad7124: fix DT configuration parsing

From: Dumitru Ceclan <[email protected]>

From: Dumitru Ceclan <[email protected]>

[ Upstream commit 61cbfb5368dd50ed0d65ce21d305aa923581db2b ]

The cfg pointer is set before reading the channel number that the configuration should point to. This causes configurations to be shifted by one channel.
For example setting bipolar to the first channel defined in the DT will cause bipolar mode to be active on the second defined channel.

Fix by moving the cfg pointer setting after reading the channel number.

---
 drivers/iio/adc/ad7124.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/iio/adc/ad7124.c +1 -2
index e7b1d517d3de..089398a7664a 100644
@@ -837,8 +837,6 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
 	st->channels = channels;
 
 	device_for_each_child_node_scoped(dev, child) {
-		cfg = &st->channels[channel].cfg;
-
 		ret = fwnode_property_read_u32(child, "reg", &channel);
 		if (ret)
 			return ret;
@@ -856,6 +854,7 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
 		st->channels[channel].ain = AD7124_CHANNEL_AINP(ain[0]) |
 						  AD7124_CHANNEL_AINM(ain[1]);
 
+		cfg = &st->channels[channel].cfg;
 		cfg->bipolar = fwnode_property_read_bool(child, "bipolar");
 
 		ret = fwnode_property_read_u32(child, "adi,reference-select", &tmp);
Signature · 1 line
2.45.2
author maintainer
#5 lore raw

Re: [PATCH 6.6.y 2/2] iio: adc: ad7124: fix DT configuration parsing

Quoted 3 lines · He Lugang

From: Dumitru Ceclan <[email protected]>

From: Dumitru Ceclan <[email protected]>

Why is this 2 different addresses?

And why was this sent twice?

confused,

greg k-h

#6 lore raw

Re: [PATCH 6.6.y 2/2] iio: adc: ad7124: fix DT configuration parsing

Quoted 5 lines · Greg KH
Quoted 3 lines · He Lugang

From: Dumitru Ceclan <[email protected]>

From: Dumitru Ceclan <[email protected]>

Quoted 8 lines

Why is this 2 different addresses?

And why was this sent twice?

confused,

greg k-h

Hi,greg

About the two email addresses because the email address mismatch message from the checkpatch.pl,so I just and the signed-off-by one,should we just ignore it?

Also pls just pass over the first patch mail because of lack of dependency.

----
Regards,
Lugang

author maintainer
#7 lore raw

Re: [PATCH 6.6.y 2/2] iio: adc: ad7124: fix DT configuration parsing

Quoted 14 lines · HeLuang
Quoted 11 lines · Greg KH
Quoted 3 lines · He Lugang

From: Dumitru Ceclan <[email protected]>

From: Dumitru Ceclan <[email protected]>

Why is this 2 different addresses?

And why was this sent twice?

confused,

Quoted 8 lines
Quoted 2 lines

greg k-h

Hi,greg

About the two email addresses because the email address mismatch message from the checkpatch.pl,so I just and the signed-off-by one,should we just ignore it?

Please use what is in the upstream commit.

Quoted 1 line

Also pls just pass over the first patch mail because of lack of dependency.

Please tell us what is going on, otherwise we get confused.

Please fix this up and resend a new series that we can take.

thanks,

greg k-h

#8 lore raw

Re: [PATCH 6.6.y 2/2] iio: adc: ad7124: fix DT configuration parsing

Quoted 24 lines · Greg KH
Quoted 14 lines · HeLuang
Quoted 11 lines · Greg KH
Quoted 3 lines · He Lugang

From: Dumitru Ceclan <[email protected]>

From: Dumitru Ceclan <[email protected]>

Why is this 2 different addresses?

And why was this sent twice?

confused,

Quoted 8 lines
Quoted 2 lines

greg k-h

Hi,greg

About the two email addresses because the email address mismatch message from the checkpatch.pl,so I just and the signed-off-by one,should we just ignore it?

Quoted 5 lines

Please use what is in the upstream commit.

Quoted 1 line

Also pls just pass over the first patch mail because of lack of dependency.

Please tell us what is going on, otherwise we get confused.

Actually
61cbfb5368dd ("iio: adc: ad7124: fix DT configuration parsing")
depends on
a6eaf02b8274 ("iio: adc: ad7124: Switch from of specific to fwnode based property handling") so just need to cherry-pick them together!

Quoted 2 lines

Please fix this up and resend a new series that we can take.

OK.

Quoted 5 lines

thanks,

greg k-h

#9 lore raw

PATCH RESEND 1/2 iio: adc: ad7124: Switch from of specific to fwnode based property handling

In reply to [email protected]

From: Jonathan Cameron <[email protected]>

commit a6eaf02b82744b424b9b2c74847282deb2c6f77b upstream

Using the generic firmware data access functions from property.h provides a number of advantages:
1) Works with different firmware types.
2) Doesn't provide a 'bad' example for new IIO drivers.
3) Lets us use the new _scoped() loops with automatic reference count cleanup for fwnode_handle

---
 drivers/iio/adc/ad7124.c | 55 +++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 32 deletions(-)
drivers/iio/adc/ad7124.c +23 -32
index b9b206fcd748..e7b1d517d3de 100644
@@ -14,7 +14,8 @@
 #include <linux/kernel.h>
 #include <linux/kfifo.h>
 #include <linux/module.h>
-#include <linux/of.h>
+#include <linux/mod_devicetable.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 
@@ -807,22 +808,19 @@ static int ad7124_check_chip_id(struct ad7124_state *st)
 	return 0;
 }
 
-static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
-					  struct device_node *np)
+static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
+				       struct device *dev)
 {
 	struct ad7124_state *st = iio_priv(indio_dev);
 	struct ad7124_channel_config *cfg;
 	struct ad7124_channel *channels;
-	struct device_node *child;
 	struct iio_chan_spec *chan;
 	unsigned int ain[2], channel = 0, tmp;
 	int ret;
 
-	st->num_channels = of_get_available_child_count(np);
-	if (!st->num_channels) {
-		dev_err(indio_dev->dev.parent, "no channel children\n");
-		return -ENODEV;
-	}
+	st->num_channels = device_get_child_node_count(dev);
+	if (!st->num_channels)
+		return dev_err_probe(dev, -ENODEV, "no channel children\n");
 
 	chan = devm_kcalloc(indio_dev->dev.parent, st->num_channels,
 			    sizeof(*chan), GFP_KERNEL);
@@ -838,39 +836,38 @@ static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
 	indio_dev->num_channels = st->num_channels;
 	st->channels = channels;
 
-	for_each_available_child_of_node(np, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		cfg = &st->channels[channel].cfg;
 
-		ret = of_property_read_u32(child, "reg", &channel);
+		ret = fwnode_property_read_u32(child, "reg", &channel);
 		if (ret)
-			goto err;
+			return ret;
 
-		if (channel >= indio_dev->num_channels) {
-			dev_err(indio_dev->dev.parent,
+		if (channel >= indio_dev->num_channels)
+			return dev_err_probe(dev, -EINVAL,
 				"Channel index >= number of channels\n");
-			ret = -EINVAL;
-			goto err;
-		}
 
-		ret = of_property_read_u32_array(child, "diff-channels",
-						 ain, 2);
+		ret = fwnode_property_read_u32_array(child, "diff-channels",
+						     ain, 2);
 		if (ret)
-			goto err;
+			return ret;
 
 		st->channels[channel].nr = channel;
 		st->channels[channel].ain = AD7124_CHANNEL_AINP(ain[0]) |
 						  AD7124_CHANNEL_AINM(ain[1]);
 
-		cfg->bipolar = of_property_read_bool(child, "bipolar");
+		cfg->bipolar = fwnode_property_read_bool(child, "bipolar");
 
-		ret = of_property_read_u32(child, "adi,reference-select", &tmp);
+		ret = fwnode_property_read_u32(child, "adi,reference-select", &tmp);
 		if (ret)
 			cfg->refsel = AD7124_INT_REF;
 		else
 			cfg->refsel = tmp;
 
-		cfg->buf_positive = of_property_read_bool(child, "adi,buffered-positive");
-		cfg->buf_negative = of_property_read_bool(child, "adi,buffered-negative");
+		cfg->buf_positive =
+			fwnode_property_read_bool(child, "adi,buffered-positive");
+		cfg->buf_negative =
+			fwnode_property_read_bool(child, "adi,buffered-negative");
 
 		chan[channel] = ad7124_channel_template;
 		chan[channel].address = channel;
@@ -880,10 +877,6 @@ static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
 	}
 
 	return 0;
-err:
-	of_node_put(child);
-
-	return ret;
 }
 
 static int ad7124_setup(struct ad7124_state *st)
@@ -943,9 +936,7 @@ static int ad7124_probe(struct spi_device *spi)
 	struct iio_dev *indio_dev;
 	int i, ret;
 
-	info = of_device_get_match_data(&spi->dev);
-	if (!info)
-		info = (void *)spi_get_device_id(spi)->driver_data;
+	info = spi_get_device_match_data(spi);
 	if (!info)
 		return -ENODEV;
 
@@ -965,7 +956,7 @@ static int ad7124_probe(struct spi_device *spi)
 	if (ret < 0)
 		return ret;
 
-	ret = ad7124_of_parse_channel_config(indio_dev, spi->dev.of_node);
+	ret = ad7124_parse_channel_config(indio_dev, &spi->dev);
 	if (ret < 0)
 		return ret;
 
Signature · 1 line
2.45.2

A message that is not in the archive (sent off-list or not yet received): [email protected]

#11 lore raw

PATCH RESEND 2/2 iio: adc: ad7124: fix DT configuration parsing

From: Dumitru Ceclan <[email protected]>

commit 61cbfb5368dd50ed0d65ce21d305aa923581db2b upstream

The cfg pointer is set before reading the channel number that the configuration should point to. This causes configurations to be shifted by one channel.
For example setting bipolar to the first channel defined in the DT will cause bipolar mode to be active on the second defined channel.

Fix by moving the cfg pointer setting after reading the channel number.

---
 drivers/iio/adc/ad7124.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/iio/adc/ad7124.c +1 -2
index e7b1d517d3de..089398a7664a 100644
@@ -837,8 +837,6 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
 	st->channels = channels;
 
 	device_for_each_child_node_scoped(dev, child) {
-		cfg = &st->channels[channel].cfg;
-
 		ret = fwnode_property_read_u32(child, "reg", &channel);
 		if (ret)
 			return ret;
@@ -856,6 +854,7 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
 		st->channels[channel].ain = AD7124_CHANNEL_AINP(ain[0]) |
 						  AD7124_CHANNEL_AINM(ain[1]);
 
+		cfg = &st->channels[channel].cfg;
 		cfg->bipolar = fwnode_property_read_bool(child, "bipolar");
 
 		ret = fwnode_property_read_u32(child, "adi,reference-select", &tmp);
Signature · 1 line
2.45.2