Hi Krzysztof, > -----Original Message----- > From: Krzysztof Kozlowski <krzk@xxxxxxxxxx> > Sent: 12 July 2025 01:44 PM > To: Pritam Manohar Sutar <pritam.sutar@xxxxxxxxxxx>; 'Krzysztof Kozlowski' > <krzysztof.kozlowski@xxxxxxxxxx> > Cc: vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; robh@xxxxxxxxxx; > krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; alim.akhtar@xxxxxxxxxxx; > andre.draszik@xxxxxxxxxx; peter.griffin@xxxxxxxxxx; neil.armstrong@xxxxxxxxxx; > kauschluss@xxxxxxxxxxx; ivo.ivanov.ivanov1@xxxxxxxxx; > 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 v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: add > ExynosAutov920 HS phy compatible > > On 09/07/2025 10:46, Pritam Manohar Sutar wrote: > > Hi Krzysztof, > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > >> Sent: 06 July 2025 03:11 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; > >> neil.armstrong@xxxxxxxxxx; kauschluss@xxxxxxxxxxx; > >> ivo.ivanov.ivanov1@xxxxxxxxx; 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 v4 1/6] dt-bindings: phy: samsung,usb3-drd-phy: > >> add > >> ExynosAutov920 HS phy compatible > >> > >> On Tue, Jul 01, 2025 at 05:37:01PM +0530, Pritam Manohar Sutar wrote: > >>> Add a dedicated compatible string for USB HS phy found in this SoC. > >>> The SoC requires two clocks, named "phy" and "ref" (same as clocks > >>> required by Exynos850). > >>> > >>> It also requires various power supplies (regulators) for the > >>> internal circuitry to work. The required voltages are: > >>> * avdd075_usb - 0.75v > >>> * avdd18_usb20 - 1.8v > >>> * avdd33_usb20 - 3.3v > >>> > >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > >> > >> No, really. Look: > >> > >>> Signed-off-by: Pritam Manohar Sutar <pritam.sutar@xxxxxxxxxxx> > >>> --- > >>> .../bindings/phy/samsung,usb3-drd-phy.yaml | 37 +++++++++++++++++++ > >>> 1 file changed, 37 insertions(+) > >>> > >>> diff --git > >>> a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > >>> b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > >>> index e906403208c0..2e29ff749bba 100644 > >>> --- > >>> a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > >>> +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yam > >>> +++ l > >>> @@ -34,6 +34,7 @@ properties: > >>> - samsung,exynos7870-usbdrd-phy > >>> - samsung,exynos850-usbdrd-phy > >>> - samsung,exynos990-usbdrd-phy > >>> + - samsung,exynosautov920-usbdrd-phy > >>> > >>> clocks: > >>> minItems: 1 > >>> @@ -110,6 +111,15 @@ properties: > >>> vddh-usbdp-supply: > >>> description: VDDh power supply for the USB DP phy. > >>> > >>> + avdd075_usb-supply: > >>> + description: 0.75V power supply for USB phy > >>> + > >>> + avdd18_usb20-supply: > >>> + description: 1.8V power supply for USB phy > >>> + > >>> + avdd33_usb20-supply: > >>> + description: 3.3V power supply for USB phy > >>> + > >> > >> None of these were here. Follow DTS coding style... but why are you > >> adding completely new supplies? > > > > Digital supplies were here. Need Analog supplies for exynosautov920, > > hence added new Analog supplies; 'a'vdd075_usb, 'a'vdd18_usb20, > > 'a'vdd33_usb20 > > > > Will add "full stops" for DTS coding style in description. > > You cannot grow one line change into 50 line change and retain the review. Yes agreed. Will remove "Reviewed-by" tag and requesting you to review the patches again and provide your valuable comments. > > > >> > >> > >>> required: > >>> - compatible > >>> - clocks > >>> @@ -235,6 +245,33 @@ allOf: > >>> > >>> reg-names: > >>> maxItems: 1 > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + contains: > >>> + enum: > >>> + - samsung,exynosautov920-usbdrd-phy > >>> + then: > >>> + properties: > >>> + clocks: > >>> + minItems: 2 > >>> + maxItems: 2 > >>> + > >>> + clock-names: > >>> + items: > >>> + - const: phy > >>> + - const: ref > >>> + > >>> + reg: > >>> + maxItems: 1 > >>> + > >>> + reg-names: > >>> + maxItems: 1 > >>> + > >>> + required: > >>> + - avdd075_usb-supply > >>> + - avdd18_usb20-supply > >>> + - avdd33_usb20-supply > >> > >> Neither was this entire diff hunk here. > >> > >> This was part of other block for a reason. > > > > Added regulators in driver. This block is added for regulators (other > > block does not have "required" field for power supplies) if excluded > > this block, "make ARCH=arm64 dtbs_check > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/samsung,usb3- > drd > > -phy.yaml" will fail as mentioned below > > > Nothing is explained in changelog/cover letter. You claim you only added Rb tag. > This is an entirely silent change while keeping the review. Will add more explanations in cover letter/changelog why this block is added. > Combined with not even following DTS style! Ok got it. Will change supplies name as below avdd075_usb => avdd075-usb avdd18_usb20 => avdd18-usb20 avdd33_usb20 => avdd33-usb20 Confirm the above change that is meant in terms of DTS style. > > It's not acceptable. > > Best regards, > Krzysztof Thank you. Regards, Pritam
