Re: [PATCH v2 6/9] coresight: Store in-connections as well as out-connections

[James Clark <james.clark@arm.com>]

On 14/03/2023 05:35, Jinlong Mao wrote:
> 
> On 3/11/2023 12:06 AM, James Clark wrote:
>> This will allow CATU to get its associated ETR in a generic way where
>> currently the enable path has some hard coded searches which avoid
>> the need to store input connections.
>>
>> Signed-off-by: James Clark <james.clark@arm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-core.c  | 56 +++++++++++++++--
>>   .../hwtracing/coresight/coresight-platform.c  | 61 ++++++++++++++++---
>>   drivers/hwtracing/coresight/coresight-sysfs.c |  1 -
>>   include/linux/coresight.h                     | 25 ++++++++
>>   4 files changed, 130 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>> b/drivers/hwtracing/coresight/coresight-core.c
>> index f457914e445e..a8ba7493c09a 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -59,6 +59,7 @@ const u32 coresight_barrier_pkt[4] = {0x7fffffff,
>> 0x7fffffff, 0x7fffffff, 0x7fff
>>   EXPORT_SYMBOL_GPL(coresight_barrier_pkt);
>>     static const struct cti_assoc_op *cti_assoc_ops;
>> +static int coresight_fixup_inputs(struct coresight_device *csdev);
>>     ssize_t coresight_simple_show_pair(struct device *_dev,
>>                     struct device_attribute *attr, char *buf)
>> @@ -1369,6 +1370,35 @@ static int coresight_fixup_orphan_conns(struct
>> coresight_device *csdev)
>>                csdev, coresight_orphan_match);
>>   }
>>   +/*
>> + * Device connections are discovered before one/both devices have
>> been created,
>> + * so inputs must be added later.
>> + */
>> +static int coresight_fixup_inputs(struct coresight_device *csdev)
>> +{
>> +    int i, ret = 0;
>> +    struct coresight_connection *out_conn;
>> +    struct coresight_connection in_conn;
>> +
>> +    for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>> +        out_conn = &csdev->pdata->out_conns[i];
>> +        if (!out_conn->remote_dev || !out_conn->remote_dev->pdata)
>> +            continue;
> 
> Hi James,
> 
> If out_conn->remote_dev is null here,  the in_conn of
> out_conn->remote_dev->pdata will never be set.
> For example, device A is connected to in_port 0 of device B. If device A
> is probed first, the in_conn of device
> B will not be set.
> Do we need to add Defer probe return here ? I tested with defer probe
> return, it works.
> 
>         for (i = 0; i < csdev->pdata->nr_outconns; i++) {
>                 out_conn = &csdev->pdata->out_conns[i];
>                 if (!out_conn->remote_dev || !out_conn->remote_dev->pdata)
> -                       continue;
> +                        return -EPROBE_DEFER;
> 
> Thanks
> Jinlong Mao

I think you are right but I thought that EPROBE_DEFER was too big of a
change and that it might break something in some unexpected way.

In V3 I used the orphan mechanism for inputs in the same way as outputs
so the problem should be gone now but without having to defer loading.