[PATCH 3/4] parser: Handle multiple reference headers

Stephen Finucane stephen at that.guru
Wed May 24 16:02:23 AEST 2017


It's possible to duplicate message headers multiple times. One common
case is the 'Received' header, but it appears that multiple
'In-Reply-To' and 'References' headers are also a thing.

Handle these cases through the use of the 'Message.get_all' function,
which returns all matching headers, instead of the 'Message.get'
function previously used.

Signed-off-by: Stephen Finucane <stephen at that.guru>
---
 patchwork/parser.py                                |  21 +-
 .../tests/series/bugs-multiple-references.mbox     | 423 +++++++++++++++++++++
 patchwork/tests/test_series.py                     |  22 ++
 3 files changed, 459 insertions(+), 7 deletions(-)
 create mode 100644 patchwork/tests/series/bugs-multiple-references.mbox

diff --git a/patchwork/parser.py b/patchwork/parser.py
index d7941e3..3526e48 100644
--- a/patchwork/parser.py
+++ b/patchwork/parser.py
@@ -335,18 +335,25 @@ def find_headers(mail):
 
 
 def find_references(mail):
-    """Construct a list of possible reply message ids."""
+    """Construct a list of possible reply message ids.
+
+    Extract 'in-reply-to' and 'references' headers from a given mail
+    and return the combined set of each. Because headers can be
+    duplicated, 'get_all' is used rather than 'get'.
+    """
     refs = []
 
     if 'In-Reply-To' in mail:
-        refs.append(mail.get('In-Reply-To').strip())
+        for in_reply_to in mail.get_all('In-Reply-To'):
+            refs.append(in_reply_to.strip())
 
     if 'References' in mail:
-        rs = mail.get('References').split()
-        rs.reverse()
-        for r in rs:
-            if r not in refs:
-                refs.append(r.strip())
+        for references_header in mail.get_all('References'):
+            references = references_header.split()
+            references.reverse()
+            for ref in references:
+                if ref not in refs:
+                    refs.append(ref.strip())
 
     return refs
 
diff --git a/patchwork/tests/series/bugs-multiple-references.mbox b/patchwork/tests/series/bugs-multiple-references.mbox
new file mode 100644
index 0000000..96e4072
--- /dev/null
+++ b/patchwork/tests/series/bugs-multiple-references.mbox
@@ -0,0 +1,423 @@
+From viresh.kumar at linaro.org Tue May 23 04:01:41 2017
+Return-Path: <linux-kernel-owner at vger.kernel.org>
+Delivered-To: patchwork at patchwork.dja.id.au
+From: Viresh Kumar <viresh.kumar at linaro.org>
+To: Rafael Wysocki <rjw at rjwysocki.net>
+Cc: Viresh Kumar <viresh.kumar at linaro.org>,
+        linaro-kernel at lists.linaro.org, linux-pm at vger.kernel.org,
+        linux-kernel at vger.kernel.org,
+        Vincent Guittot <vincent.guittot at linaro.org>
+Subject: [PATCH V2 0/4] PM / OPP: Minor cleanups
+Date: Tue, 23 May 2017 09:31:41 +0530
+Message-Id: <cover.1495511998.git.viresh.kumar at linaro.org>
+X-Mailer: git-send-email 2.13.0.70.g6367777092d9
+Sender: linux-kernel-owner at vger.kernel.org
+Precedence: bulk
+List-ID: <linux-kernel.vger.kernel.org>
+X-Mailing-List: linux-kernel at vger.kernel.org
+
+Hi,
+
+Here are few cleanup patches for the OPP core. The first two simplify
+the code that was written specifically due to the limitations that we
+had because of RCUs. We don't RCUs anymore and this can be simplified.
+
+The last two take care of specific corner cases.
+
+Rebased over pm/linux-next and tested on Exynos dual core board.
+
+V1->V2:
+- Some RBY from Stephen
+- s/while/for/ for one of the loops
+- Dropped a comment and fixed an error message
+- opp-table marked as const in one of the place.
+
+Viresh Kumar (4):
+  PM / OPP: Reorganize _generic_set_opp_regulator()
+  PM / OPP: Don't create copy of regulators unnecessarily
+  PM / OPP: opp-microvolt is not optional if regulators are set
+  PM / OPP: Don't create debugfs "supply-0" directory unnecessarily
+
+ drivers/base/power/opp/core.c    | 87 +++++++++++++++++-----------------------
+ drivers/base/power/opp/debugfs.c |  7 ++--
+ drivers/base/power/opp/of.c      | 10 ++++-
+ 3 files changed, 48 insertions(+), 56 deletions(-)
+
+
+From viresh.kumar at linaro.org Tue May 23 04:02:10 2017
+Return-Path: <linux-kernel-owner at vger.kernel.org>
+Delivered-To: patchwork at patchwork.dja.id.au
+From: Viresh Kumar <viresh.kumar at linaro.org>
+To: Rafael Wysocki <rjw at rjwysocki.net>,
+        Viresh Kumar <vireshk at kernel.org>, Nishanth Menon <nm at ti.com>,
+        Stephen Boyd <sboyd at codeaurora.org>
+Cc: Viresh Kumar <viresh.kumar at linaro.org>,
+        linaro-kernel at lists.linaro.org, linux-pm at vger.kernel.org,
+        linux-kernel at vger.kernel.org,
+        Vincent Guittot <vincent.guittot at linaro.org>
+Subject: [PATCH V2 1/4] PM / OPP: Reorganize _generic_set_opp_regulator()
+Date: Tue, 23 May 2017 09:32:10 +0530
+Message-Id: 
+ <b9f67b2be37d5e9740a5e1ed85b0e8b4e5c0eb91.1495511998.git.viresh.kumar at linaro.org>
+X-Mailer: git-send-email 2.13.0.70.g6367777092d9
+In-Reply-To: <cover.1495511998.git.viresh.kumar at linaro.org>
+References: <cover.1495511998.git.viresh.kumar at linaro.org>
+Sender: linux-kernel-owner at vger.kernel.org
+Precedence: bulk
+List-ID: <linux-kernel.vger.kernel.org>
+X-Mailing-List: linux-kernel at vger.kernel.org
+
+The code was overly complicated here because of the limitations that we
+had with RCUs (Couldn't use opp-table and OPPs outside RCU protected
+section and can't call sleep-able routines from within that). But that
+is long gone now.
+
+Reorganize _generic_set_opp_regulator() in order to avoid using "struct
+dev_pm_set_opp_data" and copying data into it for the case where
+opp_table->set_opp is not set.
+
+Signed-off-by: Viresh Kumar <viresh.kumar at linaro.org>
+Reviewed-by: Stephen Boyd <sboyd at codeaurora.org>
+---
+ drivers/base/power/opp/core.c | 73 ++++++++++++++++++++-----------------------
+ 1 file changed, 34 insertions(+), 39 deletions(-)
+
+diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
+index dae61720b314..c4590fc017ba 100644
+--- a/drivers/base/power/opp/core.c
++++ b/drivers/base/power/opp/core.c
+@@ -543,17 +543,18 @@ _generic_set_opp_clk_only(struct device *dev, struct clk *clk,
+        return ret;
+ }
+ 
+-static int _generic_set_opp(struct dev_pm_set_opp_data *data)
++static int _generic_set_opp_regulator(const struct opp_table *opp_table,
++                                     struct device *dev,
++                                     unsigned long old_freq,
++                                     unsigned long freq,
++                                     struct dev_pm_opp_supply *old_supply,
++                                     struct dev_pm_opp_supply *new_supply)
+ {
+-       struct dev_pm_opp_supply *old_supply = data->old_opp.supplies;
+-       struct dev_pm_opp_supply *new_supply = data->new_opp.supplies;
+-       unsigned long old_freq = data->old_opp.rate, freq = data->new_opp.rate;
+-       struct regulator *reg = data->regulators[0];
+-       struct device *dev= data->dev;
++       struct regulator *reg = opp_table->regulators[0];
+        int ret;
+ 
+        /* This function only supports single regulator per device */
+-       if (WARN_ON(data->regulator_count > 1)) {
++       if (WARN_ON(opp_table->regulator_count > 1)) {
+                dev_err(dev, "multiple regulators are not supported\n");
+                return -EINVAL;
+        }
+@@ -566,7 +567,7 @@ static int _generic_set_opp(struct dev_pm_set_opp_data *data)
+        }
+ 
+        /* Change frequency */
+-       ret = _generic_set_opp_clk_only(dev, data->clk, old_freq, freq);
++       ret = _generic_set_opp_clk_only(dev, opp_table->clk, old_freq, freq);
+        if (ret)
+                goto restore_voltage;
+ 
+@@ -580,12 +581,12 @@ static int _generic_set_opp(struct dev_pm_set_opp_data *data)
+        return 0;
+ 
+ restore_freq:
+-       if (_generic_set_opp_clk_only(dev, data->clk, freq, old_freq))
++       if (_generic_set_opp_clk_only(dev, opp_table->clk, freq, old_freq))
+                dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n",
+                        __func__, old_freq);
+ restore_voltage:
+        /* This shouldn't harm even if the voltages weren't updated earlier */
+-       if (old_supply->u_volt)
++       if (old_supply)
+                _set_opp_voltage(dev, reg, old_supply);
+ 
+        return ret;
+@@ -603,10 +604,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
+ {
+        struct opp_table *opp_table;
+        unsigned long freq, old_freq;
+-       int (*set_opp)(struct dev_pm_set_opp_data *data);
+        struct dev_pm_opp *old_opp, *opp;
+-       struct regulator **regulators;
+-       struct dev_pm_set_opp_data *data;
+        struct clk *clk;
+        int ret, size;
+ 
+@@ -661,38 +659,35 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
+        dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n", __func__,
+                old_freq, freq);
+ 
+-       regulators = opp_table->regulators;
+-
+        /* Only frequency scaling */
+-       if (!regulators) {
++       if (!opp_table->regulators) {
+                ret = _generic_set_opp_clk_only(dev, clk, old_freq, freq);
+-               goto put_opps;
+-       }
++       } else if (!opp_table->set_opp) {
++               ret = _generic_set_opp_regulator(opp_table, dev, old_freq, freq,
++                                                IS_ERR(old_opp) ? NULL : old_opp->supplies,
++                                                opp->supplies);
++       } else {
++               struct dev_pm_set_opp_data *data;
+ 
+-       if (opp_table->set_opp)
+-               set_opp = opp_table->set_opp;
+-       else
+-               set_opp = _generic_set_opp;
+-
+-       data = opp_table->set_opp_data;
+-       data->regulators = regulators;
+-       data->regulator_count = opp_table->regulator_count;
+-       data->clk = clk;
+-       data->dev = dev;
+-
+-       data->old_opp.rate = old_freq;
+-       size = sizeof(*opp->supplies) * opp_table->regulator_count;
+-       if (IS_ERR(old_opp))
+-               memset(data->old_opp.supplies, 0, size);
+-       else
+-               memcpy(data->old_opp.supplies, old_opp->supplies, size);
++               data = opp_table->set_opp_data;
++               data->regulators = opp_table->regulators;
++               data->regulator_count = opp_table->regulator_count;
++               data->clk = clk;
++               data->dev = dev;
+ 
+-       data->new_opp.rate = freq;
+-       memcpy(data->new_opp.supplies, opp->supplies, size);
++               data->old_opp.rate = old_freq;
++               size = sizeof(*opp->supplies) * opp_table->regulator_count;
++               if (IS_ERR(old_opp))
++                       memset(data->old_opp.supplies, 0, size);
++               else
++                       memcpy(data->old_opp.supplies, old_opp->supplies, size);
+ 
+-       ret = set_opp(data);
++               data->new_opp.rate = freq;
++               memcpy(data->new_opp.supplies, opp->supplies, size);
++
++               ret = opp_table->set_opp(data);
++       }
+ 
+-put_opps:
+        dev_pm_opp_put(opp);
+ put_old_opp:
+        if (!IS_ERR(old_opp))
+
+
+From viresh.kumar at linaro.org Tue May 23 04:02:11 2017
+Return-Path: <linux-kernel-owner at vger.kernel.org>
+From: Viresh Kumar <viresh.kumar at linaro.org>
+To: Rafael Wysocki <rjw at rjwysocki.net>,
+        Viresh Kumar <vireshk at kernel.org>, Nishanth Menon <nm at ti.com>,
+        Stephen Boyd <sboyd at codeaurora.org>
+Cc: Viresh Kumar <viresh.kumar at linaro.org>,
+        linaro-kernel at lists.linaro.org, linux-pm at vger.kernel.org,
+        linux-kernel at vger.kernel.org,
+        Vincent Guittot <vincent.guittot at linaro.org>
+Subject: [PATCH V2 2/4] PM / OPP: Don't create copy of regulators
+ unnecessarily
+Date: Tue, 23 May 2017 09:32:11 +0530
+Message-Id: 
+ <ed9beaec36649c862369a34ea209822c00d86f52.1495511998.git.viresh.kumar at linaro.org>
+X-Mailer: git-send-email 2.13.0.70.g6367777092d9
+In-Reply-To: 
+ <b9f67b2be37d5e9740a5e1ed85b0e8b4e5c0eb91.1495511998.git.viresh.kumar at linaro.org>
+References: 
+ <b9f67b2be37d5e9740a5e1ed85b0e8b4e5c0eb91.1495511998.git.viresh.kumar at linaro.org>
+In-Reply-To: <cover.1495511998.git.viresh.kumar at linaro.org>
+References: <cover.1495511998.git.viresh.kumar at linaro.org>
+Sender: linux-kernel-owner at vger.kernel.org
+Precedence: bulk
+List-ID: <linux-kernel.vger.kernel.org>
+X-Mailing-List: linux-kernel at vger.kernel.org
+
+This code was required while the OPP core was managed with help of RCUs,
+but not anymore. Get rid of unnecessary alloc/memcpy operations.
+
+Signed-off-by: Viresh Kumar <viresh.kumar at linaro.org>
+Reviewed-by: Stephen Boyd <sboyd at codeaurora.org>
+---
+ drivers/base/power/opp/core.c | 14 +++-----------
+ 1 file changed, 3 insertions(+), 11 deletions(-)
+
+diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
+index c4590fc017ba..5ee7aadf0abf 100644
+--- a/drivers/base/power/opp/core.c
++++ b/drivers/base/power/opp/core.c
+@@ -180,7 +180,7 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
+ {
+ 	struct opp_table *opp_table;
+ 	struct dev_pm_opp *opp;
+-	struct regulator *reg, **regulators;
++	struct regulator *reg;
+ 	unsigned long latency_ns = 0;
+ 	int ret, i, count;
+ 	struct {
+@@ -198,15 +198,9 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
+ 	if (!count)
+ 		goto put_opp_table;
+ 
+-	regulators = kmalloc_array(count, sizeof(*regulators), GFP_KERNEL);
+-	if (!regulators)
+-		goto put_opp_table;
+-
+ 	uV = kmalloc_array(count, sizeof(*uV), GFP_KERNEL);
+ 	if (!uV)
+-		goto free_regulators;
+-
+-	memcpy(regulators, opp_table->regulators, count * sizeof(*regulators));
++		goto put_opp_table;
+ 
+ 	mutex_lock(&opp_table->lock);
+ 
+@@ -232,15 +226,13 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
+ 	 * isn't freed, while we are executing this routine.
+ 	 */
+ 	for (i = 0; i < count; i++) {
+-		reg = regulators[i];
++		reg = opp_table->regulators[i];
+ 		ret = regulator_set_voltage_time(reg, uV[i].min, uV[i].max);
+ 		if (ret > 0)
+ 			latency_ns += ret * 1000;
+ 	}
+ 
+ 	kfree(uV);
+-free_regulators:
+-	kfree(regulators);
+ put_opp_table:
+ 	dev_pm_opp_put_opp_table(opp_table);
+ 
+
+From viresh.kumar at linaro.org Tue May 23 04:02:12 2017
+Return-Path: <linux-kernel-owner at vger.kernel.org>
+Delivered-To: patchwork at patchwork.dja.id.au
+From: Viresh Kumar <viresh.kumar at linaro.org>
+To: Rafael Wysocki <rjw at rjwysocki.net>,
+        Viresh Kumar <vireshk at kernel.org>, Nishanth Menon <nm at ti.com>,
+        Stephen Boyd <sboyd at codeaurora.org>
+Cc: Viresh Kumar <viresh.kumar at linaro.org>,
+        linaro-kernel at lists.linaro.org, linux-pm at vger.kernel.org,
+        linux-kernel at vger.kernel.org,
+        Vincent Guittot <vincent.guittot at linaro.org>
+Subject: [PATCH V2 3/4] PM / OPP: opp-microvolt is not optional if regulators
+ are set
+Date: Tue, 23 May 2017 09:32:12 +0530
+Message-Id: 
+ <dcb9872771568fec4c7d8d11ba4939cd8da279c0.1495511998.git.viresh.kumar at linaro.org>
+X-Mailer: git-send-email 2.13.0.70.g6367777092d9
+In-Reply-To: 
+ <b9f67b2be37d5e9740a5e1ed85b0e8b4e5c0eb91.1495511998.git.viresh.kumar at linaro.org>
+References: 
+ <b9f67b2be37d5e9740a5e1ed85b0e8b4e5c0eb91.1495511998.git.viresh.kumar at linaro.org>
+In-Reply-To: <cover.1495511998.git.viresh.kumar at linaro.org>
+References: <cover.1495511998.git.viresh.kumar at linaro.org>
+Sender: linux-kernel-owner at vger.kernel.org
+Precedence: bulk
+List-ID: <linux-kernel.vger.kernel.org>
+X-Mailing-List: linux-kernel at vger.kernel.org
+
+If dev_pm_opp_set_regulators() is called for a device and its regulators
+are set in the OPP core, the OPP nodes for the device must contain the
+"opp-microvolt" property, otherwise there is something wrong and we
+better error out.
+
+Signed-off-by: Viresh Kumar <viresh.kumar at linaro.org>
+---
+ drivers/base/power/opp/of.c | 10 ++++++++--
+ 1 file changed, 8 insertions(+), 2 deletions(-)
+
+diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
+index 779428676f63..57eec1ca0569 100644
+--- a/drivers/base/power/opp/of.c
++++ b/drivers/base/power/opp/of.c
+@@ -131,8 +131,14 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
+ 		prop = of_find_property(opp->np, name, NULL);
+ 
+ 		/* Missing property isn't a problem, but an invalid entry is */
+-		if (!prop)
+-			return 0;
++		if (!prop) {
++			if (!opp_table->regulator_count)
++				return 0;
++
++			dev_err(dev, "%s: opp-microvolt missing although OPP managing regulators\n",
++				__func__);
++			return -EINVAL;
++		}
+ 	}
+ 
+ 	vcount = of_property_count_u32_elems(opp->np, name);
+
+From viresh.kumar at linaro.org Tue May 23 04:02:13 2017
+Return-Path: <linux-kernel-owner at vger.kernel.org>
+Delivered-To: patchwork at patchwork.dja.id.au
+From: Viresh Kumar <viresh.kumar at linaro.org>
+To: Rafael Wysocki <rjw at rjwysocki.net>,
+        Viresh Kumar <vireshk at kernel.org>, Nishanth Menon <nm at ti.com>,
+        Stephen Boyd <sboyd at codeaurora.org>
+Cc: Viresh Kumar <viresh.kumar at linaro.org>,
+        linaro-kernel at lists.linaro.org, linux-pm at vger.kernel.org,
+        linux-kernel at vger.kernel.org,
+        Vincent Guittot <vincent.guittot at linaro.org>
+Subject: [PATCH V2 4/4] PM / OPP: Don't create debugfs "supply-0" directory
+ unnecessarily
+Date: Tue, 23 May 2017 09:32:13 +0530
+Message-Id: 
+ <0381fc50b84535fcb7964eaa345d2501c5c903b3.1495511998.git.viresh.kumar at linaro.org>
+X-Mailer: git-send-email 2.13.0.70.g6367777092d9
+In-Reply-To: 
+ <b9f67b2be37d5e9740a5e1ed85b0e8b4e5c0eb91.1495511998.git.viresh.kumar at linaro.org>
+References: 
+ <b9f67b2be37d5e9740a5e1ed85b0e8b4e5c0eb91.1495511998.git.viresh.kumar at linaro.org>
+In-Reply-To: <cover.1495511998.git.viresh.kumar at linaro.org>
+References: <cover.1495511998.git.viresh.kumar at linaro.org>
+Sender: linux-kernel-owner at vger.kernel.org
+Precedence: bulk
+List-ID: <linux-kernel.vger.kernel.org>
+X-Mailing-List: linux-kernel at vger.kernel.org
+
+We create "supply-0" debugfs directory even if the device doesn't do
+voltage scaling. That looks confusing, as if the regulator is found but
+we never managed to get voltage levels for it.
+
+Avoid creating such a directory unnecessarily.
+
+Signed-off-by: Viresh Kumar <viresh.kumar at linaro.org>
+---
+ drivers/base/power/opp/debugfs.c | 7 +++----
+ 1 file changed, 3 insertions(+), 4 deletions(-)
+
+diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c
+index 95f433db4ac7..81cf120fcf43 100644
+--- a/drivers/base/power/opp/debugfs.c
++++ b/drivers/base/power/opp/debugfs.c
+@@ -40,11 +40,10 @@ static bool opp_debug_create_supplies(struct dev_pm_opp *opp,
+ 				      struct dentry *pdentry)
+ {
+ 	struct dentry *d;
+-	int i = 0;
++	int i;
+ 	char *name;
+ 
+-	/* Always create at least supply-0 directory */
+-	do {
++	for (i = 0; i < opp_table->regulator_count; i++) {
+ 		name = kasprintf(GFP_KERNEL, "supply-%d", i);
+ 
+ 		/* Create per-opp directory */
+@@ -70,7 +69,7 @@ static bool opp_debug_create_supplies(struct dev_pm_opp *opp,
+ 		if (!debugfs_create_ulong("u_amp", S_IRUGO, d,
+ 					  &opp->supplies[i].u_amp))
+ 			return false;
+-	} while (++i < opp_table->regulator_count);
++	}
+ 
+ 	return true;
+ }
diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py
index 52fede1..06288de 100644
--- a/patchwork/tests/test_series.py
+++ b/patchwork/tests/test_series.py
@@ -167,6 +167,28 @@ class BaseSeriesTest(_BaseTestCase):
 
         self.assertSerialized(patches_a + patches_b, [2, 2])
 
+    def test_multiple_references(self):
+        """Series received with multiple reference headers.
+
+        Parse a series with four patches and a cover letter that is received
+        with multiple reference headers.
+
+        Input:
+          - [PATCH V2 0/4] PM / OPP: Minor cleanups
+            - [PATCH V2 1/4] PM / OPP: Reorganize _generic_set_opp_regulator()
+            - [PATCH V2 2/4] PM / OPP: Don't create copy of regulators
+                unnecessarily
+            - PATCH V2 3/4] PM / OPP: opp-microvolt is not optional if
+                regulators are set
+            - [PATCH V2 4/4] PM / OPP: Don't create debugfs "supply-0"
+                directory unnecessarily
+        """
+        covers, patches, _ = self._parse_mbox(
+            'bugs-multiple-references.mbox', [1, 4, 0])
+
+        self.assertSerialized(covers, [1])
+        self.assertSerialized(patches, [4])
+
     def test_no_references(self):
         """Series received with no reference headers.
 
-- 
2.9.4



More information about the Patchwork mailing list