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

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Michael,

Am 08.06.2018 um 21:28 schrieb Michael Karcher:
> Let me add my 2 cents as main author of that code:
...
>
> actually, the the block_input / block_output functions were the reason I
> included the lib8390.c file. Except for xs100_write / xs100_read, they are
> a verbatim copy from ax88796.c I'm not that enthusiastic about that idea
> anymore, but did not get around to improve it. I added a customization
> point to ax88796 for a custom block_input / block_output, because the 8390
> core already provides that customization point. What I really need is a
> customization point just for the 8390-remote-DMA-via-MMIO functions (i.e.
> xs100_write, xs100_read) instead of the whole block transfer core that
> also sets up the remote DMA engine and tries to re-initialize the card in
> case of unexplained problems.

OK, so essentially change

         if (ei_local->word16) {
                 ioread16_rep(nic_base + NE_DATAPORT, buf, count >> 1);
                 if (count & 0x01)
                         buf[count-1] = ei_inb(nic_base + NE_DATAPORT);

         } else {
                 ioread8_rep(nic_base + NE_DATAPORT, buf, count);
         }

to something like

         if (ax->block_read) {
		ax->block_read(dev, buf, count);
	} else if (ei_local->word16) {
                 ioread16_rep(nic_base + NE_DATAPORT, buf, count >> 1);
                 if (count & 0x01)
                         buf[count-1] = ei_inb(nic_base + NE_DATAPORT);

         } else {
                 ioread8_rep(nic_base + NE_DATAPORT, buf, count);
         }

and populate ax->block_read() and ax_block_write() from platform data, 
instead of substituting ax_block_input() / ax_block_output() wholesale?

I must be missing something here.

> This should get rid of
>  - xs100_block_output
>  - xs100_block_input (which has the calls to ax_reset_8390 and
> ax_NS8390_init)
>  - ax_reset_8390
>  - and thus the include of lib8390.c (which should be included only by
> ax88796.c, not by xsurf100.c)

I've got an (untested) patch that just exports ax_NS8390_init() via the 
ax_device struct, and gets rid of the lib8390.c include that way, but 
the above solution would be a lot cleaner. Adds one test for 
ax->block_read on the critical path but we already have the test for 
ei_local->word16 there. May need rearranging the tests so the majority 
of ax88796 users isn't impacted.

Anyway, your code, your call.

Cheers,

	Michael


>
> Regards,
>   Michael Karcher
>