Hi Stephen, Based on the discussion in [1], this patch is not really needed for now. So I'll just remove this patch from v4. Thanks. [1] https://patchwork.kernel.org/project/dri-devel/patch/20210914162825.v3.2.Ib06997ddd73e2ac29e185f039d85cfa8e760d641@changeid/ On Tue, Sep 14, 2021 at 5:27 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > Quoting Philip Chen (2021-09-14 16:28:43) > > Use dev_err_probe() to add logs for error cases at probing time. > > > > Signed-off-by: Philip Chen <philipchen@xxxxxxxxxxxx> > > --- > > > > Can you use a cover-letter for more than one patch series? > > > (no changes since v1) > > > > drivers/gpu/drm/bridge/parade-ps8640.c | 22 ++++++++++++++-------- > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c > > index 685e9c38b2db..e340af381e05 100644 > > --- a/drivers/gpu/drm/bridge/parade-ps8640.c > > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c > > @@ -332,8 +332,10 @@ static int ps8640_probe(struct i2c_client *client) > > return -ENODEV; > > > > ps_bridge->panel_bridge = devm_drm_panel_bridge_add(dev, panel); > > - if (IS_ERR(ps_bridge->panel_bridge)) > > - return PTR_ERR(ps_bridge->panel_bridge); > > + if (IS_ERR(ps_bridge->panel_bridge)) { > > + return dev_err_probe(dev, PTR_ERR(ps_bridge->panel_bridge), > > + "Error creating bridge device\n"); > > From what I can tell it never returns -EPROBE_DEFER? So this isn't > useful. > > > + } > > > > ps_bridge->supplies[0].supply = "vdd33"; > > ps_bridge->supplies[1].supply = "vdd12"; > > @@ -344,16 +346,20 @@ static int ps8640_probe(struct i2c_client *client) > > > > ps_bridge->gpio_powerdown = devm_gpiod_get(&client->dev, "powerdown", > > GPIOD_OUT_HIGH); > > - if (IS_ERR(ps_bridge->gpio_powerdown)) > > - return PTR_ERR(ps_bridge->gpio_powerdown); > > + if (IS_ERR(ps_bridge->gpio_powerdown)) { > > + return dev_err_probe(dev, PTR_ERR(ps_bridge->gpio_powerdown), > > + "Error getting gpio_powerdown\n"); > > This looks ok, except we don't want braces on single statement ifs. > > > + } > > > > /* > > * Assert the reset to avoid the bridge being initialized prematurely > > */ > > ps_bridge->gpio_reset = devm_gpiod_get(&client->dev, "reset", > > GPIOD_OUT_HIGH); > > - if (IS_ERR(ps_bridge->gpio_reset)) > > - return PTR_ERR(ps_bridge->gpio_reset); > > + if (IS_ERR(ps_bridge->gpio_reset)) { > > + return dev_err_probe(dev, PTR_ERR(ps_bridge->gpio_reset), > > + "Error getting gpio_reset\n"); > > Same. > > > + } > > > > ps_bridge->bridge.funcs = &ps8640_bridge_funcs; > > ps_bridge->bridge.of_node = dev->of_node;
