Re: [PATCH v5 07/14] coresight: cti: Add device tree support for custom CTI.

Suzuki Kuruppassery Poulose
2019-11-29T14:16:41Z

On 25/11/2019 21:22, Mathieu Poirier wrote:
> On Tue, Nov 19, 2019 at 11:19:05PM +0000, Mike Leach wrote:
>> Adds support for CTIs whose connections are implementation defined at
>> hardware design time, and not constrained by v8 architecture.
>>
>> These CTIs have no standard connection setup, all the settings have to
>> be defined in the device tree files. The patch creates a set of connections
>> and trigger signals based on the information provided.
>>
>> Signed-off-by: Mike Leach <mike.leach@linaro.org>
>> ---
>> .../coresight/coresight-cti-platform.c | 250 +++++++++++++++++-
>> .../hwtracing/coresight/coresight-cti-sysfs.c | 11 +
>> 2 files changed, 257 insertions(+), 4 deletions(-)

>> +static int cti_plat_create_impdef_connections(struct device *dev,
>> + struct cti_drvdata *drvdata)
>> +{
>> + int rc = 0;
>> + struct fwnode_handle *fwnode = dev_fwnode(dev);
>> + struct fwnode_handle *child = NULL;
>> +
>> + if (IS_ERR_OR_NULL(fwnode))
>> + return -EINVAL;
>> +
>> + fwnode_for_each_child_node(fwnode, child) {
>> + if (cti_plat_node_name_eq(child, CTI_DT_CONNS))
>> + rc = cti_plat_create_connection(dev, drvdata, child);
>> + if (rc != 0)
>> + break;
>> + }
>> + fwnode_handle_put(child);
>
> As far as I can tell we don't need to call fwnode_handle_put()?

Actually we do, if we break the scan in between, at least for of_nodes.
I had to literally look it down all the way down to confirm this.

So for CONFIG_OF it ends up in of_get_next_available_child(), which
drops the ref on "prev" and grabs the "next". So in case we break
the loop, we must drop the ref on the child. Otherwise, the last
result would be NULL, which would have dropped the ref on the "last"
valid entry. And this is harmless with a NULL ptr.

Suzuki