Re: [PATCH 1/9 v2] coresight: add CoreSight core layer framework

[Daniel Thompson]

On 02/07/14 20:06, Mathieu Poirier wrote:
>>> +struct dentry *cs_debugfs_parent = NULL;
>>> +
>>> +static int curr_sink = NO_SINK;
>>> +static LIST_HEAD(coresight_orph_conns);
>>> +static LIST_HEAD(coresight_devs);
>>> +static DEFINE_SEMAPHORE(coresight_mutex);
>>
>> Why is coresight_mutex a semaphore?
> 
> Bad naming convention.

Really? I only saw it used like a mutex, in other words I thought it was
incorrectly typed, rather than incorrectly named.


>>> +static void coresight_disable_link(struct coresight_device *csdev)
>>> +{
>>> +     int link_subtype;
>>> +     int refport, inport, outport;
>>> +
>>> +     inport = coresight_find_link_inport(csdev);
>>> +     outport = coresight_find_link_outport(csdev);
>>> +
>>> +     link_subtype = csdev->subtype.link_subtype;
>>> +     if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG)
>>> +             refport = inport;
>>> +     else if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_SPLIT)
>>> +             refport = outport;
>>> +     else
>>> +             refport = 0;
>>
>> I already read these 7 lines once...
> 
> It is really worth spinning off a function to save 5 lines?

Pretty marginal really. If the code here stays as it is I don't care
enough to raise this point a second time.


Daniel.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/