On Sat, Jun 30 2018, Sergio Paracuellos wrote:
> 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.
>
OK, I finally made time to dig into this.
The problem is that the default gpio.of_xlate function assumes there is
one gpio chip for each devicetree node. With this patch applied, the
one device tree node corresponds to 3 different gpio chips. For that to
work we need an xlate function.
See below for what I wrote to get it working.
With this in place:
/sys/class/gpio still contains:
export gpiochip416 gpiochip448 gpiochip480 unexport
which is a little annoying, but unavoidable I guess.
The labels on these are:
# grep . /sys/class/gpio/gpiochip4*/label
/sys/class/gpio/gpiochip416/label:1e000600.gpio
/sys/class/gpio/gpiochip448/label:1e000600.gpio
/sys/class/gpio/gpiochip480/label:1e000600.gpio
.. all the same, which is not ideal.
Your attempt to change the names doesn't work because bgpio_init() sets
the names itself. If you move the assignment to rg->chip.label to
*after* the call to bgpio_init(), the new names work:
# grep . /sys/class/gpio/gpiochip4*/label
/sys/class/gpio/gpiochip416/label:mt7621-bank2
/sys/class/gpio/gpiochip448/label:mt7621-bank1
/sys/class/gpio/gpiochip480/label:mt7621-bank0
Also with that change /proc/interrupts contains:
17: 0 0 0 0 MIPS GIC 19 mt7621-bank0, mt7621-bank1, mt7621-bank2
and
26: 0 0 0 0 mt7621-bank2 18 reset
The first line looks good - though having "gpio" in the name might be
good. Should they be 1e000600.gpio-bankN" ??
The second is a bit weird, as this isn't bank2.
It probably makes sense to have "1e000600.gpio 18 reset" there...
There is only 1 irq chip, compared with 3 gpio chips, so a different
name is appropriate. I changed the irq chip name:
mediatek_gpio_irq_chip.name = dev_name(&pdev->dev);
though maybe that assignment should go elsewhere - maybe in
mediatek_gpio_probe() as it is only needed once.
Thanks,
NeilBrown
diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 281e6214d543..814af9342d25 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -205,6 +205,22 @@ static inline const char * const mediatek_gpio_bank_name(int bank)
return bank_names[bank];
}
+static int mediatek_gpio_xlate(struct gpio_chip *chip,
+ const struct of_phandle_args *spec,
+ u32 *flags)
+{
+ int gpio = spec->args[0];
+ struct mtk_gc *rq = container_of(chip, struct mtk_gc, chip);
+
+ if (rq->bank != gpio / MTK_BANK_WIDTH)
+ return -EINVAL;
+
+ if (flags)
+ *flags = spec->args[1];
+
+ return gpio % MTK_BANK_WIDTH;
+}
+
static int
mediatek_gpio_bank_probe(struct platform_device *pdev,
struct device_node *node, int bank)
@@ -221,6 +237,8 @@ mediatek_gpio_bank_probe(struct platform_device *pdev,
rg->chip.of_node = node;
rg->bank = bank;
rg->chip.label = mediatek_gpio_bank_name(rg->bank);
+ rg->chip.of_gpio_n_cells = 2;
+ rg->chip.of_xlate = mediatek_gpio_xlate;
dat = gpio->gpio_membase + GPIO_REG_DATA + (rg->bank * GPIO_BANK_WIDE);
set = gpio->gpio_membase + GPIO_REG_DSET + (rg->bank * GPIO_BANK_WIDE);
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
