Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Date:   Thu, 30 Nov 2023 20:47:06 -0800
From:   Ashok Raj <ashok.raj@intel.com>
To:     Guenter Roeck <linux@roeck-us.net>
CC:     Ashok Raj <ashok_raj@linux.intel.com>,
        Zhang Rui <rui.zhang@intel.com>, <jdelvare@suse.com>,
        <fenghua.yu@intel.com>, <linux-hwmon@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, Ashok Raj <ashok.raj@intel.com>
Subject: Re: [PATCH 1/3] hwmon: (coretemp) Introduce enum for attr index
Message-ID: <ZWllSmdI5pVGc0+3@a4bf019067fa.jf.intel.com>
References: <20231127131651.476795-1-rui.zhang@intel.com>
 <20231127131651.476795-2-rui.zhang@intel.com>
 <ZWkDQ5y3e4oPONni@araj-dh-work.jf.intel.com>
 <0646bcf5-fd41-41e5-b7b1-06b425973c86@roeck-us.net>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <0646bcf5-fd41-41e5-b7b1-06b425973c86@roeck-us.net>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?m26JFOF/jWwsRNBSpeze8ySCb/NK1EHUUSi4xcpGlh0eDHF6S182lUyF8Q2a?=
 =?us-ascii?Q?od5ZW8l200t3xoZZ1HJKJhYQSWXMN8NekEAmRurqGTn1q9tnqj1fLpjKHwOA?=
 =?us-ascii?Q?5Uhr7ZrHckehpcDy5iV7DKQLptPdtqMyWaa724ejiGFz1El+6dEHnXNZ8Ywo?=
 =?us-ascii?Q?cPpMYvHtoRTbsSJ583MgFEqJqU0nEbaJB5RKNXFPKhO0V21jYZDr0oPMeqjl?=
 =?us-ascii?Q?ULqnLBr4BpVwEr1upyoiRqyxafd2JCopWcUYIk57cL3bqx8r3XEy/xB79qCn?=
 =?us-ascii?Q?FJ4kWMdsf8GZSV2UHMhETNd6KZxfyNQHOiHgjTyq4HOc52IhnABsdNGFaKKs?=
 =?us-ascii?Q?4tPbyBd1i+k6FLJLViTwb5aXeeTo7iNoi3llmDzGMhwqZSKVH1QzX1p0MuIF?=
 =?us-ascii?Q?DMu9lRoB7nhhtGALnkkhvxqUQ9rf5LCbMZYDlyMy++lQvaWbO4MK9e5tXZZ2?=
 =?us-ascii?Q?RvKsbC6J8Nro7fx32k1CxN8RPiCpNMtf5ozG18NpFu5AmpeAzifokoOuzNqe?=
 =?us-ascii?Q?xybv+9h2Y6LqfYh6whsyqhpCOI1W9WEEDiN9cU4M8LfNDdeC/iG0I06ttBQk?=
 =?us-ascii?Q?MEeS00H5xnBzjrnvj2s2xPTAP9xKFPe1zH7SrzKODEUlR0XWgCR3HObEqeSj?=
 =?us-ascii?Q?EWdcJFovVwp+eHX42HRZsJO0gPMbdul0hQqcEKTnsZ0+WIdphrQbynC/dIOB?=
 =?us-ascii?Q?TtzNgVdwip0XKLrBD6Bjfq6hH9gKKh5GhM0poF5HsFMLpS6M7oiCpT42MfS6?=
 =?us-ascii?Q?wXGn2rNuUg8PqTaA+SrwReQKrZDaQdFzgtlnr6gQuPtr4bCId/2A7U7J85Ex?=
 =?us-ascii?Q?T6lh7cyGEipznRDNMhi8MHEPymfO+IuUPrzKbpp/RQJFiR+45HzIXoQpZoPh?=
 =?us-ascii?Q?/+8UEWdBwgGmRGDepDjxrjQ4LQDJrp4APk3+JTN6seTFGvlqRgBdePGlOEMP?=
 =?us-ascii?Q?4P2KT/iTb6HSQ4shT5CqGgdbodXnX3OM+fA4O4mU8mTuAE9auE8BhL6KEhZb?=
 =?us-ascii?Q?lfwrhndpFkKLpCxBx0zaqZCGFuhvJG3bHaOIu5e+UsZ32xbuuXibjgWOIteA?=
 =?us-ascii?Q?edXfSLQnE+wsXmXsVr3zQsx1l/aQbIL9oG2VFW5L0hXrEYia8kfxKny2B+EL?=
 =?us-ascii?Q?oCv1q8nAFbRMa5/F5KBfiFEOvNzv0rl/IDPdvuJZ/dmX4cnD7TOIUebmFF7c?=
 =?us-ascii?Q?s1PDYPHz6GU3eUj2+ly7Q/x+wPQFwMpfLtp/KbQn4EGbfTvDvx4hbmz1GBFm?=
 =?us-ascii?Q?4a9/Q0n1clBxn/UZ8jw/tiRrvrLKIUOllaouFofbepFGLzEZvy5y/cdoxj9m?=
 =?us-ascii?Q?lHWMUKIFnZJ7WJl7clu9EgTA2BLA2MXdIkntdLb1avRCiZR25p3WXm8LW7CW?=
 =?us-ascii?Q?xbZZe09gqjE+SWtY7riZkPitpiwC4U5RIdRa4OcEHeJN7eRzEnHZAxsTT6Q7?=
 =?us-ascii?Q?JsciRAWp7cL7OBINNi620i+A7nn59sH8eDYGSguu+p2QPcD4GqrPITJG5n9W?=
 =?us-ascii?Q?TEXmmqeu4sZOyKIFuZiTQXTZ49HsHVLskcqXyLSb+/hXSq6xSb3+Wgq4RUda?=
 =?us-ascii?Q?HEYtvCxKMaOwGgCzq6ADEBnHXzDD69fa+JI1NZqO65oLeqzmMjDN2h5N236K?=
 =?us-ascii?Q?Fw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 244c6694-f6ce-428a-32f4-08dbf2289537
