[{"id":1777674,"web_url":"http://patchwork.ozlabs.org/comment/1777674/","msgid":"<20170929172709.GA3163@ti.com>","list_archive_url":null,"date":"2017-09-29T17:27:09","subject":"Re: [PATCH v4 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver","submitter":{"id":64914,"url":"http://patchwork.ozlabs.org/api/people/64914/","name":"Benoit Parrot","email":"bparrot@ti.com"},"content":"Maxime Ripard wrote on Fri [2017-Sep-22 12:08:23 +0200]:\n> The Cadence CSI-2 RX Controller is an hardware block meant to be used as a\n> bridge between a CSI-2 bus and pixel grabbers.\n> \n> It supports operating with internal or external D-PHY, with up to 4 lanes,\n> or without any D-PHY. The current code only supports the former case.\n> \n> It also support dynamic mapping of the CSI-2 virtual channels to the\n> associated pixel grabbers, but that isn't allowed at the moment either.\n> \n> Signed-off-by: Maxime Ripard \n> ---\n> drivers/media/platform/Kconfig | 1 +\n> drivers/media/platform/Makefile | 2 +\n> drivers/media/platform/cadence/Kconfig | 12 +\n> drivers/media/platform/cadence/Makefile | 1 +\n> drivers/media/platform/cadence/cdns-csi2rx.c | 486 +++++++++++++++++++++++++++\n> 5 files changed, 502 insertions(+)\n> create mode 100644 drivers/media/platform/cadence/Kconfig\n> create mode 100644 drivers/media/platform/cadence/Makefile\n> create mode 100644 drivers/media/platform/cadence/cdns-csi2rx.c\n> \n\n\n\n> +static int csi2rx_get_resources(struct csi2rx_priv *csi2rx,\n> +\t\t\t\tstruct platform_device *pdev)\n> +{\n> +\tstruct resource *res;\n> +\tunsigned char i;\n> +\tu32 reg;\n> +\n> +\tres = platform_get_resource(pdev, IORESOURCE_MEM, 0);\n> +\tcsi2rx->base = devm_ioremap_resource(&pdev->dev, res);\n> +\tif (IS_ERR(csi2rx->base))\n> +\t\treturn PTR_ERR(csi2rx->base);\n> +\n> +\tcsi2rx->sys_clk = devm_clk_get(&pdev->dev, \"sys_clk\");\n> +\tif (IS_ERR(csi2rx->sys_clk)) {\n> +\t\tdev_err(&pdev->dev, \"Couldn't get sys clock\\n\");\n> +\t\treturn PTR_ERR(csi2rx->sys_clk);\n> +\t}\n> +\n> +\tcsi2rx->p_clk = devm_clk_get(&pdev->dev, \"p_clk\");\n> +\tif (IS_ERR(csi2rx->p_clk)) {\n> +\t\tdev_err(&pdev->dev, \"Couldn't get P clock\\n\");\n> +\t\treturn PTR_ERR(csi2rx->p_clk);\n> +\t}\n> +\n> +\tcsi2rx->dphy = devm_phy_optional_get(&pdev->dev, \"dphy\");\n> +\tif (IS_ERR(csi2rx->dphy)) {\n> +\t\tdev_err(&pdev->dev, \"Couldn't get external D-PHY\\n\");\n> +\t\treturn PTR_ERR(csi2rx->dphy);\n> +\t}\n> +\n> +\t/*\n> +\t * FIXME: Once we'll have external D-PHY support, the check\n> +\t * will need to be removed.\n> +\t */\n> +\tif (csi2rx->dphy) {\n> +\t\tdev_err(&pdev->dev, \"External D-PHY not supported yet\\n\");\n> +\t\treturn -EINVAL;\n> +\t}\n\nI understand that in your current environment you do not have a\nDPHY. But I am wondering in a real setup where you will have either\nan internal or an external DPHY, how are they going to interact with\nthis driver or vice-versa?\n\n> +\n> +\tclk_prepare_enable(csi2rx->p_clk);\n> +\treg = readl(csi2rx->base + CSI2RX_DEVICE_CFG_REG);\n> +\tclk_disable_unprepare(csi2rx->p_clk);\n> +\n> +\tcsi2rx->max_lanes = (reg & 7);\n> +\tif (csi2rx->max_lanes > CSI2RX_LANES_MAX) {\n> +\t\tdev_err(&pdev->dev, \"Invalid number of lanes: %u\\n\",\n> +\t\t\tcsi2rx->max_lanes);\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tcsi2rx->max_streams = ((reg >> 4) & 7);\n> +\tif (csi2rx->max_streams > CSI2RX_STREAMS_MAX) {\n> +\t\tdev_err(&pdev->dev, \"Invalid number of streams: %u\\n\",\n> +\t\t\tcsi2rx->max_streams);\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tcsi2rx->has_internal_dphy = (reg & BIT(3)) ? true : false;\n> +\n> +\t/*\n> +\t * FIXME: Once we'll have internal D-PHY support, the check\n> +\t * will need to be removed.\n> +\t */\n> +\tif (csi2rx->has_internal_dphy) {\n> +\t\tdev_err(&pdev->dev, \"Internal D-PHY not supported yet\\n\");\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tfor (i = 0; i < csi2rx->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\tcsi2rx->pixel_clk[i] = devm_clk_get(&pdev->dev, clk_name);\n> +\t\tif (IS_ERR(csi2rx->pixel_clk[i])) {\n> +\t\t\tdev_err(&pdev->dev, \"Couldn't get clock %s\\n\", clk_name);\n> +\t\t\treturn PTR_ERR(csi2rx->pixel_clk[i]);\n> +\t\t}\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int csi2rx_parse_dt(struct csi2rx_priv *csi2rx)\n> +{\n> +\tstruct v4l2_fwnode_endpoint v4l2_ep;\n> +\tstruct device_node *ep, *remote;\n\n*remote is now unused.\n\nRegards,\nBenoit\n\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=\"cHesEap3\"; \n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y3dp85MHjz9t3f\n\tfor ;\n\tSat, 30 Sep 2017 03:29:04 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752462AbdI2R3C (ORCPT\n\t);\n\tFri, 29 Sep 2017 13:29:02 -0400","from fllnx209.ext.ti.com ([198.47.19.16]:23094 \"EHLO\n\tfllnx209.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752438AbdI2R27 (ORCPT\n\t); Fri, 29 Sep 2017 13:28:59 -0400","from dflxv15.itg.ti.com ([128.247.5.124])\n\tby fllnx209.ext.ti.com (8.15.1/8.15.1) with ESMTP id v8THREQN012675; \n\tFri, 29 Sep 2017 12:27:14 -0500","from DLEE101.ent.ti.com (dlee101.ent.ti.com [157.170.170.31])\n\tby dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id v8THR9Np014023;\n\tFri, 29 Sep 2017 12:27:09 -0500","from DLEE106.ent.ti.com (157.170.170.36) 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\tFri, 29 Sep 2017 12:27:09 -0500","from dflp33.itg.ti.com (10.64.6.16) by DLEE106.ent.ti.com\n\t(157.170.170.36) 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 12:27:09 -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 v8THR908023527;\n\tFri, 29 Sep 2017 12:27:09 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1506706034;\n\tbh=ey3LmSfZ8ZPaDnChWdwDlyrM8GL4qTDYlcVeZ6xwqNw=;\n\th=Date:From:To:CC:Subject:References:In-Reply-To;\n\tb=cHesEap3aT2NnlVudThpugFiqcyS/gQGHXd42H6Na2wfIZFQJinOkGecALqFsyJjq\n\tfz6MTz5tG57r8bRO78NBCXUDVV0f1IKwU1rO5WI7p35clnuduU+3f+1TOZ9iHDAmkt\n\tflpqsUcPcbXmpj8E7tSj5clczthVF26UJv86Sa1U=","Date":"Fri, 29 Sep 2017 12:27:09 -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 v4 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver","Message-ID":"<20170929172709.GA3163@ti.com>","References":"<20170922100823.18184-1-maxime.ripard@free-electrons.com>\n\t<20170922100823.18184-3-maxime.ripard@free-electrons.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Disposition":"inline","In-Reply-To":"<20170922100823.18184-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":1784438,"web_url":"http://patchwork.ozlabs.org/comment/1784438/","msgid":"<20171011092409.ndtr3fdo2oj3zueb@flea.lan>","list_archive_url":null,"date":"2017-10-11T09:24:09","subject":"Re: [PATCH v4 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX 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 05:27:09PM +0000, Benoit Parrot wrote:\n> > +static int csi2rx_get_resources(struct csi2rx_priv *csi2rx,\n> > +\t\t\t\tstruct platform_device *pdev)\n> > +{\n> > +\tstruct resource *res;\n> > +\tunsigned char i;\n> > +\tu32 reg;\n> > +\n> > +\tres = platform_get_resource(pdev, IORESOURCE_MEM, 0);\n> > +\tcsi2rx->base = devm_ioremap_resource(&pdev->dev, res);\n> > +\tif (IS_ERR(csi2rx->base))\n> > +\t\treturn PTR_ERR(csi2rx->base);\n> > +\n> > +\tcsi2rx->sys_clk = devm_clk_get(&pdev->dev, \"sys_clk\");\n> > +\tif (IS_ERR(csi2rx->sys_clk)) {\n> > +\t\tdev_err(&pdev->dev, \"Couldn't get sys clock\\n\");\n> > +\t\treturn PTR_ERR(csi2rx->sys_clk);\n> > +\t}\n> > +\n> > +\tcsi2rx->p_clk = devm_clk_get(&pdev->dev, \"p_clk\");\n> > +\tif (IS_ERR(csi2rx->p_clk)) {\n> > +\t\tdev_err(&pdev->dev, \"Couldn't get P clock\\n\");\n> > +\t\treturn PTR_ERR(csi2rx->p_clk);\n> > +\t}\n> > +\n> > +\tcsi2rx->dphy = devm_phy_optional_get(&pdev->dev, \"dphy\");\n> > +\tif (IS_ERR(csi2rx->dphy)) {\n> > +\t\tdev_err(&pdev->dev, \"Couldn't get external D-PHY\\n\");\n> > +\t\treturn PTR_ERR(csi2rx->dphy);\n> > +\t}\n> > +\n> > +\t/*\n> > +\t * FIXME: Once we'll have external D-PHY support, the check\n> > +\t * will need to be removed.\n> > +\t */\n> > +\tif (csi2rx->dphy) {\n> > +\t\tdev_err(&pdev->dev, \"External D-PHY not supported yet\\n\");\n> > +\t\treturn -EINVAL;\n> > +\t}\n> \n> I understand that in your current environment you do not have a\n> DPHY. But I am wondering in a real setup where you will have either\n> an internal or an external DPHY, how are they going to interact with\n> this driver or vice-versa?\n\nIt's difficult to give an answer with so little details. How would you\nchoose between those two PHYs? Is there a mux, or should we just power\none of the two? If that's the case, is there any use case were we\nmight want to power both? If not, which one should we favor, in which\nsituations?\n\nI guess all those questions actually depend on the way the integration\nhas been done, and we're not quite there yet. I guess we could do\neither a platform specific structure or a glue, depending on the\ncomplexity. The platform specific compatible will allow us to do that\nas we see fit anyway.\n\n> > +\n> > +\tclk_prepare_enable(csi2rx->p_clk);\n> > +\treg = readl(csi2rx->base + CSI2RX_DEVICE_CFG_REG);\n> > +\tclk_disable_unprepare(csi2rx->p_clk);\n> > +\n> > +\tcsi2rx->max_lanes = (reg & 7);\n> > +\tif (csi2rx->max_lanes > CSI2RX_LANES_MAX) {\n> > +\t\tdev_err(&pdev->dev, \"Invalid number of lanes: %u\\n\",\n> > +\t\t\tcsi2rx->max_lanes);\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tcsi2rx->max_streams = ((reg >> 4) & 7);\n> > +\tif (csi2rx->max_streams > CSI2RX_STREAMS_MAX) {\n> > +\t\tdev_err(&pdev->dev, \"Invalid number of streams: %u\\n\",\n> > +\t\t\tcsi2rx->max_streams);\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tcsi2rx->has_internal_dphy = (reg & BIT(3)) ? true : false;\n> > +\n> > +\t/*\n> > +\t * FIXME: Once we'll have internal D-PHY support, the check\n> > +\t * will need to be removed.\n> > +\t */\n> > +\tif (csi2rx->has_internal_dphy) {\n> > +\t\tdev_err(&pdev->dev, \"Internal D-PHY not supported yet\\n\");\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tfor (i = 0; i < csi2rx->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\tcsi2rx->pixel_clk[i] = devm_clk_get(&pdev->dev, clk_name);\n> > +\t\tif (IS_ERR(csi2rx->pixel_clk[i])) {\n> > +\t\t\tdev_err(&pdev->dev, \"Couldn't get clock %s\\n\", clk_name);\n> > +\t\t\treturn PTR_ERR(csi2rx->pixel_clk[i]);\n> > +\t\t}\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +static int csi2rx_parse_dt(struct csi2rx_priv *csi2rx)\n> > +{\n> > +\tstruct v4l2_fwnode_endpoint v4l2_ep;\n> > +\tstruct device_node *ep, *remote;\n> \n> *remote is now unused.\n\nIt's fixed, thanks!\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 3yBpTV3r41z9t2r\n\tfor ;\n\tWed, 11 Oct 2017 20:24:30 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753184AbdJKJY1 (ORCPT\n\t);\n\tWed, 11 Oct 2017 05:24:27 -0400","from mail.free-electrons.com ([62.4.15.54]:52369 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752949AbdJKJYX (ORCPT\n\t); Wed, 11 Oct 2017 05:24:23 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid 34CCD20837; Wed, 11 Oct 2017 11:24:21 +0200 (CEST)","from localhost (unknown [185.94.189.189])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id B9BB8203A2;\n\tWed, 11 Oct 2017 11:24:10 +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 11:24:09 +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 v4 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver","Message-ID":"<20171011092409.ndtr3fdo2oj3zueb@flea.lan>","References":"<20170922100823.18184-1-maxime.ripard@free-electrons.com>\n\t<20170922100823.18184-3-maxime.ripard@free-electrons.com>\n\t<20170929172709.GA3163@ti.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"5zjpskp5xiga25tl\"","Content-Disposition":"inline","In-Reply-To":"<20170929172709.GA3163@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":1784627,"web_url":"http://patchwork.ozlabs.org/comment/1784627/","msgid":"<20171011132258.GB25400@ti.com>","list_archive_url":null,"date":"2017-10-11T13:22:59","subject":"Re: [PATCH v4 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX 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 11:24:09 +0200]:\n> Hi Benoit,\n> \n> On Fri, Sep 29, 2017 at 05:27:09PM +0000, Benoit Parrot wrote:\n> > > +static int csi2rx_get_resources(struct csi2rx_priv *csi2rx,\n> > > +\t\t\t\tstruct platform_device *pdev)\n> > > +{\n> > > +\tstruct resource *res;\n> > > +\tunsigned char i;\n> > > +\tu32 reg;\n> > > +\n> > > +\tres = platform_get_resource(pdev, IORESOURCE_MEM, 0);\n> > > +\tcsi2rx->base = devm_ioremap_resource(&pdev->dev, res);\n> > > +\tif (IS_ERR(csi2rx->base))\n> > > +\t\treturn PTR_ERR(csi2rx->base);\n> > > +\n> > > +\tcsi2rx->sys_clk = devm_clk_get(&pdev->dev, \"sys_clk\");\n> > > +\tif (IS_ERR(csi2rx->sys_clk)) {\n> > > +\t\tdev_err(&pdev->dev, \"Couldn't get sys clock\\n\");\n> > > +\t\treturn PTR_ERR(csi2rx->sys_clk);\n> > > +\t}\n> > > +\n> > > +\tcsi2rx->p_clk = devm_clk_get(&pdev->dev, \"p_clk\");\n> > > +\tif (IS_ERR(csi2rx->p_clk)) {\n> > > +\t\tdev_err(&pdev->dev, \"Couldn't get P clock\\n\");\n> > > +\t\treturn PTR_ERR(csi2rx->p_clk);\n> > > +\t}\n> > > +\n> > > +\tcsi2rx->dphy = devm_phy_optional_get(&pdev->dev, \"dphy\");\n> > > +\tif (IS_ERR(csi2rx->dphy)) {\n> > > +\t\tdev_err(&pdev->dev, \"Couldn't get external D-PHY\\n\");\n> > > +\t\treturn PTR_ERR(csi2rx->dphy);\n> > > +\t}\n> > > +\n> > > +\t/*\n> > > +\t * FIXME: Once we'll have external D-PHY support, the check\n> > > +\t * will need to be removed.\n> > > +\t */\n> > > +\tif (csi2rx->dphy) {\n> > > +\t\tdev_err(&pdev->dev, \"External D-PHY not supported yet\\n\");\n> > > +\t\treturn -EINVAL;\n> > > +\t}\n> > \n> > I understand that in your current environment you do not have a\n> > DPHY. But I am wondering in a real setup where you will have either\n> > an internal or an external DPHY, how are they going to interact with\n> > this driver or vice-versa?\n> \n> It's difficult to give an answer with so little details. How would you\n> choose between those two PHYs? Is there a mux, or should we just power\n> one of the two? If that's the case, is there any use case were we\n> might want to power both? If not, which one should we favor, in which\n> situations?\n\nOops, I guess I should clarify, in this case I did not mean we would\nhave both an internal and an external DPHY. I just meant one or the other.\nBasically just want to see how you would actually handle a DPHY here whether\nit's internal or external?\n\nFor instance, using direct register access from within this driver or make\nuse of an separate phy driver...\n\n> \n> I guess all those questions actually depend on the way the integration\n> has been done, and we're not quite there yet. I guess we could do\n> either a platform specific structure or a glue, depending on the\n> complexity. The platform specific compatible will allow us to do that\n> as we see fit anyway.\n> \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=\"SaKooCRV\"; \n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yBvqN0QrFz9s7g\n\tfor ;\n\tThu, 12 Oct 2017 00:25:20 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752281AbdJKNYu (ORCPT\n\t);\n\tWed, 11 Oct 2017 09:24:50 -0400","from fllnx209.ext.ti.com ([198.47.19.16]:28053 \"EHLO\n\tfllnx209.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752114AbdJKNYs (ORCPT\n\t); Wed, 11 Oct 2017 09:24:48 -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 v9BDN4Jb019559; \n\tWed, 11 Oct 2017 08:23:04 -0500","from DLEE114.ent.ti.com (dlee114.ent.ti.com [157.170.170.25])\n\tby dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id v9BDMxgC007358; \n\tWed, 11 Oct 2017 08:22:59 -0500","from DLEE104.ent.ti.com (157.170.170.34) by DLEE114.ent.ti.com\n\t(157.170.170.25) 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:22:59 -0500","from dlep32.itg.ti.com (157.170.170.100) by DLEE104.ent.ti.com\n\t(157.170.170.34) 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:22:59 -0500","from ti.com (ileax41-snat.itg.ti.com [10.172.224.153])\n\tby dlep32.itg.ti.com (8.14.3/8.13.8) with ESMTP id v9BDMx1K015444;\n\tWed, 11 Oct 2017 08:22:59 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1507728184;\n\tbh=Q3su4V2KqjKwBWGvUsz2ojuEGr9bAjuiCPiuVRnH5Vk=;\n\th=Date:From:To:CC:Subject:References:In-Reply-To;\n\tb=SaKooCRVEGJmu3Uul0XvMzJ36TzkHdETLtVEdY1wpR/ne4LBM9JWv8b2Ijmha6KCv\n\tI+eVdYtmXqJSHrQzRGtSn+LBmiYZv0usDy+3ctx36HnPlKlnQennC1OOw0uwirZhpg\n\tt3VC+CJQL8Ug8+Cx3yh8VWSW1Ds2DGXaQvq/5WUE=","Date":"Wed, 11 Oct 2017 08:22:59 -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 v4 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver","Message-ID":"<20171011132258.GB25400@ti.com>","References":"<20170922100823.18184-1-maxime.ripard@free-electrons.com>\n\t<20170922100823.18184-3-maxime.ripard@free-electrons.com>\n\t<20170929172709.GA3163@ti.com>\n\t<20171011092409.ndtr3fdo2oj3zueb@flea.lan>","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Disposition":"inline","In-Reply-To":"<20171011092409.ndtr3fdo2oj3zueb@flea.lan>","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":1786212,"web_url":"http://patchwork.ozlabs.org/comment/1786212/","msgid":"<20171013114822.vem6bbgy4vw2lx77@flea.lan>","list_archive_url":null,"date":"2017-10-13T11:48:22","subject":"Re: [PATCH v4 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX 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 Wed, Oct 11, 2017 at 01:22:59PM +0000, Benoit Parrot wrote:\n> Maxime Ripard wrote on Wed [2017-Oct-11 11:24:09 +0200]:\n> > On Fri, Sep 29, 2017 at 05:27:09PM +0000, Benoit Parrot wrote:\n> > > > +static int csi2rx_get_resources(struct csi2rx_priv *csi2rx,\n> > > > +\t\t\t\tstruct platform_device *pdev)\n> > > > +{\n> > > > +\tstruct resource *res;\n> > > > +\tunsigned char i;\n> > > > +\tu32 reg;\n> > > > +\n> > > > +\tres = platform_get_resource(pdev, IORESOURCE_MEM, 0);\n> > > > +\tcsi2rx->base = devm_ioremap_resource(&pdev->dev, res);\n> > > > +\tif (IS_ERR(csi2rx->base))\n> > > > +\t\treturn PTR_ERR(csi2rx->base);\n> > > > +\n> > > > +\tcsi2rx->sys_clk = devm_clk_get(&pdev->dev, \"sys_clk\");\n> > > > +\tif (IS_ERR(csi2rx->sys_clk)) {\n> > > > +\t\tdev_err(&pdev->dev, \"Couldn't get sys clock\\n\");\n> > > > +\t\treturn PTR_ERR(csi2rx->sys_clk);\n> > > > +\t}\n> > > > +\n> > > > +\tcsi2rx->p_clk = devm_clk_get(&pdev->dev, \"p_clk\");\n> > > > +\tif (IS_ERR(csi2rx->p_clk)) {\n> > > > +\t\tdev_err(&pdev->dev, \"Couldn't get P clock\\n\");\n> > > > +\t\treturn PTR_ERR(csi2rx->p_clk);\n> > > > +\t}\n> > > > +\n> > > > +\tcsi2rx->dphy = devm_phy_optional_get(&pdev->dev, \"dphy\");\n> > > > +\tif (IS_ERR(csi2rx->dphy)) {\n> > > > +\t\tdev_err(&pdev->dev, \"Couldn't get external D-PHY\\n\");\n> > > > +\t\treturn PTR_ERR(csi2rx->dphy);\n> > > > +\t}\n> > > > +\n> > > > +\t/*\n> > > > +\t * FIXME: Once we'll have external D-PHY support, the check\n> > > > +\t * will need to be removed.\n> > > > +\t */\n> > > > +\tif (csi2rx->dphy) {\n> > > > +\t\tdev_err(&pdev->dev, \"External D-PHY not supported yet\\n\");\n> > > > +\t\treturn -EINVAL;\n> > > > +\t}\n> > > \n> > > I understand that in your current environment you do not have a\n> > > DPHY. But I am wondering in a real setup where you will have either\n> > > an internal or an external DPHY, how are they going to interact with\n> > > this driver or vice-versa?\n> > \n> > It's difficult to give an answer with so little details. How would you\n> > choose between those two PHYs? Is there a mux, or should we just power\n> > one of the two? If that's the case, is there any use case were we\n> > might want to power both? If not, which one should we favor, in which\n> > situations?\n> \n> Oops, I guess I should clarify, in this case I did not mean we would\n> have both an internal and an external DPHY. I just meant one or the other.\n\nOk, my bad :)\n\n> Basically just want to see how you would actually handle a DPHY here\n> whether it's internal or external?\n> \n> For instance, using direct register access from within this driver\n> or make use of an separate phy driver...\n\nSo internal would be easy, the only internal D-PHY that is supposed to\nbe integrated is Cadence's, and its registers would be located in the\nsame memory region, so that driver would handle it.\n\nIn the external case, the ideal solution would be to extend the phy\nframework to deal with more phy types. CSI would be a good candidate,\nbut we also have that issue with the DSI patches Boris has been\nsending, and other phy types like USB.\n\nThe idea would be to extend it (or subclass it) to allow to pass more\nconfiguration data that would allow to implement a D-PHY driver of its\nown. That's the long term goal obviously, and we haven't started\nworking on that. So we might have to settle for in-drivers quirks\nshort term.\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 3yD5Zf3vQQz9sP1\n\tfor ;\n\tFri, 13 Oct 2017 22:48:26 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1757380AbdJMLsY (ORCPT\n\t);\n\tFri, 13 Oct 2017 07:48:24 -0400","from mail.free-electrons.com ([62.4.15.54]:49453 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753342AbdJMLsY (ORCPT\n\t); Fri, 13 Oct 2017 07:48:24 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid B2B88207FC; Fri, 13 Oct 2017 13:48:21 +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 88E76203A1;\n\tFri, 13 Oct 2017 13:48:21 +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":"Fri, 13 Oct 2017 13:48:22 +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 v4 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver","Message-ID":"<20171013114822.vem6bbgy4vw2lx77@flea.lan>","References":"<20170922100823.18184-1-maxime.ripard@free-electrons.com>\n\t<20170922100823.18184-3-maxime.ripard@free-electrons.com>\n\t<20170929172709.GA3163@ti.com>\n\t<20171011092409.ndtr3fdo2oj3zueb@flea.lan>\n\t<20171011132258.GB25400@ti.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"shlcvjfyecllhxq3\"","Content-Disposition":"inline","In-Reply-To":"<20171011132258.GB25400@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"}}]