* [PATCH 0/3] power: supply: twl6030/32 charger
@ 2024-09-18 8:41 Andreas Kemnade
2024-09-18 8:41 ` [PATCH 1/3] dt-bindings: power: supply: Add TI TWL603X charger Andreas Kemnade
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Andreas Kemnade @ 2024-09-18 8:41 UTC (permalink / raw)
To: tony, Sebastian Reichel, linux-omap, devicetree, Lee Jones,
Rob Herring, Krzysztof Kozlowski, linux-kernel, Conor Dooley,
linux-pm
Cc: Andreas Kemnade
Add basic support for the charger in the TWL6030/32. Supported is the USB
path. AC path is not handled yet, also there is no entry yet
in /sys/class/power_supply with type battery yet.
Without this series, devices will happily drain battery when running
on mainline.
Andreas Kemnade (3):
dt-bindings: power: supply: Add TI TWL603X charger
dt-bindings: mfd: twl: add charger node also for TWL603x
power: supply: initial support for TWL6030/32
.../devicetree/bindings/mfd/ti,twl.yaml | 18 +
.../power/supply/ti,twl6030-charger.yaml | 62 ++
drivers/power/supply/Kconfig | 10 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/twl6030_charger.c | 566 ++++++++++++++++++
5 files changed, 657 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml
create mode 100644 drivers/power/supply/twl6030_charger.c
--
2.39.2
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dt-bindings: power: supply: Add TI TWL603X charger
2024-09-18 8:41 [PATCH 0/3] power: supply: twl6030/32 charger Andreas Kemnade
@ 2024-09-18 8:41 ` Andreas Kemnade
2024-09-18 10:46 ` Krzysztof Kozlowski
2024-09-18 8:41 ` [PATCH 2/3] dt-bindings: mfd: twl: add charger node also for TWL603x Andreas Kemnade
2024-09-18 8:41 ` [PATCH 3/3] power: supply: initial support for TWL6030/32 Andreas Kemnade
2 siblings, 1 reply; 15+ messages in thread
From: Andreas Kemnade @ 2024-09-18 8:41 UTC (permalink / raw)
To: tony, Sebastian Reichel, linux-omap, devicetree, Lee Jones,
Rob Herring, Krzysztof Kozlowski, linux-kernel, Conor Dooley,
linux-pm
Cc: Andreas Kemnade
Use a fallback compatible since for especially for generic
defensive setup of parameters, both 6030 and 6032 are the same and
U-Boot actually uses a generic 6030/32 function to enable the
charger.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
.../power/supply/ti,twl6030-charger.yaml | 62 +++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml
diff --git a/Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml b/Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml
new file mode 100644
index 0000000000000..fe0fe9a78761c
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/ti,twl6030-charger.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TWL6030/32 BCI (Battery Charger Interface)
+
+description: |
+ The battery charger needs to be configured to do any charging besides of
+ precharging. The GPADC in the PMIC has to be used to get the related
+ voltages.
+
+maintainers:
+ - Andreas Kemnade <andreas@kemnade.info>
+
+allOf:
+ - $ref: power-supply.yaml#
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - const: "ti,twl6030-charger"
+ - items:
+ - const: "ti,twl6032-charger"
+ - const: "ti,twl6030-charger"
+
+ interrupts:
+ minItems: 2
+ maxItems: 2
+
+ io-channels:
+ items:
+ - description: VBUS Voltage Channel
+
+ io-channel-names:
+ items:
+ - const: vusb
+
+ monitored-battery:
+ description:
+ phandle of battery characteristics devicetree node
+
+required:
+ - compatible
+ - interrupts
+ - monitored-battery
+
+additionalProperties: false
+
+examples:
+ - |
+ pmic {
+ bci {
+ compatible = "ti,twl6032-charger", "ti,twl6030-charger";
+ interrupts = <2>, <5>;
+ io-channels = <&gpadc 10>;
+ io-channel-names = "vusb";
+ monitored-battery = <&bat>;
+ };
+ };
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] dt-bindings: mfd: twl: add charger node also for TWL603x
2024-09-18 8:41 [PATCH 0/3] power: supply: twl6030/32 charger Andreas Kemnade
2024-09-18 8:41 ` [PATCH 1/3] dt-bindings: power: supply: Add TI TWL603X charger Andreas Kemnade
@ 2024-09-18 8:41 ` Andreas Kemnade
2024-09-18 10:47 ` Krzysztof Kozlowski
2024-09-18 8:41 ` [PATCH 3/3] power: supply: initial support for TWL6030/32 Andreas Kemnade
2 siblings, 1 reply; 15+ messages in thread
From: Andreas Kemnade @ 2024-09-18 8:41 UTC (permalink / raw)
To: tony, Sebastian Reichel, linux-omap, devicetree, Lee Jones,
Rob Herring, Krzysztof Kozlowski, linux-kernel, Conor Dooley,
linux-pm
Cc: Andreas Kemnade
Also the TWL603X devices have a charger, so allow to specify it here.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
.../devicetree/bindings/mfd/ti,twl.yaml | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
index e94b0fd7af0f8..4064a228cb0fc 100644
--- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
+++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
@@ -105,6 +105,11 @@ allOf:
regulator-initial-mode: false
properties:
+ bci:
+ type: object
+ properties:
+ compatible:
+ const: ti,twl6030-charger
gpadc:
type: object
properties:
@@ -136,6 +141,13 @@ allOf:
regulator-initial-mode: false
properties:
+ bci:
+ type: object
+ properties:
+ compatible:
+ items:
+ - const: ti,twl6032-charger
+ - const: ti,twl6030-charger
gpadc:
type: object
properties:
@@ -222,6 +234,12 @@ examples:
interrupt-controller;
#interrupt-cells = <1>;
+ bci {
+ compatible = "ti,twl6030-charger";
+ interrupts = <2>, <5>;
+ monitored-battery = <&bat>;
+ };
+
gpadc {
compatible = "ti,twl6030-gpadc";
interrupts = <6>;
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] power: supply: initial support for TWL6030/32
2024-09-18 8:41 [PATCH 0/3] power: supply: twl6030/32 charger Andreas Kemnade
2024-09-18 8:41 ` [PATCH 1/3] dt-bindings: power: supply: Add TI TWL603X charger Andreas Kemnade
2024-09-18 8:41 ` [PATCH 2/3] dt-bindings: mfd: twl: add charger node also for TWL603x Andreas Kemnade
@ 2024-09-18 8:41 ` Andreas Kemnade
2024-09-18 10:43 ` Krzysztof Kozlowski
2024-09-18 21:11 ` kernel test robot
2 siblings, 2 replies; 15+ messages in thread
From: Andreas Kemnade @ 2024-09-18 8:41 UTC (permalink / raw)
To: tony, Sebastian Reichel, linux-omap, devicetree, Lee Jones,
Rob Herring, Krzysztof Kozlowski, linux-kernel, Conor Dooley,
linux-pm
Cc: Andreas Kemnade
Add a driver for the charger in the TWL6030/32. For now it does not report
much in sysfs but parameters are set up for USB, charging is enabled with
the specified parameters. It stops charging when full and also restarts
charging.
This prevents ending up in a system setup where you run out of battery
although a charger is plugged in after precharge completed.
Battery voltage behavior was checked via the GPADC.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
drivers/power/supply/Kconfig | 10 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/twl6030_charger.c | 566 +++++++++++++++++++++++++
3 files changed, 577 insertions(+)
create mode 100644 drivers/power/supply/twl6030_charger.c
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index bcfa63fb9f1e2..9f2eef6787f7a 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -493,6 +493,16 @@ config CHARGER_TWL4030
help
Say Y here to enable support for TWL4030 Battery Charge Interface.
+config CHARGER_TWL6030
+ tristate "OMAP TWL6030 BCI charger driver"
+ depends on IIO && TWL4030_CORE
+ help
+ Say Y here to enable support for TWL6030/6032 Battery Charge
+ Interface.
+
+ This driver can be build as a module. If so, the module will be
+ called twl6030_charger.
+
config CHARGER_LP8727
tristate "TI/National Semiconductor LP8727 charger driver"
depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 8dcb415453171..59c4a9f40d28a 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_CHARGER_CPCAP) += cpcap-charger.o
obj-$(CONFIG_CHARGER_ISP1704) += isp1704_charger.o
obj-$(CONFIG_CHARGER_MAX8903) += max8903_charger.o
obj-$(CONFIG_CHARGER_TWL4030) += twl4030_charger.o
+obj-$(CONFIG_CHARGER_TWL6030) += twl6030_charger.o
obj-$(CONFIG_CHARGER_LP8727) += lp8727_charger.o
obj-$(CONFIG_CHARGER_LP8788) += lp8788-charger.o
obj-$(CONFIG_CHARGER_GPIO) += gpio-charger.o
diff --git a/drivers/power/supply/twl6030_charger.c b/drivers/power/supply/twl6030_charger.c
new file mode 100644
index 0000000000000..5704b8ca9d22b
--- /dev/null
+++ b/drivers/power/supply/twl6030_charger.c
@@ -0,0 +1,566 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TWL6030 charger
+ *
+ * Copyright (C) 2024 Andreas Kemnade <andreas@kemnade.info>
+ *
+ * based on older 6030 driver found in a v3.0 vendor kernel
+ *
+ * based on twl4030_bci_battery.c by TI
+ * Copyright (C) 2008 Texas Instruments, Inc.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/bits.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/twl.h>
+#include <linux/power_supply.h>
+#include <linux/notifier.h>
+#include <linux/usb/otg.h>
+#include <linux/iio/consumer.h>
+#include <linux/devm-helpers.h>
+
+#define CONTROLLER_INT_MASK 0x00
+#define CONTROLLER_CTRL1 0x01
+#define CONTROLLER_WDG 0x02
+#define CONTROLLER_STAT1 0x03
+#define CHARGERUSB_INT_STATUS 0x04
+#define CHARGERUSB_INT_MASK 0x05
+#define CHARGERUSB_STATUS_INT1 0x06
+#define CHARGERUSB_STATUS_INT2 0x07
+#define CHARGERUSB_CTRL1 0x08
+#define CHARGERUSB_CTRL2 0x09
+#define CHARGERUSB_CTRL3 0x0A
+#define CHARGERUSB_STAT1 0x0B
+#define CHARGERUSB_VOREG 0x0C
+#define CHARGERUSB_VICHRG 0x0D
+#define CHARGERUSB_CINLIMIT 0x0E
+#define CHARGERUSB_CTRLLIMIT1 0x0F
+#define CHARGERUSB_CTRLLIMIT2 0x10
+#define ANTICOLLAPSE_CTRL1 0x11
+#define ANTICOLLAPSE_CTRL2 0x12
+
+/* TWL6032 registers 0xDA to 0xDE - TWL6032_MODULE_CHARGER */
+#define CONTROLLER_CTRL2 0x00
+#define CONTROLLER_VSEL_COMP 0x01
+#define CHARGERUSB_VSYSREG 0x02
+#define CHARGERUSB_VICHRG_PC 0x03
+#define LINEAR_CHRG_STS 0x04
+
+#define LINEAR_CHRG_STS_CRYSTL_OSC_OK 0x40
+#define LINEAR_CHRG_STS_END_OF_CHARGE 0x20
+#define LINEAR_CHRG_STS_VBATOV 0x10
+#define LINEAR_CHRG_STS_VSYSOV 0x08
+#define LINEAR_CHRG_STS_DPPM_STS 0x04
+#define LINEAR_CHRG_STS_CV_STS 0x02
+#define LINEAR_CHRG_STS_CC_STS 0x01
+
+#define FG_REG_00 0x00
+#define FG_REG_01 0x01
+#define FG_REG_02 0x02
+#define FG_REG_03 0x03
+#define FG_REG_04 0x04
+#define FG_REG_05 0x05
+#define FG_REG_06 0x06
+#define FG_REG_07 0x07
+#define FG_REG_08 0x08
+#define FG_REG_09 0x09
+#define FG_REG_10 0x0A
+#define FG_REG_11 0x0B
+
+/* CONTROLLER_INT_MASK */
+#define MVAC_FAULT BIT(7)
+#define MAC_EOC BIT(6)
+#define LINCH_GATED BIT(5)
+#define MBAT_REMOVED BIT(4)
+#define MFAULT_WDG BIT(3)
+#define MBAT_TEMP BIT(2)
+#define MVBUS_DET BIT(1)
+#define MVAC_DET BIT(0)
+
+/* CONTROLLER_CTRL1 */
+#define CONTROLLER_CTRL1_EN_LINCH BIT(5)
+#define CONTROLLER_CTRL1_EN_CHARGER BIT(4)
+#define CONTROLLER_CTRL1_SEL_CHARGER BIT(3)
+
+/* CONTROLLER_STAT1 */
+#define CONTROLLER_STAT1_EXTCHRG_STATZ BIT(7)
+#define CONTROLLER_STAT1_LINCH_GATED BIT(6)
+#define CONTROLLER_STAT1_CHRG_DET_N BIT(5)
+#define CONTROLLER_STAT1_FAULT_WDG BIT(4)
+#define CONTROLLER_STAT1_VAC_DET BIT(3)
+#define VAC_DET BIT(3)
+#define CONTROLLER_STAT1_VBUS_DET BIT(2)
+#define VBUS_DET BIT(2)
+#define CONTROLLER_STAT1_BAT_REMOVED BIT(1)
+#define CONTROLLER_STAT1_BAT_TEMP_OVRANGE BIT(0)
+
+/* CHARGERUSB_INT_STATUS */
+#define EN_LINCH BIT(4)
+#define CURRENT_TERM_INT BIT(3)
+#define CHARGERUSB_STAT BIT(2)
+#define CHARGERUSB_THMREG BIT(1)
+#define CHARGERUSB_FAULT BIT(0)
+
+/* CHARGERUSB_INT_MASK */
+#define MASK_MCURRENT_TERM BIT(3)
+#define MASK_MCHARGERUSB_STAT BIT(2)
+#define MASK_MCHARGERUSB_THMREG BIT(1)
+#define MASK_MCHARGERUSB_FAULT BIT(0)
+
+/* CHARGERUSB_STATUS_INT1 */
+#define CHARGERUSB_STATUS_INT1_TMREG BIT(7)
+#define CHARGERUSB_STATUS_INT1_NO_BAT BIT(6)
+#define CHARGERUSB_STATUS_INT1_BST_OCP BIT(5)
+#define CHARGERUSB_STATUS_INT1_TH_SHUTD BIT(4)
+#define CHARGERUSB_STATUS_INT1_BAT_OVP BIT(3)
+#define CHARGERUSB_STATUS_INT1_POOR_SRC BIT(2)
+#define CHARGERUSB_STATUS_INT1_SLP_MODE BIT(1)
+#define CHARGERUSB_STATUS_INT1_VBUS_OVP BIT(0)
+
+/* CHARGERUSB_STATUS_INT2 */
+#define ICCLOOP BIT(3)
+#define CURRENT_TERM BIT(2)
+#define CHARGE_DONE BIT(1)
+#define ANTICOLLAPSE BIT(0)
+
+/* CHARGERUSB_CTRL1 */
+#define SUSPEND_BOOT BIT(7)
+#define OPA_MODE BIT(6)
+#define HZ_MODE BIT(5)
+#define TERM BIT(4)
+
+/* CHARGERUSB_CTRL2 */
+#define UA_TO_VITERM(x) (((x) / 50000 - 1) << 5)
+
+/* CHARGERUSB_CTRL3 */
+#define VBUSCHRG_LDO_OVRD BIT(7)
+#define CHARGE_ONCE BIT(6)
+#define BST_HW_PR_DIS BIT(5)
+#define AUTOSUPPLY BIT(3)
+#define BUCK_HSILIM BIT(0)
+
+/* CHARGERUSB_VOREG */
+#define UV_TO_VOREG(x) (((x) - 3500000) / 20000)
+#define VOREG_TO_UV(x) (((x) & 0x3F) * 20000 + 3500000)
+#define CHARGERUSB_VOREG_3P52 0x01
+#define CHARGERUSB_VOREG_4P0 0x19
+#define CHARGERUSB_VOREG_4P2 0x23
+#define CHARGERUSB_VOREG_4P76 0x3F
+
+/* CHARGERUSB_VICHRG */
+/*
+ * might be inaccurate for < 500 mA, diffent scale might apply,
+ * either starting from 100 mA or 300 mA
+ */
+#define UA_TO_VICHRG(x) (((x) / 100000) - 1)
+#define VICHRG_TO_UA(x) (((x) & 0xf) * 100000 + 100000)
+
+/* CHARGERUSB_CINLIMIT */
+#define CHARGERUSB_CIN_LIMIT_100 0x1
+#define CHARGERUSB_CIN_LIMIT_300 0x5
+#define CHARGERUSB_CIN_LIMIT_500 0x9
+#define CHARGERUSB_CIN_LIMIT_NONE 0xF
+
+/* CHARGERUSB_CTRLLIMIT2 */
+#define CHARGERUSB_CTRLLIMIT2_1500 0x0E
+#define LOCK_LIMIT BIT(4)
+
+/* ANTICOLLAPSE_CTRL2 */
+#define BUCK_VTH_SHIFT 5
+
+/* FG_REG_00 */
+#define CC_ACTIVE_MODE_SHIFT 6
+#define CC_AUTOCLEAR BIT(2)
+#define CC_CAL_EN BIT(1)
+#define CC_PAUSE BIT(0)
+
+#define REG_TOGGLE1 0x90
+#define REG_PWDNSTATUS1 0x93
+#define FGDITHS BIT(7)
+#define FGDITHR BIT(6)
+#define FGS BIT(5)
+#define FGR BIT(4)
+#define BBSPOR_CFG 0xE6
+#define BB_CHG_EN BIT(3)
+
+struct twl6030_charger_info {
+ struct device *dev;
+ struct power_supply *usb;
+ struct power_supply_battery_info *binfo;
+ struct work_struct work;
+ int irq_chg;
+ int input_current_limit;
+ struct iio_channel *channel_vusb;
+ struct delayed_work charger_monitor;
+};
+
+static int twl6030_charger_read(u8 reg, u8 *val)
+{
+ return twl_i2c_read_u8(TWL_MODULE_MAIN_CHARGE, val, reg);
+}
+
+static int twl6030_charger_write(u8 reg, u8 val)
+{
+ return twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, val, reg);
+}
+
+static int twl6030_config_cinlimit_reg(struct twl6030_charger_info *charger,
+ unsigned int ua)
+{
+ if (ua >= 50000 && ua <= 750000) {
+ ua = (ua - 50000) / 50000;
+ } else if ((ua > 750000) && (ua <= 1500000) &&
+ (device_is_compatible(charger->dev, "ti,twl6032-charger"))) {
+ ua = ((ua % 100000) ? 0x30 : 0x20) + ((ua - 100000) / 100000);
+ } else {
+ if (ua < 50000) {
+ dev_err(charger->dev, "invalid input current limit\n");
+ return -EINVAL;
+ }
+ /* This is no current limit */
+ ua = 0x0F;
+ }
+
+ return twl6030_charger_write(CHARGERUSB_CINLIMIT, ua);
+}
+
+/*
+ * rewriting all stuff here, resets to extremely conservative defaults were
+ * seen under some circumstances, like charge voltage to 3.5V
+ */
+static int twl6030_enable_usb(struct twl6030_charger_info *charger)
+{
+ int ret;
+
+ ret = twl6030_charger_write(CHARGERUSB_VICHRG,
+ UA_TO_VICHRG(charger->binfo->constant_charge_current_max_ua));
+ if (ret < 0)
+ return ret;
+
+ ret = twl6030_charger_write(CONTROLLER_WDG, 0xff);
+ if (ret < 0)
+ return ret;
+
+ charger->input_current_limit = 500000;
+ ret = twl6030_config_cinlimit_reg(charger, charger->input_current_limit);
+ if (ret < 0)
+ return ret;
+
+ ret = twl6030_charger_write(CHARGERUSB_CINLIMIT, CHARGERUSB_CIN_LIMIT_500);
+ if (ret < 0)
+ return ret;
+
+ ret = twl6030_charger_write(CHARGERUSB_VOREG,
+ UV_TO_VOREG(charger->binfo->constant_charge_voltage_max_uv));
+ if (ret < 0)
+ return ret;
+
+ ret = twl6030_charger_write(CHARGERUSB_CTRL1, TERM);
+ if (ret < 0)
+ return ret;
+
+ if (charger->binfo->charge_term_current_ua != -EINVAL) {
+ ret = twl6030_charger_write(CHARGERUSB_CTRL2,
+ UA_TO_VITERM(charger->binfo->charge_term_current_ua));
+ if (ret < 0)
+ return ret;
+ }
+
+ return twl6030_charger_write(CONTROLLER_CTRL1, CONTROLLER_CTRL1_EN_CHARGER);
+}
+
+static void twl6030_charger_wdg(struct work_struct *data)
+{
+ struct twl6030_charger_info *charger =
+ container_of(data, struct twl6030_charger_info,
+ charger_monitor.work);
+
+ u8 val;
+ u8 int_stat;
+ u8 stat_int1;
+ u8 stat_int2;
+
+ twl6030_charger_read(CONTROLLER_STAT1, &val);
+ twl6030_charger_read(CHARGERUSB_INT_STATUS, &int_stat);
+ twl6030_charger_read(CHARGERUSB_STATUS_INT1, &stat_int1);
+ twl6030_charger_read(CHARGERUSB_STATUS_INT2, &stat_int2);
+ dev_dbg(charger->dev,
+ "wdg: stat1: %02x %s INT_STATUS %02x STATUS_INT1 %02x STATUS_INT2 %02x\n",
+ val, (val & VBUS_DET) ? "usb online" : "usb offline",
+ int_stat, stat_int1, stat_int2);
+
+ twl6030_charger_write(CONTROLLER_WDG, 0xff);
+ schedule_delayed_work(&charger->charger_monitor,
+ msecs_to_jiffies(10000));
+}
+
+static irqreturn_t twl6030_charger_interrupt(int irq, void *arg)
+{
+ struct twl6030_charger_info *charger = arg;
+ u8 val;
+ u8 int_stat;
+ u8 stat_int1;
+ u8 stat_int2;
+
+ if (twl6030_charger_read(CONTROLLER_STAT1, &val) < 0)
+ return IRQ_HANDLED;
+
+ if (twl6030_charger_read(CHARGERUSB_INT_STATUS, &int_stat) < 0)
+ return IRQ_HANDLED;
+
+ if (twl6030_charger_read(CHARGERUSB_STATUS_INT1, &stat_int1) < 0)
+ return IRQ_HANDLED;
+
+ if (twl6030_charger_read(CHARGERUSB_STATUS_INT2, &stat_int2) < 0)
+ return IRQ_HANDLED;
+
+ dev_dbg(charger->dev,
+ "charger irq: stat1: %02x %s INT_STATUS %02x STATUS_INT1 %02x STATUS_INT2 %02x\n",
+ val, (val & VBUS_DET) ? "usb online" : "usb offline",
+ int_stat, stat_int1, stat_int2);
+ power_supply_changed(charger->usb);
+
+ if (val & VBUS_DET) {
+ if (twl6030_charger_read(CONTROLLER_CTRL1, &val) < 0)
+ return IRQ_HANDLED;
+
+ if (!(val & CONTROLLER_CTRL1_EN_CHARGER)) {
+ if (twl6030_enable_usb(charger) < 0)
+ return IRQ_HANDLED;
+
+ schedule_delayed_work(&charger->charger_monitor,
+ msecs_to_jiffies(10000));
+ }
+ } else {
+ cancel_delayed_work(&charger->charger_monitor);
+ }
+ return IRQ_HANDLED;
+}
+
+static int twl6030_charger_usb_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct twl6030_charger_info *charger = power_supply_get_drvdata(psy);
+ int ret;
+ u8 stat1;
+
+ ret = twl6030_charger_read(CONTROLLER_STAT1, &stat1);
+ if (ret)
+ return ret;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_STATUS:
+ u8 intstat;
+
+ if (!(stat1 & VBUS_DET)) {
+ val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ break;
+ }
+ ret = twl6030_charger_read(CHARGERUSB_STATUS_INT2, &intstat);
+ if (ret)
+ return ret;
+
+ if (intstat & CHARGE_DONE)
+ val->intval = POWER_SUPPLY_STATUS_FULL;
+ else if (intstat & CURRENT_TERM)
+ val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ else
+ val->intval = POWER_SUPPLY_STATUS_CHARGING;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ if (!charger->channel_vusb)
+ return -ENODATA;
+
+ ret = iio_read_channel_processed_scale(charger->channel_vusb, &val->intval, 1000);
+ if (ret < 0)
+ return ret;
+
+ break;
+ case POWER_SUPPLY_PROP_ONLINE:
+ val->intval = !!(stat1 & VBUS_DET);
+ break;
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ val->intval = charger->input_current_limit;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int twl6030_charger_usb_set_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ struct twl6030_charger_info *charger = power_supply_get_drvdata(psy);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ charger->input_current_limit = val->intval;
+ return twl6030_config_cinlimit_reg(charger, charger->input_current_limit);
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int twl6030_charger_usb_property_is_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ dev_info(&psy->dev, "is %d writeable?\n", (int)psp);
+ switch (psp) {
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static enum power_supply_property twl6030_charger_props[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+};
+
+static const struct power_supply_desc twl6030_charger_usb_desc = {
+ .name = "twl6030_usb",
+ .type = POWER_SUPPLY_TYPE_USB,
+ .properties = twl6030_charger_props,
+ .num_properties = ARRAY_SIZE(twl6030_charger_props),
+ .get_property = twl6030_charger_usb_get_property,
+ .set_property = twl6030_charger_usb_set_property,
+ .property_is_writeable = twl6030_charger_usb_property_is_writeable,
+};
+
+static int twl6030_charger_probe(struct platform_device *pdev)
+{
+ struct twl6030_charger_info *charger;
+ struct power_supply_config psy_cfg = {};
+ int ret;
+ u8 val;
+
+ charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
+ if (!charger)
+ return -ENOMEM;
+
+ charger->dev = &pdev->dev;
+ charger->irq_chg = platform_get_irq(pdev, 0);
+
+ platform_set_drvdata(pdev, charger);
+ psy_cfg.drv_data = charger;
+
+ charger->channel_vusb = devm_iio_channel_get(&pdev->dev, "vusb");
+ if (IS_ERR(charger->channel_vusb)) {
+ ret = PTR_ERR(charger->channel_vusb);
+ if (ret == -EPROBE_DEFER)
+ return ret; /* iio not ready */
+ dev_warn(&pdev->dev, "could not request vusb iio channel (%d)",
+ ret);
+ charger->channel_vusb = NULL;
+ }
+
+ charger->usb = devm_power_supply_register(&pdev->dev,
+ &twl6030_charger_usb_desc,
+ &psy_cfg);
+ if (IS_ERR(charger->usb)) {
+ return dev_err_probe(&pdev->dev, PTR_ERR(charger->usb),
+ "Failed to register usb\n");
+ }
+
+ ret = power_supply_get_battery_info(charger->usb, &charger->binfo);
+ if (ret < 0)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to get battery info\n");
+
+ dev_info(&pdev->dev, "battery with vmax %d imax: %d\n",
+ charger->binfo->constant_charge_voltage_max_uv,
+ charger->binfo->constant_charge_current_max_ua);
+
+ if (charger->binfo->constant_charge_voltage_max_uv == -EINVAL) {
+ ret = twl6030_charger_read(CHARGERUSB_CTRLLIMIT1, &val);
+ if (ret < 0)
+ return ret;
+
+ charger->binfo->constant_charge_voltage_max_uv =
+ VOREG_TO_UV(val);
+ }
+
+ if (charger->binfo->constant_charge_voltage_max_uv > 4760000 ||
+ charger->binfo->constant_charge_voltage_max_uv < 350000)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "Invalid charge voltage\n");
+
+ if (charger->binfo->constant_charge_current_max_ua == -EINVAL) {
+ ret = twl6030_charger_read(CHARGERUSB_CTRLLIMIT2, &val);
+ if (ret < 0)
+ return ret;
+
+ charger->binfo->constant_charge_current_max_ua = VICHRG_TO_UA(val);
+ }
+
+ if (charger->binfo->constant_charge_current_max_ua < 100000 ||
+ charger->binfo->constant_charge_current_max_ua > 1500000) {
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "Invalid charge current\n");
+ }
+
+ if ((charger->binfo->charge_term_current_ua != -EINVAL) &&
+ (charger->binfo->charge_term_current_ua > 400000 ||
+ charger->binfo->charge_term_current_ua < 50000)) {
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "Invalid charge termination current\n");
+ }
+
+ ret = devm_delayed_work_autocancel(&pdev->dev,
+ &charger->charger_monitor,
+ twl6030_charger_wdg);
+ if (ret < 0)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to register delayed work\n");
+
+ ret = devm_request_threaded_irq(&pdev->dev, charger->irq_chg, NULL,
+ twl6030_charger_interrupt,
+ IRQF_ONESHOT, pdev->name,
+ charger);
+ if (ret < 0) {
+ return dev_err_probe(&pdev->dev, ret,
+ "could not request irq %d\n",
+ charger->irq_chg);
+ }
+
+ /* turing to charging to configure things */
+ twl6030_charger_write(CONTROLLER_CTRL1, 0);
+ twl6030_charger_interrupt(0, charger);
+
+ return 0;
+}
+
+static const struct of_device_id twl_charger_of_match[] __maybe_unused = {
+ {.compatible = "ti,twl6030-charger", },
+ {.compatible = "ti,twl6032-charger", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, twl_charger_of_match);
+
+static struct platform_driver twl6030_charger_driver = {
+ .probe = twl6030_charger_probe,
+ .driver = {
+ .name = "twl6030_charger",
+ .of_match_table = of_match_ptr(twl_charger_of_match),
+ },
+};
+module_platform_driver(twl6030_charger_driver);
+
+MODULE_DESCRIPTION("TWL6030 Battery Charger Interface driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:twl6030_charger");
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] power: supply: initial support for TWL6030/32
2024-09-18 8:41 ` [PATCH 3/3] power: supply: initial support for TWL6030/32 Andreas Kemnade
@ 2024-09-18 10:43 ` Krzysztof Kozlowski
2024-09-18 12:43 ` Andreas Kemnade
2024-09-23 16:29 ` Andreas Kemnade
2024-09-18 21:11 ` kernel test robot
1 sibling, 2 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-09-18 10:43 UTC (permalink / raw)
To: Andreas Kemnade, tony, Sebastian Reichel, linux-omap, devicetree,
Lee Jones, Rob Herring, Krzysztof Kozlowski, linux-kernel,
Conor Dooley, linux-pm
On 18/09/2024 10:41, Andreas Kemnade wrote:
> Add a driver for the charger in the TWL6030/32. For now it does not report
> much in sysfs but parameters are set up for USB, charging is enabled with
> the specified parameters. It stops charging when full and also restarts
> charging.
> This prevents ending up in a system setup where you run out of battery
> although a charger is plugged in after precharge completed.
>
> Battery voltage behavior was checked via the GPADC.
>
Few stylistic comments below.
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> drivers/power/supply/Kconfig | 10 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/twl6030_charger.c | 566 +++++++++++++++++++++++++
> 3 files changed, 577 insertions(+)
> create mode 100644 drivers/power/supply/twl6030_charger.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index bcfa63fb9f1e2..9f2eef6787f7a 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -493,6 +493,16 @@ config CHARGER_TWL4030
> help
> Say Y here to enable support for TWL4030 Battery Charge Interface.
>
> +config CHARGER_TWL6030
> + tristate "OMAP TWL6030 BCI charger driver"
> + depends on IIO && TWL4030_CORE
|| COMPILE_TEST, at least for TWL part
(but please test first)
> + help
> + Say Y here to enable support for TWL6030/6032 Battery Charge
> + Interface.
> +
> + This driver can be build as a module. If so, the module will be
> + called twl6030_charger.
> +
> +
> +static int twl6030_charger_probe(struct platform_device *pdev)
> +{
> + struct twl6030_charger_info *charger;
> + struct power_supply_config psy_cfg = {};
> + int ret;
> + u8 val;
> +
> + charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
> + if (!charger)
> + return -ENOMEM;
> +
> + charger->dev = &pdev->dev;
> + charger->irq_chg = platform_get_irq(pdev, 0);
> +
> + platform_set_drvdata(pdev, charger);
> + psy_cfg.drv_data = charger;
> +
> + charger->channel_vusb = devm_iio_channel_get(&pdev->dev, "vusb");
> + if (IS_ERR(charger->channel_vusb)) {
> + ret = PTR_ERR(charger->channel_vusb);
> + if (ret == -EPROBE_DEFER)
> + return ret; /* iio not ready */
> + dev_warn(&pdev->dev, "could not request vusb iio channel (%d)",
> + ret);
> + charger->channel_vusb = NULL;
> + }
> +
> + charger->usb = devm_power_supply_register(&pdev->dev,
> + &twl6030_charger_usb_desc,
> + &psy_cfg);
> + if (IS_ERR(charger->usb)) {
Checkpatch...
> + return dev_err_probe(&pdev->dev, PTR_ERR(charger->usb),
> + "Failed to register usb\n");
> + }
> +
> + ret = power_supply_get_battery_info(charger->usb, &charger->binfo);
> + if (ret < 0)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to get battery info\n");
> +
> + dev_info(&pdev->dev, "battery with vmax %d imax: %d\n",
> + charger->binfo->constant_charge_voltage_max_uv,
> + charger->binfo->constant_charge_current_max_ua);
> +
> + if (charger->binfo->constant_charge_voltage_max_uv == -EINVAL) {
> + ret = twl6030_charger_read(CHARGERUSB_CTRLLIMIT1, &val);
> + if (ret < 0)
> + return ret;
> +
> + charger->binfo->constant_charge_voltage_max_uv =
> + VOREG_TO_UV(val);
> + }
> +
> + if (charger->binfo->constant_charge_voltage_max_uv > 4760000 ||
> + charger->binfo->constant_charge_voltage_max_uv < 350000)
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "Invalid charge voltage\n");
> +
> + if (charger->binfo->constant_charge_current_max_ua == -EINVAL) {
> + ret = twl6030_charger_read(CHARGERUSB_CTRLLIMIT2, &val);
> + if (ret < 0)
> + return ret;
> +
> + charger->binfo->constant_charge_current_max_ua = VICHRG_TO_UA(val);
> + }
> +
> + if (charger->binfo->constant_charge_current_max_ua < 100000 ||
> + charger->binfo->constant_charge_current_max_ua > 1500000) {
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "Invalid charge current\n");
> + }
> +
> + if ((charger->binfo->charge_term_current_ua != -EINVAL) &&
> + (charger->binfo->charge_term_current_ua > 400000 ||
> + charger->binfo->charge_term_current_ua < 50000)) {
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "Invalid charge termination current\n");
> + }
> +
> + ret = devm_delayed_work_autocancel(&pdev->dev,
> + &charger->charger_monitor,
> + twl6030_charger_wdg);
> + if (ret < 0)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to register delayed work\n");
> +
> + ret = devm_request_threaded_irq(&pdev->dev, charger->irq_chg, NULL,
> + twl6030_charger_interrupt,
> + IRQF_ONESHOT, pdev->name,
> + charger);
> + if (ret < 0) {
Drop {}, see checkpatch.
> + return dev_err_probe(&pdev->dev, ret,
> + "could not request irq %d\n",
> + charger->irq_chg);
> + }
> +
> + /* turing to charging to configure things */
> + twl6030_charger_write(CONTROLLER_CTRL1, 0);
> + twl6030_charger_interrupt(0, charger);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id twl_charger_of_match[] __maybe_unused = {
> + {.compatible = "ti,twl6030-charger", },
> + {.compatible = "ti,twl6032-charger", },
So they are compatible? Why two entries in such case?
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, twl_charger_of_match);
> +
> +static struct platform_driver twl6030_charger_driver = {
> + .probe = twl6030_charger_probe,
> + .driver = {
> + .name = "twl6030_charger",
> + .of_match_table = of_match_ptr(twl_charger_of_match),
I propose to drop of_match_ptr and maybe_unused, so this won't be
restricted only to OF
> + },
> +};
> +module_platform_driver(twl6030_charger_driver);
> +
> +MODULE_DESCRIPTION("TWL6030 Battery Charger Interface driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:twl6030_charger");
You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong (e.g. misses either
entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
for incomplete ID table.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] dt-bindings: power: supply: Add TI TWL603X charger
2024-09-18 8:41 ` [PATCH 1/3] dt-bindings: power: supply: Add TI TWL603X charger Andreas Kemnade
@ 2024-09-18 10:46 ` Krzysztof Kozlowski
0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-09-18 10:46 UTC (permalink / raw)
To: Andreas Kemnade, tony, Sebastian Reichel, linux-omap, devicetree,
Lee Jones, Rob Herring, Krzysztof Kozlowski, linux-kernel,
Conor Dooley, linux-pm
On 18/09/2024 10:41, Andreas Kemnade wrote:
> Use a fallback compatible since for especially for generic
> defensive setup of parameters, both 6030 and 6032 are the same and
> U-Boot actually uses a generic 6030/32 function to enable the
> charger.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> .../power/supply/ti,twl6030-charger.yaml | 62 +++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml
>
> diff --git a/Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml b/Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml
> new file mode 100644
> index 0000000000000..fe0fe9a78761c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: GPL-2.0
Dual license. See checkpatch.
Please run scripts/checkpatch.pl and fix reported warnings. Then please
run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/ti,twl6030-charger.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TWL6030/32 BCI (Battery Charger Interface)
> +
> +description: |
Do not need '|' unless you need to preserve formatting.
> + The battery charger needs to be configured to do any charging besides of
> + precharging. The GPADC in the PMIC has to be used to get the related
> + voltages.
> +
> +maintainers:
> + - Andreas Kemnade <andreas@kemnade.info>
> +
> +allOf:
> + - $ref: power-supply.yaml#
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
No need for items here. Just const directly.
> + - const: "ti,twl6030-charger"
Not tested.
Drop ""
> + - items:
> + - const: "ti,twl6032-charger"
> + - const: "ti,twl6030-charger"
> +
> + interrupts:
> + minItems: 2
Drop minItems
> + maxItems: 2
... and actually drop both and instead list items with description
(items: -descriptio: ... - description: ....)
> +
> + io-channels:
> + items:
> + - description: VBUS Voltage Channel
> +
> + io-channel-names:
> + items:
> + - const: vusb
> +
> + monitored-battery:
Just : true
> + description:
> + phandle of battery characteristics devicetree node
That's redundant, you do no say anything useful here.
> +
> +required:
> + - compatible
> + - interrupts
> + - monitored-battery
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + pmic {
Drop... or this is supposed to be part of parent schema?
> + bci {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
e.g. charger
> + compatible = "ti,twl6032-charger", "ti,twl6030-charger";
> + interrupts = <2>, <5>;
> + io-channels = <&gpadc 10>;
> + io-channel-names = "vusb";
> + monitored-battery = <&bat>;
> + };
> + };
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] dt-bindings: mfd: twl: add charger node also for TWL603x
2024-09-18 8:41 ` [PATCH 2/3] dt-bindings: mfd: twl: add charger node also for TWL603x Andreas Kemnade
@ 2024-09-18 10:47 ` Krzysztof Kozlowski
2024-09-18 11:35 ` Andreas Kemnade
2024-09-21 0:51 ` Rob Herring
0 siblings, 2 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-09-18 10:47 UTC (permalink / raw)
To: Andreas Kemnade, tony, Sebastian Reichel, linux-omap, devicetree,
Lee Jones, Rob Herring, Krzysztof Kozlowski, linux-kernel,
Conor Dooley, linux-pm
On 18/09/2024 10:41, Andreas Kemnade wrote:
> Also the TWL603X devices have a charger, so allow to specify it here.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> .../devicetree/bindings/mfd/ti,twl.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> index e94b0fd7af0f8..4064a228cb0fc 100644
> --- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> +++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> @@ -105,6 +105,11 @@ allOf:
> regulator-initial-mode: false
>
> properties:
> + bci:
charger
> + type: object
additionalProperties: true
Each node must end with additionalProperties or unevaluated. I think you
never tested it, because dtschema reports this.
> + properties:
> + compatible:
> + const: ti,twl6030-charger
> gpadc:
> type: object
> properties:
> @@ -136,6 +141,13 @@ allOf:
> regulator-initial-mode: false
>
> properties:
> + bci:
> + type: object
> + properties:
> + compatible:
> + items:
> + - const: ti,twl6032-charger
> + - const: ti,twl6030-charger
> gpadc:
> type: object
> properties:
> @@ -222,6 +234,12 @@ examples:
> interrupt-controller;
> #interrupt-cells = <1>;
>
> + bci {
> + compatible = "ti,twl6030-charger";
> + interrupts = <2>, <5>;
> + monitored-battery = <&bat>;
One complete example in parent node, so you can drop example from patch #1.
> + };
> +
> gpadc {
> compatible = "ti,twl6030-gpadc";
> interrupts = <6>;
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] dt-bindings: mfd: twl: add charger node also for TWL603x
2024-09-18 10:47 ` Krzysztof Kozlowski
@ 2024-09-18 11:35 ` Andreas Kemnade
2024-09-21 0:51 ` Rob Herring
1 sibling, 0 replies; 15+ messages in thread
From: Andreas Kemnade @ 2024-09-18 11:35 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: tony, Sebastian Reichel, linux-omap, devicetree, Lee Jones,
Rob Herring, Krzysztof Kozlowski, linux-kernel, Conor Dooley,
linux-pm
Am Wed, 18 Sep 2024 12:47:22 +0200
schrieb Krzysztof Kozlowski <krzk@kernel.org>:
> On 18/09/2024 10:41, Andreas Kemnade wrote:
> > Also the TWL603X devices have a charger, so allow to specify it
> > here.
> >
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> > .../devicetree/bindings/mfd/ti,twl.yaml | 18
> > ++++++++++++++++++ 1 file changed, 18 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > b/Documentation/devicetree/bindings/mfd/ti,twl.yaml index
> > e94b0fd7af0f8..4064a228cb0fc 100644 ---
> > a/Documentation/devicetree/bindings/mfd/ti,twl.yaml +++
> > b/Documentation/devicetree/bindings/mfd/ti,twl.yaml @@ -105,6
> > +105,11 @@ allOf: regulator-initial-mode: false
> >
> > properties:
> > + bci:
>
> charger
>
> > + type: object
>
> additionalProperties: true
>
> Each node must end with additionalProperties or unevaluated. I think
> you never tested it, because dtschema reports this.
>
I did run it, no complaints:
andi@aktux:~/kernel$ touch Documentation/devicetree/bindings/mfd/ti,twl.yaml
andi@aktux:~/kernel$ touch Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml
andi@aktux:~/kernel$ make ARCH=arm dt_binding_check DT_SCHEMA_FILES=twl
SCHEMA Documentation/devicetree/bindings/processed-schema.json
/home/andi/kernel/Documentation/devicetree/bindings/net/snps,dwmac.yaml: mac-mode: missing type definition
CHKDT Documentation/devicetree/bindings
LINT Documentation/devicetree/bindings
DTC [C] Documentation/devicetree/bindings/power/supply/twl4030-charger.example.dtb
DTEX Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.example.dts
DTC [C] Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.example.dtb
DTC [C] Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.example.dtb
DTC [C] Documentation/devicetree/bindings/iio/adc/ti,twl4030-madc.example.dtb
DTEX Documentation/devicetree/bindings/mfd/ti,twl.example.dts
DTC [C] Documentation/devicetree/bindings/mfd/ti,twl.example.dtb
andi@aktux:~/kernel$
Regards,
Andreas
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] power: supply: initial support for TWL6030/32
2024-09-18 10:43 ` Krzysztof Kozlowski
@ 2024-09-18 12:43 ` Andreas Kemnade
2024-09-18 12:53 ` Krzysztof Kozlowski
2024-09-23 16:29 ` Andreas Kemnade
1 sibling, 1 reply; 15+ messages in thread
From: Andreas Kemnade @ 2024-09-18 12:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: tony, Sebastian Reichel, linux-omap, devicetree, Lee Jones,
Rob Herring, Krzysztof Kozlowski, linux-kernel, Conor Dooley,
linux-pm
Am Wed, 18 Sep 2024 12:43:01 +0200
schrieb Krzysztof Kozlowski <krzk@kernel.org>:
[...]
> Drop {}, see checkpatch.
>
> > + return dev_err_probe(&pdev->dev, ret,
> > + "could not request irq %d\n",
> > + charger->irq_chg);
> > + }
> > +
Apparently checkpatch only moans about {} around single *lines*
not single *statements*, even with --strict.
Coding-style says single statements, so maybe checkpatch should be
fixed?
Same for other appearance of this pattern.
> > + /* turing to charging to configure things */
> > + twl6030_charger_write(CONTROLLER_CTRL1, 0);
> > + twl6030_charger_interrupt(0, charger);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id twl_charger_of_match[]
> > __maybe_unused = {
> > + {.compatible = "ti,twl6030-charger", },
> > + {.compatible = "ti,twl6032-charger", },
>
> So they are compatible? Why two entries in such case?
>
There is one device_is_compatible() in the file.
Regrads,
Andreas
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] power: supply: initial support for TWL6030/32
2024-09-18 12:43 ` Andreas Kemnade
@ 2024-09-18 12:53 ` Krzysztof Kozlowski
0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-09-18 12:53 UTC (permalink / raw)
To: Andreas Kemnade
Cc: tony, Sebastian Reichel, linux-omap, devicetree, Lee Jones,
Rob Herring, Krzysztof Kozlowski, linux-kernel, Conor Dooley,
linux-pm
On 18/09/2024 14:43, Andreas Kemnade wrote:
> Am Wed, 18 Sep 2024 12:43:01 +0200
> schrieb Krzysztof Kozlowski <krzk@kernel.org>:
>
> [...]
>> Drop {}, see checkpatch.
>>
>>> + return dev_err_probe(&pdev->dev, ret,
>>> + "could not request irq %d\n",
>>> + charger->irq_chg);
>>> + }
>>> +
>
> Apparently checkpatch only moans about {} around single *lines*
> not single *statements*, even with --strict.
>
> Coding-style says single statements, so maybe checkpatch should be
> fixed?
>
> Same for other appearance of this pattern.
Hm, could be. I think this still should be without {}, regardless of
checkpatch.
>
>>> + /* turing to charging to configure things */
>>> + twl6030_charger_write(CONTROLLER_CTRL1, 0);
>>> + twl6030_charger_interrupt(0, charger);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct of_device_id twl_charger_of_match[]
>>> __maybe_unused = {
>>> + {.compatible = "ti,twl6030-charger", },
>>> + {.compatible = "ti,twl6032-charger", },
>>
>> So they are compatible? Why two entries in such case?
>>
> There is one device_is_compatible() in the file.
Ah, you should rather use match data. Compatibles inside the code do not
scale.
>
> Regrads,
> Andreas
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] power: supply: initial support for TWL6030/32
2024-09-18 8:41 ` [PATCH 3/3] power: supply: initial support for TWL6030/32 Andreas Kemnade
2024-09-18 10:43 ` Krzysztof Kozlowski
@ 2024-09-18 21:11 ` kernel test robot
1 sibling, 0 replies; 15+ messages in thread
From: kernel test robot @ 2024-09-18 21:11 UTC (permalink / raw)
To: Andreas Kemnade, tony, Sebastian Reichel, linux-omap, devicetree,
Lee Jones, Rob Herring, Krzysztof Kozlowski, linux-kernel,
Conor Dooley, linux-pm
Cc: llvm, oe-kbuild-all, Andreas Kemnade
Hi Andreas,
kernel test robot noticed the following build warnings:
[auto build test WARNING on sre-power-supply/for-next]
[also build test WARNING on lee-mfd/for-mfd-next lee-leds/for-leds-next linus/master v6.11 next-20240918]
[cannot apply to lee-mfd/for-mfd-fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andreas-Kemnade/dt-bindings-power-supply-Add-TI-TWL603X-charger/20240918-164406
base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link: https://lore.kernel.org/r/20240918084132.928295-4-andreas%40kemnade.info
patch subject: [PATCH 3/3] power: supply: initial support for TWL6030/32
config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20240919/202409190409.0mOjgpq4-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 8663a75fa2f31299ab8d1d90288d9df92aadee88)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240919/202409190409.0mOjgpq4-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409190409.0mOjgpq4-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/power/supply/twl6030_charger.c:20:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/power/supply/twl6030_charger.c:20:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/power/supply/twl6030_charger.c:20:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
In file included from drivers/power/supply/twl6030_charger.c:24:
In file included from include/linux/usb/otg.h:13:
In file included from include/linux/phy/phy.h:17:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:21:
In file included from include/linux/mm.h:2228:
include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
514 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/power/supply/twl6030_charger.c:360:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
360 | u8 intstat;
| ^
8 warnings generated.
vim +360 drivers/power/supply/twl6030_charger.c
345
346 static int twl6030_charger_usb_get_property(struct power_supply *psy,
347 enum power_supply_property psp,
348 union power_supply_propval *val)
349 {
350 struct twl6030_charger_info *charger = power_supply_get_drvdata(psy);
351 int ret;
352 u8 stat1;
353
354 ret = twl6030_charger_read(CONTROLLER_STAT1, &stat1);
355 if (ret)
356 return ret;
357
358 switch (psp) {
359 case POWER_SUPPLY_PROP_STATUS:
> 360 u8 intstat;
361
362 if (!(stat1 & VBUS_DET)) {
363 val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
364 break;
365 }
366 ret = twl6030_charger_read(CHARGERUSB_STATUS_INT2, &intstat);
367 if (ret)
368 return ret;
369
370 if (intstat & CHARGE_DONE)
371 val->intval = POWER_SUPPLY_STATUS_FULL;
372 else if (intstat & CURRENT_TERM)
373 val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
374 else
375 val->intval = POWER_SUPPLY_STATUS_CHARGING;
376 break;
377 case POWER_SUPPLY_PROP_VOLTAGE_NOW:
378 if (!charger->channel_vusb)
379 return -ENODATA;
380
381 ret = iio_read_channel_processed_scale(charger->channel_vusb, &val->intval, 1000);
382 if (ret < 0)
383 return ret;
384
385 break;
386 case POWER_SUPPLY_PROP_ONLINE:
387 val->intval = !!(stat1 & VBUS_DET);
388 break;
389 case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
390 val->intval = charger->input_current_limit;
391 break;
392 default:
393 return -EINVAL;
394 }
395
396 return 0;
397 }
398
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] dt-bindings: mfd: twl: add charger node also for TWL603x
2024-09-18 10:47 ` Krzysztof Kozlowski
2024-09-18 11:35 ` Andreas Kemnade
@ 2024-09-21 0:51 ` Rob Herring
2024-09-26 7:26 ` Andreas Kemnade
1 sibling, 1 reply; 15+ messages in thread
From: Rob Herring @ 2024-09-21 0:51 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andreas Kemnade, tony, Sebastian Reichel, linux-omap, devicetree,
Lee Jones, Krzysztof Kozlowski, linux-kernel, Conor Dooley,
linux-pm
On Wed, Sep 18, 2024 at 12:47:22PM +0200, Krzysztof Kozlowski wrote:
> On 18/09/2024 10:41, Andreas Kemnade wrote:
> > Also the TWL603X devices have a charger, so allow to specify it here.
> >
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> > .../devicetree/bindings/mfd/ti,twl.yaml | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > index e94b0fd7af0f8..4064a228cb0fc 100644
> > --- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > @@ -105,6 +105,11 @@ allOf:
> > regulator-initial-mode: false
> >
> > properties:
> > + bci:
>
> charger
>
> > + type: object
>
> additionalProperties: true
>
> Each node must end with additionalProperties or unevaluated. I think you
> never tested it, because dtschema reports this.
This is under an if/then schema is why there's no errors.
This schema probably should have been 3 with a ti,twl-common.yaml schema
for the common properties, but I'm not sure it is worth changing now.
Rob
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] power: supply: initial support for TWL6030/32
2024-09-18 10:43 ` Krzysztof Kozlowski
2024-09-18 12:43 ` Andreas Kemnade
@ 2024-09-23 16:29 ` Andreas Kemnade
2024-09-24 8:00 ` Krzysztof Kozlowski
1 sibling, 1 reply; 15+ messages in thread
From: Andreas Kemnade @ 2024-09-23 16:29 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: tony, Sebastian Reichel, linux-omap, devicetree, Lee Jones,
Rob Herring, Krzysztof Kozlowski, linux-kernel, Conor Dooley,
linux-pm
Am Wed, 18 Sep 2024 12:43:01 +0200
schrieb Krzysztof Kozlowski <krzk@kernel.org>:
> On 18/09/2024 10:41, Andreas Kemnade wrote:
> > Add a driver for the charger in the TWL6030/32. For now it does not
> > report much in sysfs but parameters are set up for USB, charging is
> > enabled with the specified parameters. It stops charging when full
> > and also restarts charging.
> > This prevents ending up in a system setup where you run out of
> > battery although a charger is plugged in after precharge completed.
> >
> > Battery voltage behavior was checked via the GPADC.
> >
>
> Few stylistic comments below.
>
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> > drivers/power/supply/Kconfig | 10 +
> > drivers/power/supply/Makefile | 1 +
> > drivers/power/supply/twl6030_charger.c | 566
> > +++++++++++++++++++++++++ 3 files changed, 577 insertions(+)
> > create mode 100644 drivers/power/supply/twl6030_charger.c
> >
> > diff --git a/drivers/power/supply/Kconfig
> > b/drivers/power/supply/Kconfig index bcfa63fb9f1e2..9f2eef6787f7a
> > 100644 --- a/drivers/power/supply/Kconfig
> > +++ b/drivers/power/supply/Kconfig
> > @@ -493,6 +493,16 @@ config CHARGER_TWL4030
> > help
> > Say Y here to enable support for TWL4030 Battery Charge
> > Interface.
> > +config CHARGER_TWL6030
> > + tristate "OMAP TWL6030 BCI charger driver"
> > + depends on IIO && TWL4030_CORE
>
> || COMPILE_TEST, at least for TWL part
> (but please test first)
>
ERROR: modpost: "twl_i2c_write"
[drivers/power/supply/twl6030_charger.ko] undefined! ERROR: modpost:
"twl_i2c_read" [drivers/power/supply/twl6030_charger.ko] undefined!
>
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, twl_charger_of_match);
> > +
> > +static struct platform_driver twl6030_charger_driver = {
> > + .probe = twl6030_charger_probe,
> > + .driver = {
> > + .name = "twl6030_charger",
> > + .of_match_table =
> > of_match_ptr(twl_charger_of_match),
>
> I propose to drop of_match_ptr and maybe_unused, so this won't be
> restricted only to OF
>
so that more things get compile-tested without OF? But I see no reason
why .probe would be optimized away (and with it a lot more things) by
the compiler.
Regards,
Andreas
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] power: supply: initial support for TWL6030/32
2024-09-23 16:29 ` Andreas Kemnade
@ 2024-09-24 8:00 ` Krzysztof Kozlowski
0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-09-24 8:00 UTC (permalink / raw)
To: Andreas Kemnade
Cc: tony, Sebastian Reichel, linux-omap, devicetree, Lee Jones,
Rob Herring, Krzysztof Kozlowski, linux-kernel, Conor Dooley,
linux-pm
On 23/09/2024 18:29, Andreas Kemnade wrote:
> Am Wed, 18 Sep 2024 12:43:01 +0200
> schrieb Krzysztof Kozlowski <krzk@kernel.org>:
>
>> On 18/09/2024 10:41, Andreas Kemnade wrote:
>>> Add a driver for the charger in the TWL6030/32. For now it does not
>>> report much in sysfs but parameters are set up for USB, charging is
>>> enabled with the specified parameters. It stops charging when full
>>> and also restarts charging.
>>> This prevents ending up in a system setup where you run out of
>>> battery although a charger is plugged in after precharge completed.
>>>
>>> Battery voltage behavior was checked via the GPADC.
>>>
>>
>> Few stylistic comments below.
>>
>>> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
>>> ---
>>> drivers/power/supply/Kconfig | 10 +
>>> drivers/power/supply/Makefile | 1 +
>>> drivers/power/supply/twl6030_charger.c | 566
>>> +++++++++++++++++++++++++ 3 files changed, 577 insertions(+)
>>> create mode 100644 drivers/power/supply/twl6030_charger.c
>>>
>>> diff --git a/drivers/power/supply/Kconfig
>>> b/drivers/power/supply/Kconfig index bcfa63fb9f1e2..9f2eef6787f7a
>>> 100644 --- a/drivers/power/supply/Kconfig
>>> +++ b/drivers/power/supply/Kconfig
>>> @@ -493,6 +493,16 @@ config CHARGER_TWL4030
>>> help
>>> Say Y here to enable support for TWL4030 Battery Charge
>>> Interface.
>>> +config CHARGER_TWL6030
>>> + tristate "OMAP TWL6030 BCI charger driver"
>>> + depends on IIO && TWL4030_CORE
>>
>> || COMPILE_TEST, at least for TWL part
>> (but please test first)
>>
> ERROR: modpost: "twl_i2c_write"
> [drivers/power/supply/twl6030_charger.ko] undefined! ERROR: modpost:
> "twl_i2c_read" [drivers/power/supply/twl6030_charger.ko] undefined!
ok
>
>>
>>> + { }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, twl_charger_of_match);
>>> +
>>> +static struct platform_driver twl6030_charger_driver = {
>>> + .probe = twl6030_charger_probe,
>>> + .driver = {
>>> + .name = "twl6030_charger",
>>> + .of_match_table =
>>> of_match_ptr(twl_charger_of_match),
>>
>> I propose to drop of_match_ptr and maybe_unused, so this won't be
>> restricted only to OF
>>
> so that more things get compile-tested without OF? But I see no reason
> why .probe would be optimized away (and with it a lot more things) by
> the compiler.
No, it is not probe related by ACPI PRP0001
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] dt-bindings: mfd: twl: add charger node also for TWL603x
2024-09-21 0:51 ` Rob Herring
@ 2024-09-26 7:26 ` Andreas Kemnade
0 siblings, 0 replies; 15+ messages in thread
From: Andreas Kemnade @ 2024-09-26 7:26 UTC (permalink / raw)
To: Rob Herring
Cc: Krzysztof Kozlowski, tony, Sebastian Reichel, linux-omap,
devicetree, Lee Jones, Krzysztof Kozlowski, linux-kernel,
Conor Dooley, linux-pm
Am Fri, 20 Sep 2024 19:51:25 -0500
schrieb Rob Herring <robh@kernel.org>:
> On Wed, Sep 18, 2024 at 12:47:22PM +0200, Krzysztof Kozlowski wrote:
> > On 18/09/2024 10:41, Andreas Kemnade wrote:
> > > Also the TWL603X devices have a charger, so allow to specify it
> > > here.
> > >
> > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > ---
> > > .../devicetree/bindings/mfd/ti,twl.yaml | 18
> > > ++++++++++++++++++ 1 file changed, 18 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > > b/Documentation/devicetree/bindings/mfd/ti,twl.yaml index
> > > e94b0fd7af0f8..4064a228cb0fc 100644 ---
> > > a/Documentation/devicetree/bindings/mfd/ti,twl.yaml +++
> > > b/Documentation/devicetree/bindings/mfd/ti,twl.yaml @@ -105,6
> > > +105,11 @@ allOf: regulator-initial-mode: false
> > >
> > > properties:
> > > + bci:
> >
> > charger
> >
> > > + type: object
> >
> > additionalProperties: true
> >
Thinking again. Why additionalProperties? unevaluatedProperties
looks more reasonable for me. There are additional properties but
they should be evaluated by another schema.
> > Each node must end with additionalProperties or unevaluated. I
> > think you never tested it, because dtschema reports this.
>
> This is under an if/then schema is why there's no errors.
>
and then it just accepts anything with compatible twl6032-charger e.g.
and does not care about anything in patch 2, because it has a different
compatible.
> This schema probably should have been 3 with a ti,twl-common.yaml
> schema for the common properties, but I'm not sure it is worth
> changing now.
>
Or a ti,twl4030.yaml and a ti,twl603X.yaml. 6030 and 6032 have more in
common than the 4030.
I would propose that is something for the next more final cleaning
up/conversion round. First I would like to avoid having drained
batteries because of no charging, so allow for more automated testing
and bisecting.
I think I will prepare a v2 series on monday.
Regards,
Andreas
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-09-26 7:26 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-18 8:41 [PATCH 0/3] power: supply: twl6030/32 charger Andreas Kemnade
2024-09-18 8:41 ` [PATCH 1/3] dt-bindings: power: supply: Add TI TWL603X charger Andreas Kemnade
2024-09-18 10:46 ` Krzysztof Kozlowski
2024-09-18 8:41 ` [PATCH 2/3] dt-bindings: mfd: twl: add charger node also for TWL603x Andreas Kemnade
2024-09-18 10:47 ` Krzysztof Kozlowski
2024-09-18 11:35 ` Andreas Kemnade
2024-09-21 0:51 ` Rob Herring
2024-09-26 7:26 ` Andreas Kemnade
2024-09-18 8:41 ` [PATCH 3/3] power: supply: initial support for TWL6030/32 Andreas Kemnade
2024-09-18 10:43 ` Krzysztof Kozlowski
2024-09-18 12:43 ` Andreas Kemnade
2024-09-18 12:53 ` Krzysztof Kozlowski
2024-09-23 16:29 ` Andreas Kemnade
2024-09-24 8:00 ` Krzysztof Kozlowski
2024-09-18 21:11 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).