[PATCH 0/5] staging: mt7621-gpio: some driver cleanups

[PATCH 0/5] staging: mt7621-gpio: some driver cleanups

[Sergio Paracuellos]

The following patch series fix all remaining checkpatch complains
about this driver. Changes have not been tested and also compiled
but it should not have any problem about them.

Sergio Paracuellos (5):
  staging: mt7621-gpio: fix some warnings because of lines exceded 80
    characters
  staging: mt7621-gpio: add SPDX identifier
  dt-bindings: add compatible string for 'mtk' MediaTek
  dt-bindings: gpio: add documentation for mt7621-gpio
  staging: mt7621-gpio: remove device tree related stuff from TODO file

 .../devicetree/bindings/gpio/mtk,mt7621-gpio.txt   | 51 ++++++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt        |  1 +
 drivers/staging/mt7621-gpio/TODO                   |  1 -
 drivers/staging/mt7621-gpio/gpio-mt7621.c          | 24 +++++-----
 4 files changed, 66 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt

-- 
2.7.4

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

[PATCH 1/5] staging: mt7621-gpio: fix some warnings because of lines exceded 80 characters

[Sergio Paracuellos]

This patch silence some complains of checkpatch script because
of the use of long lines.

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

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index c9ef936..a6dcfdf 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -65,7 +65,9 @@ mtk_gpio_w32(struct mtk_gc *rg, u8 reg, u32 val)
 static inline u32
 mtk_gpio_r32(struct mtk_gc *rg, u8 reg)
 {
-	return ioread32(mediatek_gpio_membase + (reg * 0x10) + (rg->bank * 0x4));
+	u32 offset = (reg * 0x10) + (rg->bank * 0x4);
+
+	return ioread32(mediatek_gpio_membase + offset);
 }
 
 static void
@@ -140,7 +142,8 @@ mediatek_gpio_to_irq(struct gpio_chip *chip, unsigned int pin)
 {
 	struct mtk_gc *rg = to_mediatek_gpio(chip);
 
-	return irq_create_mapping(mediatek_gpio_irq_domain, pin + (rg->bank * MTK_BANK_WIDTH));
+	return irq_create_mapping(mediatek_gpio_irq_domain,
+				  pin + (rg->bank * MTK_BANK_WIDTH));
 }
 
 static int
@@ -197,7 +200,8 @@ 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, (MTK_BANK_WIDTH * i) + bit);
+			u32 map = irq_find_mapping(mediatek_gpio_irq_domain,
+						   (MTK_BANK_WIDTH * i) + bit);
 
 			generic_handle_irq(map);
 			mtk_gpio_w32(rg, GPIO_REG_STAT, BIT(bit));
@@ -287,9 +291,11 @@ static struct irq_chip mediatek_gpio_irq_chip = {
 };
 
 static int
-mediatek_gpio_gpio_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
+mediatek_gpio_gpio_map(struct irq_domain *d, unsigned int irq,
+		       irq_hw_number_t hw)
 {
-	irq_set_chip_and_handler(irq, &mediatek_gpio_irq_chip, handle_level_irq);
+	irq_set_chip_and_handler(irq, &mediatek_gpio_irq_chip,
+				 handle_level_irq);
 	irq_set_handler_data(irq, d);
 
 	return 0;
@@ -324,7 +330,8 @@ mediatek_gpio_probe(struct platform_device *pdev)
 			mediatek_gpio_bank_probe(pdev, bank);
 
 	if (mediatek_gpio_irq_domain)
-		irq_set_chained_handler(mediatek_gpio_irq, mediatek_gpio_irq_handler);
+		irq_set_chained_handler(mediatek_gpio_irq,
+					mediatek_gpio_irq_handler);
 
 	return 0;
 }
-- 
2.7.4

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

[PATCH 2/5] staging: mt7621-gpio: add SPDX identifier

[Sergio Paracuellos]

It's good to have SPDX identifiers in driver files to make it easier to
audit the kernel tree for correct licenses.

Fix up the one of staging gpio-mt7621 file to have a proper SPDX
identifier, based on the license text in the file itself. The SPDX
identifier is a legally binding shorthand, which can be used instead of
the full boiler plate text.

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 a6dcfdf..a577381 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -1,8 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation.
- *
  * Copyright (C) 2009-2011 Gabor Juhos <juhosg@openwrt.org>
  * Copyright (C) 2013 John Crispin <blogic@openwrt.org>
  */
-- 
2.7.4

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

