[PATCH 0/4] iio: adc: mediatek: add mt6765 driver support

[PATCH 0/4] iio: adc: mediatek: add mt6765 driver support

[Chun-Hung Wu]

This patch adds support of auxadc to MT6765 SoC.

Chun-Hung Wu (4):
  dt-bindings: iio: adc: mediatek: Add document for mt6765
  iio: adc: mediatek: mt6765 upstream driver
  iio: adc: mediatek: SET_LATE_SYSTEM_SLEEP_PM_OPS support
  iio: auxadc: mediatek: change to subsys_initcall

 .../devicetree/bindings/iio/adc/mt6577_auxadc.txt  |  1 +
 drivers/iio/adc/mt6577_auxadc.c                    | 75 +++++++++++++++++-----
 2 files changed, 59 insertions(+), 17 deletions(-)

-- 
1.9.1

[PATCH 1/4] dt-bindings: iio: adc: mediatek: Add document for mt6765

[Chun-Hung Wu]

Add compatible node for mt6765 auxadc

Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
---
 Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt b/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt
index 0df9bef..ab7efab 100644
--- a/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt
@@ -15,6 +15,7 @@ Required properties:
     - "mediatek,mt2712-auxadc": For MT2712 family of SoCs
     - "mediatek,mt7622-auxadc": For MT7622 family of SoCs
     - "mediatek,mt8173-auxadc": For MT8173 family of SoCs
+    - "mediatek,mt6765-auxadc": For MT6765 family of SoCs
   - reg: Address range of the AUXADC unit.
   - clocks: Should contain a clock specifier for each entry in clock-names
   - clock-names: Should contain "main".
-- 
1.9.1

Re: [PATCH 1/4] dt-bindings: iio: adc: mediatek: Add document for mt6765

[Jonathan Cameron]

On Thu, 16 May 2019 16:10:44 +0800
Chun-Hung Wu <chun-hung.wu@mediatek.com> wrote:

> Add compatible node for mt6765 auxadc
> 
> Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
Applied, but in numeric order.  There was also another clashing patch
that caused some fuzz.

thanks,

Jonathan

> ---
>  Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt b/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt
> index 0df9bef..ab7efab 100644
> --- a/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt
> @@ -15,6 +15,7 @@ Required properties:
>      - "mediatek,mt2712-auxadc": For MT2712 family of SoCs
>      - "mediatek,mt7622-auxadc": For MT7622 family of SoCs
>      - "mediatek,mt8173-auxadc": For MT8173 family of SoCs
> +    - "mediatek,mt6765-auxadc": For MT6765 family of SoCs
>    - reg: Address range of the AUXADC unit.
>    - clocks: Should contain a clock specifier for each entry in clock-names
>    - clock-names: Should contain "main".

Re: [PATCH 1/4] dt-bindings: iio: adc: mediatek: Add document for mt6765

[Chun-Hung Wu]

Hi Jonathan,

On Sat, 2019-05-18 at 11:33 +0100, Jonathan Cameron wrote:
> On Thu, 16 May 2019 16:10:44 +0800
> Chun-Hung Wu <chun-hung.wu@mediatek.com> wrote:
> 
> > Add compatible node for mt6765 auxadc
> > 
> > Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
> Applied, but in numeric order.  There was also another clashing patch
> that caused some fuzz.
No problem, thanks!

regards,
Chun-Hung
> 
> thanks,
> 
> Jonathan
> 
> > ---
> >  Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt b/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt
> > index 0df9bef..ab7efab 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/mt6577_auxadc.txt
> > @@ -15,6 +15,7 @@ Required properties:
> >      - "mediatek,mt2712-auxadc": For MT2712 family of SoCs
> >      - "mediatek,mt7622-auxadc": For MT7622 family of SoCs
> >      - "mediatek,mt8173-auxadc": For MT8173 family of SoCs
> > +    - "mediatek,mt6765-auxadc": For MT6765 family of SoCs
> >    - reg: Address range of the AUXADC unit.
> >    - clocks: Should contain a clock specifier for each entry in clock-names
> >    - clock-names: Should contain "main".
> 

[PATCH 2/4] iio: adc: mediatek: mt6765 upstream driver

