Am Dienstag, 7. Januar 2020, 23:03:54 CET schrieb Ezequiel Garcia:

On Tue, 2020-01-07 at 22:30 +0100, Heiko Stübner wrote:

quoted

Hi Ezequiel,

Am Dienstag, 7. Januar 2020, 14:20:10 CET schrieb Ezequiel Garcia:

quoted

Hi Heiko, Laurent,

On Tue, 2020-01-07 at 10:28 +0100, Heiko Stübner wrote:

quoted

Am Dienstag, 7. Januar 2020, 03:37:21 CET schrieb Laurent Pinchart:

quoted

On Mon, Jan 06, 2020 at 11:06:12PM -0300, Ezequiel Garcia wrote:

quoted

On Tue, 2020-01-07 at 02:10 +0200, Laurent Pinchart wrote:

quoted

Hi Helen,

Thank you for the patch.

On Fri, Dec 27, 2019 at 05:01:14PM -0300, Helen Koike wrote:

quoted

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 <redacted>

---

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.

quoted

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.

Hm, and will this driver also support RX1?

what is RX1 in your book? :-)

According to the TRM the rk3399 has 3 DPHYs,
tx0 - connected exclusively to dsi0
      (this is handled internally by the dw-mipi-dsi driver with controls
       in the dsi0 register space)
rx0 - connected exclusively to isp0
      (this is handled by the individual dphy driver from Helen's series)
tx1rx1 - shared between dsi1 and isp1
      (again inside the dsi1 register space)

quoted

quoted

        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.

I don't think it would necesarily break the dsi1 controller part.

You can declare both device nodes as sharing the address region,
and then the driver can request the I/O resource only when it needs to,
i.e. in the PHY .init hook.

dsi1 is of course a dw-mipi-dsi one, which in turn shares a common bridge
driver over multiple variants (non-rockchip), which expects its registers
mapped during probe.

I think it would not really work well if you need to make the whole world
follow that idea ;-) .


Hence my approach with exposing the phy interface from the dsi driver.
If you look at the dts part, it also just looks like it should be ... as
a regular phy:
	https://github.com/mmind/linux-rockchip/blob/wip/tc358749/arch/arm64/boot/dts/rockchip/rk3399.dtsi#L1764

And on the driver side there is even some short circuit protection.
When used as phy, it won't allow to be used as a component and
vice versa.

It's not super nice, but there's no real reason two devices
can't share an I/O memory resource.

Counter argument, devicetree is not a means to handle Linux
peculiarites - instead it should describe the hardware ...
and the area there _is_ the dsi1 controller ;-)

Heiko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel