subject:"\[PATCH 10\/10\] net\: New ax88796 platform driver for Amiga X\-Surf 100 Zorro board \(m68k\)"

Re: [PATCH 10/10] net: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)

2018-04-18 Thread Geert Uytterhoeven

Hi Michael,

On Wed, Apr 18, 2018 at 12:35 AM, Michael Schmitz  wrote:
> On Wed, Apr 18, 2018 at 1:53 AM, Geert Uytterhoeven
>  wrote:
>> On Tue, Apr 17, 2018 at 12:04 AM, Michael Schmitz  
>> wrote:
>>> Add platform device driver to populate the ax88796 platform data from
>>> information provided by the XSurf100 zorro device driver.
>>> This driver will have to be loaded before loading the ax88796 module,
>>> or compiled as built-in.

>>> --- /dev/null
>>> +++ b/drivers/net/ethernet/8390/xsurf100.c

>>> +static int xsurf100_probe(struct zorro_dev *zdev,
>>> + const struct zorro_device_id *ent)
>>> +{
>>
>>> +   /* error handling for ioremap regs */
>>> +   if (!ax88796_data.base_regs) {
>>> +   dev_err(>dev, "Cannot ioremap area %p (registers)\n",
>>> +   (void *)zdev->resource.start);
>>
>> Please use %pR to format struct resource.
>> Documentation/core-api/printk-formats.rst
>
> The driver uses ioremap to map two subsections of the mem resource for
> two different purposes - control register access, and ring buffer
> access. The output of %pR may be misleading here (wrong size), and
> even more so below.

Sorry, I missed it's the same resource.

FWIW, if you want to print a phys_addr_t or resource_size_t, you can
use %pa, e.g.

   dev_err(..., "... %pa ...", ... >resource.start ...);

>>> +   /* error handling for ioremap data */
>>> +   if (!ax88796_data.data_area) {
>>> +   dev_err(>dev, "Cannot ioremap area %p (32-bit 
>>> access)\n",
>>> +   (void *)zdev->resource.start + 
>>> XS100_8390_DATA32_BASE);
>>
>> %pR
>
> I've added the offset into the mem resource here to clarify what we've
> tried to map.

That's an alternative solution, fine for me.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/10] net: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)

2018-04-17 Thread Michael Schmitz

Hi Geert,

On Wed, Apr 18, 2018 at 1:53 AM, Geert Uytterhoeven
 wrote:
>> --- /dev/null
>> +++ b/drivers/net/ethernet/8390/xsurf100.c
>> @@ -0,0 +1,411 @@
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF100 \
>> +   ZORRO_ID(INDIVIDUAL_COMPUTERS, 0x64, 0)
>
> Another long define to get rid of? ;-)

I decided to leave it that way - it doesn't stick out quite as badly
as the one in the ESP driver. Give me a yell if you insist.

Cheers,

  Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/10] net: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)

2018-04-17 Thread Michael Schmitz

Hi Geert,

thanks for your suggestions!

On Wed, Apr 18, 2018 at 1:53 AM, Geert Uytterhoeven
 wrote:
> Hi Michael,
>
> Thanks for your patch!
>
> On Tue, Apr 17, 2018 at 12:04 AM, Michael Schmitz  
> wrote:
>> Add platform device driver to populate the ax88796 platform data from
>> information provided by the XSurf100 zorro device driver.
>> This driver will have to be loaded before loading the ax88796 module,
>> or compiled as built-in.
>
> Is that really true? The platform device should be probed when both the
> device and driver have been registered, but order shouldn't matter.

Loading the xsurf100 module will pull in the ax88796 module, so order
does not matter. I'll drop that.

>
>> Signed-off-by: Michael Karcher 
>
> Missing "From: Michael Karcher ..."?

Fixed the authorship now - probably got mangled when squashing in my
local edits.

