[PATCH 0/5] iio: mcp9600: Features and improvements

[PATCH 0/5] iio: mcp9600: Features and improvements

[Ben Collins]

From: Ben Collins <bcollins@kernel.org>

ChangeLog:
v2 -> v3:
  - Improve changelogs in each patch
  - Based on feedback from Andy Shevchenko <andy.shevchenko@gmail.com>
    * Set register offsets to fixed width
    * Fix typos
    * Future-proof Kconfig changes
    * Convert to using chip_info paradigm
    * Verbiage: dt -> firmware description
    * Use proper specifiers and drop castings
    * Fix register offset to be fixed-width
    * u8 for cfg var
    * Fix % type for u32 to be %u
    * Make blank lines consistent between case statements
    * FIELD_PREP -> FIELD_MODIFY
    * Remove explicit setting of 0 value in filter_level
  - Based on feedback from David Lechner <dlechner@baylibre.com>
    * Rework IIR values exposed to sysfs. Using the ratios, there was no
      way to represent "disabled" (i.e. infinity). Based on the bmp280
      driver I went with using the power coefficients (e.g. 1, 2, 4, 8,
      ...) where 1 is disabled (n=0).

v1 -> v2:
  - Break into individual patches

v1:
  - Initial patch to enable IIR and thermocouple-type
  - Recognize mcp9601

Ben Collins (5):
  dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601
  iio: mcp9600: White space cleanup for tab alignment
  iio: mcp9600: Recognize chip id for mcp9601
  iio: mcp9600: Add support for thermocouple-type
  iio: mcp9600: Add support for IIR filter

 .../iio/temperature/microchip,mcp9600.yaml    |   6 +-
 drivers/iio/temperature/Kconfig               |   8 +-
 drivers/iio/temperature/mcp9600.c             | 209 ++++++++++++++++--
 3 files changed, 201 insertions(+), 22 deletions(-)

-- 
2.50.1

[PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Ben Collins]

The mcp9600 driver supports the mcp9601 chip, but complains about not
recognizing the device id on probe. A separate patch...

	iio: mcp9600: Recognize chip id for mcp9601

...addresses this. This patch updates the dt-bindings for this chip to
reflect the change to allow explicitly setting microchip,mcp9601 as
the expected chip type.

The mcp9601 also supports features not found on the mcp9600, so this
will also allow the driver to differentiate the support of these
features.

Signed-off-by: Ben Collins <bcollins@watter.com>
---
 .../bindings/iio/temperature/microchip,mcp9600.yaml         | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
index d2cafa38a5442..d8af0912ce886 100644
--- a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
+++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/iio/temperature/microchip,mcp9600.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Microchip MCP9600 thermocouple EMF converter
+title: Microchip MCP9600 and similar thermocouple EMF converters
 
 maintainers:
   - Andrew Hepp <andrew.hepp@ahepp.dev>
@@ -14,7 +14,9 @@ description:
 
 properties:
   compatible:
-    const: microchip,mcp9600
+    enum:
+      - microchip,mcp9600
+      - microchip,mcp9601
 
   reg:
     maxItems: 1
-- 
2.50.1

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Jonathan Cameron]

On Fri, 15 Aug 2025 16:46:03 +0000
Ben Collins <bcollins@watter.com> wrote:

> The mcp9600 driver supports the mcp9601 chip, but complains about not
> recognizing the device id on probe. A separate patch...
> 
> 	iio: mcp9600: Recognize chip id for mcp9601
> 
> ...addresses this. This patch updates the dt-bindings for this chip to
> reflect the change to allow explicitly setting microchip,mcp9601 as
> the expected chip type.
> 
> The mcp9601 also supports features not found on the mcp9600, so this
> will also allow the driver to differentiate the support of these
> features.

If it's additional features only then you can still use a fallback
compatible.  Intent being that a new DT vs old kernel still 'works'.

Then for the driver on new kernels we match on the new compatible and
support those new features.  Old kernel users get to keep the ID
mismatch warning - they can upgrade if they want that to go away ;)

Krzysztof raised the same point on v2 but I'm not seeing it addressed
in that discussion.

Jonathan

> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> ---
>  .../bindings/iio/temperature/microchip,mcp9600.yaml         | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> index d2cafa38a5442..d8af0912ce886 100644
> --- a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> +++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/iio/temperature/microchip,mcp9600.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Microchip MCP9600 thermocouple EMF converter
> +title: Microchip MCP9600 and similar thermocouple EMF converters
>  
>  maintainers:
>    - Andrew Hepp <andrew.hepp@ahepp.dev>
> @@ -14,7 +14,9 @@ description:
>  
>  properties:
>    compatible:
> -    const: microchip,mcp9600
> +    enum:
> +      - microchip,mcp9600
> +      - microchip,mcp9601
>  
>    reg:
>      maxItems: 1

Re: [PATCH 0/5] iio: mcp9600: Features and improvements

[Jonathan Cameron]

On Fri, 15 Aug 2025 16:46:02 +0000
Ben Collins <bcollins@watter.com> wrote:

> From: Ben Collins <bcollins@kernel.org>

See submitting patches documentation.  Should have a version number in the [] in the patch title.

Jonathan

> 
> ChangeLog:
> v2 -> v3:
>   - Improve changelogs in each patch
>   - Based on feedback from Andy Shevchenko <andy.shevchenko@gmail.com>
>     * Set register offsets to fixed width
>     * Fix typos
>     * Future-proof Kconfig changes
>     * Convert to using chip_info paradigm
>     * Verbiage: dt -> firmware description
>     * Use proper specifiers and drop castings
>     * Fix register offset to be fixed-width
>     * u8 for cfg var
>     * Fix % type for u32 to be %u
>     * Make blank lines consistent between case statements
>     * FIELD_PREP -> FIELD_MODIFY
>     * Remove explicit setting of 0 value in filter_level
>   - Based on feedback from David Lechner <dlechner@baylibre.com>
>     * Rework IIR values exposed to sysfs. Using the ratios, there was no
>       way to represent "disabled" (i.e. infinity). Based on the bmp280
>       driver I went with using the power coefficients (e.g. 1, 2, 4, 8,
>       ...) where 1 is disabled (n=0).
> 
> v1 -> v2:
>   - Break into individual patches
> 
> v1:
>   - Initial patch to enable IIR and thermocouple-type
>   - Recognize mcp9601
> 
> Ben Collins (5):
>   dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601
>   iio: mcp9600: White space cleanup for tab alignment
>   iio: mcp9600: Recognize chip id for mcp9601
>   iio: mcp9600: Add support for thermocouple-type
>   iio: mcp9600: Add support for IIR filter
> 
>  .../iio/temperature/microchip,mcp9600.yaml    |   6 +-
>  drivers/iio/temperature/Kconfig               |   8 +-
>  drivers/iio/temperature/mcp9600.c             | 209 ++++++++++++++++--
>  3 files changed, 201 insertions(+), 22 deletions(-)
> 

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[David Lechner]

On 8/16/25 4:58 AM, Jonathan Cameron wrote:
> On Fri, 15 Aug 2025 16:46:03 +0000
> Ben Collins <bcollins@watter.com> wrote:
> 
>> The mcp9600 driver supports the mcp9601 chip, but complains about not
>> recognizing the device id on probe. A separate patch...
>>
>> 	iio: mcp9600: Recognize chip id for mcp9601
>>
>> ...addresses this. This patch updates the dt-bindings for this chip to
>> reflect the change to allow explicitly setting microchip,mcp9601 as
>> the expected chip type.
>>
>> The mcp9601 also supports features not found on the mcp9600, so this
>> will also allow the driver to differentiate the support of these
>> features.
> 
> If it's additional features only then you can still use a fallback
> compatible.  Intent being that a new DT vs old kernel still 'works'.
> 
> Then for the driver on new kernels we match on the new compatible and
> support those new features.  Old kernel users get to keep the ID
> mismatch warning - they can upgrade if they want that to go away ;)
> 
> Krzysztof raised the same point on v2 but I'm not seeing it addressed
> in that discussion.

