[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241124174820.3d91dba9@jic23-huawei>
Date: Sun, 24 Nov 2024 17:48:20 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Dumitru Ceclan <mitrutzceclan@...il.com>, Michael Hennerich
<Michael.Hennerich@...log.com>, Nuno Sa <nuno.sa@...log.com>, Michael Walle
<michael@...le.cc>, Andy Shevchenko <andy@...nel.org>,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, Guillaume Ranquet
<granquet@...libre.com>, Uwe Kleine-König
<u.kleine-koenig@...libre.com>
Subject: Re: [PATCH 2/2] iio: adc: ad7173: make struct ad_sigma_delta_info
ad7173_sigma_delta_info const
On Fri, 22 Nov 2024 11:39:53 -0600
David Lechner <dlechner@...libre.com> wrote:
> Make struct ad_sigma_delta_info ad7173_sigma_delta_info static const.
> This structure is shared by all instances of the driver, so it can't be
> safely modified by one instance without affecting all other instances.
>
> The num_slots field was being modified, so we need to make two copies of
> the structure, one for each possible value of num_slots. Then we add a
> field to the chip-specific info struct to point to the correct copy of
> the struct ad_sigma_delta_info depending on the chip's capabilities.
>
> In order to do this, all of the chip-specific info structs have to be
> moved after the struct ad_sigma_delta_info definitions.
>
> Fixes: 76a1e6a42802 ("iio: adc: ad7173: add AD7173 driver")
> Signed-off-by: David Lechner <dlechner@...libre.com>
Too big to be suitable for backporting.
How about just duplicating the structure before modifying?
Then continue as before though it will need to passed into a few places
where the global variable is used directly.
Might be sensible to first do a minimal fix with duplication then
follow up with a refactor so it is picking between static const
structures as here.
Jonathan
> ---
> drivers/iio/adc/ad7173.c | 469 +++++++++++++++++++++++++----------------------
> 1 file changed, 249 insertions(+), 220 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index 5215584438bf..ab2a7a16c477 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -166,6 +166,7 @@ struct ad7173_device_info {
> unsigned int clock;
> unsigned int id;
> char *name;
> + const struct ad_sigma_delta_info *sd_info;
> bool has_current_inputs;
> bool has_vincom_input;
> bool has_temp;
> @@ -257,223 +258,6 @@ static unsigned int ad4111_current_channel_config[] = {
> 0x18B, /* 12:IIN3+ 11:IIN3− */
> };
>
> -static const struct ad7173_device_info ad4111_device_info = {
> - .name = "ad4111",
> - .id = AD4111_ID,
> - .num_voltage_in_div = 8,
> - .num_channels = 16,
> - .num_configs = 8,
> - .num_voltage_in = 8,
> - .num_gpios = 2,
> - .higher_gpio_bits = true,
> - .has_temp = true,
> - .has_vincom_input = true,
> - .has_input_buf = true,
> - .has_current_inputs = true,
> - .has_int_ref = true,
> - .clock = 2 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad7173_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad4112_device_info = {
> - .name = "ad4112",
> - .id = AD4112_ID,
> - .num_voltage_in_div = 8,
> - .num_channels = 16,
> - .num_configs = 8,
> - .num_voltage_in = 8,
> - .num_gpios = 2,
> - .higher_gpio_bits = true,
> - .has_vincom_input = true,
> - .has_temp = true,
> - .has_input_buf = true,
> - .has_current_inputs = true,
> - .has_int_ref = true,
> - .clock = 2 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad7173_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad4113_device_info = {
> - .name = "ad4113",
> - .id = AD4113_ID,
> - .num_voltage_in_div = 8,
> - .num_channels = 16,
> - .num_configs = 8,
> - .num_voltage_in = 8,
> - .num_gpios = 2,
> - .data_reg_only_16bit = true,
> - .higher_gpio_bits = true,
> - .has_vincom_input = true,
> - .has_input_buf = true,
> - .has_int_ref = true,
> - .clock = 2 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad7173_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad4114_device_info = {
> - .name = "ad4114",
> - .id = AD4114_ID,
> - .num_voltage_in_div = 16,
> - .num_channels = 16,
> - .num_configs = 8,
> - .num_voltage_in = 16,
> - .num_gpios = 4,
> - .has_vincom_input = true,
> - .has_temp = true,
> - .has_input_buf = true,
> - .has_int_ref = true,
> - .clock = 2 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad7173_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad4115_device_info = {
> - .name = "ad4115",
> - .id = AD4115_ID,
> - .num_voltage_in_div = 16,
> - .num_channels = 16,
> - .num_configs = 8,
> - .num_voltage_in = 16,
> - .num_gpios = 4,
> - .has_vincom_input = true,
> - .has_temp = true,
> - .has_input_buf = true,
> - .has_int_ref = true,
> - .clock = 8 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad4115_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad4115_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad4116_device_info = {
> - .name = "ad4116",
> - .id = AD4116_ID,
> - .num_voltage_in_div = 11,
> - .num_channels = 16,
> - .num_configs = 8,
> - .num_voltage_in = 16,
> - .num_gpios = 4,
> - .has_vincom_input = true,
> - .has_temp = true,
> - .has_input_buf = true,
> - .has_int_ref = true,
> - .clock = 4 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad4116_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad4116_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7172_2_device_info = {
> - .name = "ad7172-2",
> - .id = AD7172_2_ID,
> - .num_voltage_in = 5,
> - .num_channels = 4,
> - .num_configs = 4,
> - .num_gpios = 2,
> - .has_temp = true,
> - .has_input_buf = true,
> - .has_int_ref = true,
> - .has_pow_supply_monitoring = true,
> - .clock = 2 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad7173_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7172_4_device_info = {
> - .name = "ad7172-4",
> - .id = AD7172_4_ID,
> - .num_voltage_in = 9,
> - .num_channels = 8,
> - .num_configs = 8,
> - .num_gpios = 4,
> - .has_input_buf = true,
> - .has_ref2 = true,
> - .has_pow_supply_monitoring = true,
> - .clock = 2 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad7173_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7173_8_device_info = {
> - .name = "ad7173-8",
> - .id = AD7173_ID,
> - .num_voltage_in = 17,
> - .num_channels = 16,
> - .num_configs = 8,
> - .num_gpios = 4,
> - .has_temp = true,
> - .has_input_buf = true,
> - .has_int_ref = true,
> - .has_ref2 = true,
> - .clock = 2 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad7173_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7175_2_device_info = {
> - .name = "ad7175-2",
> - .id = AD7175_2_ID,
> - .num_voltage_in = 5,
> - .num_channels = 4,
> - .num_configs = 4,
> - .num_gpios = 2,
> - .has_temp = true,
> - .has_input_buf = true,
> - .has_int_ref = true,
> - .has_pow_supply_monitoring = true,
> - .clock = 16 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad7175_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7175_8_device_info = {
> - .name = "ad7175-8",
> - .id = AD7175_8_ID,
> - .num_voltage_in = 17,
> - .num_channels = 16,
> - .num_configs = 8,
> - .num_gpios = 4,
> - .has_temp = true,
> - .has_input_buf = true,
> - .has_int_ref = true,
> - .has_ref2 = true,
> - .has_pow_supply_monitoring = true,
> - .clock = 16 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad7175_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7176_2_device_info = {
> - .name = "ad7176-2",
> - .id = AD7176_ID,
> - .num_voltage_in = 5,
> - .num_channels = 4,
> - .num_configs = 4,
> - .num_gpios = 2,
> - .has_int_ref = true,
> - .clock = 16 * HZ_PER_MHZ,
> - .sinc5_data_rates = ad7175_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> -};
> -
> -static const struct ad7173_device_info ad7177_2_device_info = {
> - .name = "ad7177-2",
> - .id = AD7177_ID,
> - .num_voltage_in = 5,
> - .num_channels = 4,
> - .num_configs = 4,
> - .num_gpios = 2,
> - .has_temp = true,
> - .has_input_buf = true,
> - .has_int_ref = true,
> - .has_pow_supply_monitoring = true,
> - .clock = 16 * HZ_PER_MHZ,
> - .odr_start_value = AD7177_ODR_START_VALUE,
> - .sinc5_data_rates = ad7175_sinc5_data_rates,
> - .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> -};
> -
> static const char *const ad7173_ref_sel_str[] = {
> [AD7173_SETUP_REF_SEL_EXT_REF] = "vref",
> [AD7173_SETUP_REF_SEL_EXT_REF2] = "vref2",
> @@ -752,7 +536,7 @@ static int ad7173_disable_one(struct ad_sigma_delta *sd, unsigned int chan)
> return ad_sd_write_reg(sd, AD7173_REG_CH(chan), 2, 0);
> }
>
> -static struct ad_sigma_delta_info ad7173_sigma_delta_info = {
> +static const struct ad_sigma_delta_info ad7173_sigma_delta_info_4_slots = {
> .set_channel = ad7173_set_channel,
> .append_status = ad7173_append_status,
> .disable_all = ad7173_disable_all,
> @@ -764,6 +548,252 @@ static struct ad_sigma_delta_info ad7173_sigma_delta_info = {
> .read_mask = BIT(6),
> .status_ch_mask = GENMASK(3, 0),
> .data_reg = AD7173_REG_DATA,
> + .num_slots = 4,
> +};
> +
> +static const struct ad_sigma_delta_info ad7173_sigma_delta_info_8_slots = {
> + .set_channel = ad7173_set_channel,
> + .append_status = ad7173_append_status,
> + .disable_all = ad7173_disable_all,
> + .disable_one = ad7173_disable_one,
> + .set_mode = ad7173_set_mode,
> + .get_irq_by_name = true,
> + .has_registers = true,
> + .addr_shift = 0,
> + .read_mask = BIT(6),
> + .status_ch_mask = GENMASK(3, 0),
> + .data_reg = AD7173_REG_DATA,
> + .num_slots = 8,
> +};
> +
> +static const struct ad7173_device_info ad4111_device_info = {
> + .name = "ad4111",
> + .id = AD4111_ID,
> + .sd_info = &ad7173_sigma_delta_info_8_slots,
> + .num_voltage_in_div = 8,
> + .num_channels = 16,
> + .num_configs = 8,
> + .num_voltage_in = 8,
> + .num_gpios = 2,
> + .higher_gpio_bits = true,
> + .has_temp = true,
> + .has_vincom_input = true,
> + .has_input_buf = true,
> + .has_current_inputs = true,
> + .has_int_ref = true,
> + .clock = 2 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad7173_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad4112_device_info = {
> + .name = "ad4112",
> + .id = AD4112_ID,
> + .sd_info = &ad7173_sigma_delta_info_8_slots,
> + .num_voltage_in_div = 8,
> + .num_channels = 16,
> + .num_configs = 8,
> + .num_voltage_in = 8,
> + .num_gpios = 2,
> + .higher_gpio_bits = true,
> + .has_vincom_input = true,
> + .has_temp = true,
> + .has_input_buf = true,
> + .has_current_inputs = true,
> + .has_int_ref = true,
> + .clock = 2 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad7173_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad4113_device_info = {
> + .name = "ad4113",
> + .id = AD4113_ID,
> + .sd_info = &ad7173_sigma_delta_info_8_slots,
> + .num_voltage_in_div = 8,
> + .num_channels = 16,
> + .num_configs = 8,
> + .num_voltage_in = 8,
> + .num_gpios = 2,
> + .data_reg_only_16bit = true,
> + .higher_gpio_bits = true,
> + .has_vincom_input = true,
> + .has_input_buf = true,
> + .has_int_ref = true,
> + .clock = 2 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad7173_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad4114_device_info = {
> + .name = "ad4114",
> + .id = AD4114_ID,
> + .sd_info = &ad7173_sigma_delta_info_8_slots,
> + .num_voltage_in_div = 16,
> + .num_channels = 16,
> + .num_configs = 8,
> + .num_voltage_in = 16,
> + .num_gpios = 4,
> + .has_vincom_input = true,
> + .has_temp = true,
> + .has_input_buf = true,
> + .has_int_ref = true,
> + .clock = 2 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad7173_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad4115_device_info = {
> + .name = "ad4115",
> + .id = AD4115_ID,
> + .sd_info = &ad7173_sigma_delta_info_8_slots,
> + .num_voltage_in_div = 16,
> + .num_channels = 16,
> + .num_configs = 8,
> + .num_voltage_in = 16,
> + .num_gpios = 4,
> + .has_vincom_input = true,
> + .has_temp = true,
> + .has_input_buf = true,
> + .has_int_ref = true,
> + .clock = 8 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad4115_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad4115_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad4116_device_info = {
> + .name = "ad4116",
> + .id = AD4116_ID,
> + .sd_info = &ad7173_sigma_delta_info_8_slots,
> + .num_voltage_in_div = 11,
> + .num_channels = 16,
> + .num_configs = 8,
> + .num_voltage_in = 16,
> + .num_gpios = 4,
> + .has_vincom_input = true,
> + .has_temp = true,
> + .has_input_buf = true,
> + .has_int_ref = true,
> + .clock = 4 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad4116_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad4116_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7172_2_device_info = {
> + .name = "ad7172-2",
> + .id = AD7172_2_ID,
> + .sd_info = &ad7173_sigma_delta_info_8_slots,
> + .num_voltage_in = 5,
> + .num_channels = 4,
> + .num_configs = 4,
> + .num_gpios = 2,
> + .has_temp = true,
> + .has_input_buf = true,
> + .has_int_ref = true,
> + .has_pow_supply_monitoring = true,
> + .clock = 2 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad7173_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7172_4_device_info = {
> + .name = "ad7172-4",
> + .id = AD7172_4_ID,
> + .sd_info = &ad7173_sigma_delta_info_8_slots,
> + .num_voltage_in = 9,
> + .num_channels = 8,
> + .num_configs = 8,
> + .num_gpios = 4,
> + .has_input_buf = true,
> + .has_ref2 = true,
> + .has_pow_supply_monitoring = true,
> + .clock = 2 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad7173_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7173_8_device_info = {
> + .name = "ad7173-8",
> + .id = AD7173_ID,
> + .sd_info = &ad7173_sigma_delta_info_8_slots,
> + .num_voltage_in = 17,
> + .num_channels = 16,
> + .num_configs = 8,
> + .num_gpios = 4,
> + .has_temp = true,
> + .has_input_buf = true,
> + .has_int_ref = true,
> + .has_ref2 = true,
> + .clock = 2 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad7173_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7175_2_device_info = {
> + .name = "ad7175-2",
> + .id = AD7175_2_ID,
> + .sd_info = &ad7173_sigma_delta_info_8_slots,
> + .num_voltage_in = 5,
> + .num_channels = 4,
> + .num_configs = 4,
> + .num_gpios = 2,
> + .has_temp = true,
> + .has_input_buf = true,
> + .has_int_ref = true,
> + .has_pow_supply_monitoring = true,
> + .clock = 16 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad7175_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7175_8_device_info = {
> + .name = "ad7175-8",
> + .id = AD7175_8_ID,
> + .sd_info = &ad7173_sigma_delta_info_8_slots,
> + .num_voltage_in = 17,
> + .num_channels = 16,
> + .num_configs = 8,
> + .num_gpios = 4,
> + .has_temp = true,
> + .has_input_buf = true,
> + .has_int_ref = true,
> + .has_ref2 = true,
> + .has_pow_supply_monitoring = true,
> + .clock = 16 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad7175_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7176_2_device_info = {
> + .name = "ad7176-2",
> + .id = AD7176_ID,
> + .sd_info = &ad7173_sigma_delta_info_4_slots,
> + .num_voltage_in = 5,
> + .num_channels = 4,
> + .num_configs = 4,
> + .num_gpios = 2,
> + .has_int_ref = true,
> + .clock = 16 * HZ_PER_MHZ,
> + .sinc5_data_rates = ad7175_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> +};
> +
> +static const struct ad7173_device_info ad7177_2_device_info = {
> + .name = "ad7177-2",
> + .id = AD7177_ID,
> + .sd_info = &ad7173_sigma_delta_info_4_slots,
> + .num_voltage_in = 5,
> + .num_channels = 4,
> + .num_configs = 4,
> + .num_gpios = 2,
> + .has_temp = true,
> + .has_input_buf = true,
> + .has_int_ref = true,
> + .has_pow_supply_monitoring = true,
> + .clock = 16 * HZ_PER_MHZ,
> + .odr_start_value = AD7177_ODR_START_VALUE,
> + .sinc5_data_rates = ad7175_sinc5_data_rates,
> + .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
> };
>
> static int ad7173_setup(struct iio_dev *indio_dev)
> @@ -1429,8 +1459,7 @@ static int ad7173_probe(struct spi_device *spi)
> spi->mode = SPI_MODE_3;
> spi_setup(spi);
>
> - ad7173_sigma_delta_info.num_slots = st->info->num_configs;
> - ret = ad_sd_init(&st->sd, indio_dev, spi, &ad7173_sigma_delta_info);
> + ret = ad_sd_init(&st->sd, indio_dev, spi, st->info->sd_info);
> if (ret)
> return ret;
>
>
Powered by blists - more mailing lists