diff mbox series

remoteproc: stm32: use correct format strings on 64-bit

Message ID 20230609120546.3937821-1-arnd@kernel.org (mailing list archive)
State New, archived
Headers show
Series remoteproc: stm32: use correct format strings on 64-bit | expand

Commit Message

Arnd Bergmann June 9, 2023, 12:05 p.m. UTC
From: Arnd Bergmann <arnd@arndb.de>

With CONFIG_ARCH_STM32 making it into arch/arm64, a couple of format
strings no longer work, since they rely on size_t being compatible
with %x, or they print an 'int' using %z:

drivers/remoteproc/stm32_rproc.c: In function 'stm32_rproc_mem_alloc':
drivers/remoteproc/stm32_rproc.c:122:22: error: format '%x' expects argument of type 'unsigned int', but argument 5 has type 'size_t' {aka 'long unsigned int'} [-Werror=format=]
drivers/remoteproc/stm32_rproc.c:122:40: note: format string is defined here
  122 |         dev_dbg(dev, "map memory: %pa+%x\n", &mem->dma, mem->len);
      |                                       ~^
      |                                        |
      |                                        unsigned int
      |                                       %lx
drivers/remoteproc/stm32_rproc.c:125:30: error: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'size_t' {aka 'long unsigned int'} [-Werror=format=]
drivers/remoteproc/stm32_rproc.c:125:65: note: format string is defined here
  125 |                 dev_err(dev, "Unable to map memory region: %pa+%x\n",
      |                                                                ~^
      |                                                                 |
      |                                                                 unsigned int
      |                                                                %lx
drivers/remoteproc/stm32_rproc.c: In function 'stm32_rproc_get_loaded_rsc_table':
drivers/remoteproc/stm32_rproc.c:646:30: error: format '%zx' expects argument of type 'size_t', but argument 4 has type 'int' [-Werror=format=]
drivers/remoteproc/stm32_rproc.c:646:66: note: format string is defined here
  646 |                 dev_err(dev, "Unable to map memory region: %pa+%zx\n",
      |                                                                ~~^
      |                                                                  |
      |                                                                  long unsigned int
      |                                                                %x

Fix up all three instances to work across architectures, and enable
compile testing for this driver to ensure it builds everywhere.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/remoteproc/Kconfig       | 2 +-
 drivers/remoteproc/stm32_rproc.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Arnaud Pouliquen June 12, 2023, 2:10 p.m. UTC | #1
Hi Arnd

On 6/9/23 14:05, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> With CONFIG_ARCH_STM32 making it into arch/arm64, a couple of format
> strings no longer work, since they rely on size_t being compatible
> with %x, or they print an 'int' using %z:
> 
> drivers/remoteproc/stm32_rproc.c: In function 'stm32_rproc_mem_alloc':
> drivers/remoteproc/stm32_rproc.c:122:22: error: format '%x' expects argument of type 'unsigned int', but argument 5 has type 'size_t' {aka 'long unsigned int'} [-Werror=format=]
> drivers/remoteproc/stm32_rproc.c:122:40: note: format string is defined here
>   122 |         dev_dbg(dev, "map memory: %pa+%x\n", &mem->dma, mem->len);
>       |                                       ~^
>       |                                        |
>       |                                        unsigned int
>       |                                       %lx
> drivers/remoteproc/stm32_rproc.c:125:30: error: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'size_t' {aka 'long unsigned int'} [-Werror=format=]
> drivers/remoteproc/stm32_rproc.c:125:65: note: format string is defined here
>   125 |                 dev_err(dev, "Unable to map memory region: %pa+%x\n",
>       |                                                                ~^
>       |                                                                 |
>       |                                                                 unsigned int
>       |                                                                %lx
> drivers/remoteproc/stm32_rproc.c: In function 'stm32_rproc_get_loaded_rsc_table':
> drivers/remoteproc/stm32_rproc.c:646:30: error: format '%zx' expects argument of type 'size_t', but argument 4 has type 'int' [-Werror=format=]
> drivers/remoteproc/stm32_rproc.c:646:66: note: format string is defined here
>   646 |                 dev_err(dev, "Unable to map memory region: %pa+%zx\n",
>       |                                                                ~~^
>       |                                                                  |
>       |                                                                  long unsigned int
>       |                                                                %x
> 
> Fix up all three instances to work across architectures, and enable
> compile testing for this driver to ensure it builds everywhere.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


I also have it for the stm32mp2 :)

Please find a remark below , with or without that

Reviewed-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>

> ---
>  drivers/remoteproc/Kconfig       | 2 +-
>  drivers/remoteproc/stm32_rproc.c | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index a850e9f486dd6..48845dc8fa852 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -313,7 +313,7 @@ config ST_SLIM_REMOTEPROC
>  
>  config STM32_RPROC
>  	tristate "STM32 remoteproc support"
> -	depends on ARCH_STM32
> +	depends on ARCH_STM32 || COMPILE_TEST
>  	depends on REMOTEPROC
>  	select MAILBOX
>  	help
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index a7457777aae43..cf073bac79f73 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -119,10 +119,10 @@ static int stm32_rproc_mem_alloc(struct rproc *rproc,
>  	struct device *dev = rproc->dev.parent;
>  	void *va;
>  
> -	dev_dbg(dev, "map memory: %pa+%x\n", &mem->dma, mem->len);
> +	dev_dbg(dev, "map memory: %pad+%zx\n", &mem->dma, mem->len);
>  	va = ioremap_wc(mem->dma, mem->len);
>  	if (IS_ERR_OR_NULL(va)) {
> -		dev_err(dev, "Unable to map memory region: %pa+%x\n",
> +		dev_err(dev, "Unable to map memory region: %pad+0x%zx\n",
>  			&mem->dma, mem->len);
>  		return -ENOMEM;
>  	}
> @@ -643,7 +643,7 @@ stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
>  
>  	ddata->rsc_va = devm_ioremap_wc(dev, rsc_pa, RSC_TBL_SIZE);
>  	if (IS_ERR_OR_NULL(ddata->rsc_va)) {
> -		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
> +		dev_err(dev, "Unable to map memory region: %pa+%x\n",
>  			&rsc_pa, RSC_TBL_SIZE);

What about cast the RSC_TBL_SIZE define instead to ensure to be independent from
the arch and to match with the use of RSC_TBL_SIZE?

#define RSC_TBL_SIZE		((size_t)1024)

Thanks,
Arnaud

>  		ddata->rsc_va = NULL;
>  		return ERR_PTR(-ENOMEM);
Arnd Bergmann June 12, 2023, 2:17 p.m. UTC | #2
On Mon, Jun 12, 2023, at 16:10, Arnaud POULIQUEN wrote:

>>  	ddata->rsc_va = devm_ioremap_wc(dev, rsc_pa, RSC_TBL_SIZE);
>>  	if (IS_ERR_OR_NULL(ddata->rsc_va)) {
>> -		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
>> +		dev_err(dev, "Unable to map memory region: %pa+%x\n",
>>  			&rsc_pa, RSC_TBL_SIZE);
>
> What about cast the RSC_TBL_SIZE define instead to ensure to be independent from
> the arch and to match with the use of RSC_TBL_SIZE?
>
> #define RSC_TBL_SIZE		((size_t)1024)

I have no objection to that, but I don't see it doing anything good either,
as this is a constant value that will always work.

     Arnd
Arnaud Pouliquen June 13, 2023, 10:05 a.m. UTC | #3
On 6/12/23 16:17, Arnd Bergmann wrote:
> On Mon, Jun 12, 2023, at 16:10, Arnaud POULIQUEN wrote:
> 
>>>  	ddata->rsc_va = devm_ioremap_wc(dev, rsc_pa, RSC_TBL_SIZE);
>>>  	if (IS_ERR_OR_NULL(ddata->rsc_va)) {
>>> -		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
>>> +		dev_err(dev, "Unable to map memory region: %pa+%x\n",
>>>  			&rsc_pa, RSC_TBL_SIZE);
>>
>> What about cast the RSC_TBL_SIZE define instead to ensure to be independent from
>> the arch and to match with the use of RSC_TBL_SIZE?
>>
>> #define RSC_TBL_SIZE		((size_t)1024)
> 
> I have no objection to that, but I don't see it doing anything good either,
> as this is a constant value that will always work.
> 
>      Arnd

It was just to avoid to have the constant used with different types in the code
But that's nitpicking, let's keep your proposal.

Thanks,
Arnaud
Alexandre TORGUE June 13, 2023, 10:05 a.m. UTC | #4
Hi Arnd

On 6/12/23 16:17, Arnd Bergmann wrote:
> On Mon, Jun 12, 2023, at 16:10, Arnaud POULIQUEN wrote:
> 
>>>   	ddata->rsc_va = devm_ioremap_wc(dev, rsc_pa, RSC_TBL_SIZE);
>>>   	if (IS_ERR_OR_NULL(ddata->rsc_va)) {
>>> -		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
>>> +		dev_err(dev, "Unable to map memory region: %pa+%x\n",
>>>   			&rsc_pa, RSC_TBL_SIZE);
>>
>> What about cast the RSC_TBL_SIZE define instead to ensure to be independent from
>> the arch and to match with the use of RSC_TBL_SIZE?
>>
>> #define RSC_TBL_SIZE		((size_t)1024)
> 
> I have no objection to that, but I don't see it doing anything good either,
> as this is a constant value that will always work.
> 
>       Arnd

The kernel robot shot me for my STM32MP25  PR due to this issue. Do I 
have to resend something ?

Thanks
Alex
Randy Dunlap June 15, 2023, 10:40 p.m. UTC | #5
On 6/9/23 05:05, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> With CONFIG_ARCH_STM32 making it into arch/arm64, a couple of format
> strings no longer work, since they rely on size_t being compatible
> with %x, or they print an 'int' using %z:
> 
> drivers/remoteproc/stm32_rproc.c: In function 'stm32_rproc_mem_alloc':
> drivers/remoteproc/stm32_rproc.c:122:22: error: format '%x' expects argument of type 'unsigned int', but argument 5 has type 'size_t' {aka 'long unsigned int'} [-Werror=format=]
> drivers/remoteproc/stm32_rproc.c:122:40: note: format string is defined here
>   122 |         dev_dbg(dev, "map memory: %pa+%x\n", &mem->dma, mem->len);
>       |                                       ~^
>       |                                        |
>       |                                        unsigned int
>       |                                       %lx
> drivers/remoteproc/stm32_rproc.c:125:30: error: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'size_t' {aka 'long unsigned int'} [-Werror=format=]
> drivers/remoteproc/stm32_rproc.c:125:65: note: format string is defined here
>   125 |                 dev_err(dev, "Unable to map memory region: %pa+%x\n",
>       |                                                                ~^
>       |                                                                 |
>       |                                                                 unsigned int
>       |                                                                %lx
> drivers/remoteproc/stm32_rproc.c: In function 'stm32_rproc_get_loaded_rsc_table':
> drivers/remoteproc/stm32_rproc.c:646:30: error: format '%zx' expects argument of type 'size_t', but argument 4 has type 'int' [-Werror=format=]
> drivers/remoteproc/stm32_rproc.c:646:66: note: format string is defined here
>   646 |                 dev_err(dev, "Unable to map memory region: %pa+%zx\n",
>       |                                                                ~~^
>       |                                                                  |
>       |                                                                  long unsigned int
>       |                                                                %x
> 
> Fix up all three instances to work across architectures, and enable
> compile testing for this driver to ensure it builds everywhere.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org> # build-tested

Thanks.

> ---
>  drivers/remoteproc/Kconfig       | 2 +-
>  drivers/remoteproc/stm32_rproc.c | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index a850e9f486dd6..48845dc8fa852 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -313,7 +313,7 @@ config ST_SLIM_REMOTEPROC
>  
>  config STM32_RPROC
>  	tristate "STM32 remoteproc support"
> -	depends on ARCH_STM32
> +	depends on ARCH_STM32 || COMPILE_TEST
>  	depends on REMOTEPROC
>  	select MAILBOX
>  	help
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index a7457777aae43..cf073bac79f73 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -119,10 +119,10 @@ static int stm32_rproc_mem_alloc(struct rproc *rproc,
>  	struct device *dev = rproc->dev.parent;
>  	void *va;
>  
> -	dev_dbg(dev, "map memory: %pa+%x\n", &mem->dma, mem->len);
> +	dev_dbg(dev, "map memory: %pad+%zx\n", &mem->dma, mem->len);
>  	va = ioremap_wc(mem->dma, mem->len);
>  	if (IS_ERR_OR_NULL(va)) {
> -		dev_err(dev, "Unable to map memory region: %pa+%x\n",
> +		dev_err(dev, "Unable to map memory region: %pad+0x%zx\n",
>  			&mem->dma, mem->len);
>  		return -ENOMEM;
>  	}
> @@ -643,7 +643,7 @@ stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
>  
>  	ddata->rsc_va = devm_ioremap_wc(dev, rsc_pa, RSC_TBL_SIZE);
>  	if (IS_ERR_OR_NULL(ddata->rsc_va)) {
> -		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
> +		dev_err(dev, "Unable to map memory region: %pa+%x\n",
>  			&rsc_pa, RSC_TBL_SIZE);
>  		ddata->rsc_va = NULL;
>  		return ERR_PTR(-ENOMEM);
diff mbox series

Patch

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index a850e9f486dd6..48845dc8fa852 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -313,7 +313,7 @@  config ST_SLIM_REMOTEPROC
 
 config STM32_RPROC
 	tristate "STM32 remoteproc support"
-	depends on ARCH_STM32
+	depends on ARCH_STM32 || COMPILE_TEST
 	depends on REMOTEPROC
 	select MAILBOX
 	help
diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index a7457777aae43..cf073bac79f73 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -119,10 +119,10 @@  static int stm32_rproc_mem_alloc(struct rproc *rproc,
 	struct device *dev = rproc->dev.parent;
 	void *va;
 
-	dev_dbg(dev, "map memory: %pa+%x\n", &mem->dma, mem->len);
+	dev_dbg(dev, "map memory: %pad+%zx\n", &mem->dma, mem->len);
 	va = ioremap_wc(mem->dma, mem->len);
 	if (IS_ERR_OR_NULL(va)) {
-		dev_err(dev, "Unable to map memory region: %pa+%x\n",
+		dev_err(dev, "Unable to map memory region: %pad+0x%zx\n",
 			&mem->dma, mem->len);
 		return -ENOMEM;
 	}
@@ -643,7 +643,7 @@  stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
 
 	ddata->rsc_va = devm_ioremap_wc(dev, rsc_pa, RSC_TBL_SIZE);
 	if (IS_ERR_OR_NULL(ddata->rsc_va)) {
-		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
+		dev_err(dev, "Unable to map memory region: %pa+%x\n",
 			&rsc_pa, RSC_TBL_SIZE);
 		ddata->rsc_va = NULL;
 		return ERR_PTR(-ENOMEM);