Re: [PATCH v3 2/7] ACPI: Enable PPTT support on ARM64

From: Lorenzo Pieralisi
Date: Thu Oct 19 2017 - 05:13:05 EST


On Wed, Oct 18, 2017 at 12:38:46PM -0500, Jeremy Linton wrote:
> On 10/18/2017 11:47 AM, Lorenzo Pieralisi wrote:
> >On Thu, Oct 12, 2017 at 02:48:51PM -0500, Jeremy Linton wrote:
> >>Now that we have a PPTT parser, in preparation for its use
> >>on arm64, lets build it.
> >>
> >>Signed-off-by: Jeremy Linton <jeremy.linton@xxxxxxx>
> >>---
> >> arch/arm64/Kconfig | 1 +
> >> drivers/acpi/Makefile | 1 +
> >> drivers/acpi/arm64/Kconfig | 3 +++
> >> 3 files changed, 5 insertions(+)
> >>
> >>diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> >>index 0df64a6a56d4..68c9d1289735 100644
> >>--- a/arch/arm64/Kconfig
> >>+++ b/arch/arm64/Kconfig
> >>@@ -7,6 +7,7 @@ config ARM64
> >> select ACPI_REDUCED_HARDWARE_ONLY if ACPI
> >> select ACPI_MCFG if ACPI
> >> select ACPI_SPCR_TABLE if ACPI
> >>+ select ACPI_PPTT if ACPI
> >> select ARCH_CLOCKSOURCE_DATA
> >> select ARCH_HAS_DEBUG_VIRTUAL
> >> select ARCH_HAS_DEVMEM_IS_ALLOWED
> >>diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> >>index 90265ab4437a..c92a0c937551 100644
> >>--- a/drivers/acpi/Makefile
> >>+++ b/drivers/acpi/Makefile
> >>@@ -85,6 +85,7 @@ obj-$(CONFIG_ACPI_BGRT) += bgrt.o
> >> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
> >> obj-$(CONFIG_ACPI_SPCR_TABLE) += spcr.o
> >> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
> >>+obj-$(CONFIG_ACPI_PPTT) += pptt.o
> >> # processor has its own "processor." module_param namespace
> >> processor-y := processor_driver.o
> >>diff --git a/drivers/acpi/arm64/Kconfig b/drivers/acpi/arm64/Kconfig
> >>index 5a6f80fce0d6..74b855a669ea 100644
> >>--- a/drivers/acpi/arm64/Kconfig
> >>+++ b/drivers/acpi/arm64/Kconfig
> >>@@ -7,3 +7,6 @@ config ACPI_IORT
> >> config ACPI_GTDT
> >> bool
> >>+
> >>+config ACPI_PPTT
> >>+ bool
> >>\ No newline at end of file
> >
> >I do not understand the logic. Why should we have a Kconfig option
> >in drivers/acpi/arm64 for code in drivers/acpi ?
> >
> >AFAIK PPTT is not an ACPI ARM64 specific binding.
>
> Weird hu? Originally I had the whole shebang in arm64 because the
> x86 (or whatever) bindings have not been written. My assumption is
> that once that part had been provided it could be moved.

Which part ? I asked because AFAICS the bindings are completely
generic (and are meant to be so).

> The config is sort of an artifact and "easier" to move than the
> whole file. But, as Hanjun has also been complaining about it I've
> agreed to move it to the "correct" location but keep it in the arm64
> wrapper. Of course I think that is a bit strange too, but
> whatever...

I do not want to cavil but either you have Kconfig and code in
drivers/acpi or drivers/acpi/arm64 - I would not understand a
mix of the two.

To reiterate the point, PPTT is not an ARM64 specific binding so
IMO it does not belong in drivers/acpi/arm64.

> Once the arm64 side of things are all wrapped up (and I can come up
> for some air) I willing to help with bindings on other architectures
> if anyone is truly interested. But, I view that whole exercise as
> more a "bug" fixing one than providing any real benefit at this
> point.

Please define "bindings on other architectures" because I do not
understand what you mean.

Thanks,
Lorenzo