Re: Problem with Eurotech COM1273 Dual Channel CAN PC104 Module.

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 4018 bytes --]

On 04/15/2014 03:19 PM, Oliver Hartkopp wrote:
> Something like this?

Yes, no need to move the "base = pric->reg_base" under the lock though.

Marc

> It uses the dev->dev_id which was missed in the 3e66d0138c05d9792f patch before.
> 
> Just compile-tested by now ...
> 
> diff --git a/drivers/net/can/sja1000/sja1000_isa.c b/drivers/net/can/sja1000/sja1000_isa.c
> index df136a2..46e1784 100644
> --- a/drivers/net/can/sja1000/sja1000_isa.c
> +++ b/drivers/net/can/sja1000/sja1000_isa.c
> @@ -46,6 +46,7 @@ static int clk[MAXDEV];
>  static unsigned char cdr[MAXDEV] = {[0 ... (MAXDEV - 1)] = 0xff};
>  static unsigned char ocr[MAXDEV] = {[0 ... (MAXDEV - 1)] = 0xff};
>  static int indirect[MAXDEV] = {[0 ... (MAXDEV - 1)] = -1};
> +static spinlock_t indirect_lock[MAXDEV];  /* lock for indirect access mode */
>  
>  module_param_array(port, ulong, NULL, S_IRUGO);
>  MODULE_PARM_DESC(port, "I/O port number");
> @@ -101,19 +102,28 @@ static void sja1000_isa_port_write_reg(const struct sja1000_priv *priv,
>  static u8 sja1000_isa_port_read_reg_indirect(const struct sja1000_priv *priv,
>  					     int reg)
>  {
> -	unsigned long base = (unsigned long)priv->reg_base;
> +	unsigned long base, flags;
> +	u8 readval;
>  
> +	spin_lock_irqsave(&indirect_lock[priv->dev->dev_id], flags);
> +	base = (unsigned long)priv->reg_base;
>  	outb(reg, base);
> -	return inb(base + 1);
> +	readval = inb(base + 1);
> +	spin_unlock_irqrestore(&indirect_lock[priv->dev->dev_id], flags);
> +
> +	return readval;
>  }
>  
>  static void sja1000_isa_port_write_reg_indirect(const struct sja1000_priv *priv,
>  						int reg, u8 val)
>  {
> -	unsigned long base = (unsigned long)priv->reg_base;
> +	unsigned long base, flags;
>  
> +	spin_lock_irqsave(&indirect_lock[priv->dev->dev_id], flags);
> +	base = (unsigned long)priv->reg_base;
>  	outb(reg, base);
>  	outb(val, base + 1);
> +	spin_unlock_irqrestore(&indirect_lock[priv->dev->dev_id], flags);
>  }
>  
>  static int sja1000_isa_probe(struct platform_device *pdev)
> @@ -169,6 +179,7 @@ static int sja1000_isa_probe(struct platform_device *pdev)
>  		if (iosize == SJA1000_IOSIZE_INDIRECT) {
>  			priv->read_reg = sja1000_isa_port_read_reg_indirect;
>  			priv->write_reg = sja1000_isa_port_write_reg_indirect;
> +			spin_lock_init(&indirect_lock[idx]);
>  		} else {
>  			priv->read_reg = sja1000_isa_port_read_reg;
>  			priv->write_reg = sja1000_isa_port_write_reg;
> @@ -198,6 +209,7 @@ static int sja1000_isa_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, dev);
>  	SET_NETDEV_DEV(dev, &pdev->dev);
> +	dev->dev_id = idx;
>  
>  	err = register_sja1000dev(dev);
>  	if (err) {
> 
> 
> 
> On 15.04.2014 14:33, Marc Kleine-Budde wrote:
>> On 04/15/2014 02:00 PM, Oliver Hartkopp wrote:
>>> As I wrote in the other thread:
>>>
>>>> The command i am executing is 
>>>> modprobe sja1000_isa port=0x200,0x204 irq=10,11 indirect=1 
>>>
>>> The parameters for each interface are given separately
>>
>> The indirect mode is racy, btw....
>>
>>> static u8 sja1000_isa_port_read_reg_indirect(const struct sja1000_priv *priv,
>>>                                              int reg)
>>> {
>>>         unsigned long base = (unsigned long)priv->reg_base;
>>>
>>>         outb(reg, base);
>>>         return inb(base + 1);
>>> }
>>>
>>> static void sja1000_isa_port_write_reg_indirect(const struct sja1000_priv *priv,
>>>                                                 int reg, u8 val)
>>> {
>>>         unsigned long base = (unsigned long)priv->reg_base;
>>>
>>>         outb(reg, base);
>>>         outb(val, base + 1);
>>> }
>>
>> We need locks.
>>
>> Marc
>>
> 


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]