| Message ID | 20220525145833.1165437-1-foss+kernel@0leil.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5,1/4] media: dt-bindings: ov5675: document YAML binding | expand |
Hi Jacopo, On 5/31/22 15:16, Jacopo Mondi wrote: > Hi Quentin, > one more question > > On Wed, May 25, 2022 at 04:58:31PM +0200, Quentin Schulz wrote: >> From: Quentin Schulz <quentin.schulz@theobroma-systems.com> >> >> Until now, this driver only supported ACPI. This adds support for >> Device Tree too while enabling clock and regulators in runtime PM. >> >> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com> >> --- >> >> v5: >> - fixed -Wdeclaration-after-statement for delay_us, >> >> v4: >> - added delays based on clock cycles as specified in datasheet for >> pre-power-off and post-power-on, >> - re-arranged clk handling, shutdown toggling and regulator handling to >> better match power up/down sequence defined in datasheet, >> - added comment on need for regulator being stable before releasing >> shutdown pin, >> >> v3: >> - added linux/mod_devicetable.h include, >> - moved delay for reset pulse right after the regulators are enabled, >> - removed check on is_acpi_node in favor of checks on presence of OF >> properties (e.g. devm_clk_get_optional returns NULL), >> - moved power management out of system suspend/resume into runtime PM >> callbacks, >> - removed ACPI specific comment since it's not specific to this driver, >> - changed devm_clk_get to devm_clk_get_optional, >> - remove OF use of clock-frequency (handled by devm_clk_get_optional >> directly), >> - removed name of clock (only one, so no need for anything explicit) >> when requesting a clock from OF, >> - wrapped lines to 80 chars, >> >> v2: >> - fixed unused-const-variable warning by removing of_match_ptr in >> of_match_table, reported by kernel test robot, >> >> drivers/media/i2c/ov5675.c | 149 +++++++++++++++++++++++++++++++------ >> 1 file changed, 128 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/media/i2c/ov5675.c b/drivers/media/i2c/ov5675.c >> index 82ba9f56baec8..ea801edb8e408 100644 >> --- a/drivers/media/i2c/ov5675.c >> +++ b/drivers/media/i2c/ov5675.c >> @@ -3,10 +3,14 @@ >> >> #include <asm/unaligned.h> >> #include <linux/acpi.h> >> +#include <linux/clk.h> >> #include <linux/delay.h> >> +#include <linux/gpio/consumer.h> >> #include <linux/i2c.h> >> +#include <linux/mod_devicetable.h> >> #include <linux/module.h> >> #include <linux/pm_runtime.h> >> +#include <linux/regulator/consumer.h> >> #include <media/v4l2-ctrls.h> >> #include <media/v4l2-device.h> >> #include <media/v4l2-fwnode.h> >> @@ -17,7 +21,7 @@ >> >> #define OV5675_LINK_FREQ_450MHZ 450000000ULL >> #define OV5675_SCLK 90000000LL >> -#define OV5675_MCLK 19200000 >> +#define OV5675_XVCLK_19_2 19200000 >> #define OV5675_DATA_LANES 2 >> #define OV5675_RGB_DEPTH 10 >> >> @@ -76,6 +80,14 @@ >> >> #define to_ov5675(_sd) container_of(_sd, struct ov5675, sd) >> >> +static const char * const ov5675_supply_names[] = { >> + "avdd", /* Analog power */ >> + "dovdd", /* Digital I/O power */ >> + "dvdd", /* Digital core power */ >> +}; >> + >> +#define OV5675_NUM_SUPPLIES ARRAY_SIZE(ov5675_supply_names) >> + >> enum { >> OV5675_LINK_FREQ_900MBPS, >> }; >> @@ -484,6 +496,9 @@ struct ov5675 { >> struct v4l2_subdev sd; >> struct media_pad pad; >> struct v4l2_ctrl_handler ctrl_handler; >> + struct clk *xvclk; >> + struct gpio_desc *reset_gpio; >> + struct regulator_bulk_data supplies[OV5675_NUM_SUPPLIES]; >> >> /* V4L2 Controls */ >> struct v4l2_ctrl *link_freq; >> @@ -944,6 +959,56 @@ static int ov5675_set_stream(struct v4l2_subdev *sd, int enable) >> return ret; >> } >> >> +static int ov5675_power_off(struct device *dev) > > Does this (and power_on) require __maybe_unused to avoid a warning > when compiling without CONFIG_PM support ? Have you tried that ? > It does not, because they are called manually in the probe function and its error path (but thanks for the heads up, because I definitely forgot about this one). I received some review outside of this mailing list telling me the error path is incorrect for pm_runtime so I'll have a look at it before sending a v6. Cheers, Quentin
diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml new file mode 100644 index 0000000000000..f0a48707bed77 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml @@ -0,0 +1,123 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright (c) 2022 Theobroma Systems Design und Consulting GmbH +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5675.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Omnivision OV5675 CMOS Sensor + +maintainers: + - Quentin Schulz <quentin.schulz@theobroma-systems.com> + +allOf: + - $ref: /schemas/media/video-interface-devices.yaml# + +description: | + The Omnivision OV5675 is a high performance, 1/5-inch, 5 megapixel, CMOS + image sensor that delivers 2592x1944 at 30fps. It provides full-frame, + sub-sampled, and windowed 10-bit MIPI images in various formats via the + Serial Camera Control Bus (SCCB) interface. + + This chip is programmable through I2C and two-wire SCCB. The sensor output + is available via CSI-2 serial data output (up to 2-lane). + +properties: + compatible: + const: ovti,ov5675 + + reg: + maxItems: 1 + + clocks: + description: + System input clock (aka XVCLK). From 6 to 27 MHz. + maxItems: 1 + + dovdd-supply: + description: + Digital I/O voltage supply, 1.8 volts. + + avdd-supply: + description: + Analog voltage supply, 2.8 volts. + + dvdd-supply: + description: + Digital core voltage supply, 1.2 volts. + + reset-gpios: + description: + The phandle and specifier for the GPIO that controls sensor reset. + This corresponds to the hardware pin XSHUTDN which is physically + active low. + maxItems: 1 + + port: + $ref: /schemas/graph.yaml#/$defs/port-base + additionalProperties: false + + properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + properties: + data-lanes: + minItems: 1 + maxItems: 2 + + # Supports max data transfer of 900 Mbps per lane + link-frequencies: true + +required: + - compatible + - reg + - clocks + - dovdd-supply + - avdd-supply + - dvdd-supply + - port + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/px30-cru.h> + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/pinctrl/rockchip.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ov5675: camera@36 { + compatible = "ovti,ov5675"; + reg = <0x36>; + + reset-gpios = <&gpio2 RK_PB1 GPIO_ACTIVE_LOW>; + pinctrl-names = "default"; + pinctrl-0 = <&cif_clkout_m0>; + + clocks = <&cru SCLK_CIF_OUT>; + assigned-clocks = <&cru SCLK_CIF_OUT>; + assigned-clock-rates = <19200000>; + + avdd-supply = <&vcc_1v8>; + dvdd-supply = <&vcc_1v2>; + dovdd-supply = <&vcc_2v8>; + + rotation = <90>; + orientation = <0>; + + port { + ucam_out: endpoint { + remote-endpoint = <&mipi_in_ucam>; + data-lanes = <1 2>; + link-frequencies = /bits/ 64 <450000000>; + }; + }; + }; + }; +... + diff --git a/MAINTAINERS b/MAINTAINERS index f468864fd268c..549cdec4faaf3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14551,6 +14551,7 @@ M: Shawn Tu <shawnx.tu@intel.com> L: linux-media@vger.kernel.org S: Maintained T: git git://linuxtv.org/media_tree.git +F: Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml F: drivers/media/i2c/ov5675.c OMNIVISION OV5693 SENSOR DRIVER