Received-SPF: pass (google.com: domain of linux-kernel+bounces-178279-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Peng Fan <peng.fan@nxp.com>
To: Cristian Marussi <cristian.marussi@arm.com>, Guenter Roeck
	<linux@roeck-us.net>
CC: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>, "groeck7@gmail.com"
	<groeck7@gmail.com>, "sudeep.holla@arm.com" <sudeep.holla@arm.com>,
	"jdelvare@suse.com" <jdelvare@suse.com>, "linux-hwmon@vger.kernel.org"
	<linux-hwmon@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: RE: [PATCH V3] hwmon: scmi-hwmon: implement change_mode for thermal
 zones
Thread-Topic: [PATCH V3] hwmon: scmi-hwmon: implement change_mode for thermal
 zones
Thread-Index:
 AQHaT1lW79cmzSPKCUytPMAjATcCq7DqF+KAgAAAz2CAAH0qAIAAHRkAgAACIICAAAtxAICr84uw
Date: Tue, 14 May 2024 02:53:10 +0000
Message-ID:
 <DU0PR04MB94172D2602D32AF757A72CAF88E32@DU0PR04MB9417.eurprd04.prod.outlook.com>
References: <20240125064422.347002-1-peng.fan@oss.nxp.com>
 <b839f83f-c8c7-4fa8-8597-bdde1b40168a@roeck-us.net>
 <DU0PR04MB9417DAD2DBB8820344FEFB07887A2@DU0PR04MB9417.eurprd04.prod.outlook.com>
 <540cf4b3-ebf6-4a85-84c4-c012a69db416@roeck-us.net> <ZbKHpFRGoaQpWX16@pluto>
 <da42560b-32b2-49c6-9aeb-b7fbd5b5577c@roeck-us.net> <ZbKTBmt_BP6JRLBj@pluto>
In-Reply-To: <ZbKTBmt_BP6JRLBj@pluto>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?dr5PWF7yFB3+norYy849DZOhVSywvnw7E3Y7gyJfYA9qNd+NL4JUH168KFQj?=
 =?us-ascii?Q?qCJMXzs5L2tKPya53QymOPD8sf3bbmmhSt7zmOprgFZW3dTdFPiEnOO1b/pX?=
 =?us-ascii?Q?uRZlxQWHdS8QaGNXYDunonqGD3hnr8EHbHr3oYxQc3ViD5Tiw5VNacUEtx/T?=
 =?us-ascii?Q?swVBgzcKGoqWvfO2MpDa6r2YBBiZT6ESdbx8c83IhJmJ+6P8Pt4PK9d+TL66?=
 =?us-ascii?Q?Zh6KX4DBJywKFutbLdj0vG+ZC1kd7ZdEu95l4O8ZLW1LBIVFb3VZckuMSbWM?=
 =?us-ascii?Q?Onn2rjM/QK3DtYLJDn4xi/JBvXbeqPNYgvEf1hZlkCCRG0MKdUmrmmEx47vu?=
 =?us-ascii?Q?i+QtV036sbogPOUV2eoioJmeQc+i4CNLqvjw2+uMCk4xqI608idlEtaS0lkq?=
 =?us-ascii?Q?iPjMOsUKUWfmVF3hYvhtOd2JPgEDqTUoUJVh2a7GeSXKt4BcwFvz+8lrgtpu?=
 =?us-ascii?Q?J4V0ugIl2PPkckn08rkGlG1zP9XfsNSkYKEyj4DgH1u4LVeidmmV1LB2Uei3?=
 =?us-ascii?Q?9r1gSabwOyNeD6i0W0pDwbq+Vo0gLGCftydk5UVsqJjzNG4zqMbucZKdiyCl?=
 =?us-ascii?Q?Xn06JmA1Ril/wocO5Jn+Xha4azrWXvmh7FgwXuR1+7WuBLQsPzWQI1AT0qrf?=
 =?us-ascii?Q?4EEop0ExY1DKU5pD5tpn3FFEbJQ59XK0CgZPwewq5RFgTHbPhxNc0bHUfc/5?=
 =?us-ascii?Q?jJrKc7N32xUFwTVLeVbmPFJhdozJ+1msxcYVmw+/jR81Bn+5qhg3fkCwSjHK?=
 =?us-ascii?Q?WY1aIFVBwGl5Fr56JH2//DiAZ3ZcFocHjNpAWbExJX9vF6KF2PYRFiWpUWpF?=
 =?us-ascii?Q?HhEEKVRdwqO2gXW4kHYgAhMcamkUrkXToj1Cwh+Z6R1wQbfOclCOF7pMCYCU?=
 =?us-ascii?Q?YaKzXG0cZNqE0e0Zp2plRj+G/aOtVVQ+d2F9bExZm8uXtWCB55VsjlJjsTcg?=
 =?us-ascii?Q?IaZU6+nIDUOyZ0JwRU5sTrLdqmnGpLC1s/3ipVLx9ezHAsXmlWVfWOplD5Zq?=
 =?us-ascii?Q?goZijF8etfFzHtRXKAfovTj9SQQXVZc0QtZDdXqP0VZv5mU3R1ms4UoWWH5S?=
 =?us-ascii?Q?WUZzTwRnT0/wUa+ceAWs7BkPFkaaOTry36W4WWqjZITUx6TCCKsGAP4HAq0e?=
 =?us-ascii?Q?Uxk6s+gFIESD5DDZrfgKYAFvhc+VZF3h89WNqcF5PIVyk0dgUekP60PyEWOK?=
 =?us-ascii?Q?CZUWBVAGL4gFeJpuZjcOrrC8kJF5sk9BXoeI3+llxHTGudkxdVDm4tH4q+N7?=
 =?us-ascii?Q?fM/MZgIH3n6tbah+1GyzJKLRwrSeD4VavuanC+MNTyNZhxg7baYHIWaS85oh?=
 =?us-ascii?Q?ZHXzk52aYu/ND2QZPLpurUKyWbeYa41quFYXfILzK4/HocD+WRaoo2xldwiw?=
 =?us-ascii?Q?D2OtA2Ll/ssEyUvK5p91zFkT44G0DljJOUx626TuAw2z6K/Wkz4kCFnuIQ+a?=
 =?us-ascii?Q?LbCQr5ItXDZr1ESr9tMbqQyxCnsZ5cqFvONbcnVPfZormQRdZQQekx8FUx3E?=
 =?us-ascii?Q?1JuV4N/tdOtRpuK5KL+eKNljwZ2fPLeuRAIb82kvDC5man8BYIaBRemS3vpE?=
 =?us-ascii?Q?jZsxSUb29uUF1yerPJo=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DU0PR04MB9417.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7479d026-48c6-4db6-08eb-08dc73c0fd66
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 May 2024 02:53:10.1845
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: K2H8daHNsQyirVkACOre8I0XspZF67yAeVmCEYekue1nTXiNTXiEw50vypolpEwwacpNt6O3MnBIRP4nvNUWvg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR04MB8115

> Subject: Re: [PATCH V3] hwmon: scmi-hwmon: implement change_mode for
> thermal zones
>=20
> On Thu, Jan 25, 2024 at 08:16:45AM -0800, Guenter Roeck wrote:
> > On 1/25/24 08:09, Cristian Marussi wrote:
> >
> > > Agreed, but it seems that indeed here the attempt is to make sure
> > > that an accidentally disabled sensor is turned on.
> > >
> >
> > From the patch:
> >
> > +static int scmi_hwmon_thermal_change_mode(struct
> thermal_zone_device *tz,
> > +					  enum thermal_device_mode
> new_mode) {
> > ...
> > +	if (new_mode =3D=3D THERMAL_DEVICE_ENABLED)
> > +		config |=3D SCMI_SENS_CFG_SENSOR_ENABLED_MASK;
> > +	else
> > +		config &=3D ~SCMI_SENS_CFG_SENSOR_ENABLED_MASK;
> >
> > This clearly contradicts your statement. It leaves the sensor in full
> > control by the thermal subsystem. If the thermal subsystem decides to
> > turn it off, it is turned off.
>=20
> Yes, indeed, and this is wrong as you explained; what I meant is that it =
seems
> to me now after this discussion, and from the commit message, that the
> reason (and the aim of this patch) is different from how it achieves it (=
as a
> side effect)
>=20
> "The thermal sensors maybe disabled before kernel boot, so add
> change_mode  for thermal zones to support configuring the thermal sensor =
to
> enabled  state. If reading the temperature when the sensor is disabled, t=
here
> will  be error reported."
>=20
> So when I said:
>=20
> > > Agreed, but it seems that indeed here the attempt is to make sure
> > > that an accidentally disabled sensor is turned on.
>=20
> and
>=20
> >> In this case seems like the sensor is NOT supposed to be ever
> >>disabled  (not even temporarily), it it just that you want to ensure
> >>that is enabled, so I would say @Peng, should not that be done in the
> >>fw  SCMI server ? (i.e. to turn on, early on, all those resources that
> >>are
> >>  exposed to the agent and meant to be always on)
>=20
> I implied to drop this patch and instead make sure the visible-and-always=
-
> enabled sensor is default-enabled by the SCMI server running in the firmw=
are,
> given that there won't be any need to ever disable it, from the hwmon
> interface NOR from the thermal subsystem.
>=20
> Sorry if I have not been clear (but I maybe still well-wrong anyway :D)

Sorry to bring back this old topic.

The tempsensor is disabled at boot, I will check with FW owner to enable it
by default. But the tempsensor will consume some power, if leaving
it always enabled.

Do we need to export a HWMON_T_ENABLE to temperature sensor if
leaving thermal framework only reading temp?

Thanks,
Peng.
>=20
> Thanks,
> Cristian
>=20