Messages in this thread | | | Date | Fri, 28 Jul 2023 14:21:19 +0200 | Subject | Re: [PATCH v3 3/3] drm/panel-simple: allow LVDS format override | From | Johannes Zink <> |
| |
Hi Dan,
thanks for your feedback.
On 7/28/23 14:03, Dan Carpenter wrote: > On Fri, Jul 28, 2023 at 01:54:40PM +0200, Johannes Zink wrote: >> @@ -556,7 +602,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) >> struct device_node *ddc; >> int connector_type; >> u32 bus_flags; >> - int err; >> + int err, ret; > > I don't like this at all... > >> >> panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL); >> if (!panel) >> @@ -601,6 +647,13 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) >> panel_simple_parse_panel_timing_node(dev, panel, &dt); >> } >> >> + if (desc->connector_type == DRM_MODE_CONNECTOR_LVDS) { >> + /* Optional data-mapping property for overriding bus format */ >> + ret = panel_simple_override_nondefault_lvds_datamapping(dev, panel); >> + if (ret) >> + goto free_ddc; > > This *looks* like we are returning an error but we aren't. I think we > should be. If not then we need to have a discussion about that and > add some comments. >
Good catch. This should actually be err instead of ret. This way we'd make sure to return -ENOMEM from panel_simple_probe() in case an error occured due to devm_kmemdup failing in panel_simple_override_nondefault_lvds_datamapping().
I'll send a v4 with that fixed.
Thanks for having my back! Johannes
> regards, > dan carpenter > >> + } >> + >> connector_type = desc->connector_type; >> /* Catch common mistakes for panels. */ >> switch (connector_type) { >> >> -- >> 2.39.2 > >
-- Pengutronix e.K. | Johannes Zink | Steuerwalder Str. 21 | https://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686| Fax: +49-5121-206917-5555 |
| |