Re: [PATCH v1 2/2] [Draft] dt-bindings: arm: rockchip: Add Firefly ITX-3588J board

[Krzysztof Kozlowski <krzk@kernel.org>]

On 13/12/2024 16:02, Shimrra Shai wrote:
> On 2024-12-13, Krzysztof Kozlowski wrote:
> 
>> Explain why this is draft, what does it even mean. Do you expect any
>> review or not?
> 
> Correct. As I pointed out, not 100% of things work.
> 
>> Please run scripts/checkpatch.pl and fix reported warnings. Then please
>> run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
>> Some warnings can be ignored, especially from --strict run, but the code
>> here looks like it needs a fix. Feel free to get in touch if the warning
>> is not clear.
> 
> I did this, but I do not see any warnings beyond
> 
> "Prefer a maximum 75 chars per line (possible unwrapped commit
> description?)"
> 
> for the 0th patch, which does not seem to be from the description and
> 
> "Missing commit description - Add an appropriate one"
> 
> for the others, and
> 
> "added, moved or deleted file(s), does MAINTAINERS need updating?"
> 
> on the 1st.
> 
> There don't seem to be any substantial errors indicated with the code
> itself. What issues did you find, as you said it "looks like it needs a

""Missing commit description - Add an appropriate one"" is a substantial
one - clearly we cannot take empty commits.

> fix"? Nonetheless I wasn't planning on this one being a final submit
> anyway, since as I said it was a draft because there were things not
> working yet. But if there are other problems with it, I need to know what
> they are esp. given as I said those tools have not indicated more problems
> than those and they seem to do more with not adding further info to the
> emails than the code itself, yet you say the actual code needs a fix.
> 
>> Please use scripts/get_maintainers.pl to get a list of necessary people
>> and lists to CC. It might happen, that command when run on an older
>> kernel, gives you outdated entries. Therefore please be sure you base
>> your patches on recent Linux kernel.
> 
> Thanks for all this part. When you say this though:
> 
>> You missed at least devicetree list (maybe more), so this won't be
>> tested by automated tooling. Performing review on untested code might be
>> a waste of time.
> 
> what do you mean by "device tree list"? I was not aware of this part of

I mean exactly what is written. Use the tools and the tools will do the job.

> the kernel source code. I modeled this submission off of others I've seen

This does not work like this. Use the tools, not other people's
incorrect CC list.

> here and I have only seen them submit the .dts, Makefile entry, and .yaml
> entry in rockchip.yaml. I have not seen a "device tree list" different from
> those. E.g. for this submission for the Orange Pi 5,
> 
> https://lore.kernel.org/linux-rockchip/20241111045408.1922-1-honyuenkwun@gmail.com/
> 
> those are the only items touched that I can see unless I missed something

Cc list is entirely different... Did you really read my message? I state
you Cc wrong addresses and you claim that above link has the same as
yours, which is obviously not correct. So two things - my earlier
message and above link - are kind of proofs. What else do you need?

I gave you instruction which tools to use, so I do not understand why do
you insist on not using them.



Best regards,
Krzysztof

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip