Re: [PATCH 2/3] bindings: hwmon: Add adi,adaq4224_temp as compatible string

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

 



On 12/15/23 00:49, Krzysztof Kozlowski wrote:
On 14/12/2023 15:36, Daniel Matyas wrote:
In the device ada4224 the max31827 temperature sensor will be used, so
the default values corresponding to adaq4224_temp are the same for
max31827.

Signed-off-by: Daniel Matyas <daniel.matyas@xxxxxxxxxx>

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.

---
  Documentation/devicetree/bindings/hwmon/adi,max31827.yaml | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/hwmon/adi,max31827.yaml b/Documentation/devicetree/bindings/hwmon/adi,max31827.yaml
index f60e06ab7d0a..9f3b0839aa46 100644
--- a/Documentation/devicetree/bindings/hwmon/adi,max31827.yaml
+++ b/Documentation/devicetree/bindings/hwmon/adi,max31827.yaml
@@ -20,6 +20,7 @@ properties:
        - const: adi,max31827
        - items:
            - enum:
+              - adi,adaq4224_temp

Underscores are not allowed


That isn't the main problem with this patch.
https://github.com/analogdevicesinc/linux/tree/dev_adaq4224_dts
suggests that it may be a development system which utilizes the max31827.
From there, we can see that there is a devicetree description of a board
with that name which uses

                temperature1: temperature@5f {
                        compatible = "adi,adaq4224_temp";
                        reg = <0x5f>;
                        vref-supply = <&vio>;

                        adi,comp-int;
                        adi,alarm-pol = <0>;
                        adi,fault-q = <1>;
                        adi,timeout-enable;
                };

That doesn't make sense to me. I don't know why they don't just reference max31827.
I am most definitely not going to accept a driver change just to map adi,adaq4224_temp
(or adi,adaq4224-temp) to the actually used temperature sensor chip. If we start
accepting that, we'd end up with no end of
"<vendor>,<board_name>-{temp,voltage,current,power,...}"
compatibles.

Looking more into the above mentioned repository/branch, an earlier version
of the dts file did reference adi,max31827 for that chip. It also looks like
there may be an adaq4224 ADC (per drivers/iio/adc/ad4630.c), but that would be
a SPI chip. It seems highly unlikely that a SPI ADC would have a separate I2C
interface connected to a temperature sensor. Confusing.

There is also some indication that this may some IP to be loaded into an FPGA.
which utilizes an FPGA implementation of max31827 (or maybe connects to one).
If that is the case, it should still be referenced as max31827.

All that wasted time because of "let's provide as little as possible information
about what we are actually doing" :-(.

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