Re: [PATCH RFC 3/6] drm/tilcdc: Add support for external compontised DRM encoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/02/15 18:01, Russell King - ARM Linux wrote:
On Thu, Feb 26, 2015 at 04:55:32PM +0200, Jyri Sarha wrote:
+	ret = component_bind_all(dev->dev, dev);
+	if (ret < 0) {
+		dev_err(dev->dev, "Binding subcomponents failed: %d\n", ret);

Do you need to print this?  The component helper is already fairly
verbose about what succeeds and fails.


Will remove.

+static const struct component_master_ops tilcdc_comp_ops = {
+	.add_components = tilcdc_add_external_components,

I'd much rather you used the new matching support rather than using the
old .add_components.  The new matching support is more efficient as you
only have to scan DT once, rather than each time we try to probe.  That
will mean...


That is otherwise fine, but with the match code it not possible to implement a master that may not have any components.

Would it be Ok to add a check that master->ops->add_components is defined, before calling it in find_componets() (drivers/base/component.c:120) and return 0 if it is not?

Best regards,
Jyri

@@ -613,12 +643,12 @@ static int tilcdc_pdev_probe(struct platform_device *pdev)
  		return -ENXIO;
  	}

You need to build a struct component_match array here using
component_match_add()...


-	return drm_platform_init(&tilcdc_driver, pdev);
+	return component_master_add(&pdev->dev, &tilcdc_comp_ops);

and then finally register the ops with component_master_add_with_match().

Thanks.


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux