Message-ID: <515A09D8.7040703@gmail.com>
Date:	Tue, 02 Apr 2013 00:27:36 +0200
From:	Sylwester Nawrocki <sylvester.nawrocki@...il.com>
To:	Kishon Vijay Abraham I <kishon@...com>
CC:	balbi@...com, gregkh@...uxfoundation.org, arnd@...db.de,
	akpm@...ux-foundation.org, rob@...dley.net, swarren@...dotorg.org,
	davem@...emloft.net, cesarb@...arb.net, linux-usb@...r.kernel.org,
	linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org,
	tony@...mide.com, grant.likely@...retlab.ca,
	rob.herring@...xeda.com, b-cousson@...com, linux@....linux.org.uk,
	eballetbo@...il.com, javier@...hile0.org, mchehab@...hat.com,
	santosh.shilimkar@...com, broonie@...nsource.wolfsonmicro.com,
	swarren@...dia.com, linux-doc@...r.kernel.org,
	devicetree-discuss@...ts.ozlabs.org,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework

On 03/28/2013 06:43 AM, Kishon Vijay Abraham I wrote:
> diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
>b/Documentation/devicetree/bindings/phy/phy-bindings.txt
> new file mode 100644
> index 0000000..35696b2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
> @@ -0,0 +1,76 @@
> +This document explains only the dt data binding. For general information about
> +PHY subsystem refer Documentation/phy.txt
> +
> +PHY device node
> +===============
> +
> +Optional Properties:
> +#phy-cells:	Number of cells in a PHY specifier;  The meaning of all those
> +		cells is defined by the binding for the phy node. However
> +		in-order to return the correct PHY, the PHY susbsystem
> +		requires the first cell always refers to the port.
> +
> +This property is optional because it is needed only for the case where a
> +single IP implements multiple PHYs.
> +
> +For example:
> +
> +phys: phy {
> +    compatible = "xxx";
> +    reg1 =<...>;
> +    reg2 =<...>;
> +    reg3 =<...>;
> +    .
> +    .
> +    #phy-cells =<1>;
> +    .
> +    .
> +};
> +
> +That node describes an IP block that implements 3 different PHYs. In order to
> +differentiate between these 3 PHYs, an additonal specifier should be given
> +while trying to get a reference to it. (The PHY subsystem assumes the
> +specifier is port id).
> +
> +PHY user node
> +=============
> +
> +Required Properties:
> +phys : the phandle for the PHY device (used by the PHY subsystem)
> +
> +Optional properties:
> +phy-names : the names of the PHY corresponding to the PHYs present in the
> +	    *phys* phandle
> +
> +example1:
> +phys: phy {
> +    compatible = "xxx";
> +    reg =<...>;
> +    .
> +    .
> +    phys =<&usb2_phy>,<&usb3_phy>;
> +    phy-names = "usb2phy", "usb3phy";
> +    .
> +    .
> +};
> +This node represents a controller that uses two PHYs one for usb2 and one for
> +usb3. The controller driver can get the appropriate PHY either by using
> +devm_of_phy_get/of_phy_get by passing the correct index or by using
> +of_phy_get_byname/devm_of_phy_get_byname by passing the names given in
> +*phy-names*.
> +
> +example2:
> +phys: phy {
> +    compatible = "xxx";
> +    reg =<...>;
> +    .
> +    .
> +    phys =<&phys 1>;
> +    .
> +    .
> +};
> +
> +This node represents a controller that uses one of the PHYs which is defined
> +previously. Note that the phy handle has an additional specifier "1" to
> +differentiate between the three PHYs. For this case, the controller driver
> +should use of_phy_get_with_args/devm_of_phy_get_with_args.

This means a PHY user needs to know indexes at the PHY driver ?

I have been thinking of using this for an IP which has 4 video PHYs: 2 MIPI
CSI-2 and 2 MIPI DSI. The IP has just 2 registers, each of which is shared
between one MIPI CSI-2 DPHY and one MIPI DSI DPHY. So I thought about 
creating
a single device node for this IP and using 4 indexes for the PHYs, e.g. 
0...3.
Then users of each PHY type would use only indexes 0..1 (to select their
corresponding port).

However I fail to see how this could now be represented in the bindings.

I assume struct phy::type could be used to differentiate between CSI-2 
and DSI.
And struct phy::port could be used to select specific CSI-2 or DSI channel
(0, 1). Ideally the phy users should not care about index of a PHY at 
the PHY
device tree node. E.g. there are 2 MIPI CSI-2 receivers and each has only
one PHY assigned to it. I'm just wondering how the binding should look like,
so a PHY could be associated with a receiver automatically by the phy-core,
e.g.

/* DPHY IP node */
video-phy {
	  reg = <0x10000000 8>;
};

/* MIPI DSI nodes */
dsi_0 {
      phys = <&video-phy 0>;
};

dsi_1 {
      phys = <&video-phy 1>;
};

/* MIPI CSI-2 nodes */
csi_0 {
      phys = <&video-phy 2>;
};

csi_1 {
      phys = <&video-phy 3>;
};

I'm not sure if it is not an overkill to use this the PHY framework with
a device which has only 2 registers. Perhaps something less heavy could
be designed for it. However, if the PHY framework is commonly used there
should be no issue wrt enabling the whole big infrastructure for a simple
device like this.


Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists