Re: [PATCH 1/2] dt-bindings: phy: Add Freescale i.MX8qm Mixel LVDS PHY binding

[Liu Ying]

On Sun, 2022-06-19 at 14:11 +0200, Krzysztof Kozlowski wrote:
> On 18/06/2022 11:22, Liu Ying wrote:
> > This patch adds bindings for Mixel LVDS PHY found on
> > Freescale i.MX8qm SoC.
> > 
> > Signed-off-by: Liu Ying <victor.liu@xxxxxxx>
> > ---
> >  .../bindings/phy/mixel,lvds-phy.yaml          | 64
> > +++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/mixel,lvds-
> > phy.yaml b/Documentation/devicetree/bindings/phy/mixel,lvds-
> > phy.yaml
> > new file mode 100644
> > index 000000000000..de964ffb9356
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/mixel,lvds-phy.yaml
> > @@ -0,0 +1,64 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: 
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fphy%2Fmixel%2Clvds-phy.yaml%23&amp;data=05%7C01%7Cvictor.liu%40nxp.com%7Cb68f9012ddcc44bc10cd08da51ece7f7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637912375188731290%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=MdBILPYmfYeWeCXXNxy1mu1NcU0b6EW3QztYc294dd4%3D&amp;reserved=0
> > +$schema: 
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Cvictor.liu%40nxp.com%7Cb68f9012ddcc44bc10cd08da51ece7f7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637912375188731290%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=MhHwku8rbDLAZAQh1T9CGFULMkk5MaNoj3LQnQJ6VXM%3D&amp;reserved=0
> > +
> > +title: Mixel LVDS PHY for Freescale i.MX8qm SoC
> 
> If Mixel is a vendor, it needs it's vendor prefix documented and used
> in
> compatible. Filename should match compatible. If it is not a vendor,
> then filename should be "fsl,imx8qm-lvds-phy.yaml"

Mixel is a vendor. I'll document the vendor prefix and set
'fsl,imx8qm-lvds-phy' and 'mixel,lvds-phy' as compatible's enum.
I'll keep the filename as-is.

> 
> > +
> > +maintainers:
> > +  - Liu Ying <victor.liu@xxxxxxx>
> > +
> > +description: |
> > +  The Mixel LVDS PHY IP block is found on Freescale i.MX8qm SoC.
> > +  It converts two groups of four 7/10 bits of CMOS data into two
> > +  groups of four data lanes of LVDS data streams. A phase-locked
> > +  transmit clock is transmitted in parallel with each group of
> > +  data streams over a fifth LVDS link. Every cycle of the transmit
> > +  clock, 56/80 bits of input data are sampled and transmitted
> > +  through the two groups of LVDS data streams. Together with the
> > +  transmit clocks, the two groups of LVDS data streams form two
> > +  LVDS channels.
> > +
> > +  The Mixel LVDS PHY found on Freescale i.MX8qm SoC is controlled
> > +  by Control and Status Registers(CSR) module in the SoC. The CSR
> > +  module, as a system controller, contains the PHY's registers.
> > +
> > +properties:
> > +  compatible:
> > +    const: fsl,imx8qm-lvds-phy
> > +
> > +  "#phy-cells":
> > +    const: 1
> > +    description: |
> > +      Cell allows setting the LVDS channel index of the PHY.
> > +      Index 0 is for LVDS channel0 and index 1 is for LVDS
> > channel1.
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: phy_ref
> 
> Maybe just skip the clock-names, it's not bringing anything useful,
> unless you expect some more clocks to be documented later? (but in
> such
> case question would be why they are not documented now)

I'll skip it, because it is the only clock required by the PHY IP that
I'm aware of.

Thanks,
Liu Ying

> 
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - "#phy-cells"
> > +  - clocks
> > +  - clock-names
> > +  - power-domains
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/firmware/imx/rsrc.h>
> > +    phy {
> > +        compatible = "fsl,imx8qm-lvds-phy";
> > +        #phy-cells = <1>;
> > +        clocks = <&clk IMX_SC_R_LVDS_0 IMX_SC_PM_CLK_PHY>;
> > +        clock-names = "phy_ref";
> > +        power-domains = <&pd IMX_SC_R_LVDS_0>;
> > +    };
> 
> 
> Best regards,
> Krzysztof