linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dmitry Osipenko <digetx@gmail.com>
To: "Jon Hunter" <jonathanh@nvidia.com>,
	"Laxman Dewangan" <ldewangan@nvidia.com>,
	"Vinod Koul" <vkoul@kernel.org>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Michał Mirosław" <mirq-linux@rere.qmqm.pl>
Cc: dmaengine@vger.kernel.org, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 08/14] dmaengine: tegra-apb: Fix coding style problems
Date: Thu, 16 Jan 2020 20:37:27 +0300	[thread overview]
Message-ID: <134adcfb-83fb-4bb7-986e-65217bc4f821@gmail.com> (raw)
In-Reply-To: <844c4ace-d043-a908-823d-545b5b753008@nvidia.com>

15.01.2020 12:49, Jon Hunter пишет:
> 
> 
> On 12/01/2020 17:30, Dmitry Osipenko wrote:
>> This patch fixes few dozens of coding style problems reported by
>> checkpatch and prettifies code where makes sense.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/dma/tegra20-apb-dma.c | 276 ++++++++++++++++++----------------
>>  1 file changed, 144 insertions(+), 132 deletions(-)
>>
>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
>> index dff21e80ffa4..7158bd3145c4 100644
>> --- a/drivers/dma/tegra20-apb-dma.c
>> +++ b/drivers/dma/tegra20-apb-dma.c
> 
> ...
> 
>> @@ -1003,20 +1014,23 @@ static void tegra_dma_prep_wcount(struct tegra_dma_channel *tdc,
>>  		ch_regs->csr |= len_field;
>>  }
>>  
>> -static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg(
>> -	struct dma_chan *dc, struct scatterlist *sgl, unsigned int sg_len,
>> -	enum dma_transfer_direction direction, unsigned long flags,
>> -	void *context)
>> +static struct dma_async_tx_descriptor *
>> +tegra_dma_prep_slave_sg(struct dma_chan *dc,
>> +			struct scatterlist *sgl,
>> +			unsigned int sg_len,
>> +			enum dma_transfer_direction direction,
>> +			unsigned long flags,
>> +			void *context)
>>  {
>>  	struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
>> +	struct tegra_dma_sg_req *sg_req = NULL;
>> +	u32 csr, ahb_seq, apb_ptr, apb_seq;
>> +	enum dma_slave_buswidth slave_bw;
>>  	struct tegra_dma_desc *dma_desc;
>> -	unsigned int i;
>> -	struct scatterlist *sg;
>> -	unsigned long csr, ahb_seq, apb_ptr, apb_seq;
>>  	struct list_head req_list;
>> -	struct tegra_dma_sg_req  *sg_req = NULL;
>> -	u32 burst_size;
>> -	enum dma_slave_buswidth slave_bw;
>> +	struct scatterlist *sg;
>> +	unsigned int burst_size;
>> +	unsigned int i;
> 
> This is not really consistent with the rest of the changes by having 'i'
> and 'burst_size' on separate lines.

The goal wasn't to squash everything into a single line, but to make
code more readable. In this particular case the separated lines look
better to me.

>>  
>>  	if (!tdc->config_init) {
>>  		dev_err(tdc2dev(tdc), "DMA channel is not configured\n");
>> @@ -1028,7 +1042,7 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg(
>>  	}
>>  
>>  	if (get_transfer_param(tdc, direction, &apb_ptr, &apb_seq, &csr,
>> -				&burst_size, &slave_bw) < 0)
>> +			       &burst_size, &slave_bw) < 0)
>>  		return NULL;
>>  
>>  	INIT_LIST_HEAD(&req_list);
>> @@ -1074,7 +1088,7 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg(
>>  		len = sg_dma_len(sg);
>>  
>>  		if ((len & 3) || (mem & 3) ||
>> -				(len > tdc->tdma->chip_data->max_dma_count)) {
>> +		    len > tdc->tdma->chip_data->max_dma_count) {
>>  			dev_err(tdc2dev(tdc),
>>  				"DMA length/memory address is not supported\n");
>>  			tegra_dma_desc_put(tdc, dma_desc);
>> @@ -1126,20 +1140,21 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg(
>>  	return &dma_desc->txd;
>>  }
>>  
>> -static struct dma_async_tx_descriptor *tegra_dma_prep_dma_cyclic(
>> -	struct dma_chan *dc, dma_addr_t buf_addr, size_t buf_len,
>> -	size_t period_len, enum dma_transfer_direction direction,
>> -	unsigned long flags)
>> +static struct dma_async_tx_descriptor *
>> +tegra_dma_prep_dma_cyclic(struct dma_chan *dc, dma_addr_t buf_addr,
>> +			  size_t buf_len,
>> +			  size_t period_len,
>> +			  enum dma_transfer_direction direction,
>> +			  unsigned long flags)
>>  {
>>  	struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
>> -	struct tegra_dma_desc *dma_desc = NULL;
>>  	struct tegra_dma_sg_req *sg_req = NULL;
>> -	unsigned long csr, ahb_seq, apb_ptr, apb_seq;
>> -	int len;
>> -	size_t remain_len;
>> -	dma_addr_t mem = buf_addr;
>> -	u32 burst_size;
>> +	u32 csr, ahb_seq, apb_ptr, apb_seq;
>>  	enum dma_slave_buswidth slave_bw;
>> +	struct tegra_dma_desc *dma_desc;
>> +	dma_addr_t mem = buf_addr;
>> +	unsigned int burst_size;
>> +	size_t len, remain_len;
>>  
>>  	if (!buf_len || !period_len) {
>>  		dev_err(tdc2dev(tdc), "Invalid buffer/period len\n");
>> @@ -1173,13 +1188,13 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_dma_cyclic(
>>  
>>  	len = period_len;
>>  	if ((len & 3) || (buf_addr & 3) ||
>> -			(len > tdc->tdma->chip_data->max_dma_count)) {
>> +	    len > tdc->tdma->chip_data->max_dma_count) {
>>  		dev_err(tdc2dev(tdc), "Req len/mem address is not correct\n");
>>  		return NULL;
>>  	}
>>  
>>  	if (get_transfer_param(tdc, direction, &apb_ptr, &apb_seq, &csr,
>> -				&burst_size, &slave_bw) < 0)
>> +			       &burst_size, &slave_bw) < 0)
>>  		return NULL;
>>  
>>  	ahb_seq = TEGRA_APBDMA_AHBSEQ_INTR_ENB;
>> @@ -1269,7 +1284,6 @@ static int tegra_dma_alloc_chan_resources(struct dma_chan *dc)
>>  	int ret;
>>  
>>  	dma_cookie_init(&tdc->dma_chan);
>> -	tdc->config_init = false;
> 
> Why is this removed? Does not seem to belong in this patch.

Because initially, on driver's probe, the tdc->config_init is false for
all channels and then tegra_dma_free_chan_resources() also sets it to
false. Thus there is no need to re-initilize the already initialized
variable. It's not a very good coding style if variables are
unnecessarily initialized, you probably noticed that there are few other
cases of removing the unneeded initializations of local variables in
this patch.

  reply	other threads:[~2020-01-16 17:38 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-12 17:29 [PATCH v4 00/14] NVIDIA Tegra APB DMA driver fixes and improvements Dmitry Osipenko
2020-01-12 17:29 ` [PATCH v4 01/14] dmaengine: tegra-apb: Fix use-after-free Dmitry Osipenko
2020-01-14 15:09   ` Jon Hunter
2020-01-14 20:33     ` Dmitry Osipenko
2020-01-15  9:00       ` Jon Hunter
2020-01-16 20:10         ` Dmitry Osipenko
2020-01-28 14:02           ` Jon Hunter
2020-01-28 14:51             ` Dmitry Osipenko
2020-01-29  0:12               ` Dmitry Osipenko
2020-01-29 10:42                 ` Jon Hunter
2020-01-12 17:29 ` [PATCH v4 02/14] dmaengine: tegra-apb: Implement synchronization callback Dmitry Osipenko
2020-01-14 15:15   ` Jon Hunter
2020-01-14 21:02     ` Dmitry Osipenko
2020-01-15  9:18       ` Jon Hunter
2020-01-15 10:25         ` Jon Hunter
2020-01-12 17:29 ` [PATCH v4 03/14] dmaengine: tegra-apb: Prevent race conditions on channel's freeing Dmitry Osipenko
2020-01-14 15:16   ` Jon Hunter
2020-01-12 17:29 ` [PATCH v4 04/14] dmaengine: tegra-apb: Clean up tasklet releasing Dmitry Osipenko
2020-01-14 15:36   ` Jon Hunter
2020-01-12 17:29 ` [PATCH v4 05/14] dmaengine: tegra-apb: Prevent race conditions of tasklet vs free list Dmitry Osipenko
2020-01-14 15:43   ` Jon Hunter
2020-01-12 17:29 ` [PATCH v4 06/14] dmaengine: tegra-apb: Use devm_platform_ioremap_resource Dmitry Osipenko
2020-01-14 15:44   ` Jon Hunter
2020-01-12 17:29 ` [PATCH v4 07/14] dmaengine: tegra-apb: Use devm_request_irq Dmitry Osipenko
2020-01-14 15:44   ` Jon Hunter
2020-01-12 17:30 ` [PATCH v4 08/14] dmaengine: tegra-apb: Fix coding style problems Dmitry Osipenko
2020-01-15  9:49   ` Jon Hunter
2020-01-16 17:37     ` Dmitry Osipenko [this message]
2020-01-28 14:05       ` Jon Hunter
2020-01-28 15:01         ` Dmitry Osipenko
2020-01-12 17:30 ` [PATCH v4 09/14] dmaengine: tegra-apb: Clean up runtime PM teardown Dmitry Osipenko
2020-01-15  9:57   ` Jon Hunter
2020-01-16 17:18     ` Dmitry Osipenko
2020-01-12 17:30 ` [PATCH v4 10/14] dmaengine: tegra-apb: Keep clock enabled only during of DMA transfer Dmitry Osipenko
2020-01-15 10:08   ` Jon Hunter
2020-01-16 17:01     ` Dmitry Osipenko
2020-01-12 17:30 ` [PATCH v4 11/14] dmaengine: tegra-apb: Clean up suspend-resume Dmitry Osipenko
2020-01-21 21:23   ` Dmitry Osipenko
2020-01-28 14:10     ` Jon Hunter
2020-01-28 14:53       ` Dmitry Osipenko
2020-01-12 17:30 ` [PATCH v4 12/14] dmaengine: tegra-apb: Add missing of_dma_controller_free Dmitry Osipenko
2020-01-15 10:10   ` Jon Hunter
2020-01-12 17:30 ` [PATCH v4 13/14] dmaengine: tegra-apb: Allow to compile as a loadable kernel module Dmitry Osipenko
2020-01-15 10:10   ` Jon Hunter
2020-01-12 17:30 ` [PATCH v4 14/14] dmaengine: tegra-apb: Remove MODULE_ALIAS Dmitry Osipenko
2020-01-15 10:11   ` Jon Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=134adcfb-83fb-4bb7-986e-65217bc4f821@gmail.com \
    --to=digetx@gmail.com \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=jonathanh@nvidia.com \
    --cc=ldewangan@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=mirq-linux@rere.qmqm.pl \
    --cc=thierry.reding@gmail.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).