All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] iio: adc: mt6577_auxadc: add optional 32k clock
@ 2022-10-19 14:37 ` Daniel Golle
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Golle @ 2022-10-19 14:37 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen, Matthias Brugger, linux-iio
  Cc: David Bauer, Gwendal Grignou, AngeloGioacchino Del Regno,
	linux-arm-kernel, linux-mediatek, linux-kernel

MediaTek MT7986 and MT7981 require an additional clock to be brought up
for AUXADC. Add support for that in the driver, similar to how it's
done in MediaTek's SDK[1].

[1]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/target/linux/mediatek/patches-5.4/500-auxadc-add-auxadc-32k-clk.patch
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/iio/adc/mt6577_auxadc.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
index 0e134777bdd248..e28e9691cae22a 100644
--- a/drivers/iio/adc/mt6577_auxadc.c
+++ b/drivers/iio/adc/mt6577_auxadc.c
@@ -42,6 +42,7 @@ struct mtk_auxadc_compatible {
 struct mt6577_auxadc_device {
 	void __iomem *reg_base;
 	struct clk *adc_clk;
+	struct clk *adc_32k_clk;
 	struct mutex lock;
 	const struct mtk_auxadc_compatible *dev_comp;
 };
@@ -227,6 +228,12 @@ static int mt6577_auxadc_resume(struct device *dev)
 		return ret;
 	}
 
+	ret = clk_prepare_enable(adc_dev->adc_32k_clk);
+	if (ret) {
+		pr_err("failed to enable auxadc clock\n");
+		return ret;
+	}
+
 	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
 			      MT6577_AUXADC_PDN_EN, 0);
 	mdelay(MT6577_AUXADC_POWER_READY_MS);
@@ -241,6 +248,8 @@ static int mt6577_auxadc_suspend(struct device *dev)
 
 	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
 			      0, MT6577_AUXADC_PDN_EN);
+
+	clk_disable_unprepare(adc_dev->adc_32k_clk);
 	clk_disable_unprepare(adc_dev->adc_clk);
 
 	return 0;
@@ -282,6 +291,17 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	adc_dev->adc_32k_clk = devm_clk_get_optional(&pdev->dev, "32k");
+	if (IS_ERR(adc_dev->adc_32k_clk)) {
+		dev_err(&pdev->dev, "failed to get auxadc 32k clock\n");
+		return PTR_ERR(adc_dev->adc_32k_clk);
+	}
+	ret = clk_prepare_enable(adc_dev->adc_32k_clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable auxadc 32k clock\n");
+		return ret;
+	}
+
 	adc_clk_rate = clk_get_rate(adc_dev->adc_clk);
 	if (!adc_clk_rate) {
 		ret = -EINVAL;
@@ -311,6 +331,7 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
 	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
 			      0, MT6577_AUXADC_PDN_EN);
 err_disable_clk:
+	clk_disable_unprepare(adc_dev->adc_32k_clk);
 	clk_disable_unprepare(adc_dev->adc_clk);
 	return ret;
 }
@@ -325,6 +346,7 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
 	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
 			      0, MT6577_AUXADC_PDN_EN);
 
+	clk_disable_unprepare(adc_dev->adc_32k_clk);
 	clk_disable_unprepare(adc_dev->adc_clk);
 
 	return 0;
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 1/2] iio: adc: mt6577_auxadc: add optional 32k clock
@ 2022-10-19 14:37 ` Daniel Golle
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Golle @ 2022-10-19 14:37 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen, Matthias Brugger, linux-iio
  Cc: David Bauer, Gwendal Grignou, AngeloGioacchino Del Regno,
	linux-arm-kernel, linux-mediatek, linux-kernel

MediaTek MT7986 and MT7981 require an additional clock to be brought up
for AUXADC. Add support for that in the driver, similar to how it's
done in MediaTek's SDK[1].

[1]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/target/linux/mediatek/patches-5.4/500-auxadc-add-auxadc-32k-clk.patch
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/iio/adc/mt6577_auxadc.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
index 0e134777bdd248..e28e9691cae22a 100644
--- a/drivers/iio/adc/mt6577_auxadc.c
+++ b/drivers/iio/adc/mt6577_auxadc.c
@@ -42,6 +42,7 @@ struct mtk_auxadc_compatible {
 struct mt6577_auxadc_device {
 	void __iomem *reg_base;
 	struct clk *adc_clk;
+	struct clk *adc_32k_clk;
 	struct mutex lock;
 	const struct mtk_auxadc_compatible *dev_comp;
 };
@@ -227,6 +228,12 @@ static int mt6577_auxadc_resume(struct device *dev)
 		return ret;
 	}
 
+	ret = clk_prepare_enable(adc_dev->adc_32k_clk);
+	if (ret) {
+		pr_err("failed to enable auxadc clock\n");
+		return ret;
+	}
+
 	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
 			      MT6577_AUXADC_PDN_EN, 0);
 	mdelay(MT6577_AUXADC_POWER_READY_MS);
@@ -241,6 +248,8 @@ static int mt6577_auxadc_suspend(struct device *dev)
 
 	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
 			      0, MT6577_AUXADC_PDN_EN);
+
+	clk_disable_unprepare(adc_dev->adc_32k_clk);
 	clk_disable_unprepare(adc_dev->adc_clk);
 
 	return 0;
@@ -282,6 +291,17 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	adc_dev->adc_32k_clk = devm_clk_get_optional(&pdev->dev, "32k");
+	if (IS_ERR(adc_dev->adc_32k_clk)) {
+		dev_err(&pdev->dev, "failed to get auxadc 32k clock\n");
+		return PTR_ERR(adc_dev->adc_32k_clk);
+	}
+	ret = clk_prepare_enable(adc_dev->adc_32k_clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable auxadc 32k clock\n");
+		return ret;
+	}
+
 	adc_clk_rate = clk_get_rate(adc_dev->adc_clk);
 	if (!adc_clk_rate) {
 		ret = -EINVAL;
@@ -311,6 +331,7 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
 	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
 			      0, MT6577_AUXADC_PDN_EN);
 err_disable_clk:
+	clk_disable_unprepare(adc_dev->adc_32k_clk);
 	clk_disable_unprepare(adc_dev->adc_clk);
 	return ret;
 }
@@ -325,6 +346,7 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
 	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
 			      0, MT6577_AUXADC_PDN_EN);
 
+	clk_disable_unprepare(adc_dev->adc_32k_clk);
 	clk_disable_unprepare(adc_dev->adc_clk);
 
 	return 0;
-- 
2.37.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] dt-bindings: iio: adc: mediatek,mt2701-auxadc: new 32k clock
  2022-10-19 14:37 ` Daniel Golle
@ 2022-10-19 14:38   ` Daniel Golle
  -1 siblings, 0 replies; 10+ messages in thread
From: Daniel Golle @ 2022-10-19 14:38 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen, Matthias Brugger, linux-iio
  Cc: Gwendal Grignou, AngeloGioacchino Del Regno, linux-arm-kernel,
	linux-mediatek, linux-kernel

Newer MediaTek SoCs need an additional clock to be brought up for
AUXADC to work. Add this new optional clock to
mediatek,mt2701-auxadc.yaml.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 .../bindings/iio/adc/mediatek,mt2701-auxadc.yaml          | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
index 7f79a06e76f596..c2a1813dd54152 100644
--- a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
@@ -44,10 +44,14 @@ properties:
     maxItems: 1
 
   clocks:
-    maxItems: 1
+    maxItems: 2
+    minItems: 1
 
   clock-names:
-    const: main
+    items:
+      - const: main
+      - const: 32k
+    minItems: 1
 
   "#io-channel-cells":
     const: 1
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] dt-bindings: iio: adc: mediatek,mt2701-auxadc: new 32k clock
@ 2022-10-19 14:38   ` Daniel Golle
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Golle @ 2022-10-19 14:38 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen, Matthias Brugger, linux-iio
  Cc: Gwendal Grignou, AngeloGioacchino Del Regno, linux-arm-kernel,
	linux-mediatek, linux-kernel

Newer MediaTek SoCs need an additional clock to be brought up for
AUXADC to work. Add this new optional clock to
mediatek,mt2701-auxadc.yaml.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 .../bindings/iio/adc/mediatek,mt2701-auxadc.yaml          | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
index 7f79a06e76f596..c2a1813dd54152 100644
--- a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
@@ -44,10 +44,14 @@ properties:
     maxItems: 1
 
   clocks:
-    maxItems: 1
+    maxItems: 2
+    minItems: 1
 
   clock-names:
-    const: main
+    items:
+      - const: main
+      - const: 32k
+    minItems: 1
 
   "#io-channel-cells":
     const: 1
-- 
2.37.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] dt-bindings: iio: adc: mediatek,mt2701-auxadc: new 32k clock
  2022-10-19 14:38   ` Daniel Golle
@ 2022-10-20  8:28     ` AngeloGioacchino Del Regno
  -1 siblings, 0 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-10-20  8:28 UTC (permalink / raw)
  To: Daniel Golle, Jonathan Cameron, Lars-Peter Clausen,
	Matthias Brugger, linux-iio
  Cc: Gwendal Grignou, linux-arm-kernel, linux-mediatek, linux-kernel

Il 19/10/22 16:38, Daniel Golle ha scritto:
> Newer MediaTek SoCs need an additional clock to be brought up for
> AUXADC to work. Add this new optional clock to
> mediatek,mt2701-auxadc.yaml.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>   .../bindings/iio/adc/mediatek,mt2701-auxadc.yaml          | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> index 7f79a06e76f596..c2a1813dd54152 100644
> --- a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> @@ -44,10 +44,14 @@ properties:
>       maxItems: 1
>   
>     clocks:
> -    maxItems: 1
> +    maxItems: 2
> +    minItems: 1
>   
>     clock-names:
> -    const: main
> +    items:
> +      - const: main
> +      - const: 32k

You're adding this for MT7986, and I don't see any 32KHz ADC clock on that SoC.
I suppose that your '32k' clock is CLK_INFRA_ADC_FRC_CK, currently parented to
'csw_f26m_sel', so that's 26MHz, not 32KHz.

Since you'll need the same changes for thermal as well, I would consider setting
"infra_adc_frc" as a parent of "infra_adc_26m", like so:

	GATE_INFRA1(CLK_INFRA_ADC_26M_CK, "infra_adc_26m", "infra_adc_frc", 20),
	GATE_INFRA1(CLK_INFRA_ADC_FRC_CK, "infra_adc_frc", "csw_f26m_sel", 21),

...just because there's apparently no reason to have one of them enabled but not
the other or, at least, it looks like we *always* need ADC_26M_CK enabled when
ADC_FRC_CK is enabled.

Regards,
Angelo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] dt-bindings: iio: adc: mediatek,mt2701-auxadc: new 32k clock
@ 2022-10-20  8:28     ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-10-20  8:28 UTC (permalink / raw)
  To: Daniel Golle, Jonathan Cameron, Lars-Peter Clausen,
	Matthias Brugger, linux-iio
  Cc: Gwendal Grignou, linux-arm-kernel, linux-mediatek, linux-kernel

Il 19/10/22 16:38, Daniel Golle ha scritto:
> Newer MediaTek SoCs need an additional clock to be brought up for
> AUXADC to work. Add this new optional clock to
> mediatek,mt2701-auxadc.yaml.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>   .../bindings/iio/adc/mediatek,mt2701-auxadc.yaml          | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> index 7f79a06e76f596..c2a1813dd54152 100644
> --- a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> @@ -44,10 +44,14 @@ properties:
>       maxItems: 1
>   
>     clocks:
> -    maxItems: 1
> +    maxItems: 2
> +    minItems: 1
>   
>     clock-names:
> -    const: main
> +    items:
> +      - const: main
> +      - const: 32k

You're adding this for MT7986, and I don't see any 32KHz ADC clock on that SoC.
I suppose that your '32k' clock is CLK_INFRA_ADC_FRC_CK, currently parented to
'csw_f26m_sel', so that's 26MHz, not 32KHz.

Since you'll need the same changes for thermal as well, I would consider setting
"infra_adc_frc" as a parent of "infra_adc_26m", like so:

	GATE_INFRA1(CLK_INFRA_ADC_26M_CK, "infra_adc_26m", "infra_adc_frc", 20),
	GATE_INFRA1(CLK_INFRA_ADC_FRC_CK, "infra_adc_frc", "csw_f26m_sel", 21),

...just because there's apparently no reason to have one of them enabled but not
the other or, at least, it looks like we *always* need ADC_26M_CK enabled when
ADC_FRC_CK is enabled.

Regards,
Angelo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] dt-bindings: iio: adc: mediatek,mt2701-auxadc: new 32k clock
  2022-10-20  8:28     ` AngeloGioacchino Del Regno
