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 v4-alt 3/6] ARM: OMAP3: hwmod data: add gpmc

2012-06-05 Thread Tony Lindgren 
* Mohammed, Afzal af...@ti.com [120525 03:20]:
 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)

Yes sure as long as we can see what needs to be set in the kernel.

The GPMC timings should only come from the bootloader in the device tree
case BTW, so that's also something to consider here.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4-alt 3/6] ARM: OMAP3: hwmod data: add gpmc

2012-05-25 Thread Tony Lindgren 
* 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.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 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 v4-alt 3/6] ARM: OMAP3: hwmod data: add gpmc

2012-05-23 Thread Paul Walmsley 
On Tue, 22 May 2012, Tony Lindgren wrote:

 * Paul Walmsley p...@pwsan.com [120521 23:51]:
  
  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.
 
 Also something to note here is that generating dynamic timings from the
 fixed GPMC register values won't work for other frequencies.
 
 As far as I remember, the main problem trying to convert fixed value
 GPMC timings into dynamic timings is the fact that some GPMC values
 calculated depend on clock cycles, while other values depend on time.
 
 So the cycle values remain unknown trying to upsample from fixed timings.

Yep indeed.  That's why clock rate changes have to be blocked on the GPMC 
clock.  If we don't have GPMC clock rate-independent timing data, then 
I think we'll have to keep the GPMC clock rate fixed.

  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.
 
 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 ;-)


- Paul
--
To unsubscribe from this list: send the line unsubscribe 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-22 Thread Paul Walmsley 
Hi Afzal

On Thu, 10 May 2012, Mohammed, Afzal wrote:

 On Tue, May 08, 2012 at 21:02:33, Paul Walmsley wrote:

(attribution lost)

 
   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.

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.

   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

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 is the

Re: [PATCH v4-alt 3/6] ARM: OMAP3: hwmod data: add gpmc

2012-05-22 Thread Tony Lindgren 
* Paul Walmsley p...@pwsan.com [120521 23:51]:
 
 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.

Also something to note here is that generating dynamic timings from the
fixed GPMC register values won't work for other frequencies.

As far as I remember, the main problem trying to convert fixed value
GPMC timings into dynamic timings is the fact that some GPMC values
calculated depend on clock cycles, while other values depend on time.

So the cycle values remain unknown trying to upsample from fixed timings.
 
 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.

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.
 
 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.

I'm fine with that assuming we somehow first have the values for the
most commonly used boards for smsc911x. But before that happens, we can't
really deprecate bootloader set GPMC values.

Regards,

Tony 
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 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 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-alt 3/6] ARM: OMAP3: hwmod data: add gpmc

2012-05-08 Thread Paul Walmsley 
On Mon, 7 May 2012, Mohammed, Afzal wrote:

 On Sun, May 06, 2012 at 07:34:07, Paul Walmsley wrote:

(attribution lost)

   +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

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.

 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?

 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.

Sounds like you've got a good test platform, then :-)

 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?


- Paul
--
To unsubscribe from this list: send the line unsubscribe 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 Paul Walmsley 
Hi

On Tue, 8 May 2012, Mohammed, Afzal wrote:

 If HWMOD_NO_INIT_RESET is not present, it would break GPMC on
 many of the existing boards.

IMHO, that should also be fixed as part of your changes, to remove what 
seems to be an unnecessary bootloader dependency.

http://marc.info/?l=linux-omapm=133649129220098w=2


- Paul
--
To unsubscribe from this list: send the line unsubscribe 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-alt 3/6] ARM: OMAP3: hwmod data: add gpmc

2012-05-05 Thread Paul Walmsley 
Hello Afzal

a few comments

On Wed, 2 May 2012, Afzal Mohammed wrote:

 Add gpmc hwmod and associated interconnect data
 
 Signed-off-by: Afzal Mohammed af...@ti.com

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.

...

Also, the OMAP2xxx GPMC hwmod data is missing.  It can be combined 
with this patch.

...

Also:

 +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.


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html