Re: [PATCH v2 01/14] dt-bindings: phy: Convert Cadence MHDP PHY bindings to YAML.

[Rob Herring]

On Tue, Dec 31, 2019 at 02:24:07PM +0200, Jyri Sarha wrote:
> On 23/12/2019 17:15, Yuti Amonkar wrote:
> > - Convert the MHDP PHY devicetree bindings to yaml schemas.
> > - Rename DP PHY to have generic Torrent PHY nomrnclature.
> > - Rename compatible string from "cdns,dp-phy" to "cdns,torrent-phy".
> >   This will not affect ABI as the driver has never been functional,
> >   and therefore do not exist in any active use case
> > 
> > Signed-off-by: Yuti Amonkar <yamonkar@xxxxxxxxxxx>
> > ---
> >  .../devicetree/bindings/phy/phy-cadence-dp.txt     | 30 ----------
> >  .../bindings/phy/phy-cadence-torrent.yaml          | 64 ++++++++++++++++++++++
> >  2 files changed, 64 insertions(+), 30 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/phy/phy-cadence-dp.txt
> >  create mode 100644 Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-dp.txt b/Documentation/devicetree/bindings/phy/phy-cadence-dp.txt
> > deleted file mode 100644
> > index 7f49fd54e..0000000
> > --- a/Documentation/devicetree/bindings/phy/phy-cadence-dp.txt
> > +++ /dev/null
> > @@ -1,30 +0,0 @@
> > -Cadence MHDP DisplayPort SD0801 PHY binding
> > -===========================================
> > -
> > -This binding describes the Cadence SD0801 PHY hardware included with
> > -the Cadence MHDP DisplayPort controller.
> > -
> > --------------------------------------------------------------------------------
> > -Required properties (controller (parent) node):
> > -- compatible	: Should be "cdns,dp-phy"
> > -- reg		: Defines the following sets of registers in the parent
> > -		  mhdp device:
> > -			- Offset of the DPTX PHY configuration registers
> > -			- Offset of the SD0801 PHY configuration registers
> > -- #phy-cells	: from the generic PHY bindings, must be 0.
> > -
> > -Optional properties:
> > -- num_lanes	: Number of DisplayPort lanes to use (1, 2 or 4)
> > -- max_bit_rate	: Maximum DisplayPort link bit rate to use, in Mbps (2160,
> > -		  2430, 2700, 3240, 4320, 5400 or 8100)
> > --------------------------------------------------------------------------------
> > -
> > -Example:
> > -	dp_phy: phy@f0fb030a00 {
> > -		compatible = "cdns,dp-phy";
> > -		reg = <0xf0 0xfb030a00 0x0 0x00000040>,
> > -		      <0xf0 0xfb500000 0x0 0x00100000>;
> > -		num_lanes = <4>;
> > -		max_bit_rate = <8100>;
> > -		#phy-cells = <0>;
> > -	};
> > diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> > new file mode 100644
> > index 0000000..3587312
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> > @@ -0,0 +1,64 @@
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/phy/phy-cadence-torrent.yaml#";
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#";
> > +
> > +title: Cadence Torrent SD0801 PHY binding for DisplayPort
> > +
> > +description:
> > +  This binding describes the Cadence SD0801 PHY hardware included with
> > +  the Cadence MHDP DisplayPort controller.
> > +
> > +maintainers:
> > +  - Swapnil Jakhade <sjakhade@xxxxxxxxxxx>
> > +  - Yuti Amonkar <yamonkar@xxxxxxxxxxx>
> > +
> > +properties:
> > +  compatible:
> > +    const: cdns,torrent-phy
> > +
> > +  reg:
> > +    items:
> > +      - description: Offset of the DPTX PHY configuration registers.
> 
> Isn't it possible to use torrent-phy in a configuration that does not
> have dptx? Shouldn't the "dptx_phy" reg entry be optional?
> 
> BTW, I have no idea how to indicate in a yaml binding that some named
> reg-entry is mandatory, and another is optional... anybody? Or is it
> just something to explain in the description?

If it is the last entry (which is preferred), then you just need a 
'minItems: 1' in here.

If it is the first entry which is the case here, then you need 2 entries 
under a oneOf:

oneOf:
  - items:
      - description: Offset of the SD0801 PHY configuration registers.
  - items:
      - description: Offset of the DPTX PHY configuration registers.
      - description: Offset of the SD0801 PHY configuration registers.

Otherwise, the schema looks fine to me.

Rob