On 14.05.2010 15:44, Marc Kleine-Budde wrote:
> Oliver Hartkopp wrote:
>>
>> It took a while to create a minimum invasive patch that doesn't touch all the
>> drivers, but here it is:
>>
>> Index: sja1000/sja1000.c
>> ===================================================================
>> --- sja1000/sja1000.c (Revision 1177)
>> +++ sja1000/sja1000.c (Arbeitskopie)
>> @@ -88,6 +88,29 @@
>> .brp_inc = 1,
>> };
>>
>> +
>> +static void sja1000_write_cmd_reg(struct sja1000_priv *priv, u8 val)
>> +{
>> + /* the command register needs some locking in SMP systems */
>
> nitpick:
> from my school english point of view: on SMP systems sounds better
>
Indeed :-)
>> +
>> +#ifdef CONFIG_SMP
>> +
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&priv->wr_reg_lock, flags);
>> + priv->write_reg(priv, REG_CMR, val);
>> + priv->read_reg(priv, REG_SR);
>> + spin_unlock_irqrestore(&priv->wr_reg_lock, flags);
>> +
>> +#else
>> +
>> + /* write to the command register without locking */
>> + priv->write_reg(priv, REG_CMR, val);
>> +
>> +#endif
>> +}
>> +
>> +
>> static int sja1000_probe_chip(struct net_device *dev)
>> {
>> struct sja1000_priv *priv = netdev_priv(dev);
>> @@ -305,7 +328,7 @@
>>
>> can_put_echo_skb(skb, dev, 0);
>>
>> - priv->write_reg(priv, REG_CMR, CMD_TR);
>> + sja1000_write_cmd_reg(priv, CMD_TR);
>>
>> return NETDEV_TX_OK;
>> }
>> @@ -358,7 +381,7 @@
>> cf->can_id = id;
>>
>> /* release receive buffer */
>> - priv->write_reg(priv, REG_CMR, CMD_RRB);
>> + sja1000_write_cmd_reg(priv, CMD_RRB);
>>
>> netif_rx(skb);
>>
>> @@ -393,7 +416,7 @@
>> cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
>> stats->rx_over_errors++;
>> stats->rx_errors++;
>> - priv->write_reg(priv, REG_CMR, CMD_CDO); /* clear bit */
>> + sja1000_write_cmd_reg(priv, CMD_CDO); /* clear bit */
>> }
>>
>> if (isrc & IRQ_EI) {
>> Index: sja1000/sja1000.h
>> ===================================================================
>> --- sja1000/sja1000.h (Revision 1177)
>> +++ sja1000/sja1000.h (Arbeitskopie)
>> @@ -168,6 +168,7 @@
>> void __iomem *reg_base; /* ioremap'ed address to registers */
>> unsigned long irq_flags; /* for request_irq() */
>>
>> + spinlock_t wr_reg_lock; /* lock for concurrent register writes */
>
> hmm...do we want to wrap this is ifdef CONFIG_SMP?
Would be ok for me.
>
>> u16 flags; /* custom mode flags */
>> u8 ocr; /* output control register */
>> u8 cdr; /* clock divider register */
>>
>>
>> Any Acked-by or Signed-off-by out there?
Any other feedback?
Regards,
Oliver
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
