* [PATCH] spi: atmel-quadspi: Print the controller version and used irq @ 2024-10-08 8:32 Mihai Sain 2024-10-08 9:34 ` Tudor Ambarus 2025-01-10 10:25 ` Alexander Dahl 0 siblings, 2 replies; 6+ messages in thread From: Mihai Sain @ 2024-10-08 8:32 UTC (permalink / raw) To: broonie, nicolas.ferre, alexandre.belloni, claudiu.beznea, linux-spi, linux-arm-kernel, linux-kernel Cc: 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; disable_qspick: base-commit: 87d6aab2389e5ce0197d8257d5f8ee965a67c4cd prerequisite-patch-id: 5e1313094386b146c9180d72c15bae49aaffbfa8 -- 2.47.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: atmel-quadspi: Print the controller version and used irq 2024-10-08 8:32 [PATCH] spi: atmel-quadspi: Print the controller version and used irq Mihai Sain @ 2024-10-08 9:34 ` Tudor Ambarus 2024-10-08 10:29 ` Mark Brown 2025-01-10 10:25 ` Alexander Dahl 1 sibling, 1 reply; 6+ messages in thread From: Tudor Ambarus @ 2024-10-08 9:34 UTC (permalink / raw) To: Mihai Sain, broonie, nicolas.ferre, alexandre.belloni, claudiu.beznea, linux-spi, linux-arm-kernel, linux-kernel Hi, Mihai, On 10/8/24 9:32 AM, Mihai Sain wrote: > 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); This pollutes the console. Better to add a dev_dbg if you care. And irq number doesn't bring too much value as you can see it in dt, isn't it? Cheers, ta ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: atmel-quadspi: Print the controller version and used irq 2024-10-08 9:34 ` Tudor Ambarus @ 2024-10-08 10:29 ` Mark Brown 2024-10-09 7:04 ` Tudor Ambarus 0 siblings, 1 reply; 6+ messages in thread From: Mark Brown @ 2024-10-08 10:29 UTC (permalink / raw) To: Tudor Ambarus Cc: Mihai Sain, nicolas.ferre, alexandre.belloni, claudiu.beznea, linux-spi, linux-arm-kernel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 773 bytes --] On Tue, Oct 08, 2024 at 10:34:39AM +0100, Tudor Ambarus wrote: > On 10/8/24 9:32 AM, Mihai Sain wrote: > > Add support to print the controller version and used irq > > similar to other at91 drivers (spi, twi, usart). > > + dev_info(&pdev->dev, "AT91 QSPI Controller version %#x (irq %d)\n", > > + atmel_qspi_read(aq, QSPI_VERSION), irq); > This pollutes the console. Better to add a dev_dbg if you care. > And irq number doesn't bring too much value as you can see it in dt, > isn't it? The objective of bringing the various AT91 drivers into consistency does seem useful so if this isn't OK for this driver we should probably update the other drivers as well. Ensuring that people can get at the IP version does feel useful, I guess it could also be a sysfs thing? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: atmel-quadspi: Print the controller version and used irq 2024-10-08 10:29 ` Mark Brown @ 2024-10-09 7:04 ` Tudor Ambarus 0 siblings, 0 replies; 6+ messages in thread From: Tudor Ambarus @ 2024-10-09 7:04 UTC (permalink / raw) To: Mark Brown Cc: Mihai Sain, nicolas.ferre, alexandre.belloni, claudiu.beznea, linux-spi, linux-arm-kernel, linux-kernel On 10/8/24 11:29 AM, Mark Brown wrote: > On Tue, Oct 08, 2024 at 10:34:39AM +0100, Tudor Ambarus wrote: >> On 10/8/24 9:32 AM, Mihai Sain wrote: > >>> Add support to print the controller version and used irq >>> similar to other at91 drivers (spi, twi, usart). > >>> + dev_info(&pdev->dev, "AT91 QSPI Controller version %#x (irq %d)\n", >>> + atmel_qspi_read(aq, QSPI_VERSION), irq); > >> This pollutes the console. Better to add a dev_dbg if you care. >> And irq number doesn't bring too much value as you can see it in dt, >> isn't it? > > The objective of bringing the various AT91 drivers into consistency does > seem useful so if this isn't OK for this driver we should probably right, consistency is good. > update the other drivers as well. Ensuring that people can get at the Can be a goal. My point was that printing too much info in the boot log may hide other more important information. Printing IP versions, irqs, dma channels acquired (or worse, that a driver probed successful) shall be part of debug prints if someone cares. > IP version does feel useful, I guess it could also be a sysfs thing? I'd also consider debugfs for IP version, it has less restrictions. Mihai, do as you find best, it's just a suggestion. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: atmel-quadspi: Print the controller version and used irq 2024-10-08 8:32 [PATCH] spi: atmel-quadspi: Print the controller version and used irq Mihai Sain 2024-10-08 9:34 ` Tudor Ambarus @ 2025-01-10 10:25 ` Alexander Dahl 2025-01-13 7:52 ` Mihai.Sain 1 sibling, 1 reply; 6+ messages in thread From: Alexander Dahl @ 2025-01-10 10:25 UTC (permalink / raw) To: Mihai Sain Cc: broonie, nicolas.ferre, alexandre.belloni, claudiu.beznea, linux-spi, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] spi: atmel-quadspi: Print the controller version and used irq 2025-01-10 10:25 ` Alexander Dahl @ 2025-01-13 7:52 ` Mihai.Sain 0 siblings, 0 replies; 6+ messages in thread From: Mihai.Sain @ 2025-01-13 7:52 UTC (permalink / raw) To: ada Cc: broonie, Nicolas.Ferre, alexandre.belloni, claudiu.beznea, linux-spi, linux-arm-kernel, linux-kernel > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-01-13 7:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-08 8:32 [PATCH] spi: atmel-quadspi: Print the controller version and used irq Mihai Sain 2024-10-08 9:34 ` Tudor Ambarus 2024-10-08 10:29 ` Mark Brown 2024-10-09 7:04 ` Tudor Ambarus 2025-01-10 10:25 ` Alexander Dahl 2025-01-13 7:52 ` Mihai.Sain
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).