Hi Sakari,
Thank you for the patch.
On Saturday 07 March 2015 23:41:00 Sakari Ailus wrote:
> Move the code which connects the external entity to an ISP entity into a
> separate function. This disconnects it from parsing the platform data.
>
> Signed-off-by: Sakari Ailus <sakari.ai...@iki.fi>
> ---
> drivers/media/platform/omap3isp/isp.c | 147 +++++++++++++++--------------
> 1 file changed, 74 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 4ab674d..a607f26 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -1832,6 +1832,77 @@ isp_register_subdev_group(struct isp_device *isp,
> return sensor;
> }
>
> +static int isp_link_entity(
> + struct isp_device *isp, struct media_entity *entity,
> + enum isp_interface_type interface)
> +{
> + struct media_entity *input;
> + unsigned int flags;
> + unsigned int pad;
> + unsigned int i;
> +
> + /* Connect the sensor to the correct interface module.
> + * Parallel sensors are connected directly to the CCDC, while
> + * serial sensors are connected to the CSI2a, CCP2b or CSI2c
> + * receiver through CSIPHY1 or CSIPHY2.
> + */
> + switch (interface) {
> + case ISP_INTERFACE_PARALLEL:
> + input = &isp->isp_ccdc.subdev.entity;
> + pad = CCDC_PAD_SINK;
> + flags = 0;
> + break;
> +
> + case ISP_INTERFACE_CSI2A_PHY2:
> + input = &isp->isp_csi2a.subdev.entity;
> + pad = CSI2_PAD_SINK;
> + flags = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED;
> + break;
> +
> + case ISP_INTERFACE_CCP2B_PHY1:
> + case ISP_INTERFACE_CCP2B_PHY2:
> + input = &isp->isp_ccp2.subdev.entity;
> + pad = CCP2_PAD_SINK;
> + flags = 0;
> + break;
> +
> + case ISP_INTERFACE_CSI2C_PHY1:
> + input = &isp->isp_csi2c.subdev.entity;
> + pad = CSI2_PAD_SINK;
> + flags = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED;
> + break;
> +
> + default:
> + dev_err(isp->dev, "%s: invalid interface type %u\n", __func__,
> + interface);
> + return -EINVAL;
> + }
> +
> + /*
> + * Not all interfaces are available on all revisions of the
> + * ISP. The sub-devices of those interfaces aren't initialised
> + * in such a case. Check this by ensuring the num_pads is
> + * non-zero.
> + */
> + if (!input->num_pads) {
> + dev_err(isp->dev, "%s: invalid input %u\n", entity->name,
> + interface);
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < entity->num_pads; i++) {
> + if (entity->pads[i].flags & MEDIA_PAD_FL_SOURCE)
> + break;
> + }
> + if (i == entity->num_pads) {
> + dev_err(isp->dev, "%s: no source pad in external entity\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + return media_entity_create_link(entity, i, input, pad, flags);
> +}
> +
> static int isp_register_entities(struct isp_device *isp)
> {
> struct isp_platform_data *pdata = isp->pdata;
> @@ -1894,85 +1965,15 @@ static int isp_register_entities(struct isp_device
> *isp)
>
> /* Register external entities */
> for (subdevs = pdata->subdevs; subdevs && subdevs->subdevs; ++subdevs) {
> - struct v4l2_subdev *sensor;
> - struct media_entity *input;
> - unsigned int flags;
> - unsigned int pad;
> - unsigned int i;
> + struct v4l2_subdev *sensor =
> + isp_register_subdev_group(isp, subdevs->subdevs);
>
> - sensor = isp_register_subdev_group(isp, subdevs->subdevs);
Nit-picking a bit, I'd keep the variable declaration and the function call
separate here, as isp_register_subdev_group() is much more than an
initializer. Apart from that,
Acked-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> if (sensor == NULL)
> continue;
>
> sensor->host_priv = subdevs;
>
> - /* Connect the sensor to the correct interface module. Parallel
> - * sensors are connected directly to the CCDC, while serial
> - * sensors are connected to the CSI2a, CCP2b or CSI2c receiver
> - * through CSIPHY1 or CSIPHY2.
> - */
> - switch (subdevs->interface) {
> - case ISP_INTERFACE_PARALLEL:
> - input = &isp->isp_ccdc.subdev.entity;
> - pad = CCDC_PAD_SINK;
> - flags = 0;
> - break;
> -
> - case ISP_INTERFACE_CSI2A_PHY2:
> - input = &isp->isp_csi2a.subdev.entity;
> - pad = CSI2_PAD_SINK;
> - flags = MEDIA_LNK_FL_IMMUTABLE
> - | MEDIA_LNK_FL_ENABLED;
> - break;
> -
> - case ISP_INTERFACE_CCP2B_PHY1:
> - case ISP_INTERFACE_CCP2B_PHY2:
> - input = &isp->isp_ccp2.subdev.entity;
> - pad = CCP2_PAD_SINK;
> - flags = 0;
> - break;
> -
> - case ISP_INTERFACE_CSI2C_PHY1:
> - input = &isp->isp_csi2c.subdev.entity;
> - pad = CSI2_PAD_SINK;
> - flags = MEDIA_LNK_FL_IMMUTABLE
> - | MEDIA_LNK_FL_ENABLED;
> - break;
> -
> - default:
> - dev_err(isp->dev, "%s: invalid interface type %u\n",
> - __func__, subdevs->interface);
> - ret = -EINVAL;
> - goto done;
> - }
> -
> - /*
> - * Not all interfaces are available on all revisions
> - * of the ISP. The sub-devices of those interfaces
> - * aren't initialised in such a case. Check this by
> - * ensuring the num_pads is non-zero.
> - */
> - if (!input->num_pads) {
> - dev_err(isp->dev, "%s: invalid input %u\n",
> - entity->name, subdevs->interface);
> - ret = -EINVAL;
> - goto done;
> - }
> -
> - for (i = 0; i < sensor->entity.num_pads; i++) {
> - if (sensor->entity.pads[i].flags & MEDIA_PAD_FL_SOURCE)
> - break;
> - }
> - if (i == sensor->entity.num_pads) {
> - dev_err(isp->dev,
> - "%s: no source pad in external entity\n",
> - __func__);
> - ret = -EINVAL;
> - goto done;
> - }
> -
> - ret = media_entity_create_link(&sensor->entity, i, input, pad,
> - flags);
> + ret = isp_link_entity(isp, &sensor->entity, subdevs->interface);
> if (ret < 0)
> goto done;
> }
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
