From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Mon, 20 Oct 2014 17:44:52 +0200 Subject: [U-Boot] [PATCH 04/19] dm: pmic: add implementation of driver model pmic uclass In-Reply-To: References: <1412801335-1591-1-git-send-email-p.marczak@samsung.com> <1412801335-1591-5-git-send-email-p.marczak@samsung.com> <5437DFDA.2040407@samsung.com> Message-ID: <54452DF4.10006@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 wrote: >> Hello Simon, >> >> >> On 10/10/2014 05:17 AM, Simon Glass wrote: >>> >>> Hi, >>> >>> On 8 October 2014 14:48, Przemyslaw Marczak 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 >>>> --- >>>> 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 >>>> + * >>>> + * SPDX-License-Identifier: GPL-2.0+ >>>> + */ >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> + >>>> +DECLARE_GLOBAL_DATA_PTR; >>>> + >>>> +int pmic_check_reg(struct pmic_platdata *p, unsigned reg) >>>> +{ >>>> + if (!p) >>>> + return -ENODEV; >>>> + >>>> + if (reg >= p->regs_num) { >>>> + error(" = %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 >>>> + * >>>> + * Copyright (C) 2011 Samsung Electronics >>>> + * Lukasz Majewski >>>> + * >>>> + * (C) Copyright 2010 >>>> + * Stefano Babic, DENX Software Engineering, sbabic at denx.de >>>> + * >>>> + * (C) Copyright 2008-2009 Freescale Semiconductor, Inc. >>>> + * >>>> + * SPDX-License-Identifier: GPL-2.0+ >>>> + */ >>>> + >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> + >>>> +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 >>>> + * >>>> + * Copyright (C) 2011 Samsung Electronics >>>> + * Lukasz Majewski >>>> + * >>>> + * (C) Copyright 2010 >>>> + * Stefano Babic, DENX Software Engineering, sbabic at denx.de >>>> + * >>>> + * (C) Copyright 2008-2009 Freescale Semiconductor, Inc. >>>> + * >>>> + * SPDX-License-Identifier: GPL-2.0+ >>>> + */ >>>> + >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> + >>>> +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 >>>> + * >>>> * Copyright (C) 2011-2012 Samsung Electronics >>>> * Lukasz Majewski >>>> * >>>> @@ -8,9 +11,12 @@ >>>> #ifndef __CORE_PMIC_H_ >>>> #define __CORE_PMIC_H_ >>>> >>>> +#include >>>> #include >>>> +#include >>>> #include >>>> #include >>>> +#include >>>> >>>> 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? Best Regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com