@ 2022-10-20 19:30       ` Daniel Golle
  -1 siblings, 0 replies; 10+ messages in thread
From: Daniel Golle @ 2022-10-20 19:30 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: Jonathan Cameron, Lars-Peter Clausen, Matthias Brugger,
	linux-iio, Gwendal Grignou, linux-arm-kernel, linux-mediatek,
	linux-kernel

On Thu, Oct 20, 2022 at 10:28:02AM +0200, AngeloGioacchino Del Regno wrote:
> Il 19/10/22 16:38, Daniel Golle ha scritto:
> > Newer MediaTek SoCs need an additional clock to be brought up for
> > AUXADC to work. Add this new optional clock to
> > mediatek,mt2701-auxadc.yaml.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >   .../bindings/iio/adc/mediatek,mt2701-auxadc.yaml          | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> > index 7f79a06e76f596..c2a1813dd54152 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> > @@ -44,10 +44,14 @@ properties:
> >       maxItems: 1
> >     clocks:
> > -    maxItems: 1
> > +    maxItems: 2
> > +    minItems: 1
> >     clock-names:
> > -    const: main
> > +    items:
> > +      - const: main
> > +      - const: 32k
> 
> You're adding this for MT7986, and I don't see any 32KHz ADC clock on that SoC.
> I suppose that your '32k' clock is CLK_INFRA_ADC_FRC_CK, currently parented to
> 'csw_f26m_sel', so that's 26MHz, not 32KHz.
> 
> Since you'll need the same changes for thermal as well, I would consider setting
> "infra_adc_frc" as a parent of "infra_adc_26m", like so:
> 
> 	GATE_INFRA1(CLK_INFRA_ADC_26M_CK, "infra_adc_26m", "infra_adc_frc", 20),
> 	GATE_INFRA1(CLK_INFRA_ADC_FRC_CK, "infra_adc_frc", "csw_f26m_sel", 21),
> 
> ...just because there's apparently no reason to have one of them enabled but not
> the other or, at least, it looks like we *always* need ADC_26M_CK enabled when
> ADC_FRC_CK is enabled.
> 

Yes, this change is for MT7986 and MT7981, immitating the behavior
found in MediaTek's SDK sources. Embedding the dependency into the
clock driver as you have suggested should also be possible as it is
true that you always need them both and a similar change for the
thermal driver would be needed as well.

Unless you were planning to do so already I will send a patch with your
suggested change to drivers/clk/mediatek/clk-mt7986-infracfg.c.
In any case, this series can be dropped then.

Thank you for the review!


Cheers


Daniel


> Regards,
> Angelo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] dt-bindings: iio: adc: mediatek,mt2701-auxadc: new 32k clock
@ 2022-10-20 19:30       ` Daniel Golle
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Golle @ 2022-10-20 19:30 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: Jonathan Cameron, Lars-Peter Clausen, Matthias Brugger,
	linux-iio, Gwendal Grignou, linux-arm-kernel, linux-mediatek,
	linux-kernel

On Thu, Oct 20, 2022 at 10:28:02AM +0200, AngeloGioacchino Del Regno wrote:
> Il 19/10/22 16:38, Daniel Golle ha scritto:
> > Newer MediaTek SoCs need an additional clock to be brought up for
> > AUXADC to work. Add this new optional clock to
> > mediatek,mt2701-auxadc.yaml.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >   .../bindings/iio/adc/mediatek,mt2701-auxadc.yaml          | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> > index 7f79a06e76f596..c2a1813dd54152 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
> > @@ -44,10 +44,14 @@ properties:
> >       maxItems: 1
> >     clocks:
> > -    maxItems: 1
> > +    maxItems: 2
> > +    minItems: 1
> >     clock-names:
> > -    const: main
> > +    items:
> > +      - const: main
> > +      - const: 32k
> 
> You're adding this for MT7986, and I don't see any 32KHz ADC clock on that SoC.
> I suppose that your '32k' clock is CLK_INFRA_ADC_FRC_CK, currently parented to
> 'csw_f26m_sel', so that's 26MHz, not 32KHz.
> 
> Since you'll need the same changes for thermal as well, I would consider setting
> "infra_adc_frc" as a parent of "infra_adc_26m", like so:
> 
> 	GATE_INFRA1(CLK_INFRA_ADC_26M_CK, "infra_adc_26m", "infra_adc_frc", 20),
> 	GATE_INFRA1(CLK_INFRA_ADC_FRC_CK, "infra_adc_frc", "csw_f26m_sel", 21),
> 
> ...just because there's apparently no reason to have one of them enabled but not
> the other or, at least, it looks like we *always* need ADC_26M_CK enabled when
> ADC_FRC_CK is enabled.
> 

Yes, this change is for MT7986 and MT7981, immitating the behavior
found in MediaTek's SDK sources. Embedding the dependency into the
clock driver as you have suggested should also be possible as it is
true that you always need them both and a similar change for the
thermal driver would be needed as well.

Unless you were planning to do so already I will send a patch with your
suggested change to drivers/clk/mediatek/clk-mt7986-infracfg.c.
In any case, this series can be dropped then.

Thank you for the review!


Cheers


Daniel


> Regards,
> Angelo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] dt-bindings: iio: adc: mediatek,mt2701-auxadc: new 32k clock
  2022-10-20 19:30       ` Daniel Golle
@ 2022-10-21  8:10         ` AngeloGioacchino Del Regno
  -1 siblings, 0 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-10-21  8:10 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Jonathan Cameron, Lars-Peter Clausen, Matthias Brugger,
	linux-iio, Gwendal Grignou, linux-arm-kernel, linux-mediatek,
	linux-kernel

Il 20/10/22 21:30, Daniel Golle ha scritto:
> On Thu, Oct 20, 2022 at 10:28:02AM +0200, AngeloGioacchino Del Regno wrote:
>> Il 19/10/22 16:38, Daniel Golle ha scritto:
>>> Newer MediaTek SoCs need an additional clock to be brought up for
>>> AUXADC to work. Add this new optional clock to
>>> mediatek,mt2701-auxadc.yaml.
>>>
>>> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
>>> ---
>>>    .../bindings/iio/adc/mediatek,mt2701-auxadc.yaml          | 8 ++++++--
>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
>>> index 7f79a06e76f596..c2a1813dd54152 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
>>> @@ -44,10 +44,14 @@ properties:
>>>        maxItems: 1
>>>      clocks:
>>> -    maxItems: 1
>>> +    maxItems: 2
>>> +    minItems: 1
>>>      clock-names:
>>> -    const: main
>>> +    items:
>>> +      - const: main
>>> +      - const: 32k
>>
>> You're adding this for MT7986, and I don't see any 32KHz ADC clock on that SoC.
>> I suppose that your '32k' clock is CLK_INFRA_ADC_FRC_CK, currently parented to
>> 'csw_f26m_sel', so that's 26MHz, not 32KHz.
>>
>> Since you'll need the same changes for thermal as well, I would consider setting
>> "infra_adc_frc" as a parent of "infra_adc_26m", like so:
>>
>> 	GATE_INFRA1(CLK_INFRA_ADC_26M_CK, "infra_adc_26m", "infra_adc_frc", 20),
>> 	GATE_INFRA1(CLK_INFRA_ADC_FRC_CK, "infra_adc_frc", "csw_f26m_sel", 21),
>>
>> ...just because there's apparently no reason to have one of them enabled but not
>> the other or, at least, it looks like we *always* need ADC_26M_CK enabled when
>> ADC_FRC_CK is enabled.
>>
> 
> Yes, this change is for MT7986 and MT7981, immitating the behavior
> found in MediaTek's SDK sources. Embedding the dependency into the
> clock driver as you have suggested should also be possible as it is
> true that you always need them both and a similar change for the
> thermal driver would be needed as well.
> 
> Unless you were planning to do so already I will send a patch with your
> suggested change to drivers/clk/mediatek/clk-mt7986-infracfg.c.
> In any case, this series can be dropped then.
> 
> Thank you for the review!
> 
> 

