linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: yassine.oudjana@gmail.com
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Sean Wang <sean.wang@kernel.org>,
	Andy Teng <andy.teng@mediatek.com>,
	AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@collabora.com>,
	Yassine Oudjana <y.oudjana@protonmail.com>,
	linux-mediatek@lists.infradead.org, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: Combine MediaTek MT67xx pinctrl binding docs
Date: Wed, 21 Sep 2022 12:25:20 +0300	[thread overview]
Message-ID: <8IZJIR.7AG9DG7U1XLJ@gmail.com> (raw)
In-Reply-To: <b567fb19-460c-ea1f-4c84-0724e73052fd@linaro.org>


On Wed, Sep 21 2022 at 09:20:43 AM +0200, Krzysztof Kozlowski 
<krzysztof.kozlowski@linaro.org> wrote:
> On 19/09/2022 19:01, Yassine Oudjana wrote:
>>  From: Yassine Oudjana <y.oudjana@protonmail.com>
>> 
>>  Documents for MT6779, MT6795 and MT6797 that currently exist share
>>  most properties, and each one has slightly differently worded
>>  descriptions for those properties. Combine all three documents into
>>  one common document for all MT67xx SoC pin controllers, picking a 
>> few
>>  parts from each and accounting for differences such as items in reg
>>  and reg-names properties. Also document the MT6765 pin controller
>>  which currently has a driver but no DT binding documentation. It 
>> should
>>  be possible to also include bindings for MT8183 and MT8188, but 
>> these
>>  have some additional properties that might complicate things a bit,
>>  so they are left alone for now.
>> 
> 
>>   properties:
>>     compatible:
>>  -    const: mediatek,mt6795-pinctrl
>>  +    oneOf:
>>  +      - enum:
>>  +          - mediatek,mt6765-pinctrl
>>  +          - mediatek,mt6795-pinctrl
>>  +          - mediatek,mt6797-pinctrl
>>  +      - items:
>>  +          - const: mediatek,mt6779-pinctrl
>>  +          - const: syscon
> 
> No, this is not like old bindings at all. It's not merging, it's a
> change sneaked inside huge diff. Also - probably totally untested on 
> DTS
> (or old bindings were broken).


Actually this change was made specifically so that it remains (probably 
becomes?) compatible with existing DTS and passes checks. mt6779.dtsi 
currently has the syscon compatible string but it wasn't listed along 
with mediatek,mt6779-pinctrl in the old document, but instead there was 
something in the description about putting the pinctrl node under a 
syscon node, which isn't the case in the existing DTS. This patch 
passed both dt_binding_check and dtbs_check. Anyway, I see how I failed 
to describe this change, so I'll go through the patch again and try to 
find any other small changes I might've made and forgotten about, and 
either put them in separate patches or describe them in the commit 
message, whichever one you think is better. Also, do I make those 
changes in the original documents then combine or combine first then 
make them in the new one?

Thanks,
Yassine

(Sorry for the spam, my client was misconfigured so it previously sent 
HTML instead of plain text.)

> 
> That's a no-go.
> 
> Best regards,
> Krzysztof



  reply	other threads:[~2022-09-21  9:25 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-19 17:01 [PATCH 0/4] MediaTek MT6735 pinctrl support and DT binding changes Yassine Oudjana
2022-09-19 17:01 ` [PATCH 1/4] dt-bindings: pinctrl: Combine MediaTek MT67xx pinctrl binding docs Yassine Oudjana
2022-09-20  8:06   ` AngeloGioacchino Del Regno
2022-09-21  7:11     ` Krzysztof Kozlowski
2022-09-21  9:30       ` yassine.oudjana
2022-09-21  9:45         ` AngeloGioacchino Del Regno
2022-09-21 10:24           ` yassine.oudjana
2022-09-21 10:55             ` AngeloGioacchino Del Regno
2022-09-21  7:20   ` Krzysztof Kozlowski
2022-09-21  9:25     ` yassine.oudjana [this message]
2022-09-19 17:01 ` [PATCH 2/4] arm64: dts: mediatek: mt6797: Make pin configuration nodes follow DT bindings Yassine Oudjana
2022-09-19 17:01 ` [PATCH 3/4] dt-bindings: pinctrl: mediatek,mt67xx-pinctrl: Document MT6735 pin controller bindings Yassine Oudjana
2022-09-19 17:01 ` [PATCH 4/4] pinctrl: mediatek: Add MT6735 pinctrl driver Yassine Oudjana

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8IZJIR.7AG9DG7U1XLJ@gmail.com \
    --to=yassine.oudjana@gmail.com \
    --cc=andy.teng@mediatek.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=sean.wang@kernel.org \
    --cc=y.oudjana@protonmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).