Hi Neil,
On Thu, Jan 3, 2019 at 7:40 AM Sergio Paracuellos
<sergio.paracuellos@xxxxxxxxx> wrote:
>
> On Thu, Jan 3, 2019 at 6:15 AM NeilBrown <neil@xxxxxxxxxx> wrote:
> >
> > On Mon, Dec 24 2018, Sergio Paracuellos wrote:
> >
> > > New driver for pci phy has been added, as well as. pci driver has been
> > > changed to use kernel's generic PHY API. Add related PCI PHY bindings
> > > accordly.
> > >
> > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> > > ---
> > > drivers/staging/mt7621-dts/mt7621.dtsi | 40 ++++++++++++++++++++++++++
> > > 1 file changed, 40 insertions(+)
> > >
> > > diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
> > > index 71f069d59ad8..60ddfb7699b0 100644
> > > --- a/drivers/staging/mt7621-dts/mt7621.dtsi
> > > +++ b/drivers/staging/mt7621-dts/mt7621.dtsi
> > > @@ -424,6 +424,10 @@
> > > reset-names = "pcie0", "pcie1", "pcie2";
> > > clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
> > > clock-names = "pcie0", "pcie1", "pcie2";
> > > + phys = <&pcie0_port PHY_TYPE_PCIE>,
> > > + <&pcie1_port PHY_TYPE_PCIE>,
> > > + <&pcie2_port PHY_TYPE_PCIE>;
> >
> > This doesn't compile - PHY_TYPE_PCIE is unknown.
> > If I add
> > #include <dt-bindings/phy/phy.h>
>
> It seems that my device tree is not being compiled with the rest of
> the kernel tree which is weird, so sorry
> for this. I though it was being compiling correctly.
So I fixed this. I don't know why but in some moment
'CONFIG_DTB_GNUBEE1' disappeared from my config. Now it is ok again.
Thanks and sorry for inconvenience.
>
> >
> > it gets further.
> >
> >
> > > + phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
> > >
> > > pcie@0,0 {
> > > reg = <0x0000 0 0 0 0>;
> > > @@ -449,4 +453,40 @@
> > > bus-range = <0x00 0xff>;
> > > };
> > > };
> > > +
> > > + pcie0_phy: pcie-phy@1a149000 {
> > > + compatible = "mediatek,mt7621-pci-phy";
> > > + reg = <0x1a149000 0x0700>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> >
> > This causes warning
> > drivers/staging/mt7621-dts/gbpc1.dtb: Warning (ranges_format): /pcie-phy@1a149000:ranges: empty "ranges" property but its #size-cells (0) differs from / (1)
> > drivers/staging/mt7621-dts/gbpc1.dtb: Warning (ranges_format): /pcie-phy@1a14a000:ranges: empty "ranges" property but its #size-cells (0) differs from / (1)
Actually I think there is no real need for 'ranges' property here. So
the following patch should cleanly compile and gets the DT for the pci
phy to a very minimum one:
diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi
b/drivers/staging/mt7621-dts/mt7621.dtsi
index a63b84015814..a2754f016b4d 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -1,4 +1,5 @@
#include <dt-bindings/interrupt-controller/mips-gic.h>
+#include <dt-bindings/phy/phy.h>
/ {
#address-cells = <1>;
@@ -459,19 +460,15 @@
reg = <0x1a149000 0x0700>;
#address-cells = <1>;
#size-cells = <0>;
- ranges;
- status = "disabled";
pcie0_port: pcie-phy@0 {
reg = <0>;
#phy-cells = <1>;
- status = "okay";
};
pcie1_port: pcie-phy@1 {
reg = <1>;
#phy-cells = <1>;
- status = "okay";
};
};
@@ -480,13 +477,10 @@
reg = <0x1a14a000 0x0700>;
#address-cells = <1>;
#size-cells = <0>;
- ranges;
- status = "disabled";
pcie2_port: pcie-phy@0 {
reg = <0>;
#phy-cells = <1>;
- status = "okay";
};
};
> >
> > Whether I leave it as <0>, or change it to <1>, booting results in
> >
> > [ 0.600602] mt7621-pci 1e140000.pcie: Parsing DT failed
>
> Uhmmm... The only place this can fail is getting the phy here (from
> PATCH 2) but the code seems to be correct:
>
> + snprintf(name, sizeof(name), "pcie-phy%d", slot);
> + port->phy = devm_phy_get(dev, name);
> + if (IS_ERR(port->phy))
> + return PTR_ERR(port->phy);
> +
>
> So it seems this is getting wrong stuff from DT... I'll check the DT
> warnings to check if could or not be related.
I reviewed the code and I think it should work.
Is the phy driver being loaded properly? Is any trace for the pci-phy
part in the log that put me in the way of what to check for
correctness?
Thanks in advance.
Best regards,
Sergio Paracuellos
>
> >
> > I haven't tried to dig into why it fails.
> >
> > Thanks,
> > NeilBrown
>
> Thanks for testing this.
>
> Best regards,
> Sergio Paracuellos
> >
> >
> > > + ranges;
> > > + status = "disabled";
> > > +
> > > + pcie0_port: pcie-phy@0 {
> > > + reg = <0>;
> > > + #phy-cells = <1>;
> > > + status = "okay";
> > > + };
> > > +
> > > + pcie1_port: pcie-phy@1 {
> > > + reg = <1>;
> > > + #phy-cells = <1>;
> > > + status = "okay";
> > > + };
> > > + };
> > > +
> > > + pcie1_phy: pcie-phy@1a14a000 {
> > > + compatible = "mediatek,mt7621-pci-phy";
> > > + reg = <0x1a14a000 0x0700>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + ranges;
> > > + status = "disabled";
> > > +
> > > + pcie2_port: pcie-phy@0 {
> > > + reg = <0>;
> > > + #phy-cells = <1>;
> > > + status = "okay";
> > > + };
> > > + };
> > > };
> > > --
> > > 2.19.1
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
