Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework

[Sylwester Nawrocki]

On 04/02/2013 12:38 AM, Stephen Warren wrote:
> On 04/01/2013 04:27 PM, Sylwester Nawrocki wrote:
> > On 03/28/2013 06:43 AM, Kishon Vijay Abraham I wrote:
> > > diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
>
> > > +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 ?
>
> The client node's DT has to specify which of the provider's PHYs it
> references, yes. Presumably the device driver for the client node knows
> absolutely nothing about this.

Ah, right. The device driver for the client node not necessarily need to be
aware about this. I think I got misled by the 'index' argument of
of_phy_get_with_args() which the PHY consumer need to supply. However it is
only an index pointing to a PHY specifier in its 'phys' property, hence it
would be normally 0 if the client needs only one PHY. Hopefully I got it
right this time.

> > 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.
>
> That sounds right.
>
> > Then users of each PHY type would use only indexes 0..1 (to select their
> > corresponding port).
>
> I don't follow that. If the provider exports PHYs 0..3, then the client
> nodes would refer to PHYS 0..3 not 0..1.

Indeed, it seems I just wanted to preserve the CSI/DSI "channel" information
(0, 1), but that is not really needed anywhere.

> > However I fail to see how this could now be represented in the bindings.
>
> Exactly like the example you gave below, I would expect.
>
> > 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.
>
> Details such as phy::type and phy::port sounds like internal driver
> implementation details which would have no effect on the bindings.

Yes, I seem to have mixed the phy-core implementation and the bindings a 
bit.
My intention was to have the PHYs registered with phy::port that would 
reflect
data channel id, as specified in the SoC's datasheet. However I can't 
think of
any use of it at the moment, except for debugging purpose.

> > /* 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>;
> > };
>
> That looks correct to me.
>
> > 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.
>
> I don't think the number of registers should really makes any
> difference; the whole point of the PHY framework is to decouple to
> providers and consumers, so doing anything custom for special cases
> would completely destroy the ability of the PHY framework to do that.

Ok, that's a very good argument. Something I have not been focused on
that much, given the architecture of hardware I used to work with.

I'll git it a try and I'll see if any any further questions jump out.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/