2022-07-13 15:07:17

by Tommaso Merciai

[permalink] [raw]
Subject: [PATCH v7 0/6] media: ov5693: cleanup code and add dts support

Hi All,
This series cleanup code on ov5693 driver and bring up dts support, also add
documentation for ov5693 camera sensor

Inspired by recently Quentin series:

- https://patchwork.kernel.org/project/linux-media/list/?series=64807

Tommaso Merciai (6):
media: ov5693: count num_supplies using array_size
media: ov5693: add dvdd into ov5693_supply_names array
media: ov5693: rename clk into xvclk
media: ov5693: add support for acpi clock-frequency prop
media: dt-bindings: ov5693: document YAML binding
media: ov5693: add ov5693_of_match, dts support

.../bindings/media/i2c/ovti,ov5693.yaml | 124 ++++++++++++++++++
MAINTAINERS | 1 +
drivers/media/i2c/ov5693.c | 57 +++++---
3 files changed, 163 insertions(+), 19 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml

--
2.25.1


2022-07-13 15:07:17

by Tommaso Merciai

[permalink] [raw]
Subject: [PATCH v7 1/6] media: ov5693: count num_supplies using array_size

Instead of hardcode OV5693_NUM_SUPPLIES in a define is better use
ARRAY_SIZE function to count the number of supplies from
ov5693_supply_names array

Signed-off-by: Tommaso Merciai <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
Reviewed-by: Daniel Scally <[email protected]>
---
Changes since v3:
- Add reviewed-by tag, suggested by Jacopo, Krzysztof

Changes since v4:
- Add Daniel reviewed-by tag

drivers/media/i2c/ov5693.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
index 117ff5403312..f410333c4c67 100644
--- a/drivers/media/i2c/ov5693.c
+++ b/drivers/media/i2c/ov5693.c
@@ -127,11 +127,15 @@
#define OV5693_LINK_FREQ_419_2MHZ 419200000
#define OV5693_PIXEL_RATE 167680000

-/* Miscellaneous */
-#define OV5693_NUM_SUPPLIES 2
-
#define to_ov5693_sensor(x) container_of(x, struct ov5693_device, sd)

