On Wed, Apr 1, 2020 at 5:00 PM Helen Koike <helen.koike@xxxxxxxxxxxxx> wrote: > > Hi Karthik, > > It is nice to see this driver being used and tested elsewhere. > > How did you tested the series? It would be nice to add it in the commit message. > Firstly thank you for your work on the rkisp1 and libcamera and efforts of bringing it to mainline kernel. I tested the patch series using yocto build with meta-rockchip and using media kernel tree TOT. I also used libcamera to configure the pipeline (since it was more convenient as compared to media-ctl commands). Sure I will add them to the commit message in my next commit. Would this commit be the best place to include testing instructions or include into all 4 commits that I posted ? > On 3/31/20 4:57 AM, karthik poduval wrote: > > ov5647 is one of the two camera modules as described in > > https://tinkerboarding.co.uk/wiki/index.php/CSI-camera > > > > changes ported over from https://github.com/TinkerBoard/debian_kernel.git > > > > Reported-by: Karthik Poduval <karthik.poduval@xxxxxxxxx> > > Signed-off-by: Karthik Poduval <karthik.poduval@xxxxxxxxx> > > please, see my comments from previous commit. > > > --- > > arch/arm/boot/dts/rk3288-tinker.dtsi | 37 ++++++++++++++++++++++++++++ > > I wondering if changing thinkerboard dts make sense. Is the camera really hardwired > on the tinker board, or is it an add-on? > > Regards, > Helen No the camera isn't hardwired but connects over a flex cable to an adapter board, however based on my search I found that only 2 camera adapter boards have been made for tinkerboard, IMX219 and OV5647. The only camera adapter board I have with me is ov5647. Normally offboard peripherals also need device tree entries. The tinkerbaord debain 4.4 kernel includes imx219 device entry in the device tree and provides ov5647 device tree entry as an overlay. What is the recommendation here ? Should it be an overlay or not added at all ? > > > 1 file changed, 37 insertions(+) > > > > diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi > > b/arch/arm/boot/dts/rk3288-tinker.dtsi > > index 312582c1bd37..720dd80ea3aa 100644 > > --- a/arch/arm/boot/dts/rk3288-tinker.dtsi > > +++ b/arch/arm/boot/dts/rk3288-tinker.dtsi > > @@ -107,6 +107,13 @@ > > startup-delay-us = <100000>; > > vin-supply = <&vcc_io>; > > }; > > + > > + ext_cam_clk: external-camera-clock { > > + compatible = "fixed-clock"; > > + clock-frequency = <25000000>; > > + clock-output-names = "CLK_CAMERA_25MHZ"; > > + #clock-cells = <0>; > > + }; > > }; > > > > &cpu0 { > > @@ -345,12 +352,42 @@ > > > > &i2c2 { > > status = "okay"; > > + camera0: ov5647@36 { > > + compatible = "ovti,ov5647"; > > + reg = <0x36>; > > + clocks = <&ext_cam_clk>; > > + status = "okay"; > > + enable-gpios = <&gpio2 0 GPIO_ACTIVE_LOW>; > > + port { > > + ov5647_out: endpoint { > > + remote-endpoint = <&isp_mipi_in>; > > + data-lanes = <1 2>; > > + }; > > + }; > > + }; > > }; > > > > &i2c5 { > > status = "okay"; > > }; > > > > +&isp { > > + status = "okay"; > > + phys = <&mipi_phy_rx0>; > > + phy-names = "dphy"; > > + > > + port { > > + isp_mipi_in: endpoint { > > + remote-endpoint = <&ov5647_out>; > > + data-lanes = <1 2>; > > + }; > > + }; > > +}; > > + > > +&mipi_phy_rx0 { > > + status = "okay"; > > +}; > > + > > &i2s { > > #sound-dai-cells = <0>; > > status = "okay"; > > > -- Regards, Karthik Poduval
