2024-02-11 09:47:23

by Karel Balej

[permalink] [raw]
Subject: [RFC PATCH v2 0/6] support for Marvell 88PM886 PMIC

From: Karel Balej <[email protected]>

Hello,

the following implements basic support for Marvell's 88PM886 PMIC which
is found for instance as a component of the samsung,coreprimevelte
smartphone which inspired this and also serves as a testing platform.

The code for the MFD is based primarily on this old series [1] with the
addition of poweroff based on the smartphone's downstream kernel tree
[2]. The onkey and regulators drivers are based on the latter. I am not
in possesion of the datasheet.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://github.com/CoderCharmander/g361f-kernel

Thank you and kind regards,
K. B.
---
RFC v2:
- Merge with the regulators series to have multiple devices and thus
justify the use of the MFD framework.
- Rebase on v6.8-rc3.
- Reorder patches.
- MFD RFC v1: https://lore.kernel.org/all/[email protected]/
- regulators RFC v1: https://lore.kernel.org/all/[email protected]/

Karel Balej (6):
dt-bindings: mfd: add entry for Marvell 88PM886 PMIC
mfd: add driver for Marvell 88PM886 PMIC
regulator: add regulators driver for Marvell 88PM886 PMIC
dt-bindings: input: add entry for Marvell 88PM88X PMICs onkey
input: add onkey driver for Marvell 88PM88X PMICs
MAINTAINERS: add myself for Marvell 88PM886 PMIC

.../bindings/input/marvell,88pm88x-onkey.yaml | 32 +++
.../bindings/mfd/marvell,88pm88x.yaml | 82 +++++++
.../regulator/marvell,88pm88x-regulator.yaml | 28 +++
MAINTAINERS | 11 +
drivers/input/misc/88pm88x-onkey.c | 95 ++++++++
drivers/input/misc/Kconfig | 7 +
drivers/input/misc/Makefile | 1 +
drivers/mfd/88pm88x.c | 211 ++++++++++++++++++
drivers/mfd/Kconfig | 12 +
drivers/mfd/Makefile | 1 +
drivers/regulator/88pm88x-regulator.c | 206 +++++++++++++++++
drivers/regulator/Kconfig | 6 +
drivers/regulator/Makefile | 1 +
include/linux/mfd/88pm88x.h | 46 ++++
14 files changed, 739 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
create mode 100644 Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml
create mode 100644 drivers/input/misc/88pm88x-onkey.c
create mode 100644 drivers/mfd/88pm88x.c
create mode 100644 drivers/regulator/88pm88x-regulator.c
create mode 100644 include/linux/mfd/88pm88x.h

--
2.43.0



2024-02-11 09:47:28

by Karel Balej

[permalink] [raw]
Subject: [RFC PATCH v2 2/6] mfd: add driver for Marvell 88PM886 PMIC

From: Karel Balej <[email protected]>

Marvell 88PM886 is a PMIC which provides various functions such as
onkey, battery, charger and regulators. It is found for instance in the
samsung,coreprimevelte smartphone with which this was tested.

Only implement basic support to allow for the use of regulators and
onkey omitting the currently unused register definitions and I2C
subclients which should thus be added with the subdevice drivers which
need them.

The register mapping and functions of 88PM886 are very similar to those
of 88PM880 and the downstream version of the driver handles both of
these devices. Possible future efforts to support 88PM880 should thus
make use of this driver.

Signed-off-by: Karel Balej <[email protected]>
---

Notes:
RFC v2:
- Remove some abstraction.
- Sort includes alphabetically and add linux/of.h.
- Depend on OF, remove of_match_ptr and add MODULE_DEVICE_TABLE.
- Use more temporaries and break long lines.
- Do not initialize ret in probe.
- Use the wakeup-source DT property.
- Rename ret to err.
- Address Lee's comments:
- Drop patched in presets for base regmap and related defines.
- Use full sentences in comments.
- Remove IRQ comment.
- Define regmap_config member values.
- Rename data to sys_off_data.
- Add _PMIC suffix to Kconfig.
- Use dev_err_probe.
- Do not store irq_data.
- s/WHOAMI/CHIP_ID
- Drop LINUX part of include guard name.
- Merge in the regulator series modifications in order to have more
devices and modify the commit message accordingly. Changes with
respect to the original regulator series patches:
- ret -> err
- Add temporary for dev in pm88x_initialize_subregmaps.
- Drop of_compatible for the regulators.
- Do not duplicate LDO regmap for bucks.
- Rewrite commit message.

drivers/mfd/88pm88x.c | 211 ++++++++++++++++++++++++++++++++++++
drivers/mfd/Kconfig | 12 ++
drivers/mfd/Makefile | 1 +
include/linux/mfd/88pm88x.h | 46 ++++++++
4 files changed, 270 insertions(+)
create mode 100644 drivers/mfd/88pm88x.c
create mode 100644 include/linux/mfd/88pm88x.h

diff --git a/drivers/mfd/88pm88x.c b/drivers/mfd/88pm88x.c
new file mode 100644
index 000000000000..301e3e8f26f4
--- /dev/null
+++ b/drivers/mfd/88pm88x.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/of.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/88pm88x.h>
+
+#define PM88X_REG_INT_STATUS1 0x05
+
+#define PM88X_REG_INT_ENA_1 0x0a
+#define PM88X_INT_ENA1_ONKEY BIT(0)
+
+#define PM88X_REGMAP_CONF_REG_BITS 8
+#define PM88X_REGMAP_CONF_VAL_BITS 8
+#define PM88X_REGMAP_CONF_MAX_REG 0xfe
+
+enum pm88x_irq_number {
+ PM88X_IRQ_ONKEY,
+
+ PM88X_MAX_IRQ
+};
+
+static struct regmap_irq pm88x_regmap_irqs[] = {
+ REGMAP_IRQ_REG(PM88X_IRQ_ONKEY, 0, PM88X_INT_ENA1_ONKEY),
+};
+
+static struct regmap_irq_chip pm88x_regmap_irq_chip = {
+ .name = "88pm88x",
+ .irqs = pm88x_regmap_irqs,
+ .num_irqs = ARRAY_SIZE(pm88x_regmap_irqs),
+ .num_regs = 4,
+ .status_base = PM88X_REG_INT_STATUS1,
+ .ack_base = PM88X_REG_INT_STATUS1,
+ .unmask_base = PM88X_REG_INT_ENA_1,
+};
+
+static struct resource pm88x_onkey_resources[] = {
+ DEFINE_RES_IRQ_NAMED(PM88X_IRQ_ONKEY, "88pm88x-onkey"),
+};
+
+static struct mfd_cell pm886_devs[] = {
+ {
+ .name = "88pm88x-onkey",
+ .of_compatible = "marvell,88pm88x-onkey",
+ .num_resources = ARRAY_SIZE(pm88x_onkey_resources),
+ .resources = pm88x_onkey_resources,
+ },
+ {
+ .name = "88pm88x-regulator",
+ .id = PM88X_REGULATOR_ID_LDO2,
+ },
+ {
+ .name = "88pm88x-regulator",
+ .id = PM88X_REGULATOR_ID_LDO15,
+ },
+ {
+ .name = "88pm88x-regulator",
+ .id = PM886_REGULATOR_ID_BUCK2,
+ },
+};
+
+static const struct regmap_config pm88x_i2c_regmap = {
+ .reg_bits = PM88X_REGMAP_CONF_REG_BITS,
+ .val_bits = PM88X_REGMAP_CONF_VAL_BITS,
+ .max_register = PM88X_REGMAP_CONF_MAX_REG,
+};
+
+static int pm88x_power_off_handler(struct sys_off_data *sys_off_data)
+{
+ struct pm88x_chip *chip = sys_off_data->cb_data;
+ struct regmap *regmap = chip->regmaps[PM88X_REGMAP_BASE];
+ struct device *dev = &chip->client->dev;
+ int err;
+
+ err = regmap_update_bits(regmap, PM88X_REG_MISC_CONFIG1, PM88X_SW_PDOWN,
+ PM88X_SW_PDOWN);
+ if (err) {
+ dev_err(dev, "Failed to power off the device: %d\n", err);
+ return NOTIFY_BAD;
+ }
+ return NOTIFY_DONE;
+}
+
+static int pm88x_initialize_subregmaps(struct pm88x_chip *chip)
+{
+ struct device *dev = &chip->client->dev;
+ struct i2c_client *page;
+ struct regmap *regmap;
+ int err;
+
+ /* LDO page */
+ page = devm_i2c_new_dummy_device(dev, chip->client->adapter,
+ chip->client->addr + PM88X_PAGE_OFFSET_LDO);
+ if (IS_ERR(page)) {
+ err = PTR_ERR(page);
+ dev_err(dev, "Failed to initialize LDO client: %d\n", err);
+ return err;
+ }
+ regmap = devm_regmap_init_i2c(page, &pm88x_i2c_regmap);
+ if (IS_ERR(regmap)) {
+ err = PTR_ERR(regmap);
+ dev_err(dev, "Failed to initialize LDO regmap: %d\n", err);
+ return err;
+ }
+ chip->regmaps[PM88X_REGMAP_LDO] = regmap;
+
+ return 0;
+}
+
+static int pm88x_setup_irq(struct pm88x_chip *chip,
+ struct regmap_irq_chip_data **irq_data)
+{
+ struct regmap *regmap = chip->regmaps[PM88X_REGMAP_BASE];
+ struct device *dev = &chip->client->dev;
+ int err;
+
+ /* Set interrupt clearing mode to clear on write. */
+ err = regmap_update_bits(regmap, PM88X_REG_MISC_CONFIG2,
+ PM88X_INT_INV | PM88X_INT_CLEAR | PM88X_INT_MASK_MODE,
+ PM88X_INT_WC);
+ if (err) {
+ dev_err(dev, "Failed to set interrupt clearing mode: %d\n", err);
+ return err;
+ }
+
+ err = devm_regmap_add_irq_chip(dev, regmap, chip->client->irq,
+ IRQF_ONESHOT, -1, &pm88x_regmap_irq_chip,
+ irq_data);
+ if (err) {
+ dev_err(dev, "Failed to request IRQ: %d\n", err);
+ return err;
+ }
+
+ return 0;
+}
+
+static int pm88x_probe(struct i2c_client *client)
+{
+ struct regmap_irq_chip_data *irq_data;
+ struct device *dev = &client->dev;
+ struct pm88x_chip *chip;
+ struct regmap *regmap;
+ unsigned int chip_id;
+ int err;
+
+ chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->client = client;
+ chip->whoami = (uintptr_t)device_get_match_data(dev);
+ i2c_set_clientdata(client, chip);
+
+ regmap = devm_regmap_init_i2c(client, &pm88x_i2c_regmap);
+ if (IS_ERR(regmap)) {
+ err = PTR_ERR(regmap);
+ return dev_err_probe(dev, err, "Failed to initialize regmap\n");
+ }
+ chip->regmaps[PM88X_REGMAP_BASE] = regmap;
+
+ err = regmap_read(regmap, PM88X_REG_ID, &chip_id);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to read chip ID\n");
+ if (chip->whoami != chip_id)
+ return dev_err_probe(dev, -EINVAL, "Device reported wrong chip ID: %u\n",
+ chip_id);
+
+ err = pm88x_initialize_subregmaps(chip);
+ if (err)
+ return err;
+
+ err = pm88x_setup_irq(chip, &irq_data);
+ if (err)
+ return err;
+
+ err = devm_mfd_add_devices(dev, 0, pm886_devs, ARRAY_SIZE(pm886_devs),
+ NULL, 0, regmap_irq_get_domain(irq_data));
+ if (err)
+ return dev_err_probe(dev, err, "Failed to add devices\n");
+
+ err = devm_register_power_off_handler(dev, pm88x_power_off_handler, chip);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to register power off handler\n");
+
+ device_init_wakeup(dev, device_property_read_bool(dev, "wakeup-source"));
+
+ return 0;
+}
+
+const struct of_device_id pm88x_of_match[] = {
+ { .compatible = "marvell,88pm886-a1", .data = (void *)PM886_A1_CHIP_ID },
+ { },
+};
+MODULE_DEVICE_TABLE(of, pm88x_of_match);
+
+static struct i2c_driver pm88x_i2c_driver = {
+ .driver = {
+ .name = "88pm88x",
+ .of_match_table = pm88x_of_match,
+ },
+ .probe = pm88x_probe,
+};
+module_i2c_driver(pm88x_i2c_driver);
+
+MODULE_DESCRIPTION("Marvell 88PM88X PMIC driver");
+MODULE_AUTHOR("Karel Balej <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index e7a6e45b9fac..93ae5280fc3f 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -794,6 +794,18 @@ config MFD_88PM860X
select individual components like voltage regulators, RTC and
battery-charger under the corresponding menus.

+config MFD_88PM88X_PMIC
+ bool "Marvell 88PM886 PMIC"
+ depends on I2C=y
+ depends on OF
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ select MFD_CORE
+ help
+ This enables support for Marvell 88PM886 Power Management IC.
+ This includes the I2C driver and the core APIs _only_, you have to
+ select individual components like onkey under the corresponding menus.
+
config MFD_MAX14577
tristate "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
depends on I2C
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c66f07edcd0e..c4f95a28f589 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -7,6 +7,7 @@
obj-$(CONFIG_MFD_88PM860X) += 88pm860x.o
obj-$(CONFIG_MFD_88PM800) += 88pm800.o 88pm80x.o
obj-$(CONFIG_MFD_88PM805) += 88pm805.o 88pm80x.o
+obj-$(CONFIG_MFD_88PM88X_PMIC) += 88pm88x.o
obj-$(CONFIG_MFD_ACT8945A) += act8945a.o
obj-$(CONFIG_MFD_SM501) += sm501.o
obj-$(CONFIG_ARCH_BCM2835) += bcm2835-pm.o
diff --git a/include/linux/mfd/88pm88x.h b/include/linux/mfd/88pm88x.h
new file mode 100644
index 000000000000..f1eaaecc784e
--- /dev/null
+++ b/include/linux/mfd/88pm88x.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __MFD_88PM88X_H
+#define __MFD_88PM88X_H
+
+#include <linux/mfd/core.h>
+
+#define PM886_A1_CHIP_ID 0xa1
+
+#define PM88X_REG_ID 0x00
+
+#define PM88X_REG_STATUS1 0x01
+#define PM88X_ONKEY_STS1 BIT(0)
+
+#define PM88X_REG_MISC_CONFIG1 0x14
+#define PM88X_SW_PDOWN BIT(5)
+
+#define PM88X_REG_MISC_CONFIG2 0x15
+#define PM88X_INT_INV BIT(0)
+#define PM88X_INT_CLEAR BIT(1)
+#define PM88X_INT_RC 0x00
+#define PM88X_INT_WC BIT(1)
+#define PM88X_INT_MASK_MODE BIT(2)
+
+#define PM88X_PAGE_OFFSET_LDO 1
+
+enum pm88x_regulator_id {
+ PM88X_REGULATOR_ID_LDO2,
+ PM88X_REGULATOR_ID_LDO15,
+ PM886_REGULATOR_ID_BUCK2,
+
+ PM88X_REGULATOR_ID_SENTINEL
+};
+
+enum pm88x_regmap_index {
+ PM88X_REGMAP_BASE,
+ PM88X_REGMAP_LDO,
+
+ PM88X_REGMAP_NR
+};
+
+struct pm88x_chip {
+ struct i2c_client *client;
+ unsigned int whoami;
+ struct regmap *regmaps[PM88X_REGMAP_NR];
+};
+#endif /* __MFD_88PM88X_H */
--
2.43.0


2024-02-11 09:47:50

by Karel Balej

[permalink] [raw]
Subject: [RFC PATCH v2 1/6] dt-bindings: mfd: add entry for Marvell 88PM886 PMIC

From: Karel Balej <[email protected]>

Marvell 88PM886 is a PMIC with several subdevices such as onkey,
regulators or battery and charger. It comes in at least two revisions,
A0 and A1 -- only A1 is described here at the moment.

Signed-off-by: Karel Balej <[email protected]>
---

Notes:
RFC v2:
- Address Rob's feedback:
- Drop mention of 88PM880.
- Make sure the file passes bindings check (add the necessary header
and fix `interrupt-cells`).
- Other small changes.
- Add regulators. Changes with respect to the regulator RFC series:
- Address Krzysztof's comments:
- Drop unused compatible.
- Fix sub-node pattern.

.../bindings/mfd/marvell,88pm88x.yaml | 74 +++++++++++++++++++
.../regulator/marvell,88pm88x-regulator.yaml | 28 +++++++
2 files changed, 102 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
create mode 100644 Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml

diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
new file mode 100644
index 000000000000..29ab979862d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/marvell,88pm88x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell 88PM88X PMIC core
+
+maintainers:
+ - Karel Balej <[email protected]>
+
+description:
+ Marvell 88PM886 is a PMIC providing several functions such as onkey,
+ regulators or battery and charger.
+
+properties:
+ compatible:
+ const: marvell,88pm886-a1
+
+ reg:
+ maxItems: 1
+
+ interrupt-controller: true
+
+ interrupts:
+ maxItems: 1
+
+ "#interrupt-cells":
+ const: 1
+
+ regulators:
+ $ref: /schemas/regulator/marvell,88pm88x-regulator.yaml#
+
+required:
+ - compatible
+ - reg
+ - interrupt-controller
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ pmic@30 {
+ compatible = "marvell,88pm886-a1";
+ reg = <0x30>;
+ interrupts = <0 4 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&gic>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+
+ regulators {
+ ldo2: ldo2 {
+ regulator-min-microvolt = <3100000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ ldo15: ldo15 {
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ buck2: buck2 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+ };
+ };
+ };
+...
diff --git a/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml b/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml
new file mode 100644
index 000000000000..1b4b5f1b4932
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml
@@ -0,0 +1,28 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/marvell,88pm88x-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Regulators of Marvell 88PM88X PMICs.
+
+maintainers:
+ - Karel Balej <[email protected]>
+
+description: |
+ This is a part of device tree bindings for Marvell 88PM88X MFD.
+
+ The regulators node is represented as a sub-node of the PMIC node on the
+ device tree.
+
+ See also Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml for
+ additional information and example.
+
+patternProperties:
+ "^(ldo(1[0-6]|[1-9])|buck[1-5])$":
+ type: object
+ $ref: /schemas/regulator/regulator.yaml#
+ description: LDO or buck regulator.
+ unevaluatedProperties: false
+
+additionalProperties: false
--
2.43.0


2024-02-11 09:48:02

by Karel Balej

[permalink] [raw]
Subject: [RFC PATCH v2 3/6] regulator: add regulators driver for Marvell 88PM886 PMIC

From: Karel Balej <[email protected]>

Support the LDO and buck regulators of the Marvell 88PM886 PMIC.

88PM886 LDOs match those of 88PM880 which also has several more of them.
88PM880 buck regulators descriptions do not match and they sit on a
different register page and thus need a separate I2C client and regmap.

Signed-off-by: Karel Balej <[email protected]>
---

Notes:
RFC v2:
- Drop of_compatible and related code.
- Drop unused include.
- Remove some abstraction: use only one regmap for all regulators and
only mention 88PM886 in Kconfig description.
- Reword commit message.

drivers/regulator/88pm88x-regulator.c | 206 ++++++++++++++++++++++++++
drivers/regulator/Kconfig | 6 +
drivers/regulator/Makefile | 1 +
3 files changed, 213 insertions(+)
create mode 100644 drivers/regulator/88pm88x-regulator.c

diff --git a/drivers/regulator/88pm88x-regulator.c b/drivers/regulator/88pm88x-regulator.c
new file mode 100644
index 000000000000..fd84b9604ac6
--- /dev/null
+++ b/drivers/regulator/88pm88x-regulator.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/kernel.h>
+#include <linux/linear_range.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#include <linux/mfd/88pm88x.h>
+
+#define PM88X_REG_LDO_EN1 0x09
+#define PM88X_REG_LDO_EN2 0x0a
+
+#define PM88X_REG_BUCK_EN 0x08
+
+#define PM88X_REG_LDO1_VOUT 0x20
+#define PM88X_REG_LDO2_VOUT 0x26
+#define PM88X_REG_LDO3_VOUT 0x2c
+#define PM88X_REG_LDO4_VOUT 0x32
+#define PM88X_REG_LDO5_VOUT 0x38
+#define PM88X_REG_LDO6_VOUT 0x3e
+#define PM88X_REG_LDO7_VOUT 0x44
+#define PM88X_REG_LDO8_VOUT 0x4a
+#define PM88X_REG_LDO9_VOUT 0x50
+#define PM88X_REG_LDO10_VOUT 0x56
+#define PM88X_REG_LDO11_VOUT 0x5c
+#define PM88X_REG_LDO12_VOUT 0x62
+#define PM88X_REG_LDO13_VOUT 0x68
+#define PM88X_REG_LDO14_VOUT 0x6e
+#define PM88X_REG_LDO15_VOUT 0x74
+#define PM88X_REG_LDO16_VOUT 0x7a
+
+#define PM886_REG_BUCK1_VOUT 0xa5
+#define PM886_REG_BUCK2_VOUT 0xb3
+#define PM886_REG_BUCK3_VOUT 0xc1
+#define PM886_REG_BUCK4_VOUT 0xcf
+#define PM886_REG_BUCK5_VOUT 0xdd
+
+#define PM88X_LDO_VSEL_MASK 0x0f
+#define PM88X_BUCK_VSEL_MASK 0x7f
+
+struct pm88x_regulator {
+ struct regulator_desc desc;
+ int max_uA;
+};
+
+static int pm88x_regulator_get_ilim(struct regulator_dev *rdev)
+{
+ struct pm88x_regulator *data = rdev_get_drvdata(rdev);
+
+ if (!data) {
+ dev_err(&rdev->dev, "Failed to get regulator data\n");
+ return -EINVAL;
+ }
+ return data->max_uA;
+}
+
+static const struct regulator_ops pm88x_ldo_ops = {
+ .list_voltage = regulator_list_voltage_table,
+ .map_voltage = regulator_map_voltage_iterate,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .get_current_limit = pm88x_regulator_get_ilim,
+};
+
+static const struct regulator_ops pm88x_buck_ops = {
+ .list_voltage = regulator_list_voltage_linear_range,
+ .map_voltage = regulator_map_voltage_linear_range,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .get_current_limit = pm88x_regulator_get_ilim,
+};
+
+static const unsigned int pm88x_ldo_volt_table1[] = {
+ 1700000, 1800000, 1900000, 2500000, 2800000, 2900000, 3100000, 3300000,
+};
+
+static const unsigned int pm88x_ldo_volt_table2[] = {
+ 1200000, 1250000, 1700000, 1800000, 1850000, 1900000, 2500000, 2600000,
+ 2700000, 2750000, 2800000, 2850000, 2900000, 3000000, 3100000, 3300000,
+};
+
+static const unsigned int pm88x_ldo_volt_table3[] = {
+ 1700000, 1800000, 1900000, 2000000, 2100000, 2500000, 2700000, 2800000,
+};
+
+static const struct linear_range pm88x_buck_volt_ranges1[] = {
+ REGULATOR_LINEAR_RANGE(600000, 0, 79, 12500),
+ REGULATOR_LINEAR_RANGE(1600000, 80, 84, 50000),
+};
+
+static const struct linear_range pm88x_buck_volt_ranges2[] = {
+ REGULATOR_LINEAR_RANGE(600000, 0, 79, 12500),
+ REGULATOR_LINEAR_RANGE(1600000, 80, 114, 50000),
+};
+
+static struct pm88x_regulator pm88x_ldo2 = {
+ .desc = {
+ .name = "LDO2",
+ .id = PM88X_REGULATOR_ID_LDO2,
+ .regulators_node = "regulators",
+ .of_match = "ldo2",
+ .ops = &pm88x_ldo_ops,
+ .type = REGULATOR_VOLTAGE,
+ .enable_reg = PM88X_REG_LDO_EN1,
+ .enable_mask = BIT(1),
+ .volt_table = pm88x_ldo_volt_table1,
+ .n_voltages = ARRAY_SIZE(pm88x_ldo_volt_table1),
+ .vsel_reg = PM88X_REG_LDO2_VOUT,
+ .vsel_mask = PM88X_LDO_VSEL_MASK,
+ },
+ .max_uA = 100000,
+};
+
+static struct pm88x_regulator pm88x_ldo15 = {
+ .desc = {
+ .name = "LDO15",
+ .id = PM88X_REGULATOR_ID_LDO15,
+ .regulators_node = "regulators",
+ .of_match = "ldo15",
+ .ops = &pm88x_ldo_ops,
+ .type = REGULATOR_VOLTAGE,
+ .enable_reg = PM88X_REG_LDO_EN2,
+ .enable_mask = BIT(6),
+ .volt_table = pm88x_ldo_volt_table2,
+ .n_voltages = ARRAY_SIZE(pm88x_ldo_volt_table2),
+ .vsel_reg = PM88X_REG_LDO15_VOUT,
+ .vsel_mask = PM88X_LDO_VSEL_MASK,
+ },
+ .max_uA = 200000,
+};
+
+static struct pm88x_regulator pm886_buck2 = {
+ .desc = {
+ .name = "buck2",
+ .id = PM886_REGULATOR_ID_BUCK2,
+ .regulators_node = "regulators",
+ .of_match = "buck2",
+ .ops = &pm88x_buck_ops,
+ .type = REGULATOR_VOLTAGE,
+ .n_voltages = 115,
+ .linear_ranges = pm88x_buck_volt_ranges2,
+ .n_linear_ranges = ARRAY_SIZE(pm88x_buck_volt_ranges2),
+ .vsel_reg = PM886_REG_BUCK2_VOUT,
+ .vsel_mask = PM88X_BUCK_VSEL_MASK,
+ .enable_reg = PM88X_REG_BUCK_EN,
+ .enable_mask = BIT(1),
+ },
+ .max_uA = 1200000,
+};
+
+static struct pm88x_regulator *pm88x_regulators[] = {
+ [PM88X_REGULATOR_ID_LDO2] = &pm88x_ldo2,
+ [PM88X_REGULATOR_ID_LDO15] = &pm88x_ldo15,
+ [PM886_REGULATOR_ID_BUCK2] = &pm886_buck2,
+};
+
+static int pm88x_regulator_probe(struct platform_device *pdev)
+{
+ struct pm88x_chip *chip = dev_get_drvdata(pdev->dev.parent);
+ struct regulator_config rcfg = { };
+ struct pm88x_regulator *regulator;
+ struct regulator_desc *rdesc;
+ struct regulator_dev *rdev;
+ int ret;
+
+ if (pdev->id < 0 || pdev->id >= PM88X_REGULATOR_ID_SENTINEL) {
+ dev_err(&pdev->dev, "Invalid regulator ID: %d\n", pdev->id);
+ return -EINVAL;
+ }
+
+ rcfg.dev = pdev->dev.parent;
+ regulator = pm88x_regulators[pdev->id];
+ rdesc = &regulator->desc;
+ rcfg.driver_data = regulator;
+ /* For 88PM886, regmap is the same for LDOs and bucks. */
+ rcfg.regmap = chip->regmaps[PM88X_REGMAP_LDO];
+ rdev = devm_regulator_register(&pdev->dev, rdesc, &rcfg);
+ if (IS_ERR(rdev)) {
+ ret = PTR_ERR(rdev);
+ dev_err(&pdev->dev, "Failed to register %s: %d",
+ rdesc->name, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static struct platform_driver pm88x_regulator_driver = {
+ .driver = {
+ .name = "88pm88x-regulator",
+ },
+ .probe = pm88x_regulator_probe,
+};
+module_platform_driver(pm88x_regulator_driver);
+
+MODULE_DESCRIPTION("Marvell 88PM88X PMIC regulator driver");
+MODULE_AUTHOR("Karel Balej <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 550145f82726..8872d0434412 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -91,6 +91,12 @@ config REGULATOR_88PM8607
help
This driver supports 88PM8607 voltage regulator chips.

+config REGULATOR_88PM88X
+ tristate "Marvell 88PM886 voltage regulators"
+ depends on MFD_88PM88X_PMIC
+ help
+ This driver implements support for Marvell 88PM886 voltage regulators.
+
config REGULATOR_ACT8865
tristate "Active-semi act8865 voltage regulator"
depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 46fb569e6be8..6cfe9bb7ba2e 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_REGULATOR_USERSPACE_CONSUMER) += userspace-consumer.o
obj-$(CONFIG_REGULATOR_88PG86X) += 88pg86x.o
obj-$(CONFIG_REGULATOR_88PM800) += 88pm800-regulator.o
obj-$(CONFIG_REGULATOR_88PM8607) += 88pm8607.o
+obj-$(CONFIG_REGULATOR_88PM88X) += 88pm88x-regulator.o
obj-$(CONFIG_REGULATOR_CROS_EC) += cros-ec-regulator.o
obj-$(CONFIG_REGULATOR_CPCAP) += cpcap-regulator.o
obj-$(CONFIG_REGULATOR_AAT2870) += aat2870-regulator.o
--
2.43.0


2024-02-11 09:48:26

by Karel Balej

[permalink] [raw]
Subject: [RFC PATCH v2 4/6] dt-bindings: input: add entry for Marvell 88PM88X PMICs onkey

From: Karel Balej <[email protected]>

Marvell 88PM88X PMICs provide onkey functionality -- add the bindings.

Signed-off-by: Karel Balej <[email protected]>
---

Notes:
RFC v2:
- Add wakeup-source property and reference onkey schema from MFD.
- Reword commit message.

.../bindings/input/marvell,88pm88x-onkey.yaml | 32 +++++++++++++++++++
.../bindings/mfd/marvell,88pm88x.yaml | 8 +++++
2 files changed, 40 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml

diff --git a/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml b/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
new file mode 100644
index 000000000000..5d3d451d0e1f
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/marvell,88pm88x-onkey.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Onkey driver for Marvell 88PM88X PMICs.
+
+maintainers:
+ - Karel Balej <[email protected]>
+
+description: |
+ This module is part of the 88PM88X MFD device. For more details
+ see Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml.
+
+ The onkey controller is represented as a sub-node of the PMIC node in
+ the device tree.
+
+allOf:
+ - $ref: input.yaml#
+
+properties:
+ compatible:
+ const: marvell,88pm88x-onkey
+
+ wakeup-source: true
+
+required:
+ - compatible
+
+additionalProperties: false
+...
diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
index 29ab979862d5..2507a73d4dc3 100644
--- a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
+++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
@@ -28,6 +28,9 @@ properties:
"#interrupt-cells":
const: 1

+ onkey:
+ $ref: /schemas/input/marvell,88pm88x-onkey.yaml
+
regulators:
$ref: /schemas/regulator/marvell,88pm88x-regulator.yaml#

@@ -53,6 +56,11 @@ examples:
interrupt-controller;
#interrupt-cells = <1>;

+ onkey {
+ compatible = "marvell,88pm88x-onkey";
+ wakeup-source;
+ };
+
regulators {
ldo2: ldo2 {
regulator-min-microvolt = <3100000>;
--
2.43.0


2024-02-11 09:48:48

by Karel Balej

[permalink] [raw]
Subject: [RFC PATCH v2 5/6] input: add onkey driver for Marvell 88PM88X PMICs

From: Karel Balej <[email protected]>

Marvell 88PM88X PMICs provide onkey among other things. Add client
driver to handle it. The driver currently only provides a basic support
omitting additional functions found in the vendor version, such as long
onkey and GPIO integration.

Signed-off-by: Karel Balej <[email protected]>
---

Notes:
RFC v2:
- Address Dmitry's feedback:
- Sort includes alphabetically.
- Drop onkey->irq.
- ret -> err in irq_handler and no initialization.
- Break long lines and other formatting.
- Do not clobber platform_get_irq error.
- Do not set device parent manually.
- Use input_set_capability.
- Use the wakeup-source DT property.
- Drop of_match_table.
- Use more temporaries.
- Use dev_err_probe.
- Modify Kconfig description.

drivers/input/misc/88pm88x-onkey.c | 95 ++++++++++++++++++++++++++++++
drivers/input/misc/Kconfig | 7 +++
drivers/input/misc/Makefile | 1 +
3 files changed, 103 insertions(+)
create mode 100644 drivers/input/misc/88pm88x-onkey.c

diff --git a/drivers/input/misc/88pm88x-onkey.c b/drivers/input/misc/88pm88x-onkey.c
new file mode 100644
index 000000000000..2a0bd63a63a7
--- /dev/null
+++ b/drivers/input/misc/88pm88x-onkey.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/88pm88x.h>
+
+struct pm88x_onkey {
+ struct input_dev *idev;
+ struct pm88x_chip *chip;
+};
+
+static irqreturn_t pm88x_onkey_irq_handler(int irq, void *data)
+{
+ struct pm88x_onkey *onkey = data;
+ struct regmap *regmap = onkey->chip->regmaps[PM88X_REGMAP_BASE];
+ struct input_dev *idev = onkey->idev;
+ struct device *parent = idev->dev.parent;
+ unsigned int val;
+ int err;
+
+ err = regmap_read(regmap, PM88X_REG_STATUS1, &val);
+ if (err) {
+ dev_err(parent, "Failed to read status: %d\n", err);
+ return IRQ_NONE;
+ }
+ val &= PM88X_ONKEY_STS1;
+
+ input_report_key(idev, KEY_POWER, val);
+ input_sync(idev);
+
+ return IRQ_HANDLED;
+}
+
+static int pm88x_onkey_probe(struct platform_device *pdev)
+{
+ struct pm88x_chip *chip = dev_get_drvdata(pdev->dev.parent);
+ struct device *dev = &pdev->dev;
+ struct pm88x_onkey *onkey;
+ struct input_dev *idev;
+ int irq, err;
+
+ onkey = devm_kzalloc(dev, sizeof(*onkey), GFP_KERNEL);
+ if (!onkey)
+ return -ENOMEM;
+
+ onkey->chip = chip;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return dev_err_probe(dev, irq, "Failed to get IRQ\n");
+
+ idev = devm_input_allocate_device(dev);
+ if (!idev) {
+ dev_err(dev, "Failed to allocate input device\n");
+ return -ENOMEM;
+ }
+ onkey->idev = idev;
+
+ idev->name = "88pm88x-onkey";
+ idev->phys = "88pm88x-onkey/input0";
+ idev->id.bustype = BUS_I2C;
+
+ input_set_capability(idev, EV_KEY, KEY_POWER);
+
+ err = devm_request_threaded_irq(dev, irq, NULL, pm88x_onkey_irq_handler,
+ IRQF_ONESHOT | IRQF_NO_SUSPEND, "onkey",
+ onkey);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to request IRQ\n");
+
+ err = input_register_device(idev);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to register input device\n");
+
+ device_init_wakeup(dev, device_property_read_bool(dev, "wakeup-source"));
+
+ return 0;
+}
+
+static struct platform_driver pm88x_onkey_driver = {
+ .driver = {
+ .name = "88pm88x-onkey",
+ },
+ .probe = pm88x_onkey_probe,
+};
+module_platform_driver(pm88x_onkey_driver);
+
+MODULE_DESCRIPTION("Marvell 88PM88X onkey driver");
+MODULE_AUTHOR("Karel Balej <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 6ba984d7f0b1..97a1ff83e8df 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -33,6 +33,13 @@ config INPUT_88PM80X_ONKEY
To compile this driver as a module, choose M here: the module
will be called 88pm80x_onkey.

+config INPUT_88PM88X_ONKEY
+ tristate "Marvell 88PM88X onkey support"
+ depends on MFD_88PM88X_PMIC
+ help
+ Support the onkey of Marvell 88PM88X PMICs as an input device
+ reporting power button status.
+
config INPUT_AB8500_PONKEY
tristate "AB8500 Pon (PowerOn) Key"
depends on AB8500_CORE
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 04296a4abe8e..eab7a364188c 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -7,6 +7,7 @@

obj-$(CONFIG_INPUT_88PM860X_ONKEY) += 88pm860x_onkey.o
obj-$(CONFIG_INPUT_88PM80X_ONKEY) += 88pm80x_onkey.o
+obj-$(CONFIG_INPUT_88PM88X_ONKEY) += 88pm88x-onkey.o
obj-$(CONFIG_INPUT_AB8500_PONKEY) += ab8500-ponkey.o
obj-$(CONFIG_INPUT_AD714X) += ad714x.o
obj-$(CONFIG_INPUT_AD714X_I2C) += ad714x-i2c.o
--
2.43.0


2024-02-11 09:49:07

by Karel Balej

[permalink] [raw]
Subject: [RFC PATCH v2 6/6] MAINTAINERS: add myself for Marvell 88PM886 PMIC

From: Karel Balej <[email protected]>

Add an entry to MAINTAINERS for the Marvell 88PM886 PMIC MFD, onkey and
regulator drivers.

Signed-off-by: Karel Balej <[email protected]>
---

Notes:
RFC v2:
- Only mention 88PM886 in the commit message.
- Add regulator driver.
- Rename the entry.

MAINTAINERS | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 960512bec428..c8628b9c633d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12949,6 +12949,17 @@ F: drivers/net/dsa/mv88e6xxx/
F: include/linux/dsa/mv88e6xxx.h
F: include/linux/platform_data/mv88e6xxx.h

+MARVELL 88PM886 PMIC DRIVER
+M: Karel Balej <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
+F: Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
+F: Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml
+F: drivers/input/misc/88pm88x-onkey.c
+F: drivers/mfd/88pm88x.c
+F: drivers/regulators/88pm88x-regulator.c
+F: include/linux/mfd/88pm88x.h
+
MARVELL ARMADA 3700 PHY DRIVERS
M: Miquel Raynal <[email protected]>
S: Maintained
--
2.43.0


2024-02-15 14:21:09

by Rob Herring

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/6] dt-bindings: mfd: add entry for Marvell 88PM886 PMIC

On Sun, Feb 11, 2024 at 10:35:51AM +0100, Karel Balej wrote:
> From: Karel Balej <[email protected]>
>
> Marvell 88PM886 is a PMIC with several subdevices such as onkey,
> regulators or battery and charger. It comes in at least two revisions,
> A0 and A1 -- only A1 is described here at the moment.
>
> Signed-off-by: Karel Balej <[email protected]>
> ---
>
> Notes:
> RFC v2:
> - Address Rob's feedback:
> - Drop mention of 88PM880.
> - Make sure the file passes bindings check (add the necessary header
> and fix `interrupt-cells`).
> - Other small changes.
> - Add regulators. Changes with respect to the regulator RFC series:
> - Address Krzysztof's comments:
> - Drop unused compatible.
> - Fix sub-node pattern.
>
> .../bindings/mfd/marvell,88pm88x.yaml | 74 +++++++++++++++++++

Filename should match the compatible.

In general, drop the 'x' wildcard.

> .../regulator/marvell,88pm88x-regulator.yaml | 28 +++++++
> 2 files changed, 102 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> create mode 100644 Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> new file mode 100644
> index 000000000000..29ab979862d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> @@ -0,0 +1,74 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/marvell,88pm88x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell 88PM88X PMIC core
> +
> +maintainers:
> + - Karel Balej <[email protected]>
> +
> +description:
> + Marvell 88PM886 is a PMIC providing several functions such as onkey,
> + regulators or battery and charger.
> +
> +properties:
> + compatible:
> + const: marvell,88pm886-a1
> +
> + reg:
> + maxItems: 1
> +
> + interrupt-controller: true

What is the device providing interrupts to (in DT)?

> +
> + interrupts:
> + maxItems: 1
> +
> + "#interrupt-cells":
> + const: 1
> +
> + regulators:
> + $ref: /schemas/regulator/marvell,88pm88x-regulator.yaml#

That's simple enough, I'd just move the regulator nodes into this doc.

> +
> +required:
> + - compatible
> + - reg
> + - interrupt-controller
> + - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + pmic@30 {
> + compatible = "marvell,88pm886-a1";
> + reg = <0x30>;
> + interrupts = <0 4 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-parent = <&gic>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + regulators {
> + ldo2: ldo2 {
> + regulator-min-microvolt = <3100000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + ldo15: ldo15 {
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + buck2: buck2 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
> + };
> + };
> + };
> +...
> diff --git a/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml b/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml
> new file mode 100644
> index 000000000000..1b4b5f1b4932
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml
> @@ -0,0 +1,28 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/marvell,88pm88x-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Regulators of Marvell 88PM88X PMICs.
> +
> +maintainers:
> + - Karel Balej <[email protected]>
> +
> +description: |
> + This is a part of device tree bindings for Marvell 88PM88X MFD.
> +
> + The regulators node is represented as a sub-node of the PMIC node on the
> + device tree.
> +
> + See also Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml for
> + additional information and example.
> +
> +patternProperties:
> + "^(ldo(1[0-6]|[1-9])|buck[1-5])$":
> + type: object
> + $ref: /schemas/regulator/regulator.yaml#
> + description: LDO or buck regulator.
> + unevaluatedProperties: false
> +
> +additionalProperties: false
> --
> 2.43.0
>

2024-02-15 14:26:09

by Rob Herring

[permalink] [raw]
Subject: Re: [RFC PATCH v2 4/6] dt-bindings: input: add entry for Marvell 88PM88X PMICs onkey

On Sun, Feb 11, 2024 at 10:35:54AM +0100, Karel Balej wrote:
> From: Karel Balej <[email protected]>
>
> Marvell 88PM88X PMICs provide onkey functionality -- add the bindings.
>
> Signed-off-by: Karel Balej <[email protected]>
> ---
>
> Notes:
> RFC v2:
> - Add wakeup-source property and reference onkey schema from MFD.
> - Reword commit message.
>
> .../bindings/input/marvell,88pm88x-onkey.yaml | 32 +++++++++++++++++++
> .../bindings/mfd/marvell,88pm88x.yaml | 8 +++++
> 2 files changed, 40 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml b/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
> new file mode 100644
> index 000000000000..5d3d451d0e1f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
> @@ -0,0 +1,32 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/marvell,88pm88x-onkey.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Onkey driver for Marvell 88PM88X PMICs.
> +
> +maintainers:
> + - Karel Balej <[email protected]>
> +
> +description: |
> + This module is part of the 88PM88X MFD device. For more details
> + see Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml.
> +
> + The onkey controller is represented as a sub-node of the PMIC node in
> + the device tree.

Why do you need a child node? You don't. Just add 'wakeup-source' to the
parent.

> +
> +allOf:
> + - $ref: input.yaml#

Doesn't look like you are using any properties from this?

> +
> +properties:
> + compatible:
> + const: marvell,88pm88x-onkey
> +
> + wakeup-source: true
> +
> +required:
> + - compatible
> +
> +additionalProperties: false
> +...

2024-02-18 15:19:14

by Karel Balej

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/6] dt-bindings: mfd: add entry for Marvell 88PM886 PMIC

Rob Herring, 2024-02-15T08:20:52-06:00:
> > .../bindings/mfd/marvell,88pm88x.yaml | 74 +++++++++++++++++++
>
> Filename should match the compatible.
>
> In general, drop the 'x' wildcard.

By "in general", do you mean for the drivers code also?

As I have mentioned in the commit message for the driver, the other
device is very similar and if the support for it was ever to be added
(which I personally currently have no interest in), I believe it would
make sense to extend this driver. Is it then still prefered to call it
all just 88pm886 now?

> > +properties:
> > + compatible:
> > + const: marvell,88pm886-a1

So the file should be called marvell,88pm886-a1.yaml, correct? Again, is
it prefered to call it like this even if the other revision could
eventually be added (again, I am not interested in that right now
personally)? I mean, if I was implementing support for both revisions
right now, it would make sense to name it just marvell,88pm886.yaml, no?

Thank you, kind regards,
K. B.

2024-02-19 07:30:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/6] dt-bindings: mfd: add entry for Marvell 88PM886 PMIC

On 18/02/2024 16:10, Karel Balej wrote:
> Rob Herring, 2024-02-15T08:20:52-06:00:
>>> .../bindings/mfd/marvell,88pm88x.yaml | 74 +++++++++++++++++++
>>
>> Filename should match the compatible.
>>
>> In general, drop the 'x' wildcard.
>
> By "in general", do you mean for the drivers code also?

No, not driver. The rules for wildcard, that they are discouraged, are
DT binding rules.

>
> As I have mentioned in the commit message for the driver, the other
> device is very similar and if the support for it was ever to be added
> (which I personally currently have no interest in), I believe it would
> make sense to extend this driver. Is it then still prefered to call it
> all just 88pm886 now?

Extend the driver, it's unrelated. Binding still should be named like
compatible, because that extension might never happen.

>
>>> +properties:
>>> + compatible:
>>> + const: marvell,88pm886-a1
>
> So the file should be called marvell,88pm886-a1.yaml, correct? Again, is
> it prefered to call it like this even if the other revision could
> eventually be added (again, I am not interested in that right now

If you already add two devices, flexible name would be fine. But you do
not add it now and you might never add, so keep the filename=compatible.
It is fine if it has also other compatibles later. We already accepted
many bindings like that.


Best regards,
Krzysztof