This series fixes some minor issues I've found while bowsing
in various MTD drivers.
[PATCH 1/6] mtd: r852: Fix device_create_file() usage
[PATCH 2/6] mtd: nandsim: Fix kasprintf() usage
[PATCH 3/6] mtd: cs553x_nand: Fix kasprintf() usage
[PATCH 4/6] mtd: docg3: Don't leak docg3->bbt in error path
[PATCH 5/6] mtd: docg3: Fix kasprintf() usage
[PATCH 6/6] mtd: docg3: Don't do ERR_PTR(0)
Thanks,
//richard
device_create_file() can fail, therefore we have to
handle this case and abort.
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/nand/r852.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/r852.c b/drivers/mtd/nand/r852.c
index baea83f..77e96d2 100644
--- a/drivers/mtd/nand/r852.c
+++ b/drivers/mtd/nand/r852.c
@@ -653,11 +653,15 @@ static int r852_register_nand_device(struct r852_device *dev)
if (sm_register_device(dev->mtd, dev->sm))
goto error2;
- if (device_create_file(&dev->mtd->dev, &dev_attr_media_type))
+ if (device_create_file(&dev->mtd->dev, &dev_attr_media_type)) {
message("can't create media type sysfs attribute");
+ goto error3;
+ }
dev->card_registred = 1;
return 0;
+error3:
+ nand_release(dev->mtd);
error2:
kfree(dev->mtd);
error1:
--
1.8.4.5
kasprintf() used in get_partition_name() does a dynamic
memory allocation and can fail. We have to handle that case.
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/nand/nandsim.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index f232427..52c0c1a 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -743,6 +743,11 @@ static int init_nandsim(struct mtd_info *mtd)
goto error;
}
ns->partitions[i].name = get_partition_name(i);
+ if (!ns->partitions[i].name) {
+ NS_ERR("unable to allocate memory.\n");
+ ret = -ENOMEM;
+ goto error;
+ }
ns->partitions[i].offset = next_offset;
ns->partitions[i].size = part_sz;
next_offset += ns->partitions[i].size;
@@ -756,6 +761,11 @@ static int init_nandsim(struct mtd_info *mtd)
goto error;
}
ns->partitions[i].name = get_partition_name(i);
+ if (!ns->partitions[i].name) {
+ NS_ERR("unable to allocate memory.\n");
+ ret = -ENOMEM;
+ goto error;
+ }
ns->partitions[i].offset = next_offset;
ns->partitions[i].size = remains;
ns->nbparts += 1;
--
1.8.4.5
kasprintf() does a dynamic memory allocation and can fail.
We have to handle that case.
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/nand/cs553x_nand.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/cs553x_nand.c b/drivers/mtd/nand/cs553x_nand.c
index 88109d3..aec6045 100644
--- a/drivers/mtd/nand/cs553x_nand.c
+++ b/drivers/mtd/nand/cs553x_nand.c
@@ -237,17 +237,23 @@ static int __init cs553x_init_one(int cs, int mmio, unsigned long adr)
/* Enable the following for a flash based bad block table */
this->bbt_options = NAND_BBT_USE_FLASH;
+ new_mtd->name = kasprintf(GFP_KERNEL, "cs553x_nand_cs%d", cs);
+ if (!new_mtd->name) {
+ err = -ENOMEM;
+ goto out_ior;
+ }
+
/* Scan to find existence of the device */
if (nand_scan(new_mtd, 1)) {
err = -ENXIO;
- goto out_ior;
+ goto out_free;
}
- new_mtd->name = kasprintf(GFP_KERNEL, "cs553x_nand_cs%d", cs);
-
cs553x_mtd[cs] = new_mtd;
goto out;
+out_free:
+ kfree(new_mtd->name);
out_ior:
iounmap(this->IO_ADDR_R);
out_mtd:
--
1.8.4.5
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/devices/docg3.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index be5fb2b..486936b 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -1900,7 +1900,7 @@ doc_probe_device(struct docg3_cascade *cascade, int floor, struct device *dev)
ret = 0;
if (chip_id != (u16)(~chip_id_inv)) {
- goto nomem3;
+ goto nomem4;
}
switch (chip_id) {
@@ -1910,7 +1910,7 @@ doc_probe_device(struct docg3_cascade *cascade, int floor, struct device *dev)
break;
default:
doc_err("Chip id %04x is not a DiskOnChip G3 chip\n", chip_id);
- goto nomem3;
+ goto nomem4;
}
doc_set_driver_info(chip_id, mtd);
@@ -1919,6 +1919,8 @@ doc_probe_device(struct docg3_cascade *cascade, int floor, struct device *dev)
doc_reload_bbt(docg3);
return mtd;
+nomem4:
+ kfree(docg3->bbt);
nomem3:
kfree(mtd);
nomem2:
--
1.8.4.5
kasprintf() does a dynamic memory allocation and can fail.
We have to handle that case.
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/devices/docg3.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 486936b..5e67b4a 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -1815,7 +1815,7 @@ static void doc_dbg_unregister(struct docg3 *docg3)
* @chip_id: The chip ID of the supported chip
* @mtd: The structure to fill
*/
-static void __init doc_set_driver_info(int chip_id, struct mtd_info *mtd)
+static int __init doc_set_driver_info(int chip_id, struct mtd_info *mtd)
{
struct docg3 *docg3 = mtd->priv;
int cfg;
@@ -1828,6 +1828,8 @@ static void __init doc_set_driver_info(int chip_id, struct mtd_info *mtd)
case DOC_CHIPID_G3:
mtd->name = kasprintf(GFP_KERNEL, "docg3.%d",
docg3->device_id);
+ if (!mtd->name)
+ return -ENOMEM;
docg3->max_block = 2047;
break;
}
@@ -1850,6 +1852,8 @@ static void __init doc_set_driver_info(int chip_id, struct mtd_info *mtd)
mtd->_block_isbad = doc_block_isbad;
mtd->ecclayout = &docg3_oobinfo;
mtd->ecc_strength = DOC_ECC_BCH_T;
+
+ return 0;
}
/**
@@ -1913,7 +1917,9 @@ doc_probe_device(struct docg3_cascade *cascade, int floor, struct device *dev)
goto nomem4;
}
- doc_set_driver_info(chip_id, mtd);
+ ret = doc_set_driver_info(chip_id, mtd);
+ if (ret)
+ goto nomem4;
doc_hamming_ecc_init(docg3, DOC_LAYOUT_OOB_PAGEINFO_SZ);
doc_reload_bbt(docg3);
--
1.8.4.5
Don't return a obfuscated null pointer using ERR_PTR(0).
If the no device is found clearly return -ENODEV.
This makes the code more clear and matches the comment
of doc_probe_device().
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/devices/docg3.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 5e67b4a..630e29a 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -1902,7 +1902,7 @@ doc_probe_device(struct docg3_cascade *cascade, int floor, struct device *dev)
chip_id = doc_register_readw(docg3, DOC_CHIPID);
chip_id_inv = doc_register_readw(docg3, DOC_CHIPID_INV);
- ret = 0;
+ ret = -ENODEV;
if (chip_id != (u16)(~chip_id_inv)) {
goto nomem4;
}
@@ -2068,13 +2068,10 @@ static int __init docg3_probe(struct platform_device *pdev)
mtd = doc_probe_device(cascade, floor, dev);
if (IS_ERR(mtd)) {
ret = PTR_ERR(mtd);
- goto err_probe;
- }
- if (!mtd) {
- if (floor == 0)
- goto notfound;
- else
+ if (ret == -ENODEV && floor == 0)
continue;
+ else
+ goto err_probe;
}
cascade->floors[floor] = mtd;
ret = mtd_device_parse_register(mtd, part_probes, NULL, NULL,
@@ -2091,10 +2088,9 @@ static int __init docg3_probe(struct platform_device *pdev)
doc_dbg_register(cascade->floors[0]->priv);
return 0;
-notfound:
- ret = -ENODEV;
- dev_info(dev, "No supported DiskOnChip found\n");
err_probe:
+ if (ret == -ENODEV)
+ dev_info(dev, "No supported DiskOnChip found\n");
free_bch(cascade->bch);
for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++)
if (cascade->floors[floor])
--
1.8.4.5
On Mon, Jun 01, 2015 at 11:10:50PM +0200, Richard Weinberger wrote:
> kasprintf() used in get_partition_name() does a dynamic
> memory allocation and can fail. We have to handle that case.
>
> Signed-off-by: Richard Weinberger <[email protected]>
> ---
> drivers/mtd/nand/nandsim.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
> index f232427..52c0c1a 100644
> --- a/drivers/mtd/nand/nandsim.c
> +++ b/drivers/mtd/nand/nandsim.c
> @@ -743,6 +743,11 @@ static int init_nandsim(struct mtd_info *mtd)
> goto error;
> }
> ns->partitions[i].name = get_partition_name(i);
> + if (!ns->partitions[i].name) {
> + NS_ERR("unable to allocate memory.\n");
Probably don't really need the allocation failure messages. But this
matches the current style, so we can just rip the messages out at
another time.
> + ret = -ENOMEM;
> + goto error;
> + }
> ns->partitions[i].offset = next_offset;
> ns->partitions[i].size = part_sz;
> next_offset += ns->partitions[i].size;
> @@ -756,6 +761,11 @@ static int init_nandsim(struct mtd_info *mtd)
> goto error;
> }
> ns->partitions[i].name = get_partition_name(i);
> + if (!ns->partitions[i].name) {
> + NS_ERR("unable to allocate memory.\n");
Same here.
> + ret = -ENOMEM;
> + goto error;
> + }
> ns->partitions[i].offset = next_offset;
> ns->partitions[i].size = remains;
> ns->nbparts += 1;
Brian
On Mon, Jun 01, 2015 at 11:10:49PM +0200, Richard Weinberger wrote:
> device_create_file() can fail, therefore we have to
> handle this case and abort.
>
> Signed-off-by: Richard Weinberger <[email protected]>
Pushed the first 5. Still looking at the 6th.
On Tue, 2015-06-16 at 19:07 -0700, Brian Norris wrote:
> On Mon, Jun 01, 2015 at 11:10:50PM +0200, Richard Weinberger wrote:
> > kasprintf() used in get_partition_name() does a dynamic
> > memory allocation and can fail. We have to handle that case.
[]
> > diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
[]
> > @@ -743,6 +743,11 @@ static int init_nandsim(struct mtd_info *mtd)
> > goto error;
> > }
> > ns->partitions[i].name = get_partition_name(i);
> > + if (!ns->partitions[i].name) {
> > + NS_ERR("unable to allocate memory.\n");
>
> Probably don't really need the allocation failure messages. But this
> matches the current style, so we can just rip the messages out at
> another time.
Maybe that other time can use the more typical
pr_<level> mechanisms instead of NS_<LEVEL> too.
As far as I can tell, the only thing that the
NS_<LEVEL> macros do is prefix "error: " and
"warning: " to the output.
"[nandsim] " could be added via pr_fmt and it
could be changed to "nandsim: " for commonality
with the majority of the kernel logging.
On Mon, Jun 01, 2015 at 11:10:54PM +0200, Richard Weinberger wrote:
> Don't return a obfuscated null pointer using ERR_PTR(0).
> If the no device is found clearly return -ENODEV.
> This makes the code more clear and matches the comment
> of doc_probe_device().
>
> Signed-off-by: Richard Weinberger <[email protected]>
> ---
Have you tested this patch?
> drivers/mtd/devices/docg3.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
> index 5e67b4a..630e29a 100644
> --- a/drivers/mtd/devices/docg3.c
> +++ b/drivers/mtd/devices/docg3.c
> @@ -1902,7 +1902,7 @@ doc_probe_device(struct docg3_cascade *cascade, int floor, struct device *dev)
> chip_id = doc_register_readw(docg3, DOC_CHIPID);
> chip_id_inv = doc_register_readw(docg3, DOC_CHIPID_INV);
>
> - ret = 0;
> + ret = -ENODEV;
> if (chip_id != (u16)(~chip_id_inv)) {
> goto nomem4;
> }
> @@ -2068,13 +2068,10 @@ static int __init docg3_probe(struct platform_device *pdev)
> mtd = doc_probe_device(cascade, floor, dev);
> if (IS_ERR(mtd)) {
> ret = PTR_ERR(mtd);
> - goto err_probe;
> - }
> - if (!mtd) {
> - if (floor == 0)
> - goto notfound;
> - else
> + if (ret == -ENODEV && floor == 0)
I think you might have changed the logic when refactoring here. I think
the right refactoring would be the following, no?
if (ret == -ENODEV && floor != 0)
continue;
else
goto err_probe;
> continue;
> + else
> + goto err_probe;
> }
> cascade->floors[floor] = mtd;
> ret = mtd_device_parse_register(mtd, part_probes, NULL, NULL,
> @@ -2091,10 +2088,9 @@ static int __init docg3_probe(struct platform_device *pdev)
> doc_dbg_register(cascade->floors[0]->priv);
> return 0;
>
> -notfound:
> - ret = -ENODEV;
> - dev_info(dev, "No supported DiskOnChip found\n");
> err_probe:
> + if (ret == -ENODEV)
> + dev_info(dev, "No supported DiskOnChip found\n");
> free_bch(cascade->bch);
> for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++)
> if (cascade->floors[floor])
Brian
Am 17.06.2015 um 20:41 schrieb Brian Norris:
> On Mon, Jun 01, 2015 at 11:10:54PM +0200, Richard Weinberger wrote:
>> Don't return a obfuscated null pointer using ERR_PTR(0).
>> If the no device is found clearly return -ENODEV.
>> This makes the code more clear and matches the comment
>> of doc_probe_device().
>>
>> Signed-off-by: Richard Weinberger <[email protected]>
>> ---
>
> Have you tested this patch?
nah, I don't own such a device.
>> drivers/mtd/devices/docg3.c | 16 ++++++----------
>> 1 file changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
>> index 5e67b4a..630e29a 100644
>> --- a/drivers/mtd/devices/docg3.c
>> +++ b/drivers/mtd/devices/docg3.c
>> @@ -1902,7 +1902,7 @@ doc_probe_device(struct docg3_cascade *cascade, int floor, struct device *dev)
>> chip_id = doc_register_readw(docg3, DOC_CHIPID);
>> chip_id_inv = doc_register_readw(docg3, DOC_CHIPID_INV);
>>
>> - ret = 0;
>> + ret = -ENODEV;
>> if (chip_id != (u16)(~chip_id_inv)) {
>> goto nomem4;
>> }
>> @@ -2068,13 +2068,10 @@ static int __init docg3_probe(struct platform_device *pdev)
>> mtd = doc_probe_device(cascade, floor, dev);
>> if (IS_ERR(mtd)) {
>> ret = PTR_ERR(mtd);
>> - goto err_probe;
>> - }
>> - if (!mtd) {
>> - if (floor == 0)
>> - goto notfound;
>> - else
>> + if (ret == -ENODEV && floor == 0)
>
> I think you might have changed the logic when refactoring here. I think
> the right refactoring would be the following, no?
>
> if (ret == -ENODEV && floor != 0)
> continue;
> else
> goto err_probe;
>
You are right, the logic changed. ;-\
Please drop this patch, I'll resend it for 4.3.
Thanks,
//richard
Richard Weinberger <[email protected]> writes:
> Am 17.06.2015 um 20:41 schrieb Brian Norris:
>> On Mon, Jun 01, 2015 at 11:10:54PM +0200, Richard Weinberger wrote:
>>> Don't return a obfuscated null pointer using ERR_PTR(0).
>>> If the no device is found clearly return -ENODEV.
>>> This makes the code more clear and matches the comment
>>> of doc_probe_device().
>>>
>>> Signed-off-by: Richard Weinberger <[email protected]>
>>> ---
>>
>> Have you tested this patch?
>
> nah, I don't own such a device.
But I do. If you resend a patch, please Cc me. You can even ask for a test from
time to time if you want a confirmation, I have a 2 floors docg3 device.
Cheers.
--
Robert
Hi Robert,
On Tue, Jun 23, 2015 at 10:41:33PM +0200, Robert Jarzmik wrote:
> Richard Weinberger <[email protected]> writes:
>
> > Am 17.06.2015 um 20:41 schrieb Brian Norris:
> >> Have you tested this patch?
> >
> > nah, I don't own such a device.
> But I do. If you resend a patch, please Cc me. You can even ask for a test from
> time to time if you want a confirmation, I have a 2 floors docg3 device.
Do you want to be on a MAINTAINERS entry for this driver?
Brian
Brian Norris <[email protected]> writes:
> Hi Robert,
>
> On Tue, Jun 23, 2015 at 10:41:33PM +0200, Robert Jarzmik wrote:
>> Richard Weinberger <[email protected]> writes:
>>
>> > Am 17.06.2015 um 20:41 schrieb Brian Norris:
>> >> Have you tested this patch?
>> >
>> > nah, I don't own such a device.
>> But I do. If you resend a patch, please Cc me. You can even ask for a test from
>> time to time if you want a confirmation, I have a 2 floors docg3 device.
>
> Do you want to be on a MAINTAINERS entry for this driver?
Sure.
Here is the patch, after the scissors.
Cheers.
--
Robert
---8<---
>From 20002639eac1bd7a81b0613c4bd15ae7522c269d Mon Sep 17 00:00:00 2001
From: Robert Jarzmik <[email protected]>
Date: Thu, 25 Jun 2015 19:07:48 +0200
Subject: [PATCH] MAINTAINERS: mtd: docg3: add docg3 maintainer
Add myself as maintainer of the NAND based MSystems DiskOnChip G3
driver.
Signed-off-by: Robert Jarzmik <[email protected]>
---
MAINTAINERS | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index aca2886..87d87c7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6687,6 +6687,12 @@ T: git git://linuxtv.org/anttip/media_tree.git
S: Maintained
F: drivers/media/usb/msi2500/
+MSYSTEMS DISKONCHIP G3 MTD DRIVER
+M: Robert Jarzmik <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/mtd/devices/docg3*
+
MT9M032 APTINA SENSOR DRIVER
M: Laurent Pinchart <[email protected]>
L: [email protected]
--
2.1.4
On Thu, Jun 25, 2015 at 07:14:18PM +0200, Robert Jarzmik wrote:
> From 20002639eac1bd7a81b0613c4bd15ae7522c269d Mon Sep 17 00:00:00 2001
> From: Robert Jarzmik <[email protected]>
> Date: Thu, 25 Jun 2015 19:07:48 +0200
> Subject: [PATCH] MAINTAINERS: mtd: docg3: add docg3 maintainer
>
> Add myself as maintainer of the NAND based MSystems DiskOnChip G3
> driver.
>
> Signed-off-by: Robert Jarzmik <[email protected]>
Queued, thanks