diff mbox series

[v3] net: smc91x: Fix pointer types

Message ID 20240516223004.350368-2-thorsten.blum@toblux.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [v3] net: smc91x: Fix pointer types | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 921 this patch: 921
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 925 this patch: 925
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 926 this patch: 926
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 16 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-05-17--12-00 (tests: 1036)

Commit Message

Thorsten Blum May 16, 2024, 10:30 p.m. UTC 
Use void __iomem pointers as parameters for mcf_insw() and mcf_outsw()
to align with the parameter types of readw() and writew() to fix the
following warnings reported by kernel test robot:

drivers/net/ethernet/smsc/smc91x.c:590:9: sparse: warning: incorrect type in argument 1 (different address spaces)
drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    expected void *a
drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    got void [noderef] __iomem *
drivers/net/ethernet/smsc/smc91x.c:590:9: sparse: warning: incorrect type in argument 1 (different address spaces)
drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    expected void *a
drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    got void [noderef] __iomem *
drivers/net/ethernet/smsc/smc91x.c:590:9: sparse: warning: incorrect type in argument 1 (different address spaces)
drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    expected void *a
drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    got void [noderef] __iomem *
drivers/net/ethernet/smsc/smc91x.c:483:17: sparse: warning: incorrect type in argument 1 (different address spaces)
drivers/net/ethernet/smsc/smc91x.c:483:17: sparse:    expected void *a
drivers/net/ethernet/smsc/smc91x.c:483:17: sparse:    got void [noderef] __iomem *

Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202405160853.3qyaSj8w-lkp@intel.com/
Acked-by: Nicolas Pitre <nico@fluxnic.net>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
Changes in v2:
- Use lp->base instead of __ioaddr as suggested by Andrew Lunn. They are
 essentially the same, but using lp->base results in a smaller diff
- Remove whitespace only changes as suggested by Andrew Lunn
- Preserve Acked-by: Nicolas Pitre tag (please let me know if you
 somehow disagree with the changes in v2 or v3)

Changes in v3:
- Revert changing the macros as this is unnecessary. Neither the types
  nor the __iomem attributes get lost across macro boundaries
- Preserve Reviewed-by: Andrew Lunn tag (please let me know if you
  somehow disagree with the changes in v3)
---
 drivers/net/ethernet/smsc/smc91x.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andrew Lunn May 16, 2024, 10:51 p.m. UTC | #1 
On Fri, May 17, 2024 at 12:30:05AM +0200, Thorsten Blum wrote:
> Use void __iomem pointers as parameters for mcf_insw() and mcf_outsw()
> to align with the parameter types of readw() and writew() to fix the
> following warnings reported by kernel test robot:
> 
> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    expected void *a
> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    got void [noderef] __iomem *
> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    expected void *a
> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    got void [noderef] __iomem *
> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    expected void *a
> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    got void [noderef] __iomem *
> drivers/net/ethernet/smsc/smc91x.c:483:17: sparse: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/ethernet/smsc/smc91x.c:483:17: sparse:    expected void *a
> drivers/net/ethernet/smsc/smc91x.c:483:17: sparse:    got void [noderef] __iomem *
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202405160853.3qyaSj8w-lkp@intel.com/
> Acked-by: Nicolas Pitre <nico@fluxnic.net>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---
> Changes in v2:
> - Use lp->base instead of __ioaddr as suggested by Andrew Lunn. They are
>  essentially the same, but using lp->base results in a smaller diff
> - Remove whitespace only changes as suggested by Andrew Lunn
> - Preserve Acked-by: Nicolas Pitre tag (please let me know if you
>  somehow disagree with the changes in v2 or v3)
> 
> Changes in v3:
> - Revert changing the macros as this is unnecessary. Neither the types
>   nor the __iomem attributes get lost across macro boundaries
> - Preserve Reviewed-by: Andrew Lunn tag (please let me know if you
>   somehow disagree with the changes in v3)

This fixes the warning, but we still have the macro accessing things
not passed to them. If you are going to brother to fix the warnings,
it would also be good to fix the bad practice. Please make a patchset
to do this.

It would also be good if you read:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

	Andrew
Thorsten Blum May 16, 2024, 11:21 p.m. UTC | #2 
On 17. May 2024, at 00:51, Andrew Lunn <andrew@lunn.ch> wrote:
> On Fri, May 17, 2024 at 12:30:05AM +0200, Thorsten Blum wrote:
>> Use void __iomem pointers as parameters for mcf_insw() and mcf_outsw()
>> to align with the parameter types of readw() and writew() to fix the
>> following warnings reported by kernel test robot:
>> 
>> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse: warning: incorrect type in argument 1 (different address spaces)
>> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    expected void *a
>> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    got void [noderef] __iomem *
>> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse: warning: incorrect type in argument 1 (different address spaces)
>> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    expected void *a
>> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    got void [noderef] __iomem *
>> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse: warning: incorrect type in argument 1 (different address spaces)
>> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    expected void *a
>> drivers/net/ethernet/smsc/smc91x.c:590:9: sparse:    got void [noderef] __iomem *
>> drivers/net/ethernet/smsc/smc91x.c:483:17: sparse: warning: incorrect type in argument 1 (different address spaces)
>> drivers/net/ethernet/smsc/smc91x.c:483:17: sparse:    expected void *a
>> drivers/net/ethernet/smsc/smc91x.c:483:17: sparse:    got void [noderef] __iomem *
>> 
>> Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202405160853.3qyaSj8w-lkp@intel.com/
>> Acked-by: Nicolas Pitre <nico@fluxnic.net>
>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>> ---
>> Changes in v2:
>> - Use lp->base instead of __ioaddr as suggested by Andrew Lunn. They are
>> essentially the same, but using lp->base results in a smaller diff
>> - Remove whitespace only changes as suggested by Andrew Lunn
>> - Preserve Acked-by: Nicolas Pitre tag (please let me know if you
>> somehow disagree with the changes in v2 or v3)
>> 
>> Changes in v3:
>> - Revert changing the macros as this is unnecessary. Neither the types
>>  nor the __iomem attributes get lost across macro boundaries
>> - Preserve Reviewed-by: Andrew Lunn tag (please let me know if you
>>  somehow disagree with the changes in v3)
> 
> This fixes the warning, but we still have the macro accessing things
> not passed to them. If you are going to brother to fix the warnings,
> it would also be good to fix the bad practice. Please make a patchset
> to do this.

I would prefer to submit another patch to fix the macros. I submitted v3
because the patch description for v2 was wrong (type information or
attributes don't get lost across macro boundaries) and the macro changes
are unnecessary to fix the warnings.

I should never have changed the macros, but after first adding __iomem
to mcf_insw() and mcf_outsw(), I kept getting the same errors and looked
for the problem in the SMC_* macros. I probably didn't do a clean build
or forgot to save my changes and just refactored the macros as a side
effect.

> It would also be good if you read:
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

Will do.

Thanks,
Thorsten
Thorsten Blum May 17, 2024, 10:41 a.m. UTC | #3 
On 17. May 2024, at 01:21, Thorsten Blum <thorsten.blum@toblux.com> wrote:
> On 17. May 2024, at 00:51, Andrew Lunn <andrew@lunn.ch> wrote:
>> It would also be good if you read:
>> 
>> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> 
> Will do.

Reading this was helpful and I learned that:
- net-next is closed during a merge window
- patches should be prefixed with the tree name

I'll submit the patch that cleans up ioaddr from all SMC_* macros when
net-next is open again.

Thanks,
Thorsten
diff mbox series

Patch

diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
index 45ef5ac0788a..38aa4374e813 100644
--- a/drivers/net/ethernet/smsc/smc91x.h
+++ b/drivers/net/ethernet/smsc/smc91x.h
@@ -142,14 +142,14 @@  static inline void _SMC_outw_align4(u16 val, void __iomem *ioaddr, int reg,
 #define SMC_CAN_USE_32BIT	0
 #define SMC_NOWAIT		1
 
-static inline void mcf_insw(void *a, unsigned char *p, int l)
+static inline void mcf_insw(void __iomem *a, unsigned char *p, int l)
 {
 	u16 *wp = (u16 *) p;
 	while (l-- > 0)
 		*wp++ = readw(a);
 }
 
-static inline void mcf_outsw(void *a, unsigned char *p, int l)
+static inline void mcf_outsw(void __iomem *a, unsigned char *p, int l)
 {
 	u16 *wp = (u16 *) p;
 	while (l-- > 0)