Hi Jose,

On 06/28/2017 11:25 AM, Jose Abreu wrote:

On 27-06-2017 21:34, Sylwester Nawrocki wrote:

quoted

On 06/27/2017 10:43 AM, Jose Abreu wrote:

quoted

On 25-06-2017 22:13, Sylwester Nawrocki wrote:

quoted

On 06/20/2017 07:26 PM, Jose Abreu wrote:

quoted

This is an initial submission for the Synopsys Designware HDMI RX
Controller Driver. This driver interacts with a phy driver so that
a communication between them is created and a video pipeline is
configured.
Signed-off-by: Jose Abreu <joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

The modules are installed but I think I don't have udev :/ I'm
running this on an embedded platform called ARC AXS and I'm using
buildroot with minimal options.

OK, then let's keep this request_module.

quoted

quoted

quoted

quoted

+static int dw_hdmi_v4l2_notify_complete(struct v4l2_async_notifier *notifier)
+{
+	struct dw_hdmi_dev *dw_dev = notifier_to_dw_dev(notifier);
+	int ret;
+
+	ret = v4l2_device_register_subdev_nodes(&dw_dev->v4l2_dev);

There shouldn't be multiple struct v4l2_device instances, instead we should
have only one created by the main driver. AFAIU, in your case it would be
driver associated with the dw-hdmi-soc DT node.  And normally such a top level
driver creates subdev device nodes when its all required sub-devices are
available.

I think this patch could be useful for you:
https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.linuxtv.
org_patch_41834&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19I
Jiuxx_p_Rzo2g-uHDKw&m=uNHRQkbP_-z8v5d30KFx9pcPEUhlr4ciWY3ZDAVExTA&s=dB9wpgeP
7AJg1eDRty0-RKhq3DY-7J5srIzyVoJey5I&e=

With that the dw-hdmi-soc driver would have it's v4l2-async notifier's
notify_complete() callback called only when both the hdmi-rx and the
hdmi-phy subdevs are registered.

Yeah, I saw the patches. I just implemented this way because they
are not merged yet, right?

 >>

quoted

I think these patches will be merged in v4.14-rc1, so together with your driver.
You could apply them locally and indicate that your series depends on them in
the cover letter.

Ok, will apply them locally and re-test.

Thanks.

quoted

quoted

quoted

quoted

+	if (ret) {
+		dev_err(dw_dev->dev, "failed to register subdev nodes\n");
+		return ret;
+	}
+
+	return 0;
+}
+static int dw_hdmi_rx_probe(struct platform_device *pdev)
+{
+	/* V4L2 initialization */
+	sd = &dw_dev->sd;
+	v4l2_subdev_init(sd, &dw_hdmi_sd_ops);
+	strlcpy(sd->name, dev_name(dev), sizeof(sd->name));
+	sd->dev = dev;
+	sd->internal_ops = &dw_hdmi_internal_ops;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS;

 >>>>

quoted

quoted

quoted

Don't you also need V4L2_SUBDEV_FL_HAS_DEVNODE flag set?

Ouch. Yes I need otherwise the subdev will not be associated with
the v4l2_device.

 >>

quoted

This flag indicates that the v4l2 subdev device node (/dev/v4l-subdev?)
should be created for this subdevice.

Ok, will add for controller driver only then: I think for phy
this should not be added because controller is responsible to
manage phy entirely so creating a /dev/ which can be seen by
userspace can lead to confusion, maybe?

Right, there should be no need for the PHY. It's up to you to
set it or not for the RX controller subdev. I just got confused
because in your dw_hdmi_sd_ops data structure there are ops which
would suggest that device node is used.


Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html