RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-22 Thread Mohammed, Afzal
Hi Tony, Jon,

On Thu, Jun 21, 2012 at 05:05:56, Hunter, Jon wrote:
 On 06/20/2012 10:12 AM, Tony Lindgren wrote:
  * Mohammed, Afzal af...@ti.com [120620 07:57]:

 Therefore, I am wondering if Afzal's driver needs to register the gpmc
 devices outside of the gpmc_probe() and add the devices as children. Or
 maybe we tackle that when we migrate it to DT.

V6 of this series has been posted assuming that we tackle above issue
during DT migration

  For gpmc driver to calculate timings rather than platform code doing it,
  we first need to have a generalized way to calculate gpmc timings for
  all peripherals as well as have a logic to calculate timings based on
  time  cycles, correct ? (to make sure we are talking the same thing)
  
  Yes that might be tricky..
 
 I am wondering more and more if we need to do a clean-up of the timing
 calculations prior to driver migration ...

I propose to have clean-up of timing calculations after driver migration,
v6 of this series has been posted, except the controversial issue of clock,
I believe other issues has been addressed.

Please let me know your comments.

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-20 Thread Tony Lindgren
* Mohammed, Afzal af...@ti.com [120616 02:19]:
 Hi Tony,
 
 On Fri, Jun 15, 2012 at 18:15:20, Tony Lindgren wrote:
  * Mohammed, Afzal af...@ti.com [120615 03:26]:
 
   Here clock is required even before driver is probed, i.e for platform to
   calculate timings, that has to be passed through platform data.
  
  Eventually we should be able to move the gpmc registration to the driver
  probe, especially with device tree. There's no need to set up gpmc
  before the driver probe runs for the device using gpmc. Just how the
  gpmc init gets called from the driver probe is still a bit open though..
 
 Sorry, I did not get you, can you please clarify

OK, I'll try :)
 
 By gpmc registration, if you meant registering platform device for
 gpmc peripherals, for a board that uses the new gpmc driver interface*,
 it will be done in probe only.

I was thinking when the gpmc needs to be initialized, and there should
not be any need to do it earlier than at the gpmc using driver probe
time. With device tree that is, as there's no need to stuff the gpmc
timings into a board-*.c file.

 All the responsibilities of old gpmc init is now taken care by probe;
 probe in addition does setting up gpmc for each peripheral, registering
 platform device (if the board makes use of new driver interface). If
 a board uses new gpmc driver interface, gpmc for that device is not
 setup before gpmc probe.

OK
 
  It may require some bus level hooks, or wrapper drivers for the generic
  device drivers like smsc911x.
 
 This too, not sure whether I follow you

Well smsc911x has device tree binding, and is a generic driver. How do
we trigger the gpmc initialization from a generic driver probe?

   I understand the necessity for clk rate information in driver, but seems
   unless we have a generic way to scale timings for all the kinds of
   peripheral, having it may not be of much help.
  
  We should not need to pass clock handles around. It's better to
  export some helper functions in the gpmc code for the calculation.
 
 Currently we have helper function in gpmc.c for the same, were you
 referring those ?

Yes something that let's the driver call gpmc code to do the calculation.
The other option would be to just add gpmc clock as a clock fwk node,
and then the driver could clk_get() it as ick.

Regards,

