On Mon, Sep 4, 2023 at 3:02 PM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > On 04/09/2023 14:30, Mark Brown wrote: > > On Mon, Sep 04, 2023 at 02:17:43PM +0200, Krzysztof Kozlowski wrote: > >> On 04/09/2023 13:46, wangweidong.a@xxxxxxxxxx wrote: > > > >>> + ret = regmap_read(regmap, AW87390_ID_REG, &chip_id); > >>> + if (ret) { > >>> + dev_err(&i2c->dev, "%s read chipid error. ret = %d\n", __func__, ret); > >>> + return ret; > >>> + } > > > >>> + if (chip_id != AW87390_CHIP_ID) { > >>> + dev_err(&i2c->dev, "unsupported device\n"); > > > >> Why? The compatible tells it cannot be anything else. > > > > This is very common good practice, as well as validating communication > > No, it is neither common nor good. The kernel's job is not to verify the > supplied DTS. Rob also made here a point: > > https://lore.kernel.org/all/CAL_Jsq+wcrOjh7+0c=mrg+Qz6dbhOUE-VEeQ4FoWC3Y7ENoyfQ@xxxxxxxxxxxxxx/ I disagree, if a vendor one day decides to mount a new version of a component without notifying the community or users this is really helpful. The function is named "probe()" for a reason, as in "inspect the hardware and see what we find" this has always been the case I think. Yours, Linus Walleij