>
>> Signed-off-by: Michael Schmitz 
>
>> --- a/drivers/net/ethernet/8390/Kconfig
>> +++ b/drivers/net/ethernet/8390/Kconfig
>> @@ -30,7 +30,7 @@ config PCMCIA_AXNET
>>
>>  config AX88796
>> tristate "ASIX AX88796 NE2000 clone support"
>> -   depends on (ARM || MIPS || SUPERH)
>> +   depends on (ARM || MIPS || SUPERH || AMIGA)
>
> s/AMIGA/ZORRO/, for consistency with the below.

Will do.

>
>> select CRC32
>> select PHYLIB
>> select MDIO_BITBANG
>> @@ -45,6 +45,18 @@ config AX88796_93CX6
>> ---help---
>>   Select this if your platform comes with an external 93CX6 eeprom.
>>
>> +config XSURF100
>> +   tristate "Amiga XSurf 100 AX88796/NE2000 clone support"
>> +   depends on ZORRO
>> +   depends on AX88796
>
> It's a bit unfortunate the user has to enable _two_ config options to enable
> this driver.
>
> I see two solutions for that:
>
> 1) Hide the XSURF100 symbol, so it gets enabled automatically if AX88796 is
>enabled on a Zorro bus system:
>
> config XSURF100
> tristate
> depends on ZORRO
> default AX88796
>
> 2) Hide the AX88796 symbol, and let it be selected by XSURF100:
>
> config AX88796
> tristate "ASIX AX88796 NE2000 clone support" if !ZORRO
> depends on ARM || MIPS || SUPERH || ZORRO
> ...
>
> config XSURF100
> tristate "Amiga XSurf 100 AX88796/NE2000 clone support"
> depends on ZORRO
> select AX88796

I'll use the latter -

>> --- /dev/null
>> +++ b/drivers/net/ethernet/8390/xsurf100.c
>> @@ -0,0 +1,411 @@
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF100 \
>> +   ZORRO_ID(INDIVIDUAL_COMPUTERS, 0x64, 0)
>
> Another long define to get rid of? ;-)
>
>> +/* Hard reset the card. This used to pause for the same period that a
>> + * 8390 reset command required, but that shouldn't be necessary.
>> + */
>> +static void ax_reset_8390(struct net_device *dev)
>> +{
>> +   struct ei_device *ei_local = netdev_priv(dev);
>> +   unsigned long reset_start_time = jiffies;
>> +   void __iomem *addr = (void __iomem *)dev->base_addr;
>> +
>> +   netif_dbg(ei_local, hw, dev, "resetting the 8390 t=%ld...\n", 
>> jiffies);
>> +
>> +   ei_outb(ei_inb(addr + NE_RESET), addr + NE_RESET);
>> +
>> +   ei_local->txing = 0;
>> +   ei_local->dmaing = 0;
>> +
>> +   /* This check _should_not_ be necessary, omit eventually. */
>> +   while ((ei_inb(addr + EN0_ISR) & ENISR_RESET) == 0) {
>> +   if (time_after(jiffies, reset_start_time + 2 * HZ / 100)) {
>> +   netdev_warn(dev, "%s: did not complete.\n", 
>> __func__);
>> +   break;
>> +   }
>
> cpu_relax()?
>
> How long does this usually take? If > 1 ms, you can use e.g. msleep(1)
> instead of cpu_relax().

No idea how long this will take - the reset function is lifted
straight out of ax88796.c with no modifications whatsoever.

Come to think of it - it's exported as ei_local->reset_8390 there, so
there is no good reason for even duplicating the code that I can see.
I'lll drop it.

>
>> +   }
>> +
>> +   ei_outb(ENISR_RESET, addr + EN0_ISR);   /* Ack intr. */
>> +}
>
>> +   if (ei_local->dmaing) {
>> +   netdev_err(dev,
>> +  "DMAing conflict in %s "
>> +  "[DMAstat:%d][irqlock:%d].\n",
>
> Please don't split error messages, as that makes it more difficult to
> grep for them.

Again, found like that in ax88796.c. Will fix here (and eventually in
ax88796.c).

>> +  __func__,
>> +  ei_local->dmaing, ei_local->irqlock);
>> +   return;
>
>> +static int xsurf100_probe(struct zorro_dev *zdev,
>> + const struct zorro_device_id *ent)
>> +{
>
>> +   /* error handling for ioremap regs */
>> +   if

Re: [PATCH 10/10] net: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)

2018-04-17 Thread Geert Uytterhoeven

Hi Michael,

Thanks for your patch!

On Tue, Apr 17, 2018 at 12:04 AM, Michael Schmitz  wrote:
> Add platform device driver to populate the ax88796 platform data from
> information provided by the XSurf100 zorro device driver.
> This driver will have to be loaded before loading the ax88796 module,
> or compiled as built-in.

Is that really true? The platform device should be probed when both the
device and driver have been registered, but order shouldn't matter.

> Signed-off-by: Michael Karcher 

Missing "From: Michael Karcher ..."?

> Signed-off-by: Michael Schmitz 

> --- a/drivers/net/ethernet/8390/Kconfig
> +++ b/drivers/net/ethernet/8390/Kconfig
> @@ -30,7 +30,7 @@ config PCMCIA_AXNET
>
>  config AX88796
> tristate "ASIX AX88796 NE2000 clone support"
> -   depends on (ARM || MIPS || SUPERH)
> +   depends on (ARM || MIPS || SUPERH || AMIGA)

s/AMIGA/ZORRO/, for consistency with the below.

> select CRC32
> select PHYLIB
> select MDIO_BITBANG
> @@ -45,6 +45,18 @@ config AX88796_93CX6
> ---help---
>   Select this if your platform comes with an external 93CX6 eeprom.
>
> +config XSURF100
> +   tristate "Amiga XSurf 100 AX88796/NE2000 clone support"
> +   depends on ZORRO
> +   depends on AX88796

It's a bit unfortunate the user has to enable _two_ config options to enable
this driver.

I see two solutions for that:

1) Hide the XSURF100 symbol, so it gets enabled automatically if AX88796 is
   enabled on a Zorro bus system:

config XSURF100
tristate
depends on ZORRO
default AX88796

2) Hide the AX88796 symbol, and let it be selected by XSURF100:

config AX88796
tristate "ASIX AX88796 NE2000 clone support" if !ZORRO
depends on ARM || MIPS || SUPERH || ZORRO
...

config XSURF100
tristate "Amiga XSurf 100 AX88796/NE2000 clone support"
depends on ZORRO
select AX88796

> --- /dev/null
> +++ b/drivers/net/ethernet/8390/xsurf100.c
> @@ -0,0 +1,411 @@
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF100 \
> +   ZORRO_ID(INDIVIDUAL_COMPUTERS, 0x64, 0)

Another long define to get rid of? ;-)

> +/* Hard reset the card. This used to pause for the same period that a
> + * 8390 reset command required, but that shouldn't be necessary.
> + */
> +static void ax_reset_8390(struct net_device *dev)
> +{
> +   struct ei_device *ei_local = netdev_priv(dev);
> +   unsigned long reset_start_time = jiffies;
> +   void __iomem *addr = (void __iomem *)dev->base_addr;
> +
> +   netif_dbg(ei_local, hw, dev, "resetting the 8390 t=%ld...\n", 
> jiffies);
> +
> +   ei_outb(ei_inb(addr + NE_RESET), addr + NE_RESET);
> +
> +   ei_local->txing = 0;
> +   ei_local->dmaing = 0;
> +
> +   /* This check _should_not_ be necessary, omit eventually. */
> +   while ((ei_inb(addr + EN0_ISR) & ENISR_RESET) == 0) {
> +   if (time_after(jiffies, reset_start_time + 2 * HZ / 100)) {
> +   netdev_warn(dev, "%s: did not complete.\n", __func__);
> +   break;
> +   }

cpu_relax()?

How long does this usually take? If > 1 ms, you can use e.g. msleep(1)
instead of cpu_relax().


> +   }
> +
> +   ei_outb(ENISR_RESET, addr + EN0_ISR);   /* Ack intr. */
> +}

> +   if (ei_local->dmaing) {
> +   netdev_err(dev,
> +  "DMAing conflict in %s "
> +  "[DMAstat:%d][irqlock:%d].\n",

Please don't split error messages, as that makes it more difficult to
grep for them.

> +  __func__,
> +  ei_local->dmaing, ei_local->irqlock);
> +   return;

> +static int xsurf100_probe(struct zorro_dev *zdev,
> + const struct zorro_device_id *ent)
> +{

> +   /* error handling for ioremap regs */
> +   if (!ax88796_data.base_regs) {
> +   dev_err(>dev, "Cannot ioremap area %p (registers)\n",
> +   (void *)zdev->resource.start);

Please use %pR to format struct resource.
Documentation/core-api/printk-formats.rst

> +   /* error handling for ioremap data */
> +   if (!ax88796_data.data_area) {
> +   dev_err(>dev, "Cannot ioremap area %p (32-bit 
> access)\n",
> +   (void *)zdev->resource.start + 
> XS100_8390_DATA32_BASE);

%pR

> +static void xsurf100_remove(struct zorro_dev *zdev)
> +{
> +   struct platform_device *pdev;
> +   struct xsurf100_ax_plat_data *xs100;
> +
> +   pdev = zorro_get_drvdata(zdev);
> +   xs100 = dev_get_platdata(>dev);

struct platform_device *pdev = pdev = zorro_get_drvdata(zdev);
struct xsurf100_ax_plat_data *xs100 = dev_get_platdata(>dev);

[PATCH 10/10] net: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)

2018-04-16 Thread Michael Schmitz

Add platform device driver to populate the ax88796 platform data from
information provided by the XSurf100 zorro device driver.
This driver will have to be loaded before loading the ax88796 module,
or compiled as built-in.

Signed-off-by: Michael Karcher 
Signed-off-by: Michael Schmitz 
---
 drivers/net/ethernet/8390/Kconfig|   14 +-
 drivers/net/ethernet/8390/Makefile   |1 +
 drivers/net/ethernet/8390/xsurf100.c |  411 ++
 3 files changed, 425 insertions(+), 1 deletions(-)
 create mode 100644 drivers/net/ethernet/8390/xsurf100.c

diff --git a/drivers/net/ethernet/8390/Kconfig 
b/drivers/net/ethernet/8390/Kconfig
index fdc6734..0cadd45 100644
--- a/drivers/net/ethernet/8390/Kconfig
+++ b/drivers/net/ethernet/8390/Kconfig
@@ -30,7 +30,7 @@ config PCMCIA_AXNET
 
 config AX88796
tristate "ASIX AX88796 NE2000 clone support"
-   depends on (ARM || MIPS || SUPERH)
+   depends on (ARM || MIPS || SUPERH || AMIGA)
select CRC32
select PHYLIB
select MDIO_BITBANG
@@ -45,6 +45,18 @@ config AX88796_93CX6
---help---
  Select this if your platform comes with an external 93CX6 eeprom.
 
+config XSURF100
+   tristate "Amiga XSurf 100 AX88796/NE2000 clone support"
+   depends on ZORRO
+   depends on AX88796
+   ---help---
+ This driver is for the Individual Computers X-Surf 100 Ethernet
+ card (based on the Asix AX88796 chip). If you have such a card,
+ say Y. Otherwise, say N.
+
+ To compile this driver as a module, choose M here: the module
+ will be called xsurf100.
+
 config HYDRA
tristate "Hydra support"
depends on ZORRO
diff --git a/drivers/net/ethernet/8390/Makefile 
b/drivers/net/ethernet/8390/Makefile
index f975c2f..3715f8d 100644
--- a/drivers/net/ethernet/8390/Makefile
+++ b/drivers/net/ethernet/8390/Makefile
@@ -16,4 +16,5 @@ obj-$(CONFIG_PCMCIA_PCNET) += pcnet_cs.o 8390.o
 obj-$(CONFIG_STNIC) += stnic.o 8390.o
 obj-$(CONFIG_ULTRA) += smc-ultra.o 8390.o
 obj-$(CONFIG_WD80x3) += wd.o 8390.o
+obj-$(CONFIG_XSURF100) += xsurf100.o
 obj-$(CONFIG_ZORRO8390) += zorro8390.o 8390.o
diff --git a/drivers/net/ethernet/8390/xsurf100.c 
b/drivers/net/ethernet/8390/xsurf100.c
new file mode 100644
index 000..3caece0
--- /dev/null
+++ b/drivers/net/ethernet/8390/xsurf100.c
@@ -0,0 +1,411 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF100 \
+   ZORRO_ID(INDIVIDUAL_COMPUTERS, 0x64, 0)
+
+#define XS100_IRQSTATUS_BASE 0x40
+#define XS100_8390_BASE 0x800
+
+/* Longword-access area. Translated to 2 16-bit access cycles by the
+ * X-Surf 100 FPGA
+ */
+#define XS100_8390_DATA32_BASE 0x8000
+#define XS100_8390_DATA32_SIZE 0x2000
+/* Sub-Areas for fast data register access; addresses relative to area begin */
+#define XS100_8390_DATA_READ32_BASE 0x0880
+#define XS100_8390_DATA_WRITE32_BASE 0x0C80
+#define XS100_8390_DATA_AREA_SIZE 0x80
+
+#define __NS8390_init ax_NS8390_init
+
+/* force unsigned long back to 'void __iomem *' */
+#define ax_convert_addr(_a) ((void __force __iomem *)(_a))
+
+#define ei_inb(_a) z_readb(ax_convert_addr(_a))
+#define ei_outb(_v, _a) z_writeb(_v, ax_convert_addr(_a))
+
+#define ei_inw(_a) z_readw(ax_convert_addr(_a))
+#define ei_outw(_v, _a) z_writew(_v, ax_convert_addr(_a))
+
+#define ei_inb_p(_a) ei_inb(_a)
+#define ei_outb_p(_v, _a) ei_outb(_v, _a)
+
+/* define EI_SHIFT() to take into account our register offsets */
+#define EI_SHIFT(x) (ei_local->reg_offset[(x)])
+
+/* Ensure we have our RCR base value */
+#define AX88796_PLATFORM
+
+static unsigned char version[] =
+   "ax88796.c: Copyright 2005,2007 Simtec Electronics\n";
+
+#include "lib8390.c"
+
+/* from ne.c */
+#define NE_CMD EI_SHIFT(0x00)
+#define NE_RESET   EI_SHIFT(0x1f)
+#define NE_DATAPORTEI_SHIFT(0x10)
+
+/* Hard reset the card. This used to pause for the same period that a
+ * 8390 reset command required, but that shouldn't be necessary.
+ */
+static void ax_reset_8390(struct net_device *dev)
+{
+   struct ei_device *ei_local = netdev_priv(dev);
+   unsigned long reset_start_time = jiffies;
+   void __iomem *addr = (void __iomem *)dev->base_addr;
+
+   netif_dbg(ei_local, hw, dev, "resetting the 8390 t=%ld...\n", jiffies);
+
+   ei_outb(ei_inb(addr + NE_RESET), addr + NE_RESET);
+
+   ei_local->txing = 0;
+   ei_local->dmaing = 0;
+
+   /* This check _should_not_ be necessary, omit eventually. */
+   while ((ei_inb(addr + EN0_ISR) & ENISR_RESET) == 0) {
+   if (time_after(jiffies, reset_start_time + 2 * HZ / 100)) {
+   netdev_warn(dev, "%s: did not complete.\n", __func__);
+   break;
+   }
+   }
+
+   ei_outb(ENISR_RESET, addr + EN0_ISR);   /* Ack intr. */
+}
+
+struct xsurf100_ax_plat_data {
+

Re: [PATCH 10/10] net: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)

Re: [PATCH 10/10] net: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)

Re: [PATCH 10/10] net: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)

Re: [PATCH 10/10] net: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)

[PATCH 10/10] net: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)

5 matches

Site Navigation

Mail list logo

Footer information