[PATCH 3/5] dt-bindings: add compatible string for 'mtk' MediaTek

[Sergio Paracuellos]

There is a complain of checkpatch script about the not documented
DT compatible string for vendor "mtk". Add it to vendor-prefixes.txt
file.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index b5f978a..a588a29 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -228,6 +228,7 @@ mqmaker	mqmaker Inc.
 mscc	Microsemi Corporation
 msi	Micro-Star International Co. Ltd.
 mti	Imagination Technologies Ltd. (formerly MIPS Technologies Inc.)
+mtk	MediaTek
 multi-inno	Multi-Inno Technology Co.,Ltd
 mundoreader	Mundo Reader S.L.
 murata	Murata Manufacturing Co., Ltd.
-- 
2.7.4

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

[PATCH 4/5] dt-bindings: gpio: add documentation for mt7621-gpio

[Sergio Paracuellos]

This commit add missing dt bindings documentation for mt7621-gpio
driver. After this checkpatch script complain about this
issue dissapears.

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

diff --git a/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt b/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
new file mode 100644
index 0000000..5fe4bb5
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/mtk,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:
+  - "mtk,mt7621-gpio" for Mediatek controllers
+- reg : Physical base address and length of the controller's registers
+
+Required properties for the GPIO bank node:
+- compatible:
+  - "mtk,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 = "mtk,mt7621-gpio";
+		reg = <0x600 0x100>;
+
+		gpio0: bank@0 {
+			reg = <0>;
+			compatible = "mtk,mt7621-gpio-bank";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		gpio1: bank@1 {
+			reg = <1>;
+			compatible = "mtk,mt7621-gpio-bank";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		gpio2: bank@2 {
+			reg = <2>;
+			compatible = "mtk,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 5/5] staging: mt7621-gpio: remove device tree related stuff from TODO file

[Sergio Paracuellos]

Documentation related with device tree and its checkpatch complains
have been added. Update TODO file accordingly.

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

diff --git a/drivers/staging/mt7621-gpio/TODO b/drivers/staging/mt7621-gpio/TODO
index 7143905..492dbaa 100644
--- a/drivers/staging/mt7621-gpio/TODO
+++ b/drivers/staging/mt7621-gpio/TODO
@@ -1,5 +1,4 @@
 
 - general code review and clean up
-- ensure device-tree requirements are documented
 
 Cc:  NeilBrown <neil@brown.name>
-- 
2.7.4

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

Re: [PATCH 0/5] staging: mt7621-gpio: some driver cleanups

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 1481 bytes --]

On Mon, May 14 2018, Sergio Paracuellos wrote:

> The following patch series fix all remaining checkpatch complains
> about this driver. Changes have not been tested and also compiled
> but it should not have any problem about them.

Thanks for these.  As you say, nothing in them could change behaviour of
the driver, but I tested them anyway and gpio still works (both in and
out) - no surprises.

I can give
  Reviewed-by: NeilBrown <neil@brown.name>
for all exect "dt-bindings: gpio: add documentation for mt7621-gpio".
I'll reply to the separately.

Of course the dt-binding patches will need broader review once they seem
ready to us.

Thanks,
NeilBrown


>
> Sergio Paracuellos (5):
>   staging: mt7621-gpio: fix some warnings because of lines exceded 80
>     characters
>   staging: mt7621-gpio: add SPDX identifier
>   dt-bindings: add compatible string for 'mtk' MediaTek
>   dt-bindings: gpio: add documentation for mt7621-gpio
>   staging: mt7621-gpio: remove device tree related stuff from TODO file
>
>  .../devicetree/bindings/gpio/mtk,mt7621-gpio.txt   | 51 ++++++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>  drivers/staging/mt7621-gpio/TODO                   |  1 -
>  drivers/staging/mt7621-gpio/gpio-mt7621.c          | 24 +++++-----
>  4 files changed, 66 insertions(+), 11 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
>
> -- 
> 2.7.4

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

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

Re: [PATCH 4/5] dt-bindings: gpio: add documentation for mt7621-gpio

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 3382 bytes --]

On Mon, May 14 2018, Sergio Paracuellos wrote:

> This commit add missing dt bindings documentation for mt7621-gpio
> driver. After this checkpatch script complain about this
> issue dissapears.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  .../devicetree/bindings/gpio/mtk,mt7621-gpio.txt   | 51 ++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt b/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
> new file mode 100644
> index 0000000..5fe4bb5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/mtk,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:
> +  - "mtk,mt7621-gpio" for Mediatek controllers
> +- reg : Physical base address and length of the controller's registers
> +
> +Required properties for the GPIO bank node:
> +- compatible:
> +  - "mtk,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.

This is really good, but not quite complete.
Searching for "of_" in gpio-mt7621.c I find code handling everything
you've described, but also:

	mediatek_gpio_irq = irq_of_parse_and_map(np, 0);
	if (mediatek_gpio_irq) {
		mediatek_gpio_irq_domain = irq_domain_add_linear(np,

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

so
    interrupt-parent = <&gic>;
    interrupts = <GIC_SHARED 12 IRQ_TYPE_LEVEL_HIGH>
(I think).
Then you need whatever irq_domain_add_linear() expects.
I don't know what that is...  I tried following through
code and got lost in little twisty mazes.

So if you change this patch to add the file to
drivers/staging/mt7621-gpio
then I can give it
  Reviewed-by: NeilBrown <neil@brown.name>

and then we can fix it when an understanding of the interrupts is
available.
But I cannot approve it for Documentation/devicetree/bindings yet.

Thanks a lot,
NeilBrown


> +
> +Example:
> +	gpio@600 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		compatible = "mtk,mt7621-gpio";
> +		reg = <0x600 0x100>;
> +
> +		gpio0: bank@0 {
> +			reg = <0>;
> +			compatible = "mtk,mt7621-gpio-bank";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +		};
> +
> +		gpio1: bank@1 {
> +			reg = <1>;
> +			compatible = "mtk,mt7621-gpio-bank";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +		};
> +
> +		gpio2: bank@2 {
> +			reg = <2>;
> +			compatible = "mtk,mt7621-gpio-bank";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +		};
> +	};
> -- 
> 2.7.4

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

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

Re: [PATCH 5/5] staging: mt7621-gpio: remove device tree related stuff from TODO file

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 1361 bytes --]

On Mon, May 14 2018, Sergio Paracuellos wrote:

> Documentation related with device tree and its checkpatch complains
> have been added. Update TODO file accordingly.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  drivers/staging/mt7621-gpio/TODO | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/mt7621-gpio/TODO b/drivers/staging/mt7621-gpio/TODO
> index 7143905..492dbaa 100644
> --- a/drivers/staging/mt7621-gpio/TODO
> +++ b/drivers/staging/mt7621-gpio/TODO
> @@ -1,5 +1,4 @@
>  
>  - general code review and clean up
> -- ensure device-tree requirements are documented
>  
>  Cc:  NeilBrown <neil@brown.name>
> -- 
> 2.7.4

I said before that I could give a reviewed-by for this, but obviously I
cannot as it depend on the bindings documentation which I didn't
approve.

However, it does look like I need to add things to the list.
Apart from making sure interrupts work, the only thing I see in the code
is various global variables (mediatek_gpio_membase, mediatek_gpio_irq,
mediatek_gpio_irq_domain, gc_map) which should probably be in a
drvdata allocated structure (stored with platform_set_drvdata() -
plenty of examples in drivers/gpio/gpio-*.c)
I think it would then be ready for submission to drivers/gpio
and linux-gpio@vger.kernel.org.

Thanks,
NeilBrown

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

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

Re: [PATCH 4/5] dt-bindings: gpio: add documentation for mt7621-gpio

[Sergio Paracuellos]

On Tue, May 15, 2018 at 11:21:20AM +1000, NeilBrown wrote:
> On Mon, May 14 2018, Sergio Paracuellos wrote:
> 
> > This commit add missing dt bindings documentation for mt7621-gpio
> > driver. After this checkpatch script complain about this
> > issue dissapears.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  .../devicetree/bindings/gpio/mtk,mt7621-gpio.txt   | 51 ++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt b/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
> > new file mode 100644
> > index 0000000..5fe4bb5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/mtk,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:
> > +  - "mtk,mt7621-gpio" for Mediatek controllers
> > +- reg : Physical base address and length of the controller's registers
> > +
> > +Required properties for the GPIO bank node:
> > +- compatible:
> > +  - "mtk,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.
> 
> This is really good, but not quite complete.
> Searching for "of_" in gpio-mt7621.c I find code handling everything
> you've described, but also:
> 
> 	mediatek_gpio_irq = irq_of_parse_and_map(np, 0);
> 	if (mediatek_gpio_irq) {
> 		mediatek_gpio_irq_domain = irq_domain_add_linear(np,
> 
> The GPIO controller can receive interrupts on any of the GPIOs,
> either edge or level.  It then interrupts the CPU using GIC INT12.
> 
> so
>     interrupt-parent = <&gic>;
>     interrupts = <GIC_SHARED 12 IRQ_TYPE_LEVEL_HIGH>
> (I think).
> Then you need whatever irq_domain_add_linear() expects.
> I don't know what that is...  I tried following through
> code and got lost in little twisty mazes.
> 
> So if you change this patch to add the file to
> drivers/staging/mt7621-gpio
> then I can give it
>   Reviewed-by: NeilBrown <neil@brown.name>

I moved this to the driver directory as it is because dtsi is not
including yet stuff about gpio interrupts. This should be added when
make sure that interrupt works and gtsi will be updated.

> 
> and then we can fix it when an understanding of the interrupts is
> available.
> But I cannot approve it for Documentation/devicetree/bindings yet.
> 
> Thanks a lot,
> NeilBrown
>

Best regards,
    Sergio PAracuellos 
> 
> > +
> > +Example:
> > +	gpio@600 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		compatible = "mtk,mt7621-gpio";
> > +		reg = <0x600 0x100>;
> > +
> > +		gpio0: bank@0 {
> > +			reg = <0>;
> > +			compatible = "mtk,mt7621-gpio-bank";
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +		};
> > +
> > +		gpio1: bank@1 {
> > +			reg = <1>;
> > +			compatible = "mtk,mt7621-gpio-bank";
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +		};
> > +
> > +		gpio2: bank@2 {
> > +			reg = <2>;
> > +			compatible = "mtk,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

Re: [PATCH 5/5] staging: mt7621-gpio: remove device tree related stuff from TODO file

[Sergio Paracuellos]

On Tue, May 15, 2018 at 11:26:38AM +1000, NeilBrown wrote:
> On Mon, May 14 2018, Sergio Paracuellos wrote:
> 
> > Documentation related with device tree and its checkpatch complains
> > have been added. Update TODO file accordingly.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  drivers/staging/mt7621-gpio/TODO | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/staging/mt7621-gpio/TODO b/drivers/staging/mt7621-gpio/TODO
> > index 7143905..492dbaa 100644
> > --- a/drivers/staging/mt7621-gpio/TODO
> > +++ b/drivers/staging/mt7621-gpio/TODO
> > @@ -1,5 +1,4 @@
> >  
> >  - general code review and clean up
> > -- ensure device-tree requirements are documented
> >  
> >  Cc:  NeilBrown <neil@brown.name>
> > -- 
> > 2.7.4
> 
> I said before that I could give a reviewed-by for this, but obviously I
> cannot as it depend on the bindings documentation which I didn't
> approve.
> 
> However, it does look like I need to add things to the list.
> Apart from making sure interrupts work, the only thing I see in the code
> is various global variables (mediatek_gpio_membase, mediatek_gpio_irq,
> mediatek_gpio_irq_domain, gc_map) which should probably be in a
> drvdata allocated structure (stored with platform_set_drvdata() -
> plenty of examples in drivers/gpio/gpio-*.c)
> I think it would then be ready for submission to drivers/gpio
> and linux-gpio@vger.kernel.org.

Ack! I have just update TODO with this stuff in v2 series.

> 
> Thanks,
> NeilBrown

Best regards,
    Sergio Paracuellos


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

Re: [PATCH 0/5] staging: mt7621-gpio: some driver cleanups

[Sergio Paracuellos]

On Tue, May 15, 2018 at 11:01:20AM +1000, NeilBrown wrote:
> On Mon, May 14 2018, Sergio Paracuellos wrote:
> 
> > The following patch series fix all remaining checkpatch complains
> > about this driver. Changes have not been tested and also compiled
> > but it should not have any problem about them.
> 
> Thanks for these.  As you say, nothing in them could change behaviour of
> the driver, but I tested them anyway and gpio still works (both in and
> out) - no surprises.

Thanks for testing this and review.

> 
> I can give
>   Reviewed-by: NeilBrown <neil@brown.name>
> for all exect "dt-bindings: gpio: add documentation for mt7621-gpio".
> I'll reply to the separately.

I have just sent v2 for all of this series and also replied to your
comments separately.

Hope this helps.

> 
> Of course the dt-binding patches will need broader review once they seem
> ready to us.
> 
> Thanks,
> NeilBrown

Best regards,
    Sergio Paracuellos
> 
> 
> >
> > Sergio Paracuellos (5):
> >   staging: mt7621-gpio: fix some warnings because of lines exceded 80
> >     characters
> >   staging: mt7621-gpio: add SPDX identifier
> >   dt-bindings: add compatible string for 'mtk' MediaTek
> >   dt-bindings: gpio: add documentation for mt7621-gpio
> >   staging: mt7621-gpio: remove device tree related stuff from TODO file
> >
> >  .../devicetree/bindings/gpio/mtk,mt7621-gpio.txt   | 51 ++++++++++++++++++++++
> >  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
> >  drivers/staging/mt7621-gpio/TODO                   |  1 -
> >  drivers/staging/mt7621-gpio/gpio-mt7621.c          | 24 +++++-----
> >  4 files changed, 66 insertions(+), 11 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
> >
> > -- 
> > 2.7.4


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

Re: [PATCH 3/5] dt-bindings: add compatible string for 'mtk' MediaTek

[Greg KH]

On Mon, May 14, 2018 at 04:02:32PM +0200, Sergio Paracuellos wrote:
> There is a complain of checkpatch script about the not documented
> DT compatible string for vendor "mtk". Add it to vendor-prefixes.txt
> file.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index b5f978a..a588a29 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -228,6 +228,7 @@ mqmaker	mqmaker Inc.
>  mscc	Microsemi Corporation
>  msi	Micro-Star International Co. Ltd.
>  mti	Imagination Technologies Ltd. (formerly MIPS Technologies Inc.)
> +mtk	MediaTek

MediaTek already has a binding here, it is "mediatek".  Please don't
introduce a new one for the same company.

Also, all device tree patches need to cc: the device tree maintainers to
get their review before I can accept them.

thanks,

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

Re: [PATCH 4/5] dt-bindings: gpio: add documentation for mt7621-gpio

[Greg KH]

On Mon, May 14, 2018 at 04:02:33PM +0200, Sergio Paracuellos wrote:
> This commit add missing dt bindings documentation for mt7621-gpio
> driver. After this checkpatch script complain about this
> issue dissapears.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  .../devicetree/bindings/gpio/mtk,mt7621-gpio.txt   | 51 ++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt b/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
> new file mode 100644
> index 0000000..5fe4bb5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/mtk,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:
> +  - "mtk,mt7621-gpio" for Mediatek controllers

As stated in the previous review, "mtk" is not valid, please use
"mediatek" like the rest of the kernel does :)

thanks,

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

Re: [PATCH 3/5] dt-bindings: add compatible string for 'mtk' MediaTek

[Sergio Paracuellos]

On Tue, May 15, 2018 at 09:24:30AM +0200, Greg KH wrote:
> On Mon, May 14, 2018 at 04:02:32PM +0200, Sergio Paracuellos wrote:
> > There is a complain of checkpatch script about the not documented
> > DT compatible string for vendor "mtk". Add it to vendor-prefixes.txt
> > file.
> > 
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> > index b5f978a..a588a29 100644
> > --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> > +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> > @@ -228,6 +228,7 @@ mqmaker	mqmaker Inc.
> >  mscc	Microsemi Corporation
> >  msi	Micro-Star International Co. Ltd.
> >  mti	Imagination Technologies Ltd. (formerly MIPS Technologies Inc.)
> > +mtk	MediaTek
> 
> MediaTek already has a binding here, it is "mediatek".  Please don't
> introduce a new one for the same company.

True, sorry for this.

> 
> Also, all device tree patches need to cc: the device tree maintainers to
> get their review before I can accept them.

Thanks for let me know.

> 
> thanks,
> 
> greg k-h

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

Re: [PATCH 4/5] dt-bindings: gpio: add documentation for mt7621-gpio

[Sergio Paracuellos]

On Tue, May 15, 2018 at 09:25:06AM +0200, Greg KH wrote:
> On Mon, May 14, 2018 at 04:02:33PM +0200, Sergio Paracuellos wrote:
> > This commit add missing dt bindings documentation for mt7621-gpio
> > driver. After this checkpatch script complain about this
> > issue dissapears.
> > 
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  .../devicetree/bindings/gpio/mtk,mt7621-gpio.txt   | 51 ++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt b/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
> > new file mode 100644
> > index 0000000..5fe4bb5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/mtk,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:
> > +  - "mtk,mt7621-gpio" for Mediatek controllers
> 
> As stated in the previous review, "mtk" is not valid, please use
> "mediatek" like the rest of the kernel does :)

I will and send new patch series with those fixed.

> 
> thanks,

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