Hi Wolfgang,
+void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
+{
+ int res;
+ struct ihs_fpga *fpga_0 = (struct ihs_fpga
*)CONFIG_SYS_FPGA_BASE(0);
+
+ switch (fpga) {
+ case 0:
+ out_le16((u16 *)fpga_0 + reg / sizeof(u16), data);
+ break;
+ default:
+ res = mclink_send(fpga - 1, reg, data);
+ if (res < 0)
+ printf("mclink_send reg %02x data %04x returned %d\n",
+ reg, data, res);
+ break;
+ }
+}
So no memory mapping here. That's the reason for all this fuzz.
OK - but I don't see why mclink_send() could not be used in the same
way, i. e. taking a struct member as argument?
Certainly it *can* be used using *pointers* to struct members as
argument.
What I stated yesterday was:
"We have FPGAs that are memory mapped and others that are not. They
must be accessed by the same drivers. So the alternative would be to
create FPGA instances at address NULL and getting the register offesets
by casting pointers to u16. Not very nice either."
Apparently all your FPGA accesses are for u16 data only? Then you
could rewrite fpga_set_reg() similar to this:
int fpga_set_reg(u32 fpga, u16 *addr, u16 data)
With
struct ihs_fpga {
u16 reflection_low; /* 0x0000 */
u16 versions; /* 0x0002 */
...
};
and
void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
the call looks like
fpga_set_reg(k, REG(reflection_low), REFLECTION_TESTPATTERN);
To use
int fpga_set_reg(u32 fpga, u16 *addr, u16 data)
we need something like
struct ihs_fpga system_fpgas[] = {
(struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(0),
(struct ihs_fpga *)NULL,
(struct ihs_fpga *)NULL,
(struct ihs_fpga *)NULL,
};
and
fpga_set_reg(k, system_fpgas[k].reflection_low,
REFLECTION_TESTPATTERN);
In fpga_set_reg() it would look like
mclink_send(fpga - 1, (u16)addr, data);
I personally dislike all this NULL-pointer passing and casting. But
YMMV. If that's the u-boot-way I will be happy to change it.
And voila, no need to move away from the C structs.
Note 1: You print an error message if mclink_send() returns an error;
I think this error should propagate, i. e. this function
should not return void.
Yep. Happy were the days when accessing FPGA registers could not fail
:(
Note 2: I changed the type of the first arg into "u32" so it is
consistent with the other args. Mixing native types (unsigned
int) here and defined types (as u16 instead of unsigned
short) looks weird to me.
I wanted to express the 16 bit io-implementation of our address/data
busses, while the fpga index is simply a number. Maybe that is
information overkill :)
Cheers
Dirk
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
