Re: [Patch 07/19] media: ti-vpe: cal: add CSI2 PHY LDO errata support

[Benoit Parrot]

Hans Verkuil <hverkuil@xxxxxxxxx> wrote on Mon [2019-Oct-21 12:38:03 +0200]:
> On 10/18/19 5:34 PM, Benoit Parrot wrote:
> > Apply Errata i913 every time the functional clock is enabled.
> > This should take care of suspend/resume case as well.
> > 
> > Signed-off-by: Benoit Parrot <bparrot@xxxxxx>
> > Signed-off-by: Jyri Sarha <jsarha@xxxxxx>
> > ---
> >  drivers/media/platform/ti-vpe/cal.c      | 56 +++++++++++++++++++++++-
> >  drivers/media/platform/ti-vpe/cal_regs.h | 27 ++++++++++++
> >  2 files changed, 82 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> > index 62aeedb705e9..3cbc4dca6de8 100644
> > --- a/drivers/media/platform/ti-vpe/cal.c
> > +++ b/drivers/media/platform/ti-vpe/cal.c
> > @@ -284,6 +284,13 @@ static struct cal_data dra72x_cal_data = {
> >  	.flags = 0,
> >  };
> >  
> > +static struct cal_data dra72x_es1_cal_data = {
> > +	.csi2_phy_core = dra72x_cal_csi_phy,
> > +	.num_csi2_phy = ARRAY_SIZE(dra72x_cal_csi_phy),
> > +
> > +	.flags = DRA72_CAL_PRE_ES2_LDO_DISABLE,
> > +};
> > +
> >  /*
> >   * there is one cal_dev structure in the driver, it is shared by
> >   * all instances.
> > @@ -569,9 +576,52 @@ static void cal_get_hwinfo(struct cal_dev *dev)
> >  		hwinfo);
> >  }
> >  
> > +/*
> > + *   Errata i913: CSI2 LDO Needs to be disabled when module is powered on
> > + *
> > + *   Enabling CSI2 LDO shorts it to core supply. It is crucial the 2 CSI2
> > + *   LDOs on the device are disabled if CSI-2 module is powered on
> > + *   (0x4845 B304 | 0x4845 B384 [28:27] = 0x1) or in ULPS (0x4845 B304
> > + *   | 0x4845 B384 [28:27] = 0x2) mode. Common concerns include: high
> > + *   current draw on the module supply in active mode.
> > + *
> > + *   Errata does not apply when CSI-2 module is powered off
> > + *   (0x4845 B304 | 0x4845 B384 [28:27] = 0x0).
> > + *
> > + * SW Workaround:
> > + *	Set the following register bits to disable the LDO,
> > + *	which is essentially CSI2 REG10 bit 6:
> > + *
> > + *		Core 0:  0x4845 B828 = 0x0000 0040
> > + *		Core 1:  0x4845 B928 = 0x0000 0040
> > + */
> > +static void i913_errata(struct cal_dev *dev, unsigned int port)
> > +{
> > +	u32 reg10 = reg_read(dev->cc[port], CAL_CSI2_PHY_REG10);
> > +
> > +	set_field(&reg10, CAL_CSI2_PHY_REG0_HSCLOCKCONFIG_DISABLE,
> > +		  CAL_CSI2_PHY_REG10_I933_LDO_DISABLE_MASK);
> > +
> > +	cal_dbg(1, dev, "CSI2_%d_REG10 = 0x%08x\n", port, reg10);
> > +	reg_write(dev->cc[port], CAL_CSI2_PHY_REG10, reg10);
> > +}
> > +
> >  static inline int cal_runtime_get(struct cal_dev *dev)
> 
> I'd drop the 'inline' here. It doesn't seem appropriate anymore since this
> function is now more complex.

Ok I'll fix that

Benoit

> 
> Regards,
> 
> 	Hans
> 
> >  {
> > -	return pm_runtime_get_sync(&dev->pdev->dev);
> > +	int r;
> > +
> > +	r = pm_runtime_get_sync(&dev->pdev->dev);
> > +
> > +	if (dev->flags & DRA72_CAL_PRE_ES2_LDO_DISABLE) {
> > +		/*
> > +		 * Apply errata on both port eveytime we (re-)enable
> > +		 * the clock
> > +		 */
> > +		i913_errata(dev, 0);
> > +		i913_errata(dev, 1);
> > +	}
> > +
> > +	return r;
> >  }
> >  
> >  static inline void cal_runtime_put(struct cal_dev *dev)
> > @@ -2071,6 +2121,10 @@ static const struct of_device_id cal_of_match[] = {
> >  		.compatible = "ti,dra72-cal",
> >  		.data = (void *)&dra72x_cal_data,
> >  	},
> > +	{
> > +		.compatible = "ti,dra72-pre-es2-cal",
> > +		.data = (void *)&dra72x_es1_cal_data,
> > +	},
> >  	{},
> >  };
> >  MODULE_DEVICE_TABLE(of, cal_of_match);
> > diff --git a/drivers/media/platform/ti-vpe/cal_regs.h b/drivers/media/platform/ti-vpe/cal_regs.h
> > index 68cfc922b422..78d6f015c9ea 100644
> > --- a/drivers/media/platform/ti-vpe/cal_regs.h
> > +++ b/drivers/media/platform/ti-vpe/cal_regs.h
> > @@ -10,6 +10,30 @@
> >  #ifndef __TI_CAL_REGS_H
> >  #define __TI_CAL_REGS_H
> >  
> > +/*
> > + * struct cal_dev.flags possibilities
> > + *
> > + * DRA72_CAL_PRE_ES2_LDO_DISABLE:
> > + *   Errata i913: CSI2 LDO Needs to be disabled when module is powered on
> > + *
> > + *   Enabling CSI2 LDO shorts it to core supply. It is crucial the 2 CSI2
> > + *   LDOs on the device are disabled if CSI-2 module is powered on
> > + *   (0x4845 B304 | 0x4845 B384 [28:27] = 0x1) or in ULPS (0x4845 B304
> > + *   | 0x4845 B384 [28:27] = 0x2) mode. Common concerns include: high
> > + *   current draw on the module supply in active mode.
> > + *
> > + *   Errata does not apply when CSI-2 module is powered off
> > + *   (0x4845 B304 | 0x4845 B384 [28:27] = 0x0).
> > + *
> > + * SW Workaround:
> > + *	Set the following register bits to disable the LDO,
> > + *	which is essentially CSI2 REG10 bit 6:
> > + *
> > + *		Core 0:  0x4845 B828 = 0x0000 0040
> > + *		Core 1:  0x4845 B928 = 0x0000 0040
> > + */
> > +#define DRA72_CAL_PRE_ES2_LDO_DISABLE BIT(0)
> > +
> >  #define CAL_NUM_CSI2_PORTS		2
> >  
> >  /* CAL register offsets */
> > @@ -71,6 +95,7 @@
> >  #define CAL_CSI2_PHY_REG0		0x000
> >  #define CAL_CSI2_PHY_REG1		0x004
> >  #define CAL_CSI2_PHY_REG2		0x008
> > +#define CAL_CSI2_PHY_REG10		0x028
> >  
> >  /* CAL Control Module Core Camerrx Control register offsets */
> >  #define CM_CTRL_CORE_CAMERRX_CONTROL	0x000
> > @@ -458,6 +483,8 @@
> >  #define CAL_CSI2_PHY_REG1_CLOCK_MISS_DETECTOR_STATUS_SUCCESS		0
> >  #define CAL_CSI2_PHY_REG1_RESET_DONE_STATUS_MASK		GENMASK(29, 28)
> >  
> > +#define CAL_CSI2_PHY_REG10_I933_LDO_DISABLE_MASK		BIT_MASK(6)
> > +
> >  #define CAL_CSI2_PHY_REG2_CCP2_SYNC_PATTERN_MASK		GENMASK(23, 0)
> >  #define CAL_CSI2_PHY_REG2_TRIGGER_CMD_RXTRIGESC3_MASK		GENMASK(25, 24)
> >  #define CAL_CSI2_PHY_REG2_TRIGGER_CMD_RXTRIGESC2_MASK		GENMASK(27, 26)
> > 
>