Hi Przemyslaw, On 8 May 2015 at 10:20, Przemyslaw Marczak <p.marc...@samsung.com> wrote: > 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: increment child_info pointer if operation in loop fail > > Signed-off-by: Przemyslaw Marczak <p.marc...@samsung.com> > --- > drivers/power/pmic/pmic-uclass.c | 25 +++++++++---------------- > include/power/pmic.h | 39 ++++++++++++++++++++------------------- > 2 files changed, 29 insertions(+), 35 deletions(-) >
Acked-by: Simon Glass <s...@chromium.org> Tested on sandbox: Tested-by: Simon Glass <s...@chromium.org> BTW in pmic_bind_children() you have something like this: info = child_info; while (info->prefix) { ... if (ret) { debug(" - child binding error: %d\n", ret); info++; continue; } I think this would be better: for (info = child_info; info->prefix, info++) and remove the three info++ expressions inside the loop. > diff --git a/drivers/power/pmic/pmic-uclass.c > b/drivers/power/pmic/pmic-uclass.c > index d82d3da..6766bd5 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; > @@ -68,6 +68,7 @@ int pmic_bind_childs(struct udevice *pmic, int offset, > if (!drv) { > debug(" - driver: '%s' not found!\n", > info->driver); > + info++; > continue; > } > > @@ -77,6 +78,7 @@ int pmic_bind_childs(struct udevice *pmic, int offset, > node, &child); > if (ret) { > debug(" - child binding error: %d\n", ret); > + info++; > continue; > } > > @@ -110,16 +112,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 +129,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 +142,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 > -- > 1.9.1 > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot