[PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Yang Ruibin]

Increase the reference count by calling pci_get_slot(), and remember to
decrement the reference count by calling pci_dev_put().

Signed-off-by: Yang Ruibin <11162571@vivo.com>
---
 drivers/spi/spi-pxa2xx-pci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index cc8dcf782..a7bf4568f 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -146,8 +146,10 @@ static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
 	c->num_chipselect = 1;
 
 	ret = pxa2xx_spi_pci_clk_register(dev, ssp, 50000000);
-	if (ret)
+	if (ret) {
+		pci_dev_put(dma_dev);
 		return ret;
+	}
 
 	dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
 	ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);
@@ -222,8 +224,10 @@ static int mrfld_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
 	}
 
 	ret = pxa2xx_spi_pci_clk_register(dev, ssp, 25000000);
-	if (ret)
+	if (ret) {
+		pci_dev_put(dma_dev);
 		return ret;
+	}
 
 	dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(21, 0));
 	ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);
-- 
2.34.1

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Mark Brown]

On Thu, 29 Aug 2024 11:35:11 +0800, Yang Ruibin wrote:
> Increase the reference count by calling pci_get_slot(), and remember to
> decrement the reference count by calling pci_dev_put().
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] drivers: spi: Insert the missing pci_dev_put()before return
      commit: 8a0ec8c2d736961ff556e9d331decda9048fe0f1

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Geert Uytterhoeven]

Hi Yang,

On Thu, Aug 29, 2024 at 5:35 AM Yang Ruibin <11162571@vivo.com> wrote:
> Increase the reference count by calling pci_get_slot(), and remember to
> decrement the reference count by calling pci_dev_put().
>
> Signed-off-by: Yang Ruibin <11162571@vivo.com>

Thanks for your patch, which is now commit 8a0ec8c2d736961f ("spi:
Insert the missing pci_dev_put()before return") in spi/for-next.

> --- a/drivers/spi/spi-pxa2xx-pci.c
> +++ b/drivers/spi/spi-pxa2xx-pci.c
> @@ -146,8 +146,10 @@ static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
>         c->num_chipselect = 1;
>
>         ret = pxa2xx_spi_pci_clk_register(dev, ssp, 50000000);
> -       if (ret)
> +       if (ret) {
> +               pci_dev_put(dma_dev);

dma_dev is still uninitialized at this point.

>                 return ret;
> +       }
>
>         dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));

dma_dev is initialized only here...

>         ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);

... and freed automatically by lpss_dma_put_device() in case of
any later failures since commit 609d7ffdc42199a0 ("spi: pxa2xx-pci:
Balance reference count for PCI DMA device") in v5.18.

> @@ -222,8 +224,10 @@ static int mrfld_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
>         }
>
>         ret = pxa2xx_spi_pci_clk_register(dev, ssp, 25000000);
> -       if (ret)
> +       if (ret) {
> +               pci_dev_put(dma_dev);
>                 return ret;
> +       }
>
>         dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(21, 0));
>         ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);

Likewise.

Hence this patch is not needed, and introduced two bugs.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Krzysztof Kozlowski]

On 30/08/2024 10:55, Geert Uytterhoeven wrote:
> Hi Yang,
> 
> On Thu, Aug 29, 2024 at 5:35 AM Yang Ruibin <11162571@vivo.com> wrote:
>> Increase the reference count by calling pci_get_slot(), and remember to
>> decrement the reference count by calling pci_dev_put().
>>
>> Signed-off-by: Yang Ruibin <11162571@vivo.com>
> 
> Thanks for your patch, which is now commit 8a0ec8c2d736961f ("spi:
> Insert the missing pci_dev_put()before return") in spi/for-next.
> 
>> --- a/drivers/spi/spi-pxa2xx-pci.c
>> +++ b/drivers/spi/spi-pxa2xx-pci.c
>> @@ -146,8 +146,10 @@ static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
>>         c->num_chipselect = 1;
>>
>>         ret = pxa2xx_spi_pci_clk_register(dev, ssp, 50000000);
>> -       if (ret)
>> +       if (ret) {
>> +               pci_dev_put(dma_dev);
> 
> dma_dev is still uninitialized at this point.
> 
>>                 return ret;
>> +       }
>>
>>         dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
> 
> dma_dev is initialized only here...
> 
>>         ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);
> 
> ... and freed automatically by lpss_dma_put_device() in case of
> any later failures since commit 609d7ffdc42199a0 ("spi: pxa2xx-pci:
> Balance reference count for PCI DMA device") in v5.18.
> 
>> @@ -222,8 +224,10 @@ static int mrfld_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
>>         }
>>
>>         ret = pxa2xx_spi_pci_clk_register(dev, ssp, 25000000);
>> -       if (ret)
>> +       if (ret) {
>> +               pci_dev_put(dma_dev);
>>                 return ret;
>> +       }
>>
>>         dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(21, 0));
>>         ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);
> 
> Likewise.
> 
> Hence this patch is not needed, and introduced two bugs.

Cc Greg, Jakub, David and Paolo,

It seems Vivo (at least two persons from vivo.com) is sending patches
generated through some sort of automation without really knowing what
they were doing. All of the patches look like innocent
cleanups/simplifications/fixes, but they do more.

This patch here looks like introducing two bugs.

These patches:
1. https://lore.kernel.org/all/20240830033251.232992-1-yujiaoliang@vivo.com/

2. https://lore.kernel.org/all/20240828122650.1324246-1-11162571@vivo.com/
(I sent a revert for this)

3. https://lore.kernel.org/all/20240829072016.2329466-1-11162571@vivo.com/

and probably more...

introduce dev_err_probe() outside of probe path which is not desired,
because it marks a probed (working) device as deferred.

The patches look trivial and/or helpful, so people tend to accept them
through default trust.

I kindly suggest reverse - do not trust them by default and instead do a
thorough review before accepting any cleanup/trivial patch from @vivo.com.

Best regards,
Krzysztof

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Jakub Kicinski]

On Fri, 30 Aug 2024 19:10:20 +0200 Krzysztof Kozlowski wrote:
> Cc Greg, Jakub, David and Paolo,

Thanks for the heads up!

I double checked the last 12mo of networking patches, FWIW.

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Yuesong Li]

On 2024/8/31 1:10, Krzysztof Kozlowski wrote:
> On 30/08/2024 10:55, Geert Uytterhoeven wrote:
>> Hi Yang,
>>
>> On Thu, Aug 29, 2024 at 5:35 AM Yang Ruibin <11162571@vivo.com> wrote:
>>> Increase the reference count by calling pci_get_slot(), and remember to
>>> decrement the reference count by calling pci_dev_put().
>>>
>>> Signed-off-by: Yang Ruibin <11162571@vivo.com>
>>
>> Thanks for your patch, which is now commit 8a0ec8c2d736961f ("spi:
>> Insert the missing pci_dev_put()before return") in spi/for-next.
>>
>>> --- a/drivers/spi/spi-pxa2xx-pci.c
>>> +++ b/drivers/spi/spi-pxa2xx-pci.c
>>> @@ -146,8 +146,10 @@ static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
>>>          c->num_chipselect = 1;
>>>
>>>          ret = pxa2xx_spi_pci_clk_register(dev, ssp, 50000000);
>>> -       if (ret)
>>> +       if (ret) {
>>> +               pci_dev_put(dma_dev);
>>
>> dma_dev is still uninitialized at this point.
>>
>>>                  return ret;
>>> +       }
>>>
>>>          dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
>>
>> dma_dev is initialized only here...
>>
>>>          ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);
>>
>> ... and freed automatically by lpss_dma_put_device() in case of
>> any later failures since commit 609d7ffdc42199a0 ("spi: pxa2xx-pci:
>> Balance reference count for PCI DMA device") in v5.18.
>>
>>> @@ -222,8 +224,10 @@ static int mrfld_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
>>>          }
>>>
>>>          ret = pxa2xx_spi_pci_clk_register(dev, ssp, 25000000);
>>> -       if (ret)
>>> +       if (ret) {
>>> +               pci_dev_put(dma_dev);
>>>                  return ret;
>>> +       }
>>>
>>>          dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(21, 0));
>>>          ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);
>>
>> Likewise.
>>
>> Hence this patch is not needed, and introduced two bugs.
> 
> Cc Greg, Jakub, David and Paolo,
> 
> It seems Vivo (at least two persons from vivo.com) is sending patches
> generated through some sort of automation without really knowing what
> they were doing. All of the patches look like innocent
> cleanups/simplifications/fixes, but they do more.
> 
> This patch here looks like introducing two bugs.
> 
> These patches:
> 1. https://lore.kernel.org/all/20240830033251.232992-1-yujiaoliang@vivo.com/
> 
> 2. https://lore.kernel.org/all/20240828122650.1324246-1-11162571@vivo.com/
> (I sent a revert for this)
> 
> 3. https://lore.kernel.org/all/20240829072016.2329466-1-11162571@vivo.com/
> 
> and probably more...
> 
> introduce dev_err_probe() outside of probe path which is not desired,
> because it marks a probed (working) device as deferred.
> 
> The patches look trivial and/or helpful, so people tend to accept them
> through default trust.
> 
> I kindly suggest reverse - do not trust them by default and instead do a
> thorough review before accepting any cleanup/trivial patch from @vivo.com.
> 
> Best regards,
> Krzysztof
> 
> 

Dear Geert, Krzysztof, and the Linux Kernel Community,

I hope this message finds you well. My name is Yuesong Li, and I am 
writing on behalf of VIVO to sincerely apologize for the recent issues 
caused by the patches submitted by our team members. We deeply regret 
the problems that these submissions have introduced and the concerns 
they have raised within the community.

We recognize that the patches submitted were not up to the standards 
expected by the Linux kernel community. It is clear that our team 
members did not fully understand the implications of their 
contributions, leading to errors and the need for reverts. This is 
entirely our responsibility, and we are committed to ensuring that this 
does not happen again.

To address these issues, VIVO is taking the following steps:

1.Training for employees: We are implementing a comprehensive training 
program for all employees who contribute to open source projects. This 
training will focus on understanding the intricacies of the Linux 
kernel, best practices for code submissions, and the importance of 
thorough testing and review before submitting patches.

2.Enhanced Internal Review Process: Moving forward, we will enforce a 
more rigorous internal review process for all patches before they are 
submitted to the community. This will involve senior developers with 
experience in the open source community who will guide and review the 
work of less experienced contributors.

We value the open-source community and the collaborative spirit that 
drives it. VIVO is committed to contributing positively and responsibly 
moving forward. We kindly ask for your forgiveness for the mistakes 
we've made and your understanding as we take concrete steps to improve.

Thank you for your continued dedication to the Linux kernel, and please 
feel free to reach out if there are any further concerns or if you have 
suggestions on how we can better align with the community's expectations.

Best Regards,
Yuesong Li
VIVO

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Greg Kroah-Hartman]

On Mon, Sep 02, 2024 at 03:15:48PM +0800, Yuesong Li wrote:
> 
> 
> On 2024/8/31 1:10, Krzysztof Kozlowski wrote:
> > On 30/08/2024 10:55, Geert Uytterhoeven wrote:
> > > Hi Yang,
> > > 
> > > On Thu, Aug 29, 2024 at 5:35 AM Yang Ruibin <11162571@vivo.com> wrote:
> > > > Increase the reference count by calling pci_get_slot(), and remember to
> > > > decrement the reference count by calling pci_dev_put().
> > > > 
> > > > Signed-off-by: Yang Ruibin <11162571@vivo.com>
> > > 
> > > Thanks for your patch, which is now commit 8a0ec8c2d736961f ("spi:
> > > Insert the missing pci_dev_put()before return") in spi/for-next.
> > > 
> > > > --- a/drivers/spi/spi-pxa2xx-pci.c
> > > > +++ b/drivers/spi/spi-pxa2xx-pci.c
> > > > @@ -146,8 +146,10 @@ static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
> > > >          c->num_chipselect = 1;
> > > > 
> > > >          ret = pxa2xx_spi_pci_clk_register(dev, ssp, 50000000);
> > > > -       if (ret)
> > > > +       if (ret) {
> > > > +               pci_dev_put(dma_dev);
> > > 
> > > dma_dev is still uninitialized at this point.
> > > 
> > > >                  return ret;
> > > > +       }
> > > > 
> > > >          dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
> > > 
> > > dma_dev is initialized only here...
> > > 
> > > >          ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);
> > > 
> > > ... and freed automatically by lpss_dma_put_device() in case of
> > > any later failures since commit 609d7ffdc42199a0 ("spi: pxa2xx-pci:
> > > Balance reference count for PCI DMA device") in v5.18.
> > > 
> > > > @@ -222,8 +224,10 @@ static int mrfld_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
> > > >          }
> > > > 
> > > >          ret = pxa2xx_spi_pci_clk_register(dev, ssp, 25000000);
> > > > -       if (ret)
> > > > +       if (ret) {
> > > > +               pci_dev_put(dma_dev);
> > > >                  return ret;
> > > > +       }
> > > > 
> > > >          dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(21, 0));
> > > >          ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);
> > > 
> > > Likewise.
> > > 
> > > Hence this patch is not needed, and introduced two bugs.
> > 
> > Cc Greg, Jakub, David and Paolo,
> > 
> > It seems Vivo (at least two persons from vivo.com) is sending patches
> > generated through some sort of automation without really knowing what
> > they were doing. All of the patches look like innocent
> > cleanups/simplifications/fixes, but they do more.
> > 
> > This patch here looks like introducing two bugs.
> > 
> > These patches:
> > 1. https://lore.kernel.org/all/20240830033251.232992-1-yujiaoliang@vivo.com/
> > 
> > 2. https://lore.kernel.org/all/20240828122650.1324246-1-11162571@vivo.com/
> > (I sent a revert for this)
> > 
> > 3. https://lore.kernel.org/all/20240829072016.2329466-1-11162571@vivo.com/
> > 
> > and probably more...
> > 
> > introduce dev_err_probe() outside of probe path which is not desired,
> > because it marks a probed (working) device as deferred.
> > 
> > The patches look trivial and/or helpful, so people tend to accept them
> > through default trust.
> > 
> > I kindly suggest reverse - do not trust them by default and instead do a
> > thorough review before accepting any cleanup/trivial patch from @vivo.com.
> > 
> > Best regards,
> > Krzysztof
> > 
> > 
> 
> Dear Geert, Krzysztof, and the Linux Kernel Community,
> 
> I hope this message finds you well. My name is Yuesong Li, and I am writing
> on behalf of VIVO to sincerely apologize for the recent issues caused by the
> patches submitted by our team members. We deeply regret the problems that
> these submissions have introduced and the concerns they have raised within
> the community.
> 
> We recognize that the patches submitted were not up to the standards
> expected by the Linux kernel community. It is clear that our team members
> did not fully understand the implications of their contributions, leading to
> errors and the need for reverts. This is entirely our responsibility, and we
> are committed to ensuring that this does not happen again.
> 
> To address these issues, VIVO is taking the following steps:
> 
> 1.Training for employees: We are implementing a comprehensive training
> program for all employees who contribute to open source projects. This
> training will focus on understanding the intricacies of the Linux kernel,
> best practices for code submissions, and the importance of thorough testing
> and review before submitting patches.
> 
> 2.Enhanced Internal Review Process: Moving forward, we will enforce a more
> rigorous internal review process for all patches before they are submitted
> to the community. This will involve senior developers with experience in the
> open source community who will guide and review the work of less experienced
> contributors.
> 
> We value the open-source community and the collaborative spirit that drives
> it. VIVO is committed to contributing positively and responsibly moving
> forward. We kindly ask for your forgiveness for the mistakes we've made and
> your understanding as we take concrete steps to improve.
> 
> Thank you for your continued dedication to the Linux kernel, and please feel
> free to reach out if there are any further concerns or if you have
> suggestions on how we can better align with the community's expectations.

