Re: [PATCH v5 18/18] staging: mt7621-gpio: avoid use banks in device tree

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

 



On Sat, Jun 30, 2018 at 7:47 AM, NeilBrown <neil@xxxxxxxxxx> wrote:
> 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.paracuellos@xxxxxxxxx>
>
> 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.

Thanks for pointing this out. All of them were applied so I though
there were no problem at all with any of them.
We should revert the last one or wait to your feedback about what is
breaking it and send a new patch fixing the problem.

Thanks in advance.

> NeilBrown

Best regards,
    Sergio Paracuellos

>
>> ---
>>  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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux