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

[Ryota Ozaki <ozaki-r%NetBSD.org@localhost>]

On Wed, Feb 26, 2014 at 1:18 AM, Christos Zoulas 
<christos%zoulas.com@localhost> wrote:
> 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);

Christos, thank you for reviewing again!

I've updated the patches to meet your requests.

Best regards,
  ozaki-r

>
> Best,
>
> christos