Hi Laurent.
Good to move omapdrm to a more standard way to do things.
new file mode 100644 index 000000000000..d8a8c3a3a8c5 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-lg-lb035q02.c @@ -0,0 +1,235 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- LG.Philips LB035Q02 LCD Panel Driver
Looks like a typo. As far as I know LG and Philips are not the same. But I can see this is used in several places, so I need to check up on actual status here and driver is likely OK. Google... this is fine. Some joint venture in 2001.
- Based on the omapdrm-specific panel-lg-lb035q02 driver
Will we have two drivers with the same name, or are this above already disabled from the build?
- unsigned int i;
index to arrays are default "int" IIRC. Not that it matters but noticed it.
- int ret;
- for (i = 0; i < ARRAY_SIZE(init_data); ++i) {
ret = lb035q02_write(lcd, init_data[i].index,init_data[i].value);if (ret < 0)return ret;- }
- return 0;
+}
+static const struct drm_display_mode lb035q02_mode = {
- .clock = 6500,
- .hdisplay = 320,
- .hsync_start = 320 + 20,
- .hsync_end = 320 + 20 + 2,
- .htotal = 320 + 20 + 2 + 68,
- .vdisplay = 240,
- .vsync_start = 240 + 4,
- .vsync_end = 240 + 4 + 2,
- .vtotal = 240 + 4 + 2 + 18,
- .vrefresh = 60,
- .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
- .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
We already specify all the timing details. Consider to use display_mode to specify the width/height too. So the panel specificatiosn are hardcoded only in one place.
+static int lb035q02_get_modes(struct drm_panel *panel) +{
- struct drm_connector *connector = panel->connector;
- struct drm_display_mode *mode;
- mode = drm_mode_duplicate(panel->drm, &lb035q02_mode);
- if (!mode)
return -ENOMEM;- drm_mode_set_name(mode);
- drm_mode_probed_add(connector, mode);
- connector->display_info.width_mm = 70;
- connector->display_info.height_mm = 53;
So we avoid hardcoding height/width here, but do it with the timing above.
- /*
* FIXME: According to the datasheet pixel data is sampled on the* rising edge of the clock, but the code running on the Gumstix Overo* Palo35 indicates sampling on the negative edge. This should be* tested on a real device.*/- connector->display_info.bus_flags = DRM_BUS_FLAG_DE_HIGH
| DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE| DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE;- return 1;
+}
+static const struct drm_panel_funcs lb035q02_funcs = {
- .disable = lb035q02_disable,
- .enable = lb035q02_enable,
- .get_modes = lb035q02_get_modes,
+};
+static int lb035q02_probe(struct spi_device *spi) +{
- struct lb035q02_device *lcd;
- int ret;
- lcd = devm_kzalloc(&spi->dev, sizeof(*lcd), GFP_KERNEL);
- if (lcd == NULL)
return -ENOMEM;- spi_set_drvdata(spi, lcd);
- lcd->spi = spi;
- lcd->enable_gpio = devm_gpiod_get(&spi->dev, "enable", GPIOD_OUT_LOW);
- if (IS_ERR(lcd->enable_gpio)) {
dev_err(&spi->dev, "failed to parse enable gpio\n");return PTR_ERR(lcd->enable_gpio);- }
- ret = lb035q02_init(lcd);
- if (ret < 0)
return ret;- drm_panel_init(&lcd->panel);
- lcd->panel.dev = &lcd->spi->dev;
- lcd->panel.funcs = &lb035q02_funcs;
- return drm_panel_add(&lcd->panel);
+}
+static int lb035q02_remove(struct spi_device *spi) +{
- struct lb035q02_device *lcd = spi_get_drvdata(spi);
- drm_panel_remove(&lcd->panel);
- lb035q02_disable(&lcd->panel);
Use drm_panel_disable() so the driver will benefit when we move more functionality to the drm_panel_disable() function.
- return 0;
+}
+static const struct of_device_id lb035q02_of_match[] = {
- { .compatible = "lgphilips,lb035q02", },
- {},
Some drivers use { /* sentinel */ }, to document this is on purpose the last entry.
+};
+MODULE_DEVICE_TABLE(of, lb035q02_of_match);
+static struct spi_driver lb035q02_driver = {
- .probe = lb035q02_probe,
- .remove = lb035q02_remove,
- .driver = {
.name = "panel-lg-lb035q02",.of_match_table = lb035q02_of_match,- },
+};
+module_spi_driver(lb035q02_driver);
+MODULE_ALIAS("spi:lgphilips,lb035q02"); +MODULE_AUTHOR("Tomi Valkeinen tomi.valkeinen@ti.com"); +MODULE_DESCRIPTION("LG.Philips LB035Q02 LCD Panel driver"); +MODULE_LICENSE("GPL");
This should be "GPL v2" if I read https://www.kernel.org/doc/html/latest/process/license-rules.html correct. See "MODULE_LICENSE" table.
With the above comments addressed/considered: Reviewed-by: Sam Ravnborg sam@ravnborg.org
Sam