Re: [PATCH v2 3/3] staging: mt7621-dts: add pci-phy related bindings to board's device tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux