linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] OMAP 3 CSI-2 configuration
@ 2012-10-10 20:01 Sakari Ailus
  2012-10-10 20:01 ` [PATCH v4 1/3] omap3isp: Add CSI configuration registers from control block to ISP resources Sakari Ailus
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Sakari Ailus @ 2012-10-10 20:01 UTC (permalink / raw)
  To: linux-media, linux-omap; +Cc: laurent.pinchart, tony, khilman

Hi all,

This is an update to an old patchset for CSI-2 configuration for OMAP 3430
and 3630. The patches have been tested on the 3630 only so far, and I don't
plan to test them on 3430 in the near future.

Changes made based on Kevin's and Laurent's comments since v3.

Comments, questions and other kind of feedback is very welcome.

Kind regards,

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/3] omap3isp: Add CSI configuration registers from control block to ISP resources
  2012-10-10 20:01 [PATCH v4 0/3] OMAP 3 CSI-2 configuration Sakari Ailus
@ 2012-10-10 20:01 ` Sakari Ailus
  2012-10-10 20:01 ` [PATCH v4 2/3] omap3isp: Add PHY routing configuration Sakari Ailus
  2012-10-10 20:01 ` [PATCH v4 3/3] omap3isp: Configure CSI-2 phy based on platform data Sakari Ailus
  2 siblings, 0 replies; 8+ messages in thread
From: Sakari Ailus @ 2012-10-10 20:01 UTC (permalink / raw)
  To: linux-media, linux-omap; +Cc: laurent.pinchart, tony, khilman

Add the registers used to configure the CSI-2 receiver PHY on OMAP3430 and
3630 and map them in the ISP driver. The register is part of the control
block but it only is needed by the ISP driver.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/devices.c         |   10 ++++++++++
 drivers/media/platform/omap3isp/isp.c |    6 ++++--
 drivers/media/platform/omap3isp/isp.h |    2 ++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index c00c689..9e4d5da 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -201,6 +201,16 @@ static struct resource omap3isp_resources[] = {
 		.flags		= IORESOURCE_MEM,
 	},
 	{
+		.start		= OMAP343X_CTRL_BASE + OMAP343X_CONTROL_CSIRXFE,
+		.end		= OMAP343X_CTRL_BASE + OMAP343X_CONTROL_CSIRXFE + 3,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP343X_CTRL_BASE + OMAP3630_CONTROL_CAMERA_PHY_CTRL,
+		.end		= OMAP343X_CTRL_BASE + OMAP3630_CONTROL_CAMERA_PHY_CTRL + 3,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
 		.start		= INT_34XX_CAM_IRQ,
 		.flags		= IORESOURCE_IRQ,
 	}
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 6034dca..972e7b5 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -100,7 +100,8 @@ static const struct isp_res_mapping isp_res_maps[] = {
 		       1 << OMAP3_ISP_IOMEM_RESZ |
 		       1 << OMAP3_ISP_IOMEM_SBL |
 		       1 << OMAP3_ISP_IOMEM_CSI2A_REGS1 |
-		       1 << OMAP3_ISP_IOMEM_CSIPHY2,
+		       1 << OMAP3_ISP_IOMEM_CSIPHY2 |
+		       1 << OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE,
 	},
 	{
 		.isp_rev = ISP_REVISION_15_0,
@@ -117,7 +118,8 @@ static const struct isp_res_mapping isp_res_maps[] = {
 		       1 << OMAP3_ISP_IOMEM_CSI2A_REGS2 |
 		       1 << OMAP3_ISP_IOMEM_CSI2C_REGS1 |
 		       1 << OMAP3_ISP_IOMEM_CSIPHY1 |
-		       1 << OMAP3_ISP_IOMEM_CSI2C_REGS2,
+		       1 << OMAP3_ISP_IOMEM_CSI2C_REGS2 |
+		       1 << OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL,
 	},
 };
 
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 8be7487..6fed222 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -72,6 +72,8 @@ enum isp_mem_resources {
 	OMAP3_ISP_IOMEM_CSI2C_REGS1,
 	OMAP3_ISP_IOMEM_CSIPHY1,
 	OMAP3_ISP_IOMEM_CSI2C_REGS2,
+	OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE,
+	OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL,
 	OMAP3_ISP_IOMEM_LAST
 };
 
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/3] omap3isp: Add PHY routing configuration
  2012-10-10 20:01 [PATCH v4 0/3] OMAP 3 CSI-2 configuration Sakari Ailus
  2012-10-10 20:01 ` [PATCH v4 1/3] omap3isp: Add CSI configuration registers from control block to ISP resources Sakari Ailus
@ 2012-10-10 20:01 ` Sakari Ailus
  2012-10-10 23:48   ` Laurent Pinchart
  2012-10-10 20:01 ` [PATCH v4 3/3] omap3isp: Configure CSI-2 phy based on platform data Sakari Ailus
  2 siblings, 1 reply; 8+ messages in thread
From: Sakari Ailus @ 2012-10-10 20:01 UTC (permalink / raw)
  To: linux-media, linux-omap; +Cc: laurent.pinchart, tony, khilman

Add PHY routing configuration for both 3430 and 3630. Also add register bit
definitions of CSIRXFE and CAMERA_PHY_CTRL registers on OMAP 3430 and 3630,
respectively.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 drivers/media/platform/omap3isp/ispcsiphy.c |   92 +++++++++++++++++++++++++++
 drivers/media/platform/omap3isp/ispreg.h    |   22 +++++++
 2 files changed, 114 insertions(+), 0 deletions(-)

diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index 348f67e..12ae394 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -32,6 +32,98 @@
 #include "ispreg.h"
 #include "ispcsiphy.h"
 
+static void csiphy_routing_cfg_3630(struct isp_csiphy *phy, u32 iface,
+				    bool ccp2_strobe)
+{
+	u32 reg = isp_reg_readl(
+		phy->isp, OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
+	u32 shift, mode;
+
+	switch (iface) {
+	case ISP_INTERFACE_CCP2B_PHY1:
+		reg &= ~OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2;
+		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT;
+		break;
+	case ISP_INTERFACE_CSI2C_PHY1:
+		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT;
+		mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY;
+		break;
+	case ISP_INTERFACE_CCP2B_PHY2:
+		reg |= OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2;
+		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT;
+		break;
+	case ISP_INTERFACE_CSI2A_PHY2:
+		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT;
+		mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY;
+		break;
+	}
+
+	/* Select data/clock or data/strobe mode for CCP2 */
+	switch (iface) {
+	case ISP_INTERFACE_CCP2B_PHY1:
+	case ISP_INTERFACE_CCP2B_PHY2:
+		if (ccp2_strobe)
+			mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_STROBE;
+		else
+			mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_CLOCK;
+	}
+
+	reg &= ~(OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK << shift);
+	reg |= mode << shift;
+
+	isp_reg_writel(phy->isp, reg,
+		       OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
+}
+
+static void csiphy_routing_cfg_3430(struct isp_csiphy *phy, u32 iface, bool on,
+				    bool ccp2_strobe)
+{
+	uint32_t csirxfe = OMAP343X_CONTROL_CSIRXFE_PWRDNZ
+		| OMAP343X_CONTROL_CSIRXFE_RESET;
+
+	/* Nothing to configure here. */
+	if (iface == ISP_INTERFACE_CSI2A_PHY2)
+		return;
+
+	if (iface != ISP_INTERFACE_CCP2B_PHY1)
+		return;
+
+	if (!on) {
+		isp_reg_writel(phy->isp, 0,
+			       OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
+		return;
+	}
+
+	if (ccp2_strobe)
+		csirxfe |= OMAP343X_CONTROL_CSIRXFE_SELFORM;
+
+	isp_reg_writel(phy->isp, csirxfe,
+		       OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
+}
+
+/**
+ * Configure OMAP 3 CSI PHY routing.
+ *
+ * Note that the underlying routing configuration registers are part
+ * of the control (SCM) register space and part of the CORE power
+ * domain on both 3430 and 3630, so they will not hold their contents
+ * in off-mode.
+ *
+ * @phy: relevant phy device
+ * @iface: ISP_INTERFACE_*
+ * @on: power on or off
+ * @ccp2_strobe: false: data/clock, true: data/strobe
+ */
+static void csiphy_routing_cfg(struct isp_csiphy *phy, u32 iface, bool on,
+			       bool ccp2_strobe)
+{
+	if (phy->isp->mmio_base[OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL]
+	    && on)
+		return csiphy_routing_cfg_3630(phy, iface, ccp2_strobe);
+	if (phy->isp->mmio_base[OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE])
+		return csiphy_routing_cfg_3430(phy, iface, on, ccp2_strobe);
+}
+
 /*
  * csiphy_lanes_config - Configuration of CSIPHY lanes.
  *
diff --git a/drivers/media/platform/omap3isp/ispreg.h b/drivers/media/platform/omap3isp/ispreg.h
index e2c57f3..148108b 100644
--- a/drivers/media/platform/omap3isp/ispreg.h
+++ b/drivers/media/platform/omap3isp/ispreg.h
@@ -1583,4 +1583,26 @@
 #define ISPCSIPHY_REG2_CCP2_SYNC_PATTERN_MASK		\
 	(0x7fffff << ISPCSIPHY_REG2_CCP2_SYNC_PATTERN_SHIFT)
 
+/* -----------------------------------------------------------------------------
+ * CONTROL registers for CSI-2 phy routing
+ */
+
+/* OMAP343X_CONTROL_CSIRXFE */
+#define OMAP343X_CONTROL_CSIRXFE_CSIB_INV	(1 << 7)
+#define OMAP343X_CONTROL_CSIRXFE_RESENABLE	(1 << 8)
+#define OMAP343X_CONTROL_CSIRXFE_SELFORM	(1 << 10)
+#define OMAP343X_CONTROL_CSIRXFE_PWRDNZ		(1 << 12)
+#define OMAP343X_CONTROL_CSIRXFE_RESET		(1 << 13)
+
+/* OMAP3630_CONTROL_CAMERA_PHY_CTRL */
+#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT	2
+#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT	0
+#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY		0x0
+#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_STROBE 0x1
+#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_CLOCK 0x2
+#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_GPI		0x3
+#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK		0x3
+/* CCP2B: set to receive data from PHY2 instead of PHY1 */
+#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2	(1 << 4)
+
 #endif	/* OMAP3_ISP_REG_H */
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 3/3] omap3isp: Configure CSI-2 phy based on platform data
  2012-10-10 20:01 [PATCH v4 0/3] OMAP 3 CSI-2 configuration Sakari Ailus
  2012-10-10 20:01 ` [PATCH v4 1/3] omap3isp: Add CSI configuration registers from control block to ISP resources Sakari Ailus
  2012-10-10 20:01 ` [PATCH v4 2/3] omap3isp: Add PHY routing configuration Sakari Ailus
@ 2012-10-10 20:01 ` Sakari Ailus
  2012-10-10 23:34   ` Laurent Pinchart
  2 siblings, 1 reply; 8+ messages in thread
From: Sakari Ailus @ 2012-10-10 20:01 UTC (permalink / raw)
  To: linux-media, linux-omap; +Cc: laurent.pinchart, tony, khilman

Configure CSI-2 phy based on platform data in the ISP driver. For that, the
new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same
was configured from the board code.

This patch is dependent on "omap3: Provide means for changing CSI2 PHY
configuration".

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/omap3isp/isp.h       |    3 -
 drivers/media/platform/omap3isp/ispcsiphy.c |  148 ++++++++++++++-------------
 drivers/media/platform/omap3isp/ispcsiphy.h |   10 --
 3 files changed, 78 insertions(+), 83 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 6fed222..accb3b0 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -129,9 +129,6 @@ struct isp_reg {
 
 struct isp_platform_callback {
 	u32 (*set_xclk)(struct isp_device *isp, u32 xclk, u8 xclksel);
-	int (*csiphy_config)(struct isp_csiphy *phy,
-			     struct isp_csiphy_dphy_cfg *dphy,
-			     struct isp_csiphy_lanes_cfg *lanes);
 };
 
 /*
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index 12ae394..754d7a1 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -125,36 +125,6 @@ static void csiphy_routing_cfg(struct isp_csiphy *phy, u32 iface, bool on,
 }
 
 /*
- * csiphy_lanes_config - Configuration of CSIPHY lanes.
- *
- * Updates HW configuration.
- * Called with phy->mutex taken.
- */
-static void csiphy_lanes_config(struct isp_csiphy *phy)
-{
-	unsigned int i;
-	u32 reg;
-
-	reg = isp_reg_readl(phy->isp, phy->cfg_regs, ISPCSI2_PHY_CFG);
-
-	for (i = 0; i < phy->num_data_lanes; i++) {
-		reg &= ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) |
-			 ISPCSI2_PHY_CFG_DATA_POSITION_MASK(i + 1));
-		reg |= (phy->lanes.data[i].pol <<
-			ISPCSI2_PHY_CFG_DATA_POL_SHIFT(i + 1));
-		reg |= (phy->lanes.data[i].pos <<
-			ISPCSI2_PHY_CFG_DATA_POSITION_SHIFT(i + 1));
-	}
-
-	reg &= ~(ISPCSI2_PHY_CFG_CLOCK_POL_MASK |
-		 ISPCSI2_PHY_CFG_CLOCK_POSITION_MASK);
-	reg |= phy->lanes.clk.pol << ISPCSI2_PHY_CFG_CLOCK_POL_SHIFT;
-	reg |= phy->lanes.clk.pos << ISPCSI2_PHY_CFG_CLOCK_POSITION_SHIFT;
-
-	isp_reg_writel(phy->isp, reg, phy->cfg_regs, ISPCSI2_PHY_CFG);
-}
-
-/*
  * csiphy_power_autoswitch_enable
  * @enable: Sets or clears the autoswitch function enable flag.
  */
@@ -199,43 +169,28 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 power)
 }
 
 /*
- * csiphy_dphy_config - Configure CSI2 D-PHY parameters.
- *
- * Called with phy->mutex taken.
+ * TCLK values are OK at their reset values
  */
