Re: [PATCH v2 1/3] dt-bindings: thermal: sophgo,cv180x-thermal: Add Sophgo CV180x thermal

[Haylen Chu]

On Wed, Jun 05, 2024 at 06:54:17PM +0100, Conor Dooley wrote:
> > > +  accumulation-period:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: Accumulation period for a sample
> > > +    oneOf:
> > > +      - const: 0
> > > +        description: 512 ticks
> > > +      - const: 1
> > > +        description: 1024 ticks
> > > +      - const: 2
> > > +        description: 2048 ticks
> > > +      - const: 3
> > > +        description: 4096 ticks
> > > +    default: 2
> > > +
> > > +  chop-period:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: ADC chop period
> 
> What's a "chop" and why is either this or the accumulation-period a
> fixed property of the hardware? Shouldn't this choice really be up to
> the user?

The chop-period is an ADC parameter.

Both accumulation-period and chop-period specify how the sensor
measures temperature. Making these parameters up to end users brings
extra unnecessary code complexity. Being configurable for each board
should be enough and other thermal drivers have been doing things in
this way.

> 
> > > +    oneOf:
> > > +      - const: 0
> > > +        description: 128 ticks
> > > +      - const: 1
> > > +        description: 256 ticks
> > > +      - const: 2
> > > +        description: 512 ticks
> > > +      - const: 3
> > > +        description: 1024 ticks
> 
> Can we just make the number of ticks the unit here, and above?
> Also, a "oneOf: - const" structure is just an enum.

I do not catch your idea. These values directly map to raw register
configuration, which simplify the implementation a lot.

> 
> > > +    default: 3
> > > +
> > > +  sample-cycle-us:
> > > +    description: Period between samples
> > > +    default: 1000000
> No constraints?

Sample cycle is more flexible because of hardware designing.

Best reguards,
Haylen Chu