linux-mips
[Top] [All Lists]

Re: [PATCH 2/2] gpio/MIPS/OCTEON: Add a driver for OCTEON's on-chip GPIO

To: Florian Fainelli <florian@openwrt.org>
Subject: Re: [PATCH 2/2] gpio/MIPS/OCTEON: Add a driver for OCTEON's on-chip GPIO pins.
From: David Daney <ddaney.cavm@gmail.com>
Date: Fri, 13 Apr 2012 09:17:09 -0700
Cc: David Daney <ddaney.cavm@gmail.com>, Grant Likely <grant.likely@secretlab.ca>, ralf@linux-mips.org, linux-mips@linux-mips.org, Linus Walleij <linus.walleij@stericsson.com>, Rob Herring <rob.herring@calxeda.com>, devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=R3T3qLY3gHsVzkegKlabmG/lo8L6gyQUS9yb/CePuq0=; b=xYPurOwJQNLLvb+yGOitHkq3Mhsk60+OmXfUKf+BemeQJzHAhyFCJn3tuReEPzVjGw 2VPe4MfEz54wgDBIrs1Rsqco2pJbZSpgtAFQFdR4b6hj2P514OeEFZG8g4Rk7zLV+kj0 buCMptu6G3qdYVHHWsdmlbATlqxxNgXfcj+COM7PTdg6b1h/0BCKfXjb9hKIgqYYi+iV 0sN8lWHSCMDDnXGu1nvSQR6yogDLpNZplRoc4ji6c+xmq/5e4z6BbFsLu0OH/PqqDH5u qFk90r+S7ZmLB+epYIP8pAtYDOAC/upRE6IgU/OhfBF/Qcsa54Bso3Ig3nNhMOXcnqMh 9Mng==
In-reply-to: <4F87F868.1080804@openwrt.org>
References: <1334275820-7791-1-git-send-email-ddaney.cavm@gmail.com> <1334275820-7791-3-git-send-email-ddaney.cavm@gmail.com> <4F87F868.1080804@openwrt.org>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Fedora/3.0.10-1.fc12 Thunderbird/3.0.10
On 04/13/2012 02:56 AM, Florian Fainelli wrote:
Hi David,

[...]
+/*
+ * The address offset of the GPIO configuration register for a given
+ * line.
+ */
+static unsigned int bit_cfg_reg(unsigned int gpio)
+{
+ if (gpio< 16)
+ return 8 * gpio;
+ else
+ return 8 * (gpio - 16) + 0x100;
+}

You could explicitely inline this one, though the compiler will
certainly do it by itself.


I always let the compiler decide.


[...]
+
+ if (OCTEON_IS_MODEL(OCTEON_CN66XX) ||
+ OCTEON_IS_MODEL(OCTEON_CN61XX) ||
+ OCTEON_IS_MODEL(OCTEON_CNF71XX))
+ chip->ngpio = 20;
+ else
+ chip->ngpio = 16;

What about getting the number of gpios from platform_data and/or device
tree?


Actually I am thinking about just setting it to 20 unconditionally. Anything requesting a non-present GPIO pin is buggy to begin with.

+
+ chip->direction_input = octeon_gpio_dir_in;
+ chip->get = octeon_gpio_get;
+ chip->direction_output = octeon_gpio_dir_out;
+ chip->set = octeon_gpio_set;
+ err = gpiochip_add(chip);
+ if (err)
+ goto out;
+
+ dev_info(&pdev->dev, "version: " DRV_VERSION "\n");
+out:
+ return err;
+}
+
+static int __exit octeon_gpio_remove(struct platform_device *pdev)
+{
+ struct gpio_chip *chip = pdev->dev.platform_data;
+ return gpiochip_remove(chip);
+}
+
+static struct of_device_id octeon_gpio_match[] = {
+ {
+ .compatible = "cavium,octeon-3860-gpio",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, octeon_mgmt_match);

You are using linux/of.h definitions here but you did not include it.
Also, there is a typo, you want octeon_gpio_match instead.


Good catch. I will fix that. There is also a section mismatch I need to fix.

<Prev in Thread] Current Thread [Next in Thread>