Kurt Van Dijck wrote:
> On Tue, Feb 09, 2010 at 06:26:02PM +0100, Oliver Hartkopp wrote:
>> Wolfgang Grandegger wrote:
> [...]
>>> Index: 2.6/drivers/net/can/sja1000/sja1000.c
>>> ===================================================================
>>> --- 2.6.orig/drivers/net/can/sja1000/sja1000.c
>>> +++ 2.6/drivers/net/can/sja1000/sja1000.c
>>> @@ -133,9 +133,13 @@ static void set_normal_mode(struct net_d
>>> for (i = 0; i < 100; i++) {
>>> /* check reset bit */
>>> if ((status & MOD_RM) == 0) {
>>> + u8 reg_ier = IRQ_ALL;
>>> +
>>> + if (!(priv->can.ctrlmode & CAN_CTRLMODE_BUS_ERROR))
>>> + reg_ier &= ~IRQ_BEI;
>>> priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>> - /* enable all interrupts */
>>> - priv->write_reg(priv, REG_IER, IRQ_ALL);
>>> + /* enable interrupts */
>>> + priv->write_reg(priv, REG_IER, reg_ier);
>>> return;
>>> }
>>
>> Why did you introduce the extra variable here?
>>
>> priv->can.state = CAN_STATE_ERROR_ACTIVE;
>> /* enable interrupts */
>> if (!(priv->can.ctrlmode & CAN_CTRLMODE_BUS_ERROR))
>> priv->write_reg(priv, REG_IER, (IRQ_ALL & ~IRQ_BEI));
>> else
>> priv->write_reg(priv, REG_IER, IRQ_ALL);
>>
>> Won't this work also?
> Just a question: is stack space an issue over code space?
Don't know.
In this specific case it's just the setting of compile-time calculated values.
I just was not able to see why working on a single byte variable and do
(8-)bit operations - which may be expensive on some platforms - could be the
better approach here ...
Regards,
Oliver
_______________________________________________
Socketcan-users mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-users
