Re: [PATCH 1/4] ata: pata_mpc52xx: switch to non-devm request_irq for proper ordering

[Andy Shevchenko]

On Tue, Jun 09, 2026 at 02:17:24PM -0700, Rosen Penev wrote:
> On Tue, Jun 9, 2026 at 1:36 AM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxx> wrote:
> > On Tue, Jun 09, 2026 at 09:20:35AM +0200, Niklas Cassel wrote:
> > > On Mon, Jun 08, 2026 at 02:47:15PM -0700, Rosen Penev wrote:

...

> > > Andy, since you gave a talk about this recently.
> > > Surely there must be a better way than to migrate away from the device
> > > managed APIs.
> >
> > Thank for Cc'ing me.
> >
> > > Do you have some suggestion?
> >
> > This patch either way (are we going to devm fully or rollback to non-devm)
> > is half-baked.
> Note the fixes tag. The purpose is to get this backported.

See what I wrote below.

> > What I may suggest here is to extend this patch to cover the second
> > problematic place (*) and make it as a fix for backporting.

^^^

> > After that one may carefully go and finish converting probe to devm_*().
> >
> > *) https://elixir.bootlin.com/linux/v7.1-rc7/source/drivers/ata/pata_mpc52xx.c#L733
> > The call of irq_of_parse_and_map() should be moved after devm_kzalloc().
> > So, this fix should do what it does + the above.
> Doesn't matter much. task_irq gets requested, not ata_irq.

It matters, it's about memory allocation. It should go after devm_kzalloc().

> > P.S.
> > Someone may tide up this driver a lot, but this is out of the scope of
> > this patch. FWIW, the brief list of potential refactoring patches
> > based on probe (in unsorted order):
> > - use devm_platform_ioremap_resource()

> Done in later patch in this series.

Cool! Any plans for the rest?

> > - moving towards device_property_read_*()
> > - using dev_err_probe()
> > - use temporary variable to hold struct device pointer
> > - use fwnode_irq_get()
> > - apply multipliers from units.h and time.h (like HZ_PER_MHZ)
> > - moving to a new struct dev_pm_ops and pm_sleep_ptr() macro

-- 
With Best Regards,
Andy Shevchenko