| Message ID | 20250516102650.2144487-2-pritam.sutar@samsung.com |
|---|---|
| State | New |
| Headers | show |
| Series | initial usbdrd phy support for Exynosautov920 soc | expand |
On 16/05/2025 12:26, Pritam Manohar Sutar wrote: > Add a dedicated compatible for USB phy found in this SoC > > Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com> > --- > .../bindings/phy/samsung,usb3-drd-phy.yaml | 27 +++++++++++++++++++ > 1 file changed, 27 insertions(+) A nit, subject: drop second/last, redundant "dt-schema for". The "dt-bindings" prefix is already stating that these are bindings in dtschema format. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > diff --git a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > index fdddddc7d611..c50f4218ded9 100644 > --- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > @@ -32,6 +32,7 @@ properties: > - samsung,exynos7-usbdrd-phy > - samsung,exynos7870-usbdrd-phy > - samsung,exynos850-usbdrd-phy > + - samsung,exynosautov920-usb31drd-phy > > clocks: > minItems: 2 > @@ -204,6 +205,32 @@ allOf: > reg-names: > maxItems: 1 > > + - if: > + properties: > + compatible: > + contains: > + const: samsung,exynosautov920-usb31drd-phy > + then: > + $ref: /schemas/usb/usb-switch.yaml# > + > + properties: > + clocks: > + items: Why there is no main PHY clock? > + - description: ext_xtal clock > + - description: reference clock Both external oscillator and reference clocks? What are these clocks? > + > + clock-names: > + items: > + - const: ext_xtal > + - const: ref > + > + reg: > + minItems: 1 No, there is no such syntax. Drop. > + maxItems: 1 > + > + reg-names: > + minItems: 1 No, look at existing code and do the same. > + > unevaluatedProperties: false > > examples: Best regards, Krzysztof
Hi Krzysztof, Thank you for the reviewing the patches. > -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: 20 May 2025 01:15 PM > To: Pritam Manohar Sutar <pritam.sutar@samsung.com>; 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; > m.szyprowski@samsung.com; s.nawrocki@samsung.com > Cc: 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 v2 1/2] dt-bindings: phy: samsung,usb3-drd-phy: add dt- > schema for ExynosAutov920 > > On 16/05/2025 12:26, Pritam Manohar Sutar wrote: > > Add a dedicated compatible for USB phy found in this SoC > > > > Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com> > > --- > > .../bindings/phy/samsung,usb3-drd-phy.yaml | 27 +++++++++++++++++++ > > 1 file changed, 27 insertions(+) > > A nit, subject: drop second/last, redundant "dt-schema for". The "dt-bindings" > prefix is already stating that these are bindings in dtschema format. > See also: > https://protect2.fireeye.com/v1/url?k=e78ee20f-b815db03-e78f6940- > 000babff3563-8256a76d4c2ebd67&q=1&e=1643274a-07fa-4563-af27- > b2f2eff30417&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv6.7- > rc8%2Fsource%2FDocumentation%2Fdevicetree%2Fbindings%2Fsubmitting- > patches.rst%23L18 > Will update the commit title as below "dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 compatible" > > > > diff --git > > a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > > b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > > index fdddddc7d611..c50f4218ded9 100644 > > --- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > > +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > > @@ -32,6 +32,7 @@ properties: > > - samsung,exynos7-usbdrd-phy > > - samsung,exynos7870-usbdrd-phy > > - samsung,exynos850-usbdrd-phy > > + - samsung,exynosautov920-usb31drd-phy > > > > clocks: > > minItems: 2 > > @@ -204,6 +205,32 @@ allOf: > > reg-names: > > maxItems: 1 > > > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: samsung,exynosautov920-usb31drd-phy > > + then: > > + $ref: /schemas/usb/usb-switch.yaml# > > + > > + properties: > > + clocks: > > + items: > > Why there is no main PHY clock? external crystal clk (ext_xtal) is used as main phy clk. > > > + - description: ext_xtal clock > > + - description: reference clock > > Both external oscillator and reference clocks? What are these clocks? ext_xtal is used as PHY clock to access register and reference clock for PHY operations. Will add more description in patch. > > > + > > + clock-names: > > + items: > > + - const: ext_xtal > > + - const: ref > > + > > + reg: > > + minItems: 1 > > No, there is no such syntax. Drop. Will remove this > > > + maxItems: 1 > > + > > + reg-names: > > + minItems: 1 > > No, look at existing code and do the same. Will replace "minItems" by "maxItems" > > > + > > unevaluatedProperties: false > > > > examples: > > > Best regards, > Krzysztof Thank you.
On 21/05/2025 08:48, Pritam Manohar Sutar wrote: >>> diff --git >>> a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml >>> b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml >>> index fdddddc7d611..c50f4218ded9 100644 >>> --- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml >>> +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml >>> @@ -32,6 +32,7 @@ properties: >>> - samsung,exynos7-usbdrd-phy >>> - samsung,exynos7870-usbdrd-phy >>> - samsung,exynos850-usbdrd-phy >>> + - samsung,exynosautov920-usb31drd-phy >>> >>> clocks: >>> minItems: 2 >>> @@ -204,6 +205,32 @@ allOf: >>> reg-names: >>> maxItems: 1 >>> >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + const: samsung,exynosautov920-usb31drd-phy >>> + then: >>> + $ref: /schemas/usb/usb-switch.yaml# >>> + >>> + properties: >>> + clocks: >>> + items: >> >> Why there is no main PHY clock? > > external crystal clk (ext_xtal) is used as main phy clk. So this is the main phy clock? This describes the clock input, not what you have on your board. If you change external crystal to internal clock in one design, you change the binding? No, this makes no sense. Best regards, Krzysztof
Hi Krzysztof, > -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: 21 May 2025 02:15 PM > To: Pritam Manohar Sutar <pritam.sutar@samsung.com>; 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; > m.szyprowski@samsung.com; s.nawrocki@samsung.com > Cc: 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 v2 1/2] dt-bindings: phy: samsung,usb3-drd-phy: add dt- > schema for ExynosAutov920 > > On 21/05/2025 08:48, Pritam Manohar Sutar wrote: > >>> diff --git > >>> a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > >>> b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > >>> index fdddddc7d611..c50f4218ded9 100644 > >>> --- > >>> a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml > >>> +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yam > >>> +++ l > >>> @@ -32,6 +32,7 @@ properties: > >>> - samsung,exynos7-usbdrd-phy > >>> - samsung,exynos7870-usbdrd-phy > >>> - samsung,exynos850-usbdrd-phy > >>> + - samsung,exynosautov920-usb31drd-phy > >>> > >>> clocks: > >>> minItems: 2 > >>> @@ -204,6 +205,32 @@ allOf: > >>> reg-names: > >>> maxItems: 1 > >>> > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + contains: > >>> + const: samsung,exynosautov920-usb31drd-phy > >>> + then: > >>> + $ref: /schemas/usb/usb-switch.yaml# > >>> + > >>> + properties: > >>> + clocks: > >>> + items: > >> > >> Why there is no main PHY clock? > > > > external crystal clk (ext_xtal) is used as main phy clk. > > So this is the main phy clock? This describes the clock input, not what you have > on your board. If you change external crystal to internal clock in one design, you > change the binding? No, this makes no sense. > > Yes, this is the main phy clock. Will revisit the binding and try reusing existing one. > > Best regards, > Krzysztof Thank you, Pritam
diff --git a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml index fdddddc7d611..c50f4218ded9 100644 --- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml @@ -32,6 +32,7 @@ properties: - samsung,exynos7-usbdrd-phy - samsung,exynos7870-usbdrd-phy - samsung,exynos850-usbdrd-phy + - samsung,exynosautov920-usb31drd-phy clocks: minItems: 2 @@ -204,6 +205,32 @@ allOf: reg-names: maxItems: 1 + - if: + properties: + compatible: + contains: + const: samsung,exynosautov920-usb31drd-phy + then: + $ref: /schemas/usb/usb-switch.yaml# + + properties: + clocks: + items: + - description: ext_xtal clock + - description: reference clock + + clock-names: + items: + - const: ext_xtal + - const: ref + + reg: + minItems: 1 + maxItems: 1 + + reg-names: + minItems: 1 + unevaluatedProperties: false examples:
Add a dedicated compatible for USB phy found in this SoC Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com> --- .../bindings/phy/samsung,usb3-drd-phy.yaml | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+)