From mboxrd@z Thu Jan  1 00:00:00 1970
From: Dirk Eibach <dirk.eibach@gdsys.cc>
Date: Wed, 15 May 2013 11:23:59 +0200
Subject: [U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA
 accessors on all gdsys 405ep/ex boards
In-Reply-To: <20130508101333.6DDB438119E@gemini.denx.de>
References: <1367847325-21463-1-git-send-email-dirk.eibach@gdsys.cc>
	<1367847325-21463-7-git-send-email-dirk.eibach@gdsys.cc>
	<20130506141000.81015380E1C@gemini.denx.de>
	<e8fccb4d422d619744521268691e5a9b@gdsys.cc>
	<20130506152259.E8260380E6C@gemini.denx.de>
	<449c45a6a9978c55e84d3fe7efe6f0ac@gdsys.cc>
	<20130506192356.7929A380E1C@gemini.denx.de>
	<cf2913653766edc89705f49e08758df7@gdsys.cc>
	<20130507113609.7D3F038119A@gemini.denx.de>
	<ba0b32bf657f5f3ffb6056cc6bd0452e@gdsys.cc>
	<20130507191803.7F57C380E1B@gemini.denx.de>
	<58ff5023adcbf08481bc2707b0a70f50@gdsys.cc>
	<20130508101333.6DDB438119E@gemini.denx.de>
Message-ID: <0e4604dcfff89a1bd2b7f144a5b3ceae@gdsys.cc>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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