Re: Query: Phy: How to find consumer device on dt platform

[Arnd Bergmann]

On Wednesday 29 January 2014, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 29 January 2014 02:56 AM, Arnd Bergmann wrote:
> > On Tuesday 28 January 2014, Kishon Vijay Abraham I wrote:
> >>> I have a common set of registers, which need to be programmed
> >>> differently for PCIe and SATA during phy init/exit.
> >>
> >> One way is differentiate using different compatible strings fro pcie and sata
> >> and use of_device_is_compatible to select a particular path.
> > 
> > But if the IP block is the same, the compatible string should be
> > identical.
> 
> Actually we define the compatible for 'device' no?. Here the same IP is
> configured differently as different devices in SoCs.
> > 
> >>> Therefore, in the init/exit routine of phy_ops, I need some way of
> >>> identifying that phy_init/exit has been called from PCIe driver or
> >>> SATA driver.
> >>
> >> In this case you'll be actually registering two different PHYs (each for pcie
> >> and sata), so your phy_get should give you the only the appropriate phy.
> > 
> > I would instead recommend making the mode of the PHY device the
> > argument to the phy handle in DT, so that the sata node uses
> > 
> > 	phys = <&phyA 0>;
> > 
> > and the PCIe node uses
> > 
> > 	phys = <&phyB 1>;
> > 
> > Then the binding for the phy defines that an argument of '0' means sata mode,
> > while '1' means pcie mode, plus you should define all other valid modes.
> 
> Anyway phyA and phyB points to different nodes and just from phyA and phyB we
> should be able to tell whether it is sata or pcie.
> 
> We can just have a property in phyA to specify it is SATA and phyB to specify
> it is PCIE.
> phyA {
> 	compatible="phy-pipe3";
> 	.
> 	.
> 	type=<SATA>;
> }
> phyB {
> 	compatible="phy-pipe3";
> 	.
> 	.
> 	type=<PCIE>;
> }
> Then in probe
> 	of_property_read_u32(node, "type", &pipe3->type);

But this would be contrary to how we handle all other such devices.

> In phy_init function we can follow different path for SATA and PCIE using the type
> 
> static int pipe3_init(struct phy *x) {
> 	struct pipe3 *phy = phy_get_drvdata(x);
> 
> 	switch (phy->type) {
> 	case SATA:
> 		/* do sata phy initialization here*/
> 		break;
> 	case PCIE:
> 		/* do pcie phy initialization here*/
> 		break;
> 	default:
> 		dev_err(phy->dev, "phy type not supported\n");
> 	}
> 
> 	return 0;
> }

I understand that it can be done, but that doesn't make it a good idea.
Take interrupt controllers as another example: the irqchip node provides
a set of identical resources (interrupt lines) that are connected to
various other parts of the SoC and external sources. You have to somehow
configure each line (edge/level, polarity, ...), but we intentionally
keep the configuration out of the node that describes the irq chip and
instead put it into the nodes that use it.

	Arnd
--
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/