One could make the argument that these are not entirely fallback
compatible since bit 4 of the STATUS register has a different
meaning depending on if the chip is MCP9601/L01/RL01 or not.

Interestingly, the existing bindings include interrupts for
open circuit and short circuit alert pins. But these pins
also only exist on MCP9601/L01/RL01. If we decide these aren't
fallback compatible, then those properties should have the
proper constraints added as well.

> 
> Jonathan
> 
>>
>> Signed-off-by: Ben Collins <bcollins@watter.com>
>> ---
>>  .../bindings/iio/temperature/microchip,mcp9600.yaml         | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
>> index d2cafa38a5442..d8af0912ce886 100644
>> --- a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
>> +++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
>> @@ -4,7 +4,7 @@
>>  $id: http://devicetree.org/schemas/iio/temperature/microchip,mcp9600.yaml#
>>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>>  
>> -title: Microchip MCP9600 thermocouple EMF converter
>> +title: Microchip MCP9600 and similar thermocouple EMF converters
>>  
>>  maintainers:
>>    - Andrew Hepp <andrew.hepp@ahepp.dev>
>> @@ -14,7 +14,9 @@ description:
>>  
>>  properties:
>>    compatible:
>> -    const: microchip,mcp9600
>> +    enum:
>> +      - microchip,mcp9600
>> +      - microchip,mcp9601
>>  
>>    reg:
>>      maxItems: 1
> 

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Ben Collins]

[-- Attachment #1: Type: text/plain, Size: 2360 bytes --]

On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
> > On Fri, 15 Aug 2025 16:46:03 +0000
> > Ben Collins <bcollins@watter.com> wrote:
> > 
> >> The mcp9600 driver supports the mcp9601 chip, but complains about not
> >> recognizing the device id on probe. A separate patch...
> >>
> >> 	iio: mcp9600: Recognize chip id for mcp9601
> >>
> >> ...addresses this. This patch updates the dt-bindings for this chip to
> >> reflect the change to allow explicitly setting microchip,mcp9601 as
> >> the expected chip type.
> >>
> >> The mcp9601 also supports features not found on the mcp9600, so this
> >> will also allow the driver to differentiate the support of these
> >> features.
> > 
> > If it's additional features only then you can still use a fallback
> > compatible.  Intent being that a new DT vs old kernel still 'works'.
> > 
> > Then for the driver on new kernels we match on the new compatible and
> > support those new features.  Old kernel users get to keep the ID
> > mismatch warning - they can upgrade if they want that to go away ;)
> > 
> > Krzysztof raised the same point on v2 but I'm not seeing it addressed
> > in that discussion.
> 
> One could make the argument that these are not entirely fallback
> compatible since bit 4 of the STATUS register has a different
> meaning depending on if the chip is MCP9601/L01/RL01 or not.

There are some nuances to this register between the two, but it can be
used generically as "not in range" for both.

My understanding from the docs is if VSENSE is connected on mcp9601,
then it is explicitly open-circuit detection vs. short-circuit, which
is bit 5.

> Interestingly, the existing bindings include interrupts for
> open circuit and short circuit alert pins. But these pins
> also only exist on MCP9601/L01/RL01. If we decide these aren't
> fallback compatible, then those properties should have the
> proper constraints added as well.

In my v4 patch, I'm going to remove the short/open circuit interrupts
since they are not implemented, yet.

I have VSENSE wired on my board so I can work on those interrupts and
register support in a later patch series.

-- 
 Ben Collins
 https://libjwt.io
 https://github.com/benmcollins
 --
 3EC9 7598 1672 961A 1139  173A 5D5A 57C7 242B 22CF

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[David Lechner]

On 8/17/25 11:37 AM, Ben Collins wrote:
> On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
>> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
>>> On Fri, 15 Aug 2025 16:46:03 +0000
>>> Ben Collins <bcollins@watter.com> wrote:
>>>
>>>> The mcp9600 driver supports the mcp9601 chip, but complains about not
>>>> recognizing the device id on probe. A separate patch...
>>>>
>>>> 	iio: mcp9600: Recognize chip id for mcp9601
>>>>
>>>> ...addresses this. This patch updates the dt-bindings for this chip to
>>>> reflect the change to allow explicitly setting microchip,mcp9601 as
>>>> the expected chip type.
>>>>
>>>> The mcp9601 also supports features not found on the mcp9600, so this
>>>> will also allow the driver to differentiate the support of these
>>>> features.
>>>
>>> If it's additional features only then you can still use a fallback
>>> compatible.  Intent being that a new DT vs old kernel still 'works'.
>>>
>>> Then for the driver on new kernels we match on the new compatible and
>>> support those new features.  Old kernel users get to keep the ID
>>> mismatch warning - they can upgrade if they want that to go away ;)
>>>
>>> Krzysztof raised the same point on v2 but I'm not seeing it addressed
>>> in that discussion.
>>
>> One could make the argument that these are not entirely fallback
>> compatible since bit 4 of the STATUS register has a different
>> meaning depending on if the chip is MCP9601/L01/RL01 or not.
> 
> There are some nuances to this register between the two, but it can be
> used generically as "not in range" for both.
> 
> My understanding from the docs is if VSENSE is connected on mcp9601,
> then it is explicitly open-circuit detection vs. short-circuit, which
> is bit 5.
> 
>> Interestingly, the existing bindings include interrupts for
>> open circuit and short circuit alert pins. But these pins
>> also only exist on MCP9601/L01/RL01. If we decide these aren't
>> fallback compatible, then those properties should have the
>> proper constraints added as well.
> 
> In my v4 patch, I'm going to remove the short/open circuit interrupts
> since they are not implemented, yet.

Don't remove them from the devicetree bindings. Even if the Linux driver
doesn't use it, the bindings should be as complete as possible.

https://docs.kernel.org/devicetree/bindings/writing-bindings.html

> 
> I have VSENSE wired on my board so I can work on those interrupts and
> register support in a later patch series.
> 

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Ben Collins]

[-- Attachment #1: Type: text/plain, Size: 3330 bytes --]

On Sun, Aug 17, 2025 at 11:51:22AM -0500, David Lechner wrote:
> On 8/17/25 11:37 AM, Ben Collins wrote:
> > On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
> >> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
> >>> On Fri, 15 Aug 2025 16:46:03 +0000
> >>> Ben Collins <bcollins@watter.com> wrote:
> >>>
> >>>> The mcp9600 driver supports the mcp9601 chip, but complains about not
> >>>> recognizing the device id on probe. A separate patch...
> >>>>
> >>>> 	iio: mcp9600: Recognize chip id for mcp9601
> >>>>
> >>>> ...addresses this. This patch updates the dt-bindings for this chip to
> >>>> reflect the change to allow explicitly setting microchip,mcp9601 as
> >>>> the expected chip type.
> >>>>
> >>>> The mcp9601 also supports features not found on the mcp9600, so this
> >>>> will also allow the driver to differentiate the support of these
> >>>> features.
> >>>
> >>> If it's additional features only then you can still use a fallback
> >>> compatible.  Intent being that a new DT vs old kernel still 'works'.
> >>>
> >>> Then for the driver on new kernels we match on the new compatible and
> >>> support those new features.  Old kernel users get to keep the ID
> >>> mismatch warning - they can upgrade if they want that to go away ;)
> >>>
> >>> Krzysztof raised the same point on v2 but I'm not seeing it addressed
> >>> in that discussion.
> >>
> >> One could make the argument that these are not entirely fallback
> >> compatible since bit 4 of the STATUS register has a different
> >> meaning depending on if the chip is MCP9601/L01/RL01 or not.
> > 
> > There are some nuances to this register between the two, but it can be
> > used generically as "not in range" for both.
> > 
> > My understanding from the docs is if VSENSE is connected on mcp9601,
> > then it is explicitly open-circuit detection vs. short-circuit, which
> > is bit 5.
> > 
> >> Interestingly, the existing bindings include interrupts for
> >> open circuit and short circuit alert pins. But these pins
> >> also only exist on MCP9601/L01/RL01. If we decide these aren't
> >> fallback compatible, then those properties should have the
> >> proper constraints added as well.
> > 
> > In my v4 patch, I'm going to remove the short/open circuit interrupts
> > since they are not implemented, yet.
> 
> Don't remove them from the devicetree bindings. Even if the Linux driver
> doesn't use it, the bindings should be as complete as possible.
> 
> https://docs.kernel.org/devicetree/bindings/writing-bindings.html
> 

