from:"kalle.jokiniemi"

RE: [PATCH v3 2/2] OMAP3: RX-51: define vdds_csib regulator supply

2011-05-03 Thread kalle.jokiniemi

Hi,

  -Original Message-
  From: ext Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
  Sent: 3. toukokuuta 2011 13:49
  To: Jokiniemi Kalle (Nokia-SD/Tampere)
  Cc: mauroche...@gmail.com; t...@atomide.com; linux-
  o...@vger.kernel.org; linux-media@vger.kernel.org
  Subject: Re: [PATCH v3 2/2] OMAP3: RX-51: define vdds_csib regulator supply
  
  Hi Kalle,
  
  Thanks for the patch.
  
  On Tuesday 03 May 2011 12:41:23 Kalle Jokiniemi wrote:
   The RX-51 uses the CSIb IO complex for camera operation. The
   board file is missing definition for the regulator supplying
   the CSIb complex, so this is added for better power
   management.
  
   Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com
   ---
arch/arm/mach-omap2/board-rx51-peripherals.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
  
   diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
   b/arch/arm/mach-omap2/board-rx51-peripherals.c index bbcb677..2f12425
   100644
   --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
   +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
   @@ -337,6 +337,10 @@ static struct omap2_hsmmc_info mmc[] __initdata =
  {
static struct regulator_consumer_supply rx51_vmmc1_supply =
  REGULATOR_SUPPLY(vmmc, omap_hsmmc.0);
  
   +static struct regulator_consumer_supply rx51_vaux2_supply[] = {
   +  REGULATOR_SUPPLY(vdds_csib, omap3isp),
   +};
   +
  
  What about
  
  static struct regulator_consumer_supply rx51_vaux2_supply =
   REGULATOR_SUPPLY(vdds_csib, omap3isp);
  
  instead ? :-) It would be in line with the other vaux supply definitions.
  
static struct regulator_consumer_supply rx51_vaux3_supply =
  REGULATOR_SUPPLY(vmmc, omap_hsmmc.1);
  
   @@ -400,6 +404,8 @@ static struct regulator_init_data rx51_vaux2 = {
  .valid_ops_mask = REGULATOR_CHANGE_MODE
  
  | REGULATOR_CHANGE_STATUS,
  
  },
   +  .num_consumer_supplies  = 1,
   +  .consumer_supplies  = rx51_vaux2_supply,
  
  and
  
  .consumer_supplies   = rx51_vaux2_supply,
  
  here.
  
  If you're fine with that, there's no need to resubmit, I'll modify this patch
  and push the set through my tree (let me know if I can keep your SoB line 
  with
  that change).

Perfectly fine with me, you can keep the SoB line. 

Thanks,
Kalle

  
};
  
/* VAUX3 - adds more power to VIO_18 rail */
  
  --
  Regards,
  
  Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 2/2] OMAP3: RX-51: define vdds_csib regulator supply

2011-05-02 Thread kalle.jokiniemi

Hi,

  -Original Message-
  From: ext Tony Lindgren [mailto:t...@atomide.com]
  Sent: 29. huhtikuuta 2011 12:14
  To: Jokiniemi Kalle (Nokia-SD/Tampere)
  Cc: laurent.pinch...@ideasonboard.com; mche...@infradead.org; linux-
  o...@vger.kernel.org; linux-media@vger.kernel.org
  Subject: Re: [PATCH 2/2] OMAP3: RX-51: define vdds_csib regulator supply
  
  * Kalle Jokiniemi kalle.jokini...@nokia.com [110429 00:09]:
   The RX-51 uses the CSIb IO complex for camera operation. The
   board file is missing definition for the regulator supplying
   the CSIb complex, so this is added for better power
   management.
  
   Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com
  
  This looks safe to merge along with the other patch
  
  Acked-by: Tony Lindgren t...@atomide.com

Thanks Tony, adding Mauro's correct email...

- Kalle




  
   ---
arch/arm/mach-omap2/board-rx51-peripherals.c |9 +
1 files changed, 9 insertions(+), 0 deletions(-)
  
   diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
  b/arch/arm/mach-omap2/board-rx51-peripherals.c
   index bbcb677..1324ba3 100644
   --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
   +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
   @@ -337,6 +337,13 @@ static struct omap2_hsmmc_info mmc[] __initdata =
  {
static struct regulator_consumer_supply rx51_vmmc1_supply =
  REGULATOR_SUPPLY(vmmc, omap_hsmmc.0);
  
   +static struct regulator_consumer_supply rx51_vaux2_supplies[] = {
   +  REGULATOR_SUPPLY(vdds_csib, omap3isp),
   +  {
   +  .supply = vaux2,
   +  },
   +};
   +
static struct regulator_consumer_supply rx51_vaux3_supply =
  REGULATOR_SUPPLY(vmmc, omap_hsmmc.1);
  
   @@ -400,6 +407,8 @@ static struct regulator_init_data rx51_vaux2 = {
  .valid_ops_mask = REGULATOR_CHANGE_MODE
  | REGULATOR_CHANGE_STATUS,
  },
   +  .num_consumer_supplies  = ARRAY_SIZE(rx51_vaux2_supplies),
   +  .consumer_supplies  = rx51_vaux2_supplies,
};
  
/* VAUX3 - adds more power to VIO_18 rail */
   --
   1.7.1
  
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] OMAP3: ISP: Add regulator control for omap34xx

2011-05-02 Thread kalle.jokiniemi

Hi,

  -Original Message-
  From: ext Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
  Sent: 29. huhtikuuta 2011 12:49
  To: Jokiniemi Kalle (Nokia-SD/Tampere)
  Cc: t...@atomide.com; mche...@infradead.org; linux-
  o...@vger.kernel.org; linux-media@vger.kernel.org
  Subject: Re: [PATCH 1/2] OMAP3: ISP: Add regulator control for omap34xx

  Hi Kalle,

  On Friday 29 April 2011 09:11:59 Kalle Jokiniemi wrote:
   The current omap3isp driver is missing regulator handling
   for CSIb complex in omap34xx based devices. This patch
   adds a mechanism for this to the omap3isp driver.

   Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com

  Thanks for the patch.

Sent an updated one, comments on what was done below.

  The CSIb pins are multiplexed with the parallel interface cam_d[6:9] signals,
  so the driver might need to handle the vdds_csib regulator for the parallel
  interface as well. We can leave that out now though, as I'm not sure we'll
  ever see a platform that will require that.

   ---
drivers/media/video/omap3isp/ispccp2.c |   24
  +++-
drivers/media/video/omap3isp/ispccp2.h |1 +
2 files changed, 24 insertions(+), 1 deletions(-)

   diff --git a/drivers/media/video/omap3isp/ispccp2.c
   b/drivers/media/video/omap3isp/ispccp2.c index 0e16cab..3b17b0d 100644
   --- a/drivers/media/video/omap3isp/ispccp2.c
   +++ b/drivers/media/video/omap3isp/ispccp2.c
   @@ -30,6 +30,7 @@
#include linux/module.h
#include linux/mutex.h
#include linux/uaccess.h
   +#include linux/regulator/consumer.h

#include isp.h
#include ispreg.h
   @@ -163,6 +164,9 @@ static void ccp2_if_enable(struct isp_ccp2_device
   *ccp2, u8 enable) struct isp_pipeline *pipe =
   to_isp_pipeline(ccp2-subdev.entity); int i;

   +  if (enable  ccp2-vdds_csib)
   +  regulator_enable(ccp2-vdds_csib);
   +
  /* Enable/Disable all the LCx channels */
  for (i = 0; i  CCP2_LCx_CHANS_NUM; i++)
  isp_reg_clr_set(isp, OMAP3_ISP_IOMEM_CCP2,
  ISPCCP2_LCx_CTRL(i),
   @@ -186,6 +190,8 @@ static void ccp2_if_enable(struct isp_ccp2_device
   *ccp2, u8 enable) ISPCCP2_LC01_IRQENABLE,
  ISPCCP2_LC01_IRQSTATUS_LC0_FS_IRQ);
  }

  If you resubmit the patch to address the comments below, please add a blank
  line here.

Done.

   +  if (!enable  ccp2-vdds_csib)
   +  regulator_disable(ccp2-vdds_csib);
}

/*
   @@ -1137,6 +1143,10 @@ error:
 */
void omap3isp_ccp2_cleanup(struct isp_device *isp)
{
   +  struct isp_ccp2_device *ccp2 = isp-isp_ccp2;
   +
   +  if (isp-revision == ISP_REVISION_2_0)
   +  regulator_put(ccp2-vdds_csib);

  What about testing ccp2-vdds_csib != NULL here like you do above ? Not all
  ES2.0 platforms will use a regulator, so you can end up calling
  regulator_put(NULL). regulator_put() will return immediately, but the API
  doesn't allow it explictly either.

  If regulator_put(NULL) is deemed to be safe, I would remove the revision
  check
  here. If it isn't, I would replace it with a ccp2-vdds_csib != NULL check.

Regulator_put checks for NULL, so it's safe to call always.

}

/*
   @@ -1155,10 +1165,22 @@ int omap3isp_ccp2_init(struct isp_device *isp)
   * the CSI2c or CSI2a receivers. The PHY then needs to be explicitly
   * configured.
   *
   +   * On the OMAP34xx the CSI1/CCB is operated in the CSIb IO complex,

  CSI1/CCB ? Do you mean CCP ?

  The OMAP34xx has no CCP2 support anyway, so I would s,CSI1/CCB,CSI1
  receiver,.

Done.

   +   * which is powered by vdds_csib power rail. Hence the request for
   +   * the regulator.
   +   *
   * TODO: Don't hardcode the usage of PHY1 (shared with CSI2c).
   */
   -  if (isp-revision == ISP_REVISION_15_0)
   +  if (isp-revision == ISP_REVISION_15_0) {
  ccp2-phy = isp-isp_csiphy1;
   +  } else if (isp-revision == ISP_REVISION_2_0) {
   +  ccp2-vdds_csib = regulator_get(isp-dev, vdds_csib);
   +  if (IS_ERR(ccp2-vdds_csib)) {
   +  dev_dbg(isp-dev,
   +  Could not get regulator vdds_csib\n);
   +  ccp2-vdds_csib = NULL;
   +  }
   +  }

  If you resubmit your patch to address the above comments, could you please
  reorder the code (and the comment) here and put the ES2.0 check before the
  15.0 ?

Done for both the code and comments.

