linux-ide.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).