All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: Nikolay Borisov <nik.borisov@suse.com>
Cc: Damien Le Moal <dlemoal@kernel.org>,
	linux-ide@vger.kernel.org, rosenp@gmail.com
Subject: Re: [PATCH v3] ata: sata_dwc_460ex: Don't enable interrupt if libATA core registration fails
Date: Thu, 17 Sep 2026 16:17:59 +0200	[thread overview]
Message-ID: <aqv2l4pGuk7K375x@ryzen> (raw)
In-Reply-To: <3d8e8d30-1c67-4973-aed6-5fa9737b6492@suse.com>

On Thu, Sep 17, 2026 at 04:26:41PM +0300, Nikolay Borisov wrote:
> 
> 
> On 17.09.26 г. 16:18 ч., Damien Le Moal wrote:
> > On 2026/09/17 18:35, Nikolay Borisov wrote:
> > > Recent 4bbc16a353a9 commit changed the timing when interrupts are
> > > enabled. However it didn't account for the libATA registration step. So
> > > it's possible that interrupts are enabled when the device is not
> > > registered with libATA core.
> > > 
> > > Resolve the issue by ensuring that interrupts are enabled only if the rest of the
> > > initialization is successful.
> > > 
> > > Fixes: 4bbc16a353a9 ("ata: sata_dwc_460ex: enable SATA interrupts only after IRQ handler is registered")
> > > Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
> > > ---
> > > 
> > > 
> > > Changes since v1:
> > > 	* Added proper goto label
> > > Changes since v2:
> > >      * Rework logic so that interrupts are enabled if everything is successful
> > > 	and don't call phy_exit otherwise, as the driver is quite broken w.r.t to it.
> > > 
> > >   drivers/ata/sata_dwc_460ex.c | 8 +++++---
> > >   1 file changed, 5 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> > > index 8e3fc713891a..e74335dec42a 100644
> > > --- a/drivers/ata/sata_dwc_460ex.c
> > > +++ b/drivers/ata/sata_dwc_460ex.c
> > > @@ -1185,11 +1185,13 @@ static int sata_dwc_probe(struct platform_device *ofdev)
> > >   	 * error_handler() to execute a dummy Softreset EH session
> > >   	 */
> > >   	err = ata_host_activate(host, irq, sata_dwc_isr, 0, &sata_dwc_sht);
> > > -	if (err)
> > > +	if (err) {
> > >   		dev_err(dev, "failed to activate host");
> > 
> > Why not simply "goto err_out;" here ?
> 
> That was in V2 however Niklas discovered some issue w.r.t to phy_exit.
> https://lore.kernel.org/all/aqGFjsuIMRIy5xfv@ryzen/

I have since merged a series, which is queued on ibata/linux.git (for-7.4):
https://lore.kernel.org/linux-ide/20260915084127.692494-8-cassel@kernel.org/

See:
https://lore.kernel.org/linux-ide/20260915084127.692494-13-cassel@kernel.org/

Where, if ata_host_activate() failed after ata_host_start() returned success
(which will have called call->port_start()), it will call ata_host_undo_start()
which will call ->port_stop().


Thus, the Sashiko comment on your v2:
https://lore.kernel.org/linux-ide/20260909113821.A72011F00A3A@smtp.kernel.org/

is no longer valid.


Please send your V2 again (as a V4) just so that we can get Sashiko to review
it again.

Preferably use:
$ git format-patch --base libata/for-7.4

So that it adds the proper base commit that you depend on.


Kind regards,
Niklas

      reply	other threads:[~2026-09-17 14:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17 11:35 [PATCH v3] ata: sata_dwc_460ex: Don't enable interrupt if libATA core registration fails Nikolay Borisov
2026-09-17 13:18 ` Damien Le Moal
2026-09-17 13:26   ` Nikolay Borisov
2026-09-17 14:17     ` Niklas Cassel [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=aqv2l4pGuk7K375x@ryzen \
    --to=cassel@kernel.org \
    --cc=dlemoal@kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=nik.borisov@suse.com \
    --cc=rosenp@gmail.com \
    /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.