Re: [PATCH v2 3/4] media: misc: introduce microchip_csi2dc driver
From: <hidden>
Date: 2020-08-25 12:44:41
Also in:
linux-devicetree, linux-media, lkml
On 14.08.2020 01:44, Sakari Ailus wrote:
Hi Eugen, Thanks for the patch.
Hello Sakari, Thanks for reviewing.
On Fri, Jul 03, 2020 at 10:44:15AM +0300, Eugen Hristev wrote:quoted
Microchip CSI2DC (CSI2 Demultiplexer Controller) is a misc bridge device that converts a byte stream in IDI Synopsys format (coming from a CSI2HOST) to a pixel stream that can be captured by a sensor controller. Signed-off-by: Eugen Hristev <redacted> --- drivers/media/misc/Kconfig | 9 + drivers/media/misc/Makefile | 1 + drivers/media/misc/microchip_csi2dc.c | 705 ++++++++++++++++++++++++++ 3 files changed, 715 insertions(+) create mode 100644 drivers/media/misc/microchip_csi2dc.cdiff --git a/drivers/media/misc/Kconfig b/drivers/media/misc/Kconfig index 206f39f86f46..86e892a41eba 100644 --- a/drivers/media/misc/Kconfig +++ b/drivers/media/misc/Kconfig@@ -8,6 +8,15 @@ menu "Miscellaneous helper chips" comment "Various helper chips" +config VIDEO_MICROCHIP_CSI2DC + tristate "Microchip CSI2 Demux Controller" + depends on VIDEO_V4L2 + help + CSI2 Demux Controller driver. CSI2DC is a helper chip + that converts IDI interface byte stream to a parallel pixel stream. + It supports various RAW formats as input. + Performs clock domain crossing between hardware blocks. + endmenu endifdiff --git a/drivers/media/misc/Makefile b/drivers/media/misc/Makefile index f66554cd5c45..86477781b7af 100644 --- a/drivers/media/misc/Makefile +++ b/drivers/media/misc/Makefile@@ -1 +1,2 @@ # SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_VIDEO_MICROCHIP_CSI2DC) += microchip_csi2dc.odiff --git a/drivers/media/misc/microchip_csi2dc.c b/drivers/media/misc/microchip_csi2dc.c new file mode 100644 index 000000000000..494b7c137b17 --- /dev/null +++ b/drivers/media/misc/microchip_csi2dc.c@@ -0,0 +1,705 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Microchip CSI2 Demux Controller (CSI2DC) driver + * + * Copyright (C) 2018-2020 Microchip Technology, Inc. + * + * Author: Eugen Hristev <eugen.hristev@microchip.com> + * + */ + +#include <linux/clk.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_graph.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/videodev2.h> + +#include <media/v4l2-device.h> +#include <media/v4l2-fwnode.h> +#include <media/v4l2-subdev.h> +#include <media/videobuf2-dma-contig.h> + +/* Global configuration register */ +#define CSI2DC_GCFG 0x0 + +/* MIPI sensor pixel clock is free running */ +#define CSI2DC_GCFG_MIPIFRN BIT(0) +/* Output waveform inter-line minimum delay */ +#define CSI2DC_GCFG_HLC(v) ((v) << 4) +#define CSI2DC_GCFG_HLC_MASK GENMASK(7, 4) + +/* Global control register */ +#define CSI2DC_GCTLR 0x04 +#define CSI2DC_GCTLR_SWRST BIT(0) + +/* Global status register */ +#define CSI2DC_GS 0x08 + +/* SSP interrupt status register */ +#define CSI2DC_SSPIS 0x28 +/* Pipe update register */ +#define CSI2DC_PU 0xC0 +/* Video pipe attributes update */ +#define CSI2DC_PU_VP BIT(0) + +/* Pipe update status register */ +#define CSI2DC_PUS 0xC4 + +/* Video pipeline enable register */ +#define CSI2DC_VPE 0xF8 +#define CSI2DC_VPE_ENABLE BIT(0) + +/* Video pipeline configuration register */ +#define CSI2DC_VPCFG 0xFC +/* Data type */ +#define CSI2DC_VPCFG_DT(v) ((v) << 0) +#define CSI2DC_VPCFG_DT_MASK GENMASK(5, 0) +/* Virtual channel identifier */ +#define CSI2DC_VPCFG_VC(v) ((v) << 6) +#define CSI2DC_VPCFG_VC_MASK GENMASK(7, 6) +/* Decompression enable */ +#define CSI2DC_VPCFG_DE BIT(8) +/* Decoder mode */ +#define CSI2DC_VPCFG_DM(v) ((v) << 9) +#define CSI2DC_VPCFG_DM_DECODER8TO12 0 +/* Decoder predictor 2 selection */ +#define CSI2DC_VPCFG_DP2 BIT(12) +/* Recommended memory storage */ +#define CSI2DC_VPCFG_RMS BIT(13) +/* Post adjustment */ +#define CSI2DC_VPCFG_PA BIT(14) + +/* Video pipeline column register */ +#define CSI2DC_VPCOL 0x100 +/* Column number */ +#define CSI2DC_VPCOL_COL(v) ((v) << 0) +#define CSI2DC_VPCOL_COL_MASK GENMASK(15, 0) + +/* Video pipeline row register */ +#define CSI2DC_VPROW 0x104 +/* Row number */ +#define CSI2DC_VPROW_ROW(v) ((v) << 0) +#define CSI2DC_VPROW_ROW_MASK GENMASK(15, 0) + +/* Version register */ +#define CSI2DC_VERSION 0x1FC + +/* register read/write helpers */ +#define csi2dc_readl(st, reg) readl_relaxed((st)->base + (reg)) +#define csi2dc_writel(st, reg, val) writel_relaxed((val), (st)->base + (reg)) + +/* supported RAW data types */ +#define CSI2DC_DT_RAW6 0x28 +#define CSI2DC_DT_RAW7 0x29 +#define CSI2DC_DT_RAW8 0x2A +#define CSI2DC_DT_RAW10 0x2B +#define CSI2DC_DT_RAW12 0x2C +#define CSI2DC_DT_RAW14 0x2D + +struct csi2dc_format { + u32 mbus_code; + u32 dt; +}; + +static struct csi2dc_format csi2dc_formats_list[] = {constquoted
+ { + .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10, + .dt = CSI2DC_DT_RAW10, + }, +}; + +enum mipi_csi_pads { + CSI2DC_PAD_SINK = 0, + CSI2DC_PAD_SOURCE = 1, + CSI2DC_PADS_NUM = 2, +}; + +struct csi2dc_device { + void __iomem *base; + struct v4l2_subdev csi2dc_sd; + struct device *dev; + struct v4l2_device v4l2_dev; + struct clk *pclk; + struct clk *scck; + + bool video_pipe; + + u32 num_fmts; + struct csi2dc_format **formats; + + struct csi2dc_format *cur_fmt; + struct csi2dc_format *try_fmt; + + struct media_pad pads[CSI2DC_PADS_NUM]; + + bool clk_gated; + u32 inter_line_delay; + u32 vc; + + struct v4l2_async_subdev *asd; + struct v4l2_async_notifier notifier; + + struct v4l2_subdev *input_sd; + bool completed; +}; + +static void csi2dc_vp_update(struct csi2dc_device *csi2dc) +{ + u32 vp; + + vp = CSI2DC_VPCFG_DT(csi2dc->cur_fmt->dt) & CSI2DC_VPCFG_DT_MASK; + vp |= CSI2DC_VPCFG_VC(csi2dc->vc) & CSI2DC_VPCFG_VC_MASK; + vp &= ~CSI2DC_VPCFG_DE; + vp |= CSI2DC_VPCFG_DM(CSI2DC_VPCFG_DM_DECODER8TO12); + vp &= ~CSI2DC_VPCFG_DP2; + vp &= ~CSI2DC_VPCFG_RMS; + vp |= CSI2DC_VPCFG_PA; + + csi2dc_writel(csi2dc, CSI2DC_VPCFG, vp); + csi2dc_writel(csi2dc, CSI2DC_VPE, CSI2DC_VPE_ENABLE); + csi2dc_writel(csi2dc, CSI2DC_PU, CSI2DC_PU_VP); +} + +static inline struct csi2dc_device *csi2dc_sd_to_csi2dc_device + (struct v4l2_subdev *csi2dc_sd)I'd suggest wrapping after the asterisk.quoted
+{ + return container_of(csi2dc_sd, struct csi2dc_device, csi2dc_sd); +} + +static int csi2dc_enum_mbus_code(struct v4l2_subdev *csi2dc_sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_mbus_code_enum *code) +{ + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); + + if (code->index >= csi2dc->num_fmts) + return -EINVAL; + + code->code = csi2dc->formats[code->index]->mbus_code; + return 0; +} + +static int csi2dc_try_fmt(struct v4l2_subdev *csi2dc_sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_format *req_fmt) +{ + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); + struct csi2dc_format *fmt; + + if (!csi2dc->completed) { + dev_dbg((csi2dc)->dev, "subdev not registered yet\n");Uh... but can you end up here?
Yes, if the subdevice has not yet registered. The CSI2DC registers as a subdevice for the next element in pipeline regardless of it's own subdevice. I encountered issues when trying to register the csi2dc at a later point than probe time (at the moment of it's own completion when the subdev is ready). Do you think I need to change , and have csi2dc registers itself for the next element at a later point ?
quoted
+ return 0; + } + + for (fmt = csi2dc->formats[0]; fmt; fmt++) + if (req_fmt->format.code == fmt->mbus_code) + csi2dc->try_fmt = fmt; + + /* in case we could not find the desired format, default to something */ + if (!csi2dc->try_fmt || + req_fmt->format.code != csi2dc->try_fmt->mbus_code) { + csi2dc->try_fmt = csi2dc->formats[0]; + req_fmt->format.code = csi2dc->formats[0]->mbus_code; + } + + return v4l2_subdev_call(csi2dc->input_sd, pad, set_fmt, cfg, req_fmt);For MC-enabled drivers (i.e. also those exposing sub-device nodes) the formats are set (and also enumerated) by the user and validated by the driver. See e.g. ipu3-cio2 driver for an example. It also includes DMA.
To fully understand, I do not need to make a subdev call ? Just validate or invalidate the try format ?
quoted
+} + +static int csi2dc_set_fmt(struct v4l2_subdev *csi2dc_sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_format *req_fmt) +{ + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); + int ret; + + if (!csi2dc->completed) { + dev_dbg((csi2dc)->dev, "subdev not registered yet\n"); + return 0; + } + + csi2dc_try_fmt(csi2dc_sd, cfg, req_fmt); + + if (csi2dc->try_fmt->mbus_code != req_fmt->format.code) { + dev_dbg(csi2dc->dev, "CSI2DC unsupported format 0x%x\n", + req_fmt->format.code); + return -EINVAL; + } + + ret = v4l2_subdev_call(csi2dc->input_sd, pad, set_fmt, cfg, req_fmt); + if (ret) { + dev_err(csi2dc->dev, "input subdev failed %d\n", ret); + return ret; + } + + csi2dc->cur_fmt = csi2dc->try_fmt; + /* update video pipe */ + csi2dc_vp_update(csi2dc); + + dev_dbg(csi2dc->dev, "CSI2DC new format: 0x%x\n", req_fmt->format.code); + return 0; +} + +static int csi2dc_formats_init(struct csi2dc_device *csi2dc) +{ + int i, j; + struct csi2dc_format *fmt = &csi2dc_formats_list[0]; + + csi2dc->num_fmts = 1; + + csi2dc->formats = devm_kcalloc(csi2dc->dev, csi2dc->num_fmts, + sizeof(*csi2dc->formats), GFP_KERNEL); + + for (i = 0, j = 0; i < ARRAY_SIZE(csi2dc_formats_list); i++, fmt++) + if (fmt->mbus_code == MEDIA_BUS_FMT_SRGGB10_1X10) + csi2dc->formats[j++] = fmt; + return 0; +} + +static int csi2dc_s_power(struct v4l2_subdev *csi2dc_sd, int on)Please use runtime PM instead.
And in this case, s_power on/off will be unused? The runtime PM will power on the pipeline when capture is required or it will have it powered on more than that ? Currently in my experiments when stopping the streaming, s_power off is also called afterwards, and the CSI2DC can be reset, as the MIPI lanes must reach a stop state to reinitialize capture later.
quoted
+{ + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); + int ret = 0; + + if (!csi2dc->completed) { + dev_dbg((csi2dc)->dev, "subdev not registered yet\n"); + return 0; + } + + if (on) + ret = clk_prepare_enable(csi2dc->scck); + else + clk_disable_unprepare(csi2dc->scck); + if (ret) + dev_err(csi2dc->dev, "failed to enable scck: %d\n", ret); + + /* if powering up, deassert reset line */ + if (on) + csi2dc_writel(csi2dc, CSI2DC_GCTLR, CSI2DC_GCTLR_SWRST); + + ret = v4l2_subdev_call(csi2dc->input_sd, core, s_power, on); + + /* if powering down, assert reset line */ + if (!on) + csi2dc_writel(csi2dc, CSI2DC_GCTLR, !CSI2DC_GCTLR_SWRST); + + return ret; +} + +static int csi2dc_s_stream(struct v4l2_subdev *csi2dc_sd, int enable) +{ + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); + + if (!csi2dc->completed) { + dev_dbg((csi2dc)->dev, "subdev not registered yet\n"); + return 0; + } + + return v4l2_subdev_call(csi2dc->input_sd, video, s_stream, enable); +} + +static int csi2dc_g_frame_interval(struct v4l2_subdev *csi2dc_sd, + struct v4l2_subdev_frame_interval *interval) +{ + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); + + if (!csi2dc->completed) { + dev_dbg((csi2dc)->dev, "subdev not registered yet\n"); + return 0; + } + + return v4l2_subdev_call(csi2dc->input_sd, video, g_frame_interval, + interval); +} + +static int csi2dc_s_frame_interval(struct v4l2_subdev *csi2dc_sd, + struct v4l2_subdev_frame_interval *interval) +{ + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); + + if (!csi2dc->completed) { + dev_dbg((csi2dc)->dev, "subdev not registered yet\n"); + return 0; + } + + return v4l2_subdev_call(csi2dc->input_sd, video, s_frame_interval, + interval); +} + +static int csi2dc_enum_frame_size(struct v4l2_subdev *csi2dc_sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_frame_size_enum *fse) +{ + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); + + if (!csi2dc->completed) { + dev_dbg((csi2dc)->dev, "subdev not registered yet\n"); + return 0; + } + + return v4l2_subdev_call(csi2dc->input_sd, pad, enum_frame_size, cfg, + fse); +} + +static int csi2dc_enum_frame_interval(struct v4l2_subdev *csi2dc_sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_frame_interval_enum *fie) +{ + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); + + if (!csi2dc->completed) { + dev_dbg((csi2dc)->dev, "subdev not registered yet\n"); + return 0; + } + + return v4l2_subdev_call(csi2dc->input_sd, pad, enum_frame_interval, cfg, + fie); +} + +static struct v4l2_subdev_core_ops csi2dc_core_ops = { + .s_power = csi2dc_s_power, +}; + +static struct v4l2_subdev_pad_ops csi2dc_pad_ops = { + .enum_mbus_code = csi2dc_enum_mbus_code, + .set_fmt = csi2dc_set_fmt, + .enum_frame_size = csi2dc_enum_frame_size, + .enum_frame_interval = csi2dc_enum_frame_interval, +}; + +static struct v4l2_subdev_video_ops csi2dc_video_ops = { + .s_stream = csi2dc_s_stream, + .g_frame_interval = csi2dc_g_frame_interval, + .s_frame_interval = csi2dc_s_frame_interval, +}; + +static struct v4l2_subdev_ops csi2dc_subdev_ops = { + .core = &csi2dc_core_ops, + .pad = &csi2dc_pad_ops, + .video = &csi2dc_video_ops, +};const. Applies to all above.quoted
+ +static int csi2dc_async_bound(struct v4l2_async_notifier *notifier, + struct v4l2_subdev *subdev, + struct v4l2_async_subdev *asd) +{ + struct csi2dc_device *csi2dc = container_of(notifier->v4l2_dev, + struct csi2dc_device, v4l2_dev); + csi2dc->input_sd = subdev; + + return 0; +} + +static int csi2dc_async_complete(struct v4l2_async_notifier *notifier) +{ + struct csi2dc_device *csi2dc = container_of(notifier->v4l2_dev,It'd be cleaner to wrap after "=". Or align the lines below after "(".quoted
+ struct csi2dc_device, v4l2_dev); + int ret; + + ret = v4l2_device_register_subdev_nodes(&csi2dc->v4l2_dev); + if (ret < 0) { + v4l2_err(&csi2dc->v4l2_dev, "failed to register subdev nodes\n"); + return ret; + } + + csi2dc_writel(csi2dc, CSI2DC_GCFG, + (CSI2DC_GCFG_HLC(csi2dc->inter_line_delay) & + CSI2DC_GCFG_HLC_MASK) | + (csi2dc->clk_gated ? 0 : CSI2DC_GCFG_MIPIFRN)); + + csi2dc_writel(csi2dc, CSI2DC_VPCOL, + CSI2DC_VPCOL_COL(0xFFF) & CSI2DC_VPCOL_COL_MASK); + csi2dc_writel(csi2dc, CSI2DC_VPROW, + CSI2DC_VPROW_ROW(0xFFF) & CSI2DC_VPROW_ROW_MASK); + + csi2dc->completed = true; + + return ret; +} + +static const struct v4l2_async_notifier_operations csi2dc_async_ops = { + .bound = csi2dc_async_bound, + .complete = csi2dc_async_complete, +}; + +static void csi2dc_cleanup_notifier(struct csi2dc_device *csi2dc) +{ + v4l2_async_notifier_unregister(&csi2dc->notifier); + v4l2_async_notifier_cleanup(&csi2dc->notifier); +} + +static int csi2dc_prepare_notifier(struct csi2dc_device *csi2dc, + struct device_node *input_parent) +{ + int ret; + + v4l2_async_notifier_init(&csi2dc->notifier); + + csi2dc->asd = kzalloc(sizeof(*csi2dc->asd), GFP_KERNEL); + if (!csi2dc->asd) + return -ENOMEM; + + csi2dc->asd->match_type = V4L2_ASYNC_MATCH_FWNODE; + csi2dc->asd->match.fwnode = of_fwnode_handle(input_parent); + + ret = v4l2_async_notifier_add_subdev(&csi2dc->notifier, csi2dc->asd); + if (ret) { + dev_err(csi2dc->dev, "failed to add async notifier.\n"); + v4l2_async_notifier_cleanup(&csi2dc->notifier); + goto csi2dc_prepare_notifier_err; + } + + csi2dc->notifier.ops = &csi2dc_async_ops; + + ret = v4l2_async_notifier_register(&csi2dc->v4l2_dev, + &csi2dc->notifier); + + if (ret) { + dev_err(csi2dc->dev, "fail to register async notifier.\n"); + goto csi2dc_prepare_notifier_err; + } + +csi2dc_prepare_notifier_err: + of_node_put(input_parent); + + return ret; +} + +static int csi2dc_of_parse(struct csi2dc_device *csi2dc, + struct device_node *of_node) +{ + struct device_node *input_node, *sink_node, *input_parent; + struct v4l2_fwnode_endpoint input_endpoint, sink_endpoint; + int ret; + + memset(&input_endpoint, 0, sizeof(input_endpoint)); + memset(&sink_endpoint, 0, sizeof(sink_endpoint));Could you assign a field to 0 in the declaration instead? You should also set the bus type as I believe the driver supports a single one at both endpoints.
I thought about that, but on the input pad, what bus type I can use ? Synopsys IDI is not available in kernel.
You could also use the fwnode property API instead so you could avoid of_fwnode_handle() --- up to you.quoted
+ + input_node = of_graph_get_next_endpoint(of_node, NULL); + + if (!input_node) { + dev_err(csi2dc->dev, "missing port node at %s, input node is mandatory.\n", + of_node->full_name); + return -EINVAL; + } + + ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(input_node), + &input_endpoint); + + if (ret) { + dev_err(csi2dc->dev, "endpoint not defined at %s\n", + of_node->full_name); + return ret; + } + + input_parent = of_graph_get_remote_port_parent(input_node); + if (!input_parent) { + dev_err(csi2dc->dev, "could not get input node's parent node.\n"); + return -EINVAL; + } + + sink_node = of_graph_get_next_endpoint(of_node, input_node);You'll need to obtain the endpoints by port here (and above, too).
I obtain them in the order in the device tree, one by one. You suggest to use a different API such that they are obtained by port number ?
quoted
+ + if (sink_node) + ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(sink_node), + &sink_endpoint); + + if (!sink_node || ret) { + dev_info(csi2dc->dev, "missing sink node at %s, data pipe available only.\n", + of_node->full_name); + } else { + csi2dc->video_pipe = true; + csi2dc->vc = sink_endpoint.base.id; + + dev_dbg(csi2dc->dev, "block %s %d.%d->%d.%d video pipe VC %d\n", + of_node->full_name, input_endpoint.base.port, + input_endpoint.base.id, sink_endpoint.base.port, + sink_endpoint.base.id, csi2dc->vc); + } + + csi2dc->clk_gated = of_property_read_bool(of_node, + "microchip,clk-gated"); + + dev_dbg(csi2dc->dev, "%s clock\n", + (csi2dc->clk_gated ? "Gated" : "Free running"));Extra parentheses.quoted
+ + ret = of_property_read_u32(of_node, "microchip,inter-line-delay", + &csi2dc->inter_line_delay); + + if (ret || csi2dc->inter_line_delay > 16) { + dev_dbg(csi2dc->dev, "assuming inter line delay = 16 clocks"); + csi2dc->inter_line_delay = 16; + ret = 0; + } + /* hardware automatically adds 1 */ + csi2dc->inter_line_delay--;What happens if 0 was read from DT?quoted
+ + /* prepare async notifier for subdevice completion */ + + if (sink_node) + of_node_put(sink_node);of_node_put() is NULL-safe.quoted
+ of_node_put(input_node); + + return csi2dc_prepare_notifier(csi2dc, input_parent); +} + +static int csi2dc_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct csi2dc_device *csi2dc; + struct resource *res = NULL; + int ret = 0; + + csi2dc = devm_kzalloc(dev, sizeof(*csi2dc), GFP_KERNEL); + if (!csi2dc) + return -ENOMEM; + + csi2dc->dev = dev; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; + + csi2dc->base = devm_ioremap_resource(dev, res); + + if (IS_ERR(csi2dc->base)) { + dev_err(dev, "base address not set\n"); + return PTR_ERR(csi2dc->base); + } + + csi2dc->pclk = devm_clk_get(dev, "pclk"); + if (IS_ERR(csi2dc->pclk)) { + ret = PTR_ERR(csi2dc->pclk); + dev_err(dev, "failed to get pclk: %d\n", ret); + return ret; + } + + ret = clk_prepare_enable(csi2dc->pclk); + if (ret) { + dev_err(dev, "failed to enable pclk: %d\n", ret); + return ret; + } + + csi2dc->scck = devm_clk_get(dev, "scck"); + if (IS_ERR(csi2dc->scck)) { + ret = PTR_ERR(csi2dc->scck); + dev_err(dev, "failed to get scck: %d\n", ret); + goto csi2dc_clk_fail; + } + + ret = v4l2_device_register(dev, &csi2dc->v4l2_dev); + if (ret) { + dev_err(dev, "unable to register v4l2 device.\n"); + goto csi2dc_clk_fail; + } + + v4l2_subdev_init(&csi2dc->csi2dc_sd, &csi2dc_subdev_ops); + + csi2dc->csi2dc_sd.owner = THIS_MODULE; +No need for a newline.quoted
+ csi2dc->csi2dc_sd.dev = dev; + snprintf(csi2dc->csi2dc_sd.name, sizeof(csi2dc->csi2dc_sd.name), + "%s.%d", "CSI2DC", 0);The number is always zero.quoted
+ + csi2dc->csi2dc_sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + csi2dc->csi2dc_sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE; + csi2dc->pads[CSI2DC_PAD_SINK].flags = MEDIA_PAD_FL_SINK; + csi2dc->pads[CSI2DC_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE; + + ret = media_entity_pads_init(&csi2dc->csi2dc_sd.entity, CSI2DC_PADS_NUM, + csi2dc->pads); + if (ret < 0) { + dev_err(dev, "media entity init failed\n"); + goto csi2dc_probe_entity_err; + } + + v4l2_set_subdevdata(&csi2dc->csi2dc_sd, pdev); + + platform_set_drvdata(pdev, &csi2dc->csi2dc_sd); + + ret = csi2dc_of_parse(csi2dc, dev->of_node); + if (ret) + goto csi2dc_probe_entity_err; + + ret = csi2dc_formats_init(csi2dc); + if (ret) + goto csi2dc_probe_error; + + ret = v4l2_async_register_subdev(&csi2dc->csi2dc_sd); + if (ret) { // ret = -EPROBE_DEFER;Oops.quoted
+ goto csi2dc_probe_error; + } + + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + pm_request_idle(dev); + + pr_info("Microchip CSI2DC version %x\n", + csi2dc_readl(csi2dc, CSI2DC_VERSION));What happens if the device was powered down already?
I am moving the info line before pm_request_idle, if this was what you had in mind. I am sending a v3 of the patch, with some of the comments addressed. Some others I do not fully understand yet how to change. Hopefully your replies will help me find the way. Thanks again, Eugen
quoted
+ + return 0; + +csi2dc_probe_error: + v4l2_async_unregister_subdev(&csi2dc->csi2dc_sd); + csi2dc_cleanup_notifier(csi2dc); + +csi2dc_probe_entity_err: + media_entity_cleanup(&csi2dc->csi2dc_sd.entity); + v4l2_device_unregister(&csi2dc->v4l2_dev); +csi2dc_clk_fail: + clk_disable_unprepare(csi2dc->pclk); + return ret; +} + +static int csi2dc_remove(struct platform_device *pdev) +{ + struct v4l2_subdev *csi2dc_sd = platform_get_drvdata(pdev); + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); + + pm_runtime_disable(&pdev->dev); + csi2dc_cleanup_notifier(csi2dc); + media_entity_cleanup(&csi2dc->csi2dc_sd.entity); + v4l2_device_unregister(&csi2dc->v4l2_dev); + clk_disable_unprepare(csi2dc->pclk); + + return 0; +} + +static int __maybe_unused csi2dc_runtime_suspend(struct device *dev) +{ + return 0; +} + +static int __maybe_unused csi2dc_runtime_resume(struct device *dev) +{ + return 0; +} + +static const struct dev_pm_ops csi2dc_dev_pm_ops = { + SET_RUNTIME_PM_OPS(csi2dc_runtime_suspend, csi2dc_runtime_resume, NULL) +}; + +static const struct of_device_id csi2dc_of_match[] = { + { .compatible = "microchip,sama7g5-csi2dc" }, + { } +}; + +MODULE_DEVICE_TABLE(of, csi2dc_of_match); + +static struct platform_driver csi2dc_driver = { + .probe = csi2dc_probe, + .remove = csi2dc_remove, + .driver = { + .name = "microchip-csi2dc", + .pm = &csi2dc_dev_pm_ops, + .of_match_table = of_match_ptr(csi2dc_of_match), + }, +}; + +module_platform_driver(csi2dc_driver); + +MODULE_AUTHOR("Eugen Hristev [off-list ref]"); +MODULE_DESCRIPTION("Microchip CSI2 Demux Controller driver"); +MODULE_LICENSE("GPL v2"); +MODULE_SUPPORTED_DEVICE("video");-- Kind regards, Sakari Ailus
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel