Re: [PATCH 2/2] dt-bindings: iio: adc: mediatek,mt2701-auxadc: new 32k clock

[AngeloGioacchino Del Regno]

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@xxxxxxxxxxxxxx>
> > > ---
> > >    .../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