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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.