> -#define SMC_PUSH_DATA(lp, p, l) \
> +#define SMC_PUSH_DATA(lp, p, l) \
> do { \
> - if (SMC_32BIT(lp)) { \
> + void __iomem *__ioaddr = ioaddr; \
ioaddr is not a parameter passed to this macro.
> + if (SMC_32BIT(lp)) { \
> void *__ptr = (p); \
> int __len = (l); \
> - void __iomem *__ioaddr = ioaddr; \
> if (__len >= 2 && (unsigned long)__ptr & 2) { \
> __len -= 2; \
> - SMC_outsw(ioaddr, DATA_REG(lp), __ptr, 1); \
> + SMC_outsw(__ioaddr, DATA_REG(lp), __ptr, 1); \
You probably should use lp->base here, which is passed into this
macro, and should have the correct type.
> @@ -1072,7 +1072,7 @@ static const char * chip_ids[ 16 ] = {
> */ \
> __ptr -= 2; \
> __len += 2; \
> - SMC_SET_PTR(lp, \
> + SMC_SET_PTR(lp, \
> 2|PTR_READ|PTR_RCV|PTR_AUTOINC); \
> } \
> if (SMC_CAN_USE_DATACS && lp->datacs) \
This is just a whitespace change. Please put that into a different
patch.
Andrew
On 16. May 2024, at 16:13, Andrew Lunn <[email protected]> wrote:
> -#define SMC_PUSH_DATA(lp, p, l) \
>> +#define SMC_PUSH_DATA(lp, p, l) \
>> do { \
>> - if (SMC_32BIT(lp)) { \
>> + void __iomem *__ioaddr = ioaddr; \
>
> ioaddr is not a parameter passed to this macro.
Yes, most (all?) macros in this file rely on ioaddr being implicitly
defined in the surrounding scope.
> + if (SMC_32BIT(lp)) { \
>> void *__ptr = (p); \
>> int __len = (l); \
>> - void __iomem *__ioaddr = ioaddr; \
>> if (__len >= 2 && (unsigned long)__ptr & 2) { \
>> __len -= 2; \
>> - SMC_outsw(ioaddr, DATA_REG(lp), __ptr, 1); \
>> + SMC_outsw(__ioaddr, DATA_REG(lp), __ptr, 1); \
>
> You probably should use lp->base here, which is passed into this
> macro, and should have the correct type.
ioaddr is lp->base:
void __iomem *ioaddr = lp->base;
but the type information for ioaddr gets lost across multiple macro
boundaries which is why either __ioaddr or lp->base must be used when
calling the SMC_* macros. Both __ioaddr and lp->base work, but I guess
you prefer lp->base? I'm fine with both.
> @@ -1072,7 +1072,7 @@ static const char * chip_ids[ 16 ] = {
>> */ \
>> __ptr -= 2; \
>> __len += 2; \
>> - SMC_SET_PTR(lp, \
>> + SMC_SET_PTR(lp, \
>> 2|PTR_READ|PTR_RCV|PTR_AUTOINC); \
>> } \
>> if (SMC_CAN_USE_DATACS && lp->datacs) \
>
> This is just a whitespace change. Please put that into a different
> patch.
Ok, I'll change this in v2.
Thanks,
Thorsten
Use void __iomem pointers as parameters for mcf_insw() and mcf_outsw()
to align with the parameter types of readw() and writew().
Use lp->base instead of ioaddr when calling SMC_outsw(), SMC_outsb(),
SMC_insw(), and SMC_insb() to retain its type across macro boundaries
and 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 <[email protected]>
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Acked-by: Nicolas Pitre <[email protected]>
---
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)
---
drivers/net/ethernet/smsc/smc91x.h | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
index 45ef5ac0788a..a1523fb503a3 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)
@@ -1034,7 +1034,7 @@ static const char * chip_ids[ 16 ] = {
void __iomem *__ioaddr = ioaddr; \
if (__len >= 2 && (unsigned long)__ptr & 2) { \
__len -= 2; \
- SMC_outsw(ioaddr, DATA_REG(lp), __ptr, 1); \
+ SMC_outsw(lp->base, DATA_REG(lp), __ptr, 1); \
__ptr += 2; \
} \
if (SMC_CAN_USE_DATACS && lp->datacs) \
@@ -1042,12 +1042,12 @@ static const char * chip_ids[ 16 ] = {
SMC_outsl(__ioaddr, DATA_REG(lp), __ptr, __len>>2); \
if (__len & 2) { \
__ptr += (__len & ~3); \
- SMC_outsw(ioaddr, DATA_REG(lp), __ptr, 1); \
+ SMC_outsw(lp->base, DATA_REG(lp), __ptr, 1); \
} \
} else if (SMC_16BIT(lp)) \
- SMC_outsw(ioaddr, DATA_REG(lp), p, (l) >> 1); \
+ SMC_outsw(lp->base, DATA_REG(lp), p, (l) >> 1); \
else if (SMC_8BIT(lp)) \
- SMC_outsb(ioaddr, DATA_REG(lp), p, l); \
+ SMC_outsb(lp->base, DATA_REG(lp), p, l); \
} while (0)
#define SMC_PULL_DATA(lp, p, l) \
@@ -1080,9 +1080,9 @@ static const char * chip_ids[ 16 ] = {
__len += 2; \
SMC_insl(__ioaddr, DATA_REG(lp), __ptr, __len>>2); \
} else if (SMC_16BIT(lp)) \
- SMC_insw(ioaddr, DATA_REG(lp), p, (l) >> 1); \
+ SMC_insw(lp->base, DATA_REG(lp), p, (l) >> 1); \
else if (SMC_8BIT(lp)) \
- SMC_insb(ioaddr, DATA_REG(lp), p, l); \
+ SMC_insb(lp->base, DATA_REG(lp), p, l); \
} while (0)
#endif /* _SMC91X_H_ */
--
2.45.0
On Thu, May 16, 2024 at 05:56:12PM +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().
>
> Use lp->base instead of ioaddr when calling SMC_outsw(), SMC_outsb(),
> SMC_insw(), and SMC_insb() to retain its type across macro boundaries
> and 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 <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Acked-by: Nicolas Pitre <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
You could add a follow up patch which removes the
void __iomem *__ioaddr = ioaddr; lines and uses lp->base.
The code will then be more uniform.
Andrew
On Thu, 16 May 2024, Andrew Lunn wrote:
> You could add a follow up patch which removes the
> void __iomem *__ioaddr = ioaddr; lines and uses lp->base.
> The code will then be more uniform.
Beware, It is sometimes overridden with __ioaddr = lp->datacs.
Nicolas
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 <[email protected]>
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Acked-by: Nicolas Pitre <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
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(-)
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)
--
2.45.0