[PATCH v2 00/11] staging: mt7621-gpio: use mediatek as binding instead of custom mtk

[PATCH v2 00/11] staging: mt7621-gpio: use mediatek as binding instead of custom mtk

[Sergio Paracuellos]

The following series updated mt7621-gpio driver to use 'mediatek'
which is already defined in kernel bindings documentation instead
of add a new custom one 'mtk' for this company. mt7621-gpio device-tree
documentation has been added also to the staging driver directory.

v2:
 - Join some patch ("staging: mt7621-gpio: avoid use of globals 
   and use platform_data instead") with the needed fixes to make 
   it work pointed out by NeilBrown
 - Add gpio interrupts to documentation and device tree in order to
   can try them
 - Other minor code cleanups have been added also.

After this changes only testing interrupts should be remaining to
get this out of staging.

Hope this helps.

Best regards,
     Sergio

Sergio Paracuellos (11):
  staging: mt7621-gpio: dt-bindings: add documentation for mt7621-gpio
  staging: mt7621-gpio: update TODO file
  staging: mt7621-dts: update gpios related entries to use 'mediatek'
  staging: mt7621-gpio: replace 'mtk' to use correct one 'mediatek'
  staging: mt7621-gpio: avoid use of globals and use platform_data
    instead
  staging: mt7621-dts: add interrupt device tree nodes for the gpio
    controller
  staging: mt7621-gpio: dt-bindings: add interrupt nodes to bindings doc
  staging: mt7621-gpio: avoid devm_kzalloc() hidden inside declarations
  staging: mt7621-gpio: use ternary operator in return in
    mediatek_gpio_get_direction
  staging: mt7621-gpio: use MTK_BANK_WIDTH instead of magic number
  staging: mt7621-gpio: update TODO list

 drivers/staging/mt7621-dts/mt7621.dtsi             |  11 +-
 drivers/staging/mt7621-gpio/TODO                   |   3 +-
 drivers/staging/mt7621-gpio/gpio-mt7621.c          | 111 +++++++++++++--------
 .../staging/mt7621-gpio/mediatek,mt7621-gpio.txt   |  58 +++++++++++
 4 files changed, 137 insertions(+), 46 deletions(-)
 create mode 100644 drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt

-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 01/11] staging: mt7621-gpio: dt-bindings: add documentation for mt7621-gpio

[Sergio Paracuellos]

This commit add missing dt bindings documentation for mt7621-gpio
driver. There is some missing stuff here about interrupts with is
not also being used in the mt7621.dtsi file. So just include in
staging a incomplete version before moving this to kernel's dt-bindings
place.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../staging/mt7621-gpio/mediatek,mt7621-gpio.txt   | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt

diff --git a/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt b/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
new file mode 100644
index 0000000..54de9f7
--- /dev/null
+++ b/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
@@ -0,0 +1,51 @@
+Mediatek SoC GPIO controller bindings
+
+The IP core used inside these SoCs has 3 banks of 32 GPIOs each.
+The registers of all the banks are interwoven inside one single IO range.
+We load one GPIO controller instance per bank. To make this possible
+we support 2 types of nodes. The parent node defines the memory I/O range and
+has 3 children each describing a single bank.
+
+Required properties for the top level node:
+- compatible:
+  - "mediatek,mt7621-gpio" for Mediatek controllers
+- reg : Physical base address and length of the controller's registers
+
+Required properties for the GPIO bank node:
+- compatible:
+  - "mediatek,mt7621-gpio-bank" for Mediatek banks
+- #gpio-cells : Should be two.
+  - first cell is the pin number
+  - second cell is used to specify optional parameters (unused)
+- gpio-controller : Marks the device node as a GPIO controller
+- reg : The id of the bank that the node describes.
+
+Example:
+	gpio@600 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		compatible = "mediatek,mt7621-gpio";
+		reg = <0x600 0x100>;
+
+		gpio0: bank@0 {
+			reg = <0>;
+			compatible = "mediatek,mt7621-gpio-bank";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		gpio1: bank@1 {
+			reg = <1>;
+			compatible = "mediatek,mt7621-gpio-bank";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		gpio2: bank@2 {
+			reg = <2>;
+			compatible = "mediatek,mt7621-gpio-bank";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+	};
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 02/11] staging: mt7621-gpio: update TODO file

[Sergio Paracuellos]

This commit updates TODO file with missing things to
get this driver ready to be mainlined.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/TODO | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-gpio/TODO b/drivers/staging/mt7621-gpio/TODO
index 7143905..9077b16 100644
--- a/drivers/staging/mt7621-gpio/TODO
+++ b/drivers/staging/mt7621-gpio/TODO
@@ -1,5 +1,7 @@
 
-- general code review and clean up
+- general code review and clean up 
+- avoid global variables and use drvdata allocated instead
 - ensure device-tree requirements are documented
+- make sure interrupts work
 
 Cc:  NeilBrown <neil@brown.name>
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 03/11] staging: mt7621-dts: update gpios related entries to use 'mediatek'

[Sergio Paracuellos]

Gpio driver for mt7621 is using 'mtk' as binding but in the kernel
is already defined one for this maker which is 'mediatek'. Update
device tree to use the correct one.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-dts/mt7621.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
index 9d941b5..115eb04 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -64,26 +64,26 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 
-			compatible = "mtk,mt7621-gpio";
+			compatible = "mediatek,mt7621-gpio";
 			reg = <0x600 0x100>;
 
 			gpio0: bank@0 {
 				reg = <0>;
-				compatible = "mtk,mt7621-gpio-bank";
+				compatible = "mediatek,mt7621-gpio-bank";
 				gpio-controller;
 				#gpio-cells = <2>;
 			};
 
 			gpio1: bank@1 {
 				reg = <1>;
-				compatible = "mtk,mt7621-gpio-bank";
+				compatible = "mediatek,mt7621-gpio-bank";
 				gpio-controller;
 				#gpio-cells = <2>;
 			};
 
 			gpio2: bank@2 {
 				reg = <2>;
-				compatible = "mtk,mt7621-gpio-bank";
+				compatible = "mediatek,mt7621-gpio-bank";
 				gpio-controller;
 				#gpio-cells = <2>;
 			};
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 04/11] staging: mt7621-gpio: replace 'mtk' to use correct one 'mediatek'

[Sergio Paracuellos]

Gpio driver is using mtk and there is already 'mediatek' binding
defined for this maker. Update driver to use it instead the custom
one 'mtk'.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index a577381..7d17949 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -323,7 +323,7 @@ mediatek_gpio_probe(struct platform_device *pdev)
 	}
 
 	for_each_child_of_node(np, bank)
-		if (of_device_is_compatible(bank, "mtk,mt7621-gpio-bank"))
+		if (of_device_is_compatible(bank, "mediatek,mt7621-gpio-bank"))
 			mediatek_gpio_bank_probe(pdev, bank);
 
 	if (mediatek_gpio_irq_domain)
@@ -334,7 +334,7 @@ mediatek_gpio_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id mediatek_gpio_match[] = {
-	{ .compatible = "mtk,mt7621-gpio" },
+	{ .compatible = "mediatek,mt7621-gpio" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, mediatek_gpio_match);
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 05/11] staging: mt7621-gpio: avoid use of globals and use platform_data instead

[Sergio Paracuellos]

Gpio driver have a some globals which can be avoided just
using platform_data in a proper form. This commit adds a new
struct mtk_data which includes all of those globals setting them
using platform_set_drvdata and devm_gpiochip_add_data functions.
With this properly set we are able to retrieve driver data along
the code using kernel api's so globals are not needed anymore.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 85 +++++++++++++++++++++----------
 1 file changed, 59 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 7d17949..c701259 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -31,17 +31,20 @@ enum mediatek_gpio_reg {
 	GPIO_REG_EDGE,
 };
 
