linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).