linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] PCI: brcmstb: variable is missing proper initialization
@ 2020-11-02 20:57 Jim Quinlan
  2020-11-02 21:07 ` Florian Fainelli
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Jim Quinlan @ 2020-11-02 20:57 UTC (permalink / raw)
  To: linux-pci, bcm-kernel-feedback-list, james.quinlan,
	Rafał Miłecki
  Cc: Nicolas Saenz Julienne, Lorenzo Pieralisi, Rob Herring,
	Bjorn Helgaas, Florian Fainelli, Andrew Murray, Jeremy Linton,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list

[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]

The variable 'tmp' is used multiple times in the brcm_pcie_setup()
function.  One such usage did not initialize 'tmp' to the current value of
the target register.  By luck the mistake does not currently affect
behavior;  regardless 'tmp' is now initialized properly.

Fixes: c0452137034b ("PCI: brcmstb: Add Broadcom STB PCIe host controller driver")
Suggested-by: Rafał Miłecki <zajec5@gmail.com>
Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index bea86899bd5d..9c3d2982248d 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -893,6 +893,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 		burst = 0x2; /* 512 bytes */
 
 	/* Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN */
+	tmp = readl(base + PCIE_MISC_MISC_CTRL);
 	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK);
 	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
 	u32p_replace_bits(&tmp, burst, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
-- 
2.17.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] PCI: brcmstb: variable is missing proper initialization
  2020-11-02 20:57 [PATCH v1] PCI: brcmstb: variable is missing proper initialization Jim Quinlan
@ 2020-11-02 21:07 ` Florian Fainelli
  2020-11-03 17:32   ` Nicolas Saenz Julienne
  2020-11-03 19:38 ` Bjorn Helgaas
  2020-11-20 17:07 ` Lorenzo Pieralisi
  2 siblings, 1 reply; 7+ messages in thread
From: Florian Fainelli @ 2020-11-02 21:07 UTC (permalink / raw)
  To: Jim Quinlan, linux-pci, bcm-kernel-feedback-list,
	Rafał Miłecki
  Cc: Nicolas Saenz Julienne, Lorenzo Pieralisi, Rob Herring,
	Bjorn Helgaas, Florian Fainelli, Andrew Murray, Jeremy Linton,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list



On 11/2/2020 12:57 PM, Jim Quinlan wrote:
> The variable 'tmp' is used multiple times in the brcm_pcie_setup()
> function.  One such usage did not initialize 'tmp' to the current value of
> the target register.  By luck the mistake does not currently affect
> behavior;  regardless 'tmp' is now initialized properly.
> 
> Fixes: c0452137034b ("PCI: brcmstb: Add Broadcom STB PCIe host controller driver")
> Suggested-by: Rafał Miłecki <zajec5@gmail.com>
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] PCI: brcmstb: variable is missing proper initialization
  2020-11-02 21:07 ` Florian Fainelli
@ 2020-11-03 17:32   ` Nicolas Saenz Julienne
  0 siblings, 0 replies; 7+ messages in thread
From: Nicolas Saenz Julienne @ 2020-11-03 17:32 UTC (permalink / raw)
  To: Florian Fainelli, Jim Quinlan, linux-pci,
	bcm-kernel-feedback-list, Rafał Miłecki
  Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Andrew Murray,
	Jeremy Linton,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list

[-- Attachment #1: Type: text/plain, Size: 668 bytes --]

On Mon, 2020-11-02 at 13:07 -0800, Florian Fainelli wrote:
> 
> On 11/2/2020 12:57 PM, Jim Quinlan wrote:
> > The variable 'tmp' is used multiple times in the brcm_pcie_setup()
> > function.  One such usage did not initialize 'tmp' to the current value of
> > the target register.  By luck the mistake does not currently affect
> > behavior;  regardless 'tmp' is now initialized properly.
> > 
> > Fixes: c0452137034b ("PCI: brcmstb: Add Broadcom STB PCIe host controller driver")
> > Suggested-by: Rafał Miłecki <zajec5@gmail.com>
> > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Acked-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] PCI: brcmstb: variable is missing proper initialization
  2020-11-02 20:57 [PATCH v1] PCI: brcmstb: variable is missing proper initialization Jim Quinlan
  2020-11-02 21:07 ` Florian Fainelli
@ 2020-11-03 19:38 ` Bjorn Helgaas
  2020-11-03 19:40   ` Nicolas Saenz Julienne
  2020-11-04 14:48   ` Jim Quinlan
  2020-11-20 17:07 ` Lorenzo Pieralisi
  2 siblings, 2 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2020-11-03 19:38 UTC (permalink / raw)
  To: Jim Quinlan
  Cc: linux-pci, bcm-kernel-feedback-list, Rafał Miłecki,
	Nicolas Saenz Julienne, Lorenzo Pieralisi, Rob Herring,
	Bjorn Helgaas, Florian Fainelli, Andrew Murray, Jeremy Linton,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list

On Mon, Nov 02, 2020 at 03:57:12PM -0500, Jim Quinlan wrote:
> The variable 'tmp' is used multiple times in the brcm_pcie_setup()
> function.  One such usage did not initialize 'tmp' to the current value of
> the target register.  By luck the mistake does not currently affect
> behavior;  regardless 'tmp' is now initialized properly.

This is so trivial that there's probably no reason to post this again,
but if you post a v2 for some reason, please update the subject to
match the convention ("PCI: brcmstb: Verb ..."), e.g.,

  PCI: brcmstb: Initialize "tmp" before use

The commit log does not say what the patch does, leaving it to the
reader to infer it.

Lorenzo will likely fix this up when he applies it.

Incidental curiosity: where should I look to see what
u32p_replace_bits() does?  "git grep u32p_replace_bits" shows several
calls, but no definitions.

> Fixes: c0452137034b ("PCI: brcmstb: Add Broadcom STB PCIe host controller driver")
> Suggested-by: Rafał Miłecki <zajec5@gmail.com>
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> ---
>  drivers/pci/controller/pcie-brcmstb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index bea86899bd5d..9c3d2982248d 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -893,6 +893,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>  		burst = 0x2; /* 512 bytes */
>  
>  	/* Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN */
> +	tmp = readl(base + PCIE_MISC_MISC_CTRL);
>  	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK);
>  	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
>  	u32p_replace_bits(&tmp, burst, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
> -- 
> 2.17.1
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] PCI: brcmstb: variable is missing proper initialization
  2020-11-03 19:38 ` Bjorn Helgaas
@ 2020-11-03 19:40   ` Nicolas Saenz Julienne
  2020-11-04 14:48   ` Jim Quinlan
  1 sibling, 0 replies; 7+ messages in thread
From: Nicolas Saenz Julienne @ 2020-11-03 19:40 UTC (permalink / raw)
  To: Bjorn Helgaas, Jim Quinlan
  Cc: linux-pci, bcm-kernel-feedback-list, Rafał Miłecki,
	Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Florian Fainelli,
	Andrew Murray, Jeremy Linton,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

On Tue, 2020-11-03 at 13:38 -0600, Bjorn Helgaas wrote:
> On Mon, Nov 02, 2020 at 03:57:12PM -0500, Jim Quinlan wrote:
> > The variable 'tmp' is used multiple times in the brcm_pcie_setup()
> > function.  One such usage did not initialize 'tmp' to the current value of
> > the target register.  By luck the mistake does not currently affect
> > behavior;  regardless 'tmp' is now initialized properly.
> 
> This is so trivial that there's probably no reason to post this again,
> but if you post a v2 for some reason, please update the subject to
> match the convention ("PCI: brcmstb: Verb ..."), e.g.,
> 
>   PCI: brcmstb: Initialize "tmp" before use
> 
> The commit log does not say what the patch does, leaving it to the
> reader to infer it.
> 
> Lorenzo will likely fix this up when he applies it.
> 
> Incidental curiosity: where should I look to see what
> u32p_replace_bits() does?  "git grep u32p_replace_bits" shows several
> calls, but no definitions.

It's a bunch if defines in 'include/linux/bitfield.h'

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] PCI: brcmstb: variable is missing proper initialization
  2020-11-03 19:38 ` Bjorn Helgaas
  2020-11-03 19:40   ` Nicolas Saenz Julienne
@ 2020-11-04 14:48   ` Jim Quinlan
  1 sibling, 0 replies; 7+ messages in thread
From: Jim Quinlan @ 2020-11-04 14:48 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
	maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
	Rafał Miłecki, Nicolas Saenz Julienne,
	Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Florian Fainelli,
	Andrew Murray, Jeremy Linton,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list

[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]

On Tue, Nov 3, 2020 at 2:38 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Nov 02, 2020 at 03:57:12PM -0500, Jim Quinlan wrote:
> > The variable 'tmp' is used multiple times in the brcm_pcie_setup()
> > function.  One such usage did not initialize 'tmp' to the current value of
> > the target register.  By luck the mistake does not currently affect
> > behavior;  regardless 'tmp' is now initialized properly.
>
> This is so trivial that there's probably no reason to post this again,
> but if you post a v2 for some reason, please update the subject to
> match the convention ("PCI: brcmstb: Verb ..."), e.g.,
>
>   PCI: brcmstb: Initialize "tmp" before use
>
> The commit log does not say what the patch does, leaving it to the
> reader to infer it.

Got it.
Thanks,
Jim Quinlan
Broadcom STB
>
>
>
>
> Lorenzo will likely fix this up when he applies it.
>
> Incidental curiosity: where should I look to see what
> u32p_replace_bits() does?  "git grep u32p_replace_bits" shows several
> calls, but no definitions.
>
> > Fixes: c0452137034b ("PCI: brcmstb: Add Broadcom STB PCIe host controller driver")
> > Suggested-by: Rafał Miłecki <zajec5@gmail.com>
> > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> > ---
> >  drivers/pci/controller/pcie-brcmstb.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> > index bea86899bd5d..9c3d2982248d 100644
> > --- a/drivers/pci/controller/pcie-brcmstb.c
> > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > @@ -893,6 +893,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> >               burst = 0x2; /* 512 bytes */
> >
> >       /* Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN */
> > +     tmp = readl(base + PCIE_MISC_MISC_CTRL);
> >       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK);
> >       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
> >       u32p_replace_bits(&tmp, burst, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
> > --
> > 2.17.1
> >
>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1] PCI: brcmstb: variable is missing proper initialization
  2020-11-02 20:57 [PATCH v1] PCI: brcmstb: variable is missing proper initialization Jim Quinlan
  2020-11-02 21:07 ` Florian Fainelli
  2020-11-03 19:38 ` Bjorn Helgaas
@ 2020-11-20 17:07 ` Lorenzo Pieralisi
  2 siblings, 0 replies; 7+ messages in thread
From: Lorenzo Pieralisi @ 2020-11-20 17:07 UTC (permalink / raw)
  To: linux-pci, bcm-kernel-feedback-list, Rafał Miłecki,
	Jim Quinlan
  Cc: Lorenzo Pieralisi, Florian Fainelli, Nicolas Saenz Julienne,
	open list, Andrew Murray,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Bjorn Helgaas, Rob Herring, Jeremy Linton

On Mon, 2 Nov 2020 15:57:12 -0500, Jim Quinlan wrote:
> The variable 'tmp' is used multiple times in the brcm_pcie_setup()
> function.  One such usage did not initialize 'tmp' to the current value of
> the target register.  By luck the mistake does not currently affect
> behavior;  regardless 'tmp' is now initialized properly.

Applied to pci/brcmstb, thanks!

[1/1] PCI: brcmstb: Initialize "tmp" before use
      https://git.kernel.org/lpieralisi/pci/c/ddaff0af65

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-11-20 17:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-02 20:57 [PATCH v1] PCI: brcmstb: variable is missing proper initialization Jim Quinlan
2020-11-02 21:07 ` Florian Fainelli
2020-11-03 17:32   ` Nicolas Saenz Julienne
2020-11-03 19:38 ` Bjorn Helgaas
2020-11-03 19:40   ` Nicolas Saenz Julienne
2020-11-04 14:48   ` Jim Quinlan
2020-11-20 17:07 ` Lorenzo Pieralisi

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).