Re: [Patch] make cpsw work on another eval board

[christos%zoulas.com@localhost (Christos Zoulas)]

On Feb 25,  8:22pm, ozaki-r%NetBSD.org@localhost (Ryota Ozaki) wrote:
-- Subject: Re: [Patch] make cpsw work on another eval board

| Thank you for reviewing. The patches on the gist has been updated.
| I think all of your requests are satisfied.

Thanks, looks really good now; few things:

1. line 59, the assignment of has_1000t should be in the if statement. I don't
   like double assigning, because it also prevents the compiler from warning
   about unitialized variables in unintended paths.

2. This:

+               if (sc->sc_phy_has_1000t)
+                       cpsw_write_4(sc, CPSW_SL_MACCONTROL(i),
+                           macctl | SLMACCTL_GIG);
+               else
+                       cpsw_write_4(sc, CPSW_SL_MACCONTROL(i), macctl);

I would write that as:

+               if (sc->sc_phy_has_1000t)
+                       macctl |= SLMACCTL_GIG;
+               cpsw_write_4(sc, CPSW_SL_MACCONTROL(i), macctl);

3. cpsw_ale_entry_mac_match(uint32_t *ale_entry, uint8_t *mac)
   these should be const
   cpsw_ale_entry_set_mac(uint32_t *ale_entry, uint8_t *mac)
   mac should be const

4. I would proably put the contents of the loop in 278 inside a function
        cpsw_ale_set_outgoing_mac(sc, i, mac);

Best,

christos