Hi Akshay,

On Thu, Jan 24, 2013 at 4:02 AM, Akshay Saraswat akshay.s@samsung.comwrote:

Hi Simon,

If we create two versions of dtt_get_temp(), we have to declare unnecessarily few configs and their values like CONFIG_DTT_SENSORS and CONFIG_SYS_DTT_BUS_NUM when we can easily ignore them. That's why I thought of including TMU to dtt this way. I dont think we need both TMU and I2C bus sensors together anywhere. Please suggest which way is better.

(Sorry I am on holiday for a week so not very responsive. I get back next Thurs)

The thing is that these two implementations share no code at all. I think the way you have done it is good, I was just wondering whether it would be better to have do_dtt_i2c() and do_dtt_tmu() as separate function. But I suppose that is really not any better. The alternative would be to share the same do_dtt() function, with it avoiding the i2c code if CONFIG_SYS_DTT_BUS_NUM is defined, and using a single sensor number 0 if CONFIG_DTT_SENSORS is not defined. That could use much of the same code (perhaps with a weak function for dtt_init_one(). But that is probably trying too hard - if support for multiple TMU sensors is required in the future we can worry about it then.

Sorry for the trouble.

Regards, Simon

Regards,

Akshay

------- *Original Message* -------

*Sender* : Simon Glasssjg@chromium.org

*Date* : Jan 23, 2013 05:51 (GMT+09:00)

*Title* : Re: [PATCH 6/7 v5] TMU: Add TMU support in dtt command

Hi Akshay,

On Mon, Jan 21, 2013 at 3:11 AM, Akshay Saraswat **wrote:

...

Add generic TMU support alongwith i2c sensors in dtt command to enable temperature reading in cases where TMU is present instead of i2c sensors.

Signed-off-by: Akshay Saraswat **

Changes since v4: - Removed tmu command and added to dtt.

common/cmd_dtt.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)

diff --git a/common/cmd_dtt.c b/common/cmd_dtt.c index cd94423..715f4ba 100644 --- a/common/cmd_dtt.c +++ b/common/cmd_dtt.c @@ -28,6 +28,20 @@ #include ** #include **

+#if defined CONFIG_TMU_CMD_DTT +#include **

+void dtt_get_temp(void) +{

•   int cur_temp;
  

•   if (tmu_monitor(&cur_temp) == TMU_STATUS_INIT)
  

•           printf("TMU is in unknown state, temperature is invalid
  

\n");

puts()

Should return an error result here so that do_dtt() returns 1.

...

•   else
  

•           printf("Current temperature: %u degrees Celsius \n",
  

cur_temp);

...

+}

+#else static unsigned long sensor_initialized;

static void _initialize_dtt(void) @@ -59,9 +73,13 @@ void dtt_init(void) /* switch back to original I2C bus */ I2C_SET_BUS(old_bus); } +#endif

int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) { +#if defined CONFIG_TMU_CMD_DTT

•   dtt_get_temp();
  

+#else

How about creating two versions of the dtt_get_temp() function: one with your code and one with the old code? Then you don't have an #ifdef here.

...

    int i;
    unsigned char sensors[] = CONFIG_DTT_SENSORS;
    int old_bus;

@@ -83,6 +101,7 @@ int do_dtt (cmd_tbl_t * cmdtp, int flag, int argc,

char * const argv[])

...

    /* switch back to original I2C bus */
    I2C_SET_BUS(old_bus);

+#endif

    return 0;

} /* do_dtt() */

1.7.9.5

Regards, Simon