-static void __iomem *mediatek_gpio_membase;
-static int mediatek_gpio_irq;
-static struct irq_domain *mediatek_gpio_irq_domain;
-
-static struct mtk_gc {
+struct mtk_gc {
 	struct gpio_chip chip;
 	spinlock_t lock;
 	int bank;
 	u32 rising;
 	u32 falling;
-} *gc_map[MTK_MAX_BANK];
+};
+
+struct mtk_data {
+	void __iomem *gpio_membase;
+	int gpio_irq;
+	struct irq_domain *gpio_irq_domain;
+	struct mtk_gc *gc_map[MTK_MAX_BANK];
+};
 
 static inline struct mtk_gc
 *to_mediatek_gpio(struct gpio_chip *chip)
@@ -56,15 +59,19 @@ static inline struct mtk_gc
 static inline void
 mtk_gpio_w32(struct mtk_gc *rg, u8 reg, u32 val)
 {
-	iowrite32(val, mediatek_gpio_membase + (reg * 0x10) + (rg->bank * 0x4));
+	struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
+	u32 offset = (reg * 0x10) + (rg->bank * 0x4);
+
+	iowrite32(val, gpio_data->gpio_membase + offset);
 }
 
 static inline u32
 mtk_gpio_r32(struct mtk_gc *rg, u8 reg)
 {
+	struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
 	u32 offset = (reg * 0x10) + (rg->bank * 0x4);
 
-	return ioread32(mediatek_gpio_membase + offset);
+	return ioread32(gpio_data->gpio_membase + offset);
 }
 
 static void
@@ -137,23 +144,26 @@ mediatek_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
 static int
 mediatek_gpio_to_irq(struct gpio_chip *chip, unsigned int pin)
 {
+	struct mtk_data *gpio_data = gpiochip_get_data(chip);
 	struct mtk_gc *rg = to_mediatek_gpio(chip);
 
-	return irq_create_mapping(mediatek_gpio_irq_domain,
+	return irq_create_mapping(gpio_data->gpio_irq_domain,
 				  pin + (rg->bank * MTK_BANK_WIDTH));
 }
 
 static int
 mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
 {
+	struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
 	const __be32 *id = of_get_property(bank, "reg", NULL);
 	struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
 				sizeof(struct mtk_gc), GFP_KERNEL);
+	int ret;
 
 	if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
 		return -ENOMEM;
 
-	gc_map[be32_to_cpu(*id)] = rg;
+	gpio_data->gc_map[be32_to_cpu(*id)] = rg;
 
 	memset(rg, 0, sizeof(struct mtk_gc));
 
@@ -169,10 +179,17 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
 	rg->chip.get_direction = mediatek_gpio_get_direction;
 	rg->chip.get = mediatek_gpio_get;
 	rg->chip.set = mediatek_gpio_set;
-	if (mediatek_gpio_irq_domain)
+	if (gpio_data->gpio_irq_domain)
 		rg->chip.to_irq = mediatek_gpio_to_irq;
 	rg->bank = be32_to_cpu(*id);
 
+	ret = devm_gpiochip_add_data(&pdev->dev, &rg->chip, gpio_data);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Could not register gpio %d, ret=%d\n",
+			rg->chip.ngpio, ret);
+		return ret;
+	}
+
 	/* set polarity to low for all gpios */
 	mtk_gpio_w32(rg, GPIO_REG_POL, 0);
 
