Message-ID: <a78b52ec-81bf-9ee1-9e12-135079d19b7a@axentia.se>
Date:   Wed, 15 Dec 2021 13:42:14 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.2
Subject: Re: [PATCH 1/4] dt-bindings: i2c Update PCA954x
Content-Language: en-US
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Patrick Rudolph <patrick.rudolph@9elements.com>
Cc:     Rob Herring <robh+dt@kernel.org>, linux-i2c@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20211214095021.572799-1-patrick.rudolph@9elements.com>
 <Ybh8cCU/zbfXkXYO@pendragon.ideasonboard.com>
From:   Peter Rosin <peda@axentia.se>
Organization: Axentia Technologies AB
In-Reply-To: <Ybh8cCU/zbfXkXYO@pendragon.ideasonboard.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZnBNN1pzMnRMN0xaUTlhWjRTV1dweGFhZytoMTkzTWNseGQ1ampPMFZkeVBj?=
 =?utf-8?B?SjBaMmMrOFhDNmVHRjdicnVLVzcvUU9tS3pNYTB0NTJtK1llaGVlbXhsT3Bq?=
 =?utf-8?B?WXFGT0VCSVBIZFFYT0tETHoyb3hURFpTNm9NL3dFV29FaExwa0I0R2VjcE9k?=
 =?utf-8?B?Y0lILzVXRFRjdS9oSTgyQlNvRFdJWWZpVEV1TU1QMHZVV041dSt5Mi9QK3dw?=
 =?utf-8?B?blBMaTY2V2t3MTlGaGM3TStnRGx6Z08zWllGblkzRUZXMk9ZYWxCTVp4MDJh?=
 =?utf-8?B?WVlUblU2UTNNV0UzQklOelNRNU9rU1dtenhDcTFmZDM5NlFtYkFVQU52T3p2?=
 =?utf-8?B?UEd0MHhqYTRrVkZNZVRkRWVDMng2WnE2TXBSc2tYR25adG5JOXVKVVFYU2Vh?=
 =?utf-8?B?VlJYWEJIeWhqSU9xVHdEdXNqMlRpa3lqTWl1V3BTK2x6U2hBNmhtQTZaUFYx?=
 =?utf-8?B?bHpYNElCMTUzOG80UUVQZDJlMTJGR01hMTczRHhHTmRsQlVwNGdzeWRZM29H?=
 =?utf-8?B?empocUFvWThzZ0FacWgxaFhIdEtxYW16YUZEdWErSHJHZ1V0eVFFY3h4TVBW?=
 =?utf-8?B?Rk4wVU5GQlU5RmJWWjZMTWpFK2UybVVmVVZVa0dZRVJ6ZzJWTGM3SGhicnlW?=
 =?utf-8?B?U2pUWHg4Z09UYzFIWlRUUHhYWGIvZFdOVldDVGdqREdabWlNdndRVW5udWo3?=
 =?utf-8?B?UHZjNDBNQzU5S3F3R2NtMTJFV29CNGpPdXFLVS85Szl1c3JPYXVORmxEV2li?=
 =?utf-8?B?WGdTU2FMOXRhSVdPaGFwWEZ1b0djRUNpV0tTVExMYWJiN2FmVDRlTTR5alc5?=
 =?utf-8?B?TjlxNlkxcUFtbktkNlJIQnlPb0lGT21NRmV2OXRXcldZNW9sbzdsV1B6OExE?=
 =?utf-8?B?WmdHS2xRMVpINTNFd3VSa2FvOTNKNzdZMXBpNFIxeWZ0cCtrZXcxM096OGR1?=
 =?utf-8?B?Y2dxYk5GTzZhV3VjREtZUUpWM0d4SnBCNnRndHNHU2lyN0xDS2l2cFAzZ0VI?=
 =?utf-8?B?QVErQWUybUR1ZFdwKzZHVXllcG9GZzh6ZXhCTUk4TDhoT09CQjZIVlljcmpx?=
 =?utf-8?B?a3gwazNLRDlVaW5wVFZya0xvWDJWZjNaT2dVWXh6Nm9zMG9tWDgwUk9kQ2dw?=
 =?utf-8?B?NFJ4MkJvWXhNL1pYaCs5enYySVk5Q0NDODhiTnMyS3JHY1FjZkkrMFo0ZG9C?=
 =?utf-8?B?NEtTV0VyZ1NuS1VyQnAyeHZneG5RSkJOZDhmWWRFSjhWSHpadUcyVzk2dDBi?=
 =?utf-8?B?bHdtRFpxVkF1bTVMelJUUTdreXBHVzFNMXNYMzVPdnA4NUZLUGRpaFBDbVUr?=
 =?utf-8?B?czZ2aGZBellqUXRqbFdWdGRRa3hsU3ZDSU9IZjlpTExmZytVT1dmZjdSanVL?=
 =?utf-8?B?UHB1UXV3UFBCUlUvUnNlYjJST0ZxMkNOMVJtcytJa3hqSW16OE8ySmtyV2xG?=
 =?utf-8?B?MC9aSDZvUlo0UmRIS0IyOFFoRzU2OXpCUjFPZEFjMWFnbDVLazBCcytNY2N1?=
 =?utf-8?B?VFlvZFlrYkxaaU9lUytXZU4zMElzWTVaaTNNdXVLQVJqTlAzLzhjTTZ2eDVy?=
 =?utf-8?B?ZHNma1UvT2Z6by9RWkxSQWRySlg2amtILzhTWlRNUk5kbDYvVjRBalFOcjky?=
 =?utf-8?B?Mk90S3Zkem5ZbEV1aUxzdkk4Zm9uWkFmRS9uQXdvN0ovRnpOU2xEZ1lkKzkz?=
 =?utf-8?B?cjBGTkxGZVE0c3N3TUFZMGZwcFphZzd5YitjcW5BT2UyWDMyenpraGlMd0NN?=
 =?utf-8?B?U3B2K0IzSjBpUnhuTU8rR0pGeW5JS09uQjVWaVNDcTlJQTJMMWluY3FKcGtT?=
 =?utf-8?B?cFVRenZCZ2UyZ3JIT0hZd0lGcHdiR24yNEhFKzZRRFBpK25ncDlIcEgxbVhs?=
 =?utf-8?B?V2NWRko3TnlYWk4wSGo2QXRab3BmcTlhdFBEM3NJblA0SklCYUNzWVE3eThC?=
 =?utf-8?B?bERpbzNEN0pLd24ybmhSejYyU3RUWEd3RzhOWVFCV0EvU3ZMSkJsR1YvNUZP?=
 =?utf-8?B?bmlGOHFST2Z5YTNaNXdySzlnTFo3c3VaaW16T1c2a3VZY2NCRkFHRGpJQWd5?=
 =?utf-8?B?UkRybkJQVWcyQlN5Qll6a0RPaXR5K3EwQWR5WWl1MEVvVmxYSHo3QWZpS05I?=
 =?utf-8?Q?Baeo=3D?=
X-OriginatorOrg: axentia.se
X-MS-Exchange-CrossTenant-Network-Message-Id: 79eddb59-80ec-4605-1073-08d9bfc853c1
X-MS-Exchange-CrossTenant-AuthSource: DB8PR02MB5482.eurprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Dec 2021 12:42:17.0780
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4ee68585-03e1-4785-942a-df9c1871a234
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: gyBWqjpWHve2nEreAPnih7FsW++pDibt6T6En/OtBgqK8Fd9auWpedZqjI1raXKE
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR02MB4220
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi!

On 2021-12-14 12:13, Laurent Pinchart wrote:
> Hi Patrick,
> 
> Thank you for the patch.
> 
> On Tue, Dec 14, 2021 at 10:50:18AM +0100, Patrick Rudolph wrote:
>> Add the Maxim MAX735x as supported chip to PCA954x and add an
>> example how to use it.
>>
>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
>> ---
>>  .../bindings/i2c/i2c-mux-pca954x.yaml         | 40 +++++++++++++++++++
>>  1 file changed, 40 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>> index 9f1726d0356b..bd794cb80c11 100644
>> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>> @@ -11,6 +11,7 @@ maintainers:
>>  
>>  description:
>>    The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
>> +  Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
>>  
>>  allOf:
>>    - $ref: /schemas/i2c/i2c-mux.yaml#
>> @@ -19,6 +20,9 @@ properties:
>>    compatible:
>>      oneOf:
>>        - enum:
>> +          - maxim,max7356
>> +          - maxim,max7357
>> +          - maxim,max7358
>>            - nxp,pca9540
>>            - nxp,pca9542
>>            - nxp,pca9543
>> @@ -40,6 +44,7 @@ properties:
>>  
>>    interrupts:
>>      maxItems: 1
>> +    description: Only supported on NXP devices. Unsupported on Maxim MAX735x.
> 
> Could this be modelled by a YAML schema instead ? Something like
> 
> allOf:
>   - if:
>       properties:
>         compatible:
> 	  contains:
> 	    enum:
>               - maxim,max7356
>               - maxim,max7357
>               - maxim,max7358
>     then:
>       properties:
>         interrupts: false
> 
> (untested, it would be nice to use a pattern check for the compatible
> property if possible)

Some of the existing NXP chips do not support interrupts; we should
probably treat these new chips the same as the older ones. Either by
disallowing interrupts on both kinds or by continuing to ignore the
situation.

That said, I'm slightly in favor of the latter, since these new chips
do have interrupts, just not the same flavor as the NXP chips. What
the Maxim chips do not have is support for being an
	interrupt-controller;
At least that's how I read it...

I don't know how this situation is supposed to be described? Maybe this
new kind of interrupt should be indicated with a bus-error-interrupts
property (or bikeshed along those lines)? Maybe there should be two
entries in the existing interrupts property? Maybe these new chips
should be described in a new binding specific to maxim,max7356-7358
(could still be handled by the pca954x driver of course) to keep the
yaml simpler to read?

However, there is also maxim,max7367-7369 to consider. They seem to
have interrupts of the style described by the NXP binding (haven't
checked if the registers work the same, but since they reuse the
0x70 address-range the are in all likelihood also compatible).

Cheers,
Peter

>>  
>>    "#interrupt-cells":
>>      const: 2
>> @@ -100,6 +105,41 @@ examples:
>>                  #size-cells = <0>;
>>                  reg = <4>;
>>  
>> +                rtc@51 {
>> +                    compatible = "nxp,pcf8563";
>> +                    reg = <0x51>;
>> +                };
>> +            };
>> +        };
>> +    };
>> +
>> +  - |
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        i2c-mux@74 {
>> +            compatible = "maxim,max7357";
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +            reg = <0x74>;
>> +
>> +            i2c@1 {
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +                reg = <1>;
>> +
>> +                eeprom@54 {
>> +                    compatible = "atmel,24c08";
>> +                    reg = <0x54>;
>> +                };
>> +            };
>> +
>> +            i2c@7 {
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +                reg = <7>;
>> +
>>                  rtc@51 {
>>                      compatible = "nxp,pcf8563";
>>                      reg = <0x51>;
>