DSA support for Marvell 88e6065 switch

DSA support for Marvell 88e6065 switch

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 472 bytes --]

Hi!

I'm trying to create support for Marvell 88e6065 switch... and it
seems like drivers/net/dsa supports everything, but this model.

Did someone work with this hardware before? Any idea if it would be
more suitable to support by existing 88e6060 code, or if 88e6xxx code
should serve as a base?

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: DSA support for Marvell 88e6065 switch

[Andrew Lunn]

On Thu, Nov 15, 2018 at 08:51:11PM +0100, Pavel Machek wrote:
> Hi!
> 
> I'm trying to create support for Marvell 88e6065 switch... and it
> seems like drivers/net/dsa supports everything, but this model.
> 
> Did someone work with this hardware before? Any idea if it would be
> more suitable to support by existing 88e6060 code, or if 88e6xxx code
> should serve as a base?

Hi Pavel

The 88e6xxx should be extended to support this. I think you will find
a lot of the building blocks are already in the driver. Compare the
various implementations of the functions in the mv88e6xxx_ops to what
the datasheet says for the registers, and pick those that match.

    Andrew

Re: DSA support for Marvell 88e6065 switch

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]

On Thu 2018-11-15 21:26:18, Andrew Lunn wrote:
> On Thu, Nov 15, 2018 at 08:51:11PM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > I'm trying to create support for Marvell 88e6065 switch... and it
> > seems like drivers/net/dsa supports everything, but this model.
> > 
> > Did someone work with this hardware before? Any idea if it would be
> > more suitable to support by existing 88e6060 code, or if 88e6xxx code
> > should serve as a base?
> 
> Hi Pavel
> 
> The 88e6xxx should be extended to support this. I think you will find
> a lot of the building blocks are already in the driver. Compare the
> various implementations of the functions in the mv88e6xxx_ops to what
> the datasheet says for the registers, and pick those that match.

Ok, so I played a bit.

It looks like e6065 has different register layout from those supported
by 6xxx, and is quite similar to e6060.

I understand how 88e6xxx code is supposed to work... but I don't
understand how e6060 code is supposed to be probed. It does not seem
to have device tree support. It seems to be older code, but is way
simpler, and seems to be targetted at similar hardware.

Best regards,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: DSA support for Marvell 88e6065 switch

[Andrew Lunn]

On Sun, Nov 18, 2018 at 07:07:12PM +0100, Pavel Machek wrote:
> On Thu 2018-11-15 21:26:18, Andrew Lunn wrote:
> > On Thu, Nov 15, 2018 at 08:51:11PM +0100, Pavel Machek wrote:
> > > Hi!
> > > 
> > > I'm trying to create support for Marvell 88e6065 switch... and it
> > > seems like drivers/net/dsa supports everything, but this model.
> > > 
> > > Did someone work with this hardware before? Any idea if it would be
> > > more suitable to support by existing 88e6060 code, or if 88e6xxx code
> > > should serve as a base?
> > 
> > Hi Pavel
> > 
> > The 88e6xxx should be extended to support this. I think you will find
> > a lot of the building blocks are already in the driver. Compare the
> > various implementations of the functions in the mv88e6xxx_ops to what
> > the datasheet says for the registers, and pick those that match.
> 
> Ok, so I played a bit.
> 
> It looks like e6065 has different register layout from those supported
> by 6xxx, and is quite similar to e6060.

Hi Pavel

However, if you look in the mv88e6xxx, there are quite a few functions
called mv88e6065_foo. Marvell keeps changing the register layout. When
writing code, we try to name the functions based on which family of
devices introduced those registers. But we don't have the whole
history, so we probably have some names wrong.

> I understand how 88e6xxx code is supposed to work... but I don't
> understand how e6060 code is supposed to be probed. It does not seem
> to have device tree support. It seems to be older code, but is way
> simpler, and seems to be targetted at similar hardware.

The e6060 code is really old, pretty much abandoned. To make it
usable, you are going to have to make a lot of changes. Turn it into
an mdio driver, add device tree, make it use the new dsa2.c API, etc.

I still think you should be looking at the mv88e6xxx driver, but maybe
taking bits of code from the 6060 driver and adding it to the
mv88e6xxx driver as needed.

	  Andrew

Re: DSA support for Marvell 88e6065 switch

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2691 bytes --]

HI!

> > > > I'm trying to create support for Marvell 88e6065 switch... and it
> > > > seems like drivers/net/dsa supports everything, but this model.
> > > > 
> > > > Did someone work with this hardware before? Any idea if it would be
> > > > more suitable to support by existing 88e6060 code, or if 88e6xxx code
> > > > should serve as a base?
> > > 
> > > Hi Pavel
> > > 
> > > The 88e6xxx should be extended to support this. I think you will find
> > > a lot of the building blocks are already in the driver. Compare the
> > > various implementations of the functions in the mv88e6xxx_ops to what
> > > the datasheet says for the registers, and pick those that match.
> > 
> > Ok, so I played a bit.
> > 
> > It looks like e6065 has different register layout from those supported
> > by 6xxx, and is quite similar to e6060.
> 
> Hi Pavel
> 
> However, if you look in the mv88e6xxx, there are quite a few functions
> called mv88e6065_foo. Marvell keeps changing the register layout. When
> writing code, we try to name the functions based on which family of
> devices introduced those registers. But we don't have the whole
> history, so we probably have some names wrong.

Let me check... I thought there was only one such function. Ok, I see
two such functions:

mv88e6065_phylink_validate
mv88e6065_port_set_speed

I may need to re-check, but it looked to me like even functions and
registeres labeled 6xxx are different on 6060 and 6065... which means
changes to the code would not be exactly nice and easy.

> > I understand how 88e6xxx code is supposed to work... but I don't
> > understand how e6060 code is supposed to be probed. It does not seem
> > to have device tree support. It seems to be older code, but is way
> > simpler, and seems to be targetted at similar hardware.
> 
> The e6060 code is really old, pretty much abandoned. To make it
> usable, you are going to have to make a lot of changes. Turn it into
> an mdio driver, add device tree, make it use the new dsa2.c API, etc.

If I wanted it to work, what do I need to do? AFAICT phy autoprobing
should just attach it as soon as it is compiled in?

I tried adding nodes in dts trying to make the driver attach, but no
luck so far.

> I still think you should be looking at the mv88e6xxx driver, but maybe
> taking bits of code from the 6060 driver and adding it to the
> mv88e6xxx driver as needed.

e6060 driver is much simpler, which is somehow nice. Let me look
around some more.

Thanks,
								Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: DSA support for Marvell 88e6065 switch

[Andrew Lunn]

> If I wanted it to work, what do I need to do? AFAICT phy autoprobing
> should just attach it as soon as it is compiled in?

Nope. It is a switch, not a PHY. Switches are never auto-probed
because they are not guaranteed to have ID registers.

You need to use the legacy device tree binding. Look in
Documentation/devicetree/bindings/net/dsa/dsa.txt, section Deprecated
Binding. You can get more examples if you checkout old kernels. Or
kirkwood-rd88f6281.dtsi, the dsa { } node which is disabled.

	Andrew

Re: DSA support for Marvell 88e6065 switch

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1626 bytes --]

On Mon 2018-11-19 04:25:35, Andrew Lunn wrote:
> > If I wanted it to work, what do I need to do? AFAICT phy autoprobing
> > should just attach it as soon as it is compiled in?
> 
> Nope. It is a switch, not a PHY. Switches are never auto-probed
> because they are not guaranteed to have ID registers.
> 
> You need to use the legacy device tree binding. Look in
> Documentation/devicetree/bindings/net/dsa/dsa.txt, section Deprecated
> Binding. You can get more examples if you checkout old kernels. Or
> kirkwood-rd88f6281.dtsi, the dsa { } node which is disabled.

Thanks; I ported code from mv88e66xx in the meantime, and switch
appears to be detected.

But I'm running into problems with tagging code, and I guess I'd like
some help understanding.

tag_trailer: allocates new skb, then copies data around.

tag_qca: does dev->stats.tx_packets++, and reuses existing skb.

tag_brcm: reuses existing skb.

Is qca wrong in adjusting the statistics? Why does trailer allocate
new skb?

6065 seems to use 2-byte header between "SFD" and "Destination
address" in the ethernet frame. That's ... strange place to put
header, as addresses are now shifted. I need to put ethernet in
promisc mode (by running tcpdump) to get data moving.. and can not
figure out what to do in tag_...

Can I model it after tag_qca.c (simplified, because addresses will not
need to be memmoved) or is there some special consideration I am
missing?

Thanks,
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: DSA support for Marvell 88e6065 switch

[Lennert Buytenhek]

On Thu, Nov 22, 2018 at 02:21:23PM +0100, Pavel Machek wrote:

> > > If I wanted it to work, what do I need to do? AFAICT phy autoprobing
> > > should just attach it as soon as it is compiled in?
> > 
> > Nope. It is a switch, not a PHY. Switches are never auto-probed
> > because they are not guaranteed to have ID registers.
> > 
> > You need to use the legacy device tree binding. Look in
> > Documentation/devicetree/bindings/net/dsa/dsa.txt, section Deprecated
> > Binding. You can get more examples if you checkout old kernels. Or
> > kirkwood-rd88f6281.dtsi, the dsa { } node which is disabled.
> 
> Thanks; I ported code from mv88e66xx in the meantime, and switch
> appears to be detected.
> 
> But I'm running into problems with tagging code, and I guess I'd like
> some help understanding.
> 
> tag_trailer: allocates new skb, then copies data around.
> 
> tag_qca: does dev->stats.tx_packets++, and reuses existing skb.
> 
> tag_brcm: reuses existing skb.
> 
> Is qca wrong in adjusting the statistics? Why does trailer allocate
> new skb?
> 
> 6065 seems to use 2-byte header between "SFD" and "Destination
> address" in the ethernet frame. That's ... strange place to put
> header, as addresses are now shifted. I need to put ethernet in
> promisc mode (by running tcpdump) to get data moving.. and can not
> figure out what to do in tag_...

Does this switch chip not also support trailer mode?

There's basically four tagging modes for Marvell switch chips: header
mode (the one you described), trailer mode (tag_trailer.c), DSA and
ethertype DSA.  The switch chips I worked on that didn't support
(ethertype) DSA tagging did support both header and trailer modes,
and I chose to run them in trailer mode for the reasons you describe
above, but if your chip doesn't support trailer mode, then yes,
you'll have to add support for header mode and put the underlying
interface into promiscuous mode and such.

Re: DSA support for Marvell 88e6065 switch

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2709 bytes --]

Hi!

> > > > If I wanted it to work, what do I need to do? AFAICT phy autoprobing
> > > > should just attach it as soon as it is compiled in?
> > > 
> > > Nope. It is a switch, not a PHY. Switches are never auto-probed
> > > because they are not guaranteed to have ID registers.
> > > 
> > > You need to use the legacy device tree binding. Look in
> > > Documentation/devicetree/bindings/net/dsa/dsa.txt, section Deprecated
> > > Binding. You can get more examples if you checkout old kernels. Or
> > > kirkwood-rd88f6281.dtsi, the dsa { } node which is disabled.
> > 
> > Thanks; I ported code from mv88e66xx in the meantime, and switch
> > appears to be detected.
> > 
> > But I'm running into problems with tagging code, and I guess I'd like
> > some help understanding.
> > 
> > tag_trailer: allocates new skb, then copies data around.
> > 
> > tag_qca: does dev->stats.tx_packets++, and reuses existing skb.
> > 
> > tag_brcm: reuses existing skb.

Any idea why tag trailer allocates new skb, and what is going on with
dev->stats.tx_packets++?

> > Is qca wrong in adjusting the statistics? Why does trailer allocate
> > new skb?
> > 
> > 6065 seems to use 2-byte header between "SFD" and "Destination
> > address" in the ethernet frame. That's ... strange place to put
> > header, as addresses are now shifted. I need to put ethernet in
> > promisc mode (by running tcpdump) to get data moving.. and can not
> > figure out what to do in tag_...
> 
> Does this switch chip not also support trailer mode?
> 
> There's basically four tagging modes for Marvell switch chips: header
> mode (the one you described), trailer mode (tag_trailer.c), DSA and
> ethertype DSA.  The switch chips I worked on that didn't support
> (ethertype) DSA tagging did support both header and trailer modes,
> and I chose to run them in trailer mode for the reasons you describe
> above, but if your chip doesn't support trailer mode, then yes,
> you'll have to add support for header mode and put the underlying
> interface into promiscuous mode and such.

It seems that 6060 supports both header (probably, parts of docs are
redacted) and trailer mode... but I'm working with 6065. That does not
support trailer mode... or at least word "trailer" does not appear
anywhere in the documentation.

What chip were you working with? I may want to take a look on their
wording.

6065 indeed has some kind of "egress tagging mode" (with four
options), but I have trouble understanding what it really does.

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: DSA support for Marvell 88e6065 switch

[Lennert Buytenhek]

On Thu, Nov 22, 2018 at 09:27:24PM +0100, Pavel Machek wrote:

> Hi!

Hello!


> > > > > If I wanted it to work, what do I need to do? AFAICT phy autoprobing
> > > > > should just attach it as soon as it is compiled in?
> > > > 
> > > > Nope. It is a switch, not a PHY. Switches are never auto-probed
> > > > because they are not guaranteed to have ID registers.
> > > > 
> > > > You need to use the legacy device tree binding. Look in
> > > > Documentation/devicetree/bindings/net/dsa/dsa.txt, section Deprecated
> > > > Binding. You can get more examples if you checkout old kernels. Or
> > > > kirkwood-rd88f6281.dtsi, the dsa { } node which is disabled.
> > > 
> > > Thanks; I ported code from mv88e66xx in the meantime, and switch
> > > appears to be detected.
> > > 
> > > But I'm running into problems with tagging code, and I guess I'd like
> > > some help understanding.
> > > 
> > > tag_trailer: allocates new skb, then copies data around.
> > > 
> > > tag_qca: does dev->stats.tx_packets++, and reuses existing skb.
> > > 
> > > tag_brcm: reuses existing skb.
> 
> Any idea why tag trailer allocates new skb,

I wrote this code over 10 years ago, so I don't remember all that
well, but I think that it is because you have to do manual checksumming
of the packet, as there's no way to pass down the stack that you don't
want to checksum all the way down to the end of the data area (and you
don't want the tag to be included in the checksum), and so you want to
do that before you add the trailer tag, and you'll probably have to
reallocate the data area to be able to add the tag, and you probably
won't get an exclusive skb here anyway, so you might as well allocate
a new one.


> and what is going on with dev->stats.tx_packets++?

trailer_xmit would be the hard_start_xmit function for the virtual
(slave) network interface, so this would be the right thing to do?


> > > Is qca wrong in adjusting the statistics? Why does trailer allocate
> > > new skb?
> > > 
> > > 6065 seems to use 2-byte header between "SFD" and "Destination
> > > address" in the ethernet frame. That's ... strange place to put
> > > header, as addresses are now shifted. I need to put ethernet in
> > > promisc mode (by running tcpdump) to get data moving.. and can not
> > > figure out what to do in tag_...
> > 
> > Does this switch chip not also support trailer mode?
> > 
> > There's basically four tagging modes for Marvell switch chips: header
> > mode (the one you described), trailer mode (tag_trailer.c), DSA and
> > ethertype DSA.  The switch chips I worked on that didn't support
> > (ethertype) DSA tagging did support both header and trailer modes,
> > and I chose to run them in trailer mode for the reasons you describe
> > above, but if your chip doesn't support trailer mode, then yes,
> > you'll have to add support for header mode and put the underlying
> > interface into promiscuous mode and such.
> 
> It seems that 6060 supports both header (probably, parts of docs are
> redacted) and trailer mode... but I'm working with 6065. That does not
> support trailer mode... or at least word "trailer" does not appear
> anywhere in the documentation.
> 
> What chip were you working with? I may want to take a look on their
> wording.

I think I added trailer mode just for the 6060, since it doesn't (IIRC)
support (ethertype) DSA tagging.


> 6065 indeed has some kind of "egress tagging mode" (with four
> options), but I have trouble understanding what it really does.

What are the options?

Re: DSA support for Marvell 88e6065 switch

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]

Hi!

> > > > But I'm running into problems with tagging code, and I guess I'd like
> > > > some help understanding.
> > > > 
> > > > tag_trailer: allocates new skb, then copies data around.
> > > > 
> > > > tag_qca: does dev->stats.tx_packets++, and reuses existing skb.
> > > > 
> > > > tag_brcm: reuses existing skb.
> > 
> > Any idea why tag trailer allocates new skb,
> 
> I wrote this code over 10 years ago, so I don't remember all that
> well, but I think that it is because you have to do manual checksumming
> of the packet, as there's no way to pass down the stack that you don't
> want to checksum all the way down to the end of the data area (and you
> don't want the tag to be included in the checksum), and so you want to
> do that before you add the trailer tag, and you'll probably have to
> reallocate the data area to be able to add the tag, and you probably
> won't get an exclusive skb here anyway, so you might as well allocate
> a new one.

Ok, thanks. Whether tag is checksummed or not is indeed an interesting question.

> > and what is going on with dev->stats.tx_packets++?
> 
> trailer_xmit would be the hard_start_xmit function for the virtual
> (slave) network interface, so this would be the right thing to do?

Some tag_*.c do this and some do not, so I'm still confused.

> > 6065 indeed has some kind of "egress tagging mode" (with four
> > options), but I have trouble understanding what it really does.
> 
> What are the options?

"transmit unmodified", "transmit untagged", "transmit tagged" and
"transmit all frames double tagged".

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: DSA support for Marvell 88e6065 switch

[Florian Fainelli]

On 12/5/18 12:46 PM, Pavel Machek wrote:
> Hi!
> 
>>>>> But I'm running into problems with tagging code, and I guess I'd like
>>>>> some help understanding.
>>>>>
>>>>> tag_trailer: allocates new skb, then copies data around.
>>>>>
>>>>> tag_qca: does dev->stats.tx_packets++, and reuses existing skb.
>>>>>
>>>>> tag_brcm: reuses existing skb.
>>>
>>> Any idea why tag trailer allocates new skb,
>>
>> I wrote this code over 10 years ago, so I don't remember all that
>> well, but I think that it is because you have to do manual checksumming
>> of the packet, as there's no way to pass down the stack that you don't
>> want to checksum all the way down to the end of the data area (and you
>> don't want the tag to be included in the checksum), and so you want to
>> do that before you add the trailer tag, and you'll probably have to
>> reallocate the data area to be able to add the tag, and you probably
>> won't get an exclusive skb here anyway, so you might as well allocate
>> a new one.
> 
> Ok, thanks. Whether tag is checksummed or not is indeed an interesting question.
> 
>>> and what is going on with dev->stats.tx_packets++?
>>
>> trailer_xmit would be the hard_start_xmit function for the virtual
>> (slave) network interface, so this would be the right thing to do?
> 
> Some tag_*.c do this and some do not, so I'm still confused.

This is likely just historical, or we don't have an easy way to
determine whether the SKB transmitted succeeded or not because it was
reallocated, and therefore the tagging code must make sure statistics
are maintained. I suspect it is just historical and that tag module just
has not been updated to leverage the statistics accounting done a layer
above it within DSA.

> 
>>> 6065 indeed has some kind of "egress tagging mode" (with four
>>> options), but I have trouble understanding what it really does.
>>
>> What are the options?
> 
> "transmit unmodified", "transmit untagged", "transmit tagged" and
> "transmit all frames double tagged".

It seems to me that this applies to VLAN headers, not DSA/trailer
headers though not having the data sheet I should be flat out wrong.
-- 
Florian

Well supported DSA switches, support for Marvell 88e6065 switch

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

Hi!

> I'm trying to create support for Marvell 88e6065 switch... and it
> seems like drivers/net/dsa supports everything, but this model.

I have something that works, but is far from suitable for
mainline. Few parts are good, those will be marked PATCH, rest will be
RFD. I'm not sure how much work I'll be able to put into them in
future, so here they go.

Is there well supported switch, preferably with public documentation?
Does not have to be Marvell... Three ports could be enough, and
100MBit is enough for our use. We need RSTP, and are using two VLANs... 

AFAICT 88e6176 should be quite safe choice (as it is used by Turris
Omnia), but it has more ports than we need, and supports gigabit --
that means expensive. It also lacks public documentation AFAICT :-(.

Patches are relavite to v4.19.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[PATCH] mv88e6060: Warn about errors

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

    
Errors communicating with the chip are not expected, warn about them.
    
Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index 65f10fec25b3..f43104f48dbb 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -30,8 +30,10 @@ static int reg_read(struct dsa_switch *ds, int addr, int reg)
 		int __ret;					\
 								\
 		__ret = reg_read(ds, addr, reg);		\
-		if (__ret < 0)					\
+		if (__ret < 0) {				\
+			printk("mv88e6060: error reading %x/%x\n", addr, reg);	\
 			return __ret;				\
+		}						\
 		__ret;						\
 	})
 
@@ -48,8 +50,10 @@ static int reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
 		int __ret;					\
 								\
 		__ret = reg_write(ds, addr, reg, val);		\
-		if (__ret < 0)					\
+		if (__ret < 0)  {                               \
+			printk("mv88e6060: error writing %x/%x\n", addr, reg);	\
 			return __ret;				\
+		}						\
 	})
 
 static const char *mv88e6060_get_name(struct mii_bus *bus, int sw_addr)



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[RFD] mv88e6060: Allow the driver to be probed from device tree

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 4401 bytes --]


mv88e6060: Allow the driver to be probed from device tree
    
This is NOT ready for merge. Hardcoding an address is a bad idea, and obviously
it would be good to allow module removal, too.
    
Signed-off-by: Pavel Machek <pavel@ucw.cz> (not ready)

diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index f43104f48dbb..9e3901f18518 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -77,26 +77,40 @@ static const char *mv88e6060_get_name(struct mii_bus *bus, int sw_addr)
 static enum dsa_tag_protocol mv88e6060_get_tag_protocol(struct dsa_switch *ds,
 							int port)
 {
+  //return DSA_TAG_PROTO_QCA;
+  //return DSA_TAG_PROTO_TRAILER;
 	return DSA_TAG_PROTO_TRAILER;
 }
 
+static struct mv88e6060_priv *
+alloc_priv(struct device *dsa_dev, struct mii_bus *bus, int sw_addr)
+{
+  	struct mv88e6060_priv *priv;
+
+	priv = devm_kzalloc(dsa_dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return NULL;
+
+	priv->bus = bus;
+	priv->sw_addr = sw_addr;
+	return priv;
+}
+
 static const char *mv88e6060_drv_probe(struct device *dsa_dev,
 				       struct device *host_dev, int sw_addr,
 				       void **_priv)
 {
 	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
-	struct mv88e6060_priv *priv;
 	const char *name;
 
 	name = mv88e6060_get_name(bus, sw_addr);
-	if (name) {
-		priv = devm_kzalloc(dsa_dev, sizeof(*priv), GFP_KERNEL);
-		if (!priv)
-			return NULL;
-		*_priv = priv;
-		priv->bus = bus;
-		priv->sw_addr = sw_addr;
-	}
+	
+	if (!name)
+		return NULL;
+
+	*_priv = alloc_priv(dsa_dev, bus, sw_addr);
+	if (!*_priv)
+		  return NULL;
 
 	return name;
 }
@@ -132,9 +146,12 @@ static int mv88e6060_switch_reset(struct dsa_switch *ds)
 
 		usleep_range(1000, 2000);
 	}
-	if (time_after(jiffies, timeout))
+	if (time_after(jiffies, timeout)) {
+		printk("e6060: reset timed out!\n");
 		return -ETIMEDOUT;
+	}
 
+	printk("e6060: reset ok\n");	
 	return 0;
 }
 
@@ -284,15 +301,94 @@ static struct dsa_switch_driver mv88e6060_switch_drv = {
 	.ops		= &mv88e6060_switch_ops,
 };
 
+static int mv88e6060_probe(struct mdio_device *mdiodev)
+{
+	struct device *dev = &mdiodev->dev;
+	struct device_node *np = dev->of_node;
+	const struct mv88e6060_info *compat_info;
+	struct mv88e6060_priv *chip;
+	u32 eeprom_len;
+	int err;
+
+	int addr = 0x10 /* mdiodev->addr */ ;
+
+	chip = alloc_priv(dev, mdiodev->bus, addr);
+	if (!chip)
+		return -ENOMEM;
+
+	{
+		char *name = mv88e6060_get_name(mdiodev->bus, addr);
+		printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
+	}
+	{
+	  	struct dsa_switch *ds;
+
+		ds = dsa_switch_alloc(dev, 6);
+		if (!ds)
+			return -ENOMEM;
+
+		ds->priv = chip;
+		ds->dev = dev;
+		ds->ops = &mv88e6060_switch_ops;
+		ds->ageing_time_min = 15000;
+		ds->ageing_time_max = 15000 * U8_MAX;
+
+		dev_set_drvdata(dev, ds);
+
+		return dsa_register_switch(ds);
+	}
+
+	printk("e6060: probe ok\n");
+	return 0;
+}
+
+static void mv88e6060_remove(struct mdio_device *mdiodev)
+{
+	struct dsa_switch *ds = dev_get_drvdata(&mdiodev->dev);
+	struct mv88e6060_chip *chip = ds->priv;
+
+	printk("e6060: remove.\n");
+	BUG();
+#if 0
+	mv88e6060_phy_destroy(chip);
+	mv88e6060_unregister_switch(chip);
+	mv88e6060_mdio_unregister(chip);
+#endif
+}
+
+static const struct of_device_id mv88e6060_of_match[] = {
+	{
+		.compatible = "marvell,mv88e6060",
+	},
+	{ /* sentinel */ },
+};
+
+MODULE_DEVICE_TABLE(of, mv88e6060_of_match);
+
+static struct mdio_driver mv88e6060_driver = {
+	.probe	= mv88e6060_probe,
+	.remove = mv88e6060_remove,
+	.mdiodrv.driver = {
+		.name = "mv88e6060",
+		.of_match_table = mv88e6060_of_match,
+	},
+};
+
 static int __init mv88e6060_init(void)
 {
+	int r;
+	printk("e6060: init\n");
 	register_switch_driver(&mv88e6060_switch_drv);
-	return 0;
+	r = mdio_driver_register(&mv88e6060_driver);
+	printk("e6060: init %d\n", r);
+	return r;
 }
+
 module_init(mv88e6060_init);
 
 static void __exit mv88e6060_cleanup(void)
 {
+  	mdio_driver_unregister(&mv88e6060_driver);
 	unregister_switch_driver(&mv88e6060_switch_drv);
 }
 module_exit(mv88e6060_cleanup);


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

mv88e6060: Turn e6060 driver into e6065 driver

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 4597 bytes --]

    
This is NOT ready for merge, as you lose support for 6060. Probably
not what you want.
    
Signed-off-by: Pavel Machek <pavel@ucw.cz> (not ready)

diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index 9e3901f18518..9c9b4792bbad 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -61,6 +61,7 @@ static const char *mv88e6060_get_name(struct mii_bus *bus, int sw_addr)
 	int ret;
 
 	ret = mdiobus_read(bus, sw_addr + REG_PORT(0), PORT_SWITCH_ID);
+	printk("e6060: get name: %lx\n", (long) ret);
 	if (ret >= 0) {
 		if (ret == PORT_SWITCH_ID_6060)
 			return "Marvell 88E6060 (A0)";
@@ -69,6 +70,8 @@ static const char *mv88e6060_get_name(struct mii_bus *bus, int sw_addr)
 			return "Marvell 88E6060 (B0)";
 		if ((ret & PORT_SWITCH_ID_6060_MASK) == PORT_SWITCH_ID_6060)
 			return "Marvell 88E6060";
+		if ((ret & PORT_SWITCH_ID_6060_MASK) == 0x0650)
+			return "Marvell 88E6065";
 	}
 
 	return NULL;
@@ -79,7 +82,7 @@ static enum dsa_tag_protocol mv88e6060_get_tag_protocol(struct dsa_switch *ds,
 {
   //return DSA_TAG_PROTO_QCA;
   //return DSA_TAG_PROTO_TRAILER;
-	return DSA_TAG_PROTO_TRAILER;
+	return DSA_TAG_PROTO_DADDR;
 }
 
 static struct mv88e6060_priv *
@@ -128,6 +131,7 @@ static int mv88e6060_switch_reset(struct dsa_switch *ds)
 			  ret & ~PORT_CONTROL_STATE_MASK);
 	}
 
+
 	/* Wait for transmit queues to drain. */
 	usleep_range(2000, 4000);
 
@@ -174,6 +178,11 @@ static int mv88e6060_setup_global(struct dsa_switch *ds)
 	return 0;
 }
 
+static int mv88e6060_setup_port_hacks(struct dsa_switch *ds, int p)
+{
+}
+
+
 static int mv88e6060_setup_port(struct dsa_switch *ds, int p)
 {
 	int addr = REG_PORT(p);
@@ -184,12 +193,17 @@ static int mv88e6060_setup_port(struct dsa_switch *ds, int p)
 	 * port, enable Ingress and Egress Trailer tagging mode.
 	 */
 	REG_WRITE(addr, PORT_CONTROL,
-		  dsa_is_cpu_port(ds, p) ?
-			PORT_CONTROL_TRAILER |
-			PORT_CONTROL_INGRESS_MODE |
-			PORT_CONTROL_STATE_FORWARDING :
-			PORT_CONTROL_STATE_FORWARDING);
-
+		  PORT_CONTROL_INITIAL_PRI_6065 |
+		  PORT_CONTROL_FLOOD_ALL_6065 |
+		  PORT_CONTROL_STATE_FORWARDING |
+		  (dsa_is_cpu_port(ds, p) ?
+		   PORT_CONTROL_HEADER |
+		   PORT_CONTROL_TRAILER_6065
+		   : 0));
+
+	if (dsa_is_cpu_port(ds, p))
+		mv88e6060_setup_port_hacks(ds, p);
+	
 	/* Port based VLAN map: give each port its own address
 	 * database, allow the CPU port to talk to each of the 'real'
 	 * ports, and allow each of the 'real' ports to only talk to
diff --git a/drivers/net/dsa/mv88e6060.h b/drivers/net/dsa/mv88e6060.h
index 10249bd16292..13f7e8a2107a 100644
--- a/drivers/net/dsa/mv88e6060.h
+++ b/drivers/net/dsa/mv88e6060.h
@@ -32,27 +32,30 @@
 #define PORT_SWITCH_ID_6060_MASK	0xfff0
 #define PORT_SWITCH_ID_6060_R1	0x0601
 #define PORT_SWITCH_ID_6060_R2	0x0602
-#define PORT_CONTROL		0x04
+#define PORT_CONTROL		0x04 /* Matches */
 #define PORT_CONTROL_FORCE_FLOW_CTRL	BIT(15)
-#define PORT_CONTROL_TRAILER	BIT(14)
-#define PORT_CONTROL_HEADER	BIT(11)
-#define PORT_CONTROL_INGRESS_MODE	BIT(8)
+#define PORT_CONTROL_TRAILER_6060	BIT(14)
+#define PORT_CONTROL_TRAILER_6065	(BIT(12) /* |  BIT(13) */)
+#define PORT_CONTROL_HEADER	BIT(11) /* Controls ingress/egress on 6065, 2 byte headers? */
+#define PORT_CONTROL_INGRESS_MODE_6060	BIT(8)
 #define PORT_CONTROL_VLAN_TUNNEL	BIT(7)
-#define PORT_CONTROL_STATE_MASK	0x03
+#define PORT_CONTROL_INITIAL_PRI_6065   BIT(4)
+#define PORT_CONTROL_FLOOD_ALL_6065     (BIT(2) | BIT(3))
+#define PORT_CONTROL_STATE_MASK	0x03 /* Matches */
 #define PORT_CONTROL_STATE_DISABLED	0x00
 #define PORT_CONTROL_STATE_BLOCKING	0x01
 #define PORT_CONTROL_STATE_LEARNING	0x02
 #define PORT_CONTROL_STATE_FORWARDING	0x03
-#define PORT_VLAN_MAP		0x06
+#define PORT_VLAN_MAP		0x06 /* Matches */
 #define PORT_VLAN_MAP_DBNUM_SHIFT	12
 #define PORT_VLAN_MAP_TABLE_MASK	0x1f
-#define PORT_ASSOC_VECTOR	0x0b
+#define PORT_ASSOC_VECTOR	0x0b /* Matches */
 #define PORT_ASSOC_VECTOR_MONITOR	BIT(15)
 #define PORT_ASSOC_VECTOR_PAV_MASK	0x1f
-#define PORT_RX_CNTR		0x10
-#define PORT_TX_CNTR		0x11
+#define PORT_RX_CNTR		0x10 /* Matches */
+#define PORT_TX_CNTR		0x11 /* Matches */
 
-#define REG_GLOBAL		0x0f
+#define REG_GLOBAL		0x0f 
 #define GLOBAL_STATUS		0x00
 #define GLOBAL_STATUS_SW_MODE_MASK	(0x3 << 12)
 #define GLOBAL_STATUS_SW_MODE_0	(0x0 << 12)

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[PATCH] dsa device tree bindings: fix typo and wrong example

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]

    
Fix typo and fix compatible value that is not actually permitted by the
description in the example.
   
Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
index feb007af13cb..abf1be036ac5 100644
--- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
+++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
@@ -41,7 +41,7 @@ Optional properties:
 - mdio			: Container of PHY and devices on the switches MDIO
 			  bus.
 - mdio?		: Container of PHYs and devices on the external MDIO
-			  bus. The node must contains a compatible string of
+			  bus. The node must contain a compatible string of
 			  "marvell,mv88e6xxx-mdio-external"
 
 Example:
@@ -80,7 +80,7 @@ Example:
 		#interrupt-cells = <2>;
 
 		switch0: switch@0 {
-			compatible = "marvell,mv88e6390";
+			compatible = "marvell,mv88e6190";
 			reg = <0>;
 			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
 


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[RFC] tag_daddr: add tagging scheme used by Marvel 88e6065 switch

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 5401 bytes --]

Tagging scheme used by 88e6065 is "interesting" as it moves around
ethernet addreses and forces us to use PROMISC mode on the ethernets.

I'm not sure how to call it, so I selected tag_daddr ("tag where
destination address should be").

Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 461e8a7661b7..187da100d468 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -40,6 +40,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_MTK,
 	DSA_TAG_PROTO_QCA,
 	DSA_TAG_PROTO_TRAILER,
+	DSA_TAG_PROTO_DADDR,
 	DSA_TAG_LAST,		/* MUST BE LAST */
 };
 
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 4183e4ba27a5..1c177b6c5127 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -32,6 +32,9 @@ config NET_DSA_TAG_BRCM
 config NET_DSA_TAG_BRCM_PREPEND
 	bool
 
+config NET_DSA_TAG_DADDR
+	bool "Support for DADDR tagging, found on Marvell 88e6065" 
+
 config NET_DSA_TAG_DSA
 	bool
 
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index 9e4d3536f977..4466c1b860cc 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -7,6 +7,7 @@ dsa_core-$(CONFIG_NET_DSA_LEGACY) += legacy.o
 # tagging formats
 dsa_core-$(CONFIG_NET_DSA_TAG_BRCM) += tag_brcm.o
 dsa_core-$(CONFIG_NET_DSA_TAG_BRCM_PREPEND) += tag_brcm.o
+dsa_core-$(CONFIG_NET_DSA_TAG_DADDR) += tag_daddr.o
 dsa_core-$(CONFIG_NET_DSA_TAG_DSA) += tag_dsa.o
 dsa_core-$(CONFIG_NET_DSA_TAG_EDSA) += tag_edsa.o
 dsa_core-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 9f3209ff7ffd..97d289b0ef9d 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -46,6 +46,9 @@ const struct dsa_device_ops *dsa_device_ops[DSA_TAG_LAST] = {
 #ifdef CONFIG_NET_DSA_TAG_BRCM_PREPEND
 	[DSA_TAG_PROTO_BRCM_PREPEND] = &brcm_prepend_netdev_ops,
 #endif
+#ifdef CONFIG_NET_DSA_TAG_DADDR
+	[DSA_TAG_PROTO_DADDR] = &daddr_netdev_ops,
+#endif
 #ifdef CONFIG_NET_DSA_TAG_DSA
 	[DSA_TAG_PROTO_DSA] = &dsa_netdev_ops,
 #endif
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 3964c6f7a7c0..0530b125ef72 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -199,6 +199,9 @@ void dsa_switch_unregister_notifier(struct dsa_switch *ds);
 extern const struct dsa_device_ops brcm_netdev_ops;
 extern const struct dsa_device_ops brcm_prepend_netdev_ops;
 
+/* tag_daddr.c */
+extern const struct dsa_device_ops daddr_netdev_ops;
+
 /* tag_dsa.c */
 extern const struct dsa_device_ops dsa_netdev_ops;
 
diff --git a/net/dsa/tag_daddr.c b/net/dsa/tag_daddr.c
new file mode 100644
index 000000000000..8e1e4de4df81
--- /dev/null
+++ b/net/dsa/tag_daddr.c
@@ -0,0 +1,100 @@
+/*
+ * net/dsa/tag_daddr.c - Daddr tag format handling
+ * Copyright (c) 2018 Pavel Machek <pavel@denx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/etherdevice.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+
+#include "dsa_priv.h"
+
+static struct sk_buff *daddr_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct sk_buff *nskb;
+	int padlen;
+	u8 *daddr;
+
+	padlen = 0;
+	if (skb->len < 60)
+		padlen = 60 - skb->len;
+
+	nskb = alloc_skb(NET_IP_ALIGN + skb->len + padlen + 2, GFP_ATOMIC);
+	if (!nskb)
+		return NULL;
+	skb_reserve(nskb, NET_IP_ALIGN);
+
+	skb_reset_mac_header(nskb);
+	skb_set_network_header(nskb, skb_network_header(skb) - skb->head);
+	skb_set_transport_header(nskb, skb_transport_header(skb) - skb->head);
+	skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));
+	consume_skb(skb);
+
+	if (padlen) {
+		skb_put_zero(nskb, padlen);
+	}
+
+	daddr = skb_push(nskb, 2);
+
+	nskb->data[0] = 0xff; /* dbnum << 4, 0x0e -- priority, 0x01 forcemap */
+	nskb->data[1] = 0xff; /* 0xc0 dbnum, 0x3f vlantable */
+
+	/* Hm. Seems same as PORT_VLAN_MAP value, with funny endian */
+	{
+#define PORT_VLAN_MAP_DBNUM_SHIFT	12
+	  
+		struct dsa_port *dp = dsa_slave_to_port(dev);
+		struct dsa_switch *ds = dp->ds;
+		int p = dp->index;
+
+		u16 vlan_map = ((p & 0xf) << PORT_VLAN_MAP_DBNUM_SHIFT) |
+		  		(dsa_is_cpu_port(ds, p) ? dsa_user_ports(ds) :
+			       BIT(p));
+
+		nskb->data[0] = vlan_map >> 8;
+		nskb->data[1] = vlan_map & 0xff;
+	}
+
+	return nskb;
+}
+
+#define DADDR_HDR_LEN 2
+
+static struct sk_buff *daddr_rcv(struct sk_buff *skb, struct net_device *dev,
+				   struct packet_type *pt)
+{
+	u8 ver;
+	int port;
+	u8 *phdr;
+
+	if (unlikely(!pskb_may_pull(skb, DADDR_HDR_LEN)))
+		return NULL;
+
+	/*
+	 * At this point, skb->data points to ethertype.
+	 */
+	phdr = (skb->data - 2 - 2 * ETH_ALEN);
+
+	/* Remove DADDR tag and recalculate checksum */
+	skb_pull_rcsum(skb, DADDR_HDR_LEN);
+
+	/* Get source port information */
+	port = phdr[1] & 0x07;
+
+	skb->dev = dsa_master_find_slave(dev, 0, port);
+	if (!skb->dev)
+		return NULL;
+
+	return skb;
+}
+
+const struct dsa_device_ops daddr_netdev_ops = {
+	.xmit	= daddr_xmit,
+	.rcv	= daddr_rcv,
+};

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] dsa device tree bindings: fix typo and wrong example

[Vokáč Michal]

On 6.12.2018 14:05, Pavel Machek wrote:
>      
> Fix typo and fix compatible value that is not actually permitted by the
> description in the example.

Ahoj Pavle, I think the subject should be more like:

	"dt-bindings: net: dsa: ..."

BR, Michal

>     
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> index feb007af13cb..abf1be036ac5 100644
> --- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> @@ -41,7 +41,7 @@ Optional properties:
>   - mdio			: Container of PHY and devices on the switches MDIO
>   			  bus.
>   - mdio?		: Container of PHYs and devices on the external MDIO
> -			  bus. The node must contains a compatible string of
> +			  bus. The node must contain a compatible string of
>   			  "marvell,mv88e6xxx-mdio-external"
>   
>   Example:
> @@ -80,7 +80,7 @@ Example:
>   		#interrupt-cells = <2>;
>   
>   		switch0: switch@0 {
> -			compatible = "marvell,mv88e6390";
> +			compatible = "marvell,mv88e6190";
>   			reg = <0>;
>   			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
>   
> 
> 

Re: [PATCH] dsa device tree bindings: fix typo and wrong example

[Vokáč Michal]

On 6.12.2018 14:05, Pavel Machek wrote:
>      
> Fix typo and fix compatible value that is not actually permitted by the
> description in the example.

Ahoj Pavle, I think the subject should be more like:

	"dt-bindings: net: dsa: ..."

BR, Michal

>     
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> index feb007af13cb..abf1be036ac5 100644
> --- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> @@ -41,7 +41,7 @@ Optional properties:
>   - mdio			: Container of PHY and devices on the switches MDIO
>   			  bus.
>   - mdio?		: Container of PHYs and devices on the external MDIO
> -			  bus. The node must contains a compatible string of
> +			  bus. The node must contain a compatible string of
>   			  "marvell,mv88e6xxx-mdio-external"
>   
>   Example:
> @@ -80,7 +80,7 @@ Example:
>   		#interrupt-cells = <2>;
>   
>   		switch0: switch@0 {
> -			compatible = "marvell,mv88e6390";
> +			compatible = "marvell,mv88e6190";
>   			reg = <0>;
>   			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
>   
> 
> 

Re: DSA support for Marvell 88e6065 switch

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 795 bytes --]

Hi!

> >>> 6065 indeed has some kind of "egress tagging mode" (with four
> >>> options), but I have trouble understanding what it really does.
> >>
> >> What are the options?
> > 
> > "transmit unmodified", "transmit untagged", "transmit tagged" and
> > "transmit all frames double tagged".
> 
> It seems to me that this applies to VLAN headers, not DSA/trailer
> headers though not having the data sheet I should be flat out wrong.

I guess you are right.

And while we probably could use VLAN headers instead of DSA header (in
double tagging mode or something), it would likely be complex and error-prone...

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [RFD] mv88e6060: Allow the driver to be probed from device tree

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5716 bytes --]

Hi Pavel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.20-rc5 next-20181206]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Pavel-Machek/mv88e6060-Allow-the-driver-to-be-probed-from-device-tree/20181207-013430
config: i386-randconfig-x007-201848 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/net/dsa/mv88e6060.c: In function 'mv88e6060_probe':
>> drivers/net/dsa/mv88e6060.c:316:16: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      char *name = mv88e6060_get_name(mdiodev->bus, addr);
                   ^~~~~~~~~~~~~~~~~~
>> drivers/net/dsa/mv88e6060.c:317:34: warning: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'struct mii_bus *' [-Wformat=]
      printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
                                   ~~^               ~~~~~~~~~~~~
>> drivers/net/dsa/mv88e6060.c:317:38: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'int' [-Wformat=]
      printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
                                       ~~^
                                       %x
   drivers/net/dsa/mv88e6060.c:307:6: warning: unused variable 'err' [-Wunused-variable]
     int err;
         ^~~
   drivers/net/dsa/mv88e6060.c:306:6: warning: unused variable 'eeprom_len' [-Wunused-variable]
     u32 eeprom_len;
         ^~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:304:31: warning: unused variable 'compat_info' [-Wunused-variable]
     const struct mv88e6060_info *compat_info;
                                  ^~~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:303:22: warning: unused variable 'np' [-Wunused-variable]
     struct device_node *np = dev->of_node;
                         ^~
   drivers/net/dsa/mv88e6060.c: In function 'mv88e6060_remove':
   drivers/net/dsa/mv88e6060.c:344:25: warning: unused variable 'chip' [-Wunused-variable]
     struct mv88e6060_chip *chip = ds->priv;
                            ^~~~
   Cyclomatic Complexity 1 include/linux/device.h:devm_kzalloc
   Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata
   Cyclomatic Complexity 1 include/linux/device.h:dev_set_drvdata
   Cyclomatic Complexity 1 include/linux/etherdevice.h:eth_random_addr
   Cyclomatic Complexity 1 include/net/dsa.h:dsa_to_port
   Cyclomatic Complexity 1 include/net/dsa.h:dsa_is_cpu_port
   Cyclomatic Complexity 1 include/net/dsa.h:dsa_is_user_port
   Cyclomatic Complexity 3 include/net/dsa.h:dsa_user_ports
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:mv88e6060_get_tag_protocol
   Cyclomatic Complexity 2 drivers/net/dsa/mv88e6060.c:alloc_priv
   Cyclomatic Complexity 2 drivers/net/dsa/mv88e6060.c:mv88e6060_port_to_phy_addr
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:mv88e6060_init
   Cyclomatic Complexity 0 drivers/net/dsa/mv88e6060.c:mv88e6060_remove
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:reg_read
   Cyclomatic Complexity 2 drivers/net/dsa/mv88e6060.c:mv88e6060_phy_read
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:reg_write
   Cyclomatic Complexity 2 drivers/net/dsa/mv88e6060.c:mv88e6060_phy_write
   Cyclomatic Complexity 3 drivers/net/dsa/mv88e6060.c:mv88e6060_setup_global
   Cyclomatic Complexity 4 drivers/net/dsa/mv88e6060.c:mv88e6060_setup_addr
   Cyclomatic Complexity 6 drivers/net/dsa/mv88e6060.c:mv88e6060_setup_port
   Cyclomatic Complexity 13 drivers/net/dsa/mv88e6060.c:mv88e6060_switch_reset
   Cyclomatic Complexity 6 drivers/net/dsa/mv88e6060.c:mv88e6060_setup
   Cyclomatic Complexity 5 drivers/net/dsa/mv88e6060.c:mv88e6060_get_name
   Cyclomatic Complexity 3 drivers/net/dsa/mv88e6060.c:mv88e6060_probe
   Cyclomatic Complexity 3 drivers/net/dsa/mv88e6060.c:mv88e6060_drv_probe
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:mv88e6060_cleanup
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:_GLOBAL__sub_I_00100_0_mv88e6060.c
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:_GLOBAL__sub_D_00100_1_mv88e6060.c

vim +/const +316 drivers/net/dsa/mv88e6060.c

   299	
   300	static int mv88e6060_probe(struct mdio_device *mdiodev)
   301	{
   302		struct device *dev = &mdiodev->dev;
   303		struct device_node *np = dev->of_node;
   304		const struct mv88e6060_info *compat_info;
   305		struct mv88e6060_priv *chip;
   306		u32 eeprom_len;
   307		int err;
   308	
   309		int addr = 0x10 /* mdiodev->addr */ ;
   310	
   311		chip = alloc_priv(dev, mdiodev->bus, addr);
   312		if (!chip)
   313			return -ENOMEM;
   314	
   315		{
 > 316			char *name = mv88e6060_get_name(mdiodev->bus, addr);
 > 317			printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
   318		}
   319		{
   320		  	struct dsa_switch *ds;
   321	
   322			ds = dsa_switch_alloc(dev, 6);
   323			if (!ds)
   324				return -ENOMEM;
   325	
   326			ds->priv = chip;
   327			ds->dev = dev;
   328			ds->ops = &mv88e6060_switch_ops;
   329			ds->ageing_time_min = 15000;
   330			ds->ageing_time_max = 15000 * U8_MAX;
   331	
   332			dev_set_drvdata(dev, ds);
   333	
   334			return dsa_register_switch(ds);
   335		}
   336	
   337		printk("e6060: probe ok\n");
   338		return 0;
   339	}
   340	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23961 bytes --]

Re: [PATCH] mv88e6060: Warn about errors

[David Miller]

Plain "printk" are never appropriate.

Please explicitly use pr_warn() or similar.  If there is a device context
available, either a generic device or a netdev, use one of the dev_*()
or netdev_*() variants.

Re: mv88e6060: Turn e6060 driver into e6065 driver

[David Miller]

From: Pavel Machek <pavel@ucw.cz>
Date: Thu, 6 Dec 2018 14:03:45 +0100

> @@ -79,7 +82,7 @@ static enum dsa_tag_protocol mv88e6060_get_tag_protocol(struct dsa_switch *ds,
>  {
>    //return DSA_TAG_PROTO_QCA;
>    //return DSA_TAG_PROTO_TRAILER;

These C++ style comments are not in any of my tree(s).

Your patch submission really needs to shape up if you want your patches
to be considered seriously.

Thank you.

Re: mv88e6060: Turn e6060 driver into e6065 driver

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

On Thu 2018-12-06 12:23:33, David Miller wrote:
> From: Pavel Machek <pavel@ucw.cz>
> Date: Thu, 6 Dec 2018 14:03:45 +0100
> 
> > @@ -79,7 +82,7 @@ static enum dsa_tag_protocol mv88e6060_get_tag_protocol(struct dsa_switch *ds,
> >  {
> >    //return DSA_TAG_PROTO_QCA;
> >    //return DSA_TAG_PROTO_TRAILER;
> 
> These C++ style comments are not in any of my tree(s).
> 
> Your patch submission really needs to shape up if you want your patches
> to be considered seriously.

This one should have been "RFD". It has way more serious problems then
this, as changelog tried to explain.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] mv88e6060: Warn about errors

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 552 bytes --]

On Thu 2018-12-06 12:21:59, David Miller wrote:
> 
> Plain "printk" are never appropriate.
> 
> Please explicitly use pr_warn() or similar.  If there is a device context
> available, either a generic device or a netdev, use one of the dev_*()
> or netdev_*() variants.

Can do, I guess.... is there's agreeement that such error is worth
some kind of output to the logs?

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [RFD] mv88e6060: Allow the driver to be probed from device tree

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3914 bytes --]

Hi Pavel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.20-rc6 next-20181214]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Pavel-Machek/mv88e6060-Allow-the-driver-to-be-probed-from-device-tree/20181207-013430
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

>> drivers/net/dsa/mv88e6060.c:316:48: warning: incorrect type in initializer (different modifiers)
   drivers/net/dsa/mv88e6060.c:316:48:    expected char *name
   drivers/net/dsa/mv88e6060.c:316:48:    got char const *
   drivers/net/dsa/mv88e6060.c: In function 'mv88e6060_probe':
   drivers/net/dsa/mv88e6060.c:316:16: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      char *name = mv88e6060_get_name(mdiodev->bus, addr);
                   ^~~~~~~~~~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:317:34: warning: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'struct mii_bus *' [-Wformat=]
      printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
                                   ~~^               ~~~~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:317:38: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'int' [-Wformat=]
      printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
                                       ~~^
                                       %x
   drivers/net/dsa/mv88e6060.c:307:6: warning: unused variable 'err' [-Wunused-variable]
     int err;
         ^~~
   drivers/net/dsa/mv88e6060.c:306:6: warning: unused variable 'eeprom_len' [-Wunused-variable]
     u32 eeprom_len;
         ^~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:304:31: warning: unused variable 'compat_info' [-Wunused-variable]
     const struct mv88e6060_info *compat_info;
                                  ^~~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:303:22: warning: unused variable 'np' [-Wunused-variable]
     struct device_node *np = dev->of_node;
                         ^~
   drivers/net/dsa/mv88e6060.c: In function 'mv88e6060_remove':
   drivers/net/dsa/mv88e6060.c:344:25: warning: unused variable 'chip' [-Wunused-variable]
     struct mv88e6060_chip *chip = ds->priv;
                            ^~~~

vim +316 drivers/net/dsa/mv88e6060.c

   299	
   300	static int mv88e6060_probe(struct mdio_device *mdiodev)
   301	{
   302		struct device *dev = &mdiodev->dev;
   303		struct device_node *np = dev->of_node;
   304		const struct mv88e6060_info *compat_info;
   305		struct mv88e6060_priv *chip;
   306		u32 eeprom_len;
   307		int err;
   308	
   309		int addr = 0x10 /* mdiodev->addr */ ;
   310	
   311		chip = alloc_priv(dev, mdiodev->bus, addr);
   312		if (!chip)
   313			return -ENOMEM;
   314	
   315		{
 > 316			char *name = mv88e6060_get_name(mdiodev->bus, addr);
   317			printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
   318		}
   319		{
   320		  	struct dsa_switch *ds;
   321	
   322			ds = dsa_switch_alloc(dev, 6);
   323			if (!ds)
   324				return -ENOMEM;
   325	
   326			ds->priv = chip;
   327			ds->dev = dev;
   328			ds->ops = &mv88e6060_switch_ops;
   329			ds->ageing_time_min = 15000;
   330			ds->ageing_time_max = 15000 * U8_MAX;
   331	
   332			dev_set_drvdata(dev, ds);
   333	
   334			return dsa_register_switch(ds);
   335		}
   336	
   337		printk("e6060: probe ok\n");
   338		return 0;
   339	}
   340	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66616 bytes --]

Re: [PATCH] dsa device tree bindings: fix typo and wrong example

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]

On Thu 2018-12-06 13:28:56, Vokáč Michal wrote:
> On 6.12.2018 14:05, Pavel Machek wrote:
> >      
> > Fix typo and fix compatible value that is not actually permitted by the
> > description in the example.
> 
> Ahoj Pavle, I think the subject should be more like:
> 
> 	"dt-bindings: net: dsa: ..."

Ahoj!

Well, retrying with different subject only makes sense if someone is
interested. Rob, Andrew -- can you take the patch?

Do you want me to edit the subject or can you just edit the subject
while applying?

Thanks,
								Pavel

> BR, Michal
> 
> >     
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> > index feb007af13cb..abf1be036ac5 100644
> > --- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
> > +++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> > @@ -41,7 +41,7 @@ Optional properties:
> >   - mdio			: Container of PHY and devices on the switches MDIO
> >   			  bus.
> >   - mdio?		: Container of PHYs and devices on the external MDIO
> > -			  bus. The node must contains a compatible string of
> > +			  bus. The node must contain a compatible string of
> >   			  "marvell,mv88e6xxx-mdio-external"
> >   
> >   Example:
> > @@ -80,7 +80,7 @@ Example:
> >   		#interrupt-cells = <2>;
> >   
> >   		switch0: switch@0 {
> > -			compatible = "marvell,mv88e6390";
> > +			compatible = "marvell,mv88e6190";
> >   			reg = <0>;
> >   			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
> >   
> > 
> > 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] dsa device tree bindings: fix typo and wrong example

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]

On Thu 2018-12-06 13:28:56, Vokáč Michal wrote:
> On 6.12.2018 14:05, Pavel Machek wrote:
> >      
> > Fix typo and fix compatible value that is not actually permitted by the
> > description in the example.
> 
> Ahoj Pavle, I think the subject should be more like:
> 
> 	"dt-bindings: net: dsa: ..."

Ahoj!

Well, retrying with different subject only makes sense if someone is
interested. Rob, Andrew -- can you take the patch?

Do you want me to edit the subject or can you just edit the subject
while applying?

Thanks,
								Pavel

> BR, Michal
> 
> >     
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> > index feb007af13cb..abf1be036ac5 100644
> > --- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
> > +++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> > @@ -41,7 +41,7 @@ Optional properties:
> >   - mdio			: Container of PHY and devices on the switches MDIO
> >   			  bus.
> >   - mdio?		: Container of PHYs and devices on the external MDIO
> > -			  bus. The node must contains a compatible string of
> > +			  bus. The node must contain a compatible string of
> >   			  "marvell,mv88e6xxx-mdio-external"
> >   
> >   Example:
> > @@ -80,7 +80,7 @@ Example:
> >   		#interrupt-cells = <2>;
> >   
> >   		switch0: switch@0 {
> > -			compatible = "marvell,mv88e6390";
> > +			compatible = "marvell,mv88e6190";
> >   			reg = <0>;
> >   			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
> >   
> > 
> > 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] dsa device tree bindings: fix typo and wrong example

[Florian Fainelli]

On 1/7/19 10:49 AM, Pavel Machek wrote:
> On Thu 2018-12-06 13:28:56, Vokáč Michal wrote:
>> On 6.12.2018 14:05, Pavel Machek wrote:
>>>      
>>> Fix typo and fix compatible value that is not actually permitted by the
>>> description in the example.
>>
>> Ahoj Pavle, I think the subject should be more like:
>>
>> 	"dt-bindings: net: dsa: ..."
> 
> Ahoj!
> 
> Well, retrying with different subject only makes sense if someone is
> interested. Rob, Andrew -- can you take the patch?
> 
> Do you want me to edit the subject or can you just edit the subject
> while applying?

The next steps would likely be for you to resubmit this patch with a
re-subjected patch and I am fairly sure David can pick it up right away
since this is not a functional change.

> 
> Thanks,
> 								Pavel
> 
>> BR, Michal
>>
>>>     
>>> Signed-off-by: Pavel Machek <pavel@ucw.cz>
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
>>> index feb007af13cb..abf1be036ac5 100644
>>> --- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
>>> +++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
>>> @@ -41,7 +41,7 @@ Optional properties:
>>>   - mdio			: Container of PHY and devices on the switches MDIO
>>>   			  bus.
>>>   - mdio?		: Container of PHYs and devices on the external MDIO
>>> -			  bus. The node must contains a compatible string of
>>> +			  bus. The node must contain a compatible string of
>>>   			  "marvell,mv88e6xxx-mdio-external"
>>>   
>>>   Example:
>>> @@ -80,7 +80,7 @@ Example:
>>>   		#interrupt-cells = <2>;
>>>   
>>>   		switch0: switch@0 {
>>> -			compatible = "marvell,mv88e6390";
>>> +			compatible = "marvell,mv88e6190";
>>>   			reg = <0>;
>>>   			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
>>>   
>>>
>>>
> 


-- 
Florian

Re: [PATCH] dsa device tree bindings: fix typo and wrong example

[Florian Fainelli]

On 1/7/19 10:49 AM, Pavel Machek wrote:
> On Thu 2018-12-06 13:28:56, Vokáč Michal wrote:
>> On 6.12.2018 14:05, Pavel Machek wrote:
>>>      
>>> Fix typo and fix compatible value that is not actually permitted by the
>>> description in the example.
>>
>> Ahoj Pavle, I think the subject should be more like:
>>
>> 	"dt-bindings: net: dsa: ..."
> 
> Ahoj!
> 
> Well, retrying with different subject only makes sense if someone is
> interested. Rob, Andrew -- can you take the patch?
> 
> Do you want me to edit the subject or can you just edit the subject
> while applying?

The next steps would likely be for you to resubmit this patch with a
re-subjected patch and I am fairly sure David can pick it up right away
since this is not a functional change.

> 
> Thanks,
> 								Pavel
> 
>> BR, Michal
>>
>>>     
>>> Signed-off-by: Pavel Machek <pavel@ucw.cz>
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
>>> index feb007af13cb..abf1be036ac5 100644
>>> --- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
>>> +++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
>>> @@ -41,7 +41,7 @@ Optional properties:
>>>   - mdio			: Container of PHY and devices on the switches MDIO
>>>   			  bus.
>>>   - mdio?		: Container of PHYs and devices on the external MDIO
>>> -			  bus. The node must contains a compatible string of
>>> +			  bus. The node must contain a compatible string of
>>>   			  "marvell,mv88e6xxx-mdio-external"
>>>   
>>>   Example:
>>> @@ -80,7 +80,7 @@ Example:
>>>   		#interrupt-cells = <2>;
>>>   
>>>   		switch0: switch@0 {
>>> -			compatible = "marvell,mv88e6390";
>>> +			compatible = "marvell,mv88e6190";
>>>   			reg = <0>;
>>>   			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
>>>   
>>>
>>>
> 


-- 
Florian

mv88e6xxx -- DSA support for Marvell 88e6065 switch (and maybe 88e6060?)

[Pavel Machek]

[-- Attachment #1.1: Type: text/plain, Size: 8160 bytes --]

Hi!

> > > > I'm trying to create support for Marvell 88e6065 switch... and it
> > > > seems like drivers/net/dsa supports everything, but this model.
> > > > 
> > > > Did someone work with this hardware before? Any idea if it would be
> > > > more suitable to support by existing 88e6060 code, or if 88e6xxx code
> > > > should serve as a base?
> > > 
> > > Hi Pavel
> > > 
> > > The 88e6xxx should be extended to support this. I think you will find
> > > a lot of the building blocks are already in the driver. Compare the
> > > various implementations of the functions in the mv88e6xxx_ops to what
> > > the datasheet says for the registers, and pick those that match.
> > 
> > Ok, so I played a bit.
> > 
> > It looks like e6065 has different register layout from those supported
> > by 6xxx, and is quite similar to e6060.
> 
> However, if you look in the mv88e6xxx, there are quite a few functions
> called mv88e6065_foo. Marvell keeps changing the register layout. When
> writing code, we try to name the functions based on which family of
> devices introduced those registers. But we don't have the whole
> history, so we probably have some names wrong.

Ok, so I took a long look at mv88e6xxx... and got it to work.

Good news is that modifications needed are not too heavy. Most are 
inlined below. (tag_daddr is still needed. I can send that too). 

Bad news is that only about half of all the registers are present on
6065 (6060 is similar), and I'm not sure how to do that
cleanly. Anything marked "W" is not present or reserved or different
in 6065. What would be good markup of registers that are common to all
and that are newer-generations-only be? Mark everything not present on
6065 as MV88E6085_*?

Is someone is interested in getting 6060 to work with mv88e6xxx?

Best regards,
									Pavel


commit b0a8ed2459b8bc1922441edb482d0c7a4217eb1f
Author: Pavel Machek <pavel@denx.de>
Date:   Mon Jan 28 15:12:30 2019 +0100

    6xxx: Add basic support for 6065.

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 33e99dfa00a8..0a1fc2432094 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2094,6 +2094,21 @@ static int mv88e6xxx_set_port_mode_edsa(struct mv88e6xxx_chip *chip, int port)
 				       ETH_P_EDSA);
 }
 
+static int mv88e6xxx_set_port_mode_header(struct mv88e6xxx_chip *chip, int port)
+{
+  	u16 reg;
+	int err;
+
+	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_CTL0, &reg);
+	if (err)
+		return err;
+
+	reg |= MV88E6XXX_PORT_CTL0_HEADER;
+	printk("port %d -- enabling header, val %lx\n", port, (unsigned long) reg);
+
+	return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL0, reg);
+}
+
 static int mv88e6xxx_setup_port_mode(struct mv88e6xxx_chip *chip, int port)
 {
 	if (dsa_is_dsa_port(chip->ds, port))
@@ -2109,6 +2124,11 @@ static int mv88e6xxx_setup_port_mode(struct mv88e6xxx_chip *chip, int port)
 	if (chip->info->tag_protocol == DSA_TAG_PROTO_EDSA)
 		return mv88e6xxx_set_port_mode_edsa(chip, port);
 
+	if (chip->info->tag_protocol == DSA_TAG_PROTO_DADDR)
+		return mv88e6xxx_set_port_mode_header(chip, port);
+
+	dev_warn(chip->dev, "set port mode failed");
+
 	return -EINVAL;
 }
 
@@ -2126,6 +2146,7 @@ static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
 
 	/* Upstream ports flood frames with unknown unicast or multicast DA */
 	flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
+	flood = 1; /* This is strange, but original driver also sets flood everywhere */
 	if (chip->info->ops->port_set_egress_floods)
 		return chip->info->ops->port_set_egress_floods(chip, port,
 							       flood, flood);
@@ -3777,7 +3798,58 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.phylink_validate = mv88e6390x_phylink_validate,
 };
 
+static const struct mv88e6xxx_ops mv88e6065_ops = {
+	/* MV88E6XXX_FAMILY_6095 */ /* Here */
+  //.ieee_pri_map = mv88e6085_g1_ieee_pri_map, /* FIXME */
+  //	.ip_pri_map = mv88e6085_g1_ip_pri_map, /* FIXME */
+  //	.set_switch_mac = mv88e6xxx_g1_set_switch_mac, /* FIXME */
+  	.phy_read = mv88e6185_phy_ppu_read,
+  	.phy_write = mv88e6185_phy_ppu_write,
+  	.port_set_link = mv88e6xxx_port_set_link, /* ok */
+	.port_set_duplex = mv88e6xxx_port_set_duplex, /* ok */
+	.port_set_speed = mv88e6065_port_set_speed, /* ok */
+	.port_set_frame_mode = mv88e6065_port_set_frame_mode, // -- definitely not compatible on 6065; neccessary for correct operation? */
+	.port_set_egress_floods = mv88e6352_port_set_egress_floods, /* ok */
+	//.port_set_upstream_port = mv88e6095_port_set_upstream_port, /* FIXME */
+	.port_link_state = mv88e6352_port_link_state, /* FIXME -- this one is wrong, 6065 uses dufferebt bits */
+	//.port_get_cmode = mv88e6185_port_get_cmode, /* FIXME -- wrong bits */
+	//.stats_snapshot = mv88e6xxx_g1_stats_snapshot, /* FIXME */
+	//.stats_set_histogram = mv88e6095_g1_stats_set_histogram, /* FIXME */
+	//.stats_get_sset_count = mv88e6095_stats_get_sset_count, /* FIXME */
+	//.stats_get_strings = mv88e6095_stats_get_strings, /* FIXME */
+	//.stats_get_stats = mv88e6095_stats_get_stats, /* FIXME */
+	//.mgmt_rsvd2cpu = mv88e6185_g2_mgmt_rsvd2cpu, /* FIXME */
+	//.ppu_enable = mv88e6185_g1_ppu_enable, /* FIXME */
+	//.ppu_disable = mv88e6185_g1_ppu_disable, /* FIXME */
+	.reset = NULL, /* No reset bit in global1: register 4, but we have MV88E6065_G1_RESET */
+	.vtu_getnext = mv88e6185_g1_vtu_getnext, /* ok? */
+	.vtu_loadpurge = mv88e6185_g1_vtu_loadpurge, /* FIXME */
+	.phylink_validate = mv88e6065_phylink_validate,
+};
+
 static const struct mv88e6xxx_info mv88e6xxx_table[] = {
+  	[MV88E6065] = {
+		.prod_num = 0x650,
+		.family = MV88E6XXX_FAMILY_6065,
+		.name = "Marvell 88E6065 not so hacked :-)",
+		.num_databases = 16,
+		.num_ports = 6,
+		.num_internal_phys = 6,
+		.max_vid = 4095,
+		.port_base_addr = 0x18,
+		.phy_base_addr = 0x10,
+		.global1_addr = 0x1f,
+		.global2_addr = 0,
+		.age_time_coeff = 15000,
+		.g1_irqs = 8,
+		.g2_irqs = 0,
+		.atu_move_port_mask = 0xf,
+		.pvt = false,
+		.multi_chip = true,
+		.tag_protocol = DSA_TAG_PROTO_DADDR,
+		.ops = &mv88e6065_ops,
+	},
+
 	[MV88E6085] = {
 		.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6085,
 		.family = MV88E6XXX_FAMILY_6097,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index fc82085d4296..7d6c746d4b66 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -57,10 +57,12 @@ enum mv88e6xxx_frame_mode {
 	MV88E6XXX_FRAME_MODE_DSA,
 	MV88E6XXX_FRAME_MODE_PROVIDER,
 	MV88E6XXX_FRAME_MODE_ETHERTYPE,
+	MV88E6XXX_FRAME_MODE_HEADER,
 };
 
 /* List of supported models */
 enum mv88e6xxx_model {
+  	MV88E6065,
 	MV88E6085,
 	MV88E6095,
 	MV88E6097,
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index 3f43c231f40b..cfa16b5fd41a 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -631,6 +631,31 @@ int mv88e6xxx_port_set_egress_mode(struct mv88e6xxx_chip *chip, int port,
 	return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL0, reg);
 }
 
+int mv88e6065_port_set_frame_mode(struct mv88e6xxx_chip *chip, int port,
+				  enum mv88e6xxx_frame_mode mode)
+{
+  	int err;
+	u16 reg;
+
+	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_CTL0, &reg);
+	if (err)
+		return err;
+
+	reg &= ~MV88E6XXX_PORT_CTL0_FRAME_MODE_MASK;
+
+	switch (mode) {
+	case MV88E6XXX_FRAME_MODE_NORMAL:
+	  /* FIXME ... ?  why is it needed */
+		reg |= MV88E6XXX_PORT_CTL0_FRAME_MODE_NORMAL;
+		break;
+	default:
+	  printk("Bad frame mode\n");
+		return -EINVAL;
+	}
+
+	return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL0, reg);
+}
+
 int mv88e6085_port_set_frame_mode(struct mv88e6xxx_chip *chip, int port,
 				  enum mv88e6xxx_frame_mode mode)
 {

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #1.2: delme2 --]
[-- Type: text/plain, Size: 35206 bytes --]

commit 58a8b67092027331b83311e5621e12870fef0845
Author: Pavel Machek <pavel@denx.de>
Date:   Mon Jan 28 14:59:36 2019 +0100

    6xxx: headers-only: anotate same and different registers on 6065.

diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index 5df4fc049733..6d7c7868f7d8 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -18,15 +18,15 @@
 #include "chip.h"
 
 /* Offset 0x00: Switch Global Status Register */
-#define MV88E6XXX_G1_STS				0x00
-#define MV88E6352_G1_STS_PPU_STATE			0x8000
-#define MV88E6185_G1_STS_PPU_STATE_MASK			0xc000
-#define MV88E6185_G1_STS_PPU_STATE_DISABLED_RST		0x0000
-#define MV88E6185_G1_STS_PPU_STATE_INITIALIZING		0x4000
-#define MV88E6185_G1_STS_PPU_STATE_DISABLED		0x8000
-#define MV88E6185_G1_STS_PPU_STATE_POLLING		0xc000
-#define MV88E6XXX_G1_STS_INIT_READY			0x0800
-#define MV88E6XXX_G1_STS_IRQ_AVB			8
+#define MV88E6XXX_G1_STS				0x00 /* ok */
+#define MV88E6352_G1_STS_PPU_STATE			0x8000 
+#define MV88E6185_G1_STS_PPU_STATE_MASK			0xc000 W /* not on 6065 */
+#define MV88E6185_G1_STS_PPU_STATE_DISABLED_RST		0x0000 W
+#define MV88E6185_G1_STS_PPU_STATE_INITIALIZING		0x4000 W
+#define MV88E6185_G1_STS_PPU_STATE_DISABLED		0x8000 W
+#define MV88E6185_G1_STS_PPU_STATE_POLLING		0xc000 W
+#define MV88E6XXX_G1_STS_INIT_READY			0x0800 /* ok */
+#define MV88E6XXX_G1_STS_IRQ_AVB			8      /* ok vv */
 #define MV88E6XXX_G1_STS_IRQ_DEVICE			7
 #define MV88E6XXX_G1_STS_IRQ_STATS			6
 #define MV88E6XXX_G1_STS_IRQ_VTU_PROB			5
@@ -34,13 +34,13 @@
 #define MV88E6XXX_G1_STS_IRQ_ATU_PROB			3
 #define MV88E6XXX_G1_STS_IRQ_ATU_DONE			2
 #define MV88E6XXX_G1_STS_IRQ_TCAM_DONE			1
-#define MV88E6XXX_G1_STS_IRQ_EEPROM_DONE		0
+#define MV88E6XXX_G1_STS_IRQ_EEPROM_DONE		0      /* ok ^^ */
 
 /* Offset 0x01: Switch MAC Address Register Bytes 0 & 1
  * Offset 0x02: Switch MAC Address Register Bytes 2 & 3
  * Offset 0x03: Switch MAC Address Register Bytes 4 & 5
  */
-#define MV88E6XXX_G1_MAC_01		0x01
+#define MV88E6XXX_G1_MAC_01		0x01 /* ok */
 #define MV88E6XXX_G1_MAC_23		0x02
 #define MV88E6XXX_G1_MAC_45		0x03
 
@@ -57,13 +57,13 @@
 
 /* Offset 0x04: Switch Global Control Register */
 #define MV88E6XXX_G1_CTL1			0x04
-#define MV88E6XXX_G1_CTL1_SW_RESET		0x8000
-#define MV88E6XXX_G1_CTL1_PPU_ENABLE		0x4000
-#define MV88E6352_G1_CTL1_DISCARD_EXCESS	0x2000
-#define MV88E6185_G1_CTL1_SCHED_PRIO		0x0800
-#define MV88E6185_G1_CTL1_MAX_FRAME_1632	0x0400
-#define MV88E6185_G1_CTL1_RELOAD_EEPROM		0x0200
-#define MV88E6XXX_G1_CTL1_DEVICE_EN		0x0080
+#define MV88E6XXX_G1_CTL1_SW_RESET		0x8000 W /* Not on 6065 */
+#define MV88E6XXX_G1_CTL1_PPU_ENABLE		0x4000 W /* Not on 6065 */
+#define MV88E6352_G1_CTL1_DISCARD_EXCESS	0x2000 /* ok */
+#define MV88E6185_G1_CTL1_SCHED_PRIO		0x0800 /* ok */
+#define MV88E6185_G1_CTL1_MAX_FRAME_1632	0x0400 /* ok */
+#define MV88E6185_G1_CTL1_RELOAD_EEPROM		0x0200 /* ok */ 
+#define MV88E6XXX_G1_CTL1_DEVICE_EN		0x0080 /* ?? vv */
 #define MV88E6XXX_G1_CTL1_STATS_DONE_EN		0x0040
 #define MV88E6XXX_G1_CTL1_VTU_PROBLEM_EN	0x0020
 #define MV88E6XXX_G1_CTL1_VTU_DONE_EN		0x0010
@@ -74,24 +74,24 @@
 
 /* Offset 0x05: VTU Operation Register */
 #define MV88E6XXX_G1_VTU_OP			0x05
-#define MV88E6XXX_G1_VTU_OP_BUSY		0x8000
-#define MV88E6XXX_G1_VTU_OP_MASK		0x7000
-#define MV88E6XXX_G1_VTU_OP_FLUSH_ALL		0x1000
+#define MV88E6XXX_G1_VTU_OP_BUSY		0x8000 /* ok */ 
+#define MV88E6XXX_G1_VTU_OP_MASK		0x7000 /* ok */ 
+#define MV88E6XXX_G1_VTU_OP_FLUSH_ALL		0x1000 /* ok vv */
 #define MV88E6XXX_G1_VTU_OP_NOOP		0x2000
 #define MV88E6XXX_G1_VTU_OP_VTU_LOAD_PURGE	0x3000
-#define MV88E6XXX_G1_VTU_OP_VTU_GET_NEXT	0x4000
-#define MV88E6XXX_G1_VTU_OP_STU_LOAD_PURGE	0x5000
-#define MV88E6XXX_G1_VTU_OP_STU_GET_NEXT	0x6000
-#define MV88E6XXX_G1_VTU_OP_GET_CLR_VIOLATION	0x7000
-#define MV88E6XXX_G1_VTU_OP_MEMBER_VIOLATION	BIT(6)
-#define MV88E6XXX_G1_VTU_OP_MISS_VIOLATION	BIT(5)
-#define MV88E6XXX_G1_VTU_OP_SPID_MASK		0xf
+#define MV88E6XXX_G1_VTU_OP_VTU_GET_NEXT	0x4000 /* ok ^^ */
+#define MV88E6XXX_G1_VTU_OP_STU_LOAD_PURGE	0x5000 W /* reserved on 6065 */
+#define MV88E6XXX_G1_VTU_OP_STU_GET_NEXT	0x6000 W /* reserved on 6065 */
+#define MV88E6XXX_G1_VTU_OP_GET_CLR_VIOLATION	0x7000 /* ok */
+#define MV88E6XXX_G1_VTU_OP_MEMBER_VIOLATION	BIT(6) /* ok */
+#define MV88E6XXX_G1_VTU_OP_MISS_VIOLATION	BIT(5) /* ok */
+#define MV88E6XXX_G1_VTU_OP_SPID_MASK		0xf    /* ok */
 
 /* Offset 0x06: VTU VID Register */
 #define MV88E6XXX_G1_VTU_VID		0x06
 #define MV88E6XXX_G1_VTU_VID_MASK	0x0fff
-#define MV88E6390_G1_VTU_VID_PAGE	0x2000
-#define MV88E6XXX_G1_VTU_VID_VALID	0x1000
+#define MV88E6390_G1_VTU_VID_PAGE	0x2000 W /* not on 6065 */
+#define MV88E6XXX_G1_VTU_VID_VALID	0x1000 /* ok */
 
 /* Offset 0x07: VTU/STU Data Register 1
  * Offset 0x08: VTU/STU Data Register 2
@@ -99,7 +99,7 @@
  */
 #define MV88E6XXX_G1_VTU_DATA1				0x07
 #define MV88E6XXX_G1_VTU_DATA2				0x08
-#define MV88E6XXX_G1_VTU_DATA3				0x09
+#define MV88E6XXX_G1_VTU_DATA3				0x09 W /* reserved on 6065 */
 #define MV88E6XXX_G1_VTU_STU_DATA_MASK			0x0003
 #define MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_UNMODIFIED	0x0000
 #define MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_UNTAGGED	0x0001
@@ -113,12 +113,12 @@
 /* Offset 0x0A: ATU Control Register */
 #define MV88E6XXX_G1_ATU_CTL		0x0a
 #define MV88E6065_G1_RESET              0x8000
-#define MV88E6XXX_G1_ATU_CTL_LEARN2ALL	0x0008
+#define MV88E6XXX_G1_ATU_CTL_LEARN2ALL	0x0008 W /* reserved bit */
 
 /* Offset 0x0B: ATU Operation Register */
 #define MV88E6XXX_G1_ATU_OP				0x0b
-#define MV88E6XXX_G1_ATU_OP_BUSY			0x8000
-#define MV88E6XXX_G1_ATU_OP_MASK			0x7000
+#define MV88E6XXX_G1_ATU_OP_BUSY			0x8000 /* ok */
+#define MV88E6XXX_G1_ATU_OP_MASK			0x7000 /* ok vv */
 #define MV88E6XXX_G1_ATU_OP_NOOP			0x0000
 #define MV88E6XXX_G1_ATU_OP_FLUSH_MOVE_ALL		0x1000
 #define MV88E6XXX_G1_ATU_OP_FLUSH_MOVE_NON_STATIC	0x2000
@@ -126,18 +126,18 @@
 #define MV88E6XXX_G1_ATU_OP_GET_NEXT_DB			0x4000
 #define MV88E6XXX_G1_ATU_OP_FLUSH_MOVE_ALL_DB		0x5000
 #define MV88E6XXX_G1_ATU_OP_FLUSH_MOVE_NON_STATIC_DB	0x6000
-#define MV88E6XXX_G1_ATU_OP_GET_CLR_VIOLATION		0x7000
+#define MV88E6XXX_G1_ATU_OP_GET_CLR_VIOLATION		0x7000 
 #define MV88E6XXX_G1_ATU_OP_AGE_OUT_VIOLATION		BIT(7)
 #define MV88E6XXX_G1_ATU_OP_MEMBER_VIOLATION		BIT(6)
 #define MV88E6XXX_G1_ATU_OP_MISS_VIOLATION		BIT(5)
-#define MV88E6XXX_G1_ATU_OP_FULL_VIOLATION		BIT(4)
+#define MV88E6XXX_G1_ATU_OP_FULL_VIOLATION		BIT(4) /* ok ^^ */
 
 /* Offset 0x0C: ATU Data Register */
 #define MV88E6XXX_G1_ATU_DATA				0x0c
-#define MV88E6XXX_G1_ATU_DATA_TRUNK			0x8000
+#define MV88E6XXX_G1_ATU_DATA_TRUNK			0x8000 /* ??? */
 #define MV88E6XXX_G1_ATU_DATA_TRUNK_ID_MASK		0x00f0
 #define MV88E6XXX_G1_ATU_DATA_PORT_VECTOR_MASK		0x3ff0
-#define MV88E6XXX_G1_ATU_DATA_STATE_MASK		0x000f
+#define MV88E6XXX_G1_ATU_DATA_STATE_MASK		0x000f /* ok */
 #define MV88E6XXX_G1_ATU_DATA_STATE_UNUSED		0x0000
 #define MV88E6XXX_G1_ATU_DATA_STATE_UC_MGMT		0x000d
 #define MV88E6XXX_G1_ATU_DATA_STATE_UC_STATIC		0x000e
@@ -151,7 +151,7 @@
  * Offset 0x0E: ATU MAC Address Register Bytes 2 & 3
  * Offset 0x0F: ATU MAC Address Register Bytes 4 & 5
  */
-#define MV88E6XXX_G1_ATU_MAC01		0x0d
+#define MV88E6XXX_G1_ATU_MAC01		0x0d /* ok */
 #define MV88E6XXX_G1_ATU_MAC23		0x0e
 #define MV88E6XXX_G1_ATU_MAC45		0x0f
 
@@ -164,7 +164,7 @@
  * Offset 0x16: IP-PRI Mapping Register 6
  * Offset 0x17: IP-PRI Mapping Register 7
  */
-#define MV88E6XXX_G1_IP_PRI_0	0x10
+#define MV88E6XXX_G1_IP_PRI_0	0x10 /* ok */
 #define MV88E6XXX_G1_IP_PRI_1	0x11
 #define MV88E6XXX_G1_IP_PRI_2	0x12
 #define MV88E6XXX_G1_IP_PRI_3	0x13
@@ -174,10 +174,10 @@
 #define MV88E6XXX_G1_IP_PRI_7	0x17
 
 /* Offset 0x18: IEEE-PRI Register */
-#define MV88E6XXX_G1_IEEE_PRI	0x18
+#define MV88E6XXX_G1_IEEE_PRI	0x18 /* ok */
 
 /* Offset 0x19: Core Tag Type */
-#define MV88E6185_G1_CORE_TAG_TYPE	0x19
+#define MV88E6185_G1_CORE_TAG_TYPE	0x19 W /* reserved on 6065 */
 
 /* Offset 0x1A: Monitor Control */
 #define MV88E6185_G1_MONITOR_CTL			0x1a
@@ -188,8 +188,8 @@
 #define MV88E6352_G1_MONITOR_CTL_MIRROR_DEST_MASK	0x000f
 
 /* Offset 0x1A: Monitor & MGMT Control Register */
-#define MV88E6390_G1_MONITOR_MGMT_CTL				0x1a
-#define MV88E6390_G1_MONITOR_MGMT_CTL_UPDATE			0x8000
+#define MV88E6390_G1_MONITOR_MGMT_CTL				0x1a  /* ok */
+#define MV88E6390_G1_MONITOR_MGMT_CTL_UPDATE			0x8000 W /* not on 6065 */
 #define MV88E6390_G1_MONITOR_MGMT_CTL_PTR_MASK			0x3f00
 #define MV88E6390_G1_MONITOR_MGMT_CTL_PTR_0180C280000000XLO	0x0000
 #define MV88E6390_G1_MONITOR_MGMT_CTL_PTR_0180C280000000XHI	0x0100
@@ -198,10 +198,10 @@
 #define MV88E6390_G1_MONITOR_MGMT_CTL_PTR_INGRESS_DEST		0x2000
 #define MV88E6390_G1_MONITOR_MGMT_CTL_PTR_EGRESS_DEST		0x2100
 #define MV88E6390_G1_MONITOR_MGMT_CTL_PTR_CPU_DEST		0x3000
-#define MV88E6390_G1_MONITOR_MGMT_CTL_DATA_MASK			0x00ff
+#define MV88E6390_G1_MONITOR_MGMT_CTL_DATA_MASK			0x00ff W /* different on 6065 ? */
 
 /* Offset 0x1C: Global Control 2 */
-#define MV88E6XXX_G1_CTL2			0x1c
+#define MV88E6XXX_G1_CTL2			0x1c W /* different, mostly reserved on 6065 */
 #define MV88E6185_G1_CTL2_CASCADE_PORT_MASK	0xf000
 #define MV88E6185_G1_CTL2_CASCADE_PORT_NONE	0xe000
 #define MV88E6185_G1_CTL2_CASCADE_PORT_MULTI	0xf000
@@ -230,11 +230,11 @@
 #define MV88E6390_G1_CTL2_HIST_MODE_TX		0x0080
 #define MV88E6352_G1_CTL2_CTR_MODE_MASK		0x0060
 #define MV88E6390_G1_CTL2_CTR_MODE		0x0020
-#define MV88E6XXX_G1_CTL2_DEVICE_NUMBER_MASK	0x001f
+#define MV88E6XXX_G1_CTL2_DEVICE_NUMBER_MASK	0x001f W /* reserved on 6065 */
 
 /* Offset 0x1D: Stats Operation Register */
 #define MV88E6XXX_G1_STATS_OP			0x1d
-#define MV88E6XXX_G1_STATS_OP_BUSY		0x8000
+#define MV88E6XXX_G1_STATS_OP_BUSY		0x8000 /* ok */
 #define MV88E6XXX_G1_STATS_OP_NOP		0x0000
 #define MV88E6XXX_G1_STATS_OP_FLUSH_ALL		0x1000
 #define MV88E6XXX_G1_STATS_OP_FLUSH_PORT	0x2000
@@ -242,14 +242,14 @@
 #define MV88E6XXX_G1_STATS_OP_CAPTURE_PORT	0x5000
 #define MV88E6XXX_G1_STATS_OP_HIST_RX		0x0400
 #define MV88E6XXX_G1_STATS_OP_HIST_TX		0x0800
-#define MV88E6XXX_G1_STATS_OP_HIST_RX_TX	0x0c00
+#define MV88E6XXX_G1_STATS_OP_HIST_RX_TX	0x0c00 /* ok */
 #define MV88E6XXX_G1_STATS_OP_BANK_1_BIT_9	0x0200
 #define MV88E6XXX_G1_STATS_OP_BANK_1_BIT_10	0x0400
 
 /* Offset 0x1E: Stats Counter Register Bytes 3 & 2
  * Offset 0x1F: Stats Counter Register Bytes 1 & 0
  */
-#define MV88E6XXX_G1_STATS_COUNTER_32	0x1e
+#define MV88E6XXX_G1_STATS_COUNTER_32	0x1e /* ok */
 #define MV88E6XXX_G1_STATS_COUNTER_01	0x1f
 
 int mv88e6xxx_g1_read(struct mv88e6xxx_chip *chip, int reg, u16 *val);
diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
index 48e501d15e20..f5a52b7bfb12 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.h
+++ b/drivers/net/dsa/mv88e6xxx/global2.h
@@ -18,7 +18,7 @@
 #include "chip.h"
 
 /* Offset 0x00: Interrupt Source Register */
-#define MV88E6XXX_G2_INT_SRC			0x00 /* FIXME: reserved */
+#define MV88E6XXX_G2_INT_SRC			0x00 W /* reserved */
 #define MV88E6XXX_G2_INT_SRC_WDOG		0x8000
 #define MV88E6XXX_G2_INT_SRC_JAM_LIMIT		0x4000
 #define MV88E6XXX_G2_INT_SRC_DUPLEX_MISMATCH	0x2000
@@ -30,7 +30,7 @@
 #define MV88E6XXX_G2_INT_SOURCE_WATCHDOG	15
 
 /* Offset 0x01: Interrupt Mask Register */
-#define MV88E6XXX_G2_INT_MASK			0x01 /* FIXME: reserved */
+#define MV88E6XXX_G2_INT_MASK			0x01 W /* reserved */
 #define MV88E6XXX_G2_INT_MASK_WDOG		0x8000
 #define MV88E6XXX_G2_INT_MASK_JAM_LIMIT		0x4000
 #define MV88E6XXX_G2_INT_MASK_DUPLEX_MISMATCH	0x2000
@@ -40,16 +40,16 @@
 #define MV88E6390_G2_INT_MASK_PHY		0x07fe
 
 /* Offset 0x02: MGMT Enable Register 2x */
-#define MV88E6XXX_G2_MGMT_EN_2X		0x02 /* FIXME: reserved */
+#define MV88E6XXX_G2_MGMT_EN_2X		0x02 W /* reserved */
 
 /* Offset 0x03: MGMT Enable Register 0x */
-#define MV88E6XXX_G2_MGMT_EN_0X		0x03 /* FIXME: reserved */
+#define MV88E6XXX_G2_MGMT_EN_0X		0x03 W /* reserved */
 
 /* Offset 0x04: Flow Control Delay Register */
-#define MV88E6XXX_G2_FLOW_CTL	0x04 /* FIXME: reserved */
+#define MV88E6XXX_G2_FLOW_CTL	0x04 W /* reserved */
 
 /* Offset 0x05: Switch Management Register */
-#define MV88E6XXX_G2_SWITCH_MGMT			0x05 /* FIXME: reserved */
+#define MV88E6XXX_G2_SWITCH_MGMT			0x05 W /* reserved */
 #define MV88E6XXX_G2_SWITCH_MGMT_USE_DOUBLE_TAG_DATA	0x8000
 #define MV88E6XXX_G2_SWITCH_MGMT_PREVENT_LOOPS		0x4000
 #define MV88E6XXX_G2_SWITCH_MGMT_FLOW_CTL_MSG		0x2000
@@ -57,7 +57,7 @@
 #define MV88E6XXX_G2_SWITCH_MGMT_RSVD2CPU		0x0008
 
 /* Offset 0x06: Device Mapping Table Register */
-#define MV88E6XXX_G2_DEVICE_MAPPING		0x06 /* FIXME: reserved */
+#define MV88E6XXX_G2_DEVICE_MAPPING		0x06 W /* reserved */
 #define MV88E6XXX_G2_DEVICE_MAPPING_UPDATE	0x8000
 #define MV88E6XXX_G2_DEVICE_MAPPING_DEV_MASK	0x1f00
 #define MV88E6352_G2_DEVICE_MAPPING_PORT_MASK	0x000f
@@ -70,7 +70,7 @@
 #define MV88E6XXX_G2_TRUNK_MASK_HASH		0x0800
 
 /* Offset 0x08: Trunk Mapping Table Register */
-#define MV88E6XXX_G2_TRUNK_MAPPING		0x08 /* FIXME: reserved */
+#define MV88E6XXX_G2_TRUNK_MAPPING		0x08 W /* reserved */
 #define MV88E6XXX_G2_TRUNK_MAPPING_UPDATE	0x8000
 #define MV88E6XXX_G2_TRUNK_MAPPING_ID_MASK	0x7800
 
@@ -94,11 +94,11 @@
 #define MV88E6XXX_G2_IRL_CMD_REG_MASK		0x000f
 
 /* Offset 0x0A: Ingress Rate Data Register */
-#define MV88E6XXX_G2_IRL_DATA		0x0a
+#define MV88E6XXX_G2_IRL_DATA		0x0a /* ok */
 #define MV88E6XXX_G2_IRL_DATA_MASK	0xffff
 
 /* Offset 0x0B: Cross-chip Port VLAN Register */
-#define MV88E6XXX_G2_PVT_ADDR			0x0b
+#define MV88E6XXX_G2_PVT_ADDR			0x0b W /* reserved */
 #define MV88E6XXX_G2_PVT_ADDR_BUSY		0x8000
 #define MV88E6XXX_G2_PVT_ADDR_OP_MASK		0x7000
 #define MV88E6XXX_G2_PVT_ADDR_OP_INIT_ONES	0x1000
@@ -107,11 +107,11 @@
 #define MV88E6XXX_G2_PVT_ADDR_PTR_MASK		0x01ff
 
 /* Offset 0x0C: Cross-chip Port VLAN Data Register */
-#define MV88E6XXX_G2_PVT_DATA		0x0c
+#define MV88E6XXX_G2_PVT_DATA		0x0c W /* reserved */
 #define MV88E6XXX_G2_PVT_DATA_MASK	0x7f
 
 /* Offset 0x0D: Switch MAC/WoL/WoF Register */
-#define MV88E6XXX_G2_SWITCH_MAC			0x0d
+#define MV88E6XXX_G2_SWITCH_MAC			0x0d W /* reserved */
 #define MV88E6XXX_G2_SWITCH_MAC_UPDATE		0x8000
 #define MV88E6XXX_G2_SWITCH_MAC_PTR_MASK	0x1f00
 #define MV88E6XXX_G2_SWITCH_MAC_DATA_MASK	0x00ff
@@ -120,7 +120,7 @@
 #define MV88E6XXX_G2_ATU_STATS		0x0e
 
 /* Offset 0x0F: Priority Override Table */
-#define MV88E6XXX_G2_PRIO_OVERRIDE		0x0f
+#define MV88E6XXX_G2_PRIO_OVERRIDE		0x0f W /* reserved */
 #define MV88E6XXX_G2_PRIO_OVERRIDE_UPDATE	0x8000
 #define MV88E6XXX_G2_PRIO_OVERRIDE_FPRISET	0x1000
 #define MV88E6XXX_G2_PRIO_OVERRIDE_PTR_MASK	0x0f00
@@ -130,7 +130,7 @@
 #define MV88E6XXX_G2_PRIO_OVERRIDE_DATA_MASK	0x0007
 
 /* Offset 0x14: EEPROM Command */
-#define MV88E6XXX_G2_EEPROM_CMD			0x14
+#define MV88E6XXX_G2_EEPROM_CMD			0x14 W /* reserved */
 #define MV88E6XXX_G2_EEPROM_CMD_BUSY		0x8000
 #define MV88E6XXX_G2_EEPROM_CMD_OP_MASK		0x7000
 #define MV88E6XXX_G2_EEPROM_CMD_OP_WRITE	0x3000
@@ -142,15 +142,15 @@
 #define MV88E6390_G2_EEPROM_CMD_DATA_MASK	0x00ff
 
 /* Offset 0x15: EEPROM Data */
-#define MV88E6352_G2_EEPROM_DATA	0x15
+#define MV88E6352_G2_EEPROM_DATA	0x15 W /* reserved */
 #define MV88E6352_G2_EEPROM_DATA_MASK	0xffff
 
 /* Offset 0x15: EEPROM Addr */
-#define MV88E6390_G2_EEPROM_ADDR	0x15
+#define MV88E6390_G2_EEPROM_ADDR	0x15 W /* reserved */
 #define MV88E6390_G2_EEPROM_ADDR_MASK	0xffff
 
 /* Offset 0x16: AVB Command Register */
-#define MV88E6352_G2_AVB_CMD			0x16
+#define MV88E6352_G2_AVB_CMD			0x16 W /* reserved */
 #define MV88E6352_G2_AVB_CMD_BUSY		0x8000
 #define MV88E6352_G2_AVB_CMD_OP_READ		0x4000
 #define MV88E6352_G2_AVB_CMD_OP_READ_INCR	0x6000
@@ -173,10 +173,10 @@
 #define MV88E6352_G2_AVB_CMD_ADDR_MASK		0x001f
 
 /* Offset 0x17: AVB Data Register */
-#define MV88E6352_G2_AVB_DATA		0x17
+#define MV88E6352_G2_AVB_DATA		0x17 W /* reserved */
 
 /* Offset 0x18: SMI PHY Command Register */
-#define MV88E6XXX_G2_SMI_PHY_CMD			0x18
+#define MV88E6XXX_G2_SMI_PHY_CMD			0x18 W /* reserved */
 #define MV88E6XXX_G2_SMI_PHY_CMD_BUSY			0x8000
 #define MV88E6390_G2_SMI_PHY_CMD_FUNC_MASK		0x6000
 #define MV88E6390_G2_SMI_PHY_CMD_FUNC_INTERNAL		0x0000
@@ -197,16 +197,16 @@
 #define MV88E6XXX_G2_SMI_PHY_CMD_SETUP_PTR_MASK		0x03ff
 
 /* Offset 0x19: SMI PHY Data Register */
-#define MV88E6XXX_G2_SMI_PHY_DATA	0x19
+#define MV88E6XXX_G2_SMI_PHY_DATA	0x19 W /* reserved */
 
 /* Offset 0x1A: Scratch and Misc. Register */
-#define MV88E6XXX_G2_SCRATCH_MISC_MISC		0x1a
+#define MV88E6XXX_G2_SCRATCH_MISC_MISC		0x1a W /* reserved */
 #define MV88E6XXX_G2_SCRATCH_MISC_UPDATE	0x8000
 #define MV88E6XXX_G2_SCRATCH_MISC_PTR_MASK	0x7f00
 #define MV88E6XXX_G2_SCRATCH_MISC_DATA_MASK	0x00ff
 
 /* Offset 0x1B: Watch Dog Control Register */
-#define MV88E6352_G2_WDOG_CTL			0x1b
+#define MV88E6352_G2_WDOG_CTL			0x1b W /* reserved */
 #define MV88E6352_G2_WDOG_CTL_EGRESS_EVENT	0x0080
 #define MV88E6352_G2_WDOG_CTL_RMU_TIMEOUT	0x0040
 #define MV88E6352_G2_WDOG_CTL_QC_ENABLE		0x0020
@@ -217,7 +217,7 @@
 #define MV88E6352_G2_WDOG_CTL_SWRESET		0x0001
 
 /* Offset 0x1B: Watch Dog Control Register */
-#define MV88E6390_G2_WDOG_CTL				0x1b
+#define MV88E6390_G2_WDOG_CTL				0x1b W /* reserved */
 #define MV88E6390_G2_WDOG_CTL_UPDATE			0x8000
 #define MV88E6390_G2_WDOG_CTL_PTR_MASK			0x7f00
 #define MV88E6390_G2_WDOG_CTL_PTR_INT_SOURCE		0x0000
@@ -232,14 +232,14 @@
 #define MV88E6390_G2_WDOG_CTL_FORCE_IRQ			0x0001
 
 /* Offset 0x1C: QoS Weights Register */
-#define MV88E6XXX_G2_QOS_WEIGHTS		0x1c
+#define MV88E6XXX_G2_QOS_WEIGHTS		0x1c W /* reserved */
 #define MV88E6XXX_G2_QOS_WEIGHTS_UPDATE		0x8000
 #define MV88E6352_G2_QOS_WEIGHTS_PTR_MASK	0x3f00
 #define MV88E6390_G2_QOS_WEIGHTS_PTR_MASK	0x7f00
 #define MV88E6XXX_G2_QOS_WEIGHTS_DATA_MASK	0x00ff
 
 /* Offset 0x1D: Misc Register */
-#define MV88E6XXX_G2_MISC		0x1d
+#define MV88E6XXX_G2_MISC		0x1d W /* reserved */
 #define MV88E6XXX_G2_MISC_5_BIT_PORT	0x4000
 #define MV88E6352_G2_NOEGR_POLICY	0x2000
 #define MV88E6390_G2_LAG_ID_4		0x2000
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index f32f56af8e35..5b9dde5c3334 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -19,13 +19,13 @@
 
 /* Offset 0x00: Port Status Register */
 #define MV88E6XXX_PORT_STS			0x00
-#define MV88E6XXX_PORT_STS_PAUSE_EN		0x8000
+#define MV88E6XXX_PORT_STS_PAUSE_EN		0x8000 /* ok vvv */
 #define MV88E6XXX_PORT_STS_MY_PAUSE		0x4000
-#define MV88E6XXX_PORT_STS_HD_FLOW		0x2000
+#define MV88E6XXX_PORT_STS_HD_FLOW		0x2000 W /* "link mode is resolved" on 6065 */
 #define MV88E6XXX_PORT_STS_PHY_DETECT		0x1000
-#define MV88E6XXX_PORT_STS_LINK			0x0800
-#define MV88E6XXX_PORT_STS_DUPLEX		0x0400
-#define MV88E6XXX_PORT_STS_SPEED_MASK		0x0300
+#define MV88E6XXX_PORT_STS_LINK			0x0800 W /* -- 0x1000 on 6065? */
+#define MV88E6XXX_PORT_STS_DUPLEX		0x0400 W /* -- 0x200 on 6065*/
+#define MV88E6XXX_PORT_STS_SPEED_MASK		0x0300 W /* -- mask is 0x0f00 on 6065 */
 #define MV88E6XXX_PORT_STS_SPEED_10		0x0000
 #define MV88E6XXX_PORT_STS_SPEED_100		0x0100
 #define MV88E6XXX_PORT_STS_SPEED_1000		0x0200
@@ -33,38 +33,38 @@
 #define MV88E6352_PORT_STS_EEE			0x0040
 #define MV88E6165_PORT_STS_AM_DIS		0x0040
 #define MV88E6185_PORT_STS_MGMII		0x0040
-#define MV88E6XXX_PORT_STS_TX_PAUSED		0x0020
-#define MV88E6XXX_PORT_STS_FLOW_CTL		0x0010
-#define MV88E6XXX_PORT_STS_CMODE_MASK		0x000f
-#define MV88E6XXX_PORT_STS_CMODE_100BASE_X	0x0008
-#define MV88E6XXX_PORT_STS_CMODE_1000BASE_X	0x0009
-#define MV88E6XXX_PORT_STS_CMODE_SGMII		0x000a
-#define MV88E6XXX_PORT_STS_CMODE_2500BASEX	0x000b
-#define MV88E6XXX_PORT_STS_CMODE_XAUI		0x000c
-#define MV88E6XXX_PORT_STS_CMODE_RXAUI		0x000d
-#define MV88E6185_PORT_STS_CDUPLEX		0x0008
-#define MV88E6185_PORT_STS_CMODE_MASK		0x0007
-#define MV88E6185_PORT_STS_CMODE_GMII_FD	0x0000
-#define MV88E6185_PORT_STS_CMODE_MII_100_FD_PS	0x0001
-#define MV88E6185_PORT_STS_CMODE_MII_100	0x0002
-#define MV88E6185_PORT_STS_CMODE_MII_10		0x0003
-#define MV88E6185_PORT_STS_CMODE_SERDES		0x0004
-#define MV88E6185_PORT_STS_CMODE_1000BASE_X	0x0005
-#define MV88E6185_PORT_STS_CMODE_PHY		0x0006
-#define MV88E6185_PORT_STS_CMODE_DISABLED	0x0007
+#define MV88E6XXX_PORT_STS_TX_PAUSED		0x0020 /* ok */
+#define MV88E6XXX_PORT_STS_FLOW_CTL		0x0010 /* ok */
+#define MV88E6XXX_PORT_STS_CMODE_MASK		0x000f W /* vv these are reserved or different */
+#define MV88E6XXX_PORT_STS_CMODE_100BASE_X	0x0008 W
+#define MV88E6XXX_PORT_STS_CMODE_1000BASE_X	0x0009 W
+#define MV88E6XXX_PORT_STS_CMODE_SGMII		0x000a W
+#define MV88E6XXX_PORT_STS_CMODE_2500BASEX	0x000b W
+#define MV88E6XXX_PORT_STS_CMODE_XAUI		0x000c W
+#define MV88E6XXX_PORT_STS_CMODE_RXAUI		0x000d W
+#define MV88E6185_PORT_STS_CDUPLEX		0x0008 W
+#define MV88E6185_PORT_STS_CMODE_MASK		0x0007 W
+#define MV88E6185_PORT_STS_CMODE_GMII_FD	0x0000 W
+#define MV88E6185_PORT_STS_CMODE_MII_100_FD_PS	0x0001 W
+#define MV88E6185_PORT_STS_CMODE_MII_100	0x0002 W
+#define MV88E6185_PORT_STS_CMODE_MII_10		0x0003 W
+#define MV88E6185_PORT_STS_CMODE_SERDES		0x0004 W
+#define MV88E6185_PORT_STS_CMODE_1000BASE_X	0x0005 W
+#define MV88E6185_PORT_STS_CMODE_PHY		0x0006 W
+#define MV88E6185_PORT_STS_CMODE_DISABLED	0x0007 W /* ^^ */
 
 /* Offset 0x01: MAC (or PCS or Physical) Control Register */
 #define MV88E6XXX_PORT_MAC_CTL				0x01
-#define MV88E6XXX_PORT_MAC_CTL_RGMII_DELAY_RXCLK	0x8000
-#define MV88E6XXX_PORT_MAC_CTL_RGMII_DELAY_TXCLK	0x4000
-#define MV88E6185_PORT_MAC_CTL_SYNC_OK			0x4000
-#define MV88E6390_PORT_MAC_CTL_FORCE_SPEED		0x2000
-#define MV88E6390_PORT_MAC_CTL_ALTSPEED			0x1000
-#define MV88E6352_PORT_MAC_CTL_200BASE			0x1000
-#define MV88E6185_PORT_MAC_CTL_AN_EN			0x0400
-#define MV88E6185_PORT_MAC_CTL_AN_RESTART		0x0200
-#define MV88E6185_PORT_MAC_CTL_AN_DONE			0x0100
-#define MV88E6XXX_PORT_MAC_CTL_FC			0x0080
+#define MV88E6XXX_PORT_MAC_CTL_RGMII_DELAY_RXCLK	0x8000 W /* vv reserved */
+#define MV88E6XXX_PORT_MAC_CTL_RGMII_DELAY_TXCLK	0x4000 W
+#define MV88E6185_PORT_MAC_CTL_SYNC_OK			0x4000 W
+#define MV88E6390_PORT_MAC_CTL_FORCE_SPEED		0x2000 W
+#define MV88E6390_PORT_MAC_CTL_ALTSPEED			0x1000 W
+#define MV88E6352_PORT_MAC_CTL_200BASE			0x1000 W
+#define MV88E6185_PORT_MAC_CTL_AN_EN			0x0400 W
+#define MV88E6185_PORT_MAC_CTL_AN_RESTART		0x0200 W /* ^^ reserved */
+#define MV88E6185_PORT_MAC_CTL_AN_DONE			0x0100 W /* reserved on 6065 */
+#define MV88E6XXX_PORT_MAC_CTL_FC			0x0080 /* ok vv */
 #define MV88E6XXX_PORT_MAC_CTL_FORCE_FC			0x0040
 #define MV88E6XXX_PORT_MAC_CTL_LINK_UP			0x0020
 #define MV88E6XXX_PORT_MAC_CTL_FORCE_LINK		0x0010
@@ -76,15 +76,15 @@
 #define MV88E6065_PORT_MAC_CTL_SPEED_200		0x0002
 #define MV88E6XXX_PORT_MAC_CTL_SPEED_1000		0x0002
 #define MV88E6390_PORT_MAC_CTL_SPEED_10000		0x0003
-#define MV88E6XXX_PORT_MAC_CTL_SPEED_UNFORCED		0x0003
+#define MV88E6XXX_PORT_MAC_CTL_SPEED_UNFORCED		0x0003 /* ok ^^ */
 
 /* Offset 0x02: Jamming Control Register */
-#define MV88E6097_PORT_JAM_CTL			0x02
+#define MV88E6097_PORT_JAM_CTL			0x02 W /* Not on 6065 */
 #define MV88E6097_PORT_JAM_CTL_LIMIT_OUT_MASK	0xff00
 #define MV88E6097_PORT_JAM_CTL_LIMIT_IN_MASK	0x00ff
 
 /* Offset 0x02: Flow Control Register */
-#define MV88E6390_PORT_FLOW_CTL			0x02
+#define MV88E6390_PORT_FLOW_CTL			0x02 W
 #define MV88E6390_PORT_FLOW_CTL_UPDATE		0x8000
 #define MV88E6390_PORT_FLOW_CTL_PTR_MASK	0x7f00
 #define MV88E6390_PORT_FLOW_CTL_LIMIT_IN	0x0000
@@ -92,8 +92,10 @@
 #define MV88E6390_PORT_FLOW_CTL_DATA_MASK	0x00ff
 
 /* Offset 0x03: Switch Identifier Register */
-#define MV88E6XXX_PORT_SWITCH_ID		0x03
+#define MV88E6XXX_PORT_SWITCH_ID		0x03 /* ok */
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_MASK	0xfff0
+#define MV88E6XXX_PORT_SWITCH_ID_PROD_6035	0x0350 /* very similar to 6065 */
+#define MV88E6XXX_PORT_SWITCH_ID_PROD_6065	0x0650
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6085	0x04a0
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6095	0x0950
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6097	0x0990
@@ -124,132 +126,133 @@
 
 /* Offset 0x04: Port Control Register */
 #define MV88E6XXX_PORT_CTL0					0x04
-#define MV88E6XXX_PORT_CTL0_USE_CORE_TAG			0x8000
-#define MV88E6XXX_PORT_CTL0_DROP_ON_LOCK			0x4000
-#define MV88E6XXX_PORT_CTL0_EGRESS_MODE_MASK			0x3000
+#define MV88E6XXX_PORT_CTL0_USE_CORE_TAG			0x8000 W /* not on 6065, unused */
+#define MV88E6XXX_PORT_CTL0_DROP_ON_LOCK			0x4000 /* ...but this matches? */ 
+#define MV88E6XXX_PORT_CTL0_EGRESS_MODE_MASK			0x3000 /* ok, matches 6065 */
 #define MV88E6XXX_PORT_CTL0_EGRESS_MODE_UNMODIFIED		0x0000
 #define MV88E6XXX_PORT_CTL0_EGRESS_MODE_UNTAGGED		0x1000
 #define MV88E6XXX_PORT_CTL0_EGRESS_MODE_TAGGED			0x2000
-#define MV88E6XXX_PORT_CTL0_EGRESS_MODE_ETHER_TYPE_DSA		0x3000
-#define MV88E6XXX_PORT_CTL0_HEADER				0x0800
-#define MV88E6XXX_PORT_CTL0_IGMP_MLD_SNOOP			0x0400
-#define MV88E6XXX_PORT_CTL0_DOUBLE_TAG				0x0200
+#define MV88E6XXX_PORT_CTL0_EGRESS_MODE_ETHER_TYPE_DSA		0x3000 W /* not on 6065, unused */
+#define MV88E6XXX_PORT_CTL0_HEADER				0x0800 /* ok, matches 6065 */
+#define MV88E6XXX_PORT_CTL0_IGMP_MLD_SNOOP			0x0400 /* ok */
+#define MV88E6XXX_PORT_CTL0_DOUBLE_TAG				0x0200 W /* not on 6065, unused */
 #define MV88E6XXX_PORT_CTL0_FRAME_MODE_MASK			0x0300
 #define MV88E6XXX_PORT_CTL0_FRAME_MODE_NORMAL			0x0000
-#define MV88E6XXX_PORT_CTL0_FRAME_MODE_DSA			0x0100
-#define MV88E6XXX_PORT_CTL0_FRAME_MODE_PROVIDER			0x0200
+#define MV88E6XXX_PORT_CTL0_FRAME_MODE_DSA			0x0100 
+#define MV88E6XXX_PORT_CTL0_FRAME_MODE_PROVIDER			0x0200 W /* not on 6065, not used  */
 #define MV88E6XXX_PORT_CTL0_FRAME_MODE_ETHER_TYPE_DSA		0x0300
-#define MV88E6XXX_PORT_CTL0_DSA_TAG				0x0100
-#define MV88E6XXX_PORT_CTL0_VLAN_TUNNEL				0x0080
-#define MV88E6XXX_PORT_CTL0_TAG_IF_BOTH				0x0040
-#define MV88E6185_PORT_CTL0_USE_IP				0x0020
-#define MV88E6185_PORT_CTL0_USE_TAG				0x0010
+#define MV88E6XXX_PORT_CTL0_DSA_TAG				0x0100 W /* not on 6065, unused */
+#define MV88E6XXX_PORT_CTL0_VLAN_TUNNEL				0x0080 /* ok */
+#define MV88E6XXX_PORT_CTL0_TAG_IF_BOTH				0x0040 /* ok */
+#define MV88E6185_PORT_CTL0_USE_IP				0x0020 /* ok */
+#define MV88E6185_PORT_CTL0_USE_TAG				0x0010 /* ok */
 #define MV88E6185_PORT_CTL0_FORWARD_UNKNOWN			0x0004
-#define MV88E6352_PORT_CTL0_EGRESS_FLOODS_MASK			0x000c
+#define MV88E6352_PORT_CTL0_EGRESS_FLOODS_MASK			0x000c /* ok */
 #define MV88E6352_PORT_CTL0_EGRESS_FLOODS_NO_UNKNOWN_DA		0x0000
 #define MV88E6352_PORT_CTL0_EGRESS_FLOODS_NO_UNKNOWN_MC_DA	0x0004
 #define MV88E6352_PORT_CTL0_EGRESS_FLOODS_NO_UNKNOWN_UC_DA	0x0008
 #define MV88E6352_PORT_CTL0_EGRESS_FLOODS_ALL_UNKNOWN_DA	0x000c
-#define MV88E6XXX_PORT_CTL0_STATE_MASK				0x0003
+#define MV88E6XXX_PORT_CTL0_STATE_MASK				0x0003 /* ok */
 #define MV88E6XXX_PORT_CTL0_STATE_DISABLED			0x0000
 #define MV88E6XXX_PORT_CTL0_STATE_BLOCKING			0x0001
 #define MV88E6XXX_PORT_CTL0_STATE_LEARNING			0x0002
 #define MV88E6XXX_PORT_CTL0_STATE_FORWARDING			0x0003
 
 /* Offset 0x05: Port Control 1 */
-#define MV88E6XXX_PORT_CTL1			0x05
+#define MV88E6XXX_PORT_CTL1			0x05 W /* undocumented / reserved */
 #define MV88E6XXX_PORT_CTL1_MESSAGE_PORT	0x8000
 #define MV88E6XXX_PORT_CTL1_FID_11_4_MASK	0x00ff
 
 /* Offset 0x06: Port Based VLAN Map */
 #define MV88E6XXX_PORT_BASE_VLAN		0x06
-#define MV88E6XXX_PORT_BASE_VLAN_FID_3_0_MASK	0xf000
+#define MV88E6XXX_PORT_BASE_VLAN_FID_3_0_MASK	0xf000 W /* 6065 has dbum there */
+/* ...and more bits below */
 
 /* Offset 0x07: Default Port VLAN ID & Priority */
 #define MV88E6XXX_PORT_DEFAULT_VLAN		0x07
-#define MV88E6XXX_PORT_DEFAULT_VLAN_MASK	0x0fff
+#define MV88E6XXX_PORT_DEFAULT_VLAN_MASK	0x0fff /* ok */
 
 /* Offset 0x08: Port Control 2 Register */
 #define MV88E6XXX_PORT_CTL2				0x08
-#define MV88E6XXX_PORT_CTL2_IGNORE_FCS			0x8000
+#define MV88E6XXX_PORT_CTL2_IGNORE_FCS			0x8000 /* ok vv */
 #define MV88E6XXX_PORT_CTL2_VTU_PRI_OVERRIDE		0x4000
 #define MV88E6XXX_PORT_CTL2_SA_PRIO_OVERRIDE		0x2000
-#define MV88E6XXX_PORT_CTL2_DA_PRIO_OVERRIDE		0x1000
-#define MV88E6XXX_PORT_CTL2_JUMBO_MODE_MASK		0x3000
-#define MV88E6XXX_PORT_CTL2_JUMBO_MODE_1522		0x0000
-#define MV88E6XXX_PORT_CTL2_JUMBO_MODE_2048		0x1000
-#define MV88E6XXX_PORT_CTL2_JUMBO_MODE_10240		0x2000
-#define MV88E6XXX_PORT_CTL2_8021Q_MODE_MASK		0x0c00
+#define MV88E6XXX_PORT_CTL2_DA_PRIO_OVERRIDE		0x1000 /* ok ?? */
+#define MV88E6XXX_PORT_CTL2_JUMBO_MODE_MASK		0x3000 W /* vv: incompatible */
+#define MV88E6XXX_PORT_CTL2_JUMBO_MODE_1522		0x0000 W
+#define MV88E6XXX_PORT_CTL2_JUMBO_MODE_2048		0x1000 W
+#define MV88E6XXX_PORT_CTL2_JUMBO_MODE_10240		0x2000 W /* ^^ */
+#define MV88E6XXX_PORT_CTL2_8021Q_MODE_MASK		0x0c00 /* ok vv */
 #define MV88E6XXX_PORT_CTL2_8021Q_MODE_DISABLED		0x0000
 #define MV88E6XXX_PORT_CTL2_8021Q_MODE_FALLBACK		0x0400
 #define MV88E6XXX_PORT_CTL2_8021Q_MODE_CHECK		0x0800
 #define MV88E6XXX_PORT_CTL2_8021Q_MODE_SECURE		0x0c00
 #define MV88E6XXX_PORT_CTL2_DISCARD_TAGGED		0x0200
 #define MV88E6XXX_PORT_CTL2_DISCARD_UNTAGGED		0x0100
-#define MV88E6XXX_PORT_CTL2_MAP_DA			0x0080
-#define MV88E6XXX_PORT_CTL2_DEFAULT_FORWARD		0x0040
-#define MV88E6XXX_PORT_CTL2_EGRESS_MONITOR		0x0020
-#define MV88E6XXX_PORT_CTL2_INGRESS_MONITOR		0x0010
-#define MV88E6095_PORT_CTL2_CPU_PORT_MASK		0x000f
+#define MV88E6XXX_PORT_CTL2_MAP_DA			0x0080 /* ok %% */
+#define MV88E6XXX_PORT_CTL2_DEFAULT_FORWARD		0x0040 W /* reserved on 6065 */
+#define MV88E6XXX_PORT_CTL2_EGRESS_MONITOR		0x0020 W /* reserved on 6065 */
+#define MV88E6XXX_PORT_CTL2_INGRESS_MONITOR		0x0010 W /* reserved on 6065 */
+#define MV88E6095_PORT_CTL2_CPU_PORT_MASK		0x000f W /* 6065 has qpri overrides here */
 
 /* Offset 0x09: Egress Rate Control */
-#define MV88E6XXX_PORT_EGRESS_RATE_CTL1		0x09
+#define MV88E6XXX_PORT_EGRESS_RATE_CTL1		0x09 W /*  we have ingress control here */
 
 /* Offset 0x0A: Egress Rate Control 2 */
-#define MV88E6XXX_PORT_EGRESS_RATE_CTL2		0x0a
+#define MV88E6XXX_PORT_EGRESS_RATE_CTL2		0x0a W /* so this may not be compatible */
 
 /* Offset 0x0B: Port Association Vector */
 #define MV88E6XXX_PORT_ASSOC_VECTOR			0x0b
-#define MV88E6XXX_PORT_ASSOC_VECTOR_HOLD_AT_1		0x8000
-#define MV88E6XXX_PORT_ASSOC_VECTOR_INT_AGE_OUT		0x4000
-#define MV88E6XXX_PORT_ASSOC_VECTOR_LOCKED_PORT		0x2000
-#define MV88E6XXX_PORT_ASSOC_VECTOR_IGNORE_WRONG	0x1000
-#define MV88E6XXX_PORT_ASSOC_VECTOR_REFRESH_LOCKED	0x0800
+#define MV88E6XXX_PORT_ASSOC_VECTOR_HOLD_AT_1		0x8000 W /* 6065 is different -- Ingress monitor */
+#define MV88E6XXX_PORT_ASSOC_VECTOR_INT_AGE_OUT		0x4000 W /* portsched on 6065 */
+#define MV88E6XXX_PORT_ASSOC_VECTOR_LOCKED_PORT		0x2000 /* ok */
+#define MV88E6XXX_PORT_ASSOC_VECTOR_IGNORE_WRONG	0x1000 /* ok */
+#define MV88E6XXX_PORT_ASSOC_VECTOR_REFRESH_LOCKED	0x0800 W /* 6065 has agenten */
 
 /* Offset 0x0C: Port ATU Control */
-#define MV88E6XXX_PORT_ATU_CTL		0x0c
+#define MV88E6XXX_PORT_ATU_CTL		0x0c W /* not on 6065? */
 
 /* Offset 0x0D: Priority Override Register */
-#define MV88E6XXX_PORT_PRI_OVERRIDE	0x0d
+#define MV88E6XXX_PORT_PRI_OVERRIDE	0x0d W /* not on 6065? */
 
 /* Offset 0x0E: Policy Control Register */
-#define MV88E6XXX_PORT_POLICY_CTL	0x0e
+#define MV88E6XXX_PORT_POLICY_CTL	0x0e W /* not on 6065? */
 
 /* Offset 0x0F: Port Special Ether Type */
-#define MV88E6XXX_PORT_ETH_TYPE		0x0f
+#define MV88E6XXX_PORT_ETH_TYPE		0x0f W  /* not on 6065? */
 #define MV88E6XXX_PORT_ETH_TYPE_DEFAULT	0x9100
 
 /* Offset 0x10: InDiscards Low Counter */
-#define MV88E6XXX_PORT_IN_DISCARD_LO	0x10
+#define MV88E6XXX_PORT_IN_DISCARD_LO	0x10 W /* different counter on 6065 */
 
 /* Offset 0x11: InDiscards High Counter */
-#define MV88E6XXX_PORT_IN_DISCARD_HI	0x11
+#define MV88E6XXX_PORT_IN_DISCARD_HI	0x11 W /* different counter on 6065 */
 
 /* Offset 0x12: InFiltered Counter */
-#define MV88E6XXX_PORT_IN_FILTERED	0x12
+#define MV88E6XXX_PORT_IN_FILTERED	0x12 W /* ok, depending on other bit? */
 
 /* Offset 0x13: OutFiltered Counter */
-#define MV88E6XXX_PORT_OUT_FILTERED	0x13
+#define MV88E6XXX_PORT_OUT_FILTERED	0x13 W /* not on 6065 */
 
 /* Offset 0x18: IEEE Priority Mapping Table */
 #define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE			0x18
-#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_UPDATE		0x8000
-#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_MASK			0x7000
-#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_INGRESS_PCP		0x0000
-#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_GREEN_PCP	0x1000
-#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_YELLOW_PCP	0x2000
-#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_AVB_PCP	0x3000
-#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_GREEN_DSCP	0x5000
-#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_YELLOW_DSCP	0x6000
-#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_AVB_DSCP	0x7000
-#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_PTR_MASK		0x0e00
-#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_DATA_MASK		0x01ff
+#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_UPDATE		0x8000 W /* reserved on 6065 */
+#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_MASK			0x7000 W
+#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_INGRESS_PCP		0x0000 W
+#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_GREEN_PCP	0x1000 W
+#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_YELLOW_PCP	0x2000 W
+#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_AVB_PCP	0x3000 W
+#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_GREEN_DSCP	0x5000 W
+#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_YELLOW_DSCP	0x6000 W
+#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_EGRESS_AVB_DSCP	0x7000 W
+#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_PTR_MASK		0x0e00 W
+#define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_DATA_MASK		0x01ff W /* ^^ looks different on 6065 */
 
 /* Offset 0x18: Port IEEE Priority Remapping Registers (0-3) */
-#define MV88E6095_PORT_IEEE_PRIO_REMAP_0123	0x18
+#define MV88E6095_PORT_IEEE_PRIO_REMAP_0123	0x18 W
 
 /* Offset 0x19: Port IEEE Priority Remapping Registers (4-7) */
-#define MV88E6095_PORT_IEEE_PRIO_REMAP_4567	0x19
+#define MV88E6095_PORT_IEEE_PRIO_REMAP_4567	0x19 W
 
 int mv88e6xxx_port_read(struct mv88e6xxx_chip *chip, int port, int reg,
 			u16 *val);
@@ -289,6 +292,8 @@ int mv88e6095_port_tag_remap(struct mv88e6xxx_chip *chip, int port);
 int mv88e6390_port_tag_remap(struct mv88e6xxx_chip *chip, int port);
 int mv88e6xxx_port_set_egress_mode(struct mv88e6xxx_chip *chip, int port,
 				   enum mv88e6xxx_egress_mode mode);
+int mv88e6065_port_set_frame_mode(struct mv88e6xxx_chip *chip, int port,
+				  enum mv88e6xxx_frame_mode mode);
 int mv88e6085_port_set_frame_mode(struct mv88e6xxx_chip *chip, int port,
 				  enum mv88e6xxx_frame_mode mode);
 int mv88e6351_port_set_frame_mode(struct mv88e6xxx_chip *chip, int port,

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: mv88e6xxx -- DSA support for Marvell 88e6065 switch (and maybe 88e6060?)

[Andrew Lunn]

> Is someone is interested in getting 6060 to work with mv88e6xxx?

Vivien is. 

I also have a set of patches which modernize the driver. That might be
a step towards merging it in.

> @@ -2126,6 +2146,7 @@ static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
>  
>  	/* Upstream ports flood frames with unknown unicast or multicast DA */
>  	flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
> +	flood = 1; /* This is strange, but original driver also sets flood everywhere */

This might be because the driver did not support hardware offload.
What happens if you don't have this?

> +static const struct mv88e6xxx_ops mv88e6065_ops = {
> +	/* MV88E6XXX_FAMILY_6095 */ /* Here */
> +  //.ieee_pri_map = mv88e6085_g1_ieee_pri_map, /* FIXME */
> +  //	.ip_pri_map = mv88e6085_g1_ip_pri_map, /* FIXME */
> +  //	.set_switch_mac = mv88e6xxx_g1_set_switch_mac, /* FIXME */

What you should do is see if the hardware supports these functions. If
it does not, simply leave it out of mv88e6065_ops. If it does have the
functionality, but needs a new implementation, add a mv88e6065_
version. In theory, nearly everything is optional. So you can start
simple and then add features.

	Andrew

Re: mv88e6xxx -- DSA support for Marvell 88e6065 switch (and maybe 88e6060?)

[Vivien Didelot]

Hi Pavel,

On Tue, 29 Jan 2019 23:57:32 +0100, Pavel Machek <pavel@ucw.cz> wrote:

> Ok, so I took a long look at mv88e6xxx... and got it to work.
> 
> Good news is that modifications needed are not too heavy. Most are 
> inlined below. (tag_daddr is still needed. I can send that too). 
> 
> Bad news is that only about half of all the registers are present on
> 6065 (6060 is similar), and I'm not sure how to do that
> cleanly. Anything marked "W" is not present or reserved or different
> in 6065. What would be good markup of registers that are common to all
> and that are newer-generations-only be? Mark everything not present on
> 6065 as MV88E6085_*?
> 
> Is someone is interested in getting 6060 to work with mv88e6xxx?

I am currently trying to request boards with an 88E6060 so that we can
finally get rid of the legacy code used by the mv88e6060 driver.

I think the 6065 has similar registers layout than the 6060. If so, it would
be simpler for you to add the 6065 support into the mv88e6060 driver.

Merging mv88e6060 into mv88e6xxx is the next step I have on my plate after
removing legacy.

Are you using the legacy binding with mv88e6xxx or the new ones? If you're
using the new ones, Andrew may have a patch to convert the mv88e6060 driver
to use the new bindings, that you could apply and test first. Then you can
add support for 88E6065 in the mv88e6060 driver more easily.


Thanks,

	Vivien