I couldn't find anything that would easily describe this type of layout:

properties:
...
  interrupts:
    minItems: 1
    maxItems: 4
  interrupt-names:
    minItems: 1
    items:
      - const: alert1
      - const: alert2
      - const: alert3
      - const: alert4

allOf:
  - if:
      properties:
        compatible:
          contains:
            const: microchip,mcp9601
    then:
      # Override maxItems
      interrupts:
        maxItems: 6
      # XXX Add items to existing list???
      interrupt-names:
        items:
          - const: open-circuit
          - const: short-circuit

-- 
 Ben Collins
 https://libjwt.io
 https://github.com/benmcollins
 --
 3EC9 7598 1672 961A 1139  173A 5D5A 57C7 242B 22CF

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[David Lechner]

On 8/17/25 12:34 PM, Ben Collins wrote:
> On Sun, Aug 17, 2025 at 11:51:22AM -0500, David Lechner wrote:
>> On 8/17/25 11:37 AM, Ben Collins wrote:
>>> On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
>>>> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
>>>>> On Fri, 15 Aug 2025 16:46:03 +0000
>>>>> Ben Collins <bcollins@watter.com> wrote:
>>>>>
>>>>>> The mcp9600 driver supports the mcp9601 chip, but complains about not
>>>>>> recognizing the device id on probe. A separate patch...
>>>>>>
>>>>>> 	iio: mcp9600: Recognize chip id for mcp9601
>>>>>>
>>>>>> ...addresses this. This patch updates the dt-bindings for this chip to
>>>>>> reflect the change to allow explicitly setting microchip,mcp9601 as
>>>>>> the expected chip type.
>>>>>>
>>>>>> The mcp9601 also supports features not found on the mcp9600, so this
>>>>>> will also allow the driver to differentiate the support of these
>>>>>> features.
>>>>>
>>>>> If it's additional features only then you can still use a fallback
>>>>> compatible.  Intent being that a new DT vs old kernel still 'works'.
>>>>>
>>>>> Then for the driver on new kernels we match on the new compatible and
>>>>> support those new features.  Old kernel users get to keep the ID
>>>>> mismatch warning - they can upgrade if they want that to go away ;)
>>>>>
>>>>> Krzysztof raised the same point on v2 but I'm not seeing it addressed
>>>>> in that discussion.
>>>>
>>>> One could make the argument that these are not entirely fallback
>>>> compatible since bit 4 of the STATUS register has a different
>>>> meaning depending on if the chip is MCP9601/L01/RL01 or not.
>>>
>>> There are some nuances to this register between the two, but it can be
>>> used generically as "not in range" for both.
>>>
>>> My understanding from the docs is if VSENSE is connected on mcp9601,
>>> then it is explicitly open-circuit detection vs. short-circuit, which
>>> is bit 5.
>>>
>>>> Interestingly, the existing bindings include interrupts for
>>>> open circuit and short circuit alert pins. But these pins
>>>> also only exist on MCP9601/L01/RL01. If we decide these aren't
>>>> fallback compatible, then those properties should have the
>>>> proper constraints added as well.
>>>
>>> In my v4 patch, I'm going to remove the short/open circuit interrupts
>>> since they are not implemented, yet.
>>
>> Don't remove them from the devicetree bindings. Even if the Linux driver
>> doesn't use it, the bindings should be as complete as possible.
>>
>> https://docs.kernel.org/devicetree/bindings/writing-bindings.html
>>
> 
> I couldn't find anything that would easily describe this type of layout:
> 
> properties:
> ...
>   interrupts:
>     minItems: 1
>     maxItems: 4
>   interrupt-names:
>     minItems: 1
>     items:
>       - const: alert1
>       - const: alert2
>       - const: alert3
>       - const: alert4
> 
> allOf:
>   - if:
>       properties:
>         compatible:
>           contains:
>             const: microchip,mcp9601
>     then:
>       # Override maxItems
>       interrupts:
>         maxItems: 6
>       # XXX Add items to existing list???
>       interrupt-names:
>         items:
>           - const: open-circuit
>           - const: short-circuit
> 

