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

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


Le 13/04/2012 18:17, David Daney a écrit :
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.

Since it is exposed by the sysfs entry ngpio, you can consider it being part of the ABI, and it can be used by an user to query the number of gpios available, and will also serve as the maximum number of gpios for gpiolib internally, it is certainly best to make it match what the hardware can actually do.


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




[Linux MIPS Home]     [LKML Archive]     [Linux ARM]     [Linux]     [Git]     [Photo]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

Add to Google Powered by Linux