Dear Dirk Eibach,
In message cf2913653766edc89705f49e08758df7@gdsys.cc you wrote:
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."
Yes, you wrote that. I did not understand it then, nor do I understand it now. What do you mean by "create FPGA instances at address NULL"? in any case, "getting the register offsets by casting pointers to u16" makes no sense, as this is exactly what we try to avoid. By referencing a C struct you do NOT use any offsets. You use references to struct elements; where needed, you let the compiler to the address calculations (and the type checking).
No offsets. No casts.
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);
Mo. It should look like that:
int fpga_set_reg(u32 fpga, u16 *addr, u16 data)
rc = fpga_set_reg(k, &ptr->reflection_low, REFLECTION_TESTPATTERN); if (rc) ...
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, };
Is this not redundant? If you have an array of pointers (addresses), then you can identify the FPGA by using the pointer, and don't need to pass both the pointer _and_ the index to the fpga_{g,s}et_reg() functions?
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.
Why would you need casts? The whole purpose of this is to _avoid_ casts, so that the compiler has a chance to make sure all accesses are performed using the correct alignment and size of the respective data types.
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 :)
Eventually you can drop the index alltogether when you just pass a pointer?
Best regards,
Wolfgang Denk