* [PATCH 4/4] ARM: dts: rockchip: add ov5647 camera module support to tinkerboard
@ 2020-03-31 7:57 karthik poduval
2020-04-01 23:59 ` Helen Koike
0 siblings, 1 reply; 5+ messages in thread
From: karthik poduval @ 2020-03-31 7:57 UTC (permalink / raw)
To: Linux Media Mailing List
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@gmail.com>
Signed-off-by: Karthik Poduval <karthik.poduval@gmail.com>
---
arch/arm/boot/dts/rk3288-tinker.dtsi | 37 ++++++++++++++++++++++++++++
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";
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 4/4] ARM: dts: rockchip: add ov5647 camera module support to tinkerboard
2020-03-31 7:57 [PATCH 4/4] ARM: dts: rockchip: add ov5647 camera module support to tinkerboard karthik poduval
@ 2020-04-01 23:59 ` Helen Koike
2020-04-02 2:29 ` karthik poduval
0 siblings, 1 reply; 5+ messages in thread
From: Helen Koike @ 2020-04-01 23:59 UTC (permalink / raw)
To: karthik poduval, Linux Media Mailing List
Cc: Ezequiel Garcia, Dafna Hirschfeld
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.
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@gmail.com>
> Signed-off-by: Karthik Poduval <karthik.poduval@gmail.com>
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
> 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";
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 4/4] ARM: dts: rockchip: add ov5647 camera module support to tinkerboard
2020-04-01 23:59 ` Helen Koike
@ 2020-04-02 2:29 ` karthik poduval
2020-04-02 14:46 ` Helen Koike
0 siblings, 1 reply; 5+ messages in thread
From: karthik poduval @ 2020-04-02 2:29 UTC (permalink / raw)
To: Helen Koike; +Cc: Linux Media Mailing List, Ezequiel Garcia, Dafna Hirschfeld
On Wed, Apr 1, 2020 at 5:00 PM Helen Koike <helen.koike@collabora.com> 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@gmail.com>
> > Signed-off-by: Karthik Poduval <karthik.poduval@gmail.com>
>
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 4/4] ARM: dts: rockchip: add ov5647 camera module support to tinkerboard
2020-04-02 2:29 ` karthik poduval
@ 2020-04-02 14:46 ` Helen Koike
2020-04-02 16:30 ` karthik poduval
0 siblings, 1 reply; 5+ messages in thread
From: Helen Koike @ 2020-04-02 14:46 UTC (permalink / raw)
To: karthik poduval
Cc: Linux Media Mailing List, Ezequiel Garcia, Dafna Hirschfeld
On 4/1/20 11:29 PM, karthik poduval wrote:
> On Wed, Apr 1, 2020 at 5:00 PM Helen Koike <helen.koike@collabora.com> 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 ?
Nice, I'm glad it worked fine.
I think you could just quickly mention in the commits something like:
"Tested streaming on a tinkerboard with ov5647 with libcamera command: cam ..."
>> 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@gmail.com>
>>> Signed-off-by: Karthik Poduval <karthik.poduval@gmail.com>
>>
>> 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 ?
So usually this change don't go to the board's dtsi, otherwise you are
mandating other users to use the same sensor as you did.
I'm not sure how would be the alternative though, since overlays don't usually go upstream,
and I'm not sure if there is a way to declare a "expected" node.
Regards,
Helen
>>
>>> 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";
>>>
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 4/4] ARM: dts: rockchip: add ov5647 camera module support to tinkerboard
2020-04-02 14:46 ` Helen Koike
@ 2020-04-02 16:30 ` karthik poduval
0 siblings, 0 replies; 5+ messages in thread
From: karthik poduval @ 2020-04-02 16:30 UTC (permalink / raw)
To: Helen Koike; +Cc: Linux Media Mailing List, Ezequiel Garcia, Dafna Hirschfeld
On Thu, Apr 2, 2020 at 7:46 AM Helen Koike <helen.koike@collabora.com> wrote:
>
>
>
> On 4/1/20 11:29 PM, karthik poduval wrote:
> > On Wed, Apr 1, 2020 at 5:00 PM Helen Koike <helen.koike@collabora.com> 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 ?
>
> Nice, I'm glad it worked fine.
>
> I think you could just quickly mention in the commits something like:
>
> "Tested streaming on a tinkerboard with ov5647 with libcamera command: cam ..."
>
Thanks will do.
> >> 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@gmail.com>
> >>> Signed-off-by: Karthik Poduval <karthik.poduval@gmail.com>
> >>
> >> 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 ?
>
> So usually this change don't go to the board's dtsi, otherwise you are
> mandating other users to use the same sensor as you did.
>
> I'm not sure how would be the alternative though, since overlays don't usually go upstream,
> and I'm not sure if there is a way to declare a "expected" node.
>
> Regards,
> Helen
Agree so let me abandon this commit then, the documentation devicetree
bindings example should help with anyone who is trying to bring up an
image sensor. For reference sake I am thinking of putting up my ov5647
and tinkerbaord changes to a custom yocto layer which would make it
that configuration (tinkerbaord + ov5647) specific.
>
> >>
> >>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-04-02 16:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-31 7:57 [PATCH 4/4] ARM: dts: rockchip: add ov5647 camera module support to tinkerboard karthik poduval
2020-04-01 23:59 ` Helen Koike
2020-04-02 2:29 ` karthik poduval
2020-04-02 14:46 ` Helen Koike
2020-04-02 16:30 ` karthik poduval
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.