Re: [PATCH v7 2/3] PCI: mobiveil: Add Mobiveil PCIe Host Bridge IP driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 24, 2018 at 10:29:55AM -0400, Subrahmanya Lingappa wrote:
> Adds driver for Mobiveil AXI PCIe Host Bridge Soft IP
>  - GPEX 4.0, a PCIe gen4 IP. This IP has upto 8 outbound and inbound windows
>  for the address translation.
> 
> Signed-off-by: Subrahmanya Lingappa <l.subrahmanya@xxxxxxxxxxxxxx>
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
> Cc: linux-pci@xxxxxxxxxxxxxxx
> Cc: devicetree@xxxxxxxxxxxxxxx
> ---
>  drivers/pci/host/pcie-mobiveil.c | 692 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 692 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-mobiveil.c
> 
> diff --git a/drivers/pci/host/pcie-mobiveil.c b/drivers/pci/host/pcie-mobiveil.c
> new file mode 100644
> index 0000000..28f488b
> --- /dev/null
> +++ b/drivers/pci/host/pcie-mobiveil.c
> @@ -0,0 +1,692 @@
> +/*
> + * PCIe host controller driver for Mobiveil PCIe Host controller
> + *
> + * SPDX-License-Identifier: GPL-2.0

Incorrect usage of SPDX tag, see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/license-rules.rst

(Must be first line of file, should use "//" comment in C source)

> + * Copyright (c) 2017 Mobiveil Inc.

It's 2018 already, so use 2018 here instead of 2017 in this patch and
then updating it in the next.

> +/* supported number of interrupts */
> +#define PCI_NUM_INTX    4

This is already defined in include/linux/pci.h; you don't need another
one.

> +static inline u32 csr_readl(struct mobiveil_pcie *pcie,	const u32 reg)

Should be space, not tab, after ","

> +/*
> + * Routine to check the downstream link status, its recommended to
> + * check the link status before accessing downstream config resources.

Checking the link status before accessing downstream config resources
is racy because the link may go down after you check the status but
before you actually do the access.

> + */
> +static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie)
> +{
> +	return (csr_readl(pcie, LTSSM_STATUS) &
> +		LTSSM_STATUS_L0_MASK) == LTSSM_STATUS_L0;
> +}

> +static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus,
> +					unsigned int devfn, int where)
> +{
> +	struct mobiveil_pcie *pcie = bus->sysdata;
> +
> +	if (!mobiveil_pcie_valid_device(bus, devfn))
> +		return NULL;
> +
> +	if (bus->number == pcie->root_bus_nr) {
> +		/* RC config access */
> +		return pcie->csr_axi_slave_base + where;
> +	} else {

No "else" needed, since the "if" body always returns.  Then you can
unindent the block below.

> +		/*
> +		 * EP config access (in Config/APIO space)
> +		 * Program PEX Address base (31..16 bits) with appropriate value
> +		 * (BDF) in PAB_AXI_AMAP_PEX_WIN_L0 Register.
> +		 * Relies on pci_lock serialization
> +		 */
> +		csr_writel(pcie,
> +			bus->number << PAB_BUS_SHIFT |
> +			PCI_SLOT(devfn) << PAB_DEVICE_SHIFT |
> +			PCI_FUNC(devfn) << PAB_FUNCTION_SHIFT,
> +			PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));
> +		return pcie->config_axi_slave_base + where;
> +	}

> +/*
> + * select_paged_register - routine to access paged register of root complex
> + *
> + * registers of RC are paged, for this scheme to work
> + * extracted higher 6 bits of the offset will be written to pg_sel
> + * field of PAB_CTRL register and rest of the lower 10 bits enabled with
> + * PAGE_SEL_EN are used as offset of the register.
> + *

Remove the extra blank line at the end of the comment.

> + */
> +static void select_paged_register(struct mobiveil_pcie *pcie, u32 offset)

> +	value = csr_readl(pcie, PCI_COMMAND);
> +	csr_writel(pcie, value | PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
> +		PCI_COMMAND_MASTER,	PCI_COMMAND);

Use space, not tab, after the comma.

> +	/* program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in
> +	 * PAB_AXI_PIO_CTRL Register
> +	 */

Wrong comment style, needs:

  /*
   * Program PIO ...
   * ...
   */

Bjorn



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux