A dl callback can be received anytime after mhi_prepare_for_transfer
has been called. There is a window where the callback may happen
before the probe initializes the qrtr_mhi_dev state. Move the
mhi_prepare_for_transfer call after the registering the endpoint.
Once moved, the reverse can happen where qrtr will try to send a packet
before the channels are prepared. Add a wait in the sending path to
ensure the channels are prepared before trying to do a ul transfer.
Fixes: a2e2cc0dbb11 ("net: qrtr: Start MHI channels during init")
Signed-off-by: Bhaumik Bhatt <[email protected]>
---
net/qrtr/mhi.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
index 29b4fa3..22b0395 100644
--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c
@@ -15,6 +15,7 @@ struct qrtr_mhi_dev {
struct qrtr_endpoint ep;
struct mhi_device *mhi_dev;
struct device *dev;
+ struct completion ready;
};
/* From MHI to QRTR */
@@ -50,6 +51,10 @@ static int qcom_mhi_qrtr_send(struct qrtr_endpoint *ep, struct sk_buff *skb)
struct qrtr_mhi_dev *qdev = container_of(ep, struct qrtr_mhi_dev, ep);
int rc;
+ rc = wait_for_completion_interruptible(&qdev->ready);
+ if (rc)
+ goto free_skb;
+
if (skb->sk)
sock_hold(skb->sk);
@@ -78,11 +83,6 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
struct qrtr_mhi_dev *qdev;
int rc;
- /* start channels */
- rc = mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS);
- if (rc)
- return rc;
-
qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
if (!qdev)
return -ENOMEM;
@@ -90,12 +90,22 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
qdev->mhi_dev = mhi_dev;
qdev->dev = &mhi_dev->dev;
qdev->ep.xmit = qcom_mhi_qrtr_send;
+ init_completion(&qdev->ready);
dev_set_drvdata(&mhi_dev->dev, qdev);
rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
if (rc)
return rc;
+ /* start channels */
+ rc = mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS);
+ if (rc) {
+ qrtr_endpoint_unregister(&qdev->ep);
+ dev_set_drvdata(&mhi_dev->dev, NULL);
+ return rc;
+ }
+
+ complete_all(&qdev->ready);
dev_dbg(qdev->dev, "Qualcomm MHI QRTR driver probed\n");
return 0;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
On 2021-07-21 10:52 AM, [email protected] wrote:
> On 2021-07-20 18:42, Bhaumik Bhatt wrote:
>> A dl callback can be received anytime after mhi_prepare_for_transfer
>> has been called. There is a window where the callback may happen
>> before the probe initializes the qrtr_mhi_dev state. Move the
>> mhi_prepare_for_transfer call after the registering the endpoint.
>>
>> Once moved, the reverse can happen where qrtr will try to send a
>> packet
>> before the channels are prepared. Add a wait in the sending path to
>> ensure the channels are prepared before trying to do a ul transfer.
>>
>> Fixes: a2e2cc0dbb11 ("net: qrtr: Start MHI channels during init")
>> Signed-off-by: Bhaumik Bhatt <[email protected]>
>> ---
>> net/qrtr/mhi.c | 20 +++++++++++++++-----
>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
>> index 29b4fa3..22b0395 100644
>> --- a/net/qrtr/mhi.c
>> +++ b/net/qrtr/mhi.c
>> @@ -15,6 +15,7 @@ struct qrtr_mhi_dev {
>> struct qrtr_endpoint ep;
>> struct mhi_device *mhi_dev;
>> struct device *dev;
>> + struct completion ready;
>> };
>>
>> /* From MHI to QRTR */
>> @@ -50,6 +51,10 @@ static int qcom_mhi_qrtr_send(struct qrtr_endpoint
>> *ep, struct sk_buff *skb)
>> struct qrtr_mhi_dev *qdev = container_of(ep, struct qrtr_mhi_dev,
>> ep);
>> int rc;
>>
>> + rc = wait_for_completion_interruptible(&qdev->ready);
>> + if (rc)
>> + goto free_skb;
>> +
>> if (skb->sk)
>> sock_hold(skb->sk);
>>
>> @@ -78,11 +83,6 @@ static int qcom_mhi_qrtr_probe(struct mhi_device
>> *mhi_dev,
>> struct qrtr_mhi_dev *qdev;
>> int rc;
>>
>> - /* start channels */
>> - rc = mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS);
>> - if (rc)
>> - return rc;
>> -
>> qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
>> if (!qdev)
>> return -ENOMEM;
> would it be good to init completion variable here (call
> init_completion) ?
You mean just before setting qdev->mhi_dev? I don't see why that would
make a difference
mainly because the qcom_mhi_qrtr_send() will only happen after endpoint
is
registered and DL xfer cb will also only come in after we have prepared
the
channels and completed ready with dev_data already set.
>> @@ -90,12 +90,22 @@ static int qcom_mhi_qrtr_probe(struct mhi_device
>> *mhi_dev,
>> qdev->mhi_dev = mhi_dev;
>> qdev->dev = &mhi_dev->dev;
>> qdev->ep.xmit = qcom_mhi_qrtr_send;
>> + init_completion(&qdev->ready);
>>
>
>>
>> return 0;
Thanks,
Bhaumik
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
On 2021-07-20 18:42, Bhaumik Bhatt wrote:
> A dl callback can be received anytime after mhi_prepare_for_transfer
> has been called. There is a window where the callback may happen
> before the probe initializes the qrtr_mhi_dev state. Move the
> mhi_prepare_for_transfer call after the registering the endpoint.
>
> Once moved, the reverse can happen where qrtr will try to send a packet
> before the channels are prepared. Add a wait in the sending path to
> ensure the channels are prepared before trying to do a ul transfer.
>
> Fixes: a2e2cc0dbb11 ("net: qrtr: Start MHI channels during init")
> Signed-off-by: Bhaumik Bhatt <[email protected]>
> ---
> net/qrtr/mhi.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
> index 29b4fa3..22b0395 100644
> --- a/net/qrtr/mhi.c
> +++ b/net/qrtr/mhi.c
> @@ -15,6 +15,7 @@ struct qrtr_mhi_dev {
> struct qrtr_endpoint ep;
> struct mhi_device *mhi_dev;
> struct device *dev;
> + struct completion ready;
> };
>
> /* From MHI to QRTR */
> @@ -50,6 +51,10 @@ static int qcom_mhi_qrtr_send(struct qrtr_endpoint
> *ep, struct sk_buff *skb)
> struct qrtr_mhi_dev *qdev = container_of(ep, struct qrtr_mhi_dev,
> ep);
> int rc;
>
> + rc = wait_for_completion_interruptible(&qdev->ready);
> + if (rc)
> + goto free_skb;
> +
> if (skb->sk)
> sock_hold(skb->sk);
>
> @@ -78,11 +83,6 @@ static int qcom_mhi_qrtr_probe(struct mhi_device
> *mhi_dev,
> struct qrtr_mhi_dev *qdev;
> int rc;
>
> - /* start channels */
> - rc = mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS);
> - if (rc)
> - return rc;
> -
> qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
> if (!qdev)
> return -ENOMEM;
would it be good to init completion variable here (call init_completion)
?
> @@ -90,12 +90,22 @@ static int qcom_mhi_qrtr_probe(struct mhi_device
> *mhi_dev,
> qdev->mhi_dev = mhi_dev;
> qdev->dev = &mhi_dev->dev;
> qdev->ep.xmit = qcom_mhi_qrtr_send;
> + init_completion(&qdev->ready);
>
>
> return 0;
On 2021-07-21 11:07, Bhaumik Bhatt wrote:
> On 2021-07-21 10:52 AM, [email protected] wrote:
>> On 2021-07-20 18:42, Bhaumik Bhatt wrote:
>>> A dl callback can be received anytime after mhi_prepare_for_transfer
>>> has been called. There is a window where the callback may happen
>>> before the probe initializes the qrtr_mhi_dev state. Move the
>>> mhi_prepare_for_transfer call after the registering the endpoint.
>>>
>>> Once moved, the reverse can happen where qrtr will try to send a
>>> packet
>>> before the channels are prepared. Add a wait in the sending path to
>>> ensure the channels are prepared before trying to do a ul transfer.
>>>
>>> Fixes: a2e2cc0dbb11 ("net: qrtr: Start MHI channels during init")
>>> Signed-off-by: Bhaumik Bhatt <[email protected]>
>>> ---
>>> net/qrtr/mhi.c | 20 +++++++++++++++-----
>>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
>>> index 29b4fa3..22b0395 100644
>>> --- a/net/qrtr/mhi.c
>>> +++ b/net/qrtr/mhi.c
>>> @@ -15,6 +15,7 @@ struct qrtr_mhi_dev {
>>> struct qrtr_endpoint ep;
>>> struct mhi_device *mhi_dev;
>>> struct device *dev;
>>> + struct completion ready;
>>> };
>>>
>>> /* From MHI to QRTR */
>>> @@ -50,6 +51,10 @@ static int qcom_mhi_qrtr_send(struct qrtr_endpoint
>>> *ep, struct sk_buff *skb)
>>> struct qrtr_mhi_dev *qdev = container_of(ep, struct qrtr_mhi_dev,
>>> ep);
>>> int rc;
>>>
>>> + rc = wait_for_completion_interruptible(&qdev->ready);
>>> + if (rc)
>>> + goto free_skb;
>>> +
>>> if (skb->sk)
>>> sock_hold(skb->sk);
>>>
>>> @@ -78,11 +83,6 @@ static int qcom_mhi_qrtr_probe(struct mhi_device
>>> *mhi_dev,
>>> struct qrtr_mhi_dev *qdev;
>>> int rc;
>>>
>>> - /* start channels */
>>> - rc = mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS);
>>> - if (rc)
>>> - return rc;
>>> -
>>> qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
>>> if (!qdev)
>>> return -ENOMEM;
>> would it be good to init completion variable here (call
>> init_completion) ?
> You mean just before setting qdev->mhi_dev? I don't see why that would
> make a difference
> mainly because the qcom_mhi_qrtr_send() will only happen after endpoint
> is
> registered and DL xfer cb will also only come in after we have prepared
> the
> channels and completed ready with dev_data already set.
looks like qcom_mhi_qrtr_send is not going to get called directly. i was
thinking
what if this api is called before init_completion() returns. if it is
only possible
through ep.xmit call back only, can you move it right above
qdev->ep.xmit = qcom_mhi_qrtr_send; ?
>
>>> @@ -90,12 +90,22 @@ static int qcom_mhi_qrtr_probe(struct mhi_device
>>> *mhi_dev,
>>> qdev->mhi_dev = mhi_dev;
>>> qdev->dev = &mhi_dev->dev;
>>> qdev->ep.xmit = qcom_mhi_qrtr_send;
>>> + init_completion(&qdev->ready);
>>>
>>
>>>
>>> return 0;
>
> Thanks,
> Bhaumik
> ---
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
> a Linux Foundation Collaborative Project
Thanks,
Hemant
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
On 2021-07-21 03:27 PM, [email protected] wrote:
> On 2021-07-21 11:07, Bhaumik Bhatt wrote:
>> On 2021-07-21 10:52 AM, [email protected] wrote:
>>> On 2021-07-20 18:42, Bhaumik Bhatt wrote:
>>>> A dl callback can be received anytime after mhi_prepare_for_transfer
>>>> has been called. There is a window where the callback may happen
>>>> before the probe initializes the qrtr_mhi_dev state. Move the
>>>> mhi_prepare_for_transfer call after the registering the endpoint.
>>>>
>>>> Once moved, the reverse can happen where qrtr will try to send a
>>>> packet
>>>> before the channels are prepared. Add a wait in the sending path to
>>>> ensure the channels are prepared before trying to do a ul transfer.
>>>>
>>>> Fixes: a2e2cc0dbb11 ("net: qrtr: Start MHI channels during init")
>>>> Signed-off-by: Bhaumik Bhatt <[email protected]>
>>>> ---
>>>> net/qrtr/mhi.c | 20 +++++++++++++++-----
>>>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
>>>> index 29b4fa3..22b0395 100644
>>>> --- a/net/qrtr/mhi.c
>>>> +++ b/net/qrtr/mhi.c
>>>> @@ -15,6 +15,7 @@ struct qrtr_mhi_dev {
>>>> struct qrtr_endpoint ep;
>>>> struct mhi_device *mhi_dev;
>>>> struct device *dev;
>>>> + struct completion ready;
>>>> };
>>>>
>>>> /* From MHI to QRTR */
>>>> @@ -50,6 +51,10 @@ static int qcom_mhi_qrtr_send(struct
>>>> qrtr_endpoint
>>>> *ep, struct sk_buff *skb)
>>>> struct qrtr_mhi_dev *qdev = container_of(ep, struct qrtr_mhi_dev,
>>>> ep);
>>>> int rc;
>>>>
>>>> + rc = wait_for_completion_interruptible(&qdev->ready);
>>>> + if (rc)
>>>> + goto free_skb;
>>>> +
>>>> if (skb->sk)
>>>> sock_hold(skb->sk);
>>>>
>>>> @@ -78,11 +83,6 @@ static int qcom_mhi_qrtr_probe(struct mhi_device
>>>> *mhi_dev,
>>>> struct qrtr_mhi_dev *qdev;
>>>> int rc;
>>>>
>>>> - /* start channels */
>>>> - rc = mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS);
>>>> - if (rc)
>>>> - return rc;
>>>> -
>>>> qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
>>>> if (!qdev)
>>>> return -ENOMEM;
>>> would it be good to init completion variable here (call
>>> init_completion) ?
>> You mean just before setting qdev->mhi_dev? I don't see why that would
>> make a difference
>> mainly because the qcom_mhi_qrtr_send() will only happen after
>> endpoint is
>> registered and DL xfer cb will also only come in after we have
>> prepared the
>> channels and completed ready with dev_data already set.
> looks like qcom_mhi_qrtr_send is not going to get called directly. i
> was thinking
> what if this api is called before init_completion() returns. if it is
> only possible
> through ep.xmit call back only, can you move it right above
> qdev->ep.xmit = qcom_mhi_qrtr_send; ?
>>
Ah. OK. I see your point. I will do that and upload a v2.
>>>> @@ -90,12 +90,22 @@ static int qcom_mhi_qrtr_probe(struct mhi_device
>>>> *mhi_dev,
>>>> qdev->mhi_dev = mhi_dev;
>>>> qdev->dev = &mhi_dev->dev;
>>>> qdev->ep.xmit = qcom_mhi_qrtr_send;
>>>> + init_completion(&qdev->ready);
>>>>
>>>
>>>>
>>>> return 0;
>>
>> Thanks,
>> Bhaumik
>> ---
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project
>
> Thanks,
> Hemant
> ---
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
> a Linux Foundation Collaborative Project
Thanks,
Bhaumik
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
On 2021-07-22 12:04 PM, Bhaumik Bhatt wrote:
> On 2021-07-21 03:27 PM, [email protected] wrote:
>> On 2021-07-21 11:07, Bhaumik Bhatt wrote:
>>> On 2021-07-21 10:52 AM, [email protected] wrote:
>>>> On 2021-07-20 18:42, Bhaumik Bhatt wrote:
>>>>> A dl callback can be received anytime after
>>>>> mhi_prepare_for_transfer
>>>>> has been called. There is a window where the callback may happen
>>>>> before the probe initializes the qrtr_mhi_dev state. Move the
>>>>> mhi_prepare_for_transfer call after the registering the endpoint.
>>>>>
>>>>> Once moved, the reverse can happen where qrtr will try to send a
>>>>> packet
>>>>> before the channels are prepared. Add a wait in the sending path to
>>>>> ensure the channels are prepared before trying to do a ul transfer.
>>>>>
>>>>> Fixes: a2e2cc0dbb11 ("net: qrtr: Start MHI channels during init")
>>>>> Signed-off-by: Bhaumik Bhatt <[email protected]>
>>>>> ---
>>>>> net/qrtr/mhi.c | 20 +++++++++++++++-----
>>>>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
>>>>> index 29b4fa3..22b0395 100644
>>>>> --- a/net/qrtr/mhi.c
>>>>> +++ b/net/qrtr/mhi.c
>>>>> @@ -15,6 +15,7 @@ struct qrtr_mhi_dev {
>>>>> struct qrtr_endpoint ep;
>>>>> struct mhi_device *mhi_dev;
>>>>> struct device *dev;
>>>>> + struct completion ready;
>>>>> };
>>>>>
>>>>> /* From MHI to QRTR */
>>>>> @@ -50,6 +51,10 @@ static int qcom_mhi_qrtr_send(struct
>>>>> qrtr_endpoint
>>>>> *ep, struct sk_buff *skb)
>>>>> struct qrtr_mhi_dev *qdev = container_of(ep, struct qrtr_mhi_dev,
>>>>> ep);
>>>>> int rc;
>>>>>
>>>>> + rc = wait_for_completion_interruptible(&qdev->ready);
>>>>> + if (rc)
>>>>> + goto free_skb;
>>>>> +
>>>>> if (skb->sk)
>>>>> sock_hold(skb->sk);
>>>>>
>>>>> @@ -78,11 +83,6 @@ static int qcom_mhi_qrtr_probe(struct mhi_device
>>>>> *mhi_dev,
>>>>> struct qrtr_mhi_dev *qdev;
>>>>> int rc;
>>>>>
>>>>> - /* start channels */
>>>>> - rc = mhi_prepare_for_transfer(mhi_dev,
>>>>> MHI_CH_INBOUND_ALLOC_BUFS);
>>>>> - if (rc)
>>>>> - return rc;
>>>>> -
>>>>> qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
>>>>> if (!qdev)
>>>>> return -ENOMEM;
>>>> would it be good to init completion variable here (call
>>>> init_completion) ?
>>> You mean just before setting qdev->mhi_dev? I don't see why that
>>> would
>>> make a difference
>>> mainly because the qcom_mhi_qrtr_send() will only happen after
>>> endpoint is
>>> registered and DL xfer cb will also only come in after we have
>>> prepared the
>>> channels and completed ready with dev_data already set.
>> looks like qcom_mhi_qrtr_send is not going to get called directly. i
>> was thinking
>> what if this api is called before init_completion() returns. if it is
>> only possible
>> through ep.xmit call back only, can you move it right above
>> qdev->ep.xmit = qcom_mhi_qrtr_send; ?
>>>
> Ah. OK. I see your point. I will do that and upload a v2.
>
On second thought, this is not required because the ep.xmit() will not
be called
until the qrtr_endpoint_register() is done.
So this version should be fine IMO.
>>>>> @@ -90,12 +90,22 @@ static int qcom_mhi_qrtr_probe(struct
>>>>> mhi_device *mhi_dev,
>>>>> qdev->mhi_dev = mhi_dev;
>>>>> qdev->dev = &mhi_dev->dev;
>>>>> qdev->ep.xmit = qcom_mhi_qrtr_send;
>>>>> + init_completion(&qdev->ready);
>>>>>
>>>>
>>>>>
>>>>> return 0;
>>>
>>> Thanks,
>>> Bhaumik
>>> ---
>>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>>> Forum,
>>> a Linux Foundation Collaborative Project
>>
>> Thanks,
>> Hemant
>> ---
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project
>
> Thanks,
> Bhaumik
> ---
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
> a Linux Foundation Collaborative Project
Thanks,
Bhaumik
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
On 2021-07-22 12:50, Bhaumik Bhatt wrote:
> On 2021-07-22 12:04 PM, Bhaumik Bhatt wrote:
>> On 2021-07-21 03:27 PM, [email protected] wrote:
>>> On 2021-07-21 11:07, Bhaumik Bhatt wrote:
>>>> On 2021-07-21 10:52 AM, [email protected] wrote:
>>>>> On 2021-07-20 18:42, Bhaumik Bhatt wrote:
>>>>>> A dl callback can be received anytime after
>>>>>> mhi_prepare_for_transfer
>>>>>> has been called. There is a window where the callback may happen
>>>>>> before the probe initializes the qrtr_mhi_dev state. Move the
>>>>>> mhi_prepare_for_transfer call after the registering the endpoint.
>>>>>>
>>>>>> Once moved, the reverse can happen where qrtr will try to send a
>>>>>> packet
>>>>>> before the channels are prepared. Add a wait in the sending path
>>>>>> to
>>>>>> ensure the channels are prepared before trying to do a ul
>>>>>> transfer.
>>>>>>
>>>>>> Fixes: a2e2cc0dbb11 ("net: qrtr: Start MHI channels during init")
>>>>>> Signed-off-by: Bhaumik Bhatt <[email protected]>
>>>>>> ---
>>>>>> net/qrtr/mhi.c | 20 +++++++++++++++-----
>>>>>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
>>>>>> index 29b4fa3..22b0395 100644
>>>>>> --- a/net/qrtr/mhi.c
>>>>>> +++ b/net/qrtr/mhi.c
>>>>>> @@ -15,6 +15,7 @@ struct qrtr_mhi_dev {
>>>>>> struct qrtr_endpoint ep;
>>>>>> struct mhi_device *mhi_dev;
>>>>>> struct device *dev;
>>>>>> + struct completion ready;
>>>>>> };
>>>>>>
>>>>>> /* From MHI to QRTR */
>>>>>> @@ -50,6 +51,10 @@ static int qcom_mhi_qrtr_send(struct
>>>>>> qrtr_endpoint
>>>>>> *ep, struct sk_buff *skb)
>>>>>> struct qrtr_mhi_dev *qdev = container_of(ep, struct
>>>>>> qrtr_mhi_dev, ep);
>>>>>> int rc;
>>>>>>
>>>>>> + rc = wait_for_completion_interruptible(&qdev->ready);
>>>>>> + if (rc)
>>>>>> + goto free_skb;
>>>>>> +
>>>>>> if (skb->sk)
>>>>>> sock_hold(skb->sk);
>>>>>>
>>>>>> @@ -78,11 +83,6 @@ static int qcom_mhi_qrtr_probe(struct
>>>>>> mhi_device *mhi_dev,
>>>>>> struct qrtr_mhi_dev *qdev;
>>>>>> int rc;
>>>>>>
>>>>>> - /* start channels */
>>>>>> - rc = mhi_prepare_for_transfer(mhi_dev,
>>>>>> MHI_CH_INBOUND_ALLOC_BUFS);
>>>>>> - if (rc)
>>>>>> - return rc;
>>>>>> -
>>>>>> qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
>>>>>> if (!qdev)
>>>>>> return -ENOMEM;
>>>>> would it be good to init completion variable here (call
>>>>> init_completion) ?
>>>> You mean just before setting qdev->mhi_dev? I don't see why that
>>>> would
>>>> make a difference
>>>> mainly because the qcom_mhi_qrtr_send() will only happen after
>>>> endpoint is
>>>> registered and DL xfer cb will also only come in after we have
>>>> prepared the
>>>> channels and completed ready with dev_data already set.
>>> looks like qcom_mhi_qrtr_send is not going to get called directly. i
>>> was thinking
>>> what if this api is called before init_completion() returns. if it is
>>> only possible
>>> through ep.xmit call back only, can you move it right above
>>> qdev->ep.xmit = qcom_mhi_qrtr_send; ?
>>>>
>> Ah. OK. I see your point. I will do that and upload a v2.
>>
> On second thought, this is not required because the ep.xmit() will not
> be called
> until the qrtr_endpoint_register() is done.
>
> So this version should be fine IMO.
Thanks for checking that, Bhaumik.
Reviewed-by: Hemant Kumar <[email protected]>
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project