[v7,5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC Support

Message ID 20230412111256.40013-6-okan.sahin@analog.com
State New
Headers
Series Add MAX77541/MAX77540 PMIC Support |

Commit Message

Sahin, Okan April 12, 2023, 11:12 a.m. UTC
  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 <okan.sahin@analog.com>
---
 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
  

Comments

Andy Shevchenko April 12, 2023, 1:41 p.m. UTC | #1
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 <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
> ---
>  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 <okan.sahin@analog.com>");
> +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
>
  
Lee Jones April 20, 2023, 10:34 a.m. UTC | #2
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 <okan.sahin@analog.com>
> ---
>  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 <lee@kernel.org>
  
Mark Brown April 20, 2023, 4:52 p.m. UTC | #3
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 <lee@kernel.org>

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.
  
Lee Jones April 21, 2023, 7:39 a.m. UTC | #4
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 <lee@kernel.org>
> 
> 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.
  
Mark Brown April 21, 2023, 1:33 p.m. UTC | #5
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.
  
Sahin, Okan June 13, 2023, 7:34 a.m. UTC | #6
>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
  
Lee Jones June 21, 2023, 5:13 p.m. UTC | #7
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 June 21, 2023, 5:14 p.m. UTC | #8
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 <okan.sahin@analog.com>
> ---
>  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
  
Rob Herring June 26, 2023, 5:54 p.m. UTC | #9
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
  
Lee Jones June 27, 2023, 1:56 p.m. UTC | #10
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.
  
Rob Herring June 27, 2023, 2:10 p.m. UTC | #11
On Tue, Jun 27, 2023 at 7:56 AM Lee Jones <lee@kernel.org> 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
  
Mark Brown June 27, 2023, 2:25 p.m. UTC | #12
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.
  
William Breathitt Gray June 27, 2023, 2:32 p.m. UTC | #13
On Tue, Jun 27, 2023 at 08:10:59AM -0600, Rob Herring wrote:
> On Tue, Jun 27, 2023 at 7:56 AM Lee Jones <lee@kernel.org> 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
  
Mark Brown June 27, 2023, 2:39 p.m. UTC | #14
On Tue, Jun 27, 2023 at 08:10:59AM -0600, Rob Herring wrote:
> On Tue, Jun 27, 2023 at 7:56 AM Lee Jones <lee@kernel.org> 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.
  
Lee Jones June 27, 2023, 4:33 p.m. UTC | #15
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 <lee@kernel.org> 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
  
Rob Herring June 27, 2023, 6:21 p.m. UTC | #16
On Tue, Jun 27, 2023 at 10:33 AM Lee Jones <lee@kernel.org> 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 <lee@kernel.org> 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
  
Lee Jones June 28, 2023, 1:40 p.m. UTC | #17
On Tue, 27 Jun 2023, Rob Herring wrote:

> On Tue, Jun 27, 2023 at 10:33 AM Lee Jones <lee@kernel.org> 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 <lee@kernel.org> 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/20230421073938.GO996918@google.com/

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.
  
Mark Brown June 28, 2023, 7:20 p.m. UTC | #18
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 <lee@kernel.org> 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/20230421073938.GO996918@google.com/

> 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.
  
Lee Jones June 29, 2023, 7:25 a.m. UTC | #19
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 <lee@kernel.org> 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/20230421073938.GO996918@google.com/
> 
> > 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.
  
Mark Brown June 29, 2023, 10:38 a.m. UTC | #20
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.
  
Lee Jones June 29, 2023, 3:51 p.m. UTC | #21
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/
  
Mark Brown June 29, 2023, 4 p.m. UTC | #22
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.
  
Rob Herring June 29, 2023, 5:48 p.m. UTC | #23
On Thu, Jun 29, 2023 at 10:00 AM Mark Brown <broonie@kernel.org> 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
  
Mark Brown June 29, 2023, 5:58 p.m. UTC | #24
On Thu, Jun 29, 2023 at 11:48:34AM -0600, Rob Herring wrote:
> On Thu, Jun 29, 2023 at 10:00 AM Mark Brown <broonie@kernel.org> 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).
  
Rob Herring June 29, 2023, 6:14 p.m. UTC | #25
On Thu, Jun 29, 2023 at 11:59 AM Mark Brown <broonie@kernel.org> 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 <broonie@kernel.org> 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
  
Mark Brown June 29, 2023, 6:22 p.m. UTC | #26
On Thu, Jun 29, 2023 at 12:14:00PM -0600, Rob Herring wrote:
> On Thu, Jun 29, 2023 at 11:59 AM Mark Brown <broonie@kernel.org> 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.
  
Lee Jones June 30, 2023, 7:17 a.m. UTC | #27
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 <broonie@kernel.org> 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.
  
Mark Brown June 30, 2023, 11:58 a.m. UTC | #28
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.
  

Patch

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 <okan.sahin@analog.com>");
+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 */