All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] rc: img-ir: Add and enable sys clock for img-ir
@ 2015-02-04 16:48 Sifan Naeem
  2015-02-04 17:30 ` James Hogan
  0 siblings, 1 reply; 2+ messages in thread
From: Sifan Naeem @ 2015-02-04 16:48 UTC (permalink / raw)
  To: james.hogan, mchehab; +Cc: linux-kernel, linux-media, Sifan Naeem

Gets a handle to the system clock, already described in the binding
document, and calls the appropriate common clock framework functions
to mark it prepared/enabled, the common clock framework initially
enables the clock and doesn't disable it at least until the
device/driver is removed.
It's important the systen clock is enabled before register interface is
accessed by the driver.
The system clock to IR is needed for the driver to communicate with the
IR hardware via MMIO accesses on the system bus, so it must not be
disabled during use or the driver will malfunction.

Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
---
Changes from v1:
System clock enabled in probe function before any hardware is accessed.
Error handling in probe function ensures ISR doesn't get called with 
system clock disabled.

 drivers/media/rc/img-ir/img-ir-core.c |   29 +++++++++++++++++++++++++----
 drivers/media/rc/img-ir/img-ir.h      |    2 ++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/media/rc/img-ir/img-ir-core.c b/drivers/media/rc/img-ir/img-ir-core.c
index 77c78de..a10d666 100644
--- a/drivers/media/rc/img-ir/img-ir-core.c
+++ b/drivers/media/rc/img-ir/img-ir-core.c
@@ -110,16 +110,32 @@ static int img_ir_probe(struct platform_device *pdev)
 	priv->clk = devm_clk_get(&pdev->dev, "core");
 	if (IS_ERR(priv->clk))
 		dev_warn(&pdev->dev, "cannot get core clock resource\n");
+
+	/* Get sys clock */
+	priv->sys_clk = devm_clk_get(&pdev->dev, "sys");
+	if (IS_ERR(priv->sys_clk))
+		dev_warn(&pdev->dev, "cannot get sys clock resource\n");
 	/*
-	 * The driver doesn't need to know about the system ("sys") or power
-	 * modulation ("mod") clocks yet
+	 * Enabling the system clock before the register interface is
+	 * accessed. ISR shouldn't get called with Sys Clock disabled,
+	 * hence exiting probe with an error.
 	 */
+	if (!IS_ERR(priv->sys_clk)) {
+		error = clk_prepare_enable(priv->sys_clk);
+		if (error) {
+			dev_err(&pdev->dev, "cannot enable sys clock\n");
+			return error;
+		}
+	}
 
 	/* Set up raw & hw decoder */
 	error = img_ir_probe_raw(priv);
 	error2 = img_ir_probe_hw(priv);
-	if (error && error2)
-		return (error == -ENODEV) ? error2 : error;
+	if (error && error2) {
+		if (error == -ENODEV)
+			error = error2;
+		goto err_probe;
+	}
 
 	/* Get the IRQ */
 	priv->irq = irq;
@@ -139,6 +155,9 @@ static int img_ir_probe(struct platform_device *pdev)
 err_irq:
 	img_ir_remove_hw(priv);
 	img_ir_remove_raw(priv);
+err_probe:
+	if (!IS_ERR(priv->sys_clk))
+		clk_disable_unprepare(priv->sys_clk);
 	return error;
 }
 
@@ -152,6 +171,8 @@ static int img_ir_remove(struct platform_device *pdev)
 
 	if (!IS_ERR(priv->clk))
 		clk_disable_unprepare(priv->clk);
+	if (!IS_ERR(priv->sys_clk))
+		clk_disable_unprepare(priv->sys_clk);
 	return 0;
 }
 
diff --git a/drivers/media/rc/img-ir/img-ir.h b/drivers/media/rc/img-ir/img-ir.h
index 2ddf560..f1387c0 100644
--- a/drivers/media/rc/img-ir/img-ir.h
+++ b/drivers/media/rc/img-ir/img-ir.h
@@ -138,6 +138,7 @@ struct clk;
  * @dev:		Platform device.
  * @irq:		IRQ number.
  * @clk:		Input clock.
+ * @sys_clk:		System clock.
  * @reg_base:		Iomem base address of IR register block.
  * @lock:		Protects IR registers and variables in this struct.
  * @raw:		Driver data for raw decoder.
@@ -147,6 +148,7 @@ struct img_ir_priv {
 	struct device		*dev;
 	int			irq;
 	struct clk		*clk;
+	struct clk		*sys_clk;
 	void __iomem		*reg_base;
 	spinlock_t		lock;
 
-- 
1.7.9.5


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

* Re: [PATCH v2] rc: img-ir: Add and enable sys clock for img-ir
  2015-02-04 16:48 [PATCH v2] rc: img-ir: Add and enable sys clock for img-ir Sifan Naeem
@ 2015-02-04 17:30 ` James Hogan
  0 siblings, 0 replies; 2+ messages in thread
From: James Hogan @ 2015-02-04 17:30 UTC (permalink / raw)
  To: Sifan Naeem, mchehab; +Cc: linux-kernel, linux-media

[-- Attachment #1: Type: text/plain, Size: 4001 bytes --]

On 04/02/15 16:48, Sifan Naeem wrote:
> Gets a handle to the system clock, already described in the binding
> document, and calls the appropriate common clock framework functions
> to mark it prepared/enabled, the common clock framework initially
> enables the clock and doesn't disable it at least until the
> device/driver is removed.
> It's important the systen clock is enabled before register interface is
> accessed by the driver.
> The system clock to IR is needed for the driver to communicate with the
> IR hardware via MMIO accesses on the system bus, so it must not be
> disabled during use or the driver will malfunction.
> 
> Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>

Thanks Sifan, looks good to me, doesn't break tz1090, and seems to still
cope with no clocks provided in DT.

Acked-by: James Hogan <james.hogan@imgtec.com>

Cheers
James

> ---
> Changes from v1:
> System clock enabled in probe function before any hardware is accessed.
> Error handling in probe function ensures ISR doesn't get called with 
> system clock disabled.
> 
>  drivers/media/rc/img-ir/img-ir-core.c |   29 +++++++++++++++++++++++++----
>  drivers/media/rc/img-ir/img-ir.h      |    2 ++
>  2 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/rc/img-ir/img-ir-core.c b/drivers/media/rc/img-ir/img-ir-core.c
> index 77c78de..a10d666 100644
> --- a/drivers/media/rc/img-ir/img-ir-core.c
> +++ b/drivers/media/rc/img-ir/img-ir-core.c
> @@ -110,16 +110,32 @@ static int img_ir_probe(struct platform_device *pdev)
>  	priv->clk = devm_clk_get(&pdev->dev, "core");
>  	if (IS_ERR(priv->clk))
>  		dev_warn(&pdev->dev, "cannot get core clock resource\n");
> +
> +	/* Get sys clock */
> +	priv->sys_clk = devm_clk_get(&pdev->dev, "sys");
> +	if (IS_ERR(priv->sys_clk))
> +		dev_warn(&pdev->dev, "cannot get sys clock resource\n");
>  	/*
> -	 * The driver doesn't need to know about the system ("sys") or power
> -	 * modulation ("mod") clocks yet
> +	 * Enabling the system clock before the register interface is
> +	 * accessed. ISR shouldn't get called with Sys Clock disabled,
> +	 * hence exiting probe with an error.
>  	 */
> +	if (!IS_ERR(priv->sys_clk)) {
> +		error = clk_prepare_enable(priv->sys_clk);
> +		if (error) {
> +			dev_err(&pdev->dev, "cannot enable sys clock\n");
> +			return error;
> +		}
> +	}
>  
>  	/* Set up raw & hw decoder */
>  	error = img_ir_probe_raw(priv);
>  	error2 = img_ir_probe_hw(priv);
> -	if (error && error2)
> -		return (error == -ENODEV) ? error2 : error;
> +	if (error && error2) {
> +		if (error == -ENODEV)
> +			error = error2;
> +		goto err_probe;
> +	}
>  
>  	/* Get the IRQ */
>  	priv->irq = irq;
> @@ -139,6 +155,9 @@ static int img_ir_probe(struct platform_device *pdev)
>  err_irq:
>  	img_ir_remove_hw(priv);
>  	img_ir_remove_raw(priv);
> +err_probe:
> +	if (!IS_ERR(priv->sys_clk))
> +		clk_disable_unprepare(priv->sys_clk);
>  	return error;
>  }
>  
> @@ -152,6 +171,8 @@ static int img_ir_remove(struct platform_device *pdev)
>  
>  	if (!IS_ERR(priv->clk))
>  		clk_disable_unprepare(priv->clk);
> +	if (!IS_ERR(priv->sys_clk))
> +		clk_disable_unprepare(priv->sys_clk);
>  	return 0;
>  }
>  
> diff --git a/drivers/media/rc/img-ir/img-ir.h b/drivers/media/rc/img-ir/img-ir.h
> index 2ddf560..f1387c0 100644
> --- a/drivers/media/rc/img-ir/img-ir.h
> +++ b/drivers/media/rc/img-ir/img-ir.h
> @@ -138,6 +138,7 @@ struct clk;
>   * @dev:		Platform device.
>   * @irq:		IRQ number.
>   * @clk:		Input clock.
> + * @sys_clk:		System clock.
>   * @reg_base:		Iomem base address of IR register block.
>   * @lock:		Protects IR registers and variables in this struct.
>   * @raw:		Driver data for raw decoder.
> @@ -147,6 +148,7 @@ struct img_ir_priv {
>  	struct device		*dev;
>  	int			irq;
>  	struct clk		*clk;
> +	struct clk		*sys_clk;
>  	void __iomem		*reg_base;
>  	spinlock_t		lock;
>  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

end of thread, other threads:[~2015-02-04 17:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-04 16:48 [PATCH v2] rc: img-ir: Add and enable sys clock for img-ir Sifan Naeem
2015-02-04 17:30 ` James Hogan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.