You're welcome. Nice job sending the clock commit.

For the maintainers:
This series can be abandoned, as that clock dependency was fixed elsewhere.

Regards,
Angelo



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] dt-bindings: iio: adc: mediatek,mt2701-auxadc: new 32k clock
@ 2022-10-21  8:10         ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-10-21  8:10 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Jonathan Cameron, Lars-Peter Clausen, Matthias Brugger,
	linux-iio, Gwendal Grignou, linux-arm-kernel, linux-mediatek,
	linux-kernel

Il 20/10/22 21:30, Daniel Golle ha scritto:
> On Thu, Oct 20, 2022 at 10:28:02AM +0200, AngeloGioacchino Del Regno wrote:
>> Il 19/10/22 16:38, Daniel Golle ha scritto:
>>> Newer MediaTek SoCs need an additional clock to be brought up for
>>> AUXADC to work. Add this new optional clock to
>>> mediatek,mt2701-auxadc.yaml.
>>>
>>> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
>>> ---
>>>    .../bindings/iio/adc/mediatek,mt2701-auxadc.yaml          | 8 ++++++--
>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
>>> index 7f79a06e76f596..c2a1813dd54152 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/adc/mediatek,mt2701-auxadc.yaml
>>> @@ -44,10 +44,14 @@ properties:
>>>        maxItems: 1
>>>      clocks:
>>> -    maxItems: 1
>>> +    maxItems: 2
>>> +    minItems: 1
>>>      clock-names:
>>> -    const: main
>>> +    items:
>>> +      - const: main
>>> +      - const: 32k
>>
>> You're adding this for MT7986, and I don't see any 32KHz ADC clock on that SoC.
>> I suppose that your '32k' clock is CLK_INFRA_ADC_FRC_CK, currently parented to
>> 'csw_f26m_sel', so that's 26MHz, not 32KHz.
>>
>> Since you'll need the same changes for thermal as well, I would consider setting
>> "infra_adc_frc" as a parent of "infra_adc_26m", like so:
>>
>> 	GATE_INFRA1(CLK_INFRA_ADC_26M_CK, "infra_adc_26m", "infra_adc_frc", 20),
>> 	GATE_INFRA1(CLK_INFRA_ADC_FRC_CK, "infra_adc_frc", "csw_f26m_sel", 21),
>>
>> ...just because there's apparently no reason to have one of them enabled but not
>> the other or, at least, it looks like we *always* need ADC_26M_CK enabled when
>> ADC_FRC_CK is enabled.
>>
> 
> Yes, this change is for MT7986 and MT7981, immitating the behavior
> found in MediaTek's SDK sources. Embedding the dependency into the
> clock driver as you have suggested should also be possible as it is
> true that you always need them both and a similar change for the
> thermal driver would be needed as well.
> 
> Unless you were planning to do so already I will send a patch with your
> suggested change to drivers/clk/mediatek/clk-mt7986-infracfg.c.
> In any case, this series can be dropped then.
> 
> Thank you for the review!
> 
> 

You're welcome. Nice job sending the clock commit.

For the maintainers:
This series can be abandoned, as that clock dependency was fixed elsewhere.

Regards,
Angelo



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-10-21  8:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-19 14:37 [PATCH 1/2] iio: adc: mt6577_auxadc: add optional 32k clock Daniel Golle
2022-10-19 14:37 ` Daniel Golle
2022-10-19 14:38 ` [PATCH 2/2] dt-bindings: iio: adc: mediatek,mt2701-auxadc: new " Daniel Golle
2022-10-19 14:38   ` Daniel Golle
2022-10-20  8:28   ` AngeloGioacchino Del Regno
2022-10-20  8:28     ` AngeloGioacchino Del Regno
2022-10-20 19:30     ` Daniel Golle
2022-10-20 19:30       ` Daniel Golle
2022-10-21  8:10       ` AngeloGioacchino Del Regno
2022-10-21  8:10         ` AngeloGioacchino Del Regno

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.