Re: [PATCH] gpio: Add Tegra186 support

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

 




On Tuesday 22 November 2016 11:25 PM, Thierry Reding wrote:
+static inline struct tegra_gpio *to_tegra_gpio(struct gpio_chip *chip)
+{
+	return container_of(chip, struct tegra_gpio, gpio);
+}

You dont need this as gpiochip_get_data(chip); can provide the required driver specific data.

+
+static const struct tegra_gpio_port *
+tegra186_gpio_get_port(struct tegra_gpio *gpio, unsigned int *pin)
+{
+	unsigned int start = 0, i;
+
+	for (i = 0; i < gpio->soc->num_ports; i++) {
+		const struct tegra_gpio_port *port = &gpio->soc->ports[i];
+
+		if (*pin >= start && *pin < start + port->pins) {
+			*pin -= start;
+			return port;
+		}
+
+		start += port->pins;
+	}
+
Why not get the port from pins and then calculate with fixed offset.
We will not need the loop if we know the port number.


+
+static int tegra186_gpio_direction_output(struct gpio_chip *chip,
+					  unsigned int offset, int level)
+{
+	struct tegra_gpio *gpio = to_tegra_gpio(chip);
+	void __iomem *base;
+	u32 value;
+
+	/* configure output level first */
+	chip->set(chip, offset, level);
We can directly call the apis instead of function pointer at this point.

+
+static struct lock_class_key tegra186_gpio_lock_class;
We will have two instance of the driver (normal and AON) and so this will be shared between them.
Do we really support multiple instance with same variable?


+
+	gpio->gpio.label = "tegra186-gpio";
Two instance will have same name. Better to say tegra186-gpio and tegra186-gpio-aon.


+	gpio->gpio.parent = &pdev->dev;
+
+	gpio->gpio.get_direction = tegra186_gpio_get_direction;
+	gpio->gpio.direction_input = tegra186_gpio_direction_input;
+	gpio->gpio.direction_output = tegra186_gpio_direction_output;
+	gpio->gpio.get = tegra186_gpio_get,
+	gpio->gpio.set = tegra186_gpio_set;
+	gpio->gpio.to_irq = tegra186_gpio_to_irq;
+
+	gpio->gpio.base = -1;
+
+	for (i = 0; i < gpio->soc->num_ports; i++)
+		gpio->gpio.ngpio += gpio->soc->ports[i].pins;
+

Our DT binding does not say this. We assume that we have 8 gpios per port. so this will not work at all.




+
+static const struct tegra_gpio_port tegra186_main_ports[] = {
+	[TEGRA_MAIN_GPIO_PORT_A]  = { 0x2000, 7 },
Use C99 style initialization which is like
   .offset =
   .pins =


--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux