On 01/09/16 18:06, Will Deacon wrote: > Hi Robin, > > On Tue, Aug 23, 2016 at 08:05:16PM +0100, Robin Murphy wrote: >> Now that we can properly describe the mapping between PCI RIDs and >> stream IDs via "iommu-map", and have it fed it to the driver >> automatically via of_xlate(), rework the SMMUv3 driver to benefit from >> that, and get rid of the current misuse of the "iommus" binding. >> >> Since having of_xlate wired up means that masters will now be given the >> appropriate DMA ops, we also need to make sure that default domains work >> properly. This necessitates dispensing with the "whole group at a time" >> notion for attaching to a domain, as devices which share a group get >> attached to the group's default domain one by one as they are initially >> probed. >> >> Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx> >> --- >> >> v5: Simplify init code, use firmware-agnostic (and more efficient) >> driver-based instance lookup > > I'm largely happy with this, just one question below... > >> @@ -2649,7 +2602,14 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev) >> platform_set_drvdata(pdev, smmu); >> >> /* Reset the device */ >> - return arm_smmu_device_reset(smmu); >> + ret = arm_smmu_device_reset(smmu); >> + if (ret) >> + return ret; > > ... if we fail the probe at this point, the drvdata remains set. Do you > need to clear it, or we can we guarantee that nobody is going to try > arm_smmu_get_by_node with the (failed) SMMU's device node? The device only gets added to the driver's list by driver_bound(), and really_probe() will bail before it calls that if we return nonzero from the probe function here. Since get_by_node() is thus safe, and .remove() shouldn't be called given that .probe() failed, I can't see a legitimate situation in which leaving behind a stale pointer in the drvdata of an unbound device might be problematic. Robin. > > Alternatively, we could postpone setting the drvdata until the very end > of probe. > > Will > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
