diff mbox series

[v3,2/4] Documentation: devres: Add pcim_alloc_irq_vectors()

Message ID 20210216160249.749799-3-zhengdejin5@gmail.com
State Superseded
Headers show
Series Introduce pcim_alloc_irq_vectors() | expand

Commit Message

Dejin Zheng Feb. 16, 2021, 4:02 p.m. UTC
Add pcim_alloc_irq_vectors(), a device-managed version of
pci_alloc_irq_vectors(). introducing this function can simplify
the error handling path in many drivers.

Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
---
v2 -> v3:
	- No change
v1 -> v2:
	- Modify some commit messages.
 Documentation/driver-api/driver-model/devres.rst | 1 +
 1 file changed, 1 insertion(+)

Comments

Krzysztof Wilczyński Feb. 16, 2021, 5:10 p.m. UTC | #1
Hi Dejin,

Thank you again for all the work here!

> Add pcim_alloc_irq_vectors(), a device-managed version of
> pci_alloc_irq_vectors(). introducing this function can simplify
> the error handling path in many drivers.

The second sentence should most likely start with a capital letter.

Having said that, people might ask - how does it simplify the error
handling path?

You might have to back this with a line of two to explain how does the
change achieved that, so that when someone looks at the commit message
it would be clear what the benefits of the change were.

Reviewed-by: Krzysztof Wilczyński <kw@linux.com>

Krzysztof
Dejin Zheng Feb. 17, 2021, 10:50 a.m. UTC | #2
On Tue, Feb 16, 2021 at 06:10:52PM +0100, Krzysztof Wilczyński wrote:
> Hi Dejin,

> 

> Thank you again for all the work here!

> 

> > Add pcim_alloc_irq_vectors(), a device-managed version of

> > pci_alloc_irq_vectors(). introducing this function can simplify

> > the error handling path in many drivers.

> 

> The second sentence should most likely start with a capital letter.

> 

> Having said that, people might ask - how does it simplify the error

> handling path?

> 

> You might have to back this with a line of two to explain how does the

> change achieved that, so that when someone looks at the commit message

> it would be clear what the benefits of the change were.

>

Hi Krzysztof,

The device-managed function is a conventional concept that every developer
knows. So don't worry about this. And I really can't explain its operation
mechanism to you in a sentence or two. If you are really interested, you
can read the relevant code.

I think it is meaningless to add a lot of explanations of general
concepts in the commit comments. Maybe it will be better let us put more
energy and time on the code.

BR,
Dejin

> Reviewed-by: Krzysztof Wilczyński <kw@linux.com>

> 

> Krzysztof
Andy Shevchenko Feb. 17, 2021, 1:44 p.m. UTC | #3
On Wed, Feb 17, 2021 at 06:50:04PM +0800, Dejin Zheng wrote:
> On Tue, Feb 16, 2021 at 06:10:52PM +0100, Krzysztof Wilczyński wrote:


...

> > Having said that, people might ask - how does it simplify the error

> > handling path?

> > 

> > You might have to back this with a line of two to explain how does the

> > change achieved that, so that when someone looks at the commit message

> > it would be clear what the benefits of the change were.


> The device-managed function is a conventional concept that every developer

> knows. So don't worry about this. And I really can't explain its operation

> mechanism to you in a sentence or two. If you are really interested, you

> can read the relevant code.


I tend on agree on the above. It would be enough to spell it clearly that it's
part of devres API (Managed Device Resource) and we are fine.

-- 
With Best Regards,
Andy Shevchenko
diff mbox series

Patch

diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index cd8b6e657b94..a52f65b6352f 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -380,6 +380,7 @@  PCI
   devm_pci_alloc_host_bridge()  : managed PCI host bridge allocation
   devm_pci_remap_cfgspace()	: ioremap PCI configuration space
   devm_pci_remap_cfg_resource()	: ioremap PCI configuration space resource
+  pcim_alloc_irq_vectors()      : managed IRQ vectors allocation
   pcim_enable_device()		: after success, all PCI ops become managed
   pcim_pin_device()		: keep PCI device enabled after release