| Message ID | 20250416095402.90543-5-linux@fw-web.de |
|---|---|
| State | New |
| Headers | show |
| Series | Add Bananapi R4 variants and add xsphy | expand |
On Wed, Apr 16, 2025 at 11:53:56AM GMT, Frank Wunderlich wrote: > From: Frank Wunderlich <frank-w@public-files.de> > > Add support for type switch by pericfg register between USB3/PCIe. > > Signed-off-by: Frank Wunderlich <frank-w@public-files.de> > --- > .../devicetree/bindings/phy/mediatek,xsphy.yaml | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml > index 3b5253659e6f..5033d77c1239 100644 > --- a/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml > +++ b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml > @@ -151,6 +151,22 @@ patternProperties: > minimum: 1 > maximum: 31 > > + mediatek,syscon-type: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + maxItems: 1 > + description: > + A phandle to syscon used to access the register of type switch, > + the field should always be 3 cells long. > + items: > + items: Missing -, because you have one phandle. > + - description: > + The first cell represents a phandle to syscon Don't repeat constraints in free form text. "Foo bar system controller" or "Phandle to foo bar system controller" > + - description: > + The second cell represents the register offset "Baz register offset" > + - description: > + The third cell represents the index of config segment "Index of config segment", but what is index of config? Best regards, Krzysztof
Hi Krzysztof, thanks for review. basicly i used the same binding like for tphy. Am 2025-04-17 08:56, schrieb Krzysztof Kozlowski: > On Wed, Apr 16, 2025 at 11:53:56AM GMT, Frank Wunderlich wrote: >> From: Frank Wunderlich <frank-w@public-files.de> >> >> Add support for type switch by pericfg register between USB3/PCIe. >> >> Signed-off-by: Frank Wunderlich <frank-w@public-files.de> >> --- >> .../devicetree/bindings/phy/mediatek,xsphy.yaml | 16 >> ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml >> b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml >> index 3b5253659e6f..5033d77c1239 100644 >> --- a/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml >> +++ b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml >> @@ -151,6 +151,22 @@ patternProperties: >> minimum: 1 >> maximum: 31 >> >> + mediatek,syscon-type: >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + maxItems: 1 >> + description: >> + A phandle to syscon used to access the register of type >> switch, >> + the field should always be 3 cells long. >> + items: >> + items: > > Missing -, because you have one phandle. ok, then i need to drop MaxItems and indent 2 spaces more, but no problem >> + - description: >> + The first cell represents a phandle to syscon > > Don't repeat constraints in free form text. "Foo bar system controller" > or "Phandle to foo bar system controller" i would write only "phandle to system controller". on mt7988 it is the topmisc syscon, but maybe on other SoC it is different name. >> + - description: >> + The second cell represents the register offset > > "Baz register offset" same here, only "register offset". >> + - description: >> + The third cell represents the index of config segment > > "Index of config segment", but what is index of config? unfortunately we have no detailed documentation here, but based on driver (i guess daniel ported it from SDK) this value is multiplied with BITS_PER_BYTE so it can handle up to 4 config-segments in the 32bit register (maybe configuring 4 phys). But on mt7988 we use only the first config-segment (last cell=0 in dts-patch). at the end it will look like this: mediatek,syscon-type: $ref: /schemas/types.yaml#/definitions/phandle-array description: A phandle to syscon used to access the register of type switch, the field should always be 3 cells long. items: - items: - description: Phandle to system controller - description: Register offset - description: Index of config segment enum: [0, 1, 2, 3] would this be ok? > Best regards, > Krzysztof regards Frank
On 17/04/2025 09:52, Frank Wunderlich (linux) wrote: >>> >>> + mediatek,syscon-type: >>> + $ref: /schemas/types.yaml#/definitions/phandle-array >>> + maxItems: 1 >>> + description: >>> + A phandle to syscon used to access the register of type >>> switch, >>> + the field should always be 3 cells long. >>> + items: >>> + items: >> >> Missing -, because you have one phandle. > > ok, then i need to drop MaxItems and indent 2 spaces more, but no > problem I missed that maxItems - should not be placed above description, but immediately around items. > >>> + - description: >>> + The first cell represents a phandle to syscon >> >> Don't repeat constraints in free form text. "Foo bar system controller" >> or "Phandle to foo bar system controller" > > i would write only "phandle to system controller". on mt7988 it is the > topmisc syscon, but maybe on > other SoC it is different name. This must be specific to what sort of system controller you point. You are not interested in phandle to any system controller. > >>> + - description: >>> + The second cell represents the register offset >> >> "Baz register offset" > > same here, only "register offset". Also not. You need specific register, not any register. Best regards, Krzysztof
Am 2025-04-17 09:59, schrieb Krzysztof Kozlowski: > On 17/04/2025 09:52, Frank Wunderlich (linux) wrote: >>>> >>>> + mediatek,syscon-type: >>>> + $ref: /schemas/types.yaml#/definitions/phandle-array >>>> + maxItems: 1 >>>> + description: >>>> + A phandle to syscon used to access the register of type >>>> switch, >>>> + the field should always be 3 cells long. >>>> + items: >>>> + items: >>> >>> Missing -, because you have one phandle. >> >> ok, then i need to drop MaxItems and indent 2 spaces more, but no >> problem > > I missed that maxItems - should not be placed above description, but > immediately around items. dt_binding_check complains about maxItems should not be set when having only 1 item ;) so i dropped it in my current version completely. >> >>>> + - description: >>>> + The first cell represents a phandle to syscon >>> >>> Don't repeat constraints in free form text. "Foo bar system >>> controller" >>> or "Phandle to foo bar system controller" >> >> i would write only "phandle to system controller". on mt7988 it is the >> topmisc syscon, but maybe on >> other SoC it is different name. > > This must be specific to what sort of system controller you point. You > are not interested in phandle to any system controller. how about phy configuration controller/register? >> >>>> + - description: >>>> + The second cell represents the register offset >>> >>> "Baz register offset" >> >> same here, only "register offset". > > Also not. You need specific register, not any register. > > > Best regards, > Krzysztof
On 17/04/2025 11:35, Frank Wunderlich (linux) wrote: >>>>> + - description: >>>>> + The first cell represents a phandle to syscon >>>> >>>> Don't repeat constraints in free form text. "Foo bar system >>>> controller" >>>> or "Phandle to foo bar system controller" >>> >>> i would write only "phandle to system controller". on mt7988 it is the >>> topmisc syscon, but maybe on >>> other SoC it is different name. >> >> This must be specific to what sort of system controller you point. You >> are not interested in phandle to any system controller. > > how about phy configuration controller/register? If that's distinctive enough to identify the controller and its register, then sure. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml index 3b5253659e6f..5033d77c1239 100644 --- a/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml +++ b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml @@ -151,6 +151,22 @@ patternProperties: minimum: 1 maximum: 31 + mediatek,syscon-type: + $ref: /schemas/types.yaml#/definitions/phandle-array + maxItems: 1 + description: + A phandle to syscon used to access the register of type switch, + the field should always be 3 cells long. + items: + items: + - description: + The first cell represents a phandle to syscon + - description: + The second cell represents the register offset + - description: + The third cell represents the index of config segment + enum: [0, 1, 2, 3] + required: - reg - clocks