Hi,
On 20 October 2014 09:44, Przemyslaw Marczak <p.marc...@samsung.com> wrote:
> Hello Simon,
>
> I missed some of your comments.
>
>
> On 10/11/2014 01:18 AM, Simon Glass wrote:
>>
>> Hi,
>>
>> On 10 October 2014 07:32, Przemyslaw Marczak <p.marc...@samsung.com>
>> wrote:
>>>
>>> Hello Simon,
>>>
>>>
>>> On 10/10/2014 05:17 AM, Simon Glass wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> On 8 October 2014 14:48, Przemyslaw Marczak <p.marc...@samsung.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> This is an introduction to driver-model multi class PMIC support.
>>>>> It starts with UCLASS_PMIC - a common PMIC class type for I/O, which
>>>>> doesn't need to implement any specific operations and features beside
>>>>> the platform data, which is the 'struct pmic_platdata' defined in file:
>>>>> - 'include/power/pmic.h'
>>>>>
>>>>> New files:
>>>>> - pmic-uclass.c - provides a common code for UCLASS_PMIC drivers
>>>>> - pmic_i2c.c - provides dm interface for i2c pmic drivers
>>>>> - pmic_spi.c - provides dm interface for spi pmic drivers
>>>>>
>>>>> Those files are based on a current PMIC framework files and code.
>>>>> The new files are introduced to keep code readability and allow to
>>>>> make an easy drivers migration. The old pmic framework is still kept
>>>>> and full independent.
>>>>>
>>>>> Changes:
>>>>> - new uclass-id: UCLASS_PMIC,
>>>>> - new configs: CONFIG_DM_PMIC; CONFIG_DM_PMIC_SPI; CONFIG_DM_PMIC_I2C,
>>>>>
>>>>> New pmic api is documented in: doc/README.power-framework-dm
>>>>>
>>>>> Signed-off-by: Przemyslaw Marczak <p.marc...@samsung.com>
>>>>> ---
>>>>> drivers/power/Makefile | 3 +
>>>>> drivers/power/pmic-uclass.c | 255
>>>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>>> drivers/power/pmic_i2c.c | 136 +++++++++++++++++++++++
>>>>> drivers/power/pmic_spi.c | 137 ++++++++++++++++++++++++
>>>>> include/dm/uclass-id.h | 3 +
>>>>> include/power/pmic.h | 64 +++++++++++
>>>>> 6 files changed, 598 insertions(+)
>>>>> create mode 100644 drivers/power/pmic-uclass.c
>>>>> create mode 100644 drivers/power/pmic_i2c.c
>>>>> create mode 100644 drivers/power/pmic_spi.c
>>>>>
>>>>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
>>>>> index dc64e4d..8def501 100644
>>>>> --- a/drivers/power/Makefile
>>>>> +++ b/drivers/power/Makefile
>>>>> @@ -19,3 +19,6 @@ obj-$(CONFIG_DIALOG_POWER) += power_dialog.o
>>>>> obj-$(CONFIG_POWER_FSL) += power_fsl.o
>>>>> obj-$(CONFIG_POWER_I2C) += power_i2c.o
>>>>> 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
>>>>> diff --git a/drivers/power/pmic-uclass.c b/drivers/power/pmic-uclass.c
>>>>> new file mode 100644
>>>>> index 0000000..5e8494b
>>>>> --- /dev/null
>>>>> +++ b/drivers/power/pmic-uclass.c
>>>>> @@ -0,0 +1,255 @@
>>>>> +/*
>>>>> + * Copyright (C) 2014 Samsung Electronics
>>>>> + * Przemyslaw Marczak <p.marc...@samsung.com>
>>>>> + *
>>>>> + * SPDX-License-Identifier: GPL-2.0+
>>>>> + */
>>>>> +#include <common.h>
>>>>> +#include <linux/types.h>
>>>>> +#include <fdtdec.h>
>>>>> +#include <power/pmic.h>
>>>>> +#include <dm/device-internal.h>
>>>>> +#include <dm/uclass-internal.h>
>>>>> +#include <dm/root.h>
>>>>> +#include <dm/lists.h>
>>>>> +#include <i2c.h>
>>>>> +#include <compiler.h>
>>>>> +#include <errno.h>
>>>>> +
>>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>>> +
>>>>> +int pmic_check_reg(struct pmic_platdata *p, unsigned reg)
>>>>> +{
>>>>> + if (!p)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + if (reg >= p->regs_num) {
>>>>> + error("<reg num> = %d is invalid. Should be less than
>>>>> %d",
>>>>> + reg, p->regs_num);
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +int pmic_reg_write(struct udevice *dev, unsigned reg, unsigned val)
>>>>> +{
>>>>> + struct pmic_platdata *p;
>>>>> +
>>>>> + p = dev_get_platdata(dev);
>>>>> + if (!p)
>>>>> + return -EPERM;
>>>>> +
>>>>> + switch (p->interface) {
>>>>> + case PMIC_I2C:
>>>>> +#ifdef CONFIG_DM_PMIC_I2C
>>>>> + return pmic_i2c_reg_write(dev, reg, val);
>>>>> +#else
>>>>> + return -ENOSYS;
>>>>> +#endif
>>>>> + case PMIC_SPI:
>>>>> +#ifdef CONFIG_DM_PMIC_SPI
>>>>> + return pmic_spi_reg_write(dev, reg, val);
>>>>> +#else
>>>>> + return -ENOSYS;
>>>>> +#endif
>>>>
>>>>
>>>>
>>>> Perhaps one day we should add another uclass which is some sort of
>>>> register cache. It could be implemented by I2C and SPI drivers (or
>>>> better perhaps the I2C and SPI uclasses could provide a register cache
>>>> uclass device for their main when requested).
>>>>
>>>> For now this seems fine though. I think the 'return -ENOSYS' can just
>>>> go at the end of the function and appear once.
>>>>
>>>
>>> Why do we need the registers cache?
>>> This maybe is good for a many read operations at a time in the system,
>>> but actually I think, that it is not required for the bootloader
>>> purposes.
>>>
>>> If we write some data - sequential, then we probably would like to check
>>> it
>>> in the same time, e.g. update some range of I2C registers.
>>>
>>> I don't know the Chromebook design, but how it could be useful for you?
>>
>>
>> We don't need a register cache - I was just thinking of that from the
>> kernel. But it feels like an interface like:
>>
>> int reg_read(int reg, uint32_t *value)
>> int reg_write(int reg, uint32_t value)
>> int reg_write_range(int reg_start, int reg_count, uint32_t value[])
>>
>> might be useful - it could be implemented for both I2C and SPI.
>>
>
> Ok, now I see.
>
>
>>>
>>>
>>>>> + default:
>>>>> + return -ENODEV;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +int pmic_reg_read(struct udevice *dev, unsigned reg, unsigned *val)
>>>>> +{
>>>>> + struct pmic_platdata *p;
>>>>> +
>>>>> + p = dev_get_platdata(dev);
>>>>> + if (!p)
>>>>> + return -EPERM;
>>>>> +
>>>>> + switch (p->interface) {
>>>>> + case PMIC_I2C:
>>>>> +#ifdef CONFIG_DM_PMIC_I2C
>>>>> + return pmic_i2c_reg_read(dev, reg, val);
>>>>> +#else
>>>>> + return -ENOSYS;
>>>>> +#endif
>>>>> + case PMIC_SPI:
>>>>> +#ifdef CONFIG_DM_PMIC_SPI
>>>>> + return pmic_spi_reg_read(dev, reg, val);
>>>>> +#else
>>>>> + return -ENOSYS;
>>>>> +#endif
>>>>> + default:
>>>>> + return -ENODEV;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +int pmic_probe(struct udevice *dev, struct spi_slave *spi_slave)
>>>>> +{
>>>>> + struct pmic_platdata *p;
>>>>> +
>>>>> + p = dev_get_platdata(dev);
>>>>> + if (!p)
>>>>> + return -EPERM;
>>>>> +
>>>>> + switch (p->interface) {
>>>>> + case PMIC_I2C:
>>>>> +#ifdef CONFIG_DM_PMIC_I2C
>>>>> + return pmic_i2c_probe(dev);
>>>>> +#else
>>>>> + return -ENOSYS;
>>>>> +#endif
>>>>> + case PMIC_SPI:
>>>>> + if (!spi_slave)
>>>>> + return -EINVAL;
>>>>> +#ifdef CONFIG_DM_PMIC_SPI
>>>>> + spi_slave = pmic_spi_probe(dev);
>>>>> + if (!spi_slave)
>>>>> + return -EIO;
>>>>> +
>>>>> + return 0;
>>>>> +#else
>>>>> + return -ENOSYS;
>>>>> +#endif
>>>>> + default:
>>>>> + return -ENODEV;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +struct udevice *pmic_get_by_interface(int uclass_id, int bus, int
>>>>> addr_cs)
>>>>
>>>>
>>>>
>>>> This is an interesting function! Again we can probably improve things
>>>> when we have an i2c uclass.
>>>>
>>> Thanks! But even if we have i2c uclass, then we also should know how to
>>> recognize each device, e.g. in the board code. So the interface and
>>> address
>>> will probably no change.
>>
>>
>> I'm thinking that one day you could do something like:
>>
>> // Find the regmap associated with the device
>> reg_dev = device_get_child(UCLASS_REGMAP, dev);
>>
>> // Call the regmap uclass to access registers whether it is I2C or SPI
>> regmap_write(reg_dev, value);
>>
>
> This could be an another interesting possible feature of dm.
>
>
>>>
>>>
>>>>> +{
>>>>> + struct pmic_platdata *pl;
>>>>> + struct udevice *dev;
>>>>> + struct uclass *uc;
>>>>> + int ret;
>>>>> +
>>>>> + if (uclass_id < 0 || uclass_id >= UCLASS_COUNT) {
>>>>> + error("Bad uclass id.\n");
>>>>> + return NULL;
>>>>> + }
>>>>> +
>>>>> + ret = uclass_get(uclass_id, &uc);
>>>>> + if (ret) {
>>>>> + error("PMIC uclass: %d not initialized!\n", uclass_id);
>>>>> + return NULL;
>>>>> + }
>>>>> +
>>>>> + uclass_foreach_dev(dev, uc) {
>>>>> + if (!dev || !dev->platdata)
>>>>> + continue;
>>>>> +
>>>>> + pl = dev_get_platdata(dev);
>>>>> +
>>>>> + if (pl->bus != bus)
>>>>> + continue;
>>>>> +
>>>>> + switch (pl->interface) {
>>>>> + case PMIC_I2C:
>>>>> + if (addr_cs != pl->hw.i2c.addr)
>>>>> + continue;
>>>>> + break;
>>>>> + case PMIC_SPI:
>>>>> + if (addr_cs != pl->hw.spi.cs)
>>>>> + continue;
>>>>> + break;
>>>>> + default:
>>>>> + error("Unsupported interface of: %s",
>>>>> dev->name);
>>>>> + return NULL;
>>>>> + }
>>>>> +
>>>>> + ret = device_probe(dev);
>>>>> + if (ret) {
>>>>> + error("Dev: %s probe failed", dev->name);
>>>>> + return NULL;
>>>>> + }
>>>>> + return dev;
>>>>> + }
>>>>> +
>>>>> + return NULL;
>>>>> +}
>>>>> +
>>>>> +struct udevice *pmic_get_by_name(int uclass_id, char *name)
>>>>> +{
>>>>> + struct udevice *dev;
>>>>> + struct uclass *uc;
>>>>> + int ret;
>>>>> +
>>>>> + if (uclass_id < 0 || uclass_id >= UCLASS_COUNT) {
>>>>> + error("Bad uclass id.\n");
>>>>> + return NULL;
>>>>> + }
>>>>> +
>>>>> + ret = uclass_get(uclass_id, &uc);
>>>>> + if (ret) {
>>>>> + error("PMIC uclass: %d not initialized!", uclass_id);
>>>>> + return NULL;
>>>>> + }
>>>>> +
>>>>> + uclass_foreach_dev(dev, uc) {
>>>>> + if (!dev)
>>>>> + continue;
>>>>> +
>>>>> + if (!strncmp(name, dev->name, strlen(name))) {
>>>>> + ret = device_probe(dev);
>>>>> + if (ret)
>>>>> + error("Dev: %s probe failed",
>>>>> dev->name);
>>>>> + return dev;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + return NULL;
>>>>> +}
>>>>> +
>>>>> +int pmic_init_dm(void)
>>>>
>>>>
>>>>
>>>> I don't understand why you need this function at all. Driver model
>>>> should find the pmics automatically. Is it because we don't have an
>>>> i2c uclass? In that case I think it would be better to limit this hack
>>>> to I2C. For SPI it should work correctly without this function.
>>>>
>>> Currently, it shouldn't and yes - it isn't because of the I2C uclass
>>> missing. So no one check the I2C nodes - and its child subnodes.
>>> Actually, this can be easy fixed - just don't add the "pmic" alias to the
>>> dts. But I will also change fix the code.
>>>
>>> And by the way - is there any check in the code, which protects for the
>>> device bind more than once?
>>
>>
>> No there is no such check at present.
>>
>>>
>>>
>>>>> +{
>>>>> + const void *blob = gd->fdt_blob;
>>>>> + const struct fdt_property *prop;
>>>>> + struct udevice *dev = NULL;
>>>>> + const char *path;
>>>>> + const char *alias;
>>>>> + int alias_node, node, offset, ret = 0;
>>>>> + int alias_len;
>>>>> + int len;
>>>>> +
>>>>> + alias = "pmic";
>>>>> + alias_len = strlen(alias);
>>>>> +
>>>>> + alias_node = fdt_path_offset(blob, "/aliases");
>>>>> + offset = fdt_first_property_offset(blob, alias_node);
>>>>> +
>>>>> + if (offset < 0) {
>>>>> + error("Alias node not found.");
>>>>> + return -ENODEV;
>>>>> + }
>>>>> +
>>>>> + offset = fdt_first_property_offset(blob, alias_node);
>>>>> + for (; offset > 0; offset = fdt_next_property_offset(blob,
>>>>> offset)) {
>>>>> + prop = fdt_get_property_by_offset(blob, offset, &len);
>>>>> + if (!len)
>>>>> + continue;
>>>>> +
>>>>> + path = fdt_string(blob, fdt32_to_cpu(prop->nameoff));
>>>>> +
>>>>> + if (!strncmp(alias, path, alias_len))
>>>>> + node = fdt_path_offset(blob, prop->data);
>>>>> + else
>>>>> + node = 0;
>>>>> +
>>>>> + if (node <= 0)
>>>>> + continue;
>>>>> +
>>>>> + ret = lists_bind_fdt(gd->dm_root, blob, node, &dev);
>>>>> + if (ret < 0)
>>>>> + continue;
>>>>> +
>>>>> + if (device_probe(dev))
>>>>> + error("Device: %s, probe error", dev->name);
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +UCLASS_DRIVER(pmic) = {
>>>>> + .id = UCLASS_PMIC,
>>>>> + .name = "pmic",
>>>>> +};
>>>>> diff --git a/drivers/power/pmic_i2c.c b/drivers/power/pmic_i2c.c
>>>>> new file mode 100644
>>>>> index 0000000..350d375
>>>>> --- /dev/null
>>>>> +++ b/drivers/power/pmic_i2c.c
>>>>> @@ -0,0 +1,136 @@
>>>>> +/*
>>>>> + * Copyright (C) 2014 Samsung Electronics
>>>>> + * Przemyslaw Marczak <p.marc...@samsung.com>
>>>>> + *
>>>>> + * Copyright (C) 2011 Samsung Electronics
>>>>> + * Lukasz Majewski <l.majew...@samsung.com>
>>>>> + *
>>>>> + * (C) Copyright 2010
>>>>> + * Stefano Babic, DENX Software Engineering, sba...@denx.de
>>>>> + *
>>>>> + * (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
>>>>> + *
>>>>> + * SPDX-License-Identifier: GPL-2.0+
>>>>> + */
>>>>> +
>>>>> +#include <common.h>
>>>>> +#include <linux/types.h>
>>>>> +#include <power/pmic.h>
>>>>> +#include <errno.h>
>>>>> +#include <dm.h>
>>>>> +#include <i2c.h>
>>>>> +
>>>>> +int pmic_i2c_reg_write(struct udevice *dev, unsigned reg, unsigned
>>>>> val)
>>>>> +{
>>>>> + struct pmic_platdata *p;
>>>>> + unsigned char buf[4] = { 0 };
>>>>> +
>>>>> + if (!dev)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + p = dev->platdata;
>>>>> +
>>>>> + if (pmic_check_reg(p, reg))
>>>>> + return -EINVAL;
>>>>> +
>>>>> + I2C_SET_BUS(p->bus);
>>>>> +
>>>>> + switch (pmic_i2c_tx_num) {
>>>>> + case 3:
>>>>> + if (p->byte_order == PMIC_SENSOR_BYTE_ORDER_BIG) {
>>>>> + buf[2] = (cpu_to_le32(val) >> 16) & 0xff;
>>>>> + buf[1] = (cpu_to_le32(val) >> 8) & 0xff;
>>>>> + buf[0] = cpu_to_le32(val) & 0xff;
>>>>> + } else {
>>>>> + buf[0] = (cpu_to_le32(val) >> 16) & 0xff;
>>>>> + buf[1] = (cpu_to_le32(val) >> 8) & 0xff;
>>>>> + buf[2] = cpu_to_le32(val) & 0xff;
>>>>> + }
>>>>> + break;
>>>>> + case 2:
>>>>> + if (p->byte_order == PMIC_SENSOR_BYTE_ORDER_BIG) {
>>>>> + buf[1] = (cpu_to_le32(val) >> 8) & 0xff;
>>>>> + buf[0] = cpu_to_le32(val) & 0xff;
>>>>> + } else {
>>>>> + buf[0] = (cpu_to_le32(val) >> 8) & 0xff;
>>>>> + buf[1] = cpu_to_le32(val) & 0xff;
>>>>> + }
>>>>> + break;
>>>>> + case 1:
>>>>> + buf[0] = cpu_to_le32(val) & 0xff;
>>>>> + break;
>>>>> + default:
>>>>> + printf("%s: invalid tx_num: %d", __func__,
>>>>> pmic_i2c_tx_num);
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + if (i2c_write(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num))
>>>>> + return -1;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +int pmic_i2c_reg_read(struct udevice *dev, unsigned reg, unsigned
>>>>> *val)
>>>>> +{
>>>>> + struct pmic_platdata *p;
>>>>> + unsigned char buf[4] = { 0 };
>>>>> + u32 ret_val = 0;
>>>>> +
>>>>> + if (!dev)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + p = dev->platdata;
>>>>> +
>>>>> + if (pmic_check_reg(p, reg))
>>>>> + return -EINVAL;
>>>>> +
>>>>> + I2C_SET_BUS(p->bus);
>>>>> +
>>>>> + if (i2c_read(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num))
>>>>> + return -1;
>>>>> +
>>>>> + switch (pmic_i2c_tx_num) {
>>>>> + case 3:
>>>>> + if (p->byte_order == PMIC_SENSOR_BYTE_ORDER_BIG)
>>>>> + ret_val = le32_to_cpu(buf[2] << 16
>>>>> + | buf[1] << 8 | buf[0]);
>>>>> + else
>>>>> + ret_val = le32_to_cpu(buf[0] << 16 |
>>>>> + buf[1] << 8 | buf[2]);
>>>>> + break;
>>>>> + case 2:
>>>>> + if (p->byte_order == PMIC_SENSOR_BYTE_ORDER_BIG)
>>>>> + ret_val = le32_to_cpu(buf[1] << 8 | buf[0]);
>>>>> + else
>>>>> + ret_val = le32_to_cpu(buf[0] << 8 | buf[1]);
>>>>> + break;
>>>>> + case 1:
>>>>> + ret_val = le32_to_cpu(buf[0]);
>>>>> + break;
>>>>> + default:
>>>>> + printf("%s: invalid tx_num: %d", __func__,
>>>>> pmic_i2c_tx_num);
>>>>> + return -1;
>>>>> + }
>>>>> + memcpy(val, &ret_val, sizeof(ret_val));
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +int pmic_i2c_probe(struct udevice *dev)
>>>>> +{
>>>>> + struct pmic_platdata *p;
>>>>> +
>>>>> + if (!dev)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + p = dev->platdata;
>>>>> +
>>>>> + i2c_set_bus_num(p->bus);
>>>>> + debug("Bus: %d PMIC:%s probed!\n", p->bus, dev->name);
>>>>> + if (i2c_probe(pmic_i2c_addr)) {
>>>>> + printf("Can't find PMIC:%s\n", dev->name);
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> diff --git a/drivers/power/pmic_spi.c b/drivers/power/pmic_spi.c
>>>>> new file mode 100644
>>>>> index 0000000..7851adf
>>>>> --- /dev/null
>>>>> +++ b/drivers/power/pmic_spi.c
>>>>> @@ -0,0 +1,137 @@
>>>>> +/*
>>>>> + * Copyright (C) 2014 Samsung Electronics
>>>>> + * Przemyslaw Marczak <p.marc...@samsung.com>
>>>>> + *
>>>>> + * Copyright (C) 2011 Samsung Electronics
>>>>> + * Lukasz Majewski <l.majew...@samsung.com>
>>>>> + *
>>>>> + * (C) Copyright 2010
>>>>> + * Stefano Babic, DENX Software Engineering, sba...@denx.de
>>>>> + *
>>>>> + * (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
>>>>> + *
>>>>> + * SPDX-License-Identifier: GPL-2.0+
>>>>> + */
>>>>> +
>>>>> +#include <common.h>
>>>>> +#include <linux/types.h>
>>>>> +#include <power/pmic.h>
>>>>> +#include <errno.h>
>>>>> +#include <dm.h>
>>>>> +#include <spi.h>
>>>>> +
>>>>> +static struct spi_slave *slave;
>>>>> +
>>>>> +void pmic_spi_free(struct spi_slave *slave)
>>>>> +{
>>>>> + if (slave)
>>>>> + spi_free_slave(slave);
>>>>> +}
>>>>> +
>>>>> +struct spi_slave *pmic_spi_probe(struct udevice *dev)
>>>>> +{
>>>>> + struct pmic_platdata *p;
>>>>> +
>>>>> + if (!dev)
>>>>> + return NULL;
>>>>> +
>>>>> + p = dev->platdata;
>>>>> +
>>>>> + if (pmic_check_reg(p, reg))
>>>>> + return NULL;
>>>>> +
>>>>> + return spi_setup_slave(p->bus,
>>>>> + p->hw.spi.cs,
>>>>> + p->hw.spi.clk,
>>>>> + p->hw.spi.mode);
>>>>> +}
>>>>> +
>>>>> +static int pmic_spi_reg(struct udevice *dev, unsigned reg, unsigned
>>>>> *val,
>>>>> + int write)
>>>>> +{
>>>>> + struct pmic_platdata *p;
>>>>> + u32 pmic_tx, pmic_rx;
>>>>> + u32 tmp;
>>>>> +
>>>>> + if (!dev)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + p = dev->platdata;
>>>>> +
>>>>> + if (pmic_check_reg(p, reg))
>>>>> + return -EFAULT;
>>>>> +
>>>>> + if (!slave) {
>>>>> + slave = pmic_spi_probe(p);
>>>>> +
>>>>> + if (!slave)
>>>>> + return -ENODEV;
>>>>> + }
>>>>> +
>>>>> + if (pmic_check_reg(p, reg))
>>>>> + return -EFAULT;
>>>>> +
>>>>> + if (spi_claim_bus(slave))
>>>>> + return -EIO;
>>>>> +
>>>>> + pmic_tx = p->hw.spi.prepare_tx(reg, val, write);
>>>>> +
>>>>> + tmp = cpu_to_be32(pmic_tx);
>>>>> +
>>>>> + if (spi_xfer(slave, pmic_spi_bitlen, &tmp, &pmic_rx,
>>>>> + pmic_spi_flags)) {
>>>>> + spi_release_bus(slave);
>>>>> + return -EIO;
>>>>> + }
>>>>> +
>>>>> + if (write) {
>>>>> + pmic_tx = p->hw.spi.prepare_tx(reg, val, 0);
>>>>> + tmp = cpu_to_be32(pmic_tx);
>>>>> + if (spi_xfer(slave, pmic_spi_bitlen, &tmp, &pmic_rx,
>>>>> + pmic_spi_flags)) {
>>>>> + spi_release_bus(slave);
>>>>> + return -EIO;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + spi_release_bus(slave);
>>>>> + *val = cpu_to_be32(pmic_rx);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +int pmic_spi_reg_write(struct udevice *dev, unsigned reg, unsigned
>>>>> val)
>>>>> +{
>>>>> + struct pmic_platdata *p;
>>>>> +
>>>>> + if (!dev)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + p = dev->platdata;
>>>>> +
>>>>> + if (pmic_check_reg(p, reg))
>>>>> + return -EFAULT;
>>>>> +
>>>>> + if (pmic_spi_reg(p, reg, &val, 1))
>>>>> + return -1;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +int pmic_spi_reg_read(struct udevice *dev, unsigned reg, unsigned
>>>>> *val)
>>>>> +{
>>>>> + struct pmic_platdata *p;
>>>>> +
>>>>> + if (!dev)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + p = dev->platdata;
>>>>> +
>>>>> + if (pmic_check_reg(p, reg))
>>>>> + return -EFAULT;
>>>>> +
>>>>> + if (pmic_spi_reg(p, reg, val, 0))
>>>>> + return -1;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>>>>> index e3e9296..e6d9d39 100644
>>>>> --- a/include/dm/uclass-id.h
>>>>> +++ b/include/dm/uclass-id.h
>>>>> @@ -29,6 +29,9 @@ enum uclass_id {
>>>>> UCLASS_SPI_FLASH, /* SPI flash */
>>>>> UCLASS_CROS_EC, /* Chrome OS EC */
>>>>>
>>>>> + /* PMIC uclass and PMIC related uclasses */
>>>>> + UCLASS_PMIC,
>>>>> +
>>>>> UCLASS_COUNT,
>>>>> UCLASS_INVALID = -1,
>>>>> };
>>>>> diff --git a/include/power/pmic.h b/include/power/pmic.h
>>>>> index afbc5aa..7114650 100644
>>>>> --- a/include/power/pmic.h
>>>>> +++ b/include/power/pmic.h
>>>>> @@ -1,4 +1,7 @@
>>>>> /*
>>>>> + * Copyright (C) 2014 Samsung Electronics
>>>>> + * Przemyslaw Marczak <p.marc...@samsung.com>
>>>>> + *
>>>>> * Copyright (C) 2011-2012 Samsung Electronics
>>>>> * Lukasz Majewski <l.majew...@samsung.com>
>>>>> *
>>>>> @@ -8,9 +11,12 @@
>>>>> #ifndef __CORE_PMIC_H_
>>>>> #define __CORE_PMIC_H_
>>>>>
>>>>> +#include <dm.h>
>>>>> #include <linux/list.h>
>>>>> +#include <spi.h>
>>>>> #include <i2c.h>
>>>>> #include <power/power_chrg.h>
>>>>> +#include <power/regulator.h>
>>>>>
>>>>> enum { PMIC_I2C, PMIC_SPI, PMIC_NONE};
>>>>> enum { I2C_PMIC, I2C_NUM, };
>>>>> @@ -78,6 +84,63 @@ struct pmic {
>>>>> struct list_head list;
>>>>> };
>>>>>
>>>>> +#ifdef CONFIG_DM_PMIC
>>>>> +/* struct pmic_platdata - a standard descriptor for pmic device, which
>>>>> holds
>>>>> + * an informations about interface. It is common for all pmic devices.
>>>>> + *
>>>>> + * Note:
>>>>> + * Interface fields are the same as in: struct pmic.
>>>>> + * Note: struct pmic will be removed in the future after drivers
>>>>> migration
>>>>> + *
>>>>> + * @bus - a physical bus on which device is connected
>>>>> + * @interface - an interface type, one of enum: PMIC_I2C, PMIC_SPI,
>>>>> PMIC_NONE
>>>>> + * @byte_order - one of enum: PMIC_SENSOR_BYTE_ORDER*_LITTLE/_BIG
>>>>> + * @regs_num - number of device registers
>>>>> + * @hw - one of union structure: p_i2c or p_spi
>>>>> + * based on @interface field
>>>>> +*/
>>>>> +struct pmic_platdata {
>>>>> + int bus;
>>>>> + int interface;
>>>>> + int byte_order;
>>>>> + int regs_num;
>>>>> + union hw hw;
>>>>
>>>>
>>>>
>>>> If we have a 'register cache' uclass (later, once i2c is done) then
>>>> this could just be a device.
>>>>
>>>>> +};
>>>>> +
>>>>> +/* enum pmic_op_type - used for various pmic devices operation calls,
>>>>> + * for decrease a number of functions with the same code for
>>>>> read/write
>>>>> + * or get/set.
>>>>> + *
>>>>> + * @PMIC_OP_GET - get operation
>>>>> + * @PMIC_OP_SET - set operation
>>>>> +*/
>>>>> +enum pmic_op_type {
>>>>> + PMIC_OP_GET,
>>>>> + PMIC_OP_SET,
>>>>> +};
>>>>> +
>>>>> +/* drivers/power/pmic-uclass.c */
>>>>> +int power_init_dm(void);
>>>>> +struct udevice *pmic_get_by_name(int uclass_id, char *name);
>>>>> +struct udevice *pmic_get_by_interface(int uclass_id, int bus, int
>>>>> addr_cs);
>>>>> +const char *pmic_if_str(int interface);
>>>>> +int pmic_check_reg(struct pmic_platdata *p, unsigned reg);
>>>>> +int pmic_reg_write(struct udevice *dev, unsigned reg, unsigned val);
>>>>> +int pmic_reg_read(struct udevice *dev, unsigned reg, unsigned *val);
>>>>> +int pmic_probe(struct udevice *dev, struct spi_slave *spi_slave);
>>>>> +
>>>>> +/* drivers/power/pmic_i2c.c */
>>>>> +int pmic_i2c_reg_write(struct udevice *dev, unsigned reg, unsigned
>>>>> val);
>>>>> +int pmic_i2c_reg_read(struct udevice *dev, unsigned reg, unsigned
>>>>> *val);
>>>>> +int pmic_i2c_probe(struct udevice *dev);
>>>>> +
>>>>> +/* drivers/power/pmic_spi.c */
>>>>> +int pmic_spi_reg_write(struct udevice *dev, unsigned reg, unsigned
>>>>> val);
>>>>> +int pmic_spi_reg_read(struct udevice *dev, unsigned reg, unsigned
>>>>> *val);
>>>>> +struct spi_slave *pmic_spi_probe(struct udevice *dev);
>>>>> +#endif /* CONFIG_DM_PMIC */
>>>>> +
>>>>> +#ifdef CONFIG_POWER
>>>>> int pmic_init(unsigned char bus);
>>>>> int power_init_board(void);
>>>>> int pmic_dialog_init(unsigned char bus);
>>>>> @@ -88,6 +151,7 @@ int pmic_probe(struct pmic *p);
>>>>> int pmic_reg_read(struct pmic *p, u32 reg, u32 *val);
>>>>> int pmic_reg_write(struct pmic *p, u32 reg, u32 val);
>>>>> int pmic_set_output(struct pmic *p, u32 reg, int ldo, int on);
>>>>
>>>>
>>>>
>>>> These should have comments.
>>>>
>>> Ok, I will add it.
>>>
>>>>> +#endif
>>>>>
>>>>> #define pmic_i2c_addr (p->hw.i2c.addr)
>>>>> #define pmic_i2c_tx_num (p->hw.i2c.tx_num)
>>>>> --
>>>>> 1.9.1
>>>>>
>>>>
>>>> Regards,
>>>> Simon
>>>>
>>> Thank you for the fast review :)
>>
>>
>> Very interested to see this development.
>>
>> Regards,
>> Simon
>>
>
> Thank you again.
> I'm going to check the i2c-working tree and maybe rebase the dm-pmic onto
> it.
>
> Is it good idea?
Sounds good. Once I get the main DM patches landed (hopefully this
week) I'll rebase the other series including I2C. But for now you
should be good to use it.
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
