Re: [PATCH v4 1/5] dt-bindings: phy: Add STM32MP25 COMBOPHY bindings
From: Christian Bruel
Date: Fri Aug 30 2024 - 08:54:41 EST
On 8/29/24 18:44, Conor Dooley wrote:
On Thu, Aug 29, 2024 at 01:06:53PM +0200, Christian Bruel wrote:
On 8/28/24 18:11, Conor Dooley wrote:
On Wed, Aug 28, 2024 at 04:34:48PM +0200, Christian Bruel wrote:
Document the bindings for STM32 COMBOPHY interface, used to support
the PCIe and USB3 stm32mp25 drivers.
Following entries can be used to tune caracterisation parameters
- st,output-micro-ohms and st,output-vswing-microvolt bindings entries
to tune the impedance and voltage swing using discrete simulation results
- st,rx-equalizer register to set the internal rx equalizer filter value.
Signed-off-by: Christian Bruel <christian.bruel@xxxxxxxxxxx>
---
.../bindings/phy/st,stm32mp25-combophy.yaml | 128 ++++++++++++++++++
1 file changed, 128 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/st,stm32mp25-combophy.yaml
diff --git a/Documentation/devicetree/bindings/phy/st,stm32mp25-combophy.yaml b/Documentation/devicetree/bindings/phy/st,stm32mp25-combophy.yaml
new file mode 100644
index 000000000000..8d4a40b94507
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/st,stm32mp25-combophy.yaml
@@ -0,0 +1,128 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/st,stm32mp25-combophy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STM32MP25 USB3/PCIe COMBOPHY
+
+maintainers:
+ - Christian Bruel <christian.bruel@xxxxxxxxxxx>
+
+description:
+ Single lane PHY shared (exclusive) between the USB3 and PCIe controllers.
+ Supports 5Gbit/s for USB3 and PCIe gen2 or 2.5Gbit/s for PCIe gen1.
+
+properties:
+ compatible:
+ const: st,stm32mp25-combophy
+
+ reg:
+ maxItems: 1
+
+ "#phy-cells":
+ const: 1
+
+ clocks:
+ minItems: 2
+ items:
+ - description: apb Bus clock mandatory to access registers.
+ - description: ker Internal RCC reference clock for USB3 or PCIe
+ - description: pad Optional on board clock input for PCIe only. Typically an
+ external 100Mhz oscillator wired on dedicated CLKIN pad. Used as reference
+ clock input instead of the ker
+
+ clock-names:
+ minItems: 2
+ items:
+ - const: apb
+ - const: ker
+ - const: pad
+
+ resets:
+ maxItems: 1
+
+ reset-names:
+ const: phy
+
+ power-domains:
+ maxItems: 1
+
+ wakeup-source: true
+
+ interrupts:
+ maxItems: 1
+ description: interrupt used for wakeup
+
+ access-controllers:
+ minItems: 1
+ maxItems: 2
Can you please describe the items here?
I can specialize the description: "Phandle to the rifsc firewall device to check access right."
Right, but there are potentially two access controllers here. You need
to describe which is which, so that people can hook them up in the
correct order. In what case are there two? Your dts patch only has one.
(sorry, resent in plain test)
yes, we don't have more than 1 controller in the current setup,
I'll use maxItems: 1
otherwise described in access-controllers/access-controllers.yaml, see also bindings/bus/st,stm32mp25-rifsc.yaml
+ st,syscfg:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: Phandle to the SYSCON entry required for configuring PCIe
+ or USB3.
Why is a phandle required for this lookup, rather than doing it by
compatible?
the phandle is used to select the sysconf SoC configuration register
depending on the PCIe/USB3 mode (selected by xlate function), so it's not
like a lookup here.
If "syscon_regmap_lookup_by_phandle()" is not a lookup, then I do not
know what is. An example justification for it would be that there are
multiple combophys on the same soc, each using a different sysconf
region. Your dts suggests that is not the case though, since you have
st,syscfg = <&syscfg>; in it, rather than st,syscfg = <&syscfg0>;.
I didn't get your suggestion earlier to use "syscon_regmap_lookup_by_compatible()".
We have several other syscon in the other. That's why we choose a direct syscfg phandle
This sysconf register is also used for other settings
such as the PLL, Reference clock selection, ...
+
+ st,ssc-on:
+ type: boolean
flag, not boolean, for presence based stuff. And in the driver,
s/of_property_read_bool/of_property_present/.
ok
+ description:
+ A boolean property whose presence indicates that the SSC for common clock
+ needs to be set.
And what, may I ask, does "SSC" mean? "Common clock" is also a bit of a
"linuxism", what does this actually do in the hardware block?
SSC for Spread Spectrum Clocking. It is an hardware setting for the 100Mhz PCIe reference common clock,
Ah, so not really a "common clock" linuxism at all.
I will rephrase the description
How is someone supposed to decide between on and off? Is it always on
for PCIe, or only on in some configurations? Or maybe only (or always?) on
if the pad clock is provided?
SSC is off by default and available for the PCIe pad clock. it must be defined for USB3
thanks
Christian
Cheers,
Conor.