Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1391737

Re: [PATCH V17 1/3] dmaengine: qcom_hidma: implement lower level hardware interface

From Sinan Kaya <okaya@codeaurora.org>
Newsgroups linux.kernel
Subject Re: [PATCH V17 1/3] dmaengine: qcom_hidma: implement lower level hardware interface
Date 2016-05-01 06:40 +0200
Message-ID <rtRkK-3fW-3@gated-at.bofh.it> (permalink)
References (1 earlier) <rmL0L-7lT-25@gated-at.bofh.it> <rs1Rf-vo-7@gated-at.bofh.it> <rscMH-1n6-31@gated-at.bofh.it> <rsdSq-2bx-9@gated-at.bofh.it> <rsZX4-15D-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 4/28/2016 3:30 PM, Sinan Kaya wrote:
> On 4/26/2016 12:24 PM, Vinod Koul wrote:
>>> +
>>>> +       dev_err(lldev->dev, "error 0x%x, resetting...\n",
>>>> +               cause);
>> right justify this and others as well please
>>
> 
> Can you please point me to other lines that need to be fixed please? It looks good
> to me though it doesn't to you. I want to make sure that I'm touching the right ones.
> You seem to prefer a tab following an open parenthesis in your code. Do you want me to
> add a tab for every single multi-line such that it comes this for instance?
> 
> 
> 	ret = readl_poll_timeout(lldev->trca + HIDMA_TRCA_CTRLSTS_REG, val,
> 				 	HIDMA_CH_STATE(val) == HIDMA_CH_DISABLED, 
> 					1000, 10000); 
> 
> instead of
> 
> 	ret = readl_poll_timeout(lldev->trca + HIDMA_TRCA_CTRLSTS_REG, val,
> 				 HIDMA_CH_STATE(val) == HIDMA_CH_DISABLED, 1000,
> 				 10000); 
> 
> 
>>>>>>>> +int hidma_ll_resume(struct hidma_lldev *lldev)
>>>>>>>> +{
>>>>>>>> +	return hidma_ll_enable(lldev);
>>>>>>>> +}
>>>>>>
>>>>>> why do we need this empty function, use hidma_ll_enable.
>>>>
>>>> hidma_ll_enable is a common function that gets called from multiple places. 
>>>> hidma_ll_resume and hidma_ll_pause is used by the OS interface for pausing
>>>> and resuming the DMA channel. 
>> is there a reason why we can't have the code in resume and that being called
>> internally as well as externally?
>>
> 
> I'll have the pause code in OS layer call hidma_ll_enable and get rid of pause
> function here.
> 


I posted v18 along with all the style corrections and other things you asked me.
Please let me know if there is anything outstanding.



-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

Re: [PATCH V17 1/3] dmaengine: qcom_hidma: implement lower level  hardware interface Sinan Kaya <okaya@codeaurora.org> - 2016-04-28 21:40 +0200
  Re: [PATCH V17 1/3] dmaengine: qcom_hidma: implement lower level  hardware interface Sinan Kaya <okaya@codeaurora.org> - 2016-05-01 06:40 +0200

csiph-web