On Wed, Nov 30, 2022 at 05:32:10PM +0100, Gerald Loacker wrote: > Am 30.11.2022 um 16:31 schrieb Andy Shevchenko: > > On Wed, Nov 30, 2022 at 03:53:56PM +0100, Gerald Loacker wrote: ... > >> + switch (data->devid) { > >> + case TMAG5273_MANUFACTURER_ID: > >> + /* > >> + * The device name matches the orderable part number. 'x' stands > >> + * for A, B, C or D devices, which have different I2C addresses. > >> + * Versions 1 or 2 (0 and 3 is reserved) stands for different > >> + * magnetic strengths. > >> + */ > >> + snprintf(data->name, sizeof(data->name), "tmag5273x%1u", data->version); > >> + if (data->version < 1 || data->version > 2) > >> + dev_warn(data->dev, "Unsupported device %s\n", data->name); > >> + return 0; > >> + default: > > > >> + dev_warn(data->dev, "Unknown device ID 0x%x\n", data->devid); > >> + return 0; > > > > And we still continue?! Wouldn't be a problem if that ID drastically changed in > > terms of programming model and may actually be broken by a wrong sequence? > > It was suggested by Jonathan to just print a warning instead of > returning with -ENODEV. Reason was "Often manufacturers spin new > versions of chips that are compatible enough that we give them a > fallback compatible in device tree.". For me this makes sense. Ah, I see. Maybe adding a comment summarizing above? > >> + } -- With Best Regards, Andy Shevchenko