| Message ID | 20210428164219.1115537-2-mkorpershoek@baylibre.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | Input: mtk-pmic-keys - add support for MT6358 | expand |
Hi Mattijs, On Wed, Apr 28, 2021 at 06:42:13PM +0200, Mattijs Korpershoek wrote: > mtk-pmic-keys being a child device of mt6397, it will always get probed > when mt6397_probe() is called. > > This also happens when we have no device tree node matching > mediatek,mt6397-keys. It sounds for me that creating a platform device instance in case where we know need OF node, but do not have one, is wasteful. Can mt6397-core.c and/or MFD core be adjusted to not do that. > > In that case, the mfd core warns us: > > [ 0.352175] mtk-pmic-keys: Failed to locate of_node [id: -1] > > Check return value from call to of_match_device() > in order to prevent a NULL pointer dereference. > > In case of NULL print error message and return -ENODEV > > Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> > --- > drivers/input/keyboard/mtk-pmic-keys.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c > index 62391d6c7da6..12c449eed026 100644 > --- a/drivers/input/keyboard/mtk-pmic-keys.c > +++ b/drivers/input/keyboard/mtk-pmic-keys.c > @@ -247,6 +247,9 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev) > const struct of_device_id *of_id = > of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev); > > + if (!of_id) > + return -ENODEV; > + So if we make MFD/6396 core smarter we would not be needing this. I guess there is still a possibility of someone stuffing "mtk-pmic-keys" into "driver_override" attribute of a random platform device but I wonder if we really need to take care of such scenarios... > keys = devm_kzalloc(&pdev->dev, sizeof(*keys), GFP_KERNEL); > if (!keys) > return -ENOMEM; > -- > 2.27.0 > Thanks.
Hi Dmitry, Dmitry Torokhov <dmitry.torokhov@gmail.com> writes: > Hi Mattijs, > > On Wed, Apr 28, 2021 at 06:42:13PM +0200, Mattijs Korpershoek wrote: >> mtk-pmic-keys being a child device of mt6397, it will always get probed >> when mt6397_probe() is called. >> >> This also happens when we have no device tree node matching >> mediatek,mt6397-keys. > > It sounds for me that creating a platform device instance in case where > we know need OF node, but do not have one, is wasteful. Can > mt6397-core.c and/or MFD core be adjusted to not do that. You are right. Maybe I can fix MFD core instead. I will look into it. Thanks for your review. > >> >> In that case, the mfd core warns us: >> >> [ 0.352175] mtk-pmic-keys: Failed to locate of_node [id: -1] >> >> Check return value from call to of_match_device() >> in order to prevent a NULL pointer dereference. >> >> In case of NULL print error message and return -ENODEV >> >> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> >> --- >> drivers/input/keyboard/mtk-pmic-keys.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c >> index 62391d6c7da6..12c449eed026 100644 >> --- a/drivers/input/keyboard/mtk-pmic-keys.c >> +++ b/drivers/input/keyboard/mtk-pmic-keys.c >> @@ -247,6 +247,9 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev) >> const struct of_device_id *of_id = >> of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev); >> >> + if (!of_id) >> + return -ENODEV; >> + > > So if we make MFD/6396 core smarter we would not be needing this. I > guess there is still a possibility of someone stuffing "mtk-pmic-keys" > into "driver_override" attribute of a random platform device but I > wonder if we really need to take care of such scenarios... > >> keys = devm_kzalloc(&pdev->dev, sizeof(*keys), GFP_KERNEL); >> if (!keys) >> return -ENOMEM; >> -- >> 2.27.0 >> > > Thanks. > > -- > Dmitry
Hi Dmitry, Mattijs Korpershoek <mkorpershoek@baylibre.com> writes: > Hi Dmitry, > > Dmitry Torokhov <dmitry.torokhov@gmail.com> writes: > >> Hi Mattijs, >> >> On Wed, Apr 28, 2021 at 06:42:13PM +0200, Mattijs Korpershoek wrote: >>> mtk-pmic-keys being a child device of mt6397, it will always get probed >>> when mt6397_probe() is called. >>> >>> This also happens when we have no device tree node matching >>> mediatek,mt6397-keys. >> >> It sounds for me that creating a platform device instance in case where >> we know need OF node, but do not have one, is wasteful. Can >> mt6397-core.c and/or MFD core be adjusted to not do that. > > You are right. Maybe I can fix MFD core instead. I will look into it. > > Thanks for your review. >> >>> >>> In that case, the mfd core warns us: >>> >>> [ 0.352175] mtk-pmic-keys: Failed to locate of_node [id: -1] >>> >>> Check return value from call to of_match_device() >>> in order to prevent a NULL pointer dereference. >>> >>> In case of NULL print error message and return -ENODEV >>> >>> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> >>> --- >>> drivers/input/keyboard/mtk-pmic-keys.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c >>> index 62391d6c7da6..12c449eed026 100644 >>> --- a/drivers/input/keyboard/mtk-pmic-keys.c >>> +++ b/drivers/input/keyboard/mtk-pmic-keys.c >>> @@ -247,6 +247,9 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev) >>> const struct of_device_id *of_id = >>> of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev); >>> >>> + if (!of_id) >>> + return -ENODEV; >>> + >> >> So if we make MFD/6396 core smarter we would not be needing this. I >> guess there is still a possibility of someone stuffing "mtk-pmic-keys" >> into "driver_override" attribute of a random platform device but I >> wonder if we really need to take care of such scenarios... It turns out it was possible to make 6397-core smarter. I've submitted [1] to replace this patch. Thanks again for your suggestion. Please let me know if I should add your Suggested-by: in [1]. [1] https://patchwork.kernel.org/project/linux-mediatek/patch/20210429143811.2030717-1-mkorpershoek@baylibre.com/ instead >> >>> keys = devm_kzalloc(&pdev->dev, sizeof(*keys), GFP_KERNEL); >>> if (!keys) >>> return -ENOMEM; >>> -- >>> 2.27.0 >>> >> >> Thanks. >> >> -- >> Dmitry
diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c index 62391d6c7da6..12c449eed026 100644 --- a/drivers/input/keyboard/mtk-pmic-keys.c +++ b/drivers/input/keyboard/mtk-pmic-keys.c @@ -247,6 +247,9 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev) const struct of_device_id *of_id = of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev); + if (!of_id) + return -ENODEV; + keys = devm_kzalloc(&pdev->dev, sizeof(*keys), GFP_KERNEL); if (!keys) return -ENOMEM;
mtk-pmic-keys being a child device of mt6397, it will always get probed when mt6397_probe() is called. This also happens when we have no device tree node matching mediatek,mt6397-keys. In that case, the mfd core warns us: [ 0.352175] mtk-pmic-keys: Failed to locate of_node [id: -1] Check return value from call to of_match_device() in order to prevent a NULL pointer dereference. In case of NULL print error message and return -ENODEV Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> --- drivers/input/keyboard/mtk-pmic-keys.c | 3 +++ 1 file changed, 3 insertions(+)