[U-Boot] [PATCH 05/19] dm: pmic: add implementation of driver model regulator uclass

Przemyslaw Marczak
2014-10-10T13:41:20Z

On 10/10/2014 05:10 AM, Simon Glass wrote:
> Hi,
>
> On 8 October 2014 14:48, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
>> This is the implementation of driver model regulator uclass api.
>> To use it, the CONFIG_DM_PMIC is required with driver implementation,
>> since it provides pmic devices I/O API.
>>
>> The regulator framework is based on a 'structure dm_regulator_ops',
>> which provides all regulator functions call types.
>>
>> The optional and useful regulator features are two descriptor types:
>> - struct regulator_desc - describes the regulator name and value limits
>> should be set by device driver for each regulator number.
>> - struct regulator_mode_desc - also should be defined as mode array for
>> each regulator, since regulators supports few modes, at least: ON/OFF.
>>
>> The regulator driver operations are clear and described in file:
>> include/power/regulator.h:
>>
>> Each regulator "struct driver.ops" should point to "struct dm_regulator_ops".
>> If do, then the drivers can use the regulator api(if implemented):
>> - pmic_ldo_cnt(...)
>> - pmic_buck_cnt(...)
>> - pmic_get_ldo_val(...)
>> - pmic_set_ldo_val(...)
>> - pmic_get_ldo_mode(...)
>> - pmic_set_ldo_mode(...)
>> - pmic_get_buck_val(...)
>> - pmic_set_buck_val(...)
>> - pmic_get_buck_mode(...)
>> - pmic_set_buck_mode(...)
>>
>> To get the regulator device we can use two functions:
>> - pmic_get_by_name(...)
>> - pmic_get_by_interface(...)
>
> I've just got a few high-level comment on this series so will respond
> to each patch.
>
Ok
>>
>> Main files:
>> - drivers/power/regulator-uclass.c - provides regulator common functions api
>> - include/power/regulator.h - define all structures required by the regulator
>>
>> Changes:
>> - new uclass-id: UCLASS_PMIC_REGULATOR
>> - new config: CONFIG_DM_REGULATOR
>>
>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>> ---
>> drivers/power/Makefile | 1 +
>> drivers/power/regulator-uclass.c | 250 ++++++++++++++++++++++++++++++++++++
>> include/dm/uclass-id.h | 1 +
>> include/power/pmic.h | 18 +++
>> include/power/regulator.h | 267 +++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 537 insertions(+)
>> create mode 100644 drivers/power/regulator-uclass.c
>> create mode 100644 include/power/regulator.h
>>
>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
>> index 8def501..9a0b8c4 100644
>> --- a/drivers/power/Makefile
>> +++ b/drivers/power/Makefile
>> @@ -22,3 +22,4 @@ obj-$(CONFIG_POWER_SPI) += power_spi.o
>> obj-$(CONFIG_DM_PMIC_SPI) += pmic_spi.o
>> obj-$(CONFIG_DM_PMIC_I2C) += pmic_i2c.o
>> obj-$(CONFIG_DM_PMIC) += pmic-uclass.o
>> +obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o
>> diff --git a/drivers/power/regulator-uclass.c b/drivers/power/regulator-uclass.c
>> new file mode 100644
>> index 0000000..4c9614e
>> --- /dev/null
>> +++ b/drivers/power/regulator-uclass.c
>> @@ -0,0 +1,250 @@
>> +/*
>> + * Copyright (C) 2014 Samsung Electronics
>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +#include <common.h>
>> +#include <linux/types.h>
>> +#include <fdtdec.h>
>> +#include <power/pmic.h>
>> +#include <i2c.h>
>> +#include <compiler.h>
>> +#include <dm.h>
>> +#include <dm/device.h>
>> +#include <dm/lists.h>
>> +#include <dm/device-internal.h>
>> +#include <errno.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +int pmic_ldo_cnt(struct udevice *dev)
>> +{
>> + const struct dm_regulator_ops *ops;
>> +
>> + ops = pmic_get_uclass_ops(dev, UCLASS_PMIC_REGULATOR);
>> + if (!ops)
>> + return -ENODEV;
>> +
>> + if (!ops->get_ldo_cnt)
>> + return -EPERM;
>> +
>> + return ops->get_ldo_cnt(dev);
>> +}
>> +
>> +int pmic_buck_cnt(struct udevice *dev)
>> +{
>> + const struct dm_regulator_ops *ops;
>> +
>> + ops = pmic_get_uclass_ops(dev, UCLASS_PMIC_REGULATOR);
>> + if (!ops)
>> + return -ENODEV;
>> +
>> + if (!ops->get_buck_cnt)
>> + return -EPERM;
>> +
>> + return ops->get_buck_cnt(dev);
>> +}
>> +
>> +struct regulator_desc *pmic_ldo_desc(struct udevice *dev, int ldo)
>
> I think these should return an error, with the struct * return as a parameter.
>
Ok, maybe this could be better.
>> +{
>> + const struct dm_regulator_ops *ops;
>> +
>> + ops = pmic_get_uclass_ops(dev, UCLASS_PMIC_REGULATOR);
>> + if (!ops)
>> + return NULL;
>> +
>> + if (!ops->get_val_desc)
>> + return NULL;
>> +
>> + return ops->get_val_desc(dev, DESC_TYPE_LDO, ldo);
>> +}
>> +
>> +struct regulator_desc *pmic_buck_desc(struct udevice *dev, int buck)
>> +{
>> + const struct dm_regulator_ops *ops;
>> +
>> + ops = pmic_get_uclass_ops(dev, UCLASS_PMIC_REGULATOR);
>> + if (!ops)
>> + return NULL;
>> +
>> + if (!ops->get_val_desc)
>> + return NULL;
>> +
>> + return ops->get_val_desc(dev, DESC_TYPE_BUCK, buck);
>> +}
>> +
>> +struct regulator_mode_desc *pmic_ldo_mode_desc(struct udevice *dev, int ldo,
>> + int *mode_cnt)
>> +{
>> + const struct dm_regulator_ops *ops;
>> +
>> + ops = pmic_get_uclass_ops(dev, UCLASS_PMIC_REGULATOR);
>> + if (!ops)
>> + return NULL;
>> +
>> + if (!ops->get_mode_desc_array)
>> + return NULL;
>> +
>> + return ops->get_mode_desc_array(dev, DESC_TYPE_LDO, ldo, mode_cnt);
>> +}
>> +
>> +struct regulator_mode_desc *pmic_buck_mode_desc(struct udevice *dev, int buck,
>> + int *mode_cnt)
>> +{
>> + const struct dm_regulator_ops *ops;
>> +
>> + ops = pmic_get_uclass_ops(dev, UCLASS_PMIC_REGULATOR);
>> + if (!ops)
>> + return NULL;
>> +
>> + if (!ops->get_mode_desc_array)
>> + return NULL;
>> +
>> + return ops->get_mode_desc_array(dev, DESC_TYPE_BUCK, buck, mode_cnt);
>> +}
>> +
>> +int pmic_get_ldo_val(struct udevice *dev, int ldo)
>> +{
>> + const struct dm_regulator_ops *ops;
>> + int val = -1;
>> +
>> + ops = pmic_get_uclass_ops(dev, UCLASS_PMIC_REGULATOR);
>> + if (!ops)
>> + return -ENODEV;
>> +
>> + if (!ops->ldo_val)
>> + return -EPERM;
>> +
>> + if (ops->ldo_val(dev, PMIC_OP_GET, ldo, &val))
>> + return -EIO;
>> +
>> + return val;
>> +}
>> +
>
> Regards,
> Simon
>

Thanks,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com