Hi Mike,
On Wed, Jul 25, 2012 at 1:49 PM, Mike Frysinger <vap...@gentoo.org> wrote:
> On Tuesday 24 July 2012 16:11:15 Joe Hershberger wrote:
>> --- a/drivers/net/netconsole.c
>> +++ b/drivers/net/netconsole.c
>> @@ -131,8 +131,17 @@ static void nc_send_packet(const char *buf, int len)
>> }
>>
>> if (eth->state != ETH_STATE_ACTIVE) {
>> - if (eth_init(gd->bd) < 0)
>> - return;
>> +#ifdef CONFIG_NETCONSOLE_PERSIST_ETH
>> + if (net_loop_last_protocol != NETCONS) {
>> +#endif
>> + if (eth_init(gd->bd) < 0)
>> + return;
>> +#ifdef CONFIG_NETCONSOLE_PERSIST_ETH
>> + net_loop_last_protocol = NETCONS;
>> + } else {
>> + eth_init_state_only(gd->bd);
>> + }
>> +#endif
>
> seems to me these pre/post clauses should really get refactored someway. the
> ifdef noise is significant here.
I agree.
> so in the header, something like:
>
> static inline int eth_reinit_protocol(int protocol)
> {
> #ifdef CONFIG_NETCONSOLE_PERSIST_ETH
> return protocol != NETCONS;
> #else
> return 1;
> #endif
> }
> static inline void eth_set_last_protocol(int protocol)
> {
> #ifdef CONFIG_NETCONSOLE_PERSIST_ETH
> net_loop_last_protocol = protocol;
> #endif
> }
> #ifdef CONFIG_NETCONSOLE_PERSIST_ETH
> extern enum proto_t net_loop_last_protocol;
> #else
> # define net_loop_last_protocol -1
> #endif
>
> then the source becomes the more manageable:
>
> if (eth_reinit_protocol(net_loop_last_protocol)) {
> if (eth_init(gd->bd) < 0)
> return;
> eth_set_last_protocol(NETCONS);
> } else
> eth_init_state_only(gd->bd);
Much nicer. With static inline it should compile to the same thing.
>> --- a/net/eth.c
>> +++ b/net/eth.c
>>
>> +#ifdef CONFIG_NETCONSOLE_PERSIST_ETH
>> +int eth_init_state_only(bd_t *bis)
>> +{
>> + eth_current->state = ETH_STATE_ACTIVE;
>> +
>> + return 0;
>> +}
>> +
>> +void eth_halt_state_only(void)
>> +{
>> + eth_current->state = ETH_STATE_PASSIVE;
>> +}
>> +#endif
>
> these *really* should be static inlines in the global header. they're so dirt
> simple, the overhead of the function call is probably much higher than the
> single memory store.
I can do that, but I don't think it will save anything. Since
eth_current is static, I would have to change it to eth_get_dev(), and
we're back to a function call. Thoughts?
Thanks,
-Joe
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
