Re: [RFC PATCH 10/11] omap4: thermal: add basic CPU thermal zone

2012-05-29 Thread Cousson, Benoit

On 5/28/2012 12:26 PM, Valentin, Eduardo wrote:

Hello Santosh,

On Mon, May 28, 2012 at 12:48 PM, Felipe Balbiba...@ti.com  wrote:

Hi,

On Mon, May 28, 2012 at 03:03:26PM +0530, Shilimkar, Santosh wrote:

On Fri, May 25, 2012 at 1:56 PM, Eduardo Valentin
eduardo.valen...@ti.com  wrote:

This patch exposes OMAP4 thermal sensor as a thermal zone
named cpu. Only thermal creation is done here.

TODO:

  - Add cooling bindings
  - Add extrapolation rules

Signed-off-by: Eduardo Valentineduardo.valen...@ti.com
---
  drivers/thermal/Kconfig |   12 ++
  drivers/thermal/Makefile|1 +
  drivers/thermal/omap-bandgap.c  |1 +
  drivers/thermal/omap-bandgap.h  |   12 ++
  drivers/thermal/omap4-thermal.c |   72 +++
  5 files changed, 98 insertions(+), 0 deletions(-)
  create mode 100644 drivers/thermal/omap4-thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index ffdd240..2e82797 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -39,3 +39,15 @@ config OMAP_BANDGAP
  This includes alert interrupts generation and also the TSHUT
  support.

+config OMAP4_THERMAL
+   bool Texas Instruments OMAP4 thermal support
+   depends on OMAP_BANDGAP
+   depends on ARCH_OMAP4
+   help
+ If you say yes here you get thermal support for the Texas Instruments
+ OMAP4 SoC family. The current chip supported are:
+  - OMAP4460
+

It's more of IP feature than OMAP specific, so something like

config  HAVE_BANDGAP_THERMAL_SUPPORT

and then let processor's which support enable it. That OMAP varients
in AMXX etc if needed can make use of it.


Those are just an OMAP in disguise.


If you agree, then rest of the driver also can be cleaned to avoid
omap_* in file names and variables.

Apart from this minor comment, rest of the patch looks fine to me.


Then it will appear as a fully generic bandgap driver, which is not the
case. This is really a TI thing, right ?


