| Message ID | 20250110-iio-adc-ad7313-fix-non-const-info-struct-v3-1-41e1c9cdd1a7@baylibre.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iio: adc: ad7173: fix non-const info struct | expand |
On Fri, 10 Jan 2025 11:40:06 -0600 David Lechner <dlechner@baylibre.com> wrote: > Remove the int irq_line field in struct ad_sigma_delta_info and all code > that referenced it. > > This struct is intended to be used as static const data. Currently, the > only user that doesn't uses the static const struct directly, namely the > ad7173 driver is making a copy of this struct to be able to modify the > irq_line field. However, this field is written and never used due to the > fact that ad_sd_init() which reads the field is called before > ad7173_fw_parse_device_config() which writes it. > > The runtime behavior does not change since ad_sd_init() was already > (unintentionally) being called with irq_line = 0. But, even though > this could be considered a bug, the behavior was still correct. The SPI > subsystem always uses the first interrupt in the interrupts array from > the devicetree and the devicetree bindings for this family of chips > specify that the RDY interrupt is always the first interrupt. Binding does say that kind of, but it shouldn't - we should allow for possibility of only the err being connected in the binding. The driver can of course reject that. interrupts: minItems: 1 items: - description: | Ready: multiplexed with SPI data out. While SPI CS is low, can be used to indicate the completion of a conversion. - description: | Error: The three error bits in the status register (ADC_ERROR, CRC_ERROR, and REG_ERROR) are OR'ed, inverted, and mapped to the ERROR pin. Therefore, the ERROR pin indicates that an error has occurred. interrupt-names: minItems: 1 items: - const: rdy - const: err Is the current binding that should be relaxed. Upshot, I'd specifically check rdy is the first one. Easy way being to see if it matches spi->irq. Jonathan > Therefore, > we don't actually need the special call to fwnode_irq_get_byname(), so > it is removed in this patch instead of moving it to the correct place. > > Tested-by: Guillaume Ranquet <granquet@baylibre.com> > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > > v3 changes: > * Removed spurious change that was causing compiler error. > * Rebased on iio/testing and resolved some merge conflicts. > > v2 changes: > * Fixed chip name is subject line > * Uwe's comment made me realize that the special case was actually never > being used because of the ordering bug and could safely be removed > rather than trying to preserve it. > --- > drivers/iio/adc/ad7173.c | 6 ------ > drivers/iio/adc/ad_sigma_delta.c | 5 +---- > include/linux/iio/adc/ad_sigma_delta.h | 2 -- > 3 files changed, 1 insertion(+), 12 deletions(-) > > diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c > index 6c4ed10ae580d66287857252ce9a69cfaa45db0b..b92aca39d117a315d6b55951fba7c3b51787555a 100644 > --- a/drivers/iio/adc/ad7173.c > +++ b/drivers/iio/adc/ad7173.c > @@ -1515,12 +1515,6 @@ static int ad7173_fw_parse_device_config(struct iio_dev *indio_dev) > return ret; > } > > - ret = fwnode_irq_get_byname(dev_fwnode(dev), "rdy"); > - if (ret < 0) > - return dev_err_probe(dev, ret, "Interrupt 'rdy' is required\n"); > - > - st->sigma_delta_info.irq_line = ret; > - > return ad7173_fw_parse_channel_config(indio_dev); > } > > diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c > index d5d81581ab34099cef30ec63944ce1171c80ec14..38a72ced10326656b30fd39d7a72cefe8c4c1aa5 100644 > --- a/drivers/iio/adc/ad_sigma_delta.c > +++ b/drivers/iio/adc/ad_sigma_delta.c > @@ -801,10 +801,7 @@ int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev, > > spin_lock_init(&sigma_delta->irq_lock); > > - if (info->irq_line) > - sigma_delta->irq_line = info->irq_line; > - else > - sigma_delta->irq_line = spi->irq; > + sigma_delta->irq_line = spi->irq; > > sigma_delta->rdy_gpiod = devm_gpiod_get_optional(&spi->dev, "rdy", GPIOD_IN); > if (IS_ERR(sigma_delta->rdy_gpiod)) > diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h > index 417073c52380f60a1a45a4924f4f556b64832295..521e3dc95db9117b7df12710eaae3f373d1df7bc 100644 > --- a/include/linux/iio/adc/ad_sigma_delta.h > +++ b/include/linux/iio/adc/ad_sigma_delta.h > @@ -53,7 +53,6 @@ struct iio_dev; > * be used. > * @irq_flags: flags for the interrupt used by the triggered buffer > * @num_slots: Number of sequencer slots > - * @irq_line: IRQ for reading conversions. If 0, spi->irq will be used > * @num_resetclks: Number of SPI clk cycles with MOSI=1 to reset the chip. > */ > struct ad_sigma_delta_info { > @@ -70,7 +69,6 @@ struct ad_sigma_delta_info { > unsigned int data_reg; > unsigned long irq_flags; > unsigned int num_slots; > - int irq_line; > unsigned int num_resetclks; > }; > >
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c index 6c4ed10ae580d66287857252ce9a69cfaa45db0b..b92aca39d117a315d6b55951fba7c3b51787555a 100644 --- a/drivers/iio/adc/ad7173.c +++ b/drivers/iio/adc/ad7173.c @@ -1515,12 +1515,6 @@ static int ad7173_fw_parse_device_config(struct iio_dev *indio_dev) return ret; } - ret = fwnode_irq_get_byname(dev_fwnode(dev), "rdy"); - if (ret < 0) - return dev_err_probe(dev, ret, "Interrupt 'rdy' is required\n"); - - st->sigma_delta_info.irq_line = ret; - return ad7173_fw_parse_channel_config(indio_dev); } diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c index d5d81581ab34099cef30ec63944ce1171c80ec14..38a72ced10326656b30fd39d7a72cefe8c4c1aa5 100644 --- a/drivers/iio/adc/ad_sigma_delta.c +++ b/drivers/iio/adc/ad_sigma_delta.c @@ -801,10 +801,7 @@ int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev, spin_lock_init(&sigma_delta->irq_lock); - if (info->irq_line) - sigma_delta->irq_line = info->irq_line; - else - sigma_delta->irq_line = spi->irq; + sigma_delta->irq_line = spi->irq; sigma_delta->rdy_gpiod = devm_gpiod_get_optional(&spi->dev, "rdy", GPIOD_IN); if (IS_ERR(sigma_delta->rdy_gpiod)) diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h index 417073c52380f60a1a45a4924f4f556b64832295..521e3dc95db9117b7df12710eaae3f373d1df7bc 100644 --- a/include/linux/iio/adc/ad_sigma_delta.h +++ b/include/linux/iio/adc/ad_sigma_delta.h @@ -53,7 +53,6 @@ struct iio_dev; * be used. * @irq_flags: flags for the interrupt used by the triggered buffer * @num_slots: Number of sequencer slots - * @irq_line: IRQ for reading conversions. If 0, spi->irq will be used * @num_resetclks: Number of SPI clk cycles with MOSI=1 to reset the chip. */ struct ad_sigma_delta_info { @@ -70,7 +69,6 @@ struct ad_sigma_delta_info { unsigned int data_reg; unsigned long irq_flags; unsigned int num_slots; - int irq_line; unsigned int num_resetclks; };