Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9100e41b-291e-9723-4188-b4d3e5adb6f8@linaro.org>
Date:   Thu, 31 Aug 2023 14:01:28 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.14.0
Subject: Re: [PATCH 1/2] dt-binding: Add custom property for MAX7357
Content-Language: en-US
To:     Naresh Solanki <naresh.solanki@9elements.com>
Cc:     Peter Rosin <peda@axentia.se>, Andi Shyti <andi.shyti@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Patrick Rudolph <patrick.rudolph@9elements.com>,
        linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230830115744.4102929-1-Naresh.Solanki@9elements.com>
 <b467c423-dbd4-8a14-8fa4-169658e21693@linaro.org>
 <CABqG17hB_GvQ56ZB+wjhSrDtZLreZ4vPc+3AfUj6AdA3Btd1+Q@mail.gmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <CABqG17hB_GvQ56ZB+wjhSrDtZLreZ4vPc+3AfUj6AdA3Btd1+Q@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 31/08/2023 11:45, Naresh Solanki wrote:
> Hi
> 
> On Wed, 30 Aug 2023 at 20:08, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 30/08/2023 13:57, Naresh Solanki wrote:
>>> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>>>
>>> Add a custom property "maxim,bus-lockup-fix" to enable proprietary
>>> features on MAX7357. The driver configures MAX7357 to isolate the
>>> failing channel and trigger a flush-out sequence for bus lock-up
>>> resolution.
>>
>> 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.
> Ack
>>
>> It is "dt-bindings" not binding and several other fields are needed.
> Ack
>>
>> Also "add custom property" is quite generic. When you add next custom
>> property you are going to have two commits with the same subject. Just
>> make it descriptive - "Add foobar for MAX7357"
> Missed Properties in this Patch Series, Will Be Addressed in V2.
> So in that case I guess the below title should be fine?
> dt-bindings: i2c: Add custom properties for MAX7357/MAX7358

No, because you do not solve that part of my feedback:

>> Also "add custom property" is quite generic. When you add next custom
>> property you are going to have two commits with the same subject.

I said, everything will be now "add custom properties", right?

The prefix is dt-bindings: i2c: pca954x:


> 
>>
>>
>>
>>>
>>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>>> ---
>>>  .../devicetree/bindings/i2c/i2c-mux-pca954x.yaml  | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>>> index 2d7bb998b0e9..984d4614a270 100644
>>> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>>> @@ -71,6 +71,11 @@ properties:
>>>      description: A voltage regulator supplying power to the chip. On PCA9846
>>>        the regulator supplies power to VDD2 (core logic) and optionally to VDD1.
>>>
>>> +  maxim,bus-lockup-fix:
>>> +    type: boolean
>>> +    description: Isolates only the stuck channel and generates a flush-out sequence
>>> +      to attempt to clear the bus lock-up.
>>
>> Why wouldn't you want it to be enabled all the time? Why should it be
>> configurable per-board?
> The chip doesn't enable these features by default & it is left to
> discretion of board designer to enable the same.


That does not explain anything. Enable them by default in such case or
come with some argument.

Best regards,
Krzysztof