Hi Tomasz, On 9/26/20 10:00 AM, Tomasz Figa wrote: > Hi Helen, > > On Wed, Jul 22, 2020 at 12:55:32PM -0300, Helen Koike wrote: >> From: Shunqian Zheng <zhengsq@xxxxxxxxxxxxxx> >> >> RK3399 has two ISPs, but only isp0 was tested. >> 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 <zhengsq@xxxxxxxxxxxxxx> >> Signed-off-by: Jacob Chen <jacob2.chen@xxxxxxxxxxxxxx> >> Signed-off-by: Helen Koike <helen.koike@xxxxxxxxxxxxx> >> >> --- >> >> V4: >> - update clock names >> >> V3: >> - clean up clocks >> >> V2: >> - re-order power-domains property >> >> V1: >> 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 | 25 ++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> index dba9641947a3a..ed8ba75dbbce8 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> @@ -1721,6 +1721,31 @@ 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_WRAPPER>, >> + <&cru HCLK_ISP0_WRAPPER>; >> + clock-names = "isp", "aclk", "hclk"; >> + iommus = <&isp0_mmu>; >> + phys = <&mipi_dphy_rx0>; >> + phy-names = "dphy"; >> + power-domains = <&power RK3399_PD_ISP0>; > > Should this have status = "disabled" too? The mipi_dphy_rx0 node is > disabled by default too, so in the default configuration the driver > would always fail to probe. I'm thinking what is the overall guideline here. Since isp and mipi_dphy are always present in the rk3399, shouldn't they always be enabled? Or since they are only useful if a sensor is present, we should let the dts of the board to enable it? Thanks Helen > > Best regards, > Tomasz >
