From: Rolf Eike Beer <eike-kernel@sf-tec.de>
To: Alex Chiang <achiang@hp.com>
Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
jbarnes@virtuousgeek.org, kristen.c.accardi@intel.com,
kaneshige.kenji@jp.fujitsu.com
Subject: Re: [PATCH 04/13] PCI: acpiphp: remove 'name' parameter
Date: Sun, 17 Aug 2008 10:59:43 +0200 [thread overview]
Message-ID: <200808171059.50802.eike-kernel@sf-tec.de> (raw)
In-Reply-To: <20080817001635.5461.39872.stgit@blender.achiang>
[-- Attachment #1: Type: text/plain, Size: 8165 bytes --]
Alex Chiang wrote:
> We do not need to manage our own name parameter, especially since
> the PCI core can change it on our behalf, in the case of duplicate
> slot names.
>
> Remove 'name' from acpiphp's version of struct slot.
>
> Cc: jbarnes@virtuousgeek.org
> Cc: kristen.c.accardi@intel.com
> Cc: kaneshige.kenji@jp.fujitsu.com
> Signed-off-by: Alex Chiang <achiang@hp.com>
> ---
>
> drivers/pci/hotplug/acpiphp.h | 9 +++++----
> drivers/pci/hotplug/acpiphp_core.c | 36
> +++++++++++++++++++----------------- 2 files changed, 24 insertions(+), 21
> deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
> index 5a58b07..f9e244d 100644
> --- a/drivers/pci/hotplug/acpiphp.h
> +++ b/drivers/pci/hotplug/acpiphp.h
> @@ -50,9 +50,6 @@
> #define info(format, arg...) printk(KERN_INFO "%s: " format, MY_NAME , ##
> arg) #define warn(format, arg...) printk(KERN_WARNING "%s: " format,
> MY_NAME , ## arg)
>
> -/* name size which is used for entries in pcihpfs */
> -#define SLOT_NAME_SIZE 20 /* {_SUN} */
> -
> struct acpiphp_bridge;
> struct acpiphp_slot;
>
> @@ -63,9 +60,13 @@ struct slot {
> struct hotplug_slot *hotplug_slot;
> struct acpiphp_slot *acpi_slot;
> struct hotplug_slot_info info;
> - char name[SLOT_NAME_SIZE];
> };
>
> +static inline const char *slot_name(struct slot *slot)
> +{
> + return hotplug_slot_name(slot->hotplug_slot);
> +}
> +
> /*
> * struct acpiphp_bridge - PCI bridge information
> *
I don't see a point in this function. Why not call hotplug_slot_name()
directly?
> diff --git a/drivers/pci/hotplug/acpiphp_core.c
> b/drivers/pci/hotplug/acpiphp_core.c index e984176..687cac3 100644
> --- a/drivers/pci/hotplug/acpiphp_core.c
> +++ b/drivers/pci/hotplug/acpiphp_core.c
> @@ -44,6 +44,9 @@
>
> #define MY_NAME "acpiphp"
>
> +/* name size which is used for entries in pcihpfs */
> +#define SLOT_NAME_SIZE 20 /* {_SUN} */
> +
> static int debug;
> int acpiphp_debug;
>
> @@ -84,7 +87,6 @@ static struct hotplug_slot_ops acpi_hotplug_slot_ops = {
> .get_adapter_status = get_adapter_status,
> };
>
> -
> /**
> * acpiphp_register_attention - set attention LED callback
> * @info: must be completely filled with LED callbacks
Fuzz.
> @@ -92,7 +94,7 @@ static struct hotplug_slot_ops acpi_hotplug_slot_ops = {
> * Description: This is used to register a hardware specific ACPI
> * driver that manipulates the attention LED. All the fields in
> * info must be set.
> - */
> + **/
> int acpiphp_register_attention(struct acpiphp_attention_info *info)
> {
> int retval = -EINVAL;
Fuzz. Make the patch look bigger than it actually is. But that's just a note,
Jesse will have to judge if this is acceptable.
> @@ -113,7 +115,7 @@ int acpiphp_register_attention(struct
> acpiphp_attention_info *info) * Description: This is used to un-register a
> hardware specific acpi * driver that manipulates the attention LED. The
> pointer to the * info struct must be the same as the one used to set it.
> - */
> + **/
> int acpiphp_unregister_attention(struct acpiphp_attention_info *info)
> {
> int retval = -EINVAL;
> @@ -136,7 +138,7 @@ static int enable_slot(struct hotplug_slot
> *hotplug_slot) {
> struct slot *slot = hotplug_slot->private;
>
> - dbg("%s - physical_slot = %s\n", __func__, hotplug_slot->name);
> + dbg("%s - physical_slot = %s\n", __func__, slot_name(slot));
>
> /* enable the specified slot */
> return acpiphp_enable_slot(slot->acpi_slot);
> @@ -154,7 +156,7 @@ static int disable_slot(struct hotplug_slot
> *hotplug_slot) struct slot *slot = hotplug_slot->private;
> int retval;
>
> - dbg("%s - physical_slot = %s\n", __func__, hotplug_slot->name);
> + dbg("%s - physical_slot = %s\n", __func__, slot_name(slot));
>
> /* disable the specified slot */
> retval = acpiphp_disable_slot(slot->acpi_slot);
> @@ -177,7 +179,7 @@ static int disable_slot(struct hotplug_slot
> *hotplug_slot) {
> int retval = -ENODEV;
>
> - dbg("%s - physical_slot = %s\n", __func__, hotplug_slot->name);
> + dbg("%s - physical_slot = %s\n", __func__,
> hotplug_slot_name(hotplug_slot));
>
> if (attention_info && try_module_get(attention_info->owner)) {
> retval = attention_info->set_attn(hotplug_slot, status);
> @@ -200,7 +202,7 @@ static int get_power_status(struct hotplug_slot
> *hotplug_slot, u8 *value) {
> struct slot *slot = hotplug_slot->private;
>
> - dbg("%s - physical_slot = %s\n", __func__, hotplug_slot->name);
> + dbg("%s - physical_slot = %s\n", __func__, slot_name(slot));
>
> *value = acpiphp_get_power_status(slot->acpi_slot);
>
> @@ -222,7 +224,7 @@ static int get_attention_status(struct hotplug_slot
> *hotplug_slot, u8 *value) {
> int retval = -EINVAL;
>
> - dbg("%s - physical_slot = %s\n", __func__, hotplug_slot->name);
> + dbg("%s - physical_slot = %s\n", __func__,
> hotplug_slot_name(hotplug_slot));
>
> if (attention_info && try_module_get(attention_info->owner)) {
> retval = attention_info->get_attn(hotplug_slot, value);
> @@ -245,7 +247,7 @@ static int get_latch_status(struct hotplug_slot
> *hotplug_slot, u8 *value) {
> struct slot *slot = hotplug_slot->private;
>
> - dbg("%s - physical_slot = %s\n", __func__, hotplug_slot->name);
> + dbg("%s - physical_slot = %s\n", __func__, slot_name(slot));
>
> *value = acpiphp_get_latch_status(slot->acpi_slot);
>
> @@ -265,7 +267,7 @@ static int get_adapter_status(struct hotplug_slot
> *hotplug_slot, u8 *value) {
> struct slot *slot = hotplug_slot->private;
>
> - dbg("%s - physical_slot = %s\n", __func__, hotplug_slot->name);
> + dbg("%s - physical_slot = %s\n", __func__, slot_name(slot));
>
> *value = acpiphp_get_adapter_status(slot->acpi_slot);
>
> @@ -299,7 +301,7 @@ static void release_slot(struct hotplug_slot
> *hotplug_slot) {
> struct slot *slot = hotplug_slot->private;
>
> - dbg("%s - physical_slot = %s\n", __func__, hotplug_slot->name);
> + dbg("%s - physical_slot = %s\n", __func__, slot_name(slot));
>
> kfree(slot->hotplug_slot);
> kfree(slot);
> @@ -310,6 +312,7 @@ int acpiphp_register_hotplug_slot(struct acpiphp_slot
> *acpiphp_slot) {
> struct slot *slot;
> int retval = -ENOMEM;
> + char name[SLOT_NAME_SIZE];
>
> slot = kzalloc(sizeof(*slot), GFP_KERNEL);
> if (!slot)
> @@ -321,8 +324,6 @@ int acpiphp_register_hotplug_slot(struct acpiphp_slot
> *acpiphp_slot)
>
> slot->hotplug_slot->info = &slot->info;
>
> - slot->hotplug_slot->name = slot->name;
> -
> slot->hotplug_slot->private = slot;
> slot->hotplug_slot->release = &release_slot;
> slot->hotplug_slot->ops = &acpi_hotplug_slot_ops;
> @@ -336,12 +337,13 @@ int acpiphp_register_hotplug_slot(struct acpiphp_slot
> *acpiphp_slot) slot->hotplug_slot->info->cur_bus_speed = PCI_SPEED_UNKNOWN;
>
> acpiphp_slot->slot = slot;
> - snprintf(slot->name, sizeof(slot->name), "%u", slot->acpi_slot->sun);
> + memset(name, 0, SLOT_NAME_SIZE);
> + snprintf(name, SLOT_NAME_SIZE, "%u", slot->acpi_slot->sun);
The memset() is not needed at all. And the sizeof is IMHO a good idea anyway
as it allows to get rid of the define.
> retval = pci_hp_register(slot->hotplug_slot,
> acpiphp_slot->bridge->pci_bus,
> acpiphp_slot->device,
> - slot->name);
> + name);
> if (retval == -EBUSY)
> goto error_hpslot;
> if (retval) {
> @@ -349,7 +351,7 @@ int acpiphp_register_hotplug_slot(struct acpiphp_slot
> *acpiphp_slot) goto error_hpslot;
> }
>
> - info("Slot [%s] registered\n", slot->hotplug_slot->name);
> + info("Slot [%s] registered\n", slot_name(slot));
>
> return 0;
> error_hpslot:
> @@ -366,7 +368,7 @@ void acpiphp_unregister_hotplug_slot(struct
> acpiphp_slot *acpiphp_slot) struct slot *slot = acpiphp_slot->slot;
> int retval = 0;
>
> - info ("Slot [%s] unregistered\n", slot->hotplug_slot->name);
> + info ("Slot [%s] unregistered\n", slot_name(slot));
>
> retval = pci_hp_deregister(slot->hotplug_slot);
> if (retval)
Greetings,
Eike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
next prev parent reply other threads:[~2008-08-17 9:06 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-17 0:14 [PATCH 00/13] PCI: let the core manage slot names Alex Chiang
2008-08-17 0:15 ` [PATCH 01/13] PCI Hotplug core: add 'name' param pci_hp_register interface Alex Chiang
2008-08-17 0:16 ` [PATCH 02/13] PCI: prevent duplicate slot names Alex Chiang
2008-08-21 10:24 ` Kenji Kaneshige
2008-09-09 9:04 ` Alex Chiang
2008-09-09 10:00 ` [PATCH v2 00/13] PCI: let the core manage " Alex Chiang
2008-09-09 10:00 ` [PATCH v2 01/13] PCI Hotplug core: add 'name' param pci_hp_register interface Alex Chiang
2008-09-09 12:05 ` Matthew Wilcox
2008-09-09 17:18 ` Alex Chiang
2008-09-09 10:00 ` [PATCH v2 02/13] PCI: prevent duplicate slot names Alex Chiang
2008-09-09 13:07 ` Matthew Wilcox
2008-09-22 21:38 ` Alex Chiang
2008-09-22 22:42 ` Matthew Wilcox
2008-09-10 14:58 ` Rolf Eike Beer
2008-09-22 21:40 ` Alex Chiang
2008-09-11 2:47 ` Kenji Kaneshige
2008-09-11 10:37 ` Alex Chiang
2008-09-23 0:05 ` Alex Chiang
2008-09-09 10:00 ` [PATCH v2 03/13] PCI, PCI Hotplug: introduce slot_name helpers Alex Chiang
2008-09-09 14:30 ` Matthew Wilcox
2008-09-09 10:00 ` [PATCH v2 04/13] PCI: acpiphp: remove 'name' parameter Alex Chiang
2008-09-09 14:38 ` Matthew Wilcox
2008-09-23 1:16 ` Alex Chiang
2008-09-09 10:00 ` [PATCH v2 05/13] PCI: cpci_hotplug: stop managing hotplug_slot->name Alex Chiang
2008-09-09 15:04 ` Matthew Wilcox
2008-09-09 21:11 ` Scott Murray
2008-09-09 10:00 ` [PATCH v2 06/13] PCI: cpqphp: " Alex Chiang
2008-09-09 15:08 ` Matthew Wilcox
2008-09-23 1:20 ` Alex Chiang
2008-09-09 10:00 ` [PATCH v2 07/13] PCI: fakephp: remove 'name' parameter Alex Chiang
2008-09-09 10:00 ` [PATCH v2 08/13] PCI: ibmphp: stop managing hotplug_slot->name Alex Chiang
2008-09-09 10:00 ` [PATCH v2 09/13] PCI: pciehp: remove 'name' parameter Alex Chiang
2008-09-09 10:00 ` [PATCH v2 10/13] PCI: rpaphp: stop managing hotplug_slot->name Alex Chiang
2008-09-09 10:00 ` [PATCH v2 11/13] PCI: SGI Hotplug: stop managing bss_hotplug_slot->name Alex Chiang
2008-09-09 10:01 ` [PATCH v2 12/13] PCI: shcphp: remove 'name' parameter Alex Chiang
2008-09-09 10:01 ` [PATCH v2 13/13] PCI: Hotplug core: remove 'name' Alex Chiang
2008-09-11 2:43 ` [PATCH 02/13] PCI: prevent duplicate slot names Kenji Kaneshige
2008-08-17 0:16 ` [PATCH 03/13] PCI, PCI Hotplug: introduce slot_name helpers Alex Chiang
2008-08-17 0:16 ` [PATCH 04/13] PCI: acpiphp: remove 'name' parameter Alex Chiang
2008-08-17 8:59 ` Rolf Eike Beer [this message]
2008-08-19 18:39 ` Alex Chiang
2008-08-19 21:01 ` Rolf Eike Beer
2008-08-19 21:26 ` Alex Chiang
2008-08-19 21:40 ` Jesse Barnes
2008-08-19 23:50 ` Alex Chiang
2008-08-20 2:25 ` Kenji Kaneshige
2008-08-17 0:16 ` [PATCH 05/13] PCI: cpci_hotplug: stop managing hotplug_slot->name Alex Chiang
2008-08-17 0:16 ` [PATCH 06/13] PCI: cpqphp: " Alex Chiang
2008-08-17 0:17 ` [PATCH 07/13] PCI: fakephp: remove 'name' parameter Alex Chiang
2008-08-17 0:17 ` [PATCH 08/13] PCI: ibmphp: stop managing hotplug_slot->name Alex Chiang
2008-08-17 0:17 ` [PATCH 09/13] PCI: pciehp: remove 'name' parameter Alex Chiang
2008-08-17 0:17 ` [PATCH 10/13] PCI: rpaphp: stop managing hotplug_slot->name Alex Chiang
2008-08-17 0:17 ` [PATCH 11/13] PCI: SGI Hotplug: stop managing bss_hotplug_slot->name Alex Chiang
2008-08-17 0:17 ` [PATCH 12/13] PCI: shcphp: remove 'name' parameter Alex Chiang
2008-08-17 0:17 ` [PATCH 13/13] PCI: Hotplug core: remove 'name' Alex Chiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200808171059.50802.eike-kernel@sf-tec.de \
--to=eike-kernel@sf-tec.de \
--cc=achiang@hp.com \
--cc=jbarnes@virtuousgeek.org \
--cc=kaneshige.kenji@jp.fujitsu.com \
--cc=kristen.c.accardi@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox