devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/5] Add Texas Instruments BQ25703 Charger
@ 2024-08-29 21:30 Chris Morgan
  2024-08-29 21:30 ` [RFC 1/5] dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger Chris Morgan
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Chris Morgan @ 2024-08-29 21:30 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-rockchip, devicetree, broonie, lgirdwood, sre, heiko,
	conor+dt, krzk+dt, robh, lee, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add support for the Texas Instruments BQ25703 charger manager. The
device integrates a boost converter with the charger manager. This
series adds the device as an MFD with separate regulator and power
supply drivers. This allows us to manage a circular dependency with
a type-c port manager which depends on the regulator for usb-otg
but supplies power to the bq25703 charger.

Requesting comments on this series as I want to confirm this is the
best way to integrate these functions.

Chris Morgan (5):
  dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger
  mfd: bq257xx: Add support for BQ25703 core driver
  power: supply: bq257xx: Add support for BQ257XX charger manager
  regulator: bq257xx: Add bq257xx boost regulator driver
  arm64: dts: rockchip: Add USB and charger to Gameforce Ace

 .../devicetree/bindings/mfd/ti,bq25703a.yaml  | 143 +++
 .../dts/rockchip/rk3588s-gameforce-ace.dts    | 120 +++
 drivers/mfd/Kconfig                           |  11 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/bq257xx.c                         | 118 +++
 drivers/power/supply/Kconfig                  |   7 +
 drivers/power/supply/Makefile                 |   1 +
 drivers/power/supply/bq257xx_charger.c        | 811 ++++++++++++++++++
 drivers/regulator/Kconfig                     |   8 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/bq257xx-regulator.c         | 195 +++++
 include/linux/mfd/bq257xx.h                   | 120 +++
 12 files changed, 1536 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
 create mode 100644 drivers/mfd/bq257xx.c
 create mode 100644 drivers/power/supply/bq257xx_charger.c
 create mode 100644 drivers/regulator/bq257xx-regulator.c
 create mode 100644 include/linux/mfd/bq257xx.h

-- 
2.34.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC 1/5] dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger
  2024-08-29 21:30 [RFC 0/5] Add Texas Instruments BQ25703 Charger Chris Morgan
@ 2024-08-29 21:30 ` Chris Morgan
  2024-08-30 16:30   ` Rob Herring
  2024-08-29 21:30 ` [RFC 2/5] mfd: bq257xx: Add support for BQ25703 core driver Chris Morgan
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Chris Morgan @ 2024-08-29 21:30 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-rockchip, devicetree, broonie, lgirdwood, sre, heiko,
	conor+dt, krzk+dt, robh, lee, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Document the Texas instruments BQ25703 series of charger managers/
buck/boost regulators.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 .../devicetree/bindings/mfd/ti,bq25703a.yaml  | 143 ++++++++++++++++++
 1 file changed, 143 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml

diff --git a/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml b/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
new file mode 100644
index 000000000000..e555aa60f9ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
@@ -0,0 +1,143 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/ti,bq25703a.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: BQ25703 Charger Manager/Buck/Boost Converter
+
+maintainers:
+  - Chris Morgan <macromorgan@hotmail.com>
+
+properties:
+  compatible:
+    const: ti,bq25703a
+
+  reg:
+    const: 0x6b
+    description: I2C slave address
+
+  interrupts:
+    maxItems: 1
+
+  power-supplies:
+    description:
+      phandle of the power supply that provides input power
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  ti,charge-current:
+    description:
+      maximum current to apply to charging the battery
+    minimum: 0
+    maximum: 8128000
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  ti,current-limit:
+    description:
+      maximum total input current allowed
+    minimum: 50000
+    maximum: 6400000
+    default: 3250000
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  ti,max-charge-voltage:
+    description:
+      maximum voltage to apply to charging the battery
+    minimum: 1024000
+    maximum: 19200000
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  ti,minimum-sys-voltage:
+    description:
+      minimum system voltage while on battery power, with default value
+      depending based on cell configuration
+    minimum: 1024000
+    maximum: 16128000
+    default:
+      enum: [3584000, 6144000, 9216000, 16128000]
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  regulators:
+    type: object
+    additionalProperties: false
+    description:
+      Boost converter regulator output of bq257xx
+
+    properties:
+      "usb-otg-vbus":
+        type: object
+        $ref: /schemas/regulator/regulator.yaml
+
+        properties:
+          regulator-name: true
+          regulator-min-microamp:
+            minimum: 0
+            maximum: 6350000
+          regulator-max-microamp:
+            minimum: 0
+            maximum: 6350000
+          regulator-min-microvolt:
+            minimum: 4480000
+            maximum: 20800000
+          regulator-max-microvolt:
+            minimum: 4480000
+            maximum: 20800000
+          enable-gpios:
+            description:
+              The BQ25703 may require both a register write and a GPIO
+              toggle to enable the boost regulator.
+
+        additionalProperties: true
+
+        required:
+          - regulator-name
+          - regulator-min-microamp
+          - regulator-max-microamp
+          - regulator-min-microvolt
+          - regulator-max-microvolt
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - power-supplies
+  - ti,charge-current
+  - ti,current-limit
+  - ti,max-charge-voltage
+  - ti,minimum-sys-voltage
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/pinctrl/rockchip.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bq25703: bq25703@6b {
+            compatible = "ti,bq25703a";
+            reg = <0x6b>;
+            interrupt-parent = <&gpio0>;
+            interrupts = <RK_PD5 IRQ_TYPE_LEVEL_LOW>;
+            power-supplies = <&fusb302>;
+            ti,charge-current = <2500000>;
+            ti,current-limit = <5000000>;
+            ti,max-charge-voltage = <8750000>;
+            ti,minimum-sys-voltage = <7400000>;
+
+            regulators {
+                usb_otg_vbus: usb-otg-vbus {
+                    enable-gpios = <&gpio4 RK_PA6 GPIO_ACTIVE_HIGH>;
+                    regulator-max-microamp = <960000>;
+                    regulator-max-microvolt = <5088000>;
+                    regulator-min-microamp = <512000>;
+                    regulator-min-microvolt = <4992000>;
+                    regulator-name = "usb_otg_vbus";
+                };
+            };
+        };
+    };
+
+...
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC 2/5] mfd: bq257xx: Add support for BQ25703 core driver
  2024-08-29 21:30 [RFC 0/5] Add Texas Instruments BQ25703 Charger Chris Morgan
  2024-08-29 21:30 ` [RFC 1/5] dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger Chris Morgan
@ 2024-08-29 21:30 ` Chris Morgan
  2024-09-03 15:25   ` Lee Jones
  2024-08-29 21:31 ` [RFC 3/5] power: supply: bq257xx: Add support for BQ257XX charger manager Chris Morgan
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Chris Morgan @ 2024-08-29 21:30 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-rockchip, devicetree, broonie, lgirdwood, sre, heiko,
	conor+dt, krzk+dt, robh, lee, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

The Texas Instruments BQ25703A is an integrated charger manager and
boost converter.

The MFD driver initalizes the device for the regulator driver
and power supply driver.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/mfd/Kconfig         |  11 ++++
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/bq257xx.c       | 118 +++++++++++++++++++++++++++++++++++
 include/linux/mfd/bq257xx.h | 120 ++++++++++++++++++++++++++++++++++++
 4 files changed, 250 insertions(+)
 create mode 100644 drivers/mfd/bq257xx.c
 create mode 100644 include/linux/mfd/bq257xx.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index bc8be2e593b6..712951ae7341 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1537,6 +1537,17 @@ config MFD_TI_LMU
 	  LM36274.  It consists of backlight, LED and regulator driver.
 	  It provides consistent device controls for lighting functions.
 
+config MFD_BQ257XX
+	tristate "TI BQ257XX Buck/Boost Charge Controller"
+	depends on I2C
+	select MFD_CORE
+	select REGMAP_I2C
+	help
+	  Support Texas Instruments BQ25703 Buck/Boost converter with
+	  charge controller. It consists of regulators that provide
+	  system voltage and OTG voltage, and a charger manager for
+	  batteries containing one or more cells.
+
 config MFD_OMAP_USB_HOST
 	bool "TI OMAP USBHS core and TLL driver"
 	depends on USB_EHCI_HCD_OMAP || USB_OHCI_HCD_OMAP3
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 02b651cd7535..90bc65d83c5f 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_MFD_SM501)		+= sm501.o
 obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835-pm.o
 obj-$(CONFIG_MFD_BCM590XX)	+= bcm590xx.o
 obj-$(CONFIG_MFD_BD9571MWV)	+= bd9571mwv.o
+obj-$(CONFIG_MFD_BQ257XX)	+= bq257xx.o
 obj-$(CONFIG_MFD_CROS_EC_DEV)	+= cros_ec_dev.o
 obj-$(CONFIG_MFD_CS42L43)	+= cs42l43.o
 obj-$(CONFIG_MFD_CS42L43_I2C)	+= cs42l43-i2c.o
diff --git a/drivers/mfd/bq257xx.c b/drivers/mfd/bq257xx.c
new file mode 100644
index 000000000000..c612262f9a1e
--- /dev/null
+++ b/drivers/mfd/bq257xx.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+/* BQ257XX MFD Driver
+ * Copyright (C) 2024 Chris Morgan <macromorgan@hotmail.com>
+ * Based off of BQ256XX Battery Charger Driver and
+ * Rockchip RK808 MFD Driver
+ */
+
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/mfd/bq257xx.h>
+#include <linux/mfd/core.h>
+#include <linux/regmap.h>
+
+static const struct regmap_range bq25703_readonly_reg_ranges[] = {
+	regmap_reg_range(BQ25703_CHARGER_STATUS, BQ25703_MANUFACT_DEV_ID),
+};
+
+static const struct regmap_access_table bq25703_writeable_regs = {
+	.no_ranges = bq25703_readonly_reg_ranges,
+	.n_no_ranges = ARRAY_SIZE(bq25703_readonly_reg_ranges),
+};
+
+static const struct regmap_range bq25703_volatile_reg_ranges[] = {
+	regmap_reg_range(BQ25703_CHARGE_OPTION_0, BQ25703_IIN_HOST),
+	regmap_reg_range(BQ25703_CHARGER_STATUS, BQ25703_ADC_OPTION),
+};
+
+static const struct regmap_access_table bq25703_volatile_regs = {
+	.yes_ranges = bq25703_volatile_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(bq25703_volatile_reg_ranges),
+};
+
+static const struct regmap_config bq25703_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = BQ25703_ADC_OPTION,
+	.cache_type = REGCACHE_RBTREE,
+	.wr_table = &bq25703_writeable_regs,
+	.volatile_table = &bq25703_volatile_regs,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+static const struct mfd_cell bq25703_cells[] = {
+	MFD_CELL_NAME("bq257xx-regulator"),
+	MFD_CELL_NAME("bq257xx-charger"),
+};
+
+static int bq257xx_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct bq257xx_device *bq;
+	const struct mfd_cell *cells;
+	int nr_cells;
+	int ret = 0;
+
+	bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
+	if (!bq)
+		return -ENOMEM;
+
+	bq->client = client;
+	bq->dev = dev;
+	bq->variant = (long)i2c_get_match_data(client);
+
+	switch (bq->variant) {
+	case BQ25703A:
+		bq->regmap_cfg = &bq25703_regmap_config;
+		cells = bq25703_cells;
+		nr_cells = ARRAY_SIZE(bq25703_cells);
+		break;
+	default:
+		dev_err(dev, "Unsupported BQ257XX ID %ld\n", bq->variant);
+		return -EINVAL;
+	}
+
+	bq->regmap = devm_regmap_init_i2c(client, bq->regmap_cfg);
+
+	if (IS_ERR(bq->regmap)) {
+		dev_err(dev, "Failed to allocate register map\n");
+		return PTR_ERR(bq->regmap);
+	}
+
+	i2c_set_clientdata(client, bq);
+
+	ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO,
+				   cells, nr_cells, NULL, 0, NULL);
+	if (ret) {
+		dev_err(&client->dev, "failed to add MFD devices %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static const struct i2c_device_id bq257xx_i2c_ids[] = {
+	{ "bq25703a" },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, bq257xx_i2c_ids);
+
+static const struct of_device_id bq257xx_of_match[] = {
+	{ .compatible = "ti,bq25703a", .data = (void *)BQ25703A, },
+	{}
+};
+MODULE_DEVICE_TABLE(of, bq257xx_of_match);
+
+static struct i2c_driver bq257xx_driver = {
+	.driver = {
+		.name = "bq257xx",
+		.of_match_table = bq257xx_of_match,
+	},
+	.probe = bq257xx_probe,
+	.id_table = bq257xx_i2c_ids,
+};
+module_i2c_driver(bq257xx_driver);
+
+MODULE_DESCRIPTION("bq257xx buck/boost/charger MFD driver");
+MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/bq257xx.h b/include/linux/mfd/bq257xx.h
new file mode 100644
index 000000000000..51f6501c6441
--- /dev/null
+++ b/include/linux/mfd/bq257xx.h
@@ -0,0 +1,120 @@
+/* SPDX-License-Identifier: GPL-2.0
+ * Register definitions for TI BQ257XX
+ * Heavily based off of BQ256XX Battery Charger Driver
+ * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
+ */
+
+#define BQ257XX_MANUFACTURER			"Texas Instruments"
+
+#define BQ25703_CHARGE_OPTION_0			0x00
+#define BQ25703_CHARGE_CURRENT			0x02
+#define BQ25703_MAX_CHARGE_VOLT			0x04
+#define BQ25703_OTG_VOLT			0x06
+#define BQ25703_OTG_CURRENT			0x08
+#define BQ25703_INPUT_VOLTAGE			0x0a
+#define BQ25703_MIN_VSYS			0x0c
+#define BQ25703_IIN_HOST			0x0e
+#define BQ25703_CHARGER_STATUS			0x20
+#define BQ25703_PROCHOT_STATUS			0x22
+#define BQ25703_IIN_DPM				0x24
+#define BQ25703_ADCIBAT_CHG			0x28
+#define BQ25703_ADCIINCMPIN			0x2a
+#define BQ25703_ADCVSYSVBAT			0x2c
+#define BQ25703_MANUFACT_DEV_ID			0x2e
+#define BQ25703_CHARGE_OPTION_1			0x30
+#define BQ25703_CHARGE_OPTION_2			0x32
+#define BQ25703_CHARGE_OPTION_3			0x34
+#define BQ25703_ADC_OPTION			0x3a
+
+#define BQ25703_EN_LWPWR			BIT(15)
+#define BQ25703_WDTMR_ADJ_MASK			GENMASK(14, 13)
+#define BQ25703_WDTMR_DISABLE			0
+#define BQ25703_WDTMR_5_SEC			1
+#define BQ25703_WDTMR_88_SEC			2
+#define BQ25703_WDTMR_175_SEC			3
+
+#define BQ25703_ICHG_MASK			GENMASK(12, 6)
+#define BQ25703_ICHG_STEP_UA			64000
+#define BQ25703_ICHG_MIN_UA			64000
+#define BQ25703_ICHG_MAX_UA			8128000
+
+#define BQ25703_MAX_CHARGE_VOLT_MASK		GENMASK(15, 4)
+#define BQ25703_VBATREG_STEP_UV			16000
+#define BQ25703_VBATREG_MIN_UV			1024000
+#define BQ25703_VBATREG_MAX_UV			19200000
+
+#define BQ25703_OTG_VOLT_MASK			GENMASK(13, 6)
+#define BQ25703_OTG_VOLT_STEP_UV		64000
+#define BQ25703_OTG_VOLT_MIN_UV			4480000
+#define BQ25703_OTG_VOLT_MAX_UV			20800000
+#define BQ25703_OTG_VOLT_NUM_VOLT		256
+
+#define BQ25703_OTG_CUR_MASK			GENMASK(14, 8)
+#define BQ25703_OTG_CUR_STEP_UA			50000
+#define BQ25703_OTG_CUR_MAX_UA			6350000
+
+#define BQ25703_MINVSYS_MASK			GENMASK(13, 8)
+#define BQ25703_MINVSYS_STEP_UV			256000
+#define BQ25703_MINVSYS_MIN_UV			1024000
+#define BQ25703_MINVSYS_MAX_UV			16128000
+
+#define BQ25703_STS_AC_STAT			BIT(15)
+#define BQ25703_STS_IN_FCHRG			BIT(10)
+#define BQ25703_STS_IN_PCHRG			BIT(9)
+#define BQ25703_STS_FAULT_ACOV			BIT(7)
+#define BQ25703_STS_FAULT_BATOC			BIT(6)
+#define BQ25703_STS_FAULT_ACOC			BIT(5)
+
+#define BQ25703_IINDPM_MASK			GENMASK(14, 8)
+#define BQ25703_IINDPM_STEP_UA			50000
+#define BQ25703_IINDPM_MIN_UA			50000
+#define BQ25703_IINDPM_MAX_UA			6400000
+#define BQ25703_IINDPM_DEFAULT_UA		3300000
+#define BQ25703_IINDPM_OFFSET_UA		50000
+
+#define BQ25703_ADCIBAT_DISCHG_MASK		GENMASK(6, 0)
+#define BQ25703_ADCIBAT_CHG_MASK		GENMASK(14, 8)
+#define BQ25703_ADCIBAT_CHG_STEP_UA		64000
+#define BQ25703_ADCIBAT_DIS_STEP_UA		256000
+
+#define BQ25703_ADCIIN				GENMASK(15, 8)
+#define BQ25703_ADCIINCMPIN_STEP		50000
+
+#define BQ25703_ADCVSYS_MASK			GENMASK(15, 8)
+#define BQ25703_ADCVBAT_MASK			GENMASK(7, 0)
+#define BQ25703_ADCVSYSVBAT_OFFSET_UV		2880000
+#define BQ25703_ADCVSYSVBAT_STEP		64000
+
+#define BQ25703_ADC_CH_MASK			GENMASK(7, 0)
+#define BQ25703_ADC_CONV_EN			BIT(15)
+#define BQ25703_ADC_START			BIT(14)
+#define BQ25703_ADC_FULL_SCALE			BIT(13)
+#define BQ25703_ADC_CMPIN_EN			BIT(7)
+#define BQ25703_ADC_VBUS_EN			BIT(6)
+#define BQ25703_ADC_PSYS_EN			BIT(5)
+#define BQ25703_ADC_IIN_EN			BIT(4)
+#define BQ25703_ADC_IDCHG_EN			BIT(3)
+#define BQ25703_ADC_ICHG_EN			BIT(2)
+#define BQ25703_ADC_VSYS_EN			BIT(1)
+#define BQ25703_ADC_VBAT_EN			BIT(0)
+
+#define BQ25703_EN_OTG_MASK			BIT(12)
+
+enum bq257xx_id {
+	BQ25703A,
+};
+
+/**
+ * struct bq257xx_device -
+ * @client: i2c client structure
+ * @regmap: register map structure
+ * @dev: device structure
+ * @regmap_cfg: device specific regmap cfg
+ */
+struct bq257xx_device {
+	struct i2c_client *client;
+	struct regmap *regmap;
+	struct device *dev;
+	const struct regmap_config *regmap_cfg;
+	long variant;
+};
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC 3/5] power: supply: bq257xx: Add support for BQ257XX charger manager
  2024-08-29 21:30 [RFC 0/5] Add Texas Instruments BQ25703 Charger Chris Morgan
  2024-08-29 21:30 ` [RFC 1/5] dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger Chris Morgan
  2024-08-29 21:30 ` [RFC 2/5] mfd: bq257xx: Add support for BQ25703 core driver Chris Morgan
@ 2024-08-29 21:31 ` Chris Morgan
  2024-09-16 10:49   ` Sebastian Reichel
  2024-08-29 21:31 ` [RFC 4/5] regulator: bq257xx: Add bq257xx boost regulator driver Chris Morgan
  2024-08-29 21:31 ` [RFC 5/5] arm64: dts: rockchip: Add USB and charger to Gameforce Ace Chris Morgan
  4 siblings, 1 reply; 13+ messages in thread
From: Chris Morgan @ 2024-08-29 21:31 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-rockchip, devicetree, broonie, lgirdwood, sre, heiko,
	conor+dt, krzk+dt, robh, lee, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add support for the charger function of the BQ257XX. The device is
capable of charging batteries with a layout of 1 to 4 cells in
series.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/power/supply/Kconfig           |   7 +
 drivers/power/supply/Makefile          |   1 +
 drivers/power/supply/bq257xx_charger.c | 811 +++++++++++++++++++++++++
 3 files changed, 819 insertions(+)
 create mode 100644 drivers/power/supply/bq257xx_charger.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index bcfa63fb9f1e..9c5327245264 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -720,6 +720,13 @@ config CHARGER_BQ2515X
 	  rail, ADC for battery and system monitoring, and push-button
 	  controller.
 
+config CHARGER_BQ257XX
+	tristate "TI BQ257XX battery charger family"
+	depends on MFD_BQ257XX
+	help
+	  Say Y to enable support for the TI BQ257XX family of battery
+	  charging integrated circuits.
+
 config CHARGER_BQ25890
 	tristate "TI BQ25890 battery charger driver"
 	depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 8dcb41545317..7652e9575f75 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
 obj-$(CONFIG_CHARGER_BQ24257)	+= bq24257_charger.o
 obj-$(CONFIG_CHARGER_BQ24735)	+= bq24735-charger.o
 obj-$(CONFIG_CHARGER_BQ2515X)	+= bq2515x_charger.o
+obj-$(CONFIG_CHARGER_BQ257XX)	+= bq257xx_charger.o
 obj-$(CONFIG_CHARGER_BQ25890)	+= bq25890_charger.o
 obj-$(CONFIG_CHARGER_BQ25980)	+= bq25980_charger.o
 obj-$(CONFIG_CHARGER_BQ256XX)	+= bq256xx_charger.o
diff --git a/drivers/power/supply/bq257xx_charger.c b/drivers/power/supply/bq257xx_charger.c
new file mode 100644
index 000000000000..58dcb8583897
--- /dev/null
+++ b/drivers/power/supply/bq257xx_charger.c
@@ -0,0 +1,811 @@
+// SPDX-License-Identifier: GPL-2.0
+/* BQ257XX Battery Charger Driver
+ * Copyright (C) 2024 Chris Morgan <macromorgan@hotmail.com>
+ * Based off of BQ256XX Battery Charger Driver
+ * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
+ */
+
+#include <linux/bitfield.h>
+#include <linux/i2c.h>
+#include <linux/mfd/bq257xx.h>
+#include <linux/moduleparam.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+
+/* Forward declaration of driver data. */
+struct bq257xx_chg;
+
+/**
+ * struct bq257xx_chip_info - chip specific routines
+ * @bq257xx_hw_init: init function for hw
+ * @bq257xx_hw_shutdown: shutdown function for hw
+ * @bq257xx_get_state: get and update state of hardware
+ * @bq257xx_set_ichg: set maximum charge current (in uA)
+ * @bq257xx_set_vbatreg: set maximum charge voltage (in uV)
+ * @bq257xx_set_iindpm: set maximum input current (in uA)
+ *
+ * Functions for variants of the BQ257XX IC revisions that are made
+ * available generic portions of the driver code (such as the main
+ * probe function).
+ */
+struct bq257xx_chip_info {
+	int (*bq257xx_hw_init)(struct bq257xx_chg *pdata);
+	void (*bq257xx_hw_shutdown)(struct bq257xx_chg *pdata);
+	int (*bq257xx_get_state)(struct bq257xx_chg *pdata);
+	int (*bq257xx_set_ichg)(struct bq257xx_chg *pdata, int ichg);
+	int (*bq257xx_set_vbatreg)(struct bq257xx_chg *pdata, int vbatreg);
+	int (*bq257xx_set_iindpm)(struct bq257xx_chg *pdata, int iindpm);
+};
+
+/**
+ * struct bq257xx_chg - driver data for charger
+ * @bq257xx_chip_info: hw specific functions
+ * @bq257xx_device: parent MFD device
+ * @charger: power supply device
+ * @online: charger input is present
+ * @fast_charge: charger is in fast charge mode
+ * @pre_charge: charger is in pre-charge mode
+ * @ov_fault: charger reports over voltage fault
+ * @batoc_fault: charger reports battery over current fault
+ * @oc_fault: charger reports over current fault
+ * @usb_type: USB type reported from parent power supply
+ * @supplied: Status of parent power supply
+ * @iindpm_max: maximum input current limit (uA)
+ * @vbat_max: maximum charge voltage (uV)
+ * @ichg_max: maximum charge current (uA)
+ * @vsys_min: minimum system voltage (uV)
+ */
+struct bq257xx_chg {
+	const struct bq257xx_chip_info *chip;
+	struct bq257xx_device *bq;
+	struct power_supply *charger;
+	bool online;
+	bool fast_charge;
+	bool pre_charge;
+	bool ov_fault;
+	bool batoc_fault;
+	bool oc_fault;
+	int usb_type;
+	int supplied;
+	u32 iindpm_max;
+	u32 vbat_max;
+	u32 ichg_max;
+	u32 vsys_min;
+};
+
+/**
+ * bq25703_get_state() - Get the current state of the device
+ * @pdata: driver platform data
+ *
+ * Get the current state of the charger. Check if the charger is
+ * powered, what kind of charge state (if any) the device is in,
+ * and if there are any active faults.
+ *
+ * Return: Returns 0 on success, or error on failure to read device.
+ */
+static int bq25703_get_state(struct bq257xx_chg *pdata)
+{
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(pdata->bq->regmap, BQ25703_CHARGER_STATUS, &reg);
+	if (ret)
+		return ret;
+
+	pdata->online = reg & BQ25703_STS_AC_STAT;
+	pdata->fast_charge = reg & BQ25703_STS_IN_FCHRG;
+	pdata->pre_charge = reg & BQ25703_STS_IN_PCHRG;
+	pdata->ov_fault = reg & BQ25703_STS_FAULT_ACOV;
+	pdata->batoc_fault = reg & BQ25703_STS_FAULT_BATOC;
+	pdata->oc_fault = reg & BQ25703_STS_FAULT_ACOC;
+
+	return 0;
+}
+
+/**
+ * bq25703_get_min_vsys() - Get the minimum system voltage
+ * @pdata: driver platform data
+ *
+ * Return: Returns minimum system voltage in uV on success, or error on
+ *         failure to read device.
+ */
+static int bq25703_get_min_vsys(struct bq257xx_chg *pdata)
+{
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(pdata->bq->regmap, BQ25703_MIN_VSYS,
+			  &reg);
+	if (ret)
+		return ret;
+
+	reg = FIELD_GET(BQ25703_MINVSYS_MASK, reg);
+	return (reg * BQ25703_MINVSYS_STEP_UV) + BQ25703_MINVSYS_MIN_UV;
+}
+
+/**
+ * bq25703_set_min_vsys() - Set the minimum system voltage
+ * @pdata: driver platform data
+ * @ichg: voltage value to set in uV.
+ *
+ * This function takes a requested minimum system voltage value, clamps
+ * it between the minimum supported value by the charger and a user
+ * defined minimum system value, and then writes the value to the
+ * appropriate register.
+ *
+ * Return: Returns 0 on success or error if an error occurs.
+ */
+static int bq25703_set_min_vsys(struct bq257xx_chg *pdata, int vsys)
+{
+	unsigned int reg;
+	int vsys_min = pdata->vsys_min;
+
+	vsys = clamp(vsys, BQ25703_MINVSYS_MIN_UV, vsys_min);
+	reg = ((vsys - BQ25703_MINVSYS_MIN_UV) / BQ25703_MINVSYS_STEP_UV);
+	reg = FIELD_PREP(BQ25703_MINVSYS_MASK, reg);
+
+	return regmap_write(pdata->bq->regmap, BQ25703_MIN_VSYS,
+			    reg);
+}
+
+/**
+ * bq25703_get_cur() - Get the reported current from the battery
+ * @pdata: driver platform data
+ *
+ * Return: Returns reported current from battery in uA as an absolute
+ *         value (charge and discharge are both positive integers).
+ *         Returns error if unable to read register value.
+ */
+static int bq25703_get_cur(struct bq257xx_chg *pdata)
+{
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(pdata->bq->regmap, BQ25703_ADCIBAT_CHG, &reg);
+	if (ret < 0)
+		return ret;
+
+	if (pdata->online)
+		return FIELD_GET(BQ25703_ADCIBAT_CHG_MASK, reg) *
+			BQ25703_ADCIBAT_CHG_STEP_UA;
+	else
+		return FIELD_GET(BQ25703_ADCIBAT_DISCHG_MASK, reg) *
+			BQ25703_ADCIBAT_DIS_STEP_UA;
+}
+
+/**
+ * bq25703_get_ichg_cur() - Get the maximum reported charge current
+ * @pdata: driver platform data
+ *
+ * Return: Returns maximum set charge current from charger in uA.
+ *         Returns error if unable to read register value.
+ */
+static int bq25703_get_ichg_cur(struct bq257xx_chg *pdata)
+{
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(pdata->bq->regmap, BQ25703_CHARGE_CURRENT, &reg);
+	if (ret)
+		return ret;
+
+	return FIELD_GET(BQ25703_ICHG_MASK, reg) * BQ25703_ICHG_STEP_UA;
+}
+
+/**
+ * bq25703_set_ichg_cur() - Set the maximum charge current
+ * @pdata: driver platform data
+ * @ichg: current value to set in uA.
+ *
+ * This function takes a requested maximum charge current value, clamps
+ * it between the minimum supported value by the charger and a user
+ * defined maximum charging value, and then writes the value to the
+ * appropriate register.
+ *
+ * Return: Returns 0 on success or error if an error occurs.
+ */
+static int bq25703_set_ichg_cur(struct bq257xx_chg *pdata, int ichg)
+{
+	unsigned int reg;
+	int ichg_max = pdata->ichg_max;
+
+	ichg = clamp(ichg, BQ25703_ICHG_MIN_UA, ichg_max);
+	reg = FIELD_PREP(BQ25703_ICHG_MASK, (ichg / BQ25703_ICHG_STEP_UA));
+
+	return regmap_write(pdata->bq->regmap, BQ25703_CHARGE_CURRENT,
+			    reg);
+}
+
+/**
+ * bq25703_get_chrg_volt() - Get the maximum set charge voltage
+ * @pdata: driver platform data
+ *
+ * Return: Returns maximum set charge voltage from charger in uV.
+ *         Returns error if unable to read register value.
+ */
+static int bq25703_get_chrg_volt(struct bq257xx_chg *pdata)
+{
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(pdata->bq->regmap, BQ25703_MAX_CHARGE_VOLT,
+			  &reg);
+	if (ret)
+		return ret;
+
+	return FIELD_GET(BQ25703_MAX_CHARGE_VOLT_MASK, reg) *
+		BQ25703_VBATREG_STEP_UV;
+}
+
+/**
+ * bq25703_set_chrg_volt() - Set the maximum charge voltage
+ * @pdata: driver platform data
+ * @vbat: voltage value to set in uV.
+ *
+ * This function takes a requested maximum charge voltage value, clamps
+ * it between the minimum supported value by the charger and a user
+ * defined maximum charging value, and then writes the value to the
+ * appropriate register.
+ *
+ * Return: Returns 0 on success or error if an error occurs.
+ */
+static int bq25703_set_chrg_volt(struct bq257xx_chg *pdata, int vbat)
+{
+	unsigned int reg;
+	int vbat_max = pdata->vbat_max;
+
+	vbat = clamp(vbat, BQ25703_VBATREG_MIN_UV, vbat_max);
+
+	reg = FIELD_PREP(BQ25703_MAX_CHARGE_VOLT_MASK,
+			 (vbat / BQ25703_VBATREG_STEP_UV));
+
+	return regmap_write(pdata->bq->regmap, BQ25703_MAX_CHARGE_VOLT,
+			    reg);
+}
+
+/**
+ * bq25703_get_iindpm() - Get the maximum set input current
+ * @pdata: driver platform data
+ *
+ * Read the actual input current limit from the device. This can differ
+ * from the value programmed due to some autonomous functions that may
+ * be enabled (but are not currently). This is why there is a different
+ * register used.
+ *
+ * Return: Returns maximum set input current from charger in uA.
+ *         Returns error if unable to read register value.
+ */
+static int bq25703_get_iindpm(struct bq257xx_chg *pdata)
+{
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(pdata->bq->regmap, BQ25703_IIN_DPM, &reg);
+	if (ret)
+		return ret;
+
+	reg = FIELD_GET(BQ25703_IINDPM_MASK, reg);
+	return (reg * BQ25703_IINDPM_STEP_UA) + BQ25703_IINDPM_OFFSET_UA;
+}
+
+/**
+ * bq25703_set_iindpm() - Set the maximum input current
+ * @pdata: driver platform data
+ * @iindpm: current value in uA.
+ *
+ * This function takes a requested maximum input current value, clamps
+ * it between the minimum supported value by the charger and a user
+ * defined maximum input value, and then writes the value to the
+ * appropriate register.
+ *
+ * Return: Returns 0 on success or error if an error occurs.
+ */
+static int bq25703_set_iindpm(struct bq257xx_chg *pdata, int iindpm)
+{
+	unsigned int reg;
+	int iindpm_max = pdata->iindpm_max;
+
+	iindpm = clamp(iindpm, BQ25703_IINDPM_MIN_UA, iindpm_max);
+
+	reg = ((iindpm - BQ25703_IINDPM_OFFSET_UA) / BQ25703_IINDPM_STEP_UA);
+
+	return regmap_write(pdata->bq->regmap, BQ25703_IIN_HOST,
+			    FIELD_PREP(BQ25703_IINDPM_MASK, reg));
+}
+
+/**
+ * bq25703_get_vbat() - Get the reported voltage from the battery
+ * @pdata: driver platform data
+ *
+ * Return: Returns reported voltage from battery in uV.
+ *         Returns error if unable to read register value.
+ */
+static int bq25703_get_vbat(struct bq257xx_chg *pdata)
+{
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(pdata->bq->regmap, BQ25703_ADCVSYSVBAT, &reg);
+	if (ret)
+		return ret;
+
+	reg = FIELD_GET(BQ25703_ADCVBAT_MASK, reg);
+
+	return (reg * BQ25703_ADCVSYSVBAT_STEP) + BQ25703_ADCVSYSVBAT_OFFSET_UV;
+
+}
+
+/**
+ * bq25703_hw_init() - Set all the required registers to init the charger
+ * @pdata: driver platform data
+ *
+ * Initialize the BQ25703 by first disabling the watchdog timer (which
+ * shuts off the charger in the absence of periodic writes). Then, set
+ * the charge current, charge voltage, minimum system voltage, and
+ * input current limit. Disable low power mode to allow ADCs and
+ * interrupts. Enable the ADC, start the ADC, set the ADC scale to
+ * full, and enable each individual ADC channel.
+ *
+ * Return: Returns 0 on success or error code on error.
+ */
+static int bq25703_hw_init(struct bq257xx_chg *pdata)
+{
+	int ret = 0;
+
+	regmap_update_bits(pdata->bq->regmap, BQ25703_CHARGE_OPTION_0,
+			   BQ25703_WDTMR_ADJ_MASK,
+			   FIELD_PREP(BQ25703_WDTMR_ADJ_MASK,
+				      BQ25703_WDTMR_DISABLE));
+
+	ret = pdata->chip->bq257xx_set_ichg(pdata, pdata->ichg_max);
+	if (ret)
+		return ret;
+
+	ret = pdata->chip->bq257xx_set_vbatreg(pdata, pdata->vbat_max);
+	if (ret)
+		return ret;
+
+	ret = bq25703_set_min_vsys(pdata, pdata->vsys_min);
+	if (ret)
+		return ret;
+
+	ret = pdata->chip->bq257xx_set_iindpm(pdata, pdata->iindpm_max);
+	if (ret)
+		return ret;
+
+	regmap_update_bits(pdata->bq->regmap, BQ25703_CHARGE_OPTION_0,
+			   BQ25703_EN_LWPWR, !BQ25703_EN_LWPWR);
+
+	regmap_update_bits(pdata->bq->regmap, BQ25703_ADC_OPTION,
+			   BQ25703_ADC_CONV_EN, BQ25703_ADC_CONV_EN);
+
+	regmap_update_bits(pdata->bq->regmap, BQ25703_ADC_OPTION,
+			   BQ25703_ADC_START, BQ25703_ADC_START);
+
+	regmap_update_bits(pdata->bq->regmap, BQ25703_ADC_OPTION,
+			   BQ25703_ADC_FULL_SCALE, BQ25703_ADC_FULL_SCALE);
+
+	regmap_update_bits(pdata->bq->regmap, BQ25703_ADC_OPTION,
+			   BQ25703_ADC_CH_MASK,
+			   (BQ25703_ADC_CMPIN_EN | BQ25703_ADC_VBUS_EN |
+			   BQ25703_ADC_PSYS_EN | BQ25703_ADC_IIN_EN |
+			   BQ25703_ADC_IDCHG_EN | BQ25703_ADC_ICHG_EN |
+			   BQ25703_ADC_VSYS_EN | BQ25703_ADC_VBAT_EN));
+
+	return ret;
+}
+
+/**
+ * bq25703_hw_shutdown() - Set registers for shutdown
+ * @pdata: driver platform data
+ *
+ * Enable low power mode for the device while in shutdown.
+ */
+static void bq25703_hw_shutdown(struct bq257xx_chg *pdata)
+{
+	regmap_update_bits(pdata->bq->regmap, BQ25703_CHARGE_OPTION_0,
+			   BQ25703_EN_LWPWR, BQ25703_EN_LWPWR);
+}
+
+static int bq257xx_set_charger_property(struct power_supply *psy,
+		enum power_supply_property prop,
+		const union power_supply_propval *val)
+{
+	struct bq257xx_chg *pdata = power_supply_get_drvdata(psy);
+	int ret = -EINVAL;
+
+	switch (prop) {
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		ret = pdata->chip->bq257xx_set_iindpm(pdata, val->intval);
+		if (ret)
+			return ret;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
+		ret = pdata->chip->bq257xx_set_vbatreg(pdata, val->intval);
+		if (ret)
+			return ret;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+		ret = pdata->chip->bq257xx_set_ichg(pdata, val->intval);
+		if (ret)
+			return ret;
+		break;
+
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int bq257xx_get_charger_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	struct bq257xx_chg *pdata = power_supply_get_drvdata(psy);
+	int ret = 0;
+
+	ret = pdata->chip->bq257xx_get_state(pdata);
+	if (ret)
+		return ret;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		if (!pdata->online)
+			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+		else if (!bq25703_get_cur(pdata))
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		else if (pdata->fast_charge || pdata->pre_charge)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		break;
+
+	case POWER_SUPPLY_PROP_HEALTH:
+		if (pdata->ov_fault || pdata->batoc_fault)
+			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+		else if (pdata->oc_fault)
+			val->intval = POWER_SUPPLY_HEALTH_OVERCURRENT;
+		else
+			val->intval = POWER_SUPPLY_HEALTH_GOOD;
+		break;
+
+	case POWER_SUPPLY_PROP_MANUFACTURER:
+		val->strval = BQ257XX_MANUFACTURER;
+		break;
+
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = pdata->online;
+		break;
+
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		ret = bq25703_get_iindpm(pdata);
+		if (ret < 0)
+			return ret;
+		val->intval = ret;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
+		ret = bq25703_get_chrg_volt(pdata);
+		if (ret < 0)
+			return ret;
+		val->intval = ret;
+		break;
+
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = bq25703_get_cur(pdata);
+		if (ret < 0)
+			return ret;
+
+		if (pdata->online)
+			val->intval = ret;
+		else
+			val->intval = -ret;
+		break;
+
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = bq25703_get_vbat(pdata);
+		if (ret < 0)
+			return ret;
+		val->intval = ret;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+		ret = bq25703_get_ichg_cur(pdata);
+		if (ret < 0)
+			return ret;
+		val->intval = ret;
+		break;
+
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+		ret = bq25703_get_min_vsys(pdata);
+		if (ret < 0)
+			return ret;
+		val->intval = ret;
+		break;
+
+	case POWER_SUPPLY_PROP_USB_TYPE:
+		val->intval = pdata->usb_type;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static enum power_supply_property bq257xx_power_supply_props[] = {
+	POWER_SUPPLY_PROP_MANUFACTURER,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN,
+	POWER_SUPPLY_PROP_USB_TYPE,
+};
+
+static int bq257xx_property_is_writeable(struct power_supply *psy,
+					 enum power_supply_property prop)
+{
+	switch (prop) {
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static enum power_supply_usb_type bq25703_usb_types[] = {
+	POWER_SUPPLY_USB_TYPE_C,
+	POWER_SUPPLY_USB_TYPE_PD,
+	POWER_SUPPLY_USB_TYPE_PD_DRP,
+	POWER_SUPPLY_USB_TYPE_PD_PPS,
+	POWER_SUPPLY_USB_TYPE_UNKNOWN,
+};
+
+/**
+ * bq257xx_external_power_changed() - Handler for external power change
+ * @psy: Power supply data
+ *
+ * When the external power into the charger is changed, check the USB
+ * type so that it can be reported. Additionally, update the max input
+ * current and max charging current to the value reported if it is a
+ * USB PD charger, otherwise use the default value. Note that each time
+ * a charger is removed the max charge current register is erased, so
+ * it must be set again each time the input changes or the device will
+ * not charge.
+ */
+static void bq257xx_external_power_changed(struct power_supply *psy)
+{
+	struct bq257xx_chg *pdata = power_supply_get_drvdata(psy);
+	union power_supply_propval val;
+	int ret;
+	int imax = pdata->iindpm_max;
+
+	pdata->chip->bq257xx_get_state(pdata);
+
+	pdata->supplied = power_supply_am_i_supplied(pdata->charger);
+	if (pdata->supplied < 0)
+		return;
+
+	if (pdata->supplied == 0)
+		goto out;
+
+	ret = power_supply_get_property_from_supplier(psy,
+						      POWER_SUPPLY_PROP_USB_TYPE,
+						      &val);
+	if (ret)
+		return;
+
+	pdata->usb_type = val.intval;
+
+	if ((pdata->usb_type == POWER_SUPPLY_USB_TYPE_PD) ||
+	    (pdata->usb_type == POWER_SUPPLY_USB_TYPE_PD_DRP) ||
+	    (pdata->usb_type == POWER_SUPPLY_USB_TYPE_PD)) {
+		ret = power_supply_get_property_from_supplier(psy,
+							      POWER_SUPPLY_PROP_CURRENT_MAX,
+							      &val);
+		if (ret)
+			return;
+
+		if (val.intval)
+			imax = val.intval;
+	}
+
+	if (pdata->supplied) {
+		pdata->chip->bq257xx_set_ichg(pdata, imax);
+		pdata->chip->bq257xx_set_iindpm(pdata, imax);
+		pdata->chip->bq257xx_set_vbatreg(pdata, pdata->vbat_max);
+	}
+
+out:
+	power_supply_changed(psy);
+}
+
+static irqreturn_t bq257xx_irq_handler_thread(int irq, void *private)
+{
+	struct bq257xx_chg *pdata = private;
+
+	bq257xx_external_power_changed(pdata->charger);
+	return IRQ_HANDLED;
+}
+
+static const struct power_supply_desc bq257xx_power_supply_desc = {
+	.name = "bq257xx-charger",
+	.type = POWER_SUPPLY_TYPE_USB,
+	.usb_types = bq25703_usb_types,
+	.num_usb_types = ARRAY_SIZE(bq25703_usb_types),
+	.properties = bq257xx_power_supply_props,
+	.num_properties = ARRAY_SIZE(bq257xx_power_supply_props),
+	.get_property = bq257xx_get_charger_property,
+	.set_property = bq257xx_set_charger_property,
+	.property_is_writeable = bq257xx_property_is_writeable,
+	.external_power_changed = bq257xx_external_power_changed,
+};
+
+static const struct bq257xx_chip_info bq25703_chip_info = {
+		.bq257xx_hw_init = &bq25703_hw_init,
+		.bq257xx_hw_shutdown = &bq25703_hw_shutdown,
+		.bq257xx_get_state = &bq25703_get_state,
+		.bq257xx_set_ichg = &bq25703_set_ichg_cur,
+		.bq257xx_set_vbatreg = &bq25703_set_chrg_volt,
+		.bq257xx_set_iindpm = &bq25703_set_iindpm,
+};
+
+static int bq257xx_power_supply_init(struct bq257xx_chg *pdata,
+		struct power_supply_config *psy_cfg, struct device *dev)
+{
+	pdata->charger = devm_power_supply_register(dev,
+						    &bq257xx_power_supply_desc,
+						    psy_cfg);
+	if (IS_ERR(pdata->charger)) {
+		dev_err_probe(dev, PTR_ERR(pdata->charger),
+			      "power supply register charger failed: %ld\n",
+			      PTR_ERR(pdata->charger));
+		return PTR_ERR(pdata->charger);
+	}
+
+	return 0;
+}
+
+/**
+ * bq257xx_parse_dt() - Parse the device tree for required properties
+ * @pdata: driver platform data
+ * @psy_cfg: power supply config data
+ * @dev: device struct
+ *
+ * Read the device tree to identify the minimum system voltage, the
+ * maximum charge current, the maximum charge voltage, and the maximum
+ * input current.
+ *
+ * Return: Returns 0 on success or error code on error.
+ */
+static int bq257xx_parse_dt(struct bq257xx_chg *pdata,
+		struct power_supply_config *psy_cfg, struct device *dev)
+{
+	int ret = 0;
+
+	psy_cfg->drv_data = pdata;
+	psy_cfg->of_node = dev->of_node;
+
+	ret = device_property_read_u32(dev,
+				       "ti,minimum-sys-voltage",
+				       &pdata->vsys_min);
+	if (ret) {
+		dev_err(dev, "Cannot read ti,minimum-sys-voltage from dt\n");
+		return ret;
+	}
+
+	ret = device_property_read_u32(dev,
+				       "ti,charge-current",
+				       &pdata->ichg_max);
+	if (ret) {
+		dev_err(dev, "Cannot read ti,charge-current from dt\n");
+		return ret;
+	}
+
+	ret = device_property_read_u32(dev,
+				       "ti,max-charge-voltage",
+				       &pdata->vbat_max);
+	if (ret) {
+		dev_err(dev, "Cannot read ti,max-charge-voltage from dt\n");
+		return ret;
+	}
+
+	ret = device_property_read_u32(dev,
+				       "ti,current-limit",
+				       &pdata->iindpm_max);
+	if (ret)
+		pdata->iindpm_max = BQ25703_IINDPM_DEFAULT_UA;
+
+	return 0;
+}
+
+static int bq257xx_charger_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct bq257xx_device *bq = dev_get_drvdata(pdev->dev.parent);
+	struct bq257xx_chg *pdata;
+	struct power_supply_config psy_cfg = { };
+	int ret;
+
+	pdev->dev.of_node = pdev->dev.parent->of_node;
+	pdev->dev.of_node_reused = true;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(struct bq257xx_chg), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	pdata->bq = bq;
+
+	switch (pdata->bq->variant) {
+	case BQ25703A:
+		pdata->chip = &bq25703_chip_info;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = bq257xx_parse_dt(pdata, &psy_cfg, dev);
+	if (ret) {
+		dev_err(dev, "Failed to read device tree properties%d\n", ret);
+		return ret;
+	}
+
+	ret = pdata->chip->bq257xx_hw_init(pdata);
+	if (ret) {
+		dev_err(dev, "Cannot initialize the chip.\n");
+		return ret;
+	}
+
+	ret = bq257xx_power_supply_init(pdata, &psy_cfg, dev);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, pdata);
+
+	if (bq->client->irq) {
+		ret = devm_request_threaded_irq(dev, bq->client->irq, NULL,
+						bq257xx_irq_handler_thread,
+						IRQF_TRIGGER_RISING |
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT,
+						dev_name(&bq->client->dev), pdata);
+		if (ret < 0) {
+			dev_err(dev, "get irq fail: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
+static void bq257xx_charger_shutdown(struct platform_device *pdev)
+{
+	struct bq257xx_chg *pdata = platform_get_drvdata(pdev);
+
+	pdata->chip->bq257xx_hw_shutdown(pdata);
+}
+
+static struct platform_driver bq257xx_chg_driver = {
+	.driver = {
+		.name = "bq257xx-charger",
+	},
+	.probe = bq257xx_charger_probe,
+	.shutdown = bq257xx_charger_shutdown,
+};
+module_platform_driver(bq257xx_chg_driver);
+
+MODULE_DESCRIPTION("bq257xx charger driver");
+MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
+MODULE_LICENSE("GPL");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC 4/5] regulator: bq257xx: Add bq257xx boost regulator driver
  2024-08-29 21:30 [RFC 0/5] Add Texas Instruments BQ25703 Charger Chris Morgan
                   ` (2 preceding siblings ...)
  2024-08-29 21:31 ` [RFC 3/5] power: supply: bq257xx: Add support for BQ257XX charger manager Chris Morgan
@ 2024-08-29 21:31 ` Chris Morgan
  2024-08-29 21:31 ` [RFC 5/5] arm64: dts: rockchip: Add USB and charger to Gameforce Ace Chris Morgan
  4 siblings, 0 replies; 13+ messages in thread
From: Chris Morgan @ 2024-08-29 21:31 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-rockchip, devicetree, broonie, lgirdwood, sre, heiko,
	conor+dt, krzk+dt, robh, lee, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add support for the boost regulator found in the Texas Instruments
BQ25703. The boost regulator is capable of outputting between 4.48
and 20.8 volts and between 0 and 6.35 amps.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/regulator/Kconfig             |   8 ++
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/bq257xx-regulator.c | 195 ++++++++++++++++++++++++++
 3 files changed, 204 insertions(+)
 create mode 100644 drivers/regulator/bq257xx-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 4b411a09c1a6..db432e3fb37e 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -286,6 +286,14 @@ config REGULATOR_BD96801
 	  This driver can also be built as a module. If so, the module
 	  will be called bd96801-regulator.
 
+config REGULATOR_BQ257XX
+	tristate "TI BQ257XX regulator family"
+	depends on MFD_BQ257XX
+	depends on GPIOLIB || COMPILE_TEST
+	help
+	  Say Y to enable support for the boost regulator function of
+	  the BQ257XX family of charger circuits.
+
 config REGULATOR_CPCAP
 	tristate "Motorola CPCAP regulator"
 	depends on MFD_CPCAP
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index a61fa42b13c4..6e8cdb0f554f 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_REGULATOR_BD71828) += bd71828-regulator.o
 obj-$(CONFIG_REGULATOR_BD718XX) += bd718x7-regulator.o
 obj-$(CONFIG_REGULATOR_BD9571MWV) += bd9571mwv-regulator.o
 obj-$(CONFIG_REGULATOR_BD957XMUF) += bd9576-regulator.o
+obj-$(CONFIG_REGULATOR_BQ257XX) += bq257xx-regulator.o
 obj-$(CONFIG_REGULATOR_DA903X)	+= da903x-regulator.o
 obj-$(CONFIG_REGULATOR_BD96801) += bd96801-regulator.o
 obj-$(CONFIG_REGULATOR_DA9052)	+= da9052-regulator.o
diff --git a/drivers/regulator/bq257xx-regulator.c b/drivers/regulator/bq257xx-regulator.c
new file mode 100644
index 000000000000..9246b091288c
--- /dev/null
+++ b/drivers/regulator/bq257xx-regulator.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0
+/* BQ257XX Battery Charger Driver
+ * Copyright (C) 2024 Chris Morgan <macromorgan@hotmail.com>
+ * Based off of BQ256XX Battery Charger driver and
+ * RK808 Regulator driver.
+ */
+
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/mfd/bq257xx.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+struct bq257xx_reg_data {
+	struct bq257xx_device *bq;
+	struct regulator_dev *bq257xx_reg;
+	struct gpio_desc *otg_en_gpio;
+	struct regulator_desc desc;
+};
+
+static int bq25703_vbus_get_cur_limit(struct regulator_dev *rdev)
+{
+	struct bq257xx_reg_data *pdata = rdev_get_drvdata(rdev);
+	int ret;
+	unsigned int reg;
+
+	ret = regmap_read(pdata->bq->regmap, BQ25703_OTG_CURRENT, &reg);
+	if (ret)
+		return ret;
+	return FIELD_GET(BQ25703_OTG_CUR_MASK, reg) * BQ25703_OTG_CUR_STEP_UA;
+}
+
+/*
+ * Check if the minimum current and maximum current requested are
+ * sane values, then set the register accordingly.
+ */
+static int bq25703_vbus_set_cur_limit(struct regulator_dev *rdev,
+				      int min_uA, int max_uA)
+{
+	struct bq257xx_reg_data *pdata = rdev_get_drvdata(rdev);
+	unsigned int reg;
+
+	if ((min_uA > BQ25703_OTG_CUR_MAX_UA) || (max_uA < 0))
+		return -EINVAL;
+
+	reg = (max_uA / BQ25703_OTG_CUR_STEP_UA);
+
+	/* Catch rounding errors since our step is 50000uA. */
+	if ((reg * BQ25703_OTG_CUR_STEP_UA) < min_uA)
+		return -EINVAL;
+
+	return regmap_write(pdata->bq->regmap, BQ25703_OTG_CURRENT,
+			    FIELD_PREP(BQ25703_OTG_CUR_MASK, reg));
+}
+
+static int bq25703_vbus_enable(struct regulator_dev *rdev)
+{
+	struct bq257xx_reg_data *pdata = rdev_get_drvdata(rdev);
+
+	if (pdata->otg_en_gpio)
+		gpiod_set_value_cansleep(pdata->otg_en_gpio, 1);
+	return regulator_enable_regmap(rdev);
+}
+
+static int bq25703_vbus_disable(struct regulator_dev *rdev)
+{
+	struct bq257xx_reg_data *pdata = rdev_get_drvdata(rdev);
+
+	if (pdata->otg_en_gpio)
+		gpiod_set_value_cansleep(pdata->otg_en_gpio, 0);
+	return regulator_disable_regmap(rdev);
+}
+
+static const struct regulator_ops bq25703_vbus_ops = {
+	.enable = bq25703_vbus_enable,
+	.disable = bq25703_vbus_disable,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_linear,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_current_limit = bq25703_vbus_get_cur_limit,
+	.set_current_limit = bq25703_vbus_set_cur_limit,
+};
+
+static const struct regulator_desc bq25703_vbus_desc = {
+	.name = "usb_otg_vbus",
+	.of_match = of_match_ptr("usb-otg-vbus"),
+	.regulators_node = of_match_ptr("regulators"),
+	.type = REGULATOR_VOLTAGE,
+	.owner = THIS_MODULE,
+	.ops = &bq25703_vbus_ops,
+	.min_uV = BQ25703_OTG_VOLT_MIN_UV,
+	.uV_step = BQ25703_OTG_VOLT_STEP_UV,
+	.n_voltages = BQ25703_OTG_VOLT_NUM_VOLT,
+	.enable_mask = BQ25703_EN_OTG_MASK,
+	.enable_reg = BQ25703_CHARGE_OPTION_3,
+	.enable_val = BQ25703_EN_OTG_MASK,
+	.disable_val = 0,
+	.vsel_reg = BQ25703_OTG_VOLT,
+	.vsel_mask = BQ25703_OTG_VOLT_MASK,
+};
+
+/* Get optional GPIO for OTG regulator enable. */
+static void bq257xx_reg_dt_parse_gpio(struct platform_device *pdev)
+{
+	struct device_node *child, *subchild;
+	struct bq257xx_reg_data *pdata = platform_get_drvdata(pdev);
+
+	child = of_get_child_by_name(pdev->dev.of_node,
+				     pdata->desc.regulators_node);
+	if (!child)
+		return;
+
+	subchild = of_get_child_by_name(child, pdata->desc.of_match);
+	if (!subchild)
+		return;
+
+	of_node_put(child);
+
+	pdata->otg_en_gpio = devm_fwnode_gpiod_get_index(&pdev->dev,
+							 of_fwnode_handle(subchild),
+							 "enable", 0,
+							 GPIOD_OUT_LOW,
+							 pdata->desc.of_match);
+
+	of_node_put(subchild);
+
+	if (IS_ERR_OR_NULL(pdata->otg_en_gpio)) {
+		dev_err(&pdev->dev, "Error getting enable gpio: %ld\n",
+			PTR_ERR(pdata->otg_en_gpio));
+	}
+}
+
+static int bq257xx_regulator_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct bq257xx_device *bq = dev_get_drvdata(pdev->dev.parent);
+	struct bq257xx_reg_data *pdata;
+	struct device_node *np = dev->of_node;
+	struct regulator_init_data *init_data;
+	struct regulator_config cfg = {};
+	int ret;
+
+	pdev->dev.of_node = pdev->dev.parent->of_node;
+	pdev->dev.of_node_reused = true;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(struct bq257xx_reg_data), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	pdata->bq = bq;
+
+	switch (pdata->bq->variant) {
+	case BQ25703A:
+		pdata->desc = bq25703_vbus_desc;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	platform_set_drvdata(pdev, pdata);
+	bq257xx_reg_dt_parse_gpio(pdev);
+
+	cfg.dev = &pdev->dev;
+	cfg.init_data = init_data;
+	cfg.driver_data = pdata;
+	cfg.of_node = np;
+	cfg.regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!cfg.regmap)
+		return -ENODEV;
+
+	pdata->bq257xx_reg = devm_regulator_register(dev, &pdata->desc, &cfg);
+	if (IS_ERR(pdata->bq257xx_reg)) {
+		return dev_err_probe(&pdev->dev, PTR_ERR(pdata->bq257xx_reg),
+				     "error registering bq257xx regulator");
+	}
+
+	return ret;
+}
+
+static struct platform_driver bq257xx_reg_driver = {
+	.driver = {
+		.name = "bq257xx-regulator",
+	},
+	.probe = bq257xx_regulator_probe,
+};
+
+module_platform_driver(bq257xx_reg_driver);
+
+MODULE_DESCRIPTION("bq257xx regulator driver");
+MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
+MODULE_LICENSE("GPL");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC 5/5] arm64: dts: rockchip: Add USB and charger to Gameforce Ace
  2024-08-29 21:30 [RFC 0/5] Add Texas Instruments BQ25703 Charger Chris Morgan
                   ` (3 preceding siblings ...)
  2024-08-29 21:31 ` [RFC 4/5] regulator: bq257xx: Add bq257xx boost regulator driver Chris Morgan
@ 2024-08-29 21:31 ` Chris Morgan
  2024-08-31 10:39   ` Krzysztof Kozlowski
  4 siblings, 1 reply; 13+ messages in thread
From: Chris Morgan @ 2024-08-29 21:31 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-rockchip, devicetree, broonie, lgirdwood, sre, heiko,
	conor+dt, krzk+dt, robh, lee, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add support for the BQ25703 charger manager and boost regulator to
the Gameforce Ace. This also allows us to add the USB Type-C port
manager which has a dependency on the boost regulator.

This specific patch has a dependency on the following series:
https://lore.kernel.org/linux-rockchip/20240829204517.398669-1-macroalpha82@gmail.com/

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 .../dts/rockchip/rk3588s-gameforce-ace.dts    | 120 ++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts b/arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts
index 91efb9dafc89..371f84d5ba6b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts
@@ -575,6 +575,56 @@ &i2c6 {
 	pinctrl-0 = <&i2c6m3_xfer>;
 	status = "okay";
 
+	fusb302: typec-portc@22 {
+		compatible = "fcs,fusb302";
+		reg = <0x22>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PC7 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-0 = <&usbc0_int>;
+		pinctrl-names = "default";
+		vbus-supply = <&usb_otg_vbus>;
+
+		connector {
+			compatible = "usb-c-connector";
+			data-role = "dual";
+			label = "USB-C";
+			op-sink-microwatt = <1000000>;
+			power-role = "dual";
+			self-powered;
+			sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
+				     PDO_FIXED(9000, 3000, PDO_FIXED_USB_COMM)
+				     PDO_FIXED(12000, 3000, PDO_FIXED_USB_COMM)>;
+			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
+			try-power-role = "sink";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					usbc0_orien_sw: endpoint {
+						remote-endpoint = <&usbdp_phy0_orientation_switch>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					usbc0_role_sw: endpoint {
+						remote-endpoint = <&dwc3_0_role_switch>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+					dp_altmode_mux: endpoint {
+						remote-endpoint = <&usbdp_phy0_dp_altmode_mux>;
+					};
+				};
+			};
+		};
+	};
+
 	rtc_hym8563: rtc@51 {
 		compatible = "haoyu,hym8563";
 		reg = <0x51>;
@@ -603,8 +653,40 @@ cw2015@62 {
 			 0x2F 0x00 0x64 0xA5 0xB5 0x1C 0xF0 0x49>;
 		cellwise,monitor-interval-ms = <5000>;
 		monitored-battery = <&battery>;
+		power-supplies = <&bq25703>;
 		status = "okay";
 	};
+
+	bq25703: bq25703@6b {
+		compatible = "ti,bq25703a";
+		reg = <0x6b>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PD5 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-0 = <&charger_int_h>;
+		pinctrl-names = "default";
+		power-supplies = <&fusb302>;
+		ti,charge-current = <2500000>;
+		ti,current-limit = <5000000>;
+		ti,max-charge-voltage = <8750000>;
+		ti,minimum-sys-voltage = <7400000>;
+
+		regulators {
+			usb_otg_vbus: usb-otg-vbus {
+				enable-gpios = <&gpio4 RK_PA6 GPIO_ACTIVE_HIGH>;
+				pinctrl-0 = <&boost_enable_h>;
+				pinctrl-names = "default";
+				regulator-max-microamp = <960000>;
+				regulator-max-microvolt = <5088000>;
+				regulator-min-microamp = <512000>;
+				regulator-min-microvolt = <4992000>;
+				regulator-name = "usb_otg_vbus";
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+		};
+	};
 };
 
 &i2c7 {
@@ -1235,3 +1317,41 @@ bluetooth {
 		shutdown-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_HIGH>;
 	};
 };
+
+&usb_host0_xhci {
+	usb-role-switch;
+	status = "okay";
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		dwc3_0_role_switch: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&usbc0_role_sw>;
+		};
+	};
+};
+
+&usbdp_phy0 {
+	mode-switch;
+	orientation-switch;
+	sbu1-dc-gpios = <&gpio4 RK_PA0 GPIO_ACTIVE_HIGH>;
+	sbu2-dc-gpios = <&gpio4 RK_PA1 GPIO_ACTIVE_HIGH>;
+	rockchip,dp-lane-mux = <2 3>;
+	status = "okay";
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		usbdp_phy0_orientation_switch: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&usbc0_orien_sw>;
+		};
+
+		usbdp_phy0_dp_altmode_mux: endpoint@1 {
+			reg = <1>;
+			remote-endpoint = <&dp_altmode_mux>;
+		};
+	};
+};
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC 1/5] dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger
  2024-08-29 21:30 ` [RFC 1/5] dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger Chris Morgan
@ 2024-08-30 16:30   ` Rob Herring
  2024-08-30 20:52     ` Chris Morgan
  2024-09-16  9:33     ` Sebastian Reichel
  0 siblings, 2 replies; 13+ messages in thread