-static void csiphy_dphy_config(struct isp_csiphy *phy)
-{
-	u32 reg;
-
-	/* Set up ISPCSIPHY_REG0 */
-	reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG0);
-
-	reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK |
-		 ISPCSIPHY_REG0_THS_SETTLE_MASK);
-	reg |= phy->dphy.ths_term << ISPCSIPHY_REG0_THS_TERM_SHIFT;
-	reg |= phy->dphy.ths_settle << ISPCSIPHY_REG0_THS_SETTLE_SHIFT;
-
-	isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG0);
-
-	/* Set up ISPCSIPHY_REG1 */
-	reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG1);
-
-	reg &= ~(ISPCSIPHY_REG1_TCLK_TERM_MASK |
-		 ISPCSIPHY_REG1_TCLK_MISS_MASK |
-		 ISPCSIPHY_REG1_TCLK_SETTLE_MASK);
-	reg |= phy->dphy.tclk_term << ISPCSIPHY_REG1_TCLK_TERM_SHIFT;
-	reg |= phy->dphy.tclk_miss << ISPCSIPHY_REG1_TCLK_MISS_SHIFT;
-	reg |= phy->dphy.tclk_settle << ISPCSIPHY_REG1_TCLK_SETTLE_SHIFT;
+#define TCLK_TERM	0
+#define TCLK_MISS	1
+#define TCLK_SETTLE	14
 
-	isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG1);
-}
-
-static int csiphy_config(struct isp_csiphy *phy,
-			 struct isp_csiphy_dphy_cfg *dphy,
-			 struct isp_csiphy_lanes_cfg *lanes)
+static int omap3isp_csiphy_config(struct isp_csiphy *phy)
 {
+	struct isp_csi2_device *csi2 = phy->csi2;
+	struct isp_pipeline *pipe = to_isp_pipeline(&csi2->subdev.entity);
+	struct isp_v4l2_subdevs_group *subdevs = pipe->external->host_priv;
+	struct isp_csiphy_lanes_cfg *lanes;
+	int csi2_ddrclk_khz;
 	unsigned int used_lanes = 0;
 	unsigned int i;
+	u32 reg;
+
+	if (subdevs->interface == ISP_INTERFACE_CCP2B_PHY1
+	    || subdevs->interface == ISP_INTERFACE_CCP2B_PHY2)
+		lanes = &subdevs->bus.ccp2.lanecfg;
+	else
+		lanes = &subdevs->bus.csi2.lanecfg;
 
 	/* Clock and data lanes verification */
 	for (i = 0; i < phy->num_data_lanes; i++) {
@@ -254,10 +209,56 @@ static int csiphy_config(struct isp_csiphy *phy,
 	if (lanes->clk.pos == 0 || used_lanes & (1 << lanes->clk.pos))
 		return -EINVAL;
 
-	mutex_lock(&phy->mutex);
-	phy->dphy = *dphy;
-	phy->lanes = *lanes;
-	mutex_unlock(&phy->mutex);
+	csiphy_routing_cfg(phy, subdevs->interface, true,
+			   subdevs->bus.ccp2.phy_layer);
+
+	/* DPHY timing configuration */
+	/* CSI-2 is DDR and we only count used lanes. */
+	csi2_ddrclk_khz = pipe->external_rate / 1000
+		/ (2 * hweight32(used_lanes)) * pipe->external_width;
+
+	reg = isp_reg_readl(csi2->isp, phy->phy_regs, ISPCSIPHY_REG0);
+
+	reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK |
+		 ISPCSIPHY_REG0_THS_SETTLE_MASK);
+	/* THS_TERM: Programmed value = ceil(12.5 ns/DDRClk period) - 1. */
+	reg |= (DIV_ROUND_UP(25 * csi2_ddrclk_khz, 2000000) - 1)
+		<< ISPCSIPHY_REG0_THS_TERM_SHIFT;
+	/* THS_SETTLE: Programmed value = ceil(90 ns/DDRClk period) + 3. */
+	reg |= (DIV_ROUND_UP(90 * csi2_ddrclk_khz, 1000000) + 3)
+		<< ISPCSIPHY_REG0_THS_SETTLE_SHIFT;
+
+	isp_reg_writel(csi2->isp, reg, phy->phy_regs, ISPCSIPHY_REG0);
+
+	reg = isp_reg_readl(csi2->isp, phy->phy_regs, ISPCSIPHY_REG1);
+
+	reg &= ~(ISPCSIPHY_REG1_TCLK_TERM_MASK |
+		 ISPCSIPHY_REG1_TCLK_MISS_MASK |
+		 ISPCSIPHY_REG1_TCLK_SETTLE_MASK);
+	reg |= TCLK_TERM << ISPCSIPHY_REG1_TCLK_TERM_SHIFT;
+	reg |= TCLK_MISS << ISPCSIPHY_REG1_TCLK_MISS_SHIFT;
+	reg |= TCLK_SETTLE << ISPCSIPHY_REG1_TCLK_SETTLE_SHIFT;
+
+	isp_reg_writel(csi2->isp, reg, phy->phy_regs, ISPCSIPHY_REG1);
+
+	/* DPHY lane configuration */
+	reg = isp_reg_readl(csi2->isp, phy->cfg_regs, ISPCSI2_PHY_CFG);
+
+	for (i = 0; i < phy->num_data_lanes; i++) {
+		reg &= ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) |
+			 ISPCSI2_PHY_CFG_DATA_POSITION_MASK(i + 1));
+		reg |= (lanes->data[i].pol <<
+			ISPCSI2_PHY_CFG_DATA_POL_SHIFT(i + 1));
+		reg |= (lanes->data[i].pos <<
+			ISPCSI2_PHY_CFG_DATA_POSITION_SHIFT(i + 1));
+	}
+
+	reg &= ~(ISPCSI2_PHY_CFG_CLOCK_POL_MASK |
+		 ISPCSI2_PHY_CFG_CLOCK_POSITION_MASK);
+	reg |= lanes->clk.pol << ISPCSI2_PHY_CFG_CLOCK_POL_SHIFT;
+	reg |= lanes->clk.pos << ISPCSI2_PHY_CFG_CLOCK_POSITION_SHIFT;
+
+	isp_reg_writel(csi2->isp, reg, phy->cfg_regs, ISPCSI2_PHY_CFG);
 
 	return 0;
 }
@@ -282,8 +283,9 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy)
 	if (rval < 0)
 		goto done;
 
-	csiphy_dphy_config(phy);
-	csiphy_lanes_config(phy);
+	rval = omap3isp_csiphy_config(phy);
+	if (rval < 0)
+		goto done;
 
 	rval = csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_ON);
 	if (rval) {
@@ -303,6 +305,14 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
 {
 	mutex_lock(&phy->mutex);
 	if (phy->phy_in_use) {
+		struct isp_csi2_device *csi2 = phy->csi2;
+		struct isp_pipeline *pipe =
+			to_isp_pipeline(&csi2->subdev.entity);
+		struct isp_v4l2_subdevs_group *subdevs =
+			pipe->external->host_priv;
+
+		csiphy_routing_cfg(phy, subdevs->interface, false,
+				   subdevs->bus.ccp2.phy_layer);
 		csiphy_power_autoswitch_enable(phy, false);
 		csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_OFF);
 		regulator_disable(phy->vdd);
@@ -319,8 +329,6 @@ int omap3isp_csiphy_init(struct isp_device *isp)
 	struct isp_csiphy *phy1 = &isp->isp_csiphy1;
 	struct isp_csiphy *phy2 = &isp->isp_csiphy2;
 
-	isp->platform_cb.csiphy_config = csiphy_config;
-
 	phy2->isp = isp;
 	phy2->csi2 = &isp->isp_csi2a;
 	phy2->num_data_lanes = ISP_CSIPHY2_NUM_DATA_LANES;
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.h b/drivers/media/platform/omap3isp/ispcsiphy.h
index e93a661..14551fd 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.h
+++ b/drivers/media/platform/omap3isp/ispcsiphy.h
@@ -32,14 +32,6 @@
 struct isp_csi2_device;
 struct regulator;
 
-struct isp_csiphy_dphy_cfg {
-	u8 ths_term;
-	u8 ths_settle;
-	u8 tclk_term;
-	unsigned tclk_miss:1;
-	u8 tclk_settle;
-};
-
 struct isp_csiphy {
 	struct isp_device *isp;
 	struct mutex mutex;	/* serialize csiphy configuration */
@@ -52,8 +44,6 @@ struct isp_csiphy {
 	unsigned int phy_regs;
 
 	u8 num_data_lanes;	/* number of CSI2 Data Lanes supported */
-	struct isp_csiphy_lanes_cfg lanes;
-	struct isp_csiphy_dphy_cfg dphy;
 };
 
 int omap3isp_csiphy_acquire(struct isp_csiphy *phy);
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 3/3] omap3isp: Configure CSI-2 phy based on platform data
  2012-10-10 20:01 ` [PATCH v4 3/3] omap3isp: Configure CSI-2 phy based on platform data Sakari Ailus
@ 2012-10-10 23:34   ` Laurent Pinchart
  2012-10-13 11:14     ` Sakari Ailus
  0 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2012-10-10 23:34 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, linux-omap, tony, khilman

Hi Sakari,

Thanks for the patch.

On Wednesday 10 October 2012 23:01:42 Sakari Ailus wrote:
> Configure CSI-2 phy based on platform data in the ISP driver. For that, the
> new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same
> was configured from the board code.
> 
> This patch is dependent on "omap3: Provide means for changing CSI2 PHY
> configuration".

Is it still ?

> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/omap3isp/isp.h       |    3 -
>  drivers/media/platform/omap3isp/ispcsiphy.c |  148 ++++++++++++------------
>  drivers/media/platform/omap3isp/ispcsiphy.h |   10 --
>  3 files changed, 78 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.h
> b/drivers/media/platform/omap3isp/isp.h index 6fed222..accb3b0 100644
> --- a/drivers/media/platform/omap3isp/isp.h
> +++ b/drivers/media/platform/omap3isp/isp.h
> @@ -129,9 +129,6 @@ struct isp_reg {
> 
>  struct isp_platform_callback {
>  	u32 (*set_xclk)(struct isp_device *isp, u32 xclk, u8 xclksel);
> -	int (*csiphy_config)(struct isp_csiphy *phy,
> -			     struct isp_csiphy_dphy_cfg *dphy,
> -			     struct isp_csiphy_lanes_cfg *lanes);
>  };
> 
>  /*
> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
> b/drivers/media/platform/omap3isp/ispcsiphy.c index 12ae394..754d7a1 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> @@ -125,36 +125,6 @@ static void csiphy_routing_cfg(struct isp_csiphy *phy,
> u32 iface, bool on, }
> 
>  /*
> - * csiphy_lanes_config - Configuration of CSIPHY lanes.
> - *
> - * Updates HW configuration.
> - * Called with phy->mutex taken.
> - */
> -static void csiphy_lanes_config(struct isp_csiphy *phy)
> -{
> -	unsigned int i;
> -	u32 reg;
> -
> -	reg = isp_reg_readl(phy->isp, phy->cfg_regs, ISPCSI2_PHY_CFG);
> -
> -	for (i = 0; i < phy->num_data_lanes; i++) {
> -		reg &= ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) |
> -			 ISPCSI2_PHY_CFG_DATA_POSITION_MASK(i + 1));
> -		reg |= (phy->lanes.data[i].pol <<
> -			ISPCSI2_PHY_CFG_DATA_POL_SHIFT(i + 1));
> -		reg |= (phy->lanes.data[i].pos <<
> -			ISPCSI2_PHY_CFG_DATA_POSITION_SHIFT(i + 1));
> -	}
> -
> -	reg &= ~(ISPCSI2_PHY_CFG_CLOCK_POL_MASK |
> -		 ISPCSI2_PHY_CFG_CLOCK_POSITION_MASK);
> -	reg |= phy->lanes.clk.pol << ISPCSI2_PHY_CFG_CLOCK_POL_SHIFT;
> -	reg |= phy->lanes.clk.pos << ISPCSI2_PHY_CFG_CLOCK_POSITION_SHIFT;
> -
> -	isp_reg_writel(phy->isp, reg, phy->cfg_regs, ISPCSI2_PHY_CFG);
> -}
> -
> -/*
>   * csiphy_power_autoswitch_enable
>   * @enable: Sets or clears the autoswitch function enable flag.
>   */
> @@ -199,43 +169,28 @@ static int csiphy_set_power(struct isp_csiphy *phy,
> u32 power) }
> 
>  /*
> - * csiphy_dphy_config - Configure CSI2 D-PHY parameters.
> - *
> - * Called with phy->mutex taken.
> + * TCLK values are OK at their reset values
>   */
> -static void csiphy_dphy_config(struct isp_csiphy *phy)
> -{
> -	u32 reg;
> -
> -	/* Set up ISPCSIPHY_REG0 */
> -	reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG0);
> -
> -	reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK |
> -		 ISPCSIPHY_REG0_THS_SETTLE_MASK);
> -	reg |= phy->dphy.ths_term << ISPCSIPHY_REG0_THS_TERM_SHIFT;
> -	reg |= phy->dphy.ths_settle << ISPCSIPHY_REG0_THS_SETTLE_SHIFT;
> -
> -	isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG0);
> -
> -	/* Set up ISPCSIPHY_REG1 */
> -	reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG1);
> -
> -	reg &= ~(ISPCSIPHY_REG1_TCLK_TERM_MASK |
> -		 ISPCSIPHY_REG1_TCLK_MISS_MASK |
> -		 ISPCSIPHY_REG1_TCLK_SETTLE_MASK);
> -	reg |= phy->dphy.tclk_term << ISPCSIPHY_REG1_TCLK_TERM_SHIFT;
> -	reg |= phy->dphy.tclk_miss << ISPCSIPHY_REG1_TCLK_MISS_SHIFT;
> -	reg |= phy->dphy.tclk_settle << ISPCSIPHY_REG1_TCLK_SETTLE_SHIFT;
> +#define TCLK_TERM	0
> +#define TCLK_MISS	1
> +#define TCLK_SETTLE	14
> 
> -	isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG1);
> -}
> -
> -static int csiphy_config(struct isp_csiphy *phy,
> -			 struct isp_csiphy_dphy_cfg *dphy,
> -			 struct isp_csiphy_lanes_cfg *lanes)
> +static int omap3isp_csiphy_config(struct isp_csiphy *phy)
>  {
> +	struct isp_csi2_device *csi2 = phy->csi2;
> +	struct isp_pipeline *pipe = to_isp_pipeline(&csi2->subdev.entity);
> +	struct isp_v4l2_subdevs_group *subdevs = pipe->external->host_priv;
> +	struct isp_csiphy_lanes_cfg *lanes;
> +	int csi2_ddrclk_khz;
>  	unsigned int used_lanes = 0;
>  	unsigned int i;
> +	u32 reg;
> +
> +	if (subdevs->interface == ISP_INTERFACE_CCP2B_PHY1
> +	    || subdevs->interface == ISP_INTERFACE_CCP2B_PHY2)
> +		lanes = &subdevs->bus.ccp2.lanecfg;
> +	else
> +		lanes = &subdevs->bus.csi2.lanecfg;
> 
>  	/* Clock and data lanes verification */
>  	for (i = 0; i < phy->num_data_lanes; i++) {
> @@ -254,10 +209,56 @@ static int csiphy_config(struct isp_csiphy *phy,
>  	if (lanes->clk.pos == 0 || used_lanes & (1 << lanes->clk.pos))
>  		return -EINVAL;
> 
> -	mutex_lock(&phy->mutex);
> -	phy->dphy = *dphy;
> -	phy->lanes = *lanes;
> -	mutex_unlock(&phy->mutex);
> +	csiphy_routing_cfg(phy, subdevs->interface, true,
> +			   subdevs->bus.ccp2.phy_layer);
> +
> +	/* DPHY timing configuration */
> +	/* CSI-2 is DDR and we only count used lanes. */
> +	csi2_ddrclk_khz = pipe->external_rate / 1000
> +		/ (2 * hweight32(used_lanes)) * pipe->external_width;
> +
> +	reg = isp_reg_readl(csi2->isp, phy->phy_regs, ISPCSIPHY_REG0);
> +
> +	reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK |
> +		 ISPCSIPHY_REG0_THS_SETTLE_MASK);
> +	/* THS_TERM: Programmed value = ceil(12.5 ns/DDRClk period) - 1. */
> +	reg |= (DIV_ROUND_UP(25 * csi2_ddrclk_khz, 2000000) - 1)
> +		<< ISPCSIPHY_REG0_THS_TERM_SHIFT;
> +	/* THS_SETTLE: Programmed value = ceil(90 ns/DDRClk period) + 3. */
> +	reg |= (DIV_ROUND_UP(90 * csi2_ddrclk_khz, 1000000) + 3)
> +		<< ISPCSIPHY_REG0_THS_SETTLE_SHIFT;
> +
> +	isp_reg_writel(csi2->isp, reg, phy->phy_regs, ISPCSIPHY_REG0);
> +
> +	reg = isp_reg_readl(csi2->isp, phy->phy_regs, ISPCSIPHY_REG1);
> +
> +	reg &= ~(ISPCSIPHY_REG1_TCLK_TERM_MASK |
> +		 ISPCSIPHY_REG1_TCLK_MISS_MASK |
> +		 ISPCSIPHY_REG1_TCLK_SETTLE_MASK);
> +	reg |= TCLK_TERM << ISPCSIPHY_REG1_TCLK_TERM_SHIFT;
> +	reg |= TCLK_MISS << ISPCSIPHY_REG1_TCLK_MISS_SHIFT;
> +	reg |= TCLK_SETTLE << ISPCSIPHY_REG1_TCLK_SETTLE_SHIFT;
> +
> +	isp_reg_writel(csi2->isp, reg, phy->phy_regs, ISPCSIPHY_REG1);
> +
> +	/* DPHY lane configuration */
> +	reg = isp_reg_readl(csi2->isp, phy->cfg_regs, ISPCSI2_PHY_CFG);
> +
> +	for (i = 0; i < phy->num_data_lanes; i++) {
> +		reg &= ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) |
> +			 ISPCSI2_PHY_CFG_DATA_POSITION_MASK(i + 1));
> +		reg |= (lanes->data[i].pol <<
> +			ISPCSI2_PHY_CFG_DATA_POL_SHIFT(i + 1));
> +		reg |= (lanes->data[i].pos <<
> +			ISPCSI2_PHY_CFG_DATA_POSITION_SHIFT(i + 1));
> +	}
> +
> +	reg &= ~(ISPCSI2_PHY_CFG_CLOCK_POL_MASK |
> +		 ISPCSI2_PHY_CFG_CLOCK_POSITION_MASK);
> +	reg |= lanes->clk.pol << ISPCSI2_PHY_CFG_CLOCK_POL_SHIFT;
> +	reg |= lanes->clk.pos << ISPCSI2_PHY_CFG_CLOCK_POSITION_SHIFT;
> +
> +	isp_reg_writel(csi2->isp, reg, phy->cfg_regs, ISPCSI2_PHY_CFG);
> 
>  	return 0;
>  }
> @@ -282,8 +283,9 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy)
>  	if (rval < 0)
>  		goto done;
> 
> -	csiphy_dphy_config(phy);
> -	csiphy_lanes_config(phy);
> +	rval = omap3isp_csiphy_config(phy);
> +	if (rval < 0)
> +		goto done;
> 
>  	rval = csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_ON);
>  	if (rval) {
> @@ -303,6 +305,14 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
>  {
>  	mutex_lock(&phy->mutex);
>  	if (phy->phy_in_use) {
> +		struct isp_csi2_device *csi2 = phy->csi2;
> +		struct isp_pipeline *pipe =
> +			to_isp_pipeline(&csi2->subdev.entity);
> +		struct isp_v4l2_subdevs_group *subdevs =
> +			pipe->external->host_priv;
> +
> +		csiphy_routing_cfg(phy, subdevs->interface, false,
> +				   subdevs->bus.ccp2.phy_layer);
>  		csiphy_power_autoswitch_enable(phy, false);
>  		csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_OFF);
>  		regulator_disable(phy->vdd);
> @@ -319,8 +329,6 @@ int omap3isp_csiphy_init(struct isp_device *isp)
>  	struct isp_csiphy *phy1 = &isp->isp_csiphy1;
>  	struct isp_csiphy *phy2 = &isp->isp_csiphy2;
> 
> -	isp->platform_cb.csiphy_config = csiphy_config;
> -
>  	phy2->isp = isp;
>  	phy2->csi2 = &isp->isp_csi2a;
>  	phy2->num_data_lanes = ISP_CSIPHY2_NUM_DATA_LANES;
> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.h
> b/drivers/media/platform/omap3isp/ispcsiphy.h index e93a661..14551fd 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.h
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.h
> @@ -32,14 +32,6 @@
>  struct isp_csi2_device;
>  struct regulator;
> 
> -struct isp_csiphy_dphy_cfg {
> -	u8 ths_term;
> -	u8 ths_settle;
> -	u8 tclk_term;
> -	unsigned tclk_miss:1;
> -	u8 tclk_settle;
> -};
> -
>  struct isp_csiphy {
>  	struct isp_device *isp;
>  	struct mutex mutex;	/* serialize csiphy configuration */
> @@ -52,8 +44,6 @@ struct isp_csiphy {
>  	unsigned int phy_regs;
> 
>  	u8 num_data_lanes;	/* number of CSI2 Data Lanes supported */
> -	struct isp_csiphy_lanes_cfg lanes;
> -	struct isp_csiphy_dphy_cfg dphy;
>  };
> 
>  int omap3isp_csiphy_acquire(struct isp_csiphy *phy);

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/3] omap3isp: Add PHY routing configuration
  2012-10-10 20:01 ` [PATCH v4 2/3] omap3isp: Add PHY routing configuration Sakari Ailus
@ 2012-10-10 23:48   ` Laurent Pinchart
  2012-10-13 11:21     ` Sakari Ailus
  0 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2012-10-10 23:48 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, linux-omap, tony, khilman

Hi Sakari,

Thanks for the patch.

On Wednesday 10 October 2012 23:01:41 Sakari Ailus wrote:
> Add PHY routing configuration for both 3430 and 3630. Also add register bit
> definitions of CSIRXFE and CAMERA_PHY_CTRL registers on OMAP 3430 and 3630,
> respectively.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  drivers/media/platform/omap3isp/ispcsiphy.c |   92 ++++++++++++++++++++++++
>  drivers/media/platform/omap3isp/ispreg.h    |   22 +++++++
>  2 files changed, 114 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
> b/drivers/media/platform/omap3isp/ispcsiphy.c index 348f67e..12ae394 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> @@ -32,6 +32,98 @@
>  #include "ispreg.h"
>  #include "ispcsiphy.h"
> 
> +static void csiphy_routing_cfg_3630(struct isp_csiphy *phy, u32 iface,
> +				    bool ccp2_strobe)
> +{
> +	u32 reg = isp_reg_readl(
> +		phy->isp, OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
> +	u32 shift, mode;
> +
> +	switch (iface) {
> +	case ISP_INTERFACE_CCP2B_PHY1:
> +		reg &= ~OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2;
> +		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT;
> +		break;
> +	case ISP_INTERFACE_CSI2C_PHY1:
> +		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT;
> +		mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY;
> +		break;
> +	case ISP_INTERFACE_CCP2B_PHY2:
> +		reg |= OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2;
> +		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT;
> +		break;
> +	case ISP_INTERFACE_CSI2A_PHY2:
> +		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT;
> +		mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY;
> +		break;
> +	}
> +
> +	/* Select data/clock or data/strobe mode for CCP2 */
> +	switch (iface) {
> +	case ISP_INTERFACE_CCP2B_PHY1:
> +	case ISP_INTERFACE_CCP2B_PHY2:
> +		if (ccp2_strobe)
> +			mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_STROBE;
> +		else
> +			mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_CLOCK;
> +	}
> +
> +	reg &= ~(OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK << shift);
> +	reg |= mode << shift;
> +
> +	isp_reg_writel(phy->isp, reg,
> +		       OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
> +}
> +
> +static void csiphy_routing_cfg_3430(struct isp_csiphy *phy, u32 iface, bool
> on,
> +				    bool ccp2_strobe)
> +{
> +	uint32_t csirxfe = OMAP343X_CONTROL_CSIRXFE_PWRDNZ
> +		| OMAP343X_CONTROL_CSIRXFE_RESET;

Anything wrong with u32 ? :-)

(I would also align the | with the = but that's nitpicking)

> +
> +	/* Nothing to configure here. */
> +	if (iface == ISP_INTERFACE_CSI2A_PHY2)
> +		return;
> +
> +	if (iface != ISP_INTERFACE_CCP2B_PHY1)
> +		return;

Can't you get rid of the first check ?

> +	if (!on) {
> +		isp_reg_writel(phy->isp, 0,
> +			       OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
> +		return;
> +	}
> +
> +	if (ccp2_strobe)
> +		csirxfe |= OMAP343X_CONTROL_CSIRXFE_SELFORM;
> +
> +	isp_reg_writel(phy->isp, csirxfe,
> +		       OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
> +}
> +
> +/**
> + * Configure OMAP 3 CSI PHY routing.
> + *
> + * Note that the underlying routing configuration registers are part
> + * of the control (SCM) register space and part of the CORE power
> + * domain on both 3430 and 3630, so they will not hold their contents
> + * in off-mode.

Could you please add a sentence to explain why that's not an issue ?

> + * @phy: relevant phy device
> + * @iface: ISP_INTERFACE_*
> + * @on: power on or off
> + * @ccp2_strobe: false: data/clock, true: data/strobe
> + */
> +static void csiphy_routing_cfg(struct isp_csiphy *phy, u32 iface, bool on,
> +			       bool ccp2_strobe)
> +{
> +	if (phy->isp->mmio_base[OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL]
> +	    && on)
> +		return csiphy_routing_cfg_3630(phy, iface, ccp2_strobe);
> +	if (phy->isp->mmio_base[OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE])
> +		return csiphy_routing_cfg_3430(phy, iface, on, ccp2_strobe);
> +}
> +
>  /*
>   * csiphy_lanes_config - Configuration of CSIPHY lanes.
>   *
> diff --git a/drivers/media/platform/omap3isp/ispreg.h
> b/drivers/media/platform/omap3isp/ispreg.h index e2c57f3..148108b 100644
> --- a/drivers/media/platform/omap3isp/ispreg.h
> +++ b/drivers/media/platform/omap3isp/ispreg.h
> @@ -1583,4 +1583,26 @@
>  #define ISPCSIPHY_REG2_CCP2_SYNC_PATTERN_MASK		\
>  	(0x7fffff << ISPCSIPHY_REG2_CCP2_SYNC_PATTERN_SHIFT)
> 
> +/*
> ---------------------------------------------------------------------------
> -- + * CONTROL registers for CSI-2 phy routing
> + */
> +
> +/* OMAP343X_CONTROL_CSIRXFE */
> +#define OMAP343X_CONTROL_CSIRXFE_CSIB_INV	(1 << 7)
> +#define OMAP343X_CONTROL_CSIRXFE_RESENABLE	(1 << 8)
> +#define OMAP343X_CONTROL_CSIRXFE_SELFORM	(1 << 10)
> +#define OMAP343X_CONTROL_CSIRXFE_PWRDNZ		(1 << 12)
> +#define OMAP343X_CONTROL_CSIRXFE_RESET		(1 << 13)
> +
> +/* OMAP3630_CONTROL_CAMERA_PHY_CTRL */
> +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT	2
> +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT	0
> +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY		0x0
> +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_STROBE 0x1
> +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_CLOCK 0x2
> +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_GPI		0x3
> +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK		0x3
> +/* CCP2B: set to receive data from PHY2 instead of PHY1 */
> +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2	(1 << 4)
> +

As the registers addresses are declared in a platform header, do you think it 
would make sense to declare those there as well ? If not I'm fine with keeping 
them here.

>  #endif	/* OMAP3_ISP_REG_H */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 3/3] omap3isp: Configure CSI-2 phy based on platform data
  2012-10-10 23:34   ` Laurent Pinchart
@ 2012-10-13 11:14     ` Sakari Ailus
  0 siblings, 0 replies; 8+ messages in thread
From: Sakari Ailus @ 2012-10-13 11:14 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media, linux-omap, tony, khilman

On Thu, Oct 11, 2012 at 01:34:30AM +0200, Laurent Pinchart wrote:
> On Wednesday 10 October 2012 23:01:42 Sakari Ailus wrote:
> > Configure CSI-2 phy based on platform data in the ISP driver. For that, the
> > new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same
> > was configured from the board code.
> > 
> > This patch is dependent on "omap3: Provide means for changing CSI2 PHY
> > configuration".
> 
> Is it still ?

Not anymore. It depends on the previous patch (used to be for linux-omap
instead) so I'll remove that statement.

Regards,

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/3] omap3isp: Add PHY routing configuration
  2012-10-10 23:48   ` Laurent Pinchart
@ 2012-10-13 11:21     ` Sakari Ailus
  0 siblings, 0 replies; 8+ messages in thread
From: Sakari Ailus @ 2012-10-13 11:21 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media, linux-omap, tony, khilman

Hi Laurent,

On Thu, Oct 11, 2012 at 01:48:12AM +0200, Laurent Pinchart wrote:
> On Wednesday 10 October 2012 23:01:41 Sakari Ailus wrote:
> > Add PHY routing configuration for both 3430 and 3630. Also add register bit
> > definitions of CSIRXFE and CAMERA_PHY_CTRL registers on OMAP 3430 and 3630,
> > respectively.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> >  drivers/media/platform/omap3isp/ispcsiphy.c |   92 ++++++++++++++++++++++++
> >  drivers/media/platform/omap3isp/ispreg.h    |   22 +++++++
> >  2 files changed, 114 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
> > b/drivers/media/platform/omap3isp/ispcsiphy.c index 348f67e..12ae394 100644
> > --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> > +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> > @@ -32,6 +32,98 @@
> >  #include "ispreg.h"
> >  #include "ispcsiphy.h"
> > 
> > +static void csiphy_routing_cfg_3630(struct isp_csiphy *phy, u32 iface,
> > +				    bool ccp2_strobe)
> > +{
> > +	u32 reg = isp_reg_readl(
> > +		phy->isp, OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
> > +	u32 shift, mode;
> > +
> > +	switch (iface) {
> > +	case ISP_INTERFACE_CCP2B_PHY1:
> > +		reg &= ~OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2;
> > +		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT;
> > +		break;
> > +	case ISP_INTERFACE_CSI2C_PHY1:
> > +		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT;
> > +		mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY;
> > +		break;
> > +	case ISP_INTERFACE_CCP2B_PHY2:
> > +		reg |= OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2;
> > +		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT;
> > +		break;
> > +	case ISP_INTERFACE_CSI2A_PHY2:
> > +		shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT;
> > +		mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY;
> > +		break;
> > +	}
> > +
> > +	/* Select data/clock or data/strobe mode for CCP2 */
> > +	switch (iface) {
> > +	case ISP_INTERFACE_CCP2B_PHY1:
> > +	case ISP_INTERFACE_CCP2B_PHY2:
> > +		if (ccp2_strobe)
> > +			mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_STROBE;
> > +		else
> > +			mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_CLOCK;
> > +	}
> > +
> > +	reg &= ~(OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK << shift);
> > +	reg |= mode << shift;
> > +
> > +	isp_reg_writel(phy->isp, reg,
> > +		       OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
> > +}
> > +
> > +static void csiphy_routing_cfg_3430(struct isp_csiphy *phy, u32 iface, bool
> > on,
> > +				    bool ccp2_strobe)
> > +{
> > +	uint32_t csirxfe = OMAP343X_CONTROL_CSIRXFE_PWRDNZ
> > +		| OMAP343X_CONTROL_CSIRXFE_RESET;
> 
> Anything wrong with u32 ? :-)

Nothing really. uint32_t is a standard C99 type, and also allowed in the
kernel. I can use u32, too.

> (I would also align the | with the = but that's nitpicking)

Can do.

> > +
> > +	/* Nothing to configure here. */
> > +	if (iface == ISP_INTERFACE_CSI2A_PHY2)
> > +		return;
> > +
> > +	if (iface != ISP_INTERFACE_CCP2B_PHY1)
> > +		return;
> 
> Can't you get rid of the first check ?

Good point. I'll remove it.

> > +	if (!on) {
> > +		isp_reg_writel(phy->isp, 0,
> > +			       OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
> > +		return;
> > +	}
> > +
> > +	if (ccp2_strobe)
> > +		csirxfe |= OMAP343X_CONTROL_CSIRXFE_SELFORM;
> > +
> > +	isp_reg_writel(phy->isp, csirxfe,
> > +		       OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
> > +}
> > +
> > +/**
> > + * Configure OMAP 3 CSI PHY routing.
> > + *
> > + * Note that the underlying routing configuration registers are part
> > + * of the control (SCM) register space and part of the CORE power
> > + * domain on both 3430 and 3630, so they will not hold their contents
> > + * in off-mode.
> 
> Could you please add a sentence to explain why that's not an issue ?

This patch does not use the function for anything yet; I've kept if from the
separate from the next one that does for the reason it'd get quite big.
Should I add it to the next patch where the function is being used instead?

> > + * @phy: relevant phy device
> > + * @iface: ISP_INTERFACE_*
> > + * @on: power on or off
> > + * @ccp2_strobe: false: data/clock, true: data/strobe
> > + */
> > +static void csiphy_routing_cfg(struct isp_csiphy *phy, u32 iface, bool on,
> > +			       bool ccp2_strobe)
> > +{
> > +	if (phy->isp->mmio_base[OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL]
> > +	    && on)
> > +		return csiphy_routing_cfg_3630(phy, iface, ccp2_strobe);
> > +	if (phy->isp->mmio_base[OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE])
> > +		return csiphy_routing_cfg_3430(phy, iface, on, ccp2_strobe);
> > +}
> > +
> >  /*
> >   * csiphy_lanes_config - Configuration of CSIPHY lanes.
> >   *
> > diff --git a/drivers/media/platform/omap3isp/ispreg.h
> > b/drivers/media/platform/omap3isp/ispreg.h index e2c57f3..148108b 100644
> > --- a/drivers/media/platform/omap3isp/ispreg.h
> > +++ b/drivers/media/platform/omap3isp/ispreg.h
> > @@ -1583,4 +1583,26 @@
> >  #define ISPCSIPHY_REG2_CCP2_SYNC_PATTERN_MASK		\
> >  	(0x7fffff << ISPCSIPHY_REG2_CCP2_SYNC_PATTERN_SHIFT)
> > 
> > +/*
> > ---------------------------------------------------------------------------
> > -- + * CONTROL registers for CSI-2 phy routing
> > + */
> > +
> > +/* OMAP343X_CONTROL_CSIRXFE */
> > +#define OMAP343X_CONTROL_CSIRXFE_CSIB_INV	(1 << 7)
> > +#define OMAP343X_CONTROL_CSIRXFE_RESENABLE	(1 << 8)
> > +#define OMAP343X_CONTROL_CSIRXFE_SELFORM	(1 << 10)
> > +#define OMAP343X_CONTROL_CSIRXFE_PWRDNZ		(1 << 12)
> > +#define OMAP343X_CONTROL_CSIRXFE_RESET		(1 << 13)
> > +
> > +/* OMAP3630_CONTROL_CAMERA_PHY_CTRL */
> > +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT	2
> > +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT	0
> > +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY		0x0
> > +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_STROBE 0x1
> > +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_CLOCK 0x2
> > +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_GPI		0x3
> > +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK		0x3
> > +/* CCP2B: set to receive data from PHY2 instead of PHY1 */
> > +#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2	(1 << 4)
> > +
> 
> As the registers addresses are declared in a platform header, do you think it 
> would make sense to declare those there as well ? If not I'm fine with keeping 
> them here.

Do you mean control.h? The issue with that is that control.h is not located
in a directory where it could be conveniently included from. One more reason
to keep them here is that this way no-one else would accidentally start
using them.

Kind regards,

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-10-13 11:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-10 20:01 [PATCH v4 0/3] OMAP 3 CSI-2 configuration Sakari Ailus
2012-10-10 20:01 ` [PATCH v4 1/3] omap3isp: Add CSI configuration registers from control block to ISP resources Sakari Ailus
2012-10-10 20:01 ` [PATCH v4 2/3] omap3isp: Add PHY routing configuration Sakari Ailus
2012-10-10 23:48   ` Laurent Pinchart
2012-10-13 11:21     ` Sakari Ailus
2012-10-10 20:01 ` [PATCH v4 3/3] omap3isp: Configure CSI-2 phy based on platform data Sakari Ailus
2012-10-10 23:34   ` Laurent Pinchart
2012-10-13 11:14     ` Sakari Ailus

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).