On 02/05/2017 04:48 PM, Laurent Pinchart wrote:
> Hi Steve,
>
> On Tuesday 24 Jan 2017 18:07:55 Steve Longerbeam wrote:
>> On 01/24/2017 04:02 AM, Philipp Zabel wrote:
>>> On Fri, 2017-01-20 at 15:03 +0100, Hans Verkuil wrote:
>>>>> +
>>>>> +int vidsw_g_mbus_config(struct v4l2_subdev *sd, struct v4l2_mbus_config
>>>>> *cfg)
>>>>> +{
>>>>> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
>>>>> + struct media_pad *pad;
>>>>> + int ret;
>>>>> +
>>>>> + if (vidsw->active == -1) {
>>>>> + dev_err(sd->dev, "no configuration for inactive mux\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + /*
>>>>> + * Retrieve media bus configuration from the entity connected to the
>>>>> + * active input
>>>>> + */
>>>>> + pad = media_entity_remote_pad(&vidsw->pads[vidsw->active]);
>>>>> + if (pad) {
>>>>> + sd = media_entity_to_v4l2_subdev(pad->entity);
>>>>> + ret = v4l2_subdev_call(sd, video, g_mbus_config, cfg);
>>>>> + if (ret == -ENOIOCTLCMD)
>>>>> + pad = NULL;
>>>>> + else if (ret < 0) {
>>>>> + dev_err(sd->dev, "failed to get source
> configuration\n");
>>>>> + return ret;
>>>>> + }
>>>>> + }
>>>>> + if (!pad) {
>>>>> + /* Mirror the input side on the output side */
>>>>> + cfg->type = vidsw->endpoint[vidsw->active].bus_type;
>>>>> + if (cfg->type == V4L2_MBUS_PARALLEL ||
>>>>> + cfg->type == V4L2_MBUS_BT656)
>>>>> + cfg->flags = vidsw->endpoint[vidsw-
>> active].bus.parallel.flags;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>
>>>> I am not certain this op is needed at all. In the current kernel this op
>>>> is only used by soc_camera, pxa_camera and omap3isp (somewhat dubious).
>>>> Normally this information should come from the device tree and there
>>>> should be no need for this op.
>>>>
>>>> My (tentative) long-term plan was to get rid of this op.
>>>>
>>>> If you don't need it, then I recommend it is removed.
>>
>> Hi Hans, the imx-media driver was only calling g_mbus_config to the camera
>> sensor, and it was doing that to determine the sensor's bus type. This info
>> was already available from parsing a v4l2_of_endpoint from the sensor node.
>> So it was simple to remove the g_mbus_config calls, and instead rely on the
>> parsed sensor v4l2_of_endpoint.
>
> That's not a good point. The imx-media driver must not parse the sensor DT
> node as it is not aware of what bindings the sensor is compatible with.
> Information must instead be queried from the sensor subdev at runtime, through
> the g_mbus_config() operation.
>
> Of course, if you can get the information from the imx-media DT node, that's
> certainly an option. It's only information provided by the sensor driver that
> you have no choice but query using a subdev operation.
Shouldn't this come from the imx-media DT node? BTW, why is omap3isp using this?
The reason I am suspicious about this op is that it came from soc-camera and
predates the DT. The contents of v4l2_mbus_config seems very much like a HW
description to me, i.e. something that belongs in the DT.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
