| Message ID | 1478015868-10309-2-git-send-email-andrew.smirnov@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 1, 2016 at 4:57 PM, Andrey Smirnov <andrew.smirnov@gmail.com> 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@gmail.com> (...) > -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. 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(). It's no strong preference: I will still apply this patch set because it is overall very very good. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 4, 2016 at 5:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Tue, Nov 1, 2016 at 4:57 PM, Andrey Smirnov <andrew.smirnov@gmail.com> 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@gmail.com> > (...) >> -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. Thanks, Andrey -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 4, 2016 at 9:09 PM, Andrey Smirnov <andrew.smirnov@gmail.com> wrote: > On Fri, Nov 4, 2016 at 5:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Tue, Nov 1, 2016 at 4:57 PM, Andrey Smirnov <andrew.smirnov@gmail.com> 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@gmail.com> >> (...) >>> -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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pinctrl/pinctrl-sx150x.c b/drivers/pinctrl/pinctrl-sx150x.c index d2d4211..41b9e6a 100644 --- a/drivers/pinctrl/pinctrl-sx150x.c +++ b/drivers/pinctrl/pinctrl-sx150x.c @@ -861,14 +861,6 @@ static const struct i2c_device_id sx150x_id[] = { {} }; -static const struct of_device_id sx150x_of_match[] = { - { .compatible = "semtech,sx1508q" }, - { .compatible = "semtech,sx1509q" }, - { .compatible = "semtech,sx1506q" }, - { .compatible = "semtech,sx1502q" }, - {}, -}; - static int sx150x_init_io(struct sx150x_pinctrl *pctl, u8 base, u16 cfg) { int err = 0; @@ -1049,7 +1041,6 @@ static int sx150x_probe(struct i2c_client *client, static struct i2c_driver sx150x_driver = { .driver = { .name = "sx150x-pinctrl", - .of_match_table = of_match_ptr(sx150x_of_match), }, .probe = sx150x_probe, .id_table = sx150x_id,
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@gmail.com> --- drivers/pinctrl/pinctrl-sx150x.c | 9 --------- 1 file changed, 9 deletions(-)