Re: [PATCH v2 1/7] iio: adc: palmas: remove adc_wakeupX_data

["Patrik Dahlström" <risca@dalakolonin.se>]

On Mon, Apr 03, 2023 at 06:49:36PM +0200, H. Nikolaus Schaller wrote:
> Hi Patrik,
> 
> > Am 02.04.2023 um 18:42 schrieb Patrik Dahlström <risca@dalakolonin.se>:
> > 
> > It does not seem to be used by anyone and later patches in this series
> > are made simpler by first removing this. There is now a lot of dead code
> > that cannot be reached, until later patches revive it. Arguably, this is
> > preferred over removing the code only to add it again.
> > 
> > Signed-off-by: Patrik Dahlström <risca@dalakolonin.se>
> > ---
> > drivers/iio/adc/palmas_gpadc.c | 50 ++++------------------------------
> > include/linux/mfd/palmas.h     |  8 ------
> > 2 files changed, 6 insertions(+), 52 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
> > index 24d7c096e4b8..943ac579eb1f 100644
> > --- a/drivers/iio/adc/palmas_gpadc.c
> > +++ b/drivers/iio/adc/palmas_gpadc.c
> > @@ -76,6 +76,12 @@ static struct palmas_gpadc_info palmas_gpadc_info[] = {
> > 	PALMAS_ADC_INFO(IN15, 0, 0, 0, 0, INVALID, INVALID, true),
> > };
> > 
> > +struct palmas_adc_wakeup_property {
> > +	int adc_channel_number;
> > +	int adc_high_threshold;
> > +	int adc_low_threshold;
> > +};
> > +
> > /*
> >  * struct palmas_gpadc - the palmas_gpadc structure
> >  * @ch0_current:	channel 0 current source setting
> > @@ -492,11 +498,6 @@ static int palmas_gpadc_get_adc_dt_data(struct platform_device *pdev,
> > 	return 0;
> > }
> > 
> > -static void palmas_disable_wakeup(void *dev)
> 
> something seems to be mixed up here.
> 
> There is no palmas_disable_wakeup() upstream that can be removed. So this patch
> can not be applied as 1/7 to any upstream kernel.
> 
> Please rebase your series on either linus/master or linux-next/master.

I'm sorry for the confusion. I should have been more clear in the cover
letter.

This series is based on Jonathan Cameron's iio tree[1], plus the patches at
[2] and [3]. The first patch is already part of linux-next and I was under
the impression that [3] would be soon too.

Would it be best to rebase this series on linux-next instead?

[1] git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
[2] https://patchwork.kernel.org/project/linux-iio/patch/20230313205029.1881745-1-risca@dalakolonin.se/
[3] https://patchwork.kernel.org/project/linux-iio/patch/20230318163039.56115-1-jic23@kernel.org/

> 
> BR,
> Nikolaus
> 
> > -{
> > -	device_wakeup_disable(dev);
> > -}
> > -
> > static int palmas_gpadc_probe(struct platform_device *pdev)
> > {
> > 	struct palmas_gpadc *adc;
> > @@ -547,36 +548,6 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
> > 		return dev_err_probe(adc->dev, ret,
> > 				     "request irq %d failed\n", adc->irq);
> > 
> > -	if (gpadc_pdata->adc_wakeup1_data) {
> > -		memcpy(&adc->wakeup1_data, gpadc_pdata->adc_wakeup1_data,
> > -			sizeof(adc->wakeup1_data));
> > -		adc->wakeup1_enable = true;
> > -		adc->irq_auto_0 =  platform_get_irq(pdev, 1);
> > -		ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_0,
> > -						NULL, palmas_gpadc_irq_auto,
> > -						IRQF_ONESHOT,
> > -						"palmas-adc-auto-0", adc);
> > -		if (ret < 0)
> > -			return dev_err_probe(adc->dev, ret,
> > -					     "request auto0 irq %d failed\n",
> > -					     adc->irq_auto_0);
> > -	}
> > -
> > -	if (gpadc_pdata->adc_wakeup2_data) {
> > -		memcpy(&adc->wakeup2_data, gpadc_pdata->adc_wakeup2_data,
> > -				sizeof(adc->wakeup2_data));
> > -		adc->wakeup2_enable = true;
> > -		adc->irq_auto_1 =  platform_get_irq(pdev, 2);
> > -		ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_1,
> > -						NULL, palmas_gpadc_irq_auto,
> > -						IRQF_ONESHOT,
> > -						"palmas-adc-auto-1", adc);
> > -		if (ret < 0)
> > -			return dev_err_probe(adc->dev, ret,
> > -					     "request auto1 irq %d failed\n",
> > -					     adc->irq_auto_1);
> > -	}
> > -
> > 	/* set the current source 0 (value 0/5/15/20 uA => 0..3) */
> > 	if (gpadc_pdata->ch0_current <= 1)
> > 		adc->ch0_current = PALMAS_ADC_CH0_CURRENT_SRC_0;
> > @@ -616,15 +587,6 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
> > 			palmas_gpadc_calibrate(adc, i);
> > 	}
> > 
> > -	if (adc->wakeup1_enable || adc->wakeup2_enable) {
> > -		device_wakeup_enable(&pdev->dev);
> > -		ret = devm_add_action_or_reset(&pdev->dev,
> > -					       palmas_disable_wakeup,
> > -					       &pdev->dev);
> > -		if (ret)
> > -			return ret;
> > -	}
> > -
> > 	return 0;
> > }
> > 
> > diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
> > index 1e61c7e9f50d..55f22adb1a9e 100644
> > --- a/include/linux/mfd/palmas.h
> > +++ b/include/linux/mfd/palmas.h
> > @@ -129,12 +129,6 @@ struct palmas_pmic_driver_data {
> > 			    struct regulator_config config);
> > };
> > 
> > -struct palmas_adc_wakeup_property {
> > -	int adc_channel_number;
> > -	int adc_high_threshold;
> > -	int adc_low_threshold;
> > -};
> > -
> > struct palmas_gpadc_platform_data {
> > 	/* Channel 3 current source is only enabled during conversion */
> > 	int ch3_current;	/* 0: off; 1: 10uA; 2: 400uA; 3: 800 uA */
> > @@ -153,8 +147,6 @@ struct palmas_gpadc_platform_data {
> > 	int start_polarity;
> > 
> > 	int auto_conversion_period_ms;
> > -	struct palmas_adc_wakeup_property *adc_wakeup1_data;
> > -	struct palmas_adc_wakeup_property *adc_wakeup2_data;
> > };
> > 
> > struct palmas_reg_init {
> > -- 
> > 2.25.1
> > 
>