From: Rob Herring @ 2024-08-30 16:30 UTC (permalink / raw)
  To: Chris Morgan
  Cc: linux-pm, linux-rockchip, devicetree, broonie, lgirdwood, sre,
	heiko, conor+dt, krzk+dt, lee, Chris Morgan

On Thu, Aug 29, 2024 at 04:30:58PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Document the Texas instruments BQ25703 series of charger managers/
> buck/boost regulators.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  .../devicetree/bindings/mfd/ti,bq25703a.yaml  | 143 ++++++++++++++++++
>  1 file changed, 143 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml b/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> new file mode 100644
> index 000000000000..e555aa60f9ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> @@ -0,0 +1,143 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/ti,bq25703a.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: BQ25703 Charger Manager/Buck/Boost Converter

BQ25703A?

> +
> +maintainers:
> +  - Chris Morgan <macromorgan@hotmail.com>
> +
> +properties:
> +  compatible:
> +    const: ti,bq25703a
> +
> +  reg:
> +    const: 0x6b
> +    description: I2C slave address

Drop description.

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  power-supplies:
> +    description:
> +      phandle of the power supply that provides input power
> +    $ref: /schemas/types.yaml#/definitions/phandle

Already has a type. You need a reference to power-supply.yaml at the 
top level and 'maxItems: 1' here.

> +
> +  ti,charge-current:
> +    description:
> +      maximum current to apply to charging the battery
> +    minimum: 0
> +    maximum: 8128000
> +    $ref: /schemas/types.yaml#/definitions/uint32

I guess this is copied from other TI parts, but really this should move 
to a property with a unit suffix. Or these shared properties moved to a 
shared schema so we aren't redefining the type multiple times.

Same for the others here.

> +
> +  ti,current-limit:
> +    description:
> +      maximum total input current allowed
> +    minimum: 50000
> +    maximum: 6400000
> +    default: 3250000
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  ti,max-charge-voltage:
> +    description:
> +      maximum voltage to apply to charging the battery
> +    minimum: 1024000
> +    maximum: 19200000
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  ti,minimum-sys-voltage:
> +    description:
> +      minimum system voltage while on battery power, with default value
> +      depending based on cell configuration
> +    minimum: 1024000
> +    maximum: 16128000
> +    default:
> +      enum: [3584000, 6144000, 9216000, 16128000]
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  regulators:
> +    type: object
> +    additionalProperties: false
> +    description:
> +      Boost converter regulator output of bq257xx

Doesn't this apply to "usb-otg-vbus"?

Really, only one regulator, so you don't need a container node.

> +
> +    properties:
> +      "usb-otg-vbus":

Don't need quotes.

> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml
> +
> +        properties:
> +          regulator-name: true
> +          regulator-min-microamp:
> +            minimum: 0
> +            maximum: 6350000
> +          regulator-max-microamp:
> +            minimum: 0
> +            maximum: 6350000
> +          regulator-min-microvolt:
> +            minimum: 4480000
> +            maximum: 20800000
> +          regulator-max-microvolt:
> +            minimum: 4480000
> +            maximum: 20800000
> +          enable-gpios:
> +            description:
> +              The BQ25703 may require both a register write and a GPIO
> +              toggle to enable the boost regulator.
> +
> +        additionalProperties: true

Nope.

> +
> +        required:
> +          - regulator-name
> +          - regulator-min-microamp
> +          - regulator-max-microamp
> +          - regulator-min-microvolt
> +          - regulator-max-microvolt
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - power-supplies
> +  - ti,charge-current
> +  - ti,current-limit
> +  - ti,max-charge-voltage
> +  - ti,minimum-sys-voltage
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/pinctrl/rockchip.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        bq25703: bq25703@6b {

charger@6b

> +            compatible = "ti,bq25703a";
> +            reg = <0x6b>;
> +            interrupt-parent = <&gpio0>;
> +            interrupts = <RK_PD5 IRQ_TYPE_LEVEL_LOW>;
> +            power-supplies = <&fusb302>;
> +            ti,charge-current = <2500000>;
> +            ti,current-limit = <5000000>;
> +            ti,max-charge-voltage = <8750000>;
> +            ti,minimum-sys-voltage = <7400000>;
> +
> +            regulators {
> +                usb_otg_vbus: usb-otg-vbus {
> +                    enable-gpios = <&gpio4 RK_PA6 GPIO_ACTIVE_HIGH>;
> +                    regulator-max-microamp = <960000>;
> +                    regulator-max-microvolt = <5088000>;
> +                    regulator-min-microamp = <512000>;
> +                    regulator-min-microvolt = <4992000>;
> +                    regulator-name = "usb_otg_vbus";
> +                };
> +            };
> +        };
> +    };
> +
> +...
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC 1/5] dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger
  2024-08-30 16:30   ` Rob Herring
@ 2024-08-30 20:52     ` Chris Morgan
  2024-09-16  9:42       ` Sebastian Reichel
  2024-09-16  9:33     ` Sebastian Reichel
  1 sibling, 1 reply; 13+ messages in thread
From: Chris Morgan @ 2024-08-30 20:52 UTC (permalink / raw)
  To: Rob Herring
  Cc: Chris Morgan, linux-pm, linux-rockchip, devicetree, broonie,
	lgirdwood, sre, heiko, conor+dt, krzk+dt, lee

On Fri, Aug 30, 2024 at 11:30:42AM -0500, Rob Herring wrote:
> On Thu, Aug 29, 2024 at 04:30:58PM -0500, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Document the Texas instruments BQ25703 series of charger managers/
> > buck/boost regulators.
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  .../devicetree/bindings/mfd/ti,bq25703a.yaml  | 143 ++++++++++++++++++
> >  1 file changed, 143 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml b/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> > new file mode 100644
> > index 000000000000..e555aa60f9ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> > @@ -0,0 +1,143 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/ti,bq25703a.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: BQ25703 Charger Manager/Buck/Boost Converter
> 
> BQ25703A?

I'm not entirely sure on the nomenclature. I know for sure I have a
BQ25703A in my device, but I don't know if the A is important or not.
I'll just drop it until I know for sure to alleviate confusion.

> 
> > +
> > +maintainers:
> > +  - Chris Morgan <macromorgan@hotmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,bq25703a
> > +
> > +  reg:
> > +    const: 0x6b
> > +    description: I2C slave address
> 
> Drop description.

Acknowledged.

> 
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  power-supplies:
> > +    description:
> > +      phandle of the power supply that provides input power
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> 
> Already has a type. You need a reference to power-supply.yaml at the 
> top level and 'maxItems: 1' here.

Gotcha, thank you.

> 
> > +
> > +  ti,charge-current:
> > +    description:
> > +      maximum current to apply to charging the battery
> > +    minimum: 0
> > +    maximum: 8128000
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> I guess this is copied from other TI parts, but really this should move 
> to a property with a unit suffix. Or these shared properties moved to a 
> shared schema so we aren't redefining the type multiple times.
> 
> Same for the others here.

Correct, I tried to reuse the same TI specific properties. I suppose I
could also eliminate ti,charge-current and ti,max-charge-voltage, and
then just require a phandle to a simple-battery node which contains
those two values in a vendor independent form. What do you think?

ti,current-limit and ti,minimum-system-voltage though are other
possible questions I have. Basically I could also create a vsys
regulator that represents the vsys coming off this device too (I
currently omit this entirely). This regulator wouldn't be programable
but would report the existing input current and input voltage for
the system, which in my case I'm pretty sure is stepped down to 5v
or less and then fed into an RK806 PMIC (I lack the schematics so
I don't know for sure). Of course if I did that then I'd have a
valid reason to add a "regulators" node since I'd have more than one
regulator.

> 
> > +
> > +  ti,current-limit:
> > +    description:
> > +      maximum total input current allowed
> > +    minimum: 50000
> > +    maximum: 6400000
> > +    default: 3250000
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  ti,max-charge-voltage:
> > +    description:
> > +      maximum voltage to apply to charging the battery
> > +    minimum: 1024000
> > +    maximum: 19200000
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  ti,minimum-sys-voltage:
> > +    description:
> > +      minimum system voltage while on battery power, with default value
> > +      depending based on cell configuration
> > +    minimum: 1024000
> > +    maximum: 16128000
> > +    default:
> > +      enum: [3584000, 6144000, 9216000, 16128000]
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  regulators:
> > +    type: object
> > +    additionalProperties: false
> > +    description:
> > +      Boost converter regulator output of bq257xx
> 
> Doesn't this apply to "usb-otg-vbus"?
> 
> Really, only one regulator, so you don't need a container node.
> 

It does, that's a mistake on my part. And see above; I could add a
regulator for the vsys (even though you wouldn't be able to set the
voltage or current).

> > +
> > +    properties:
> > +      "usb-otg-vbus":
> 
> Don't need quotes.

Acknowledged.

> 
> > +        type: object
> > +        $ref: /schemas/regulator/regulator.yaml
> > +
> > +        properties:
> > +          regulator-name: true
> > +          regulator-min-microamp:
> > +            minimum: 0
> > +            maximum: 6350000
> > +          regulator-max-microamp:
> > +            minimum: 0
> > +            maximum: 6350000
> > +          regulator-min-microvolt:
> > +            minimum: 4480000
> > +            maximum: 20800000
> > +          regulator-max-microvolt:
> > +            minimum: 4480000
> > +            maximum: 20800000
> > +          enable-gpios:
> > +            description:
> > +              The BQ25703 may require both a register write and a GPIO
> > +              toggle to enable the boost regulator.
> > +
> > +        additionalProperties: true
> 
> Nope.
> 

Acknowledged.

> > +
> > +        required:
> > +          - regulator-name
> > +          - regulator-min-microamp
> > +          - regulator-max-microamp
> > +          - regulator-min-microvolt
> > +          - regulator-max-microvolt
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - power-supplies
> > +  - ti,charge-current
> > +  - ti,current-limit
> > +  - ti,max-charge-voltage
> > +  - ti,minimum-sys-voltage
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/pinctrl/rockchip.h>
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        bq25703: bq25703@6b {
> 
> charger@6b
> 

Acknowledged.

> > +            compatible = "ti,bq25703a";
> > +            reg = <0x6b>;
> > +            interrupt-parent = <&gpio0>;
> > +            interrupts = <RK_PD5 IRQ_TYPE_LEVEL_LOW>;
> > +            power-supplies = <&fusb302>;
> > +            ti,charge-current = <2500000>;
> > +            ti,current-limit = <5000000>;
> > +            ti,max-charge-voltage = <8750000>;
> > +            ti,minimum-sys-voltage = <7400000>;
> > +
> > +            regulators {
> > +                usb_otg_vbus: usb-otg-vbus {
> > +                    enable-gpios = <&gpio4 RK_PA6 GPIO_ACTIVE_HIGH>;
> > +                    regulator-max-microamp = <960000>;
> > +                    regulator-max-microvolt = <5088000>;
> > +                    regulator-min-microamp = <512000>;
> > +                    regulator-min-microvolt = <4992000>;
> > +                    regulator-name = "usb_otg_vbus";
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> > -- 
> > 2.34.1
> > 

Thank you for your feedback,
Chris

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC 5/5] arm64: dts: rockchip: Add USB and charger to Gameforce Ace
  2024-08-29 21:31 ` [RFC 5/5] arm64: dts: rockchip: Add USB and charger to Gameforce Ace Chris Morgan
@ 2024-08-31 10:39   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-31 10:39 UTC (permalink / raw)
  To: Chris Morgan, linux-pm
  Cc: linux-rockchip, devicetree, broonie, lgirdwood, sre, heiko,
	conor+dt, krzk+dt, robh, lee, Chris Morgan

On 29/08/2024 23:31, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add support for the BQ25703 charger manager and boost regulator to
> the Gameforce Ace. This also allows us to add the USB Type-C port
> manager which has a dependency on the boost regulator.
> 
> This specific patch has a dependency on the following series:
> https://lore.kernel.org/linux-rockchip/20240829204517.398669-1-macroalpha82@gmail.com/
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  .../dts/rockchip/rk3588s-gameforce-ace.dts    | 120 ++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts b/arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts
> index 91efb9dafc89..371f84d5ba6b 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-gameforce-ace.dts
> @@ -575,6 +575,56 @@ &i2c6 {
>  	pinctrl-0 = <&i2c6m3_xfer>;
>  	status = "okay";
>  
> +	fusb302: typec-portc@22 {

typec@


> +		compatible = "fcs,fusb302";
> +		reg = <0x22>;
> +		interrupt-parent = <&gpio0>;
> +		interrupts = <RK_PC7 IRQ_TYPE_LEVEL_LOW>;
> +		pinctrl-0 = <&usbc0_int>;
> +		pinctrl-names = "default";
> +		vbus-supply = <&usb_otg_vbus>;
> +
> +		connector {
> +			compatible = "usb-c-connector";
> +			data-role = "dual";
> +			label = "USB-C";
> +			op-sink-microwatt = <1000000>;
> +			power-role = "dual";
> +			self-powered;
> +			sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> +				     PDO_FIXED(9000, 3000, PDO_FIXED_USB_COMM)
> +				     PDO_FIXED(12000, 3000, PDO_FIXED_USB_COMM)>;
> +			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> +			try-power-role = "sink";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +					usbc0_orien_sw: endpoint {
> +						remote-endpoint = <&usbdp_phy0_orientation_switch>;
> +					};
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +					usbc0_role_sw: endpoint {
> +						remote-endpoint = <&dwc3_0_role_switch>;
> +					};
> +				};
> +
> +				port@2 {
> +					reg = <2>;
> +					dp_altmode_mux: endpoint {
> +						remote-endpoint = <&usbdp_phy0_dp_altmode_mux>;
> +					};
> +				};
> +			};
> +		};
> +	};
> +
>  	rtc_hym8563: rtc@51 {
>  		compatible = "haoyu,hym8563";
>  		reg = <0x51>;
> @@ -603,8 +653,40 @@ cw2015@62 {
>  			 0x2F 0x00 0x64 0xA5 0xB5 0x1C 0xF0 0x49>;
>  		cellwise,monitor-interval-ms = <5000>;
>  		monitored-battery = <&battery>;
> +		power-supplies = <&bq25703>;
>  		status = "okay";
>  	};
> +
> +	bq25703: bq25703@6b {

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,bq25703a";
> +		reg = <0x6b>;
> +		interrupt-parent = <&gpio0>;
> +		interrupts = <RK_PD5 IRQ_TYPE_LEVEL_LOW>;
> +		pinctrl-0 = <&charger_int_h>;
> +		pinctrl-names = "default";
> +		power-supplies = <&fusb302>;



Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC 2/5] mfd: bq257xx: Add support for BQ25703 core driver
  2024-08-29 21:30 ` [RFC 2/5] mfd: bq257xx: Add support for BQ25703 core driver Chris Morgan
@ 2024-09-03 15:25   ` Lee Jones
  0 siblings, 0 replies; 13+ messages in thread
From: Lee Jones @ 2024-09-03 15:25 UTC (permalink / raw)
  To: Chris Morgan
  Cc: linux-pm, linux-rockchip, devicetree, broonie, lgirdwood, sre,
	heiko, conor+dt, krzk+dt, robh, Chris Morgan

On Thu, 29 Aug 2024, Chris Morgan wrote:

> From: Chris Morgan <macromorgan@hotmail.com>
> 
> The Texas Instruments BQ25703A is an integrated charger manager and
> boost converter.
> 
> The MFD driver initalizes the device for the regulator driver
> and power supply driver.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/mfd/Kconfig         |  11 ++++
>  drivers/mfd/Makefile        |   1 +
>  drivers/mfd/bq257xx.c       | 118 +++++++++++++++++++++++++++++++++++
>  include/linux/mfd/bq257xx.h | 120 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 250 insertions(+)
>  create mode 100644 drivers/mfd/bq257xx.c
>  create mode 100644 include/linux/mfd/bq257xx.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index bc8be2e593b6..712951ae7341 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1537,6 +1537,17 @@ config MFD_TI_LMU
>  	  LM36274.  It consists of backlight, LED and regulator driver.
>  	  It provides consistent device controls for lighting functions.
>  
> +config MFD_BQ257XX
> +	tristate "TI BQ257XX Buck/Boost Charge Controller"
> +	depends on I2C
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	help
> +	  Support Texas Instruments BQ25703 Buck/Boost converter with
> +	  charge controller. It consists of regulators that provide
> +	  system voltage and OTG voltage, and a charger manager for
> +	  batteries containing one or more cells.
> +
>  config MFD_OMAP_USB_HOST
>  	bool "TI OMAP USBHS core and TLL driver"
>  	depends on USB_EHCI_HCD_OMAP || USB_OHCI_HCD_OMAP3
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 02b651cd7535..90bc65d83c5f 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_MFD_SM501)		+= sm501.o
>  obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835-pm.o
>  obj-$(CONFIG_MFD_BCM590XX)	+= bcm590xx.o
>  obj-$(CONFIG_MFD_BD9571MWV)	+= bd9571mwv.o
> +obj-$(CONFIG_MFD_BQ257XX)	+= bq257xx.o
>  obj-$(CONFIG_MFD_CROS_EC_DEV)	+= cros_ec_dev.o
>  obj-$(CONFIG_MFD_CS42L43)	+= cs42l43.o
>  obj-$(CONFIG_MFD_CS42L43_I2C)	+= cs42l43-i2c.o
> diff --git a/drivers/mfd/bq257xx.c b/drivers/mfd/bq257xx.c
> new file mode 100644
> index 000000000000..c612262f9a1e
> --- /dev/null
> +++ b/drivers/mfd/bq257xx.c
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* BQ257XX MFD Driver

The top line of a multi-line comment should be empty.

Please drop _all_ mentions of "MFD" - it's not a thing.

> + * Copyright (C) 2024 Chris Morgan <macromorgan@hotmail.com>

'\n' here.

> + * Based off of BQ256XX Battery Charger Driver and
> + * Rockchip RK808 MFD Driver

You really don't have to mention this.

9 out of every 10 (I just made those figures up) drivers in the kernel
take inspiration from existing ones.  These sentences don't add
anything IMHO.

> + */
> +
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/mfd/bq257xx.h>
> +#include <linux/mfd/core.h>
> +#include <linux/regmap.h>
> +
> +static const struct regmap_range bq25703_readonly_reg_ranges[] = {
> +	regmap_reg_range(BQ25703_CHARGER_STATUS, BQ25703_MANUFACT_DEV_ID),
> +};
> +
> +static const struct regmap_access_table bq25703_writeable_regs = {
> +	.no_ranges = bq25703_readonly_reg_ranges,
> +	.n_no_ranges = ARRAY_SIZE(bq25703_readonly_reg_ranges),
> +};
> +
> +static const struct regmap_range bq25703_volatile_reg_ranges[] = {
> +	regmap_reg_range(BQ25703_CHARGE_OPTION_0, BQ25703_IIN_HOST),
> +	regmap_reg_range(BQ25703_CHARGER_STATUS, BQ25703_ADC_OPTION),
> +};
> +
> +static const struct regmap_access_table bq25703_volatile_regs = {
> +	.yes_ranges = bq25703_volatile_reg_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(bq25703_volatile_reg_ranges),
> +};
> +
> +static const struct regmap_config bq25703_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 16,
> +	.max_register = BQ25703_ADC_OPTION,
> +	.cache_type = REGCACHE_RBTREE,
> +	.wr_table = &bq25703_writeable_regs,
> +	.volatile_table = &bq25703_volatile_regs,
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +static const struct mfd_cell bq25703_cells[] = {
> +	MFD_CELL_NAME("bq257xx-regulator"),
> +	MFD_CELL_NAME("bq257xx-charger"),
> +};
> +
> +static int bq257xx_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct bq257xx_device *bq;

Call this ddata.

> +	const struct mfd_cell *cells;
> +	int nr_cells;
> +	int ret = 0;

The pre-initialisation is redundant.

> +	bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
> +	if (!bq)
> +		return -ENOMEM;
> +
> +	bq->client = client;
> +	bq->dev = dev;

You don't need both.

> +	bq->variant = (long)i2c_get_match_data(client);

Pretty sure this cast is not correct and maybe not even required.

> +	switch (bq->variant) {
> +	case BQ25703A:

I wouldn't add all of this complexity until its needed.

> +		bq->regmap_cfg = &bq25703_regmap_config;

Where is this consumed?

> +		cells = bq25703_cells;
> +		nr_cells = ARRAY_SIZE(bq25703_cells);
> +		break;
> +	default:
> +		dev_err(dev, "Unsupported BQ257XX ID %ld\n", bq->variant);
> +		return -EINVAL;
> +	}
> +
> +	bq->regmap = devm_regmap_init_i2c(client, bq->regmap_cfg);
> +

Remove this line.

> +	if (IS_ERR(bq->regmap)) {
> +		dev_err(dev, "Failed to allocate register map\n");
> +		return PTR_ERR(bq->regmap);
> +	}
> +
> +	i2c_set_clientdata(client, bq);
> +
> +	ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO,
> +				   cells, nr_cells, NULL, 0, NULL);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to add MFD devices %d\n", ret);

No such thing.

"Failed to register child devices"
q
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct i2c_device_id bq257xx_i2c_ids[] = {
> +	{ "bq25703a" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, bq257xx_i2c_ids);
> +
> +static const struct of_device_id bq257xx_of_match[] = {
> +	{ .compatible = "ti,bq25703a", .data = (void *)BQ25703A, },

Cast almost certainly not required.

> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, bq257xx_of_match);
> +
> +static struct i2c_driver bq257xx_driver = {
> +	.driver = {
> +		.name = "bq257xx",
> +		.of_match_table = bq257xx_of_match,
> +	},
> +	.probe = bq257xx_probe,
> +	.id_table = bq257xx_i2c_ids,
> +};
> +module_i2c_driver(bq257xx_driver);
> +
> +MODULE_DESCRIPTION("bq257xx buck/boost/charger MFD driver");

Not an "MFD driver"

> +MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/bq257xx.h b/include/linux/mfd/bq257xx.h
> new file mode 100644
> index 000000000000..51f6501c6441
> --- /dev/null
> +++ b/include/linux/mfd/bq257xx.h
> @@ -0,0 +1,120 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + * Register definitions for TI BQ257XX
> + * Heavily based off of BQ256XX Battery Charger Driver
> + * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
> + */

All as above.

> +#define BQ257XX_MANUFACTURER			"Texas Instruments"

Errr, no thank you.  Just use strings where you need them.

> +#define BQ25703_CHARGE_OPTION_0			0x00
> +#define BQ25703_CHARGE_CURRENT			0x02
> +#define BQ25703_MAX_CHARGE_VOLT			0x04
> +#define BQ25703_OTG_VOLT			0x06
> +#define BQ25703_OTG_CURRENT			0x08
> +#define BQ25703_INPUT_VOLTAGE			0x0a
> +#define BQ25703_MIN_VSYS			0x0c
> +#define BQ25703_IIN_HOST			0x0e
> +#define BQ25703_CHARGER_STATUS			0x20
> +#define BQ25703_PROCHOT_STATUS			0x22
> +#define BQ25703_IIN_DPM				0x24
> +#define BQ25703_ADCIBAT_CHG			0x28
> +#define BQ25703_ADCIINCMPIN			0x2a
> +#define BQ25703_ADCVSYSVBAT			0x2c
> +#define BQ25703_MANUFACT_DEV_ID			0x2e
> +#define BQ25703_CHARGE_OPTION_1			0x30
> +#define BQ25703_CHARGE_OPTION_2			0x32
> +#define BQ25703_CHARGE_OPTION_3			0x34
> +#define BQ25703_ADC_OPTION			0x3a
> +
> +#define BQ25703_EN_LWPWR			BIT(15)
> +#define BQ25703_WDTMR_ADJ_MASK			GENMASK(14, 13)
> +#define BQ25703_WDTMR_DISABLE			0
> +#define BQ25703_WDTMR_5_SEC			1
> +#define BQ25703_WDTMR_88_SEC			2
> +#define BQ25703_WDTMR_175_SEC			3
> +
> +#define BQ25703_ICHG_MASK			GENMASK(12, 6)
> +#define BQ25703_ICHG_STEP_UA			64000
> +#define BQ25703_ICHG_MIN_UA			64000
> +#define BQ25703_ICHG_MAX_UA			8128000
> +
> +#define BQ25703_MAX_CHARGE_VOLT_MASK		GENMASK(15, 4)
> +#define BQ25703_VBATREG_STEP_UV			16000
> +#define BQ25703_VBATREG_MIN_UV			1024000
> +#define BQ25703_VBATREG_MAX_UV			19200000
> +
> +#define BQ25703_OTG_VOLT_MASK			GENMASK(13, 6)
> +#define BQ25703_OTG_VOLT_STEP_UV		64000
> +#define BQ25703_OTG_VOLT_MIN_UV			4480000
> +#define BQ25703_OTG_VOLT_MAX_UV			20800000
> +#define BQ25703_OTG_VOLT_NUM_VOLT		256
> +
> +#define BQ25703_OTG_CUR_MASK			GENMASK(14, 8)
> +#define BQ25703_OTG_CUR_STEP_UA			50000
> +#define BQ25703_OTG_CUR_MAX_UA			6350000
> +
> +#define BQ25703_MINVSYS_MASK			GENMASK(13, 8)
> +#define BQ25703_MINVSYS_STEP_UV			256000
> +#define BQ25703_MINVSYS_MIN_UV			1024000
> +#define BQ25703_MINVSYS_MAX_UV			16128000
> +
> +#define BQ25703_STS_AC_STAT			BIT(15)
> +#define BQ25703_STS_IN_FCHRG			BIT(10)
> +#define BQ25703_STS_IN_PCHRG			BIT(9)
> +#define BQ25703_STS_FAULT_ACOV			BIT(7)
> +#define BQ25703_STS_FAULT_BATOC			BIT(6)
> +#define BQ25703_STS_FAULT_ACOC			BIT(5)
> +
> +#define BQ25703_IINDPM_MASK			GENMASK(14, 8)
> +#define BQ25703_IINDPM_STEP_UA			50000
> +#define BQ25703_IINDPM_MIN_UA			50000
> +#define BQ25703_IINDPM_MAX_UA			6400000
> +#define BQ25703_IINDPM_DEFAULT_UA		3300000
> +#define BQ25703_IINDPM_OFFSET_UA		50000
> +
> +#define BQ25703_ADCIBAT_DISCHG_MASK		GENMASK(6, 0)
> +#define BQ25703_ADCIBAT_CHG_MASK		GENMASK(14, 8)
> +#define BQ25703_ADCIBAT_CHG_STEP_UA		64000
> +#define BQ25703_ADCIBAT_DIS_STEP_UA		256000
> +
> +#define BQ25703_ADCIIN				GENMASK(15, 8)
> +#define BQ25703_ADCIINCMPIN_STEP		50000
> +
> +#define BQ25703_ADCVSYS_MASK			GENMASK(15, 8)
> +#define BQ25703_ADCVBAT_MASK			GENMASK(7, 0)
> +#define BQ25703_ADCVSYSVBAT_OFFSET_UV		2880000
> +#define BQ25703_ADCVSYSVBAT_STEP		64000
> +
> +#define BQ25703_ADC_CH_MASK			GENMASK(7, 0)
> +#define BQ25703_ADC_CONV_EN			BIT(15)
> +#define BQ25703_ADC_START			BIT(14)
> +#define BQ25703_ADC_FULL_SCALE			BIT(13)
> +#define BQ25703_ADC_CMPIN_EN			BIT(7)
> +#define BQ25703_ADC_VBUS_EN			BIT(6)
> +#define BQ25703_ADC_PSYS_EN			BIT(5)
> +#define BQ25703_ADC_IIN_EN			BIT(4)
> +#define BQ25703_ADC_IDCHG_EN			BIT(3)
> +#define BQ25703_ADC_ICHG_EN			BIT(2)
> +#define BQ25703_ADC_VSYS_EN			BIT(1)
> +#define BQ25703_ADC_VBAT_EN			BIT(0)
> +
> +#define BQ25703_EN_OTG_MASK			BIT(12)
> +
> +enum bq257xx_id {
> +	BQ25703A,
> +};
> +
> +/**

Are you sure you want to use kernel-doc here?

I would advise against it.

> + * struct bq257xx_device -
> + * @client: i2c client structure
> + * @regmap: register map structure
> + * @dev: device structure
> + * @regmap_cfg: device specific regmap cfg

It's also wrong.  Did you test build with W=1?

> + */
> +struct bq257xx_device {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +	struct device *dev;
> +	const struct regmap_config *regmap_cfg;
> +	long variant;
> +};
> -- 
> 2.34.1
> 

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC 1/5] dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger
  2024-08-30 16:30   ` Rob Herring
  2024-08-30 20:52     ` Chris Morgan
@ 2024-09-16  9:33     ` Sebastian Reichel
  1 sibling, 0 replies; 13+ messages in thread
From: Sebastian Reichel @ 2024-09-16  9:33 UTC (permalink / raw)
  To: Rob Herring
  Cc: Chris Morgan, linux-pm, linux-rockchip, devicetree, broonie,
	lgirdwood, heiko, conor+dt, krzk+dt, lee, Chris Morgan

[-- Attachment #1: Type: text/plain, Size: 6544 bytes --]

Hi,

On Fri, Aug 30, 2024 at 11:30:42AM GMT, Rob Herring wrote:
> On Thu, Aug 29, 2024 at 04:30:58PM -0500, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Document the Texas instruments BQ25703 series of charger managers/
> > buck/boost regulators.
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  .../devicetree/bindings/mfd/ti,bq25703a.yaml  | 143 ++++++++++++++++++
> >  1 file changed, 143 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml b/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> > new file mode 100644
> > index 000000000000..e555aa60f9ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/ti,bq25703a.yaml
> > @@ -0,0 +1,143 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/ti,bq25703a.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: BQ25703 Charger Manager/Buck/Boost Converter
> 
> BQ25703A?
> 
> > +
> > +maintainers:
> > +  - Chris Morgan <macromorgan@hotmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,bq25703a
> > +
> > +  reg:
> > +    const: 0x6b
> > +    description: I2C slave address
> 
> Drop description.
> 
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  power-supplies:
> > +    description:
> > +      phandle of the power supply that provides input power
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> 
> Already has a type. You need a reference to power-supply.yaml at the 
> top level and 'maxItems: 1' here.
> 
> > +
> > +  ti,charge-current:
> > +    description:
> > +      maximum current to apply to charging the battery
> > +    minimum: 0
> > +    maximum: 8128000
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> I guess this is copied from other TI parts, but really this should move 
> to a property with a unit suffix. Or these shared properties moved to a 
> shared schema so we aren't redefining the type multiple times.
> 
> Same for the others here.

This is effectively information about the battery cells. The driver
should use the simple battery infrastructure and reference it via
monitored-battery. See also:

Documentation/devicetree/bindings/power/supply/battery.yaml

> > +  ti,current-limit:
> > +    description:
> > +      maximum total input current allowed
> > +    minimum: 50000
> > +    maximum: 6400000
> > +    default: 3250000
> > +    $ref: /schemas/types.yaml#/definitions/uint32

Use "input-current-limit-microamp" as property name.

> > +  ti,max-charge-voltage:
> > +    description:
> > +      maximum voltage to apply to charging the battery
> > +    minimum: 1024000
> > +    maximum: 19200000
> > +    $ref: /schemas/types.yaml#/definitions/uint32

That also belong into the simple-battery node.

> > +  ti,minimum-sys-voltage:
> > +    description:
> > +      minimum system voltage while on battery power, with default value
> > +      depending based on cell configuration
> > +    minimum: 1024000
> > +    maximum: 16128000
> > +    default:
> > +      enum: [3584000, 6144000, 9216000, 16128000]
> > +    $ref: /schemas/types.yaml#/definitions/uint32

Property should have -uvolt suffix.

Greetings,

-- Sebastian

> > +  regulators:
> > +    type: object
> > +    additionalProperties: false
> > +    description:
> > +      Boost converter regulator output of bq257xx
> 
> Doesn't this apply to "usb-otg-vbus"?
> 
> Really, only one regulator, so you don't need a container node.
> 
> > +
> > +    properties:
> > +      "usb-otg-vbus":
> 
> Don't need quotes.
> 
> > +        type: object
> > +        $ref: /schemas/regulator/regulator.yaml
> > +
> > +        properties:
> > +          regulator-name: true
> > +          regulator-min-microamp:
> > +            minimum: 0
> > +            maximum: 6350000
> > +          regulator-max-microamp:
> > +            minimum: 0
> > +            maximum: 6350000
> > +          regulator-min-microvolt:
> > +            minimum: 4480000
> > +            maximum: 20800000
> > +          regulator-max-microvolt:
> > +            minimum: 4480000
> > +            maximum: 20800000
> > +          enable-gpios:
> > +            description:
> > +              The BQ25703 may require both a register write and a GPIO
> > +              toggle to enable the boost regulator.
> > +
> > +        additionalProperties: true
> 
> Nope.
> 
> > +
> > +        required:
> > +          - regulator-name
> > +          - regulator-min-microamp
> > +          - regulator-max-microamp
> > +          - regulator-min-microvolt
> > +          - regulator-max-microvolt
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - power-supplies
> > +  - ti,charge-current
> > +  - ti,current-limit
> > +  - ti,max-charge-voltage
> > +  - ti,minimum-sys-voltage
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/pinctrl/rockchip.h>
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        bq25703: bq25703@6b {
> 
> charger@6b
> 
> > +            compatible = "ti,bq25703a";
> > +            reg = <0x6b>;
> > +            interrupt-parent = <&gpio0>;
> > +            interrupts = <RK_PD5 IRQ_TYPE_LEVEL_LOW>;
> > +            power-supplies = <&fusb302>;
> > +            ti,charge-current = <2500000>;
> > +            ti,current-limit = <5000000>;
> > +            ti,max-charge-voltage = <8750000>;
> > +            ti,minimum-sys-voltage = <7400000>;
> > +
> > +            regulators {
> > +                usb_otg_vbus: usb-otg-vbus {
> > +                    enable-gpios = <&gpio4 RK_PA6 GPIO_ACTIVE_HIGH>;
> > +                    regulator-max-microamp = <960000>;
> > +                    regulator-max-microvolt = <5088000>;
> > +                    regulator-min-microamp = <512000>;
> > +                    regulator-min-microvolt = <4992000>;
> > +                    regulator-name = "usb_otg_vbus";
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> > -- 
> > 2.34.1
> > 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC 1/5] dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger
  2024-08-30 20:52     ` Chris Morgan
@ 2024-09-16  9:42       ` Sebastian Reichel
  0 siblings, 0 replies; 13+ messages in thread
From: Sebastian Reichel @ 2024-09-16  9:42 UTC (permalink / raw)
  To: Chris Morgan
  Cc: Rob Herring, Chris Morgan, linux-pm, linux-rockchip, devicetree,
	broonie, lgirdwood, heiko, conor+dt, krzk+dt, lee

[-- Attachment #1: Type: text/plain, Size: 1776 bytes --]

Hi,

On Fri, Aug 30, 2024 at 03:52:04PM GMT, Chris Morgan wrote:
> > > +
> > > +  ti,charge-current:
> > > +    description:
> > > +      maximum current to apply to charging the battery
> > > +    minimum: 0
> > > +    maximum: 8128000
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > 
> > I guess this is copied from other TI parts, but really this should move 
> > to a property with a unit suffix. Or these shared properties moved to a 
> > shared schema so we aren't redefining the type multiple times.
> > 
> > Same for the others here.
> 
> Correct, I tried to reuse the same TI specific properties. I suppose I
> could also eliminate ti,charge-current and ti,max-charge-voltage, and
> then just require a phandle to a simple-battery node which contains
> those two values in a vendor independent form. What do you think?

Yes. The bindings using vendor specific properties for this are from
before the simple battery binding existed.

> ti,current-limit and ti,minimum-system-voltage though are other
> possible questions I have. Basically I could also create a vsys
> regulator that represents the vsys coming off this device too (I
> currently omit this entirely). This regulator wouldn't be programable
> but would report the existing input current and input voltage for
> the system, which in my case I'm pretty sure is stepped down to 5v
> or less and then fed into an RK806 PMIC (I lack the schematics so
> I don't know for sure). Of course if I did that then I'd have a
> valid reason to add a "regulators" node since I'd have more than one
> regulator.

I like this approach, since it allows properly describing the
hardware setup in DT and avoids the vendor specific properties.

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC 3/5] power: supply: bq257xx: Add support for BQ257XX charger manager
  2024-08-29 21:31 ` [RFC 3/5] power: supply: bq257xx: Add support for BQ257XX charger manager Chris Morgan
@ 2024-09-16 10:49   ` Sebastian Reichel
  0 siblings, 0 replies; 13+ messages in thread
From: Sebastian Reichel @ 2024-09-16 10:49 UTC (permalink / raw)
  To: Chris Morgan
  Cc: linux-pm, linux-rockchip, devicetree, broonie, lgirdwood, heiko,
	conor+dt, krzk+dt, robh, lee, Chris Morgan

[-- Attachment #1: Type: text/plain, Size: 29003 bytes --]

Hi,

On Thu, Aug 29, 2024 at 04:31:00PM GMT, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add support for the charger function of the BQ257XX. The device is
> capable of charging batteries with a layout of 1 to 4 cells in
> series.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---

I skipped reviewing the parts related to the requested binding changes.

>  drivers/power/supply/Kconfig           |   7 +
>  drivers/power/supply/Makefile          |   1 +
>  drivers/power/supply/bq257xx_charger.c | 811 +++++++++++++++++++++++++
>  3 files changed, 819 insertions(+)
>  create mode 100644 drivers/power/supply/bq257xx_charger.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index bcfa63fb9f1e..9c5327245264 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -720,6 +720,13 @@ config CHARGER_BQ2515X
>  	  rail, ADC for battery and system monitoring, and push-button
>  	  controller.
>  
> +config CHARGER_BQ257XX
> +	tristate "TI BQ257XX battery charger family"
> +	depends on MFD_BQ257XX
> +	help
> +	  Say Y to enable support for the TI BQ257XX family of battery
> +	  charging integrated circuits.
> +
>  config CHARGER_BQ25890
>  	tristate "TI BQ25890 battery charger driver"
>  	depends on I2C
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 8dcb41545317..7652e9575f75 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -93,6 +93,7 @@ obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
>  obj-$(CONFIG_CHARGER_BQ24257)	+= bq24257_charger.o
>  obj-$(CONFIG_CHARGER_BQ24735)	+= bq24735-charger.o
>  obj-$(CONFIG_CHARGER_BQ2515X)	+= bq2515x_charger.o
> +obj-$(CONFIG_CHARGER_BQ257XX)	+= bq257xx_charger.o
>  obj-$(CONFIG_CHARGER_BQ25890)	+= bq25890_charger.o
>  obj-$(CONFIG_CHARGER_BQ25980)	+= bq25980_charger.o
>  obj-$(CONFIG_CHARGER_BQ256XX)	+= bq256xx_charger.o
> diff --git a/drivers/power/supply/bq257xx_charger.c b/drivers/power/supply/bq257xx_charger.c
> new file mode 100644
> index 000000000000..58dcb8583897
> --- /dev/null
> +++ b/drivers/power/supply/bq257xx_charger.c
> @@ -0,0 +1,811 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* BQ257XX Battery Charger Driver
> + * Copyright (C) 2024 Chris Morgan <macromorgan@hotmail.com>
> + * Based off of BQ256XX Battery Charger Driver
> + * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/i2c.h>

is the i2c header actually used?

> +#include <linux/mfd/bq257xx.h>
> +#include <linux/moduleparam.h>

development leftover?

> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +
> +/* Forward declaration of driver data. */
> +struct bq257xx_chg;
> +
> +/**
> + * struct bq257xx_chip_info - chip specific routines
> + * @bq257xx_hw_init: init function for hw
> + * @bq257xx_hw_shutdown: shutdown function for hw
> + * @bq257xx_get_state: get and update state of hardware
> + * @bq257xx_set_ichg: set maximum charge current (in uA)
> + * @bq257xx_set_vbatreg: set maximum charge voltage (in uV)
> + * @bq257xx_set_iindpm: set maximum input current (in uA)
> + *
> + * Functions for variants of the BQ257XX IC revisions that are made
> + * available generic portions of the driver code (such as the main
> + * probe function).
> + */
> +struct bq257xx_chip_info {
> +	int (*bq257xx_hw_init)(struct bq257xx_chg *pdata);
> +	void (*bq257xx_hw_shutdown)(struct bq257xx_chg *pdata);
> +	int (*bq257xx_get_state)(struct bq257xx_chg *pdata);
> +	int (*bq257xx_set_ichg)(struct bq257xx_chg *pdata, int ichg);
> +	int (*bq257xx_set_vbatreg)(struct bq257xx_chg *pdata, int vbatreg);
> +	int (*bq257xx_set_iindpm)(struct bq257xx_chg *pdata, int iindpm);
> +};
> +
> +/**
> + * struct bq257xx_chg - driver data for charger
> + * @bq257xx_chip_info: hw specific functions
> + * @bq257xx_device: parent MFD device
> + * @charger: power supply device
> + * @online: charger input is present
> + * @fast_charge: charger is in fast charge mode
> + * @pre_charge: charger is in pre-charge mode
> + * @ov_fault: charger reports over voltage fault
> + * @batoc_fault: charger reports battery over current fault
> + * @oc_fault: charger reports over current fault
> + * @usb_type: USB type reported from parent power supply
> + * @supplied: Status of parent power supply
> + * @iindpm_max: maximum input current limit (uA)
> + * @vbat_max: maximum charge voltage (uV)
> + * @ichg_max: maximum charge current (uA)
> + * @vsys_min: minimum system voltage (uV)
> + */
> +struct bq257xx_chg {
> +	const struct bq257xx_chip_info *chip;
> +	struct bq257xx_device *bq;
> +	struct power_supply *charger;
> +	bool online;
> +	bool fast_charge;
> +	bool pre_charge;
> +	bool ov_fault;
> +	bool batoc_fault;
> +	bool oc_fault;
> +	int usb_type;
> +	int supplied;
> +	u32 iindpm_max;
> +	u32 vbat_max;
> +	u32 ichg_max;
> +	u32 vsys_min;
> +};
> +
> +/**
> + * bq25703_get_state() - Get the current state of the device
> + * @pdata: driver platform data
> + *
> + * Get the current state of the charger. Check if the charger is
> + * powered, what kind of charge state (if any) the device is in,
> + * and if there are any active faults.
> + *
> + * Return: Returns 0 on success, or error on failure to read device.
> + */
> +static int bq25703_get_state(struct bq257xx_chg *pdata)
> +{
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(pdata->bq->regmap, BQ25703_CHARGER_STATUS, &reg);
> +	if (ret)
> +		return ret;
> +
> +	pdata->online = reg & BQ25703_STS_AC_STAT;
> +	pdata->fast_charge = reg & BQ25703_STS_IN_FCHRG;
> +	pdata->pre_charge = reg & BQ25703_STS_IN_PCHRG;
> +	pdata->ov_fault = reg & BQ25703_STS_FAULT_ACOV;
> +	pdata->batoc_fault = reg & BQ25703_STS_FAULT_BATOC;
> +	pdata->oc_fault = reg & BQ25703_STS_FAULT_ACOC;
> +
> +	return 0;
> +}
> +
> +/**
> + * bq25703_get_min_vsys() - Get the minimum system voltage
> + * @pdata: driver platform data
> + *
> + * Return: Returns minimum system voltage in uV on success, or error on
> + *         failure to read device.
> + */
> +static int bq25703_get_min_vsys(struct bq257xx_chg *pdata)
> +{
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(pdata->bq->regmap, BQ25703_MIN_VSYS,
> +			  &reg);
> +	if (ret)
> +		return ret;
> +
> +	reg = FIELD_GET(BQ25703_MINVSYS_MASK, reg);
> +	return (reg * BQ25703_MINVSYS_STEP_UV) + BQ25703_MINVSYS_MIN_UV;
> +}
> +
> +/**
> + * bq25703_set_min_vsys() - Set the minimum system voltage
> + * @pdata: driver platform data
> + * @ichg: voltage value to set in uV.
> + *
> + * This function takes a requested minimum system voltage value, clamps
> + * it between the minimum supported value by the charger and a user
> + * defined minimum system value, and then writes the value to the
> + * appropriate register.
> + *
> + * Return: Returns 0 on success or error if an error occurs.
> + */
> +static int bq25703_set_min_vsys(struct bq257xx_chg *pdata, int vsys)
> +{
> +	unsigned int reg;
> +	int vsys_min = pdata->vsys_min;
> +
> +	vsys = clamp(vsys, BQ25703_MINVSYS_MIN_UV, vsys_min);
> +	reg = ((vsys - BQ25703_MINVSYS_MIN_UV) / BQ25703_MINVSYS_STEP_UV);
> +	reg = FIELD_PREP(BQ25703_MINVSYS_MASK, reg);
> +
> +	return regmap_write(pdata->bq->regmap, BQ25703_MIN_VSYS,
> +			    reg);
> +}
> +
> +/**
> + * bq25703_get_cur() - Get the reported current from the battery
> + * @pdata: driver platform data
> + *
> + * Return: Returns reported current from battery in uA as an absolute
> + *         value (charge and discharge are both positive integers).
> + *         Returns error if unable to read register value.
> + */
> +static int bq25703_get_cur(struct bq257xx_chg *pdata)
> +{
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(pdata->bq->regmap, BQ25703_ADCIBAT_CHG, &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (pdata->online)
> +		return FIELD_GET(BQ25703_ADCIBAT_CHG_MASK, reg) *
> +			BQ25703_ADCIBAT_CHG_STEP_UA;
> +	else
> +		return FIELD_GET(BQ25703_ADCIBAT_DISCHG_MASK, reg) *
> +			BQ25703_ADCIBAT_DIS_STEP_UA;
> +}
> +
> +/**
> + * bq25703_get_ichg_cur() - Get the maximum reported charge current
> + * @pdata: driver platform data
> + *
> + * Return: Returns maximum set charge current from charger in uA.
> + *         Returns error if unable to read register value.
> + */
> +static int bq25703_get_ichg_cur(struct bq257xx_chg *pdata)
> +{
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(pdata->bq->regmap, BQ25703_CHARGE_CURRENT, &reg);
> +	if (ret)
> +		return ret;
> +
> +	return FIELD_GET(BQ25703_ICHG_MASK, reg) * BQ25703_ICHG_STEP_UA;
> +}
> +
> +/**
> + * bq25703_set_ichg_cur() - Set the maximum charge current
> + * @pdata: driver platform data
> + * @ichg: current value to set in uA.
> + *
> + * This function takes a requested maximum charge current value, clamps
> + * it between the minimum supported value by the charger and a user
> + * defined maximum charging value, and then writes the value to the
> + * appropriate register.
> + *
> + * Return: Returns 0 on success or error if an error occurs.
> + */
> +static int bq25703_set_ichg_cur(struct bq257xx_chg *pdata, int ichg)
> +{
> +	unsigned int reg;
> +	int ichg_max = pdata->ichg_max;
> +
> +	ichg = clamp(ichg, BQ25703_ICHG_MIN_UA, ichg_max);
> +	reg = FIELD_PREP(BQ25703_ICHG_MASK, (ichg / BQ25703_ICHG_STEP_UA));
> +
> +	return regmap_write(pdata->bq->regmap, BQ25703_CHARGE_CURRENT,
> +			    reg);
> +}
> +
> +/**
> + * bq25703_get_chrg_volt() - Get the maximum set charge voltage
> + * @pdata: driver platform data
> + *
> + * Return: Returns maximum set charge voltage from charger in uV.
> + *         Returns error if unable to read register value.
> + */
> +static int bq25703_get_chrg_volt(struct bq257xx_chg *pdata)
> +{
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(pdata->bq->regmap, BQ25703_MAX_CHARGE_VOLT,
> +			  &reg);
> +	if (ret)
> +		return ret;
> +
> +	return FIELD_GET(BQ25703_MAX_CHARGE_VOLT_MASK, reg) *
> +		BQ25703_VBATREG_STEP_UV;
> +}
> +
> +/**
> + * bq25703_set_chrg_volt() - Set the maximum charge voltage
> + * @pdata: driver platform data
> + * @vbat: voltage value to set in uV.
> + *
> + * This function takes a requested maximum charge voltage value, clamps
> + * it between the minimum supported value by the charger and a user
> + * defined maximum charging value, and then writes the value to the
> + * appropriate register.
> + *
> + * Return: Returns 0 on success or error if an error occurs.
> + */
> +static int bq25703_set_chrg_volt(struct bq257xx_chg *pdata, int vbat)
> +{
> +	unsigned int reg;
> +	int vbat_max = pdata->vbat_max;
> +
> +	vbat = clamp(vbat, BQ25703_VBATREG_MIN_UV, vbat_max);
> +
> +	reg = FIELD_PREP(BQ25703_MAX_CHARGE_VOLT_MASK,
> +			 (vbat / BQ25703_VBATREG_STEP_UV));
> +
> +	return regmap_write(pdata->bq->regmap, BQ25703_MAX_CHARGE_VOLT,
> +			    reg);
> +}
> +
> +/**
> + * bq25703_get_iindpm() - Get the maximum set input current
> + * @pdata: driver platform data
> + *
> + * Read the actual input current limit from the device. This can differ
> + * from the value programmed due to some autonomous functions that may
> + * be enabled (but are not currently). This is why there is a different
> + * register used.
> + *
> + * Return: Returns maximum set input current from charger in uA.
> + *         Returns error if unable to read register value.
> + */
> +static int bq25703_get_iindpm(struct bq257xx_chg *pdata)
> +{
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(pdata->bq->regmap, BQ25703_IIN_DPM, &reg);
> +	if (ret)
> +		return ret;
> +
> +	reg = FIELD_GET(BQ25703_IINDPM_MASK, reg);
> +	return (reg * BQ25703_IINDPM_STEP_UA) + BQ25703_IINDPM_OFFSET_UA;
> +}
> +
> +/**
> + * bq25703_set_iindpm() - Set the maximum input current
> + * @pdata: driver platform data
> + * @iindpm: current value in uA.
> + *
> + * This function takes a requested maximum input current value, clamps
> + * it between the minimum supported value by the charger and a user
> + * defined maximum input value, and then writes the value to the
> + * appropriate register.
> + *
> + * Return: Returns 0 on success or error if an error occurs.
> + */
> +static int bq25703_set_iindpm(struct bq257xx_chg *pdata, int iindpm)
> +{
> +	unsigned int reg;
> +	int iindpm_max = pdata->iindpm_max;
> +
> +	iindpm = clamp(iindpm, BQ25703_IINDPM_MIN_UA, iindpm_max);
> +
> +	reg = ((iindpm - BQ25703_IINDPM_OFFSET_UA) / BQ25703_IINDPM_STEP_UA);
> +
> +	return regmap_write(pdata->bq->regmap, BQ25703_IIN_HOST,
> +			    FIELD_PREP(BQ25703_IINDPM_MASK, reg));
> +}
> +
> +/**
> + * bq25703_get_vbat() - Get the reported voltage from the battery
> + * @pdata: driver platform data
> + *
> + * Return: Returns reported voltage from battery in uV.
> + *         Returns error if unable to read register value.
> + */
> +static int bq25703_get_vbat(struct bq257xx_chg *pdata)
> +{
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(pdata->bq->regmap, BQ25703_ADCVSYSVBAT, &reg);
> +	if (ret)
> +		return ret;
> +
> +	reg = FIELD_GET(BQ25703_ADCVBAT_MASK, reg);
> +
> +	return (reg * BQ25703_ADCVSYSVBAT_STEP) + BQ25703_ADCVSYSVBAT_OFFSET_UV;
> +
> +}
> +
> +/**
> + * bq25703_hw_init() - Set all the required registers to init the charger
> + * @pdata: driver platform data
> + *
> + * Initialize the BQ25703 by first disabling the watchdog timer (which
> + * shuts off the charger in the absence of periodic writes). Then, set
> + * the charge current, charge voltage, minimum system voltage, and
> + * input current limit. Disable low power mode to allow ADCs and
> + * interrupts. Enable the ADC, start the ADC, set the ADC scale to
> + * full, and enable each individual ADC channel.
> + *
> + * Return: Returns 0 on success or error code on error.
> + */
> +static int bq25703_hw_init(struct bq257xx_chg *pdata)
> +{
> +	int ret = 0;

You need the regmap a bunch of times, so create a local variable for
it :)

> +
> +	regmap_update_bits(pdata->bq->regmap, BQ25703_CHARGE_OPTION_0,
> +			   BQ25703_WDTMR_ADJ_MASK,
> +			   FIELD_PREP(BQ25703_WDTMR_ADJ_MASK,
> +				      BQ25703_WDTMR_DISABLE));
> +
> +	ret = pdata->chip->bq257xx_set_ichg(pdata, pdata->ichg_max);
> +	if (ret)
> +		return ret;
> +
> +	ret = pdata->chip->bq257xx_set_vbatreg(pdata, pdata->vbat_max);
> +	if (ret)
> +		return ret;
> +
> +	ret = bq25703_set_min_vsys(pdata, pdata->vsys_min);
> +	if (ret)
> +		return ret;
> +
> +	ret = pdata->chip->bq257xx_set_iindpm(pdata, pdata->iindpm_max);
> +	if (ret)
> +		return ret;
> +
> +	regmap_update_bits(pdata->bq->regmap, BQ25703_CHARGE_OPTION_0,
> +			   BQ25703_EN_LWPWR, !BQ25703_EN_LWPWR);
> +
> +	regmap_update_bits(pdata->bq->regmap, BQ25703_ADC_OPTION,
> +			   BQ25703_ADC_CONV_EN, BQ25703_ADC_CONV_EN);
> +
> +	regmap_update_bits(pdata->bq->regmap, BQ25703_ADC_OPTION,
> +			   BQ25703_ADC_START, BQ25703_ADC_START);
> +
> +	regmap_update_bits(pdata->bq->regmap, BQ25703_ADC_OPTION,
> +			   BQ25703_ADC_FULL_SCALE, BQ25703_ADC_FULL_SCALE);
> +
> +	regmap_update_bits(pdata->bq->regmap, BQ25703_ADC_OPTION,
> +			   BQ25703_ADC_CH_MASK,
> +			   (BQ25703_ADC_CMPIN_EN | BQ25703_ADC_VBUS_EN |
> +			   BQ25703_ADC_PSYS_EN | BQ25703_ADC_IIN_EN |
> +			   BQ25703_ADC_IDCHG_EN | BQ25703_ADC_ICHG_EN |
> +			   BQ25703_ADC_VSYS_EN | BQ25703_ADC_VBAT_EN));
> +
> +	return ret;
> +}
> +
> +/**
> + * bq25703_hw_shutdown() - Set registers for shutdown
> + * @pdata: driver platform data
> + *
> + * Enable low power mode for the device while in shutdown.
> + */
> +static void bq25703_hw_shutdown(struct bq257xx_chg *pdata)
> +{
> +	regmap_update_bits(pdata->bq->regmap, BQ25703_CHARGE_OPTION_0,
> +			   BQ25703_EN_LWPWR, BQ25703_EN_LWPWR);
> +}
> +
> +static int bq257xx_set_charger_property(struct power_supply *psy,
> +		enum power_supply_property prop,
> +		const union power_supply_propval *val)
> +{
> +	struct bq257xx_chg *pdata = power_supply_get_drvdata(psy);
> +	int ret = -EINVAL;
> +
> +	switch (prop) {
> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +		ret = pdata->chip->bq257xx_set_iindpm(pdata, val->intval);
> +		if (ret)
> +			return ret;
> +		break;

return pdata->chip->bq257xx_set_iindpm(...);

> +
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
> +		ret = pdata->chip->bq257xx_set_vbatreg(pdata, val->intval);
> +		if (ret)
> +			return ret;
> +		break;

same.

> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
> +		ret = pdata->chip->bq257xx_set_ichg(pdata, val->intval);
> +		if (ret)
> +			return ret;
> +		break;

same.

> +	default:
> +		break;

return -EINVAL

> +	}
> +
> +	return ret;

get rid of this and ret.

> +}
> +
> +static int bq257xx_get_charger_property(struct power_supply *psy,
> +				enum power_supply_property psp,
> +				union power_supply_propval *val)
> +{
> +	struct bq257xx_chg *pdata = power_supply_get_drvdata(psy);
> +	int ret = 0;
> +
> +	ret = pdata->chip->bq257xx_get_state(pdata);
> +	if (ret)
> +		return ret;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_STATUS:
> +		if (!pdata->online)
> +			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> +		else if (!bq25703_get_cur(pdata))
> +			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +		else if (pdata->fast_charge || pdata->pre_charge)
> +			val->intval = POWER_SUPPLY_STATUS_CHARGING;
> +		else
> +			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_HEALTH:
> +		if (pdata->ov_fault || pdata->batoc_fault)
> +			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> +		else if (pdata->oc_fault)
> +			val->intval = POWER_SUPPLY_HEALTH_OVERCURRENT;
> +		else
> +			val->intval = POWER_SUPPLY_HEALTH_GOOD;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_MANUFACTURER:
> +		val->strval = BQ257XX_MANUFACTURER;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		val->intval = pdata->online;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +		ret = bq25703_get_iindpm(pdata);
> +		if (ret < 0)
> +			return ret;
> +		val->intval = ret;

Maybe feed val as second argument to bq25703_get_iindpm(), so
that you can just do

return bq25703_get_iindpm(pdata, val);

> +		break;
> +
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
> +		ret = bq25703_get_chrg_volt(pdata);
> +		if (ret < 0)
> +			return ret;
> +		val->intval = ret;
> +		break;

same.

> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		ret = bq25703_get_cur(pdata);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (pdata->online)
> +			val->intval = ret;
> +		else
> +			val->intval = -ret;
> +		break;

same, if you move the online check into bq25703_get_cur.

> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		ret = bq25703_get_vbat(pdata);
> +		if (ret < 0)
> +			return ret;
> +		val->intval = ret;
> +		break;

same.

> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
> +		ret = bq25703_get_ichg_cur(pdata);
> +		if (ret < 0)
> +			return ret;
> +		val->intval = ret;
> +		break;

same.

> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> +		ret = bq25703_get_min_vsys(pdata);
> +		if (ret < 0)
> +			return ret;
> +		val->intval = ret;
> +		break;

same.

> +	case POWER_SUPPLY_PROP_USB_TYPE:
> +		val->intval = pdata->usb_type;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static enum power_supply_property bq257xx_power_supply_props[] = {
> +	POWER_SUPPLY_PROP_MANUFACTURER,
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_ONLINE,
> +	POWER_SUPPLY_PROP_HEALTH,
> +	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
> +	POWER_SUPPLY_PROP_VOLTAGE_MIN,
> +	POWER_SUPPLY_PROP_USB_TYPE,
> +};
> +
> +static int bq257xx_property_is_writeable(struct power_supply *psy,
> +					 enum power_supply_property prop)
> +{
> +	switch (prop) {
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static enum power_supply_usb_type bq25703_usb_types[] = {
> +	POWER_SUPPLY_USB_TYPE_C,
> +	POWER_SUPPLY_USB_TYPE_PD,
> +	POWER_SUPPLY_USB_TYPE_PD_DRP,
> +	POWER_SUPPLY_USB_TYPE_PD_PPS,
> +	POWER_SUPPLY_USB_TYPE_UNKNOWN,
> +};
> +
> +/**
> + * bq257xx_external_power_changed() - Handler for external power change
> + * @psy: Power supply data
> + *
> + * When the external power into the charger is changed, check the USB
> + * type so that it can be reported. Additionally, update the max input
> + * current and max charging current to the value reported if it is a
> + * USB PD charger, otherwise use the default value. Note that each time
> + * a charger is removed the max charge current register is erased, so
> + * it must be set again each time the input changes or the device will
> + * not charge.
> + */
> +static void bq257xx_external_power_changed(struct power_supply *psy)
> +{
> +	struct bq257xx_chg *pdata = power_supply_get_drvdata(psy);
> +	union power_supply_propval val;
> +	int ret;
> +	int imax = pdata->iindpm_max;
> +
> +	pdata->chip->bq257xx_get_state(pdata);
> +
> +	pdata->supplied = power_supply_am_i_supplied(pdata->charger);
> +	if (pdata->supplied < 0)
> +		return;
> +
> +	if (pdata->supplied == 0)
> +		goto out;
> +
> +	ret = power_supply_get_property_from_supplier(psy,
> +						      POWER_SUPPLY_PROP_USB_TYPE,
> +						      &val);
> +	if (ret)
> +		return;
> +
> +	pdata->usb_type = val.intval;
> +
> +	if ((pdata->usb_type == POWER_SUPPLY_USB_TYPE_PD) ||
> +	    (pdata->usb_type == POWER_SUPPLY_USB_TYPE_PD_DRP) ||
> +	    (pdata->usb_type == POWER_SUPPLY_USB_TYPE_PD)) {
> +		ret = power_supply_get_property_from_supplier(psy,
> +							      POWER_SUPPLY_PROP_CURRENT_MAX,
> +							      &val);
> +		if (ret)
> +			return;
> +
> +		if (val.intval)
> +			imax = val.intval;
> +	}
> +
> +	if (pdata->supplied) {
> +		pdata->chip->bq257xx_set_ichg(pdata, imax);
> +		pdata->chip->bq257xx_set_iindpm(pdata, imax);
> +		pdata->chip->bq257xx_set_vbatreg(pdata, pdata->vbat_max);
> +	}
> +
> +out:
> +	power_supply_changed(psy);
> +}
> +
> +static irqreturn_t bq257xx_irq_handler_thread(int irq, void *private)
> +{
> +	struct bq257xx_chg *pdata = private;
> +
> +	bq257xx_external_power_changed(pdata->charger);
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct power_supply_desc bq257xx_power_supply_desc = {
> +	.name = "bq257xx-charger",
> +	.type = POWER_SUPPLY_TYPE_USB,
> +	.usb_types = bq25703_usb_types,
> +	.num_usb_types = ARRAY_SIZE(bq25703_usb_types),

This recently changed into a bitmask.

> +	.properties = bq257xx_power_supply_props,
> +	.num_properties = ARRAY_SIZE(bq257xx_power_supply_props),
> +	.get_property = bq257xx_get_charger_property,
> +	.set_property = bq257xx_set_charger_property,
> +	.property_is_writeable = bq257xx_property_is_writeable,
> +	.external_power_changed = bq257xx_external_power_changed,
> +};
> +
> +static const struct bq257xx_chip_info bq25703_chip_info = {
> +		.bq257xx_hw_init = &bq25703_hw_init,
> +		.bq257xx_hw_shutdown = &bq25703_hw_shutdown,
> +		.bq257xx_get_state = &bq25703_get_state,
> +		.bq257xx_set_ichg = &bq25703_set_ichg_cur,
> +		.bq257xx_set_vbatreg = &bq25703_set_chrg_volt,
> +		.bq257xx_set_iindpm = &bq25703_set_iindpm,
> +};
> +
> +static int bq257xx_power_supply_init(struct bq257xx_chg *pdata,
> +		struct power_supply_config *psy_cfg, struct device *dev)
> +{
> +	pdata->charger = devm_power_supply_register(dev,
> +						    &bq257xx_power_supply_desc,
> +						    psy_cfg);
> +	if (IS_ERR(pdata->charger)) {
> +		dev_err_probe(dev, PTR_ERR(pdata->charger),
> +			      "power supply register charger failed: %ld\n",
> +			      PTR_ERR(pdata->charger));
> +		return PTR_ERR(pdata->charger);

dev_err_probe() returns the error code, so you can just do
return dev_err_probe(...). Also it prints the error code,
so no need for the ": %ld" part either.

After that simplification I don't see the point in having
an extra function for this.

> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * bq257xx_parse_dt() - Parse the device tree for required properties
> + * @pdata: driver platform data
> + * @psy_cfg: power supply config data
> + * @dev: device struct
> + *
> + * Read the device tree to identify the minimum system voltage, the
> + * maximum charge current, the maximum charge voltage, and the maximum
> + * input current.
> + *
> + * Return: Returns 0 on success or error code on error.
> + */
> +static int bq257xx_parse_dt(struct bq257xx_chg *pdata,
> +		struct power_supply_config *psy_cfg, struct device *dev)
> +{
> +	int ret = 0;
> +
> +	psy_cfg->drv_data = pdata;
> +	psy_cfg->of_node = dev->of_node;

replace the last one with

psy_cfg.fwnode = dev_fwnode(dev);

> +	ret = device_property_read_u32(dev,
> +				       "ti,minimum-sys-voltage",
> +				       &pdata->vsys_min);
> +	if (ret) {
> +		dev_err(dev, "Cannot read ti,minimum-sys-voltage from dt\n");
> +		return ret;

return dev_err_probe(dev, ret, ...);

> +	}
> +
> +	ret = device_property_read_u32(dev,
> +				       "ti,charge-current",
> +				       &pdata->ichg_max);
> +	if (ret) {
> +		dev_err(dev, "Cannot read ti,charge-current from dt\n");
> +		return ret;
> +	}
> +
> +	ret = device_property_read_u32(dev,
> +				       "ti,max-charge-voltage",
> +				       &pdata->vbat_max);
> +	if (ret) {
> +		dev_err(dev, "Cannot read ti,max-charge-voltage from dt\n");
> +		return ret;
> +	}
> +
> +	ret = device_property_read_u32(dev,
> +				       "ti,current-limit",
> +				       &pdata->iindpm_max);
> +	if (ret)
> +		pdata->iindpm_max = BQ25703_IINDPM_DEFAULT_UA;

same for the above, but will change anyways because of binding
change.

> +
> +	return 0;
> +}
> +
> +static int bq257xx_charger_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct bq257xx_device *bq = dev_get_drvdata(pdev->dev.parent);
> +	struct bq257xx_chg *pdata;
> +	struct power_supply_config psy_cfg = { };
> +	int ret;
> +
> +	pdev->dev.of_node = pdev->dev.parent->of_node;
> +	pdev->dev.of_node_reused = true;

replace above two lines with

device_set_of_node_from_dev(dev, pdev->dev.parent);

> +
> +	pdata = devm_kzalloc(&pdev->dev, sizeof(struct bq257xx_chg), GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;

use sizeof(*pdata)

> +	pdata->bq = bq;
> +
> +	switch (pdata->bq->variant) {
> +	case BQ25703A:
> +		pdata->chip = &bq25703_chip_info;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = bq257xx_parse_dt(pdata, &psy_cfg, dev);
> +	if (ret) {
> +		dev_err(dev, "Failed to read device tree properties%d\n", ret);

no need for this error message, since bq257xx_parse_dt already prints a
message for all error cases.

> +		return ret;
> +	}
> +
> +	ret = pdata->chip->bq257xx_hw_init(pdata);
> +	if (ret) {
> +		dev_err(dev, "Cannot initialize the chip.\n");
> +		return ret;
> +	}
> +
> +	ret = bq257xx_power_supply_init(pdata, &psy_cfg, dev);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, pdata);
> +
> +	if (bq->client->irq) {
> +		ret = devm_request_threaded_irq(dev, bq->client->irq, NULL,
> +						bq257xx_irq_handler_thread,
> +						IRQF_TRIGGER_RISING |
> +						IRQF_TRIGGER_FALLING |
> +						IRQF_ONESHOT,
> +						dev_name(&bq->client->dev), pdata);
> +		if (ret < 0) {
> +			dev_err(dev, "get irq fail: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static void bq257xx_charger_shutdown(struct platform_device *pdev)
> +{
> +	struct bq257xx_chg *pdata = platform_get_drvdata(pdev);
> +
> +	pdata->chip->bq257xx_hw_shutdown(pdata);
> +}
> +
> +static struct platform_driver bq257xx_chg_driver = {
> +	.driver = {
> +		.name = "bq257xx-charger",
> +	},
> +	.probe = bq257xx_charger_probe,
> +	.shutdown = bq257xx_charger_shutdown,
> +};
> +module_platform_driver(bq257xx_chg_driver);
> +
> +MODULE_DESCRIPTION("bq257xx charger driver");
> +MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
> +MODULE_LICENSE("GPL");

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-09-16 10:49 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-29 21:30 [RFC 0/5] Add Texas Instruments BQ25703 Charger Chris Morgan
2024-08-29 21:30 ` [RFC 1/5] dt-bindings: mfd: ti,bq25703a: Add TI BQ25703A Charger Chris Morgan
2024-08-30 16:30   ` Rob Herring
2024-08-30 20:52     ` Chris Morgan
2024-09-16  9:42       ` Sebastian Reichel
2024-09-16  9:33     ` Sebastian Reichel
2024-08-29 21:30 ` [RFC 2/5] mfd: bq257xx: Add support for BQ25703 core driver Chris Morgan
2024-09-03 15:25   ` Lee Jones
2024-08-29 21:31 ` [RFC 3/5] power: supply: bq257xx: Add support for BQ257XX charger manager Chris Morgan
2024-09-16 10:49   ` Sebastian Reichel
2024-08-29 21:31 ` [RFC 4/5] regulator: bq257xx: Add bq257xx boost regulator driver Chris Morgan
2024-08-29 21:31 ` [RFC 5/5] arm64: dts: rockchip: Add USB and charger to Gameforce Ace Chris Morgan
2024-08-31 10:39   ` Krzysztof Kozlowski

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 NNTP newsgroup(s).