X-MS-Exchange-CrossTenant-AuthSource: SJ1PR11MB6201.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Dec 2023 04:47:12.0136
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: qD8J6WHz1Fsi4sOpu+CfbWztVQIYncCg03b9a56hpG/P6Mte7wSuyB/4nvfOlcOO46UdXpkvnr4H84+0jJMtxA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4711
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Thu, 30 Nov 2023 20:47:34 -0800 (PST)

On Thu, Nov 30, 2023 at 08:14:48PM -0800, Guenter Roeck wrote:
> On 11/30/23 13:51, Ashok Raj wrote:
> > On Mon, Nov 27, 2023 at 09:16:49PM +0800, Zhang Rui wrote:
> > > Introduce enum coretemp_attr_index to better describe the index of each
> > > sensor attribute and the maximum number of basic/possible attributes.
> > > 
> > > No functional change.
> > > 
> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > ---
> > >   drivers/hwmon/coretemp.c | 12 ++++++++++--
> > >   1 file changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> > > index ba82d1e79c13..6053ed3761c2 100644
> > > --- a/drivers/hwmon/coretemp.c
> > > +++ b/drivers/hwmon/coretemp.c
> > > @@ -43,10 +43,18 @@ MODULE_PARM_DESC(tjmax, "TjMax value in degrees Celsius");
> > >   #define BASE_SYSFS_ATTR_NO	2	/* Sysfs Base attr no for coretemp */
> > >   #define NUM_REAL_CORES		128	/* Number of Real cores per cpu */
> > >   #define CORETEMP_NAME_LENGTH	28	/* String Length of attrs */
> > > -#define MAX_CORE_ATTRS		4	/* Maximum no of basic attrs */
> > > -#define TOTAL_ATTRS		(MAX_CORE_ATTRS + 1)
> > >   #define MAX_CORE_DATA		(NUM_REAL_CORES + BASE_SYSFS_ATTR_NO)
> > > +enum coretemp_attr_index {
> > > +	ATTR_LABEL,
> > > +	ATTR_CRIT_ALARM,
> > > +	ATTR_TEMP,
> > > +	ATTR_TJMAX,
> > > +	ATTR_TTARGET,
> > > +	TOTAL_ATTRS,			/* Maximum no of possible attrs */
> > > +	MAX_CORE_ATTRS = ATTR_TJMAX + 1	/* Maximum no of basic attrs */
> > 
> > This seems odd. TOTAL_ATTRS being the last entry seems fine, but defining a
> > MAX_CORE_ATTR the way above sounds a bit hacky.
> > 
> 
> Complaining is easy. What do you suggest that would be better ?
> 
Fair enough.

How about 

ATTR_LABEL,
ATTR_CRIT_ALARM,
ATTR_TEMP,
ATTR_TJMAX,
MAX_CORE_ATTRS,		/* One more than TJMAX */
ATTR_TTARGET = MAX_CORE_ATTRS,
TOTAL_ATTRS

Each enum can be assigned any value, but this way they are just increasing
order.