Hi Mauro,
On Fri, Dec 07, 2018 at 08:07:54AM -0500, Mauro Carvalho Chehab wrote:
> As warned by smatch:
> drivers/media/platform/pxa_camera.c:2400 pxa_camera_probe() error: we
> previously assumed 'pcdev->pdata' could be null (see line 2397)
>
> It would be possible that neither DT nor platform data would be
> provided. This is a Kernel bug, so warn about that and bail.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+sams...@kernel.org>
> ---
> drivers/media/platform/pxa_camera.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/platform/pxa_camera.c
> b/drivers/media/platform/pxa_camera.c
> index 5f930560eb30..f91f8fd424c4 100644
> --- a/drivers/media/platform/pxa_camera.c
> +++ b/drivers/media/platform/pxa_camera.c
> @@ -2396,6 +2396,9 @@ static int pxa_camera_probe(struct platform_device
> *pdev)
> pcdev->pdata = pdev->dev.platform_data;
> if (pdev->dev.of_node && !pcdev->pdata) {
> err = pxa_camera_pdata_from_dt(&pdev->dev, pcdev, &pcdev->asd);
> + } else if (!pcdev->pdata) {
This fixes the issue, but the current checks remain a bit odd.
The driver seems to prefer platform data over OF. I wonder if that's
intentional or not.
In that case, I'd roughly write this as:
if (pcdev->pdata) {
...;
} else if (pdev->dev.of_node) {
...;
} else {
return -ENODEV;
}
I'm not sure WARN_ON(1) is necessary. A lot of drivers simply do it this
way without WARN_ON().
> + WARN_ON(1);
> + return -ENODEV;
> } else {
> pcdev->platform_flags = pcdev->pdata->flags;
> pcdev->mclk = pcdev->pdata->mclk_10khz * 10000;
--
Regards,
Sakari Ailus
sakari.ai...@linux.intel.com
