Weird/Unneeded call to msleep in exynos_mipi_dsi_wr_data in exynos_mipi_dsi_common.c

Weird/Unneeded call to msleep in exynos_mipi_dsi_wr_data in exynos_mipi_dsi_common.c

[nick]

Greetings Fellow Maintainers,
Sorry if I wasting your time but it seems there is a unneeded call to msleep and rather trivial fix me to fix in the file,exynos_mipi_dsi_common.c for the function,exynos_mipi_dsi_wr_data . If there is a valid reason for this call please let me known when any of you  have some free time. Otherwise I will send in a patch to remove this unneeded
call to msleep.
Thanks,
Nick  

Weird/Unneeded call to msleep in exynos_mipi_dsi_wr_data in exynos_mipi_dsi_common.c

[Krzysztof Kozłowski]

On 17.12.2014 23:57, nick wrote:
> Greetings Fellow Maintainers,
> Sorry if I wasting your time but it seems there is a unneeded call to msleep

Hi,

1. Please describe exactly why do you think this is not needed.
2. Do you have Exynos-based hardware to test your changes?

Best regards,
Krzysztof

> and rather trivial fix me to fix in the file,exynos_mipi_dsi_common.c for the function,exynos_mipi_dsi_wr_data . If there is a valid reason for this call please let me known when any of you  have some free time. Otherwise I will send in a patch to remove this unneeded
> call to msleep.
> Thanks,
> Nick  

Weird/Unneeded call to msleep in exynos_mipi_dsi_wr_data in exynos_mipi_dsi_common.c

[nick]

Krzysztof,
If we look at the code for this function, it already is handling the data correctly. In addition  the locks 
seem to be better protection then msleep. Further more is no reason for this delay as we are neither resetting 
the hardware or waiting for the hardware here so why is it needed? I don't have Exynos based hardware lying
around through to test it.
Nick
On 2014-12-18 05:26 AM, Krzysztof Koz?owski wrote:
> On 17.12.2014 23:57, nick wrote:
>> Greetings Fellow Maintainers,
>> Sorry if I wasting your time but it seems there is a unneeded call to msleep
> 
> Hi,
> 
> 1. Please describe exactly why do you think this is not needed.
> 2. Do you have Exynos-based hardware to test your changes?
> 
> Best regards,
> Krzysztof
> 
>> and rather trivial fix me to fix in the file,exynos_mipi_dsi_common.c for the function,exynos_mipi_dsi_wr_data . If there is a valid reason for this call please let me known when any of you  have some free time. Otherwise I will send in a patch to remove this unneeded
>> call to msleep.
>> Thanks,
>> Nick  
> 
> 

Weird/Unneeded call to msleep in exynos_mipi_dsi_wr_data in exynos_mipi_dsi_common.c

[Lucas Stach]

Am Donnerstag, den 18.12.2014, 08:35 -0500 schrieb nick:
> Krzysztof,
> If we look at the code for this function, it already is handling the data correctly. In addition  the locks 
> seem to be better protection then msleep. Further more is no reason for this delay as we are neither resetting 
> the hardware or waiting for the hardware here so why is it needed? I don't have Exynos based hardware lying
> around through to test it.

If you can't test it, don't touch it. It's that simple.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Weird/Unneeded call to msleep in exynos_mipi_dsi_wr_data in exynos_mipi_dsi_common.c

[nick]

Lucas,
That's fair do you known of anyone who does have the hardware so we can test my patch. If you do then we can get this fixed rather
easily.
Cheers Nick 

On 2014-12-18 08:39 AM, Lucas Stach wrote:
> Am Donnerstag, den 18.12.2014, 08:35 -0500 schrieb nick:
>> Krzysztof,
>> If we look at the code for this function, it already is handling the data correctly. In addition  the locks 
>> seem to be better protection then msleep. Further more is no reason for this delay as we are neither resetting 
>> the hardware or waiting for the hardware here so why is it needed? I don't have Exynos based hardware lying
>> around through to test it.
> 
> If you can't test it, don't touch it. It's that simple.
> 
> Regards,
> Lucas
> 

Weird/Unneeded call to msleep in exynos_mipi_dsi_wr_data in exynos_mipi_dsi_common.c

[Tomi Valkeinen]

On 18/12/14 15:48, nick wrote:
> Lucas,
> That's fair do you known of anyone who does have the hardware so we can test my patch. If you do then we can get this fixed rather
> easily.
> Cheers Nick 
> 
> On 2014-12-18 08:39 AM, Lucas Stach wrote:
>> Am Donnerstag, den 18.12.2014, 08:35 -0500 schrieb nick:
>>> Krzysztof,
>>> If we look at the code for this function, it already is handling the data correctly. In addition  the locks 
>>> seem to be better protection then msleep. Further more is no reason for this delay as we are neither resetting 
>>> the hardware or waiting for the hardware here so why is it needed? I don't have Exynos based hardware lying
>>> around through to test it.
>>
>> If you can't test it, don't touch it. It's that simple.

There seems to be multiple msleep(20)s in exynos_mipi_dsi_common.c, a
few with /* FIXME */ and a few without any comments. Looks like bad (but
relatively harmless) code to me, but as Lucas said, if you can't test
it, don't touch it.

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141218/0ff61970/attachment.sig>

Weird/Unneeded call to msleep in exynos_mipi_dsi_wr_data in exynos_mipi_dsi_common.c

[nick]

On 2014-12-18 09:05 AM, Tomi Valkeinen wrote:
> On 18/12/14 15:48, nick wrote:
>> Lucas,
>> That's fair do you known of anyone who does have the hardware so we can test my patch. If you do then we can get this fixed rather
>> easily.
>> Cheers Nick 
>>
>> On 2014-12-18 08:39 AM, Lucas Stach wrote:
>>> Am Donnerstag, den 18.12.2014, 08:35 -0500 schrieb nick:
>>>> Krzysztof,
>>>> If we look at the code for this function, it already is handling the data correctly. In addition  the locks 
>>>> seem to be better protection then msleep. Further more is no reason for this delay as we are neither resetting 
>>>> the hardware or waiting for the hardware here so why is it needed? I don't have Exynos based hardware lying
>>>> around through to test it.
>>>
>>> If you can't test it, don't touch it. It's that simple.
> 
> There seems to be multiple msleep(20)s in exynos_mipi_dsi_common.c, a
> few with /* FIXME */ and a few without any comments. Looks like bad (but
> relatively harmless) code to me, but as Lucas said, if you can't test
> it, don't touch it.
> 
>  Tomi
> 
> 
Tomi,
That is totally fair, sorry for wasting your time :(. I was just trying to help out.
Cheers Nick