[PATCH] serial: sirf: fix line over 80 characters style issue

[PATCH] serial: sirf: fix line over 80 characters style issue

[Barry Song]

From: Qipan Li <Qipan.Li@csr.com>

According to key customer's requirement, fix "line over 80
characters".

Signed-off-by: Qipan Li <Qipan.Li@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/tty/serial/sirfsoc_uart.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
index 4102192..89f1e59 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -525,7 +525,8 @@ static void sirfsoc_rx_tmo_process_tl(unsigned long param)
 
 	spin_lock_irqsave(&port->lock, flags);
 	while (DMA_COMPLETE == dmaengine_tx_status(sirfport->rx_dma_chan,
-		sirfport->rx_dma_items[sirfport->rx_completed].cookie, &tx_state)) {
+		sirfport->rx_dma_items[sirfport->rx_completed].cookie,
+		&tx_state)) {
 		sirfsoc_uart_insert_rx_buf_to_tty(sirfport,
 					SIRFSOC_RX_DMA_BUF_SIZE);
 		sirfport->rx_completed++;
@@ -707,7 +708,8 @@ static void sirfsoc_uart_rx_dma_complete_tl(unsigned long param)
 	struct dma_tx_state tx_state;
 	spin_lock_irqsave(&port->lock, flags);
 	while (DMA_COMPLETE == dmaengine_tx_status(sirfport->rx_dma_chan,
-			sirfport->rx_dma_items[sirfport->rx_completed].cookie, &tx_state)) {
+		sirfport->rx_dma_items[sirfport->rx_completed].cookie,
+		&tx_state)) {
 		sirfsoc_uart_insert_rx_buf_to_tty(sirfport,
 					SIRFSOC_RX_DMA_BUF_SIZE);
 		if (rd_regl(port, ureg->sirfsoc_int_en_reg) &
@@ -1303,7 +1305,8 @@ static int sirfsoc_uart_probe(struct platform_device *pdev)
 		"sirf,uart-has-rtscts");
 	if (of_device_is_compatible(pdev->dev.of_node, "sirf,prima2-uart"))
 		sirfport->uart_reg->uart_type = SIRF_REAL_UART;
-	if (of_device_is_compatible(pdev->dev.of_node, "sirf,prima2-usp-uart")) {
+	if (of_device_is_compatible(pdev->dev.of_node,
+		"sirf,prima2-usp-uart")) {
 		sirfport->uart_reg->uart_type =	SIRF_USP_UART;
 		if (!sirfport->hw_flow_ctrl)
 			goto usp_no_flow_control;
@@ -1360,11 +1363,12 @@ usp_no_flow_control:
 		goto err;
 	}
 	tasklet_init(&sirfport->rx_dma_complete_tasklet,
-			sirfsoc_uart_rx_dma_complete_tl, (unsigned long)sirfport);
+		sirfsoc_uart_rx_dma_complete_tl, (unsigned long)sirfport);
 	tasklet_init(&sirfport->rx_tmo_process_tasklet,
 			sirfsoc_rx_tmo_process_tl, (unsigned long)sirfport);
 	port->mapbase = res->start;
-	port->membase = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	port->membase = devm_ioremap(&pdev->dev,
+			res->start, resource_size(res));
 	if (!port->membase) {
 		dev_err(&pdev->dev, "Cannot remap resource.\n");
 		ret = -ENOMEM;
-- 
2.0.4



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at www.aptx.com.

Re: [PATCH] serial: sirf: fix line over 80 characters style issue

[Greg KH]

On Mon, Aug 18, 2014 at 05:32:52PM +0800, Barry Song wrote:
> From: Qipan Li <Qipan.Li@csr.com>
> 
> According to key customer's requirement, fix "line over 80
> characters".

Someone is forcing you to fix all upstream kernel.org code for 80
columns?  Who?

> Signed-off-by: Qipan Li <Qipan.Li@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
>  drivers/tty/serial/sirfsoc_uart.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
> index 4102192..89f1e59 100644
> --- a/drivers/tty/serial/sirfsoc_uart.c
> +++ b/drivers/tty/serial/sirfsoc_uart.c
> @@ -525,7 +525,8 @@ static void sirfsoc_rx_tmo_process_tl(unsigned long param)
>  
>  	spin_lock_irqsave(&port->lock, flags);
>  	while (DMA_COMPLETE == dmaengine_tx_status(sirfport->rx_dma_chan,
> -		sirfport->rx_dma_items[sirfport->rx_completed].cookie, &tx_state)) {
> +		sirfport->rx_dma_items[sirfport->rx_completed].cookie,
> +		&tx_state)) {

Isn't that now harder to read?

>  		sirfsoc_uart_insert_rx_buf_to_tty(sirfport,
>  					SIRFSOC_RX_DMA_BUF_SIZE);
>  		sirfport->rx_completed++;
> @@ -707,7 +708,8 @@ static void sirfsoc_uart_rx_dma_complete_tl(unsigned long param)
>  	struct dma_tx_state tx_state;
>  	spin_lock_irqsave(&port->lock, flags);
>  	while (DMA_COMPLETE == dmaengine_tx_status(sirfport->rx_dma_chan,
> -			sirfport->rx_dma_items[sirfport->rx_completed].cookie, &tx_state)) {
> +		sirfport->rx_dma_items[sirfport->rx_completed].cookie,
> +		&tx_state)) {

Same here.

>  		sirfsoc_uart_insert_rx_buf_to_tty(sirfport,
>  					SIRFSOC_RX_DMA_BUF_SIZE);
>  		if (rd_regl(port, ureg->sirfsoc_int_en_reg) &
> @@ -1303,7 +1305,8 @@ static int sirfsoc_uart_probe(struct platform_device *pdev)
>  		"sirf,uart-has-rtscts");
>  	if (of_device_is_compatible(pdev->dev.of_node, "sirf,prima2-uart"))
>  		sirfport->uart_reg->uart_type = SIRF_REAL_UART;
> -	if (of_device_is_compatible(pdev->dev.of_node, "sirf,prima2-usp-uart")) {
> +	if (of_device_is_compatible(pdev->dev.of_node,
> +		"sirf,prima2-usp-uart")) {

Same.

>  		sirfport->uart_reg->uart_type =	SIRF_USP_UART;
>  		if (!sirfport->hw_flow_ctrl)
>  			goto usp_no_flow_control;
> @@ -1360,11 +1363,12 @@ usp_no_flow_control:
>  		goto err;
>  	}
>  	tasklet_init(&sirfport->rx_dma_complete_tasklet,
> -			sirfsoc_uart_rx_dma_complete_tl, (unsigned long)sirfport);
> +		sirfsoc_uart_rx_dma_complete_tl, (unsigned long)sirfport);

Same.

You can break this line at the last variable instead.

>  	tasklet_init(&sirfport->rx_tmo_process_tasklet,
>  			sirfsoc_rx_tmo_process_tl, (unsigned long)sirfport);
>  	port->mapbase = res->start;
> -	port->membase = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> +	port->membase = devm_ioremap(&pdev->dev,
> +			res->start, resource_size(res));

same here, it could be made to look better.

Sorry, not going to take this, if your customer blames you, point them
at me.

seriously.

greg k-h

Re: [PATCH] serial: sirf: fix line over 80 characters style issue

[Barry Song]

On 14-9-9 上午7:15, "Greg KH" <gregkh@linuxfoundation.org> wrote:

>On Mon, Aug 18, 2014 at 05:32:52PM +0800, Barry Song wrote:
>> From: Qipan Li <Qipan.Li@csr.com>
>> 
>> According to key customer's requirement, fix "line over 80
>> characters".
>
>Someone is forcing you to fix all upstream kernel.org code for 80
>columns?  Who?

we were asked to fix csr platform codes some days ago. They have been
fixed locally. 
You know, the difficulty is maintaining the difference between mainline
and local codes.
So I thought this is better if you can merge it.

>
>> Signed-off-by: Qipan Li <Qipan.Li@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>>  drivers/tty/serial/sirfsoc_uart.c | 14 +++++++++-----
>>  1 file changed, 9 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/tty/serial/sirfsoc_uart.c
>>b/drivers/tty/serial/sirfsoc_uart.c
>> index 4102192..89f1e59 100644
>> --- a/drivers/tty/serial/sirfsoc_uart.c
>> +++ b/drivers/tty/serial/sirfsoc_uart.c
>> @@ -525,7 +525,8 @@ static void sirfsoc_rx_tmo_process_tl(unsigned long
>>param)
>>  
>>  	spin_lock_irqsave(&port->lock, flags);
>>  	while (DMA_COMPLETE == dmaengine_tx_status(sirfport->rx_dma_chan,
>> -		sirfport->rx_dma_items[sirfport->rx_completed].cookie, &tx_state)) {
>> +		sirfport->rx_dma_items[sirfport->rx_completed].cookie,
>> +		&tx_state)) {
>
>Isn't that now harder to read?
>
>>  		sirfsoc_uart_insert_rx_buf_to_tty(sirfport,
>>  					SIRFSOC_RX_DMA_BUF_SIZE);
>>  		sirfport->rx_completed++;
>> @@ -707,7 +708,8 @@ static void
>>sirfsoc_uart_rx_dma_complete_tl(unsigned long param)
>>  	struct dma_tx_state tx_state;
>>  	spin_lock_irqsave(&port->lock, flags);
>>  	while (DMA_COMPLETE == dmaengine_tx_status(sirfport->rx_dma_chan,
>> -			sirfport->rx_dma_items[sirfport->rx_completed].cookie, &tx_state)) {
>> +		sirfport->rx_dma_items[sirfport->rx_completed].cookie,
>> +		&tx_state)) {
>
>Same here.
>
>>  		sirfsoc_uart_insert_rx_buf_to_tty(sirfport,
>>  					SIRFSOC_RX_DMA_BUF_SIZE);
>>  		if (rd_regl(port, ureg->sirfsoc_int_en_reg) &
>> @@ -1303,7 +1305,8 @@ static int sirfsoc_uart_probe(struct
>>platform_device *pdev)
>>  		"sirf,uart-has-rtscts");
>>  	if (of_device_is_compatible(pdev->dev.of_node, "sirf,prima2-uart"))
>>  		sirfport->uart_reg->uart_type = SIRF_REAL_UART;
>> -	if (of_device_is_compatible(pdev->dev.of_node,
>>"sirf,prima2-usp-uart")) {
>> +	if (of_device_is_compatible(pdev->dev.of_node,
>> +		"sirf,prima2-usp-uart")) {
>
>Same.
>
>>  		sirfport->uart_reg->uart_type =	SIRF_USP_UART;
>>  		if (!sirfport->hw_flow_ctrl)
>>  			goto usp_no_flow_control;
>> @@ -1360,11 +1363,12 @@ usp_no_flow_control:
>>  		goto err;
>>  	}
>>  	tasklet_init(&sirfport->rx_dma_complete_tasklet,
>> -			sirfsoc_uart_rx_dma_complete_tl, (unsigned long)sirfport);
>> +		sirfsoc_uart_rx_dma_complete_tl, (unsigned long)sirfport);
>
>Same.
>
>You can break this line at the last variable instead.
>
>>  	tasklet_init(&sirfport->rx_tmo_process_tasklet,
>>  			sirfsoc_rx_tmo_process_tl, (unsigned long)sirfport);
>>  	port->mapbase = res->start;
>> -	port->membase = devm_ioremap(&pdev->dev, res->start,
>>resource_size(res));
>> +	port->membase = devm_ioremap(&pdev->dev,
>> +			res->start, resource_size(res));
>
>same here, it could be made to look better.
>
>Sorry, not going to take this, if your customer blames you, point them
>at me.

I understand you are very helpful. But pointing them to you will actually
make things worse as I think we have no chance to convince them as we have
tried our best to do that.
The benefit of this patch is making people happy. But if you are not
happy, I can make myself unhappy to maintain two versions.
 
>
>seriously.
>
>greg k-h
>

-barry


--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] serial: sirf: fix line over 80 characters style issue

[Greg KH]

On Tue, Sep 09, 2014 at 08:16:47AM +0800, Barry Song wrote:
> 
> 
> On 14-9-9 上午7:15, "Greg KH" <gregkh@linuxfoundation.org> wrote:
> 
> >On Mon, Aug 18, 2014 at 05:32:52PM +0800, Barry Song wrote:
> >> From: Qipan Li <Qipan.Li@csr.com>
> >> 
> >> According to key customer's requirement, fix "line over 80
> >> characters".
> >
> >Someone is forcing you to fix all upstream kernel.org code for 80
> >columns?  Who?
> 
> we were asked to fix csr platform codes some days ago. They have been
> fixed locally. 

What is "locally"?

And you really are forced to do foolish cleanups for no reason in
mainline kernel code for no reason?

> You know, the difficulty is maintaining the difference between
> mainline and local codes.
> So I thought this is better if you can merge it.

I'm not taking stuff that isn't correct.  Or is foolish.  Neither should
you.

> >Sorry, not going to take this, if your customer blames you, point them
> >at me.
> 
> I understand you are very helpful. But pointing them to you will actually
> make things worse as I think we have no chance to convince them as we have
> tried our best to do that.
> The benefit of this patch is making people happy. But if you are not
> happy, I can make myself unhappy to maintain two versions.

Try fixing the code to look better, your changes were not valid ones,
you are quieting some automatic tool checker, you aren't doing the
correct thing and following the "spirit" of the rule here.

Make the code look cleaner, and I'll be glad to take the change.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] serial: sirf: fix line over 80 characters style issue

[Barry Song]

On 14-9-9 上午9:33, "Greg KH" <gregkh@linuxfoundation.org> wrote:

>On Tue, Sep 09, 2014 at 08:16:47AM +0800, Barry Song wrote:
>> 
>> 
>> On 14-9-9 上午7:15, "Greg KH" <gregkh@linuxfoundation.org> wrote:
>> 
>> >On Mon, Aug 18, 2014 at 05:32:52PM +0800, Barry Song wrote:
>> >> From: Qipan Li <Qipan.Li@csr.com>
>> >> 
>> >> According to key customer's requirement, fix "line over 80
>> >> characters".
>> >
>> >Someone is forcing you to fix all upstream kernel.org code for 80
>> >columns?  Who?
>> 
>> we were asked to fix csr platform codes some days ago. They have been
>> fixed locally. 
>
>What is "locally"?
>
>And you really are forced to do foolish cleanups for no reason in
>mainline kernel code for no reason?

"Locally" means the code tree in our local git repo. Nobody asked me to
cleanup all mainline codes, the asked changes are for sirf platform codes
whose authors are sirf platform engineers.

>
>> You know, the difficulty is maintaining the difference between
>> mainline and local codes.
>> So I thought this is better if you can merge it.
>
>I'm not taking stuff that isn't correct.  Or is foolish.  Neither should
>you.

Never. I always think we need to make codes have good readability.

>
>> >Sorry, not going to take this, if your customer blames you, point them
>> >at me.
>> 
>> I understand you are very helpful. But pointing them to you will
>>actually
>> make things worse as I think we have no chance to convince them as we
>>have
>> tried our best to do that.
>> The benefit of this patch is making people happy. But if you are not
>> happy, I can make myself unhappy to maintain two versions.
>
>Try fixing the code to look better, your changes were not valid ones,
>you are quieting some automatic tool checker, you aren't doing the
>correct thing and following the "spirit" of the rule here.
>
>Make the code look cleaner, and I'll be glad to take the change.

As you said, most old codes looks better with "line over 80 chars", so I
would like to know what is the way you are happy. Is it fixing all "line
over 80 chars" with changes following the "spirit", or only splitting
those lines which will follow "spirit" better if we split them.
If the lines over 80 chars have been good and following the "spirit", do
they need to be fixed?

>
>greg k-h

-barry


--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] serial: sirf: fix line over 80 characters style issue

[Greg KH]

On Tue, Sep 09, 2014 at 10:16:07AM +0800, Barry Song wrote:
> 
> 
> On 14-9-9 上午9:33, "Greg KH" <gregkh@linuxfoundation.org> wrote:
> 
> >On Tue, Sep 09, 2014 at 08:16:47AM +0800, Barry Song wrote:
> >> 
> >> 
> >> On 14-9-9 上午7:15, "Greg KH" <gregkh@linuxfoundation.org> wrote:
> >> 
> >> >On Mon, Aug 18, 2014 at 05:32:52PM +0800, Barry Song wrote:
> >> >> From: Qipan Li <Qipan.Li@csr.com>
> >> >> 
> >> >> According to key customer's requirement, fix "line over 80
> >> >> characters".
> >> >
> >> >Someone is forcing you to fix all upstream kernel.org code for 80
> >> >columns?  Who?
> >> 
> >> we were asked to fix csr platform codes some days ago. They have been
> >> fixed locally. 
> >
> >What is "locally"?
> >
> >And you really are forced to do foolish cleanups for no reason in
> >mainline kernel code for no reason?
> 
> "Locally" means the code tree in our local git repo. Nobody asked me to
> cleanup all mainline codes, the asked changes are for sirf platform codes
> whose authors are sirf platform engineers.
> 
> >
> >> You know, the difficulty is maintaining the difference between
> >> mainline and local codes.
> >> So I thought this is better if you can merge it.
> >
> >I'm not taking stuff that isn't correct.  Or is foolish.  Neither should
> >you.
> 
> Never. I always think we need to make codes have good readability.
> 
> >
> >> >Sorry, not going to take this, if your customer blames you, point them
> >> >at me.
> >> 
> >> I understand you are very helpful. But pointing them to you will
> >>actually
> >> make things worse as I think we have no chance to convince them as we
> >>have
> >> tried our best to do that.
> >> The benefit of this patch is making people happy. But if you are not
> >> happy, I can make myself unhappy to maintain two versions.
> >
> >Try fixing the code to look better, your changes were not valid ones,
> >you are quieting some automatic tool checker, you aren't doing the
> >correct thing and following the "spirit" of the rule here.
> >
> >Make the code look cleaner, and I'll be glad to take the change.
> 
> As you said, most old codes looks better with "line over 80 chars", so I
> would like to know what is the way you are happy. Is it fixing all "line
> over 80 chars" with changes following the "spirit", or only splitting
> those lines which will follow "spirit" better if we split them.
> If the lines over 80 chars have been good and following the "spirit", do
> they need to be fixed?

No they do not, the rule is there to make the code readable, don't go
through gyrations to match the 80 column rule that makes the code less
readable, as that is counter productive.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] serial: sirf: fix line over 80 characters style issue

[Barry Song]

On 14-9-9 上午10:33, "Greg KH" <gregkh@linuxfoundation.org> wrote:

>On Tue, Sep 09, 2014 at 10:16:07AM +0800, Barry Song wrote:
>> 
>> 
>> On 14-9-9 上午9:33, "Greg KH" <gregkh@linuxfoundation.org> wrote:
>> 
>> >On Tue, Sep 09, 2014 at 08:16:47AM +0800, Barry Song wrote:
>> >> 
>> >> 
>> >> On 14-9-9 上午7:15, "Greg KH" <gregkh@linuxfoundation.org> wrote:
>> >> 
>> >> >On Mon, Aug 18, 2014 at 05:32:52PM +0800, Barry Song wrote:
>> >> >> From: Qipan Li <Qipan.Li@csr.com>
>> >> >> 
>> >> >> According to key customer's requirement, fix "line over 80
>> >> >> characters".
>> >> >
>> >> >Someone is forcing you to fix all upstream kernel.org code for 80
>> >> >columns?  Who?
>> >> 
>> >> we were asked to fix csr platform codes some days ago. They have been
>> >> fixed locally.
>> >
>> >What is "locally"?
>> >
>> >And you really are forced to do foolish cleanups for no reason in
>> >mainline kernel code for no reason?
>> 
>> "Locally" means the code tree in our local git repo. Nobody asked me to
>> cleanup all mainline codes, the asked changes are for sirf platform
>>codes
>> whose authors are sirf platform engineers.
>> 
>> >
>> >> You know, the difficulty is maintaining the difference between
>> >> mainline and local codes.
>> >> So I thought this is better if you can merge it.
>> >
>> >I'm not taking stuff that isn't correct.  Or is foolish.  Neither
>>should
>> >you.
>> 
>> Never. I always think we need to make codes have good readability.
>> 
>> >
>> >> >Sorry, not going to take this, if your customer blames you, point
>>them
>> >> >at me.
>> >> 
>> >> I understand you are very helpful. But pointing them to you will
>> >>actually
>> >> make things worse as I think we have no chance to convince them as we
>> >>have
>> >> tried our best to do that.
>> >> The benefit of this patch is making people happy. But if you are not
>> >> happy, I can make myself unhappy to maintain two versions.
>> >
>> >Try fixing the code to look better, your changes were not valid ones,
>> >you are quieting some automatic tool checker, you aren't doing the
>> >correct thing and following the "spirit" of the rule here.
>> >
>> >Make the code look cleaner, and I'll be glad to take the change.
>> 
>> As you said, most old codes looks better with "line over 80 chars", so I
>> would like to know what is the way you are happy. Is it fixing all "line
>> over 80 chars" with changes following the "spirit", or only splitting
>> those lines which will follow "spirit" better if we split them.
>> If the lines over 80 chars have been good and following the "spirit", do
>> they need to be fixed?
>
>No they do not, the rule is there to make the code readable, don't go
>through gyrations to match the 80 column rule that makes the code less
>readable, as that is counter productive.

We have been doing a checkpatch pre-commit hook in git server for a long
time. As you know , a basic hook will reject any changes which have
checkpatch issues including "line over 80 chars". In my original
understanding, this depends on the context of codes. So we disabled "the
line over 80 chars" in checkpatch.pl and moved to manual reviews for this.
Finally, this triggered a quality control mechanism of users and caused
many fault reports from customers who were doing serious checkpatch.pl. we
were asked to fix the "bad" quality codes. This is the story.
 
There are still some gaps to make people understand this and finally all
people become happy. But I am still happy to continue to fix the
situation, and your comments will surely help.
I am still happy to take this one as a start to fix what should be fixed.

>
>thanks,
>
>greg k-h

-barry


--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html