| Message ID | 1335779839-30420-1-git-send-email-jaccon.bastiaansen@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Monday 30 April 2012, Jaccon Bastiaansen wrote: > The use of the inw/outw functions by the cs89x0 platform driver > results in NULL pointer references. > > Signed-off-by: Jaccon Bastiaansen <jaccon.bastiaansen@gmail.com> > --- > drivers/net/ethernet/cirrus/cs89x0.c | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) It's actually broken on most platforms already, and the #ifdef is about to go away since IXP2xxx is getting removed in v3.5. > diff --git a/drivers/net/ethernet/cirrus/cs89x0.c b/drivers/net/ethernet/cirrus/cs89x0.c > index b9406cb..95737e9 100644 > --- a/drivers/net/ethernet/cirrus/cs89x0.c > +++ b/drivers/net/ethernet/cirrus/cs89x0.c > @@ -369,6 +369,18 @@ writeword(unsigned long base_addr, int portno, u16 value) > { > __raw_writel(value, base_addr + (portno << 1)); > } > +#elif defined(CONFIG_CS89x0_PLATFORM) > +static u16 > +readword(unsigned long base_addr, int portno) > +{ > + return ioread16(base_addr + portno); > +} > + > +static void > +writeword(unsigned long base_addr, int portno, u16 value) > +{ > + iowrite16(value, base_addr + portno); > +} > #else > static u16 > readword(unsigned long base_addr, int portno) I think the best solution would be to always using ioread32/iowrite32 in the #else path, and change the ISA code to do an ioport_map for the base address, passing around the virtual address as an __iomem pointer. Arnd -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Arnd, 2012/4/30 Arnd Bergmann <arnd@arndb.de>: > On Monday 30 April 2012, Jaccon Bastiaansen wrote: >> The use of the inw/outw functions by the cs89x0 platform driver >> results in NULL pointer references. >> >> Signed-off-by: Jaccon Bastiaansen <jaccon.bastiaansen@gmail.com> >> --- >> drivers/net/ethernet/cirrus/cs89x0.c | 12 ++++++++++++ >> 1 files changed, 12 insertions(+), 0 deletions(-) > > It's actually broken on most platforms already, and the #ifdef is > about to go away since IXP2xxx is getting removed in v3.5. > >> diff --git a/drivers/net/ethernet/cirrus/cs89x0.c b/drivers/net/ethernet/cirrus/cs89x0.c >> index b9406cb..95737e9 100644 >> --- a/drivers/net/ethernet/cirrus/cs89x0.c >> +++ b/drivers/net/ethernet/cirrus/cs89x0.c >> @@ -369,6 +369,18 @@ writeword(unsigned long base_addr, int portno, u16 value) >> { >> __raw_writel(value, base_addr + (portno << 1)); >> } >> +#elif defined(CONFIG_CS89x0_PLATFORM) >> +static u16 >> +readword(unsigned long base_addr, int portno) >> +{ >> + return ioread16(base_addr + portno); >> +} >> + >> +static void >> +writeword(unsigned long base_addr, int portno, u16 value) >> +{ >> + iowrite16(value, base_addr + portno); >> +} >> #else >> static u16 >> readword(unsigned long base_addr, int portno) > > I think the best solution would be to always using ioread32/iowrite32 > in the #else path, and change the ISA code to do an ioport_map > for the base address, passing around the virtual address as an __iomem > pointer. > > Arnd > -- So if I understand you correctly you would like to have an iopart_map() call in the cs89x0_probe() function and use the return value of that iopart_map() call as ioaddr parameter of the cs89x0_probe1() function. Is this correct? This would make the cs89x0_probe() function similar to the cs89x0_platform_probe() function where the return value of the ioremap() call is used as ioaddr parameter of the cs89x0_probe1() function. But why do you want to convert the current 16 bit accesses in the #else path to 32 bit accesses? Why not using ioread16()/iowrite16()? Regards, Jaccon -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 03 May 2012, Jaccon Bastiaansen wrote: > So if I understand you correctly you would like to have an > iopart_map() call in the cs89x0_probe() function and use the return > value of that iopart_map() call as ioaddr parameter of the > cs89x0_probe1() function. Is this correct? This would make the > cs89x0_probe() function similar to the cs89x0_platform_probe() > function where the return value of the ioremap() call is used as > ioaddr parameter of the cs89x0_probe1() function. Correct. Currently the code relies on some platforms defining the inw/outw functions to the same thing as readw/writew, which is not a correct behaviour. If we change it to always use ioread16/iowrite16, it will be correct in either case. > But why do you want to convert the current 16 bit accesses in the > #else path to 32 bit accesses? Why not using ioread16()/iowrite16()? Sorry, typo on my side, I meant ioread16/iowrite16. Arnd -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/cirrus/cs89x0.c b/drivers/net/ethernet/cirrus/cs89x0.c index b9406cb..95737e9 100644 --- a/drivers/net/ethernet/cirrus/cs89x0.c +++ b/drivers/net/ethernet/cirrus/cs89x0.c @@ -369,6 +369,18 @@ writeword(unsigned long base_addr, int portno, u16 value) { __raw_writel(value, base_addr + (portno << 1)); } +#elif defined(CONFIG_CS89x0_PLATFORM) +static u16 +readword(unsigned long base_addr, int portno) +{ + return ioread16(base_addr + portno); +} + +static void +writeword(unsigned long base_addr, int portno, u16 value) +{ + iowrite16(value, base_addr + portno); +} #else static u16 readword(unsigned long base_addr, int portno)
The use of the inw/outw functions by the cs89x0 platform driver results in NULL pointer references. Signed-off-by: Jaccon Bastiaansen <jaccon.bastiaansen@gmail.com> --- drivers/net/ethernet/cirrus/cs89x0.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)