All of lore.kernel.org
 help / color / mirror / Atom feed
From: mr.nuke.me@gmail.com
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>
Cc: linux-arm-msm@vger.kernel.org, linux-phy@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 5/7] dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY
Date: Wed, 10 Apr 2024 11:29:28 -0500	[thread overview]
Message-ID: <d827ec3c-84fd-9352-b321-79bdc4bdcd40@gmail.com> (raw)
In-Reply-To: <a23adb9c-6377-467b-ac3c-0ec51fc97253@linaro.org>



On 4/10/24 02:02, Krzysztof Kozlowski wrote:
> On 10/04/2024 08:59, Krzysztof Kozlowski wrote:
>> On 09/04/2024 22:19, mr.nuke.me@gmail.com wrote:
>>>>
>>>>
>>>>>    
>>>>>      clock-names:
>>>>>        items:
>>>>>          - const: aux
>>>>>          - const: cfg_ahb
>>>>>          - const: pipe
>>>>> +      - const: anoc
>>>>> +      - const: snoc
>>>>
>>>> OK, you did not test it. Neither this, nor DTS. I stop review, please
>>>> test first.
>>>
>>> I ran both `checkpatch.pl` and `make dt_binding_check`. What in this
>>> patch makes you say I "did not test it", and what test or tests did I miss?
>>>
>>
>> ... and no, you did not. If you tested, you would easily see error:
>> 	clock-names: ['aux', 'cfg_ahb', 'pipe'] is too short
>>
>> When you receive comment from reviewer, please investigate thoroughly
>> what could get wrong. Don't answer just to get rid of reviewer. It's
>> fine to make mistakes, but if reviewer points to issue and you
>> immediately respond "no issue", that's waste of my time.
> 
> To clarify: "no issue" response is waste of my time. If you responded
> "oh, I see the error, but I don't know how to fix it", it would be ok, I
> can clarify and help in this.

I apologize if I gave you this impression. I tried to follow the testing 
process, it did not turn out as expected. Obviously, I missed something. 
I tried to ask what I missed, and in order for that question to make 
sense, I need to describe what I tried.

It turns out what I missed was "make check_dtbs". I only found that out 
after an automated email from Rob describing some troubleshooting steps.

If I may have a few sentences to rant, I see the dt-schema as a hurdle 
to making an otherwise useful change. I am told I can ask for help when 
I get stuck, yet I manage to insult the maintainer by aking for help. I 
find this very intimidating.

Alex

WARNING: multiple messages have this Message-ID (diff)
From: mr.nuke.me@gmail.com
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>
Cc: linux-arm-msm@vger.kernel.org, linux-phy@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 5/7] dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY
Date: Wed, 10 Apr 2024 11:29:28 -0500	[thread overview]
Message-ID: <d827ec3c-84fd-9352-b321-79bdc4bdcd40@gmail.com> (raw)
In-Reply-To: <a23adb9c-6377-467b-ac3c-0ec51fc97253@linaro.org>



On 4/10/24 02:02, Krzysztof Kozlowski wrote:
> On 10/04/2024 08:59, Krzysztof Kozlowski wrote:
>> On 09/04/2024 22:19, mr.nuke.me@gmail.com wrote:
>>>>
>>>>
>>>>>    
>>>>>      clock-names:
>>>>>        items:
>>>>>          - const: aux
>>>>>          - const: cfg_ahb
>>>>>          - const: pipe
>>>>> +      - const: anoc
>>>>> +      - const: snoc
>>>>
>>>> OK, you did not test it. Neither this, nor DTS. I stop review, please
>>>> test first.
>>>
>>> I ran both `checkpatch.pl` and `make dt_binding_check`. What in this
>>> patch makes you say I "did not test it", and what test or tests did I miss?
>>>
>>
>> ... and no, you did not. If you tested, you would easily see error:
>> 	clock-names: ['aux', 'cfg_ahb', 'pipe'] is too short
>>
>> When you receive comment from reviewer, please investigate thoroughly
>> what could get wrong. Don't answer just to get rid of reviewer. It's
>> fine to make mistakes, but if reviewer points to issue and you
>> immediately respond "no issue", that's waste of my time.
> 
> To clarify: "no issue" response is waste of my time. If you responded
> "oh, I see the error, but I don't know how to fix it", it would be ok, I
> can clarify and help in this.

I apologize if I gave you this impression. I tried to follow the testing 
process, it did not turn out as expected. Obviously, I missed something. 
I tried to ask what I missed, and in order for that question to make 
sense, I need to describe what I tried.

It turns out what I missed was "make check_dtbs". I only found that out 
after an automated email from Rob describing some troubleshooting steps.

If I may have a few sentences to rant, I see the dt-schema as a hurdle 
to making an otherwise useful change. I am told I can ask for help when 
I get stuck, yet I manage to insult the maintainer by aking for help. I 
find this very intimidating.

Alex

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

  reply	other threads:[~2024-04-10 16:29 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20240409190833.3485824-1-mr.nuke.me@gmail.com>
2024-04-09 19:08 ` [PATCH v2 1/7] dt-bindings: clock: Add PCIe pipe related clocks for IPQ9574 Alexandru Gagniuc
2024-04-09 19:08 ` [PATCH v2 2/7] clk: qcom: gcc-ipq9574: Add PCIe pipe clocks Alexandru Gagniuc
2024-04-09 19:08 ` [PATCH v2 3/7] dt-bindings: PCI: qcom: Add IPQ9574 PCIe controller Alexandru Gagniuc
2024-04-09 20:08   ` Krzysztof Kozlowski
2024-04-11 18:00     ` mr.nuke.me
2024-04-11 19:09       ` Krzysztof Kozlowski
2024-04-09 19:08 ` [PATCH v2 4/7] PCI: qcom: Add support for IPQ9574 Alexandru Gagniuc
2024-04-10 11:50   ` Konrad Dybcio
2024-04-17  7:05     ` Manivannan Sadhasivam
2024-04-09 19:08 ` [PATCH v2 5/7] dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY Alexandru Gagniuc
2024-04-09 19:08   ` Alexandru Gagniuc
2024-04-09 20:09   ` Krzysztof Kozlowski
2024-04-09 20:09     ` Krzysztof Kozlowski
2024-04-09 20:19     ` mr.nuke.me
2024-04-09 20:19       ` mr.nuke.me
2024-04-09 20:28       ` Krzysztof Kozlowski
2024-04-09 20:28         ` Krzysztof Kozlowski
2024-04-10  6:59       ` Krzysztof Kozlowski
2024-04-10  6:59         ` Krzysztof Kozlowski
2024-04-10  7:02         ` Krzysztof Kozlowski
2024-04-10  7:02           ` Krzysztof Kozlowski
2024-04-10 16:29           ` mr.nuke.me [this message]
2024-04-10 16:29             ` mr.nuke.me
2024-04-10 19:36             ` Krzysztof Kozlowski
2024-04-10 19:36               ` Krzysztof Kozlowski
2024-04-11 17:24               ` mr.nuke.me
2024-04-11 17:24                 ` mr.nuke.me
2024-04-11 19:08                 ` Krzysztof Kozlowski
2024-04-11 19:08                   ` Krzysztof Kozlowski
2024-04-09 20:49   ` Rob Herring
2024-04-09 20:49     ` Rob Herring
2024-04-09 19:08 ` [PATCH v2 6/7] phy: qcom-qmp-pcie: add support for " Alexandru Gagniuc
2024-04-09 19:08   ` Alexandru Gagniuc
2024-04-09 19:08 ` [PATCH v2 7/7] arm64: dts: qcom: ipq9574: add PCIe2 nodes Alexandru Gagniuc

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=d827ec3c-84fd-9352-b321-79bdc4bdcd40@gmail.com \
    --to=mr.nuke.me@gmail.com \
    --cc=andersson@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kishon@kernel.org \
    --cc=konrad.dybcio@linaro.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=robh@kernel.org \
    --cc=vkoul@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.