* [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c @ 2013-05-14 13:00 Jiang Liu 2013-05-14 13:00 ` [PATCH v1 2/2] ACPI, pci_root: use acpi_handle_print() and pr_xxx() to print messages Jiang Liu 2013-05-28 22:37 ` [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c Bjorn Helgaas 0 siblings, 2 replies; 12+ messages in thread From: Jiang Liu @ 2013-05-14 13:00 UTC (permalink / raw) To: Rafael J . Wysocki, Bjorn Helgaas, Yinghai Lu Cc: Jiang Liu, Gu Zheng, Toshi Kani, Myron Stowe, Yijing Wang, Jiang Liu, linux-pci, linux-kernel, Len Brown, linux-acpi Now the global list acpi_pci_roots pci_root.c is useless, remove it. Signed-off-by: Jiang Liu <jiang.liu@huawei.com> Cc: Len Brown <lenb@kernel.org> Cc: "Rafael J. Wysocki" <rjw@sisk.pl> Cc: linux-acpi@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/acpi/pci_root.c | 25 +++---------------------- include/acpi/acpi_bus.h | 1 - 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index b80e06e..91ddfd6 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -65,10 +65,6 @@ static struct acpi_scan_handler pci_root_handler = { .detach = acpi_pci_root_remove, }; -/* Lock to protect both acpi_pci_roots lists */ -static DEFINE_MUTEX(acpi_pci_root_lock); -static LIST_HEAD(acpi_pci_roots); - static DEFINE_MUTEX(osc_lock); /** @@ -423,7 +419,6 @@ static int acpi_pci_root_add(struct acpi_device *device, } } - INIT_LIST_HEAD(&root->node); root->device = device; root->segment = segment & 0xFFFF; strcpy(acpi_device_name(device), ACPI_PCI_ROOT_DEVICE_NAME); @@ -501,10 +496,6 @@ static int acpi_pci_root_add(struct acpi_device *device, * TBD: Need PCI interface for enumeration/configuration of roots. */ - mutex_lock(&acpi_pci_root_lock); - list_add_tail(&root->node, &acpi_pci_roots); - mutex_unlock(&acpi_pci_root_lock); - /* * Scan the Root Bridge * -------------------- @@ -518,7 +509,7 @@ static int acpi_pci_root_add(struct acpi_device *device, "Bus %04x:%02x not present in PCI namespace\n", root->segment, (unsigned int)root->secondary.start); result = -ENODEV; - goto out_del_root; + goto end; } /* ASPM setting */ @@ -538,20 +529,13 @@ static int acpi_pci_root_add(struct acpi_device *device, if (system_state != SYSTEM_BOOTING) { pcibios_resource_survey_bus(root->bus); pci_assign_unassigned_bus_resources(root->bus); - } - - /* need to after hot-added ioapic is registered */ - if (system_state != SYSTEM_BOOTING) + /* need to after hot-added ioapic is registered */ pci_enable_bridges(root->bus); + } pci_bus_add_devices(root->bus); return 1; -out_del_root: - mutex_lock(&acpi_pci_root_lock); - list_del(&root->node); - mutex_unlock(&acpi_pci_root_lock); - end: kfree(root); return result; @@ -568,9 +552,6 @@ static void acpi_pci_root_remove(struct acpi_device *device) pci_remove_root_bus(root->bus); - mutex_lock(&acpi_pci_root_lock); - list_del(&root->node); - mutex_unlock(&acpi_pci_root_lock); kfree(root); } diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 22ba56e..4eb9a88 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -447,7 +447,6 @@ int register_acpi_bus_type(struct acpi_bus_type *); int unregister_acpi_bus_type(struct acpi_bus_type *); struct acpi_pci_root { - struct list_head node; struct acpi_device * device; struct pci_bus *bus; u16 segment; -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 2/2] ACPI, pci_root: use acpi_handle_print() and pr_xxx() to print messages 2013-05-14 13:00 [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c Jiang Liu @ 2013-05-14 13:00 ` Jiang Liu 2013-05-30 17:33 ` Bjorn Helgaas 2013-05-28 22:37 ` [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c Bjorn Helgaas 1 sibling, 1 reply; 12+ messages in thread From: Jiang Liu @ 2013-05-14 13:00 UTC (permalink / raw) To: Rafael J . Wysocki, Bjorn Helgaas, Yinghai Lu Cc: Jiang Liu, Gu Zheng, Toshi Kani, Myron Stowe, Yijing Wang, Jiang Liu, linux-pci, linux-kernel, Len Brown, linux-acpi Use acpi_handle_print() and pr_xxx() to print messages in pci_root.c. Signed-off-by: Jiang Liu <jiang.liu@huawei.com> Cc: Len Brown <lenb@kernel.org> Cc: "Rafael J. Wysocki" <rjw@sisk.pl> Cc: linux-acpi@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/acpi/pci_root.c | 70 ++++++++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 38 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 91ddfd6..21dda5a 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -379,23 +379,24 @@ static int acpi_pci_root_add(struct acpi_device *device, struct acpi_pci_root *root; u32 flags, base_flags; bool is_osc_granted = false; + acpi_handle handle = device->handle; root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL); if (!root) return -ENOMEM; segment = 0; - status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL, + status = acpi_evaluate_integer(handle, METHOD_NAME__SEG, NULL, &segment); if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { - printk(KERN_ERR PREFIX "can't evaluate _SEG\n"); + acpi_handle_err(handle, "can't evaluate _SEG\n"); result = -ENODEV; goto end; } /* Check _CRS first, then _BBN. If no _BBN, default to zero. */ root->secondary.flags = IORESOURCE_BUS; - status = try_get_root_bridge_busnr(device->handle, &root->secondary); + status = try_get_root_bridge_busnr(handle, &root->secondary); if (ACPI_FAILURE(status)) { /* * We need both the start and end of the downstream bus range @@ -404,16 +405,16 @@ static int acpi_pci_root_add(struct acpi_device *device, * can do is assume [_BBN-0xFF] or [0-0xFF]. */ root->secondary.end = 0xFF; - printk(KERN_WARNING FW_BUG PREFIX - "no secondary bus range in _CRS\n"); - status = acpi_evaluate_integer(device->handle, METHOD_NAME__BBN, + acpi_handle_warn(handle, + FW_BUG "no secondary bus range in _CRS\n"); + status = acpi_evaluate_integer(handle, METHOD_NAME__BBN, NULL, &bus); if (ACPI_SUCCESS(status)) root->secondary.start = bus; else if (status == AE_NOT_FOUND) root->secondary.start = 0; else { - printk(KERN_ERR PREFIX "can't evaluate _BBN\n"); + acpi_handle_err(handle, "can't evaluate _BBN\n"); result = -ENODEV; goto end; } @@ -425,11 +426,11 @@ static int acpi_pci_root_add(struct acpi_device *device, strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); device->driver_data = root; - printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n", + pr_info(PREFIX "%s [%s] (domain %04x %pR)\n", acpi_device_name(device), acpi_device_bid(device), root->segment, &root->secondary); - root->mcfg_addr = acpi_pci_root_get_mcfg_addr(device->handle); + root->mcfg_addr = acpi_pci_root_get_mcfg_addr(handle); /* * All supported architectures that use ACPI have support for @@ -473,7 +474,7 @@ static int acpi_pci_root_add(struct acpi_device *device, dev_info(&device->dev, "Requesting ACPI _OSC control (0x%02x)\n", flags); - status = acpi_pci_osc_control_set(device->handle, &flags, + status = acpi_pci_osc_control_set(handle, &flags, OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL); if (ACPI_SUCCESS(status)) { is_osc_granted = true; @@ -505,9 +506,9 @@ static int acpi_pci_root_add(struct acpi_device *device, */ root->bus = pci_acpi_scan_root(root); if (!root->bus) { - printk(KERN_ERR PREFIX - "Bus %04x:%02x not present in PCI namespace\n", - root->segment, (unsigned int)root->secondary.start); + acpi_handle_err(handle, + "Bus %04x:%02x not present in PCI namespace\n", + root->segment, (unsigned int)root->secondary.start); result = -ENODEV; goto end; } @@ -517,8 +518,8 @@ static int acpi_pci_root_add(struct acpi_device *device, if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) pcie_clear_aspm(root->bus); } else { - pr_info("ACPI _OSC control for PCIe not granted, " - "disabling ASPM\n"); + acpi_handle_info(handle, + "ACPI _OSC control for PCIe not granted, disabling ASPM\n"); pcie_no_aspm(); } @@ -571,12 +572,13 @@ static void handle_root_bridge_insertion(acpi_handle handle) struct acpi_device *device; if (!acpi_bus_get_device(handle, &device)) { - printk(KERN_DEBUG "acpi device exists...\n"); + acpi_handle_printk(KERN_DEBUG, handle, + "acpi device exists...\n"); return; } if (acpi_bus_scan(handle)) - printk(KERN_ERR "cannot add bridge to acpi list\n"); + acpi_handle_err(handle, "cannot add bridge to acpi list\n"); } static void handle_root_bridge_removal(struct acpi_device *device) @@ -602,7 +604,6 @@ static void handle_root_bridge_removal(struct acpi_device *device) static void _handle_hotplug_event_root(struct work_struct *work) { struct acpi_pci_root *root; - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER }; struct acpi_hp_work *hp_work; acpi_handle handle; u32 type; @@ -613,13 +614,11 @@ static void _handle_hotplug_event_root(struct work_struct *work) root = acpi_pci_find_root(handle); - acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); - switch (type) { case ACPI_NOTIFY_BUS_CHECK: /* bus enumerate */ - printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__, - (char *)buffer.pointer); + acpi_handle_printk(KERN_DEBUG, handle, + "Bus check notify on %s\n", __func__); if (!root) handle_root_bridge_insertion(handle); @@ -627,27 +626,26 @@ static void _handle_hotplug_event_root(struct work_struct *work) case ACPI_NOTIFY_DEVICE_CHECK: /* device check */ - printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__, - (char *)buffer.pointer); + acpi_handle_printk(KERN_DEBUG, handle, + "Device check notify on %s\n", __func__); if (!root) handle_root_bridge_insertion(handle); break; case ACPI_NOTIFY_EJECT_REQUEST: /* request device eject */ - printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__, - (char *)buffer.pointer); + acpi_handle_printk(KERN_DEBUG, handle, + "Device eject notify on %s\n", __func__); if (root) handle_root_bridge_removal(root->device); break; default: - printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n", - type, (char *)buffer.pointer); + acpi_handle_warn(handle, + "notify_handler: unknown event type 0x%x\n", type); break; } kfree(hp_work); /* allocated in handle_hotplug_event_bridge */ - kfree(buffer.pointer); } static void handle_hotplug_event_root(acpi_handle handle, u32 type, @@ -661,9 +659,6 @@ static acpi_status __init find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) { acpi_status status; - char objname[64]; - struct acpi_buffer buffer = { .length = sizeof(objname), - .pointer = objname }; int *count = (int *)context; if (!acpi_is_root_bridge(handle)) @@ -671,16 +666,15 @@ find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) (*count)++; - acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); - status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY, handle_hotplug_event_root, NULL); if (ACPI_FAILURE(status)) - printk(KERN_DEBUG "acpi root: %s notify handler is not installed, exit status: %u\n", - objname, (unsigned int)status); + acpi_handle_printk(KERN_DEBUG, handle, + "notify handler is not installed, exit status: %u\n", + (unsigned int)status); else - printk(KERN_DEBUG "acpi root: %s notify handler is installed\n", - objname); + acpi_handle_printk(KERN_DEBUG, handle, + "notify handler is installed\n"); return AE_OK; } -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] ACPI, pci_root: use acpi_handle_print() and pr_xxx() to print messages 2013-05-14 13:00 ` [PATCH v1 2/2] ACPI, pci_root: use acpi_handle_print() and pr_xxx() to print messages Jiang Liu @ 2013-05-30 17:33 ` Bjorn Helgaas 2013-05-30 19:51 ` Rafael J. Wysocki 0 siblings, 1 reply; 12+ messages in thread From: Bjorn Helgaas @ 2013-05-30 17:33 UTC (permalink / raw) To: Jiang Liu Cc: Rafael J . Wysocki, Yinghai Lu, Jiang Liu, Gu Zheng, Toshi Kani, Myron Stowe, Yijing Wang, linux-pci, linux-kernel, Len Brown, linux-acpi On Tue, May 14, 2013 at 09:00:39PM +0800, Jiang Liu wrote: > Use acpi_handle_print() and pr_xxx() to print messages in pci_root.c. > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > Cc: Len Brown <lenb@kernel.org> > Cc: "Rafael J. Wysocki" <rjw@sisk.pl> > Cc: linux-acpi@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > drivers/acpi/pci_root.c | 70 ++++++++++++++++++++++--------------------------- > 1 file changed, 32 insertions(+), 38 deletions(-) > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index 91ddfd6..21dda5a 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -379,23 +379,24 @@ static int acpi_pci_root_add(struct acpi_device *device, > struct acpi_pci_root *root; > u32 flags, base_flags; > bool is_osc_granted = false; > + acpi_handle handle = device->handle; > > root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL); > if (!root) > return -ENOMEM; > > segment = 0; > - status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL, > + status = acpi_evaluate_integer(handle, METHOD_NAME__SEG, NULL, > &segment); > if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { > - printk(KERN_ERR PREFIX "can't evaluate _SEG\n"); > + acpi_handle_err(handle, "can't evaluate _SEG\n"); I previously acked this, but I noticed that we are making a mix of dev_printk() and acpi_handle_printk() here. The difference is like this: acpi PNP0A08:00: ACPI _OSC support notification failed, disabling PCIe ASPM ACPI: \_SB_.PCI0: ACPI _OSC support notification failed, disabling PCIe ASPM I'm not sure which direction we should go here, but I think we should choose one or the other and use it consistently. Personally, I think the internal DSDT names should be available *somewhere*, but not necessarily used in run-of-the-mill chit-chat. For that reason, I prefer dev_printk(), though I have the feeling that Rafael is moving toward eliminating the struct acpi_device, so he might prefer acpi_handle_printk(). Bjorn > result = -ENODEV; > goto end; > } > > /* Check _CRS first, then _BBN. If no _BBN, default to zero. */ > root->secondary.flags = IORESOURCE_BUS; > - status = try_get_root_bridge_busnr(device->handle, &root->secondary); > + status = try_get_root_bridge_busnr(handle, &root->secondary); > if (ACPI_FAILURE(status)) { > /* > * We need both the start and end of the downstream bus range > @@ -404,16 +405,16 @@ static int acpi_pci_root_add(struct acpi_device *device, > * can do is assume [_BBN-0xFF] or [0-0xFF]. > */ > root->secondary.end = 0xFF; > - printk(KERN_WARNING FW_BUG PREFIX > - "no secondary bus range in _CRS\n"); > - status = acpi_evaluate_integer(device->handle, METHOD_NAME__BBN, > + acpi_handle_warn(handle, > + FW_BUG "no secondary bus range in _CRS\n"); > + status = acpi_evaluate_integer(handle, METHOD_NAME__BBN, > NULL, &bus); > if (ACPI_SUCCESS(status)) > root->secondary.start = bus; > else if (status == AE_NOT_FOUND) > root->secondary.start = 0; > else { > - printk(KERN_ERR PREFIX "can't evaluate _BBN\n"); > + acpi_handle_err(handle, "can't evaluate _BBN\n"); > result = -ENODEV; > goto end; > } > @@ -425,11 +426,11 @@ static int acpi_pci_root_add(struct acpi_device *device, > strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); > device->driver_data = root; > > - printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n", > + pr_info(PREFIX "%s [%s] (domain %04x %pR)\n", > acpi_device_name(device), acpi_device_bid(device), > root->segment, &root->secondary); > > - root->mcfg_addr = acpi_pci_root_get_mcfg_addr(device->handle); > + root->mcfg_addr = acpi_pci_root_get_mcfg_addr(handle); > > /* > * All supported architectures that use ACPI have support for > @@ -473,7 +474,7 @@ static int acpi_pci_root_add(struct acpi_device *device, > dev_info(&device->dev, > "Requesting ACPI _OSC control (0x%02x)\n", flags); > > - status = acpi_pci_osc_control_set(device->handle, &flags, > + status = acpi_pci_osc_control_set(handle, &flags, > OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL); > if (ACPI_SUCCESS(status)) { > is_osc_granted = true; > @@ -505,9 +506,9 @@ static int acpi_pci_root_add(struct acpi_device *device, > */ > root->bus = pci_acpi_scan_root(root); > if (!root->bus) { > - printk(KERN_ERR PREFIX > - "Bus %04x:%02x not present in PCI namespace\n", > - root->segment, (unsigned int)root->secondary.start); > + acpi_handle_err(handle, > + "Bus %04x:%02x not present in PCI namespace\n", > + root->segment, (unsigned int)root->secondary.start); > result = -ENODEV; > goto end; > } > @@ -517,8 +518,8 @@ static int acpi_pci_root_add(struct acpi_device *device, > if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) > pcie_clear_aspm(root->bus); > } else { > - pr_info("ACPI _OSC control for PCIe not granted, " > - "disabling ASPM\n"); > + acpi_handle_info(handle, > + "ACPI _OSC control for PCIe not granted, disabling ASPM\n"); > pcie_no_aspm(); > } > > @@ -571,12 +572,13 @@ static void handle_root_bridge_insertion(acpi_handle handle) > struct acpi_device *device; > > if (!acpi_bus_get_device(handle, &device)) { > - printk(KERN_DEBUG "acpi device exists...\n"); > + acpi_handle_printk(KERN_DEBUG, handle, > + "acpi device exists...\n"); > return; > } > > if (acpi_bus_scan(handle)) > - printk(KERN_ERR "cannot add bridge to acpi list\n"); > + acpi_handle_err(handle, "cannot add bridge to acpi list\n"); > } > > static void handle_root_bridge_removal(struct acpi_device *device) > @@ -602,7 +604,6 @@ static void handle_root_bridge_removal(struct acpi_device *device) > static void _handle_hotplug_event_root(struct work_struct *work) > { > struct acpi_pci_root *root; > - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER }; > struct acpi_hp_work *hp_work; > acpi_handle handle; > u32 type; > @@ -613,13 +614,11 @@ static void _handle_hotplug_event_root(struct work_struct *work) > > root = acpi_pci_find_root(handle); > > - acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); > - > switch (type) { > case ACPI_NOTIFY_BUS_CHECK: > /* bus enumerate */ > - printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__, > - (char *)buffer.pointer); > + acpi_handle_printk(KERN_DEBUG, handle, > + "Bus check notify on %s\n", __func__); > if (!root) > handle_root_bridge_insertion(handle); > > @@ -627,27 +626,26 @@ static void _handle_hotplug_event_root(struct work_struct *work) > > case ACPI_NOTIFY_DEVICE_CHECK: > /* device check */ > - printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__, > - (char *)buffer.pointer); > + acpi_handle_printk(KERN_DEBUG, handle, > + "Device check notify on %s\n", __func__); > if (!root) > handle_root_bridge_insertion(handle); > break; > > case ACPI_NOTIFY_EJECT_REQUEST: > /* request device eject */ > - printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__, > - (char *)buffer.pointer); > + acpi_handle_printk(KERN_DEBUG, handle, > + "Device eject notify on %s\n", __func__); > if (root) > handle_root_bridge_removal(root->device); > break; > default: > - printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n", > - type, (char *)buffer.pointer); > + acpi_handle_warn(handle, > + "notify_handler: unknown event type 0x%x\n", type); > break; > } > > kfree(hp_work); /* allocated in handle_hotplug_event_bridge */ > - kfree(buffer.pointer); > } > > static void handle_hotplug_event_root(acpi_handle handle, u32 type, > @@ -661,9 +659,6 @@ static acpi_status __init > find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) > { > acpi_status status; > - char objname[64]; > - struct acpi_buffer buffer = { .length = sizeof(objname), > - .pointer = objname }; > int *count = (int *)context; > > if (!acpi_is_root_bridge(handle)) > @@ -671,16 +666,15 @@ find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) > > (*count)++; > > - acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); > - > status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY, > handle_hotplug_event_root, NULL); > if (ACPI_FAILURE(status)) > - printk(KERN_DEBUG "acpi root: %s notify handler is not installed, exit status: %u\n", > - objname, (unsigned int)status); > + acpi_handle_printk(KERN_DEBUG, handle, > + "notify handler is not installed, exit status: %u\n", > + (unsigned int)status); > else > - printk(KERN_DEBUG "acpi root: %s notify handler is installed\n", > - objname); > + acpi_handle_printk(KERN_DEBUG, handle, > + "notify handler is installed\n"); > > return AE_OK; > } > -- > 1.8.1.2 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] ACPI, pci_root: use acpi_handle_print() and pr_xxx() to print messages 2013-05-30 17:33 ` Bjorn Helgaas @ 2013-05-30 19:51 ` Rafael J. Wysocki 2013-05-31 16:20 ` Jiang Liu 0 siblings, 1 reply; 12+ messages in thread From: Rafael J. Wysocki @ 2013-05-30 19:51 UTC (permalink / raw) To: Bjorn Helgaas Cc: Jiang Liu, Yinghai Lu, Jiang Liu, Gu Zheng, Toshi Kani, Myron Stowe, Yijing Wang, linux-pci, linux-kernel, Len Brown, linux-acpi On Thursday, May 30, 2013 11:33:56 AM Bjorn Helgaas wrote: > On Tue, May 14, 2013 at 09:00:39PM +0800, Jiang Liu wrote: > > Use acpi_handle_print() and pr_xxx() to print messages in pci_root.c. > > > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > > Cc: Len Brown <lenb@kernel.org> > > Cc: "Rafael J. Wysocki" <rjw@sisk.pl> > > Cc: linux-acpi@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > --- > > drivers/acpi/pci_root.c | 70 ++++++++++++++++++++++--------------------------- > > 1 file changed, 32 insertions(+), 38 deletions(-) > > > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > > index 91ddfd6..21dda5a 100644 > > --- a/drivers/acpi/pci_root.c > > +++ b/drivers/acpi/pci_root.c > > @@ -379,23 +379,24 @@ static int acpi_pci_root_add(struct acpi_device *device, > > struct acpi_pci_root *root; > > u32 flags, base_flags; > > bool is_osc_granted = false; > > + acpi_handle handle = device->handle; > > > > root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL); > > if (!root) > > return -ENOMEM; > > > > segment = 0; > > - status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL, > > + status = acpi_evaluate_integer(handle, METHOD_NAME__SEG, NULL, > > &segment); > > if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { > > - printk(KERN_ERR PREFIX "can't evaluate _SEG\n"); > > + acpi_handle_err(handle, "can't evaluate _SEG\n"); > > I previously acked this, but I noticed that we are making a mix of > dev_printk() and acpi_handle_printk() here. The difference is like this: > > acpi PNP0A08:00: ACPI _OSC support notification failed, disabling PCIe ASPM > ACPI: \_SB_.PCI0: ACPI _OSC support notification failed, disabling PCIe ASPM > > I'm not sure which direction we should go here, but I think we should > choose one or the other and use it consistently. Personally, I think the > internal DSDT names should be available *somewhere*, but not necessarily > used in run-of-the-mill chit-chat. For that reason, I prefer > dev_printk(), though I have the feeling that Rafael is moving toward > eliminating the struct acpi_device, so he might prefer > acpi_handle_printk(). I really don't care that much, but I agree that they should be used consistently. Thanks, Rafael > > result = -ENODEV; > > goto end; > > } > > > > /* Check _CRS first, then _BBN. If no _BBN, default to zero. */ > > root->secondary.flags = IORESOURCE_BUS; > > - status = try_get_root_bridge_busnr(device->handle, &root->secondary); > > + status = try_get_root_bridge_busnr(handle, &root->secondary); > > if (ACPI_FAILURE(status)) { > > /* > > * We need both the start and end of the downstream bus range > > @@ -404,16 +405,16 @@ static int acpi_pci_root_add(struct acpi_device *device, > > * can do is assume [_BBN-0xFF] or [0-0xFF]. > > */ > > root->secondary.end = 0xFF; > > - printk(KERN_WARNING FW_BUG PREFIX > > - "no secondary bus range in _CRS\n"); > > - status = acpi_evaluate_integer(device->handle, METHOD_NAME__BBN, > > + acpi_handle_warn(handle, > > + FW_BUG "no secondary bus range in _CRS\n"); > > + status = acpi_evaluate_integer(handle, METHOD_NAME__BBN, > > NULL, &bus); > > if (ACPI_SUCCESS(status)) > > root->secondary.start = bus; > > else if (status == AE_NOT_FOUND) > > root->secondary.start = 0; > > else { > > - printk(KERN_ERR PREFIX "can't evaluate _BBN\n"); > > + acpi_handle_err(handle, "can't evaluate _BBN\n"); > > result = -ENODEV; > > goto end; > > } > > @@ -425,11 +426,11 @@ static int acpi_pci_root_add(struct acpi_device *device, > > strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); > > device->driver_data = root; > > > > - printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n", > > + pr_info(PREFIX "%s [%s] (domain %04x %pR)\n", > > acpi_device_name(device), acpi_device_bid(device), > > root->segment, &root->secondary); > > > > - root->mcfg_addr = acpi_pci_root_get_mcfg_addr(device->handle); > > + root->mcfg_addr = acpi_pci_root_get_mcfg_addr(handle); > > > > /* > > * All supported architectures that use ACPI have support for > > @@ -473,7 +474,7 @@ static int acpi_pci_root_add(struct acpi_device *device, > > dev_info(&device->dev, > > "Requesting ACPI _OSC control (0x%02x)\n", flags); > > > > - status = acpi_pci_osc_control_set(device->handle, &flags, > > + status = acpi_pci_osc_control_set(handle, &flags, > > OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL); > > if (ACPI_SUCCESS(status)) { > > is_osc_granted = true; > > @@ -505,9 +506,9 @@ static int acpi_pci_root_add(struct acpi_device *device, > > */ > > root->bus = pci_acpi_scan_root(root); > > if (!root->bus) { > > - printk(KERN_ERR PREFIX > > - "Bus %04x:%02x not present in PCI namespace\n", > > - root->segment, (unsigned int)root->secondary.start); > > + acpi_handle_err(handle, > > + "Bus %04x:%02x not present in PCI namespace\n", > > + root->segment, (unsigned int)root->secondary.start); > > result = -ENODEV; > > goto end; > > } > > @@ -517,8 +518,8 @@ static int acpi_pci_root_add(struct acpi_device *device, > > if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) > > pcie_clear_aspm(root->bus); > > } else { > > - pr_info("ACPI _OSC control for PCIe not granted, " > > - "disabling ASPM\n"); > > + acpi_handle_info(handle, > > + "ACPI _OSC control for PCIe not granted, disabling ASPM\n"); > > pcie_no_aspm(); > > } > > > > @@ -571,12 +572,13 @@ static void handle_root_bridge_insertion(acpi_handle handle) > > struct acpi_device *device; > > > > if (!acpi_bus_get_device(handle, &device)) { > > - printk(KERN_DEBUG "acpi device exists...\n"); > > + acpi_handle_printk(KERN_DEBUG, handle, > > + "acpi device exists...\n"); > > return; > > } > > > > if (acpi_bus_scan(handle)) > > - printk(KERN_ERR "cannot add bridge to acpi list\n"); > > + acpi_handle_err(handle, "cannot add bridge to acpi list\n"); > > } > > > > static void handle_root_bridge_removal(struct acpi_device *device) > > @@ -602,7 +604,6 @@ static void handle_root_bridge_removal(struct acpi_device *device) > > static void _handle_hotplug_event_root(struct work_struct *work) > > { > > struct acpi_pci_root *root; > > - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER }; > > struct acpi_hp_work *hp_work; > > acpi_handle handle; > > u32 type; > > @@ -613,13 +614,11 @@ static void _handle_hotplug_event_root(struct work_struct *work) > > > > root = acpi_pci_find_root(handle); > > > > - acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); > > - > > switch (type) { > > case ACPI_NOTIFY_BUS_CHECK: > > /* bus enumerate */ > > - printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__, > > - (char *)buffer.pointer); > > + acpi_handle_printk(KERN_DEBUG, handle, > > + "Bus check notify on %s\n", __func__); > > if (!root) > > handle_root_bridge_insertion(handle); > > > > @@ -627,27 +626,26 @@ static void _handle_hotplug_event_root(struct work_struct *work) > > > > case ACPI_NOTIFY_DEVICE_CHECK: > > /* device check */ > > - printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__, > > - (char *)buffer.pointer); > > + acpi_handle_printk(KERN_DEBUG, handle, > > + "Device check notify on %s\n", __func__); > > if (!root) > > handle_root_bridge_insertion(handle); > > break; > > > > case ACPI_NOTIFY_EJECT_REQUEST: > > /* request device eject */ > > - printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__, > > - (char *)buffer.pointer); > > + acpi_handle_printk(KERN_DEBUG, handle, > > + "Device eject notify on %s\n", __func__); > > if (root) > > handle_root_bridge_removal(root->device); > > break; > > default: > > - printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n", > > - type, (char *)buffer.pointer); > > + acpi_handle_warn(handle, > > + "notify_handler: unknown event type 0x%x\n", type); > > break; > > } > > > > kfree(hp_work); /* allocated in handle_hotplug_event_bridge */ > > - kfree(buffer.pointer); > > } > > > > static void handle_hotplug_event_root(acpi_handle handle, u32 type, > > @@ -661,9 +659,6 @@ static acpi_status __init > > find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) > > { > > acpi_status status; > > - char objname[64]; > > - struct acpi_buffer buffer = { .length = sizeof(objname), > > - .pointer = objname }; > > int *count = (int *)context; > > > > if (!acpi_is_root_bridge(handle)) > > @@ -671,16 +666,15 @@ find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) > > > > (*count)++; > > > > - acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); > > - > > status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY, > > handle_hotplug_event_root, NULL); > > if (ACPI_FAILURE(status)) > > - printk(KERN_DEBUG "acpi root: %s notify handler is not installed, exit status: %u\n", > > - objname, (unsigned int)status); > > + acpi_handle_printk(KERN_DEBUG, handle, > > + "notify handler is not installed, exit status: %u\n", > > + (unsigned int)status); > > else > > - printk(KERN_DEBUG "acpi root: %s notify handler is installed\n", > > - objname); > > + acpi_handle_printk(KERN_DEBUG, handle, > > + "notify handler is installed\n"); > > > > return AE_OK; > > } > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] ACPI, pci_root: use acpi_handle_print() and pr_xxx() to print messages 2013-05-30 19:51 ` Rafael J. Wysocki @ 2013-05-31 16:20 ` Jiang Liu 2013-05-31 19:18 ` Bjorn Helgaas 0 siblings, 1 reply; 12+ messages in thread From: Jiang Liu @ 2013-05-31 16:20 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Yinghai Lu, Jiang Liu, Gu Zheng, Toshi Kani, Myron Stowe, Yijing Wang, linux-pci, linux-kernel, Len Brown, linux-acpi, Bjorn Helgaas On Fri 31 May 2013 03:51:51 AM CST, Rafael J. Wysocki wrote: > On Thursday, May 30, 2013 11:33:56 AM Bjorn Helgaas wrote: >> On Tue, May 14, 2013 at 09:00:39PM +0800, Jiang Liu wrote: >>> Use acpi_handle_print() and pr_xxx() to print messages in pci_root.c. >>> >>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> >>> Cc: Len Brown <lenb@kernel.org> >>> Cc: "Rafael J. Wysocki" <rjw@sisk.pl> >>> Cc: linux-acpi@vger.kernel.org >>> Cc: linux-kernel@vger.kernel.org >>> --- >>> drivers/acpi/pci_root.c | 70 ++++++++++++++++++++++--------------------------- >>> 1 file changed, 32 insertions(+), 38 deletions(-) >>> >>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >>> index 91ddfd6..21dda5a 100644 >>> --- a/drivers/acpi/pci_root.c >>> +++ b/drivers/acpi/pci_root.c >>> @@ -379,23 +379,24 @@ static int acpi_pci_root_add(struct acpi_device *device, >>> struct acpi_pci_root *root; >>> u32 flags, base_flags; >>> bool is_osc_granted = false; >>> + acpi_handle handle = device->handle; >>> >>> root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL); >>> if (!root) >>> return -ENOMEM; >>> >>> segment = 0; >>> - status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL, >>> + status = acpi_evaluate_integer(handle, METHOD_NAME__SEG, NULL, >>> &segment); >>> if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { >>> - printk(KERN_ERR PREFIX "can't evaluate _SEG\n"); >>> + acpi_handle_err(handle, "can't evaluate _SEG\n"); >> >> I previously acked this, but I noticed that we are making a mix of >> dev_printk() and acpi_handle_printk() here. The difference is like this: >> >> acpi PNP0A08:00: ACPI _OSC support notification failed, disabling PCIe ASPM >> ACPI: \_SB_.PCI0: ACPI _OSC support notification failed, disabling PCIe ASPM >> >> I'm not sure which direction we should go here, but I think we should >> choose one or the other and use it consistently. Personally, I think the >> internal DSDT names should be available *somewhere*, but not necessarily >> used in run-of-the-mill chit-chat. For that reason, I prefer >> dev_printk(), though I have the feeling that Rafael is moving toward >> eliminating the struct acpi_device, so he might prefer >> acpi_handle_printk(). > > I really don't care that much, but I agree that they should be used > consistently. Hi Bjorn and Rafael, I will work on this tomorrow. In some cases, only handle is available, so we could only use acpi_handle_printk(). So I think the directly may be: use dev_printk() if possible, otherwise use acpi_handle_printk() instead. Is that the right way to go? Regards! Gerry > > Thanks, > Rafael > > >>> result = -ENODEV; >>> goto end; >>> } >>> >>> /* Check _CRS first, then _BBN. If no _BBN, default to zero. */ >>> root->secondary.flags = IORESOURCE_BUS; >>> - status = try_get_root_bridge_busnr(device->handle, &root->secondary); >>> + status = try_get_root_bridge_busnr(handle, &root->secondary); >>> if (ACPI_FAILURE(status)) { >>> /* >>> * We need both the start and end of the downstream bus range >>> @@ -404,16 +405,16 @@ static int acpi_pci_root_add(struct acpi_device *device, >>> * can do is assume [_BBN-0xFF] or [0-0xFF]. >>> */ >>> root->secondary.end = 0xFF; >>> - printk(KERN_WARNING FW_BUG PREFIX >>> - "no secondary bus range in _CRS\n"); >>> - status = acpi_evaluate_integer(device->handle, METHOD_NAME__BBN, >>> + acpi_handle_warn(handle, >>> + FW_BUG "no secondary bus range in _CRS\n"); >>> + status = acpi_evaluate_integer(handle, METHOD_NAME__BBN, >>> NULL, &bus); >>> if (ACPI_SUCCESS(status)) >>> root->secondary.start = bus; >>> else if (status == AE_NOT_FOUND) >>> root->secondary.start = 0; >>> else { >>> - printk(KERN_ERR PREFIX "can't evaluate _BBN\n"); >>> + acpi_handle_err(handle, "can't evaluate _BBN\n"); >>> result = -ENODEV; >>> goto end; >>> } >>> @@ -425,11 +426,11 @@ static int acpi_pci_root_add(struct acpi_device *device, >>> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS); >>> device->driver_data = root; >>> >>> - printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n", >>> + pr_info(PREFIX "%s [%s] (domain %04x %pR)\n", >>> acpi_device_name(device), acpi_device_bid(device), >>> root->segment, &root->secondary); >>> >>> - root->mcfg_addr = acpi_pci_root_get_mcfg_addr(device->handle); >>> + root->mcfg_addr = acpi_pci_root_get_mcfg_addr(handle); >>> >>> /* >>> * All supported architectures that use ACPI have support for >>> @@ -473,7 +474,7 @@ static int acpi_pci_root_add(struct acpi_device *device, >>> dev_info(&device->dev, >>> "Requesting ACPI _OSC control (0x%02x)\n", flags); >>> >>> - status = acpi_pci_osc_control_set(device->handle, &flags, >>> + status = acpi_pci_osc_control_set(handle, &flags, >>> OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL); >>> if (ACPI_SUCCESS(status)) { >>> is_osc_granted = true; >>> @@ -505,9 +506,9 @@ static int acpi_pci_root_add(struct acpi_device *device, >>> */ >>> root->bus = pci_acpi_scan_root(root); >>> if (!root->bus) { >>> - printk(KERN_ERR PREFIX >>> - "Bus %04x:%02x not present in PCI namespace\n", >>> - root->segment, (unsigned int)root->secondary.start); >>> + acpi_handle_err(handle, >>> + "Bus %04x:%02x not present in PCI namespace\n", >>> + root->segment, (unsigned int)root->secondary.start); >>> result = -ENODEV; >>> goto end; >>> } >>> @@ -517,8 +518,8 @@ static int acpi_pci_root_add(struct acpi_device *device, >>> if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) >>> pcie_clear_aspm(root->bus); >>> } else { >>> - pr_info("ACPI _OSC control for PCIe not granted, " >>> - "disabling ASPM\n"); >>> + acpi_handle_info(handle, >>> + "ACPI _OSC control for PCIe not granted, disabling ASPM\n"); >>> pcie_no_aspm(); >>> } >>> >>> @@ -571,12 +572,13 @@ static void handle_root_bridge_insertion(acpi_handle handle) >>> struct acpi_device *device; >>> >>> if (!acpi_bus_get_device(handle, &device)) { >>> - printk(KERN_DEBUG "acpi device exists...\n"); >>> + acpi_handle_printk(KERN_DEBUG, handle, >>> + "acpi device exists...\n"); >>> return; >>> } >>> >>> if (acpi_bus_scan(handle)) >>> - printk(KERN_ERR "cannot add bridge to acpi list\n"); >>> + acpi_handle_err(handle, "cannot add bridge to acpi list\n"); >>> } >>> >>> static void handle_root_bridge_removal(struct acpi_device *device) >>> @@ -602,7 +604,6 @@ static void handle_root_bridge_removal(struct acpi_device *device) >>> static void _handle_hotplug_event_root(struct work_struct *work) >>> { >>> struct acpi_pci_root *root; >>> - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER }; >>> struct acpi_hp_work *hp_work; >>> acpi_handle handle; >>> u32 type; >>> @@ -613,13 +614,11 @@ static void _handle_hotplug_event_root(struct work_struct *work) >>> >>> root = acpi_pci_find_root(handle); >>> >>> - acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); >>> - >>> switch (type) { >>> case ACPI_NOTIFY_BUS_CHECK: >>> /* bus enumerate */ >>> - printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__, >>> - (char *)buffer.pointer); >>> + acpi_handle_printk(KERN_DEBUG, handle, >>> + "Bus check notify on %s\n", __func__); >>> if (!root) >>> handle_root_bridge_insertion(handle); >>> >>> @@ -627,27 +626,26 @@ static void _handle_hotplug_event_root(struct work_struct *work) >>> >>> case ACPI_NOTIFY_DEVICE_CHECK: >>> /* device check */ >>> - printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__, >>> - (char *)buffer.pointer); >>> + acpi_handle_printk(KERN_DEBUG, handle, >>> + "Device check notify on %s\n", __func__); >>> if (!root) >>> handle_root_bridge_insertion(handle); >>> break; >>> >>> case ACPI_NOTIFY_EJECT_REQUEST: >>> /* request device eject */ >>> - printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__, >>> - (char *)buffer.pointer); >>> + acpi_handle_printk(KERN_DEBUG, handle, >>> + "Device eject notify on %s\n", __func__); >>> if (root) >>> handle_root_bridge_removal(root->device); >>> break; >>> default: >>> - printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n", >>> - type, (char *)buffer.pointer); >>> + acpi_handle_warn(handle, >>> + "notify_handler: unknown event type 0x%x\n", type); >>> break; >>> } >>> >>> kfree(hp_work); /* allocated in handle_hotplug_event_bridge */ >>> - kfree(buffer.pointer); >>> } >>> >>> static void handle_hotplug_event_root(acpi_handle handle, u32 type, >>> @@ -661,9 +659,6 @@ static acpi_status __init >>> find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) >>> { >>> acpi_status status; >>> - char objname[64]; >>> - struct acpi_buffer buffer = { .length = sizeof(objname), >>> - .pointer = objname }; >>> int *count = (int *)context; >>> >>> if (!acpi_is_root_bridge(handle)) >>> @@ -671,16 +666,15 @@ find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) >>> >>> (*count)++; >>> >>> - acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); >>> - >>> status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY, >>> handle_hotplug_event_root, NULL); >>> if (ACPI_FAILURE(status)) >>> - printk(KERN_DEBUG "acpi root: %s notify handler is not installed, exit status: %u\n", >>> - objname, (unsigned int)status); >>> + acpi_handle_printk(KERN_DEBUG, handle, >>> + "notify handler is not installed, exit status: %u\n", >>> + (unsigned int)status); >>> else >>> - printk(KERN_DEBUG "acpi root: %s notify handler is installed\n", >>> - objname); >>> + acpi_handle_printk(KERN_DEBUG, handle, >>> + "notify handler is installed\n"); >>> >>> return AE_OK; >>> } >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] ACPI, pci_root: use acpi_handle_print() and pr_xxx() to print messages 2013-05-31 16:20 ` Jiang Liu @ 2013-05-31 19:18 ` Bjorn Helgaas 2013-06-03 13:16 ` [PATCH] PCI, ACPI: use dev_printk() to keep messages consistent Jiang Liu 0 siblings, 1 reply; 12+ messages in thread From: Bjorn Helgaas @ 2013-05-31 19:18 UTC (permalink / raw) To: Jiang Liu Cc: Rafael J. Wysocki, Yinghai Lu, Jiang Liu, Gu Zheng, Toshi Kani, Myron Stowe, Yijing Wang, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Len Brown, linux-acpi@vger.kernel.org On Fri, May 31, 2013 at 10:20 AM, Jiang Liu <liuj97@gmail.com> wrote: > On Fri 31 May 2013 03:51:51 AM CST, Rafael J. Wysocki wrote: >> On Thursday, May 30, 2013 11:33:56 AM Bjorn Helgaas wrote: >>> On Tue, May 14, 2013 at 09:00:39PM +0800, Jiang Liu wrote: >>>> Use acpi_handle_print() and pr_xxx() to print messages in pci_root.c. >>>> >>>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com> >>>> Cc: Len Brown <lenb@kernel.org> >>>> Cc: "Rafael J. Wysocki" <rjw@sisk.pl> >>>> Cc: linux-acpi@vger.kernel.org >>>> Cc: linux-kernel@vger.kernel.org >>>> --- >>>> drivers/acpi/pci_root.c | 70 ++++++++++++++++++++++--------------------------- >>>> 1 file changed, 32 insertions(+), 38 deletions(-) >>>> >>>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >>>> index 91ddfd6..21dda5a 100644 >>>> --- a/drivers/acpi/pci_root.c >>>> +++ b/drivers/acpi/pci_root.c >>>> @@ -379,23 +379,24 @@ static int acpi_pci_root_add(struct acpi_device *device, >>>> struct acpi_pci_root *root; >>>> u32 flags, base_flags; >>>> bool is_osc_granted = false; >>>> + acpi_handle handle = device->handle; >>>> >>>> root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL); >>>> if (!root) >>>> return -ENOMEM; >>>> >>>> segment = 0; >>>> - status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL, >>>> + status = acpi_evaluate_integer(handle, METHOD_NAME__SEG, NULL, >>>> &segment); >>>> if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { >>>> - printk(KERN_ERR PREFIX "can't evaluate _SEG\n"); >>>> + acpi_handle_err(handle, "can't evaluate _SEG\n"); >>> >>> I previously acked this, but I noticed that we are making a mix of >>> dev_printk() and acpi_handle_printk() here. The difference is like this: >>> >>> acpi PNP0A08:00: ACPI _OSC support notification failed, disabling PCIe ASPM >>> ACPI: \_SB_.PCI0: ACPI _OSC support notification failed, disabling PCIe ASPM >>> >>> I'm not sure which direction we should go here, but I think we should >>> choose one or the other and use it consistently. Personally, I think the >>> internal DSDT names should be available *somewhere*, but not necessarily >>> used in run-of-the-mill chit-chat. For that reason, I prefer >>> dev_printk(), though I have the feeling that Rafael is moving toward >>> eliminating the struct acpi_device, so he might prefer >>> acpi_handle_printk(). >> >> I really don't care that much, but I agree that they should be used >> consistently. > Hi Bjorn and Rafael, > I will work on this tomorrow. In some cases, only handle is > available, so we > could only use acpi_handle_printk(). So I think the directly may be: > use dev_printk() > if possible, otherwise use acpi_handle_printk() instead. Is that the > right way to go? That sounds good to me. Bjorn ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] PCI, ACPI: use dev_printk() to keep messages consistent 2013-05-31 19:18 ` Bjorn Helgaas @ 2013-06-03 13:16 ` Jiang Liu 2013-06-03 21:09 ` Bjorn Helgaas 0 siblings, 1 reply; 12+ messages in thread From: Jiang Liu @ 2013-06-03 13:16 UTC (permalink / raw) To: Bjorn Helgaas, Rafael J . Wysocki; +Cc: Jiang Liu, Yijing Wang, linux-pci As pointed out by Bjorn that acpi_handle_printk() generates messages like "ACPI: \_SB_.PCI0: xxxxx" and dev_printk() generates messages like "acpi PNP0A08:00: xxxx", so use dev_printk() to keep messages consistent when possible. Signed-off-by: Jiang Liu <jiang.liu@huawei.com> --- Hi Bjorn, Could you please help to fold this patch onto "PCI/ACPI: Use acpi_handle_print() and pr_xxx() to print messages" of branch "pci/jiang-remove-global-list"? Thanks! Gerry --- drivers/acpi/pci_root.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 73a934d..4a8f5b4 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -388,7 +388,7 @@ static int acpi_pci_root_add(struct acpi_device *device, status = acpi_evaluate_integer(handle, METHOD_NAME__SEG, NULL, &segment); if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { - acpi_handle_err(handle, "can't evaluate _SEG\n"); + dev_err(&device->dev, "can't evaluate _SEG\n"); result = -ENODEV; goto end; } @@ -404,8 +404,8 @@ static int acpi_pci_root_add(struct acpi_device *device, * can do is assume [_BBN-0xFF] or [0-0xFF]. */ root->secondary.end = 0xFF; - acpi_handle_warn(handle, - FW_BUG "no secondary bus range in _CRS\n"); + dev_warn(&device->dev, + FW_BUG "no secondary bus range in _CRS\n"); status = acpi_evaluate_integer(handle, METHOD_NAME__BBN, NULL, &bus); if (ACPI_SUCCESS(status)) @@ -413,7 +413,7 @@ static int acpi_pci_root_add(struct acpi_device *device, else if (status == AE_NOT_FOUND) root->secondary.start = 0; else { - acpi_handle_err(handle, "can't evaluate _BBN\n"); + dev_err(&device->dev, "can't evaluate _BBN\n"); result = -ENODEV; goto end; } @@ -451,9 +451,9 @@ static int acpi_pci_root_add(struct acpi_device *device, */ root->bus = pci_acpi_scan_root(root); if (!root->bus) { - acpi_handle_err(handle, - "Bus %04x:%02x not present in PCI namespace\n", - root->segment, (unsigned int)root->secondary.start); + dev_err(&device->dev, + "Bus %04x:%02x not present in PCI namespace\n", + root->segment, (unsigned int)root->secondary.start); result = -ENODEV; goto end; } @@ -511,8 +511,8 @@ static int acpi_pci_root_add(struct acpi_device *device, "ACPI _OSC request failed (%s), " "returned control mask: 0x%02x\n", acpi_format_exception(status), flags); - acpi_handle_info(handle, - "ACPI _OSC control for PCIe not granted, disabling ASPM\n"); + dev_info(&device->dev, + "ACPI _OSC control for PCIe not granted, disabling ASPM\n"); pcie_no_aspm(); } } else { -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] PCI, ACPI: use dev_printk() to keep messages consistent 2013-06-03 13:16 ` [PATCH] PCI, ACPI: use dev_printk() to keep messages consistent Jiang Liu @ 2013-06-03 21:09 ` Bjorn Helgaas 0 siblings, 0 replies; 12+ messages in thread From: Bjorn Helgaas @ 2013-06-03 21:09 UTC (permalink / raw) To: Jiang Liu Cc: Rafael J . Wysocki, Jiang Liu, Yijing Wang, linux-pci@vger.kernel.org On Mon, Jun 3, 2013 at 7:16 AM, Jiang Liu <liuj97@gmail.com> wrote: > As pointed out by Bjorn that acpi_handle_printk() generates messages > like "ACPI: \_SB_.PCI0: xxxxx" and dev_printk() generates messages > like "acpi PNP0A08:00: xxxx", so use dev_printk() to keep messages > consistent when possible. > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > --- > Hi Bjorn, > Could you please help to fold this patch onto "PCI/ACPI: Use > acpi_handle_print() and pr_xxx() to print messages" of branch > "pci/jiang-remove-global-list"? Done. I also changed one acpi_handle_print() in handle_root_bridge_insertion() where we do have a device. This is in my "next" branch for v3.11. Thanks! > drivers/acpi/pci_root.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index 73a934d..4a8f5b4 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -388,7 +388,7 @@ static int acpi_pci_root_add(struct acpi_device *device, > status = acpi_evaluate_integer(handle, METHOD_NAME__SEG, NULL, > &segment); > if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { > - acpi_handle_err(handle, "can't evaluate _SEG\n"); > + dev_err(&device->dev, "can't evaluate _SEG\n"); > result = -ENODEV; > goto end; > } > @@ -404,8 +404,8 @@ static int acpi_pci_root_add(struct acpi_device *device, > * can do is assume [_BBN-0xFF] or [0-0xFF]. > */ > root->secondary.end = 0xFF; > - acpi_handle_warn(handle, > - FW_BUG "no secondary bus range in _CRS\n"); > + dev_warn(&device->dev, > + FW_BUG "no secondary bus range in _CRS\n"); > status = acpi_evaluate_integer(handle, METHOD_NAME__BBN, > NULL, &bus); > if (ACPI_SUCCESS(status)) > @@ -413,7 +413,7 @@ static int acpi_pci_root_add(struct acpi_device *device, > else if (status == AE_NOT_FOUND) > root->secondary.start = 0; > else { > - acpi_handle_err(handle, "can't evaluate _BBN\n"); > + dev_err(&device->dev, "can't evaluate _BBN\n"); > result = -ENODEV; > goto end; > } > @@ -451,9 +451,9 @@ static int acpi_pci_root_add(struct acpi_device *device, > */ > root->bus = pci_acpi_scan_root(root); > if (!root->bus) { > - acpi_handle_err(handle, > - "Bus %04x:%02x not present in PCI namespace\n", > - root->segment, (unsigned int)root->secondary.start); > + dev_err(&device->dev, > + "Bus %04x:%02x not present in PCI namespace\n", > + root->segment, (unsigned int)root->secondary.start); > result = -ENODEV; > goto end; > } > @@ -511,8 +511,8 @@ static int acpi_pci_root_add(struct acpi_device *device, > "ACPI _OSC request failed (%s), " > "returned control mask: 0x%02x\n", > acpi_format_exception(status), flags); > - acpi_handle_info(handle, > - "ACPI _OSC control for PCIe not granted, disabling ASPM\n"); > + dev_info(&device->dev, > + "ACPI _OSC control for PCIe not granted, disabling ASPM\n"); > pcie_no_aspm(); > } > } else { > -- > 1.8.1.2 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c 2013-05-14 13:00 [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c Jiang Liu 2013-05-14 13:00 ` [PATCH v1 2/2] ACPI, pci_root: use acpi_handle_print() and pr_xxx() to print messages Jiang Liu @ 2013-05-28 22:37 ` Bjorn Helgaas 2013-05-28 23:21 ` Rafael J. Wysocki 1 sibling, 1 reply; 12+ messages in thread From: Bjorn Helgaas @ 2013-05-28 22:37 UTC (permalink / raw) To: Jiang Liu Cc: Rafael J . Wysocki, Yinghai Lu, Jiang Liu, Gu Zheng, Toshi Kani, Myron Stowe, Yijing Wang, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Len Brown, linux-acpi@vger.kernel.org On Tue, May 14, 2013 at 7:00 AM, Jiang Liu <liuj97@gmail.com> wrote: > Now the global list acpi_pci_roots pci_root.c is useless, remove it. > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > Cc: Len Brown <lenb@kernel.org> > Cc: "Rafael J. Wysocki" <rjw@sisk.pl> > Cc: linux-acpi@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > drivers/acpi/pci_root.c | 25 +++---------------------- > include/acpi/acpi_bus.h | 1 - > 2 files changed, 3 insertions(+), 23 deletions(-) This and the "use acpi_handle_print()" patch look fine to me. Rafael, I can take them, or if you want to take them yourself, you can add Acked-by: Bjorn Helgaas <bhelgaas@google.com> Bjorn > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index b80e06e..91ddfd6 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -65,10 +65,6 @@ static struct acpi_scan_handler pci_root_handler = { > .detach = acpi_pci_root_remove, > }; > > -/* Lock to protect both acpi_pci_roots lists */ > -static DEFINE_MUTEX(acpi_pci_root_lock); > -static LIST_HEAD(acpi_pci_roots); > - > static DEFINE_MUTEX(osc_lock); > > /** > @@ -423,7 +419,6 @@ static int acpi_pci_root_add(struct acpi_device *device, > } > } > > - INIT_LIST_HEAD(&root->node); > root->device = device; > root->segment = segment & 0xFFFF; > strcpy(acpi_device_name(device), ACPI_PCI_ROOT_DEVICE_NAME); > @@ -501,10 +496,6 @@ static int acpi_pci_root_add(struct acpi_device *device, > * TBD: Need PCI interface for enumeration/configuration of roots. > */ > > - mutex_lock(&acpi_pci_root_lock); > - list_add_tail(&root->node, &acpi_pci_roots); > - mutex_unlock(&acpi_pci_root_lock); > - > /* > * Scan the Root Bridge > * -------------------- > @@ -518,7 +509,7 @@ static int acpi_pci_root_add(struct acpi_device *device, > "Bus %04x:%02x not present in PCI namespace\n", > root->segment, (unsigned int)root->secondary.start); > result = -ENODEV; > - goto out_del_root; > + goto end; > } > > /* ASPM setting */ > @@ -538,20 +529,13 @@ static int acpi_pci_root_add(struct acpi_device *device, > if (system_state != SYSTEM_BOOTING) { > pcibios_resource_survey_bus(root->bus); > pci_assign_unassigned_bus_resources(root->bus); > - } > - > - /* need to after hot-added ioapic is registered */ > - if (system_state != SYSTEM_BOOTING) > + /* need to after hot-added ioapic is registered */ > pci_enable_bridges(root->bus); > + } > > pci_bus_add_devices(root->bus); > return 1; > > -out_del_root: > - mutex_lock(&acpi_pci_root_lock); > - list_del(&root->node); > - mutex_unlock(&acpi_pci_root_lock); > - > end: > kfree(root); > return result; > @@ -568,9 +552,6 @@ static void acpi_pci_root_remove(struct acpi_device *device) > > pci_remove_root_bus(root->bus); > > - mutex_lock(&acpi_pci_root_lock); > - list_del(&root->node); > - mutex_unlock(&acpi_pci_root_lock); > kfree(root); > } > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index 22ba56e..4eb9a88 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -447,7 +447,6 @@ int register_acpi_bus_type(struct acpi_bus_type *); > int unregister_acpi_bus_type(struct acpi_bus_type *); > > struct acpi_pci_root { > - struct list_head node; > struct acpi_device * device; > struct pci_bus *bus; > u16 segment; > -- > 1.8.1.2 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c 2013-05-28 22:37 ` [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c Bjorn Helgaas @ 2013-05-28 23:21 ` Rafael J. Wysocki 2013-05-28 23:48 ` Rafael J. Wysocki 0 siblings, 1 reply; 12+ messages in thread From: Rafael J. Wysocki @ 2013-05-28 23:21 UTC (permalink / raw) To: Bjorn Helgaas Cc: Jiang Liu, Yinghai Lu, Jiang Liu, Gu Zheng, Toshi Kani, Myron Stowe, Yijing Wang, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Len Brown, linux-acpi@vger.kernel.org On Tuesday, May 28, 2013 04:37:03 PM Bjorn Helgaas wrote: > On Tue, May 14, 2013 at 7:00 AM, Jiang Liu <liuj97@gmail.com> wrote: > > Now the global list acpi_pci_roots pci_root.c is useless, remove it. > > > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > > Cc: Len Brown <lenb@kernel.org> > > Cc: "Rafael J. Wysocki" <rjw@sisk.pl> > > Cc: linux-acpi@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > --- > > drivers/acpi/pci_root.c | 25 +++---------------------- > > include/acpi/acpi_bus.h | 1 - > > 2 files changed, 3 insertions(+), 23 deletions(-) > > This and the "use acpi_handle_print()" patch look fine to me. > > Rafael, I can take them, or if you want to take them yourself, you can add > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> OK, I'll take them. Thanks, Rafael > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > > index b80e06e..91ddfd6 100644 > > --- a/drivers/acpi/pci_root.c > > +++ b/drivers/acpi/pci_root.c > > @@ -65,10 +65,6 @@ static struct acpi_scan_handler pci_root_handler = { > > .detach = acpi_pci_root_remove, > > }; > > > > -/* Lock to protect both acpi_pci_roots lists */ > > -static DEFINE_MUTEX(acpi_pci_root_lock); > > -static LIST_HEAD(acpi_pci_roots); > > - > > static DEFINE_MUTEX(osc_lock); > > > > /** > > @@ -423,7 +419,6 @@ static int acpi_pci_root_add(struct acpi_device *device, > > } > > } > > > > - INIT_LIST_HEAD(&root->node); > > root->device = device; > > root->segment = segment & 0xFFFF; > > strcpy(acpi_device_name(device), ACPI_PCI_ROOT_DEVICE_NAME); > > @@ -501,10 +496,6 @@ static int acpi_pci_root_add(struct acpi_device *device, > > * TBD: Need PCI interface for enumeration/configuration of roots. > > */ > > > > - mutex_lock(&acpi_pci_root_lock); > > - list_add_tail(&root->node, &acpi_pci_roots); > > - mutex_unlock(&acpi_pci_root_lock); > > - > > /* > > * Scan the Root Bridge > > * -------------------- > > @@ -518,7 +509,7 @@ static int acpi_pci_root_add(struct acpi_device *device, > > "Bus %04x:%02x not present in PCI namespace\n", > > root->segment, (unsigned int)root->secondary.start); > > result = -ENODEV; > > - goto out_del_root; > > + goto end; > > } > > > > /* ASPM setting */ > > @@ -538,20 +529,13 @@ static int acpi_pci_root_add(struct acpi_device *device, > > if (system_state != SYSTEM_BOOTING) { > > pcibios_resource_survey_bus(root->bus); > > pci_assign_unassigned_bus_resources(root->bus); > > - } > > - > > - /* need to after hot-added ioapic is registered */ > > - if (system_state != SYSTEM_BOOTING) > > + /* need to after hot-added ioapic is registered */ > > pci_enable_bridges(root->bus); > > + } > > > > pci_bus_add_devices(root->bus); > > return 1; > > > > -out_del_root: > > - mutex_lock(&acpi_pci_root_lock); > > - list_del(&root->node); > > - mutex_unlock(&acpi_pci_root_lock); > > - > > end: > > kfree(root); > > return result; > > @@ -568,9 +552,6 @@ static void acpi_pci_root_remove(struct acpi_device *device) > > > > pci_remove_root_bus(root->bus); > > > > - mutex_lock(&acpi_pci_root_lock); > > - list_del(&root->node); > > - mutex_unlock(&acpi_pci_root_lock); > > kfree(root); > > } > > > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > > index 22ba56e..4eb9a88 100644 > > --- a/include/acpi/acpi_bus.h > > +++ b/include/acpi/acpi_bus.h > > @@ -447,7 +447,6 @@ int register_acpi_bus_type(struct acpi_bus_type *); > > int unregister_acpi_bus_type(struct acpi_bus_type *); > > > > struct acpi_pci_root { > > - struct list_head node; > > struct acpi_device * device; > > struct pci_bus *bus; > > u16 segment; > > -- > > 1.8.1.2 > > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c 2013-05-28 23:21 ` Rafael J. Wysocki @ 2013-05-28 23:48 ` Rafael J. Wysocki 0 siblings, 0 replies; 12+ messages in thread From: Rafael J. Wysocki @ 2013-05-28 23:48 UTC (permalink / raw) To: Bjorn Helgaas Cc: Jiang Liu, Yinghai Lu, Jiang Liu, Gu Zheng, Toshi Kani, Myron Stowe, Yijing Wang, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Len Brown, linux-acpi@vger.kernel.org On Wednesday, May 29, 2013 01:21:08 AM Rafael J. Wysocki wrote: > On Tuesday, May 28, 2013 04:37:03 PM Bjorn Helgaas wrote: > > On Tue, May 14, 2013 at 7:00 AM, Jiang Liu <liuj97@gmail.com> wrote: > > > Now the global list acpi_pci_roots pci_root.c is useless, remove it. > > > > > > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > > > Cc: Len Brown <lenb@kernel.org> > > > Cc: "Rafael J. Wysocki" <rjw@sisk.pl> > > > Cc: linux-acpi@vger.kernel.org > > > Cc: linux-kernel@vger.kernel.org > > > --- > > > drivers/acpi/pci_root.c | 25 +++---------------------- > > > include/acpi/acpi_bus.h | 1 - > > > 2 files changed, 3 insertions(+), 23 deletions(-) > > > > This and the "use acpi_handle_print()" patch look fine to me. > > > > Rafael, I can take them, or if you want to take them yourself, you can add > > > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > OK, I'll take them. Well, if I do that, there will be conflicts between our trees, so I think it's better if you handle them. Please add Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> to both. Thanks, Rafael > > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > > > index b80e06e..91ddfd6 100644 > > > --- a/drivers/acpi/pci_root.c > > > +++ b/drivers/acpi/pci_root.c > > > @@ -65,10 +65,6 @@ static struct acpi_scan_handler pci_root_handler = { > > > .detach = acpi_pci_root_remove, > > > }; > > > > > > -/* Lock to protect both acpi_pci_roots lists */ > > > -static DEFINE_MUTEX(acpi_pci_root_lock); > > > -static LIST_HEAD(acpi_pci_roots); > > > - > > > static DEFINE_MUTEX(osc_lock); > > > > > > /** > > > @@ -423,7 +419,6 @@ static int acpi_pci_root_add(struct acpi_device *device, > > > } > > > } > > > > > > - INIT_LIST_HEAD(&root->node); > > > root->device = device; > > > root->segment = segment & 0xFFFF; > > > strcpy(acpi_device_name(device), ACPI_PCI_ROOT_DEVICE_NAME); > > > @@ -501,10 +496,6 @@ static int acpi_pci_root_add(struct acpi_device *device, > > > * TBD: Need PCI interface for enumeration/configuration of roots. > > > */ > > > > > > - mutex_lock(&acpi_pci_root_lock); > > > - list_add_tail(&root->node, &acpi_pci_roots); > > > - mutex_unlock(&acpi_pci_root_lock); > > > - > > > /* > > > * Scan the Root Bridge > > > * -------------------- > > > @@ -518,7 +509,7 @@ static int acpi_pci_root_add(struct acpi_device *device, > > > "Bus %04x:%02x not present in PCI namespace\n", > > > root->segment, (unsigned int)root->secondary.start); > > > result = -ENODEV; > > > - goto out_del_root; > > > + goto end; > > > } > > > > > > /* ASPM setting */ > > > @@ -538,20 +529,13 @@ static int acpi_pci_root_add(struct acpi_device *device, > > > if (system_state != SYSTEM_BOOTING) { > > > pcibios_resource_survey_bus(root->bus); > > > pci_assign_unassigned_bus_resources(root->bus); > > > - } > > > - > > > - /* need to after hot-added ioapic is registered */ > > > - if (system_state != SYSTEM_BOOTING) > > > + /* need to after hot-added ioapic is registered */ > > > pci_enable_bridges(root->bus); > > > + } > > > > > > pci_bus_add_devices(root->bus); > > > return 1; > > > > > > -out_del_root: > > > - mutex_lock(&acpi_pci_root_lock); > > > - list_del(&root->node); > > > - mutex_unlock(&acpi_pci_root_lock); > > > - > > > end: > > > kfree(root); > > > return result; > > > @@ -568,9 +552,6 @@ static void acpi_pci_root_remove(struct acpi_device *device) > > > > > > pci_remove_root_bus(root->bus); > > > > > > - mutex_lock(&acpi_pci_root_lock); > > > - list_del(&root->node); > > > - mutex_unlock(&acpi_pci_root_lock); > > > kfree(root); > > > } > > > > > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > > > index 22ba56e..4eb9a88 100644 > > > --- a/include/acpi/acpi_bus.h > > > +++ b/include/acpi/acpi_bus.h > > > @@ -447,7 +447,6 @@ int register_acpi_bus_type(struct acpi_bus_type *); > > > int unregister_acpi_bus_type(struct acpi_bus_type *); > > > > > > struct acpi_pci_root { > > > - struct list_head node; > > > struct acpi_device * device; > > > struct pci_bus *bus; > > > u16 segment; > > > -- > > > 1.8.1.2 > > > > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c @ 2013-05-14 12:59 Jiang Liu 0 siblings, 0 replies; 12+ messages in thread From: Jiang Liu @ 2013-05-14 12:59 UTC (permalink / raw) To: Rafael J . Wysocki, Bjorn Helgaas, Yinghai Lu Cc: Jiang Liu, Gu Zheng, Toshi Kani, Myron Stowe, Yijing Wang, Jiang Liu, linux-pci, linux-kernel, Len Brown, linux-acpi Now the global list acpi_pci_roots pci_root.c is useless, remove it. Signed-off-by: Jiang Liu <jiang.liu@huawei.com> Cc: Len Brown <lenb@kernel.org> Cc: "Rafael J. Wysocki" <rjw@sisk.pl> Cc: linux-acpi@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/acpi/pci_root.c | 25 +++---------------------- include/acpi/acpi_bus.h | 1 - 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index b80e06e..91ddfd6 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -65,10 +65,6 @@ static struct acpi_scan_handler pci_root_handler = { .detach = acpi_pci_root_remove, }; -/* Lock to protect both acpi_pci_roots lists */ -static DEFINE_MUTEX(acpi_pci_root_lock); -static LIST_HEAD(acpi_pci_roots); - static DEFINE_MUTEX(osc_lock); /** @@ -423,7 +419,6 @@ static int acpi_pci_root_add(struct acpi_device *device, } } - INIT_LIST_HEAD(&root->node); root->device = device; root->segment = segment & 0xFFFF; strcpy(acpi_device_name(device), ACPI_PCI_ROOT_DEVICE_NAME); @@ -501,10 +496,6 @@ static int acpi_pci_root_add(struct acpi_device *device, * TBD: Need PCI interface for enumeration/configuration of roots. */ - mutex_lock(&acpi_pci_root_lock); - list_add_tail(&root->node, &acpi_pci_roots); - mutex_unlock(&acpi_pci_root_lock); - /* * Scan the Root Bridge * -------------------- @@ -518,7 +509,7 @@ static int acpi_pci_root_add(struct acpi_device *device, "Bus %04x:%02x not present in PCI namespace\n", root->segment, (unsigned int)root->secondary.start); result = -ENODEV; - goto out_del_root; + goto end; } /* ASPM setting */ @@ -538,20 +529,13 @@ static int acpi_pci_root_add(struct acpi_device *device, if (system_state != SYSTEM_BOOTING) { pcibios_resource_survey_bus(root->bus); pci_assign_unassigned_bus_resources(root->bus); - } - - /* need to after hot-added ioapic is registered */ - if (system_state != SYSTEM_BOOTING) + /* need to after hot-added ioapic is registered */ pci_enable_bridges(root->bus); + } pci_bus_add_devices(root->bus); return 1; -out_del_root: - mutex_lock(&acpi_pci_root_lock); - list_del(&root->node); - mutex_unlock(&acpi_pci_root_lock); - end: kfree(root); return result; @@ -568,9 +552,6 @@ static void acpi_pci_root_remove(struct acpi_device *device) pci_remove_root_bus(root->bus); - mutex_lock(&acpi_pci_root_lock); - list_del(&root->node); - mutex_unlock(&acpi_pci_root_lock); kfree(root); } diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 22ba56e..4eb9a88 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -447,7 +447,6 @@ int register_acpi_bus_type(struct acpi_bus_type *); int unregister_acpi_bus_type(struct acpi_bus_type *); struct acpi_pci_root { - struct list_head node; struct acpi_device * device; struct pci_bus *bus; u16 segment; -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-06-03 21:09 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-05-14 13:00 [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c Jiang Liu 2013-05-14 13:00 ` [PATCH v1 2/2] ACPI, pci_root: use acpi_handle_print() and pr_xxx() to print messages Jiang Liu 2013-05-30 17:33 ` Bjorn Helgaas 2013-05-30 19:51 ` Rafael J. Wysocki 2013-05-31 16:20 ` Jiang Liu 2013-05-31 19:18 ` Bjorn Helgaas 2013-06-03 13:16 ` [PATCH] PCI, ACPI: use dev_printk() to keep messages consistent Jiang Liu 2013-06-03 21:09 ` Bjorn Helgaas 2013-05-28 22:37 ` [PATCH v1 1/2] ACPI, PCI: remove unused global list acpi_pci_roots in pci_root.c Bjorn Helgaas 2013-05-28 23:21 ` Rafael J. Wysocki 2013-05-28 23:48 ` Rafael J. Wysocki -- strict thread matches above, loose matches on Subject: below -- 2013-05-14 12:59 Jiang Liu
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).