* [PATCH] PCI/ERR: Rename pci_aer_clear_device_status() to pcie_clear_device_status()
@ 2020-07-17 19:56 Bjorn Helgaas
2020-07-17 20:20 ` Kuppuswamy, Sathyanarayanan
0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Helgaas @ 2020-07-17 19:56 UTC (permalink / raw)
To: linux-pci
Cc: Jonathan Cameron, Kuppuswamy Sathyanarayanan, Sean Kelley,
Lorenzo Pieralisi, linuxarm, Bjorn Helgaas
From: Bjorn Helgaas <bhelgaas@google.com>
pci_aer_clear_device_status() clears the error bits in the PCIe Device
Status Register (PCI_EXP_DEVSTA). Every PCIe device has this register,
regardless of whether it supports AER.
Rename pci_aer_clear_device_status() to pcie_clear_device_status() to make
clear that it is PCIe-specific but not AER-specific. No functional change
intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pci.h | 4 ++--
drivers/pci/pcie/aer.c | 4 ++--
drivers/pci/pcie/err.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index c6c0c455f59f..c5f271e6e276 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -657,16 +657,16 @@ void pci_no_aer(void);
void pci_aer_init(struct pci_dev *dev);
void pci_aer_exit(struct pci_dev *dev);
extern const struct attribute_group aer_stats_attr_group;
+void pcie_clear_device_status(struct pci_dev *dev);
void pci_aer_clear_fatal_status(struct pci_dev *dev);
-void pci_aer_clear_device_status(struct pci_dev *dev);
int pci_aer_clear_status(struct pci_dev *dev);
int pci_aer_raw_clear_status(struct pci_dev *dev);
#else
static inline void pci_no_aer(void) { }
static inline void pci_aer_init(struct pci_dev *d) { }
static inline void pci_aer_exit(struct pci_dev *d) { }
+static inline void pcie_clear_device_status(struct pci_dev *dev) { }
static inline void pci_aer_clear_fatal_status(struct pci_dev *dev) { }
-static inline void pci_aer_clear_device_status(struct pci_dev *dev) { }
static inline int pci_aer_clear_status(struct pci_dev *dev) { return -EINVAL; }
static inline int pci_aer_raw_clear_status(struct pci_dev *dev) { return -EINVAL; }
#endif
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index ca886bf91fd9..d3ea667c8520 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -241,7 +241,7 @@ int pci_disable_pcie_error_reporting(struct pci_dev *dev)
}
EXPORT_SYMBOL_GPL(pci_disable_pcie_error_reporting);
-void pci_aer_clear_device_status(struct pci_dev *dev)
+void pcie_clear_device_status(struct pci_dev *dev)
{
u16 sta;
@@ -947,7 +947,7 @@ static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)
if (aer)
pci_write_config_dword(dev, aer + PCI_ERR_COR_STATUS,
info->status);
- pci_aer_clear_device_status(dev);
+ pcie_clear_device_status(dev);
} else if (info->severity == AER_NONFATAL)
pcie_do_recovery(dev, pci_channel_io_normal, aer_root_reset);
else if (info->severity == AER_FATAL)
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index 467686ee2d8b..55755bc493f1 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -197,7 +197,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
pci_dbg(dev, "broadcast resume message\n");
pci_walk_bus(bus, report_resume, &status);
- pci_aer_clear_device_status(dev);
+ pcie_clear_device_status(dev);
pci_aer_clear_nonfatal_status(dev);
pci_info(dev, "device recovery successful\n");
return status;
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] PCI/ERR: Rename pci_aer_clear_device_status() to pcie_clear_device_status()
2020-07-17 19:56 [PATCH] PCI/ERR: Rename pci_aer_clear_device_status() to pcie_clear_device_status() Bjorn Helgaas
@ 2020-07-17 20:20 ` Kuppuswamy, Sathyanarayanan
2020-07-21 21:53 ` Bjorn Helgaas
0 siblings, 1 reply; 4+ messages in thread
From: Kuppuswamy, Sathyanarayanan @ 2020-07-17 20:20 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci
Cc: Jonathan Cameron, Sean Kelley, Lorenzo Pieralisi, linuxarm,
Bjorn Helgaas
Hi Bjorn,
On 7/17/20 12:56 PM, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> pci_aer_clear_device_status() clears the error bits in the PCIe Device
> Status Register (PCI_EXP_DEVSTA). Every PCIe device has this register,
> regardless of whether it supports AER.
Since its not related to AER, can we move it out of AER driver ? May be
to pci.c ?
>
> Rename pci_aer_clear_device_status() to pcie_clear_device_status() to make
> clear that it is PCIe-specific but not AER-specific. No functional change
> intended.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/pci/pci.h | 4 ++--
> drivers/pci/pcie/aer.c | 4 ++--
> drivers/pci/pcie/err.c | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index c6c0c455f59f..c5f271e6e276 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -657,16 +657,16 @@ void pci_no_aer(void);
> void pci_aer_init(struct pci_dev *dev);
> void pci_aer_exit(struct pci_dev *dev);
> extern const struct attribute_group aer_stats_attr_group;
> +void pcie_clear_device_status(struct pci_dev *dev);
> void pci_aer_clear_fatal_status(struct pci_dev *dev);
> -void pci_aer_clear_device_status(struct pci_dev *dev);
> int pci_aer_clear_status(struct pci_dev *dev);
> int pci_aer_raw_clear_status(struct pci_dev *dev);
> #else
> static inline void pci_no_aer(void) { }
> static inline void pci_aer_init(struct pci_dev *d) { }
> static inline void pci_aer_exit(struct pci_dev *d) { }
> +static inline void pcie_clear_device_status(struct pci_dev *dev) { }
> static inline void pci_aer_clear_fatal_status(struct pci_dev *dev) { }
> -static inline void pci_aer_clear_device_status(struct pci_dev *dev) { }
> static inline int pci_aer_clear_status(struct pci_dev *dev) { return -EINVAL; }
> static inline int pci_aer_raw_clear_status(struct pci_dev *dev) { return -EINVAL; }
> #endif
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index ca886bf91fd9..d3ea667c8520 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -241,7 +241,7 @@ int pci_disable_pcie_error_reporting(struct pci_dev *dev)
> }
> EXPORT_SYMBOL_GPL(pci_disable_pcie_error_reporting);
>
> -void pci_aer_clear_device_status(struct pci_dev *dev)
> +void pcie_clear_device_status(struct pci_dev *dev)
> {
> u16 sta;
>
> @@ -947,7 +947,7 @@ static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)
> if (aer)
> pci_write_config_dword(dev, aer + PCI_ERR_COR_STATUS,
> info->status);
> - pci_aer_clear_device_status(dev);
> + pcie_clear_device_status(dev);
> } else if (info->severity == AER_NONFATAL)
> pcie_do_recovery(dev, pci_channel_io_normal, aer_root_reset);
> else if (info->severity == AER_FATAL)
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index 467686ee2d8b..55755bc493f1 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -197,7 +197,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
> pci_dbg(dev, "broadcast resume message\n");
> pci_walk_bus(bus, report_resume, &status);
>
> - pci_aer_clear_device_status(dev);
> + pcie_clear_device_status(dev);
> pci_aer_clear_nonfatal_status(dev);
> pci_info(dev, "device recovery successful\n");
> return status;
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] PCI/ERR: Rename pci_aer_clear_device_status() to pcie_clear_device_status()
2020-07-17 20:20 ` Kuppuswamy, Sathyanarayanan
@ 2020-07-21 21:53 ` Bjorn Helgaas
2020-07-21 22:08 ` Kuppuswamy, Sathyanarayanan
0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Helgaas @ 2020-07-21 21:53 UTC (permalink / raw)
To: Kuppuswamy, Sathyanarayanan
Cc: linux-pci, Jonathan Cameron, Sean Kelley, Lorenzo Pieralisi,
linuxarm, Bjorn Helgaas
On Fri, Jul 17, 2020 at 01:20:22PM -0700, Kuppuswamy, Sathyanarayanan wrote:
> Hi Bjorn,
>
> On 7/17/20 12:56 PM, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> >
> > pci_aer_clear_device_status() clears the error bits in the PCIe Device
> > Status Register (PCI_EXP_DEVSTA). Every PCIe device has this register,
> > regardless of whether it supports AER.
>
> Since its not related to AER, can we move it out of AER driver ? May be
> to pci.c ?
OK. pci.c is really a grab bag of random stuff, but it *is* true that
this doesn't really seem to belong in aer.c.
So I don't mind moving it to pci.c (does just before
pcie_clear_root_pme_status() seem like a reasonable spot?)
But looking at this again makes me wonder whether putting the
pcie_aer_is_native() test inside pcie_clear_device_status() is the
right thing. It seems like that test fits better with the AER code,
i.e., in the *callers* of pcie_clear_device_status().
It would mean repeating the test, since we call it twice, but it seems
like it might match up with the spec better. And I have a slight
aversion to functions that can silently return without doing what it
looks like they're supposed to do.
I can fix this all up if that seems right. Or let me know if you have
alternate ideas.
Bjorn
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI/ERR: Rename pci_aer_clear_device_status() to pcie_clear_device_status()
2020-07-21 21:53 ` Bjorn Helgaas
@ 2020-07-21 22:08 ` Kuppuswamy, Sathyanarayanan
0 siblings, 0 replies; 4+ messages in thread
From: Kuppuswamy, Sathyanarayanan @ 2020-07-21 22:08 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Jonathan Cameron, Sean Kelley, Lorenzo Pieralisi,
linuxarm, Bjorn Helgaas
On 7/21/20 2:53 PM, Bjorn Helgaas wrote:
> On Fri, Jul 17, 2020 at 01:20:22PM -0700, Kuppuswamy, Sathyanarayanan wrote:
>> Hi Bjorn,
>>
>> On 7/17/20 12:56 PM, Bjorn Helgaas wrote:
>>> From: Bjorn Helgaas <bhelgaas@google.com>
>>>
>>> pci_aer_clear_device_status() clears the error bits in the PCIe Device
>>> Status Register (PCI_EXP_DEVSTA). Every PCIe device has this register,
>>> regardless of whether it supports AER.
>>
>> Since its not related to AER, can we move it out of AER driver ? May be
>> to pci.c ?
>
> OK. pci.c is really a grab bag of random stuff, but it *is* true that
> this doesn't really seem to belong in aer.c.
>
> So I don't mind moving it to pci.c (does just before
> pcie_clear_root_pme_status() seem like a reasonable spot?)
>
> But looking at this again makes me wonder whether putting the
> pcie_aer_is_native() test inside pcie_clear_device_status() is the
> right thing. It seems like that test fits better with the AER code,
> i.e., in the *callers* of pcie_clear_device_status().
Yes. pcie_aer_is_native() should be left in AER driver.
>
> It would mean repeating the test, since we call it twice, but it seems
> like it might match up with the spec better. And I have a slight
> aversion to functions that can silently return without doing what it
> looks like they're supposed to do.
>
> I can fix this all up if that seems right. Or let me know if you have
> alternate ideas.
Agree with your approach. Its ok to add separate check for
pcie_aer_is_native().
>
> Bjorn
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-21 22:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17 19:56 [PATCH] PCI/ERR: Rename pci_aer_clear_device_status() to pcie_clear_device_status() Bjorn Helgaas
2020-07-17 20:20 ` Kuppuswamy, Sathyanarayanan
2020-07-21 21:53 ` Bjorn Helgaas
2020-07-21 22:08 ` Kuppuswamy, Sathyanarayanan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).