[PATCH v5 0/2] mfd: rn5t618: add ADC support

[PATCH v5 0/2] mfd: rn5t618: add ADC support

[Andreas Kemnade]

This series adds support for the ADC in the RN5T618/RC5T619.
It depends on the IRQ support added in the RTC support series here:
https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/

I tested the driver only with the RC5T619 but it should work with the with 
the RN5T618 as well based on these facts:
- The corresponding register definitions originally went into the kernel
  for the RN5T618
- Public datasheet sections about the ADC look same.
- Out-of-tree code for these chips look same regarding to ADC

But due to missing hardware I cannot test the patches 2/3 and 3/3 which
add support for the RN5T618 ADC.
I marked these untested patches as RFC, and IMHO they require a Tested-By.
Feel free to ignore them if the whole series would be delayed just because
of missing Tested-By for those.

Changes in v5:
- fixed typo

Changes in v4:
- drop RFC patches, seems nobody finds time to test now
- added trivial change and Reviewed-By

Changes in v3:
- re-included former 2/5 of these patches, since it was not applied

Changes in v2:
- got an "Applied, thanks" message for the first two, so I do not include
  them anymore
- some cleanups for the ADC driver itself

Andreas Kemnade (2):
  mfd: rn5t618: add ADC subdevice for RC5T619
  iio: adc: rn5t618: Add ADC driver for RN5T618/RC5T619

 drivers/iio/adc/Kconfig       |  10 ++
 drivers/iio/adc/Makefile      |   1 +
 drivers/iio/adc/rn5t618-adc.c | 256 ++++++++++++++++++++++++++++++++++
 drivers/mfd/rn5t618.c         |   1 +
 4 files changed, 268 insertions(+)
 create mode 100644 drivers/iio/adc/rn5t618-adc.c

-- 
2.20.1

[PATCH v5 1/2] mfd: rn5t618: add ADC subdevice for RC5T619

[Andreas Kemnade]

This adds a subdevice for the ADC in the RC5T619

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
depends on:
https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/

Changes in v3:
re-added it to the series because of
"Oh, it looks like there was a conflict.  Could you collect any Acks
(including mine) rebase and resend please?"

 drivers/mfd/rn5t618.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
index 073de8e0e78b..321836f78120 100644
--- a/drivers/mfd/rn5t618.c
+++ b/drivers/mfd/rn5t618.c
@@ -24,6 +24,7 @@ static const struct mfd_cell rn5t618_cells[] = {
 };
 
 static const struct mfd_cell rc5t619_cells[] = {
+	{ .name = "rn5t618-adc" },
 	{ .name = "rn5t618-regulator" },
 	{ .name = "rc5t619-rtc" },
 	{ .name = "rn5t618-wdt" },
-- 
2.20.1

[PATCH v5 2/2] iio: adc: rn5t618: Add ADC driver for RN5T618/RC5T619

[Andreas Kemnade]

Both chips have an A/D converter capable of measuring
things like VBAT, VUSB and analog inputs.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
depends on (build and runtime):
https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/

Changes in v4:
- merged ret = and return into one line
- added Reviewed-by:

Changes in v3:
- output scale instead of processed
- prefixed symbols with RN5T618

Changes in v2:
- enum for channels
- bulk read instead of single byte read for conversion
  result
- fix get_virq error handling
- use devm for registering device and requesting IRQ

 drivers/iio/adc/Kconfig       |  10 ++
 drivers/iio/adc/Makefile      |   1 +
 drivers/iio/adc/rn5t618-adc.c | 256 ++++++++++++++++++++++++++++++++++
 3 files changed, 267 insertions(+)
 create mode 100644 drivers/iio/adc/rn5t618-adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 5d8540b7b427..e7e74e775336 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -766,6 +766,16 @@ config RCAR_GYRO_ADC
 	  To compile this driver as a module, choose M here: the
 	  module will be called rcar-gyroadc.
 
+config RN5T618_ADC
+	tristate "ADC for the RN5T618/RC5T619 family of chips"
+	depends on MFD_RN5T618
+	help
+	  Say yes here to build support for the integrated ADC inside the
+	  RN5T618/619 series PMICs:
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rn5t618-adc.
+
 config ROCKCHIP_SARADC
 	tristate "Rockchip SARADC driver"
 	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index a1f1fbec0f87..ea95f89c91cd 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_QCOM_VADC_COMMON) += qcom-vadc-common.o
 obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
 obj-$(CONFIG_QCOM_PM8XXX_XOADC) += qcom-pm8xxx-xoadc.o
 obj-$(CONFIG_RCAR_GYRO_ADC) += rcar-gyroadc.o
