Hi Joe,
> -----Original Message-----
> From: Joe Hershberger [mailto:joe.hershber...@gmail.com]
> Sent: Friday, June 26, 2015 1:41 AM
> To: Ciubotariu Codrin Constantin-B43658
> Cc: u-boot; Joe Hershberger; Sun York-R58495
> Subject: Re: [U-Boot] [PATCH 08/11 v2] drivers/net/vsc9953: Add VLAN commands
> for VSC9953
>
> > @@ -270,6 +270,31 @@ static void vsc9953_port_vlan_pvid_set(int port_no, int
> pvid)
> > field_set(pvid,
> CONFIG_VSC9953_PORT_VLAN_CFG_VID_MASK));
> > }
> >
> > +#ifdef CONFIG_VSC9953_CMD
>
> Why does this need to be defined outside of the #ifdef already at the
> bottom of the file?
I added it here so that vsc9953_port_vlan_pvid_get() could be next to its pair,
vsc9953_port_vlan_pvid_set(). If you think that it should be at the bottom of
the file I can move it.
> > + /* Administrative down */
> > + if ((!vsc9953_l2sw.port[port_nr].enabled)) {
>
> Why do you have double "((" and "))"?
By mistake, I will remove one pair.
> > +#ifdef CONFIG_VSC9953_CMD
>
> Why does this need to be defined outside of the #ifdef already at the
> bottom of the file?
I did this so that similar get/set() functions to be close to one another.
Also, the functions guarded by CONFIG_VSC9953_CMD are used only when the ethsw
commands are enabled (CONFIG_VSC9953_CMD defined). If a user decides to use the
driver for VSC9953, with the switch working in unmanaged state and he doesn't
need the commands to configure the switch, he could compile u-boot with
CONFIG_VSC9953_CMD undefined. In this case, some warnings will appear at
compile time suggesting that functions like vsc9953_port_vlan_egr_untag_get()
are defined but not used.
> > + /* Administrative down */
> > + if ((!vsc9953_l2sw.port[port_no].enabled)) {
>
> Why do you have double "((" and "))"?
By mistake, I will remove one pair.
> > +/* Shiw egress tagging configuration for a VSC9953 port */
>
> Shiw -> Show
Yes, it's a typo.
> > + if (!!(val & (field_set((1 << port_no),
> > + CONFIG_VSC9953_VLAN_PORT_MASK))))
>
> There is no need for "!!" in an if statement. Drop it and the extra
> parenthesis.
Ok.
Thanks and best regards,
Codrin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
