On 18.05.2017 14:55, Mikko Perttunen wrote:
> On 18.05.2017 14:42, Dmitry Osipenko wrote:
>> On 20.03.2017 21:44, Mikko Perttunen wrote:
>>> ...
>>> struct host1x_channel *host1x_channel_request(struct device *dev)
>>> {
>>> struct host1x *host = dev_get_drvdata(dev->parent);
>>> + struct host1x_channel_list *chlist = &host->channel_list;
>>> unsigned int max_channels = host->info->nb_channels;
>>> struct host1x_channel *channel = NULL;
>>> - unsigned long index;
>>> + unsigned int index;
>>> int err;
>>>
>>> - mutex_lock(&host->chlist_mutex);
>>> + down(&chlist->alloc_sema);
>>> +
>>
>> It's a bit hard to follow lockings in the code, could you please explain why
>> you've added this semaphore? What problem it solves?
>
> Sure. The role of the semaphore is to allow the caller to sleep waiting for a
> free channel, and also to be sure that a channel is available once the down()
> call finishes. Currently the channel allocation model is such that each unit
> allocates a single channel for itself, in which case this is not useful, but
> with later chips with more available channels we can move to a model where a
> channel is allocated for each userspace client. In this case we can allow the
> application to sleep if there are no available channels.
>
> Of course, there is also the possibility of just returning -EBUSY if there are
> no channels - that's probably fine too..
>
Currently, the host1x_channel_request() is only invoked during the drivers probe
routine, it has nothing to do with userspace. I'd suggest to drop that semaphore
from the patch since it's unrelated to the current channel allocation model.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
