RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD
Hi Jon, On Wed, Jun 13, 2012 at 20:21:50, Hunter, Jon wrote: I do not think it is practically possible. Please see timing calculations in arch/arm/mach-omap2/gpmc-*, the way it is done for different peripherals are different, and we cannot expect gpmc driver to do those as that would require gpmc driver being aware of type of peripheral connected. And all those gpmc-* timing calculation needs to be done before driver is ready, they rely on functions like gpmc_get_fclk_rate(), which in turn requires the clk rate to be available before driver is probed. So I see that the various gpmc-*.c files have some form of _retime() function. However, at the end of the day they all call gpmc_cs_set_timings() to convert time into gpmc clocks. Converting time to gpmc clocks is completely independent of the actual device and so this can be performed by the driver. We just need to populate the gpmc_timings struct and pass to the driver to convert to clocks and program into the registers. gpmc_cs_set_timings() does currently convert time to clock cycles required, and this gpmc driver have the capability to do it. What I was saying is a different issue, input to gpmc_cs_set_timings, which is time sometimes in turn is a function of time or to be exact depends on gpmc clock period also. So timings provided to gpmc_cs_set_timings for a particular frequency may not hold good for another frequency, unless we change the input time to gpmc_cs_set_timings based on gpmc clock. If you see gpmc-* files, many a times, they need to know value of gpmc fclk, to calculate the input time to be fed for gpmc_cs_set_timings 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 v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD
Hi Jon, On Thu, Jun 14, 2012 at 11:47:03, Mohammed, Afzal wrote: gpmc_cs_set_timings() does currently convert time to clock cycles required, and this gpmc driver have the capability to do it. What I was saying is a different issue, input to gpmc_cs_set_timings, which is time sometimes in turn is a function of time or to be exact depends on gpmc clock period also. So timings provided to gpmc_cs_set_timings for a particular frequency may not hold good for another frequency, unless we change the input time to gpmc_cs_set_timings based on gpmc clock. If you see gpmc-* files, many a times, they need to know value of gpmc fclk, to calculate the input time to be fed for gpmc_cs_set_timings And the way value of gpmc period has to be handled differs depending on the peripheral. 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 V2 5/5] Input: ads7846: set proper debounce time in driver level
On Thu, Jun 14, 2012 at 10:16:55, Zumeng Chen wrote: 于 2012年06月13日 20:18, Hiremath, Vaibhav 写道: On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote: From: Zumeng Chenzumeng.c...@windriver.com If we don't set proper debouce time for ads7846, then there are flooded interrupt counters of ads7846 responding to one time touch on screen, so the driver couldn't work well. And since most OMAP3 series boards pass NULL pointer of board_pdata to omap_ads7846_init, so it's more proper to set it in driver level after having gpio_request done. This patch has been validated on 3530evm. Signed-off-by: Zumeng Chenzumeng.c...@windriver.com Signed-off-by: Syed Mohammed Khasimkha...@ti.com --- drivers/input/touchscreen/ads7846.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c index f02028e..459ff29 100644 --- a/drivers/input/touchscreen/ads7846.c +++ b/drivers/input/touchscreen/ads7846.c @@ -980,6 +980,10 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784 } ts-gpio_pendown = pdata-gpio_pendown; +#ifdef CONFIG_ARCH_OMAP3 + /* 310 means about 10 microsecond for omap3 */ + gpio_set_debounce(pdata-gpio_pendown, 310); +#endif Zumeng, With my sign-off you are changing the original code, that too without my sign-off and ack. I wouldn't mind you to submit patches from my tree, but the expectation is if you are changing the original code, it should be under your sign-off. Thanks, good to learn. I'll remove in next time. Coming to the patch, #ifdef in driver is not recommended, and this 10msec delay is specific to OMAP GPIO and driver should not be aware of this, that's where you will find the original patch handling it in board file. According to the git blame of the board-omap3evm.c I think 96974a24 did a good thing to all the related codes for omap3 boards. So I think we can call board and driver as BSP level :-) If #ifdef in driver can save many codes, I guess it's deserved. No, not really. In the same commit, the debounce time is already handled as an argument to the function omap_ads7846_init(), and that’s the way it should be. So no need for #ifdefs in driver... Thanks, Vaibhav -- 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 v5 03/14] ARM: OMAP2+: gpmc: driver migration helper
* Mohammed, Afzal af...@ti.com [120613 06:50]: Hi Tony, On Wed, Jun 13, 2012 at 17:48:15, Mohammed, Afzal wrote: On Wed, Jun 13, 2012 at 17:34:58, Tony Lindgren wrote: There should no longer be any need to initialize GPMC early. It should behave like any other device driver, and also work as a loadable module. Once driver migration is complete for all the boards, we can remove the initcall and make it loadable. But till that time as we need to support both old new interface, I feel this is necessary as old interface would happen in arch initcall, and old gpmc_init is now gpmc_probe, and hence it needs to get executed before arch initcall, necessitating this. Seems you missed this in the flood of mails, please let me know your comments on this. This is a commit that has to be reverted once driver migration is complete for all boards. Yes sounds good to me. Tony -- 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 v5 04/14] ARM: OMAP2+: gpmc: minimal driver support
* Mohammed, Afzal af...@ti.com [120613 06:56]: Hi Tony, On Wed, Jun 13, 2012 at 19:20:35, Mohammed, Afzal wrote: Hi Tony, On Wed, Jun 13, 2012 at 19:14:08, Tony Lindgren wrote: Actually why do you even need to store the revision? You can just read it from the hardware as needed. Earlier for wr_access wr_data_mux_bus, we were checking, cpu_is_34xx, but I feel it should instead be based on IP revision as it is the IP revision that decides the availability of those register bits. Or you meant, even there read revision register ? Yeah that should be fine as this really should only happen during init and whatever revision specific features can be initialized for GPMC. Tony -- 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 v5 04/14] ARM: OMAP2+: gpmc: minimal driver support
Hi Tony, On Thu, Jun 14, 2012 at 12:05:25, Tony Lindgren wrote: * Mohammed, Afzal af...@ti.com [120613 06:56]: Or you meant, even there read revision register ? Yeah that should be fine as this really should only happen during init and whatever revision specific features can be initialized for GPMC. Sorry, I got confused, we need revision to be available while setting time also, so you meant to store it as a variable or read revision at probe as well as during setting time ? 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 V2 5/5] Input: ads7846: set proper debounce time in driver level
On 06/14/12 07:46, Zumeng Chen wrote: 于 2012年06月13日 20:18, Hiremath, Vaibhav 写道: On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote: From: Zumeng Chenzumeng.c...@windriver.com If we don't set proper debouce time for ads7846, then there are flooded interrupt counters of ads7846 responding to one time touch on screen, so the driver couldn't work well. And since most OMAP3 series boards pass NULL pointer of board_pdata to omap_ads7846_init, so it's more proper to set it in driver level after having gpio_request done. This patch has been validated on 3530evm. Signed-off-by: Zumeng Chenzumeng.c...@windriver.com Signed-off-by: Syed Mohammed Khasimkha...@ti.com --- drivers/input/touchscreen/ads7846.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c index f02028e..459ff29 100644 --- a/drivers/input/touchscreen/ads7846.c +++ b/drivers/input/touchscreen/ads7846.c @@ -980,6 +980,10 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784 } ts-gpio_pendown = pdata-gpio_pendown; +#ifdef CONFIG_ARCH_OMAP3 +/* 310 means about 10 microsecond for omap3 */ +gpio_set_debounce(pdata-gpio_pendown, 310); +#endif Zumeng, With my sign-off you are changing the original code, that too without my sign-off and ack. I wouldn't mind you to submit patches from my tree, but the expectation is if you are changing the original code, it should be under your sign-off. Thanks, good to learn. I'll remove in next time. Coming to the patch, #ifdef in driver is not recommended, and this 10msec delay is specific to OMAP GPIO and driver should not be aware of this, that's where you will find the original patch handling it in board file. According to the git blame of the board-omap3evm.c I think 96974a24 did a good thing to all the related codes for omap3 boards. So I think we can call board and driver as BSP level :-) If #ifdef in driver can save many codes, I guess it's deserved. No, platform/board specific ifdefs in the generic driver code are never deserved. How about the attached patch, does it fix the problem for you? -- Regards, Igor. diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c index 1706ebc..c187586 100644 --- a/arch/arm/mach-omap2/common-board-devices.c +++ b/arch/arm/mach-omap2/common-board-devices.c @@ -63,28 +63,30 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce, struct spi_board_info *spi_bi = ads7846_spi_board_info; int err; - if (board_pdata board_pdata-get_pendown_state) { - err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown); - if (err) { - pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err); - return; - } - gpio_export(gpio_pendown, 0); - - if (gpio_debounce) - gpio_set_debounce(gpio_pendown, gpio_debounce); + err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown); + if (err) { + pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err); + return; } + if (gpio_debounce) + gpio_set_debounce(gpio_pendown, gpio_debounce); + spi_bi-bus_num = bus_num; spi_bi-irq = gpio_to_irq(gpio_pendown); if (board_pdata) { board_pdata-gpio_pendown = gpio_pendown; spi_bi-platform_data = board_pdata; + if (board_pdata-get_pendown_state) + gpio_export(gpio_pendown, 0); } else { ads7846_config.gpio_pendown = gpio_pendown; } + if (!board_pdata || (board_pdata !board_pdata-get_pendown_state)) + gpio_free(gpio_pendown); + spi_register_board_info(ads7846_spi_board_info, 1); } #else
Re: [PATCH] OMAP: Beagle: fix TFP410 powerdown GPIO init
On Wed, Jun 13, 2012 at 2:20 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: Commit e813a55eb9c9bc6c8039fb16332cf43402125b30 (OMAP: board-files: remove custom PD GPIO handling for DVI output) moved TFP410 chip's powerdown-gpio handling from the board files to the tfp410 driver. One gpio_request_one(powerdown-gpio, ...) was mistakenly left unremoved in the Beagle board file. This causes the tfp410 driver to fail to request the gpio on Beagle, causing the driver to fail and thus the DVI output doesn't work. Can you take the one I sent earlier instead? http://www.spinics.net/lists/linux-omap/msg69913.html -- 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 V2 5/5] Input: ads7846: set proper debounce time in driver level
于 2012年06月14日 14:31, Hiremath, Vaibhav 写道: On Thu, Jun 14, 2012 at 10:16:55, Zumeng Chen wrote: 于 2012年06月13日 20:18, Hiremath, Vaibhav 写道: On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote: From: Zumeng Chenzumeng.c...@windriver.com If we don't set proper debouce time for ads7846, then there are flooded interrupt counters of ads7846 responding to one time touch on screen, so the driver couldn't work well. And since most OMAP3 series boards pass NULL pointer of board_pdata to omap_ads7846_init, so it's more proper to set it in driver level after having gpio_request done. This patch has been validated on 3530evm. Signed-off-by: Zumeng Chenzumeng.c...@windriver.com Signed-off-by: Syed Mohammed Khasimkha...@ti.com --- drivers/input/touchscreen/ads7846.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c index f02028e..459ff29 100644 --- a/drivers/input/touchscreen/ads7846.c +++ b/drivers/input/touchscreen/ads7846.c @@ -980,6 +980,10 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784 } ts-gpio_pendown = pdata-gpio_pendown; +#ifdef CONFIG_ARCH_OMAP3 + /* 310 means about 10 microsecond for omap3 */ + gpio_set_debounce(pdata-gpio_pendown, 310); +#endif Zumeng, With my sign-off you are changing the original code, that too without my sign-off and ack. I wouldn't mind you to submit patches from my tree, but the expectation is if you are changing the original code, it should be under your sign-off. Thanks, good to learn. I'll remove in next time. Coming to the patch, #ifdef in driver is not recommended, and this 10msec delay is specific to OMAP GPIO and driver should not be aware of this, that's where you will find the original patch handling it in board file. According to the git blame of the board-omap3evm.c I think 96974a24 did a good thing to all the related codes for omap3 boards. So I think we can call board and driver as BSP level :-) If #ifdef in driver can save many codes, I guess it's deserved. No, not really. In the same commit, the debounce time is already handled as an argument to the function omap_ads7846_init(), and that’s the way it should be. That means you'd like to implement the same get_pendown_state for every omap3 board? Currently, board_pdata is NULL. And actually, the reason why I agree 96974a24 is that get_pendown_state for all omap3 boards is the common chip level gpio operations. so I think we should set debounce for them in one common point. But since Igor and you don't like them, I have created and tested the attachment patch, and I'd like Mike to check if convenience too But obviously there are more codes than mine before :-) Regards, Zumeng So no need for #ifdefs in driver... Thanks, Vaibhav diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c index 56cce1e..887bd35 100644 --- a/arch/arm/mach-omap2/board-omap3evm.c +++ b/arch/arm/mach-omap2/board-omap3evm.c @@ -58,7 +58,6 @@ #include hsmmc.h #include common-board-devices.h -#define OMAP3_EVM_TS_GPIO 175 #define OMAP3_EVM_EHCI_VBUS 22 #define OMAP3_EVM_EHCI_SELECT 61 diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c index 719f62e..8cf0140 100644 --- a/arch/arm/mach-omap2/common-board-devices.c +++ b/arch/arm/mach-omap2/common-board-devices.c @@ -34,6 +34,11 @@ static struct omap2_mcspi_device_config ads7846_mcspi_config = { .turbo_mode = 0, }; + +static int omap3_get_pendown_state(void) +{ + return !gpio_get_value(OMAP3_EVM_TS_GPIO); +} static struct ads7846_platform_data ads7846_config = { .x_max = 0x0fff, @@ -45,6 +50,7 @@ static struct ads7846_platform_data ads7846_config = { .debounce_rep = 1, .gpio_pendown = -EINVAL, .keep_vref_on = 1, + .get_pendown_state = omap3_get_pendown_state, }; static struct spi_board_info ads7846_spi_board_info __initdata = { @@ -63,17 +69,14 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce, struct spi_board_info *spi_bi = ads7846_spi_board_info; int err; - if (board_pdata board_pdata-get_pendown_state) { - err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown); - if (err) { - pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err); - return; - } - gpio_export(gpio_pendown, 0); - - if (gpio_debounce) - gpio_set_debounce(gpio_pendown, gpio_debounce); + err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown); + if (err) { + pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err); + return; } + gpio_export(gpio_pendown, 0); + if (gpio_debounce) + gpio_set_debounce(gpio_pendown, gpio_debounce); spi_bi-bus_num = bus_num; spi_bi-irq = gpio_to_irq(gpio_pendown); diff --git a/arch/arm/mach-omap2/common-board-devices.h b/arch/arm/mach-omap2/common-board-devices.h index a0b4a428..4c4ef6a 100644 ---
RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD
Hi Jon, On Wed, Jun 13, 2012 at 20:21:50, Hunter, Jon wrote: If the clk handle for the gpmc is passed to the gpmc driver, then there is no reason why the driver cannot do this. I believe passing clk details through platform data is an abuse of clock framework. 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 v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD
Hi Jon, On Wed, Jun 13, 2012 at 20:38:09, Hunter, Jon wrote: On 06/13/2012 08:05 AM, Mohammed, Afzal wrote: Do you mean that gpmc driver should have the capability to calculate peripheral timings at runtime based on frequency ?, I am not sure how this can be handled by gpmc driver as calculation for different peripherals are done in different way, requiring gpmc driver to know about connected peripheral, that would imply that gpmc driver would not be peripheral agnostic. I guess I still don't agree with this. From the gpmc timing point of view it should not care what device is connected, it just needs to know the associated timings. Therefore, the gpmc driver just needs the time associated with the different timings fields in the various GPMC_CONFIGx registers and then convert to clocks based upon the gpmc fclk. The only item that is device specific here is the actual timing values and these can be passed to the driver. Furthermore, gpmc_cs_set_timings function is completely device agnostic. Why can we not call this from within the driver? Why does it need to be called outside the driver? As mentioned in one of my previous mails, input to gpmc_cs_set_timings sometimes are a function of gpmc clock, and that function can vary with the type of peripheral connected. 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] OMAP: Beagle: fix TFP410 powerdown GPIO init
On Wed, 2012-06-13 at 23:58 -0700, Russ Dill wrote: On Wed, Jun 13, 2012 at 2:20 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: Commit e813a55eb9c9bc6c8039fb16332cf43402125b30 (OMAP: board-files: remove custom PD GPIO handling for DVI output) moved TFP410 chip's powerdown-gpio handling from the board files to the tfp410 driver. One gpio_request_one(powerdown-gpio, ...) was mistakenly left unremoved in the Beagle board file. This causes the tfp410 driver to fail to request the gpio on Beagle, causing the driver to fail and thus the DVI output doesn't work. Can you take the one I sent earlier instead? http://www.spinics.net/lists/linux-omap/msg69913.html Hmm, that probably doesn't apply. The power-down GPIO is now handled in the tfp410 driver, not in the board files. Looking more closely at the board file, what are these nDVI_PWR_EN, DVI_LDO_EN and DVI_PU gpios? There's no enable line on tfp410. Only the power-down gpio, which is none of the above... Tomi signature.asc Description: This is a digitally signed message part
Re: [PATCH] OMAP: Beagle: fix TFP410 powerdown GPIO init
On Thu, Jun 14, 2012 at 12:18 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-06-13 at 23:58 -0700, Russ Dill wrote: On Wed, Jun 13, 2012 at 2:20 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: Commit e813a55eb9c9bc6c8039fb16332cf43402125b30 (OMAP: board-files: remove custom PD GPIO handling for DVI output) moved TFP410 chip's powerdown-gpio handling from the board files to the tfp410 driver. One gpio_request_one(powerdown-gpio, ...) was mistakenly left unremoved in the Beagle board file. This causes the tfp410 driver to fail to request the gpio on Beagle, causing the driver to fail and thus the DVI output doesn't work. Can you take the one I sent earlier instead? http://www.spinics.net/lists/linux-omap/msg69913.html Hmm, that probably doesn't apply. The power-down GPIO is now handled in the tfp410 driver, not in the board files. Give me a branch to rebase it onto and I will. Looking more closely at the board file, what are these nDVI_PWR_EN, DVI_LDO_EN and DVI_PU gpios? There's no enable line on tfp410. Only the power-down gpio, which is none of the above... On my schematic, DVI_LDO_EN is labeled more sanely, DVI_PU. I'm not sure of the history of that. My patch does remove any reference to DVI_LDO_EN. nDVI_PWR_EN is labeled AUX_3V3_DIS on my schematic. AUX_3V3_DIS is connected to the disable pin of an LDO that provides power for the DVI transceiver. -- 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: usage of sparse or other trick for improved type safety
Hi Richard, all, On Tue, Jun 12, 2012 at 6:34 PM, Woodruff, Richard r-woodru...@ti.com wrote: Hi Tony, From: Tony Lindgren [mailto:t...@atomide.com] Sent: Friday, May 25, 2012 2:53 AM Thanks for quick input. Apologies on slow ack of it. Before we had these frameworks in place it was often hard to debug when something did not work for some omaps. Things would just not work or would stop working for some drivers with no ideas what was going on. So yeah, having these kind of frameworks in place has helped a lot to avoid breakage like you're describing above. Trees which had to hit the lowest power states for full customer boards all employed some form of abstraction of clock, power, voltage. The one today in mainline today is the most clean. Aspects around auto-generation are very good even if a bit big in size. Perhaps main grump I hear is the number of abstraction layers sometimes makes debugging difficult. It would be nice to find smart tricks for compile to prune away some. Still one can experience some of the mystery issues, as the PRCM connection to IP-device utilizes a protocol which the device side can mess up. If the device does not shut down its local function and associated clocks cleanly it will show 'stuck in transition' and this gates final global changes. In one of the closed implementation we would bug() drivers who did not shut down their internal clocks properly before allowing global clock release. Most of the issues seen in field are at drivers/peripheral-ip. For some external subsystems it might be possible to let the omap kernel manage powerdomains and other resource via remote proc interface, assuming these registers are ioremapped in the omap kernel side and not owned by the external subsystem. The messaging to do this would add some undesired latencies though, but maybe those would not be so critical for the external subsystems as they tend to be full on or completely off type accelerators. Humm. Maybe for some. Guess walking what is used and sorting might tell. The way some subsystems 'ideal' operation is described from designers implies tight control of timing and sequence for things like power state. A RPC doesn't feel like it fits with intent. However... practically speaking from 'full system view' RPC may fit sometimes. A subsystem exporting hooks to save 100uW using its optimal state set against other components running in 500mW range minimizes usefulness. The other alternative of course is to implement similar frameworks for the external subsystems. Some of this might be possible to implement as simple macros with some type checks if it's subsystem specific. For doing it for all omap devices, macros were soon found not to be enough as the various bits all over need to be linked together for managing various resources. Agree. I don't know insides of all subsystems. Though what I know or hear is kind of a mixed picture. OMAP has an ultra high level of integration. As such you might find something like DSP-Bios may provide a good hook but quick integration of a previously standalone single purpose piece does not have time to be readapted. The type checking question kind of grows out of this. Linux might today offer a clean run-time api which is place where high wall should be built with type check. But... the driver might not be able to function yet with the API alone given state of evolution of both ends. Regarding the API and type checking I think the following must be enforced: 1. proper type checking in the API, possibly by the compiler, 2. clean separation of an API into an internal part (used only by the framework implementation) and an external part (used by the callers of the API). About 1: using enum for the parameters did not help much AFACIT. The compiler cannot tell if the parameter from a variable is in the allowed range. Any thought? About 2: while introducing the functional power states I came across this problem. Ideally the current states macros (PWRDM_POWER_* and PWRSTS_*) shall be used by the internal code only in powerdomain*.[ch] while the callers (pm.c etc) shall use the new macros (PWRDM_FUNC_PWRST_*) and API (mainly pwrdm_*_func_* and omap_set_pwrdm_state). Here below is a patch extract (trimmed for brevity) to illustrate the problem. From here is see a few possible options: 1. clearly separate the internal and external parts of the API in the header file using comments (as done in the patch below) or with a new #ifdef POWERDOMAIN_INTERNAL_API (ugly I know), 2. create a new internal only header file (powerdomain_internal.h) and include it only from powerdomain*.[ch], 3. move the external API to pm.h and keep the internal API in powerdomain.h Although I am using the func power states as an example I think this is a applicable to all PM frameworks APIs. What do you think? diff --git a/arch/arm/mach-omap2/powerdomain.h
Re: [PATCH V2 5/5] Input: ads7846: set proper debounce time in driver level
On 06/14/12 10:08, Zumeng Chen wrote: 于 2012年06月14日 14:44, Igor Grinberg 写道: On 06/14/12 07:46, Zumeng Chen wrote: 于 2012年06月13日 20:18, Hiremath, Vaibhav 写道: On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote: From: Zumeng Chenzumeng.c...@windriver.com If we don't set proper debouce time for ads7846, then there are flooded interrupt counters of ads7846 responding to one time touch on screen, so the driver couldn't work well. And since most OMAP3 series boards pass NULL pointer of board_pdata to omap_ads7846_init, so it's more proper to set it in driver level after having gpio_request done. This patch has been validated on 3530evm. Signed-off-by: Zumeng Chenzumeng.c...@windriver.com Signed-off-by: Syed Mohammed Khasimkha...@ti.com --- drivers/input/touchscreen/ads7846.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c index f02028e..459ff29 100644 --- a/drivers/input/touchscreen/ads7846.c +++ b/drivers/input/touchscreen/ads7846.c @@ -980,6 +980,10 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784 } ts-gpio_pendown = pdata-gpio_pendown; +#ifdef CONFIG_ARCH_OMAP3 +/* 310 means about 10 microsecond for omap3 */ +gpio_set_debounce(pdata-gpio_pendown, 310); +#endif Zumeng, With my sign-off you are changing the original code, that too without my sign-off and ack. I wouldn't mind you to submit patches from my tree, but the expectation is if you are changing the original code, it should be under your sign-off. Thanks, good to learn. I'll remove in next time. Coming to the patch, #ifdef in driver is not recommended, and this 10msec delay is specific to OMAP GPIO and driver should not be aware of this, that's where you will find the original patch handling it in board file. According to the git blame of the board-omap3evm.c I think 96974a24 did a good thing to all the related codes for omap3 boards. So I think we can call board and driver as BSP level :-) If #ifdef in driver can save many codes, I guess it's deserved. No, platform/board specific ifdefs in the generic driver code are never deserved. How about the attached patch, does it fix the problem for you? Sorry Igor, I just read your patch, yes, most are there, and we just have to set get_pendown_state as you early said. Really *thanks* for your drive :) No, with my patch, you don't need the get_pendown_state() and you don't need to change the board-omap3evm.c ADS7846 related code. So, I'm going to submit my patch properly. Regards, Zumeng -- Regards, Igor. ads7846.patch diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c index 1706ebc..c187586 100644 --- a/arch/arm/mach-omap2/common-board-devices.c +++ b/arch/arm/mach-omap2/common-board-devices.c @@ -63,28 +63,30 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce, struct spi_board_info *spi_bi = ads7846_spi_board_info; int err; -if (board_pdata board_pdata-get_pendown_state) { -err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown); -if (err) { -pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err); -return; -} -gpio_export(gpio_pendown, 0); - -if (gpio_debounce) -gpio_set_debounce(gpio_pendown, gpio_debounce); +err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown); +if (err) { +pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err); +return; } +if (gpio_debounce) +gpio_set_debounce(gpio_pendown, gpio_debounce); + spi_bi-bus_num = bus_num; spi_bi-irq = gpio_to_irq(gpio_pendown); if (board_pdata) { board_pdata-gpio_pendown = gpio_pendown; spi_bi-platform_data = board_pdata; +if (board_pdata-get_pendown_state) +gpio_export(gpio_pendown, 0); } else { ads7846_config.gpio_pendown = gpio_pendown; } +if (!board_pdata || (board_pdata !board_pdata-get_pendown_state)) +gpio_free(gpio_pendown); + spi_register_board_info(ads7846_spi_board_info, 1); } #else -- Regards, Igor. -- 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] OMAP: Beagle: fix TFP410 powerdown GPIO init
On Thu, 2012-06-14 at 00:59 -0700, Russ Dill wrote: On Thu, Jun 14, 2012 at 12:18 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-06-13 at 23:58 -0700, Russ Dill wrote: On Wed, Jun 13, 2012 at 2:20 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: Commit e813a55eb9c9bc6c8039fb16332cf43402125b30 (OMAP: board-files: remove custom PD GPIO handling for DVI output) moved TFP410 chip's powerdown-gpio handling from the board files to the tfp410 driver. One gpio_request_one(powerdown-gpio, ...) was mistakenly left unremoved in the Beagle board file. This causes the tfp410 driver to fail to request the gpio on Beagle, causing the driver to fail and thus the DVI output doesn't work. Can you take the one I sent earlier instead? http://www.spinics.net/lists/linux-omap/msg69913.html Hmm, that probably doesn't apply. The power-down GPIO is now handled in the tfp410 driver, not in the board files. Give me a branch to rebase it onto and I will. v3.5-rc2 Looking more closely at the board file, what are these nDVI_PWR_EN, DVI_LDO_EN and DVI_PU gpios? There's no enable line on tfp410. Only the power-down gpio, which is none of the above... On my schematic, DVI_LDO_EN is labeled more sanely, DVI_PU. I'm not sure of the history of that. My patch does remove any reference to DVI_LDO_EN. nDVI_PWR_EN is labeled AUX_3V3_DIS on my schematic. AUX_3V3_DIS is connected to the disable pin of an LDO that provides power for the DVI transceiver. Okay. These are a bit problematic, because we're in the process of removing these kinds of things from the board file, as they cannot be supported with device tree. The tfp410 driver in v3.5 doesn't even have the platform_enable/disable callback anymore. Those do not belong to tfp410 driver, but I don't really know how they should be handled. This is one of the questions about device tree that is unclear to me... Tomi signature.asc Description: This is a digitally signed message part
[PATCH] ARM: OMAP: fix the ads7846 init code
In case a board provides the gpio_pendown and not board_pdata, the GPIO debounce is not taken care of. Fix this by taking care of GPIO debounce in any case. Signed-off-by: Igor Grinberg grinb...@compulab.co.il --- arch/arm/mach-omap2/common-board-devices.c | 22 -- 1 files changed, 12 insertions(+), 10 deletions(-) diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c index 1706ebc..c187586 100644 --- a/arch/arm/mach-omap2/common-board-devices.c +++ b/arch/arm/mach-omap2/common-board-devices.c @@ -63,28 +63,30 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce, struct spi_board_info *spi_bi = ads7846_spi_board_info; int err; - if (board_pdata board_pdata-get_pendown_state) { - err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown); - if (err) { - pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err); - return; - } - gpio_export(gpio_pendown, 0); - - if (gpio_debounce) - gpio_set_debounce(gpio_pendown, gpio_debounce); + err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown); + if (err) { + pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err); + return; } + if (gpio_debounce) + gpio_set_debounce(gpio_pendown, gpio_debounce); + spi_bi-bus_num = bus_num; spi_bi-irq = gpio_to_irq(gpio_pendown); if (board_pdata) { board_pdata-gpio_pendown = gpio_pendown; spi_bi-platform_data = board_pdata; + if (board_pdata-get_pendown_state) + gpio_export(gpio_pendown, 0); } else { ads7846_config.gpio_pendown = gpio_pendown; } + if (!board_pdata || (board_pdata !board_pdata-get_pendown_state)) + gpio_free(gpio_pendown); + spi_register_board_info(ads7846_spi_board_info, 1); } #else -- 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
Re: [PATCH] OMAP: Beagle: fix TFP410 powerdown GPIO init
On Thu, Jun 14, 2012 at 1:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Thu, 2012-06-14 at 00:59 -0700, Russ Dill wrote: On Thu, Jun 14, 2012 at 12:18 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-06-13 at 23:58 -0700, Russ Dill wrote: On Wed, Jun 13, 2012 at 2:20 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: Commit e813a55eb9c9bc6c8039fb16332cf43402125b30 (OMAP: board-files: remove custom PD GPIO handling for DVI output) moved TFP410 chip's powerdown-gpio handling from the board files to the tfp410 driver. One gpio_request_one(powerdown-gpio, ...) was mistakenly left unremoved in the Beagle board file. This causes the tfp410 driver to fail to request the gpio on Beagle, causing the driver to fail and thus the DVI output doesn't work. Can you take the one I sent earlier instead? http://www.spinics.net/lists/linux-omap/msg69913.html Hmm, that probably doesn't apply. The power-down GPIO is now handled in the tfp410 driver, not in the board files. Give me a branch to rebase it onto and I will. v3.5-rc2 Looking more closely at the board file, what are these nDVI_PWR_EN, DVI_LDO_EN and DVI_PU gpios? There's no enable line on tfp410. Only the power-down gpio, which is none of the above... On my schematic, DVI_LDO_EN is labeled more sanely, DVI_PU. I'm not sure of the history of that. My patch does remove any reference to DVI_LDO_EN. nDVI_PWR_EN is labeled AUX_3V3_DIS on my schematic. AUX_3V3_DIS is connected to the disable pin of an LDO that provides power for the DVI transceiver. Okay. These are a bit problematic, because we're in the process of removing these kinds of things from the board file, as they cannot be supported with device tree. The tfp410 driver in v3.5 doesn't even have the platform_enable/disable callback anymore. Those do not belong to tfp410 driver, but I don't really know how they should be handled. This is one of the questions about device tree that is unclear to me... Are you talking about the AUX_3V3_DIS pin? The boardfile currently just initializes that low, that should be too much of a problem to put into DT, or if someone is so inclined assign it to a regulator via DT. -- 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 V2 5/5] Input: ads7846: set proper debounce time in driver level
于 2012年06月14日 16:06, Igor Grinberg 写道: On 06/14/12 10:08, Zumeng Chen wrote: 于 2012年06月14日 14:44, Igor Grinberg 写道: On 06/14/12 07:46, Zumeng Chen wrote: 于 2012年06月13日 20:18, Hiremath, Vaibhav 写道: On Wed, Jun 13, 2012 at 07:14:10, Zumeng Chen wrote: From: Zumeng Chenzumeng.c...@windriver.com If we don't set proper debouce time for ads7846, then there are flooded interrupt counters of ads7846 responding to one time touch on screen, so the driver couldn't work well. And since most OMAP3 series boards pass NULL pointer of board_pdata to omap_ads7846_init, so it's more proper to set it in driver level after having gpio_request done. This patch has been validated on 3530evm. Signed-off-by: Zumeng Chenzumeng.c...@windriver.com Signed-off-by: Syed Mohammed Khasimkha...@ti.com --- drivers/input/touchscreen/ads7846.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c index f02028e..459ff29 100644 --- a/drivers/input/touchscreen/ads7846.c +++ b/drivers/input/touchscreen/ads7846.c @@ -980,6 +980,10 @@ static int __devinit ads7846_setup_pendown(struct spi_device *spi, struct ads784 } ts-gpio_pendown = pdata-gpio_pendown; +#ifdef CONFIG_ARCH_OMAP3 +/* 310 means about 10 microsecond for omap3 */ +gpio_set_debounce(pdata-gpio_pendown, 310); +#endif Zumeng, With my sign-off you are changing the original code, that too without my sign-off and ack. I wouldn't mind you to submit patches from my tree, but the expectation is if you are changing the original code, it should be under your sign-off. Thanks, good to learn. I'll remove in next time. Coming to the patch, #ifdef in driver is not recommended, and this 10msec delay is specific to OMAP GPIO and driver should not be aware of this, that's where you will find the original patch handling it in board file. According to the git blame of the board-omap3evm.c I think 96974a24 did a good thing to all the related codes for omap3 boards. So I think we can call board and driver as BSP level :-) If #ifdef in driver can save many codes, I guess it's deserved. No, platform/board specific ifdefs in the generic driver code are never deserved. How about the attached patch, does it fix the problem for you? Sorry Igor, I just read your patch, yes, most are there, and we just have to set get_pendown_state as you early said. Really *thanks* for your drive :) No, with my patch, you don't need the get_pendown_state() and you don't need to change the board-omap3evm.c ADS7846 related code. So, I'm going to submit my patch properly. Feel free to do that Regards, Zumeng -- Regards, Igor. ads7846.patch diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c index 1706ebc..c187586 100644 --- a/arch/arm/mach-omap2/common-board-devices.c +++ b/arch/arm/mach-omap2/common-board-devices.c @@ -63,28 +63,30 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce, struct spi_board_info *spi_bi =ads7846_spi_board_info; int err; - if (board_pdata board_pdata-get_pendown_state) { - err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown); - if (err) { - pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err); - return; - } - gpio_export(gpio_pendown, 0); - - if (gpio_debounce) - gpio_set_debounce(gpio_pendown, gpio_debounce); + err = gpio_request_one(gpio_pendown, GPIOF_IN, TSPenDown); + if (err) { + pr_err(Couldn't obtain gpio for TSPenDown: %d\n, err); + return; } + if (gpio_debounce) + gpio_set_debounce(gpio_pendown, gpio_debounce); + spi_bi-bus_num = bus_num; spi_bi-irq = gpio_to_irq(gpio_pendown); if (board_pdata) { board_pdata-gpio_pendown = gpio_pendown; spi_bi-platform_data = board_pdata; + if (board_pdata-get_pendown_state) + gpio_export(gpio_pendown, 0); } else { ads7846_config.gpio_pendown = gpio_pendown; } + if (!board_pdata || (board_pdata !board_pdata-get_pendown_state)) + gpio_free(gpio_pendown); + spi_register_board_info(ads7846_spi_board_info, 1); } #else -- 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] OMAP: Beagle: fix TFP410 powerdown GPIO init
On Thu, 2012-06-14 at 01:17 -0700, Russ Dill wrote: On Thu, Jun 14, 2012 at 1:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: Okay. These are a bit problematic, because we're in the process of removing these kinds of things from the board file, as they cannot be supported with device tree. The tfp410 driver in v3.5 doesn't even have the platform_enable/disable callback anymore. Those do not belong to tfp410 driver, but I don't really know how they should be handled. This is one of the questions about device tree that is unclear to me... Are you talking about the AUX_3V3_DIS pin? The boardfile currently just initializes that low, that should be too much of a problem to put into DT, or if someone is so inclined assign it to a regulator via DT. Ah, sorry. You are right. The only gpio handled in the platform_enable/disable calls is the power-down gpio, and that is handled by the tfp410 driver properly. So there shouldn't be a problem there. Tomi signature.asc Description: This is a digitally signed message part
Re: [PATCH v5 04/14] ARM: OMAP2+: gpmc: minimal driver support
* Mohammed, Afzal af...@ti.com [120613 23:44]: Hi Tony, On Thu, Jun 14, 2012 at 12:05:25, Tony Lindgren wrote: * Mohammed, Afzal af...@ti.com [120613 06:56]: Or you meant, even there read revision register ? Yeah that should be fine as this really should only happen during init and whatever revision specific features can be initialized for GPMC. Sorry, I got confused, we need revision to be available while setting time also, so you meant to store it as a variable or read revision at probe as well as during setting time ? You can set up GPMC specific flags during init, something like OMAP_GPMC_HAS_XYZ, or you can set up function pointers if only some features are available on some revisions. Regards, Tony -- 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 v5 11/14] ARM: OMAP2+: gpmc: handle connected peripherals
Hi Jon, On Wed, Jun 13, 2012 at 21:01:08, Hunter, Jon wrote: On 06/11/2012 09:27 AM, Afzal Mohammed wrote: +static __devinit int gpmc_setup_cs(struct gpmc_peripheral *g_per, + struct gpmc_cs_data *cs, struct resource *res) { + int num, ret; + + num = gpmc_setup_cs_mem(cs, res); + if (IS_ERR_VALUE(num)) + return num; + + ret = gpmc_setup_cs_config_timing(g_per, cs); + if (IS_ERR_VALUE(ret)) + return ret; + + num += gpmc_setup_cs_irq(cs, res + num); What happens if the above function returns an error? That has been deliberately done as that implies user has not specified irq flag hence is not an error. But it seems, I should print error information if user tries to use an interrupt and if driver does not want/unable to use gpmc interrupt. 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 v5 04/14] ARM: OMAP2+: gpmc: minimal driver support
Hi Tony, On Thu, Jun 14, 2012 at 14:09:58, Tony Lindgren wrote: * Mohammed, Afzal af...@ti.com [120613 23:44]: Sorry, I got confused, we need revision to be available while setting time also, so you meant to store it as a variable or read revision at probe as well as during setting time ? You can set up GPMC specific flags during init, something like OMAP_GPMC_HAS_XYZ, or you can set up function pointers if only some features are available on some revisions. Ok, got it 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 v5 05/14] ARM: OMAP2+: gpmc: resource creation helpers
Hi Jon, On Wed, Jun 13, 2012 at 21:03:44, Hunter, Jon wrote: On 06/13/2012 12:29 AM, Mohammed, Afzal wrote: Irq memory resource creation functions returns number of resources, if memory function only is modified, that will cause loss of uniformity w.r.t irq function, even though both does similar things. Ok, I see what you mean but from a readability standpoint it looked odd. The other function is returning n where as this is just returning 1 on success but it is not clear that this actually means 1 resource. Ok, I will add comments to make it clear 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 v5 06/14] ARM: OMAP2+: gpmc: CS configuration helper
Hi Jon, On Wed, Jun 13, 2012 at 21:09:52, Hunter, Jon wrote: Sure, but reviewing the function it still looks odd from a readability standpoint. At least it made me think what is going on here So a comment is definitely needed. 2. A bad setting in the configuration passed. Hopefully, people will stick to the flags but we know that we expect the device type to be a 0 or 2 and so should we check? Value of device type is something driver has to worry about, not its users, they have been provided with two flags, one for NAND other for NOR. Yes, but the driver does not seem to worry about it. In other words, there is no error checking. Ok so not a big deal a comment should suffice. Ok, comments will be added to make intentions clear 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 v5 10/14] ARM: OMAP2+: gpmc: waitpin helper
Hi Jon, On Wed, Jun 13, 2012 at 21:14:30, Hunter, Jon wrote: On 06/13/2012 02:37 AM, Mohammed, Afzal wrote: In that case we would be directly depending on user flag whose value may or may not change and I don't think it is good to directly depend on it for waitpin # calculation. You are already dependent on it. In other words, you are going to set What I meant is we are not dependent on absolute value of flag to find waitpin, and I disagree in depending on its absolute value, which can change, while flag would be the same. 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 v5 14/14] ARM: OMAP2+: gpmc: writeprotect helper
Hi Tony, On Wed, Jun 13, 2012 at 21:58:53, Hunter, Jon wrote: Presently there are no peripherals in mainline turning on writeprotect, initially intention was to just disable writeprotect at the end of probe and not provide platforms opportunity to enable writeprotect as none need it, still a provision has been provided for platform to enable it. Probably these to be taken care when there is a requirement. Ok, but I am still not happy about this. So I did find that our omap2420h4 board does route the write protect to both NOR and NAND. Therefore, it does appear to be a valid use-case that multiple child devices can share the write-protect. So maybe we do not need to reserve the write-protect like we are doing for chip-selects, but I think that : devices should indicate if they use the write-protect pin and we should either have this enable on boot as a global setting not specific to a child device or ensure that multiple devices using the wp have the same configuration of the wp on boot. In other words, if one device says enable on boot and the other does not, then warn. Which one of Jon's above suggestions would you prefer, i.e. writeprotect as a global setting for all devices Or Ensure that multiple devices using wp have same configuration Or anything else you have in mind ? 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 6/9] ARM: OMAP2+: gpmc-smsc911x: Adapt to use gpmc driver
* Afzal Mohammed af...@ti.com [120611 08:20]: +__init gpmc_smsc911x_update(struct omap_smsc911x_platform_data *gpmc_cfg) +{ + int ret; + struct gpmc_device_pdata *gpmc_pdev; + struct gpmc_cs_data *gpmc_cs; + + gpmc_pdev = kzalloc(sizeof(*gpmc_pdev), GFP_KERNEL); + if (gpmc_pdev == NULL) + return gpmc_pdev; + + gpmc_cs = kzalloc(sizeof(*gpmc_cs), GFP_KERNEL); + if (gpmc_pdev == NULL) { + kfree(gpmc_pdev); + return NULL; + } Here your should check for if (!gpmc_cs), not gpmc_cs. Might be worth checking all your patches for similar copy and paste typos. Where do gpmc_pdev and gpmc_cs get used? Where are they stored to the pdata? Tony -- 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 6/9] ARM: OMAP2+: gpmc-smsc911x: Adapt to use gpmc driver
Hi Tony, On Thu, Jun 14, 2012 at 14:26:52, Tony Lindgren wrote: * Afzal Mohammed af...@ti.com [120611 08:20]: +__init gpmc_smsc911x_update(struct omap_smsc911x_platform_data *gpmc_cfg) +{ + int ret; + struct gpmc_device_pdata *gpmc_pdev; + struct gpmc_cs_data *gpmc_cs; + + gpmc_pdev = kzalloc(sizeof(*gpmc_pdev), GFP_KERNEL); + if (gpmc_pdev == NULL) + return gpmc_pdev; + + gpmc_cs = kzalloc(sizeof(*gpmc_cs), GFP_KERNEL); + if (gpmc_pdev == NULL) { + kfree(gpmc_pdev); + return NULL; + } Here your should check for if (!gpmc_cs), not gpmc_cs. Might My mistake, I will correct it Where do gpmc_pdev and gpmc_cs get used? Where are they stored to the pdata? It is done in board file, as in [1] Regards Afzal [1] ARM: OMAP2+: board omap3evm: use gpmc driver diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c index 639bd07..aa9429d 100644 --- a/arch/arm/mach-omap2/board-omap3evm.c +++ b/arch/arm/mach-omap2/board-omap3evm.c @@ -43,6 +43,7 @@ #include plat/board.h #include plat/usb.h +#include plat/gpmc.h #include common.h #include plat/mcspi.h #include video/omapdss.h @@ -102,6 +103,12 @@ static void __init omap3_evm_get_revision(void) } } +static struct gpmc_device_pdata *gpmc_device_data[2]; + +static struct gpmc_pdata gpmc_data = { + .device_pdata = gpmc_device_data, +}; + #if defined(CONFIG_SMSC911X) || defined(CONFIG_SMSC911X_MODULE) #include plat/gpmc-smsc911x.h @@ -122,7 +129,9 @@ static inline void __init omap3evm_init_smsc911x(void) smsc911x_cfg.gpio_reset = OMAP3EVM_GEN2_ETHR_GPIO_RST; } - gpmc_smsc911x_init(smsc911x_cfg); + *gpmc_device_data = gpmc_smsc911x_update(smsc911x_cfg); + if (!*gpmc_device_data) + pr_err(error: unable to initilaize gpmc smsc911x\n); } #else @@ -658,6 +667,7 @@ static void __init omap3_evm_init(void) usbhs_init(usbhs_bdata); omap_ads7846_init(1, OMAP3_EVM_TS_GPIO, 310, NULL); omap3evm_init_smsc911x(); + omap_gpmc_init(gpmc_data); omap3_evm_display_init(); omap3_evm_wl12xx_init(); } -- 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: [PATCHv3 03/20] ARM: OMAP4: PM: add support for device off
On Wed, 2012-06-13 at 17:21 +0200, Jean Pihet wrote: Hi Tero, I have a few remarks regarding the genericity of this code. I think it is better if the code in powerdomain.c stays generic and that the platform specific checks and operations be moved in the specific functions (via the pwrdm_ops struct). Well, I was thinking about this, however it looks like I need to copy over most of the implementation of omap_set_pwrdm_state and get_next_achievable_state if I want to do that, or alternatively overload the underlying omap4 pwrdm code which does not seem that good solution either. On Tue, Jun 12, 2012 at 5:31 PM, Tero Kristo t-kri...@ti.com wrote: On OMAP4+, device wide off-mode has its own enable mechanism in addition to powerdomain target states. This patch adds support for this on top of functional power states by overloading the OFF state for core pwrdm. On pwrdm level, the deepest power state supported by core pwrdm is OSWR. When user (e.g. suspend) programs core pwrdm target as OFF, the functional power state for the domain will be OSWR with the additional device off enabled. Previous power state information will reflect this also. Signed-off-by: Tero Kristo t-kri...@ti.com --- arch/arm/mach-omap2/powerdomain.c | 17 + arch/arm/mach-omap2/powerdomain.h | 13 +++- arch/arm/mach-omap2/powerdomain44xx.c | 48 +++ arch/arm/mach-omap2/powerdomains44xx_data.c |3 +- 4 files changed, 79 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index ac63f86..78a9308 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -677,6 +677,8 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) int sleep_switch = -1, ret = 0, hwsup = 0; int new_func_pwrst, next_func_pwrst, pwrst, logic; u8 curr_pwrst; + bool extra_off_enable = false; + bool has_extra_off = false; if (!pwrdm || IS_ERR(pwrdm)) { pr_debug(%s: invalid params: pwrdm=%p\n, __func__, pwrdm); @@ -687,6 +689,13 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) mutex_lock(pwrdm-lock); + /* Check if powerdomain has extra off mode handling */ + if (pwrdm-flags PWRDM_HAS_EXTRA_OFF_ENABLE) { + has_extra_off = true; + if (func_pwrst == PWRDM_FUNC_PWRST_OFF) + extra_off_enable = true; + } Could those checks be moved in the OMAP4 specific functions, so that the power domain code stays generic? I'll try to do that and see what happens. The problem I tried to tackle with this implementation is that even when core next state is programmed to func OFF, we must program the powerdomain itself to OSWR, which the current functional pwrst framework does not support too well or as you say, I need to re-write the omap4 pwrdm state handling code itself which kind of eats the benefit of the functional power states away. + new_func_pwrst = pwrdm_get_achievable_func_pwrst(pwrdm, func_pwrst); pwrst = pwrdm_func_to_pwrst(pwrdm, new_func_pwrst); logic = pwrdm_func_to_logic_pwrst(pwrdm, new_func_pwrst); @@ -741,6 +750,9 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 func_pwrst) break; } + if (has_extra_off arch_pwrdm-pwrdm_enable_off) + arch_pwrdm-pwrdm_enable_off(pwrdm, extra_off_enable); + out: mutex_unlock(pwrdm-lock); return ret; @@ -810,6 +822,11 @@ int pwrdm_read_next_func_pwrst(struct powerdomain *pwrdm) int next_pwrst = pwrdm_read_next_pwrst(pwrdm); int next_logic = pwrdm_read_logic_retst(pwrdm); + if (pwrdm-flags PWRDM_HAS_EXTRA_OFF_ENABLE + arch_pwrdm-pwrdm_read_next_off + arch_pwrdm-pwrdm_read_next_off(pwrdm)) + return PWRDM_FUNC_PWRST_OFF; + Same comment here. The OMAP4 specific function for pwrdm_read_next_pwrst could return PWRDM_FUNC_PWRST_OFF. Same issue. Anyway, I'll try to fiddle with the code bit more and see what happens. -Tero -- 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 v5 14/14] ARM: OMAP2+: gpmc: writeprotect helper
* Mohammed, Afzal af...@ti.com [120614 01:59]: Hi Tony, On Wed, Jun 13, 2012 at 21:58:53, Hunter, Jon wrote: Presently there are no peripherals in mainline turning on writeprotect, initially intention was to just disable writeprotect at the end of probe and not provide platforms opportunity to enable writeprotect as none need it, still a provision has been provided for platform to enable it. Probably these to be taken care when there is a requirement. Ok, but I am still not happy about this. So I did find that our omap2420h4 board does route the write protect to both NOR and NAND. Therefore, it does appear to be a valid use-case that multiple child devices can share the write-protect. So maybe we do not need to reserve the write-protect like we are doing for chip-selects, but I think that : devices should indicate if they use the write-protect pin and we should either have this enable on boot as a global setting not specific to a child device or ensure that multiple devices using the wp have the same configuration of the wp on boot. In other words, if one device says enable on boot and the other does not, then warn. Which one of Jon's above suggestions would you prefer, i.e. writeprotect as a global setting for all devices Or Ensure that multiple devices using wp have same configuration Or anything else you have in mind ? If it's shared it can cause undesired effects on the mounted rootfs if one device changes it. So some kind of checking for a shared case should be available before a device is allowed to change it. Tony -- 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 3/3] ARM: OMAP2+: gpmc: handle additional timings
Hi Tony, On Thu, Jun 14, 2012 at 14:59:57, Tony Lindgren wrote: * Afzal Mohammed af...@ti.com [120611 07:21]: + GPMC_SET_ONE(GPMC_CS_CONFIG6, 0, 3, bus_turnaround); + GPMC_SET_ONE(GPMC_CS_CONFIG6, 8, 11, cycle2cycle_delay); + + GPMC_SET_ONE(GPMC_CS_CONFIG1, 18, 19, wait_monitoring); + GPMC_SET_ONE(GPMC_CS_CONFIG1, 25, 26, clk_activation); + Thinking about this, the CONFIG1 bits have been set with gpmc_cs_write_reg because these are part of the static configuration and do not need to be dynamically calculated as they are tick based. For example, tusb6010 sets GPMC_CONFIG1_CLKACTIVATIONTIME(1) during init. But aren't we deciding number of ticks based on clock period ? If we take case of onenand, based on the knowledge of clock period, number of ticks are calculated. And similarly to decide cycle2cycledelay, busturnaround, we decide number of ticks based on peripheral datasheet timings gpmc clock, hence shouldn't it also be dynamically calculated similar to timings that were existing earlier. Regards Afzal Writing these over and over again during DVFS does not make sense, they should be only initialized once. Regards, Tony -- 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 -- 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: [PATCHv2 07/12] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
Hi (revised patch below) On Sun, 10 Jun 2012, Tony Lindgren wrote: * Paul Walmsley p...@pwsan.com [120610 17:56]: --- /dev/null +++ b/include/linux/platform_data/aess.h This should be include/linux/platform_data/omap-aess.h or similar as there are other aess controllers too. Hmm, I guess there could be other SoCs that include this IP block; probably there is nothing OMAP-specific about it. So I moved this file to include/sound/aess.h. What do you think? +/* + * AESS_AUTO_GATING_ENABLE_OFFSET: offset in bytes of the AESS IP + * block's AESS_AUTO_GATING_ENABLE__1 register from the IP block's + * base address + */ +#define AESS_AUTO_GATING_ENABLE_OFFSET 0x07c + +/* Register bitfields in the AESS_AUTO_GATING_ENABLE__1 register */ +#define AESS_AUTO_GATING_ENABLE_SHIFT 0 Also these should be OMAP_AESS_AUTOGATING etc, or even better, XYZ_AESS_AUTOGATING where XYZ is the type of the AESS controller. Hmm, do you think this is really needed? The other files in include/sound don't have SOUND_ or AUDIO_ prefixe. This should be static inline function as it's in the header, and again something like omap_aess_enable_autogating. I made it static inline, but didn't rename this one by the same rationale above. +/** + * hwmod_aess_preprogram - enable AESS internal autogating (called by hwmod) + * @oh: struct omap_hwmod * + * + * The AESS will not IdleAck to the PRCM until its internal autogating + * is enabled. Since internal autogating is disabled by default after + * AESS reset, we must enable autogating after the hwmod code resets + * the AESS. Returns 0. + */ +static int __maybe_unused hwmod_aess_preprogram(struct omap_hwmod *oh) +{ + void __iomem *va; + + va = omap_hwmod_get_mpu_rt_va(oh); + if (!va) + return -EINVAL; + + aess_enable_autogating(va); + + return 0; +} Then this function should not be in this header, instead it should be a static function somewhere in the omap hwmod code in some .c file. That's because this header should only have omap aess specific driver code, no hwmod code should be needed here. Moved this to arch/arm/mach-omap2/omap_hwmod_reset.c and prefixed it with omap_. Are you okay with this approach? - Paul From: Paul Walmsley p...@pwsan.com Date: Thu, 14 Jun 2012 03:35:13 -0600 Subject: [PATCH] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup Enable the AESS auto-gating control bit during AESS hwmod setup. This fixes the following boot warning on OMAP4: omap_hwmod: aess: _wait_target_disable failed Without this patch, the AESS IP block does not indicate to the PRCM that it is idle after it is reset. This prevents some types of SoC power management until something sets the auto-gating control bit. This second version of this patch moves the control functions to include/linux/platform_data/aess.h at Tony's request: http://www.spinics.net/lists/arm-kernel/msg178329.html Signed-off-by: Paul Walmsley p...@pwsan.com Cc: Benoît Cousson b-cous...@ti.com Cc: Péter Ujfalusi peter.ujfal...@ti.com Cc: Tony Lindgren t...@atomide.com --- arch/arm/mach-omap2/Makefile |2 +- arch/arm/mach-omap2/omap_hwmod_44xx_data.c |1 + arch/arm/mach-omap2/omap_hwmod_reset.c | 52 + arch/arm/plat-omap/include/plat/omap_hwmod.h |5 +++ include/sound/aess.h | 53 ++ 5 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 arch/arm/mach-omap2/omap_hwmod_reset.c create mode 100644 include/sound/aess.h diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index fa742f3..4381e04 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -7,7 +7,7 @@ obj-y := id.o io.o control.o mux.o devices.o serial.o gpmc.o timer.o pm.o \ common.o gpio.o dma.o wd_timer.o display.o i2c.o hdq1w.o omap-2-3-common= irq.o sdrc.o -hwmod-common = omap_hwmod.o \ +hwmod-common = omap_hwmod.o omap_hwmod_reset.o \ omap_hwmod_common_data.o clock-common = clock.o clock_common_data.o \ clkt_dpll.o clkt_clksel.o diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c index 6b0aedc..d1c4f3c 100644 --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c @@ -313,6 +313,7 @@ static struct omap_hwmod_class_sysconfig omap44xx_aess_sysc = { static struct omap_hwmod_class omap44xx_aess_hwmod_class = { .name = aess, .sysc = omap44xx_aess_sysc, + .setup_preprogram = omap_hwmod_aess_preprogram, }; /* aess */ diff --git a/arch/arm/mach-omap2/omap_hwmod_reset.c
Re: [PATCHv2 07/12] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
* Paul Walmsley p...@pwsan.com [120614 02:53]: Hi (revised patch below) On Sun, 10 Jun 2012, Tony Lindgren wrote: * Paul Walmsley p...@pwsan.com [120610 17:56]: --- /dev/null +++ b/include/linux/platform_data/aess.h This should be include/linux/platform_data/omap-aess.h or similar as there are other aess controllers too. Hmm, I guess there could be other SoCs that include this IP block; probably there is nothing OMAP-specific about it. So I moved this file to include/sound/aess.h. What do you think? Sounds good to me. +/* + * AESS_AUTO_GATING_ENABLE_OFFSET: offset in bytes of the AESS IP + * block's AESS_AUTO_GATING_ENABLE__1 register from the IP block's + * base address + */ +#define AESS_AUTO_GATING_ENABLE_OFFSET 0x07c + +/* Register bitfields in the AESS_AUTO_GATING_ENABLE__1 register */ +#define AESS_AUTO_GATING_ENABLE_SHIFT0 Also these should be OMAP_AESS_AUTOGATING etc, or even better, XYZ_AESS_AUTOGATING where XYZ is the type of the AESS controller. Hmm, do you think this is really needed? The other files in include/sound don't have SOUND_ or AUDIO_ prefixe. No you're right, those parts should follow what the driver is doing. This should be static inline function as it's in the header, and again something like omap_aess_enable_autogating. I made it static inline, but didn't rename this one by the same rationale above. OK +/** + * hwmod_aess_preprogram - enable AESS internal autogating (called by hwmod) + * @oh: struct omap_hwmod * + * + * The AESS will not IdleAck to the PRCM until its internal autogating + * is enabled. Since internal autogating is disabled by default after + * AESS reset, we must enable autogating after the hwmod code resets + * the AESS. Returns 0. + */ +static int __maybe_unused hwmod_aess_preprogram(struct omap_hwmod *oh) +{ + void __iomem *va; + + va = omap_hwmod_get_mpu_rt_va(oh); + if (!va) + return -EINVAL; + + aess_enable_autogating(va); + + return 0; +} Then this function should not be in this header, instead it should be a static function somewhere in the omap hwmod code in some .c file. That's because this header should only have omap aess specific driver code, no hwmod code should be needed here. Moved this to arch/arm/mach-omap2/omap_hwmod_reset.c and prefixed it with omap_. Are you okay with this approach? Yup looks good to me, thanks! We should probably get an ack from the ASoC people for the header changes. Regards, Tony - Paul From: Paul Walmsley p...@pwsan.com Date: Thu, 14 Jun 2012 03:35:13 -0600 Subject: [PATCH] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup Enable the AESS auto-gating control bit during AESS hwmod setup. This fixes the following boot warning on OMAP4: omap_hwmod: aess: _wait_target_disable failed Without this patch, the AESS IP block does not indicate to the PRCM that it is idle after it is reset. This prevents some types of SoC power management until something sets the auto-gating control bit. This second version of this patch moves the control functions to include/linux/platform_data/aess.h at Tony's request: http://www.spinics.net/lists/arm-kernel/msg178329.html Signed-off-by: Paul Walmsley p...@pwsan.com Cc: Benoît Cousson b-cous...@ti.com Cc: Péter Ujfalusi peter.ujfal...@ti.com Cc: Tony Lindgren t...@atomide.com --- arch/arm/mach-omap2/Makefile |2 +- arch/arm/mach-omap2/omap_hwmod_44xx_data.c |1 + arch/arm/mach-omap2/omap_hwmod_reset.c | 52 + arch/arm/plat-omap/include/plat/omap_hwmod.h |5 +++ include/sound/aess.h | 53 ++ 5 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 arch/arm/mach-omap2/omap_hwmod_reset.c create mode 100644 include/sound/aess.h diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index fa742f3..4381e04 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -7,7 +7,7 @@ obj-y := id.o io.o control.o mux.o devices.o serial.o gpmc.o timer.o pm.o \ common.o gpio.o dma.o wd_timer.o display.o i2c.o hdq1w.o omap-2-3-common = irq.o sdrc.o -hwmod-common = omap_hwmod.o \ +hwmod-common = omap_hwmod.o omap_hwmod_reset.o \ omap_hwmod_common_data.o clock-common = clock.o clock_common_data.o \ clkt_dpll.o clkt_clksel.o diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c index 6b0aedc..d1c4f3c 100644 --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c +++
Re: [PATCH 0/6] ARM: OMAP: hwmod: remove runtime cpu_is checking
Paul, On 04/30/2012 08:11 PM, Paul Walmsley wrote: Hi Kevin, On Fri, 27 Apr 2012, Kevin Hilman wrote: This series attempts to remove all the runtime cpu_is* checking in omap_hwmod.c in favor of using function pointers initialized at init time. This series was motivated by the addition of support for the AM335x series which was done by adding several more cpu_is* checks, and provided the proverbial straw that broke the camel's back. In addition to the cleanup, this provides a much cleaner way of adding additional SoC support since it no longer requires adding additional runtime cpu_is* checks. Boot tested on OMAP3530/Overo and OMAP4430/Panda. Thanks Kevin, this series should bridge the gap nicely between the current code, and the point in time that we're able to move these functions into PRM/CM device driver code. And allow us to avoid adding more cpu_is_*() when we merge Vaibhav's hwmod changes. As you've probably seen, the patches have been updated here. The function pointers have been made file-static rather than per-hwmod; this should conserve some memory and cache. Since they don't change on a per-hwmod basis, this should be safe. Also added quite a bit of kerneldoc -- basically I have an internal rule here now to ensure this exists on all functions and structures that are added, so it would be great to have this on future code :-) The updated series is at git://git.pwsan.com/linux-2.6 in the hwmod_soc_conditional_cleanup_3.5 branch. Please let me know if you have any comments. It's only compile-tested here so far. I'll be rebasing my local copy of Vaibhav's series on it and will plan to send it upstream for 3.5. I guess this series missed 3.5 merge window. I was re-basing the OMAP5 minimal support against the 3.5-rc2 and one of the dependency for that series is $subject series. Tony, How do we get the dependent series merged early enough so that OMAP5 support can be merged in linux-omap master to avoid any late build breaks. Regards santosh -- 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 3/3] ARM: OMAP2+: gpmc: handle additional timings
Hi Tony, On Wed, Jun 13, 2012 at 17:28:33, Mohammed, Afzal wrote: On Wed, Jun 13, 2012 at 17:24:45, Tony Lindgren wrote: If there's a chance it causing file system corruption, we should test it carefully on the boards before applying. If that's done, then there's probably no need for warnings. It's safer to disable NAND for untested boards if there's a chance of breaking the timings. Actually this patch breaks at least DMA on tusb6010 on n8x0. That's a MUSB hardware in a wrapper connected to GPMC that's very picky with the timings. Got any hints what should be done with the cycle2cycle stuff for tusb6010? Not as of now, let me try to find out. Probably with the below patch [1], we can get values set by bootloader calculate back value to entered in Kernel, of course that may not work if tusb6010 works with different gpmc i/p frequency. Meanwhile, let me try to get datasheet find proper values. Regards Afzal diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c index eec011a..bc75e6c 100644 --- a/arch/arm/mach-omap2/gpmc.c +++ b/arch/arm/mach-omap2/gpmc.c @@ -336,11 +336,15 @@ int gpmc_cs_calc_divider(int cs, unsigned int sync_clk) return div; } +static void gpmc_print_cs_timings(int cs); + int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t) { int div; u32 l; + gpmc_print_cs_timings(cs); + div = gpmc_cs_calc_divider(cs, t-sync_clk); if (div 0) return -1; -- 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 3/3] ARM: OMAP2+: gpmc: handle additional timings
* Mohammed, Afzal af...@ti.com [120614 03:15]: Hi Tony, On Wed, Jun 13, 2012 at 17:28:33, Mohammed, Afzal wrote: On Wed, Jun 13, 2012 at 17:24:45, Tony Lindgren wrote: If there's a chance it causing file system corruption, we should test it carefully on the boards before applying. If that's done, then there's probably no need for warnings. It's safer to disable NAND for untested boards if there's a chance of breaking the timings. Actually this patch breaks at least DMA on tusb6010 on n8x0. That's a MUSB hardware in a wrapper connected to GPMC that's very picky with the timings. Got any hints what should be done with the cycle2cycle stuff for tusb6010? Not as of now, let me try to find out. Probably with the below patch [1], we can get values set by bootloader calculate back value to entered in Kernel, of course that may not work if tusb6010 works with different gpmc i/p frequency. Well I took a look at the values, and it seems the only difference is the static GPMC_CONFIG1_CLKACTIVATIONTIME(1) that your patch now overwrites 0. Tony -- 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 v5 14/14] ARM: OMAP2+: gpmc: writeprotect helper
Hi, On Thu, Jun 14, 2012 at 15:06:25, Tony Lindgren wrote: * Mohammed, Afzal af...@ti.com [120614 01:59]: On Wed, Jun 13, 2012 at 21:58:53, Hunter, Jon wrote: devices should indicate if they use the write-protect pin and we should either have this enable on boot as a global setting not specific to a child device or ensure that multiple devices using the wp have the same configuration of the wp on boot. In other words, if one device says enable on boot and the other does not, then warn. Which one of Jon's above suggestions would you prefer, i.e. writeprotect as a global setting for all devices Or Ensure that multiple devices using wp have same configuration Or anything else you have in mind ? If it's shared it can cause undesired effects on the mounted rootfs if one device changes it. So some kind of checking for a shared case should be available before a device is allowed to change it. Ok, I will add a check for shared case. 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
[PATCH 00/17] Big OMAP I2C Cleanup
Hi guys, I have dropped a few patches from the series and also tested every single patch on my pandaboard. I2C messages are still working fine with panda after each patch. There's still lots of work to be done on the i2c-omap.c driver but it's now easier to read, IMO. Felipe Balbi (17): i2c: omap: simplify num_bytes handling i2c: omap: decrease indentation level on data handling i2c: omap: add blank lines i2c: omap: simplify omap_i2c_ack_stat() i2c: omap: split out [XR]DR and [XR]RDY i2c: omap: improve 1p153 errata handling i2c: omap: re-factor receive/transmit data loop i2c: omap: switch over to do {} while loop i2c: omap: ack IRQ in parts i2c: omap: get rid of the complete label i2c: omap: switch to devm_* API i2c: omap: switch to platform_get_irq() i2c: omap: bus: add a receiver flag i2c: omap: simplify errata check i2c: omap: always return IRQ_HANDLED i2c: omap: simplify IRQ exit path i2c: omap: resize fifos before each message drivers/i2c/busses/i2c-omap.c | 388 +++-- 1 file changed, 222 insertions(+), 166 deletions(-) -- 1.7.10.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
[PATCH 11/17] i2c: omap: switch to devm_* API
that helps deleting some boiler plate code and lets driver-core manage our resources for us. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 43 +++-- 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index c20c45f..d805270 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1045,7 +1045,7 @@ omap_i2c_probe(struct platform_device *pdev) { struct omap_i2c_dev *dev; struct i2c_adapter *adap; - struct resource *mem, *irq, *ioarea; + struct resource *mem, *irq; struct omap_i2c_bus_platform_data *pdata = pdev-dev.platform_data; struct device_node *node = pdev-dev.of_node; const struct of_device_id *match; @@ -1058,23 +1058,17 @@ omap_i2c_probe(struct platform_device *pdev) dev_err(pdev-dev, no mem resource?\n); return -ENODEV; } + irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!irq) { dev_err(pdev-dev, no irq resource?\n); return -ENODEV; } - ioarea = request_mem_region(mem-start, resource_size(mem), - pdev-name); - if (!ioarea) { - dev_err(pdev-dev, I2C region already claimed\n); - return -EBUSY; - } - - dev = kzalloc(sizeof(struct omap_i2c_dev), GFP_KERNEL); + dev = devm_kzalloc(pdev-dev, sizeof(struct omap_i2c_dev), GFP_KERNEL); if (!dev) { - r = -ENOMEM; - goto err_release_region; + dev_err(pdev-dev, not enough memory\n); + return -ENOMEM; } match = of_match_device(of_match_ptr(omap_i2c_of_match), pdev-dev); @@ -1097,10 +1091,10 @@ omap_i2c_probe(struct platform_device *pdev) dev-dev = pdev-dev; dev-irq = irq-start; - dev-base = ioremap(mem-start, resource_size(mem)); + dev-base = devm_request_and_ioremap(pdev-dev, mem); if (!dev-base) { - r = -ENOMEM; - goto err_free_mem; + dev_err(pdev-dev, ioremap failed\n); + return -ENOMEM; } platform_set_drvdata(pdev, dev); @@ -1151,7 +1145,7 @@ omap_i2c_probe(struct platform_device *pdev) isr = (dev-rev OMAP_I2C_OMAP1_REV_2) ? omap_i2c_omap1_isr : omap_i2c_isr; - r = request_irq(dev-irq, isr, 0, pdev-name, dev); + r = devm_request_irq(pdev-dev, dev-irq, isr, 0, pdev-name, dev); if (r) { dev_err(dev-dev, failure requesting irq %i\n, dev-irq); @@ -1177,24 +1171,16 @@ omap_i2c_probe(struct platform_device *pdev) r = i2c_add_numbered_adapter(adap); if (r) { dev_err(dev-dev, failure adding adapter\n); - goto err_free_irq; + goto err_unuse_clocks; } of_i2c_register_devices(adap); return 0; -err_free_irq: - free_irq(dev-irq, dev); err_unuse_clocks: omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); pm_runtime_put(dev-dev); - iounmap(dev-base); -err_free_mem: - platform_set_drvdata(pdev, NULL); - kfree(dev); -err_release_region: - release_mem_region(mem-start, resource_size(mem)); return r; } @@ -1203,17 +1189,10 @@ static int omap_i2c_remove(struct platform_device *pdev) { struct omap_i2c_dev *dev = platform_get_drvdata(pdev); - struct resource *mem; - platform_set_drvdata(pdev, NULL); - - free_irq(dev-irq, dev); i2c_del_adapter(dev-adapter); omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); - iounmap(dev-base); - kfree(dev); - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(mem-start, resource_size(mem)); + return 0; } -- 1.7.10.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
[PATCH 07/17] i2c: omap: re-factor receive/transmit data loop
re-factor the common parts to a separate function, so that code is easier to read and understand. No functional changes. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 208 + 1 file changed, 84 insertions(+), 124 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 52861c2..381bb36 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -795,12 +795,81 @@ static int errata_omap3_1p153(struct omap_i2c_dev *dev) return 0; } +static void omap_i2c_receive_data(struct omap_i2c_dev *dev, u8 num_bytes, + bool is_rdr) +{ + u16 w; + + while (num_bytes--) { + if (!dev-buf_len) { + dev_err(dev-dev, %s without data, + is_rdr ? RDR : RRDY); + break; + } + + w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); + *dev-buf++ = w; + dev-buf_len--; + + /* +* Data reg in 2430, omap3 and +* omap4 is 8 bit wide +*/ + if (dev-flags OMAP_I2C_FLAG_16BIT_DATA_REG) { + if (dev-buf_len) { + *dev-buf++ = w 8; + dev-buf_len--; + } + } + } +} + +static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes, + bool is_xdr) +{ + u16 w; + + while (num_bytes--) { + if (!dev-buf_len) { + dev_err(dev-dev, %s without data, + is_xdr ? XDR : XRDY); + break; + } + + w = *dev-buf++; + dev-buf_len--; + + /* +* Data reg in 2430, omap3 and +* omap4 is 8 bit wide +*/ + if (dev-flags OMAP_I2C_FLAG_16BIT_DATA_REG) { + if (dev-buf_len) { + w |= *dev-buf++ 8; + dev-buf_len--; + } + } + + if (dev-errata I2C_OMAP3_1P153) { + int ret; + + ret = errata_omap3_1p153(dev); + if (ret 0) + return ret; + } + + omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w); + } + + return 0; +} + static irqreturn_t omap_i2c_isr(int this_irq, void *dev_id) { struct omap_i2c_dev *dev = dev_id; u16 bits; - u16 stat, w; + u16 stat; int err, count = 0; if (pm_runtime_suspended(dev-dev)) @@ -853,30 +922,7 @@ complete: if (dev-fifo_size) num_bytes = dev-fifo_size; - while (num_bytes--) { - if (!dev-buf_len) { - dev_err(dev-dev, - RDR IRQ while no data -requested\n); - break; - } - - w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); - *dev-buf++ = w; - dev-buf_len--; - - /* -* Data reg in 2430, omap3 and -* omap4 is 8 bit wide -*/ - if (dev-flags - OMAP_I2C_FLAG_16BIT_DATA_REG) { - if (dev-buf_len) { - *dev-buf++ = w 8; - dev-buf_len--; - } - } - } + omap_i2c_receive_data(dev, num_bytes, true); if (dev-errata I2C_OMAP_ERRATA_I207) i2c_omap_errata_i207(dev, stat); @@ -891,78 +937,23 @@ complete: if (dev-fifo_size) num_bytes = dev-fifo_size; - while (num_bytes--) { - if (!dev-buf_len) { - dev_err(dev-dev, - RRDY IRQ while no data -requested\n); - break; - } - - w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); -
[PATCH 09/17] i2c: omap: ack IRQ in parts
According to flow diagrams on OMAP TRMs, we should ACK the IRQ as they happen. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index f978b14..c00ba7d 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -894,21 +894,20 @@ omap_i2c_isr(int this_irq, void *dev_id) } complete: - /* -* Ack the stat in one go, but [R/X]DR and [R/X]RDY should be -* acked after the data operation is complete. -* Ref: TRM SWPU114Q Figure 18-31 -*/ - omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, stat - ~(OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR | - OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)); - - if (stat OMAP_I2C_STAT_NACK) + if (stat OMAP_I2C_STAT_NACK) { + dev_err(dev-dev, No Acknowledge\n); err |= OMAP_I2C_STAT_NACK; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; + } if (stat OMAP_I2C_STAT_AL) { dev_err(dev-dev, Arbitration lost\n); err |= OMAP_I2C_STAT_AL; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; } /* @@ -989,12 +988,18 @@ complete: if (stat OMAP_I2C_STAT_ROVR) { dev_err(dev-dev, Receive overrun\n); - dev-cmd_err |= OMAP_I2C_STAT_ROVR; + err |= OMAP_I2C_STAT_ROVR; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_ROVR); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; } if (stat OMAP_I2C_STAT_XUDF) { dev_err(dev-dev, Transmit underflow\n); - dev-cmd_err |= OMAP_I2C_STAT_XUDF; + err |= OMAP_I2C_STAT_XUDF; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XUDF); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; } } while (stat); -- 1.7.10.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
[PATCH 10/17] i2c: omap: get rid of the complete label
we can ack stat and complete the command from the errata handling itself. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index c00ba7d..c20c45f 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -893,7 +893,6 @@ omap_i2c_isr(int this_irq, void *dev_id) return IRQ_HANDLED; } -complete: if (stat OMAP_I2C_STAT_NACK) { dev_err(dev-dev, No Acknowledge\n); err |= OMAP_I2C_STAT_NACK; @@ -958,10 +957,12 @@ complete: num_bytes = dev-fifo_size; ret = omap_i2c_transmit_data(dev, num_bytes, true); - stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); if (ret 0) { err |= OMAP_I2C_STAT_XUDF; - goto complete; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XUDF | + OMAP_I2C_STAT_XDR); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; } omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XDR); @@ -976,10 +977,12 @@ complete: num_bytes = dev-fifo_size; ret = omap_i2c_transmit_data(dev, num_bytes, false); - stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); if (ret 0) { err |= OMAP_I2C_STAT_XUDF; - goto complete; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XUDF | + OMAP_I2C_STAT_XRDY); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; } omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XRDY); -- 1.7.10.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
[PATCH 14/17] i2c: omap: simplify errata check
omap_i2c_dev is allocated with kzalloc(), so we need not initialize b_hw to zero. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c |4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index fad5f6f..fc5b8bc 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1139,9 +1139,7 @@ omap_i2c_probe(struct platform_device *pdev) dev-fifo_size = (dev-fifo_size / 2); - if (dev-rev = OMAP_I2C_REV_ON_3530_4430) - dev-b_hw = 0; /* Disable hardware fixes */ - else + if (dev-rev OMAP_I2C_REV_ON_3530_4430) dev-b_hw = 1; /* Enable hardware fixes */ /* calculate wakeup latency constraint for MPU */ -- 1.7.10.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
[PATCH 16/17] i2c: omap: simplify IRQ exit path
instead of having multiple return points, use a goto statement to make that clearer. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 33 - 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 5b78a73..6e75d03 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -889,33 +889,27 @@ omap_i2c_isr(int this_irq, void *dev_id) stat = ~(OMAP_I2C_STAT_RDR | OMAP_I2C_STAT_RRDY); } - if (!stat) { - /* my work here is done */ - omap_i2c_complete_cmd(dev, err); - return IRQ_HANDLED; - } + if (!stat) + goto out; dev_dbg(dev-dev, IRQ (ISR = 0x%04x)\n, stat); if (count++ == 100) { dev_warn(dev-dev, Too much work in one IRQ\n); - omap_i2c_complete_cmd(dev, err); - return IRQ_HANDLED; + goto out; } if (stat OMAP_I2C_STAT_NACK) { dev_err(dev-dev, No Acknowledge\n); err |= OMAP_I2C_STAT_NACK; omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK); - omap_i2c_complete_cmd(dev, err); - return IRQ_HANDLED; + goto out; } if (stat OMAP_I2C_STAT_AL) { dev_err(dev-dev, Arbitration lost\n); err |= OMAP_I2C_STAT_AL; omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK); - omap_i2c_complete_cmd(dev, err); - return IRQ_HANDLED; + goto out; } /* @@ -928,8 +922,7 @@ omap_i2c_isr(int this_irq, void *dev_id) OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR | OMAP_I2C_STAT_ARDY)); - omap_i2c_complete_cmd(dev, err); - return IRQ_HANDLED; + goto out; } if (stat OMAP_I2C_STAT_RDR) { @@ -970,8 +963,7 @@ omap_i2c_isr(int this_irq, void *dev_id) err |= OMAP_I2C_STAT_XUDF; omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XUDF | OMAP_I2C_STAT_XDR); - omap_i2c_complete_cmd(dev, err); - return IRQ_HANDLED; + goto out; } omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XDR); @@ -990,8 +982,7 @@ omap_i2c_isr(int this_irq, void *dev_id) err |= OMAP_I2C_STAT_XUDF; omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XUDF | OMAP_I2C_STAT_XRDY); - omap_i2c_complete_cmd(dev, err); - return IRQ_HANDLED; + goto out; } omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XRDY); @@ -1002,19 +993,19 @@ omap_i2c_isr(int this_irq, void *dev_id) dev_err(dev-dev, Receive overrun\n); err |= OMAP_I2C_STAT_ROVR; omap_i2c_ack_stat(dev, OMAP_I2C_STAT_ROVR); - omap_i2c_complete_cmd(dev, err); - return IRQ_HANDLED; + goto out; } if (stat OMAP_I2C_STAT_XUDF) { dev_err(dev-dev, Transmit underflow\n); err |= OMAP_I2C_STAT_XUDF; omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XUDF); - omap_i2c_complete_cmd(dev, err); - return IRQ_HANDLED; + goto out; } } while (stat); +out: + omap_i2c_complete_cmd(dev, err); return IRQ_HANDLED; } -- 1.7.10.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
[PATCH 17/17] i2c: omap: resize fifos before each message
This patch will try to avoid the usage of draining feature by reconfiguring the FIFO the start condition of each message based on the message's size. By doing that, we will be better utilizing the FIFO when doing big transfers. While at that also drop the now uneeded check for dev-buf_len as we always know the amount of data to be transmitted. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 83 + 1 file changed, 51 insertions(+), 32 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 6e75d03..759fdbd 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -193,6 +193,7 @@ struct omap_i2c_dev { u8 *regs; size_t buf_len; struct i2c_adapter adapter; + u8 threshold; u8 fifo_size; /* use as flag and value * fifo_size==0 implies no fifo * if set, should be trsh+1 @@ -459,13 +460,6 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, scll); omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, sclh); - if (dev-fifo_size) { - /* Note: setup required fifo size - 1. RTRSH and XTRSH */ - buf = (dev-fifo_size - 1) 8 | OMAP_I2C_BUF_RXFIF_CLR | - (dev-fifo_size - 1) | OMAP_I2C_BUF_TXFIF_CLR; - omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf); - } - /* Take the I2C module out of reset: */ omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN); @@ -508,6 +502,45 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *dev) return 0; } +static void omap_i2c_resize_fifo(struct omap_i2c_dev *dev, u8 size, bool is_rx) +{ + u16 buf; + + if (dev-flags OMAP_I2C_FLAG_NO_FIFO) + return; + + /* +* Set up notification threshold based on message size. We're doing +* this to try and avoid draining feature as much as possible. Whenever +* we have big messages to transfer (bigger than our total fifo size) +* then we might use draining feature to transfer the remaining bytes. +*/ + + dev-threshold = clamp(size, (u8) 1, dev-fifo_size); + + buf = omap_i2c_read_reg(dev, OMAP_I2C_BUF_REG); + + if (is_rx) { + /* Clear RX Threshold */ + buf = ~(0x3f 8); + buf |= ((dev-threshold - 1) 8) | OMAP_I2C_BUF_RXFIF_CLR; + } else { + /* Clear TX Threshold */ + buf = ~0x3f; + buf |= (dev-threshold - 1) | OMAP_I2C_BUF_TXFIF_CLR; + } + + omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf); + + if (dev-rev OMAP_I2C_REV_ON_3530_4430) + dev-b_hw = 1; /* Enable hardware fixes */ + + /* calculate wakeup latency constraint for MPU */ + if (dev-set_mpu_wkup_lat != NULL) + dev-latency = (100 * dev-threshold) / + (1000 * dev-speed / 8); +} + /* * Low level master read/write transaction. */ @@ -524,6 +557,9 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, if (msg-len == 0) return -EINVAL; + dev-receiver = !!(msg-flags I2C_M_RD); + omap_i2c_resize_fifo(dev, msg-len, dev-receiver); + omap_i2c_write_reg(dev, OMAP_I2C_SA_REG, msg-addr); /* REVISIT: Could the STB bit of I2C_CON be used with probing? */ @@ -539,7 +575,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, init_completion(dev-cmd_complete); dev-cmd_err = 0; - dev-receiver = !!(msg-flags I2C_M_RD); w = OMAP_I2C_CON_EN | OMAP_I2C_CON_MST | OMAP_I2C_CON_STT; @@ -803,12 +838,6 @@ static void omap_i2c_receive_data(struct omap_i2c_dev *dev, u8 num_bytes, u16 w; while (num_bytes--) { - if (!dev-buf_len) { - dev_err(dev-dev, %s without data, - is_rdr ? RDR : RRDY); - break; - } - w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); *dev-buf++ = w; dev-buf_len--; @@ -818,10 +847,8 @@ static void omap_i2c_receive_data(struct omap_i2c_dev *dev, u8 num_bytes, * omap4 is 8 bit wide */ if (dev-flags OMAP_I2C_FLAG_16BIT_DATA_REG) { - if (dev-buf_len) { - *dev-buf++ = w 8; - dev-buf_len--; - } + *dev-buf++ = w 8; + dev-buf_len--; } } } @@ -832,12 +859,6 @@ static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8
[PATCH 15/17] i2c: omap: always return IRQ_HANDLED
otherwise we could get our IRQ line disabled due to many spurious IRQs. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index fc5b8bc..5b78a73 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1015,7 +1015,7 @@ omap_i2c_isr(int this_irq, void *dev_id) } } while (stat); - return count ? IRQ_HANDLED : IRQ_NONE; + return IRQ_HANDLED; } static const struct i2c_algorithm omap_i2c_algo = { -- 1.7.10.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
[PATCH 13/17] i2c: omap: bus: add a receiver flag
that way we can ignore TX IRQs while in receiver mode and ignore RX IRQs while in transmitter mode. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 43d3289..fad5f6f 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -199,6 +199,7 @@ struct omap_i2c_dev { */ u8 rev; unsignedb_hw:1; /* bad h/w fixes */ + unsignedreceiver:1; /* true when we're in receiver mode */ u16 iestate;/* Saved interrupt register */ u16 pscstate; u16 scllstate; @@ -538,6 +539,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, init_completion(dev-cmd_complete); dev-cmd_err = 0; + dev-receiver = !!(msg-flags I2C_M_RD); w = OMAP_I2C_CON_EN | OMAP_I2C_CON_MST | OMAP_I2C_CON_STT; @@ -880,6 +882,13 @@ omap_i2c_isr(int this_irq, void *dev_id) stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); stat = bits; + /* If we're in receiver mode, ignore XDR/XRDY */ + if (dev-receiver) { + stat = ~(OMAP_I2C_STAT_XDR | OMAP_I2C_STAT_XRDY); + } else { + stat = ~(OMAP_I2C_STAT_RDR | OMAP_I2C_STAT_RRDY); + } + if (!stat) { /* my work here is done */ omap_i2c_complete_cmd(dev, err); -- 1.7.10.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
[PATCH 12/17] i2c: omap: switch to platform_get_irq()
that's a nice helper from drivers core which will give us the exact IRQ number, instead of a pointer to an IRQ resource. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index d805270..43d3289 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1045,11 +1045,12 @@ omap_i2c_probe(struct platform_device *pdev) { struct omap_i2c_dev *dev; struct i2c_adapter *adap; - struct resource *mem, *irq; + struct resource *mem; struct omap_i2c_bus_platform_data *pdata = pdev-dev.platform_data; struct device_node *node = pdev-dev.of_node; const struct of_device_id *match; irq_handler_t isr; + int irq; int r; /* NOTE: driver uses the static register mapping */ @@ -1059,10 +1060,10 @@ omap_i2c_probe(struct platform_device *pdev) return -ENODEV; } - irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); - if (!irq) { + irq = platform_get_irq(pdev, 0); + if (irq 0) { dev_err(pdev-dev, no irq resource?\n); - return -ENODEV; + return irq; } dev = devm_kzalloc(pdev-dev, sizeof(struct omap_i2c_dev), GFP_KERNEL); @@ -1090,7 +1091,7 @@ omap_i2c_probe(struct platform_device *pdev) } dev-dev = pdev-dev; - dev-irq = irq-start; + dev-irq = irq; dev-base = devm_request_and_ioremap(pdev-dev, mem); if (!dev-base) { dev_err(pdev-dev, ioremap failed\n); -- 1.7.10.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
[PATCH 08/17] i2c: omap: switch over to do {} while loop
this will make sure that we execute at least once. No functional changes otherwise. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 21 +++-- 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 381bb36..f978b14 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -870,20 +870,29 @@ omap_i2c_isr(int this_irq, void *dev_id) struct omap_i2c_dev *dev = dev_id; u16 bits; u16 stat; - int err, count = 0; + int err = 0, count = 0; if (pm_runtime_suspended(dev-dev)) return IRQ_NONE; - bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG); - while ((stat = (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG))) bits) { + do { + bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG); + stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); + stat = bits; + + if (!stat) { + /* my work here is done */ + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; + } + dev_dbg(dev-dev, IRQ (ISR = 0x%04x)\n, stat); if (count++ == 100) { dev_warn(dev-dev, Too much work in one IRQ\n); - break; + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; } - err = 0; complete: /* * Ack the stat in one go, but [R/X]DR and [R/X]RDY should be @@ -987,7 +996,7 @@ complete: dev_err(dev-dev, Transmit underflow\n); dev-cmd_err |= OMAP_I2C_STAT_XUDF; } - } + } while (stat); return count ? IRQ_HANDLED : IRQ_NONE; } -- 1.7.10.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
[PATCH 06/17] i2c: omap: improve 1p153 errata handling
Make it not depend on ISR's local variables in order to make it easier to re-factor the transmit data loop. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 47 + 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 0661ca1..52861c2 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -768,21 +768,24 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id) * data to DATA_REG. Otherwise some data bytes can be lost while transferring * them from the memory to the I2C interface. */ -static int errata_omap3_1p153(struct omap_i2c_dev *dev, u16 *stat, int *err) +static int errata_omap3_1p153(struct omap_i2c_dev *dev) { - unsigned long timeout = 1; + unsigned long timeout = 1; + u16 stat; - while (--timeout !(*stat OMAP_I2C_STAT_XUDF)) { - if (*stat (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) { + do { + stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); + if (stat OMAP_I2C_STAT_XUDF) + break; + + if (stat (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) { omap_i2c_ack_stat(dev, (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)); - *err |= OMAP_I2C_STAT_XUDF; return -ETIMEDOUT; } cpu_relax(); - *stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); - } + } while (--timeout); if (!timeout) { dev_err(dev-dev, timeout waiting on XUDF bit\n); @@ -946,9 +949,18 @@ complete: } } - if ((dev-errata I2C_OMAP3_1P153) - errata_omap3_1p153(dev, stat, err)) - goto complete; + if (dev-errata I2C_OMAP3_1P153) { + int ret; + + ret = errata_omap3_1p153(dev); + stat = omap_i2c_read_reg(dev, + OMAP_I2C_STAT_REG); + + if (ret 0) { + err |= OMAP_I2C_STAT_XUDF; + goto complete; + } + } omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w); } @@ -986,9 +998,18 @@ complete: } } - if ((dev-errata I2C_OMAP3_1P153) - errata_omap3_1p153(dev, stat, err)) - goto complete; + if (dev-errata I2C_OMAP3_1P153) { + int ret; + + ret = errata_omap3_1p153(dev); + stat = omap_i2c_read_reg(dev, + OMAP_I2C_STAT_REG); + + if (ret 0) { + err |= OMAP_I2C_STAT_XUDF; + goto complete; + } + } omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w); } -- 1.7.10.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
[PATCH 05/17] i2c: omap: split out [XR]DR and [XR]RDY
While they do pretty much the same thing, there are a few peculiarities. Specially WRT erratas, it's best to split those out and re-factor the read/write loop to another function which both cases call. This last part will be done on another patch. While at that, also avoid an unncessary register read since dev-fifo_len will always contain the correct amount of data to be transferred. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 126 ++--- 1 file changed, 92 insertions(+), 34 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index fa9ddb6..0661ca1 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -844,36 +844,62 @@ complete: return IRQ_HANDLED; } - if (stat (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR)) { + if (stat OMAP_I2C_STAT_RDR) { u8 num_bytes = 1; + if (dev-fifo_size) + num_bytes = dev-fifo_size; + + while (num_bytes--) { + if (!dev-buf_len) { + dev_err(dev-dev, + RDR IRQ while no data +requested\n); + break; + } + + w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); + *dev-buf++ = w; + dev-buf_len--; + + /* +* Data reg in 2430, omap3 and +* omap4 is 8 bit wide +*/ + if (dev-flags + OMAP_I2C_FLAG_16BIT_DATA_REG) { + if (dev-buf_len) { + *dev-buf++ = w 8; + dev-buf_len--; + } + } + } + if (dev-errata I2C_OMAP_ERRATA_I207) i2c_omap_errata_i207(dev, stat); - if (dev-fifo_size) { - if (stat OMAP_I2C_STAT_RRDY) - num_bytes = dev-fifo_size; - else/* read RXSTAT on RDR interrupt */ - num_bytes = (omap_i2c_read_reg(dev, - OMAP_I2C_BUFSTAT_REG) -8) 0x3F; - } + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_RDR); + continue; + } + + if (stat OMAP_I2C_STAT_RRDY) { + u8 num_bytes = 1; + + if (dev-fifo_size) + num_bytes = dev-fifo_size; + while (num_bytes--) { if (!dev-buf_len) { - if (stat OMAP_I2C_STAT_RRDY) - dev_err(dev-dev, + dev_err(dev-dev, RRDY IRQ while no data -requested\n); - if (stat OMAP_I2C_STAT_RDR) - dev_err(dev-dev, - RDR IRQ while no data -requested\n); +requested\n); break; } w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); *dev-buf++ = w; dev-buf_len--; + /* * Data reg in 2430, omap3 and * omap4 is 8 bit wide @@ -886,36 +912,68 @@ complete: } } } - omap_i2c_ack_stat(dev, (OMAP_I2C_STAT_RRDY | - OMAP_I2C_STAT_RDR)); + + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_RRDY); continue; } - if (stat (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)) { + if (stat OMAP_I2C_STAT_XDR) { u8 num_bytes = 1; -
[PATCH 04/17] i2c: omap: simplify omap_i2c_ack_stat()
stat BIT(1) is the same as BIT(1), so let's simplify things a bit by removing stat from all omap_i2c_ack_stat() calls. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 07ae391..fa9ddb6 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -774,8 +774,8 @@ static int errata_omap3_1p153(struct omap_i2c_dev *dev, u16 *stat, int *err) while (--timeout !(*stat OMAP_I2C_STAT_XUDF)) { if (*stat (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) { - omap_i2c_ack_stat(dev, *stat (OMAP_I2C_STAT_XRDY | - OMAP_I2C_STAT_XDR)); + omap_i2c_ack_stat(dev, (OMAP_I2C_STAT_XRDY | + OMAP_I2C_STAT_XDR)); *err |= OMAP_I2C_STAT_XUDF; return -ETIMEDOUT; } @@ -835,10 +835,11 @@ complete: */ if (stat (OMAP_I2C_STAT_ARDY | OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) { - omap_i2c_ack_stat(dev, stat - (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR | - OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR | - OMAP_I2C_STAT_ARDY)); + omap_i2c_ack_stat(dev, (OMAP_I2C_STAT_RRDY | + OMAP_I2C_STAT_RDR | + OMAP_I2C_STAT_XRDY | + OMAP_I2C_STAT_XDR | + OMAP_I2C_STAT_ARDY)); omap_i2c_complete_cmd(dev, err); return IRQ_HANDLED; } @@ -885,8 +886,8 @@ complete: } } } - omap_i2c_ack_stat(dev, - stat (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR)); + omap_i2c_ack_stat(dev, (OMAP_I2C_STAT_RRDY | + OMAP_I2C_STAT_RDR)); continue; } @@ -933,8 +934,8 @@ complete: omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w); } - omap_i2c_ack_stat(dev, - stat (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)); + omap_i2c_ack_stat(dev, (OMAP_I2C_STAT_XRDY | + OMAP_I2C_STAT_XDR)); continue; } -- 1.7.10.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
[PATCH 03/17] i2c: omap: add blank lines
trivial patch to aid readability. No functional changes. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c |5 + 1 file changed, 5 insertions(+) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 39d5583..07ae391 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -829,6 +829,7 @@ complete: dev_err(dev-dev, Arbitration lost\n); err |= OMAP_I2C_STAT_AL; } + /* * ProDB0017052: Clear ARDY bit twice */ @@ -841,6 +842,7 @@ complete: omap_i2c_complete_cmd(dev, err); return IRQ_HANDLED; } + if (stat (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR)) { u8 num_bytes = 1; @@ -887,6 +889,7 @@ complete: stat (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR)); continue; } + if (stat (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)) { u8 num_bytes = 1; if (dev-fifo_size) { @@ -934,10 +937,12 @@ complete: stat (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)); continue; } + if (stat OMAP_I2C_STAT_ROVR) { dev_err(dev-dev, Receive overrun\n); dev-cmd_err |= OMAP_I2C_STAT_ROVR; } + if (stat OMAP_I2C_STAT_XUDF) { dev_err(dev-dev, Transmit underflow\n); dev-cmd_err |= OMAP_I2C_STAT_XUDF; -- 1.7.10.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
[PATCH 01/17] i2c: omap: simplify num_bytes handling
trivial patch, no functional changes Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c |6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 801df60..9b532cd 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -855,8 +855,7 @@ complete: OMAP_I2C_BUFSTAT_REG) 8) 0x3F; } - while (num_bytes) { - num_bytes--; + while (num_bytes--) { w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); if (dev-buf_len) { *dev-buf++ = w; @@ -898,8 +897,7 @@ complete: OMAP_I2C_BUFSTAT_REG) 0x3F; } - while (num_bytes) { - num_bytes--; + while (num_bytes--) { w = 0; if (dev-buf_len) { w = *dev-buf++; -- 1.7.10.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
[PATCH 02/17] i2c: omap: decrease indentation level on data handling
trivial patch, no functional changes. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 63 - 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 9b532cd..39d5583 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -856,22 +856,7 @@ complete: 8) 0x3F; } while (num_bytes--) { - w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); - if (dev-buf_len) { - *dev-buf++ = w; - dev-buf_len--; - /* -* Data reg in 2430, omap3 and -* omap4 is 8 bit wide -*/ - if (dev-flags -OMAP_I2C_FLAG_16BIT_DATA_REG) { - if (dev-buf_len) { - *dev-buf++ = w 8; - dev-buf_len--; - } - } - } else { + if (!dev-buf_len) { if (stat OMAP_I2C_STAT_RRDY) dev_err(dev-dev, RRDY IRQ while no data @@ -882,6 +867,21 @@ complete: requested\n); break; } + + w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); + *dev-buf++ = w; + dev-buf_len--; + /* +* Data reg in 2430, omap3 and +* omap4 is 8 bit wide +*/ + if (dev-flags + OMAP_I2C_FLAG_16BIT_DATA_REG) { + if (dev-buf_len) { + *dev-buf++ = w 8; + dev-buf_len--; + } + } } omap_i2c_ack_stat(dev, stat (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR)); @@ -898,22 +898,7 @@ complete: 0x3F; } while (num_bytes--) { - w = 0; - if (dev-buf_len) { - w = *dev-buf++; - dev-buf_len--; - /* -* Data reg in 2430, omap3 and -* omap4 is 8 bit wide -*/ - if (dev-flags -OMAP_I2C_FLAG_16BIT_DATA_REG) { - if (dev-buf_len) { - w |= *dev-buf++ 8; - dev-buf_len--; - } - } - } else { + if (!dev-buf_len) { if (stat OMAP_I2C_STAT_XRDY) dev_err(dev-dev, XRDY IRQ while no @@ -925,6 +910,20 @@ complete: break; } + w = *dev-buf++; + dev-buf_len--; + /* +* Data reg in 2430, omap3 and +* omap4 is 8 bit wide +*/ + if (dev-flags + OMAP_I2C_FLAG_16BIT_DATA_REG) { + if (dev-buf_len) { + w |= *dev-buf++ 8; + dev-buf_len--; + } +
[PATCH v3 0/4] dt: device tree support for TI EMIF driver for 3.6
Tony, Here is the EMIF driver DT support which was kept on hold for the driver to get merged. The series has been already reviewed on the list. This series adds device tree support for TI EMIF SDRAM controller driver. For this, a binding has been added for representing AC timing parameters and other details of LPDDR2 memories. v3: Rebased against the 3.5-rc2. The series is aslo available at: git://github.com/SantoshShilimkar/linux.git for_3.6/emif_dt Aneesh V (4): dt: device tree bindings for LPDDR2 memories dt: emif: device tree bindings for TI's EMIF sdram controller arm: dts: EMIF and LPDDR2 device tree data for OMAP4 boards memory: emif: add device tree support to emif driver .../devicetree/bindings/lpddr2/lpddr2-timings.txt | 52 .../devicetree/bindings/lpddr2/lpddr2.txt | 102 +++ .../bindings/memory-controllers/ti/emif.txt| 55 arch/arm/boot/dts/elpida_ecb240abacn.dtsi | 67 + arch/arm/boot/dts/omap4-panda.dts | 13 + arch/arm/boot/dts/omap4-sdp.dts| 13 + arch/arm/boot/dts/omap4.dtsi | 18 ++ drivers/memory/emif.c | 285 +++- 8 files changed, 604 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/lpddr2/lpddr2-timings.txt create mode 100644 Documentation/devicetree/bindings/lpddr2/lpddr2.txt create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti/emif.txt create mode 100644 arch/arm/boot/dts/elpida_ecb240abacn.dtsi -- 1.7.9.5 -- 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
[PATCH v3 1/4] dt: device tree bindings for LPDDR2 memories
From: Aneesh V ane...@ti.com device tree bindings for LPDDR2 SDRAM memories compliant to JESD209-2 standard. The 'lpddr2' binding in-turn uses another binding 'lpddr2-timings' for specifying the AC timing parameters of the memory device at different speed-bins. Reviewed-by: Benoit Cousson b-cous...@ti.com Reviewed-by: Grant Likely grant.lik...@secretlab.ca Tested-by: Lokesh Vutla lokeshvu...@ti.com Signed-off-by: Aneesh V ane...@ti.com [santosh.shilim...@ti.com: Rebased against 3.5-rc] Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- .../devicetree/bindings/lpddr2/lpddr2-timings.txt | 52 ++ .../devicetree/bindings/lpddr2/lpddr2.txt | 102 2 files changed, 154 insertions(+) create mode 100644 Documentation/devicetree/bindings/lpddr2/lpddr2-timings.txt create mode 100644 Documentation/devicetree/bindings/lpddr2/lpddr2.txt diff --git a/Documentation/devicetree/bindings/lpddr2/lpddr2-timings.txt b/Documentation/devicetree/bindings/lpddr2/lpddr2-timings.txt new file mode 100644 index 000..9ceb19e --- /dev/null +++ b/Documentation/devicetree/bindings/lpddr2/lpddr2-timings.txt @@ -0,0 +1,52 @@ +* AC timing parameters of LPDDR2(JESD209-2) memories for a given speed-bin + +Required properties: +- compatible : Should be jedec,lpddr2-timings +- min-freq : minimum DDR clock frequency for the speed-bin. Type is u32 +- max-freq : maximum DDR clock frequency for the speed-bin. Type is u32 + +Optional properties: + +The following properties represent AC timing parameters from the memory +data-sheet of the device for a given speed-bin. All these properties are +of type u32 and the default unit is ps (pico seconds). Parameters with +a different unit have a suffix indicating the unit such as 'tRAS-max-ns' +- tRCD +- tWR +- tRAS-min +- tRRD +- tWTR +- tXP +- tRTP +- tDQSCK-max +- tFAW +- tZQCS +- tZQinit +- tRPab +- tZQCL +- tCKESR +- tRAS-max-ns +- tDQSCK-max-derated + +Example: + +timings_elpida_ECB240ABACN_400mhz: lpddr2-timings@0 { + compatible = jedec,lpddr2-timings; + min-freq= 1000; + max-freq= 4; + tRPab = 21000; + tRCD= 18000; + tWR = 15000; + tRAS-min= 42000; + tRRD= 1; + tWTR= 7500; + tXP = 7500; + tRTP= 7500; + tCKESR = 15000; + tDQSCK-max = 5500; + tFAW= 5; + tZQCS = 9; + tZQCL = 36; + tZQinit = 100; + tRAS-max-ns = 7; +}; diff --git a/Documentation/devicetree/bindings/lpddr2/lpddr2.txt b/Documentation/devicetree/bindings/lpddr2/lpddr2.txt new file mode 100644 index 000..58354a0 --- /dev/null +++ b/Documentation/devicetree/bindings/lpddr2/lpddr2.txt @@ -0,0 +1,102 @@ +* LPDDR2 SDRAM memories compliant to JEDEC JESD209-2 + +Required properties: +- compatible : Should be one of - jedec,lpddr2-nvm, jedec,lpddr2-s2, + jedec,lpddr2-s4 + + ti,jedec-lpddr2-s2 should be listed if the memory part is LPDDR2-S2 type + + ti,jedec-lpddr2-s4 should be listed if the memory part is LPDDR2-S4 type + + ti,jedec-lpddr2-nvm should be listed if the memory part is LPDDR2-NVM type + +- density : u32 representing density in Mb (Mega bits) + +- io-width : u32 representing bus width. Possible values are 8, 16, and 32 + +Optional properties: + +The following optional properties represent the minimum value of some AC +timing parameters of the DDR device in terms of number of clock cycles. +These values shall be obtained from the device data-sheet. +- tRRD-min-tck +- tWTR-min-tck +- tXP-min-tck +- tRTP-min-tck +- tCKE-min-tck +- tRPab-min-tck +- tRCD-min-tck +- tWR-min-tck +- tRASmin-min-tck +- tCKESR-min-tck +- tFAW-min-tck + +Child nodes: +- The lpddr2 node may have one or more child nodes of type lpddr2-timings. + lpddr2-timings provides AC timing parameters of the device for + a given speed-bin. The user may provide the timings for as many + speed-bins as is required. Please see Documentation/devicetree/ + bindings/lpddr2/lpddr2-timings.txt for more information on lpddr2-timings + +Example: + +elpida_ECB240ABACN : lpddr2 { + compatible = Elpida,ECB240ABACN,jedec,lpddr2-s4; + density = 2048; + io-width= 32; + + tRPab-min-tck = 3; + tRCD-min-tck= 3; + tWR-min-tck = 3; + tRASmin-min-tck = 3; + tRRD-min-tck= 2; + tWTR-min-tck= 2; + tXP-min-tck = 2; + tRTP-min-tck= 2; + tCKE-min-tck= 3; + tCKESR-min-tck = 3; + tFAW-min-tck= 8; + + timings_elpida_ECB240ABACN_400mhz: lpddr2-timings@0 { + compatible = jedec,lpddr2-timings; + min-freq= 1000; + max-freq= 4; + tRPab = 21000; + tRCD= 18000; +
[PATCH v3 3/4] arm: dts: EMIF and LPDDR2 device tree data for OMAP4 boards
From: Aneesh V ane...@ti.com Device tree data for the EMIF sdram controllers in OMAP4 and LPDDR2 memory devices attached to OMAP4 boards. Reviewed-by: Benoit Cousson b-cous...@ti.com Reviewed-by: Grant Likely grant.lik...@secretlab.ca Tested-by: Lokesh Vutla lokeshvu...@ti.com Signed-off-by: Aneesh V ane...@ti.com [santosh.shilim...@ti.com: Rebased against 3.5-rc] Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- arch/arm/boot/dts/elpida_ecb240abacn.dtsi | 67 + arch/arm/boot/dts/omap4-panda.dts | 13 ++ arch/arm/boot/dts/omap4-sdp.dts | 13 ++ arch/arm/boot/dts/omap4.dtsi | 18 4 files changed, 111 insertions(+) create mode 100644 arch/arm/boot/dts/elpida_ecb240abacn.dtsi diff --git a/arch/arm/boot/dts/elpida_ecb240abacn.dtsi b/arch/arm/boot/dts/elpida_ecb240abacn.dtsi new file mode 100644 index 000..f97f70f --- /dev/null +++ b/arch/arm/boot/dts/elpida_ecb240abacn.dtsi @@ -0,0 +1,67 @@ +/* + * Common devices used in different OMAP boards + */ + +/ { + elpida_ECB240ABACN: lpddr2 { + compatible = Elpida,ECB240ABACN,jedec,lpddr2-s4; + density = 2048; + io-width= 32; + + tRPab-min-tck = 3; + tRCD-min-tck= 3; + tWR-min-tck = 3; + tRASmin-min-tck = 3; + tRRD-min-tck= 2; + tWTR-min-tck= 2; + tXP-min-tck = 2; + tRTP-min-tck= 2; + tCKE-min-tck= 3; + tCKESR-min-tck = 3; + tFAW-min-tck= 8; + + timings_elpida_ECB240ABACN_400mhz: lpddr2-timings@0 { + compatible = jedec,lpddr2-timings; + min-freq= 1000; + max-freq= 4; + tRPab = 21000; + tRCD= 18000; + tWR = 15000; + tRAS-min= 42000; + tRRD= 1; + tWTR= 7500; + tXP = 7500; + tRTP= 7500; + tCKESR = 15000; + tDQSCK-max = 5500; + tFAW= 5; + tZQCS = 9; + tZQCL = 36; + tZQinit = 100; + tRAS-max-ns = 7; + tDQSCK-max-derated = 6000; + }; + + timings_elpida_ECB240ABACN_200mhz: lpddr2-timings@1 { + compatible = jedec,lpddr2-timings; + min-freq= 1000; + max-freq= 2; + tRPab = 21000; + tRCD= 18000; + tWR = 15000; + tRAS-min= 42000; + tRRD= 1; + tWTR= 1; + tXP = 7500; + tRTP= 7500; + tCKESR = 15000; + tDQSCK-max = 5500; + tFAW= 5; + tZQCS = 9; + tZQCL = 36; + tZQinit = 100; + tRAS-max-ns = 7; + tDQSCK-max-derated = 6000; + }; + }; +}; diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts index 1efe0c5..08a0b21 100644 --- a/arch/arm/boot/dts/omap4-panda.dts +++ b/arch/arm/boot/dts/omap4-panda.dts @@ -8,6 +8,7 @@ /dts-v1/; /include/ omap4.dtsi +/include/ elpida_ecb240abacn.dtsi / { model = TI OMAP4 PandaBoard; @@ -32,6 +33,18 @@ linux,default-trigger = mmc0; }; }; + + ocp { + emif1: emif@0x4c00 { + cs1-used; + device-handle = elpida_ECB240ABACN; + }; + + emif2: emif@0x4d00 { + cs1-used; + device-handle = elpida_ECB240ABACN; + }; + }; }; i2c1 { diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts index d08c4d1..562f03e 100644 --- a/arch/arm/boot/dts/omap4-sdp.dts +++ b/arch/arm/boot/dts/omap4-sdp.dts @@ -8,6 +8,7 @@ /dts-v1/; /include/ omap4.dtsi +/include/ elpida_ecb240abacn.dtsi / { model = TI OMAP4 SDP board; @@ -70,6 +71,18 @@ gpios = gpio5 11 0; /* 139 */ }; }; + + ocp { +
[PATCH v3 2/4] dt: emif: device tree bindings for TI's EMIF sdram controller
From: Aneesh V ane...@ti.com EMIF - External Memory Interface - is an SDRAM controller used in TI SoCs. EMIF supports, based on the IP revision, one or more of DDR2/DDR3/LPDDR2 protocols. This binding describes a given instance of the EMIF IP and memory parts attached to it. Reviewed-by: Benoit Cousson b-cous...@ti.com Reviewed-by: Grant Likely grant.lik...@secretlab.ca Tested-by: Lokesh Vutla lokeshvu...@ti.com Signed-off-by: Aneesh V ane...@ti.com [santosh.shilim...@ti.com: Rebased against 3.5-rc] Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- .../bindings/memory-controllers/ti/emif.txt| 55 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti/emif.txt diff --git a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt new file mode 100644 index 000..938f8e1 --- /dev/null +++ b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt @@ -0,0 +1,55 @@ +* EMIF family of TI SDRAM controllers + +EMIF - External Memory Interface - is an SDRAM controller used in +TI SoCs. EMIF supports, based on the IP revision, one or more of +DDR2/DDR3/LPDDR2 protocols. This binding describes a given instance +of the EMIF IP and memory parts attached to it. + +Required properties: +- compatible : Should be of the form ti,emif-ip-rev where ip-rev + is the IP revision of the specific EMIF instance. + +- phy-type : u32 indicating the DDR phy type. Following are the + allowed values + 1 : Attila PHY + 2 : Intelli PHY + +- device-handle: phandle to a lpddr2 node representing the memory part + +- ti,hwmods: For TI hwmods processing and omap device creation + the value shall be emifn where n is the number of the EMIF + instance with base 1. + +Optional properties: +- cs1-used : Have this property if CS1 of this EMIF + instance has a memory part attached to it. If there is a memory + part attached to CS1, it should be the same type as the one on CS0, + so there is no need to give the details of this memory part. + +- cal-resistor-per-cs : Have this property if the board has one + calibration resistor per chip-select. + +- hw-caps-read-idle-ctrl: Have this property if the controller + supports read idle window programming + +- hw-caps-dll-calib-ctrl: Have this property if the controller + supports dll calibration control + +- hw-caps-ll-interface : Have this property if the controller + has a low latency interface and corresponding interrupt events + +- hw-caps-temp-alert : Have this property if the controller + has capability for generating SDRAM temperature alerts + +Example: + +emif1: emif@0x4c00 { + compatible = ti,emif-4d; + ti,hwmods = emif2; + phy-type= 1; + device-handle = elpida_ECB240ABACN; + cs1-used; + hw-caps-read-idle-ctrl; + hw-caps-ll-interface; + hw-caps-temp-alert; +}; -- 1.7.9.5 -- 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
[PATCH v3 4/4] memory: emif: add device tree support to emif driver
From: Aneesh V ane...@ti.com Device tree support for the EMIF driver. Reviewed-by: Benoit Cousson b-cous...@ti.com Reviewed-by: Grant Likely grant.lik...@secretlab.ca Tested-by: Lokesh Vutla lokeshvu...@ti.com Signed-off-by: Aneesh V ane...@ti.com [santosh.shilim...@ti.com: Rebased against 3.5-rc] Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org --- drivers/memory/emif.c | 285 - 1 file changed, 284 insertions(+), 1 deletion(-) diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c index 33a4396..10a2955 100644 --- a/drivers/memory/emif.c +++ b/drivers/memory/emif.c @@ -18,6 +18,7 @@ #include linux/platform_device.h #include linux/interrupt.h #include linux/slab.h +#include linux/of.h #include linux/debugfs.h #include linux/seq_file.h #include linux/module.h @@ -49,6 +50,7 @@ * frequency in effect at the moment) * @plat_data: Pointer to saved platform data. * @debugfs_root: dentry to the root folder for EMIF in debugfs + * @np_ddr:Pointer to ddr device tree node */ struct emif_data { u8 duplicate; @@ -63,6 +65,7 @@ struct emif_data { struct emif_regs*curr_regs; struct emif_platform_data *plat_data; struct dentry *debugfs_root; + struct device_node *np_ddr; }; static struct emif_data *emif1; @@ -1148,6 +1151,270 @@ static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs, return valid; } +#if defined(CONFIG_OF) +static void __init_or_module of_get_custom_configs(struct device_node *np_emif, + struct emif_data *emif) +{ + struct emif_custom_configs *cust_cfgs = NULL; + int len; + const int *lpmode, *poll_intvl; + + lpmode = of_get_property(np_emif, low-power-mode, len); + poll_intvl = of_get_property(np_emif, temp-alert-poll-interval, len); + + if (lpmode || poll_intvl) + cust_cfgs = devm_kzalloc(emif-dev, sizeof(*cust_cfgs), + GFP_KERNEL); + + if (!cust_cfgs) + return; + + if (lpmode) { + cust_cfgs-mask |= EMIF_CUSTOM_CONFIG_LPMODE; + cust_cfgs-lpmode = *lpmode; + of_property_read_u32(np_emif, + low-power-mode-timeout-performance, + cust_cfgs-lpmode_timeout_performance); + of_property_read_u32(np_emif, + low-power-mode-timeout-power, + cust_cfgs-lpmode_timeout_power); + of_property_read_u32(np_emif, + low-power-mode-freq-threshold, + cust_cfgs-lpmode_freq_threshold); + } + + if (poll_intvl) { + cust_cfgs-mask |= + EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL; + cust_cfgs-temp_alert_poll_interval_ms = *poll_intvl; + } + + if (!is_custom_config_valid(cust_cfgs, emif-dev)) { + devm_kfree(emif-dev, cust_cfgs); + return; + } + + emif-plat_data-custom_configs = cust_cfgs; +} + +static void __init_or_module of_get_min_tck(struct device_node *np, + struct emif_data *emif) +{ + int ret = 0; + struct lpddr2_min_tck *min; + + min = devm_kzalloc(emif-dev, sizeof(*min), GFP_KERNEL); + if (!min) + goto default_min_tck; + + ret |= of_property_read_u32(np, tRPab-min-tck, min-tRPab); + ret |= of_property_read_u32(np, tRCD-min-tck, min-tRCD); + ret |= of_property_read_u32(np, tWR-min-tck, min-tWR); + ret |= of_property_read_u32(np, tRASmin-min-tck, min-tRASmin); + ret |= of_property_read_u32(np, tRRD-min-tck, min-tRRD); + ret |= of_property_read_u32(np, tWTR-min-tck, min-tWTR); + ret |= of_property_read_u32(np, tXP-min-tck, min-tXP); + ret |= of_property_read_u32(np, tRTP-min-tck, min-tRTP); + ret |= of_property_read_u32(np, tCKE-min-tck, min-tCKE); + ret |= of_property_read_u32(np, tCKESR-min-tck, min-tCKESR); + ret |= of_property_read_u32(np, tFAW-min-tck, min-tFAW); + + if (ret) { + devm_kfree(emif-dev, min); + goto default_min_tck; + } + + emif-plat_data-min_tck = min; + return; + +default_min_tck: + dev_warn(emif-dev, %s: using default min-tck values\n, __func__); + emif-plat_data-min_tck = lpddr2_jedec_min_tck; +} + +static int __init_or_module of_do_get_timings(struct device_node *np, + struct lpddr2_timings *tim) +{ + int ret; + + ret = of_property_read_u32(np, max-freq, tim-max_freq); + ret |=
Re: [PATCH 01/17] i2c: omap: simplify num_bytes handling
On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: trivial patch, no functional changes Signed-off-by: Felipe Balbi ba...@ti.com --- Looks good. Reviewed-by : Santosh Shilimkar santosh.shilim...@ti.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 3/3] ARM: OMAP2+: gpmc: handle additional timings
Hi Tony, On Thu, Jun 14, 2012 at 15:49:02, Tony Lindgren wrote: Well I took a look at the values, and it seems the only difference is the static GPMC_CONFIG1_CLKACTIVATIONTIME(1) that your patch now overwrites 0. It seems change below should be part of $subject. Please let me know your comments Regards Afzal diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c index fd4c48d..938896c 100644 --- a/arch/arm/mach-omap2/gpmc-onenand.c +++ b/arch/arm/mach-omap2/gpmc-onenand.c @@ -321,6 +321,8 @@ static int omap2_onenand_set_sync_mode(struct omap_onenand_platform_data *cfg, t.rd_cycle = gpmc_ticks_to_ns(fclk_offset + (latency + 1) * div + ticks_cez); + t.clk_activation = fclk_offset_ns; + /* Write */ if (sync_write) { t.adv_wr_off = t.adv_rd_off; @@ -354,7 +356,6 @@ static int omap2_onenand_set_sync_mode(struct omap_onenand_platform_data *cfg, (sync_read ? GPMC_CONFIG1_READTYPE_SYNC : 0) | (sync_write ? GPMC_CONFIG1_WRITEMULTIPLE_SUPP : 0) | (sync_write ? GPMC_CONFIG1_WRITETYPE_SYNC : 0) | - GPMC_CONFIG1_CLKACTIVATIONTIME(fclk_offset) | GPMC_CONFIG1_PAGE_LEN(2) | (cpu_is_omap34xx() ? 0 : (GPMC_CONFIG1_WAIT_READ_MON | diff --git a/arch/arm/mach-omap2/usb-tusb6010.c b/arch/arm/mach-omap2/usb-tusb6010.c index db84a46..5c98755 100644 --- a/arch/arm/mach-omap2/usb-tusb6010.c +++ b/arch/arm/mach-omap2/usb-tusb6010.c @@ -174,6 +174,8 @@ static int tusb_set_sync_mode(unsigned sysclk_ps, unsigned fclk_ps) tmp = t.cs_wr_off * 1000 + 7000 /* t_scsn_rdy_z */; t.wr_cycle = next_clk(t.cs_wr_off, tmp, fclk_ps); + t.clk_activation = gpmc_ticks_to_ns(1); + return gpmc_cs_set_timings(sync_cs, t); } @@ -283,7 +285,6 @@ tusb6010_setup_interface(struct musb_hdrc_platform_data *data, | GPMC_CONFIG1_READTYPE_SYNC | GPMC_CONFIG1_WRITEMULTIPLE_SUPP | GPMC_CONFIG1_WRITETYPE_SYNC - | GPMC_CONFIG1_CLKACTIVATIONTIME(1) | GPMC_CONFIG1_PAGE_LEN(2) | GPMC_CONFIG1_WAIT_READ_MON | GPMC_CONFIG1_WAIT_WRITE_MON -- 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 02/17] i2c: omap: decrease indentation level on data handling
On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: trivial patch, no functional changes. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 63 - 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 9b532cd..39d5583 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -856,22 +856,7 @@ complete: 8) 0x3F; } while (num_bytes--) { - w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); - if (dev-buf_len) { - *dev-buf++ = w; - dev-buf_len--; - /* - * Data reg in 2430, omap3 and - * omap4 is 8 bit wide - */ - if (dev-flags - OMAP_I2C_FLAG_16BIT_DATA_REG) { - if (dev-buf_len) { - *dev-buf++ = w 8; - dev-buf_len--; - } - } - } else { + if (!dev-buf_len) { if (stat OMAP_I2C_STAT_RRDY) dev_err(dev-dev, RRDY IRQ while no data @@ -882,6 +867,21 @@ complete: requested\n); break; } + + w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); + *dev-buf++ = w; + dev-buf_len--; + /* + * Data reg in 2430, omap3 and + * omap4 is 8 bit wide + */ + if (dev-flags + OMAP_I2C_FLAG_16BIT_DATA_REG) { + if (dev-buf_len) { + *dev-buf++ = w 8; + dev-buf_len--; + } + } } omap_i2c_ack_stat(dev, stat (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR)); @@ -898,22 +898,7 @@ complete: 0x3F; } while (num_bytes--) { - w = 0; - if (dev-buf_len) { - w = *dev-buf++; - dev-buf_len--; - /* - * Data reg in 2430, omap3 and - * omap4 is 8 bit wide - */ - if (dev-flags - OMAP_I2C_FLAG_16BIT_DATA_REG) { - if (dev-buf_len) { - w |= *dev-buf++ 8; - dev-buf_len--; - } - } - } else { + if (!dev-buf_len) { if (stat OMAP_I2C_STAT_XRDY) dev_err(dev-dev, XRDY IRQ while no @@ -925,6 +910,20 @@ complete: break; } + w = *dev-buf++; + dev-buf_len--; + /* + * Data reg in 2430, omap3 and + * omap4 is 8 bit wide + */ + if (dev-flags + OMAP_I2C_FLAG_16BIT_DATA_REG) { + if (dev-buf_len) { + w |=
Re: [PATCH 02/17] i2c: omap: decrease indentation level on data handling
On Thu, Jun 14, 2012 at 04:11:14PM +0530, Shilimkar, Santosh wrote: On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: trivial patch, no functional changes. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 63 - 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 9b532cd..39d5583 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -856,22 +856,7 @@ complete: 8) 0x3F; } while (num_bytes--) { - w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); - if (dev-buf_len) { - *dev-buf++ = w; - dev-buf_len--; - /* - * Data reg in 2430, omap3 and - * omap4 is 8 bit wide - */ - if (dev-flags - OMAP_I2C_FLAG_16BIT_DATA_REG) { - if (dev-buf_len) { - *dev-buf++ = w 8; - dev-buf_len--; - } - } - } else { + if (!dev-buf_len) { if (stat OMAP_I2C_STAT_RRDY) dev_err(dev-dev, RRDY IRQ while no data @@ -882,6 +867,21 @@ complete: requested\n); break; } + + w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); + *dev-buf++ = w; + dev-buf_len--; + /* + * Data reg in 2430, omap3 and + * omap4 is 8 bit wide + */ + if (dev-flags + OMAP_I2C_FLAG_16BIT_DATA_REG) { + if (dev-buf_len) { + *dev-buf++ = w 8; + dev-buf_len--; + } + } } omap_i2c_ack_stat(dev, stat (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR)); @@ -898,22 +898,7 @@ complete: 0x3F; } while (num_bytes--) { - w = 0; - if (dev-buf_len) { - w = *dev-buf++; - dev-buf_len--; - /* - * Data reg in 2430, omap3 and - * omap4 is 8 bit wide - */ - if (dev-flags - OMAP_I2C_FLAG_16BIT_DATA_REG) { - if (dev-buf_len) { - w |= *dev-buf++ 8; - dev-buf_len--; - } - } - } else { + if (!dev-buf_len) { if (stat OMAP_I2C_STAT_XRDY) dev_err(dev-dev, XRDY IRQ while no @@ -925,6 +910,20 @@ complete: break; } + w = *dev-buf++; + dev-buf_len--; + /* + * Data reg in 2430, omap3 and + * omap4 is 8 bit wide + */ + if (dev-flags +
Re: [PATCH 03/17] i2c: omap: add blank lines
On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: trivial patch to aid readability. No functional changes. Signed-off-by: Felipe Balbi ba...@ti.com --- Not sure if it is needed but may be spliting the series into clean up and functional update like logic changes etc may be better for merge as well as testing. Regards santosh -- 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 04/17] i2c: omap: simplify omap_i2c_ack_stat()
On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: stat BIT(1) is the same as BIT(1), so let's simplify things a bit by removing stat from all omap_i2c_ack_stat() calls. Signed-off-by: Felipe Balbi ba...@ti.com --- Indeed. Looks good to me. Reviewed-by : Santosh Shilimkar santosh.shilim...@ti.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 v2 0/4] ARM: OMAP2+: dmtimer: cleanup related to devm API and clk usage
On Fri, Apr 20, 2012 at 6:09 PM, Tarun Kanti DebBarma tarun.ka...@ti.com wrote: The devm API usage in probe() simplifies error handling operation. Since iclk is not used in the driver it is removed from wherever not needed. Corrected the timer fck name mis-match between clock44xx_data.c and omap_hwmod_44xx_data.c. Added omap_hwmod_get_main_clk() API. There is no more need to construct clock names using sprintf() to be used in clk_get() during initialization. Reference: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Commit: e816b57a337ea3b755de72bec38c10c864f23015 (Linux 3.4-rc3) Series is available here for reference: git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev dmtimer_cleanup_for_3.5 Tested on following platforms: OMAP5, OMAP4430SDP, OMAP3430SDP, OMAP2430SDP. Could not test on OMAP2420 due to unavailability of board. v2: - Use devm_request_and_ioremap() instead of request_mem_region() and ioremap() - Add omap_hwmod_get_main_clk() API - Reverted changes of clock names from OMAP2 and OMAP3 platforms Cc: Cousson, Benoit b-cous...@ti.com Cc: Paul Walmsley p...@pwsan.com Cc: Tony Lindgren t...@atomide.com Cc: Kevin Hilman khil...@ti.com Cc: Rajendra Nayak rna...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Tarun Kanti DebBarma (4): ARM: OMAP: dmtimer: use devm_ API and do some cleanup in probe() ARM: OMAP2+: hwmod: add omap_hwmod_get_main_clk() API ARM: OMAP2+: dmtimer: cleanup iclk usage ARM: OMAP2+: dmtimer: cleanup fclk usage [ping] Only one of the patches in the series (ARM: OMAP2+: dmtimer: cleanup iclk usage) has been taken. I just applied the remaining patches on top of Linux 3.5-rc2 and tested on OMAP3 and OMAP4. Can the remaining patches be taken as well? -- Tarun arch/arm/mach-omap2/clock44xx_data.c | 33 ++--- arch/arm/mach-omap2/omap_hwmod.c | 15 arch/arm/mach-omap2/timer.c | 10 +- arch/arm/plat-omap/dmtimer.c | 51 -- arch/arm/plat-omap/include/plat/dmtimer.h | 2 +- arch/arm/plat-omap/include/plat/omap_hwmod.h | 2 + 6 files changed, 46 insertions(+), 67 deletions(-) -- 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 v2 0/4] ARM: OMAP2+: dmtimer: cleanup related to devm API and clk usage
Tony, On Thu, Jun 14, 2012 at 4:24 PM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Fri, Apr 20, 2012 at 6:09 PM, Tarun Kanti DebBarma tarun.ka...@ti.com wrote: The devm API usage in probe() simplifies error handling operation. Since iclk is not used in the driver it is removed from wherever not needed. Corrected the timer fck name mis-match between clock44xx_data.c and omap_hwmod_44xx_data.c. Added omap_hwmod_get_main_clk() API. There is no more need to construct clock names using sprintf() to be used in clk_get() during initialization. Reference: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Commit: e816b57a337ea3b755de72bec38c10c864f23015 (Linux 3.4-rc3) Series is available here for reference: git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev dmtimer_cleanup_for_3.5 Tested on following platforms: OMAP5, OMAP4430SDP, OMAP3430SDP, OMAP2430SDP. Could not test on OMAP2420 due to unavailability of board. v2: - Use devm_request_and_ioremap() instead of request_mem_region() and ioremap() - Add omap_hwmod_get_main_clk() API - Reverted changes of clock names from OMAP2 and OMAP3 platforms Cc: Cousson, Benoit b-cous...@ti.com Cc: Paul Walmsley p...@pwsan.com Cc: Tony Lindgren t...@atomide.com Cc: Kevin Hilman khil...@ti.com Cc: Rajendra Nayak rna...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Tarun Kanti DebBarma (4): ARM: OMAP: dmtimer: use devm_ API and do some cleanup in probe() ARM: OMAP2+: hwmod: add omap_hwmod_get_main_clk() API ARM: OMAP2+: dmtimer: cleanup iclk usage ARM: OMAP2+: dmtimer: cleanup fclk usage [ping] Only one of the patches in the series (ARM: OMAP2+: dmtimer: cleanup iclk usage) has been taken. I just applied the remaining patches on top of Linux 3.5-rc2 and tested on OMAP3 and OMAP4. Can the remaining patches be taken as well? Just to let you know that this cleanup is essential for Benoit's OMAP4 hwmod cleanup and OMAP5 minimal support series. Regards santosh -- 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 06/17] i2c: omap: improve 1p153 errata handling
On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: Make it not depend on ISR's local variables in order to make it easier to re-factor the transmit data loop. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 47 + 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 0661ca1..52861c2 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -768,21 +768,24 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id) * data to DATA_REG. Otherwise some data bytes can be lost while transferring * them from the memory to the I2C interface. */ -static int errata_omap3_1p153(struct omap_i2c_dev *dev, u16 *stat, int *err) +static int errata_omap3_1p153(struct omap_i2c_dev *dev) { - unsigned long timeout = 1; + unsigned long timeout = 1; Not related to this patch but this 1 timeout magic needs to be fixed as well. The patch looks fine. Regards santosh -- 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 08/17] i2c: omap: switch over to do {} while loop
On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: this will make sure that we execute at least once. No functional changes otherwise. Signed-off-by: Felipe Balbi ba...@ti.com --- Executing at least once instead of never is still a functional change :-) Regards Santosh -- 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 08/17] i2c: omap: switch over to do {} while loop
On Thu, Jun 14, 2012 at 04:33:39PM +0530, Shilimkar, Santosh wrote: On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: this will make sure that we execute at least once. No functional changes otherwise. Signed-off-by: Felipe Balbi ba...@ti.com --- Executing at least once instead of never is still a functional change :-) there's a check for spurious interrupts. The idea was mainly to initialise stat and bits correctly at the beginning of the handler. Besides that otherwise is telling you that: except this, there are no other functional changes ;) -- balbi signature.asc Description: Digital signature
Re: [PATCH 09/17] i2c: omap: ack IRQ in parts
On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: According to flow diagrams on OMAP TRMs, we should ACK the IRQ as they happen. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index f978b14..c00ba7d 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -894,21 +894,20 @@ omap_i2c_isr(int this_irq, void *dev_id) } complete: - /* - * Ack the stat in one go, but [R/X]DR and [R/X]RDY should be - * acked after the data operation is complete. - * Ref: TRM SWPU114Q Figure 18-31 - */ - omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, stat - ~(OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR | - OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)); - - if (stat OMAP_I2C_STAT_NACK) + if (stat OMAP_I2C_STAT_NACK) { + dev_err(dev-dev, No Acknowledge\n); err |= OMAP_I2C_STAT_NACK; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; + } Do you think making I2C IRQ ack + complete as one small static inline function can save clean the code further. I see it has been used in multiple paths. -- 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 04/17] i2c: omap: simplify omap_i2c_ack_stat()
On Thu, Jun 14, 2012 at 01:20:37PM +0300, Felipe Balbi wrote: stat BIT(1) is the same as BIT(1), so let's simplify things a bit by removing stat from all omap_i2c_ack_stat() calls. This doesn't feel right, and the explanation is definitely wrong. stat BIT(1) is not the same as BIT(1) _unless_ you're saying that stat always has BIT(1) already set. Can you guarantee that in this code? If so, how? What happens if you read the status register, and it has bit 1 clear. immediately after the read, the status register bit 1 becomes set, and then you write bit 1 set (because you've dropped the stat BIT(1) from the code.) Is it not going to acknowledge that bit-1-set but because you haven't read it, you're going to miss that event? This feels like a buggy change to me. -- 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 06/17] i2c: omap: improve 1p153 errata handling
On Thu, Jun 14, 2012 at 01:20:39PM +0300, Felipe Balbi wrote: -static int errata_omap3_1p153(struct omap_i2c_dev *dev, u16 *stat, int *err) +static int errata_omap3_1p153(struct omap_i2c_dev *dev) { - unsigned long timeout = 1; + unsigned long timeout = 1; + u16 stat; Eww, no, not more of this lets add tabs to align auto variable declarations. This is detrimental when you add another variable whose type is longer than the current indentation - you have to reformat the entire block. It's really not worth it. Stick to just using one space, like the rest of the kernel code. Like the code which Linus writes. And thereby help to avoid future pointless churn whinges. -- 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 15/17] i2c: omap: always return IRQ_HANDLED
On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: otherwise we could get our IRQ line disabled due to many spurious IRQs. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index fc5b8bc..5b78a73 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1015,7 +1015,7 @@ omap_i2c_isr(int this_irq, void *dev_id) } } while (stat); - return count ? IRQ_HANDLED : IRQ_NONE; + return IRQ_HANDLED; no sure if this is correct. if you have IRQ flood and instead of _actually_ handling it, if you return handled, you still have interrupt pending, right? if (count++ == 100) { dev_warn(dev-dev, Too much work in one IRQ\n); break; } ofcourse we do get warning message already, so as such the change should be fine. Just want to understand the change bit more. Regards Santosh Regards santosh -- 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 09/17] i2c: omap: ack IRQ in parts
On Thu, Jun 14, 2012 at 01:20:42PM +0300, Felipe Balbi wrote: According to flow diagrams on OMAP TRMs, we should ACK the IRQ as they happen. You don't explain that you're adding a new dev_err() statement into the driver for a missing acknowledge. What if you're probing for a device - can this cause spam to the kernel console? What if you have protocol mangling enabled with the ignore lack of ack bit set ? Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index f978b14..c00ba7d 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -894,21 +894,20 @@ omap_i2c_isr(int this_irq, void *dev_id) } complete: - /* - * Ack the stat in one go, but [R/X]DR and [R/X]RDY should be - * acked after the data operation is complete. - * Ref: TRM SWPU114Q Figure 18-31 - */ - omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, stat - ~(OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR | - OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)); - - if (stat OMAP_I2C_STAT_NACK) + if (stat OMAP_I2C_STAT_NACK) { + dev_err(dev-dev, No Acknowledge\n); err |= OMAP_I2C_STAT_NACK; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; + } if (stat OMAP_I2C_STAT_AL) { dev_err(dev-dev, Arbitration lost\n); err |= OMAP_I2C_STAT_AL; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; } /* @@ -989,12 +988,18 @@ complete: if (stat OMAP_I2C_STAT_ROVR) { dev_err(dev-dev, Receive overrun\n); - dev-cmd_err |= OMAP_I2C_STAT_ROVR; + err |= OMAP_I2C_STAT_ROVR; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_ROVR); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; } if (stat OMAP_I2C_STAT_XUDF) { dev_err(dev-dev, Transmit underflow\n); - dev-cmd_err |= OMAP_I2C_STAT_XUDF; + err |= OMAP_I2C_STAT_XUDF; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XUDF); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; } } while (stat); -- 1.7.10.4 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- 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 3/3] ARM: OMAP2+: gpmc: handle additional timings
* Mohammed, Afzal af...@ti.com [120614 02:46]: Hi Tony, On Thu, Jun 14, 2012 at 14:59:57, Tony Lindgren wrote: * Afzal Mohammed af...@ti.com [120611 07:21]: + GPMC_SET_ONE(GPMC_CS_CONFIG6, 0, 3, bus_turnaround); + GPMC_SET_ONE(GPMC_CS_CONFIG6, 8, 11, cycle2cycle_delay); + + GPMC_SET_ONE(GPMC_CS_CONFIG1, 18, 19, wait_monitoring); + GPMC_SET_ONE(GPMC_CS_CONFIG1, 25, 26, clk_activation); + Thinking about this, the CONFIG1 bits have been set with gpmc_cs_write_reg because these are part of the static configuration and do not need to be dynamically calculated as they are tick based. For example, tusb6010 sets GPMC_CONFIG1_CLKACTIVATIONTIME(1) during init. But aren't we deciding number of ticks based on clock period ? If we take case of onenand, based on the knowledge of clock period, number of ticks are calculated. And similarly to decide cycle2cycledelay, busturnaround, we decide number of ticks based on peripheral datasheet timings gpmc clock, hence shouldn't it also be dynamically calculated similar to timings that were existing earlier. Hmm indeed onenand is setting that dynamically. OK, let's try your approach and make sure we patch in the missing values so we don't overwrite the values set with gpmc_cs_write_reg. Regards, Tony -- 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 15/17] i2c: omap: always return IRQ_HANDLED
On Thu, Jun 14, 2012 at 04:48:56PM +0530, Shilimkar, Santosh wrote: On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: otherwise we could get our IRQ line disabled due to many spurious IRQs. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index fc5b8bc..5b78a73 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1015,7 +1015,7 @@ omap_i2c_isr(int this_irq, void *dev_id) } } while (stat); - return count ? IRQ_HANDLED : IRQ_NONE; + return IRQ_HANDLED; no sure if this is correct. if you have IRQ flood and instead of _actually_ handling it, if you return handled, you still have interrupt pending, right? The point of returning IRQ_NONE is to indicate to the interrupt layer that the interrupt you received was not processed by any interrupt handler, and therefore to provide a way of preventing the system being brought to a halt though a stuck interrupt line. So, if you do process an interrupt, you should always return IRQ_HANDLED even if you couldn't complete its processing (eg, because you've serviced it 100 times.) -- 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 00/17] Big OMAP I2C Cleanup
On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: Hi guys, I have dropped a few patches from the series and also tested every single patch on my pandaboard. I2C messages are still working fine with panda after each patch. There's still lots of work to be done on the i2c-omap.c driver but it's now easier to read, IMO. Great cleanup. Have reviewed the entire series based on limited knowledge i have about the OMAP i2c driver. Apart from those comments, the series looks good to me. We may to take similar dig at MMC and SPI driver one day :-p Regards santosh -- 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 15/17] i2c: omap: always return IRQ_HANDLED
On Thu, Jun 14, 2012 at 4:53 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Thu, Jun 14, 2012 at 04:48:56PM +0530, Shilimkar, Santosh wrote: On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: otherwise we could get our IRQ line disabled due to many spurious IRQs. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index fc5b8bc..5b78a73 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1015,7 +1015,7 @@ omap_i2c_isr(int this_irq, void *dev_id) } } while (stat); - return count ? IRQ_HANDLED : IRQ_NONE; + return IRQ_HANDLED; no sure if this is correct. if you have IRQ flood and instead of _actually_ handling it, if you return handled, you still have interrupt pending, right? The point of returning IRQ_NONE is to indicate to the interrupt layer that the interrupt you received was not processed by any interrupt handler, and therefore to provide a way of preventing the system being brought to a halt though a stuck interrupt line. So, if you do process an interrupt, you should always return IRQ_HANDLED even if you couldn't complete its processing (eg, because you've serviced it 100 times.) That make sense. Thanks for explanation Russell. Regards santosh -- 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
[PATCH] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.
'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes an issue where the ULPI PHYs were not held in reset while initializing the EHCI controller. However, it also changes behavior in omap-usb-host.c omap_usbhs_init by releasing reset while the configuration in that function was done. This change caused a regression on BB-xM where USB would not function if 'usb start' had been run from u-boot before booting. A change was made to release reset a little bit earlier which fixed the issue on BB-xM and did not cause any regressions on 3430 sdp, the board for which the fix was originally made. This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle before adding hcd.', (3aa2ae74) caused a regression on OMAP5. The original fix to hold the EHCI controller in reset during initialization was correct, however it appears that changing omap_usbhs_init to not hold the PHYs in reset during it's configuration was incorrect. This patch first reverts both fixes, and then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in reset as the original patch had done. It also is sure to incorporate the _cansleep change that has been made in the meantime. I've tested this on Beagleboard xM, I'd really like to get an ack from the 3430 sdp and OMAP5 guys before getting this merged. Signed-off-by: Russ Dill russ.d...@gmail.com --- drivers/mfd/omap-usb-host.c | 45 ++ drivers/usb/host/ehci-omap.c | 18 - 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c index 7e96bb2..10984e2 100644 --- a/drivers/mfd/omap-usb-host.c +++ b/drivers/mfd/omap-usb-host.c @@ -25,6 +25,7 @@ #include linux/clk.h #include linux/dma-mapping.h #include linux/spinlock.h +#include linux/gpio.h #include plat/cpu.h #include plat/usb.h #include linux/pm_runtime.h @@ -502,6 +503,19 @@ static void omap_usbhs_init(struct device *dev) pm_runtime_get_sync(dev); spin_lock_irqsave(omap-lock, flags); + if (pdata-ehci_data-phy_reset) { + if (gpio_is_valid(pdata-ehci_data-reset_gpio_port[0])) + gpio_request_one(pdata-ehci_data-reset_gpio_port[0], +GPIOF_OUT_INIT_LOW, USB1 PHY reset); + + if (gpio_is_valid(pdata-ehci_data-reset_gpio_port[1])) + gpio_request_one(pdata-ehci_data-reset_gpio_port[1], +GPIOF_OUT_INIT_LOW, USB2 PHY reset); + + /* Hold the PHY in RESET for enough time till DIR is high */ + udelay(10); + } + omap-usbhs_rev = usbhs_read(omap-uhh_base, OMAP_UHH_REVISION); dev_dbg(dev, OMAP UHH_REVISION 0x%x\n, omap-usbhs_rev); @@ -580,10 +594,39 @@ static void omap_usbhs_init(struct device *dev) usbhs_omap_tll_init(dev, OMAP_TLL_CHANNEL_COUNT); } + if (pdata-ehci_data-phy_reset) { + /* Hold the PHY in RESET for enough time till +* PHY is settled and ready +*/ + udelay(10); + + if (gpio_is_valid(pdata-ehci_data-reset_gpio_port[0])) + gpio_set_value_cansleep + (pdata-ehci_data-reset_gpio_port[0], 1); + + if (gpio_is_valid(pdata-ehci_data-reset_gpio_port[1])) + gpio_set_value_cansleep + (pdata-ehci_data-reset_gpio_port[1], 1); + } + spin_unlock_irqrestore(omap-lock, flags); pm_runtime_put_sync(dev); } +static void omap_usbhs_deinit(struct device *dev) +{ + struct usbhs_hcd_omap *omap = dev_get_drvdata(dev); + struct usbhs_omap_platform_data *pdata = omap-platdata; + + if (pdata-ehci_data-phy_reset) { + if (gpio_is_valid(pdata-ehci_data-reset_gpio_port[0])) + gpio_free(pdata-ehci_data-reset_gpio_port[0]); + + if (gpio_is_valid(pdata-ehci_data-reset_gpio_port[1])) + gpio_free(pdata-ehci_data-reset_gpio_port[1]); + } +} + /** * usbhs_omap_probe - initialize TI-based HCDs @@ -767,6 +810,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev) goto end_probe; err_alloc: + omap_usbhs_deinit(pdev-dev); iounmap(omap-tll_base); err_tll: @@ -818,6 +862,7 @@ static int __devexit usbhs_omap_remove(struct platform_device *pdev) { struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev); + omap_usbhs_deinit(pdev-dev); iounmap(omap-tll_base); iounmap(omap-uhh_base); clk_put(omap-init_60m_fclk); diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index a44294d..d0815b6 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct platform_device
Re: [PATCH] OMAP: Beagle: fix TFP410 powerdown GPIO init
On Thu, Jun 14, 2012 at 1:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Thu, 2012-06-14 at 00:59 -0700, Russ Dill wrote: On Thu, Jun 14, 2012 at 12:18 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-06-13 at 23:58 -0700, Russ Dill wrote: On Wed, Jun 13, 2012 at 2:20 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: Commit e813a55eb9c9bc6c8039fb16332cf43402125b30 (OMAP: board-files: remove custom PD GPIO handling for DVI output) moved TFP410 chip's powerdown-gpio handling from the board files to the tfp410 driver. One gpio_request_one(powerdown-gpio, ...) was mistakenly left unremoved in the Beagle board file. This causes the tfp410 driver to fail to request the gpio on Beagle, causing the driver to fail and thus the DVI output doesn't work. Can you take the one I sent earlier instead? http://www.spinics.net/lists/linux-omap/msg69913.html Hmm, that probably doesn't apply. The power-down GPIO is now handled in the tfp410 driver, not in the board files. Give me a branch to rebase it onto and I will. v3.5-rc2 This one (v4) already applies to v3.5-rc2 http://www.spinics.net/lists/linux-omap/msg70042.html -- 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 04/17] i2c: omap: simplify omap_i2c_ack_stat()
Hi, On Thu, Jun 14, 2012 at 12:13:33PM +0100, Russell King - ARM Linux wrote: On Thu, Jun 14, 2012 at 01:20:37PM +0300, Felipe Balbi wrote: stat BIT(1) is the same as BIT(1), so let's simplify things a bit by removing stat from all omap_i2c_ack_stat() calls. This doesn't feel right, and the explanation is definitely wrong. stat BIT(1) is not the same as BIT(1) _unless_ you're saying that stat always has BIT(1) already set. Can you guarantee that in this code? If so, how? What happens if you read the status register, and it has bit 1 clear. immediately after the read, the status register bit 1 becomes set, and then you write bit 1 set (because you've dropped the stat BIT(1) from the code.) Is it not going to acknowledge that bit-1-set but because you haven't read it, you're going to miss that event? This feels like a buggy change to me. I fail to see that situation would happen with this driver. See what it does (extremely simplified): if (stat NACK) { ... omap_i2c_ack_stat(dev, stat NACK); } if (stat RDR) { ... omap_i2c_ack_stat(dev, stat RDR); } and so on. The tricky place is only WRT errata handling, for example: if (*stat (NACK | AL)) { omap_i2c_ack_stat(dev, *stat (XRDY | XDR)); ... } but in this case, the errata says we must clear XRDY and XDR if that errata triggers, so if they just got enabled or not, it doesn't matter. Another tricky place is RDR | RRDY (likewise for XDR | XRDY): if (stat (RDR | RRDY)) { ... omap_i2c_ack_stat(dev, stat (RDR | RRDY)); } again here there will be no issues because those IRQs never fire simultaneously and one will only after after we have handled the previous, that's because the same FIFO is used anyway and we won't shift data into FIFO until we tell the IP hey, I'm done with the FIFO, you can shift more data. Right ? -- balbi signature.asc Description: Digital signature
Re: [PATCH V3 1/2] of: Add generic device tree DMA helpers
On Wednesday 13 June 2012, Jon Hunter wrote: As I said previously, I think just encoding the direction but not the client specific ID (meaning we would have to disambiguate the more complex cases that Stephen mentioned using a dma-names property) would be the best because it keeps the common case simple, but I could live with other things going in there too. Ok, so you are saying that there are some DMA controllers where there is no channel/request ID and only a direction is needed? So an even simpler case than what I had imagined. No, that was not the case I was thinking about. So in that case, I don't see why the first cell after the phandle could not be an index which could be either a direction or request-id and then the next cell after that be a secondary match variable. So simple case with just a index (either req-id or direction) ... dmas = dmac0 index More complex case ... dmas = dmac0 index match For example, for OMAP index = req-id and match = direction ... dmas = dmac0 req-id direction Or am I way off the page? The intention was instead to remove the need for the /index/ in those cases, because having a client-specific index in here makes it inconsistent with other similar bindings (reg, interrupts, gpios, ...) that people are familiar with. They use an implicit index by counting the fields in the respective properties. The existing method we have for avoiding index numbers is to use named fields, like dmas = dmac0 matchA, dmac1 matchB, dmac2 matchC; dma-names = rx, rx, tx; This is similar to how we use named interrupt and mmio resources, but it requires that we always request the dma lines by name, which is slightly more complex than we might want it to be. Because the vast majority of cases just use a single channel, or one channel per direction, my idea was to encode the direction in the dmas property, which lets us request a dma channel by direction from the driver side, without explicitly encoding the name. This would let us handle the following cases very easily: 1. one read-write channel dmas = dmac 0x3 match; 2. a choice of two read-write channels: dmas = dmacA 0x3 matchA, dmacB 0x3 matchB; 3. one read-channel, one write channel: dmas = dmac 0x1 match-read, dmac 0x2 match-write; 4. a choice of two read channels and one write channel: dmas = dmacA 0x1 match-readA, dmacA 0x2 match-write dmacB match-readB; And only the cases where we have more multiple channels that differ in more aspects would require named properties: 5. two different channels dmas = dmac 0x3 match-rwdata, dmac 0x1 match-status; dma-names = rwdata, status; 6. same as 5, but with a choice of channels: dmas = dmacA 0x3 match-rwdataA, dmacA 0x1 match-status; dmacB 0x3 match-rwdataB; dma-names = rwdata, status, rwdata; With a definition like that, we can implement a very simple device driver interface for the common cases, and a slightly more complex one for the more complex cases: 1. chan = of_dma_request_channel(dev-of_node, 0); 2. chan = of_dma_request_channel(dev-of_node, 0); 3. rxchan = of_dma_request_channel(dev-of_node, DMA_MEM_TO_DEV); txchan = of_dma_request_channel(dev-of_node, DMA_DEV_TO_MEM); 4. rxchan = of_dma_request_channel(dev-of_node, DMA_MEM_TO_DEV); txchan = of_dma_request_channel(dev-of_node, DMA_DEV_TO_MEM); 5. chan = of_dma_request_named_channel(dev-of_node, rwdata, 0); auxchan = of_dma_request_named_channel(dev-of_node, status, DMA_DEV_TO_MEM); 6. chan = of_dma_request_named_channel(dev-of_node, rwdata, 0); auxchan = of_dma_request_named_channel(dev-of_node, status, DMA_DEV_TO_MEM); Arnd -- 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 09/17] i2c: omap: ack IRQ in parts
On Thu, Jun 14, 2012 at 04:41:45PM +0530, Shilimkar, Santosh wrote: On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi ba...@ti.com wrote: According to flow diagrams on OMAP TRMs, we should ACK the IRQ as they happen. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/i2c/busses/i2c-omap.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index f978b14..c00ba7d 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -894,21 +894,20 @@ omap_i2c_isr(int this_irq, void *dev_id) } complete: - /* - * Ack the stat in one go, but [R/X]DR and [R/X]RDY should be - * acked after the data operation is complete. - * Ref: TRM SWPU114Q Figure 18-31 - */ - omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, stat - ~(OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR | - OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)); - - if (stat OMAP_I2C_STAT_NACK) + if (stat OMAP_I2C_STAT_NACK) { + dev_err(dev-dev, No Acknowledge\n); err |= OMAP_I2C_STAT_NACK; + omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK); + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; + } Do you think making I2C IRQ ack + complete as one small static inline function can save clean the code further. I see it has been used in multiple paths. done in a later patch -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/3] ARM: OMAP2+: gpmc: handle additional timings
* Mohammed, Afzal af...@ti.com [120614 03:43]: Hi Tony, On Thu, Jun 14, 2012 at 15:49:02, Tony Lindgren wrote: Well I took a look at the values, and it seems the only difference is the static GPMC_CONFIG1_CLKACTIVATIONTIME(1) that your patch now overwrites 0. It seems change below should be part of $subject. Please let me know your comments Well I could not get this to apply either on top of the $subject nor all your patches for some reason, but I manually applied the tusb6010 part with the following change.. --- a/arch/arm/mach-omap2/usb-tusb6010.c +++ b/arch/arm/mach-omap2/usb-tusb6010.c @@ -174,6 +174,8 @@ static int tusb_set_sync_mode(unsigned sysclk_ps, unsigned fclk_ps) tmp = t.cs_wr_off * 1000 + 7000 /* t_scsn_rdy_z */; t.wr_cycle = next_clk(t.cs_wr_off, tmp, fclk_ps); + t.clk_activation = gpmc_ticks_to_ns(1); + ..this should be just 1 as it's one tick, not ns. return gpmc_cs_set_timings(sync_cs, t); } @@ -283,7 +285,6 @@ tusb6010_setup_interface(struct musb_hdrc_platform_data *data, | GPMC_CONFIG1_READTYPE_SYNC | GPMC_CONFIG1_WRITEMULTIPLE_SUPP | GPMC_CONFIG1_WRITETYPE_SYNC - | GPMC_CONFIG1_CLKACTIVATIONTIME(1) | GPMC_CONFIG1_PAGE_LEN(2) | GPMC_CONFIG1_WAIT_READ_MON | GPMC_CONFIG1_WAIT_WRITE_MON And that makes tusb6010 work as earlier with your patches. For onenand I'm getting the following error: omap2-onenand omap2-onenand: Cannot request GPMC CS Regards, Tony -- 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 09/17] i2c: omap: ack IRQ in parts
On Thu, Jun 14, 2012 at 12:20:17PM +0100, Russell King - ARM Linux wrote: On Thu, Jun 14, 2012 at 01:20:42PM +0300, Felipe Balbi wrote: According to flow diagrams on OMAP TRMs, we should ACK the IRQ as they happen. You don't explain that you're adding a new dev_err() statement into the driver for a missing acknowledge. What if you're probing for a device - can this cause spam to the kernel console? What if you have protocol mangling enabled with the ignore lack of ack bit set ? indeed... maybe dev_vdbg() would fit better, or just don't add anything. -- balbi signature.asc Description: Digital signature
Re: [PATCH 06/17] i2c: omap: improve 1p153 errata handling
Hi, On Thu, Jun 14, 2012 at 12:17:46PM +0100, Russell King - ARM Linux wrote: On Thu, Jun 14, 2012 at 01:20:39PM +0300, Felipe Balbi wrote: -static int errata_omap3_1p153(struct omap_i2c_dev *dev, u16 *stat, int *err) +static int errata_omap3_1p153(struct omap_i2c_dev *dev) { - unsigned long timeout = 1; + unsigned long timeout = 1; + u16 stat; Eww, no, not more of this lets add tabs to align auto variable declarations. This is detrimental when you add another variable whose type is longer than the current indentation - you have to reformat the entire block. It's really not worth it. Stick to just using one space, like the rest of the kernel code. Like the code which Linus writes. And thereby help to avoid future pointless churn whinges. fair enough, no need to get so over the top like that, it's just a tab. Will fix it. -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/3] ARM: OMAP2+: gpmc: handle additional timings
* Mohammed, Afzal af...@ti.com [120614 03:43]: Hi Tony, On Thu, Jun 14, 2012 at 15:49:02, Tony Lindgren wrote: Well I took a look at the values, and it seems the only difference is the static GPMC_CONFIG1_CLKACTIVATIONTIME(1) that your patch now overwrites 0. It seems change below should be part of $subject. Please let me know your comments Regards Afzal diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c index fd4c48d..938896c 100644 --- a/arch/arm/mach-omap2/gpmc-onenand.c +++ b/arch/arm/mach-omap2/gpmc-onenand.c @@ -321,6 +321,8 @@ static int omap2_onenand_set_sync_mode(struct omap_onenand_platform_data *cfg, t.rd_cycle = gpmc_ticks_to_ns(fclk_offset + (latency + 1) * div + ticks_cez); + t.clk_activation = fclk_offset_ns; + This too should be fclk_offset, not fclk_offset_ns. Tony -- 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: [CFT 07/11] spi: omap2-mcspi: add DMA engine support
On Thu, Jun 07, 2012 at 12:08:35PM +0100, Russell King wrote: Add DMA engine support to the OMAP SPI driver. This supplements the private DMA API implementation contained within this driver, and the driver can be independently switched at build time between using DMA engine and the private DMA API for the transmit and receive sides. Tested-by: Shubhrajyoti shubhrajy...@ti.com Acked-by: Grant Likely grant.lik...@secretlab.ca Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk Right, now that we have working OMAP in mainline again... [ cut here ] WARNING: at lib/dma-debug.c:865 check_unmap+0x1a0/0x6f8() ks8851 spi1.0: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0x9f5c3002] [size=592 bytes] Modules linked in: Backtrace: [c0017dd0] (dump_backtrace+0x0/0x10c) from [c0346870] (dump_stack+0x18/0x1c) r7:df79fd78 r6:c01a2200 r5:c04036bd r4:0361 [c0346858] (dump_stack+0x0/0x1c) from [c0033c48] (warn_slowpath_common+0x58/0x70) [c0033bf0] (warn_slowpath_common+0x0/0x70) from [c0033d04] (warn_slowpath_fmt+0x38/0x40) r8:df79fdf8 r7: r6:0250 r5: r4:9f5c3002 [c0033ccc] (warn_slowpath_fmt+0x0/0x40) from [c01a2200] (check_unmap+0x1a0/0x6f8) r3:c040ea22 r2:c0403a0f [c01a2060] (check_unmap+0x0/0x6f8) from [c01a2978] (debug_dma_unmap_page+0x74/0x80) [c01a2904] (debug_dma_unmap_page+0x0/0x80) from [c021bd64] (omap2_mcspi_txrx_dma+0x33c/0x54c) [c021ba28] (omap2_mcspi_txrx_dma+0x0/0x54c) from [c021c590] (omap2_mcspi_work+0x1b8/0x2b8) [c021c3d8] (omap2_mcspi_work+0x0/0x2b8) from [c021c974] (omap2_mcspi_transfer_one_message+0x2e4/0x310) [c021c690] (omap2_mcspi_transfer_one_message+0x0/0x310) from [c021a9f8] (spi_pump_messages+0x130/0x154) [c021a8c8] (spi_pump_messages+0x0/0x154) from [c00508dc] (kthread_worker_fn+0x108/0x188) r7:df6a3d94 r6:df79e000 r5:df6a3d90 r4:df6a3da4 [c00507d4] (kthread_worker_fn+0x0/0x188) from [c0050a60] (kthread+0x8c/0x98) [c00509d4] (kthread+0x0/0x98) from [c0039134] (do_exit+0x0/0x314) r7:0013 r6:c0039134 r5:c00509d4 r4:df443d78 ---[ end trace 1b75b31a2719ed1f ]--- So, trying to figure this out... the result is not nice. If the spi message has is_dma_mapped = false, then we potentially map the DMA buffers against mcspi-dev. This struct device is the same as the master-dev.parent. However, when we come to complete a transfer, we unmap them against the spi_device's struct device - in other words a different device. That's the reason for the warning. However, when using DMA engine, both of these struct device's are the wrong one to be using - the right one to use is the one assocated with the DMA engine. However, this presents a problem with transfers with is_dma_mapped = true. SPI device drivers appear to assume that the right struct device to use to map for DMA is master-dev.parent. That's fine if your SPI master device is the struct device performing the DMA, but with DMA engine involved, this is not true. Not sure at the moment what to do about that one. -- 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 3/3] ARM: OMAP2+: gpmc: handle additional timings
Hi Tony, On Thu, Jun 14, 2012 at 17:22:08, Tony Lindgren wrote: * Mohammed, Afzal af...@ti.com [120614 03:43]: + t.clk_activation = fclk_offset_ns; + This too should be fclk_offset, not fclk_offset_ns. As gpmc_cs_set_timing convert it to ticks from ns, shouldn't we put it in ns ? 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 2/4] ARM: OMAP2+: Move the stubbed prm and cm functions to prcm.c file and make them __weak.
Hi Paul, Some prm and cm registers read/write and status functions are built only for some custom OMAP2+ builds and are stubbed in header files for other builds under ifdef statements. But this results in adding new CONFIG_ARCH_OMAPXXX checks when SOCs are added in the future. So move them to a common place for OMAP2+ and make them 'weak' implementations. Thanks for the patch. These stubs aren't needed any longer, so we can just get rid of them; no need to mark them __weak. I've got a patch to just drop them here to be posted soon as part of a larger PRCM cleanup. Is the PRCM cleanup series available now ? Thanks, Sricharan -- 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 3/3] ARM: OMAP2+: gpmc: handle additional timings
Hi Tony, On Thu, Jun 14, 2012 at 17:19:06, Tony Lindgren wrote: * Mohammed, Afzal af...@ti.com [120614 03:43]: It seems change below should be part of $subject. For onenand I'm getting the following error: omap2-onenand omap2-onenand: Cannot request GPMC CS Without this change (not referring to 3/3, but diff that was sent in previous mail), was not this error present 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: [CFT 07/11] spi: omap2-mcspi: add DMA engine support
On Thu, Jun 14, 2012 at 12:53:35PM +0100, Russell King - ARM Linux wrote: On Thu, Jun 07, 2012 at 12:08:35PM +0100, Russell King wrote: Add DMA engine support to the OMAP SPI driver. This supplements the private DMA API implementation contained within this driver, and the driver can be independently switched at build time between using DMA engine and the private DMA API for the transmit and receive sides. Tested-by: Shubhrajyoti shubhrajy...@ti.com Acked-by: Grant Likely grant.lik...@secretlab.ca Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk Right, now that we have working OMAP in mainline again... Another warning: [ cut here ] WARNING: at /home/rmk/git/linux-rmk/drivers/base/dd.c:257 driver_probe_device+0x78/0x21c() Modules linked in: Backtrace: [c0017d0c] (dump_backtrace+0x0/0x10c) from [c033e208] (dump_stack+0x18/0x1c) r7: r6:c01ff28c r5:c040050c r4:0101 [c033e1f0] (dump_stack+0x0/0x1c) from [c00337ec] (warn_slowpath_common+0x58/0x70) [c0033794] (warn_slowpath_common+0x0/0x70) from [c0033828] (warn_slowpath_null+0x24/0x2c) r8:c04aa4d8 r7:c04aa63c r6:de70ce00 r5:de70ce34 r4:de70ce00 [c0033804] (warn_slowpath_null+0x0/0x2c) from [c01ff28c] (driver_probe_device+0x78/0x21c) [c01ff214] (driver_probe_device+0x0/0x21c) from [c01ff49c] (__driver_attach+0x6c/0x90) r7:de443ec8 r6:c04aa63c r5:de70ce34 r4:de70ce00 [c01ff430] (__driver_attach+0x0/0x90) from [c01fda70] (bus_for_each_dev+0x58/0x98) r6:c04aa63c r5:c01ff430 r4: [c01fda18] (bus_for_each_dev+0x0/0x98) from [c01ff0f4] (driver_attach+0x20/0x28) r7:de6c9e80 r6:c04aa63c r5:c04aa63c r4:c0465b80 [c01ff0d4] (driver_attach+0x0/0x28) from [c01fe2f4] (bus_add_driver+0xb4/0x230) [c01fe240] (bus_add_driver+0x0/0x230) from [c01ffb24] (driver_register+0xac/0x138) [c01ffa78] (driver_register+0x0/0x138) from [c0215d4c] (spi_register_driver+0x4c/0x60) r8:005b r7:c045f848 r6:0006 r5:0018 r4:c0465b80 [c0215d00] (spi_register_driver+0x0/0x60) from [c045414c] (ks8851_init+0x14/0x1c) [c0454138] (ks8851_init+0x0/0x1c) from [c0008770] (do_one_initcall+0x9c/0x164) [c00086d4] (do_one_initcall+0x0/0x164) from [c0436410] (kernel_init+0x128/0x210) [c04362e8] (kernel_init+0x0/0x210) from [c0038754] (do_exit+0x0/0x72c) ---[ end trace 4dcda79f5e89dd84 ]--- ks8851 spi1.0: message enable is 0 ks8851 spi1.0: eth0: revision 0, MAC 08:00:28:01:4d:c6, IRQ 194, has EEPROM The relevant line: WARN_ON(!list_empty(dev-devres_head)); Which suggests that someone is using devres against the struct device for the KS8851 device before the driver is bound. That's a bug, plain and simple. I've not yet been able to track down what it is or where it's being done, but it is something that has been introduced during the last merge window. devm_* APIs should only be used by _drivers_ against the struct device that they are driving - because the lifetime of these things is bounded by the point at which the driver is bound to that struct device, to the point that it is unbound from that struct device (and at that point, all devm_* stuff against the struct device gets destroyed.) -- 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 3/3] ARM: OMAP2+: gpmc: handle additional timings
Hi Tony, On Thu, Jun 14, 2012 at 17:19:06, Tony Lindgren wrote: * Mohammed, Afzal af...@ti.com [120614 03:43]: For onenand I'm getting the following error: omap2-onenand omap2-onenand: Cannot request GPMC CS I am a bit confused with this message, for onenand, we only have, pr_err(%s: Cannot request GPMC CS\n, __func__); dev_err is used only for nand 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] OMAP: Beagle: fix TFP410 powerdown GPIO init
On Thu, 2012-06-14 at 04:40 -0700, Russ Dill wrote: On Thu, Jun 14, 2012 at 1:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Thu, 2012-06-14 at 00:59 -0700, Russ Dill wrote: On Thu, Jun 14, 2012 at 12:18 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-06-13 at 23:58 -0700, Russ Dill wrote: On Wed, Jun 13, 2012 at 2:20 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: Commit e813a55eb9c9bc6c8039fb16332cf43402125b30 (OMAP: board-files: remove custom PD GPIO handling for DVI output) moved TFP410 chip's powerdown-gpio handling from the board files to the tfp410 driver. One gpio_request_one(powerdown-gpio, ...) was mistakenly left unremoved in the Beagle board file. This causes the tfp410 driver to fail to request the gpio on Beagle, causing the driver to fail and thus the DVI output doesn't work. Can you take the one I sent earlier instead? http://www.spinics.net/lists/linux-omap/msg69913.html Hmm, that probably doesn't apply. The power-down GPIO is now handled in the tfp410 driver, not in the board files. Give me a branch to rebase it onto and I will. v3.5-rc2 This one (v4) already applies to v3.5-rc2 http://www.spinics.net/lists/linux-omap/msg70042.html My ack for v4: Acked-by: Tomi Valkeinen tomi.valkei...@ti.com And I'd reword the description, fix instead of cleanup. The boot warnings are errors. I don't think the DVI output works at all with beagle in the current mainline. Btw, please cc me when sending patches related to display. Tomi signature.asc Description: This is a digitally signed message part
RE: [PATCH 3/3] ARM: OMAP2+: gpmc: handle additional timings
Hi Tony, On Thu, Jun 14, 2012 at 17:39:41, Mohammed, Afzal wrote: On Thu, Jun 14, 2012 at 17:19:06, Tony Lindgren wrote: For onenand I'm getting the following error: omap2-onenand omap2-onenand: Cannot request GPMC CS I am a bit confused with this message, for onenand, we only have, pr_err(%s: Cannot request GPMC CS\n, __func__); dev_err is used only for nand This error can happen if my patches are not present, dev_err for onenand driver was removed by, [mtd: onenand: omap2: obtain memory from resource], seems, mtd patches are not in your present branch 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
[PATCH 0/3] ARM: OMAP2/3: Move McBSP fck alias to hwmod data
Hello, To keep the McBSP fck alias handling in sync among OMAP versions. Change the legacy implementation for OMAP2/3 regarding to McBSP fck alias. OMAP4 (and OMAP5) uses the hwmod data to specify the aliases for the fcks and it is also needed for the coming McBSP DT support. Tested on OMAP3 BeagleBoard, compile tested it for OMAP2. Regards, Peter --- Peter Ujfalusi (3): ARM: OMAP2: Move McBSP fck clock alias to hwmod data for OMAP2420 ARM: OMAP2: Move McBSP fck clock alias to hwmod data for OMAP2430 ARM: OMAP3: Move McBSP fck clock alias to hwmod data arch/arm/mach-omap2/clock2420_data.c |4 arch/arm/mach-omap2/clock2430_data.c | 10 -- arch/arm/mach-omap2/clock3xxx_data.c | 10 -- arch/arm/mach-omap2/omap_hwmod_2420_data.c |9 + arch/arm/mach-omap2/omap_hwmod_2430_data.c | 15 +++ arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 21 + 6 files changed, 45 insertions(+), 24 deletions(-) -- 1.7.8.6 -- 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
[PATCH 1/3] ARM: OMAP2: Move McBSP fck clock alias to hwmod data for OMAP2420
Remove the existing alias for pad_fck, prcm_fck from the clock data and add them as opt_clks to the hwmod data. Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com --- arch/arm/mach-omap2/clock2420_data.c |4 arch/arm/mach-omap2/omap_hwmod_2420_data.c |9 + 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/arch/arm/mach-omap2/clock2420_data.c b/arch/arm/mach-omap2/clock2420_data.c index bace930..7e39015 100644 --- a/arch/arm/mach-omap2/clock2420_data.c +++ b/arch/arm/mach-omap2/clock2420_data.c @@ -1774,8 +1774,6 @@ static struct omap_clk omap2420_clks[] = { CLK(NULL, osc_ck, osc_ck,CK_242X), CLK(NULL, sys_ck, sys_ck,CK_242X), CLK(NULL, alt_ck, alt_ck,CK_242X), - CLK(omap-mcbsp.1, pad_fck, mcbsp_clks,CK_242X), - CLK(omap-mcbsp.2, pad_fck, mcbsp_clks,CK_242X), CLK(NULL, mcbsp_clks, mcbsp_clks,CK_242X), /* internal analog sources */ CLK(NULL, dpll_ck, dpll_ck, CK_242X), @@ -1784,8 +1782,6 @@ static struct omap_clk omap2420_clks[] = { /* internal prcm root sources */ CLK(NULL, func_54m_ck, func_54m_ck, CK_242X), CLK(NULL, core_ck, core_ck, CK_242X), - CLK(omap-mcbsp.1, prcm_fck, func_96m_ck, CK_242X), - CLK(omap-mcbsp.2, prcm_fck, func_96m_ck, CK_242X), CLK(NULL, func_96m_ck, func_96m_ck, CK_242X), CLK(NULL, func_48m_ck, func_48m_ck, CK_242X), CLK(NULL, func_12m_ck, func_12m_ck, CK_242X), diff --git a/arch/arm/mach-omap2/omap_hwmod_2420_data.c b/arch/arm/mach-omap2/omap_hwmod_2420_data.c index a7640d1..910d256 100644 --- a/arch/arm/mach-omap2/omap_hwmod_2420_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_2420_data.c @@ -192,6 +192,11 @@ static struct omap_hwmod_class omap2420_mcbsp_hwmod_class = { .name = mcbsp, }; +static struct omap_hwmod_opt_clk mcbsp_opt_clks[] = { + { .role = pad_fck, .clk = mcbsp_clks }, + { .role = prcm_fck, .clk = func_96m_ck }, +}; + /* mcbsp1 */ static struct omap_hwmod_irq_info omap2420_mcbsp1_irqs[] = { { .name = tx, .irq = 59 }, @@ -214,6 +219,8 @@ static struct omap_hwmod omap2420_mcbsp1_hwmod = { .idlest_idle_bit = OMAP24XX_ST_MCBSP1_SHIFT, }, }, + .opt_clks = mcbsp_opt_clks, + .opt_clks_cnt = ARRAY_SIZE(mcbsp_opt_clks), }; /* mcbsp2 */ @@ -238,6 +245,8 @@ static struct omap_hwmod omap2420_mcbsp2_hwmod = { .idlest_idle_bit = OMAP24XX_ST_MCBSP2_SHIFT, }, }, + .opt_clks = mcbsp_opt_clks, + .opt_clks_cnt = ARRAY_SIZE(mcbsp_opt_clks), }; static struct omap_hwmod_class_sysconfig omap2420_msdi_sysc = { -- 1.7.8.6 -- 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