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 <[email protected]>
>> ---
>>
>> 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
> 

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to