[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Dirk Eibach]

Hello Wolfgang,

would you please have a look at this? I need some feedback to get this
finally sorted.

Cheers
Dirk

2013/5/15 Dirk Eibach <dirk.eibach at gdsys.cc>:
> Hello Wolfgang,
>
> sorry for the delay, I had to clean some other things on my ToDoList.
>
> I cleaned up my patchseries and wanted to start implementing your proposal.
> And ran into a problem. Remember, the basic reason for the the generic FPGA
> accessors was, that we have a certain amount of basically identical FPGAs
> which are only connected to the CPU in different ways. The drivers accessing
> those FPGAs should be agnostic of that and simply be able to access those
> FPGAs by index.
>
>
>> It appears you need both the element address and the offset in your
>> fpga_set_reg() code, so you should pass this information, like that
>> [note that we no longer need an array of addresses]:
>>
>> struct ihs_fpga fpga_ptr = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE;
>> ...
>>
>> void fpga_set_reg(u32 fpga, u16 *reg, off_t regoff, u16 data)
>> {
>>         if (fpga)
>>                 return mclink_send(fpga - 1, regoff, data);
>>
>>         return out_le16(reg, data);
>> }
>>
>>> where mclink_send() is the routine to access the non memory mapped
>>> FPGAs through the memory mapped FPGA:
>>>    int mclink_send(u8 slave, u16 addr, u16 data)
>>>    {
>>>         ...
>>>         fpga_set_reg(0, &system_fpgas[0].mc_tx_address, addr);
>>>         fpga_set_reg(0, &system_fpgas[0].mc_tx_data, data);
>>>         fpga_set_reg(0, &system_fpgas[0].mc_tx_cmd, (slave & 0x03) <<
>>> 14);
>>>    }
>>
>>
>> And this becomes:
>>
>>         fpga_set_reg(0,
>>                      &fpga_ptr->mc_tx_address,
>>                      offsetof(struct ihs_fpga, mc_tx_address),
>>                      addr);
>>
>> etc. Yes, this is long and ugly to write, so you may want to define a
>> helper macro like:
>>
>> #define FPGA_SET_REG(ix,fld,val)        \
>>         fpga_set_reg((ix),              \
>>         &fpga_ptr->fld,                 \
>>         offsetof(struct ihs_fpga, fld), \
>>         val)
>>
>> so this turns into a plain and simple
>>
>>         FPGA_SET_REG(0, mc_tx_address, addr);
>>         FPGA_SET_REG(0, mc_tx_data, data);
>>         FPGA_SET_REG(0, mc_tx_cmd, (slave & 0x03) << 14);
>
>
> This works nicely for our memory mapped FPGA. But how about the other FPGAs?
> I don't have a fpga_ptr for them. Setting it NULL would make FPGA_SET_REG
> dereference a NULL pointer.
> Certainly I might call
>         fpga_set_reg(1,
>                      NULL,
>                      offsetof(struct ihs_fpga, mc_tx_address),
>                      addr);
> but that would not be generic any more.
>
> Do you have any idea how to solve this?
>
>
> Cheers
> Dirk
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot