Re: [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1

On Mon, Jul 19, 2021 at 8:29 PM Stephan Gerhold [off-list ref] wrote:

On Mon, Jul 19, 2021 at 07:19:05PM +0300, Andy Shevchenko wrote:

quoted

On Mon, Jul 19, 2021 at 6:11 PM Stephan Gerhold [off-list ref] wrote:

quoted

On Mon, Jul 19, 2021 at 06:01:01PM +0300, Andy Shevchenko wrote:

quoted

On Mon, Jul 19, 2021 at 5:07 PM Linus Walleij [off-list ref] wrote:

quoted

On Mon, Jul 19, 2021 at 1:26 PM Stephan Gerhold [off-list ref] wrote:

...

quoted

quoted

quoted

quoted

quoted

 #include <linux/acpi.h>
+#include <linux/of_irq.h>

(...)

quoted

+       irq_info = bmc150_accel_interrupts_int1;
+       if (irq == of_irq_get_byname(dev->of_node, "INT2"))
+               irq_info = bmc150_accel_interrupts_int2;

This looks a bit DT-specific, but I don't see that ACPI has
named IRQs so I don't know what to do about it either.

Yeah, we only have so far the (de facto) established way of naming
GPIO based IRQs, and not IOxAPIC ones.

quoted

What does platform_get_irq_byname() do on ACPI systems?

See above.

quoted

If there is no obvious fix I would leave it like this until the
first ACPI used needing this comes along, but I think maybe
Andy has suggestions.

The platform_get_irq_byname() should do something similar that has
been done in platform_get_irq() WRT ACPI.
Here for sure the platform_get_irq_byname() or its optional variant
should be used.

I don't think there is a platform device here, we only have the
i2c_client or spi_device. That's why I didn't use
platform_get_irq_byname(). :)

Is there something equivalent for I2C/SPI drivers?

Not yet. You probably need to supply some code there to allow
multi-IRQ devices (in resource provider agnostic way).

You need to provide fwnode_get_irq_byname() to be similar with
https://elixir.bootlin.com/linux/latest/source/drivers/base/property.c#L1010

Then use it in the drivers.

And/or integrate into frameworks somehow (something in between the
lines: https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L461).

Well, I don't think anyone has an ACPI use case for this right now so
it's probably better if this is done by someone who actually needs this
and can test it somewhere. :)

I actually just "copied" this approach from some other IIO drivers where
this is done similarly (and additionally checked the source code to make
sure this won't break anything for ACPI platforms).

I see in today's Linux Next snapshot:

drivers/iio/accel/fxls8962af-core.c:774:        irq =
of_irq_get_byname(of_node, "INT2");
drivers/iio/accel/mma8452.c:1616:               irq2 =
of_irq_get_byname(client->dev.of_node, "INT2");
drivers/iio/gyro/fxas21002c_core.c:834: irq1 = of_irq_get_byname(np, "INT1");
drivers/iio/imu/adis16480.c:1265:               irq =
of_irq_get_byname(of_node, adis16480_int_pin_names[i]);
drivers/iio/imu/bmi160/bmi160_core.c:655:       irq =
of_irq_get_byname(of_node, "INT1");
drivers/iio/imu/bmi160/bmi160_core.c:661:       irq =
of_irq_get_byname(of_node, "INT2");

I believe we may stop distributing this and actually start using a
common API. I don't want this to be spread again over all IIO. Btw, I
have LSM9DS0, which supports two INT pins for IMU and currently it
uses hard coded pin mapping.

Side note to Jonathan, I believe the below may be (lazily) converted
to fwnode / device properties APIs.

drivers/iio/adc/ab8500-gpadc.c:1041:    nchans =
of_get_available_child_count(np);
drivers/iio/adc/ad7124.c:747:   st->num_channels =
of_get_available_child_count(np);
drivers/iio/adc/berlin2-adc.c:287:      struct device_node *parent_np
= of_get_parent(pdev->dev.of_node);
drivers/iio/adc/qcom-pm8xxx-xoadc.c:830:        adc->nchans =
of_get_available_child_count(np);
drivers/iio/adc/qcom-spmi-adc5.c:650:   channel_name = of_get_property(node,
drivers/iio/adc/qcom-spmi-adc5.c:813:   adc->nchannels =
of_get_available_child_count(node);
drivers/iio/adc/qcom-spmi-vadc.c:743:   vadc->nchannels =
of_get_available_child_count(node);
drivers/iio/adc/stm32-adc.c:1630:static int
stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
drivers/iio/adc/stm32-adc.c:1935:       ret =
stm32_adc_of_get_resolution(indio_dev);
drivers/iio/adc/xilinx-xadc-core.c:1248:        chan_node =
of_get_child_by_name(np, "xlnx,channels");
drivers/iio/imu/adis16480.c:1293:static int
adis16480_of_get_ext_clk_pin(struct adis16480 *st,
drivers/iio/imu/adis16480.c:1328:       pin =
adis16480_of_get_ext_clk_pin(st, of_node);
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c:68:           mux_node =
of_get_child_by_name(dev->of_node, "i2c-gate")
;
drivers/iio/industrialio-core.c:1877:   label =
of_get_property(indio_dev->dev.of_node, "label", NULL);
drivers/iio/inkern.c:228:               if (np && !of_get_property(np,
"io-channel-ranges", NULL))
drivers/iio/temperature/ltc2983.c:1275: st->num_channels =
of_get_available_child_count(dev->of_node);


-- 
With Best Regards,
Andy Shevchenko