Skip to content

drivers: video: ov7670: Add support for PCLK free-run mode #92645

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thaoluonguw
Copy link
Contributor

  • Add a Kconfig CONFIG_VIDEO_OV7670_PCLK_FREE_RUN for OV7670

  • Update the OV7670 driver’s initialization register array

    • COM10 = 0x00 when CONFIG_VIDEO_OV7670_PCLK_FREE_RUN=y

    • COM10 = 0x20 when disabled

This update supports for peripherals which requiring an uninterrupted clock (ex. Renesas CEU)

Add a Kconfig boolean CONFIG_VIDEO_OV7670_PCLK_FREE_RUN
Update the OV7670 driver’s initialization register array
 - COM10 = 0x00 when CONFIG_VIDEO_OV7670_PCLK_FREE_RUN=y
 - COM10 = 0x20 when disabled

Apply this to guarantee continuous PCLK for Renesas RA CEU
and other peripherals requiring an uninterrupted clock

Signed-off-by: Khanh Nguyen <[email protected]>
Copy link

sonarqubecloud bot commented Jul 3, 2025

Copy link
Contributor

@josuah josuah left a comment

Choose a reason for hiding this comment

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

Thank you for this change! This will help adapt the sensor to diverse hardware.

It Linux, there is ov7670,pclk-hb-disable used for this as it is not a standard name found in (video-interfaces.yaml).

It is defined here in Linux ovti,ov7670.txt making it free-runing by default.

In Zephyr, would it be interesting to implement it as a devicetree boolean in ovti,ov7670.yaml?

For instance:

  ov7670,pclk-hb-disable:
    type: boolean
    description: |
      Set COM10 bit 5 to make the pixel clock (PCLK) to be gated during H-blank.
      By default, let PCLK free-run continuously even during horizontal or vertical
      blanking.

Then on the init, before/after loading ov7670_init_regtbl, something like this:

        if (config->pclk_hb_disable) {
                /* read modify write */
        }

Which assumes pclk_hb_disable added (i.e. as a boolean or bitfield option) to struct ov7670_config and then at the bottom as well in const struct ov7670_config ov7670_config_##inst = {...};.

This is a bit more effort but allow to keep all the data format related configurations of sensosr in one place (devicetree).

Thank you again for upstreaming these changes!

@avolmat-st
Copy link

Thanks for the patch. I also think that if Linux already define a device-tree binding to control this, it is better to also implement it.

@ngphibang
Copy link
Contributor

In Zephyr, would it be interesting to implement it as a devicetree boolean in ovti,ov7670.yaml ?

I have the same thought, this is HW-oriented so better to be in devicetree.

@avolmat-st
Copy link

Based on PR #84229 there were actually thought about having it ALWAYS in free running mode. In such case we wouldn't even have to introduce a device-tree property or a Kconfig. Are we sure we still need a non free running mode ?

If bit 5 doesn't have to be set, then the question is simply about the polarity of the signals, (LSBs of that register). In this PR it is set to 0x00 while in PR #94354 it is set to 0x03.

Proper would be to have this done via the endpoint properties and set the register based on properties read from the device-tree.
If you prefer this can be done in another PR and for the time being we'd first have to see if either 0x00 or 0x03 can work for everybody. If one conf is ok for all then we can apply it, if no then endpoint device-tree readying become mandatory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Video Video subsystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants