linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] s5p-csis driver updates for 3.1
@ 2011-07-06 17:13 Sylwester Nawrocki
  2011-07-06 17:13 ` [PATCH 1/3] s5p-csis: Handle all available power supplies Sylwester Nawrocki
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Sylwester Nawrocki @ 2011-07-06 17:13 UTC (permalink / raw)
  To: linux-media
  Cc: m.szyprowski, kyungmin.park, laurent.pinchart, s.nawrocki,
	sw0312.kim, riverful.kim

The following are a few updates for s5p-csis MIPI-CSI receiver driver.
I have originally missed the fact there are multiple voltages to be
supplied externally for the MIPI-CSI receiver and PHY and the driver
have to make sure they are all enabled at PMIC. The patch 1/3 fixes
it by adding a relevant regulator supply.
Patch 2/3 fixes issues with resume from suspend to memory. Finally
patch 3/3 enables a device node for s5p-mipi-csis subdev as 
it is more flexible to control the subdev from library/application
rather than doing this in the kernel.


Sylwester Nawrocki (3):
  s5p-csis: Handle all available power supplies
  s5p-csis: Rework of the system suspend/resume helpers
  s5p-csis: Enable v4l subdev device node

 drivers/media/video/s5p-fimc/mipi-csis.c |   88 +++++++++++++++++-------------
 1 files changed, 50 insertions(+), 38 deletions(-)


Regards,
-
Sylwester Nawrocki
Samsung Poland R&D Center


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] s5p-csis: Handle all available power supplies
  2011-07-06 17:13 [PATCH 0/3] s5p-csis driver updates for 3.1 Sylwester Nawrocki
@ 2011-07-06 17:13 ` Sylwester Nawrocki
  2011-07-06 21:47   ` Laurent Pinchart
  2011-07-06 17:13 ` [PATCH 2/3] s5p-csis: Rework of the system suspend/resume helpers Sylwester Nawrocki
  2011-07-06 17:13 ` [PATCH 3/3] s5p-csis: Enable v4l subdev device node Sylwester Nawrocki
  2 siblings, 1 reply; 7+ messages in thread
From: Sylwester Nawrocki @ 2011-07-06 17:13 UTC (permalink / raw)
  To: linux-media
  Cc: m.szyprowski, kyungmin.park, laurent.pinchart, s.nawrocki,
	sw0312.kim, riverful.kim

On the SoCs this driver is intended to support the are three
separate pins to supply the MIPI-CSIS subsystem: 1.1V or 1.2V,
1.8V and power supply for an internal PLL.
This patch adds support for two separate voltage supplies
to cover properly board configurations where PMIC requires
to configure independently each external supply of the MIPI-CSI
device. The 1.8V and PLL supply are assigned a single "vdd18"
regulator supply as it seems more reasonable than creating
separate regulator supplies for them.

Reported-by: HeungJun Kim <riverful.kim@samsung.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/s5p-fimc/mipi-csis.c |   42 +++++++++++++++++------------
 1 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/media/video/s5p-fimc/mipi-csis.c b/drivers/media/video/s5p-fimc/mipi-csis.c
index ef056d6..4a529b4 100644
--- a/drivers/media/video/s5p-fimc/mipi-csis.c
+++ b/drivers/media/video/s5p-fimc/mipi-csis.c
@@ -81,6 +81,12 @@ static char *csi_clock_name[] = {
 };
 #define NUM_CSIS_CLOCKS	ARRAY_SIZE(csi_clock_name)
 
+static const char * const csis_supply_name[] = {
+	"vdd11", /* 1.1V or 1.2V (s5pc100) MIPI CSI suppply */
+	"vdd18", /* VDD 1.8V and MIPI CSI PLL supply */
+};
+#define CSIS_NUM_SUPPLIES ARRAY_SIZE(csis_supply_name)
+
 enum {
 	ST_POWERED	= 1,
 	ST_STREAMING	= 2,
@@ -109,9 +115,9 @@ struct csis_state {
 	struct platform_device *pdev;
 	struct resource *regs_res;
 	void __iomem *regs;
+	struct regulator_bulk_data supply[CSIS_NUM_SUPPLIES];
 	struct clk *clock[NUM_CSIS_CLOCKS];
 	int irq;
-	struct regulator *supply;
 	u32 flags;
 	const struct csis_pix_format *csis_fmt;
 	struct v4l2_mbus_framefmt format;
@@ -460,6 +466,7 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev)
 	struct resource *regs_res;
 	struct csis_state *state;
 	int ret = -ENOMEM;
+	int i;
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
 	if (!state)
@@ -519,13 +526,14 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev)
 		goto e_clkput;
 	}
 
+	for (i = 0; i < CSIS_NUM_SUPPLIES; i++)
+		state->supply[i].supply = csis_supply_name[i];
+
 	if (!pdata->fixed_phy_vdd) {
-		state->supply = regulator_get(&pdev->dev, "vdd");
-		if (IS_ERR(state->supply)) {
-			ret = PTR_ERR(state->supply);
-			state->supply = NULL;
+		ret = regulator_bulk_get(&pdev->dev, CSIS_NUM_SUPPLIES,
+					 state->supply);
+		if (ret)
 			goto e_clkput;
-		}
 	}
 
 	ret = request_irq(state->irq, s5pcsis_irq_handler, 0,
@@ -561,8 +569,7 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev)
 e_irqfree:
 	free_irq(state->irq, state);
 e_regput:
-	if (state->supply)
-		regulator_put(state->supply);
+	regulator_bulk_free(CSIS_NUM_SUPPLIES, state->supply);
 e_clkput:
 	clk_disable(state->clock[CSIS_CLK_MUX]);
 	s5pcsis_clk_put(state);
@@ -592,8 +599,9 @@ static int s5pcsis_suspend(struct device *dev)
 		ret = pdata->phy_enable(state->pdev, false);
 		if (ret)
 			goto unlock;
-		if (state->supply) {
-			ret = regulator_disable(state->supply);
+		if (!pdata->fixed_phy_vdd) {
+			ret = regulator_bulk_disable(CSIS_NUM_SUPPLIES,
+						     state->supply);
 			if (ret)
 				goto unlock;
 		}
@@ -622,16 +630,17 @@ static int s5pcsis_resume(struct device *dev)
 		goto unlock;
 
 	if (!(state->flags & ST_POWERED)) {
-		if (state->supply)
-			ret = regulator_enable(state->supply);
+		if (!pdata->fixed_phy_vdd)
+			ret = regulator_bulk_enable(CSIS_NUM_SUPPLIES,
+						    state->supply);
 		if (ret)
 			goto unlock;
-
 		ret = pdata->phy_enable(state->pdev, true);
 		if (!ret) {
 			state->flags |= ST_POWERED;
-		} else if (state->supply) {
-			regulator_disable(state->supply);
+		} else if (!pdata->fixed_phy_vdd) {
+			regulator_bulk_disable(CSIS_NUM_SUPPLIES,
+					       state->supply);
 			goto unlock;
 		}
 		clk_enable(state->clock[CSIS_CLK_GATE]);
@@ -679,8 +688,7 @@ static int __devexit s5pcsis_remove(struct platform_device *pdev)
 	pm_runtime_set_suspended(&pdev->dev);
 
 	s5pcsis_clk_put(state);
-	if (state->supply)
-		regulator_put(state->supply);
+	regulator_bulk_free(CSIS_NUM_SUPPLIES, state->supply);
 
 	media_entity_cleanup(&state->sd.entity);
 	free_irq(state->irq, state);
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] s5p-csis: Rework of the system suspend/resume helpers
  2011-07-06 17:13 [PATCH 0/3] s5p-csis driver updates for 3.1 Sylwester Nawrocki
  2011-07-06 17:13 ` [PATCH 1/3] s5p-csis: Handle all available power supplies Sylwester Nawrocki
