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

Hi Conor:

On Wed, Jul 15, 2026 at 12:54 AM Conor Dooley <[email protected]> wrote:
>
> 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.

According to the manual, the chip configuration registers are not
contiguous but are scattered throughout the memory. They are generally
as follows:

0x16000100 - 0x16000154:  General Chip Configuration / Chip Sampling
Parameters / Chip Counter Registers
0x16000400 - 0x16000424:  PLL Clock Configuration Registers -> This is
already used in clk driver;
0x16000490 - 0x160004a8:  GPIO Remapping Configuration Register ->
This is already used in pinctrl driver;
0x16000500 - 0x1600050c:  USB PHY Configuration Register
0x16001040 - 0x160014c0:  Interrupt routing Registers -> This is
already used in irqchip driver;
0x16001500 - 0x16001524:  Thermal Registers -> This is already used in
thermal driver;
0x16001600 - 0x1600180c:  External Interrupt Registers -> EIO irqchip driver;
0x16003fe0  - 0x16003ffc:  CHIP ID Registers

Expanding the range to `0x16000100 - 0x16003ffc` might not be the best approach.
In contrast, the chip ID register is independent. Additionally, apart
from the first section of general-purpose chip configuration
registers, the other registers are referenced by their respective
modules.

So, is `reg = <0x0 0x16003fe0 0x0 0x20>;` a good choice?
>
> >
> > 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

-- 
Thanks.
Binbin