On Mon, Jun 18 2018, Sergio Paracuellos wrote:

> Banks shouldn't be defined in DT if number of resources
> per bank is not variable. We actually know that this SoC
> has three banks so take that into account in order to don't
> overspecify the device tree. Device tree will only have one
> node making it simple. Update device tree, binding doc and
> code accordly.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuel...@gmail.com>

I'm sorry that I've been silent one these for a while - busy any all
that.

This last patch doesn't work.  My test case works with all but this one
applied, but this breaks it.  I haven't had a chance to look into why
yet.  Sorry.

NeilBrown

> ---
>  drivers/staging/mt7621-dts/gbpc1.dts               | 10 ++--
>  drivers/staging/mt7621-dts/mt7621.dtsi             | 31 ++----------
>  drivers/staging/mt7621-gpio/gpio-mt7621.c          | 21 ++++----
>  .../staging/mt7621-gpio/mediatek,mt7621-gpio.txt   | 59 
> +++++-----------------
>  4 files changed, 31 insertions(+), 90 deletions(-)
>
> diff --git a/drivers/staging/mt7621-dts/gbpc1.dts 
> b/drivers/staging/mt7621-dts/gbpc1.dts
> index 6b13d85..352945d 100644
> --- a/drivers/staging/mt7621-dts/gbpc1.dts
> +++ b/drivers/staging/mt7621-dts/gbpc1.dts
> @@ -32,7 +32,7 @@
>  
>               reset {
>                       label = "reset";
> -                     gpios = <&gpio0 18 GPIO_ACTIVE_HIGH>;
> +                     gpios = <&gpio 18 GPIO_ACTIVE_HIGH>;
>                       linux,code = <KEY_RESTART>;
>               };
>       };
> @@ -42,22 +42,22 @@
>  
>               system {
>                       label = "gb-pc1:green:system";
> -                     gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;
> +                     gpios = <&gpio 6 GPIO_ACTIVE_LOW>;
>               };
>  
>               status {
>                       label = "gb-pc1:green:status";
> -                     gpios = <&gpio0 8 GPIO_ACTIVE_LOW>;
> +                     gpios = <&gpio 8 GPIO_ACTIVE_LOW>;
>               };
>  
>               lan1 {
>                       label = "gb-pc1:green:lan1";
> -                     gpios = <&gpio0 24 GPIO_ACTIVE_LOW>;
> +                     gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
>               };
>  
>               lan2 {
>                       label = "gb-pc1:green:lan2";
> -                     gpios = <&gpio0 25 GPIO_ACTIVE_LOW>;
> +                     gpios = <&gpio 25 GPIO_ACTIVE_LOW>;
>               };
>       };
>  };
> diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi 
> b/drivers/staging/mt7621-dts/mt7621.dtsi
> index acb6b8c..02746af 100644
> --- a/drivers/staging/mt7621-dts/mt7621.dtsi
> +++ b/drivers/staging/mt7621-dts/mt7621.dtsi
> @@ -61,37 +61,14 @@
>               };
>  
>               gpio: gpio@600 {
> -                     #address-cells = <1>;
> -                     #size-cells = <0>;
> -
> +                     #gpio-cells = <2>;
> +                     #interrupt-cells = <2>;
>                       compatible = "mediatek,mt7621-gpio";
> +                     gpio-controller;
> +                     interrupt-controller;
>                       reg = <0x600 0x100>;
> -
>                       interrupt-parent = <&gic>;
>                       interrupts = <GIC_SHARED 12 IRQ_TYPE_LEVEL_HIGH>;
> -                     interrupt-controller;
> -                     #interrupt-cells = <2>;
> -
> -                     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>;
> -                     };
>               };
>  
>               i2c: i2c@900 {
> diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c 
> b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> index 9a4a12f..281e621 100644
> --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> @@ -206,23 +206,20 @@ static inline const char * const 
> mediatek_gpio_bank_name(int bank)
>  }
>  
>  static int
> -mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node 
> *bank)
> +mediatek_gpio_bank_probe(struct platform_device *pdev,
> +                      struct device_node *node, int bank)
>  {
>       struct mtk_data *gpio = dev_get_drvdata(&pdev->dev);
> -     const __be32 *id = of_get_property(bank, "reg", NULL);
>       struct mtk_gc *rg;
>       void __iomem *dat, *set, *ctrl, *diro;
>       int ret;
>  
> -     if (!id || be32_to_cpu(*id) >= MTK_BANK_CNT)
> -             return -EINVAL;
> -
> -     rg = &gpio->gc_map[be32_to_cpu(*id)];
> +     rg = &gpio->gc_map[bank];
>       memset(rg, 0, sizeof(*rg));
>  
>       spin_lock_init(&rg->lock);
> -     rg->chip.of_node = bank;
> -     rg->bank = be32_to_cpu(*id);
> +     rg->chip.of_node = node;
> +     rg->bank = bank;
>       rg->chip.label = mediatek_gpio_bank_name(rg->bank);
>  
>       dat = gpio->gpio_membase + GPIO_REG_DATA + (rg->bank * GPIO_BANK_WIDE);
> @@ -283,9 +280,10 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, 
> struct device_node *bank)
>  static int
>  mediatek_gpio_probe(struct platform_device *pdev)
>  {
> -     struct device_node *bank, *np = pdev->dev.of_node;
> +     struct device_node *np = pdev->dev.of_node;
>       struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>       struct mtk_data *gpio_data;
> +     int i;
>  
>       gpio_data = devm_kzalloc(&pdev->dev, sizeof(*gpio_data), GFP_KERNEL);
>       if (!gpio_data)
> @@ -299,9 +297,8 @@ mediatek_gpio_probe(struct platform_device *pdev)
>       gpio_data->dev = &pdev->dev;
>       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);
> +     for (i = 0; i < MTK_BANK_CNT; i++)
> +             mediatek_gpio_bank_probe(pdev, np, i);
>  
>       return 0;
>  }
> diff --git a/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt 
> b/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
> index 30d8a02..ba45558 100644
> --- a/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
> +++ b/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
> @@ -1,68 +1,35 @@
> -Mediatek SoC GPIO controller bindings
> +Mediatek MT7621 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. Also the GPIO controller can 
> receive
> +We load one GPIO controller instance per 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:
> +- #gpio-cells : Should be two. The first cell is the GPIO pin number and the
> +   second cell specifies GPIO flags, as defined in <dt-bindings/gpio/gpio.h>.
> +   Only the GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> +   interrupt. Should be 2. The first cell defines the interrupt number,
> +   the second encodes the triger flags encoded as described in
> +   Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>  - 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.
>  - interrupt-controller : Mark the device node as an interrupt controller.
> -- #interrupt-cells : Should be 2. The first cell defines the interrupt 
> number.
> -   The second cell bits[3:0] is used to specify trigger type as follows:
> -     - 1 = low-to-high edge triggered.
> -     - 2 = high-to-low edge triggered.
> -     - 4 = active high level-sensitive.
> -     - 8 = active low level-sensitive.
> -
> -
> -Required properties for the GPIO bank node:
> -- compatible:
> -  - "mediatek,mt7621-gpio-bank" for Mediatek banks
> -- #gpio-cells : Should be two. The first cell is the GPIO pin number and the
> -   second cell specifies GPIO flags, as defined in <dt-bindings/gpio/gpio.h>.
> -   Only the GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
>  - 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>;
> -
> +             #gpio-cells = <2>;
> +             #interrupt-cells = <2>;
>               compatible = "mediatek,mt7621-gpio";
> +             gpio-controller;
> +             interrupt-controller;
>               reg = <0x600 0x100>;
> -
>               interrupt-parent = <&gic>;
>               interrupts = <GIC_SHARED 12 IRQ_TYPE_LEVEL_HIGH>;
> -             interrupt-controller;
> -             #interrupt-cells = <2>;
> -
> -             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

Attachment: signature.asc
Description: PGP signature

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

Reply via email to