lkml.org 
[lkml]   [2014]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v1] PCI: tegra: Enable root port specific features
On Tue, Oct 07, 2014 at 08:44:05AM -0700, Stephen Warren wrote:
> On 10/07/2014 03:27 AM, Vidya Sagar wrote:
> > Enables root port to advertise its ASPM-L1 capability
> > resulting in possible link entry to L1 when an ASPM-L1 capable
> > device is connected
> > Enables per-controller & per-TMS clock clamping by default
> > Enabling above features result in more power saving
> >
> > It also avoids PM message truncation by waiting for DLLP to finish
> > before entering into L1 or L2
> >
> > Also, it adds helper functions to access root port registers
>
> > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
>
> > @@ -1870,8 +1920,10 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie)
> >
> > tegra_pcie_port_enable(port);
> >
> > - if (tegra_pcie_port_check_link(port))
> > + if (tegra_pcie_port_check_link(port)) {
> > + tegra_pcie_enable_rp_features(port);
> > continue;
> > + }
> >
> > dev_info(pcie->dev, "link %u down, ignoring\n", port->index);
>
> Wouldn't it be better to have the error case inside the if block; most
> error-handling is that way. For example:
>
> if (!tegra_pcie_port_check_link(port)) {
> dev_info(pcie->dev, "link %u down, ignoring\n", port->index);
> tegra_pcie_port_disable(port);
> tegra_pcie_port_free(port);
> continue;
> }
>
> tegra_pcie_enable_rp_features(port);
> ... any other "good case" code (of which there admittedly is none at
> present)

Yes, that's a little more idiomatic. I think it could be a separate
cleanup patch, though.

Thierry
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2014-10-08 10:21    [W:0.044 / U:0.852 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site