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

[G Gregory]

On 19 April 2016 at 11:26, Tomasz Nowicki <tn@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
>> ---
>>   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