Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:27PM +0100, Michael Tretter wrote:
> Unfortunately, the PXP_HW_VERSION register reports the PXP on the i.MX7D
> and on the i.MX6ULL as version 3.0, although the PXP versions on these
> SoCs have significant differences.
>
> Use the compatible to configure the ctrl0 register as required dependent
> on the platform.
>
> Signed-off-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index 1d649b9cadad..4e182f80a36b 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -19,6 +19,7 @@
> #include <linux/iopoll.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/sched.h>
> #include <linux/slab.h>
>
> @@ -191,6 +192,11 @@ static struct pxp_fmt *find_format(struct v4l2_format *f)
> return &formats[k];
> }
>
> +struct pxp_ctx;
Please add a blank line here.
> +struct pxp_pdata {
> + u32 (*data_path_ctrl0)(struct pxp_ctx *ctx);
> +};
> +
> struct pxp_dev {
> struct v4l2_device v4l2_dev;
> struct video_device vfd;
> @@ -199,6 +205,7 @@ struct pxp_dev {
> void __iomem *mmio;
>
> u32 hw_version;
> + const struct pxp_pdata *pdata;
>
> atomic_t num_inst;
> struct mutex dev_mutex;
> @@ -726,7 +733,7 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
> }
> }
>
> -static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> +static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
> {
> u32 ctrl0;
>
> @@ -756,6 +763,16 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> return ctrl0;
> }
>
> +static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> +{
> + struct pxp_dev *dev = ctx->dev;
> +
> + if (dev->pdata && dev->pdata->data_path_ctrl0)
> + return dev->pdata->data_path_ctrl0(ctx);
> +
> + return pxp_imx6ull_data_path_ctrl0(ctx);
Do you need this fallback, given that all compatible strings give you
valid pdata ? I'd rather be explicit.
This function then becomes so small that I would inline it in the
caller.
> +}
> +
> static void pxp_set_data_path(struct pxp_ctx *ctx)
> {
> struct pxp_dev *dev = ctx->dev;
> @@ -1711,6 +1728,8 @@ static int pxp_probe(struct platform_device *pdev)
> if (!dev)
> return -ENOMEM;
>
> + dev->pdata = of_device_get_match_data(&pdev->dev);
> +
> dev->clk = devm_clk_get(&pdev->dev, "axi");
> if (IS_ERR(dev->clk)) {
> ret = PTR_ERR(dev->clk);
> @@ -1811,8 +1830,12 @@ static int pxp_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct pxp_pdata pxp_imx6ull_pdata = {
> + .data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
> +};
> +
> static const struct of_device_id pxp_dt_ids[] = {
> - { .compatible = "fsl,imx6ull-pxp", .data = NULL },
> + { .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
> { },
> };
> MODULE_DEVICE_TABLE(of, pxp_dt_ids);
--
Regards,
Laurent Pinchart