Thanks for doing this.  I've now dropped all pending vivo patches that
were in my review queues and will wait for this process to happen so
that they can be resubmitted after proper review by your internal
groups.

good luck!

greg k-h

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Andy Shevchenko]

On Fri, Aug 30, 2024 at 10:55:06AM +0200, Geert Uytterhoeven wrote:
> On Thu, Aug 29, 2024 at 5:35 AM Yang Ruibin <11162571@vivo.com> wrote:
> > Increase the reference count by calling pci_get_slot(), and remember to
> > decrement the reference count by calling pci_dev_put().
> >
> > Signed-off-by: Yang Ruibin <11162571@vivo.com>
> 
> Thanks for your patch, which is now commit 8a0ec8c2d736961f ("spi:
> Insert the missing pci_dev_put()before return") in spi/for-next.

Which should be reverted.
Who is going to send one?

I also have one revert for double applied spi-ppc44x commit.

But Mark haven't replied if I need to send a v2 of that as it has Subject and
first line of the commit message generated by `git revert` without editing.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Mark Brown]

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

On Fri, Aug 30, 2024 at 10:12:03PM +0300, Andy Shevchenko wrote:

> But Mark haven't replied if I need to send a v2 of that as it has Subject and
> first line of the commit message generated by `git revert` without editing.

Yes, you should resend.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Andy Shevchenko]

On Fri, Aug 30, 2024 at 08:56:56PM +0100, Mark Brown wrote:
> On Fri, Aug 30, 2024 at 10:12:03PM +0300, Andy Shevchenko wrote:
> 
> > But Mark haven't replied if I need to send a v2 of that as it has Subject and
> > first line of the commit message generated by `git revert` without editing.
> 
> Yes, you should resend.

Got it!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Mark Brown]

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

On Fri, Aug 30, 2024 at 10:55:06AM +0200, Geert Uytterhoeven wrote:
> On Thu, Aug 29, 2024 at 5:35 AM Yang Ruibin <11162571@vivo.com> wrote:

> > -       if (ret)
> > +       if (ret) {
> > +               pci_dev_put(dma_dev);

> dma_dev is still uninitialized at this point.

I'm a bit concerned that this isn't picked up by an allmodconfig with
the -Werror...  I'm currently using GCC 12 for that.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Nathan Chancellor]

On Fri, Aug 30, 2024 at 08:57:47PM +0100, Mark Brown wrote:
> On Fri, Aug 30, 2024 at 10:55:06AM +0200, Geert Uytterhoeven wrote:
> > On Thu, Aug 29, 2024 at 5:35 AM Yang Ruibin <11162571@vivo.com> wrote:
> 
> > > -       if (ret)
> > > +       if (ret) {
> > > +               pci_dev_put(dma_dev);
> 
> > dma_dev is still uninitialized at this point.
> 
> I'm a bit concerned that this isn't picked up by an allmodconfig with
> the -Werror...  I'm currently using GCC 12 for that.

It shows up with -Wmaybe-uninitialized for GCC but that's disabled for
the normal kernel build with commit 78a5255ffb6a ("Stop the ad-hoc games
with -Wno-maybe-initialized"). With GCC 12:

  drivers/spi/spi-pxa2xx-pci.c: In function ‘mrfld_spi_setup’:
  drivers/spi/spi-pxa2xx-pci.c:228:17: error: ‘dma_dev’ may be used uninitialized [-Werror=maybe-uninitialized]
    228 |                 pci_dev_put(dma_dev);
        |                 ^~~~~~~~~~~~~~~~~~~~
  drivers/spi/spi-pxa2xx-pci.c:198:25: note: ‘dma_dev’ was declared here
    198 |         struct pci_dev *dma_dev;
        |                         ^~~~~~~
  drivers/spi/spi-pxa2xx-pci.c: In function ‘lpss_spi_setup’:
  drivers/spi/spi-pxa2xx-pci.c:150:17: error: ‘dma_dev’ may be used uninitialized [-Werror=maybe-uninitialized]
    150 |                 pci_dev_put(dma_dev);
        |                 ^~~~~~~~~~~~~~~~~~~~
  drivers/spi/spi-pxa2xx-pci.c:100:25: note: ‘dma_dev’ was declared here
    100 |         struct pci_dev *dma_dev;
        |                         ^~~~~~~

Clang has it under -Wuninitialized, where it was caught with a regular
allmodconfig build:

  drivers/spi/spi-pxa2xx-pci.c:150:15: error: variable 'dma_dev' is uninitialized when used here [-Werror,-Wuninitialized]
    150 |                 pci_dev_put(dma_dev);
        |                             ^~~~~~~
  drivers/spi/spi-pxa2xx-pci.c:100:25: note: initialize the variable 'dma_dev' to silence this warning
    100 |         struct pci_dev *dma_dev;
        |                                ^
        |                                 = NULL
  drivers/spi/spi-pxa2xx-pci.c:228:15: error: variable 'dma_dev' is uninitialized when used here [-Werror,-Wuninitialized]
    228 |                 pci_dev_put(dma_dev);
        |                             ^~~~~~~
  drivers/spi/spi-pxa2xx-pci.c:198:25: note: initialize the variable 'dma_dev' to silence this warning
    198 |         struct pci_dev *dma_dev;
        |                                ^
        |                                 = NULL

Perhaps a KCFLAGS=-Wmaybe-uninitialized in your make command or adding

  subdir-ccflags-$(CONFIG_CC_IS_GCC) := -Wmaybe-uninitialized

to the makefiles of the drivers that you maintain might not be a bad
idea.

Cheers,
Nathan

Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

[Mark Brown]

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

On Fri, Aug 30, 2024 at 03:23:32PM -0700, Nathan Chancellor wrote:
> On Fri, Aug 30, 2024 at 08:57:47PM +0100, Mark Brown wrote:

> > I'm a bit concerned that this isn't picked up by an allmodconfig with
> > the -Werror...  I'm currently using GCC 12 for that.
> 
> It shows up with -Wmaybe-uninitialized for GCC but that's disabled for
> the normal kernel build with commit 78a5255ffb6a ("Stop the ad-hoc games
> with -Wno-maybe-initialized"). With GCC 12:

...

> Perhaps a KCFLAGS=-Wmaybe-uninitialized in your make command or adding
> 
>   subdir-ccflags-$(CONFIG_CC_IS_GCC) := -Wmaybe-uninitialized
> 
> to the makefiles of the drivers that you maintain might not be a bad
> idea.

If it's causing so many false positives that it's been disabled then
that'll just cause trouble, it's always miserable whenever a subsystem
decides to go with custom warning options.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]