Re: [PATCH v4 02/13] driver core: Set DMA ownership during driver bind/unbind

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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