[v2,1/4] clk: bcm2835: More flexible IO register remapping

Commit Message

Phil Elwell June 12, 2017, 2:25 p.m. UTC

The BCM2835 AUX block contains two registers - AUXIRQ and AUXENB.
The addition of an irqchip driver using AUXIRQ is hampered by
the current DT node reserving both registers with a compatible string
claimed by this bcm2835-aux-clk driver.

Ease the transition to separate DT nodes by detecting and handling
the case where this driver's MEM resource has been reduced to include
only the AUXENB register. Otherwise, use devm_ioremap to remap the
region without reserving it.

Signed-off-by: Phil Elwell <[e-mail]>
---
 drivers/clk/bcm/clk-bcm2835-aux.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Florian Fainelli June 12, 2017, 6:19 p.m. UTC | #1

On 06/12/2017 07:25 AM, Phil Elwell wrote:
> The BCM2835 AUX block contains two registers - AUXIRQ and AUXENB.
> The addition of an irqchip driver using AUXIRQ is hampered by
> the current DT node reserving both registers with a compatible string
> claimed by this bcm2835-aux-clk driver.
> 
> Ease the transition to separate DT nodes by detecting and handling
> the case where this driver's MEM resource has been reduced to include
> only the AUXENB register. Otherwise, use devm_ioremap to remap the
> region without reserving it.
> 
> Signed-off-by: Phil Elwell <[e-mail]>
> ---
>  drivers/clk/bcm/clk-bcm2835-aux.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/bcm/clk-bcm2835-aux.c b/drivers/clk/bcm/clk-bcm2835-aux.c
> index bd750cf..70d389e 100644
> --- a/drivers/clk/bcm/clk-bcm2835-aux.c
> +++ b/drivers/clk/bcm/clk-bcm2835-aux.c
> @@ -37,9 +37,22 @@ static int bcm2835_aux_clk_probe(struct platform_device *pdev)
>  	parent = __clk_get_name(parent_clk);
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	reg = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(reg))
> -		return PTR_ERR(reg);
> +	/*
> +	 * If the MEM resource is only 4 bytes long it covers just the
> +	 * AUXENB register, otherwise it is the entire AUX block.
> +	 */

Would be worth commenting why devm_ioremap_resource() is used here (to
avoid the implicit request mem_region) and not in the other case. Might
also be worth adding a comment that explains that this is temporary and
this kludge should be removed in the future in favor of the new bindings
only.

Thanks!

Patch

diff --git a/drivers/clk/bcm/clk-bcm2835-aux.c b/drivers/clk/bcm/clk-bcm2835-aux.c
index bd750cf..70d389e 100644
--- a/drivers/clk/bcm/clk-bcm2835-aux.c
+++ b/drivers/clk/bcm/clk-bcm2835-aux.c
@@ -37,9 +37,22 @@  static int bcm2835_aux_clk_probe(struct platform_device *pdev)
 	parent = __clk_get_name(parent_clk);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(dev, res);
-	if (IS_ERR(reg))
-		return PTR_ERR(reg);
+	/*
+	 * If the MEM resource is only 4 bytes long it covers just the
+	 * AUXENB register, otherwise it is the entire AUX block.
+	 */
+	if (resource_size(res) == 4) {
+		reg = devm_ioremap_resource(dev, res);
+		if (IS_ERR(reg))
+			return PTR_ERR(reg);
+		gate = reg;
+	} else {
+		reg = devm_ioremap(dev, res->start, resource_size(res));
+		if (IS_ERR(reg))
+			return PTR_ERR(reg);
+		gate = reg + BCM2835_AUXENB;
+	}
+
 
 	onecell = devm_kmalloc(dev, sizeof(*onecell) + sizeof(*onecell->hws) *
 			       BCM2835_AUX_CLOCK_COUNT, GFP_KERNEL);
@@ -47,7 +60,6 @@  static int bcm2835_aux_clk_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	onecell->num = BCM2835_AUX_CLOCK_COUNT;
 
-	gate = reg + BCM2835_AUXENB;
 	onecell->hws[BCM2835_AUX_CLOCK_UART] =
 		clk_hw_register_gate(dev, "aux_uart", parent, 0, gate, 0, 0, NULL);