Hi Sakari,
On Saturday 07 January 2012 23:51:24 Sakari Ailus wrote:
> Laurent Pinchart wrote:
> > On Tuesday 20 December 2011 21:28:05 Sakari Ailus wrote:
> >> From: Sakari Ailus <sakari.ailus@xxxxxx>
> >>
> >> Configure CSI-2 phy based on platform data in the ISP driver rather than
> >> in platform code.
> >>
> >> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx>
[snip]
> >> diff --git a/drivers/media/video/omap3isp/ispcsiphy.c
> >> b/drivers/media/video/omap3isp/ispcsiphy.c index 5be37ce..f027ece 100644
> >> --- a/drivers/media/video/omap3isp/ispcsiphy.c
> >> +++ b/drivers/media/video/omap3isp/ispcsiphy.c
> >> @@ -28,6 +28,8 @@
> >>
> >> #include <linux/device.h>
> >> #include <linux/regulator/consumer.h>
> >>
> >> +#include "../../../../arch/arm/mach-omap2/control.h"
> >> +
> >
> > #include <mach/control.h>
> >
> > (untested) ?
>
> I'm afraid it won't work. The above directory isn't in any include path
> and likely shouldn't be. That file is included locally elsewhere, I
> believe.
You're right, I spoke too fast.
> I wonder what would be the right way to access that register definition
> I need from the file:
>
> OMAP343X_CTRL_BASE
> OMAP3630_CONTROL_CAMERA_PHY_CTRL
Maybe the file (or part of it) should be moved to an include directory ?
> >> #include "isp.h"
> >> #include "ispreg.h"
> >> #include "ispcsiphy.h"
> >>
> >> @@ -138,15 +140,78 @@ static void csiphy_dphy_config(struct isp_csiphy
> >> *phy) isp_reg_writel(phy->isp, reg, phy->phy_regs, ISPCSIPHY_REG1);
> >>
> >> }
> >>
> >> -static int csiphy_config(struct isp_csiphy *phy,
> >> - struct isp_csiphy_dphy_cfg *dphy,
> >> - struct isp_csiphy_lanes_cfg *lanes)
> >> +/*
> >> + * TCLK values are OK at their reset values
> >> + */
> >> +#define TCLK_TERM 0
> >> +#define TCLK_MISS 1
> >> +#define TCLK_SETTLE 14
> >> +
> >> +int omap3isp_csiphy_config(struct isp_device *isp,
> >> + struct v4l2_subdev *csi2_subdev,
> >> + struct v4l2_subdev *sensor,
> >> + struct v4l2_mbus_framefmt *sensor_fmt)
> >>
> >> {
> >>
> >> + struct isp_v4l2_subdevs_group *subdevs = sensor->host_priv;
> >> + struct isp_csi2_device *csi2 = v4l2_get_subdevdata(csi2_subdev);
> >> + struct isp_csiphy_dphy_cfg csi2phy;
> >> + int csi2_ddrclk_khz;
> >> + struct isp_csiphy_lanes_cfg *lanes;
> >>
> >> unsigned int used_lanes = 0;
> >> unsigned int i;
> >>
> >> + u32 cam_phy_ctrl;
> >> +
> >> + if (subdevs->interface == ISP_INTERFACE_CCP2B_PHY1
> >> + || subdevs->interface == ISP_INTERFACE_CCP2B_PHY2)
> >> + lanes = subdevs->bus.ccp2.lanecfg;
> >> + else
> >> + lanes = subdevs->bus.csi2.lanecfg;
> >
> > Shouldn't lane configuration be retrieved from the sensor instead ?
> > Sensors could use different lane configuration depending on the mode.
> > This could also be implemented later when needed, but I don't think it
> > would be too difficult to get it right now.
>
> I think we'd first need to standardise the CSI-2 bus configuration. I
> don't see a practical need to make the lane configuration dynamic. You
> could just use a lower frequency to achieve the same if you really need to.
>
> Ideally it might be nice to do but there's really nothing I know that
> required or even benefited from it --- at least for now.
Does this mean that lane configuration needs to be duplicated in board code,
on for the SMIA++ platform data and one of the OMAP3 ISP platform data ?
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
