RE: [PATCH v4 3/3] ARM: OMAP2+: onenand: prepare for gpmc driver migration
Hi Jon, On Tue, Jun 26, 2012 at 20:26:40, Hunter, Jon wrote: On 06/26/2012 09:39 AM, Jon Hunter wrote: a single global variable called onenand_flags for storing the current state of sync_read, sync_write, vhf and hf. At least this would be only one global instead of 4. Not a big deal. V5 has been posted with above changes Reviewed-by: Jon Hunter jon-hun...@ti.com Thanks Afzal -- 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 v4 3/3] ARM: OMAP2+: onenand: prepare for gpmc driver migration
Hi Jon, On Mon, Jun 25, 2012 at 21:42:14, Hunter, Jon wrote: On 06/22/2012 04:01 AM, Afzal Mohammed wrote: +static int hf, vhf, sync_read, sync_write, latency; I am wondering if we can remove hf, vhf, sync_read/write variables completely. We already have flags from sync_read/write and so we could just use the cfg-flags variable and remove sync_read/write variables. For default frequency, sync_write can get turned off, so flag may or may not be same as sync_write At the same time, we could create flags for ONENAND_FREQ_HF and ONENAND_FREQ_VHF or something like that. It could be nice to store the latency in onenand_data too. In other words, keep all the configuration in one place. I have a feeling as though platform data fields should not be altered once platform device is registered (as platform data being specific to the board, thinking further, should they be const?, except for a case where it is created by a common helper function for multiple boards with varying capabilities of peripheral). Other than sync_read, all others like hf, vhf, latency, sync_write are updated during driver callback, so if we are going to put these in platform private data fields, platform private data fields has to be updated after platform device is registered. Regards Afzal -- 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 v4 3/3] ARM: OMAP2+: onenand: prepare for gpmc driver migration
Hi Afzal, On 06/26/2012 03:29 AM, Mohammed, Afzal wrote: Hi Jon, On Mon, Jun 25, 2012 at 21:42:14, Hunter, Jon wrote: On 06/22/2012 04:01 AM, Afzal Mohammed wrote: +static int hf, vhf, sync_read, sync_write, latency; I am wondering if we can remove hf, vhf, sync_read/write variables completely. We already have flags from sync_read/write and so we could just use the cfg-flags variable and remove sync_read/write variables. For default frequency, sync_write can get turned off, so flag may or may not be same as sync_write Good point. I missed that. At the same time, we could create flags for ONENAND_FREQ_HF and ONENAND_FREQ_VHF or something like that. It could be nice to store the latency in onenand_data too. In other words, keep all the configuration in one place. I have a feeling as though platform data fields should not be altered once platform device is registered (as platform data being specific to the board, thinking further, should they be const?, except for a case where it is created by a common helper function for multiple boards with varying capabilities of peripheral). Other than sync_read, all others like hf, vhf, latency, sync_write are updated during driver callback, so if we are going to put these in platform private data fields, platform private data fields has to be updated after platform device is registered. May be this is splitting hairs then but I wonder if we should just have a single global variable called onenand_flags for storing the current state of sync_read, sync_write, vhf and hf. At least this would be only one global instead of 4. Not a big deal. Cheers Jon -- 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 v4 3/3] ARM: OMAP2+: onenand: prepare for gpmc driver migration
Hi Afzal, On 06/26/2012 09:39 AM, Jon Hunter wrote: Hi Afzal, On 06/26/2012 03:29 AM, Mohammed, Afzal wrote: Hi Jon, On Mon, Jun 25, 2012 at 21:42:14, Hunter, Jon wrote: On 06/22/2012 04:01 AM, Afzal Mohammed wrote: +static int hf, vhf, sync_read, sync_write, latency; I am wondering if we can remove hf, vhf, sync_read/write variables completely. We already have flags from sync_read/write and so we could just use the cfg-flags variable and remove sync_read/write variables. For default frequency, sync_write can get turned off, so flag may or may not be same as sync_write Good point. I missed that. At the same time, we could create flags for ONENAND_FREQ_HF and ONENAND_FREQ_VHF or something like that. It could be nice to store the latency in onenand_data too. In other words, keep all the configuration in one place. I have a feeling as though platform data fields should not be altered once platform device is registered (as platform data being specific to the board, thinking further, should they be const?, except for a case where it is created by a common helper function for multiple boards with varying capabilities of peripheral). Other than sync_read, all others like hf, vhf, latency, sync_write are updated during driver callback, so if we are going to put these in platform private data fields, platform private data fields has to be updated after platform device is registered. May be this is splitting hairs then but I wonder if we should just have a single global variable called onenand_flags for storing the current state of sync_read, sync_write, vhf and hf. At least this would be only one global instead of 4. Not a big deal. Apart from the above minor nit-pick, please add ... Reviewed-by: Jon Hunter jon-hun...@ti.com Cheers Jon -- 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 v4 3/3] ARM: OMAP2+: onenand: prepare for gpmc driver migration
Hi Afzal, Looks much better! On 06/22/2012 04:01 AM, Afzal Mohammed wrote: Reorganize gpmc-onenand initialization so that changes required for gpmc driver migration can be made smooth. Ensuring sync read/write are disabled in onenand cannot be expected to work properly unless GPMC is setup, this has been removed. Refactor set_async_mode set_sync_mode functions to separate out timing calculation actual configuration (GPMC OneNAND side). Thanks to Jon for his suggestions. Signed-off-by: Afzal Mohammed af...@ti.com --- v4: Reorganize set_sync/async functions in a better way v3: Refactor set_sync/async functions to separate out timing and configurations v2: Move ensuring that async mode in OneNAND has been setup from set_sync to setup function, improve commit message arch/arm/mach-omap2/gpmc-onenand.c | 153 +++- 1 file changed, 83 insertions(+), 70 deletions(-) diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c index 8863e0a..878182b 100644 --- a/arch/arm/mach-omap2/gpmc-onenand.c +++ b/arch/arm/mach-omap2/gpmc-onenand.c @@ -15,6 +15,7 @@ #include linux/platform_device.h #include linux/mtd/onenand_regs.h #include linux/io.h +#include linux/err.h #include asm/mach/flash.h @@ -25,6 +26,7 @@ #define ONENAND_IO_SIZE SZ_128K +static int hf, vhf, sync_read, sync_write, latency; I am wondering if we can remove hf, vhf, sync_read/write variables completely. We already have flags from sync_read/write and so we could just use the cfg-flags variable and remove sync_read/write variables. At the same time, we could create flags for ONENAND_FREQ_HF and ONENAND_FREQ_VHF or something like that. It could be nice to store the latency in onenand_data too. In other words, keep all the configuration in one place. Otherwise looks good. Cheers Jon -- 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
