[{"id":1773548,"web_url":"http://patchwork.ozlabs.org/comment/1773548/","msgid":"<20170922123849.hcm76tlplnvd44mt@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-22T12:38:49","subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Maxime,\n\nOn Fri, Sep 22, 2017 at 01:47:03PM +0200, Maxime Ripard wrote:\n> The Cadence MIPI-CSI2 TX controller is an hardware block meant to be used\n> as a bridge between pixel interfaces and a CSI-2 bus.\n> \n> It supports operating with an internal or external D-PHY, with up to 4\n> lanes, or without any D-PHY. The current code only supports the former\n> case.\n> \n> While the virtual channel input on the pixel interface can be directly\n> mapped to CSI2, the datatype input is actually a selection signal (3-bits)\n> mapping to a table of up to 8 preconfigured datatypes/formats (programmed\n> at start-up)\n> \n> The block supports up to 8 input datatypes.\n> \n> Signed-off-by: Maxime Ripard \n> ---\n> drivers/media/platform/cadence/Kconfig | 6 +\n> drivers/media/platform/cadence/Makefile | 1 +\n> drivers/media/platform/cadence/cdns-csi2tx.c | 479 +++++++++++++++++++++++++++\n> 3 files changed, 486 insertions(+)\n> create mode 100644 drivers/media/platform/cadence/cdns-csi2tx.c\n> \n> diff --git a/drivers/media/platform/cadence/Kconfig b/drivers/media/platform/cadence/Kconfig\n> index d1b6bbb6a0eb..db49328ee8b2 100644\n> --- a/drivers/media/platform/cadence/Kconfig\n> +++ b/drivers/media/platform/cadence/Kconfig\n> @@ -9,4 +9,10 @@ config VIDEO_CADENCE_CSI2RX\n> \tdepends on VIDEO_V4L2_SUBDEV_API\n> \tselect V4L2_FWNODE\n> \n> +config VIDEO_CADENCE_CSI2TX\n> +\ttristate \"Cadence MIPI-CSI2 TX Controller\"\n> +\tdepends on MEDIA_CONTROLLER\n> +\tdepends on VIDEO_V4L2_SUBDEV_API\n> +\tselect V4L2_FWNODE\n> +\n> endif\n> diff --git a/drivers/media/platform/cadence/Makefile b/drivers/media/platform/cadence/Makefile\n> index 99a4086b7448..7fe992273162 100644\n> --- a/drivers/media/platform/cadence/Makefile\n> +++ b/drivers/media/platform/cadence/Makefile\n> @@ -1 +1,2 @@\n> obj-$(CONFIG_VIDEO_CADENCE_CSI2RX)\t+= cdns-csi2rx.o\n> +obj-$(CONFIG_VIDEO_CADENCE_CSI2TX)\t+= cdns-csi2tx.o\n> diff --git a/drivers/media/platform/cadence/cdns-csi2tx.c b/drivers/media/platform/cadence/cdns-csi2tx.c\n> new file mode 100644\n> index 000000000000..859bbce8772b\n> --- /dev/null\n> +++ b/drivers/media/platform/cadence/cdns-csi2tx.c\n> @@ -0,0 +1,479 @@\n> +/*\n> + * Driver for Cadence MIPI-CSI2 TX Controller\n> + *\n> + * Copyright (C) 2017 Cadence Design Systems Inc.\n> + *\n> + * This program is free software; you can redistribute it and/or modify it\n> + * under the terms of the GNU General Public License as published by the\n> + * Free Software Foundation; either version 2 of the License, or (at your\n> + * option) any later version.\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#define CSI2TX_DEVICE_CONFIG_REG\t0x00\n> +\n> +#define CSI2TX_CONFIG_REG\t\t0x20\n> +#define CSI2TX_CONFIG_CFG_REQ\t\t\tBIT(2)\n> +#define CSI2TX_CONFIG_SRST_REQ\t\t\tBIT(1)\n> +\n> +#define CSI2TX_DPHY_CFG_REG\t\t0x28\n> +#define CSI2TX_DPHY_CFG_CLK_RESET\t\tBIT(16)\n> +#define CSI2TX_DPHY_CFG_LANE_RESET(n)\t\tBIT((n) + 12)\n> +#define CSI2TX_DPHY_CFG_MODE_MASK\t\tGENMASK(9, 8)\n> +#define CSI2TX_DPHY_CFG_MODE_LPDT\t\t(2 << 8)\n> +#define CSI2TX_DPHY_CFG_MODE_HS\t\t\t(1 << 8)\n> +#define CSI2TX_DPHY_CFG_MODE_ULPS\t\t(0 << 8)\n> +#define CSI2TX_DPHY_CFG_CLK_ENABLE\t\tBIT(4)\n> +#define CSI2TX_DPHY_CFG_LANE_ENABLE(n)\t\tBIT(n)\n> +\n> +#define CSI2TX_DPHY_CLK_WAKEUP_REG\t0x2c\n> +#define CSI2TX_DPHY_CLK_WAKEUP_ULPS_CYCLES(n)\t((n) & 0xffff)\n> +\n> +#define CSI2TX_DT_CFG_REG(n)\t\t(0x80 + (n) * 8)\n> +#define CSI2TX_DT_CFG_DT(n)\t\t\t(((n) & 0x3f) << 2)\n> +\n> +#define CSI2TX_DT_FORMAT_REG(n)\t\t(0x84 + (n) * 8)\n> +#define CSI2TX_DT_FORMAT_BYTES_PER_LINE(n)\t(((n) & 0xffff) << 16)\n> +#define CSI2TX_DT_FORMAT_MAX_LINE_NUM(n)\t((n) & 0xffff)\n> +\n> +#define CSI2TX_STREAM_IF_CFG_REG(n)\t(0x100 + (n) * 4)\n> +#define CSI2TX_STREAM_IF_CFG_FILL_LEVEL(n)\t((n) & 0x1f)\n> +\n> +#define CSI2TX_STREAMS_MAX\t4\n> +\n> +enum csi2tx_pads {\n> +\tCSI2TX_PAD_SOURCE,\n> +\tCSI2TX_PAD_SINK_STREAM0,\n> +\tCSI2TX_PAD_SINK_STREAM1,\n> +\tCSI2TX_PAD_SINK_STREAM2,\n> +\tCSI2TX_PAD_SINK_STREAM3,\n> +\tCSI2TX_PAD_MAX,\n> +};\n> +\n> +struct csi2tx_fmt {\n> +\tu32\tmbus;\n> +\tu32\tdt;\n> +\tu32\tbpp;\n> +};\n> +\n> +struct csi2tx_priv {\n> +\tstruct device\t\t\t*dev;\n> +\tatomic_t\t\t\tcount;\n> +\n> +\tvoid __iomem\t\t\t*base;\n> +\n> +\tstruct clk\t\t\t*esc_clk;\n> +\tstruct clk\t\t\t*p_clk;\n> +\tstruct clk\t\t\t*pixel_clk[CSI2TX_STREAMS_MAX];\n> +\n> +\tstruct v4l2_subdev\t\tsubdev;\n> +\tstruct media_pad\t\tpads[CSI2TX_PAD_MAX];\n> +\tstruct v4l2_mbus_framefmt\tpad_fmts[CSI2TX_PAD_MAX];\n> +\n> +\tbool\t\t\t\thas_internal_dphy;\n> +\tunsigned int\t\t\tlanes;\n> +\tunsigned int\t\t\tmax_lanes;\n> +\tunsigned int\t\t\tmax_streams;\n> +};\n> +\n> +static struct csi2tx_fmt csi2tx_formats[] = {\n\nconst?\n\n> +\t{\n> +\t\t.mbus\t= MEDIA_BUS_FMT_UYVY8_1X16,\n> +\t\t.bpp\t= 2,\n> +\t\t.dt\t= 0x1e,\n> +\t},\n> +\t{\n> +\t\t.mbus\t= MEDIA_BUS_FMT_RGB888_1X24,\n> +\t\t.bpp\t= 3,\n> +\t\t.dt\t= 0x24,\n> +\t},\n> +};\n> +\n> +static inline\n> +struct csi2tx_priv *v4l2_subdev_to_csi2tx(struct v4l2_subdev *subdev)\n> +{\n> +\treturn container_of(subdev, struct csi2tx_priv, subdev);\n> +}\n> +\n> +static void csi2tx_reset(struct csi2tx_priv *csi2tx)\n> +{\n> +\twritel(CSI2TX_CONFIG_SRST_REQ, csi2tx->base + CSI2TX_CONFIG_REG);\n> +\n> +\tusleep_range(10, 20);\n> +}\n> +\n> +static struct csi2tx_fmt *csitx_get_fmt_from_mbus(struct v4l2_mbus_framefmt *mfmt)\n> +{\n> +\tint i;\n> +\n> +\tif (!mfmt)\n> +\t\treturn NULL;\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(csi2tx_formats); i++)\n> +\t\tif (csi2tx_formats[i].mbus == mfmt->code)\n> +\t\t\treturn &csi2tx_formats[i];\n> +\n> +\treturn NULL;\n> +}\n> +\n> +static int csi2tx_enum_mbus_code(struct v4l2_subdev *subdev,\n> +\t\t\t\t struct v4l2_subdev_pad_config *cfg,\n> +\t\t\t\t struct v4l2_subdev_mbus_code_enum *code)\n> +{\n> +\tif (code->pad || code->index >= ARRAY_SIZE(csi2tx_formats))\n> +\t\treturn -EINVAL;\n> +\n> +\tcode->code = csi2tx_formats[code->index].mbus;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int csi2tx_get_pad_format(struct v4l2_subdev *subdev,\n> +\t\t\t\t struct v4l2_subdev_pad_config *cfg,\n> +\t\t\t\t struct v4l2_subdev_format *fmt)\n> +{\n> +\tstruct csi2tx_priv *csi2tx = v4l2_subdev_to_csi2tx(subdev);\n> +\n> +\tfmt->format = csi2tx->pad_fmts[fmt->pad];\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int csi2tx_set_pad_format(struct v4l2_subdev *subdev,\n> +\t\t\t\t struct v4l2_subdev_pad_config *cfg,\n> +\t\t\t\t struct v4l2_subdev_format *fmt)\n> +{\n> +\tstruct csi2tx_priv *csi2tx = v4l2_subdev_to_csi2tx(subdev);\n> +\n> +\tcsi2tx->pad_fmts[fmt->pad] = fmt->format;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static const struct v4l2_subdev_pad_ops csi2tx_pad_ops = {\n> +\t.enum_mbus_code\t= csi2tx_enum_mbus_code,\n> +\t.get_fmt\t= csi2tx_get_pad_format,\n> +\t.set_fmt\t= csi2tx_set_pad_format,\n> +};\n> +\n> +static int csi2tx_start(struct csi2tx_priv *csi2tx)\n> +{\n> +\tu32 reg;\n> +\tint i;\n> +\n> +\t/*\n> +\t * We're not the first users, there's no need to enable the\n> +\t * whole controller.\n> +\t */\n> +\tif (atomic_inc_return(&csi2tx->count) > 1)\n> +\t\treturn 0;\n> +\n> +\tcsi2tx_reset(csi2tx);\n> +\n> +\twritel(CSI2TX_CONFIG_CFG_REQ, csi2tx->base + CSI2TX_CONFIG_REG);\n> +\n> +\tusleep_range(10, 20);\n> +\n> +\twritel(CSI2TX_DPHY_CLK_WAKEUP_ULPS_CYCLES(32),\n> +\t csi2tx->base + CSI2TX_DPHY_CLK_WAKEUP_REG);\n> +\n> +\t/* Put our lanes (clock and data) out of reset */\n> +\treg = CSI2TX_DPHY_CFG_CLK_RESET | CSI2TX_DPHY_CFG_MODE_LPDT;\n> +\tfor (i = 0; i < csi2tx->lanes; i++)\n> +\t\treg |= CSI2TX_DPHY_CFG_LANE_RESET(i);\n> +\twritel(reg, csi2tx->base + CSI2TX_DPHY_CFG_REG);\n> +\n> +\tusleep_range(10, 20);\n> +\n> +\t/* Enable our (clock and data) lanes */\n> +\treg |= CSI2TX_DPHY_CFG_CLK_ENABLE;\n> +\tfor (i = 0; i < csi2tx->lanes; i++)\n> +\t\treg |= CSI2TX_DPHY_CFG_LANE_ENABLE(i);\n> +\twritel(reg, csi2tx->base + CSI2TX_DPHY_CFG_REG);\n> +\n> +\tusleep_range(10, 20);\n> +\n> +\t/* Switch to HS mode */\n> +\treg &= ~CSI2TX_DPHY_CFG_MODE_MASK;\n> +\twritel(reg | CSI2TX_DPHY_CFG_MODE_HS,\n> +\t csi2tx->base + CSI2TX_DPHY_CFG_REG);\n> +\n> +\tusleep_range(10, 20);\n> +\n> +\t/*\n> +\t * Create a static mapping between the CSI virtual channels\n> +\t * and the input streams.\n\nWhich virtual channel is used here?\n\n> +\t *\n> +\t * This should be enhanced, but v4l2 lacks the support for\n> +\t * changing that mapping dynamically.\n> +\t */\n> +\tfor (i = CSI2TX_PAD_SINK_STREAM0; i < CSI2TX_PAD_MAX; i++) {\n> +\t\tstruct v4l2_mbus_framefmt *mfmt = &csi2tx->pad_fmts[i];\n> +\t\tstruct csi2tx_fmt *fmt = csitx_get_fmt_from_mbus(mfmt);\n> +\t\tint stream = i - CSI2TX_PAD_SINK_STREAM0;\n\nunsigned?\n\n> +\n> +\t\t/*\n> +\t\t * If no-one set a format, we consider this pad\n> +\t\t * disabled and just skip it.\n> +\t\t */\n> +\t\tif (!fmt)\n\nThe pad should have a valid format even if the user didn't configure it.\n\nInstead you should use the link state to determine whether the link is\nactive or not.\n\n> +\t\t\tcontinue;\n> +\n> +\t\t/*\n> +\t\t * We use the stream ID there, but it's wrong.\n> +\t\t *\n> +\t\t * A stream could very well send a data type that is\n> +\t\t * not equal to its stream ID. We need to find a\n> +\t\t * proper way to address it.\n\nStream IDs will presumably be used in V4L2 for a different purpose. Does\nthe hardware documentation call them such?\n\n> +\t\t */\n> +\t\twritel(CSI2TX_DT_CFG_DT(fmt->dt),\n> +\t\t csi2tx->base + CSI2TX_DT_CFG_REG(stream));\n> +\n> +\t\twritel(CSI2TX_DT_FORMAT_BYTES_PER_LINE(mfmt->width * fmt->bpp) |\n> +\t\t CSI2TX_DT_FORMAT_MAX_LINE_NUM(mfmt->height + 1),\n> +\t\t csi2tx->base + CSI2TX_DT_FORMAT_REG(stream));\n> +\n> +\t\t/*\n> +\t\t * TODO: This needs to be calculated based on the\n> +\t\t * clock rate.\n\nClock rate of what? Input?\n\n> +\t\t */\n> +\t\twritel(CSI2TX_STREAM_IF_CFG_FILL_LEVEL(4),\n> +\t\t csi2tx->base + CSI2TX_STREAM_IF_CFG_REG(stream));\n> +\t}\n> +\n> +\t/* Disable the configuration mode */\n> +\twritel(0, csi2tx->base + CSI2TX_CONFIG_REG);\n\nShouldn't you start streaming on the downstream sub-device as well?\n\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int csi2tx_stop(struct csi2tx_priv *csi2tx)\n> +{\n> +\t/*\n> +\t * Let the last user turn off the lights\n> +\t */\n> +\tif (!atomic_dec_and_test(&csi2tx->count))\n> +\t\treturn 0;\n> +\n> +\t/* FIXME: Disable the IP here */\n\nShouldn't this be addressed?\n\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int csi2tx_s_stream(struct v4l2_subdev *subdev, int enable)\n> +{\n> +\tstruct csi2tx_priv *csi2tx = v4l2_subdev_to_csi2tx(subdev);\n> +\tint ret;\n> +\n> +\tif (enable)\n> +\t\tret = csi2tx_start(csi2tx);\n> +\telse\n> +\t\tret = csi2tx_stop(csi2tx);\n> +\n> +\treturn ret;\n> +}\n> +\n> +static const struct v4l2_subdev_video_ops csi2tx_video_ops = {\n> +\t.s_stream\t= csi2tx_s_stream,\n> +};\n> +\n> +static struct v4l2_subdev_ops csi2tx_subdev_ops = {\n\nconst?\n\n> +\t.pad\t\t= &csi2tx_pad_ops,\n> +\t.video\t\t= &csi2tx_video_ops,\n> +};\n> +\n> +static int csi2tx_get_resources(struct csi2tx_priv *csi2tx,\n> +\t\t\t\tstruct platform_device *pdev)\n> +{\n> +\tstruct resource *res;\n> +\tu32 reg;\n> +\tint i;\n> +\n> +\tres = platform_get_resource(pdev, IORESOURCE_MEM, 0);\n> +\tcsi2tx->base = devm_ioremap_resource(&pdev->dev, res);\n> +\tif (IS_ERR(csi2tx->base)) {\n> +\t\tdev_err(&pdev->dev, \"Couldn't map our registers\\n\");\n> +\t\treturn PTR_ERR(csi2tx->base);\n> +\t}\n> +\n> +\tcsi2tx->p_clk = devm_clk_get(&pdev->dev, \"p_clk\");\n> +\tif (IS_ERR(csi2tx->p_clk)) {\n> +\t\tdev_err(&pdev->dev, \"Couldn't get p_clk\\n\");\n> +\t\treturn PTR_ERR(csi2tx->p_clk);\n> +\t}\n> +\n> +\tcsi2tx->esc_clk = devm_clk_get(&pdev->dev, \"esc_clk\");\n> +\tif (IS_ERR(csi2tx->esc_clk)) {\n> +\t\tdev_err(&pdev->dev, \"Couldn't get the esc_clk\\n\");\n> +\t\treturn PTR_ERR(csi2tx->esc_clk);\n> +\t}\n> +\n> +\tclk_prepare_enable(csi2tx->p_clk);\n> +\treg = readl(csi2tx->base + CSI2TX_DEVICE_CONFIG_REG);\n> +\tclk_disable_unprepare(csi2tx->p_clk);\n> +\n> +\tcsi2tx->max_lanes = (reg & 7);\n> +\tif (csi2tx->max_lanes > 4) {\n> +\t\tdev_err(&pdev->dev, \"Invalid number of lanes: %u\\n\",\n> +\t\t\tcsi2tx->max_lanes);\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tcsi2tx->max_streams = ((reg >> 4) & 7);\n> +\tif (csi2tx->max_streams > CSI2TX_STREAMS_MAX) {\n> +\t\tdev_err(&pdev->dev, \"Invalid number of streams: %u\\n\",\n> +\t\t\tcsi2tx->max_streams);\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tcsi2tx->has_internal_dphy = (reg & BIT(3)) ? true : false;\n> +\n> +\tfor (i = 0; i < csi2tx->max_streams; i++) {\n> +\t\tchar clk_name[16];\n> +\n> +\t\tsnprintf(clk_name, sizeof(clk_name), \"pixel_if%u_clk\", i);\n> +\t\tcsi2tx->pixel_clk[i] = devm_clk_get(&pdev->dev, clk_name);\n> +\t\tif (IS_ERR(csi2tx->pixel_clk[i])) {\n> +\t\t\tdev_err(&pdev->dev, \"Couldn't get clock %s\\n\",\n> +\t\t\t\tclk_name);\n> +\t\t\treturn PTR_ERR(csi2tx->pixel_clk[i]);\n> +\t\t}\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int csi2tx_get_num_lanes(struct device *dev)\n> +{\n> +\tstruct v4l2_fwnode_endpoint v4l2_ep;\n> +\tstruct device_node *ep, *np = dev->of_node;\n\nnp is only used once as a function argument. I wouldn't use a local\nvariable for this. Up to you.\n\n> +\tint ret = 0;\n\nYou could omit initialisation. ret is always assigned below.\n\n> +\n> +\tep = of_graph_get_endpoint_by_regs(np, 0, 0);\n> +\tif (!ep)\n> +\t\treturn -EINVAL;\n> +\n> +\tret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep);\n> +\tif (ret) {\n> +\t\tdev_err(dev, \"Could not parse v4l2 endpoint\\n\");\n> +\t\tgoto out;\n> +\t}\n> +\n> +\tif (v4l2_ep.bus_type != V4L2_MBUS_CSI2) {\n> +\t\tdev_err(dev, \"Unsupported media bus type: 0x%x\\n\",\n> +\t\t\tv4l2_ep.bus_type);\n> +\t\tret = -EINVAL;\n> +\t\tgoto out;\n> +\t}\n> +\n> +\tret = v4l2_ep.bus.mipi_csi2.num_data_lanes;\n> +\n> +out:\n> +\tof_node_put(ep);\n> +\treturn ret;\n> +}\n> +\n> +static int csi2tx_probe(struct platform_device *pdev)\n> +{\n> +\tstruct csi2tx_priv *csi2tx;\n> +\tint i, ret;\n> +\n> +\tcsi2tx = kzalloc(sizeof(*csi2tx), GFP_KERNEL);\n> +\tif (!csi2tx)\n> +\t\treturn -ENOMEM;\n> +\tplatform_set_drvdata(pdev, csi2tx);\n> +\tcsi2tx->dev = &pdev->dev;\n> +\n> +\tret = csi2tx_get_resources(csi2tx, pdev);\n> +\tif (ret)\n> +\t\tgoto err_free_priv;\n> +\n> +\tv4l2_subdev_init(&csi2tx->subdev, &csi2tx_subdev_ops);\n> +\tcsi2tx->subdev.owner = THIS_MODULE;\n> +\tcsi2tx->subdev.dev = &pdev->dev;\n> +\tcsi2tx->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;\n> +\tv4l2_set_subdevdata(&csi2tx->subdev, &pdev->dev);\n> +\tsnprintf(csi2tx->subdev.name, V4L2_SUBDEV_NAME_SIZE, \"%s.%s\",\n> +\t\t KBUILD_MODNAME, dev_name(&pdev->dev));\n> +\n> +\tcsi2tx->lanes = csi2tx_get_num_lanes(&pdev->dev);\n> +\tif (csi2tx->lanes < 0) {\n> +\t\tdev_err(&pdev->dev, \"Invalid number of lanes, bailing out.\\n\");\n> +\t\tret = csi2tx->lanes;\n> +\t\tgoto err_free_priv;\n> +\t}\n> +\n> +\tif (csi2tx->lanes > csi2tx->max_lanes) {\n> +\t\tdev_err(&pdev->dev,\n> +\t\t\t\"Current configuration uses more lanes than supported\\n\");\n> +\t\tret = -EINVAL;\n> +\t\tgoto err_free_priv;\n> +\t}\n> +\n> +\t/* Create our media pads */\n> +\tcsi2tx->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;\n> +\tcsi2tx->pads[CSI2TX_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;\n> +\tfor (i = CSI2TX_PAD_SINK_STREAM0; i < CSI2TX_PAD_MAX; i++)\n> +\t\tcsi2tx->pads[i].flags = MEDIA_PAD_FL_SINK;\n> +\n> +\tret = media_entity_pads_init(&csi2tx->subdev.entity, CSI2TX_PAD_MAX,\n> +\t\t\t\t csi2tx->pads);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = v4l2_async_register_subdev(&csi2tx->subdev);\n> +\tif (ret < 0)\n> +\t\tgoto err_free_priv;\n> +\n> +\tdev_info(&pdev->dev,\n> +\t\t \"Probed CSI2TX with %u/%u lanes, %u streams, %s D-PHY\\n\",\n> +\t\t csi2tx->lanes, csi2tx->max_lanes, csi2tx->max_streams,\n> +\t\t csi2tx->has_internal_dphy ? \"internal\" : \"no\");\n> +\n> +\treturn 0;\n> +\n> +err_free_priv:\n> +\tkfree(csi2tx);\n> +\treturn ret;\n> +}\n> +\n> +static int csi2tx_remove(struct platform_device *pdev)\n> +{\n> +\tstruct csi2tx_priv *csi2tx = platform_get_drvdata(pdev);\n> +\n> +\tv4l2_async_unregister_subdev(&csi2tx->subdev);\n> +\tkfree(csi2tx);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static const struct of_device_id csi2tx_of_table[] = {\n> +\t{ .compatible = \"cdns,csi2tx\" },\n> +\t{ },\n> +};\n> +MODULE_DEVICE_TABLE(of, csi2tx_of_table);\n> +\n> +static struct platform_driver csi2tx_driver = {\n> +\t.probe\t= csi2tx_probe,\n> +\t.remove\t= csi2tx_remove,\n> +\n> +\t.driver\t= {\n> +\t\t.name\t\t= \"cdns-csi2tx\",\n> +\t\t.of_match_table\t= csi2tx_of_table,\n> +\t},\n> +};\n> +module_platform_driver(csi2tx_driver);","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xzChb4214z9sPm\n\tfor ;\n\tFri, 22 Sep 2017 22:38:55 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752337AbdIVMiy (ORCPT\n\t);\n\tFri, 22 Sep 2017 08:38:54 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:44210 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1752325AbdIVMix (ORCPT\n\t); Fri, 22 Sep 2017 08:38:53 -0400","from valkosipuli.localdomain (valkosipuli.retiisi.org.uk\n\t[IPv6:2001:1bc8:1a6:d3d5::80:2])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id 6855E600FC;\n\tFri, 22 Sep 2017 15:38:50 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1dvNEH-0005fd-VB; Fri, 22 Sep 2017 15:38:50 +0300"],"Date":"Fri, 22 Sep 2017 15:38:49 +0300","From":"Sakari Ailus ","To":"Maxime Ripard ","Cc":"Mauro Carvalho Chehab , Mark Rutland\n\t, Rob Herring , Laurent\n\tPinchart , \n\tlinux-media@vger.kernel.org, devicetree@vger.kernel.org, Cyprian Wronka\n\t, Richard Sproul , Alan Douglas\n\t, Steve Creaney , Thomas\n\tPetazzoni , Boris Brezillon\n\t, Niklas =?iso-8859-1?q?S=F6derlun?=\n\t=?iso-8859-1?q?d?= ,\n\tHans Verkuil , Sakari Ailus\n\t, \n\tBenoit Parrot , nm@ti.com","Subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","Message-ID":"<20170922123849.hcm76tlplnvd44mt@valkosipuli.retiisi.org.uk>","References":"<20170922114703.30511-1-maxime.ripard@free-electrons.com>\n\t<20170922114703.30511-3-maxime.ripard@free-electrons.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170922114703.30511-3-maxime.ripard@free-electrons.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1773695,"web_url":"http://patchwork.ozlabs.org/comment/1773695/","msgid":"<20170922153036.u7k3wmuldphkk6w3@flea.lan>","list_archive_url":null,"date":"2017-09-22T15:30:36","subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","submitter":{"id":12916,"url":"http://patchwork.ozlabs.org/api/people/12916/","name":"Maxime Ripard","email":"maxime.ripard@free-electrons.com"},"content":"Hi Sakari,\n\nI'll address the minor comments you had and that I stripped.\n\nOn Fri, Sep 22, 2017 at 12:38:49PM +0000, Sakari Ailus wrote:\n> > +\t/*\n> > +\t * Create a static mapping between the CSI virtual channels\n> > +\t * and the input streams.\n> \n> Which virtual channel is used here?\n\nLike I was trying to explain in the cover letter, the virtual channel\nis not under that block's control. The input video interfaces have an\nadditional signal that comes from the upstream device which sets the\nvirtual channel.\n\nIt's transparent to the CSI2-TX block, even though it's\nthere. Depending on the implementation, it can be either fixed or can\nchange, it's up to the other block's designer. The only restriction is\nthat it cannot change while a streaming is occuring.\n\n> > +\n> > +\t\t/*\n> > +\t\t * If no-one set a format, we consider this pad\n> > +\t\t * disabled and just skip it.\n> > +\t\t */\n> > +\t\tif (!fmt)\n> \n> The pad should have a valid format even if the user didn't configure\n> it.\n\nWhich format should be by default then?\n\n> Instead you should use the link state to determine whether the link is\n> active or not.\n\nOk, will do.\n\n> > +\t\t\tcontinue;\n> > +\n> > +\t\t/*\n> > +\t\t * We use the stream ID there, but it's wrong.\n> > +\t\t *\n> > +\t\t * A stream could very well send a data type that is\n> > +\t\t * not equal to its stream ID. We need to find a\n> > +\t\t * proper way to address it.\n> \n> Stream IDs will presumably be used in V4L2 for a different purpose. Does\n> the hardware documentation call them such?\n\nInput video interfaces are called streams, yes, and then they are\nnumbered. If it's just confusing because of a collision with one of\nv4l2's nomenclature, I'm totally fine to change it to some other name.\n\n> > +\t\t */\n> > +\t\twritel(CSI2TX_DT_CFG_DT(fmt->dt),\n> > +\t\t csi2tx->base + CSI2TX_DT_CFG_REG(stream));\n> > +\n> > +\t\twritel(CSI2TX_DT_FORMAT_BYTES_PER_LINE(mfmt->width * fmt->bpp) |\n> > +\t\t CSI2TX_DT_FORMAT_MAX_LINE_NUM(mfmt->height + 1),\n> > +\t\t csi2tx->base + CSI2TX_DT_FORMAT_REG(stream));\n> > +\n> > +\t\t/*\n> > +\t\t * TODO: This needs to be calculated based on the\n> > +\t\t * clock rate.\n> \n> Clock rate of what? Input?\n\nOf the CSI2 link, so output. I guess I should make that clearer.\n\n> \n> > +\t\t */\n> > +\t\twritel(CSI2TX_STREAM_IF_CFG_FILL_LEVEL(4),\n> > +\t\t csi2tx->base + CSI2TX_STREAM_IF_CFG_REG(stream));\n> > +\t}\n> > +\n> > +\t/* Disable the configuration mode */\n> > +\twritel(0, csi2tx->base + CSI2TX_CONFIG_REG);\n> \n> Shouldn't you start streaming on the downstream sub-device as well?\n\nI appreciate it's a pretty weak argument, but the current setup we\nhave is in the FPGA is:\n\ncapture <- CSI2-RX <- CSI2-TX <- pattern generator\n\nSo far, the CSI2-RX block is calling its remote sub-device, which is\nCSI2-TX. If CSI2-RX is calling its remote sub-device (CSI2-RX), we\njust found ourselves in an endless loop.\n\nI guess it should be easier, and fixable, when we'll have an actual\ndevice without such a loopback.\n\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +static int csi2tx_stop(struct csi2tx_priv *csi2tx)\n> > +{\n> > +\t/*\n> > +\t * Let the last user turn off the lights\n> > +\t */\n> > +\tif (!atomic_dec_and_test(&csi2tx->count))\n> > +\t\treturn 0;\n> > +\n> > +\t/* FIXME: Disable the IP here */\n> \n> Shouldn't this be addressed?\n\nYes, but it's still unclear how at the moment. It will of course\neventually be implemented.\n\nThanks!\nMaxime","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xzHVm5T2kz9t3Z\n\tfor ;\n\tSat, 23 Sep 2017 01:30:40 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751977AbdIVPaj (ORCPT\n\t);\n\tFri, 22 Sep 2017 11:30:39 -0400","from mail.free-electrons.com ([62.4.15.54]:39359 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751923AbdIVPai (ORCPT\n\t); Fri, 22 Sep 2017 11:30:38 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid 3C6D9209EB; Fri, 22 Sep 2017 17:30:36 +0200 (CEST)","from localhost (LStLambert-657-1-97-87.w90-63.abo.wanadoo.fr\n\t[90.63.216.87])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id 0F9A3208F6;\n\tFri, 22 Sep 2017 17:30:36 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT,\n\tURIBL_BLOCKED shortcircuit=ham autolearn=disabled version=3.4.0","Date":"Fri, 22 Sep 2017 17:30:36 +0200","From":"Maxime Ripard ","To":"Sakari Ailus ","Cc":"Mauro Carvalho Chehab , Mark Rutland\n\t, Rob Herring , Laurent\n\tPinchart , \n\tlinux-media@vger.kernel.org, devicetree@vger.kernel.org, Cyprian Wronka\n\t, Richard Sproul , Alan Douglas\n\t, Steve Creaney , Thomas\n\tPetazzoni , Boris Brezillon\n\t, Niklas =?iso-8859-1?q?S=F6derlun?=\n\t=?iso-8859-1?q?d?= ,\n\tHans Verkuil , Sakari Ailus\n\t, \n\tBenoit Parrot , nm@ti.com","Subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","Message-ID":"<20170922153036.u7k3wmuldphkk6w3@flea.lan>","References":"<20170922114703.30511-1-maxime.ripard@free-electrons.com>\n\t<20170922114703.30511-3-maxime.ripard@free-electrons.com>\n\t<20170922123849.hcm76tlplnvd44mt@valkosipuli.retiisi.org.uk>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"j4nwboesqc6za5ms\"","Content-Disposition":"inline","In-Reply-To":"<20170922123849.hcm76tlplnvd44mt@valkosipuli.retiisi.org.uk>","User-Agent":"NeoMutt/20170914 (1.9.0)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1775224,"web_url":"http://patchwork.ozlabs.org/comment/1775224/","msgid":"<20170926080014.7a3lbe23rvzpcmkq@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-09-26T08:00:14","subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Maxime,\n\nOn Fri, Sep 22, 2017 at 05:30:36PM +0200, Maxime Ripard wrote:\n> Hi Sakari,\n> \n> I'll address the minor comments you had and that I stripped.\n> \n> On Fri, Sep 22, 2017 at 12:38:49PM +0000, Sakari Ailus wrote:\n> > > +\t/*\n> > > +\t * Create a static mapping between the CSI virtual channels\n> > > +\t * and the input streams.\n> > \n> > Which virtual channel is used here?\n> \n> Like I was trying to explain in the cover letter, the virtual channel\n> is not under that block's control. The input video interfaces have an\n> additional signal that comes from the upstream device which sets the\n> virtual channel.\n\nOh, I missed while reviewing the set.\n\nPresumably either driver would be in control of that then (this one or the\nupstream sub-device).\n\n> \n> It's transparent to the CSI2-TX block, even though it's\n> there. Depending on the implementation, it can be either fixed or can\n> change, it's up to the other block's designer. The only restriction is\n> that it cannot change while a streaming is occuring.\n> \n> > > +\n> > > +\t\t/*\n> > > +\t\t * If no-one set a format, we consider this pad\n> > > +\t\t * disabled and just skip it.\n> > > +\t\t */\n> > > +\t\tif (!fmt)\n> > \n> > The pad should have a valid format even if the user didn't configure\n> > it.\n> \n> Which format should be by default then?\n\nAny valid format for the device should be good.\n\n> \n> > Instead you should use the link state to determine whether the link is\n> > active or not.\n> \n> Ok, will do.\n> \n> > > +\t\t\tcontinue;\n> > > +\n> > > +\t\t/*\n> > > +\t\t * We use the stream ID there, but it's wrong.\n> > > +\t\t *\n> > > +\t\t * A stream could very well send a data type that is\n> > > +\t\t * not equal to its stream ID. We need to find a\n> > > +\t\t * proper way to address it.\n> > \n> > Stream IDs will presumably be used in V4L2 for a different purpose. Does\n> > the hardware documentation call them such?\n> \n> Input video interfaces are called streams, yes, and then they are\n> numbered. If it's just confusing because of a collision with one of\n> v4l2's nomenclature, I'm totally fine to change it to some other name.\n\nIf the hardware documentation uses it then let's stick with it.\n\n> \n> > > +\t\t */\n> > > +\t\twritel(CSI2TX_DT_CFG_DT(fmt->dt),\n> > > +\t\t csi2tx->base + CSI2TX_DT_CFG_REG(stream));\n> > > +\n> > > +\t\twritel(CSI2TX_DT_FORMAT_BYTES_PER_LINE(mfmt->width * fmt->bpp) |\n> > > +\t\t CSI2TX_DT_FORMAT_MAX_LINE_NUM(mfmt->height + 1),\n> > > +\t\t csi2tx->base + CSI2TX_DT_FORMAT_REG(stream));\n> > > +\n> > > +\t\t/*\n> > > +\t\t * TODO: This needs to be calculated based on the\n> > > +\t\t * clock rate.\n> > \n> > Clock rate of what? Input?\n> \n> Of the CSI2 link, so output. I guess I should make that clearer.\n\nPlease.\n\n> \n> > \n> > > +\t\t */\n> > > +\t\twritel(CSI2TX_STREAM_IF_CFG_FILL_LEVEL(4),\n> > > +\t\t csi2tx->base + CSI2TX_STREAM_IF_CFG_REG(stream));\n> > > +\t}\n> > > +\n> > > +\t/* Disable the configuration mode */\n> > > +\twritel(0, csi2tx->base + CSI2TX_CONFIG_REG);\n> > \n> > Shouldn't you start streaming on the downstream sub-device as well?\n> \n> I appreciate it's a pretty weak argument, but the current setup we\n> have is in the FPGA is:\n> \n> capture <- CSI2-RX <- CSI2-TX <- pattern generator\n> \n> So far, the CSI2-RX block is calling its remote sub-device, which is\n> CSI2-TX. If CSI2-RX is calling its remote sub-device (CSI2-RX), we\n> just found ourselves in an endless loop.\n> \n> I guess it should be easier, and fixable, when we'll have an actual\n> device without such a loopback.\n\nWhat's the intended use case of the device, capture or output?\n\nHow do you currently start the pipeline?\n\nWe have a few corner cases in V4L2 for such devices in graph parsing and\nstream control. The parsing of the device's fwnode graph endpoints are what\nthe device can do, but it doesn't know where the parsing should continue\nand which part of the graph is already parsed.\n\nThat will be addressed but right now a driver just needs to know.\n\n> \n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > +static int csi2tx_stop(struct csi2tx_priv *csi2tx)\n> > > +{\n> > > +\t/*\n> > > +\t * Let the last user turn off the lights\n> > > +\t */\n> > > +\tif (!atomic_dec_and_test(&csi2tx->count))\n> > > +\t\treturn 0;\n> > > +\n> > > +\t/* FIXME: Disable the IP here */\n> > \n> > Shouldn't this be addressed?\n> \n> Yes, but it's still unclear how at the moment. It will of course\n> eventually be implemented.\n\nOk.","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y1YKM0Vxzz9tXj\n\tfor ;\n\tTue, 26 Sep 2017 18:00:23 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S967849AbdIZIAT (ORCPT );\n\tTue, 26 Sep 2017 04:00:19 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:54938 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S967328AbdIZIAR (ORCPT\n\t); Tue, 26 Sep 2017 04:00:17 -0400","from valkosipuli.localdomain (valkosipuli.retiisi.org.uk\n\t[IPv6:2001:1bc8:1a6:d3d5::80:2])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id 4EB9260102;\n\tTue, 26 Sep 2017 11:00:15 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from )\n\tid 1dwkms-0007Cn-Pj; Tue, 26 Sep 2017 11:00:14 +0300"],"Date":"Tue, 26 Sep 2017 11:00:14 +0300","From":"Sakari Ailus ","To":"Maxime Ripard ","Cc":"Mauro Carvalho Chehab , Mark Rutland\n\t, Rob Herring , Laurent\n\tPinchart , \n\tlinux-media@vger.kernel.org, devicetree@vger.kernel.org, Cyprian Wronka\n\t, Richard Sproul , Alan Douglas\n\t, Steve Creaney , Thomas\n\tPetazzoni , Boris Brezillon\n\t, Niklas =?iso-8859-1?q?S=F6derlun?=\n\t=?iso-8859-1?q?d?= ,\n\tHans Verkuil , Sakari Ailus\n\t, \n\tBenoit Parrot , nm@ti.com","Subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","Message-ID":"<20170926080014.7a3lbe23rvzpcmkq@valkosipuli.retiisi.org.uk>","References":"<20170922114703.30511-1-maxime.ripard@free-electrons.com>\n\t<20170922114703.30511-3-maxime.ripard@free-electrons.com>\n\t<20170922123849.hcm76tlplnvd44mt@valkosipuli.retiisi.org.uk>\n\t<20170922153036.u7k3wmuldphkk6w3@flea.lan>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170922153036.u7k3wmuldphkk6w3@flea.lan>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1777708,"web_url":"http://patchwork.ozlabs.org/comment/1777708/","msgid":"<20170929182125.GB3163@ti.com>","list_archive_url":null,"date":"2017-09-29T18:21:25","subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","submitter":{"id":64914,"url":"http://patchwork.ozlabs.org/api/people/64914/","name":"Benoit Parrot","email":"bparrot@ti.com"},"content":"Maxime,\n\nThank you for the patch.\n\nMaxime Ripard wrote on Fri [2017-Sep-22 13:47:03 +0200]:\n> The Cadence MIPI-CSI2 TX controller is an hardware block meant to be used\n> as a bridge between pixel interfaces and a CSI-2 bus.\n> \n> It supports operating with an internal or external D-PHY, with up to 4\n> lanes, or without any D-PHY. The current code only supports the former\n> case.\n> \n> While the virtual channel input on the pixel interface can be directly\n> mapped to CSI2, the datatype input is actually a selection signal (3-bits)\n> mapping to a table of up to 8 preconfigured datatypes/formats (programmed\n> at start-up)\n> \n> The block supports up to 8 input datatypes.\n> \n> Signed-off-by: Maxime Ripard \n> ---\n> drivers/media/platform/cadence/Kconfig | 6 +\n> drivers/media/platform/cadence/Makefile | 1 +\n> drivers/media/platform/cadence/cdns-csi2tx.c | 479 +++++++++++++++++++++++++++\n> 3 files changed, 486 insertions(+)\n> create mode 100644 drivers/media/platform/cadence/cdns-csi2tx.c\n> \n> diff --git a/drivers/media/platform/cadence/Kconfig b/drivers/media/platform/cadence/Kconfig\n> index d1b6bbb6a0eb..db49328ee8b2 100644\n> --- a/drivers/media/platform/cadence/Kconfig\n> +++ b/drivers/media/platform/cadence/Kconfig\n> @@ -9,4 +9,10 @@ config VIDEO_CADENCE_CSI2RX\n> \tdepends on VIDEO_V4L2_SUBDEV_API\n> \tselect V4L2_FWNODE\n> \n> +config VIDEO_CADENCE_CSI2TX\n> +\ttristate \"Cadence MIPI-CSI2 TX Controller\"\n> +\tdepends on MEDIA_CONTROLLER\n> +\tdepends on VIDEO_V4L2_SUBDEV_API\n> +\tselect V4L2_FWNODE\n> +\n> endif\n> diff --git a/drivers/media/platform/cadence/Makefile b/drivers/media/platform/cadence/Makefile\n> index 99a4086b7448..7fe992273162 100644\n> --- a/drivers/media/platform/cadence/Makefile\n> +++ b/drivers/media/platform/cadence/Makefile\n> @@ -1 +1,2 @@\n> obj-$(CONFIG_VIDEO_CADENCE_CSI2RX)\t+= cdns-csi2rx.o\n> +obj-$(CONFIG_VIDEO_CADENCE_CSI2TX)\t+= cdns-csi2tx.o\n> diff --git a/drivers/media/platform/cadence/cdns-csi2tx.c b/drivers/media/platform/cadence/cdns-csi2tx.c\n> new file mode 100644\n> index 000000000000..859bbce8772b\n> --- /dev/null\n> +++ b/drivers/media/platform/cadence/cdns-csi2tx.c\n> @@ -0,0 +1,479 @@\n> +/*\n> + * Driver for Cadence MIPI-CSI2 TX Controller\n> + *\n> + * Copyright (C) 2017 Cadence Design Systems Inc.\n> + *\n> + * This program is free software; you can redistribute it and/or modify it\n> + * under the terms of the GNU General Public License as published by the\n> + * Free Software Foundation; either version 2 of the License, or (at your\n> + * option) any later version.\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#define CSI2TX_DEVICE_CONFIG_REG\t0x00\n> +\n> +#define CSI2TX_CONFIG_REG\t\t0x20\n> +#define CSI2TX_CONFIG_CFG_REQ\t\t\tBIT(2)\n> +#define CSI2TX_CONFIG_SRST_REQ\t\t\tBIT(1)\n> +\n> +#define CSI2TX_DPHY_CFG_REG\t\t0x28\n> +#define CSI2TX_DPHY_CFG_CLK_RESET\t\tBIT(16)\n> +#define CSI2TX_DPHY_CFG_LANE_RESET(n)\t\tBIT((n) + 12)\n> +#define CSI2TX_DPHY_CFG_MODE_MASK\t\tGENMASK(9, 8)\n> +#define CSI2TX_DPHY_CFG_MODE_LPDT\t\t(2 << 8)\n> +#define CSI2TX_DPHY_CFG_MODE_HS\t\t\t(1 << 8)\n> +#define CSI2TX_DPHY_CFG_MODE_ULPS\t\t(0 << 8)\n> +#define CSI2TX_DPHY_CFG_CLK_ENABLE\t\tBIT(4)\n> +#define CSI2TX_DPHY_CFG_LANE_ENABLE(n)\t\tBIT(n)\n> +\n> +#define CSI2TX_DPHY_CLK_WAKEUP_REG\t0x2c\n> +#define CSI2TX_DPHY_CLK_WAKEUP_ULPS_CYCLES(n)\t((n) & 0xffff)\n> +\n> +#define CSI2TX_DT_CFG_REG(n)\t\t(0x80 + (n) * 8)\n> +#define CSI2TX_DT_CFG_DT(n)\t\t\t(((n) & 0x3f) << 2)\n> +\n> +#define CSI2TX_DT_FORMAT_REG(n)\t\t(0x84 + (n) * 8)\n> +#define CSI2TX_DT_FORMAT_BYTES_PER_LINE(n)\t(((n) & 0xffff) << 16)\n> +#define CSI2TX_DT_FORMAT_MAX_LINE_NUM(n)\t((n) & 0xffff)\n> +\n> +#define CSI2TX_STREAM_IF_CFG_REG(n)\t(0x100 + (n) * 4)\n> +#define CSI2TX_STREAM_IF_CFG_FILL_LEVEL(n)\t((n) & 0x1f)\n> +\n> +#define CSI2TX_STREAMS_MAX\t4\n> +\n> +enum csi2tx_pads {\n> +\tCSI2TX_PAD_SOURCE,\n> +\tCSI2TX_PAD_SINK_STREAM0,\n> +\tCSI2TX_PAD_SINK_STREAM1,\n> +\tCSI2TX_PAD_SINK_STREAM2,\n> +\tCSI2TX_PAD_SINK_STREAM3,\n> +\tCSI2TX_PAD_MAX,\n> +};\n> +\n> +struct csi2tx_fmt {\n> +\tu32\tmbus;\n> +\tu32\tdt;\n> +\tu32\tbpp;\n> +};\n> +\n> +struct csi2tx_priv {\n> +\tstruct device\t\t\t*dev;\n> +\tatomic_t\t\t\tcount;\n> +\n> +\tvoid __iomem\t\t\t*base;\n> +\n> +\tstruct clk\t\t\t*esc_clk;\n> +\tstruct clk\t\t\t*p_clk;\n> +\tstruct clk\t\t\t*pixel_clk[CSI2TX_STREAMS_MAX];\n> +\n> +\tstruct v4l2_subdev\t\tsubdev;\n> +\tstruct media_pad\t\tpads[CSI2TX_PAD_MAX];\n> +\tstruct v4l2_mbus_framefmt\tpad_fmts[CSI2TX_PAD_MAX];\n> +\n> +\tbool\t\t\t\thas_internal_dphy;\n\nI assume dphy support is for a subsequent revision?\n\n> +\tunsigned int\t\t\tlanes;\n> +\tunsigned int\t\t\tmax_lanes;\n> +\tunsigned int\t\t\tmax_streams;\n> +};\n> +\n> +static struct csi2tx_fmt csi2tx_formats[] = {\n> +\t{\n> +\t\t.mbus\t= MEDIA_BUS_FMT_UYVY8_1X16,\n> +\t\t.bpp\t= 2,\n> +\t\t.dt\t= 0x1e,\n> +\t},\n> +\t{\n> +\t\t.mbus\t= MEDIA_BUS_FMT_RGB888_1X24,\n> +\t\t.bpp\t= 3,\n> +\t\t.dt\t= 0x24,\n> +\t},\n> +};\n> +\n> +static inline\n> +struct csi2tx_priv *v4l2_subdev_to_csi2tx(struct v4l2_subdev *subdev)\n> +{\n> +\treturn container_of(subdev, struct csi2tx_priv, subdev);\n> +}\n> +\n> +static void csi2tx_reset(struct csi2tx_priv *csi2tx)\n> +{\n> +\twritel(CSI2TX_CONFIG_SRST_REQ, csi2tx->base + CSI2TX_CONFIG_REG);\n> +\n> +\tusleep_range(10, 20);\n> +}\n> +\n> +static struct csi2tx_fmt *csitx_get_fmt_from_mbus(struct v4l2_mbus_framefmt *mfmt)\n> +{\n> +\tint i;\n> +\n> +\tif (!mfmt)\n> +\t\treturn NULL;\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(csi2tx_formats); i++)\n> +\t\tif (csi2tx_formats[i].mbus == mfmt->code)\n> +\t\t\treturn &csi2tx_formats[i];\n> +\n> +\treturn NULL;\n> +}\n> +\n> +static int csi2tx_enum_mbus_code(struct v4l2_subdev *subdev,\n> +\t\t\t\t struct v4l2_subdev_pad_config *cfg,\n> +\t\t\t\t struct v4l2_subdev_mbus_code_enum *code)\n> +{\n> +\tif (code->pad || code->index >= ARRAY_SIZE(csi2tx_formats))\n> +\t\treturn -EINVAL;\n> +\n> +\tcode->code = csi2tx_formats[code->index].mbus;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int csi2tx_get_pad_format(struct v4l2_subdev *subdev,\n> +\t\t\t\t struct v4l2_subdev_pad_config *cfg,\n> +\t\t\t\t struct v4l2_subdev_format *fmt)\n> +{\n> +\tstruct csi2tx_priv *csi2tx = v4l2_subdev_to_csi2tx(subdev);\n> +\n> +\tfmt->format = csi2tx->pad_fmts[fmt->pad];\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int csi2tx_set_pad_format(struct v4l2_subdev *subdev,\n> +\t\t\t\t struct v4l2_subdev_pad_config *cfg,\n> +\t\t\t\t struct v4l2_subdev_format *fmt)\n> +{\n> +\tstruct csi2tx_priv *csi2tx = v4l2_subdev_to_csi2tx(subdev);\n> +\n> +\tcsi2tx->pad_fmts[fmt->pad] = fmt->format;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static const struct v4l2_subdev_pad_ops csi2tx_pad_ops = {\n> +\t.enum_mbus_code\t= csi2tx_enum_mbus_code,\n> +\t.get_fmt\t= csi2tx_get_pad_format,\n> +\t.set_fmt\t= csi2tx_set_pad_format,\n> +};\n> +\n> +static int csi2tx_start(struct csi2tx_priv *csi2tx)\n> +{\n> +\tu32 reg;\n> +\tint i;\n> +\n> +\t/*\n> +\t * We're not the first users, there's no need to enable the\n> +\t * whole controller.\n> +\t */\n> +\tif (atomic_inc_return(&csi2tx->count) > 1)\n> +\t\treturn 0;\n> +\n> +\tcsi2tx_reset(csi2tx);\n> +\n> +\twritel(CSI2TX_CONFIG_CFG_REQ, csi2tx->base + CSI2TX_CONFIG_REG);\n> +\n> +\tusleep_range(10, 20);\n> +\n> +\twritel(CSI2TX_DPHY_CLK_WAKEUP_ULPS_CYCLES(32),\n> +\t csi2tx->base + CSI2TX_DPHY_CLK_WAKEUP_REG);\n> +\n> +\t/* Put our lanes (clock and data) out of reset */\n> +\treg = CSI2TX_DPHY_CFG_CLK_RESET | CSI2TX_DPHY_CFG_MODE_LPDT;\n> +\tfor (i = 0; i < csi2tx->lanes; i++)\n> +\t\treg |= CSI2TX_DPHY_CFG_LANE_RESET(i);\n> +\twritel(reg, csi2tx->base + CSI2TX_DPHY_CFG_REG);\n> +\n> +\tusleep_range(10, 20);\n> +\n> +\t/* Enable our (clock and data) lanes */\n> +\treg |= CSI2TX_DPHY_CFG_CLK_ENABLE;\n> +\tfor (i = 0; i < csi2tx->lanes; i++)\n> +\t\treg |= CSI2TX_DPHY_CFG_LANE_ENABLE(i);\n> +\twritel(reg, csi2tx->base + CSI2TX_DPHY_CFG_REG);\n> +\n> +\tusleep_range(10, 20);\n> +\n> +\t/* Switch to HS mode */\n> +\treg &= ~CSI2TX_DPHY_CFG_MODE_MASK;\n> +\twritel(reg | CSI2TX_DPHY_CFG_MODE_HS,\n> +\t csi2tx->base + CSI2TX_DPHY_CFG_REG);\n> +\n> +\tusleep_range(10, 20);\n> +\n> +\t/*\n> +\t * Create a static mapping between the CSI virtual channels\n> +\t * and the input streams.\n> +\t *\n> +\t * This should be enhanced, but v4l2 lacks the support for\n> +\t * changing that mapping dynamically.\n> +\t */\n> +\tfor (i = CSI2TX_PAD_SINK_STREAM0; i < CSI2TX_PAD_MAX; i++) {\n> +\t\tstruct v4l2_mbus_framefmt *mfmt = &csi2tx->pad_fmts[i];\n> +\t\tstruct csi2tx_fmt *fmt = csitx_get_fmt_from_mbus(mfmt);\n> +\t\tint stream = i - CSI2TX_PAD_SINK_STREAM0;\n> +\n> +\t\t/*\n> +\t\t * If no-one set a format, we consider this pad\n> +\t\t * disabled and just skip it.\n> +\t\t */\n> +\t\tif (!fmt)\n> +\t\t\tcontinue;\n> +\n\nAs Sakari already pointed out setting a valid default is the\nusual practice.\n\n> +\t\t/*\n> +\t\t * We use the stream ID there, but it's wrong.\n> +\t\t *\n> +\t\t * A stream could very well send a data type that is\n> +\t\t * not equal to its stream ID. We need to find a\n> +\t\t * proper way to address it.\n> +\t\t */\n\nI don't quite get the above comment, from the code below it looks like\nyou are using the current fmt as a source to provide the correct DT.\nAm I missing soemthing?\n\n> +\t\twritel(CSI2TX_DT_CFG_DT(fmt->dt),\n> +\t\t csi2tx->base + CSI2TX_DT_CFG_REG(stream));\n> +\n> +\t\twritel(CSI2TX_DT_FORMAT_BYTES_PER_LINE(mfmt->width * fmt->bpp) |\n> +\t\t CSI2TX_DT_FORMAT_MAX_LINE_NUM(mfmt->height + 1),\n> +\t\t csi2tx->base + CSI2TX_DT_FORMAT_REG(stream));\n> +\n> +\t\t/*\n> +\t\t * TODO: This needs to be calculated based on the\n> +\t\t * clock rate.\n> +\t\t */\n> +\t\twritel(CSI2TX_STREAM_IF_CFG_FILL_LEVEL(4),\n> +\t\t csi2tx->base + CSI2TX_STREAM_IF_CFG_REG(stream));\n> +\t}\n> +\n> +\t/* Disable the configuration mode */\n> +\twritel(0, csi2tx->base + CSI2TX_CONFIG_REG);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int csi2tx_stop(struct csi2tx_priv *csi2tx)\n> +{\n> +\t/*\n> +\t * Let the last user turn off the lights\n> +\t */\n> +\tif (!atomic_dec_and_test(&csi2tx->count))\n> +\t\treturn 0;\n> +\n> +\t/* FIXME: Disable the IP here */\n> +\n> +\treturn 0;\n> +}\n\nAt this point both _start() and _stop() only return 0.\nAre there any cases where any of these might fail to \"start\" or \"stop\"?\n\n> +\n> +static int csi2tx_s_stream(struct v4l2_subdev *subdev, int enable)\n> +{\n> +\tstruct csi2tx_priv *csi2tx = v4l2_subdev_to_csi2tx(subdev);\n> +\tint ret;\n> +\n> +\tif (enable)\n> +\t\tret = csi2tx_start(csi2tx);\n> +\telse\n> +\t\tret = csi2tx_stop(csi2tx);\n> +\n> +\treturn ret;\n> +}\n> +\n> +static const struct v4l2_subdev_video_ops csi2tx_video_ops = {\n> +\t.s_stream\t= csi2tx_s_stream,\n> +};\n> +\n> +static struct v4l2_subdev_ops csi2tx_subdev_ops = {\n> +\t.pad\t\t= &csi2tx_pad_ops,\n> +\t.video\t\t= &csi2tx_video_ops,\n> +};\n> +\n> +static int csi2tx_get_resources(struct csi2tx_priv *csi2tx,\n> +\t\t\t\tstruct platform_device *pdev)\n> +{\n> +\tstruct resource *res;\n> +\tu32 reg;\n> +\tint i;\n> +\n> +\tres = platform_get_resource(pdev, IORESOURCE_MEM, 0);\n> +\tcsi2tx->base = devm_ioremap_resource(&pdev->dev, res);\n> +\tif (IS_ERR(csi2tx->base)) {\n> +\t\tdev_err(&pdev->dev, \"Couldn't map our registers\\n\");\n> +\t\treturn PTR_ERR(csi2tx->base);\n> +\t}\n> +\n> +\tcsi2tx->p_clk = devm_clk_get(&pdev->dev, \"p_clk\");\n> +\tif (IS_ERR(csi2tx->p_clk)) {\n> +\t\tdev_err(&pdev->dev, \"Couldn't get p_clk\\n\");\n> +\t\treturn PTR_ERR(csi2tx->p_clk);\n> +\t}\n> +\n> +\tcsi2tx->esc_clk = devm_clk_get(&pdev->dev, \"esc_clk\");\n> +\tif (IS_ERR(csi2tx->esc_clk)) {\n> +\t\tdev_err(&pdev->dev, \"Couldn't get the esc_clk\\n\");\n> +\t\treturn PTR_ERR(csi2tx->esc_clk);\n> +\t}\n> +\n> +\tclk_prepare_enable(csi2tx->p_clk);\n> +\treg = readl(csi2tx->base + CSI2TX_DEVICE_CONFIG_REG);\n> +\tclk_disable_unprepare(csi2tx->p_clk);\n> +\n> +\tcsi2tx->max_lanes = (reg & 7);\n> +\tif (csi2tx->max_lanes > 4) {\n> +\t\tdev_err(&pdev->dev, \"Invalid number of lanes: %u\\n\",\n> +\t\t\tcsi2tx->max_lanes);\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tcsi2tx->max_streams = ((reg >> 4) & 7);\n> +\tif (csi2tx->max_streams > CSI2TX_STREAMS_MAX) {\n> +\t\tdev_err(&pdev->dev, \"Invalid number of streams: %u\\n\",\n> +\t\t\tcsi2tx->max_streams);\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tcsi2tx->has_internal_dphy = (reg & BIT(3)) ? true : false;\n> +\n> +\tfor (i = 0; i < csi2tx->max_streams; i++) {\n> +\t\tchar clk_name[16];\n> +\n> +\t\tsnprintf(clk_name, sizeof(clk_name), \"pixel_if%u_clk\", i);\n> +\t\tcsi2tx->pixel_clk[i] = devm_clk_get(&pdev->dev, clk_name);\n> +\t\tif (IS_ERR(csi2tx->pixel_clk[i])) {\n> +\t\t\tdev_err(&pdev->dev, \"Couldn't get clock %s\\n\",\n> +\t\t\t\tclk_name);\n> +\t\t\treturn PTR_ERR(csi2tx->pixel_clk[i]);\n> +\t\t}\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int csi2tx_get_num_lanes(struct device *dev)\n> +{\n> +\tstruct v4l2_fwnode_endpoint v4l2_ep;\n> +\tstruct device_node *ep, *np = dev->of_node;\n> +\tint ret = 0;\n> +\n> +\tep = of_graph_get_endpoint_by_regs(np, 0, 0);\n> +\tif (!ep)\n> +\t\treturn -EINVAL;\n> +\n> +\tret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep);\n> +\tif (ret) {\n> +\t\tdev_err(dev, \"Could not parse v4l2 endpoint\\n\");\n> +\t\tgoto out;\n> +\t}\n> +\n> +\tif (v4l2_ep.bus_type != V4L2_MBUS_CSI2) {\n> +\t\tdev_err(dev, \"Unsupported media bus type: 0x%x\\n\",\n> +\t\t\tv4l2_ep.bus_type);\n> +\t\tret = -EINVAL;\n> +\t\tgoto out;\n> +\t}\n> +\n> +\tret = v4l2_ep.bus.mipi_csi2.num_data_lanes;\n> +\n> +out:\n> +\tof_node_put(ep);\n> +\treturn ret;\n> +}\n> +\n> +static int csi2tx_probe(struct platform_device *pdev)\n> +{\n> +\tstruct csi2tx_priv *csi2tx;\n> +\tint i, ret;\n> +\n> +\tcsi2tx = kzalloc(sizeof(*csi2tx), GFP_KERNEL);\n> +\tif (!csi2tx)\n> +\t\treturn -ENOMEM;\n> +\tplatform_set_drvdata(pdev, csi2tx);\n> +\tcsi2tx->dev = &pdev->dev;\n> +\n> +\tret = csi2tx_get_resources(csi2tx, pdev);\n> +\tif (ret)\n> +\t\tgoto err_free_priv;\n> +\n> +\tv4l2_subdev_init(&csi2tx->subdev, &csi2tx_subdev_ops);\n> +\tcsi2tx->subdev.owner = THIS_MODULE;\n> +\tcsi2tx->subdev.dev = &pdev->dev;\n> +\tcsi2tx->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;\n> +\tv4l2_set_subdevdata(&csi2tx->subdev, &pdev->dev);\n> +\tsnprintf(csi2tx->subdev.name, V4L2_SUBDEV_NAME_SIZE, \"%s.%s\",\n> +\t\t KBUILD_MODNAME, dev_name(&pdev->dev));\n> +\n> +\tcsi2tx->lanes = csi2tx_get_num_lanes(&pdev->dev);\n> +\tif (csi2tx->lanes < 0) {\n> +\t\tdev_err(&pdev->dev, \"Invalid number of lanes, bailing out.\\n\");\n> +\t\tret = csi2tx->lanes;\n> +\t\tgoto err_free_priv;\n> +\t}\n\ncsi2tx->lanes is unsigned so it will never be negative.\n\n> +\n> +\tif (csi2tx->lanes > csi2tx->max_lanes) {\n> +\t\tdev_err(&pdev->dev,\n> +\t\t\t\"Current configuration uses more lanes than supported\\n\");\n> +\t\tret = -EINVAL;\n> +\t\tgoto err_free_priv;\n> +\t}\n> +\n> +\t/* Create our media pads */\n> +\tcsi2tx->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;\n> +\tcsi2tx->pads[CSI2TX_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;\n> +\tfor (i = CSI2TX_PAD_SINK_STREAM0; i < CSI2TX_PAD_MAX; i++)\n> +\t\tcsi2tx->pads[i].flags = MEDIA_PAD_FL_SINK;\n> +\n> +\tret = media_entity_pads_init(&csi2tx->subdev.entity, CSI2TX_PAD_MAX,\n> +\t\t\t\t csi2tx->pads);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = v4l2_async_register_subdev(&csi2tx->subdev);\n> +\tif (ret < 0)\n> +\t\tgoto err_free_priv;\n> +\n> +\tdev_info(&pdev->dev,\n> +\t\t \"Probed CSI2TX with %u/%u lanes, %u streams, %s D-PHY\\n\",\n> +\t\t csi2tx->lanes, csi2tx->max_lanes, csi2tx->max_streams,\n> +\t\t csi2tx->has_internal_dphy ? \"internal\" : \"no\");\n> +\n> +\treturn 0;\n> +\n> +err_free_priv:\n> +\tkfree(csi2tx);\n> +\treturn ret;\n> +}\n> +\n> +static int csi2tx_remove(struct platform_device *pdev)\n> +{\n> +\tstruct csi2tx_priv *csi2tx = platform_get_drvdata(pdev);\n> +\n> +\tv4l2_async_unregister_subdev(&csi2tx->subdev);\n> +\tkfree(csi2tx);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static const struct of_device_id csi2tx_of_table[] = {\n> +\t{ .compatible = \"cdns,csi2tx\" },\n> +\t{ },\n> +};\n> +MODULE_DEVICE_TABLE(of, csi2tx_of_table);\n> +\n> +static struct platform_driver csi2tx_driver = {\n> +\t.probe\t= csi2tx_probe,\n> +\t.remove\t= csi2tx_remove,\n> +\n> +\t.driver\t= {\n> +\t\t.name\t\t= \"cdns-csi2tx\",\n> +\t\t.of_match_table\t= csi2tx_of_table,\n> +\t},\n> +};\n> +module_platform_driver(csi2tx_driver);\n> -- \n> 2.13.5\n> \n\nRegards,\nBenoit\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ti.com header.i=@ti.com header.b=\"RZ9ZHjWe\"; \n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y3g0j5TwSz9t3C\n\tfor ;\n\tSat, 30 Sep 2017 04:23:17 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752303AbdI2SXP (ORCPT\n\t);\n\tFri, 29 Sep 2017 14:23:15 -0400","from fllnx209.ext.ti.com ([198.47.19.16]:24472 \"EHLO\n\tfllnx209.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752172AbdI2SXO (ORCPT\n\t); Fri, 29 Sep 2017 14:23:14 -0400","from dlelxv90.itg.ti.com ([172.17.2.17])\n\tby fllnx209.ext.ti.com (8.15.1/8.15.1) with ESMTP id v8TILVh0018427; \n\tFri, 29 Sep 2017 13:21:31 -0500","from DLEE108.ent.ti.com (dlee108.ent.ti.com [157.170.170.38])\n\tby dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id v8TILQXo007557; \n\tFri, 29 Sep 2017 13:21:26 -0500","from DLEE114.ent.ti.com (157.170.170.25) by DLEE108.ent.ti.com\n\t(157.170.170.38) with Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.1.845.34;\n\tFri, 29 Sep 2017 13:21:25 -0500","from dflp33.itg.ti.com (10.64.6.16) by DLEE114.ent.ti.com\n\t(157.170.170.25) with Microsoft SMTP Server (version=TLS1_0,\n\tcipher=TLS_RSA_WITH_AES_256_CBC_SHA) id 15.1.845.34 via Frontend\n\tTransport; Fri, 29 Sep 2017 13:21:25 -0500","from ti.com (ileax41-snat.itg.ti.com [10.172.224.153])\n\tby dflp33.itg.ti.com (8.14.3/8.13.8) with ESMTP id v8TILPSj016692;\n\tFri, 29 Sep 2017 13:21:25 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1506709291;\n\tbh=6tvgldRo9Dqs96ViQF+0JUxEd+a3viqqFBehB3pr52Q=;\n\th=Date:From:To:CC:Subject:References:In-Reply-To;\n\tb=RZ9ZHjWek0WbQafCe9OfaGfNYXzaEymnawNgdB6vDr0bhfUaCVEp2GFOiWqzzaCgN\n\tXhd69qgTsv7UaJ67yYGog8vJr/0+/KLEtRGANd4joSdSsp7RGANqXjyut+xYJPhCgC\n\tEZOY7UexleXZW0664ineGkWzNRdhYA0dFPBOA1Gg=","Date":"Fri, 29 Sep 2017 13:21:25 -0500","From":"Benoit Parrot ","To":"Maxime Ripard ","CC":"Mauro Carvalho Chehab , Mark Rutland\n\t, Rob Herring , Laurent\n\tPinchart , \n\t, , Cyprian\n\tWronka , Richard Sproul , \n\tAlan Douglas ,\n\tSteve Creaney , Thomas Petazzoni\n\t, Boris Brezillon\n\t, Niklas =?iso-8859-1?q?S=F6derlun?=\n\t=?iso-8859-1?q?d?= ,\n\tHans Verkuil , Sakari Ailus\n\t, ","Subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","Message-ID":"<20170929182125.GB3163@ti.com>","References":"<20170922114703.30511-1-maxime.ripard@free-electrons.com>\n\t<20170922114703.30511-3-maxime.ripard@free-electrons.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Disposition":"inline","In-Reply-To":"<20170922114703.30511-3-maxime.ripard@free-electrons.com>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-EXCLAIMER-MD-CONFIG":"e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1784461,"web_url":"http://patchwork.ozlabs.org/comment/1784461/","msgid":"<20171011094721.hwbsk736ncx5wstt@flea.lan>","list_archive_url":null,"date":"2017-10-11T09:47:21","subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","submitter":{"id":12916,"url":"http://patchwork.ozlabs.org/api/people/12916/","name":"Maxime Ripard","email":"maxime.ripard@free-electrons.com"},"content":"Hi Sakari,\n\nSorry for the belated answer.\n\nOn Tue, Sep 26, 2017 at 08:00:14AM +0000, Sakari Ailus wrote:\n> > On Fri, Sep 22, 2017 at 12:38:49PM +0000, Sakari Ailus wrote:\n> > > > +\t/*\n> > > > +\t * Create a static mapping between the CSI virtual channels\n> > > > +\t * and the input streams.\n> > > \n> > > Which virtual channel is used here?\n> > \n> > Like I was trying to explain in the cover letter, the virtual channel\n> > is not under that block's control. The input video interfaces have an\n> > additional signal that comes from the upstream device which sets the\n> > virtual channel.\n> \n> Oh, I missed while reviewing the set.\n> \n> Presumably either driver would be in control of that then (this one or the\n> upstream sub-device).\n\nI don't really see how this driver could be under such control. I\nguess it would depend on the integreation, but the upstream (sub-)\ndevice is very likely to be under control of that signal, so I guess\nit should be its role to change that. But we should also have that\ninformation available so that the mixing in the CSI link is reported\nproperly in the media API (looking at Niklas' initial implementation).\n\n> > > \n> > > > +\t\t */\n> > > > +\t\twritel(CSI2TX_STREAM_IF_CFG_FILL_LEVEL(4),\n> > > > +\t\t csi2tx->base + CSI2TX_STREAM_IF_CFG_REG(stream));\n> > > > +\t}\n> > > > +\n> > > > +\t/* Disable the configuration mode */\n> > > > +\twritel(0, csi2tx->base + CSI2TX_CONFIG_REG);\n> > > \n> > > Shouldn't you start streaming on the downstream sub-device as well?\n> > \n> > I appreciate it's a pretty weak argument, but the current setup we\n> > have is in the FPGA is:\n> > \n> > capture <- CSI2-RX <- CSI2-TX <- pattern generator\n> > \n> > So far, the CSI2-RX block is calling its remote sub-device, which is\n> > CSI2-TX. If CSI2-RX is calling its remote sub-device (CSI2-RX), we\n> > just found ourselves in an endless loop.\n> > \n> > I guess it should be easier, and fixable, when we'll have an actual\n> > device without such a loopback.\n> \n> What's the intended use case of the device, capture or output?\n\nBy device, you mean the CSI2-TX block, or something else?\n\nIf CSI2-TX, I guess it's more likely to be for output, but that might\nbe used for capture as well.\n\n> How do you currently start the pipeline?\n\nThe capture device is the v4l2 device, and when the capture starts, it\nenables the CSI2-RX which in turn enables CSI2-TX. The pattern\ngenerator is enabled all the time.\n\n> We have a few corner cases in V4L2 for such devices in graph parsing and\n> stream control. The parsing of the device's fwnode graph endpoints are what\n> the device can do, but it doesn't know where the parsing should continue\n> and which part of the graph is already parsed.\n> \n> That will be addressed but right now a driver just needs to know.\n\nI'm not quite sure I got what you wanted me to fix or change.\n\nThanks!\nMaxime","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yBq095q9Wz9sBZ\n\tfor ;\n\tWed, 11 Oct 2017 20:47:37 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752294AbdJKJrg (ORCPT\n\t);\n\tWed, 11 Oct 2017 05:47:36 -0400","from mail.free-electrons.com ([62.4.15.54]:52955 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751462AbdJKJrf (ORCPT\n\t); Wed, 11 Oct 2017 05:47:35 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid 66D9420857; Wed, 11 Oct 2017 11:47:33 +0200 (CEST)","from localhost (unknown [185.94.189.189])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id EDE95207E2;\n\tWed, 11 Oct 2017 11:47:22 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT\n\tshortcircuit=ham autolearn=disabled version=3.4.0","Date":"Wed, 11 Oct 2017 11:47:21 +0200","From":"Maxime Ripard ","To":"Sakari Ailus ","Cc":"Mauro Carvalho Chehab , Mark Rutland\n\t, Rob Herring , Laurent\n\tPinchart , \n\tlinux-media@vger.kernel.org, devicetree@vger.kernel.org, Cyprian Wronka\n\t, Richard Sproul , Alan Douglas\n\t, Steve Creaney , Thomas\n\tPetazzoni , Boris Brezillon\n\t, Niklas =?iso-8859-1?q?S=F6derlun?=\n\t=?iso-8859-1?q?d?= ,\n\tHans Verkuil , Sakari Ailus\n\t, \n\tBenoit Parrot , nm@ti.com","Subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","Message-ID":"<20171011094721.hwbsk736ncx5wstt@flea.lan>","References":"<20170922114703.30511-1-maxime.ripard@free-electrons.com>\n\t<20170922114703.30511-3-maxime.ripard@free-electrons.com>\n\t<20170922123849.hcm76tlplnvd44mt@valkosipuli.retiisi.org.uk>\n\t<20170922153036.u7k3wmuldphkk6w3@flea.lan>\n\t<20170926080014.7a3lbe23rvzpcmkq@valkosipuli.retiisi.org.uk>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"br4mtgjntl4dx72l\"","Content-Disposition":"inline","In-Reply-To":"<20170926080014.7a3lbe23rvzpcmkq@valkosipuli.retiisi.org.uk>","User-Agent":"NeoMutt/20170914 (1.9.0)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1784548,"web_url":"http://patchwork.ozlabs.org/comment/1784548/","msgid":"<20171011115544.w7eswyhke6dskgbb@flea>","list_archive_url":null,"date":"2017-10-11T11:55:44","subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","submitter":{"id":12916,"url":"http://patchwork.ozlabs.org/api/people/12916/","name":"Maxime Ripard","email":"maxime.ripard@free-electrons.com"},"content":"Hi Benoit,\n\nOn Fri, Sep 29, 2017 at 06:21:25PM +0000, Benoit Parrot wrote:\n> > +struct csi2tx_priv {\n> > +\tstruct device\t\t\t*dev;\n> > +\tatomic_t\t\t\tcount;\n> > +\n> > +\tvoid __iomem\t\t\t*base;\n> > +\n> > +\tstruct clk\t\t\t*esc_clk;\n> > +\tstruct clk\t\t\t*p_clk;\n> > +\tstruct clk\t\t\t*pixel_clk[CSI2TX_STREAMS_MAX];\n> > +\n> > +\tstruct v4l2_subdev\t\tsubdev;\n> > +\tstruct media_pad\t\tpads[CSI2TX_PAD_MAX];\n> > +\tstruct v4l2_mbus_framefmt\tpad_fmts[CSI2TX_PAD_MAX];\n> > +\n> > +\tbool\t\t\t\thas_internal_dphy;\n> \n> I assume dphy support is for a subsequent revision?\n\nYes, the situation is similar to the CSI2-RX driver.\n\n> > +\t\t/*\n> > +\t\t * We use the stream ID there, but it's wrong.\n> > +\t\t *\n> > +\t\t * A stream could very well send a data type that is\n> > +\t\t * not equal to its stream ID. We need to find a\n> > +\t\t * proper way to address it.\n> > +\t\t */\n> \n> I don't quite get the above comment, from the code below it looks like\n> you are using the current fmt as a source to provide the correct DT.\n> Am I missing soemthing?\n\nYes, so far the datatype is inferred from the format set. Is there\nanything wrong with that?\n\n> \n> > +\t\twritel(CSI2TX_DT_CFG_DT(fmt->dt),\n> > +\t\t csi2tx->base + CSI2TX_DT_CFG_REG(stream));\n> > +\n> > +\t\twritel(CSI2TX_DT_FORMAT_BYTES_PER_LINE(mfmt->width * fmt->bpp) |\n> > +\t\t CSI2TX_DT_FORMAT_MAX_LINE_NUM(mfmt->height + 1),\n> > +\t\t csi2tx->base + CSI2TX_DT_FORMAT_REG(stream));\n> > +\n> > +\t\t/*\n> > +\t\t * TODO: This needs to be calculated based on the\n> > +\t\t * clock rate.\n> > +\t\t */\n> > +\t\twritel(CSI2TX_STREAM_IF_CFG_FILL_LEVEL(4),\n> > +\t\t csi2tx->base + CSI2TX_STREAM_IF_CFG_REG(stream));\n> > +\t}\n> > +\n> > +\t/* Disable the configuration mode */\n> > +\twritel(0, csi2tx->base + CSI2TX_CONFIG_REG);\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +static int csi2tx_stop(struct csi2tx_priv *csi2tx)\n> > +{\n> > +\t/*\n> > +\t * Let the last user turn off the lights\n> > +\t */\n> > +\tif (!atomic_dec_and_test(&csi2tx->count))\n> > +\t\treturn 0;\n> > +\n> > +\t/* FIXME: Disable the IP here */\n> > +\n> > +\treturn 0;\n> > +}\n> \n> At this point both _start() and _stop() only return 0.\n> Are there any cases where any of these might fail to \"start\" or \"stop\"?\n\nNone that I know of.\n\nThere might be some errors with the video stream itself, but that\ncan be detected after the block has been enabled.\n\n> > +\tcsi2tx->lanes = csi2tx_get_num_lanes(&pdev->dev);\n> > +\tif (csi2tx->lanes < 0) {\n> > +\t\tdev_err(&pdev->dev, \"Invalid number of lanes, bailing out.\\n\");\n> > +\t\tret = csi2tx->lanes;\n> > +\t\tgoto err_free_priv;\n> > +\t}\n> \n> csi2tx->lanes is unsigned so it will never be negative.\n\nAh, right, I'll change that.\n\nThanks!\nMaxime","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yBsrC1ppSz9sNx\n\tfor ;\n\tWed, 11 Oct 2017 22:55:55 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752029AbdJKLzu (ORCPT\n\t);\n\tWed, 11 Oct 2017 07:55:50 -0400","from mail.free-electrons.com ([62.4.15.54]:56949 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751994AbdJKLzs (ORCPT\n\t); Wed, 11 Oct 2017 07:55:48 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid 5B33720857; Wed, 11 Oct 2017 13:55:46 +0200 (CEST)","from localhost (unknown [185.94.189.189])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id DFA5C20850;\n\tWed, 11 Oct 2017 13:55:45 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT,\n\tURIBL_BLOCKED shortcircuit=ham autolearn=disabled version=3.4.0","Date":"Wed, 11 Oct 2017 13:55:44 +0200","From":"Maxime Ripard ","To":"Benoit Parrot ","Cc":"Mauro Carvalho Chehab , Mark Rutland\n\t, Rob Herring , Laurent\n\tPinchart , \n\tlinux-media@vger.kernel.org, devicetree@vger.kernel.org, Cyprian Wronka\n\t, Richard Sproul , Alan Douglas\n\t, Steve Creaney , Thomas\n\tPetazzoni , Boris Brezillon\n\t, Niklas =?iso-8859-1?q?S=F6derlun?=\n\t=?iso-8859-1?q?d?= ,\n\tHans Verkuil , Sakari Ailus\n\t, nm@ti.com","Subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","Message-ID":"<20171011115544.w7eswyhke6dskgbb@flea>","References":"<20170922114703.30511-1-maxime.ripard@free-electrons.com>\n\t<20170922114703.30511-3-maxime.ripard@free-electrons.com>\n\t<20170929182125.GB3163@ti.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"qcs55slvibc6hjsc\"","Content-Disposition":"inline","In-Reply-To":"<20170929182125.GB3163@ti.com>","User-Agent":"NeoMutt/20170914 (1.9.0)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1784646,"web_url":"http://patchwork.ozlabs.org/comment/1784646/","msgid":"<20171011133639.GC25400@ti.com>","list_archive_url":null,"date":"2017-10-11T13:36:39","subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","submitter":{"id":64914,"url":"http://patchwork.ozlabs.org/api/people/64914/","name":"Benoit Parrot","email":"bparrot@ti.com"},"content":"Hi Maxime,\n\nMaxime Ripard wrote on Wed [2017-Oct-11 13:55:44 +0200]:\n> Hi Benoit,\n> \n> On Fri, Sep 29, 2017 at 06:21:25PM +0000, Benoit Parrot wrote:\n> > > +struct csi2tx_priv {\n> > > +\tstruct device\t\t\t*dev;\n> > > +\tatomic_t\t\t\tcount;\n> > > +\n> > > +\tvoid __iomem\t\t\t*base;\n> > > +\n> > > +\tstruct clk\t\t\t*esc_clk;\n> > > +\tstruct clk\t\t\t*p_clk;\n> > > +\tstruct clk\t\t\t*pixel_clk[CSI2TX_STREAMS_MAX];\n> > > +\n> > > +\tstruct v4l2_subdev\t\tsubdev;\n> > > +\tstruct media_pad\t\tpads[CSI2TX_PAD_MAX];\n> > > +\tstruct v4l2_mbus_framefmt\tpad_fmts[CSI2TX_PAD_MAX];\n> > > +\n> > > +\tbool\t\t\t\thas_internal_dphy;\n> > \n> > I assume dphy support is for a subsequent revision?\n> \n> Yes, the situation is similar to the CSI2-RX driver.\n> \n> > > +\t\t/*\n> > > +\t\t * We use the stream ID there, but it's wrong.\n> > > +\t\t *\n> > > +\t\t * A stream could very well send a data type that is\n> > > +\t\t * not equal to its stream ID. We need to find a\n> > > +\t\t * proper way to address it.\n> > > +\t\t */\n> > \n> > I don't quite get the above comment, from the code below it looks like\n> > you are using the current fmt as a source to provide the correct DT.\n> > Am I missing soemthing?\n> \n> Yes, so far the datatype is inferred from the format set. Is there\n> anything wrong with that?\n\nNo, nothing wrong with that behavior it just doesn't not match the comment\nabove, where it is says that the DT is set to the stream ID...\n\nRegards,\nBenoit\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ti.com header.i=@ti.com header.b=\"vxYHNbhh\"; \n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yBw6f4X3Sz9s7B\n\tfor ;\n\tThu, 12 Oct 2017 00:38:34 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752134AbdJKNid (ORCPT\n\t);\n\tWed, 11 Oct 2017 09:38:33 -0400","from fllnx209.ext.ti.com ([198.47.19.16]:28579 \"EHLO\n\tfllnx209.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751462AbdJKNic (ORCPT\n\t); Wed, 11 Oct 2017 09:38:32 -0400","from dlelxv90.itg.ti.com ([172.17.2.17])\n\tby fllnx209.ext.ti.com (8.15.1/8.15.1) with ESMTP id v9BDajDd022267; \n\tWed, 11 Oct 2017 08:36:45 -0500","from DLEE101.ent.ti.com (dlee101.ent.ti.com [157.170.170.31])\n\tby dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id v9BDaegK031650; \n\tWed, 11 Oct 2017 08:36:40 -0500","from DLEE107.ent.ti.com (157.170.170.37) by DLEE101.ent.ti.com\n\t(157.170.170.31) with Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.1.845.34;\n\tWed, 11 Oct 2017 08:36:40 -0500","from dflp32.itg.ti.com (10.64.6.15) by DLEE107.ent.ti.com\n\t(157.170.170.37) with Microsoft SMTP Server (version=TLS1_0,\n\tcipher=TLS_RSA_WITH_AES_256_CBC_SHA) id 15.1.845.34 via Frontend\n\tTransport; Wed, 11 Oct 2017 08:36:39 -0500","from ti.com (ileax41-snat.itg.ti.com [10.172.224.153])\n\tby dflp32.itg.ti.com (8.14.3/8.13.8) with ESMTP id v9BDad62004358;\n\tWed, 11 Oct 2017 08:36:39 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1507729005;\n\tbh=puFcJ6/4fljSwS23aslTyVU8KbgQxRY7eGCS5Qwe5AQ=;\n\th=Date:From:To:CC:Subject:References:In-Reply-To;\n\tb=vxYHNbhhHnFJVezMk1K+P2qTdFp4itWfmsFhmW/pwLwqW/Shm/NczUsLK/jPgIr+N\n\tuxmUXdD5df6qDFOu5hUhjCxkUfvYPFHGTEk0bqoBbXLVHH5OQLu4SNRyr9fCiF8ePE\n\t1HguWp8K9G1BvidRW9WHwrWDCYdRpHS3eUGZ/Su8=","Date":"Wed, 11 Oct 2017 08:36:39 -0500","From":"Benoit Parrot ","To":"Maxime Ripard ","CC":"Mauro Carvalho Chehab , Mark Rutland\n\t, Rob Herring , Laurent\n\tPinchart , \n\t, , Cyprian\n\tWronka , Richard Sproul , \n\tAlan Douglas ,\n\tSteve Creaney , Thomas Petazzoni\n\t, Boris Brezillon\n\t, Niklas =?iso-8859-1?q?S=F6derlun?=\n\t=?iso-8859-1?q?d?= ,\n\tHans Verkuil , Sakari Ailus\n\t, ","Subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","Message-ID":"<20171011133639.GC25400@ti.com>","References":"<20170922114703.30511-1-maxime.ripard@free-electrons.com>\n\t<20170922114703.30511-3-maxime.ripard@free-electrons.com>\n\t<20170929182125.GB3163@ti.com>\n\t<20171011115544.w7eswyhke6dskgbb@flea>","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Disposition":"inline","In-Reply-To":"<20171011115544.w7eswyhke6dskgbb@flea>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-EXCLAIMER-MD-CONFIG":"e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1786208,"web_url":"http://patchwork.ozlabs.org/comment/1786208/","msgid":"<20171013113826.65x26y7g4reofowl@flea.lan>","list_archive_url":null,"date":"2017-10-13T11:38:26","subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","submitter":{"id":12916,"url":"http://patchwork.ozlabs.org/api/people/12916/","name":"Maxime Ripard","email":"maxime.ripard@free-electrons.com"},"content":"On Wed, Oct 11, 2017 at 01:36:39PM +0000, Benoit Parrot wrote:\n> > > > +\t\t/*\n> > > > +\t\t * We use the stream ID there, but it's wrong.\n> > > > +\t\t *\n> > > > +\t\t * A stream could very well send a data type that is\n> > > > +\t\t * not equal to its stream ID. We need to find a\n> > > > +\t\t * proper way to address it.\n> > > > +\t\t */\n> > > \n> > > I don't quite get the above comment, from the code below it looks like\n> > > you are using the current fmt as a source to provide the correct DT.\n> > > Am I missing soemthing?\n> > \n> > Yes, so far the datatype is inferred from the format set. Is there\n> > anything wrong with that?\n> \n> No, nothing wrong with that behavior it just doesn't not match the comment\n> above, where it is says that the DT is set to the stream ID...\n\nAs explained in the cover letter, you actually have two datatypes, the\ninput one that is in the 0-8 range, which is then mapped through that\nregister to a MIPI-CSI2 datatype. The comment refers to the input\ndatatype, not the CSI's.\n\nMaxime","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yD5MB5tGGz9sP1\n\tfor ;\n\tFri, 13 Oct 2017 22:38:30 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1757807AbdJMLi3 (ORCPT\n\t);\n\tFri, 13 Oct 2017 07:38:29 -0400","from mail.free-electrons.com ([62.4.15.54]:49365 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1757746AbdJMLi2 (ORCPT\n\t); Fri, 13 Oct 2017 07:38:28 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid 0D0B520760; Fri, 13 Oct 2017 13:38:26 +0200 (CEST)","from localhost (LStLambert-657-1-97-87.w90-63.abo.wanadoo.fr\n\t[90.63.216.87])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id D6241203B5;\n\tFri, 13 Oct 2017 13:38:25 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT,\n\tURIBL_BLOCKED shortcircuit=ham autolearn=disabled version=3.4.0","Date":"Fri, 13 Oct 2017 13:38:26 +0200","From":"Maxime Ripard ","To":"Benoit Parrot ","Cc":"Mauro Carvalho Chehab , Mark Rutland\n\t, Rob Herring , Laurent\n\tPinchart , \n\tlinux-media@vger.kernel.org, devicetree@vger.kernel.org, Cyprian Wronka\n\t, Richard Sproul , Alan Douglas\n\t, Steve Creaney , Thomas\n\tPetazzoni , Boris Brezillon\n\t, Niklas =?iso-8859-1?q?S=F6derlun?=\n\t=?iso-8859-1?q?d?= ,\n\tHans Verkuil , Sakari Ailus\n\t, nm@ti.com","Subject":"Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver","Message-ID":"<20171013113826.65x26y7g4reofowl@flea.lan>","References":"<20170922114703.30511-1-maxime.ripard@free-electrons.com>\n\t<20170922114703.30511-3-maxime.ripard@free-electrons.com>\n\t<20170929182125.GB3163@ti.com>\n\t<20171011115544.w7eswyhke6dskgbb@flea>\n\t<20171011133639.GC25400@ti.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"4z2ec5jlfufdvmms\"","Content-Disposition":"inline","In-Reply-To":"<20171011133639.GC25400@ti.com>","User-Agent":"NeoMutt/20170914 (1.9.0)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}}]