| Message ID | 1600328345-27627-1-git-send-email-guohanjun@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ACPI: First step to decouple ACPICA debug functionality from ACPI driver | expand |
Hi Hanjun, On Thu, Sep 17, 2020 at 10:34 AM Hanjun Guo <guohanjun@huawei.com> wrote: > > For now, ACPI driver debug functionality is mixed of pr_* functions and > ACPI_DEBUG_PRINT() which is provided ACPICA core directly, ACPICA debug > functions are not friendly for users and also make ACPICA core deeply > coupled with ACPI drivers. > > With the evolution of the ACPI driver code, lots of the ACPICA debug > functions used in ACPI drivers were removed away, this makes the ACPICA > debug in ACPI driver to be fragile, for example, some of the COMPONENT > such as ACPI_CONTAINER_COMPONENT and ACPI_MEMORY_DEVICE_COMPONENT are not > used anymore, they leaved as dead code. > > From another aspert, removing the ACPICA debug functions didn't raise > concerns in the past, so I believe the ACPICA debug in ACPI driver can be > removed and replace with equivalent pr_* debug functions, then decouple > ACPICA debug functionality from ACPI driver. This is a worthy goal, but the patch series appears to be a mixed bag of changes some of which are not directly related to this goal. > In order to decouple ACPICA debug functionality from ACPI driver, I do it > in two steps: > - Remove the dead ACPICA functionality code, and remove the not used > COMPONENT; > - Remove all the ACPICA debug code from ACPI drivers. > > This patch set is the first step to decouple ACPICA debug functionality > from ACPI driver, just remove the dead ACPICA functionality code and > some cleanups for ACPI drivers, should no functional change if you don't > apply the last two patches. > > Patch 1/25 ~ patch 23/25 are removing the dead code and cleanups; > Patch 24/25 ~ patch 25/25 are the actual ABI change. > > If the ABI change is making sense, I will go further to remove the > ACPICA debug functionality from ACPI driver, just keep it inside > the ACPICA core. > > Hanjun Guo (25): > ACPI: cmos_rtc: Remove the ACPI_MODULE_NAME() This, for example, should be a separate cleanup patch. > ACPI: configfs: Decouple with ACPICA > ACPI: configfs: Add the missing config_item_put() This appears to be a fix that should go in separate from the rest of the series. > ACPI: debug: Remove the not used function Another separate cleanup. > ACPI: LPSS: Remove the ACPI_MODULE_NAME() Yet another one. So can you please split up the patch set into several smaller and more manageable ones? Thanks!
Hi Rafael, On 2020/9/17 23:08, Rafael J. Wysocki wrote: > Hi Hanjun, > > On Thu, Sep 17, 2020 at 10:34 AM Hanjun Guo <guohanjun@huawei.com> wrote: >> >> For now, ACPI driver debug functionality is mixed of pr_* functions and >> ACPI_DEBUG_PRINT() which is provided ACPICA core directly, ACPICA debug >> functions are not friendly for users and also make ACPICA core deeply >> coupled with ACPI drivers. >> >> With the evolution of the ACPI driver code, lots of the ACPICA debug >> functions used in ACPI drivers were removed away, this makes the ACPICA >> debug in ACPI driver to be fragile, for example, some of the COMPONENT >> such as ACPI_CONTAINER_COMPONENT and ACPI_MEMORY_DEVICE_COMPONENT are not >> used anymore, they leaved as dead code. >> >> From another aspert, removing the ACPICA debug functions didn't raise >> concerns in the past, so I believe the ACPICA debug in ACPI driver can be >> removed and replace with equivalent pr_* debug functions, then decouple >> ACPICA debug functionality from ACPI driver. > > This is a worthy goal, but the patch series appears to be a mixed bag > of changes some of which are not directly related to this goal. Sorry for that, I sent this patch set in a hurry, I will update as you suggested. > >> In order to decouple ACPICA debug functionality from ACPI driver, I do it >> in two steps: >> - Remove the dead ACPICA functionality code, and remove the not used >> COMPONENT; >> - Remove all the ACPICA debug code from ACPI drivers. >> >> This patch set is the first step to decouple ACPICA debug functionality >> from ACPI driver, just remove the dead ACPICA functionality code and >> some cleanups for ACPI drivers, should no functional change if you don't >> apply the last two patches. >> >> Patch 1/25 ~ patch 23/25 are removing the dead code and cleanups; >> Patch 24/25 ~ patch 25/25 are the actual ABI change. >> >> If the ABI change is making sense, I will go further to remove the >> ACPICA debug functionality from ACPI driver, just keep it inside >> the ACPICA core. >> >> Hanjun Guo (25): >> ACPI: cmos_rtc: Remove the ACPI_MODULE_NAME() > > This, for example, should be a separate cleanup patch. ACPI_MODULE_NAME() and _COMPONENT are both used for ACPICA debug functionality, so I will put them in the decouple patch set. > >> ACPI: configfs: Decouple with ACPICA >> ACPI: configfs: Add the missing config_item_put() > > This appears to be a fix that should go in separate from the rest of the series. Will send a fix first! > >> ACPI: debug: Remove the not used function > > Another separate cleanup. > >> ACPI: LPSS: Remove the ACPI_MODULE_NAME() > > Yet another one. > > So can you please split up the patch set into several smaller and more > manageable ones? Will do, thanks for your comments! Thanks Hanjun
On Fri, Sep 18, 2020 at 3:55 AM Hanjun Guo <guohanjun@huawei.com> wrote: > > Hi Rafael, > > On 2020/9/17 23:08, Rafael J. Wysocki wrote: > > Hi Hanjun, > > > > On Thu, Sep 17, 2020 at 10:34 AM Hanjun Guo <guohanjun@huawei.com> wrote: > >> > >> For now, ACPI driver debug functionality is mixed of pr_* functions and > >> ACPI_DEBUG_PRINT() which is provided ACPICA core directly, ACPICA debug > >> functions are not friendly for users and also make ACPICA core deeply > >> coupled with ACPI drivers. > >> > >> With the evolution of the ACPI driver code, lots of the ACPICA debug > >> functions used in ACPI drivers were removed away, this makes the ACPICA > >> debug in ACPI driver to be fragile, for example, some of the COMPONENT > >> such as ACPI_CONTAINER_COMPONENT and ACPI_MEMORY_DEVICE_COMPONENT are not > >> used anymore, they leaved as dead code. > >> > >> From another aspert, removing the ACPICA debug functions didn't raise > >> concerns in the past, so I believe the ACPICA debug in ACPI driver can be > >> removed and replace with equivalent pr_* debug functions, then decouple > >> ACPICA debug functionality from ACPI driver. > > > > This is a worthy goal, but the patch series appears to be a mixed bag > > of changes some of which are not directly related to this goal. > > Sorry for that, I sent this patch set in a hurry, I will update > as you suggested. > > > > >> In order to decouple ACPICA debug functionality from ACPI driver, I do it > >> in two steps: > >> - Remove the dead ACPICA functionality code, and remove the not used > >> COMPONENT; > >> - Remove all the ACPICA debug code from ACPI drivers. > >> > >> This patch set is the first step to decouple ACPICA debug functionality > >> from ACPI driver, just remove the dead ACPICA functionality code and > >> some cleanups for ACPI drivers, should no functional change if you don't > >> apply the last two patches. > >> > >> Patch 1/25 ~ patch 23/25 are removing the dead code and cleanups; > >> Patch 24/25 ~ patch 25/25 are the actual ABI change. > >> > >> If the ABI change is making sense, I will go further to remove the > >> ACPICA debug functionality from ACPI driver, just keep it inside > >> the ACPICA core. > >> > >> Hanjun Guo (25): > >> ACPI: cmos_rtc: Remove the ACPI_MODULE_NAME() > > > > This, for example, should be a separate cleanup patch. > > ACPI_MODULE_NAME() and _COMPONENT are both used for ACPICA > debug functionality, so I will put them in the decouple > patch set. So if the ACPICA debug functionality is not used in the given C file, you can drop these macros from there right away without any side effects. Why don't you do that in a separate series of patches then? Thanks!
Hi Rafael, On 2020/9/18 21:34, Rafael J. Wysocki wrote: > On Fri, Sep 18, 2020 at 3:55 AM Hanjun Guo <guohanjun@huawei.com> wrote: >> >> Hi Rafael, >> >> On 2020/9/17 23:08, Rafael J. Wysocki wrote: >>> Hi Hanjun, >>> >>> On Thu, Sep 17, 2020 at 10:34 AM Hanjun Guo <guohanjun@huawei.com> wrote: >>>> >>>> For now, ACPI driver debug functionality is mixed of pr_* functions and >>>> ACPI_DEBUG_PRINT() which is provided ACPICA core directly, ACPICA debug >>>> functions are not friendly for users and also make ACPICA core deeply >>>> coupled with ACPI drivers. >>>> >>>> With the evolution of the ACPI driver code, lots of the ACPICA debug >>>> functions used in ACPI drivers were removed away, this makes the ACPICA >>>> debug in ACPI driver to be fragile, for example, some of the COMPONENT >>>> such as ACPI_CONTAINER_COMPONENT and ACPI_MEMORY_DEVICE_COMPONENT are not >>>> used anymore, they leaved as dead code. >>>> >>>> From another aspert, removing the ACPICA debug functions didn't raise >>>> concerns in the past, so I believe the ACPICA debug in ACPI driver can be >>>> removed and replace with equivalent pr_* debug functions, then decouple >>>> ACPICA debug functionality from ACPI driver. >>> >>> This is a worthy goal, but the patch series appears to be a mixed bag >>> of changes some of which are not directly related to this goal. >> >> Sorry for that, I sent this patch set in a hurry, I will update >> as you suggested. >>>> >>>> Hanjun Guo (25): >>>> ACPI: cmos_rtc: Remove the ACPI_MODULE_NAME() >>> >>> This, for example, should be a separate cleanup patch. >> >> ACPI_MODULE_NAME() and _COMPONENT are both used for ACPICA >> debug functionality, so I will put them in the decouple >> patch set. > > So if the ACPICA debug functionality is not used in the given C file, > you can drop these macros from there right away without any side > effects. > > Why don't you do that in a separate series of patches then? Good point, so I will split this patch set into four parts: - The bugfix patch goes in separate from the rest of the series, already sent out. - Cleanup patches which is not related to ACPICA debug, - ACPI: debug: Remove the not used function - ACPI: memhotplug: Remove the state for memory device - ACPI: processor: Remove the duplicated ACPI_PROCESSOR_CLASS macro - ACPI: SBS: Simplify the driver init code - ACPI: SBS: Simplify the code using module_acpi_driver() - ACPI: tiny-power-button: Simplify the code using module_acpi_driver() - A patch set removing all the leftover ACPICA debug functionality which is not used in the C file, no functional change. - A patch set for actual ABI change (RFC). Do it make sense to you? Thanks Hanjun