Re: [PATCH v5 2/4] hwmon: max31827: Add support for max31828 and max31829

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/27/23 08:05, Matyas, Daniel wrote:


-----Original Message-----
From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
Sent: Friday, October 27, 2023 5:52 PM
To: Matyas, Daniel <Daniel.Matyas@xxxxxxxxxx>
Cc: Jean Delvare <jdelvare@xxxxxxxx>; Jonathan Corbet
<corbet@xxxxxxx>; linux-hwmon@xxxxxxxxxxxxxxx; linux-
doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
Subject: Re: [PATCH v5 2/4] hwmon: max31827: Add support for
max31828 and max31829

[External]

On 10/27/23 06:00, Matyas, Daniel wrote:
[ ... ]

I also don't understand why that would be chip specific. I don't see
anything along that line in the datasheet.

Ah, wait ... I guess that is supposed to reflect the chip default.
I don't see why the chip default makes a difference - a well defined
default must be set either way. Again, there is no guarantee that the
chip is in its default state when the driver is loaded.

The well defined default was set in v4, but I deleted it, because the
default value in hex for max31827 and max31828 alarm polarity, and
max31827 fault queue is 0x0. I had 2 #defines for these values, but you
said:
" Since MAX31827_ALRM_POL_LOW is 0, this code doesn't really do
anything and just pollutes the code."

So, I thought I should remove it altogether, since res is set to 0 in the
beginning and the default value of these chips (i.e. 0) is implicitly set.


Also, why are the default values added in this patch and not in the
previous patch ?


In v4 these default values were set in the previous patch.


I asked you (or meant to ask you) to stop overwriting 0 with 0 in a
variable. I didn't mean to ask you (if I did) to stop writing the default value
into the chip. Sorry if I did; if so, that was a misunderstanding.

Guenter

Well, writing the default value into res, would just overwrite 0 with 0. Should I still do it?


No, that is not correct. You don't know what is in the chip register.
It may not be the chip default.

Guenter




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux