On 06/15/17 15:38, Dave Stevenson wrote:
> Hi Hans.
>
> "On 15 June 2017 at 08:12, Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>> Hi Dave,
>>
>> Here is a quick review of this driver. Once a v2 is posted I'll do a more
>> thorough
>> check.
>
> Thank you. I wasn't expecting such a quick response.
>
>> On 06/14/2017 05:15 PM, Dave Stevenson wrote:
>>>
>>> Add driver for the Unicam camera receiver block on
>>> BCM283x processors.
>>>
>>> Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
>>> ---
>>> drivers/media/platform/Kconfig | 1 +
>>> drivers/media/platform/Makefile | 2 +
>>> drivers/media/platform/bcm2835/Kconfig | 14 +
>>> drivers/media/platform/bcm2835/Makefile | 3 +
>>> drivers/media/platform/bcm2835/bcm2835-unicam.c | 2100
>>> ++++++++++++++++++++++
>>> drivers/media/platform/bcm2835/vc4-regs-unicam.h | 257 +++
>>> 6 files changed, 2377 insertions(+)
>>> create mode 100644 drivers/media/platform/bcm2835/Kconfig
>>> create mode 100644 drivers/media/platform/bcm2835/Makefile
>>> create mode 100644 drivers/media/platform/bcm2835/bcm2835-unicam.c
>>> create mode 100644 drivers/media/platform/bcm2835/vc4-regs-unicam.h
>>>
>>> +static int unicam_s_input(struct file *file, void *priv, unsigned int i)
>>> +{
>>> + struct unicam_device *dev = video_drvdata(file);
>>> + int ret;
>>> +
>>> + if (v4l2_subdev_has_op(dev->sensor, video, s_routing))
>>> + ret = v4l2_subdev_call(dev->sensor, video, s_routing, i,
>>> 0, 0);
>>> + else
>>> + ret = -EINVAL; /* v4l2-compliance insists on -EINVAL */
>>
>>
>> Drop this if-else entirely. s_routing makes really no sense when using a
>> device
>> tree. In this particular case there really is just one input, period.
>
> I added this due to the ADV7282-M analogue to CSI bridge chip (uses
> adv7180.c driver). It uses s_routing to select the physical input /
> input type.
> If this is dropped, what is the correct mechanism for selecting the
> input? Unless I've missed it, s_routing is not a call that is exposed
> to userspace, so we're stuck with composite input 1.
>
> I had asked this question in previously [1], and whilst Sakari had
> kindly replied with "s_routing() video op as it stands now is awful, I
> hope no-one uses it", the fact is that it is used.
>
> [1] http://www.spinics.net/lists/linux-media/msg115550.html
s_routing was developed for USB and PCI(e) devices and predates the device tree.
Basically USB and PCI drivers will have card definitions where USB/PCI card IDs
are mapped to card descriptions, and that includes information on the various
inputs (composite, S-Video, etc) that are available on the backplane and how those
physical connectors are hooked up to the pins on the video ICs.
The enum/s/g_input ioctls all show the end-user view, i.e. they enumerate the
inputs on the backpanel of the product. The s_routing op was created to map
such inputs to actual pins on the ICs.
For platform devices we would do this in the device tree today, but some of
the necessary bindings are still missing. Specifically those for connectors,
AFAIK those are not yet defined. It's been discussed, but never finalized.
So if this was done correctly you would use the connector endpoints in the
device tree to enumerate the inputs and use how they are connected to the
other blocks as the routing information (i.e. pad number).
I would say that is the advanced course and to do this later.
Do you even have hardware where you can switch between inputs?
Regards,
Hans
