Hi Krzysztof, > -----Original Message----- > From: Krzysztof Kozlowski <krzk@xxxxxxxxxx> > Sent: 29 August 2025 04:56 PM > To: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>; 'Pritam Manohar Sutar' > <pritam.sutar@xxxxxxxxxxx> > Cc: vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; robh@xxxxxxxxxx; > krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; andre.draszik@xxxxxxxxxx; > peter.griffin@xxxxxxxxxx; kauschluss@xxxxxxxxxxx; > ivo.ivanov.ivanov1@xxxxxxxxx; igor.belwon@xxxxxxxxxxxxxxxxxxxxxxxxxx; > johan@xxxxxxxxxx; m.szyprowski@xxxxxxxxxxx; s.nawrocki@xxxxxxxxxxx; > linux-phy@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-samsung- > soc@xxxxxxxxxxxxxxx; rosa.pila@xxxxxxxxxxx; dev.tailor@xxxxxxxxxxx; > faraz.ata@xxxxxxxxxxx; muhammed.ali@xxxxxxxxxxx; > selvarasu.g@xxxxxxxxxxx > 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@xxxxxxxxxx> > >> Sent: Friday, August 29, 2025 4:07 PM > >> To: Pritam Manohar Sutar <pritam.sutar@xxxxxxxxxxx> > >> Cc: vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; robh@xxxxxxxxxx; > >> krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; alim.akhtar@xxxxxxxxxxx; > >> andre.draszik@xxxxxxxxxx; peter.griffin@xxxxxxxxxx; > >> kauschluss@xxxxxxxxxxx; ivo.ivanov.ivanov1@xxxxxxxxx; > >> igor.belwon@xxxxxxxxxxxxxxxxxxxxxxxxxx; > >> johan@xxxxxxxxxx; m.szyprowski@xxxxxxxxxxx; s.nawrocki@xxxxxxxxxxx; > >> linux-phy@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > >> linux-kernel@xxxxxxxxxxxxxxx; linux-arm- kernel@xxxxxxxxxxxxxxxxxxx; > >> linux-samsung-soc@xxxxxxxxxxxxxxx; > >> rosa.pila@xxxxxxxxxxx; dev.tailor@xxxxxxxxxxx; > faraz.ata@xxxxxxxxxxx; > >> muhammed.ali@xxxxxxxxxxx; selvarasu.g@xxxxxxxxxxx > >> 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@xxxxxxxxxx> > >>>> Sent: 26 August 2025 02:05 PM > >>>> To: Pritam Manohar Sutar <pritam.sutar@xxxxxxxxxxx> > >>>> Cc: vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; robh@xxxxxxxxxx; > >>> . > >>> . > >>> [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
