On Fri, Nov 4, 2016 at 9:09 PM, Andrey Smirnov <andrew.smirnov@xxxxxxxxx> wrote: > On Fri, Nov 4, 2016 at 5:28 AM, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: >> On Tue, Nov 1, 2016 at 4:57 PM, Andrey Smirnov <andrew.smirnov@xxxxxxxxx> wrote: >> >>> None of the OF match table entries contain any compatiblity strings that >>> could not be matched against using i2c_device_id table above and >>> of_modalias_node. Besides that entries in OF match table do not cary >>> proper device variant information which is need by the drive. Those two >>> facts combined, IMHO, make a compelling case for removal of that code >>> altogether. >>> >>> Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx> >> (...) >>> -static const struct of_device_id sx150x_of_match[] = { >>> - { .compatible = "semtech,sx1508q" }, >>> - { .compatible = "semtech,sx1509q" }, >>> - { .compatible = "semtech,sx1506q" }, >>> - { .compatible = "semtech,sx1502q" }, >>> - {}, >>> -}; >> >> I'm a bit hesitant about this since we should ideally first match on the >> compatible string for any device. We have tried to alleviate the situation >> in I2C devices but it has been a bit so-so. >> > > Ah, good to know. Let's do it that way then. > >> It would be best if we make a separate patch after this tjat adds it >> back, set the variant data also in the .data of the match and >> use of_device_get_match_data(). > > Do you prefer it as a separate patch, or, instead, should I change > this patch of the series to do what you describe? I'd be happy to do > either and it seems like it would be a trivial change so it should > invalidate any of the testing done by Neil. Yeah it would ideally be a modification of this patch. Whatever is easiest for you to do! BTW this is a great patch set and I'm very grateful for yours+Neils combines contributions on getting this driver into shape. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html