On Wed, Sep 29, 2021 at 02:43:59AM +0200, Saheed O. Bolarinwa wrote:
> From: "Bolarinwa O. Saheed" <refactormyself@xxxxxxxxx>
>
> The clkpm_enabled member of the struct pcie_link_state stores the
> current Clock PM state for the device. However, when the state changes
> it is persisted and can be retrieve by calling pcie_get_clkpm_state()
> introduced in patch [1/3] in this series.
>
> This patch:
> - removes clkpm_enabled from the struct pcie_link_state
> - removes all instance where clkpm_enable is set
> - replaces references to clkpm_enabled with a call to
> pcie_get_clkpm_state()
Similar commit log comments as previous patch.
> Signed-off-by: Bolarinwa O. Saheed <refactormyself@xxxxxxxxx>
> ---
> drivers/pci/pcie/aspm.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 9e65da9a22dd..368828cd427d 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -61,7 +61,6 @@ struct pcie_link_state {
> u32 aspm_disable:7; /* Disabled ASPM state */
>
> /* Clock PM state */
> - u32 clkpm_enabled:1; /* Current Clock PM state */
> u32 clkpm_disable:1; /* Clock PM disabled */
>
> /* Exit latencies */
> @@ -190,7 +189,6 @@ static void pcie_set_clkpm_nocheck(struct pcie_link_state *link, int enable)
> pcie_capability_clear_and_set_word(child, PCI_EXP_LNKCTL,
> PCI_EXP_LNKCTL_CLKREQ_EN,
> val);
> - link->clkpm_enabled = !!enable;
> }
>
> static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
> @@ -203,14 +201,13 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
> if (!capable || link->clkpm_disable)
> enable = 0;
> /* Need nothing if the specified equals to current state */
> - if (link->clkpm_enabled == enable)
> + if (pcie_get_clkpm_state(link->pdev) == enable)
Instead of pcie_get_clkpm_state(), I think I would add this:
static int pcie_clkpm_enabled(struct pci_dev *pdev)
{
struct pci_dev *child;
struct pci_bus *linkbus = pdev->subordinate;
u16 ctl;
/* CLKREQ_EN is only applicable for Upstream Ports */
list_for_each_entry(child, &linkbus->devices, bus_list) {
pcie_capability_read_word(PCI_EXP_LNKCTL, &ctl);
if (!(ctl & PCI_EXP_LNKCTL_CLKREQ_EN))
return 0;
}
return 1;
}
And I would rename pcie_is_clkpm_capable() from the previous patch to
match, i.e., pcie_clkpm_capable(). I suggested "bool" for it, but maybe
it should stay "int" to match this. They both could be "bool", but
that seems a little messy because "enable" comes from
policy_to_clkpm_state(), so it would involve quite a few more changes.
> return;
> pcie_set_clkpm_nocheck(link, enable);
> }
>
> static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
> {
> - link->clkpm_enabled = pcie_get_clkpm_state(link->pdev);
> link->clkpm_disable = blacklist ? 1 : 0;
> }
>
> @@ -1287,7 +1284,7 @@ static ssize_t clkpm_show(struct device *dev,
> struct pci_dev *pdev = to_pci_dev(dev);
> struct pcie_link_state *link = pcie_aspm_get_link(pdev);
>
> - return sysfs_emit(buf, "%d\n", link->clkpm_enabled);
> + return sysfs_emit(buf, "%d\n", pcie_get_clkpm_state(link->pdev));
> }
>
> static ssize_t clkpm_store(struct device *dev,
> --
> 2.20.1
>
