On Thu, Dec 23, 2021 at 11:02:54AM +0800, Lu Baolu wrote:
> Hi Greg,
>
> On 12/22/21 8:47 PM, Greg Kroah-Hartman wrote:
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void device_dma_cleanup(struct device *dev, struct device_driver *drv)
> > > +{
> > > + if (!dev->bus->dma_configure)
> > > + return;
> > > +
> > > + if (!drv->suppress_auto_claim_dma_owner)
> > > + iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API);
> > > +}
> > > +
> > > static int really_probe(struct device *dev, struct device_driver *drv)
> > > {
> > > bool test_remove = IS_ENABLED(CONFIG_DEBUG_TEST_DRIVER_REMOVE) &&
> > > @@ -574,11 +601,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
> > > if (ret)
> > > goto pinctrl_bind_failed;
> > > - if (dev->bus->dma_configure) {
> > > - ret = dev->bus->dma_configure(dev);
> > > - if (ret)
> > > - goto probe_failed;
> > > - }
> > > + if (device_dma_configure(dev, drv))
> > > + goto pinctrl_bind_failed;
> > Are you sure you are jumping to the proper error path here? It is not
> > obvious why you changed this.
>
> The error handling path in really_probe() seems a bit wrong. For
> example,
>
> 572 /* If using pinctrl, bind pins now before probing */
> 573 ret = pinctrl_bind_pins(dev);
> 574 if (ret)
> 575 goto pinctrl_bind_failed;
>
> [...]
>
> 663 pinctrl_bind_failed:
> 664 device_links_no_driver(dev);
> 665 devres_release_all(dev);
> 666 arch_teardown_dma_ops(dev);
> 667 kfree(dev->dma_range_map);
> 668 dev->dma_range_map = NULL;
> 669 driver_sysfs_remove(dev);
> ^^^^^^^^^^^^^^^^^^^^^^^^^
> 670 dev->driver = NULL;
> 671 dev_set_drvdata(dev, NULL);
> 672 if (dev->pm_domain && dev->pm_domain->dismiss)
> 673 dev->pm_domain->dismiss(dev);
> 674 pm_runtime_reinit(dev);
> 675 dev_pm_set_driver_flags(dev, 0);
> 676 done:
> 677 return ret;
>
> The driver_sysfs_remove() will be called even driver_sysfs_add() hasn't
> been called yet. I can fix this in a separated patch if I didn't miss
> anything.
If this is a bug in the existing kernel, please submit it as a separate
patch so that it can be properly backported to all affected kernels.
Never bury it in an unrelated change that will never get sent to older
kernels.
greg k-h