+static const char * const ov5693_supply_names[] = {
+ "avdd", /* Analog power */
+ "dovdd", /* Digital I/O power */
+};
+
+#define OV5693_NUM_SUPPLIES ARRAY_SIZE(ov5693_supply_names)
+
struct ov5693_reg {
u32 reg;
u8 val;
@@ -352,11 +356,6 @@ static const s64 link_freq_menu_items[] = {
OV5693_LINK_FREQ_419_2MHZ
};

-static const char * const ov5693_supply_names[] = {
- "avdd",
- "dovdd",
-};
-
static const char * const ov5693_test_pattern_menu[] = {
"Disabled",
"Random Data",
--
2.25.1

2022-07-13 15:07:49

by Tommaso Merciai

[permalink] [raw]
Subject: [PATCH v7 3/6] media: ov5693: rename clk into xvclk

Rename clk pdata pointer into xvclk (system clock input).
Same for clk_rate into xvclk_rate. This is more explicit

Signed-off-by: Tommaso Merciai <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
Changes since v3:
- Add reviewed-by tag, suggested by Jacopo, Krzysztof

drivers/media/i2c/ov5693.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
index 9e84468d920e..d2adc5513a21 100644
--- a/drivers/media/i2c/ov5693.c
+++ b/drivers/media/i2c/ov5693.c
@@ -157,7 +157,7 @@ struct ov5693_device {
struct gpio_desc *reset;
struct gpio_desc *powerdown;
struct regulator_bulk_data supplies[OV5693_NUM_SUPPLIES];
- struct clk *clk;
+ struct clk *xvclk;

struct ov5693_mode {
struct v4l2_rect crop;
@@ -794,7 +794,7 @@ static void ov5693_sensor_powerdown(struct ov5693_device *ov5693)

regulator_bulk_disable(OV5693_NUM_SUPPLIES, ov5693->supplies);

- clk_disable_unprepare(ov5693->clk);
+ clk_disable_unprepare(ov5693->xvclk);
}

static int ov5693_sensor_powerup(struct ov5693_device *ov5693)
@@ -804,7 +804,7 @@ static int ov5693_sensor_powerup(struct ov5693_device *ov5693)
gpiod_set_value_cansleep(ov5693->reset, 1);
gpiod_set_value_cansleep(ov5693->powerdown, 1);

- ret = clk_prepare_enable(ov5693->clk);
+ ret = clk_prepare_enable(ov5693->xvclk);
if (ret) {
dev_err(ov5693->dev, "Failed to enable clk\n");
goto fail_power;
@@ -1390,7 +1390,7 @@ static int ov5693_check_hwcfg(struct ov5693_device *ov5693)
static int ov5693_probe(struct i2c_client *client)
{
struct ov5693_device *ov5693;
- u32 clk_rate;
+ u32 xvclk_rate;
int ret = 0;

ov5693 = devm_kzalloc(&client->dev, sizeof(*ov5693), GFP_KERNEL);
@@ -1408,16 +1408,16 @@ static int ov5693_probe(struct i2c_client *client)

v4l2_i2c_subdev_init(&ov5693->sd, client, &ov5693_ops);

- ov5693->clk = devm_clk_get(&client->dev, "xvclk");
- if (IS_ERR(ov5693->clk)) {
+ ov5693->xvclk = devm_clk_get(&client->dev, "xvclk");
+ if (IS_ERR(ov5693->xvclk)) {
dev_err(&client->dev, "Error getting clock\n");
- return PTR_ERR(ov5693->clk);
+ return PTR_ERR(ov5693->xvclk);
}

- clk_rate = clk_get_rate(ov5693->clk);
- if (clk_rate != OV5693_XVCLK_FREQ)
+ xvclk_rate = clk_get_rate(ov5693->xvclk);
+ if (xvclk_rate != OV5693_XVCLK_FREQ)
dev_warn(&client->dev, "Found clk freq %u, expected %u\n",
- clk_rate, OV5693_XVCLK_FREQ);
+ xvclk_rate, OV5693_XVCLK_FREQ);

ret = ov5693_configure_gpios(ov5693);
if (ret)
--
2.25.1

2022-07-13 15:08:27

by Tommaso Merciai

[permalink] [raw]
Subject: [PATCH v7 6/6] media: ov5693: add ov5693_of_match, dts support

Add ov5693_of_match. Device tree support

Signed-off-by: Tommaso Merciai <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
Reviewed-by: Daniel Scally <[email protected]>
---
Changes since v2:
- Change patch order 7 -> 6 as suggested by Sakari
- Add missing signed-off as suggested by Jacopo

Changes since v3:
- Add reviewed-by tags, suggested by Jacopo, Krzysztof

Changes since v4:
- Remove wrong Sakari reviewed-by tag, suggested by Krzysztof, Sakari
- Add Daniel reviewed-by tag

drivers/media/i2c/ov5693.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
index 701468267f20..82a9b2de7735 100644
--- a/drivers/media/i2c/ov5693.c
+++ b/drivers/media/i2c/ov5693.c
@@ -1533,10 +1533,17 @@ static const struct acpi_device_id ov5693_acpi_match[] = {
};
MODULE_DEVICE_TABLE(acpi, ov5693_acpi_match);

+static const struct of_device_id ov5693_of_match[] = {
+ { .compatible = "ovti,ov5693", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ov5693_of_match);
+
static struct i2c_driver ov5693_driver = {
.driver = {
.name = "ov5693",
.acpi_match_table = ov5693_acpi_match,
+ .of_match_table = ov5693_of_match,
.pm = &ov5693_pm_ops,
},
.probe_new = ov5693_probe,
--
2.25.1

2022-07-13 15:19:08

by Tommaso Merciai

[permalink] [raw]
Subject: [PATCH v7 4/6] media: ov5693: add support for acpi clock-frequency prop

Add support for ACPI-based platforms that specify the clock frequency by
using the "clock-frequency" property instead of specifying a clock
provider reference

Signed-off-by: Tommaso Merciai <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
Changes since v2:
- Fix commit body as suggested by Sakari, Jacopo
- Add details to commit body as suggested by Jacopo
- Move ov5693_check_hwcfg into ov5693_hwcfg
- Fix xvclk_rate position as suggested by Jacopo
- Use devm_clk_get_optional instead of devm_clk_get as suggested Jacopo

Changes since v3:
- Fix commit body as suggested by Jacopo
- Add reviewed-by tags, suggested by Jacopo, Krzysztof

Changes since v4:
- Remove wrong Sakari reviewed-by tag, suggested by Krzysztof, Sakari
- Revert ov5693_check_hwcfg function changes. Keep clk init on probe function
as suggested by Sakari
- Fix commit body
- Fix commit msg

drivers/media/i2c/ov5693.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
index d2adc5513a21..701468267f20 100644
--- a/drivers/media/i2c/ov5693.c
+++ b/drivers/media/i2c/ov5693.c
@@ -1408,13 +1408,25 @@ static int ov5693_probe(struct i2c_client *client)

v4l2_i2c_subdev_init(&ov5693->sd, client, &ov5693_ops);

- ov5693->xvclk = devm_clk_get(&client->dev, "xvclk");
- if (IS_ERR(ov5693->xvclk)) {
- dev_err(&client->dev, "Error getting clock\n");
- return PTR_ERR(ov5693->xvclk);
+ ov5693->xvclk = devm_clk_get_optional(&client->dev, "xvclk");
+ if (IS_ERR(ov5693->xvclk))
+ return dev_err_probe(&client->dev, PTR_ERR(ov5693->xvclk),
+ "failed to get xvclk: %ld\n",
+ PTR_ERR(ov5693->xvclk));
+
+ if (ov5693->xvclk) {
+ xvclk_rate = clk_get_rate(ov5693->xvclk);
+ } else {
+ ret = fwnode_property_read_u32(dev_fwnode(&client->dev),
+ "clock-frequency",
+ &xvclk_rate);
+
+ if (ret) {
+ dev_err(&client->dev, "can't get clock frequency");
+ return ret;
+ }
}

- xvclk_rate = clk_get_rate(ov5693->xvclk);
if (xvclk_rate != OV5693_XVCLK_FREQ)
dev_warn(&client->dev, "Found clk freq %u, expected %u\n",
xvclk_rate, OV5693_XVCLK_FREQ);
--
2.25.1

2022-07-13 15:30:30

by Tommaso Merciai

[permalink] [raw]
Subject: [PATCH v7 5/6] media: dt-bindings: ov5693: document YAML binding

Add documentation of device tree in YAML schema for the OV5693
CMOS image sensor from Omnivision

Signed-off-by: Tommaso Merciai <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
Changes since v1:
- Fix allOf position as suggested by Krzysztof
- Remove port description as suggested by Krzysztof
- Fix EOF as suggested by Krzysztof

Changes since v2:
- Fix commit body as suggested by Krzysztof

Changes since v3:
- Add reviewed-by tags, suggested by Jacopo, Krzysztof

Changes since v4:
- Remove wrong Sakari reviewed-by tag, suggested by Krzysztof, Sakari

Changes since v5:
- Remove dovdd-supply, avdd-supply, dvdd-supply from required properties
as suggested by Jacopo

Changes since v6:
- Add missing port node as suggested by Sakari, Rob bot

.../bindings/media/i2c/ovti,ov5693.yaml | 124 ++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 125 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
new file mode 100644
index 000000000000..359dc08440a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
@@ -0,0 +1,124 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2022 Amarulasolutions
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Omnivision OV5693 CMOS Sensor
+
+maintainers:
+ - Tommaso Merciai <[email protected]>
+
+description: |
+ The Omnivision OV5693 is a high performance, 1/4-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.
+
+ OV5693 is controlled via I2C and two-wire Serial Camera Control Bus (SCCB).
+ The sensor output is available via CSI-2 serial data output (up to 2-lane).
+
+allOf:
+ - $ref: /schemas/media/video-interface-devices.yaml#
+
+properties:
+ compatible:
+ const: ovti,ov5693
+
+ 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.8V.
+
+ avdd-supply:
+ description:
+ Analog voltage supply, 2.8V.
+
+ dvdd-supply:
+ description:
+ Digital core voltage supply, 1.2V.
+
+ 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:
+ description: MIPI CSI-2 transmitter port
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ additionalProperties: false
+
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ link-frequencies: true
+
+ data-lanes:
+ minItems: 1
+ maxItems: 2
+
+ required:
+ - data-lanes
+ - link-frequencies
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - 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>;
+
+ ov5693: camera@36 {
+ compatible = "ovti,ov5693";
+ 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 f679152bdbad..61c2d28aff4f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14838,6 +14838,7 @@ M: Daniel Scally <[email protected]>
L: [email protected]
S: Maintained
T: git git://linuxtv.org/media_tree.git
+F: Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
F: drivers/media/i2c/ov5693.c

OMNIVISION OV5695 SENSOR DRIVER
--
2.25.1

2022-07-13 15:30:31

by Tommaso Merciai

[permalink] [raw]
Subject: [PATCH v7 2/6] media: ov5693: add dvdd into ov5693_supply_names array

Add missing "dvdd" (Digital circuit power) entry into ov5693_supply_names
array

Signed-off-by: Tommaso Merciai <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
Reviewed-by: Daniel Scally <[email protected]>
---
Changes since v3:
- Add reviewed-by tag, suggested by Jacopo, Krzysztof

Changes since v4:
- Add Daniel reviewed-by tag

drivers/media/i2c/ov5693.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/media/i2c/ov5693.c b/drivers/media/i2c/ov5693.c
index f410333c4c67..9e84468d920e 100644
--- a/drivers/media/i2c/ov5693.c
+++ b/drivers/media/i2c/ov5693.c
@@ -132,6 +132,7 @@
static const char * const ov5693_supply_names[] = {
"avdd", /* Analog power */
"dovdd", /* Digital I/O power */
+ "dvdd", /* Digital circuit power */
};

#define OV5693_NUM_SUPPLIES ARRAY_SIZE(ov5693_supply_names)
--
2.25.1