On Mon, Feb 10, 2025 at 11:10:13PM +0200, Laurent Pinchart wrote: > On Thu, Feb 06, 2025 at 04:48:55PM -0500, Frank Li wrote: > > On Thu, Feb 06, 2025 at 11:18:08PM +0200, Laurent Pinchart wrote: > > > On Wed, Feb 05, 2025 at 12:18:10PM -0500, Frank Li wrote: > > > > Add MIPI CSI phy binding doc for i.MX8QXP, i.MX8QM and i.MX8ULP. > > > > > > s/CSI/CSI-2/ in the subject line, here and below. > > > s/phy/PHY/ > > > > > > > Signed-off-by: Frank Li <Frank.Li@xxxxxxx> > > > > --- > > > > change from v1 to v2 > > > > - Add missed fsl,imx8qm-mipi-cphy, which failback to fsl,imx8qxp-mipi-cphy > > > > - Move reg to required. Previous 8ulp use fsl,offset in downstream version. > > > > which should be reg. So move it to required > > > > --- > > > > .../bindings/phy/fsl,imx8qxp-mipi-cphy.yaml | 57 ++++++++++++++++++++++ > > > > 1 file changed, 57 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8qxp-mipi-cphy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8qxp-mipi-cphy.yaml > > > > new file mode 100644 > > > > index 0000000000000..7335b9262d0e7 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/phy/fsl,imx8qxp-mipi-cphy.yaml > > > > @@ -0,0 +1,57 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/phy/fsl,imx8qxp-mipi-cphy.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Freescale i.MX8 SoC MIPI CSI PHY > > > > + > > > > +maintainers: > > > > + - Frank Li <Frank.Li@xxxxxxx> > > > > + > > > > +properties: > > > > + "#phy-cells": > > > > + const: 0 > > > > + > > > > + compatible: > > > > + oneOf: > > > > + - enum: > > > > + - fsl,imx8qxp-mipi-cphy > > > > + - fsl,imx8ulp-mipi-cphy > > > > + - items: > > > > + - const: fsl,imx8qm-mipi-cphy > > > > + - const: fsl,imx8qxp-mipi-cphy > > > > > > Why are those called cphy when, as far as I can tell from the > > > documentation, they are D-PHYs ? Does that stand for *C*SI PHY ? > > > > There are already have D-PHYS for MIPI display phy binding. cphy just means > > for camera PHY. > > Ah OK. I would probably have gone for *-mipi-dphy-rx then, but I'm OK > with the proposed "cphy". Explaining this in the description would be > useful. > > > > I find > > > it slightly confusing, but not so much that I'd ask for a change. It's > > > just a name at the end of the day. > > > > > > Apart from that the binding looks fairly OK. Except maybe from the fact > > > that this device is not a PHY :-( It has two PHY control registers, but > > > the rest seems related to the glue logic at the output of the CSI-2 > > > receiver. I wonder if we should go the syscon route. > > > > Do you means use phandle to syscon node in csi-2 driver? Actually this > > ways is not perferred by device tree team because it should be exported > > as what actual function, such as PHY or RESET by use standard interface. > > > > We met similar case at other substream. > > I don't like syscon much either, but in this specific case I'm not sure > what else we could do. This device really aggregates some control over > the PHY and over the glue logic at the output of the CSI-2 controller. > Modelling it as "just a PHY" will cause problem as soon as you'll want > to configure the other parameters. I think your comments is quite good and it's one hack. I check register layout of csr at imx8qm/imx8qxp. It is dedicated for each csi2 moudule. This glue layer should be second 'reg' resource, which direct control in csi2's driver. v3 should be simpler than this version. https://lore.kernel.org/imx/20250210-8qxp_camera-v3-0-324f5105accc@xxxxxxx/T/#t Frank > > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + power-domains: > > > > + maxItems: 1 > > > > + > > > > +required: > > > > + - "#phy-cells" > > > > + - compatible > > > > + - reg > > > > + > > > > +allOf: > > > > + - if: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + enum: > > > > + - fsl,imx8qxp-mipi-cphy > > > > + then: > > > > + required: > > > > + - power-domains > > > > + > > > > +additionalProperties: false > > > > + > > > > +examples: > > > > + - | > > > > + phy@58221000 { > > > > + compatible = "fsl,imx8qxp-mipi-cphy"; > > > > + reg = <0x58221000 0x10000>; > > > > + #phy-cells = <0>; > > > > + power-domains = <&pd 0>; > > > > + }; > > > > + > > -- > Regards, > > Laurent Pinchart
