[PATCH 04/24] drm/bridge/sii8620: add continuations to messages

[Archit Taneja]

On 01/23/2017 04:03 PM, Andrzej Hajda wrote:
> On 23.01.2017 09:31, Archit Taneja wrote:
>>
>> On 01/20/2017 01:08 PM, Andrzej Hajda wrote:
>>> Due to asynchronous nature of MHL flow of execution is dispersed. Logical
>>> continuation of some actions happens after response of peer, i.e in interrupt
>>> handler. To simplify coding continuation mechanism has been added - it is now
>>> possible to provide continuation callback, which will be called after peer
>>> responds to given action.
>> Could you wrap this to 75 chars?
>
> OK
>
>>
>>> Signed-off-by: Andrzej Hajda <a.hajda at samsung.com>
>>> ---
>>>  drivers/gpu/drm/bridge/sil-sii8620.c | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> index 75867c0..cde0074 100644
>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> @@ -78,12 +78,15 @@ struct sii8620_mt_msg;
>>>  typedef void (*sii8620_mt_msg_cb)(struct sii8620 *ctx,
>>>  				  struct sii8620_mt_msg *msg);
>>>
>>> +typedef void (*sii8620_cb)(struct sii8620 *ctx, int ret);
>>> +
>>>  struct sii8620_mt_msg {
>>>  	struct list_head node;
>>>  	u8 reg[4];
>>>  	u8 ret;
>>>  	sii8620_mt_msg_cb send;
>>>  	sii8620_mt_msg_cb recv;
>>> +	sii8620_cb continuation;
>>>  };
>>>
>>>  static const u8 sii8620_i2c_page[] = {
>>> @@ -258,6 +261,8 @@ static void sii8620_mt_work(struct sii8620 *ctx)
>>>  				       node);
>>>  		if (msg->recv)
>>>  			msg->recv(ctx, msg);
>>> +		if (msg->continuation)
>>> +			msg->continuation(ctx, msg->ret);
>> I was wondering if instead of executing the continuation via a callback,
>> would it make sense to insert them as a new message instead?
>>
>> I don't have the complete context of this, so feel free to ignore the suggestion
>> if doesn't make sense.
>
> My assumption was that continuation should be tied to the message it was
> attached to - ie it should be called after response for the message and
> it should be called with the result of the response.
> If we assume messages are fully serialized (as it is now) it could be
> done as you suggested, but I prefer to leave possibility to de-serialize
> message queue in the future.

Sure. Let's leave it as is.

Archit

>
> Regards
> Andrzej
>
>>
>> Thanks,
>> Archit
>>
>>>  		list_del(&msg->node);
>>>  		kfree(msg);
>>>  	}
>>> @@ -310,6 +315,21 @@ static struct sii8620_mt_msg *sii8620_mt_msg_new(struct sii8620 *ctx)
>>>  	return msg;
>>>  }
>>>
>>> +static void sii8620_mt_set_cont(struct sii8620 *ctx, sii8620_cb cont)
>>> +{
>>> +	struct sii8620_mt_msg *msg;
>>> +
>>> +	if (ctx->error)
>>> +		return;
>>> +
>>> +	if (list_empty(&ctx->mt_queue)) {
>>> +		ctx->error = -EINVAL;
>>> +		return;
>>> +	}
>>> +	msg = list_last_entry(&ctx->mt_queue, struct sii8620_mt_msg, node);
>>> +	msg->continuation = cont;
>>> +}
>>> +
>>>  static void sii8620_mt_msc_cmd(struct sii8620 *ctx, u8 cmd, u8 arg1, u8 arg2)
>>>  {
>>>  	struct sii8620_mt_msg *msg = sii8620_mt_msg_new(ctx);
>>>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project