* [Patch 00/19] media: ti-vpe: cal: maintenance
@ 2019-10-18 15:34 Benoit Parrot
2019-10-18 15:34 ` [Patch 01/19] dt-bindings: media: cal: update binding to use syscon Benoit Parrot
` (19 more replies)
0 siblings, 20 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
This a collection of backlog patches I have been carrying for the CAL
driver.
- Add support for SoC variants.
- Switches to syscon/regmap to access a system controller register for
the DPHY configuration. This register has different bit layout depending
on the SoC version.
- It adds supports for pre ES2.0 silicon errata.
- Reworked the DPHY initialization sequence to match the technical
reference manual and provide a more robust restartability.
- Adds the missing ability to power subdevice.
- Update the devicetree binding and then converts it to dt-schema
Benoit Parrot (18):
dt-bindings: media: cal: update binding to use syscon
dt-bindings: media: cal: update binding example
media: ti-vpe: cal: Add per platform data support
media: ti-vpe: cal: Enable DMABUF export
dt-bindings: media: cal: update binding to add PHY LDO errata support
media: ti-vpe: cal: add CSI2 PHY LDO errata support
media: ti-vpe: cal: Fix ths_term/ths_settle parameters
media: ti-vpe: cal: Fix pixel processing parameters
media: ti-vpe: cal: Align DPHY init sequence with docs
dt-bindings: media: cal: update binding to add DRA76x support
media: ti-vpe: cal: Add DRA76x support
dt-bindings: media: cal: update binding to add AM654 support
media: ti-vpe: cal: Add AM654 support
media: ti-vpe: cal: Add subdev s_power hooks
media: ti-vpe: cal: Properly calculate max resolution boundary
media: ti-vpe: cal: Fix a WARN issued when start streaming fails
media: ti-vpe: cal: fix enum_mbus_code/frame_size subdev arguments
dt-bindings: media: cal: convert binding to yaml
Nikhil Devshatwar (1):
media: ti-vpe: cal: Restrict DMA to avoid memory corruption
.../devicetree/bindings/media/ti,cal.yaml | 186 +++++
.../devicetree/bindings/media/ti-cal.txt | 72 --
drivers/media/platform/Kconfig | 2 +-
drivers/media/platform/ti-vpe/cal.c | 775 ++++++++++++++----
drivers/media/platform/ti-vpe/cal_regs.h | 27 +
5 files changed, 830 insertions(+), 232 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/ti,cal.yaml
delete mode 100644 Documentation/devicetree/bindings/media/ti-cal.txt
--
2.17.1
^ permalink raw reply [flat|nested] 40+ messages in thread
* [Patch 01/19] dt-bindings: media: cal: update binding to use syscon
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-29 13:20 ` Rob Herring
2019-10-18 15:34 ` [Patch 02/19] dt-bindings: media: cal: update binding example Benoit Parrot
` (18 subsequent siblings)
19 siblings, 1 reply; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
Update Device Tree bindings for the CAL driver to use syscon to access
the phy config register instead of trying to map it directly.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
Documentation/devicetree/bindings/media/ti-cal.txt | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
index ae9b52f37576..782f801b12a9 100644
--- a/Documentation/devicetree/bindings/media/ti-cal.txt
+++ b/Documentation/devicetree/bindings/media/ti-cal.txt
@@ -10,9 +10,14 @@ Required properties:
- compatible: must be "ti,dra72-cal"
- reg: CAL Top level, Receiver Core #0, Receiver Core #1 and Camera RX
control address space
-- reg-names: cal_top, cal_rx_core0, cal_rx_core1, and camerrx_control
+- reg-names: cal_top, cal_rx_core0, cal_rx_core1 and camerrx_control
registers
- interrupts: should contain IRQ line for the CAL;
+- syscon-camerrx: phandle to the device control module and offset to the
+ control_camerarx_core register
+ This node is meant to replace the "camerrx_control" reg
+ entry above but "camerrx_control" is still handled
+ for backward compatibility.
CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
should contain a 'port' child node with child 'endpoint' node. Please
@@ -25,13 +30,12 @@ Example:
ti,hwmods = "cal";
reg = <0x4845B000 0x400>,
<0x4845B800 0x40>,
- <0x4845B900 0x40>,
- <0x4A002e94 0x4>;
+ <0x4845B900 0x40>;
reg-names = "cal_top",
"cal_rx_core0",
- "cal_rx_core1",
- "camerrx_control";
+ "cal_rx_core1";
interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+ syscon-camerrx = <&scm_conf 0xE94>;
#address-cells = <1>;
#size-cells = <0>;
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 02/19] dt-bindings: media: cal: update binding example
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
2019-10-18 15:34 ` [Patch 01/19] dt-bindings: media: cal: update binding to use syscon Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-29 13:21 ` Rob Herring
2019-10-18 15:34 ` [Patch 03/19] media: ti-vpe: cal: Add per platform data support Benoit Parrot
` (17 subsequent siblings)
19 siblings, 1 reply; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
Update binding example to show proper endpoint properties and linkage.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
.../devicetree/bindings/media/ti-cal.txt | 32 ++++++++++---------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
index 782f801b12a9..6b56fddcfc4a 100644
--- a/Documentation/devicetree/bindings/media/ti-cal.txt
+++ b/Documentation/devicetree/bindings/media/ti-cal.txt
@@ -27,7 +27,6 @@ Documentation/devicetree/bindings/media/video-interfaces.txt.
Example:
cal: cal@4845b000 {
compatible = "ti,dra72-cal";
- ti,hwmods = "cal";
reg = <0x4845B000 0x400>,
<0x4845B800 0x40>,
<0x4845B900 0x40>;
@@ -45,9 +44,10 @@ Example:
csi2_0: port@0 {
reg = <0>;
- endpoint {
- slave-mode;
- remote-endpoint = <&ar0330_1>;
+ csi2_phy0: endpoint@0 {
+ remote-endpoint = <&csi2_cam0>;
+ clock-lanes = <0>;
+ data-lanes = <1 2>;
};
};
csi2_1: port@1 {
@@ -57,19 +57,21 @@ Example:
};
i2c5: i2c@4807c000 {
- ar0330@10 {
- compatible = "ti,ar0330";
- reg = <0x10>;
+ status = "okay";
+ clock-frequency = <400000>;
- port {
- #address-cells = <1>;
- #size-cells = <0>;
+ ov5640@3c {
+ compatible = "ovti,ov5640";
+ reg = <0x3c>;
+
+ clocks = <&clk_ov5640_fixed>;
+ clock-names = "xclk";
- ar0330_1: endpoint {
- reg = <0>;
- clock-lanes = <1>;
- data-lanes = <0 2 3 4>;
- remote-endpoint = <&csi2_0>;
+ port {
+ csi2_cam0: endpoint {
+ remote-endpoint = <&csi2_phy0>;
+ clock-lanes = <0>;
+ data-lanes = <1 2>;
};
};
};
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 03/19] media: ti-vpe: cal: Add per platform data support
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
2019-10-18 15:34 ` [Patch 01/19] dt-bindings: media: cal: update binding to use syscon Benoit Parrot
2019-10-18 15:34 ` [Patch 02/19] dt-bindings: media: cal: update binding example Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-29 13:18 ` Rob Herring
2019-10-18 15:34 ` [Patch 04/19] media: ti-vpe: cal: Enable DMABUF export Benoit Parrot
` (16 subsequent siblings)
19 siblings, 1 reply; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
First this patch adds a method to access the CTRL_CORE_CAMERRX_CONTROL
register to use the syscon mechanism. For backward compatibility we also
handle using the existing camerrx_control "reg" entry if a syscon node
is not found.
In addition the register bit layout for the CTRL_CORE_CAMERRX_CONTROL
changes depending on the device. In order to support this we need to use
a register access scheme based on data configuration instead of using
static macro.
In this case we make use of the regmap facility and create data set
based on the various device and phy available.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 281 +++++++++++++++++++++-------
1 file changed, 212 insertions(+), 69 deletions(-)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 223161f9c403..26f95a81e4ed 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -14,6 +14,8 @@
#include <linux/delay.h>
#include <linux/pm_runtime.h>
#include <linux/slab.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
#include <linux/videodev2.h>
#include <linux/of_device.h>
#include <linux/of_graph.h>
@@ -220,20 +222,66 @@ struct cal_dmaqueue {
int ini_jiffies;
};
-struct cm_data {
+struct cc_data {
void __iomem *base;
struct resource *res;
- unsigned int camerrx_control;
-
struct platform_device *pdev;
};
-struct cc_data {
- void __iomem *base;
- struct resource *res;
+/* CTRL_CORE_CAMERRX_CONTROL register field id */
+enum cal_camerarx_field {
+ F_CTRLCLKEN,
+ F_CAMMODE,
+ F_LANEENABLE,
+ F_CSI_MODE,
- struct platform_device *pdev;
+ F_MAX_FIELDS,
+};
+
+struct cal_csi2_phy {
+ struct regmap_field *fields[F_MAX_FIELDS];
+ struct reg_field *base_fields;
+ const int num_lanes;
+};
+
+struct cal_data {
+ const int num_csi2_phy;
+ struct cal_csi2_phy *csi2_phy_core;
+
+ const unsigned int flags;
+};
+
+static struct reg_field dra72x_ctrl_core_csi0_reg_fields[F_MAX_FIELDS] = {
+ [F_CTRLCLKEN] = REG_FIELD(0, 10, 10),
+ [F_CAMMODE] = REG_FIELD(0, 11, 12),
+ [F_LANEENABLE] = REG_FIELD(0, 13, 16),
+ [F_CSI_MODE] = REG_FIELD(0, 17, 17),
+};
+
+static struct reg_field dra72x_ctrl_core_csi1_reg_fields[F_MAX_FIELDS] = {
+ [F_CTRLCLKEN] = REG_FIELD(0, 0, 0),
+ [F_CAMMODE] = REG_FIELD(0, 1, 2),
+ [F_LANEENABLE] = REG_FIELD(0, 3, 4),
+ [F_CSI_MODE] = REG_FIELD(0, 5, 5),
+};
+
+static struct cal_csi2_phy dra72x_cal_csi_phy[] = {
+ {
+ .base_fields = dra72x_ctrl_core_csi0_reg_fields,
+ .num_lanes = 4,
+ },
+ {
+ .base_fields = dra72x_ctrl_core_csi1_reg_fields,
+ .num_lanes = 2,
+ },
+};
+
+static struct cal_data dra72x_cal_data = {
+ .csi2_phy_core = dra72x_cal_csi_phy,
+ .num_csi2_phy = ARRAY_SIZE(dra72x_cal_csi_phy),
+
+ .flags = 0,
};
/*
@@ -247,8 +295,15 @@ struct cal_dev {
struct platform_device *pdev;
struct v4l2_device v4l2_dev;
+ /* Controller flags for special cases */
+ unsigned int flags;
+
+ struct cal_data *data;
+
/* Control Module handle */
- struct cm_data *cm;
+ struct regmap *syscon_camerrx;
+ u32 syscon_camerrx_offset;
+
/* Camera Core Module handle */
struct cc_data *cc[CAL_NUM_CSI2_PORTS];
@@ -359,73 +414,113 @@ static inline void set_field(u32 *valp, u32 field, u32 mask)
*valp = val;
}
-/*
- * Control Module block access
- */
-static struct cm_data *cm_create(struct cal_dev *dev)
+static u32 cal_data_get_phy_max_lanes(struct cal_ctx *ctx)
{
- struct platform_device *pdev = dev->pdev;
- struct cm_data *cm;
+ struct cal_dev *dev = ctx->dev;
+ u32 phy_id = ctx->csi2_port - 1;
- cm = devm_kzalloc(&pdev->dev, sizeof(*cm), GFP_KERNEL);
- if (!cm)
- return ERR_PTR(-ENOMEM);
+ return dev->data->csi2_phy_core[phy_id].num_lanes;
+}
+
+static u32 cal_data_get_num_csi2_phy(struct cal_dev *dev)
+{
+ return dev->data->num_csi2_phy;
+}
+
+static int cal_camerarx_regmap_init(struct cal_dev *dev)
+{
+ struct reg_field *field;
+ struct cal_csi2_phy *phy;
+ int i, j;
+
+ if (!dev->data)
+ return -EINVAL;
- cm->res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
- "camerrx_control");
- cm->base = devm_ioremap_resource(&pdev->dev, cm->res);
- if (IS_ERR(cm->base)) {
+ for (i = 0; i < cal_data_get_num_csi2_phy(dev); i++) {
+ phy = &dev->data->csi2_phy_core[i];
+ for (j = 0; j < F_MAX_FIELDS; j++) {
+ field = &phy->base_fields[j];
+ /*
+ * Here we update the reg offset with the
+ * value found in DT
+ */
+ field->reg = dev->syscon_camerrx_offset;
+ phy->fields[j] =
+ devm_regmap_field_alloc(&dev->pdev->dev,
+ dev->syscon_camerrx,
+ *field);
+ if (IS_ERR(phy->fields[j])) {
+ cal_err(dev, "Unable to allocate regmap fields\n");
+ return PTR_ERR(phy->fields[j]);
+ }
+ }
+ }
+ return 0;
+}
+
+static const struct regmap_config cal_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+};
+
+static struct regmap *cal_get_camerarx_regmap(struct cal_dev *dev)
+{
+ struct platform_device *pdev = dev->pdev;
+ struct regmap *regmap;
+ void __iomem *base;
+ u32 reg_io_width;
+ struct regmap_config r_config = cal_regmap_config;
+ struct resource *res;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ "camerrx_control");
+ base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(base)) {
cal_err(dev, "failed to ioremap\n");
- return ERR_CAST(cm->base);
+ return ERR_CAST(base);
}
cal_dbg(1, dev, "ioresource %s at %pa - %pa\n",
- cm->res->name, &cm->res->start, &cm->res->end);
+ res->name, &res->start, &res->end);
+
+ reg_io_width = 4;
+ r_config.reg_stride = reg_io_width;
+ r_config.val_bits = reg_io_width * 8;
+ r_config.max_register = resource_size(res) - reg_io_width;
- return cm;
+ regmap = regmap_init_mmio(NULL, base, &r_config);
+ if (IS_ERR(regmap))
+ pr_err("regmap init failed\n");
+
+ return regmap;
}
+/*
+ * Control Module CAMERARX block access
+ */
static void camerarx_phy_enable(struct cal_ctx *ctx)
{
- u32 val;
-
- if (!ctx->dev->cm->base) {
- ctx_err(ctx, "cm not mapped\n");
- return;
- }
-
- val = reg_read(ctx->dev->cm, CM_CTRL_CORE_CAMERRX_CONTROL);
- if (ctx->csi2_port == 1) {
- set_field(&val, 1, CM_CAMERRX_CTRL_CSI0_CTRLCLKEN_MASK);
- set_field(&val, 0, CM_CAMERRX_CTRL_CSI0_CAMMODE_MASK);
- /* enable all lanes by default */
- set_field(&val, 0xf, CM_CAMERRX_CTRL_CSI0_LANEENABLE_MASK);
- set_field(&val, 1, CM_CAMERRX_CTRL_CSI0_MODE_MASK);
- } else if (ctx->csi2_port == 2) {
- set_field(&val, 1, CM_CAMERRX_CTRL_CSI1_CTRLCLKEN_MASK);
- set_field(&val, 0, CM_CAMERRX_CTRL_CSI1_CAMMODE_MASK);
- /* enable all lanes by default */
- set_field(&val, 0x3, CM_CAMERRX_CTRL_CSI1_LANEENABLE_MASK);
- set_field(&val, 1, CM_CAMERRX_CTRL_CSI1_MODE_MASK);
- }
- reg_write(ctx->dev->cm, CM_CTRL_CORE_CAMERRX_CONTROL, val);
+ struct cal_csi2_phy *phy;
+ u32 phy_id = ctx->csi2_port - 1;
+ u32 max_lanes;
+
+ phy = &ctx->dev->data->csi2_phy_core[phy_id];
+ regmap_field_write(phy->fields[F_CAMMODE], 0);
+ /* Always enable all lanes at the phy control level */
+ max_lanes = (1 << cal_data_get_phy_max_lanes(ctx)) - 1;
+ regmap_field_write(phy->fields[F_LANEENABLE], max_lanes);
+ regmap_field_write(phy->fields[F_CSI_MODE], 1);
+ regmap_field_write(phy->fields[F_CTRLCLKEN], 1);
}
static void camerarx_phy_disable(struct cal_ctx *ctx)
{
- u32 val;
-
- if (!ctx->dev->cm->base) {
- ctx_err(ctx, "cm not mapped\n");
- return;
- }
+ struct cal_csi2_phy *phy;
+ u32 phy_id = ctx->csi2_port - 1;
- val = reg_read(ctx->dev->cm, CM_CTRL_CORE_CAMERRX_CONTROL);
- if (ctx->csi2_port == 1)
- set_field(&val, 0x0, CM_CAMERRX_CTRL_CSI0_CTRLCLKEN_MASK);
- else if (ctx->csi2_port == 2)
- set_field(&val, 0x0, CM_CAMERRX_CTRL_CSI1_CTRLCLKEN_MASK);
- reg_write(ctx->dev->cm, CM_CTRL_CORE_CAMERRX_CONTROL, val);
+ phy = &ctx->dev->data->csi2_phy_core[phy_id];
+ regmap_field_write(phy->fields[F_CTRLCLKEN], 0);
}
/*
@@ -508,12 +603,6 @@ static void cal_quickdump_regs(struct cal_dev *dev)
resource_size(dev->ctx[1]->cc->res),
false);
}
-
- cal_info(dev, "CAMERRX_Control Registers @ %pa:\n",
- &dev->cm->res->start);
- print_hex_dump(KERN_INFO, "", DUMP_PREFIX_OFFSET, 16, 4,
- (__force const void *)dev->cm->base,
- resource_size(dev->cm->res), false);
}
/*
@@ -1804,10 +1893,16 @@ static struct cal_ctx *cal_create_instance(struct cal_dev *dev, int inst)
return NULL;
}
+static const struct of_device_id cal_of_match[];
+
static int cal_probe(struct platform_device *pdev)
{
struct cal_dev *dev;
struct cal_ctx *ctx;
+ const struct of_device_id *match;
+ struct device_node *parent = pdev->dev.of_node;
+ struct regmap *syscon_camerrx = NULL;
+ u32 syscon_camerrx_offset = 0;
int ret;
int irq;
int i;
@@ -1816,6 +1911,18 @@ static int cal_probe(struct platform_device *pdev)
if (!dev)
return -ENOMEM;
+ match = of_match_device(of_match_ptr(cal_of_match), &pdev->dev);
+ if (!match)
+ return -ENODEV;
+
+ if (match->data) {
+ dev->data = (struct cal_data *)match->data;
+ dev->flags = dev->data->flags;
+ } else {
+ dev_err(&pdev->dev, "Could not get feature data based on compatible version\n");
+ return -ENODEV;
+ }
+
/* set pseudo v4l2 device name so we can use v4l2_printk */
strscpy(dev->v4l2_dev.name, CAL_MODULE_NAME,
sizeof(dev->v4l2_dev.name));
@@ -1823,6 +1930,43 @@ static int cal_probe(struct platform_device *pdev)
/* save pdev pointer */
dev->pdev = pdev;
+ if (parent && of_property_read_bool(parent, "syscon-camerrx")) {
+ syscon_camerrx =
+ syscon_regmap_lookup_by_phandle(parent,
+ "syscon-camerrx");
+ if (IS_ERR(syscon_camerrx)) {
+ dev_err(&pdev->dev, "failed to get syscon-camerrx regmap\n");
+ return PTR_ERR(syscon_camerrx);
+ }
+
+ if (of_property_read_u32_index(parent, "syscon-camerrx", 1,
+ &syscon_camerrx_offset)) {
+ dev_err(&pdev->dev, "failed to get syscon-camerrx offset\n");
+ return -EINVAL;
+ }
+ } else {
+ /*
+ * Backward DTS compatibility.
+ * If syscon entry is not present then check if the
+ * camerrx_control resource is present.
+ */
+ syscon_camerrx = cal_get_camerarx_regmap(dev);
+ if (IS_ERR(syscon_camerrx)) {
+ dev_err(&pdev->dev, "failed to get camerrx_control regmap\n");
+ return PTR_ERR(syscon_camerrx);
+ }
+ /* In this case the base already point to the direct
+ * CM register so no need for an offset
+ */
+ syscon_camerrx_offset = 0;
+ }
+
+ dev->syscon_camerrx = syscon_camerrx;
+ dev->syscon_camerrx_offset = syscon_camerrx_offset;
+ ret = cal_camerarx_regmap_init(dev);
+ if (ret)
+ return ret;
+
dev->res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"cal_top");
dev->base = devm_ioremap_resource(&pdev->dev, dev->res);
@@ -1841,10 +1985,6 @@ static int cal_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, dev);
- dev->cm = cm_create(dev);
- if (IS_ERR(dev->cm))
- return PTR_ERR(dev->cm);
-
dev->cc[0] = cc_create(dev, 0);
if (IS_ERR(dev->cc[0]))
return PTR_ERR(dev->cc[0]);
@@ -1924,7 +2064,10 @@ static int cal_remove(struct platform_device *pdev)
#if defined(CONFIG_OF)
static const struct of_device_id cal_of_match[] = {
- { .compatible = "ti,dra72-cal", },
+ {
+ .compatible = "ti,dra72-cal",
+ .data = (void *)&dra72x_cal_data,
+ },
{},
};
MODULE_DEVICE_TABLE(of, cal_of_match);
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 04/19] media: ti-vpe: cal: Enable DMABUF export
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (2 preceding siblings ...)
2019-10-18 15:34 ` [Patch 03/19] media: ti-vpe: cal: Add per platform data support Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 05/19] media: ti-vpe: cal: Restrict DMA to avoid memory corruption Benoit Parrot
` (15 subsequent siblings)
19 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot, Jyri Sarha
Allow CAL to be able to export DMA buffer.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 26f95a81e4ed..78fe2da61d77 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1488,6 +1488,7 @@ static const struct v4l2_ioctl_ops cal_ioctl_ops = {
.vidioc_querybuf = vb2_ioctl_querybuf,
.vidioc_qbuf = vb2_ioctl_qbuf,
.vidioc_dqbuf = vb2_ioctl_dqbuf,
+ .vidioc_expbuf = vb2_ioctl_expbuf,
.vidioc_enum_input = cal_enum_input,
.vidioc_g_input = cal_g_input,
.vidioc_s_input = cal_s_input,
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 05/19] media: ti-vpe: cal: Restrict DMA to avoid memory corruption
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (3 preceding siblings ...)
2019-10-18 15:34 ` [Patch 04/19] media: ti-vpe: cal: Enable DMABUF export Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 06/19] dt-bindings: media: cal: update binding to add PHY LDO errata support Benoit Parrot
` (14 subsequent siblings)
19 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Nikhil Devshatwar, Benoit Parrot
From: Nikhil Devshatwar <nikhil.nd@ti.com>
When setting DMA for video capture from CSI channel, if the DMA size
is not given, it ends up writing as much data as sent by the camera.
This may lead to overwriting the buffers causing memory corruption.
Observed green lines on the default framebuffer.
Restrict the DMA to maximum height as specified in the S_FMT ioctl.
Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 78fe2da61d77..62aeedb705e9 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -769,12 +769,13 @@ static void pix_proc_config(struct cal_ctx *ctx)
}
static void cal_wr_dma_config(struct cal_ctx *ctx,
- unsigned int width)
+ unsigned int width, unsigned int height)
{
u32 val;
val = reg_read(ctx->dev, CAL_WR_DMA_CTRL(ctx->csi2_port));
set_field(&val, ctx->csi2_port, CAL_WR_DMA_CTRL_CPORT_MASK);
+ set_field(&val, height, CAL_WR_DMA_CTRL_YSIZE_MASK);
set_field(&val, CAL_WR_DMA_CTRL_DTAG_PIX_DAT,
CAL_WR_DMA_CTRL_DTAG_MASK);
set_field(&val, CAL_WR_DMA_CTRL_MODE_CONST,
@@ -1397,7 +1398,8 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
csi2_lane_config(ctx);
csi2_ctx_config(ctx);
pix_proc_config(ctx);
- cal_wr_dma_config(ctx, ctx->v_fmt.fmt.pix.bytesperline);
+ cal_wr_dma_config(ctx, ctx->v_fmt.fmt.pix.bytesperline,
+ ctx->v_fmt.fmt.pix.height);
cal_wr_dma_addr(ctx, addr);
csi2_ppi_enable(ctx);
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 06/19] dt-bindings: media: cal: update binding to add PHY LDO errata support
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (4 preceding siblings ...)
2019-10-18 15:34 ` [Patch 05/19] media: ti-vpe: cal: Restrict DMA to avoid memory corruption Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-29 13:23 ` Rob Herring
2019-10-18 15:34 ` [Patch 07/19] media: ti-vpe: cal: add CSI2 " Benoit Parrot
` (13 subsequent siblings)
19 siblings, 1 reply; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot, Jyri Sarha
Update Device Tree bindings for the CAL driver to add support for
the CSI2 PHY LDO errata workaround for pre-es2 devices.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
Documentation/devicetree/bindings/media/ti-cal.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
index 6b56fddcfc4a..d43870a2324b 100644
--- a/Documentation/devicetree/bindings/media/ti-cal.txt
+++ b/Documentation/devicetree/bindings/media/ti-cal.txt
@@ -7,7 +7,9 @@ processing capability to connect CSI2 image-sensor modules to the
DRA72x device.
Required properties:
-- compatible: must be "ti,dra72-cal"
+- compatible:
+ Should be "ti,dra72-cal", for DRA72 controllers
+ Should be "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
- reg: CAL Top level, Receiver Core #0, Receiver Core #1 and Camera RX
control address space
- reg-names: cal_top, cal_rx_core0, cal_rx_core1 and camerrx_control
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 07/19] media: ti-vpe: cal: add CSI2 PHY LDO errata support
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (5 preceding siblings ...)
2019-10-18 15:34 ` [Patch 06/19] dt-bindings: media: cal: update binding to add PHY LDO errata support Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-21 10:38 ` Hans Verkuil
2019-10-18 15:34 ` [Patch 08/19] media: ti-vpe: cal: Fix ths_term/ths_settle parameters Benoit Parrot
` (12 subsequent siblings)
19 siblings, 1 reply; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot, Jyri Sarha
Apply Errata i913 every time the functional clock is enabled.
This should take care of suspend/resume case as well.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 56 +++++++++++++++++++++++-
drivers/media/platform/ti-vpe/cal_regs.h | 27 ++++++++++++
2 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 62aeedb705e9..3cbc4dca6de8 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -284,6 +284,13 @@ static struct cal_data dra72x_cal_data = {
.flags = 0,
};
+static struct cal_data dra72x_es1_cal_data = {
+ .csi2_phy_core = dra72x_cal_csi_phy,
+ .num_csi2_phy = ARRAY_SIZE(dra72x_cal_csi_phy),
+
+ .flags = DRA72_CAL_PRE_ES2_LDO_DISABLE,
+};
+
/*
* there is one cal_dev structure in the driver, it is shared by
* all instances.
@@ -569,9 +576,52 @@ static void cal_get_hwinfo(struct cal_dev *dev)
hwinfo);
}
+/*
+ * Errata i913: CSI2 LDO Needs to be disabled when module is powered on
+ *
+ * Enabling CSI2 LDO shorts it to core supply. It is crucial the 2 CSI2
+ * LDOs on the device are disabled if CSI-2 module is powered on
+ * (0x4845 B304 | 0x4845 B384 [28:27] = 0x1) or in ULPS (0x4845 B304
+ * | 0x4845 B384 [28:27] = 0x2) mode. Common concerns include: high
+ * current draw on the module supply in active mode.
+ *
+ * Errata does not apply when CSI-2 module is powered off
+ * (0x4845 B304 | 0x4845 B384 [28:27] = 0x0).
+ *
+ * SW Workaround:
+ * Set the following register bits to disable the LDO,
+ * which is essentially CSI2 REG10 bit 6:
+ *
+ * Core 0: 0x4845 B828 = 0x0000 0040
+ * Core 1: 0x4845 B928 = 0x0000 0040
+ */
+static void i913_errata(struct cal_dev *dev, unsigned int port)
+{
+ u32 reg10 = reg_read(dev->cc[port], CAL_CSI2_PHY_REG10);
+
+ set_field(®10, CAL_CSI2_PHY_REG0_HSCLOCKCONFIG_DISABLE,
+ CAL_CSI2_PHY_REG10_I933_LDO_DISABLE_MASK);
+
+ cal_dbg(1, dev, "CSI2_%d_REG10 = 0x%08x\n", port, reg10);
+ reg_write(dev->cc[port], CAL_CSI2_PHY_REG10, reg10);
+}
+
static inline int cal_runtime_get(struct cal_dev *dev)
{
- return pm_runtime_get_sync(&dev->pdev->dev);
+ int r;
+
+ r = pm_runtime_get_sync(&dev->pdev->dev);
+
+ if (dev->flags & DRA72_CAL_PRE_ES2_LDO_DISABLE) {
+ /*
+ * Apply errata on both port eveytime we (re-)enable
+ * the clock
+ */
+ i913_errata(dev, 0);
+ i913_errata(dev, 1);
+ }
+
+ return r;
}
static inline void cal_runtime_put(struct cal_dev *dev)
@@ -2071,6 +2121,10 @@ static const struct of_device_id cal_of_match[] = {
.compatible = "ti,dra72-cal",
.data = (void *)&dra72x_cal_data,
},
+ {
+ .compatible = "ti,dra72-pre-es2-cal",
+ .data = (void *)&dra72x_es1_cal_data,
+ },
{},
};
MODULE_DEVICE_TABLE(of, cal_of_match);
diff --git a/drivers/media/platform/ti-vpe/cal_regs.h b/drivers/media/platform/ti-vpe/cal_regs.h
index 68cfc922b422..78d6f015c9ea 100644
--- a/drivers/media/platform/ti-vpe/cal_regs.h
+++ b/drivers/media/platform/ti-vpe/cal_regs.h
@@ -10,6 +10,30 @@
#ifndef __TI_CAL_REGS_H
#define __TI_CAL_REGS_H
+/*
+ * struct cal_dev.flags possibilities
+ *
+ * DRA72_CAL_PRE_ES2_LDO_DISABLE:
+ * Errata i913: CSI2 LDO Needs to be disabled when module is powered on
+ *
+ * Enabling CSI2 LDO shorts it to core supply. It is crucial the 2 CSI2
+ * LDOs on the device are disabled if CSI-2 module is powered on
+ * (0x4845 B304 | 0x4845 B384 [28:27] = 0x1) or in ULPS (0x4845 B304
+ * | 0x4845 B384 [28:27] = 0x2) mode. Common concerns include: high
+ * current draw on the module supply in active mode.
+ *
+ * Errata does not apply when CSI-2 module is powered off
+ * (0x4845 B304 | 0x4845 B384 [28:27] = 0x0).
+ *
+ * SW Workaround:
+ * Set the following register bits to disable the LDO,
+ * which is essentially CSI2 REG10 bit 6:
+ *
+ * Core 0: 0x4845 B828 = 0x0000 0040
+ * Core 1: 0x4845 B928 = 0x0000 0040
+ */
+#define DRA72_CAL_PRE_ES2_LDO_DISABLE BIT(0)
+
#define CAL_NUM_CSI2_PORTS 2
/* CAL register offsets */
@@ -71,6 +95,7 @@
#define CAL_CSI2_PHY_REG0 0x000
#define CAL_CSI2_PHY_REG1 0x004
#define CAL_CSI2_PHY_REG2 0x008
+#define CAL_CSI2_PHY_REG10 0x028
/* CAL Control Module Core Camerrx Control register offsets */
#define CM_CTRL_CORE_CAMERRX_CONTROL 0x000
@@ -458,6 +483,8 @@
#define CAL_CSI2_PHY_REG1_CLOCK_MISS_DETECTOR_STATUS_SUCCESS 0
#define CAL_CSI2_PHY_REG1_RESET_DONE_STATUS_MASK GENMASK(29, 28)
+#define CAL_CSI2_PHY_REG10_I933_LDO_DISABLE_MASK BIT_MASK(6)
+
#define CAL_CSI2_PHY_REG2_CCP2_SYNC_PATTERN_MASK GENMASK(23, 0)
#define CAL_CSI2_PHY_REG2_TRIGGER_CMD_RXTRIGESC3_MASK GENMASK(25, 24)
#define CAL_CSI2_PHY_REG2_TRIGGER_CMD_RXTRIGESC2_MASK GENMASK(27, 26)
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 08/19] media: ti-vpe: cal: Fix ths_term/ths_settle parameters
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (6 preceding siblings ...)
2019-10-18 15:34 ` [Patch 07/19] media: ti-vpe: cal: add CSI2 " Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 09/19] media: ti-vpe: cal: Fix pixel processing parameters Benoit Parrot
` (11 subsequent siblings)
19 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
The current method to calculate the ddr clk period is wrong.
Therefore the ths_term calculation is incorrect.
Also it was wrongly assumed that the ths_settle parameter
was based on the control clock instead of the pixel clock.
Since the DPHY can tolerate quite a bit a of variation,
capture was still mostly working with the 2 tested modes
when the pixel clock was close to the control clock
(i.e. 96 Mhz). But it would quickly stops working when
using different modes or when customers used different
sensors altogether.
Calculating the DDRClk period needs to take into account
the pixel bit width and the number of active data lanes.
Based on the latest technical reference manual these
parameters should now be calculated as follows:
THS_TERM: Programmed value = floor(20 ns/DDRClk period)
THS_SETTLE: Programmed value = floor(105 ns/DDRClk period) + 4
Also originally 'depth' was used to represent the number of
bits a pixel would use once stored in memory (i.e. the
container size). To accurately calculate the THS_* parameters
we need to use the actual number of bits per pixels coming
in from the sensor. So we are renaming 'depth' to 'bpp' (bits
per pixels) and update the format table to show the actual
number of bits per pixel being received.
The "container" size will be derived from the "bpp" value.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 98 +++++++++++++----------------
1 file changed, 44 insertions(+), 54 deletions(-)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 3cbc4dca6de8..5cc44c5b0ad8 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -73,8 +73,6 @@ static const struct v4l2_fract
#define CAL_NUM_INPUT 1
#define CAL_NUM_CONTEXT 2
-#define bytes_per_line(pixel, bpp) (ALIGN(pixel * bpp, 16))
-
#define reg_read(dev, offset) ioread32(dev->base + offset)
#define reg_write(dev, offset, val) iowrite32(val, dev->base + offset)
@@ -93,102 +91,103 @@ static const struct v4l2_fract
struct cal_fmt {
u32 fourcc;
u32 code;
- u8 depth;
+ /* Bits per pixel */
+ u8 bpp;
};
static struct cal_fmt cal_formats[] = {
{
.fourcc = V4L2_PIX_FMT_YUYV,
.code = MEDIA_BUS_FMT_YUYV8_2X8,
- .depth = 16,
+ .bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_UYVY,
.code = MEDIA_BUS_FMT_UYVY8_2X8,
- .depth = 16,
+ .bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_YVYU,
.code = MEDIA_BUS_FMT_YVYU8_2X8,
- .depth = 16,
+ .bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_VYUY,
.code = MEDIA_BUS_FMT_VYUY8_2X8,
- .depth = 16,
+ .bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_RGB565, /* gggbbbbb rrrrrggg */
.code = MEDIA_BUS_FMT_RGB565_2X8_LE,
- .depth = 16,
+ .bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_RGB565X, /* rrrrrggg gggbbbbb */
.code = MEDIA_BUS_FMT_RGB565_2X8_BE,
- .depth = 16,
+ .bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_RGB555, /* gggbbbbb arrrrrgg */
.code = MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE,
- .depth = 16,
+ .bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_RGB555X, /* arrrrrgg gggbbbbb */
.code = MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE,
- .depth = 16,
+ .bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_RGB24, /* rgb */
.code = MEDIA_BUS_FMT_RGB888_2X12_LE,
- .depth = 24,
+ .bpp = 24,
}, {
.fourcc = V4L2_PIX_FMT_BGR24, /* bgr */
.code = MEDIA_BUS_FMT_RGB888_2X12_BE,
- .depth = 24,
+ .bpp = 24,
}, {
.fourcc = V4L2_PIX_FMT_RGB32, /* argb */
.code = MEDIA_BUS_FMT_ARGB8888_1X32,
- .depth = 32,
+ .bpp = 32,
}, {
.fourcc = V4L2_PIX_FMT_SBGGR8,
.code = MEDIA_BUS_FMT_SBGGR8_1X8,
- .depth = 8,
+ .bpp = 8,
}, {
.fourcc = V4L2_PIX_FMT_SGBRG8,
.code = MEDIA_BUS_FMT_SGBRG8_1X8,
- .depth = 8,
+ .bpp = 8,
}, {
.fourcc = V4L2_PIX_FMT_SGRBG8,
.code = MEDIA_BUS_FMT_SGRBG8_1X8,
- .depth = 8,
+ .bpp = 8,
}, {
.fourcc = V4L2_PIX_FMT_SRGGB8,
.code = MEDIA_BUS_FMT_SRGGB8_1X8,
- .depth = 8,
+ .bpp = 8,
}, {
.fourcc = V4L2_PIX_FMT_SBGGR10,
.code = MEDIA_BUS_FMT_SBGGR10_1X10,
- .depth = 16,
+ .bpp = 10,
}, {
.fourcc = V4L2_PIX_FMT_SGBRG10,
.code = MEDIA_BUS_FMT_SGBRG10_1X10,
- .depth = 16,
+ .bpp = 10,
}, {
.fourcc = V4L2_PIX_FMT_SGRBG10,
.code = MEDIA_BUS_FMT_SGRBG10_1X10,
- .depth = 16,
+ .bpp = 10,
}, {
.fourcc = V4L2_PIX_FMT_SRGGB10,
.code = MEDIA_BUS_FMT_SRGGB10_1X10,
- .depth = 16,
+ .bpp = 10,
}, {
.fourcc = V4L2_PIX_FMT_SBGGR12,
.code = MEDIA_BUS_FMT_SBGGR12_1X12,
- .depth = 16,
+ .bpp = 12,
}, {
.fourcc = V4L2_PIX_FMT_SGBRG12,
.code = MEDIA_BUS_FMT_SGBRG12_1X12,
- .depth = 16,
+ .bpp = 12,
}, {
.fourcc = V4L2_PIX_FMT_SGRBG12,
.code = MEDIA_BUS_FMT_SGRBG12_1X12,
- .depth = 16,
+ .bpp = 12,
}, {
.fourcc = V4L2_PIX_FMT_SRGGB12,
.code = MEDIA_BUS_FMT_SRGGB12_1X12,
- .depth = 16,
+ .bpp = 12,
},
};
@@ -873,41 +872,28 @@ static void cal_wr_dma_addr(struct cal_ctx *ctx, unsigned int dmaaddr)
#define TCLK_TERM 0
#define TCLK_MISS 1
#define TCLK_SETTLE 14
-#define THS_SETTLE 15
static void csi2_phy_config(struct cal_ctx *ctx)
{
unsigned int reg0, reg1;
unsigned int ths_term, ths_settle;
- unsigned int ddrclkperiod_us;
+ unsigned int csi2_ddrclk_khz;
+ struct v4l2_fwnode_bus_mipi_csi2 *mipi_csi2 =
+ &ctx->endpoint.bus.mipi_csi2;
+ u32 num_lanes = mipi_csi2->num_data_lanes;
- /*
- * THS_TERM: Programmed value = floor(20 ns/DDRClk period) - 2.
- */
- ddrclkperiod_us = ctx->external_rate / 2000000;
- ddrclkperiod_us = 1000000 / ddrclkperiod_us;
- ctx_dbg(1, ctx, "ddrclkperiod_us: %d\n", ddrclkperiod_us);
+ /* DPHY timing configuration */
+ /* CSI-2 is DDR and we only count used lanes. */
+ csi2_ddrclk_khz = ctx->external_rate / 1000
+ / (2 * num_lanes) * ctx->fmt->bpp;
+ ctx_dbg(1, ctx, "csi2_ddrclk_khz: %d\n", csi2_ddrclk_khz);
- ths_term = 20000 / ddrclkperiod_us;
- ths_term = (ths_term >= 2) ? ths_term - 2 : ths_term;
+ /* THS_TERM: Programmed value = floor(20 ns/DDRClk period) */
+ ths_term = 20 * csi2_ddrclk_khz / 1000000;
ctx_dbg(1, ctx, "ths_term: %d (0x%02x)\n", ths_term, ths_term);
- /*
- * THS_SETTLE: Programmed value = floor(176.3 ns/CtrlClk period) - 1.
- * Since CtrlClk is fixed at 96Mhz then we get
- * ths_settle = floor(176.3 / 10.416) - 1 = 15
- * If we ever switch to a dynamic clock then this code might be useful
- *
- * unsigned int ctrlclkperiod_us;
- * ctrlclkperiod_us = 96000000 / 1000000;
- * ctrlclkperiod_us = 1000000 / ctrlclkperiod_us;
- * ctx_dbg(1, ctx, "ctrlclkperiod_us: %d\n", ctrlclkperiod_us);
-
- * ths_settle = 176300 / ctrlclkperiod_us;
- * ths_settle = (ths_settle > 1) ? ths_settle - 1 : ths_settle;
- */
-
- ths_settle = THS_SETTLE;
+ /* THS_SETTLE: Programmed value = floor(105 ns/DDRClk period) + 4 */
+ ths_settle = (105 * csi2_ddrclk_khz / 1000000) + 4;
ctx_dbg(1, ctx, "ths_settle: %d (0x%02x)\n", ths_settle, ths_settle);
reg0 = reg_read(ctx->cc, CAL_CSI2_PHY_REG0);
@@ -1119,6 +1105,8 @@ static int cal_calc_format_size(struct cal_ctx *ctx,
const struct cal_fmt *fmt,
struct v4l2_format *f)
{
+ u32 bpl;
+
if (!fmt) {
ctx_dbg(3, ctx, "No cal_fmt provided!\n");
return -EINVAL;
@@ -1126,8 +1114,10 @@ static int cal_calc_format_size(struct cal_ctx *ctx,
v4l_bound_align_image(&f->fmt.pix.width, 48, MAX_WIDTH, 2,
&f->fmt.pix.height, 32, MAX_HEIGHT, 0, 0);
- f->fmt.pix.bytesperline = bytes_per_line(f->fmt.pix.width,
- fmt->depth >> 3);
+
+ bpl = (f->fmt.pix.width * ALIGN(fmt->bpp, 8)) >> 3;
+ f->fmt.pix.bytesperline = ALIGN(bpl, 16);
+
f->fmt.pix.sizeimage = f->fmt.pix.height *
f->fmt.pix.bytesperline;
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 09/19] media: ti-vpe: cal: Fix pixel processing parameters
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (7 preceding siblings ...)
2019-10-18 15:34 ` [Patch 08/19] media: ti-vpe: cal: Fix ths_term/ths_settle parameters Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 10/19] media: ti-vpe: cal: Align DPHY init sequence with docs Benoit Parrot
` (10 subsequent siblings)
19 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
The pixel processing unit was hard coded to only handle 8 bits per pixel
from input to output.
We now add handling for 10, 12 and 16 bits per pixel at the source and
setting the in-memory size (i.e. container size) to 16 bits for these 3
cases.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 41 ++++++++++++++++++++++++++---
1 file changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 5cc44c5b0ad8..3c293e263397 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -803,13 +803,48 @@ static void csi2_ctx_config(struct cal_ctx *ctx)
static void pix_proc_config(struct cal_ctx *ctx)
{
- u32 val;
+ u32 val, extract, pack;
+
+ switch (ctx->fmt->bpp) {
+ case 8:
+ extract = CAL_PIX_PROC_EXTRACT_B8;
+ pack = CAL_PIX_PROC_PACK_B8;
+ break;
+ case 10:
+ extract = CAL_PIX_PROC_EXTRACT_B10_MIPI;
+ pack = CAL_PIX_PROC_PACK_B16;
+ break;
+ case 12:
+ extract = CAL_PIX_PROC_EXTRACT_B12_MIPI;
+ pack = CAL_PIX_PROC_PACK_B16;
+ break;
+ case 16:
+ extract = CAL_PIX_PROC_EXTRACT_B16_LE;
+ pack = CAL_PIX_PROC_PACK_B16;
+ break;
+ default:
+ /*
+ * If you see this warning then it means that you added
+ * some new entry in the cal_formats[] array with a different
+ * bit per pixel values then the one supported below.
+ * Either add support for the new bpp value below or adjust
+ * the new entry to use one of the value below.
+ *
+ * Instead of failing here just use 8 bpp as a default.
+ */
+ dev_warn_once(&ctx->dev->pdev->dev,
+ "%s:%d:%s: bpp:%d unsupported! Overwritten with 8.\n",
+ __FILE__, __LINE__, __func__, ctx->fmt->bpp);
+ extract = CAL_PIX_PROC_EXTRACT_B8;
+ pack = CAL_PIX_PROC_PACK_B8;
+ break;
+ }
val = reg_read(ctx->dev, CAL_PIX_PROC(ctx->csi2_port));
- set_field(&val, CAL_PIX_PROC_EXTRACT_B8, CAL_PIX_PROC_EXTRACT_MASK);
+ set_field(&val, extract, CAL_PIX_PROC_EXTRACT_MASK);
set_field(&val, CAL_PIX_PROC_DPCMD_BYPASS, CAL_PIX_PROC_DPCMD_MASK);
set_field(&val, CAL_PIX_PROC_DPCME_BYPASS, CAL_PIX_PROC_DPCME_MASK);
- set_field(&val, CAL_PIX_PROC_PACK_B8, CAL_PIX_PROC_PACK_MASK);
+ set_field(&val, pack, CAL_PIX_PROC_PACK_MASK);
set_field(&val, ctx->csi2_port, CAL_PIX_PROC_CPORT_MASK);
set_field(&val, CAL_GEN_ENABLE, CAL_PIX_PROC_EN_MASK);
reg_write(ctx->dev, CAL_PIX_PROC(ctx->csi2_port), val);
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 10/19] media: ti-vpe: cal: Align DPHY init sequence with docs
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (8 preceding siblings ...)
2019-10-18 15:34 ` [Patch 09/19] media: ti-vpe: cal: Fix pixel processing parameters Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 11/19] dt-bindings: media: cal: update binding to add DRA76x support Benoit Parrot
` (9 subsequent siblings)
19 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
The current CSI2 DPHY initialization sequence although
functional does not match with the documented sequence
in the Technical Reference Manual.
This may affect capture re-startability in stop/start
situations.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 199 ++++++++++++++++++++++++----
1 file changed, 172 insertions(+), 27 deletions(-)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 3c293e263397..c912ea82b534 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -689,29 +689,76 @@ static void disable_irqs(struct cal_ctx *ctx)
reg_write(ctx->dev, CAL_CSI2_VC_IRQENABLE(1), 0);
}
-static void csi2_init(struct cal_ctx *ctx)
+static void csi2_phy_config(struct cal_ctx *ctx);
+
+static void csi2_phy_init(struct cal_ctx *ctx)
{
int i;
u32 val;
+ /* Steps
+ * 1. Configure D-PHY mode and enable required lanes
+ * 2. Reset complex IO - Wait for completion of reset
+ * Note if the external sensor is not sending byte clock,
+ * the reset will timeout
+ * 3 Program Stop States
+ * A. Program THS_TERM, THS_SETTLE, etc... Timings parameters
+ * in terms of DDR clock periods
+ * B. Enable stop state transition timeouts
+ * 4.Force FORCERXMODE
+ * D. Enable pull down using pad control
+ * E. Power up PHY
+ * F. Wait for power up completion
+ * G. Wait for all enabled lane to reach stop state
+ * H. Disable pull down using pad control
+ */
+
+ /* 1. Configure D-PHY mode and enable required lanes */
+ camerarx_phy_enable(ctx);
+
+ /* 2. Reset complex IO - Do not wait for reset completion */
+ val = reg_read(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port));
+ set_field(&val, CAL_CSI2_COMPLEXIO_CFG_RESET_CTRL_OPERATIONAL,
+ CAL_CSI2_COMPLEXIO_CFG_RESET_CTRL_MASK);
+ reg_write(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port), val);
+ ctx_dbg(3, ctx, "CAL_CSI2_COMPLEXIO_CFG(%d) = 0x%08x De-assert Complex IO Reset\n",
+ ctx->csi2_port,
+ reg_read(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port)));
+
+ /* Dummy read to allow SCP to complete */
+ val = reg_read(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port));
+
+ /* 3.A. Program Phy Timing Parameters */
+ csi2_phy_config(ctx);
+
+ /* 3.B. Program Stop States */
val = reg_read(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port));
- set_field(&val, CAL_GEN_ENABLE,
- CAL_CSI2_TIMING_FORCE_RX_MODE_IO1_MASK);
set_field(&val, CAL_GEN_ENABLE,
CAL_CSI2_TIMING_STOP_STATE_X16_IO1_MASK);
set_field(&val, CAL_GEN_DISABLE,
CAL_CSI2_TIMING_STOP_STATE_X4_IO1_MASK);
set_field(&val, 407, CAL_CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK);
reg_write(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port), val);
- ctx_dbg(3, ctx, "CAL_CSI2_TIMING(%d) = 0x%08x\n", ctx->csi2_port,
+ ctx_dbg(3, ctx, "CAL_CSI2_TIMING(%d) = 0x%08x Stop States\n",
+ ctx->csi2_port,
+ reg_read(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port)));
+
+ /* 4. Force FORCERXMODE */
+ val = reg_read(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port));
+ set_field(&val, CAL_GEN_ENABLE,
+ CAL_CSI2_TIMING_FORCE_RX_MODE_IO1_MASK);
+ reg_write(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port), val);
+ ctx_dbg(3, ctx, "CAL_CSI2_TIMING(%d) = 0x%08x Force RXMODE\n",
+ ctx->csi2_port,
reg_read(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port)));
+ /* E. Power up the PHY using the complex IO */
val = reg_read(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port));
- set_field(&val, CAL_CSI2_COMPLEXIO_CFG_RESET_CTRL_OPERATIONAL,
- CAL_CSI2_COMPLEXIO_CFG_RESET_CTRL_MASK);
set_field(&val, CAL_CSI2_COMPLEXIO_CFG_PWR_CMD_STATE_ON,
CAL_CSI2_COMPLEXIO_CFG_PWR_CMD_MASK);
reg_write(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port), val);
+
+ /* F. Wait for power up completion */
for (i = 0; i < 10; i++) {
if (reg_read_field(ctx->dev,
CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port),
@@ -720,18 +767,104 @@ static void csi2_init(struct cal_ctx *ctx)
break;
usleep_range(1000, 1100);
}
- ctx_dbg(3, ctx, "CAL_CSI2_COMPLEXIO_CFG(%d) = 0x%08x\n", ctx->csi2_port,
- reg_read(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port)));
+ ctx_dbg(3, ctx, "CAL_CSI2_COMPLEXIO_CFG(%d) = 0x%08x Powered UP %s\n",
+ ctx->csi2_port,
+ reg_read(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port)),
+ (i >= 10) ? "(timeout)" : "");
+}
- val = reg_read(ctx->dev, CAL_CTRL);
- set_field(&val, CAL_CTRL_BURSTSIZE_BURST128, CAL_CTRL_BURSTSIZE_MASK);
- set_field(&val, 0xF, CAL_CTRL_TAGCNT_MASK);
- set_field(&val, CAL_CTRL_POSTED_WRITES_NONPOSTED,
- CAL_CTRL_POSTED_WRITES_MASK);
- set_field(&val, 0xFF, CAL_CTRL_MFLAGL_MASK);
- set_field(&val, 0xFF, CAL_CTRL_MFLAGH_MASK);
- reg_write(ctx->dev, CAL_CTRL, val);
- ctx_dbg(3, ctx, "CAL_CTRL = 0x%08x\n", reg_read(ctx->dev, CAL_CTRL));
+static void csi2_wait_for_phy(struct cal_ctx *ctx)
+{
+ int i;
+
+ /* Steps
+ * 2. Wait for completion of reset
+ * Note if the external sensor is not sending byte clock,
+ * the reset will timeout
+ * 4.Force FORCERXMODE
+ * G. Wait for all enabled lane to reach stop state
+ * H. Disable pull down using pad control
+ */
+
+ /* 2. Wait for reset completion */
+ for (i = 0; i < 250; i++) {
+ if (reg_read_field(ctx->dev,
+ CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port),
+ CAL_CSI2_COMPLEXIO_CFG_RESET_DONE_MASK) ==
+ CAL_CSI2_COMPLEXIO_CFG_RESET_DONE_RESETCOMPLETED)
+ break;
+ usleep_range(1000, 1100);
+ }
+ ctx_dbg(3, ctx, "CAL_CSI2_COMPLEXIO_CFG(%d) = 0x%08x Complex IO Reset Done (%d) %s\n",
+ ctx->csi2_port,
+ reg_read(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port)), i,
+ (i >= 250) ? "(timeout)" : "");
+
+ /* 4. G. Wait for all enabled lane to reach stop state */
+ for (i = 0; i < 10; i++) {
+ if (reg_read_field(ctx->dev,
+ CAL_CSI2_TIMING(ctx->csi2_port),
+ CAL_CSI2_TIMING_FORCE_RX_MODE_IO1_MASK) ==
+ CAL_GEN_DISABLE)
+ break;
+ usleep_range(1000, 1100);
+ }
+ ctx_dbg(3, ctx, "CAL_CSI2_TIMING(%d) = 0x%08x Stop State Reached %s\n",
+ ctx->csi2_port,
+ reg_read(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port)),
+ (i >= 10) ? "(timeout)" : "");
+
+ ctx_dbg(1, ctx, "CSI2_%d_REG1 = 0x%08x (Bit(31,28) should be set!)\n",
+ (ctx->csi2_port - 1), reg_read(ctx->cc, CAL_CSI2_PHY_REG1));
+}
+
+static void csi2_phy_deinit(struct cal_ctx *ctx)
+{
+ int i;
+ u32 val;
+
+ /* Power down the PHY using the complex IO */
+ val = reg_read(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port));
+ set_field(&val, CAL_CSI2_COMPLEXIO_CFG_PWR_CMD_STATE_OFF,
+ CAL_CSI2_COMPLEXIO_CFG_PWR_CMD_MASK);
+ reg_write(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port), val);
+
+ /* Wait for power down completion */
+ for (i = 0; i < 10; i++) {
+ if (reg_read_field(ctx->dev,
+ CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port),
+ CAL_CSI2_COMPLEXIO_CFG_PWR_STATUS_MASK) ==
+ CAL_CSI2_COMPLEXIO_CFG_PWR_STATUS_STATE_OFF)
+ break;
+ usleep_range(1000, 1100);
+ }
+ ctx_dbg(3, ctx, "CAL_CSI2_COMPLEXIO_CFG(%d) = 0x%08x Powered Down %s\n",
+ ctx->csi2_port,
+ reg_read(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port)),
+ (i >= 10) ? "(timeout)" : "");
+
+ /* Assert Comple IO Reset */
+ val = reg_read(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port));
+ set_field(&val, CAL_CSI2_COMPLEXIO_CFG_RESET_CTRL,
+ CAL_CSI2_COMPLEXIO_CFG_RESET_CTRL_MASK);
+ reg_write(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port), val);
+
+ /* Wait for power down completion */
+ for (i = 0; i < 10; i++) {
+ if (reg_read_field(ctx->dev,
+ CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port),
+ CAL_CSI2_COMPLEXIO_CFG_RESET_DONE_MASK) ==
+ CAL_CSI2_COMPLEXIO_CFG_RESET_DONE_RESETONGOING)
+ break;
+ usleep_range(1000, 1100);
+ }
+ ctx_dbg(3, ctx, "CAL_CSI2_COMPLEXIO_CFG(%d) = 0x%08x Complex IO in Reset (%d) %s\n",
+ ctx->csi2_port,
+ reg_read(ctx->dev, CAL_CSI2_COMPLEXIO_CFG(ctx->csi2_port)), i,
+ (i >= 10) ? "(timeout)" : "");
+
+ /* Disable the phy */
+ camerarx_phy_disable(ctx);
}
static void csi2_lane_config(struct cal_ctx *ctx)
@@ -894,6 +1027,16 @@ static void cal_wr_dma_config(struct cal_ctx *ctx,
reg_write(ctx->dev, CAL_WR_DMA_XSIZE(ctx->csi2_port), val);
ctx_dbg(3, ctx, "CAL_WR_DMA_XSIZE(%d) = 0x%08x\n", ctx->csi2_port,
reg_read(ctx->dev, CAL_WR_DMA_XSIZE(ctx->csi2_port)));
+
+ val = reg_read(ctx->dev, CAL_CTRL);
+ set_field(&val, CAL_CTRL_BURSTSIZE_BURST128, CAL_CTRL_BURSTSIZE_MASK);
+ set_field(&val, 0xF, CAL_CTRL_TAGCNT_MASK);
+ set_field(&val, CAL_CTRL_POSTED_WRITES_NONPOSTED,
+ CAL_CTRL_POSTED_WRITES_MASK);
+ set_field(&val, 0xFF, CAL_CTRL_MFLAGL_MASK);
+ set_field(&val, 0xFF, CAL_CTRL_MFLAGH_MASK);
+ reg_write(ctx->dev, CAL_CTRL, val);
+ ctx_dbg(3, ctx, "CAL_CTRL = 0x%08x\n", reg_read(ctx->dev, CAL_CTRL));
}
static void cal_wr_dma_addr(struct cal_ctx *ctx, unsigned int dmaaddr)
@@ -1466,17 +1609,14 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
cal_runtime_get(ctx->dev);
- enable_irqs(ctx);
- camerarx_phy_enable(ctx);
- csi2_init(ctx);
- csi2_phy_config(ctx);
- csi2_lane_config(ctx);
csi2_ctx_config(ctx);
pix_proc_config(ctx);
cal_wr_dma_config(ctx, ctx->v_fmt.fmt.pix.bytesperline,
ctx->v_fmt.fmt.pix.height);
- cal_wr_dma_addr(ctx, addr);
- csi2_ppi_enable(ctx);
+ csi2_lane_config(ctx);
+
+ enable_irqs(ctx);
+ csi2_phy_init(ctx);
ret = v4l2_subdev_call(ctx->sensor, video, s_stream, 1);
if (ret) {
@@ -1485,6 +1625,10 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
goto err;
}
+ csi2_wait_for_phy(ctx);
+ cal_wr_dma_addr(ctx, addr);
+ csi2_ppi_enable(ctx);
+
if (debug >= 4)
cal_quickdump_regs(ctx->dev);
@@ -1505,11 +1649,12 @@ static void cal_stop_streaming(struct vb2_queue *vq)
struct cal_buffer *buf, *tmp;
unsigned long flags;
- if (v4l2_subdev_call(ctx->sensor, video, s_stream, 0))
- ctx_err(ctx, "stream off failed in subdev\n");
-
csi2_ppi_disable(ctx);
disable_irqs(ctx);
+ csi2_phy_deinit(ctx);
+
+ if (v4l2_subdev_call(ctx->sensor, video, s_stream, 0))
+ ctx_err(ctx, "stream off failed in subdev\n");
/* Release all active buffers */
spin_lock_irqsave(&ctx->slock, flags);
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 11/19] dt-bindings: media: cal: update binding to add DRA76x support
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (9 preceding siblings ...)
2019-10-18 15:34 ` [Patch 10/19] media: ti-vpe: cal: Align DPHY init sequence with docs Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-29 13:24 ` Rob Herring
2019-10-18 15:34 ` [Patch 12/19] media: ti-vpe: cal: Add " Benoit Parrot
` (8 subsequent siblings)
19 siblings, 1 reply; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
Update Device Tree bindings for the CAL driver to add DRA76x support.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
Documentation/devicetree/bindings/media/ti-cal.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
index d43870a2324b..29fbbfb8be0d 100644
--- a/Documentation/devicetree/bindings/media/ti-cal.txt
+++ b/Documentation/devicetree/bindings/media/ti-cal.txt
@@ -10,6 +10,7 @@ Required properties:
- compatible:
Should be "ti,dra72-cal", for DRA72 controllers
Should be "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
+ Should be "ti,dra76-cal", for DRA76 controllers
- reg: CAL Top level, Receiver Core #0, Receiver Core #1 and Camera RX
control address space
- reg-names: cal_top, cal_rx_core0, cal_rx_core1 and camerrx_control
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 12/19] media: ti-vpe: cal: Add DRA76x support
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (10 preceding siblings ...)
2019-10-18 15:34 ` [Patch 11/19] dt-bindings: media: cal: update binding to add DRA76x support Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 13/19] dt-bindings: media: cal: update binding to add AM654 support Benoit Parrot
` (7 subsequent siblings)
19 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
Add the needed control module register bit layout to support
the DRA76x family of devices.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 36 +++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index c912ea82b534..f590fe712d3d 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -290,6 +290,38 @@ static struct cal_data dra72x_es1_cal_data = {
.flags = DRA72_CAL_PRE_ES2_LDO_DISABLE,
};
+static struct reg_field dra76x_ctrl_core_csi0_reg_fields[F_MAX_FIELDS] = {
+ [F_CTRLCLKEN] = REG_FIELD(0, 8, 8),
+ [F_CAMMODE] = REG_FIELD(0, 9, 10),
+ [F_CSI_MODE] = REG_FIELD(0, 11, 11),
+ [F_LANEENABLE] = REG_FIELD(0, 27, 31),
+};
+
+static struct reg_field dra76x_ctrl_core_csi1_reg_fields[F_MAX_FIELDS] = {
+ [F_CTRLCLKEN] = REG_FIELD(0, 0, 0),
+ [F_CAMMODE] = REG_FIELD(0, 1, 2),
+ [F_CSI_MODE] = REG_FIELD(0, 3, 3),
+ [F_LANEENABLE] = REG_FIELD(0, 24, 26),
+};
+
+static struct cal_csi2_phy dra76x_cal_csi_phy[] = {
+ {
+ .base_fields = dra76x_ctrl_core_csi0_reg_fields,
+ .num_lanes = 5,
+ },
+ {
+ .base_fields = dra76x_ctrl_core_csi1_reg_fields,
+ .num_lanes = 3,
+ },
+};
+
+static struct cal_data dra76x_cal_data = {
+ .csi2_phy_core = dra76x_cal_csi_phy,
+ .num_csi2_phy = ARRAY_SIZE(dra76x_cal_csi_phy),
+
+ .flags = 0,
+};
+
/*
* there is one cal_dev structure in the driver, it is shared by
* all instances.
@@ -2295,6 +2327,10 @@ static const struct of_device_id cal_of_match[] = {
.compatible = "ti,dra72-pre-es2-cal",
.data = (void *)&dra72x_es1_cal_data,
},
+ {
+ .compatible = "ti,dra76-cal",
+ .data = (void *)&dra76x_cal_data,
+ },
{},
};
MODULE_DEVICE_TABLE(of, cal_of_match);
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 13/19] dt-bindings: media: cal: update binding to add AM654 support
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (11 preceding siblings ...)
2019-10-18 15:34 ` [Patch 12/19] media: ti-vpe: cal: Add " Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-29 13:24 ` Rob Herring
2019-10-18 15:34 ` [Patch 14/19] media: ti-vpe: cal: Add " Benoit Parrot
` (6 subsequent siblings)
19 siblings, 1 reply; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
Update Device Tree bindings for the CAL driver to add AM654 support.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
Documentation/devicetree/bindings/media/ti-cal.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
index 29fbbfb8be0d..b7bf8492dc2a 100644
--- a/Documentation/devicetree/bindings/media/ti-cal.txt
+++ b/Documentation/devicetree/bindings/media/ti-cal.txt
@@ -11,6 +11,7 @@ Required properties:
Should be "ti,dra72-cal", for DRA72 controllers
Should be "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
Should be "ti,dra76-cal", for DRA76 controllers
+ Should be "ti,am654-cal", for AM654 controllers
- reg: CAL Top level, Receiver Core #0, Receiver Core #1 and Camera RX
control address space
- reg-names: cal_top, cal_rx_core0, cal_rx_core1 and camerrx_control
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 14/19] media: ti-vpe: cal: Add AM654 support
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (12 preceding siblings ...)
2019-10-18 15:34 ` [Patch 13/19] dt-bindings: media: cal: update binding to add AM654 support Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 15/19] media: ti-vpe: cal: Add subdev s_power hooks Benoit Parrot
` (5 subsequent siblings)
19 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
Add the needed control module register bit layout to support the AM654
family of devices.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/Kconfig | 2 +-
drivers/media/platform/ti-vpe/cal.c | 28 +++++++++++++++++++++++++++-
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 997de1a83ff9..24713e79c1ec 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -151,7 +151,7 @@ source "drivers/media/platform/sunxi/Kconfig"
config VIDEO_TI_CAL
tristate "TI CAL (Camera Adaptation Layer) driver"
depends on VIDEO_DEV && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
- depends on SOC_DRA7XX || COMPILE_TEST
+ depends on SOC_DRA7XX || ARCH_K3 || COMPILE_TEST
select VIDEOBUF2_DMA_CONTIG
select V4L2_FWNODE
help
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index f590fe712d3d..42b0063b610e 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -322,6 +322,26 @@ static struct cal_data dra76x_cal_data = {
.flags = 0,
};
+static struct reg_field am654_ctrl_core_csi0_reg_fields[F_MAX_FIELDS] = {
+ [F_CTRLCLKEN] = REG_FIELD(0, 15, 15),
+ [F_CAMMODE] = REG_FIELD(0, 24, 25),
+ [F_LANEENABLE] = REG_FIELD(0, 0, 4),
+};
+
+static struct cal_csi2_phy am654_cal_csi_phy[] = {
+ {
+ .base_fields = am654_ctrl_core_csi0_reg_fields,
+ .num_lanes = 5,
+ },
+};
+
+static struct cal_data am654_cal_data = {
+ .csi2_phy_core = am654_cal_csi_phy,
+ .num_csi2_phy = ARRAY_SIZE(am654_cal_csi_phy),
+
+ .flags = 0,
+};
+
/*
* there is one cal_dev structure in the driver, it is shared by
* all instances.
@@ -548,7 +568,9 @@ static void camerarx_phy_enable(struct cal_ctx *ctx)
/* Always enable all lanes at the phy control level */
max_lanes = (1 << cal_data_get_phy_max_lanes(ctx)) - 1;
regmap_field_write(phy->fields[F_LANEENABLE], max_lanes);
- regmap_field_write(phy->fields[F_CSI_MODE], 1);
+ /* F_CSI_MODE is not present on every architecture */
+ if (phy->fields[F_CSI_MODE])
+ regmap_field_write(phy->fields[F_CSI_MODE], 1);
regmap_field_write(phy->fields[F_CTRLCLKEN], 1);
}
@@ -2331,6 +2353,10 @@ static const struct of_device_id cal_of_match[] = {
.compatible = "ti,dra76-cal",
.data = (void *)&dra76x_cal_data,
},
+ {
+ .compatible = "ti,am654-cal",
+ .data = (void *)&am654_cal_data,
+ },
{},
};
MODULE_DEVICE_TABLE(of, cal_of_match);
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 15/19] media: ti-vpe: cal: Add subdev s_power hooks
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (13 preceding siblings ...)
2019-10-18 15:34 ` [Patch 14/19] media: ti-vpe: cal: Add " Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 16/19] media: ti-vpe: cal: Properly calculate max resolution boundary Benoit Parrot
` (4 subsequent siblings)
19 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
Based on V4L2 documentations bridge driver must power on and off sub
device explicitly if media-controller mode is not used.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 42b0063b610e..e61d2f3fd1da 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1661,6 +1661,12 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
if (ret < 0)
goto err;
+ ret = v4l2_subdev_call(ctx->sensor, core, s_power, 1);
+ if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV) {
+ ctx_err(ctx, "power on failed in subdev\n");
+ goto err;
+ }
+
cal_runtime_get(ctx->dev);
csi2_ctx_config(ctx);
@@ -1674,6 +1680,7 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
ret = v4l2_subdev_call(ctx->sensor, video, s_stream, 1);
if (ret) {
+ v4l2_subdev_call(ctx->sensor, core, s_power, 0);
ctx_err(ctx, "stream on failed in subdev\n");
cal_runtime_put(ctx->dev);
goto err;
@@ -1702,6 +1709,7 @@ static void cal_stop_streaming(struct vb2_queue *vq)
struct cal_dmaqueue *dma_q = &ctx->vidq;
struct cal_buffer *buf, *tmp;
unsigned long flags;
+ int ret;
csi2_ppi_disable(ctx);
disable_irqs(ctx);
@@ -1710,6 +1718,10 @@ static void cal_stop_streaming(struct vb2_queue *vq)
if (v4l2_subdev_call(ctx->sensor, video, s_stream, 0))
ctx_err(ctx, "stream off failed in subdev\n");
+ ret = v4l2_subdev_call(ctx->sensor, core, s_power, 0);
+ if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
+ ctx_err(ctx, "power off failed in subdev\n");
+
/* Release all active buffers */
spin_lock_irqsave(&ctx->slock, flags);
list_for_each_entry_safe(buf, tmp, &dma_q->active, list) {
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 16/19] media: ti-vpe: cal: Properly calculate max resolution boundary
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (14 preceding siblings ...)
2019-10-18 15:34 ` [Patch 15/19] media: ti-vpe: cal: Add subdev s_power hooks Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 17/19] media: ti-vpe: cal: Fix a WARN issued when start streaming fails Benoit Parrot
` (3 subsequent siblings)
19 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
Currently we were using an arbitrarily small maximum resolution mostly
based on available sensor capabilities. However the hardware DMA limits
are much higher than the statically define maximum resolution we were
using.
There we rework the boundary check code to handle the maximum width and
height based on the maximum line width in bytes and re-calculating the
pixel width based on the given pixel format.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index e61d2f3fd1da..a0ff67412410 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -34,8 +34,8 @@
#define CAL_MODULE_NAME "cal"
-#define MAX_WIDTH 1920
-#define MAX_HEIGHT 1200
+#define MAX_WIDTH_BYTES (8192 * 8)
+#define MAX_HEIGHT_LINES 16383
#define CAL_VERSION "0.1.0"
@@ -1337,15 +1337,21 @@ static int cal_calc_format_size(struct cal_ctx *ctx,
const struct cal_fmt *fmt,
struct v4l2_format *f)
{
- u32 bpl;
+ u32 bpl, max_width;
if (!fmt) {
ctx_dbg(3, ctx, "No cal_fmt provided!\n");
return -EINVAL;
}
- v4l_bound_align_image(&f->fmt.pix.width, 48, MAX_WIDTH, 2,
- &f->fmt.pix.height, 32, MAX_HEIGHT, 0, 0);
+ /*
+ * Maximum width is bound by the DMA max width in bytes.
+ * We need to recalculate the actual maxi width depending on the
+ * number of bytes per pixels required.
+ */
+ max_width = MAX_WIDTH_BYTES / (ALIGN(fmt->bpp, 8) >> 3);
+ v4l_bound_align_image(&f->fmt.pix.width, 48, max_width, 2,
+ &f->fmt.pix.height, 32, MAX_HEIGHT_LINES, 0, 0);
bpl = (f->fmt.pix.width * ALIGN(fmt->bpp, 8)) >> 3;
f->fmt.pix.bytesperline = ALIGN(bpl, 16);
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 17/19] media: ti-vpe: cal: Fix a WARN issued when start streaming fails
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (15 preceding siblings ...)
2019-10-18 15:34 ` [Patch 16/19] media: ti-vpe: cal: Properly calculate max resolution boundary Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 18/19] media: ti-vpe: cal: fix enum_mbus_code/frame_size subdev arguments Benoit Parrot
` (2 subsequent siblings)
19 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
When start_streaming fails after the buffers have been queued we have to
make sure all buffers are returned to user-space properly otherwise a
v4l2 level WARN is generated.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index a0ff67412410..b55987ee70dd 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1702,10 +1702,15 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
return 0;
err:
+ spin_lock_irqsave(&ctx->slock, flags);
+ vb2_buffer_done(&ctx->cur_frm->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
+ ctx->cur_frm = NULL;
+ ctx->next_frm = NULL;
list_for_each_entry_safe(buf, tmp, &dma_q->active, list) {
list_del(&buf->list);
vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
}
+ spin_unlock_irqrestore(&ctx->slock, flags);
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 18/19] media: ti-vpe: cal: fix enum_mbus_code/frame_size subdev arguments
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (16 preceding siblings ...)
2019-10-18 15:34 ` [Patch 17/19] media: ti-vpe: cal: Fix a WARN issued when start streaming fails Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 19/19] dt-bindings: media: cal: convert binding to yaml Benoit Parrot
2019-10-21 10:50 ` [Patch 00/19] media: ti-vpe: cal: maintenance Hans Verkuil
19 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
Make sure that both enum_mbus_code() and enum_framesize() properly
populate the .which parameter member, otherwise -EINVAL is return
causing the subdev asynchronous registration handshake to fail.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
drivers/media/platform/ti-vpe/cal.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index b55987ee70dd..6a404350edb3 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1500,6 +1500,7 @@ static int cal_enum_framesizes(struct file *file, void *fh,
fse.index = fsize->index;
fse.pad = 0;
fse.code = fmt->code;
+ fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(ctx->sensor, pad, enum_frame_size, NULL, &fse);
if (ret)
@@ -1840,6 +1841,7 @@ static int cal_async_bound(struct v4l2_async_notifier *notifier,
memset(&mbus_code, 0, sizeof(mbus_code));
mbus_code.index = j;
+ mbus_code.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(subdev, pad, enum_mbus_code,
NULL, &mbus_code);
if (ret)
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Patch 19/19] dt-bindings: media: cal: convert binding to yaml
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (17 preceding siblings ...)
2019-10-18 15:34 ` [Patch 18/19] media: ti-vpe: cal: fix enum_mbus_code/frame_size subdev arguments Benoit Parrot
@ 2019-10-18 15:34 ` Benoit Parrot
2019-10-21 10:49 ` Hans Verkuil
` (2 more replies)
2019-10-21 10:50 ` [Patch 00/19] media: ti-vpe: cal: maintenance Hans Verkuil
19 siblings, 3 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-18 15:34 UTC (permalink / raw)
To: Hans Verkuil, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Benoit Parrot
Convert ti-cal.txt to ti,cal.yaml.
Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
.../devicetree/bindings/media/ti,cal.yaml | 186 ++++++++++++++++++
.../devicetree/bindings/media/ti-cal.txt | 82 --------
2 files changed, 186 insertions(+), 82 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/ti,cal.yaml
delete mode 100644 Documentation/devicetree/bindings/media/ti-cal.txt
diff --git a/Documentation/devicetree/bindings/media/ti,cal.yaml b/Documentation/devicetree/bindings/media/ti,cal.yaml
new file mode 100644
index 000000000000..c3fbb22b4571
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,cal.yaml
@@ -0,0 +1,186 @@
+# SPDX-License-Identifier: (GPL-2.0)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/ti,cal.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL) Device Tree Bindings
+
+maintainers:
+ - Benoit Parrot <bparrot@ti.com>
+
+description: |-
+ The Camera Adaptation Layer (CAL) is a key component for image capture
+ applications. The capture module provides the system interface and the
+ processing capability to connect CSI2 image-sensor modules to the
+ DRA72x device.
+
+ CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
+ should contain a 'port' child node with child 'endpoint' node. Please
+ refer to the bindings defined in
+ Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+ compatible should be
+ "ti,dra72-cal", for DRA72 controllers
+ "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
+ "ti,dra76-cal", for DRA76 controllers
+ "ti,am654-cal", for AM654 controllers
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - ti,dra72-cal
+ - ti,dra72-pre-es2-cal
+ - ti,dra76-cal
+ - ti,am654-cal
+
+ reg:
+ minItems: 2
+ items:
+ - description: The CAL main register region
+ - description: The RX Core0 (DPHY0) register region
+ - description: The RX Core1 (DPHY1) register region
+
+ reg-names:
+ minItems: 2
+ items:
+ - const: cal_top
+ - const: cal_rx_core0
+ - const: cal_rx_core1
+
+ interrupts:
+ maxItems: 1
+
+ syscon-camerrx:
+ maxItems: 1
+ items:
+ - description:
+ phandle to the device control module and offset to the
+ control_camerarx_core register
+
+ clocks:
+ maxItems: 1
+ description: functional clock
+
+ clock-names:
+ items:
+ - const: fck
+
+ power-domains:
+ description:
+ List of phandle and PM domain specifier as documented in
+ Documentation/devicetree/bindings/power/power_domain.txt
+ maxItems: 1
+
+ # See ./video-interfaces.txt for details
+ ports:
+ maxItems: 1
+ type: object
+ additionalProperties: false
+
+ properties:
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ patternProperties:
+ '^port@[0-9a-fA-F]+$':
+ minItems: 1
+ maxItems: 2
+ type: object
+ additionalProperties: false
+
+ properties:
+ reg:
+ minItems: 1
+ items:
+ - description: The port id
+
+ patternProperties:
+ '^endpoint@[0-9a-fA-F]+$':
+ minItems: 1
+ type: object
+ additionalProperties: false
+
+ properties:
+ clock-lanes:
+ maxItems: 1
+
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ remote-endpoint: true
+
+ required:
+ - remote-endpoint
+
+ required:
+ - reg
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - syscon-camerrx
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ cal: cal@4845b000 {
+ compatible = "ti,dra72-cal";
+ reg = <0x4845B000 0x400>,
+ <0x4845B800 0x40>,
+ <0x4845B900 0x40>;
+ reg-names = "cal_top",
+ "cal_rx_core0",
+ "cal_rx_core1";
+ interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+ syscon-camerrx = <&scm_conf 0xE94>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ csi2_0: port@0 {
+ reg = <0>;
+ csi2_phy0: endpoint@0 {
+ remote-endpoint = <&csi2_cam0>;
+ clock-lanes = <0>;
+ data-lanes = <1 2>;
+ };
+ };
+ };
+ };
+
+ i2c5: i2c@4807c000 {
+ status = "okay";
+ clock-frequency = <400000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ov5640@3c {
+ compatible = "ovti,ov5640";
+ reg = <0x3c>;
+
+ clocks = <&clk_ov5640_fixed>;
+ clock-names = "xclk";
+
+ port {
+ csi2_cam0: endpoint@0 {
+ remote-endpoint = <&csi2_phy0>;
+ clock-lanes = <0>;
+ data-lanes = <1 2>;
+ };
+ };
+ };
+ };
+
+...
diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
deleted file mode 100644
index b7bf8492dc2a..000000000000
--- a/Documentation/devicetree/bindings/media/ti-cal.txt
+++ /dev/null
@@ -1,82 +0,0 @@
-Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL)
-------------------------------------------------------
-
-The Camera Adaptation Layer (CAL) is a key component for image capture
-applications. The capture module provides the system interface and the
-processing capability to connect CSI2 image-sensor modules to the
-DRA72x device.
-
-Required properties:
-- compatible:
- Should be "ti,dra72-cal", for DRA72 controllers
- Should be "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
- Should be "ti,dra76-cal", for DRA76 controllers
- Should be "ti,am654-cal", for AM654 controllers
-- reg: CAL Top level, Receiver Core #0, Receiver Core #1 and Camera RX
- control address space
-- reg-names: cal_top, cal_rx_core0, cal_rx_core1 and camerrx_control
- registers
-- interrupts: should contain IRQ line for the CAL;
-- syscon-camerrx: phandle to the device control module and offset to the
- control_camerarx_core register
- This node is meant to replace the "camerrx_control" reg
- entry above but "camerrx_control" is still handled
- for backward compatibility.
-
-CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
-should contain a 'port' child node with child 'endpoint' node. Please
-refer to the bindings defined in
-Documentation/devicetree/bindings/media/video-interfaces.txt.
-
-Example:
- cal: cal@4845b000 {
- compatible = "ti,dra72-cal";
- reg = <0x4845B000 0x400>,
- <0x4845B800 0x40>,
- <0x4845B900 0x40>;
- reg-names = "cal_top",
- "cal_rx_core0",
- "cal_rx_core1";
- interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
- syscon-camerrx = <&scm_conf 0xE94>;
- #address-cells = <1>;
- #size-cells = <0>;
-
- ports {
- #address-cells = <1>;
- #size-cells = <0>;
-
- csi2_0: port@0 {
- reg = <0>;
- csi2_phy0: endpoint@0 {
- remote-endpoint = <&csi2_cam0>;
- clock-lanes = <0>;
- data-lanes = <1 2>;
- };
- };
- csi2_1: port@1 {
- reg = <1>;
- };
- };
- };
-
- i2c5: i2c@4807c000 {
- status = "okay";
- clock-frequency = <400000>;
-
- ov5640@3c {
- compatible = "ovti,ov5640";
- reg = <0x3c>;
-
- clocks = <&clk_ov5640_fixed>;
- clock-names = "xclk";
-
- port {
- csi2_cam0: endpoint {
- remote-endpoint = <&csi2_phy0>;
- clock-lanes = <0>;
- data-lanes = <1 2>;
- };
- };
- };
- };
--
2.17.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [Patch 07/19] media: ti-vpe: cal: add CSI2 PHY LDO errata support
2019-10-18 15:34 ` [Patch 07/19] media: ti-vpe: cal: add CSI2 " Benoit Parrot
@ 2019-10-21 10:38 ` Hans Verkuil
2019-10-23 16:03 ` Benoit Parrot
0 siblings, 1 reply; 40+ messages in thread
From: Hans Verkuil @ 2019-10-21 10:38 UTC (permalink / raw)
To: Benoit Parrot, Rob Herring
Cc: linux-media, devicetree, linux-kernel, Jyri Sarha
On 10/18/19 5:34 PM, Benoit Parrot wrote:
> Apply Errata i913 every time the functional clock is enabled.
> This should take care of suspend/resume case as well.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
> drivers/media/platform/ti-vpe/cal.c | 56 +++++++++++++++++++++++-
> drivers/media/platform/ti-vpe/cal_regs.h | 27 ++++++++++++
> 2 files changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index 62aeedb705e9..3cbc4dca6de8 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -284,6 +284,13 @@ static struct cal_data dra72x_cal_data = {
> .flags = 0,
> };
>
> +static struct cal_data dra72x_es1_cal_data = {
> + .csi2_phy_core = dra72x_cal_csi_phy,
> + .num_csi2_phy = ARRAY_SIZE(dra72x_cal_csi_phy),
> +
> + .flags = DRA72_CAL_PRE_ES2_LDO_DISABLE,
> +};
> +
> /*
> * there is one cal_dev structure in the driver, it is shared by
> * all instances.
> @@ -569,9 +576,52 @@ static void cal_get_hwinfo(struct cal_dev *dev)
> hwinfo);
> }
>
> +/*
> + * Errata i913: CSI2 LDO Needs to be disabled when module is powered on
> + *
> + * Enabling CSI2 LDO shorts it to core supply. It is crucial the 2 CSI2
> + * LDOs on the device are disabled if CSI-2 module is powered on
> + * (0x4845 B304 | 0x4845 B384 [28:27] = 0x1) or in ULPS (0x4845 B304
> + * | 0x4845 B384 [28:27] = 0x2) mode. Common concerns include: high
> + * current draw on the module supply in active mode.
> + *
> + * Errata does not apply when CSI-2 module is powered off
> + * (0x4845 B304 | 0x4845 B384 [28:27] = 0x0).
> + *
> + * SW Workaround:
> + * Set the following register bits to disable the LDO,
> + * which is essentially CSI2 REG10 bit 6:
> + *
> + * Core 0: 0x4845 B828 = 0x0000 0040
> + * Core 1: 0x4845 B928 = 0x0000 0040
> + */
> +static void i913_errata(struct cal_dev *dev, unsigned int port)
> +{
> + u32 reg10 = reg_read(dev->cc[port], CAL_CSI2_PHY_REG10);
> +
> + set_field(®10, CAL_CSI2_PHY_REG0_HSCLOCKCONFIG_DISABLE,
> + CAL_CSI2_PHY_REG10_I933_LDO_DISABLE_MASK);
> +
> + cal_dbg(1, dev, "CSI2_%d_REG10 = 0x%08x\n", port, reg10);
> + reg_write(dev->cc[port], CAL_CSI2_PHY_REG10, reg10);
> +}
> +
> static inline int cal_runtime_get(struct cal_dev *dev)
I'd drop the 'inline' here. It doesn't seem appropriate anymore since this
function is now more complex.
Regards,
Hans
> {
> - return pm_runtime_get_sync(&dev->pdev->dev);
> + int r;
> +
> + r = pm_runtime_get_sync(&dev->pdev->dev);
> +
> + if (dev->flags & DRA72_CAL_PRE_ES2_LDO_DISABLE) {
> + /*
> + * Apply errata on both port eveytime we (re-)enable
> + * the clock
> + */
> + i913_errata(dev, 0);
> + i913_errata(dev, 1);
> + }
> +
> + return r;
> }
>
> static inline void cal_runtime_put(struct cal_dev *dev)
> @@ -2071,6 +2121,10 @@ static const struct of_device_id cal_of_match[] = {
> .compatible = "ti,dra72-cal",
> .data = (void *)&dra72x_cal_data,
> },
> + {
> + .compatible = "ti,dra72-pre-es2-cal",
> + .data = (void *)&dra72x_es1_cal_data,
> + },
> {},
> };
> MODULE_DEVICE_TABLE(of, cal_of_match);
> diff --git a/drivers/media/platform/ti-vpe/cal_regs.h b/drivers/media/platform/ti-vpe/cal_regs.h
> index 68cfc922b422..78d6f015c9ea 100644
> --- a/drivers/media/platform/ti-vpe/cal_regs.h
> +++ b/drivers/media/platform/ti-vpe/cal_regs.h
> @@ -10,6 +10,30 @@
> #ifndef __TI_CAL_REGS_H
> #define __TI_CAL_REGS_H
>
> +/*
> + * struct cal_dev.flags possibilities
> + *
> + * DRA72_CAL_PRE_ES2_LDO_DISABLE:
> + * Errata i913: CSI2 LDO Needs to be disabled when module is powered on
> + *
> + * Enabling CSI2 LDO shorts it to core supply. It is crucial the 2 CSI2
> + * LDOs on the device are disabled if CSI-2 module is powered on
> + * (0x4845 B304 | 0x4845 B384 [28:27] = 0x1) or in ULPS (0x4845 B304
> + * | 0x4845 B384 [28:27] = 0x2) mode. Common concerns include: high
> + * current draw on the module supply in active mode.
> + *
> + * Errata does not apply when CSI-2 module is powered off
> + * (0x4845 B304 | 0x4845 B384 [28:27] = 0x0).
> + *
> + * SW Workaround:
> + * Set the following register bits to disable the LDO,
> + * which is essentially CSI2 REG10 bit 6:
> + *
> + * Core 0: 0x4845 B828 = 0x0000 0040
> + * Core 1: 0x4845 B928 = 0x0000 0040
> + */
> +#define DRA72_CAL_PRE_ES2_LDO_DISABLE BIT(0)
> +
> #define CAL_NUM_CSI2_PORTS 2
>
> /* CAL register offsets */
> @@ -71,6 +95,7 @@
> #define CAL_CSI2_PHY_REG0 0x000
> #define CAL_CSI2_PHY_REG1 0x004
> #define CAL_CSI2_PHY_REG2 0x008
> +#define CAL_CSI2_PHY_REG10 0x028
>
> /* CAL Control Module Core Camerrx Control register offsets */
> #define CM_CTRL_CORE_CAMERRX_CONTROL 0x000
> @@ -458,6 +483,8 @@
> #define CAL_CSI2_PHY_REG1_CLOCK_MISS_DETECTOR_STATUS_SUCCESS 0
> #define CAL_CSI2_PHY_REG1_RESET_DONE_STATUS_MASK GENMASK(29, 28)
>
> +#define CAL_CSI2_PHY_REG10_I933_LDO_DISABLE_MASK BIT_MASK(6)
> +
> #define CAL_CSI2_PHY_REG2_CCP2_SYNC_PATTERN_MASK GENMASK(23, 0)
> #define CAL_CSI2_PHY_REG2_TRIGGER_CMD_RXTRIGESC3_MASK GENMASK(25, 24)
> #define CAL_CSI2_PHY_REG2_TRIGGER_CMD_RXTRIGESC2_MASK GENMASK(27, 26)
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 19/19] dt-bindings: media: cal: convert binding to yaml
2019-10-18 15:34 ` [Patch 19/19] dt-bindings: media: cal: convert binding to yaml Benoit Parrot
@ 2019-10-21 10:49 ` Hans Verkuil
2019-10-23 16:06 ` Benoit Parrot
2019-10-22 7:46 ` Sakari Ailus
2019-10-29 14:22 ` Rob Herring
2 siblings, 1 reply; 40+ messages in thread
From: Hans Verkuil @ 2019-10-21 10:49 UTC (permalink / raw)
To: Benoit Parrot, Rob Herring; +Cc: linux-media, devicetree, linux-kernel
On 10/18/19 5:34 PM, Benoit Parrot wrote:
> Convert ti-cal.txt to ti,cal.yaml.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
Can you add this bindings file to MAINTAINERS as well in a separate patch?
Thanks!
Hans
> ---
> .../devicetree/bindings/media/ti,cal.yaml | 186 ++++++++++++++++++
> .../devicetree/bindings/media/ti-cal.txt | 82 --------
> 2 files changed, 186 insertions(+), 82 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/ti,cal.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/ti-cal.txt
>
> diff --git a/Documentation/devicetree/bindings/media/ti,cal.yaml b/Documentation/devicetree/bindings/media/ti,cal.yaml
> new file mode 100644
> index 000000000000..c3fbb22b4571
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/ti,cal.yaml
> @@ -0,0 +1,186 @@
> +# SPDX-License-Identifier: (GPL-2.0)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/ti,cal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL) Device Tree Bindings
> +
> +maintainers:
> + - Benoit Parrot <bparrot@ti.com>
> +
> +description: |-
> + The Camera Adaptation Layer (CAL) is a key component for image capture
> + applications. The capture module provides the system interface and the
> + processing capability to connect CSI2 image-sensor modules to the
> + DRA72x device.
> +
> + CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> + should contain a 'port' child node with child 'endpoint' node. Please
> + refer to the bindings defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> + compatible should be
> + "ti,dra72-cal", for DRA72 controllers
> + "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
> + "ti,dra76-cal", for DRA76 controllers
> + "ti,am654-cal", for AM654 controllers
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - ti,dra72-cal
> + - ti,dra72-pre-es2-cal
> + - ti,dra76-cal
> + - ti,am654-cal
> +
> + reg:
> + minItems: 2
> + items:
> + - description: The CAL main register region
> + - description: The RX Core0 (DPHY0) register region
> + - description: The RX Core1 (DPHY1) register region
> +
> + reg-names:
> + minItems: 2
> + items:
> + - const: cal_top
> + - const: cal_rx_core0
> + - const: cal_rx_core1
> +
> + interrupts:
> + maxItems: 1
> +
> + syscon-camerrx:
> + maxItems: 1
> + items:
> + - description:
> + phandle to the device control module and offset to the
> + control_camerarx_core register
> +
> + clocks:
> + maxItems: 1
> + description: functional clock
> +
> + clock-names:
> + items:
> + - const: fck
> +
> + power-domains:
> + description:
> + List of phandle and PM domain specifier as documented in
> + Documentation/devicetree/bindings/power/power_domain.txt
> + maxItems: 1
> +
> + # See ./video-interfaces.txt for details
> + ports:
> + maxItems: 1
> + type: object
> + additionalProperties: false
> +
> + properties:
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + patternProperties:
> + '^port@[0-9a-fA-F]+$':
> + minItems: 1
> + maxItems: 2
> + type: object
> + additionalProperties: false
> +
> + properties:
> + reg:
> + minItems: 1
> + items:
> + - description: The port id
> +
> + patternProperties:
> + '^endpoint@[0-9a-fA-F]+$':
> + minItems: 1
> + type: object
> + additionalProperties: false
> +
> + properties:
> + clock-lanes:
> + maxItems: 1
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + remote-endpoint: true
> +
> + required:
> + - remote-endpoint
> +
> + required:
> + - reg
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> + - syscon-camerrx
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + cal: cal@4845b000 {
> + compatible = "ti,dra72-cal";
> + reg = <0x4845B000 0x400>,
> + <0x4845B800 0x40>,
> + <0x4845B900 0x40>;
> + reg-names = "cal_top",
> + "cal_rx_core0",
> + "cal_rx_core1";
> + interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> + syscon-camerrx = <&scm_conf 0xE94>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + csi2_0: port@0 {
> + reg = <0>;
> + csi2_phy0: endpoint@0 {
> + remote-endpoint = <&csi2_cam0>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
> +
> + i2c5: i2c@4807c000 {
> + status = "okay";
> + clock-frequency = <400000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ov5640@3c {
> + compatible = "ovti,ov5640";
> + reg = <0x3c>;
> +
> + clocks = <&clk_ov5640_fixed>;
> + clock-names = "xclk";
> +
> + port {
> + csi2_cam0: endpoint@0 {
> + remote-endpoint = <&csi2_phy0>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
> deleted file mode 100644
> index b7bf8492dc2a..000000000000
> --- a/Documentation/devicetree/bindings/media/ti-cal.txt
> +++ /dev/null
> @@ -1,82 +0,0 @@
> -Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL)
> -------------------------------------------------------
> -
> -The Camera Adaptation Layer (CAL) is a key component for image capture
> -applications. The capture module provides the system interface and the
> -processing capability to connect CSI2 image-sensor modules to the
> -DRA72x device.
> -
> -Required properties:
> -- compatible:
> - Should be "ti,dra72-cal", for DRA72 controllers
> - Should be "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
> - Should be "ti,dra76-cal", for DRA76 controllers
> - Should be "ti,am654-cal", for AM654 controllers
> -- reg: CAL Top level, Receiver Core #0, Receiver Core #1 and Camera RX
> - control address space
> -- reg-names: cal_top, cal_rx_core0, cal_rx_core1 and camerrx_control
> - registers
> -- interrupts: should contain IRQ line for the CAL;
> -- syscon-camerrx: phandle to the device control module and offset to the
> - control_camerarx_core register
> - This node is meant to replace the "camerrx_control" reg
> - entry above but "camerrx_control" is still handled
> - for backward compatibility.
> -
> -CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> -should contain a 'port' child node with child 'endpoint' node. Please
> -refer to the bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Example:
> - cal: cal@4845b000 {
> - compatible = "ti,dra72-cal";
> - reg = <0x4845B000 0x400>,
> - <0x4845B800 0x40>,
> - <0x4845B900 0x40>;
> - reg-names = "cal_top",
> - "cal_rx_core0",
> - "cal_rx_core1";
> - interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> - syscon-camerrx = <&scm_conf 0xE94>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - ports {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - csi2_0: port@0 {
> - reg = <0>;
> - csi2_phy0: endpoint@0 {
> - remote-endpoint = <&csi2_cam0>;
> - clock-lanes = <0>;
> - data-lanes = <1 2>;
> - };
> - };
> - csi2_1: port@1 {
> - reg = <1>;
> - };
> - };
> - };
> -
> - i2c5: i2c@4807c000 {
> - status = "okay";
> - clock-frequency = <400000>;
> -
> - ov5640@3c {
> - compatible = "ovti,ov5640";
> - reg = <0x3c>;
> -
> - clocks = <&clk_ov5640_fixed>;
> - clock-names = "xclk";
> -
> - port {
> - csi2_cam0: endpoint {
> - remote-endpoint = <&csi2_phy0>;
> - clock-lanes = <0>;
> - data-lanes = <1 2>;
> - };
> - };
> - };
> - };
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 00/19] media: ti-vpe: cal: maintenance
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
` (18 preceding siblings ...)
2019-10-18 15:34 ` [Patch 19/19] dt-bindings: media: cal: convert binding to yaml Benoit Parrot
@ 2019-10-21 10:50 ` Hans Verkuil
2019-10-23 16:05 ` Benoit Parrot
19 siblings, 1 reply; 40+ messages in thread
From: Hans Verkuil @ 2019-10-21 10:50 UTC (permalink / raw)
To: Benoit Parrot, Rob Herring; +Cc: linux-media, devicetree, linux-kernel
Hi Benoit,
This series looks good to me. I had just one small comment on patch 7
and a comment on patch 19.
Just post a v2 for just patch 7/19 and post a patch 20/19 for the requested
MAINTAINERS change.
Once I have Rob's Acks I can merge this.
Regards,
Hans
On 10/18/19 5:34 PM, Benoit Parrot wrote:
> This a collection of backlog patches I have been carrying for the CAL
> driver.
>
> - Add support for SoC variants.
>
> - Switches to syscon/regmap to access a system controller register for
> the DPHY configuration. This register has different bit layout depending
> on the SoC version.
>
> - It adds supports for pre ES2.0 silicon errata.
>
> - Reworked the DPHY initialization sequence to match the technical
> reference manual and provide a more robust restartability.
>
> - Adds the missing ability to power subdevice.
>
> - Update the devicetree binding and then converts it to dt-schema
>
> Benoit Parrot (18):
> dt-bindings: media: cal: update binding to use syscon
> dt-bindings: media: cal: update binding example
> media: ti-vpe: cal: Add per platform data support
> media: ti-vpe: cal: Enable DMABUF export
> dt-bindings: media: cal: update binding to add PHY LDO errata support
> media: ti-vpe: cal: add CSI2 PHY LDO errata support
> media: ti-vpe: cal: Fix ths_term/ths_settle parameters
> media: ti-vpe: cal: Fix pixel processing parameters
> media: ti-vpe: cal: Align DPHY init sequence with docs
> dt-bindings: media: cal: update binding to add DRA76x support
> media: ti-vpe: cal: Add DRA76x support
> dt-bindings: media: cal: update binding to add AM654 support
> media: ti-vpe: cal: Add AM654 support
> media: ti-vpe: cal: Add subdev s_power hooks
> media: ti-vpe: cal: Properly calculate max resolution boundary
> media: ti-vpe: cal: Fix a WARN issued when start streaming fails
> media: ti-vpe: cal: fix enum_mbus_code/frame_size subdev arguments
> dt-bindings: media: cal: convert binding to yaml
>
> Nikhil Devshatwar (1):
> media: ti-vpe: cal: Restrict DMA to avoid memory corruption
>
> .../devicetree/bindings/media/ti,cal.yaml | 186 +++++
> .../devicetree/bindings/media/ti-cal.txt | 72 --
> drivers/media/platform/Kconfig | 2 +-
> drivers/media/platform/ti-vpe/cal.c | 775 ++++++++++++++----
> drivers/media/platform/ti-vpe/cal_regs.h | 27 +
> 5 files changed, 830 insertions(+), 232 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/ti,cal.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/ti-cal.txt
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 19/19] dt-bindings: media: cal: convert binding to yaml
2019-10-18 15:34 ` [Patch 19/19] dt-bindings: media: cal: convert binding to yaml Benoit Parrot
2019-10-21 10:49 ` Hans Verkuil
@ 2019-10-22 7:46 ` Sakari Ailus
2019-10-23 16:18 ` Benoit Parrot
2019-10-29 14:22 ` Rob Herring
2 siblings, 1 reply; 40+ messages in thread
From: Sakari Ailus @ 2019-10-22 7:46 UTC (permalink / raw)
To: Benoit Parrot
Cc: Hans Verkuil, Rob Herring, linux-media, devicetree, linux-kernel
Hi Benoit,
Thanks for the patch.
On Fri, Oct 18, 2019 at 10:34:37AM -0500, Benoit Parrot wrote:
> Convert ti-cal.txt to ti,cal.yaml.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
> .../devicetree/bindings/media/ti,cal.yaml | 186 ++++++++++++++++++
> .../devicetree/bindings/media/ti-cal.txt | 82 --------
> 2 files changed, 186 insertions(+), 82 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/ti,cal.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/ti-cal.txt
>
> diff --git a/Documentation/devicetree/bindings/media/ti,cal.yaml b/Documentation/devicetree/bindings/media/ti,cal.yaml
> new file mode 100644
> index 000000000000..c3fbb22b4571
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/ti,cal.yaml
> @@ -0,0 +1,186 @@
> +# SPDX-License-Identifier: (GPL-2.0)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/ti,cal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL) Device Tree Bindings
> +
> +maintainers:
> + - Benoit Parrot <bparrot@ti.com>
> +
> +description: |-
> + The Camera Adaptation Layer (CAL) is a key component for image capture
> + applications. The capture module provides the system interface and the
> + processing capability to connect CSI2 image-sensor modules to the
> + DRA72x device.
> +
> + CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> + should contain a 'port' child node with child 'endpoint' node. Please
> + refer to the bindings defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> + compatible should be
> + "ti,dra72-cal", for DRA72 controllers
> + "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
> + "ti,dra76-cal", for DRA76 controllers
> + "ti,am654-cal", for AM654 controllers
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - ti,dra72-cal
> + - ti,dra72-pre-es2-cal
> + - ti,dra76-cal
> + - ti,am654-cal
> +
> + reg:
> + minItems: 2
> + items:
> + - description: The CAL main register region
> + - description: The RX Core0 (DPHY0) register region
> + - description: The RX Core1 (DPHY1) register region
> +
> + reg-names:
> + minItems: 2
> + items:
> + - const: cal_top
> + - const: cal_rx_core0
> + - const: cal_rx_core1
> +
> + interrupts:
> + maxItems: 1
> +
> + syscon-camerrx:
> + maxItems: 1
> + items:
> + - description:
> + phandle to the device control module and offset to the
> + control_camerarx_core register
> +
> + clocks:
> + maxItems: 1
> + description: functional clock
> +
> + clock-names:
> + items:
> + - const: fck
> +
> + power-domains:
> + description:
> + List of phandle and PM domain specifier as documented in
> + Documentation/devicetree/bindings/power/power_domain.txt
> + maxItems: 1
> +
> + # See ./video-interfaces.txt for details
> + ports:
> + maxItems: 1
> + type: object
> + additionalProperties: false
> +
> + properties:
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + patternProperties:
> + '^port@[0-9a-fA-F]+$':
> + minItems: 1
> + maxItems: 2
Obviously you need a port node to connect a sensor. But can the device do
something useful without one? I guess it may be a matter of taste whether
you require one.
> + type: object
> + additionalProperties: false
> +
> + properties:
> + reg:
> + minItems: 1
> + items:
> + - description: The port id
> +
> + patternProperties:
> + '^endpoint@[0-9a-fA-F]+$':
> + minItems: 1
The graph bindings allows for no endpoints.
> + type: object
> + additionalProperties: false
> +
> + properties:
> + clock-lanes:
> + maxItems: 1
Does the device support lane reordering? If not, you could omit the
clock-lanes property. It wasn't documented earlier either albeit the
example had it for some reason.
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + remote-endpoint: true
> +
> + required:
> + - remote-endpoint
> +
> + required:
> + - reg
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> + - syscon-camerrx
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + cal: cal@4845b000 {
> + compatible = "ti,dra72-cal";
> + reg = <0x4845B000 0x400>,
> + <0x4845B800 0x40>,
> + <0x4845B900 0x40>;
> + reg-names = "cal_top",
> + "cal_rx_core0",
> + "cal_rx_core1";
> + interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> + syscon-camerrx = <&scm_conf 0xE94>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + csi2_0: port@0 {
> + reg = <0>;
> + csi2_phy0: endpoint@0 {
> + remote-endpoint = <&csi2_cam0>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
> +
> + i2c5: i2c@4807c000 {
> + status = "okay";
> + clock-frequency = <400000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ov5640@3c {
> + compatible = "ovti,ov5640";
> + reg = <0x3c>;
> +
> + clocks = <&clk_ov5640_fixed>;
> + clock-names = "xclk";
> +
> + port {
> + csi2_cam0: endpoint@0 {
> + remote-endpoint = <&csi2_phy0>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
> deleted file mode 100644
> index b7bf8492dc2a..000000000000
> --- a/Documentation/devicetree/bindings/media/ti-cal.txt
> +++ /dev/null
> @@ -1,82 +0,0 @@
> -Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL)
> -------------------------------------------------------
> -
> -The Camera Adaptation Layer (CAL) is a key component for image capture
> -applications. The capture module provides the system interface and the
> -processing capability to connect CSI2 image-sensor modules to the
> -DRA72x device.
> -
> -Required properties:
> -- compatible:
> - Should be "ti,dra72-cal", for DRA72 controllers
> - Should be "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
> - Should be "ti,dra76-cal", for DRA76 controllers
> - Should be "ti,am654-cal", for AM654 controllers
> -- reg: CAL Top level, Receiver Core #0, Receiver Core #1 and Camera RX
> - control address space
> -- reg-names: cal_top, cal_rx_core0, cal_rx_core1 and camerrx_control
> - registers
> -- interrupts: should contain IRQ line for the CAL;
> -- syscon-camerrx: phandle to the device control module and offset to the
> - control_camerarx_core register
> - This node is meant to replace the "camerrx_control" reg
> - entry above but "camerrx_control" is still handled
> - for backward compatibility.
> -
> -CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> -should contain a 'port' child node with child 'endpoint' node. Please
> -refer to the bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Example:
> - cal: cal@4845b000 {
> - compatible = "ti,dra72-cal";
> - reg = <0x4845B000 0x400>,
> - <0x4845B800 0x40>,
> - <0x4845B900 0x40>;
> - reg-names = "cal_top",
> - "cal_rx_core0",
> - "cal_rx_core1";
> - interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> - syscon-camerrx = <&scm_conf 0xE94>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - ports {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - csi2_0: port@0 {
> - reg = <0>;
> - csi2_phy0: endpoint@0 {
> - remote-endpoint = <&csi2_cam0>;
> - clock-lanes = <0>;
> - data-lanes = <1 2>;
> - };
> - };
> - csi2_1: port@1 {
> - reg = <1>;
> - };
> - };
> - };
> -
> - i2c5: i2c@4807c000 {
> - status = "okay";
> - clock-frequency = <400000>;
> -
> - ov5640@3c {
> - compatible = "ovti,ov5640";
> - reg = <0x3c>;
> -
> - clocks = <&clk_ov5640_fixed>;
> - clock-names = "xclk";
> -
> - port {
> - csi2_cam0: endpoint {
> - remote-endpoint = <&csi2_phy0>;
> - clock-lanes = <0>;
> - data-lanes = <1 2>;
> - };
> - };
> - };
> - };
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 07/19] media: ti-vpe: cal: add CSI2 PHY LDO errata support
2019-10-21 10:38 ` Hans Verkuil
@ 2019-10-23 16:03 ` Benoit Parrot
0 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-23 16:03 UTC (permalink / raw)
To: Hans Verkuil
Cc: Rob Herring, linux-media, devicetree, linux-kernel, Jyri Sarha
Hans Verkuil <hverkuil@xs4all.nl> wrote on Mon [2019-Oct-21 12:38:03 +0200]:
> On 10/18/19 5:34 PM, Benoit Parrot wrote:
> > Apply Errata i913 every time the functional clock is enabled.
> > This should take care of suspend/resume case as well.
> >
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > Signed-off-by: Jyri Sarha <jsarha@ti.com>
> > ---
> > drivers/media/platform/ti-vpe/cal.c | 56 +++++++++++++++++++++++-
> > drivers/media/platform/ti-vpe/cal_regs.h | 27 ++++++++++++
> > 2 files changed, 82 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> > index 62aeedb705e9..3cbc4dca6de8 100644
> > --- a/drivers/media/platform/ti-vpe/cal.c
> > +++ b/drivers/media/platform/ti-vpe/cal.c
> > @@ -284,6 +284,13 @@ static struct cal_data dra72x_cal_data = {
> > .flags = 0,
> > };
> >
> > +static struct cal_data dra72x_es1_cal_data = {
> > + .csi2_phy_core = dra72x_cal_csi_phy,
> > + .num_csi2_phy = ARRAY_SIZE(dra72x_cal_csi_phy),
> > +
> > + .flags = DRA72_CAL_PRE_ES2_LDO_DISABLE,
> > +};
> > +
> > /*
> > * there is one cal_dev structure in the driver, it is shared by
> > * all instances.
> > @@ -569,9 +576,52 @@ static void cal_get_hwinfo(struct cal_dev *dev)
> > hwinfo);
> > }
> >
> > +/*
> > + * Errata i913: CSI2 LDO Needs to be disabled when module is powered on
> > + *
> > + * Enabling CSI2 LDO shorts it to core supply. It is crucial the 2 CSI2
> > + * LDOs on the device are disabled if CSI-2 module is powered on
> > + * (0x4845 B304 | 0x4845 B384 [28:27] = 0x1) or in ULPS (0x4845 B304
> > + * | 0x4845 B384 [28:27] = 0x2) mode. Common concerns include: high
> > + * current draw on the module supply in active mode.
> > + *
> > + * Errata does not apply when CSI-2 module is powered off
> > + * (0x4845 B304 | 0x4845 B384 [28:27] = 0x0).
> > + *
> > + * SW Workaround:
> > + * Set the following register bits to disable the LDO,
> > + * which is essentially CSI2 REG10 bit 6:
> > + *
> > + * Core 0: 0x4845 B828 = 0x0000 0040
> > + * Core 1: 0x4845 B928 = 0x0000 0040
> > + */
> > +static void i913_errata(struct cal_dev *dev, unsigned int port)
> > +{
> > + u32 reg10 = reg_read(dev->cc[port], CAL_CSI2_PHY_REG10);
> > +
> > + set_field(®10, CAL_CSI2_PHY_REG0_HSCLOCKCONFIG_DISABLE,
> > + CAL_CSI2_PHY_REG10_I933_LDO_DISABLE_MASK);
> > +
> > + cal_dbg(1, dev, "CSI2_%d_REG10 = 0x%08x\n", port, reg10);
> > + reg_write(dev->cc[port], CAL_CSI2_PHY_REG10, reg10);
> > +}
> > +
> > static inline int cal_runtime_get(struct cal_dev *dev)
>
> I'd drop the 'inline' here. It doesn't seem appropriate anymore since this
> function is now more complex.
Ok I'll fix that
Benoit
>
> Regards,
>
> Hans
>
> > {
> > - return pm_runtime_get_sync(&dev->pdev->dev);
> > + int r;
> > +
> > + r = pm_runtime_get_sync(&dev->pdev->dev);
> > +
> > + if (dev->flags & DRA72_CAL_PRE_ES2_LDO_DISABLE) {
> > + /*
> > + * Apply errata on both port eveytime we (re-)enable
> > + * the clock
> > + */
> > + i913_errata(dev, 0);
> > + i913_errata(dev, 1);
> > + }
> > +
> > + return r;
> > }
> >
> > static inline void cal_runtime_put(struct cal_dev *dev)
> > @@ -2071,6 +2121,10 @@ static const struct of_device_id cal_of_match[] = {
> > .compatible = "ti,dra72-cal",
> > .data = (void *)&dra72x_cal_data,
> > },
> > + {
> > + .compatible = "ti,dra72-pre-es2-cal",
> > + .data = (void *)&dra72x_es1_cal_data,
> > + },
> > {},
> > };
> > MODULE_DEVICE_TABLE(of, cal_of_match);
> > diff --git a/drivers/media/platform/ti-vpe/cal_regs.h b/drivers/media/platform/ti-vpe/cal_regs.h
> > index 68cfc922b422..78d6f015c9ea 100644
> > --- a/drivers/media/platform/ti-vpe/cal_regs.h
> > +++ b/drivers/media/platform/ti-vpe/cal_regs.h
> > @@ -10,6 +10,30 @@
> > #ifndef __TI_CAL_REGS_H
> > #define __TI_CAL_REGS_H
> >
> > +/*
> > + * struct cal_dev.flags possibilities
> > + *
> > + * DRA72_CAL_PRE_ES2_LDO_DISABLE:
> > + * Errata i913: CSI2 LDO Needs to be disabled when module is powered on
> > + *
> > + * Enabling CSI2 LDO shorts it to core supply. It is crucial the 2 CSI2
> > + * LDOs on the device are disabled if CSI-2 module is powered on
> > + * (0x4845 B304 | 0x4845 B384 [28:27] = 0x1) or in ULPS (0x4845 B304
> > + * | 0x4845 B384 [28:27] = 0x2) mode. Common concerns include: high
> > + * current draw on the module supply in active mode.
> > + *
> > + * Errata does not apply when CSI-2 module is powered off
> > + * (0x4845 B304 | 0x4845 B384 [28:27] = 0x0).
> > + *
> > + * SW Workaround:
> > + * Set the following register bits to disable the LDO,
> > + * which is essentially CSI2 REG10 bit 6:
> > + *
> > + * Core 0: 0x4845 B828 = 0x0000 0040
> > + * Core 1: 0x4845 B928 = 0x0000 0040
> > + */
> > +#define DRA72_CAL_PRE_ES2_LDO_DISABLE BIT(0)
> > +
> > #define CAL_NUM_CSI2_PORTS 2
> >
> > /* CAL register offsets */
> > @@ -71,6 +95,7 @@
> > #define CAL_CSI2_PHY_REG0 0x000
> > #define CAL_CSI2_PHY_REG1 0x004
> > #define CAL_CSI2_PHY_REG2 0x008
> > +#define CAL_CSI2_PHY_REG10 0x028
> >
> > /* CAL Control Module Core Camerrx Control register offsets */
> > #define CM_CTRL_CORE_CAMERRX_CONTROL 0x000
> > @@ -458,6 +483,8 @@
> > #define CAL_CSI2_PHY_REG1_CLOCK_MISS_DETECTOR_STATUS_SUCCESS 0
> > #define CAL_CSI2_PHY_REG1_RESET_DONE_STATUS_MASK GENMASK(29, 28)
> >
> > +#define CAL_CSI2_PHY_REG10_I933_LDO_DISABLE_MASK BIT_MASK(6)
> > +
> > #define CAL_CSI2_PHY_REG2_CCP2_SYNC_PATTERN_MASK GENMASK(23, 0)
> > #define CAL_CSI2_PHY_REG2_TRIGGER_CMD_RXTRIGESC3_MASK GENMASK(25, 24)
> > #define CAL_CSI2_PHY_REG2_TRIGGER_CMD_RXTRIGESC2_MASK GENMASK(27, 26)
> >
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 00/19] media: ti-vpe: cal: maintenance
2019-10-21 10:50 ` [Patch 00/19] media: ti-vpe: cal: maintenance Hans Verkuil
@ 2019-10-23 16:05 ` Benoit Parrot
0 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-23 16:05 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Rob Herring, linux-media, devicetree, linux-kernel
Hans Verkuil <hverkuil@xs4all.nl> wrote on Mon [2019-Oct-21 12:50:02 +0200]:
> Hi Benoit,
>
> This series looks good to me. I had just one small comment on patch 7
> and a comment on patch 19.
>
> Just post a v2 for just patch 7/19 and post a patch 20/19 for the requested
> MAINTAINERS change.
>
> Once I have Rob's Acks I can merge this.
Ok, thanks.
I'll send a v2 probably later this week or early next.
Benoit
>
> Regards,
>
> Hans
>
> On 10/18/19 5:34 PM, Benoit Parrot wrote:
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 19/19] dt-bindings: media: cal: convert binding to yaml
2019-10-21 10:49 ` Hans Verkuil
@ 2019-10-23 16:06 ` Benoit Parrot
0 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-23 16:06 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Rob Herring, linux-media, devicetree, linux-kernel
Hans Verkuil <hverkuil@xs4all.nl> wrote on Mon [2019-Oct-21 12:49:00 +0200]:
> On 10/18/19 5:34 PM, Benoit Parrot wrote:
> > Convert ti-cal.txt to ti,cal.yaml.
> >
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
>
> Can you add this bindings file to MAINTAINERS as well in a separate patch?
Yes I can do that, no problem.
Benoit
>
> Thanks!
>
> Hans
>
> > ---
> > .../devicetree/bindings/media/ti,cal.yaml | 186 ++++++++++++++++++
> > .../devicetree/bindings/media/ti-cal.txt | 82 --------
> > 2 files changed, 186 insertions(+), 82 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/media/ti,cal.yaml
> > delete mode 100644 Documentation/devicetree/bindings/media/ti-cal.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/ti,cal.yaml b/Documentation/devicetree/bindings/media/ti,cal.yaml
> > new file mode 100644
> > index 000000000000..c3fbb22b4571
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/ti,cal.yaml
> > @@ -0,0 +1,186 @@
> > +# SPDX-License-Identifier: (GPL-2.0)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/ti,cal.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL) Device Tree Bindings
> > +
> > +maintainers:
> > + - Benoit Parrot <bparrot@ti.com>
> > +
> > +description: |-
> > + The Camera Adaptation Layer (CAL) is a key component for image capture
> > + applications. The capture module provides the system interface and the
> > + processing capability to connect CSI2 image-sensor modules to the
> > + DRA72x device.
> > +
> > + CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> > + should contain a 'port' child node with child 'endpoint' node. Please
> > + refer to the bindings defined in
> > + Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +
> > + compatible should be
> > + "ti,dra72-cal", for DRA72 controllers
> > + "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
> > + "ti,dra76-cal", for DRA76 controllers
> > + "ti,am654-cal", for AM654 controllers
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - ti,dra72-cal
> > + - ti,dra72-pre-es2-cal
> > + - ti,dra76-cal
> > + - ti,am654-cal
> > +
> > + reg:
> > + minItems: 2
> > + items:
> > + - description: The CAL main register region
> > + - description: The RX Core0 (DPHY0) register region
> > + - description: The RX Core1 (DPHY1) register region
> > +
> > + reg-names:
> > + minItems: 2
> > + items:
> > + - const: cal_top
> > + - const: cal_rx_core0
> > + - const: cal_rx_core1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + syscon-camerrx:
> > + maxItems: 1
> > + items:
> > + - description:
> > + phandle to the device control module and offset to the
> > + control_camerarx_core register
> > +
> > + clocks:
> > + maxItems: 1
> > + description: functional clock
> > +
> > + clock-names:
> > + items:
> > + - const: fck
> > +
> > + power-domains:
> > + description:
> > + List of phandle and PM domain specifier as documented in
> > + Documentation/devicetree/bindings/power/power_domain.txt
> > + maxItems: 1
> > +
> > + # See ./video-interfaces.txt for details
> > + ports:
> > + maxItems: 1
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + patternProperties:
> > + '^port@[0-9a-fA-F]+$':
> > + minItems: 1
> > + maxItems: 2
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + reg:
> > + minItems: 1
> > + items:
> > + - description: The port id
> > +
> > + patternProperties:
> > + '^endpoint@[0-9a-fA-F]+$':
> > + minItems: 1
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + clock-lanes:
> > + maxItems: 1
> > +
> > + data-lanes:
> > + minItems: 1
> > + maxItems: 4
> > +
> > + remote-endpoint: true
> > +
> > + required:
> > + - remote-endpoint
> > +
> > + required:
> > + - reg
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - interrupts
> > + - syscon-camerrx
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > + cal: cal@4845b000 {
> > + compatible = "ti,dra72-cal";
> > + reg = <0x4845B000 0x400>,
> > + <0x4845B800 0x40>,
> > + <0x4845B900 0x40>;
> > + reg-names = "cal_top",
> > + "cal_rx_core0",
> > + "cal_rx_core1";
> > + interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > + syscon-camerrx = <&scm_conf 0xE94>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + csi2_0: port@0 {
> > + reg = <0>;
> > + csi2_phy0: endpoint@0 {
> > + remote-endpoint = <&csi2_cam0>;
> > + clock-lanes = <0>;
> > + data-lanes = <1 2>;
> > + };
> > + };
> > + };
> > + };
> > +
> > + i2c5: i2c@4807c000 {
> > + status = "okay";
> > + clock-frequency = <400000>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + ov5640@3c {
> > + compatible = "ovti,ov5640";
> > + reg = <0x3c>;
> > +
> > + clocks = <&clk_ov5640_fixed>;
> > + clock-names = "xclk";
> > +
> > + port {
> > + csi2_cam0: endpoint@0 {
> > + remote-endpoint = <&csi2_phy0>;
> > + clock-lanes = <0>;
> > + data-lanes = <1 2>;
> > + };
> > + };
> > + };
> > + };
> > +
> > +...
> > diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
> > deleted file mode 100644
> > index b7bf8492dc2a..000000000000
> > --- a/Documentation/devicetree/bindings/media/ti-cal.txt
> > +++ /dev/null
> > @@ -1,82 +0,0 @@
> > -Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL)
> > -------------------------------------------------------
> > -
> > -The Camera Adaptation Layer (CAL) is a key component for image capture
> > -applications. The capture module provides the system interface and the
> > -processing capability to connect CSI2 image-sensor modules to the
> > -DRA72x device.
> > -
> > -Required properties:
> > -- compatible:
> > - Should be "ti,dra72-cal", for DRA72 controllers
> > - Should be "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
> > - Should be "ti,dra76-cal", for DRA76 controllers
> > - Should be "ti,am654-cal", for AM654 controllers
> > -- reg: CAL Top level, Receiver Core #0, Receiver Core #1 and Camera RX
> > - control address space
> > -- reg-names: cal_top, cal_rx_core0, cal_rx_core1 and camerrx_control
> > - registers
> > -- interrupts: should contain IRQ line for the CAL;
> > -- syscon-camerrx: phandle to the device control module and offset to the
> > - control_camerarx_core register
> > - This node is meant to replace the "camerrx_control" reg
> > - entry above but "camerrx_control" is still handled
> > - for backward compatibility.
> > -
> > -CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> > -should contain a 'port' child node with child 'endpoint' node. Please
> > -refer to the bindings defined in
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -Example:
> > - cal: cal@4845b000 {
> > - compatible = "ti,dra72-cal";
> > - reg = <0x4845B000 0x400>,
> > - <0x4845B800 0x40>,
> > - <0x4845B900 0x40>;
> > - reg-names = "cal_top",
> > - "cal_rx_core0",
> > - "cal_rx_core1";
> > - interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > - syscon-camerrx = <&scm_conf 0xE94>;
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > -
> > - ports {
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > -
> > - csi2_0: port@0 {
> > - reg = <0>;
> > - csi2_phy0: endpoint@0 {
> > - remote-endpoint = <&csi2_cam0>;
> > - clock-lanes = <0>;
> > - data-lanes = <1 2>;
> > - };
> > - };
> > - csi2_1: port@1 {
> > - reg = <1>;
> > - };
> > - };
> > - };
> > -
> > - i2c5: i2c@4807c000 {
> > - status = "okay";
> > - clock-frequency = <400000>;
> > -
> > - ov5640@3c {
> > - compatible = "ovti,ov5640";
> > - reg = <0x3c>;
> > -
> > - clocks = <&clk_ov5640_fixed>;
> > - clock-names = "xclk";
> > -
> > - port {
> > - csi2_cam0: endpoint {
> > - remote-endpoint = <&csi2_phy0>;
> > - clock-lanes = <0>;
> > - data-lanes = <1 2>;
> > - };
> > - };
> > - };
> > - };
> >
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 19/19] dt-bindings: media: cal: convert binding to yaml
2019-10-22 7:46 ` Sakari Ailus
@ 2019-10-23 16:18 ` Benoit Parrot
2019-10-24 6:13 ` Sakari Ailus
0 siblings, 1 reply; 40+ messages in thread
From: Benoit Parrot @ 2019-10-23 16:18 UTC (permalink / raw)
To: Sakari Ailus
Cc: Hans Verkuil, Rob Herring, linux-media, devicetree, linux-kernel
Sakari Ailus <sakari.ailus@iki.fi> wrote on Tue [2019-Oct-22 10:46:23 +0300]:
> Hi Benoit,
>
> Thanks for the patch.
>
> On Fri, Oct 18, 2019 at 10:34:37AM -0500, Benoit Parrot wrote:
> > Convert ti-cal.txt to ti,cal.yaml.
> >
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> > .../devicetree/bindings/media/ti,cal.yaml | 186 ++++++++++++++++++
> > .../devicetree/bindings/media/ti-cal.txt | 82 --------
> > 2 files changed, 186 insertions(+), 82 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/media/ti,cal.yaml
> > delete mode 100644 Documentation/devicetree/bindings/media/ti-cal.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/ti,cal.yaml b/Documentation/devicetree/bindings/media/ti,cal.yaml
> > new file mode 100644
> > index 000000000000..c3fbb22b4571
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/ti,cal.yaml
> > @@ -0,0 +1,186 @@
> > +# SPDX-License-Identifier: (GPL-2.0)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/ti,cal.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL) Device Tree Bindings
> > +
> > +maintainers:
> > + - Benoit Parrot <bparrot@ti.com>
> > +
> > +description: |-
> > + The Camera Adaptation Layer (CAL) is a key component for image capture
> > + applications. The capture module provides the system interface and the
> > + processing capability to connect CSI2 image-sensor modules to the
> > + DRA72x device.
> > +
> > + CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> > + should contain a 'port' child node with child 'endpoint' node. Please
> > + refer to the bindings defined in
> > + Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +
> > + compatible should be
> > + "ti,dra72-cal", for DRA72 controllers
> > + "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
> > + "ti,dra76-cal", for DRA76 controllers
> > + "ti,am654-cal", for AM654 controllers
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - ti,dra72-cal
> > + - ti,dra72-pre-es2-cal
> > + - ti,dra76-cal
> > + - ti,am654-cal
> > +
> > + reg:
> > + minItems: 2
> > + items:
> > + - description: The CAL main register region
> > + - description: The RX Core0 (DPHY0) register region
> > + - description: The RX Core1 (DPHY1) register region
> > +
> > + reg-names:
> > + minItems: 2
> > + items:
> > + - const: cal_top
> > + - const: cal_rx_core0
> > + - const: cal_rx_core1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + syscon-camerrx:
> > + maxItems: 1
> > + items:
> > + - description:
> > + phandle to the device control module and offset to the
> > + control_camerarx_core register
> > +
> > + clocks:
> > + maxItems: 1
> > + description: functional clock
> > +
> > + clock-names:
> > + items:
> > + - const: fck
> > +
> > + power-domains:
> > + description:
> > + List of phandle and PM domain specifier as documented in
> > + Documentation/devicetree/bindings/power/power_domain.txt
> > + maxItems: 1
> > +
> > + # See ./video-interfaces.txt for details
> > + ports:
> > + maxItems: 1
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + patternProperties:
> > + '^port@[0-9a-fA-F]+$':
> > + minItems: 1
> > + maxItems: 2
>
> Obviously you need a port node to connect a sensor. But can the device do
> something useful without one? I guess it may be a matter of taste whether
> you require one.
In an ideal world all of these would be covered by a video-interfaces.yaml
file I would just need to include... :)
But I'll try and add some more "required" and see how much trouble it gets
me.
>
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + reg:
> > + minItems: 1
> > + items:
> > + - description: The port id
> > +
> > + patternProperties:
> > + '^endpoint@[0-9a-fA-F]+$':
> > + minItems: 1
>
> The graph bindings allows for no endpoints.
Maybe but then it is not much use.
>
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + clock-lanes:
> > + maxItems: 1
>
> Does the device support lane reordering? If not, you could omit the
> clock-lanes property. It wasn't documented earlier either albeit the
> example had it for some reason.
Not sure what you mean by lane re-ordering here, but this IP needs to know
which lanes are used for data and which lane is the clock lane.
I cannot just assume that clock lane is always lane 0, I have a sensor
where the clock lane is #2 for instance and the data lanes are 0, 1, 3, 4.
But at any rate before it was not specifically documented because I thought
it was covered by the "see
Documentation/devicetree/bindings/media/video-interfaces.txt" or de we need
to duplicate that in every binding?
Benoit
>
> > +
> > + data-lanes:
> > + minItems: 1
> > + maxItems: 4
> > +
> > + remote-endpoint: true
> > +
> > + required:
> > + - remote-endpoint
> > +
> > + required:
> > + - reg
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - interrupts
> > + - syscon-camerrx
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > + cal: cal@4845b000 {
> > + compatible = "ti,dra72-cal";
> > + reg = <0x4845B000 0x400>,
> > + <0x4845B800 0x40>,
> > + <0x4845B900 0x40>;
> > + reg-names = "cal_top",
> > + "cal_rx_core0",
> > + "cal_rx_core1";
> > + interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > + syscon-camerrx = <&scm_conf 0xE94>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + csi2_0: port@0 {
> > + reg = <0>;
> > + csi2_phy0: endpoint@0 {
> > + remote-endpoint = <&csi2_cam0>;
> > + clock-lanes = <0>;
> > + data-lanes = <1 2>;
> > + };
> > + };
> > + };
> > + };
> > +
> > + i2c5: i2c@4807c000 {
> > + status = "okay";
> > + clock-frequency = <400000>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + ov5640@3c {
> > + compatible = "ovti,ov5640";
> > + reg = <0x3c>;
> > +
> > + clocks = <&clk_ov5640_fixed>;
> > + clock-names = "xclk";
> > +
> > + port {
> > + csi2_cam0: endpoint@0 {
> > + remote-endpoint = <&csi2_phy0>;
> > + clock-lanes = <0>;
> > + data-lanes = <1 2>;
> > + };
> > + };
> > + };
> > + };
> > +
> > +...
> > diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
> > deleted file mode 100644
> > index b7bf8492dc2a..000000000000
> > --- a/Documentation/devicetree/bindings/media/ti-cal.txt
> > +++ /dev/null
> > @@ -1,82 +0,0 @@
> > -Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL)
> > -------------------------------------------------------
> > -
> > -The Camera Adaptation Layer (CAL) is a key component for image capture
> > -applications. The capture module provides the system interface and the
> > -processing capability to connect CSI2 image-sensor modules to the
> > -DRA72x device.
> > -
> > -Required properties:
> > -- compatible:
> > - Should be "ti,dra72-cal", for DRA72 controllers
> > - Should be "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
> > - Should be "ti,dra76-cal", for DRA76 controllers
> > - Should be "ti,am654-cal", for AM654 controllers
> > -- reg: CAL Top level, Receiver Core #0, Receiver Core #1 and Camera RX
> > - control address space
> > -- reg-names: cal_top, cal_rx_core0, cal_rx_core1 and camerrx_control
> > - registers
> > -- interrupts: should contain IRQ line for the CAL;
> > -- syscon-camerrx: phandle to the device control module and offset to the
> > - control_camerarx_core register
> > - This node is meant to replace the "camerrx_control" reg
> > - entry above but "camerrx_control" is still handled
> > - for backward compatibility.
> > -
> > -CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> > -should contain a 'port' child node with child 'endpoint' node. Please
> > -refer to the bindings defined in
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -Example:
> > - cal: cal@4845b000 {
> > - compatible = "ti,dra72-cal";
> > - reg = <0x4845B000 0x400>,
> > - <0x4845B800 0x40>,
> > - <0x4845B900 0x40>;
> > - reg-names = "cal_top",
> > - "cal_rx_core0",
> > - "cal_rx_core1";
> > - interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > - syscon-camerrx = <&scm_conf 0xE94>;
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > -
> > - ports {
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > -
> > - csi2_0: port@0 {
> > - reg = <0>;
> > - csi2_phy0: endpoint@0 {
> > - remote-endpoint = <&csi2_cam0>;
> > - clock-lanes = <0>;
> > - data-lanes = <1 2>;
> > - };
> > - };
> > - csi2_1: port@1 {
> > - reg = <1>;
> > - };
> > - };
> > - };
> > -
> > - i2c5: i2c@4807c000 {
> > - status = "okay";
> > - clock-frequency = <400000>;
> > -
> > - ov5640@3c {
> > - compatible = "ovti,ov5640";
> > - reg = <0x3c>;
> > -
> > - clocks = <&clk_ov5640_fixed>;
> > - clock-names = "xclk";
> > -
> > - port {
> > - csi2_cam0: endpoint {
> > - remote-endpoint = <&csi2_phy0>;
> > - clock-lanes = <0>;
> > - data-lanes = <1 2>;
> > - };
> > - };
> > - };
> > - };
>
> --
> Kind regards,
>
> Sakari Ailus
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 19/19] dt-bindings: media: cal: convert binding to yaml
2019-10-23 16:18 ` Benoit Parrot
@ 2019-10-24 6:13 ` Sakari Ailus
0 siblings, 0 replies; 40+ messages in thread
From: Sakari Ailus @ 2019-10-24 6:13 UTC (permalink / raw)
To: Benoit Parrot
Cc: Hans Verkuil, Rob Herring, linux-media, devicetree, linux-kernel
Hi Benoit,
On Wed, Oct 23, 2019 at 11:18:45AM -0500, Benoit Parrot wrote:
> Sakari Ailus <sakari.ailus@iki.fi> wrote on Tue [2019-Oct-22 10:46:23 +0300]:
> > Hi Benoit,
> >
> > Thanks for the patch.
> >
> > On Fri, Oct 18, 2019 at 10:34:37AM -0500, Benoit Parrot wrote:
> > > Convert ti-cal.txt to ti,cal.yaml.
> > >
> > > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > > ---
> > > .../devicetree/bindings/media/ti,cal.yaml | 186 ++++++++++++++++++
> > > .../devicetree/bindings/media/ti-cal.txt | 82 --------
> > > 2 files changed, 186 insertions(+), 82 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/media/ti,cal.yaml
> > > delete mode 100644 Documentation/devicetree/bindings/media/ti-cal.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/ti,cal.yaml b/Documentation/devicetree/bindings/media/ti,cal.yaml
> > > new file mode 100644
> > > index 000000000000..c3fbb22b4571
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/ti,cal.yaml
> > > @@ -0,0 +1,186 @@
> > > +# SPDX-License-Identifier: (GPL-2.0)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/ti,cal.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL) Device Tree Bindings
> > > +
> > > +maintainers:
> > > + - Benoit Parrot <bparrot@ti.com>
> > > +
> > > +description: |-
> > > + The Camera Adaptation Layer (CAL) is a key component for image capture
> > > + applications. The capture module provides the system interface and the
> > > + processing capability to connect CSI2 image-sensor modules to the
> > > + DRA72x device.
> > > +
> > > + CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> > > + should contain a 'port' child node with child 'endpoint' node. Please
> > > + refer to the bindings defined in
> > > + Documentation/devicetree/bindings/media/video-interfaces.txt.
> > > +
> > > + compatible should be
> > > + "ti,dra72-cal", for DRA72 controllers
> > > + "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
> > > + "ti,dra76-cal", for DRA76 controllers
> > > + "ti,am654-cal", for AM654 controllers
> > > +
> > > +properties:
> > > + compatible:
> > > + items:
> > > + - enum:
> > > + - ti,dra72-cal
> > > + - ti,dra72-pre-es2-cal
> > > + - ti,dra76-cal
> > > + - ti,am654-cal
> > > +
> > > + reg:
> > > + minItems: 2
> > > + items:
> > > + - description: The CAL main register region
> > > + - description: The RX Core0 (DPHY0) register region
> > > + - description: The RX Core1 (DPHY1) register region
> > > +
> > > + reg-names:
> > > + minItems: 2
> > > + items:
> > > + - const: cal_top
> > > + - const: cal_rx_core0
> > > + - const: cal_rx_core1
> > > +
> > > + interrupts:
> > > + maxItems: 1
> > > +
> > > + syscon-camerrx:
> > > + maxItems: 1
> > > + items:
> > > + - description:
> > > + phandle to the device control module and offset to the
> > > + control_camerarx_core register
> > > +
> > > + clocks:
> > > + maxItems: 1
> > > + description: functional clock
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: fck
> > > +
> > > + power-domains:
> > > + description:
> > > + List of phandle and PM domain specifier as documented in
> > > + Documentation/devicetree/bindings/power/power_domain.txt
> > > + maxItems: 1
> > > +
> > > + # See ./video-interfaces.txt for details
> > > + ports:
> > > + maxItems: 1
> > > + type: object
> > > + additionalProperties: false
> > > +
> > > + properties:
> > > + "#address-cells":
> > > + const: 1
> > > +
> > > + "#size-cells":
> > > + const: 0
> > > +
> > > + patternProperties:
> > > + '^port@[0-9a-fA-F]+$':
> > > + minItems: 1
> > > + maxItems: 2
> >
> > Obviously you need a port node to connect a sensor. But can the device do
> > something useful without one? I guess it may be a matter of taste whether
> > you require one.
>
> In an ideal world all of these would be covered by a video-interfaces.yaml
> file I would just need to include... :)
>
> But I'll try and add some more "required" and see how much trouble it gets
> me.
> >
> > > + type: object
> > > + additionalProperties: false
> > > +
> > > + properties:
> > > + reg:
> > > + minItems: 1
> > > + items:
> > > + - description: The port id
> > > +
> > > + patternProperties:
> > > + '^endpoint@[0-9a-fA-F]+$':
> > > + minItems: 1
> >
> > The graph bindings allows for no endpoints.
>
> Maybe but then it is not much use.
I guess you can require the endpoint in device bindings still. But the
question is: do you need to?
> >
> > > + type: object
> > > + additionalProperties: false
> > > +
> > > + properties:
> > > + clock-lanes:
> > > + maxItems: 1
> >
> > Does the device support lane reordering? If not, you could omit the
> > clock-lanes property. It wasn't documented earlier either albeit the
> > example had it for some reason.
>
> Not sure what you mean by lane re-ordering here, but this IP needs to know
> which lanes are used for data and which lane is the clock lane.
>
> I cannot just assume that clock lane is always lane 0, I have a sensor
> where the clock lane is #2 for instance and the data lanes are 0, 1, 3, 4.
>
> But at any rate before it was not specifically documented because I thought
> it was covered by the "see
> Documentation/devicetree/bindings/media/video-interfaces.txt" or de we need
> to duplicate that in every binding?
Then you should indeed have clock-lanes here as you need to know the
position of the clock lane. Most devices either just can make use of the
number of lanes or the order of the lanes is freely configurable (e.g. OMAP
3 ISP). The former group just needs data-lanes.
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 03/19] media: ti-vpe: cal: Add per platform data support
2019-10-18 15:34 ` [Patch 03/19] media: ti-vpe: cal: Add per platform data support Benoit Parrot
@ 2019-10-29 13:18 ` Rob Herring
2019-10-30 13:34 ` Benoit Parrot
0 siblings, 1 reply; 40+ messages in thread
From: Rob Herring @ 2019-10-29 13:18 UTC (permalink / raw)
To: Benoit Parrot; +Cc: Hans Verkuil, linux-media, devicetree, linux-kernel
On Fri, Oct 18, 2019 at 10:34:21AM -0500, Benoit Parrot wrote:
> First this patch adds a method to access the CTRL_CORE_CAMERRX_CONTROL
> register to use the syscon mechanism. For backward compatibility we also
> handle using the existing camerrx_control "reg" entry if a syscon node
> is not found.
>
> In addition the register bit layout for the CTRL_CORE_CAMERRX_CONTROL
> changes depending on the device. In order to support this we need to use
> a register access scheme based on data configuration instead of using
> static macro.
>
> In this case we make use of the regmap facility and create data set
> based on the various device and phy available.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
> drivers/media/platform/ti-vpe/cal.c | 281 +++++++++++++++++++++-------
> 1 file changed, 212 insertions(+), 69 deletions(-)
> @@ -1816,6 +1911,18 @@ static int cal_probe(struct platform_device *pdev)
> if (!dev)
> return -ENOMEM;
>
> + match = of_match_device(of_match_ptr(cal_of_match), &pdev->dev);
Use of_device_get_match_data() instead.
> + if (!match)
> + return -ENODEV;
> +
> + if (match->data) {
> + dev->data = (struct cal_data *)match->data;
> + dev->flags = dev->data->flags;
> + } else {
> + dev_err(&pdev->dev, "Could not get feature data based on compatible version\n");
> + return -ENODEV;
> + }
> +
> /* set pseudo v4l2 device name so we can use v4l2_printk */
> strscpy(dev->v4l2_dev.name, CAL_MODULE_NAME,
> sizeof(dev->v4l2_dev.name));
> @@ -1823,6 +1930,43 @@ static int cal_probe(struct platform_device *pdev)
> /* save pdev pointer */
> dev->pdev = pdev;
>
> + if (parent && of_property_read_bool(parent, "syscon-camerrx")) {
> + syscon_camerrx =
> + syscon_regmap_lookup_by_phandle(parent,
> + "syscon-camerrx");
> + if (IS_ERR(syscon_camerrx)) {
> + dev_err(&pdev->dev, "failed to get syscon-camerrx regmap\n");
> + return PTR_ERR(syscon_camerrx);
> + }
> +
> + if (of_property_read_u32_index(parent, "syscon-camerrx", 1,
> + &syscon_camerrx_offset)) {
Kind of odd to read the property twice and using functions that don't
match the type. We have functions to retrieve phandle and args.
> + dev_err(&pdev->dev, "failed to get syscon-camerrx offset\n");
> + return -EINVAL;
> + }
> + } else {
> + /*
> + * Backward DTS compatibility.
> + * If syscon entry is not present then check if the
> + * camerrx_control resource is present.
> + */
> + syscon_camerrx = cal_get_camerarx_regmap(dev);
> + if (IS_ERR(syscon_camerrx)) {
> + dev_err(&pdev->dev, "failed to get camerrx_control regmap\n");
> + return PTR_ERR(syscon_camerrx);
> + }
> + /* In this case the base already point to the direct
> + * CM register so no need for an offset
> + */
> + syscon_camerrx_offset = 0;
> + }
> +
> + dev->syscon_camerrx = syscon_camerrx;
> + dev->syscon_camerrx_offset = syscon_camerrx_offset;
> + ret = cal_camerarx_regmap_init(dev);
> + if (ret)
> + return ret;
> +
> dev->res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "cal_top");
> dev->base = devm_ioremap_resource(&pdev->dev, dev->res);
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 01/19] dt-bindings: media: cal: update binding to use syscon
2019-10-18 15:34 ` [Patch 01/19] dt-bindings: media: cal: update binding to use syscon Benoit Parrot
@ 2019-10-29 13:20 ` Rob Herring
2019-10-30 13:39 ` Benoit Parrot
0 siblings, 1 reply; 40+ messages in thread
From: Rob Herring @ 2019-10-29 13:20 UTC (permalink / raw)
To: Benoit Parrot; +Cc: Hans Verkuil, linux-media, devicetree, linux-kernel
On Fri, Oct 18, 2019 at 10:34:19AM -0500, Benoit Parrot wrote:
> Update Device Tree bindings for the CAL driver to use syscon to access
> the phy config register instead of trying to map it directly.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
> Documentation/devicetree/bindings/media/ti-cal.txt | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
> index ae9b52f37576..782f801b12a9 100644
> --- a/Documentation/devicetree/bindings/media/ti-cal.txt
> +++ b/Documentation/devicetree/bindings/media/ti-cal.txt
> @@ -10,9 +10,14 @@ Required properties:
> - compatible: must be "ti,dra72-cal"
> - reg: CAL Top level, Receiver Core #0, Receiver Core #1 and Camera RX
> control address space
> -- reg-names: cal_top, cal_rx_core0, cal_rx_core1, and camerrx_control
> +- reg-names: cal_top, cal_rx_core0, cal_rx_core1 and camerrx_control
> registers
> - interrupts: should contain IRQ line for the CAL;
> +- syscon-camerrx: phandle to the device control module and offset to the
> + control_camerarx_core register
> + This node is meant to replace the "camerrx_control" reg
> + entry above but "camerrx_control" is still handled
> + for backward compatibility.
Needs a vendor prefix and drop 'syscon'.
>
> CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> should contain a 'port' child node with child 'endpoint' node. Please
> @@ -25,13 +30,12 @@ Example:
> ti,hwmods = "cal";
> reg = <0x4845B000 0x400>,
> <0x4845B800 0x40>,
> - <0x4845B900 0x40>,
> - <0x4A002e94 0x4>;
> + <0x4845B900 0x40>;
> reg-names = "cal_top",
> "cal_rx_core0",
> - "cal_rx_core1",
> - "camerrx_control";
> + "cal_rx_core1";
> interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> + syscon-camerrx = <&scm_conf 0xE94>;
> #address-cells = <1>;
> #size-cells = <0>;
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 02/19] dt-bindings: media: cal: update binding example
2019-10-18 15:34 ` [Patch 02/19] dt-bindings: media: cal: update binding example Benoit Parrot
@ 2019-10-29 13:21 ` Rob Herring
2019-10-30 13:41 ` Benoit Parrot
0 siblings, 1 reply; 40+ messages in thread
From: Rob Herring @ 2019-10-29 13:21 UTC (permalink / raw)
To: Benoit Parrot; +Cc: Hans Verkuil, linux-media, devicetree, linux-kernel
On Fri, Oct 18, 2019 at 10:34:20AM -0500, Benoit Parrot wrote:
> Update binding example to show proper endpoint properties and linkage.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
> .../devicetree/bindings/media/ti-cal.txt | 32 ++++++++++---------
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
> index 782f801b12a9..6b56fddcfc4a 100644
> --- a/Documentation/devicetree/bindings/media/ti-cal.txt
> +++ b/Documentation/devicetree/bindings/media/ti-cal.txt
> @@ -27,7 +27,6 @@ Documentation/devicetree/bindings/media/video-interfaces.txt.
> Example:
> cal: cal@4845b000 {
> compatible = "ti,dra72-cal";
> - ti,hwmods = "cal";
> reg = <0x4845B000 0x400>,
> <0x4845B800 0x40>,
> <0x4845B900 0x40>;
> @@ -45,9 +44,10 @@ Example:
>
> csi2_0: port@0 {
> reg = <0>;
> - endpoint {
> - slave-mode;
> - remote-endpoint = <&ar0330_1>;
> + csi2_phy0: endpoint@0 {
Unit address without 'reg' is not correct.
> + remote-endpoint = <&csi2_cam0>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> };
> };
> csi2_1: port@1 {
> @@ -57,19 +57,21 @@ Example:
> };
>
> i2c5: i2c@4807c000 {
> - ar0330@10 {
> - compatible = "ti,ar0330";
> - reg = <0x10>;
> + status = "okay";
Don't show status in examples.
> + clock-frequency = <400000>;
>
> - port {
> - #address-cells = <1>;
> - #size-cells = <0>;
> + ov5640@3c {
camera-sensor@3c
> + compatible = "ovti,ov5640";
> + reg = <0x3c>;
> +
> + clocks = <&clk_ov5640_fixed>;
> + clock-names = "xclk";
>
> - ar0330_1: endpoint {
> - reg = <0>;
> - clock-lanes = <1>;
> - data-lanes = <0 2 3 4>;
> - remote-endpoint = <&csi2_0>;
> + port {
> + csi2_cam0: endpoint {
> + remote-endpoint = <&csi2_phy0>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> };
> };
> };
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 06/19] dt-bindings: media: cal: update binding to add PHY LDO errata support
2019-10-18 15:34 ` [Patch 06/19] dt-bindings: media: cal: update binding to add PHY LDO errata support Benoit Parrot
@ 2019-10-29 13:23 ` Rob Herring
0 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2019-10-29 13:23 UTC (permalink / raw)
To: Benoit Parrot
Cc: Hans Verkuil, linux-media, devicetree, linux-kernel,
Benoit Parrot, Jyri Sarha
On Fri, 18 Oct 2019 10:34:24 -0500, Benoit Parrot wrote:
> Update Device Tree bindings for the CAL driver to add support for
> the CSI2 PHY LDO errata workaround for pre-es2 devices.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
> Documentation/devicetree/bindings/media/ti-cal.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 11/19] dt-bindings: media: cal: update binding to add DRA76x support
2019-10-18 15:34 ` [Patch 11/19] dt-bindings: media: cal: update binding to add DRA76x support Benoit Parrot
@ 2019-10-29 13:24 ` Rob Herring
0 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2019-10-29 13:24 UTC (permalink / raw)
To: Benoit Parrot
Cc: Hans Verkuil, linux-media, devicetree, linux-kernel, Benoit Parrot
On Fri, 18 Oct 2019 10:34:29 -0500, Benoit Parrot wrote:
> Update Device Tree bindings for the CAL driver to add DRA76x support.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
> Documentation/devicetree/bindings/media/ti-cal.txt | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 13/19] dt-bindings: media: cal: update binding to add AM654 support
2019-10-18 15:34 ` [Patch 13/19] dt-bindings: media: cal: update binding to add AM654 support Benoit Parrot
@ 2019-10-29 13:24 ` Rob Herring
0 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2019-10-29 13:24 UTC (permalink / raw)
To: Benoit Parrot
Cc: Hans Verkuil, linux-media, devicetree, linux-kernel, Benoit Parrot
On Fri, 18 Oct 2019 10:34:31 -0500, Benoit Parrot wrote:
> Update Device Tree bindings for the CAL driver to add AM654 support.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
> Documentation/devicetree/bindings/media/ti-cal.txt | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 19/19] dt-bindings: media: cal: convert binding to yaml
2019-10-18 15:34 ` [Patch 19/19] dt-bindings: media: cal: convert binding to yaml Benoit Parrot
2019-10-21 10:49 ` Hans Verkuil
2019-10-22 7:46 ` Sakari Ailus
@ 2019-10-29 14:22 ` Rob Herring
2019-10-30 13:54 ` Benoit Parrot
2 siblings, 1 reply; 40+ messages in thread
From: Rob Herring @ 2019-10-29 14:22 UTC (permalink / raw)
To: Benoit Parrot; +Cc: Hans Verkuil, linux-media, devicetree, linux-kernel
On Fri, Oct 18, 2019 at 10:34:37AM -0500, Benoit Parrot wrote:
> Convert ti-cal.txt to ti,cal.yaml.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
> .../devicetree/bindings/media/ti,cal.yaml | 186 ++++++++++++++++++
> .../devicetree/bindings/media/ti-cal.txt | 82 --------
> 2 files changed, 186 insertions(+), 82 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/ti,cal.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/ti-cal.txt
>
> diff --git a/Documentation/devicetree/bindings/media/ti,cal.yaml b/Documentation/devicetree/bindings/media/ti,cal.yaml
> new file mode 100644
> index 000000000000..c3fbb22b4571
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/ti,cal.yaml
> @@ -0,0 +1,186 @@
> +# SPDX-License-Identifier: (GPL-2.0)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/ti,cal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL) Device Tree Bindings
> +
> +maintainers:
> + - Benoit Parrot <bparrot@ti.com>
> +
> +description: |-
> + The Camera Adaptation Layer (CAL) is a key component for image capture
> + applications. The capture module provides the system interface and the
> + processing capability to connect CSI2 image-sensor modules to the
> + DRA72x device.
> +
> + CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> + should contain a 'port' child node with child 'endpoint' node. Please
> + refer to the bindings defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> + compatible should be
> + "ti,dra72-cal", for DRA72 controllers
> + "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
> + "ti,dra76-cal", for DRA76 controllers
> + "ti,am654-cal", for AM654 controllers
Drop these or add as comments to the schema below.
> +
> +properties:
> + compatible:
> + items:
You can drop 'items' here since there is only 1.
> + - enum:
> + - ti,dra72-cal
> + - ti,dra72-pre-es2-cal
> + - ti,dra76-cal
> + - ti,am654-cal
> +
> + reg:
> + minItems: 2
> + items:
> + - description: The CAL main register region
> + - description: The RX Core0 (DPHY0) register region
> + - description: The RX Core1 (DPHY1) register region
> +
> + reg-names:
> + minItems: 2
> + items:
> + - const: cal_top
> + - const: cal_rx_core0
> + - const: cal_rx_core1
> +
> + interrupts:
> + maxItems: 1
> +
> + syscon-camerrx:
Needs a type definition.
> + maxItems: 1
> + items:
> + - description:
> + phandle to the device control module and offset to the
> + control_camerarx_core register
> +
> + clocks:
> + maxItems: 1
> + description: functional clock
You can drop the description.
> +
> + clock-names:
> + items:
Drop items.
> + - const: fck
> +
> + power-domains:
> + description:
> + List of phandle and PM domain specifier as documented in
> + Documentation/devicetree/bindings/power/power_domain.txt
> + maxItems: 1
> +
> + # See ./video-interfaces.txt for details
> + ports:
> + maxItems: 1
But ports is not an array...
> + type: object
> + additionalProperties: false
> +
> + properties:
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + patternProperties:
> + '^port@[0-9a-fA-F]+$':
In a device binding, you need to specify 'port@0' and 'port@1' (assuming
0 and 1) and say what they are.
> + minItems: 1
> + maxItems: 2
Not valid for an object (aka node).
> + type: object
> + additionalProperties: false
> +
> + properties:
> + reg:
> + minItems: 1
> + items:
> + - description: The port id
> +
> + patternProperties:
> + '^endpoint@[0-9a-fA-F]+$':
Just 'endpoint' is valid too.
> + minItems: 1
> + type: object
> + additionalProperties: false
> +
> + properties:
> + clock-lanes:
> + maxItems: 1
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> +
> + remote-endpoint: true
> +
> + required:
> + - remote-endpoint
> +
> + required:
> + - reg
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> + - syscon-camerrx
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + cal: cal@4845b000 {
> + compatible = "ti,dra72-cal";
> + reg = <0x4845B000 0x400>,
> + <0x4845B800 0x40>,
> + <0x4845B900 0x40>;
> + reg-names = "cal_top",
> + "cal_rx_core0",
> + "cal_rx_core1";
> + interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> + syscon-camerrx = <&scm_conf 0xE94>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + csi2_0: port@0 {
> + reg = <0>;
> + csi2_phy0: endpoint@0 {
> + remote-endpoint = <&csi2_cam0>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
> +
> + i2c5: i2c@4807c000 {
> + status = "okay";
> + clock-frequency = <400000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ov5640@3c {
> + compatible = "ovti,ov5640";
> + reg = <0x3c>;
> +
> + clocks = <&clk_ov5640_fixed>;
> + clock-names = "xclk";
> +
> + port {
> + csi2_cam0: endpoint@0 {
> + remote-endpoint = <&csi2_phy0>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
> +
> +...
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 03/19] media: ti-vpe: cal: Add per platform data support
2019-10-29 13:18 ` Rob Herring
@ 2019-10-30 13:34 ` Benoit Parrot
0 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-30 13:34 UTC (permalink / raw)
To: Rob Herring; +Cc: Hans Verkuil, linux-media, devicetree, linux-kernel
Rob Herring <robh@kernel.org> wrote on Tue [2019-Oct-29 08:18:55 -0500]:
> On Fri, Oct 18, 2019 at 10:34:21AM -0500, Benoit Parrot wrote:
> > First this patch adds a method to access the CTRL_CORE_CAMERRX_CONTROL
> > register to use the syscon mechanism. For backward compatibility we also
> > handle using the existing camerrx_control "reg" entry if a syscon node
> > is not found.
> >
> > In addition the register bit layout for the CTRL_CORE_CAMERRX_CONTROL
> > changes depending on the device. In order to support this we need to use
> > a register access scheme based on data configuration instead of using
> > static macro.
> >
> > In this case we make use of the regmap facility and create data set
> > based on the various device and phy available.
> >
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> > drivers/media/platform/ti-vpe/cal.c | 281 +++++++++++++++++++++-------
> > 1 file changed, 212 insertions(+), 69 deletions(-)
>
>
> > @@ -1816,6 +1911,18 @@ static int cal_probe(struct platform_device *pdev)
> > if (!dev)
> > return -ENOMEM;
> >
> > + match = of_match_device(of_match_ptr(cal_of_match), &pdev->dev);
>
> Use of_device_get_match_data() instead.
Ok I'll change that.
>
> > + if (!match)
> > + return -ENODEV;
> > +
> > + if (match->data) {
> > + dev->data = (struct cal_data *)match->data;
> > + dev->flags = dev->data->flags;
> > + } else {
> > + dev_err(&pdev->dev, "Could not get feature data based on compatible version\n");
> > + return -ENODEV;
> > + }
> > +
> > /* set pseudo v4l2 device name so we can use v4l2_printk */
> > strscpy(dev->v4l2_dev.name, CAL_MODULE_NAME,
> > sizeof(dev->v4l2_dev.name));
> > @@ -1823,6 +1930,43 @@ static int cal_probe(struct platform_device *pdev)
> > /* save pdev pointer */
> > dev->pdev = pdev;
> >
> > + if (parent && of_property_read_bool(parent, "syscon-camerrx")) {
> > + syscon_camerrx =
> > + syscon_regmap_lookup_by_phandle(parent,
> > + "syscon-camerrx");
> > + if (IS_ERR(syscon_camerrx)) {
> > + dev_err(&pdev->dev, "failed to get syscon-camerrx regmap\n");
> > + return PTR_ERR(syscon_camerrx);
> > + }
> > +
> > + if (of_property_read_u32_index(parent, "syscon-camerrx", 1,
> > + &syscon_camerrx_offset)) {
>
> Kind of odd to read the property twice and using functions that don't
> match the type. We have functions to retrieve phandle and args.
Yeah, I wanted to make a distinction between the node being present and
any other kind of errors, so we can have a little more precise error
message.
>
> > + dev_err(&pdev->dev, "failed to get syscon-camerrx offset\n");
> > + return -EINVAL;
> > + }
> > + } else {
> > + /*
> > + * Backward DTS compatibility.
> > + * If syscon entry is not present then check if the
> > + * camerrx_control resource is present.
> > + */
> > + syscon_camerrx = cal_get_camerarx_regmap(dev);
> > + if (IS_ERR(syscon_camerrx)) {
> > + dev_err(&pdev->dev, "failed to get camerrx_control regmap\n");
> > + return PTR_ERR(syscon_camerrx);
> > + }
> > + /* In this case the base already point to the direct
> > + * CM register so no need for an offset
> > + */
> > + syscon_camerrx_offset = 0;
> > + }
> > +
> > + dev->syscon_camerrx = syscon_camerrx;
> > + dev->syscon_camerrx_offset = syscon_camerrx_offset;
> > + ret = cal_camerarx_regmap_init(dev);
> > + if (ret)
> > + return ret;
> > +
> > dev->res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > "cal_top");
> > dev->base = devm_ioremap_resource(&pdev->dev, dev->res);
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 01/19] dt-bindings: media: cal: update binding to use syscon
2019-10-29 13:20 ` Rob Herring
@ 2019-10-30 13:39 ` Benoit Parrot
0 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-30 13:39 UTC (permalink / raw)
To: Rob Herring; +Cc: Hans Verkuil, linux-media, devicetree, linux-kernel
Rob Herring <robh@kernel.org> wrote on Tue [2019-Oct-29 08:20:09 -0500]:
> On Fri, Oct 18, 2019 at 10:34:19AM -0500, Benoit Parrot wrote:
> > Update Device Tree bindings for the CAL driver to use syscon to access
> > the phy config register instead of trying to map it directly.
> >
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> > Documentation/devicetree/bindings/media/ti-cal.txt | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
> > index ae9b52f37576..782f801b12a9 100644
> > --- a/Documentation/devicetree/bindings/media/ti-cal.txt
> > +++ b/Documentation/devicetree/bindings/media/ti-cal.txt
> > @@ -10,9 +10,14 @@ Required properties:
> > - compatible: must be "ti,dra72-cal"
> > - reg: CAL Top level, Receiver Core #0, Receiver Core #1 and Camera RX
> > control address space
> > -- reg-names: cal_top, cal_rx_core0, cal_rx_core1, and camerrx_control
> > +- reg-names: cal_top, cal_rx_core0, cal_rx_core1 and camerrx_control
> > registers
> > - interrupts: should contain IRQ line for the CAL;
> > +- syscon-camerrx: phandle to the device control module and offset to the
> > + control_camerarx_core register
> > + This node is meant to replace the "camerrx_control" reg
> > + entry above but "camerrx_control" is still handled
> > + for backward compatibility.
>
> Needs a vendor prefix and drop 'syscon'.
Hmm, I had chosen this syntax based on previous examples.
But I can change it. How about "ti,camerrx_control"?
>
> >
> > CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> > should contain a 'port' child node with child 'endpoint' node. Please
> > @@ -25,13 +30,12 @@ Example:
> > ti,hwmods = "cal";
> > reg = <0x4845B000 0x400>,
> > <0x4845B800 0x40>,
> > - <0x4845B900 0x40>,
> > - <0x4A002e94 0x4>;
> > + <0x4845B900 0x40>;
> > reg-names = "cal_top",
> > "cal_rx_core0",
> > - "cal_rx_core1",
> > - "camerrx_control";
> > + "cal_rx_core1";
> > interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > + syscon-camerrx = <&scm_conf 0xE94>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 02/19] dt-bindings: media: cal: update binding example
2019-10-29 13:21 ` Rob Herring
@ 2019-10-30 13:41 ` Benoit Parrot
0 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-30 13:41 UTC (permalink / raw)
To: Rob Herring; +Cc: Hans Verkuil, linux-media, devicetree, linux-kernel
Rob Herring <robh@kernel.org> wrote on Tue [2019-Oct-29 08:21:57 -0500]:
> On Fri, Oct 18, 2019 at 10:34:20AM -0500, Benoit Parrot wrote:
> > Update binding example to show proper endpoint properties and linkage.
> >
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> > .../devicetree/bindings/media/ti-cal.txt | 32 ++++++++++---------
> > 1 file changed, 17 insertions(+), 15 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/ti-cal.txt b/Documentation/devicetree/bindings/media/ti-cal.txt
> > index 782f801b12a9..6b56fddcfc4a 100644
> > --- a/Documentation/devicetree/bindings/media/ti-cal.txt
> > +++ b/Documentation/devicetree/bindings/media/ti-cal.txt
> > @@ -27,7 +27,6 @@ Documentation/devicetree/bindings/media/video-interfaces.txt.
> > Example:
> > cal: cal@4845b000 {
> > compatible = "ti,dra72-cal";
> > - ti,hwmods = "cal";
> > reg = <0x4845B000 0x400>,
> > <0x4845B800 0x40>,
> > <0x4845B900 0x40>;
> > @@ -45,9 +44,10 @@ Example:
> >
> > csi2_0: port@0 {
> > reg = <0>;
> > - endpoint {
> > - slave-mode;
> > - remote-endpoint = <&ar0330_1>;
> > + csi2_phy0: endpoint@0 {
>
> Unit address without 'reg' is not correct.
Ok so just "endpoint" then?
>
> > + remote-endpoint = <&csi2_cam0>;
> > + clock-lanes = <0>;
> > + data-lanes = <1 2>;
> > };
> > };
> > csi2_1: port@1 {
> > @@ -57,19 +57,21 @@ Example:
> > };
> >
> > i2c5: i2c@4807c000 {
> > - ar0330@10 {
> > - compatible = "ti,ar0330";
> > - reg = <0x10>;
> > + status = "okay";
>
> Don't show status in examples.
Ok.
>
> > + clock-frequency = <400000>;
> >
> > - port {
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > + ov5640@3c {
>
> camera-sensor@3c
Ok.
>
> > + compatible = "ovti,ov5640";
> > + reg = <0x3c>;
> > +
> > + clocks = <&clk_ov5640_fixed>;
> > + clock-names = "xclk";
> >
> > - ar0330_1: endpoint {
> > - reg = <0>;
> > - clock-lanes = <1>;
> > - data-lanes = <0 2 3 4>;
> > - remote-endpoint = <&csi2_0>;
> > + port {
> > + csi2_cam0: endpoint {
> > + remote-endpoint = <&csi2_phy0>;
> > + clock-lanes = <0>;
> > + data-lanes = <1 2>;
> > };
> > };
> > };
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Patch 19/19] dt-bindings: media: cal: convert binding to yaml
2019-10-29 14:22 ` Rob Herring
@ 2019-10-30 13:54 ` Benoit Parrot
0 siblings, 0 replies; 40+ messages in thread
From: Benoit Parrot @ 2019-10-30 13:54 UTC (permalink / raw)
To: Rob Herring; +Cc: Hans Verkuil, linux-media, devicetree, linux-kernel
Rob Herring <robh@kernel.org> wrote on Tue [2019-Oct-29 09:22:53 -0500]:
> On Fri, Oct 18, 2019 at 10:34:37AM -0500, Benoit Parrot wrote:
> > Convert ti-cal.txt to ti,cal.yaml.
> >
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> > .../devicetree/bindings/media/ti,cal.yaml | 186 ++++++++++++++++++
> > .../devicetree/bindings/media/ti-cal.txt | 82 --------
> > 2 files changed, 186 insertions(+), 82 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/media/ti,cal.yaml
> > delete mode 100644 Documentation/devicetree/bindings/media/ti-cal.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/ti,cal.yaml b/Documentation/devicetree/bindings/media/ti,cal.yaml
> > new file mode 100644
> > index 000000000000..c3fbb22b4571
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/ti,cal.yaml
> > @@ -0,0 +1,186 @@
> > +# SPDX-License-Identifier: (GPL-2.0)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/ti,cal.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Texas Instruments DRA72x CAMERA ADAPTATION LAYER (CAL) Device Tree Bindings
> > +
> > +maintainers:
> > + - Benoit Parrot <bparrot@ti.com>
> > +
> > +description: |-
> > + The Camera Adaptation Layer (CAL) is a key component for image capture
> > + applications. The capture module provides the system interface and the
> > + processing capability to connect CSI2 image-sensor modules to the
> > + DRA72x device.
> > +
> > + CAL supports 2 camera port nodes on MIPI bus. Each CSI2 camera port nodes
> > + should contain a 'port' child node with child 'endpoint' node. Please
> > + refer to the bindings defined in
> > + Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +
> > + compatible should be
> > + "ti,dra72-cal", for DRA72 controllers
> > + "ti,dra72-pre-es2-cal", for DRA72 controllers pre ES2.0
> > + "ti,dra76-cal", for DRA76 controllers
> > + "ti,am654-cal", for AM654 controllers
>
> Drop these or add as comments to the schema below.
Ok.
>
> > +
> > +properties:
> > + compatible:
> > + items:
>
> You can drop 'items' here since there is only 1.
Ok so just enum then?
>
> > + - enum:
> > + - ti,dra72-cal
> > + - ti,dra72-pre-es2-cal
> > + - ti,dra76-cal
> > + - ti,am654-cal
> > +
> > + reg:
> > + minItems: 2
> > + items:
> > + - description: The CAL main register region
> > + - description: The RX Core0 (DPHY0) register region
> > + - description: The RX Core1 (DPHY1) register region
> > +
> > + reg-names:
> > + minItems: 2
> > + items:
> > + - const: cal_top
> > + - const: cal_rx_core0
> > + - const: cal_rx_core1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + syscon-camerrx:
>
> Needs a type definition.
Hmm, is there a phandle generic type?
>
> > + maxItems: 1
> > + items:
> > + - description:
> > + phandle to the device control module and offset to the
> > + control_camerarx_core register
> > +
> > + clocks:
> > + maxItems: 1
> > + description: functional clock
>
> You can drop the description.
Ok.
>
> > +
> > + clock-names:
> > + items:
>
> Drop items.
Ok.
>
> > + - const: fck
> > +
> > + power-domains:
> > + description:
> > + List of phandle and PM domain specifier as documented in
> > + Documentation/devicetree/bindings/power/power_domain.txt
> > + maxItems: 1
> > +
> > + # See ./video-interfaces.txt for details
> > + ports:
> > + maxItems: 1
>
> But ports is not an array...
So remove maxItems? Not sure what you mean here?
>
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + patternProperties:
> > + '^port@[0-9a-fA-F]+$':
>
> In a device binding, you need to specify 'port@0' and 'port@1' (assuming
> 0 and 1) and say what they are.
Well depending on the 'variant' the device might have 1 or 2 physical csi2
ports so how would you represent that then?
>
> > + minItems: 1
> > + maxItems: 2
>
> Not valid for an object (aka node).
Again not sure how else to represent this.
>
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + reg:
> > + minItems: 1
> > + items:
> > + - description: The port id
> > +
> > + patternProperties:
> > + '^endpoint@[0-9a-fA-F]+$':
>
> Just 'endpoint' is valid too.
Ok I'll change that.
>
> > + minItems: 1
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + clock-lanes:
> > + maxItems: 1
> > +
> > + data-lanes:
> > + minItems: 1
> > + maxItems: 4
> > +
> > + remote-endpoint: true
> > +
> > + required:
> > + - remote-endpoint
> > +
> > + required:
> > + - reg
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - interrupts
> > + - syscon-camerrx
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > + cal: cal@4845b000 {
> > + compatible = "ti,dra72-cal";
> > + reg = <0x4845B000 0x400>,
> > + <0x4845B800 0x40>,
> > + <0x4845B900 0x40>;
> > + reg-names = "cal_top",
> > + "cal_rx_core0",
> > + "cal_rx_core1";
> > + interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > + syscon-camerrx = <&scm_conf 0xE94>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + csi2_0: port@0 {
> > + reg = <0>;
> > + csi2_phy0: endpoint@0 {
> > + remote-endpoint = <&csi2_cam0>;
> > + clock-lanes = <0>;
> > + data-lanes = <1 2>;
> > + };
> > + };
> > + };
> > + };
> > +
> > + i2c5: i2c@4807c000 {
> > + status = "okay";
> > + clock-frequency = <400000>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + ov5640@3c {
> > + compatible = "ovti,ov5640";
> > + reg = <0x3c>;
> > +
> > + clocks = <&clk_ov5640_fixed>;
> > + clock-names = "xclk";
> > +
> > + port {
> > + csi2_cam0: endpoint@0 {
> > + remote-endpoint = <&csi2_phy0>;
> > + clock-lanes = <0>;
> > + data-lanes = <1 2>;
> > + };
> > + };
> > + };
> > + };
> > +
> > +...
^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2019-10-30 13:54 UTC | newest]
Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-18 15:34 [Patch 00/19] media: ti-vpe: cal: maintenance Benoit Parrot
2019-10-18 15:34 ` [Patch 01/19] dt-bindings: media: cal: update binding to use syscon Benoit Parrot
2019-10-29 13:20 ` Rob Herring
2019-10-30 13:39 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 02/19] dt-bindings: media: cal: update binding example Benoit Parrot
2019-10-29 13:21 ` Rob Herring
2019-10-30 13:41 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 03/19] media: ti-vpe: cal: Add per platform data support Benoit Parrot
2019-10-29 13:18 ` Rob Herring
2019-10-30 13:34 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 04/19] media: ti-vpe: cal: Enable DMABUF export Benoit Parrot
2019-10-18 15:34 ` [Patch 05/19] media: ti-vpe: cal: Restrict DMA to avoid memory corruption Benoit Parrot
2019-10-18 15:34 ` [Patch 06/19] dt-bindings: media: cal: update binding to add PHY LDO errata support Benoit Parrot
2019-10-29 13:23 ` Rob Herring
2019-10-18 15:34 ` [Patch 07/19] media: ti-vpe: cal: add CSI2 " Benoit Parrot
2019-10-21 10:38 ` Hans Verkuil
2019-10-23 16:03 ` Benoit Parrot
2019-10-18 15:34 ` [Patch 08/19] media: ti-vpe: cal: Fix ths_term/ths_settle parameters Benoit Parrot
2019-10-18 15:34 ` [Patch 09/19] media: ti-vpe: cal: Fix pixel processing parameters Benoit Parrot
2019-10-18 15:34 ` [Patch 10/19] media: ti-vpe: cal: Align DPHY init sequence with docs Benoit Parrot
2019-10-18 15:34 ` [Patch 11/19] dt-bindings: media: cal: update binding to add DRA76x support Benoit Parrot
2019-10-29 13:24 ` Rob Herring
2019-10-18 15:34 ` [Patch 12/19] media: ti-vpe: cal: Add " Benoit Parrot
2019-10-18 15:34 ` [Patch 13/19] dt-bindings: media: cal: update binding to add AM654 support Benoit Parrot
2019-10-29 13:24 ` Rob Herring
2019-10-18 15:34 ` [Patch 14/19] media: ti-vpe: cal: Add " Benoit Parrot
2019-10-18 15:34 ` [Patch 15/19] media: ti-vpe: cal: Add subdev s_power hooks Benoit Parrot
2019-10-18 15:34 ` [Patch 16/19] media: ti-vpe: cal: Properly calculate max resolution boundary Benoit Parrot
2019-10-18 15:34 ` [Patch 17/19] media: ti-vpe: cal: Fix a WARN issued when start streaming fails Benoit Parrot
2019-10-18 15:34 ` [Patch 18/19] media: ti-vpe: cal: fix enum_mbus_code/frame_size subdev arguments Benoit Parrot
2019-10-18 15:34 ` [Patch 19/19] dt-bindings: media: cal: convert binding to yaml Benoit Parrot
2019-10-21 10:49 ` Hans Verkuil
2019-10-23 16:06 ` Benoit Parrot
2019-10-22 7:46 ` Sakari Ailus
2019-10-23 16:18 ` Benoit Parrot
2019-10-24 6:13 ` Sakari Ailus
2019-10-29 14:22 ` Rob Herring
2019-10-30 13:54 ` Benoit Parrot
2019-10-21 10:50 ` [Patch 00/19] media: ti-vpe: cal: maintenance Hans Verkuil
2019-10-23 16:05 ` Benoit Parrot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).