svn commit: r336130 - head/sys/dev/pci

[Wojciech Macek]

We will fix it, sorry for troubles.

@wma

2018-07-09 18:10 GMT+02:00 Nathan Whitehorn <nwhitehorn at freebsd.org>:

> Maybe this file should be renamed and/or split anyway? It's not actually
> very generic and is used only on (some) ARM systems.
> -Nathan
>
>
> On 07/09/18 09:00, Andrew Turner wrote:
>
>> This breaks at least armv7. Please either fix, or revert this.
>>
>> I have some comments below I was planning on adding to the review, but
>> you committed before I had the time to do so.
>>
>> On 9 Jul 2018, at 10:00, Wojciech Macek <wma at freebsd.org> wrote:
>>>
>>> Author: wma
>>> Date: Mon Jul  9 09:00:16 2018
>>> New Revision: 336130
>>> URL: https://svnweb.freebsd.org/changeset/base/336130
>>>
>>> Log:
>>>   ARM64: Add quirk mechanism to pci_host_generic_acpi
>>>
>>>   Add few quirks which are necessary to use AHCI on ThX2
>>>
>>>   Submitted by:          Patryk Duda <pdk at semihalf.com>
>>>   Obtained from:         Semihalf
>>>   Sponsored by:          Cavium
>>>   Differential revision: https://reviews.freebsd.org/D15929
>>>
>>> Modified:
>>>   head/sys/dev/pci/pci_host_generic.c
>>>
>>> Modified: head/sys/dev/pci/pci_host_generic.c
>>> ============================================================
>>> ==================
>>> --- head/sys/dev/pci/pci_host_generic.c Mon Jul  9 08:55:07 2018
>>> (r336129)
>>> +++ head/sys/dev/pci/pci_host_generic.c Mon Jul  9 09:00:16 2018
>>> (r336130)
>>> @@ -69,6 +69,25 @@ __FBSDID("$FreeBSD$");
>>>         (((func) & PCIE_FUNC_MASK) << PCIE_FUNC_SHIFT)  |       \
>>>         ((reg) & PCIE_REG_MASK))
>>>
>>> +typedef void (*pci_host_generic_quirk_function)(device_t);
>>> +
>>> +struct pci_host_generic_quirk_entry {
>>> +       int impl;
>>> +       int part;
>>> +       int var;
>>> +       int rev;
>>> +       pci_host_generic_quirk_function func;
>>> +};
>>> +
>>> +struct pci_host_generic_block_entry {
>>> +       int impl;
>>> +       int part;
>>> +       int var;
>>> +       int rev;
>>> +       int bus;
>>> +       int slot;
>>> +};
>>> +
>>> /* Forward prototypes */
>>>
>>> static uint32_t generic_pcie_read_config(device_t dev, u_int bus, u_int
>>> slot,
>>> @@ -80,7 +99,22 @@ static int generic_pcie_read_ivar(device_t dev,
>>> device
>>>      uintptr_t *result);
>>> static int generic_pcie_write_ivar(device_t dev, device_t child, int
>>> index,
>>>      uintptr_t value);
>>> +static void pci_host_generic_apply_quirks(device_t);
>>> +static void thunderx2_ahci_bar_quirk(device_t);
>>>
>>> +struct pci_host_generic_quirk_entry pci_host_generic_quirks[] =
>>> +{
>>> +       {CPU_IMPL_CAVIUM, CPU_PART_THUNDERX2, 0, 0,
>>> thunderx2_ahci_bar_quirk},
>>> +       {0, 0, 0, 0, NULL}
>>>
>> This breaks non-arm64, please fix.
>>
>> +};
>>> +
>>> +struct pci_host_generic_block_entry pci_host_generic_blocked[] =
>>> +{
>>> +       /* ThunderX2 AHCI on second socket */
>>> +       {CPU_IMPL_CAVIUM, CPU_PART_THUNDERX2, 0, 0, 0x80, 0x10},
>>> +       {0, 0, 0, 0, 0, 0}
>>>
>> And this.
>>
>> +};
>>> +
>>> int
>>> pci_host_generic_core_attach(device_t dev)
>>> {
>>> @@ -134,9 +168,30 @@ pci_host_generic_core_attach(device_t dev)
>>>                 return (error);
>>>         }
>>>
>>> +       pci_host_generic_apply_quirks(dev);
>>> +
>>>         return (0);
>>> }
>>>
>>> +static void
>>> +pci_host_generic_apply_quirks(device_t dev)
>>> +{
>>> +       struct pci_host_generic_quirk_entry *quirk;
>>> +
>>> +       quirk = pci_host_generic_quirks;
>>> +       while (1) {
>>> +               if (quirk->impl == 0)
>>> +                       break;
>>> +
>>> +               if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK,
>>> +                   quirk->impl, quirk->part, quirk->var, quirk->rev) &&
>>> +                   quirk->func != NULL)
>>>
>> This is arm64 specific.
>>
>> +                       quirk->func(dev);
>>> +
>>> +               quirk++;
>>> +       }
>>> +}
>>> +
>>> static uint32_t
>>> generic_pcie_read_config(device_t dev, u_int bus, u_int slot,
>>>      u_int func, u_int reg, int bytes)
>>> @@ -146,11 +201,25 @@ generic_pcie_read_config(device_t dev, u_int bus,
>>> u_in
>>>         bus_space_tag_t t;
>>>         uint64_t offset;
>>>         uint32_t data;
>>> +       struct pci_host_generic_block_entry *block;
>>>
>>>         if ((bus > PCI_BUSMAX) || (slot > PCI_SLOTMAX) ||
>>>             (func > PCI_FUNCMAX) || (reg > PCIE_REGMAX))
>>>                 return (~0U);
>>>
>>> +       block = pci_host_generic_blocked;
>>> +       while (1) {
>>> +               if (block->impl == 0)
>>> +                       break;
>>> +
>>> +               if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK,
>>> +                   block->impl, block->part, block->var, block->rev) &&
>>> +                   block->bus == bus && block->slot == slot)
>>>
>> This is also arm64 specific.
>>
>> +                       return (~0);
>>> +
>>> +               block++;
>>> +       }
>>> +
>>>         sc = device_get_softc(dev);
>>>
>>>         offset = PCIE_ADDR_OFFSET(bus, slot, func, reg);
>>> @@ -392,3 +461,20 @@ static device_method_t generic_pcie_methods[] = {
>>>
>>> DEFINE_CLASS_0(pcib, generic_pcie_core_driver,
>>>      generic_pcie_methods, sizeof(struct generic_pcie_core_softc));
>>> +
>>> +static void thunderx2_ahci_bar_quirk(device_t dev)
>>>
>> This should be dependent on arm64 and an appropriate SOC_ check.
>>
>> +{
>>> +
>>> +       /*
>>> +        * XXX:
>>> +        * On ThunderX2, AHCI BAR2 address is wrong. It needs to
>>> precisely
>>> +        * match the one described in datasheet. Fixup it
>>> unconditionally.
>>> +        */
>>> +       if (device_get_unit(dev) == 0) {
>>> +               device_printf(dev, "running AHCI BAR fixup\n");
>>> +               PCIB_WRITE_CONFIG(dev, 0, 16, 0, 0x18, 0x01440000, 4);
>>> +               PCIB_WRITE_CONFIG(dev, 0, 16, 0, 0x1c, 0x40, 4);
>>> +               PCIB_WRITE_CONFIG(dev, 0, 16, 1, 0x18, 0x01450000, 4);
>>> +               PCIB_WRITE_CONFIG(dev, 0, 16, 1, 0x1c, 0x40, 4);
>>> +       }
>>> +}
>>>
>> Andrew
>>
>>
>>
>>
>