| Message ID | 20240209055203.17144-1-zajec5@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [V2] dt-bindings: thermal: mediatek,thermal: document AUXADC 32k clock | expand |
On 09/02/2024 06:52, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > SoCs MT7981 and MT7986 include a newer thermal block (V3) that requires > enabling one more clock called AUXADC 32k. Require it in binding. > > Cc: Daniel Golle <daniel@makrotopia.org> > Cc: Sam Shih <sam.shih@mediatek.com> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
Il 09/02/24 06:52, Rafał Miłecki ha scritto: > From: Rafał Miłecki <rafal@milecki.pl> > > SoCs MT7981 and MT7986 include a newer thermal block (V3) that requires > enabling one more clock called AUXADC 32k. Require it in binding. > > Cc: Daniel Golle <daniel@makrotopia.org> > Cc: Sam Shih <sam.shih@mediatek.com> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> So, I've made some research on this matter.. and this is a NACK. I'll keep it short: you don't really have any 32KHz clock input for the ADC in the infracfg clock controller (nor others). What you're trying to say here is that you have a FastRC Oscillator input clock (in your case, this is CLK_INFRA_ADC_FRC_CK): this clock is responsible for providing an input clock of either 26MHz or something that is MPLL/16 (I do not know what MPLL outputs on your platform, but those PLLs are usually giving out "big numbers"). The FRC osc in the AUXADC will perhaps output a 32KHz clock for internal reference and/or whatever else, but I'm not even sure that it's really going to be 32k then internally to the IP block. (I said "I'll keep it short" btw :-P) Anyway. The FRC clock is also required for the INFRA_ADC_26M clock to work correctly and, in the clock controller driver(s) of both 7981 and 7986, that is parented to.... ... surprise surprise ... INFRA_ADC_FRC_CK :-) Finally, you do *not* need this commit, INFRA does *not* provide an adc_32k clock to the thermal block, and you do *not* need to add the FRC clock in the devicetree of 7981, nor 7986. Mind you - at least in mt7986a.dtsi, there is an adc_32k clock in the thermal node, so the right way to get your thermal node validated is not to modify the binding, but to adhere to it by removing the extra, unneeded, clock. Cheers! Angelo
On 9.02.2024 10:13, AngeloGioacchino Del Regno wrote: > Il 09/02/24 06:52, Rafał Miłecki ha scritto: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> SoCs MT7981 and MT7986 include a newer thermal block (V3) that requires >> enabling one more clock called AUXADC 32k. Require it in binding. >> >> Cc: Daniel Golle <daniel@makrotopia.org> >> Cc: Sam Shih <sam.shih@mediatek.com> >> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > > So, I've made some research on this matter.. and this is a NACK. Well, I can only thank you for the research. Let's drop this patch. I'll sort out the rest later.
Il 12/02/24 07:11, Rafał Miłecki ha scritto: > On 9.02.2024 10:13, AngeloGioacchino Del Regno wrote: >> Il 09/02/24 06:52, Rafał Miłecki ha scritto: >>> From: Rafał Miłecki <rafal@milecki.pl> >>> >>> SoCs MT7981 and MT7986 include a newer thermal block (V3) that requires >>> enabling one more clock called AUXADC 32k. Require it in binding. >>> >>> Cc: Daniel Golle <daniel@makrotopia.org> >>> Cc: Sam Shih <sam.shih@mediatek.com> >>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >> >> So, I've made some research on this matter.. and this is a NACK. > > Well, I can only thank you for the research. > > Let's drop this patch. I'll sort out the rest later. Thank you for bringing that up - this was necessary to clarify this and to actually make both of us aware of the mistake in the device trees that we have for those SoCs. Take your time, btw, you're doing a great job. Cheers, Angelo
diff --git a/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml index d96a2e32bd8f..e7373d78618c 100644 --- a/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml +++ b/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml @@ -15,9 +15,6 @@ description: controls a mux in the apmixedsys register space via AHB bus accesses, so a phandle to the APMIXEDSYS is also needed. -allOf: - - $ref: thermal-sensor.yaml# - properties: compatible: enum: @@ -38,14 +35,18 @@ properties: maxItems: 1 clocks: + minItems: 2 items: - description: Main clock needed for register access - description: The AUXADC clock + - description: AUXADC 32k clock clock-names: + minItems: 2 items: - const: therm - const: auxadc + - const: adc_32k mediatek,auxadc: $ref: /schemas/types.yaml#/definitions/phandle @@ -76,6 +77,30 @@ required: - mediatek,auxadc - mediatek,apmixedsys +allOf: + - $ref: thermal-sensor.yaml# + - if: + properties: + compatible: + contains: + enum: + - mediatek,mt7981-thermal + - mediatek,mt7986-thermal + then: + properties: + clocks: + minItems: 3 + + clock-names: + minItems: 3 + else: + properties: + clocks: + maxItems: 2 + + clock-names: + maxItems: 2 + unevaluatedProperties: false examples: