Hi Andy, On 06.07.2016 05:15, Andy Yan wrote: > Hi Wadim: > > On 2016年06月09日 16:23, Wadim Egorov wrote: >> Hi, >> >> On 08.06.2016 16:17, Lee Jones wrote: >>> On Thu, 02 Jun 2016, Wadim Egorov wrote: >>> >>>> The RK818 chip is a power management IC for multimedia and handheld >>> "Power Management IC (PMIC)" >>> >>>> devices. It contains the following components: >>>> >>>> - Regulators >>>> - RTC >>>> - Clkout >>> Clocking >>> >>>> - battery support >>> Battery support >>> >>>> Both chips RK808 and RK818 are using a similar register map. >>> "Both RK808 ad RK818 chips" >>> >>>> So we can reuse the RTC and Clkout functionality. >>> Swap '.' for ','. >>> >>>> Signed-off-by: Wadim Egorov <w.egorov@xxxxxxxxx> >>>> --- >>>> drivers/mfd/Kconfig | 4 +- >>>> drivers/mfd/rk808.c | 231 >>>> ++++++++++++++++++++++++++++++++++++++-------- >>>> include/linux/mfd/rk808.h | 162 ++++++++++++++++++++++++++++++-- >>>> 3 files changed, 350 insertions(+), 47 deletions(-) >>>> >>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >>>> index 1bcf601..7ba464b 100644 >>>> --- a/drivers/mfd/Kconfig >>>> +++ b/drivers/mfd/Kconfig >>>> @@ -839,13 +839,13 @@ config MFD_RC5T583 >>>> different functionality of the device. >>>> config MFD_RK808 >>>> - tristate "Rockchip RK808 Power Management chip" >>>> + tristate "Rockchip RK808/RK818 Power Management chip" >>> "Chip" >>> >>>> depends on I2C && OF >>>> select MFD_CORE >>>> select REGMAP_I2C >>>> select REGMAP_IRQ >>>> help >>>> - If you say yes here you get support for the RK808 >>>> + If you say yes here you get support for the RK808 and RK818 >>>> Power Management chips. >>>> This driver provides common support for accessing the device >>>> through I2C interface. The device supports multiple >>>> sub-devices >>>> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c >>>> index 49d7f62..3cf9724 100644 >>>> --- a/drivers/mfd/rk808.c >>>> +++ b/drivers/mfd/rk808.c >>>> @@ -1,11 +1,15 @@ >>>> /* >>>> - * MFD core driver for Rockchip RK808 >>>> + * MFD core driver for Rockchip RK808/RK818 >>>> * >>>> * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd >>>> * >>>> * Author: Chris Zhong <zyw@xxxxxxxxxxxxxx> >>>> * Author: Zhang Qing <zhangqing@xxxxxxxxxxxxxx> >>>> * >>>> + * Copyright (C) 2016 PHYTEC Messtechnik GmbH >>>> + * >>>> + * Author: Wadim Egorov <w.egorov@xxxxxxxxx> >>>> + * >>>> * This program is free software; you can redistribute it and/or >>>> modify it >>>> * under the terms and conditions of the GNU General Public License, >>>> * version 2, as published by the Free Software Foundation. >>>> @@ -22,12 +26,7 @@ >>>> #include <linux/mfd/core.h> >>>> #include <linux/module.h> >>>> #include <linux/regmap.h> >>>> - >>>> -struct rk808_reg_data { >>>> - int addr; >>>> - int mask; >>>> - int value; >>>> -}; >>> Why are you moving this to the header? >> It is now part of the rk808 struct. >> >>>> +#include <linux/of_device.h> >>>> static bool rk808_is_volatile_reg(struct device *dev, unsigned >>>> int reg) >>>> { >>>> @@ -57,6 +56,14 @@ static bool rk808_is_volatile_reg(struct device >>>> *dev, unsigned int reg) >>>> return false; >>>> } >>>> +static const struct regmap_config rk818_regmap_config = { >>>> + .reg_bits = 8, >>>> + .val_bits = 8, >>>> + .max_register = RK818_USB_CTRL_REG, >>>> + .cache_type = REGCACHE_RBTREE, >>>> + .volatile_reg = rk808_is_volatile_reg, >>>> +}; >>>> + >>>> static const struct regmap_config rk808_regmap_config = { >>>> .reg_bits = 8, >>>> .val_bits = 8, >>>> @@ -83,7 +90,17 @@ static const struct mfd_cell rk808s[] = { >>>> }, >>>> }; >>>> -static const struct rk808_reg_data pre_init_reg[] = { >>>> +static const struct mfd_cell rk818s[] = { >>>> + { .name = "rk808-clkout", }, >>> How does this differ to a normal -clock driver? >> I don't know. It is a normal clock driver. >> >>>> + { .name = "rk808-regulator", }, >>>> + { >>>> + .name = "rk808-rtc", >>>> + .num_resources = ARRAY_SIZE(rtc_resources), >>>> + .resources = &rtc_resources[0], >>> .resources = rtc_resources, ? >>> >>>> + }, >>>> +}; >>>> + >>>> +static const struct rk8xx_reg_data rk808_pre_init_reg[] = { >>>> { RK808_BUCK3_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_150MA }, >>>> { RK808_BUCK4_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_200MA }, >>>> { RK808_BOOST_CONFIG_REG, BOOST_ILMIN_MASK, BOOST_ILMIN_100MA }, >>>> @@ -94,6 +111,22 @@ static const struct rk808_reg_data >>>> pre_init_reg[] = { >>>> VB_LO_SEL_3500MV }, >>>> }; >>>> +static const struct rk8xx_reg_data rk818_pre_init_reg[] = { >>>> + { RK818_USB_CTRL_REG, RK818_USB_ILIM_SEL_MASK, >>>> + RK818_USB_ILMIN_2000MA }, >>>> + /* close charger when usb lower then 3.4V */ >>>> + { RK818_USB_CTRL_REG, RK818_USB_CHG_SD_VSEL_MASK, (0x7 << >>>> 4) }, >>>> + /* no action when vref */ >>>> + { RK818_H5V_EN_REG, BIT(1), RK818_REF_RDY_CTRL }, >>>> + /* enable HDMI 5V */ >>>> + { RK818_H5V_EN_REG, BIT(0), RK818_H5V_EN }, >>>> + /* improve efficiency */ >>>> + { RK818_BUCK2_CONFIG_REG, BUCK2_RATE_MASK, BUCK_ILMIN_250MA }, >>>> + { RK818_BUCK4_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_250MA }, >>>> + { RK818_BOOST_CONFIG_REG, BOOST_ILMIN_MASK, >>>> BOOST_ILMIN_100MA }, >>>> + { RK808_VB_MON_REG, MASK_ALL, VB_LO_ACT | >>>> VB_LO_SEL_3500MV }, >>>> +}; >>> The alignment here looks odd. >> I will fix it in the next version. >> >>>> static const struct regmap_irq rk808_irqs[] = { >>>> /* INT_STS */ >>>> [RK808_IRQ_VOUT_LO] = { >>>> @@ -136,6 +169,76 @@ static const struct regmap_irq rk808_irqs[] = { >>>> }, >>>> }; >>>> +static const struct regmap_irq rk818_irqs[] = { >>>> + /* INT_STS */ >>>> + [RK818_IRQ_VOUT_LO] = { >>>> + .mask = RK818_IRQ_VOUT_LO_MSK, >>>> + .reg_offset = 0, >>>> + }, >>>> + [RK818_IRQ_VB_LO] = { >>>> + .mask = RK818_IRQ_VB_LO_MSK, >>>> + .reg_offset = 0, >>>> + }, >>>> + [RK818_IRQ_PWRON] = { >>>> + .mask = RK818_IRQ_PWRON_MSK, >>>> + .reg_offset = 0, >>>> + }, >>>> + [RK818_IRQ_PWRON_LP] = { >>>> + .mask = RK818_IRQ_PWRON_LP_MSK, >>>> + .reg_offset = 0, >>>> + }, >>>> + [RK818_IRQ_HOTDIE] = { >>>> + .mask = RK818_IRQ_HOTDIE_MSK, >>>> + .reg_offset = 0, >>>> + }, >>>> + [RK818_IRQ_RTC_ALARM] = { >>>> + .mask = RK818_IRQ_RTC_ALARM_MSK, >>>> + .reg_offset = 0, >>>> + }, >>>> + [RK818_IRQ_RTC_PERIOD] = { >>>> + .mask = RK818_IRQ_RTC_PERIOD_MSK, >>>> + .reg_offset = 0, >>>> + }, >>>> + [RK818_IRQ_USB_OV] = { >>>> + .mask = RK818_IRQ_USB_OV_MSK, >>>> + .reg_offset = 0, >>>> + }, >>>> + >>>> + /* INT_STS2 */ >>>> + [RK818_IRQ_PLUG_IN] = { >>>> + .mask = RK818_IRQ_PLUG_IN_MSK, >>>> + .reg_offset = 1, >>>> + }, >>>> + [RK818_IRQ_PLUG_OUT] = { >>>> + .mask = RK818_IRQ_PLUG_OUT_MSK, >>>> + .reg_offset = 1, >>>> + }, >>>> + [RK818_IRQ_CHG_OK] = { >>>> + .mask = RK818_IRQ_CHG_OK_MSK, >>>> + .reg_offset = 1, >>>> + }, >>>> + [RK818_IRQ_CHG_TE] = { >>>> + .mask = RK818_IRQ_CHG_TE_MSK, >>>> + .reg_offset = 1, >>>> + }, >>>> + [RK818_IRQ_CHG_TS1] = { >>>> + .mask = RK818_IRQ_CHG_TS1_MSK, >>>> + .reg_offset = 1, >>>> + }, >>>> + [RK818_IRQ_TS2] = { >>>> + .mask = RK818_IRQ_TS2_MSK, >>>> + .reg_offset = 1, >>>> + }, >>>> + [RK818_IRQ_CHG_CVTLIM] = { >>>> + .mask = RK818_IRQ_CHG_CVTLIM_MSK, >>>> + .reg_offset = 1, >>>> + }, >>>> + [RK818_IRQ_DISCHG_ILIM] = { >>>> + .mask = RK818_IRQ_DISCHG_ILIM_MSK, >>>> + .reg_offset = 1, >>>> + }, >>>> +}; >>>> + >>>> static struct regmap_irq_chip rk808_irq_chip = { >>>> .name = "rk808", >>>> .irqs = rk808_irqs, >>>> @@ -148,6 +251,18 @@ static struct regmap_irq_chip rk808_irq_chip = { >>>> .init_ack_masked = true, >>>> }; >>>> +static struct regmap_irq_chip rk818_irq_chip = { >>>> + .name = "rk818", >>>> + .irqs = rk818_irqs, >>>> + .num_irqs = ARRAY_SIZE(rk818_irqs), >>>> + .num_regs = 2, >>>> + .irq_reg_stride = 2, >>>> + .status_base = RK818_INT_STS_REG1, >>>> + .mask_base = RK818_INT_STS_MSK_REG1, >>>> + .ack_base = RK818_INT_STS_REG1, >>>> + .init_ack_masked = true, >>>> +}; >>>> + >>>> static struct i2c_client *rk808_i2c_client; >>>> static void rk808_device_shutdown(void) >>>> { >>>> @@ -167,6 +282,48 @@ static void rk808_device_shutdown(void) >>>> dev_err(&rk808_i2c_client->dev, "power off error!\n"); >>>> } >>>> +static const struct of_device_id rk808_of_match[] = { >>>> + { .compatible = "rockchip,rk808", .data = (void *) RK808_ID }, >>>> + { .compatible = "rockchip,rk818", .data = (void *) RK818_ID }, >>>> + { }, >>>> +}; >>>> +MODULE_DEVICE_TABLE(of, rk808_of_match); >>>> + >>>> +static int rk8xx_match_device(struct rk808 *rk808, struct device >>>> *dev) >>> If you're going to do it this way, please switch these parameters. >>> >>> It's more common to return the pointer however. >> If it's more common I will return a pointer to rk808. >> >>>> +{ >>>> + const struct of_device_id *of_id; >>>> + >>>> + of_id = of_match_device(rk808_of_match, dev); >>>> + if (!of_id) { >>>> + dev_err(dev, "Unable to match OF ID\n"); >>>> + return -ENODEV; >>>> + } >>>> + rk808->variant = (long) of_id->data; >>>> + >>>> + switch (rk808->variant) { >>>> + case RK808_ID: >>>> + rk808->nr_cells = ARRAY_SIZE(rk808s); >>>> + rk808->cells = rk808s; >>>> + rk808->regmap_cfg = &rk808_regmap_config; >>>> + rk808->regmap_irq_chip = &rk808_irq_chip; >>>> + rk808->pre_init_reg = rk808_pre_init_reg; >>>> + rk808->nr_pre_init_regs = ARRAY_SIZE(rk808_pre_init_reg); >>>> + break; >>>> + case RK818_ID: >>>> + rk808->nr_cells = ARRAY_SIZE(rk818s); >>>> + rk808->cells = rk818s; >>>> + rk808->regmap_cfg = &rk818_regmap_config; >>>> + rk808->regmap_irq_chip = &rk818_irq_chip; >>>> + rk808->pre_init_reg = rk818_pre_init_reg; >>>> + rk808->nr_pre_init_regs = ARRAY_SIZE(rk818_pre_init_reg); >>>> + break; >>>> + default: >>>> + dev_err(dev, "unsupported RK8XX ID %lu\n", rk808->variant); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> static int rk808_probe(struct i2c_client *client, >>>> const struct i2c_device_id *id) >>>> { >>>> @@ -176,46 +333,52 @@ static int rk808_probe(struct i2c_client >>>> *client, >>>> int ret; >>>> int i; >>>> - if (!client->irq) { >>>> - dev_err(&client->dev, "No interrupt support, no core IRQ\n"); >>>> - return -EINVAL; >>>> - } >>>> - >>>> rk808 = devm_kzalloc(&client->dev, sizeof(*rk808), GFP_KERNEL); >>>> if (!rk808) >>>> return -ENOMEM; >>>> - rk808->regmap = devm_regmap_init_i2c(client, >>>> &rk808_regmap_config); >>>> + ret = rk8xx_match_device(rk808, &client->dev); >>> Is there a way to dynamically probe the device? No device ID you can >>> read directly from the silicon? >> AFAIK there is no device ID register. At least it is not documented in >> the manual. > > register 0x17 and 0x18 is the ID register, 0x17 is the MSB, 0x18 is LSB > > for RK818, register 0x17 is 0x81, 0x18 is 0x81 thank you for sharing this information. I have no RK808 PMIC device here, so I would also need the IDs for RK808. Lee, you have already applied this series. But I can't find the patches in your kernel tree. I would like to read the device ID from the register in the probe function. Do you want me to base my changes on top of this series or send a new version? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html