Hello Przemyslaw, On Mon, Aug 03, 2015 at 05:01:12PM +0200, Przemyslaw Marczak wrote:
Hello Peng,
I have few comments.
On 07/28/2015 04:48 PM, Peng Fan wrote:
- Support driver model for pfuze100.
- Introduce a new Kconfig entry DM_PMIC_PFUZE100 for pfuze100
- This driver intends to support PF100, PF200 and PF3000, so add the device id into the udevice_id array.
Signed-off-by: Peng Fan Peng.Fan@freescale.com Cc: Przemyslaw Marczak p.marczak@samsung.com Cc: Simon Glass sjg@chromium.org
drivers/power/pmic/Kconfig | 7 +++ drivers/power/pmic/pmic_pfuze100.c | 89 ++++++++++++++++++++++++++++++++++++++ include/power/pfuze100_pmic.h | 3 ++ 3 files changed, 99 insertions(+)
diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig index 164f421..0df91be 100644 --- a/drivers/power/pmic/Kconfig +++ b/drivers/power/pmic/Kconfig @@ -10,6 +10,13 @@ config DM_PMIC
- 'drivers/power/pmic/pmic-uclass.c'
- 'include/power/pmic.h'
+config DM_PMIC_PFUZE100
- bool "Enable Driver Model for PMIC PFUZE100"
- depends on DM_PMIC
- ---help---
- This config enables implementation of driver-model pmic uclass features
- for PMIC PFUZE100. The driver implements read/write operations.
config DM_PMIC_MAX77686 bool "Enable Driver Model for PMIC MAX77686" depends on DM_PMIC diff --git a/drivers/power/pmic/pmic_pfuze100.c b/drivers/power/pmic/pmic_pfuze100.c index 22a04c0..be9d05c 100644 --- a/drivers/power/pmic/pmic_pfuze100.c +++ b/drivers/power/pmic/pmic_pfuze100.c
Please keep the new convention of file naming and use just pfuze100.c. Then, later we will remove the old files.
Ok. Will rename the file to pfuze100.c in V2.
@@ -2,15 +2,22 @@
- Copyright (C) 2014 Gateworks Corporation
- Tim Harvey tharvey@gateworks.com
- Copyright (C) 2015 Freescale Semiconductor, Inc
- Peng Fan Peng.Fan@freescale.com
*/
- SPDX-License-Identifier: GPL-2.0+
#include <common.h> +#include <fdtdec.h> #include <errno.h> +#include <dm.h> #include <i2c.h> #include <power/pmic.h> +#include <power/regulator.h> #include <power/pfuze100_pmic.h>
+#ifndef CONFIG_DM_PMIC int power_pfuze100_init(unsigned char bus) { static const char name[] = "PFUZE100"; @@ -30,3 +37,85 @@ int power_pfuze100_init(unsigned char bus)
return 0; } +#else +DECLARE_GLOBAL_DATA_PTR;
+static const struct pmic_child_info pmic_children_info[] = {
- /* sw[x], swbst */
The driver name is used at two places, so could please define it in the header?
Ok. Will fix in V2.
- { .prefix = "s", .driver = "pfuze100_regulator" },
- /* vgen[x], vsnvs, vcc, v33, vcc_sd */
- { .prefix = "v", .driver = "pfuze100_regulator" },
- { },
+};
+static int pfuze100_reg_count(struct udevice *dev) +{
This enum name is too general, so please add proper prefix.
Will fix this in V2.
- return PMIC_NUM_OF_REGS;
+}
+static int pfuze100_write(struct udevice *dev, uint reg, const uint8_t *buff,
int len)+{
- if (dm_i2c_write(dev, reg, buff, len)) {
error("write error to device: %p register: %#x!", dev, reg);return -EIO;- }
- return 0;
+}
+static int pfuze100_read(struct udevice *dev, uint reg, uint8_t *buff, int len) +{
- if (dm_i2c_read(dev, reg, buff, len)) {
error("read error from device: %p register: %#x!", dev, reg);return -EIO;- }
- return 0;
+}
+static int pfuze100_bind(struct udevice *dev) +{
Please sort the variables below.
Ok.
[...]
Regards, Peng. --