Received-SPF: pass (google.com: domain of linux-kernel+bounces-65035-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <eb97a068-4de6-4500-993b-86737844de70@gmail.com>
Date: Wed, 14 Feb 2024 11:19:10 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] dt-bindings: pwm: mediatek,mt2712: add compatible for
 MT7988
To: Conor Dooley <conor@kernel.org>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>, Rob Herring <robh@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>,
 =?UTF-8?Q?Uwe_Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>,
 John Crispin <john@phrozen.org>, linux-pwm@vger.kernel.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
 linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org,
 =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <rafal@milecki.pl>
References: <20240213164633.25447-1-zajec5@gmail.com>
 <20240213-resource-evaluator-0754cfd5882d@spud>
 <d4391868-ddcd-4f66-b539-28d245fa83df@gmail.com>
 <e957b044-fe84-4b72-bdf1-cbc40c722019@collabora.com>
 <20240214-reversion-arguably-37bbee9caf78@spud>
Content-Language: en-US
From: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com>
In-Reply-To: <20240214-reversion-arguably-37bbee9caf78@spud>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 14.02.2024 11:06, Conor Dooley wrote:
> On Wed, Feb 14, 2024 at 10:27:54AM +0100, AngeloGioacchino Del Regno wrote:
>> Il 14/02/24 07:34, Rafał Miłecki ha scritto:
>>> On 13.02.2024 19:18, Conor Dooley wrote:
>>>> On Tue, Feb 13, 2024 at 05:46:32PM +0100, Rafał Miłecki wrote:
>>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>>
>>>>> MT7988 has on-SoC controller that can control up to 8 PWMs.
>>>>
>>>> I see a binding and a dts patch, but no driver patch, how come?
>>>
>>> I believe that to avoid cross-trees patchsets (which are sometimes
>>> tricky for maintainers) there are two ways of submiting such changes:
>>> 1. dt-binding + driver; then (separately) DTS
>>> 2. dt-binding + DTS; then (separately) driver
>>>
>>> I chose later in this case as my personal priority right now is to deal
>>> with all MediaTek DTS files.
>>>
>>> Is that wrong or unacceptable?
>>>
>>
>> It's not wrong but it's partially unacceptable, at least on my side.
> 
>> I want to put emphasis on sending the binding with the driver, as this allows
>> for a better review on everyone's side because we do see the full picture and
>> we can give better advices: in this case, I'm not sure whether adding a new
>> compatible for MT7988 in an enum is a good idea, as the compatible string may
>> be shared with one of the *eleven* SoCs that are supported in the PWM driver,
>> meaning that (hardware speaking!) the PWM controller in 7988 might be the same
>> as the one in mt1234.
> 
> Re-ordering to make my reply make more sense...
> 
>> In my opinion (and I believe many do agree with me), sending the binding along
>> with the driver is the right choice, and if you also want to include the dts
>> that is also appreciated: series can go through multiple maintainers applying
>> subsets - it's ok to do.
> 
> Ye, either of those two makes my life a lot easier. I can then at least
> go and check the driver patch to see if things match up. In this case, I
> would want to check that the driver requires changes to support this
> device, given the commit message mentions nothing about the difference
> between this device and others. I'd still probably request that the
> commit message be improved to explain the lack of a fallback, but at
> least I would be clear about what I want and could provide a conditional
> Ack.
> 
> If you're not sending the bindings patch with the driver, there's an
> extra onus on you to explain exactly what makes this device incompatible
> with the other devices in the enum, although in an ideal world it'd make
> no difference and every bindings patch would contain that information.

I understand, thanks guys for discussing this with me.

I'll send V2 with Linux driver part.


>>>
>>>> Also, what makes this incompatibly different with the other devices in
>>>> the binding, like the 8183?
>>>
>>> It can control 8 PWMs unlike any other SoC block except for MT2712.
>>> It uses different registers than MT2712 thought.
> 
> Put this information in your commit message next time :)
> 
> Cheers,
> Conor.