Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

boards: shields: Add shield for NXP's ov5640 camera modules #72434

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

ngphibang
Copy link
Contributor

@ngphibang ngphibang commented May 7, 2024

This PR adds a shield for NXP's ov5640 camera modules which use a 44-pins board-to-board connector and MIPI CSI or DVP (parallel) inteface. These camera modules are made specifically for and provided together with NXP i.MX RT11xx EVK boards.

It is split from #69810 to ease the review process.

It already got some review from @danieldegrasse.

boards/shields/wuxi_ov5640/doc/index.rst Outdated Show resolved Hide resolved
boards/shields/wuxi_ov5640/wuxi_ov5640.overlay Outdated Show resolved Hide resolved
boards/shields/wuxi_ov5640/wuxi_ov5640.overlay Outdated Show resolved Hide resolved
@ngphibang
Copy link
Contributor Author

@danieldegrasse : Is it ok for you now ?
BTW, the PR is dependent on #71854 and #71859 and should be merged after them

@ngphibang
Copy link
Contributor Author

As requested by @decsny and @dleach02, moved the commit that enables the video feature on RT1170 here as this PR can be considered as the last one which complete the feature.

Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as #72435 - I don't understand why the shield is being made nxp specific ; there should be a generic definition for the 42-pin connector ; see https://docs.zephyrproject.org/latest/hardware/porting/shields.html#gpio-nexus-nodes

@danieldegrasse
Copy link
Collaborator

@kartben what we have done in the past to handle (somewhat bespoke) connectors NXP has dreamed up is define a GPIO nexus node like nxp_mipi_connector: https://github.com/zephyrproject-rtos/zephyr/blob/master/boards/shields/rk055hdmipi4m/rk055hdmipi4m.overlay#L32. Would that work here?

@kartben
Copy link
Collaborator

kartben commented May 22, 2024

@kartben what we have done in the past to handle (somewhat bespoke) connectors NXP has dreamed up is define a GPIO nexus node like nxp_mipi_connector: master/boards/shields/rk055hdmipi4m/rk055hdmipi4m.overlay#L32. Would that work here?

yes, that would be great :)

@@ -0,0 +1,68 @@
.. _wuxi_ov5640:

WUXI_OV5640 Camera Module
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding the same comment I just made on #72435 here too -- if this module is in fact NXP specific and not just a generic module with a 42-pin connector for the OV5640 one would get off Aliexpress like I initially assumed, then the README (and potentially the actual name of the shield) needs to reflect this better. If you also have a picture of the module and a link where people can order it and/or get the datasheet that would be even better to avoid any ambiguities.

Copy link
Contributor Author

@ngphibang ngphibang May 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kartben Thanks for your comment. This camera module is a 44-pins board-to-board connector (e.g. Hirose DF40 series) provided together with the NXP's RT1170 EVK / EVKB and it seems we couldn't order it elsewhere.
After searching on the internet, it appears that this shield is NXP specific and not just a generic module. There is a similar camera module I found here but the pinouts are different.

Here are pictures of the camera module and the camera connector on i.MX RT1170 EVK.
PXL_20240527_165347401
PXL_20240527_165335359
PXL_20240527_154508867

I reworked the PR as your suggestions.

@ngphibang ngphibang requested a review from kartben May 27, 2024 19:44
@ngphibang ngphibang changed the title boards: shields: Add shield for NXP's iMX RT11xx camera modules boards: shields: Add shield for NXP's ov5640 camera modules May 30, 2024
danieldegrasse
danieldegrasse previously approved these changes May 30, 2024
danieldegrasse
danieldegrasse previously approved these changes May 30, 2024
@ngphibang
Copy link
Contributor Author

ngphibang commented May 30, 2024

Forgot to change nxp_btb44_cam to nxp_btb44_ov5640 when references to the shield in boards/nxp/mimxrt1170_evk/doc/index.rst. Just fixed it. Sorry

Comment on lines 63 to 77
/*
* This node describes the GPIO pins mapping of the camera
* connector, J2 on the EVK. This camera interface is supported
* on several NXP RT11xx EVKs, and is used with several camera
* modules available as zephyr shields, e.g. ov5640 module
*/
nxp_cam_connector: cam-connector {
compatible = "gpio-nexus";
#gpio-cells = <2>;
gpio-map-mask = <0xffffffff 0xffffffc0>;
gpio-map-pass-thru = <0 0x3f>;
gpio-map = <9 0 &gpio11 15 0>, /* Pin 9, RESETB */
<17 0 &gpio9 25 0>; /* Pin 17, PWDN */
};
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this will also be used for "several NXP RT11xx EVKs" (ex. RT1160 EVK as the exact same connector AFAICT), it would be better to define the GPIO nexus as its own compatible in dts/bindings/gpio (Iike e.g. arduino-header-r3 etc.). This is also an opportunity to document all the pins of the connector, not just 9 and 17 (again, see e.g arduino-header-r3 description for an example)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, done