I have to agree with Felipe here. I do see your point to have a name
which is applicable to AMxx, but having such generic naming is not
helping either :-( as this bandgap driver is not supposed to be
generic.

I though of ti_bandgap.*, but still seams to be misleading..


Yeah, I guess omap_bandgap is good enough. This issues is mainly due to 
the version in OMAP4 for my point of view.


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


Re: [RFC PATCH 10/11] omap4: thermal: add basic CPU thermal zone

2012-05-28 Thread Shilimkar, Santosh
On Fri, May 25, 2012 at 1:56 PM, Eduardo Valentin
eduardo.valen...@ti.com wrote:
 This patch exposes OMAP4 thermal sensor as a thermal zone
 named cpu. Only thermal creation is done here.

 TODO:

  - Add cooling bindings
  - Add extrapolation rules

 Signed-off-by: Eduardo Valentin eduardo.valen...@ti.com
 ---
  drivers/thermal/Kconfig         |   12 ++
  drivers/thermal/Makefile        |    1 +
  drivers/thermal/omap-bandgap.c  |    1 +
  drivers/thermal/omap-bandgap.h  |   12 ++
  drivers/thermal/omap4-thermal.c |   72 
 +++
  5 files changed, 98 insertions(+), 0 deletions(-)
  create mode 100644 drivers/thermal/omap4-thermal.c

 diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
 index ffdd240..2e82797 100644
 --- a/drivers/thermal/Kconfig
 +++ b/drivers/thermal/Kconfig
 @@ -39,3 +39,15 @@ config OMAP_BANDGAP
          This includes alert interrupts generation and also the TSHUT
          support.

 +config OMAP4_THERMAL
 +       bool Texas Instruments OMAP4 thermal support
 +       depends on OMAP_BANDGAP
 +       depends on ARCH_OMAP4
 +       help
 +         If you say yes here you get thermal support for the Texas 
 Instruments
 +         OMAP4 SoC family. The current chip supported are:
 +          - OMAP4460
 +
It's more of IP feature than OMAP specific, so something like

config  HAVE_BANDGAP_THERMAL_SUPPORT

and then let processor's which support enable it. That OMAP varients
in AMXX etc if needed can make use of it.

If you agree, then rest of the driver also can be cleaned to avoid
omap_* in file names and variables.

Apart from this minor comment, rest of the patch looks fine to me.

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


Re: [RFC PATCH 10/11] omap4: thermal: add basic CPU thermal zone

2012-05-28 Thread Felipe Balbi
Hi,

On Mon, May 28, 2012 at 03:03:26PM +0530, Shilimkar, Santosh wrote:
 On Fri, May 25, 2012 at 1:56 PM, Eduardo Valentin
 eduardo.valen...@ti.com wrote:
  This patch exposes OMAP4 thermal sensor as a thermal zone
  named cpu. Only thermal creation is done here.
 
  TODO:
 
   - Add cooling bindings
   - Add extrapolation rules
 
  Signed-off-by: Eduardo Valentin eduardo.valen...@ti.com
  ---
   drivers/thermal/Kconfig         |   12 ++
   drivers/thermal/Makefile        |    1 +
   drivers/thermal/omap-bandgap.c  |    1 +
   drivers/thermal/omap-bandgap.h  |   12 ++
   drivers/thermal/omap4-thermal.c |   72 
  +++
   5 files changed, 98 insertions(+), 0 deletions(-)
   create mode 100644 drivers/thermal/omap4-thermal.c
 
  diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
  index ffdd240..2e82797 100644
  --- a/drivers/thermal/Kconfig
  +++ b/drivers/thermal/Kconfig
  @@ -39,3 +39,15 @@ config OMAP_BANDGAP
           This includes alert interrupts generation and also the TSHUT
           support.
 
  +config OMAP4_THERMAL
  +       bool Texas Instruments OMAP4 thermal support
  +       depends on OMAP_BANDGAP
  +       depends on ARCH_OMAP4
  +       help
  +         If you say yes here you get thermal support for the Texas 
  Instruments
  +         OMAP4 SoC family. The current chip supported are:
  +          - OMAP4460
  +
 It's more of IP feature than OMAP specific, so something like
 
 config  HAVE_BANDGAP_THERMAL_SUPPORT
 
 and then let processor's which support enable it. That OMAP varients
 in AMXX etc if needed can make use of it.

Those are just an OMAP in disguise.

 If you agree, then rest of the driver also can be cleaned to avoid
 omap_* in file names and variables.
 
 Apart from this minor comment, rest of the patch looks fine to me.

Then it will appear as a fully generic bandgap driver, which is not the
case. This is really a TI thing, right ?

-- 
balbi


signature.asc
Description: Digital signature


Re: [RFC PATCH 10/11] omap4: thermal: add basic CPU thermal zone

2012-05-28 Thread Valentin, Eduardo
Hello Santosh,

On Mon, May 28, 2012 at 12:48 PM, Felipe Balbi ba...@ti.com wrote:
 Hi,

 On Mon, May 28, 2012 at 03:03:26PM +0530, Shilimkar, Santosh wrote:
 On Fri, May 25, 2012 at 1:56 PM, Eduardo Valentin
 eduardo.valen...@ti.com wrote:
  This patch exposes OMAP4 thermal sensor as a thermal zone
  named cpu. Only thermal creation is done here.
 
  TODO:
 
   - Add cooling bindings
   - Add extrapolation rules
 
  Signed-off-by: Eduardo Valentin eduardo.valen...@ti.com
  ---
   drivers/thermal/Kconfig         |   12 ++
   drivers/thermal/Makefile        |    1 +
   drivers/thermal/omap-bandgap.c  |    1 +
   drivers/thermal/omap-bandgap.h  |   12 ++
   drivers/thermal/omap4-thermal.c |   72 
  +++
   5 files changed, 98 insertions(+), 0 deletions(-)
   create mode 100644 drivers/thermal/omap4-thermal.c
 
  diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
  index ffdd240..2e82797 100644
  --- a/drivers/thermal/Kconfig
  +++ b/drivers/thermal/Kconfig
  @@ -39,3 +39,15 @@ config OMAP_BANDGAP
           This includes alert interrupts generation and also the TSHUT
           support.
 
  +config OMAP4_THERMAL
  +       bool Texas Instruments OMAP4 thermal support
  +       depends on OMAP_BANDGAP
  +       depends on ARCH_OMAP4
  +       help
  +         If you say yes here you get thermal support for the Texas 
  Instruments
  +         OMAP4 SoC family. The current chip supported are:
  +          - OMAP4460
  +
 It's more of IP feature than OMAP specific, so something like

 config  HAVE_BANDGAP_THERMAL_SUPPORT

 and then let processor's which support enable it. That OMAP varients
 in AMXX etc if needed can make use of it.

 Those are just an OMAP in disguise.

 If you agree, then rest of the driver also can be cleaned to avoid
 omap_* in file names and variables.

 Apart from this minor comment, rest of the patch looks fine to me.

 Then it will appear as a fully generic bandgap driver, which is not the
 case. This is really a TI thing, right ?

I have to agree with Felipe here. I do see your point to have a name
which is applicable to AMxx, but having such generic naming is not
helping either :-( as this bandgap driver is not supposed to be
generic.

I though of ti_bandgap.*, but still seams to be misleading..

I guess same applies to usb_phy and scm.

Again, if you have a name that fits better I won't hesitate to use it :-)


 --
 balbi



-- 

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