RE: [PATCH v7 0/4] Extend device_get_match_data() to struct bus_type

[Biju Das]

Hi Andy Shevchenko,

> Subject: Re: [PATCH v7 0/4] Extend device_get_match_data() to struct
> bus_type
> 
> On Thu, Aug 10, 2023 at 09:05:10AM +0000, Biju Das wrote:
> > > On Tue, 8 Aug 2023 15:18:52 +0300
> > > Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > > > On Mon, Aug 07, 2023 at 01:37:12PM -0700, Dmitry Torokhov wrote:
> > > > > On Mon, Aug 07, 2023 at 05:54:07PM +0300, Andy Shevchenko wrote:
> 
> ...
> 
> > > > > So in legacy ID lookup path we can safely assume that values
> > > > > below
> > > > > 4096 are scalars and return NULL from the new
> > > > > device_get_match_data(). This way current drivers using the
> > > > > values as indices or doing direct comparisons against them can
> > > > > continue doing manual look up and using them as they see fit.
> > > > > And we can
> > > convert the drivers at our leisure.
> > > >
> > > > It's a good idea, but I believe will be received as hack.
> > > > But why not to try? We indeed have an error pointers for the last
> > > > page and NULL (which is only up to 16 IIRC) and reserved space in
> > > > the first page. To be more robust I would check all enums that are
> > > > being used in I2C ID tables for maximum value and if that is less
> > > > than 16, use
> > > > ZERO_OR_NULL_PTR() instead of custom stuff.
> > > >
> > > See iio/adc/max1363 example that has 37ish.
> > >
> > > Could tidy that one up first and hopefully not find any others that
> > > are in subsystems not keen on the move away from enums.
> >
> > If there is no objection, I can fix this using i2c_get_match_data()
> > for
> > iio/adc/max1363 and device_match_data() will return ZERO_OR_NULL_PTR()
> > if max enum ID in the ID lookup table is less than 16. And the drivers
> > that use legacy ID's will fallback to ID lookup. So that there won't
> > be any regression.
> 
> I'm good with this approach, but make sure you checked the whole kernel
> source tree for a such.

Checking against 16 is too short I guess??

drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h has 18 enums.

/*device enum */
enum inv_devices {
	INV_MPU6050,
	INV_MPU6500,
	INV_MPU6515,
	INV_MPU6880,
	INV_MPU6000,
	INV_MPU9150,
	INV_MPU9250,
	INV_MPU9255,
	INV_ICM20608,
	INV_ICM20608D,
	INV_ICM20609,
	INV_ICM20689,
	INV_ICM20600,
	INV_ICM20602,
	INV_ICM20690,
	INV_IAM20680,
	INV_NUM_PARTS
};

The new helper function

+static bool i2c_is_client_uses_legacy_id_table(const struct i2c_driver *driver)
+{
+	const struct i2c_device_id *id = driver->id_table;
+	kernel_ulong_t max_val = 0;
+
+	if (!id)
+		return FALSE;
+
+	while (id->name[0]) {
+		if (id->driver_data > max_val)
+			max_val = id->driver_data;
+		id++;
+	}
+
+	return ZERO_OR_NULL_PTR(max_val);
+}
+

Cheers,
Biju