Re: [PATCH 1/2] dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300

On Tue, Jul 14, 2026 at 02:53:40PM +0800, Binbin Zhou wrote:
> Hi Conor:
> 
> Thanks for your reply.
> 
> On Sat, Jul 11, 2026 at 12:21 AM Conor Dooley <[email protected]> wrote:
> >
> > On Fri, Jul 10, 2026 at 04:24:59PM +0800, Binbin Zhou wrote:
> > > Add a new compatible string `loongson,ls2k0300-thermal` for the thermal
> > > sensor found on the Loongson-2K0300 SoC.
> > >
> > > The hardware differs from the existing SoCs in its register layout: it
> > > requires two register regions (one for the thermal sensor control and
> > > another for the CPU ID).
> > >
> > > Update the binding to describe this new requirement.
> > >
> > > Signed-off-by: Binbin Zhou <[email protected]>
> > > ---
> > >  .../thermal/loongson,ls2k-thermal.yaml        | 68 ++++++++++++++-----
> > >  1 file changed, 50 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > index 79e691b08341..b5cbfd201105 100644
> > > --- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > +++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > @@ -10,13 +10,11 @@ maintainers:
> > >    - zhanghongchen <[email protected]>
> > >    - Yinbo Zhu <[email protected]>
> > >
> > > -allOf:
> > > -  - $ref: /schemas/thermal/thermal-sensor.yaml#
> > > -
> > >  properties:
> > >    compatible:
> > >      oneOf:
> > >        - enum:
> > > +          - loongson,ls2k0300-thermal
> > >            - loongson,ls2k1000-thermal
> > >            - loongson,ls2k2000-thermal
> > >        - items:
> > > @@ -39,23 +37,46 @@ required:
> > >    - reg
> > >    - interrupts
> > >
> > > -if:
> > > -  properties:
> > > -    compatible:
> > > -      contains:
> > > -        enum:
> > > -          - loongson,ls2k2000-thermal
> > > +allOf:
> > > +  - $ref: /schemas/thermal/thermal-sensor.yaml#
> > >
> > > -then:
> > > -  properties:
> > > -    reg:
> > > -      minItems: 2
> > > -      maxItems: 2
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - loongson,ls2k0300-thermal
> > > +    then:
> > > +      properties:
> > > +        reg:
> > > +          items:
> > > +            - description: Thermal base register region
> > > +            - description: CPU ID register region
> > >
> > > -else:
> > > -  properties:
> > > -    reg:
> > > -      maxItems: 1
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - loongson,ls2k1000-thermal
> > > +    then:
> > > +      properties:
> > > +        reg:
> > > +          items:
> > > +            - description: Thermal base register region
> > > +
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - loongson,ls2k2000-thermal
> > > +    then:
> > > +      properties:
> > > +        reg:
> > > +          items:
> > > +            - description: Thermal base register region
> > > +            - description: Thermal data output register region
> > >
> > >  unevaluatedProperties: false
> > >
> > > @@ -69,3 +90,14 @@ examples:
> > >          interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> > >          #thermal-sensor-cells = <1>;
> > >      };
> > > +
> > > +  - |
> > > +    #include <dt-bindings/interrupt-controller/irq.h>
> > > +    thermal-sensor@16001500 {
> > > +       compatible = "loongson,ls2k0300-thermal";
> > > +       reg = <0x16001500 0x30>,
> > > +             <0x16003ff0 0x8>;
> >
> > Quite frankly, the address and size of this look like the second
> > register region here is actually a few bytes in a syscon that is being
> > misrepresented.
> > What lies at the addresses immediately before and after 0x16003ff0?
> 
> Yes, it can be viewed as part of the system configuration registers,
> which appear somewhat disorganized. Within this section,
> 0x16003fe0–0x16003ffc represents eight chip ID registers.
> 
> 0x16003fe0 --> the 4th chip id
> 0x16003fe4 --> the 5th chip id
> 0x16003fe8 --> the 6th chip id
> 0x16003fec --> the 7th chip id
> 0x16003ff0 --> the 0th chip id
> 0x16003ff4 --> the 1st chip id
> 0x16003ff8 --> the 2nd chip id
> 0x16003ffc --> the 3rd chip id
> 
> Perhaps I shouldn’t have referenced `0x16003ff0` separately here. It
> would be more reasonable to declare the entire chip ID address space
> as a separate syscon and have it referenced by thermal driver.

Probably, but even being of size 0x20 feels suspiciously small and that
it is likely that this is part of an even larger grouping of misc.
registers.

> 
> For example:
> 
> In DTS{i}:
> chipid_syscon: syscon@16003fe0 {
>        compatible = “loongson,ls2k0300-cphipid-syscon”, “syscon”;
>        reg = <0x0 0x16003fe0 0x0 0x20>;
> };
> 
> In thermal driver:
> Use
> `syscon_regmap_lookup_by_phandle(np, "loongson,chipid"); `
> Or:
> `syscon_regmap_lookup_by_compatible("loongson,ls2k0300-cphipid-syscon");`
> 
> get the chip id address space.
> 
> >
> >
> > Thanks,
> > Conor.
> >
> > > +       interrupt-parent = <&liointc1>;
> > > +       interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
> > > +       #thermal-sensor-cells = <1>;
> > > +    };
> > > --
> > > 2.52.0
> > >
> 
> -- 
> Thanks.
> Binbin

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCalZpuQAKCRB4tDGHoIJi
0hrJAP9oL8N9deSXz9dWt+ZEZYfbS19yv8wqpWxkFNHzB6DWwQEAtSc8DI7IDfPv
ibyhUOue/ShcNI/iaFNCSty4HW1vkw0=
=x7Y7
-----END PGP SIGNATURE-----