Re: [Linaro-acpi] [PATCH V6 12/13] pci, pci-thunder-ecam: Add ACPI support for ThunderX ECAM.

G Gregory
2016-04-19T10:41:30Z

On 19 April 2016 at 11:26, Tomasz Nowicki <tn@semihalf.com> wrote:
> On 15.04.2016 19:06, Tomasz Nowicki wrote:
>>
>> Passes 1.x miss PCI enhanced allocation (EA) header for fixed-BARs,
>> thus these passes should use Cavium-specific config access functions
>> that synthesize the missing EA capabilities.
>>
>> We already have DT driver which addresses errata requirements and
>> allows to use special PCI config accessors. Currently this driver uses
>> compatible = "cavium,pci-host-thunder-ecam" to mach against the errata.
>> For ACPI case we need explicit errata number and corresponding
>> DECLARE_ACPI_MCFG_FIXUP fixup code.
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>> arch/arm64/Kconfig | 14 ++++++++++++++
>> arch/arm64/include/asm/cpufeature.h | 3 ++-
>> arch/arm64/kernel/cpu_errata.c | 8 ++++++++
>> drivers/pci/host/pci-thunder-ecam.c | 35
>> +++++++++++++++++++++++++++++++++++
>> 4 files changed, 59 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 1bded87..b7614b8 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -445,6 +445,20 @@ config CAVIUM_ERRATUM_27456
>>
>> If unsure, say Y.
>>
>> +config CAVIUM_ERRATUM_24575
>> + bool "Cavium erratum 24575: Enhanced Allocation (EA) emualaion"
>> + default y
>> + help
>> + Enable workaround for erratum 24575.
>> +
>> + Early versions of the Cavium Thunder CN88XX processor are
>> missing
>> + Enhanced Allocation (EA) capabilities for the fixed BAR
>> addresses used
>> + by the on-SoC hardware blocks. The erratum adds config access
>> + functions that synthesize the missing EA capabilities for
>> versions
>> + that are missing that information.
>> +
>> + If unsure, say Y.
>> +
>> endmenu
>>
>>
>> diff --git a/arch/arm64/include/asm/cpufeature.h
>> b/arch/arm64/include/asm/cpufeature.h
>> index b9b6494..a78364e 100644
>> --- a/arch/arm64/include/asm/cpufeature.h
>> +++ b/arch/arm64/include/asm/cpufeature.h
>> @@ -35,8 +35,9 @@
>> #define ARM64_ALT_PAN_NOT_UAO 10
>> #define ARM64_HAS_VIRT_HOST_EXTN 11
>> #define ARM64_WORKAROUND_CAVIUM_27456 12
>> +#define ARM64_WORKAROUND_CAVIUM_24575 13
>>
>> -#define ARM64_NCAPS 13
>> +#define ARM64_NCAPS 14
>>
>> #ifndef __ASSEMBLY__
>>
>> diff --git a/arch/arm64/kernel/cpu_errata.c
>> b/arch/arm64/kernel/cpu_errata.c
>> index 06afd04..89c13d7 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -97,6 +97,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>> (1 << MIDR_VARIANT_SHIFT) | 1),
>> },
>> #endif
>> +#ifdef CONFIG_CAVIUM_ERRATUM_24575
>> + {
>> + /* Cavium ThunderX, pass 1.x */
>> + .desc = "Cavium erratum 24575",
>> + .capability = ARM64_WORKAROUND_CAVIUM_24575,
>> + MIDR_RANGE(MIDR_THUNDERX, 0x00, 0x01),
>> + },
>> +#endif
>> {
>> }
>> };
>> diff --git a/drivers/pci/host/pci-thunder-ecam.c
>> b/drivers/pci/host/pci-thunder-ecam.c
>> index f67c6d7..01de697 100644
>> --- a/drivers/pci/host/pci-thunder-ecam.c
>> +++ b/drivers/pci/host/pci-thunder-ecam.c
>> @@ -11,10 +11,14 @@
>> #include <linux/ioport.h>
>> #include <linux/of_pci.h>
>> #include <linux/of.h>
>> +#include <linux/pci-acpi.h>
>> #include <linux/platform_device.h>
>>
>> +#include <asm/virt.h>
>> +
>> #include "../ecam.h"
>>
>> +
>> static void set_val(u32 v, int where, int size, u32 *val)
>> {
>> int shift = (where & 3) * 8;
>> @@ -376,5 +380,36 @@ static struct platform_driver thunder_ecam_driver = {
>> };
>> module_platform_driver(thunder_ecam_driver);
>>
>> +#ifdef CONFIG_ACPI
>> +
>> +static bool needs_cavium_erratum_24575(struct pci_cfg_fixup *fixup,
>> + struct acpi_pci_root *root)
>> +{
>> + /*
>> + * We must match errata code and be hypervisor, quirk does not
>> apply
>> + * for virtual machines.
>> + */
>> + return cpus_have_cap(ARM64_WORKAROUND_CAVIUM_24575) &&
>> + is_hyp_mode_available();
>> +}
>> +
>> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575,
>> &pci_thunder_ecam_ops,
>> + 0, PCI_MCFG_BUS_ANY);
>> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575,
>> &pci_thunder_ecam_ops,
>> + 1, PCI_MCFG_BUS_ANY);
>> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575,
>> &pci_thunder_ecam_ops,
>> + 2, PCI_MCFG_BUS_ANY);
>> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575,
>> &pci_thunder_ecam_ops,
>> + 3, PCI_MCFG_BUS_ANY);
>> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575,
>> &pci_thunder_ecam_ops,
>> + 10, PCI_MCFG_BUS_ANY);
>> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575,
>> &pci_thunder_ecam_ops,
>> + 11, PCI_MCFG_BUS_ANY);
>> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575,
>> &pci_thunder_ecam_ops,
>> + 12, PCI_MCFG_BUS_ANY);
>> +DECLARE_ACPI_MCFG_FIXUP(NULL, needs_cavium_erratum_24575,
>> &pci_thunder_ecam_ops,
>> + 13, PCI_MCFG_BUS_ANY);
>> +#endif
>> +
>
>
> I wonder if we can identify quirk based on _DSD properties, like that:
>
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package (2) {"cavium,pci-host-thunder-ecam", 1},
> }
> })
>
> similar for PEM driver:
>
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package (2) {"cavium,pci-host-thunder-pem", 1},
> }
> })
>
> Do you think it is right thing to do?
>

Well if your adding new properties why not add a Cavium specific _CID?

Graeme