Re: Davicom DM9000A on MPC5200B (powerpc) works using a dirty offsetting and byte trick
Henk, On Montag, 9. März 2009, Henk Stegeman wrote: > I don't understand how this would work, > > Now I do one byte-swap, which works. > -I byteswap in software, for 16-bit cycles by byte swapping and for 8 > bit cycles by adding an offset of 1. > (The byte swapping on the chipselect is off) > > Your advice includes two byteswaps, one by re-routing the data bus and > one by enabling the byte swap on the chip-select. My experience is the chip select byte swap feature only works correctly if you connect a little endian device like I showed you. > Or does one of them not really swap bytes? Let me show you how it works. You must ensure you can write/read data in any data width, but at the side of the little endian device it always must be in the correct endianess. This example uses a 32 bit data width, but it works for 16 bit, too. - LE shows how a real litte endian CPU would write data - MPC1 shows how MPC5200 will do it, without any byte swap and DO at the MPC5200 side is also D0 at the little endian device - MPC2 shows how MPC5200 will do it, with D0 at the MPC5200 side is D24 at the little endian device - MPC3 shows how MPC5200 will do it, connected like MPC2 but also the chip select byte swap feature enabled - LE DEV shows how the little endian device expects the data You want to write this data at the given offset into the little endian device: Bytes: 0:0x34, 1:0x12, 2:0x78, 3:0x56 Worte: 0:0x1234 2:0x5678 LONG:0:0x56781234 Writing as bytes: Bytes: 0:0x34, 1:0x12, 2:0x78, 3:0x56 Offset LEMPC1 MPC2 MPC3 LE DEV 0 0x34 0x56 0x34 0x34 0x34 1 0x12 0x78 0x12 0x12 0x12 2 0x78 0x12 0x78 0x78 0x78 3 0x56 0x34 0x56 0x56 0x56 ------> these are correct > this is wrong Writing as words: Words: 0:0x1234 2:0x5678 Offset LE MPC1 MPC2 MPC3 LE DEV 0 0x34 0x78 0x12 0x34 0x34 (1) 0x12 0x56 0x34 0x12 0x12 2 0x78 0x34 0x56 0x78 0x78 (3) 0x56 0x12 0x78 0x56 0x56 -------> these are correct ----> these are wrong Writing as longs: LONG: 0:0x56781234 Offset LE MPC1 MPC2 MPC3 LE DEV 0 0x34 0x34 0x56 0x34 0x34 (1) 0x12 0x12 0x78 0x12 0x12 (2) 0x78 0x78 0x12 0x78 0x78 (3) 0x56 0x56 0x34 0x56 0x56 --- -------> these are correct --> this is wrong So, the MPC3 example always writes correct data. Hope it helps, Juergen -- Pengutronix e.K. | Juergen Beisert | Linux Solutions for Science and Industry | Phone: +49-8766-939 228 | Vertretung Sued/Muenchen, Germany | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de/ | ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Davicom DM9000A on MPC5200B (powerpc) works using a dirty offsetting and byte trick
Juergen, I don't understand how this would work, Now I do one byte-swap, which works. -I byteswap in software, for 16-bit cycles by byte swapping and for 8 bit cycles by adding an offset of 1. (The byte swapping on the chipselect is off) Your advice includes two byteswaps, one by re-routing the data bus and one by enabling the byte swap on the chip-select. Or does one of them not really swap bytes? Henk On Sat, Mar 7, 2009 at 11:09 AM, Juergen Beisert wrote: > Henk, > > On Freitag, 20. Februar 2009, Henk Stegeman wrote: >> - Any suggestions to what could be wrong here? Or does the MPC5200 in >> this case only byte swap u16 reads, but a u8 read is unchanged? > > You should not follow the Freescale bus signal names when you connect your > external little endian device. Otherwise the offsets are always wrong. > > Do it in this way instead: > > MPC LE Device > D[0..7] <-> D[24..31] > D[8..15] <-> D[16..23] > D[16..23] <-> D[8..15] > D[24..31] <-> D[0..7] > > If you connect your device in such a way, just enable CS's byte swap feature > depending on your bus size and you are done (no additional software > manipulation required). Now you can write bytes, words or longs and you will > always write the correct data into the corresponding device register. > > Hope it helps > Juergen > > -- > Pengutronix e.K. | Juergen Beisert | > Linux Solutions for Science and Industry | Phone: +49-8766-939 228 | > Vertretung Sued/Muenchen, Germany | Fax: +49-5121-206917- | > Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de/ | > ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Davicom DM9000A on MPC5200B (powerpc) works using a dirty offsetting and byte trick
Henk, On Freitag, 20. Februar 2009, Henk Stegeman wrote: > - Any suggestions to what could be wrong here? Or does the MPC5200 in > this case only byte swap u16 reads, but a u8 read is unchanged? You should not follow the Freescale bus signal names when you connect your external little endian device. Otherwise the offsets are always wrong. Do it in this way instead: MPC LE Device D[0..7] <-> D[24..31] D[8..15] <-> D[16..23] D[16..23] <-> D[8..15] D[24..31] <-> D[0..7] If you connect your device in such a way, just enable CS's byte swap feature depending on your bus size and you are done (no additional software manipulation required). Now you can write bytes, words or longs and you will always write the correct data into the corresponding device register. Hope it helps Juergen -- Pengutronix e.K. | Juergen Beisert | Linux Solutions for Science and Industry | Phone: +49-8766-939 228 | Vertretung Sued/Muenchen, Germany | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de/ | ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Davicom DM9000A on MPC5200B (powerpc) works using a dirty offsetting and byte trick
On Fri, Feb 20, 2009 at 2:51 AM, Henk Stegeman wrote:
> I have the following definition for this network device in my dts:
>
> enet1:ether...@fb00 {
> #size-cells = <1>;
> #address-cells = <1>;
The ethernet node doesn't have any children, so drop the #size-cells
and #address-cells properties.
> device_type = "network";
Drop device_type too. It only makes sense if you're running real OpenFirmware.
> compatible = "davicom,dm9000";
> reg = <0xfb00 0x0002 // DM9000 Address
> register
> 0xfb02 0x0002>; // DM9000 Data register
Just make this reg = <0xfb00 0x0004>;. Specify in the
documentation for the "davicom,dm9000" binding that the address
register is at offset 0 and the data register is at offset 2.
Actually, since this is the local plus bus, you should have this node
as a child of the localplus node. Like this:
localbus {
compatible = "fsl,mpc5200b-lpb", "fsl,mpc5200-lpb", "simple-bus";
#address-cells = <2>; // first cell is CS#, second is offset
#size-cells = <1>;
ranges = <1 0 0xfb00 0x4>; // CS#1, offset 0, mapped to
0xfb00, size=4
enet1: ether...@fb00 {
compatible = "davicom,dm9000";
reg = <0xfb00 0x0002// DM9000 Address register
mac-address = [ 00 00 00 00 00 00 ]; // Filled in by u-boot
interrupts = <1 1 0>; // External interrupt 1
};
};
Doing it this way provides drivers with the ability to get the chip
select number, which is important if you ever decide to use the
Localplus fifo to transfer to/from the device.
> To pass this information to the unmodified DM9000 driver I put
> together a wrapper arch/powerpc/sysdev/dm9000_dev.c (see below) for
> the of_ part (I reused most of the work from
> arch/powerpc/sysdev/tsi108_dev.c):
>
> Because the dm9000 driver uses the data address both as pointer to u16
> and pointer to u8, this works for a little-endian cpu to the
> little-endian dm9000, but not for the big endian MPC5200 to little
> endian dm9000.
> So I add an offset of 1 to this pointer when it is passed to the
> dm9000 driver. This offset of 1 is wrong for the u16 accesses of the
> DM9000 driver, besides that the data needs to be byteswapped for the
> dm9000 driver.
If the driver cannot handle big endian machines, then it is a driver
bug. Don't be afraid to modify the driver to fix this and send a
patch.
> For these two reasons I wrote the functions dm9000_outblk_16bit
> dm9000_intblk_16bit dm9000_dumpblk_16bit which are passed via the
> platform_data.
>
> Apart from comments on my assumptions I have the following questions
> about this situation and my code:
> - Is it the right way to make a separate arch/powerpc/sysdev/*_dev.c
> for reading the device-tree and passing it to a driver, in stead of
> adding it to the driver itself?
Personally, I'd use the of_platform bus infrastructure to probe the
new device. Register an of_platform_driver which will bind against
the device node for the ethernet device. Then you have a choice:
option 1) Your driver's probe method can either create a child
platform device which the original driver can bind against with the
correct pdata, or
option 2) your new driver can call into the original driver at a point
that bypasses the platform bus bindings (because they are handled by
the of_platform bus instead).
I typically choose option 2 because it requires less overhead and less
memory (one fewer probe call and one fewer struct device), but it will
probably require a little bit of refactoring the original driver to
provide call points to bypass the platform bus binding bit of the
driver. I've done this many times, but it does depend on the driver
maintainer being okay with multiple bus bindings (platform and
of_platform) for a driver. This is an ongoing debate.
See drivers/video/xilinxfb.c for an example of a driver with both
platform and of_platform bus bindings. You'll notice at the end of
the file that both a platform driver and an of_platform driver are
getting registered.
> - I think the best solution to handle the separate address and data
> register is the 2 entry register array in the device tree as above,
> this accounts for an odd connection to the DM9000's CMD pin. Agree?
no. If CMD could appear at a different offset, then define an
optional property (maybe cmd-reg-offset = <2>;) to handle the case
where CMD appears at a different offset.
> - The MPC5200's chip-select can be configured to do byte-swapping on
> read and write, however when I configured it as such and I removed my
> offsetting by 1 and byte-swapping code It didn't work.
> - Any suggestions to what could be wrong here? Or does the MPC5200 in
> this case only byte swap u16 reads, but a u8 read is unchanged?
This I don't know. I'd have to play with it to figure it out.
> - What about how the DM9000 driver de
Davicom DM9000A on MPC5200B (powerpc) works using a dirty offsetting and byte trick
Hello,
My board has a Davicom DM9000A ethernet controller on the MPC5200B's
Local Plus bus.
With the following setup and code the controller *works fine* with the
MPC5200B, but I think the code + setup to make it happen could be a
lot cleaner.
The DM9000A is setup for 16 bit mode, and wired as such to the
MPC5200B, A1 is wired to the DM9000 CMD pin.
The DM9000 is accessed trough two registers, address and data register.
The chip-select used is CS1 and it is setup in U-boot as follows:
/* Start addres: FB00
Data Bus Size: 2 bytes
Address Bus Size: 16 bit
Mux off.
*/
#define CFG_CS1_CFG = 0x04041500
The resulting physical DM9000 register map is:
FB00 : address register
FB02 : data register
I have the following definition for this network device in my dts:
enet1:ether...@fb00 {
#size-cells = <1>;
#address-cells = <1>;
device_type = "network";
compatible = "davicom,dm9000";
reg = <0xfb00 0x0002// DM9000 Address
register
0xfb02 0x0002>; // DM9000 Data register
mac-address = [ 00 00 00 00 00 00 ]; // Filled in by u-boot
interrupts = <1 1 0>; // External interrupt 1
interrupt-parent = <&mpc5200_pic>;
};
To pass this information to the unmodified DM9000 driver I put
together a wrapper arch/powerpc/sysdev/dm9000_dev.c (see below) for
the of_ part (I reused most of the work from
arch/powerpc/sysdev/tsi108_dev.c):
Because the dm9000 driver uses the data address both as pointer to u16
and pointer to u8, this works for a little-endian cpu to the
little-endian dm9000, but not for the big endian MPC5200 to little
endian dm9000.
So I add an offset of 1 to this pointer when it is passed to the
dm9000 driver. This offset of 1 is wrong for the u16 accesses of the
DM9000 driver, besides that the data needs to be byteswapped for the
dm9000 driver.
For these two reasons I wrote the functions dm9000_outblk_16bit
dm9000_intblk_16bit dm9000_dumpblk_16bit which are passed via the
platform_data.
Apart from comments on my assumptions I have the following questions
about this situation and my code:
- Is it the right way to make a separate arch/powerpc/sysdev/*_dev.c
for reading the device-tree and passing it to a driver, in stead of
adding it to the driver itself?
- I think the best solution to handle the separate address and data
register is the 2 entry register array in the device tree as above,
this accounts for an odd connection to the DM9000's CMD pin. Agree?
- The MPC5200's chip-select can be configured to do byte-swapping on
read and write, however when I configured it as such and I removed my
offsetting by 1 and byte-swapping code It didn't work.
- Any suggestions to what could be wrong here? Or does the MPC5200 in
this case only byte swap u16 reads, but a u8 read is unchanged?
- What about how the DM9000 driver deals with this u8 read and u16
read, is this correct?
Thanks,
Henk
/*
* dm9000 device setup code
*
* Maintained by Henk Stegeman < henk.stege...@gmail.com >
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; either version 2 of the License, or (at your
* option) any later version.
*/
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#ifdef DEBUG
#define DBG(fmt...) do { printk(fmt); } while(0)
#else
#define DBG(fmt...) do { } while(0)
#endif
#define U16_SWAP_BYTES(v) (((v << 8) & 0xff00) | ((v >> 8) & 0x00ff))
static void dm9000_outblk_16bit(void __iomem *reg, void *data, int count)
{
u16 * pU16 = data;
count = ( count + 1 ) >> 1;
while (count > 0) {
/* subtract 1 from reg to get back at the 16bit reg addr */
out_be16(reg - 1, U16_SWAP_BYTES(* pU16));
pU16 ++;
count --;
}
}
static void dm9000_inblk_16bit(void __iomem *reg, void *data, int count)
{
u16 * pU16 = data;
count = ( count + 1 ) >> 1;
while (count > 0) {
/* subtract 1 from reg to get back at the 16bit reg addr */
* pU16 = in_be16(reg - 1);
* pU16 = U16_SWAP_BYTES(* pU16);
pU16 ++;
count --;
}
}
static void dm9000_dumpblk_16bit(void __iomem *reg, int count)
{
volatile u16 tmp;
count = ( count + 1 ) >> 1;
while (count > 0) {
/* subtract 1 from reg to get back at the 16bit reg addr */
tmp = in_be16(reg - 1);
count --;
}
}
#define NUM_DM9000_RESOURCES 3
static int __init dm9000_eth_of_init(void)
{
st
