diff mbox series

[v3,11/24] iio: accel: kxcjk-1013: Start using chip_info variables instead of enum

Message ID 20241024191200.229894-12-andriy.shevchenko@linux.intel.com (mailing list archive)
State Accepted
Headers show
Series iio: Clean up acpi_match_device() use cases | expand

Commit Message

Andy Shevchenko Oct. 24, 2024, 7:05 p.m. UTC 
Instead of having a enum and keeping IDs as driver data pointers,
just have a chip_info struct per supported device.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 171 +++++++++++++++++++--------------
 1 file changed, 99 insertions(+), 72 deletions(-)

Comments

Jonathan Cameron Oct. 26, 2024, 11:26 a.m. UTC | #1 
On Thu, 24 Oct 2024 22:05:00 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Instead of having a enum and keeping IDs as driver data pointers,
> just have a chip_info struct per supported device.
I'm not keen longer term on acpi_type, as the various bits dependent on that
should probably be done via optional callbacks in the kx_chipset_info
structure, but this is a sensible intermediate step.

I see the chipset one goes away later hence no comment on that.

I did a bit of white space massaging whilst applying this.
Hopefully that won't make me mess up applying the following patches.

>  enum kxcjk1013_mode {
> @@ -425,27 +472,28 @@ static int kiox010a_dsm(struct device *dev, int fn_index)
>  }
>  
>  static const struct acpi_device_id kx_acpi_match[] = {
> -	{"KXCJ1013", KXCJK1013},
> -	{"KXCJ1008", KXCJ91008},
> -	{"KXCJ9000", KXCJ91008},
> -	{"KIOX0008", KXCJ91008},
> -	{"KIOX0009", KXTJ21009},
> -	{"KIOX000A", KXCJ91008},
> -	{"KIOX010A", KXCJ91008}, /* KXCJ91008 in the display of a yoga 2-in-1 */
> -	{"KIOX020A", KXCJ91008}, /* KXCJ91008 in the base of a yoga 2-in-1 */
> -	{"KXTJ1009", KXTJ21009},
> -	{"KXJ2109",  KXTJ21009},
> -	{"SMO8500",  KXCJ91008},
> +	{"KIOX0008", (kernel_ulong_t)&kxcj91008_info },
> +	{"KIOX0009", (kernel_ulong_t)&kxtj21009_info },
> +	{"KIOX000A", (kernel_ulong_t)&kxcj91008_info },
> +	/* KXCJ91008 in the display of a yoga 2-in-1 */
> +	{"KIOX010A", (kernel_ulong_t)&kxcj91008_kiox010a_info },
> +	/* KXCJ91008 in the base of a yoga 2-in-1 */
> +	{"KIOX020A", (kernel_ulong_t)&kxcj91008_kiox020a_info },
> +	{"KXCJ1008", (kernel_ulong_t)&kxcj91008_info },
> +	{"KXCJ1013", (kernel_ulong_t)&kxcjk1013_info },
> +	{"KXCJ9000", (kernel_ulong_t)&kxcj91008_info },
> +	{"KXJ2109",  (kernel_ulong_t)&kxtj21009_info },
> +	{"KXTJ1009", (kernel_ulong_t)&kxtj21009_info },
> +	{"SMO8500",  (kernel_ulong_t)&kxcj91008_smo8500_info },
I'll tweak the spacing on this as well whilst here.
You did one end effectively, might as well do the other.

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(acpi, kx_acpi_match);

> @@ -1711,22 +1739,21 @@ static const struct dev_pm_ops kxcjk1013_pm_ops = {
>  };
>  
>  static const struct i2c_device_id kxcjk1013_id[] = {
> -	{"kxcjk1013", KXCJK1013},
> -	{"kxcj91008", KXCJ91008},
> -	{"kxtj21009", KXTJ21009},
> -	{"kxtf9",     KXTF9},
> -	{"kx023-1025", KX0231025},
> +	{"kxcjk1013",  (kernel_ulong_t)&kxcjk1013_info },
> +	{"kxcj91008",  (kernel_ulong_t)&kxcj91008_info },
> +	{"kxtj21009",  (kernel_ulong_t)&kxtj21009_info },
> +	{"kxtf9", (kernel_ulong_t)&kxtf9_info },
> +	{"kx023-1025", (kernel_ulong_t)&kx0231025_info },
>  	{}
I'm going to tweak the spacing of that whilst we are touching it.
Andy Shevchenko Oct. 28, 2024, 9:48 a.m. UTC | #2 
On Sat, Oct 26, 2024 at 12:26:13PM +0100, Jonathan Cameron wrote:
> On Thu, 24 Oct 2024 22:05:00 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Instead of having a enum and keeping IDs as driver data pointers,
> > just have a chip_info struct per supported device.
> I'm not keen longer term on acpi_type, as the various bits dependent on that
> should probably be done via optional callbacks in the kx_chipset_info
> structure, but this is a sensible intermediate step.

Yeah, I decided to postpone that because the main point of this series is to
clean up acpi_match_device(). And indeed I was thinking about callbacks and
other things (flags, string literals, etc) to be moved to chip_info.

> I see the chipset one goes away later hence no comment on that.
> 
> I did a bit of white space massaging whilst applying this.
> Hopefully that won't make me mess up applying the following patches.

Thanks!

(OTOH I'm not sure what spacing you meant because in the result it still seems
 slightly inconsistent.)
diff mbox series

Patch

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index f97bdbbe71ed..37c82fdf7c43 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -234,6 +234,55 @@  static const struct kx_chipset_regs kx0231025_regs = {
 	.wake_thres	= KX023_REG_ATH,
 };
 
+struct kx_chipset_info {
+	const struct kx_chipset_regs *regs;
+	enum kx_chipset chipset;
+	enum kx_acpi_type acpi_type;
+};
+
+static const struct kx_chipset_info kxcjk1013_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXCJK1013,
+};
+
+static const struct kx_chipset_info kxcj91008_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXCJ91008,
+};
+
+static const struct kx_chipset_info kxcj91008_kiox010a_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXCJ91008,
+	.acpi_type = ACPI_KIOX010A,
+};
+
+static const struct kx_chipset_info kxcj91008_kiox020a_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXCJ91008,
+	.acpi_type = ACPI_GENERIC,
+};
+
+static const struct kx_chipset_info kxcj91008_smo8500_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXCJ91008,
+	.acpi_type = ACPI_SMO8500,
+};
+
+static const struct kx_chipset_info kxtj21009_info = {
+	.regs = &kxcjk1013_regs,
+	.chipset = KXTJ21009,
+};
+
+static const struct kx_chipset_info kxtf9_info = {
+	.regs = &kxtf9_regs,
+	.chipset = KXTF9,
+};
+
+static const struct kx_chipset_info kx0231025_info = {
+	.regs = &kx0231025_regs,
+	.chipset = KX0231025,
+};
+
 enum kxcjk1013_axis {
 	AXIS_X,
 	AXIS_Y,
@@ -261,9 +310,7 @@  struct kxcjk1013_data {
 	int ev_enable_state;
 	bool motion_trigger_on;
 	int64_t timestamp;
-	enum kx_chipset chipset;
-	enum kx_acpi_type acpi_type;
-	const struct kx_chipset_regs *regs;
+	const struct kx_chipset_info *info;
 };
 
 enum kxcjk1013_mode {
@@ -425,27 +472,28 @@  static int kiox010a_dsm(struct device *dev, int fn_index)
 }
 
 static const struct acpi_device_id kx_acpi_match[] = {
-	{"KXCJ1013", KXCJK1013},
-	{"KXCJ1008", KXCJ91008},
-	{"KXCJ9000", KXCJ91008},
-	{"KIOX0008", KXCJ91008},
-	{"KIOX0009", KXTJ21009},
-	{"KIOX000A", KXCJ91008},
-	{"KIOX010A", KXCJ91008}, /* KXCJ91008 in the display of a yoga 2-in-1 */
-	{"KIOX020A", KXCJ91008}, /* KXCJ91008 in the base of a yoga 2-in-1 */
-	{"KXTJ1009", KXTJ21009},
-	{"KXJ2109",  KXTJ21009},
-	{"SMO8500",  KXCJ91008},
+	{"KIOX0008", (kernel_ulong_t)&kxcj91008_info },
+	{"KIOX0009", (kernel_ulong_t)&kxtj21009_info },
+	{"KIOX000A", (kernel_ulong_t)&kxcj91008_info },
+	/* KXCJ91008 in the display of a yoga 2-in-1 */
+	{"KIOX010A", (kernel_ulong_t)&kxcj91008_kiox010a_info },
+	/* KXCJ91008 in the base of a yoga 2-in-1 */
+	{"KIOX020A", (kernel_ulong_t)&kxcj91008_kiox020a_info },
+	{"KXCJ1008", (kernel_ulong_t)&kxcj91008_info },
+	{"KXCJ1013", (kernel_ulong_t)&kxcjk1013_info },
+	{"KXCJ9000", (kernel_ulong_t)&kxcj91008_info },
+	{"KXJ2109",  (kernel_ulong_t)&kxtj21009_info },
+	{"KXTJ1009", (kernel_ulong_t)&kxtj21009_info },
+	{"SMO8500",  (kernel_ulong_t)&kxcj91008_smo8500_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, kx_acpi_match);
-
 #endif
 
 static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 			      enum kxcjk1013_mode mode)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
@@ -471,7 +519,7 @@  static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 static int kxcjk1013_get_mode(struct kxcjk1013_data *data,
 			      enum kxcjk1013_mode *mode)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
@@ -490,7 +538,7 @@  static int kxcjk1013_get_mode(struct kxcjk1013_data *data,
 
 static int kxcjk1013_set_range(struct kxcjk1013_data *data, int range_index)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(data->client, regs->ctrl1);
@@ -517,11 +565,11 @@  static int kxcjk1013_set_range(struct kxcjk1013_data *data, int range_index)
 
 static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 #ifdef CONFIG_ACPI
-	if (data->acpi_type == ACPI_KIOX010A) {
+	if (data->info->acpi_type == ACPI_KIOX010A) {
 		/* Make sure the kbd and touchpad on 2-in-1s using 2 KXCJ91008-s work */
 		kiox010a_dsm(&data->client->dev, KIOX010A_SET_LAPTOP_MODE);
 	}
@@ -586,7 +634,7 @@  static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	}
 
 	/* On KX023, route all used interrupts to INT1 for now */
-	if (data->chipset == KX0231025 && data->client->irq > 0) {
+	if (data->info->chipset == KX0231025 && data->client->irq > 0) {
 		ret = i2c_smbus_write_byte_data(data->client, KX023_REG_INC4,
 						KX023_REG_INC4_DRDY1 |
 						KX023_REG_INC4_WUFI1);
@@ -607,8 +655,8 @@  static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 
 static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
 {
+	int idx = data->info->chipset;
 	int i;
-	int idx = data->chipset;
 
 	for (i = 0; i < ARRAY_SIZE(odr_start_up_times[idx]); ++i) {
 		if (odr_start_up_times[idx][i].odr_bits == data->odr_bits)
@@ -641,7 +689,7 @@  static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
 
 static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_write_byte_data(data->client, regs->wake_timer, data->wake_dur);
@@ -663,7 +711,7 @@  static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
 static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 						bool status)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 	enum kxcjk1013_mode store_mode;
 
@@ -726,7 +774,7 @@  static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
 					      bool status)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 	enum kxcjk1013_mode store_mode;
 
@@ -814,7 +862,7 @@  static int kxcjk1013_convert_odr_value(const struct kx_odr_map *map,
 
 static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 {
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 	enum kxcjk1013_mode store_mode;
 	const struct kx_odr_map *odr_setting;
@@ -823,7 +871,7 @@  static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 	if (ret < 0)
 		return ret;
 
-	if (data->chipset == KXTF9)
+	if (data->info->chipset == KXTF9)
 		odr_setting = kxcjk1013_find_odr_value(kxtf9_samp_freq_table,
 						       ARRAY_SIZE(kxtf9_samp_freq_table),
 						       val, val2);
@@ -867,7 +915,7 @@  static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 
 static int kxcjk1013_get_odr(struct kxcjk1013_data *data, int *val, int *val2)
 {
-	if (data->chipset == KXTF9)
+	if (data->info->chipset == KXTF9)
 		return kxcjk1013_convert_odr_value(kxtf9_samp_freq_table,
 						   ARRAY_SIZE(kxtf9_samp_freq_table),
 						   data->odr_bits, val, val2);
@@ -1134,7 +1182,7 @@  static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
 	const char *str;
 
-	if (data->chipset == KXTF9)
+	if (data->info->chipset == KXTF9)
 		str = kxtf9_samp_freq_avail;
 	else
 		str = kxcjk1013_samp_freq_avail;
@@ -1251,7 +1299,7 @@  static void kxcjk1013_trig_reen(struct iio_trigger *trig)
 {
 	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(data->client, regs->int_rel);
@@ -1306,7 +1354,7 @@  static const struct iio_trigger_ops kxcjk1013_trigger_ops = {
 static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
 {
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 
 	int ret = i2c_smbus_read_byte_data(data->client, regs->int_src2);
 	if (ret < 0) {
@@ -1373,7 +1421,7 @@  static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 {
 	struct iio_dev *indio_dev = private;
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
-	const struct kx_chipset_regs *regs = data->regs;
+	const struct kx_chipset_regs *regs = data->info->regs;
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(data->client, regs->int_src1);
@@ -1383,7 +1431,7 @@  static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 	}
 
 	if (ret & KXCJK1013_REG_INT_SRC1_BIT_WUFS) {
-		if (data->chipset == KXTF9)
+		if (data->info->chipset == KXTF9)
 			iio_push_event(indio_dev,
 				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 				       0,
@@ -1425,8 +1473,7 @@  static irqreturn_t kxcjk1013_data_rdy_trig_poll(int irq, void *private)
 }
 
 static const char *kxcjk1013_match_acpi_device(struct device *dev,
-					       enum kx_chipset *chipset,
-					       enum kx_acpi_type *acpi_type,
+					       const struct kx_chipset_info **info,
 					       const char **label)
 {
 	const struct acpi_device_id *id;
@@ -1435,16 +1482,12 @@  static const char *kxcjk1013_match_acpi_device(struct device *dev,
 	if (!id)
 		return NULL;
 
-	if (strcmp(id->id, "SMO8500") == 0) {
-		*acpi_type = ACPI_SMO8500;
-	} else if (strcmp(id->id, "KIOX010A") == 0) {
-		*acpi_type = ACPI_KIOX010A;
+	if (strcmp(id->id, "KIOX010A") == 0)
 		*label = "accel-display";
-	} else if (strcmp(id->id, "KIOX020A") == 0) {
+	else if (strcmp(id->id, "KIOX020A") == 0)
 		*label = "accel-base";
-	}
 
-	*chipset = (enum kx_chipset)id->driver_data;
+	*info = (const struct kx_chipset_info *)id->driver_data;
 
 	return dev_name(dev);
 }
@@ -1496,31 +1539,16 @@  static int kxcjk1013_probe(struct i2c_client *client)
 	msleep(20);
 
 	if (id) {
-		data->chipset = (enum kx_chipset)(id->driver_data);
 		name = id->name;
+		data->info = (const struct kx_chipset_info *)(id->driver_data);
 	} else if (ACPI_HANDLE(&client->dev)) {
-		name = kxcjk1013_match_acpi_device(&client->dev,
-						   &data->chipset,
-						   &data->acpi_type,
+		name = kxcjk1013_match_acpi_device(&client->dev, &data->info,
 						   &indio_dev->label);
 	} else
 		return -ENODEV;
 
-	switch (data->chipset) {
-	case KXCJK1013:
-	case KXCJ91008:
-	case KXTJ21009:
-		data->regs = &kxcjk1013_regs;
-		break;
-	case KXTF9:
-		data->regs = &kxtf9_regs;
-		break;
-	case KX0231025:
-		data->regs = &kx0231025_regs;
-		break;
-	default:
+	if (!data->info)
 		return -EINVAL;
-	}
 
 	ret = kxcjk1013_chip_init(data);
 	if (ret < 0)
@@ -1535,7 +1563,7 @@  static int kxcjk1013_probe(struct i2c_client *client)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &kxcjk1013_iio_info;
 
-	if (client->irq > 0 && data->acpi_type != ACPI_SMO8500) {
+	if (client->irq > 0 && data->info->acpi_type != ACPI_SMO8500) {
 		ret = devm_request_threaded_irq(&client->dev, client->irq,
 						kxcjk1013_data_rdy_trig_poll,
 						kxcjk1013_event_handler,
@@ -1711,22 +1739,21 @@  static const struct dev_pm_ops kxcjk1013_pm_ops = {
 };
 
 static const struct i2c_device_id kxcjk1013_id[] = {
-	{"kxcjk1013", KXCJK1013},
-	{"kxcj91008", KXCJ91008},
-	{"kxtj21009", KXTJ21009},
-	{"kxtf9",     KXTF9},
-	{"kx023-1025", KX0231025},
+	{"kxcjk1013",  (kernel_ulong_t)&kxcjk1013_info },
+	{"kxcj91008",  (kernel_ulong_t)&kxcj91008_info },
+	{"kxtj21009",  (kernel_ulong_t)&kxtj21009_info },
+	{"kxtf9", (kernel_ulong_t)&kxtf9_info },
+	{"kx023-1025", (kernel_ulong_t)&kx0231025_info },
 	{}
 };
-
 MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
 
 static const struct of_device_id kxcjk1013_of_match[] = {
-	{ .compatible = "kionix,kxcjk1013", },
-	{ .compatible = "kionix,kxcj91008", },
-	{ .compatible = "kionix,kxtj21009", },
-	{ .compatible = "kionix,kxtf9", },
-	{ .compatible = "kionix,kx023-1025", },
+	{ .compatible = "kionix,kxcjk1013", &kxcjk1013_info },
+	{ .compatible = "kionix,kxcj91008", &kxcj91008_info },
+	{ .compatible = "kionix,kxtj21009", &kxtj21009_info },
+	{ .compatible = "kionix,kxtf9", &kxtf9_info },
+	{ .compatible = "kionix,kx023-1025", &kx0231025_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, kxcjk1013_of_match);