Failure to detect card ins/rem with pd6729

[Komuro]

>On Sun, Sep 26, 2004 at 08:20:28AM +0900, Komuro wrote:
>> + *  poll_interval=n
>> + *     Sets the card status polling delay, in 1/1000 second increments.
>> + *     Polling only affects detection of card insert and eject events.
>> + *     The default is 1000.
>> + *     (poll_interval parameter is not used, if irq_mode = 1)
>
>Please don't do this.  HZ isn't guaranteed to be 1000, so poll_interval
>units are rather undefined.  Use something real (msec) and then convert
>it to jiffies-based units, rounding up as necessary.

I removed the poll_interval parameter like yenta_socket.c.


>> -static void set_bridge_state(struct pd6729_socket *socket)
>> +static void pd6729_interrupt_wrapper(unsigned long data)
>>  {
>> -	indirect_write(socket, I365_GBLCTL, 0x00);
>> -	indirect_write(socket, I365_GENCTL, 0x00);
>> +	struct pd6729_socket *socket = (struct pd6729_socket *) data;
>>  
>> -	indirect_setbit(socket, I365_INTCTL, 0x08);
>> +	pd6729_interrupt(0, (void *)socket, NULL);
>> +	socket->poll_timer.expires = jiffies + poll_interval;
>> +	add_timer(&socket->poll_timer);
>
>Use mod_timer() - its more efficient.

OK. I fixed this.


>> -
>>  static int pd6729_get_socket(struct pcmcia_socket *sock, socket_state_t *s
tat
>> e)
>>  {
>>  	struct pd6729_socket *socket = container_of(sock, struct pd6729_socket, s
ock
>> et);
>
>You're using that broken mailer that screws patches again.  Just a
>warning; I won't accept a patch from you sent like this when it's
>"perfect".
>
>The way around this, of course, is to ensure that your code wraps before
>column 80 as per the coding style recommendations.

Sorry. I attached the patch.


>> +static irqreturn_t pd6729_test(int irq, void *dev, struct pt_regs *regs)
>> +{
>> +	dprintk("-> hit on irq %d\n", irq);
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int pd6729_check_irq(int irq, int flags)
>> +{
>> +	if (request_irq(irq, pd6729_test, flags, "x", pd6729_test) != 0)
>> +		return -1;
>> +	free_irq(irq, pd6729_test);
>> +	return 0;
>> +}
>
>This is actually a meaningless test.  Whether an IRQ is available
>or not depends what device drivers you have loaded, and drivers
>loaded after this will cause the data gathered by this to become
>invalid.
>
>You're not even checking to see if the IRQ actually works, so in
>whole this test achieves almost nothing.

Actually, the PD6729 chipset lacks the functionality of
detect whether interrupt can be used.
The PD6729 can not generate a dummy interrupt by software.


>> @@ -640,11 +721,20 @@
>>  		goto err_out_disable;
>>  	}
>>  
>> +	if (dev->irq == 0)
>> +		irq_mode = 0;	/* fall back to ISA interrupt mode */
>
>IRQ 0 is not guaranteed to be "no irq" available; you should use a
>defined constant like NO_IRQ - some architectures define this constant
>for this purpose.

I fixed this.


>> -	/* Register the interrupt handler */
>> -	if ((ret = request_irq(dev->irq, pd6729_interrupt, SA_SHIRQ, "pd6729", so
cke
>> t))) {
>
>Again, more mailer crappage.
>

Best Regards
Komuro
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pd6729.test5.diff
Type: application/octet-stream
Size: 10046 bytes
Desc: not available
Url : http://lists.infradead.org/pipermail/linux-pcmcia/attachments/20040926/dda7b78f/pd6729.test5-0001.obj