* [PATCH] PCI: ibmphp: Adjusting the parameter list of a function
@ 2022-08-14 15:19 Abd-Alrhman Masalkhi
2022-08-18 23:07 ` Bjorn Helgaas
0 siblings, 1 reply; 3+ messages in thread
From: Abd-Alrhman Masalkhi @ 2022-08-14 15:19 UTC (permalink / raw)
To: bhelgaas
Cc: kw, os.vaslot, makvihas, linux-pci, linux-kernel, Abd-Alrhman Masalkhi
ibmphp_init_devno() takes a struct slot **, and it should be changed
to take struct slot * instead.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
drivers/pci/hotplug/TODO | 3 ---
drivers/pci/hotplug/ibmphp.h | 3 ++-
drivers/pci/hotplug/ibmphp_core.c | 30 +++++++++++++++---------------
drivers/pci/hotplug/ibmphp_ebda.c | 2 +-
4 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/hotplug/TODO b/drivers/pci/hotplug/TODO
index 88f217c82b4f..63a9fed407ed 100644
--- a/drivers/pci/hotplug/TODO
+++ b/drivers/pci/hotplug/TODO
@@ -30,9 +30,6 @@ ibmphp:
or ibmphp should store a pointer to its bus in struct slot. Probably the
former.
-* ibmphp_init_devno() takes a struct slot **, it could instead take a
- struct slot *.
-
* The return value of pci_hp_register() is not checked.
* The various slot data structures are difficult to follow and need to be
diff --git a/drivers/pci/hotplug/ibmphp.h b/drivers/pci/hotplug/ibmphp.h
index 0399c60d2ec1..148a421eb262 100644
--- a/drivers/pci/hotplug/ibmphp.h
+++ b/drivers/pci/hotplug/ibmphp.h
@@ -734,7 +734,8 @@ struct controller {
/* Functions */
-int ibmphp_init_devno(struct slot **); /* This function is called from EBDA, so we need it not be static */
+/* ibmphp_init_devno() is called from EBDA, so we need it not be static */
+int ibmphp_init_devno(struct slot *cur_slot);
int ibmphp_do_disable_slot(struct slot *slot_cur);
int ibmphp_update_slot_info(struct slot *); /* This function is called from HPC, so we need it to not be static */
int ibmphp_configure_card(struct pci_func *, u8);
diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
index 197997e264a2..107752b11f2c 100644
--- a/drivers/pci/hotplug/ibmphp_core.c
+++ b/drivers/pci/hotplug/ibmphp_core.c
@@ -109,7 +109,7 @@ static int __init get_max_slots(void)
* Parameters: struct slot
* Returns 0 or errors
*/
-int ibmphp_init_devno(struct slot **cur_slot)
+int ibmphp_init_devno(struct slot *cur_slot)
{
struct irq_routing_table *rtable;
int len;
@@ -130,21 +130,21 @@ int ibmphp_init_devno(struct slot **cur_slot)
return -1;
}
for (loop = 0; loop < len; loop++) {
- if ((*cur_slot)->number == rtable->slots[loop].slot &&
- (*cur_slot)->bus == rtable->slots[loop].bus) {
- (*cur_slot)->device = PCI_SLOT(rtable->slots[loop].devfn);
+ if (cur_slot->number == rtable->slots[loop].slot &&
+ cur_slot->bus == rtable->slots[loop].bus) {
+ cur_slot->device = PCI_SLOT(rtable->slots[loop].devfn);
for (i = 0; i < 4; i++)
- (*cur_slot)->irq[i] = IO_APIC_get_PCI_irq_vector((int) (*cur_slot)->bus,
- (int) (*cur_slot)->device, i);
-
- debug("(*cur_slot)->irq[0] = %x\n",
- (*cur_slot)->irq[0]);
- debug("(*cur_slot)->irq[1] = %x\n",
- (*cur_slot)->irq[1]);
- debug("(*cur_slot)->irq[2] = %x\n",
- (*cur_slot)->irq[2]);
- debug("(*cur_slot)->irq[3] = %x\n",
- (*cur_slot)->irq[3]);
+ cur_slot->irq[i] = IO_APIC_get_PCI_irq_vector((int) cur_slot->bus,
+ (int) cur_slot->device, i);
+
+ debug("cur_slot->irq[0] = %x\n",
+ cur_slot->irq[0]);
+ debug("cur_slot->irq[1] = %x\n",
+ cur_slot->irq[1]);
+ debug("cur_slot->irq[2] = %x\n",
+ cur_slot->irq[2]);
+ debug("cur_slot->irq[3] = %x\n",
+ cur_slot->irq[3]);
debug("rtable->exclusive_irqs = %x\n",
rtable->exclusive_irqs);
diff --git a/drivers/pci/hotplug/ibmphp_ebda.c b/drivers/pci/hotplug/ibmphp_ebda.c
index 7fb75401ad8a..5c7821ad9c2c 100644
--- a/drivers/pci/hotplug/ibmphp_ebda.c
+++ b/drivers/pci/hotplug/ibmphp_ebda.c
@@ -880,7 +880,7 @@ static int __init ebda_rsrc_controller(void)
if (rc)
goto error;
- rc = ibmphp_init_devno(&tmp_slot);
+ rc = ibmphp_init_devno(tmp_slot);
if (rc)
goto error;
tmp_slot->hotplug_slot.ops = &ibmphp_hotplug_slot_ops;
--
2.29.0.rc1.dirty
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] PCI: ibmphp: Adjusting the parameter list of a function
2022-08-14 15:19 [PATCH] PCI: ibmphp: Adjusting the parameter list of a function Abd-Alrhman Masalkhi
@ 2022-08-18 23:07 ` Bjorn Helgaas
2022-08-19 12:38 ` [PATCH v2] PCI: ibmphp: Convert from double pointer to single pointer parameter Abd-Alrhman Masalkhi
0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Helgaas @ 2022-08-18 23:07 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi
Cc: bhelgaas, kw, os.vaslot, makvihas, linux-pci, linux-kernel
On Sun, Aug 14, 2022 at 05:19:56PM +0200, Abd-Alrhman Masalkhi wrote:
> ibmphp_init_devno() takes a struct slot **, and it should be changed
> to take struct slot * instead.
The commit log needs to explain *why* it should be changed from
"struct slot **" to "struct slot *".
And the subject line should give a clue about that, too.
Both subject and commit log should be "imperative," e.g., they should
read as if you were giving a command. "PCI: ibmphp: Convert ..."
This commit log says what *should* happen. It should read as though
it's a command carried out by the commit: "Convert 'struct slot **' to
'struct slot *' because ..."
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> drivers/pci/hotplug/TODO | 3 ---
> drivers/pci/hotplug/ibmphp.h | 3 ++-
> drivers/pci/hotplug/ibmphp_core.c | 30 +++++++++++++++---------------
> drivers/pci/hotplug/ibmphp_ebda.c | 2 +-
> 4 files changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pci/hotplug/TODO b/drivers/pci/hotplug/TODO
> index 88f217c82b4f..63a9fed407ed 100644
> --- a/drivers/pci/hotplug/TODO
> +++ b/drivers/pci/hotplug/TODO
> @@ -30,9 +30,6 @@ ibmphp:
> or ibmphp should store a pointer to its bus in struct slot. Probably the
> former.
>
> -* ibmphp_init_devno() takes a struct slot **, it could instead take a
> - struct slot *.
> -
> * The return value of pci_hp_register() is not checked.
>
> * The various slot data structures are difficult to follow and need to be
> diff --git a/drivers/pci/hotplug/ibmphp.h b/drivers/pci/hotplug/ibmphp.h
> index 0399c60d2ec1..148a421eb262 100644
> --- a/drivers/pci/hotplug/ibmphp.h
> +++ b/drivers/pci/hotplug/ibmphp.h
> @@ -734,7 +734,8 @@ struct controller {
>
> /* Functions */
>
> -int ibmphp_init_devno(struct slot **); /* This function is called from EBDA, so we need it not be static */
> +/* ibmphp_init_devno() is called from EBDA, so we need it not be static */
> +int ibmphp_init_devno(struct slot *cur_slot);
> int ibmphp_do_disable_slot(struct slot *slot_cur);
> int ibmphp_update_slot_info(struct slot *); /* This function is called from HPC, so we need it to not be static */
> int ibmphp_configure_card(struct pci_func *, u8);
> diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
> index 197997e264a2..107752b11f2c 100644
> --- a/drivers/pci/hotplug/ibmphp_core.c
> +++ b/drivers/pci/hotplug/ibmphp_core.c
> @@ -109,7 +109,7 @@ static int __init get_max_slots(void)
> * Parameters: struct slot
> * Returns 0 or errors
> */
> -int ibmphp_init_devno(struct slot **cur_slot)
> +int ibmphp_init_devno(struct slot *cur_slot)
> {
> struct irq_routing_table *rtable;
> int len;
> @@ -130,21 +130,21 @@ int ibmphp_init_devno(struct slot **cur_slot)
> return -1;
> }
> for (loop = 0; loop < len; loop++) {
> - if ((*cur_slot)->number == rtable->slots[loop].slot &&
> - (*cur_slot)->bus == rtable->slots[loop].bus) {
> - (*cur_slot)->device = PCI_SLOT(rtable->slots[loop].devfn);
> + if (cur_slot->number == rtable->slots[loop].slot &&
> + cur_slot->bus == rtable->slots[loop].bus) {
> + cur_slot->device = PCI_SLOT(rtable->slots[loop].devfn);
> for (i = 0; i < 4; i++)
> - (*cur_slot)->irq[i] = IO_APIC_get_PCI_irq_vector((int) (*cur_slot)->bus,
> - (int) (*cur_slot)->device, i);
> -
> - debug("(*cur_slot)->irq[0] = %x\n",
> - (*cur_slot)->irq[0]);
> - debug("(*cur_slot)->irq[1] = %x\n",
> - (*cur_slot)->irq[1]);
> - debug("(*cur_slot)->irq[2] = %x\n",
> - (*cur_slot)->irq[2]);
> - debug("(*cur_slot)->irq[3] = %x\n",
> - (*cur_slot)->irq[3]);
> + cur_slot->irq[i] = IO_APIC_get_PCI_irq_vector((int) cur_slot->bus,
> + (int) cur_slot->device, i);
> +
> + debug("cur_slot->irq[0] = %x\n",
> + cur_slot->irq[0]);
> + debug("cur_slot->irq[1] = %x\n",
> + cur_slot->irq[1]);
> + debug("cur_slot->irq[2] = %x\n",
> + cur_slot->irq[2]);
> + debug("cur_slot->irq[3] = %x\n",
> + cur_slot->irq[3]);
>
> debug("rtable->exclusive_irqs = %x\n",
> rtable->exclusive_irqs);
> diff --git a/drivers/pci/hotplug/ibmphp_ebda.c b/drivers/pci/hotplug/ibmphp_ebda.c
> index 7fb75401ad8a..5c7821ad9c2c 100644
> --- a/drivers/pci/hotplug/ibmphp_ebda.c
> +++ b/drivers/pci/hotplug/ibmphp_ebda.c
> @@ -880,7 +880,7 @@ static int __init ebda_rsrc_controller(void)
> if (rc)
> goto error;
>
> - rc = ibmphp_init_devno(&tmp_slot);
> + rc = ibmphp_init_devno(tmp_slot);
> if (rc)
> goto error;
> tmp_slot->hotplug_slot.ops = &ibmphp_hotplug_slot_ops;
> --
> 2.29.0.rc1.dirty
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2] PCI: ibmphp: Convert from double pointer to single pointer parameter
2022-08-18 23:07 ` Bjorn Helgaas
@ 2022-08-19 12:38 ` Abd-Alrhman Masalkhi
0 siblings, 0 replies; 3+ messages in thread
From: Abd-Alrhman Masalkhi @ 2022-08-19 12:38 UTC (permalink / raw)
To: bhelgaas, helgaas
Cc: kw, os.vaslot, makvihas, linux-pci, linux-kernel, Abd-Alrhman Masalkhi
Change the parameter of type 'struct slot **' to 'struct slot *' of
ibmphp_init_devno() since using a double pointer is unnecessary in
this case and converting it to a single pointer parameter is better
for performance and readability.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
drivers/pci/hotplug/TODO | 3 ---
drivers/pci/hotplug/ibmphp.h | 2 +-
drivers/pci/hotplug/ibmphp_core.c | 30 +++++++++++++++---------------
drivers/pci/hotplug/ibmphp_ebda.c | 2 +-
4 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/hotplug/TODO b/drivers/pci/hotplug/TODO
index 88f217c82b4f..63a9fed407ed 100644
--- a/drivers/pci/hotplug/TODO
+++ b/drivers/pci/hotplug/TODO
@@ -30,9 +30,6 @@ ibmphp:
or ibmphp should store a pointer to its bus in struct slot. Probably the
former.
-* ibmphp_init_devno() takes a struct slot **, it could instead take a
- struct slot *.
-
* The return value of pci_hp_register() is not checked.
* The various slot data structures are difficult to follow and need to be
diff --git a/drivers/pci/hotplug/ibmphp.h b/drivers/pci/hotplug/ibmphp.h
index 0399c60d2ec1..28fe92cf544a 100644
--- a/drivers/pci/hotplug/ibmphp.h
+++ b/drivers/pci/hotplug/ibmphp.h
@@ -734,7 +734,7 @@ struct controller {
/* Functions */
-int ibmphp_init_devno(struct slot **); /* This function is called from EBDA, so we need it not be static */
+int ibmphp_init_devno(struct slot *cur_slot); /* This function is called from EBDA, so we need it not be static */
int ibmphp_do_disable_slot(struct slot *slot_cur);
int ibmphp_update_slot_info(struct slot *); /* This function is called from HPC, so we need it to not be static */
int ibmphp_configure_card(struct pci_func *, u8);
diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
index 197997e264a2..107752b11f2c 100644
--- a/drivers/pci/hotplug/ibmphp_core.c
+++ b/drivers/pci/hotplug/ibmphp_core.c
@@ -109,7 +109,7 @@ static int __init get_max_slots(void)
* Parameters: struct slot
* Returns 0 or errors
*/
-int ibmphp_init_devno(struct slot **cur_slot)
+int ibmphp_init_devno(struct slot *cur_slot)
{
struct irq_routing_table *rtable;
int len;
@@ -130,21 +130,21 @@ int ibmphp_init_devno(struct slot **cur_slot)
return -1;
}
for (loop = 0; loop < len; loop++) {
- if ((*cur_slot)->number == rtable->slots[loop].slot &&
- (*cur_slot)->bus == rtable->slots[loop].bus) {
- (*cur_slot)->device = PCI_SLOT(rtable->slots[loop].devfn);
+ if (cur_slot->number == rtable->slots[loop].slot &&
+ cur_slot->bus == rtable->slots[loop].bus) {
+ cur_slot->device = PCI_SLOT(rtable->slots[loop].devfn);
for (i = 0; i < 4; i++)
- (*cur_slot)->irq[i] = IO_APIC_get_PCI_irq_vector((int) (*cur_slot)->bus,
- (int) (*cur_slot)->device, i);
-
- debug("(*cur_slot)->irq[0] = %x\n",
- (*cur_slot)->irq[0]);
- debug("(*cur_slot)->irq[1] = %x\n",
- (*cur_slot)->irq[1]);
- debug("(*cur_slot)->irq[2] = %x\n",
- (*cur_slot)->irq[2]);
- debug("(*cur_slot)->irq[3] = %x\n",
- (*cur_slot)->irq[3]);
+ cur_slot->irq[i] = IO_APIC_get_PCI_irq_vector((int) cur_slot->bus,
+ (int) cur_slot->device, i);
+
+ debug("cur_slot->irq[0] = %x\n",
+ cur_slot->irq[0]);
+ debug("cur_slot->irq[1] = %x\n",
+ cur_slot->irq[1]);
+ debug("cur_slot->irq[2] = %x\n",
+ cur_slot->irq[2]);
+ debug("cur_slot->irq[3] = %x\n",
+ cur_slot->irq[3]);
debug("rtable->exclusive_irqs = %x\n",
rtable->exclusive_irqs);
diff --git a/drivers/pci/hotplug/ibmphp_ebda.c b/drivers/pci/hotplug/ibmphp_ebda.c
index 7fb75401ad8a..5c7821ad9c2c 100644
--- a/drivers/pci/hotplug/ibmphp_ebda.c
+++ b/drivers/pci/hotplug/ibmphp_ebda.c
@@ -880,7 +880,7 @@ static int __init ebda_rsrc_controller(void)
if (rc)
goto error;
- rc = ibmphp_init_devno(&tmp_slot);
+ rc = ibmphp_init_devno(tmp_slot);
if (rc)
goto error;
tmp_slot->hotplug_slot.ops = &ibmphp_hotplug_slot_ops;
--
2.29.0.rc1.dirty
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-19 12:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-14 15:19 [PATCH] PCI: ibmphp: Adjusting the parameter list of a function Abd-Alrhman Masalkhi
2022-08-18 23:07 ` Bjorn Helgaas
2022-08-19 12:38 ` [PATCH v2] PCI: ibmphp: Convert from double pointer to single pointer parameter Abd-Alrhman Masalkhi
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).