On Wed, Oct 13, 2021 at 06:33:56AM -0500, Bjorn Helgaas wrote:

On Wed, Oct 13, 2021 at 12:26:42PM +0300, Andy Shevchenko wrote:

quoted

On Wed, Oct 13, 2021 at 2:33 AM Bjorn Helgaas [off-list ref] wrote:

quoted

On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:

...

quoted

It's a bit unusual. Other to_*_dev() are not NULL-aware IIRC.

It is a little unusual.  I only found three of 77 that are NULL-aware:

  to_moxtet_driver()
  to_siox_driver()
  to_spi_driver()

It seems worthwhile to me because it makes the patch and the resulting
code significantly cleaner.

I'm not objecting the change, just a remark.

...

quoted

quoted

+       for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
+               if (id->vendor == vendor && id->device == device)

quoted

+                       break;

return true;

quoted

        return id && id->vendor;

return false;

Good cleanup for a follow-up patch, but doesn't seem directly related
to the objective here.

True. Maybe you can bake one while not forgotten?

...

quoted

quoted

+       return drv && drv->resume ?
+                       drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);

One line?

I don't think I touched that line.

Then why they are both in + section?

...

quoted

quoted

+       struct pci_driver *drv = to_pci_driver(dev->dev.driver);
        const struct pci_error_handlers *err_handler =
-                       dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
+                       drv ? drv->err_handler : NULL;

Isn't dev->driver == to_pci_driver(dev->dev.driver)?

Yes, I think so, but not sure what you're getting at here, can you
elaborate?

Getting pointer from another pointer seems waste of resources, why we
can't simply

	struct pci_driver *drv = dev->driver;

?

...

quoted

Stray change? Or is it in a separate patch in your tree?

Could be skipped.  The string now fits on one line so I combined it to
make it more greppable.

This is inconsistency in your changes, in one case you are objecting of
doing something close to the changed lines, in the other you are doing
unrelated change.

-- 
With Best Regards,
Andy Shevchenko