@ 2011-07-06 17:13 ` Sylwester Nawrocki
  2011-07-06 17:13 ` [PATCH 3/3] s5p-csis: Enable v4l subdev device node Sylwester Nawrocki
  2 siblings, 0 replies; 7+ messages in thread
From: Sylwester Nawrocki @ 2011-07-06 17:13 UTC (permalink / raw)
  To: linux-media
  Cc: m.szyprowski, kyungmin.park, laurent.pinchart, s.nawrocki,
	sw0312.kim, riverful.kim

Do not resume the device during system resume if it was idle
before system suspend, as this causes resume from suspend
to RAM failures on exynos4. For this purpose runtime PM and
system sleep helpers are separated.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/s5p-fimc/mipi-csis.c |   45 ++++++++++++++++--------------
 1 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/media/video/s5p-fimc/mipi-csis.c b/drivers/media/video/s5p-fimc/mipi-csis.c
index 4a529b4..5b38be7 100644
--- a/drivers/media/video/s5p-fimc/mipi-csis.c
+++ b/drivers/media/video/s5p-fimc/mipi-csis.c
@@ -561,7 +561,6 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev)
 	/* .. and a pointer to the subdev. */
 	platform_set_drvdata(pdev, &state->sd);
 
-	state->flags = ST_SUSPENDED;
 	pm_runtime_enable(&pdev->dev);
 
 	return 0;
@@ -582,7 +581,7 @@ e_free:
 	return ret;
 }
 
-static int s5pcsis_suspend(struct device *dev)
+static int s5pcsis_pm_suspend(struct device *dev, bool runtime)
 {
 	struct s5p_platform_mipi_csis *pdata = dev->platform_data;
 	struct platform_device *pdev = to_platform_device(dev);
@@ -607,14 +606,15 @@ static int s5pcsis_suspend(struct device *dev)
 		}
 		clk_disable(state->clock[CSIS_CLK_GATE]);
 		state->flags &= ~ST_POWERED;
+		if (!runtime)
+			state->flags |= ST_SUSPENDED;
 	}
-	state->flags |= ST_SUSPENDED;
  unlock:
 	mutex_unlock(&state->lock);
 	return ret ? -EAGAIN : 0;
 }
 
-static int s5pcsis_resume(struct device *dev)
+static int s5pcsis_pm_resume(struct device *dev, bool runtime)
 {
 	struct s5p_platform_mipi_csis *pdata = dev->platform_data;
 	struct platform_device *pdev = to_platform_device(dev);
@@ -626,7 +626,7 @@ static int s5pcsis_resume(struct device *dev)
 		 __func__, state->flags);
 
 	mutex_lock(&state->lock);
-	if (!(state->flags & ST_SUSPENDED))
+	if (!runtime && !(state->flags & ST_SUSPENDED))
 		goto unlock;
 
 	if (!(state->flags & ST_POWERED)) {
@@ -647,32 +647,34 @@ static int s5pcsis_resume(struct device *dev)
 	}
 	if (state->flags & ST_STREAMING)
 		s5pcsis_start_stream(state);
-
-	state->flags &= ~ST_SUSPENDED;
+	if (!runtime)
+		state->flags &= ~ST_SUSPENDED;
  unlock:
 	mutex_unlock(&state->lock);
 	return ret ? -EAGAIN : 0;
 }
 
 #ifdef CONFIG_PM_SLEEP
-static int s5pcsis_pm_suspend(struct device *dev)
+static int s5pcsis_suspend(struct device *dev)
 {
-	return s5pcsis_suspend(dev);
+	return s5pcsis_pm_suspend(dev, false);
 }
 
-static int s5pcsis_pm_resume(struct device *dev)
+static int s5pcsis_resume(struct device *dev)
 {
-	int ret;
-
-	ret = s5pcsis_resume(dev);
+	return s5pcsis_pm_resume(dev, false);
+}
+#endif
 
-	if (!ret) {
-		pm_runtime_disable(dev);
-		ret = pm_runtime_set_active(dev);
-		pm_runtime_enable(dev);
-	}
+#ifdef CONFIG_PM_RUNTIME
+static int s5pcsis_runtime_suspend(struct device *dev)
+{
+	return s5pcsis_pm_suspend(dev, true);
+}
 
-	return ret;
+static int s5pcsis_runtime_resume(struct device *dev)
+{
+	return s5pcsis_pm_resume(dev, true);
 }
 #endif
 
@@ -700,8 +702,9 @@ static int __devexit s5pcsis_remove(struct platform_device *pdev)
 }
 
 static const struct dev_pm_ops s5pcsis_pm_ops = {
-	SET_RUNTIME_PM_OPS(s5pcsis_suspend, s5pcsis_resume, NULL)
-	SET_SYSTEM_SLEEP_PM_OPS(s5pcsis_pm_suspend, s5pcsis_pm_resume)
+	SET_RUNTIME_PM_OPS(s5pcsis_runtime_suspend, s5pcsis_runtime_resume,
+			   NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(s5pcsis_suspend, s5pcsis_resume)
 };
 
 static struct platform_driver s5pcsis_driver = {
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] s5p-csis: Enable v4l subdev device node
  2011-07-06 17:13 [PATCH 0/3] s5p-csis driver updates for 3.1 Sylwester Nawrocki
  2011-07-06 17:13 ` [PATCH 1/3] s5p-csis: Handle all available power supplies Sylwester Nawrocki
  2011-07-06 17:13 ` [PATCH 2/3] s5p-csis: Rework of the system suspend/resume helpers Sylwester Nawrocki
@ 2011-07-06 17:13 ` Sylwester Nawrocki
  2 siblings, 0 replies; 7+ messages in thread
From: Sylwester Nawrocki @ 2011-07-06 17:13 UTC (permalink / raw)
  To: linux-media
  Cc: m.szyprowski, kyungmin.park, laurent.pinchart, s.nawrocki,
	sw0312.kim, riverful.kim

Set v4l2_subdev flags for a host driver to create a sub-device
node for the driver so the subdev can be directly configured
by applications.

Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/media/video/s5p-fimc/mipi-csis.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/s5p-fimc/mipi-csis.c b/drivers/media/video/s5p-fimc/mipi-csis.c
index 5b38be7..180abd6 100644
--- a/drivers/media/video/s5p-fimc/mipi-csis.c
+++ b/drivers/media/video/s5p-fimc/mipi-csis.c
@@ -546,6 +546,7 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev)
 	v4l2_subdev_init(&state->sd, &s5pcsis_subdev_ops);
 	state->sd.owner = THIS_MODULE;
 	strlcpy(state->sd.name, dev_name(&pdev->dev), sizeof(state->sd.name));
+	state->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	state->csis_fmt = &s5pcsis_formats[0];
 
 	state->pads[CSIS_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] s5p-csis: Handle all available power supplies
  2011-07-06 17:13 ` [PATCH 1/3] s5p-csis: Handle all available power supplies Sylwester Nawrocki
@ 2011-07-06 21:47   ` Laurent Pinchart
  2011-07-07 13:34     ` Sylwester Nawrocki
  0 siblings, 1 reply; 7+ messages in thread
From: Laurent Pinchart @ 2011-07-06 21:47 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-media, m.szyprowski, kyungmin.park, sw0312.kim, riverful.kim

Hi Sylwester,

On Wednesday 06 July 2011 19:13:39 Sylwester Nawrocki wrote:
> On the SoCs this driver is intended to support the are three
> separate pins to supply the MIPI-CSIS subsystem: 1.1V or 1.2V,
> 1.8V and power supply for an internal PLL.
> This patch adds support for two separate voltage supplies
> to cover properly board configurations where PMIC requires
> to configure independently each external supply of the MIPI-CSI
> device. The 1.8V and PLL supply are assigned a single "vdd18"
> regulator supply as it seems more reasonable than creating
> separate regulator supplies for them.
> 
> Reported-by: HeungJun Kim <riverful.kim@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/video/s5p-fimc/mipi-csis.c |   42
> +++++++++++++++++------------ 1 files changed, 25 insertions(+), 17
> deletions(-)
> 
> diff --git a/drivers/media/video/s5p-fimc/mipi-csis.c
> b/drivers/media/video/s5p-fimc/mipi-csis.c index ef056d6..4a529b4 100644
> --- a/drivers/media/video/s5p-fimc/mipi-csis.c
> +++ b/drivers/media/video/s5p-fimc/mipi-csis.c
> @@ -81,6 +81,12 @@ static char *csi_clock_name[] = {
>  };
>  #define NUM_CSIS_CLOCKS	ARRAY_SIZE(csi_clock_name)
> 
> +static const char * const csis_supply_name[] = {
> +	"vdd11", /* 1.1V or 1.2V (s5pc100) MIPI CSI suppply */
> +	"vdd18", /* VDD 1.8V and MIPI CSI PLL supply */
> +};
> +#define CSIS_NUM_SUPPLIES ARRAY_SIZE(csis_supply_name)
> +
>  enum {
>  	ST_POWERED	= 1,
>  	ST_STREAMING	= 2,
> @@ -109,9 +115,9 @@ struct csis_state {
>  	struct platform_device *pdev;
>  	struct resource *regs_res;
>  	void __iomem *regs;
> +	struct regulator_bulk_data supply[CSIS_NUM_SUPPLIES];

I would have called this supplies, but that's nitpicking. Otherwise the patch 
looks good to me.

>  	struct clk *clock[NUM_CSIS_CLOCKS];
>  	int irq;
> -	struct regulator *supply;
>  	u32 flags;
>  	const struct csis_pix_format *csis_fmt;
>  	struct v4l2_mbus_framefmt format;
> @@ -460,6 +466,7 @@ static int __devinit s5pcsis_probe(struct
> platform_device *pdev) struct resource *regs_res;
>  	struct csis_state *state;
>  	int ret = -ENOMEM;
> +	int i;
> 
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
>  	if (!state)
> @@ -519,13 +526,14 @@ static int __devinit s5pcsis_probe(struct
> platform_device *pdev) goto e_clkput;
>  	}
> 
> +	for (i = 0; i < CSIS_NUM_SUPPLIES; i++)
> +		state->supply[i].supply = csis_supply_name[i];
> +
>  	if (!pdata->fixed_phy_vdd) {
> -		state->supply = regulator_get(&pdev->dev, "vdd");
> -		if (IS_ERR(state->supply)) {
> -			ret = PTR_ERR(state->supply);
> -			state->supply = NULL;
> +		ret = regulator_bulk_get(&pdev->dev, CSIS_NUM_SUPPLIES,
> +					 state->supply);
> +		if (ret)
>  			goto e_clkput;
> -		}
>  	}
> 
>  	ret = request_irq(state->irq, s5pcsis_irq_handler, 0,
> @@ -561,8 +569,7 @@ static int __devinit s5pcsis_probe(struct
> platform_device *pdev) e_irqfree:
>  	free_irq(state->irq, state);
>  e_regput:
> -	if (state->supply)
> -		regulator_put(state->supply);
> +	regulator_bulk_free(CSIS_NUM_SUPPLIES, state->supply);
>  e_clkput:
>  	clk_disable(state->clock[CSIS_CLK_MUX]);
>  	s5pcsis_clk_put(state);
> @@ -592,8 +599,9 @@ static int s5pcsis_suspend(struct device *dev)
>  		ret = pdata->phy_enable(state->pdev, false);
>  		if (ret)
>  			goto unlock;
> -		if (state->supply) {
> -			ret = regulator_disable(state->supply);
> +		if (!pdata->fixed_phy_vdd) {
> +			ret = regulator_bulk_disable(CSIS_NUM_SUPPLIES,
> +						     state->supply);
>  			if (ret)
>  				goto unlock;
>  		}
> @@ -622,16 +630,17 @@ static int s5pcsis_resume(struct device *dev)
>  		goto unlock;
> 
>  	if (!(state->flags & ST_POWERED)) {
> -		if (state->supply)
> -			ret = regulator_enable(state->supply);
> +		if (!pdata->fixed_phy_vdd)
> +			ret = regulator_bulk_enable(CSIS_NUM_SUPPLIES,
> +						    state->supply);
>  		if (ret)
>  			goto unlock;
> -
>  		ret = pdata->phy_enable(state->pdev, true);
>  		if (!ret) {
>  			state->flags |= ST_POWERED;
> -		} else if (state->supply) {
> -			regulator_disable(state->supply);
> +		} else if (!pdata->fixed_phy_vdd) {
> +			regulator_bulk_disable(CSIS_NUM_SUPPLIES,
> +					       state->supply);
>  			goto unlock;
>  		}
>  		clk_enable(state->clock[CSIS_CLK_GATE]);
> @@ -679,8 +688,7 @@ static int __devexit s5pcsis_remove(struct
> platform_device *pdev) pm_runtime_set_suspended(&pdev->dev);
> 
>  	s5pcsis_clk_put(state);
> -	if (state->supply)
> -		regulator_put(state->supply);
> +	regulator_bulk_free(CSIS_NUM_SUPPLIES, state->supply);
> 
>  	media_entity_cleanup(&state->sd.entity);
>  	free_irq(state->irq, state);

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] s5p-csis: Handle all available power supplies
  2011-07-06 21:47   ` Laurent Pinchart
@ 2011-07-07 13:34     ` Sylwester Nawrocki
  2011-08-25 11:43       ` [PATCH v2] " Sylwester Nawrocki
  0 siblings, 1 reply; 7+ messages in thread
From: Sylwester Nawrocki @ 2011-07-07 13:34 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, m.szyprowski, kyungmin.park, sw0312.kim, riverful.kim

Hi Laurent,

On 07/06/2011 11:47 PM, Laurent Pinchart wrote:
> Hi Sylwester,
> 
> On Wednesday 06 July 2011 19:13:39 Sylwester Nawrocki wrote:
>> On the SoCs this driver is intended to support the are three
>> separate pins to supply the MIPI-CSIS subsystem: 1.1V or 1.2V,
>> 1.8V and power supply for an internal PLL.
>> This patch adds support for two separate voltage supplies
>> to cover properly board configurations where PMIC requires
>> to configure independently each external supply of the MIPI-CSI
>> device. The 1.8V and PLL supply are assigned a single "vdd18"
>> regulator supply as it seems more reasonable than creating
>> separate regulator supplies for them.
>>
>> Reported-by: HeungJun Kim <riverful.kim@samsung.com>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  drivers/media/video/s5p-fimc/mipi-csis.c |   42
>> +++++++++++++++++------------ 1 files changed, 25 insertions(+), 17
>> deletions(-)
>>
>> diff --git a/drivers/media/video/s5p-fimc/mipi-csis.c
>> b/drivers/media/video/s5p-fimc/mipi-csis.c index ef056d6..4a529b4 100644
>> --- a/drivers/media/video/s5p-fimc/mipi-csis.c
>> +++ b/drivers/media/video/s5p-fimc/mipi-csis.c
>> @@ -81,6 +81,12 @@ static char *csi_clock_name[] = {
>>  };
>>  #define NUM_CSIS_CLOCKS	ARRAY_SIZE(csi_clock_name)
>>
>> +static const char * const csis_supply_name[] = {
>> +	"vdd11", /* 1.1V or 1.2V (s5pc100) MIPI CSI suppply */
>> +	"vdd18", /* VDD 1.8V and MIPI CSI PLL supply */
>> +};
>> +#define CSIS_NUM_SUPPLIES ARRAY_SIZE(csis_supply_name)
>> +
>>  enum {
>>  	ST_POWERED	= 1,
>>  	ST_STREAMING	= 2,
>> @@ -109,9 +115,9 @@ struct csis_state {
>>  	struct platform_device *pdev;
>>  	struct resource *regs_res;
>>  	void __iomem *regs;
>> +	struct regulator_bulk_data supply[CSIS_NUM_SUPPLIES];
> 
> I would have called this supplies, but that's nitpicking. Otherwise the patch 
> looks good to me.

No problem, I have already renamed it. It seems to make more sense like this.
Thanks for the review!


Regards,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] s5p-csis: Handle all available power supplies
  2011-07-07 13:34     ` Sylwester Nawrocki
@ 2011-08-25 11:43       ` Sylwester Nawrocki
  0 siblings, 0 replies; 7+ messages in thread
From: Sylwester Nawrocki @ 2011-08-25 11:43 UTC (permalink / raw)
  To: linux-media
  Cc: m.szyprowski, kyungmin.park, sw0312.kim, riverful.kim, s.nawrocki

On the SoCs this driver is intended to support the are three separate
pins to supply the MIPI-CSIS subsystem: 1.1V or 1.2V, 1.8V and power
supply for an internal PLL.
This patch adds support for two separate voltage supplies to cover
properly board configurations where PMIC requires to configure
independently each external supply of the MIPI-CSI device. The 1.8V
and PLL supply are assigned a single "vdd18" regulator supply name
as it seems more reasonable than creating separate regulator supplies
for them.

While at here stop using the 'fixed_phy_vdd' platform_data field.
It has been introduced for boards where the MIPI-CSIS supplies are
not controllable. However it is not needed as those boards can use
the dummy regulator.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
There is a minor change introduced in this patch comparing to the first
version - the not really needed fixed_phy_vdd platform data property is
killed which simplifies the code a bit. The struct s5p_platform_mipi_csis
definition will be cleaned in a subsequent patch.

---
 drivers/media/video/s5p-fimc/mipi-csis.c |   49 ++++++++++++++++--------------
 1 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/media/video/s5p-fimc/mipi-csis.c b/drivers/media/video/s5p-fimc/mipi-csis.c
index ef056d6..e34d4ba 100644
--- a/drivers/media/video/s5p-fimc/mipi-csis.c
+++ b/drivers/media/video/s5p-fimc/mipi-csis.c
@@ -81,6 +81,12 @@ static char *csi_clock_name[] = {
 };
 #define NUM_CSIS_CLOCKS	ARRAY_SIZE(csi_clock_name)
 
+static const char * const csis_supply_name[] = {
+	"vdd11", /* 1.1V or 1.2V (s5pc100) MIPI CSI suppply */
+	"vdd18", /* VDD 1.8V and MIPI CSI PLL supply */
+};
+#define CSIS_NUM_SUPPLIES ARRAY_SIZE(csis_supply_name)
+
 enum {
 	ST_POWERED	= 1,
 	ST_STREAMING	= 2,
@@ -109,9 +115,9 @@ struct csis_state {
 	struct platform_device *pdev;
 	struct resource *regs_res;
 	void __iomem *regs;
+	struct regulator_bulk_data supplies[CSIS_NUM_SUPPLIES];
 	struct clk *clock[NUM_CSIS_CLOCKS];
 	int irq;
-	struct regulator *supply;
 	u32 flags;
 	const struct csis_pix_format *csis_fmt;
 	struct v4l2_mbus_framefmt format;
@@ -460,6 +466,7 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev)
 	struct resource *regs_res;
 	struct csis_state *state;
 	int ret = -ENOMEM;
+	int i;
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
 	if (!state)
@@ -519,14 +526,13 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev)
 		goto e_clkput;
 	}
 
-	if (!pdata->fixed_phy_vdd) {
-		state->supply = regulator_get(&pdev->dev, "vdd");
-		if (IS_ERR(state->supply)) {
-			ret = PTR_ERR(state->supply);
-			state->supply = NULL;
-			goto e_clkput;
-		}
-	}
+	for (i = 0; i < CSIS_NUM_SUPPLIES; i++)
+		state->supplies[i].supply = csis_supply_name[i];
+
+	ret = regulator_bulk_get(&pdev->dev, CSIS_NUM_SUPPLIES,
+				 state->supplies);
+	if (ret)
+		goto e_clkput;
 
 	ret = request_irq(state->irq, s5pcsis_irq_handler, 0,
 			  dev_name(&pdev->dev), state);
@@ -561,8 +567,7 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev)
 e_irqfree:
 	free_irq(state->irq, state);
 e_regput:
-	if (state->supply)
-		regulator_put(state->supply);
+	regulator_bulk_free(CSIS_NUM_SUPPLIES, state->supplies);
 e_clkput:
 	clk_disable(state->clock[CSIS_CLK_MUX]);
 	s5pcsis_clk_put(state);
@@ -592,11 +597,10 @@ static int s5pcsis_suspend(struct device *dev)
 		ret = pdata->phy_enable(state->pdev, false);
 		if (ret)
 			goto unlock;
-		if (state->supply) {
-			ret = regulator_disable(state->supply);
-			if (ret)
-				goto unlock;
-		}
+		ret = regulator_bulk_disable(CSIS_NUM_SUPPLIES,
+					     state->supplies);
+		if (ret)
+			goto unlock;
 		clk_disable(state->clock[CSIS_CLK_GATE]);
 		state->flags &= ~ST_POWERED;
 	}
@@ -622,16 +626,16 @@ static int s5pcsis_resume(struct device *dev)
 		goto unlock;
 
 	if (!(state->flags & ST_POWERED)) {
-		if (state->supply)
-			ret = regulator_enable(state->supply);
+		ret = regulator_bulk_enable(CSIS_NUM_SUPPLIES,
+					    state->supplies);
 		if (ret)
 			goto unlock;
-
 		ret = pdata->phy_enable(state->pdev, true);
 		if (!ret) {
 			state->flags |= ST_POWERED;
-		} else if (state->supply) {
-			regulator_disable(state->supply);
+		} else {
+			regulator_bulk_disable(CSIS_NUM_SUPPLIES,
+					       state->supplies);
 			goto unlock;
 		}
 		clk_enable(state->clock[CSIS_CLK_GATE]);
@@ -679,8 +683,7 @@ static int __devexit s5pcsis_remove(struct platform_device *pdev)
 	pm_runtime_set_suspended(&pdev->dev);
 
 	s5pcsis_clk_put(state);
-	if (state->supply)
-		regulator_put(state->supply);
+	regulator_bulk_free(CSIS_NUM_SUPPLIES, state->supplies);
 
 	media_entity_cleanup(&state->sd.entity);
 	free_irq(state->irq, state);
-- 
1.7.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2011-08-25 11:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-06 17:13 [PATCH 0/3] s5p-csis driver updates for 3.1 Sylwester Nawrocki
2011-07-06 17:13 ` [PATCH 1/3] s5p-csis: Handle all available power supplies Sylwester Nawrocki
2011-07-06 21:47   ` Laurent Pinchart
2011-07-07 13:34     ` Sylwester Nawrocki
2011-08-25 11:43       ` [PATCH v2] " Sylwester Nawrocki
2011-07-06 17:13 ` [PATCH 2/3] s5p-csis: Rework of the system suspend/resume helpers Sylwester Nawrocki
2011-07-06 17:13 ` [PATCH 3/3] s5p-csis: Enable v4l subdev device node Sylwester Nawrocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).