@@ -184,10 +201,12 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
 static void
 mediatek_gpio_irq_handler(struct irq_desc *desc)
 {
+	struct gpio_chip *gc = irq_desc_get_handler_data(desc);
+	struct mtk_data *gpio_data = gpiochip_get_data(gc);
 	int i;
 
 	for (i = 0; i < MTK_MAX_BANK; i++) {
-		struct mtk_gc *rg = gc_map[i];
+		struct mtk_gc *rg = gpio_data->gc_map[i];
 		unsigned long pending;
 		int bit;
 
@@ -197,7 +216,7 @@ mediatek_gpio_irq_handler(struct irq_desc *desc)
 		pending = mtk_gpio_r32(rg, GPIO_REG_STAT);
 
 		for_each_set_bit(bit, &pending, MTK_BANK_WIDTH) {
-			u32 map = irq_find_mapping(mediatek_gpio_irq_domain,
+			u32 map = irq_find_mapping(gpio_data->gpio_irq_domain,
 						   (MTK_BANK_WIDTH * i) + bit);
 
 			generic_handle_irq(map);
@@ -209,9 +228,11 @@ mediatek_gpio_irq_handler(struct irq_desc *desc)
 static void
 mediatek_gpio_irq_unmask(struct irq_data *d)
 {
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct mtk_data *gpio_data = gpiochip_get_data(gc);
 	int pin = d->hwirq;
 	int bank = pin / 32;
-	struct mtk_gc *rg = gc_map[bank];
+	struct mtk_gc *rg = gpio_data->gc_map[bank];
 	unsigned long flags;
 	u32 rise, fall;
 
@@ -230,9 +251,11 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
 static void
 mediatek_gpio_irq_mask(struct irq_data *d)
 {
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct mtk_data *gpio_data = gpiochip_get_data(gc);
 	int pin = d->hwirq;
 	int bank = pin / 32;
-	struct mtk_gc *rg = gc_map[bank];
+	struct mtk_gc *rg = gpio_data->gc_map[bank];
 	unsigned long flags;
 	u32 rise, fall;
 
@@ -251,9 +274,11 @@ mediatek_gpio_irq_mask(struct irq_data *d)
 static int
 mediatek_gpio_irq_type(struct irq_data *d, unsigned int type)
 {
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct mtk_data *gpio_data = gpiochip_get_data(gc);
 	int pin = d->hwirq;
 	int bank = pin / 32;
-	struct mtk_gc *rg = gc_map[bank];
+	struct mtk_gc *rg = gpio_data->gc_map[bank];
 	u32 mask = BIT(d->hwirq);
 
 	if (!rg)
@@ -308,27 +333,35 @@ mediatek_gpio_probe(struct platform_device *pdev)
 {
 	struct device_node *bank, *np = pdev->dev.of_node;
 	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct mtk_data *gpio_data;
 
-	mediatek_gpio_membase = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(mediatek_gpio_membase))
-		return PTR_ERR(mediatek_gpio_membase);
+	gpio_data = devm_kzalloc(&pdev->dev, sizeof(*gpio_data), GFP_KERNEL);
+	if (!gpio_data)
+		return -ENOMEM;
 
-	mediatek_gpio_irq = irq_of_parse_and_map(np, 0);
-	if (mediatek_gpio_irq) {
-		mediatek_gpio_irq_domain = irq_domain_add_linear(np,
+	gpio_data->gpio_membase = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(gpio_data->gpio_membase))
+		return PTR_ERR(gpio_data->gpio_membase);
+
+	gpio_data->gpio_irq = irq_of_parse_and_map(np, 0);
+	if (gpio_data->gpio_irq) {
+		gpio_data->gpio_irq_domain = irq_domain_add_linear(np,
 			MTK_MAX_BANK * MTK_BANK_WIDTH,
 			&irq_domain_ops, NULL);
-		if (!mediatek_gpio_irq_domain)
+		if (!gpio_data->gpio_irq_domain)
 			dev_err(&pdev->dev, "irq_domain_add_linear failed\n");
 	}
 
+	platform_set_drvdata(pdev, gpio_data);
+
 	for_each_child_of_node(np, bank)
 		if (of_device_is_compatible(bank, "mediatek,mt7621-gpio-bank"))
 			mediatek_gpio_bank_probe(pdev, bank);
 
-	if (mediatek_gpio_irq_domain)
-		irq_set_chained_handler(mediatek_gpio_irq,
-					mediatek_gpio_irq_handler);
+	if (gpio_data->gpio_irq_domain)
+		irq_set_chained_handler_and_data(gpio_data->gpio_irq,
+						 mediatek_gpio_irq_handler,
+						 gpio_data);
 
 	return 0;
 }
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 06/11] staging: mt7621-dts: add interrupt device tree nodes for the gpio controller

[Sergio Paracuellos]

The GPIO controller of mt7621 can receive interrupts on any
of the GPIOs, either edge or level. It then interrupts the CPU using
GIC INT12. Update device tree accordly.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-dts/mt7621.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
index 115eb04..240d396 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -67,6 +67,9 @@
 			compatible = "mediatek,mt7621-gpio";
 			reg = <0x600 0x100>;
 
+			interrupt-parent = <&gic>;
+			interrupts = <GIC_SHARED 12 IRQ_TYPE_LEVEL_HIGH>;
+
 			gpio0: bank@0 {
 				reg = <0>;
 				compatible = "mediatek,mt7621-gpio-bank";
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 07/11] staging: mt7621-gpio: dt-bindings: add interrupt nodes to bindings doc

[Sergio Paracuellos]

Interrupt related stuff for gpio controller in mt7621 was missing
in device tree documentation. Add it to complete documentation for
this driver.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt b/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
index 54de9f7..af64092 100644
--- a/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
+++ b/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
@@ -4,12 +4,16 @@ The IP core used inside these SoCs has 3 banks of 32 GPIOs each.
 The registers of all the banks are interwoven inside one single IO range.
 We load one GPIO controller instance per bank. To make this possible
 we support 2 types of nodes. The parent node defines the memory I/O range and
-has 3 children each describing a single bank.
+has 3 children each describing a single bank. Also the GPIO controller can receive
+interrupts on any of the GPIOs, either edge or level. It then interrupts the CPU
+using GIC INT12.
 
 Required properties for the top level node:
 - compatible:
   - "mediatek,mt7621-gpio" for Mediatek controllers
 - reg : Physical base address and length of the controller's registers
+- interrupt-parent : phandle of the parent interrupt controller.
+- interrupts = Interrupt specifier for the controllers interrupt
 
 Required properties for the GPIO bank node:
 - compatible:
@@ -28,6 +32,9 @@ Example:
 		compatible = "mediatek,mt7621-gpio";
 		reg = <0x600 0x100>;
 
+		interrupt-parent = <&gic>;
+		interrupts = <GIC_SHARED 12 IRQ_TYPE_LEVEL_HIGH>;
+
 		gpio0: bank@0 {
 			reg = <0>;
 			compatible = "mediatek,mt7621-gpio-bank";
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 08/11] staging: mt7621-gpio: avoid devm_kzalloc() hidden inside declarations

[Sergio Paracuellos]

Driver probe function includes a allocation using devm_kzalloc
which is "hidden" a bit inside the declarations. Extract this
to a better place to increase readability. Also because we are
allocating zeroed memory 'memset' statement is not needed at all.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index c701259..2d16d62 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -156,17 +156,18 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
 {
 	struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
 	const __be32 *id = of_get_property(bank, "reg", NULL);
-	struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
-				sizeof(struct mtk_gc), GFP_KERNEL);
+	struct mtk_gc *rg;
 	int ret;
 
-	if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
+	if (!id || be32_to_cpu(*id) > MTK_MAX_BANK)
+		return -EINVAL;
+
+	rg = devm_kzalloc(&pdev->dev, sizeof(struct mtk_gc), GFP_KERNEL);
+	if (!rg)
 		return -ENOMEM;
 
 	gpio_data->gc_map[be32_to_cpu(*id)] = rg;
 
-	memset(rg, 0, sizeof(struct mtk_gc));
-
 	spin_lock_init(&rg->lock);
 
 	rg->chip.parent = &pdev->dev;
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 08/11] staging: mt7621-gpio: avoid devm_kzalloc() hidden inside declarations

[Dan Carpenter]

On Wed, May 16, 2018 at 07:30:01AM +0200, Sergio Paracuellos wrote:
> diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> index c701259..2d16d62 100644
> --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> @@ -156,17 +156,18 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
>  {
>  	struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
>  	const __be32 *id = of_get_property(bank, "reg", NULL);
> -	struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
> -				sizeof(struct mtk_gc), GFP_KERNEL);
> +	struct mtk_gc *rg;
>  	int ret;
>  
> -	if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
> +	if (!id || be32_to_cpu(*id) > MTK_MAX_BANK)

You didn't introduce this, but it should be >= MTK_MAX_BANK.  (Off by
one).

regards,
dan carpenter

Re: [PATCH v2 08/11] staging: mt7621-gpio: avoid devm_kzalloc() hidden inside declarations

[Sergio Paracuellos]

On Wed, May 16, 2018 at 11:04:52AM +0300, Dan Carpenter wrote:
> On Wed, May 16, 2018 at 07:30:01AM +0200, Sergio Paracuellos wrote:
> > diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > index c701259..2d16d62 100644
> > --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > @@ -156,17 +156,18 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
> >  {
> >  	struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
> >  	const __be32 *id = of_get_property(bank, "reg", NULL);
> > -	struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
> > -				sizeof(struct mtk_gc), GFP_KERNEL);
> > +	struct mtk_gc *rg;
> >  	int ret;
> >  
> > -	if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
> > +	if (!id || be32_to_cpu(*id) > MTK_MAX_BANK)
> 
> You didn't introduce this, but it should be >= MTK_MAX_BANK.  (Off by
> one).

Totally true, thanks for pointing this out, Dan. 
I'll fix this and send v3 of the whole series.

> 
> regards,
> dan carpenter
> 

Best regards,
    Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 08/11] staging: mt7621-gpio: avoid devm_kzalloc() hidden inside declarations

[Dan Carpenter]

On Wed, May 16, 2018 at 10:56:05AM +0200, Sergio Paracuellos wrote:
> On Wed, May 16, 2018 at 11:04:52AM +0300, Dan Carpenter wrote:
> > On Wed, May 16, 2018 at 07:30:01AM +0200, Sergio Paracuellos wrote:
> > > diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > > index c701259..2d16d62 100644
> > > --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > > +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > > @@ -156,17 +156,18 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
> > >  {
> > >  	struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
> > >  	const __be32 *id = of_get_property(bank, "reg", NULL);
> > > -	struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
> > > -				sizeof(struct mtk_gc), GFP_KERNEL);
> > > +	struct mtk_gc *rg;
> > >  	int ret;
> > >  
> > > -	if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
> > > +	if (!id || be32_to_cpu(*id) > MTK_MAX_BANK)
> > 
> > You didn't introduce this, but it should be >= MTK_MAX_BANK.  (Off by
> > one).
> 
> Totally true, thanks for pointing this out, Dan. 
> I'll fix this and send v3 of the whole series.

I think you can just send the off by one fix by itself unless there are
complaints about this patch series.  The bug was there before, so it's
not a new thing.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 08/11] staging: mt7621-gpio: avoid devm_kzalloc() hidden inside declarations