- Kalle

  ret = ccp2_init_entities(ccp2);
  if (ret  0)
   diff --git a/drivers/media/video/omap3isp/ispccp2.h
   b/drivers/media/video/omap3isp/ispccp2.h index 5505a86..6674e9d 100644
   --- a/drivers/media/video/omap3isp/ispccp2.h
   +++ b/drivers/media/video/omap3isp/ispccp2.h
   @@ -81,6 +81,7 @@ struct isp_ccp2_device {
  struct isp_interface_mem_config mem_cfg;
  struct isp_video video_in;
  struct isp_csiphy *phy;
   +  struct regulator *vdds_csib;
  unsigned int error;

RE: [PATCH v2 2/2] OMAP3: RX-51: define vdds_csib regulator supply

2011-05-02 Thread kalle.jokiniemi

Hi,

  -Original Message-
  From: Jokiniemi Kalle (Nokia-SD/Tampere)


--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
 +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
 @@ -337,6 +337,13 @@ static struct omap2_hsmmc_info mmc[] __initdata
  =
{
  static struct regulator_consumer_supply rx51_vmmc1_supply =
 REGULATOR_SUPPLY(vmmc, omap_hsmmc.0);

 +static struct regulator_consumer_supply rx51_vaux2_supplies[] = {
 +   REGULATOR_SUPPLY(vdds_csib, omap3isp),
 +   {
 +   .supply = vaux2,
 +   },
   
Just for my curiosity, what is the the second consumer supply (vaux2) 
  for ?
  
  I must admit, that I just copied the format from the other regulator 
  definitions.
  No idea really. Maybe those could be removed from the others as well?

Now that I looked closer, there's no such thing in the other definitions, I 
wonder
where I managed to find that. Maybe it was the older kernel I was using... I'll
update the patch and remove the vaux2 part.

- Kalle

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] OMAP3: ISP: Add regulator control for omap34xx

2011-04-29 Thread kalle.jokiniemi

Hi,

  -Original Message-
  From: ext Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
  Sent: 29. huhtikuuta 2011 12:49
  To: Jokiniemi Kalle (Nokia-SD/Tampere)
  Cc: t...@atomide.com; mche...@infradead.org; linux-
  o...@vger.kernel.org; linux-media@vger.kernel.org
  Subject: Re: [PATCH 1/2] OMAP3: ISP: Add regulator control for omap34xx

  Hi Kalle,

  On Friday 29 April 2011 09:11:59 Kalle Jokiniemi wrote:
   The current omap3isp driver is missing regulator handling
   for CSIb complex in omap34xx based devices. This patch
   adds a mechanism for this to the omap3isp driver.

   Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com

  Thanks for the patch.

No problem, thanks for the comments. Good comments, I'll spin another
patch after weekend to address the issues.

- Kalle

  The CSIb pins are multiplexed with the parallel interface cam_d[6:9] signals,
  so the driver might need to handle the vdds_csib regulator for the parallel
  interface as well. We can leave that out now though, as I'm not sure we'll
  ever see a platform that will require that.

   ---
drivers/media/video/omap3isp/ispccp2.c |   24
  +++-
drivers/media/video/omap3isp/ispccp2.h |1 +
2 files changed, 24 insertions(+), 1 deletions(-)

   diff --git a/drivers/media/video/omap3isp/ispccp2.c
   b/drivers/media/video/omap3isp/ispccp2.c index 0e16cab..3b17b0d 100644
   --- a/drivers/media/video/omap3isp/ispccp2.c
   +++ b/drivers/media/video/omap3isp/ispccp2.c
   @@ -30,6 +30,7 @@
#include linux/module.h
#include linux/mutex.h
#include linux/uaccess.h
   +#include linux/regulator/consumer.h

#include isp.h
#include ispreg.h
   @@ -163,6 +164,9 @@ static void ccp2_if_enable(struct isp_ccp2_device
   *ccp2, u8 enable) struct isp_pipeline *pipe =
   to_isp_pipeline(ccp2-subdev.entity); int i;

   +  if (enable  ccp2-vdds_csib)
   +  regulator_enable(ccp2-vdds_csib);
   +
  /* Enable/Disable all the LCx channels */
  for (i = 0; i  CCP2_LCx_CHANS_NUM; i++)
  isp_reg_clr_set(isp, OMAP3_ISP_IOMEM_CCP2,
  ISPCCP2_LCx_CTRL(i),
   @@ -186,6 +190,8 @@ static void ccp2_if_enable(struct isp_ccp2_device
   *ccp2, u8 enable) ISPCCP2_LC01_IRQENABLE,
  ISPCCP2_LC01_IRQSTATUS_LC0_FS_IRQ);
  }

  If you resubmit the patch to address the comments below, please add a blank
  line here.

   +  if (!enable  ccp2-vdds_csib)
   +  regulator_disable(ccp2-vdds_csib);
}

/*
   @@ -1137,6 +1143,10 @@ error:
 */
void omap3isp_ccp2_cleanup(struct isp_device *isp)
{
   +  struct isp_ccp2_device *ccp2 = isp-isp_ccp2;
   +
   +  if (isp-revision == ISP_REVISION_2_0)
   +  regulator_put(ccp2-vdds_csib);

  What about testing ccp2-vdds_csib != NULL here like you do above ? Not all
  ES2.0 platforms will use a regulator, so you can end up calling
  regulator_put(NULL). regulator_put() will return immediately, but the API
  doesn't allow it explictly either.

  If regulator_put(NULL) is deemed to be safe, I would remove the revision
  check
  here. If it isn't, I would replace it with a ccp2-vdds_csib != NULL check.

}

/*
   @@ -1155,10 +1165,22 @@ int omap3isp_ccp2_init(struct isp_device *isp)
   * the CSI2c or CSI2a receivers. The PHY then needs to be explicitly
   * configured.
   *
   +   * On the OMAP34xx the CSI1/CCB is operated in the CSIb IO complex,

  CSI1/CCB ? Do you mean CCP ?

  The OMAP34xx has no CCP2 support anyway, so I would s,CSI1/CCB,CSI1
  receiver,.

   +   * which is powered by vdds_csib power rail. Hence the request for
   +   * the regulator.
   +   *
   * TODO: Don't hardcode the usage of PHY1 (shared with CSI2c).
   */
   -  if (isp-revision == ISP_REVISION_15_0)
   +  if (isp-revision == ISP_REVISION_15_0) {
  ccp2-phy = isp-isp_csiphy1;
   +  } else if (isp-revision == ISP_REVISION_2_0) {
   +  ccp2-vdds_csib = regulator_get(isp-dev, vdds_csib);
   +  if (IS_ERR(ccp2-vdds_csib)) {
   +  dev_dbg(isp-dev,
   +  Could not get regulator vdds_csib\n);
   +  ccp2-vdds_csib = NULL;
   +  }
   +  }

  If you resubmit your patch to address the above comments, could you please
  reorder the code (and the comment) here and put the ES2.0 check before the
  15.0 ?

  ret = ccp2_init_entities(ccp2);
  if (ret  0)
   diff --git a/drivers/media/video/omap3isp/ispccp2.h
   b/drivers/media/video/omap3isp/ispccp2.h index 5505a86..6674e9d 100644
   --- a/drivers/media/video/omap3isp/ispccp2.h
   +++ b/drivers/media/video/omap3isp/ispccp2.h
   @@ -81,6 +81,7 @@ struct isp_ccp2_device {
  struct isp_interface_mem_config mem_cfg;
  struct isp_video video_in;
  struct isp_csiphy *phy;
   +  struct regulator *vdds_csib;
  unsigned int error;
  enum isp_pipeline_stream_state state;

[RFC] Regulator state after regulator_get

2011-04-28 Thread kalle.jokiniemi

Hello regulator FW and OMAP3 ISP fellows,

I'm currently optimizing power management for Nokia N900 MeeGo DE release,
and found an issue with how regulators are handled at boot.

The N900 uses VAUX2 regulator in OMAP3430 to power the CSIb IO complex
that is used by the camera. While implementing regulator FW support to
handle this regulator in the camera driver I noticed a problem with the
regulator init sequence:

If the device driver using the regulator does not enable and disable the
regulator after regulator_get, the regulator is left in the state that it was
after bootloader. In case of N900 this is a problem as the regulator is left
on to leak current. Of course there is the option to let regulator FW disable
all unused regulators, but this will break the N900 functionality, as the
regulator handling is not in place for many drivers. 

I found couple of solutions to this:
1. reset all regulators that have users (regulator_get is called on them) with
a regulator_enable/disable cycle within the regulator FW.
2. enable/disable the specific vdds_csib regulator in the omap3isp driver
to reset this one specific regulator to disabled state.

So, please share comments on which approach is more appropriate to take?
Or maybe there is option 3?

Here are example code for the two options (based on .37 kernel, will update
on top of appropriate tree once right solution is agreed):

Option1:

If a consumer device of a regulator gets a regulator, but
does not enable/disable it during probe, the regulator may
be left active from boot, even though it is not needed. If
it were needed it would be enabled by the consumer device.

So reset the regulator on first regulator_get call to make
sure that any regulator that has users is not left active
needlessly.

Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com
---
 drivers/regulator/core.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ba521f0..040d850 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1152,6 +1152,12 @@ found:
module_put(rdev-owner);
}
 
+   /* Reset regulator to make sure it is disabled, if not needed */
+   if (!rdev-open_count) {
+   regulator_enable(regulator);
+   regulator_disable(regulator);
+   }
+
rdev-open_count++;
if (exclusive) {
rdev-exclusive = 1;
--

Option2:

Do a enable/disable cycle of the vdds_csib regulator to make
sure it is disabled after init in case there are no other
users for it. Otherwise regulator framework may leave it
active in case it was activated by bootloader.

Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com
---
 drivers/media/video/isp/ispccp2.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/isp/ispccp2.c 
b/drivers/media/video/isp/ispccp2.c
index 2d6b014..e11957f 100644
--- a/drivers/media/video/isp/ispccp2.c
+++ b/drivers/media/video/isp/ispccp2.c
@@ -1201,6 +1201,11 @@ int isp_ccp2_init(struct isp_device *isp)
}
}
 
+   if (ccp2-vdds_csib) {
+   regulator_enable(ccp2-vdds);
+   regulator_disable(ccp2-vdds);
+   }
+
ret = isp_ccp2_init_entities(ccp2);
if (ret  0)
goto out;
--




Thanks,
Kalle

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC] Regulator state after regulator_get

2011-04-28 Thread kalle.jokiniemi

Hi,

  -Original Message-
  From: Sakari Ailus [mailto:sakari.ai...@nokia.com]
  Sent: 28. huhtikuuta 2011 12:34
  To: Jokiniemi Kalle (Nokia-SD/Tampere)
  Cc: broo...@opensource.wolfsonmicro.com; l...@slimlogic.co.uk;
  mche...@infradead.org; svarba...@mm-sol.com; saagui...@ti.com;
  grosikopu...@mm-sol.com; Zutshi Vimarsh (Nokia-SD/Helsinki); linux-
  ker...@vger.kernel.org; linux-media@vger.kernel.org; Laurent Pinchart
  Subject: Re: [RFC] Regulator state after regulator_get

  Hello,

  Jokiniemi Kalle (Nokia-SD/Tampere) wrote:
   Hello regulator FW and OMAP3 ISP fellows,

   I'm currently optimizing power management for Nokia N900 MeeGo DE
  release,
   and found an issue with how regulators are handled at boot.

   The N900 uses VAUX2 regulator in OMAP3430 to power the CSIb IO complex
   that is used by the camera. While implementing regulator FW support to
   handle this regulator in the camera driver I noticed a problem with the
   regulator init sequence:

   If the device driver using the regulator does not enable and disable the
   regulator after regulator_get, the regulator is left in the state that it 
   was
   after bootloader. In case of N900 this is a problem as the regulator is 
   left
   on to leak current. Of course there is the option to let regulator FW 
   disable
   all unused regulators, but this will break the N900 functionality, as the
   regulator handling is not in place for many drivers.

   I found couple of solutions to this:
   1. reset all regulators that have users (regulator_get is called on them) 
   with
   a regulator_enable/disable cycle within the regulator FW.
   2. enable/disable the specific vdds_csib regulator in the omap3isp driver
   to reset this one specific regulator to disabled state.

   So, please share comments on which approach is more appropriate to take?
   Or maybe there is option 3?

   Here are example code for the two options (based on .37 kernel, will update
   on top of appropriate tree once right solution is agreed):

   Option1:

   If a consumer device of a regulator gets a regulator, but
   does not enable/disable it during probe, the regulator may
   be left active from boot, even though it is not needed. If
   it were needed it would be enabled by the consumer device.

   So reset the regulator on first regulator_get call to make
   sure that any regulator that has users is not left active
   needlessly.

  I'm not an expert in the regulator framework, but I'd think that one
  should be able to assume a regulator is in a sane state after boot. The
  fact that the regulator is enabled when it has no users should likely be
  fixed in the boot loader. Is that an option?

Not an option, the device has shipped and there will be no updates to
the bootloader.

  Does the problem exist in other boards beyond N900?

Don't know, I currently have only N900 to test with.

  Another alternative to the first option you proposed could be to add a
  flags field to regulator_consumer_supply, and use a flag to recognise
  regulators which need to be disabled during initialisation. The flag
  could be set by using a new macro e.g. REGULATOR_SUPPLY_NASTY() when
  defining the regulator.

This sounds like a good option actually. Liam, Mark, any opinions?

- Kalle

  Just my 0,05 euros. ;-)

  Cc Laurent Pinchart.

  Regards,

  --
  Sakari Ailus
  sakari.ai...@maxwell.research.nokia.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC] Regulator state after regulator_get

2011-04-28 Thread kalle.jokiniemi

Hi,

  -Original Message-
  From: ext Mark Brown [mailto:broo...@opensource.wolfsonmicro.com]
  Sent: 28. huhtikuuta 2011 13:06
  To: Jokiniemi Kalle (Nokia-SD/Tampere)
  Cc: l...@slimlogic.co.uk; mche...@infradead.org; svarba...@mm-sol.com;
  saagui...@ti.com; grosikopu...@mm-sol.com; Zutshi Vimarsh (Nokia-
  SD/Helsinki); Ailus Sakari (Nokia-SD/Helsinki); linux-ker...@vger.kernel.org;
  linux-media@vger.kernel.org
  Subject: Re: [RFC] Regulator state after regulator_get

  On Thu, Apr 28, 2011 at 09:01:03AM +, kalle.jokini...@nokia.com wrote:

   If the device driver using the regulator does not enable and disable the
   regulator after regulator_get, the regulator is left in the state that it 
   was
   after bootloader. In case of N900 this is a problem as the regulator is 
   left
   on to leak current. Of course there is the option to let regulator FW 
   disable
   all unused regulators, but this will break the N900 functionality, as the
   regulator handling is not in place for many drivers.

  You should use regulator_full_constraints() if your board has a fully
  described set of regulators.  This will cause the framework to power
  down any regulators which aren't in use after init has completed.  If
  you have some regulators with no consumers or missing consumers you need
  to mark them as always_on in their constraints.

I don't have a full set of regulators described, that's why things broke when
I tried the regulator_full_constraints call earlier. But I don't think it would 
be too
big issue to check the current after boot configuration and define all the
regulators as you suggest. I will try this approach.

   So reset the regulator on first regulator_get call to make
   sure that any regulator that has users is not left active
   needlessly.

  This would cause lots of breakage, it would mean that all regulators
  that aren't always_on would get bounced off at least once during startup
  - that's not going to be great for things like the backlight.

OK, this is not a viable solution.

Thanks for the comments,
Kalle

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC] Regulator state after regulator_get

2011-04-28 Thread kalle.jokiniemi

  -Original Message-
  From: ext Mark Brown [mailto:broo...@opensource.wolfsonmicro.com]
  Sent: 28. huhtikuuta 2011 14:15
  To: Jokiniemi Kalle (Nokia-SD/Tampere)
  Cc: l...@slimlogic.co.uk; mche...@infradead.org; svarba...@mm-sol.com;
  saagui...@ti.com; grosikopu...@mm-sol.com; Zutshi Vimarsh (Nokia-
  SD/Helsinki); Ailus Sakari (Nokia-SD/Helsinki); linux-ker...@vger.kernel.org;
  linux-media@vger.kernel.org
  Subject: Re: [RFC] Regulator state after regulator_get

  On Thu, Apr 28, 2011 at 11:08:08AM +, kalle.jokini...@nokia.com wrote:

   I don't have a full set of regulators described, that's why things broke 
   when
   I tried the regulator_full_constraints call earlier. But I don't think it 
   would be
  too
   big issue to check the current after boot configuration and define all the
   regulators as you suggest. I will try this approach.

  As I said in my previous mail if you've got a reagulator you don't know
  about which is on you can give it an always_on constraint until you
  figure out what (if anything) should be controlling it.

Yeps, I plan to check the PMIC for the regulators that are left on after boot
In the current working setup and then put those always_on flags on those
regulators in the final setup.

Thanks for the help!

- Kalle
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v3 2/2] OMAP3: RX-51: define vdds_csib regulator supply

RE: [PATCH 2/2] OMAP3: RX-51: define vdds_csib regulator supply

RE: [PATCH 1/2] OMAP3: ISP: Add regulator control for omap34xx

RE: [PATCH v2 2/2] OMAP3: RX-51: define vdds_csib regulator supply

RE: [PATCH 1/2] OMAP3: ISP: Add regulator control for omap34xx

[RFC] Regulator state after regulator_get

RE: [RFC] Regulator state after regulator_get

RE: [RFC] Regulator state after regulator_get

RE: [RFC] Regulator state after regulator_get

9 matches

Site Navigation

Mail list logo

Footer information