[RFC] omap3isp: add support for CSI1 bus

Message ID 20170306075659.GB23509@amd (mailing list archive)
State RFC, archived
Delegated to: Laurent Pinchart
Headers

Commit Message

Pavel Machek March 6, 2017, 7:57 a.m. UTC
  omap3isp: add rest of CSI1 support

CSI1 needs one more bit to be set up. Do just that.

Signed-off-by: Pavel Machek <pavel@ucw.cz>

---

Hmm. Looking at that... num_data_lanes probably should be modified in
local variable, not globally like this. Should I do that?

Anything else that needs fixing?

index 24a9fc5..6feba36 100644
  

Comments

Pavel Machek March 10, 2017, 1:41 p.m. UTC | #1
On Mon 2017-03-06 08:56:59, Pavel Machek wrote:
> omap3isp: add rest of CSI1 support
> 
> CSI1 needs one more bit to be set up. Do just that.
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> ---
> 
> Hmm. Looking at that... num_data_lanes probably should be modified in
> local variable, not globally like this. Should I do that?
> 
> Anything else that needs fixing?

Ping? Feedback here would be nice. This is last "interesting" piece of
the hardware support...

Best regards,
								Pavel

> index 24a9fc5..6feba36 100644
> --- a/drivers/media/platform/omap3isp/ispccp2.c
> +++ b/drivers/media/platform/omap3isp/ispccp2.c
> @@ -21,6 +23,7 @@
>  #include <linux/mutex.h>
>  #include <linux/uaccess.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
>  
>  #include "isp.h"
>  #include "ispreg.h"
> @@ -1149,6 +1152,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..cd6351b 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> @@ -197,9 +200,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 +306,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:
>
  
Pavel Machek April 11, 2017, 6:15 p.m. UTC | #2
On Fri 2017-03-10 14:41:31, Pavel Machek wrote:
> On Mon 2017-03-06 08:56:59, Pavel Machek wrote:
> > omap3isp: add rest of CSI1 support
> > 
> > CSI1 needs one more bit to be set up. Do just that.
> > 
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > 
> > ---
> > 
> > Hmm. Looking at that... num_data_lanes probably should be modified in
> > local variable, not globally like this. Should I do that?
> > 
> > Anything else that needs fixing?
> 
> Ping? Feedback here would be nice. This is last "interesting" piece of
> the hardware support...

Any news here? You complained that I was not pushy enough in the past
;-).

								Pavel
								
> > index 24a9fc5..6feba36 100644
> > --- a/drivers/media/platform/omap3isp/ispccp2.c
> > +++ b/drivers/media/platform/omap3isp/ispccp2.c
> > @@ -21,6 +23,7 @@
> >  #include <linux/mutex.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/regulator/consumer.h>
> > +#include <linux/regmap.h>
> >  
> >  #include "isp.h"
> >  #include "ispreg.h"
> > @@ -1149,6 +1152,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..cd6351b 100644
> > --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> > +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> > @@ -197,9 +200,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 +306,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:
> > 
> 
> 
>
  

Patch

--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -21,6 +23,7 @@ 
 #include <linux/mutex.h>
 #include <linux/uaccess.h>
 #include <linux/regulator/consumer.h>
+#include <linux/regmap.h>
 
 #include "isp.h"
 #include "ispreg.h"
@@ -1149,6 +1152,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..cd6351b 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -197,9 +200,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 +306,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: