* [PATCH v4 1/8] mfd: rt5033: Drop rt5033-battery sub-device
2023-05-06 15:54 ` [PATCH v4 0/8] Add RT5033 charger device driver Jakob Hauser
@ 2023-05-06 15:54 ` Jakob Hauser
2023-05-06 15:54 ` [PATCH v4 2/8] mfd: rt5033: Fix chip revision readout Jakob Hauser
` (6 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Jakob Hauser @ 2023-05-06 15:54 UTC (permalink / raw)
To: Sebastian Reichel, Lee Jones, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski
Cc: Beomho Seo, Chanwoo Choi, Stephan Gerhold, Raymond Hackley,
Pavel Machek, Axel Lin, ChiYuan Huang, Linus Walleij,
Henrik Grimler, linux-pm, devicetree, linux-kernel, phone-devel,
~postmarketos/upstreaming
From: Stephan Gerhold <stephan@gerhold.net>
The fuel gauge in the RT5033 PMIC (rt5033-battery) has its own I2C bus
and interrupt lines. Therefore, it is not part of the MFD device
and needs to be specified separately in the device tree.
Cc: Beomho Seo <beomho.seo@samsung.com>
Cc: Chanwoo Choi <cw00.choi@samsung.com>
Fixes: 0b271258544b ("mfd: rt5033: Add Richtek RT5033 driver core.")
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mfd/rt5033.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/mfd/rt5033.c b/drivers/mfd/rt5033.c
index a5e520fe50a1..8029d444b794 100644
--- a/drivers/mfd/rt5033.c
+++ b/drivers/mfd/rt5033.c
@@ -40,9 +40,6 @@ static const struct mfd_cell rt5033_devs[] = {
{
.name = "rt5033-charger",
.of_compatible = "richtek,rt5033-charger",
- }, {
- .name = "rt5033-battery",
- .of_compatible = "richtek,rt5033-battery",
}, {
.name = "rt5033-led",
.of_compatible = "richtek,rt5033-led",
--
2.39.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 2/8] mfd: rt5033: Fix chip revision readout
2023-05-06 15:54 ` [PATCH v4 0/8] Add RT5033 charger device driver Jakob Hauser
2023-05-06 15:54 ` [PATCH v4 1/8] mfd: rt5033: Drop rt5033-battery sub-device Jakob Hauser
@ 2023-05-06 15:54 ` Jakob Hauser
2023-05-06 15:54 ` [PATCH v4 3/8] mfd: rt5033: Fix STAT_MASK, HZ_MASK and AICR defines Jakob Hauser
` (5 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Jakob Hauser @ 2023-05-06 15:54 UTC (permalink / raw)
To: Sebastian Reichel, Lee Jones, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski
Cc: Beomho Seo, Chanwoo Choi, Stephan Gerhold, Raymond Hackley,
Pavel Machek, Axel Lin, ChiYuan Huang, Linus Walleij,
Henrik Grimler, linux-pm, devicetree, linux-kernel, phone-devel,
~postmarketos/upstreaming
After reading the data from the DEVICE_ID register, mask 0x0f needs to be
applied to extract the revision of the chip [1].
The other part of the DEVICE_ID register, mask 0xf0, is a vendor identification
code. That's how it is set up at similar products of Richtek, e.g. RT9455 [2]
page 21 top.
[1] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/mfd/rt5033_core.c#L484
[2] https://www.richtek.com/assets/product_file/RT9455/DS9455-00.pdf
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
drivers/mfd/rt5033.c | 5 +++--
include/linux/mfd/rt5033-private.h | 4 ++++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/rt5033.c b/drivers/mfd/rt5033.c
index 8029d444b794..3eee4242ee02 100644
--- a/drivers/mfd/rt5033.c
+++ b/drivers/mfd/rt5033.c
@@ -55,7 +55,7 @@ static const struct regmap_config rt5033_regmap_config = {
static int rt5033_i2c_probe(struct i2c_client *i2c)
{
struct rt5033_dev *rt5033;
- unsigned int dev_id;
+ unsigned int dev_id, chip_rev;
int ret;
rt5033 = devm_kzalloc(&i2c->dev, sizeof(*rt5033), GFP_KERNEL);
@@ -78,7 +78,8 @@ static int rt5033_i2c_probe(struct i2c_client *i2c)
dev_err(&i2c->dev, "Device not found\n");
return -ENODEV;
}
- dev_info(&i2c->dev, "Device found Device ID: %04x\n", dev_id);
+ chip_rev = dev_id & RT5033_CHIP_REV_MASK;
+ dev_info(&i2c->dev, "Device found (rev. %d)\n", chip_rev);
ret = regmap_add_irq_chip(rt5033->regmap, rt5033->irq,
IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
diff --git a/include/linux/mfd/rt5033-private.h b/include/linux/mfd/rt5033-private.h
index 6bb432f6a96c..b035a67cec73 100644
--- a/include/linux/mfd/rt5033-private.h
+++ b/include/linux/mfd/rt5033-private.h
@@ -71,6 +71,10 @@ enum rt5033_reg {
/* RT5033 CHGCTRL2 register */
#define RT5033_CHGCTRL2_CV_MASK 0xfc
+/* RT5033 DEVICE_ID register */
+#define RT5033_VENDOR_ID_MASK 0xf0
+#define RT5033_CHIP_REV_MASK 0x0f
+
/* RT5033 CHGCTRL3 register */
#define RT5033_CHGCTRL3_CFO_EN_MASK 0x40
#define RT5033_CHGCTRL3_TIMER_MASK 0x38
--
2.39.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 3/8] mfd: rt5033: Fix STAT_MASK, HZ_MASK and AICR defines
2023-05-06 15:54 ` [PATCH v4 0/8] Add RT5033 charger device driver Jakob Hauser
2023-05-06 15:54 ` [PATCH v4 1/8] mfd: rt5033: Drop rt5033-battery sub-device Jakob Hauser
2023-05-06 15:54 ` [PATCH v4 2/8] mfd: rt5033: Fix chip revision readout Jakob Hauser
@ 2023-05-06 15:54 ` Jakob Hauser
2023-05-06 15:54 ` [PATCH v4 4/8] mfd: rt5033: Apply preparatory changes before adding rt5033-charger driver Jakob Hauser
` (4 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Jakob Hauser @ 2023-05-06 15:54 UTC (permalink / raw)
To: Sebastian Reichel, Lee Jones, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski
Cc: Beomho Seo, Chanwoo Choi, Stephan Gerhold, Raymond Hackley,
Pavel Machek, Axel Lin, ChiYuan Huang, Linus Walleij,
Henrik Grimler, linux-pm, devicetree, linux-kernel, phone-devel,
~postmarketos/upstreaming
The charger state mask RT5033_CHG_STAT_MASK should be 0x30 [1][2].
The high impedance mask RT5033_RT_HZ_MASK is actually value 0x02 [3] and is
assosiated to the RT5033 CHGCTRL1 register [4]. Accordingly also change
RT5033_CHARGER_HZ_ENABLE to 0x02 to avoid the need of a bit shift upon
application.
For input current limiting AICR mode, the define for the 1000 mA step was
missing [5]. Additionally add the define for DISABLE option. Concerning the
mask, remove RT5033_AICR_MODE_MASK because there is already
RT5033_CHGCTRL1_IAICR_MASK further up. They are redundant and the upper one
makes more sense to have the masks of a register colleted there as an
overview.
[1] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L669-L682
[2] https://github.com/torvalds/linux/blob/v6.0/include/linux/mfd/rt5033-private.h#L59-L62
[3] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/include/linux/battery/charger/rt5033_charger.h#L44
[4] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L223
[5] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L278
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
Acked-for-MFD-by: Lee Jones <lee@kernel.org>
---
include/linux/mfd/rt5033-private.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/linux/mfd/rt5033-private.h b/include/linux/mfd/rt5033-private.h
index b035a67cec73..b6773ebf4e6b 100644
--- a/include/linux/mfd/rt5033-private.h
+++ b/include/linux/mfd/rt5033-private.h
@@ -55,7 +55,7 @@ enum rt5033_reg {
};
/* RT5033 Charger state register */
-#define RT5033_CHG_STAT_MASK 0x20
+#define RT5033_CHG_STAT_MASK 0x30
#define RT5033_CHG_STAT_DISCHARGING 0x00
#define RT5033_CHG_STAT_FULL 0x10
#define RT5033_CHG_STAT_CHARGING 0x20
@@ -67,6 +67,7 @@ enum rt5033_reg {
/* RT5033 CHGCTRL1 register */
#define RT5033_CHGCTRL1_IAICR_MASK 0xe0
#define RT5033_CHGCTRL1_MODE_MASK 0x01
+#define RT5033_CHGCTRL1_HZ_MASK 0x02
/* RT5033 CHGCTRL2 register */
#define RT5033_CHGCTRL2_CV_MASK 0xfc
@@ -92,7 +93,6 @@ enum rt5033_reg {
/* RT5033 RT CTRL1 register */
#define RT5033_RT_CTRL1_UUG_MASK 0x02
-#define RT5033_RT_HZ_MASK 0x01
/* RT5033 control register */
#define RT5033_CTRL_FCCM_BUCK_MASK BIT(0)
@@ -119,13 +119,14 @@ enum rt5033_reg {
* register), AICR mode limits the input current. For example, the AIRC 100
* mode limits the input current to 100 mA.
*/
+#define RT5033_AICR_DISABLE 0x00
#define RT5033_AICR_100_MODE 0x20
#define RT5033_AICR_500_MODE 0x40
#define RT5033_AICR_700_MODE 0x60
#define RT5033_AICR_900_MODE 0x80
+#define RT5033_AICR_1000_MODE 0xa0
#define RT5033_AICR_1500_MODE 0xc0
#define RT5033_AICR_2000_MODE 0xe0
-#define RT5033_AICR_MODE_MASK 0xe0
/* RT5033 use internal timer need to set time */
#define RT5033_FAST_CHARGE_TIMER4 0x00
@@ -195,7 +196,7 @@ enum rt5033_reg {
/* RT5033 charger high impedance mode */
#define RT5033_CHARGER_HZ_DISABLE 0x00
-#define RT5033_CHARGER_HZ_ENABLE 0x01
+#define RT5033_CHARGER_HZ_ENABLE 0x02
/* RT5033 regulator BUCK output voltage uV */
#define RT5033_REGULATOR_BUCK_VOLTAGE_MIN 1000000U
--
2.39.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 4/8] mfd: rt5033: Apply preparatory changes before adding rt5033-charger driver
2023-05-06 15:54 ` [PATCH v4 0/8] Add RT5033 charger device driver Jakob Hauser
` (2 preceding siblings ...)
2023-05-06 15:54 ` [PATCH v4 3/8] mfd: rt5033: Fix STAT_MASK, HZ_MASK and AICR defines Jakob Hauser
@ 2023-05-06 15:54 ` Jakob Hauser
2023-05-06 15:54 ` [PATCH v4 5/8] power: supply: rt5033_charger: Add RT5033 charger device driver Jakob Hauser
` (3 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Jakob Hauser @ 2023-05-06 15:54 UTC (permalink / raw)
To: Sebastian Reichel, Lee Jones, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski
Cc: Beomho Seo, Chanwoo Choi, Stephan Gerhold, Raymond Hackley,
Pavel Machek, Axel Lin, ChiYuan Huang, Linus Walleij,
Henrik Grimler, linux-pm, devicetree, linux-kernel, phone-devel,
~postmarketos/upstreaming
Order the register blocks to have the masks in descending manner.
Add new defines for constant voltage shift (RT5033_CHGCTRL2_CV_SHIFT),
MIVR mask (RT5033_CHGCTRL4_MIVR_MASK), pre-charge current shift
(RT5033_CHGCTRL4_IPREC_SHIFT), internal timer disable
(RT5033_INT_TIMER_DISABLE), termination disable (RT5033_TE_DISABLE),
CFO disable (RT5033_CFO_DISABLE), UUG disable (RT5033_CHARGER_UUG_DISABLE).
The fast charge timer type needs to be written on mask 0x38
(RT5033_CHGCTRL3_TIMER_MASK). To avoid a bit shift on application, change the
values of the timer types to fit the mask. Added the timout duration as a
comment. And the timer between TIMER8 and TIMER12 is most likely TIMER10, see
e.g. RT5036 [1] page 28 bottom.
Add value options for MIVR (Minimum Input Voltage Regulation).
Move RT5033_TE_ENABLE_MASK to the block "RT5033 CHGCTRL1 register", in order
to have the masks of the register collected there. To fit the naming scheme,
rename it to RT5033_CHGCTRL1_TE_EN_MASK.
Move RT5033_CHG_MAX_CURRENT to the block "RT5033 charger fast-charge current".
Add new defines RT5033_CV_MAX_VOLTAGE and RT5033_CHG_MAX_PRE_CURRENT to the
blocks "RT5033 charger constant charge voltage" and "RT5033 charger pre-charge
current limits".
In include/linux/mfd/rt5033.h, turn power_supply "psy" into a pointer in order
to use it in devm_power_supply_register().
[1] https://media.digikey.com/pdf/Data%20Sheets/Richtek%20PDF/RT5036%20%20Preliminary.pdf
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
include/linux/mfd/rt5033-private.h | 53 ++++++++++++++++++++----------
include/linux/mfd/rt5033.h | 2 +-
2 files changed, 36 insertions(+), 19 deletions(-)
diff --git a/include/linux/mfd/rt5033-private.h b/include/linux/mfd/rt5033-private.h
index b6773ebf4e6b..0221f806d139 100644
--- a/include/linux/mfd/rt5033-private.h
+++ b/include/linux/mfd/rt5033-private.h
@@ -55,22 +55,24 @@ enum rt5033_reg {
};
/* RT5033 Charger state register */
+#define RT5033_CHG_STAT_TYPE_MASK 0x60
+#define RT5033_CHG_STAT_TYPE_PRE 0x20
+#define RT5033_CHG_STAT_TYPE_FAST 0x60
#define RT5033_CHG_STAT_MASK 0x30
#define RT5033_CHG_STAT_DISCHARGING 0x00
#define RT5033_CHG_STAT_FULL 0x10
#define RT5033_CHG_STAT_CHARGING 0x20
#define RT5033_CHG_STAT_NOT_CHARGING 0x30
-#define RT5033_CHG_STAT_TYPE_MASK 0x60
-#define RT5033_CHG_STAT_TYPE_PRE 0x20
-#define RT5033_CHG_STAT_TYPE_FAST 0x60
/* RT5033 CHGCTRL1 register */
#define RT5033_CHGCTRL1_IAICR_MASK 0xe0
-#define RT5033_CHGCTRL1_MODE_MASK 0x01
+#define RT5033_CHGCTRL1_TE_EN_MASK 0x08
#define RT5033_CHGCTRL1_HZ_MASK 0x02
+#define RT5033_CHGCTRL1_MODE_MASK 0x01
/* RT5033 CHGCTRL2 register */
#define RT5033_CHGCTRL2_CV_MASK 0xfc
+#define RT5033_CHGCTRL2_CV_SHIFT 0x02
/* RT5033 DEVICE_ID register */
#define RT5033_VENDOR_ID_MASK 0xf0
@@ -82,14 +84,15 @@ enum rt5033_reg {
#define RT5033_CHGCTRL3_TIMER_EN_MASK 0x01
/* RT5033 CHGCTRL4 register */
-#define RT5033_CHGCTRL4_EOC_MASK 0x07
+#define RT5033_CHGCTRL4_MIVR_MASK 0xe0
#define RT5033_CHGCTRL4_IPREC_MASK 0x18
+#define RT5033_CHGCTRL4_IPREC_SHIFT 0x03
+#define RT5033_CHGCTRL4_EOC_MASK 0x07
/* RT5033 CHGCTRL5 register */
-#define RT5033_CHGCTRL5_VPREC_MASK 0x0f
#define RT5033_CHGCTRL5_ICHG_MASK 0xf0
#define RT5033_CHGCTRL5_ICHG_SHIFT 0x04
-#define RT5033_CHG_MAX_CURRENT 0x0d
+#define RT5033_CHGCTRL5_VPREC_MASK 0x0f
/* RT5033 RT CTRL1 register */
#define RT5033_RT_CTRL1_UUG_MASK 0x02
@@ -128,20 +131,28 @@ enum rt5033_reg {
#define RT5033_AICR_1500_MODE 0xc0
#define RT5033_AICR_2000_MODE 0xe0
-/* RT5033 use internal timer need to set time */
-#define RT5033_FAST_CHARGE_TIMER4 0x00
-#define RT5033_FAST_CHARGE_TIMER6 0x01
-#define RT5033_FAST_CHARGE_TIMER8 0x02
-#define RT5033_FAST_CHARGE_TIMER9 0x03
-#define RT5033_FAST_CHARGE_TIMER12 0x04
-#define RT5033_FAST_CHARGE_TIMER14 0x05
-#define RT5033_FAST_CHARGE_TIMER16 0x06
+/* RT5033 charger minimum input voltage regulation */
+#define RT5033_CHARGER_MIVR_DISABLE 0x00
+#define RT5033_CHARGER_MIVR_4200MV 0x20
+#define RT5033_CHARGER_MIVR_4300MV 0x40
+#define RT5033_CHARGER_MIVR_4400MV 0x60
+#define RT5033_CHARGER_MIVR_4500MV 0x80
+#define RT5033_CHARGER_MIVR_4600MV 0xa0
+#define RT5033_CHARGER_MIVR_4700MV 0xc0
+#define RT5033_CHARGER_MIVR_4800MV 0xe0
+/* RT5033 use internal timer need to set time */
+#define RT5033_FAST_CHARGE_TIMER4 0x00 /* 4 hrs */
+#define RT5033_FAST_CHARGE_TIMER6 0x08 /* 6 hrs */
+#define RT5033_FAST_CHARGE_TIMER8 0x10 /* 8 hrs */
+#define RT5033_FAST_CHARGE_TIMER10 0x18 /* 10 hrs */
+#define RT5033_FAST_CHARGE_TIMER12 0x20 /* 12 hrs */
+#define RT5033_FAST_CHARGE_TIMER14 0x28 /* 14 hrs */
+#define RT5033_FAST_CHARGE_TIMER16 0x30 /* 16 hrs */
+
+#define RT5033_INT_TIMER_DISABLE 0x00
#define RT5033_INT_TIMER_ENABLE 0x01
-/* RT5033 charger termination enable mask */
-#define RT5033_TE_ENABLE_MASK 0x08
-
/*
* RT5033 charger opa mode. RT5033 has two opa modes for OTG: charger mode
* and boost mode.
@@ -150,25 +161,30 @@ enum rt5033_reg {
#define RT5033_BOOST_MODE 0x01
/* RT5033 charger termination enable */
+#define RT5033_TE_DISABLE 0x00
#define RT5033_TE_ENABLE 0x08
/* RT5033 charger CFO enable */
+#define RT5033_CFO_DISABLE 0x00
#define RT5033_CFO_ENABLE 0x40
/* RT5033 charger constant charge voltage (as in CHGCTRL2 register), uV */
#define RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN 3650000U
#define RT5033_CHARGER_CONST_VOLTAGE_STEP_NUM 25000U
#define RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX 4400000U
+#define RT5033_CV_MAX_VOLTAGE 0x1e
/* RT5033 charger pre-charge current limits (as in CHGCTRL4 register), uA */
#define RT5033_CHARGER_PRE_CURRENT_LIMIT_MIN 350000U
#define RT5033_CHARGER_PRE_CURRENT_STEP_NUM 100000U
#define RT5033_CHARGER_PRE_CURRENT_LIMIT_MAX 650000U
+#define RT5033_CHG_MAX_PRE_CURRENT 0x03
/* RT5033 charger fast-charge current (as in CHGCTRL5 register), uA */
#define RT5033_CHARGER_FAST_CURRENT_MIN 700000U
#define RT5033_CHARGER_FAST_CURRENT_STEP_NUM 100000U
#define RT5033_CHARGER_FAST_CURRENT_MAX 2000000U
+#define RT5033_CHG_MAX_CURRENT 0x0d
/*
* RT5033 charger const-charge end of charger current (
@@ -192,6 +208,7 @@ enum rt5033_reg {
* RT5033 charger UUG. It enables MOS auto control by H/W charger
* circuit.
*/
+#define RT5033_CHARGER_UUG_DISABLE 0x00
#define RT5033_CHARGER_UUG_ENABLE 0x02
/* RT5033 charger high impedance mode */
diff --git a/include/linux/mfd/rt5033.h b/include/linux/mfd/rt5033.h
index 8f306ac15a27..e99e2ab0c1c1 100644
--- a/include/linux/mfd/rt5033.h
+++ b/include/linux/mfd/rt5033.h
@@ -51,7 +51,7 @@ struct rt5033_charger_data {
struct rt5033_charger {
struct device *dev;
struct rt5033_dev *rt5033;
- struct power_supply psy;
+ struct power_supply *psy;
struct rt5033_charger_data *chg;
};
--
2.39.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 5/8] power: supply: rt5033_charger: Add RT5033 charger device driver
2023-05-06 15:54 ` [PATCH v4 0/8] Add RT5033 charger device driver Jakob Hauser
` (3 preceding siblings ...)
2023-05-06 15:54 ` [PATCH v4 4/8] mfd: rt5033: Apply preparatory changes before adding rt5033-charger driver Jakob Hauser
@ 2023-05-06 15:54 ` Jakob Hauser
2023-05-08 6:50 ` Sebastian Reichel
2023-05-06 15:54 ` [PATCH v4 6/8] power: supply: rt5033_charger: Add cable detection and USB OTG supply Jakob Hauser
` (2 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Jakob Hauser @ 2023-05-06 15:54 UTC (permalink / raw)
To: Sebastian Reichel, Lee Jones, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski
Cc: Beomho Seo, Chanwoo Choi, Stephan Gerhold, Raymond Hackley,
Pavel Machek, Axel Lin, ChiYuan Huang, Linus Walleij,
Henrik Grimler, linux-pm, devicetree, linux-kernel, phone-devel,
~postmarketos/upstreaming
This patch adds device driver of Richtek RT5033 PMIC. The driver supports
switching charger. rt5033 charger provides three charging modes. The charging
modes are pre-charge mode, fast charge mode and constant voltage mode. They
vary in charge rate, the charge parameters can be controlled by i2c interface.
Cc: Beomho Seo <beomho.seo@samsung.com>
Cc: Chanwoo Choi <cw00.choi@samsung.com>
Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/Kconfig | 8 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/rt5033_charger.c | 464 ++++++++++++++++++++++++++
3 files changed, 473 insertions(+)
create mode 100644 drivers/power/supply/rt5033_charger.c
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index c78be9f322e6..ea11797670ca 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -766,6 +766,14 @@ config BATTERY_RT5033
The fuelgauge calculates and determines the battery state of charge
according to battery open circuit voltage.
+config CHARGER_RT5033
+ tristate "RT5033 battery charger support"
+ depends on MFD_RT5033
+ help
+ This adds support for battery charger in Richtek RT5033 PMIC.
+ The device supports pre-charge mode, fast charge mode and
+ constant voltage mode.
+
config CHARGER_RT9455
tristate "Richtek RT9455 battery charger driver"
depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 4adbfba02d05..dfc624bbcf1d 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_BATTERY_MAX17040) += max17040_battery.o
obj-$(CONFIG_BATTERY_MAX17042) += max17042_battery.o
obj-$(CONFIG_BATTERY_MAX1721X) += max1721x_battery.o
obj-$(CONFIG_BATTERY_RT5033) += rt5033_battery.o
+obj-$(CONFIG_CHARGER_RT5033) += rt5033_charger.o
obj-$(CONFIG_CHARGER_RT9455) += rt9455_charger.o
obj-$(CONFIG_CHARGER_RT9467) += rt9467-charger.o
obj-$(CONFIG_CHARGER_RT9471) += rt9471.o
diff --git a/drivers/power/supply/rt5033_charger.c b/drivers/power/supply/rt5033_charger.c
new file mode 100644
index 000000000000..038530d2f0a0
--- /dev/null
+++ b/drivers/power/supply/rt5033_charger.c
@@ -0,0 +1,464 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Battery charger driver for RT5033
+ *
+ * Copyright (C) 2014 Samsung Electronics, Co., Ltd.
+ * Author: Beomho Seo <beomho.seo@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/rt5033-private.h>
+#include <linux/mfd/rt5033.h>
+
+static int rt5033_get_charger_state(struct rt5033_charger *charger)
+{
+ struct regmap *regmap = charger->rt5033->regmap;
+ unsigned int reg_data;
+ int state;
+
+ if (!regmap)
+ return POWER_SUPPLY_STATUS_UNKNOWN;
+
+ regmap_read(regmap, RT5033_REG_CHG_STAT, ®_data);
+
+ switch (reg_data & RT5033_CHG_STAT_MASK) {
+ case RT5033_CHG_STAT_DISCHARGING:
+ state = POWER_SUPPLY_STATUS_DISCHARGING;
+ break;
+ case RT5033_CHG_STAT_CHARGING:
+ state = POWER_SUPPLY_STATUS_CHARGING;
+ break;
+ case RT5033_CHG_STAT_FULL:
+ state = POWER_SUPPLY_STATUS_FULL;
+ break;
+ case RT5033_CHG_STAT_NOT_CHARGING:
+ state = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ break;
+ default:
+ state = POWER_SUPPLY_STATUS_UNKNOWN;
+ }
+
+ return state;
+}
+
+static int rt5033_get_charger_type(struct rt5033_charger *charger)
+{
+ struct regmap *regmap = charger->rt5033->regmap;
+ unsigned int reg_data;
+ int state;
+
+ regmap_read(regmap, RT5033_REG_CHG_STAT, ®_data);
+
+ switch (reg_data & RT5033_CHG_STAT_TYPE_MASK) {
+ case RT5033_CHG_STAT_TYPE_FAST:
+ state = POWER_SUPPLY_CHARGE_TYPE_FAST;
+ break;
+ case RT5033_CHG_STAT_TYPE_PRE:
+ state = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+ break;
+ default:
+ state = POWER_SUPPLY_CHARGE_TYPE_NONE;
+ }
+
+ return state;
+}
+
+static int rt5033_get_charger_current_limit(struct rt5033_charger *charger)
+{
+ struct regmap *regmap = charger->rt5033->regmap;
+ unsigned int state, reg_data, data;
+
+ regmap_read(regmap, RT5033_REG_CHG_CTRL5, ®_data);
+
+ state = (reg_data & RT5033_CHGCTRL5_ICHG_MASK)
+ >> RT5033_CHGCTRL5_ICHG_SHIFT;
+
+ data = RT5033_CHARGER_FAST_CURRENT_MIN +
+ RT5033_CHARGER_FAST_CURRENT_STEP_NUM * state;
+
+ return data;
+}
+
+static int rt5033_get_charger_const_voltage(struct rt5033_charger *charger)
+{
+ struct regmap *regmap = charger->rt5033->regmap;
+ unsigned int state, reg_data, data;
+
+ regmap_read(regmap, RT5033_REG_CHG_CTRL2, ®_data);
+
+ state = (reg_data & RT5033_CHGCTRL2_CV_MASK)
+ >> RT5033_CHGCTRL2_CV_SHIFT;
+
+ data = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN +
+ RT5033_CHARGER_CONST_VOLTAGE_STEP_NUM * state;
+
+ return data;
+}
+
+static inline int rt5033_init_const_charge(struct rt5033_charger *charger)
+{
+ struct rt5033_charger_data *chg = charger->chg;
+ int ret;
+ unsigned int val;
+ u8 reg_data;
+
+ /* Set constant voltage mode */
+ if (chg->const_uvolt < RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN ||
+ chg->const_uvolt > RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX) {
+ dev_err(charger->dev,
+ "Value 'constant-charge-voltage-max-microvolt' out of range\n");
+ return -EINVAL;
+ }
+
+ if (chg->const_uvolt == RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN)
+ reg_data = 0x00;
+ else if (chg->const_uvolt == RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX)
+ reg_data = RT5033_CV_MAX_VOLTAGE;
+ else {
+ val = chg->const_uvolt;
+ val -= RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN;
+ val /= RT5033_CHARGER_CONST_VOLTAGE_STEP_NUM;
+ reg_data = val;
+ }
+
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL2,
+ RT5033_CHGCTRL2_CV_MASK,
+ reg_data << RT5033_CHGCTRL2_CV_SHIFT);
+ if (ret) {
+ dev_err(charger->dev, "Failed regmap update\n");
+ return -EINVAL;
+ }
+
+ /* Set end of charge current */
+ if (chg->eoc_uamp < RT5033_CHARGER_EOC_MIN ||
+ chg->eoc_uamp > RT5033_CHARGER_EOC_MAX) {
+ dev_err(charger->dev,
+ "Value 'charge-term-current-microamp' out of range\n");
+ return -EINVAL;
+ }
+
+ if (chg->eoc_uamp == RT5033_CHARGER_EOC_MIN)
+ reg_data = 0x01;
+ else if (chg->eoc_uamp == RT5033_CHARGER_EOC_MAX)
+ reg_data = 0x07;
+ else {
+ val = chg->eoc_uamp;
+ if (val < RT5033_CHARGER_EOC_REF) {
+ val -= RT5033_CHARGER_EOC_MIN;
+ val /= RT5033_CHARGER_EOC_STEP_NUM1;
+ reg_data = 0x01 + val;
+ } else if (val > RT5033_CHARGER_EOC_REF) {
+ val -= RT5033_CHARGER_EOC_REF;
+ val /= RT5033_CHARGER_EOC_STEP_NUM2;
+ reg_data = 0x04 + val;
+ } else {
+ reg_data = 0x04;
+ }
+ }
+
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL4,
+ RT5033_CHGCTRL4_EOC_MASK, reg_data);
+ if (ret) {
+ dev_err(charger->dev, "Failed regmap update\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static inline int rt5033_init_fast_charge(struct rt5033_charger *charger)
+{
+ struct rt5033_charger_data *chg = charger->chg;
+ int ret;
+ unsigned int val;
+ u8 reg_data;
+
+ /* Set limit input current */
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL1,
+ RT5033_CHGCTRL1_IAICR_MASK, RT5033_AICR_2000_MODE);
+ if (ret) {
+ dev_err(charger->dev, "Failed regmap update\n");
+ return -EINVAL;
+ }
+
+ /* Set fast-charge mode charging current */
+ if (chg->fast_uamp < RT5033_CHARGER_FAST_CURRENT_MIN ||
+ chg->fast_uamp > RT5033_CHARGER_FAST_CURRENT_MAX) {
+ dev_err(charger->dev,
+ "Value 'constant-charge-current-max-microamp' out of range\n");
+ return -EINVAL;
+ }
+
+ if (chg->fast_uamp == RT5033_CHARGER_FAST_CURRENT_MIN)
+ reg_data = 0x00;
+ else if (chg->fast_uamp == RT5033_CHARGER_FAST_CURRENT_MAX)
+ reg_data = RT5033_CHG_MAX_CURRENT;
+ else {
+ val = chg->fast_uamp;
+ val -= RT5033_CHARGER_FAST_CURRENT_MIN;
+ val /= RT5033_CHARGER_FAST_CURRENT_STEP_NUM;
+ reg_data = val;
+ }
+
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL5,
+ RT5033_CHGCTRL5_ICHG_MASK,
+ reg_data << RT5033_CHGCTRL5_ICHG_SHIFT);
+ if (ret) {
+ dev_err(charger->dev, "Failed regmap update\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static inline int rt5033_init_pre_charge(struct rt5033_charger *charger)
+{
+ struct rt5033_charger_data *chg = charger->chg;
+ int ret;
+ unsigned int val;
+ u8 reg_data;
+
+ /* Set pre-charge threshold voltage */
+ if (chg->pre_uvolt < RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MIN ||
+ chg->pre_uvolt > RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MAX) {
+ dev_err(charger->dev,
+ "Value 'precharge-upper-limit-microvolt' out of range\n");
+ return -EINVAL;
+ }
+
+ if (chg->pre_uvolt == RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MIN)
+ reg_data = 0x00;
+ else if (chg->pre_uvolt == RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MAX)
+ reg_data = 0x0f;
+ else {
+ val = chg->pre_uvolt;
+ val -= RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MIN;
+ val /= RT5033_CHARGER_PRE_THRESHOLD_STEP_NUM;
+ reg_data = val;
+ }
+
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL5,
+ RT5033_CHGCTRL5_VPREC_MASK, reg_data);
+ if (ret) {
+ dev_err(charger->dev, "Failed regmap update\n");
+ return -EINVAL;
+ }
+
+ /* Set pre-charge mode charging current */
+ if (chg->pre_uamp < RT5033_CHARGER_PRE_CURRENT_LIMIT_MIN ||
+ chg->pre_uamp > RT5033_CHARGER_PRE_CURRENT_LIMIT_MAX) {
+ dev_err(charger->dev,
+ "Value 'precharge-current-microamp' out of range\n");
+ return -EINVAL;
+ }
+
+ if (chg->pre_uamp == RT5033_CHARGER_PRE_CURRENT_LIMIT_MIN)
+ reg_data = 0x00;
+ else if (chg->pre_uamp == RT5033_CHARGER_PRE_CURRENT_LIMIT_MAX)
+ reg_data = RT5033_CHG_MAX_PRE_CURRENT;
+ else {
+ val = chg->pre_uamp;
+ val -= RT5033_CHARGER_PRE_CURRENT_LIMIT_MIN;
+ val /= RT5033_CHARGER_PRE_CURRENT_STEP_NUM;
+ reg_data = val;
+ }
+
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL4,
+ RT5033_CHGCTRL4_IPREC_MASK,
+ reg_data << RT5033_CHGCTRL4_IPREC_SHIFT);
+ if (ret) {
+ dev_err(charger->dev, "Failed regmap update\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int rt5033_charger_reg_init(struct rt5033_charger *charger)
+{
+ int ret = 0;
+
+ /* Enable charging termination */
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL1,
+ RT5033_CHGCTRL1_TE_EN_MASK, RT5033_TE_ENABLE);
+ if (ret) {
+ dev_err(charger->dev, "Failed to enable charging termination.\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Disable minimum input voltage regulation (MIVR), this improves
+ * the charging performance.
+ */
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL4,
+ RT5033_CHGCTRL4_MIVR_MASK, RT5033_CHARGER_MIVR_DISABLE);
+ if (ret) {
+ dev_err(charger->dev, "Failed to disable MIVR.\n");
+ return -EINVAL;
+ }
+
+ ret = rt5033_init_pre_charge(charger);
+ if (ret)
+ return ret;
+
+ ret = rt5033_init_fast_charge(charger);
+ if (ret)
+ return ret;
+
+ ret = rt5033_init_const_charge(charger);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static enum power_supply_property rt5033_charger_props[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_CHARGE_TYPE,
+ POWER_SUPPLY_PROP_CURRENT_MAX,
+ POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
+ POWER_SUPPLY_PROP_MODEL_NAME,
+ POWER_SUPPLY_PROP_MANUFACTURER,
+ POWER_SUPPLY_PROP_ONLINE,
+};
+
+static int rt5033_charger_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct rt5033_charger *charger = power_supply_get_drvdata(psy);
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_STATUS:
+ val->intval = rt5033_get_charger_state(charger);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_TYPE:
+ val->intval = rt5033_get_charger_type(charger);
+ break;
+ case POWER_SUPPLY_PROP_CURRENT_MAX:
+ val->intval = rt5033_get_charger_current_limit(charger);
+ break;
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+ val->intval = rt5033_get_charger_const_voltage(charger);
+ break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ val->strval = RT5033_CHARGER_MODEL;
+ break;
+ case POWER_SUPPLY_PROP_MANUFACTURER:
+ val->strval = RT5033_MANUFACTURER;
+ break;
+ case POWER_SUPPLY_PROP_ONLINE:
+ val->intval = (rt5033_get_charger_state(charger) ==
+ POWER_SUPPLY_STATUS_CHARGING);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static struct rt5033_charger_data *rt5033_charger_dt_init(
+ struct rt5033_charger *charger)
+{
+ struct rt5033_charger_data *chg;
+ struct power_supply_battery_info *info;
+ int ret;
+
+ chg = devm_kzalloc(charger->dev, sizeof(*chg), GFP_KERNEL);
+ if (!chg)
+ return ERR_PTR(-ENOMEM);
+
+ ret = power_supply_get_battery_info(charger->psy, &info);
+ if (ret) {
+ dev_err(charger->dev, "failed to get battery info\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ /* Assign data. Validity will be checked in the init functions. */
+ chg->pre_uamp = info->precharge_current_ua;
+ chg->fast_uamp = info->constant_charge_current_max_ua;
+ chg->eoc_uamp = info->charge_term_current_ua;
+ chg->pre_uvolt = info->precharge_voltage_max_uv;
+ chg->const_uvolt = info->constant_charge_voltage_max_uv;
+
+ return chg;
+}
+
+static const struct power_supply_desc rt5033_charger_desc = {
+ .name = "rt5033-charger",
+ .type = POWER_SUPPLY_TYPE_USB,
+ .properties = rt5033_charger_props,
+ .num_properties = ARRAY_SIZE(rt5033_charger_props),
+ .get_property = rt5033_charger_get_property,
+};
+
+static int rt5033_charger_probe(struct platform_device *pdev)
+{
+ struct rt5033_charger *charger;
+ struct rt5033_dev *rt5033 = dev_get_drvdata(pdev->dev.parent);
+ struct power_supply_config psy_cfg = {};
+ int ret;
+
+ charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
+ if (!charger)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, charger);
+ charger->dev = &pdev->dev;
+ charger->rt5033 = rt5033;
+
+ psy_cfg.of_node = pdev->dev.of_node;
+ psy_cfg.drv_data = charger;
+
+ charger->psy = devm_power_supply_register(&pdev->dev,
+ &rt5033_charger_desc,
+ &psy_cfg);
+ if (IS_ERR(charger->psy)) {
+ dev_err(&pdev->dev, "failed: power supply register\n");
+ return PTR_ERR(charger->psy);
+ }
+
+ charger->chg = rt5033_charger_dt_init(charger);
+ if (IS_ERR_OR_NULL(charger->chg))
+ return -ENODEV;
+
+ ret = rt5033_charger_reg_init(charger);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static const struct platform_device_id rt5033_charger_id[] = {
+ { "rt5033-charger", },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, rt5033_charger_id);
+
+static const struct of_device_id rt5033_charger_of_match[] = {
+ { .compatible = "richtek,rt5033-charger", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, rt5033_charger_of_match);
+
+static struct platform_driver rt5033_charger_driver = {
+ .driver = {
+ .name = "rt5033-charger",
+ .of_match_table = rt5033_charger_of_match,
+ },
+ .probe = rt5033_charger_probe,
+ .id_table = rt5033_charger_id,
+};
+module_platform_driver(rt5033_charger_driver);
+
+MODULE_DESCRIPTION("Richtek RT5033 charger driver");
+MODULE_AUTHOR("Beomho Seo <beomho.seo@samsung.com>");
+MODULE_LICENSE("GPL v2");
--
2.39.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v4 5/8] power: supply: rt5033_charger: Add RT5033 charger device driver
2023-05-06 15:54 ` [PATCH v4 5/8] power: supply: rt5033_charger: Add RT5033 charger device driver Jakob Hauser
@ 2023-05-08 6:50 ` Sebastian Reichel
2023-05-08 20:27 ` Jakob Hauser
0 siblings, 1 reply; 21+ messages in thread
From: Sebastian Reichel @ 2023-05-08 6:50 UTC (permalink / raw)
To: Jakob Hauser
Cc: Lee Jones, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Beomho Seo, Chanwoo Choi, Stephan Gerhold,
Raymond Hackley, Pavel Machek, Axel Lin, ChiYuan Huang,
Linus Walleij, Henrik Grimler, linux-pm, devicetree, linux-kernel,
phone-devel, ~postmarketos/upstreaming
[-- Attachment #1: Type: text/plain, Size: 17351 bytes --]
Hi,
On Sat, May 06, 2023 at 05:54:32PM +0200, Jakob Hauser wrote:
> This patch adds device driver of Richtek RT5033 PMIC. The driver supports
> switching charger. rt5033 charger provides three charging modes. The charging
> modes are pre-charge mode, fast charge mode and constant voltage mode. They
> vary in charge rate, the charge parameters can be controlled by i2c interface.
>
> Cc: Beomho Seo <beomho.seo@samsung.com>
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
Driver looks mostly good. I still found a couple of small things
though. Please find them inline.
> drivers/power/supply/Kconfig | 8 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/rt5033_charger.c | 464 ++++++++++++++++++++++++++
> 3 files changed, 473 insertions(+)
> create mode 100644 drivers/power/supply/rt5033_charger.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index c78be9f322e6..ea11797670ca 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -766,6 +766,14 @@ config BATTERY_RT5033
> The fuelgauge calculates and determines the battery state of charge
> according to battery open circuit voltage.
>
> +config CHARGER_RT5033
> + tristate "RT5033 battery charger support"
> + depends on MFD_RT5033
> + help
> + This adds support for battery charger in Richtek RT5033 PMIC.
> + The device supports pre-charge mode, fast charge mode and
> + constant voltage mode.
> +
> config CHARGER_RT9455
> tristate "Richtek RT9455 battery charger driver"
> depends on I2C
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 4adbfba02d05..dfc624bbcf1d 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_BATTERY_MAX17040) += max17040_battery.o
> obj-$(CONFIG_BATTERY_MAX17042) += max17042_battery.o
> obj-$(CONFIG_BATTERY_MAX1721X) += max1721x_battery.o
> obj-$(CONFIG_BATTERY_RT5033) += rt5033_battery.o
> +obj-$(CONFIG_CHARGER_RT5033) += rt5033_charger.o
> obj-$(CONFIG_CHARGER_RT9455) += rt9455_charger.o
> obj-$(CONFIG_CHARGER_RT9467) += rt9467-charger.o
> obj-$(CONFIG_CHARGER_RT9471) += rt9471.o
> diff --git a/drivers/power/supply/rt5033_charger.c b/drivers/power/supply/rt5033_charger.c
> new file mode 100644
> index 000000000000..038530d2f0a0
> --- /dev/null
> +++ b/drivers/power/supply/rt5033_charger.c
> @@ -0,0 +1,464 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Battery charger driver for RT5033
> + *
> + * Copyright (C) 2014 Samsung Electronics, Co., Ltd.
> + * Author: Beomho Seo <beomho.seo@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
no need for the GPL2 text with SPDX license header
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/mfd/rt5033-private.h>
> +#include <linux/mfd/rt5033.h>
> +
> +static int rt5033_get_charger_state(struct rt5033_charger *charger)
> +{
> + struct regmap *regmap = charger->rt5033->regmap;
> + unsigned int reg_data;
> + int state;
> +
> + if (!regmap)
> + return POWER_SUPPLY_STATUS_UNKNOWN;
> +
> + regmap_read(regmap, RT5033_REG_CHG_STAT, ®_data);
> +
> + switch (reg_data & RT5033_CHG_STAT_MASK) {
> + case RT5033_CHG_STAT_DISCHARGING:
> + state = POWER_SUPPLY_STATUS_DISCHARGING;
> + break;
> + case RT5033_CHG_STAT_CHARGING:
> + state = POWER_SUPPLY_STATUS_CHARGING;
> + break;
> + case RT5033_CHG_STAT_FULL:
> + state = POWER_SUPPLY_STATUS_FULL;
> + break;
> + case RT5033_CHG_STAT_NOT_CHARGING:
> + state = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + break;
> + default:
> + state = POWER_SUPPLY_STATUS_UNKNOWN;
> + }
> +
> + return state;
> +}
> +
> +static int rt5033_get_charger_type(struct rt5033_charger *charger)
> +{
> + struct regmap *regmap = charger->rt5033->regmap;
> + unsigned int reg_data;
> + int state;
> +
> + regmap_read(regmap, RT5033_REG_CHG_STAT, ®_data);
> +
> + switch (reg_data & RT5033_CHG_STAT_TYPE_MASK) {
> + case RT5033_CHG_STAT_TYPE_FAST:
> + state = POWER_SUPPLY_CHARGE_TYPE_FAST;
> + break;
> + case RT5033_CHG_STAT_TYPE_PRE:
> + state = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> + break;
> + default:
> + state = POWER_SUPPLY_CHARGE_TYPE_NONE;
> + }
> +
> + return state;
> +}
> +
> +static int rt5033_get_charger_current_limit(struct rt5033_charger *charger)
> +{
> + struct regmap *regmap = charger->rt5033->regmap;
> + unsigned int state, reg_data, data;
> +
> + regmap_read(regmap, RT5033_REG_CHG_CTRL5, ®_data);
> +
> + state = (reg_data & RT5033_CHGCTRL5_ICHG_MASK)
> + >> RT5033_CHGCTRL5_ICHG_SHIFT;
> +
> + data = RT5033_CHARGER_FAST_CURRENT_MIN +
> + RT5033_CHARGER_FAST_CURRENT_STEP_NUM * state;
> +
> + return data;
> +}
> +
> +static int rt5033_get_charger_const_voltage(struct rt5033_charger *charger)
> +{
> + struct regmap *regmap = charger->rt5033->regmap;
> + unsigned int state, reg_data, data;
> +
> + regmap_read(regmap, RT5033_REG_CHG_CTRL2, ®_data);
> +
> + state = (reg_data & RT5033_CHGCTRL2_CV_MASK)
> + >> RT5033_CHGCTRL2_CV_SHIFT;
> +
> + data = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN +
> + RT5033_CHARGER_CONST_VOLTAGE_STEP_NUM * state;
> +
> + return data;
> +}
> +
> +static inline int rt5033_init_const_charge(struct rt5033_charger *charger)
> +{
> + struct rt5033_charger_data *chg = charger->chg;
> + int ret;
> + unsigned int val;
> + u8 reg_data;
> +
> + /* Set constant voltage mode */
> + if (chg->const_uvolt < RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN ||
> + chg->const_uvolt > RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX) {
> + dev_err(charger->dev,
> + "Value 'constant-charge-voltage-max-microvolt' out of range\n");
> + return -EINVAL;
> + }
> +
> + if (chg->const_uvolt == RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN)
> + reg_data = 0x00;
> + else if (chg->const_uvolt == RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX)
> + reg_data = RT5033_CV_MAX_VOLTAGE;
> + else {
> + val = chg->const_uvolt;
> + val -= RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN;
> + val /= RT5033_CHARGER_CONST_VOLTAGE_STEP_NUM;
> + reg_data = val;
> + }
> +
> + ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL2,
> + RT5033_CHGCTRL2_CV_MASK,
> + reg_data << RT5033_CHGCTRL2_CV_SHIFT);
> + if (ret) {
> + dev_err(charger->dev, "Failed regmap update\n");
> + return -EINVAL;
> + }
> +
> + /* Set end of charge current */
> + if (chg->eoc_uamp < RT5033_CHARGER_EOC_MIN ||
> + chg->eoc_uamp > RT5033_CHARGER_EOC_MAX) {
> + dev_err(charger->dev,
> + "Value 'charge-term-current-microamp' out of range\n");
> + return -EINVAL;
> + }
> +
> + if (chg->eoc_uamp == RT5033_CHARGER_EOC_MIN)
> + reg_data = 0x01;
> + else if (chg->eoc_uamp == RT5033_CHARGER_EOC_MAX)
> + reg_data = 0x07;
> + else {
> + val = chg->eoc_uamp;
> + if (val < RT5033_CHARGER_EOC_REF) {
> + val -= RT5033_CHARGER_EOC_MIN;
> + val /= RT5033_CHARGER_EOC_STEP_NUM1;
> + reg_data = 0x01 + val;
> + } else if (val > RT5033_CHARGER_EOC_REF) {
> + val -= RT5033_CHARGER_EOC_REF;
> + val /= RT5033_CHARGER_EOC_STEP_NUM2;
> + reg_data = 0x04 + val;
> + } else {
> + reg_data = 0x04;
> + }
> + }
> +
> + ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL4,
> + RT5033_CHGCTRL4_EOC_MASK, reg_data);
> + if (ret) {
> + dev_err(charger->dev, "Failed regmap update\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static inline int rt5033_init_fast_charge(struct rt5033_charger *charger)
> +{
> + struct rt5033_charger_data *chg = charger->chg;
> + int ret;
> + unsigned int val;
> + u8 reg_data;
> +
> + /* Set limit input current */
> + ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL1,
> + RT5033_CHGCTRL1_IAICR_MASK, RT5033_AICR_2000_MODE);
> + if (ret) {
> + dev_err(charger->dev, "Failed regmap update\n");
> + return -EINVAL;
> + }
> +
> + /* Set fast-charge mode charging current */
> + if (chg->fast_uamp < RT5033_CHARGER_FAST_CURRENT_MIN ||
> + chg->fast_uamp > RT5033_CHARGER_FAST_CURRENT_MAX) {
> + dev_err(charger->dev,
> + "Value 'constant-charge-current-max-microamp' out of range\n");
> + return -EINVAL;
> + }
> +
> + if (chg->fast_uamp == RT5033_CHARGER_FAST_CURRENT_MIN)
> + reg_data = 0x00;
> + else if (chg->fast_uamp == RT5033_CHARGER_FAST_CURRENT_MAX)
> + reg_data = RT5033_CHG_MAX_CURRENT;
> + else {
> + val = chg->fast_uamp;
> + val -= RT5033_CHARGER_FAST_CURRENT_MIN;
> + val /= RT5033_CHARGER_FAST_CURRENT_STEP_NUM;
> + reg_data = val;
> + }
> +
> + ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL5,
> + RT5033_CHGCTRL5_ICHG_MASK,
> + reg_data << RT5033_CHGCTRL5_ICHG_SHIFT);
> + if (ret) {
> + dev_err(charger->dev, "Failed regmap update\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static inline int rt5033_init_pre_charge(struct rt5033_charger *charger)
> +{
> + struct rt5033_charger_data *chg = charger->chg;
> + int ret;
> + unsigned int val;
> + u8 reg_data;
> +
> + /* Set pre-charge threshold voltage */
> + if (chg->pre_uvolt < RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MIN ||
> + chg->pre_uvolt > RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MAX) {
> + dev_err(charger->dev,
> + "Value 'precharge-upper-limit-microvolt' out of range\n");
> + return -EINVAL;
> + }
> +
> + if (chg->pre_uvolt == RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MIN)
> + reg_data = 0x00;
> + else if (chg->pre_uvolt == RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MAX)
> + reg_data = 0x0f;
> + else {
> + val = chg->pre_uvolt;
> + val -= RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MIN;
> + val /= RT5033_CHARGER_PRE_THRESHOLD_STEP_NUM;
> + reg_data = val;
> + }
> +
> + ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL5,
> + RT5033_CHGCTRL5_VPREC_MASK, reg_data);
> + if (ret) {
> + dev_err(charger->dev, "Failed regmap update\n");
> + return -EINVAL;
> + }
> +
> + /* Set pre-charge mode charging current */
> + if (chg->pre_uamp < RT5033_CHARGER_PRE_CURRENT_LIMIT_MIN ||
> + chg->pre_uamp > RT5033_CHARGER_PRE_CURRENT_LIMIT_MAX) {
> + dev_err(charger->dev,
> + "Value 'precharge-current-microamp' out of range\n");
> + return -EINVAL;
> + }
> +
> + if (chg->pre_uamp == RT5033_CHARGER_PRE_CURRENT_LIMIT_MIN)
> + reg_data = 0x00;
> + else if (chg->pre_uamp == RT5033_CHARGER_PRE_CURRENT_LIMIT_MAX)
> + reg_data = RT5033_CHG_MAX_PRE_CURRENT;
> + else {
> + val = chg->pre_uamp;
> + val -= RT5033_CHARGER_PRE_CURRENT_LIMIT_MIN;
> + val /= RT5033_CHARGER_PRE_CURRENT_STEP_NUM;
> + reg_data = val;
> + }
> +
> + ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL4,
> + RT5033_CHGCTRL4_IPREC_MASK,
> + reg_data << RT5033_CHGCTRL4_IPREC_SHIFT);
> + if (ret) {
> + dev_err(charger->dev, "Failed regmap update\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int rt5033_charger_reg_init(struct rt5033_charger *charger)
> +{
> + int ret = 0;
> +
> + /* Enable charging termination */
> + ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL1,
> + RT5033_CHGCTRL1_TE_EN_MASK, RT5033_TE_ENABLE);
> + if (ret) {
> + dev_err(charger->dev, "Failed to enable charging termination.\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * Disable minimum input voltage regulation (MIVR), this improves
> + * the charging performance.
> + */
> + ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL4,
> + RT5033_CHGCTRL4_MIVR_MASK, RT5033_CHARGER_MIVR_DISABLE);
> + if (ret) {
> + dev_err(charger->dev, "Failed to disable MIVR.\n");
> + return -EINVAL;
> + }
> +
> + ret = rt5033_init_pre_charge(charger);
> + if (ret)
> + return ret;
> +
> + ret = rt5033_init_fast_charge(charger);
> + if (ret)
> + return ret;
> +
> + ret = rt5033_init_const_charge(charger);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static enum power_supply_property rt5033_charger_props[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_CHARGE_TYPE,
> + POWER_SUPPLY_PROP_CURRENT_MAX,
> + POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
> + POWER_SUPPLY_PROP_MODEL_NAME,
> + POWER_SUPPLY_PROP_MANUFACTURER,
> + POWER_SUPPLY_PROP_ONLINE,
> +};
> +
> +static int rt5033_charger_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct rt5033_charger *charger = power_supply_get_drvdata(psy);
> + int ret = 0;
ret is unused.
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> + val->intval = rt5033_get_charger_state(charger);
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> + val->intval = rt5033_get_charger_type(charger);
> + break;
> + case POWER_SUPPLY_PROP_CURRENT_MAX:
> + val->intval = rt5033_get_charger_current_limit(charger);
> + break;
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> + val->intval = rt5033_get_charger_const_voltage(charger);
> + break;
> + case POWER_SUPPLY_PROP_MODEL_NAME:
> + val->strval = RT5033_CHARGER_MODEL;
> + break;
> + case POWER_SUPPLY_PROP_MANUFACTURER:
> + val->strval = RT5033_MANUFACTURER;
> + break;
> + case POWER_SUPPLY_PROP_ONLINE:
> + val->intval = (rt5033_get_charger_state(charger) ==
> + POWER_SUPPLY_STATUS_CHARGING);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static struct rt5033_charger_data *rt5033_charger_dt_init(
> + struct rt5033_charger *charger)
> +{
> + struct rt5033_charger_data *chg;
> + struct power_supply_battery_info *info;
> + int ret;
> +
> + chg = devm_kzalloc(charger->dev, sizeof(*chg), GFP_KERNEL);
> + if (!chg)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = power_supply_get_battery_info(charger->psy, &info);
> + if (ret) {
> + dev_err(charger->dev, "failed to get battery info\n");
> + return ERR_PTR(-EINVAL);
> + }
You can use the battery_info from the power-supply core:
info = charger->psy->battery_info;
if (!info)
return dev_err_probe(charger->dev, -EINVAL, "missing battery info\n");
> + /* Assign data. Validity will be checked in the init functions. */
> + chg->pre_uamp = info->precharge_current_ua;
> + chg->fast_uamp = info->constant_charge_current_max_ua;
> + chg->eoc_uamp = info->charge_term_current_ua;
> + chg->pre_uvolt = info->precharge_voltage_max_uv;
> + chg->const_uvolt = info->constant_charge_voltage_max_uv;
> +
> + return chg;
> +}
> +
> +static const struct power_supply_desc rt5033_charger_desc = {
> + .name = "rt5033-charger",
> + .type = POWER_SUPPLY_TYPE_USB,
> + .properties = rt5033_charger_props,
> + .num_properties = ARRAY_SIZE(rt5033_charger_props),
> + .get_property = rt5033_charger_get_property,
> +};
> +
> +static int rt5033_charger_probe(struct platform_device *pdev)
> +{
> + struct rt5033_charger *charger;
> + struct rt5033_dev *rt5033 = dev_get_drvdata(pdev->dev.parent);
> + struct power_supply_config psy_cfg = {};
> + int ret;
> +
> + charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
> + if (!charger)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, charger);
> + charger->dev = &pdev->dev;
> + charger->rt5033 = rt5033;
You are only using the regmap, so just get and store that:
charger->regmap = dev_get_regmap(pdev->dev.parent);
> + psy_cfg.of_node = pdev->dev.of_node;
> + psy_cfg.drv_data = charger;
> +
> + charger->psy = devm_power_supply_register(&pdev->dev,
> + &rt5033_charger_desc,
> + &psy_cfg);
> + if (IS_ERR(charger->psy)) {
> + dev_err(&pdev->dev, "failed: power supply register\n");
> + return PTR_ERR(charger->psy);
> + }
dev_err_probe()
> +
> + charger->chg = rt5033_charger_dt_init(charger);
> + if (IS_ERR_OR_NULL(charger->chg))
> + return -ENODEV;
> +
> + ret = rt5033_charger_reg_init(charger);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static const struct platform_device_id rt5033_charger_id[] = {
> + { "rt5033-charger", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(platform, rt5033_charger_id);
> +
> +static const struct of_device_id rt5033_charger_of_match[] = {
> + { .compatible = "richtek,rt5033-charger", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, rt5033_charger_of_match);
> +
> +static struct platform_driver rt5033_charger_driver = {
> + .driver = {
> + .name = "rt5033-charger",
> + .of_match_table = rt5033_charger_of_match,
> + },
> + .probe = rt5033_charger_probe,
> + .id_table = rt5033_charger_id,
> +};
> +module_platform_driver(rt5033_charger_driver);
> +
> +MODULE_DESCRIPTION("Richtek RT5033 charger driver");
> +MODULE_AUTHOR("Beomho Seo <beomho.seo@samsung.com>");
> +MODULE_LICENSE("GPL v2");
Thanks,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 5/8] power: supply: rt5033_charger: Add RT5033 charger device driver
2023-05-08 6:50 ` Sebastian Reichel
@ 2023-05-08 20:27 ` Jakob Hauser
0 siblings, 0 replies; 21+ messages in thread
From: Jakob Hauser @ 2023-05-08 20:27 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Beomho Seo, Chanwoo Choi, Stephan Gerhold,
Raymond Hackley, Pavel Machek, Axel Lin, ChiYuan Huang,
Linus Walleij, Henrik Grimler, linux-pm, devicetree, linux-kernel,
phone-devel, ~postmarketos/upstreaming
Hi Sebastian,
On 08.05.23 08:50, Sebastian Reichel wrote:
> Hi,
>
> On Sat, May 06, 2023 at 05:54:32PM +0200, Jakob Hauser wrote:
>> This patch adds device driver of Richtek RT5033 PMIC. The driver supports
>> switching charger. rt5033 charger provides three charging modes. The charging
>> modes are pre-charge mode, fast charge mode and constant voltage mode. They
>> vary in charge rate, the charge parameters can be controlled by i2c interface.
>>
>> Cc: Beomho Seo <beomho.seo@samsung.com>
>> Cc: Chanwoo Choi <cw00.choi@samsung.com>
>> Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
>> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>
> Driver looks mostly good. I still found a couple of small things
> though. Please find them inline.
...
The change requests look reasonable. Thanks for additional hints. I'll
implement them in v5.
Kind regards,
Jakob
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 6/8] power: supply: rt5033_charger: Add cable detection and USB OTG supply
2023-05-06 15:54 ` [PATCH v4 0/8] Add RT5033 charger device driver Jakob Hauser
` (4 preceding siblings ...)
2023-05-06 15:54 ` [PATCH v4 5/8] power: supply: rt5033_charger: Add RT5033 charger device driver Jakob Hauser
@ 2023-05-06 15:54 ` Jakob Hauser
2023-05-08 11:43 ` Sebastian Reichel
2023-05-06 15:54 ` [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger Jakob Hauser
2023-05-06 15:54 ` [PATCH v4 8/8] dt-bindings: Add rt5033 mfd, regulator and charger Jakob Hauser
7 siblings, 1 reply; 21+ messages in thread
From: Jakob Hauser @ 2023-05-06 15:54 UTC (permalink / raw)
To: Sebastian Reichel, Lee Jones, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski
Cc: Beomho Seo, Chanwoo Choi, Stephan Gerhold, Raymond Hackley,
Pavel Machek, Axel Lin, ChiYuan Huang, Linus Walleij,
Henrik Grimler, linux-pm, devicetree, linux-kernel, phone-devel,
~postmarketos/upstreaming
Implement cable detection by extcon and handle the driver according to the
connector type.
There are basically three types of action: "set_charging", "set_otg" and
"set_disconnect".
A forth helper function to "unset_otg" was added because this is used in both
"set_charging" and "set_disconnect". In the first case it covers the rather
rare event that someone changes from OTG to charging without disconnect. In
the second case, when disconnecting, the values are set back to the ones from
initialization to return into a defined state.
Additionally, there is "set_mivr". When connecting to e.g. a laptop/PC, the
minimum input voltage regulation (MIVR) shall prevent a voltage drop if the
cable or the supply is weak. The MIVR value is set to 4600MV, same as in the
Android driver [1]. When disconnecting, MIVR is set back to DISABLED.
In the function rt5033_get_charger_state(): When in OTG mode, the chip
reports status "charging". Change this to "discharging" because there is
no charging going on in OTG mode [2].
[1] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L499
[2] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L686-L687
Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
drivers/power/supply/rt5033_charger.c | 268 +++++++++++++++++++++++++-
include/linux/mfd/rt5033.h | 8 +
2 files changed, 274 insertions(+), 2 deletions(-)
diff --git a/drivers/power/supply/rt5033_charger.c b/drivers/power/supply/rt5033_charger.c
index 038530d2f0a0..fe63c16a9d51 100644
--- a/drivers/power/supply/rt5033_charger.c
+++ b/drivers/power/supply/rt5033_charger.c
@@ -10,7 +10,10 @@
* published by the Free Software Foundation.
*/
+#include <linux/devm-helpers.h>
+#include <linux/extcon.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/platform_device.h>
#include <linux/power_supply.h>
#include <linux/mfd/rt5033-private.h>
@@ -44,6 +47,10 @@ static int rt5033_get_charger_state(struct rt5033_charger *charger)
state = POWER_SUPPLY_STATUS_UNKNOWN;
}
+ /* For OTG mode, RT5033 would still report "charging" */
+ if (charger->otg)
+ state = POWER_SUPPLY_STATUS_DISCHARGING;
+
return state;
}
@@ -135,6 +142,9 @@ static inline int rt5033_init_const_charge(struct rt5033_charger *charger)
return -EINVAL;
}
+ /* Store that value for later usage */
+ charger->cv_regval = reg_data;
+
/* Set end of charge current */
if (chg->eoc_uamp < RT5033_CHARGER_EOC_MIN ||
chg->eoc_uamp > RT5033_CHARGER_EOC_MAX) {
@@ -318,6 +328,152 @@ static int rt5033_charger_reg_init(struct rt5033_charger *charger)
return 0;
}
+static int rt5033_charger_set_otg(struct rt5033_charger *charger)
+{
+ int ret;
+
+ mutex_lock(&charger->lock);
+
+ /* Set OTG boost v_out to 5 volts */
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL2,
+ RT5033_CHGCTRL2_CV_MASK,
+ 0x37 << RT5033_CHGCTRL2_CV_SHIFT);
+ if (ret) {
+ dev_err(charger->dev, "Failed set OTG boost v_out\n");
+ return -EINVAL;
+ }
+
+ /* Set operation mode to OTG */
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL1,
+ RT5033_CHGCTRL1_MODE_MASK, RT5033_BOOST_MODE);
+ if (ret) {
+ dev_err(charger->dev, "Failed to update OTG mode.\n");
+ return -EINVAL;
+ }
+
+ /* In case someone switched from charging to OTG directly */
+ if (charger->online)
+ charger->online = false;
+
+ charger->otg = true;
+
+ mutex_unlock(&charger->lock);
+
+ return 0;
+}
+
+static int rt5033_charger_unset_otg(struct rt5033_charger *charger)
+{
+ int ret;
+ u8 data;
+
+ /* Restore constant voltage for charging */
+ data = charger->cv_regval;
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL2,
+ RT5033_CHGCTRL2_CV_MASK,
+ data << RT5033_CHGCTRL2_CV_SHIFT);
+ if (ret) {
+ dev_err(charger->dev, "Failed to restore constant voltage\n");
+ return -EINVAL;
+ }
+
+ /* Set operation mode to charging */
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL1,
+ RT5033_CHGCTRL1_MODE_MASK, RT5033_CHARGER_MODE);
+ if (ret) {
+ dev_err(charger->dev, "Failed to update charger mode.\n");
+ return -EINVAL;
+ }
+
+ charger->otg = false;
+
+ return 0;
+}
+
+static int rt5033_charger_set_charging(struct rt5033_charger *charger)
+{
+ int ret;
+
+ mutex_lock(&charger->lock);
+
+ /* In case someone switched from OTG to charging directly */
+ if (charger->otg) {
+ ret = rt5033_charger_unset_otg(charger);
+ if (ret)
+ return -EINVAL;
+ }
+
+ charger->online = true;
+
+ mutex_unlock(&charger->lock);
+
+ return 0;
+}
+
+static int rt5033_charger_set_mivr(struct rt5033_charger *charger)
+{
+ int ret;
+
+ mutex_lock(&charger->lock);
+
+ /*
+ * When connected via USB connector type SDP (Standard Downstream Port),
+ * the minimum input voltage regulation (MIVR) should be enabled. It
+ * prevents an input voltage drop due to insufficient current provided
+ * by the adapter or USB input. As a downside, it may reduces the
+ * charging current and thus slows the charging.
+ */
+ ret = regmap_update_bits(charger->rt5033->regmap, RT5033_REG_CHG_CTRL4,
+ RT5033_CHGCTRL4_MIVR_MASK, RT5033_CHARGER_MIVR_4600MV);
+ if (ret) {
+ dev_err(charger->dev, "Failed to set MIVR level.\n");
+ return -EINVAL;
+ }
+
+ charger->mivr_enabled = true;
+
+ mutex_unlock(&charger->lock);
+
+ /* Beyond this, do the same steps like setting charging */
+ rt5033_charger_set_charging(charger);
+
+ return 0;
+}
+
+static int rt5033_charger_set_disconnect(struct rt5033_charger *charger)
+{
+ int ret;
+
+ mutex_lock(&charger->lock);
+
+ /* Disable MIVR if enabled */
+ if (charger->mivr_enabled) {
+ ret = regmap_update_bits(charger->rt5033->regmap,
+ RT5033_REG_CHG_CTRL4,
+ RT5033_CHGCTRL4_MIVR_MASK,
+ RT5033_CHARGER_MIVR_DISABLE);
+ if (ret) {
+ dev_err(charger->dev, "Failed to disable MIVR.\n");
+ return -EINVAL;
+ }
+
+ charger->mivr_enabled = false;
+ }
+
+ if (charger->otg) {
+ ret = rt5033_charger_unset_otg(charger);
+ if (ret)
+ return -EINVAL;
+ }
+
+ if (charger->online)
+ charger->online = false;
+
+ mutex_unlock(&charger->lock);
+
+ return 0;
+}
+
static enum power_supply_property rt5033_charger_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_CHARGE_TYPE,
@@ -355,8 +511,7 @@ static int rt5033_charger_get_property(struct power_supply *psy,
val->strval = RT5033_MANUFACTURER;
break;
case POWER_SUPPLY_PROP_ONLINE:
- val->intval = (rt5033_get_charger_state(charger) ==
- POWER_SUPPLY_STATUS_CHARGING);
+ val->intval = charger->online;
break;
default:
return -EINVAL;
@@ -392,6 +547,86 @@ static struct rt5033_charger_data *rt5033_charger_dt_init(
return chg;
}
+static void rt5033_charger_extcon_work(struct work_struct *work)
+{
+ struct rt5033_charger *charger =
+ container_of(work, struct rt5033_charger, extcon_work);
+ struct extcon_dev *edev = charger->edev;
+ int connector, state;
+ int ret;
+
+ for (connector = EXTCON_USB_HOST; connector <= EXTCON_CHG_USB_PD;
+ connector++) {
+ state = extcon_get_state(edev, connector);
+ if (state == 1)
+ break;
+ }
+
+ /*
+ * Adding a delay between extcon notification and extcon action. This
+ * makes extcon action execution more reliable. Without the delay the
+ * execution sometimes fails, possibly because the chip is busy or not
+ * ready.
+ */
+ msleep(100);
+
+ switch (connector) {
+ case EXTCON_CHG_USB_SDP:
+ ret = rt5033_charger_set_mivr(charger);
+ if (ret) {
+ dev_err(charger->dev, "failed to set USB mode\n");
+ break;
+ }
+ dev_info(charger->dev, "USB mode. connector type: %d\n",
+ connector);
+ break;
+ case EXTCON_CHG_USB_DCP:
+ case EXTCON_CHG_USB_CDP:
+ case EXTCON_CHG_USB_ACA:
+ case EXTCON_CHG_USB_FAST:
+ case EXTCON_CHG_USB_SLOW:
+ case EXTCON_CHG_WPT:
+ case EXTCON_CHG_USB_PD:
+ ret = rt5033_charger_set_charging(charger);
+ if (ret) {
+ dev_err(charger->dev, "failed to set charging\n");
+ break;
+ }
+ dev_info(charger->dev, "charging. connector type: %d\n",
+ connector);
+ break;
+ case EXTCON_USB_HOST:
+ ret = rt5033_charger_set_otg(charger);
+ if (ret) {
+ dev_err(charger->dev, "failed to set OTG\n");
+ break;
+ }
+ dev_info(charger->dev, "OTG enabled\n");
+ break;
+ default:
+ ret = rt5033_charger_set_disconnect(charger);
+ if (ret) {
+ dev_err(charger->dev, "failed to set disconnect\n");
+ break;
+ }
+ dev_info(charger->dev, "disconnected\n");
+ break;
+ }
+
+ power_supply_changed(charger->psy);
+}
+
+static int rt5033_charger_extcon_notifier(struct notifier_block *nb,
+ unsigned long event, void *param)
+{
+ struct rt5033_charger *charger =
+ container_of(nb, struct rt5033_charger, extcon_nb);
+
+ schedule_work(&charger->extcon_work);
+
+ return NOTIFY_OK;
+}
+
static const struct power_supply_desc rt5033_charger_desc = {
.name = "rt5033-charger",
.type = POWER_SUPPLY_TYPE_USB,
@@ -405,6 +640,7 @@ static int rt5033_charger_probe(struct platform_device *pdev)
struct rt5033_charger *charger;
struct rt5033_dev *rt5033 = dev_get_drvdata(pdev->dev.parent);
struct power_supply_config psy_cfg = {};
+ struct device_node *np_conn, *np_edev;
int ret;
charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
@@ -414,6 +650,7 @@ static int rt5033_charger_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, charger);
charger->dev = &pdev->dev;
charger->rt5033 = rt5033;
+ mutex_init(&charger->lock);
psy_cfg.of_node = pdev->dev.of_node;
psy_cfg.drv_data = charger;
@@ -434,6 +671,33 @@ static int rt5033_charger_probe(struct platform_device *pdev)
if (ret)
return ret;
+ /*
+ * Extcon support is not vital for the charger to work. If no extcon
+ * is available, just emit a warning and leave the probe function.
+ */
+ np_conn = of_parse_phandle(pdev->dev.of_node, "connector", 0);
+ np_edev = of_get_parent(np_conn);
+ charger->edev = extcon_find_edev_by_node(np_edev);
+ if (IS_ERR(charger->edev)) {
+ dev_warn(&pdev->dev, "no extcon device found in device-tree\n");
+ goto out;
+ }
+
+ ret = devm_work_autocancel(&pdev->dev, &charger->extcon_work,
+ rt5033_charger_extcon_work);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to initialize extcon work\n");
+ return ret;
+ }
+
+ charger->extcon_nb.notifier_call = rt5033_charger_extcon_notifier;
+ ret = devm_extcon_register_notifier_all(&pdev->dev, charger->edev,
+ &charger->extcon_nb);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register extcon notifier\n");
+ return ret;
+ }
+out:
return 0;
}
diff --git a/include/linux/mfd/rt5033.h b/include/linux/mfd/rt5033.h
index e99e2ab0c1c1..d2c613764756 100644
--- a/include/linux/mfd/rt5033.h
+++ b/include/linux/mfd/rt5033.h
@@ -53,6 +53,14 @@ struct rt5033_charger {
struct rt5033_dev *rt5033;
struct power_supply *psy;
struct rt5033_charger_data *chg;
+ struct extcon_dev *edev;
+ struct notifier_block extcon_nb;
+ struct work_struct extcon_work;
+ struct mutex lock;
+ bool online;
+ bool otg;
+ bool mivr_enabled;
+ u8 cv_regval;
};
#endif /* __RT5033_H__ */
--
2.39.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v4 6/8] power: supply: rt5033_charger: Add cable detection and USB OTG supply
2023-05-06 15:54 ` [PATCH v4 6/8] power: supply: rt5033_charger: Add cable detection and USB OTG supply Jakob Hauser
@ 2023-05-08 11:43 ` Sebastian Reichel
2023-05-08 20:36 ` Jakob Hauser
0 siblings, 1 reply; 21+ messages in thread
From: Sebastian Reichel @ 2023-05-08 11:43 UTC (permalink / raw)
To: Jakob Hauser
Cc: Lee Jones, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Beomho Seo, Chanwoo Choi, Stephan Gerhold,
Raymond Hackley, Pavel Machek, Axel Lin, ChiYuan Huang,
Linus Walleij, Henrik Grimler, linux-pm, devicetree, linux-kernel,
phone-devel, ~postmarketos/upstreaming
[-- Attachment #1: Type: text/plain, Size: 2369 bytes --]
Hi,
On Sat, May 06, 2023 at 05:54:33PM +0200, Jakob Hauser wrote:
> Implement cable detection by extcon and handle the driver according to the
> connector type.
>
> There are basically three types of action: "set_charging", "set_otg" and
> "set_disconnect".
>
> A forth helper function to "unset_otg" was added because this is used in both
> "set_charging" and "set_disconnect". In the first case it covers the rather
> rare event that someone changes from OTG to charging without disconnect. In
> the second case, when disconnecting, the values are set back to the ones from
> initialization to return into a defined state.
>
> Additionally, there is "set_mivr". When connecting to e.g. a laptop/PC, the
> minimum input voltage regulation (MIVR) shall prevent a voltage drop if the
> cable or the supply is weak. The MIVR value is set to 4600MV, same as in the
> Android driver [1]. When disconnecting, MIVR is set back to DISABLED.
>
> In the function rt5033_get_charger_state(): When in OTG mode, the chip
> reports status "charging". Change this to "discharging" because there is
> no charging going on in OTG mode [2].
>
> [1] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L499
> [2] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L686-L687
>
> Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
> ---
> [...]
> diff --git a/include/linux/mfd/rt5033.h b/include/linux/mfd/rt5033.h
> index e99e2ab0c1c1..d2c613764756 100644
> --- a/include/linux/mfd/rt5033.h
> +++ b/include/linux/mfd/rt5033.h
> @@ -53,6 +53,14 @@ struct rt5033_charger {
> struct rt5033_dev *rt5033;
> struct power_supply *psy;
> struct rt5033_charger_data *chg;
> + struct extcon_dev *edev;
> + struct notifier_block extcon_nb;
> + struct work_struct extcon_work;
> + struct mutex lock;
> + bool online;
> + bool otg;
> + bool mivr_enabled;
> + u8 cv_regval;
> };
Please move 'struct rt5033_charger' and 'struct rt5033_charger_data'
to drivers/power/supply/rt5033_charger.c; they are not supposed to
be used directly by others. Making it private helps to avoid
cross-subsystem series for future maintanence patches.
Otherwise LGTM.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 6/8] power: supply: rt5033_charger: Add cable detection and USB OTG supply
2023-05-08 11:43 ` Sebastian Reichel
@ 2023-05-08 20:36 ` Jakob Hauser
0 siblings, 0 replies; 21+ messages in thread
From: Jakob Hauser @ 2023-05-08 20:36 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Beomho Seo, Chanwoo Choi, Stephan Gerhold,
Raymond Hackley, Pavel Machek, Axel Lin, ChiYuan Huang,
Linus Walleij, Henrik Grimler, linux-pm, devicetree, linux-kernel,
phone-devel, ~postmarketos/upstreaming
Hi Sebastian,
On 08.05.23 13:43, Sebastian Reichel wrote:
> On Sat, May 06, 2023 at 05:54:33PM +0200, Jakob Hauser wrote:
...
>> diff --git a/include/linux/mfd/rt5033.h b/include/linux/mfd/rt5033.h
>> index e99e2ab0c1c1..d2c613764756 100644
>> --- a/include/linux/mfd/rt5033.h
>> +++ b/include/linux/mfd/rt5033.h
>> @@ -53,6 +53,14 @@ struct rt5033_charger {
>> struct rt5033_dev *rt5033;
>> struct power_supply *psy;
>> struct rt5033_charger_data *chg;
>> + struct extcon_dev *edev;
>> + struct notifier_block extcon_nb;
>> + struct work_struct extcon_work;
>> + struct mutex lock;
>> + bool online;
>> + bool otg;
>> + bool mivr_enabled;
>> + u8 cv_regval;
>> };
>
> Please move 'struct rt5033_charger' and 'struct rt5033_charger_data'
> to drivers/power/supply/rt5033_charger.c; they are not supposed to
> be used directly by others. Making it private helps to avoid
> cross-subsystem series for future maintanence patches.
Makes sense, I'll change that.
For completeness, I'll add an additional patch to move the 'struct
rt5033_battery' [1] to drivers/power/supply/rt5033_battery.c.
[1]
https://github.com/torvalds/linux/blob/v6.4-rc1/include/linux/mfd/rt5033.h#L35-L40
Kind regards,
Jakob
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger
2023-05-06 15:54 ` [PATCH v4 0/8] Add RT5033 charger device driver Jakob Hauser
` (5 preceding siblings ...)
2023-05-06 15:54 ` [PATCH v4 6/8] power: supply: rt5033_charger: Add cable detection and USB OTG supply Jakob Hauser
@ 2023-05-06 15:54 ` Jakob Hauser
2023-05-08 11:35 ` Sebastian Reichel
2023-05-06 15:54 ` [PATCH v4 8/8] dt-bindings: Add rt5033 mfd, regulator and charger Jakob Hauser
7 siblings, 1 reply; 21+ messages in thread
From: Jakob Hauser @ 2023-05-06 15:54 UTC (permalink / raw)
To: Sebastian Reichel, Lee Jones, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski
Cc: Beomho Seo, Chanwoo Choi, Stephan Gerhold, Raymond Hackley,
Pavel Machek, Axel Lin, ChiYuan Huang, Linus Walleij,
Henrik Grimler, linux-pm, devicetree, linux-kernel, phone-devel,
~postmarketos/upstreaming
The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger
can, let's get this value.
Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c
index 5c04cf305219..a6520716d813 100644
--- a/drivers/power/supply/rt5033_battery.c
+++ b/drivers/power/supply/rt5033_battery.c
@@ -12,6 +12,26 @@
#include <linux/mfd/rt5033-private.h>
#include <linux/mfd/rt5033.h>
+static int rt5033_battery_get_status(struct i2c_client *client)
+{
+ struct power_supply *charger;
+ union power_supply_propval val;
+ int ret;
+
+ charger = power_supply_get_by_name("rt5033-charger");
+ if (!charger)
+ return POWER_SUPPLY_STATUS_UNKNOWN;
+
+ ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val);
+ if (ret) {
+ power_supply_put(charger);
+ return POWER_SUPPLY_STATUS_UNKNOWN;
+ }
+
+ power_supply_put(charger);
+ return val.intval;
+}
+
static int rt5033_battery_get_capacity(struct i2c_client *client)
{
struct rt5033_battery *battery = i2c_get_clientdata(client);
@@ -84,6 +104,9 @@ static int rt5033_battery_get_property(struct power_supply *psy,
case POWER_SUPPLY_PROP_CAPACITY:
val->intval = rt5033_battery_get_capacity(battery->client);
break;
+ case POWER_SUPPLY_PROP_STATUS:
+ val->intval = rt5033_battery_get_status(battery->client);
+ break;
default:
return -EINVAL;
}
@@ -96,6 +119,7 @@ static enum power_supply_property rt5033_battery_props[] = {
POWER_SUPPLY_PROP_VOLTAGE_OCV,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_CAPACITY,
+ POWER_SUPPLY_PROP_STATUS,
};
static const struct regmap_config rt5033_battery_regmap_config = {
--
2.39.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger
2023-05-06 15:54 ` [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger Jakob Hauser
@ 2023-05-08 11:35 ` Sebastian Reichel
2023-05-08 21:18 ` Jakob Hauser
0 siblings, 1 reply; 21+ messages in thread
From: Sebastian Reichel @ 2023-05-08 11:35 UTC (permalink / raw)
To: Jakob Hauser
Cc: Lee Jones, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Beomho Seo, Chanwoo Choi, Stephan Gerhold,
Raymond Hackley, Pavel Machek, Axel Lin, ChiYuan Huang,
Linus Walleij, Henrik Grimler, linux-pm, devicetree, linux-kernel,
phone-devel, ~postmarketos/upstreaming
[-- Attachment #1: Type: text/plain, Size: 2367 bytes --]
Hi,
On Sat, May 06, 2023 at 05:54:34PM +0200, Jakob Hauser wrote:
> The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger
> can, let's get this value.
>
> Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
> ---
> drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c
> index 5c04cf305219..a6520716d813 100644
> --- a/drivers/power/supply/rt5033_battery.c
> +++ b/drivers/power/supply/rt5033_battery.c
> @@ -12,6 +12,26 @@
> #include <linux/mfd/rt5033-private.h>
> #include <linux/mfd/rt5033.h>
>
> +static int rt5033_battery_get_status(struct i2c_client *client)
> +{
> + struct power_supply *charger;
> + union power_supply_propval val;
> + int ret;
> +
> + charger = power_supply_get_by_name("rt5033-charger");
> + if (!charger)
> + return POWER_SUPPLY_STATUS_UNKNOWN;
> +
> + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val);
> + if (ret) {
> + power_supply_put(charger);
> + return POWER_SUPPLY_STATUS_UNKNOWN;
> + }
struct rt5033_battery *battery = i2c_get_clientdata(client);
ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val);
if (ret)
val.intval = POWER_SUPPLY_STATUS_UNKNOWN;
> +
> + power_supply_put(charger);
> + return val.intval;
> +}
> +
> static int rt5033_battery_get_capacity(struct i2c_client *client)
> {
> struct rt5033_battery *battery = i2c_get_clientdata(client);
> @@ -84,6 +104,9 @@ static int rt5033_battery_get_property(struct power_supply *psy,
> case POWER_SUPPLY_PROP_CAPACITY:
> val->intval = rt5033_battery_get_capacity(battery->client);
> break;
> + case POWER_SUPPLY_PROP_STATUS:
> + val->intval = rt5033_battery_get_status(battery->client);
> + break;
> default:
> return -EINVAL;
> }
> @@ -96,6 +119,7 @@ static enum power_supply_property rt5033_battery_props[] = {
> POWER_SUPPLY_PROP_VOLTAGE_OCV,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_CAPACITY,
> + POWER_SUPPLY_PROP_STATUS,
> };
>
> static const struct regmap_config rt5033_battery_regmap_config = {
> --
> 2.39.2
>
Otherwise LGTM.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger
2023-05-08 11:35 ` Sebastian Reichel
@ 2023-05-08 21:18 ` Jakob Hauser
2023-05-08 22:06 ` Sebastian Reichel
0 siblings, 1 reply; 21+ messages in thread
From: Jakob Hauser @ 2023-05-08 21:18 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Beomho Seo, Chanwoo Choi, Stephan Gerhold,
Raymond Hackley, Pavel Machek, Axel Lin, ChiYuan Huang,
Linus Walleij, Henrik Grimler, linux-pm, devicetree, linux-kernel,
phone-devel, ~postmarketos/upstreaming
Hi Sebastian,
On 08.05.23 13:35, Sebastian Reichel wrote:
> Hi,
>
> On Sat, May 06, 2023 at 05:54:34PM +0200, Jakob Hauser wrote:
>> The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger
>> can, let's get this value.
>>
>> Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
>> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
>> ---
>> drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c
>> index 5c04cf305219..a6520716d813 100644
>> --- a/drivers/power/supply/rt5033_battery.c
>> +++ b/drivers/power/supply/rt5033_battery.c
>> @@ -12,6 +12,26 @@
>> #include <linux/mfd/rt5033-private.h>
>> #include <linux/mfd/rt5033.h>
>>
>> +static int rt5033_battery_get_status(struct i2c_client *client)
>> +{
>> + struct power_supply *charger;
>> + union power_supply_propval val;
>> + int ret;
>> +
>> + charger = power_supply_get_by_name("rt5033-charger");
>> + if (!charger)
>> + return POWER_SUPPLY_STATUS_UNKNOWN;
>> +
>> + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val);
>> + if (ret) {
>> + power_supply_put(charger);
>> + return POWER_SUPPLY_STATUS_UNKNOWN;
>> + }
>
> struct rt5033_battery *battery = i2c_get_clientdata(client);
> ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val);
> if (ret)
> val.intval = POWER_SUPPLY_STATUS_UNKNOWN;
I don't think this works. There is no direct relationship between
rt5033-charger and rt5033-battery. They operate independently from each
other.
I had a short try and the status property of rt5033-battery was "unknown".
Just for the record, the full function I tried was:
static int rt5033_battery_get_status(struct i2c_client *client)
{
struct rt5033_battery *battery = i2c_get_clientdata(client);
union power_supply_propval val;
int ret;
ret = power_supply_get_property_from_supplier(battery->psy,
POWER_SUPPLY_PROP_STATUS,
&val);
if (ret)
val.intval = POWER_SUPPLY_STATUS_UNKNOWN;
return val.intval;
}
Later on I added a read-out of the "ret" value. It is "-19". I guess
that's the "return -ENODEV;" from function
power_supply_get_property_from_supplier(). [2]
[2]
https://github.com/torvalds/linux/blob/v6.4-rc1/drivers/power/supply/power_supply_core.c#L397-L421
>> +
>> + power_supply_put(charger);
>> + return val.intval;
>> +}
>> +
>> static int rt5033_battery_get_capacity(struct i2c_client *client)
>> {
>> struct rt5033_battery *battery = i2c_get_clientdata(client);
>> @@ -84,6 +104,9 @@ static int rt5033_battery_get_property(struct power_supply *psy,
>> case POWER_SUPPLY_PROP_CAPACITY:
>> val->intval = rt5033_battery_get_capacity(battery->client);
>> break;
>> + case POWER_SUPPLY_PROP_STATUS:
>> + val->intval = rt5033_battery_get_status(battery->client);
>> + break;
>> default:
>> return -EINVAL;
>> }
>> @@ -96,6 +119,7 @@ static enum power_supply_property rt5033_battery_props[] = {
>> POWER_SUPPLY_PROP_VOLTAGE_OCV,
>> POWER_SUPPLY_PROP_PRESENT,
>> POWER_SUPPLY_PROP_CAPACITY,
>> + POWER_SUPPLY_PROP_STATUS,
>> };
>>
>> static const struct regmap_config rt5033_battery_regmap_config = {
>> --
>> 2.39.2
>>
>
> Otherwise LGTM.
>
> -- Sebastian
Kind regards,
Jakob
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger
2023-05-08 21:18 ` Jakob Hauser
@ 2023-05-08 22:06 ` Sebastian Reichel
2023-05-09 1:01 ` Jakob Hauser
0 siblings, 1 reply; 21+ messages in thread
From: Sebastian Reichel @ 2023-05-08 22:06 UTC (permalink / raw)
To: Jakob Hauser
Cc: Lee Jones, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Beomho Seo, Chanwoo Choi, Stephan Gerhold,
Raymond Hackley, Pavel Machek, Axel Lin, ChiYuan Huang,
Linus Walleij, Henrik Grimler, linux-pm, devicetree, linux-kernel,
phone-devel, ~postmarketos/upstreaming
[-- Attachment #1: Type: text/plain, Size: 4382 bytes --]
Hi,
On Mon, May 08, 2023 at 11:18:28PM +0200, Jakob Hauser wrote:
> On 08.05.23 13:35, Sebastian Reichel wrote:
> > On Sat, May 06, 2023 at 05:54:34PM +0200, Jakob Hauser wrote:
> > > The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger
> > > can, let's get this value.
> > >
> > > Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
> > > Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
> > > ---
> > > drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++
> > > 1 file changed, 24 insertions(+)
> > >
> > > diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c
> > > index 5c04cf305219..a6520716d813 100644
> > > --- a/drivers/power/supply/rt5033_battery.c
> > > +++ b/drivers/power/supply/rt5033_battery.c
> > > @@ -12,6 +12,26 @@
> > > #include <linux/mfd/rt5033-private.h>
> > > #include <linux/mfd/rt5033.h>
> > > +static int rt5033_battery_get_status(struct i2c_client *client)
> > > +{
> > > + struct power_supply *charger;
> > > + union power_supply_propval val;
> > > + int ret;
> > > +
> > > + charger = power_supply_get_by_name("rt5033-charger");
> > > + if (!charger)
> > > + return POWER_SUPPLY_STATUS_UNKNOWN;
> > > +
> > > + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val);
> > > + if (ret) {
> > > + power_supply_put(charger);
> > > + return POWER_SUPPLY_STATUS_UNKNOWN;
> > > + }
> >
> > struct rt5033_battery *battery = i2c_get_clientdata(client);
> > ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val);
> > if (ret)
> > val.intval = POWER_SUPPLY_STATUS_UNKNOWN;
>
> I don't think this works. There is no direct relationship between
> rt5033-charger and rt5033-battery. They operate independently from each
> other.
That should be fine as long as the supply dependency is properly declared.
> I had a short try and the status property of rt5033-battery was "unknown".
>
> Just for the record, the full function I tried was:
>
> static int rt5033_battery_get_status(struct i2c_client *client)
> {
> struct rt5033_battery *battery = i2c_get_clientdata(client);
> union power_supply_propval val;
> int ret;
>
> ret = power_supply_get_property_from_supplier(battery->psy,
> POWER_SUPPLY_PROP_STATUS,
> &val);
> if (ret)
> val.intval = POWER_SUPPLY_STATUS_UNKNOWN;
>
> return val.intval;
> }
>
> Later on I added a read-out of the "ret" value. It is "-19". I guess that's
> the "return -ENODEV;" from function
> power_supply_get_property_from_supplier(). [2]
>
> [2] https://github.com/torvalds/linux/blob/v6.4-rc1/drivers/power/supply/power_supply_core.c#L397-L421
I suppose your DT is missing the connection between the charger and
the battery:
rt5033_charger: charger {
compatible = "rt5033-charger";
...
}
fuel-gauge {
compatible = "rt5033-battery";
...
power-supplies = <&rt5033_charger>; // you are probably missing this
};
See also Documentation/devicetree/bindings/power/supply/power-supply.yaml
-- Sebastian
>
> > > +
> > > + power_supply_put(charger);
> > > + return val.intval;
> > > +}
> > > +
> > > static int rt5033_battery_get_capacity(struct i2c_client *client)
> > > {
> > > struct rt5033_battery *battery = i2c_get_clientdata(client);
> > > @@ -84,6 +104,9 @@ static int rt5033_battery_get_property(struct power_supply *psy,
> > > case POWER_SUPPLY_PROP_CAPACITY:
> > > val->intval = rt5033_battery_get_capacity(battery->client);
> > > break;
> > > + case POWER_SUPPLY_PROP_STATUS:
> > > + val->intval = rt5033_battery_get_status(battery->client);
> > > + break;
> > > default:
> > > return -EINVAL;
> > > }
> > > @@ -96,6 +119,7 @@ static enum power_supply_property rt5033_battery_props[] = {
> > > POWER_SUPPLY_PROP_VOLTAGE_OCV,
> > > POWER_SUPPLY_PROP_PRESENT,
> > > POWER_SUPPLY_PROP_CAPACITY,
> > > + POWER_SUPPLY_PROP_STATUS,
> > > };
> > > static const struct regmap_config rt5033_battery_regmap_config = {
> > > --
> > > 2.39.2
> > >
> >
> > Otherwise LGTM.
> >
> > -- Sebastian
>
> Kind regards,
> Jakob
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger
2023-05-08 22:06 ` Sebastian Reichel
@ 2023-05-09 1:01 ` Jakob Hauser
2023-05-09 7:25 ` Sebastian Reichel
0 siblings, 1 reply; 21+ messages in thread
From: Jakob Hauser @ 2023-05-09 1:01 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Beomho Seo, Chanwoo Choi, Stephan Gerhold,
Raymond Hackley, Pavel Machek, Axel Lin, ChiYuan Huang,
Linus Walleij, Henrik Grimler, linux-pm, devicetree, linux-kernel,
phone-devel, ~postmarketos/upstreaming
Hi Sebastian,
On 09.05.23 00:06, Sebastian Reichel wrote:
> Hi,
>
> On Mon, May 08, 2023 at 11:18:28PM +0200, Jakob Hauser wrote:
>> On 08.05.23 13:35, Sebastian Reichel wrote:
>>> On Sat, May 06, 2023 at 05:54:34PM +0200, Jakob Hauser wrote:
>>>> The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger
>>>> can, let's get this value.
>>>>
>>>> Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
>>>> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
>>>> ---
>>>> drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++
>>>> 1 file changed, 24 insertions(+)
>>>>
>>>> diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c
>>>> index 5c04cf305219..a6520716d813 100644
>>>> --- a/drivers/power/supply/rt5033_battery.c
>>>> +++ b/drivers/power/supply/rt5033_battery.c
>>>> @@ -12,6 +12,26 @@
>>>> #include <linux/mfd/rt5033-private.h>
>>>> #include <linux/mfd/rt5033.h>
>>>> +static int rt5033_battery_get_status(struct i2c_client *client)
>>>> +{
>>>> + struct power_supply *charger;
>>>> + union power_supply_propval val;
>>>> + int ret;
>>>> +
>>>> + charger = power_supply_get_by_name("rt5033-charger");
>>>> + if (!charger)
>>>> + return POWER_SUPPLY_STATUS_UNKNOWN;
>>>> +
>>>> + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val);
>>>> + if (ret) {
>>>> + power_supply_put(charger);
>>>> + return POWER_SUPPLY_STATUS_UNKNOWN;
>>>> + }
>>>
>>> struct rt5033_battery *battery = i2c_get_clientdata(client);
>>> ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val);
>>> if (ret)
>>> val.intval = POWER_SUPPLY_STATUS_UNKNOWN;
>>
>> I don't think this works. There is no direct relationship between
>> rt5033-charger and rt5033-battery. They operate independently from each
>> other.
>
> That should be fine as long as the supply dependency is properly declared.
>
>> I had a short try and the status property of rt5033-battery was "unknown".
>>
>> Just for the record, the full function I tried was:
>>
>> static int rt5033_battery_get_status(struct i2c_client *client)
>> {
>> struct rt5033_battery *battery = i2c_get_clientdata(client);
>> union power_supply_propval val;
>> int ret;
>>
>> ret = power_supply_get_property_from_supplier(battery->psy,
>> POWER_SUPPLY_PROP_STATUS,
>> &val);
>> if (ret)
>> val.intval = POWER_SUPPLY_STATUS_UNKNOWN;
>>
>> return val.intval;
>> }
>>
>> Later on I added a read-out of the "ret" value. It is "-19". I guess that's
>> the "return -ENODEV;" from function
>> power_supply_get_property_from_supplier(). [2]
>>
>> [2] https://github.com/torvalds/linux/blob/v6.4-rc1/drivers/power/supply/power_supply_core.c#L397-L421
>
> I suppose your DT is missing the connection between the charger and
> the battery:
>
> rt5033_charger: charger {
> compatible = "rt5033-charger";
> ...
> }
>
> fuel-gauge {
> compatible = "rt5033-battery";
> ...
> power-supplies = <&rt5033_charger>; // you are probably missing this
> };
>
> See also Documentation/devicetree/bindings/power/supply/power-supply.yaml
...
Thanks for the hints.
This leads to updating the dt-bindings because adding the
"power-supplies" property is important to be aware of.
Btw. first it didn't work. It took me quite some time to debug. I needed
to add "psy_cfg.of_node = client->dev.of_node;" to the rt5033-battery
probe function.
Now it works. However, there is a new problem. The battery driver gets
probed first. The charger driver a bit later. In the meantime the
battery driver spams dmesg with several "Failed to register power
supply" because the charger driver isn't available yet. Once the charger
driver is there, it works fine and dmesg becomes silent.
With the current state of the patchset:
dmesg | grep rt5033
[ 13.628030] rt5033 6-0034: Device found (rev. 6)
[ 13.633552] rt5033-led: Failed to locate of_node [id: -1]
[ 13.790478] rt5033-charger rt5033-charger: DMA mask not set
With the changes discussed here:
dmesg | grep rt5033
[ 15.741915] rt5033-battery 4-0035: Failed to register power supply
[ 15.752894] rt5033-battery 4-0035: Failed to register power supply
[ 15.795458] rt5033-battery 4-0035: Failed to register power supply
[ 15.910760] rt5033-battery 4-0035: Failed to register power supply
[ 15.913187] rt5033 6-0034: Device found (rev. 6)
[ 15.914341] rt5033-led: Failed to locate of_node [id: -1]
[ 15.920052] rt5033-battery 4-0035: Failed to register power supply
[ 15.927262] rt5033-battery 4-0035: Failed to register power supply
[ 16.017131] rt5033-battery 4-0035: Failed to register power supply
[ 16.017401] rt5033-charger rt5033-charger: DMA mask not set
The message is comming from the rt5033-battery probe function, it's the
power_supply_register() that fails.
Any ideas what could be done about this?
Kind regards,
Jakob
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger
2023-05-09 1:01 ` Jakob Hauser
@ 2023-05-09 7:25 ` Sebastian Reichel
2023-05-09 23:00 ` Jakob Hauser
0 siblings, 1 reply; 21+ messages in thread
From: Sebastian Reichel @ 2023-05-09 7:25 UTC (permalink / raw)
To: Jakob Hauser
Cc: Lee Jones, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Beomho Seo, Chanwoo Choi, Stephan Gerhold,
Raymond Hackley, Pavel Machek, Axel Lin, ChiYuan Huang,
Linus Walleij, Henrik Grimler, linux-pm, devicetree, linux-kernel,
phone-devel, ~postmarketos/upstreaming
[-- Attachment #1: Type: text/plain, Size: 5991 bytes --]
Hi,
On Tue, May 09, 2023 at 03:01:32AM +0200, Jakob Hauser wrote:
> On 09.05.23 00:06, Sebastian Reichel wrote:
> > On Mon, May 08, 2023 at 11:18:28PM +0200, Jakob Hauser wrote:
> > > On 08.05.23 13:35, Sebastian Reichel wrote:
> > > > On Sat, May 06, 2023 at 05:54:34PM +0200, Jakob Hauser wrote:
> > > > > The rt5033-battery fuelgauge can't get a status by itself. The rt5033-charger
> > > > > can, let's get this value.
> > > > >
> > > > > Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
> > > > > Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
> > > > > ---
> > > > > drivers/power/supply/rt5033_battery.c | 24 ++++++++++++++++++++++++
> > > > > 1 file changed, 24 insertions(+)
> > > > >
> > > > > diff --git a/drivers/power/supply/rt5033_battery.c b/drivers/power/supply/rt5033_battery.c
> > > > > index 5c04cf305219..a6520716d813 100644
> > > > > --- a/drivers/power/supply/rt5033_battery.c
> > > > > +++ b/drivers/power/supply/rt5033_battery.c
> > > > > @@ -12,6 +12,26 @@
> > > > > #include <linux/mfd/rt5033-private.h>
> > > > > #include <linux/mfd/rt5033.h>
> > > > > +static int rt5033_battery_get_status(struct i2c_client *client)
> > > > > +{
> > > > > + struct power_supply *charger;
> > > > > + union power_supply_propval val;
> > > > > + int ret;
> > > > > +
> > > > > + charger = power_supply_get_by_name("rt5033-charger");
> > > > > + if (!charger)
> > > > > + return POWER_SUPPLY_STATUS_UNKNOWN;
> > > > > +
> > > > > + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_STATUS, &val);
> > > > > + if (ret) {
> > > > > + power_supply_put(charger);
> > > > > + return POWER_SUPPLY_STATUS_UNKNOWN;
> > > > > + }
> > > >
> > > > struct rt5033_battery *battery = i2c_get_clientdata(client);
> > > > ret = power_supply_get_property_from_supplier(battery->psy, POWER_SUPPLY_PROP_STATUS, &val);
> > > > if (ret)
> > > > val.intval = POWER_SUPPLY_STATUS_UNKNOWN;
> > >
> > > I don't think this works. There is no direct relationship between
> > > rt5033-charger and rt5033-battery. They operate independently from each
> > > other.
> >
> > That should be fine as long as the supply dependency is properly declared.
> >
> > > I had a short try and the status property of rt5033-battery was "unknown".
> > >
> > > Just for the record, the full function I tried was:
> > >
> > > static int rt5033_battery_get_status(struct i2c_client *client)
> > > {
> > > struct rt5033_battery *battery = i2c_get_clientdata(client);
> > > union power_supply_propval val;
> > > int ret;
> > >
> > > ret = power_supply_get_property_from_supplier(battery->psy,
> > > POWER_SUPPLY_PROP_STATUS,
> > > &val);
> > > if (ret)
> > > val.intval = POWER_SUPPLY_STATUS_UNKNOWN;
> > >
> > > return val.intval;
> > > }
> > >
> > > Later on I added a read-out of the "ret" value. It is "-19". I guess that's
> > > the "return -ENODEV;" from function
> > > power_supply_get_property_from_supplier(). [2]
> > >
> > > [2] https://github.com/torvalds/linux/blob/v6.4-rc1/drivers/power/supply/power_supply_core.c#L397-L421
> >
> > I suppose your DT is missing the connection between the charger and
> > the battery:
> >
> > rt5033_charger: charger {
> > compatible = "rt5033-charger";
> > ...
> > }
> >
> > fuel-gauge {
> > compatible = "rt5033-battery";
> > ...
> > power-supplies = <&rt5033_charger>; // you are probably missing this
> > };
> >
> > See also Documentation/devicetree/bindings/power/supply/power-supply.yaml
>
> ...
>
> Thanks for the hints.
>
> This leads to updating the dt-bindings because adding the "power-supplies"
> property is important to be aware of.
It should already be part of the binding, because richtek,rt5033-battery.yaml has
allOf:
- $ref: power-supply.yaml#
> Btw. first it didn't work. It took me quite some time to debug. I needed to
> add "psy_cfg.of_node = client->dev.of_node;" to the rt5033-battery probe
> function.
>
> Now it works. However, there is a new problem. The battery driver gets
> probed first. The charger driver a bit later. In the meantime the battery
> driver spams dmesg with several "Failed to register power supply" because
> the charger driver isn't available yet. Once the charger driver is there, it
> works fine and dmesg becomes silent.
>
> With the current state of the patchset:
> dmesg | grep rt5033
> [ 13.628030] rt5033 6-0034: Device found (rev. 6)
> [ 13.633552] rt5033-led: Failed to locate of_node [id: -1]
> [ 13.790478] rt5033-charger rt5033-charger: DMA mask not set
>
> With the changes discussed here:
> dmesg | grep rt5033
> [ 15.741915] rt5033-battery 4-0035: Failed to register power supply
> [ 15.752894] rt5033-battery 4-0035: Failed to register power supply
> [ 15.795458] rt5033-battery 4-0035: Failed to register power supply
> [ 15.910760] rt5033-battery 4-0035: Failed to register power supply
> [ 15.913187] rt5033 6-0034: Device found (rev. 6)
> [ 15.914341] rt5033-led: Failed to locate of_node [id: -1]
> [ 15.920052] rt5033-battery 4-0035: Failed to register power supply
> [ 15.927262] rt5033-battery 4-0035: Failed to register power supply
> [ 16.017131] rt5033-battery 4-0035: Failed to register power supply
> [ 16.017401] rt5033-charger rt5033-charger: DMA mask not set
>
> The message is comming from the rt5033-battery probe function, it's the
> power_supply_register() that fails.
>
> Any ideas what could be done about this?
Replace the dev_err() with dev_err_probe():
if (IS_ERR(battery->psy))
return dev_err_probe(&client->dev, PTR_ERR(battery->psy), "Failed to register power supply\n");
That will avoid printing an error for -EPROBE_DEFER.
Greetings,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger
2023-05-09 7:25 ` Sebastian Reichel
@ 2023-05-09 23:00 ` Jakob Hauser
0 siblings, 0 replies; 21+ messages in thread
From: Jakob Hauser @ 2023-05-09 23:00 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Beomho Seo, Chanwoo Choi, Stephan Gerhold,
Raymond Hackley, Pavel Machek, Axel Lin, ChiYuan Huang,
Linus Walleij, Henrik Grimler, linux-pm, devicetree, linux-kernel,
phone-devel, ~postmarketos/upstreaming
Hi Sebastian,
On 09.05.23 09:25, Sebastian Reichel wrote:
> On Tue, May 09, 2023 at 03:01:32AM +0200, Jakob Hauser wrote:
>> On 09.05.23 00:06, Sebastian Reichel wrote:
...
>>> I suppose your DT is missing the connection between the charger and
>>> the battery:
>>>
>>> rt5033_charger: charger {
>>> compatible = "rt5033-charger";
>>> ...
>>> }
>>>
>>> fuel-gauge {
>>> compatible = "rt5033-battery";
>>> ...
>>> power-supplies = <&rt5033_charger>; // you are probably missing this
>>> };
>>>
>>> See also Documentation/devicetree/bindings/power/supply/power-supply.yaml
>>
>> ...
>>
>> Thanks for the hints.
>>
>> This leads to updating the dt-bindings because adding the "power-supplies"
>> property is important to be aware of.
>
> It should already be part of the binding, because richtek,rt5033-battery.yaml has
>
> allOf:
> - $ref: power-supply.yaml#
Uh, I see, you're two steps ahead ;)
>> Btw. first it didn't work. It took me quite some time to debug. I needed to
>> add "psy_cfg.of_node = client->dev.of_node;" to the rt5033-battery probe
>> function.
>>
>> Now it works. However, there is a new problem. The battery driver gets
>> probed first. The charger driver a bit later. In the meantime the battery
>> driver spams dmesg with several "Failed to register power supply" because
>> the charger driver isn't available yet. Once the charger driver is there, it
>> works fine and dmesg becomes silent.
>>
>> With the current state of the patchset:
>> dmesg | grep rt5033
>> [ 13.628030] rt5033 6-0034: Device found (rev. 6)
>> [ 13.633552] rt5033-led: Failed to locate of_node [id: -1]
>> [ 13.790478] rt5033-charger rt5033-charger: DMA mask not set
>>
>> With the changes discussed here:
>> dmesg | grep rt5033
>> [ 15.741915] rt5033-battery 4-0035: Failed to register power supply
>> [ 15.752894] rt5033-battery 4-0035: Failed to register power supply
>> [ 15.795458] rt5033-battery 4-0035: Failed to register power supply
>> [ 15.910760] rt5033-battery 4-0035: Failed to register power supply
>> [ 15.913187] rt5033 6-0034: Device found (rev. 6)
>> [ 15.914341] rt5033-led: Failed to locate of_node [id: -1]
>> [ 15.920052] rt5033-battery 4-0035: Failed to register power supply
>> [ 15.927262] rt5033-battery 4-0035: Failed to register power supply
>> [ 16.017131] rt5033-battery 4-0035: Failed to register power supply
>> [ 16.017401] rt5033-charger rt5033-charger: DMA mask not set
>>
>> The message is comming from the rt5033-battery probe function, it's the
>> power_supply_register() that fails.
>>
>> Any ideas what could be done about this?
>
> Replace the dev_err() with dev_err_probe():
>
> if (IS_ERR(battery->psy))
> return dev_err_probe(&client->dev, PTR_ERR(battery->psy), "Failed to register power supply\n");
>
> That will avoid printing an error for -EPROBE_DEFER.
Confirming, that works. Thanks!
Kind regards,
Jakob
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 8/8] dt-bindings: Add rt5033 mfd, regulator and charger
2023-05-06 15:54 ` [PATCH v4 0/8] Add RT5033 charger device driver Jakob Hauser
` (6 preceding siblings ...)
2023-05-06 15:54 ` [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger Jakob Hauser
@ 2023-05-06 15:54 ` Jakob Hauser
2023-05-09 9:18 ` Krzysztof Kozlowski
7 siblings, 1 reply; 21+ messages in thread
From: Jakob Hauser @ 2023-05-06 15:54 UTC (permalink / raw)
To: Sebastian Reichel, Lee Jones, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski
Cc: Beomho Seo, Chanwoo Choi, Stephan Gerhold, Raymond Hackley,
Pavel Machek, Axel Lin, ChiYuan Huang, Linus Walleij,
Henrik Grimler, linux-pm, devicetree, linux-kernel, phone-devel,
~postmarketos/upstreaming
Add device tree binding documentation for rt5033 multifunction device, voltage
regulator and battery charger.
Cc: Beomho Seo <beomho.seo@samsung.com>
Cc: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
The patch is based on next-20230505.
The drivers for rt5033 (mfd) and rt5033-regulator are existing. Whereas the
the driver rt5033-charger is new in this patchset.
.../bindings/mfd/richtek,rt5033.yaml | 113 ++++++++++++++++++
.../power/supply/richtek,rt5033-charger.yaml | 64 ++++++++++
2 files changed, 177 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/richtek,rt5033.yaml
create mode 100644 Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
diff --git a/Documentation/devicetree/bindings/mfd/richtek,rt5033.yaml b/Documentation/devicetree/bindings/mfd/richtek,rt5033.yaml
new file mode 100644
index 000000000000..0aa0a556b50f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/richtek,rt5033.yaml
@@ -0,0 +1,113 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/richtek,rt5033.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Richtek RT5033 Power Management Integrated Circuit
+
+maintainers:
+ - Jakob Hauser <jahau@rocketmail.com>
+
+description:
+ RT5033 is a multifunction device which includes battery charger, fuel gauge,
+ flash LED current source, LDO and synchronous Buck converter for portable
+ applications. It is interfaced to host controller using I2C interface. The
+ battery fuel gauge uses a separate I2C bus.
+
+properties:
+ compatible:
+ const: richtek,rt5033
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ regulators:
+ description:
+ The regulators of RT5033 have to be instantiated under a sub-node named
+ "regulators". For SAFE_LDO voltage there is only one value of 4.9 V. LDO
+ voltage ranges from 1.2 V to 3.0 V in 0.1 V steps. BUCK voltage ranges
+ from 1.0 V to 3.0 V in 0.1 V steps.
+ type: object
+ patternProperties:
+ "^(SAFE_LDO|LDO|BUCK)$":
+ type: object
+ $ref: /schemas/regulator/regulator.yaml#
+ unevaluatedProperties: false
+ additionalProperties: false
+
+ charger:
+ type: object
+ $ref: /schemas/power/supply/richtek,rt5033-charger.yaml#
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ battery: battery {
+ compatible = "simple-battery";
+ precharge-current-microamp = <450000>;
+ constant-charge-current-max-microamp = <1000000>;
+ charge-term-current-microamp = <150000>;
+ precharge-upper-limit-microvolt = <3500000>;
+ constant-charge-voltage-max-microvolt = <4350000>;
+ };
+
+ extcon {
+ usb_con: connector {
+ compatible = "usb-b-connector";
+ label = "micro-USB";
+ type = "micro";
+ };
+ };
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmic@34 {
+ compatible = "richtek,rt5033";
+ reg = <0x34>;
+
+ interrupt-parent = <&msmgpio>;
+ interrupts = <62 IRQ_TYPE_EDGE_FALLING>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pmic_int_default>;
+
+ regulators {
+ safe_ldo_reg: SAFE_LDO {
+ regulator-name = "SAFE_LDO";
+ regulator-min-microvolt = <4900000>;
+ regulator-max-microvolt = <4900000>;
+ regulator-always-on;
+ };
+ ldo_reg: LDO {
+ regulator-name = "LDO";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ };
+ buck_reg: BUCK {
+ regulator-name = "BUCK";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ };
+ };
+
+ charger {
+ compatible = "richtek,rt5033-charger";
+ monitored-battery = <&battery>;
+ connector = <&usb_con>;
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml b/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
new file mode 100644
index 000000000000..b8607cc6ec63
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/richtek,rt5033-charger.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Richtek RT5033 PIMC Battery Charger
+
+maintainers:
+ - Jakob Hauser <jahau@rocketmail.com>
+
+description:
+ The battery charger of the multifunction device RT5033 has to be instantiated
+ under sub-node named "charger" using the following format.
+
+properties:
+ compatible:
+ const: richtek,rt5033-charger
+
+ monitored-battery:
+ description: |
+ Phandle to the monitored battery according to battery.yaml. The battery
+ node needs to contain five parameters.
+
+ precharge-current-microamp:
+ Current of pre-charge mode. The pre-charge current levels are 350 mA
+ to 650 mA programmed by I2C per 100 mA.
+
+ constant-charge-current-max-microamp:
+ Current of fast-charge mode. The fast-charge current levels are 700 mA
+ to 2000 mA programmed by I2C per 100 mA.
+
+ charge-term-current-microamp:
+ This property is end of charge current. Its level ranges from 150 mA
+ to 600 mA. Between 150 mA and 300 mA in 50 mA steps, between 300 mA and
+ 600 mA in 100 mA steps.
+
+ precharge-upper-limit-microvolt:
+ Voltage of pre-charge mode. If the battery voltage is below the pre-charge
+ threshold voltage, the charger is in pre-charge mode with pre-charge
+ current. Its levels are 2.3 V to 3.8 V programmed by I2C per 0.1 V.
+
+ constant-charge-voltage-max-microvolt:
+ Battery regulation voltage of constant voltage mode. This voltage levels
+ from 3.65 V to 4.4 V by I2C per 0.025 V.
+
+ connector:
+ description:
+ Phandle to a USB connector according to usb-connector.yaml. The connector
+ should be a child of the extcon device.
+ maxItems: 1
+
+required:
+ - monitored-battery
+
+additionalProperties: false
+
+examples:
+ - |
+ charger {
+ compatible = "richtek,rt5033-charger";
+ monitored-battery = <&battery>;
+ connector = <&usb_con>;
+ };
--
2.39.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v4 8/8] dt-bindings: Add rt5033 mfd, regulator and charger
2023-05-06 15:54 ` [PATCH v4 8/8] dt-bindings: Add rt5033 mfd, regulator and charger Jakob Hauser
@ 2023-05-09 9:18 ` Krzysztof Kozlowski
2023-05-09 23:08 ` Jakob Hauser
0 siblings, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-09 9:18 UTC (permalink / raw)
To: Jakob Hauser, Sebastian Reichel, Lee Jones, Liam Girdwood,
Mark Brown, Rob Herring, Krzysztof Kozlowski
Cc: Beomho Seo, Chanwoo Choi, Stephan Gerhold, Raymond Hackley,
Pavel Machek, Axel Lin, ChiYuan Huang, Linus Walleij,
Henrik Grimler, linux-pm, devicetree, linux-kernel, phone-devel,
~postmarketos/upstreaming
On 06/05/2023 17:54, Jakob Hauser wrote:
> Add device tree binding documentation for rt5033 multifunction device, voltage
> regulator and battery charger.
>
> Cc: Beomho Seo <beomho.seo@samsung.com>
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> The patch is based on next-20230505.
>
> The drivers for rt5033 (mfd) and rt5033-regulator are existing. Whereas the
> the driver rt5033-charger is new in this patchset.
>
> .../bindings/mfd/richtek,rt5033.yaml | 113 ++++++++++++++++++
> .../power/supply/richtek,rt5033-charger.yaml | 64 ++++++++++
> 2 files changed, 177 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/richtek,rt5033.yaml
> create mode 100644 Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/richtek,rt5033.yaml b/Documentation/devicetree/bindings/mfd/richtek,rt5033.yaml
> new file mode 100644
> index 000000000000..0aa0a556b50f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/richtek,rt5033.yaml
> @@ -0,0 +1,113 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/richtek,rt5033.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RT5033 Power Management Integrated Circuit
> +
> +maintainers:
> + - Jakob Hauser <jahau@rocketmail.com>
> +
> +description:
> + RT5033 is a multifunction device which includes battery charger, fuel gauge,
> + flash LED current source, LDO and synchronous Buck converter for portable
> + applications. It is interfaced to host controller using I2C interface. The
> + battery fuel gauge uses a separate I2C bus.
> +
> +properties:
> + compatible:
> + const: richtek,rt5033
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + regulators:
> + description:
> + The regulators of RT5033 have to be instantiated under a sub-node named
> + "regulators". For SAFE_LDO voltage there is only one value of 4.9 V. LDO
> + voltage ranges from 1.2 V to 3.0 V in 0.1 V steps. BUCK voltage ranges
> + from 1.0 V to 3.0 V in 0.1 V steps.
> + type: object
> + patternProperties:
> + "^(SAFE_LDO|LDO|BUCK)$":
> + type: object
> + $ref: /schemas/regulator/regulator.yaml#
> + unevaluatedProperties: false
> + additionalProperties: false
> +
> + charger:
> + type: object
> + $ref: /schemas/power/supply/richtek,rt5033-charger.yaml#
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + battery: battery {
> + compatible = "simple-battery";
> + precharge-current-microamp = <450000>;
> + constant-charge-current-max-microamp = <1000000>;
> + charge-term-current-microamp = <150000>;
> + precharge-upper-limit-microvolt = <3500000>;
> + constant-charge-voltage-max-microvolt = <4350000>;
> + };
> +
> + extcon {
> + usb_con: connector {
> + compatible = "usb-b-connector";
> + label = "micro-USB";
> + type = "micro";
> + };
> + };
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pmic@34 {
> + compatible = "richtek,rt5033";
> + reg = <0x34>;
> +
> + interrupt-parent = <&msmgpio>;
> + interrupts = <62 IRQ_TYPE_EDGE_FALLING>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&pmic_int_default>;
> +
> + regulators {
> + safe_ldo_reg: SAFE_LDO {
> + regulator-name = "SAFE_LDO";
> + regulator-min-microvolt = <4900000>;
> + regulator-max-microvolt = <4900000>;
> + regulator-always-on;
> + };
> + ldo_reg: LDO {
> + regulator-name = "LDO";
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> + };
> + buck_reg: BUCK {
> + regulator-name = "BUCK";
> + regulator-min-microvolt = <1200000>;
> + regulator-max-microvolt = <1200000>;
> + };
> + };
> +
> + charger {
> + compatible = "richtek,rt5033-charger";
> + monitored-battery = <&battery>;
> + connector = <&usb_con>;
> + };
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml b/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
> new file mode 100644
> index 000000000000..b8607cc6ec63
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/richtek,rt5033-charger.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RT5033 PIMC Battery Charger
PMIC
> +
> +maintainers:
> + - Jakob Hauser <jahau@rocketmail.com>
> +
> +description:
> + The battery charger of the multifunction device RT5033 has to be instantiated
> + under sub-node named "charger" using the following format.
> +
> +properties:
> + compatible:
> + const: richtek,rt5033-charger
> +
> + monitored-battery:
> + description: |
> + Phandle to the monitored battery according to battery.yaml. The battery
> + node needs to contain five parameters.
> +
> + precharge-current-microamp:
> + Current of pre-charge mode. The pre-charge current levels are 350 mA
> + to 650 mA programmed by I2C per 100 mA.
> +
> + constant-charge-current-max-microamp:
> + Current of fast-charge mode. The fast-charge current levels are 700 mA
> + to 2000 mA programmed by I2C per 100 mA.
> +
> + charge-term-current-microamp:
> + This property is end of charge current. Its level ranges from 150 mA
> + to 600 mA. Between 150 mA and 300 mA in 50 mA steps, between 300 mA and
> + 600 mA in 100 mA steps.
> +
> + precharge-upper-limit-microvolt:
> + Voltage of pre-charge mode. If the battery voltage is below the pre-charge
> + threshold voltage, the charger is in pre-charge mode with pre-charge
> + current. Its levels are 2.3 V to 3.8 V programmed by I2C per 0.1 V.
> +
> + constant-charge-voltage-max-microvolt:
> + Battery regulation voltage of constant voltage mode. This voltage levels
> + from 3.65 V to 4.4 V by I2C per 0.025 V.
> +
> + connector:
> + description:
> + Phandle to a USB connector according to usb-connector.yaml. The connector
> + should be a child of the extcon device.
> + maxItems: 1
Missing type/ref... but then you will notice you have conflicting ref
with existing connector. connector is usb-connector.yaml, not phandle. I
am not sure if we need generic property, so let's go with device
specific - richtek,usb-connector
> +
> +required:
> + - monitored-battery
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 8/8] dt-bindings: Add rt5033 mfd, regulator and charger
2023-05-09 9:18 ` Krzysztof Kozlowski
@ 2023-05-09 23:08 ` Jakob Hauser
0 siblings, 0 replies; 21+ messages in thread
From: Jakob Hauser @ 2023-05-09 23:08 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Lee Jones, Liam Girdwood, Mark Brown,
Rob Herring, Beomho Seo, Chanwoo Choi, Stephan Gerhold,
Raymond Hackley, Pavel Machek, Axel Lin, ChiYuan Huang,
Linus Walleij, Henrik Grimler, linux-pm, devicetree, linux-kernel,
phone-devel, ~postmarketos/upstreaming
Hi Krzysztof,
On 09.05.23 11:18, Krzysztof Kozlowski wrote:
> On 06/05/2023 17:54, Jakob Hauser wrote:
...
>> diff --git a/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml b/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
>> new file mode 100644
>> index 000000000000..b8607cc6ec63
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
>> @@ -0,0 +1,64 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/power/supply/richtek,rt5033-charger.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Richtek RT5033 PIMC Battery Charger
>
> PMIC
Good catch.
>> +
>> +maintainers:
>> + - Jakob Hauser <jahau@rocketmail.com>
>> +
>> +description:
>> + The battery charger of the multifunction device RT5033 has to be instantiated
>> + under sub-node named "charger" using the following format.
>> +
>> +properties:
>> + compatible:
>> + const: richtek,rt5033-charger
>> +
>> + monitored-battery:
>> + description: |
>> + Phandle to the monitored battery according to battery.yaml. The battery
>> + node needs to contain five parameters.
>> +
>> + precharge-current-microamp:
>> + Current of pre-charge mode. The pre-charge current levels are 350 mA
>> + to 650 mA programmed by I2C per 100 mA.
>> +
>> + constant-charge-current-max-microamp:
>> + Current of fast-charge mode. The fast-charge current levels are 700 mA
>> + to 2000 mA programmed by I2C per 100 mA.
>> +
>> + charge-term-current-microamp:
>> + This property is end of charge current. Its level ranges from 150 mA
>> + to 600 mA. Between 150 mA and 300 mA in 50 mA steps, between 300 mA and
>> + 600 mA in 100 mA steps.
>> +
>> + precharge-upper-limit-microvolt:
>> + Voltage of pre-charge mode. If the battery voltage is below the pre-charge
>> + threshold voltage, the charger is in pre-charge mode with pre-charge
>> + current. Its levels are 2.3 V to 3.8 V programmed by I2C per 0.1 V.
>> +
>> + constant-charge-voltage-max-microvolt:
>> + Battery regulation voltage of constant voltage mode. This voltage levels
>> + from 3.65 V to 4.4 V by I2C per 0.025 V.
>> +
>> + connector:
>> + description:
>> + Phandle to a USB connector according to usb-connector.yaml. The connector
>> + should be a child of the extcon device.
>> + maxItems: 1
>
> Missing type/ref... but then you will notice you have conflicting ref
> with existing connector. connector is usb-connector.yaml, not phandle. I
> am not sure if we need generic property, so let's go with device
> specific - richtek,usb-connector
Actually, as we already switched to the connector thing, I would have
liked to do it properly, if at all. Again looking for examples how it
should be done, the usb-connector.yaml schema gets mostly used in the
USB and PHY domain. Those examples don't fit the specific case well. I
didn't find examples in the power supply domain.
Therefore, giving in, I'll chage it to richtek,usb-connector.
It's still hard for me to understand why phandle excton shouldn't fit
the specific simple case. Now we have neither a proper "connector" thing
nor a simple extcon phandle.
...
Kind regards,
Jakob
^ permalink raw reply [flat|nested] 21+ messages in thread