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
