On Mon, Jun 04 2018, Rosen Penev wrote: > This currently fixes the remaining dtb warnings: > > Node /pcie@1e140000/pcie0 has a reg or ranges property, but no unit name > Node /pcie@1e140000/pcie1 has a reg or ranges property, but no unit name > Node /pcie@1e140000/pcie2 has a reg or ranges property, but no unit name > Node /pcie@1e140000/pcie0 node name is not "pci" or "pcie" > Node /pcie@1e140000/pcie0 missing ranges for PCI bridge (or not a bridge) > Node /pcie@1e140000/pcie0 missing bus-range for PCI bridge > Node /pcie@1e140000/pcie1 node name is not "pci" or "pcie" > Node /pcie@1e140000/pcie1 missing ranges for PCI bridge (or not a bridge) > Node /pcie@1e140000/pcie1 missing bus-range for PCI bridge > Node /pcie@1e140000/pcie2 node name is not "pci" or "pcie" > Node /pcie@1e140000/pcie2 missing ranges for PCI bridge (or not a bridge) > Node /pcie@1e140000/pcie2 missing bus-range for PCI bridge > Warning (unit_address_format): Failed prerequisite 'pci_bridge' > Warning (pci_device_reg): Failed prerequisite 'pci_bridge' > Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge' > > In addition, it sets each pcie port to disabled in order to allow future > boards to be added that only have 1 or 2 pcie lanes wired up. gbpc1.dts > was changed to accomidate.
I don't think you are using the word "lanes" correctly.
A lane is a pair of wires for carrying data. a x1 PCIe slot has
1 lane and send all data serially. A x8 PCIe slot has 8 lanes
and sends 8 bits at a time.
The enable/disable here seem to apply to ports (hosts? end-points?).
The mt7621 has a 3port pcie bridge (switch?) and 3 individual pci
end-points which can connect to 3 different devices.
>
> Signed-off-by: Rosen Penev <ros...@gmail.com>
> ---
> drivers/staging/mt7621-dts/gbpc1.dts | 15 +++++++++++++++
> drivers/staging/mt7621-dts/mt7621.dtsi | 24 ++++++++++++------------
> 2 files changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/mt7621-dts/gbpc1.dts
> b/drivers/staging/mt7621-dts/gbpc1.dts
> index 6b13d85d9d34..cd7f90e4ec6d 100644
> --- a/drivers/staging/mt7621-dts/gbpc1.dts
> +++ b/drivers/staging/mt7621-dts/gbpc1.dts
> @@ -113,7 +113,22 @@
> };
>
> &pcie {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie_pins>;
> status = "okay";
> +
> + pcie@0,0 {
> + status = "okay";
> + };
> +
> + pcie@1,0 {
> + status = "okay";
> + };
> +
> + pcie@2,0 {
> + status = "okay";
> + };
> +
> };
>
> ðernet {
> diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi
> b/drivers/staging/mt7621-dts/mt7621.dtsi
> index 5a94fcb96402..16583647797f 100644
> --- a/drivers/staging/mt7621-dts/mt7621.dtsi
> +++ b/drivers/staging/mt7621-dts/mt7621.dtsi
> @@ -452,31 +452,31 @@
> clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
> clock-names = "pcie0", "pcie1", "pcie2";
>
> - pcie0 {
> + pcie@0,0 {
> reg = <0x0000 0 0 0 0>;
> -
> #address-cells = <3>;
> #size-cells = <2>;
> -
> - device_type = "pci";
> + ranges;
> + num-lanes = <1>;
> + status = "disabled";
Some of this looks good, however...
1/ The 'status = "disabled"' doesn't actually work. I removed the
'status = "okay"' for pcie@2,0 in gbpc1, and all 3 PCIe devices were
still active. Maybe we just need to add code somewhere.
2/ Why do you remove 'device_type = "pci";'?? Without a device type,
no-one knows what sort of device it is - though that doesn't seem to
stop it from working
3/ the "num-lanes" might make sense, but no code actually processes
it. "num-lanes" is only examined by:
drivers/pci/dwc/pcie-designware.c: ret = of_property_read_u32(np,
"num-lanes", &lanes);
drivers/pci/host/pcie-mediatek.c: err = of_property_read_u32(node,
"num-lanes", &port->lane);
drivers/pci/host/pcie-rockchip.c: err = of_property_read_u32(node,
"num-lanes", &rockchip->lanes);
so I cannot see why it belongs here.
So:
I like the name change (pcie0 -> pcie@0,0)
I like the status="disabled", except that it doesn't work.
I think the addition of "ranges;" is correct.
I don't understand the rest.
Thanks,
NeilBrown
> };
>
> - pcie1 {
> + pcie@1,0 {
> reg = <0x0800 0 0 0 0>;
> -
> #address-cells = <3>;
> #size-cells = <2>;
> -
> - device_type = "pci";
> + ranges;
> + num-lanes = <1>;
> + status = "disabled";
> };
>
> - pcie2 {
> + pcie@2,0 {
> reg = <0x1000 0 0 0 0>;
> -
> #address-cells = <3>;
> #size-cells = <2>;
> -
> - device_type = "pci";
> + ranges;
> + num-lanes = <1>;
> + status = "disabled";
> };
> };
> };
> --
> 2.17.1
signature.asc
Description: PGP signature
_______________________________________________ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
