* [PATCH 00/10] Add support for AXP192 PMIC
@ 2022-06-03 13:57 Aidan MacDonald
2022-06-03 13:57 ` [PATCH 01/10] regmap-irq: Add get_irq_reg to support unusual register layouts Aidan MacDonald
` (9 more replies)
0 siblings, 10 replies; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-03 13:57 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood
Cc: lars, rafael, linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
Hi all,
This patch series adds support for the X-Powers AXP192 PMIC to the
AXP20x driver framework.
The first patch is a small change to regmap-irq to support the AXP192's
unusual IRQ register layout. It isn't possible to include all of the
IRQ registers in one regmap-irq chip without this.
The rest of the changes are pretty straightforward, I think the only
notable parts are the axp20x_adc driver where there seems to be some
opportunities for code reuse (the axp192 is nearly a duplicate of the
axp20x) and the addition of a new pinctrl driver for the axp192, since
the axp20x pinctrl driver was not very easy to adapt.
Aidan MacDonald (10):
regmap-irq: Add get_irq_reg to support unusual register layouts
dt-bindings: mfd: add bindings for AXP192 MFD device
dt-bindings: iio: adc: axp209: Add AXP192 compatible
dt-bindings: power: supply: axp20x: Add AXP192 compatible
dt-bindings: gpio: Add AXP192 GPIO bindings
mfd: axp20x: Add support for AXP192
regulator: axp20x: Add support for AXP192
iio: adc: axp20x_adc: Add support for AXP192
power: supply: axp20x_usb_power: Add support for AXP192
pinctrl: Add AXP192 pin control driver
.../bindings/gpio/x-powers,axp192-gpio.yaml | 59 ++
.../bindings/iio/adc/x-powers,axp209-adc.yaml | 18 +
.../bindings/mfd/x-powers,axp152.yaml | 1 +
.../x-powers,axp20x-usb-power-supply.yaml | 1 +
drivers/base/regmap/regmap-irq.c | 19 +-
drivers/iio/adc/axp20x_adc.c | 289 ++++++++-
drivers/mfd/axp20x-i2c.c | 2 +
drivers/mfd/axp20x.c | 150 +++++
drivers/pinctrl/Kconfig | 14 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-axp192.c | 589 ++++++++++++++++++
drivers/power/supply/axp20x_usb_power.c | 75 ++-
drivers/regulator/axp20x-regulator.c | 101 ++-
include/linux/mfd/axp20x.h | 84 +++
include/linux/regmap.h | 5 +
15 files changed, 1375 insertions(+), 33 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
create mode 100644 drivers/pinctrl/pinctrl-axp192.c
--
2.35.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 01/10] regmap-irq: Add get_irq_reg to support unusual register layouts
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
@ 2022-06-03 13:57 ` Aidan MacDonald
2022-06-06 17:43 ` Guru Das Srinagesh
2022-06-03 13:57 ` [PATCH 02/10] dt-bindings: mfd: add bindings for AXP192 MFD device Aidan MacDonald
` (8 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-03 13:57 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood
Cc: lars, rafael, linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
Add a new callback, get_irq_reg, for regmap IRQ chips, to support devices
with unusual register layouts. This is required in the rare cases where
the offset of an IRQ register is not constant with respect to the base
register. This is probably best illustrated with an example:
mask status
IRQ0 0x40 0x44
IRQ1 0x41 0x45
IRQ2 0x42 0x46
IRQ3 0x43 0x47
IRQ4 0x4a 0x4d
If we set mask_base = 0x40 and status_base = 0x44, the offsets of each
register relative to the base are:
mask status
IRQ0 0 0
IRQ1 1 1
IRQ2 2 2
IRQ3 3 3
IRQ4 10 9
The existing mapping mechanisms can't include IRQ4 in the same irqchip
as IRQ0-3 because the offset of IRQ4's register depends on which type
of register we're asking for, ie. which base register is used.
The get_irq_reg callback allows drivers to specify an arbitrary mapping
of (base register, register index) pairs to register addresses, instead
of the default linear mapping "base_register + register_index". This
allows unusual layouts, like the one above, to be handled using a single
regmap IRQ chip.
The drawback is that when get_irq_reg is used, it's impossible to use
bulk reads for status registers even if some of them are contiguous,
because the mapping is opaque to regmap-irq. This should be acceptable
for the case of a few infrequently-polled status registers.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
drivers/base/regmap/regmap-irq.c | 19 +++++++++----------
include/linux/regmap.h | 5 +++++
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 400c7412a7dc..e50437b18284 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -55,7 +55,9 @@ static int sub_irq_reg(struct regmap_irq_chip_data *data,
unsigned int offset;
int reg = 0;
- if (!chip->sub_reg_offsets || !chip->not_fixed_stride) {
+ if (chip->get_irq_reg) {
+ reg = chip->get_irq_reg(base_reg, i);
+ } else if (!chip->sub_reg_offsets || !chip->not_fixed_stride) {
/* Assume linear mapping */
reg = base_reg + (i * map->reg_stride * data->irq_reg_stride);
} else {
@@ -97,7 +99,6 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
struct regmap *map = d->map;
int i, j, ret;
u32 reg;
- u32 unmask_offset;
u32 val;
if (d->chip->runtime_pm) {
@@ -141,11 +142,11 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
dev_err(d->map->dev,
"Failed to sync unmasks in %x\n",
reg);
- unmask_offset = d->chip->unmask_base -
- d->chip->mask_base;
+
/* clear mask with unmask_base register */
+ reg = sub_irq_reg(d, d->chip->unmask_base, i);
ret = regmap_irq_update_bits(d,
- reg + unmask_offset,
+ reg,
d->mask_buf_def[i],
d->mask_buf[i]);
} else {
@@ -480,7 +481,7 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
}
} else if (!map->use_single_read && map->reg_stride == 1 &&
- data->irq_reg_stride == 1) {
+ data->irq_reg_stride == 1 && !chip->get_irq_reg) {
u8 *buf8 = data->status_reg_buf;
u16 *buf16 = data->status_reg_buf;
@@ -632,7 +633,6 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
int ret = -ENOMEM;
int num_type_reg;
u32 reg;
- u32 unmask_offset;
if (chip->num_regs <= 0)
return -EINVAL;
@@ -773,10 +773,9 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
ret = regmap_irq_update_bits(d, reg,
d->mask_buf[i], ~d->mask_buf[i]);
else if (d->chip->unmask_base) {
- unmask_offset = d->chip->unmask_base -
- d->chip->mask_base;
+ reg = sub_irq_reg(d, d->chip->unmask_base, i);
ret = regmap_irq_update_bits(d,
- reg + unmask_offset,
+ reg,
d->mask_buf[i],
d->mask_buf[i]);
} else
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 8952fa3d0d59..4828021ab9e8 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1495,6 +1495,10 @@ struct regmap_irq_sub_irq_map {
* after handling the interrupts in regmap_irq_handler().
* @set_type_virt: Driver specific callback to extend regmap_irq_set_type()
* and configure virt regs.
+ * @get_irq_reg: Callback to map a register index in range [0, num_regs[
+ * to a register, relative to a specific base register. This
+ * is mainly useful for devices where the register offsets
+ * change depending on the base register.
* @irq_drv_data: Driver specific IRQ data which is passed as parameter when
* driver specific pre/post interrupt handler is called.
*
@@ -1545,6 +1549,7 @@ struct regmap_irq_chip {
int (*handle_post_irq)(void *irq_drv_data);
int (*set_type_virt)(unsigned int **buf, unsigned int type,
unsigned long hwirq, int reg);
+ int (*get_irq_reg)(unsigned int base_reg, int i);
void *irq_drv_data;
};
--
2.35.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 02/10] dt-bindings: mfd: add bindings for AXP192 MFD device
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
2022-06-03 13:57 ` [PATCH 01/10] regmap-irq: Add get_irq_reg to support unusual register layouts Aidan MacDonald
@ 2022-06-03 13:57 ` Aidan MacDonald
2022-06-05 22:49 ` Rob Herring
2022-06-27 11:47 ` Lee Jones
2022-06-03 13:57 ` [PATCH 03/10] dt-bindings: iio: adc: axp209: Add AXP192 compatible Aidan MacDonald
` (7 subsequent siblings)
9 siblings, 2 replies; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-03 13:57 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood
Cc: lars, rafael, linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
The AXP192 is another X-Powers PMIC similar to the existing ones.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
index 3a53bae611bc..33c9b1b3cc04 100644
--- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
+++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
@@ -84,6 +84,7 @@ properties:
oneOf:
- enum:
- x-powers,axp152
+ - x-powers,axp192
- x-powers,axp202
- x-powers,axp209
- x-powers,axp221
--
2.35.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 03/10] dt-bindings: iio: adc: axp209: Add AXP192 compatible
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
2022-06-03 13:57 ` [PATCH 01/10] regmap-irq: Add get_irq_reg to support unusual register layouts Aidan MacDonald
2022-06-03 13:57 ` [PATCH 02/10] dt-bindings: mfd: add bindings for AXP192 MFD device Aidan MacDonald
@ 2022-06-03 13:57 ` Aidan MacDonald
2022-06-03 16:34 ` Jonathan Cameron
2022-06-05 22:50 ` Rob Herring
2022-06-03 13:57 ` [PATCH 04/10] dt-bindings: power: supply: axp20x: " Aidan MacDonald
` (6 subsequent siblings)
9 siblings, 2 replies; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-03 13:57 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood
Cc: lars, rafael, linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
The AXP192 is identical to the AXP20x, except for two additional
GPIO ADC channels.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
.../bindings/iio/adc/x-powers,axp209-adc.yaml | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml b/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
index d6d3d8590171..1a68e650ac7d 100644
--- a/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
@@ -14,6 +14,23 @@ description: |
Device is a child of an axp209 multifunction device
ADC channels and their indexes per variant:
+ AXP192
+ ------
+ 0 | acin_v
+ 1 | acin_i
+ 2 | vbus_v
+ 3 | vbus_i
+ 4 | pmic_temp
+ 5 | gpio0_v
+ 6 | gpio1_v
+ 7 | gpio2_v
+ 8 | gpio3_v
+ 9 | ipsout_v
+ 10 | batt_v
+ 11 | batt_chrg_i
+ 12 | batt_dischrg_i
+ 13 | ts_v
+
AXP209
------
0 | acin_v
@@ -50,6 +67,7 @@ description: |
properties:
compatible:
oneOf:
+ - const: x-powers,axp192-adc
- const: x-powers,axp209-adc
- const: x-powers,axp221-adc
- const: x-powers,axp813-adc
--
2.35.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 04/10] dt-bindings: power: supply: axp20x: Add AXP192 compatible
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
` (2 preceding siblings ...)
2022-06-03 13:57 ` [PATCH 03/10] dt-bindings: iio: adc: axp209: Add AXP192 compatible Aidan MacDonald
@ 2022-06-03 13:57 ` Aidan MacDonald
2022-06-05 22:50 ` Rob Herring
2022-06-03 13:57 ` [PATCH 05/10] dt-bindings: gpio: Add AXP192 GPIO bindings Aidan MacDonald
` (5 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-03 13:57 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood
Cc: lars, rafael, linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
The AXP192's USB power supply is similar to the AXP202 but it has
different USB current limits.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
.../bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml b/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml
index 0c371b55c9e1..e800b3b97f0d 100644
--- a/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml
+++ b/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml
@@ -22,6 +22,7 @@ properties:
compatible:
oneOf:
- enum:
+ - x-powers,axp192-usb-power-supply
- x-powers,axp202-usb-power-supply
- x-powers,axp221-usb-power-supply
- x-powers,axp223-usb-power-supply
--
2.35.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 05/10] dt-bindings: gpio: Add AXP192 GPIO bindings
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
` (3 preceding siblings ...)
2022-06-03 13:57 ` [PATCH 04/10] dt-bindings: power: supply: axp20x: " Aidan MacDonald
@ 2022-06-03 13:57 ` Aidan MacDonald
2022-06-05 22:55 ` Rob Herring
2022-06-03 13:57 ` [PATCH 06/10] mfd: axp20x: Add support for AXP192 Aidan MacDonald
` (4 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-03 13:57 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood
Cc: lars, rafael, linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
The AXP192 PMIC is different enough from the PMICs supported by
the AXP20x GPIO driver to warrant a separate driver. The AXP192
driver also supports interrupts and pinconf settings.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
.../bindings/gpio/x-powers,axp192-gpio.yaml | 59 +++++++++++++++++++
1 file changed, 59 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
diff --git a/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
new file mode 100644
index 000000000000..7a985640ade8
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/gpio/x-powers,axp192-gpio.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: X-Powers AXP192 GPIO Device Tree Bindings
+
+maintainers:
+ - Chen-Yu Tsai <wens@csie.org>
+
+properties:
+ "#gpio-cells":
+ const: 2
+ description: >
+ The first cell is the pin number and the second is the GPIO flags.
+
+ compatible:
+ oneOf:
+ - enum:
+ - x-powers,axp192-gpio
+
+ gpio-controller: true
+
+patternProperties:
+ "^.*-pins?$":
+ $ref: /schemas/pinctrl/pinmux-node.yaml#
+
+ properties:
+ pins:
+ items:
+ enum:
+ - GPIO0
+ - GPIO1
+ - GPIO2
+ - GPIO3
+ - GPIO4
+ - N_RSTO
+
+ function:
+ enum:
+ - output
+ - input
+ - ldo
+ - pwm
+ - adc
+ - low_output
+ - floating
+ - ext_chg_ctl
+ - ldo_status
+
+required:
+ - compatible
+ - "#gpio-cells"
+ - gpio-controller
+
+additionalProperties: false
+
+...
--
2.35.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 06/10] mfd: axp20x: Add support for AXP192
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
` (4 preceding siblings ...)
2022-06-03 13:57 ` [PATCH 05/10] dt-bindings: gpio: Add AXP192 GPIO bindings Aidan MacDonald
@ 2022-06-03 13:57 ` Aidan MacDonald
2022-06-27 11:54 ` Lee Jones
2022-06-03 13:57 ` [PATCH 07/10] regulator: " Aidan MacDonald
` (3 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-03 13:57 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood
Cc: lars, rafael, linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
The AXP192 PMIC is similar to the AXP202/AXP209, but with different
regulators, additional GPIOs, and a different IRQ register layout.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
drivers/mfd/axp20x-i2c.c | 2 +
drivers/mfd/axp20x.c | 150 +++++++++++++++++++++++++++++++++++++
include/linux/mfd/axp20x.h | 84 +++++++++++++++++++++
3 files changed, 236 insertions(+)
diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
index 00ab48018d8d..9ada58fad77f 100644
--- a/drivers/mfd/axp20x-i2c.c
+++ b/drivers/mfd/axp20x-i2c.c
@@ -62,6 +62,7 @@ static int axp20x_i2c_remove(struct i2c_client *i2c)
#ifdef CONFIG_OF
static const struct of_device_id axp20x_i2c_of_match[] = {
{ .compatible = "x-powers,axp152", .data = (void *)AXP152_ID },
+ { .compatible = "x-powers,axp192", .data = (void *)AXP192_ID },
{ .compatible = "x-powers,axp202", .data = (void *)AXP202_ID },
{ .compatible = "x-powers,axp209", .data = (void *)AXP209_ID },
{ .compatible = "x-powers,axp221", .data = (void *)AXP221_ID },
@@ -75,6 +76,7 @@ MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
static const struct i2c_device_id axp20x_i2c_id[] = {
{ "axp152", 0 },
+ { "axp192", 0 },
{ "axp202", 0 },
{ "axp209", 0 },
{ "axp221", 0 },
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 8161a5dc68e8..7f64e5c83fe2 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -34,6 +34,7 @@
static const char * const axp20x_model_names[] = {
"AXP152",
+ "AXP192",
"AXP202",
"AXP209",
"AXP221",
@@ -92,6 +93,35 @@ static const struct regmap_access_table axp20x_volatile_table = {
.n_yes_ranges = ARRAY_SIZE(axp20x_volatile_ranges),
};
+static const struct regmap_range axp192_writeable_ranges[] = {
+ regmap_reg_range(AXP192_DATACACHE(0), AXP192_DATACACHE(5)),
+ regmap_reg_range(AXP192_PWR_OUT_CTRL, AXP192_IRQ5_STATE),
+ regmap_reg_range(AXP20X_DCDC_MODE, AXP192_N_RSTO_CTRL),
+ regmap_reg_range(AXP20X_CC_CTRL, AXP20X_CC_CTRL),
+};
+
+static const struct regmap_range axp192_volatile_ranges[] = {
+ regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP192_USB_OTG_STATUS),
+ regmap_reg_range(AXP192_IRQ1_STATE, AXP192_IRQ4_STATE),
+ regmap_reg_range(AXP192_IRQ5_STATE, AXP192_IRQ5_STATE),
+ regmap_reg_range(AXP20X_ACIN_V_ADC_H, AXP20X_IPSOUT_V_HIGH_L),
+ regmap_reg_range(AXP20X_TIMER_CTRL, AXP20X_TIMER_CTRL),
+ regmap_reg_range(AXP192_GPIO2_0_STATE, AXP192_GPIO2_0_STATE),
+ regmap_reg_range(AXP192_GPIO4_3_STATE, AXP192_GPIO4_3_STATE),
+ regmap_reg_range(AXP192_N_RSTO_CTRL, AXP192_N_RSTO_CTRL),
+ regmap_reg_range(AXP20X_CHRG_CC_31_24, AXP20X_CC_CTRL),
+};
+
+static const struct regmap_access_table axp192_writeable_table = {
+ .yes_ranges = axp192_writeable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(axp192_writeable_ranges),
+};
+
+static const struct regmap_access_table axp192_volatile_table = {
+ .yes_ranges = axp192_volatile_ranges,
+ .n_yes_ranges = ARRAY_SIZE(axp192_volatile_ranges),
+};
+
/* AXP22x ranges are shared with the AXP809, as they cover the same range */
static const struct regmap_range axp22x_writeable_ranges[] = {
regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
@@ -173,6 +203,25 @@ static const struct resource axp152_pek_resources[] = {
DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
};
+static const struct resource axp192_gpio_resources[] = {
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO0_INPUT, "GPIO0"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO1_INPUT, "GPIO1"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO2_INPUT, "GPIO2"),
+};
+
+static const struct resource axp192_ac_power_supply_resources[] = {
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_PLUGIN, "ACIN_PLUGIN"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_REMOVAL, "ACIN_REMOVAL"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_OVER_V, "ACIN_OVER_V"),
+};
+
+static const struct resource axp192_usb_power_supply_resources[] = {
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_VALID, "VBUS_VALID"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
+};
+
static const struct resource axp20x_ac_power_supply_resources[] = {
DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_ACIN_PLUGIN, "ACIN_PLUGIN"),
DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_ACIN_REMOVAL, "ACIN_REMOVAL"),
@@ -245,6 +294,15 @@ static const struct regmap_config axp152_regmap_config = {
.cache_type = REGCACHE_RBTREE,
};
+static const struct regmap_config axp192_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .wr_table = &axp192_writeable_table,
+ .volatile_table = &axp192_volatile_table,
+ .max_register = AXP20X_CC_CTRL,
+ .cache_type = REGCACHE_RBTREE,
+};
+
static const struct regmap_config axp20x_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
@@ -304,6 +362,55 @@ static const struct regmap_irq axp152_regmap_irqs[] = {
INIT_REGMAP_IRQ(AXP152, GPIO0_INPUT, 2, 0),
};
+static const struct regmap_irq axp192_regmap_irqs[] = {
+ INIT_REGMAP_IRQ(AXP192, ACIN_OVER_V, 0, 7),
+ INIT_REGMAP_IRQ(AXP192, ACIN_PLUGIN, 0, 6),
+ INIT_REGMAP_IRQ(AXP192, ACIN_REMOVAL, 0, 5),
+ INIT_REGMAP_IRQ(AXP192, VBUS_OVER_V, 0, 4),
+ INIT_REGMAP_IRQ(AXP192, VBUS_PLUGIN, 0, 3),
+ INIT_REGMAP_IRQ(AXP192, VBUS_REMOVAL, 0, 2),
+ INIT_REGMAP_IRQ(AXP192, VBUS_V_LOW, 0, 1),
+ INIT_REGMAP_IRQ(AXP192, BATT_PLUGIN, 1, 7),
+ INIT_REGMAP_IRQ(AXP192, BATT_REMOVAL, 1, 6),
+ INIT_REGMAP_IRQ(AXP192, BATT_ENT_ACT_MODE, 1, 5),
+ INIT_REGMAP_IRQ(AXP192, BATT_EXIT_ACT_MODE, 1, 4),
+ INIT_REGMAP_IRQ(AXP192, CHARG, 1, 3),
+ INIT_REGMAP_IRQ(AXP192, CHARG_DONE, 1, 2),
+ INIT_REGMAP_IRQ(AXP192, BATT_TEMP_HIGH, 1, 1),
+ INIT_REGMAP_IRQ(AXP192, BATT_TEMP_LOW, 1, 0),
+ INIT_REGMAP_IRQ(AXP192, DIE_TEMP_HIGH, 2, 7),
+ INIT_REGMAP_IRQ(AXP192, CHARG_I_LOW, 2, 6),
+ INIT_REGMAP_IRQ(AXP192, DCDC1_V_LONG, 2, 5),
+ INIT_REGMAP_IRQ(AXP192, DCDC2_V_LONG, 2, 4),
+ INIT_REGMAP_IRQ(AXP192, DCDC3_V_LONG, 2, 3),
+ INIT_REGMAP_IRQ(AXP192, PEK_SHORT, 2, 1),
+ INIT_REGMAP_IRQ(AXP192, PEK_LONG, 2, 0),
+ INIT_REGMAP_IRQ(AXP192, N_OE_PWR_ON, 3, 7),
+ INIT_REGMAP_IRQ(AXP192, N_OE_PWR_OFF, 3, 6),
+ INIT_REGMAP_IRQ(AXP192, VBUS_VALID, 3, 5),
+ INIT_REGMAP_IRQ(AXP192, VBUS_NOT_VALID, 3, 4),
+ INIT_REGMAP_IRQ(AXP192, VBUS_SESS_VALID, 3, 3),
+ INIT_REGMAP_IRQ(AXP192, VBUS_SESS_END, 3, 2),
+ INIT_REGMAP_IRQ(AXP192, LOW_PWR_LVL, 3, 0),
+ INIT_REGMAP_IRQ(AXP192, TIMER, 4, 7),
+ INIT_REGMAP_IRQ(AXP192, GPIO2_INPUT, 4, 2),
+ INIT_REGMAP_IRQ(AXP192, GPIO1_INPUT, 4, 1),
+ INIT_REGMAP_IRQ(AXP192, GPIO0_INPUT, 4, 0),
+};
+
+static int axp192_get_irq_reg(unsigned int base_reg, int i)
+{
+ /* linear mapping for IRQ1 to IRQ4 */
+ if (i < 4)
+ return base_reg + i;
+
+ /* handle IRQ5 separately */
+ if (base_reg == AXP192_IRQ1_EN)
+ return AXP192_IRQ5_EN;
+ else
+ return AXP192_IRQ5_STATE;
+}
+
static const struct regmap_irq axp20x_regmap_irqs[] = {
INIT_REGMAP_IRQ(AXP20X, ACIN_OVER_V, 0, 7),
INIT_REGMAP_IRQ(AXP20X, ACIN_PLUGIN, 0, 6),
@@ -514,6 +621,19 @@ static const struct regmap_irq_chip axp152_regmap_irq_chip = {
.num_regs = 3,
};
+static const struct regmap_irq_chip axp192_regmap_irq_chip = {
+ .name = "axp192_irq_chip",
+ .status_base = AXP192_IRQ1_STATE,
+ .ack_base = AXP192_IRQ1_STATE,
+ .mask_base = AXP192_IRQ1_EN,
+ .mask_invert = true,
+ .init_ack_masked = true,
+ .irqs = axp192_regmap_irqs,
+ .num_irqs = ARRAY_SIZE(axp192_regmap_irqs),
+ .num_regs = 5,
+ .get_irq_reg = axp192_get_irq_reg,
+};
+
static const struct regmap_irq_chip axp20x_regmap_irq_chip = {
.name = "axp20x_irq_chip",
.status_base = AXP20X_IRQ1_STATE,
@@ -588,6 +708,30 @@ static const struct regmap_irq_chip axp809_regmap_irq_chip = {
.num_regs = 5,
};
+static const struct mfd_cell axp192_cells[] = {
+ {
+ .name = "axp192-gpio",
+ .of_compatible = "x-powers,axp192-gpio",
+ .num_resources = ARRAY_SIZE(axp192_gpio_resources),
+ .resources = axp192_gpio_resources,
+ }, {
+ .name = "axp20x-regulator",
+ }, {
+ .name = "axp192-adc",
+ .of_compatible = "x-powers,axp192-adc",
+ }, {
+ .name = "axp20x-ac-power-supply",
+ .of_compatible = "x-powers,axp202-ac-power-supply",
+ .num_resources = ARRAY_SIZE(axp192_ac_power_supply_resources),
+ .resources = axp192_ac_power_supply_resources,
+ }, {
+ .name = "axp20x-usb-power-supply",
+ .of_compatible = "x-powers,axp192-usb-power-supply",
+ .num_resources = ARRAY_SIZE(axp192_usb_power_supply_resources),
+ .resources = axp192_usb_power_supply_resources,
+ }
+};
+
static const struct mfd_cell axp20x_cells[] = {
{
.name = "axp20x-gpio",
@@ -865,6 +1009,12 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
axp20x->regmap_cfg = &axp152_regmap_config;
axp20x->regmap_irq_chip = &axp152_regmap_irq_chip;
break;
+ case AXP192_ID:
+ axp20x->nr_cells = ARRAY_SIZE(axp192_cells);
+ axp20x->cells = axp192_cells;
+ axp20x->regmap_cfg = &axp192_regmap_config;
+ axp20x->regmap_irq_chip = &axp192_regmap_irq_chip;
+ break;
case AXP202_ID:
case AXP209_ID:
axp20x->nr_cells = ARRAY_SIZE(axp20x_cells);
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 9ab0e2fca7ea..18546e124919 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -12,6 +12,7 @@
enum axp20x_variants {
AXP152_ID = 0,
+ AXP192_ID,
AXP202_ID,
AXP209_ID,
AXP221_ID,
@@ -24,6 +25,7 @@ enum axp20x_variants {
NR_AXP20X_VARIANTS,
};
+#define AXP192_DATACACHE(m) (0x06 + (m))
#define AXP20X_DATACACHE(m) (0x04 + (m))
/* Power supply */
@@ -45,6 +47,13 @@ enum axp20x_variants {
#define AXP152_DCDC_FREQ 0x37
#define AXP152_DCDC_MODE 0x80
+#define AXP192_USB_OTG_STATUS 0x04
+#define AXP192_PWR_OUT_CTRL 0x12
+#define AXP192_DCDC2_V_OUT 0x23
+#define AXP192_DCDC1_V_OUT 0x26
+#define AXP192_DCDC3_V_OUT 0x27
+#define AXP192_LDO2_3_V_OUT 0x28
+
#define AXP20X_PWR_INPUT_STATUS 0x00
#define AXP20X_PWR_OP_MODE 0x01
#define AXP20X_USB_OTG_STATUS 0x02
@@ -139,6 +148,17 @@ enum axp20x_variants {
#define AXP152_IRQ2_STATE 0x49
#define AXP152_IRQ3_STATE 0x4a
+#define AXP192_IRQ1_EN 0x40
+#define AXP192_IRQ2_EN 0x41
+#define AXP192_IRQ3_EN 0x42
+#define AXP192_IRQ4_EN 0x43
+#define AXP192_IRQ1_STATE 0x44
+#define AXP192_IRQ2_STATE 0x45
+#define AXP192_IRQ3_STATE 0x46
+#define AXP192_IRQ4_STATE 0x47
+#define AXP192_IRQ5_EN 0x4a
+#define AXP192_IRQ5_STATE 0x4d
+
#define AXP20X_IRQ1_EN 0x40
#define AXP20X_IRQ2_EN 0x41
#define AXP20X_IRQ3_EN 0x42
@@ -153,6 +173,11 @@ enum axp20x_variants {
#define AXP20X_IRQ6_STATE 0x4d
/* ADC */
+#define AXP192_GPIO2_V_ADC_H 0x68
+#define AXP192_GPIO2_V_ADC_L 0x69
+#define AXP192_GPIO3_V_ADC_H 0x6a
+#define AXP192_GPIO3_V_ADC_L 0x6b
+
#define AXP20X_ACIN_V_ADC_H 0x56
#define AXP20X_ACIN_V_ADC_L 0x57
#define AXP20X_ACIN_I_ADC_H 0x58
@@ -182,6 +207,8 @@ enum axp20x_variants {
#define AXP20X_IPSOUT_V_HIGH_L 0x7f
/* Power supply */
+#define AXP192_GPIO30_IN_RANGE 0x85
+
#define AXP20X_DCDC_MODE 0x80
#define AXP20X_ADC_EN1 0x82
#define AXP20X_ADC_EN2 0x83
@@ -210,6 +237,16 @@ enum axp20x_variants {
#define AXP152_PWM1_FREQ_Y 0x9c
#define AXP152_PWM1_DUTY_CYCLE 0x9d
+#define AXP192_GPIO0_CTRL 0x90
+#define AXP192_LDO_IO0_V_OUT 0x91
+#define AXP192_GPIO1_CTRL 0x92
+#define AXP192_GPIO2_CTRL 0x93
+#define AXP192_GPIO2_0_STATE 0x94
+#define AXP192_GPIO4_3_CTRL 0x95
+#define AXP192_GPIO4_3_STATE 0x96
+#define AXP192_GPIO2_0_PULL 0x97
+#define AXP192_N_RSTO_CTRL 0x9e
+
#define AXP20X_GPIO0_CTRL 0x90
#define AXP20X_LDO5_V_OUT 0x91
#define AXP20X_GPIO1_CTRL 0x92
@@ -287,6 +324,17 @@ enum axp20x_variants {
#define AXP288_FG_TUNE5 0xed
/* Regulators IDs */
+enum {
+ AXP192_DCDC1 = 0,
+ AXP192_DCDC2,
+ AXP192_DCDC3,
+ AXP192_LDO1,
+ AXP192_LDO2,
+ AXP192_LDO3,
+ AXP192_LDO_IO0,
+ AXP192_REG_ID_MAX,
+};
+
enum {
AXP20X_LDO1 = 0,
AXP20X_LDO2,
@@ -440,6 +488,42 @@ enum {
AXP152_IRQ_GPIO0_INPUT,
};
+enum axp192_irqs {
+ AXP192_IRQ_ACIN_OVER_V = 1,
+ AXP192_IRQ_ACIN_PLUGIN,
+ AXP192_IRQ_ACIN_REMOVAL,
+ AXP192_IRQ_VBUS_OVER_V,
+ AXP192_IRQ_VBUS_PLUGIN,
+ AXP192_IRQ_VBUS_REMOVAL,
+ AXP192_IRQ_VBUS_V_LOW,
+ AXP192_IRQ_BATT_PLUGIN,
+ AXP192_IRQ_BATT_REMOVAL,
+ AXP192_IRQ_BATT_ENT_ACT_MODE,
+ AXP192_IRQ_BATT_EXIT_ACT_MODE,
+ AXP192_IRQ_CHARG,
+ AXP192_IRQ_CHARG_DONE,
+ AXP192_IRQ_BATT_TEMP_HIGH,
+ AXP192_IRQ_BATT_TEMP_LOW,
+ AXP192_IRQ_DIE_TEMP_HIGH,
+ AXP192_IRQ_CHARG_I_LOW,
+ AXP192_IRQ_DCDC1_V_LONG,
+ AXP192_IRQ_DCDC2_V_LONG,
+ AXP192_IRQ_DCDC3_V_LONG,
+ AXP192_IRQ_PEK_SHORT = 22,
+ AXP192_IRQ_PEK_LONG,
+ AXP192_IRQ_N_OE_PWR_ON,
+ AXP192_IRQ_N_OE_PWR_OFF,
+ AXP192_IRQ_VBUS_VALID,
+ AXP192_IRQ_VBUS_NOT_VALID,
+ AXP192_IRQ_VBUS_SESS_VALID,
+ AXP192_IRQ_VBUS_SESS_END,
+ AXP192_IRQ_LOW_PWR_LVL = 31,
+ AXP192_IRQ_TIMER,
+ AXP192_IRQ_GPIO2_INPUT = 37,
+ AXP192_IRQ_GPIO1_INPUT,
+ AXP192_IRQ_GPIO0_INPUT,
+};
+
enum {
AXP20X_IRQ_ACIN_OVER_V = 1,
AXP20X_IRQ_ACIN_PLUGIN,
--
2.35.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 07/10] regulator: axp20x: Add support for AXP192
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
` (5 preceding siblings ...)
2022-06-03 13:57 ` [PATCH 06/10] mfd: axp20x: Add support for AXP192 Aidan MacDonald
@ 2022-06-03 13:57 ` Aidan MacDonald
2022-06-06 14:36 ` Mark Brown
2022-06-03 13:57 ` [PATCH 08/10] iio: adc: axp20x_adc: " Aidan MacDonald
` (2 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-03 13:57 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood
Cc: lars, rafael, linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
Add support for the AXP192 PMIC.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
drivers/regulator/axp20x-regulator.c | 101 ++++++++++++++++++++++++---
1 file changed, 92 insertions(+), 9 deletions(-)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index d260c442b788..1edf2bbf1c16 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -27,6 +27,29 @@
#include <linux/regulator/machine.h>
#include <linux/regulator/of_regulator.h>
+#define AXP192_GPIO0_FUNC_MASK GENMASK(2, 0)
+
+#define AXP192_IO_ENABLED 0x02
+#define AXP192_IO_DISABLED 0x06
+
+#define AXP192_WORKMODE_DCDC1_MASK BIT_MASK(3)
+#define AXP192_WORKMODE_DCDC2_MASK BIT_MASK(2)
+#define AXP192_WORKMODE_DCDC3_MASK BIT_MASK(1)
+
+#define AXP192_DCDC1_V_OUT_MASK GENMASK(6, 0)
+#define AXP192_DCDC2_V_OUT_MASK GENMASK(5, 0)
+#define AXP192_DCDC3_V_OUT_MASK GENMASK(6, 0)
+#define AXP192_LDO2_V_OUT_MASK GENMASK(7, 4)
+#define AXP192_LDO3_V_OUT_MASK GENMASK(3, 0)
+#define AXP192_LDO_IO0_V_OUT_MASK GENMASK(7, 4)
+
+#define AXP192_PWR_OUT_EXTEN_MASK BIT_MASK(6)
+#define AXP192_PWR_OUT_DCDC2_MASK BIT_MASK(4)
+#define AXP192_PWR_OUT_LDO3_MASK BIT_MASK(3)
+#define AXP192_PWR_OUT_LDO2_MASK BIT_MASK(2)
+#define AXP192_PWR_OUT_DCDC3_MASK BIT_MASK(1)
+#define AXP192_PWR_OUT_DCDC1_MASK BIT_MASK(0)
+
#define AXP20X_GPIO0_FUNC_MASK GENMASK(3, 0)
#define AXP20X_GPIO1_FUNC_MASK GENMASK(3, 0)
@@ -375,25 +398,32 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
switch (axp20x->variant) {
case AXP209_ID:
- if (id == AXP20X_DCDC2) {
+ if (id == AXP20X_LDO3) {
slew_rates = axp209_dcdc2_ldo3_slew_rates;
rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates);
reg = AXP20X_DCDC2_LDO3_V_RAMP;
- mask = AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE_MASK |
- AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN_MASK;
+ mask = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE_MASK |
+ AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN_MASK;
enable = (ramp > 0) ?
- AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN : 0;
+ AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN : 0;
break;
}
- if (id == AXP20X_LDO3) {
+ fallthrough;
+
+ case AXP192_ID:
+ /*
+ * AXP192 and AXP209 share the same DCDC2 ramp configuration
+ */
+ if ((axp20x->variant == AXP209_ID && id == AXP20X_DCDC2) ||
+ (axp20x->variant == AXP192_ID && id == AXP20X_DCDC2)) {
slew_rates = axp209_dcdc2_ldo3_slew_rates;
rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates);
reg = AXP20X_DCDC2_LDO3_V_RAMP;
- mask = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE_MASK |
- AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN_MASK;
+ mask = AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE_MASK |
+ AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN_MASK;
enable = (ramp > 0) ?
- AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN : 0;
+ AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN : 0;
break;
}
@@ -401,6 +431,7 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
break;
fallthrough;
+
default:
/* Not supported for this regulator */
return -ENOTSUPP;
@@ -415,7 +446,8 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
if (ramp > slew_rates[i])
break;
- if (id == AXP20X_DCDC2)
+ if ((axp20x->variant == AXP209_ID && id == AXP20X_DCDC2) ||
+ (axp20x->variant == AXP192_ID && id == AXP192_DCDC2))
cfg = AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE(i);
else
cfg = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE(i);
@@ -511,6 +543,29 @@ static const struct regulator_ops axp20x_ops_sw = {
.is_enabled = regulator_is_enabled_regmap,
};
+static const struct regulator_desc axp192_regulators[] = {
+ AXP_DESC(AXP192, DCDC1, "dcdc1", "vin1", 700, 3500, 25,
+ AXP192_DCDC1_V_OUT, AXP192_DCDC1_V_OUT_MASK,
+ AXP192_PWR_OUT_CTRL, AXP192_PWR_OUT_DCDC1_MASK),
+ AXP_DESC(AXP192, DCDC2, "dcdc2", "vin2", 700, 2275, 25,
+ AXP192_DCDC2_V_OUT, AXP192_DCDC2_V_OUT_MASK,
+ AXP192_PWR_OUT_CTRL, AXP192_PWR_OUT_DCDC2_MASK),
+ AXP_DESC(AXP192, DCDC3, "dcdc3", "vin3", 700, 3500, 25,
+ AXP192_DCDC3_V_OUT, AXP192_DCDC3_V_OUT_MASK,
+ AXP192_PWR_OUT_CTRL, AXP192_PWR_OUT_DCDC3_MASK),
+ AXP_DESC_FIXED(AXP192, LDO1, "ldo1", "acin", 1250),
+ AXP_DESC(AXP192, LDO2, "ldo2", "ldoin", 1800, 3300, 100,
+ AXP192_LDO2_3_V_OUT, AXP192_LDO2_V_OUT_MASK,
+ AXP192_PWR_OUT_CTRL, AXP192_PWR_OUT_LDO2_MASK),
+ AXP_DESC(AXP192, LDO3, "ldo3", "ldoin", 1800, 3300, 100,
+ AXP192_LDO2_3_V_OUT, AXP192_LDO3_V_OUT_MASK,
+ AXP192_PWR_OUT_CTRL, AXP192_PWR_OUT_LDO3_MASK),
+ AXP_DESC_IO(AXP192, LDO_IO0, "ldo_io0", "ips", 700, 3300, 100,
+ AXP192_LDO_IO0_V_OUT, AXP192_LDO_IO0_V_OUT_MASK,
+ AXP192_GPIO0_CTRL, AXP192_GPIO0_FUNC_MASK,
+ AXP192_IO_ENABLED, AXP192_IO_DISABLED),
+};
+
static const struct linear_range axp20x_ldo4_ranges[] = {
REGULATOR_LINEAR_RANGE(1250000,
AXP20X_LDO4_V_OUT_1250mV_START,
@@ -1008,6 +1063,12 @@ static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
u32 min, max, def, step;
switch (axp20x->variant) {
+ case AXP192_ID:
+ min = 900;
+ max = 2025;
+ def = 1500;
+ step = 75;
+ break;
case AXP202_ID:
case AXP209_ID:
min = 750;
@@ -1100,6 +1161,24 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
unsigned int mask;
switch (axp20x->variant) {
+ case AXP192_ID:
+ switch (id) {
+ case AXP192_DCDC1:
+ mask = AXP192_WORKMODE_DCDC1_MASK;
+ break;
+ case AXP192_DCDC2:
+ mask = AXP192_WORKMODE_DCDC2_MASK;
+ break;
+ case AXP192_DCDC3:
+ mask = AXP192_WORKMODE_DCDC3_MASK;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ workmode <<= ffs(mask) - 1;
+ break;
+
case AXP202_ID:
case AXP209_ID:
if ((id != AXP20X_DCDC2) && (id != AXP20X_DCDC3))
@@ -1220,6 +1299,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
bool drivevbus = false;
switch (axp20x->variant) {
+ case AXP192_ID:
+ regulators = axp192_regulators;
+ nregulators = AXP192_REG_ID_MAX;
+ break;
case AXP202_ID:
case AXP209_ID:
regulators = axp20x_regulators;
--
2.35.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 08/10] iio: adc: axp20x_adc: Add support for AXP192
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
` (6 preceding siblings ...)
2022-06-03 13:57 ` [PATCH 07/10] regulator: " Aidan MacDonald
@ 2022-06-03 13:57 ` Aidan MacDonald
2022-06-03 16:47 ` Jonathan Cameron
2022-06-03 13:57 ` [PATCH 09/10] power: supply: axp20x_usb_power: " Aidan MacDonald
2022-06-03 13:57 ` [PATCH 10/10] pinctrl: Add AXP192 pin control driver Aidan MacDonald
9 siblings, 1 reply; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-03 13:57 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood
Cc: lars, rafael, linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
The AXP192 is identical to the AXP20x, except for the addition of
two more GPIO ADC channels.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
drivers/iio/adc/axp20x_adc.c | 289 +++++++++++++++++++++++++++++++++--
1 file changed, 280 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
index 53bf7d4899d2..7d2bf9529420 100644
--- a/drivers/iio/adc/axp20x_adc.c
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -21,6 +21,9 @@
#include <linux/iio/machine.h>
#include <linux/mfd/axp20x.h>
+#define AXP192_ADC_EN1_MASK GENMASK(7, 0)
+#define AXP192_ADC_EN2_MASK (BIT(7) | GENMASK(3, 0))
+
#define AXP20X_ADC_EN1_MASK GENMASK(7, 0)
#define AXP20X_ADC_EN2_MASK (GENMASK(3, 2) | BIT(7))
@@ -31,6 +34,15 @@
#define AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(x) ((x) & BIT(0))
#define AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(x) (((x) & BIT(0)) << 1)
+#define AXP192_GPIO30_IN_RANGE_GPIO0 BIT(0)
+#define AXP192_GPIO30_IN_RANGE_GPIO1 BIT(1)
+#define AXP192_GPIO30_IN_RANGE_GPIO2 BIT(2)
+#define AXP192_GPIO30_IN_RANGE_GPIO3 BIT(3)
+#define AXP192_GPIO30_IN_RANGE_GPIO0_VAL(x) ((x) & BIT(0))
+#define AXP192_GPIO30_IN_RANGE_GPIO1_VAL(x) (((x) & BIT(0)) << 1)
+#define AXP192_GPIO30_IN_RANGE_GPIO2_VAL(x) (((x) & BIT(0)) << 2)
+#define AXP192_GPIO30_IN_RANGE_GPIO3_VAL(x) (((x) & BIT(0)) << 3)
+
#define AXP20X_ADC_RATE_MASK GENMASK(7, 6)
#define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4)
#define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK)
@@ -70,6 +82,25 @@ struct axp20x_adc_iio {
const struct axp_data *data;
};
+enum axp192_adc_channel_v {
+ AXP192_ACIN_V = 0,
+ AXP192_VBUS_V,
+ AXP192_TS_IN,
+ AXP192_GPIO0_V,
+ AXP192_GPIO1_V,
+ AXP192_GPIO2_V,
+ AXP192_GPIO3_V,
+ AXP192_IPSOUT_V,
+ AXP192_BATT_V,
+};
+
+enum axp192_adc_channel_i {
+ AXP192_ACIN_I = 0,
+ AXP192_VBUS_I,
+ AXP192_BATT_CHRG_I,
+ AXP192_BATT_DISCHRG_I,
+};
+
enum axp20x_adc_channel_v {
AXP20X_ACIN_V = 0,
AXP20X_VBUS_V,
@@ -157,6 +188,43 @@ static struct iio_map axp22x_maps[] = {
* The only exception is for the battery. batt_v will be in_voltage6_raw and
* charge current in_current6_raw and discharge current will be in_current7_raw.
*/
+static const struct iio_chan_spec axp192_adc_channels[] = {
+ AXP20X_ADC_CHANNEL(AXP192_ACIN_V, "acin_v", IIO_VOLTAGE,
+ AXP20X_ACIN_V_ADC_H),
+ AXP20X_ADC_CHANNEL(AXP192_ACIN_I, "acin_i", IIO_CURRENT,
+ AXP20X_ACIN_I_ADC_H),
+ AXP20X_ADC_CHANNEL(AXP192_VBUS_V, "vbus_v", IIO_VOLTAGE,
+ AXP20X_VBUS_V_ADC_H),
+ AXP20X_ADC_CHANNEL(AXP192_VBUS_I, "vbus_i", IIO_CURRENT,
+ AXP20X_VBUS_I_ADC_H),
+ {
+ .type = IIO_TEMP,
+ .address = AXP20X_TEMP_ADC_H,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OFFSET),
+ .datasheet_name = "pmic_temp",
+ },
+ AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO0_V, "gpio0_v", IIO_VOLTAGE,
+ AXP20X_GPIO0_V_ADC_H),
+ AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO1_V, "gpio1_v", IIO_VOLTAGE,
+ AXP20X_GPIO1_V_ADC_H),
+ AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO2_V, "gpio2_v", IIO_VOLTAGE,
+ AXP192_GPIO2_V_ADC_H),
+ AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO3_V, "gpio3_v", IIO_VOLTAGE,
+ AXP192_GPIO3_V_ADC_H),
+ AXP20X_ADC_CHANNEL(AXP192_IPSOUT_V, "ipsout_v", IIO_VOLTAGE,
+ AXP20X_IPSOUT_V_HIGH_H),
+ AXP20X_ADC_CHANNEL(AXP192_BATT_V, "batt_v", IIO_VOLTAGE,
+ AXP20X_BATT_V_H),
+ AXP20X_ADC_CHANNEL(AXP192_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT,
+ AXP20X_BATT_CHRG_I_H),
+ AXP20X_ADC_CHANNEL(AXP192_BATT_DISCHRG_I, "batt_dischrg_i", IIO_CURRENT,
+ AXP20X_BATT_DISCHRG_I_H),
+ AXP20X_ADC_CHANNEL(AXP192_TS_IN, "ts_v", IIO_VOLTAGE,
+ AXP20X_TS_IN_H),
+};
+
static const struct iio_chan_spec axp20x_adc_channels[] = {
AXP20X_ADC_CHANNEL(AXP20X_ACIN_V, "acin_v", IIO_VOLTAGE,
AXP20X_ACIN_V_ADC_H),
@@ -277,6 +345,44 @@ static int axp813_adc_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
}
+static int axp192_adc_scale_voltage(int channel, int *val, int *val2)
+{
+ switch (channel) {
+ case AXP192_ACIN_V:
+ case AXP192_VBUS_V:
+ *val = 1;
+ *val2 = 700000;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ case AXP192_GPIO0_V:
+ case AXP192_GPIO1_V:
+ case AXP192_GPIO2_V:
+ case AXP192_GPIO3_V:
+ *val = 0;
+ *val2 = 500000;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ case AXP192_BATT_V:
+ *val = 1;
+ *val2 = 100000;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ case AXP192_IPSOUT_V:
+ *val = 1;
+ *val2 = 400000;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ case AXP192_TS_IN:
+ /* 0.8 mV per LSB */
+ *val = 0;
+ *val2 = 800000;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ default:
+ return -EINVAL;
+ }
+}
+
static int axp20x_adc_scale_voltage(int channel, int *val, int *val2)
{
switch (channel) {
@@ -380,6 +486,29 @@ static int axp20x_adc_scale_current(int channel, int *val, int *val2)
}
}
+static int axp192_adc_scale(struct iio_chan_spec const *chan, int *val,
+ int *val2)
+{
+ switch (chan->type) {
+ case IIO_VOLTAGE:
+ return axp192_adc_scale_voltage(chan->channel, val, val2);
+
+ case IIO_CURRENT:
+ /*
+ * AXP192 current channels are identical to the AXP20x,
+ * therefore we can re-use the scaling function.
+ */
+ return axp20x_adc_scale_current(chan->channel, val, val2);
+
+ case IIO_TEMP:
+ *val = 100;
+ return IIO_VAL_INT;
+
+ default:
+ return -EINVAL;
+ }
+}
+
static int axp20x_adc_scale(struct iio_chan_spec const *chan, int *val,
int *val2)
{
@@ -439,6 +568,42 @@ static int axp813_adc_scale(struct iio_chan_spec const *chan, int *val,
}
}
+static int axp192_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
+ int *val)
+{
+ struct axp20x_adc_iio *info = iio_priv(indio_dev);
+ int ret;
+
+ ret = regmap_read(info->regmap, AXP192_GPIO30_IN_RANGE, val);
+ if (ret < 0)
+ return ret;
+
+ switch (channel) {
+ case AXP192_GPIO0_V:
+ *val &= AXP192_GPIO30_IN_RANGE_GPIO0;
+ break;
+
+ case AXP192_GPIO1_V:
+ *val &= AXP192_GPIO30_IN_RANGE_GPIO1;
+ break;
+
+ case AXP192_GPIO2_V:
+ *val &= AXP192_GPIO30_IN_RANGE_GPIO2;
+ break;
+
+ case AXP192_GPIO3_V:
+ *val &= AXP192_GPIO30_IN_RANGE_GPIO3;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ *val = *val ? 700000 : 0;
+
+ return IIO_VAL_INT;
+}
+
static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
int *val)
{
@@ -467,6 +632,22 @@ static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
return IIO_VAL_INT;
}
+static int axp192_adc_offset(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val)
+{
+ switch (chan->type) {
+ case IIO_VOLTAGE:
+ return axp192_adc_offset_voltage(indio_dev, chan->channel, val);
+
+ case IIO_TEMP:
+ *val = -1447;
+ return IIO_VAL_INT;
+
+ default:
+ return -EINVAL;
+ }
+}
+
static int axp20x_adc_offset(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val)
{
@@ -483,6 +664,25 @@ static int axp20x_adc_offset(struct iio_dev *indio_dev,
}
}
+static int axp192_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_OFFSET:
+ return axp192_adc_offset(indio_dev, chan, val);
+
+ case IIO_CHAN_INFO_SCALE:
+ return axp192_adc_scale(chan, val, val2);
+
+ case IIO_CHAN_INFO_RAW:
+ return axp20x_adc_raw(indio_dev, chan, val);
+
+ default:
+ return -EINVAL;
+ }
+}
+
static int axp20x_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val,
int *val2, long mask)
@@ -543,6 +743,54 @@ static int axp813_read_raw(struct iio_dev *indio_dev,
}
}
+static int axp192_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int val, int val2,
+ long mask)
+{
+ struct axp20x_adc_iio *info = iio_priv(indio_dev);
+ unsigned int reg, regval;
+
+ /*
+ * The AXP192 PMIC allows the user to choose between 0V and 0.7V offsets
+ * for (independently) GPIO0-3 when in ADC mode.
+ */
+ if (mask != IIO_CHAN_INFO_OFFSET)
+ return -EINVAL;
+
+ if (val != 0 && val != 700000)
+ return -EINVAL;
+
+ val = val ? 1 : 0;
+
+ switch (chan->channel) {
+ case AXP192_GPIO0_V:
+ reg = AXP192_GPIO30_IN_RANGE_GPIO0;
+ regval = AXP192_GPIO30_IN_RANGE_GPIO0_VAL(val);
+ break;
+
+ case AXP192_GPIO1_V:
+ reg = AXP192_GPIO30_IN_RANGE_GPIO1;
+ regval = AXP192_GPIO30_IN_RANGE_GPIO1_VAL(val);
+ break;
+
+ case AXP192_GPIO2_V:
+ reg = AXP192_GPIO30_IN_RANGE_GPIO2;
+ regval = AXP192_GPIO30_IN_RANGE_GPIO2_VAL(val);
+ break;
+
+ case AXP192_GPIO3_V:
+ reg = AXP192_GPIO30_IN_RANGE_GPIO3;
+ regval = AXP192_GPIO30_IN_RANGE_GPIO3_VAL(val);
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(info->regmap, AXP192_GPIO30_IN_RANGE, reg,
+ regval);
+}
+
static int axp20x_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int val, int val2,
long mask)
@@ -581,6 +829,18 @@ static int axp20x_write_raw(struct iio_dev *indio_dev,
regval);
}
+static int axp192_read_label(struct iio_dev *iio_dev,
+ const struct iio_chan_spec *chan, char *label)
+{
+ return snprintf(label, PAGE_SIZE, "%s\n", chan->datasheet_name);
+}
+
+static const struct iio_info axp192_adc_iio_info = {
+ .read_raw = axp192_read_raw,
+ .write_raw = axp192_write_raw,
+ .read_label = axp192_read_label,
+};
+
static const struct iio_info axp20x_adc_iio_info = {
.read_raw = axp20x_read_raw,
.write_raw = axp20x_write_raw,
@@ -620,19 +880,29 @@ struct axp_data {
int num_channels;
struct iio_chan_spec const *channels;
unsigned long adc_en1_mask;
+ unsigned long adc_en2_mask;
int (*adc_rate)(struct axp20x_adc_iio *info,
int rate);
- bool adc_en2;
struct iio_map *maps;
};
+static const struct axp_data axp192_data = {
+ .iio_info = &axp192_adc_iio_info,
+ .num_channels = ARRAY_SIZE(axp192_adc_channels),
+ .channels = axp192_adc_channels,
+ .adc_en1_mask = AXP192_ADC_EN1_MASK,
+ .adc_en2_mask = AXP192_ADC_EN2_MASK,
+ .adc_rate = axp20x_adc_rate,
+ .maps = axp20x_maps,
+};
+
static const struct axp_data axp20x_data = {
.iio_info = &axp20x_adc_iio_info,
.num_channels = ARRAY_SIZE(axp20x_adc_channels),
.channels = axp20x_adc_channels,
.adc_en1_mask = AXP20X_ADC_EN1_MASK,
+ .adc_en2_mask = AXP20X_ADC_EN2_MASK,
.adc_rate = axp20x_adc_rate,
- .adc_en2 = true,
.maps = axp20x_maps,
};
@@ -642,7 +912,6 @@ static const struct axp_data axp22x_data = {
.channels = axp22x_adc_channels,
.adc_en1_mask = AXP22X_ADC_EN1_MASK,
.adc_rate = axp22x_adc_rate,
- .adc_en2 = false,
.maps = axp22x_maps,
};
@@ -652,11 +921,11 @@ static const struct axp_data axp813_data = {
.channels = axp813_adc_channels,
.adc_en1_mask = AXP22X_ADC_EN1_MASK,
.adc_rate = axp813_adc_rate,
- .adc_en2 = false,
.maps = axp22x_maps,
};
static const struct of_device_id axp20x_adc_of_match[] = {
+ { .compatible = "x-powers,axp192-adc", .data = (void *)&axp192_data, },
{ .compatible = "x-powers,axp209-adc", .data = (void *)&axp20x_data, },
{ .compatible = "x-powers,axp221-adc", .data = (void *)&axp22x_data, },
{ .compatible = "x-powers,axp813-adc", .data = (void *)&axp813_data, },
@@ -665,6 +934,7 @@ static const struct of_device_id axp20x_adc_of_match[] = {
MODULE_DEVICE_TABLE(of, axp20x_adc_of_match);
static const struct platform_device_id axp20x_adc_id_match[] = {
+ { .name = "axp192-adc", .driver_data = (kernel_ulong_t)&axp192_data, },
{ .name = "axp20x-adc", .driver_data = (kernel_ulong_t)&axp20x_data, },
{ .name = "axp22x-adc", .driver_data = (kernel_ulong_t)&axp22x_data, },
{ .name = "axp813-adc", .driver_data = (kernel_ulong_t)&axp813_data, },
@@ -710,10 +980,11 @@ static int axp20x_probe(struct platform_device *pdev)
/* Enable the ADCs on IP */
regmap_write(info->regmap, AXP20X_ADC_EN1, info->data->adc_en1_mask);
- if (info->data->adc_en2)
- /* Enable GPIO0/1 and internal temperature ADCs */
+ if (info->data->adc_en2_mask)
+ /* Enable GPIO and internal temperature ADCs */
regmap_update_bits(info->regmap, AXP20X_ADC_EN2,
- AXP20X_ADC_EN2_MASK, AXP20X_ADC_EN2_MASK);
+ info->data->adc_en2_mask,
+ info->data->adc_en2_mask);
/* Configure ADCs rate */
info->data->adc_rate(info, 100);
@@ -738,7 +1009,7 @@ static int axp20x_probe(struct platform_device *pdev)
fail_map:
regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
- if (info->data->adc_en2)
+ if (info->data->adc_en2_mask)
regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
return ret;
@@ -754,7 +1025,7 @@ static int axp20x_remove(struct platform_device *pdev)
regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
- if (info->data->adc_en2)
+ if (info->data->adc_en2_mask)
regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
return 0;
--
2.35.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 09/10] power: supply: axp20x_usb_power: Add support for AXP192
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
` (7 preceding siblings ...)
2022-06-03 13:57 ` [PATCH 08/10] iio: adc: axp20x_adc: " Aidan MacDonald
@ 2022-06-03 13:57 ` Aidan MacDonald
2022-06-05 15:13 ` kernel test robot
2022-06-03 13:57 ` [PATCH 10/10] pinctrl: Add AXP192 pin control driver Aidan MacDonald
9 siblings, 1 reply; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-03 13:57 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood
Cc: lars, rafael, linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
The AXP192 is mostly the same as the AXP202 but has a different
current limit.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
drivers/power/supply/axp20x_usb_power.c | 75 +++++++++++++++++++++++--
1 file changed, 70 insertions(+), 5 deletions(-)
diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
index a1e6d1d44808..e1266b8265bc 100644
--- a/drivers/power/supply/axp20x_usb_power.c
+++ b/drivers/power/supply/axp20x_usb_power.c
@@ -48,6 +48,9 @@
#define AXP813_VBUS_CLIMIT_2000mA 2
#define AXP813_VBUS_CLIMIT_2500mA 3
+#define AXP192_VBUS_CLIMIT_EN BIT(1)
+#define AXP192_VBUS_CLIMIT_100mA BIT(0)
+
#define AXP20X_ADC_EN1_VBUS_CURR BIT(2)
#define AXP20X_ADC_EN1_VBUS_VOLT BIT(3)
@@ -121,6 +124,24 @@ static void axp20x_usb_power_poll_vbus(struct work_struct *work)
mod_delayed_work(system_power_efficient_wq, &power->vbus_detect, DEBOUNCE_TIME);
}
+static int axp192_get_current_max(struct axp20x_usb_power *power, int *val)
+{
+ unsigned int v;
+ int ret = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
+
+ if (ret)
+ return ret;
+
+ if (!(v & AXP192_VBUS_CLIMIT_EN))
+ *val = -1;
+ else if (v & AXP192_VBUS_CLIMIT_100mA)
+ *val = 100000;
+ else
+ *val = 500000;
+
+ return 0;
+}
+
static int axp20x_get_current_max(struct axp20x_usb_power *power, int *val)
{
unsigned int v;
@@ -179,7 +200,7 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
enum power_supply_property psp, union power_supply_propval *val)
{
struct axp20x_usb_power *power = power_supply_get_drvdata(psy);
- unsigned int input, v;
+ unsigned int input, v, reg;
int ret;
switch (psp) {
@@ -215,6 +236,8 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
case POWER_SUPPLY_PROP_CURRENT_MAX:
if (power->axp20x_id == AXP813_ID)
return axp813_get_current_max(power, &val->intval);
+ else if (power->axp20x_id == AXP192_ID)
+ return axp192_get_current_max(power, &val->intval);
return axp20x_get_current_max(power, &val->intval);
case POWER_SUPPLY_PROP_CURRENT_NOW:
if (IS_ENABLED(CONFIG_AXP20X_ADC)) {
@@ -256,15 +279,26 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
val->intval = POWER_SUPPLY_HEALTH_GOOD;
- if (power->axp20x_id == AXP202_ID) {
- ret = regmap_read(power->regmap,
- AXP20X_USB_OTG_STATUS, &v);
+ switch (power->axp20x_id) {
+ case AXP192_ID:
+ /* Same layout as the AXP202, but different address */
+ reg = AXP192_USB_OTG_STATUS;
+ fallthrough;
+
+ case AXP202_ID:
+ if (power->axp20x_id == AXP202_ID)
+ reg = AXP20X_USB_OTG_STATUS;
+
+ ret = regmap_read(power->regmap, reg, &v);
if (ret)
return ret;
if (!(v & AXP20X_USB_STATUS_VBUS_VALID))
val->intval =
POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
+
+ default:
+ break;
}
break;
case POWER_SUPPLY_PROP_PRESENT:
@@ -316,6 +350,24 @@ static int axp20x_usb_power_set_voltage_min(struct axp20x_usb_power *power,
return -EINVAL;
}
+static int axp192_usb_power_set_current_max(struct axp20x_usb_power *power,
+ int intval)
+{
+ int val = AXP192_VBUS_CLIMIT_EN;
+ const int mask = AXP192_VBUS_CLIMIT_EN | AXP192_VBUS_CLIMIT_100mA;
+
+ switch (intval) {
+ case 100000:
+ val |= AXP192_VBUS_CLIMIT_100mA;
+ fallthrough;
+ case 500000:
+ return regmap_update_bits(power->regmap,
+ AXP20X_VBUS_IPSOUT_MGMT, mask, val);
+ default:
+ return -EINVAL;
+ }
+}
+
static int axp813_usb_power_set_current_max(struct axp20x_usb_power *power,
int intval)
{
@@ -383,6 +435,9 @@ static int axp20x_usb_power_set_property(struct power_supply *psy,
if (power->axp20x_id == AXP813_ID)
return axp813_usb_power_set_current_max(power,
val->intval);
+ else if (power->axp20x_id == AXP192_ID)
+ return axp192_usb_power_set_current_max(power,
+ val->intval);
return axp20x_usb_power_set_current_max(power, val->intval);
default:
@@ -468,6 +523,13 @@ struct axp_data {
enum axp20x_variants axp20x_id;
};
+static const struct axp_data axp192_data = {
+ .power_desc = &axp20x_usb_power_desc,
+ .irq_names = axp20x_irq_names,
+ .num_irq_names = ARRAY_SIZE(axp20x_irq_names),
+ .axp20x_id = AXP192_ID,
+};
+
static const struct axp_data axp202_data = {
.power_desc = &axp20x_usb_power_desc,
.irq_names = axp20x_irq_names,
@@ -600,7 +662,7 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
if (ret)
return ret;
- if (power->axp20x_id == AXP202_ID) {
+ if (power->axp20x_id == AXP192_ID || power->axp20x_id == AXP202_ID) {
/* Enable vbus valid checking */
ret = regmap_update_bits(power->regmap, AXP20X_VBUS_MON,
AXP20X_VBUS_MON_VBUS_VALID,
@@ -659,6 +721,9 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
static const struct of_device_id axp20x_usb_power_match[] = {
{
+ .compatible = "x-powers,axp192-usb-power-supply",
+ .data = &axp192_data,
+ }, {
.compatible = "x-powers,axp202-usb-power-supply",
.data = &axp202_data,
}, {
--
2.35.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 10/10] pinctrl: Add AXP192 pin control driver
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
` (8 preceding siblings ...)
2022-06-03 13:57 ` [PATCH 09/10] power: supply: axp20x_usb_power: " Aidan MacDonald
@ 2022-06-03 13:57 ` Aidan MacDonald
2022-06-15 13:44 ` Linus Walleij
2022-06-15 14:51 ` Andy Shevchenko
9 siblings, 2 replies; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-03 13:57 UTC (permalink / raw)
To: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood
Cc: lars, rafael, linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
The AXP192 PMIC's GPIO registers are much different from the GPIO
registers of the AXP20x and AXP813 PMICs supported by the existing
pinctrl-axp209 driver. It makes more sense to add a new driver for
the AXP192, rather than add support in the existing axp20x driver.
The pinctrl-axp192 driver is considerably more flexible in terms of
register layout and should be able to support other X-Powers PMICs.
Interrupts and pull down resistor configuration are supported too.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
drivers/pinctrl/Kconfig | 14 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-axp192.c | 589 +++++++++++++++++++++++++++++++
3 files changed, 604 insertions(+)
create mode 100644 drivers/pinctrl/pinctrl-axp192.c
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index f52960d2dfbe..a71e35de333d 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -113,6 +113,20 @@ config PINCTRL_AT91PIO4
Say Y here to enable the at91 pinctrl/gpio driver for Atmel PIO4
controller available on sama5d2 SoC.
+config PINCTRL_AXP192
+ tristate "X-Powers AXP192 PMIC pinctrl and GPIO Support"
+ depends on MFD_AXP20X
+ depends on OF
+ select PINMUX
+ select GENERIC_PINCONF
+ select GPIOLIB
+ help
+ AXP PMICs provides multiple GPIOs that can be muxed for different
+ functions. This driver bundles a pinctrl driver to select the function
+ muxing and a GPIO driver to handle the GPIO when the GPIO function is
+ selected.
+ Say Y to enable pinctrl and GPIO support for the AXP192 PMIC.
+
config PINCTRL_AXP209
tristate "X-Powers AXP209 PMIC pinctrl and GPIO Support"
depends on MFD_AXP20X
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index e76f5cdc64b0..9d2b6420c5dd 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_PINCTRL_ARTPEC6) += pinctrl-artpec6.o
obj-$(CONFIG_PINCTRL_AS3722) += pinctrl-as3722.o
obj-$(CONFIG_PINCTRL_AT91) += pinctrl-at91.o
obj-$(CONFIG_PINCTRL_AT91PIO4) += pinctrl-at91-pio4.o
+obj-$(CONFIG_PINCTRL_AXP192) += pinctrl-axp192.o
obj-$(CONFIG_PINCTRL_AXP209) += pinctrl-axp209.o
obj-$(CONFIG_PINCTRL_BM1880) += pinctrl-bm1880.o
obj-$(CONFIG_PINCTRL_DA850_PUPD) += pinctrl-da850-pupd.o
diff --git a/drivers/pinctrl/pinctrl-axp192.c b/drivers/pinctrl/pinctrl-axp192.c
new file mode 100644
index 000000000000..0ff2d0b84978
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-axp192.c
@@ -0,0 +1,589 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AXP192 pinctrl and GPIO driver
+ *
+ * Copyright (C) 2022 Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/axp20x.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+enum {
+ AXP192_FUNC_OUTPUT = 0,
+ AXP192_FUNC_INPUT,
+ AXP192_FUNC_LDO,
+ AXP192_FUNC_PWM,
+ AXP192_FUNC_ADC,
+ AXP192_FUNC_LOW_OUTPUT,
+ AXP192_FUNC_FLOATING,
+ AXP192_FUNC_EXT_CHG_CTL,
+ AXP192_FUNC_LDO_STATUS,
+ AXP192_FUNCS_NB,
+};
+
+struct axp192_pctl_function {
+ const char *name;
+ /* Mux value written to the control register to select the function (-1 if unsupported) */
+ const u8 *muxvals;
+ const char * const *groups;
+ unsigned int ngroups;
+};
+
+struct axp192_pctl_reg_info {
+ u8 reg;
+ u8 mask;
+};
+
+struct axp192_pctl_desc {
+ unsigned int npins;
+ const struct pinctrl_pin_desc *pins;
+ /* Description of the function control register for each pin */
+ const struct axp192_pctl_reg_info *ctrl_regs;
+ /* Description of the output signal register for each pin */
+ const struct axp192_pctl_reg_info *out_regs;
+ /* Description of the input signal register for each pin */
+ const struct axp192_pctl_reg_info *in_regs;
+ /* Description of the pull down resistor config register for each pin */
+ const struct axp192_pctl_reg_info *pull_down_regs;
+
+ unsigned int nfunctions;
+ const struct axp192_pctl_function *functions;
+};
+
+static const struct pinctrl_pin_desc axp192_pins[] = {
+ PINCTRL_PIN(0, "GPIO0"),
+ PINCTRL_PIN(1, "GPIO1"),
+ PINCTRL_PIN(2, "GPIO2"),
+ PINCTRL_PIN(3, "GPIO3"),
+ PINCTRL_PIN(4, "GPIO4"),
+ PINCTRL_PIN(5, "N_RSTO"),
+};
+
+static const char * const axp192_io_groups[] = { "GPIO0", "GPIO1", "GPIO2",
+ "GPIO3", "GPIO4", "N_RSTO" };
+static const char * const axp192_ldo_groups[] = { "GPIO0" };
+static const char * const axp192_pwm_groups[] = { "GPIO1", "GPIO2" };
+static const char * const axp192_adc_groups[] = { "GPIO0", "GPIO1", "GPIO2", "GPIO3" };
+static const char * const axp192_extended_io_groups[] = { "GPIO0", "GPIO1", "GPIO2" };
+static const char * const axp192_ext_chg_ctl_groups[] = { "GPIO3", "GPIO4" };
+static const char * const axp192_ldo_status_groups[] = { "N_RSTO" };
+
+static const u8 axp192_output_muxvals[] = { 0, 0, 0, 1, 1, 2 };
+static const u8 axp192_input_muxvals[] = { 1, 1, 1, 2, 2, 3 };
+static const u8 axp192_ldo_muxvals[] = { 2, -1, -1, -1, -1, -1 };
+static const u8 axp192_pwm_muxvals[] = { -1, 2, 2, -1, -1, -1 };
+static const u8 axp192_adc_muxvals[] = { 4, 4, 4, 3, -1, -1 };
+static const u8 axp192_low_output_muxvals[] = { 5, 5, 5, -1, -1, -1 };
+static const u8 axp192_floating_muxvals[] = { 6, 6, 6, -1, -1, -1 };
+static const u8 axp192_ext_chg_ctl_muxvals[] = { -1, -1, -1, 0, 0, -1 };
+static const u8 axp192_ldo_status_muxvals[] = { -1, -1, -1, -1, -1, 0 };
+
+static const struct axp192_pctl_function axp192_functions[AXP192_FUNCS_NB] = {
+ [AXP192_FUNC_OUTPUT] = {
+ .name = "output",
+ .muxvals = axp192_output_muxvals,
+ .groups = axp192_io_groups,
+ .ngroups = ARRAY_SIZE(axp192_io_groups),
+ },
+ [AXP192_FUNC_INPUT] = {
+ .name = "input",
+ .muxvals = axp192_input_muxvals,
+ .groups = axp192_io_groups,
+ .ngroups = ARRAY_SIZE(axp192_io_groups),
+ },
+ [AXP192_FUNC_LDO] = {
+ .name = "ldo",
+ .muxvals = axp192_ldo_muxvals,
+ .groups = axp192_ldo_groups,
+ .ngroups = ARRAY_SIZE(axp192_ldo_groups),
+ },
+ [AXP192_FUNC_PWM] = {
+ .name = "pwm",
+ .muxvals = axp192_pwm_muxvals,
+ .groups = axp192_pwm_groups,
+ .ngroups = ARRAY_SIZE(axp192_pwm_groups),
+ },
+ [AXP192_FUNC_ADC] = {
+ .name = "adc",
+ .muxvals = axp192_adc_muxvals,
+ .groups = axp192_adc_groups,
+ .ngroups = ARRAY_SIZE(axp192_adc_groups),
+ },
+ [AXP192_FUNC_LOW_OUTPUT] = {
+ .name = "low_output",
+ .muxvals = axp192_low_output_muxvals,
+ .groups = axp192_extended_io_groups,
+ .ngroups = ARRAY_SIZE(axp192_extended_io_groups),
+ },
+ [AXP192_FUNC_FLOATING] = {
+ .name = "floating",
+ .muxvals = axp192_floating_muxvals,
+ .groups = axp192_extended_io_groups,
+ .ngroups = ARRAY_SIZE(axp192_extended_io_groups),
+ },
+ [AXP192_FUNC_EXT_CHG_CTL] = {
+ .name = "ext_chg_ctl",
+ .muxvals = axp192_ext_chg_ctl_muxvals,
+ .groups = axp192_ext_chg_ctl_groups,
+ .ngroups = ARRAY_SIZE(axp192_ext_chg_ctl_groups),
+ },
+ [AXP192_FUNC_LDO_STATUS] = {
+ .name = "ldo_status",
+ .muxvals = axp192_ldo_status_muxvals,
+ .groups = axp192_ldo_groups,
+ .ngroups = ARRAY_SIZE(axp192_ldo_status_groups),
+ },
+};
+
+static const struct axp192_pctl_reg_info axp192_pin_ctrl_regs[] = {
+ { .reg = AXP192_GPIO0_CTRL, .mask = 0x07 },
+ { .reg = AXP192_GPIO1_CTRL, .mask = 0x07 },
+ { .reg = AXP192_GPIO2_CTRL, .mask = 0x07 },
+ { .reg = AXP192_GPIO4_3_CTRL, .mask = 0x03 },
+ { .reg = AXP192_GPIO4_3_CTRL, .mask = 0x0c },
+ { .reg = AXP192_N_RSTO_CTRL, .mask = 0xc0 },
+};
+
+static const struct axp192_pctl_reg_info axp192_pin_in_regs[] = {
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(4) },
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(5) },
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(6) },
+ { .reg = AXP192_GPIO4_3_STATE, .mask = BIT(4) },
+ { .reg = AXP192_GPIO4_3_STATE, .mask = BIT(5) },
+ { .reg = AXP192_N_RSTO_CTRL, .mask = BIT(4) },
+};
+
+static const struct axp192_pctl_reg_info axp192_pin_out_regs[] = {
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(0) },
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(1) },
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(2) },
+ { .reg = AXP192_GPIO4_3_STATE, .mask = BIT(0) },
+ { .reg = AXP192_GPIO4_3_STATE, .mask = BIT(1) },
+ { .reg = AXP192_N_RSTO_CTRL, .mask = BIT(5) },
+};
+
+static const struct axp192_pctl_reg_info axp192_pull_down_regs[] = {
+ { .reg = AXP192_GPIO2_0_PULL, .mask = BIT(0) },
+ { .reg = AXP192_GPIO2_0_PULL, .mask = BIT(1) },
+ { .reg = AXP192_GPIO2_0_PULL, .mask = BIT(2) },
+ { .reg = 0, .mask = 0 /* unsupported */ },
+ { .reg = 0, .mask = 0 /* unsupported */ },
+ { .reg = 0, .mask = 0 /* unsupported */ },
+};
+
+static const struct axp192_pctl_desc axp192_data = {
+ .npins = ARRAY_SIZE(axp192_pins),
+ .pins = axp192_pins,
+ .ctrl_regs = axp192_pin_ctrl_regs,
+ .out_regs = axp192_pin_out_regs,
+ .in_regs = axp192_pin_in_regs,
+ .pull_down_regs = axp192_pull_down_regs,
+
+ .nfunctions = ARRAY_SIZE(axp192_functions),
+ .functions = axp192_functions,
+};
+
+
+struct axp192_pctl {
+ struct gpio_chip chip;
+ struct regmap *regmap;
+ struct pinctrl_dev *pctl_dev;
+ struct device *dev;
+ const struct axp192_pctl_desc *desc;
+ int *irqs;
+};
+
+static int axp192_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct axp192_pctl *pctl = gpiochip_get_data(chip);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->in_regs[offset];
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(pctl->regmap, reginfo->reg, &val);
+ if (ret)
+ return ret;
+
+ return !!(val & reginfo->mask);
+}
+
+static int axp192_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+ struct axp192_pctl *pctl = gpiochip_get_data(chip);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->ctrl_regs[offset];
+ const u8 *input_muxvals = pctl->desc->functions[AXP192_FUNC_INPUT].muxvals;
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(pctl->regmap, reginfo->reg, &val);
+ if (ret)
+ return ret;
+
+ if ((val & reginfo->mask) == (input_muxvals[offset] << (ffs(reginfo->mask) - 1)))
+ return GPIO_LINE_DIRECTION_IN;
+ else
+ return GPIO_LINE_DIRECTION_OUT;
+}
+
+static void axp192_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+ struct axp192_pctl *pctl = gpiochip_get_data(chip);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->out_regs[offset];
+
+ regmap_update_bits(pctl->regmap, reginfo->reg, reginfo->mask, value ? reginfo->mask : 0);
+}
+
+static int axp192_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+ return pinctrl_gpio_direction_input(chip->base + offset);
+}
+
+static int axp192_gpio_direction_output(struct gpio_chip *chip, unsigned int offset, int value)
+{
+ chip->set(chip, offset, value);
+ return 0;
+}
+
+static int axp192_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+ struct axp192_pctl *pctl = gpiochip_get_data(chip);
+
+ return pctl->irqs[offset];
+}
+
+static int axp192_pinconf_get_pull_down(struct pinctrl_dev *pctldev, unsigned int pin)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->pull_down_regs[pin];
+ unsigned int val;
+ int ret;
+
+ if (!reginfo->mask)
+ return -EOPNOTSUPP;
+
+ ret = regmap_read(pctl->regmap, reginfo->reg, &val);
+ if (ret)
+ return ret;
+
+ return !!(val & reginfo->mask);
+}
+
+static int axp192_pinconf_set_pull_down(struct pinctrl_dev *pctldev, unsigned int pin, int value)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->pull_down_regs[pin];
+
+ if (!reginfo->mask)
+ return -EOPNOTSUPP;
+
+ return regmap_update_bits(pctl->regmap, reginfo->reg, reginfo->mask,
+ value ? reginfo->mask : 0);
+}
+
+static int axp192_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin, unsigned long *config)
+{
+ enum pin_config_param param = pinconf_to_config_param(*config);
+ unsigned int arg = 1;
+ int ret;
+
+ switch (param) {
+ case PIN_CONFIG_BIAS_DISABLE:
+ ret = axp192_pinconf_get_pull_down(pctldev, pin);
+ if (ret < 0)
+ return ret;
+ else if (ret != 0)
+ return -EINVAL;
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ ret = axp192_pinconf_get_pull_down(pctldev, pin);
+ if (ret < 0)
+ return ret;
+ else if (ret == 0)
+ return -EINVAL;
+ break;
+
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ *config = pinconf_to_config_packed(param, arg);
+ return 0;
+}
+
+static int axp192_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
+ unsigned long *configs, unsigned int num_configs)
+{
+ int ret;
+ unsigned int cfg;
+
+ for (cfg = 0; cfg < num_configs; ++cfg) {
+ switch (pinconf_to_config_param(configs[cfg])) {
+ case PIN_CONFIG_BIAS_DISABLE:
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ continue;
+ default:
+ return -EOPNOTSUPP;
+ }
+ }
+
+ for (cfg = 0; cfg < num_configs; ++cfg) {
+ switch (pinconf_to_config_param(configs[cfg])) {
+ case PIN_CONFIG_BIAS_DISABLE:
+ ret = axp192_pinconf_set_pull_down(pctldev, pin, 0);
+ if (ret)
+ return ret;
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ ret = axp192_pinconf_set_pull_down(pctldev, pin, 1);
+ if (ret)
+ return ret;
+ break;
+
+ default:
+ /* unreachable */
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static const struct pinconf_ops axp192_conf_ops = {
+ .is_generic = true,
+ .pin_config_get = axp192_pinconf_get,
+ .pin_config_set = axp192_pinconf_set,
+ .pin_config_group_get = axp192_pinconf_get,
+ .pin_config_group_set = axp192_pinconf_set,
+};
+
+static int axp192_pmx_set(struct pinctrl_dev *pctldev, unsigned int offset, u8 config)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->ctrl_regs[offset];
+ unsigned int regval = config << (ffs(reginfo->mask) - 1);
+
+ return regmap_update_bits(pctl->regmap, reginfo->reg, reginfo->mask, regval);
+}
+
+static int axp192_pmx_func_cnt(struct pinctrl_dev *pctldev)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->desc->nfunctions;
+}
+
+static const char *axp192_pmx_func_name(struct pinctrl_dev *pctldev, unsigned int selector)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->desc->functions[selector].name;
+}
+
+static int axp192_pmx_func_groups(struct pinctrl_dev *pctldev, unsigned int selector,
+ const char * const **groups, unsigned int *num_groups)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ *groups = pctl->desc->functions[selector].groups;
+ *num_groups = pctl->desc->functions[selector].ngroups;
+
+ return 0;
+}
+
+static int axp192_pmx_set_mux(struct pinctrl_dev *pctldev,
+ unsigned int function, unsigned int group)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const u8 *muxvals = pctl->desc->functions[function].muxvals;
+
+ if (muxvals[group] == (u8)-1)
+ return -EINVAL;
+
+ /*
+ * Switching to LDO or PWM function will enable LDO/PWM output, so it's
+ * better to ignore these requests and let the regulator or PWM drivers
+ * handle muxing to avoid interfering with them.
+ */
+ if (function == AXP192_FUNC_LDO || function == AXP192_FUNC_PWM)
+ return 0;
+
+ return axp192_pmx_set(pctldev, group, muxvals[group]);
+}
+
+static int axp192_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range,
+ unsigned int offset, bool input)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const u8 *muxvals = input ? pctl->desc->functions[AXP192_FUNC_INPUT].muxvals
+ : pctl->desc->functions[AXP192_FUNC_OUTPUT].muxvals;
+
+ return axp192_pmx_set(pctldev, offset, muxvals[offset]);
+}
+
+static const struct pinmux_ops axp192_pmx_ops = {
+ .get_functions_count = axp192_pmx_func_cnt,
+ .get_function_name = axp192_pmx_func_name,
+ .get_function_groups = axp192_pmx_func_groups,
+ .set_mux = axp192_pmx_set_mux,
+ .gpio_set_direction = axp192_pmx_gpio_set_direction,
+ .strict = true,
+};
+
+static int axp192_groups_cnt(struct pinctrl_dev *pctldev)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->desc->npins;
+}
+
+static const char *axp192_group_name(struct pinctrl_dev *pctldev, unsigned int selector)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->desc->pins[selector].name;
+}
+
+static int axp192_group_pins(struct pinctrl_dev *pctldev, unsigned int selector,
+ const unsigned int **pins, unsigned int *num_pins)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ *pins = &pctl->desc->pins[selector].number;
+ *num_pins = 1;
+
+ return 0;
+}
+
+static const struct pinctrl_ops axp192_pctrl_ops = {
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_group,
+ .dt_free_map = pinconf_generic_dt_free_map,
+ .get_groups_count = axp192_groups_cnt,
+ .get_group_name = axp192_group_name,
+ .get_group_pins = axp192_group_pins,
+};
+
+static int axp192_pctl_probe(struct platform_device *pdev)
+{
+ struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
+ struct axp192_pctl *pctl;
+ struct pinctrl_desc *pctrl_desc;
+ int ret, i;
+
+ if (!of_device_is_available(pdev->dev.of_node))
+ return -ENODEV;
+
+ if (!axp20x) {
+ dev_err(&pdev->dev, "Parent drvdata not set\n");
+ return -EINVAL;
+ }
+
+ pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
+ if (!pctl)
+ return -ENOMEM;
+
+ pctl->desc = of_device_get_match_data(&pdev->dev);
+ pctl->regmap = axp20x->regmap;
+ pctl->dev = &pdev->dev;
+
+ pctl->chip.base = -1;
+ pctl->chip.can_sleep = true;
+ pctl->chip.request = gpiochip_generic_request;
+ pctl->chip.free = gpiochip_generic_free;
+ pctl->chip.parent = &pdev->dev;
+ pctl->chip.label = dev_name(&pdev->dev);
+ pctl->chip.owner = THIS_MODULE;
+ pctl->chip.get = axp192_gpio_get;
+ pctl->chip.get_direction = axp192_gpio_get_direction;
+ pctl->chip.set = axp192_gpio_set;
+ pctl->chip.direction_input = axp192_gpio_direction_input;
+ pctl->chip.direction_output = axp192_gpio_direction_output;
+ pctl->chip.to_irq = axp192_gpio_to_irq;
+ pctl->chip.ngpio = pctl->desc->npins;
+
+ pctl->irqs = devm_kcalloc(&pdev->dev, pctl->desc->npins, sizeof(int), GFP_KERNEL);
+ if (!pctl->irqs)
+ return -ENOMEM;
+
+ for (i = 0; i < pctl->desc->npins; ++i) {
+ ret = platform_get_irq_byname_optional(pdev, pctl->desc->pins[i].name);
+ if (ret > 0)
+ pctl->irqs[i] = regmap_irq_get_virq(axp20x->regmap_irqc, ret);
+ }
+
+ platform_set_drvdata(pdev, pctl);
+
+ pctrl_desc = devm_kzalloc(&pdev->dev, sizeof(*pctrl_desc), GFP_KERNEL);
+ if (!pctrl_desc)
+ return -ENOMEM;
+
+ pctrl_desc->name = dev_name(&pdev->dev);
+ pctrl_desc->owner = THIS_MODULE;
+ pctrl_desc->pins = pctl->desc->pins;
+ pctrl_desc->npins = pctl->desc->npins;
+ pctrl_desc->pctlops = &axp192_pctrl_ops;
+ pctrl_desc->pmxops = &axp192_pmx_ops;
+ pctrl_desc->confops = &axp192_conf_ops;
+
+ pctl->pctl_dev = devm_pinctrl_register(&pdev->dev, pctrl_desc, pctl);
+ if (IS_ERR(pctl->pctl_dev)) {
+ dev_err(&pdev->dev, "couldn't register pinctrl driver\n");
+ return PTR_ERR(pctl->pctl_dev);
+ }
+
+ ret = devm_gpiochip_add_data(&pdev->dev, &pctl->chip, pctl);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register GPIO chip\n");
+ return ret;
+ }
+
+ ret = gpiochip_add_pin_range(&pctl->chip, dev_name(&pdev->dev),
+ pctl->desc->pins->number,
+ pctl->desc->pins->number,
+ pctl->desc->npins);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to add pin range\n");
+ return ret;
+ }
+
+ dev_info(&pdev->dev, "AXP192 pinctrl and GPIO driver loaded\n");
+
+ return 0;
+}
+
+static const struct of_device_id axp192_pctl_match[] = {
+ { .compatible = "x-powers,axp192-gpio", .data = &axp192_data, },
+ { }
+};
+MODULE_DEVICE_TABLE(of, axp192_pctl_match);
+
+static struct platform_driver axp192_pctl_driver = {
+ .probe = axp192_pctl_probe,
+ .driver = {
+ .name = "axp192-gpio",
+ .of_match_table = axp192_pctl_match,
+ },
+};
+
+module_platform_driver(axp192_pctl_driver);
+
+MODULE_AUTHOR("Aidan MacDonald <aidanmacdonald.0x0@gmail.com>");
+MODULE_DESCRIPTION("AXP192 PMIC pinctrl and GPIO driver");
+MODULE_LICENSE("GPL");
--
2.35.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 03/10] dt-bindings: iio: adc: axp209: Add AXP192 compatible
2022-06-03 13:57 ` [PATCH 03/10] dt-bindings: iio: adc: axp209: Add AXP192 compatible Aidan MacDonald
@ 2022-06-03 16:34 ` Jonathan Cameron
2022-06-04 11:33 ` Aidan MacDonald
2022-06-05 22:50 ` Rob Herring
1 sibling, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2022-06-03 16:34 UTC (permalink / raw)
To: Aidan MacDonald
Cc: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
lee.jones, sre, broonie, gregkh, lgirdwood, lars, rafael,
linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
On Fri, 3 Jun 2022 14:57:07 +0100
Aidan MacDonald <aidanmacdonald.0x0@gmail.com> wrote:
> The AXP192 is identical to the AXP20x, except for two additional
> GPIO ADC channels.
>
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
There is an argument that could be made here to say this should
have a fallback compatible to a suitable existing part as the driver
would work, we'd just be missing a couple of channels.
I don't feel strongly either way, but thought I'd raise the possibility.
I guess it's irrelevant if an updated kernel is needed anyway to have
it functional because of support needed for some other part of the chip
though.
Jonathan
> ---
> .../bindings/iio/adc/x-powers,axp209-adc.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml b/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
> index d6d3d8590171..1a68e650ac7d 100644
> --- a/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
> @@ -14,6 +14,23 @@ description: |
> Device is a child of an axp209 multifunction device
> ADC channels and their indexes per variant:
>
> + AXP192
> + ------
> + 0 | acin_v
> + 1 | acin_i
> + 2 | vbus_v
> + 3 | vbus_i
> + 4 | pmic_temp
> + 5 | gpio0_v
> + 6 | gpio1_v
> + 7 | gpio2_v
> + 8 | gpio3_v
> + 9 | ipsout_v
> + 10 | batt_v
> + 11 | batt_chrg_i
> + 12 | batt_dischrg_i
> + 13 | ts_v
> +
> AXP209
> ------
> 0 | acin_v
> @@ -50,6 +67,7 @@ description: |
> properties:
> compatible:
> oneOf:
> + - const: x-powers,axp192-adc
> - const: x-powers,axp209-adc
> - const: x-powers,axp221-adc
> - const: x-powers,axp813-adc
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 08/10] iio: adc: axp20x_adc: Add support for AXP192
2022-06-03 13:57 ` [PATCH 08/10] iio: adc: axp20x_adc: " Aidan MacDonald
@ 2022-06-03 16:47 ` Jonathan Cameron
2022-06-04 11:47 ` Aidan MacDonald
0 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2022-06-03 16:47 UTC (permalink / raw)
To: Aidan MacDonald
Cc: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
lee.jones, sre, broonie, gregkh, lgirdwood, lars, rafael,
linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
On Fri, 3 Jun 2022 14:57:12 +0100
Aidan MacDonald <aidanmacdonald.0x0@gmail.com> wrote:
> The AXP192 is identical to the AXP20x, except for the addition of
> two more GPIO ADC channels.
>
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
Hi Aidan,
A few minor questions and comments inline.
Thanks,
Jonathan
> ---
> drivers/iio/adc/axp20x_adc.c | 289 +++++++++++++++++++++++++++++++++--
> 1 file changed, 280 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index 53bf7d4899d2..7d2bf9529420 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -21,6 +21,9 @@
> #include <linux/iio/machine.h>
> #include <linux/mfd/axp20x.h>
>
> +#define AXP192_ADC_EN1_MASK GENMASK(7, 0)
> +#define AXP192_ADC_EN2_MASK (BIT(7) | GENMASK(3, 0))
Obviously doesn't really mater, but for consistency, maybe match the
ordering in AXP20X_ADC_EN2_MASK below?
> +
> #define AXP20X_ADC_EN1_MASK GENMASK(7, 0)
>
> #define AXP20X_ADC_EN2_MASK (GENMASK(3, 2) | BIT(7))
> @@ -31,6 +34,15 @@
> #define AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(x) ((x) & BIT(0))
> #define AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(x) (((x) & BIT(0)) << 1)
>
> +#define AXP192_GPIO30_IN_RANGE_GPIO0 BIT(0)
> +#define AXP192_GPIO30_IN_RANGE_GPIO1 BIT(1)
> +#define AXP192_GPIO30_IN_RANGE_GPIO2 BIT(2)
> +#define AXP192_GPIO30_IN_RANGE_GPIO3 BIT(3)
> +#define AXP192_GPIO30_IN_RANGE_GPIO0_VAL(x) ((x) & BIT(0))
> +#define AXP192_GPIO30_IN_RANGE_GPIO1_VAL(x) (((x) & BIT(0)) << 1)
> +#define AXP192_GPIO30_IN_RANGE_GPIO2_VAL(x) (((x) & BIT(0)) << 2)
> +#define AXP192_GPIO30_IN_RANGE_GPIO3_VAL(x) (((x) & BIT(0)) << 3)
FIELD_PREP() using the masks is cleaner than hand rolling it.
If you fancy updating the driver in general whilst here that would be great,
but I don't mind if not.
> +
> #define AXP20X_ADC_RATE_MASK GENMASK(7, 6)
> #define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4)
> #define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK)
> @@ -70,6 +82,25 @@ struct axp20x_adc_iio {
> const struct axp_data *data;
> };
>
> +enum axp192_adc_channel_v {
> + AXP192_ACIN_V = 0,
> + AXP192_VBUS_V,
> + AXP192_TS_IN,
> + AXP192_GPIO0_V,
> + AXP192_GPIO1_V,
> + AXP192_GPIO2_V,
> + AXP192_GPIO3_V,
> + AXP192_IPSOUT_V,
> + AXP192_BATT_V,
> +};
> +
> +enum axp192_adc_channel_i {
> + AXP192_ACIN_I = 0,
> + AXP192_VBUS_I,
> + AXP192_BATT_CHRG_I,
> + AXP192_BATT_DISCHRG_I,
> +};
> +
> enum axp20x_adc_channel_v {
> AXP20X_ACIN_V = 0,
> AXP20X_VBUS_V,
> @@ -157,6 +188,43 @@ static struct iio_map axp22x_maps[] = {
> * The only exception is for the battery. batt_v will be in_voltage6_raw and
> * charge current in_current6_raw and discharge current will be in_current7_raw.
> */
> +static const struct iio_chan_spec axp192_adc_channels[] = {
> + AXP20X_ADC_CHANNEL(AXP192_ACIN_V, "acin_v", IIO_VOLTAGE,
> + AXP20X_ACIN_V_ADC_H),
> + AXP20X_ADC_CHANNEL(AXP192_ACIN_I, "acin_i", IIO_CURRENT,
> + AXP20X_ACIN_I_ADC_H),
> + AXP20X_ADC_CHANNEL(AXP192_VBUS_V, "vbus_v", IIO_VOLTAGE,
> + AXP20X_VBUS_V_ADC_H),
> + AXP20X_ADC_CHANNEL(AXP192_VBUS_I, "vbus_i", IIO_CURRENT,
> + AXP20X_VBUS_I_ADC_H),
> + {
> + .type = IIO_TEMP,
> + .address = AXP20X_TEMP_ADC_H,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET),
> + .datasheet_name = "pmic_temp",
> + },
> + AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO0_V, "gpio0_v", IIO_VOLTAGE,
> + AXP20X_GPIO0_V_ADC_H),
> + AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO1_V, "gpio1_v", IIO_VOLTAGE,
> + AXP20X_GPIO1_V_ADC_H),
> + AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO2_V, "gpio2_v", IIO_VOLTAGE,
> + AXP192_GPIO2_V_ADC_H),
> + AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO3_V, "gpio3_v", IIO_VOLTAGE,
> + AXP192_GPIO3_V_ADC_H),
> + AXP20X_ADC_CHANNEL(AXP192_IPSOUT_V, "ipsout_v", IIO_VOLTAGE,
> + AXP20X_IPSOUT_V_HIGH_H),
> + AXP20X_ADC_CHANNEL(AXP192_BATT_V, "batt_v", IIO_VOLTAGE,
> + AXP20X_BATT_V_H),
> + AXP20X_ADC_CHANNEL(AXP192_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT,
> + AXP20X_BATT_CHRG_I_H),
> + AXP20X_ADC_CHANNEL(AXP192_BATT_DISCHRG_I, "batt_dischrg_i", IIO_CURRENT,
> + AXP20X_BATT_DISCHRG_I_H),
> + AXP20X_ADC_CHANNEL(AXP192_TS_IN, "ts_v", IIO_VOLTAGE,
> + AXP20X_TS_IN_H),
> +};
> +
> static const struct iio_chan_spec axp20x_adc_channels[] = {
> AXP20X_ADC_CHANNEL(AXP20X_ACIN_V, "acin_v", IIO_VOLTAGE,
> AXP20X_ACIN_V_ADC_H),
> @@ -277,6 +345,44 @@ static int axp813_adc_raw(struct iio_dev *indio_dev,
> return IIO_VAL_INT;
> }
>
> +static int axp192_adc_scale_voltage(int channel, int *val, int *val2)
> +{
> + switch (channel) {
> + case AXP192_ACIN_V:
> + case AXP192_VBUS_V:
> + *val = 1;
> + *val2 = 700000;
> + return IIO_VAL_INT_PLUS_MICRO;
> +
> + case AXP192_GPIO0_V:
> + case AXP192_GPIO1_V:
> + case AXP192_GPIO2_V:
> + case AXP192_GPIO3_V:
> + *val = 0;
> + *val2 = 500000;
> + return IIO_VAL_INT_PLUS_MICRO;
> +
> + case AXP192_BATT_V:
> + *val = 1;
> + *val2 = 100000;
> + return IIO_VAL_INT_PLUS_MICRO;
> +
> + case AXP192_IPSOUT_V:
> + *val = 1;
> + *val2 = 400000;
> + return IIO_VAL_INT_PLUS_MICRO;
> +
> + case AXP192_TS_IN:
> + /* 0.8 mV per LSB */
> + *val = 0;
> + *val2 = 800000;
> + return IIO_VAL_INT_PLUS_MICRO;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> static int axp20x_adc_scale_voltage(int channel, int *val, int *val2)
> {
> switch (channel) {
> @@ -380,6 +486,29 @@ static int axp20x_adc_scale_current(int channel, int *val, int *val2)
> }
> }
>
> +static int axp192_adc_scale(struct iio_chan_spec const *chan, int *val,
> + int *val2)
> +{
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + return axp192_adc_scale_voltage(chan->channel, val, val2);
> +
> + case IIO_CURRENT:
> + /*
> + * AXP192 current channels are identical to the AXP20x,
> + * therefore we can re-use the scaling function.
> + */
> + return axp20x_adc_scale_current(chan->channel, val, val2);
> +
> + case IIO_TEMP:
> + *val = 100;
> + return IIO_VAL_INT;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> static int axp20x_adc_scale(struct iio_chan_spec const *chan, int *val,
> int *val2)
> {
> @@ -439,6 +568,42 @@ static int axp813_adc_scale(struct iio_chan_spec const *chan, int *val,
> }
> }
>
> +static int axp192_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
> + int *val)
> +{
> + struct axp20x_adc_iio *info = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regmap_read(info->regmap, AXP192_GPIO30_IN_RANGE, val);
> + if (ret < 0)
> + return ret;
> +
> + switch (channel) {
> + case AXP192_GPIO0_V:
> + *val &= AXP192_GPIO30_IN_RANGE_GPIO0;
Obviously existing case does it this way, but I'd prefer a local variable
and FIELD_GET() for these. Reusing *val for this is confusing.
A precursor patch to tidy up earlier code would be great if you have
time.
> + break;
> +
> + case AXP192_GPIO1_V:
> + *val &= AXP192_GPIO30_IN_RANGE_GPIO1;
> + break;
> +
> + case AXP192_GPIO2_V:
> + *val &= AXP192_GPIO30_IN_RANGE_GPIO2;
> + break;
> +
> + case AXP192_GPIO3_V:
> + *val &= AXP192_GPIO30_IN_RANGE_GPIO3;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + *val = *val ? 700000 : 0;
> +
> + return IIO_VAL_INT;
> +}
> +
> static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
> int *val)
> {
> @@ -467,6 +632,22 @@ static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
> return IIO_VAL_INT;
> }
>
> +static int axp192_adc_offset(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val)
> +{
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + return axp192_adc_offset_voltage(indio_dev, chan->channel, val);
> +
> + case IIO_TEMP:
> + *val = -1447;
> + return IIO_VAL_INT;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> static int axp20x_adc_offset(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan, int *val)
> {
> @@ -483,6 +664,25 @@ static int axp20x_adc_offset(struct iio_dev *indio_dev,
> }
> }
>
> +static int axp192_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_OFFSET:
> + return axp192_adc_offset(indio_dev, chan, val);
> +
> + case IIO_CHAN_INFO_SCALE:
> + return axp192_adc_scale(chan, val, val2);
> +
> + case IIO_CHAN_INFO_RAW:
> + return axp20x_adc_raw(indio_dev, chan, val);
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> static int axp20x_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan, int *val,
> int *val2, long mask)
> @@ -543,6 +743,54 @@ static int axp813_read_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static int axp192_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val, int val2,
> + long mask)
> +{
> + struct axp20x_adc_iio *info = iio_priv(indio_dev);
> + unsigned int reg, regval;
> +
> + /*
> + * The AXP192 PMIC allows the user to choose between 0V and 0.7V offsets
> + * for (independently) GPIO0-3 when in ADC mode.
> + */
> + if (mask != IIO_CHAN_INFO_OFFSET)
> + return -EINVAL;
> +
> + if (val != 0 && val != 700000)
> + return -EINVAL;
> +
> + val = val ? 1 : 0;
Use a local variable for this. It's no longer val in any way so the
reuse is confusing.
> +
> + switch (chan->channel) {
> + case AXP192_GPIO0_V:
> + reg = AXP192_GPIO30_IN_RANGE_GPIO0;
> + regval = AXP192_GPIO30_IN_RANGE_GPIO0_VAL(val);
FIELD_PREP()
> + break;
> +
> + case AXP192_GPIO1_V:
> + reg = AXP192_GPIO30_IN_RANGE_GPIO1;
> + regval = AXP192_GPIO30_IN_RANGE_GPIO1_VAL(val);
> + break;
> +
> + case AXP192_GPIO2_V:
> + reg = AXP192_GPIO30_IN_RANGE_GPIO2;
> + regval = AXP192_GPIO30_IN_RANGE_GPIO2_VAL(val);
> + break;
> +
> + case AXP192_GPIO3_V:
> + reg = AXP192_GPIO30_IN_RANGE_GPIO3;
> + regval = AXP192_GPIO30_IN_RANGE_GPIO3_VAL(val);
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return regmap_update_bits(info->regmap, AXP192_GPIO30_IN_RANGE, reg,
> + regval);
> +}
> +
> static int axp20x_write_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan, int val, int val2,
> long mask)
> @@ -581,6 +829,18 @@ static int axp20x_write_raw(struct iio_dev *indio_dev,
> regval);
> }
>
> +static int axp192_read_label(struct iio_dev *iio_dev,
> + const struct iio_chan_spec *chan, char *label)
> +{
> + return snprintf(label, PAGE_SIZE, "%s\n", chan->datasheet_name);
> +}
Unless I missed a previous patch adding labels to the other devices supported,
this is the first driver to use these. Why do they make sense here but not
to add to existing supported devices?
I don't particularly mind this addition, just looking for an explanation.
> +
> +static const struct iio_info axp192_adc_iio_info = {
> + .read_raw = axp192_read_raw,
> + .write_raw = axp192_write_raw,
> + .read_label = axp192_read_label,
> +};
> +
> static const struct iio_info axp20x_adc_iio_info = {
> .read_raw = axp20x_read_raw,
> .write_raw = axp20x_write_raw,
> @@ -620,19 +880,29 @@ struct axp_data {
> int num_channels;
> struct iio_chan_spec const *channels;
> unsigned long adc_en1_mask;
> + unsigned long adc_en2_mask;
> int (*adc_rate)(struct axp20x_adc_iio *info,
> int rate);
> - bool adc_en2;
> struct iio_map *maps;
> };
>
> +static const struct axp_data axp192_data = {
> + .iio_info = &axp192_adc_iio_info,
> + .num_channels = ARRAY_SIZE(axp192_adc_channels),
> + .channels = axp192_adc_channels,
> + .adc_en1_mask = AXP192_ADC_EN1_MASK,
> + .adc_en2_mask = AXP192_ADC_EN2_MASK,
> + .adc_rate = axp20x_adc_rate,
> + .maps = axp20x_maps,
> +};
> +
> static const struct axp_data axp20x_data = {
> .iio_info = &axp20x_adc_iio_info,
> .num_channels = ARRAY_SIZE(axp20x_adc_channels),
> .channels = axp20x_adc_channels,
> .adc_en1_mask = AXP20X_ADC_EN1_MASK,
> + .adc_en2_mask = AXP20X_ADC_EN2_MASK,
> .adc_rate = axp20x_adc_rate,
> - .adc_en2 = true,
> .maps = axp20x_maps,
> };
>
> @@ -642,7 +912,6 @@ static const struct axp_data axp22x_data = {
> .channels = axp22x_adc_channels,
> .adc_en1_mask = AXP22X_ADC_EN1_MASK,
> .adc_rate = axp22x_adc_rate,
> - .adc_en2 = false,
> .maps = axp22x_maps,
> };
>
> @@ -652,11 +921,11 @@ static const struct axp_data axp813_data = {
> .channels = axp813_adc_channels,
> .adc_en1_mask = AXP22X_ADC_EN1_MASK,
> .adc_rate = axp813_adc_rate,
> - .adc_en2 = false,
> .maps = axp22x_maps,
> };
>
> static const struct of_device_id axp20x_adc_of_match[] = {
> + { .compatible = "x-powers,axp192-adc", .data = (void *)&axp192_data, },
> { .compatible = "x-powers,axp209-adc", .data = (void *)&axp20x_data, },
> { .compatible = "x-powers,axp221-adc", .data = (void *)&axp22x_data, },
> { .compatible = "x-powers,axp813-adc", .data = (void *)&axp813_data, },
> @@ -665,6 +934,7 @@ static const struct of_device_id axp20x_adc_of_match[] = {
> MODULE_DEVICE_TABLE(of, axp20x_adc_of_match);
>
> static const struct platform_device_id axp20x_adc_id_match[] = {
> + { .name = "axp192-adc", .driver_data = (kernel_ulong_t)&axp192_data, },
> { .name = "axp20x-adc", .driver_data = (kernel_ulong_t)&axp20x_data, },
> { .name = "axp22x-adc", .driver_data = (kernel_ulong_t)&axp22x_data, },
> { .name = "axp813-adc", .driver_data = (kernel_ulong_t)&axp813_data, },
> @@ -710,10 +980,11 @@ static int axp20x_probe(struct platform_device *pdev)
> /* Enable the ADCs on IP */
> regmap_write(info->regmap, AXP20X_ADC_EN1, info->data->adc_en1_mask);
>
> - if (info->data->adc_en2)
> - /* Enable GPIO0/1 and internal temperature ADCs */
> + if (info->data->adc_en2_mask)
> + /* Enable GPIO and internal temperature ADCs */
> regmap_update_bits(info->regmap, AXP20X_ADC_EN2,
> - AXP20X_ADC_EN2_MASK, AXP20X_ADC_EN2_MASK);
> + info->data->adc_en2_mask,
> + info->data->adc_en2_mask);
>
> /* Configure ADCs rate */
> info->data->adc_rate(info, 100);
> @@ -738,7 +1009,7 @@ static int axp20x_probe(struct platform_device *pdev)
> fail_map:
> regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
>
> - if (info->data->adc_en2)
> + if (info->data->adc_en2_mask)
> regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
>
> return ret;
> @@ -754,7 +1025,7 @@ static int axp20x_remove(struct platform_device *pdev)
>
> regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
>
> - if (info->data->adc_en2)
> + if (info->data->adc_en2_mask)
> regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
>
> return 0;
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 03/10] dt-bindings: iio: adc: axp209: Add AXP192 compatible
2022-06-03 16:34 ` Jonathan Cameron
@ 2022-06-04 11:33 ` Aidan MacDonald
0 siblings, 0 replies; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-04 11:33 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
lee.jones, sre, broonie, gregkh, lgirdwood, lars, rafael,
linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
Jonathan Cameron <jic23@kernel.org> writes:
> On Fri, 3 Jun 2022 14:57:07 +0100
> Aidan MacDonald <aidanmacdonald.0x0@gmail.com> wrote:
>
>> The AXP192 is identical to the AXP20x, except for two additional
>> GPIO ADC channels.
>>
>> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
>
> There is an argument that could be made here to say this should
> have a fallback compatible to a suitable existing part as the driver
> would work, we'd just be missing a couple of channels.
>
> I don't feel strongly either way, but thought I'd raise the possibility.
>
> I guess it's irrelevant if an updated kernel is needed anyway to have
> it functional because of support needed for some other part of the chip
> though.
>
> Jonathan
>
That may be possible, but you can't probe the IIO driver without having
an updated MFD driver so I'm not sure a fallback compatible is needed.
Regards,
Aidan
>> ---
>> .../bindings/iio/adc/x-powers,axp209-adc.yaml | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml b/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
>> index d6d3d8590171..1a68e650ac7d 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
>> +++ b/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
>> @@ -14,6 +14,23 @@ description: |
>> Device is a child of an axp209 multifunction device
>> ADC channels and their indexes per variant:
>>
>> + AXP192
>> + ------
>> + 0 | acin_v
>> + 1 | acin_i
>> + 2 | vbus_v
>> + 3 | vbus_i
>> + 4 | pmic_temp
>> + 5 | gpio0_v
>> + 6 | gpio1_v
>> + 7 | gpio2_v
>> + 8 | gpio3_v
>> + 9 | ipsout_v
>> + 10 | batt_v
>> + 11 | batt_chrg_i
>> + 12 | batt_dischrg_i
>> + 13 | ts_v
>> +
>> AXP209
>> ------
>> 0 | acin_v
>> @@ -50,6 +67,7 @@ description: |
>> properties:
>> compatible:
>> oneOf:
>> + - const: x-powers,axp192-adc
>> - const: x-powers,axp209-adc
>> - const: x-powers,axp221-adc
>> - const: x-powers,axp813-adc
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 08/10] iio: adc: axp20x_adc: Add support for AXP192
2022-06-03 16:47 ` Jonathan Cameron
@ 2022-06-04 11:47 ` Aidan MacDonald
2022-06-04 14:27 ` Jonathan Cameron
0 siblings, 1 reply; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-04 11:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
lee.jones, sre, broonie, gregkh, lgirdwood, lars, rafael,
linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
Jonathan Cameron <jic23@kernel.org> writes:
> On Fri, 3 Jun 2022 14:57:12 +0100
> Aidan MacDonald <aidanmacdonald.0x0@gmail.com> wrote:
>
>> The AXP192 is identical to the AXP20x, except for the addition of
>> two more GPIO ADC channels.
>>
>> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> Hi Aidan,
>
> A few minor questions and comments inline.
>
> Thanks,
>
> Jonathan
>
>> ---
>> drivers/iio/adc/axp20x_adc.c | 289 +++++++++++++++++++++++++++++++++--
>> 1 file changed, 280 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
>> index 53bf7d4899d2..7d2bf9529420 100644
>> --- a/drivers/iio/adc/axp20x_adc.c
>> +++ b/drivers/iio/adc/axp20x_adc.c
>> @@ -21,6 +21,9 @@
>> #include <linux/iio/machine.h>
>> #include <linux/mfd/axp20x.h>
>>
>> +#define AXP192_ADC_EN1_MASK GENMASK(7, 0)
>> +#define AXP192_ADC_EN2_MASK (BIT(7) | GENMASK(3, 0))
>
> Obviously doesn't really mater, but for consistency, maybe match the
> ordering in AXP20X_ADC_EN2_MASK below?
>
Ack, will do
>> +
>> #define AXP20X_ADC_EN1_MASK GENMASK(7, 0)
>>
>> #define AXP20X_ADC_EN2_MASK (GENMASK(3, 2) | BIT(7))
>> @@ -31,6 +34,15 @@
>> #define AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(x) ((x) & BIT(0))
>> #define AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(x) (((x) & BIT(0)) << 1)
>>
>> +#define AXP192_GPIO30_IN_RANGE_GPIO0 BIT(0)
>> +#define AXP192_GPIO30_IN_RANGE_GPIO1 BIT(1)
>> +#define AXP192_GPIO30_IN_RANGE_GPIO2 BIT(2)
>> +#define AXP192_GPIO30_IN_RANGE_GPIO3 BIT(3)
>> +#define AXP192_GPIO30_IN_RANGE_GPIO0_VAL(x) ((x) & BIT(0))
>> +#define AXP192_GPIO30_IN_RANGE_GPIO1_VAL(x) (((x) & BIT(0)) << 1)
>> +#define AXP192_GPIO30_IN_RANGE_GPIO2_VAL(x) (((x) & BIT(0)) << 2)
>> +#define AXP192_GPIO30_IN_RANGE_GPIO3_VAL(x) (((x) & BIT(0)) << 3)
>
> FIELD_PREP() using the masks is cleaner than hand rolling it.
> If you fancy updating the driver in general whilst here that would be great,
> but I don't mind if not.
>
Yeah, I don't mind, I'll include a cleanup patch in my v2.
>> +
>> #define AXP20X_ADC_RATE_MASK GENMASK(7, 6)
>> #define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4)
>> #define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK)
>> @@ -70,6 +82,25 @@ struct axp20x_adc_iio {
>> const struct axp_data *data;
>> };
>>
>> +enum axp192_adc_channel_v {
>> + AXP192_ACIN_V = 0,
>> + AXP192_VBUS_V,
>> + AXP192_TS_IN,
>> + AXP192_GPIO0_V,
>> + AXP192_GPIO1_V,
>> + AXP192_GPIO2_V,
>> + AXP192_GPIO3_V,
>> + AXP192_IPSOUT_V,
>> + AXP192_BATT_V,
>> +};
>> +
>> +enum axp192_adc_channel_i {
>> + AXP192_ACIN_I = 0,
>> + AXP192_VBUS_I,
>> + AXP192_BATT_CHRG_I,
>> + AXP192_BATT_DISCHRG_I,
>> +};
>> +
>> enum axp20x_adc_channel_v {
>> AXP20X_ACIN_V = 0,
>> AXP20X_VBUS_V,
>> @@ -157,6 +188,43 @@ static struct iio_map axp22x_maps[] = {
>> * The only exception is for the battery. batt_v will be in_voltage6_raw and
>> * charge current in_current6_raw and discharge current will be in_current7_raw.
>> */
>> +static const struct iio_chan_spec axp192_adc_channels[] = {
>> + AXP20X_ADC_CHANNEL(AXP192_ACIN_V, "acin_v", IIO_VOLTAGE,
>> + AXP20X_ACIN_V_ADC_H),
>> + AXP20X_ADC_CHANNEL(AXP192_ACIN_I, "acin_i", IIO_CURRENT,
>> + AXP20X_ACIN_I_ADC_H),
>> + AXP20X_ADC_CHANNEL(AXP192_VBUS_V, "vbus_v", IIO_VOLTAGE,
>> + AXP20X_VBUS_V_ADC_H),
>> + AXP20X_ADC_CHANNEL(AXP192_VBUS_I, "vbus_i", IIO_CURRENT,
>> + AXP20X_VBUS_I_ADC_H),
>> + {
>> + .type = IIO_TEMP,
>> + .address = AXP20X_TEMP_ADC_H,
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> + BIT(IIO_CHAN_INFO_SCALE) |
>> + BIT(IIO_CHAN_INFO_OFFSET),
>> + .datasheet_name = "pmic_temp",
>> + },
>> + AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO0_V, "gpio0_v", IIO_VOLTAGE,
>> + AXP20X_GPIO0_V_ADC_H),
>> + AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO1_V, "gpio1_v", IIO_VOLTAGE,
>> + AXP20X_GPIO1_V_ADC_H),
>> + AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO2_V, "gpio2_v", IIO_VOLTAGE,
>> + AXP192_GPIO2_V_ADC_H),
>> + AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO3_V, "gpio3_v", IIO_VOLTAGE,
>> + AXP192_GPIO3_V_ADC_H),
>> + AXP20X_ADC_CHANNEL(AXP192_IPSOUT_V, "ipsout_v", IIO_VOLTAGE,
>> + AXP20X_IPSOUT_V_HIGH_H),
>> + AXP20X_ADC_CHANNEL(AXP192_BATT_V, "batt_v", IIO_VOLTAGE,
>> + AXP20X_BATT_V_H),
>> + AXP20X_ADC_CHANNEL(AXP192_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT,
>> + AXP20X_BATT_CHRG_I_H),
>> + AXP20X_ADC_CHANNEL(AXP192_BATT_DISCHRG_I, "batt_dischrg_i", IIO_CURRENT,
>> + AXP20X_BATT_DISCHRG_I_H),
>> + AXP20X_ADC_CHANNEL(AXP192_TS_IN, "ts_v", IIO_VOLTAGE,
>> + AXP20X_TS_IN_H),
>> +};
>> +
>> static const struct iio_chan_spec axp20x_adc_channels[] = {
>> AXP20X_ADC_CHANNEL(AXP20X_ACIN_V, "acin_v", IIO_VOLTAGE,
>> AXP20X_ACIN_V_ADC_H),
>> @@ -277,6 +345,44 @@ static int axp813_adc_raw(struct iio_dev *indio_dev,
>> return IIO_VAL_INT;
>> }
>>
>> +static int axp192_adc_scale_voltage(int channel, int *val, int *val2)
>> +{
>> + switch (channel) {
>> + case AXP192_ACIN_V:
>> + case AXP192_VBUS_V:
>> + *val = 1;
>> + *val2 = 700000;
>> + return IIO_VAL_INT_PLUS_MICRO;
>> +
>> + case AXP192_GPIO0_V:
>> + case AXP192_GPIO1_V:
>> + case AXP192_GPIO2_V:
>> + case AXP192_GPIO3_V:
>> + *val = 0;
>> + *val2 = 500000;
>> + return IIO_VAL_INT_PLUS_MICRO;
>> +
>> + case AXP192_BATT_V:
>> + *val = 1;
>> + *val2 = 100000;
>> + return IIO_VAL_INT_PLUS_MICRO;
>> +
>> + case AXP192_IPSOUT_V:
>> + *val = 1;
>> + *val2 = 400000;
>> + return IIO_VAL_INT_PLUS_MICRO;
>> +
>> + case AXP192_TS_IN:
>> + /* 0.8 mV per LSB */
>> + *val = 0;
>> + *val2 = 800000;
>> + return IIO_VAL_INT_PLUS_MICRO;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> static int axp20x_adc_scale_voltage(int channel, int *val, int *val2)
>> {
>> switch (channel) {
>> @@ -380,6 +486,29 @@ static int axp20x_adc_scale_current(int channel, int *val, int *val2)
>> }
>> }
>>
>> +static int axp192_adc_scale(struct iio_chan_spec const *chan, int *val,
>> + int *val2)
>> +{
>> + switch (chan->type) {
>> + case IIO_VOLTAGE:
>> + return axp192_adc_scale_voltage(chan->channel, val, val2);
>> +
>> + case IIO_CURRENT:
>> + /*
>> + * AXP192 current channels are identical to the AXP20x,
>> + * therefore we can re-use the scaling function.
>> + */
>> + return axp20x_adc_scale_current(chan->channel, val, val2);
>> +
>> + case IIO_TEMP:
>> + *val = 100;
>> + return IIO_VAL_INT;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> static int axp20x_adc_scale(struct iio_chan_spec const *chan, int *val,
>> int *val2)
>> {
>> @@ -439,6 +568,42 @@ static int axp813_adc_scale(struct iio_chan_spec const *chan, int *val,
>> }
>> }
>>
>> +static int axp192_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
>> + int *val)
>> +{
>> + struct axp20x_adc_iio *info = iio_priv(indio_dev);
>> + int ret;
>> +
>> + ret = regmap_read(info->regmap, AXP192_GPIO30_IN_RANGE, val);
>> + if (ret < 0)
>> + return ret;
>> +
>> + switch (channel) {
>> + case AXP192_GPIO0_V:
>> + *val &= AXP192_GPIO30_IN_RANGE_GPIO0;
>
> Obviously existing case does it this way, but I'd prefer a local variable
> and FIELD_GET() for these. Reusing *val for this is confusing.
>
> A precursor patch to tidy up earlier code would be great if you have
> time.
>
Ack
>> + break;
>> +
>> + case AXP192_GPIO1_V:
>> + *val &= AXP192_GPIO30_IN_RANGE_GPIO1;
>> + break;
>> +
>> + case AXP192_GPIO2_V:
>> + *val &= AXP192_GPIO30_IN_RANGE_GPIO2;
>> + break;
>> +
>> + case AXP192_GPIO3_V:
>> + *val &= AXP192_GPIO30_IN_RANGE_GPIO3;
>> + break;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + *val = *val ? 700000 : 0;
>> +
>> + return IIO_VAL_INT;
>> +}
>> +
>> static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
>> int *val)
>> {
>> @@ -467,6 +632,22 @@ static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
>> return IIO_VAL_INT;
>> }
>>
>> +static int axp192_adc_offset(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan, int *val)
>> +{
>> + switch (chan->type) {
>> + case IIO_VOLTAGE:
>> + return axp192_adc_offset_voltage(indio_dev, chan->channel, val);
>> +
>> + case IIO_TEMP:
>> + *val = -1447;
>> + return IIO_VAL_INT;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> static int axp20x_adc_offset(struct iio_dev *indio_dev,
>> struct iio_chan_spec const *chan, int *val)
>> {
>> @@ -483,6 +664,25 @@ static int axp20x_adc_offset(struct iio_dev *indio_dev,
>> }
>> }
>>
>> +static int axp192_read_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan, int *val,
>> + int *val2, long mask)
>> +{
>> + switch (mask) {
>> + case IIO_CHAN_INFO_OFFSET:
>> + return axp192_adc_offset(indio_dev, chan, val);
>> +
>> + case IIO_CHAN_INFO_SCALE:
>> + return axp192_adc_scale(chan, val, val2);
>> +
>> + case IIO_CHAN_INFO_RAW:
>> + return axp20x_adc_raw(indio_dev, chan, val);
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> static int axp20x_read_raw(struct iio_dev *indio_dev,
>> struct iio_chan_spec const *chan, int *val,
>> int *val2, long mask)
>> @@ -543,6 +743,54 @@ static int axp813_read_raw(struct iio_dev *indio_dev,
>> }
>> }
>>
>> +static int axp192_write_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan, int val, int val2,
>> + long mask)
>> +{
>> + struct axp20x_adc_iio *info = iio_priv(indio_dev);
>> + unsigned int reg, regval;
>> +
>> + /*
>> + * The AXP192 PMIC allows the user to choose between 0V and 0.7V offsets
>> + * for (independently) GPIO0-3 when in ADC mode.
>> + */
>> + if (mask != IIO_CHAN_INFO_OFFSET)
>> + return -EINVAL;
>> +
>> + if (val != 0 && val != 700000)
>> + return -EINVAL;
>> +
>> + val = val ? 1 : 0;
>
> Use a local variable for this. It's no longer val in any way so the
> reuse is confusing.
>
Ack, I'll also clean up the existing code that I copied this from.
>> +
>> + switch (chan->channel) {
>> + case AXP192_GPIO0_V:
>> + reg = AXP192_GPIO30_IN_RANGE_GPIO0;
>> + regval = AXP192_GPIO30_IN_RANGE_GPIO0_VAL(val);
>
> FIELD_PREP()
>
Ack
>> + break;
>> +
>> + case AXP192_GPIO1_V:
>> + reg = AXP192_GPIO30_IN_RANGE_GPIO1;
>> + regval = AXP192_GPIO30_IN_RANGE_GPIO1_VAL(val);
>> + break;
>> +
>> + case AXP192_GPIO2_V:
>> + reg = AXP192_GPIO30_IN_RANGE_GPIO2;
>> + regval = AXP192_GPIO30_IN_RANGE_GPIO2_VAL(val);
>> + break;
>> +
>> + case AXP192_GPIO3_V:
>> + reg = AXP192_GPIO30_IN_RANGE_GPIO3;
>> + regval = AXP192_GPIO30_IN_RANGE_GPIO3_VAL(val);
>> + break;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return regmap_update_bits(info->regmap, AXP192_GPIO30_IN_RANGE, reg,
>> + regval);
>> +}
>> +
>> static int axp20x_write_raw(struct iio_dev *indio_dev,
>> struct iio_chan_spec const *chan, int val, int val2,
>> long mask)
>> @@ -581,6 +829,18 @@ static int axp20x_write_raw(struct iio_dev *indio_dev,
>> regval);
>> }
>>
>> +static int axp192_read_label(struct iio_dev *iio_dev,
>> + const struct iio_chan_spec *chan, char *label)
>> +{
>> + return snprintf(label, PAGE_SIZE, "%s\n", chan->datasheet_name);
>> +}
>
> Unless I missed a previous patch adding labels to the other devices supported,
> this is the first driver to use these. Why do they make sense here but not
> to add to existing supported devices?
>
> I don't particularly mind this addition, just looking for an explanation.
>
That'd be because 1d4ef9b39ebecca8 ("iio: core: Add optional symbolic
label to a device channel") added read_label in 2020, while the AXP
driver was introduced in 2017. I could add read_label for the other
chips while I'm here, for consistency.
One question I have is why read_label exists when the kernel already has
unique names for IIO channels. Why not just expose the datasheet_name to
userspace from the IIO core instead of making drivers do it?
>> +
>> +static const struct iio_info axp192_adc_iio_info = {
>> + .read_raw = axp192_read_raw,
>> + .write_raw = axp192_write_raw,
>> + .read_label = axp192_read_label,
>> +};
>> +
>> static const struct iio_info axp20x_adc_iio_info = {
>> .read_raw = axp20x_read_raw,
>> .write_raw = axp20x_write_raw,
>> @@ -620,19 +880,29 @@ struct axp_data {
>> int num_channels;
>> struct iio_chan_spec const *channels;
>> unsigned long adc_en1_mask;
>> + unsigned long adc_en2_mask;
>> int (*adc_rate)(struct axp20x_adc_iio *info,
>> int rate);
>> - bool adc_en2;
>> struct iio_map *maps;
>> };
>>
>> +static const struct axp_data axp192_data = {
>> + .iio_info = &axp192_adc_iio_info,
>> + .num_channels = ARRAY_SIZE(axp192_adc_channels),
>> + .channels = axp192_adc_channels,
>> + .adc_en1_mask = AXP192_ADC_EN1_MASK,
>> + .adc_en2_mask = AXP192_ADC_EN2_MASK,
>> + .adc_rate = axp20x_adc_rate,
>> + .maps = axp20x_maps,
>> +};
>> +
>> static const struct axp_data axp20x_data = {
>> .iio_info = &axp20x_adc_iio_info,
>> .num_channels = ARRAY_SIZE(axp20x_adc_channels),
>> .channels = axp20x_adc_channels,
>> .adc_en1_mask = AXP20X_ADC_EN1_MASK,
>> + .adc_en2_mask = AXP20X_ADC_EN2_MASK,
>> .adc_rate = axp20x_adc_rate,
>> - .adc_en2 = true,
>> .maps = axp20x_maps,
>> };
>>
>> @@ -642,7 +912,6 @@ static const struct axp_data axp22x_data = {
>> .channels = axp22x_adc_channels,
>> .adc_en1_mask = AXP22X_ADC_EN1_MASK,
>> .adc_rate = axp22x_adc_rate,
>> - .adc_en2 = false,
>> .maps = axp22x_maps,
>> };
>>
>> @@ -652,11 +921,11 @@ static const struct axp_data axp813_data = {
>> .channels = axp813_adc_channels,
>> .adc_en1_mask = AXP22X_ADC_EN1_MASK,
>> .adc_rate = axp813_adc_rate,
>> - .adc_en2 = false,
>> .maps = axp22x_maps,
>> };
>>
>> static const struct of_device_id axp20x_adc_of_match[] = {
>> + { .compatible = "x-powers,axp192-adc", .data = (void *)&axp192_data, },
>> { .compatible = "x-powers,axp209-adc", .data = (void *)&axp20x_data, },
>> { .compatible = "x-powers,axp221-adc", .data = (void *)&axp22x_data, },
>> { .compatible = "x-powers,axp813-adc", .data = (void *)&axp813_data, },
>> @@ -665,6 +934,7 @@ static const struct of_device_id axp20x_adc_of_match[] = {
>> MODULE_DEVICE_TABLE(of, axp20x_adc_of_match);
>>
>> static const struct platform_device_id axp20x_adc_id_match[] = {
>> + { .name = "axp192-adc", .driver_data = (kernel_ulong_t)&axp192_data, },
>> { .name = "axp20x-adc", .driver_data = (kernel_ulong_t)&axp20x_data, },
>> { .name = "axp22x-adc", .driver_data = (kernel_ulong_t)&axp22x_data, },
>> { .name = "axp813-adc", .driver_data = (kernel_ulong_t)&axp813_data, },
>> @@ -710,10 +980,11 @@ static int axp20x_probe(struct platform_device *pdev)
>> /* Enable the ADCs on IP */
>> regmap_write(info->regmap, AXP20X_ADC_EN1, info->data->adc_en1_mask);
>>
>> - if (info->data->adc_en2)
>> - /* Enable GPIO0/1 and internal temperature ADCs */
>> + if (info->data->adc_en2_mask)
>> + /* Enable GPIO and internal temperature ADCs */
>> regmap_update_bits(info->regmap, AXP20X_ADC_EN2,
>> - AXP20X_ADC_EN2_MASK, AXP20X_ADC_EN2_MASK);
>> + info->data->adc_en2_mask,
>> + info->data->adc_en2_mask);
>>
>> /* Configure ADCs rate */
>> info->data->adc_rate(info, 100);
>> @@ -738,7 +1009,7 @@ static int axp20x_probe(struct platform_device *pdev)
>> fail_map:
>> regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
>>
>> - if (info->data->adc_en2)
>> + if (info->data->adc_en2_mask)
>> regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
>>
>> return ret;
>> @@ -754,7 +1025,7 @@ static int axp20x_remove(struct platform_device *pdev)
>>
>> regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
>>
>> - if (info->data->adc_en2)
>> + if (info->data->adc_en2_mask)
>> regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
>>
>> return 0;
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 08/10] iio: adc: axp20x_adc: Add support for AXP192
2022-06-04 11:47 ` Aidan MacDonald
@ 2022-06-04 14:27 ` Jonathan Cameron
2022-06-07 10:49 ` Aidan MacDonald
0 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2022-06-04 14:27 UTC (permalink / raw)
To: Aidan MacDonald, lars
Cc: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
lee.jones, sre, broonie, gregkh, lgirdwood, rafael, linux-gpio,
devicetree, linux-iio, linux-pm, linux-kernel
On Sat, 04 Jun 2022 12:47:38 +0100
Aidan MacDonald <aidanmacdonald.0x0@gmail.com> wrote:
> Jonathan Cameron <jic23@kernel.org> writes:
>
> > On Fri, 3 Jun 2022 14:57:12 +0100
> > Aidan MacDonald <aidanmacdonald.0x0@gmail.com> wrote:
> >
> >> The AXP192 is identical to the AXP20x, except for the addition of
> >> two more GPIO ADC channels.
> >>
> >> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> > Hi Aidan,
> >
> > A few minor questions and comments inline.
> >
> > Thanks,
> >
> > Jonathan
> >
> > Unless I missed a previous patch adding labels to the other devices supported,
> > this is the first driver to use these. Why do they make sense here but not
> > to add to existing supported devices?
> >
> > I don't particularly mind this addition, just looking for an explanation.
> >
>
> That'd be because 1d4ef9b39ebecca8 ("iio: core: Add optional symbolic
> label to a device channel") added read_label in 2020, while the AXP
> driver was introduced in 2017. I could add read_label for the other
> chips while I'm here, for consistency.
Thanks, I don't really mind either way on adding support for additional parts.
>
> One question I have is why read_label exists when the kernel already has
> unique names for IIO channels. Why not just expose the datasheet_name to
> userspace from the IIO core instead of making drivers do it?
In general, datasheet_name refers to the name of the pin on a datasheet for this
device, whereas label can refer to how it is used.
There are dt bindings to allow a per channel label letting a driver (where it
makes sense) provide them for each individual ADC channel.
(e.g. the ad7768-1 driver does this).
On other devices they come from entirely different sources such as the hardcoded
choices in hid-sensor-custom-intel-hinge.
I vaguely recall that we've talked in the past about exposing datasheet name directly
but for many devices it's not that useful (the user doesn't care if a channel is
aux channel 1 or 7, but rather what it is wired up to).
At the moment this driver just exposes all channels rather than having
per channel bindings, so we don't have the option to use labeling in the device
tree to assign the names. If it's particularly useful to you to have labels
that are datasheet names that's fine.
Jonathan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 09/10] power: supply: axp20x_usb_power: Add support for AXP192
2022-06-03 13:57 ` [PATCH 09/10] power: supply: axp20x_usb_power: " Aidan MacDonald
@ 2022-06-05 15:13 ` kernel test robot
0 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2022-06-05 15:13 UTC (permalink / raw)
To: Aidan MacDonald, linus.walleij, brgl, robh+dt,
krzysztof.kozlowski+dt, wens, jic23, lee.jones, sre, broonie,
gregkh, lgirdwood
Cc: llvm, kbuild-all, lars, rafael, linux-gpio, devicetree,
linux-iio, linux-pm, linux-kernel
Hi Aidan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on lee-mfd/for-mfd-next]
[also build test WARNING on linusw-pinctrl/devel broonie-regmap/for-next jic23-iio/togreg sre-power-supply/for-next v5.18 next-20220603]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Aidan-MacDonald/Add-support-for-AXP192-PMIC/20220605-165501
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: riscv-randconfig-r023-20220605 (https://download.01.org/0day-ci/archive/20220605/202206052337.XAGi8JAq-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 416a5080d89066029f9889dc23f94de47c2fa895)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/1eaea00a34314bd851023b9feeea16d1219174a3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Aidan-MacDonald/Add-support-for-AXP192-PMIC/20220605-165501
git checkout 1eaea00a34314bd851023b9feeea16d1219174a3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/power/supply/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/power/supply/axp20x_usb_power.c:13:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/power/supply/axp20x_usb_power.c:13:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/power/supply/axp20x_usb_power.c:13:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
~~~~~~~~~~ ^
>> drivers/power/supply/axp20x_usb_power.c:300:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
default:
^
drivers/power/supply/axp20x_usb_power.c:300:3: note: insert 'break;' to avoid fall-through
default:
^
break;
8 warnings generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for RISCV_SBI_CPUIDLE
Depends on CPU_IDLE && RISCV && RISCV_SBI
Selected by
- SOC_VIRT && CPU_IDLE
vim +300 drivers/power/supply/axp20x_usb_power.c
198
199 static int axp20x_usb_power_get_property(struct power_supply *psy,
200 enum power_supply_property psp, union power_supply_propval *val)
201 {
202 struct axp20x_usb_power *power = power_supply_get_drvdata(psy);
203 unsigned int input, v, reg;
204 int ret;
205
206 switch (psp) {
207 case POWER_SUPPLY_PROP_VOLTAGE_MIN:
208 ret = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
209 if (ret)
210 return ret;
211
212 val->intval = AXP20X_VBUS_VHOLD_uV(v);
213 return 0;
214 case POWER_SUPPLY_PROP_VOLTAGE_NOW:
215 if (IS_ENABLED(CONFIG_AXP20X_ADC)) {
216 ret = iio_read_channel_processed(power->vbus_v,
217 &val->intval);
218 if (ret)
219 return ret;
220
221 /*
222 * IIO framework gives mV but Power Supply framework
223 * gives uV.
224 */
225 val->intval *= 1000;
226 return 0;
227 }
228
229 ret = axp20x_read_variable_width(power->regmap,
230 AXP20X_VBUS_V_ADC_H, 12);
231 if (ret < 0)
232 return ret;
233
234 val->intval = ret * 1700; /* 1 step = 1.7 mV */
235 return 0;
236 case POWER_SUPPLY_PROP_CURRENT_MAX:
237 if (power->axp20x_id == AXP813_ID)
238 return axp813_get_current_max(power, &val->intval);
239 else if (power->axp20x_id == AXP192_ID)
240 return axp192_get_current_max(power, &val->intval);
241 return axp20x_get_current_max(power, &val->intval);
242 case POWER_SUPPLY_PROP_CURRENT_NOW:
243 if (IS_ENABLED(CONFIG_AXP20X_ADC)) {
244 ret = iio_read_channel_processed(power->vbus_i,
245 &val->intval);
246 if (ret)
247 return ret;
248
249 /*
250 * IIO framework gives mA but Power Supply framework
251 * gives uA.
252 */
253 val->intval *= 1000;
254 return 0;
255 }
256
257 ret = axp20x_read_variable_width(power->regmap,
258 AXP20X_VBUS_I_ADC_H, 12);
259 if (ret < 0)
260 return ret;
261
262 val->intval = ret * 375; /* 1 step = 0.375 mA */
263 return 0;
264 default:
265 break;
266 }
267
268 /* All the properties below need the input-status reg value */
269 ret = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &input);
270 if (ret)
271 return ret;
272
273 switch (psp) {
274 case POWER_SUPPLY_PROP_HEALTH:
275 if (!(input & AXP20X_PWR_STATUS_VBUS_PRESENT)) {
276 val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
277 break;
278 }
279
280 val->intval = POWER_SUPPLY_HEALTH_GOOD;
281
282 switch (power->axp20x_id) {
283 case AXP192_ID:
284 /* Same layout as the AXP202, but different address */
285 reg = AXP192_USB_OTG_STATUS;
286 fallthrough;
287
288 case AXP202_ID:
289 if (power->axp20x_id == AXP202_ID)
290 reg = AXP20X_USB_OTG_STATUS;
291
292 ret = regmap_read(power->regmap, reg, &v);
293 if (ret)
294 return ret;
295
296 if (!(v & AXP20X_USB_STATUS_VBUS_VALID))
297 val->intval =
298 POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
299
> 300 default:
301 break;
302 }
303 break;
304 case POWER_SUPPLY_PROP_PRESENT:
305 val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_PRESENT);
306 break;
307 case POWER_SUPPLY_PROP_ONLINE:
308 val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_USED);
309 break;
310 default:
311 return -EINVAL;
312 }
313
314 return 0;
315 }
316
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 02/10] dt-bindings: mfd: add bindings for AXP192 MFD device
2022-06-03 13:57 ` [PATCH 02/10] dt-bindings: mfd: add bindings for AXP192 MFD device Aidan MacDonald
@ 2022-06-05 22:49 ` Rob Herring
2022-06-27 11:47 ` Lee Jones
1 sibling, 0 replies; 36+ messages in thread
From: Rob Herring @ 2022-06-05 22:49 UTC (permalink / raw)
To: Aidan MacDonald
Cc: gregkh, rafael, devicetree, linux-iio, krzysztof.kozlowski+dt,
linus.walleij, lee.jones, linux-gpio, linux-kernel, lgirdwood,
linux-pm, broonie, robh+dt, wens, sre, jic23, lars, brgl
On Fri, 03 Jun 2022 14:57:06 +0100, Aidan MacDonald wrote:
> The AXP192 is another X-Powers PMIC similar to the existing ones.
>
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> ---
> Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 03/10] dt-bindings: iio: adc: axp209: Add AXP192 compatible
2022-06-03 13:57 ` [PATCH 03/10] dt-bindings: iio: adc: axp209: Add AXP192 compatible Aidan MacDonald
2022-06-03 16:34 ` Jonathan Cameron
@ 2022-06-05 22:50 ` Rob Herring
1 sibling, 0 replies; 36+ messages in thread
From: Rob Herring @ 2022-06-05 22:50 UTC (permalink / raw)
To: Aidan MacDonald
Cc: lars, linux-kernel, linus.walleij, linux-iio, linux-pm, sre,
linux-gpio, devicetree, gregkh, broonie, wens, rafael,
krzysztof.kozlowski+dt, jic23, brgl, lee.jones, robh+dt,
lgirdwood
On Fri, 03 Jun 2022 14:57:07 +0100, Aidan MacDonald wrote:
> The AXP192 is identical to the AXP20x, except for two additional
> GPIO ADC channels.
>
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> ---
> .../bindings/iio/adc/x-powers,axp209-adc.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 04/10] dt-bindings: power: supply: axp20x: Add AXP192 compatible
2022-06-03 13:57 ` [PATCH 04/10] dt-bindings: power: supply: axp20x: " Aidan MacDonald
@ 2022-06-05 22:50 ` Rob Herring
0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2022-06-05 22:50 UTC (permalink / raw)
To: Aidan MacDonald
Cc: linux-pm, wens, brgl, broonie, gregkh, linux-gpio, sre, robh+dt,
krzysztof.kozlowski+dt, rafael, lars, linus.walleij, devicetree,
lgirdwood, linux-iio, linux-kernel, jic23, lee.jones
On Fri, 03 Jun 2022 14:57:08 +0100, Aidan MacDonald wrote:
> The AXP192's USB power supply is similar to the AXP202 but it has
> different USB current limits.
>
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> ---
> .../bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 05/10] dt-bindings: gpio: Add AXP192 GPIO bindings
2022-06-03 13:57 ` [PATCH 05/10] dt-bindings: gpio: Add AXP192 GPIO bindings Aidan MacDonald
@ 2022-06-05 22:55 ` Rob Herring
2022-06-07 10:34 ` Aidan MacDonald
0 siblings, 1 reply; 36+ messages in thread
From: Rob Herring @ 2022-06-05 22:55 UTC (permalink / raw)
To: Aidan MacDonald
Cc: linus.walleij, brgl, krzysztof.kozlowski+dt, wens, jic23,
lee.jones, sre, broonie, gregkh, lgirdwood, lars, rafael,
linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
On Fri, Jun 03, 2022 at 02:57:09PM +0100, Aidan MacDonald wrote:
> The AXP192 PMIC is different enough from the PMICs supported by
> the AXP20x GPIO driver to warrant a separate driver. The AXP192
> driver also supports interrupts and pinconf settings.
>
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> ---
> .../bindings/gpio/x-powers,axp192-gpio.yaml | 59 +++++++++++++++++++
> 1 file changed, 59 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
>
> diff --git a/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
> new file mode 100644
> index 000000000000..7a985640ade8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/gpio/x-powers,axp192-gpio.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: X-Powers AXP192 GPIO Device Tree Bindings
> +
> +maintainers:
> + - Chen-Yu Tsai <wens@csie.org>
> +
> +properties:
> + "#gpio-cells":
> + const: 2
> + description: >
> + The first cell is the pin number and the second is the GPIO flags.
> +
> + compatible:
> + oneOf:
> + - enum:
No need for 'oneOf' with only 1 entry.
> + - x-powers,axp192-gpio
> +
> + gpio-controller: true
> +
> +patternProperties:
> + "^.*-pins?$":
You can omit '^.*'
Why does 's' need to be optional?
> + $ref: /schemas/pinctrl/pinmux-node.yaml#
> +
> + properties:
> + pins:
> + items:
> + enum:
> + - GPIO0
> + - GPIO1
> + - GPIO2
> + - GPIO3
> + - GPIO4
> + - N_RSTO
> +
> + function:
> + enum:
> + - output
> + - input
> + - ldo
> + - pwm
> + - adc
> + - low_output
> + - floating
> + - ext_chg_ctl
> + - ldo_status
> +
> +required:
> + - compatible
> + - "#gpio-cells"
> + - gpio-controller
> +
> +additionalProperties: false
> +
> +...
> --
> 2.35.1
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 07/10] regulator: axp20x: Add support for AXP192
2022-06-03 13:57 ` [PATCH 07/10] regulator: " Aidan MacDonald
@ 2022-06-06 14:36 ` Mark Brown
0 siblings, 0 replies; 36+ messages in thread
From: Mark Brown @ 2022-06-06 14:36 UTC (permalink / raw)
To: Aidan MacDonald
Cc: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, gregkh, lgirdwood, lars, rafael,
linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 143 bytes --]
On Fri, Jun 03, 2022 at 02:57:11PM +0100, Aidan MacDonald wrote:
> Add support for the AXP192 PMIC.
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 01/10] regmap-irq: Add get_irq_reg to support unusual register layouts
2022-06-03 13:57 ` [PATCH 01/10] regmap-irq: Add get_irq_reg to support unusual register layouts Aidan MacDonald
@ 2022-06-06 17:43 ` Guru Das Srinagesh
2022-06-07 10:46 ` Aidan MacDonald
0 siblings, 1 reply; 36+ messages in thread
From: Guru Das Srinagesh @ 2022-06-06 17:43 UTC (permalink / raw)
To: Aidan MacDonald
Cc: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood, lars, rafael,
linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
On Fri, Jun 03, 2022 at 02:57:05PM +0100, Aidan MacDonald wrote:
> Add a new callback, get_irq_reg, for regmap IRQ chips, to support devices
> with unusual register layouts. This is required in the rare cases where
> the offset of an IRQ register is not constant with respect to the base
> register. This is probably best illustrated with an example:
>
> mask status
> IRQ0 0x40 0x44
> IRQ1 0x41 0x45
> IRQ2 0x42 0x46
> IRQ3 0x43 0x47
> IRQ4 0x4a 0x4d
>
> If we set mask_base = 0x40 and status_base = 0x44, the offsets of each
> register relative to the base are:
>
> mask status
> IRQ0 0 0
> IRQ1 1 1
> IRQ2 2 2
> IRQ3 3 3
> IRQ4 10 9
>
> The existing mapping mechanisms can't include IRQ4 in the same irqchip
> as IRQ0-3 because the offset of IRQ4's register depends on which type
> of register we're asking for, ie. which base register is used.
>
> The get_irq_reg callback allows drivers to specify an arbitrary mapping
> of (base register, register index) pairs to register addresses, instead
> of the default linear mapping "base_register + register_index". This
> allows unusual layouts, like the one above, to be handled using a single
> regmap IRQ chip.
>
> The drawback is that when get_irq_reg is used, it's impossible to use
> bulk reads for status registers even if some of them are contiguous,
> because the mapping is opaque to regmap-irq. This should be acceptable
> for the case of a few infrequently-polled status registers.
This patch does two things:
1. Add a new callback `get_irq_reg`
2. Replace unmask_offset calculation with call to sub_irq_reg()
Could you please split the patch into two to better reflect this?
Thank you.
Guru Das.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 05/10] dt-bindings: gpio: Add AXP192 GPIO bindings
2022-06-05 22:55 ` Rob Herring
@ 2022-06-07 10:34 ` Aidan MacDonald
2022-06-07 15:17 ` Rob Herring
0 siblings, 1 reply; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-07 10:34 UTC (permalink / raw)
To: Rob Herring
Cc: linus.walleij, brgl, krzysztof.kozlowski+dt, wens, jic23,
lee.jones, sre, broonie, gregkh, lgirdwood, lars, rafael,
linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
Rob Herring <robh@kernel.org> writes:
> On Fri, Jun 03, 2022 at 02:57:09PM +0100, Aidan MacDonald wrote:
>> The AXP192 PMIC is different enough from the PMICs supported by
>> the AXP20x GPIO driver to warrant a separate driver. The AXP192
>> driver also supports interrupts and pinconf settings.
>>
>> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
>> ---
>> .../bindings/gpio/x-powers,axp192-gpio.yaml | 59 +++++++++++++++++++
>> 1 file changed, 59 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
>> new file mode 100644
>> index 000000000000..7a985640ade8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
>> @@ -0,0 +1,59 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/gpio/x-powers,axp192-gpio.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: X-Powers AXP192 GPIO Device Tree Bindings
>> +
>> +maintainers:
>> + - Chen-Yu Tsai <wens@csie.org>
>> +
>> +properties:
>> + "#gpio-cells":
>> + const: 2
>> + description: >
>> + The first cell is the pin number and the second is the GPIO flags.
>> +
>> + compatible:
>> + oneOf:
>> + - enum:
>
> No need for 'oneOf' with only 1 entry.
>
Got it.
>> + - x-powers,axp192-gpio
>> +
>> + gpio-controller: true
>> +
>> +patternProperties:
>> + "^.*-pins?$":
>
> You can omit '^.*'
>
> Why does 's' need to be optional?
>
TBH I just copied this from x-powers,axp209-gpio.yaml. A similar pattern
is used in a few other bindings, eg. allwinner,sun4i-a10-pinctrl.yaml.
I guess it's to allow the node names to sound more natural when there's
only one pin.
I am going to send a v2 with '-pins?$' but if you would prefer to have
'-pins$' that's fine. I don't mind either way.
Regards,
Aidan
>> + $ref: /schemas/pinctrl/pinmux-node.yaml#
>> +
>> + properties:
>> + pins:
>> + items:
>> + enum:
>> + - GPIO0
>> + - GPIO1
>> + - GPIO2
>> + - GPIO3
>> + - GPIO4
>> + - N_RSTO
>> +
>> + function:
>> + enum:
>> + - output
>> + - input
>> + - ldo
>> + - pwm
>> + - adc
>> + - low_output
>> + - floating
>> + - ext_chg_ctl
>> + - ldo_status
>> +
>> +required:
>> + - compatible
>> + - "#gpio-cells"
>> + - gpio-controller
>> +
>> +additionalProperties: false
>> +
>> +...
>> --
>> 2.35.1
>>
>>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 01/10] regmap-irq: Add get_irq_reg to support unusual register layouts
2022-06-06 17:43 ` Guru Das Srinagesh
@ 2022-06-07 10:46 ` Aidan MacDonald
0 siblings, 0 replies; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-07 10:46 UTC (permalink / raw)
To: Guru Das Srinagesh
Cc: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, lee.jones, sre, broonie, gregkh, lgirdwood, lars, rafael,
linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
Guru Das Srinagesh <quic_gurus@quicinc.com> writes:
> On Fri, Jun 03, 2022 at 02:57:05PM +0100, Aidan MacDonald wrote:
>> Add a new callback, get_irq_reg, for regmap IRQ chips, to support devices
>> with unusual register layouts. This is required in the rare cases where
>> the offset of an IRQ register is not constant with respect to the base
>> register. This is probably best illustrated with an example:
>>
>> mask status
>> IRQ0 0x40 0x44
>> IRQ1 0x41 0x45
>> IRQ2 0x42 0x46
>> IRQ3 0x43 0x47
>> IRQ4 0x4a 0x4d
>>
>> If we set mask_base = 0x40 and status_base = 0x44, the offsets of each
>> register relative to the base are:
>>
>> mask status
>> IRQ0 0 0
>> IRQ1 1 1
>> IRQ2 2 2
>> IRQ3 3 3
>> IRQ4 10 9
>>
>> The existing mapping mechanisms can't include IRQ4 in the same irqchip
>> as IRQ0-3 because the offset of IRQ4's register depends on which type
>> of register we're asking for, ie. which base register is used.
>>
>> The get_irq_reg callback allows drivers to specify an arbitrary mapping
>> of (base register, register index) pairs to register addresses, instead
>> of the default linear mapping "base_register + register_index". This
>> allows unusual layouts, like the one above, to be handled using a single
>> regmap IRQ chip.
>>
>> The drawback is that when get_irq_reg is used, it's impossible to use
>> bulk reads for status registers even if some of them are contiguous,
>> because the mapping is opaque to regmap-irq. This should be acceptable
>> for the case of a few infrequently-polled status registers.
>
> This patch does two things:
>
> 1. Add a new callback `get_irq_reg`
> 2. Replace unmask_offset calculation with call to sub_irq_reg()
>
> Could you please split the patch into two to better reflect this?
>
> Thank you.
>
> Guru Das.
No problem, I'll do that in my v2.
Regards,
Aidan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 08/10] iio: adc: axp20x_adc: Add support for AXP192
2022-06-04 14:27 ` Jonathan Cameron
@ 2022-06-07 10:49 ` Aidan MacDonald
0 siblings, 0 replies; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-07 10:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: lars, linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
lee.jones, sre, broonie, gregkh, lgirdwood, rafael, linux-gpio,
devicetree, linux-iio, linux-pm, linux-kernel
Jonathan Cameron <jic23@kernel.org> writes:
> On Sat, 04 Jun 2022 12:47:38 +0100
> Aidan MacDonald <aidanmacdonald.0x0@gmail.com> wrote:
>
>> Jonathan Cameron <jic23@kernel.org> writes:
>>
>> > On Fri, 3 Jun 2022 14:57:12 +0100
>> > Aidan MacDonald <aidanmacdonald.0x0@gmail.com> wrote:
>> >
>> >> The AXP192 is identical to the AXP20x, except for the addition of
>> >> two more GPIO ADC channels.
>> >>
>> >> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
>> > Hi Aidan,
>> >
>> > A few minor questions and comments inline.
>> >
>> > Thanks,
>> >
>> > Jonathan
>> >
>> > Unless I missed a previous patch adding labels to the other devices supported,
>> > this is the first driver to use these. Why do they make sense here but not
>> > to add to existing supported devices?
>> >
>> > I don't particularly mind this addition, just looking for an explanation.
>> >
>>
>> That'd be because 1d4ef9b39ebecca8 ("iio: core: Add optional symbolic
>> label to a device channel") added read_label in 2020, while the AXP
>> driver was introduced in 2017. I could add read_label for the other
>> chips while I'm here, for consistency.
>
> Thanks, I don't really mind either way on adding support for additional parts.
>
>>
>> One question I have is why read_label exists when the kernel already has
>> unique names for IIO channels. Why not just expose the datasheet_name to
>> userspace from the IIO core instead of making drivers do it?
>
> In general, datasheet_name refers to the name of the pin on a datasheet for this
> device, whereas label can refer to how it is used.
> There are dt bindings to allow a per channel label letting a driver (where it
> makes sense) provide them for each individual ADC channel.
> (e.g. the ad7768-1 driver does this).
>
> On other devices they come from entirely different sources such as the hardcoded
> choices in hid-sensor-custom-intel-hinge.
>
> I vaguely recall that we've talked in the past about exposing datasheet name directly
> but for many devices it's not that useful (the user doesn't care if a channel is
> aux channel 1 or 7, but rather what it is wired up to).
>
> At the moment this driver just exposes all channels rather than having
> per channel bindings, so we don't have the option to use labeling in the device
> tree to assign the names. If it's particularly useful to you to have labels
> that are datasheet names that's fine.
>
> Jonathan
Thanks for the explanation, makes sense that "gpio0_v" is probably not
all that useful. I was thinking mainly of channels like battery charge
current where the channel usage is fixed by hardware. The labels aren't
terribly useful to me so I'll just leave them out, I'd rather not bother
with adding dt labels.
Regards,
Aidan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 05/10] dt-bindings: gpio: Add AXP192 GPIO bindings
2022-06-07 10:34 ` Aidan MacDonald
@ 2022-06-07 15:17 ` Rob Herring
2022-06-07 15:40 ` Aidan MacDonald
0 siblings, 1 reply; 36+ messages in thread
From: Rob Herring @ 2022-06-07 15:17 UTC (permalink / raw)
To: Aidan MacDonald
Cc: linus.walleij, brgl, krzysztof.kozlowski+dt, wens, jic23,
lee.jones, sre, broonie, gregkh, lgirdwood, lars, rafael,
linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
On Tue, Jun 07, 2022 at 11:34:19AM +0100, Aidan MacDonald wrote:
>
> Rob Herring <robh@kernel.org> writes:
>
> > On Fri, Jun 03, 2022 at 02:57:09PM +0100, Aidan MacDonald wrote:
> >> The AXP192 PMIC is different enough from the PMICs supported by
> >> the AXP20x GPIO driver to warrant a separate driver. The AXP192
> >> driver also supports interrupts and pinconf settings.
> >>
> >> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> >> ---
> >> .../bindings/gpio/x-powers,axp192-gpio.yaml | 59 +++++++++++++++++++
> >> 1 file changed, 59 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
> >> new file mode 100644
> >> index 000000000000..7a985640ade8
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
> >> @@ -0,0 +1,59 @@
> >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> >> +%YAML 1.2
> >> +---
> >> +$id: "http://devicetree.org/schemas/gpio/x-powers,axp192-gpio.yaml#"
> >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> >> +
> >> +title: X-Powers AXP192 GPIO Device Tree Bindings
> >> +
> >> +maintainers:
> >> + - Chen-Yu Tsai <wens@csie.org>
> >> +
> >> +properties:
> >> + "#gpio-cells":
> >> + const: 2
> >> + description: >
> >> + The first cell is the pin number and the second is the GPIO flags.
> >> +
> >> + compatible:
> >> + oneOf:
> >> + - enum:
> >
> > No need for 'oneOf' with only 1 entry.
> >
>
> Got it.
>
> >> + - x-powers,axp192-gpio
> >> +
> >> + gpio-controller: true
> >> +
> >> +patternProperties:
> >> + "^.*-pins?$":
> >
> > You can omit '^.*'
> >
> > Why does 's' need to be optional?
> >
>
> TBH I just copied this from x-powers,axp209-gpio.yaml. A similar pattern
> is used in a few other bindings, eg. allwinner,sun4i-a10-pinctrl.yaml.
> I guess it's to allow the node names to sound more natural when there's
> only one pin.
Those cases were for compatibility with existing users. That shouldn't
be the case here.
Rob
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 05/10] dt-bindings: gpio: Add AXP192 GPIO bindings
2022-06-07 15:17 ` Rob Herring
@ 2022-06-07 15:40 ` Aidan MacDonald
0 siblings, 0 replies; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-07 15:40 UTC (permalink / raw)
To: Rob Herring
Cc: linus.walleij, brgl, krzysztof.kozlowski+dt, wens, jic23,
lee.jones, sre, broonie, gregkh, lgirdwood, lars, rafael,
linux-gpio, devicetree, linux-iio, linux-pm, linux-kernel
Rob Herring <robh@kernel.org> writes:
> On Tue, Jun 07, 2022 at 11:34:19AM +0100, Aidan MacDonald wrote:
>>
>> Rob Herring <robh@kernel.org> writes:
>>
>> > On Fri, Jun 03, 2022 at 02:57:09PM +0100, Aidan MacDonald wrote:
>> >> The AXP192 PMIC is different enough from the PMICs supported by
>> >> the AXP20x GPIO driver to warrant a separate driver. The AXP192
>> >> driver also supports interrupts and pinconf settings.
>> >>
>> >> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
>> >> ---
>> >> .../bindings/gpio/x-powers,axp192-gpio.yaml | 59 +++++++++++++++++++
>> >> 1 file changed, 59 insertions(+)
>> >> create mode 100644 Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
>> >>
>> >> diff --git
>> >> a/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
>> >> b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
>> >> new file mode 100644
>> >> index 000000000000..7a985640ade8
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
>> >> @@ -0,0 +1,59 @@
>> >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> >> +%YAML 1.2
>> >> +---
>> >> +$id: "http://devicetree.org/schemas/gpio/x-powers,axp192-gpio.yaml#"
>> >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> >> +
>> >> +title: X-Powers AXP192 GPIO Device Tree Bindings
>> >> +
>> >> +maintainers:
>> >> + - Chen-Yu Tsai <wens@csie.org>
>> >> +
>> >> +properties:
>> >> + "#gpio-cells":
>> >> + const: 2
>> >> + description: >
>> >> + The first cell is the pin number and the second is the GPIO flags.
>> >> +
>> >> + compatible:
>> >> + oneOf:
>> >> + - enum:
>> >
>> > No need for 'oneOf' with only 1 entry.
>> >
>>
>> Got it.
>>
>> >> + - x-powers,axp192-gpio
>> >> +
>> >> + gpio-controller: true
>> >> +
>> >> +patternProperties:
>> >> + "^.*-pins?$":
>> >
>> > You can omit '^.*'
>> >
>> > Why does 's' need to be optional?
>> >
>>
>> TBH I just copied this from x-powers,axp209-gpio.yaml. A similar pattern
>> is used in a few other bindings, eg. allwinner,sun4i-a10-pinctrl.yaml.
>> I guess it's to allow the node names to sound more natural when there's
>> only one pin.
>
> Those cases were for compatibility with existing users. That shouldn't
> be the case here.
>
> Rob
OK, thanks for the clarification. I will use '-pins$' then.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 10/10] pinctrl: Add AXP192 pin control driver
2022-06-03 13:57 ` [PATCH 10/10] pinctrl: Add AXP192 pin control driver Aidan MacDonald
@ 2022-06-15 13:44 ` Linus Walleij
2022-06-15 18:06 ` Michael Walle
2022-06-15 14:51 ` Andy Shevchenko
1 sibling, 1 reply; 36+ messages in thread
From: Linus Walleij @ 2022-06-15 13:44 UTC (permalink / raw)
To: Aidan MacDonald, Michael Walle
Cc: brgl, robh+dt, krzysztof.kozlowski+dt, wens, jic23, lee.jones,
sre, broonie, gregkh, lgirdwood, lars, rafael, linux-gpio,
devicetree, linux-iio, linux-pm, linux-kernel
On Fri, Jun 3, 2022 at 3:56 PM Aidan MacDonald
<aidanmacdonald.0x0@gmail.com> wrote:
> The AXP192 PMIC's GPIO registers are much different from the GPIO
> registers of the AXP20x and AXP813 PMICs supported by the existing
> pinctrl-axp209 driver. It makes more sense to add a new driver for
> the AXP192, rather than add support in the existing axp20x driver.
>
> The pinctrl-axp192 driver is considerably more flexible in terms of
> register layout and should be able to support other X-Powers PMICs.
> Interrupts and pull down resistor configuration are supported too.
>
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
Looks good to me (TM) but I'd like Michael Walle to take a look
to check if this is one of those drivers that could make use of
gpio-regmap.c CONFIG_GPIO_REGMAP to make it even
simpler.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 10/10] pinctrl: Add AXP192 pin control driver
2022-06-03 13:57 ` [PATCH 10/10] pinctrl: Add AXP192 pin control driver Aidan MacDonald
2022-06-15 13:44 ` Linus Walleij
@ 2022-06-15 14:51 ` Andy Shevchenko
2022-06-17 12:15 ` Aidan MacDonald
1 sibling, 1 reply; 36+ messages in thread
From: Andy Shevchenko @ 2022-06-15 14:51 UTC (permalink / raw)
To: Aidan MacDonald
Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Chen-Yu Tsai, Jonathan Cameron, Lee Jones,
Sebastian Reichel, Mark Brown, Greg Kroah-Hartman, Liam Girdwood,
Lars-Peter Clausen, Rafael J. Wysocki, open list:GPIO SUBSYSTEM,
devicetree, linux-iio, Linux PM, Linux Kernel Mailing List
On Fri, Jun 3, 2022 at 6:29 PM Aidan MacDonald
<aidanmacdonald.0x0@gmail.com> wrote:
>
> The AXP192 PMIC's GPIO registers are much different from the GPIO
> registers of the AXP20x and AXP813 PMICs supported by the existing
> pinctrl-axp209 driver. It makes more sense to add a new driver for
> the AXP192, rather than add support in the existing axp20x driver.
>
> The pinctrl-axp192 driver is considerably more flexible in terms of
> register layout and should be able to support other X-Powers PMICs.
> Interrupts and pull down resistor configuration are supported too.
Thank you for contribution, overall looks good, below some not very
critical comments.
...
> +static const struct axp192_pctl_reg_info axp192_pin_ctrl_regs[] = {
> + { .reg = AXP192_GPIO0_CTRL, .mask = 0x07 },
> + { .reg = AXP192_GPIO1_CTRL, .mask = 0x07 },
> + { .reg = AXP192_GPIO2_CTRL, .mask = 0x07 },
> + { .reg = AXP192_GPIO4_3_CTRL, .mask = 0x03 },
> + { .reg = AXP192_GPIO4_3_CTRL, .mask = 0x0c },
> + { .reg = AXP192_N_RSTO_CTRL, .mask = 0xc0 },
> +};
GENMASK()
...
> + if ((val & reginfo->mask) == (input_muxvals[offset] << (ffs(reginfo->mask) - 1)))
> + return GPIO_LINE_DIRECTION_IN;
> + else
Redundant.
Also applies for the other similar cases in your code. Note, this is
also redundant for 'continue' and 'break' in case of loops.
> + return GPIO_LINE_DIRECTION_OUT;
...
> + if (!reginfo->mask)
> + return -EOPNOTSUPP;
Please, double check that this is used by the pin control subsystem
and not ENOTSUP in your case here.
...
> + default:
> + return -EOPNOTSUPP;
Ditto.
...
> + default:
> + return -EOPNOTSUPP;
Ditto.
...
> + default:
> + /* unreachable */
> + break;
return 0?! Perhaps you need to return an error?
> + }
> + }
> +
> + return 0;
...
> + if (muxvals[group] == (u8)-1)
limits.h and U8_MAX? Or GENMASK()? Choose one which suits you.
> + return -EINVAL;
...
> + if (!of_device_is_available(pdev->dev.of_node))
> + return -ENODEV;
Dead code.
> + if (!axp20x) {
> + dev_err(&pdev->dev, "Parent drvdata not set\n");
> + return -EINVAL;
> + }
Another useless piece of code.
...
> + pctl->desc = of_device_get_match_data(&pdev->dev);
device_get_match_data()
...
> + pctl->chip.to_irq = axp192_gpio_to_irq;
Why a custom method?
...
> + pctl->pctl_dev = devm_pinctrl_register(&pdev->dev, pctrl_desc, pctl);
> + if (IS_ERR(pctl->pctl_dev)) {
> + dev_err(&pdev->dev, "couldn't register pinctrl driver\n");
> + return PTR_ERR(pctl->pctl_dev);
Here and everywhere else in ->probe() and Co, use
return dev_err_probe(...);
pattern.
> + }
...
> + ret = gpiochip_add_pin_range(&pctl->chip, dev_name(&pdev->dev),
> + pctl->desc->pins->number,
> + pctl->desc->pins->number,
> + pctl->desc->npins);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to add pin range\n");
> + return ret;
> + }
We have a specific callback where you may put this, otherwise on some
systems it may not work as expected.
...
> + dev_info(&pdev->dev, "AXP192 pinctrl and GPIO driver loaded\n");
Useless.
...
> +static struct platform_driver axp192_pctl_driver = {
> + .probe = axp192_pctl_probe,
> + .driver = {
> + .name = "axp192-gpio",
> + .of_match_table = axp192_pctl_match,
> + },
> +};
> +
Redundant blank line.
> +module_platform_driver(axp192_pctl_driver);
...
Globally two comments:
1) I also believe that you may utilize gpio-regmap API;
2) try to get rid of OFisms, make it property provider agnostic.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 10/10] pinctrl: Add AXP192 pin control driver
2022-06-15 13:44 ` Linus Walleij
@ 2022-06-15 18:06 ` Michael Walle
0 siblings, 0 replies; 36+ messages in thread
From: Michael Walle @ 2022-06-15 18:06 UTC (permalink / raw)
To: Linus Walleij, Aidan MacDonald
Cc: brgl, robh+dt, krzysztof.kozlowski+dt, wens, jic23, lee.jones,
sre, broonie, gregkh, lgirdwood, lars, rafael, linux-gpio,
devicetree, linux-iio, linux-pm, linux-kernel
Am 15. Juni 2022 15:44:04 OEZ schrieb Linus Walleij <linus.walleij@linaro.org>:
>On Fri, Jun 3, 2022 at 3:56 PM Aidan MacDonald
><aidanmacdonald.0x0@gmail.com> wrote:
>
>> The AXP192 PMIC's GPIO registers are much different from the GPIO
>> registers of the AXP20x and AXP813 PMICs supported by the existing
>> pinctrl-axp209 driver. It makes more sense to add a new driver for
>> the AXP192, rather than add support in the existing axp20x driver.
>>
>> The pinctrl-axp192 driver is considerably more flexible in terms of
>> register layout and should be able to support other X-Powers PMICs.
>> Interrupts and pull down resistor configuration are supported too.
>>
>> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
>
>Looks good to me (TM) but I'd like Michael Walle to take a look
>to check if this is one of those drivers that could make use of
>gpio-regmap.c CONFIG_GPIO_REGMAP to make it even
>simpler.
>
>Yours,
>Linus Walleij
FWIW, I can look at it at the end of next week. I'm on vacation.
-michael
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 10/10] pinctrl: Add AXP192 pin control driver
2022-06-15 14:51 ` Andy Shevchenko
@ 2022-06-17 12:15 ` Aidan MacDonald
2022-06-17 16:08 ` Andy Shevchenko
0 siblings, 1 reply; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-17 12:15 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Chen-Yu Tsai, Jonathan Cameron, Lee Jones,
Sebastian Reichel, Mark Brown, Greg Kroah-Hartman, Liam Girdwood,
Lars-Peter Clausen, Rafael J. Wysocki, open list:GPIO SUBSYSTEM,
devicetree, linux-iio, Linux PM, Linux Kernel Mailing List
Andy Shevchenko <andy.shevchenko@gmail.com> writes:
> On Fri, Jun 3, 2022 at 6:29 PM Aidan MacDonald
> <aidanmacdonald.0x0@gmail.com> wrote:
>>
>> The AXP192 PMIC's GPIO registers are much different from the GPIO
>> registers of the AXP20x and AXP813 PMICs supported by the existing
>> pinctrl-axp209 driver. It makes more sense to add a new driver for
>> the AXP192, rather than add support in the existing axp20x driver.
>>
>> The pinctrl-axp192 driver is considerably more flexible in terms of
>> register layout and should be able to support other X-Powers PMICs.
>> Interrupts and pull down resistor configuration are supported too.
>
> Thank you for contribution, overall looks good, below some not very
> critical comments.
>
> ...
>
Thanks very much for the review. I'll fix up the issues you spotted
in v3. (v2 doesn't make any changes to the pinctrl driver.)
>> +static const struct axp192_pctl_reg_info axp192_pin_ctrl_regs[] = {
>> + { .reg = AXP192_GPIO0_CTRL, .mask = 0x07 },
>> + { .reg = AXP192_GPIO1_CTRL, .mask = 0x07 },
>> + { .reg = AXP192_GPIO2_CTRL, .mask = 0x07 },
>> + { .reg = AXP192_GPIO4_3_CTRL, .mask = 0x03 },
>> + { .reg = AXP192_GPIO4_3_CTRL, .mask = 0x0c },
>> + { .reg = AXP192_N_RSTO_CTRL, .mask = 0xc0 },
>> +};
>
> GENMASK()
>
> ...
>
>> + if ((val & reginfo->mask) == (input_muxvals[offset] << (ffs(reginfo->mask) - 1)))
>> + return GPIO_LINE_DIRECTION_IN;
>
>> + else
>
> Redundant.
> Also applies for the other similar cases in your code. Note, this is
> also redundant for 'continue' and 'break' in case of loops.
>
Sorry, I'm not sure what you're referring to here. The "else"?
I'm missing the generalization.
>> + return GPIO_LINE_DIRECTION_OUT;
>
> ...
>
>> + if (!reginfo->mask)
>> + return -EOPNOTSUPP;
>
> Please, double check that this is used by the pin control subsystem
> and not ENOTSUP in your case here.
Whoops. You're right, it should be ENOTSUPP.
>
> ...
>
>> + default:
>> + return -EOPNOTSUPP;
>
> Ditto.
>
> ...
>
>> + default:
>> + return -EOPNOTSUPP;
>
> Ditto.
>
> ...
>
>> + default:
>> + /* unreachable */
>> + break;
>
> return 0?! Perhaps you need to return an error?
>
Yeah, that sounds like a good idea for maintainability. I think
there's no need to check that the requested configs are supported
beforehand since the caller must deal with errors in the middle of
the sequence anyway, so I'll drop that check and add ENOTSUPP here.
>> + }
>> + }
>> +
>> + return 0;
>
> ...
>
>> + if (muxvals[group] == (u8)-1)
>
> limits.h and U8_MAX? Or GENMASK()? Choose one which suits you.
>
>> + return -EINVAL;
>
> ...
>
>> + if (!of_device_is_available(pdev->dev.of_node))
>> + return -ENODEV;
>
> Dead code.
>
OK. Did some digging, and this is useless because the parent mfd
device is checking availability.
>> + if (!axp20x) {
>> + dev_err(&pdev->dev, "Parent drvdata not set\n");
>> + return -EINVAL;
>> + }
>
> Another useless piece of code.
>
> ...
>
>> + pctl->desc = of_device_get_match_data(&pdev->dev);
>
> device_get_match_data()
>
> ...
>
>> + pctl->chip.to_irq = axp192_gpio_to_irq;
>
> Why a custom method?
>
> ...
>
The irq chip is part of the mfd device, not the gpio chip. There does
not seem to be any default implementation for this case so I have to
provide one. A similar example is gpio-wm8994.
I did notice I'm doing something wrong by calling regmap_irq_get_virq()
in the probe function, which creates an irq mapping; I think I should be
doing that in the to_irq() callback like the other drivers do.
>> + pctl->pctl_dev = devm_pinctrl_register(&pdev->dev, pctrl_desc, pctl);
>> + if (IS_ERR(pctl->pctl_dev)) {
>> + dev_err(&pdev->dev, "couldn't register pinctrl driver\n");
>> + return PTR_ERR(pctl->pctl_dev);
>
> Here and everywhere else in ->probe() and Co, use
>
> return dev_err_probe(...);
>
> pattern.
>
>> + }
>
> ...
>
>> + ret = gpiochip_add_pin_range(&pctl->chip, dev_name(&pdev->dev),
>> + pctl->desc->pins->number,
>> + pctl->desc->pins->number,
>> + pctl->desc->npins);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to add pin range\n");
>> + return ret;
>> + }
>
> We have a specific callback where you may put this, otherwise on some
> systems it may not work as expected.
>
> ...
>
Ah, sorry, I see that function is deprecated. The documentation points
to doing this in the device tree instead. So if I understand correctly
I should follow the example of pinctrl-thunderbay and add gpio-ranges:
pinctrl0: gpio@0 {
compatible = "x-powers,axp192-gpio";
gpio-controller;
#gpio-cells = <2>;
gpio-ranges = <&pinctrl0 0 0 6>;
};
which means I'll have to update the gpio DT bindings. I'm guessing the
callback you mentioned is add_pin_ranges() or of_gpio_ranges_fallback()
but neither of those seem appropriate in this case. The DT node should
be good enough.
>> + dev_info(&pdev->dev, "AXP192 pinctrl and GPIO driver loaded\n");
>
> Useless.
>
> ...
>
>> +static struct platform_driver axp192_pctl_driver = {
>> + .probe = axp192_pctl_probe,
>> + .driver = {
>> + .name = "axp192-gpio",
>> + .of_match_table = axp192_pctl_match,
>> + },
>> +};
>
>> +
>
> Redundant blank line.
>
>> +module_platform_driver(axp192_pctl_driver);
>
> ...
>
> Globally two comments:
> 1) I also believe that you may utilize gpio-regmap API;
> 2) try to get rid of OFisms, make it property provider agnostic.
I wasn't aware of gpio-regmap, will check it out.
Regards,
Aidan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 10/10] pinctrl: Add AXP192 pin control driver
2022-06-17 12:15 ` Aidan MacDonald
@ 2022-06-17 16:08 ` Andy Shevchenko
0 siblings, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2022-06-17 16:08 UTC (permalink / raw)
To: Aidan MacDonald
Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Chen-Yu Tsai, Jonathan Cameron, Lee Jones,
Sebastian Reichel, Mark Brown, Greg Kroah-Hartman, Liam Girdwood,
Lars-Peter Clausen, Rafael J. Wysocki, open list:GPIO SUBSYSTEM,
devicetree, linux-iio, Linux PM, Linux Kernel Mailing List
On Fri, Jun 17, 2022 at 2:14 PM Aidan MacDonald
<aidanmacdonald.0x0@gmail.com> wrote:
> Andy Shevchenko <andy.shevchenko@gmail.com> writes:
> > On Fri, Jun 3, 2022 at 6:29 PM Aidan MacDonald
> > <aidanmacdonald.0x0@gmail.com> wrote:
...
> >> + if ((val & reginfo->mask) == (input_muxvals[offset] << (ffs(reginfo->mask) - 1)))
> >> + return GPIO_LINE_DIRECTION_IN;
> >
> >> + else
> >
> > Redundant.
> > Also applies for the other similar cases in your code. Note, this is
> > also redundant for 'continue' and 'break' in case of loops.
>
> Sorry, I'm not sure what you're referring to here. The "else"?
Yes.
> I'm missing the generalization.
>
> >> + return GPIO_LINE_DIRECTION_OUT;
...
> >> + pctl->chip.to_irq = axp192_gpio_to_irq;
> >
> > Why a custom method?
>
> The irq chip is part of the mfd device, not the gpio chip. There does
> not seem to be any default implementation for this case so I have to
> provide one. A similar example is gpio-wm8994.
>
> I did notice I'm doing something wrong by calling regmap_irq_get_virq()
> in the probe function, which creates an irq mapping; I think I should be
> doing that in the to_irq() callback like the other drivers do.
It may be done using different approaches, but this part should be
carefully reviewed by GPIO / pin control maintainers.
...
> Ah, sorry, I see that function is deprecated. The documentation points
> to doing this in the device tree instead. So if I understand correctly
> I should follow the example of pinctrl-thunderbay and add gpio-ranges:
>
> pinctrl0: gpio@0 {
> compatible = "x-powers,axp192-gpio";
> gpio-controller;
> #gpio-cells = <2>;
> gpio-ranges = <&pinctrl0 0 0 6>;
> };
>
> which means I'll have to update the gpio DT bindings. I'm guessing the
> callback you mentioned is add_pin_ranges() or of_gpio_ranges_fallback()
> but neither of those seem appropriate in this case. The DT node should
> be good enough.
Sounds good.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 02/10] dt-bindings: mfd: add bindings for AXP192 MFD device
2022-06-03 13:57 ` [PATCH 02/10] dt-bindings: mfd: add bindings for AXP192 MFD device Aidan MacDonald
2022-06-05 22:49 ` Rob Herring
@ 2022-06-27 11:47 ` Lee Jones
1 sibling, 0 replies; 36+ messages in thread
From: Lee Jones @ 2022-06-27 11:47 UTC (permalink / raw)
To: Aidan MacDonald
Cc: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, sre, broonie, gregkh, lgirdwood, lars, rafael, linux-gpio,
devicetree, linux-iio, linux-pm, linux-kernel
On Fri, 03 Jun 2022, Aidan MacDonald wrote:
> The AXP192 is another X-Powers PMIC similar to the existing ones.
>
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> ---
> Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 1 +
> 1 file changed, 1 insertion(+)
Applied, thanks.
--
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 06/10] mfd: axp20x: Add support for AXP192
2022-06-03 13:57 ` [PATCH 06/10] mfd: axp20x: Add support for AXP192 Aidan MacDonald
@ 2022-06-27 11:54 ` Lee Jones
2022-06-27 13:02 ` Aidan MacDonald
0 siblings, 1 reply; 36+ messages in thread
From: Lee Jones @ 2022-06-27 11:54 UTC (permalink / raw)
To: Aidan MacDonald
Cc: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, sre, broonie, gregkh, lgirdwood, lars, rafael, linux-gpio,
devicetree, linux-iio, linux-pm, linux-kernel
On Fri, 03 Jun 2022, Aidan MacDonald wrote:
> The AXP192 PMIC is similar to the AXP202/AXP209, but with different
> regulators, additional GPIOs, and a different IRQ register layout.
>
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> ---
> drivers/mfd/axp20x-i2c.c | 2 +
> drivers/mfd/axp20x.c | 150 +++++++++++++++++++++++++++++++++++++
> include/linux/mfd/axp20x.h | 84 +++++++++++++++++++++
> 3 files changed, 236 insertions(+)
>
> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
> index 00ab48018d8d..9ada58fad77f 100644
> --- a/drivers/mfd/axp20x-i2c.c
> +++ b/drivers/mfd/axp20x-i2c.c
> @@ -62,6 +62,7 @@ static int axp20x_i2c_remove(struct i2c_client *i2c)
> #ifdef CONFIG_OF
> static const struct of_device_id axp20x_i2c_of_match[] = {
> { .compatible = "x-powers,axp152", .data = (void *)AXP152_ID },
> + { .compatible = "x-powers,axp192", .data = (void *)AXP192_ID },
> { .compatible = "x-powers,axp202", .data = (void *)AXP202_ID },
> { .compatible = "x-powers,axp209", .data = (void *)AXP209_ID },
> { .compatible = "x-powers,axp221", .data = (void *)AXP221_ID },
> @@ -75,6 +76,7 @@ MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
>
> static const struct i2c_device_id axp20x_i2c_id[] = {
> { "axp152", 0 },
> + { "axp192", 0 },
> { "axp202", 0 },
> { "axp209", 0 },
> { "axp221", 0 },
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 8161a5dc68e8..7f64e5c83fe2 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -34,6 +34,7 @@
>
> static const char * const axp20x_model_names[] = {
> "AXP152",
> + "AXP192",
> "AXP202",
> "AXP209",
> "AXP221",
> @@ -92,6 +93,35 @@ static const struct regmap_access_table axp20x_volatile_table = {
> .n_yes_ranges = ARRAY_SIZE(axp20x_volatile_ranges),
> };
>
> +static const struct regmap_range axp192_writeable_ranges[] = {
> + regmap_reg_range(AXP192_DATACACHE(0), AXP192_DATACACHE(5)),
> + regmap_reg_range(AXP192_PWR_OUT_CTRL, AXP192_IRQ5_STATE),
> + regmap_reg_range(AXP20X_DCDC_MODE, AXP192_N_RSTO_CTRL),
> + regmap_reg_range(AXP20X_CC_CTRL, AXP20X_CC_CTRL),
> +};
> +
> +static const struct regmap_range axp192_volatile_ranges[] = {
> + regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP192_USB_OTG_STATUS),
> + regmap_reg_range(AXP192_IRQ1_STATE, AXP192_IRQ4_STATE),
> + regmap_reg_range(AXP192_IRQ5_STATE, AXP192_IRQ5_STATE),
> + regmap_reg_range(AXP20X_ACIN_V_ADC_H, AXP20X_IPSOUT_V_HIGH_L),
> + regmap_reg_range(AXP20X_TIMER_CTRL, AXP20X_TIMER_CTRL),
> + regmap_reg_range(AXP192_GPIO2_0_STATE, AXP192_GPIO2_0_STATE),
> + regmap_reg_range(AXP192_GPIO4_3_STATE, AXP192_GPIO4_3_STATE),
> + regmap_reg_range(AXP192_N_RSTO_CTRL, AXP192_N_RSTO_CTRL),
> + regmap_reg_range(AXP20X_CHRG_CC_31_24, AXP20X_CC_CTRL),
> +};
> +
> +static const struct regmap_access_table axp192_writeable_table = {
> + .yes_ranges = axp192_writeable_ranges,
> + .n_yes_ranges = ARRAY_SIZE(axp192_writeable_ranges),
> +};
> +
> +static const struct regmap_access_table axp192_volatile_table = {
> + .yes_ranges = axp192_volatile_ranges,
> + .n_yes_ranges = ARRAY_SIZE(axp192_volatile_ranges),
> +};
> +
> /* AXP22x ranges are shared with the AXP809, as they cover the same range */
> static const struct regmap_range axp22x_writeable_ranges[] = {
> regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
> @@ -173,6 +203,25 @@ static const struct resource axp152_pek_resources[] = {
> DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
> };
>
> +static const struct resource axp192_gpio_resources[] = {
> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO0_INPUT, "GPIO0"),
> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO1_INPUT, "GPIO1"),
> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO2_INPUT, "GPIO2"),
> +};
> +
> +static const struct resource axp192_ac_power_supply_resources[] = {
> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_PLUGIN, "ACIN_PLUGIN"),
> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_REMOVAL, "ACIN_REMOVAL"),
> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_OVER_V, "ACIN_OVER_V"),
> +};
> +
> +static const struct resource axp192_usb_power_supply_resources[] = {
> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_VALID, "VBUS_VALID"),
> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
> +};
> +
> static const struct resource axp20x_ac_power_supply_resources[] = {
> DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_ACIN_PLUGIN, "ACIN_PLUGIN"),
> DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_ACIN_REMOVAL, "ACIN_REMOVAL"),
> @@ -245,6 +294,15 @@ static const struct regmap_config axp152_regmap_config = {
> .cache_type = REGCACHE_RBTREE,
> };
>
> +static const struct regmap_config axp192_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .wr_table = &axp192_writeable_table,
> + .volatile_table = &axp192_volatile_table,
> + .max_register = AXP20X_CC_CTRL,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> static const struct regmap_config axp20x_regmap_config = {
> .reg_bits = 8,
> .val_bits = 8,
> @@ -304,6 +362,55 @@ static const struct regmap_irq axp152_regmap_irqs[] = {
> INIT_REGMAP_IRQ(AXP152, GPIO0_INPUT, 2, 0),
> };
>
> +static const struct regmap_irq axp192_regmap_irqs[] = {
> + INIT_REGMAP_IRQ(AXP192, ACIN_OVER_V, 0, 7),
> + INIT_REGMAP_IRQ(AXP192, ACIN_PLUGIN, 0, 6),
> + INIT_REGMAP_IRQ(AXP192, ACIN_REMOVAL, 0, 5),
> + INIT_REGMAP_IRQ(AXP192, VBUS_OVER_V, 0, 4),
> + INIT_REGMAP_IRQ(AXP192, VBUS_PLUGIN, 0, 3),
> + INIT_REGMAP_IRQ(AXP192, VBUS_REMOVAL, 0, 2),
> + INIT_REGMAP_IRQ(AXP192, VBUS_V_LOW, 0, 1),
> + INIT_REGMAP_IRQ(AXP192, BATT_PLUGIN, 1, 7),
> + INIT_REGMAP_IRQ(AXP192, BATT_REMOVAL, 1, 6),
> + INIT_REGMAP_IRQ(AXP192, BATT_ENT_ACT_MODE, 1, 5),
> + INIT_REGMAP_IRQ(AXP192, BATT_EXIT_ACT_MODE, 1, 4),
> + INIT_REGMAP_IRQ(AXP192, CHARG, 1, 3),
> + INIT_REGMAP_IRQ(AXP192, CHARG_DONE, 1, 2),
> + INIT_REGMAP_IRQ(AXP192, BATT_TEMP_HIGH, 1, 1),
> + INIT_REGMAP_IRQ(AXP192, BATT_TEMP_LOW, 1, 0),
> + INIT_REGMAP_IRQ(AXP192, DIE_TEMP_HIGH, 2, 7),
> + INIT_REGMAP_IRQ(AXP192, CHARG_I_LOW, 2, 6),
> + INIT_REGMAP_IRQ(AXP192, DCDC1_V_LONG, 2, 5),
> + INIT_REGMAP_IRQ(AXP192, DCDC2_V_LONG, 2, 4),
> + INIT_REGMAP_IRQ(AXP192, DCDC3_V_LONG, 2, 3),
> + INIT_REGMAP_IRQ(AXP192, PEK_SHORT, 2, 1),
> + INIT_REGMAP_IRQ(AXP192, PEK_LONG, 2, 0),
> + INIT_REGMAP_IRQ(AXP192, N_OE_PWR_ON, 3, 7),
> + INIT_REGMAP_IRQ(AXP192, N_OE_PWR_OFF, 3, 6),
> + INIT_REGMAP_IRQ(AXP192, VBUS_VALID, 3, 5),
> + INIT_REGMAP_IRQ(AXP192, VBUS_NOT_VALID, 3, 4),
> + INIT_REGMAP_IRQ(AXP192, VBUS_SESS_VALID, 3, 3),
> + INIT_REGMAP_IRQ(AXP192, VBUS_SESS_END, 3, 2),
> + INIT_REGMAP_IRQ(AXP192, LOW_PWR_LVL, 3, 0),
> + INIT_REGMAP_IRQ(AXP192, TIMER, 4, 7),
> + INIT_REGMAP_IRQ(AXP192, GPIO2_INPUT, 4, 2),
> + INIT_REGMAP_IRQ(AXP192, GPIO1_INPUT, 4, 1),
> + INIT_REGMAP_IRQ(AXP192, GPIO0_INPUT, 4, 0),
> +};
> +
> +static int axp192_get_irq_reg(unsigned int base_reg, int i)
Nit: If you have to respin this set, please rename 'i'.
Unless used as an iterator, 'i' is a terrible variable name.
> +{
> + /* linear mapping for IRQ1 to IRQ4 */
> + if (i < 4)
> + return base_reg + i;
> +
> + /* handle IRQ5 separately */
> + if (base_reg == AXP192_IRQ1_EN)
> + return AXP192_IRQ5_EN;
> + else
> + return AXP192_IRQ5_STATE;
> +}
> +
> static const struct regmap_irq axp20x_regmap_irqs[] = {
> INIT_REGMAP_IRQ(AXP20X, ACIN_OVER_V, 0, 7),
> INIT_REGMAP_IRQ(AXP20X, ACIN_PLUGIN, 0, 6),
> @@ -514,6 +621,19 @@ static const struct regmap_irq_chip axp152_regmap_irq_chip = {
> .num_regs = 3,
> };
>
> +static const struct regmap_irq_chip axp192_regmap_irq_chip = {
> + .name = "axp192_irq_chip",
> + .status_base = AXP192_IRQ1_STATE,
> + .ack_base = AXP192_IRQ1_STATE,
> + .mask_base = AXP192_IRQ1_EN,
> + .mask_invert = true,
> + .init_ack_masked = true,
> + .irqs = axp192_regmap_irqs,
> + .num_irqs = ARRAY_SIZE(axp192_regmap_irqs),
> + .num_regs = 5,
> + .get_irq_reg = axp192_get_irq_reg,
> +};
> +
> static const struct regmap_irq_chip axp20x_regmap_irq_chip = {
> .name = "axp20x_irq_chip",
> .status_base = AXP20X_IRQ1_STATE,
> @@ -588,6 +708,30 @@ static const struct regmap_irq_chip axp809_regmap_irq_chip = {
> .num_regs = 5,
> };
>
> +static const struct mfd_cell axp192_cells[] = {
> + {
> + .name = "axp192-gpio",
> + .of_compatible = "x-powers,axp192-gpio",
> + .num_resources = ARRAY_SIZE(axp192_gpio_resources),
> + .resources = axp192_gpio_resources,
> + }, {
> + .name = "axp20x-regulator",
Nit: Is it possible to put one line entries at the bottom?
And format like this:
{ .name = "axp20x-regulator" }
> + }, {
> + .name = "axp192-adc",
> + .of_compatible = "x-powers,axp192-adc",
> + }, {
> + .name = "axp20x-ac-power-supply",
> + .of_compatible = "x-powers,axp202-ac-power-supply",
> + .num_resources = ARRAY_SIZE(axp192_ac_power_supply_resources),
> + .resources = axp192_ac_power_supply_resources,
> + }, {
> + .name = "axp20x-usb-power-supply",
> + .of_compatible = "x-powers,axp192-usb-power-supply",
> + .num_resources = ARRAY_SIZE(axp192_usb_power_supply_resources),
> + .resources = axp192_usb_power_supply_resources,
> + }
> +};
For my own reference (apply this as-is to your sign-off block):
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
--
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 06/10] mfd: axp20x: Add support for AXP192
2022-06-27 11:54 ` Lee Jones
@ 2022-06-27 13:02 ` Aidan MacDonald
0 siblings, 0 replies; 36+ messages in thread
From: Aidan MacDonald @ 2022-06-27 13:02 UTC (permalink / raw)
To: Lee Jones
Cc: linus.walleij, brgl, robh+dt, krzysztof.kozlowski+dt, wens,
jic23, sre, broonie, gregkh, lgirdwood, lars, rafael, linux-gpio,
devicetree, linux-iio, linux-pm, linux-kernel
Lee Jones <lee.jones@linaro.org> writes:
> On Fri, 03 Jun 2022, Aidan MacDonald wrote:
>
>> The AXP192 PMIC is similar to the AXP202/AXP209, but with different
>> regulators, additional GPIOs, and a different IRQ register layout.
>>
>> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
>> ---
>> drivers/mfd/axp20x-i2c.c | 2 +
>> drivers/mfd/axp20x.c | 150 +++++++++++++++++++++++++++++++++++++
>> include/linux/mfd/axp20x.h | 84 +++++++++++++++++++++
>> 3 files changed, 236 insertions(+)
>>
>> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
>> index 00ab48018d8d..9ada58fad77f 100644
>> --- a/drivers/mfd/axp20x-i2c.c
>> +++ b/drivers/mfd/axp20x-i2c.c
>> @@ -62,6 +62,7 @@ static int axp20x_i2c_remove(struct i2c_client *i2c)
>> #ifdef CONFIG_OF
>> static const struct of_device_id axp20x_i2c_of_match[] = {
>> { .compatible = "x-powers,axp152", .data = (void *)AXP152_ID },
>> + { .compatible = "x-powers,axp192", .data = (void *)AXP192_ID },
>> { .compatible = "x-powers,axp202", .data = (void *)AXP202_ID },
>> { .compatible = "x-powers,axp209", .data = (void *)AXP209_ID },
>> { .compatible = "x-powers,axp221", .data = (void *)AXP221_ID },
>> @@ -75,6 +76,7 @@ MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
>>
>> static const struct i2c_device_id axp20x_i2c_id[] = {
>> { "axp152", 0 },
>> + { "axp192", 0 },
>> { "axp202", 0 },
>> { "axp209", 0 },
>> { "axp221", 0 },
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index 8161a5dc68e8..7f64e5c83fe2 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -34,6 +34,7 @@
>>
>> static const char * const axp20x_model_names[] = {
>> "AXP152",
>> + "AXP192",
>> "AXP202",
>> "AXP209",
>> "AXP221",
>> @@ -92,6 +93,35 @@ static const struct regmap_access_table axp20x_volatile_table = {
>> .n_yes_ranges = ARRAY_SIZE(axp20x_volatile_ranges),
>> };
>>
>> +static const struct regmap_range axp192_writeable_ranges[] = {
>> + regmap_reg_range(AXP192_DATACACHE(0), AXP192_DATACACHE(5)),
>> + regmap_reg_range(AXP192_PWR_OUT_CTRL, AXP192_IRQ5_STATE),
>> + regmap_reg_range(AXP20X_DCDC_MODE, AXP192_N_RSTO_CTRL),
>> + regmap_reg_range(AXP20X_CC_CTRL, AXP20X_CC_CTRL),
>> +};
>> +
>> +static const struct regmap_range axp192_volatile_ranges[] = {
>> + regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP192_USB_OTG_STATUS),
>> + regmap_reg_range(AXP192_IRQ1_STATE, AXP192_IRQ4_STATE),
>> + regmap_reg_range(AXP192_IRQ5_STATE, AXP192_IRQ5_STATE),
>> + regmap_reg_range(AXP20X_ACIN_V_ADC_H, AXP20X_IPSOUT_V_HIGH_L),
>> + regmap_reg_range(AXP20X_TIMER_CTRL, AXP20X_TIMER_CTRL),
>> + regmap_reg_range(AXP192_GPIO2_0_STATE, AXP192_GPIO2_0_STATE),
>> + regmap_reg_range(AXP192_GPIO4_3_STATE, AXP192_GPIO4_3_STATE),
>> + regmap_reg_range(AXP192_N_RSTO_CTRL, AXP192_N_RSTO_CTRL),
>> + regmap_reg_range(AXP20X_CHRG_CC_31_24, AXP20X_CC_CTRL),
>> +};
>> +
>> +static const struct regmap_access_table axp192_writeable_table = {
>> + .yes_ranges = axp192_writeable_ranges,
>> + .n_yes_ranges = ARRAY_SIZE(axp192_writeable_ranges),
>> +};
>> +
>> +static const struct regmap_access_table axp192_volatile_table = {
>> + .yes_ranges = axp192_volatile_ranges,
>> + .n_yes_ranges = ARRAY_SIZE(axp192_volatile_ranges),
>> +};
>> +
>> /* AXP22x ranges are shared with the AXP809, as they cover the same range */
>> static const struct regmap_range axp22x_writeable_ranges[] = {
>> regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
>> @@ -173,6 +203,25 @@ static const struct resource axp152_pek_resources[] = {
>> DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
>> };
>>
>> +static const struct resource axp192_gpio_resources[] = {
>> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO0_INPUT, "GPIO0"),
>> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO1_INPUT, "GPIO1"),
>> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO2_INPUT, "GPIO2"),
>> +};
>> +
>> +static const struct resource axp192_ac_power_supply_resources[] = {
>> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_PLUGIN, "ACIN_PLUGIN"),
>> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_REMOVAL, "ACIN_REMOVAL"),
>> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_OVER_V, "ACIN_OVER_V"),
>> +};
>> +
>> +static const struct resource axp192_usb_power_supply_resources[] = {
>> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
>> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
>> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_VALID, "VBUS_VALID"),
>> + DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
>> +};
>> +
>> static const struct resource axp20x_ac_power_supply_resources[] = {
>> DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_ACIN_PLUGIN, "ACIN_PLUGIN"),
>> DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_ACIN_REMOVAL, "ACIN_REMOVAL"),
>> @@ -245,6 +294,15 @@ static const struct regmap_config axp152_regmap_config = {
>> .cache_type = REGCACHE_RBTREE,
>> };
>>
>> +static const struct regmap_config axp192_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> + .wr_table = &axp192_writeable_table,
>> + .volatile_table = &axp192_volatile_table,
>> + .max_register = AXP20X_CC_CTRL,
>> + .cache_type = REGCACHE_RBTREE,
>> +};
>> +
>> static const struct regmap_config axp20x_regmap_config = {
>> .reg_bits = 8,
>> .val_bits = 8,
>> @@ -304,6 +362,55 @@ static const struct regmap_irq axp152_regmap_irqs[] = {
>> INIT_REGMAP_IRQ(AXP152, GPIO0_INPUT, 2, 0),
>> };
>>
>> +static const struct regmap_irq axp192_regmap_irqs[] = {
>> + INIT_REGMAP_IRQ(AXP192, ACIN_OVER_V, 0, 7),
>> + INIT_REGMAP_IRQ(AXP192, ACIN_PLUGIN, 0, 6),
>> + INIT_REGMAP_IRQ(AXP192, ACIN_REMOVAL, 0, 5),
>> + INIT_REGMAP_IRQ(AXP192, VBUS_OVER_V, 0, 4),
>> + INIT_REGMAP_IRQ(AXP192, VBUS_PLUGIN, 0, 3),
>> + INIT_REGMAP_IRQ(AXP192, VBUS_REMOVAL, 0, 2),
>> + INIT_REGMAP_IRQ(AXP192, VBUS_V_LOW, 0, 1),
>> + INIT_REGMAP_IRQ(AXP192, BATT_PLUGIN, 1, 7),
>> + INIT_REGMAP_IRQ(AXP192, BATT_REMOVAL, 1, 6),
>> + INIT_REGMAP_IRQ(AXP192, BATT_ENT_ACT_MODE, 1, 5),
>> + INIT_REGMAP_IRQ(AXP192, BATT_EXIT_ACT_MODE, 1, 4),
>> + INIT_REGMAP_IRQ(AXP192, CHARG, 1, 3),
>> + INIT_REGMAP_IRQ(AXP192, CHARG_DONE, 1, 2),
>> + INIT_REGMAP_IRQ(AXP192, BATT_TEMP_HIGH, 1, 1),
>> + INIT_REGMAP_IRQ(AXP192, BATT_TEMP_LOW, 1, 0),
>> + INIT_REGMAP_IRQ(AXP192, DIE_TEMP_HIGH, 2, 7),
>> + INIT_REGMAP_IRQ(AXP192, CHARG_I_LOW, 2, 6),
>> + INIT_REGMAP_IRQ(AXP192, DCDC1_V_LONG, 2, 5),
>> + INIT_REGMAP_IRQ(AXP192, DCDC2_V_LONG, 2, 4),
>> + INIT_REGMAP_IRQ(AXP192, DCDC3_V_LONG, 2, 3),
>> + INIT_REGMAP_IRQ(AXP192, PEK_SHORT, 2, 1),
>> + INIT_REGMAP_IRQ(AXP192, PEK_LONG, 2, 0),
>> + INIT_REGMAP_IRQ(AXP192, N_OE_PWR_ON, 3, 7),
>> + INIT_REGMAP_IRQ(AXP192, N_OE_PWR_OFF, 3, 6),
>> + INIT_REGMAP_IRQ(AXP192, VBUS_VALID, 3, 5),
>> + INIT_REGMAP_IRQ(AXP192, VBUS_NOT_VALID, 3, 4),
>> + INIT_REGMAP_IRQ(AXP192, VBUS_SESS_VALID, 3, 3),
>> + INIT_REGMAP_IRQ(AXP192, VBUS_SESS_END, 3, 2),
>> + INIT_REGMAP_IRQ(AXP192, LOW_PWR_LVL, 3, 0),
>> + INIT_REGMAP_IRQ(AXP192, TIMER, 4, 7),
>> + INIT_REGMAP_IRQ(AXP192, GPIO2_INPUT, 4, 2),
>> + INIT_REGMAP_IRQ(AXP192, GPIO1_INPUT, 4, 1),
>> + INIT_REGMAP_IRQ(AXP192, GPIO0_INPUT, 4, 0),
>> +};
>> +
>> +static int axp192_get_irq_reg(unsigned int base_reg, int i)
>
> Nit: If you have to respin this set, please rename 'i'.
>
> Unless used as an iterator, 'i' is a terrible variable name.
>
Ack. I had to rework the regmap changes and split them out to their
own series, so I'll fix this when I rebase.
>> +{
>> + /* linear mapping for IRQ1 to IRQ4 */
>> + if (i < 4)
>> + return base_reg + i;
>> +
>> + /* handle IRQ5 separately */
>> + if (base_reg == AXP192_IRQ1_EN)
>> + return AXP192_IRQ5_EN;
>> + else
>> + return AXP192_IRQ5_STATE;
>> +}
>> +
>> static const struct regmap_irq axp20x_regmap_irqs[] = {
>> INIT_REGMAP_IRQ(AXP20X, ACIN_OVER_V, 0, 7),
>> INIT_REGMAP_IRQ(AXP20X, ACIN_PLUGIN, 0, 6),
>> @@ -514,6 +621,19 @@ static const struct regmap_irq_chip axp152_regmap_irq_chip = {
>> .num_regs = 3,
>> };
>>
>> +static const struct regmap_irq_chip axp192_regmap_irq_chip = {
>> + .name = "axp192_irq_chip",
>> + .status_base = AXP192_IRQ1_STATE,
>> + .ack_base = AXP192_IRQ1_STATE,
>> + .mask_base = AXP192_IRQ1_EN,
>> + .mask_invert = true,
>> + .init_ack_masked = true,
>> + .irqs = axp192_regmap_irqs,
>> + .num_irqs = ARRAY_SIZE(axp192_regmap_irqs),
>> + .num_regs = 5,
>> + .get_irq_reg = axp192_get_irq_reg,
>> +};
>> +
>> static const struct regmap_irq_chip axp20x_regmap_irq_chip = {
>> .name = "axp20x_irq_chip",
>> .status_base = AXP20X_IRQ1_STATE,
>> @@ -588,6 +708,30 @@ static const struct regmap_irq_chip axp809_regmap_irq_chip = {
>> .num_regs = 5,
>> };
>>
>> +static const struct mfd_cell axp192_cells[] = {
>> + {
>> + .name = "axp192-gpio",
>> + .of_compatible = "x-powers,axp192-gpio",
>> + .num_resources = ARRAY_SIZE(axp192_gpio_resources),
>> + .resources = axp192_gpio_resources,
>> + }, {
>> + .name = "axp20x-regulator",
>
> Nit: Is it possible to put one line entries at the bottom?
>
> And format like this:
>
> { .name = "axp20x-regulator" }
>
OK.
>> + }, {
>> + .name = "axp192-adc",
>> + .of_compatible = "x-powers,axp192-adc",
>> + }, {
>> + .name = "axp20x-ac-power-supply",
>> + .of_compatible = "x-powers,axp202-ac-power-supply",
>> + .num_resources = ARRAY_SIZE(axp192_ac_power_supply_resources),
>> + .resources = axp192_ac_power_supply_resources,
>> + }, {
>> + .name = "axp20x-usb-power-supply",
>> + .of_compatible = "x-powers,axp192-usb-power-supply",
>> + .num_resources = ARRAY_SIZE(axp192_usb_power_supply_resources),
>> + .resources = axp192_usb_power_supply_resources,
>> + }
>> +};
>
> For my own reference (apply this as-is to your sign-off block):
>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
Thanks!
Regards, Aidan
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2022-06-27 13:07 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-03 13:57 [PATCH 00/10] Add support for AXP192 PMIC Aidan MacDonald
2022-06-03 13:57 ` [PATCH 01/10] regmap-irq: Add get_irq_reg to support unusual register layouts Aidan MacDonald
2022-06-06 17:43 ` Guru Das Srinagesh
2022-06-07 10:46 ` Aidan MacDonald
2022-06-03 13:57 ` [PATCH 02/10] dt-bindings: mfd: add bindings for AXP192 MFD device Aidan MacDonald
2022-06-05 22:49 ` Rob Herring
2022-06-27 11:47 ` Lee Jones
2022-06-03 13:57 ` [PATCH 03/10] dt-bindings: iio: adc: axp209: Add AXP192 compatible Aidan MacDonald
2022-06-03 16:34 ` Jonathan Cameron
2022-06-04 11:33 ` Aidan MacDonald
2022-06-05 22:50 ` Rob Herring
2022-06-03 13:57 ` [PATCH 04/10] dt-bindings: power: supply: axp20x: " Aidan MacDonald
2022-06-05 22:50 ` Rob Herring
2022-06-03 13:57 ` [PATCH 05/10] dt-bindings: gpio: Add AXP192 GPIO bindings Aidan MacDonald
2022-06-05 22:55 ` Rob Herring
2022-06-07 10:34 ` Aidan MacDonald
2022-06-07 15:17 ` Rob Herring
2022-06-07 15:40 ` Aidan MacDonald
2022-06-03 13:57 ` [PATCH 06/10] mfd: axp20x: Add support for AXP192 Aidan MacDonald
2022-06-27 11:54 ` Lee Jones
2022-06-27 13:02 ` Aidan MacDonald
2022-06-03 13:57 ` [PATCH 07/10] regulator: " Aidan MacDonald
2022-06-06 14:36 ` Mark Brown
2022-06-03 13:57 ` [PATCH 08/10] iio: adc: axp20x_adc: " Aidan MacDonald
2022-06-03 16:47 ` Jonathan Cameron
2022-06-04 11:47 ` Aidan MacDonald
2022-06-04 14:27 ` Jonathan Cameron
2022-06-07 10:49 ` Aidan MacDonald
2022-06-03 13:57 ` [PATCH 09/10] power: supply: axp20x_usb_power: " Aidan MacDonald
2022-06-05 15:13 ` kernel test robot
2022-06-03 13:57 ` [PATCH 10/10] pinctrl: Add AXP192 pin control driver Aidan MacDonald
2022-06-15 13:44 ` Linus Walleij
2022-06-15 18:06 ` Michael Walle
2022-06-15 14:51 ` Andy Shevchenko
2022-06-17 12:15 ` Aidan MacDonald
2022-06-17 16:08 ` Andy Shevchenko
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).