On 9/7/24 3:53 AM, Jessica Zhang wrote: > > > On 9/6/2024 3:14 PM, Jessica Zhang wrote: >> >> >> On 9/4/2024 7:15 AM, Tejas Vipin wrote: >>> Changes the himax-hx83112a panel to use multi style functions for >>> improved error handling. >>> >>> Signed-off-by: Tejas Vipin <tejasvipin76@xxxxxxxxx> >> >> Reviewed-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx> > > Hi Tejas, > > Just a heads up, it seems that this might be a duplicate of this change [1]? > > Thanks, > > Jessica Zhang > > [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1 Ah, thanks for letting me know. I hadn't realized someone else had started working on this too. However, I would argue that my patch [2] is a better candidate for merging because of the following reasons: 1) Removes unnecessary error printing: The mipi_dsi_*_multi() functions all have inbuilt error printing which makes printing errors after hx83112a_on unnecessary as is addressed in [2] like so: > - ret = hx83112a_on(ctx); > + ret = hx83112a_on(ctx->dsi); > if (ret < 0) { > - dev_err(dev, "Failed to initialize panel: %d\n", ret); > gpiod_set_value_cansleep(ctx->reset_gpio, 1); > regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); > - return ret; > } [2] also removes the unnecessary dev_err after regulator_bulk_enable as was addressed in [3] like so: > ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); > - if (ret < 0) { > - dev_err(dev, "Failed to enable regulators: %d\n", ret); > + if (ret < 0) > return ret; > - } 2) Better formatting The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted quite right according to what has been done so far. They are written as such in [1]: > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1, > 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e); Where they should be written as such in [2]: > + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1, > + 0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e); All in all, the module generated using my patch ends up being a teensy bit smaller (1% smaller). But if chronology is what is important, then it would at least be nice to see the above changes as part of Abhishek's patch too. And I'll be sure to look at the mail in the drm inbox now onwards :p [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1 [2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@xxxxxxxxx/ [3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@xxxxxxxxxxxxxx/ -- Tejas Vipin
