Re: [PATCH v2 0/2] drm/panel: Add support for Raydium RM68200 panel

2018-03-12 Thread Philippe CORNU
Hi Thierry,

On 03/12/2018 09:04 AM, Thierry Reding wrote:
> On Fri, Mar 02, 2018 at 04:32:20PM +0100, Philippe Cornu wrote:
>> The Raydium Semiconductor Corporation RM68200 is a 5.5" 720x1280
>> TFT LCD panel connected using a MIPI-DSI video interface.
>>
>> Version 2:
>> - Add Rob Herring Reviewed-by on dt-bindings.
>> - Update Kconfig & driver thanks to Thierry Reding comments: no more
>>DRV_NAME, DRM_WARN_ONCE instead of DRV_NAME where applicable, use
>>backlight_enable/disable() & devm_of_find_backlight(), no extra
>>gpio reset to 0, no more msg if successful, use RM68200 instead of
>>rm68200 where necessary.
>>
>> Version 1:
>> - Initial commit
>>
>> Philippe Cornu (2):
>>dt-bindings/display/panel: Add support for Raydium rm68200 dsi panel
>>drm/panel: Add support for Raydium RM68200 panel driver
>>
>>   .../bindings/display/panel/raydium,rm68200.txt |  25 ++
>>   drivers/gpu/drm/panel/Kconfig  |   8 +
>>   drivers/gpu/drm/panel/Makefile |   1 +
>>   drivers/gpu/drm/panel/panel-raydium-rm68200.c  | 437 
>> +
>>   4 files changed, 471 insertions(+)
>>   create mode 100644 
>> Documentation/devicetree/bindings/display/panel/raydium,rm68200.txt
>>   create mode 100755 drivers/gpu/drm/panel/panel-raydium-rm68200.c
> 
> Applied, thanks. There were a few tiny things I wasn't entirely happy
> about, but I fixed those up myself.
> 
> Most notable was the use of DRM_WARN_ONCE(), which is bad in this case
> because it won't show any subsequent errors, ever again. I think in this
> case the proper solution is DRM_ERROR_RATELIMITED(), or erroring out on
> the first error, assuming that subsequent operations won't succeed
> either. I went with the rate limitation.

Big thank you for your nice cleanups and reworks, I will follow those in 
the future :-)

I take the opportunity to thank you also for the merge of the 2 otm8009a 
patches.

Moreover, if you think I need to update otm8009a driver with the same 
reworks (include order, DRM_ERROR_RATELIMITED, error msg, remove msg if 
successfull, struct data alignment...), please tell me :)
Note: I plan to update it soon with new backlight_enable/disable api...

Thank you again,
Philippe :-)

> 
> Thierry
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 0/2] drm/panel: Add support for Raydium RM68200 panel

2018-03-12 Thread Thierry Reding
On Fri, Mar 02, 2018 at 04:32:20PM +0100, Philippe Cornu wrote:
> The Raydium Semiconductor Corporation RM68200 is a 5.5" 720x1280
> TFT LCD panel connected using a MIPI-DSI video interface.
> 
> Version 2:
> - Add Rob Herring Reviewed-by on dt-bindings.
> - Update Kconfig & driver thanks to Thierry Reding comments: no more
>   DRV_NAME, DRM_WARN_ONCE instead of DRV_NAME where applicable, use
>   backlight_enable/disable() & devm_of_find_backlight(), no extra
>   gpio reset to 0, no more msg if successful, use RM68200 instead of
>   rm68200 where necessary.
> 
> Version 1:
> - Initial commit
> 
> Philippe Cornu (2):
>   dt-bindings/display/panel: Add support for Raydium rm68200 dsi panel
>   drm/panel: Add support for Raydium RM68200 panel driver
> 
>  .../bindings/display/panel/raydium,rm68200.txt |  25 ++
>  drivers/gpu/drm/panel/Kconfig  |   8 +
>  drivers/gpu/drm/panel/Makefile |   1 +
>  drivers/gpu/drm/panel/panel-raydium-rm68200.c  | 437 
> +
>  4 files changed, 471 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/raydium,rm68200.txt
>  create mode 100755 drivers/gpu/drm/panel/panel-raydium-rm68200.c

Applied, thanks. There were a few tiny things I wasn't entirely happy
about, but I fixed those up myself.

Most notable was the use of DRM_WARN_ONCE(), which is bad in this case
because it won't show any subsequent errors, ever again. I think in this
case the proper solution is DRM_ERROR_RATELIMITED(), or erroring out on
the first error, assuming that subsequent operations won't succeed
either. I went with the rate limitation.

Thierry


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel