Re: [PATCH 3/3] hwmon: max31827: Compatible for adaq4224

From: Guenter Roeck <linux@roeck-us.net>
To: Daniel Matyas <daniel.matyas@analog.com>
Cc: Jean Delvare <jdelvare@suse.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org
Subject: Re: [PATCH 3/3] hwmon: max31827: Compatible for adaq4224
Date: Thu, 14 Dec 2023 07:14:18 -0800	[thread overview]
Message-ID: <937db277-1d3d-459a-a597-475bdc0229fe@roeck-us.net> (raw)
In-Reply-To: <20231214143648.175336-3-daniel.matyas@analog.com>

On 12/14/23 06:36, Daniel Matyas wrote:
> Compatible string "adi,adaq4224_temp" is accepted in device tree.
> When this string is seen in the device tree, the name of the device
> changes to "adaq4224_temp" and the default configuration of max31827
> is loaded.
> 
> This modification was requested by the costumer, so that whenever one

customer

> analyzes the available devices, one can know for sure, that max31827 is
> part of the adaq4224.
> 

There is (officially) no such chip. I have no idea if this is a new chip
or something else, and I have no idea how it relates to max31827.
Either case, the "_temp" in the chip name doesn't make sense to me.

> Signed-off-by: Daniel Matyas <daniel.matyas@analog.com>
> ---
>   drivers/hwmon/max31827.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/max31827.c b/drivers/hwmon/max31827.c
> index db93492193bd..c3500a5b2c29 100644
> --- a/drivers/hwmon/max31827.c
> +++ b/drivers/hwmon/max31827.c
> @@ -48,7 +48,7 @@
>   #define MAX31827_M_DGR_TO_16_BIT(x)	(((x) << 4) / 1000)
>   #define MAX31827_DEVICE_ENABLE(x)	((x) ? 0xA : 0x0)
>   
> -enum chips { max31827 = 1, max31828, max31829 };
> +enum chips { max31827 = 1, max31828, max31829, adaq4224_temp };
>   
>   enum max31827_cnv {
>   	MAX31827_CNV_1_DIV_64_HZ = 1,
> @@ -592,6 +592,7 @@ static const struct i2c_device_id max31827_i2c_ids[] = {
>   	{ "max31827", max31827 },
>   	{ "max31828", max31828 },
>   	{ "max31829", max31829 },
> +	{ "adaq4224_temp", adaq4224_temp },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(i2c, max31827_i2c_ids);
> @@ -620,6 +621,9 @@ static int max31827_init_client(struct max31827_state *st)
>   	res |= FIELD_PREP(MAX31827_CONFIGURATION_TIMEOUT_MASK, !prop);
>   
>   	type = (enum chips)(uintptr_t)device_get_match_data(dev);
> +	if (type == adaq4224_temp) {
> +		dev->driver->name = "adaq4224_temp";
> +	}
>   

No, sorry, we don't make such changes. The driver has a fixed name
which is independent of the device connected to it.

Guenter