* [PATCH v2 0/5] Omap3isp CCP2 support
@ 2017-08-16 12:51 Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 1/5] omap3isp: Parse CSI1 configuration from the device tree Sakari Ailus
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Sakari Ailus @ 2017-08-16 12:51 UTC (permalink / raw)
To: linux-media; +Cc: pavel, laurent.pinchart
Hi folks,
Here's a respin of the omap3isp ccp2 support patches.
since v1:
- Root out patches already merged or not needed (omapisp: Ignore endpoints
with invalid configuration).
- Rework the patch skipping CSI-2 receiver configuration for CCP2
("omap3isp: Skip CSI-2 receiver initialisation in CCP2 configuration").
Instead of trying to figure out which receiver (CSI-2 or CCP2) is used,
store the information to the isp_csiphy struct instead.
- Added a patch to correctly release a CSI-2 phy --- the PHY configuration
coming from DT was previously ignored.
Pavel Machek (2):
omap3isp: Parse CSI1 configuration from the device tree
omap3isp: Correctly set IO_OUT_SEL and VP_CLK_POL for CCP2 mode
Sakari Ailus (3):
omap3isp: Always initialise isp and mutex for csiphy1
omap3isp: csiphy: Don't assume the CSI receiver is a CSI2 module
omap3isp: Correctly configure routing in PHY release
drivers/media/platform/omap3isp/isp.c | 105 +++++++++++++++++++++-------
drivers/media/platform/omap3isp/ispccp2.c | 9 ++-
drivers/media/platform/omap3isp/ispcsi2.c | 4 +-
drivers/media/platform/omap3isp/ispcsiphy.c | 59 ++++++++--------
drivers/media/platform/omap3isp/ispcsiphy.h | 6 +-
drivers/media/platform/omap3isp/ispreg.h | 4 ++
drivers/media/platform/omap3isp/omap3isp.h | 1 +
7 files changed, 126 insertions(+), 62 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/5] omap3isp: Parse CSI1 configuration from the device tree
2017-08-16 12:51 [PATCH v2 0/5] Omap3isp CCP2 support Sakari Ailus
@ 2017-08-16 12:51 ` Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 2/5] omap3isp: Correctly set IO_OUT_SEL and VP_CLK_POL for CCP2 mode Sakari Ailus
` (3 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2017-08-16 12:51 UTC (permalink / raw)
To: linux-media; +Cc: pavel, laurent.pinchart
From: Pavel Machek <pavel@ucw.cz>
Add support for parsing CSI1 configuration.
Signed-off-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> # on Beagleboard-xM + MPT9P031
---
drivers/media/platform/omap3isp/isp.c | 105 +++++++++++++++++++++--------
drivers/media/platform/omap3isp/omap3isp.h | 1 +
2 files changed, 79 insertions(+), 27 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 79aff6b989a1..6cb1f0495804 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2018,6 +2018,7 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
struct v4l2_fwnode_endpoint vep;
unsigned int i;
int ret;
+ bool csi1 = false;
ret = v4l2_fwnode_endpoint_parse(fwnode, &vep);
if (ret)
@@ -2047,41 +2048,91 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
case ISP_OF_PHY_CSIPHY1:
case ISP_OF_PHY_CSIPHY2:
- /* FIXME: always assume CSI-2 for now. */
+ switch (vep.bus_type) {
+ case V4L2_MBUS_CCP2:
+ case V4L2_MBUS_CSI1:
+ dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
+ csi1 = true;
+ break;
+ case V4L2_MBUS_CSI2:
+ dev_dbg(dev, "CSI-2 configuration\n");
+ csi1 = false;
+ break;
+ default:
+ dev_err(dev, "unsupported bus type %u\n",
+ vep.bus_type);
+ return -EINVAL;
+ }
+
switch (vep.base.port) {
case ISP_OF_PHY_CSIPHY1:
- buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
+ if (csi1)
+ buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
+ else
+ buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
break;
case ISP_OF_PHY_CSIPHY2:
- buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
+ if (csi1)
+ buscfg->interface = ISP_INTERFACE_CCP2B_PHY2;
+ else
+ buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
break;
}
- buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
- buscfg->bus.csi2.lanecfg.clk.pol =
- vep.bus.mipi_csi2.lane_polarities[0];
- dev_dbg(dev, "clock lane polarity %u, pos %u\n",
- buscfg->bus.csi2.lanecfg.clk.pol,
- buscfg->bus.csi2.lanecfg.clk.pos);
-
- buscfg->bus.csi2.num_data_lanes =
- vep.bus.mipi_csi2.num_data_lanes;
-
- for (i = 0; i < buscfg->bus.csi2.num_data_lanes; i++) {
- buscfg->bus.csi2.lanecfg.data[i].pos =
- vep.bus.mipi_csi2.data_lanes[i];
- buscfg->bus.csi2.lanecfg.data[i].pol =
- vep.bus.mipi_csi2.lane_polarities[i + 1];
+ if (csi1) {
+ buscfg->bus.ccp2.lanecfg.clk.pos =
+ vep.bus.mipi_csi1.clock_lane;
+ buscfg->bus.ccp2.lanecfg.clk.pol =
+ vep.bus.mipi_csi1.lane_polarity[0];
+ dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+ buscfg->bus.ccp2.lanecfg.clk.pol,
+ buscfg->bus.ccp2.lanecfg.clk.pos);
+
+ buscfg->bus.ccp2.lanecfg.data[0].pos =
+ vep.bus.mipi_csi1.data_lane;
+ buscfg->bus.ccp2.lanecfg.data[0].pol =
+ vep.bus.mipi_csi1.lane_polarity[1];
+
dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
- buscfg->bus.csi2.lanecfg.data[i].pol,
- buscfg->bus.csi2.lanecfg.data[i].pos);
+ buscfg->bus.ccp2.lanecfg.data[0].pol,
+ buscfg->bus.ccp2.lanecfg.data[0].pos);
+
+ buscfg->bus.ccp2.strobe_clk_pol =
+ vep.bus.mipi_csi1.clock_inv;
+ buscfg->bus.ccp2.phy_layer = vep.bus.mipi_csi1.strobe;
+ buscfg->bus.ccp2.ccp2_mode =
+ vep.bus_type == V4L2_MBUS_CCP2;
+ buscfg->bus.ccp2.vp_clk_pol = 1;
+
+ buscfg->bus.ccp2.crc = 1;
+ } else {
+ buscfg->bus.csi2.lanecfg.clk.pos =
+ vep.bus.mipi_csi2.clock_lane;
+ buscfg->bus.csi2.lanecfg.clk.pol =
+ vep.bus.mipi_csi2.lane_polarities[0];
+ dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+ buscfg->bus.csi2.lanecfg.clk.pol,
+ buscfg->bus.csi2.lanecfg.clk.pos);
+
+ buscfg->bus.csi2.num_data_lanes =
+ vep.bus.mipi_csi2.num_data_lanes;
+
+ for (i = 0; i < buscfg->bus.csi2.num_data_lanes; i++) {
+ buscfg->bus.csi2.lanecfg.data[i].pos =
+ vep.bus.mipi_csi2.data_lanes[i];
+ buscfg->bus.csi2.lanecfg.data[i].pol =
+ vep.bus.mipi_csi2.lane_polarities[i + 1];
+ dev_dbg(dev,
+ "data lane %u polarity %u, pos %u\n", i,
+ buscfg->bus.csi2.lanecfg.data[i].pol,
+ buscfg->bus.csi2.lanecfg.data[i].pos);
+ }
+ /*
+ * FIXME: now we assume the CRC is always there.
+ * Implement a way to obtain this information from the
+ * sensor. Frame descriptors, perhaps?
+ */
+ buscfg->bus.csi2.crc = 1;
}
-
- /*
- * FIXME: now we assume the CRC is always there.
- * Implement a way to obtain this information from the
- * sensor. Frame descriptors, perhaps?
- */
- buscfg->bus.csi2.crc = 1;
break;
default:
diff --git a/drivers/media/platform/omap3isp/omap3isp.h b/drivers/media/platform/omap3isp/omap3isp.h
index dfd3cbe26ccd..9fb4d5bce004 100644
--- a/drivers/media/platform/omap3isp/omap3isp.h
+++ b/drivers/media/platform/omap3isp/omap3isp.h
@@ -110,6 +110,7 @@ struct isp_ccp2_cfg {
unsigned int ccp2_mode:1;
unsigned int phy_layer:1;
unsigned int vpclk_div:2;
+ unsigned int vp_clk_pol:1;
struct isp_csiphy_lanes_cfg lanecfg;
};
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/5] omap3isp: Correctly set IO_OUT_SEL and VP_CLK_POL for CCP2 mode
2017-08-16 12:51 [PATCH v2 0/5] Omap3isp CCP2 support Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 1/5] omap3isp: Parse CSI1 configuration from the device tree Sakari Ailus
@ 2017-08-16 12:51 ` Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 3/5] omap3isp: Always initialise isp and mutex for csiphy1 Sakari Ailus
` (2 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2017-08-16 12:51 UTC (permalink / raw)
To: linux-media; +Cc: pavel, laurent.pinchart
From: Pavel Machek <pavel@ucw.cz>
ISP CSI1 module needs all the bits correctly set to work.
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> # on Beagleboard-xM + MPT9P031
---
drivers/media/platform/omap3isp/ispccp2.c | 7 +++++--
drivers/media/platform/omap3isp/ispreg.h | 4 ++++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c
index 8b6f7d2e79a0..47210b102bcb 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -213,14 +213,17 @@ static int ccp2_phyif_config(struct isp_ccp2_device *ccp2,
struct isp_device *isp = to_isp_device(ccp2);
u32 val;
- /* CCP2B mode */
val = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCP2, ISPCCP2_CTRL) |
- ISPCCP2_CTRL_IO_OUT_SEL | ISPCCP2_CTRL_MODE;
+ ISPCCP2_CTRL_MODE;
/* Data/strobe physical layer */
BIT_SET(val, ISPCCP2_CTRL_PHY_SEL_SHIFT, ISPCCP2_CTRL_PHY_SEL_MASK,
buscfg->phy_layer);
+ BIT_SET(val, ISPCCP2_CTRL_IO_OUT_SEL_SHIFT,
+ ISPCCP2_CTRL_IO_OUT_SEL_MASK, buscfg->ccp2_mode);
BIT_SET(val, ISPCCP2_CTRL_INV_SHIFT, ISPCCP2_CTRL_INV_MASK,
buscfg->strobe_clk_pol);
+ BIT_SET(val, ISPCCP2_CTRL_VP_CLK_POL_SHIFT,
+ ISPCCP2_CTRL_VP_CLK_POL_MASK, buscfg->vp_clk_pol);
isp_reg_writel(isp, val, OMAP3_ISP_IOMEM_CCP2, ISPCCP2_CTRL);
val = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCP2, ISPCCP2_CTRL);
diff --git a/drivers/media/platform/omap3isp/ispreg.h b/drivers/media/platform/omap3isp/ispreg.h
index b5ea8da0b904..d08483919a77 100644
--- a/drivers/media/platform/omap3isp/ispreg.h
+++ b/drivers/media/platform/omap3isp/ispreg.h
@@ -87,6 +87,8 @@
#define ISPCCP2_CTRL_PHY_SEL_MASK 0x1
#define ISPCCP2_CTRL_PHY_SEL_SHIFT 1
#define ISPCCP2_CTRL_IO_OUT_SEL (1 << 2)
+#define ISPCCP2_CTRL_IO_OUT_SEL_MASK 0x1
+#define ISPCCP2_CTRL_IO_OUT_SEL_SHIFT 2
#define ISPCCP2_CTRL_MODE (1 << 4)
#define ISPCCP2_CTRL_VP_CLK_FORCE_ON (1 << 9)
#define ISPCCP2_CTRL_INV (1 << 10)
@@ -94,6 +96,8 @@
#define ISPCCP2_CTRL_INV_SHIFT 10
#define ISPCCP2_CTRL_VP_ONLY_EN (1 << 11)
#define ISPCCP2_CTRL_VP_CLK_POL (1 << 12)
+#define ISPCCP2_CTRL_VP_CLK_POL_MASK 0x1
+#define ISPCCP2_CTRL_VP_CLK_POL_SHIFT 12
#define ISPCCP2_CTRL_VPCLK_DIV_SHIFT 15
#define ISPCCP2_CTRL_VPCLK_DIV_MASK 0x1ffff /* [31:15] */
#define ISPCCP2_CTRL_VP_OUT_CTRL_SHIFT 8 /* 3430 bits */
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/5] omap3isp: Always initialise isp and mutex for csiphy1
2017-08-16 12:51 [PATCH v2 0/5] Omap3isp CCP2 support Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 1/5] omap3isp: Parse CSI1 configuration from the device tree Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 2/5] omap3isp: Correctly set IO_OUT_SEL and VP_CLK_POL for CCP2 mode Sakari Ailus
@ 2017-08-16 12:51 ` Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 4/5] omap3isp: csiphy: Don't assume the CSI receiver is a CSI2 module Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 5/5] omap3isp: Correctly configure routing in PHY release Sakari Ailus
4 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2017-08-16 12:51 UTC (permalink / raw)
To: linux-media; +Cc: pavel, laurent.pinchart
The PHY is still relevant for CCP2.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> # on Beagleboard-xM + MPT9P031
---
drivers/media/platform/omap3isp/ispcsiphy.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index addc6efbb033..2028bb519108 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -345,13 +345,14 @@ int omap3isp_csiphy_init(struct isp_device *isp)
phy2->phy_regs = OMAP3_ISP_IOMEM_CSIPHY2;
mutex_init(&phy2->mutex);
+ phy1->isp = isp;
+ mutex_init(&phy1->mutex);
+
if (isp->revision == ISP_REVISION_15_0) {
- phy1->isp = isp;
phy1->csi2 = &isp->isp_csi2c;
phy1->num_data_lanes = ISP_CSIPHY1_NUM_DATA_LANES;
phy1->cfg_regs = OMAP3_ISP_IOMEM_CSI2C_REGS1;
phy1->phy_regs = OMAP3_ISP_IOMEM_CSIPHY1;
- mutex_init(&phy1->mutex);
}
return 0;
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 4/5] omap3isp: csiphy: Don't assume the CSI receiver is a CSI2 module
2017-08-16 12:51 [PATCH v2 0/5] Omap3isp CCP2 support Sakari Ailus
` (2 preceding siblings ...)
2017-08-16 12:51 ` [PATCH v2 3/5] omap3isp: Always initialise isp and mutex for csiphy1 Sakari Ailus
@ 2017-08-16 12:51 ` Sakari Ailus
2017-08-16 12:54 ` Laurent Pinchart
2017-08-16 12:51 ` [PATCH v2 5/5] omap3isp: Correctly configure routing in PHY release Sakari Ailus
4 siblings, 1 reply; 16+ messages in thread
From: Sakari Ailus @ 2017-08-16 12:51 UTC (permalink / raw)
To: linux-media; +Cc: pavel, laurent.pinchart
The CSI PHY is associated with a CSI receiver. The code assumes this
receiver is a CSI2 module and relies on the CSI2 module object heavily to
access the ISP or pipeline objects. However, the receiver could also be a
CSI1/CCP2 module.
Pass a new CSI receiver entity pointer to the CSI PHY acquire function, and
replace all hardcoded usage of the CSI2 module with that CSI receiver
entity.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> # on Beagleboard-xM + MPT9P031
Acked-by: Pavel Machek <pavel@ucw.cz>
---
drivers/media/platform/omap3isp/ispccp2.c | 2 +-
drivers/media/platform/omap3isp/ispcsi2.c | 4 +--
drivers/media/platform/omap3isp/ispcsiphy.c | 45 +++++++++++++----------------
drivers/media/platform/omap3isp/ispcsiphy.h | 6 ++--
4 files changed, 27 insertions(+), 30 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c
index 47210b102bcb..3db8df09cd9a 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -841,7 +841,7 @@ static int ccp2_s_stream(struct v4l2_subdev *sd, int enable)
switch (enable) {
case ISP_PIPELINE_STREAM_CONTINUOUS:
if (ccp2->phy) {
- ret = omap3isp_csiphy_acquire(ccp2->phy);
+ ret = omap3isp_csiphy_acquire(ccp2->phy, &sd->entity);
if (ret < 0)
return ret;
}
diff --git a/drivers/media/platform/omap3isp/ispcsi2.c b/drivers/media/platform/omap3isp/ispcsi2.c
index 7dae2fe0d42d..3ec37fed710b 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -490,7 +490,7 @@ int omap3isp_csi2_reset(struct isp_csi2_device *csi2)
if (!csi2->available)
return -ENODEV;
- if (csi2->phy->phy_in_use)
+ if (csi2->phy->entity)
return -EBUSY;
isp_reg_set(isp, csi2->regs1, ISPCSI2_SYSCONFIG,
@@ -1053,7 +1053,7 @@ static int csi2_set_stream(struct v4l2_subdev *sd, int enable)
switch (enable) {
case ISP_PIPELINE_STREAM_CONTINUOUS:
- if (omap3isp_csiphy_acquire(csi2->phy) < 0)
+ if (omap3isp_csiphy_acquire(csi2->phy, &sd->entity) < 0)
return -ENODEV;
if (csi2->output & CSI2_OUTPUT_MEMORY)
omap3isp_sbl_enable(isp, OMAP3_ISP_SBL_CSI2A_WRITE);
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index 2028bb519108..ed1eb9907ae0 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -164,22 +164,17 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 power)
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_bus_cfg *buscfg = pipe->external->host_priv;
+ struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
+ struct isp_async_subdev *isd =
+ container_of(pipe->external->asd, struct isp_async_subdev, asd);
+ struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
+ pipe->external->host_priv : &isd->bus;
struct isp_csiphy_lanes_cfg *lanes;
int csi2_ddrclk_khz;
unsigned int num_data_lanes, used_lanes = 0;
unsigned int i;
u32 reg;
- if (!buscfg) {
- struct isp_async_subdev *isd =
- container_of(pipe->external->asd,
- struct isp_async_subdev, asd);
- buscfg = &isd->bus;
- }
-
if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1
|| buscfg->interface == ISP_INTERFACE_CCP2B_PHY2) {
lanes = &buscfg->bus.ccp2.lanecfg;
@@ -222,7 +217,7 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
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 = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG0);
reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK |
ISPCSIPHY_REG0_THS_SETTLE_MASK);
@@ -233,9 +228,9 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
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);
+ isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG0);
- reg = isp_reg_readl(csi2->isp, phy->phy_regs, ISPCSIPHY_REG1);
+ reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG1);
reg &= ~(ISPCSIPHY_REG1_TCLK_TERM_MASK |
ISPCSIPHY_REG1_TCLK_MISS_MASK |
@@ -244,10 +239,10 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
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);
+ isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG1);
/* DPHY lane configuration */
- reg = isp_reg_readl(csi2->isp, phy->cfg_regs, ISPCSI2_PHY_CFG);
+ reg = isp_reg_readl(phy->isp, phy->cfg_regs, ISPCSI2_PHY_CFG);
for (i = 0; i < num_data_lanes; i++) {
reg &= ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) |
@@ -263,12 +258,12 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
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);
+ isp_reg_writel(phy->isp, reg, phy->cfg_regs, ISPCSI2_PHY_CFG);
return 0;
}
-int omap3isp_csiphy_acquire(struct isp_csiphy *phy)
+int omap3isp_csiphy_acquire(struct isp_csiphy *phy, struct media_entity *entity)
{
int rval;
@@ -288,6 +283,8 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy)
if (rval < 0)
goto done;
+ phy->entity = entity;
+
rval = omap3isp_csiphy_config(phy);
if (rval < 0)
goto done;
@@ -301,10 +298,10 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy)
csiphy_power_autoswitch_enable(phy, true);
}
-
- phy->phy_in_use = 1;
-
done:
+ if (rval < 0)
+ phy->entity = NULL;
+
mutex_unlock(&phy->mutex);
return rval;
}
@@ -312,10 +309,8 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy)
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);
+ if (phy->entity) {
+ struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
struct isp_bus_cfg *buscfg = pipe->external->host_priv;
csiphy_routing_cfg(phy, buscfg->interface, false,
@@ -325,7 +320,7 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_OFF);
}
regulator_disable(phy->vdd);
- phy->phy_in_use = 0;
+ phy->entity = NULL;
}
mutex_unlock(&phy->mutex);
}
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.h b/drivers/media/platform/omap3isp/ispcsiphy.h
index 978ca5c80a6c..91543a09b28a 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.h
+++ b/drivers/media/platform/omap3isp/ispcsiphy.h
@@ -25,9 +25,10 @@ struct regulator;
struct isp_csiphy {
struct isp_device *isp;
struct mutex mutex; /* serialize csiphy configuration */
- u8 phy_in_use;
struct isp_csi2_device *csi2;
struct regulator *vdd;
+ /* the entity that acquired the phy */
+ struct media_entity *entity;
/* mem resources - enums as defined in enum isp_mem_resources */
unsigned int cfg_regs;
@@ -36,7 +37,8 @@ struct isp_csiphy {
u8 num_data_lanes; /* number of CSI2 Data Lanes supported */
};
-int omap3isp_csiphy_acquire(struct isp_csiphy *phy);
+int omap3isp_csiphy_acquire(struct isp_csiphy *phy,
+ struct media_entity *entity);
void omap3isp_csiphy_release(struct isp_csiphy *phy);
int omap3isp_csiphy_init(struct isp_device *isp);
void omap3isp_csiphy_cleanup(struct isp_device *isp);
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 5/5] omap3isp: Correctly configure routing in PHY release
2017-08-16 12:51 [PATCH v2 0/5] Omap3isp CCP2 support Sakari Ailus
` (3 preceding siblings ...)
2017-08-16 12:51 ` [PATCH v2 4/5] omap3isp: csiphy: Don't assume the CSI receiver is a CSI2 module Sakari Ailus
@ 2017-08-16 12:51 ` Sakari Ailus
2017-08-16 16:52 ` [PATCH v2.1 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field Sakari Ailus
4 siblings, 1 reply; 16+ messages in thread
From: Sakari Ailus @ 2017-08-16 12:51 UTC (permalink / raw)
To: linux-media; +Cc: pavel, laurent.pinchart
The PHY configuration was obtained from DT when the PHY was acquired but
the same was not done when it was released. Fix this.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/platform/omap3isp/ispcsiphy.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index ed1eb9907ae0..45ed1adbd9ae 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -155,6 +155,17 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 power)
return 0;
}
+static struct isp_bus_cfg *omap3isp_csiphy_get_phy_cfg(
+ struct isp_csiphy *phy)
+{
+ struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
+ struct isp_async_subdev *isd =
+ container_of(pipe->external->asd, struct isp_async_subdev, asd);
+
+ return pipe->external->host_priv ?
+ pipe->external->host_priv : &isd->bus;
+}
+
/*
* TCLK values are OK at their reset values
*/
@@ -165,10 +176,7 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 power)
static int omap3isp_csiphy_config(struct isp_csiphy *phy)
{
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_async_subdev *isd =
- container_of(pipe->external->asd, struct isp_async_subdev, asd);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
- pipe->external->host_priv : &isd->bus;
+ struct isp_bus_cfg *buscfg = omap3isp_csiphy_get_phy_cfg(phy);
struct isp_csiphy_lanes_cfg *lanes;
int csi2_ddrclk_khz;
unsigned int num_data_lanes, used_lanes = 0;
@@ -310,8 +318,7 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
{
mutex_lock(&phy->mutex);
if (phy->entity) {
- struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv;
+ struct isp_bus_cfg *buscfg = omap3isp_csiphy_get_phy_cfg(phy);
csiphy_routing_cfg(phy, buscfg->interface, false,
buscfg->bus.ccp2.phy_layer);
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 4/5] omap3isp: csiphy: Don't assume the CSI receiver is a CSI2 module
2017-08-16 12:51 ` [PATCH v2 4/5] omap3isp: csiphy: Don't assume the CSI receiver is a CSI2 module Sakari Ailus
@ 2017-08-16 12:54 ` Laurent Pinchart
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2017-08-16 12:54 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, pavel
Hi Sakari,
Thank you for the patch.
On Wednesday 16 Aug 2017 15:51:49 Sakari Ailus wrote:
> The CSI PHY is associated with a CSI receiver. The code assumes this
> receiver is a CSI2 module and relies on the CSI2 module object heavily to
> access the ISP or pipeline objects. However, the receiver could also be a
> CSI1/CCP2 module.
>
> Pass a new CSI receiver entity pointer to the CSI PHY acquire function, and
> replace all hardcoded usage of the CSI2 module with that CSI receiver
> entity.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> # on
> Beagleboard-xM + MPT9P031
> Acked-by: Pavel Machek <pavel@ucw.cz>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/omap3isp/ispccp2.c | 2 +-
> drivers/media/platform/omap3isp/ispcsi2.c | 4 +--
> drivers/media/platform/omap3isp/ispcsiphy.c | 45 +++++++++++--------------
> drivers/media/platform/omap3isp/ispcsiphy.h | 6 ++--
> 4 files changed, 27 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispccp2.c
> b/drivers/media/platform/omap3isp/ispccp2.c index
> 47210b102bcb..3db8df09cd9a 100644
> --- a/drivers/media/platform/omap3isp/ispccp2.c
> +++ b/drivers/media/platform/omap3isp/ispccp2.c
> @@ -841,7 +841,7 @@ static int ccp2_s_stream(struct v4l2_subdev *sd, int
> enable) switch (enable) {
> case ISP_PIPELINE_STREAM_CONTINUOUS:
> if (ccp2->phy) {
> - ret = omap3isp_csiphy_acquire(ccp2->phy);
> + ret = omap3isp_csiphy_acquire(ccp2->phy, &sd->entity);
> if (ret < 0)
> return ret;
> }
> diff --git a/drivers/media/platform/omap3isp/ispcsi2.c
> b/drivers/media/platform/omap3isp/ispcsi2.c index
> 7dae2fe0d42d..3ec37fed710b 100644
> --- a/drivers/media/platform/omap3isp/ispcsi2.c
> +++ b/drivers/media/platform/omap3isp/ispcsi2.c
> @@ -490,7 +490,7 @@ int omap3isp_csi2_reset(struct isp_csi2_device *csi2)
> if (!csi2->available)
> return -ENODEV;
>
> - if (csi2->phy->phy_in_use)
> + if (csi2->phy->entity)
> return -EBUSY;
>
> isp_reg_set(isp, csi2->regs1, ISPCSI2_SYSCONFIG,
> @@ -1053,7 +1053,7 @@ static int csi2_set_stream(struct v4l2_subdev *sd, int
> enable)
>
> switch (enable) {
> case ISP_PIPELINE_STREAM_CONTINUOUS:
> - if (omap3isp_csiphy_acquire(csi2->phy) < 0)
> + if (omap3isp_csiphy_acquire(csi2->phy, &sd->entity) < 0)
> return -ENODEV;
> if (csi2->output & CSI2_OUTPUT_MEMORY)
> omap3isp_sbl_enable(isp, OMAP3_ISP_SBL_CSI2A_WRITE);
> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
> b/drivers/media/platform/omap3isp/ispcsiphy.c index
> 2028bb519108..ed1eb9907ae0 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> @@ -164,22 +164,17 @@ static int csiphy_set_power(struct isp_csiphy *phy,
> u32 power)
>
> 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_bus_cfg *buscfg = pipe->external->host_priv;
> + struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
> + struct isp_async_subdev *isd =
> + container_of(pipe->external->asd, struct isp_async_subdev,
asd);
> + struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
> + pipe->external->host_priv : &isd->bus;
> struct isp_csiphy_lanes_cfg *lanes;
> int csi2_ddrclk_khz;
> unsigned int num_data_lanes, used_lanes = 0;
> unsigned int i;
> u32 reg;
>
> - if (!buscfg) {
> - struct isp_async_subdev *isd =
> - container_of(pipe->external->asd,
> - struct isp_async_subdev, asd);
> - buscfg = &isd->bus;
> - }
> -
> if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1
>
> || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2) {
>
> lanes = &buscfg->bus.ccp2.lanecfg;
> @@ -222,7 +217,7 @@ static int omap3isp_csiphy_config(struct isp_csiphy
> *phy) 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 = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG0);
>
> reg &= ~(ISPCSIPHY_REG0_THS_TERM_MASK |
> ISPCSIPHY_REG0_THS_SETTLE_MASK);
> @@ -233,9 +228,9 @@ static int omap3isp_csiphy_config(struct isp_csiphy
> *phy) 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);
> + isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG0);
>
> - reg = isp_reg_readl(csi2->isp, phy->phy_regs, ISPCSIPHY_REG1);
> + reg = isp_reg_readl(phy->isp, phy->phy_regs, ISPCSIPHY_REG1);
>
> reg &= ~(ISPCSIPHY_REG1_TCLK_TERM_MASK |
> ISPCSIPHY_REG1_TCLK_MISS_MASK |
> @@ -244,10 +239,10 @@ static int omap3isp_csiphy_config(struct isp_csiphy
> *phy) 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);
> + isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG1);
>
> /* DPHY lane configuration */
> - reg = isp_reg_readl(csi2->isp, phy->cfg_regs, ISPCSI2_PHY_CFG);
> + reg = isp_reg_readl(phy->isp, phy->cfg_regs, ISPCSI2_PHY_CFG);
>
> for (i = 0; i < num_data_lanes; i++) {
> reg &= ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) |
> @@ -263,12 +258,12 @@ static int omap3isp_csiphy_config(struct isp_csiphy
> *phy) 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);
> + isp_reg_writel(phy->isp, reg, phy->cfg_regs, ISPCSI2_PHY_CFG);
>
> return 0;
> }
>
> -int omap3isp_csiphy_acquire(struct isp_csiphy *phy)
> +int omap3isp_csiphy_acquire(struct isp_csiphy *phy, struct media_entity
> *entity) {
> int rval;
>
> @@ -288,6 +283,8 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy)
> if (rval < 0)
> goto done;
>
> + phy->entity = entity;
> +
> rval = omap3isp_csiphy_config(phy);
> if (rval < 0)
> goto done;
> @@ -301,10 +298,10 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy)
>
> csiphy_power_autoswitch_enable(phy, true);
> }
> -
> - phy->phy_in_use = 1;
> -
> done:
> + if (rval < 0)
> + phy->entity = NULL;
> +
> mutex_unlock(&phy->mutex);
> return rval;
> }
> @@ -312,10 +309,8 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy)
> 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);
> + if (phy->entity) {
> + struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
> struct isp_bus_cfg *buscfg = pipe->external->host_priv;
>
> csiphy_routing_cfg(phy, buscfg->interface, false,
> @@ -325,7 +320,7 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
> csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_OFF);
> }
> regulator_disable(phy->vdd);
> - phy->phy_in_use = 0;
> + phy->entity = NULL;
> }
> mutex_unlock(&phy->mutex);
> }
> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.h
> b/drivers/media/platform/omap3isp/ispcsiphy.h index
> 978ca5c80a6c..91543a09b28a 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.h
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.h
> @@ -25,9 +25,10 @@ struct regulator;
> struct isp_csiphy {
> struct isp_device *isp;
> struct mutex mutex; /* serialize csiphy configuration */
> - u8 phy_in_use;
> struct isp_csi2_device *csi2;
> struct regulator *vdd;
> + /* the entity that acquired the phy */
> + struct media_entity *entity;
>
> /* mem resources - enums as defined in enum isp_mem_resources */
> unsigned int cfg_regs;
> @@ -36,7 +37,8 @@ struct isp_csiphy {
> u8 num_data_lanes; /* number of CSI2 Data Lanes supported */
> };
>
> -int omap3isp_csiphy_acquire(struct isp_csiphy *phy);
> +int omap3isp_csiphy_acquire(struct isp_csiphy *phy,
> + struct media_entity *entity);
> void omap3isp_csiphy_release(struct isp_csiphy *phy);
> int omap3isp_csiphy_init(struct isp_device *isp);
> void omap3isp_csiphy_cleanup(struct isp_device *isp);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2.1 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
2017-08-16 12:51 ` [PATCH v2 5/5] omap3isp: Correctly configure routing in PHY release Sakari Ailus
@ 2017-08-16 16:52 ` Sakari Ailus
2017-08-16 17:05 ` [PATCH v2.2 " Sakari Ailus
0 siblings, 1 reply; 16+ messages in thread
From: Sakari Ailus @ 2017-08-16 16:52 UTC (permalink / raw)
To: linux-media; +Cc: pavel, laurent.pinchart
struct v4l2_subdev.host_priv is intended to be used by another driver. This
is hardly good design but back in the days of platform data was a quick
hack to get things done.
As the sub-device specific bus information can be stored to the ISP driver
specific struct allocated along with v4l2_async_subdev, keep the
information there and only there.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
This patch replaces patch "omap3isp: Correctly configure routing in PHY
release".
drivers/media/platform/omap3isp/isp.c | 31 +++++++++--------------------
drivers/media/platform/omap3isp/ispccdc.c | 20 ++++++++++++-------
drivers/media/platform/omap3isp/ispccp2.c | 4 +++-
drivers/media/platform/omap3isp/ispcsi2.c | 3 ++-
drivers/media/platform/omap3isp/ispcsiphy.c | 11 +++++-----
5 files changed, 33 insertions(+), 36 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..27c577fac8e9 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
return -EINVAL;
}
-static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
- struct v4l2_subdev *subdev,
- struct v4l2_async_subdev *asd)
-{
- struct isp_async_subdev *isd =
- container_of(asd, struct isp_async_subdev, asd);
-
- isd->sd = subdev;
- isd->sd->host_priv = &isd->bus;
-
- return 0;
-}
-
static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
{
struct isp_device *isp = container_of(async, struct isp_device,
notifier);
struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
struct v4l2_subdev *sd;
- struct isp_bus_cfg *bus;
int ret;
ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
@@ -2215,13 +2201,15 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
return ret;
list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
- /* Only try to link entities whose interface was set on bound */
- if (sd->host_priv) {
- bus = (struct isp_bus_cfg *)sd->host_priv;
- ret = isp_link_entity(isp, &sd->entity, bus->interface);
- if (ret < 0)
- return ret;
- }
+ struct isp_async_subdev *isd;
+
+ if (!sd->asd)
+ continue;
+
+ isd = container_of(sd->asd, struct isp_async_subdev, asd);
+ ret = isp_link_entity(isp, &sd->entity, isd->bus.interface);
+ if (ret < 0)
+ return ret;
}
ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
@@ -2399,7 +2387,6 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
- isp->notifier.bound = isp_subdev_notifier_bound;
isp->notifier.complete = isp_subdev_notifier_complete;
ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
diff --git a/drivers/media/platform/omap3isp/ispccdc.c b/drivers/media/platform/omap3isp/ispccdc.c
index 4947876cfadf..0145b3dcd7a7 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1139,8 +1139,12 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
- ->bus.parallel;
+ struct isp_pipeline *pipe =
+ to_isp_pipeline(&ccdc->subdev.entity);
+
+ parcfg = &container_of(pipe->external->asd,
+ struct isp_async_subdev,
+ asd)->bus.bus.parallel;
ccdc->bt656 = parcfg->bt656;
}
@@ -2412,11 +2416,13 @@ static int ccdc_link_validate(struct v4l2_subdev *sd,
/* We've got a parallel sensor here. */
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- struct isp_parallel_cfg *parcfg =
- &((struct isp_bus_cfg *)
- media_entity_to_v4l2_subdev(link->source->entity)
- ->host_priv)->bus.parallel;
- parallel_shift = parcfg->data_lane_shift;
+ struct isp_pipeline *pipe =
+ to_isp_pipeline(&ccdc->subdev.entity);
+
+ parallel_shift =
+ container_of(pipe->external->asd,
+ struct isp_async_subdev,
+ asd)->bus.bus.parallel.data_lane_shift;
} else {
parallel_shift = 0;
}
diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c
index 3db8df09cd9a..8561c4e4c5ac 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -350,6 +350,7 @@ static void ccp2_lcx_config(struct isp_ccp2_device *ccp2,
*/
static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
{
+ struct isp_pipeline *pipe = to_isp_pipeline(&ccp2->subdev.entity);
const struct isp_bus_cfg *buscfg;
struct v4l2_mbus_framefmt *format;
struct media_pad *pad;
@@ -361,7 +362,8 @@ static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
pad = media_entity_remote_pad(&ccp2->pads[CCP2_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
- buscfg = sensor->host_priv;
+ buscfg = &container_of(pipe->external->asd,
+ struct isp_async_subdev, asd)->bus;
ret = ccp2_phyif_config(ccp2, &buscfg->bus.ccp2);
if (ret < 0)
diff --git a/drivers/media/platform/omap3isp/ispcsi2.c b/drivers/media/platform/omap3isp/ispcsi2.c
index 3ec37fed710b..2802f70fc8b3 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -566,7 +566,8 @@ static int csi2_configure(struct isp_csi2_device *csi2)
pad = media_entity_remote_pad(&csi2->pads[CSI2_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
- buscfg = sensor->host_priv;
+ buscfg = &container_of(pipe->external->asd,
+ struct isp_async_subdev, asd)->bus;
csi2->frame_skip = 0;
v4l2_subdev_call(sensor, sensor, g_skip_frames, &csi2->frame_skip);
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index ed1eb9907ae0..ef79bf37c2dd 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -165,10 +165,9 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 power)
static int omap3isp_csiphy_config(struct isp_csiphy *phy)
{
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_async_subdev *isd =
- container_of(pipe->external->asd, struct isp_async_subdev, asd);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
- pipe->external->host_priv : &isd->bus;
+ struct isp_bus_cfg *buscfg =
+ &container_of(pipe->external->asd,
+ struct isp_async_subdev, asd)->bus;
struct isp_csiphy_lanes_cfg *lanes;
int csi2_ddrclk_khz;
unsigned int num_data_lanes, used_lanes = 0;
@@ -311,7 +310,9 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
mutex_lock(&phy->mutex);
if (phy->entity) {
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv;
+ struct isp_bus_cfg *buscfg =
+ &container_of(pipe->external->asd,
+ struct isp_async_subdev, asd)->bus;
csiphy_routing_cfg(phy, buscfg->interface, false,
buscfg->bus.ccp2.phy_layer);
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2.2 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
2017-08-16 16:52 ` [PATCH v2.1 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field Sakari Ailus
@ 2017-08-16 17:05 ` Sakari Ailus
2017-08-16 17:24 ` Laurent Pinchart
0 siblings, 1 reply; 16+ messages in thread
From: Sakari Ailus @ 2017-08-16 17:05 UTC (permalink / raw)
To: linux-media; +Cc: pavel, laurent.pinchart
struct v4l2_subdev.host_priv is intended to be used by another driver. This
is hardly good design but back in the days of platform data was a quick
hack to get things done.
As the sub-device specific bus information can be stored to the ISP driver
specific struct allocated along with v4l2_async_subdev, keep the
information there and only there.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v2.1:
- Remove struct isp_async_subdev.sd field as it is now redundant.
drivers/media/platform/omap3isp/isp.c | 31 +++++++++--------------------
drivers/media/platform/omap3isp/isp.h | 1 -
drivers/media/platform/omap3isp/ispccdc.c | 20 ++++++++++++-------
drivers/media/platform/omap3isp/ispccp2.c | 4 +++-
drivers/media/platform/omap3isp/ispcsi2.c | 3 ++-
drivers/media/platform/omap3isp/ispcsiphy.c | 11 +++++-----
6 files changed, 33 insertions(+), 37 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..27c577fac8e9 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
return -EINVAL;
}
-static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
- struct v4l2_subdev *subdev,
- struct v4l2_async_subdev *asd)
-{
- struct isp_async_subdev *isd =
- container_of(asd, struct isp_async_subdev, asd);
-
- isd->sd = subdev;
- isd->sd->host_priv = &isd->bus;
-
- return 0;
-}
-
static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
{
struct isp_device *isp = container_of(async, struct isp_device,
notifier);
struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
struct v4l2_subdev *sd;
- struct isp_bus_cfg *bus;
int ret;
ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
@@ -2215,13 +2201,15 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
return ret;
list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
- /* Only try to link entities whose interface was set on bound */
- if (sd->host_priv) {
- bus = (struct isp_bus_cfg *)sd->host_priv;
- ret = isp_link_entity(isp, &sd->entity, bus->interface);
- if (ret < 0)
- return ret;
- }
+ struct isp_async_subdev *isd;
+
+ if (!sd->asd)
+ continue;
+
+ isd = container_of(sd->asd, struct isp_async_subdev, asd);
+ ret = isp_link_entity(isp, &sd->entity, isd->bus.interface);
+ if (ret < 0)
+ return ret;
}
ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
@@ -2399,7 +2387,6 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
- isp->notifier.bound = isp_subdev_notifier_bound;
isp->notifier.complete = isp_subdev_notifier_complete;
ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 2f2ae609c548..25472c81dcdd 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -226,7 +226,6 @@ struct isp_device {
};
struct isp_async_subdev {
- struct v4l2_subdev *sd;
struct isp_bus_cfg bus;
struct v4l2_async_subdev asd;
};
diff --git a/drivers/media/platform/omap3isp/ispccdc.c b/drivers/media/platform/omap3isp/ispccdc.c
index 4947876cfadf..0145b3dcd7a7 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1139,8 +1139,12 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
- ->bus.parallel;
+ struct isp_pipeline *pipe =
+ to_isp_pipeline(&ccdc->subdev.entity);
+
+ parcfg = &container_of(pipe->external->asd,
+ struct isp_async_subdev,
+ asd)->bus.bus.parallel;
ccdc->bt656 = parcfg->bt656;
}
@@ -2412,11 +2416,13 @@ static int ccdc_link_validate(struct v4l2_subdev *sd,
/* We've got a parallel sensor here. */
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- struct isp_parallel_cfg *parcfg =
- &((struct isp_bus_cfg *)
- media_entity_to_v4l2_subdev(link->source->entity)
- ->host_priv)->bus.parallel;
- parallel_shift = parcfg->data_lane_shift;
+ struct isp_pipeline *pipe =
+ to_isp_pipeline(&ccdc->subdev.entity);
+
+ parallel_shift =
+ container_of(pipe->external->asd,
+ struct isp_async_subdev,
+ asd)->bus.bus.parallel.data_lane_shift;
} else {
parallel_shift = 0;
}
diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c
index 3db8df09cd9a..8561c4e4c5ac 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -350,6 +350,7 @@ static void ccp2_lcx_config(struct isp_ccp2_device *ccp2,
*/
static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
{
+ struct isp_pipeline *pipe = to_isp_pipeline(&ccp2->subdev.entity);
const struct isp_bus_cfg *buscfg;
struct v4l2_mbus_framefmt *format;
struct media_pad *pad;
@@ -361,7 +362,8 @@ static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
pad = media_entity_remote_pad(&ccp2->pads[CCP2_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
- buscfg = sensor->host_priv;
+ buscfg = &container_of(pipe->external->asd,
+ struct isp_async_subdev, asd)->bus;
ret = ccp2_phyif_config(ccp2, &buscfg->bus.ccp2);
if (ret < 0)
diff --git a/drivers/media/platform/omap3isp/ispcsi2.c b/drivers/media/platform/omap3isp/ispcsi2.c
index 3ec37fed710b..2802f70fc8b3 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -566,7 +566,8 @@ static int csi2_configure(struct isp_csi2_device *csi2)
pad = media_entity_remote_pad(&csi2->pads[CSI2_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
- buscfg = sensor->host_priv;
+ buscfg = &container_of(pipe->external->asd,
+ struct isp_async_subdev, asd)->bus;
csi2->frame_skip = 0;
v4l2_subdev_call(sensor, sensor, g_skip_frames, &csi2->frame_skip);
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index ed1eb9907ae0..ef79bf37c2dd 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -165,10 +165,9 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 power)
static int omap3isp_csiphy_config(struct isp_csiphy *phy)
{
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_async_subdev *isd =
- container_of(pipe->external->asd, struct isp_async_subdev, asd);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
- pipe->external->host_priv : &isd->bus;
+ struct isp_bus_cfg *buscfg =
+ &container_of(pipe->external->asd,
+ struct isp_async_subdev, asd)->bus;
struct isp_csiphy_lanes_cfg *lanes;
int csi2_ddrclk_khz;
unsigned int num_data_lanes, used_lanes = 0;
@@ -311,7 +310,9 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
mutex_lock(&phy->mutex);
if (phy->entity) {
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv;
+ struct isp_bus_cfg *buscfg =
+ &container_of(pipe->external->asd,
+ struct isp_async_subdev, asd)->bus;
csiphy_routing_cfg(phy, buscfg->interface, false,
buscfg->bus.ccp2.phy_layer);
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2.2 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
2017-08-16 17:05 ` [PATCH v2.2 " Sakari Ailus
@ 2017-08-16 17:24 ` Laurent Pinchart
2017-08-16 17:26 ` Sakari Ailus
2017-08-16 17:39 ` [PATCH v2.3 " Sakari Ailus
0 siblings, 2 replies; 16+ messages in thread
From: Laurent Pinchart @ 2017-08-16 17:24 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, pavel
Hi Sakari,
Thank you for the patch.
On Wednesday 16 Aug 2017 20:05:52 Sakari Ailus wrote:
> struct v4l2_subdev.host_priv is intended to be used by another driver. This
> is hardly good design but back in the days of platform data was a quick
> hack to get things done.
>
> As the sub-device specific bus information can be stored to the ISP driver
> specific struct allocated along with v4l2_async_subdev, keep the
> information there and only there.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
This looks better to me than the previous approach. Please see below for one
last comment.
> ---
> since v2.1:
>
> - Remove struct isp_async_subdev.sd field as it is now redundant.
>
> drivers/media/platform/omap3isp/isp.c | 31 ++++++++------------------
> drivers/media/platform/omap3isp/isp.h | 1 -
> drivers/media/platform/omap3isp/ispccdc.c | 20 ++++++++++++-------
> drivers/media/platform/omap3isp/ispccp2.c | 4 +++-
> drivers/media/platform/omap3isp/ispcsi2.c | 3 ++-
> drivers/media/platform/omap3isp/ispcsiphy.c | 11 +++++-----
> 6 files changed, 33 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 6cb1f0495804..27c577fac8e9
> 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
> return -EINVAL;
> }
>
> -static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> - struct v4l2_subdev *subdev,
> - struct v4l2_async_subdev *asd)
> -{
> - struct isp_async_subdev *isd =
> - container_of(asd, struct isp_async_subdev, asd);
> -
> - isd->sd = subdev;
> - isd->sd->host_priv = &isd->bus;
> -
> - return 0;
> -}
> -
> static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
> {
> struct isp_device *isp = container_of(async, struct isp_device,
> notifier);
> struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
> struct v4l2_subdev *sd;
> - struct isp_bus_cfg *bus;
> int ret;
>
> ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
> @@ -2215,13 +2201,15 @@ static int isp_subdev_notifier_complete(struct
> v4l2_async_notifier *async) return ret;
>
> list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> - /* Only try to link entities whose interface was set on bound
*/
> - if (sd->host_priv) {
> - bus = (struct isp_bus_cfg *)sd->host_priv;
> - ret = isp_link_entity(isp, &sd->entity, bus-
>interface);
> - if (ret < 0)
> - return ret;
> - }
> + struct isp_async_subdev *isd;
> +
> + if (!sd->asd)
> + continue;
> +
> + isd = container_of(sd->asd, struct isp_async_subdev, asd);
> + ret = isp_link_entity(isp, &sd->entity, isd->bus.interface);
> + if (ret < 0)
> + return ret;
> }
>
> ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> @@ -2399,7 +2387,6 @@ static int isp_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_register_entities;
>
> - isp->notifier.bound = isp_subdev_notifier_bound;
> isp->notifier.complete = isp_subdev_notifier_complete;
>
> ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
> diff --git a/drivers/media/platform/omap3isp/isp.h
> b/drivers/media/platform/omap3isp/isp.h index 2f2ae609c548..25472c81dcdd
> 100644
> --- a/drivers/media/platform/omap3isp/isp.h
> +++ b/drivers/media/platform/omap3isp/isp.h
> @@ -226,7 +226,6 @@ struct isp_device {
> };
>
> struct isp_async_subdev {
> - struct v4l2_subdev *sd;
> struct isp_bus_cfg bus;
> struct v4l2_async_subdev asd;
> };
> diff --git a/drivers/media/platform/omap3isp/ispccdc.c
> b/drivers/media/platform/omap3isp/ispccdc.c index
> 4947876cfadf..0145b3dcd7a7 100644
> --- a/drivers/media/platform/omap3isp/ispccdc.c
> +++ b/drivers/media/platform/omap3isp/ispccdc.c
> @@ -1139,8 +1139,12 @@ static void ccdc_configure(struct isp_ccdc_device
> *ccdc) pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> if (ccdc->input == CCDC_INPUT_PARALLEL) {
> - parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
> - ->bus.parallel;
> + struct isp_pipeline *pipe =
> + to_isp_pipeline(&ccdc->subdev.entity);
> +
> + parcfg = &container_of(pipe->external->asd,
> + struct isp_async_subdev,
> + asd)->bus.bus.parallel;
You use this construct in many places, how about adding
struct isp_bus_cfg *to_bus_cfg(struct v4l2_subdev *sd)
{
return &container_of(sd->asd, struct isp_async_subdev, asd)->bus;
}
or possibly a macro to handle the const/non-const cases.
> ccdc->bt656 = parcfg->bt656;
> }
>
> @@ -2412,11 +2416,13 @@ static int ccdc_link_validate(struct v4l2_subdev
> *sd,
>
> /* We've got a parallel sensor here. */
> if (ccdc->input == CCDC_INPUT_PARALLEL) {
> - struct isp_parallel_cfg *parcfg =
> - &((struct isp_bus_cfg *)
> - media_entity_to_v4l2_subdev(link->source->entity)
> - ->host_priv)->bus.parallel;
> - parallel_shift = parcfg->data_lane_shift;
> + struct isp_pipeline *pipe =
> + to_isp_pipeline(&ccdc->subdev.entity);
> +
> + parallel_shift =
> + container_of(pipe->external->asd,
> + struct isp_async_subdev,
> + asd)->bus.bus.parallel.data_lane_shift;
> } else {
> parallel_shift = 0;
> }
> diff --git a/drivers/media/platform/omap3isp/ispccp2.c
> b/drivers/media/platform/omap3isp/ispccp2.c index
> 3db8df09cd9a..8561c4e4c5ac 100644
> --- a/drivers/media/platform/omap3isp/ispccp2.c
> +++ b/drivers/media/platform/omap3isp/ispccp2.c
> @@ -350,6 +350,7 @@ static void ccp2_lcx_config(struct isp_ccp2_device
> *ccp2, */
> static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
> {
> + struct isp_pipeline *pipe = to_isp_pipeline(&ccp2->subdev.entity);
> const struct isp_bus_cfg *buscfg;
> struct v4l2_mbus_framefmt *format;
> struct media_pad *pad;
> @@ -361,7 +362,8 @@ static int ccp2_if_configure(struct isp_ccp2_device
> *ccp2)
>
> pad = media_entity_remote_pad(&ccp2->pads[CCP2_PAD_SINK]);
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> - buscfg = sensor->host_priv;
> + buscfg = &container_of(pipe->external->asd,
> + struct isp_async_subdev, asd)->bus;
>
> ret = ccp2_phyif_config(ccp2, &buscfg->bus.ccp2);
> if (ret < 0)
> diff --git a/drivers/media/platform/omap3isp/ispcsi2.c
> b/drivers/media/platform/omap3isp/ispcsi2.c index
> 3ec37fed710b..2802f70fc8b3 100644
> --- a/drivers/media/platform/omap3isp/ispcsi2.c
> +++ b/drivers/media/platform/omap3isp/ispcsi2.c
> @@ -566,7 +566,8 @@ static int csi2_configure(struct isp_csi2_device *csi2)
>
> pad = media_entity_remote_pad(&csi2->pads[CSI2_PAD_SINK]);
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> - buscfg = sensor->host_priv;
> + buscfg = &container_of(pipe->external->asd,
> + struct isp_async_subdev, asd)->bus;
>
> csi2->frame_skip = 0;
> v4l2_subdev_call(sensor, sensor, g_skip_frames, &csi2->frame_skip);
> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
> b/drivers/media/platform/omap3isp/ispcsiphy.c index
> ed1eb9907ae0..ef79bf37c2dd 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> @@ -165,10 +165,9 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32
> power) static int omap3isp_csiphy_config(struct isp_csiphy *phy)
> {
> struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
> - struct isp_async_subdev *isd =
> - container_of(pipe->external->asd, struct isp_async_subdev,
asd);
> - struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
> - pipe->external->host_priv : &isd->bus;
> + struct isp_bus_cfg *buscfg =
> + &container_of(pipe->external->asd,
> + struct isp_async_subdev, asd)->bus;
> struct isp_csiphy_lanes_cfg *lanes;
> int csi2_ddrclk_khz;
> unsigned int num_data_lanes, used_lanes = 0;
> @@ -311,7 +310,9 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
> mutex_lock(&phy->mutex);
> if (phy->entity) {
> struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
> - struct isp_bus_cfg *buscfg = pipe->external->host_priv;
> + struct isp_bus_cfg *buscfg =
> + &container_of(pipe->external->asd,
> + struct isp_async_subdev, asd)->bus;
>
> csiphy_routing_cfg(phy, buscfg->interface, false,
> buscfg->bus.ccp2.phy_layer);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2.2 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
2017-08-16 17:24 ` Laurent Pinchart
@ 2017-08-16 17:26 ` Sakari Ailus
2017-08-16 17:39 ` [PATCH v2.3 " Sakari Ailus
1 sibling, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2017-08-16 17:26 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Sakari Ailus, linux-media, pavel
On Wed, Aug 16, 2017 at 08:24:19PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Wednesday 16 Aug 2017 20:05:52 Sakari Ailus wrote:
> > struct v4l2_subdev.host_priv is intended to be used by another driver. This
> > is hardly good design but back in the days of platform data was a quick
> > hack to get things done.
> >
> > As the sub-device specific bus information can be stored to the ISP driver
> > specific struct allocated along with v4l2_async_subdev, keep the
> > information there and only there.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> This looks better to me than the previous approach. Please see below for one
> last comment.
>
> > ---
> > since v2.1:
> >
> > - Remove struct isp_async_subdev.sd field as it is now redundant.
> >
> > drivers/media/platform/omap3isp/isp.c | 31 ++++++++------------------
> > drivers/media/platform/omap3isp/isp.h | 1 -
> > drivers/media/platform/omap3isp/ispccdc.c | 20 ++++++++++++-------
> > drivers/media/platform/omap3isp/ispccp2.c | 4 +++-
> > drivers/media/platform/omap3isp/ispcsi2.c | 3 ++-
> > drivers/media/platform/omap3isp/ispcsiphy.c | 11 +++++-----
> > 6 files changed, 33 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/media/platform/omap3isp/isp.c
> > b/drivers/media/platform/omap3isp/isp.c index 6cb1f0495804..27c577fac8e9
> > 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> > @@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
> > return -EINVAL;
> > }
> >
> > -static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> > - struct v4l2_subdev *subdev,
> > - struct v4l2_async_subdev *asd)
> > -{
> > - struct isp_async_subdev *isd =
> > - container_of(asd, struct isp_async_subdev, asd);
> > -
> > - isd->sd = subdev;
> > - isd->sd->host_priv = &isd->bus;
> > -
> > - return 0;
> > -}
> > -
> > static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
> > {
> > struct isp_device *isp = container_of(async, struct isp_device,
> > notifier);
> > struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
> > struct v4l2_subdev *sd;
> > - struct isp_bus_cfg *bus;
> > int ret;
> >
> > ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
> > @@ -2215,13 +2201,15 @@ static int isp_subdev_notifier_complete(struct
> > v4l2_async_notifier *async) return ret;
> >
> > list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> > - /* Only try to link entities whose interface was set on bound
> */
> > - if (sd->host_priv) {
> > - bus = (struct isp_bus_cfg *)sd->host_priv;
> > - ret = isp_link_entity(isp, &sd->entity, bus-
> >interface);
> > - if (ret < 0)
> > - return ret;
> > - }
> > + struct isp_async_subdev *isd;
> > +
> > + if (!sd->asd)
> > + continue;
> > +
> > + isd = container_of(sd->asd, struct isp_async_subdev, asd);
> > + ret = isp_link_entity(isp, &sd->entity, isd->bus.interface);
> > + if (ret < 0)
> > + return ret;
> > }
> >
> > ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> > @@ -2399,7 +2387,6 @@ static int isp_probe(struct platform_device *pdev)
> > if (ret < 0)
> > goto error_register_entities;
> >
> > - isp->notifier.bound = isp_subdev_notifier_bound;
> > isp->notifier.complete = isp_subdev_notifier_complete;
> >
> > ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
> > diff --git a/drivers/media/platform/omap3isp/isp.h
> > b/drivers/media/platform/omap3isp/isp.h index 2f2ae609c548..25472c81dcdd
> > 100644
> > --- a/drivers/media/platform/omap3isp/isp.h
> > +++ b/drivers/media/platform/omap3isp/isp.h
> > @@ -226,7 +226,6 @@ struct isp_device {
> > };
> >
> > struct isp_async_subdev {
> > - struct v4l2_subdev *sd;
> > struct isp_bus_cfg bus;
> > struct v4l2_async_subdev asd;
> > };
> > diff --git a/drivers/media/platform/omap3isp/ispccdc.c
> > b/drivers/media/platform/omap3isp/ispccdc.c index
> > 4947876cfadf..0145b3dcd7a7 100644
> > --- a/drivers/media/platform/omap3isp/ispccdc.c
> > +++ b/drivers/media/platform/omap3isp/ispccdc.c
> > @@ -1139,8 +1139,12 @@ static void ccdc_configure(struct isp_ccdc_device
> > *ccdc) pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
> > sensor = media_entity_to_v4l2_subdev(pad->entity);
> > if (ccdc->input == CCDC_INPUT_PARALLEL) {
> > - parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
> > - ->bus.parallel;
> > + struct isp_pipeline *pipe =
> > + to_isp_pipeline(&ccdc->subdev.entity);
> > +
> > + parcfg = &container_of(pipe->external->asd,
> > + struct isp_async_subdev,
> > + asd)->bus.bus.parallel;
>
> You use this construct in many places, how about adding
>
> struct isp_bus_cfg *to_bus_cfg(struct v4l2_subdev *sd)
> {
> return &container_of(sd->asd, struct isp_async_subdev, asd)->bus;
> }
>
> or possibly a macro to handle the const/non-const cases.
I'll add that and send v2.3.
>
> > ccdc->bt656 = parcfg->bt656;
> > }
> >
> > @@ -2412,11 +2416,13 @@ static int ccdc_link_validate(struct v4l2_subdev
> > *sd,
> >
> > /* We've got a parallel sensor here. */
> > if (ccdc->input == CCDC_INPUT_PARALLEL) {
> > - struct isp_parallel_cfg *parcfg =
> > - &((struct isp_bus_cfg *)
> > - media_entity_to_v4l2_subdev(link->source->entity)
> > - ->host_priv)->bus.parallel;
> > - parallel_shift = parcfg->data_lane_shift;
> > + struct isp_pipeline *pipe =
> > + to_isp_pipeline(&ccdc->subdev.entity);
> > +
> > + parallel_shift =
> > + container_of(pipe->external->asd,
> > + struct isp_async_subdev,
> > + asd)->bus.bus.parallel.data_lane_shift;
> > } else {
> > parallel_shift = 0;
> > }
> > diff --git a/drivers/media/platform/omap3isp/ispccp2.c
> > b/drivers/media/platform/omap3isp/ispccp2.c index
> > 3db8df09cd9a..8561c4e4c5ac 100644
> > --- a/drivers/media/platform/omap3isp/ispccp2.c
> > +++ b/drivers/media/platform/omap3isp/ispccp2.c
> > @@ -350,6 +350,7 @@ static void ccp2_lcx_config(struct isp_ccp2_device
> > *ccp2, */
> > static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
> > {
> > + struct isp_pipeline *pipe = to_isp_pipeline(&ccp2->subdev.entity);
> > const struct isp_bus_cfg *buscfg;
> > struct v4l2_mbus_framefmt *format;
> > struct media_pad *pad;
> > @@ -361,7 +362,8 @@ static int ccp2_if_configure(struct isp_ccp2_device
> > *ccp2)
> >
> > pad = media_entity_remote_pad(&ccp2->pads[CCP2_PAD_SINK]);
> > sensor = media_entity_to_v4l2_subdev(pad->entity);
> > - buscfg = sensor->host_priv;
> > + buscfg = &container_of(pipe->external->asd,
> > + struct isp_async_subdev, asd)->bus;
> >
> > ret = ccp2_phyif_config(ccp2, &buscfg->bus.ccp2);
> > if (ret < 0)
> > diff --git a/drivers/media/platform/omap3isp/ispcsi2.c
> > b/drivers/media/platform/omap3isp/ispcsi2.c index
> > 3ec37fed710b..2802f70fc8b3 100644
> > --- a/drivers/media/platform/omap3isp/ispcsi2.c
> > +++ b/drivers/media/platform/omap3isp/ispcsi2.c
> > @@ -566,7 +566,8 @@ static int csi2_configure(struct isp_csi2_device *csi2)
> >
> > pad = media_entity_remote_pad(&csi2->pads[CSI2_PAD_SINK]);
> > sensor = media_entity_to_v4l2_subdev(pad->entity);
> > - buscfg = sensor->host_priv;
> > + buscfg = &container_of(pipe->external->asd,
> > + struct isp_async_subdev, asd)->bus;
> >
> > csi2->frame_skip = 0;
> > v4l2_subdev_call(sensor, sensor, g_skip_frames, &csi2->frame_skip);
> > diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
> > b/drivers/media/platform/omap3isp/ispcsiphy.c index
> > ed1eb9907ae0..ef79bf37c2dd 100644
> > --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> > +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> > @@ -165,10 +165,9 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32
> > power) static int omap3isp_csiphy_config(struct isp_csiphy *phy)
> > {
> > struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
> > - struct isp_async_subdev *isd =
> > - container_of(pipe->external->asd, struct isp_async_subdev,
> asd);
> > - struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
> > - pipe->external->host_priv : &isd->bus;
> > + struct isp_bus_cfg *buscfg =
> > + &container_of(pipe->external->asd,
> > + struct isp_async_subdev, asd)->bus;
> > struct isp_csiphy_lanes_cfg *lanes;
> > int csi2_ddrclk_khz;
> > unsigned int num_data_lanes, used_lanes = 0;
> > @@ -311,7 +310,9 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
> > mutex_lock(&phy->mutex);
> > if (phy->entity) {
> > struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
> > - struct isp_bus_cfg *buscfg = pipe->external->host_priv;
> > + struct isp_bus_cfg *buscfg =
> > + &container_of(pipe->external->asd,
> > + struct isp_async_subdev, asd)->bus;
> >
> > csiphy_routing_cfg(phy, buscfg->interface, false,
> > buscfg->bus.ccp2.phy_layer);
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2.3 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
2017-08-16 17:24 ` Laurent Pinchart
2017-08-16 17:26 ` Sakari Ailus
@ 2017-08-16 17:39 ` Sakari Ailus
2017-08-16 19:32 ` Laurent Pinchart
1 sibling, 1 reply; 16+ messages in thread
From: Sakari Ailus @ 2017-08-16 17:39 UTC (permalink / raw)
To: linux-media; +Cc: pavel, laurent.pinchart
struct v4l2_subdev.host_priv is intended to be used by another driver. This
is hardly good design but back in the days of platform data was a quick
hack to get things done.
As the sub-device specific bus information can be stored to the ISP driver
specific struct allocated along with v4l2_async_subdev, keep the
information there and only there.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v2.2:
- Introduce a macro v4l2_subdev_to_bus_cfg() in order to remove
non-trivial usage in places where conversion from sub-device to ISP bus
configuration is needed.
drivers/media/platform/omap3isp/isp.c | 29 +++++++----------------------
drivers/media/platform/omap3isp/isp.h | 4 +++-
drivers/media/platform/omap3isp/ispccdc.c | 12 ++++++------
drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
drivers/media/platform/omap3isp/ispcsi2.c | 2 +-
drivers/media/platform/omap3isp/ispcsiphy.c | 8 +++-----
6 files changed, 22 insertions(+), 36 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..c3014c82d64d 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
return -EINVAL;
}
-static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
- struct v4l2_subdev *subdev,
- struct v4l2_async_subdev *asd)
-{
- struct isp_async_subdev *isd =
- container_of(asd, struct isp_async_subdev, asd);
-
- isd->sd = subdev;
- isd->sd->host_priv = &isd->bus;
-
- return 0;
-}
-
static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
{
struct isp_device *isp = container_of(async, struct isp_device,
notifier);
struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
struct v4l2_subdev *sd;
- struct isp_bus_cfg *bus;
int ret;
ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
@@ -2215,13 +2201,13 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
return ret;
list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
- /* Only try to link entities whose interface was set on bound */
- if (sd->host_priv) {
- bus = (struct isp_bus_cfg *)sd->host_priv;
- ret = isp_link_entity(isp, &sd->entity, bus->interface);
- if (ret < 0)
- return ret;
- }
+ if (!sd->asd)
+ continue;
+
+ ret = isp_link_entity(isp, &sd->entity,
+ v4l2_subdev_to_bus_cfg(sd)->interface);
+ if (ret < 0)
+ return ret;
}
ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
@@ -2399,7 +2385,6 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
- isp->notifier.bound = isp_subdev_notifier_bound;
isp->notifier.complete = isp_subdev_notifier_complete;
ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 2f2ae609c548..e528df6efc09 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -226,11 +226,13 @@ struct isp_device {
};
struct isp_async_subdev {
- struct v4l2_subdev *sd;
struct isp_bus_cfg bus;
struct v4l2_async_subdev asd;
};
+#define v4l2_subdev_to_bus_cfg(sd) \
+ (&container_of((sd)->asd, struct isp_async_subdev, asd)->bus)
+
#define v4l2_dev_to_isp_device(dev) \
container_of(dev, struct isp_device, v4l2_dev)
diff --git a/drivers/media/platform/omap3isp/ispccdc.c b/drivers/media/platform/omap3isp/ispccdc.c
index 4947876cfadf..80fed9526945 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1139,7 +1139,8 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
+ parcfg = &v4l2_subdev_to_bus_cfg(
+ to_isp_pipeline(&ccdc->subdev.entity)->external)
->bus.parallel;
ccdc->bt656 = parcfg->bt656;
}
@@ -2412,11 +2413,10 @@ static int ccdc_link_validate(struct v4l2_subdev *sd,
/* We've got a parallel sensor here. */
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- struct isp_parallel_cfg *parcfg =
- &((struct isp_bus_cfg *)
- media_entity_to_v4l2_subdev(link->source->entity)
- ->host_priv)->bus.parallel;
- parallel_shift = parcfg->data_lane_shift;
+ parallel_shift =
+ v4l2_subdev_to_bus_cfg(
+ to_isp_pipeline(&ccdc->subdev.entity)->external)
+ ->bus.parallel.data_lane_shift;
} else {
parallel_shift = 0;
}
diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c
index 3db8df09cd9a..e062939d0d05 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -350,6 +350,7 @@ static void ccp2_lcx_config(struct isp_ccp2_device *ccp2,
*/
static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
{
+ struct isp_pipeline *pipe = to_isp_pipeline(&ccp2->subdev.entity);
const struct isp_bus_cfg *buscfg;
struct v4l2_mbus_framefmt *format;
struct media_pad *pad;
@@ -361,7 +362,7 @@ static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
pad = media_entity_remote_pad(&ccp2->pads[CCP2_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
- buscfg = sensor->host_priv;
+ buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
ret = ccp2_phyif_config(ccp2, &buscfg->bus.ccp2);
if (ret < 0)
diff --git a/drivers/media/platform/omap3isp/ispcsi2.c b/drivers/media/platform/omap3isp/ispcsi2.c
index 3ec37fed710b..a4d3d030e81e 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -566,7 +566,7 @@ static int csi2_configure(struct isp_csi2_device *csi2)
pad = media_entity_remote_pad(&csi2->pads[CSI2_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
- buscfg = sensor->host_priv;
+ buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
csi2->frame_skip = 0;
v4l2_subdev_call(sensor, sensor, g_skip_frames, &csi2->frame_skip);
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index ed1eb9907ae0..a28fb79abaac 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -165,10 +165,7 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 power)
static int omap3isp_csiphy_config(struct isp_csiphy *phy)
{
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_async_subdev *isd =
- container_of(pipe->external->asd, struct isp_async_subdev, asd);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
- pipe->external->host_priv : &isd->bus;
+ struct isp_bus_cfg *buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
struct isp_csiphy_lanes_cfg *lanes;
int csi2_ddrclk_khz;
unsigned int num_data_lanes, used_lanes = 0;
@@ -311,7 +308,8 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
mutex_lock(&phy->mutex);
if (phy->entity) {
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv;
+ struct isp_bus_cfg *buscfg =
+ v4l2_subdev_to_bus_cfg(pipe->external);
csiphy_routing_cfg(phy, buscfg->interface, false,
buscfg->bus.ccp2.phy_layer);
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2.3 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
2017-08-16 17:39 ` [PATCH v2.3 " Sakari Ailus
@ 2017-08-16 19:32 ` Laurent Pinchart
2017-08-16 20:39 ` [PATCH v2.4 " Sakari Ailus
0 siblings, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2017-08-16 19:32 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, pavel
Hi Sakari,
Thank you for the patch.
On Wednesday 16 Aug 2017 20:39:43 Sakari Ailus wrote:
> struct v4l2_subdev.host_priv is intended to be used by another driver. This
> is hardly good design but back in the days of platform data was a quick
> hack to get things done.
>
> As the sub-device specific bus information can be stored to the ISP driver
> specific struct allocated along with v4l2_async_subdev, keep the
> information there and only there.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> since v2.2:
>
> - Introduce a macro v4l2_subdev_to_bus_cfg() in order to remove
> non-trivial usage in places where conversion from sub-device to ISP bus
> configuration is needed.
>
> drivers/media/platform/omap3isp/isp.c | 29 ++++++--------------------
> drivers/media/platform/omap3isp/isp.h | 4 +++-
> drivers/media/platform/omap3isp/ispccdc.c | 12 ++++++------
> drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
> drivers/media/platform/omap3isp/ispcsi2.c | 2 +-
> drivers/media/platform/omap3isp/ispcsiphy.c | 8 +++-----
> 6 files changed, 22 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 6cb1f0495804..c3014c82d64d
> 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
> return -EINVAL;
> }
>
> -static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> - struct v4l2_subdev *subdev,
> - struct v4l2_async_subdev *asd)
> -{
> - struct isp_async_subdev *isd =
> - container_of(asd, struct isp_async_subdev, asd);
> -
> - isd->sd = subdev;
> - isd->sd->host_priv = &isd->bus;
> -
> - return 0;
> -}
> -
> static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
> {
> struct isp_device *isp = container_of(async, struct isp_device,
> notifier);
> struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
> struct v4l2_subdev *sd;
> - struct isp_bus_cfg *bus;
> int ret;
>
> ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
> @@ -2215,13 +2201,13 @@ static int isp_subdev_notifier_complete(struct
> v4l2_async_notifier *async) return ret;
>
> list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> - /* Only try to link entities whose interface was set on bound
*/
> - if (sd->host_priv) {
> - bus = (struct isp_bus_cfg *)sd->host_priv;
> - ret = isp_link_entity(isp, &sd->entity, bus-
>interface);
> - if (ret < 0)
> - return ret;
> - }
> + if (!sd->asd)
> + continue;
> +
> + ret = isp_link_entity(isp, &sd->entity,
> + v4l2_subdev_to_bus_cfg(sd)->interface);
> + if (ret < 0)
> + return ret;
> }
>
> ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> @@ -2399,7 +2385,6 @@ static int isp_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_register_entities;
>
> - isp->notifier.bound = isp_subdev_notifier_bound;
> isp->notifier.complete = isp_subdev_notifier_complete;
>
> ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
> diff --git a/drivers/media/platform/omap3isp/isp.h
> b/drivers/media/platform/omap3isp/isp.h index 2f2ae609c548..e528df6efc09
> 100644
> --- a/drivers/media/platform/omap3isp/isp.h
> +++ b/drivers/media/platform/omap3isp/isp.h
> @@ -226,11 +226,13 @@ struct isp_device {
> };
>
> struct isp_async_subdev {
> - struct v4l2_subdev *sd;
> struct isp_bus_cfg bus;
> struct v4l2_async_subdev asd;
> };
>
> +#define v4l2_subdev_to_bus_cfg(sd) \
> + (&container_of((sd)->asd, struct isp_async_subdev, asd)->bus)
> +
> #define v4l2_dev_to_isp_device(dev) \
> container_of(dev, struct isp_device, v4l2_dev)
>
> diff --git a/drivers/media/platform/omap3isp/ispccdc.c
> b/drivers/media/platform/omap3isp/ispccdc.c index
> 4947876cfadf..80fed9526945 100644
> --- a/drivers/media/platform/omap3isp/ispccdc.c
> +++ b/drivers/media/platform/omap3isp/ispccdc.c
> @@ -1139,7 +1139,8 @@ static void ccdc_configure(struct isp_ccdc_device
> *ccdc) pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> if (ccdc->input == CCDC_INPUT_PARALLEL) {
> - parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
> + parcfg = &v4l2_subdev_to_bus_cfg(
> + to_isp_pipeline(&ccdc->subdev.entity)->external)
> ->bus.parallel;
> ccdc->bt656 = parcfg->bt656;
> }
> @@ -2412,11 +2413,10 @@ static int ccdc_link_validate(struct v4l2_subdev
> *sd,
>
> /* We've got a parallel sensor here. */
> if (ccdc->input == CCDC_INPUT_PARALLEL) {
> - struct isp_parallel_cfg *parcfg =
> - &((struct isp_bus_cfg *)
> - media_entity_to_v4l2_subdev(link->source->entity)
> - ->host_priv)->bus.parallel;
> - parallel_shift = parcfg->data_lane_shift;
> + parallel_shift =
> + v4l2_subdev_to_bus_cfg(
> + to_isp_pipeline(&ccdc->subdev.entity)-
>external)
> + ->bus.parallel.data_lane_shift;
This oopses because pipe->external is NULL :-( pipe->external is set in
isp_video_check_external_subdevs(), which is called after
media_pipeline_start().
> } else {
> parallel_shift = 0;
> }
> diff --git a/drivers/media/platform/omap3isp/ispccp2.c
> b/drivers/media/platform/omap3isp/ispccp2.c index
> 3db8df09cd9a..e062939d0d05 100644
> --- a/drivers/media/platform/omap3isp/ispccp2.c
> +++ b/drivers/media/platform/omap3isp/ispccp2.c
> @@ -350,6 +350,7 @@ static void ccp2_lcx_config(struct isp_ccp2_device
> *ccp2, */
> static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
> {
> + struct isp_pipeline *pipe = to_isp_pipeline(&ccp2->subdev.entity);
> const struct isp_bus_cfg *buscfg;
> struct v4l2_mbus_framefmt *format;
> struct media_pad *pad;
> @@ -361,7 +362,7 @@ static int ccp2_if_configure(struct isp_ccp2_device
> *ccp2)
>
> pad = media_entity_remote_pad(&ccp2->pads[CCP2_PAD_SINK]);
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> - buscfg = sensor->host_priv;
> + buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
>
> ret = ccp2_phyif_config(ccp2, &buscfg->bus.ccp2);
> if (ret < 0)
> diff --git a/drivers/media/platform/omap3isp/ispcsi2.c
> b/drivers/media/platform/omap3isp/ispcsi2.c index
> 3ec37fed710b..a4d3d030e81e 100644
> --- a/drivers/media/platform/omap3isp/ispcsi2.c
> +++ b/drivers/media/platform/omap3isp/ispcsi2.c
> @@ -566,7 +566,7 @@ static int csi2_configure(struct isp_csi2_device *csi2)
>
> pad = media_entity_remote_pad(&csi2->pads[CSI2_PAD_SINK]);
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> - buscfg = sensor->host_priv;
> + buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
>
> csi2->frame_skip = 0;
> v4l2_subdev_call(sensor, sensor, g_skip_frames, &csi2->frame_skip);
> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
> b/drivers/media/platform/omap3isp/ispcsiphy.c index
> ed1eb9907ae0..a28fb79abaac 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> @@ -165,10 +165,7 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32
> power) static int omap3isp_csiphy_config(struct isp_csiphy *phy)
> {
> struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
> - struct isp_async_subdev *isd =
> - container_of(pipe->external->asd, struct isp_async_subdev,
asd);
> - struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
> - pipe->external->host_priv : &isd->bus;
> + struct isp_bus_cfg *buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
> struct isp_csiphy_lanes_cfg *lanes;
> int csi2_ddrclk_khz;
> unsigned int num_data_lanes, used_lanes = 0;
> @@ -311,7 +308,8 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
> mutex_lock(&phy->mutex);
> if (phy->entity) {
> struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
> - struct isp_bus_cfg *buscfg = pipe->external->host_priv;
> + struct isp_bus_cfg *buscfg =
> + v4l2_subdev_to_bus_cfg(pipe->external);
>
> csiphy_routing_cfg(phy, buscfg->interface, false,
> buscfg->bus.ccp2.phy_layer);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2.4 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
2017-08-16 19:32 ` Laurent Pinchart
@ 2017-08-16 20:39 ` Sakari Ailus
2017-08-17 13:27 ` [PATCH v2.5 " Sakari Ailus
0 siblings, 1 reply; 16+ messages in thread
From: Sakari Ailus @ 2017-08-16 20:39 UTC (permalink / raw)
To: linux-media; +Cc: pavel, laurent.pinchart
struct v4l2_subdev.host_priv is intended to be used by another driver. This
is hardly good design but back in the days of platform data was a quick
hack to get things done.
As the sub-device specific bus information can be stored to the ISP driver
specific struct allocated along with v4l2_async_subdev, keep the
information there and only there.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v2.3:
- Fix obtaining the external entity in ispccdc.c.
drivers/media/platform/omap3isp/isp.c | 29 +++++++----------------------
drivers/media/platform/omap3isp/isp.h | 4 +++-
drivers/media/platform/omap3isp/ispccdc.c | 12 ++++++------
drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
drivers/media/platform/omap3isp/ispcsi2.c | 2 +-
drivers/media/platform/omap3isp/ispcsiphy.c | 8 +++-----
6 files changed, 22 insertions(+), 36 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..c3014c82d64d 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
return -EINVAL;
}
-static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
- struct v4l2_subdev *subdev,
- struct v4l2_async_subdev *asd)
-{
- struct isp_async_subdev *isd =
- container_of(asd, struct isp_async_subdev, asd);
-
- isd->sd = subdev;
- isd->sd->host_priv = &isd->bus;
-
- return 0;
-}
-
static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
{
struct isp_device *isp = container_of(async, struct isp_device,
notifier);
struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
struct v4l2_subdev *sd;
- struct isp_bus_cfg *bus;
int ret;
ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
@@ -2215,13 +2201,13 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
return ret;
list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
- /* Only try to link entities whose interface was set on bound */
- if (sd->host_priv) {
- bus = (struct isp_bus_cfg *)sd->host_priv;
- ret = isp_link_entity(isp, &sd->entity, bus->interface);
- if (ret < 0)
- return ret;
- }
+ if (!sd->asd)
+ continue;
+
+ ret = isp_link_entity(isp, &sd->entity,
+ v4l2_subdev_to_bus_cfg(sd)->interface);
+ if (ret < 0)
+ return ret;
}
ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
@@ -2399,7 +2385,6 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
- isp->notifier.bound = isp_subdev_notifier_bound;
isp->notifier.complete = isp_subdev_notifier_complete;
ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 2f2ae609c548..e528df6efc09 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -226,11 +226,13 @@ struct isp_device {
};
struct isp_async_subdev {
- struct v4l2_subdev *sd;
struct isp_bus_cfg bus;
struct v4l2_async_subdev asd;
};
+#define v4l2_subdev_to_bus_cfg(sd) \
+ (&container_of((sd)->asd, struct isp_async_subdev, asd)->bus)
+
#define v4l2_dev_to_isp_device(dev) \
container_of(dev, struct isp_device, v4l2_dev)
diff --git a/drivers/media/platform/omap3isp/ispccdc.c b/drivers/media/platform/omap3isp/ispccdc.c
index 4947876cfadf..1c12a8420801 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1139,7 +1139,8 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
+ parcfg = &v4l2_subdev_to_bus_cfg(
+ to_isp_pipeline(&ccdc->subdev.entity)->external)
->bus.parallel;
ccdc->bt656 = parcfg->bt656;
}
@@ -2412,11 +2413,10 @@ static int ccdc_link_validate(struct v4l2_subdev *sd,
/* We've got a parallel sensor here. */
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- struct isp_parallel_cfg *parcfg =
- &((struct isp_bus_cfg *)
- media_entity_to_v4l2_subdev(link->source->entity)
- ->host_priv)->bus.parallel;
- parallel_shift = parcfg->data_lane_shift;
+ parallel_shift =
+ v4l2_subdev_to_bus_cfg(media_entity_to_v4l2_subdev(
+ link->source->entity))
+ ->bus.parallel.data_lane_shift;
} else {
parallel_shift = 0;
}
diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c
index 3db8df09cd9a..e062939d0d05 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -350,6 +350,7 @@ static void ccp2_lcx_config(struct isp_ccp2_device *ccp2,
*/
static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
{
+ struct isp_pipeline *pipe = to_isp_pipeline(&ccp2->subdev.entity);
const struct isp_bus_cfg *buscfg;
struct v4l2_mbus_framefmt *format;
struct media_pad *pad;
@@ -361,7 +362,7 @@ static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
pad = media_entity_remote_pad(&ccp2->pads[CCP2_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
- buscfg = sensor->host_priv;
+ buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
ret = ccp2_phyif_config(ccp2, &buscfg->bus.ccp2);
if (ret < 0)
diff --git a/drivers/media/platform/omap3isp/ispcsi2.c b/drivers/media/platform/omap3isp/ispcsi2.c
index 3ec37fed710b..a4d3d030e81e 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -566,7 +566,7 @@ static int csi2_configure(struct isp_csi2_device *csi2)
pad = media_entity_remote_pad(&csi2->pads[CSI2_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
- buscfg = sensor->host_priv;
+ buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
csi2->frame_skip = 0;
v4l2_subdev_call(sensor, sensor, g_skip_frames, &csi2->frame_skip);
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index ed1eb9907ae0..a28fb79abaac 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -165,10 +165,7 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 power)
static int omap3isp_csiphy_config(struct isp_csiphy *phy)
{
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_async_subdev *isd =
- container_of(pipe->external->asd, struct isp_async_subdev, asd);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
- pipe->external->host_priv : &isd->bus;
+ struct isp_bus_cfg *buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
struct isp_csiphy_lanes_cfg *lanes;
int csi2_ddrclk_khz;
unsigned int num_data_lanes, used_lanes = 0;
@@ -311,7 +308,8 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
mutex_lock(&phy->mutex);
if (phy->entity) {
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv;
+ struct isp_bus_cfg *buscfg =
+ v4l2_subdev_to_bus_cfg(pipe->external);
csiphy_routing_cfg(phy, buscfg->interface, false,
buscfg->bus.ccp2.phy_layer);
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2.5 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
2017-08-16 20:39 ` [PATCH v2.4 " Sakari Ailus
@ 2017-08-17 13:27 ` Sakari Ailus
2017-08-17 18:29 ` Laurent Pinchart
0 siblings, 1 reply; 16+ messages in thread
From: Sakari Ailus @ 2017-08-17 13:27 UTC (permalink / raw)
To: linux-media; +Cc: pavel, laurent.pinchart
struct v4l2_subdev.host_priv is intended to be used by another driver. This
is hardly good design but back in the days of platform data was a quick
hack to get things done.
As the sub-device specific bus information can be stored to the ISP driver
specific struct allocated along with v4l2_async_subdev, keep the
information there and only there.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v2.4:
- Use temporary variables to avoid somewhat lengthy statements.
drivers/media/platform/omap3isp/isp.c | 29 +++++++----------------------
drivers/media/platform/omap3isp/isp.h | 4 +++-
drivers/media/platform/omap3isp/ispccdc.c | 16 +++++++++-------
drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
drivers/media/platform/omap3isp/ispcsi2.c | 2 +-
drivers/media/platform/omap3isp/ispcsiphy.c | 8 +++-----
6 files changed, 25 insertions(+), 37 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..c3014c82d64d 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
return -EINVAL;
}
-static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
- struct v4l2_subdev *subdev,
- struct v4l2_async_subdev *asd)
-{
- struct isp_async_subdev *isd =
- container_of(asd, struct isp_async_subdev, asd);
-
- isd->sd = subdev;
- isd->sd->host_priv = &isd->bus;
-
- return 0;
-}
-
static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
{
struct isp_device *isp = container_of(async, struct isp_device,
notifier);
struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
struct v4l2_subdev *sd;
- struct isp_bus_cfg *bus;
int ret;
ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
@@ -2215,13 +2201,13 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
return ret;
list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
- /* Only try to link entities whose interface was set on bound */
- if (sd->host_priv) {
- bus = (struct isp_bus_cfg *)sd->host_priv;
- ret = isp_link_entity(isp, &sd->entity, bus->interface);
- if (ret < 0)
- return ret;
- }
+ if (!sd->asd)
+ continue;
+
+ ret = isp_link_entity(isp, &sd->entity,
+ v4l2_subdev_to_bus_cfg(sd)->interface);
+ if (ret < 0)
+ return ret;
}
ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
@@ -2399,7 +2385,6 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
- isp->notifier.bound = isp_subdev_notifier_bound;
isp->notifier.complete = isp_subdev_notifier_complete;
ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 2f2ae609c548..e528df6efc09 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -226,11 +226,13 @@ struct isp_device {
};
struct isp_async_subdev {
- struct v4l2_subdev *sd;
struct isp_bus_cfg bus;
struct v4l2_async_subdev asd;
};
+#define v4l2_subdev_to_bus_cfg(sd) \
+ (&container_of((sd)->asd, struct isp_async_subdev, asd)->bus)
+
#define v4l2_dev_to_isp_device(dev) \
container_of(dev, struct isp_device, v4l2_dev)
diff --git a/drivers/media/platform/omap3isp/ispccdc.c b/drivers/media/platform/omap3isp/ispccdc.c
index 4947876cfadf..b66276ab5765 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1139,8 +1139,10 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
- ->bus.parallel;
+ struct v4l2_subdev *sd =
+ to_isp_pipeline(&ccdc->subdev.entity)->external;
+
+ parcfg = &v4l2_subdev_to_bus_cfg(sd)->bus.parallel;
ccdc->bt656 = parcfg->bt656;
}
@@ -2412,11 +2414,11 @@ static int ccdc_link_validate(struct v4l2_subdev *sd,
/* We've got a parallel sensor here. */
if (ccdc->input == CCDC_INPUT_PARALLEL) {
- struct isp_parallel_cfg *parcfg =
- &((struct isp_bus_cfg *)
- media_entity_to_v4l2_subdev(link->source->entity)
- ->host_priv)->bus.parallel;
- parallel_shift = parcfg->data_lane_shift;
+ struct v4l2_subdev *sd =
+ media_entity_to_v4l2_subdev(link->source->entity);
+ struct isp_bus_cfg *bus_cfg = v4l2_subdev_to_bus_cfg(sd);
+
+ parallel_shift = bus_cfg->bus.parallel.data_lane_shift;
} else {
parallel_shift = 0;
}
diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c
index 3db8df09cd9a..e062939d0d05 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -350,6 +350,7 @@ static void ccp2_lcx_config(struct isp_ccp2_device *ccp2,
*/
static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
{
+ struct isp_pipeline *pipe = to_isp_pipeline(&ccp2->subdev.entity);
const struct isp_bus_cfg *buscfg;
struct v4l2_mbus_framefmt *format;
struct media_pad *pad;
@@ -361,7 +362,7 @@ static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
pad = media_entity_remote_pad(&ccp2->pads[CCP2_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
- buscfg = sensor->host_priv;
+ buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
ret = ccp2_phyif_config(ccp2, &buscfg->bus.ccp2);
if (ret < 0)
diff --git a/drivers/media/platform/omap3isp/ispcsi2.c b/drivers/media/platform/omap3isp/ispcsi2.c
index 3ec37fed710b..a4d3d030e81e 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -566,7 +566,7 @@ static int csi2_configure(struct isp_csi2_device *csi2)
pad = media_entity_remote_pad(&csi2->pads[CSI2_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
- buscfg = sensor->host_priv;
+ buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
csi2->frame_skip = 0;
v4l2_subdev_call(sensor, sensor, g_skip_frames, &csi2->frame_skip);
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index ed1eb9907ae0..a28fb79abaac 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -165,10 +165,7 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 power)
static int omap3isp_csiphy_config(struct isp_csiphy *phy)
{
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_async_subdev *isd =
- container_of(pipe->external->asd, struct isp_async_subdev, asd);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
- pipe->external->host_priv : &isd->bus;
+ struct isp_bus_cfg *buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
struct isp_csiphy_lanes_cfg *lanes;
int csi2_ddrclk_khz;
unsigned int num_data_lanes, used_lanes = 0;
@@ -311,7 +308,8 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
mutex_lock(&phy->mutex);
if (phy->entity) {
struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
- struct isp_bus_cfg *buscfg = pipe->external->host_priv;
+ struct isp_bus_cfg *buscfg =
+ v4l2_subdev_to_bus_cfg(pipe->external);
csiphy_routing_cfg(phy, buscfg->interface, false,
buscfg->bus.ccp2.phy_layer);
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2.5 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field
2017-08-17 13:27 ` [PATCH v2.5 " Sakari Ailus
@ 2017-08-17 18:29 ` Laurent Pinchart
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2017-08-17 18:29 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, pavel
Hi Sakari,
Thank you for the patch.
On Thursday 17 Aug 2017 16:27:38 Sakari Ailus wrote:
> struct v4l2_subdev.host_priv is intended to be used by another driver. This
> is hardly good design but back in the days of platform data was a quick
> hack to get things done.
>
> As the sub-device specific bus information can be stored to the ISP driver
> specific struct allocated along with v4l2_async_subdev, keep the
> information there and only there.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> since v2.4:
>
> - Use temporary variables to avoid somewhat lengthy statements.
>
> drivers/media/platform/omap3isp/isp.c | 29 ++++++--------------------
> drivers/media/platform/omap3isp/isp.h | 4 +++-
> drivers/media/platform/omap3isp/ispccdc.c | 16 +++++++++-------
> drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
> drivers/media/platform/omap3isp/ispcsi2.c | 2 +-
> drivers/media/platform/omap3isp/ispcsiphy.c | 8 +++-----
> 6 files changed, 25 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 6cb1f0495804..c3014c82d64d
> 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
> return -EINVAL;
> }
>
> -static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> - struct v4l2_subdev *subdev,
> - struct v4l2_async_subdev *asd)
> -{
> - struct isp_async_subdev *isd =
> - container_of(asd, struct isp_async_subdev, asd);
> -
> - isd->sd = subdev;
> - isd->sd->host_priv = &isd->bus;
> -
> - return 0;
> -}
> -
> static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
> {
> struct isp_device *isp = container_of(async, struct isp_device,
> notifier);
> struct v4l2_device *v4l2_dev = &isp->v4l2_dev;
> struct v4l2_subdev *sd;
> - struct isp_bus_cfg *bus;
> int ret;
>
> ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
> @@ -2215,13 +2201,13 @@ static int isp_subdev_notifier_complete(struct
> v4l2_async_notifier *async) return ret;
>
> list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> - /* Only try to link entities whose interface was set on bound
*/
> - if (sd->host_priv) {
> - bus = (struct isp_bus_cfg *)sd->host_priv;
> - ret = isp_link_entity(isp, &sd->entity, bus-
>interface);
> - if (ret < 0)
> - return ret;
> - }
> + if (!sd->asd)
> + continue;
> +
> + ret = isp_link_entity(isp, &sd->entity,
> + v4l2_subdev_to_bus_cfg(sd)->interface);
> + if (ret < 0)
> + return ret;
> }
>
> ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> @@ -2399,7 +2385,6 @@ static int isp_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_register_entities;
>
> - isp->notifier.bound = isp_subdev_notifier_bound;
> isp->notifier.complete = isp_subdev_notifier_complete;
>
> ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
> diff --git a/drivers/media/platform/omap3isp/isp.h
> b/drivers/media/platform/omap3isp/isp.h index 2f2ae609c548..e528df6efc09
> 100644
> --- a/drivers/media/platform/omap3isp/isp.h
> +++ b/drivers/media/platform/omap3isp/isp.h
> @@ -226,11 +226,13 @@ struct isp_device {
> };
>
> struct isp_async_subdev {
> - struct v4l2_subdev *sd;
> struct isp_bus_cfg bus;
> struct v4l2_async_subdev asd;
> };
>
> +#define v4l2_subdev_to_bus_cfg(sd) \
> + (&container_of((sd)->asd, struct isp_async_subdev, asd)->bus)
> +
> #define v4l2_dev_to_isp_device(dev) \
> container_of(dev, struct isp_device, v4l2_dev)
>
> diff --git a/drivers/media/platform/omap3isp/ispccdc.c
> b/drivers/media/platform/omap3isp/ispccdc.c index
> 4947876cfadf..b66276ab5765 100644
> --- a/drivers/media/platform/omap3isp/ispccdc.c
> +++ b/drivers/media/platform/omap3isp/ispccdc.c
> @@ -1139,8 +1139,10 @@ static void ccdc_configure(struct isp_ccdc_device
> *ccdc) pad = media_entity_remote_pad(&ccdc->pads[CCDC_PAD_SINK]);
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> if (ccdc->input == CCDC_INPUT_PARALLEL) {
> - parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
> - ->bus.parallel;
> + struct v4l2_subdev *sd =
> + to_isp_pipeline(&ccdc->subdev.entity)->external;
> +
> + parcfg = &v4l2_subdev_to_bus_cfg(sd)->bus.parallel;
> ccdc->bt656 = parcfg->bt656;
> }
>
> @@ -2412,11 +2414,11 @@ static int ccdc_link_validate(struct v4l2_subdev
> *sd,
>
> /* We've got a parallel sensor here. */
> if (ccdc->input == CCDC_INPUT_PARALLEL) {
> - struct isp_parallel_cfg *parcfg =
> - &((struct isp_bus_cfg *)
> - media_entity_to_v4l2_subdev(link->source->entity)
> - ->host_priv)->bus.parallel;
> - parallel_shift = parcfg->data_lane_shift;
> + struct v4l2_subdev *sd =
> + media_entity_to_v4l2_subdev(link->source->entity);
> + struct isp_bus_cfg *bus_cfg = v4l2_subdev_to_bus_cfg(sd);
> +
> + parallel_shift = bus_cfg->bus.parallel.data_lane_shift;
> } else {
> parallel_shift = 0;
> }
> diff --git a/drivers/media/platform/omap3isp/ispccp2.c
> b/drivers/media/platform/omap3isp/ispccp2.c index
> 3db8df09cd9a..e062939d0d05 100644
> --- a/drivers/media/platform/omap3isp/ispccp2.c
> +++ b/drivers/media/platform/omap3isp/ispccp2.c
> @@ -350,6 +350,7 @@ static void ccp2_lcx_config(struct isp_ccp2_device
> *ccp2, */
> static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
> {
> + struct isp_pipeline *pipe = to_isp_pipeline(&ccp2->subdev.entity);
> const struct isp_bus_cfg *buscfg;
> struct v4l2_mbus_framefmt *format;
> struct media_pad *pad;
> @@ -361,7 +362,7 @@ static int ccp2_if_configure(struct isp_ccp2_device
> *ccp2)
>
> pad = media_entity_remote_pad(&ccp2->pads[CCP2_PAD_SINK]);
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> - buscfg = sensor->host_priv;
> + buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
>
> ret = ccp2_phyif_config(ccp2, &buscfg->bus.ccp2);
> if (ret < 0)
> diff --git a/drivers/media/platform/omap3isp/ispcsi2.c
> b/drivers/media/platform/omap3isp/ispcsi2.c index
> 3ec37fed710b..a4d3d030e81e 100644
> --- a/drivers/media/platform/omap3isp/ispcsi2.c
> +++ b/drivers/media/platform/omap3isp/ispcsi2.c
> @@ -566,7 +566,7 @@ static int csi2_configure(struct isp_csi2_device *csi2)
>
> pad = media_entity_remote_pad(&csi2->pads[CSI2_PAD_SINK]);
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> - buscfg = sensor->host_priv;
> + buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
>
> csi2->frame_skip = 0;
> v4l2_subdev_call(sensor, sensor, g_skip_frames, &csi2->frame_skip);
> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
> b/drivers/media/platform/omap3isp/ispcsiphy.c index
> ed1eb9907ae0..a28fb79abaac 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> @@ -165,10 +165,7 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32
> power) static int omap3isp_csiphy_config(struct isp_csiphy *phy)
> {
> struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
> - struct isp_async_subdev *isd =
> - container_of(pipe->external->asd, struct isp_async_subdev,
asd);
> - struct isp_bus_cfg *buscfg = pipe->external->host_priv ?
> - pipe->external->host_priv : &isd->bus;
> + struct isp_bus_cfg *buscfg = v4l2_subdev_to_bus_cfg(pipe->external);
> struct isp_csiphy_lanes_cfg *lanes;
> int csi2_ddrclk_khz;
> unsigned int num_data_lanes, used_lanes = 0;
> @@ -311,7 +308,8 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
> mutex_lock(&phy->mutex);
> if (phy->entity) {
> struct isp_pipeline *pipe = to_isp_pipeline(phy->entity);
> - struct isp_bus_cfg *buscfg = pipe->external->host_priv;
> + struct isp_bus_cfg *buscfg =
> + v4l2_subdev_to_bus_cfg(pipe->external);
>
> csiphy_routing_cfg(phy, buscfg->interface, false,
> buscfg->bus.ccp2.phy_layer);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-08-17 18:28 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-16 12:51 [PATCH v2 0/5] Omap3isp CCP2 support Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 1/5] omap3isp: Parse CSI1 configuration from the device tree Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 2/5] omap3isp: Correctly set IO_OUT_SEL and VP_CLK_POL for CCP2 mode Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 3/5] omap3isp: Always initialise isp and mutex for csiphy1 Sakari Ailus
2017-08-16 12:51 ` [PATCH v2 4/5] omap3isp: csiphy: Don't assume the CSI receiver is a CSI2 module Sakari Ailus
2017-08-16 12:54 ` Laurent Pinchart
2017-08-16 12:51 ` [PATCH v2 5/5] omap3isp: Correctly configure routing in PHY release Sakari Ailus
2017-08-16 16:52 ` [PATCH v2.1 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field Sakari Ailus
2017-08-16 17:05 ` [PATCH v2.2 " Sakari Ailus
2017-08-16 17:24 ` Laurent Pinchart
2017-08-16 17:26 ` Sakari Ailus
2017-08-16 17:39 ` [PATCH v2.3 " Sakari Ailus
2017-08-16 19:32 ` Laurent Pinchart
2017-08-16 20:39 ` [PATCH v2.4 " Sakari Ailus
2017-08-17 13:27 ` [PATCH v2.5 " Sakari Ailus
2017-08-17 18:29 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.