Skip to content

Commit

Permalink
gpio: Remove dynamic allocation from populate_parent_alloc_arg()
Browse files Browse the repository at this point in the history
The gpiolib is unique in the way it uses intermediate fwspecs
when feeding an interrupt specifier to the parent domain, as it
relies on the populate_parent_alloc_arg() callback to perform
a dynamic allocation.

This is pretty inefficient (we free the structure almost immediately),
and the only reason this isn't a stack allocation is that our
ThunderX friend uses MSIs rather than a FW-constructed structure.

Let's solve it by providing a new type composed of the union
of a struct irq_fwspec and a msi_info_t, which satisfies both
requirements. This allows us to use a stack allocation, and we
can move the handful of users to this new scheme.

Also perform some additional cleanup, such as getting rid of the
stub versions of the irq_domain_translate_*cell helpers, which
are never used when CONFIG_IRQ_DOMAIN_HIERARCHY isn't selected.

Tested on a Tegra186.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: Daniel Palmer <daniel@thingy.jp>
Cc: Romain Perier <romain.perier@gmail.com>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Robert Richter <rric@kernel.org>
Cc: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Bartosz Golaszewski <brgl@bgdev.pl>
Link: https://lore.kernel.org/r/20220707182314.66610-2-prabhakar.mahadev-lad.rj@bp.renesas.com
  • Loading branch information
Marc Zyngier committed Jul 10, 2022
1 parent a111daf commit 91a29af
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 101 deletions.
15 changes: 6 additions & 9 deletions drivers/gpio/gpio-msc313.c
Original file line number Diff line number Diff line change
Expand Up @@ -550,23 +550,20 @@ static struct irq_chip msc313_gpio_irqchip = {
* so we need to provide the fwspec. Essentially gpiochip_populate_parent_fwspec_twocell
* that puts GIC_SPI into the first cell.
*/
static void *msc313_gpio_populate_parent_fwspec(struct gpio_chip *gc,
unsigned int parent_hwirq,
unsigned int parent_type)
static int msc313_gpio_populate_parent_fwspec(struct gpio_chip *gc,
union gpio_irq_fwspec *gfwspec,
unsigned int parent_hwirq,
unsigned int parent_type)
{
struct irq_fwspec *fwspec;

fwspec = kmalloc(sizeof(*fwspec), GFP_KERNEL);
if (!fwspec)
return NULL;
struct irq_fwspec *fwspec = &gfwspec->fwspec;

fwspec->fwnode = gc->irq.parent_domain->fwnode;
fwspec->param_count = 3;
fwspec->param[0] = GIC_SPI;
fwspec->param[1] = parent_hwirq;
fwspec->param[2] = parent_type;

return fwspec;
return 0;
}

static int msc313e_gpio_child_to_parent_hwirq(struct gpio_chip *chip,
Expand Down
15 changes: 6 additions & 9 deletions drivers/gpio/gpio-tegra.c
Original file line number Diff line number Diff line change
Expand Up @@ -443,23 +443,20 @@ static int tegra_gpio_child_to_parent_hwirq(struct gpio_chip *chip,
return 0;
}

static void *tegra_gpio_populate_parent_fwspec(struct gpio_chip *chip,
unsigned int parent_hwirq,
unsigned int parent_type)
static int tegra_gpio_populate_parent_fwspec(struct gpio_chip *chip,
union gpio_irq_fwspec *gfwspec,
unsigned int parent_hwirq,
unsigned int parent_type)
{
struct irq_fwspec *fwspec;

fwspec = kmalloc(sizeof(*fwspec), GFP_KERNEL);
if (!fwspec)
return NULL;
struct irq_fwspec *fwspec = &gfwspec->fwspec;

fwspec->fwnode = chip->irq.parent_domain->fwnode;
fwspec->param_count = 3;
fwspec->param[0] = 0;
fwspec->param[1] = parent_hwirq;
fwspec->param[2] = parent_type;

return fwspec;
return 0;
}

#ifdef CONFIG_PM_SLEEP
Expand Down
15 changes: 6 additions & 9 deletions drivers/gpio/gpio-tegra186.c
Original file line number Diff line number Diff line change
Expand Up @@ -621,24 +621,21 @@ static int tegra186_gpio_irq_domain_translate(struct irq_domain *domain,
return 0;
}

static void *tegra186_gpio_populate_parent_fwspec(struct gpio_chip *chip,
unsigned int parent_hwirq,
unsigned int parent_type)
static int tegra186_gpio_populate_parent_fwspec(struct gpio_chip *chip,
union gpio_irq_fwspec *gfwspec,
unsigned int parent_hwirq,
unsigned int parent_type)
{
struct tegra_gpio *gpio = gpiochip_get_data(chip);
struct irq_fwspec *fwspec;

fwspec = kmalloc(sizeof(*fwspec), GFP_KERNEL);
if (!fwspec)
return NULL;
struct irq_fwspec *fwspec = &gfwspec->fwspec;

fwspec->fwnode = chip->irq.parent_domain->fwnode;
fwspec->param_count = 3;
fwspec->param[0] = gpio->soc->instance;
fwspec->param[1] = parent_hwirq;
fwspec->param[2] = parent_type;

return fwspec;
return 0;
}

static int tegra186_gpio_child_to_parent_hwirq(struct gpio_chip *chip,
Expand Down
15 changes: 6 additions & 9 deletions drivers/gpio/gpio-thunderx.c
Original file line number Diff line number Diff line change
Expand Up @@ -408,18 +408,15 @@ static int thunderx_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
return 0;
}

static void *thunderx_gpio_populate_parent_alloc_info(struct gpio_chip *chip,
unsigned int parent_hwirq,
unsigned int parent_type)
static int thunderx_gpio_populate_parent_alloc_info(struct gpio_chip *chip,
union gpio_irq_fwspec *gfwspec,
unsigned int parent_hwirq,
unsigned int parent_type)
{
msi_alloc_info_t *info;

info = kmalloc(sizeof(*info), GFP_KERNEL);
if (!info)
return NULL;
msi_alloc_info_t *info = &gfwspec->msiinfo;

info->hwirq = parent_hwirq;
return info;
return 0;
}

static int thunderx_gpio_probe(struct pci_dev *pdev,
Expand Down
15 changes: 6 additions & 9 deletions drivers/gpio/gpio-visconti.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,23 +103,20 @@ static int visconti_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
return -EINVAL;
}

static void *visconti_gpio_populate_parent_fwspec(struct gpio_chip *chip,
unsigned int parent_hwirq,
unsigned int parent_type)
static int visconti_gpio_populate_parent_fwspec(struct gpio_chip *chip,
union gpio_irq_fwspec *gfwspec,
unsigned int parent_hwirq,
unsigned int parent_type)
{
struct irq_fwspec *fwspec;

fwspec = kmalloc(sizeof(*fwspec), GFP_KERNEL);
if (!fwspec)
return NULL;
struct irq_fwspec *fwspec = &gfwspec->fwspec;

fwspec->fwnode = chip->irq.parent_domain->fwnode;
fwspec->param_count = 3;
fwspec->param[0] = 0;
fwspec->param[1] = parent_hwirq;
fwspec->param[2] = parent_type;

return fwspec;
return 0;
}

static int visconti_gpio_probe(struct platform_device *pdev)
Expand Down
42 changes: 18 additions & 24 deletions drivers/gpio/gpiolib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
irq_hw_number_t hwirq;
unsigned int type = IRQ_TYPE_NONE;
struct irq_fwspec *fwspec = data;
void *parent_arg;
union gpio_irq_fwspec gpio_parent_fwspec = {};
unsigned int parent_hwirq;
unsigned int parent_type;
struct gpio_irq_chip *girq = &gc->irq;
Expand Down Expand Up @@ -1147,14 +1147,15 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
irq_set_probe(irq);

/* This parent only handles asserted level IRQs */
parent_arg = girq->populate_parent_alloc_arg(gc, parent_hwirq, parent_type);
if (!parent_arg)
return -ENOMEM;
ret = girq->populate_parent_alloc_arg(gc, &gpio_parent_fwspec,
parent_hwirq, parent_type);
if (ret)
return ret;

chip_dbg(gc, "alloc_irqs_parent for %d parent hwirq %d\n",
irq, parent_hwirq);
irq_set_lockdep_class(irq, gc->irq.lock_key, gc->irq.request_key);
ret = irq_domain_alloc_irqs_parent(d, irq, 1, parent_arg);
ret = irq_domain_alloc_irqs_parent(d, irq, 1, &gpio_parent_fwspec);
/*
* If the parent irqdomain is msi, the interrupts have already
* been allocated, so the EEXIST is good.
Expand All @@ -1166,7 +1167,6 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
"failed to allocate parent hwirq %d for hwirq %lu\n",
parent_hwirq, hwirq);

kfree(parent_arg);
return ret;
}

Expand Down Expand Up @@ -1230,34 +1230,28 @@ static bool gpiochip_hierarchy_is_hierarchical(struct gpio_chip *gc)
return !!gc->irq.parent_domain;
}

void *gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *gc,
unsigned int parent_hwirq,
unsigned int parent_type)
int gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *gc,
union gpio_irq_fwspec *gfwspec,
unsigned int parent_hwirq,
unsigned int parent_type)
{
struct irq_fwspec *fwspec;

fwspec = kmalloc(sizeof(*fwspec), GFP_KERNEL);
if (!fwspec)
return NULL;
struct irq_fwspec *fwspec = &gfwspec->fwspec;

fwspec->fwnode = gc->irq.parent_domain->fwnode;
fwspec->param_count = 2;
fwspec->param[0] = parent_hwirq;
fwspec->param[1] = parent_type;

return fwspec;
return 0;
}
EXPORT_SYMBOL_GPL(gpiochip_populate_parent_fwspec_twocell);

void *gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *gc,
unsigned int parent_hwirq,
unsigned int parent_type)
int gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *gc,
union gpio_irq_fwspec *gfwspec,
unsigned int parent_hwirq,
unsigned int parent_type)
{
struct irq_fwspec *fwspec;

fwspec = kmalloc(sizeof(*fwspec), GFP_KERNEL);
if (!fwspec)
return NULL;
struct irq_fwspec *fwspec = &gfwspec->fwspec;

fwspec->fwnode = gc->irq.parent_domain->fwnode;
fwspec->param_count = 4;
Expand All @@ -1266,7 +1260,7 @@ void *gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *gc,
fwspec->param[2] = 0;
fwspec->param[3] = parent_type;

return fwspec;
return 0;
}
EXPORT_SYMBOL_GPL(gpiochip_populate_parent_fwspec_fourcell);

Expand Down
15 changes: 6 additions & 9 deletions drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
Original file line number Diff line number Diff line change
Expand Up @@ -966,16 +966,13 @@ static int pmic_gpio_child_to_parent_hwirq(struct gpio_chip *chip,
return 0;
}

static void *pmic_gpio_populate_parent_fwspec(struct gpio_chip *chip,
unsigned int parent_hwirq,
unsigned int parent_type)
static int pmic_gpio_populate_parent_fwspec(struct gpio_chip *chip,
union gpio_irq_fwspec *gfwspec,
unsigned int parent_hwirq,
unsigned int parent_type)
{
struct pmic_gpio_state *state = gpiochip_get_data(chip);
struct irq_fwspec *fwspec;

fwspec = kzalloc(sizeof(*fwspec), GFP_KERNEL);
if (!fwspec)
return NULL;
struct irq_fwspec *fwspec = &gfwspec->fwspec;

fwspec->fwnode = chip->irq.parent_domain->fwnode;

Expand All @@ -985,7 +982,7 @@ static void *pmic_gpio_populate_parent_fwspec(struct gpio_chip *chip,
/* param[2] must be left as 0 */
fwspec->param[3] = parent_type;

return fwspec;
return 0;
}

static int pmic_gpio_probe(struct platform_device *pdev)
Expand Down
42 changes: 19 additions & 23 deletions include/linux/gpio/driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include <linux/property.h>
#include <linux/types.h>

#include <asm/msi.h>

struct gpio_desc;
struct of_phandle_args;
struct device_node;
Expand All @@ -23,6 +25,13 @@ enum gpio_lookup_flags;

struct gpio_chip;

union gpio_irq_fwspec {
struct irq_fwspec fwspec;
#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
msi_alloc_info_t msiinfo;
#endif
};

#define GPIO_LINE_DIRECTION_IN 1
#define GPIO_LINE_DIRECTION_OUT 0

Expand Down Expand Up @@ -103,9 +112,10 @@ struct gpio_irq_chip {
* variant named &gpiochip_populate_parent_fwspec_fourcell is also
* available.
*/
void *(*populate_parent_alloc_arg)(struct gpio_chip *gc,
unsigned int parent_hwirq,
unsigned int parent_type);
int (*populate_parent_alloc_arg)(struct gpio_chip *gc,
union gpio_irq_fwspec *fwspec,
unsigned int parent_hwirq,
unsigned int parent_type);

/**
* @child_offset_to_irq:
Expand Down Expand Up @@ -646,28 +656,14 @@ struct bgpio_pdata {

#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY

void *gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *gc,
int gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *gc,
union gpio_irq_fwspec *gfwspec,
unsigned int parent_hwirq,
unsigned int parent_type);
int gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *gc,
union gpio_irq_fwspec *gfwspec,
unsigned int parent_hwirq,
unsigned int parent_type);
void *gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *gc,
unsigned int parent_hwirq,
unsigned int parent_type);

#else

static inline void *gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *gc,
unsigned int parent_hwirq,
unsigned int parent_type)
{
return NULL;
}

static inline void *gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *gc,
unsigned int parent_hwirq,
unsigned int parent_type)
{
return NULL;
}

#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */

Expand Down

0 comments on commit 91a29af

Please sign in to comment.