Overview
********

This shield supports ov5640 camera modules which use a 44-pins board-to-board connector and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This shield supports ov5640 camera modules which use a 44-pins board-to-board connector and
This shield supports ov5640 camera modules which use a 44-pin board-to-board connector and

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, fixed

boards/shields/nxp_btb44_ov5640/doc/index.rst Show resolved Hide resolved
Requirements
************

This shield can only be used with a board which provides a 44-pins board-to-board
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This shield can only be used with a board which provides a 44-pins board-to-board
This shield can only be used with a board which provides a 44-pin board-to-board

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, fixed

Comment on lines +18 to +108
+----------------------+--------------------+
| Camera Connector Pin | Function |
+======================+====================+
| 1 | AGND |
+----------------------+--------------------+
| 2 | AF_GND |
+----------------------+--------------------+
| 3 | STROBE |
+----------------------+--------------------+
| 4 | AF_VCC |
+----------------------+--------------------+
| 5 | SDA |
+----------------------+--------------------+
| 6 | VCMSINK |
+----------------------+--------------------+
| 7 | SCL |
+----------------------+--------------------+
| 8 | AVDD |
+----------------------+--------------------+
| 9 | RESETB |
+----------------------+--------------------+
| 10 | GPIO1 |
+----------------------+--------------------+
| 11 | PCLK |
+----------------------+--------------------+
| 12 | GPIO0 |
+----------------------+--------------------+
| 13 | VSYNC |
+----------------------+--------------------+
| 14 | FREX |
+----------------------+--------------------+
| 15 | HREF |
+----------------------+--------------------+
| 16 | MIPI_CSI_DP1 / D9 |
+----------------------+--------------------+
| 17 | PWDN |
+----------------------+--------------------+
| 18 | MIPI_CSI_DN1 / D8 |
+----------------------+--------------------+
| 19 | MIPI_CSI_DP1 / D9 |
+----------------------+--------------------+
| 20 | DGND |
+----------------------+--------------------+
| 21 | MIPI_CSI_DN1 / D8 |
+----------------------+--------------------+
| 22 | MIPI_CSI_CLKP / D7 |
+----------------------+--------------------+
| 23 | MIPI_CSI_CLKP / D7 |
+----------------------+--------------------+
| 24 | MIPI_CSI_CLKN / D6 |
+----------------------+--------------------+
| 25 | MIPI_CSI_CLKN / D6 |
+----------------------+--------------------+
| 26 | DGND |
+----------------------+--------------------+
| 27 | MIPI_CSI_DP0 / D5 |
+----------------------+--------------------+
| 28 | MIPI_CSI_DP0 / D5 |
+----------------------+--------------------+
| 29 | MIPI_CSI_DN0 / D4 |
+----------------------+--------------------+
| 30 | MIPI_CSI_DN0 / D4 |
+----------------------+--------------------+
| 31 | D3 |
+----------------------+--------------------+
| 32 | DGND |
+----------------------+--------------------+
| 33 | D2 |
+----------------------+--------------------+
| 34 | XCLK |
+----------------------+--------------------+
| 35 | D1 |
+----------------------+--------------------+
| 36 | DVDD |
+----------------------+--------------------+
| 37 | D0 |
+----------------------+--------------------+
| 38 | DOVDD |
+----------------------+--------------------+
| 39 | DGND |
+----------------------+--------------------+
| 40 | DGND |
+----------------------+--------------------+
| 41 | GND |
+----------------------+--------------------+
| 42 | GND |
+----------------------+--------------------+
| 43 | GND |
+----------------------+--------------------+
| 44 | AF_GND |
+----------------------+--------------------+
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would probably make sense to also add in the documentation of the new binding for nexus (again, see e.g. the description field of arduino-header-r3 as an example)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, done

Add a shield for NXP ov5640 camera modules. This shield uses a 44-pin
board-to-board connector which is supported on NXP RT1170 and RT1160 EVKs.

Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
Enable video feature on cm7 evk and evkb which are tested with ov5640
camera.

Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
@dleach02 dleach02 added this to the v3.7.0 milestone Jun 6, 2024
@dleach02
Copy link
Member

@kartben can you return to this PR and confirm that @ngphibang has addressed your change requests?

@jhedberg jhedberg merged commit 59af15a into zephyrproject-rtos:main Jun 11, 2024
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Shields Shields (add-on boards) platform: NXP Drivers NXP Semiconductors, drivers platform: NXP NXP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants