2023-04-12 11:33:00

by Sahin, Okan

[permalink] [raw]
Subject: [PATCH v7 0/5] Add MAX77541/MAX77540 PMIC Support

MFD, regulator and ADC driver and related bindings for MAX77540/MAX77541.
The patches are required to be applied in sequence.

Changes in v7:
* Patch 1: "dt-bindings: regulator: max77541: Add ADI MAX77541/MAX77540 Regulator"
* NO CHANGE
* Patch 2: "regulator: max77541: Add ADI MAX77541/MAX77540 Regulator Support"
* Add explanation into Kconfig
* Patch 3: "iio: adc: : max77541 Add ADI MAX77541 ADC Support"
* NO CHANGE
* Patch 4: "dt-bindings: mfd: max77541: adi,max77541.yaml Add MAX77541 bindings"
* NO CHANGE
* Patch 5: "mfd: max77541: Add MAX77541/MAX77540 PMIC Support"
* Drop chip_info structure, use only id.
* Use plain number as data.

Changes in v6:
* Patch 1: "dt-bindings: regulator: max77541: Add ADI MAX77541/MAX77540 Regulator"
* NO CHANGE
* Patch 2: "regulator: max77541: Add ADI MAX77541/MAX77540 Regulator Support"
* Drop unnecessary headers
* Patch 3: "iio: adc: : max77541 Add ADI MAX77541 ADC Support"
* Drop unnecessary headers
* Patch 4: "dt-bindings: mfd: max77541: adi,max77541.yaml Add MAX77541 bindings"
* NO CHANGE
* Patch 5: "mfd: max77541: Add MAX77541/MAX77540 PMIC Support"
* Add more explanation to Kconfig
* Drop unnecessary headers
* Change differentiate method for different IC's
* Modify order of registers in header file

Changes in v5:
* Patch 1: "dt-bindings: regulator: max77541: Add ADI MAX77541/MAX77540 Regulator"
* Drop compatible properties.
* Patch 2: "regulator: max77541: Add ADI MAX77541/MAX77540 Regulator Support"
* Change if-else ladder to switch case for chip->id
* Drop driver_data in platform_device_id
* Patch 3: "iio: adc: : max77541 Add ADI MAX77541 ADC Support"
* Drop max77541_adc_iio struct
* Patch 4: "dt-bindings: mfd: max77541: adi,max77541.yaml Add MAX77541 bindings"
* Drop allOf
* Patch 5: "mfd: max77541: Add MAX77541/MAX77540 PMIC Support"
* Dont use compatible when using MFD_CELL_OF MACRO

Changes in v4:
* Patch 1: "dt-bindings: regulator: Add ADI MAX77541/MAX77540 Regulator"
* NO CHANGE
* Patch 2: "drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support"
* Drop OF ID Table
* Drop driver_data in platform_device_id
* Patch 3: "drivers: iio: adc: Add ADI MAX77541 ADC Support"
* Add missing blank line
* Patch 4: "dt-bindings: mfd: adi,max77541.yaml Add MAX77541 bindings"
* NO CHANGE(Order of patchset changed, and [4/5] has dependency to [1/5])
* Patch 5: "drivers: mfd: Add MAX77541/MAX77540 PMIC Support"
* Use pointers in the driver_data
* Use probe_new instead of probe
* Use PLATFORM_DEVID_NONE macro instead of "-1"

Changes in v3:
* Patch 1: "drivers: mfd: Add ADI MAX77541/MAX77540 PMIC Support"
* Change struct name from max77541_dev to max77541
* Adjust max-line-length lower than 80
* Patch 2: "dt-bindings: mfd: Add ADI MAX77541/MAX77540"
* Remove adc object as we do not need
* Remove adc node from example
* Patch 3: "drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support"
* Change node name from "BUCK#_id" to "buck#_id" in regulator desc
* Patch 4: "dt-bindings: regulator: Add ADI MAX77541/MAX77540 Regulator"
* Change node name from "BUCK" to "buck" in regulators
* Patch 5: "drivers: iio: adc: Add ADI MAX77541 ADC Support"
* Convert voltage values from V to mV for scaling.
* Convert temperature values from C to miliC for scale and offset
* Do not set offset bit in info_mask_separate for voltage that does not need offset
* Remove unnecessary dev_get_drvdata() instead of it use dev_get_regmap to have regmap.
* Assing hard coded name for adc dev name

Changes in v2:
* Patch 1: "drivers: mfd: Add MAX77541/MAX77540 PMIC Support"
* Drop "this patch adds" from commit message.
* Drop redundant blank lines.
* Drop module version
* Use definition for parameter of devm_mfd_add_devices(.., -1,..)
* Use desc in chip_info to adding desc for different devices.
* Add missing headers and forward declarations.
* Drop unused elements from max77541_dev struct
* Add chip_info into max77541_dev struct to identify different devices.
* Patch 2: "dt-bindings: mfd: adi,max77541.yaml Add MAX77541 bindings"
* Drop "this patch adds" from commit message.
* Fix $ref path
* Drop adc part under allOf
* Keep only one example (more complex one)
* Fix make dt_binding_check errors.(trailing space, No newline)
* Patch 3: "drivers: regulator: Add MAX77541 Regulator Support"
* Drop "this patch adds" from commit message.
* Add trailing comma for required structs.
* Fix wrong indentation.
* Drop redundant blank lines.
* Drop max77541_regulator_dev struct.
* Use "regulator_desc *desc" for both regulator
regarding to "max77541->id"
* Patch 4: "dt-bindings: regulator: max77541-regulator.yaml Add MAX77541
Regulator bindings"
* Drop "this patch adds" from commit message.
* Chance filename (matching compatible), so adi,max77541-regulator.yaml
* Fix make dt_binding_check errors.(trailing space, No newline)
* Patch 5: "drivers: iio: adc: Add MAX77541 ADC Support"
* Drop "this patch adds" from commit message.
* Drop redundant blank lines.
* Fix wrong include path.
* Use switch instead of if-else for range setting in max77541_adc_scale
* Move max77541_adc_range enum from max77541.h to here.
* Use definition from units.h
* Drop unused elements from max77541_adc_iio struct
* Drop the .data from platform_device_id

Okan Sahin (5):
dt-bindings: regulator: max77541: Add ADI MAX77541/MAX77540 Regulator
regulator: max77541: Add ADI MAX77541/MAX77540 Regulator Support
iio: adc: max77541: Add ADI MAX77541 ADC Support
dt-bindings: mfd: max77541: Add ADI MAX77541/MAX77540
mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

.../devicetree/bindings/mfd/adi,max77541.yaml | 68 ++++++
.../regulator/adi,max77541-regulator.yaml | 38 +++
drivers/iio/adc/Kconfig | 11 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/max77541-adc.c | 194 +++++++++++++++
drivers/mfd/Kconfig | 13 +
drivers/mfd/Makefile | 1 +
drivers/mfd/max77541.c | 224 ++++++++++++++++++
drivers/regulator/Kconfig | 11 +
drivers/regulator/Makefile | 1 +
drivers/regulator/max77541-regulator.c | 153 ++++++++++++
include/linux/mfd/max77541.h | 91 +++++++
12 files changed, 806 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77541.yaml
create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77541-regulator.yaml
create mode 100644 drivers/iio/adc/max77541-adc.c
create mode 100644 drivers/mfd/max77541.c
create mode 100644 drivers/regulator/max77541-regulator.c
create mode 100644 include/linux/mfd/max77541.h

--
2.30.2


2023-04-12 12:24:37

by Sahin, Okan

[permalink] [raw]
Subject: [PATCH v7 1/5] dt-bindings: regulator: max77541: Add ADI MAX77541/MAX77540 Regulator

Add ADI MAX77541/MAX77540 Regulator devicetree document.

Signed-off-by: Okan Sahin <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../regulator/adi,max77541-regulator.yaml | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77541-regulator.yaml

diff --git a/Documentation/devicetree/bindings/regulator/adi,max77541-regulator.yaml b/Documentation/devicetree/bindings/regulator/adi,max77541-regulator.yaml
new file mode 100644
index 000000000000..9e36d5467b56
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/adi,max77541-regulator.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/adi,max77541-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Buck Converter for MAX77540/MAX77541
+
+maintainers:
+ - Okan Sahin <[email protected]>
+
+description: |
+ This is a part of device tree bindings for ADI MAX77540/MAX77541
+
+ The buck converter is represented as a sub-node of the PMIC node on the device tree.
+
+ The device has two buck regulators.
+ See also Documentation/devicetree/bindings/mfd/adi,max77541.yaml for
+ additional information and example.
+
+patternProperties:
+ "^buck[12]$":
+ type: object
+ $ref: regulator.yaml#
+ additionalProperties: false
+ description: |
+ Buck regulator.
+
+ properties:
+ regulator-name: true
+ regulator-always-on: true
+ regulator-boot-on: true
+ regulator-min-microvolt:
+ minimum: 300000
+ regulator-max-microvolt:
+ maximum: 5200000
+
+additionalProperties: false
--
2.30.2

2023-04-12 12:30:29

by Sahin, Okan

[permalink] [raw]
Subject: [PATCH v7 3/5] iio: adc: max77541: Add ADI MAX77541 ADC Support

The MAX77541 has an 8-bit Successive Approximation Register (SAR) ADC
with four multiplexers for supporting the telemetry feature.

Signed-off-by: Okan Sahin <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
---
drivers/iio/adc/Kconfig | 11 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/max77541-adc.c | 194 +++++++++++++++++++++++++++++++++
3 files changed, 206 insertions(+)
create mode 100644 drivers/iio/adc/max77541-adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 45af2302be53..518e7bd453aa 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -735,6 +735,17 @@ config MAX1363
To compile this driver as a module, choose M here: the module will be
called max1363.