We usually do this the other way around. The base binding lists
all of the possibilities then an -if: constraint limits them
if needed.


So don't change what is there already and then add:


allOf:
  - if:
      properties:
        compatible:
          not:
            contains:
              const: microchip,mcp9601
    then:
      properties:
        interrupts:
          maxItems: 4
        interrupt-names:
          maxItems: 4
          enum:
            - alert1
            - alert2
            - alert3
            - alert4

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Ben Collins]

[-- Attachment #1: Type: text/plain, Size: 5768 bytes --]

On Sun, Aug 17, 2025 at 12:59:48PM -0500, David Lechner wrote:
> On 8/17/25 12:34 PM, Ben Collins wrote:
> > On Sun, Aug 17, 2025 at 11:51:22AM -0500, David Lechner wrote:
> >> On 8/17/25 11:37 AM, Ben Collins wrote:
> >>> On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
> >>>> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
> >>>>> On Fri, 15 Aug 2025 16:46:03 +0000
> >>>>> Ben Collins <bcollins@watter.com> wrote:
> >>>>>
> >>>>>> The mcp9600 driver supports the mcp9601 chip, but complains about not
> >>>>>> recognizing the device id on probe. A separate patch...
> >>>>>>
> >>>>>> 	iio: mcp9600: Recognize chip id for mcp9601
> >>>>>>
> >>>>>> ...addresses this. This patch updates the dt-bindings for this chip to
> >>>>>> reflect the change to allow explicitly setting microchip,mcp9601 as
> >>>>>> the expected chip type.
> >>>>>>
> >>>>>> The mcp9601 also supports features not found on the mcp9600, so this
> >>>>>> will also allow the driver to differentiate the support of these
> >>>>>> features.
> >>>>>
> >>>>> If it's additional features only then you can still use a fallback
> >>>>> compatible.  Intent being that a new DT vs old kernel still 'works'.
> >>>>>
> >>>>> Then for the driver on new kernels we match on the new compatible and
> >>>>> support those new features.  Old kernel users get to keep the ID
> >>>>> mismatch warning - they can upgrade if they want that to go away ;)
> >>>>>
> >>>>> Krzysztof raised the same point on v2 but I'm not seeing it addressed
> >>>>> in that discussion.
> >>>>
> >>>> One could make the argument that these are not entirely fallback
> >>>> compatible since bit 4 of the STATUS register has a different
> >>>> meaning depending on if the chip is MCP9601/L01/RL01 or not.
> >>>
> >>> There are some nuances to this register between the two, but it can be
> >>> used generically as "not in range" for both.
> >>>
> >>> My understanding from the docs is if VSENSE is connected on mcp9601,
> >>> then it is explicitly open-circuit detection vs. short-circuit, which
> >>> is bit 5.
> >>>
> >>>> Interestingly, the existing bindings include interrupts for
> >>>> open circuit and short circuit alert pins. But these pins
> >>>> also only exist on MCP9601/L01/RL01. If we decide these aren't
> >>>> fallback compatible, then those properties should have the
> >>>> proper constraints added as well.
> >>>
> >>> In my v4 patch, I'm going to remove the short/open circuit interrupts
> >>> since they are not implemented, yet.
> >>
> >> Don't remove them from the devicetree bindings. Even if the Linux driver
> >> doesn't use it, the bindings should be as complete as possible.
> >>
> >> https://docs.kernel.org/devicetree/bindings/writing-bindings.html
> >>
> > 
> > I couldn't find anything that would easily describe this type of layout:
> > 
> > properties:
> > ...
> >   interrupts:
> >     minItems: 1
> >     maxItems: 4
> >   interrupt-names:
> >     minItems: 1
> >     items:
> >       - const: alert1
> >       - const: alert2
> >       - const: alert3
> >       - const: alert4
> > 
> > allOf:
> >   - if:
> >       properties:
> >         compatible:
> >           contains:
> >             const: microchip,mcp9601
> >     then:
> >       # Override maxItems
> >       interrupts:
> >         maxItems: 6
> >       # XXX Add items to existing list???
> >       interrupt-names:
> >         items:
> >           - const: open-circuit
> >           - const: short-circuit
> > 
> 
> We usually do this the other way around. The base binding lists
> all of the possibilities then an -if: constraint limits them
> if needed.
> 
> 
> So don't change what is there already and then add:
> 
> 
> allOf:
>   - if:
>       properties:
>         compatible:
>           not:
>             contains:
>               const: microchip,mcp9601
>     then:
>       properties:
>         interrupts:
>           maxItems: 4
>         interrupt-names:
>           maxItems: 4
>           enum:
>             - alert1
>             - alert2
>             - alert3
>             - alert4

This might be a little more complicated. I want to add a boolean for
microchip,vsense so the SC/OC aren't even available without that flag
being true (default false).

I could just assume that having the interrupts means this flag is true,
but that doesn't cover the case where the interrupts might not be used
or even wired up, but the SC/OC detection in the status register can be
used.

I was going with this:

  interrupts:
    minItems: 1
    maxItems: 4

  interrupt-names:
    minItems: 1
    items:
      - const: alert1
      - const: alert2
      - const: alert3
      - const: alert4

  microchip,vsense:
    default: false
    description:
      This flag indicates that the chip has been wired with VSENSE to
      enable open and short circuit detect. By default, this is false,
      since there's no way to detect that the chip is wired correctly.
    type: boolean

...

allOf:
  - if:
      properties:
        # XXX Does this work like logical AND? Passes dt_binding_check
        microchip,vsense: true
        compatible:
          contains:
            const: microchip,mcp9601
    then:
      properties:
        interrupts:
          minItems: 1
          maxItems: 6
        interrupt-names:
          items:
            - const: alert1
            - const: alert2
            - const: alert3
            - const: alert4
            - const: open-circuit
            - const: short-circuit
-- 
 Ben Collins
 https://libjwt.io
 https://github.com/benmcollins
 --
 3EC9 7598 1672 961A 1139  173A 5D5A 57C7 242B 22CF

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Ben Collins]

[-- Attachment #1: Type: text/plain, Size: 3919 bytes --]

On Sun, Aug 17, 2025 at 05:02:49PM -0500, Ben Collins wrote:
> On Sun, Aug 17, 2025 at 12:59:48PM -0500, David Lechner wrote:
> > On 8/17/25 12:34 PM, Ben Collins wrote:
> > > On Sun, Aug 17, 2025 at 11:51:22AM -0500, David Lechner wrote:
> > >> On 8/17/25 11:37 AM, Ben Collins wrote:
> > >>> On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
> > >>>> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
> > >>>>> On Fri, 15 Aug 2025 16:46:03 +0000
> > >>>>> Ben Collins <bcollins@watter.com> wrote:
> > >>>>>
> > >>>>>> The mcp9600 driver supports the mcp9601 chip, but complains about not
> > >>>>>> recognizing the device id on probe. A separate patch...
> > >>>>>>
> > >>>>>> 	iio: mcp9600: Recognize chip id for mcp9601
> > >>>>>>
> > >>>>>> ...addresses this. This patch updates the dt-bindings for this chip to
> > >>>>>> reflect the change to allow explicitly setting microchip,mcp9601 as
> > >>>>>> the expected chip type.
> > >>>>>>
> > >>>>>> The mcp9601 also supports features not found on the mcp9600, so this
> > >>>>>> will also allow the driver to differentiate the support of these
> > >>>>>> features.
> > >>>>>
> > >>>>> If it's additional features only then you can still use a fallback
> > >>>>> compatible.  Intent being that a new DT vs old kernel still 'works'.
> > >>>>>
> > >>>>> Then for the driver on new kernels we match on the new compatible and
> > >>>>> support those new features.  Old kernel users get to keep the ID
> > >>>>> mismatch warning - they can upgrade if they want that to go away ;)
> > >>>>>
> > >>>>> Krzysztof raised the same point on v2 but I'm not seeing it addressed
> > >>>>> in that discussion.
> > >>>>
> > >>>> One could make the argument that these are not entirely fallback
> > >>>> compatible since bit 4 of the STATUS register has a different
> > >>>> meaning depending on if the chip is MCP9601/L01/RL01 or not.
> > >>>
> > >>> There are some nuances to this register between the two, but it can be
> > >>> used generically as "not in range" for both.
> > >>>
> > >>> My understanding from the docs is if VSENSE is connected on mcp9601,
> > >>> then it is explicitly open-circuit detection vs. short-circuit, which
> > >>> is bit 5.
> > >>>
> > >>>> Interestingly, the existing bindings include interrupts for
> > >>>> open circuit and short circuit alert pins. But these pins
> > >>>> also only exist on MCP9601/L01/RL01. If we decide these aren't
> > >>>> fallback compatible, then those properties should have the
> > >>>> proper constraints added as well.
> > >>>
> > >>> In my v4 patch, I'm going to remove the short/open circuit interrupts
> > >>> since they are not implemented, yet.
> > >>
> > >> Don't remove them from the devicetree bindings. Even if the Linux driver
> > >> doesn't use it, the bindings should be as complete as possible.
> > >>
> > >> https://docs.kernel.org/devicetree/bindings/writing-bindings.html
> > >>
> > > 
> > > I couldn't find anything that would easily describe this type of layout:
...
> > We usually do this the other way around. The base binding lists
> > all of the possibilities then an -if: constraint limits them
> > if needed.
> > 
> > 
> > So don't change what is there already and then add:
> > 
...
> This might be a little more complicated. I want to add a boolean for
> microchip,vsense so the SC/OC aren't even available without that flag
> being true (default false).
> 
> I could just assume that having the interrupts means this flag is true,
> but that doesn't cover the case where the interrupts might not be used
> or even wired up, but the SC/OC detection in the status register can be
> used.
> 
> I was going with this:
> 

Nevermind, I figured this out. I'll send v4 soon.


-- 
 Ben Collins
 https://libjwt.io
 https://github.com/benmcollins
 --
 3EC9 7598 1672 961A 1139  173A 5D5A 57C7 242B 22CF

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Krzysztof Kozlowski]

On 17/08/2025 23:10, Ben Collins wrote:
>>>>>
>>>>
>>>> I couldn't find anything that would easily describe this type of layout:
> ...
>>> We usually do this the other way around. The base binding lists
>>> all of the possibilities then an -if: constraint limits them
>>> if needed.
>>>
>>>
>>> So don't change what is there already and then add:
>>>
> ...
>> This might be a little more complicated. I want to add a boolean for
>> microchip,vsense so the SC/OC aren't even available without that flag
>> being true (default false).
>>
>> I could just assume that having the interrupts means this flag is true,
>> but that doesn't cover the case where the interrupts might not be used
>> or even wired up, but the SC/OC detection in the status register can be
>> used.
>>
>> I was going with this:
>>
> 
> Nevermind, I figured this out. I'll send v4 soon.

You received from David correct code, good idea... yet you ignored it
and sent something incorrect - breaking ABI.
Best regards,
Krzysztof