devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Florian Fainelli <florian.fainelli@broadcom.com>,
	Markus Mayer <mmayer@broadcom.com>,
	Rob Herring <robh+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>
Cc: Linux ARM Kernel List <linux-arm-kernel@lists.infradead.org>,
	Device Tree Mailing List <devicetree@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/4] memory: brcmstb_dpfe: introduce version-specific compatible strings
Date: Wed, 6 Dec 2023 18:33:53 +0100	[thread overview]
Message-ID: <7d30acd6-1f00-47d1-b7ed-05e7bdab119c@linaro.org> (raw)
In-Reply-To: <cb1d5118-b4f6-46b3-814b-2edf16372d01@broadcom.com>

On 06/12/2023 17:19, Florian Fainelli wrote:
> 
> 
> On 12/6/2023 3:09 AM, Krzysztof Kozlowski wrote:
>> On 05/12/2023 19:47, Markus Mayer wrote:
>>> Introduce compatible strings brcm,dpfe-cpu-v1 through brcm,dpfe-cpu-v3
>>> to the Broadcom DPFE driver.
>>
>> No, why?
>>
>>>
>>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>>> ---
>>>   drivers/memory/brcmstb_dpfe.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/memory/brcmstb_dpfe.c b/drivers/memory/brcmstb_dpfe.c
>>> index a7ab3d377206..66876b409e59 100644
>>> --- a/drivers/memory/brcmstb_dpfe.c
>>> +++ b/drivers/memory/brcmstb_dpfe.c
>>> @@ -924,6 +924,12 @@ static const struct of_device_id brcmstb_dpfe_of_match[] = {
>>>   	{ .compatible = "brcm,bcm7271-dpfe-cpu", .data = &dpfe_api_old_v2 },
>>>   	{ .compatible = "brcm,bcm7278-dpfe-cpu", .data = &dpfe_api_old_v2 },
>>>   	{ .compatible = "brcm,bcm7211-dpfe-cpu", .data = &dpfe_api_new_v2 },
>>> +
>>> +	/* Match specific DCPU versions */
>>> +	{ .compatible = "brcm,dpfe-cpu-v1", .data = &dpfe_api_old_v2 },
>>> +	{ .compatible = "brcm,dpfe-cpu-v2", .data = &dpfe_api_new_v2 },
>>> +	{ .compatible = "brcm,dpfe-cpu-v3", .data = &dpfe_api_v3 },
>>
>> Pointless change.
> 
> Is it possible to ask you as a maintainer to stop having those knee jerk 
> reactions and try to understand things a bit better, or simply request a 
> better explanation from the submitter?

I asked: "Why?". None of the commits explain the rationale behind the
change. None of them say why such change is needed. They all repeat what
the patch is doing, which is pretty easy to see from the diff. The
commit must answer the trickiest question: why are we doing this?

Best regards,
Krzysztof


  reply	other threads:[~2023-12-06 17:33 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-05 18:47 [PATCH 0/4] memory: brcmstb_dpfe: support DPFE API v4 Markus Mayer
2023-12-05 18:47 ` [PATCH 1/4] dt-bindings: memory: additional compatible strings for Broadcom DPFE Markus Mayer
2023-12-06 11:09   ` Krzysztof Kozlowski
2023-12-06 16:32     ` Florian Fainelli
2023-12-06 17:29       ` Krzysztof Kozlowski
2023-12-06 17:36         ` Florian Fainelli
2023-12-06 17:42           ` Krzysztof Kozlowski
2023-12-05 18:47 ` [PATCH 2/4] memory: brcmstb_dpfe: introduce version-specific compatible strings Markus Mayer
2023-12-05 19:05   ` Florian Fainelli
2023-12-06 11:09   ` Krzysztof Kozlowski
2023-12-06 16:19     ` Florian Fainelli
2023-12-06 17:33       ` Krzysztof Kozlowski [this message]
2023-12-05 18:47 ` [PATCH 3/4] memory: brcmstb_dpfe: support DPFE API v4 Markus Mayer
2023-12-05 19:05   ` Florian Fainelli
2023-12-06 11:10   ` Krzysztof Kozlowski
2023-12-06 16:18     ` Florian Fainelli
2023-12-06 17:31       ` Krzysztof Kozlowski
2023-12-06 18:48         ` Markus Mayer
2023-12-05 18:47 ` [PATCH 4/4] memory: brcmstb_dpfe: introduce best-effort API detection Markus Mayer
2023-12-05 19:06   ` Florian Fainelli
2023-12-06 11:13   ` Krzysztof Kozlowski
2023-12-06 16:24     ` Florian Fainelli
2023-12-06 11:14 ` [PATCH 0/4] memory: brcmstb_dpfe: support DPFE API v4 Krzysztof Kozlowski

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=7d30acd6-1f00-47d1-b7ed-05e7bdab119c@linaro.org \
    --to=krzysztof.kozlowski@linaro.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=florian.fainelli@broadcom.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mmayer@broadcom.com \
    --cc=robh+dt@kernel.org \
    /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).