Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven [mailto:geert@xxxxxxxxxxxxxx] > Sent: 30 July 2018 11:04 > To: Biju Das <biju.das@xxxxxxxxxxxxxx> > Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>; open list:GPIO SUBSYSTEM > <linux-gpio@xxxxxxxxxxxxxxx>; Simon Horman <horms@xxxxxxxxxxxx>; > Geert Uytterhoeven <geert+renesas@xxxxxxxxx>; Chris Paterson > <Chris.Paterson2@xxxxxxxxxxx>; Fabrizio Castro > <fabrizio.castro@xxxxxxxxxxxxxx>; Linux-Renesas <linux-renesas- > soc@xxxxxxxxxxxxxxx> > Subject: Re: [PATCH 1/4] gpio: rcar: Enhance gpio-ranges support > > Hi Biju, > > On Fri, Jul 27, 2018 at 11:57 AM Biju Das <biju.das@xxxxxxxxxxxxxx> wrote: > > Enhance gpio-ranges to support more than one gpio-range. > > > > Signed-off-by: Biju Das <biju.das@xxxxxxxxxxxxxx> > > Reviewed-by: Fabrizio Castro <fabrizio.castro@xxxxxxxxxxxxxx> > > Thanks for your patch! > > However, I'm wondering if this works as intended, as the discontinuity is not > in the pins, but in the GPIO bits. > > > --- > > cat /sys/kernel/debug/pinctrl/e6060000.pin-controller-sh-pfc/gpio- > > GPIO ranges handled: > > 0: e6050000.gpio GPIOS [1001 - 1023] PINS [0 - 22] > > 0: e6051000.gpio GPIOS [978 - 1000] PINS [32 - 54] > > 0: e6052000.gpio GPIOS [946 - 977] PINS [64 - 95] > > 0: e6053000.gpio GPIOS [926 - 942] PINS [96 - 112] > > 17: e6053000.gpio GPIOS [943 - 945] PINS [123 - 125] > > The above two lines are the result of: > > + gpio-ranges = <&pfc 0 96 17>, <&pfc 17 123 3>; > > > 0: e6054000.gpio GPIOS [900 - 925] PINS [128 - 153] > > 0: e6055000.gpio GPIOS [868 - 899] PINS [160 - 191] > > --- > > drivers/gpio/gpio-rcar.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c index > > 350390c..a7bbe78 100644 > > --- a/drivers/gpio/gpio-rcar.c > > +++ b/drivers/gpio/gpio-rcar.c > > @@ -399,13 +399,22 @@ static int gpio_rcar_parse_dt(struct > gpio_rcar_priv *p, unsigned int *npins) > > struct device_node *np = p->pdev->dev.of_node; > > const struct gpio_rcar_info *info; > > struct of_phandle_args args; > > - int ret; > > + int index = 0, ret; > > > > info = of_device_get_match_data(&p->pdev->dev); > > > > - ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, 0, > &args); > > - *npins = ret == 0 ? args.args[2] : RCAR_MAX_GPIO_PER_BANK; > > p->has_both_edge_trigger = info->has_both_edge_trigger; > > + *npins = 0; > > + for (;; index++) { > > + ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, > > + index, &args); > > + if (ret) { > > + if (index == 0) > > + *npins = RCAR_MAX_GPIO_PER_BANK; > > + break; > > + } > > + *npins += args.args[2]; > > + } > > So after this, *npins will be the total number of GPIOs present in this bank > (17 + 3 = 20), which will be used as gpio_chip.ngpio. > > All GPIO operations will use the passed offset as the bit number. > gpio_rcar_resume() will resume bits 0..ngpio - 1 of each register. > gpio_rcar_set_multiple() uses GENMASK(chip->ngpio - 1, 0) as the bank > mask, and uses it to mask of bits in the registers. > > Hence all of the above assumes the register bits for the GPIOs are 0..19. > However, according to the datasheet, the GPIOs in bank 3 are 0..16 and > 27..29. So accessing GPIOs 17..19 in the bank will write to bits 17..19, not to > 27..29! I have done the below GPIO operation using sysfs to check this. (As per this gpio 943 mapped to gpiobit 17 which request Pin123(GP3_27) But I haven't checked GPIO registers to check whether it is actually setting the values to GP3_17 or GP3_27. Will check this. root@iwg23s:~# echo 943 > /sys/class/gpio/export [ 51.796570] sh-pfc e6060000.pin-controller: request pin 123 (GP_3_27) for e6053000.gpio:943 [ 51.804960] sh-pfc e6060000.pin-controller: write_reg addr = e6060010, value = 0x0, field = 4, r_width = 32, f_width = 1 Kernel logs:- ---------------- [ 0.110156] gpio gpiochip3: (e6053000.gpio): created GPIO range 0->16 ==> e6060000.pin-controller PIN 96->112 [ 0.110187] gpio gpiochip3: (e6053000.gpio): created GPIO range 17->19 ==> e6060000.pin-controller PIN 123->125 [ 0.110363] gpio gpiochip3: (e6053000.gpio): added GPIO chardev (254:3) [ 0.110460] gpiochip_setup_dev: registered GPIOs 926 to 945 on device: gpiochip3 (e6053000.gpio) PIncontrol related for GPIO 3 ---------------------------------------- pin 96 (GP_3_0): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 97 (GP_3_1): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 98 (GP_3_2): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 99 (GP_3_3): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 100 (GP_3_4): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 101 (GP_3_5): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 102 (GP_3_6): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 103 (GP_3_7): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 104 (GP_3_8): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 105 (GP_3_9): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 106 (GP_3_10): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 107 (GP_3_11): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 108 (GP_3_12): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 109 (GP_3_13): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 110 (GP_3_14): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 111 (GP_3_15): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 112 (GP_3_16): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 123 (GP_3_27): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 124 (GP_3_28): (MUX UNCLAIMED) (GPIO UNCLAIMED) pin 125 (GP_3_29): (MUX UNCLAIMED) (GPIO UNCLAIMED) range seen by pin control driver ------------------------------------------- cat /sys/kernel/debug/pinctrl/e6060000.pin-controller-sh-pfc/gpio-ranges 0: e6050000.gpio GPIOS [1001 - 1023] PINS [0 - 22] 0: e6051000.gpio GPIOS [978 - 1000] PINS [32 - 54] 0: e6052000.gpio GPIOS [946 - 977] PINS [64 - 95] 0: e6053000.gpio GPIOS [926 - 942] PINS [96 - 112] 17: e6053000.gpio GPIOS [943 - 945] PINS [123 - 125]-- > New gpio range mapping pin 123-125 0: e6054000.gpio GPIOS [900 - 925] PINS [128 - 153] 0: e6055000.gpio GPIOS [868 - 899] PINS [160 - 191] range seen by gpio driver ----------------------------- [ 0.105883] gpio_rcar e6050000.gpio: driving 23 GPIOs [ 0.106328] gpio_rcar e6051000.gpio: driving 23 GPIOs [ 0.106682] gpio_rcar e6052000.gpio: driving 32 GPIOs [ 0.107028] gpio_rcar e6053000.gpio: driving 20 GPIOs [ 0.107388] gpio_rcar e6054000.gpio: driving 26 GPIOs [ 0.107718] gpio_rcar e6055000.gpio: driving 32 GPIOs Regards, Biju > A simple way to work around this is to set ngpios to the highest bit number in > use + 1. But you still need a mechanism to avoid accessing the unused bits in > the gap between 16 and 27. > > > > if (*npins == 0 || *npins > RCAR_MAX_GPIO_PER_BANK) { > > dev_warn(&p->pdev->dev, > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
