[PATCH] dmaengine: apple-admac: Avoid accessing registers in probe

[PATCH] dmaengine: apple-admac: Avoid accessing registers in probe

[Sasha Finkelstein via B4 Relay]

From: Sasha Finkelstein <fnkl.kernel@gmail.com>

The ADMAC attached to the AOP has complex power sequencing, and is
power gated when the probe callback runs. Move the register reads
to other functions, where we can guarantee that the hardware is
switched on.

Fixes: 568aa6dd641f ("dmaengine: apple-admac: Allocate cache SRAM to channels")
Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
---
 drivers/dma/apple-admac.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/apple-admac.c b/drivers/dma/apple-admac.c
index 9588773dd2eb670a2f6115fdaef39a0e88248015..037ec38730cf980eee11ebb8ec17be7623879cf8 100644
--- a/drivers/dma/apple-admac.c
+++ b/drivers/dma/apple-admac.c
@@ -153,6 +153,8 @@ static int admac_alloc_sram_carveout(struct admac_data *ad,
 {
 	struct admac_sram *sram;
 	int i, ret = 0, nblocks;
+	ad->txcache.size = readl_relaxed(ad->base + REG_TX_SRAM_SIZE);
+	ad->rxcache.size = readl_relaxed(ad->base + REG_RX_SRAM_SIZE);
 
 	if (dir == DMA_MEM_TO_DEV)
 		sram = &ad->txcache;
@@ -912,12 +914,7 @@ static int admac_probe(struct platform_device *pdev)
 		goto free_irq;
 	}
 
-	ad->txcache.size = readl_relaxed(ad->base + REG_TX_SRAM_SIZE);
-	ad->rxcache.size = readl_relaxed(ad->base + REG_RX_SRAM_SIZE);
-
 	dev_info(&pdev->dev, "Audio DMA Controller\n");
-	dev_info(&pdev->dev, "imprint %x TX cache %u RX cache %u\n",
-		 readl_relaxed(ad->base + REG_IMPRINT), ad->txcache.size, ad->rxcache.size);
 
 	return 0;
 

---
base-commit: 9f16d5e6f220661f73b36a4be1b21575651d8833
change-id: 20241124-admac-power-7f32c2a1850a

Re: [PATCH] dmaengine: apple-admac: Avoid accessing registers in probe

[Vinod Koul]

On 24-11-24, 16:48, Sasha Finkelstein via B4 Relay wrote:
> From: Sasha Finkelstein <fnkl.kernel@gmail.com>
> 
> The ADMAC attached to the AOP has complex power sequencing, and is
> power gated when the probe callback runs. Move the register reads
> to other functions, where we can guarantee that the hardware is
> switched on.

So looking at the driver, there is code to turn power, so what ensures
that power is up while we are in alloc callback

> 
> Fixes: 568aa6dd641f ("dmaengine: apple-admac: Allocate cache SRAM to channels")
> Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
> ---
>  drivers/dma/apple-admac.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/apple-admac.c b/drivers/dma/apple-admac.c
> index 9588773dd2eb670a2f6115fdaef39a0e88248015..037ec38730cf980eee11ebb8ec17be7623879cf8 100644
> --- a/drivers/dma/apple-admac.c
> +++ b/drivers/dma/apple-admac.c
> @@ -153,6 +153,8 @@ static int admac_alloc_sram_carveout(struct admac_data *ad,
>  {
>  	struct admac_sram *sram;
>  	int i, ret = 0, nblocks;

Empty line before this please

> +	ad->txcache.size = readl_relaxed(ad->base + REG_TX_SRAM_SIZE);
> +	ad->rxcache.size = readl_relaxed(ad->base + REG_RX_SRAM_SIZE);
>  
>  	if (dir == DMA_MEM_TO_DEV)
>  		sram = &ad->txcache;
> @@ -912,12 +914,7 @@ static int admac_probe(struct platform_device *pdev)
>  		goto free_irq;
>  	}
>  
> -	ad->txcache.size = readl_relaxed(ad->base + REG_TX_SRAM_SIZE);
> -	ad->rxcache.size = readl_relaxed(ad->base + REG_RX_SRAM_SIZE);
> -
>  	dev_info(&pdev->dev, "Audio DMA Controller\n");
> -	dev_info(&pdev->dev, "imprint %x TX cache %u RX cache %u\n",
> -		 readl_relaxed(ad->base + REG_IMPRINT), ad->txcache.size, ad->rxcache.size);
>  
>  	return 0;
>  
> 
> ---
> base-commit: 9f16d5e6f220661f73b36a4be1b21575651d8833
> change-id: 20241124-admac-power-7f32c2a1850a
> 

-- 
~Vinod

Re: [PATCH] dmaengine: apple-admac: Avoid accessing registers in probe

[Sasha Finkelstein]

On Mon, 2 Dec 2024 at 17:27, Vinod Koul <vkoul@kernel.org> wrote:
> So looking at the driver, there is code to turn power, so what ensures
> that power is up while we are in alloc callback

There are two devices that use admacs, mca, and aop, with aop
only having a downstream driver for now. Aop's admac is controlled
by aop firmware, which does the power switching. Naturally, that firmware
has it's own idea of how power is supposed to be managed, so we can't
sequence it in such a way that the relevant admac has power when we
are running probe().

Re: [PATCH] dmaengine: apple-admac: Avoid accessing registers in probe

[Vinod Koul]

On Sun, 24 Nov 2024 16:48:28 +0100, Sasha Finkelstein wrote:
> The ADMAC attached to the AOP has complex power sequencing, and is
> power gated when the probe callback runs. Move the register reads
> to other functions, where we can guarantee that the hardware is
> switched on.
> 
> 

Applied, thanks!

[1/1] dmaengine: apple-admac: Avoid accessing registers in probe
      commit: 8d55e8a16f019211163f1180fd9f9fbe05901900

Best regards,
-- 
~Vinod