+obj-$(CONFIG_RN5T618_ADC) += rn5t618-adc.o
 obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
 obj-$(CONFIG_SC27XX_ADC) += sc27xx_adc.o
 obj-$(CONFIG_SPEAR_ADC) += spear_adc.o
diff --git a/drivers/iio/adc/rn5t618-adc.c b/drivers/iio/adc/rn5t618-adc.c
new file mode 100644
index 000000000000..f21027e4e26a
--- /dev/null
+++ b/drivers/iio/adc/rn5t618-adc.c
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * ADC driver for the RICOH RN5T618 power management chip family
+ *
+ * Copyright (C) 2019 Andreas Kemnade
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mfd/rn5t618.h>
+#include <linux/platform_device.h>
+#include <linux/completion.h>
+#include <linux/regmap.h>
+#include <linux/iio/iio.h>
+#include <linux/slab.h>
+
+#define RN5T618_ADC_CONVERSION_TIMEOUT   (msecs_to_jiffies(500))
+#define RN5T618_REFERENCE_VOLT 2500
+
+/* mask for selecting channels for single conversion */
+#define RN5T618_ADCCNT3_CHANNEL_MASK 0x7
+/* average 4-time conversion mode */
+#define RN5T618_ADCCNT3_AVG BIT(3)
+/* set for starting a single conversion, gets cleared by hw when done */
+#define RN5T618_ADCCNT3_GODONE BIT(4)
+/* automatic conversion, period is in ADCCNT2, selected channels are
+ * in ADCCNT1
+ */
+#define RN5T618_ADCCNT3_AUTO BIT(5)
+#define RN5T618_ADCEND_IRQ BIT(0)
+
+struct rn5t618_adc_data {
+	struct device *dev;
+	struct rn5t618 *rn5t618;
+	struct completion conv_completion;
+	int irq;
+};
+
+struct rn5t618_channel_ratios {
+	u16 numerator;
+	u16 denominator;
+};
+
+enum rn5t618_channels {
+	LIMMON = 0,
+	VBAT,
+	VADP,
+	VUSB,
+	VSYS,
+	VTHM,
+	AIN1,
+	AIN0
+};
+
+static const struct rn5t618_channel_ratios rn5t618_ratios[8] = {
+	[LIMMON] = {50, 32}, /* measured across 20mOhm, amplified by 32 */
+	[VBAT] = {2, 1},
+	[VADP] = {3, 1},
+	[VUSB] = {3, 1},
+	[VSYS] = {3, 1},
+	[VTHM] = {1, 1},
+	[AIN1] = {1, 1},
+	[AIN0] = {1, 1},
+};
+
+static int rn5t618_read_adc_reg(struct rn5t618 *rn5t618, int reg, u16 *val)
+{
+	u8 data[2];
+	int ret;
+
+	ret = regmap_bulk_read(rn5t618->regmap, reg, data, sizeof(data));
+	if (ret < 0)
+		return ret;
+
+	*val = (data[0] << 4) | (data[1] & 0xF);
+
+	return 0;
+}
+
+static irqreturn_t rn5t618_adc_irq(int irq, void *data)
+{
+	struct rn5t618_adc_data *adc = data;
+	unsigned int r = 0;
+	int ret;
+
+	/* clear low & high threshold irqs */
+	regmap_write(adc->rn5t618->regmap, RN5T618_IR_ADC1, 0);
+	regmap_write(adc->rn5t618->regmap, RN5T618_IR_ADC2, 0);
+
+	ret = regmap_read(adc->rn5t618->regmap, RN5T618_IR_ADC3, &r);
+	if (ret < 0)
+		dev_err(adc->dev, "failed to read IRQ status: %d\n", ret);
+
+	regmap_write(adc->rn5t618->regmap, RN5T618_IR_ADC3, 0);
+
+	if (r & RN5T618_ADCEND_IRQ)
+		complete(&adc->conv_completion);
+
+	return IRQ_HANDLED;
+}
+
+static int rn5t618_adc_read(struct iio_dev *iio_dev,
+			    const struct iio_chan_spec *chan,
+			    int *val, int *val2, long mask)
+{
+	struct rn5t618_adc_data *adc = iio_priv(iio_dev);
+	u16 raw;
+	int ret;
+
+	if (mask == IIO_CHAN_INFO_SCALE) {
+		*val = RN5T618_REFERENCE_VOLT *
+		       rn5t618_ratios[chan->channel].numerator;
+		*val2 = rn5t618_ratios[chan->channel].denominator * 4095;
+
+		return IIO_VAL_FRACTIONAL;
+	}
+
+	/* select channel */
+	ret = regmap_update_bits(adc->rn5t618->regmap, RN5T618_ADCCNT3,
+				 RN5T618_ADCCNT3_CHANNEL_MASK,
+				 chan->channel);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_write(adc->rn5t618->regmap, RN5T618_EN_ADCIR3,
+			   RN5T618_ADCEND_IRQ);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_update_bits(adc->rn5t618->regmap, RN5T618_ADCCNT3,
+				 RN5T618_ADCCNT3_AVG,
+				 mask == IIO_CHAN_INFO_AVERAGE_RAW ?
+				 RN5T618_ADCCNT3_AVG : 0);
+	if (ret < 0)
+		return ret;
+
+	init_completion(&adc->conv_completion);
+	/* single conversion */
+	ret = regmap_update_bits(adc->rn5t618->regmap, RN5T618_ADCCNT3,
+				 RN5T618_ADCCNT3_GODONE,
+				 RN5T618_ADCCNT3_GODONE);
+	if (ret < 0)
+		return ret;
+
+	ret = wait_for_completion_timeout(&adc->conv_completion,
+					  RN5T618_ADC_CONVERSION_TIMEOUT);
+	if (ret == 0) {
+		dev_warn(adc->dev, "timeout waiting for adc result\n");
+		return -ETIMEDOUT;
+	}
+
+	ret = rn5t618_read_adc_reg(adc->rn5t618,
+				   RN5T618_ILIMDATAH + 2 * chan->channel,
+				   &raw);
+	if (ret < 0)
+		return ret;
+
+	*val = raw;
+
+	return IIO_VAL_INT;
+}
+
+static const struct iio_info rn5t618_adc_iio_info = {
+	.read_raw = &rn5t618_adc_read,
+};
+
+#define RN5T618_ADC_CHANNEL(_channel, _type, _name) { \
+	.type = _type, \
+	.channel = _channel, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+			      BIT(IIO_CHAN_INFO_AVERAGE_RAW) | \
+			      BIT(IIO_CHAN_INFO_SCALE), \
+	.datasheet_name = _name, \
+	.indexed = 1. \
+}
+
+static const struct iio_chan_spec rn5t618_adc_iio_channels[] = {
+	RN5T618_ADC_CHANNEL(LIMMON, IIO_CURRENT, "LIMMON"),
+	RN5T618_ADC_CHANNEL(VBAT, IIO_VOLTAGE, "VBAT"),
+	RN5T618_ADC_CHANNEL(VADP, IIO_VOLTAGE, "VADP"),
+	RN5T618_ADC_CHANNEL(VUSB, IIO_VOLTAGE, "VUSB"),
+	RN5T618_ADC_CHANNEL(VSYS, IIO_VOLTAGE, "VSYS"),
+	RN5T618_ADC_CHANNEL(VTHM, IIO_VOLTAGE, "VTHM"),
+	RN5T618_ADC_CHANNEL(AIN1, IIO_VOLTAGE, "AIN1"),
+	RN5T618_ADC_CHANNEL(AIN0, IIO_VOLTAGE, "AIN0")
+};
+
+static int rn5t618_adc_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct iio_dev *iio_dev;
+	struct rn5t618_adc_data *adc;
+	struct rn5t618 *rn5t618 = dev_get_drvdata(pdev->dev.parent);
+
+	iio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*adc));
+	if (!iio_dev) {
+		dev_err(&pdev->dev, "failed allocating iio device\n");
+		return -ENOMEM;
+	}
+
+	adc = iio_priv(iio_dev);
+	adc->dev = &pdev->dev;
+	adc->rn5t618 = rn5t618;
+
+	if (rn5t618->irq_data)
+		adc->irq = regmap_irq_get_virq(rn5t618->irq_data,
+					       RN5T618_IRQ_ADC);
+
+	if (adc->irq <= 0) {
+		dev_err(&pdev->dev, "get virq failed\n");
+		return -EINVAL;
+	}
+
+	init_completion(&adc->conv_completion);
+
+	iio_dev->name = dev_name(&pdev->dev);
+	iio_dev->dev.parent = &pdev->dev;
+	iio_dev->info = &rn5t618_adc_iio_info;
+	iio_dev->modes = INDIO_DIRECT_MODE;
+	iio_dev->channels = rn5t618_adc_iio_channels;
+	iio_dev->num_channels = ARRAY_SIZE(rn5t618_adc_iio_channels);
+
+	/* stop any auto-conversion */
+	ret = regmap_write(rn5t618->regmap, RN5T618_ADCCNT3, 0);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, iio_dev);
+
+	ret = devm_request_threaded_irq(adc->dev, adc->irq, NULL,
+					rn5t618_adc_irq,
+					IRQF_ONESHOT, dev_name(adc->dev),
+					adc);
+	if (ret < 0) {
+		dev_err(adc->dev, "request irq %d failed: %d\n", adc->irq, ret);
+		return ret;
+	}
+
+	return devm_iio_device_register(adc->dev, iio_dev);
+}
+
+static struct platform_driver rn5t618_adc_driver = {
+	.driver = {
+		.name   = "rn5t618-adc",
+	},
+	.probe = rn5t618_adc_probe,
+};
+
+module_platform_driver(rn5t618_adc_driver);
+MODULE_ALIAS("platform:rn5t618-adc");
+MODULE_DESCRIPTION("RICOH RN5T618 ADC driver");
+MODULE_LICENSE("GPL");
-- 
2.20.1

