From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 257A1C433DB for ; Thu, 4 Feb 2021 18:35:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E13A064D9A for ; Thu, 4 Feb 2021 18:35:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238171AbhBDSfs (ORCPT ); Thu, 4 Feb 2021 13:35:48 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:28132 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239092AbhBDSfF (ORCPT ); Thu, 4 Feb 2021 13:35:05 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612463615; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=dKrOz7FAs3tJoHyGXBNrG1BWtJMXb7u2kkdGtqwvLV8=; b=gHwt0chJ6QlqM7nWZwrMwt7Wxbmkz7Gnt44L3cUUzWZYRFwmY/yji7ehcqulso1Er6lyRe +pVE6/Ev/rXMj97SDFy4CjeDFEPhoIVnU0MlZAaYouB4ILql0BUGnh7UryLVPFolmRaq7e rK5PW29keHNwJwM8+q2qZS+jCeeiOTc= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-149-HeFibZSpMLewjTMixsjMSA-1; Thu, 04 Feb 2021 13:33:33 -0500 X-MC-Unique: HeFibZSpMLewjTMixsjMSA-1 Received: by mail-ed1-f71.google.com with SMTP id bd22so3656353edb.4 for ; Thu, 04 Feb 2021 10:33:33 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=dKrOz7FAs3tJoHyGXBNrG1BWtJMXb7u2kkdGtqwvLV8=; b=QC2+jzHzTUCnp0yYfp0GoMSnNmtkZ9g5VFNWPYHavicibp3SoDaKHGPNwN2JFTJdpF uHyYFWajnM9rxVscL7n2WMNRfMMHxYK0eSJSzRRfufa4vpMCRBPe0DPHXdBCj2OjWCcG KNwQGg32/DYawMz0jic6VYqsXS1NvCoAnSbwR0G8k+k22vuBlY/SijAlquWGv7FtnnM1 SiMClWFeQOpqfG5zRODICtMMrfqFLv6veVE1KK2xmMNRL43BbYTjMHyePE60Q5i374Rl YyNT3UeTz8K6j3d5m8FYZsDC7AUMB2+L5janyqestfwBGwTzkTjLsQYVC1vKMy1SA4u3 H13A== X-Gm-Message-State: AOAM5313VenOvrmjkAx6XzTSfLIpt5WjutPyKjVJaytnMHQhlUsF4TfI wR08IGQqrFSaryDzzhOWSfzCiK9ps24xv2BjuowvgV3ywjLn1c/PAYEmtNPU6bQfvrFzskdW/Lp vcBAntxjPcMXbAZ0D9DfqHC0K X-Received: by 2002:a17:906:69c2:: with SMTP id g2mr389155ejs.249.1612463612099; Thu, 04 Feb 2021 10:33:32 -0800 (PST) X-Google-Smtp-Source: ABdhPJwI8tZuMkp0s3kgIPgKAX8NrZicJU/N+6+o/Y4gR3AXjZ6IKYn4nargW7ZnRxYDkHzp6c90Wg== X-Received: by 2002:a17:906:69c2:: with SMTP id g2mr388812ejs.249.1612463607793; Thu, 04 Feb 2021 10:33:27 -0800 (PST) Received: from x1.localdomain (2001-1c00-0c1e-bf00-37a3-353b-be90-1238.cable.dynamic.v6.ziggo.nl. [2001:1c00:c1e:bf00:37a3:353b:be90:1238]) by smtp.gmail.com with ESMTPSA id k22sm2987184edv.33.2021.02.04.10.33.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 04 Feb 2021 10:33:26 -0800 (PST) Subject: Re: [PATCH v3 4/5] ACPI: video: Clean up printing messages To: "Rafael J. Wysocki" , Linux ACPI Cc: Linux PM , LKML , "Rafael J. Wysocki" , Zhang Rui , Erik Kaneda , Joe Perches , Hanjun Guo References: <2367702.B5bJTmGzJm@kreacher> <1991501.dpTHplkurC@kreacher> <1961054.9MKZ8ejxOh@kreacher> <1924490.ZvBDFke9FE@kreacher> From: Hans de Goede Message-ID: Date: Thu, 4 Feb 2021 19:33:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: <1924490.ZvBDFke9FE@kreacher> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 2/3/21 7:48 PM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > > Replace the ACPI_DEBUG_PRINT() instances in acpi_video.c with > acpi_handle_debug() calls and the ACPI_EXCEPTION()/ACPI_ERROR()/ > ACPI_WARNING() instances in there with acpi_handle_info() calls, > which among other things causes the excessive log levels of those > messages to be increased. > > Drop the _COMPONENT and ACPI_MODULE_NAME() definitions that are not > used any more from acpi_video.c, drop the no longer needed > ACPI_VIDEO_COMPONENT definition from the headers and update the > documentation accordingly. > > While at it, add a pr_fmt() definition to acpi_video.c, replace the > direct printk() invocations in there with acpi_handle_info() or > pr_info() (and reduce the excessive log level where applicable) and > drop the PREFIX sybmbol definition which is not necessary any more > from acpi_video.c. > > Also make unrelated janitorial changes to fix up white space and > use ACPI_FAILURE() instead of negating ACPI_SUCCESS(). > > Signed-off-by: Rafael J. Wysocki Thanks, patch looks good to me: Reviewed-by: Hans de Goede Regards, Hans > --- > > v2 -> v3: Replace more !ACPI_SUCCESS() instances with ACPI_FAILURE(). > > v1 -> v2: Changelog update. > > --- > Documentation/firmware-guide/acpi/debug.rst | 1 > drivers/acpi/acpi_video.c | 99 ++++++++++++++-------------- > drivers/acpi/sysfs.c | 1 > include/acpi/acpi_drivers.h | 1 > 4 files changed, 51 insertions(+), 51 deletions(-) > > Index: linux-pm/drivers/acpi/acpi_video.c > =================================================================== > --- linux-pm.orig/drivers/acpi/acpi_video.c > +++ linux-pm/drivers/acpi/acpi_video.c > @@ -7,6 +7,8 @@ > * Copyright (C) 2006 Thomas Tuttle > */ > > +#define pr_fmt(fmt) "ACPI: video: " fmt > + > #include > #include > #include > @@ -26,16 +28,11 @@ > #include > #include > > -#define PREFIX "ACPI: " > - > #define ACPI_VIDEO_BUS_NAME "Video Bus" > #define ACPI_VIDEO_DEVICE_NAME "Video Device" > > #define MAX_NAME_LEN 20 > > -#define _COMPONENT ACPI_VIDEO_COMPONENT > -ACPI_MODULE_NAME("video"); > - > MODULE_AUTHOR("Bruno Ducrot"); > MODULE_DESCRIPTION("ACPI Video Driver"); > MODULE_LICENSE("GPL"); > @@ -326,11 +323,11 @@ acpi_video_device_lcd_query_levels(acpi_ > *levels = NULL; > > status = acpi_evaluate_object(handle, "_BCL", NULL, &buffer); > - if (!ACPI_SUCCESS(status)) > + if (ACPI_FAILURE(status)) > return status; > obj = (union acpi_object *)buffer.pointer; > if (!obj || (obj->type != ACPI_TYPE_PACKAGE)) { > - printk(KERN_ERR PREFIX "Invalid _BCL data\n"); > + acpi_handle_info(handle, "Invalid _BCL data\n"); > status = -EFAULT; > goto err; > } > @@ -354,7 +351,7 @@ acpi_video_device_lcd_set_level(struct a > status = acpi_execute_simple_method(device->dev->handle, > "_BCM", level); > if (ACPI_FAILURE(status)) { > - ACPI_ERROR((AE_INFO, "Evaluating _BCM failed")); > + acpi_handle_info(device->dev->handle, "_BCM evaluation failed\n"); > return -EIO; > } > > @@ -368,7 +365,7 @@ acpi_video_device_lcd_set_level(struct a > return 0; > } > > - ACPI_ERROR((AE_INFO, "Current brightness invalid")); > + acpi_handle_info(device->dev->handle, "Current brightness invalid\n"); > return -EINVAL; > } > > @@ -622,9 +619,8 @@ acpi_video_device_lcd_get_level_current( > * BQC returned an invalid level. > * Stop using it. > */ > - ACPI_WARNING((AE_INFO, > - "%s returned an invalid level", > - buf)); > + acpi_handle_info(device->dev->handle, > + "%s returned an invalid level", buf); > device->cap._BQC = device->cap._BCQ = 0; > } else { > /* > @@ -635,7 +631,8 @@ acpi_video_device_lcd_get_level_current( > * ACPI video backlight still works w/ buggy _BQC. > * http://bugzilla.kernel.org/show_bug.cgi?id=12233 > */ > - ACPI_WARNING((AE_INFO, "Evaluating %s failed", buf)); > + acpi_handle_info(device->dev->handle, > + "%s evaluation failed", buf); > device->cap._BQC = device->cap._BCQ = 0; > } > } > @@ -675,7 +672,7 @@ acpi_video_device_EDID(struct acpi_video > if (obj && obj->type == ACPI_TYPE_BUFFER) > *edid = obj; > else { > - printk(KERN_ERR PREFIX "Invalid _DDC data\n"); > + acpi_handle_info(device->dev->handle, "Invalid _DDC data\n"); > status = -EFAULT; > kfree(obj); > } > @@ -827,10 +824,9 @@ int acpi_video_get_levels(struct acpi_de > int result = 0; > u32 value; > > - if (!ACPI_SUCCESS(acpi_video_device_lcd_query_levels(device->handle, > - &obj))) { > - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Could not query available " > - "LCD brightness level\n")); > + if (ACPI_FAILURE(acpi_video_device_lcd_query_levels(device->handle, &obj))) { > + acpi_handle_debug(device->handle, > + "Could not query available LCD brightness level\n"); > result = -ENODEV; > goto out; > } > @@ -842,7 +838,6 @@ int acpi_video_get_levels(struct acpi_de > > br = kzalloc(sizeof(*br), GFP_KERNEL); > if (!br) { > - printk(KERN_ERR "can't allocate memory\n"); > result = -ENOMEM; > goto out; > } > @@ -863,7 +858,7 @@ int acpi_video_get_levels(struct acpi_de > for (i = 0; i < obj->package.count; i++) { > o = (union acpi_object *)&obj->package.elements[i]; > if (o->type != ACPI_TYPE_INTEGER) { > - printk(KERN_ERR PREFIX "Invalid data\n"); > + acpi_handle_info(device->handle, "Invalid data\n"); > continue; > } > value = (u32) o->integer.value; > @@ -900,7 +895,8 @@ int acpi_video_get_levels(struct acpi_de > br->levels[i] = br->levels[i - level_ac_battery]; > count += level_ac_battery; > } else if (level_ac_battery > ACPI_VIDEO_FIRST_LEVEL) > - ACPI_ERROR((AE_INFO, "Too many duplicates in _BCL package")); > + acpi_handle_info(device->handle, > + "Too many duplicates in _BCL package"); > > /* Check if the _BCL package is in a reversed order */ > if (max_level == br->levels[ACPI_VIDEO_FIRST_LEVEL]) { > @@ -910,8 +906,8 @@ int acpi_video_get_levels(struct acpi_de > sizeof(br->levels[ACPI_VIDEO_FIRST_LEVEL]), > acpi_video_cmp_level, NULL); > } else if (max_level != br->levels[count - 1]) > - ACPI_ERROR((AE_INFO, > - "Found unordered _BCL package")); > + acpi_handle_info(device->handle, > + "Found unordered _BCL package"); > > br->count = count; > *dev_br = br; > @@ -989,9 +985,9 @@ set_level: > if (result) > goto out_free_levels; > > - ACPI_DEBUG_PRINT((ACPI_DB_INFO, > - "found %d brightness levels\n", > - br->count - ACPI_VIDEO_FIRST_LEVEL)); > + acpi_handle_debug(device->dev->handle, "found %d brightness levels\n", > + br->count - ACPI_VIDEO_FIRST_LEVEL); > + > return 0; > > out_free_levels: > @@ -1023,7 +1019,8 @@ static void acpi_video_device_find_cap(s > if (acpi_has_method(device->dev->handle, "_BQC")) { > device->cap._BQC = 1; > } else if (acpi_has_method(device->dev->handle, "_BCQ")) { > - printk(KERN_WARNING FW_BUG "_BCQ is used instead of _BQC\n"); > + acpi_handle_info(device->dev->handle, > + "_BCQ is used instead of _BQC\n"); > device->cap._BCQ = 1; > } > > @@ -1083,8 +1080,7 @@ static int acpi_video_bus_check(struct a > /* Does this device support video switching? */ > if (video->cap._DOS || video->cap._DOD) { > if (!video->cap._DOS) { > - printk(KERN_WARNING FW_BUG > - "ACPI(%s) defines _DOD but not _DOS\n", > + pr_info(FW_BUG "ACPI(%s) defines _DOD but not _DOS\n", > acpi_device_bid(video->device)); > } > video->flags.multihead = 1; > @@ -1272,7 +1268,8 @@ acpi_video_device_bind(struct acpi_video > ids = &video->attached_array[i]; > if (device->device_id == (ids->value.int_val & 0xffff)) { > ids->bind_info = device; > - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "device_bind %d\n", i)); > + acpi_handle_debug(video->device->handle, "%s: %d\n", > + __func__, i); > } > } > } > @@ -1324,20 +1321,22 @@ static int acpi_video_device_enumerate(s > return AE_NOT_EXIST; > > status = acpi_evaluate_object(video->device->handle, "_DOD", NULL, &buffer); > - if (!ACPI_SUCCESS(status)) { > - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _DOD")); > + if (ACPI_FAILURE(status)) { > + acpi_handle_info(video->device->handle, > + "_DOD evaluation failed: %s\n", > + acpi_format_exception(status)); > return status; > } > > dod = buffer.pointer; > if (!dod || (dod->type != ACPI_TYPE_PACKAGE)) { > - ACPI_EXCEPTION((AE_INFO, status, "Invalid _DOD data")); > + acpi_handle_info(video->device->handle, "Invalid _DOD data\n"); > status = -EFAULT; > goto out; > } > > - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %d video heads in _DOD\n", > - dod->package.count)); > + acpi_handle_debug(video->device->handle, "Found %d video heads in _DOD\n", > + dod->package.count); > > active_list = kcalloc(1 + dod->package.count, > sizeof(struct acpi_video_enumerated_device), > @@ -1352,15 +1351,18 @@ static int acpi_video_device_enumerate(s > obj = &dod->package.elements[i]; > > if (obj->type != ACPI_TYPE_INTEGER) { > - printk(KERN_ERR PREFIX > - "Invalid _DOD data in element %d\n", i); > + acpi_handle_info(video->device->handle, > + "Invalid _DOD data in element %d\n", i); > continue; > } > > active_list[count].value.int_val = obj->integer.value; > active_list[count].bind_info = NULL; > - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "dod element[%d] = %d\n", i, > - (int)obj->integer.value)); > + > + acpi_handle_debug(video->device->handle, > + "_DOD element[%d] = %d\n", i, > + (int)obj->integer.value); > + > count++; > } > > @@ -1451,7 +1453,8 @@ acpi_video_switch_brightness(struct work > > out: > if (result) > - printk(KERN_ERR PREFIX "Failed to switch the brightness\n"); > + acpi_handle_info(device->dev->handle, > + "Failed to switch brightness\n"); > } > > int acpi_video_get_edid(struct acpi_device *device, int type, int device_id, > @@ -1601,8 +1604,8 @@ static void acpi_video_bus_notify(struct > break; > > default: > - ACPI_DEBUG_PRINT((ACPI_DB_INFO, > - "Unsupported event [0x%x]\n", event)); > + acpi_handle_debug(device->handle, "Unsupported event [0x%x]\n", > + event); > break; > } > > @@ -1675,8 +1678,7 @@ static void acpi_video_device_notify(acp > keycode = KEY_DISPLAY_OFF; > break; > default: > - ACPI_DEBUG_PRINT((ACPI_DB_INFO, > - "Unsupported event [0x%x]\n", event)); > + acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event); > break; > } > > @@ -1812,11 +1814,12 @@ static void acpi_video_dev_register_back > &device->cooling_dev->device.kobj, > "thermal_cooling"); > if (result) > - printk(KERN_ERR PREFIX "Create sysfs link\n"); > + pr_info("sysfs link creation failed\n"); > + > result = sysfs_create_link(&device->cooling_dev->device.kobj, > &device->dev->dev.kobj, "device"); > if (result) > - printk(KERN_ERR PREFIX "Create sysfs link\n"); > + pr_info("Reverse sysfs link creation failed\n"); > } > > static void acpi_video_run_bcl_for_osi(struct acpi_video_bus *video) > @@ -2030,7 +2033,7 @@ static int acpi_video_bus_add(struct acp > acpi_video_bus_match, NULL, > device, NULL); > if (status == AE_ALREADY_EXISTS) { > - printk(KERN_WARNING FW_BUG > + pr_info(FW_BUG > "Duplicate ACPI video bus devices for the" > " same VGA controller, please try module " > "parameter \"video.allow_duplicates=1\"" > @@ -2073,7 +2076,7 @@ static int acpi_video_bus_add(struct acp > if (error) > goto err_put_video; > > - printk(KERN_INFO PREFIX "%s [%s] (multi-head: %s rom: %s post: %s)\n", > + pr_info("%s [%s] (multi-head: %s rom: %s post: %s)\n", > ACPI_VIDEO_DEVICE_NAME, acpi_device_bid(device), > video->flags.multihead ? "yes" : "no", > video->flags.rom ? "yes" : "no", > Index: linux-pm/Documentation/firmware-guide/acpi/debug.rst > =================================================================== > --- linux-pm.orig/Documentation/firmware-guide/acpi/debug.rst > +++ linux-pm/Documentation/firmware-guide/acpi/debug.rst > @@ -59,7 +59,6 @@ shows the supported mask values, current > ACPI_SYSTEM_COMPONENT 0x02000000 > ACPI_THERMAL_COMPONENT 0x04000000 > ACPI_MEMORY_DEVICE_COMPONENT 0x08000000 > - ACPI_VIDEO_COMPONENT 0x10000000 > ACPI_PROCESSOR_COMPONENT 0x20000000 > > debug_level > Index: linux-pm/drivers/acpi/sysfs.c > =================================================================== > --- linux-pm.orig/drivers/acpi/sysfs.c > +++ linux-pm/drivers/acpi/sysfs.c > @@ -59,7 +59,6 @@ static const struct acpi_dlayer acpi_deb > ACPI_DEBUG_INIT(ACPI_SYSTEM_COMPONENT), > ACPI_DEBUG_INIT(ACPI_THERMAL_COMPONENT), > ACPI_DEBUG_INIT(ACPI_MEMORY_DEVICE_COMPONENT), > - ACPI_DEBUG_INIT(ACPI_VIDEO_COMPONENT), > ACPI_DEBUG_INIT(ACPI_PROCESSOR_COMPONENT), > }; > > Index: linux-pm/include/acpi/acpi_drivers.h > =================================================================== > --- linux-pm.orig/include/acpi/acpi_drivers.h > +++ linux-pm/include/acpi/acpi_drivers.h > @@ -22,7 +22,6 @@ > #define ACPI_SYSTEM_COMPONENT 0x02000000 > #define ACPI_THERMAL_COMPONENT 0x04000000 > #define ACPI_MEMORY_DEVICE_COMPONENT 0x08000000 > -#define ACPI_VIDEO_COMPONENT 0x10000000 > #define ACPI_PROCESSOR_COMPONENT 0x20000000 > > /* > > >