RE: [PATCH] net: ethernet: renesas: sh_eth: do not access POST registers if not exist

From: Chris Brandt <Chris.Brandt@renesas.com>
To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
	"David S . Miller" <davem@davemloft.net>
Cc: Simon Horman <horms+renesas@verge.net.au>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Daniel Palmer <daniel@0x0f.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-renesas-soc@vger.kernel.org"
	<linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH] net: ethernet: renesas: sh_eth: do not access POST registers if not exist
Date: Mon, 29 Aug 2016 14:41:18 +0000	[thread overview]
Message-ID: <SG2PR06MB11653F9E20F38972CA11EF328AE10@SG2PR06MB1165.apcprd06.prod.outlook.com> (raw)
In-Reply-To: <87690577-4526-ce66-6ce3-68ac8b20e737@cogentembedded.com>

On 08/28/2016, Sergei Shtylyov wrote:

>> The RZ/A1 has a TSU, but since it only has one Ethernet port, it does 
>> not have POST registers.
>
>    I'm not sure the reason is having one port... do you have the old SH manuals somewhere? :-)

Yes, I used to support the SH7757. It had dual ETHER and dual GETHER (but only the GETHER had a TSU).

Since the GETHER had 2 ports, and the same TSU is used for both, there is an option where as you put in CAM entries for multicast frame and such, you can select if you would like CAM entry packets received on one port to be automatically relayed over to the other port for processing (ie, bridge network).
The POST1-POST4 registers are what you use to select what CAM entries you want relayed.

For example: 

   Register: CAM Entry Table POST1 Register (TSU_POST1)
       Bits: 31 to 28
   Bit name: POST0[3:0]
Description: These bits set the conditions for referring to CAM entry table 0. By
             setting multiple bits to 1, multiple conditions can be selected.

             POST0[3]: CAM entry table 0 is referred to in port 0 reception
             POST0[2]: CAM entry table 0 is referred to in port 0 to 1 relay.
             POST0[1]: CAM entry table 0 is referred to in port 1 reception.
             POST0[0]: CAM entry table 0 is referred to in port 1 to 0 relay

So, as you can see, having the POST registers means nothing if you only have 1 Ethernet port attached to the TSU.

Note, if you look at function sh_eth_tsu_get_post_bit, the relay functionality is never used anyway because each entry will only ever be set to "port 0 reception" or "port 1 reception".

>>  	.shift_rd0	= 1,
>>  };
>>
>> @@ -2460,6 +2461,9 @@ static void sh_eth_tsu_enable_cam_entry_post(struct net_device *ndev,
>>  	u32 tmp;
>>  	void *reg_offset;
>>
>> +	if (mdp->cd->tsu_no_post)
>> +		return;
>> +
>>  	reg_offset = sh_eth_tsu_get_post_reg_offset(mdp, entry);
>
>    I'd check check for SH_ETH_OFFSET_INVALID in the above function and return NULL if so; then
> we can check for NULL here...

That was similar to my first fix. But, then I got to thinking that if a new RZ comes out with dual Ethernet, the designers might add in the POST registers back in the TSU. And in that case, it would be nice to reuse the sh_eth_is_rz_fast_ether array (just add in the extra registers).

But...maybe checking for SH_ETH_OFFSET_INVALID is good for now instead of worrying about that.

> Oh, and I'll have to correct your language and terminology. :-/ Should be
> "if they don't exist" in the subject.

Ya, my first subject line was too long, so I kept trying to shorten it....and then it turn into bad grammer.

I think a more clear subject is "Fix TSU without relay feature accesses"

Chris