wt., 12 lut 2019 o 12:14 Lee Jones [off-list ref] napisał(a):

On Tue, 12 Feb 2019, Bartosz Golaszewski wrote:

quoted

wt., 12 lut 2019 o 11:18 Lee Jones [off-list ref] napisał(a):

quoted

On Tue, 12 Feb 2019, Bartosz Golaszewski wrote:

quoted

wt., 12 lut 2019 o 10:55 Lee Jones [off-list ref] napisał(a):

quoted

 * The declaration of a superfluous struct
 * 100 lines of additional/avoidable code
 * Hacky hoop jumping trying to fudge VIRQs into resources
 * Resources were designed for HWIRQs (unless a domain is present)
 * Loads of additional/avoidable CPU cycles setting all this up

While the above may be right, this one is negligible and you know it. :)

You have nested for() loops.  You *are* wasting lots of cycles.

quoted

quoted

Need I go on? :)

Surely the fact that you are using both sides of an API
(devm_regmap_init_i2c and regmap_irq_get_*) in the same driver, must
set some alarm bells ringing?

This whole HWIRQ setting, VIRQ getting, resource hacking is a mess.

And for what?  To avoid passing IRQ data to a child driver?

What do you propose? Should I go back to the approach in v1 and pass
the regmap_irq_chip_data to child drivers?

I'm saying you should remove all of this hackery and pass IRQs as they
are supposed to be passed (like everyone else does).

I'm not sure what you mean by "like everyone else does" - different
mfd drivers seem to be doing different things. Is a simple struct
containing virtual irq numbers passed to sub-drivers fine?

How do you plan on deriving the VIRQs to place into the struct?

Exampe:

struct max77650_gpio_pdata {
    int gpi_irq;
};

In MFD driver:

struct max77650_gpio_pdata *gpio_data = devm_kmalloc(dev, sizeof(*gpio_data));

gpio_data->gpi_irq = regmap_irq_get_virq(irqchip_data, GPI_NUM);

gpio_cell.platform_data = gpio_data;

In GPIO driver:

struct max77650_gpio_pdata *gpio_data = pdev->dev.platform_data;

int irq = gpio_data->gpi_irq;

Bart