Hi Vinay,
On 17 June 2016 at 19:23, Vinay Simha BN simhavcs@gmail.com wrote:
v6:
- emil review comments incorporated PANEL_NUM_REGULATORS dropped, return ret added at necessary places, if checks dropped for backlight and gpios
Looks like some of my suggestions went below the radar. Did you miss them or I've I lost the plot somewhere ? In case of the latter don't be shy to point out :-)
+struct jdi_panel {
struct drm_panel base;struct mipi_dsi_device *dsi;struct regulator_bulk_data supplies[3];
struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
+static int jdi_panel_off(struct jdi_panel *jdi) +{
struct mipi_dsi_device *dsi = jdi->dsi;int ret;dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;ret = mipi_dsi_dcs_set_display_off(dsi);if (ret < 0)return ret;
IMHO neither this function nor its caller jdi_panel_unprepare() should stop in the middle/return prematurely.
Or in other words - one should change the function return type to void and drop the returns.
+static int jdi_panel_unprepare(struct drm_panel *panel) +{
struct jdi_panel *jdi = to_jdi_panel(panel);struct device *dev = &jdi->dsi->dev;int ret;if (!jdi->prepared)return 0;ret = jdi_panel_off(jdi);if (ret) {dev_err(panel->dev, "failed to set panel off: %d\n", ret);return ret;
As suggested above, drop this return.
+static int jdi_panel_prepare(struct drm_panel *panel) +{
struct jdi_panel *jdi = to_jdi_panel(panel);struct device *dev = &jdi->dsi->dev;int ret;if (jdi->prepared)return 0;ret = regulator_bulk_enable(ARRAY_SIZE(jdi->supplies), jdi->supplies);if (ret < 0) {dev_err(dev, "regulator enable failed, %d\n", ret);return ret;}msleep(20);if (jdi->reset_gpio) {
You can drop the check.
gpiod_set_value(jdi->reset_gpio, 1);usleep_range(10, 20);}if (jdi->enable_gpio) {
Ditto.
+poweroff:
gpiod_set_value(jdi->enable_gpio, 0);gpiod_set_value(jdi->reset_gpio, 0);
Just noticed that you're missing regulator_bulk_disable() here.
Regards, Emil