lkml.org 
[lkml]   [2014]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/3] mcb: Add PCI carrier for MEN Chameleon Bus
On Tue, Feb 18, 2014 at 04:34:13PM +0100, Johannes Thumshirn wrote:
> Add support for MCB over PCI devices. Both PCI attached on-board Chameleon FPGAs
> as well as CompactPCI based MCB carrier cards are supported with this driver.
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> ---
> drivers/mcb/Kconfig | 13 ++++++
> drivers/mcb/Makefile | 2 +
> drivers/mcb/mcb-pci.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 123 insertions(+)
> create mode 100644 drivers/mcb/mcb-pci.c
>
> diff --git a/drivers/mcb/Kconfig b/drivers/mcb/Kconfig
> index 9e7d6f5..8b058bc 100644
> --- a/drivers/mcb/Kconfig
> +++ b/drivers/mcb/Kconfig
> @@ -14,3 +14,16 @@ menuconfig MCB
>
> If build as a module, the module is called mcb.ko
>
> +if MCB
> +config MCB_PCI
> + tristate "PCI based MCB carrier"
> + default m if MCB

'if MCB' is redundant (MCB has to be set for this option to even be
available).

[..]
> +++ b/drivers/mcb/mcb-pci.c
> @@ -0,0 +1,108 @@

Copyright/license blurb?

> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/mcb.h>
> +
> +#include "mcb-internal.h"
> +
> +static struct mcb_bus *bus;
> +struct priv {
> + void __iomem *base;
> +};

This seems weird. Why is the bus not part of your private data? Seems
like you're unnecessarily restricting yourself to supporting only one of
these devices at a time...

> +
> +static int mcb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> + struct priv *priv;
> + phys_addr_t mapbase;
> + int ret = 0;

No need to initialize.

> + int num_cells;
> + unsigned long flags;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(struct priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ret = pci_enable_device(pdev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to enable PCI device\n");
> + return -ENODEV;
> + }
> +
> + mapbase = pci_resource_start(pdev, 0);
> + if (!mapbase) {
> + dev_err(&pdev->dev, "No PCI resource\n");
> + goto err_start;
> + }
> +
> + ret = pci_request_region(pdev, 0, KBUILD_MODNAME);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to request PCI BARs\n");
> + goto err_start;
> + }
> +
> + priv->base = pci_iomap(pdev, 0, 0);

Hmm. There should really be a devm_* variant for PCI resources.

[..]
> +
> +static void mcb_pci_remove(struct pci_dev *pdev)
> +{
> + struct priv *priv = pci_get_drvdata(pdev);
> +
> + mcb_release_bus(bus);
> +
> + pci_iounmap(pdev, priv->base);
> + pci_release_region(pdev, 0);
> + pci_disable_device(pdev);
> + pci_set_drvdata(pdev, NULL);
> +}
> +
> +static struct pci_device_id mcb_pci_tbl[] = {

const?

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation


\
 
 \ /
  Last update: 2014-02-19 04:01    [W:0.161 / U:1.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site