[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <000001dc1c02$cc89fda0$659df8e0$@samsung.com>
Date: Tue, 2 Sep 2025 17:42:01 +0530
From: "Pritam Manohar Sutar" <pritam.sutar@...sung.com>
To: "'Krzysztof Kozlowski'" <krzk@...nel.org>, "'Alim Akhtar'"
<alim.akhtar@...sung.com>
Cc: <vkoul@...nel.org>, <kishon@...nel.org>, <robh@...nel.org>,
<krzk+dt@...nel.org>, <conor+dt@...nel.org>, <andre.draszik@...aro.org>,
<peter.griffin@...aro.org>, <kauschluss@...root.org>,
<ivo.ivanov.ivanov1@...il.com>, <igor.belwon@...tallysanemainliners.org>,
<johan@...nel.org>, <m.szyprowski@...sung.com>, <s.nawrocki@...sung.com>,
<linux-phy@...ts.infradead.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
<linux-samsung-soc@...r.kernel.org>, <rosa.pila@...sung.com>,
<dev.tailor@...sung.com>, <faraz.ata@...sung.com>,
<muhammed.ali@...sung.com>, <selvarasu.g@...sung.com>
Subject: RE: [PATCH v7 5/6] dt-bindings: phy: samsung,usb3-drd-phy: add
ExynosAutov920 combo ssphy
Hi Krzysztof,
> -----Original Message-----
> From: Pritam Manohar Sutar <pritam.sutar@...sung.com>
> Sent: 29 August 2025 05:33 PM
> To: 'Krzysztof Kozlowski' <krzk@...nel.org>; 'Alim Akhtar'
> <alim.akhtar@...sung.com>
> Cc: 'vkoul@...nel.org' <vkoul@...nel.org>; 'kishon@...nel.org'
> <kishon@...nel.org>; 'robh@...nel.org' <robh@...nel.org>;
> 'krzk+dt@...nel.org' <krzk+dt@...nel.org>; 'conor+dt@...nel.org'
> <conor+dt@...nel.org>; 'andre.draszik@...aro.org'
> <andre.draszik@...aro.org>; 'peter.griffin@...aro.org'
> <peter.griffin@...aro.org>; 'kauschluss@...root.org' <kauschluss@...root.org>;
> 'ivo.ivanov.ivanov1@...il.com' <ivo.ivanov.ivanov1@...il.com>;
> 'igor.belwon@...tallysanemainliners.org'
> <igor.belwon@...tallysanemainliners.org>; 'johan@...nel.org'
> <johan@...nel.org>; 'm.szyprowski@...sung.com'
> <m.szyprowski@...sung.com>; 's.nawrocki@...sung.com'
> <s.nawrocki@...sung.com>; 'linux-phy@...ts.infradead.org' <linux-
> phy@...ts.infradead.org>; 'devicetree@...r.kernel.org'
> <devicetree@...r.kernel.org>; 'linux-kernel@...r.kernel.org' <linux-
> kernel@...r.kernel.org>; 'linux-arm-kernel@...ts.infradead.org' <linux-arm-
> kernel@...ts.infradead.org>; 'linux-samsung-soc@...r.kernel.org' <linux-
> samsung-soc@...r.kernel.org>; 'rosa.pila@...sung.com'
> <rosa.pila@...sung.com>; 'dev.tailor@...sung.com'
> <dev.tailor@...sung.com>; 'faraz.ata@...sung.com'
> <faraz.ata@...sung.com>; 'muhammed.ali@...sung.com'
> <muhammed.ali@...sung.com>; 'selvarasu.g@...sung.com'
> <selvarasu.g@...sung.com>
> Subject: RE: [PATCH v7 5/6] dt-bindings: phy: samsung,usb3-drd-phy: add
> ExynosAutov920 combo ssphy
>
> Hi Krzysztof,
>
> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzk@...nel.org>
> > Sent: 29 August 2025 04:56 PM
> > To: Alim Akhtar <alim.akhtar@...sung.com>; 'Pritam Manohar Sutar'
> > <pritam.sutar@...sung.com>
> > Cc: vkoul@...nel.org; kishon@...nel.org; robh@...nel.org;
> > krzk+dt@...nel.org; conor+dt@...nel.org; andre.draszik@...aro.org;
> > peter.griffin@...aro.org; kauschluss@...root.org;
> > ivo.ivanov.ivanov1@...il.com; igor.belwon@...tallysanemainliners.org;
> > johan@...nel.org; m.szyprowski@...sung.com; s.nawrocki@...sung.com;
> > linux-phy@...ts.infradead.org; devicetree@...r.kernel.org; linux-
> > kernel@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> > linux-samsung- soc@...r.kernel.org; rosa.pila@...sung.com;
> > dev.tailor@...sung.com; faraz.ata@...sung.com;
> > muhammed.ali@...sung.com; selvarasu.g@...sung.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@...nel.org>
> > >> Sent: Friday, August 29, 2025 4:07 PM
> > >> To: Pritam Manohar Sutar <pritam.sutar@...sung.com>
> > >> Cc: vkoul@...nel.org; kishon@...nel.org; robh@...nel.org;
> > >> krzk+dt@...nel.org; conor+dt@...nel.org; alim.akhtar@...sung.com;
> > >> andre.draszik@...aro.org; peter.griffin@...aro.org;
> > >> kauschluss@...root.org; ivo.ivanov.ivanov1@...il.com;
> > >> igor.belwon@...tallysanemainliners.org;
> > >> johan@...nel.org; m.szyprowski@...sung.com;
> s.nawrocki@...sung.com;
> > >> linux-phy@...ts.infradead.org; devicetree@...r.kernel.org;
> > >> linux-kernel@...r.kernel.org; linux-arm-
> > >> kernel@...ts.infradead.org; linux-samsung-soc@...r.kernel.org;
> > >> rosa.pila@...sung.com; dev.tailor@...sung.com;
> > faraz.ata@...sung.com;
> > >> muhammed.ali@...sung.com; selvarasu.g@...sung.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@...nel.org>
> > >>>> Sent: 26 August 2025 02:05 PM
> > >>>> To: Pritam Manohar Sutar <pritam.sutar@...sung.com>
> > >>>> Cc: vkoul@...nel.org; kishon@...nel.org; robh@...nel.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.
Can you please confirm, if below message looks fine?
"
dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo ssphy
The USBDRD31 5nm controller consists of Synopsys USB20 phy and
USB31 SSP+ combophy. Document support for the USB31 SSP+ phy found
on combophy of the ExynosAutov920 SoC.
"
>
> > 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
Thank you.
Regards,
Pritam
Powered by blists - more mailing lists