Move the bindings out of drivers/staging and place them in
Documentation/devicetree/bindings instead.
Also, add DT nodes for RK3399 and verify with make ARCH=arm64 dtbs_check
Tested by verifying images streamed from RockPi 4 board with imx219
module.
Helen Koike (2):
dt-bindings: phy: phy-rockchip-dphy-rx0: move rockchip dphy rx0
bindings out of staging
dt-bindings: media: rkisp1: move rockchip-isp1 bindings out of staging
Shunqian Zheng (2):
arm64: dts: rockchip: add rx0 mipi-phy for rk3399
arm64: dts: rockchip: add isp0 node for rk3399
.../bindings/media/rockchip-isp1.yaml | 0
.../bindings/phy/rockchip-mipi-dphy-rx0.yaml | 0
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 38 +++++++++++++++++++
3 files changed, 38 insertions(+)
rename {drivers/staging/media/rkisp1/Documentation => Documentation}/devicetree/bindings/media/rockchip-isp1.yaml (100%)
rename {drivers/staging/media/phy-rockchip-dphy-rx0/Documentation => Documentation}/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml (100%)
--
2.26.0
Move rkisp1 bindings to Documentation/devicetree/bindings/media
Signed-off-by: Helen Koike <[email protected]>
---
.../devicetree/bindings/media/rockchip-isp1.yaml | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename {drivers/staging/media/rkisp1/Documentation => Documentation}/devicetree/bindings/media/rockchip-isp1.yaml (100%)
diff --git a/drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.yaml b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
similarity index 100%
rename from drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
rename to Documentation/devicetree/bindings/media/rockchip-isp1.yaml
--
2.26.0
From: Shunqian Zheng <[email protected]>
Designware MIPI D-PHY, used for ISP0 in rk3399.
Verified with:
make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
Signed-off-by: Shunqian Zheng <[email protected]>
Signed-off-by: Jacob Chen <[email protected]>
Signed-off-by: Helen Koike <[email protected]>
---
This patchset came from the original ISP series from Rockchip:
https://patchwork.kernel.org/patch/10267409/
The only difference is:
- add phy-cells
- update compatible to "rockchip,rk3399-mipi-dphy-rx0"
- commit message
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 33cc21fcf4c10..fc0295d2a65a1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1394,6 +1394,17 @@ io_domains: io-domains {
status = "disabled";
};
+ mipi_dphy_rx0: mipi-dphy-rx0 {
+ compatible = "rockchip,rk3399-mipi-dphy-rx0";
+ 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>;
+ status = "disabled";
+ };
+
u2phy0: usb2-phy@e450 {
compatible = "rockchip,rk3399-usb2phy";
reg = <0xe450 0x10>;
--
2.26.0
Move phy-rockchip-dphy-rx0 bindings to Documentation/devicetree/bindings/phy
Signed-off-by: Helen Koike <[email protected]>
---
.../devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename {drivers/staging/media/phy-rockchip-dphy-rx0/Documentation => Documentation}/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml (100%)
diff --git a/drivers/staging/media/phy-rockchip-dphy-rx0/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml b/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
similarity index 100%
rename from drivers/staging/media/phy-rockchip-dphy-rx0/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
rename to Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
--
2.26.0
From: Shunqian Zheng <[email protected]>
RK3399 has two ISPs, but only ISP0 was tested at present.
Add isp0 node in rk3399 dtsi
Verified with:
make ARCH=arm64 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/rockchip-isp1.yaml
Signed-off-by: Shunqian Zheng <[email protected]>
Signed-off-by: Jacob Chen <[email protected]>
Signed-off-by: Helen Koike <[email protected]>
---
This patch was originally part of this patchset:
https://patchwork.kernel.org/patch/10267431/
The only difference is:
- add phy properties
- add ports
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 27 ++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index fc0295d2a65a1..815099a0cd0dd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1718,6 +1718,33 @@ vopb_mmu: iommu@ff903f00 {
status = "disabled";
};
+ isp0: isp0@ff910000 {
+ compatible = "rockchip,rk3399-cif-isp";
+ reg = <0x0 0xff910000 0x0 0x4000>;
+ interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru SCLK_ISP0>,
+ <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
+ <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
+ clock-names = "clk_isp",
+ "aclk_isp", "aclk_isp_wrap",
+ "hclk_isp", "hclk_isp_wrap";
+ power-domains = <&power RK3399_PD_ISP0>;
+ iommus = <&isp0_mmu>;
+ phys = <&mipi_dphy_rx0>;
+ phy-names = "dphy";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ };
+ };
+ };
+
isp0_mmu: iommu@ff914000 {
compatible = "rockchip,iommu";
reg = <0x0 0xff914000 0x0 0x100>, <0x0 0xff915000 0x0 0x100>;
--
2.26.0
Hi Helen,
> # SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> %YAML 1.2
> ---
> $id: http://devicetree.org/schemas/media/rockchip-isp1.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> title: Rockchip SoC Image Signal Processing unit v1
Where do we need 'v1' for? Is there a 'v2'?
>
> maintainers:
> - Helen Koike <[email protected]>
>
> description: |
> Rockchip ISP1 is the Camera interface for the Rockchip series of SoCs
> which contains image processing, scaling, and compression functions.
>
> properties:
> compatible:
> const: rockchip,rk3399-cif-isp
>
> reg:
> maxItems: 1
>
> interrupts:
> maxItems: 1
>
> iommus:
> maxItems: 1
>
> power-domains:
> maxItems: 1
>
> phys:
> maxItems: 1
> description: phandle for the PHY port
>
> phy-names:
> const: dphy
>
> clocks:
> items:
> - description: ISP clock
> - description: ISP AXI clock clock
> - description: ISP AXI clock wrapper clock
> - description: ISP AHB clock clock
> - description: ISP AHB wrapper clock
>
> clock-names:
> items:
> - const: clk_isp
> - const: aclk_isp
> - const: aclk_isp_wrap
> - const: hclk_isp
> - const: hclk_isp_wrap
>
> # See ./video-interfaces.txt for details
> ports:
> type: object
> additionalProperties: false
>
> properties:
> "#address-cells":
> const: 1
>
> "#size-cells":
> const: 0
>
> port@0:
> type: object
> description: connection point for sensors at MIPI-DPHY RX0
> additionalProperties: false
Nothing required here?
>
> properties:
> "#address-cells":
> const: 1
>
> "#size-cells":
> const: 0
>
> reg:
> const: 0
>
> patternProperties:
> endpoint:
> type: object
> additionalProperties: false
>
> properties:
> reg:
> maxItems: 1
>
> data-lanes:
> minItems: 1
> maxItems: 4
>
> remote-endpoint: true
>
> required:
> - port@0
The use of '@0' makes "#address-cells" and "#size-cells" also a requirement.
- "#address-cells"
- "#size-cells"
>
> required:
> - compatible
How about 'reg'?
- reg
> - interrupts
> - clocks
> - clock-names
> - power-domains
> - iommus
> - phys
> - phy-names
> - ports
>
> additionalProperties: false
>
> examples:
> - |
>
> #include <dt-bindings/clock/rk3399-cru.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/power/rk3399-power.h>
>
> parent0: parent@0 {
> #address-cells = <2>;
> #size-cells = <2>;
>
> isp0: isp0@ff910000 {
> compatible = "rockchip,rk3399-cif-isp";
> reg = <0x0 0xff910000 0x0 0x4000>;
> interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH 0>;
> clocks = <&cru SCLK_ISP0>,
> <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
> <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
> clock-names = "clk_isp",
> "aclk_isp", "aclk_isp_wrap",
> "hclk_isp", "hclk_isp_wrap";
> power-domains = <&power RK3399_PD_ISP0>;
> iommus = <&isp0_mmu>;
> phys = <&dphy>;
> phy-names = "dphy";
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@0 {
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <0>;
>
> mipi_in_wcam: endpoint@0 {
> reg = <0>;
> remote-endpoint = <&wcam_out>;
> data-lanes = <1 2>;
> };
>
> mipi_in_ucam: endpoint@1 {
> reg = <1>;
> remote-endpoint = <&ucam_out>;
> data-lanes = <1>;
> };
> };
> };
> };
>
> i2c7: i2c@ff160000 {
> clock-frequency = <400000>;
> #address-cells = <1>;
> #size-cells = <0>;
Incomplete example.
From i2c-rk3x.yaml:
required:
- compatible
- reg
- interrupts
- clocks
- clock-names
>
> wcam: camera@36 {
> compatible = "ovti,ov5695";
> reg = <0x36>;
>
> port {
> wcam_out: endpoint {
> remote-endpoint = <&mipi_in_wcam>;
> data-lanes = <1 2>;
> };
> };
> };
>
> ucam: camera@3c {
> compatible = "ovti,ov2685";
> reg = <0x3c>;
>
> port {
> ucam_out: endpoint {
> remote-endpoint = <&mipi_in_ucam>;
> data-lanes = <1>;
> };
> };
> };
> };
> };
Hi Helen,
> # SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> %YAML 1.2
> ---
> $id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy-rx0.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings
>
> maintainers:
> - Helen Koike <[email protected]>
> - Ezequiel Garcia <[email protected]>
>
> 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-rx0
>
> reg:
> maxItems: 1
If 'reg' is not used => remove it.
>
> 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 D-PHY RX0 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>
>
> mipi_dphy_rx0: mipi-dphy-rx0 {
> compatible = "rockchip,rk3399-mipi-dphy-rx0";
> 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 Helen,
> From: Helen Koike <[email protected]>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 33cc21fcf4c10..fc0295d2a65a1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1394,6 +1394,17 @@ io_domains: io-domains {
> status = "disabled";
> };
>
> + mipi_dphy_rx0: mipi-dphy-rx0 {
For Heiko sort syscon@ff770000 subnodes alphabetical or reg value first?
> + compatible = "rockchip,rk3399-mipi-dphy-rx0";
> + clocks = <&cru SCLK_MIPIDPHY_REF>,
> + <&cru SCLK_DPHY_RX0_CFG>,
> + <&cru PCLK_VIO_GRF>;
Align ^
> + clock-names = "dphy-ref", "dphy-cfg", "grf";
> + power-domains = <&power RK3399_PD_VIO>;
> + #phy-cells = <0>;
> + status = "disabled";
> + };
> +
> u2phy0: usb2-phy@e450 {
> compatible = "rockchip,rk3399-usb2phy";
> reg = <0xe450 0x10>;
> --
> 2.26.0
Am Donnerstag, 2. April 2020, 15:48:02 CEST schrieb Johan Jonker:
> Hi Helen,
>
> > From: Helen Koike <[email protected]>
>
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > index 33cc21fcf4c10..fc0295d2a65a1 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > @@ -1394,6 +1394,17 @@ io_domains: io-domains {
> > status = "disabled";
> > };
> >
>
> > + mipi_dphy_rx0: mipi-dphy-rx0 {
>
> For Heiko sort syscon@ff770000 subnodes alphabetical or reg value first?
Similar to main nodes ... so things without reg alphabetical,
the rest by reg address
>
> > + compatible = "rockchip,rk3399-mipi-dphy-rx0";
> > + clocks = <&cru SCLK_MIPIDPHY_REF>,
>
> > + <&cru SCLK_DPHY_RX0_CFG>,
> > + <&cru PCLK_VIO_GRF>;
>
> Align ^
>
> > + clock-names = "dphy-ref", "dphy-cfg", "grf";
> > + power-domains = <&power RK3399_PD_VIO>;
> > + #phy-cells = <0>;
> > + status = "disabled";
> > + };
> > +
> > u2phy0: usb2-phy@e450 {
> > compatible = "rockchip,rk3399-usb2phy";
> > reg = <0xe450 0x10>;
>
>
Hi Johan,
On 4/2/20 9:16 AM, Johan Jonker wrote:
> Hi Helen,
>
>> # SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> %YAML 1.2
>> ---
>> $id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy-rx0.yaml#
>> $schema: http://devicetree.org/meta-schemas/core.yaml#
>>
>> title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings
>>
>> maintainers:
>> - Helen Koike <[email protected]>
>> - Ezequiel Garcia <[email protected]>
>>
>> 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-rx0
>>
>
>> reg:
>> maxItems: 1
>
> If 'reg' is not used => remove it.
ok, I'll add a patch removing it.
Thanks,
Helen
>
>>
>> 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 D-PHY RX0 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>
>>
>> mipi_dphy_rx0: mipi-dphy-rx0 {
>> compatible = "rockchip,rk3399-mipi-dphy-rx0";
>> 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 Johan,
Thanks for your review.
On 4/2/20 8:35 AM, Johan Jonker wrote:
> Hi Helen,
>
>> # SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> %YAML 1.2
>> ---
>> $id: http://devicetree.org/schemas/media/rockchip-isp1.yaml#
>> $schema: http://devicetree.org/meta-schemas/core.yaml#
>>
>
>> title: Rockchip SoC Image Signal Processing unit v1
>
> Where do we need 'v1' for? Is there a 'v2'?
ISPv1 is the Rockchip's name for the IP block.
>
>>
>> maintainers:
>> - Helen Koike <[email protected]>
>>
>> description: |
>> Rockchip ISP1 is the Camera interface for the Rockchip series of SoCs
>> which contains image processing, scaling, and compression functions.
>>
>> properties:
>> compatible:
>> const: rockchip,rk3399-cif-isp
>>
>> reg:
>> maxItems: 1
>>
>> interrupts:
>> maxItems: 1
>>
>> iommus:
>> maxItems: 1
>>
>> power-domains:
>> maxItems: 1
>>
>> phys:
>> maxItems: 1
>> description: phandle for the PHY port
>>
>> phy-names:
>> const: dphy
>>
>> clocks:
>> items:
>> - description: ISP clock
>> - description: ISP AXI clock clock
>> - description: ISP AXI clock wrapper clock
>> - description: ISP AHB clock clock
>> - description: ISP AHB wrapper clock
>>
>> clock-names:
>> items:
>> - const: clk_isp
>> - const: aclk_isp
>> - const: aclk_isp_wrap
>> - const: hclk_isp
>> - const: hclk_isp_wrap
>>
>> # See ./video-interfaces.txt for details
>> ports:
>> type: object
>> additionalProperties: false
>>
>> properties:
>> "#address-cells":
>> const: 1
>>
>> "#size-cells":
>> const: 0
>>
>> port@0:
>> type: object
>> description: connection point for sensors at MIPI-DPHY RX0
>
>> additionalProperties: false
>
> Nothing required here?
I was thinking that if there is no endpoint, then nothing is required.
But if there is, then #address-cells, #size-cells and reg are. I guess
I can just add them as required.
I'll add it in the patchseries.
>
>>
>> properties:
>> "#address-cells":
>> const: 1
>>
>> "#size-cells":
>> const: 0
>>
>> reg:
>> const: 0
>>
>> patternProperties:
>> endpoint:
>> type: object
>> additionalProperties: false
>>
>> properties:
>> reg:
>> maxItems: 1
>>
>> data-lanes:
>> minItems: 1
>> maxItems: 4
>>
>> remote-endpoint: true
>>
>> required:
>
>> - port@0
>
> The use of '@0' makes "#address-cells" and "#size-cells" also a requirement.
>
> - "#address-cells"
> - "#size-cells"
Ok, I'll add it.
>
>>
>> required:
>> - compatible
>
> How about 'reg'?
>
> - reg
ack, I'll add another patch in the series fixing this.
>
>> - interrupts
>> - clocks
>> - clock-names
>> - power-domains
>> - iommus
>> - phys
>> - phy-names
>> - ports
>>
>> additionalProperties: false
>>
>> examples:
>> - |
>>
>> #include <dt-bindings/clock/rk3399-cru.h>
>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>> #include <dt-bindings/power/rk3399-power.h>
>>
>> parent0: parent@0 {
>> #address-cells = <2>;
>> #size-cells = <2>;
>>
>> isp0: isp0@ff910000 {
>> compatible = "rockchip,rk3399-cif-isp";
>> reg = <0x0 0xff910000 0x0 0x4000>;
>> interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH 0>;
>> clocks = <&cru SCLK_ISP0>,
>> <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
>> <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
>> clock-names = "clk_isp",
>> "aclk_isp", "aclk_isp_wrap",
>> "hclk_isp", "hclk_isp_wrap";
>> power-domains = <&power RK3399_PD_ISP0>;
>> iommus = <&isp0_mmu>;
>> phys = <&dphy>;
>> phy-names = "dphy";
>>
>> ports {
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> port@0 {
>> #address-cells = <1>;
>> #size-cells = <0>;
>> reg = <0>;
>>
>> mipi_in_wcam: endpoint@0 {
>> reg = <0>;
>> remote-endpoint = <&wcam_out>;
>> data-lanes = <1 2>;
>> };
>>
>> mipi_in_ucam: endpoint@1 {
>> reg = <1>;
>> remote-endpoint = <&ucam_out>;
>> data-lanes = <1>;
>> };
>> };
>> };
>> };
>>
>
>> i2c7: i2c@ff160000 {
>> clock-frequency = <400000>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>
> Incomplete example.
> From i2c-rk3x.yaml:
>
> required:
> - compatible
> - reg
> - interrupts
> - clocks
> - clock-names
The idea was to exemplify how to connect to the sensor nodes below.
But I don't see a problem adding a complete i2c example, I'll add it.
Thanks
Helen
>
>>
>> wcam: camera@36 {
>> compatible = "ovti,ov5695";
>> reg = <0x36>;
>>
>> port {
>> wcam_out: endpoint {
>> remote-endpoint = <&mipi_in_wcam>;
>> data-lanes = <1 2>;
>> };
>> };
>> };
>>
>> ucam: camera@3c {
>> compatible = "ovti,ov2685";
>> reg = <0x3c>;
>>
>> port {
>> ucam_out: endpoint {
>> remote-endpoint = <&mipi_in_ucam>;
>> data-lanes = <1>;
>> };
>> };
>> };
>> };
>> };
On 4/2/20 4:31 PM, Heiko Stübner wrote:
> Am Donnerstag, 2. April 2020, 15:48:02 CEST schrieb Johan Jonker:
>> Hi Helen,
>>
>>> From: Helen Koike <[email protected]>
>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> index 33cc21fcf4c10..fc0295d2a65a1 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> @@ -1394,6 +1394,17 @@ io_domains: io-domains {
>>> status = "disabled";
>>> };
>>>
>>
>>> + mipi_dphy_rx0: mipi-dphy-rx0 {
>>
>> For Heiko sort syscon@ff770000 subnodes alphabetical or reg value first?
>
> Similar to main nodes ... so things without reg alphabetical,
> the rest by reg address
>
alphabetical first:
io-domains
mipi-dphy-rx0
usb2-phy@e450
.@..
or
with reg values first:
.@..
emmc_phy: phy@f780
mipi-dphy-rx0
pcie-phy
>
>>
>>> + compatible = "rockchip,rk3399-mipi-dphy-rx0";
>>> + clocks = <&cru SCLK_MIPIDPHY_REF>,
>>
>>> + <&cru SCLK_DPHY_RX0_CFG>,
>>> + <&cru PCLK_VIO_GRF>;
>>
>> Align ^
>>
>>> + clock-names = "dphy-ref", "dphy-cfg", "grf";
>>> + power-domains = <&power RK3399_PD_VIO>;
>>> + #phy-cells = <0>;
>>> + status = "disabled";
>>> + };
>>> +
>>> u2phy0: usb2-phy@e450 {
>>> compatible = "rockchip,rk3399-usb2phy";
>>> reg = <0xe450 0x10>;
>>
>>
>
>
>
>
Hi,
On 4/2/20 11:31 AM, Heiko Stübner wrote:
> Am Donnerstag, 2. April 2020, 15:48:02 CEST schrieb Johan Jonker:
>> Hi Helen,
>>
>>> From: Helen Koike <[email protected]>
>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> index 33cc21fcf4c10..fc0295d2a65a1 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> @@ -1394,6 +1394,17 @@ io_domains: io-domains {
>>> status = "disabled";
>>> };
>>>
>>
>>> + mipi_dphy_rx0: mipi-dphy-rx0 {
>>
>> For Heiko sort syscon@ff770000 subnodes alphabetical or reg value first?
>
> Similar to main nodes ... so things without reg alphabetical,
> the rest by reg address
>
>
>>
>>> + compatible = "rockchip,rk3399-mipi-dphy-rx0";
>>> + clocks = <&cru SCLK_MIPIDPHY_REF>,
>>
>>> + <&cru SCLK_DPHY_RX0_CFG>,
>>> + <&cru PCLK_VIO_GRF>;
>>
>> Align ^
ack.
Thanks
Helen
>>
>>> + clock-names = "dphy-ref", "dphy-cfg", "grf";
>>> + power-domains = <&power RK3399_PD_VIO>;
>>> + #phy-cells = <0>;
>>> + status = "disabled";
>>> + };
>>> +
>>> u2phy0: usb2-phy@e450 {
>>> compatible = "rockchip,rk3399-usb2phy";
>>> reg = <0xe450 0x10>;
>>
>>
>
>
>
>
Am Donnerstag, 2. April 2020, 16:37:52 CEST schrieb Johan Jonker:
> On 4/2/20 4:31 PM, Heiko St?bner wrote:
> > Am Donnerstag, 2. April 2020, 15:48:02 CEST schrieb Johan Jonker:
> >> Hi Helen,
> >>
> >>> From: Helen Koike <[email protected]>
> >>
> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> index 33cc21fcf4c10..fc0295d2a65a1 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> @@ -1394,6 +1394,17 @@ io_domains: io-domains {
> >>> status = "disabled";
> >>> };
> >>>
> >>
> >>> + mipi_dphy_rx0: mipi-dphy-rx0 {
> >>
> >> For Heiko sort syscon@ff770000 subnodes alphabetical or reg value first?
> >
> > Similar to main nodes ... so things without reg alphabetical,
> > the rest by reg address
> >
> alphabetical first:
>
> io-domains
> mipi-dphy-rx0
> usb2-phy@e450
like this ... aka similar to what we do in the core nodes.
For the record, pinctrl at the bottom of a soc.dtsi is ok.
Heiko
> .@..
>
> or
>
> with reg values first:
>
> .@..
> emmc_phy: phy@f780
> mipi-dphy-rx0
> pcie-phy
>
> >
> >>
> >>> + compatible = "rockchip,rk3399-mipi-dphy-rx0";
> >>> + clocks = <&cru SCLK_MIPIDPHY_REF>,
> >>
> >>> + <&cru SCLK_DPHY_RX0_CFG>,
> >>> + <&cru PCLK_VIO_GRF>;
> >>
> >> Align ^
> >>
> >>> + clock-names = "dphy-ref", "dphy-cfg", "grf";
> >>> + power-domains = <&power RK3399_PD_VIO>;
> >>> + #phy-cells = <0>;
> >>> + status = "disabled";
> >>> + };
> >>> +
> >>> u2phy0: usb2-phy@e450 {
> >>> compatible = "rockchip,rk3399-usb2phy";
> >>> reg = <0xe450 0x10>;
> >>
> >>
> >
> >
> >
> >
>
>
(+Kishon)
Hi Helen,
I was wondering if we couldn't also move the phy driver out of staging.
Thanks,
Ezequiel
On Wed, 2020-04-01 at 21:02 -0300, Helen Koike wrote:
> Move phy-rockchip-dphy-rx0 bindings to Documentation/devicetree/bindings/phy
>
> Signed-off-by: Helen Koike <[email protected]>
> ---
> .../devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml | 0
> 1 file changed, 0 insertions(+), 0 deletions(-)
> rename {drivers/staging/media/phy-rockchip-dphy-rx0/Documentation => Documentation}/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml (100%)
>
> diff --git a/drivers/staging/media/phy-rockchip-dphy-rx0/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
> b/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
> similarity index 100%
> rename from drivers/staging/media/phy-rockchip-dphy-rx0/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
> rename to Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
Hi Helen,
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index fc0295d2a65a1..815099a0cd0dd 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1718,6 +1718,33 @@ vopb_mmu: iommu@ff903f00 {
> status = "disabled";
> };
>
> + isp0: isp0@ff910000 {
> + compatible = "rockchip,rk3399-cif-isp";
> + reg = <0x0 0xff910000 0x0 0x4000>;
> + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru SCLK_ISP0>,
> + <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
> + <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
> + clock-names = "clk_isp",
> + "aclk_isp", "aclk_isp_wrap",
> + "hclk_isp", "hclk_isp_wrap";
> + power-domains = <&power RK3399_PD_ISP0>;
> + iommus = <&isp0_mmu>;
> + phys = <&mipi_dphy_rx0>;
> + phy-names = "dphy";
Maybe a little sort? But keep rest as it is. Also in example.
iommus = <&isp0_mmu>;
phys = <&mipi_dphy_rx0>;
phy-names = "dphy";
power-domains = <&power RK3399_PD_ISP0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
Move reg above #address-cells. Change that in example as well.
reg = <0>;
#address-cells = <1>;
#size-cells = <0>;
> + };
> + };
> + };
> +
> isp0_mmu: iommu@ff914000 {
> compatible = "rockchip,iommu";
> reg = <0x0 0xff914000 0x0 0x100>, <0x0 0xff915000 0x0 0x100>;
> --
> 2.26.0
On 4/2/20 2:20 PM, Johan Jonker wrote:
> Hi Helen,
>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index fc0295d2a65a1..815099a0cd0dd 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -1718,6 +1718,33 @@ vopb_mmu: iommu@ff903f00 {
>> status = "disabled";
>> };
>>
>> + isp0: isp0@ff910000 {
>> + compatible = "rockchip,rk3399-cif-isp";
>> + reg = <0x0 0xff910000 0x0 0x4000>;
>> + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH 0>;
>> + clocks = <&cru SCLK_ISP0>,
>> + <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
>> + <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
>> + clock-names = "clk_isp",
>> + "aclk_isp", "aclk_isp_wrap",
>> + "hclk_isp", "hclk_isp_wrap";
>
>> + power-domains = <&power RK3399_PD_ISP0>;
>> + iommus = <&isp0_mmu>;
>> + phys = <&mipi_dphy_rx0>;
>> + phy-names = "dphy";
>
> Maybe a little sort? But keep rest as it is. Also in example.
>
> iommus = <&isp0_mmu>;
> phys = <&mipi_dphy_rx0>;
> phy-names = "dphy";
> power-domains = <&power RK3399_PD_ISP0>;
Are you proposing only to move power-domains after phy? And keep the rest?
What is the main logic?
Thanks
Helen
>
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>
> Move reg above #address-cells. Change that in example as well.
>
> reg = <0>;
> #address-cells = <1>;
> #size-cells = <0>;
>
>> + };
>> + };
>> + };
>> +
>> isp0_mmu: iommu@ff914000 {
>> compatible = "rockchip,iommu";
>> reg = <0x0 0xff914000 0x0 0x100>, <0x0 0xff915000 0x0 0x100>;
>> --
>> 2.26.0
>
On 4/2/20 9:46 PM, Helen Koike wrote:
>
>
> On 4/2/20 2:20 PM, Johan Jonker wrote:
>> Hi Helen,
>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> index fc0295d2a65a1..815099a0cd0dd 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> @@ -1718,6 +1718,33 @@ vopb_mmu: iommu@ff903f00 {
>>> status = "disabled";
>>> };
>>>
>>> + isp0: isp0@ff910000 {
>>> + compatible = "rockchip,rk3399-cif-isp";
>>> + reg = <0x0 0xff910000 0x0 0x4000>;
>>> + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH 0>;
>>> + clocks = <&cru SCLK_ISP0>,
>>> + <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
>>> + <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
>>> + clock-names = "clk_isp",
>>> + "aclk_isp", "aclk_isp_wrap",
>>> + "hclk_isp", "hclk_isp_wrap";
>>
>>> + power-domains = <&power RK3399_PD_ISP0>;
>>> + iommus = <&isp0_mmu>;
>>> + phys = <&mipi_dphy_rx0>;
>>> + phy-names = "dphy";
>>
>> Maybe a little sort? But keep rest as it is. Also in example.
>>
>> iommus = <&isp0_mmu>;
>> phys = <&mipi_dphy_rx0>;
>> phy-names = "dphy";
>> power-domains = <&power RK3399_PD_ISP0>;
>
> Are you proposing only to move power-domains after phy? And keep the rest?
> What is the main logic?
There is no hard rule... It mostly depend on Heiko...
For nodes:
Sort things without reg alphabetical first,
then sort the rest by reg address.
Inside nodes:
If exists on top: compatible, reg and interrupts.
In alphabetical order the required properties.
Then in alphabetical order the other properties.
And as last things that start with '#' in alphabetical order.
>
> Thanks
> Helen
>
>>
>>> +
>>> + ports {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + port@0 {
>>
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + reg = <0>;
>>
>> Move reg above #address-cells. Change that in example as well.
>>
>> reg = <0>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>>> + };
>>> + };
>>> + };
>>> +
>>> isp0_mmu: iommu@ff914000 {
>>> compatible = "rockchip,iommu";
>>> reg = <0x0 0xff914000 0x0 0x100>, <0x0 0xff915000 0x0 0x100>;
>>> --
>>> 2.26.0
>>
On 4/2/20 1:17 PM, Ezequiel Garcia wrote:
> (+Kishon)
>
> Hi Helen,
>
> I was wondering if we couldn't also move the phy driver out of staging.
I think we can, let's move it.
Regards,
Helen
>
> Thanks,
> Ezequiel
>
> On Wed, 2020-04-01 at 21:02 -0300, Helen Koike wrote:
>> Move phy-rockchip-dphy-rx0 bindings to Documentation/devicetree/bindings/phy
>>
>> Signed-off-by: Helen Koike <[email protected]>
>> ---
>> .../devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml | 0
>> 1 file changed, 0 insertions(+), 0 deletions(-)
>> rename {drivers/staging/media/phy-rockchip-dphy-rx0/Documentation => Documentation}/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml (100%)
>>
>> diff --git a/drivers/staging/media/phy-rockchip-dphy-rx0/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
>> b/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
>> similarity index 100%
>> rename from drivers/staging/media/phy-rockchip-dphy-rx0/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
>> rename to Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
>
>
>