On August 27, 2017 5:36:58 AM PDT, Pavel Machek <pa...@denx.de> wrote:
>Hi!
>
>So I fought with the driver a bit more, and now I have something that
>kind-of-works.
>
>"great great hack" belows worries me.
>
>Yeah, disabled code needs to be removed before merge.
>
>No, tag_ksz part probably is not acceptable. Do you see solution
>better than just copying it into tag_ksz1 file?

You could have all Micrel tag implementations live under net/dsa/tag_ksz.c and 
have e.g: DSA_TAG_PROTO_KSZ for the current (newer) switches and 
DSA_TAG_PROTO_KSZ_LEGACY (or any other name) for the older switches and you 
would provide two sets of function pointers depending on which protocol is 
requested by the switch.

Considering the minor difference needed in tagging here, it might be acceptable 
to actually keep the current functions and just have the xmit() call check what 
get_tag_protocol returns and use word 1 or 0 based on that. Even though that's 
a fast path it shouldn't hurt performance too much. If it does, we can always 
copy the tagging protocol into dsa_slave_priv so you have a fast access to it.

>
>Any more comments, etc?

The MII emulation bits are interesting, was it not sufficient if you 
implemented phy_read and phy_write operations that perform the necessary 
internal PHY accesses or maybe you don't get access to standard MII registers? 
b53 does such a thing and we merely just need to do a simple shift to access 
the MII register number, thus avoiding the translation.

>
>Help would be welcome.

I concur with Andrew, try to get a patch series, even an RFC one together so we 
can review things individually. 

How functional is your driver so far? I'd say the basic stuff to get working: 
counters (debugging), link management (auto-negotiation, forced, etc.) and 
basic bridging: all ports separate by default and working port to port 
switching when brought together in a bridge. VLAN, FDB, MDB, other ethtool 
goodies can be added later on.

-- 
Florian

Reply via email to