| Message ID | 20220302080132.20946-1-quic_jinlmao@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | coresight: core: Fix coresight device probe failure issue | expand |
On 02/03/2022 08:01, Mao Jinlong wrote: > It is possibe that probe failure issue happens when the device > and its child_device's probe happens at the same time. > In coresight_make_links, has_conns_grp is true for parent, but > has_conns_grp is false for child device as has_conns_grp is set > to true in coresight_create_conns_sysfs_group. The probe of parent > device will fail at this condition. Add has_conns_grp check for > child device before make the links and make the process from > device_register to connection_create be atomic to avoid this > probe failure issue. > > Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com> > Suggested-by: Mike Leach <mike.leach@linaro.org> > Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> > --- > drivers/hwtracing/coresight/coresight-core.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index 88653d1c06a4..7ce78dddfe31 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -1382,7 +1382,7 @@ static int coresight_fixup_device_conns(struct coresight_device *csdev) > continue; > conn->child_dev = > coresight_find_csdev_by_fwnode(conn->child_fwnode); > - if (conn->child_dev) { > + if (conn->child_dev && conn->child_dev->has_conns_grp) { > ret = coresight_make_links(csdev, conn, > conn->child_dev); > if (ret) > @@ -1619,6 +1619,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > csdev->dev.fwnode = fwnode_handle_get(dev_fwnode(desc->dev)); > dev_set_name(&csdev->dev, "%s", desc->name); > > + mutex_lock(&coresight_mutex); > ret = device_register(&csdev->dev); > if (ret) { > put_device(&csdev->dev); > @@ -1645,8 +1646,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > } > } > > - mutex_lock(&coresight_mutex); > - > ret = coresight_create_conns_sysfs_group(csdev); > if (!ret) > ret = coresight_fixup_device_conns(csdev); > @@ -1666,6 +1665,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > err_free_csdev: > kfree(csdev); > err_out: > + mutex_unlock(&coresight_mutex); This appears to be wrong. We may do an unlock when we didn't lock it in the first place. Please could you double check. Also, I think it may be neater to move the kfree(csdev) to the only user and return straight away from there. Cheers Suzuki > /* Cleanup the connection information */ > coresight_release_platform_data(NULL, desc->pdata); > return ERR_PTR(ret);
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 88653d1c06a4..7ce78dddfe31 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1382,7 +1382,7 @@ static int coresight_fixup_device_conns(struct coresight_device *csdev) continue; conn->child_dev = coresight_find_csdev_by_fwnode(conn->child_fwnode); - if (conn->child_dev) { + if (conn->child_dev && conn->child_dev->has_conns_grp) { ret = coresight_make_links(csdev, conn, conn->child_dev); if (ret) @@ -1619,6 +1619,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) csdev->dev.fwnode = fwnode_handle_get(dev_fwnode(desc->dev)); dev_set_name(&csdev->dev, "%s", desc->name); + mutex_lock(&coresight_mutex); ret = device_register(&csdev->dev); if (ret) { put_device(&csdev->dev); @@ -1645,8 +1646,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) } } - mutex_lock(&coresight_mutex); - ret = coresight_create_conns_sysfs_group(csdev); if (!ret) ret = coresight_fixup_device_conns(csdev); @@ -1666,6 +1665,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) err_free_csdev: kfree(csdev); err_out: + mutex_unlock(&coresight_mutex); /* Cleanup the connection information */ coresight_release_platform_data(NULL, desc->pdata); return ERR_PTR(ret);
It is possibe that probe failure issue happens when the device and its child_device's probe happens at the same time. In coresight_make_links, has_conns_grp is true for parent, but has_conns_grp is false for child device as has_conns_grp is set to true in coresight_create_conns_sysfs_group. The probe of parent device will fail at this condition. Add has_conns_grp check for child device before make the links and make the process from device_register to connection_create be atomic to avoid this probe failure issue. Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com> Suggested-by: Mike Leach <mike.leach@linaro.org> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> --- drivers/hwtracing/coresight/coresight-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)