diff mbox series

spi: atmel-quadspi: Print the controller version and used irq

Message ID 20241008083226.51163-1-mihai.sain@microchip.com
State New
Headers show
Series spi: atmel-quadspi: Print the controller version and used irq | expand

Commit Message

Mihai Sain Oct. 8, 2024, 8:32 a.m. UTC
Add support to print the controller version and used irq
similar to other at91 drivers (spi, twi, usart).

Signed-off-by: Mihai Sain <mihai.sain@microchip.com>
---
 drivers/spi/atmel-quadspi.c | 2 ++
 1 file changed, 2 insertions(+)


base-commit: 87d6aab2389e5ce0197d8257d5f8ee965a67c4cd
prerequisite-patch-id: 5e1313094386b146c9180d72c15bae49aaffbfa8

Comments

Alexander Dahl Jan. 10, 2025, 10:25 a.m. UTC | #1
Hello Mihai,

just saw I made a similar patch for myself lately, so regarding the
discussion of the need of such a patch, I would opt for it.  However,
see below …

Am Tue, Oct 08, 2024 at 11:32:26AM +0300 schrieb Mihai Sain:
> Add support to print the controller version and used irq
> similar to other at91 drivers (spi, twi, usart).
> 
> Signed-off-by: Mihai Sain <mihai.sain@microchip.com>
> ---
>  drivers/spi/atmel-quadspi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> index 95cdfc28361e..757f07132585 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
> @@ -687,6 +687,8 @@ static int atmel_qspi_probe(struct platform_device *pdev)
>  	pm_runtime_mark_last_busy(&pdev->dev);
>  	pm_runtime_put_autosuspend(&pdev->dev);
>  
> +	dev_info(&pdev->dev, "AT91 QSPI Controller version %#x (irq %d)\n",
> +		 atmel_qspi_read(aq, QSPI_VERSION), irq);
>  	return 0;

I think this should go above pm_runtime functions, because it does i/o
on a register.  This is how my diff looks like:

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index 3d95b887235e6..7405b66e14b30 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -1356,6 +1356,7 @@ static int atmel_qspi_probe(struct platform_device *pdev)
         struct atmel_qspi *aq;
         struct resource *res;
         int irq, err = 0;
+        u32 version;
 
         ctrl = devm_spi_alloc_host(&pdev->dev, sizeof(*aq));
         if (!ctrl)
@@ -1470,6 +1471,12 @@ static int atmel_qspi_probe(struct platform_device *pdev)
                 pm_runtime_dont_use_autosuspend(&pdev->dev);
                 goto dma_release;
         }
+
+        version = atmel_qspi_read(aq, QSPI_VERSION) & 0x00000fff;
+        dev_info(&pdev->dev,
+                 "Atmel QSPI Controller version 0x%x at %pR (irq %d)\n",
+                 version, pdev->resource, irq);
+
         pm_runtime_mark_last_busy(&pdev->dev);
         pm_runtime_put_autosuspend(&pdev->dev);

Greets
Alex
Mihai Sain Jan. 13, 2025, 7:52 a.m. UTC | #2
> Hello Mihai,

Hi Alexander,

> 
> just saw I made a similar patch for myself lately, so regarding the discussion
> of the need of such a patch, I would opt for it.  However, see below …
> 
> Am Tue, Oct 08, 2024 at 11:32:26AM +0300 schrieb Mihai Sain:
> > Add support to print the controller version and used irq similar to
> > other at91 drivers (spi, twi, usart).
> >
> > Signed-off-by: Mihai Sain <mihai.sain@microchip.com>
> > ---
> >  drivers/spi/atmel-quadspi.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> > index 95cdfc28361e..757f07132585 100644
> > --- a/drivers/spi/atmel-quadspi.c
> > +++ b/drivers/spi/atmel-quadspi.c
> > @@ -687,6 +687,8 @@ static int atmel_qspi_probe(struct platform_device *pdev)
> >       pm_runtime_mark_last_busy(&pdev->dev);
> >       pm_runtime_put_autosuspend(&pdev->dev);
> >
> > +     dev_info(&pdev->dev, "AT91 QSPI Controller version %#x (irq %d)\n",
> > +              atmel_qspi_read(aq, QSPI_VERSION), irq);
> >       return 0;
> 
> I think this should go above pm_runtime functions, because it does i/o on a
> register.  This is how my diff looks like:

Yes, you're right 😊
I also think that this print should be done using dev_dbg().

> 
> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c index
> 3d95b887235e6..7405b66e14b30 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
> @@ -1356,6 +1356,7 @@ static int atmel_qspi_probe(struct platform_device *pdev)
>          struct atmel_qspi *aq;
>          struct resource *res;
>          int irq, err = 0;
> +        u32 version;
> 
>          ctrl = devm_spi_alloc_host(&pdev->dev, sizeof(*aq));
>          if (!ctrl)
> @@ -1470,6 +1471,12 @@ static int atmel_qspi_probe(struct platform_device
> *pdev)
>                  pm_runtime_dont_use_autosuspend(&pdev->dev);
>                  goto dma_release;
>          }
> +
> +        version = atmel_qspi_read(aq, QSPI_VERSION) & 0x00000fff;
> +        dev_info(&pdev->dev,
> +                 "Atmel QSPI Controller version 0x%x at %pR (irq %d)\n",
> +                 version, pdev->resource, irq);
> +
>          pm_runtime_mark_last_busy(&pdev->dev);
>          pm_runtime_put_autosuspend(&pdev->dev);
> 
> Greets
> Alex

Regards,
Mihai
diff mbox series

Patch

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index 95cdfc28361e..757f07132585 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -687,6 +687,8 @@  static int atmel_qspi_probe(struct platform_device *pdev)
 	pm_runtime_mark_last_busy(&pdev->dev);
 	pm_runtime_put_autosuspend(&pdev->dev);
 
+	dev_info(&pdev->dev, "AT91 QSPI Controller version %#x (irq %d)\n",
+		 atmel_qspi_read(aq, QSPI_VERSION), irq);
 	return 0;
 
 disable_qspick: