On Wed, Jul 27, 2022 at 6:43 PM Potin Lai <potin.lai.pt@xxxxxxxxx> wrote: > > Add manufacturer and device ID checking during probe, and skip the > checking if chip model not supported. > > Supported: > - HDC1000 > - HDC1010 > - HDC1050 > - HDC1080 > > Not supported: > - HDC1008 Almost there! ... > +static bool is_valid_hdc100x(struct i2c_client *client) > +{ > + const struct hdc100x_chip_data *data; > + int mfr_id, dev_id; > + data = device_get_match_data(&client->dev); > + if (!data->support_mfr_check) > + return true; Logically this part does belong to ->probe() and doesn't here (too wide for this specific helper). > + mfr_id = hdc100x_read_mfr_id(client); > + dev_id = hdc100x_read_dev_id(client); > + if (mfr_id == HDC100X_MFR_ID && > + (dev_id == 0x1000 || dev_id == 0x1050)) > + return true; > + > + return false; > +} ... > + if (!is_valid_hdc100x(client)) > + return -EINVAL; Means here you add const struct hdc100x_chip_data *data; struct device *dev = &client->dev; ... data = device_get_match_data(dev); if (data->support_mfr_check && !is_valid_hdc100x(client)) return -EINVAL; (Introducing a temporary variable for struct device pointer might also help in future to refactor to make code neater.) -- With Best Regards, Andy Shevchenko