> > diff --git
> > a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> > b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
>
> > +- avdd-supply: Power supply for controller (1.05V)
>
> > + "cml": The Tegra clock of that name
>
> Both those changes need to mention that those additions are only required
> for Tegra30, not Tegra20. In other words,
>
> +- avdd-supply: Power supply for controller (1.05V) (not required for
> Tegra20)
>
> + "cml": The Tegra clock of that name (not required for Tegra20)
>
> You probably also want to explicitly mention nvidia,tegra30-pcie as a legal
> compatible value.
>
> > diff --git a/drivers/pci/host/pci-tegra.c
> > b/drivers/pci/host/pci-tegra.c
>
> > +/* used to differentiate tegra chips code */ struct
> > +tegra_pcie_soc_data {
> > + unsigned int num_max_ports;
>
> nit: "num max" seems redundant. max_ports or num_ports would be better.
>
> > struct tegra_pcie_port {
> > @@ -384,7 +408,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus,
> unsigned int devfn,
> > struct tegra_pcie_port *port;
> >
> > list_for_each_entry(port, &pcie->ports, list) {
> > - if (port->index + 1 == slot) {
> > + if (port->index == slot) {
>
> This and the equivalent change in tegra_pcie_write_conf() seem like a bug-
> fix unrelated to the addition of Tegra30 support. Hence, they should be a
> separate patch.
>
>
> > @@ -398,7 +422,6 @@ static int tegra_pcie_read_conf(struct pci_bus *bus,
> unsigned int devfn,
> > *value = 0xffffffff;
> > return PCIBIOS_DEVICE_NOT_FOUND;
> > }
> > -
> > addr += tegra_pcie_conf_offset(devfn, where);
>
> Unnecessary white-space change.
>
> > @@ -1549,10 +1660,9 @@ static int tegra_pcie_remove(struct
> platform_device *pdev)
> > struct tegra_pcie_bus *bus;
> > int err;
> >
> > - list_for_each_entry(bus, &pcie->busses, list) {
> > + list_for_each_entry(bus, &pcie->busses, list)
> > vunmap(bus->area->addr);
> > - kfree(bus);
> > - }
> > + kfree(bus);
>
> This doesn't look right. Can you please explain it further? This is looping
> over every bus in a dynamic list, so surely each entry needs to be freed? Did
> you do this to avoid a crash? If so, the issue is likely that the loop should use
> list_for_each_entry_safe() rather than list_for_each_entry().
Yes you are right, it is not required. I will revert it along with taking care of other comments.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
