On 24.06.2022 17:47, Laurent Pinchart wrote:
Hi Dafna,
On Fri, Jun 24, 2022 at 05:34:00PM +0300, Dafna Hirschfeld wrote:
On 15.06.2022 04:10, Paul Elder wrote:
> To make it possible to use the rkisp1_info after probe time (for
> instance to make code conditional on the ISP version), save it in the
> main rkisp1_device structure. To achieve this, also move the info
> structure into the common header, and document it.
>
> While at it, drop a NULL check in rkisp1_probe() for the match data as
> it can't happen.
>
> Signed-off-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> .../platform/rockchip/rkisp1/rkisp1-common.h | 22 +++++++++++++++++++
> .../platform/rockchip/rkisp1/rkisp1-dev.c | 15 +++----------
> 2 files changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index a67fe7b1dfa1..50d31a254b03 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -91,6 +91,26 @@ enum rkisp1_isp_pad {
> RKISP1_ISP_PAD_MAX
> };
>
> +/*
> + * struct rkisp1_info - Model-specific ISP Information
> + *
> + * @clks: array of ISP clock names
> + * @clk_size: number of entries in the @clks array
> + * @isrs: array of ISP interrupt descriptors
> + * @isr_size: number of entires in the @isrs array
> + * @isp_ver: ISP version
> + *
> + * This structure contains information about the ISP specific to a particular
> + * ISP model, version, or integration in a particular SoC.
> + */
> +struct rkisp1_info {
> + const char * const *clks;
> + unsigned int clk_size;
> + const struct rkisp1_isr_data *isrs;
> + unsigned int isr_size;
> + enum rkisp1_cif_isp_version isp_ver;
> +};
> +
> /*
> * struct rkisp1_sensor_async - A container for the v4l2_async_subdev to add to the notifier
> * of the v4l2-async API
> @@ -395,6 +415,7 @@ struct rkisp1_debug {
> * @pipe: media pipeline
> * @stream_lock: serializes {start/stop}_streaming callbacks between the capture devices.
> * @debug: debug params to be exposed on debugfs
> + * @info: version-specific ISP information
> */
> struct rkisp1_device {
> void __iomem *base_addr;
> @@ -413,6 +434,7 @@ struct rkisp1_device {
> struct media_pipeline pipe;
> struct mutex stream_lock; /* serialize {start/stop}_streaming cb between capture devices */
> struct rkisp1_debug debug;
> + const struct rkisp1_info *info;
> };
>
> /*
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> index 258980ef4783..39ae35074062 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> @@ -105,14 +105,6 @@ struct rkisp1_isr_data {
> irqreturn_t (*isr)(int irq, void *ctx);
> };
>
> -struct rkisp1_info {
> - const char * const *clks;
> - unsigned int clk_size;
> - const struct rkisp1_isr_data *isrs;
> - unsigned int isr_size;
> - enum rkisp1_cif_isp_version isp_ver;
> -};
> -
> /* ----------------------------------------------------------------------------
> * Sensor DT bindings
> */
> @@ -469,14 +461,13 @@ static int rkisp1_probe(struct platform_device *pdev)
> int ret, irq;
> u32 cif_id;
>
> - info = of_device_get_match_data(&pdev->dev);
> - if (!info)
> - return -ENODEV;
> -
> rkisp1 = devm_kzalloc(dev, sizeof(*rkisp1), GFP_KERNEL);
> if (!rkisp1)
> return -ENOMEM;
>
> + info = of_device_get_match_data(dev);
Why did you omit the check 'if(!info)'?
Because it can't happen. The probe() function is only called if the
platform device matches one of the of_device_id, so
of_device_get_match_data() can't return NULL.
I see now it is also written in the commit log,
Reviewed-by: Dafna Hirschfeld <dafna@xxxxxxxxxxxx>
> + rkisp1->info = info;
> +
> dev_set_drvdata(dev, rkisp1);
> rkisp1->dev = dev;
>
--
Regards,
Laurent Pinchart
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-rockchip
