Failure to detect card ins/rem with pd6729

[Russell King]

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.

> -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.

> -
>  static int pd6729_get_socket(struct pcmcia_socket *sock, socket_state_t *stat
> e)
>  {
>  	struct pd6729_socket *socket = container_of(sock, struct pd6729_socket, sock
> 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.

> +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.

> @@ -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.

> -	/* Register the interrupt handler */
> -	if ((ret = request_irq(dev->irq, pd6729_interrupt, SA_SHIRQ, "pd6729", socke
> t))) {

Again, more mailer crappage.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core