On Friday 31 January 2014, Pratyush Anand wrote:
> On Thu, Jan 30, 2014 at 09:34:19PM +0800, Arnd Bergmann wrote:
> > On Thursday 30 January 2014, Mohit Kumar wrote:
> >
> > Ah, so this is what the ID gets used for. I would indeed encode this in the
> > phy node, unlike the configuration of whether it's used for PCIe or SATA.
>
> ok.. ll use "phy_id = <n>;" in phy node.
Ok. Minor comment: the preferred style would be 'phy-id' rather than 'phy_id'
in DT,
> > > +static int pcie_miphy_init(struct spear13xx_phy_priv *phypriv)
> > > +{
> > > + if (of_machine_is_compatible("st,spear1340"))
> > > + return spear1340_pcie_miphy_init(phypriv);
> > > + else if (of_machine_is_compatible("st,spear1310"))
> > > + return spear1310_pcie_miphy_init(phypriv);
> > > + else
> > > + return -EINVAL;
> > > +}
> >
> > You should never check global properties such as the machine compatible
> > value to make local decisions. You have two options here: Either use
> > two different compatible strings for the phy node, or encode the
> > difference in another property. If the only difference between spear1310
> > and spear1340 is the location of the register within the "misc" syscon
> > space, a good represenation would be to put the register offset next
> > to the syscon phandle in the same property. That way it could transparently
> > work for future SoCs.
>
> Currently, there is only difference in misc syscon space. But, as I
> said few phy register definition might also change in different soc.
>
> Ok.. I ll go with first option, ie different compatible string for
> different socs. It seems most logical also.
Ok.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
