* [bug report] drm/mcde: Add new driver for ST-Ericsson MCDE
@ 2019-05-29 11:06 Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-05-29 11:06 UTC (permalink / raw)
To: linus.walleij; +Cc: dri-devel
Hello Linus Walleij,
The patch 5fc537bfd000: "drm/mcde: Add new driver for ST-Ericsson
MCDE" from May 24, 2019, leads to the following static checker
warning:
drivers/gpu/drm/mcde/mcde_dsi.c:917 mcde_dsi_bind()
warn: 'bridge' isn't an ERR_PTR
drivers/gpu/drm/mcde/mcde_dsi.c
882 /* Obtain the clocks */
883 d->hs_clk = devm_clk_get(dev, "hs");
884 if (IS_ERR(d->hs_clk)) {
885 dev_err(dev, "unable to get HS clock\n");
886 return PTR_ERR(d->hs_clk);
887 }
888
889 d->lp_clk = devm_clk_get(dev, "lp");
890 if (IS_ERR(d->lp_clk)) {
891 dev_err(dev, "unable to get LP clock\n");
892 return PTR_ERR(d->lp_clk);
893 }
894
895 /* Assert RESET through the PRCMU, active low */
896 /* FIXME: which DSI block? */
897 regmap_update_bits(d->prcmu, PRCM_DSI_SW_RESET,
898 PRCM_DSI_SW_RESET_DSI0_SW_RESETN, 0);
899
900 usleep_range(100, 200);
901
902 /* De-assert RESET again */
903 regmap_update_bits(d->prcmu, PRCM_DSI_SW_RESET,
904 PRCM_DSI_SW_RESET_DSI0_SW_RESETN,
905 PRCM_DSI_SW_RESET_DSI0_SW_RESETN);
906
907 /* Start up the hardware */
908 mcde_dsi_start(d);
909
910 /* Look for a panel as a child to this node */
911 for_each_available_child_of_node(dev->of_node, child) {
912 panel = of_drm_find_panel(child);
913 if (IS_ERR(panel)) {
914 dev_err(dev, "failed to find panel try bridge (%lu)\n",
915 PTR_ERR(panel));
916 bridge = of_drm_find_bridge(child);
917 if (IS_ERR(bridge)) {
of_drm_find_bridge() returns NULL on error, not error pointers.
918 dev_err(dev, "failed to find bridge (%lu)\n",
919 PTR_ERR(bridge));
^^^^^^^^^^^^^^^
920 return PTR_ERR(bridge);
^^^^^^^^^^^^^^^
Should this be -EPROBEDEFER? I'm not sure of the rules.
921 }
922 }
923 }
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [bug report] drm/mcde: Add new driver for ST-Ericsson MCDE
@ 2019-05-29 11:32 Dan Carpenter
2019-05-29 14:05 ` Linus Walleij
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2019-05-29 11:32 UTC (permalink / raw)
To: linus.walleij; +Cc: dri-devel
Hello Linus Walleij,
The patch 5fc537bfd000: "drm/mcde: Add new driver for ST-Ericsson
MCDE" from May 24, 2019, leads to the following static checker
warning:
drivers/gpu/drm/mcde/mcde_drv.c:488 mcde_probe()
error: uninitialized symbol 'match'.
drivers/gpu/drm/mcde/mcde_drv.c
470 writel(0xFFFFFFFF, mcde->regs + MCDE_RISERR);
471
472 /* Spawn child devices for the DSI ports */
473 devm_of_platform_populate(dev);
474
475 /* Create something that will match the subdrivers when we bind */
476 for (i = 0; i < ARRAY_SIZE(mcde_component_drivers); i++) {
477 struct device_driver *drv = &mcde_component_drivers[i]->driver;
478 struct device *p = NULL, *d;
479
480 while ((d = bus_find_device(&platform_bus_type, p, drv,
481 (void *)platform_bus_type.match))) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The concern would be that this condintion is never met. I suspet that's
not possible?
482 put_device(p);
483 component_match_add(dev, &match, mcde_compare_dev, d);
484 p = d;
485 }
486 put_device(p);
487 }
488 if (IS_ERR(match)) {
489 dev_err(dev, "could not create component match\n");
490 ret = PTR_ERR(match);
491 goto clk_disable;
492 }
493 ret = component_master_add_with_match(&pdev->dev, &mcde_drm_comp_ops,
494 match);
495 if (ret) {
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [bug report] drm/mcde: Add new driver for ST-Ericsson MCDE
2019-05-29 11:32 Dan Carpenter
@ 2019-05-29 14:05 ` Linus Walleij
0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-05-29 14:05 UTC (permalink / raw)
To: Dan Carpenter; +Cc: open list:DRM PANEL DRIVERS
On Wed, May 29, 2019 at 1:32 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> drivers/gpu/drm/mcde/mcde_drv.c:488 mcde_probe()
> error: uninitialized symbol 'match'.
(...)
> 480 while ((d = bus_find_device(&platform_bus_type, p, drv,
> 481 (void *)platform_bus_type.match))) {
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> The concern would be that this condintion is never met. I suspet that's
> not possible?
Component drivers have this property, they rely on the subcomponents to
be there for the functional whole to work. If it's not, all hell break loose in
different ways. So at least one subcomponent (DSI in this case) needs
to be found.
But it's fine to initialize match to NULL if it makes the static checks
happier!
But that just masks the deeper problem, which I found in the qcom
driver: component_master_add_with_match() can be called
with match set to NULL, which it actually cannot handle.
This is a generic problem in DRM and needs to be fixed everywhere.
I made a patch like this:
diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c
index baf63fb6850a..bc11c446e594 100644
--- a/drivers/gpu/drm/mcde/mcde_drv.c
+++ b/drivers/gpu/drm/mcde/mcde_drv.c
@@ -319,7 +319,7 @@ static int mcde_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct drm_device *drm;
struct mcde *mcde;
- struct component_match *match;
+ struct component_match *match = NULL;
struct resource *res;
u32 pid;
u32 val;
@@ -485,7 +485,7 @@ static int mcde_probe(struct platform_device *pdev)
}
put_device(p);
}
- if (IS_ERR(match)) {
+ if (IS_ERR_OR_NULL(match)) {
dev_err(dev, "could not create component match\n");
ret = PTR_ERR(match);
goto clk_disable;
But I need to test that on real hardware before I submit it.
Thanks,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [bug report] drm/mcde: Add new driver for ST-Ericsson MCDE
@ 2019-05-29 11:35 Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-05-29 11:35 UTC (permalink / raw)
To: linus.walleij; +Cc: dri-devel
Hello Linus Walleij,
This is a semi-automatic email about new static checker warnings.
The patch 5fc537bfd000: "drm/mcde: Add new driver for ST-Ericsson
MCDE" from May 24, 2019, leads to the following Smatch complaint:
drivers/gpu/drm/mcde/mcde_dsi.c:908 mcde_dsi_bind()
error: we previously assumed 'd->mdsi' could be null (see line 879)
drivers/gpu/drm/mcde/mcde_dsi.c
878 /* If the display attached before binding, set this up */
879 if (d->mdsi)
^^^^^^^
Check for NULL
880 d->mcde->mdsi = d->mdsi;
881
882 /* Obtain the clocks */
883 d->hs_clk = devm_clk_get(dev, "hs");
884 if (IS_ERR(d->hs_clk)) {
885 dev_err(dev, "unable to get HS clock\n");
886 return PTR_ERR(d->hs_clk);
887 }
888
889 d->lp_clk = devm_clk_get(dev, "lp");
890 if (IS_ERR(d->lp_clk)) {
891 dev_err(dev, "unable to get LP clock\n");
892 return PTR_ERR(d->lp_clk);
893 }
894
895 /* Assert RESET through the PRCMU, active low */
896 /* FIXME: which DSI block? */
897 regmap_update_bits(d->prcmu, PRCM_DSI_SW_RESET,
898 PRCM_DSI_SW_RESET_DSI0_SW_RESETN, 0);
899
900 usleep_range(100, 200);
901
902 /* De-assert RESET again */
903 regmap_update_bits(d->prcmu, PRCM_DSI_SW_RESET,
904 PRCM_DSI_SW_RESET_DSI0_SW_RESETN,
905 PRCM_DSI_SW_RESET_DSI0_SW_RESETN);
906
907 /* Start up the hardware */
908 mcde_dsi_start(d);
^
d->mdsi is dereferenced without checking for NULL inside here.
909
910 /* Look for a panel as a child to this node */
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [bug report] drm/mcde: Add new driver for ST-Ericsson MCDE
@ 2020-08-31 11:39 Dan Carpenter
2020-09-12 10:29 ` Linus Walleij
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2020-08-31 11:39 UTC (permalink / raw)
To: linus.walleij; +Cc: dri-devel
Hello Linus Walleij,
The patch 5fc537bfd000: "drm/mcde: Add new driver for ST-Ericsson
MCDE" from May 24, 2019, leads to the following static checker
warning:
drivers/gpu/drm/mcde/mcde_display.c:570 mcde_configure_channel()
error: uninitialized symbol 'val'.
drivers/gpu/drm/mcde/mcde_display.c
552 case MCDE_VIDEO_TE_FLOW:
553 val = MCDE_CHNLXSYNCHMOD_SRC_SYNCH_HARDWARE
554 << MCDE_CHNLXSYNCHMOD_SRC_SYNCH_SHIFT;
555 val |= MCDE_CHNLXSYNCHMOD_OUT_SYNCH_SRC_TE0
556 << MCDE_CHNLXSYNCHMOD_OUT_SYNCH_SRC_SHIFT;
557 break;
558 case MCDE_VIDEO_FORMATTER_FLOW:
559 val = MCDE_CHNLXSYNCHMOD_SRC_SYNCH_HARDWARE
560 << MCDE_CHNLXSYNCHMOD_SRC_SYNCH_SHIFT;
561 val |= MCDE_CHNLXSYNCHMOD_OUT_SYNCH_SRC_FORMATTER
562 << MCDE_CHNLXSYNCHMOD_OUT_SYNCH_SRC_SHIFT;
563 break;
564 default:
565 dev_err(mcde->dev, "unknown flow mode %d\n",
566 mcde->flow_mode);
"val" not initialized on this path.
567 break;
568 }
569
570 writel(val, mcde->regs + sync);
571
572 /* Set up pixels per line and lines per frame */
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-12 10:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-29 11:06 [bug report] drm/mcde: Add new driver for ST-Ericsson MCDE Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2019-05-29 11:32 Dan Carpenter
2019-05-29 14:05 ` Linus Walleij
2019-05-29 11:35 Dan Carpenter
2020-08-31 11:39 Dan Carpenter
2020-09-12 10:29 ` Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.