Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1676538

Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver

From Sylwester Nawrocki <s.nawrocki@samsung.com>
Newsgroups linux.kernel
Subject Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver
Date 2017-06-28 12:40 +0200
Message-ID <tXiy5-5Cj-7@gated-at.bofh.it> (permalink)
References (2 earlier) <tWn6P-1Mu-37@gated-at.bofh.it> <tWUm5-6qI-5@gated-at.bofh.it> <tX5rb-5FM-7@gated-at.bofh.it> <tXiy6-5Cj-9@gated-at.bofh.it> <tXhsl-50j-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hi Jose,

On 06/28/2017 11:25 AM, Jose Abreu wrote:
> On 27-06-2017 21:34, Sylwester Nawrocki wrote:
>> On 06/27/2017 10:43 AM, Jose Abreu wrote:
>>> On 25-06-2017 22:13, Sylwester Nawrocki wrote:
>>>> On 06/20/2017 07:26 PM, Jose Abreu wrote:
>>>>> 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@synopsys.com>

> 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.

>>>>> +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?
 >>
>> 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.

>>>>> +	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;
 >>>>
>>>> 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.
 >>
>> 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

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH v4 0/4] Synopsys Designware HDMI Video Capture Controller + PHY Jose Abreu <Jose.Abreu@synopsys.com> - 2017-06-20 19:30 +0200
  [PATCH v4 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX Jose Abreu <Jose.Abreu@synopsys.com> - 2017-06-20 19:30 +0200
    Re: [PATCH v4 4/4] dt-bindings: media: Document Synopsys Designware  HDMI RX Rob Herring <robh@kernel.org> - 2017-06-24 00:00 +0200
      Re: [PATCH v4 4/4] dt-bindings: media: Document Synopsys Designware  HDMI RX Jose Abreu <Jose.Abreu@synopsys.com> - 2017-06-26 18:50 +0200
  [PATCH v4 1/4] [media] platform: Add Synopsys Designware HDMI RX PHY e405 Driver Jose Abreu <Jose.Abreu@synopsys.com> - 2017-06-20 19:30 +0200
    Re: [PATCH v4 1/4] [media] platform: Add Synopsys Designware HDMI RX  PHY e405 Driver Sylwester Nawrocki <snawrocki@kernel.org> - 2017-06-25 23:20 +0200
      Re: [PATCH v4 1/4] [media] platform: Add Synopsys Designware HDMI RX  PHY e405 Driver Jose Abreu <Jose.Abreu@synopsys.com> - 2017-06-26 18:50 +0200
  Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI RX  Controller Driver Sylwester Nawrocki <snawrocki@kernel.org> - 2017-06-25 23:20 +0200
    Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI RX  Controller Driver Jose Abreu <Jose.Abreu@synopsys.com> - 2017-06-27 10:50 +0200
      Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI RX  Controller Driver Sylwester Nawrocki <snawrocki@kernel.org> - 2017-06-27 22:40 +0200
        Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI RX  Controller Driver Jose Abreu <Jose.Abreu@synopsys.com> - 2017-06-28 11:30 +0200
          Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI  RX Controller Driver Sylwester Nawrocki <s.nawrocki@samsung.com> - 2017-06-28 12:40 +0200

csiph-web