[Chun-Hung Wu]

1. Add calibrated sample data support
2. Use of_match_table to decide each platform's
   feature set

Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
---
 drivers/iio/adc/mt6577_auxadc.c | 54 +++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
index 95d76ab..e1bdcc0 100644
--- a/drivers/iio/adc/mt6577_auxadc.c
+++ b/drivers/iio/adc/mt6577_auxadc.c
@@ -42,10 +42,26 @@
 #define MT6577_AUXADC_POWER_READY_MS          1
 #define MT6577_AUXADC_SAMPLE_READY_US         25
 
+struct mtk_auxadc_compatible {
+	bool sample_data_cali;
+	bool check_global_idle;
+};
+
 struct mt6577_auxadc_device {
 	void __iomem *reg_base;
 	struct clk *adc_clk;
 	struct mutex lock;
+	const struct mtk_auxadc_compatible *dev_comp;
+};
+
+static const struct mtk_auxadc_compatible mt8173_compat = {
+	.sample_data_cali = false,
+	.check_global_idle = true,
+};
+
+static const struct mtk_auxadc_compatible mt6765_compat = {
+	.sample_data_cali = true,
+	.check_global_idle = false,
 };
 
 #define MT6577_AUXADC_CHANNEL(idx) {				    \
@@ -74,6 +90,11 @@ struct mt6577_auxadc_device {
 	MT6577_AUXADC_CHANNEL(15),
 };
 
+static int mt_auxadc_get_cali_data(int rawdata, bool enable_cali)
+{
+	return rawdata;
+}
+
 static inline void mt6577_auxadc_mod_reg(void __iomem *reg,
 					 u32 or_mask, u32 and_mask)
 {
@@ -120,15 +141,17 @@ static int mt6577_auxadc_read(struct iio_dev *indio_dev,
 	/* we must delay here for hardware sample channel data */
 	udelay(MT6577_AUXADC_SAMPLE_READY_US);
 
-	/* check MTK_AUXADC_CON2 if auxadc is idle */
-	ret = readl_poll_timeout(adc_dev->reg_base + MT6577_AUXADC_CON2, val,
-				 ((val & MT6577_AUXADC_STA) == 0),
-				 MT6577_AUXADC_SLEEP_US,
-				 MT6577_AUXADC_TIMEOUT_US);
-	if (ret < 0) {
-		dev_err(indio_dev->dev.parent,
-			"wait for auxadc idle time out\n");
-		goto err_timeout;
+	if (adc_dev->dev_comp->check_global_idle) {
+		/* check MTK_AUXADC_CON2 if auxadc is idle */
+		ret = readl_poll_timeout(adc_dev->reg_base + MT6577_AUXADC_CON2,
+					 val, ((val & MT6577_AUXADC_STA) == 0),
+					 MT6577_AUXADC_SLEEP_US,
+					 MT6577_AUXADC_TIMEOUT_US);
+		if (ret < 0) {
+			dev_err(indio_dev->dev.parent,
+				"wait for auxadc idle time out\n");
+			goto err_timeout;
+		}
 	}
 
 	/* read channel and make sure ready bit == 1 */
@@ -163,6 +186,8 @@ static int mt6577_auxadc_read_raw(struct iio_dev *indio_dev,
 				  int *val2,
 				  long info)
 {
+	struct mt6577_auxadc_device *adc_dev = iio_priv(indio_dev);
+
 	switch (info) {
 	case IIO_CHAN_INFO_PROCESSED:
 		*val = mt6577_auxadc_read(indio_dev, chan);
@@ -172,6 +197,8 @@ static int mt6577_auxadc_read_raw(struct iio_dev *indio_dev,
 				chan->channel);
 			return *val;
 		}
+		if (adc_dev->dev_comp->sample_data_cali)
+			*val = mt_auxadc_get_cali_data(*val, true);
 		return IIO_VAL_INT;
 
 	default:
@@ -304,10 +331,11 @@ static SIMPLE_DEV_PM_OPS(mt6577_auxadc_pm_ops,
 			 mt6577_auxadc_resume);
 
 static const struct of_device_id mt6577_auxadc_of_match[] = {
-	{ .compatible = "mediatek,mt2701-auxadc", },
-	{ .compatible = "mediatek,mt2712-auxadc", },
-	{ .compatible = "mediatek,mt7622-auxadc", },
-	{ .compatible = "mediatek,mt8173-auxadc", },
+	{ .compatible = "mediatek,mt2701-auxadc", .data = &mt8173_compat},
+	{ .compatible = "mediatek,mt2712-auxadc", .data = &mt8173_compat},
+	{ .compatible = "mediatek,mt7622-auxadc", .data = &mt8173_compat},
+	{ .compatible = "mediatek,mt8173-auxadc", .data = &mt8173_compat},
+	{ .compatible = "mediatek,mt6765-auxadc", .data = &mt6765_compat},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, mt6577_auxadc_of_match);
-- 
1.9.1

Re: [PATCH 2/4] iio: adc: mediatek: mt6765 upstream driver

[Jonathan Cameron]

On Thu, 16 May 2019 16:10:45 +0800
Chun-Hung Wu <chun-hung.wu@mediatek.com> wrote:

Hi, 

I changed the title to just say we were adding support to
the existing driver.

Applied to the togreg branch of iio.git and pushed out as
testing for the autobuilders to play with it.

Thanks,

Jonathan



> 1. Add calibrated sample data support
> 2. Use of_match_table to decide each platform's
>    feature set
> 
> Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
> ---
>  drivers/iio/adc/mt6577_auxadc.c | 54 +++++++++++++++++++++++++++++++----------
>  1 file changed, 41 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> index 95d76ab..e1bdcc0 100644
> --- a/drivers/iio/adc/mt6577_auxadc.c
> +++ b/drivers/iio/adc/mt6577_auxadc.c
> @@ -42,10 +42,26 @@
>  #define MT6577_AUXADC_POWER_READY_MS          1
>  #define MT6577_AUXADC_SAMPLE_READY_US         25
>  
> +struct mtk_auxadc_compatible {
> +	bool sample_data_cali;
> +	bool check_global_idle;
> +};
> +
>  struct mt6577_auxadc_device {
>  	void __iomem *reg_base;
>  	struct clk *adc_clk;
>  	struct mutex lock;
> +	const struct mtk_auxadc_compatible *dev_comp;
> +};
> +
> +static const struct mtk_auxadc_compatible mt8173_compat = {
> +	.sample_data_cali = false,
> +	.check_global_idle = true,
> +};
> +
> +static const struct mtk_auxadc_compatible mt6765_compat = {
> +	.sample_data_cali = true,
> +	.check_global_idle = false,
>  };
>  
>  #define MT6577_AUXADC_CHANNEL(idx) {				    \
> @@ -74,6 +90,11 @@ struct mt6577_auxadc_device {
>  	MT6577_AUXADC_CHANNEL(15),
>  };
>  
> +static int mt_auxadc_get_cali_data(int rawdata, bool enable_cali)
> +{
> +	return rawdata;
> +}
> +
>  static inline void mt6577_auxadc_mod_reg(void __iomem *reg,
>  					 u32 or_mask, u32 and_mask)
>  {
> @@ -120,15 +141,17 @@ static int mt6577_auxadc_read(struct iio_dev *indio_dev,
>  	/* we must delay here for hardware sample channel data */
>  	udelay(MT6577_AUXADC_SAMPLE_READY_US);
>  
> -	/* check MTK_AUXADC_CON2 if auxadc is idle */
> -	ret = readl_poll_timeout(adc_dev->reg_base + MT6577_AUXADC_CON2, val,
> -				 ((val & MT6577_AUXADC_STA) == 0),
> -				 MT6577_AUXADC_SLEEP_US,
> -				 MT6577_AUXADC_TIMEOUT_US);
> -	if (ret < 0) {
> -		dev_err(indio_dev->dev.parent,
> -			"wait for auxadc idle time out\n");
> -		goto err_timeout;
> +	if (adc_dev->dev_comp->check_global_idle) {
> +		/* check MTK_AUXADC_CON2 if auxadc is idle */
> +		ret = readl_poll_timeout(adc_dev->reg_base + MT6577_AUXADC_CON2,
> +					 val, ((val & MT6577_AUXADC_STA) == 0),
> +					 MT6577_AUXADC_SLEEP_US,
> +					 MT6577_AUXADC_TIMEOUT_US);
> +		if (ret < 0) {
> +			dev_err(indio_dev->dev.parent,
> +				"wait for auxadc idle time out\n");
> +			goto err_timeout;
> +		}
>  	}
>  
>  	/* read channel and make sure ready bit == 1 */
> @@ -163,6 +186,8 @@ static int mt6577_auxadc_read_raw(struct iio_dev *indio_dev,
>  				  int *val2,
>  				  long info)
>  {
> +	struct mt6577_auxadc_device *adc_dev = iio_priv(indio_dev);
> +
>  	switch (info) {
>  	case IIO_CHAN_INFO_PROCESSED:
>  		*val = mt6577_auxadc_read(indio_dev, chan);
> @@ -172,6 +197,8 @@ static int mt6577_auxadc_read_raw(struct iio_dev *indio_dev,
>  				chan->channel);
>  			return *val;
>  		}
> +		if (adc_dev->dev_comp->sample_data_cali)
> +			*val = mt_auxadc_get_cali_data(*val, true);
>  		return IIO_VAL_INT;
>  
>  	default:
> @@ -304,10 +331,11 @@ static SIMPLE_DEV_PM_OPS(mt6577_auxadc_pm_ops,
>  			 mt6577_auxadc_resume);
>  
>  static const struct of_device_id mt6577_auxadc_of_match[] = {
> -	{ .compatible = "mediatek,mt2701-auxadc", },
> -	{ .compatible = "mediatek,mt2712-auxadc", },
> -	{ .compatible = "mediatek,mt7622-auxadc", },
> -	{ .compatible = "mediatek,mt8173-auxadc", },
> +	{ .compatible = "mediatek,mt2701-auxadc", .data = &mt8173_compat},
> +	{ .compatible = "mediatek,mt2712-auxadc", .data = &mt8173_compat},
> +	{ .compatible = "mediatek,mt7622-auxadc", .data = &mt8173_compat},
> +	{ .compatible = "mediatek,mt8173-auxadc", .data = &mt8173_compat},
> +	{ .compatible = "mediatek,mt6765-auxadc", .data = &mt6765_compat},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, mt6577_auxadc_of_match);

[PATCH 3/4] iio: adc: mediatek: SET_LATE_SYSTEM_SLEEP_PM_OPS support

[Chun-Hung Wu]

  Move suspend/resume to late_suspend and
early_resume to gurantee users can use auxadc
driver at suspend/resume stage.

Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
---
 drivers/iio/adc/mt6577_auxadc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
index e1bdcc0..58d7cb2 100644
--- a/drivers/iio/adc/mt6577_auxadc.c
+++ b/drivers/iio/adc/mt6577_auxadc.c
@@ -326,9 +326,10 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(mt6577_auxadc_pm_ops,
-			 mt6577_auxadc_suspend,
-			 mt6577_auxadc_resume);
+static const struct dev_pm_ops mt6577_auxadc_pm_ops = {
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(mt6577_auxadc_suspend,
+				     mt6577_auxadc_resume)
+};
 
 static const struct of_device_id mt6577_auxadc_of_match[] = {
 	{ .compatible = "mediatek,mt2701-auxadc", .data = &mt8173_compat},
-- 
1.9.1

Re: [PATCH 3/4] iio: adc: mediatek: SET_LATE_SYSTEM_SLEEP_PM_OPS support

[Jonathan Cameron]

On Thu, 16 May 2019 16:10:46 +0800
Chun-Hung Wu <chun-hung.wu@mediatek.com> wrote:

>   Move suspend/resume to late_suspend and
> early_resume to gurantee users can use auxadc
guarantee

> driver at suspend/resume stage.
No problem with the patch content, but we need a reason why they may
want to do so?

Thanks,

Jonathan
> 
> Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
> ---
>  drivers/iio/adc/mt6577_auxadc.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> index e1bdcc0..58d7cb2 100644
> --- a/drivers/iio/adc/mt6577_auxadc.c
> +++ b/drivers/iio/adc/mt6577_auxadc.c
> @@ -326,9 +326,10 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static SIMPLE_DEV_PM_OPS(mt6577_auxadc_pm_ops,
> -			 mt6577_auxadc_suspend,
> -			 mt6577_auxadc_resume);
> +static const struct dev_pm_ops mt6577_auxadc_pm_ops = {
> +	SET_LATE_SYSTEM_SLEEP_PM_OPS(mt6577_auxadc_suspend,
> +				     mt6577_auxadc_resume)
> +};
>  
>  static const struct of_device_id mt6577_auxadc_of_match[] = {
>  	{ .compatible = "mediatek,mt2701-auxadc", .data = &mt8173_compat},

Re: [PATCH 3/4] iio: adc: mediatek: SET_LATE_SYSTEM_SLEEP_PM_OPS support

[Chun-Hung Wu]

Hi Jonathan:

  Thanks for the prompt reply,

On Sat, 2019-05-18 at 11:35 +0100, Jonathan Cameron wrote:
> On Thu, 16 May 2019 16:10:46 +0800
> Chun-Hung Wu <chun-hung.wu@mediatek.com> wrote:
> 
> >   Move suspend/resume to late_suspend and
> > early_resume to gurantee users can use auxadc
> guarantee
> 
will fix it in next version.
> > driver at suspend/resume stage.
> No problem with the patch content, but we need a reason why they may
> want to do so?
Our thermal drivers uses auxadc at suspend/resume stage.
In order to avoid auxadc suspended prior to thermal driver,
we move auxadc to late_suspend and early_resume.

Thanks,
Chun-Hung
> 
> Thanks,
> 
> Jonathan
> > 
> > Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
> > ---
> >  drivers/iio/adc/mt6577_auxadc.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> > index e1bdcc0..58d7cb2 100644
> > --- a/drivers/iio/adc/mt6577_auxadc.c
> > +++ b/drivers/iio/adc/mt6577_auxadc.c
> > @@ -326,9 +326,10 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > -static SIMPLE_DEV_PM_OPS(mt6577_auxadc_pm_ops,
> > -			 mt6577_auxadc_suspend,
> > -			 mt6577_auxadc_resume);
> > +static const struct dev_pm_ops mt6577_auxadc_pm_ops = {
> > +	SET_LATE_SYSTEM_SLEEP_PM_OPS(mt6577_auxadc_suspend,
> > +				     mt6577_auxadc_resume)
> > +};
> >  
> >  static const struct of_device_id mt6577_auxadc_of_match[] = {
> >  	{ .compatible = "mediatek,mt2701-auxadc", .data = &mt8173_compat},
> 

Re: [PATCH 3/4] iio: adc: mediatek: SET_LATE_SYSTEM_SLEEP_PM_OPS support

[Matthias Brugger]

On 20/05/2019 08:03, Chun-Hung Wu wrote:
> Hi Jonathan:
> 
>   Thanks for the prompt reply,
> 
> On Sat, 2019-05-18 at 11:35 +0100, Jonathan Cameron wrote:
>> On Thu, 16 May 2019 16:10:46 +0800
>> Chun-Hung Wu <chun-hung.wu@mediatek.com> wrote:
>>
>>>   Move suspend/resume to late_suspend and
>>> early_resume to gurantee users can use auxadc
>> guarantee
>>
> will fix it in next version.
>>> driver at suspend/resume stage.
>> No problem with the patch content, but we need a reason why they may
>> want to do so?
> Our thermal drivers uses auxadc at suspend/resume stage.
> In order to avoid auxadc suspended prior to thermal driver,
> we move auxadc to late_suspend and early_resume.
> 

That sounds like a user of device_links [1] to me.

[1] https://www.kernel.org/doc/html/latest/driver-api/device_link.html

> Thanks,
> Chun-Hung
>>
>> Thanks,
>>
>> Jonathan
>>>
>>> Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
>>> ---
>>>  drivers/iio/adc/mt6577_auxadc.c | 7 ++++---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
>>> index e1bdcc0..58d7cb2 100644
>>> --- a/drivers/iio/adc/mt6577_auxadc.c
>>> +++ b/drivers/iio/adc/mt6577_auxadc.c
>>> @@ -326,9 +326,10 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
>>>  	return 0;
>>>  }
>>>  
>>> -static SIMPLE_DEV_PM_OPS(mt6577_auxadc_pm_ops,
>>> -			 mt6577_auxadc_suspend,
>>> -			 mt6577_auxadc_resume);
>>> +static const struct dev_pm_ops mt6577_auxadc_pm_ops = {
>>> +	SET_LATE_SYSTEM_SLEEP_PM_OPS(mt6577_auxadc_suspend,
>>> +				     mt6577_auxadc_resume)
>>> +};
>>>  
>>>  static const struct of_device_id mt6577_auxadc_of_match[] = {
>>>  	{ .compatible = "mediatek,mt2701-auxadc", .data = &mt8173_compat},
>>
> 
> 

Re: [PATCH 3/4] iio: adc: mediatek: SET_LATE_SYSTEM_SLEEP_PM_OPS support

[Chun-Hung Wu]

Hi Matthias:

  Thanks for your suggestion, I think device_links is a good way to
make dependency of module's suspend/resume order.

Hi Jonathan:

  Is it ok to keep using late_suspend and early_resume, or do you think
it's better to use device_links?

Thanks,
Chun-Hung
On Wed, 2019-05-22 at 18:28 +0200, Matthias Brugger wrote:
> 
> On 20/05/2019 08:03, Chun-Hung Wu wrote:
> > Hi Jonathan:
> > 
> >   Thanks for the prompt reply,
> > 
> > On Sat, 2019-05-18 at 11:35 +0100, Jonathan Cameron wrote:
> >> On Thu, 16 May 2019 16:10:46 +0800
> >> Chun-Hung Wu <chun-hung.wu@mediatek.com> wrote:
> >>
> >>>   Move suspend/resume to late_suspend and
> >>> early_resume to gurantee users can use auxadc
> >> guarantee
> >>
> > will fix it in next version.
> >>> driver at suspend/resume stage.
> >> No problem with the patch content, but we need a reason why they may
> >> want to do so?
> > Our thermal drivers uses auxadc at suspend/resume stage.
> > In order to avoid auxadc suspended prior to thermal driver,
> > we move auxadc to late_suspend and early_resume.
> > 
> 
> That sounds like a user of device_links [1] to me.
> 
> [1] https://www.kernel.org/doc/html/latest/driver-api/device_link.html
> 
> > Thanks,
> > Chun-Hung
> >>
> >> Thanks,
> >>
> >> Jonathan
> >>>
> >>> Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
> >>> ---
> >>>  drivers/iio/adc/mt6577_auxadc.c | 7 ++++---
> >>>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> >>> index e1bdcc0..58d7cb2 100644
> >>> --- a/drivers/iio/adc/mt6577_auxadc.c
> >>> +++ b/drivers/iio/adc/mt6577_auxadc.c
> >>> @@ -326,9 +326,10 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
> >>>  	return 0;
> >>>  }
> >>>  
> >>> -static SIMPLE_DEV_PM_OPS(mt6577_auxadc_pm_ops,
> >>> -			 mt6577_auxadc_suspend,
> >>> -			 mt6577_auxadc_resume);
> >>> +static const struct dev_pm_ops mt6577_auxadc_pm_ops = {
> >>> +	SET_LATE_SYSTEM_SLEEP_PM_OPS(mt6577_auxadc_suspend,
> >>> +				     mt6577_auxadc_resume)
> >>> +};
> >>>  
> >>>  static const struct of_device_id mt6577_auxadc_of_match[] = {
> >>>  	{ .compatible = "mediatek,mt2701-auxadc", .data = &mt8173_compat},
> >>
> > 
> > 

Re: [PATCH 3/4] iio: adc: mediatek: SET_LATE_SYSTEM_SLEEP_PM_OPS support

[Jonathan Cameron]

On Tue, 28 May 2019 18:59:56 +0800
Chun-Hung Wu <chun-hung.wu@mediatek.com> wrote:

> Hi Matthias:
> 
>   Thanks for your suggestion, I think device_links is a good way to
> make dependency of module's suspend/resume order.
> 
> Hi Jonathan:
> 
>   Is it ok to keep using late_suspend and early_resume, or do you think
> it's better to use device_links?
I think device links would be preferable as the reasoning becomes explicit.
As I understand them they are also a less fragile solution.

Thanks,

Jonathan

> 
> Thanks,
> Chun-Hung
> On Wed, 2019-05-22 at 18:28 +0200, Matthias Brugger wrote:
> > 
> > On 20/05/2019 08:03, Chun-Hung Wu wrote:  
> > > Hi Jonathan:
> > > 
> > >   Thanks for the prompt reply,
> > > 
> > > On Sat, 2019-05-18 at 11:35 +0100, Jonathan Cameron wrote:  
> > >> On Thu, 16 May 2019 16:10:46 +0800
> > >> Chun-Hung Wu <chun-hung.wu@mediatek.com> wrote:
> > >>  
> > >>>   Move suspend/resume to late_suspend and
> > >>> early_resume to gurantee users can use auxadc  
> > >> guarantee
> > >>  
> > > will fix it in next version.  
> > >>> driver at suspend/resume stage.  
> > >> No problem with the patch content, but we need a reason why they may
> > >> want to do so?  
> > > Our thermal drivers uses auxadc at suspend/resume stage.
> > > In order to avoid auxadc suspended prior to thermal driver,
> > > we move auxadc to late_suspend and early_resume.
> > >   
> > 
> > That sounds like a user of device_links [1] to me.
> > 
> > [1] https://www.kernel.org/doc/html/latest/driver-api/device_link.html
> >   
> > > Thanks,
> > > Chun-Hung  
> > >>
> > >> Thanks,
> > >>
> > >> Jonathan  
> > >>>
> > >>> Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
> > >>> ---
> > >>>  drivers/iio/adc/mt6577_auxadc.c | 7 ++++---
> > >>>  1 file changed, 4 insertions(+), 3 deletions(-)
> > >>>
> > >>> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> > >>> index e1bdcc0..58d7cb2 100644
> > >>> --- a/drivers/iio/adc/mt6577_auxadc.c
> > >>> +++ b/drivers/iio/adc/mt6577_auxadc.c
> > >>> @@ -326,9 +326,10 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
> > >>>  	return 0;
> > >>>  }
> > >>>  
> > >>> -static SIMPLE_DEV_PM_OPS(mt6577_auxadc_pm_ops,
> > >>> -			 mt6577_auxadc_suspend,
> > >>> -			 mt6577_auxadc_resume);
> > >>> +static const struct dev_pm_ops mt6577_auxadc_pm_ops = {
> > >>> +	SET_LATE_SYSTEM_SLEEP_PM_OPS(mt6577_auxadc_suspend,
> > >>> +				     mt6577_auxadc_resume)
> > >>> +};
> > >>>  
> > >>>  static const struct of_device_id mt6577_auxadc_of_match[] = {
> > >>>  	{ .compatible = "mediatek,mt2701-auxadc", .data = &mt8173_compat},  
> > >>  
> > > 
> > >   
> 
> 

[PATCH 4/4] iio: auxadc: mediatek: change to subsys_initcall

[Chun-Hung Wu]

  Move auxadc platform_driver_register() from module_init
to subsys_initcall because auxadc user drivers
are all moudle drivers, need to gurantee
auxadc driver ready before module_init.

Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
---
 drivers/iio/adc/mt6577_auxadc.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
index 58d7cb2..cb8e3dd 100644
--- a/drivers/iio/adc/mt6577_auxadc.c
+++ b/drivers/iio/adc/mt6577_auxadc.c
@@ -350,7 +350,19 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
 	.probe	= mt6577_auxadc_probe,
 	.remove	= mt6577_auxadc_remove,
 };
-module_platform_driver(mt6577_auxadc_driver);
+
+static int __init mt6577_auxadc_init(void)
+{
+	return platform_driver_register(&mt6577_auxadc_driver);
+}
+
+static void __exit mt6577_auxadc_exit(void)
+{
+	platform_driver_unregister(&mt6577_auxadc_driver);
+}
+
+subsys_initcall(mt6577_auxadc_init);
+module_exit(mt6577_auxadc_exit);
 
 MODULE_AUTHOR("Zhiyong Tao <zhiyong.tao@mediatek.com>");
 MODULE_DESCRIPTION("MTK AUXADC Device Driver");
-- 
1.9.1

Re: [PATCH 4/4] iio: auxadc: mediatek: change to subsys_initcall

[Jonathan Cameron]

On Thu, 16 May 2019 16:10:47 +0800
Chun-Hung Wu <chun-hung.wu@mediatek.com> wrote:

>   Move auxadc platform_driver_register() from module_init
> to subsys_initcall because auxadc user drivers
> are all moudle drivers, need to gurantee
> auxadc driver ready before module_init.
> 
Is it not possible to make them use deferred handling to come
back later if this isn't yet available?

subsys_initcall often ends up being a more fragile approach.

Thanks,

Jonathan

> Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
> ---
>  drivers/iio/adc/mt6577_auxadc.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> index 58d7cb2..cb8e3dd 100644
> --- a/drivers/iio/adc/mt6577_auxadc.c
> +++ b/drivers/iio/adc/mt6577_auxadc.c
> @@ -350,7 +350,19 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
>  	.probe	= mt6577_auxadc_probe,
>  	.remove	= mt6577_auxadc_remove,
>  };
> -module_platform_driver(mt6577_auxadc_driver);
> +
> +static int __init mt6577_auxadc_init(void)
> +{
> +	return platform_driver_register(&mt6577_auxadc_driver);
> +}
> +
> +static void __exit mt6577_auxadc_exit(void)
> +{
> +	platform_driver_unregister(&mt6577_auxadc_driver);
> +}
> +
> +subsys_initcall(mt6577_auxadc_init);
> +module_exit(mt6577_auxadc_exit);
>  
>  MODULE_AUTHOR("Zhiyong Tao <zhiyong.tao@mediatek.com>");
>  MODULE_DESCRIPTION("MTK AUXADC Device Driver");

Re: [PATCH 4/4] iio: auxadc: mediatek: change to subsys_initcall

[Chun-Hung Wu]

Hi Jonathan,

On Sat, 2019-05-18 at 11:36 +0100, Jonathan Cameron wrote:
> On Thu, 16 May 2019 16:10:47 +0800
> Chun-Hung Wu <chun-hung.wu@mediatek.com> wrote:
> 
> >   Move auxadc platform_driver_register() from module_init
> > to subsys_initcall because auxadc user drivers
> > are all moudle drivers, need to gurantee
> > auxadc driver ready before module_init.
> > 
> Is it not possible to make them use deferred handling to come
> back later if this isn't yet available?
> 
> subsys_initcall often ends up being a more fragile approach.

Agreed, I will ask auxadc driver users to add deferred handling
instead of moving auxadc platform_driver_register() from module_init
to subsys_initcall.

Thanks,
Chun-hung

> 
> Thanks,
> 
> Jonathan
> 
> > Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
> > ---
> >  drivers/iio/adc/mt6577_auxadc.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> > index 58d7cb2..cb8e3dd 100644
> > --- a/drivers/iio/adc/mt6577_auxadc.c
> > +++ b/drivers/iio/adc/mt6577_auxadc.c
> > @@ -350,7 +350,19 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
> >  	.probe	= mt6577_auxadc_probe,
> >  	.remove	= mt6577_auxadc_remove,
> >  };
> > -module_platform_driver(mt6577_auxadc_driver);
> > +
> > +static int __init mt6577_auxadc_init(void)
> > +{
> > +	return platform_driver_register(&mt6577_auxadc_driver);
> > +}
> > +
> > +static void __exit mt6577_auxadc_exit(void)
> > +{
> > +	platform_driver_unregister(&mt6577_auxadc_driver);
> > +}
> > +
> > +subsys_initcall(mt6577_auxadc_init);
> > +module_exit(mt6577_auxadc_exit);
> >  
> >  MODULE_AUTHOR("Zhiyong Tao <zhiyong.tao@mediatek.com>");
> >  MODULE_DESCRIPTION("MTK AUXADC Device Driver");
>