The cleanup includes: - pmic.h - fix mistakes in a few comments - pmic operations: value 'reg_count' - redefine as function call - fix function name: pmic_bind_childs() -> pmic_bind_children() - pmic_bind_children: change the 'while' loop with the 'for' - add implementation of pmic_reg_count() method - pmic_bind_children() - update function call name - Kconfig: add new line at the end of file - Update MAX77686 driver code
Signed-off-by: Przemyslaw Marczak p.marczak@samsung.com Acked-by: Simon Glass sjg@chromium.org Tested on sandbox: Tested-by: Simon Glass sjg@chromium.org --- Changes V2: - cleanup of the pmic_bind_children() loop - split with: dm: pmic: max77686: update driver code --- drivers/power/pmic/Kconfig | 2 +- drivers/power/pmic/max77686.c | 15 ++++++++++----- drivers/power/pmic/pmic-uclass.c | 31 +++++++++---------------------- include/power/pmic.h | 39 ++++++++++++++++++++------------------- 4 files changed, 40 insertions(+), 47 deletions(-)
diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig index af68783..d99d9e3 100644 --- a/drivers/power/pmic/Kconfig +++ b/drivers/power/pmic/Kconfig @@ -15,4 +15,4 @@ config DM_PMIC_MAX77686 depends on DM_PMIC ---help--- This config enables implementation of driver-model pmic uclass features - for PMIC MAX77686. The driver implements read/write operations. \ No newline at end of file + for PMIC MAX77686. The driver implements read/write operations. diff --git a/drivers/power/pmic/max77686.c b/drivers/power/pmic/max77686.c index e9503e2..3523b4a 100644 --- a/drivers/power/pmic/max77686.c +++ b/drivers/power/pmic/max77686.c @@ -16,12 +16,17 @@
DECLARE_GLOBAL_DATA_PTR;
-static const struct pmic_child_info pmic_childs_info[] = { +static const struct pmic_child_info pmic_children_info[] = { { .prefix = "ldo", .driver = MAX77686_LDO_DRIVER }, { .prefix = "buck", .driver = MAX77686_BUCK_DRIVER }, { }, };
+static int max77686_reg_count(struct udevice *dev) +{ + return MAX77686_NUM_OF_REGS; +} + static int max77686_write(struct udevice *dev, uint reg, const uint8_t *buff, int len) { @@ -47,7 +52,7 @@ static int max77686_bind(struct udevice *dev) { int regulators_node; const void *blob = gd->fdt_blob; - int childs; + int children;
regulators_node = fdt_subnode_offset(blob, dev->of_offset, "voltage-regulators"); @@ -59,8 +64,8 @@ static int max77686_bind(struct udevice *dev)
debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
- childs = pmic_bind_childs(dev, regulators_node, pmic_childs_info); - if (!childs) + children = pmic_bind_children(dev, regulators_node, pmic_children_info); + if (!children) debug("%s: %s - no child found\n", __func__, dev->name);
/* Always return success for this device */ @@ -68,7 +73,7 @@ static int max77686_bind(struct udevice *dev) }
static struct dm_pmic_ops max77686_ops = { - .reg_count = MAX77686_NUM_OF_REGS, + .reg_count = max77686_reg_count, .read = max77686_read, .write = max77686_write, }; diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c index d82d3da..812ac13 100644 --- a/drivers/power/pmic/pmic-uclass.c +++ b/drivers/power/pmic/pmic-uclass.c @@ -27,8 +27,8 @@ static ulong str_get_num(const char *ptr, const char *maxptr) return simple_strtoul(ptr, NULL, 0); }
-int pmic_bind_childs(struct udevice *pmic, int offset, - const struct pmic_child_info *child_info) +int pmic_bind_children(struct udevice *pmic, int offset, + const struct pmic_child_info *child_info) { const struct pmic_child_info *info; const void *blob = gd->fdt_blob; @@ -53,14 +53,10 @@ int pmic_bind_childs(struct udevice *pmic, int offset, node);
child = NULL; - info = child_info; - while (info->prefix) { + for (info = child_info; info->prefix && info->driver; info++) { prefix_len = strlen(info->prefix); - if (strncasecmp(info->prefix, node_name, prefix_len) || - !info->driver) { - info++; + if (strncasecmp(info->prefix, node_name, prefix_len)) continue; - }
debug(" - compatible prefix: '%s'\n", info->prefix);
@@ -110,16 +106,16 @@ int pmic_get(const char *name, struct udevice **devp) int pmic_reg_count(struct udevice *dev) { const struct dm_pmic_ops *ops = dev_get_driver_ops(dev); - if (!ops) + + if (!ops || !ops->reg_count) return -ENOSYS;
- return ops->reg_count; + return ops->reg_count(dev); }
int pmic_read(struct udevice *dev, uint reg, uint8_t *buffer, int len) { const struct dm_pmic_ops *ops = dev_get_driver_ops(dev); - int ret;
if (!buffer) return -EFAULT; @@ -127,17 +123,12 @@ int pmic_read(struct udevice *dev, uint reg, uint8_t *buffer, int len) if (!ops || !ops->read) return -ENOSYS;
- ret = ops->read(dev, reg, buffer, len); - if (ret) - return ret; - - return 0; + return ops->read(dev, reg, buffer, len); }
int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) { const struct dm_pmic_ops *ops = dev_get_driver_ops(dev); - int ret;
if (!buffer) return -EFAULT; @@ -145,11 +136,7 @@ int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) if (!ops || !ops->write) return -ENOSYS;
- ret = ops->write(dev, reg, buffer, len); - if (ret) - return ret; - - return 0; + return ops->write(dev, reg, buffer, len); }
UCLASS_DRIVER(pmic) = { diff --git a/include/power/pmic.h b/include/power/pmic.h index f7ae781..eb152ef 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -11,9 +11,9 @@ #ifndef __CORE_PMIC_H_ #define __CORE_PMIC_H_
-#include <linux/list.h> -#include <spi.h> #include <i2c.h> +#include <spi.h> +#include <linux/list.h> #include <power/power_chrg.h>
enum { PMIC_I2C, PMIC_SPI, PMIC_NONE}; @@ -90,10 +90,10 @@ struct pmic { * U-Boot PMIC Framework * ===================== * - * UCLASS_PMIC - The is designed to provide an I/O interface for PMIC devices. + * UCLASS_PMIC - This is designed to provide an I/O interface for PMIC devices. * * For the multi-function PMIC devices, this can be used as parent I/O device - * for each IC's interface. Then, each children uses its parent for read/write. + * for each IC's interface. Then, each child uses its parent for read/write. * * The driver model tree could look like this: * @@ -112,8 +112,8 @@ struct pmic { * We can find two PMIC cases in boards design: * - single I/O interface * - multiple I/O interfaces - * We bind single PMIC device for each interface, to provide an I/O as a parent, - * of proper child devices. Each child usually implements a different function, + * We bind a single PMIC device for each interface, to provide an I/O for + * its child devices. And each child usually implements a different function, * controlled by the same interface. * * The binding should be done automatically. If device tree nodes/subnodes are @@ -131,7 +131,7 @@ struct pmic { * Note: * Each PMIC interface driver should use a different compatible string. * - * If each pmic child device driver need access the PMIC-specific registers, + * If a PMIC child device driver needs access the PMIC-specific registers, * it need know only the register address and the access can be done through * the parent pmic driver. Like in the example: * @@ -141,10 +141,10 @@ struct pmic { * | |_ dev: my_regulator (set value/etc..) (is child) - UCLASS_REGULATOR * * To ensure such device relationship, the pmic device driver should also bind - * all its child devices, like in the example below. It should be done by call - * the 'pmic_bind_childs()' - please refer to the description of this function - * in this header file. This function, should be called in the driver's '.bind' - * method. + * all its child devices, like in the example below. It can be done by calling + * the 'pmic_bind_children()' - please refer to the function description, which + * can be found in this header file. This function, should be called inside the + * driver's bind() method. * * For the example driver, please refer the MAX77686 driver: * - 'drivers/power/pmic/max77686.c' @@ -156,18 +156,19 @@ struct pmic { * Should be implemented by UCLASS_PMIC device drivers. The standard * device operations provides the I/O interface for it's childs. * - * @reg_count: devices register count + * @reg_count: device's register count * @read: read 'len' bytes at "reg" and store it into the 'buffer' * @write: write 'len' bytes from the 'buffer' to the register at 'reg' address */ struct dm_pmic_ops { - int reg_count; + int (*reg_count)(struct udevice *dev); int (*read)(struct udevice *dev, uint reg, uint8_t *buffer, int len); int (*write)(struct udevice *dev, uint reg, const uint8_t *buffer, int len); };
-/* enum pmic_op_type - used for various pmic devices operation calls, +/** + * enum pmic_op_type - used for various pmic devices operation calls, * for reduce a number of lines with the same code for read/write or get/set. * * @PMIC_OP_GET - get operation @@ -181,7 +182,7 @@ enum pmic_op_type { /** * struct pmic_child_info - basic device's child info for bind child nodes with * the driver by the node name prefix and driver name. This is a helper struct - * for function: pmic_bind_childs(). + * for function: pmic_bind_children(). * * @prefix - child node name prefix (or its name if is unique or single) * @driver - driver name for the sub-node with prefix @@ -194,7 +195,7 @@ struct pmic_child_info { /* drivers/power/pmic-uclass.c */
/** - * pmic_bind_childs() - bind drivers for given parent pmic, using child info + * pmic_bind_children() - bind drivers for given parent pmic, using child info * found in 'child_info' array. * * @pmic - pmic device - the parent of found child's @@ -216,7 +217,7 @@ struct pmic_child_info { * (pmic - bind automatically by compatible) * compatible = "my_pmic"; * ... - * (pmic's childs - bind by pmic_bind_childs()) + * (pmic's childs - bind by pmic_bind_children()) * (nodes prefix: "ldo", driver: "my_regulator_ldo") * ldo1 { ... }; * ldo2 { ... }; @@ -226,8 +227,8 @@ struct pmic_child_info { * buck2 { ... }; * }; */ -int pmic_bind_childs(struct udevice *pmic, int offset, - const struct pmic_child_info *child_info); +int pmic_bind_children(struct udevice *pmic, int offset, + const struct pmic_child_info *child_info);
/** * pmic_get: get the pmic device using its name