Re: [PATCH v2 1/2] drivers: gpu: drm: add driver for samsung s6e3fc2x01 cmd mode panel
Hi,
On 07.10.2022 14:14, Nia Espera wrote:
+
+#define dsi_dcs_write_seq(dsi, seq...) do {\
+ static const u8 d[] = { seq }; \
+ int ret;\
+ ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \
+ if (ret < 0) \
+ return ret; \
+ } while (0)
+
There is now a standard macro for this - mipi_dsi_dcs_write_seq() [1] ,
so you don't need to reinvent it.
+static void samsung_s6e3fc2x01_reset(struct samsung_s6e3fc2x01 *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(5000, 6000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(2000, 3000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(1, 11000);
+}
There is a high chance that first gpiod_set_value() is not needed, only
the last 2.
[1] https://elixir.bootlin.com/linux/latest/C/ident/mipi_dsi_dcs_write_seq
--
Regards
Alexey Minnekhanov
Re: [PATCH v2 1/2] drivers: gpu: drm: add driver for samsung s6e3fc2x01 cmd mode panel
Hi Nia,
On Fri, Oct 7, 2022 at 8:16 AM Nia Espera wrote:
> +static int samsung_s6e3fc2x01_prepare(struct drm_panel *panel)
> +{
> + struct samsung_s6e3fc2x01 *ctx = to_samsung_s6e3fc2x01(panel);
> + struct device *dev = >dsi->dev;
> + int ret;
> +
> + if (ctx->prepared)
> + return 0;
> +
> + ret = regulator_enable(ctx->supply);
> + if (ret < 0) {
> + dev_err(dev, "Failed to enable regulator: %d\n", ret);
> + return ret;
> + }
> +
> + samsung_s6e3fc2x01_reset(ctx);
> +
> + ret = samsung_s6e3fc2x01_on(ctx);
> + if (ret < 0) {
> + dev_err(dev, "Failed to initialize panel: %d\n", ret);
> + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
You should also call regulator_disable() here in the case of failure.
> +static int samsung_s6e3fc2x01_unprepare(struct drm_panel *panel)
> +{
> + struct samsung_s6e3fc2x01 *ctx = to_samsung_s6e3fc2x01(panel);
> + struct device *dev = >dsi->dev;
> + int ret;
> +
> + if (!ctx->prepared)
> + return 0;
> +
> + ret = samsung_s6e3fc2x01_off(ctx);
> + if (ret < 0)
> + dev_err(dev, "Failed to un-initialize panel: %d\n", ret);
> +
> + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
regulator_disable() should be called here as well.
