Hi Codrin,
On Tue, Jun 30, 2015 at 9:02 AM, Codrin Constantin Ciubotariu
<codrin.ciubota...@freescale.com> wrote:
> 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.
OK, I think that makes sense the way you had it. Also, it wouldn't
make sense to move out of drivers/net/vsc9953.c, so leave it here.
>> > + /* 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.
Sounds good. You can leave this here.
Thanks,
-Joe
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
