RE: [PATCH v7 5/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo ssphy

["Pritam Manohar Sutar" <pritam.sutar@samsung.com>]

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 29 August 2025 04:56 PM
> To: Alim Akhtar <alim.akhtar@samsung.com>; 'Pritam Manohar Sutar'
> <pritam.sutar@samsung.com>
> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; andre.draszik@linaro.org;
> peter.griffin@linaro.org; kauschluss@disroot.org;
> ivo.ivanov.ivanov1@gmail.com; igor.belwon@mentallysanemainliners.org;
> johan@kernel.org; m.szyprowski@samsung.com; s.nawrocki@samsung.com;
> linux-phy@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; rosa.pila@samsung.com; dev.tailor@samsung.com;
> faraz.ata@samsung.com; muhammed.ali@samsung.com;
> selvarasu.g@samsung.com
> Subject: Re: [PATCH v7 5/6] dt-bindings: phy: samsung,usb3-drd-phy: add
> ExynosAutov920 combo ssphy
> 
> On 29/08/2025 12:58, Alim Akhtar wrote:
> > Hi Krzysztof
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: Friday, August 29, 2025 4:07 PM
> >> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> >> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> >> krzk+dt@kernel.org; conor+dt@kernel.org; alim.akhtar@samsung.com;
> >> andre.draszik@linaro.org; peter.griffin@linaro.org;
> >> kauschluss@disroot.org; ivo.ivanov.ivanov1@gmail.com;
> >> igor.belwon@mentallysanemainliners.org;
> >> johan@kernel.org; m.szyprowski@samsung.com; s.nawrocki@samsung.com;
> >> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; linux-arm- kernel@lists.infradead.org;
> >> linux-samsung-soc@vger.kernel.org;
> >> rosa.pila@samsung.com; dev.tailor@samsung.com;
> faraz.ata@samsung.com;
> >> muhammed.ali@samsung.com; selvarasu.g@samsung.com
> >> Subject: Re: [PATCH v7 5/6] dt-bindings: phy: samsung,usb3-drd-phy:
> >> add
> >> ExynosAutov920 combo ssphy
> >>
> >> On 29/08/2025 12:15, Pritam Manohar Sutar wrote:
> >>> Hi Krzysztof
> >>>
> >>>> -----Original Message-----
> >>>> From: Krzysztof Kozlowski <krzk@kernel.org>
> >>>> Sent: 26 August 2025 02:05 PM
> >>>> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>
> >>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> >>> .
> >>> .
> >>> [snip]
> >>> .
> >>> .
> >>>>>> Subject: Re: [PATCH v7 5/6] dt-bindings: phy: samsung,usb3-drd-phy:
> >>>>>> add
> >>>>>> ExynosAutov920 combo ssphy
> >>>>>>
> >>>>>> On Fri, Aug 22, 2025 at 03:08:44PM +0530, Pritam Manohar Sutar wrote:
> >>>>>>> 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).
> >>>>> 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).
> >>>>>
> >>>>> 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?
> > 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
> 

Thank you for the pointers, will refer the examples and 
update the commit messages accordingly. 

> and here another anti-pattern:
> 23f793850e9ee7390584c0809f085d6c88de7d3f
> 
> (and before you ask why above carries my Rb tag, then note that
> 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).
> 
> Best regards,
> Krzysztof

Thank you.

Regards,
Pritam