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@xxxxxxxxxxxxxxxx
> Cc: kristen.c.accardi@xxxxxxxxx
> Cc: kaneshige.kenji@xxxxxxxxxxxxxx
> Signed-off-by: Alex Chiang <achiang@xxxxxx>
> ---
>
> 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:
signature.asc
Description: This is a digitally signed message part.
