From: Heiko Stuebner <heiko@sntech.de> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Ezequiel Garcia <ezequiel@collabora.com>, Helen Koike <helen.koike@collabora.com>, linux-rockchip@lists.infradead.org, mark.rutland@arm.com, devicetree@vger.kernel.org, eddie.cai.linux@gmail.com, mchehab@kernel.org, gregkh@linuxfoundation.org, andrey.konovalov@linaro.org, linux-kernel@vger.kernel.org, tfiga@chromium.org, robh+dt@kernel.org, hans.verkuil@cisco.com, sakari.ailus@linux.intel.com, joacim.zetterling@gmail.com, kernel@collabora.com, linux-media@vger.kernel.org, jacob-chen@iotwrt.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v12 09/11] media: staging: dt-bindings: add Rockchip MIPI RX D-PHY yaml bindings Date: Tue, 07 Jan 2020 23:12:35 +0100 [thread overview] Message-ID: <1854581.byi6MNbzft@phil> (raw) In-Reply-To: <20200107215739.GB7869@pendragon.ideasonboard.com> Am Dienstag, 7. Januar 2020, 22:57:39 CET schrieb Laurent Pinchart: > Hi Heiko, > > On Tue, Jan 07, 2020 at 10:30:28PM +0100, Heiko Stübner wrote: > > Am Dienstag, 7. Januar 2020, 14:20:10 CET schrieb Ezequiel Garcia: > > > On Tue, 2020-01-07 at 10:28 +0100, Heiko Stübner wrote: > > > > Am Dienstag, 7. Januar 2020, 03:37:21 CET schrieb Laurent Pinchart: > > > > > On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote: > > > > > > On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > > > > > > > Hi Helen, > > > > > > > > > > > > > > Thank you for the patch. > > > > > > > > > > > > > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > > > > > > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > > > > > > > > > > > > > This was tested and verified with: > > > > > > > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi- > > > > > > > > dphy.yaml Documentation/devicetree/bindings/phy/ > > > > > > > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > Changes in v12: > > > > > > > > - The commit replaces the following commit in previous series named > > > > > > > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > > > > > > > This new patch adds yaml binding and was verified with > > > > > > > > make dtbs_check and make dt_binding_check > > > > > > > > > > > > > > > > Changes in v11: None > > > > > > > > Changes in v10: > > > > > > > > - unsquash > > > > > > > > > > > > > > > > Changes in v9: > > > > > > > > - fix title division style > > > > > > > > - squash > > > > > > > > - move to staging > > > > > > > > > > > > > > > > Changes in v8: None > > > > > > > > Changes in v7: > > > > > > > > - updated doc with new design and tested example > > > > > > > > > > > > > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > > > > > > > 1 file changed, 75 insertions(+) > > > > > > > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > b/drivers/staging/media/phy- > > > > > > > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > new file mode 100644 > > > > > > > > index 000000000000..af97f1b3e005 > > > > > > > > --- /dev/null > > > > > > > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > @@ -0,0 +1,75 @@ > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > > > > > +%YAML 1.2 > > > > > > > > +--- > > > > > > > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > > + > > > > > > > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > > > > > > > > > > > > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? > > > > > > > > > > > > The driver currently only supports RX0, but I think you are right, > > > > > > it should say RX here. This binding could be extended for RX1. > > > > > > > > > > > > > Looking at the PHY driver, it seems to handle all PHYs with a single > > > > > > > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > > > > > > > > > > > > I am not following this. The driver handles just one PHY. Each PHY > > > > > > should have its own node. > > > > > > > > > > Looking at the registers, it seems that the different PHYs are > > > > > intertwined and we would could have trouble handling the different PHYs > > > > > with different DT nodes and thus struct device instances. > > > > > > > > I have to confess to not following _ALL_ of the threads, so may say > > > > something stupid, but I don't think the PHYs are intertwined so much. > > > > > > > > Where RX0 is controlled from the "General Register Files" alone > > > > [register dumping ground for soc designers], the TX1RX1-phy > > > > actually gets controlled from inside the dsi1 register area it seems. > > > > > > > > So in my previous (still unsucessful) tests, I was rolling with something like > > > > https://github.com/mmind/linux-rockchip/commit/e0d4b03976d2aab85a8c1630be937ea003b5df88 > > > > > > > > With the actual "logic" picked from the vendor kernel, that just double- > > > > maps the dsi1-registers in both dsi and dphy driver, which was strange. > > > > > > Describing each PHY in its own device node (as we currently do) > > > results in: > > > > > > mipi_dphy_tx1rx1: mipi-dphy-tx1rx1@ff968000 { > > > compatible = "rockchip,rk3399-mipi-dphy"; > > > reg = <0x0 0xff968000 0x0 0x8000>; > > > rockchip,grf = <&grf>; > > > }; > > > > 0xff968000 actually really is the dsi1 controller, so we'll already > > have a node for that area. That is the reason I went that way to make > > the rockchip-dsi optionally also behave as phy-provider. > > > > So when it's used in combination with drm and a panel or so it will > > behave as dsi controller, but when requested via the phy-framework > > it will expose the dphy functionality. > > Doesn't RX1/TX1 also expose controls through GRF ? For instance > GRF_SOC_CON9 has a dphy_rx1_clk_inv_sel bit. There are parts in the GRF but the whole phy-write stuff is inside the dsi controller area. See the vendor kernel at https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/phy/rockchip/phy-rockchip-mipi-rx.c#L1427 where you get write_grf_reg() for GRF bits but also the write_txrx_reg() and mipidphy1_wr_reg() calls that access registers inside the dsi1 controller space. Heiko > > > > grf: syscon@ff770000 { > > > mipi_dphy_rx0: mipi-dphy-rx0 { > > > compatible = "rockchip,rk3399-mipi-dphy"; > > > }; > > > }; > > > > > > Which is mildly ugly, as it uses two mechanism to describe > > > the GRF resource. In addition, the driver will then _infer_ > > > which device node is RX0 and which is TX1RX1, from this. > > > > > > Perhaps Laurent's proposal, describing each PHY explicitly, > > > would be cleaner? > > > > so I really think we shouldn't merge these two things together, > > especially to not break the dsi1 controller part. > >
WARNING: multiple messages have this Message-ID (diff)
From: Heiko Stuebner <heiko@sntech.de> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, eddie.cai.linux@gmail.com, kernel@collabora.com, gregkh@linuxfoundation.org, andrey.konovalov@linaro.org, linux-kernel@vger.kernel.org, tfiga@chromium.org, linux-rockchip@lists.infradead.org, Helen Koike <helen.koike@collabora.com>, robh+dt@kernel.org, hans.verkuil@cisco.com, linux-arm-kernel@lists.infradead.org, sakari.ailus@linux.intel.com, joacim.zetterling@gmail.com, mchehab@kernel.org, Ezequiel Garcia <ezequiel@collabora.com>, jacob-chen@iotwrt.com, linux-media@vger.kernel.org Subject: Re: [PATCH v12 09/11] media: staging: dt-bindings: add Rockchip MIPI RX D-PHY yaml bindings Date: Tue, 07 Jan 2020 23:12:35 +0100 [thread overview] Message-ID: <1854581.byi6MNbzft@phil> (raw) In-Reply-To: <20200107215739.GB7869@pendragon.ideasonboard.com> Am Dienstag, 7. Januar 2020, 22:57:39 CET schrieb Laurent Pinchart: > Hi Heiko, > > On Tue, Jan 07, 2020 at 10:30:28PM +0100, Heiko Stübner wrote: > > Am Dienstag, 7. Januar 2020, 14:20:10 CET schrieb Ezequiel Garcia: > > > On Tue, 2020-01-07 at 10:28 +0100, Heiko Stübner wrote: > > > > Am Dienstag, 7. Januar 2020, 03:37:21 CET schrieb Laurent Pinchart: > > > > > On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote: > > > > > > On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote: > > > > > > > Hi Helen, > > > > > > > > > > > > > > Thank you for the patch. > > > > > > > > > > > > > > On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote: > > > > > > > > Add yaml DT bindings for Rockchip MIPI D-PHY RX > > > > > > > > > > > > > > > > This was tested and verified with: > > > > > > > > mv drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi- > > > > > > > > dphy.yaml Documentation/devicetree/bindings/phy/ > > > > > > > > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > > > Signed-off-by: Helen Koike <helen.koike@collabora.com> > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > Changes in v12: > > > > > > > > - The commit replaces the following commit in previous series named > > > > > > > > media: staging: dt-bindings: Document the Rockchip MIPI RX D-PHY bindings > > > > > > > > This new patch adds yaml binding and was verified with > > > > > > > > make dtbs_check and make dt_binding_check > > > > > > > > > > > > > > > > Changes in v11: None > > > > > > > > Changes in v10: > > > > > > > > - unsquash > > > > > > > > > > > > > > > > Changes in v9: > > > > > > > > - fix title division style > > > > > > > > - squash > > > > > > > > - move to staging > > > > > > > > > > > > > > > > Changes in v8: None > > > > > > > > Changes in v7: > > > > > > > > - updated doc with new design and tested example > > > > > > > > > > > > > > > > .../bindings/phy/rockchip-mipi-dphy.yaml | 75 +++++++++++++++++++ > > > > > > > > 1 file changed, 75 insertions(+) > > > > > > > > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > > > > > > > > > diff --git a/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > b/drivers/staging/media/phy- > > > > > > > > rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > new file mode 100644 > > > > > > > > index 000000000000..af97f1b3e005 > > > > > > > > --- /dev/null > > > > > > > > +++ b/drivers/staging/media/phy-rockchip-dphy/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy.yaml > > > > > > > > @@ -0,0 +1,75 @@ > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > > > > > +%YAML 1.2 > > > > > > > > +--- > > > > > > > > +$id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy.yaml# > > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > > + > > > > > > > > +title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings > > > > > > > > > > > > > > Should this be s/RX0/RX/ ? Or do you expect different bindings for RX1 ? > > > > > > > > > > > > The driver currently only supports RX0, but I think you are right, > > > > > > it should say RX here. This binding could be extended for RX1. > > > > > > > > > > > > > Looking at the PHY driver, it seems to handle all PHYs with a single > > > > > > > struct device. Should we thus use #phy-cells = <1> to select the PHY ? > > > > > > > > > > > > I am not following this. The driver handles just one PHY. Each PHY > > > > > > should have its own node. > > > > > > > > > > Looking at the registers, it seems that the different PHYs are > > > > > intertwined and we would could have trouble handling the different PHYs > > > > > with different DT nodes and thus struct device instances. > > > > > > > > I have to confess to not following _ALL_ of the threads, so may say > > > > something stupid, but I don't think the PHYs are intertwined so much. > > > > > > > > Where RX0 is controlled from the "General Register Files" alone > > > > [register dumping ground for soc designers], the TX1RX1-phy > > > > actually gets controlled from inside the dsi1 register area it seems. > > > > > > > > So in my previous (still unsucessful) tests, I was rolling with something like > > > > https://github.com/mmind/linux-rockchip/commit/e0d4b03976d2aab85a8c1630be937ea003b5df88 > > > > > > > > With the actual "logic" picked from the vendor kernel, that just double- > > > > maps the dsi1-registers in both dsi and dphy driver, which was strange. > > > > > > Describing each PHY in its own device node (as we currently do) > > > results in: > > > > > > mipi_dphy_tx1rx1: mipi-dphy-tx1rx1@ff968000 { > > > compatible = "rockchip,rk3399-mipi-dphy"; > > > reg = <0x0 0xff968000 0x0 0x8000>; > > > rockchip,grf = <&grf>; > > > }; > > > > 0xff968000 actually really is the dsi1 controller, so we'll already > > have a node for that area. That is the reason I went that way to make > > the rockchip-dsi optionally also behave as phy-provider. > > > > So when it's used in combination with drm and a panel or so it will > > behave as dsi controller, but when requested via the phy-framework > > it will expose the dphy functionality. > > Doesn't RX1/TX1 also expose controls through GRF ? For instance > GRF_SOC_CON9 has a dphy_rx1_clk_inv_sel bit. There are parts in the GRF but the whole phy-write stuff is inside the dsi controller area. See the vendor kernel at https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/phy/rockchip/phy-rockchip-mipi-rx.c#L1427 where you get write_grf_reg() for GRF bits but also the write_txrx_reg() and mipidphy1_wr_reg() calls that access registers inside the dsi1 controller space. Heiko > > > > grf: syscon@ff770000 { > > > mipi_dphy_rx0: mipi-dphy-rx0 { > > > compatible = "rockchip,rk3399-mipi-dphy"; > > > }; > > > }; > > > > > > Which is mildly ugly, as it uses two mechanism to describe > > > the GRF resource. In addition, the driver will then _infer_ > > > which device node is RX0 and which is TX1RX1, from this. > > > > > > Perhaps Laurent's proposal, describing each PHY explicitly, > > > would be cleaner? > > > > so I really think we shouldn't merge these two things together, > > especially to not break the dsi1 controller part. > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-01-07 22:12 UTC|newest] Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-27 20:01 [PATCH v12 00/11] Rockchip ISP Driver Helen Koike 2019-12-27 20:01 ` Helen Koike 2019-12-27 20:01 ` [PATCH v12 01/11] media: staging: phy-rockchip-dphy: add Rockchip MIPI Synopsys DPHY driver Helen Koike 2019-12-27 20:01 ` Helen Koike 2019-12-30 18:25 ` Ezequiel Garcia 2019-12-30 18:25 ` Ezequiel Garcia 2020-01-07 1:11 ` Laurent Pinchart 2020-01-07 1:11 ` Laurent Pinchart 2020-01-07 15:58 ` Ezequiel Garcia 2020-01-07 15:58 ` Ezequiel Garcia 2020-01-07 16:18 ` Laurent Pinchart 2020-01-07 16:18 ` Laurent Pinchart 2019-12-27 20:01 ` [PATCH v12 02/11] media: staging: rkisp1: add Rockchip ISP1 base driver Helen Koike 2019-12-30 18:13 ` Ezequiel Garcia 2019-12-30 18:13 ` Ezequiel Garcia 2019-12-27 20:01 ` [PATCH v12 03/11] media: staging: rkisp1: add streaming paths Helen Koike 2019-12-27 20:01 ` Helen Koike 2019-12-27 20:01 ` [PATCH v12 04/11] media: staging: rkisp1: add user space ABI definitions Helen Koike 2019-12-27 20:01 ` Helen Koike 2019-12-27 20:01 ` [PATCH v12 05/11] media: staging: rkisp1: add capture device for statistics Helen Koike 2019-12-27 20:01 ` Helen Koike 2019-12-27 20:01 ` [PATCH v12 06/11] media: staging: rkisp1: add output device for parameters Helen Koike 2019-12-27 20:01 ` Helen Koike 2019-12-27 20:01 ` [PATCH v12 07/11] media: staging: rkisp1: add document for rkisp1 meta buffer format Helen Koike 2019-12-27 20:01 ` Helen Koike 2019-12-27 20:01 ` [PATCH v12 08/11] media: staging: dt-bindings: add Rockchip ISP1 yaml bindings Helen Koike 2019-12-27 20:01 ` Helen Koike 2020-01-06 22:27 ` Rob Herring 2020-01-06 22:27 ` Rob Herring 2020-01-06 22:27 ` Rob Herring 2020-01-06 23:59 ` Laurent Pinchart 2020-01-06 23:59 ` Laurent Pinchart 2020-01-07 13:45 ` Ezequiel Garcia 2020-01-07 13:45 ` Ezequiel Garcia 2020-01-07 16:19 ` Laurent Pinchart 2020-01-07 16:19 ` Laurent Pinchart 2020-01-07 14:01 ` Sakari Ailus 2020-01-07 14:01 ` Sakari Ailus 2020-01-08 16:50 ` Helen Koike 2020-01-08 16:50 ` Helen Koike 2020-01-08 18:08 ` Sakari Ailus 2020-01-08 18:08 ` Sakari Ailus 2020-01-08 18:08 ` Sakari Ailus 2019-12-27 20:01 ` [PATCH v12 09/11] media: staging: dt-bindings: add Rockchip MIPI RX D-PHY " Helen Koike 2019-12-27 20:01 ` Helen Koike 2020-01-06 22:29 ` Rob Herring 2020-01-06 22:29 ` Rob Herring 2020-01-06 22:29 ` Rob Herring 2020-01-07 0:10 ` Laurent Pinchart 2020-01-07 0:10 ` Laurent Pinchart 2020-01-07 2:06 ` Ezequiel Garcia 2020-01-07 2:06 ` Ezequiel Garcia 2020-01-07 2:37 ` Laurent Pinchart 2020-01-07 2:37 ` Laurent Pinchart 2020-01-07 9:28 ` Heiko Stübner 2020-01-07 9:28 ` Heiko Stübner 2020-01-07 13:20 ` Ezequiel Garcia 2020-01-07 13:20 ` Ezequiel Garcia 2020-01-07 21:30 ` Heiko Stübner 2020-01-07 21:30 ` Heiko Stübner 2020-01-07 21:57 ` Laurent Pinchart 2020-01-07 21:57 ` Laurent Pinchart 2020-01-07 22:12 ` Heiko Stuebner [this message] 2020-01-07 22:12 ` Heiko Stuebner 2020-01-07 22:03 ` Ezequiel Garcia 2020-01-07 22:03 ` Ezequiel Garcia 2020-01-07 22:25 ` Heiko Stuebner 2020-01-07 22:25 ` Heiko Stuebner 2020-01-07 22:41 ` Ezequiel Garcia 2020-01-07 22:41 ` Ezequiel Garcia 2019-12-27 20:01 ` [PATCH v12 10/11] media: staging: rkisp1: add TODO file for staging Helen Koike 2019-12-27 20:01 ` Helen Koike 2019-12-27 20:01 ` [PATCH v12 11/11] MAINTAINERS: add entry for Rockchip ISP1 driver Helen Koike 2019-12-27 20:01 ` Helen Koike
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1854581.byi6MNbzft@phil \ --to=heiko@sntech.de \ --cc=andrey.konovalov@linaro.org \ --cc=devicetree@vger.kernel.org \ --cc=eddie.cai.linux@gmail.com \ --cc=ezequiel@collabora.com \ --cc=gregkh@linuxfoundation.org \ --cc=hans.verkuil@cisco.com \ --cc=helen.koike@collabora.com \ --cc=jacob-chen@iotwrt.com \ --cc=joacim.zetterling@gmail.com \ --cc=kernel@collabora.com \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=linux-rockchip@lists.infradead.org \ --cc=mark.rutland@arm.com \ --cc=mchehab@kernel.org \ --cc=robh+dt@kernel.org \ --cc=sakari.ailus@linux.intel.com \ --cc=tfiga@chromium.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.