Hello Peng,

I have few comments.

On 07/28/2015 04:48 PM, Peng Fan wrote:
1. Support driver model for pfuze100.
2. Introduce a new Kconfig entry DM_PMIC_PFUZE100 for pfuze100
3. 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....@freescale.com>
Cc: Przemyslaw Marczak <p.marc...@samsung.com>
Cc: Simon Glass <s...@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.

@@ -2,15 +2,22 @@
   * Copyright (C) 2014 Gateworks Corporation
   * Tim Harvey <thar...@gateworks.com>
   *
+ * Copyright (C) 2015 Freescale Semiconductor, Inc
+ * Peng Fan <peng....@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?

+       { .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.

+       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.

+       int regulators_node;
+       const void *blob = gd->fdt_blob;
+       int children;
+

...snip...

Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to