Tony
 
 *: [1] converts omap3evm  beagle board to use new interface, in addition
  to [1], similar to it, one more series would be posted to convert remaining
  boards also to use the new gpmc driver interface. As these cannot be tested
  by me and as it depends on final shape of this basic driver conversion
  series (or the present series), they have not yet been converted
 
 [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg69917.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 v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-20 Thread Mohammed, Afzal
Hi Tony,

On Wed, Jun 20, 2012 at 18:58:49, Tony Lindgren wrote:
 * Mohammed, Afzal af...@ti.com [120616 02:19]:
  On Fri, Jun 15, 2012 at 18:15:20, Tony Lindgren wrote:

  By gpmc registration, if you meant registering platform device for
  gpmc peripherals, for a board that uses the new gpmc driver interface*,
  it will be done in probe only.
 
 I was thinking when the gpmc needs to be initialized, and there should
 not be any need to do it earlier than at the gpmc using driver probe
 time. With device tree that is, as there's no need to stuff the gpmc
 timings into a board-*.c file.

I believe by gpmc needs to be initialized, you meant calculating gpmc
timings, determining configuration, the things that are done in functions
like gpmc_smsc911x_update etc. as in [1] and not initializing gpmc at
hardware level. With the above assumption, I feel we need to have a way
first to generalize gpmc timing calculation for different peripherals as
suggested by Jon as well as have logic to handle timings that depends on
cycles too.

   It may require some bus level hooks, or wrapper drivers for the generic
   device drivers like smsc911x.
  
  This too, not sure whether I follow you
 
 Well smsc911x has device tree binding, and is a generic driver. How do
 we trigger the gpmc initialization from a generic driver probe?

Not sure whether device tree have capability to represent something like
child devices, if non bus devices can have child devices, then we
can have peripherals connected to gpmc as childs, but may be this will
remain only as a dream; I need to get into DT to find things out

   We should not need to pass clock handles around. It's better to
   export some helper functions in the gpmc code for the calculation.
  
  Currently we have helper function in gpmc.c for the same, were you
  referring those ?
 
 Yes something that let's the driver call gpmc code to do the calculation.
 The other option would be to just add gpmc clock as a clock fwk node,
 and then the driver could clk_get() it as ick.

For gpmc driver to calculate timings rather than platform code doing it,
we first need to have a generalized way to calculate gpmc timings for
all peripherals as well as have a logic to calculate timings based on
time  cycles, correct ? (to make sure we are talking the same thing)

Regards
Afzal

[1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg69926.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 v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-20 Thread Tony Lindgren
* Mohammed, Afzal af...@ti.com [120620 07:57]:
 Hi Tony,
 
 On Wed, Jun 20, 2012 at 18:58:49, Tony Lindgren wrote:
  * Mohammed, Afzal af...@ti.com [120616 02:19]:
   On Fri, Jun 15, 2012 at 18:15:20, Tony Lindgren wrote:
 
   By gpmc registration, if you meant registering platform device for
   gpmc peripherals, for a board that uses the new gpmc driver interface*,
   it will be done in probe only.
  
  I was thinking when the gpmc needs to be initialized, and there should
  not be any need to do it earlier than at the gpmc using driver probe
  time. With device tree that is, as there's no need to stuff the gpmc
  timings into a board-*.c file.
 
 I believe by gpmc needs to be initialized, you meant calculating gpmc
 timings, determining configuration, the things that are done in functions
 like gpmc_smsc911x_update etc. as in [1] and not initializing gpmc at
 hardware level. With the above assumption, I feel we need to have a way
 first to generalize gpmc timing calculation for different peripherals as
 suggested by Jon as well as have logic to handle timings that depends on
 cycles too.

Yup. We'll be only getting the timings from device tree. Ideally the gpmc
code would just do all the calculations, but it sounds like device timings
may require some calculation on the gpmc consumer driver side too.

It may require some bus level hooks, or wrapper drivers for the generic
device drivers like smsc911x.
   
   This too, not sure whether I follow you
  
  Well smsc911x has device tree binding, and is a generic driver. How do
  we trigger the gpmc initialization from a generic driver probe?
 
 Not sure whether device tree have capability to represent something like
 child devices, if non bus devices can have child devices, then we
 can have peripherals connected to gpmc as childs, but may be this will
 remain only as a dream; I need to get into DT to find things out

Yes the that's there set up automatically. But for the timings we
could have something like this to associate the following invented
gpmc timings with smsc911x:

/* in omap[234].dtsi */
gpmc: gpmc@480121234 {
compatible = ti,gpmc;
reg = 0x480121234 0x1234;
#address-cells = 0;
#size-cells = 0;
};
...

/* in some board *.dts file */
gpmc {
gpmc_cs1: gpmc_cs1_timings {
...
};
};

net@48001234 {
compatible = smsc,lan91c111;
reg = 0x48001234 0x1;
interrupts = 12;
gpmc_timings = gpmc_cs1;
};

 
We should not need to pass clock handles around. It's better to
export some helper functions in the gpmc code for the calculation.
   
   Currently we have helper function in gpmc.c for the same, were you
   referring those ?
  
  Yes something that let's the driver call gpmc code to do the calculation.
  The other option would be to just add gpmc clock as a clock fwk node,
  and then the driver could clk_get() it as ick.
 
 For gpmc driver to calculate timings rather than platform code doing it,
 we first need to have a generalized way to calculate gpmc timings for
 all peripherals as well as have a logic to calculate timings based on
 time  cycles, correct ? (to make sure we are talking the same thing)

Yes that might be tricky..

Tony

 
 [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg69926.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 v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-20 Thread Jon Hunter

On 06/20/2012 10:12 AM, Tony Lindgren wrote:
 * Mohammed, Afzal af...@ti.com [120620 07:57]:
 Hi Tony,

 On Wed, Jun 20, 2012 at 18:58:49, Tony Lindgren wrote:
 * Mohammed, Afzal af...@ti.com [120616 02:19]:
 On Fri, Jun 15, 2012 at 18:15:20, Tony Lindgren wrote:

 By gpmc registration, if you meant registering platform device for
 gpmc peripherals, for a board that uses the new gpmc driver interface*,
 it will be done in probe only.

 I was thinking when the gpmc needs to be initialized, and there should
 not be any need to do it earlier than at the gpmc using driver probe
 time. With device tree that is, as there's no need to stuff the gpmc
 timings into a board-*.c file.

 I believe by gpmc needs to be initialized, you meant calculating gpmc
 timings, determining configuration, the things that are done in functions
 like gpmc_smsc911x_update etc. as in [1] and not initializing gpmc at
 hardware level. With the above assumption, I feel we need to have a way
 first to generalize gpmc timing calculation for different peripherals as
 suggested by Jon as well as have logic to handle timings that depends on
 cycles too.
 
 Yup. We'll be only getting the timings from device tree. Ideally the gpmc
 code would just do all the calculations, but it sounds like device timings
 may require some calculation on the gpmc consumer driver side too.
 
 It may require some bus level hooks, or wrapper drivers for the generic
 device drivers like smsc911x.

 This too, not sure whether I follow you

 Well smsc911x has device tree binding, and is a generic driver. How do
 we trigger the gpmc initialization from a generic driver probe?

 Not sure whether device tree have capability to represent something like
 child devices, if non bus devices can have child devices, then we
 can have peripherals connected to gpmc as childs, but may be this will
 remain only as a dream; I need to get into DT to find things out
 
 Yes the that's there set up automatically. But for the timings we
 could have something like this to associate the following invented
 gpmc timings with smsc911x:
 
 /* in omap[234].dtsi */
 gpmc: gpmc@480121234 {
   compatible = ti,gpmc;
   reg = 0x480121234 0x1234;
   #address-cells = 0;
   #size-cells = 0;
 };
 ...
 
 /* in some board *.dts file */
 gpmc {
   gpmc_cs1: gpmc_cs1_timings {
   ...
   };
 };
 
 net@48001234 {
   compatible = smsc,lan91c111;
   reg = 0x48001234 0x1;
   interrupts = 12;
   gpmc_timings = gpmc_cs1;
 };

The above make sense to me, but I am struggling to see how we can make
the above work with the proposed driver Afzal has created. The problem
is how devices are getting registered with Afzal's driver in comparison
to how device tree will register them.

In Afzal's driver the gpmc_probe() is going to request and configure the
chip-selects and if successful, then call platform_device_register() to
register the child devices. So all child devices get registered during
the gpmc_probe() itself.

With device tree, my understanding is that, it will first register the
gpmc device and then the children afterwards. In other words, the
gpmc_probe will be called first and the children registered/probed after.

Therefore, I am wondering if Afzal's driver needs to register the gpmc
devices outside of the gpmc_probe() and add the devices as children. Or
maybe we tackle that when we migrate it to DT.

 We should not need to pass clock handles around. It's better to
 export some helper functions in the gpmc code for the calculation.

 Currently we have helper function in gpmc.c for the same, were you
 referring those ?

 Yes something that let's the driver call gpmc code to do the calculation.
 The other option would be to just add gpmc clock as a clock fwk node,
 and then the driver could clk_get() it as ick.

 For gpmc driver to calculate timings rather than platform code doing it,
 we first need to have a generalized way to calculate gpmc timings for
 all peripherals as well as have a logic to calculate timings based on
 time  cycles, correct ? (to make sure we are talking the same thing)
 
 Yes that might be tricky..

I am wondering more and more if we need to do a clean-up of the timing
calculations prior to driver migration ...

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


RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-16 Thread Mohammed, Afzal
Hi Tony,

On Fri, Jun 15, 2012 at 18:15:20, Tony Lindgren wrote:
 * Mohammed, Afzal af...@ti.com [120615 03:26]:

  Here clock is required even before driver is probed, i.e for platform to
  calculate timings, that has to be passed through platform data.
 
 Eventually we should be able to move the gpmc registration to the driver
 probe, especially with device tree. There's no need to set up gpmc
 before the driver probe runs for the device using gpmc. Just how the
 gpmc init gets called from the driver probe is still a bit open though..

Sorry, I did not get you, can you please clarify

By gpmc registration, if you meant registering platform device for
gpmc peripherals, for a board that uses the new gpmc driver interface*,
it will be done in probe only.

All the responsibilities of old gpmc init is now taken care by probe;
probe in addition does setting up gpmc for each peripheral, registering
platform device (if the board makes use of new driver interface). If
a board uses new gpmc driver interface, gpmc for that device is not
setup before gpmc probe.

 It may require some bus level hooks, or wrapper drivers for the generic
 device drivers like smsc911x.

This too, not sure whether I follow you

  
  I understand the necessity for clk rate information in driver, but seems
  unless we have a generic way to scale timings for all the kinds of
  peripheral, having it may not be of much help.
 
 We should not need to pass clock handles around. It's better to
 export some helper functions in the gpmc code for the calculation.

Currently we have helper function in gpmc.c for the same, were you
referring those ?

Regards
Afzal

*: [1] converts omap3evm  beagle board to use new interface, in addition
 to [1], similar to it, one more series would be posted to convert remaining
 boards also to use the new gpmc driver interface. As these cannot be tested
 by me and as it depends on final shape of this basic driver conversion
 series (or the present series), they have not yet been converted

[1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg69917.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 v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-15 Thread Mohammed, Afzal
Hi Jon,

On Fri, Jun 15, 2012 at 00:28:44, Hunter, Jon wrote:
 On 06/14/2012 08:32 AM, Mohammed, Afzal wrote:
  On Thu, Jun 14, 2012 at 18:52:55, Hunter, Jon wrote:

  Why? You currently have a global variable storing the clock handle. It
  can be quite common for drivers to know the clock frequencies of their
  functional clocks. How else can drivers calculate timings?
  
  
  Please see Russell King's comments,
  
  [1] http://permalink.gmane.org/gmane.linux.ports.ppc.embedded/27634
  [2] 
  http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg05365.html
 
 Thanks. So I still think you need to get rid of the global variable for
 storing the gpmc fclk, that is really my point.
 
 So if you look at commit [1] mentioned by Russell in the above thread,
 the appropriate thing to do would be to create a gpmc clock alias for
 all OMAP2+ devices and then you could simply call the following from the
 gpmc probe ...
 
   gpmc_fck = clk_get(pdev-dev, fck);
 
 You could then store somewhere in one of the gpmc structures.

Here clock is required even before driver is probed, i.e for platform to
calculate timings, that has to be passed through platform data.

I understand the necessity for clk rate information in driver, but seems
unless we have a generic way to scale timings for all the kinds of
peripheral, having it may not be of much help.

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-15 Thread Mohammed, Afzal
Hi Jon,

On Fri, Jun 15, 2012 at 02:21:50, Hunter, Jon wrote:
 On 06/14/2012 01:17 AM, Mohammed, Afzal wrote:

  gpmc_cs_set_timings() does currently convert time to clock cycles required,
  and this gpmc driver have the capability to do it.
  
  What I was saying is a different issue, input to gpmc_cs_set_timings, which
  is time sometimes in turn is a function of time or to be exact depends on
  gpmc clock period also. So timings provided to gpmc_cs_set_timings for a
  particular frequency may not hold good for another frequency, unless we
  change the input time to gpmc_cs_set_timings based on gpmc clock.
 
 Absolutely! No argument there.
 
  If you see gpmc-* files, many a times, they need to know value of gpmc fclk,
  to calculate the input time to be fed for gpmc_cs_set_timings
 
 I understand, but that is not my point. My point is that timings should
 be provided in nanoseconds for all devices. Then the gpmc driver can
 query the gpmc fclk and convert to gpmc cycles. This way the gpmc driver
 will not care what device is connected in terms of the timings and can
 convert them.

But the way gpmc clock period has its effect on timings required for
different peripherals is different, i.e. input timing to be provided for
gpmc_cs_set_timings depends on gpmc clk  it's effect varies with the
peripheral

Even for the same peripheral, directly scaling timings for a different
frequency may not work

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-15 Thread Tony Lindgren
* Mohammed, Afzal af...@ti.com [120615 03:26]:
 Hi Jon,
 
 On Fri, Jun 15, 2012 at 00:28:44, Hunter, Jon wrote:
  On 06/14/2012 08:32 AM, Mohammed, Afzal wrote:
   On Thu, Jun 14, 2012 at 18:52:55, Hunter, Jon wrote:
 
   Why? You currently have a global variable storing the clock handle. It
   can be quite common for drivers to know the clock frequencies of their
   functional clocks. How else can drivers calculate timings?
   
   
   Please see Russell King's comments,
   
   [1] http://permalink.gmane.org/gmane.linux.ports.ppc.embedded/27634
   [2] 
   http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg05365.html
  
  Thanks. So I still think you need to get rid of the global variable for
  storing the gpmc fclk, that is really my point.
  
  So if you look at commit [1] mentioned by Russell in the above thread,
  the appropriate thing to do would be to create a gpmc clock alias for
  all OMAP2+ devices and then you could simply call the following from the
  gpmc probe ...
  
  gpmc_fck = clk_get(pdev-dev, fck);
  
  You could then store somewhere in one of the gpmc structures.
 
 Here clock is required even before driver is probed, i.e for platform to
 calculate timings, that has to be passed through platform data.

Eventually we should be able to move the gpmc registration to the driver
probe, especially with device tree. There's no need to set up gpmc
before the driver probe runs for the device using gpmc. Just how the
gpmc init gets called from the driver probe is still a bit open though..
It may require some bus level hooks, or wrapper drivers for the generic
device drivers like smsc911x.
 
 I understand the necessity for clk rate information in driver, but seems
 unless we have a generic way to scale timings for all the kinds of
 peripheral, having it may not be of much help.

We should not need to pass clock handles around. It's better to
export some helper functions in the gpmc code for the calculation.

Regards,

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


Re: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-15 Thread Jon Hunter
Hi Paul,

On 06/14/2012 07:20 PM, Paul Walmsley wrote:
 On Thu, 14 Jun 2012, Jon Hunter wrote:
 
 What does make this a bit more difficult is the function
 gpmc_round_ns_to_ticks(). It appears to convert nanoseconds to ticks and
 back to nanoseconds. I am guessing to account for some rounding error. I
 am curious what impact this function is having on the timing. Ideally
 this should be handle in gpmc_cs_set_timings().
 
 I don't have the code in front of me at the moment, nor my old GPMC timing 
 spreadsheet, so this comment may be superfluous.  But since kernel 
 arithmetic needs to be in integers, it might be necessary to do some of 
 the timing calculations in picosecond units to avoid roundoff issues.

A lot of the GPMC timing functions are converting to pico-seconds and
then rounding up to nano-seconds. So I think that the calculations are ok.

The real problem is, that as Afzal has highlighted, is that there is no
common method between GPMC peripherals (onenand, smsc, etc) for
populating the gpmc timings structure. This is going to cause a bit of a
headache for migrating them over to DT. So what we need to do and maybe
this is another series, separate from Afzal's series, is really clean-up
the populating of the timings.

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


RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-14 Thread Mohammed, Afzal
Hi Jon,

On Wed, Jun 13, 2012 at 20:21:50, Hunter, Jon wrote:

  I do not think it is practically possible. Please see timing calculations
  in arch/arm/mach-omap2/gpmc-*, the way it is done for different
  peripherals are different, and we cannot expect gpmc driver to do those as
  that would require gpmc driver being aware of type of peripheral connected.
  
  And all those gpmc-* timing calculation needs to be done before driver
  is ready, they rely on functions like gpmc_get_fclk_rate(), which in turn
  requires the clk rate to be available before driver is probed.
 
 So I see that the various gpmc-*.c files have some form of _retime()
 function. However, at the end of the day they all call
 gpmc_cs_set_timings() to convert time into gpmc clocks. Converting time
 to gpmc clocks is completely independent of the actual device and so
 this can be performed by the driver. We just need to populate the
 gpmc_timings struct and pass to the driver to convert to clocks and
 program into the registers.

gpmc_cs_set_timings() does currently convert time to clock cycles required,
and this gpmc driver have the capability to do it.

What I was saying is a different issue, input to gpmc_cs_set_timings, which
is time sometimes in turn is a function of time or to be exact depends on
gpmc clock period also. So timings provided to gpmc_cs_set_timings for a
particular frequency may not hold good for another frequency, unless we
change the input time to gpmc_cs_set_timings based on gpmc clock.

If you see gpmc-* files, many a times, they need to know value of gpmc fclk,
to calculate the input time to be fed for gpmc_cs_set_timings

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


RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-14 Thread Mohammed, Afzal
Hi Jon,

On Thu, Jun 14, 2012 at 11:47:03, Mohammed, Afzal wrote:

 gpmc_cs_set_timings() does currently convert time to clock cycles required,
 and this gpmc driver have the capability to do it.
 
 What I was saying is a different issue, input to gpmc_cs_set_timings, which
 is time sometimes in turn is a function of time or to be exact depends on
 gpmc clock period also. So timings provided to gpmc_cs_set_timings for a
 particular frequency may not hold good for another frequency, unless we
 change the input time to gpmc_cs_set_timings based on gpmc clock.
 
 If you see gpmc-* files, many a times, they need to know value of gpmc fclk,
 to calculate the input time to be fed for gpmc_cs_set_timings

And the way value of gpmc period has to be handled differs depending
on the peripheral.

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


RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-14 Thread Mohammed, Afzal
Hi Jon,

On Wed, Jun 13, 2012 at 20:21:50, Hunter, Jon wrote:
 
 If the clk handle for the gpmc is passed to the gpmc driver, then there
 is no reason why the driver cannot do this.

I believe passing clk details through platform data is an abuse of
clock framework.

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


RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-14 Thread Mohammed, Afzal
Hi Jon,

On Wed, Jun 13, 2012 at 20:38:09, Hunter, Jon wrote:
 On 06/13/2012 08:05 AM, Mohammed, Afzal wrote:

  Do you mean that gpmc driver should have the capability to calculate 
  peripheral
  timings at runtime based on frequency ?, I am not sure how this can be 
  handled
  by gpmc driver as calculation for different peripherals are done in 
  different
  way, requiring gpmc driver to know about connected peripheral, that would 
  imply
  that gpmc driver would not be peripheral agnostic.
 
 I guess I still don't agree with this. From the gpmc timing point of
 view it should not care what device is connected, it just needs to know
 the associated timings. Therefore, the gpmc driver just needs the time
 associated with the different timings fields in the various GPMC_CONFIGx
 registers and then convert to clocks based upon the gpmc fclk. The only
 item that is device specific here is the actual timing values and these
 can be passed to the driver.
 
 Furthermore, gpmc_cs_set_timings function is completely device agnostic.
 Why can we not call this from within the driver? Why does it need to be
 called outside the driver?

As mentioned in one of my previous mails, input to gpmc_cs_set_timings
sometimes are a function of gpmc clock, and that function can vary with
the type of peripheral connected.

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


Re: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-14 Thread Jon Hunter

On 06/14/2012 02:03 AM, Mohammed, Afzal wrote:
 Hi Jon,
 
 On Wed, Jun 13, 2012 at 20:21:50, Hunter, Jon wrote:
  
 If the clk handle for the gpmc is passed to the gpmc driver, then there
 is no reason why the driver cannot do this.
 
 I believe passing clk details through platform data is an abuse of
 clock framework.

Why? You currently have a global variable storing the clock handle. It
can be quite common for drivers to know the clock frequencies of their
functional clocks. How else can drivers calculate timings?

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


RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-14 Thread Mohammed, Afzal
Hi Jon,

On Thu, Jun 14, 2012 at 18:52:55, Hunter, Jon wrote:
 On 06/14/2012 02:03 AM, Mohammed, Afzal wrote:
  On Wed, Jun 13, 2012 at 20:21:50, Hunter, Jon wrote:

  If the clk handle for the gpmc is passed to the gpmc driver, then there
  is no reason why the driver cannot do this.
  
  I believe passing clk details through platform data is an abuse of
  clock framework.
 
 Why? You currently have a global variable storing the clock handle. It
 can be quite common for drivers to know the clock frequencies of their
 functional clocks. How else can drivers calculate timings?


Please see Russell King's comments,

[1] http://permalink.gmane.org/gmane.linux.ports.ppc.embedded/27634
[2] http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg05365.html

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-14 Thread Jon Hunter

On 06/14/2012 08:32 AM, Mohammed, Afzal wrote:
 Hi Jon,
 
 On Thu, Jun 14, 2012 at 18:52:55, Hunter, Jon wrote:
 On 06/14/2012 02:03 AM, Mohammed, Afzal wrote:
 On Wed, Jun 13, 2012 at 20:21:50, Hunter, Jon wrote:
 
 If the clk handle for the gpmc is passed to the gpmc driver, then there
 is no reason why the driver cannot do this.

 I believe passing clk details through platform data is an abuse of
 clock framework.

 Why? You currently have a global variable storing the clock handle. It
 can be quite common for drivers to know the clock frequencies of their
 functional clocks. How else can drivers calculate timings?
 
 
 Please see Russell King's comments,
 
 [1] http://permalink.gmane.org/gmane.linux.ports.ppc.embedded/27634
 [2] 
 http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg05365.html

Thanks. So I still think you need to get rid of the global variable for
storing the gpmc fclk, that is really my point.

So if you look at commit [1] mentioned by Russell in the above thread,
the appropriate thing to do would be to create a gpmc clock alias for
all OMAP2+ devices and then you could simply call the following from the
gpmc probe ...

gpmc_fck = clk_get(pdev-dev, fck);

You could then store somewhere in one of the gpmc structures.

Cheers
Jon

[1] 39a80c7f379e1c1d3e63b204b8353b7381d0a3d5
--
To unsubscribe from this list: send the line unsubscribe 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-14 Thread Jon Hunter
Hi Afzal,

On 06/14/2012 01:17 AM, Mohammed, Afzal wrote:
 Hi Jon,
 
 On Wed, Jun 13, 2012 at 20:21:50, Hunter, Jon wrote:
 
 I do not think it is practically possible. Please see timing calculations
 in arch/arm/mach-omap2/gpmc-*, the way it is done for different
 peripherals are different, and we cannot expect gpmc driver to do those as
 that would require gpmc driver being aware of type of peripheral connected.

 And all those gpmc-* timing calculation needs to be done before driver
 is ready, they rely on functions like gpmc_get_fclk_rate(), which in turn
 requires the clk rate to be available before driver is probed.

 So I see that the various gpmc-*.c files have some form of _retime()
 function. However, at the end of the day they all call
 gpmc_cs_set_timings() to convert time into gpmc clocks. Converting time
 to gpmc clocks is completely independent of the actual device and so
 this can be performed by the driver. We just need to populate the
 gpmc_timings struct and pass to the driver to convert to clocks and
 program into the registers.
 
 gpmc_cs_set_timings() does currently convert time to clock cycles required,
 and this gpmc driver have the capability to do it.
 
 What I was saying is a different issue, input to gpmc_cs_set_timings, which
 is time sometimes in turn is a function of time or to be exact depends on
 gpmc clock period also. So timings provided to gpmc_cs_set_timings for a
 particular frequency may not hold good for another frequency, unless we
 change the input time to gpmc_cs_set_timings based on gpmc clock.

Absolutely! No argument there.

 If you see gpmc-* files, many a times, they need to know value of gpmc fclk,
 to calculate the input time to be fed for gpmc_cs_set_timings

I understand, but that is not my point. My point is that timings should
be provided in nanoseconds for all devices. Then the gpmc driver can
query the gpmc fclk and convert to gpmc cycles. This way the gpmc driver
will not care what device is connected in terms of the timings and can
convert them.

What does make this a bit more difficult is the function
gpmc_round_ns_to_ticks(). It appears to convert nanoseconds to ticks and
back to nanoseconds. I am guessing to account for some rounding error. I
am curious what impact this function is having on the timing. Ideally
this should be handle in gpmc_cs_set_timings().

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


Re: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-14 Thread Paul Walmsley
On Thu, 14 Jun 2012, Jon Hunter wrote:

 What does make this a bit more difficult is the function
 gpmc_round_ns_to_ticks(). It appears to convert nanoseconds to ticks and
 back to nanoseconds. I am guessing to account for some rounding error. I
 am curious what impact this function is having on the timing. Ideally
 this should be handle in gpmc_cs_set_timings().

I don't have the code in front of me at the moment, nor my old GPMC timing 
spreadsheet, so this comment may be superfluous.  But since kernel 
arithmetic needs to be in integers, it might be necessary to do some of 
the timing calculations in picosecond units to avoid roundoff issues.


- 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 v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-13 Thread Tony Lindgren
* Mohammed, Afzal af...@ti.com [120612 22:24]:
 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.

Yeah I also think the GPMC code should handle the L3 timings, and dynamically
calculate them for DVFS when L3 frequency changes. Does the GPMC have enough
information now to do that?

Additionally GPMC should add constraints to DVFS if not enough data is know
to scale L3.

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


RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-13 Thread Mohammed, Afzal
Hi Tony,

On Wed, Jun 13, 2012 at 17:32:09, Tony Lindgren wrote:
 * Mohammed, Afzal af...@ti.com [120612 22:24]:
  Hi Jon,
  
  On Tue, Jun 12, 2012 at 23:10:01, Hunter, Jon wrote:

   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.
 
 Yeah I also think the GPMC code should handle the L3 timings, and dynamically
 calculate them for DVFS when L3 frequency changes. Does the GPMC have enough
 information now to do that?

Do you mean that gpmc driver should have the capability to calculate peripheral
timings at runtime based on frequency ?, I am not sure how this can be handled
by gpmc driver as calculation for different peripherals are done in different
way, requiring gpmc driver to know about connected peripheral, that would imply
that gpmc driver would not be peripheral agnostic.

Or else some sort of a callback to be used ?

Out of the 20,14 are depending on bootloader, both omap3evm  beagle has been
converted to utilize runtime calculation, but for other 12 boards, first
we need to get values used by bootloader (those include peripherals that doesn't
have gpmc-* helpers), then derive it based on expression, after that only, we
will have information to achieve it and those are the ones that I do not
have access to.
 
Regards
Afzal

 
 Additionally GPMC should add constraints to DVFS if not enough data is know
 to scale L3.
 
 Tony
 

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


Re: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-13 Thread Tony Lindgren
* Mohammed, Afzal af...@ti.com [120613 06:10]:
 Hi Tony,
 
 On Wed, Jun 13, 2012 at 17:32:09, Tony Lindgren wrote:
  * Mohammed, Afzal af...@ti.com [120612 22:24]:
   Hi Jon,
   
   On Tue, Jun 12, 2012 at 23:10:01, Hunter, Jon wrote:
 
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.
  
  Yeah I also think the GPMC code should handle the L3 timings, and 
  dynamically
  calculate them for DVFS when L3 frequency changes. Does the GPMC have enough
  information now to do that?
 
 Do you mean that gpmc driver should have the capability to calculate 
 peripheral
 timings at runtime based on frequency ?, I am not sure how this can be handled
 by gpmc driver as calculation for different peripherals are done in different
 way, requiring gpmc driver to know about connected peripheral, that would 
 imply
 that gpmc driver would not be peripheral agnostic.
 
 Or else some sort of a callback to be used ?

Oops yeah right, we have some platform_retime functions that are in the
driver platform init code. It would be best that the drivers can do the
recalculation with no platform init code callbacks needed.
 
 Out of the 20,14 are depending on bootloader, both omap3evm  beagle has been
 converted to utilize runtime calculation, but for other 12 boards, first
 we need to get values used by bootloader (those include peripherals that 
 doesn't
 have gpmc-* helpers), then derive it based on expression, after that only, we
 will have information to achieve it and those are the ones that I do not
 have access to.

It's OK to use fixed timings as long as we disable L3 scaling. Some of
these values we'll probably never be able to calculate dynamically.

Regards,

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


RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-13 Thread Mohammed, Afzal
Hi Tony,

On Wed, Jun 13, 2012 at 19:09:38, Tony Lindgren wrote:
 * Mohammed, Afzal af...@ti.com [120613 06:10]:
  On Wed, Jun 13, 2012 at 17:32:09, Tony Lindgren wrote:

  Do you mean that gpmc driver should have the capability to calculate 
  peripheral
  timings at runtime based on frequency ?, I am not sure how this can be 
  handled
  by gpmc driver as calculation for different peripherals are done in 
  different
  way, requiring gpmc driver to know about connected peripheral, that would 
  imply
  that gpmc driver would not be peripheral agnostic.
  
  Or else some sort of a callback to be used ?
 
 Oops yeah right, we have some platform_retime functions that are in the
 driver platform init code. It would be best that the drivers can do the
 recalculation with no platform init code callbacks needed.

Other than using a platform callback, I am not getting any idea as of now,
how this can be achieved, let me think over it, if you have any suggestions
on ways to deal it, please let me know.

  
  Out of the 20,14 are depending on bootloader, both omap3evm  beagle has 
  been
  converted to utilize runtime calculation, but for other 12 boards, first
  we need to get values used by bootloader (those include peripherals that 
  doesn't
  have gpmc-* helpers), then derive it based on expression, after that only, 
  we
  will have information to achieve it and those are the ones that I do not
  have access to.
 
 It's OK to use fixed timings as long as we disable L3 scaling. Some of
 these values we'll probably never be able to calculate dynamically.

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 v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-13 Thread Jon Hunter
Hi Afzal,

On 06/13/2012 12:20 AM, Mohammed, Afzal wrote:
 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.

So I see that the various gpmc-*.c files have some form of _retime()
function. However, at the end of the day they all call
gpmc_cs_set_timings() to convert time into gpmc clocks. Converting time
to gpmc clocks is completely independent of the actual device and so
this can be performed by the driver. We just need to populate the
gpmc_timings struct and pass to the driver to convert to clocks and
program into the registers.

If the clk handle for the gpmc is passed to the gpmc driver, then there
is no reason why the driver cannot do this.

Obviously, I could be missing something fundamental here, but my
assumption is that if the driver has the handle the fclk (and hence can
query the clock rate) and has then gpmc_timings struct (with timings in
time), then that is all it needs.

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


Re: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-13 Thread Jon Hunter
Hi Afzal,

On 06/13/2012 08:05 AM, Mohammed, Afzal wrote:
 Hi Tony,
 
 On Wed, Jun 13, 2012 at 17:32:09, Tony Lindgren wrote:
 * Mohammed, Afzal af...@ti.com [120612 22:24]:
 Hi Jon,

 On Tue, Jun 12, 2012 at 23:10:01, Hunter, Jon wrote:
 
 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.

 Yeah I also think the GPMC code should handle the L3 timings, and dynamically
 calculate them for DVFS when L3 frequency changes. Does the GPMC have enough
 information now to do that?
 
 Do you mean that gpmc driver should have the capability to calculate 
 peripheral
 timings at runtime based on frequency ?, I am not sure how this can be handled
 by gpmc driver as calculation for different peripherals are done in different
 way, requiring gpmc driver to know about connected peripheral, that would 
 imply
 that gpmc driver would not be peripheral agnostic.

I guess I still don't agree with this. From the gpmc timing point of
view it should not care what device is connected, it just needs to know
the associated timings. Therefore, the gpmc driver just needs the time
associated with the different timings fields in the various GPMC_CONFIGx
registers and then convert to clocks based upon the gpmc fclk. The only
item that is device specific here is the actual timing values and these
can be passed to the driver.

Furthermore, gpmc_cs_set_timings function is completely device agnostic.
Why can we not call this from within the driver? Why does it need to be
called outside the driver?

I am looking for you to prove me wrong about this, but if the only
reason is because we need to fclk rate and timings, then as I have
mentioned before we can pass these to the driver. So I am looking for a
stronger reason than this of why this would not work.

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


RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-12 Thread Mohammed, Afzal
Hi Jon,

On Tue, Jun 12, 2012 at 01:26:29, Hunter, Jon wrote:

  +   pdev = omap_device_build(name, -1, oh, pdata,
  +   sizeof(*pdata), NULL, 0, 0);
  +   if (IS_ERR(pdev)) {
  +   WARN(1, Can't build omap_device for %s:%s.\n,
  +   name, oh-name);
  +   return PTR_ERR(pdev);
  +   }
  +
  +   gpmc_l3_clk = clk_get(NULL, oh-main_clk);
  +   if (IS_ERR(gpmc_l3_clk)) {
  +   pr_err(Could not get GPMC clock\n);
  +   return PTR_ERR(gpmc_l3_clk);
  +   }
 
 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

 Also we should attempt to get the clk before calling omap_device_build
 which is registering the driver.

omap_device_build registers only device, but I agree that clk should
be obtained before it as driver can get probed at that 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: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-12 Thread Jon Hunter

On 06/12/2012 01:53 AM, Mohammed, Afzal wrote:
 Hi Jon,
 
 On Tue, Jun 12, 2012 at 01:26:29, Hunter, Jon wrote:
 
 +   pdev = omap_device_build(name, -1, oh, pdata,
 +   sizeof(*pdata), NULL, 0, 0);
 +   if (IS_ERR(pdev)) {
 +   WARN(1, Can't build omap_device for %s:%s.\n,
 +   name, oh-name);
 +   return PTR_ERR(pdev);
 +   }
 +
 +   gpmc_l3_clk = clk_get(NULL, oh-main_clk);
 +   if (IS_ERR(gpmc_l3_clk)) {
 +   pr_err(Could not get GPMC clock\n);
 +   return PTR_ERR(gpmc_l3_clk);
 +   }

 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.

 Also we should attempt to get the clk before calling omap_device_build
 which is registering the driver.
 
 omap_device_build registers only device, but I agree that clk should
 be obtained before it as driver can get probed at that point

Exactly.

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


RE: [PATCH 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


[PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-11 Thread Afzal Mohammed
Create API for platforms to adapt gpmc to HWMOD

Signed-off-by: Afzal Mohammed af...@ti.com
---
 arch/arm/mach-omap2/gpmc.c |   31 +++
 arch/arm/plat-omap/include/plat/gpmc.h |2 ++
 2 files changed, 33 insertions(+)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 517953f..b471c2f 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -27,6 +27,7 @@
 
 #include asm/mach-types.h
 #include plat/gpmc.h
+#include plat/omap_device.h
 
 #include plat/sdrc.h
 
@@ -898,6 +899,36 @@ static int __init gpmc_init(void)
 }
 postcore_initcall(gpmc_init);
 
+__init int omap_gpmc_init(struct gpmc_pdata *pdata)
+{
+   struct omap_hwmod *oh;
+   struct platform_device *pdev;
+   char *name = omap-gpmc;
+   char *oh_name = gpmc;
+
+   oh = omap_hwmod_lookup(oh_name);
+   if (!oh) {
+   pr_err(Could not look up %s\n, oh_name);
+   return -ENODEV;
+   }
+
+   pdev = omap_device_build(name, -1, oh, pdata,
+   sizeof(*pdata), NULL, 0, 0);
+   if (IS_ERR(pdev)) {
+   WARN(1, Can't build omap_device for %s:%s.\n,
+   name, oh-name);
+   return PTR_ERR(pdev);
+   }
+
+   gpmc_l3_clk = clk_get(NULL, oh-main_clk);
+   if (IS_ERR(gpmc_l3_clk)) {
+   pr_err(Could not get GPMC clock\n);
+   return PTR_ERR(gpmc_l3_clk);
+   }
+
+   return 0;
+}
+
 static irqreturn_t gpmc_handle_irq(int irq, void *dev)
 {
int i;
diff --git a/arch/arm/plat-omap/include/plat/gpmc.h 
b/arch/arm/plat-omap/include/plat/gpmc.h
index 21a8cce..698fa33 100644
--- a/arch/arm/plat-omap/include/plat/gpmc.h
+++ b/arch/arm/plat-omap/include/plat/gpmc.h
@@ -207,6 +207,8 @@ struct gpmc_nand_regs {
 extern void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs);
 extern int gpmc_get_client_irq(unsigned irq_config);
 
+extern int omap_gpmc_init(struct gpmc_pdata *pdata);
+
 extern unsigned int gpmc_ns_to_ticks(unsigned int time_ns);
 extern unsigned int gpmc_ps_to_ticks(unsigned int time_ps);
 extern unsigned int gpmc_ticks_to_ns(unsigned int ticks);
-- 
1.7.10.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 v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD

2012-06-11 Thread Jon Hunter
Hi Afzal,

On 06/11/2012 09:26 AM, Afzal Mohammed wrote:
 Create API for platforms to adapt gpmc to HWMOD
 
 Signed-off-by: Afzal Mohammed af...@ti.com
 ---
  arch/arm/mach-omap2/gpmc.c |   31 +++
  arch/arm/plat-omap/include/plat/gpmc.h |2 ++
  2 files changed, 33 insertions(+)
 
 diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
 index 517953f..b471c2f 100644
 --- a/arch/arm/mach-omap2/gpmc.c
 +++ b/arch/arm/mach-omap2/gpmc.c
 @@ -27,6 +27,7 @@
  
  #include asm/mach-types.h
  #include plat/gpmc.h
 +#include plat/omap_device.h
  
  #include plat/sdrc.h
  
 @@ -898,6 +899,36 @@ static int __init gpmc_init(void)
  }
  postcore_initcall(gpmc_init);
  
 +__init int omap_gpmc_init(struct gpmc_pdata *pdata)
 +{
 + struct omap_hwmod *oh;
 + struct platform_device *pdev;
 + char *name = omap-gpmc;
 + char *oh_name = gpmc;
 +
 + oh = omap_hwmod_lookup(oh_name);
 + if (!oh) {
 + pr_err(Could not look up %s\n, oh_name);
 + return -ENODEV;
 + }
 +
 + pdev = omap_device_build(name, -1, oh, pdata,
 + sizeof(*pdata), NULL, 0, 0);
 + if (IS_ERR(pdev)) {
 + WARN(1, Can't build omap_device for %s:%s.\n,
 + name, oh-name);
 + return PTR_ERR(pdev);
 + }
 +
 + gpmc_l3_clk = clk_get(NULL, oh-main_clk);
 + if (IS_ERR(gpmc_l3_clk)) {
 + pr_err(Could not get GPMC clock\n);
 + return PTR_ERR(gpmc_l3_clk);
 + }

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.

Also we should attempt to get the clk before calling omap_device_build
which is registering the driver.

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