| Message ID | 20170706103851.GA9555@amd (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Pavel, On Thu, Jul 06, 2017 at 12:38:51PM +0200, Pavel Machek wrote: > Hi! > > > > > > > I expect to have most of them in during the next merge window. > > > > > > > > > > So git://linuxtv.org/media_tree.git branch master is the right one to > > > > > work one? > > > > > > > > I also pushed the rebased ccp2 branch there: > > > > > > > > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=ccp2> > > > > > > > > It's now right on the top of media-tree master. > > > > > > Is ccp2 branch expected to go into 4.13, too? > > > > Hi Pavel, > > > > What I've done is just rebased the ccp2 branch. In other words, the patches > > in that branch are no more ready than they were. > > I thought they were ready even back then :-). > > > To get these merged we should ideally > > > > 1) Make sure there will be no regressions, > > Well, all I have running recent kernels is N900. If ccp branch works > for you on N9, that's probably as much testing as we can get. > > > 2) clean things up in the omap3isp; which resources are needed and when > > (e.g. regulators, PHY configuration) isn't clear at the moment and > > > > 2) have one driver using the implementation. > > > > At least 1) is needed. I think a number of framework patches could be > > mergeable before 2) and 3) are done. I can prepare a set later this week. > > But even that'd be likely for 4.14, not 4.13. > > Yep, it is too late for v4.13 now. But getting stuff ready for v4.14 > would be good. > > I started looking through the patches; I believe they are safe, but it > is probably better to review the series you've just mailed. > > The driver using the implementation -- yes, I have it all working on > n900 (incuding userland, I can actually take photos.) I can post the > series, or better link to kernel.org. > > Right now, my goal would be to get sensor working on N900 with > mainline (without flash and focus). > > I'd very much like any comment on patch attached below. > > Age Commit message (Expand) Author Files Lines > 2017-06-16 omap3isp: Destroy CSI-2 phy mutexes in error and module > 2017-06-16 omap3isp: Skip CSI-2 receiver initialisation in CCP2 > 2017-06-16 omap3isp: Correctly put the last iterated endpoint > 2017-06-16 omap3isp: Always initialise isp and mutex for csiphy1 > 2017-06-16 omap3isp: Return -EPROBE_DEFER if the required > 2017-06-16 omap3isp: Correctly set IO_OUT_SEL and VP_CLK_POL for CCP2 > 2017-06-16 omap3isp: Make external sub-device bus configuration a > 2017-06-15 omap3isp: Parse CSI1 configuration from the device tree > 2017-06-15 omap3isp: Check for valid port in endpoints Sakari > 2017-06-15 omap3isp: Ignore endpoints with invalid configuration > > # Nothing changes for bus_type == V4L2_MBUS_CSI2. FIXME: Is bus_type > set correctly? > > 2017-06-15 smiapp: add CCP2 support Pavel Machek 1 > > # bus_type will be guess, so no code changes on existing system: > > 2017-06-15 v4l: Add support for CSI-1 and CCP2 busses Sakari > > # Reads unused value -> can't break anything: > > 2017-06-13 v4l: fwnode: Obtain data bus type from FW Sakari > > # No code changes -> totally safe: > > 2017-06-13 v4l: fwnode: Call CSI2 bus csi2, not csi Sakari > 2017-06-13 dt: bindings: Add strobe property for CCP2 Sakari > 2017-06-13 dt: bindings: Explicitly specify bus type > > Best regards, > Pavel > > commit 1220492dd4c1872c8036caa573680f95aabc69bc > Author: Pavel <pavel@ucw.cz> > Date: Tue Feb 28 12:02:26 2017 +0100 > > omap3isp: add CSI1 support > > Use proper code path for csi1/ccp2 support. > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c > index 24a9fc5..47210b1 100644 > --- a/drivers/media/platform/omap3isp/ispccp2.c > +++ b/drivers/media/platform/omap3isp/ispccp2.c > @@ -1149,6 +1149,7 @@ int omap3isp_ccp2_init(struct isp_device *isp) > "Could not get regulator vdds_csib\n"); > ccp2->vdds_csib = NULL; > } > + ccp2->phy = &isp->isp_csiphy2; > } else if (isp->revision == ISP_REVISION_15_0) { > ccp2->phy = &isp->isp_csiphy1; > } > diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c > index 50c0f64..862fdd3 100644 > --- a/drivers/media/platform/omap3isp/ispcsiphy.c > +++ b/drivers/media/platform/omap3isp/ispcsiphy.c > @@ -197,9 +197,10 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy) > } > > if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1 > - || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2) > + || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2) { > lanes = &buscfg->bus.ccp2.lanecfg; > - else > + phy->num_data_lanes = 1; > + } else > lanes = &buscfg->bus.csi2.lanecfg; > > /* Clock and data lanes verification */ > @@ -302,13 +303,16 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy) > if (rval < 0) > goto done; > > - rval = csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_ON); > - if (rval) { > - regulator_disable(phy->vdd); > - goto done; > + if (phy->isp->revision == ISP_REVISION_15_0) { Shouldn't you make the related changes to omap3isp_csiphy_release() as well? Other than that the patch looks good to me. > + rval = csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_ON); > + if (rval) { > + regulator_disable(phy->vdd); > + goto done; > + } > + > + csiphy_power_autoswitch_enable(phy, true); > } > > - csiphy_power_autoswitch_enable(phy, true); > phy->phy_in_use = 1; > > done: >
Hi! > > > 1) Make sure there will be no regressions, > > > > Well, all I have running recent kernels is N900. If ccp branch works > > for you on N9, that's probably as much testing as we can get. > > > > > 2) clean things up in the omap3isp; which resources are needed and when > > > (e.g. regulators, PHY configuration) isn't clear at the moment and > > > > > > 2) have one driver using the implementation. > > > > > > At least 1) is needed. I think a number of framework patches could be > > > mergeable before 2) and 3) are done. I can prepare a set later this week. > > > But even that'd be likely for 4.14, not 4.13. > > > > Yep, it is too late for v4.13 now. But getting stuff ready for v4.14 > > would be good. ... > > @@ -302,13 +303,16 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy) > > if (rval < 0) > > goto done; > > > > - rval = csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_ON); > > - if (rval) { > > - regulator_disable(phy->vdd); > > - goto done; > > + if (phy->isp->revision == ISP_REVISION_15_0) { > > Shouldn't you make the related changes to omap3isp_csiphy_release() as > well? > > Other than that the patch looks good to me. Ah, yes, that needs to be fixed. Thanks for review. I'll refresh the series. I believe we now have everything neccessary to have useful driver for 4.14. Series is still based on 4.12-rc3, I can rebase it when there's better base. Best regards, Pavel
diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c index 24a9fc5..47210b1 100644 --- a/drivers/media/platform/omap3isp/ispccp2.c +++ b/drivers/media/platform/omap3isp/ispccp2.c @@ -1149,6 +1149,7 @@ int omap3isp_ccp2_init(struct isp_device *isp) "Could not get regulator vdds_csib\n"); ccp2->vdds_csib = NULL; } + ccp2->phy = &isp->isp_csiphy2; } else if (isp->revision == ISP_REVISION_15_0) { ccp2->phy = &isp->isp_csiphy1; } diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c index 50c0f64..862fdd3 100644 --- a/drivers/media/platform/omap3isp/ispcsiphy.c +++ b/drivers/media/platform/omap3isp/ispcsiphy.c @@ -197,9 +197,10 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy) } if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1 - || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2) + || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2) { lanes = &buscfg->bus.ccp2.lanecfg; - else + phy->num_data_lanes = 1; + } else lanes = &buscfg->bus.csi2.lanecfg; /* Clock and data lanes verification */ @@ -302,13 +303,16 @@ int omap3isp_csiphy_acquire(struct isp_csiphy *phy) if (rval < 0) goto done; - rval = csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_ON); - if (rval) { - regulator_disable(phy->vdd); - goto done; + if (phy->isp->revision == ISP_REVISION_15_0) { + rval = csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_ON); + if (rval) { + regulator_disable(phy->vdd); + goto done; + } + + csiphy_power_autoswitch_enable(phy, true); } - csiphy_power_autoswitch_enable(phy, true); phy->phy_in_use = 1; done: