Re: [PATCH 1/2] dt-bindings: phy: Add YAML schemas for Intel Combo phy

From: Dilip Kota
Date: Wed Jan 15 2020 - 02:52:55 EST



On 1/14/2020 10:31 PM, Rob Herring wrote:
On Tue, Jan 14, 2020 at 3:18 AM Dilip Kota <eswara.kota@xxxxxxxxxxxxxxx> wrote:

On 1/8/2020 10:18 PM, Rob Herring wrote:
On Fri, Dec 20, 2019 at 03:28:27PM +0800, Dilip Kota wrote:
Combo phy subsystem provides PHY support to number of
controllers, viz. PCIe, SATA and EMAC.
Adding YAML schemas for the same.

Signed-off-by: Dilip Kota <eswara.kota@xxxxxxxxxxxxxxx>
---
.../devicetree/bindings/phy/intel,combo-phy.yaml | 147 +++++++++++++++++++++
1 file changed, 147 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/intel,combo-phy.yaml b/Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
new file mode 100644
index 000000000000..fc9cbad9dd88
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
@@ -0,0 +1,147 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/intel,combo-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel Combo phy Subsystem
+
+maintainers:
+ - Dilip Kota <eswara.kota@xxxxxxxxxxxxxxx>
+
+description: |
+ Intel combo phy subsystem supports PHYs for PCIe, EMAC and SATA
+ controllers. A single combo phy provides two PHY instances.
+
+properties:
+ $nodename:
+ pattern: "^combophy@[0-9]+$"
+
+ compatible:
+ items:
+ - const: intel,combo-phy
+ - const: simple-bus
This will cause the schema to be applied to every 'simple-bus'. You need
a custom 'select' to prevent that. There's several examples in the tree.
Ok, i will add as below:

# We need a select here so we don't match all nodes with 'simple-bus'
select:
properties:
compatible:
contains:
const: intel,combo-phy
required:
- compatible

Though I'm not sure you need child nodes here.

+
+ cell-index:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Index of Combo phy hardware instance.
Drop this. Not used for FDT.
Ok, I will remove this and use the 'aliases' to get the hardware instance.
+
+ resets:
+ maxItems: 2
+
+ reset-names:
+ items:
+ - const: phy
+ - const: core
+
+ intel,syscfg:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: Chip configuration registers handle
+
+ intel,hsio:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: HSIO registers handle
+
+ intel,bid:
+ description: Index of HSIO bus
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - minimum: 0
+ - maximum: 1
If this is related to intel,hsio, just make it an args cell for
intel,hsio.
No. Actually, this is specific to the combophy instance on the HSIO bus.
I see , this can be removed and value can be derived from the hardware
instance value mentioned through 'aliases'
Generally, 'aliases' should be optional. Why do you need an index?
What's the difference between the blocks?

If it wasn't clear, I was suggesting doing:

intel,hsio = <&hsio 1>;
On the SoC, total 4 combophy (0,1,2 and 3) instances are present -> 'cell-index'
2 instances (0,1) are present on the HSIOL NoC
Other 2 instances (2,3) are present on the HSIOR NoC
On the both HSIO NoCs, combophy instances are referred as 0 and 1 -> 'bid'

'bid' is required while accessing the registers in hsio block, to configure the COMBOPHY mode and clock
'cell-index' is required while accessing sysconfig registers to enable the pcie phy pad ref clock.

<&hsio 1>
'bid' is specific to the combophy, not all the DT nodes using &hsio has a need.
I think it is better to pass the bid value as a entry of combophy DT node.

I will add dt entry something like 'hw-instance-id' instead of cell-index or aliases.

Regards,
Dilip


Rob