[PATCH] serial: samsung: fix the inconsistency in spinlock

[PATCH] serial: samsung: fix the inconsistency in spinlock

[Anand Moon]

From: Anand Moon <linux.amoon@gmail.com>

changes fix the correct order of the spin_lock_irqrestore/save.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/tty/serial/samsung.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index d72cd73..96fe14d 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -759,9 +759,9 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
 	}
 
 	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) {
-		spin_unlock(&port->lock);
+		spin_unlock_irqrestore(&port->lock, flags);
 		uart_write_wakeup(port);
-		spin_lock(&port->lock);
+		spin_lock_irqsave(&port->lock, flags);
 	}
 
 	if (uart_circ_empty(xmit))
-- 
1.9.1

Re: [PATCH] serial: samsung: fix the inconsistency in spinlock

[Peter Hurley]

Hi Anand,

On 02/18/2016 09:40 AM, Anand Moon wrote:
> From: Anand Moon <linux.amoon@gmail.com>
> 
> changes fix the correct order of the spin_lock_irqrestore/save.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  drivers/tty/serial/samsung.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> index d72cd73..96fe14d 100644
> --- a/drivers/tty/serial/samsung.c
> +++ b/drivers/tty/serial/samsung.c
> @@ -759,9 +759,9 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
>  	}
>  
>  	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) {
> -		spin_unlock(&port->lock);
> +		spin_unlock_irqrestore(&port->lock, flags);
>  		uart_write_wakeup(port);
> -		spin_lock(&port->lock);
> +		spin_lock_irqsave(&port->lock, flags);

This driver shouldn't be dropping the spin lock at for write wakeup.
If this is causing lock-ups in a line discipline, the line discipline
needs fixed.

Regards,
Peter Hurley


>  	}
>  
>  	if (uart_circ_empty(xmit))
> 

Re: [PATCH] serial: samsung: fix the inconsistency in spinlock

[Anand Moon]

Hi Peter,

On 18 February 2016 at 23:18, Peter Hurley <peter@hurleysoftware.com> wrote:
> Hi Anand,
>
> On 02/18/2016 09:40 AM, Anand Moon wrote:
>> From: Anand Moon <linux.amoon@gmail.com>
>>
>> changes fix the correct order of the spin_lock_irqrestore/save.
>>
>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>> ---
>>  drivers/tty/serial/samsung.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>> index d72cd73..96fe14d 100644
>> --- a/drivers/tty/serial/samsung.c
>> +++ b/drivers/tty/serial/samsung.c
>> @@ -759,9 +759,9 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
>>       }
>>
>>       if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) {
>> -             spin_unlock(&port->lock);
>> +             spin_unlock_irqrestore(&port->lock, flags);
>>               uart_write_wakeup(port);
>> -             spin_lock(&port->lock);
>> +             spin_lock_irqsave(&port->lock, flags);
>
> This driver shouldn't be dropping the spin lock at for write wakeup.
> If this is causing lock-ups in a line discipline, the line discipline
> needs fixed.
>

Thanks for pointing out.
Their is no lock up, just the inconstancy of the spin_lock.
Then I will resend this patch dropping the spin_unlock/spin_lock
around uart_write_wakeup.
Is that ok with you.

> Regards,
> Peter Hurley
>
>
>>       }
>>
>>       if (uart_circ_empty(xmit))
>>
>

Best Regards
-Anand Moon

Re: [PATCH] serial: samsung: fix the inconsistency in spinlock

[Peter Hurley]

On 02/18/2016 11:14 AM, Anand Moon wrote:
> Thanks for pointing out.
> Their is no lock up, just the inconstancy of the spin_lock.
> Then I will resend this patch dropping the spin_unlock/spin_lock
> around uart_write_wakeup.
> Is that ok with you.

Yes.

Re: [PATCH] serial: samsung: fix the inconsistency in spinlock

[Krzysztof Kozlowski]

2016-02-19 4:14 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
> Hi Peter,
>
> On 18 February 2016 at 23:18, Peter Hurley <peter@hurleysoftware.com> wrote:
>> Hi Anand,
>>
>> On 02/18/2016 09:40 AM, Anand Moon wrote:
>>> From: Anand Moon <linux.amoon@gmail.com>
>>>
>>> changes fix the correct order of the spin_lock_irqrestore/save.
>>>
>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>> ---
>>>  drivers/tty/serial/samsung.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>>> index d72cd73..96fe14d 100644
>>> --- a/drivers/tty/serial/samsung.c
>>> +++ b/drivers/tty/serial/samsung.c
>>> @@ -759,9 +759,9 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
>>>       }
>>>
>>>       if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) {
>>> -             spin_unlock(&port->lock);
>>> +             spin_unlock_irqrestore(&port->lock, flags);
>>>               uart_write_wakeup(port);
>>> -             spin_lock(&port->lock);
>>> +             spin_lock_irqsave(&port->lock, flags);
>>
>> This driver shouldn't be dropping the spin lock at for write wakeup.
>> If this is causing lock-ups in a line discipline, the line discipline
>> needs fixed.
>>
>
> Thanks for pointing out.
> Their is no lock up, just the inconstancy of the spin_lock.
> Then I will resend this patch dropping the spin_unlock/spin_lock
> around uart_write_wakeup.
> Is that ok with you.

Anand, before doing that, can you check Peter's second sentence? I
mean the "If this is causing lock-ups in a line discipline, the line
discipline needs fixed.".
Don't drop the spin-locks "just because". I would be happy to see more
detailed explanation in changelog.

Best regards,
Krzysztof

Re: [PATCH] serial: samsung: fix the inconsistency in spinlock

[Anand Moon]

Hi Krzysztof,

On 19 February 2016 at 11:39, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> 2016-02-19 4:14 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>> Hi Peter,
>>
>> On 18 February 2016 at 23:18, Peter Hurley <peter@hurleysoftware.com> wrote:
>>> Hi Anand,
>>>
>>> On 02/18/2016 09:40 AM, Anand Moon wrote:
>>>> From: Anand Moon <linux.amoon@gmail.com>
>>>>
>>>> changes fix the correct order of the spin_lock_irqrestore/save.
>>>>
>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>> ---
>>>>  drivers/tty/serial/samsung.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>>>> index d72cd73..96fe14d 100644
>>>> --- a/drivers/tty/serial/samsung.c
>>>> +++ b/drivers/tty/serial/samsung.c
>>>> @@ -759,9 +759,9 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
>>>>       }
>>>>
>>>>       if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) {
>>>> -             spin_unlock(&port->lock);
>>>> +             spin_unlock_irqrestore(&port->lock, flags);
>>>>               uart_write_wakeup(port);
>>>> -             spin_lock(&port->lock);
>>>> +             spin_lock_irqsave(&port->lock, flags);
>>>
>>> This driver shouldn't be dropping the spin lock at for write wakeup.
>>> If this is causing lock-ups in a line discipline, the line discipline
>>> needs fixed.
>>>
>>
>> Thanks for pointing out.
>> Their is no lock up, just the inconstancy of the spin_lock.
>> Then I will resend this patch dropping the spin_unlock/spin_lock
>> around uart_write_wakeup.
>> Is that ok with you.
>
> Anand, before doing that, can you check Peter's second sentence? I
> mean the "If this is causing lock-ups in a line discipline, the line
> discipline needs fixed.".
> Don't drop the spin-locks "just because". I would be happy to see more
> detailed explanation in changelog.
>
> Best regards,
> Krzysztof

Yes I understood the meaning of the sentence. Already the
s3c24xx_serial_tx_chars function.
holds the lock port->lock for safe IRQ execution.

http://lxr.free-electrons.com/source/drivers/tty/serial/samsung.c#L701

Best Regards
-Anand Moon

Re: [PATCH] serial: samsung: fix the inconsistency in spinlock

[Krzysztof Kozlowski]

On 19.02.2016 15:51, Anand Moon wrote:
> Hi Krzysztof,
> 
> On 19 February 2016 at 11:39, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> 2016-02-19 4:14 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>> Hi Peter,
>>>
>>> On 18 February 2016 at 23:18, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>> Hi Anand,
>>>>
>>>> On 02/18/2016 09:40 AM, Anand Moon wrote:
>>>>> From: Anand Moon <linux.amoon@gmail.com>
>>>>>
>>>>> changes fix the correct order of the spin_lock_irqrestore/save.
>>>>>
>>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>>> ---
>>>>>  drivers/tty/serial/samsung.c | 4 ++--
>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>>>>> index d72cd73..96fe14d 100644
>>>>> --- a/drivers/tty/serial/samsung.c
>>>>> +++ b/drivers/tty/serial/samsung.c
>>>>> @@ -759,9 +759,9 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
>>>>>       }
>>>>>
>>>>>       if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) {
>>>>> -             spin_unlock(&port->lock);
>>>>> +             spin_unlock_irqrestore(&port->lock, flags);
>>>>>               uart_write_wakeup(port);
>>>>> -             spin_lock(&port->lock);
>>>>> +             spin_lock_irqsave(&port->lock, flags);
>>>>
>>>> This driver shouldn't be dropping the spin lock at for write wakeup.
>>>> If this is causing lock-ups in a line discipline, the line discipline
>>>> needs fixed.
>>>>
>>>
>>> Thanks for pointing out.
>>> Their is no lock up, just the inconstancy of the spin_lock.
>>> Then I will resend this patch dropping the spin_unlock/spin_lock
>>> around uart_write_wakeup.
>>> Is that ok with you.
>>
>> Anand, before doing that, can you check Peter's second sentence? I
>> mean the "If this is causing lock-ups in a line discipline, the line
>> discipline needs fixed.".
>> Don't drop the spin-locks "just because". I would be happy to see more
>> detailed explanation in changelog.
>>
>> Best regards,
>> Krzysztof
> 
> Yes I understood the meaning of the sentence. Already the
> s3c24xx_serial_tx_chars function.
> holds the lock port->lock for safe IRQ execution.

I am sorry but I don't get your explanation. I mentioned Peter's
thoughts about lockups after adding locking over uart_write(). However
you are referring to s3c24xx_serial_tx_chars() holding the spin lock...
I am missing the point...

Best regards,
Krzysztof

Re: [PATCH] serial: samsung: fix the inconsistency in spinlock

[Anand Moon]

Hi Krzysztof,

On 19 February 2016 at 13:14, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> On 19.02.2016 15:51, Anand Moon wrote:
>> Hi Krzysztof,
>>
>> On 19 February 2016 at 11:39, Krzysztof Kozlowski
>> <k.kozlowski@samsung.com> wrote:
>>> 2016-02-19 4:14 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>>> Hi Peter,
>>>>
>>>> On 18 February 2016 at 23:18, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>>> Hi Anand,
>>>>>
>>>>> On 02/18/2016 09:40 AM, Anand Moon wrote:
>>>>>> From: Anand Moon <linux.amoon@gmail.com>
>>>>>>
>>>>>> changes fix the correct order of the spin_lock_irqrestore/save.
>>>>>>
>>>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>>>> ---
>>>>>>  drivers/tty/serial/samsung.c | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>>>>>> index d72cd73..96fe14d 100644
>>>>>> --- a/drivers/tty/serial/samsung.c
>>>>>> +++ b/drivers/tty/serial/samsung.c
>>>>>> @@ -759,9 +759,9 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
>>>>>>       }
>>>>>>
>>>>>>       if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) {
>>>>>> -             spin_unlock(&port->lock);
>>>>>> +             spin_unlock_irqrestore(&port->lock, flags);
>>>>>>               uart_write_wakeup(port);
>>>>>> -             spin_lock(&port->lock);
>>>>>> +             spin_lock_irqsave(&port->lock, flags);
>>>>>
>>>>> This driver shouldn't be dropping the spin lock at for write wakeup.
>>>>> If this is causing lock-ups in a line discipline, the line discipline
>>>>> needs fixed.
>>>>>
>>>>
>>>> Thanks for pointing out.
>>>> Their is no lock up, just the inconstancy of the spin_lock.
>>>> Then I will resend this patch dropping the spin_unlock/spin_lock
>>>> around uart_write_wakeup.
>>>> Is that ok with you.
>>>
>>> Anand, before doing that, can you check Peter's second sentence? I
>>> mean the "If this is causing lock-ups in a line discipline, the line
>>> discipline needs fixed.".
>>> Don't drop the spin-locks "just because". I would be happy to see more
>>> detailed explanation in changelog.
>>>
>>> Best regards,
>>> Krzysztof
>>
>> Yes I understood the meaning of the sentence. Already the
>> s3c24xx_serial_tx_chars function.
>> holds the lock port->lock for safe IRQ execution.
>
> I am sorry but I don't get your explanation. I mentioned Peter's
> thoughts about lockups after adding locking over uart_write(). However
> you are referring to s3c24xx_serial_tx_chars() holding the spin lock...
> I am missing the point...
>
> Best regards,
> Krzysztof
>

I should be sorry I could not explain you in technical terms.
Interrupt routine already hold the port->lock

s3c24xx_serial_tx_chars
     \
     spin_lock_irqsave(&port->lock, flags);
     \...
    spin_unlock(&port->lock);
     uart_write_wakeup(port);
     spin_lock(&port->lock);
     \
     spin_unlock_irqrestore(&port->lock, flags);

In my next patch I have tried to remove the spin_unlock/spin_lock over
uart_write_wakeup(port);

Best Regards.
-Anand Moon

Re: [PATCH] serial: samsung: fix the inconsistency in spinlock

[Krzysztof Kozlowski]

On 19.02.2016 17:23, Anand Moon wrote:
> Hi Krzysztof,
> 
> On 19 February 2016 at 13:14, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> On 19.02.2016 15:51, Anand Moon wrote:
>>> Hi Krzysztof,
>>>
>>> On 19 February 2016 at 11:39, Krzysztof Kozlowski
>>> <k.kozlowski@samsung.com> wrote:
>>>> 2016-02-19 4:14 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>>>> Hi Peter,
>>>>>
>>>>> On 18 February 2016 at 23:18, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>>>> Hi Anand,
>>>>>>
>>>>>> On 02/18/2016 09:40 AM, Anand Moon wrote:
>>>>>>> From: Anand Moon <linux.amoon@gmail.com>
>>>>>>>
>>>>>>> changes fix the correct order of the spin_lock_irqrestore/save.
>>>>>>>
>>>>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>>>>> ---
>>>>>>>  drivers/tty/serial/samsung.c | 4 ++--
>>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>>>>>>> index d72cd73..96fe14d 100644
>>>>>>> --- a/drivers/tty/serial/samsung.c
>>>>>>> +++ b/drivers/tty/serial/samsung.c
>>>>>>> @@ -759,9 +759,9 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
>>>>>>>       }
>>>>>>>
>>>>>>>       if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) {
>>>>>>> -             spin_unlock(&port->lock);
>>>>>>> +             spin_unlock_irqrestore(&port->lock, flags);
>>>>>>>               uart_write_wakeup(port);
>>>>>>> -             spin_lock(&port->lock);
>>>>>>> +             spin_lock_irqsave(&port->lock, flags);
>>>>>>
>>>>>> This driver shouldn't be dropping the spin lock at for write wakeup.
>>>>>> If this is causing lock-ups in a line discipline, the line discipline
>>>>>> needs fixed.
>>>>>>
>>>>>
>>>>> Thanks for pointing out.
>>>>> Their is no lock up, just the inconstancy of the spin_lock.
>>>>> Then I will resend this patch dropping the spin_unlock/spin_lock
>>>>> around uart_write_wakeup.
>>>>> Is that ok with you.
>>>>
>>>> Anand, before doing that, can you check Peter's second sentence? I
>>>> mean the "If this is causing lock-ups in a line discipline, the line
>>>> discipline needs fixed.".
>>>> Don't drop the spin-locks "just because". I would be happy to see more
>>>> detailed explanation in changelog.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>
>>> Yes I understood the meaning of the sentence. Already the
>>> s3c24xx_serial_tx_chars function.
>>> holds the lock port->lock for safe IRQ execution.
>>
>> I am sorry but I don't get your explanation. I mentioned Peter's
>> thoughts about lockups after adding locking over uart_write(). However
>> you are referring to s3c24xx_serial_tx_chars() holding the spin lock...
>> I am missing the point...
>>
>> Best regards,
>> Krzysztof
>>
> 
> I should be sorry I could not explain you in technical terms.
> Interrupt routine already hold the port->lock
> 
> s3c24xx_serial_tx_chars
>      \
>      spin_lock_irqsave(&port->lock, flags);
>      \...
>     spin_unlock(&port->lock);
>      uart_write_wakeup(port);
>      spin_lock(&port->lock);
>      \
>      spin_unlock_irqrestore(&port->lock, flags);
> 

This is obvious.

> In my next patch I have tried to remove the spin_unlock/spin_lock over
> uart_write_wakeup(port);

Which may create lockups. Previously there was no port locking around
uart_write_wakeup. Now there will be. You are effectively adding locking
over uart_write_wakeup().
Again, we are back at the Peter's message - just check the damned lockups...

BR,
Krzysztof

BR

Re: [PATCH] serial: samsung: fix the inconsistency in spinlock

[Anand Moon]

Hi Krzysztof,

On 19 February 2016 at 13:57, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> On 19.02.2016 17:23, Anand Moon wrote:
>> Hi Krzysztof,
>>
>> On 19 February 2016 at 13:14, Krzysztof Kozlowski
>> <k.kozlowski@samsung.com> wrote:
>>> On 19.02.2016 15:51, Anand Moon wrote:
>>>> Hi Krzysztof,
>>>>
>>>> On 19 February 2016 at 11:39, Krzysztof Kozlowski
>>>> <k.kozlowski@samsung.com> wrote:
>>>>> 2016-02-19 4:14 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>>>>> Hi Peter,
>>>>>>
>>>>>> On 18 February 2016 at 23:18, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>>>>> Hi Anand,
>>>>>>>
>>>>>>> On 02/18/2016 09:40 AM, Anand Moon wrote:
>>>>>>>> From: Anand Moon <linux.amoon@gmail.com>
>>>>>>>>
>>>>>>>> changes fix the correct order of the spin_lock_irqrestore/save.
>>>>>>>>
>>>>>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>>>>>> ---
>>>>>>>>  drivers/tty/serial/samsung.c | 4 ++--
>>>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>>>>>>>> index d72cd73..96fe14d 100644
>>>>>>>> --- a/drivers/tty/serial/samsung.c
>>>>>>>> +++ b/drivers/tty/serial/samsung.c
>>>>>>>> @@ -759,9 +759,9 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
>>>>>>>>       }
>>>>>>>>
>>>>>>>>       if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) {
>>>>>>>> -             spin_unlock(&port->lock);
>>>>>>>> +             spin_unlock_irqrestore(&port->lock, flags);
>>>>>>>>               uart_write_wakeup(port);
>>>>>>>> -             spin_lock(&port->lock);
>>>>>>>> +             spin_lock_irqsave(&port->lock, flags);
>>>>>>>
>>>>>>> This driver shouldn't be dropping the spin lock at for write wakeup.
>>>>>>> If this is causing lock-ups in a line discipline, the line discipline
>>>>>>> needs fixed.
>>>>>>>
>>>>>>
>>>>>> Thanks for pointing out.
>>>>>> Their is no lock up, just the inconstancy of the spin_lock.
>>>>>> Then I will resend this patch dropping the spin_unlock/spin_lock
>>>>>> around uart_write_wakeup.
>>>>>> Is that ok with you.
>>>>>
>>>>> Anand, before doing that, can you check Peter's second sentence? I
>>>>> mean the "If this is causing lock-ups in a line discipline, the line
>>>>> discipline needs fixed.".
>>>>> Don't drop the spin-locks "just because". I would be happy to see more
>>>>> detailed explanation in changelog.
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>
>>>> Yes I understood the meaning of the sentence. Already the
>>>> s3c24xx_serial_tx_chars function.
>>>> holds the lock port->lock for safe IRQ execution.
>>>
>>> I am sorry but I don't get your explanation. I mentioned Peter's
>>> thoughts about lockups after adding locking over uart_write(). However
>>> you are referring to s3c24xx_serial_tx_chars() holding the spin lock...
>>> I am missing the point...
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> I should be sorry I could not explain you in technical terms.
>> Interrupt routine already hold the port->lock
>>
>> s3c24xx_serial_tx_chars
>>      \
>>      spin_lock_irqsave(&port->lock, flags);
>>      \...
>>     spin_unlock(&port->lock);
>>      uart_write_wakeup(port);
>>      spin_lock(&port->lock);
>>      \
>>      spin_unlock_irqrestore(&port->lock, flags);
>>
>
> This is obvious.
>
>> In my next patch I have tried to remove the spin_unlock/spin_lock over
>> uart_write_wakeup(port);
>
> Which may create lockups. Previously there was no port locking around
> uart_write_wakeup. Now there will be. You are effectively adding locking
> over uart_write_wakeup().
> Again, we are back at the Peter's message - just check the damned lockups...
>
> BR,
> Krzysztof
>
> BR
>

Lets drop this patch. I have send new one earlier.
https://lkml.org/lkml/2016/2/19/2

If you have any comment on that.
Sorry for the confusion.

Best Regards.
-Anand Moon

Re: [PATCH] serial: samsung: fix the inconsistency in spinlock

[Krzysztof Kozlowski]

>>> In my next patch I have tried to remove the spin_unlock/spin_lock over
>>> uart_write_wakeup(port);
>>
>> Which may create lockups. Previously there was no port locking around
>> uart_write_wakeup. Now there will be. You are effectively adding locking
>> over uart_write_wakeup().
>> Again, we are back at the Peter's message - just check the damned lockups...
>>
>> BR,
>> Krzysztof
>>
>> BR
>>
>
> Lets drop this patch. I have send new one earlier.
> https://lkml.org/lkml/2016/2/19/2
>
> If you have any comment on that.
> Sorry for the confusion.

I was commenting your future patch about dropping spinlocks.
Apparently there is huge misunderstanding here...

Best regards,
Krzysztof