RE: Please help! AM35xx mm/slab.c BUG
Hi, On Fri, Jun 08, 2012 at 01:20:13, CF Adad wrote: Thanks for your thoughts! I may try fiddling a bit just to see if that helps. 5 series of patches for gpmc modifications [1-5] has been posted, In case it helps in resolving your issue, please let me know. You will have to use the new interface to make use of runtime calculation of smsc911x timing, [5.x] can be referred for how to do board modifications for smsc911x (please comment out any other gpmc peripheral initialization in your board code, seems you have nand, comment out nand initialization, even nand can be made to work with new changes, but avoiding it will probably reduce your burden) As your eth is 9221, either you can provide the timings based on it from board file or apply [6] over [1-5] (base: 3.5-rc1) Regards Afzal [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg69501.html [2] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg69881.html [3] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg69891.html [4] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg69897.html [5] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg69917.html [5.x] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg69924.html [6] diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c index 4bfe721..34816b9 100644 --- a/arch/arm/mach-omap2/gpmc-smsc911x.c +++ b/arch/arm/mach-omap2/gpmc-smsc911x.c @@ -105,12 +105,12 @@ static void gpmc_smsc911x_timing(struct gpmc_time_ctrl *time_ctrl, { struct gpmc_timings t; /* SMSC 9220 timings */ - unsigned tcycle_r = 165; + unsigned tcycle_r = 45; unsigned tcsl_r = 32; unsigned tcsh_r = 133; unsigned tcsdv_r = 30; unsigned tdoff_r = 9; - unsigned tcycle_w = 165; + unsigned tcycle_w = 45; unsigned tcsl_w = 32; unsigned tcsh_w = 133; unsigned tdsu_w = 7; -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4-alt 3/6] ARM: OMAP3: hwmod data: add gpmc
Hi Tony, On Tue, May 22, 2012 at 22:12:15, Tony Lindgren wrote: for these cases. Otherwise we'll be breaking old boards with smsc911x where the timings for the FPGA controlling smsc911x are unknown. Were you actually referring to sdp boards that work with smc91x driver using 91c96 ? 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 Tue, Jun 12, 2012 at 14:10:08, Mohammed, Afzal wrote: + l |= conf GPMC_CONFIG1_DEVICETYPE_NAND; + l |= conf GPMC_CONFIG1_DEVICESIZE_16; I can see that it works to use the above definitions as masks because of the possible values that can be programmed into these fields. However, from a read-ability standpoint this is unclear and requires people to review the documentation to understand what you are doing here. Furthermore, if any new device-types or sizes were added in the future this could lead to bugs. Hence, it would be better to define a mask for these fields. I had thought about it initially, but then it was felt it will lead to a less simple code, that path was not taken, let me revisit this. Thinking again over it, I am feeling above is sufficient, reason same as said earlier, to keep code simple currently this is sufficient to handle GPMC bit patterns for IPs presently available. What you are suggesting is to take care of the imaginary case when new GPMC IP happens with new device type or size, I think that should be handled when such a scenario happens. Probably, it is better here to add a comment to make intention 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 3/3] ARM: OMAP2+: gpmc: handle additional timings
Hi Jon, On Tue, Jun 12, 2012 at 23:06:53, Hunter, Jon wrote: Should you at least warn, if you are going to over-write a value? Yes, that would be better except for too much logging, if Tony prefers that way I will add. 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/3] ARM: OMAP2+: onenand: cleanup for gpmc driver conversion
Hi Jon, On Tue, Jun 12, 2012 at 23:00:48, Hunter, Jon wrote: On 06/12/2012 01:16 AM, Mohammed, Afzal wrote: With the existing code, set_async was done as part of set_sync, hence requiring GPMC to be configured twice after driver takes control, with your suggestion too, GPMC would have to be configured twice. I am just suggesting that you place the call to set_async_mode in the gpmc_onenand_setup() instead of the gpmc_onenand_init() and remove the calls from set_sync (like you have done). So I don't see that these would configure the GPMC twice. As gpmc_onenand_setup is a callback by onenand driver, we would have lost the opportunity to configure onenand before driver is probed. This would cause requirement of double GPMC configuring and we lost the opportunity to configure GPMC before driver is probed. And the first step for onenand configuration is always to set it to async mode (with the way it is done now), so it seems reasonable to rely on normal GPMC configuration for async then do reconfigure for sync. 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 Tue, Jun 12, 2012 at 23:10:01, Hunter, Jon wrote: On 06/12/2012 01:53 AM, Mohammed, Afzal wrote: On Tue, Jun 12, 2012 at 01:26:29, Hunter, Jon wrote: My preference would be to store gpmc_l3_clk in the pdata and pass to probe via the pdata. The aim would be to remove the global gpmc_l3_clk altogether. For timing calculation by platform outside of driver, we need clk rate Right but potentially, this could be done by the driver. 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. 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 03/14] ARM: OMAP2+: gpmc: driver migration helper
Hi Jon, On Tue, Jun 12, 2012 at 23:16:50, Hunter, Jon wrote: Ok, I see that this is necessary until all boards are migrated. However, please label with a FIXME to make it clear that this is to be removed. Ok, I will update this. 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 Tue, Jun 12, 2012 at 23:32:05, Hunter, Jon wrote: Well looking at the function it seems that you either return an error code or 1. So if you are never going to return anything other than 1 on success it may as well be 0. 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. 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 Tue, Jun 12, 2012 at 23:36:17, Hunter, Jon wrote: Well it is unclear what the code flow is for using this helper as this change simply adds the helper. If someone other function is writing to the CONFIG1 register before this function, then you may wish to highlight the programming sequence in the changelog or at least describe why this function performs a read-modify-write and not just a write. Logic followed here: CS configuration helper needs to worry only about bits that are relevant for CS configuration and don't alter any other bits/registers. Same is applicable for time setting helper. 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 Tue, Jun 12, 2012 at 23:39:32, Hunter, Jon wrote: On 06/12/2012 07:58 AM, Mohammed, Afzal wrote: Thinking again over it, I am feeling above is sufficient, reason same as said earlier, to keep code simple currently this is sufficient to handle GPMC bit patterns for IPs presently available. What you are suggesting is to take care of the imaginary case when new GPMC IP happens with new device type or size, I think that should be handled when such a scenario happens. Probably, it is better here to add a comment to make intention clear. That is one possibility but I think that more important reasons are ... 1. Readability, at first it appears that we are always configuring the CS for NAND. However, this is not the case. Maybe a comment here can help as you mentioned. As far as the users of GPMC is considered there is no confusion. Eg. For device type, they have been provided with two macros, GPMC_CONFIG1_DEVICETYPE_NOR, GPMC_CONFIG1_DEVICETYPE_NAND So for NOR, user can feel satisfied by giving NOR flag, little does he know that driver doesn't do anything with the flag, but he still gets what he want NOR flag is defined as zero. Yes even if he doesn't specify any type, device type will be set as NOR, but then that is the default - the only other option 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. 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 1/3] ARM: OMAP2+: nand: unify init functions
Hi Jon, On Mon, Jun 11, 2012 at 21:13:45, Hunter, Jon wrote: Which boards have been tested with this change? Beagle board Reviewed-by: Jon Hunter jon-hun...@ti.com Thanks 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: Please help! AM35xx mm/slab.c BUG
Hi , On Wed, Jun 06, 2012 at 13:21:23, CF Adad wrote: Thanks again. I'm really starting to think the GPMC almost has to be contributing. Does adding cycle2cycle delay / bus turnaround prevent the issue ?, SMSC datasheet mentions about special restrictions on back to back read and write-read, reading BYTE_TEST should take care of it, not sure whether driver takes care of all scenarios as per datasheet. Perhaps cycle2cycledelay would help us achieve it if driver doesn't take care of 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 00/10] Prepare for GPMC driver conversion (w.r.t MTD)
Hi, On Mon, Jun 04, 2012 at 12:15:01, Mohammed, Afzal wrote: Hi, This series cleans up gpmc mtd interactions so that GPMC driver conversion which is going to happen shortly would happen smoothly by not creating much disturbance outside of arch/arm/*omap*/ This series, 1. provides the ability for OMAP NAND driver to configure GPMC-NAND registers by NAND driver itself instead of using exported GPMC symbols 2. modifies GPMC to provide OMAP ONENAND NAND drivers with GPMC allocated address space as resource 3. creates a fictitious GPMC interrupt chip and provide the clients with interrupts that could be handled using standard APIs (helps in removing the requirement for driver of peripheral connected to GPMC having the knowledge about GPMC interrupt handling). The only user is OMAP NAND driver, it has also been modified to take advantage of this This series has been made over 3.5-rc1 Ping Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4-alt 3/6] ARM: OMAP3: hwmod data: add gpmc
Hi Tony, On Fri, May 25, 2012 at 12:56:59, Tony Lindgren wrote: * Paul Walmsley p...@pwsan.com [120523 17:55]: On Tue, 22 May 2012, Tony Lindgren wrote: Unfortunately for many of the older boards these values will probably remain unknown. So the better approach here is to just disable frequency scaling for these cases. Otherwise we'll be breaking old boards with smsc911x where the timings for the FPGA controlling smsc911x are unknown. If we somehow manage to get those values without breaking support for these boards, then yes I agree we should deprecate hardcoded and bootloader values. I was thinking that if we log the register values supplied by the bootloader to the console, then someone can write a patch to the board file or DT data to set those register values explicitly in the kernel, once they're known. Or at least just report them to the l-o list. So then that should reduce the problem cases down to boards which no one reports the data to the mailing list within a few mainline kernel releases -- i.e., boards which are unmaintained. For those boards, I'd suggest that we just drop GPMC support until someone shows up who has a board and can test-boot it. Or just drop the board completely ;-) OK seems fair to me. It still allows us to boot the older boards with minimal changes (but without L3 frequency scaling). Sounds like those registers should be dumped only if no configuration is specified to avoid spamming the console. Shall I take it as go ahead to create a patch on Documentation/feature-removal-schedule.txt in the next version of gpmc series stating that implicit boot loader GPMC timings will be removed in three Kernel releases ? (i.e. as per Paul's suggestion, along with other points he has mentioned) 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 0/3] GPMC NAND isr using standard API
Hi Artem, On Tue, May 22, 2012 at 11:44:43, Artem Bityutskiy wrote: You merge the 2 trees and work on top of that? Or you wait for 3.5-r1 when everything is merged and work on top of that? I will merge 2 trees do Tony, are you ok with that ? Regards Afzal N�r��yb�X��ǧv�^�){.n�+{��f��{ay�ʇڙ�,j��f���h���z��w��� ���j:+v���w�j�mzZ+�ݢj��!�i
RE: [PATCH v4-alt 3/6] ARM: OMAP3: hwmod data: add gpmc
Hi Paul, On Tue, May 22, 2012 at 12:17:30, Paul Walmsley wrote: Hi Afzal On Thu, 10 May 2012, Mohammed, Afzal wrote: On Tue, May 08, 2012 at 21:02:33, Paul Walmsley wrote: (attribution lost) Hmm, second time, normally I try to delete as much as possible contents from the original mail to make it more readable while keeping the core I'd suggest implementing two ways of programming the GPMC from the kernel. The first, preferred, method would be used with boards that we have timing data for that is independent from the GPMC clock rate -- i.e., timing data in nanoseconds or picoseconds. These boards will be capable of CORE DVFS. The second, deprecated, method would be used with boards that we only have GPMC clock rate-dependent timing data for -- i.e., raw GPMC register data. These boards will not be CORE DVFS-capable. It should be possible for the kernel to configure the GPMC with either one of these methods, either from DT or from platform_data. I'd suggest starting by adding code to allow a board file/DT to configure the GPMC to set the timings for a given chip-select based on clock rate-independent data (the first method above). Some good starting points for this code would be in the arch/arm/mach-omap2/gpmc-*.c files. Then the rate-independent data can be added for the boards which have available schematics or for which we have the data. For the DT case, you'll probably need to define a clock rate-independent binding if you haven't already. Next, I'd suggest implementing the code to allow GPMC timing configuration from raw register data (the second method above). This is hackish but for some boards, this is all we'll have. This will also presumably require some extra DT bindings for the register data. If this method is used, this code should also call a PM function to block clock rate changes on the GPMC clock, and an explanatory warning should be logged to the console. For boards that we don't have access to, and all someone would have are the register values set by the bootloader, I'd propose a phased approach: 1. The kernel should log the bootloader-provided GPMC timing registers to the console during boot, along with a warning message that indicates that GPMC for these boards will cease to function in the near future unless patches are provided to update the kernel board file and/or DT data with the GPMC register contents. This should allow people who have those boards and who care about them to submit kernel patches to allow the GPMC-attached devices to continue to function. 2. A patch to Documentation/feature-removal-schedule.txt should be submitted which states that support for implicit bootloader GPMC timings will be removed within two or three kernel releases. 3. The board maintainers and anyone who has contributed to the mainline git tree for those boards who seems to actually have the board should be contacted with a short E-mail message asking them to update their board GPMC timings. 4. When the expiration date specified in #2 is released, HWMOD_INIT_NO_RESET would be removed from the GPMC hwmod, and the GPMC code should refuse to initialize unless explicit timing data has been provided. If this protocol is followed, I wouldn't have a significant objection to specifying HWMOD_INIT_NO_RESET for the GPMC. That's because, under these conditions, the flag will only be present for a short period of time, and there will be a strong incentive for people with those boards to update the mainline board file/DT data with the GPMC timings. Otherwise their boards will be broken. Summarized GPMC tasks as per my understanding based on Tony's yours comments and that I am working on as follows, 1. convert to driver 2. remove dependency of bootloader for configuration Approach being taken is to migrate to driver while keeping old interface w.r.t boards intact (i.e. configuring gpmc in board files for old interface can be done the way it is done now, tasks now achieved in gpmc_init would be done by probe, only that much, but that would not make any difference for boards using old interface) along with having new interface till all boards are converted to use new interface. Once all boards are converted to use new interface, old interface would be removed. For boards using new interface, in probe, in addition to the tasks presently done by gpmc_init, it would do configuring for boards, creating platform devices for the peripherals connected. Configuration that is not presently done in Kernel would be handled the way you have suggested; first preference clk rate independent, if not possible then use register values, if that also not possible do as per your points 1-4. GPMC configuration that would be added newly in the Kernel would be using new interface Tony, Paul, please let me know if you have any divergent views on the above. Another issue on OMAP3EVM
RE: [PATCH 0/3] GPMC NAND isr using standard API
Hi Ivan, On Sat, May 19, 2012 at 18:20:18, Ivan Djelic wrote: Hi Afzal, I tried to take your series of patches, but I had issues with the first [1] (I did not try the others): it depends on the following patch, which is not in the l2-mtd-2.6 tree: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg68258.html and it does not apply anyway to l2-mtd-2.6 because of (at least) the following patches: http://lists.infradead.org/pipermail/linux-mtd/2012-April/040631.html http://lists.infradead.org/pipermail/linux-mtd/2012-April/040724.html So, do you think you could rebase your series on l2-mtd-2.6 ? And maybe merge the 3 series into a single one, if they have circular dependencies ? I am not sure what the workflow should be here, all patch series were made over omap tree, if it is generated over mtd tree, similar issue would happen for omap platform patches. In any case, for your reference, the 3 series of patches of had been rebased over mtd tree, and is available, g...@gitorious.org:x0148406-public/linux-kernel.git gpmc-mtd. To prevent confusion the 3 patch series has not been posted. Tony, Artem, how should the conflict between omap mtd trees be handled for patch series ? I believe it is better to send the 3 patch series into one as mentioned by Ivan, and planning to do so, but in that case over which tree should it be based ? 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] ARM: OMAP2+: fix gpmc request_irq
Hi Santosh, On Fri, May 18, 2012 at 12:33:16, Shilimkar, Santosh wrote: + Afzal who has been doing quite a bit of GMPC work recently. On Fri, May 18, 2012 at 10:13 AM, Ming Lei ming@canonical.com wrote: The IRQ52 on OMAP2+ is not a shared interrupt line. If IRQF_SHARED is passed to request_irq and dev_id is set as NULL, request_irq will return -EINVAL. GPMC IRQ line can shared between the multiple devices you connect on it. : Are you sure it fails ? I just tried booting OMAP4 with 3.4-rc4 and don't see the irq failure error message. What I am missing ? Issue happens on l-o master, this was mentioned in [1], now I feel right solution is to keep dev-id. Regards Afzal [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg68749.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: tps65910
Hi Maxim, On Fri, May 18, 2012 at 13:23:53, Maxim Podbereznyy wrote: Hey guys! Who knows how to initialize and use tps65910 driver in linux with am3517? I designed a board using the Craneboard schematic as a reference. Mistral provides the kernel 2.6.32 and it works fine. Now I'm porting the BSP to kernel 3.0.17 and don't know how to implement voltage regulator features with the tps65910 driver (it appeared in kernel 3.0 ) under kernel 3.0 environment. I'm watching board-omap3beagle.c as a reference and tps65910.c driver and can't find any correlation. Any suggestions? Any help is appreciated! AM335X EVM (support not yet in mainline) works with tps65910, in case AM335X release helps you, git://arago-project.org/git/projects/linux-am33x.git v3.2_AM335xPSP_04.06.00.07 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] ARM: OMAP2+: fix gpmc request_irq
Hi Santosh, On Fri, May 18, 2012 at 15:43:31, Shilimkar, Santosh wrote: On the side note, looks like GMPC too needs to be converted to sparse IRQ since it seems to be creating a dummy irqchip and dispatching secondary handlers based on chip selects. GPMC dummy chip is being modified to so that irq descriptors are created dynamically [1], with a final goal of reaching something similar to [2], please let me know if any comments on those. Hi Ming, Requesting irq is called by driver of IP, while whether it is shared or not depends on SoC where IP lives, so ideally it seems platform should inform the driver whether it is shared driver should do what platform tells. Or else to be safe, it should be made shared always ? This may not make much sense now w.r.t gpmc, but would be applicable once gpmc becomes a driver (undergoing conversion), but may not be that important as there are no SoCs presently sharing gpmc interrupt (afaik) Regards Afzal [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg68745.html [2] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg67496.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 v4 04/39] ARM: OMAP2+: gpmc: Acquire NAND CS value
Hi Thomas, On Tue, May 15, 2012 at 12:00:32, Thomas Weber wrote: I need a help. Can someone familiar with boards - devkit8000, omap3touchbook, overo boards, let me know GPMC CS on which NAND is connected. On Devkit8000 the NAND is connected to CS0. Thanks for the information I thought that all NAND devices for booting are connected to CS0, because of ROM code? According to spruf98w.pdf: 25.4.7.4 NAND ... * The device is connected to CS0. Yes, expecting they should be, looking for confirmation. 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 1/3] ARM: OMAP2+: gpmc: Modify interrupt handling
Hi Tony, On Tue, May 15, 2012 at 20:08:34, Mohammed, Afzal wrote: Modify interrupt handling such that interrupts can be handled by GPMC client drivers using standard interrupt APIs rather than requiring the drivers to have knowledge about GPMC interrupt handling. Currently only NAND related interrupts has been considered (which is the case even without this change) as the only user of GPMC interrupt is NAND. : - ret = request_irq(gpmc_irq, gpmc_handle_irq, IRQF_SHARED, gpmc, NULL); If this series could not be considered for 3.5, to prevent failure of request_irq, either, 355f8ee ARM: OMAP2+: GPMC: resolve type-conversion warning from sparse, should be avoided, or diff [1] would be required, as shared irq needs dev-id. Regards Afzal [1] diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c index 46b09da..9e1b726 100644 --- a/arch/arm/mach-omap2/gpmc.c +++ b/arch/arm/mach-omap2/gpmc.c @@ -768,7 +768,7 @@ static int __init gpmc_init(void) irq++; } - ret = request_irq(gpmc_irq, gpmc_handle_irq, IRQF_SHARED, gpmc, NULL); + ret = request_irq(gpmc_irq, gpmc_handle_irq, 0, gpmc, NULL); if (ret) pr_err(gpmc: irq-%d could not claim: err %d\n, gpmc_irq, ret); -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 04/39] ARM: OMAP2+: gpmc: Acquire NAND CS value
Hi All, On Thu, May 03, 2012 at 14:12:11, Mohammed, Afzal wrote: Some boards depend on bootloader to update chip select value for NAND. It is felt that Kernel should not depend on bootloader to get CS, as for a particular board CS is hardwired and is fixed, hence this can directly be updated in Kernel. But as CS value for boards that depend on this behaviour is not available, educate gpmc driver to acquire chip select value for NAND. this ideally should be removed once CS for those boards are available. Do you know how many boards require this? If so which are those boards? devkit8000, beagle board, omap3touchbook, overo. Beagle board, found out to be zero. I need a help. Can someone familiar with boards - devkit8000, omap3touchbook, overo boards, let me know GPMC CS on which NAND is connected. Hi Tony, I am planning to provide actual CS # used for NAND on above boards instead of finding the value at runtime. Is there any reason that NAND CS# is found out at runtime ? (hence remove necessity of omap_nand_flash_init()). Presence of this also causes an additional dependency of bootloader. As CS # depends on wiring on the board, my understanding is that it will be fixed for a given board. Are you ok if acquiring NAND CS # is removed ? Removal of this helps in simplifying gpmc driver (undergoing conversion). Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4-alt 0/6] GPMC driver migrate one
Hi Tony, On Sat, May 12, 2012 at 01:30:49, Tony Lindgren wrote: Let's try to get merged these into l-o master around -rc2 so we can have them tested properly for a few weeks before v3.6 merge window. Sure, this will be my goal 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 v3] ARM: OMAP3: gpmc: add BCH ecc api and modes
Hi Ivan, On Thu, May 10, 2012 at 23:15:27, Ivan Djelic wrote: So, when Afzal's patches are pushed, I'll submit a new, single MTD patch. But this is not going to happen this merge window as I understood, may be not even the next one. We need to make UBIFS happy sooner than that, I think. So may be we go forward with your original patch? I'm OK with this too, as the patches are ready and tested. The MTD patch is [2], it depends on [1] which has been pushed, then dropped by Tony. Do you need me to repost [2] ? Tony, sorry to backpedal on this: would you re-push patch [1], if indeed Afzal's patches are not going to be merged soon ? In the meantime, I can prepare a patch on top of Afzal's to have a smooth transition w.r.t BCH support. What do you think ? A new series [A-D] has been sent for handling GPMC NAND registers by NAND driver itself. This is being targeted for 3.5. Hopefully if every one is in agreement, we can avoid patching for BCH support again when GPMC driver migration happens. And the effect of GPMC driver migration on NAND driver can be reduced when it happens. Can you try a patch on top of this series checks if it works for you, if more is required from my side let me know. Regards Afzal [A] http://marc.info/?l=linux-omapm=133675113218509w=2 [B] http://marc.info/?l=linux-omapm=133675123118577w=2 [C] http://marc.info/?l=linux-omapm=133675123718579w=2 [D] http://marc.info/?l=linux-omapm=133675124818580w=2 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4-alt 3/6] ARM: OMAP3: hwmod data: add gpmc
Hi Paul, On Thu, May 10, 2012 at 11:33:44, Mohammed, Afzal wrote: Hi Paul, On Tue, May 08, 2012 at 21:02:33, Paul Walmsley wrote: Major reason was that there are some boards that rely on bootloader settings, eg. kernel does not do any setting for smsc911x. I did not want to break those, at least it causes problem with omap3evm, see below But this is the whole point. The Linux GPMC driver and integration code should be setting up the GPMC registers based on board file and/or DT data, before the kernel touches any GPMC devices. We don't want to rely on the bootloader settings unless we absolutely have no other choice. Otherwise, the stability of the kernel could easily be impacted by the bootloader's GPMC timings and other register settings, and we want to minimize those potential sources of variation. So if we absolutely have no choice than to keep HWMOD_INIT_NO_RESET here, which doesn't sound like the case so far, we need to understand exactly why this is so. There are 14 out of 20 boards partially or fully relying on bootloader settings. I will try to do configuration for smsc911x in Kernel itself, this is the only one that can be tested from my side (on omap3evm), but there are other peripherals like NOR, quaduart, onenand-flash (different from omap-onenand), then smc91x (timings are not set from kernel for sdp boards), these would affect 7 boards of both omap2 omap3. To get configuration done from Kernel properly without having these boards is too tough for me. So I request you to keep HWMOD_INIT_NO_RESET (I will add configuration for smsc911x), please let me know your comments. ping Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4-alt 3/6] ARM: OMAP3: hwmod data: add gpmc
Hi Paul, On Tue, May 08, 2012 at 21:02:33, Paul Walmsley wrote: Major reason was that there are some boards that rely on bootloader settings, eg. kernel does not do any setting for smsc911x. I did not want to break those, at least it causes problem with omap3evm, see below But this is the whole point. The Linux GPMC driver and integration code should be setting up the GPMC registers based on board file and/or DT data, before the kernel touches any GPMC devices. We don't want to rely on the bootloader settings unless we absolutely have no other choice. Otherwise, the stability of the kernel could easily be impacted by the bootloader's GPMC timings and other register settings, and we want to minimize those potential sources of variation. So if we absolutely have no choice than to keep HWMOD_INIT_NO_RESET here, which doesn't sound like the case so far, we need to understand exactly why this is so. There are 14 out of 20 boards partially or fully relying on bootloader settings. I will try to do configuration for smsc911x in Kernel itself, this is the only one that can be tested from my side (on omap3evm), but there are other peripherals like NOR, quaduart, onenand-flash (different from omap-onenand), then smc91x (timings are not set from kernel for sdp boards), these would affect 7 boards of both omap2 omap3. To get configuration done from Kernel properly without having these boards is too tough for me. So I request you to keep HWMOD_INIT_NO_RESET (I will add configuration for smsc911x), please let me know your comments. Removing it causes multiple problems, one is that CS0 is valid after reset of module, with base address starting from zero, which causes requesting resource failure for CS0, with the way it is currently coded (GPMC resource starts from 1MB). Can't this be handled by using a custom hwmod reset function for the GPMC? No too familiar with hwmod, let me try that Another issue on OMAP3EVM is the detection of evm revision based on phy id, by reading hardcoded address, 0x2c00. It had to be skipped to reach till above mentioned. Looking at omap3_evm_get_revision() it seems that the OMAP3EVM detection happens by reading the Ethernet chip's revision register. So can't this be fixed by programming the GPMC appropriately to access the Ethernet chip first? I did not get you, may be let me explain the problem once again, 0x2c00 is physical address configured by bootloader. And omap3evm board file depends on this value to read eth chip revision register. Ideally this register should be accessed by smsc911x driver only. Once gpmc is reset, physical address for smsc911x can vary. With gpmc driver conversion, address of smsc911x register would be available only to smsc911x driver, and this address would not be setup until gpmc driver is probed, and so would not available for omap3_evm_get_revision, which has to be called before gpmc device registration. And here we are depending on eth revision register to find board revision. Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4-alt 0/6] GPMC driver migrate one
Hi Tony, On Wed, May 09, 2012 at 03:06:26, Tony Lindgren wrote: To resolve this and as per your earlier question on whether old along with new interface can be made to work parallely, here is suggestion from my end to deal with it. I think this is the only way to keep this all building and booting for each patch in the series, no? If so, then we should select this option. The first patch should be broken up into more readable patches, it seems that you can do that without breaking things. Bisectability has been maintained in the patches. Ok, I will proceed by keeping old new interface together, will try to achieve it in smaller patches and without hacks. Please let me know whether you are fine in taking patch series as in v4 (that converts all boards at once, with further revisions based on review comments). It seems that there are still some pending comments and we need to have the hwmod entries merged first by Paul. Ok, first I will try to get hwmod in. 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 v3] ARM: OMAP3: gpmc: add BCH ecc api and modes
Hi Ivan, On Wed, May 09, 2012 at 21:01:42, Tony Lindgren wrote: Note that I could prepare a new MTD patch with BCH ecc code included, allowing to drop the GPMC BCH ecc api. OK, let's do that then. I'll drop this patch and you can coordinate your patch with Afzal. Now that some review comments has been received on the series, let me try to come up with a suitable way forward and contact you within a day or two. Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 01/39] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Tue, May 08, 2012 at 20:38:24, Hunter, Jon wrote: Different ways of doing things, this looks cleaner to me as already it is checked, and time of execution in both cases would not differ much. Sure. However, I think that this could be simplified so that it is easier to read. At a minimum you may wish to add some comments to explain the purposes of the multi-level if-statements as it is not intuitive for the reader. Ok What happens if a device uses more than one wait-pin? In other words a device with two chip-selects that uses two wait-pins? Please re-read http://www.mail-archive.com/linux-omap@vger.kernel.org/msg67702.html and your reply Ok thats fine. Sorry for my repeated questions, but I think that this just highlights that this code is not clear in it purpose. So again may be some comments would make this all clearer. Ok Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 02/39] ARM: OMAP2+: gpmc: Adapt to HWMOD
Hi Jon, On Tue, May 08, 2012 at 20:43:19, Hunter, Jon wrote: In fact if you migrate to runtime pm then we should not have the clk_get in the gpmc_init any more. Even if converted to RPM, to get clk rate, clk_get has to be done somewhere, right ? Yes exactly. However, you could now do this in the driver itself like the probe function. Or may be just pass the frequency of the gpmc fclk to the driver and let the driver convert to the period. No reason we need to convert to the period outside of the driver. Hence, we can keep the function to do the conversion static in the driver and don't need to expose externally. But platform need period before driver is probed (for calculating peripheral timings to be passed for driver) Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 01/39] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Mon, May 07, 2012 at 21:32:58, Hunter, Jon wrote: + /* no waitpin */ + case 0: + break; + default: + dev_err(gpmc-dev, multiple waitpins selected on CS:%u\n, cs); + return -EINVAL; + break; + } Why not combined case 0 and default? Both are invalid configurations so just report invalid selection. Case 0 is not invalid, a case where waitpin is not used, default refers to when a user selects multiple waitpins wrongly. Ok. Then for case 0, just return here. If the polarity is set, you could print an error here. Different ways of doing things, this looks cleaner to me as already it is checked, and time of execution in both cases would not differ much. + if (gd-have_waitpin) { + if (gd-waitpin != idx || + gd-waitpin_polarity != polarity) { + dev_err(gpmc-dev, error: conflict: waitpin %u with polarity %d on device %s.%d\n, + gd-waitpin, gd-waitpin_polarity, + gd-name, gd-id); + return -EBUSY; + } + } else { Don't need the else as you are going to return in the above. Not always, only in case of error Ok, but seems that it can be simplified a little. What happens if a device uses more than one wait-pin? In other words a device with two chip-selects that uses two wait-pins? Please re-read http://www.mail-archive.com/linux-omap@vger.kernel.org/msg67702.html and your reply Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 02/39] ARM: OMAP2+: gpmc: Adapt to HWMOD
Hi Jon, On Mon, May 07, 2012 at 21:42:35, Hunter, Jon wrote: Clk_prd is a platform data passed to the driver, so platform code updates it, where else can it be done ? The point is that you can pass what ever you like. You do not need to pass the frequency you can pass the clock handle instead. As clk rate is required in platform code for timing calculation, and already available, period was passed What happens it the clk_get() of the gpmc_l3_clk fails during the init? Thanks for bringing this point, invalid clk_prd has to be handled by driver. In fact if you migrate to runtime pm then we should not have the clk_get in the gpmc_init any more. Even if converted to RPM, to get clk rate, clk_get has to be done somewhere, right ? Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 01/39] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Mon, May 07, 2012 at 21:53:34, Hunter, Jon wrote: Write protect is a pin and there is only one. Like the waitpins and CS signals this needs to be associated with a device. It would make sense that this is associated with the cs data. As far as platform are concerned, it is associated with cs, it is only that while configuring CS, it is propagated such that it is done once. Hmmm ... but here it looks like if write-protect is used you are going to turn it on. A feature like this seems that it does need to be handled by the device's driver. Enabling and disabling write-protect are functions that I would expect are exported to the device's driver and not directly enabled in the gpmc driver during probe. However, maybe is could be valid for the write protect to be enabled by default which could make sense. However, I don't see anywhere else in the driver to control it. Shouldn't we warn on multiple devices trying to use write-protect when parsing their configuration? Even if a single device sets write protect, kernel will log it. That does not seem sufficient. It should be flagged as an error. Write protect is a resource like the CS and waitpins and so I would have thought that it should be reserved in the same way. Isn't it valid for multiple devices to make use of write protect pin ? Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 00/39] OMAP GPMC driver conversion
Hi Tony, On Tue, May 01, 2012 at 17:49:03, Mohammed, Afzal wrote: GPMC driver conversion patch series. Some peripherals has GPMC helper functions, these has been modified to cater to the needs of GPMC driver. All the boards using GPMC has been adapted to use the new GPMC driver. GPMC HWMOD entry for OMAP2/3 has been added. On OMAP3, kernel does spit omap_hwmod: gpmc: cannot be enabled for reset (3), but peripherals connected via GPMC are working. Really shaky about OMAP2 GPMC HWMOD entry. Would be helpful if someone can help me in resolving warning on OMAP3 verify whether OMAP2 entry is proper. The series adapts to HWMOD. Drivers, NAND OneNAND of OMAP has been modified to make use of GPMC changes cleaned up the now unnecessary exported symbol usages. This series has been made on top of, 5e136da Linux-omap rebuilt: Updated to -rc5, A patch by Javier Martinez Canillas jav...@dowhile0.org, OMAP3: igep0020: Add support for Micron NAND Flash storage memory, has also been incorporated into the series as this was necessary for igep0020 board. This has been tested on omap3 evm (SMSC911x) beagle board (NAND) Please let me know your comments on this series. Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4-alt 3/6] ARM: OMAP3: hwmod data: add gpmc
Hi Paul, On Mon, May 07, 2012 at 16:42:16, Mohammed, Afzal wrote: +static struct omap_hwmod omap3xxx_gpmc_hwmod = { + .name = gpmc, + .class = omap3xxx_gpmc_hwmod_class, + .clkdm_name = l3_init_clkdm, + .mpu_irqs = omap3xxx_gpmc_irqs, + .main_clk = gpmc_fck, + .flags = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET, +}; Is there some reason why you are setting the HWMOD_INIT_NO_RESET flag here? Seems to me that the kernel should not rely on the bootloader GPMC configuration, but should use a configuration from the board file or DT. Major reason was that there are some boards that rely on bootloader settings, eg. kernel does not do any setting for smsc911x. I did not want to break those, at least it causes problem with omap3evm If HWMOD_NO_INIT_RESET is not present, it would break GPMC on many of the existing boards. New version of GPMC HWMOD patch has been posted with HWMOD_NO_IDLEST flag, which is a squashed one with OMAP2xxx HWMOD Regards AFzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 01/39] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Fri, May 04, 2012 at 21:50:16, Hunter, Jon wrote: As mentioned in the cover letter, Additional features that currently boards in mainline does not make use of like, waitpin interrupt handling, changes to leverage revision 6 IP differences has not been incorporated. Priority in this series is to convert into a driver, get all boards working on mainline. Once all boards are working with gpmc driver, these features which are not required for currently supported boards can be added later. Yes, but I meant why 2 and not say 5? Anyway, I think that this should be marked with a comment like a TODO so it is clear that this needs to be re-visited. Ok, it will be marked with TODO I think that it make more sense to have the wait-pin information part of the gpmc_cs_data structure for the following reasons ... Waitpin information is indeed a part of cs as far as boards are concerned, it is only that it gets propogated to device Why does the device's driver care? From my point of view, the wait-pin is just part of the interface setup/configuration. The external device may require this and assumes that this has been setup along with the CS and interface timing, but the device's driver should not care. Remember this is just a ready signal used to stall the access. Once configured, software should be unaware of it. By device, it is referred to gpmc device which only gpmc driver is aware, the peripheral device's driver is not at all aware. Also, any reason why waitpin_polarity is an int? I see you define LOW as -1, but I not sure why LOW cannot be 0 as 0 is programmed into the register for an active low wait signal. Only intention is not to alter default waitpin polarity value, i.e. if any board does make use of it w/o knowledge of Kernel, I don't want to break it, there may be an argument saying that the board code is buggy, while if some board does so, it is, but don't want to break working one. Here unless user explicitly set the flag, it does nothing on polarity Ok. Do such scenario's exist today? Please note that board code will be removed in the future and device-tree will replace. So if there are no cases today, I would not be concerned. Unless this could be something that has already been configured by the bootloader. However, in that case would be even call this function? Let me check this, here only intent was to play safe, as I have only two boards to test. +int gpmc_cs_reconfigure(char *name, int id, struct gpmc_cs_data *cs) What scenario is this function used in? May be worth adding a comment about the function. Ok, it was required for OneNAND, as it needs to reconfigure Ok, but why is that? Unless this is something generic about one-nand I don't comprehend. I have a high-level understanding of one-nand, but I am not familiar with the specifics that require it to be reconfigured. Not too familiar with OneNAND, from the code it has been understood that to set into sync mode, first it may have to set to async mode, read frequency from OneNAND, then setup sync mode for that frequency. + gpmc_setup_writeprotect(gpmc); Write protect is a pin and there is only one. Like the waitpins and CS signals this needs to be associated with a device. It would make sense that this is associated with the cs data. As far as platform are concerned, it is associated with cs, it is only that while configuring CS, it is propagated such that it is done once. Hmmm ... but here it looks like if write-protect is used you are going to turn it on. A feature like this seems that it does need to be handled by the device's driver. Enabling and disabling write-protect are functions that I would expect are exported to the device's driver and not directly enabled in the gpmc driver during probe. However, maybe is could be valid for the write protect to be enabled by default which could make sense. However, I don't see anywhere else in the driver to control it. Shouldn't we warn on multiple devices trying to use write-protect when parsing their configuration? Even if a single device sets write protect, kernel will log 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 v4 01/39] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Fri, May 04, 2012 at 21:57:10, Hunter, Jon wrote: - gpmc_write_reg(GPMC_SYSCONFIG, l); - gpmc_mem_init(); + switch (conf GPMC_WAITPIN_MASK) { + case GPMC_WAITPIN_0: + idx = GPMC_WAITPIN_IDX0; + break; + case GPMC_WAITPIN_1: + idx = GPMC_WAITPIN_IDX1; + break; + case GPMC_WAITPIN_2: + idx = GPMC_WAITPIN_IDX2; + break; + case GPMC_WAITPIN_3: + idx = GPMC_WAITPIN_IDX3; + break; + /* no waitpin */ + case 0: + break; + default: + dev_err(gpmc-dev, multiple waitpins selected on CS:%u\n, cs); + return -EINVAL; + break; + } Why not combined case 0 and default? Both are invalid configurations so just report invalid selection. Case 0 is not invalid, a case where waitpin is not used, default refers to when a user selects multiple waitpins wrongly. - /* initalize the irq_chained */ - irq = OMAP_GPMC_IRQ_BASE; - for (cs = 0; cs GPMC_CS_NUM; cs++) { - irq_set_chip_and_handler(irq, dummy_irq_chip, - handle_simple_irq); - set_irq_flags(irq, IRQF_VALID); - irq++; + switch (conf GPMC_WAITPIN_POLARITY_MASK) { + case GPMC_WAITPIN_ACTIVE_LOW: + polarity = LOW; + break; + case GPMC_WAITPIN_ACTIVE_HIGH: + polarity = HIGH; + break; + /* no waitpin */ + case 0: + break; + default: + dev_err(gpmc-dev, waitpin polarity set to low high\n); + return -EINVAL; + break; } Again, combine case 0 and default as these are invalid. Similar to above + if (gd-have_waitpin) { + if (gd-waitpin != idx || + gd-waitpin_polarity != polarity) { + dev_err(gpmc-dev, error: conflict: waitpin %u with polarity %d on device %s.%d\n, + gd-waitpin, gd-waitpin_polarity, + gd-name, gd-id); + return -EBUSY; + } + } else { Don't need the else as you are going to return in the above. Not always, only in case of error + gd-have_waitpin = true; + gd-waitpin = idx; + gd-waitpin_polarity = polarity; + } + + l = ~GPMC_CONFIG1_WAIT_PIN_SEL_MASK; + l |= GPMC_CONFIG1_WAIT_PIN_SEL(idx); + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, l); + } else if (polarity) { + dev_err(gpmc-dev, error: waitpin polarity specified with out wait pin number on device %s.%d\n, + gd-name, gd-id); + return -EINVAL; Drop this else-if. The above switch statements will report the bad configuration. This seems a bit redundant. This is required as switch statements will not report error if polarity is specified, w/o waitpin to be used. Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 02/39] ARM: OMAP2+: gpmc: Adapt to HWMOD
Hi Jon, On Fri, May 04, 2012 at 22:00:21, Hunter, Jon wrote: + + pdata-clk_prd = gpmc_get_fclk_period(); Does this need to be done here? May be this should be done in the probe function. You could store the handle to the main clk in the pdata. This is done so that migration of gpmc driver to the drivers folder would be smooth, remember that this function will still live here. Sure, but why call this here? Clk_prd is a platform data passed to the driver, so platform code updates it, where else can it be done ? Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4-alt 3/6] ARM: OMAP3: hwmod data: add gpmc
Hi Paul, On Sun, May 06, 2012 at 07:34:07, Paul Walmsley wrote: Did you notice the warning that this patch generates on boot? omap_hwmod: gpmc: cannot be enabled for reset (3) The HWMOD_NO_IDLEST hwmod flag is needed, since there is no GPMC IDLEST bit. Yes, putting that flag makes warning go away, Thanks ... Also, the OMAP2xxx GPMC hwmod data is missing. It can be combined with this patch. In v4, it has been sent as two patches - one for 3xxx one for 2xxx, do you want to combine both ? +static struct omap_hwmod omap3xxx_gpmc_hwmod = { + .name = gpmc, + .class = omap3xxx_gpmc_hwmod_class, + .clkdm_name = l3_init_clkdm, + .mpu_irqs = omap3xxx_gpmc_irqs, + .main_clk = gpmc_fck, + .flags = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET, +}; Is there some reason why you are setting the HWMOD_INIT_NO_RESET flag here? Seems to me that the kernel should not rely on the bootloader GPMC configuration, but should use a configuration from the board file or DT. Major reason was that there are some boards that rely on bootloader settings, eg. kernel does not do any setting for smsc911x. I did not want to break those, at least it causes problem with omap3evm, see below Another was that this was created based on, eb42b5d ARM: OMAP4: hwmod data: add GPMC and an internal one for AM335X, Both had HWMOD_INIT_NO_RESET flag. Removing it causes multiple problems, one is that CS0 is valid after reset of module, with base address starting from zero, which causes requesting resource failure for CS0, with the way it is currently coded (GPMC resource starts from 1MB). Even upon skipping CS0, kernel oops at seemingly unrelated location as follows, probably due to gpmc reset, as bootloader configured values are modified to reset value. Omap3 evm uses smsc911x. [2.660095] WARNING: at /home/afzal/dev/SA/src/-kernel/kernel/lockdep.c:956 __bfs+0x1f0/0x230() [2.669708] Modules linked in: [2.672973] [c001a568] (unwind_backtrace+0x0/0xf0) from [c003cfec] (warn_slowpath_common+0x4c/0x64) [2.682891] [c003cfec] (warn_slowpath_common+0x4c/0x64) from [c003d020] (warn_slowpath_null+0x1c/0x24) [2.693084] [c003d020] (warn_slowpath_null+0x1c/0x24) from [c00827d0] (__bfs+0x1f0/0x230) [2.702087] [c00827d0] (__bfs+0x1f0/0x230) from [c0084a74] (check_usage_forwards+0x60/0x10c) [2.711364] [c0084a74] (check_usage_forwards+0x60/0x10c) from [c00857b8] (mark_lock+0x1bc/0x64c) [2.720977] [c00857b8] (mark_lock+0x1bc/0x64c) from [c0086634] (__lock_acquire+0x9ec/0x1c64) [2.730255] [c0086634] (__lock_acquire+0x9ec/0x1c64) from [c0087e3c] (lock_acquire+0x98/0x100) [2.739715] [c0087e3c] (lock_acquire+0x98/0x100) from [c004a5f8] (run_timer_softirq+0x13c/0x378) [2.749359] [c004a5f8] (run_timer_softirq+0x13c/0x378) from [c00438b0] (__do_softirq+0xc8/0x1f4) [2.759002] [c00438b0] (__do_softirq+0xc8/0x1f4) from [c0043e64] (irq_exit+0x90/0x98) [2.767639] [c0043e64] (irq_exit+0x90/0x98) from [c00141dc] (handle_IRQ+0x50/0xac) [2.776000] [c00141dc] (handle_IRQ+0x50/0xac) from [c00086fc] (omap3_intc_handle_irq+0x54/0x68) [2.785552] [c00086fc] (omap3_intc_handle_irq+0x54/0x68) from [c0425724] (__irq_svc+0x4 Another issue on OMAP3EVM is the detection of evm revision based on phy id, by reading hardcoded address, 0x2c00. It had to be skipped to reach till above mentioned. Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 01/39] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Tue, May 01, 2012 at 23:23:00, Hunter, Jon wrote: Hi Afzal, Looks good! Some minor comments ... Thanks +#defineGPMC_WAITPIN_IDX0 0x0 +#defineGPMC_WAITPIN_IDX1 0x1 +#defineGPMC_WAITPIN_IDX2 0x2 +#defineGPMC_WAITPIN_IDX3 0x3 +#defineGPMC_NR_WAITPIN 0x4 How about an enum here? Also OMAP4 only have 3 wait pins and so the number is device dependent. Ok #define GPMC_MEM_START 0x #define GPMC_MEM_END 0x3FFF #define BOOT_ROM_SPACE 0x10/* 1MB */ @@ -64,6 +106,55 @@ #define ENABLE_PREFETCH(0x1 7) #define DMA_MPU_MODE 2 +#defineDRIVER_NAME omap-gpmc + +#defineGPMC_NR_IRQ 2 Why has this been changed to 2? It was 6 in the previous version. As we discussed before the number of IRQs should be detected based upon GPMC IP version. As mentioned in the cover letter, Additional features that currently boards in mainline does not make use of like, waitpin interrupt handling, changes to leverage revision 6 IP differences has not been incorporated. Priority in this series is to convert into a driver, get all boards working on mainline. Once all boards are working with gpmc driver, these features which are not required for currently supported boards can be added later. +struct gpmc_device { + char*name; + int id; + void*pdata; + unsignedpdata_size; + struct resource *per_res; + unsignedper_res_cnt; + struct resource *gpmc_res; + unsignedgpmc_res_cnt; + boolhave_waitpin; + unsignedwaitpin; + int waitpin_polarity; I think that it make more sense to have the wait-pin information part of the gpmc_cs_data structure for the following reasons ... Waitpin information is indeed a part of cs as far as boards are concerned, it is only that it gets propogated to device 1. If a device can use multiple CS, then it could use multiple wait signals. 2. Eventually, all board information needs to move to device tree. By having it in the pdata it will be easier to migrate to device tree. It may be nice to have have_waitpin be an integer too, that indicates if wait is used for both read and write or just one or the other. Also, any reason why waitpin_polarity is an int? I see you define LOW as -1, but I not sure why LOW cannot be 0 as 0 is programmed into the register for an active low wait signal. Only intention is not to alter default waitpin polarity value, i.e. if any board does make use of it w/o knowledge of Kernel, I don't want to break it, there may be an argument saying that the board code is buggy, while if some board does so, it is, but don't want to break working one. Here unless user explicitly set the flag, it does nothing on polarity + if (idx != ~0x0) { + if (gd-have_waitpin) { + if (gd-waitpin != idx || + gd-waitpin_polarity != polarity) { + dev_err(gpmc-dev, error: conflict: waitpin %u with polarity %d on device %s.%d\n, + gd-waitpin, gd-waitpin_polarity, + gd-name, gd-id); + return -EBUSY; + } + } else { + gd-have_waitpin = true; + gd-waitpin = idx; + gd-waitpin_polarity = polarity; + } I am not sure about the above code. What happens if a device has multiple CS signals and uses multiple wait signals? I am also not sure why gpmc_setup_cs_waitpin and gpmc_setup_waitpin cannot be combined. I think that it would be a fair assumption to make that anyone using the waitpin does so inconjunction with the CS (I think that they would have too). However, if there is a reason for splitting it up this way please explain why. Initially it was done per CS, the problem happened while trying to convert tusb6010. It had multiple CS, but using same waitpin. Problem with incorporating this onto CS is we have to sacrifice on wait pin conflict prevention capability. The code now handles case of wait pin conflict per device, the code can be extended to have multiple wait pin per device also. But I prefer to defer it to later, not as part of this series, as this feature what you asking for is not required for any of the existing boards. I can add it in this series if there is a strong opinion in the community for the same in this series. Policy that is being followed here is first sit, then straighten legs, ;) +static int gpmc_setup_cs_nonres(struct gpmc *gpmc, +
RE: [PATCH v4 02/39] ARM: OMAP2+: gpmc: Adapt to HWMOD
Hi Jon, On Wed, May 02, 2012 at 02:11:48, Hunter, Jon wrote: + + pdata-clk_prd = gpmc_get_fclk_period(); Does this need to be done here? May be this should be done in the probe function. You could store the handle to the main clk in the pdata. This is done so that migration of gpmc driver to the drivers folder would be smooth, remember that this function will still live here. + pr_err(error: clk_get on %s\n, oh-main_clk); + return -EINVAL; } clk_enable(gpmc_l3_clk); I would have thought we should be able to remove the gpmc_init function completely by now. Most of the code should be moved to the probe function. Also now with hwmod in place, we should be able to remove the clk_enable/disable functions and use the pm_runtime APIs instead. There was no plan to add rpm in this series, but now that you have brought it up, I will adapt the driver to rpm. Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 04/39] ARM: OMAP2+: gpmc: Acquire NAND CS value
Hi Jon, On Wed, May 02, 2012 at 02:46:02, Hunter, Jon wrote: Some boards depend on bootloader to update chip select value for NAND. It is felt that Kernel should not depend on bootloader to get CS, as for a particular board CS is hardwired and is fixed, hence this can directly be updated in Kernel. But as CS value for boards that depend on this behaviour is not available, educate gpmc driver to acquire chip select value for NAND. this ideally should be removed once CS for those boards are available. Do you know how many boards require this? If so which are those boards? devkit8000, beagle board, omap3touchbook, overo. Beagle board, found out to be zero. Should this code be marked with a FIXME? Will let Tony decide 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 v4 06/39] ARM: OMAP2+: onenand: return value in init function
Hi Sergei, On Tue, May 01, 2012 at 22:19:37, Sergei Shtylyov wrote: + +#if defined(CONFIG_MTD_ONENAND_OMAP2) || \ + defined(CONFIG_MTD_ONENAND_OMAP2_MODULE) You can use IS_ENABLED(CONFIG_MTD_ONENAND_OMAP2) instead these two. Thanks for educating me. 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: OMAP3EVM not booting on l-o master
Hi Kevin, On Tue, May 01, 2012 at 02:10:28, Hilman, Kevin wrote: Afzal, care to give this one a test as well? It should have the same result but is a more targetted fix. With and ack from Tero and a Tested-by from you, I'll post this to the list and and queue it up for v3.5. Over what commit id, should this patch be applied ?, unable to apply it over 297624c ARM: OMAP3: PM: Remove IO Daisychain control from cpuidle, the one that I tested your previous patch. Regards Afzal From 274ded7fbe573bbbe45db80bdc1989272077c011 Mon Sep 17 00:00:00 2001 From: Kevin Hilman khil...@ti.com Date: Fri, 27 Apr 2012 16:05:51 -0700 Subject: [PATCH] ARM: OMAP3: PM: leave PRCM interrupts disabled until explicitly enabled. -- 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: OMAP3EVM not booting on l-o master
Hi Kevin, On Tue, May 01, 2012 at 19:38:00, Hilman, Kevin wrote: Afzal, care to give this one a test as well? It should have the same result but is a more targetted fix. With and ack from Tero and a Tested-by from you, I'll post this to the list and and queue it up for v3.5. With an additional diff [1], your patch makes omap3evm boot Without [1], compiler error Regards Afzal [1] diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c index a8c6bd8..a0309de 100644 --- a/arch/arm/mach-omap2/prm2xxx_3xxx.c +++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c @@ -15,6 +15,7 @@ #include linux/errno.h #include linux/err.h #include linux/io.h +#include linux/irq.h #include common.h #include plat/cpu.h -- 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: OMAP3EVM not booting on l-o master
Hi Kevin, On Sat, Apr 28, 2012 at 04:45:21, Hilman, Kevin wrote: Afzal, care to test the patch below to see if it fixes your boot problem on OMAP3EVM with the IO chain series? Your patch fixes the boot problem 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 v3 0/9] Convert OMAP GPMC to driver
Hi Tony, On Wed, Apr 25, 2012 at 22:14:25, Tony Lindgren wrote: Once driver is acceptable, platform code for other peripherals connected via GPMC would be adapted to make use of GPMC driver. And then the board modifications. But before that HWMOD entry has to be populated for respective SoC(s ?). No, we can't do it this way, it breaks things. We need to first convert everything to use the new GPMC driver, then move it. Sorry, my statements were not clear, I meant once driver is acceptable still living in mach-omap2, platform code dealing with peripherals on gpmc would be adapted to make use of gpmc driver. Moving driver to new location is only planned as a separate commit after, before mentioned changes. Hope with this clarification we are on same track. Now DESTINATION FOR THIS DRIVER has to be decided. Original plan was to consider GPMC as MFD. The peripheral(s) connected to GPMC being considered childs of MFD. Let's not put it into MFD. This is a bus driver. But that decision can wait as we cleary have quite a few things to convert first under arch/arm/mach-omap2. Various options that could be seen so far on where this driver can go, 1. mfd 2. misc 3. drivers/platform/arm/ (create an new one?) 4. memory (create a new one ?) It's a parallel bus driver, not memory not misc, not MFD. Greg suggested to send gpmc driver to new memory folder. In the next version, gpmc driver would still be in mach-omap2, moving to new location can be done later. 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: [TMP] OMAP3EVM: Test gpmc nand smsc911x
Hi Tony, On Wed, Apr 25, 2012 at 22:17:26, Tony Lindgren wrote: Obviously we can't merge any of this if until all the board-*.c files are changed and tested. Can you maybe still keep the old interfaces in addition to the new ones so we can do the conversion one board-*.c file at a time while keeping things working? As there are peripherals using helper functions, to handle those, you meant to create a new altered similar function to deal for each too ?, while keeping existing functions as such. i.e. like having gpmc_smsc911x_init gpmc_smsc911x_new_init for each peripheral ?, with gpmc_smsc911x_new_init using gpmc driver, and gpmc_smsc911x_init as is. 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: OMAP3EVM not booting on l-o master
Hi Tero, On Mon, Apr 23, 2012 at 17:29:48, Kristo, Tero wrote: Okay thats good (although I wonder why the attachment got corrupted.) Did you check if the device suspends / resumes properly also? Can you check what do you have in the /proc/interrupts for the hwmod_io interrupt just after boot / after one suspend? Device suspends resumes properly. At boot 2, after one suspend-resume 3 Regards Afzal
RE: OMAP3EVM not booting on l-o master
Hi Tero, With this branch you meant, HEAD @ 297624c ARM: OMAP3: PM: Remove IO Daisychain control from cpuidle, right ? (the one Paul suggested to try) Regards Afzal On Mon, Apr 23, 2012 at 14:54:04, Kristo, Tero wrote: On Fri, 2012-04-06 at 07:52 +, Mohammed, Afzal wrote: Hi Paul, On Fri, Apr 06, 2012 at 12:43:06, Paul Walmsley wrote: Perhaps you might be willing to add some debugging to omap_mux_late_init() to find out what part of that function is causing it to hang? It is getting hung as interrupt handler omap_hwmod_mux_handle_irq is being repeatedly called. Hi Afzal, can you try the attached patch with this branch and omap3evm board? I don't have the board myself so I can't test it myself (I tested this with omap3beagle and it works with that one.) -Tero
RE: OMAP3EVM not booting on l-o master
Hi Tero, On Mon, Apr 23, 2012 at 15:43:22, Kristo, Tero wrote: can you try the attached patch with this branch and omap3evm board? I don't have the board myself so I can't test it myself (I tested this with omap3beagle and it works with that one.) With your patch, OMAP3EVM boots normally. Tested by creating same changes as in your patch as below (patch could not be applied, seems it was corrupted) Regards Afzal diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c index 65c3391..17349e3 100644 --- a/arch/arm/mach-omap2/mux.c +++ b/arch/arm/mach-omap2/mux.c @@ -41,6 +41,7 @@ #include control.h #include mux.h #include prm.h +#include pm.h #define OMAP_MUX_BASE_OFFSET 0x30/* Offset from CTRL_BASE */ #define OMAP_MUX_BASE_SZ 0x5ca @@ -427,6 +428,13 @@ static irqreturn_t omap_hwmod_mux_handle_irq(int irq, void *unused) return IRQ_HANDLED; } +static irqreturn_t omap34xx_hwmod_mux_handle_irq(int irq, void *unused) +{ + omap_hwmod_for_each(_omap_hwmod_mux_handle_irq, NULL); + prcm_int_ack_io(); + return IRQ_HANDLED; +} + /* Assumes the calling function takes care of locking */ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state) { @@ -792,6 +800,7 @@ static int __init omap_mux_late_init(void) { struct omap_mux_partition *partition; int ret; + irq_handler_t irq_handler; list_for_each_entry(partition, mux_partitions, node) { struct omap_mux_entry *e, *tmp; @@ -812,12 +821,19 @@ static int __init omap_mux_late_init(void) } } +#ifdef CONFIG_PM + if (cpu_is_omap34xx()) + irq_handler = omap34xx_hwmod_mux_handle_irq; + else + irq_handler = omap_hwmod_mux_handle_irq; + ret = request_irq(omap_prcm_event_to_irq(io), - omap_hwmod_mux_handle_irq, IRQF_SHARED | IRQF_NO_SUSPEND, + irq_handler, IRQF_SHARED | IRQF_NO_SUSPEND, hwmod_io, omap_mux_late_init); if (ret) pr_warning(mux: Failed to setup hwmod io irq %d\n, ret); +#endif omap_mux_dbg_init(); diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h index 36fa90b..09ba9e7 100644 --- a/arch/arm/mach-omap2/pm.h +++ b/arch/arm/mach-omap2/pm.h @@ -20,6 +20,7 @@ extern void omap3_pm_off_mode_enable(int); extern void omap_sram_idle(void); extern int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state); extern int omap3_idle_init(void); +extern void prcm_int_ack_io(void); extern int omap4_idle_init(void); extern int omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused); extern int (*omap_pm_suspend)(void); diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c index afe3dda..cea4408 100644 --- a/arch/arm/mach-omap2/pm34xx.c +++ b/arch/arm/mach-omap2/pm34xx.c @@ -182,14 +182,10 @@ static int prcm_clear_mod_irqs(s16 module, u8 regs, u32 ignore_bits) return c; } -static irqreturn_t _prcm_int_handle_io(int irq, void *unused) +void prcm_int_ack_io(void) { - int c; - - c = prcm_clear_mod_irqs(WKUP_MOD, 1, + prcm_clear_mod_irqs(WKUP_MOD, 1, ~(OMAP3430_ST_IO_MASK | OMAP3430_ST_IO_CHAIN_MASK)); - - return c ? IRQ_HANDLED : IRQ_NONE; } static irqreturn_t _prcm_int_handle_wakeup(int irq, void *unused) @@ -683,16 +679,6 @@ static int __init omap3_pm_init(void) goto err1; } - /* IO interrupt is shared with mux code */ - ret = request_irq(omap_prcm_event_to_irq(io), - _prcm_int_handle_io, IRQF_SHARED | IRQF_NO_SUSPEND, pm_io, - omap3_pm_init); - - if (ret) { - pr_err(pm: Failed to request pm_io irq\n); - goto err1; - } - ret = pwrdm_for_each(pwrdms_setup, NULL); if (ret) { printk(KERN_ERR Failed to setup powerdomains\n);
RE: [PATCH v4 0/7] Add TI EMIF SDRAM controller driver
Hi Aneesh, On Fri, Apr 13, 2012 at 01:28:55, V, Aneesh wrote: Thanks. I will wait for your review then. Once I have your comments I will re-work and submit in the new directory structure proposed. Do you have a plan on submitting EMIF driver in the new proposed (drivers/memory) directory. Or shall I submit GPMC driver by creating the new memory directory. Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 0/7] Add TI EMIF SDRAM controller driver
Hi Santosh, On Mon, Apr 23, 2012 at 16:34:46, Shilimkar, Santosh wrote: Please go ahead in creating directory. Thanks for the quick reply. 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 3/9] ARM: at91: add at91sam9g20ek boards dt support
+ omap ml Hi Jean, Andrew, Nicolas, On Wed, Apr 11, 2012 at 21:31:13, Jean-Christophe PLAGNIOL-VILLARD wrote: + ahb { + apb { + dbgu: serial@f200 { + status = okay; + }; + + usart0: serial@fffb { + status = okay; + }; + + usart1: serial@fffb4000 { + status = okay; + }; + + macb0: ethernet@fffc4000 { + phy-mode = rmii; + status = okay; + }; + + usb1: gadget@fffa4000 { + atmel,vbus-gpio = pioC 5 0; + status = okay; + }; + }; + + nand0: nand@4000 { + nand-bus-width = 8; + nand-ecc-mode = soft; + nand-on-flash-bbt; + status = okay; I have a few queries about handling static memory controller (SMC) of ATMEL 1. How is SMC configured when DT is used ? 2. With d6a0166 atmel/nand: add DT support, ek_add_device_nand() is no more present (which was earlier configuring SMC), so is SMC configuration handled by Kernel on DT boot or is it done by bootloader when DT ? 3. How ek_add_device_dm9000() is going to be achieved with DT ? 4. Is there any plan to create a driver out of SMC code use DT with it ? Reason for bringing these queries is that TI OMAP chips have GPMC [1], SMC in ATMEL seems somewhat similar and currently GPMC is configured in platform code, we are creating a driver out of that code [2]. There are different views on where GPMC driver should go, mfd, misc folders etc, but could not yet get concrete suggestions even with a loud cry. If ATMEL is also going driver way, then probably our voice together may be heard and hopefully it will expedite the matter. Regards Afzal [1] GPMC (General Purpose Memory Controller) in brief: GPMC is an unified memory controller dedicated to interfacing external memory devices like Asynchronous SRAM like memories and application specific integrated circuit devices. Asynchronous, synchronous, and page mode burst NOR flash devices NAND flash Pseudo-SRAM devices GPMC has to be configured as required by timings of the connected peripheral. It needs to be configured only initially. Once it is configured it can be used to handle different protocols like NAND, NOR. Various kinds of devices like ethernet, uart, usb, fpga etc can work using GPMC interface. GPMC has a seperate additional functionality of NAND handling [2] https://lkml.org/lkml/2012/4/5/210 -- 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/9] ARM: at91: add at91sam9g20ek boards dt support
Hi Jean, On Thu, Apr 12, 2012 at 17:15:59, Jean-Christophe PLAGNIOL-VILLARD wrote: If ATMEL is also going driver way, then probably our voice together may be heard and hopefully it will expedite the matter. I'm going to add it too my idea was to create something similiar as the pintrl you register the ddifferent bank supported buy the SoC and then the driver request the bank for the dev_name at soc level you will set the default timings and then the drvier may manipulate them I already update the API of sam9_smc to abstract the access to the register. Is SMC code being converted to driver ? Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 0/7] Add TI EMIF SDRAM controller driver
Hi Greg, On Thu, Apr 12, 2012 at 18:40:45, Greg KH wrote: On Thu, Apr 12, 2012 at 12:17:49PM +0530, Santosh Shilimkar wrote: I was hoping that we will have some thing like drivers/memory/* but since it doesn't exist, we used drivers/misc. Why not create it? I have no objection to that, it makes it more obvious as to what this really is. There is another memory controller used in a few TI SoCs, namely GPMC [1], do you prefer having it too there. As of now it is not a driver, platform code handles GPMC, a patch series for converting it into a driver (but still residing in platform folder) was sent a few days back [2,3]. Regards Afzal [1] GPMC (General Purpose Memory Controller) in brief: GPMC is an unified memory controller dedicated to interfacing external memory devices like Asynchronous SRAM like memories and application specific integrated circuit devices. Asynchronous, synchronous, and page mode burst NOR flash devices NAND flash Pseudo-SRAM devices GPMC has to be configured as required by timings of the connected peripheral. It needs to be configured only initially. Once it is configured it can be used to handle different protocols like NAND, NOR. Various kinds of devices like ethernet, uart, usb, fpga etc can work using GPMC interface. GPMC has a seperate additional functionality of NAND handling [2] https://lkml.org/lkml/2012/4/5/210 [3] https://lkml.org/lkml/2012/4/5/212 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 0/7] Add TI EMIF SDRAM controller driver
Hi Greg, On Thu, Apr 12, 2012 at 19:40:50, Greg KH wrote: There is another memory controller used in a few TI SoCs, namely GPMC [1], do you prefer having it too there. Sure, why not? Thanks a lot, we were struggling to find a suitable location for the driver. 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 v3 1/9] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Tue, Apr 10, 2012 at 01:20:37, Hunter, Jon wrote: Because num_irq (or available irqs for fictitious irq chip) is platform specific. Ok, so OMAP_GPMC_NR_IRQS is defined and will not vary from device to device, so why pass this? Why not use it directly? Because OMAP_GPMC_NR_IRQS is platform specific, final intention is to not have any platform specific header files in GPMC driver, not sure as of now whether it would be possible. So keeping platform specific things away from the driver as much as possible. And consider scenario where GPMC IP is used in other than OMAP family, even though this a theoretical case, wanted to stress the point that intention is to keep driver platform independent. Or else dynamic allocation of irqs may have to be used. Furthermore, GPMC_NR_IRQ is defined as 6 which is correct for OMAP2/3 but not for OMAP4/5, it is 5. Therefore, we need to detect whether we are using an OMAP2/3 or OMAP4+ and set num_irqs based upon this. This could be done in the probe and we can avoid passing this. Is it dependent on OMAPX or GPMC IP version? if it is IP version, then driver can be enhanced to handle it, if not, platform has to pass this information. + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); + if (res == NULL) + dev_warn(gpmc-dev, Failed to get resource: irq\n); + else + gpmc-master_irq = res-start; Why not return an error if the IRQ is not found? We don't know if anyone will be trying to use them. Why do you want to do that ? Because this indicates a BUG :-) I disagree, this need not be considered a bug always, for eg. If gpmc irq is not connected to intc If someone (say NAND) wants to work with irq, and if interrupt is not available, NAND driver can fail, and suppose smsc911x ethernet is present and he is not using gpmc irq, if we fail here, smsc911x also would not work, which would have worked otherwise. It is a different matter that even NAND setup will happen properly, even if interrupt is not available, it is only when NAND is told to work with IRQ, it fails, please see nand patches. And as of now in mainline (with the code as is), there is not a single board that will need gpmc irq for gpmc peripherals to work. I feel we should try to get more devices working rather than preventing more devices from working, when it could have. I understand your point, but then you are hiding a BUG. If someone introduces a BUG that causes this to fail, then it is easier to detect, find and fix. From my perspective get the resources should never fail and if it does and break the driver for all devices, then so be it, because a BUG has been introduced. Ok, this may not be critical at this point but still is should not fail. Agree for resources which are a must for device to work, not for resources that can enhance its capability. + for (gdq = gp-device_pdata, gd = gpmc-device; *gdq; gdq++, i++) { + ret = gpmc_setup_device(*gdq, gd, gpmc); + if (IS_ERR_VALUE(ret)) + dev_err(gpmc-dev, gpmc setup on %s failed\n, + (*gdq)-name); + else + gd++; + } Would a while loop be simpler? My preference is to go with for Ok, just wondering if this could be cleaned up a little. For travelling through array of pointers, for looks natural to me, if you have a better way, please send it, it can be folded in next version. 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 v3 1/9] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Wed, Apr 11, 2012 at 00:53:14, Hunter, Jon wrote: I agree with your argument but I was thinking today only OMAP uses the GPMC so we could not worry about this. Ok, leave as-is, but can we modify the code as follows as the else if is not really needed... if (gpmc-num_irq GPMC_NR_IRQ) { dev_warn(gpmc-dev, Insufficient interrupts for device\n); return -EINVAL; } gpmc-num_irq = GPMC_NR_IRQ; Yes, it is better Furthermore, GPMC_NR_IRQ is defined as 6 which is correct for OMAP2/3 but not for OMAP4/5, it is 5. Therefore, we need to detect whether we are using an OMAP2/3 or OMAP4+ and set num_irqs based upon this. This could be done in the probe and we can avoid passing this. Is it dependent on OMAPX or GPMC IP version? if it is IP version, then driver can be enhanced to handle it, if not, platform has to pass this information. Here are the GPMC IP revisions ... OMAP5430 = 0x0060 OMAP4430 = 0x0060 OMAP3630 = 0x0050 OMAP3430 = 0x0050 So this should work for OMAP. We should check OMAP2 as well. What about the AMxxx devices? I badly needed this information, thanks. AM3359 = 0x0060, it has only 2 waitpin interrupts + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); + if (res == NULL) + dev_warn(gpmc-dev, Failed to get resource: irq\n); + else + gpmc-master_irq = res-start; Why not return an error if the IRQ is not found? We don't know if anyone will be trying to use them. Why do you want to do that ? Because this indicates a BUG :-) I disagree, this need not be considered a bug always, for eg. If gpmc irq is not connected to intc Ok, so for devices existing today this indicates a bug ;-) I do not want to consider that case to be bug enough for probe to fail, there are other drivers which does similar enhancing its use cases, eg. 1e351a9 mfd: Make TPS65910 usable without interrupts At a minimum you need to improve the error handing here. If the platform_get_resource fails you are still calling gpmc_setup_irq() which appears to be pointless. It would be better if the gpmc irq chip is not initialised in this case so that drivers attempting to request these irqs failed. Please see gpmc_setup_irq, if irq is not present, it returns in the beginning, and gpmc_irq_chip is not initialized in that case. + for (gdq = gp-device_pdata, gd = gpmc-device; *gdq; gdq++, i++) { + ret = gpmc_setup_device(*gdq, gd, gpmc); + if (IS_ERR_VALUE(ret)) + dev_err(gpmc-dev, gpmc setup on %s failed\n, + (*gdq)-name); + else + gd++; + } Would a while loop be simpler? My preference is to go with for Ok, just wondering if this could be cleaned up a little. For travelling through array of pointers, for looks natural to me, if you have a better way, please send it, it can be folded in next version. Could you have num_devices to indicate how many platform devices there are and then a simple for-loop of 0 to num_devices? This will cause coding to be done by platform to be less simple, and my preference is not to use another variable 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 v3 4/9] ARM: OMAP2+: gpmc-nand: populate gpmc configs
Hi Jon, On Wed, Apr 11, 2012 at 00:54:58, Hunter, Jon wrote: +#defineGPMC_NAND_CONFIG_NUM4 Where does 4 come from? It depends on the use case, for NAND, with the way it was being configured (or number of times gpmc_cs_configure was being called) 4 are needed. 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 v3 1/9] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Fri, Apr 06, 2012 at 01:51:41, Hunter, Jon wrote: +struct gpmc_irq{ + unsignedirq; + u32 regval; Are you using regval to indicate the bit-mask? If so, maybe call it bitmask instead. Yes, bitmask would be better. + switch (gpmc-irq[i].regval) { + case GPMC_IRQ_WAIT0EDGEDETECTION: + case GPMC_IRQ_WAIT1EDGEDETECTION: + case GPMC_IRQ_WAIT2EDGEDETECTION: + case GPMC_IRQ_WAIT3EDGEDETECTION: + val = __ffs(gpmc-irq[i].regval); + val -= __ffs(GPMC_IRQ_WAIT0EDGEDETECTION); + gpmc_cs_configure(cs-cs, + GPMC_CONFIG_WAITPIN, val); Why is the configuration of the wait pin done here? It is possible to use the wait pin may be used without enabling the interrupt. Where do you handle allocating the wait pins to ensure two devices don't attempt to use the same one? Like how the CS are managed. Also, where you you configure the polarity of the wait pins? I would have thought it would make sense to have the wait pin configured as part of the cs configuration. I will revisit waitpin configurations. + for (i = 0, j = 0, cs = gdp-cs_data; i gdp-num_cs; cs++, i++) { + dev-gpmc_res[j] = gpmc_setup_cs_mem(cs, gdp, gpmc); + if (dev-gpmc_res[j++].flags IORESOURCE_MEM) + j += gpmc_setup_cs_irq(gpmc, gdp, cs, + dev-gpmc_res + j); + else { + dev_err(gpmc-dev, error: setup for %s\n, gdp-name); + devm_kfree(gpmc-dev, dev-gpmc_res); + dev-gpmc_res = NULL; + dev-num_gpmc_res = 0; + return -EINVAL; + } } This section of code is not straight-forward to read. I see what you are doing, but I am wondering if this could be improved. First of all, returning a structure from a function is making this code harder to read. Per the CodingStyle document in the kernel, it is preferred for a function to return success or failure if the function is an action, like a setup. Secondly, do you need to pass cs, gdp and gpmc to gpmc_setup_cs_mem()? It appears that gdp and gpmc are only used for prints. You could probably avoid passing gdp and move the print to outside this function. This section will be modified to make it clearer. + if (gpmc-num_irq GPMC_NR_IRQ) { + dev_warn(gpmc-dev, Insufficient interrupts for device\n); + return -EINVAL; + } else if (gpmc-num_irq GPMC_NR_IRQ) + gpmc-num_irq = GPMC_NR_IRQ; Hmmm ... why not just have ... if (gpmc-num_irq != GPMC_NR_IRQ) { dev_warn(...); return -EINVAL; } This will cause irq setup to fail if num_irq GPMC_NR_IRQ, even though irq setup could have been done w/o any problem, only because platform indicated willingness to accommodate more number of interrupts than actually required for this device. This also raises the question why bother passing num_irq if we always want it to be GPMC_NR_IRQ? Why not always initialise them all driver? Because num_irq (or available irqs for fictitious irq chip) is platform specific. + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); + if (res == NULL) + dev_warn(gpmc-dev, Failed to get resource: irq\n); + else + gpmc-master_irq = res-start; Why not return an error if the IRQ is not found? We don't know if anyone will be trying to use them. Why do you want to do that ? If someone (say NAND) wants to work with irq, and if interrupt is not available, NAND driver can fail, and suppose smsc911x ethernet is present and he is not using gpmc irq, if we fail here, smsc911x also would not work, which would have worked otherwise. It is a different matter that even NAND setup will happen properly, even if interrupt is not available, it is only when NAND is told to work with IRQ, it fails, please see nand patches. And as of now in mainline (with the code as is), there is not a single board that will need gpmc irq for gpmc peripherals to work. I feel we should try to get more devices working rather than preventing more devices from working, when it could have. + for (gdq = gp-device_pdata, gd = gpmc-device; *gdq; gdq++, i++) { + ret = gpmc_setup_device(*gdq, gd, gpmc); + if (IS_ERR_VALUE(ret)) + dev_err(gpmc-dev, gpmc setup on %s failed\n, + (*gdq)-name); + else + gd++; + } Would a while loop be simpler? My preference is to go with for The above loop appears to terminate when *gdq == 0. Is this always guaranteed? In other words, safe? This is a
RE: OMAP3EVM not booting on l-o master
Hi Paul, On Fri, Apr 06, 2012 at 12:43:06, Paul Walmsley wrote: Perhaps you might be willing to add some debugging to omap_mux_late_init() to find out what part of that function is causing it to hang? It is getting hung as interrupt handler omap_hwmod_mux_handle_irq is being repeatedly called. 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: OMAP3EVM not booting on l-o master
Hi Paul, On Thu, Apr 05, 2012 at 15:03:36, Paul Walmsley wrote: Could you try booting with initcall_debug and posting the boot log? Logs as follows, Regards Afzal Uncompressing Linux... done, booting the kernel. [0.00] Booting Linux on physical CPU 0 [0.00] Linux version 3.4.0-rc1-11705-g33fc21e (af...@linux-psp-server.india.ext.ti.com) (gcc version 4.5.3 20110311 (prerelease) (GCC) ) #5 SMP Thu Apr 5 15:19:27 IST 2012 [0.00] CPU: ARMv7 Processor [411fc083] revision 3 (ARMv7), cr=10c53c7d [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT nonaliasing instruction cache [0.00] Machine: OMAP3 EVM [0.00] Memory policy: ECC disabled, Data cache writeback [0.00] On node 0 totalpages: 32512 [0.00] free_area_init_node: node 0, pgdat c06b3cc0, node_mem_map c0c0a000 [0.00] Normal zone: 256 pages used for memmap [0.00] Normal zone: 0 pages reserved [0.00] Normal zone: 32256 pages, LIFO batch:7 [0.00] OMAP3430/3530 ES3.1 (l2cache iva sgx neon isp ) [0.00] Clocking rate (Crystal/Core/MPU): 26.0/332/500 MHz [0.00] PERCPU: Embedded 8 pages/cpu @c0d0e000 s11456 r8192 d13120 u32768 [0.00] pcpu-alloc: s11456 r8192 d13120 u32768 alloc=8*4096 [0.00] pcpu-alloc: [0] 0 [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 32256 [0.00] Kernel command line: console=ttyO0,115200n8 root/dev/ram rw earlyprintk mem=128M initrd=0x81338000,0x1f6c2f initcall_debug debug [0.00] PID hash table entries: 512 (order: -1, 2048 bytes) [0.00] Dentry cache hash table entries: 16384 (order: 4, 65536 bytes) [0.00] Inode-cache hash table entries: 8192 (order: 3, 32768 bytes) [0.00] Memory: 127MB = 127MB total [0.00] Memory: 114536k/114536k available, 16536k reserved, 0K highmem [0.00] Virtual kernel memory layout: [0.00] vector : 0x - 0x1000 ( 4 kB) [0.00] fixmap : 0xfff0 - 0xfffe ( 896 kB) [0.00] vmalloc : 0xc880 - 0xff00 ( 872 MB) [0.00] lowmem : 0xc000 - 0xc800 ( 128 MB) [0.00] modules : 0xbf00 - 0xc000 ( 16 MB) [0.00] .text : 0xc0008000 - 0xc05d2e34 (5932 kB) [0.00] .init : 0xc05d3000 - 0xc0621cc0 ( 316 kB) [0.00] .data : 0xc0622000 - 0xc06b5958 ( 591 kB) [0.00].bss : 0xc06b597c - 0xc0c09be0 (5457 kB) [0.00] Hierarchical RCU implementation. [0.00] NR_IRQS:474 [0.00] IRQ: Found an INTC at 0xfa20 (revision 4.0) with 96 interrupts [0.00] Total of 96 interrupts on 1 active controller [0.00] OMAP clockevent source: GPTIMER1 at 32768 Hz [0.00] sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999ms [0.00] Console: colour dummy device 80x30 [0.00] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar [0.00] ... MAX_LOCKDEP_SUBCLASSES: 8 [0.00] ... MAX_LOCK_DEPTH: 48 [0.00] ... MAX_LOCKDEP_KEYS:8191 [0.00] ... CLASSHASH_SIZE: 4096 [0.00] ... MAX_LOCKDEP_ENTRIES: 16384 [0.00] ... MAX_LOCKDEP_CHAINS: 32768 [0.00] ... CHAINHASH_SIZE: 16384 [0.00] memory used by lock dependency info: 3695 kB [0.00] per task-struct memory footprint: 1152 bytes [0.001129] Calibrating delay loop... 497.82 BogoMIPS (lpj=1941504) [0.085906] pid_max: default: 32768 minimum: 301 [0.086883] Security Framework initialized [0.087219] Mount-cache hash table entries: 512 [0.093780] CPU: Testing write buffer coherency: ok [0.094970] CPU0: thread -1, cpu 0, socket -1, mpidr 0 [0.095001] calling init_static_idmap+0x0/0xe0 @ 1 [0.095123] Setting up static identity map for 0x804278b8 - 0x80427928 [0.095153] initcall init_static_idmap+0x0/0xe0 returned 0 after 0 usecs [0.095184] calling omap4_sar_ram_init+0x0/0x60 @ 1 [0.095214] initcall omap4_sar_ram_init+0x0/0x60 returned -12 after 0 usecs [0.095245] initcall omap4_sar_ram_init+0x0/0x60 returned with error code -12 [0.095275] calling omap_l2_cache_init+0x0/0xec @ 1 [0.095306] initcall omap_l2_cache_init+0x0/0xec returned -19 after 0 usecs [0.095336] calling spawn_ksoftirqd+0x0/0x54 @ 1 [0.095825] initcall spawn_ksoftirqd+0x0/0x54 returned 0 after 0 usecs [0.095855] calling init_workqueues+0x0/0x3c8 @ 1 [0.097015] initcall init_workqueues+0x0/0x3c8 returned 0 after 0 usecs [0.097045] calling migration_init+0x0/0x78 @ 1 [0.097106] initcall migration_init+0x0/0x78 returned 0 after 0 usecs [0.097137] calling cpu_stop_init+0x0/0xd8 @ 1 [0.097473] initcall cpu_stop_init+0x0/0xd8 returned 0 after 0 usecs [0.097503] calling rcu_scheduler_really_started+0x0/0x18 @ 1 [0.097534] initcall
RE: OMAP3EVM not booting on l-o master
Hi Govindraj, On Thu, Apr 05, 2012 at 15:53:25, R, Govindraj wrote: Can you try this patch [1], Just to confirm its serial mux issue however still the patch is not aligned on how to fix it. With that patch too, Kernel does not boot. 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: OMAP3EVM not booting on l-o master
Hi Paul, On Thu, Apr 05, 2012 at 15:37:42, Paul Walmsley wrote: I just booted the 'io_chain_devel_3.5' branch from git://git.pwsan.com/linux-2.6 on a 35xx ES3.0 BeagleBoard with no problems. Could you please try booting this branch on your OMAP3EVM? I am unable to fetch using git protocol, hence tried, http://git.pwsan.com/linux-2.6 http://git.pwsan.com/linux-2.6.git but giving error, fatal: http://git.pwsan.com/linux-2.6/info/refs not found: did you run git update-server-info on the server? Any other to try ? 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: OMAP3EVM not booting on l-o master
Hi Tony, On Thu, Apr 05, 2012 at 22:46:53, Tony Lindgren wrote: What happens if you disable CONFIG_OMAP_MUX? Then it boots properly. 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: OMAP3EVM not booting on l-o master
Hi Paul, On Fri, Apr 06, 2012 at 01:22:20, Paul Walmsley wrote: http://git.kernel.org/?p=linux/kernel/git/pjw/omap-devel.git;a=summary If you open it in your web browser, it shows http and https URLs for the tree that you should be able to pull from. You'll want the io_chain_devel_3.5 branch. OMAP3EVM does not boot with this branch, hung at the same point. 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: OMAP3EVM not booting on l-o master
Hi, On Wed, Apr 04, 2012 at 11:06:39, Mohammed, Afzal wrote: OMAP3EVM is not booting on l-o master, with default configuration, HEAD @33fc21e Linux-omap rebuilt: Updated to v3.4-rc1, merged in most of pending branches. Reverting merge commit 58adb29 Merge branch 'io_chain_devel_3.4' of git://git.pwsan.com/linux-2.6 into prm makes OMAP3EVM boot. Bisecting into this merge could not be done as build was breaking for most of the individual commits in the merge (except when HEAD is the last commit of the merge, as below). Assuming broken builds to be good for bisect, it could get built for, cca6430 ARM: OMAP3: PM: Remove IO Daisychain control from cpuidle but with it OMAP3EVM does not boot. 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
OMAP3EVM not booting on l-o master
Hi, OMAP3EVM is not booting on l-o master, with default configuration, HEAD @33fc21e Linux-omap rebuilt: Updated to v3.4-rc1, merged in most of pending branches. It was booting on l-o master two weeks old with same setup. Any clues on resolving this issue ? Regards Afzal Logs as follows, Uncompressing Linux... done, booting the kernel. [0.00] Booting Linux on physical CPU 0 [0.00] Linux version 3.4.0-rc1-11705-g33fc21e (af...@linux-psp-server.india.ext.ti.com) (gcc version 4.5.3 20110311 (prerelease) (GCC) ) #1 SMP Wed Apr 4 10:27:23 IST 2012 [0.00] CPU: ARMv7 Processor [411fc083] revision 3 (ARMv7), cr=10c53c7d [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT nonaliasing instruction cache [0.00] Machine: OMAP3 EVM [0.00] Memory policy: ECC disabled, Data cache writeback [0.00] OMAP3430/3530 ES3.1 (l2cache iva sgx neon isp ) [0.00] Clocking rate (Crystal/Core/MPU): 26.0/332/500 MHz [0.00] PERCPU: Embedded 8 pages/cpu @c0d0e000 s11456 r8192 d13120 u32768 [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 32256 [0.00] Kernel command line: console=ttyO0,115200n8 root/dev/ram rw earlyprintk mem=128M initrd=0x81338000,0x1f6c2f [0.00] PID hash table entries: 512 (order: -1, 2048 bytes) [0.00] Dentry cache hash table entries: 16384 (order: 4, 65536 bytes) [0.00] Inode-cache hash table entries: 8192 (order: 3, 32768 bytes) [0.00] Memory: 127MB = 127MB total [0.00] Memory: 114536k/114536k available, 16536k reserved, 0K highmem [0.00] Virtual kernel memory layout: [0.00] vector : 0x - 0x1000 ( 4 kB) [0.00] fixmap : 0xfff0 - 0xfffe ( 896 kB) [0.00] vmalloc : 0xc880 - 0xff00 ( 872 MB) [0.00] lowmem : 0xc000 - 0xc800 ( 128 MB) [0.00] modules : 0xbf00 - 0xc000 ( 16 MB) [0.00] .text : 0xc0008000 - 0xc05d2e34 (5932 kB) [0.00] .init : 0xc05d3000 - 0xc0621cc0 ( 316 kB) [0.00] .data : 0xc0622000 - 0xc06b5958 ( 591 kB) [0.00].bss : 0xc06b597c - 0xc0c09be0 (5457 kB) [0.00] Hierarchical RCU implementation. [0.00] NR_IRQS:474 [0.00] IRQ: Found an INTC at 0xfa20 (revision 4.0) with 96 interrupts [0.00] Total of 96 interrupts on 1 active controller [0.00] OMAP clockevent source: GPTIMER1 at 32768 Hz [0.00] sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999ms [0.00] Console: colour dummy device 80x30 [0.00] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar [0.00] ... MAX_LOCKDEP_SUBCLASSES: 8 [0.00] ... MAX_LOCK_DEPTH: 48 [0.00] ... MAX_LOCKDEP_KEYS:8191 [0.00] ... CLASSHASH_SIZE: 4096 [0.00] ... MAX_LOCKDEP_ENTRIES: 16384 [0.00] ... MAX_LOCKDEP_CHAINS: 32768 [0.00] ... CHAINHASH_SIZE: 16384 [0.00] memory used by lock dependency info: 3695 kB [0.00] per task-struct memory footprint: 1152 bytes [0.001129] Calibrating delay loop... 497.82 BogoMIPS (lpj=1941504) [0.085906] pid_max: default: 32768 minimum: 301 [0.086883] Security Framework initialized [0.087249] Mount-cache hash table entries: 512 [0.093780] CPU: Testing write buffer coherency: ok [0.094970] CPU0: thread -1, cpu 0, socket -1, mpidr 0 [0.095092] Setting up static identity map for 0x804278b8 - 0x80427928 [0.097442] Brought up 1 CPUs [0.097473] SMP: Total of 1 processors activated (497.82 BogoMIPS). [0.124389] dummy: [0.127258] NET: Registered protocol family 16 [0.128936] GPMC revision 5.0 [0.143035] gpiochip_add: registered GPIOs 0 to 31 on device: gpio [0.143737] OMAP GPIO hardware version 2.5 [0.145141] gpiochip_add: registered GPIOs 32 to 63 on device: gpio [0.147125] gpiochip_add: registered GPIOs 64 to 95 on device: gpio [0.149536] gpiochip_add: registered GPIOs 96 to 127 on device: gpio [0.151428] gpiochip_add: registered GPIOs 128 to 159 on device: gpio [0.153320] gpiochip_add: registered GPIOs 160 to 191 on device: gpio [0.163635] omap_mux_init: Add partition: #1: core, flags: 0 [0.182525] Reprogramming SDRC clock to 33200 Hz [0.290496] hw-breakpoint: debug architecture 0x4 unsupported. [0.310638] omap-mcbsp.2: alias fck already exists [0.311676] omap-mcbsp.3: alias fck already exists [0.318115] OMAP DMA hardware revision 4.0 [0.409606] bio: create slab bio-0 at 0 [0.414764] dummy: [0.414947] dummy: Failed to create debugfs directory [0.422912] SCSI subsystem initialized [0.425506] omap2_mcspi omap2_mcspi.1: master is unqueued, this is deprecated [0.429168] omap2_mcspi omap2_mcspi.2: master is unqueued, this is deprecated [0.431213]
RE: [RFC][PATCH 1/5] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Tue, Mar 27, 2012 at 21:01:56, Hunter, Jon wrote: These are for the peripheral resources not in control of GPMC, like gpio irq. These are copied in gpmc_create_child. Right, I see they are copied during gpmc_create_child. However, I don't see where they are initialised before that. The function gpmc_setup_child is only initialising gpmc_res and gpmc_num_res and not res and num_res. So I still don't see who is initialising these before they are copied. These are to be initialized by platform code, NAND have none so is not currently seen, but devices like ethernet have to. I see you did not incorporate any clean-up in v2. Do you want me to send you some patches to include? Thanks for offering to help. Cleanup is being deferred to be done once driver is finalized, I may need your help later. 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: [RFC][PATCH 1/5] ARM: OMAP2+: gpmc: driver conversion
Hi Benoit, On Fri, Mar 23, 2012 at 21:09:21, Cousson, Benoit wrote: But EMIF does not have anything to do in MFD either :-) What was the feedback for this series? We discussed that at Linaro connect, but it looks like MFD is becoming the place for miscellaneous drivers that we do not know where to put. Maybe we should introduce a driver/memory/ directory for memory controller. At least for EMIF. In the case of GMPC, it is slightly different because it can handle NOR/NAND memory but as well behave like an ISA bus controller for Ethernet ISA chip. But since it can control several devices thanks the chipselects lines it has, it behaves like a multi-protocol bus controller. But in anycase, it does not look like an MFD for my point of view. For me a MFD is like a small soc, it does contain several completely unrelated block (Power, Audio, GPIO...), but does share some memory space / IRQ lines. Is this the only controller doing that kind of stuff in the kernel so far? An additional intention of sending RFC early was to find out whether selection of MFD was acceptable. Coding so far has been done so that it can easily move to MFD or other types. Frankly I had not done much research on where GPMC driver should belong, my thought was first convert it into a driver (but vaguely in mind it may have to go to MFD), later decide on where it should go. Probably task of finding correct place for GPMC driver should happen in parallel with driver conversion. For davinci EMIF, afaik, no conclusion has been reached yet. I will try to find whether there is anything similar already in Kernel, and find out a suitable place for GPMC driver. If an initial patch just to rearrange the code to have similar section together then new changes in a another patch, would that be fine? Well, if this is just comestic, I will even do that after the driver conversion. Because if you do that before you will move some piece of code that you might completely delete later. So you should fix the code first and then potentially, move some part if that will improve the readability. Ok 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: [RFC][PATCH 0/5] Convert GPMC to driver
Hi Santosh, On Fri, Mar 23, 2012 at 16:05:47, Shilimkar, Santosh wrote: Much needed series. Thanks Afzal for doing it. Probably going through this first version, your thanks would have evaporated; else if (thanks 0) redirected to Vaibhav Hiremath Sekhar Nori. 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: [RFC][PATCH 1/5] ARM: OMAP2+: gpmc: driver conversion
Hi Balbi, On Fri, Mar 23, 2012 at 21:59:00, Balbi, Felipe wrote: yeah, I was thinking about drivers/ocd (off-chip devices) or drivers/mmio... and that should be flexible enough to hold gpmc, lli and c2c (from OMAP's perspective). Ok, I will check feasibility of having GPMC driver at those places. I wouldn't do that. I would only move after the driver is cleaned up. Are you concerned with the diffstat alone ? that'd be silly :-p Ok 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: [RFC][PATCH 1/5] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Sat, Mar 24, 2012 at 04:51:13, Hunter, Jon wrote: +struct gpmc_child { + char*name; + int id; + struct resource *res; + unsignednum_res; + struct resource gpmc_res[GPMC_CS_NUM]; Does this imply a gpmc child device can use more than one chip-select? I am trying to understand the link between number of resources and GPMC_CS_NUM. Yes, relevant portion in commit message as follows, A peripheral connected to GPMC can have multiple address spaces using different chip select. Hence GPMC driver has been provided capability to distinguish this scenario, i.e. create platform devices only once for each connected peripheral, and not for each configured chip select. The peripheral that made it necessary was tusb6010. + unsignedgpmc_num_res; + void*pdata; + unsignedpdata_size; +}; Does pdata_size need to be stored? If pdata is always of type gpmc_device_pdata then you could avoid using void * and elsewhere use sizeof. This is the size of platform data of peripheral connected to GPMC, like NAND. - reg_addr = gpmc_base + GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx; - __raw_writel(val, reg_addr); + reg_addr = gpmc-io_base + GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx; + writel(val, reg_addr); } I understand this was inherited from the original code, but I think that we should drop the GPMC_CS0_OFFSET. We are already passing an index and so we should use this as an offset. This obviously implies changing the defintion of the GPMC_CS_ registers in gpmc.h. This would save one addition too. Ok +/* This is a duplication of an existing function; before GPMC probe + invocation, platform code may need to find divider value, hence + other function (gpmc_cs_calc_divider) is not removed, functions + like it that are required by platform, probably can be put in + common omap platform file. gpmc_calc_divider will get invoked + only after GPMC driver gets probed. gpmc_cs_calc_divider is not + invoked by GPMC driver to cleanly separate platform driver + code, although both should return sme value. */ -int gpmc_nand_read(int cs, int cmd) +static int gpmc_calc_divider(u32 sync_clk) { - int rval = -EINVAL; - - switch (cmd) { - case GPMC_NAND_DATA: - rval = gpmc_cs_read_byte(cs, GPMC_CS_NAND_DATA); - break; + int div; + u32 l; - default: - printk(KERN_ERR gpmc_read_nand_ctrl: Not supported\n); - } - return rval; + l = sync_clk + (gpmc-fclk_rate - 1); This was a little confusing to me at first. When I see fclk_rate I think frequency (eg. clk_get_rate()) and not period. The original code had a function called gpmc_get_fclk_period. I would consider renaming the variable to fclk_period to be clear. Agree +#define GPMC_SET_ONE(reg, st, end, field) \ Nit-pick, set-one is a bit generic, maybe GPMC_SET_TIME? Agree + if (cpu_is_omap34xx()) { + GPMC_SET_ONE(GPMC_CS_CONFIG6, 16, 19, wr_data_mux_bus); + GPMC_SET_ONE(GPMC_CS_CONFIG6, 24, 28, wr_access); + } OMAP4/5 also has the above fields and so maybe this should be (!cpu_is_omap24xx()). Will try to remove the usage of cpu_is_xxx itself + + /* caller is expected to have initialized CONFIG1 to cover +* at least sync vs async +*/ + l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1); + if (l (GPMC_CONFIG1_READTYPE_SYNC | GPMC_CONFIG1_WRITETYPE_SYNC)) { Is it valid to have READTYPE != WRITETYPE? I am wondering if there should be a check here to see if READTYPE and WRITETYPE are not equal? Seems possible, but not sure, will look into this + printk(KERN_DEBUG GPMC CS%d CLK period is %lu ns (div %d)\n, + cs, (div * gpmc_get_fclk_period()) / 1000, div); + l= ~0x03; I think we should define GPMC_CONFIG1_FCLK_DIV_MASK for this. Ok + *base = (l 0x3f) GPMC_CHUNK_SHIFT; Define GPMC_CONFIG7_BASEADDRESS_MASK Ok + mask = (l 8) 0x0f; Define GPMC_CONFIG7_MASKADDRESS_MASK/SHIFT Ok +static __devinit int gpmc_setup_child(struct gpmc_device_pdata *gdev) Nit-pick, gdev was a bit confusing to me, maybe just pdata instead? Ok + gpmc-child_device[i].gpmc_res[j] = res; So struct res is a local variable and you are storing in a global structure? Did you intend to store the address of the pdata struct passed? No, copy res structure + gpmc-ecc_used = -EINVAL; Why not 0? That may modify default behavior of OMAP NAND driver w.r.t ECC, will check this. + spin_lock_init(gpmc-mem_lock); + platform_set_drvdata(pdev, gpmc); l = gpmc_read_reg(GPMC_REVISION); - printk(KERN_INFO GPMC revision %d.%d\n, (l 4) 0x0f, l 0x0f); - /* Set smart idle mode and automatic L3 clock
RE: [RFC][PATCH 1/5] ARM: OMAP2+: gpmc: driver conversion
Hi Jon, On Mon, Mar 26, 2012 at 23:12:26, Hunter, Jon wrote: Also, I don't see where the gpmc_child-res and gpmc_child-num_res are actually used. Are these needed? These are for the peripheral resources not in control of GPMC, like gpio irq. These are copied in gpmc_create_child. Gpmc_device_data is dedicated to each CS, gpmc_pdata is required at least to inform driver about clock rate. Ok, understood! So the struct gpmc_device_pdata only has a single chip-select entry and so looking at the code you will have multiple instances of this structure of a gpmc device that uses more than one chip-select. Any reason you did it this way and not have a single pdata struct for each device defining all chip-selects it uses? When coding started, multiple CS for a peripheral was not taken into consideration, later handling multiple CS was incorporated making it this way. But your suggestion seems better to me, will see if code can modified accordingly in later patch series (v2 already sent) Generally, as the change involved moving a lot of code, seems more reviews are on those than the actual changes than what I intended to get reviewed, next patch series will be modified not to move existing code, hence some of your suggested changes may not be present in it, probably those to be done as another cleanup patch. Yes I understand. However, it is a good opportunity to clean some of this up even if it is existing code :-) Ok 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: [TMP][PATCH 5/5] OMAP3EVM: Test gpmc-nand
Hi Jon, On Mon, Mar 26, 2012 at 23:21:50, Hunter, Jon wrote: I see this is marked as a temp patch, but this is actually needed to register the device. Actually, we would need to do this for all boards, right? Yes, as NAND support on OMAP3EVM was not in mainline, made it TMP. Once GPMC driver interfaces are acceptable, other boards too will be modified accordingly. Rather than registering the device here, may be we should add a function in arch/arm/mach-omap2/devices.c called omap_gpmc_init() that is called from all of the boards files passing the pdata structure. Then the omap_gmpc_init() function should use omap_device_build() API to register the device. If you look at arch/arm/mach-omap2/devices.c you can look at omap4_keyboard_init() as an example. Let me know if this is clear. Yes that is the final plan, i.e. once driver is ok, it has to be adapted to HWMOD, and this will be taken care. 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: [TMP][PATCH 5/5] OMAP3EVM: Test gpmc-nand
Hi Jon, On Tue, Mar 27, 2012 at 10:49:32, Mohammed, Afzal wrote: Hi Jon, On Mon, Mar 26, 2012 at 23:21:50, Hunter, Jon wrote: I see this is marked as a temp patch, but this is actually needed to register the device. Actually, we would need to do this for all boards, right? Yes, as NAND support on OMAP3EVM was not in mainline, made it TMP. Once GPMC driver interfaces are acceptable, other boards too will be modified accordingly. Forgot to add that this patch was meant for testing and as the way platforms deal with GPMC may change, this was kept as TMP for now. 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: [RFC][PATCH 1/5] ARM: OMAP2+: gpmc: driver conversion
Hi Benoit, On Fri, Mar 23, 2012 at 15:07:30, Cousson, Benoit wrote: Final destination aimed for this driver is MFD. Why? Are you sure this is appropriate? This is not really a multifunction device but rather a bus device that can manage multiple kind of devices. Agree, this not an MFD, but can we call this a bus?, as there is nothing like GPMC protocol. We considered it logically as MFD proceeded and there was a similar attempt for davinci EMIF [1,2]. arch/arm/mach-omap2/gpmc.c | 1083 +++- You should probably find the proper location first, move the code and convert to driver. I will let Tony comment but this is the strategy today for all this pseudo driver that should not be in OMAP arch directory anymore. Please let me know whether you have any suggestions on where GPMC driver should live. + printk(KERN_DEBUG GPMC CS%d: %-10s* %3d ns, %3d ticks= %d\n, Nit, but since you are cleaning extensively this code, you should use pr_ macros instead or even dev_ macros since you do have a real driver now with real devices now. Sure, this was overlooked +struct gpmc_cs_config { + u32 config1; + u32 config2; + u32 config3; + u32 config4; + u32 config5; + u32 config6; + u32 config7; + int is_valid; +}; OK, so this code was just moved and not removed. Becasue of these big code move, the patch is not super readable. We cannot really see what part is new and what was changed. Maybe you should try to split that in sevarl patches or minized the move. Yes, I was really in two minds before the coding started. Lot of code in this patch has been moved from one place to other, this was done to put codes that handle similar things together, so that trees can be made visible easily in the forest. And once the patch is applied, as similar sections are together, it may be easy to make further changes If an initial patch just to rearrange the code to have similar section together then new changes in a another patch, would that be fine? +static int __init gpmc_clk_init(void) +{ + char *ck = NULL; + + if (cpu_is_omap24xx()) + ck = core_l3_ck; + else if (cpu_is_omap34xx()) + ck = gpmc_fck; + else if (cpu_is_omap44xx()) + ck = gpmc_ck; Please don't do that anymore. The CLKDEV array is done to create alias and avoid this kind of hacks. Moreover you should rely on hwmod for device creation and thus main clock alias will already be populated for free. There are not added, they are existing code, result of rearranging the code. These sections were given not given much importance as these won't go into driver. But noted the point you are making. Thanks for your quick comments. Regards Afzal [1] http://lkml.indiana.edu/hypermail/linux/kernel/1202.2/03228.html [2] http://davinci-linux-open-source.1494791.n2.nabble.com/PATCH-arm-davinci-configure-davinci-aemif-chipselects-through-OF-tt7059739.html#none -- 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] cpufreq: OMAP: specify range for voltage scaling
Hi Kevin, On Fri, Mar 02, 2012 at 03:37:51, Hilman, Kevin wrote: Thanks, will queue this with the CPUfreq changes for MPU DVFS. Actually, not quite yet... After some testing with the SMPS regulators, this won't quite work with the current SMPS regulators. Does this actually work with the regulators you're using? Yes, it works on AM335X (doesn't have VC/VP) using TPS65910, it always sets voltage to lowest step possible within minus OPP tolerance range. 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/RFT 1/1] OMAP2+: cpufreq: scale voltage along with frequency
Hi Kevin, On Thu, Feb 23, 2012 at 00:12:06, Hilman, Kevin wrote: And in my patch plus - minus was not used as regulator framework will try to set voltage for the least voltage which sometimes corresponds to exact OPP required value. sometimes? I was not clear in my previous statement, let me explain it differently. Regulator framework will always try to set lowest voltage in the range. Upon, regulator_set_voltage(reg, OPPVOLT, (OPPVOLT+RESOLUTION-1)), if OPPVOLT is a value that can be set by the regulator, that will be set by the regulator, this is what I meant by 'sometimes'. Otherwise it will set the next possible step. Using your example above, what if the closest value was 1.259V? Wouldn't you then need +/- range? In that case, it will set to next step after 1.259V. If +/- is used, it may happen that SoC will work for a particular frequency at a voltage lower than it has been characterized (if you ask me to define characterized, I don't know, but what I know is that SoC can work at a lower frequency for the voltage of an OPP, but not vice versa). Still as voltage will be only very less than that specified by OPP, it may not be a problem to use +/- Please have a look at my v2. The drivers will get pre and post notifiers, with the caveat that upon DVFS failure, the freq in the post notifier might be the same as the one in the pre notifier, indicating that no change was made. Yes, I overlooked your freq.new update for error condition. Earlier while adding support for DVFS on AM335X, it was noticed that, lpj was going wrong after dvfs failure, then those error notifiers were added to recover properly. Later it was realized that it was due to a bug in cpufreq framework, d08de0c1 [CPUFREQ] update lpj only if frequency has changed fixed it, probably those extra handling in my patch is not required now, if drivers can understand your pre post notifiers properly (if drivers can't, then maybe it is driver problem) 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/RFT 1/1] OMAP2+: cpufreq: scale voltage along with frequency
Hi Kevin, On Thu, Feb 23, 2012 at 10:58:57, Mohammed, Afzal wrote: Using your example above, what if the closest value was 1.259V? Wouldn't you then need +/- range? In that case, it will set to next step after 1.259V. If +/- is used, it may happen that SoC will work for a particular frequency at a voltage lower than it has been characterized (if you ask me to define characterized, I don't know, but what I know is that SoC can work at a lower frequency for the voltage of an OPP, but not vice versa). Still as voltage will be only very less than that specified by OPP, it may not be a problem to use +/- Upon discussing with Sekhar, realized that I was wrong in the above statements, +/- OPP tolerance range should be used. 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/RFT 1/1] OMAP2+: cpufreq: scale voltage along with frequency
Hi Kevin, On Wed, Feb 22, 2012 at 05:36:12, Hilman, Kevin wrote: In this case, volt comes from the OPP table, and was requested using a rounding call into the OPP table, so the resolution problem is handled there. If 'volt' cannot be set by the regulator, then the OPP tables are also broken. Also, in your patch, you only add some offset. If you want to be approximate, shouldn't you have plus and minus? IMO, we should let the OPP table handle that, and not the CPUfreq driver. I have a different opinion. Consider following case, Voltage to set for OPPX is 1262.5mV and regulator has steps of 10mV, and suppose regulator steps are .., 1260mV, 1270mV etc. Regulator framework will not be able to set voltage for OPPX (certainly if set_voltage_sel is used) if no range specified. But instead if range of 1262.5 - (1262.5 + 10 - 1) is provided, regulator can certainly set voltage for 1270mV (a nearest value) Resolution in my patch was not meant to be resolution per se, it was meant so that a voltage step can always be found for the regulator (assuming worst resolution is 12.5mV), and for regulator to get a step, resolution had to be used. Ideal solution may be to use a variable to have a default resolution of worst regulator and update it to that of regulator used (perhaps making use of something like module parameters) If regulator for a particular SoC is changed, exact OPP voltage may not be settable, but a near value should be sufficient, this can't be achieved if range is not specified. That happens exactly for AM335X, voltage for one of OPP is 1.26V, but as regulator cannot set the value, it is being set at 1.2625V and if no range is specified, it will not work. And in my patch plus - minus was not used as regulator framework will try to set voltage for the least voltage which sometimes corresponds to exact OPP required value. For cpufreq omap driver to work with various regulators, it would be better to specify ranges. I agree, my version is not very robust in the face of errors from the regulator framework. Hoever, I'm not crazy about the extra notifications in your proposed patch. I think it's cleaner to always pre and post notify. If there's a failure, the post notify will have the same freq as the pre-notify, but that's not a big problem. Yes, agree that error handling path is bulky. A problem that can happen is that drivers who has registered cpufreq notifier will think that frequency has changed, that may cause problem in addition to delay time getting altered. But whether these are worth handling with a penalty of bulky error handling, probably you know better. 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/RFT 1/1] OMAP2+: cpufreq: scale voltage along with frequency
Hi Kevin, On Fri, Feb 17, 2012 at 00:50:43, Hilman, Kevin wrote: + /* scaling up? scale voltage before frequency */ + if (mpu_reg (freqs.new freqs.old)) + regulator_set_voltage(mpu_reg, volt, volt); Probably voltage ranges has to be specified, otherwise if I understand correctly, if exact voltage 'volt' is a value that cannot be set by voltage regulator, it may not work properly. ret = clk_set_rate(mpu_clk, freqs.new * 1000); - freqs.new = omap_getspeed(policy-cpu); + /* scaling down? scale voltage after frequency */ + if (mpu_reg (freqs.new freqs.old)) + regulator_set_voltage(mpu_reg, volt, volt); + + freqs.new = omap_getspeed(policy-cpu); It would be better to handle error cases too, we have a patch for doing DVFS for AM335X as follows Regards Afzal 8-- --- drivers/cpufreq/omap-cpufreq.c | 97 +--- 1 files changed, 91 insertions(+), 6 deletions(-) diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c index 5d04c57..a897a9e 100644 --- a/drivers/cpufreq/omap-cpufreq.c +++ b/drivers/cpufreq/omap-cpufreq.c @@ -25,6 +25,7 @@ #include linux/opp.h #include linux/cpu.h #include linux/module.h +#include linux/regulator/consumer.h #include asm/system.h #include asm/smp_plat.h @@ -37,6 +38,8 @@ #include mach/hardware.h +#define DEFAULT_RESOLUTION 12500 + #ifdef CONFIG_SMP struct lpj_info { unsigned long ref; @@ -52,6 +55,7 @@ static atomic_t freq_table_users = ATOMIC_INIT(0); static struct clk *mpu_clk; static char *mpu_clk_name; static struct device *mpu_dev; +static struct regulator *mpu_reg; static int omap_verify_speed(struct cpufreq_policy *policy) { @@ -78,6 +82,8 @@ static int omap_target(struct cpufreq_policy *policy, unsigned int i; int ret = 0; struct cpufreq_freqs freqs; + struct opp *opp; + int volt_old = 0, volt_new = 0; if (!freq_table) { dev_err(mpu_dev, %s: cpu%d: no freq table!\n, __func__, @@ -105,16 +111,45 @@ static int omap_target(struct cpufreq_policy *policy, if (freqs.old == freqs.new policy-cur == freqs.new) return ret; + opp = opp_find_freq_exact(mpu_dev, freqs.new * 1000, true); + if (IS_ERR(opp)) { + dev_err(mpu_dev, %s: cpu%d: no opp match for freq %d\n, + __func__, policy-cpu, target_freq); + return -EINVAL; + } + + volt_new = opp_get_voltage(opp); + if (!volt_new) { + dev_err(mpu_dev, %s: cpu%d: no opp voltage for freq %d\n, + __func__, policy-cpu, target_freq); + return -EINVAL; + } + + volt_old = regulator_get_voltage(mpu_reg); + +#ifdef CONFIG_CPU_FREQ_DEBUG + pr_info(cpufreq-omap: frequency transition: %u -- %u\n, + freqs.old, freqs.new); + pr_info(cpufreq-omap: voltage transition: %d -- %d\n, + volt_old, volt_new); +#endif + + if (freqs.new freqs.old) { + ret = regulator_set_voltage(mpu_reg, volt_new, + volt_new + DEFAULT_RESOLUTION - 1); + if (ret) { + dev_err(mpu_dev, %s: unable to set voltage to %d uV (for %u MHz)\n, + __func__, volt_new, freqs.new/1000); + return ret; + } + } + /* notifiers */ for_each_cpu(i, policy-cpus) { freqs.cpu = i; cpufreq_notify_transition(freqs, CPUFREQ_PRECHANGE); } -#ifdef CONFIG_CPU_FREQ_DEBUG - pr_info(cpufreq-omap: transition: %u -- %u\n, freqs.old, freqs.new); -#endif - ret = clk_set_rate(mpu_clk, freqs.new * 1000); freqs.new = omap_getspeed(policy-cpu); @@ -150,6 +185,38 @@ static int omap_target(struct cpufreq_policy *policy, cpufreq_notify_transition(freqs, CPUFREQ_POSTCHANGE); } + if (freqs.new freqs.old) { + ret = regulator_set_voltage(mpu_reg, volt_new, + volt_new + DEFAULT_RESOLUTION - 1); + if (ret) { + unsigned int temp; + + dev_err(mpu_dev, %s: unable to set voltage to %d uV (for %u MHz)\n, + __func__, volt_new, freqs.new/1000); + + if (clk_set_rate(mpu_clk, freqs.old * 1000)) { + dev_err(mpu_dev, + %s: failed restoring clock rate to %u MHz, clock rate is %u MHz, + __func__, + freqs.old/1000, freqs.new/1000); + return ret; + } + + temp = freqs.new; + freqs.new = freqs.old; +
RE: GPMC in HWMOD (and a related AM335X issue)
Hi Benoit, HWMOD entries currently does contain GPMC, is it due to the reason that GPMC is not yet adapted to omap_device / omap_hwmod or is there any other reason for not having it in HWMOD (as GPMC not yet a driver?) Yes, that's the reason. Nobody had the time to update that driver yet to omap_device and thus we did not create the hwmod entry for it. And since we do not use the GPMC on OMAP4 boards, we did not have some good way to test any change on this driver. Do not hesitate to update that driver and add the DT support if you want:-) Thank you the reply. Within the constraints I have, if possible, I will try to add 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: Reg pinmux driver for OMAP based SoC- AM335X
Hi Tony, On Tue, Jan 31, 2012 at 22:35:11, Tony Lindgren wrote: The plan is to deprecate the existing arch/arm/*omap*/*mux* code, and cut it down to minimum. And then add DT only mux support that should work for at least omap2+. That should work for am335x too. There's currently some discussion going on regarding the pinmux device tree binding and how dynamic mux states should be handled. But I'd assume we'll have basic support available very soon. The driver I'm working leaves all the data out of kernel, and the driver just knows how to handle a pinmux register instances. Then debugging tools can be done in userspace via debugfs to display more detailed SoC specific information. So it might be worth waiting just a little bit longer. If you have resources to spend on the pinmuxing, then creating some userspace tool to display SoC specific information would be nice :) For the userspace tool, we can use the data in arch/arm/mach-omap2/mux*2*.[ch]. Thank you for the detailed reply. I will wait for it and willing to help you in its testing. I have been tasked on getting AM335X supported well in mainline, and will move to other aspects of it while waiting. 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
Reg pinmux driver for OMAP based SoC- AM335X
Hi Tony, I am working on implementing pincontrol driver for AM335X SoC (OMAP34XX family). Is there any specific plan you have in mind w.r.t incorporating pincontrol driver for OMAP family of SoC's. There was an initial patch for OMAP4 pin control driver, but it seems you were in favour of a DT based approach. As per my understanding after going through few threads (lot more remaining) is that pincontrol driver can be either a DT based or non-DT based (either way we are going out of platform folders), and it seems you prefer a DT based approach (have seen mentioning about pinmux-simple.c). Tegra pincontrol driver posted recently, seems to use DT to distinguish only the SoC type, with all the pincontrol information for SoC present in Kernel. I would be thankful if you can give me some pointers on how to proceed for AM335X pincontrol driver and/or any work in progress for OMAP pincontrol driver. Regards Afzal -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH-V4 0/3] Introducing TI's New SoC/board AM335XEVM
Hi Kevin, On Thu, Dec 01, 2011 at 20:40:03, Hilman, Kevin wrote: Hiremath, Vaibhav hvaib...@ti.com writes: : We can detect the board using on-board EEPROM, so same mach-id should work for both EVM and Beagle. And also going forward with device tree approach we may not need different id's, right? Right, which is why I'm wondering why are there sevral new AM33x mach-types when only one of them is being used: 3684TI AM335X IA EVMam335xiaevm Afzal Mohammed 3589TI AM335X EVM am335xevm Vaibhav Bedia 3808Beaglebone Boardbeaglebone Steven Kipisz Russell has been trying to cleanup athe mach-types, so if these others are not going to be used, I suggest they be deleted. 3684-TI AM335X IA EVM is required as IA EVM uses a different UART, because of which early prints can't be captured, problem mentioned in http://marc.info/?l=linux-omapm=131286938723617w=2 Note: Paul advised to post to linux-arm-ker...@lists.infradead.org, but it never made to LAKML, even though I sent couple of times and got a reply Is being held until the list moderator can review it for approval. The reason it is being held: Message has a suspicious header 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 RESEND v2 2/2] regulator: TPS65910: VDD1/2 voltage selector count
Hi Liam, On Tue, Nov 08, 2011 at 21:26:39, Mark Brown wrote: On Tue, Nov 08, 2011 at 06:54:10PM +0530, Afzal Mohammed wrote: Count of selector voltage is required for regulator_set_voltage to work via set_voltage_sel. VDD1/2 currently have it as zero, so regulator_set_voltage won't work for VDD1/2. Update count (n_voltages) for VDD1/2. Acked-by: Mark Brown broo...@opensource.wolfsonmicro.com Can you please help this patch to get into mainline Kernel. Without this VDD1 2 voltages cannot be varied on TPS65910. This patch applies cleanly to current mainline and is not dependent on any other patch. 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 v3 1/6] mfd: TPS65910: Handle non-existent devices
Hi Kyle, On Thu, Nov 03, 2011 at 22:38:01, Kyle Manna wrote: + ret = mfd_add_devices(tps65910-dev, -1, tps65910s, + ARRAY_SIZE(tps65910s), NULL, 0); + if (ret 0) + goto err2; Isn't goto err sufficient 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/2] regulator:TPS65910: VDD1/2 voltage selector count
Hi Brown, On Fri, Nov 04, 2011 at 19:34:22, Mark Brown wrote: On Fri, Nov 04, 2011 at 06:18:48PM +0530, Afzal Mohammed wrote: if (i == TPS65910_REG_VDD1 || i == TPS65910_REG_VDD2) { pmic-desc[i].ops = tps65910_ops_dcdc; + pmic-desc[i].n_voltages = VDD1_2_NUM_VOLTS * 3; This looks suspicous - what's the * 3 about? Gain in voltage possible is x1, 0x2 0x3, I am shaky about this change, but voltage could be changed with this, cc'ing original author's Specification @ http://www.ti.com/product/tps65910 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 1/2] mfd: TPS65910: Improvements
Hi Mark, On Fri, Nov 04, 2011 at 20:54:11, Mark Brown wrote: ...should be in two separate commits. Ok 2. struct tps65910_platform_data usage can be avoided by making use of struct tps65910_board to simplify driver. Hence remove it Why is this a simplification? Note that one of the reasons platform data is kept separate from any runtime data the device needs is that it makes the configuration available easier to see and understand. Ok, I understand that this simplification should not be done. 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/2] regulator:TPS65910: VDD1/2 voltage selector count
Hi Mark, On Fri, Nov 04, 2011 at 20:55:59, Mark Brown wrote: On Fri, Nov 04, 2011 at 02:26:05PM +, Mohammed, Afzal wrote: Hi Brown, Ahem. I am sorry for that if it offended you, not deliberate, this may have to do with our different cultures, for me it is always a confusion whether I should call a person with their first or second name (unless he writes it at the end of mail). Normally my name is written Mohammed Afzal, but I am called Afzal. On Fri, Nov 04, 2011 at 19:34:22, Mark Brown wrote: That doesn't really clarify things - the question is why the number of voltages we can set is three times a constant called _NUM_VOLTS? _NUM_VOLTS is the number of voltage steps. I will try to come up with a better explanation or rather the right solution, new to regulator world as of now. 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/2] regulator:TPS65910: VDD1/2 voltage selector count
Hi Mark, On Fri, Nov 04, 2011 at 21:48:56, Mark Brown wrote: So that definitely seems wrong then - n_voltages is supposed to be the number of voltages that can be selected so if the regulator supports _NUM_VOLTS steps then I'd expect to see that constant used directly. Otherwise I'd suggest that the magic number needs a #define. A gain of 0x1, 0x2, 0x3 is possible for each of the voltage steps, so we have a total of _NUM_VOLTS * 3 steps (although some of them would be same values). Let me know your opinion on using _NUM_VOLTS * NUM_GAIN instead, with #define NUM_GAIN 3. 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/2] regulator:TPS65910: VDD1/2 voltage selector count
Hi Mark, On Fri, Nov 04, 2011 at 22:10:09, Mark Brown wrote: What do you mean when you say gain? Effective voltage expression is (value1 * 12.5mV + 562.5 mV) * value2. In this value2 is being called as gain. value1 can have values from 3 to 75, both inclusive (73 steps) value2 can have from 1 to 3, both inclusive (3 numbers) 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 v3 1/6] mfd: TPS65910: Handle non-existent devices
Hi Kyla, On Thu, Nov 03, 2011 at 22:38:01, Kyle Manna wrote: The JTAGREVNUM register contains a silicon revision number in the lower four bits and the upper four bits are to always read 0. To detect the presence of the device, attempt to read JTAGREVNUM register and check that it returns a valid value. If the I2C device fails to respond or returns an invalid value, return -ENODEV. : : + /* Check that the device is there */ + ret = tps65910_i2c_read(tps65910, TPS65910_JTAGVERNUM, 1, reg); + if (ret 0 || (reg ~JTAGVERNUM_VERNUM_MASK)) { + dev_err(tps65910-dev, unknown version: JTAGREVNUM = 0x%x\n, +reg); + ret = -ENODEV; goto err; + } If i2c read fails, 0 would get printed as version. Perhaps it would be better to print version irrespective of whether it is unknown or not, and return error for unknown value, something like, unsigned char buff; /* Check that the device is actually there */ ret = tps65910_i2c_read(tps65910, 0x0, 1, buff); if (ret 0) { dev_err(tps65910-dev, could not be detected\n); ret = -ENODEV; goto err; } dev_info(tps65910-dev, JTAGREVNUM 0x%x\n, buff); if (buff ~JTAGVERNUM_VERNUM_MASK) { dev_err(tps65910-dev, unknown version\n); ret = -ENODEV; goto err; } 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] ARM: OMAP: Add support for dmtimer v2 ip (Re: [PATCH v15 06/12] OMAP: dmtimer: switch-over to platform device driver)
Hi Tony, On Sat, Sep 17, 2011 at 07:05:31, Tony Lindgren wrote: Afzal, care to check if that works for AM335X/TI816X/TI814X? With following patch over yours, AM335X (the only board with me) boots up fine. Regards Afzal From ff64a239e60f9b517860eb2fe9c4f88a188ca51d Mon Sep 17 00:00:00 2001 From: Afzal Mohammed af...@ti.com Date: Mon, 19 Sep 2011 10:06:59 +0530 Subject: [PATCH] ARM: OMAP: dmtimer register safe access Access dmtimer registers after setting it's parent clock. If default parent is not physically present, accessing register causes abort, so access registers after proper parent is set. Signed-off-by: Afzal Mohammed af...@ti.com --- arch/arm/mach-omap2/timer.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index 1746c69..ababc4d 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -172,7 +172,6 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer, } omap_hwmod_enable(oh); - __omap_dm_timer_init_regs(timer); sys_timer_reserved |= (1 (gptimer_id - 1)); @@ -190,6 +189,7 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer, clk_put(src); } } + __omap_dm_timer_init_regs(timer); __omap_dm_timer_reset(timer, 1, 1); timer-posted = 1; -- 1.6.2.4
RE: [PATCH v15 06/12] OMAP: dmtimer: switch-over to platform device driver
Hi Tony, On Thu, Sep 15, 2011 at 11:12:53, DebBarma, Tarun Kanti wrote: On Thu, Sep 15, 2011 at 3:15 AM, Tony Lindgren t...@atomide.com wrote: * Tarun Kanti DebBarma tarun.ka...@ti.com [110908 13:36]: removed from timer code. New set of timers present on OMAP4 are now supported. Also, as we don't need the support for different register offsets for the first two omap4 timers, please rather implement support for the new timers and the timeouts directly in plat-omap/dmtimer.c. That way we can still keep the minimal timer support simple for clocksource and clockevent. Of course this means that we'll be only supporting the first two timers as system timers on omap4, but that's fine. Ok, I can make the change! But, do we have to keep OMAP5 in mind right now where even timers[1,2] require addition of offsets? We need clocksource clockevent to be able to work with timers requiring addition of offsets. Without this AM335X, TI816X and TI814X SoC's will not boot. 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
Early prints on different UART's for same machine type
Hi, This is regarding board with multiple variations using an upcoming SoC from TI. Variation of the board is detected by reading EEPROM using I2C after init_machine gets invoked. In one of the variation UART# used is different. Because of this decompressor logs (and early prints) can't be captured for the board variant using this different UART#. Our solution for this problem is to use different mach-id for the board using different UART#. But this solution seems to be overkill as only to capture decompressor logs, we have to use a new machine id. As I2C will not work at the decompressor stage, I2C can't be used to detect the board type. As far as I know, ATAG is not parsed in decompressor stage and so ATAG cannot be used to determine board variant or UART#. Otherwise we will have to add capability in decompressor to parse ATAG, detect console to be used, and modify uncompress.h to select proper console for capturing decompressor logs. This seems a bad solution. Our bootloader has the capability to detect the board variant. So the best solution for us so far seems to add a new machine id, if any better solutions are possible, please let us know. 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