Hi Paweł
Looks good, thanks for addressing all the review feedback.
On Fri, Feb 01, 2019 at 06:28:52PM +0100, Paweł Chmiel wrote:
This patch adds Samsung S6E63M0 AMOLED LCD panel driver, connected over spi. It's based on already removed, non dt s6e63m0 driver and panel-samsung-ld9040. It can be found for example in some of Samsung Aries based phones.
Signed-off-by: Paweł Chmiel pawel.mikolaj.chmiel@gmail.com
If you consider (do not change unless you think it better) the following nits than you can add my:
Reviewed-by: Sam Ravnborg sam@ravnborg.org
Sam
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 3f3537719beb..be05ed5218eb 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -158,6 +158,13 @@ config DRM_PANEL_SAMSUNG_S6E63J0X03 depends on BACKLIGHT_CLASS_DEVICE select VIDEOMODE_HELPERS
+config DRM_PANEL_SAMSUNG_S6E63M0
- tristate "Samsung S6E63M0 RGB/SPI panel"
- depends on OF
- depends on SPI
- depends on BACKLIGHT_CLASS_DEVICE
- select VIDEOMODE_HELPERS
With the use of display_mode the above "select VIDEOMODE_HELPERS" is likely no longer required. Please check.
A help text would be nice.
config DRM_PANEL_SAMSUNG_S6E8AA0 tristate "Samsung S6E8AA0 DSI video mode panel" depends on OF
+#include <video/of_videomode.h> +#include <video/videomode.h>
Please check if these two files are required.
+struct s6e63m0 {
- struct device *dev;
- struct drm_panel panel;
- struct backlight_device *bl_dev;
- struct regulator_bulk_data supplies[2];
- struct gpio_desc *reset_gpio;
- struct videomode vm;
vm is no longer used - delete it.
- bool prepared;
- bool enabled;
- /*
* This field is tested by functions directly accessing bus before* transfer, transfer is skipped if it is set. In case of transfer* failure or unexpected response the field is set to error value.* Such construct allows to eliminate many checks in higher level* functions.*/- int error;
+};
+static int s6e63m0_get_modes(struct drm_panel *panel) +{
- struct drm_connector *connector = panel->connector;
- struct drm_display_mode *mode;
- mode = drm_mode_duplicate(panel->drm, &default_mode);
- if (!mode) {
DRM_ERROR("failed to add mode %ux%ux@%u\n",default_mode.hdisplay, default_mode.vdisplay,default_mode.vrefresh);
I recall I have seen a generic way to print the above, but I have failed to find it. Maybe it is just my memory that fools me.
The above is fine.
- s6e63m0_backlight_register(ctx);
Is it correct that we continue even if we fail to register backlight?
- return 0;
+}
+static const struct of_device_id s6e63m0_of_match[] = {
- { .compatible = "samsung,s6e63m0" },
- { }
Add /* sentinel */ comment?
+};