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: <76cae9bf-c81a-684a-c22b-9548dd82c8d4@linaro.org>
Date:   Sun, 20 Nov 2022 11:40:07 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v3 4/7] dt-bindings: pinctrl: add bindings for Mediatek
 MT8365 SoC
Content-Language: en-US
To:     Kevin Hilman <khilman@baylibre.com>,
        =?UTF-8?Q?Bernhard_Rosenkr=c3=a4nzer?= <bero@baylibre.com>,
        linux-mediatek@lists.infradead.org
Cc:     linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
        angelogiocchino.delregno@collabora.com
References: <20221117210356.3178578-1-bero@baylibre.com>
 <20221117210356.3178578-5-bero@baylibre.com>
 <06e916b7-f8f2-6de5-f86e-7b020c052451@linaro.org>
 <7ha64o9h1d.fsf@baylibre.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <7ha64o9h1d.fsf@baylibre.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 18/11/2022 20:52, Kevin Hilman wrote:
> Hi Krzysztof,
> 
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes:
> 
>> On 17/11/2022 22:03, Bernhard Rosenkränzer wrote:
>>> Add devicetree bindings for Mediatek MT8365 pinctrl driver.
>>>
>>> Signed-off-by: Bernhard Rosenkränzer <bero@baylibre.com>
>>
>> Thank you for your patch. There is something to discuss/improve.
>>
>>> +
>>> +  pins-are-numbered:
>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>> +    description: |
>>> +      Specify the subnodes are using numbered pinmux to specify pins.
>>
>> Why would you name pins differently per board? And why this different
>> naming of the same pins is a property of hardware?
>>
>> This looks like something to drop.
> 
> Yeah, having this as a flag kind of implies that this could be present
> for some boards but not others.  But in practice, the driver requires it
> to be present or just fails[1].  What's the right way to describe that?
> We're just trying to add a binding that reflects the existing driver.

Uh, what an interesting property. What's the point of it then? Why
failing to probe on a missing property which does nothing else?

The solution is also to drop that property from the driver.

> We also noticed that there's another documented binding with this
> same flag[2] where similiarily, the driver simply requires it to be
> present[2].
> 
> So is the way this flag is documented in the stm32 binding OK for the
> mediatek one also?  If not, what would you suggest?

I would like to understand why do we need this property and what is
described by it. Because if it's purpose is only to fail or not fail
driver probe, then we should just drop it everywhere.


> 

Best regards,
Krzysztof