[U-Boot] [PATCH 1/3] thermal: Add multiple instance support

Simon Glass sjg at chromium.org
Thu Mar 21 05:22:15 UTC 2019


Hi Keerthy,

On Tue, 19 Mar 2019 at 22:23, keerthy <j-keerthy at ti.com> wrote:
>
>
>
> On 3/19/2019 6:53 AM, Simon Glass wrote:
> > Hi Keerthy,
> >
> > On Mon, 11 Mar 2019 at 14:13, Keerthy <j-keerthy at ti.com> wrote:
> >>
> >> Currently single instance temperature read out is supported.
> >> Enhance the same to support multiple instances.
> >>
> >> Signed-off-by: Keerthy <j-keerthy at ti.com>
> >> ---
> >>   arch/arm/mach-imx/cpu.c          |  2 +-
> >>   drivers/mmc/omap_hsmmc.c         |  2 +-
> >>   drivers/thermal/imx_thermal.c    |  2 +-
> >>   drivers/thermal/thermal-uclass.c |  4 ++--
> >>   drivers/thermal/ti-bandgap.c     | 13 ++++++++++---
> >>   include/thermal.h                |  6 ++++--
> >>   6 files changed, 19 insertions(+), 10 deletions(-)
> >
> > Please can you add a simple test for the thermal uclass in test/dm?
>
> Okay. I will try to add that.
>
> >
> >>
> >> diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c
> >> index 6b83f92662..27d12b410e 100644
> >> --- a/arch/arm/mach-imx/cpu.c
> >> +++ b/arch/arm/mach-imx/cpu.c
> >> @@ -231,7 +231,7 @@ int print_cpuinfo(void)
> >>          printf("(%dC to %dC)", minc, maxc);
> >>          ret = uclass_get_device(UCLASS_THERMAL, 0, &thermal_dev);
> >>          if (!ret) {
> >> -               ret = thermal_get_temp(thermal_dev, &cpu_tmp);
> >> +               ret = thermal_get_temp(thermal_dev, 0, &cpu_tmp);
> >>
> >>                  if (!ret)
> >>                          printf(" at %dC\n", cpu_tmp);
> >> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> >> index 826a39fad7..1872273dd9 100644
> >> --- a/drivers/mmc/omap_hsmmc.c
> >> +++ b/drivers/mmc/omap_hsmmc.c
> >> @@ -642,7 +642,7 @@ static int omap_hsmmc_execute_tuning(struct udevice *dev, uint opcode)
> >>                  printf("Couldn't get thermal device for tuning\n");
> >>                  return ret;
> >>          }
> >> -       ret = thermal_get_temp(thermal_dev, &temperature);
> >> +       ret = thermal_get_temp(thermal_dev, 0, &temperature);
> >>          if (ret) {
> >>                  printf("Couldn't get temperature for tuning\n");
> >>                  return ret;
> >> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> >> index e50b85bd59..271a61356c 100644
> >> --- a/drivers/thermal/imx_thermal.c
> >> +++ b/drivers/thermal/imx_thermal.c
> >> @@ -207,7 +207,7 @@ static int read_cpu_temperature(struct udevice *dev)
> >>   }
> >>   #endif
> >>
> >> -int imx_thermal_get_temp(struct udevice *dev, int *temp)
> >> +int imx_thermal_get_temp(struct udevice *dev, int instance, int *temp)
> >>   {
> >>          struct thermal_data *priv = dev_get_priv(dev);
> >>          int cpu_tmp = 0;
> >> diff --git a/drivers/thermal/thermal-uclass.c b/drivers/thermal/thermal-uclass.c
> >> index a4ea1e2914..b4a31280cb 100644
> >> --- a/drivers/thermal/thermal-uclass.c
> >> +++ b/drivers/thermal/thermal-uclass.c
> >> @@ -13,14 +13,14 @@
> >>   #include <linux/list.h>
> >>
> >>
> >> -int thermal_get_temp(struct udevice *dev, int *temp)
> >> +int thermal_get_temp(struct udevice *dev, u8 instance, int *temp)
> >>   {
> >>          const struct dm_thermal_ops *ops = device_get_ops(dev);
> >>
> >>          if (!ops->get_temp)
> >>                  return -ENOSYS;
> >>
> >> -       return ops->get_temp(dev, temp);
> >> +       return ops->get_temp(dev, instance, temp);
> >>   }
> >>
> >>   UCLASS_DRIVER(thermal) = {
> >> diff --git a/drivers/thermal/ti-bandgap.c b/drivers/thermal/ti-bandgap.c
> >> index b490391e96..6d3606b0fc 100644
> >> --- a/drivers/thermal/ti-bandgap.c
> >> +++ b/drivers/thermal/ti-bandgap.c
> >> @@ -27,6 +27,7 @@
> >>   struct ti_bandgap {
> >>          ulong                   base;
> >>          int                     temperature;    /* in mili degree celsius */
> >> +       int                     sens_cnt;
> >>   };
> >>
> >>   /*
> >> @@ -158,11 +159,16 @@ static int dra752_adc_to_temp[] = {
> >>          123800, 124200, 124600, 124900, 125000, 125000,
> >>   };
> >>
> >> -static int ti_bandgap_get_temp(struct udevice *dev,  int *temp)
> >> +static int ti_bandgap_get_temp(struct udevice *dev, u8 instance, int *temp)
> >>   {
> >>          struct ti_bandgap *bgp = dev_get_priv(dev);
> >>
> >> -       bgp->temperature = 0x3ff & readl(bgp->base + CTRL_CORE_TEMP_SENSOR_MPU);
> >> +       if (instance >= bgp->sens_cnt) {
> >> +               printf("Only %d insatnces are supported\n", bgp->sens_cnt);
> >> +               return -EINVAL;
> >> +       }
> >> +
> >> +       bgp->temperature = 0x3ff & readl(bgp->base + instance * 4);
> >>          *temp = dra752_adc_to_temp[bgp->temperature - DRA752_ADC_START_VALUE];
> >>
> >>          return 0;
> >> @@ -177,13 +183,14 @@ static int ti_bandgap_probe(struct udevice *dev)
> >>          struct ti_bandgap *bgp = dev_get_priv(dev);
> >>
> >>          bgp->base = devfdt_get_addr_index(dev, 1);
> >> +       bgp->sens_cnt = dev_get_driver_data(dev);
> >>
> >>          return 0;
> >>   }
> >>
> >>   static const struct udevice_id of_ti_bandgap_match[] = {
> >>          {
> >> -               .compatible = "ti,dra752-bandgap",
> >> +               .compatible = "ti,dra752-bandgap", .data = 3,
> >>          },
> >>          {},
> >>   };
> >> diff --git a/include/thermal.h b/include/thermal.h
> >> index 11d75256e0..82d80f3ded 100644
> >> --- a/include/thermal.h
> >> +++ b/include/thermal.h
> >> @@ -9,7 +9,7 @@
> >>
> >>   #include <dm.h>
> >>
> >> -int thermal_get_temp(struct udevice *dev, int *temp);
> >> +int thermal_get_temp(struct udevice *dev, u8 instance, int *temp);
> >
> > Please add function comment.
>
> Okay.
>
> >
> >>
> >>   /**
> >>    * struct dm_thermal_ops - Driver model Thermal operations
> >> @@ -25,9 +25,11 @@ struct dm_thermal_ops {
> >>           * It will enable and initialize any Thermal hardware as necessary.
> >>           *
> >>           * @dev:        The Thermal device
> >> +        * @instance:   The instance of sensor in case multiple sensors are
> >> +        *              present if single sensor then this should be 0
> >>           * @temp:       pointer that returns the measured temperature
> >>           */
> >> -       int (*get_temp)(struct udevice *dev, int *temp);
> >> +       int (*get_temp)(struct udevice *dev, u8 instance, int *temp);
> >
> > I don't like the name 'instance' very much. With pwm we use 'channel'.
> > Would that be OK?
>
> How about 'sensors'? Basically multiple sensors give out temperatures
> of different thermal zones.

Hmm I still prefer channel to fit in with PWM. But if you really want
to use 'sensor' (singular, not plural) then OK.

Regards,
Simon


More information about the U-Boot mailing list