Re: [RESEND PATCH v5 1/5] mfd: RK808: Add RK818 support
From: Andy Yan
Date: Wed Jul 06 2016 - 04:57:17 EST
Hi Wadim:
On 2016å07æ06æ 16:03, Wadim Egorov wrote:
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.
I have checked with the RK808 IC designer, the values of register
0x17 and 0x18 are both 0x00.
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?