Hi Akshay, On Thu, Jan 24, 2013 at 4:02 AM, Akshay Saraswat <[email protected]>wrote:
> 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 Glass<[email protected]> > > *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 > > > >
<<201301232033203_QKNMBDIF.gif>>
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
