| Message ID | abb46a587b76d379ad32d53817d837d8a5fea8bd.1661789204.git.christophe.leroy@csgroup.eu |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: Get rid of ARCH_NR_GPIOS (v1) | expand |
On Wed, Aug 31, 2022, at 7:49 AM, Christophe Leroy wrote: > Le 30/08/2022 à 22:18, Andy Shevchenko a écrit : >> On Mon, Aug 29, 2022 at 7:19 PM Christophe Leroy >> <christophe.leroy@csgroup.eu> wrote: >>> >>> Since commit 14e85c0e69d5 ("gpio: remove gpio_descs global array") >>> there is no limitation on the number of GPIOs that can be allocated >>> in the system since the allocation is fully dynamic. >>> >>> ARCH_NR_GPIOS is today only used in order to provide downwards >>> gpiobase allocation from that value, while static allocation is >>> performed upwards from 0. However that has the disadvantage of >>> limiting the number of GPIOs that can be registered in the system. >>> >>> To overcome this limitation without requiring each and every >>> platform to provide its 'best-guess' maximum number, rework the >>> allocation to allocate upwards, allowing approx 2 millions of >>> GPIOs. >>> >>> In order to still allow static allocation for legacy drivers, define >>> GPIO_DYNAMIC_BASE with the value 256 as the start for dynamic >>> allocation. >> >> Not sure about 256, but I understand that this can only be the best guess. >> > > Well, it's already just a precaution. Linus W's expectation is that > static ones are allocated at first, they should already be allocated > when we start doing dynamic allocations so he was even thinking that we > could have started at 0 already. > > But I can start higher if you think it is safer, maybe at 512 which is > the default ARCH_NR_GPIOS today. FWIW, I went through the drivers that set the base to a value other than zero or -1, to see what they use: arch/arm/common/scoop.c: devptr->gpio.base = inf->gpio_base; // 204 arch/arm/mach-s3c/gpio-samsung.c: .base = S3C2410_GPM(0), // 384 arch/arm/mach-s3c/gpio-samsung.c: .base = S3C64XX_GPQ(0), // 197 arch/arm/mach-s3c/mach-h1940.c: .base = H1940_LATCH_GPIO(0), // 384 + 22 arch/arm/mach-sa1100/simpad.c: cs3_gpio.base = SIMPAD_CS3_GPIO_BASE; // 27 + 11 arch/arm/plat-orion/gpio.c: ochip->chip.base = gpio_base; // 64 + 32 arch/mips/alchemy/common/gpiolib.c: .base = ALCHEMY_GPIO2_BASE, // 32 + 16 arch/mips/alchemy/common/gpiolib.c: .base = AU1300_GPIO_BASE, // 0 + 75 arch/mips/kernel/gpio_txx9.c: txx9_gpio_chip.base = base; // 0 + 16 arch/mips/txx9/generic/setup.c: iocled->chip.base = basenum; -1 drivers/bcma/driver_gpio.c: chip->base = bus->num * BCMA_GPIO_MAX_PINS; // probably 0 drivers/gpio/gpio-adp5520.c: gc->base = pdata->gpio_start; // unused drivers/gpio/gpio-adp5588.c: gc->base = pdata->gpio_start; // unused drivers/gpio/gpio-arizona.c: arizona_gpio->gpio_chip.base = pdata->gpio_base; // 197 drivers/gpio/gpio-brcmstb.c: gc->base = gpio_base; // 2 * 32 drivers/gpio/gpio-bt8xx.c: c->base = modparam_gpiobase; // from modprobe drivers/gpio/gpio-da9052.c: gpio->gp.base = pdata->gpio_base; // unused drivers/gpio/gpio-da9055.c: gpio->gp.base = pdata->gpio_base; // unused drivers/gpio/gpio-davinci.c: chips->chip.base = pdata->no_auto_base ? pdata->base : -1; // 0 + 144 drivers/gpio/gpio-dwapb.c: port->gc.base = pp->gpio_base; // from DT, deprecated drivers/gpio/gpio-f7188x.c: .base = _base, \ // 10*10, unused drivers/gpio/gpio-htc-egpio.c: chip->base = pdata->chip[i].gpio_base; // 192 + 5 * 8 drivers/gpio/gpio-ich.c: chip->base = modparam_gpiobase; // from modprobe drivers/gpio/gpio-kempld.c: chip->base = pdata->gpio_base; // 0 drivers/gpio/gpio-lpc32xx.c: .base = LPC32XX_GPO_P3_GRP, // 104 drivers/gpio/gpio-madera.c: madera_gpio->gpio_chip.base = pdata->gpio_base; // -1 drivers/gpio/gpio-max730x.c: ts->chip.base = pdata->base; // 200 + 28 (timberdale) drivers/gpio/gpio-max732x.c: gc->base = gpio_start; // 192 drivers/gpio/gpio-mc33880.c: mc->chip.base = pdata->base; // 100 (timberdale) drivers/gpio/gpio-merrifield.c: priv->chip.base = gpio_base; // 0 + 213 drivers/gpio/gpio-mmio.c: gc->base = pdata->base; // 197 + 32 drivers/gpio/gpio-mockup.c: gc->base = base; // module parama drivers/gpio/gpio-mvebu.c: mvchip->chip.base = id * MVEBU_MAX_GPIO_PER_BANK; // 4 * 32, from DT drivers/gpio/gpio-omap.c: bank->chip.base = OMAP_MPUIO(0); // 192 drivers/gpio/gpio-omap.c: bank->chip.base = gpio; // 7 * 32 drivers/gpio/gpio-palmas.c: palmas_gpio->gpio_chip.base = palmas_pdata->gpio_base; // unused drivers/gpio/gpio-pca953x.c: gc->base = chip->gpio_start; // ???? used a lot drivers/gpio/gpio-pcf857x.c: gpio->chip.base = pdata ? pdata->gpio_base : -1; // 160 drivers/gpio/gpio-rc5t583.c: rc5t583_gpio->gpio_chip.base = pdata->gpio_base; // unused drivers/gpio/gpio-rockchip.c: gc->base = bank->pin_base; // 8 * 32 drivers/gpio/gpio-sch311x.c: block->chip.base = sch311x_gpio_blocks[i].base; // 6 * 10 drivers/gpio/gpio-sta2x11.c: gpio->base = gpio_base; // unused drivers/gpio/gpio-timberdale.c: gc->base = pdata->gpio_base; // 0 + 100 drivers/gpio/gpio-tps6586x.c: tps6586x_gpio->gpio_chip.base = pdata->gpio_base; // -1 drivers/gpio/gpio-tps65910.c: tps65910_gpio->gpio_chip.base = pdata->gpio_base; // -1 drivers/gpio/gpio-ucb1400.c: ucb->gc.base = ucb->gpio_offset; // 0 drivers/gpio/gpio-wm831x.c: wm831x_gpio->gpio_chip.base = pdata->gpio_base; // 197 + 64 drivers/gpio/gpio-wm8350.c: wm8350_gpio->gpio_chip.base = pdata->gpio_base; // 0 drivers/gpio/gpio-wm8994.c: wm8994_gpio->gpio_chip.base = pdata->gpio_base; 197 + 8 drivers/input/keyboard/adp5588-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused drivers/input/keyboard/adp5589-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused drivers/leds/leds-pca9532.c: data->gpio.base = pdata->gpio_base; // unused drivers/leds/leds-tca6507.c: tca->gpio.base = pdata->gpio_base; // unused drivers/mfd/asic3.c: asic->gpio.base = pdata->gpio_base; // 300 + 100 drivers/mfd/htc-i2cpld.c: gpio_chip->base = plat_chip_data->gpio_in_base; // 192 + 16 + 8*8 drivers/mfd/sm501.c: gchip->base = base; // 0 drivers/mfd/tps65010.c: tps->chip.base = board->base; // 204 drivers/mfd/ucb1x00-core.c: ucb->gpio.base = pdata->gpio_base; // 27 + 13 drivers/pinctrl/nomadik/pinctrl-nomadik.c: chip->base = id * NMK_GPIO_PER_CHIP; // 9 * 32 drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c: pctrl->gpio_bank[id].gc.base = args.args[1]; // 8*32, from DT drivers/pinctrl/pinctrl-at91.c: chip->base = alias_idx * MAX_NB_GPIO_PER_BANK; // 5*32 drivers/pinctrl/pinctrl-ingenic.c: jzgc->gc.base = bank * 32; // 6 * 32, from DT drivers/pinctrl/pinctrl-mcp23s08.c: mcp->chip.base = base; // -1 drivers/pinctrl/pinctrl-oxnas.c: .base = GPIO_BANK_START(_bank), // 2*32 drivers/pinctrl/pinctrl-pic32.c: .base = GPIO_BANK_START(_bank), // 10 * 16 drivers/pinctrl/pinctrl-pistachio.c: .base = _pin_base, // 9 * 16 drivers/pinctrl/pinctrl-st.c: bank->gpio_chip.base = bank_num * ST_GPIO_PINS_PER_BANK; // 26 * 8 drivers/pinctrl/renesas/gpio.c: gc->base = pfc->nr_gpio_pins; // ??? don't understand drivers/pinctrl/samsung/pinctrl-samsung.c: gc->base = bank->grange.base; drivers/pinctrl/stm32/pinctrl-stm32.c: bank->gpio_chip.base = args.args[1]; drivers/pinctrl/stm32/pinctrl-stm32.c: bank->gpio_chip.base = bank_nr * STM32_GPIO_PINS_PER_BANK; drivers/pinctrl/stm32/pinctrl-stm32.c: bank->gpio_chip.base = bank_nr * STM32_GPIO_PINS_PER_BANK; drivers/pinctrl/sunxi/pinctrl-sunxi.c: pctl->chip->base = pctl->desc->pin_base; sound/soc/codecs/wm5100.c: wm5100->gpio_chip.base = wm5100->pdata.gpio_base; // 197 + 8 + 6 sound/soc/codecs/wm8903.c: wm8903->gpio_chip.base = pdata->gpio_base; // 197 + 8 sound/soc/codecs/wm8962.c: wm8962->gpio_chip.base = pdata->gpio_base; // 197 + 8 sound/soc/codecs/wm8996.c: wm8996->gpio_chip.base = wm8996->pdata.gpio_base; // 197 + 8 Arnd
Le 31/08/2022 à 22:55, Arnd Bergmann a écrit : > On Wed, Aug 31, 2022, at 7:49 AM, Christophe Leroy wrote: >> Le 30/08/2022 à 22:18, Andy Shevchenko a écrit : >>> On Mon, Aug 29, 2022 at 7:19 PM Christophe Leroy >>> <christophe.leroy@csgroup.eu> wrote: >>>> >>>> Since commit 14e85c0e69d5 ("gpio: remove gpio_descs global array") >>>> there is no limitation on the number of GPIOs that can be allocated >>>> in the system since the allocation is fully dynamic. >>>> >>>> ARCH_NR_GPIOS is today only used in order to provide downwards >>>> gpiobase allocation from that value, while static allocation is >>>> performed upwards from 0. However that has the disadvantage of >>>> limiting the number of GPIOs that can be registered in the system. >>>> >>>> To overcome this limitation without requiring each and every >>>> platform to provide its 'best-guess' maximum number, rework the >>>> allocation to allocate upwards, allowing approx 2 millions of >>>> GPIOs. >>>> >>>> In order to still allow static allocation for legacy drivers, define >>>> GPIO_DYNAMIC_BASE with the value 256 as the start for dynamic >>>> allocation. >>> >>> Not sure about 256, but I understand that this can only be the best guess. >>> >> >> Well, it's already just a precaution. Linus W's expectation is that >> static ones are allocated at first, they should already be allocated >> when we start doing dynamic allocations so he was even thinking that we >> could have started at 0 already. >> >> But I can start higher if you think it is safer, maybe at 512 which is >> the default ARCH_NR_GPIOS today. > > FWIW, I went through the drivers that set the base to a value other than > zero or -1, to see what they use: > > arch/arm/mach-s3c/gpio-samsung.c: .base = S3C2410_GPM(0), // 384 > arch/arm/mach-s3c/mach-h1940.c: .base = H1940_LATCH_GPIO(0), // 384 + 22 > drivers/mfd/asic3.c: asic->gpio.base = pdata->gpio_base; // 300 + 100 > drivers/mfd/htc-i2cpld.c: gpio_chip->base = plat_chip_data->gpio_in_base; // 192 + 16 + 8*8 Ok, so 256 is not enough, have to go to 512. Christophe
(Nuno, one point below for you) On Wed, Aug 31, 2022 at 11:55 PM Arnd Bergmann <arnd@arndb.de> wrote: ... > drivers/gpio/gpio-adp5520.c: gc->base = pdata->gpio_start; // unused > drivers/gpio/gpio-adp5588.c: gc->base = pdata->gpio_start; // unused > drivers/input/keyboard/adp5588-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused > drivers/input/keyboard/adp5589-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused I believe we should convert them to -1. > drivers/gpio/gpio-bt8xx.c: c->base = modparam_gpiobase; // from modprobe > drivers/gpio/gpio-ich.c: chip->base = modparam_gpiobase; // from modprobe I believe it was designed for Intel hardware and so it can't be higher than 512 - ngpios, where the latter one is small enough (dozen or a couple of dozens of pins). > drivers/gpio/gpio-dwapb.c: port->gc.base = pp->gpio_base; // from DT, deprecated
Hi Andy, CC linux-sh On Fri, Sep 2, 2022 at 12:53 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Wed, Aug 31, 2022 at 11:55 PM Arnd Bergmann <arnd@arndb.de> wrote: > > drivers/pinctrl/renesas/gpio.c: gc->base = pfc->nr_gpio_pins; // ??? don't understand > > I think, w/o looking into the code, that this just guarantees the > continuous numbering for all banks (chips) on the platform. This part of the code depends on CONFIG_PINCTRL_SH_FUNC_GPIO, which is used only on SH. 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
On Fri, 2022-09-02 at 13:52 +0300, Andy Shevchenko wrote: > (Nuno, one point below for you) > > On Wed, Aug 31, 2022 at 11:55 PM Arnd Bergmann <arnd@arndb.de> wrote: > > ... > > > drivers/gpio/gpio-adp5520.c: gc->base = pdata->gpio_start; // > > unused > > drivers/gpio/gpio-adp5588.c: gc->base = pdata- > > >gpio_start; // unused > > drivers/input/keyboard/adp5588-keys.c: kpad->gc.base = gpio_data- > > >gpio_start; // unused > > drivers/input/keyboard/adp5589-keys.c: kpad->gc.base = gpio_data- > > >gpio_start; // unused > > I believe we should convert them to -1. > Well, the adp5588-keys.c was already refactored [1] to use FW properties so that -1 will be used. In the process, gpio-adp5588.c was dropped. For the adp5589-keys.c driver, we might also need to do a similar work as I suspect there's no platform making use of pdata. Hence, yes, I believe -1 is the way to go. Ditto for gpio-adp5520.c... [1]: https://lore.kernel.org/linux-input/Yw7hRIbsTqOWVeyJ@google.com/T/#m382bec5c587241010d453ce1000bea2d34b86380 - Nuno Sá >
On Fri, Sep 2, 2022, at 12:52 PM, Andy Shevchenko wrote: > On Wed, Aug 31, 2022 at 11:55 PM Arnd Bergmann <arnd@arndb.de> wrote: > > ... > >> drivers/gpio/gpio-adp5520.c: gc->base = pdata->gpio_start; // unused >> drivers/gpio/gpio-adp5588.c: gc->base = pdata->gpio_start; // unused >> drivers/input/keyboard/adp5588-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused >> drivers/input/keyboard/adp5589-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused > > I believe we should convert them to -1. This is probably something we should do separately, but a lot of the drivers currently don't have support for probing from DT or any other firmware interface but rely on platform_data definitions from a board file that was never part of the upstream kernel. We are going to remove a lot more board files early next year, and I was hoping to follow up with a treewide cleanup of such drivers and remove a lot of them entirely. >> drivers/gpio/gpio-dwapb.c: port->gc.base = pp->gpio_base; // from DT, deprecated > > From board files, since some platforms expect a fixed number for it. >> drivers/gpio/gpio-pca953x.c: gc->base = chip->gpio_start; // ???? used a lot > > To answer this one needs to go via all board files (most of them ARM > 32-bit based) and look, but it means almost the same case as per Intel > above: 512-ngpios. Right, I went through all the board files the other drivers, this one just happens to be used more than the others: arch/arm/mach-davinci/board-da850-evm.c:#include <linux/platform_data/pca953x.h> arch/arm/mach-ep93xx/vision_ep9307.c:#include <linux/platform_data/pca953x.h> arch/arm/mach-mmp/ttc_dkb.c:#include <linux/platform_data/pca953x.h> arch/arm/mach-pxa/cm-x300.c:#include <linux/platform_data/pca953x.h> arch/arm/mach-pxa/spitz.c:#include <linux/platform_data/pca953x.h> arch/arm/mach-pxa/zeus.c:#include <linux/platform_data/pca953x.h> arch/arm/mach-pxa/zylonite_pxa300.c:#include <linux/platform_data/pca953x.h> arch/arm/mach-s3c/mach-crag6410.c:#include <linux/platform_data/pca953x.h> The only ones that have known users though are crag6410 and vision_ep9307, the other ones will be removed. Vision-ep9307 has 128 GPIOs total, crag6410 is complicated because it many different GPIO controllers in various combinations. >> drivers/pinctrl/renesas/gpio.c: gc->base = pfc->nr_gpio_pins; // ??? don't understand > > I think, w/o looking into the code, that this just guarantees the > continuous numbering for all banks (chips) on the platform. Yes, that seems to be the idea most of the pinctrl drivers. Arnd
diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h index f3bb8a2bf788..4ebbb58f06ea 100644 --- a/arch/arm/include/asm/gpio.h +++ b/arch/arm/include/asm/gpio.h @@ -2,7 +2,6 @@ #ifndef _ARCH_ARM_GPIO_H #define _ARCH_ARM_GPIO_H -/* Note: this may rely upon the value of ARCH_NR_GPIOS set in mach/gpio.h */ #include <asm-generic/gpio.h> /* The trivial gpiolib dispatchers */ diff --git a/drivers/gpio/gpio-sta2x11.c b/drivers/gpio/gpio-sta2x11.c index e07cca0f8d35..1206b2877caa 100644 --- a/drivers/gpio/gpio-sta2x11.c +++ b/drivers/gpio/gpio-sta2x11.c @@ -108,8 +108,8 @@ static void gsta_gpio_setup(struct gsta_gpio *chip) /* called from probe */ struct gpio_chip *gpio = &chip->gpio; /* - * ARCH_NR_GPIOS is currently 256 and dynamic allocation starts - * from the end. However, for compatibility, we need the first + * Dynamic allocation starts at GPIO_DYNAMIC_BASE. + * However, for compatibility, we need the first * ConneXt device to start from gpio 0: it's the main chipset * on most boards so documents and drivers assume gpio0..gpio127 */ @@ -129,7 +129,6 @@ static void gsta_gpio_setup(struct gsta_gpio *chip) /* called from probe */ /* * After the first device, turn to dynamic gpio numbers. - * For example, with ARCH_NR_GPIOS = 256 we can fit two cards */ if (!gpio_base) gpio_base = -1; diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 3a6f29eeb72d..40ebc3d42a78 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -183,14 +183,14 @@ EXPORT_SYMBOL_GPL(gpiod_to_chip); static int gpiochip_find_base(int ngpio) { struct gpio_device *gdev; - int base = ARCH_NR_GPIOS - ngpio; + int base = GPIO_DYNAMIC_BASE; - list_for_each_entry_reverse(gdev, &gpio_devices, list) { + list_for_each_entry(gdev, &gpio_devices, list) { /* found a free space? */ - if (gdev->base + gdev->ngpio <= base) + if (gdev->base >= base + ngpio) break; - /* nope, check the space right before the chip */ - base = gdev->base - ngpio; + /* nope, check the space right after the chip */ + base = gdev->base + gdev->ngpio; } if (gpio_is_valid(base)) { diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index aea9aee1f3e9..06a817d9de79 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -11,40 +11,18 @@ #include <linux/gpio/driver.h> #include <linux/gpio/consumer.h> -/* Platforms may implement their GPIO interface with library code, +/* + * Platforms may implement their GPIO interface with library code, * at a small performance cost for non-inlined operations and some * extra memory (for code and for per-GPIO table entries). - * - * While the GPIO programming interface defines valid GPIO numbers - * to be in the range 0..MAX_INT, this library restricts them to the - * smaller range 0..ARCH_NR_GPIOS-1. - * - * ARCH_NR_GPIOS is somewhat arbitrary; it usually reflects the sum of - * builtin/SoC GPIOs plus a number of GPIOs on expanders; the latter is - * actually an estimate of a board-specific value. */ -#ifndef ARCH_NR_GPIOS -#if defined(CONFIG_ARCH_NR_GPIO) && CONFIG_ARCH_NR_GPIO > 0 -#define ARCH_NR_GPIOS CONFIG_ARCH_NR_GPIO -#else -#define ARCH_NR_GPIOS 512 -#endif -#endif - /* - * "valid" GPIO numbers are nonnegative and may be passed to - * setup routines like gpio_request(). only some valid numbers - * can successfully be requested and used. - * - * Invalid GPIO numbers are useful for indicating no-such-GPIO in - * platform data and other tables. + * At the end we want all GPIOs to be dynamically allocated from 0. + * However, some legacy drivers still perform fixed allocation. + * Until they are all fixed, leave 0-256 space for them. */ - -static inline bool gpio_is_valid(int number) -{ - return number >= 0 && number < ARCH_NR_GPIOS; -} +#define GPIO_DYNAMIC_BASE 256 struct device; struct gpio; @@ -140,12 +118,6 @@ static inline void gpio_unexport(unsigned gpio) #include <linux/kernel.h> -static inline bool gpio_is_valid(int number) -{ - /* only non-negative numbers are valid */ - return number >= 0; -} - /* platforms that don't directly support access to GPIOs through I2C, SPI, * or other blocking infrastructure can use these wrappers. */ @@ -169,4 +141,19 @@ static inline void gpio_set_value_cansleep(unsigned gpio, int value) #endif /* !CONFIG_GPIOLIB */ +/* + * "valid" GPIO numbers are nonnegative and may be passed to + * setup routines like gpio_request(). only some valid numbers + * can successfully be requested and used. + * + * Invalid GPIO numbers are useful for indicating no-such-GPIO in + * platform data and other tables. + */ + +static inline bool gpio_is_valid(int number) +{ + /* only non-negative numbers are valid */ + return number >= 0; +} + #endif /* _ASM_GENERIC_GPIO_H */
Since commit 14e85c0e69d5 ("gpio: remove gpio_descs global array") there is no limitation on the number of GPIOs that can be allocated in the system since the allocation is fully dynamic. ARCH_NR_GPIOS is today only used in order to provide downwards gpiobase allocation from that value, while static allocation is performed upwards from 0. However that has the disadvantage of limiting the number of GPIOs that can be registered in the system. To overcome this limitation without requiring each and every platform to provide its 'best-guess' maximum number, rework the allocation to allocate upwards, allowing approx 2 millions of GPIOs. In order to still allow static allocation for legacy drivers, define GPIO_DYNAMIC_BASE with the value 256 as the start for dynamic allocation. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- arch/arm/include/asm/gpio.h | 1 - drivers/gpio/gpio-sta2x11.c | 5 ++-- drivers/gpio/gpiolib.c | 10 +++---- include/asm-generic/gpio.h | 55 ++++++++++++++----------------------- 4 files changed, 28 insertions(+), 43 deletions(-)