Re: [PATCH v9,2/7] dt-bindings: thermal: Add dt-binding document for LVTS thermal controllers

Balsam CHIHI
2022-09-14T14:03:22Z

Hi Angelo,

I've got the following errors after implementing these changes :
[...]
nvmem-cells:
minItems: 1
description: Calibration eFuse data for LVTS

nvmem-cell-names:
minItems: 1
items:
pattern: 'lvts-calib-data[0-9]+$'

"#thermal-sensor-cells":
const: 1

allOf:
- $ref: thermal-sensor.yaml#

- if:
properties:
compatible:
contains:
enum:
- mediatek,mt8192-lvts-ap
- mediatek,mt8192-lvts-mcu
then:
properties:
nvmem-cells:
maxItems: 1

nvmem-cell-names:
maxItems: 1

- if:
properties:
compatible:
contains:
enum:
- mediatek,mt8195-lvts-ap
- mediatek,mt8195-lvts-mcu
then:
properties:
nvmem-cells:
maxItems: 2

nvmem-cell-names:
maxItems: 2
[...]

$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
LINT Documentation/devicetree/bindings
CHKDT Documentation/devicetree/bindings/processed-schema.json
/home/balsam/src/linux-mtk-lvts-newThermalOF/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml:
properties:nvmem-cell-names:items: {'pattern':
'lvts-calib-data[0-9]+$'} is not of type 'array'
from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
SCHEMA Documentation/devicetree/bindings/processed-schema.json
/home/balsam/src/linux-mtk-lvts-newThermalOF/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml:
ignoring, error in schema: properties: nvmem-cell-names: items
DTEX Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.example.dts
DTC Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.example.dtb
CHECK Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.example.dtb
Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.example.dtb:0:0:
/example-0/soc/thermal-sensor@1100b000: failed to match any schema
with compatible: ['mediatek,mt8192-lvts-ap']

am I missing something?

Best regards,
Balsam

On Wed, Sep 14, 2022 at 2:19 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 17/08/22 10:07, bchihi@baylibre.com ha scritto:
> > From: Alexandre Bailon <abailon@baylibre.com>
> >
> > Add dt-binding document for mt8192 and mt8195 LVTS thermal controllers.
> >
> > Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> > Co-developed-by: Balsam CHIHI <bchihi@baylibre.com>
> > Signed-off-by: Balsam CHIHI <bchihi@baylibre.com>
> > ---
> > .../thermal/mediatek,lvts-thermal.yaml | 152 ++++++++++++++++++
> > 1 file changed, 152 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> > new file mode 100644
> > index 000000000000..31d9e220513a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> > @@ -0,0 +1,152 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/thermal/mediatek,lvts-thermal.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek SoC LVTS thermal controller
>
> title: MediaTek SoC Low Voltage Thermal Sensor (LVTS)
>
> > +
> > +maintainers:
> > + - Yu-Chia Chang <ethan.chang@mediatek.com>
> > + - Ben Tseng <ben.tseng@mediatek.com>
> > +
> > +description: |
>
> description:
> LVTS is a thermal management architecture composed of three subsystems,
> a Sensing device - Thermal Sensing Micro Circuit Unit (TSMCU),
> a Convertor - Low Voltage Thermal Sensor convertor (LVTS), and
> a Digital controller (LVTS_CTRL).
>
> > + LVTS (Low Voltage Thermal Sensor).
> > + The architecture will be first used on mt8192 and mt8195.
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - mediatek,mt8192-lvts-ap
> > + - mediatek,mt8192-lvts-mcu
> > + - mediatek,mt8195-lvts-ap
> > + - mediatek,mt8195-lvts-mcu
> > +
> > + "#thermal-sensor-cells":
> > + const: 1
> > +
> > + reg:
> > + maxItems: 1
> > + description: LVTS instance registers.
>
> This description looks obvious, as it doesn't really say anything "new"...
> I would rather drop it.
>
> > +
> > + interrupts:
> > + maxItems: 1
> > + description: LVTS instance interrupts.
>
> Same here
>
> > +
> > + clocks:
> > + maxItems: 1
> > + description: LVTS instance clock.
>
> and here.
>
> > +
> > + resets:
> > + maxItems: 1
> > + description: |
> > + LVTS instance SW reset for HW AP/MCU domain to clean temporary data
> > + on HW initialization/resume.
>
> What about something like...
>
> resets:
> items:
> - description: LVTS reset for clearing temporary data on AP/MCU
>
> > +
> > + nvmem-cells:
> > + minItems: 1
> > + maxItems: 2
> > + description: Calibration efuse data for LVTS
>
> nvmem-cells:
> minItems: 1
> items:
> - description: Calibration eFuse data for LVTS
> - description: Additional eFuse data (?)
>
>
> > +
> > + nvmem-cell-names:
> > + minItems: 1
> > + maxItems: 2
> > + description: Calibration efuse cell names for LVTS
>
> Actually, maxItems is not really two, but it depends on how many
> eFuse arrays / nvmem cells we have for each SoC, so I was thinking...
>
> ...what about doing something like
>
> nvmem-cell-names:
> minItems: 1
> items:
> pattern: 'lvts-calib-data[0-9]+$'
>
> and then,
> if:
> properties:
> compatible:
> contains:
> enum:
> - mediatek,blahblah-something
> then:
> properties:
> nvmem-cell-names:
> maxItems: 2 (or 3, 4, 5...)
>
> P.S.: I haven't tried any binding check on the proposed lines.
>
> Krzysztof, any opinions on that?
>
> Regards,
> Angelo
>
>
>