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

[Nathan Whitehorn]

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
>
>
>