Hi Krzysztof

-----Original Message-----
From: Krzysztof Kozlowski <krzk@kernel.org>
Sent: Friday, August 29, 2025 4:56 PM
To: Alim Akhtar <alim.akhtar@samsung.com>; 'Pritam Manohar Sutar'
[off-list ref]
Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
krzk+dt@kernel.org; conor+dt@kernel.org; andre.draszik@linaro.org;

[Snip]

quoted

quoted

quoted

quoted

quoted

quoted

On Fri, Aug 22, 2025 at 03:08:44PM +0530, Pritam Manohar Sutar

wrote:

quoted

quoted

quoted

quoted

quoted

quoted

quoted

This phy supports USB3.1 SSP+(10Gbps) protocol and is backwards
compatible to the USB3.0 SS(5Gbps). It requires two clocks,
named "phy" and "ref". The required supplies for USB3.1 are
named as vdd075_usb30(0.75v), vdd18_usb30(1.8v).

Please do not describe the schema, but hardware. This sentence
does not help me in my question further.

This is a combo phy having Synopsys usb20 and usb30 phys (these 2
phys are

totally different).

quoted

One PHY only supports usb2.0 and data rates whereas another one
does
usb3.1 ssp+ and usb3.1 ssp

This patch only explains about usb30 (since these are two
different
phys) phy

and omitted inclusion of usb20 reference (added separate patch for
this patch no 3).

quoted

Hope this is clear.

No. That sentence still explains what schema is doing.

Ok, let me simplify the commit message further something like below.
Anyways, the coverletter contains more details about it.

"dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo
ssphy

  Add schema for combo ssphy found on this SoC.
"

Please confirm if this looks fine?
If so, will reflect the similar commit messages in patch 1 and 3.

Please read my first comment again. I do not see how does this
satisfy hardware explanation.

Just went through the conversation above, until what extent hardware
description need to be explain in the commit?
Do we have any guideline for the same?
Could you please help with an example from previous any commit or some

other patches?

quoted

I understand that mentioning, “two clocks, two supplies etc" are part
of schema, one may or may not capture that in the commit.
However mentioning, “this hardware (SoC) contain a combo PHY which

supports usb3.1 and usb3.0" is not ok?


Maybe that's just language, but to me the commit msg did not describe
hardware after first sentence, but said what schema requires (some
clocks and supplies). Other examples:
00399bbe02d2bb6fd8d6eb90573ec305616449f4
e4c9a7b475e5d0d9b2440ee48f91d1364eabd6cb

Thanks for suggestion as always, I hope the author can follow this. 

and here another anti-pattern:
23f793850e9ee7390584c0809f085d6c88de7d3f

(and before you ask why above carries my Rb tag, then note that

I will not ask :-)

Samsung's revenue is around 220 billion USD, so for sure it has a lot,
really a lot of resources to review patches internally and improve their
quality before posting).

Hmm..., however let me check internally.

Best regards,
Krzysztof