Received-SPF: pass (google.com: domain of linux-kernel+bounces-196468-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: "Sabau, Radu bogdan" <Radu.Sabau@analog.com>
To: "Sabau, Radu bogdan" <Radu.Sabau@analog.com>,
        Jean Delvare
	<jdelvare@suse.com>, Guenter Roeck <linux@roeck-us.net>,
        Jonathan Corbet
	<corbet@lwn.net>,
        "linux-hwmon@vger.kernel.org"
	<linux-hwmon@vger.kernel.org>,
        "linux-doc@vger.kernel.org"
	<linux-doc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v5] drivers: hwmon: max31827: Add PEC support
Thread-Topic: [PATCH v5] drivers: hwmon: max31827: Add PEC support
Thread-Index: AQHasoQWyglzjeI7aUWz4Ib9poW0nLGxAsVg
Date: Fri, 31 May 2024 08:26:56 +0000
Message-ID: 
 <SA1PR03MB6499B0B15E16917912A24429F7FC2@SA1PR03MB6499.namprd03.prod.outlook.com>
References: <20240530112505.14831-1-radu.sabau@analog.com>
In-Reply-To: <20240530112505.14831-1-radu.sabau@analog.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?mh6qjXWvmAlPiANJA1Le/laMBQ0Jg/CY3Mnt2IMUSJ+2mZxYbnN9mX1zDj8C?=
 =?us-ascii?Q?I5MQo8GyzHvvSbJG1r9JtNTGd5HcMVQUMwzzjQ4pt5HsSnzLpNEOxwC3dTkH?=
 =?us-ascii?Q?7ZDZBVotS9zbtlYcr2mODtsvSZgvTWg523EXr4yByy8nhX8exCewInpKGOmm?=
 =?us-ascii?Q?5l/0tPLbUq7xGTTNPk8a89jR/56XwXb+ZUrXe6YZXvKW5hDnmTyk/dhEZqiI?=
 =?us-ascii?Q?QIchSFYCu4NdLhIvitJOkYC155wN1nR99WS3UxzE40tXX2c5A8lJeOZkbp1+?=
 =?us-ascii?Q?5Y2bHtHWXlLwUpxtUY7o71eTTGXk4IrzTIFaFp+hbhLkTfhoSJyXh9iJtHNj?=
 =?us-ascii?Q?JLY4Kjhvt+tcaPJfXrt6gWYNOu3HpuKmqwztL2mHHIp+lXb4FtOiP/UQrhzQ?=
 =?us-ascii?Q?HAbb/2grOyp+h8yQmszIjUkBygvNadul0lpMPuRit+iwUZjC6HqIPQcfDKv2?=
 =?us-ascii?Q?Uu68VNiBTPLaRLAvUu25WdKvdjv9098p5Z8KULBDFcXEeW/H0DF9wziRINer?=
 =?us-ascii?Q?xCSXPBoXUeS7fGaqMIeoXC2tzBetTmWZ0YgY6BGvVKaCtG7hHyF+PBbdJAH9?=
 =?us-ascii?Q?TRKLu0WWebEhsjcJ2J1Dwbc0FfXJHfAqoznxFtws2YGIzP9UyieqFjP7lUBB?=
 =?us-ascii?Q?rYwFUxQ/Qr1b92XSPT6n5LDqN7+qOBc3/dUxKvlUNScrkMl1OGN/nzn9mgWe?=
 =?us-ascii?Q?CVFgbSyd1UQSSNXxOBt94CnK00DjEbt4YmhlPLkUm1oBBMpzGdcvAMB+Rhmx?=
 =?us-ascii?Q?ARiOqJg1uEWZpOLMTaXbd7n+90PF6Ol2Yg4v2MOm/fHOIrIeGxZ5Qe1uFTbk?=
 =?us-ascii?Q?DWazkp+7qv8eU0vDar0H2VTjppp+I3L5hAP3k81OkkdFoGmgQxC30cdnnJjS?=
 =?us-ascii?Q?T4VdyJNJ7XlDC/9kg55AbJt/sivKDnByAEHOjHjOFO0Z4pS25oVgbJ4RBq5f?=
 =?us-ascii?Q?Yy2vqnV2BMpnr+7E1rQqfSWTRJpod8pWhh8GEH2+wO1/rkXQj3b+mRPMDUGE?=
 =?us-ascii?Q?/8JKX7hFr7aoZKjx3rErWBho247CQ8CpJBNq28yMZ+0xji+Hu1FPhILhAmpL?=
 =?us-ascii?Q?M9dhuPoWdKkX4UUSl1R06RsjTuTLriJK2y0t/dWJJQKEKYGhzNy2ANsIvbbm?=
 =?us-ascii?Q?CN7ejDmanHgfOmNRUGhM7wNhEuMdbw5nsfwZ4hSS9Ui9DpB/wJyRWdRBFELW?=
 =?us-ascii?Q?VDuorrgDpNXUooFXhXQuz3N9ZMLYhxWrN8kZ1rWJKH6yvYrLeZs8ngJPlSOn?=
 =?us-ascii?Q?oc6V4AErpogpnM9aDqf+88t97GRsfivxZRyq+Oqx8PsQBXM+x82HdGmalbwW?=
 =?us-ascii?Q?Mni/4QEs7oZVhGvy0uGRWO0wZZk3LheIWB3nohWDQzfl2eKPJ1Q6rEQjaud6?=
 =?us-ascii?Q?3b6rzhOalzOCfgBWK7hpNURDJaUaoirW540JMokCE+A2/sis9ilHnAGSZrBJ?=
 =?us-ascii?Q?bZYKwBUMcedC7u78wZxCZBGqnMvJYEthnyET0VfwkJgAc6sdL+pxZupdXOEl?=
 =?us-ascii?Q?ikxsB0nWSf/mvp3LXGXf67JCt+LHwaLeCXrx+cfPOT7tH+iRx11fWt4ugr/4?=
 =?us-ascii?Q?WqxyWv0FzoDxmF/pGiVQRqTGKj0IT7inQviNfiS/?=
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: analog.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SA1PR03MB6499.namprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5eb0d7f9-f638-429b-4b8d-08dc814b6f3c
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 May 2024 08:26:56.8367
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: eaa689b4-8f87-40e0-9c6f-7228de4d754a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: CA/YUtr6DyZ1ZbHYApSPCDb5jDneXtTLbcy013cpNhiOf2X6Suo2gkRlBkWiD8ckeVnsF3qbU00CUe7ETgl8+w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR03MB7450
X-Proofpoint-GUID: fLbkgVrbxEOIAPL1pc_jd8yzxJkGbEPW
X-Proofpoint-ORIG-GUID: fLbkgVrbxEOIAPL1pc_jd8yzxJkGbEPW
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.12.28.16
 definitions=2024-05-31_04,2024-05-30_01,2024-05-17_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 clxscore=1015
 bulkscore=0 spamscore=0 malwarescore=0 suspectscore=0 impostorscore=0
 adultscore=0 mlxlogscore=999 priorityscore=1501 mlxscore=0
 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.19.0-2405170001 definitions=main-2405310062


> -----Original Message-----
> From: Radu Sabau <radu.sabau@analog.com>
> Sent: Thursday, May 30, 2024 2:25 PM
> To: Jean Delvare <jdelvare@suse.com>; Guenter Roeck <linux@roeck-us.net>;=
 Jonathan Corbet <corbet@lwn.net>; linux-
> hwmon@vger.kernel.org; linux-doc@vger.kernel.org; linux-kernel@vger.kerne=
l.org
> Cc: Sabau, Radu bogdan <Radu.Sabau@analog.com>
> Subject: [PATCH v5] drivers: hwmon: max31827: Add PEC support
>=20
> Add support for PEC by configuring chip accordingly to the hwmon core
> PEC attribute handling.
> Add chip_write function to be used by max31827_write when setting
> the attribute.
>=20
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> Change log:
> v2:
>  *Rebase on top of v6.9
>  *Attach pec attribute only to i2c device
>  *Fix bug to attach pec attribute to i2c device if the device supports it=
.
> v3:
>  *Use only one variable to write PEC_EN bit in configuration register
>  *Use regmap_set_bits to set PEC_EN bit when requested instead of
>   regmap_update_bits.
>  *Fix typo in commit message.
> v4:
>  *Use regmap_clear_bits to clear PEC_EN bit when requested instead of
>   regmap_update_bits.
> v5:
>  *Adapt driver to the new hwmon PEC attribute handling from the hwmon cor=
e.
> ---
>  Documentation/hwmon/max31827.rst | 13 ++++++++++---
>  drivers/hwmon/max31827.c         | 17 ++++++++++++++++-
>  2 files changed, 26 insertions(+), 4 deletions(-)
>=20
> diff --git a/Documentation/hwmon/max31827.rst b/Documentation/hwmon/max31=
827.rst
> index 44ab9dc064cb..9c11a9518c67 100644
> --- a/Documentation/hwmon/max31827.rst
> +++ b/Documentation/hwmon/max31827.rst
> @@ -131,7 +131,14 @@ The Fault Queue bits select how many consecutive tem=
perature faults must occur
>  before overtemperature or undertemperature faults are indicated in the
>  corresponding status bits.
>=20
> -Notes
> ------
> +PEC Support
> +-----------
> +
> +When reading a register value, the PEC byte is computed and sent by the =
chip.
> +
> +PEC on word data transaction respresents a signifcant increase in bandwi=
tdh
> +usage (+33% for both write and reads) in normal conditions.
>=20
> -PEC is not implemented.
> +Since this operation implies there will be an extra delay to each
> +transaction, PEC can be disabled or enabled through sysfs.
> +Just write 1  to the "pec" file for enabling PEC and 0 for disabling it.
> diff --git a/drivers/hwmon/max31827.c b/drivers/hwmon/max31827.c
> index f8a13b30f100..4a5f86afd84e 100644
> --- a/drivers/hwmon/max31827.c
> +++ b/drivers/hwmon/max31827.c
> @@ -24,6 +24,7 @@
>=20
>  #define MAX31827_CONFIGURATION_1SHOT_MASK	BIT(0)
>  #define MAX31827_CONFIGURATION_CNV_RATE_MASK	GENMASK(3, 1)
> +#define MAX31827_CONFIGURATION_PEC_EN_MASK	BIT(4)
>  #define MAX31827_CONFIGURATION_TIMEOUT_MASK	BIT(5)
>  #define MAX31827_CONFIGURATION_RESOLUTION_MASK	GENMASK(7, 6)
>  #define MAX31827_CONFIGURATION_ALRM_POL_MASK	BIT(8)
> @@ -332,6 +333,18 @@ static int max31827_read(struct device *dev, enum hw=
mon_sensor_types type,
>  	return ret;
>  }
>=20
> +static int max31827_chip_write(struct *regmap, u32 attr, long val)

regmap parameter is declared wrong here, will send fix in next patch.

> +{
> +	switch (attr) {
> +	case hwmon_chip_pec:
> +		return regmap_update_bits(regmap, MAX31827_CONFIGURATION_REG,
> +					  MAX38127_CONFIGURATION_PEC_EN_MASK,

typo, MAX38127 -> MAX31827

> +					  val ? MAX38127_CONFIGURATION_PEC_EN_MASK : 0);

typo, MAX38127 -> MAX31827

> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
>  static int max31827_write(struct device *dev, enum hwmon_sensor_types ty=
pe,
>  			  u32 attr, int channel, long val)
>  {
> @@ -340,6 +353,8 @@ static int max31827_write(struct device *dev, enum hw=
mon_sensor_types type,
>  	int ret;
>=20
>  	switch (type) {
> +	case hwmon_chip:
> +		return max31827_chip_write(st->regmap, attr, val);
>  	case hwmon_temp:
>  		switch (attr) {
>  		case hwmon_temp_enable:
> @@ -583,7 +598,7 @@ static const struct hwmon_channel_info *max31827_info=
[] =3D {
>  					 HWMON_T_MIN_HYST | HWMON_T_MIN_ALARM |
>  					 HWMON_T_MAX | HWMON_T_MAX_HYST |
>  					 HWMON_T_MAX_ALARM),
> -	HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL),
> +	HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL | HWMON_C_PEC),
>  	NULL,
>  };

This patch is wrong due to the fact that hwmon_chip case is duplicated insi=
de the switch, and therefore
I will send another patch fixing this and also modifying max31827_chip_writ=
e.

>=20
> --
> 2.34.1