| Message ID | 20191227200116.2612137-10-helen.koike@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Rockchip ISP Driver | expand |
On Fri, Dec 27, 2019 at 2:02 PM Helen Koike <helen.koike@collabora.com> wrote: > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > This was tested and verified with: > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml Documentation/devicetree/bindings/phy/ > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > --- > > Changes in v12: > - The commit replaces the following commit in previous series named > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > This new patch adds yaml binding and was verified with > make dtbs_check and make dt_binding_check > > Changes in v11: None > Changes in v10: > - unsquash > > Changes in v9: > - fix title division style > - squash > - move to staging > > Changes in v8: None > Changes in v7: > - updated doc with new design and tested example > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > 1 file changed, 75 insertions(+) > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml Reviewed-by: Rob Herring <robh@kernel.org>
Hi Helen, Thank you for the patch. On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > This was tested and verified with: > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml Documentation/devicetree/bindings/phy/ > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > --- > > Changes in v12: > - The commit replaces the following commit in previous series named > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > This new patch adds yaml binding and was verified with > make dtbs_check and make dt_binding_check > > Changes in v11: None > Changes in v10: > - unsquash > > Changes in v9: > - fix title division style > - squash > - move to staging > > Changes in v8: None > Changes in v7: > - updated doc with new design and tested example > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > 1 file changed, 75 insertions(+) > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > new file mode 100644 > index 000000000000..af97f1b3e005 > --- /dev/null > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > @@ -0,0 +1,75 @@ > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? Looking at the PHY driver, it seems to handle all PHYs with a single struct device. Should we thus use #phy-cells = <1> to select the PHY ? > + > +maintainers: > + - Helen Koike <helen.koike@collabora.com> > + - Ezequiel Garcia <ezequiel@collabora.com> > + > +description: | > + The Rockchip SoC has a MIPI D-PHY bus with an RX0 entry which connects to > + the ISP1 (Image Signal Processing unit v1.0) for CSI cameras. > + > +properties: > + compatible: > + const: rockchip,rk3399-mipi-dphy > + > + reg: > + maxItems: 1 > + > + clocks: > + items: > + - description: Mipi d-phy ref clock > + - description: Mipi d-phy rx0 cfg clock s/Mipi d-phy/MIPI D-PHY/ > + - description: Video in/out general register file clock > + > + clock-names: > + items: > + - const: dphy-ref > + - const: dphy-cfg > + - const: grf > + > + '#phy-cells': > + const: 0 > + > + power-domains: > + description: Video in/out power domain. > + maxItems: 1 > + > +required: > + - compatible > + - clocks > + - clock-names > + - '#phy-cells' > + - power-domains > + > +additionalProperties: false > + > +examples: > + - | > + > + /* > + * MIPI RX D-PHY use registers in "general register files", it > + * should be a child of the GRF. > + * > + * grf: syscon@ff770000 { > + * compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd"; > + * ... missing * }; > + */ > + > + #include <dt-bindings/clock/rk3399-cru.h> > + #include <dt-bindings/power/rk3399-power.h> > + > + dphy: mipi-dphy { > + compatible = "rockchip,rk3399-mipi-dphy"; > + clocks = <&cru SCLK_MIPIDPHY_REF>, > + <&cru SCLK_DPHY_RX0_CFG>, > + <&cru PCLK_VIO_GRF>; > + clock-names = "dphy-ref", "dphy-cfg", "grf"; > + power-domains = <&power RK3399_PD_VIO>; > + #phy-cells = <0>; > + };
Hi Laurent, Thanks a lot for reviewing this. On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > Hi Helen, > > Thank you for the patch. > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > This was tested and verified with: > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml Documentation/devicetree/bindings/phy/ > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > --- > > > > Changes in v12: > > - The commit replaces the following commit in previous series named > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > This new patch adds yaml binding and was verified with > > make dtbs_check and make dt_binding_check > > > > Changes in v11: None > > Changes in v10: > > - unsquash > > > > Changes in v9: > > - fix title division style > > - squash > > - move to staging > > > > Changes in v8: None > > Changes in v7: > > - updated doc with new design and tested example > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > 1 file changed, 75 insertions(+) > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml b/drivers/staging/media/phy- > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > new file mode 100644 > > index 000000000000..af97f1b3e005 > > --- /dev/null > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > @@ -0,0 +1,75 @@ > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? The driver currently only supports RX0, but I think you are right, it should say RX here. This binding could be extended for RX1. > Looking at the PHY driver, it seems to handle all PHYs with a single > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > I am not following this. The driver handles just one PHY. Each PHY should have its own node. > > + > > +maintainers: > > + - Helen Koike <helen.koike@collabora.com> > > + - Ezequiel Garcia <ezequiel@collabora.com> > > + > > +description: | > > + The Rockchip SoC has a MIPI D-PHY bus with an RX0 entry which connects to > > + the ISP1 (Image Signal Processing unit v1.0) for CSI cameras. > > + > > +properties: > > + compatible: > > + const: rockchip,rk3399-mipi-dphy > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + items: > > + - description: Mipi d-phy ref clock > > + - description: Mipi d-phy rx0 cfg clock > > s/Mipi d-phy/MIPI D-PHY/ > Yep. > > + - description: Video in/out general register file clock > > + > > + clock-names: > > + items: > > + - const: dphy-ref > > + - const: dphy-cfg > > + - const: grf > > + > > + '#phy-cells': > > + const: 0 > > + > > + power-domains: > > + description: Video in/out power domain. > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - clocks > > + - clock-names > > + - '#phy-cells' > > + - power-domains > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + > > + /* > > + * MIPI RX D-PHY use registers in "general register files", it > > + * should be a child of the GRF. > > + * > > + * grf: syscon@ff770000 { > > + * compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd"; > > + * ... > > missing > > * }; > OK. > > + */ > > + > > + #include <dt-bindings/clock/rk3399-cru.h> > > + #include <dt-bindings/power/rk3399-power.h> > > + > > + dphy: mipi-dphy { > > + compatible = "rockchip,rk3399-mipi-dphy"; > > + clocks = <&cru SCLK_MIPIDPHY_REF>, > > + <&cru SCLK_DPHY_RX0_CFG>, > > + <&cru PCLK_VIO_GRF>; > > + clock-names = "dphy-ref", "dphy-cfg", "grf"; > > + power-domains = <&power RK3399_PD_VIO>; > > + #phy-cells = <0>; > > + };
On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote: > On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > > Hi Helen, > > > > Thank you for the patch. > > > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > > > This was tested and verified with: > > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml Documentation/devicetree/bindings/phy/ > > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > --- > > > > > > Changes in v12: > > > - The commit replaces the following commit in previous series named > > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > > This new patch adds yaml binding and was verified with > > > make dtbs_check and make dt_binding_check > > > > > > Changes in v11: None > > > Changes in v10: > > > - unsquash > > > > > > Changes in v9: > > > - fix title division style > > > - squash > > > - move to staging > > > > > > Changes in v8: None > > > Changes in v7: > > > - updated doc with new design and tested example > > > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > > 1 file changed, 75 insertions(+) > > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml b/drivers/staging/media/phy- > > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > new file mode 100644 > > > index 000000000000..af97f1b3e005 > > > --- /dev/null > > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > @@ -0,0 +1,75 @@ > > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? > > The driver currently only supports RX0, but I think you are right, > it should say RX here. This binding could be extended for RX1. > > > Looking at the PHY driver, it seems to handle all PHYs with a single > > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > > I am not following this. The driver handles just one PHY. Each PHY > should have its own node. Looking at the registers, it seems that the different PHYs are intertwined and we would could have trouble handling the different PHYs with different DT nodes and thus struct device instances. > > > + > > > +maintainers: > > > + - Helen Koike <helen.koike@collabora.com> > > > + - Ezequiel Garcia <ezequiel@collabora.com> > > > + > > > +description: | > > > + The Rockchip SoC has a MIPI D-PHY bus with an RX0 entry which connects to > > > + the ISP1 (Image Signal Processing unit v1.0) for CSI cameras. > > > + > > > +properties: > > > + compatible: > > > + const: rockchip,rk3399-mipi-dphy > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + clocks: > > > + items: > > > + - description: Mipi d-phy ref clock > > > + - description: Mipi d-phy rx0 cfg clock > > > > s/Mipi d-phy/MIPI D-PHY/ > > Yep. > > > > + - description: Video in/out general register file clock > > > + > > > + clock-names: > > > + items: > > > + - const: dphy-ref > > > + - const: dphy-cfg > > > + - const: grf > > > + > > > + '#phy-cells': > > > + const: 0 > > > + > > > + power-domains: > > > + description: Video in/out power domain. > > > + maxItems: 1 > > > + > > > +required: > > > + - compatible > > > + - clocks > > > + - clock-names > > > + - '#phy-cells' > > > + - power-domains > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + > > > + /* > > > + * MIPI RX D-PHY use registers in "general register files", it > > > + * should be a child of the GRF. > > > + * > > > + * grf: syscon@ff770000 { > > > + * compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd"; > > > + * ... > > > > missing > > > > * }; > > OK. > > > > + */ > > > + > > > + #include <dt-bindings/clock/rk3399-cru.h> > > > + #include <dt-bindings/power/rk3399-power.h> > > > + > > > + dphy: mipi-dphy { > > > + compatible = "rockchip,rk3399-mipi-dphy"; > > > + clocks = <&cru SCLK_MIPIDPHY_REF>, > > > + <&cru SCLK_DPHY_RX0_CFG>, > > > + <&cru PCLK_VIO_GRF>; > > > + clock-names = "dphy-ref", "dphy-cfg", "grf"; > > > + power-domains = <&power RK3399_PD_VIO>; > > > + #phy-cells = <0>; > > > + };
Am Dienstag, 7. Januar 2020, 03:37:21 CET schrieb Laurent Pinchart: > On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote: > > On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > > > Hi Helen, > > > > > > Thank you for the patch. > > > > > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > > > > > This was tested and verified with: > > > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml Documentation/devicetree/bindings/phy/ > > > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > > > --- > > > > > > > > Changes in v12: > > > > - The commit replaces the following commit in previous series named > > > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > > > This new patch adds yaml binding and was verified with > > > > make dtbs_check and make dt_binding_check > > > > > > > > Changes in v11: None > > > > Changes in v10: > > > > - unsquash > > > > > > > > Changes in v9: > > > > - fix title division style > > > > - squash > > > > - move to staging > > > > > > > > Changes in v8: None > > > > Changes in v7: > > > > - updated doc with new design and tested example > > > > > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > > > 1 file changed, 75 insertions(+) > > > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml b/drivers/staging/media/phy- > > > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > new file mode 100644 > > > > index 000000000000..af97f1b3e005 > > > > --- /dev/null > > > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > @@ -0,0 +1,75 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > > > > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? > > > > The driver currently only supports RX0, but I think you are right, > > it should say RX here. This binding could be extended for RX1. > > > > > Looking at the PHY driver, it seems to handle all PHYs with a single > > > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > > > > I am not following this. The driver handles just one PHY. Each PHY > > should have its own node. > > Looking at the registers, it seems that the different PHYs are > intertwined and we would could have trouble handling the different PHYs > with different DT nodes and thus struct device instances. I have to confess to not following _ALL_ of the threads, so may say something stupid, but I don't think the PHYs are intertwined so much. Where RX0 is controlled from the "General Register Files" alone [register dumping ground for soc designers], the TX1RX1-phy actually gets controlled from inside the dsi1 register area it seems. So in my previous (still unsucessful) tests, I was rolling with something like https://github.com/mmind/linux-rockchip/commit/e0d4b03976d2aab85a8c1630be937ea003b5df88 With the actual "logic" picked from the vendor kernel, that just double- maps the dsi1-registers in both dsi and dphy driver, which was strange. Heiko
Hi Heiko, Laurent, On Tue, 2020-01-07 at 10:28 +0100, Heiko Stübner wrote: > Am Dienstag, 7. Januar 2020, 03:37:21 CET schrieb Laurent Pinchart: > > On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote: > > > On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > > > > Hi Helen, > > > > > > > > Thank you for the patch. > > > > > > > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > > > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > > > > > > > This was tested and verified with: > > > > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi- > > > > > dphy.yaml Documentation/devicetree/bindings/phy/ > > > > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > > > > > --- > > > > > > > > > > Changes in v12: > > > > > - The commit replaces the following commit in previous series named > > > > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > > > > This new patch adds yaml binding and was verified with > > > > > make dtbs_check and make dt_binding_check > > > > > > > > > > Changes in v11: None > > > > > Changes in v10: > > > > > - unsquash > > > > > > > > > > Changes in v9: > > > > > - fix title division style > > > > > - squash > > > > > - move to staging > > > > > > > > > > Changes in v8: None > > > > > Changes in v7: > > > > > - updated doc with new design and tested example > > > > > > > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > > > > 1 file changed, 75 insertions(+) > > > > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > b/drivers/staging/media/phy- > > > > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > new file mode 100644 > > > > > index 000000000000..af97f1b3e005 > > > > > --- /dev/null > > > > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > @@ -0,0 +1,75 @@ > > > > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > > +%YAML 1.2 > > > > > +--- > > > > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > + > > > > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > > > > > > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? > > > > > > The driver currently only supports RX0, but I think you are right, > > > it should say RX here. This binding could be extended for RX1. > > > > > > > Looking at the PHY driver, it seems to handle all PHYs with a single > > > > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > > > > > > I am not following this. The driver handles just one PHY. Each PHY > > > should have its own node. > > > > Looking at the registers, it seems that the different PHYs are > > intertwined and we would could have trouble handling the different PHYs > > with different DT nodes and thus struct device instances. > > I have to confess to not following _ALL_ of the threads, so may say > something stupid, but I don't think the PHYs are intertwined so much. > > Where RX0 is controlled from the "General Register Files" alone > [register dumping ground for soc designers], the TX1RX1-phy > actually gets controlled from inside the dsi1 register area it seems. > > So in my previous (still unsucessful) tests, I was rolling with something like > https://github.com/mmind/linux-rockchip/commit/e0d4b03976d2aab85a8c1630be937ea003b5df88 > > With the actual "logic" picked from the vendor kernel, that just double- > maps the dsi1-registers in both dsi and dphy driver, which was strange. > > Describing each PHY in its own device node (as we currently do) results in: mipi_dphy_tx1rx1: mipi-dphy-tx1rx1@ff968000 { compatible = "rockchip,rk3399-mipi-dphy"; reg = <0x0 0xff968000 0x0 0x8000>; rockchip,grf = <&grf>; }; grf: syscon@ff770000 { mipi_dphy_rx0: mipi-dphy-rx0 { compatible = "rockchip,rk3399-mipi-dphy"; }; }; Which is mildly ugly, as it uses two mechanism to describe the GRF resource. In addition, the driver will then _infer_ which device node is RX0 and which is TX1RX1, from this. Perhaps Laurent's proposal, describing each PHY explicitly, would be cleaner? Thanks, Ezequiel
Hi Ezequiel, Am Dienstag, 7. Januar 2020, 14:20:10 CET schrieb Ezequiel Garcia: > Hi Heiko, Laurent, > > On Tue, 2020-01-07 at 10:28 +0100, Heiko Stübner wrote: > > Am Dienstag, 7. Januar 2020, 03:37:21 CET schrieb Laurent Pinchart: > > > On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote: > > > > On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > > > > > Hi Helen, > > > > > > > > > > Thank you for the patch. > > > > > > > > > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > > > > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > > > > > > > > > This was tested and verified with: > > > > > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi- > > > > > > dphy.yaml Documentation/devicetree/bindings/phy/ > > > > > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > > > > > > > --- > > > > > > > > > > > > Changes in v12: > > > > > > - The commit replaces the following commit in previous series named > > > > > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > > > > > This new patch adds yaml binding and was verified with > > > > > > make dtbs_check and make dt_binding_check > > > > > > > > > > > > Changes in v11: None > > > > > > Changes in v10: > > > > > > - unsquash > > > > > > > > > > > > Changes in v9: > > > > > > - fix title division style > > > > > > - squash > > > > > > - move to staging > > > > > > > > > > > > Changes in v8: None > > > > > > Changes in v7: > > > > > > - updated doc with new design and tested example > > > > > > > > > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > > > > > 1 file changed, 75 insertions(+) > > > > > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > b/drivers/staging/media/phy- > > > > > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > new file mode 100644 > > > > > > index 000000000000..af97f1b3e005 > > > > > > --- /dev/null > > > > > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > @@ -0,0 +1,75 @@ > > > > > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > > > +%YAML 1.2 > > > > > > +--- > > > > > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > + > > > > > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > > > > > > > > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? > > > > > > > > The driver currently only supports RX0, but I think you are right, > > > > it should say RX here. This binding could be extended for RX1. > > > > > > > > > Looking at the PHY driver, it seems to handle all PHYs with a single > > > > > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > > > > > > > > I am not following this. The driver handles just one PHY. Each PHY > > > > should have its own node. > > > > > > Looking at the registers, it seems that the different PHYs are > > > intertwined and we would could have trouble handling the different PHYs > > > with different DT nodes and thus struct device instances. > > > > I have to confess to not following _ALL_ of the threads, so may say > > something stupid, but I don't think the PHYs are intertwined so much. > > > > Where RX0 is controlled from the "General Register Files" alone > > [register dumping ground for soc designers], the TX1RX1-phy > > actually gets controlled from inside the dsi1 register area it seems. > > > > So in my previous (still unsucessful) tests, I was rolling with something like > > https://github.com/mmind/linux-rockchip/commit/e0d4b03976d2aab85a8c1630be937ea003b5df88 > > > > With the actual "logic" picked from the vendor kernel, that just double- > > maps the dsi1-registers in both dsi and dphy driver, which was strange. > > > > > > Describing each PHY in its own device node (as we currently do) > results in: > > mipi_dphy_tx1rx1: mipi-dphy-tx1rx1@ff968000 { > compatible = "rockchip,rk3399-mipi-dphy"; > reg = <0x0 0xff968000 0x0 0x8000>; > rockchip,grf = <&grf>; > }; 0xff968000 actually really is the dsi1 controller, so we'll already have a node for that area. That is the reason I went that way to make the rockchip-dsi optionally also behave as phy-provider. So when it's used in combination with drm and a panel or so it will behave as dsi controller, but when requested via the phy-framework it will expose the dphy functionality. > grf: syscon@ff770000 { > mipi_dphy_rx0: mipi-dphy-rx0 { > compatible = "rockchip,rk3399-mipi-dphy"; > }; > }; > > Which is mildly ugly, as it uses two mechanism to describe > the GRF resource. In addition, the driver will then _infer_ > which device node is RX0 and which is TX1RX1, from this. > > Perhaps Laurent's proposal, describing each PHY explicitly, > would be cleaner? so I really think we shouldn't merge these two things together, especially to not break the dsi1 controller part. Heiko
Hi Heiko, On Tue, Jan 07, 2020 at 10:30:28PM +0100, Heiko Stübner wrote: > Am Dienstag, 7. Januar 2020, 14:20:10 CET schrieb Ezequiel Garcia: > > On Tue, 2020-01-07 at 10:28 +0100, Heiko Stübner wrote: > > > Am Dienstag, 7. Januar 2020, 03:37:21 CET schrieb Laurent Pinchart: > > > > On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote: > > > > > On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > > > > > > Hi Helen, > > > > > > > > > > > > Thank you for the patch. > > > > > > > > > > > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > > > > > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > > > > > > > > > > > This was tested and verified with: > > > > > > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi- > > > > > > > dphy.yaml Documentation/devicetree/bindings/phy/ > > > > > > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > Changes in v12: > > > > > > > - The commit replaces the following commit in previous series named > > > > > > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > > > > > > This new patch adds yaml binding and was verified with > > > > > > > make dtbs_check and make dt_binding_check > > > > > > > > > > > > > > Changes in v11: None > > > > > > > Changes in v10: > > > > > > > - unsquash > > > > > > > > > > > > > > Changes in v9: > > > > > > > - fix title division style > > > > > > > - squash > > > > > > > - move to staging > > > > > > > > > > > > > > Changes in v8: None > > > > > > > Changes in v7: > > > > > > > - updated doc with new design and tested example > > > > > > > > > > > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > > > > > > 1 file changed, 75 insertions(+) > > > > > > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > b/drivers/staging/media/phy- > > > > > > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > new file mode 100644 > > > > > > > index 000000000000..af97f1b3e005 > > > > > > > --- /dev/null > > > > > > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > @@ -0,0 +1,75 @@ > > > > > > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > > > > +%YAML 1.2 > > > > > > > +--- > > > > > > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > + > > > > > > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > > > > > > > > > > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? > > > > > > > > > > The driver currently only supports RX0, but I think you are right, > > > > > it should say RX here. This binding could be extended for RX1. > > > > > > > > > > > Looking at the PHY driver, it seems to handle all PHYs with a single > > > > > > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > > > > > > > > > > I am not following this. The driver handles just one PHY. Each PHY > > > > > should have its own node. > > > > > > > > Looking at the registers, it seems that the different PHYs are > > > > intertwined and we would could have trouble handling the different PHYs > > > > with different DT nodes and thus struct device instances. > > > > > > I have to confess to not following _ALL_ of the threads, so may say > > > something stupid, but I don't think the PHYs are intertwined so much. > > > > > > Where RX0 is controlled from the "General Register Files" alone > > > [register dumping ground for soc designers], the TX1RX1-phy > > > actually gets controlled from inside the dsi1 register area it seems. > > > > > > So in my previous (still unsucessful) tests, I was rolling with something like > > > https://github.com/mmind/linux-rockchip/commit/e0d4b03976d2aab85a8c1630be937ea003b5df88 > > > > > > With the actual "logic" picked from the vendor kernel, that just double- > > > maps the dsi1-registers in both dsi and dphy driver, which was strange. > > > > Describing each PHY in its own device node (as we currently do) > > results in: > > > > mipi_dphy_tx1rx1: mipi-dphy-tx1rx1@ff968000 { > > compatible = "rockchip,rk3399-mipi-dphy"; > > reg = <0x0 0xff968000 0x0 0x8000>; > > rockchip,grf = <&grf>; > > }; > > 0xff968000 actually really is the dsi1 controller, so we'll already > have a node for that area. That is the reason I went that way to make > the rockchip-dsi optionally also behave as phy-provider. > > So when it's used in combination with drm and a panel or so it will > behave as dsi controller, but when requested via the phy-framework > it will expose the dphy functionality. Doesn't RX1/TX1 also expose controls through GRF ? For instance GRF_SOC_CON9 has a dphy_rx1_clk_inv_sel bit. > > grf: syscon@ff770000 { > > mipi_dphy_rx0: mipi-dphy-rx0 { > > compatible = "rockchip,rk3399-mipi-dphy"; > > }; > > }; > > > > Which is mildly ugly, as it uses two mechanism to describe > > the GRF resource. In addition, the driver will then _infer_ > > which device node is RX0 and which is TX1RX1, from this. > > > > Perhaps Laurent's proposal, describing each PHY explicitly, > > would be cleaner? > > so I really think we shouldn't merge these two things together, > especially to not break the dsi1 controller part.
On Tue, 2020-01-07 at 22:30 +0100, Heiko Stübner wrote: > Hi Ezequiel, > > Am Dienstag, 7. Januar 2020, 14:20:10 CET schrieb Ezequiel Garcia: > > Hi Heiko, Laurent, > > > > On Tue, 2020-01-07 at 10:28 +0100, Heiko Stübner wrote: > > > Am Dienstag, 7. Januar 2020, 03:37:21 CET schrieb Laurent Pinchart: > > > > On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote: > > > > > On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > > > > > > Hi Helen, > > > > > > > > > > > > Thank you for the patch. > > > > > > > > > > > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > > > > > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > > > > > > > > > > > This was tested and verified with: > > > > > > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi- > > > > > > > dphy.yaml Documentation/devicetree/bindings/phy/ > > > > > > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > Changes in v12: > > > > > > > - The commit replaces the following commit in previous series named > > > > > > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > > > > > > This new patch adds yaml binding and was verified with > > > > > > > make dtbs_check and make dt_binding_check > > > > > > > > > > > > > > Changes in v11: None > > > > > > > Changes in v10: > > > > > > > - unsquash > > > > > > > > > > > > > > Changes in v9: > > > > > > > - fix title division style > > > > > > > - squash > > > > > > > - move to staging > > > > > > > > > > > > > > Changes in v8: None > > > > > > > Changes in v7: > > > > > > > - updated doc with new design and tested example > > > > > > > > > > > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > > > > > > 1 file changed, 75 insertions(+) > > > > > > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > b/drivers/staging/media/phy- > > > > > > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > new file mode 100644 > > > > > > > index 000000000000..af97f1b3e005 > > > > > > > --- /dev/null > > > > > > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > @@ -0,0 +1,75 @@ > > > > > > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > > > > +%YAML 1.2 > > > > > > > +--- > > > > > > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > + > > > > > > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > > > > > > > > > > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? > > > > > > > > > > The driver currently only supports RX0, but I think you are right, > > > > > it should say RX here. This binding could be extended for RX1. > > > > > > > > > > > Looking at the PHY driver, it seems to handle all PHYs with a single > > > > > > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > > > > > > > > > > I am not following this. The driver handles just one PHY. Each PHY > > > > > should have its own node. > > > > > > > > Looking at the registers, it seems that the different PHYs are > > > > intertwined and we would could have trouble handling the different PHYs > > > > with different DT nodes and thus struct device instances. > > > > > > I have to confess to not following _ALL_ of the threads, so may say > > > something stupid, but I don't think the PHYs are intertwined so much. > > > > > > Where RX0 is controlled from the "General Register Files" alone > > > [register dumping ground for soc designers], the TX1RX1-phy > > > actually gets controlled from inside the dsi1 register area it seems. > > > > > > So in my previous (still unsucessful) tests, I was rolling with something like > > > https://github.com/mmind/linux-rockchip/commit/e0d4b03976d2aab85a8c1630be937ea003b5df88 > > > > > > With the actual "logic" picked from the vendor kernel, that just double- > > > maps the dsi1-registers in both dsi and dphy driver, which was strange. > > > > > > > > > > Describing each PHY in its own device node (as we currently do) > > results in: > > > > mipi_dphy_tx1rx1: mipi-dphy-tx1rx1@ff968000 { > > compatible = "rockchip,rk3399-mipi-dphy"; > > reg = <0x0 0xff968000 0x0 0x8000>; > > rockchip,grf = <&grf>; > > }; > > 0xff968000 actually really is the dsi1 controller, so we'll already > have a node for that area. That is the reason I went that way to make > the rockchip-dsi optionally also behave as phy-provider. > > So when it's used in combination with drm and a panel or so it will > behave as dsi controller, but when requested via the phy-framework > it will expose the dphy functionality. > Hm, and will this driver also support RX1? > > > grf: syscon@ff770000 { > > mipi_dphy_rx0: mipi-dphy-rx0 { > > compatible = "rockchip,rk3399-mipi-dphy"; > > }; > > }; > > > > Which is mildly ugly, as it uses two mechanism to describe > > the GRF resource. In addition, the driver will then _infer_ > > which device node is RX0 and which is TX1RX1, from this. > > > > Perhaps Laurent's proposal, describing each PHY explicitly, > > would be cleaner? > > so I really think we shouldn't merge these two things together, > especially to not break the dsi1 controller part. > I don't think it would necesarily break the dsi1 controller part. You can declare both device nodes as sharing the address region, and then the driver can request the I/O resource only when it needs to, i.e. in the PHY .init hook. It's not super nice, but there's no real reason two devices can't share an I/O memory resource. I like this approach because it exposes the different PHYs explicitly, instead of implicitly. Thanks, Ezequiel
Am Dienstag, 7. Januar 2020, 22:57:39 CET schrieb Laurent Pinchart: > Hi Heiko, > > On Tue, Jan 07, 2020 at 10:30:28PM +0100, Heiko Stübner wrote: > > Am Dienstag, 7. Januar 2020, 14:20:10 CET schrieb Ezequiel Garcia: > > > On Tue, 2020-01-07 at 10:28 +0100, Heiko Stübner wrote: > > > > Am Dienstag, 7. Januar 2020, 03:37:21 CET schrieb Laurent Pinchart: > > > > > On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote: > > > > > > On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > > > > > > > Hi Helen, > > > > > > > > > > > > > > Thank you for the patch. > > > > > > > > > > > > > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > > > > > > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > > > > > > > > > > > > > This was tested and verified with: > > > > > > > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi- > > > > > > > > dphy.yaml Documentation/devicetree/bindings/phy/ > > > > > > > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > Changes in v12: > > > > > > > > - The commit replaces the following commit in previous series named > > > > > > > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > > > > > > > This new patch adds yaml binding and was verified with > > > > > > > > make dtbs_check and make dt_binding_check > > > > > > > > > > > > > > > > Changes in v11: None > > > > > > > > Changes in v10: > > > > > > > > - unsquash > > > > > > > > > > > > > > > > Changes in v9: > > > > > > > > - fix title division style > > > > > > > > - squash > > > > > > > > - move to staging > > > > > > > > > > > > > > > > Changes in v8: None > > > > > > > > Changes in v7: > > > > > > > > - updated doc with new design and tested example > > > > > > > > > > > > > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > > > > > > > 1 file changed, 75 insertions(+) > > > > > > > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > b/drivers/staging/media/phy- > > > > > > > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > new file mode 100644 > > > > > > > > index 000000000000..af97f1b3e005 > > > > > > > > --- /dev/null > > > > > > > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > @@ -0,0 +1,75 @@ > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > > > > > +%YAML 1.2 > > > > > > > > +--- > > > > > > > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > > + > > > > > > > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > > > > > > > > > > > > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? > > > > > > > > > > > > The driver currently only supports RX0, but I think you are right, > > > > > > it should say RX here. This binding could be extended for RX1. > > > > > > > > > > > > > Looking at the PHY driver, it seems to handle all PHYs with a single > > > > > > > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > > > > > > > > > > > > I am not following this. The driver handles just one PHY. Each PHY > > > > > > should have its own node. > > > > > > > > > > Looking at the registers, it seems that the different PHYs are > > > > > intertwined and we would could have trouble handling the different PHYs > > > > > with different DT nodes and thus struct device instances. > > > > > > > > I have to confess to not following _ALL_ of the threads, so may say > > > > something stupid, but I don't think the PHYs are intertwined so much. > > > > > > > > Where RX0 is controlled from the "General Register Files" alone > > > > [register dumping ground for soc designers], the TX1RX1-phy > > > > actually gets controlled from inside the dsi1 register area it seems. > > > > > > > > So in my previous (still unsucessful) tests, I was rolling with something like > > > > https://github.com/mmind/linux-rockchip/commit/e0d4b03976d2aab85a8c1630be937ea003b5df88 > > > > > > > > With the actual "logic" picked from the vendor kernel, that just double- > > > > maps the dsi1-registers in both dsi and dphy driver, which was strange. > > > > > > Describing each PHY in its own device node (as we currently do) > > > results in: > > > > > > mipi_dphy_tx1rx1: mipi-dphy-tx1rx1@ff968000 { > > > compatible = "rockchip,rk3399-mipi-dphy"; > > > reg = <0x0 0xff968000 0x0 0x8000>; > > > rockchip,grf = <&grf>; > > > }; > > > > 0xff968000 actually really is the dsi1 controller, so we'll already > > have a node for that area. That is the reason I went that way to make > > the rockchip-dsi optionally also behave as phy-provider. > > > > So when it's used in combination with drm and a panel or so it will > > behave as dsi controller, but when requested via the phy-framework > > it will expose the dphy functionality. > > Doesn't RX1/TX1 also expose controls through GRF ? For instance > GRF_SOC_CON9 has a dphy_rx1_clk_inv_sel bit. There are parts in the GRF but the whole phy-write stuff is inside the dsi controller area. See the vendor kernel at https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/phy/rockchip/phy-rockchip-mipi-rx.c#L1427 where you get write_grf_reg() for GRF bits but also the write_txrx_reg() and mipidphy1_wr_reg() calls that access registers inside the dsi1 controller space. Heiko > > > > grf: syscon@ff770000 { > > > mipi_dphy_rx0: mipi-dphy-rx0 { > > > compatible = "rockchip,rk3399-mipi-dphy"; > > > }; > > > }; > > > > > > Which is mildly ugly, as it uses two mechanism to describe > > > the GRF resource. In addition, the driver will then _infer_ > > > which device node is RX0 and which is TX1RX1, from this. > > > > > > Perhaps Laurent's proposal, describing each PHY explicitly, > > > would be cleaner? > > > > so I really think we shouldn't merge these two things together, > > especially to not break the dsi1 controller part. > >
Am Dienstag, 7. Januar 2020, 23:03:54 CET schrieb Ezequiel Garcia: > On Tue, 2020-01-07 at 22:30 +0100, Heiko Stübner wrote: > > Hi Ezequiel, > > > > Am Dienstag, 7. Januar 2020, 14:20:10 CET schrieb Ezequiel Garcia: > > > Hi Heiko, Laurent, > > > > > > On Tue, 2020-01-07 at 10:28 +0100, Heiko Stübner wrote: > > > > Am Dienstag, 7. Januar 2020, 03:37:21 CET schrieb Laurent Pinchart: > > > > > On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote: > > > > > > On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > > > > > > > Hi Helen, > > > > > > > > > > > > > > Thank you for the patch. > > > > > > > > > > > > > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > > > > > > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > > > > > > > > > > > > > This was tested and verified with: > > > > > > > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi- > > > > > > > > dphy.yaml Documentation/devicetree/bindings/phy/ > > > > > > > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > Changes in v12: > > > > > > > > - The commit replaces the following commit in previous series named > > > > > > > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > > > > > > > This new patch adds yaml binding and was verified with > > > > > > > > make dtbs_check and make dt_binding_check > > > > > > > > > > > > > > > > Changes in v11: None > > > > > > > > Changes in v10: > > > > > > > > - unsquash > > > > > > > > > > > > > > > > Changes in v9: > > > > > > > > - fix title division style > > > > > > > > - squash > > > > > > > > - move to staging > > > > > > > > > > > > > > > > Changes in v8: None > > > > > > > > Changes in v7: > > > > > > > > - updated doc with new design and tested example > > > > > > > > > > > > > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > > > > > > > 1 file changed, 75 insertions(+) > > > > > > > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > b/drivers/staging/media/phy- > > > > > > > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > new file mode 100644 > > > > > > > > index 000000000000..af97f1b3e005 > > > > > > > > --- /dev/null > > > > > > > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > @@ -0,0 +1,75 @@ > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > > > > > +%YAML 1.2 > > > > > > > > +--- > > > > > > > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > > + > > > > > > > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > > > > > > > > > > > > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? > > > > > > > > > > > > The driver currently only supports RX0, but I think you are right, > > > > > > it should say RX here. This binding could be extended for RX1. > > > > > > > > > > > > > Looking at the PHY driver, it seems to handle all PHYs with a single > > > > > > > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > > > > > > > > > > > > I am not following this. The driver handles just one PHY. Each PHY > > > > > > should have its own node. > > > > > > > > > > Looking at the registers, it seems that the different PHYs are > > > > > intertwined and we would could have trouble handling the different PHYs > > > > > with different DT nodes and thus struct device instances. > > > > > > > > I have to confess to not following _ALL_ of the threads, so may say > > > > something stupid, but I don't think the PHYs are intertwined so much. > > > > > > > > Where RX0 is controlled from the "General Register Files" alone > > > > [register dumping ground for soc designers], the TX1RX1-phy > > > > actually gets controlled from inside the dsi1 register area it seems. > > > > > > > > So in my previous (still unsucessful) tests, I was rolling with something like > > > > https://github.com/mmind/linux-rockchip/commit/e0d4b03976d2aab85a8c1630be937ea003b5df88 > > > > > > > > With the actual "logic" picked from the vendor kernel, that just double- > > > > maps the dsi1-registers in both dsi and dphy driver, which was strange. > > > > > > > > > > > > > > Describing each PHY in its own device node (as we currently do) > > > results in: > > > > > > mipi_dphy_tx1rx1: mipi-dphy-tx1rx1@ff968000 { > > > compatible = "rockchip,rk3399-mipi-dphy"; > > > reg = <0x0 0xff968000 0x0 0x8000>; > > > rockchip,grf = <&grf>; > > > }; > > > > 0xff968000 actually really is the dsi1 controller, so we'll already > > have a node for that area. That is the reason I went that way to make > > the rockchip-dsi optionally also behave as phy-provider. > > > > So when it's used in combination with drm and a panel or so it will > > behave as dsi controller, but when requested via the phy-framework > > it will expose the dphy functionality. > > > > Hm, and will this driver also support RX1? what is RX1 in your book? :-) According to the TRM the rk3399 has 3 DPHYs, tx0 - connected exclusively to dsi0 (this is handled internally by the dw-mipi-dsi driver with controls in the dsi0 register space) rx0 - connected exclusively to isp0 (this is handled by the individual dphy driver from Helen's series) tx1rx1 - shared between dsi1 and isp1 (again inside the dsi1 register space) > > > > > > grf: syscon@ff770000 { > > > mipi_dphy_rx0: mipi-dphy-rx0 { > > > compatible = "rockchip,rk3399-mipi-dphy"; > > > }; > > > }; > > > > > > Which is mildly ugly, as it uses two mechanism to describe > > > the GRF resource. In addition, the driver will then _infer_ > > > which device node is RX0 and which is TX1RX1, from this. > > > > > > Perhaps Laurent's proposal, describing each PHY explicitly, > > > would be cleaner? > > > > so I really think we shouldn't merge these two things together, > > especially to not break the dsi1 controller part. > > > > I don't think it would necesarily break the dsi1 controller part. > > You can declare both device nodes as sharing the address region, > and then the driver can request the I/O resource only when it needs to, > i.e. in the PHY .init hook. dsi1 is of course a dw-mipi-dsi one, which in turn shares a common bridge driver over multiple variants (non-rockchip), which expects its registers mapped during probe. I think it would not really work well if you need to make the whole world follow that idea ;-) . Hence my approach with exposing the phy interface from the dsi driver. If you look at the dts part, it also just looks like it should be ... as a regular phy: https://github.com/mmind/linux-rockchip/blob/wip/tc358749/arch/arm64/boot/dts/rockchip/rk3399.dtsi#L1764 And on the driver side there is even some short circuit protection. When used as phy, it won't allow to be used as a component and vice versa. > It's not super nice, but there's no real reason two devices > can't share an I/O memory resource. Counter argument, devicetree is not a means to handle Linux peculiarites - instead it should describe the hardware ... and the area there _is_ the dsi1 controller ;-) Heiko
On Tue, 2020-01-07 at 23:25 +0100, Heiko Stuebner wrote: > Am Dienstag, 7. Januar 2020, 23:03:54 CET schrieb Ezequiel Garcia: > > On Tue, 2020-01-07 at 22:30 +0100, Heiko Stübner wrote: > > > Hi Ezequiel, > > > > > > Am Dienstag, 7. Januar 2020, 14:20:10 CET schrieb Ezequiel Garcia: > > > > Hi Heiko, Laurent, > > > > > > > > On Tue, 2020-01-07 at 10:28 +0100, Heiko Stübner wrote: > > > > > Am Dienstag, 7. Januar 2020, 03:37:21 CET schrieb Laurent Pinchart: > > > > > > On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote: > > > > > > > On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > > > > > > > > Hi Helen, > > > > > > > > > > > > > > > > Thank you for the patch. > > > > > > > > > > > > > > > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > > > > > > > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > > > > > > > > > > > > > > > This was tested and verified with: > > > > > > > > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi- > > > > > > > > > dphy.yaml Documentation/devicetree/bindings/phy/ > > > > > > > > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > Changes in v12: > > > > > > > > > - The commit replaces the following commit in previous series named > > > > > > > > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > > > > > > > > This new patch adds yaml binding and was verified with > > > > > > > > > make dtbs_check and make dt_binding_check > > > > > > > > > > > > > > > > > > Changes in v11: None > > > > > > > > > Changes in v10: > > > > > > > > > - unsquash > > > > > > > > > > > > > > > > > > Changes in v9: > > > > > > > > > - fix title division style > > > > > > > > > - squash > > > > > > > > > - move to staging > > > > > > > > > > > > > > > > > > Changes in v8: None > > > > > > > > > Changes in v7: > > > > > > > > > - updated doc with new design and tested example > > > > > > > > > > > > > > > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > > > > > > > > 1 file changed, 75 insertions(+) > > > > > > > > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > b/drivers/staging/media/phy- > > > > > > > > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > new file mode 100644 > > > > > > > > > index 000000000000..af97f1b3e005 > > > > > > > > > --- /dev/null > > > > > > > > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > @@ -0,0 +1,75 @@ > > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > > > > > > +%YAML 1.2 > > > > > > > > > +--- > > > > > > > > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > > > + > > > > > > > > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > > > > > > > > > > > > > > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? > > > > > > > > > > > > > > The driver currently only supports RX0, but I think you are right, > > > > > > > it should say RX here. This binding could be extended for RX1. > > > > > > > > > > > > > > > Looking at the PHY driver, it seems to handle all PHYs with a single > > > > > > > > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > > > > > > > > > > > > > > I am not following this. The driver handles just one PHY. Each PHY > > > > > > > should have its own node. > > > > > > > > > > > > Looking at the registers, it seems that the different PHYs are > > > > > > intertwined and we would could have trouble handling the different PHYs > > > > > > with different DT nodes and thus struct device instances. > > > > > > > > > > I have to confess to not following _ALL_ of the threads, so may say > > > > > something stupid, but I don't think the PHYs are intertwined so much. > > > > > > > > > > Where RX0 is controlled from the "General Register Files" alone > > > > > [register dumping ground for soc designers], the TX1RX1-phy > > > > > actually gets controlled from inside the dsi1 register area it seems. > > > > > > > > > > So in my previous (still unsucessful) tests, I was rolling with something like > > > > > https://github.com/mmind/linux-rockchip/commit/e0d4b03976d2aab85a8c1630be937ea003b5df88 > > > > > > > > > > With the actual "logic" picked from the vendor kernel, that just double- > > > > > maps the dsi1-registers in both dsi and dphy driver, which was strange. > > > > > > > > > > > > > > > > > > Describing each PHY in its own device node (as we currently do) > > > > results in: > > > > > > > > mipi_dphy_tx1rx1: mipi-dphy-tx1rx1@ff968000 { > > > > compatible = "rockchip,rk3399-mipi-dphy"; > > > > reg = <0x0 0xff968000 0x0 0x8000>; > > > > rockchip,grf = <&grf>; > > > > }; > > > > > > 0xff968000 actually really is the dsi1 controller, so we'll already > > > have a node for that area. That is the reason I went that way to make > > > the rockchip-dsi optionally also behave as phy-provider. > > > > > > So when it's used in combination with drm and a panel or so it will > > > behave as dsi controller, but when requested via the phy-framework > > > it will expose the dphy functionality. > > > > > > > Hm, and will this driver also support RX1? > > what is RX1 in your book? :-) > I meant tx1rx1 configured as a receiver, sorry for the confusion! > According to the TRM the rk3399 has 3 DPHYs, > tx0 - connected exclusively to dsi0 > (this is handled internally by the dw-mipi-dsi driver with controls > in the dsi0 register space) > rx0 - connected exclusively to isp0 > (this is handled by the individual dphy driver from Helen's series) > tx1rx1 - shared between dsi1 and isp1 > (again inside the dsi1 register space) > Exactly. > > > > > grf: syscon@ff770000 { > > > > mipi_dphy_rx0: mipi-dphy-rx0 { > > > > compatible = "rockchip,rk3399-mipi-dphy"; > > > > }; > > > > }; > > > > > > > > Which is mildly ugly, as it uses two mechanism to describe > > > > the GRF resource. In addition, the driver will then _infer_ > > > > which device node is RX0 and which is TX1RX1, from this. > > > > > > > > Perhaps Laurent's proposal, describing each PHY explicitly, > > > > would be cleaner? > > > > > > so I really think we shouldn't merge these two things together, > > > especially to not break the dsi1 controller part. > > > > > > > I don't think it would necesarily break the dsi1 controller part. > > > > You can declare both device nodes as sharing the address region, > > and then the driver can request the I/O resource only when it needs to, > > i.e. in the PHY .init hook. > > dsi1 is of course a dw-mipi-dsi one, which in turn shares a common bridge > driver over multiple variants (non-rockchip), which expects its registers > mapped during probe. > > I think it would not really work well if you need to make the whole world > follow that idea ;-) . > > > Hence my approach with exposing the phy interface from the dsi driver. > If you look at the dts part, it also just looks like it should be ... as > a regular phy: > https://github.com/mmind/linux-rockchip/blob/wip/tc358749/arch/arm64/boot/dts/rockchip/rk3399.dtsi#L1764 > > And on the driver side there is even some short circuit protection. > When used as phy, it won't allow to be used as a component and > vice versa. > OK, it's more clear now. If this bindings will only ever support the RX0 PHY, will not support anything else, then we don't need to worry. It can stay as a GRF syscon child, and will have phy-cells = 0. Thanks, Ezequiel
diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml new file mode 100644 index 000000000000..af97f1b3e005 --- /dev/null +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml @@ -0,0 +1,75 @@ +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings + +maintainers: + - Helen Koike <helen.koike@collabora.com> + - Ezequiel Garcia <ezequiel@collabora.com> + +description: | + The Rockchip SoC has a MIPI D-PHY bus with an RX0 entry which connects to + the ISP1 (Image Signal Processing unit v1.0) for CSI cameras. + +properties: + compatible: + const: rockchip,rk3399-mipi-dphy + + reg: + maxItems: 1 + + clocks: + items: + - description: Mipi d-phy ref clock + - description: Mipi d-phy rx0 cfg clock + - description: Video in/out general register file clock + + clock-names: + items: + - const: dphy-ref + - const: dphy-cfg + - const: grf + + '#phy-cells': + const: 0 + + power-domains: + description: Video in/out power domain. + maxItems: 1 + +required: + - compatible + - clocks + - clock-names + - '#phy-cells' + - power-domains + +additionalProperties: false + +examples: + - | + + /* + * MIPI RX D-PHY use registers in "general register files", it + * should be a child of the GRF. + * + * grf: syscon@ff770000 { + * compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd"; + * ... + */ + + #include <dt-bindings/clock/rk3399-cru.h> + #include <dt-bindings/power/rk3399-power.h> + + dphy: mipi-dphy { + compatible = "rockchip,rk3399-mipi-dphy"; + clocks = <&cru SCLK_MIPIDPHY_REF>, + <&cru SCLK_DPHY_RX0_CFG>, + <&cru PCLK_VIO_GRF>; + clock-names = "dphy-ref", "dphy-cfg", "grf"; + power-domains = <&power RK3399_PD_VIO>; + #phy-cells = <0>; + };
Add yaml DT bindings for Rockchip MIPI D-PHY RX This was tested and verified with: mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml Documentation/devicetree/bindings/phy/ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml Signed-off-by: Helen Koike <helen.koike@collabora.com> --- Changes in v12: - The commit replaces the following commit in previous series named media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings This new patch adds yaml binding and was verified with make dtbs_check and make dt_binding_check Changes in v11: None Changes in v10: - unsquash Changes in v9: - fix title division style - squash - move to staging Changes in v8: None Changes in v7: - updated doc with new design and tested example .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml