Hi Geert,
Hi Michael,
On Sat, Aug 30, 2014 at 3:25 AM, Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
that's certainly correct - the EtherNAT version of the USB driver didn't
need the ROM accessors but the version including NetUSBee support does for
sure.
Thanks!
Ack'ed-by: Michael Schmitz <schmitzmic@xxxxxxxxx>
If CONFIG_ATARI_ROM_ISA=n:
drivers/usb/host/isp116x.h: In function ‘isp116x_write_addr’:
drivers/usb/host/isp116x.h:382: error: implicit declaration of function
‘isa_rom_writew_raw’
drivers/usb/host/isp116x.h: In function ‘isp116x_raw_write_data16’:
drivers/usb/host/isp116x.h:394: error: implicit declaration of function
‘isa_rom_writew’
drivers/usb/host/isp116x.h: In function ‘isp116x_read_data16’:
drivers/usb/host/isp116x.h:402: error: implicit declaration of function
‘isa_rom_readw_raw’
drivers/usb/host/isp116x.h: In function ‘isp116x_raw_read_data16’:
drivers/usb/host/isp116x.h:411: error: implicit declaration of function
‘isa_rom_readw’
The isa_rom_*() calls should depend on CONFIG_ATARI_ROM_ISA instead of
on CONFIG_ATARI.
Upon closer look, I think I broke the NetUSBee support if CONFIG_ATARI_ROM_ISA
is not set, as the mapping from isp_*() to low-level I/O accessors is
different.
That is right - CONFIG_ATARI_ROM_ISA is required for proper function of
the NetUSBee. The option should have been set by Kconfig magic anyway -
need to check that but obviously you did succeed to configure it
without... That option ensures that the correct bus accessor is available.
BTW, what I have in this case is:
#ifdef CONFIG_ATARI
/*
* 16 bit data bus byte swapped in hardware on both Atari variants.
* EtherNAT variant of ISP1160 integration is memory mapped at 0x800000XX,
* and uses regular readw/__raw_readw (but semantics swapped).
* NetUSBee variant is hooked up through ROM port and uses ROM port
* IO access, with fake IO address of 0x3XX.
* Selection of the appropriate accessors relies on ioremapped addresses
still
* retaining the 0x3XX bit.
*/
#define isp_readw(p) ((((unsigned long)(__pa(p)) & 0x00000F00) ==
0x00000300UL) ? isa_rom_readw_raw(__pa(p)) : __raw_readw((p)))
#define isp_writew(v, p) ((((unsigned long)(__pa(p)) & 0x00000F00) ==
0x00000300UL) ? isa_rom_writew_raw((v), __pa(p)) : __raw_writew((v), (p)))
#define isp_raw_readw(p) ((((unsigned long)(__pa(p)) & 0x00000F00) ==
0x00000300UL) ? isa_rom_readw(__pa(p)) : readw((p)))
#define isp_raw_writew(v, p) ((((unsigned long)(__pa(p)) & 0x00000F00)
== 0x00000300UL) ? isa_rom_writew((v), __pa(p)) : writew((v), (p)))
#else
/* sane hardware */
#define isp_readw readw
#define isp_writew writew
#define isp_raw_readw __raw_readw
#define isp_raw_writew __raw_writew
#endif
Much messier than yours due to the ioremap stuff.
On NetUSBee with CONFIG_ATARI_ROM_ISA=y, we now have:
#ifdef CONFIG_ATARI_ROM_ISA
#define isp_readw(p) (__raw_readw((p)))
#define isp_writew(v, p) (__raw_writew((v), (p)))
#define isp_raw_readw(p) (readw((p)))
#define isp_raw_writew(v, p) (writew((v), (p)))
#else
/* sane hardware */
#define isp_readw readw
#define isp_writew writew
#define isp_raw_readw __raw_readw
#define isp_raw_writew __raw_writew
#endif
Before my patch, there was an "#ifdef CONFIG_ATARI", so the plain isp_*()
mapped to the raw ops on Atari, but to the plain ones elsewhere, while the
isp_raw_*() mapped to the plain ops on Atari, but the raw ops elsewhere.
Correct - the semantics of raw vs. normal access is reversed on _both_
the Atari variants of the ISP1160 due to byte-swapping in the bus
interface.
Now, if NetUSBee is _not_ configured, and CONFIG_ATARI_ROM_ISA is not
set, your patch selects the 'sane' option for Atari (the EtherNAT
interface). That won't work - the weird vs. sane selection needs to be
done based on platform not bus type. I missed that earlier.
The plain ops do byteswapping on m68k. If you change the mapping, perhaps
you can get rid of some of the #ifdefs in drivers/usb/host/isp116x-hcd.c?
We had tried that to no avail. The #ifdefs are there only for cases
where the buffer is not aligned properly to a 16-bit boundary, so swaps
of the first and last word would be messed up.
The original driver uses normal reads for everything but sending USB
data to the chip, assuming a native endian bus interface for the chip so
byte swapping is done by the readw / writew. USB data are prepared as
little endian in the memory buffer, so no swapping must be done there.
The Atari implementation has the wires (bytes) crossed in hardware (just
like with the IDE interface - why stop once you've made a stupid design
mistake). No swapping by readw/writew must be done in software (taken
care by hardware) but we must swap the USB packet data (to cancel the
swap done in hardware).
I've pondered this many times and seen no way to avoid the defines.
P.S. Shall I revert my patch for now?
Yes, please. I forgot about the consequences for the EtherNAT earlier.
Still need to ensure CONFIG_ATARI_ROM_ISA is set for NetUSBee though.
Maybe select ATARI_ROM_ISA if ATARI_USB is selected in Kconfig.bus, just
to be safe?
The help text there could use an update as well - mention NetUSBee as
user of ATARI_ROM_ISA in addition to EtherNEC, and mention the need for
ATARI_ROM_ISA for the NetUSBee in the ATARI_USB section, if you rather
not pre-select ATARI_ROM_ISA there. Does this make sense?
Cheers,
Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
