* [Linux-kernel-mentees] [PATCH 2/11 RFC] misc: rtsx: Confirm that pcie_capability_read_word() is successful
2020-07-06 9:31 [Linux-kernel-mentees] [PATCH 0/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*() Saheed Olayemi Bolarinwa
@ 2020-07-06 9:31 ` Saheed Olayemi Bolarinwa
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 3/11 RFC] PCI: pciehp: Validate with the return value of pcie_capability_read_*() Saheed Olayemi Bolarinwa
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-06 9:31 UTC (permalink / raw)
To: helgaas, Arnd Bergmann, Greg Kroah-Hartman
Cc: Bolarinwa Olayemi Saheed, linux-kernel-mentees, linux-kernel
From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
On failure pcie_capability_read_dword() sets it's last parameter, val
to 0. In which case (val & PCI_EXP_DEVCTL2_LTR_EN) evaluates to 0.
However, with Patch 10/10, it is possible that val is set to ~0 on
failure. This would introduce a bug because (x & x) == (~0 & x).
This bug can be avoided without changing the function's behaviour if the
return value of pcie_capability_read_dword is checked to confirm success.
Check the return value of pcie_capability_read_dword() to ensure success.
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
drivers/misc/cardreader/rts5227.c | 5 +++--
drivers/misc/cardreader/rts5249.c | 5 +++--
drivers/misc/cardreader/rts5260.c | 5 +++--
drivers/misc/cardreader/rts5261.c | 5 +++--
4 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/misc/cardreader/rts5227.c b/drivers/misc/cardreader/rts5227.c
index 3a9467aaa435..7a20a4898d07 100644
--- a/drivers/misc/cardreader/rts5227.c
+++ b/drivers/misc/cardreader/rts5227.c
@@ -92,6 +92,7 @@ static void rts5227_force_power_down(struct rtsx_pcr *pcr, u8 pm_state)
static int rts5227_extra_init_hw(struct rtsx_pcr *pcr)
{
u16 cap;
+ int ret;
rtsx_pci_init_cmd(pcr);
@@ -105,8 +106,8 @@ static int rts5227_extra_init_hw(struct rtsx_pcr *pcr)
/* LED shine disabled, set initial shine cycle period */
rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, OLT_LED_CTL, 0x0F, 0x02);
/* Configure LTR */
- pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &cap);
- if (cap & PCI_EXP_DEVCTL2_LTR_EN)
+ ret = pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &cap);
+ if (!ret && (cap & PCI_EXP_DEVCTL2_LTR_EN))
rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, LTR_CTL, 0xFF, 0xA3);
/* Configure OBFF */
rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, OBFF_CFG, 0x03, 0x03);
diff --git a/drivers/misc/cardreader/rts5249.c b/drivers/misc/cardreader/rts5249.c
index 6c6c9e95a29f..2b05e8663431 100644
--- a/drivers/misc/cardreader/rts5249.c
+++ b/drivers/misc/cardreader/rts5249.c
@@ -95,6 +95,7 @@ static void rts5249_init_from_cfg(struct rtsx_pcr *pcr)
{
struct rtsx_cr_option *option = &(pcr->option);
u32 lval;
+ int ret;
if (CHK_PCI_PID(pcr, PID_524A))
rtsx_pci_read_config_dword(pcr,
@@ -118,8 +119,8 @@ static void rts5249_init_from_cfg(struct rtsx_pcr *pcr)
if (option->ltr_en) {
u16 val;
- pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &val);
- if (val & PCI_EXP_DEVCTL2_LTR_EN) {
+ ret = pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &val);
+ if (!ret && (val & PCI_EXP_DEVCTL2_LTR_EN)) {
option->ltr_enabled = true;
option->ltr_active = true;
rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
diff --git a/drivers/misc/cardreader/rts5260.c b/drivers/misc/cardreader/rts5260.c
index 7a9dbb778e84..934aeaeebfaf 100644
--- a/drivers/misc/cardreader/rts5260.c
+++ b/drivers/misc/cardreader/rts5260.c
@@ -498,6 +498,7 @@ static void rts5260_init_from_cfg(struct rtsx_pcr *pcr)
{
struct rtsx_cr_option *option = &pcr->option;
u32 lval;
+ int ret;
rtsx_pci_read_config_dword(pcr, PCR_ASPM_SETTING_5260, &lval);
@@ -518,8 +519,8 @@ static void rts5260_init_from_cfg(struct rtsx_pcr *pcr)
if (option->ltr_en) {
u16 val;
- pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &val);
- if (val & PCI_EXP_DEVCTL2_LTR_EN) {
+ ret = pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &val);
+ if (!ret && (val & PCI_EXP_DEVCTL2_LTR_EN)) {
option->ltr_enabled = true;
option->ltr_active = true;
rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
diff --git a/drivers/misc/cardreader/rts5261.c b/drivers/misc/cardreader/rts5261.c
index 195822ec858e..2b6f61696e19 100644
--- a/drivers/misc/cardreader/rts5261.c
+++ b/drivers/misc/cardreader/rts5261.c
@@ -438,9 +438,10 @@ static void rts5261_init_from_cfg(struct rtsx_pcr *pcr)
rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, 0xFF, 0);
if (option->ltr_en) {
u16 val;
+ int ret;
- pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &val);
- if (val & PCI_EXP_DEVCTL2_LTR_EN) {
+ ret = pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &val);
+ if (!ret && (val & PCI_EXP_DEVCTL2_LTR_EN)) {
option->ltr_enabled = true;
option->ltr_active = true;
rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
--
2.18.2
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Linux-kernel-mentees] [PATCH 3/11 RFC] PCI: pciehp: Validate with the return value of pcie_capability_read_*()
2020-07-06 9:31 [Linux-kernel-mentees] [PATCH 0/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*() Saheed Olayemi Bolarinwa
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 2/11 RFC] misc: rtsx: Confirm that pcie_capability_read_word() is successful Saheed Olayemi Bolarinwa
@ 2020-07-06 9:31 ` Saheed Olayemi Bolarinwa
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 4/11 " Saheed Olayemi Bolarinwa
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-06 9:31 UTC (permalink / raw)
To: helgaas
Cc: Bolarinwa Olayemi Saheed, linux-kernel, Lukas Wunner, linux-pci,
linux-kernel-mentees
From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
On any failure pcie_capability_read_word() sets it's last parameter, *val
to 0. If pci_config_read_word() fails the *val is reset to 0. Any function
which check only for a frabricated ~0 which fail in this case.
Include a check on the return value of pcie_capability_read_dword() to
confirm success or failure.
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
drivers/pci/hotplug/pciehp_hpc.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 53433b37e181..5af281d97d4f 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -86,10 +86,11 @@ static int pcie_poll_cmd(struct controller *ctrl, int timeout)
{
struct pci_dev *pdev = ctrl_dev(ctrl);
u16 slot_status;
+ int ret;
do {
- pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
- if (slot_status == (u16) ~0) {
+ ret = pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
+ if (ret || (slot_status == (u16) ~0)) {
ctrl_info(ctrl, "%s: no response from device\n",
__func__);
return 0;
@@ -156,6 +157,7 @@ static void pcie_do_write_cmd(struct controller *ctrl, u16 cmd,
{
struct pci_dev *pdev = ctrl_dev(ctrl);
u16 slot_ctrl_orig, slot_ctrl;
+ int ret;
mutex_lock(&ctrl->ctrl_lock);
@@ -164,8 +166,8 @@ static void pcie_do_write_cmd(struct controller *ctrl, u16 cmd,
*/
pcie_wait_cmd(ctrl);
- pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
- if (slot_ctrl == (u16) ~0) {
+ ret = pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
+ if (ret || (slot_ctrl == (u16) ~0)) {
ctrl_info(ctrl, "%s: no response from device\n", __func__);
goto out;
}
@@ -430,7 +432,7 @@ void pciehp_get_latch_status(struct controller *ctrl, u8 *status)
* removed immediately after the check so the caller may need to take
* this into account.
*
- * It the hotplug controller itself is not available anymore returns
+ * If the hotplug controller itself is not available anymore returns
* %-ENODEV.
*/
int pciehp_card_present(struct controller *ctrl)
@@ -591,8 +593,8 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id)
}
read_status:
- pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &status);
- if (status == (u16) ~0) {
+ ret = pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &status);
+ if (ret || (status == (u16) ~0)) {
ctrl_info(ctrl, "%s: no response from device\n", __func__);
if (parent)
pm_runtime_put(parent);
--
2.18.2
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Linux-kernel-mentees] [PATCH 4/11 RFC] PCI: pciehp: Validate with the return value of pcie_capability_read_*()
2020-07-06 9:31 [Linux-kernel-mentees] [PATCH 0/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*() Saheed Olayemi Bolarinwa
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 2/11 RFC] misc: rtsx: Confirm that pcie_capability_read_word() is successful Saheed Olayemi Bolarinwa
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 3/11 RFC] PCI: pciehp: Validate with the return value of pcie_capability_read_*() Saheed Olayemi Bolarinwa
@ 2020-07-06 9:31 ` Saheed Olayemi Bolarinwa
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 5/11 RFC] PCI: pciehp: Make "Power On" the default Saheed Olayemi Bolarinwa
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-06 9:31 UTC (permalink / raw)
To: helgaas
Cc: Bolarinwa Olayemi Saheed, linux-kernel, linux-pci, linux-kernel-mentees
From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
On failure pcie_capability_read_dword() sets it's last parameter,
val to 0.
However, with Patch 11/11, it is possible that val is set to ~0 on
failure. This would introduce a bug because (x & x) == (~0 & x).
This bug can be avoided if the return value of pcie_capability_read_dword
is checked to confirm success.
Check the return value of pcie_capability_read_dword() to ensure success.
Return a value that indicate the result of pcie_capability_read_dword().
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
drivers/pci/hotplug/pciehp_hpc.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 5af281d97d4f..0b691e37fd04 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -277,10 +277,11 @@ static void pcie_wait_for_presence(struct pci_dev *pdev)
{
int timeout = 1250;
u16 slot_status;
+ int ret;
do {
- pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
- if (slot_status & PCI_EXP_SLTSTA_PDS)
+ ret = pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
+ if (!ret && (slot_status & PCI_EXP_SLTSTA_PDS))
return;
msleep(10);
timeout -= 10;
@@ -354,12 +355,13 @@ int pciehp_get_raw_indicator_status(struct hotplug_slot *hotplug_slot,
struct controller *ctrl = to_ctrl(hotplug_slot);
struct pci_dev *pdev = ctrl_dev(ctrl);
u16 slot_ctrl;
+ int ret;
pci_config_pm_runtime_get(pdev);
- pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
+ ret = pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
pci_config_pm_runtime_put(pdev);
*status = (slot_ctrl & (PCI_EXP_SLTCTL_AIC | PCI_EXP_SLTCTL_PIC)) >> 6;
- return 0;
+ return pcibios_err_to_errno(ret);
}
int pciehp_get_attention_status(struct hotplug_slot *hotplug_slot, u8 *status)
@@ -367,9 +369,10 @@ int pciehp_get_attention_status(struct hotplug_slot *hotplug_slot, u8 *status)
struct controller *ctrl = to_ctrl(hotplug_slot);
struct pci_dev *pdev = ctrl_dev(ctrl);
u16 slot_ctrl;
+ int ret;
pci_config_pm_runtime_get(pdev);
- pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
+ ret = pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
pci_config_pm_runtime_put(pdev);
ctrl_dbg(ctrl, "%s: SLOTCTRL %x, value read %x\n", __func__,
pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_ctrl);
@@ -389,7 +392,7 @@ int pciehp_get_attention_status(struct hotplug_slot *hotplug_slot, u8 *status)
break;
}
- return 0;
+ return pcibios_err_to_errno(ret);
}
void pciehp_get_power_status(struct controller *ctrl, u8 *status)
--
2.18.2
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Linux-kernel-mentees] [PATCH 5/11 RFC] PCI: pciehp: Make "Power On" the default
2020-07-06 9:31 [Linux-kernel-mentees] [PATCH 0/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*() Saheed Olayemi Bolarinwa
` (2 preceding siblings ...)
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 4/11 " Saheed Olayemi Bolarinwa
@ 2020-07-06 9:31 ` Saheed Olayemi Bolarinwa
2020-07-10 0:14 ` Bjorn Helgaas
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 7/11 RFC] PCI: Validate with the return value of pcie_capability_read_*() Saheed Olayemi Bolarinwa
` (3 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-06 9:31 UTC (permalink / raw)
To: helgaas
Cc: Bolarinwa Olayemi Saheed, linux-kernel, linux-pci, linux-kernel-mentees
From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
The default case of the switch statement is redundant since
PCI_EXP_SLTCTL_PCC is only a single bit. pcie_capability_read_word()
currently causes "On" value to be set if it fails. Patch 11/11
changes the behaviour of pcie_capability_read_word() so on falure the
"Off" value will be set.
Make the function set status to "Power On" by default and only set to
Set "Power Off" only if pcie_capability_read_word() is successful and
(slot_ctrl & PCI_EXP_SLTCTL_PCC) == PCI_EXP_SLTCTL_PWR_OFF.
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
drivers/pci/hotplug/pciehp_hpc.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 0b691e37fd04..78f806a9c6f1 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -399,22 +399,16 @@ void pciehp_get_power_status(struct controller *ctrl, u8 *status)
{
struct pci_dev *pdev = ctrl_dev(ctrl);
u16 slot_ctrl;
+ int ret;
- pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
+ *status = 1; /* On */
+ ret = pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
ctrl_dbg(ctrl, "%s: SLOTCTRL %x value read %x\n", __func__,
pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_ctrl);
- switch (slot_ctrl & PCI_EXP_SLTCTL_PCC) {
- case PCI_EXP_SLTCTL_PWR_ON:
- *status = 1; /* On */
- break;
- case PCI_EXP_SLTCTL_PWR_OFF:
+ if (!ret &&
+ ((slot_ctrl & PCI_EXP_SLTCTL_PCC) == PCI_EXP_SLTCTL_PWR_OFF))
*status = 0; /* Off */
- break;
- default:
- *status = 0xFF;
- break;
- }
}
void pciehp_get_latch_status(struct controller *ctrl, u8 *status)
--
2.18.2
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH 5/11 RFC] PCI: pciehp: Make "Power On" the default
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 5/11 RFC] PCI: pciehp: Make "Power On" the default Saheed Olayemi Bolarinwa
@ 2020-07-10 0:14 ` Bjorn Helgaas
2020-07-10 21:42 ` Saheed Bolarinwa
0 siblings, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2020-07-10 0:14 UTC (permalink / raw)
To: Saheed Olayemi Bolarinwa; +Cc: linux-pci, linux-kernel-mentees, linux-kernel
On Mon, Jul 06, 2020 at 11:31:15AM +0200, Saheed Olayemi Bolarinwa wrote:
> From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
>
> The default case of the switch statement is redundant since
> PCI_EXP_SLTCTL_PCC is only a single bit. pcie_capability_read_word()
> currently causes "On" value to be set if it fails. Patch 11/11
> changes the behaviour of pcie_capability_read_word() so on falure the
> "Off" value will be set.
s/falure/failure/
Split this into two patches. The removal of the default case should
be in its own patch to make it trivial to review.
> Make the function set status to "Power On" by default and only set to
> Set "Power Off" only if pcie_capability_read_word() is successful and
> (slot_ctrl & PCI_EXP_SLTCTL_PCC) == PCI_EXP_SLTCTL_PWR_OFF.
>
> Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
> Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> ---
> drivers/pci/hotplug/pciehp_hpc.c | 16 +++++-----------
> 1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 0b691e37fd04..78f806a9c6f1 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -399,22 +399,16 @@ void pciehp_get_power_status(struct controller *ctrl, u8 *status)
> {
> struct pci_dev *pdev = ctrl_dev(ctrl);
> u16 slot_ctrl;
> + int ret;
>
> - pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
> + *status = 1; /* On */
> + ret = pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
> ctrl_dbg(ctrl, "%s: SLOTCTRL %x value read %x\n", __func__,
> pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_ctrl);
>
> - switch (slot_ctrl & PCI_EXP_SLTCTL_PCC) {
> - case PCI_EXP_SLTCTL_PWR_ON:
> - *status = 1; /* On */
> - break;
> - case PCI_EXP_SLTCTL_PWR_OFF:
> + if (!ret &&
> + ((slot_ctrl & PCI_EXP_SLTCTL_PCC) == PCI_EXP_SLTCTL_PWR_OFF))
> *status = 0; /* Off */
> - break;
> - default:
> - *status = 0xFF;
> - break;
> - }
> }
>
> void pciehp_get_latch_status(struct controller *ctrl, u8 *status)
> --
> 2.18.2
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH 5/11 RFC] PCI: pciehp: Make "Power On" the default
2020-07-10 0:14 ` Bjorn Helgaas
@ 2020-07-10 21:42 ` Saheed Bolarinwa
0 siblings, 0 replies; 11+ messages in thread
From: Saheed Bolarinwa @ 2020-07-10 21:42 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel-mentees, linux-kernel
On 7/10/20 2:14 AM, Bjorn Helgaas wrote:
> On Mon, Jul 06, 2020 at 11:31:15AM +0200, Saheed Olayemi Bolarinwa wrote:
>> From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
>>
>> The default case of the switch statement is redundant since
>> PCI_EXP_SLTCTL_PCC is only a single bit. pcie_capability_read_word()
>> currently causes "On" value to be set if it fails. Patch 11/11
>> changes the behaviour of pcie_capability_read_word() so on falure the
>> "Off" value will be set.
> s/falure/failure/
>
> Split this into two patches. The removal of the default case should
> be in its own patch to make it trivial to review.
>
Thank you for the review. It is now split into two in the version 3
which I just sent.
- Saheed
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Linux-kernel-mentees] [PATCH 7/11 RFC] PCI: Validate with the return value of pcie_capability_read_*()
2020-07-06 9:31 [Linux-kernel-mentees] [PATCH 0/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*() Saheed Olayemi Bolarinwa
` (3 preceding siblings ...)
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 5/11 RFC] PCI: pciehp: Make "Power On" the default Saheed Olayemi Bolarinwa
@ 2020-07-06 9:31 ` Saheed Olayemi Bolarinwa
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 8/11 RFC] PCI/PM: Use error return value from pcie_capability_read_*() Saheed Olayemi Bolarinwa
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-06 9:31 UTC (permalink / raw)
To: helgaas
Cc: Bolarinwa Olayemi Saheed, linux-kernel, linux-pci, linux-kernel-mentees
From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
On failure pcie_capability_read_dword() sets it's last parameter,
val to 0.
However, with Patch 11/11, it is possible that val is set to ~0 on
failure. This would introduce a bug because (x & x) == (~0 & x).
This bug can be avoided if the return value of pcie_capability_read_dword
is checked to confirm success.
Check the return value of pcie_capability_read_dword() to ensure success.
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
drivers/pci/probe.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2f66988cea25..3c87a8a1d4b5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1121,10 +1121,11 @@ EXPORT_SYMBOL(pci_add_new_bus);
static void pci_enable_crs(struct pci_dev *pdev)
{
u16 root_cap = 0;
+ int ret;
/* Enable CRS Software Visibility if supported */
- pcie_capability_read_word(pdev, PCI_EXP_RTCAP, &root_cap);
- if (root_cap & PCI_EXP_RTCAP_CRSVIS)
+ ret = pcie_capability_read_word(pdev, PCI_EXP_RTCAP, &root_cap);
+ if (!ret && (root_cap & PCI_EXP_RTCAP_CRSVIS))
pcie_capability_set_word(pdev, PCI_EXP_RTCTL,
PCI_EXP_RTCTL_CRSSVE);
}
@@ -1519,9 +1520,10 @@ void set_pcie_port_type(struct pci_dev *pdev)
void set_pcie_hotplug_bridge(struct pci_dev *pdev)
{
u32 reg32;
+ int ret;
- pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, ®32);
- if (reg32 & PCI_EXP_SLTCAP_HPC)
+ ret = pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, ®32);
+ if (!ret && (reg32 & PCI_EXP_SLTCAP_HPC))
pdev->is_hotplug_bridge = 1;
}
@@ -2057,10 +2059,11 @@ int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
bool pcie_relaxed_ordering_enabled(struct pci_dev *dev)
{
u16 v;
+ int ret;
- pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &v);
+ ret = pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &v);
- return !!(v & PCI_EXP_DEVCTL_RELAX_EN);
+ return (!ret && !!(v & PCI_EXP_DEVCTL_RELAX_EN));
}
EXPORT_SYMBOL(pcie_relaxed_ordering_enabled);
@@ -2096,16 +2099,17 @@ static void pci_configure_ltr(struct pci_dev *dev)
struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
struct pci_dev *bridge;
u32 cap, ctl;
+ int ret;
if (!pci_is_pcie(dev))
return;
- pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
- if (!(cap & PCI_EXP_DEVCAP2_LTR))
+ ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
+ if (ret || !(cap & PCI_EXP_DEVCAP2_LTR))
return;
- pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl);
- if (ctl & PCI_EXP_DEVCTL2_LTR_EN) {
+ ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl);
+ if (!ret && (ctl & PCI_EXP_DEVCTL2_LTR_EN)) {
if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
dev->ltr_path = 1;
return;
@@ -2142,12 +2146,13 @@ static void pci_configure_eetlp_prefix(struct pci_dev *dev)
struct pci_dev *bridge;
int pcie_type;
u32 cap;
+ int ret;
if (!pci_is_pcie(dev))
return;
- pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
- if (!(cap & PCI_EXP_DEVCAP2_EE_PREFIX))
+ ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
+ if (ret || !(cap & PCI_EXP_DEVCAP2_EE_PREFIX))
return;
pcie_type = pci_pcie_type(dev);
--
2.18.2
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Linux-kernel-mentees] [PATCH 8/11 RFC] PCI/PM: Use error return value from pcie_capability_read_*()
2020-07-06 9:31 [Linux-kernel-mentees] [PATCH 0/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*() Saheed Olayemi Bolarinwa
` (4 preceding siblings ...)
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 7/11 RFC] PCI: Validate with the return value of pcie_capability_read_*() Saheed Olayemi Bolarinwa
@ 2020-07-06 9:31 ` Saheed Olayemi Bolarinwa
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 10/11 RFC] PCI/ASPM: " Saheed Olayemi Bolarinwa
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 11/11 RFC] PCI: Remove "*val = 0" " Saheed Olayemi Bolarinwa
7 siblings, 0 replies; 11+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-06 9:31 UTC (permalink / raw)
To: helgaas
Cc: Bolarinwa Olayemi Saheed, linux-kernel, linux-pci, linux-kernel-mentees
From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
On failure pcie_capability_read_dword() sets it's last parameter,
val to 0.
However, with Patch 11/11, it is possible that val is set to ~0 on
failure. This would introduce a bug because (x & x) == (~0 & x).
This bug can be avoided if the return value of pcie_capability_read_dword
is checked to confirm success.
Check the return value of pcie_capability_read_dword() to ensure success.
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
drivers/pci/pci.c | 52 ++++++++++++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 19 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ce096272f52b..9f18ffbf7bd4 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3207,6 +3207,7 @@ void pci_configure_ari(struct pci_dev *dev)
{
u32 cap;
struct pci_dev *bridge;
+ int ret;
if (pcie_ari_disabled || !pci_is_pcie(dev) || dev->devfn)
return;
@@ -3215,8 +3216,8 @@ void pci_configure_ari(struct pci_dev *dev)
if (!bridge)
return;
- pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
- if (!(cap & PCI_EXP_DEVCAP2_ARI))
+ ret = pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
+ if (ret || !(cap & PCI_EXP_DEVCAP2_ARI))
return;
if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI)) {
@@ -3606,6 +3607,7 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
struct pci_bus *bus = dev->bus;
struct pci_dev *bridge;
u32 cap, ctl2;
+ int ret;
if (!pci_is_pcie(dev))
return -EINVAL;
@@ -3629,28 +3631,29 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
while (bus->parent) {
bridge = bus->self;
- pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
+ ret = pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2,
+ &cap);
switch (pci_pcie_type(bridge)) {
/* Ensure switch ports support AtomicOp routing */
case PCI_EXP_TYPE_UPSTREAM:
case PCI_EXP_TYPE_DOWNSTREAM:
- if (!(cap & PCI_EXP_DEVCAP2_ATOMIC_ROUTE))
+ if (ret || !(cap & PCI_EXP_DEVCAP2_ATOMIC_ROUTE))
return -EINVAL;
break;
/* Ensure root port supports all the sizes we care about */
case PCI_EXP_TYPE_ROOT_PORT:
- if ((cap & cap_mask) != cap_mask)
+ if (ret || ((cap & cap_mask) != cap_mask))
return -EINVAL;
break;
}
/* Ensure upstream ports don't block AtomicOps on egress */
if (pci_pcie_type(bridge) == PCI_EXP_TYPE_UPSTREAM) {
- pcie_capability_read_dword(bridge, PCI_EXP_DEVCTL2,
- &ctl2);
- if (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_EGRESS_BLOCK)
+ ret = pcie_capability_read_dword(bridge,
+ PCI_EXP_DEVCTL2, &ctl2);
+ if (!ret && (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_EGRESS_BLOCK)
return -EINVAL;
}
@@ -4507,12 +4510,13 @@ EXPORT_SYMBOL(pci_wait_for_pending_transaction);
bool pcie_has_flr(struct pci_dev *dev)
{
u32 cap;
+ int ret;
if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
return false;
- pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
- return cap & PCI_EXP_DEVCAP_FLR;
+ ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
+ return (!ret && !!(cap & PCI_EXP_DEVCAP_FLR));
}
EXPORT_SYMBOL_GPL(pcie_has_flr);
@@ -4672,7 +4676,7 @@ static bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active,
msleep(20);
for (;;) {
pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
- ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
+ ret = !!(!ret && (lnk_status & PCI_EXP_LNKSTA_DLLLA));
if (ret == active)
break;
if (timeout <= 0)
@@ -5774,6 +5778,7 @@ u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
enum pci_bus_speed next_speed;
enum pcie_link_width next_width;
u32 bw, next_bw;
+ int ret;
if (speed)
*speed = PCI_SPEED_UNKNOWN;
@@ -5783,7 +5788,12 @@ u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
bw = 0;
while (dev) {
- pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnksta);
+ ret = pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnksta);
+
+ if (ret) {
+ dev = pci_upstream_bridge(dev);
+ continue;
+ }
next_speed = pcie_link_speed[lnksta & PCI_EXP_LNKSTA_CLS];
next_width = (lnksta & PCI_EXP_LNKSTA_NLW) >>
@@ -5820,6 +5830,7 @@ EXPORT_SYMBOL(pcie_bandwidth_available);
enum pci_bus_speed pcie_get_speed_cap(struct pci_dev *dev)
{
u32 lnkcap2, lnkcap;
+ int ret;
/*
* Link Capabilities 2 was added in PCIe r3.0, sec 7.8.18. The
@@ -5830,16 +5841,18 @@ enum pci_bus_speed pcie_get_speed_cap(struct pci_dev *dev)
* should use the Supported Link Speeds field in Link Capabilities,
* where only 2.5 GT/s and 5.0 GT/s speeds were defined.
*/
- pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, &lnkcap2);
+ ret = pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, &lnkcap2);
/* PCIe r3.0-compliant */
- if (lnkcap2)
+ if (!ret && lnkcap2)
return PCIE_LNKCAP2_SLS2SPEED(lnkcap2);
- pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
- if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_5_0GB)
+ ret = pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
+ if (!ret &&
+ ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_5_0GB))
return PCIE_SPEED_5_0GT;
- else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_2_5GB)
+ else if (!ret &&
+ ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_2_5GB))
return PCIE_SPEED_2_5GT;
return PCI_SPEED_UNKNOWN;
@@ -5856,9 +5869,10 @@ EXPORT_SYMBOL(pcie_get_speed_cap);
enum pcie_link_width pcie_get_width_cap(struct pci_dev *dev)
{
u32 lnkcap;
+ int ret;
- pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
- if (lnkcap)
+ ret = pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
+ if (!ret && lnkcap)
return (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
return PCIE_LNK_WIDTH_UNKNOWN;
--
2.18.2
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Linux-kernel-mentees] [PATCH 10/11 RFC] PCI/ASPM: Use error return value from pcie_capability_read_*()
2020-07-06 9:31 [Linux-kernel-mentees] [PATCH 0/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*() Saheed Olayemi Bolarinwa
` (5 preceding siblings ...)
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 8/11 RFC] PCI/PM: Use error return value from pcie_capability_read_*() Saheed Olayemi Bolarinwa
@ 2020-07-06 9:31 ` Saheed Olayemi Bolarinwa
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 11/11 RFC] PCI: Remove "*val = 0" " Saheed Olayemi Bolarinwa
7 siblings, 0 replies; 11+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-06 9:31 UTC (permalink / raw)
To: helgaas
Cc: Bolarinwa Olayemi Saheed, linux-kernel, linux-pci, linux-kernel-mentees
From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
On failure pcie_capability_read_dword() sets it's last parameter,
val to 0.
However, with Patch 11/11, it is possible that val is set to ~0 on
failure. This would introduce a bug because (x & x) == (~0 & x).
This bug can be avoided if the return value of pcie_capability_read_dword
is checked to confirm success.
Check the return value of pcie_capability_read_dword() to ensure success.
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
drivers/pci/pcie/aspm.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index b17e5ffd31b1..32aa9d57672a 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -176,7 +176,7 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
{
- int capable = 1, enabled = 1;
+ int ret, capable = 1, enabled = 1;
u32 reg32;
u16 reg16;
struct pci_dev *child;
@@ -184,14 +184,14 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
/* All functions should have the same cap and state, take the worst */
list_for_each_entry(child, &linkbus->devices, bus_list) {
- pcie_capability_read_dword(child, PCI_EXP_LNKCAP, ®32);
- if (!(reg32 & PCI_EXP_LNKCAP_CLKPM)) {
+ ret = pcie_capability_read_dword(child, PCI_EXP_LNKCAP, ®32);
+ if (ret || !(reg32 & PCI_EXP_LNKCAP_CLKPM)) {
capable = 0;
enabled = 0;
break;
}
- pcie_capability_read_word(child, PCI_EXP_LNKCTL, ®16);
- if (!(reg16 & PCI_EXP_LNKCTL_CLKREQ_EN))
+ ret = pcie_capability_read_word(child, PCI_EXP_LNKCTL, ®16);
+ if (ret || !(reg16 & PCI_EXP_LNKCTL_CLKREQ_EN))
enabled = 0;
}
link->clkpm_enabled = enabled;
@@ -205,6 +205,7 @@ static bool pcie_retrain_link(struct pcie_link_state *link)
struct pci_dev *parent = link->pdev;
unsigned long end_jiffies;
u16 reg16;
+ int ret;
pcie_capability_read_word(parent, PCI_EXP_LNKCTL, ®16);
reg16 |= PCI_EXP_LNKCTL_RL;
@@ -222,8 +223,8 @@ static bool pcie_retrain_link(struct pcie_link_state *link)
/* Wait for link training end. Break out after waiting for timeout */
end_jiffies = jiffies + LINK_RETRAIN_TIMEOUT;
do {
- pcie_capability_read_word(parent, PCI_EXP_LNKSTA, ®16);
- if (!(reg16 & PCI_EXP_LNKSTA_LT))
+ ret = pcie_capability_read_word(parent, PCI_EXP_LNKSTA, ®16);
+ if (ret || !(reg16 & PCI_EXP_LNKSTA_LT))
break;
msleep(1);
} while (time_before(jiffies, end_jiffies));
@@ -237,7 +238,7 @@ static bool pcie_retrain_link(struct pcie_link_state *link)
*/
static void pcie_aspm_configure_common_clock(struct pcie_link_state *link)
{
- int same_clock = 1;
+ int ret, same_clock = 1;
u16 reg16, parent_reg, child_reg[8];
struct pci_dev *child, *parent = link->pdev;
struct pci_bus *linkbus = parent->subordinate;
@@ -249,24 +250,24 @@ static void pcie_aspm_configure_common_clock(struct pcie_link_state *link)
BUG_ON(!pci_is_pcie(child));
/* Check downstream component if bit Slot Clock Configuration is 1 */
- pcie_capability_read_word(child, PCI_EXP_LNKSTA, ®16);
- if (!(reg16 & PCI_EXP_LNKSTA_SLC))
+ ret = pcie_capability_read_word(child, PCI_EXP_LNKSTA, ®16);
+ if (ret || !(reg16 & PCI_EXP_LNKSTA_SLC))
same_clock = 0;
/* Check upstream component if bit Slot Clock Configuration is 1 */
- pcie_capability_read_word(parent, PCI_EXP_LNKSTA, ®16);
- if (!(reg16 & PCI_EXP_LNKSTA_SLC))
+ ret = pcie_capability_read_word(parent, PCI_EXP_LNKSTA, ®16);
+ if (ret || !(reg16 & PCI_EXP_LNKSTA_SLC))
same_clock = 0;
/* Port might be already in common clock mode */
- pcie_capability_read_word(parent, PCI_EXP_LNKCTL, ®16);
- if (same_clock && (reg16 & PCI_EXP_LNKCTL_CCC)) {
+ ret = pcie_capability_read_word(parent, PCI_EXP_LNKCTL, ®16);
+ if (!ret && same_clock && (reg16 & PCI_EXP_LNKCTL_CCC)) {
bool consistent = true;
list_for_each_entry(child, &linkbus->devices, bus_list) {
- pcie_capability_read_word(child, PCI_EXP_LNKCTL,
+ ret = pcie_capability_read_word(child, PCI_EXP_LNKCTL,
®16);
- if (!(reg16 & PCI_EXP_LNKCTL_CCC)) {
+ if (ret || !(reg16 & PCI_EXP_LNKCTL_CCC)) {
consistent = false;
break;
}
--
2.18.2
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Linux-kernel-mentees] [PATCH 11/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*()
2020-07-06 9:31 [Linux-kernel-mentees] [PATCH 0/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*() Saheed Olayemi Bolarinwa
` (6 preceding siblings ...)
2020-07-06 9:31 ` [Linux-kernel-mentees] [PATCH 10/11 RFC] PCI/ASPM: " Saheed Olayemi Bolarinwa
@ 2020-07-06 9:31 ` Saheed Olayemi Bolarinwa
7 siblings, 0 replies; 11+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-06 9:31 UTC (permalink / raw)
To: helgaas
Cc: Mike Marciniszyn, linuxppc-dev, Arnd Bergmann, Jason Gunthorpe,
Sam Bobroff, Bolarinwa Olayemi Saheed, Dennis Dalessandro,
Rafael J. Wysocki, linux-kernel, Len Brown Lukas Wunner,
linux-acpi, Doug Ledford, Russell Currey, linux-pci,
Oliver O'Halloran, linux-kernel-mentees, linux-rdma
From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
**TODO**
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
This patch depends on all of the preceeding patches in this series,
otherwise it will introduce bugs as pointed out in the commit message
of each.
drivers/pci/access.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 79c4a2ef269a..ec95edbb1ac8 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -413,13 +413,6 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val)
if (pcie_capability_reg_implemented(dev, pos)) {
ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val);
- /*
- * Reset *val to 0 if pci_read_config_word() fails, it may
- * have been written as 0xFFFF if hardware error happens
- * during pci_read_config_word().
- */
- if (ret)
- *val = 0;
return ret;
}
@@ -448,13 +441,6 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val)
if (pcie_capability_reg_implemented(dev, pos)) {
ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val);
- /*
- * Reset *val to 0 if pci_read_config_dword() fails, it may
- * have been written as 0xFFFFFFFF if hardware error happens
- * during pci_read_config_dword().
- */
- if (ret)
- *val = 0;
return ret;
}
--
2.18.2
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 11+ messages in thread