Re: [PATCH v5 1/2] mfd: rn5t618: add ADC subdevice for RC5T619

[Lee Jones]

On Sun, 23 Feb 2020, Andreas Kemnade wrote:

> This adds a subdevice for the ADC in the RC5T619
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> depends on:
> https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/
> 
> Changes in v3:
> re-added it to the series because of
> "Oh, it looks like there was a conflict.  Could you collect any Acks
> (including mine) rebase and resend please?"

Looks like there is still a conflict.  Sure, it's not a complicated
fix, but that's beside the point.  What tree is this set based on?

>  drivers/mfd/rn5t618.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
> index 073de8e0e78b..321836f78120 100644
> --- a/drivers/mfd/rn5t618.c
> +++ b/drivers/mfd/rn5t618.c
> @@ -24,6 +24,7 @@ static const struct mfd_cell rn5t618_cells[] = {
>  };
>  
>  static const struct mfd_cell rc5t619_cells[] = {
> +	{ .name = "rn5t618-adc" },
>  	{ .name = "rn5t618-regulator" },
>  	{ .name = "rc5t619-rtc" },

In what upstream tree is this line present?

>  	{ .name = "rn5t618-wdt" },

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v5 1/2] mfd: rn5t618: add ADC subdevice for RC5T619

[Andreas Kemnade]

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

On Wed, 26 Feb 2020 15:40:55 +0000
Lee Jones <lee.jones@linaro.org> wrote:

> On Sun, 23 Feb 2020, Andreas Kemnade wrote:
> 
> > This adds a subdevice for the ADC in the RC5T619
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> > depends on:
> > https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/
> > 
> > Changes in v3:
> > re-added it to the series because of
> > "Oh, it looks like there was a conflict.  Could you collect any Acks
> > (including mine) rebase and resend please?"  
> 
> Looks like there is still a conflict.  Sure, it's not a complicated
> fix, but that's beside the point.  What tree is this set based on?
> 
It must be applied on top of my rc5t619 rtc series here:
https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/

I expected it to make it into 5.6 and when I first sent the RTC series
 (in October) I had no idea when I will continue with other stuff.

That is why I sent this ADC series separately, also to give the IIO
maintainer plenty of time to review. 

Do you want me to resend all that pending stuff together in one series?
I have little experience with this multi-subdevice process.

Regards,
Andreas

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

Re: [PATCH v5 1/2] mfd: rn5t618: add ADC subdevice for RC5T619

[Lee Jones]

On Wed, 26 Feb 2020, Andreas Kemnade wrote:

> On Wed, 26 Feb 2020 15:40:55 +0000
> Lee Jones <lee.jones@linaro.org> wrote:
> 
> > On Sun, 23 Feb 2020, Andreas Kemnade wrote:
> > 
> > > This adds a subdevice for the ADC in the RC5T619
> > > 
> > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > ---
> > > depends on:
> > > https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/
> > > 
> > > Changes in v3:
> > > re-added it to the series because of
> > > "Oh, it looks like there was a conflict.  Could you collect any Acks
> > > (including mine) rebase and resend please?"  
> > 
> > Looks like there is still a conflict.  Sure, it's not a complicated
> > fix, but that's beside the point.  What tree is this set based on?
> > 
> It must be applied on top of my rc5t619 rtc series here:
> https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/
> 
> I expected it to make it into 5.6 and when I first sent the RTC series
>  (in October) I had no idea when I will continue with other stuff.
> 
> That is why I sent this ADC series separately, also to give the IIO
> maintainer plenty of time to review. 

If a patch-set can or should be applied on its own, you should send it
based on an upstream commit, or else things like this happen.

My advice would be to maintain topic branches, each based on an
upstream release, which you can merge together into an integration
branch for full coverage testing.

> Do you want me to resend all that pending stuff together in one series?
> I have little experience with this multi-subdevice process.

It makes more sense to rebase this set onto the latest full release
and resubmit this set on its own.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v5 1/2] mfd: rn5t618: add ADC subdevice for RC5T619

[Andreas Kemnade]

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

On Wed, 26 Feb 2020 17:46:40 +0000
Lee Jones <lee.jones@linaro.org> wrote:

> On Wed, 26 Feb 2020, Andreas Kemnade wrote:
> 
> > On Wed, 26 Feb 2020 15:40:55 +0000
> > Lee Jones <lee.jones@linaro.org> wrote:
> >   
> > > On Sun, 23 Feb 2020, Andreas Kemnade wrote:
> > >   
> > > > This adds a subdevice for the ADC in the RC5T619
> > > > 
> > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > ---
> > > > depends on:
> > > > https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/
> > > > 
> > > > Changes in v3:
> > > > re-added it to the series because of
> > > > "Oh, it looks like there was a conflict.  Could you collect any Acks
> > > > (including mine) rebase and resend please?"    
> > > 
> > > Looks like there is still a conflict.  Sure, it's not a complicated
> > > fix, but that's beside the point.  What tree is this set based on?
> > >   
> > It must be applied on top of my rc5t619 rtc series here:
> > https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/
> > 
> > I expected it to make it into 5.6 and when I first sent the RTC series
> >  (in October) I had no idea when I will continue with other stuff.
> > 
> > That is why I sent this ADC series separately, also to give the IIO
> > maintainer plenty of time to review.   
> 
> If a patch-set can or should be applied on its own, you should send it
> based on an upstream commit, or else things like this happen.
> 
It cannot without breaking bisectability. The RTC series adds IRQ support in
the PMIC which is used here.

> My advice would be to maintain topic branches, each based on an
> upstream release, which you can merge together into an integration
> branch for full coverage testing.
> 
> > Do you want me to resend all that pending stuff together in one series?
> > I have little experience with this multi-subdevice process.  
> 
> It makes more sense to rebase this set onto the latest full release
> and resubmit this set on its own.
> 
So, I resend the RC5T619 RTC series mentioned above as you answered
upont Nikolaus question and wait with this series until review is
through.
Ok, so wait and rebase it onto 5.7-rc1 or 5.8-rc1 or whatever release
the RTC stuff will appear.
So you are not going to create an ib-mfd-rtc-iio branch.

Regards,
Andreas

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

Re: [PATCH v5 2/2] iio: adc: rn5t618: Add ADC driver for RN5T618/RC5T619

[Andreas Kemnade]

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

Hi,

On Sun, 23 Feb 2020 14:16:38 +0100
Andreas Kemnade <andreas@kemnade.info> wrote:


> +	if (rn5t618->irq_data)
> +		adc->irq = regmap_irq_get_virq(rn5t618->irq_data,
> +					       RN5T618_IRQ_ADC);
> +
RN5T618_IRQ_ADC is defined in the RTC patchset mentioned as dependency,
which adds IRQ support and I am asked to rebase it on top of plain mainline.
Does that mean that the kernel git source does not need to be bisectable
anymore?


Regards,
Andreas

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

Re: [PATCH v5 1/2] mfd: rn5t618: add ADC subdevice for RC5T619

[Lee Jones]

On Wed, 26 Feb 2020, Andreas Kemnade wrote:

> On Wed, 26 Feb 2020 17:46:40 +0000
> Lee Jones <lee.jones@linaro.org> wrote:
> 
> > On Wed, 26 Feb 2020, Andreas Kemnade wrote:
> > 
> > > On Wed, 26 Feb 2020 15:40:55 +0000
> > > Lee Jones <lee.jones@linaro.org> wrote:
> > >   
> > > > On Sun, 23 Feb 2020, Andreas Kemnade wrote:
> > > >   
> > > > > This adds a subdevice for the ADC in the RC5T619
> > > > > 
> > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > > ---
> > > > > depends on:
> > > > > https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/
> > > > > 
> > > > > Changes in v3:
> > > > > re-added it to the series because of
> > > > > "Oh, it looks like there was a conflict.  Could you collect any Acks
> > > > > (including mine) rebase and resend please?"    
> > > > 
> > > > Looks like there is still a conflict.  Sure, it's not a complicated
> > > > fix, but that's beside the point.  What tree is this set based on?
> > > >   
> > > It must be applied on top of my rc5t619 rtc series here:
> > > https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/
> > > 
> > > I expected it to make it into 5.6 and when I first sent the RTC series
> > >  (in October) I had no idea when I will continue with other stuff.
> > > 
> > > That is why I sent this ADC series separately, also to give the IIO
> > > maintainer plenty of time to review.   
> > 
> > If a patch-set can or should be applied on its own, you should send it
> > based on an upstream commit, or else things like this happen.
> > 
> It cannot without breaking bisectability. The RTC series adds IRQ support in
> the PMIC which is used here.

Then Kconfig should reflect that.

Or, if that's not possible, then you should not break-up and submit
sets which cannot be applied by themselves.  Either submit the whole
set together, or submit them piece by piece, not submitting the next
part until it's predecessor has been applied.

> > My advice would be to maintain topic branches, each based on an
> > upstream release, which you can merge together into an integration
> > branch for full coverage testing.
> > 
> > > Do you want me to resend all that pending stuff together in one series?
> > > I have little experience with this multi-subdevice process.  
> > 
> > It makes more sense to rebase this set onto the latest full release
> > and resubmit this set on its own.
> > 
> So, I resend the RC5T619 RTC series mentioned above as you answered
> upont Nikolaus question and wait with this series until review is
> through.
> Ok, so wait and rebase it onto 5.7-rc1 or 5.8-rc1 or whatever release
> the RTC stuff will appear.
> So you are not going to create an ib-mfd-rtc-iio branch.

As above.

If you go the whole-patch-set route, yes, either myself or someone
else would have to create an immutable pull-request, but you don't
have to concern yourself with that.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v5 1/2] mfd: rn5t618: add ADC subdevice for RC5T619

[Andreas Kemnade]

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

On Thu, 27 Feb 2020 09:40:06 +0000
Lee Jones <lee.jones@linaro.org> wrote:

> On Wed, 26 Feb 2020, Andreas Kemnade wrote:
> 
> > On Wed, 26 Feb 2020 17:46:40 +0000
> > Lee Jones <lee.jones@linaro.org> wrote:
> >   
> > > On Wed, 26 Feb 2020, Andreas Kemnade wrote:
> > >   
> > > > On Wed, 26 Feb 2020 15:40:55 +0000
> > > > Lee Jones <lee.jones@linaro.org> wrote:
> > > >     
> > > > > On Sun, 23 Feb 2020, Andreas Kemnade wrote:
> > > > >     
> > > > > > This adds a subdevice for the ADC in the RC5T619
> > > > > > 
> > > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > > > > > ---
> > > > > > depends on:
> > > > > > https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/
> > > > > > 
> > > > > > Changes in v3:
> > > > > > re-added it to the series because of
> > > > > > "Oh, it looks like there was a conflict.  Could you collect any Acks
> > > > > > (including mine) rebase and resend please?"      
> > > > > 
> > > > > Looks like there is still a conflict.  Sure, it's not a complicated
> > > > > fix, but that's beside the point.  What tree is this set based on?
> > > > >     
> > > > It must be applied on top of my rc5t619 rtc series here:
> > > > https://lore.kernel.org/lkml/20191220122416.31881-1-andreas@kemnade.info/
> > > > 
> > > > I expected it to make it into 5.6 and when I first sent the RTC series
> > > >  (in October) I had no idea when I will continue with other stuff.
> > > > 
> > > > That is why I sent this ADC series separately, also to give the IIO
> > > > maintainer plenty of time to review.     
> > > 
> > > If a patch-set can or should be applied on its own, you should send it
> > > based on an upstream commit, or else things like this happen.
> > >   
> > It cannot without breaking bisectability. The RTC series adds IRQ support in
> > the PMIC which is used here.  
> 
> Then Kconfig should reflect that.
> 
> Or, if that's not possible, then you should not break-up and submit
> sets which cannot be applied by themselves.  Either submit the whole
> set together, or submit them piece by piece, not submitting the next
> part until it's predecessor has been applied.
> 
I will send you a complete series containing both RTC and ADC support.
Then you can decide wether you
1. apply the whole series (both things)
2. apply RTC for 5.7 and this series later
3. ignore them  (not my preferred choice ;-) ).

BTW: The way I did was based on the following note in 
Documentation/process/submitting-patches.rst

"If one patch depends on another patch in order for a change to be
complete, that is OK.  Simply note **"this patch depends on patch X"**
in your patch description."


Regards,
Andreas

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