RE: Please help! AM35xx mm/slab.c BUG

2012-06-12 Thread Mohammed, Afzal
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

2012-06-12 Thread Mohammed, Afzal
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

2012-06-12 Thread Mohammed, Afzal
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

2012-06-12 Thread Mohammed, Afzal
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

2012-06-12 Thread Mohammed, Afzal
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

2012-06-12 Thread Mohammed, Afzal
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

2012-06-12 Thread Mohammed, Afzal
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

2012-06-12 Thread Mohammed, Afzal
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

2012-06-12 Thread Mohammed, Afzal
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

2012-06-12 Thread Mohammed, Afzal
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

2012-06-11 Thread Mohammed, Afzal
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

2012-06-07 Thread Mohammed, Afzal
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)

2012-06-07 Thread Mohammed, Afzal
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

2012-05-25 Thread Mohammed, Afzal
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

2012-05-22 Thread Mohammed, Afzal
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

2012-05-22 Thread Mohammed, Afzal
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

2012-05-21 Thread Mohammed, Afzal
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

2012-05-18 Thread Mohammed, Afzal
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

2012-05-18 Thread Mohammed, Afzal
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

2012-05-18 Thread Mohammed, Afzal
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

2012-05-15 Thread Mohammed, Afzal
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

2012-05-15 Thread Mohammed, Afzal
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

2012-05-14 Thread Mohammed, Afzal
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

2012-05-13 Thread Mohammed, Afzal
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

2012-05-11 Thread Mohammed, Afzal
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

2012-05-11 Thread Mohammed, Afzal
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

2012-05-10 Thread Mohammed, Afzal
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

2012-05-10 Thread Mohammed, Afzal
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

2012-05-10 Thread Mohammed, Afzal
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

2012-05-09 Thread Mohammed, Afzal
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

2012-05-09 Thread Mohammed, Afzal
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

2012-05-08 Thread Mohammed, Afzal
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

2012-05-08 Thread Mohammed, Afzal
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

2012-05-08 Thread Mohammed, Afzal
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

2012-05-08 Thread Mohammed, Afzal
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

2012-05-08 Thread Mohammed, Afzal
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

2012-05-07 Thread Mohammed, Afzal
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

2012-05-07 Thread Mohammed, Afzal
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

2012-05-07 Thread Mohammed, Afzal
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

2012-05-07 Thread Mohammed, Afzal
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

2012-05-03 Thread Mohammed, Afzal
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

2012-05-03 Thread Mohammed, Afzal
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

2012-05-03 Thread Mohammed, Afzal
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

2012-05-03 Thread Mohammed, Afzal
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

2012-05-01 Thread Mohammed, Afzal
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

2012-05-01 Thread Mohammed, Afzal
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

2012-04-28 Thread Mohammed, Afzal
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

2012-04-25 Thread Mohammed, Afzal
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

2012-04-25 Thread Mohammed, Afzal
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

2012-04-24 Thread Mohammed, Afzal
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

2012-04-23 Thread Mohammed, Afzal
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

2012-04-23 Thread Mohammed, Afzal
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

2012-04-23 Thread Mohammed, Afzal
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

2012-04-23 Thread Mohammed, Afzal
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

2012-04-12 Thread Mohammed, Afzal
+ 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

2012-04-12 Thread Mohammed, Afzal
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

2012-04-12 Thread Mohammed, Afzal
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

2012-04-12 Thread Mohammed, Afzal
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

2012-04-10 Thread Mohammed, Afzal
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

2012-04-10 Thread Mohammed, Afzal
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

2012-04-10 Thread Mohammed, Afzal
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

2012-04-06 Thread Mohammed, Afzal
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

2012-04-06 Thread Mohammed, Afzal
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

2012-04-05 Thread Mohammed, Afzal
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

2012-04-05 Thread Mohammed, Afzal
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

2012-04-05 Thread Mohammed, Afzal
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

2012-04-05 Thread Mohammed, Afzal
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

2012-04-05 Thread Mohammed, Afzal
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

2012-04-04 Thread Mohammed, Afzal
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

2012-04-03 Thread Mohammed, Afzal
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

2012-03-27 Thread Mohammed, Afzal
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

2012-03-26 Thread Mohammed, Afzal
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

2012-03-26 Thread Mohammed, Afzal
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

2012-03-26 Thread Mohammed, Afzal
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

2012-03-26 Thread Mohammed, Afzal
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

2012-03-26 Thread Mohammed, Afzal
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

2012-03-26 Thread Mohammed, Afzal
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

2012-03-26 Thread Mohammed, Afzal
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

2012-03-23 Thread Mohammed, Afzal
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

2012-03-02 Thread Mohammed, Afzal
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

2012-02-22 Thread Mohammed, Afzal
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

2012-02-22 Thread Mohammed, Afzal
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

2012-02-21 Thread Mohammed, Afzal
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

2012-02-16 Thread Mohammed, Afzal
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)

2012-02-10 Thread Mohammed, Afzal
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

2012-02-01 Thread Mohammed, Afzal
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

2012-01-31 Thread Mohammed, Afzal
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

2011-12-01 Thread Mohammed, Afzal
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

2011-11-22 Thread Mohammed, Afzal
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

2011-11-04 Thread Mohammed, Afzal
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

2011-11-04 Thread Mohammed, Afzal
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

2011-11-04 Thread Mohammed, Afzal
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

2011-11-04 Thread Mohammed, Afzal
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

2011-11-04 Thread Mohammed, Afzal
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

2011-11-04 Thread Mohammed, Afzal
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

2011-11-03 Thread Mohammed, Afzal
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)

2011-09-18 Thread Mohammed, Afzal
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

2011-09-15 Thread Mohammed, Afzal
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

2011-08-08 Thread Mohammed, Afzal
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


<    1   2   3