Re: [PATCH v2 5/7] dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/10/24 14:36, Krzysztof Kozlowski wrote:
On 10/04/2024 18:29, mr.nuke.me@xxxxxxxxx wrote:


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@xxxxxxxxx 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.

No, the dt_binding_check fails. You don't need to go to dtbs_check even,
because the binding already has a failure.


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.

I don't feel insulted but I feel my time is wasted if I tell you to test
your binding and you immediately within few minutes respond "I tested",
but then:
1. Bot confirms it was not tested,
2. I apply your patch and test it and see the failure.

Thank you for double checking, and I am sorry this escalated in this manner. I believed you the first time that something is wrong, and I had a hard time figuring out what.

I am now able to repro the errors, and the below changes appear to work. Is that sufficient?

   clocks:
     minItems: 3
     maxItems: 5

   clock-names:
     minItems: 3
     items:
       - ... (5 clock names here)

For ipq6018/ipq8074:

      properties:
        clocks:
          maxItems: 3
        clock-names:
          maxItems: 3

For ipq9574:

      properties:
        clocks:
          minItems: 5
        clock-names:
          minItems: 5






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux