All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] ide-pmac: media-bay support fixes
Date: Sun, 06 Jul 2008 08:25:29 +1000	[thread overview]
Message-ID: <1215296729.8970.17.camel@pasglop> (raw)
In-Reply-To: <200807051756.26506.bzolnier@gmail.com>

> In init_irq() we unmask IRQs just before registering IRQ handler but we
> we don't clear pending IRQs before the unmask (simply reading the Status
> register should be enough).
> 
> [ Previously ide_port_wait_ready() would do it during ide_device_add()
>   call and before the IRQ handler is registered but now it will be skipped
>   because of ->noprobe being set. ]

I'm pretty sure I added some reads of the status reg before enable_irq()
and that didn't fix it but I may have fubar'ed. I'll dbl check.

> Arrghhh, this was actually caused by a brain glitch on my side...
> 
> While preparing this patch I was under the impression that ->init_dev can
> be called only by ide through ide_device_add(), which is of course untrue
> since it can be called by mediabay through ide_port_scan()...
> 
> However when I think deeper about it I recall that I first implemented
> it as ->init_hwif (for which the assumption was true) and later converted
> the patch to ->init_dev because I noticed the assumption and realized
> that it needs to be ->init_dev to make it work for warm-plug...
> 
> Scary... :)

Yup. Later, I though about ways not to add a new state but the
patch was done, so let's go with it for now.

> This is a kind of tangential issue to pmac stuff.
> 
> Could you resend it as a separate patch with your S-o-b: line?

Oh, that's just debug stuff for me to track down the bug. Do you want
to merge it ? I told you I just send whatever hacks I did to get it
going so far, I'll clean things up when I have the irq stuff solved.

> >  	/*
> >  	 * We must always disable IRQ, as probe_for_drive will assert IRQ, but
> >  	 * we'll install our IRQ driver much later...
> > @@ -798,6 +802,7 @@ static int ide_probe_port(ide_hwif_t *hw
> >  		(void) probe_for_drive(drive);
> >  		if (drive->present)
> >  			rc = 0;
> > +		ide_busy_sleep(hwif);
> 
> I don't quite get this chunk.
> 
> Is it a workaround for interrupt storm problem?

Yup, tho didn't work.

> [...]
> 
> I integrated the rest in a verbatim form with pmac patches
> (two of them got 'take 4' as a result, the other two remain unchanged):
> 
> pmac-media-bay-support-fixes-take-4.patch
> pmac-store-pmif-instead-of-hwif-in-driver_data-take-2.patch
> pmac-add-init_dev-method-take-4.patch
> pmac-move-ide_find_port-call-to-pmac_ide_setup_device-take-2.patch
> 
> [ in the usual place ]

Ok. Will look at it on monday (ie. tomorrow for me).

Cheers,
Ben.


      reply	other threads:[~2008-07-05 22:25 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-16 19:24 [PATCH 1/4] ide-pmac: media-bay support fixes Bartlomiej Zolnierkiewicz
2008-06-17  3:39 ` Benjamin Herrenschmidt
2008-06-17  3:49   ` Benjamin Herrenschmidt
2008-06-17  9:41     ` Bartlomiej Zolnierkiewicz
2008-06-17  9:58       ` Bartlomiej Zolnierkiewicz
2008-06-23  5:35       ` Benjamin Herrenschmidt
2008-06-23  5:54         ` Benjamin Herrenschmidt
2008-06-23  6:41           ` Benjamin Herrenschmidt
2008-06-23 10:47             ` Benjamin Herrenschmidt
2008-06-23 21:45               ` Bartlomiej Zolnierkiewicz
2008-06-24 10:33                 ` Benjamin Herrenschmidt
2008-06-23 21:00             ` Bartlomiej Zolnierkiewicz
2008-06-24 10:34               ` Benjamin Herrenschmidt
2008-06-24 18:51                 ` Bartlomiej Zolnierkiewicz
2008-06-24 18:55                   ` Bartlomiej Zolnierkiewicz
2008-06-26  4:54                     ` Benjamin Herrenschmidt
2008-06-26  8:51                       ` Bartlomiej Zolnierkiewicz
2008-06-26  9:01                         ` Benjamin Herrenschmidt
2008-06-26  9:40                           ` Bartlomiej Zolnierkiewicz
2008-07-03  5:33                             ` Benjamin Herrenschmidt
2008-07-03  6:47                               ` Benjamin Herrenschmidt
2008-07-03  7:33                                 ` Benjamin Herrenschmidt
2008-07-05 15:56                                   ` Bartlomiej Zolnierkiewicz
2008-07-05 22:25                                     ` Benjamin Herrenschmidt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1215296729.8970.17.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=bzolnier@gmail.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.