+config MAX77541_ADC
+ tristate "Analog Devices MAX77541 ADC driver"
+ depends on MFD_MAX77541
+ help
+ This driver controls a Analog Devices MAX77541 ADC
+ via I2C bus. This device has one adc. Say yes here to build
+ support for Analog Devices MAX77541 ADC interface.
+
+ To compile this driver as a module, choose M here:
+ the module will be called max77541-adc.
+
config MAX9611
tristate "Maxim max9611/max9612 ADC driver"
depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 36c18177322a..f8433b560c3b 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_MAX11205) += max11205.o
obj-$(CONFIG_MAX11410) += max11410.o
obj-$(CONFIG_MAX1241) += max1241.o
obj-$(CONFIG_MAX1363) += max1363.o
+obj-$(CONFIG_MAX77541_ADC) += max77541-adc.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
diff --git a/drivers/iio/adc/max77541-adc.c b/drivers/iio/adc/max77541-adc.c
new file mode 100644
index 000000000000..21d024bde16b
--- /dev/null
+++ b/drivers/iio/adc/max77541-adc.c
@@ -0,0 +1,194 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 Analog Devices, Inc.
+ * ADI MAX77541 ADC Driver with IIO interface
+ */
+
+#include <linux/bitfield.h>
+#include <linux/iio/iio.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/units.h>
+
+#include <linux/mfd/max77541.h>
+
+enum max77541_adc_range {
+ LOW_RANGE,
+ MID_RANGE,
+ HIGH_RANGE,
+};
+
+enum max77541_adc_channel {
+ MAX77541_ADC_VSYS_V,
+ MAX77541_ADC_VOUT1_V,
+ MAX77541_ADC_VOUT2_V,
+ MAX77541_ADC_TEMP,
+};
+
+static int max77541_adc_offset(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2)
+{
+ switch (chan->channel) {
+ case MAX77541_ADC_TEMP:
+ *val = DIV_ROUND_CLOSEST(ABSOLUTE_ZERO_MILLICELSIUS, 1725);
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int max77541_adc_scale(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2)
+{
+ struct regmap **regmap = iio_priv(indio_dev);
+ unsigned int reg_val;
+ int ret;
+
+ switch (chan->channel) {
+ case MAX77541_ADC_VSYS_V:
+ *val = 25;
+ return IIO_VAL_INT;
+ case MAX77541_ADC_VOUT1_V:
+ case MAX77541_ADC_VOUT2_V:
+ ret = regmap_read(*regmap, MAX77541_REG_M2_CFG1, &reg_val);
+ if (ret)
+ return ret;
+
+ reg_val = FIELD_GET(MAX77541_BITS_MX_CFG1_RNG, reg_val);
+ switch (reg_val) {
+ case LOW_RANGE:
+ *val = 6;
+ *val2 = 250000;
+ break;
+ case MID_RANGE:
+ *val = 12;
+ *val2 = 500000;
+ break;
+ case HIGH_RANGE:
+ *val = 25;
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+
+ return IIO_VAL_INT_PLUS_MICRO;
+ case MAX77541_ADC_TEMP:
+ *val = 1725;
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int max77541_adc_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val)
+{
+ struct regmap **regmap = iio_priv(indio_dev);
+ int ret;
+
+ ret = regmap_read(*regmap, chan->address, val);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+}
+
+#define MAX77541_ADC_CHANNEL_V(_channel, _name, _type, _reg) \
+ { \
+ .type = _type, \
+ .indexed = 1, \
+ .channel = _channel, \
+ .address = _reg, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .datasheet_name = _name, \
+ }
+
+#define MAX77541_ADC_CHANNEL_TEMP(_channel, _name, _type, _reg) \
+ { \
+ .type = _type, \
+ .indexed = 1, \
+ .channel = _channel, \
+ .address = _reg, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE) |\
+ BIT(IIO_CHAN_INFO_OFFSET),\
+ .datasheet_name = _name, \
+ }
+
+static const struct iio_chan_spec max77541_adc_channels[] = {
+ MAX77541_ADC_CHANNEL_V(MAX77541_ADC_VSYS_V, "vsys_v", IIO_VOLTAGE,
+ MAX77541_REG_ADC_DATA_CH1),
+ MAX77541_ADC_CHANNEL_V(MAX77541_ADC_VOUT1_V, "vout1_v", IIO_VOLTAGE,
+ MAX77541_REG_ADC_DATA_CH2),
+ MAX77541_ADC_CHANNEL_V(MAX77541_ADC_VOUT2_V, "vout2_v", IIO_VOLTAGE,
+ MAX77541_REG_ADC_DATA_CH3),
+ MAX77541_ADC_CHANNEL_TEMP(MAX77541_ADC_TEMP, "temp", IIO_TEMP,
+ MAX77541_REG_ADC_DATA_CH6),
+};
+
+static int max77541_adc_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 max77541_adc_offset(indio_dev, chan, val, val2);
+ case IIO_CHAN_INFO_SCALE:
+ return max77541_adc_scale(indio_dev, chan, val, val2);
+ case IIO_CHAN_INFO_RAW:
+ return max77541_adc_raw(indio_dev, chan, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info max77541_adc_info = {
+ .read_raw = max77541_adc_read_raw,
+};
+
+static int max77541_adc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct iio_dev *indio_dev;
+ struct regmap **regmap;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*regmap));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ regmap = iio_priv(indio_dev);
+
+ *regmap = dev_get_regmap(dev->parent, NULL);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ indio_dev->name = "max77541";
+ indio_dev->info = &max77541_adc_info;
+ indio_dev->channels = max77541_adc_channels;
+ indio_dev->num_channels = ARRAY_SIZE(max77541_adc_channels);
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct platform_device_id max77541_adc_platform_id[] = {
+ { "max77541-adc" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, max77541_adc_platform_id);
+
+static struct platform_driver max77541_adc_driver = {
+ .driver = {
+ .name = "max77541-adc",
+ },
+ .probe = max77541_adc_probe,
+ .id_table = max77541_adc_platform_id,
+};
+module_platform_driver(max77541_adc_driver);
+
+MODULE_AUTHOR("Okan Sahin <[email protected]>");
+MODULE_DESCRIPTION("MAX77541 ADC driver");
+MODULE_LICENSE("GPL");
--
2.30.2

2023-04-12 12:30:34

by Sahin, Okan

[permalink] [raw]
Subject: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

MFD driver for MAX77541/MAX77540 to enable its sub
devices.

The MAX77541 is a multi-function devices. It includes
buck converter and ADC.

The MAX77540 is a high-efficiency buck converter
with two 3A switching phases.

They have same regmap except for ADC part of MAX77541.

Signed-off-by: Okan Sahin <[email protected]>
---
drivers/mfd/Kconfig | 13 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/max77541.c | 224 +++++++++++++++++++++++++++++++++++
include/linux/mfd/max77541.h | 91 ++++++++++++++
4 files changed, 329 insertions(+)
create mode 100644 drivers/mfd/max77541.c
create mode 100644 include/linux/mfd/max77541.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index fcc141e067b9..de89245ce1cb 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -777,6 +777,19 @@ config MFD_MAX14577
additional drivers must be enabled in order to use the functionality
of the device.

+config MFD_MAX77541
+ tristate "Analog Devices MAX77541/77540 PMIC Support"
+ depends on I2C=y
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
+ Say yes here to add support for Analog Devices MAX77541 and
+ MAX77540 Power Management ICs. This driver provides
+ common support for accessing the device; additional drivers
+ must be enabled in order to use the functionality of the device.
+ There are regulators and adc.
+
config MFD_MAX77620
bool "Maxim Semiconductor MAX77620 and MAX20024 PMIC Support"
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 2f6c89d1e277..1c8540ddead6 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -151,6 +151,7 @@ obj-$(CONFIG_MFD_DA9063) += da9063.o
obj-$(CONFIG_MFD_DA9150) += da9150-core.o

obj-$(CONFIG_MFD_MAX14577) += max14577.o
+obj-$(CONFIG_MFD_MAX77541) += max77541.o
obj-$(CONFIG_MFD_MAX77620) += max77620.o
obj-$(CONFIG_MFD_MAX77650) += max77650.o
obj-$(CONFIG_MFD_MAX77686) += max77686.o
diff --git a/drivers/mfd/max77541.c b/drivers/mfd/max77541.c
new file mode 100644
index 000000000000..4a3bad3493b3
--- /dev/null
+++ b/drivers/mfd/max77541.c
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 Analog Devices, Inc.
+ * Driver for the MAX77540 and MAX77541
+ */
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max77541.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+static const struct regmap_config max77541_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+static const struct regmap_irq max77541_src_irqs[] = {
+ { .mask = MAX77541_BIT_INT_SRC_TOPSYS },
+ { .mask = MAX77541_BIT_INT_SRC_BUCK },
+};
+
+static const struct regmap_irq_chip max77541_src_irq_chip = {
+ .name = "max77541-src",
+ .status_base = MAX77541_REG_INT_SRC,
+ .mask_base = MAX77541_REG_INT_SRC_M,
+ .num_regs = 1,
+ .irqs = max77541_src_irqs,
+ .num_irqs = ARRAY_SIZE(max77541_src_irqs),
+};
+
+static const struct regmap_irq max77541_topsys_irqs[] = {
+ { .mask = MAX77541_BIT_TOPSYS_INT_TJ_120C },
+ { .mask = MAX77541_BIT_TOPSYS_INT_TJ_140C },
+ { .mask = MAX77541_BIT_TOPSYS_INT_TSHDN },
+ { .mask = MAX77541_BIT_TOPSYS_INT_UVLO },
+ { .mask = MAX77541_BIT_TOPSYS_INT_ALT_SWO },
+ { .mask = MAX77541_BIT_TOPSYS_INT_EXT_FREQ_DET },
+};
+
+static const struct regmap_irq_chip max77541_topsys_irq_chip = {
+ .name = "max77541-topsys",
+ .status_base = MAX77541_REG_TOPSYS_INT,
+ .mask_base = MAX77541_REG_TOPSYS_INT_M,
+ .num_regs = 1,
+ .irqs = max77541_topsys_irqs,
+ .num_irqs = ARRAY_SIZE(max77541_topsys_irqs),
+};
+
+static const struct regmap_irq max77541_buck_irqs[] = {
+ { .mask = MAX77541_BIT_BUCK_INT_M1_POK_FLT },
+ { .mask = MAX77541_BIT_BUCK_INT_M2_POK_FLT },
+ { .mask = MAX77541_BIT_BUCK_INT_M1_SCFLT },
+ { .mask = MAX77541_BIT_BUCK_INT_M2_SCFLT },
+};
+
+static const struct regmap_irq_chip max77541_buck_irq_chip = {
+ .name = "max77541-buck",
+ .status_base = MAX77541_REG_BUCK_INT,
+ .mask_base = MAX77541_REG_BUCK_INT_M,
+ .num_regs = 1,
+ .irqs = max77541_buck_irqs,
+ .num_irqs = ARRAY_SIZE(max77541_buck_irqs),
+};
+
+static const struct regmap_irq max77541_adc_irqs[] = {
+ { .mask = MAX77541_BIT_ADC_INT_CH1_I },
+ { .mask = MAX77541_BIT_ADC_INT_CH2_I },
+ { .mask = MAX77541_BIT_ADC_INT_CH3_I },
+ { .mask = MAX77541_BIT_ADC_INT_CH6_I },
+};
+
+static const struct regmap_irq_chip max77541_adc_irq_chip = {
+ .name = "max77541-adc",
+ .status_base = MAX77541_REG_ADC_INT,
+ .mask_base = MAX77541_REG_ADC_INT_M,
+ .num_regs = 1,
+ .irqs = max77541_adc_irqs,
+ .num_irqs = ARRAY_SIZE(max77541_adc_irqs),
+};
+
+static const struct mfd_cell max77540_devs[] = {
+ MFD_CELL_OF("max77540-regulator", NULL, NULL, 0, 0, NULL),
+};
+
+static const struct mfd_cell max77541_devs[] = {
+ MFD_CELL_OF("max77541-regulator", NULL, NULL, 0, 0, NULL),
+ MFD_CELL_OF("max77541-adc", NULL, NULL, 0, 0, NULL),
+};
+
+static int max77541_pmic_irq_init(struct device *dev)
+{
+ struct max77541 *max77541 = dev_get_drvdata(dev);
+ int irq = max77541->i2c->irq;
+ int ret;
+
+ ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+ IRQF_ONESHOT | IRQF_SHARED, 0,
+ &max77541_src_irq_chip,
+ &max77541->irq_data);
+ if (ret)
+ return ret;
+
+ ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+ IRQF_ONESHOT | IRQF_SHARED, 0,
+ &max77541_topsys_irq_chip,
+ &max77541->irq_topsys);
+ if (ret)
+ return ret;
+
+ ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+ IRQF_ONESHOT | IRQF_SHARED, 0,
+ &max77541_buck_irq_chip,
+ &max77541->irq_buck);
+ if (ret)
+ return ret;
+
+ if (max77541->id == MAX77541) {
+ ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+ IRQF_ONESHOT | IRQF_SHARED, 0,
+ &max77541_adc_irq_chip,
+ &max77541->irq_adc);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int max77541_pmic_setup(struct device *dev)
+{
+ struct max77541 *max77541 = dev_get_drvdata(dev);
+ const struct mfd_cell *cells;
+ int n_devs;
+ int ret;
+
+ switch (max77541->id) {
+ case MAX77540:
+ cells = max77540_devs;
+ n_devs = ARRAY_SIZE(max77540_devs);
+ break;
+ case MAX77541:
+ cells = max77541_devs;
+ n_devs = ARRAY_SIZE(max77541_devs);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = max77541_pmic_irq_init(dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to initialize IRQ\n");
+
+ ret = device_init_wakeup(dev, true);
+ if (ret)
+ return dev_err_probe(dev, ret, "Unable to init wakeup\n");
+
+ return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
+ cells, n_devs, NULL, 0, NULL);
+}
+
+static int max77541_probe(struct i2c_client *client)
+{
+ const struct i2c_device_id *id = i2c_client_get_device_id(client);
+ struct device *dev = &client->dev;
+ struct max77541 *max77541;
+
+ max77541 = devm_kzalloc(dev, sizeof(*max77541), GFP_KERNEL);
+ if (!max77541)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, max77541);
+ max77541->i2c = client;
+
+ max77541->id = (enum max7754x_ids)device_get_match_data(dev);
+ if (!max77541->id)
+ max77541->id = (enum max7754x_ids)id->driver_data;
+
+ if (!max77541->id)
+ return -EINVAL;
+
+ max77541->regmap = devm_regmap_init_i2c(client,
+ &max77541_regmap_config);
+ if (IS_ERR(max77541->regmap))
+ return dev_err_probe(dev, PTR_ERR(max77541->regmap),
+ "Failed to allocate register map\n");
+
+ return max77541_pmic_setup(dev);
+}
+
+static const struct of_device_id max77541_of_id[] = {
+ {
+ .compatible = "adi,max77540",
+ .data = (void *)MAX77540,
+ },
+ {
+ .compatible = "adi,max77541",
+ .data = (void *)MAX77541,
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max77541_of_id);
+
+static const struct i2c_device_id max77541_id[] = {
+ { "max77540", MAX77540 },
+ { "max77541", MAX77541 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max77541_id);
+
+static struct i2c_driver max77541_driver = {
+ .driver = {
+ .name = "max77541",
+ .of_match_table = max77541_of_id,
+ },
+ .probe_new = max77541_probe,
+ .id_table = max77541_id,
+};
+module_i2c_driver(max77541_driver);
+
+MODULE_DESCRIPTION("MAX7740/MAX7741 Driver");
+MODULE_AUTHOR("Okan Sahin <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/max77541.h b/include/linux/mfd/max77541.h
new file mode 100644
index 000000000000..fe5c0a3dc637
--- /dev/null
+++ b/include/linux/mfd/max77541.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __MFD_MAX77541_H
+#define __MFD_MAX77541_H
+
+#include <linux/bits.h>
+#include <linux/types.h>
+
+/* REGISTERS */
+#define MAX77541_REG_INT_SRC 0x00
+#define MAX77541_REG_INT_SRC_M 0x01
+
+#define MAX77541_BIT_INT_SRC_TOPSYS BIT(0)
+#define MAX77541_BIT_INT_SRC_BUCK BIT(1)
+
+#define MAX77541_REG_TOPSYS_INT 0x02
+#define MAX77541_REG_TOPSYS_INT_M 0x03
+
+#define MAX77541_BIT_TOPSYS_INT_TJ_120C BIT(0)
+#define MAX77541_BIT_TOPSYS_INT_TJ_140C BIT(1)
+#define MAX77541_BIT_TOPSYS_INT_TSHDN BIT(2)
+#define MAX77541_BIT_TOPSYS_INT_UVLO BIT(3)
+#define MAX77541_BIT_TOPSYS_INT_ALT_SWO BIT(4)
+#define MAX77541_BIT_TOPSYS_INT_EXT_FREQ_DET BIT(5)
+
+/* REGULATORS */
+#define MAX77541_REG_BUCK_INT 0x20
+#define MAX77541_REG_BUCK_INT_M 0x21
+
+#define MAX77541_BIT_BUCK_INT_M1_POK_FLT BIT(0)
+#define MAX77541_BIT_BUCK_INT_M2_POK_FLT BIT(1)
+#define MAX77541_BIT_BUCK_INT_M1_SCFLT BIT(4)
+#define MAX77541_BIT_BUCK_INT_M2_SCFLT BIT(5)
+
+#define MAX77541_REG_EN_CTRL 0x0B
+
+#define MAX77541_BIT_M1_EN BIT(0)
+#define MAX77541_BIT_M2_EN BIT(1)
+
+#define MAX77541_REG_M1_VOUT 0x23
+#define MAX77541_REG_M2_VOUT 0x33
+
+#define MAX77541_BITS_MX_VOUT GENMASK(7, 0)
+
+#define MAX77541_REG_M1_CFG1 0x25
+#define MAX77541_REG_M2_CFG1 0x35
+
+#define MAX77541_BITS_MX_CFG1_RNG GENMASK(7, 6)
+
+/* ADC */
+#define MAX77541_REG_ADC_INT 0x70
+#define MAX77541_REG_ADC_INT_M 0x71
+
+#define MAX77541_BIT_ADC_INT_CH1_I BIT(0)
+#define MAX77541_BIT_ADC_INT_CH2_I BIT(1)
+#define MAX77541_BIT_ADC_INT_CH3_I BIT(2)
+#define MAX77541_BIT_ADC_INT_CH6_I BIT(5)
+
+#define MAX77541_REG_ADC_DATA_CH1 0x72
+#define MAX77541_REG_ADC_DATA_CH2 0x73
+#define MAX77541_REG_ADC_DATA_CH3 0x74
+#define MAX77541_REG_ADC_DATA_CH6 0x77
+
+/* INTERRUPT MASKS*/
+#define MAX77541_REG_INT_SRC_MASK 0x00
+#define MAX77541_REG_TOPSYS_INT_MASK 0x00
+#define MAX77541_REG_BUCK_INT_MASK 0x00
+
+#define MAX77541_MAX_REGULATORS 2
+
+enum max7754x_ids {
+ MAX77540 = 1,
+ MAX77541,
+};
+
+struct regmap;
+struct regmap_irq_chip_data;
+struct i2c_client;
+
+struct max77541 {
+ struct i2c_client *i2c;
+ struct regmap *regmap;
+ enum max7754x_ids id;
+
+ struct regmap_irq_chip_data *irq_data;
+ struct regmap_irq_chip_data *irq_buck;
+ struct regmap_irq_chip_data *irq_topsys;
+ struct regmap_irq_chip_data *irq_adc;
+};
+
+#endif /* __MFD_MAX77541_H */
--
2.30.2

2023-04-12 13:10:49

by Sahin, Okan

[permalink] [raw]
Subject: [PATCH v7 4/5] dt-bindings: mfd: max77541: Add ADI MAX77541/MAX77540

Add ADI MAX77541/MAX77540 devicetree document.

Signed-off-by: Okan Sahin <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../devicetree/bindings/mfd/adi,max77541.yaml | 68 +++++++++++++++++++
1 file changed, 68 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77541.yaml

diff --git a/Documentation/devicetree/bindings/mfd/adi,max77541.yaml b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
new file mode 100644
index 000000000000..c7895b2c38c9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/adi,max77541.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MAX77540/MAX77541 PMIC from ADI
+
+maintainers:
+ - Okan Sahin <[email protected]>
+
+description: |
+ MAX77540 is a Power Management IC with 2 buck regulators.
+
+ MAX77541 is a Power Management IC with 2 buck regulators and 1 ADC.
+
+properties:
+ compatible:
+ enum:
+ - adi,max77540
+ - adi,max77541
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ regulators:
+ $ref: /schemas/regulator/adi,max77541-regulator.yaml#
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmic@69 {
+ compatible = "adi,max77541";
+ reg = <0x69>;
+ interrupt-parent = <&gpio>;
+ interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+
+ regulators {
+ buck1 {
+ regulator-min-microvolt = <500000>;
+ regulator-max-microvolt = <5200000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+ buck2 {
+ regulator-min-microvolt = <500000>;
+ regulator-max-microvolt = <5200000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+ };
+ };
+ };
--
2.30.2

2023-04-12 13:42:34

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Wed, Apr 12, 2023 at 02:12:46PM +0300, Okan Sahin wrote:
> MFD driver for MAX77541/MAX77540 to enable its sub
> devices.
>
> The MAX77541 is a multi-function devices. It includes
> buck converter and ADC.
>
> The MAX77540 is a high-efficiency buck converter
> with two 3A switching phases.
>
> They have same regmap except for ADC part of MAX77541.

Since Lee should be happy with your change for ID tables, I'm fine with the
rest and altogether.

Reviewed-by: Andy Shevchenko <[email protected]>

> Signed-off-by: Okan Sahin <[email protected]>
> ---
> drivers/mfd/Kconfig | 13 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/max77541.c | 224 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/max77541.h | 91 ++++++++++++++
> 4 files changed, 329 insertions(+)
> create mode 100644 drivers/mfd/max77541.c
> create mode 100644 include/linux/mfd/max77541.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index fcc141e067b9..de89245ce1cb 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -777,6 +777,19 @@ config MFD_MAX14577
> additional drivers must be enabled in order to use the functionality
> of the device.
>
> +config MFD_MAX77541
> + tristate "Analog Devices MAX77541/77540 PMIC Support"
> + depends on I2C=y
> + select MFD_CORE
> + select REGMAP_I2C
> + select REGMAP_IRQ
> + help
> + Say yes here to add support for Analog Devices MAX77541 and
> + MAX77540 Power Management ICs. This driver provides
> + common support for accessing the device; additional drivers
> + must be enabled in order to use the functionality of the device.
> + There are regulators and adc.
> +
> config MFD_MAX77620
> bool "Maxim Semiconductor MAX77620 and MAX20024 PMIC Support"
> depends on I2C=y
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 2f6c89d1e277..1c8540ddead6 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -151,6 +151,7 @@ obj-$(CONFIG_MFD_DA9063) += da9063.o
> obj-$(CONFIG_MFD_DA9150) += da9150-core.o
>
> obj-$(CONFIG_MFD_MAX14577) += max14577.o
> +obj-$(CONFIG_MFD_MAX77541) += max77541.o
> obj-$(CONFIG_MFD_MAX77620) += max77620.o
> obj-$(CONFIG_MFD_MAX77650) += max77650.o
> obj-$(CONFIG_MFD_MAX77686) += max77686.o
> diff --git a/drivers/mfd/max77541.c b/drivers/mfd/max77541.c
> new file mode 100644
> index 000000000000..4a3bad3493b3
> --- /dev/null
> +++ b/drivers/mfd/max77541.c
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2022 Analog Devices, Inc.
> + * Driver for the MAX77540 and MAX77541
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max77541.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +static const struct regmap_config max77541_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static const struct regmap_irq max77541_src_irqs[] = {
> + { .mask = MAX77541_BIT_INT_SRC_TOPSYS },
> + { .mask = MAX77541_BIT_INT_SRC_BUCK },
> +};
> +
> +static const struct regmap_irq_chip max77541_src_irq_chip = {
> + .name = "max77541-src",
> + .status_base = MAX77541_REG_INT_SRC,
> + .mask_base = MAX77541_REG_INT_SRC_M,
> + .num_regs = 1,
> + .irqs = max77541_src_irqs,
> + .num_irqs = ARRAY_SIZE(max77541_src_irqs),
> +};
> +
> +static const struct regmap_irq max77541_topsys_irqs[] = {
> + { .mask = MAX77541_BIT_TOPSYS_INT_TJ_120C },
> + { .mask = MAX77541_BIT_TOPSYS_INT_TJ_140C },
> + { .mask = MAX77541_BIT_TOPSYS_INT_TSHDN },
> + { .mask = MAX77541_BIT_TOPSYS_INT_UVLO },
> + { .mask = MAX77541_BIT_TOPSYS_INT_ALT_SWO },
> + { .mask = MAX77541_BIT_TOPSYS_INT_EXT_FREQ_DET },
> +};
> +
> +static const struct regmap_irq_chip max77541_topsys_irq_chip = {
> + .name = "max77541-topsys",
> + .status_base = MAX77541_REG_TOPSYS_INT,
> + .mask_base = MAX77541_REG_TOPSYS_INT_M,
> + .num_regs = 1,
> + .irqs = max77541_topsys_irqs,
> + .num_irqs = ARRAY_SIZE(max77541_topsys_irqs),
> +};
> +
> +static const struct regmap_irq max77541_buck_irqs[] = {
> + { .mask = MAX77541_BIT_BUCK_INT_M1_POK_FLT },
> + { .mask = MAX77541_BIT_BUCK_INT_M2_POK_FLT },
> + { .mask = MAX77541_BIT_BUCK_INT_M1_SCFLT },
> + { .mask = MAX77541_BIT_BUCK_INT_M2_SCFLT },
> +};
> +
> +static const struct regmap_irq_chip max77541_buck_irq_chip = {
> + .name = "max77541-buck",
> + .status_base = MAX77541_REG_BUCK_INT,
> + .mask_base = MAX77541_REG_BUCK_INT_M,
> + .num_regs = 1,
> + .irqs = max77541_buck_irqs,
> + .num_irqs = ARRAY_SIZE(max77541_buck_irqs),
> +};
> +
> +static const struct regmap_irq max77541_adc_irqs[] = {
> + { .mask = MAX77541_BIT_ADC_INT_CH1_I },
> + { .mask = MAX77541_BIT_ADC_INT_CH2_I },
> + { .mask = MAX77541_BIT_ADC_INT_CH3_I },
> + { .mask = MAX77541_BIT_ADC_INT_CH6_I },
> +};
> +
> +static const struct regmap_irq_chip max77541_adc_irq_chip = {
> + .name = "max77541-adc",
> + .status_base = MAX77541_REG_ADC_INT,
> + .mask_base = MAX77541_REG_ADC_INT_M,
> + .num_regs = 1,
> + .irqs = max77541_adc_irqs,
> + .num_irqs = ARRAY_SIZE(max77541_adc_irqs),
> +};
> +
> +static const struct mfd_cell max77540_devs[] = {
> + MFD_CELL_OF("max77540-regulator", NULL, NULL, 0, 0, NULL),
> +};
> +
> +static const struct mfd_cell max77541_devs[] = {
> + MFD_CELL_OF("max77541-regulator", NULL, NULL, 0, 0, NULL),
> + MFD_CELL_OF("max77541-adc", NULL, NULL, 0, 0, NULL),
> +};
> +
> +static int max77541_pmic_irq_init(struct device *dev)
> +{
> + struct max77541 *max77541 = dev_get_drvdata(dev);
> + int irq = max77541->i2c->irq;
> + int ret;
> +
> + ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
> + IRQF_ONESHOT | IRQF_SHARED, 0,
> + &max77541_src_irq_chip,
> + &max77541->irq_data);
> + if (ret)
> + return ret;
> +
> + ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
> + IRQF_ONESHOT | IRQF_SHARED, 0,
> + &max77541_topsys_irq_chip,
> + &max77541->irq_topsys);
> + if (ret)
> + return ret;
> +
> + ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
> + IRQF_ONESHOT | IRQF_SHARED, 0,
> + &max77541_buck_irq_chip,
> + &max77541->irq_buck);
> + if (ret)
> + return ret;
> +
> + if (max77541->id == MAX77541) {
> + ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
> + IRQF_ONESHOT | IRQF_SHARED, 0,
> + &max77541_adc_irq_chip,
> + &max77541->irq_adc);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int max77541_pmic_setup(struct device *dev)
> +{
> + struct max77541 *max77541 = dev_get_drvdata(dev);
> + const struct mfd_cell *cells;
> + int n_devs;
> + int ret;
> +
> + switch (max77541->id) {
> + case MAX77540:
> + cells = max77540_devs;
> + n_devs = ARRAY_SIZE(max77540_devs);
> + break;
> + case MAX77541:
> + cells = max77541_devs;
> + n_devs = ARRAY_SIZE(max77541_devs);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = max77541_pmic_irq_init(dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to initialize IRQ\n");
> +
> + ret = device_init_wakeup(dev, true);
> + if (ret)
> + return dev_err_probe(dev, ret, "Unable to init wakeup\n");
> +
> + return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> + cells, n_devs, NULL, 0, NULL);
> +}
> +
> +static int max77541_probe(struct i2c_client *client)
> +{
> + const struct i2c_device_id *id = i2c_client_get_device_id(client);
> + struct device *dev = &client->dev;
> + struct max77541 *max77541;
> +
> + max77541 = devm_kzalloc(dev, sizeof(*max77541), GFP_KERNEL);
> + if (!max77541)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, max77541);
> + max77541->i2c = client;
> +
> + max77541->id = (enum max7754x_ids)device_get_match_data(dev);
> + if (!max77541->id)
> + max77541->id = (enum max7754x_ids)id->driver_data;
> +
> + if (!max77541->id)
> + return -EINVAL;
> +
> + max77541->regmap = devm_regmap_init_i2c(client,
> + &max77541_regmap_config);
> + if (IS_ERR(max77541->regmap))
> + return dev_err_probe(dev, PTR_ERR(max77541->regmap),
> + "Failed to allocate register map\n");
> +
> + return max77541_pmic_setup(dev);
> +}
> +
> +static const struct of_device_id max77541_of_id[] = {
> + {
> + .compatible = "adi,max77540",
> + .data = (void *)MAX77540,
> + },
> + {
> + .compatible = "adi,max77541",
> + .data = (void *)MAX77541,
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max77541_of_id);
> +
> +static const struct i2c_device_id max77541_id[] = {
> + { "max77540", MAX77540 },
> + { "max77541", MAX77541 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, max77541_id);
> +
> +static struct i2c_driver max77541_driver = {
> + .driver = {
> + .name = "max77541",
> + .of_match_table = max77541_of_id,
> + },
> + .probe_new = max77541_probe,
> + .id_table = max77541_id,
> +};
> +module_i2c_driver(max77541_driver);
> +
> +MODULE_DESCRIPTION("MAX7740/MAX7741 Driver");
> +MODULE_AUTHOR("Okan Sahin <[email protected]>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/max77541.h b/include/linux/mfd/max77541.h
> new file mode 100644
> index 000000000000..fe5c0a3dc637
> --- /dev/null
> +++ b/include/linux/mfd/max77541.h
> @@ -0,0 +1,91 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef __MFD_MAX77541_H
> +#define __MFD_MAX77541_H
> +
> +#include <linux/bits.h>
> +#include <linux/types.h>
> +
> +/* REGISTERS */
> +#define MAX77541_REG_INT_SRC 0x00
> +#define MAX77541_REG_INT_SRC_M 0x01
> +
> +#define MAX77541_BIT_INT_SRC_TOPSYS BIT(0)
> +#define MAX77541_BIT_INT_SRC_BUCK BIT(1)
> +
> +#define MAX77541_REG_TOPSYS_INT 0x02
> +#define MAX77541_REG_TOPSYS_INT_M 0x03
> +
> +#define MAX77541_BIT_TOPSYS_INT_TJ_120C BIT(0)
> +#define MAX77541_BIT_TOPSYS_INT_TJ_140C BIT(1)
> +#define MAX77541_BIT_TOPSYS_INT_TSHDN BIT(2)
> +#define MAX77541_BIT_TOPSYS_INT_UVLO BIT(3)
> +#define MAX77541_BIT_TOPSYS_INT_ALT_SWO BIT(4)
> +#define MAX77541_BIT_TOPSYS_INT_EXT_FREQ_DET BIT(5)
> +
> +/* REGULATORS */
> +#define MAX77541_REG_BUCK_INT 0x20
> +#define MAX77541_REG_BUCK_INT_M 0x21
> +
> +#define MAX77541_BIT_BUCK_INT_M1_POK_FLT BIT(0)
> +#define MAX77541_BIT_BUCK_INT_M2_POK_FLT BIT(1)
> +#define MAX77541_BIT_BUCK_INT_M1_SCFLT BIT(4)
> +#define MAX77541_BIT_BUCK_INT_M2_SCFLT BIT(5)
> +
> +#define MAX77541_REG_EN_CTRL 0x0B
> +
> +#define MAX77541_BIT_M1_EN BIT(0)
> +#define MAX77541_BIT_M2_EN BIT(1)
> +
> +#define MAX77541_REG_M1_VOUT 0x23
> +#define MAX77541_REG_M2_VOUT 0x33
> +
> +#define MAX77541_BITS_MX_VOUT GENMASK(7, 0)
> +
> +#define MAX77541_REG_M1_CFG1 0x25
> +#define MAX77541_REG_M2_CFG1 0x35
> +
> +#define MAX77541_BITS_MX_CFG1_RNG GENMASK(7, 6)
> +
> +/* ADC */
> +#define MAX77541_REG_ADC_INT 0x70
> +#define MAX77541_REG_ADC_INT_M 0x71
> +
> +#define MAX77541_BIT_ADC_INT_CH1_I BIT(0)
> +#define MAX77541_BIT_ADC_INT_CH2_I BIT(1)
> +#define MAX77541_BIT_ADC_INT_CH3_I BIT(2)
> +#define MAX77541_BIT_ADC_INT_CH6_I BIT(5)
> +
> +#define MAX77541_REG_ADC_DATA_CH1 0x72
> +#define MAX77541_REG_ADC_DATA_CH2 0x73
> +#define MAX77541_REG_ADC_DATA_CH3 0x74
> +#define MAX77541_REG_ADC_DATA_CH6 0x77
> +
> +/* INTERRUPT MASKS*/
> +#define MAX77541_REG_INT_SRC_MASK 0x00
> +#define MAX77541_REG_TOPSYS_INT_MASK 0x00
> +#define MAX77541_REG_BUCK_INT_MASK 0x00
> +
> +#define MAX77541_MAX_REGULATORS 2
> +
> +enum max7754x_ids {
> + MAX77540 = 1,
> + MAX77541,
> +};
> +
> +struct regmap;
> +struct regmap_irq_chip_data;
> +struct i2c_client;
> +
> +struct max77541 {
> + struct i2c_client *i2c;
> + struct regmap *regmap;
> + enum max7754x_ids id;
> +
> + struct regmap_irq_chip_data *irq_data;
> + struct regmap_irq_chip_data *irq_buck;
> + struct regmap_irq_chip_data *irq_topsys;
> + struct regmap_irq_chip_data *irq_adc;
> +};
> +
> +#endif /* __MFD_MAX77541_H */
> --
> 2.30.2
>

--
With Best Regards,
Andy Shevchenko


2023-04-12 13:43:17

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v7 3/5] iio: adc: max77541: Add ADI MAX77541 ADC Support

On Wed, Apr 12, 2023 at 02:12:44PM +0300, Okan Sahin wrote:
> The MAX77541 has an 8-bit Successive Approximation Register (SAR) ADC
> with four multiplexers for supporting the telemetry feature.

Reviewed-by: Andy Shevchenko <[email protected]>

> Signed-off-by: Okan Sahin <[email protected]>
> Reviewed-by: Jonathan Cameron <[email protected]>
> ---
> drivers/iio/adc/Kconfig | 11 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/max77541-adc.c | 194 +++++++++++++++++++++++++++++++++
> 3 files changed, 206 insertions(+)
> create mode 100644 drivers/iio/adc/max77541-adc.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 45af2302be53..518e7bd453aa 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -735,6 +735,17 @@ config MAX1363
> To compile this driver as a module, choose M here: the module will be
> called max1363.
>
> +config MAX77541_ADC
> + tristate "Analog Devices MAX77541 ADC driver"
> + depends on MFD_MAX77541
> + help
> + This driver controls a Analog Devices MAX77541 ADC
> + via I2C bus. This device has one adc. Say yes here to build
> + support for Analog Devices MAX77541 ADC interface.
> +
> + To compile this driver as a module, choose M here:
> + the module will be called max77541-adc.
> +
> config MAX9611
> tristate "Maxim max9611/max9612 ADC driver"
> depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 36c18177322a..f8433b560c3b 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -67,6 +67,7 @@ obj-$(CONFIG_MAX11205) += max11205.o
> obj-$(CONFIG_MAX11410) += max11410.o
> obj-$(CONFIG_MAX1241) += max1241.o
> obj-$(CONFIG_MAX1363) += max1363.o
> +obj-$(CONFIG_MAX77541_ADC) += max77541-adc.o
> obj-$(CONFIG_MAX9611) += max9611.o
> obj-$(CONFIG_MCP320X) += mcp320x.o
> obj-$(CONFIG_MCP3422) += mcp3422.o
> diff --git a/drivers/iio/adc/max77541-adc.c b/drivers/iio/adc/max77541-adc.c
> new file mode 100644
> index 000000000000..21d024bde16b
> --- /dev/null
> +++ b/drivers/iio/adc/max77541-adc.c
> @@ -0,0 +1,194 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2022 Analog Devices, Inc.
> + * ADI MAX77541 ADC Driver with IIO interface
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/iio/iio.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/units.h>
> +
> +#include <linux/mfd/max77541.h>
> +
> +enum max77541_adc_range {
> + LOW_RANGE,
> + MID_RANGE,
> + HIGH_RANGE,
> +};
> +
> +enum max77541_adc_channel {
> + MAX77541_ADC_VSYS_V,
> + MAX77541_ADC_VOUT1_V,
> + MAX77541_ADC_VOUT2_V,
> + MAX77541_ADC_TEMP,
> +};
> +
> +static int max77541_adc_offset(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2)
> +{
> + switch (chan->channel) {
> + case MAX77541_ADC_TEMP:
> + *val = DIV_ROUND_CLOSEST(ABSOLUTE_ZERO_MILLICELSIUS, 1725);
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int max77541_adc_scale(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2)
> +{
> + struct regmap **regmap = iio_priv(indio_dev);
> + unsigned int reg_val;
> + int ret;
> +
> + switch (chan->channel) {
> + case MAX77541_ADC_VSYS_V:
> + *val = 25;
> + return IIO_VAL_INT;
> + case MAX77541_ADC_VOUT1_V:
> + case MAX77541_ADC_VOUT2_V:
> + ret = regmap_read(*regmap, MAX77541_REG_M2_CFG1, &reg_val);
> + if (ret)
> + return ret;
> +
> + reg_val = FIELD_GET(MAX77541_BITS_MX_CFG1_RNG, reg_val);
> + switch (reg_val) {
> + case LOW_RANGE:
> + *val = 6;
> + *val2 = 250000;
> + break;
> + case MID_RANGE:
> + *val = 12;
> + *val2 = 500000;
> + break;
> + case HIGH_RANGE:
> + *val = 25;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> + case MAX77541_ADC_TEMP:
> + *val = 1725;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int max77541_adc_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val)
> +{
> + struct regmap **regmap = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regmap_read(*regmap, chan->address, val);
> + if (ret)
> + return ret;
> +
> + return IIO_VAL_INT;
> +}
> +
> +#define MAX77541_ADC_CHANNEL_V(_channel, _name, _type, _reg) \
> + { \
> + .type = _type, \
> + .indexed = 1, \
> + .channel = _channel, \
> + .address = _reg, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .datasheet_name = _name, \
> + }
> +
> +#define MAX77541_ADC_CHANNEL_TEMP(_channel, _name, _type, _reg) \
> + { \
> + .type = _type, \
> + .indexed = 1, \
> + .channel = _channel, \
> + .address = _reg, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE) |\
> + BIT(IIO_CHAN_INFO_OFFSET),\
> + .datasheet_name = _name, \
> + }
> +
> +static const struct iio_chan_spec max77541_adc_channels[] = {
> + MAX77541_ADC_CHANNEL_V(MAX77541_ADC_VSYS_V, "vsys_v", IIO_VOLTAGE,
> + MAX77541_REG_ADC_DATA_CH1),
> + MAX77541_ADC_CHANNEL_V(MAX77541_ADC_VOUT1_V, "vout1_v", IIO_VOLTAGE,
> + MAX77541_REG_ADC_DATA_CH2),
> + MAX77541_ADC_CHANNEL_V(MAX77541_ADC_VOUT2_V, "vout2_v", IIO_VOLTAGE,
> + MAX77541_REG_ADC_DATA_CH3),
> + MAX77541_ADC_CHANNEL_TEMP(MAX77541_ADC_TEMP, "temp", IIO_TEMP,
> + MAX77541_REG_ADC_DATA_CH6),
> +};
> +
> +static int max77541_adc_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 max77541_adc_offset(indio_dev, chan, val, val2);
> + case IIO_CHAN_INFO_SCALE:
> + return max77541_adc_scale(indio_dev, chan, val, val2);
> + case IIO_CHAN_INFO_RAW:
> + return max77541_adc_raw(indio_dev, chan, val);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info max77541_adc_info = {
> + .read_raw = max77541_adc_read_raw,
> +};
> +
> +static int max77541_adc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct iio_dev *indio_dev;
> + struct regmap **regmap;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*regmap));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + regmap = iio_priv(indio_dev);
> +
> + *regmap = dev_get_regmap(dev->parent, NULL);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + indio_dev->name = "max77541";
> + indio_dev->info = &max77541_adc_info;
> + indio_dev->channels = max77541_adc_channels;
> + indio_dev->num_channels = ARRAY_SIZE(max77541_adc_channels);
> +
> + return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static const struct platform_device_id max77541_adc_platform_id[] = {
> + { "max77541-adc" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, max77541_adc_platform_id);
> +
> +static struct platform_driver max77541_adc_driver = {
> + .driver = {
> + .name = "max77541-adc",
> + },
> + .probe = max77541_adc_probe,
> + .id_table = max77541_adc_platform_id,
> +};
> +module_platform_driver(max77541_adc_driver);
> +
> +MODULE_AUTHOR("Okan Sahin <[email protected]>");
> +MODULE_DESCRIPTION("MAX77541 ADC driver");
> +MODULE_LICENSE("GPL");
> --
> 2.30.2
>

--
With Best Regards,
Andy Shevchenko


2023-04-20 10:46:37

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Wed, 12 Apr 2023, Okan Sahin wrote:

> MFD driver for MAX77541/MAX77540 to enable its sub
> devices.
>
> The MAX77541 is a multi-function devices. It includes
> buck converter and ADC.
>
> The MAX77540 is a high-efficiency buck converter
> with two 3A switching phases.
>
> They have same regmap except for ADC part of MAX77541.
>
> Signed-off-by: Okan Sahin <[email protected]>
> ---
> drivers/mfd/Kconfig | 13 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/max77541.c | 224 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/max77541.h | 91 ++++++++++++++
> 4 files changed, 329 insertions(+)
> create mode 100644 drivers/mfd/max77541.c
> create mode 100644 include/linux/mfd/max77541.h

Looks good.

Once the regulator driver has been reviewed, I can take the set.

Please apply this if you have to resubmit:

For my own reference (apply this as-is to your sign-off block):

Acked-for-MFD-by: Lee Jones <[email protected]>

--
Lee Jones [李琼斯]

2023-04-20 16:55:55

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Thu, Apr 20, 2023 at 11:34:38AM +0100, Lee Jones wrote:

> Once the regulator driver has been reviewed, I can take the set.

> Please apply this if you have to resubmit:

> For my own reference (apply this as-is to your sign-off block):

> Acked-for-MFD-by: Lee Jones <[email protected]>

For situations like this where there's a depends on to the MFD it'd be
great if you could just apply the MFD rather than waiting, the
individual drivers can either get applied on top or just go via the
subsystem and have everything sort itself out in the merge window. It'd
help things move along faster and be less confusing.

These serieses tend to get so many resends that I'm often just not
looking at them, previously I'd have just applied the function driver
when it's ready but with the complaints when the core ends up missing
the merge window but function drivers are going in I stopped. In the
past I've ended up missing things because either there's multiple
serieses for similarly named devices out at once or (less often) some
change results in a repeat review being needed so it's easier to just
wait for things to settle down.


Attachments:
(No filename) (1.13 kB)
signature.asc (499.00 B)
Download all attachments

2023-04-21 07:50:19

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Thu, 20 Apr 2023, Mark Brown wrote:

> On Thu, Apr 20, 2023 at 11:34:38AM +0100, Lee Jones wrote:
>
> > Once the regulator driver has been reviewed, I can take the set.
>
> > Please apply this if you have to resubmit:
>
> > For my own reference (apply this as-is to your sign-off block):
>
> > Acked-for-MFD-by: Lee Jones <[email protected]>
>
> For situations like this where there's a depends on to the MFD it'd be
> great if you could just apply the MFD rather than waiting, the
> individual drivers can either get applied on top or just go via the
> subsystem and have everything sort itself out in the merge window. It'd
> help things move along faster and be less confusing.
>
> These serieses tend to get so many resends that I'm often just not
> looking at them, previously I'd have just applied the function driver
> when it's ready but with the complaints when the core ends up missing
> the merge window but function drivers are going in I stopped. In the
> past I've ended up missing things because either there's multiple
> serieses for similarly named devices out at once or (less often) some
> change results in a repeat review being needed so it's easier to just
> wait for things to settle down.

I'll try anything once!

Fair warning, I think this is going to massively complicate things.

Either we're going to be left with a situation where child-driver
maintainers are scrabbling around looking for previous versions for the
MFD pull-request or contributors being forced to wait a full cycle for
their dependencies to arrive in the maintainer's base.

I'm not sure why simply providing your Ack when you're happy with the
driver and forgetting about the set until the pull-request arrives, like
we've been doing for nearly a decade now, isn't working for you anymore
but I'm mostly sure this method will be a regression.

--
Lee Jones [李琼斯]

2023-04-21 13:41:39

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Fri, Apr 21, 2023 at 08:39:38AM +0100, Lee Jones wrote:

> I'll try anything once!

> Fair warning, I think this is going to massively complicate things.

> Either we're going to be left with a situation where child-driver
> maintainers are scrabbling around looking for previous versions for the
> MFD pull-request or contributors being forced to wait a full cycle for
> their dependencies to arrive in the maintainer's base.

If people are resending after the MFD has gone in they really ought to
be including the pull request in the cover letter, with some combination
of either referencing the mail or just saying "this depends on the
signed tag at url+tag", the same way they would for any other dependency.

I can't see how you applying stuff when you can slow things down TBH,
the MFD bits will be applied faster and either people can pull in a
shared tag or you can apply more commits on top of the existing core
driver.

> I'm not sure why simply providing your Ack when you're happy with the
> driver and forgetting about the set until the pull-request arrives, like
> we've been doing for nearly a decade now, isn't working for you anymore
> but I'm mostly sure this method will be a regression.

Like I said I've not been doing that, I've mostly been just applying the
driver when it's ready. This might not have been so visible to you
since it means that the regulator driver doesn't appear in the series by
the time the MFD settles down. The whole "Acked-for-MFD" has always
been a bit confusing TBH, it's not a normal ack ("go ahead and apply
this, I'm fine with it") so it was never clear what the intention was.

Before I started just applying the drivers there used to be constant
problems with things like tags going missing (which some of the time is
the submitter just not carrying them but can also be the result of some
churn causing them to be deliberately dropped due to changes) or
forgetting the series as you suggest and then not looking at some other
very similarly named series that was also getting lots of versions after
thinking it was one that had been reviewed already. It was all very
frustrating. Not doing the tags until the dependencies have settled
down means that if it's in my inbox it at least consistently needs some
kind of attention and that the submitter didn't drop tags or anything so
I know why there's no tag on it even though the version number is high,
though it's not ideal either.


Attachments:
(No filename) (2.43 kB)
signature.asc (499.00 B)
Download all attachments

2023-06-13 07:41:56

by Sahin, Okan

[permalink] [raw]
Subject: RE: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

>On Fri, Apr 21, 2023 at 08:39:38AM +0100, Lee Jones wrote:
>
>> I'll try anything once!
>
>> Fair warning, I think this is going to massively complicate things.
>
>> Either we're going to be left with a situation where child-driver
>> maintainers are scrabbling around looking for previous versions for the
>> MFD pull-request or contributors being forced to wait a full cycle for
>> their dependencies to arrive in the maintainer's base.
>
>If people are resending after the MFD has gone in they really ought to
>be including the pull request in the cover letter, with some combination
>of either referencing the mail or just saying "this depends on the
>signed tag at url+tag", the same way they would for any other dependency.
>
>I can't see how you applying stuff when you can slow things down TBH,
>the MFD bits will be applied faster and either people can pull in a
>shared tag or you can apply more commits on top of the existing core
>driver.
>
>> I'm not sure why simply providing your Ack when you're happy with the
>> driver and forgetting about the set until the pull-request arrives, like
>> we've been doing for nearly a decade now, isn't working for you anymore
>> but I'm mostly sure this method will be a regression.
>
>Like I said I've not been doing that, I've mostly been just applying the
>driver when it's ready. This might not have been so visible to you
>since it means that the regulator driver doesn't appear in the series by
>the time the MFD settles down. The whole "Acked-for-MFD" has always
>been a bit confusing TBH, it's not a normal ack ("go ahead and apply
>this, I'm fine with it") so it was never clear what the intention was.
>
>Before I started just applying the drivers there used to be constant
>problems with things like tags going missing (which some of the time is
>the submitter just not carrying them but can also be the result of some
>churn causing them to be deliberately dropped due to changes) or
>forgetting the series as you suggest and then not looking at some other
>very similarly named series that was also getting lots of versions after
>thinking it was one that had been reviewed already. It was all very
>frustrating. Not doing the tags until the dependencies have settled
>down means that if it's in my inbox it at least consistently needs some
>kind of attention and that the submitter didn't drop tags or anything so
>I know why there's no tag on it even though the version number is high,
>though it's not ideal either.

Hi Mark and Lee,

Is there anything that I need to do for this patch set. I have received reviewed
by tag for all of them so far.

Regards,
Okan Sahin

2023-06-21 17:34:18

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Tue, 13 Jun 2023, Sahin, Okan wrote:

> >On Fri, Apr 21, 2023 at 08:39:38AM +0100, Lee Jones wrote:
> >
> >> I'll try anything once!
> >
> >> Fair warning, I think this is going to massively complicate things.
> >
> >> Either we're going to be left with a situation where child-driver
> >> maintainers are scrabbling around looking for previous versions for the
> >> MFD pull-request or contributors being forced to wait a full cycle for
> >> their dependencies to arrive in the maintainer's base.
> >
> >If people are resending after the MFD has gone in they really ought to
> >be including the pull request in the cover letter, with some combination
> >of either referencing the mail or just saying "this depends on the
> >signed tag at url+tag", the same way they would for any other dependency.
> >
> >I can't see how you applying stuff when you can slow things down TBH,
> >the MFD bits will be applied faster and either people can pull in a
> >shared tag or you can apply more commits on top of the existing core
> >driver.
> >
> >> I'm not sure why simply providing your Ack when you're happy with the
> >> driver and forgetting about the set until the pull-request arrives, like
> >> we've been doing for nearly a decade now, isn't working for you anymore
> >> but I'm mostly sure this method will be a regression.
> >
> >Like I said I've not been doing that, I've mostly been just applying the
> >driver when it's ready. This might not have been so visible to you
> >since it means that the regulator driver doesn't appear in the series by
> >the time the MFD settles down. The whole "Acked-for-MFD" has always
> >been a bit confusing TBH, it's not a normal ack ("go ahead and apply
> >this, I'm fine with it") so it was never clear what the intention was.
> >
> >Before I started just applying the drivers there used to be constant
> >problems with things like tags going missing (which some of the time is
> >the submitter just not carrying them but can also be the result of some
> >churn causing them to be deliberately dropped due to changes) or
> >forgetting the series as you suggest and then not looking at some other
> >very similarly named series that was also getting lots of versions after
> >thinking it was one that had been reviewed already. It was all very
> >frustrating. Not doing the tags until the dependencies have settled
> >down means that if it's in my inbox it at least consistently needs some
> >kind of attention and that the submitter didn't drop tags or anything so
> >I know why there's no tag on it even though the version number is high,
> >though it's not ideal either.
>
> Hi Mark and Lee,
>
> Is there anything that I need to do for this patch set. I have received reviewed
> by tag for all of them so far.

Since we are so late in the day, I'm going to just apply this for v6.5.

The remainder can then be applied, friction free, for v6.6.

--
Lee Jones [李琼斯]

2023-06-21 17:47:54

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Wed, 12 Apr 2023, Okan Sahin wrote:

> MFD driver for MAX77541/MAX77540 to enable its sub
> devices.
>
> The MAX77541 is a multi-function devices. It includes
> buck converter and ADC.
>
> The MAX77540 is a high-efficiency buck converter
> with two 3A switching phases.
>
> They have same regmap except for ADC part of MAX77541.
>
> Signed-off-by: Okan Sahin <[email protected]>
> ---
> drivers/mfd/Kconfig | 13 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/max77541.c | 224 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/max77541.h | 91 ++++++++++++++
> 4 files changed, 329 insertions(+)
> create mode 100644 drivers/mfd/max77541.c
> create mode 100644 include/linux/mfd/max77541.h

Applied, thanks

--
Lee Jones [李琼斯]

2023-06-26 18:14:14

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Wed, Jun 21, 2023 at 06:13:15PM +0100, Lee Jones wrote:
> On Tue, 13 Jun 2023, Sahin, Okan wrote:
>
> > >On Fri, Apr 21, 2023 at 08:39:38AM +0100, Lee Jones wrote:
> > >
> > >> I'll try anything once!
> > >
> > >> Fair warning, I think this is going to massively complicate things.
> > >
> > >> Either we're going to be left with a situation where child-driver
> > >> maintainers are scrabbling around looking for previous versions for the
> > >> MFD pull-request or contributors being forced to wait a full cycle for
> > >> their dependencies to arrive in the maintainer's base.
> > >
> > >If people are resending after the MFD has gone in they really ought to
> > >be including the pull request in the cover letter, with some combination
> > >of either referencing the mail or just saying "this depends on the
> > >signed tag at url+tag", the same way they would for any other dependency.
> > >
> > >I can't see how you applying stuff when you can slow things down TBH,
> > >the MFD bits will be applied faster and either people can pull in a
> > >shared tag or you can apply more commits on top of the existing core
> > >driver.
> > >
> > >> I'm not sure why simply providing your Ack when you're happy with the
> > >> driver and forgetting about the set until the pull-request arrives, like
> > >> we've been doing for nearly a decade now, isn't working for you anymore
> > >> but I'm mostly sure this method will be a regression.
> > >
> > >Like I said I've not been doing that, I've mostly been just applying the
> > >driver when it's ready. This might not have been so visible to you
> > >since it means that the regulator driver doesn't appear in the series by
> > >the time the MFD settles down. The whole "Acked-for-MFD" has always
> > >been a bit confusing TBH, it's not a normal ack ("go ahead and apply
> > >this, I'm fine with it") so it was never clear what the intention was.
> > >
> > >Before I started just applying the drivers there used to be constant
> > >problems with things like tags going missing (which some of the time is
> > >the submitter just not carrying them but can also be the result of some
> > >churn causing them to be deliberately dropped due to changes) or
> > >forgetting the series as you suggest and then not looking at some other
> > >very similarly named series that was also getting lots of versions after
> > >thinking it was one that had been reviewed already. It was all very
> > >frustrating. Not doing the tags until the dependencies have settled
> > >down means that if it's in my inbox it at least consistently needs some
> > >kind of attention and that the submitter didn't drop tags or anything so
> > >I know why there's no tag on it even though the version number is high,
> > >though it's not ideal either.
> >
> > Hi Mark and Lee,
> >
> > Is there anything that I need to do for this patch set. I have received reviewed
> > by tag for all of them so far.
>
> Since we are so late in the day, I'm going to just apply this for v6.5.
>
> The remainder can then be applied, friction free, for v6.6.

Now we have undocmented bindings in use by the driver (as pointed out by
'make dt_compatible_check').

The whole series has all the acks/reviews needed for you to apply the
whole thing, so why not take the whole thing? Plus this series has been
sitting for 2 months. Not a great experience for submitters...

Rob

2023-06-27 14:12:46

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Mon, 26 Jun 2023, Rob Herring wrote:

> On Wed, Jun 21, 2023 at 06:13:15PM +0100, Lee Jones wrote:
> > On Tue, 13 Jun 2023, Sahin, Okan wrote:
> >
> > > >On Fri, Apr 21, 2023 at 08:39:38AM +0100, Lee Jones wrote:
> > > >
> > > >> I'll try anything once!
> > > >
> > > >> Fair warning, I think this is going to massively complicate things.
> > > >
> > > >> Either we're going to be left with a situation where child-driver
> > > >> maintainers are scrabbling around looking for previous versions for the
> > > >> MFD pull-request or contributors being forced to wait a full cycle for
> > > >> their dependencies to arrive in the maintainer's base.
> > > >
> > > >If people are resending after the MFD has gone in they really ought to
> > > >be including the pull request in the cover letter, with some combination
> > > >of either referencing the mail or just saying "this depends on the
> > > >signed tag at url+tag", the same way they would for any other dependency.
> > > >
> > > >I can't see how you applying stuff when you can slow things down TBH,
> > > >the MFD bits will be applied faster and either people can pull in a
> > > >shared tag or you can apply more commits on top of the existing core
> > > >driver.
> > > >
> > > >> I'm not sure why simply providing your Ack when you're happy with the
> > > >> driver and forgetting about the set until the pull-request arrives, like
> > > >> we've been doing for nearly a decade now, isn't working for you anymore
> > > >> but I'm mostly sure this method will be a regression.
> > > >
> > > >Like I said I've not been doing that, I've mostly been just applying the
> > > >driver when it's ready. This might not have been so visible to you
> > > >since it means that the regulator driver doesn't appear in the series by
> > > >the time the MFD settles down. The whole "Acked-for-MFD" has always
> > > >been a bit confusing TBH, it's not a normal ack ("go ahead and apply
> > > >this, I'm fine with it") so it was never clear what the intention was.
> > > >
> > > >Before I started just applying the drivers there used to be constant
> > > >problems with things like tags going missing (which some of the time is
> > > >the submitter just not carrying them but can also be the result of some
> > > >churn causing them to be deliberately dropped due to changes) or
> > > >forgetting the series as you suggest and then not looking at some other
> > > >very similarly named series that was also getting lots of versions after
> > > >thinking it was one that had been reviewed already. It was all very
> > > >frustrating. Not doing the tags until the dependencies have settled
> > > >down means that if it's in my inbox it at least consistently needs some
> > > >kind of attention and that the submitter didn't drop tags or anything so
> > > >I know why there's no tag on it even though the version number is high,
> > > >though it's not ideal either.
> > >
> > > Hi Mark and Lee,
> > >
> > > Is there anything that I need to do for this patch set. I have received reviewed
> > > by tag for all of them so far.
> >
> > Since we are so late in the day, I'm going to just apply this for v6.5.
> >
> > The remainder can then be applied, friction free, for v6.6.
>
> Now we have undocmented bindings in use by the driver (as pointed out by
> 'make dt_compatible_check').
>
> The whole series has all the acks/reviews needed for you to apply the
> whole thing, so why not take the whole thing? Plus this series has been
> sitting for 2 months. Not a great experience for submitters...

Patches are missing Acked-by tags.

Reviewed-by != Acked-by

I cannot merge other subsystem's patches without and Acked-by.

--
Lee Jones [李琼斯]

2023-06-27 14:33:28

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Tue, Jun 27, 2023 at 7:56 AM Lee Jones <[email protected]> wrote:
>
> On Mon, 26 Jun 2023, Rob Herring wrote:
>
> > On Wed, Jun 21, 2023 at 06:13:15PM +0100, Lee Jones wrote:
> > > On Tue, 13 Jun 2023, Sahin, Okan wrote:
> > >
> > > > >On Fri, Apr 21, 2023 at 08:39:38AM +0100, Lee Jones wrote:
> > > > >
> > > > >> I'll try anything once!
> > > > >
> > > > >> Fair warning, I think this is going to massively complicate things.
> > > > >
> > > > >> Either we're going to be left with a situation where child-driver
> > > > >> maintainers are scrabbling around looking for previous versions for the
> > > > >> MFD pull-request or contributors being forced to wait a full cycle for
> > > > >> their dependencies to arrive in the maintainer's base.
> > > > >
> > > > >If people are resending after the MFD has gone in they really ought to
> > > > >be including the pull request in the cover letter, with some combination
> > > > >of either referencing the mail or just saying "this depends on the
> > > > >signed tag at url+tag", the same way they would for any other dependency.
> > > > >
> > > > >I can't see how you applying stuff when you can slow things down TBH,
> > > > >the MFD bits will be applied faster and either people can pull in a
> > > > >shared tag or you can apply more commits on top of the existing core
> > > > >driver.
> > > > >
> > > > >> I'm not sure why simply providing your Ack when you're happy with the
> > > > >> driver and forgetting about the set until the pull-request arrives, like
> > > > >> we've been doing for nearly a decade now, isn't working for you anymore
> > > > >> but I'm mostly sure this method will be a regression.
> > > > >
> > > > >Like I said I've not been doing that, I've mostly been just applying the
> > > > >driver when it's ready. This might not have been so visible to you
> > > > >since it means that the regulator driver doesn't appear in the series by
> > > > >the time the MFD settles down. The whole "Acked-for-MFD" has always
> > > > >been a bit confusing TBH, it's not a normal ack ("go ahead and apply
> > > > >this, I'm fine with it") so it was never clear what the intention was.
> > > > >
> > > > >Before I started just applying the drivers there used to be constant
> > > > >problems with things like tags going missing (which some of the time is
> > > > >the submitter just not carrying them but can also be the result of some
> > > > >churn causing them to be deliberately dropped due to changes) or
> > > > >forgetting the series as you suggest and then not looking at some other
> > > > >very similarly named series that was also getting lots of versions after
> > > > >thinking it was one that had been reviewed already. It was all very
> > > > >frustrating. Not doing the tags until the dependencies have settled
> > > > >down means that if it's in my inbox it at least consistently needs some
> > > > >kind of attention and that the submitter didn't drop tags or anything so
> > > > >I know why there's no tag on it even though the version number is high,
> > > > >though it's not ideal either.
> > > >
> > > > Hi Mark and Lee,
> > > >
> > > > Is there anything that I need to do for this patch set. I have received reviewed
> > > > by tag for all of them so far.
> > >
> > > Since we are so late in the day, I'm going to just apply this for v6.5.
> > >
> > > The remainder can then be applied, friction free, for v6.6.
> >
> > Now we have undocmented bindings in use by the driver (as pointed out by
> > 'make dt_compatible_check').
> >
> > The whole series has all the acks/reviews needed for you to apply the
> > whole thing, so why not take the whole thing? Plus this series has been
> > sitting for 2 months. Not a great experience for submitters...
>
> Patches are missing Acked-by tags.
>
> Reviewed-by != Acked-by

Reviewed-by > Acked-by

>
> I cannot merge other subsystem's patches without and Acked-by.

I (and Krzysztof) give one or the other. If I'm taking a patch, then
it's neither. I'm pretty sure Mark only gives Reviewed-by when he is
not taking something.

Rob

2023-06-27 14:37:04

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Tue, Jun 27, 2023 at 02:56:15PM +0100, Lee Jones wrote:
> On Mon, 26 Jun 2023, Rob Herring wrote:

> > The whole series has all the acks/reviews needed for you to apply the
> > whole thing, so why not take the whole thing? Plus this series has been
> > sitting for 2 months. Not a great experience for submitters...

> Patches are missing Acked-by tags.

> Reviewed-by != Acked-by

> I cannot merge other subsystem's patches without and Acked-by.

Things should be OK to merge if they've got a Reviewed-by, that's
generally treated a lot like a stronger ack and people won't tend to
bother with an ack if they've done enough that they're happy to do a
Reviewed-by.

In any case, there's no need to hold all the bits that do seem OK for
some leaf driver that has some problems, the leaf driver can always get
applied incrementally either in the same branch or in another tree based
on a cross tree merge. That way the bits that are fine can make
progress and there's less people getting the resends of the bits that
still need work.


Attachments:
(No filename) (1.04 kB)
signature.asc (499.00 B)
Download all attachments

2023-06-27 15:06:00

by William Breathitt Gray

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Tue, Jun 27, 2023 at 08:10:59AM -0600, Rob Herring wrote:
> On Tue, Jun 27, 2023 at 7:56 AM Lee Jones <[email protected]> wrote:
> >
> > On Mon, 26 Jun 2023, Rob Herring wrote:
> >
> > > On Wed, Jun 21, 2023 at 06:13:15PM +0100, Lee Jones wrote:
> > > > On Tue, 13 Jun 2023, Sahin, Okan wrote:
> > > >
> > > > > >On Fri, Apr 21, 2023 at 08:39:38AM +0100, Lee Jones wrote:
> > > > > >
> > > > > >> I'll try anything once!
> > > > > >
> > > > > >> Fair warning, I think this is going to massively complicate things.
> > > > > >
> > > > > >> Either we're going to be left with a situation where child-driver
> > > > > >> maintainers are scrabbling around looking for previous versions for the
> > > > > >> MFD pull-request or contributors being forced to wait a full cycle for
> > > > > >> their dependencies to arrive in the maintainer's base.
> > > > > >
> > > > > >If people are resending after the MFD has gone in they really ought to
> > > > > >be including the pull request in the cover letter, with some combination
> > > > > >of either referencing the mail or just saying "this depends on the
> > > > > >signed tag at url+tag", the same way they would for any other dependency.
> > > > > >
> > > > > >I can't see how you applying stuff when you can slow things down TBH,
> > > > > >the MFD bits will be applied faster and either people can pull in a
> > > > > >shared tag or you can apply more commits on top of the existing core
> > > > > >driver.
> > > > > >
> > > > > >> I'm not sure why simply providing your Ack when you're happy with the
> > > > > >> driver and forgetting about the set until the pull-request arrives, like
> > > > > >> we've been doing for nearly a decade now, isn't working for you anymore
> > > > > >> but I'm mostly sure this method will be a regression.
> > > > > >
> > > > > >Like I said I've not been doing that, I've mostly been just applying the
> > > > > >driver when it's ready. This might not have been so visible to you
> > > > > >since it means that the regulator driver doesn't appear in the series by
> > > > > >the time the MFD settles down. The whole "Acked-for-MFD" has always
> > > > > >been a bit confusing TBH, it's not a normal ack ("go ahead and apply
> > > > > >this, I'm fine with it") so it was never clear what the intention was.
> > > > > >
> > > > > >Before I started just applying the drivers there used to be constant
> > > > > >problems with things like tags going missing (which some of the time is
> > > > > >the submitter just not carrying them but can also be the result of some
> > > > > >churn causing them to be deliberately dropped due to changes) or
> > > > > >forgetting the series as you suggest and then not looking at some other
> > > > > >very similarly named series that was also getting lots of versions after
> > > > > >thinking it was one that had been reviewed already. It was all very
> > > > > >frustrating. Not doing the tags until the dependencies have settled
> > > > > >down means that if it's in my inbox it at least consistently needs some
> > > > > >kind of attention and that the submitter didn't drop tags or anything so
> > > > > >I know why there's no tag on it even though the version number is high,
> > > > > >though it's not ideal either.
> > > > >
> > > > > Hi Mark and Lee,
> > > > >
> > > > > Is there anything that I need to do for this patch set. I have received reviewed
> > > > > by tag for all of them so far.
> > > >
> > > > Since we are so late in the day, I'm going to just apply this for v6.5.
> > > >
> > > > The remainder can then be applied, friction free, for v6.6.
> > >
> > > Now we have undocmented bindings in use by the driver (as pointed out by
> > > 'make dt_compatible_check').
> > >
> > > The whole series has all the acks/reviews needed for you to apply the
> > > whole thing, so why not take the whole thing? Plus this series has been
> > > sitting for 2 months. Not a great experience for submitters...
> >
> > Patches are missing Acked-by tags.
> >
> > Reviewed-by != Acked-by
>
> Reviewed-by > Acked-by
>
> >
> > I cannot merge other subsystem's patches without and Acked-by.
>
> I (and Krzysztof) give one or the other. If I'm taking a patch, then
> it's neither. I'm pretty sure Mark only gives Reviewed-by when he is
> not taking something.
>
> Rob

It does seem a bit ambiguous whether an "Acked-by" indicates a
"Reviewed-by + acceptance of the changes" or just a brief look-over with
acceptance of the changes. FWIW the documentation does use the word
"reviewed" when describing Acked-by. [^1]

However, I would argue that a Reviewed-by has a implicit acceptance of
the changes: why else provide a Reviewed-by line for the commit message
if you fundamentally disagree with the changes being merged? So a
Reviewed-by given by a maintainer should be seen as approval for those
changes to be merged.

William Breathitt Gray

[^1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by


Attachments:
(No filename) (4.97 kB)
signature.asc (235.00 B)
Download all attachments

2023-06-27 15:12:42

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Tue, Jun 27, 2023 at 08:10:59AM -0600, Rob Herring wrote:
> On Tue, Jun 27, 2023 at 7:56 AM Lee Jones <[email protected]> wrote:

> > I cannot merge other subsystem's patches without and Acked-by.

> I (and Krzysztof) give one or the other. If I'm taking a patch, then
> it's neither. I'm pretty sure Mark only gives Reviewed-by when he is
> not taking something.

Yes, I'm unlikely to give any kind of tag if I'm expecting to apply
something.


Attachments:
(No filename) (457.00 B)
signature.asc (499.00 B)
Download all attachments

2023-06-27 16:52:09

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Tue, 27 Jun 2023, William Breathitt Gray wrote:

> On Tue, Jun 27, 2023 at 08:10:59AM -0600, Rob Herring wrote:
> > On Tue, Jun 27, 2023 at 7:56 AM Lee Jones <[email protected]> wrote:
> > >
> > > On Mon, 26 Jun 2023, Rob Herring wrote:
> > >
> > > > On Wed, Jun 21, 2023 at 06:13:15PM +0100, Lee Jones wrote:
> > > > > On Tue, 13 Jun 2023, Sahin, Okan wrote:
> > > > >
> > > > > > >On Fri, Apr 21, 2023 at 08:39:38AM +0100, Lee Jones wrote:
> > > > > > >
> > > > > > >> I'll try anything once!
> > > > > > >
> > > > > > >> Fair warning, I think this is going to massively complicate things.
> > > > > > >
> > > > > > >> Either we're going to be left with a situation where child-driver
> > > > > > >> maintainers are scrabbling around looking for previous versions for the
> > > > > > >> MFD pull-request or contributors being forced to wait a full cycle for
> > > > > > >> their dependencies to arrive in the maintainer's base.
> > > > > > >
> > > > > > >If people are resending after the MFD has gone in they really ought to
> > > > > > >be including the pull request in the cover letter, with some combination
> > > > > > >of either referencing the mail or just saying "this depends on the
> > > > > > >signed tag at url+tag", the same way they would for any other dependency.
> > > > > > >
> > > > > > >I can't see how you applying stuff when you can slow things down TBH,
> > > > > > >the MFD bits will be applied faster and either people can pull in a
> > > > > > >shared tag or you can apply more commits on top of the existing core
> > > > > > >driver.
> > > > > > >
> > > > > > >> I'm not sure why simply providing your Ack when you're happy with the
> > > > > > >> driver and forgetting about the set until the pull-request arrives, like
> > > > > > >> we've been doing for nearly a decade now, isn't working for you anymore
> > > > > > >> but I'm mostly sure this method will be a regression.
> > > > > > >
> > > > > > >Like I said I've not been doing that, I've mostly been just applying the
> > > > > > >driver when it's ready. This might not have been so visible to you
> > > > > > >since it means that the regulator driver doesn't appear in the series by
> > > > > > >the time the MFD settles down. The whole "Acked-for-MFD" has always
> > > > > > >been a bit confusing TBH, it's not a normal ack ("go ahead and apply
> > > > > > >this, I'm fine with it") so it was never clear what the intention was.
> > > > > > >
> > > > > > >Before I started just applying the drivers there used to be constant
> > > > > > >problems with things like tags going missing (which some of the time is
> > > > > > >the submitter just not carrying them but can also be the result of some
> > > > > > >churn causing them to be deliberately dropped due to changes) or
> > > > > > >forgetting the series as you suggest and then not looking at some other
> > > > > > >very similarly named series that was also getting lots of versions after
> > > > > > >thinking it was one that had been reviewed already. It was all very
> > > > > > >frustrating. Not doing the tags until the dependencies have settled
> > > > > > >down means that if it's in my inbox it at least consistently needs some
> > > > > > >kind of attention and that the submitter didn't drop tags or anything so
> > > > > > >I know why there's no tag on it even though the version number is high,
> > > > > > >though it's not ideal either.
> > > > > >
> > > > > > Hi Mark and Lee,
> > > > > >
> > > > > > Is there anything that I need to do for this patch set. I have received reviewed
> > > > > > by tag for all of them so far.
> > > > >
> > > > > Since we are so late in the day, I'm going to just apply this for v6.5.
> > > > >
> > > > > The remainder can then be applied, friction free, for v6.6.
> > > >
> > > > Now we have undocmented bindings in use by the driver (as pointed out by
> > > > 'make dt_compatible_check').
> > > >
> > > > The whole series has all the acks/reviews needed for you to apply the
> > > > whole thing, so why not take the whole thing? Plus this series has been
> > > > sitting for 2 months. Not a great experience for submitters...
> > >
> > > Patches are missing Acked-by tags.
> > >
> > > Reviewed-by != Acked-by
> >
> > Reviewed-by > Acked-by
> >
> > >
> > > I cannot merge other subsystem's patches without and Acked-by.
> >
> > I (and Krzysztof) give one or the other. If I'm taking a patch, then
> > it's neither. I'm pretty sure Mark only gives Reviewed-by when he is
> > not taking something.
> >
> > Rob
>
> It does seem a bit ambiguous whether an "Acked-by" indicates a
> "Reviewed-by + acceptance of the changes" or just a brief look-over with
> acceptance of the changes. FWIW the documentation does use the word
> "reviewed" when describing Acked-by. [^1]
>
> However, I would argue that a Reviewed-by has a implicit acceptance of
> the changes: why else provide a Reviewed-by line for the commit message
> if you fundamentally disagree with the changes being merged? So a

Where MFD is concerned the complexities are seldom 'whether' a patch
should be merged, but rather 'how' it should be merged.

In order to solve some of these issues in the past, I created a bespoke
tag for scenarios where I'd like to indicate that a submission had been
reviewed, but I also intended to take the patch via the MFD tree once
all of the other pieces were ready. Despite using this tag for around a
decade, it did cause occasional confusion, even amongst maintainers I'd
been working with for the longest time, so I recently stopped using it
and replaced it with a standard Reviewed-by, to mean that it's reviewed
but permission was *not* given for someone else to merge it - since my
understanding, according to the documentation, is that an Acked-by is
required for that.

Recent discussions with other maintainers culminated in an agreement
that I would start only taking the MFD pieces and follow-up with a
pull-request for an immutable branch for them to pull from. Since there
is no more time to create, test and submit a maintainer-maintainer
pull-request, I decided to merge this patch anyway, so the leaf drivers
can be applied in a couple of weeks, after the merge-window is closed.

Which brings us to where we are now!

Without different tag which doesn't exist today, I'm not entirely sure
how to solve this issue. Ideas welcome.

> Reviewed-by given by a maintainer should be seen as approval for those
> changes to be merged.
>
> William Breathitt Gray
>
> [^1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

--
Lee Jones [李琼斯]

2023-06-27 18:53:33

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Tue, Jun 27, 2023 at 10:33 AM Lee Jones <[email protected]> wrote:
>
> On Tue, 27 Jun 2023, William Breathitt Gray wrote:
>
> > On Tue, Jun 27, 2023 at 08:10:59AM -0600, Rob Herring wrote:
> > > On Tue, Jun 27, 2023 at 7:56 AM Lee Jones <[email protected]> wrote:
> > > >
> > > > On Mon, 26 Jun 2023, Rob Herring wrote:
> > > >
> > > > > On Wed, Jun 21, 2023 at 06:13:15PM +0100, Lee Jones wrote:
> > > > > > On Tue, 13 Jun 2023, Sahin, Okan wrote:
> > > > > >
> > > > > > > >On Fri, Apr 21, 2023 at 08:39:38AM +0100, Lee Jones wrote:
> > > > > > > >
> > > > > > > >> I'll try anything once!
> > > > > > > >
> > > > > > > >> Fair warning, I think this is going to massively complicate things.
> > > > > > > >
> > > > > > > >> Either we're going to be left with a situation where child-driver
> > > > > > > >> maintainers are scrabbling around looking for previous versions for the
> > > > > > > >> MFD pull-request or contributors being forced to wait a full cycle for
> > > > > > > >> their dependencies to arrive in the maintainer's base.
> > > > > > > >
> > > > > > > >If people are resending after the MFD has gone in they really ought to
> > > > > > > >be including the pull request in the cover letter, with some combination
> > > > > > > >of either referencing the mail or just saying "this depends on the
> > > > > > > >signed tag at url+tag", the same way they would for any other dependency.
> > > > > > > >
> > > > > > > >I can't see how you applying stuff when you can slow things down TBH,
> > > > > > > >the MFD bits will be applied faster and either people can pull in a
> > > > > > > >shared tag or you can apply more commits on top of the existing core
> > > > > > > >driver.
> > > > > > > >
> > > > > > > >> I'm not sure why simply providing your Ack when you're happy with the
> > > > > > > >> driver and forgetting about the set until the pull-request arrives, like
> > > > > > > >> we've been doing for nearly a decade now, isn't working for you anymore
> > > > > > > >> but I'm mostly sure this method will be a regression.
> > > > > > > >
> > > > > > > >Like I said I've not been doing that, I've mostly been just applying the
> > > > > > > >driver when it's ready. This might not have been so visible to you
> > > > > > > >since it means that the regulator driver doesn't appear in the series by
> > > > > > > >the time the MFD settles down. The whole "Acked-for-MFD" has always
> > > > > > > >been a bit confusing TBH, it's not a normal ack ("go ahead and apply
> > > > > > > >this, I'm fine with it") so it was never clear what the intention was.
> > > > > > > >
> > > > > > > >Before I started just applying the drivers there used to be constant
> > > > > > > >problems with things like tags going missing (which some of the time is
> > > > > > > >the submitter just not carrying them but can also be the result of some
> > > > > > > >churn causing them to be deliberately dropped due to changes) or
> > > > > > > >forgetting the series as you suggest and then not looking at some other
> > > > > > > >very similarly named series that was also getting lots of versions after
> > > > > > > >thinking it was one that had been reviewed already. It was all very
> > > > > > > >frustrating. Not doing the tags until the dependencies have settled
> > > > > > > >down means that if it's in my inbox it at least consistently needs some
> > > > > > > >kind of attention and that the submitter didn't drop tags or anything so
> > > > > > > >I know why there's no tag on it even though the version number is high,
> > > > > > > >though it's not ideal either.
> > > > > > >
> > > > > > > Hi Mark and Lee,
> > > > > > >
> > > > > > > Is there anything that I need to do for this patch set. I have received reviewed
> > > > > > > by tag for all of them so far.
> > > > > >
> > > > > > Since we are so late in the day, I'm going to just apply this for v6.5.
> > > > > >
> > > > > > The remainder can then be applied, friction free, for v6.6.
> > > > >
> > > > > Now we have undocmented bindings in use by the driver (as pointed out by
> > > > > 'make dt_compatible_check').
> > > > >
> > > > > The whole series has all the acks/reviews needed for you to apply the
> > > > > whole thing, so why not take the whole thing? Plus this series has been
> > > > > sitting for 2 months. Not a great experience for submitters...
> > > >
> > > > Patches are missing Acked-by tags.
> > > >
> > > > Reviewed-by != Acked-by
> > >
> > > Reviewed-by > Acked-by
> > >
> > > >
> > > > I cannot merge other subsystem's patches without and Acked-by.
> > >
> > > I (and Krzysztof) give one or the other. If I'm taking a patch, then
> > > it's neither. I'm pretty sure Mark only gives Reviewed-by when he is
> > > not taking something.
> > >
> > > Rob
> >
> > It does seem a bit ambiguous whether an "Acked-by" indicates a
> > "Reviewed-by + acceptance of the changes" or just a brief look-over with
> > acceptance of the changes. FWIW the documentation does use the word
> > "reviewed" when describing Acked-by. [^1]
> >
> > However, I would argue that a Reviewed-by has a implicit acceptance of
> > the changes: why else provide a Reviewed-by line for the commit message
> > if you fundamentally disagree with the changes being merged? So a
>
> Where MFD is concerned the complexities are seldom 'whether' a patch
> should be merged, but rather 'how' it should be merged.
>
> In order to solve some of these issues in the past, I created a bespoke
> tag for scenarios where I'd like to indicate that a submission had been
> reviewed, but I also intended to take the patch via the MFD tree once
> all of the other pieces were ready. Despite using this tag for around a
> decade, it did cause occasional confusion, even amongst maintainers I'd
> been working with for the longest time, so I recently stopped using it
> and replaced it with a standard Reviewed-by, to mean that it's reviewed
> but permission was *not* given for someone else to merge it - since my
> understanding, according to the documentation, is that an Acked-by is
> required for that.
>
> Recent discussions with other maintainers culminated in an agreement
> that I would start only taking the MFD pieces and follow-up with a
> pull-request for an immutable branch for them to pull from. Since there
> is no more time to create, test and submit a maintainer-maintainer
> pull-request, I decided to merge this patch anyway, so the leaf drivers
> can be applied in a couple of weeks, after the merge-window is closed.
>
> Which brings us to where we are now!
>
> Without different tag which doesn't exist today, I'm not entirely sure
> how to solve this issue. Ideas welcome.

IMO, a series with interdependencies, which most cases of a new MFD
are, should be applied as a series. That's generally what happens
everywhere else. Creating a branch and PR seems like extra work for
everyone. The downside to that is any API changes outside of MFD would
need some coordination. That coordination would only be needed when a
subsystem has some API change and there's a new MFD using that
subsystem rather than by default for every new MFD.

Another option is just that you take all the binding patches since the
MFD binding depends on the others. The drivers can still go via the
subsystem. Not totally ideal to have branches of drivers missing
bindings, but better than mainline missing bindings.

Rob

2023-06-28 14:22:52

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Tue, 27 Jun 2023, Rob Herring wrote:

> On Tue, Jun 27, 2023 at 10:33 AM Lee Jones <[email protected]> wrote:
> >
> > On Tue, 27 Jun 2023, William Breathitt Gray wrote:
> >
> > > On Tue, Jun 27, 2023 at 08:10:59AM -0600, Rob Herring wrote:
> > > > On Tue, Jun 27, 2023 at 7:56 AM Lee Jones <[email protected]> wrote:
> > > > >
> > > > > On Mon, 26 Jun 2023, Rob Herring wrote:
> > > > >
> > > > > > On Wed, Jun 21, 2023 at 06:13:15PM +0100, Lee Jones wrote:
> > > > > > > On Tue, 13 Jun 2023, Sahin, Okan wrote:
> > > > > > >
> > > > > > > > >On Fri, Apr 21, 2023 at 08:39:38AM +0100, Lee Jones wrote:
> > > > > > > > >
> > > > > > > > >> I'll try anything once!
> > > > > > > > >
> > > > > > > > >> Fair warning, I think this is going to massively complicate things.
> > > > > > > > >
> > > > > > > > >> Either we're going to be left with a situation where child-driver
> > > > > > > > >> maintainers are scrabbling around looking for previous versions for the
> > > > > > > > >> MFD pull-request or contributors being forced to wait a full cycle for
> > > > > > > > >> their dependencies to arrive in the maintainer's base.
> > > > > > > > >
> > > > > > > > >If people are resending after the MFD has gone in they really ought to
> > > > > > > > >be including the pull request in the cover letter, with some combination
> > > > > > > > >of either referencing the mail or just saying "this depends on the
> > > > > > > > >signed tag at url+tag", the same way they would for any other dependency.
> > > > > > > > >
> > > > > > > > >I can't see how you applying stuff when you can slow things down TBH,
> > > > > > > > >the MFD bits will be applied faster and either people can pull in a
> > > > > > > > >shared tag or you can apply more commits on top of the existing core
> > > > > > > > >driver.
> > > > > > > > >
> > > > > > > > >> I'm not sure why simply providing your Ack when you're happy with the
> > > > > > > > >> driver and forgetting about the set until the pull-request arrives, like
> > > > > > > > >> we've been doing for nearly a decade now, isn't working for you anymore
> > > > > > > > >> but I'm mostly sure this method will be a regression.
> > > > > > > > >
> > > > > > > > >Like I said I've not been doing that, I've mostly been just applying the
> > > > > > > > >driver when it's ready. This might not have been so visible to you
> > > > > > > > >since it means that the regulator driver doesn't appear in the series by
> > > > > > > > >the time the MFD settles down. The whole "Acked-for-MFD" has always
> > > > > > > > >been a bit confusing TBH, it's not a normal ack ("go ahead and apply
> > > > > > > > >this, I'm fine with it") so it was never clear what the intention was.
> > > > > > > > >
> > > > > > > > >Before I started just applying the drivers there used to be constant
> > > > > > > > >problems with things like tags going missing (which some of the time is
> > > > > > > > >the submitter just not carrying them but can also be the result of some
> > > > > > > > >churn causing them to be deliberately dropped due to changes) or
> > > > > > > > >forgetting the series as you suggest and then not looking at some other
> > > > > > > > >very similarly named series that was also getting lots of versions after
> > > > > > > > >thinking it was one that had been reviewed already. It was all very
> > > > > > > > >frustrating. Not doing the tags until the dependencies have settled
> > > > > > > > >down means that if it's in my inbox it at least consistently needs some
> > > > > > > > >kind of attention and that the submitter didn't drop tags or anything so
> > > > > > > > >I know why there's no tag on it even though the version number is high,
> > > > > > > > >though it's not ideal either.
> > > > > > > >
> > > > > > > > Hi Mark and Lee,
> > > > > > > >
> > > > > > > > Is there anything that I need to do for this patch set. I have received reviewed
> > > > > > > > by tag for all of them so far.
> > > > > > >
> > > > > > > Since we are so late in the day, I'm going to just apply this for v6.5.
> > > > > > >
> > > > > > > The remainder can then be applied, friction free, for v6.6.
> > > > > >
> > > > > > Now we have undocmented bindings in use by the driver (as pointed out by
> > > > > > 'make dt_compatible_check').
> > > > > >
> > > > > > The whole series has all the acks/reviews needed for you to apply the
> > > > > > whole thing, so why not take the whole thing? Plus this series has been
> > > > > > sitting for 2 months. Not a great experience for submitters...
> > > > >
> > > > > Patches are missing Acked-by tags.
> > > > >
> > > > > Reviewed-by != Acked-by
> > > >
> > > > Reviewed-by > Acked-by
> > > >
> > > > >
> > > > > I cannot merge other subsystem's patches without and Acked-by.
> > > >
> > > > I (and Krzysztof) give one or the other. If I'm taking a patch, then
> > > > it's neither. I'm pretty sure Mark only gives Reviewed-by when he is
> > > > not taking something.
> > > >
> > > > Rob
> > >
> > > It does seem a bit ambiguous whether an "Acked-by" indicates a
> > > "Reviewed-by + acceptance of the changes" or just a brief look-over with
> > > acceptance of the changes. FWIW the documentation does use the word
> > > "reviewed" when describing Acked-by. [^1]
> > >
> > > However, I would argue that a Reviewed-by has a implicit acceptance of
> > > the changes: why else provide a Reviewed-by line for the commit message
> > > if you fundamentally disagree with the changes being merged? So a
> >
> > Where MFD is concerned the complexities are seldom 'whether' a patch
> > should be merged, but rather 'how' it should be merged.
> >
> > In order to solve some of these issues in the past, I created a bespoke
> > tag for scenarios where I'd like to indicate that a submission had been
> > reviewed, but I also intended to take the patch via the MFD tree once
> > all of the other pieces were ready. Despite using this tag for around a
> > decade, it did cause occasional confusion, even amongst maintainers I'd
> > been working with for the longest time, so I recently stopped using it
> > and replaced it with a standard Reviewed-by, to mean that it's reviewed
> > but permission was *not* given for someone else to merge it - since my
> > understanding, according to the documentation, is that an Acked-by is
> > required for that.
> >
> > Recent discussions with other maintainers culminated in an agreement
> > that I would start only taking the MFD pieces and follow-up with a
> > pull-request for an immutable branch for them to pull from. Since there
> > is no more time to create, test and submit a maintainer-maintainer
> > pull-request, I decided to merge this patch anyway, so the leaf drivers
> > can be applied in a couple of weeks, after the merge-window is closed.
> >
> > Which brings us to where we are now!
> >
> > Without different tag which doesn't exist today, I'm not entirely sure
> > how to solve this issue. Ideas welcome.
>
> IMO, a series with interdependencies, which most cases of a new MFD
> are, should be applied as a series. That's generally what happens
> everywhere else. Creating a branch and PR seems like extra work for
> everyone. The downside to that is any API changes outside of MFD would

This is what we've been doing for the last decade. However, I'm getting
mixed messages from folk. Mark recently asked for something completely
different (which I did say would be a bad idea at the time):

https://lore.kernel.org/all/[email protected]/

Could we please just pick a strategy and go with it?

> need some coordination. That coordination would only be needed when a
> subsystem has some API change and there's a new MFD using that
> subsystem rather than by default for every new MFD.
>
> Another option is just that you take all the binding patches since the
> MFD binding depends on the others. The drivers can still go via the
> subsystem. Not totally ideal to have branches of drivers missing
> bindings, but better than mainline missing bindings.

My original method of taking everything with Acks was fine IMHO.

--
Lee Jones [李琼斯]

2023-06-28 14:25:18

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 1/5] dt-bindings: regulator: max77541: Add ADI MAX77541/MAX77540 Regulator

On Wed, 12 Apr 2023, Okan Sahin wrote:

> Add ADI MAX77541/MAX77540 Regulator devicetree document.
>
> Signed-off-by: Okan Sahin <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> ---
> .../regulator/adi,max77541-regulator.yaml | 38 +++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77541-regulator.yaml

Applied, thanks

--
Lee Jones [李琼斯]

2023-06-28 14:26:08

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 3/5] iio: adc: max77541: Add ADI MAX77541 ADC Support

On Wed, 12 Apr 2023, Okan Sahin wrote:

> The MAX77541 has an 8-bit Successive Approximation Register (SAR) ADC
> with four multiplexers for supporting the telemetry feature.
>
> Signed-off-by: Okan Sahin <[email protected]>
> Reviewed-by: Jonathan Cameron <[email protected]>
> ---
> drivers/iio/adc/Kconfig | 11 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/max77541-adc.c | 194 +++++++++++++++++++++++++++++++++
> 3 files changed, 206 insertions(+)
> create mode 100644 drivers/iio/adc/max77541-adc.c

Applied, thanks

--
Lee Jones [李琼斯]

2023-06-28 14:49:11

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 4/5] dt-bindings: mfd: max77541: Add ADI MAX77541/MAX77540

On Wed, 12 Apr 2023, Okan Sahin wrote:

> Add ADI MAX77541/MAX77540 devicetree document.
>
> Signed-off-by: Okan Sahin <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> ---
> .../devicetree/bindings/mfd/adi,max77541.yaml | 68 +++++++++++++++++++
> 1 file changed, 68 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77541.yaml

Applied, thanks

--
Lee Jones [李琼斯]

2023-06-28 19:47:52

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Wed, Jun 28, 2023 at 02:40:13PM +0100, Lee Jones wrote:
> On Tue, 27 Jun 2023, Rob Herring wrote:
> > On Tue, Jun 27, 2023 at 10:33 AM Lee Jones <[email protected]> wrote:

> > IMO, a series with interdependencies, which most cases of a new MFD
> > are, should be applied as a series. That's generally what happens
> > everywhere else. Creating a branch and PR seems like extra work for
> > everyone. The downside to that is any API changes outside of MFD would

> This is what we've been doing for the last decade. However, I'm getting
> mixed messages from folk. Mark recently asked for something completely
> different (which I did say would be a bad idea at the time):

> https://lore.kernel.org/all/[email protected]/

> Could we please just pick a strategy and go with it?

The basic ask from me is for things that cause these serieses to make
progress, ideally in ways that minimise the amount of noise that they
generate (given that they're generally pretty routine). Applying
patches when they're ready at least mitigates the size of the series,
makes it easy to tell that they're OK and doesn't preclude applying more
patches on top of it if that's a thing that people want to do.

> > need some coordination. That coordination would only be needed when a
> > subsystem has some API change and there's a new MFD using that
> > subsystem rather than by default for every new MFD.

> > Another option is just that you take all the binding patches since the
> > MFD binding depends on the others. The drivers can still go via the
> > subsystem. Not totally ideal to have branches of drivers missing
> > bindings, but better than mainline missing bindings.

> My original method of taking everything with Acks was fine IMHO.

As I mentioned before the number of resends of what are frequently very
similar serieses (eg, two PMICs from the same vendor in flight at the
same time) was causing me real issues with tags going AWOL and things
getting lost in the noise.


Attachments:
(No filename) (1.98 kB)
signature.asc (499.00 B)
Download all attachments

2023-06-29 07:59:27

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

TL;DR: I'm reverting back to the old style of cross-subsystem patch
management where sets get merged as sets with maintainer Acks (or
Reviews!).

On Wed, 28 Jun 2023, Mark Brown wrote:
> On Wed, Jun 28, 2023 at 02:40:13PM +0100, Lee Jones wrote:
> > On Tue, 27 Jun 2023, Rob Herring wrote:
> > > On Tue, Jun 27, 2023 at 10:33 AM Lee Jones <[email protected]> wrote:
>
> > > IMO, a series with interdependencies, which most cases of a new MFD
> > > are, should be applied as a series. That's generally what happens
> > > everywhere else. Creating a branch and PR seems like extra work for
> > > everyone. The downside to that is any API changes outside of MFD would
>
> > This is what we've been doing for the last decade. However, I'm getting
> > mixed messages from folk. Mark recently asked for something completely
> > different (which I did say would be a bad idea at the time):
>
> > https://lore.kernel.org/all/[email protected]/
>
> > Could we please just pick a strategy and go with it?
>
> The basic ask from me is for things that cause these serieses to make
> progress, ideally in ways that minimise the amount of noise that they
> generate (given that they're generally pretty routine). Applying
> patches when they're ready at least mitigates the size of the series,
> makes it easy to tell that they're OK and doesn't preclude applying more
> patches on top of it if that's a thing that people want to do.
>
> > > need some coordination. That coordination would only be needed when a
> > > subsystem has some API change and there's a new MFD using that
> > > subsystem rather than by default for every new MFD.
>
> > > Another option is just that you take all the binding patches since the
> > > MFD binding depends on the others. The drivers can still go via the
> > > subsystem. Not totally ideal to have branches of drivers missing
> > > bindings, but better than mainline missing bindings.
>
> > My original method of taking everything with Acks was fine IMHO.
>
> As I mentioned before the number of resends of what are frequently very
> similar serieses (eg, two PMICs from the same vendor in flight at the
> same time) was causing me real issues with tags going AWOL and things
> getting lost in the noise.

As much as I empathise with each of these points (I feel it too), the
alternative seems to be causing more issues for more people. With that
in mind, I'm going to revert back to how we've been doing things for a
long time now. Please try to Ack and forget. If a contributor fails to
apply a previously issued tag, we'll have to bring that up at the time.

--
Lee Jones [李琼斯]

2023-06-29 11:07:18

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Thu, Jun 29, 2023 at 08:25:00AM +0100, Lee Jones wrote:
> On Wed, 28 Jun 2023, Mark Brown wrote:

> > As I mentioned before the number of resends of what are frequently very
> > similar serieses (eg, two PMICs from the same vendor in flight at the
> > same time) was causing me real issues with tags going AWOL and things
> > getting lost in the noise.

> As much as I empathise with each of these points (I feel it too), the
> alternative seems to be causing more issues for more people. With that
> in mind, I'm going to revert back to how we've been doing things for a
> long time now. Please try to Ack and forget. If a contributor fails to
> apply a previously issued tag, we'll have to bring that up at the time.

The thing that's causing a lot of the issues here is that you're only
applying the serieses en masse, blocking things on getting absolutely
everything lined up (including this time over a merge window). I really
don't understand why you feel you're forced to batch everything together
like this.


Attachments:
(No filename) (1.02 kB)
signature.asc (499.00 B)
Download all attachments

2023-06-29 16:07:16

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Thu, Jun 29, 2023 at 04:51:34PM +0100, Lee Jones wrote:
> On Thu, 29 Jun 2023, Mark Brown wrote:

> > The thing that's causing a lot of the issues here is that you're only
> > applying the serieses en masse, blocking things on getting absolutely
> > everything lined up (including this time over a merge window). I really
> > don't understand why you feel you're forced to batch everything together
> > like this.

> https://lore.kernel.org/all/CAL_Jsq+Z64tuMO8a2Y=2GrXZ8q0L4Z2avCiphsn0HOOC71Dzjg@mail.gmail.com/

That says it's a bit unusual to use a separate branch with a PR, it
doesn't say anything about it being the end of the world to pick up
parts of a series that are ready without picking up the whole lot,
especially when we're getting to the merge window.


Attachments:
(No filename) (787.00 B)
signature.asc (499.00 B)
Download all attachments

2023-06-29 16:26:25

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Thu, 29 Jun 2023, Mark Brown wrote:

> On Thu, Jun 29, 2023 at 08:25:00AM +0100, Lee Jones wrote:
> > On Wed, 28 Jun 2023, Mark Brown wrote:
>
> > > As I mentioned before the number of resends of what are frequently very
> > > similar serieses (eg, two PMICs from the same vendor in flight at the
> > > same time) was causing me real issues with tags going AWOL and things
> > > getting lost in the noise.
>
> > As much as I empathise with each of these points (I feel it too), the
> > alternative seems to be causing more issues for more people. With that
> > in mind, I'm going to revert back to how we've been doing things for a
> > long time now. Please try to Ack and forget. If a contributor fails to
> > apply a previously issued tag, we'll have to bring that up at the time.
>
> The thing that's causing a lot of the issues here is that you're only
> applying the serieses en masse, blocking things on getting absolutely
> everything lined up (including this time over a merge window). I really
> don't understand why you feel you're forced to batch everything together
> like this.

https://lore.kernel.org/all/CAL_Jsq+Z64tuMO8a2Y=2GrXZ8q0L4Z2avCiphsn0HOOC71Dzjg@mail.gmail.com/

--
Lee Jones [李琼斯]

2023-06-29 18:00:34

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Thu, Jun 29, 2023 at 10:00 AM Mark Brown <[email protected]> wrote:
>
> On Thu, Jun 29, 2023 at 04:51:34PM +0100, Lee Jones wrote:
> > On Thu, 29 Jun 2023, Mark Brown wrote:
>
> > > The thing that's causing a lot of the issues here is that you're only
> > > applying the serieses en masse, blocking things on getting absolutely
> > > everything lined up (including this time over a merge window). I really
> > > don't understand why you feel you're forced to batch everything together
> > > like this.
>
> > https://lore.kernel.org/all/CAL_Jsq+Z64tuMO8a2Y=2GrXZ8q0L4Z2avCiphsn0HOOC71Dzjg@mail.gmail.com/
>
> That says it's a bit unusual to use a separate branch with a PR, it
> doesn't say anything about it being the end of the world to pick up
> parts of a series that are ready without picking up the whole lot,
> especially when we're getting to the merge window.

There's some risk the core part could change affecting the sub
components after you pick it up, or that the author just abandons
getting the core part upstream and we're left with a dead driver.

I'm fine if the sub-components are picked up first (and land in next
first), but the MFD binding picked up first is a problem usually. And
that has happened multiple times. Also, it does mean the MFD branch
will have binding warnings, but that's Lee's problem if he cares
(probably not).

Rob

2023-06-29 18:15:43

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Thu, Jun 29, 2023 at 11:48:34AM -0600, Rob Herring wrote:
> On Thu, Jun 29, 2023 at 10:00 AM Mark Brown <[email protected]> wrote:

> > That says it's a bit unusual to use a separate branch with a PR, it
> > doesn't say anything about it being the end of the world to pick up
> > parts of a series that are ready without picking up the whole lot,
> > especially when we're getting to the merge window.

> There's some risk the core part could change affecting the sub
> components after you pick it up, or that the author just abandons
> getting the core part upstream and we're left with a dead driver.

Right, I'm suggesting applying the core part without waiting for every
single leaf driver to be lined up rather than the other way around -
that way the core part is stable and the leaf drivers only have issues
with changes in their subsystems that they'll have anyway even with
waiting. Leaf drivers can be added on top as they're ready and if
something misses a release then it can go through the subsystem, and if
people do end up wandering off then you've still got whatever did get
merged in case someone else wants to pick things up.

> I'm fine if the sub-components are picked up first (and land in next
> first), but the MFD binding picked up first is a problem usually. And
> that has happened multiple times. Also, it does mean the MFD branch
> will have binding warnings, but that's Lee's problem if he cares
> (probably not).

Linus complained about picking up the subcomponents without the core,
especially if the MFD doesn't end up landing in the same release for
whatever reason (which is fair enough, especially when the MFD does
miss).


Attachments:
(No filename) (1.66 kB)
signature.asc (499.00 B)
Download all attachments

2023-06-29 18:25:19

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Thu, Jun 29, 2023 at 11:59 AM Mark Brown <[email protected]> wrote:
>
> On Thu, Jun 29, 2023 at 11:48:34AM -0600, Rob Herring wrote:
> > On Thu, Jun 29, 2023 at 10:00 AM Mark Brown <[email protected]> wrote:
>
> > > That says it's a bit unusual to use a separate branch with a PR, it
> > > doesn't say anything about it being the end of the world to pick up
> > > parts of a series that are ready without picking up the whole lot,
> > > especially when we're getting to the merge window.
>
> > There's some risk the core part could change affecting the sub
> > components after you pick it up, or that the author just abandons
> > getting the core part upstream and we're left with a dead driver.
>
> Right, I'm suggesting applying the core part without waiting for every
> single leaf driver to be lined up rather than the other way around -
> that way the core part is stable and the leaf drivers only have issues
> with changes in their subsystems that they'll have anyway even with
> waiting. Leaf drivers can be added on top as they're ready and if
> something misses a release then it can go through the subsystem, and if
> people do end up wandering off then you've still got whatever did get
> merged in case someone else wants to pick things up.

I misunderstood. I thought you wanted to apply things to get them out
of your queue. That doesn't work when the leaf drivers depend on the
core, so what do we do there? A branch or Lee takes everything? That's
almost always the case with the bindings as the core binding
references the child node bindings. My preference there would be that
Lee picks up all the bindings with the core driver.

Rob

2023-06-29 18:25:22

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Thu, Jun 29, 2023 at 12:14:00PM -0600, Rob Herring wrote:
> On Thu, Jun 29, 2023 at 11:59 AM Mark Brown <[email protected]> wrote:

> > Right, I'm suggesting applying the core part without waiting for every
> > single leaf driver to be lined up rather than the other way around -
> > that way the core part is stable and the leaf drivers only have issues
> > with changes in their subsystems that they'll have anyway even with
> > waiting. Leaf drivers can be added on top as they're ready and if
> > something misses a release then it can go through the subsystem, and if
> > people do end up wandering off then you've still got whatever did get
> > merged in case someone else wants to pick things up.

> I misunderstood. I thought you wanted to apply things to get them out
> of your queue.

Well, I *do* but that's got issues especially when things get stuck so
I'm not going to.

> That doesn't work when the leaf drivers depend on the
> core, so what do we do there? A branch or Lee takes everything? That's
> almost always the case with the bindings as the core binding
> references the child node bindings. My preference there would be that
> Lee picks up all the bindings with the core driver.

My suggestion is that once the core is ready to apply that and also
start applying everything else to Lee's tree as it's ready. A branch
also works and might come in handy anyway in the case where there's some
subsystem wide updates in some other subsystem (since it avoids having
to pull the whole MFD tree in or anything like that) but it's not
essential to the idea.


Attachments:
(No filename) (1.59 kB)
signature.asc (499.00 B)
Download all attachments

2023-06-30 07:33:29

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Thu, 29 Jun 2023, Mark Brown wrote:

> On Thu, Jun 29, 2023 at 12:14:00PM -0600, Rob Herring wrote:
> > On Thu, Jun 29, 2023 at 11:59 AM Mark Brown <[email protected]> wrote:
>
> > > Right, I'm suggesting applying the core part without waiting for every
> > > single leaf driver to be lined up rather than the other way around -
> > > that way the core part is stable and the leaf drivers only have issues
> > > with changes in their subsystems that they'll have anyway even with
> > > waiting. Leaf drivers can be added on top as they're ready and if
> > > something misses a release then it can go through the subsystem, and if
> > > people do end up wandering off then you've still got whatever did get
> > > merged in case someone else wants to pick things up.
>
> > I misunderstood. I thought you wanted to apply things to get them out
> > of your queue.
>
> Well, I *do* but that's got issues especially when things get stuck so
> I'm not going to.
>
> > That doesn't work when the leaf drivers depend on the
> > core, so what do we do there? A branch or Lee takes everything? That's
> > almost always the case with the bindings as the core binding
> > references the child node bindings. My preference there would be that
> > Lee picks up all the bindings with the core driver.
>
> My suggestion is that once the core is ready to apply that and also
> start applying everything else to Lee's tree as it's ready. A branch
> also works and might come in handy anyway in the case where there's some
> subsystem wide updates in some other subsystem (since it avoids having
> to pull the whole MFD tree in or anything like that) but it's not
> essential to the idea.

The issue we currently have is that the core usually comes with a header
file which is included by some or all of the leaf drivers. If leaf
drivers are pulled in without that header, the drivers will fail to
build which will make people grumpy.

The suggestion of a separate branch that's added to over time as leaf
drivers become ready is even more work that a one-hit strategy. It will
also mean littering the working branch which a bunch more merges and/or
more frequent rebases than I'm happy with.

--
Lee Jones [李琼斯]

2023-06-30 12:41:27

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

On Fri, Jun 30, 2023 at 08:17:51AM +0100, Lee Jones wrote:
> On Thu, 29 Jun 2023, Mark Brown wrote:

> > My suggestion is that once the core is ready to apply that and also
> > start applying everything else to Lee's tree as it's ready. A branch
> > also works and might come in handy anyway in the case where there's some
> > subsystem wide updates in some other subsystem (since it avoids having
> > to pull the whole MFD tree in or anything like that) but it's not
> > essential to the idea.

> The issue we currently have is that the core usually comes with a header
> file which is included by some or all of the leaf drivers. If leaf
> drivers are pulled in without that header, the drivers will fail to
> build which will make people grumpy.

Which is why I'm not suggesting doing that.

> The suggestion of a separate branch that's added to over time as leaf
> drivers become ready is even more work that a one-hit strategy. It will
> also mean littering the working branch which a bunch more merges and/or
> more frequent rebases than I'm happy with.

As I said you don't *need* to do the separate branch, it's more of a
nice to have in case cross tree issues come up, and it'll probably work
fine most of the time to just put the incremental commits directly on
your main branch even if there was a separate branch for the initial
batch. This all becomes especially true as we get close to the merge
window and the likelyhood of new cross tree issues decreases.


Attachments:
(No filename) (1.47 kB)
signature.asc (499.00 B)
Download all attachments