Re: [PATCH v3.4-rc3] MTD: NAND: ams-delta: Fix request_mem_region() failure
Hi, * Janusz Krzysztofik jkrzy...@tis.icnet.pl [120430 11:15]: Dnia czwartek, 26 kwietnia 2012 08:20:59 Artem Bityutskiy pisze: On Wed, 2012-04-25 at 19:01 +0200, Janusz Krzysztofik wrote: Both drivers use separate subsets of registers that belong to the OMAP1 MPU I/O device, but are used for controlling different sets of I/O pins. The NAND driver reads/writes the folowing registers: - OMAP_MPUIO_INPUT_LATCH, - OMAP_MPUIO_OUTPUT, - OMAP_MPUIO_IO_CNTL, while the keypad driver - the following: - OMAP_MPUIO_KBR_LATCH, - OMAP_MPUIO_KBC, - OMAP_MPUIO_KBD_MASKIT - OMAP_MPUIO_GPIO_DEBOUNCING. Both subsets are non-overlapping, and we rely on the drivers being free of bugs and doing their job correctly, not stepping on each others' feet, I guess. First of all, I think this information should be in the commit message. Also, some sort of comment in the driver code would be nice. If locking the memory region is too coarse approach, the should have a small separate omap-specific MPUIO subsystem which will be used by drivers to access MPUIO? Another question - should request_mem_region() be also removed from the omap-gpio driver then? I think it is more sensible to put a comment there that it is sharing MPIO with other drivers, instead of having an illusion of exclusive memory region ownership. But this is up to the OMAP community - I can take this patch to my l2-mtd tree if you get an ack from Tony or other OMAP guys. Tony, Would I get your Ack for this fix if I extend the commit message as Artem suggested? If not, what do you think should be a correct way to fix the regression? Well how about adding some exported functions to drivers/gpio/gpio_omap.c like omap_mpuio_latch? For the regression fix, if you guys want to do what Janusz is suggesting, then assuming the patch description also contains some decent long term plan to properly fix it: Acked-by: Tony Lindgren t...@atomide.com -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3.4-rc3] MTD: NAND: ams-delta: Fix request_mem_region() failure
On Fri, 4 May 2012 10:11:25 Tony Lindgren wrote: Hi, * Janusz Krzysztofik jkrzy...@tis.icnet.pl [120430 11:15]: Dnia czwartek, 26 kwietnia 2012 08:20:59 Artem Bityutskiy pisze: On Wed, 2012-04-25 at 19:01 +0200, Janusz Krzysztofik wrote: Both drivers use separate subsets of registers that belong to the OMAP1 MPU I/O device, but are used for controlling different sets of I/O pins. The NAND driver reads/writes the folowing registers: - OMAP_MPUIO_INPUT_LATCH, - OMAP_MPUIO_OUTPUT, - OMAP_MPUIO_IO_CNTL, while the keypad driver - the following: - OMAP_MPUIO_KBR_LATCH, - OMAP_MPUIO_KBC, - OMAP_MPUIO_KBD_MASKIT - OMAP_MPUIO_GPIO_DEBOUNCING. Both subsets are non-overlapping, and we rely on the drivers being free of bugs and doing their job correctly, not stepping on each others' feet, I guess. First of all, I think this information should be in the commit message. Also, some sort of comment in the driver code would be nice. If locking the memory region is too coarse approach, the should have a small separate omap-specific MPUIO subsystem which will be used by drivers to access MPUIO? Another question - should request_mem_region() be also removed from the omap-gpio driver then? I think it is more sensible to put a comment there that it is sharing MPIO with other drivers, instead of having an illusion of exclusive memory region ownership. But this is up to the OMAP community - I can take this patch to my l2-mtd tree if you get an ack from Tony or other OMAP guys. Tony, Would I get your Ack for this fix if I extend the commit message as Artem suggested? If not, what do you think should be a correct way to fix the regression? Well how about adding some exported functions to drivers/gpio/gpio_omap.c like omap_mpuio_latch? For the regression fix, if you guys want to do what Janusz is suggesting, then assuming the patch description also contains some decent long term plan to properly fix it: Acked-by: Tony Lindgren t...@atomide.com Thanks. Now that we know you prefer to keep the memory requested from inside the gpio-omap, which I was about to suggest to drop back as an alternative fix, I'll try to cook a new description for my patch, and perhaps add a comment replacing the request_mem_region function removed from the ams-delta NAND driver, in order to satisfy both yours and Artem's comments in a few days. Regards, Janusz -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3.4-rc3] MTD: NAND: ams-delta: Fix request_mem_region() failure
Dnia czwartek, 26 kwietnia 2012 08:20:59 Artem Bityutskiy pisze: On Wed, 2012-04-25 at 19:01 +0200, Janusz Krzysztofik wrote: Both drivers use separate subsets of registers that belong to the OMAP1 MPU I/O device, but are used for controlling different sets of I/O pins. The NAND driver reads/writes the folowing registers: - OMAP_MPUIO_INPUT_LATCH, - OMAP_MPUIO_OUTPUT, - OMAP_MPUIO_IO_CNTL, while the keypad driver - the following: - OMAP_MPUIO_KBR_LATCH, - OMAP_MPUIO_KBC, - OMAP_MPUIO_KBD_MASKIT - OMAP_MPUIO_GPIO_DEBOUNCING. Both subsets are non-overlapping, and we rely on the drivers being free of bugs and doing their job correctly, not stepping on each others' feet, I guess. First of all, I think this information should be in the commit message. Also, some sort of comment in the driver code would be nice. If locking the memory region is too coarse approach, the should have a small separate omap-specific MPUIO subsystem which will be used by drivers to access MPUIO? Another question - should request_mem_region() be also removed from the omap-gpio driver then? I think it is more sensible to put a comment there that it is sharing MPIO with other drivers, instead of having an illusion of exclusive memory region ownership. But this is up to the OMAP community - I can take this patch to my l2-mtd tree if you get an ack from Tony or other OMAP guys. Tony, Would I get your Ack for this fix if I extend the commit message as Artem suggested? If not, what do you think should be a correct way to fix the regression? Thanks, Janusz -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3.4-rc3] MTD: NAND: ams-delta: Fix request_mem_region() failure
On Tue, 2012-04-17 at 15:49 +0200, Janusz Krzysztofik wrote: A call to request_mem_region() has been introduced in the omap-gpio driver recently (commit 96751fcbe5438e95514b025e9cee7a6d38038f40, gpio/omap: Use devm_ API and add request_mem_region). This change prevented the Amstrad Delta NAND driver, which was doing the same in order to take control over OMAP MPU I/O lines that the NAND device hangs off, from loading successfully. There is another driver, omap-keypad, which also manipulates OMAP MPUIO registers, but has never been calling request_mem_region() on startup, so it's not affected by the change in the gpio-omap and works correctly. Drop request_mem_region() call and related bits from ams-delta NAND driver. Created and tested against linux-3.4-rc3. Signed-off-by: Janusz Krzysztofik jkrzy...@tis.icnet.pl How about race conditions? Where is the guarantee that these 2 drivers won't affect each other when doing I/O at the same time to the same HW resources? -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part
Re: [PATCH v3.4-rc3] MTD: NAND: ams-delta: Fix request_mem_region() failure
Dnia środa, 25 kwietnia 2012 18:13:38 Artem Bityutskiy pisze: On Tue, 2012-04-17 at 15:49 +0200, Janusz Krzysztofik wrote: A call to request_mem_region() has been introduced in the omap-gpio driver recently (commit 96751fcbe5438e95514b025e9cee7a6d38038f40, gpio/omap: Use devm_ API and add request_mem_region). This change prevented the Amstrad Delta NAND driver, which was doing the same in order to take control over OMAP MPU I/O lines that the NAND device hangs off, from loading successfully. There is another driver, omap-keypad, which also manipulates OMAP MPUIO registers, but has never been calling request_mem_region() on startup, so it's not affected by the change in the gpio-omap and works correctly. Drop request_mem_region() call and related bits from ams-delta NAND driver. Created and tested against linux-3.4-rc3. Signed-off-by: Janusz Krzysztofik jkrzy...@tis.icnet.pl How about race conditions? Where is the guarantee that these 2 drivers won't affect each other when doing I/O at the same time to the same HW resources? Both drivers use separate subsets of registers that belong to the OMAP1 MPU I/O device, but are used for controlling different sets of I/O pins. The NAND driver reads/writes the folowing registers: - OMAP_MPUIO_INPUT_LATCH, - OMAP_MPUIO_OUTPUT, - OMAP_MPUIO_IO_CNTL, while the keypad driver - the following: - OMAP_MPUIO_KBR_LATCH, - OMAP_MPUIO_KBC, - OMAP_MPUIO_KBD_MASKIT - OMAP_MPUIO_GPIO_DEBOUNCING. Both subsets are non-overlapping, and we rely on the drivers being free of bugs and doing their job correctly, not stepping on each others' feet, I guess. Thanks, Janusz -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3.4-rc3] MTD: NAND: ams-delta: Fix request_mem_region() failure
On Wed, 2012-04-25 at 19:01 +0200, Janusz Krzysztofik wrote: Both drivers use separate subsets of registers that belong to the OMAP1 MPU I/O device, but are used for controlling different sets of I/O pins. The NAND driver reads/writes the folowing registers: - OMAP_MPUIO_INPUT_LATCH, - OMAP_MPUIO_OUTPUT, - OMAP_MPUIO_IO_CNTL, while the keypad driver - the following: - OMAP_MPUIO_KBR_LATCH, - OMAP_MPUIO_KBC, - OMAP_MPUIO_KBD_MASKIT - OMAP_MPUIO_GPIO_DEBOUNCING. Both subsets are non-overlapping, and we rely on the drivers being free of bugs and doing their job correctly, not stepping on each others' feet, I guess. First of all, I think this information should be in the commit message. Also, some sort of comment in the driver code would be nice. If locking the memory region is too coarse approach, the should have a small separate omap-specific MPUIO subsystem which will be used by drivers to access MPUIO? Another question - should request_mem_region() be also removed from the omap-gpio driver then? I think it is more sensible to put a comment there that it is sharing MPIO with other drivers, instead of having an illusion of exclusive memory region ownership. But this is up to the OMAP community - I can take this patch to my l2-mtd tree if you get an ack from Tony or other OMAP guys. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part
[PATCH v3.4-rc3] MTD: NAND: ams-delta: Fix request_mem_region() failure
A call to request_mem_region() has been introduced in the omap-gpio
driver recently (commit 96751fcbe5438e95514b025e9cee7a6d38038f40,
gpio/omap: Use devm_ API and add request_mem_region). This change
prevented the Amstrad Delta NAND driver, which was doing the same in
order to take control over OMAP MPU I/O lines that the NAND device hangs
off, from loading successfully.
There is another driver, omap-keypad, which also manipulates OMAP MPUIO
registers, but has never been calling request_mem_region() on startup,
so it's not affected by the change in the gpio-omap and works correctly.
Drop request_mem_region() call and related bits from ams-delta NAND
driver.
Created and tested against linux-3.4-rc3.
Signed-off-by: Janusz Krzysztofik jkrzy...@tis.icnet.pl
---
drivers/mtd/nand/ams-delta.c | 12 +---
1 files changed, 1 insertions(+), 11 deletions(-)
diff --git a/drivers/mtd/nand/ams-delta.c b/drivers/mtd/nand/ams-delta.c
index 7341695..af76da3 100644
--- a/drivers/mtd/nand/ams-delta.c
+++ b/drivers/mtd/nand/ams-delta.c
@@ -212,18 +212,11 @@ static int __devinit ams_delta_init(struct
platform_device *pdev)
/* Link the private data with the MTD structure */
ams_delta_mtd-priv = this;
- if (!request_mem_region(res-start, resource_size(res),
- dev_name(pdev-dev))) {
- dev_err(pdev-dev, request_mem_region failed\n);
- err = -EBUSY;
- goto out_free;
- }
-
io_base = ioremap(res-start, resource_size(res));
if (io_base == NULL) {
dev_err(pdev-dev, ioremap failed\n);
err = -EIO;
- goto out_release_io;
+ goto out_free;
}
this-priv = io_base;
@@ -271,8 +264,6 @@ out_gpio:
platform_set_drvdata(pdev, NULL);
gpio_free(AMS_DELTA_GPIO_PIN_NAND_RB);
iounmap(io_base);
-out_release_io:
- release_mem_region(res-start, resource_size(res));
out_free:
kfree(ams_delta_mtd);
out:
@@ -293,7 +284,6 @@ static int __devexit ams_delta_cleanup(struct
platform_device *pdev)
gpio_free_array(_mandatory_gpio, ARRAY_SIZE(_mandatory_gpio));
gpio_free(AMS_DELTA_GPIO_PIN_NAND_RB);
iounmap(io_base);
- release_mem_region(res-start, resource_size(res));
/* Free the MTD device structure */
kfree(ams_delta_mtd);
--
1.7.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
