Re: [PATCH net-next v2 2/4] can: cc770: add legacy ISA bus driver for the CC770 and AN82527

[Wolfgang Zarre <info@essax.com>]

Hello Wolfgang,

> Hi Wolfgang,
>
> On 12/31/2011 10:39 AM, Wolfgang Zarre wrote:
>> Hello Wolfgang,
>>
>>> Hello Wolfgang,
>>>> Hi Wolfgang,
>>>>
>>>> On 12/21/2011 07:32 PM, Wolfgang Zarre wrote:
>>>>> Hello Wolfgang,
>>>> ...
>>>>
>>>>
>>>> Again, please check if you have netif_start_queue() at the end of the
>>>> open function.
>>>>
>>>
>>> As said I'm using eec921ac28fde243456078a557768808d93d94a3
>>>
>>> However, I'll try further to investigate that issue due the fact
>>> having it
>>> running with my lincan without problems and therefore it should be
>>> possible
>>> to find the problem.
>>>
>>
>> I found the problem which was then at the end quite simple to understand
>> why it
>> get stuck due the fact not receiving an interrupt for TX and due that no
>> reactivation of the queue.
>>
>> I think that maybe also the hacks in the TX functions are obsolete with the
>> fix assuming that the repeated interrupts just happen by indirect access.
>
> OK, to understand you correctly...
>
>> Here my fix which worked for me:
>> --------------------------------------------------------------------------------------------------------
>>
>> diff --git a/drivers/net/can/cc770/cc770.c b/drivers/net/can/cc770/cc770.c
>> index 2d12f89..dad6707 100644
>> --- a/drivers/net/can/cc770/cc770.c
>> +++ b/drivers/net/can/cc770/cc770.c
>> @@ -460,15 +460,6 @@ static netdev_tx_t cc770_start_xmit(struct sk_buff
>> *skb, struct net_device *dev)
>>
>>       stats->tx_bytes += dlc;
>>
>> -
>> -    /*
>> -     * HM: We had some cases of repeated IRQs so make sure the
>> -     * INT is acknowledged I know it's already further up, but
>> -     * doing again fixed the issue
>> -     */
>> -    cc770_write_reg(priv, msgobj[mo].ctrl0,
>> -            MSGVAL_UNC | TXIE_UNC | RXIE_UNC | INTPND_RES);
>> -
>>       return NETDEV_TX_OK;
>>   }
>>
>> @@ -689,12 +680,6 @@ static void cc770_tx_interrupt(struct net_device
>> *dev, unsigned int o)
>>       /* Nothing more to send, switch off interrupts */
>>       cc770_write_reg(priv, msgobj[mo].ctrl0,
>>               MSGVAL_RES | TXIE_RES | RXIE_RES | INTPND_RES);
>> -    /*
>> -     * We had some cases of repeated IRQ so make sure the
>> -     * INT is acknowledged
>> -     */
>> -    cc770_write_reg(priv, msgobj[mo].ctrl0,
>> -            MSGVAL_UNC | TXIE_UNC | RXIE_UNC | INTPND_RES);
>>
>>       stats->tx_packets++;
>>       can_get_echo_skb(dev, 0);
>> diff --git a/drivers/net/can/cc770/cc770_isa.c
>> b/drivers/net/can/cc770/cc770_isa.c
>> index 4be5fe2..48fc128 100644
>> --- a/drivers/net/can/cc770/cc770_isa.c
>> +++ b/drivers/net/can/cc770/cc770_isa.c
>> @@ -148,8 +148,7 @@ static void cc770_isa_port_write_reg_indirect(const
>> struct cc770_priv *priv,
>>   {
>>       unsigned long base = (unsigned long)priv->reg_base;
>>
>> -    outb(reg, base);
>> -    outb(val, base + 1);
>> +    outw( reg + ( val<<  8), base);
>
> That modification does fix your problem, right? The others above don't
> help nor harm but we don't know if it's really realted to the same
> problem. I wll dig a bit deeper.

Exactly. The others above I removed because facing the opposite, even
missing interrupts but then just to avoid other possible side effects
and then assuming that they might be related.


>
>>   }
>>
>>   static int __devinit cc770_isa_probe(struct platform_device *pdev)
>>
>> ---------------------------------------------------------------------------------------------
>>
>>
>>
>> Please let me know if this is OK for You, maybe You can do some tests as
>> well.
>
> My board does not use indirect accesses.
>

Ah Ok, I see, but let me know if I may help with additional tests You would
like to have beside the other mentioned tests as long as I have the hardware.

>> Would continue then with further tests regarding error conditions, however
>> I realised another small issue with dropped packages at reception.
>>
>> As soon as You read the first time from the socket and then You stop
>> reading
>> the packages are not counted as 'dropped' any more which is IMHO not
>> correct
>> because as soon as You stop reading they should be counted as dropped
>> again.
>
> Will have a closer look now...

Thanks a lot.
Also I'll inform You as soon as I can continue with some further tests
regarding bus states and error counters and maybe also with patches in
case I have.

>
> Wolfgang.

Wolfgang