| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <51f4ac00-40c1-4dc2-9ab2-611f18462a84@linaro.org>
Date: Sat, 30 Sep 2023 17:04:18 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Mehdi Djait <mehdi.djait@...tlin.com>, mchehab@...nel.org,
heiko@...ech.de, ezequiel@...guardiasur.com.ar,
hverkuil-cisco@...all.nl, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org
Cc: paul.kocialkowski@...tlin.com, miquel.raynal@...tlin.com,
maxime.chevallier@...tlin.com, luca.ceresoli@...tlin.com,
thomas.petazzoni@...tlin.com, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v6 2/3] media: rockchip: Add a driver for Rockhip's camera
interface
On 29/09/2023 12:08, Mehdi Djait wrote:
> Introduce a driver for the camera interface on some Rockchip platforms.
>
...
> +static const struct of_device_id rk_vip_plat_of_match[] = {
> + {
> + .compatible = "rockchip,px30-vip",
> + .data = &px30_vip_match_data,
> + },
> + {},
> +};
> +
> +void rk_vip_soft_reset(struct rk_vip_device *vip_dev)
> +{
> + reset_control_assert(vip_dev->vip_rst);
> +
> + udelay(5);
> +
> + reset_control_deassert(vip_dev->vip_rst);
> +}
> +
> +static int rk_vip_plat_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *match;
> + struct device_node *node = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + struct v4l2_device *v4l2_dev;
> + struct rk_vip_device *vip_dev;
> + const struct vip_match_data *data;
> + struct resource *res;
> + int i, ret, irq;
> +
> + match = of_match_node(rk_vip_plat_of_match, node);
> + if (IS_ERR(match))
of_match_node does no return ERR_PTR.
> + return PTR_ERR(match);
> +
> + vip_dev = devm_kzalloc(dev, sizeof(*vip_dev), GFP_KERNEL);
> + if (!vip_dev)
> + return -ENOMEM;
> +
> + dev_set_drvdata(dev, vip_dev);
> + vip_dev->dev = dev;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + ret = devm_request_irq(dev, irq, rk_vip_irq_pingpong, IRQF_SHARED,
Shared? With devm? This is error-prone pattern (as discussed few times
on the list). Are you 100% sure that all your use-cases are correct?
> + dev_driver_string(dev), dev);
> + if (ret < 0) {
> + dev_err(dev, "request irq failed: %d\n", ret);
return dev_err_probe
> + return ret;
> + }
> +
> + vip_dev->irq = irq;
> + data = match->data;
> + vip_dev->chip_id = data->chip_id;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + vip_dev->base_addr = devm_ioremap_resource(dev, res);
Use wrapper for these two.
> +
> + if (IS_ERR(vip_dev->base_addr))
> + return PTR_ERR(vip_dev->base_addr);
> +
> + for (i = 0; i < data->clks_num; i++)
> + vip_dev->clks[i].id = data->clks[i];
> +
> + vip_dev->num_clk = data->clks_num;
> +
> + ret = devm_clk_bulk_get(dev, vip_dev->num_clk, vip_dev->clks);
> + if (ret)
> + return ret;
> +
> + vip_dev->vip_rst = devm_reset_control_array_get(dev, false, false);
> + if (IS_ERR(vip_dev->vip_rst))
> + return PTR_ERR(vip_dev->vip_rst);
> +
> + /* Initialize the stream */
> + rk_vip_stream_init(vip_dev);
> + strscpy(vip_dev->media_dev.model, "rk_vip",
> + sizeof(vip_dev->media_dev.model));
> + vip_dev->media_dev.dev = &pdev->dev;
> + v4l2_dev = &vip_dev->v4l2_dev;
> + v4l2_dev->mdev = &vip_dev->media_dev;
> + strscpy(v4l2_dev->name, "rk_vip", sizeof(v4l2_dev->name));
> +
> + ret = v4l2_device_register(vip_dev->dev, &vip_dev->v4l2_dev);
> + if (ret < 0)
> + return ret;
> +
> + media_device_init(&vip_dev->media_dev);
> +
> + ret = media_device_register(&vip_dev->media_dev);
> + if (ret < 0) {
> + v4l2_err(v4l2_dev, "Failed to register media device: %d\n",
> + ret);
> + goto err_unreg_v4l2_dev;
> + }
> +
> + /* create & register platform subdev (from of_node) */
> + ret = rk_vip_register_platform_subdevs(vip_dev);
> + if (ret < 0)
> + goto err_unreg_media_dev;
> +
> + vip_dev->sensor.std = V4L2_STD_NTSC;
> + rk_vip_set_default_format(vip_dev);
> + pm_runtime_enable(&pdev->dev);
> +
> + return 0;
> +
> +err_unreg_media_dev:
> + media_device_unregister(&vip_dev->media_dev);
> +err_unreg_v4l2_dev:
> + v4l2_device_unregister(&vip_dev->v4l2_dev);
> + return ret;
> +}
> +
> +static int rk_vip_plat_remove(struct platform_device *pdev)
> +{
> + struct rk_vip_device *vip_dev = platform_get_drvdata(pdev);
> +
> + pm_runtime_disable(&pdev->dev);
> +
> + media_device_unregister(&vip_dev->media_dev);
> + v4l2_device_unregister(&vip_dev->v4l2_dev);
> + rk_vip_unregister_stream_vdev(vip_dev);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused rk_vip_runtime_suspend(struct device *dev)
> +{
> + struct rk_vip_device *vip_dev = dev_get_drvdata(dev);
> +
> + clk_bulk_disable_unprepare(vip_dev->num_clk, vip_dev->clks);
> +
> + return pinctrl_pm_select_sleep_state(dev);
> +}
> +
> +static int __maybe_unused rk_vip_runtime_resume(struct device *dev)
> +{
> + struct rk_vip_device *vip_dev = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = pinctrl_pm_select_default_state(dev);
> + if (ret < 0)
> + return ret;
> +
> + return clk_bulk_prepare_enable(vip_dev->num_clk, vip_dev->clks);
> +}
> +
> +static const struct dev_pm_ops rk_vip_plat_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(rk_vip_runtime_suspend, rk_vip_runtime_resume, NULL)
> +};
> +
> +static struct platform_driver rk_vip_plat_drv = {
> + .driver = {
> + .name = VIP_DRIVER_NAME,
> + .of_match_table = of_match_ptr(rk_vip_plat_of_match),
drop of_match_ptr, causes warnings in your code.
Best regards,
Krzysztof
Powered by blists - more mailing lists