Received-SPF: pass (google.com: domain of linux-kernel+bounces-204374-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 6 Jun 2024 13:32:46 +0000
From: Haylen Chu <heylenay@outlook.com>
To: Conor Dooley <conor@kernel.org>, Inochi Amaoto <inochiama@outlook.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Zhang Rui <rui.zhang@intel.com>, Lukasz Luba <lukasz.luba@arm.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Chen Wang <unicorn_wang@outlook.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Jisheng Zhang <jszhang@kernel.org>, linux-pm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v2 1/3] dt-bindings: thermal: sophgo,cv180x-thermal: Add
 Sophgo CV180x thermal
Message-ID:
 <SEYPR01MB4221F0E46F600E013974F21BD7FA2@SEYPR01MB4221.apcprd01.prod.exchangelabs.com>
References: <SG2PR01MB4218013241B3EED779D3BAE8D7F82@SG2PR01MB4218.apcprd01.prod.exchangelabs.com>
 <SG2PR01MB42189977B4172405F5704CC4D7F82@SG2PR01MB4218.apcprd01.prod.exchangelabs.com>
 <IA1PR20MB49531F55C8D7DC5D0050CAF9BBF92@IA1PR20MB4953.namprd20.prod.outlook.com>
 <20240605-tightwad-janitor-82cfceb1469d@spud>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240605-tightwad-janitor-82cfceb1469d@spud>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: SEYPR01MB4221:EE_|TY0PR0101MB4585:EE_
X-MS-Office365-Filtering-Correlation-Id: 897ad3e1-835f-4798-ba35-08dc862d2bac
X-Microsoft-Antispam: BCL:0;ARA:14566002|461199019|3412199016|440099019;
X-Microsoft-Antispam-Message-Info:
	PHQiJ6SYFtx8JefPfJdb8OKNuNemZM8hB6boojVMRwyocQMLeIH05/k+Gk/Ast4J3RTJCf8v6LgGfTLIumNQwJS7ishwilHnfNUqqj6MfZJmi/897269nlTum1t/NCefv5dyqzZZCGpGT5gMxiWg5mTrURgZ3Dh/FHJYEqKXAKNDTcEQ1IbukHW6STN2TWp4BraIDzeVtuNFpM3EAtlW7sc1Nksa53pSkZoGVgLObSztrDracxfPH4wndOmZ1eelcnXUBFMiCn+2r/v7f7+aqwJgMFZpw0TnUFMM+pbJJC8xQyqs2go3JTBKuze8i/B+MZZRPvdNedUJpn4gL0N1fC/Qxj6q9MmxohzZVBPNObC7y8RaapmJSzPLKQKx7FgtfGmqML99YzdGw0sOWxxgBB3guhNP2PwYhNMC9ridskKAGYqyhvZVc/35ASMIEAoF6qyn8RZSj49uux5yWwxZoyammkAdvmjKOTEpObgdYsTW2ZlIEiuYfONstAqcV/tnaIWcjzo5EYlFO47CzXkXmmKqw2ZBrjq3+q2NyAymgcTFyGlCv5h3rI435RIetJ4d
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?6Qoh+eC7T/Jhs6tmQyrPLx6WigUMcr+EPuprH49c20qlHlTtXwd3BmhTRP6D?=
 =?us-ascii?Q?2Twwxgr7RE0r4Kd8QxhGry7yqg28iGSf5nJQaHGXDHsdqhwUqj4uC38S7YeP?=
 =?us-ascii?Q?q3FTfOsW/lasmgJwBUegk5MsqvunmVJLMn6TgBY8Kqye2SyzXX1AqiiyKYaV?=
 =?us-ascii?Q?9x92JyrHr7oOkMhq0Wy+gfrdOViQ5n9bsNx/Zht+OYjHFr/uP1NY0QglEy9w?=
 =?us-ascii?Q?SN4Z7VpX7Oi4ezaStc+2CWX/89SHlUk5drcnqQz6UUCTz0J9j44Wnj7BBNFm?=
 =?us-ascii?Q?l2WpZITb3EoUHMA33HzHl13mnAWc0iutysIfYGO9bADOTXGW5fc3NzMIRMcw?=
 =?us-ascii?Q?Uv3p6ZtIwc9YQIzmQeR8XhsMQPp2ocrnbWoFSLxaS4E1FpCKGo/JCdfugKxG?=
 =?us-ascii?Q?noLg5k+U3uE+kuupCvrqy7IDg49s3vFgscoFUN0MUy1yH2O2DWIc+9g5qYq0?=
 =?us-ascii?Q?8yTb6Rm7M+BS6qDahxKfLeUnM61zC7FgFRqFpQUWBx6LBmCCUwTavRztvL0b?=
 =?us-ascii?Q?om3DpF5/RNJbr27aL8CBPwrFJLbx6zxDzE8a88C5ZRx95KqA3pWsJiQZLqr2?=
 =?us-ascii?Q?vX1QoXJgrJS2WCN1aELpX3gLiuTICIQA0FfyTzYxHYvJ0BIVHe/KvW/KIJPn?=
 =?us-ascii?Q?IF+ngIgWy5hxnn4vPg59B1Yx3K+e0o9aeZo4gDKoSMu5Dd8Tg/wGFD7onfNm?=
 =?us-ascii?Q?o30YwonS4UFLbvj+POc1JnyD6IsInz0CdZRFUSExjbPFsAD09XJJne7Fnw0V?=
 =?us-ascii?Q?FS011LTgUw2q5r5K7Xq+rRDTIdLgrIAQYkTUrK51pIwvppt9sISaldzcpHVs?=
 =?us-ascii?Q?0+1lw7DTUpqUYFT/OLPXpRYAEHQ+euCTdwDtYaZ46VQYoWc/DyOAz3bPuUQj?=
 =?us-ascii?Q?8GXO2OSmk/G6pex+JqstqRqP0/DrdhN3BoExzRv57kPSLLG/QK4C8ogL5cko?=
 =?us-ascii?Q?BmAGD3dkd3XV1N96r9WaHBzn2nybIjKmIvRQscyoKNC9P1aV3v0Evo+LhxOM?=
 =?us-ascii?Q?QwOcmRe4+zrOGvofEiyTyKRP8BgnNQ32Q6IMMm01nPHEsTpCdQ1CgMaHMb1D?=
 =?us-ascii?Q?7rcBftlyhhdV23CwtHkCN1qn7pRqx1H5Pc1CkKvMvuxAMrG05ieLkPGcy8Dx?=
 =?us-ascii?Q?GRZ3Tqzqh3MBo8JtWdGBBsyCNPlOtl4OJiGaZ9BVRR+VRBviFMlTM/wfKKeh?=
 =?us-ascii?Q?qLG9Ike5o8XPLWMMiAQ+eRrFb0EvjI5LYGYfAeUcHORmKv79hCKwzvX7uwU?=
 =?us-ascii?Q?=3D?=
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 897ad3e1-835f-4798-ba35-08dc862d2bac
X-MS-Exchange-CrossTenant-AuthSource: SEYPR01MB4221.apcprd01.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jun 2024 13:32:54.6993
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg:
	00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TY0PR0101MB4585

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