diff for duplicates of <ccefbd0b-3397-a26e-95e7-059fcced9154@st.com> diff --git a/a/1.txt b/N1/1.txt index 76bf5ed..afb83b7 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,37 +1,66 @@ -SGkgRGFuLA0KDQpUaGFua3MgZm9yIHlvdXIgcGF0Y2guIE9uZSBtaW5vciBjb21tZW50Og0KDQpP -biA1LzE1LzE5IDExOjMxIEFNLCBEYW4gQ2FycGVudGVyIHdyb3RlOg0KPiBUaGUgcHJvYmxlbSBp -cyB0aGF0IG9uIDY0Yml0IHN5c3RlbXMgdGhlbiB3ZSBkb24ndCBjbGVhciB0aGUgaGlnaGVyDQo+ -IGJpdHMgb2YgdGhlICJwZW5kaW5nIiB2YXJpYWJsZS4gIFNvIHdoZW4gd2UgZG86DQo+IA0KPiAJ -YWNrID0gcGVuZGluZyAmIH5CSVQoU1RNRlhfUkVHX0lSUV9TUkNfRU5fR1BJTyk7DQo+IAlpZiAo -YWNrKSB7DQo+IA0KPiB0aGUgaWYgKGFjaykgY29uZGl0aW9uIHJlbGllcyBvbiB1bmluaXRpYWxp -emVkIGRhdGEuICBUaGUgZml4IGl0IHRoYXQNCj4gSSd2ZSBjaGFuZ2VkICJwZW5kaW5nIiBmcm9t -IGFuIHVuc2lnbmVkIGxvbmcgdG8gYSB1MzIuICBJIGNoYW5nZWQgIm4iIGFzDQo+IHdlbGwsIGJl -Y2F1c2UgdGhhdCdzIGEgbnVtYmVyIGluIHRoZSAwLTEwIHJhbmdlIGFuZCBpdCBmaXRzIGVhc2ls -eQ0KPiBpbnNpZGUgYW4gaW50LiAgV2UgZG8gbmVlZCB0byBhZGQgYSBjYXN0IHRvICJwZW5kaW5n -IiB3aGVuIHdlIHVzZSBpdCBpbg0KPiB0aGUgZm9yX2VhY2hfc2V0X2JpdCgpIGxvb3AsIGJ1dCB0 -aGF0IGRvZXNuJ3QgY2F1c2UgYSBwcm9ibGUsIGl0J3MNCj4gZmluZS4NCj4gDQo+IEZpeGVzOiAw -NjI1MmFkZTkxNTYgKCJtZmQ6IEFkZCBTVCBNdWx0aS1GdW5jdGlvbiBlWHBhbmRlciAoU1RNRlgp -IGNvcmUgZHJpdmVyIikNCj4gU2lnbmVkLW9mZi1ieTogRGFuIENhcnBlbnRlciA8ZGFuLmNhcnBl -bnRlckBvcmFjbGUuY29tPg0KPiAtLS0NCj4gICBkcml2ZXJzL21mZC9zdG1meC5jIHwgOCArKysr -LS0tLQ0KPiAgIDEgZmlsZSBjaGFuZ2VkLCA0IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0p -DQo+IA0KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9tZmQvc3RtZnguYyBiL2RyaXZlcnMvbWZkL3N0 -bWZ4LmMNCj4gaW5kZXggZmU4ZWZiYTJkNDVmLi5mZWU3NWI1ZDA5OGUgMTAwNjQ0DQo+IC0tLSBh -L2RyaXZlcnMvbWZkL3N0bWZ4LmMNCj4gKysrIGIvZHJpdmVycy9tZmQvc3RtZnguYw0KPiBAQCAt -MjA0LDEyICsyMDQsMTIgQEAgc3RhdGljIHN0cnVjdCBpcnFfY2hpcCBzdG1meF9pcnFfY2hpcCA9 -IHsNCj4gICBzdGF0aWMgaXJxcmV0dXJuX3Qgc3RtZnhfaXJxX2hhbmRsZXIoaW50IGlycSwgdm9p -ZCAqZGF0YSkNCj4gICB7DQo+ICAgCXN0cnVjdCBzdG1meCAqc3RtZnggPSBkYXRhOw0KPiAtCXVu -c2lnbmVkIGxvbmcgbiwgcGVuZGluZzsNCj4gKwl1MzIgcGVuZGluZzsNCj4gICAJdTMyIGFjazsN -Cj4gKwlpbnQgbjsNCj4gICAJaW50IHJldDsNCg0KQ291bGQgeW91IGdyb3VwOg0KDQp1MzIgcGVu -ZGluZywgYWNrOw0KaW50IG4sIHJldDsNCg0KPiAgIA0KPiAtCXJldCA9IHJlZ21hcF9yZWFkKHN0 -bWZ4LT5tYXAsIFNUTUZYX1JFR19JUlFfUEVORElORywNCj4gLQkJCSAgKHUzMiAqKSZwZW5kaW5n -KTsNCj4gKwlyZXQgPSByZWdtYXBfcmVhZChzdG1meC0+bWFwLCBTVE1GWF9SRUdfSVJRX1BFTkRJ -TkcsICZwZW5kaW5nKTsNCj4gICAJaWYgKHJldCkNCj4gICAJCXJldHVybiBJUlFfTk9ORTsNCj4g -ICANCj4gQEAgLTIyNCw3ICsyMjQsNyBAQCBzdGF0aWMgaXJxcmV0dXJuX3Qgc3RtZnhfaXJxX2hh -bmRsZXIoaW50IGlycSwgdm9pZCAqZGF0YSkNCj4gICAJCQlyZXR1cm4gSVJRX05PTkU7DQo+ICAg -CX0NCj4gICANCj4gLQlmb3JfZWFjaF9zZXRfYml0KG4sICZwZW5kaW5nLCBTVE1GWF9SRUdfSVJR -X1NSQ19NQVgpDQo+ICsJZm9yX2VhY2hfc2V0X2JpdChuLCAodW5zaWduZWQgbG9uZyAqKSZwZW5k -aW5nLCBTVE1GWF9SRUdfSVJRX1NSQ19NQVgpDQo+ICAgCQloYW5kbGVfbmVzdGVkX2lycShpcnFf -ZmluZF9tYXBwaW5nKHN0bWZ4LT5pcnFfZG9tYWluLCBuKSk7DQo+ICAgDQo+ICAgCXJldHVybiBJ -UlFfSEFORExFRDsNCj4gDQoNCkkndmUgdGVzdGVkIGl0IG9uIHN0bTMybXAxNTdjLWV2MSwgc28g -eW91IGNhbiBhZGQgbXkNClRlc3RlZC1ieTogQW1lbGllIERlbGF1bmF5IDxhbWVsaWUuZGVsYXVu -YXlAc3QuY29tPg0KDQpSZWdhcmRzLA0KQW1lbGll +Hi Dan, + +Thanks for your patch. One minor comment: + +On 5/15/19 11:31 AM, Dan Carpenter wrote: +> The problem is that on 64bit systems then we don't clear the higher +> bits of the "pending" variable. So when we do: +> +> ack = pending & ~BIT(STMFX_REG_IRQ_SRC_EN_GPIO); +> if (ack) { +> +> the if (ack) condition relies on uninitialized data. The fix it that +> I've changed "pending" from an unsigned long to a u32. I changed "n" as +> well, because that's a number in the 0-10 range and it fits easily +> inside an int. We do need to add a cast to "pending" when we use it in +> the for_each_set_bit() loop, but that doesn't cause a proble, it's +> fine. +> +> Fixes: 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver") +> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> +> --- +> drivers/mfd/stmfx.c | 8 ++++---- +> 1 file changed, 4 insertions(+), 4 deletions(-) +> +> diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c +> index fe8efba2d45f..fee75b5d098e 100644 +> --- a/drivers/mfd/stmfx.c +> +++ b/drivers/mfd/stmfx.c +> @@ -204,12 +204,12 @@ static struct irq_chip stmfx_irq_chip = { +> static irqreturn_t stmfx_irq_handler(int irq, void *data) +> { +> struct stmfx *stmfx = data; +> - unsigned long n, pending; +> + u32 pending; +> u32 ack; +> + int n; +> int ret; + +Could you group: + +u32 pending, ack; +int n, ret; + +> +> - ret = regmap_read(stmfx->map, STMFX_REG_IRQ_PENDING, +> - (u32 *)&pending); +> + ret = regmap_read(stmfx->map, STMFX_REG_IRQ_PENDING, &pending); +> if (ret) +> return IRQ_NONE; +> +> @@ -224,7 +224,7 @@ static irqreturn_t stmfx_irq_handler(int irq, void *data) +> return IRQ_NONE; +> } +> +> - for_each_set_bit(n, &pending, STMFX_REG_IRQ_SRC_MAX) +> + for_each_set_bit(n, (unsigned long *)&pending, STMFX_REG_IRQ_SRC_MAX) +> handle_nested_irq(irq_find_mapping(stmfx->irq_domain, n)); +> +> return IRQ_HANDLED; +> + +I've tested it on stm32mp157c-ev1, so you can add my +Tested-by: Amelie Delaunay <amelie.delaunay@st.com> + +Regards, +Amelie diff --git a/a/content_digest b/N1/content_digest index ae9fdff..2c7b025 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,7 +1,7 @@ "ref\020190515093141.GA3409@mwanda\0" "From\0Amelie DELAUNAY <amelie.delaunay@st.com>\0" "Subject\0Re: [PATCH] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()\0" - "Date\0Mon, 03 Jun 2019 09:20:01 +0000\0" + "Date\0Mon, 3 Jun 2019 09:20:01 +0000\0" "To\0Dan Carpenter <dan.carpenter@oracle.com>" " Lee Jones <lee.jones@linaro.org>\0" "Cc\0Maxime Coquelin <mcoquelin.stm32@gmail.com>" @@ -11,42 +11,71 @@ " kernel-janitors@vger.kernel.org <kernel-janitors@vger.kernel.org>\0" "\00:1\0" "b\0" - "SGkgRGFuLA0KDQpUaGFua3MgZm9yIHlvdXIgcGF0Y2guIE9uZSBtaW5vciBjb21tZW50Og0KDQpP\n" - "biA1LzE1LzE5IDExOjMxIEFNLCBEYW4gQ2FycGVudGVyIHdyb3RlOg0KPiBUaGUgcHJvYmxlbSBp\n" - "cyB0aGF0IG9uIDY0Yml0IHN5c3RlbXMgdGhlbiB3ZSBkb24ndCBjbGVhciB0aGUgaGlnaGVyDQo+\n" - "IGJpdHMgb2YgdGhlICJwZW5kaW5nIiB2YXJpYWJsZS4gIFNvIHdoZW4gd2UgZG86DQo+IA0KPiAJ\n" - "YWNrID0gcGVuZGluZyAmIH5CSVQoU1RNRlhfUkVHX0lSUV9TUkNfRU5fR1BJTyk7DQo+IAlpZiAo\n" - "YWNrKSB7DQo+IA0KPiB0aGUgaWYgKGFjaykgY29uZGl0aW9uIHJlbGllcyBvbiB1bmluaXRpYWxp\n" - "emVkIGRhdGEuICBUaGUgZml4IGl0IHRoYXQNCj4gSSd2ZSBjaGFuZ2VkICJwZW5kaW5nIiBmcm9t\n" - "IGFuIHVuc2lnbmVkIGxvbmcgdG8gYSB1MzIuICBJIGNoYW5nZWQgIm4iIGFzDQo+IHdlbGwsIGJl\n" - "Y2F1c2UgdGhhdCdzIGEgbnVtYmVyIGluIHRoZSAwLTEwIHJhbmdlIGFuZCBpdCBmaXRzIGVhc2ls\n" - "eQ0KPiBpbnNpZGUgYW4gaW50LiAgV2UgZG8gbmVlZCB0byBhZGQgYSBjYXN0IHRvICJwZW5kaW5n\n" - "IiB3aGVuIHdlIHVzZSBpdCBpbg0KPiB0aGUgZm9yX2VhY2hfc2V0X2JpdCgpIGxvb3AsIGJ1dCB0\n" - "aGF0IGRvZXNuJ3QgY2F1c2UgYSBwcm9ibGUsIGl0J3MNCj4gZmluZS4NCj4gDQo+IEZpeGVzOiAw\n" - "NjI1MmFkZTkxNTYgKCJtZmQ6IEFkZCBTVCBNdWx0aS1GdW5jdGlvbiBlWHBhbmRlciAoU1RNRlgp\n" - "IGNvcmUgZHJpdmVyIikNCj4gU2lnbmVkLW9mZi1ieTogRGFuIENhcnBlbnRlciA8ZGFuLmNhcnBl\n" - "bnRlckBvcmFjbGUuY29tPg0KPiAtLS0NCj4gICBkcml2ZXJzL21mZC9zdG1meC5jIHwgOCArKysr\n" - "LS0tLQ0KPiAgIDEgZmlsZSBjaGFuZ2VkLCA0IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0p\n" - "DQo+IA0KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9tZmQvc3RtZnguYyBiL2RyaXZlcnMvbWZkL3N0\n" - "bWZ4LmMNCj4gaW5kZXggZmU4ZWZiYTJkNDVmLi5mZWU3NWI1ZDA5OGUgMTAwNjQ0DQo+IC0tLSBh\n" - "L2RyaXZlcnMvbWZkL3N0bWZ4LmMNCj4gKysrIGIvZHJpdmVycy9tZmQvc3RtZnguYw0KPiBAQCAt\n" - "MjA0LDEyICsyMDQsMTIgQEAgc3RhdGljIHN0cnVjdCBpcnFfY2hpcCBzdG1meF9pcnFfY2hpcCA9\n" - "IHsNCj4gICBzdGF0aWMgaXJxcmV0dXJuX3Qgc3RtZnhfaXJxX2hhbmRsZXIoaW50IGlycSwgdm9p\n" - "ZCAqZGF0YSkNCj4gICB7DQo+ICAgCXN0cnVjdCBzdG1meCAqc3RtZnggPSBkYXRhOw0KPiAtCXVu\n" - "c2lnbmVkIGxvbmcgbiwgcGVuZGluZzsNCj4gKwl1MzIgcGVuZGluZzsNCj4gICAJdTMyIGFjazsN\n" - "Cj4gKwlpbnQgbjsNCj4gICAJaW50IHJldDsNCg0KQ291bGQgeW91IGdyb3VwOg0KDQp1MzIgcGVu\n" - "ZGluZywgYWNrOw0KaW50IG4sIHJldDsNCg0KPiAgIA0KPiAtCXJldCA9IHJlZ21hcF9yZWFkKHN0\n" - "bWZ4LT5tYXAsIFNUTUZYX1JFR19JUlFfUEVORElORywNCj4gLQkJCSAgKHUzMiAqKSZwZW5kaW5n\n" - "KTsNCj4gKwlyZXQgPSByZWdtYXBfcmVhZChzdG1meC0+bWFwLCBTVE1GWF9SRUdfSVJRX1BFTkRJ\n" - "TkcsICZwZW5kaW5nKTsNCj4gICAJaWYgKHJldCkNCj4gICAJCXJldHVybiBJUlFfTk9ORTsNCj4g\n" - "ICANCj4gQEAgLTIyNCw3ICsyMjQsNyBAQCBzdGF0aWMgaXJxcmV0dXJuX3Qgc3RtZnhfaXJxX2hh\n" - "bmRsZXIoaW50IGlycSwgdm9pZCAqZGF0YSkNCj4gICAJCQlyZXR1cm4gSVJRX05PTkU7DQo+ICAg\n" - "CX0NCj4gICANCj4gLQlmb3JfZWFjaF9zZXRfYml0KG4sICZwZW5kaW5nLCBTVE1GWF9SRUdfSVJR\n" - "X1NSQ19NQVgpDQo+ICsJZm9yX2VhY2hfc2V0X2JpdChuLCAodW5zaWduZWQgbG9uZyAqKSZwZW5k\n" - "aW5nLCBTVE1GWF9SRUdfSVJRX1NSQ19NQVgpDQo+ICAgCQloYW5kbGVfbmVzdGVkX2lycShpcnFf\n" - "ZmluZF9tYXBwaW5nKHN0bWZ4LT5pcnFfZG9tYWluLCBuKSk7DQo+ICAgDQo+ICAgCXJldHVybiBJ\n" - "UlFfSEFORExFRDsNCj4gDQoNCkkndmUgdGVzdGVkIGl0IG9uIHN0bTMybXAxNTdjLWV2MSwgc28g\n" - "eW91IGNhbiBhZGQgbXkNClRlc3RlZC1ieTogQW1lbGllIERlbGF1bmF5IDxhbWVsaWUuZGVsYXVu\n" - YXlAc3QuY29tPg0KDQpSZWdhcmRzLA0KQW1lbGll + "Hi Dan,\n" + "\n" + "Thanks for your patch. One minor comment:\n" + "\n" + "On 5/15/19 11:31 AM, Dan Carpenter wrote:\n" + "> The problem is that on 64bit systems then we don't clear the higher\n" + "> bits of the \"pending\" variable. So when we do:\n" + "> \n" + "> \tack = pending & ~BIT(STMFX_REG_IRQ_SRC_EN_GPIO);\n" + "> \tif (ack) {\n" + "> \n" + "> the if (ack) condition relies on uninitialized data. The fix it that\n" + "> I've changed \"pending\" from an unsigned long to a u32. I changed \"n\" as\n" + "> well, because that's a number in the 0-10 range and it fits easily\n" + "> inside an int. We do need to add a cast to \"pending\" when we use it in\n" + "> the for_each_set_bit() loop, but that doesn't cause a proble, it's\n" + "> fine.\n" + "> \n" + "> Fixes: 06252ade9156 (\"mfd: Add ST Multi-Function eXpander (STMFX) core driver\")\n" + "> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>\n" + "> ---\n" + "> drivers/mfd/stmfx.c | 8 ++++----\n" + "> 1 file changed, 4 insertions(+), 4 deletions(-)\n" + "> \n" + "> diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c\n" + "> index fe8efba2d45f..fee75b5d098e 100644\n" + "> --- a/drivers/mfd/stmfx.c\n" + "> +++ b/drivers/mfd/stmfx.c\n" + "> @@ -204,12 +204,12 @@ static struct irq_chip stmfx_irq_chip = {\n" + "> static irqreturn_t stmfx_irq_handler(int irq, void *data)\n" + "> {\n" + "> \tstruct stmfx *stmfx = data;\n" + "> -\tunsigned long n, pending;\n" + "> +\tu32 pending;\n" + "> \tu32 ack;\n" + "> +\tint n;\n" + "> \tint ret;\n" + "\n" + "Could you group:\n" + "\n" + "u32 pending, ack;\n" + "int n, ret;\n" + "\n" + "> \n" + "> -\tret = regmap_read(stmfx->map, STMFX_REG_IRQ_PENDING,\n" + "> -\t\t\t (u32 *)&pending);\n" + "> +\tret = regmap_read(stmfx->map, STMFX_REG_IRQ_PENDING, &pending);\n" + "> \tif (ret)\n" + "> \t\treturn IRQ_NONE;\n" + "> \n" + "> @@ -224,7 +224,7 @@ static irqreturn_t stmfx_irq_handler(int irq, void *data)\n" + "> \t\t\treturn IRQ_NONE;\n" + "> \t}\n" + "> \n" + "> -\tfor_each_set_bit(n, &pending, STMFX_REG_IRQ_SRC_MAX)\n" + "> +\tfor_each_set_bit(n, (unsigned long *)&pending, STMFX_REG_IRQ_SRC_MAX)\n" + "> \t\thandle_nested_irq(irq_find_mapping(stmfx->irq_domain, n));\n" + "> \n" + "> \treturn IRQ_HANDLED;\n" + "> \n" + "\n" + "I've tested it on stm32mp157c-ev1, so you can add my\n" + "Tested-by: Amelie Delaunay <amelie.delaunay@st.com>\n" + "\n" + "Regards,\n" + Amelie -974de24e279c846ef7ac18dfc6720c2b50a0d400535ee84deeb34245cc695f75 +ae55cdc2bdb96187ca24583e333e21fec19db0e5a8b7f5184575fc50b4f7bb95
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.