[Sergio Paracuellos]

On Wed, May 16, 2018 at 12:40:57PM +0300, Dan Carpenter wrote:
> On Wed, May 16, 2018 at 10:56:05AM +0200, Sergio Paracuellos wrote:
> > On Wed, May 16, 2018 at 11:04:52AM +0300, Dan Carpenter wrote:
> > > On Wed, May 16, 2018 at 07:30:01AM +0200, Sergio Paracuellos wrote:
> > > > diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > > > index c701259..2d16d62 100644
> > > > --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > > > +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > > > @@ -156,17 +156,18 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
> > > >  {
> > > >  	struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
> > > >  	const __be32 *id = of_get_property(bank, "reg", NULL);
> > > > -	struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
> > > > -				sizeof(struct mtk_gc), GFP_KERNEL);
> > > > +	struct mtk_gc *rg;
> > > >  	int ret;
> > > >  
> > > > -	if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
> > > > +	if (!id || be32_to_cpu(*id) > MTK_MAX_BANK)
> > > 
> > > You didn't introduce this, but it should be >= MTK_MAX_BANK.  (Off by
> > > one).
> > 
> > Totally true, thanks for pointing this out, Dan. 
> > I'll fix this and send v3 of the whole series.
> 
> I think you can just send the off by one fix by itself unless there are
> complaints about this patch series.  The bug was there before, so it's
> not a new thing.

Thanks for clarification, Dan. I had sent already v3 before reading this :(.

Anyway, I'll take this into account from now.

> 
> regards,
> dan carpenter
> 
Best regards,
   Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 09/11] staging: mt7621-gpio: use ternary operator in return in mediatek_gpio_get_direction

[Sergio Paracuellos]

This commits replaces if statement and two returns in favour
of a only one return using a ternary operator.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 2d16d62..379efc4 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -135,10 +135,7 @@ mediatek_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
 	t = mtk_gpio_r32(rg, GPIO_REG_CTRL);
 	spin_unlock_irqrestore(&rg->lock, flags);
 
-	if (t & BIT(offset))
-		return 0;
-
-	return 1;
+	return (t & BIT(offset)) ? 0 : 1;
 }
 
 static int
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 10/11] staging: mt7621-gpio: use MTK_BANK_WIDTH instead of magic number

[Sergio Paracuellos]

There are some places where magic number '32' is being used to get
the gpio bank. There already exist a definition MTK_BANK_WIDTH
with this value, so just use it instead.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 379efc4..9124c1d 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -229,7 +229,7 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct mtk_data *gpio_data = gpiochip_get_data(gc);
 	int pin = d->hwirq;
-	int bank = pin / 32;
+	int bank = pin / MTK_BANK_WIDTH;
 	struct mtk_gc *rg = gpio_data->gc_map[bank];
 	unsigned long flags;
 	u32 rise, fall;
@@ -252,7 +252,7 @@ mediatek_gpio_irq_mask(struct irq_data *d)
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct mtk_data *gpio_data = gpiochip_get_data(gc);
 	int pin = d->hwirq;
-	int bank = pin / 32;
+	int bank = pin / MTK_BANK_WIDTH;
 	struct mtk_gc *rg = gpio_data->gc_map[bank];
 	unsigned long flags;
 	u32 rise, fall;
@@ -275,7 +275,7 @@ mediatek_gpio_irq_type(struct irq_data *d, unsigned int type)
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct mtk_data *gpio_data = gpiochip_get_data(gc);
 	int pin = d->hwirq;
-	int bank = pin / 32;
+	int bank = pin / MTK_BANK_WIDTH;
 	struct mtk_gc *rg = gpio_data->gc_map[bank];
 	u32 mask = BIT(d->hwirq);
 
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 11/11] staging: mt7621-gpio: update TODO list

[Sergio Paracuellos]

Some of the remaining stuff included in TODO list
have been complete. So update this file accordly.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/TODO | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/TODO b/drivers/staging/mt7621-gpio/TODO
index 9077b16..9089833 100644
--- a/drivers/staging/mt7621-gpio/TODO
+++ b/drivers/staging/mt7621-gpio/TODO
@@ -1,7 +1,4 @@
 
-- general code review and clean up 
-- avoid global variables and use drvdata allocated instead
-- ensure device-tree requirements are documented
 - make sure interrupts work
 
 Cc:  NeilBrown <neil@brown.name>
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel