[U-Boot] [PATCH v7 2/4] DM: thermal: Add imx thermal DM driver

[Simon Glass]

Hi,

On 20 November 2014 at 06:14, Ye.Li <B37916 at freescale.com> wrote:
> Add a new thermal uclass for thermal sensor and implement the imx
> thermal driver basing on this uclass.
>
> Signed-off-by: Ye.Li <B37916 at freescale.com>
> ---
>  drivers/Makefile                 |    1 +
>  drivers/thermal/Makefile         |    9 ++
>  drivers/thermal/imx_thermal.c    |  177 ++++++++++++++++++++++++++++++++++++++
>  drivers/thermal/thermal-uclass.c |   30 +++++++
>  include/dm/uclass-id.h           |    1 +
>  include/imx_thermal.h            |   17 ++++
>  include/thermal.h                |   42 +++++++++
>  7 files changed, 277 insertions(+), 0 deletions(-)
>

[snip]

> diff --git a/include/thermal.h b/include/thermal.h
> new file mode 100644
> index 0000000..80b549b
> --- /dev/null
> +++ b/include/thermal.h
> @@ -0,0 +1,42 @@
> +/*
> + *
> + * (C) Copyright 2014 Freescale Semiconductor, Inc
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef _THERMAL_H_
> +#define _THERMAL_H_
> +
> +#include <dm.h>
> +
> +int thermal_get_temp(struct udevice *dev, int *temp);
> +
> +/**
> + * struct struct dm_thermal_ops - Driver model Thermal operations
> + *
> + * The uclass interface is implemented by all Thermal devices which use
> + * driver model.
> + */
> +struct dm_thermal_ops {
> +       /**
> +        * Get the current temperature
> +        *
> +        * The device provided is the slave device. It's parent controller
> +        * will be used to provide the communication.
> +        *
> +        * This must be called before doing any transfers with a Thermal slave. It
> +        * will enable and initialize any Thermal hardware as necessary, and make
> +        * sure that the SCK line is in the correct idle state. It is not
> +        * allowed to claim the same bus for several slaves without releasing
> +        * the bus in between.
> +        *
> +        * @dev:        The Thermal device
> +        *
> +        * Returns: 0 if the bus was claimed successfully, or a negative value
> +        * if it wasn't.
> +        */

I know this comment is very late - sorry but I did not see this uclass
at the time.

Please can you fix up the command above? It seems to be copied from
SPI and makes no sense. Also please document the @temp parameter.

> +       int (*get_temp)(struct udevice *dev, int *temp);
> +};
> +
> +#endif /* _THERMAL_H_ */

Finally, all uclasses should have test code. In this case there should
be something like test/dm/thermal.c with a simple test to get the
device and check its temperature. You can add a driver for sandbox to
make this work. If we don't follow this rule right from the start then
the next person to come along and add to your uclass will not feel
obliged to write tests.

You will probably find test/dm/gpio.c helpful, although you will only
need a few lines of it for your test (e.g. uclass_get_device()
followed by thermal_get_temp()). For sandbox,
drivers/misc/i2c_eeprom_emul.c might help although again it has more
than you need.

Regards,
Simon