* Re: [PATCH v6 0/5] drm: sun8i: Add DE2 HDMI video support
From: Jean-Francois Moine @ 2016-11-21 18:14 UTC (permalink / raw)
To: Ondřej Jirman
Cc: moinejf-Re5JQEeQqe8AvxtiuMwx3w, Dave Airlie, Maxime Ripard,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <1c9a02ff-ce6c-7ad7-36fa-8a2ea0b7675e-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org>
On Mon, 21 Nov 2016 01:54:53 +0100
Ondřej Jirman <megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org> wrote:
> Dne 20.11.2016 v 12:32 Jean-Francois Moine napsal(a):
> > This patchset series adds HDMI video support to the Allwinner
> > sun8i SoCs which include the display engine 2 (DE2).
> > The driver contains the code for the A83T and H3, but it could be
> > used/extended for other SoCs as the A64, H2 and H5.
>
> Hi,
>
> I'm trying to test your patches on Orange Pi PC, and I've run into a few
> issues: (I'm using sunxi-ng with the same patches as last time, to make
> it work with your driver)
>
> 1] I just get pink output on the monitor - there's some signal, but it's
> pink (or more like magenta).
>
> dmesg ouput indicates no error:
>
> [ 1.887823] [drm] Initialized
> [ 1.888503] sun8i-de2 1000000.de-controller: bound
> 1c0c000.lcd-controller (ops 0xc0a63894)
> [ 2.057298] sun8i-de2 1000000.de-controller: bound 1ee0000.hdmi (ops
> 0xc0a63b54)
> [ 2.057304] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [ 2.057307] [drm] No driver support for vblank timestamp query.
> [ 2.690862] Console: switching to colour frame buffer device 240x67
> [ 2.723059] sun8i-de2 1000000.de-controller: fb0: frame buffer device
[snip]
My H3 boards work correctly, except the Orange PI 2 when it cannot read
the EDID (but it is OK after reboot).
Did you check if the EDID was correctly read?
Which resolution do you expect?
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH 2/2] Add basic support for DW CSI-2 Host IPK
From: Ramiro Oliveira @ 2016-11-21 18:13 UTC (permalink / raw)
To: Sakari Ailus, Ramiro Oliveira, g
Cc: robh+dt, mark.rutland, mchehab, devicetree, linux-kernel,
linux-media, davem, gregkh, geert+renesas, akpm, linux, hverkuil,
laurent.pinchart+renesas, arnd, sudipm.mukherjee, tiffany.lin,
minghsiu.tsai, jean-christophe.trotin, andrew-ct.chen,
simon.horman, songjun.wu, bparrot, CARLOS.PALMINHA
In-Reply-To: <20161117163426.GA13965@valkosipuli.retiisi.org.uk>
Hi Sakari,
Thank you for your feedback.
On 11/17/2016 4:34 PM, Sakari Ailus wrote:
> Hi Ramiro,
>
> Thank you for the patchset. Please see my comments below.
>
> If you've got a CSI-2 receiver, why do you use the DV timing presets? What
> do you use those for?
>
Our CSI-2 Host has two different output types, one that doesn't need any extra
info about the video timings, and one that needs some info about the input video
timings. Since the video timings of any input (Camera) should be somewhat
similar to the DV timing presets I decided to use them.
I'm not sure if this answers your question.
> On Mon, Nov 14, 2016 at 02:20:23PM +0000, Ramiro Oliveira wrote:
>> Add support for basic configuration of the DW CSI-2 Host IPK
>>
>> Signed-off-by: Ramiro Oliveira <roliveir@synopsys.com>
>> ---
>> MAINTAINERS | 7 +
>> drivers/media/platform/Kconfig | 1 +
>> drivers/media/platform/Makefile | 2 +
>> drivers/media/platform/dwc/Kconfig | 36 ++
>> drivers/media/platform/dwc/Makefile | 3 +
>> drivers/media/platform/dwc/dw_mipi_csi.c | 687 +++++++++++++++++++++++
>> drivers/media/platform/dwc/dw_mipi_csi.h | 179 ++++++
>> drivers/media/platform/dwc/plat_ipk.c | 835 ++++++++++++++++++++++++++++
>> drivers/media/platform/dwc/plat_ipk.h | 97 ++++
>> drivers/media/platform/dwc/plat_ipk_video.h | 97 ++++
>> drivers/media/platform/dwc/video_device.c | 741 ++++++++++++++++++++++++
>> drivers/media/platform/dwc/video_device.h | 101 ++++
>> 12 files changed, 2786 insertions(+)
>> create mode 100644 drivers/media/platform/dwc/Kconfig
>> create mode 100644 drivers/media/platform/dwc/Makefile
>> create mode 100644 drivers/media/platform/dwc/dw_mipi_csi.c
>> create mode 100644 drivers/media/platform/dwc/dw_mipi_csi.h
>> create mode 100644 drivers/media/platform/dwc/plat_ipk.c
>> create mode 100644 drivers/media/platform/dwc/plat_ipk.h
>> create mode 100644 drivers/media/platform/dwc/plat_ipk_video.h
>> create mode 100644 drivers/media/platform/dwc/video_device.c
>> create mode 100644 drivers/media/platform/dwc/video_device.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 6a71422..f54dfd8 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -10618,6 +10618,13 @@ S: Maintained
>> F: drivers/staging/media/st-cec/
>> F: Documentation/devicetree/bindings/media/stih-cec.txt
>>
>> +SYNOPSYS DESIGNWARE CSI-2 HOST IPK DRIVER
>> +M: Ramiro Oliveira <roliveir@synopsys.com>
>> +L: linux-media@vger.kernel.org
>> +T: git git://linuxtv.org/media_tree.git
>> +S: Maintained
>> +F: drivers/media/platform/dwc/
>> +
>> SYNOPSYS DESIGNWARE DMAC DRIVER
>> M: Viresh Kumar <vireshk@kernel.org>
>> M: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
>> index 754edbf1..6fef760f 100644
>> --- a/drivers/media/platform/Kconfig
>> +++ b/drivers/media/platform/Kconfig
>> @@ -120,6 +120,7 @@ source "drivers/media/platform/am437x/Kconfig"
>> source "drivers/media/platform/xilinx/Kconfig"
>> source "drivers/media/platform/rcar-vin/Kconfig"
>> source "drivers/media/platform/atmel/Kconfig"
>> +source "drivers/media/platform/dwc/Kconfig"
>>
>> config VIDEO_TI_CAL
>> tristate "TI CAL (Camera Adaptation Layer) driver"
>> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
>> index f842933..623f5d2 100644
>> --- a/drivers/media/platform/Makefile
>> +++ b/drivers/media/platform/Makefile
>> @@ -61,6 +61,8 @@ obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar-vin/
>>
>> obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel/
>>
>> +obj-$(CONFIG_VIDEO_DWC) += dwc/
>> +
>> ccflags-y += -I$(srctree)/drivers/media/i2c
>>
>> obj-$(CONFIG_VIDEO_MEDIATEK_VPU) += mtk-vpu/
>> diff --git a/drivers/media/platform/dwc/Kconfig b/drivers/media/platform/dwc/Kconfig
>> new file mode 100644
>> index 0000000..fb8533b
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/Kconfig
>> @@ -0,0 +1,36 @@
>> +config VIDEO_DWC
>> + bool "Designware Cores CSI-2 IPK (EXPERIMENTAL)"
>> + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA
>> + help
>> + Say Y here to enable support for the DesignWare Cores CSI-2 Host IP
>> + Prototyping Kit.
>> +
>> +if VIDEO_DWC
>> +config DWC_PLATFORM
>> + tristate "SNPS DWC MIPI CSI2 Host"
>> + depends on VIDEO_DWC
>> + help
>> + This is the V4L2 plaftorm driver driver for the DWC CSI-2 HOST IPK
>> +
>> + To compile this driver as a module, choose M here
>> +
>> +
>> +config DWC_MIPI_CSI2_HOST
>> + tristate "SNPS DWC MIPI CSI2 Host"
>> + select GENERIC_PHY
>> + depends on VIDEO_DWC
>> + help
>> + This is a V4L2 driver for SNPS DWC MIPI-CSI2
>> +
>> + To compile this driver as a module, choose M here
>> +
>> +config DWC_VIDEO_DEVICE
>> + tristate "DWC VIDEO DEVICE"
>> + select VIDEOBUF2_VMALLOC
>> + select VIDEOBUF2_DMA_CONTIG
>> + depends on VIDEO_DWC
>> + help
>> + This is a V4L2 driver for SNPS Video device
>> + To compile this driver as a module, choose M here
>> +
>> +endif # VIDEO_DWC
>> diff --git a/drivers/media/platform/dwc/Makefile b/drivers/media/platform/dwc/Makefile
>> new file mode 100644
>> index 0000000..75c74b7
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/Makefile
>> @@ -0,0 +1,3 @@
>> +obj-$(CONFIG_DWC_PLATFORM) += plat_ipk.o
>> +obj-$(CONFIG_DWC_MIPI_CSI2_HOST) += dw_mipi_csi.o
>> +obj-$(CONFIG_DWC_VIDEO_DEVICE) += video_device.o
>> diff --git a/drivers/media/platform/dwc/dw_mipi_csi.c b/drivers/media/platform/dwc/dw_mipi_csi.c
>> new file mode 100644
>> index 0000000..1337a5e
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/dw_mipi_csi.c
>> @@ -0,0 +1,687 @@
>> +/*
>> + * DWC MIPI CSI-2 Host device driver
>> + *
>> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
>> + * Author: Ramiro Oliveira <ramiro.oliveira@synopsys.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published
>> + * by the Free Software Foundation, either version 2 of the License,
>> + * or (at your option) any later version.
>> + */
>> +
>> +
>> +#include "dw_mipi_csi.h"
>> +
>> +/** @short Driver args: debug */
>> +static unsigned int debug;
>> +module_param(debug, uint, 0644);
>> +MODULE_PARM_DESC(debug, "Activates debug info");
>
> There should be no need for module specific debug parameters. Please use
> dynamic debug instead.
>
I'll remove it.
>> +
>> +/*
>> + * Basic IO read and write operations
>> + */
>> +
>> +/**
>> + * @short Video formats supported by the MIPI CSI-2
>> + */
>> +static const struct mipi_fmt dw_mipi_csi_formats[] = {
>> + {
>> + .name = "RAW BGGR 8",
>
> Do you use the name field for something? If not, I'd suggest to drop it.
>
No, I guess I don't really use it for anything. I'll remove it.
>> + .code = MEDIA_BUS_FMT_SBGGR8_1X8,
>> + .depth = 8,
>> + },
>> + {
>> + .name = "RAW10",
>> + .code = MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE,
>> + .depth = 10,
>> + },
>> + {
>> + .name = "RGB565",
>> + .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
>> + .depth = 16,
>> + },
>> + {
>> + .name = "BGR565",
>> + .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
>> + .depth = 16,
>> + },
>> + {
>> + .name = "RGB888",
>> + .code = MEDIA_BUS_FMT_RGB888_2X12_LE,
>> + .depth = 24,
>> + },
>> + {
>> + .name = "BGR888",
>> + .code = MEDIA_BUS_FMT_RGB888_2X12_BE,
>> + .depth = 24,
>> + },
>> +};
>> +
>> +static struct mipi_csi_dev *
>> +sd_to_mipi_csi_dev(struct v4l2_subdev *sdev)
>> +{
>> + return container_of(sdev, struct mipi_csi_dev, sd);
>> +}
>> +
>> +void
>> +dw_mipi_csi_write(struct mipi_csi_dev *dev,
>> + unsigned int address, unsigned int data)
>> +{
>> + iowrite32(data, dev->base_address + address);
>> +}
>> +
>> +u32
>> +dw_mipi_csi_read(struct mipi_csi_dev *dev, unsigned long address)
>> +{
>> + return ioread32(dev->base_address + address);
>> +}
>> +
>> +void
>> +dw_mipi_csi_write_part(struct mipi_csi_dev *dev,
>> + unsigned long address, unsigned long data,
>> + unsigned char shift, unsigned char width)
>> +{
>> + u32 mask = (1 << width) - 1;
>> + u32 temp = dw_mipi_csi_read(dev, address);
>> +
>> + temp &= ~(mask << shift);
>> + temp |= (data & mask) << shift;
>> + dw_mipi_csi_write(dev, address, temp);
>> +}
>> +
>> +static const struct mipi_fmt *
>> +find_dw_mipi_csi_format(struct v4l2_mbus_framefmt *mf)
>> +{
>> + int i;
>
> unsigned int
>
I'll change it.
>> +
>> + for (i = 0; i < ARRAY_SIZE(dw_mipi_csi_formats); i++)
>> + if (mf->code == dw_mipi_csi_formats[i].code)
>> + return &dw_mipi_csi_formats[i];
>> + return NULL;
>> +}
>> +
>> +int
>> +enable_video_output(struct mipi_csi_dev *dev)
>> +{
>> + return 0;
>> +}
>
> This one is unused.
>
>> +
>> +int
>> +disable_video_output(struct mipi_csi_dev *dev)
>> +{
>> + phy_power_off(dev->phy);
>> + return 0;
>> +}
>
> This one as well.
>
I'll remove both.
>> +
>> +static void
>> +dw_mipi_csi_reset(struct mipi_csi_dev *dev)
>> +{
>> + dw_mipi_csi_write(dev, R_CSI2_CTRL_RESETN, 0);
>> + /* mdelay(1); */
>> + dw_mipi_csi_write(dev, R_CSI2_CTRL_RESETN, 1);
>> +}
>> +
>> +static int
>> +dw_mipi_csi_mask_irq_power_off(struct mipi_csi_dev *dev)
>> +{
>> + /* set only one lane (lane 0) as active (ON) */
>> + dw_mipi_csi_write(dev, R_CSI2_N_LANES, 0);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY_FATAL, 0);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT_FATAL, 0);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_FRAME_FATAL, 0);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY, 0);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT, 0);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_LINE, 0);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_IPI, 0);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_CTRL_RESETN, 0);
>> +
>> + return 0;
>> +
>> +}
>> +
>> +static int
>> +dw_mipi_csi_hw_stdby(struct mipi_csi_dev *dev)
>> +{
>> + /* set only one lane (lane 0) as active (ON) */
>> + dw_mipi_csi_reset(dev);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_N_LANES, 0);
>> +
>> + phy_init(dev->phy);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY_FATAL, 0xFFFFFFFF);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT_FATAL, 0xFFFFFFFF);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_FRAME_FATAL, 0xFFFFFFFF);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY, 0xFFFFFFFF);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT, 0xFFFFFFFF);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_LINE, 0xFFFFFFFF);
>> + dw_mipi_csi_write(dev, R_CSI2_MASK_INT_IPI, 0xFFFFFFFF);
>> +
>> + return 0;
>> +
>> +}
>> +
>> +void
>
> Shouldn't this be static? The same applies to many other symbols as well.
>
You're right, I'll change them all.
>> +dw_mipi_csi_set_ipi_fmt(struct mipi_csi_dev *dev)
>> +{
>> + switch (dev->fmt->code) {
>> + case MEDIA_BUS_FMT_RGB565_2X8_BE:
>> + case MEDIA_BUS_FMT_RGB565_2X8_LE:
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RGB565);
>> + v4l2_dbg(1, debug, &dev->sd, "DT: RGB 565");
>> + break;
>> +
>> + case MEDIA_BUS_FMT_RGB888_2X12_LE:
>> + case MEDIA_BUS_FMT_RGB888_2X12_BE:
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RGB888);
>> + v4l2_dbg(1, debug, &dev->sd, "DT: RGB 888");
>> + break;
>> + case MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE:
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RAW10);
>> + v4l2_dbg(1, debug, &dev->sd, "DT: RAW 10");
>> + break;
>> + case MEDIA_BUS_FMT_SBGGR8_1X8:
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RAW8);
>> + v4l2_dbg(1, debug, &dev->sd, "DT: RAW 8");
>> + break;
>> + default:
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RGB565);
>> + v4l2_dbg(1, debug, &dev->sd, "Error");
>> + break;
>> + }
>> +}
>> +
>> +void
>> +__dw_mipi_csi_fill_timings(struct mipi_csi_dev *dev,
>> + const struct v4l2_bt_timings *bt)
>> +{
>> +
>> + if (bt == NULL)
>> + return;
>> +
>> + dev->hw.hsa = bt->hsync;
>> + dev->hw.hbp = bt->hbackporch;
>> + dev->hw.hsd = bt->hsync;
>> + dev->hw.htotal = bt->height + bt->vfrontporch +
>> + bt->vsync + bt->vbackporch;
>> + dev->hw.vsa = bt->vsync;
>> + dev->hw.vbp = bt->vbackporch;
>> + dev->hw.vfp = bt->vfrontporch;
>> + dev->hw.vactive = bt->height;
>> +}
>
> There seem to be quite a few unused functions here.
>
I've removed everything that wasn't being used.
>> +
>> +void
>> +dw_mipi_csi_start(struct mipi_csi_dev *dev)
>> +{
>> + const struct v4l2_bt_timings *bt = &v4l2_dv_timings_presets[0].bt;
>> +
>> + __dw_mipi_csi_fill_timings(dev, bt);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_N_LANES, (dev->hw.num_lanes - 1));
>> + v4l2_dbg(1, debug, &dev->sd, "N Lanes: %d\n", dev->hw.num_lanes);
>> +
>> + /*IPI Related Configuration */
>> + if ((dev->hw.output_type == IPI_OUT)
>> + || (dev->hw.output_type == BOTH_OUT)) {
>> +
>> + dw_mipi_csi_write_part(dev, R_CSI2_IPI_MODE, dev->hw.ipi_mode,
>> + 0, 1);
>> + v4l2_dbg(1, debug, &dev->sd, "IPI MODE: %d\n",
>> + dev->hw.ipi_mode);
>> +
>> + dw_mipi_csi_write_part(dev, R_CSI2_IPI_MODE,
>> + dev->hw.ipi_color_mode, 8, 1);
>> + v4l2_dbg(1, debug, &dev->sd, "Color Mode: %d\n",
>> + dev->hw.ipi_color_mode);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_VCID, dev->hw.virtual_ch);
>> + v4l2_dbg(1, debug, &dev->sd, "Virtual Channel: %d\n",
>> + dev->hw.virtual_ch);
>> +
>> + dw_mipi_csi_write_part(dev, R_CSI2_IPI_MEM_FLUSH,
>> + dev->hw.ipi_auto_flush, 8, 1);
>> + v4l2_dbg(1, debug, &dev->sd, "Auto-flush: %d\n",
>> + dev->hw.ipi_auto_flush);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_HSA_TIME, dev->hw.hsa);
>> + v4l2_dbg(1, debug, &dev->sd, "HSA: %d\n", dev->hw.hsa);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_HBP_TIME, dev->hw.hbp);
>> + v4l2_dbg(1, debug, &dev->sd, "HBP: %d\n", dev->hw.hbp);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_HSD_TIME, dev->hw.hsd);
>> + v4l2_dbg(1, debug, &dev->sd, "HSD: %d\n", dev->hw.hsd);
>> +
>> + if (dev->hw.ipi_mode == AUTO_TIMING) {
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_HLINE_TIME,
>> + dev->hw.htotal);
>> + v4l2_dbg(1, debug, &dev->sd, "H total: %d\n",
>> + dev->hw.htotal);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_VSA_LINES,
>> + dev->hw.vsa);
>> + v4l2_dbg(1, debug, &dev->sd, "VSA: %d\n", dev->hw.vsa);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_VBP_LINES,
>> + dev->hw.vbp);
>> + v4l2_dbg(1, debug, &dev->sd, "VBP: %d\n", dev->hw.vbp);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_VFP_LINES,
>> + dev->hw.vfp);
>> + v4l2_dbg(1, debug, &dev->sd, "VFP: %d\n", dev->hw.vfp);
>> +
>> + dw_mipi_csi_write(dev, R_CSI2_IPI_VACTIVE_LINES,
>> + dev->hw.vactive);
>> + v4l2_dbg(1, debug, &dev->sd, "V Active: %d\n",
>> + dev->hw.vactive);
>> + }
>> + }
>> +
>> + phy_power_on(dev->phy);
>> +}
>> +
>> +static int
>> +dw_mipi_csi_enum_mbus_code(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_mbus_code_enum *code)
>> +{
>> + if (code->index >= ARRAY_SIZE(dw_mipi_csi_formats))
>> + return -EINVAL;
>> +
>> + code->code = dw_mipi_csi_formats[code->index].code;
>> + return 0;
>> +}
>> +
>> +static struct mipi_fmt const *
>> +dw_mipi_csi_try_format(struct v4l2_mbus_framefmt *mf)
>> +{
>> + struct mipi_fmt const *fmt;
>> +
>> + fmt = find_dw_mipi_csi_format(mf);
>> + if (fmt == NULL)
>> + fmt = &dw_mipi_csi_formats[0];
>> +
>> + mf->code = fmt->code;
>> + return fmt;
>> +}
>> +
>> +static struct v4l2_mbus_framefmt *
>> +__dw_mipi_csi_get_format(struct mipi_csi_dev *dev,
>> + struct v4l2_subdev_pad_config *cfg,
>> + enum v4l2_subdev_format_whence which)
>> +{
>> + if (which == V4L2_SUBDEV_FORMAT_TRY)
>> + return cfg ? v4l2_subdev_get_try_format(&dev->sd, cfg,
>> + 0) : NULL;
>> +
>> + return &dev->format;
>> +}
>> +
>> +static int
>> +dw_mipi_csi_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *fmt)
>> +{
>> + struct mipi_csi_dev *dev = sd_to_mipi_csi_dev(sd);
>> + struct mipi_fmt const *dev_fmt;
>> + struct v4l2_mbus_framefmt *mf;
>> + int i = 0;
>
> unsigned int
>
Changing it.
>> + const struct v4l2_bt_timings *bt_r = &v4l2_dv_timings_presets[0].bt;
>> +
>> + mf = __dw_mipi_csi_get_format(dev, cfg, fmt->which);
>> +
>> + dev_fmt = dw_mipi_csi_try_format(&fmt->format);
>> + if (dev_fmt) {
>> + *mf = fmt->format;
>> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>> + dev->fmt = dev_fmt;
>> + dw_mipi_csi_set_ipi_fmt(dev);
>> + }
>> + while (v4l2_dv_timings_presets[i].bt.width) {
>> + const struct v4l2_bt_timings *bt =
>> + &v4l2_dv_timings_presets[i].bt;
>> + if (mf->width == bt->width && mf->height == bt->width) {
>> + __dw_mipi_csi_fill_timings(dev, bt);
>> + return 0;
>> + }
>> + i++;
>> + }
>> +
>> + __dw_mipi_csi_fill_timings(dev, bt_r);
>> + return 0;
>> +
>> +}
>> +
>> +static int
>> +dw_mipi_csi_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *fmt)
>> +{
>> + struct mipi_csi_dev *dev = sd_to_mipi_csi_dev(sd);
>> + struct v4l2_mbus_framefmt *mf;
>> +
>> + mf = __dw_mipi_csi_get_format(dev, cfg, fmt->which);
>> + if (!mf)
>> + return -EINVAL;
>> +
>> + mutex_lock(&dev->lock);
>> + fmt->format = *mf;
>> + mutex_unlock(&dev->lock);
>> + return 0;
>> +}
>> +
>> +static int
>> +dw_mipi_csi_s_power(struct v4l2_subdev *sd, int on)
>> +{
>> + struct mipi_csi_dev *dev = sd_to_mipi_csi_dev(sd);
>> +
>> + if (on) {
>> + dw_mipi_csi_hw_stdby(dev);
>> + dw_mipi_csi_start(dev);
>> + } else {
>> + dw_mipi_csi_mask_irq_power_off(dev);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +dw_mipi_csi_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>> +{
>> + struct v4l2_mbus_framefmt *format =
>> + v4l2_subdev_get_try_format(sd, fh->pad, 0);
>> +
>> + format->colorspace = V4L2_COLORSPACE_SRGB;
>> + format->code = dw_mipi_csi_formats[0].code;
>> + format->width = MIN_WIDTH;
>> + format->height = MIN_HEIGHT;
>> + format->field = V4L2_FIELD_NONE;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct v4l2_subdev_internal_ops dw_mipi_csi_sd_internal_ops = {
>> + .open = dw_mipi_csi_open,
>> +};
>> +
>> +static struct v4l2_subdev_core_ops dw_mipi_csi_core_ops = {
>> + .s_power = dw_mipi_csi_s_power,
>> +};
>> +
>> +static struct v4l2_subdev_pad_ops dw_mipi_csi_pad_ops = {
>> + .enum_mbus_code = dw_mipi_csi_enum_mbus_code,
>> + .get_fmt = dw_mipi_csi_get_fmt,
>> + .set_fmt = dw_mipi_csi_set_fmt,
>> +};
>> +
>> +static struct v4l2_subdev_ops dw_mipi_csi_subdev_ops = {
>> + .core = &dw_mipi_csi_core_ops,
>> + .pad = &dw_mipi_csi_pad_ops,
>> +};
>> +
>> +static irqreturn_t
>> +dw_mipi_csi_irq1(int irq, void *dev_id)
>> +{
>> +
>> + struct mipi_csi_dev *dev = dev_id;
>> + u32 int_status, ind_status;
>> + unsigned long flags;
>> +
>> + int_status = dw_mipi_csi_read(dev, R_CSI2_INTERRUPT);
>> + spin_lock_irqsave(&dev->slock, flags);
>> +
>> + if (int_status & CSI2_INT_PHY_FATAL) {
>> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PHY_FATAL);
>> + pr_info_ratelimited("%08X CSI INT PHY FATAL: %08X\n",
>> + (uint32_t) dev->base_address, ind_status);
>
> I don't think you should use pr_info() for interrupts. This is basically
> debug information, isn't it?
>
Yes. It's useful in testing, but not so much in production. Changing it to debug.
>> + }
>> +
>> + if (int_status & CSI2_INT_PKT_FATAL) {
>> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PKT_FATAL);
>> + pr_info_ratelimited("%08X CSI INT PKT FATAL: %08X\n",
>> + (uint32_t) dev->base_address, ind_status);
>> + }
>> +
>> + if (int_status & CSI2_INT_FRAME_FATAL) {
>> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_FRAME_FATAL);
>> + pr_info_ratelimited("%08X CSI INT FRAME FATAL: %08X\n",
>> + (uint32_t) dev->base_address, ind_status);
>> + }
>> +
>> + if (int_status & CSI2_INT_PHY) {
>> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PHY);
>> + pr_info_ratelimited("%08X CSI INT PHY: %08X\n",
>> + (uint32_t) dev->base_address, ind_status);
>> + }
>> +
>> + if (int_status & CSI2_INT_PKT) {
>> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PKT);
>> + pr_info_ratelimited("%08X CSI INT PKT: %08X\n",
>> + (uint32_t) dev->base_address, ind_status);
>> + }
>> +
>> + if (int_status & CSI2_INT_LINE) {
>> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_LINE);
>> + pr_info_ratelimited("%08X CSI INT LINE: %08X\n",
>> + (uint32_t) dev->base_address, ind_status);
>> + }
>> +
>> + if (int_status & CSI2_INT_IPI) {
>> + ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_IPI);
>> + pr_info_ratelimited("%08X CSI INT IPI: %08X\n",
>> + (uint32_t) dev->base_address, ind_status);
>> + }
>> + spin_unlock_irqrestore(&dev->slock, flags);
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int
>> +dw_mipi_csi_parse_dt(struct platform_device *pdev, struct mipi_csi_dev *dev)
>> +{
>> + struct device_node *node = pdev->dev.of_node;
>> + int reg;
>> + int ret = 0;
>> +
>> + /* Device tree information */
>> + ret = of_property_read_u32(node, "data-lanes", &dev->hw.num_lanes);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Couldn't read data-lanes\n");
>> + return ret;
>> + }
>> +
>> + ret = of_property_read_u32(node, "output-type", &dev->hw.output_type);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Couldn't read output-type\n");
>> + return ret;
>> + }
>> +
>> + ret = of_property_read_u32(node, "ipi-mode", &dev->hw.ipi_mode);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Couldn't read ipi-mode\n");
>> + return ret;
>> + }
>> +
>> + ret =
>> + of_property_read_u32(node, "ipi-auto-flush",
>> + &dev->hw.ipi_auto_flush);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Couldn't read ipi-auto-flush\n");
>> + return ret;
>> + }
>> +
>> + ret =
>> + of_property_read_u32(node, "ipi-color-mode",
>> + &dev->hw.ipi_color_mode);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Couldn't read ipi-color-mode\n");
>> + return ret;
>> + }
>> +
>> + ret =
>> + of_property_read_u32(node, "virtual-channel", &dev->hw.virtual_ch);
>
> Is this some thing that should come from DT? Sure, we don't really have
> proper support for virtual channels right now, but could you use zero for
> now?
>
I'm not sure if I'm understanding wha't you're saying.
This info is used by one of our CSI-2 Host outputs to determine which virtual
channel info should be processed.
>> + if (ret) {
>> + dev_err(&pdev->dev, "Couldn't read virtual-channel\n");
>> + return ret;
>> + }
>> +
>> + node = of_get_child_by_name(node, "port");
>> + if (!node)
>> + return -EINVAL;
>> +
>> + ret = of_property_read_u32(node, "reg", ®);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Couldn't read reg value\n");
>> + return ret;
>> + }
>> + dev->index = reg - 1;
>> +
>> + if (dev->index >= CSI_MAX_ENTITIES)
>> + return -ENXIO;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id dw_mipi_csi_of_match[];
>> +
>> +/**
>> + * @short Initialization routine - Entry point of the driver
>> + * @param[in] pdev pointer to the platform device structure
>> + * @return 0 on success and a negative number on failure
>> + * Refer to Linux errors.
>> + */
>> +static int mipi_csi_probe(struct platform_device *pdev)
>> +{
>> + const struct of_device_id *of_id;
>> + struct device *dev = &pdev->dev;
>> + struct resource *res = NULL;
>> + struct mipi_csi_dev *mipi_csi;
>> + int ret = -ENOMEM;
>> +
>> + dev_info(&pdev->dev, "Installing MIPI CSI-2 module\n");
>> +
>> + dev_dbg(&pdev->dev, "Device registration\n");
>
> At least one of these is redundant. I'd just remove both.
>
I'll remove them.
>> + mipi_csi = devm_kzalloc(dev, sizeof(*mipi_csi), GFP_KERNEL);
>> + if (!dev)
>> + return -ENOMEM;
>> +
>> + mutex_init(&mipi_csi->lock);
>> + spin_lock_init(&mipi_csi->slock);
>> + mipi_csi->pdev = pdev;
>> +
>> + of_id = of_match_node(dw_mipi_csi_of_match, dev->of_node);
>> + if (WARN_ON(of_id == NULL))
>> + return -EINVAL;
>> +
>> + ret = dw_mipi_csi_parse_dt(pdev, mipi_csi);
>> + if (ret < 0)
>> + return ret;
>> +
>> + dev_info(dev, "Request DPHY\n");
>> + mipi_csi->phy = devm_phy_get(dev, "csi2-dphy");
>> + if (IS_ERR(mipi_csi->phy))
>> + return PTR_ERR(mipi_csi->phy);
>
> It'd be better to tell what's wrong when there's an error instead of
> printing a series of messages even when all is well.
>
You're right, I'll remove the prints regarding success and add some to inform of
errors.
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + mipi_csi->base_address = devm_ioremap_resource(dev, res);
>> +
>> + if (IS_ERR(mipi_csi->base_address))
>> + return PTR_ERR(mipi_csi->base_address);
>> +
>> + mipi_csi->ctrl_irq_number = platform_get_irq(pdev, 0);
>> + if (mipi_csi->ctrl_irq_number <= 0) {
>> + dev_err(dev, "IRQ number not set.\n");
>> + return mipi_csi->ctrl_irq_number;
>> + }
>> +
>> + ret = devm_request_irq(dev, mipi_csi->ctrl_irq_number,
>> + dw_mipi_csi_irq1, IRQF_SHARED,
>> + dev_name(dev), mipi_csi);
>> + if (ret) {
>> + dev_err(dev, "IRQ failed\n");
>> + goto end;
>> + }
>> +
>> + v4l2_subdev_init(&mipi_csi->sd, &dw_mipi_csi_subdev_ops);
>> + mipi_csi->sd.owner = THIS_MODULE;
>> + snprintf(mipi_csi->sd.name, sizeof(mipi_csi->sd.name), "%s.%d",
>> + CSI_DEVICE_NAME, mipi_csi->index);
>> + mipi_csi->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> + mipi_csi->fmt = &dw_mipi_csi_formats[0];
>> +
>> + mipi_csi->format.code = dw_mipi_csi_formats[0].code;
>> + mipi_csi->format.width = MIN_WIDTH;
>> + mipi_csi->format.height = MIN_HEIGHT;
>> +
>> + mipi_csi->pads[CSI_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
>> + mipi_csi->pads[CSI_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
>> + ret = media_entity_pads_init(&mipi_csi->sd.entity,
>> + CSI_PADS_NUM, mipi_csi->pads);
>> +
>> + if (ret < 0) {
>> + dev_err(dev, "Media Entity init failed\n");
>> + goto entity_cleanup;
>> + }
>> +
>> + /* This allows to retrieve the platform device id by the host driver */
>> + v4l2_set_subdevdata(&mipi_csi->sd, pdev);
>> +
>> + /* .. and a pointer to the subdev. */
>> + platform_set_drvdata(pdev, &mipi_csi->sd);
>> +
>> + dw_mipi_csi_mask_irq_power_off(mipi_csi);
>> + dev_info(dev, "DW MIPI CSI-2 Host registered successfully\n");
>> + return 0;
>> +
>> +entity_cleanup:
>> + media_entity_cleanup(&mipi_csi->sd.entity);
>> +end:
>> + return ret;
>> +}
>> +
>> +/**
>> + * @short Exit routine - Exit point of the driver
>> + * @param[in] pdev pointer to the platform device structure
>> + * @return 0 on success and a negative number on failure
>> + * Refer to Linux errors.
>> + */
>> +static int mipi_csi_remove(struct platform_device *pdev)
>> +{
>> + struct v4l2_subdev *sd = platform_get_drvdata(pdev);
>> + struct mipi_csi_dev *mipi_csi = sd_to_mipi_csi_dev(sd);
>> +
>> + dev_dbg(&pdev->dev, "Removing MIPI CSI-2 module\n");
>> + media_entity_cleanup(&mipi_csi->sd.entity);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short of_device_id structure
>> + */
>> +static const struct of_device_id dw_mipi_csi_of_match[] = {
>> + {
>> + .compatible = "snps,dw-mipi-csi"},
>> + { /* sentinel */ },
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, dw_mipi_csi_of_match);
>> +
>> +/**
>> + * @short Platform driver structure
>> + */
>> +static struct platform_driver __refdata dw_mipi_csi_pdrv = {
>> + .remove = mipi_csi_remove,
>> + .probe = mipi_csi_probe,
>> + .driver = {
>> + .name = CSI_DEVICE_NAME,
>> + .owner = THIS_MODULE,
>> + .of_match_table = dw_mipi_csi_of_match,
>> + },
>> +};
>> +
>> +module_platform_driver(dw_mipi_csi_pdrv);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Ramiro Oliveira <roliveir@synopsys.com>");
>> +MODULE_DESCRIPTION("Synopsys DW MIPI CSI-2 Host driver");
>> diff --git a/drivers/media/platform/dwc/dw_mipi_csi.h b/drivers/media/platform/dwc/dw_mipi_csi.h
>> new file mode 100644
>> index 0000000..5e4c48c
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/dw_mipi_csi.h
>> @@ -0,0 +1,179 @@
>> +/*
>> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef DW_MIPI_CSI_H_
>> +#define DW_MIPI_CSI_H_
>> +
>> +#include <linux/module.h>
>> +#include <linux/errno.h>
>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/delay.h>
>> +#include <linux/wait.h>
>> +#include <linux/string.h>
>> +#include <linux/phy/phy.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-dv-timings.h>
>> +#include <linux/videodev2.h>
>> +#include <linux/io.h>
>
> Please arrange alphabetically. Same for other files.
>
>> +
>> +#include "plat_ipk_video.h"
>> +
>> +#define CSI_DEVICE_NAME "dw-mipi-csi"
>> +
>> +/** @short DWC MIPI CSI-2 register addresses*/
>> +enum register_addresses {
>> + R_CSI2_VERSION = 0x00,
>> + R_CSI2_N_LANES = 0x04,
>> + R_CSI2_CTRL_RESETN = 0x08,
>> + R_CSI2_INTERRUPT = 0x0C,
>> + R_CSI2_DATA_IDS_1 = 0x10,
>> + R_CSI2_DATA_IDS_2 = 0x14,
>> + R_CSI2_IPI_MODE = 0x80,
>> + R_CSI2_IPI_VCID = 0x84,
>> + R_CSI2_IPI_DATA_TYPE = 0x88,
>> + R_CSI2_IPI_MEM_FLUSH = 0x8C,
>> + R_CSI2_IPI_HSA_TIME = 0x90,
>> + R_CSI2_IPI_HBP_TIME = 0x94,
>> + R_CSI2_IPI_HSD_TIME = 0x98,
>> + R_CSI2_IPI_HLINE_TIME = 0x9C,
>> + R_CSI2_IPI_VSA_LINES = 0xB0,
>> + R_CSI2_IPI_VBP_LINES = 0xB4,
>> + R_CSI2_IPI_VFP_LINES = 0xB8,
>> + R_CSI2_IPI_VACTIVE_LINES = 0xBC,
>> + R_CSI2_INT_PHY_FATAL = 0xe0,
>> + R_CSI2_MASK_INT_PHY_FATAL = 0xe4,
>> + R_CSI2_FORCE_INT_PHY_FATAL = 0xe8,
>> + R_CSI2_INT_PKT_FATAL = 0xf0,
>> + R_CSI2_MASK_INT_PKT_FATAL = 0xf4,
>> + R_CSI2_FORCE_INT_PKT_FATAL = 0xf8,
>> + R_CSI2_INT_FRAME_FATAL = 0x100,
>> + R_CSI2_MASK_INT_FRAME_FATAL = 0x104,
>> + R_CSI2_FORCE_INT_FRAME_FATAL = 0x108,
>> + R_CSI2_INT_PHY = 0x110,
>> + R_CSI2_MASK_INT_PHY = 0x114,
>> + R_CSI2_FORCE_INT_PHY = 0x118,
>> + R_CSI2_INT_PKT = 0x120,
>> + R_CSI2_MASK_INT_PKT = 0x124,
>> + R_CSI2_FORCE_INT_PKT = 0x128,
>> + R_CSI2_INT_LINE = 0x130,
>> + R_CSI2_MASK_INT_LINE = 0x134,
>> + R_CSI2_FORCE_INT_LINE = 0x138,
>> + R_CSI2_INT_IPI = 0x140,
>> + R_CSI2_MASK_INT_IPI = 0x144,
>> + R_CSI2_FORCE_INT_IPI = 0x148
>> +};
>> +
>> +/** @short IPI Data Types */
>> +enum data_type {
>> + CSI_2_YUV420_8 = 0x18,
>> + CSI_2_YUV420_10 = 0x19,
>> + CSI_2_YUV420_8_LEG = 0x1A,
>> + CSI_2_YUV420_8_SHIFT = 0x1C,
>> + CSI_2_YUV420_10_SHIFT = 0x1D,
>> + CSI_2_YUV422_8 = 0x1E,
>> + CSI_2_YUV422_10 = 0x1F,
>> + CSI_2_RGB444 = 0x20,
>> + CSI_2_RGB555 = 0x21,
>> + CSI_2_RGB565 = 0x22,
>> + CSI_2_RGB666 = 0x23,
>> + CSI_2_RGB888 = 0x24,
>> + CSI_2_RAW6 = 0x28,
>> + CSI_2_RAW7 = 0x29,
>> + CSI_2_RAW8 = 0x2A,
>> + CSI_2_RAW10 = 0x2B,
>> + CSI_2_RAW12 = 0x2C,
>> + CSI_2_RAW14 = 0x2D,
>> +};
>> +
>> +/** @short Interrupt Masks */
>> +enum interrupt_type {
>> + CSI2_INT_PHY_FATAL = 1 << 0,
>> + CSI2_INT_PKT_FATAL = 1 << 1,
>> + CSI2_INT_FRAME_FATAL = 1 << 2,
>> + CSI2_INT_PHY = 1 << 16,
>> + CSI2_INT_PKT = 1 << 17,
>> + CSI2_INT_LINE = 1 << 18,
>> + CSI2_INT_IPI = 1 << 19,
>> +
>> +};
>> +
>> +/** @short DWC MIPI CSI-2 output types*/
>> +enum output_type {
>> + IPI_OUT = 0,
>> + IDI_OUT = 1,
>> + BOTH_OUT = 2
>> +};
>> +
>> +/** @short IPI output types*/
>> +enum ipi_output_type {
>> + CAMERA_TIMING = 0,
>> + AUTO_TIMING = 1
>> +};
>> +
>> +/**
>> + * @short Format template
>> + */
>> +struct mipi_fmt {
>> + const char *name;
>> + u32 code;
>> + u8 depth;
>> +};
>> +
>> +struct csi_hw {
>> +
>> + uint32_t num_lanes;
>> + uint32_t output_type;
>> +
>> + /*IPI Info */
>> + uint32_t ipi_mode;
>> + uint32_t ipi_color_mode;
>> + uint32_t ipi_auto_flush;
>> + uint32_t virtual_ch;
>> +
>> + uint32_t hsa;
>> + uint32_t hbp;
>> + uint32_t hsd;
>> + uint32_t htotal;
>> +
>> + uint32_t vsa;
>> + uint32_t vbp;
>> + uint32_t vfp;
>> + uint32_t vactive;
>> +};
>> +
>> +/**
>> + * @short Structure to embed device driver information
>> + */
>> +struct mipi_csi_dev {
>> + struct v4l2_subdev sd;
>> + struct video_device vdev;
>> +
>> + struct mutex lock;
>> + spinlock_t slock;
>> + struct media_pad pads[CSI_PADS_NUM];
>> + struct platform_device *pdev;
>> + u8 index;
>> +
>> + /** Store current format */
>> + const struct mipi_fmt *fmt;
>> + struct v4l2_mbus_framefmt format;
>> +
>> + /** Device Tree Information */
>> + void __iomem *base_address;
>> + uint32_t ctrl_irq_number;
>> +
>> + struct csi_hw hw;
>> +
>> + struct phy *phy;
>> +};
>> +
>> +#endif /* DW_MIPI_CSI */
>> diff --git a/drivers/media/platform/dwc/plat_ipk.c b/drivers/media/platform/dwc/plat_ipk.c
>> new file mode 100644
>> index 0000000..48b9e36
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/plat_ipk.c
>> @@ -0,0 +1,835 @@
>> +/**
>> + * DWC MIPI CSI-2 Host IPK platform device driver
>> + *
>> + * Based on Omnivision OV7670 Camera Driver
>> + * Copyright (C) 2011 - 2013 Samsung Electronics Co., Ltd.
>> + * Author: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> + *
>> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
>> + * Author: Ramiro Oliveira <ramiro.oliveira@synopsys.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published
>> + * by the Free Software Foundation, either version 2 of the License,
>> + * or (at your option) any later version.
>> + */
>> +
>> +#include "plat_ipk.h"
>> +#include "video_device.h"
>> +#include "dw_mipi_csi.h"
>> +#include <media/v4l2-of.h>
>> +#include <media/v4l2-subdev.h>
>> +
>> +static int
>> +__plat_ipk_pipeline_s_format(struct plat_ipk_media_pipeline *ep,
>> + struct v4l2_subdev_format *fmt)
>> +{
>> +
>> + struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
>> + static const u8 seq[IDX_MAX] = {IDX_SENSOR, IDX_CSI, IDX_VDEV};
>> +
>> + fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
>> + v4l2_subdev_call(p->subdevs[seq[IDX_CSI]], pad, set_fmt, NULL, fmt);
>> +
>> + return 0;
>> +}
>> +
>> +static void
>> +plat_ipk_pipeline_prepare(struct plat_ipk_pipeline *p, struct media_entity *me)
>> +{
>> + struct v4l2_subdev *sd;
>> + int i = 0;
>
> unsigned int?
>
>> +
>> + for (i = 0; i < IDX_MAX; i++)
>> + p->subdevs[i] = NULL;
>> +
>> + while (1) {
>> + struct media_pad *pad = NULL;
>> +
>> + for (i = 0; i < me->num_pads; i++) {
>> + struct media_pad *spad = &me->pads[i];
>> +
>> + if (!(spad->flags & MEDIA_PAD_FL_SINK))
>> + continue;
>> +
>> + pad = media_entity_remote_pad(spad);
>> + if (pad)
>> + break;
>> + }
>> + if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
>> + break;
>> +
>> + sd = media_entity_to_v4l2_subdev(pad->entity);
>> +
>> + switch (sd->grp_id) {
>> + case GRP_ID_SENSOR:
>> + p->subdevs[IDX_SENSOR] = sd;
>> + break;
>> + case GRP_ID_CSI:
>> + p->subdevs[IDX_CSI] = sd;
>> + break;
>> + case GRP_ID_VIDEODEV:
>> + p->subdevs[IDX_VDEV] = sd;
>> + break;
>> + default:
>> + break;
>> + }
>> + me = &sd->entity;
>> + if (me->num_pads == 1)
>> + break;
>> + }
>> +}
>> +
>> +static int __subdev_set_power(struct v4l2_subdev *sd, int on)
>> +{
>> + int *use_count;
>> + int ret;
>> +
>> + if (sd == NULL) {
>> + pr_info("null subdev\n");
>> + return -ENXIO;
>> + }
>> + use_count = &sd->entity.use_count;
>> + if (on && (*use_count)++ > 0)
>> + return 0;
>> + else if (!on && (*use_count == 0 || --(*use_count) > 0))
>> + return 0;
>> +
>> + pr_debug("%d %s !\n", on, sd->entity.name);
>
> Please do use dev_*() family of functions instead.
>
This print is not really useful in production, so I'll remove it.
>> + ret = v4l2_subdev_call(sd, core, s_power, on);
>> +
>> + return ret != -ENOIOCTLCMD ? ret : 0;
>> +}
>> +
>> +static int plat_ipk_pipeline_s_power(struct plat_ipk_pipeline *p, bool on)
>> +{
>> + static const u8 seq[IDX_MAX] = {IDX_CSI, IDX_SENSOR, IDX_VDEV};
>> + int i, ret = 0;
>> +
>> + for (i = 0; i < IDX_MAX; i++) {
>> + unsigned int idx = seq[i];
>> +
>> + if (p->subdevs[idx] == NULL)
>> + pr_info("No device registered on %d\n", idx);
>> + else {
>> + ret = __subdev_set_power(p->subdevs[idx], on);
>> + if (ret < 0 && ret != -ENXIO)
>> + goto error;
>> + }
>> + }
>> + return 0;
>> +error:
>> + for (; i >= 0; i--) {
>> + unsigned int idx = seq[i];
>> +
>> + __subdev_set_power(p->subdevs[idx], !on);
>> + }
>> + return ret;
>> +}
>> +
>> +static int
>> +__plat_ipk_pipeline_open(struct plat_ipk_media_pipeline *ep,
>> + struct media_entity *me, bool prepare)
>> +{
>> + struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
>> + int ret;
>> +
>> + if (WARN_ON(p == NULL || me == NULL))
>> + return -EINVAL;
>> +
>> + if (prepare)
>> + plat_ipk_pipeline_prepare(p, me);
>> +
>> + ret = plat_ipk_pipeline_s_power(p, 1);
>> + if (!ret)
>> + return 0;
>> +
>> + return ret;
>> +}
>> +
>> +static int __plat_ipk_pipeline_close(struct plat_ipk_media_pipeline *ep)
>> +{
>> + struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
>> + int ret;
>> +
>> + ret = plat_ipk_pipeline_s_power(p, 0);
>> +
>> + return ret == -ENXIO ? 0 : ret;
>> +}
>> +
>> +static int
>> +__plat_ipk_pipeline_s_stream(struct plat_ipk_media_pipeline *ep, bool on)
>> +{
>> + static const u8 seq[IDX_MAX] = {IDX_SENSOR, IDX_CSI, IDX_VDEV};
>> + struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
>> + int i, ret = 0;
>> +
>> + for (i = 0; i < IDX_MAX; i++) {
>> + unsigned int idx = seq[i];
>> +
>> + if (p->subdevs[idx] == NULL)
>> + pr_debug("No device registered on %d\n", idx);
>> + else {
>> + ret =
>> + v4l2_subdev_call(p->subdevs[idx], video, s_stream,
>> + on);
>> +
>> + if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
>> + goto error;
>> + }
>> + }
>> + return 0;
>> +error:
>> + for (; i >= 0; i--) {
>> + unsigned int idx = seq[i];
>> +
>> + v4l2_subdev_call(p->subdevs[idx], video, s_stream, !on);
>> + }
>> + return ret;
>> +}
>> +
>> +static const struct plat_ipk_media_pipeline_ops plat_ipk_pipeline_ops = {
>> + .open = __plat_ipk_pipeline_open,
>> + .close = __plat_ipk_pipeline_close,
>> + .set_format = __plat_ipk_pipeline_s_format,
>> + .set_stream = __plat_ipk_pipeline_s_stream,
>> +};
>> +
>> +static struct plat_ipk_media_pipeline *
>> +plat_ipk_pipeline_create(struct plat_ipk_dev *plat_ipk)
>> +{
>> + struct plat_ipk_pipeline *p;
>> +
>> + p = kzalloc(sizeof(*p), GFP_KERNEL);
>> + if (!p)
>> + return NULL;
>> +
>> + list_add_tail(&p->list, &plat_ipk->pipelines);
>> +
>> + p->ep.ops = &plat_ipk_pipeline_ops;
>> + return &p->ep;
>> +}
>> +
>> +static void
>> +plat_ipk_pipelines_free(struct plat_ipk_dev *plat_ipk)
>> +{
>> + while (!list_empty(&plat_ipk->pipelines)) {
>> + struct plat_ipk_pipeline *p;
>> +
>> + p = list_entry(plat_ipk->pipelines.next, typeof(*p), list);
>> + list_del(&p->list);
>> + kfree(p);
>> + }
>> +}
>> +
>> +static int
>> +plat_ipk_parse_port_node(struct plat_ipk_dev *plat_ipk,
>> + struct device_node *port, unsigned int index)
>> +{
>> + struct device_node *rem, *ep;
>> + struct v4l2_of_endpoint endpoint;
>> + struct plat_ipk_source_info *pd = &plat_ipk->sensor[index].pdata;
>> +
>> + /* Assume here a port node can have only one endpoint node. */
>> + ep = of_get_next_child(port, NULL);
>> + if (!ep)
>> + return 0;
>> +
>> + v4l2_of_parse_endpoint(ep, &endpoint);
>> + if (WARN_ON(endpoint.base.port == 0) || index >= PLAT_MAX_SENSORS)
>> + return -EINVAL;
>> +
>> + pd->mux_id = endpoint.base.port - 1;
>> +
>> + rem = of_graph_get_remote_port_parent(ep);
>> + of_node_put(ep);
>> + if (rem == NULL) {
>> + v4l2_info(&plat_ipk->v4l2_dev,
>> + "Remote device at %s not found\n", ep->full_name);
>> + return 0;
>> + }
>> +
>> + if (WARN_ON(index >= ARRAY_SIZE(plat_ipk->sensor)))
>> + return -EINVAL;
>> +
>> + plat_ipk->sensor[index].asd.match_type = V4L2_ASYNC_MATCH_OF;
>> + plat_ipk->sensor[index].asd.match.of.node = rem;
>> + plat_ipk->async_subdevs[index] = &plat_ipk->sensor[index].asd;
>> +
>> + plat_ipk->num_sensors++;
>> +
>> + of_node_put(rem);
>> + return 0;
>> +}
>> +
>> +
>> +static int plat_ipk_register_sensor_entities(struct plat_ipk_dev *plat_ipk)
>> +{
>> + struct device_node *parent = plat_ipk->pdev->dev.of_node;
>> + struct device_node *node;
>> + int index = 0;
>> + int ret;
>> +
>> + plat_ipk->num_sensors = 0;
>> +
>> + for_each_available_child_of_node(parent, node) {
>> + struct device_node *port;
>> +
>> + if (of_node_cmp(node->name, "csi2"))
>> + continue;
>> + port = of_get_next_child(node, NULL);
>> + if (!port)
>> + continue;
>> +
>> + ret = plat_ipk_parse_port_node(plat_ipk, port, index);
>> + if (ret < 0)
>> + return ret;
>> + index++;
>> + }
>> + return 0;
>> +}
>> +
>> +static int
>> +__of_get_port_id(struct device_node *np)
>> +{
>> + u32 reg = 0;
>> +
>> + np = of_get_child_by_name(np, "port");
>> + if (!np)
>> + return -EINVAL;
>> + of_property_read_u32(np, "reg", ®);
>> +
>> + return reg - 1;
>> +}
>> +
>> +static int register_videodev_entity(struct plat_ipk_dev *plat_ipk,
>> + struct video_device_dev *vid_dev)
>> +{
>> + struct v4l2_subdev *sd;
>> + struct plat_ipk_media_pipeline *ep;
>> + int ret;
>> +
>> + sd = &vid_dev->subdev;
>> + sd->grp_id = GRP_ID_VIDEODEV;
>> +
>> + ep = plat_ipk_pipeline_create(plat_ipk);
>> + if (!ep)
>> + return -ENOMEM;
>> +
>> + v4l2_set_subdev_hostdata(sd, ep);
>> +
>> + ret = v4l2_device_register_subdev(&plat_ipk->v4l2_dev, sd);
>> + if (!ret)
>> + plat_ipk->vid_dev = vid_dev;
>> + else
>> + v4l2_err(&plat_ipk->v4l2_dev,
>> + "Failed to register Video Device\n");
>> + return ret;
>> +}
>> +
>> +static int register_mipi_csi_entity(struct plat_ipk_dev *plat_ipk,
>> + struct platform_device *pdev, struct v4l2_subdev *sd)
>> +{
>> + struct device_node *node = pdev->dev.of_node;
>> + int id, ret;
>> +
>> + id = node ? __of_get_port_id(node) : max(0, pdev->id);
>> +
>> + if (WARN_ON(id < 0 || id >= CSI_MAX_ENTITIES))
>> + return -ENOENT;
>> +
>> + if (WARN_ON(plat_ipk->mipi_csi[id].sd))
>> + return -EBUSY;
>> +
>> + sd->grp_id = GRP_ID_CSI;
>> + ret = v4l2_device_register_subdev(&plat_ipk->v4l2_dev, sd);
>> +
>> + if (!ret)
>> + plat_ipk->mipi_csi[id].sd = sd;
>> + else
>> + v4l2_err(&plat_ipk->v4l2_dev,
>> + "Failed to register MIPI-CSI.%d (%d)\n", id, ret);
>> + return ret;
>> +}
>> +
>> +static int plat_ipk_register_platform_entity(struct plat_ipk_dev *plat_ipk,
>> + struct platform_device *pdev, int plat_entity)
>> +{
>> + struct device *dev = &pdev->dev;
>> + int ret = -EPROBE_DEFER;
>> + void *drvdata;
>> +
>> +
>
> Extra newline.
>
Removing it.
>> + device_lock(dev);
>> + if (!dev->driver || !try_module_get(dev->driver->owner))
>> + goto dev_unlock;
>> +
>> + drvdata = dev_get_drvdata(dev);
>> +
>> + if (drvdata) {
>> + switch (plat_entity) {
>> + case IDX_VDEV:
>> + ret = register_videodev_entity(plat_ipk, drvdata);
>> + break;
>> + case IDX_CSI:
>> + ret = register_mipi_csi_entity(plat_ipk, pdev, drvdata);
>> + break;
>> + default:
>> + ret = -ENODEV;
>> + }
>> + } else
>> + dev_err(&plat_ipk->pdev->dev, "%s no drvdata\n", dev_name(dev));
>> + module_put(dev->driver->owner);
>> +dev_unlock:
>> + device_unlock(dev);
>> + if (ret == -EPROBE_DEFER)
>> + dev_info(&plat_ipk->pdev->dev,
>> + "deferring %s device registration\n", dev_name(dev));
>> + else if (ret < 0)
>> + dev_err(&plat_ipk->pdev->dev,
>> + "%s device registration failed (%d)\n", dev_name(dev),
>> + ret);
>> + return ret;
>> +}
>> +
>> +static int
>> +plat_ipk_register_platform_entities(struct plat_ipk_dev *plat_ipk,
>> + struct device_node *parent)
>> +{
>> + struct device_node *node;
>> + int ret = 0;
>> +
>> + for_each_available_child_of_node(parent, node) {
>> + struct platform_device *pdev;
>> + int plat_entity = -1;
>> +
>> + pdev = of_find_device_by_node(node);
>> + if (!pdev)
>> + continue;
>> +
>> + if (!strcmp(node->name, VIDEODEV_OF_NODE_NAME))
>> + plat_entity = IDX_VDEV;
>> + else if (!strcmp(node->name, CSI_OF_NODE_NAME))
>> + plat_entity = IDX_CSI;
>> +
>> + if (plat_entity >= 0)
>> + ret = plat_ipk_register_platform_entity(plat_ipk, pdev,
>> + plat_entity);
>> + put_device(&pdev->dev);
>> + if (ret < 0)
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static void
>> +plat_ipk_unregister_entities(struct plat_ipk_dev *plat_ipk)
>> +{
>> + int i;
>> + struct video_device_dev *dev = plat_ipk->vid_dev;
>> +
>> + if (dev == NULL)
>> + return;
>> + v4l2_device_unregister_subdev(&dev->subdev);
>> + dev->ve.pipe = NULL;
>> + plat_ipk->vid_dev = NULL;
>> +
>> + for (i = 0; i < CSI_MAX_ENTITIES; i++) {
>> + if (plat_ipk->mipi_csi[i].sd == NULL)
>> + continue;
>> + v4l2_device_unregister_subdev(plat_ipk->mipi_csi[i].sd);
>> + plat_ipk->mipi_csi[i].sd = NULL;
>> + }
>> +
>> + v4l2_info(&plat_ipk->v4l2_dev, "Unregistered all entities\n");
>> +}
>> +
>> +static int
>> +__plat_ipk_create_videodev_sink_links(struct plat_ipk_dev *plat_ipk,
>> + struct media_entity *source,
>> + int pad)
>> +{
>> + struct media_entity *sink;
>> + int ret = 0;
>> +
>> + if (!plat_ipk->vid_dev)
>> + return 0;
>> +
>> + sink = &plat_ipk->vid_dev->subdev.entity;
>> + ret = media_create_pad_link(source, pad, sink,
>> + CSI_PAD_SOURCE, MEDIA_LNK_FL_ENABLED);
>> + if (ret)
>> + return ret;
>> +
>> + ret = media_entity_call(sink, link_setup, &sink->pads[0],
>> + &source->pads[pad], 0);
>
> What's the purpose of calling link_setup() here?
>
None, this was useful in the past but it's not being used right now. I'll remove it.
>> + if (ret)
>> + return 0;
>> +
>> + v4l2_info(&plat_ipk->v4l2_dev, "created link [%s] -> [%s]\n",
>> + source->name, sink->name);
>> +
>> + return 0;
>> +}
>> +
>> +
>> +static int
>> +__plat_ipk_create_videodev_source_links(struct plat_ipk_dev *plat_ipk)
>> +{
>> + struct media_entity *source, *sink;
>> + int ret = 0;
>> +
>> + struct video_device_dev *vid_dev = plat_ipk->vid_dev;
>> +
>> + if (vid_dev == NULL)
>> + return -ENODEV;
>> +
>> + source = &vid_dev->subdev.entity;
>> + sink = &vid_dev->ve.vdev.entity;
>> +
>> + ret = media_create_pad_link(source, VIDEO_DEV_SD_PAD_SOURCE_DMA,
>> + sink, 0, MEDIA_LNK_FL_ENABLED);
>> +
>> + v4l2_info(&plat_ipk->v4l2_dev, "created link [%s] -> [%s]\n",
>> + source->name, sink->name);
>> + return ret;
>> +}
>> +
>> +static int
>> +plat_ipk_create_links(struct plat_ipk_dev *plat_ipk)
>> +{
>> + struct v4l2_subdev *csi_sensor[CSI_MAX_ENTITIES] = { NULL };
>> + struct v4l2_subdev *sensor, *csi;
>> + struct media_entity *source;
>> + struct plat_ipk_source_info *pdata;
>> + int i, pad, ret = 0;
>> +
>> + for (i = 0; i < plat_ipk->num_sensors; i++) {
>> + if (plat_ipk->sensor[i].subdev == NULL)
>> + continue;
>> +
>> + sensor = plat_ipk->sensor[i].subdev;
>> + pdata = v4l2_get_subdev_hostdata(sensor);
>> + if (!pdata)
>> + continue;
>> +
>> + source = NULL;
>> +
>> + csi = plat_ipk->mipi_csi[pdata->mux_id].sd;
>> + if (WARN(csi == NULL, "MIPI-CSI interface specified but dw-mipi-csi module is not loaded!\n"))
>> + return -EINVAL;
>> +
>> + pad = sensor->entity.num_pads - 1;
>> + ret = media_create_pad_link(&sensor->entity, pad,
>> + &csi->entity, CSI_PAD_SINK,
>> + MEDIA_LNK_FL_IMMUTABLE |
>> + MEDIA_LNK_FL_ENABLED);
>> +
>> + if (ret)
>> + return ret;
>> + v4l2_info(&plat_ipk->v4l2_dev, "created link [%s] -> [%s]\n",
>> + sensor->entity.name, csi->entity.name);
>
> dev_dbg() ?
>
Yes. Changing it.
>> +
>> + csi_sensor[pdata->mux_id] = sensor;
>> + }
>> +
>> + for (i = 0; i < CSI_MAX_ENTITIES; i++) {
>> + if (plat_ipk->mipi_csi[i].sd == NULL) {
>> + pr_info("no link\n");
>> + continue;
>> + }
>> +
>> + source = &plat_ipk->mipi_csi[i].sd->entity;
>> + pad = VIDEO_DEV_SD_PAD_SINK_CSI;
>> +
>> + ret = __plat_ipk_create_videodev_sink_links(plat_ipk, source,
>> + pad);
>> + }
>> +
>> + ret = __plat_ipk_create_videodev_source_links(plat_ipk);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return ret;
>> +}
>> +
>> +static int __plat_ipk_modify_pipeline(struct media_entity *entity, bool enable)
>> +{
>> + struct plat_ipk_video_entity *ve;
>> + struct plat_ipk_pipeline *p;
>> + struct video_device *vdev;
>> + int ret;
>> +
>> + vdev = media_entity_to_video_device(entity);
>> +
>> + if (vdev->entity.use_count == 0)
>> + return 0;
>> +
>> + ve = vdev_to_plat_ipk_video_entity(vdev);
>> + p = to_plat_ipk_pipeline(ve->pipe);
>> +
>> + if (enable)
>> + ret = __plat_ipk_pipeline_open(ve->pipe, entity, true);
>> + else
>> + ret = __plat_ipk_pipeline_close(ve->pipe);
>> +
>> + if (ret == 0 && !enable)
>> + memset(p->subdevs, 0, sizeof(p->subdevs));
>> +
>> + return ret;
>> +}
>> +
>> +
>> +static int
>> +__plat_ipk_modify_pipelines(struct media_entity *entity, bool enable,
>> + struct media_entity_graph *graph)
>> +{
>> + struct media_entity *entity_err = entity;
>> + int ret;
>> +
>> + media_entity_graph_walk_start(graph, entity);
>> +
>> + while ((entity = media_entity_graph_walk_next(graph))) {
>> + if (!is_media_entity_v4l2_video_device(entity))
>> + continue;
>> +
>> + ret = __plat_ipk_modify_pipeline(entity, enable);
>> +
>> + if (ret < 0)
>> + goto err;
>> + }
>> +
>> + return 0;
>> +
>> +err:
>> + media_entity_graph_walk_start(graph, entity_err);
>> +
>> + while ((entity_err = media_entity_graph_walk_next(graph))) {
>> + if (!is_media_entity_v4l2_video_device(entity_err))
>> + continue;
>> +
>> + __plat_ipk_modify_pipeline(entity_err, !enable);
>> +
>> + if (entity_err == entity)
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int
>> +plat_ipk_link_notify(struct media_link *link, unsigned int flags,
>> + unsigned int notification)
>> +{
>> + struct media_entity_graph *graph =
>> + &container_of(link->graph_obj.mdev, struct plat_ipk_dev,
>> + media_dev)->link_setup_graph;
>> + struct media_entity *sink = link->sink->entity;
>> + int ret = 0;
>> +
>> + pr_debug("Link notify\n");
>
> This probably made sense at development time but should be removed now.
>
Yes. You're right.
>> +
>> + if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH) {
>> + ret = media_entity_graph_walk_init(graph, link->graph_obj.mdev);
>> + if (ret)
>> + return ret;
>> + if (!(flags & MEDIA_LNK_FL_ENABLED))
>> + ret = __plat_ipk_modify_pipelines(sink, false, graph);
>> +
>> + } else if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH) {
>> + if (link->flags & MEDIA_LNK_FL_ENABLED)
>> + ret = __plat_ipk_modify_pipelines(sink, true, graph);
>> + media_entity_graph_walk_cleanup(graph);
>> + }
>> +
>> + return ret ? -EPIPE : 0;
>> +}
>
> Missing newline.
>
Adding one.
>> +static const struct media_device_ops plat_ipk_media_ops = {
>> + .link_notify = plat_ipk_link_notify,
>> +};
>> +
>> +
>> +static int
>> +subdev_notifier_bound(struct v4l2_async_notifier *notifier,
>> + struct v4l2_subdev *subdev, struct v4l2_async_subdev *asd)
>> +{
>> + struct plat_ipk_dev *plat_ipk = notifier_to_plat_ipk(notifier);
>> + struct plat_ipk_sensor_info *si = NULL;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(plat_ipk->sensor); i++)
>> + if (plat_ipk->sensor[i].asd.match.of.node ==
>> + subdev->dev->of_node)
>> + si = &plat_ipk->sensor[i];
>> +
>> + if (si == NULL)
>> + return -EINVAL;
>> +
>> + v4l2_set_subdev_hostdata(subdev, &si->pdata);
>> +
>> + subdev->grp_id = GRP_ID_SENSOR;
>> +
>> + si->subdev = subdev;
>> +
>> + v4l2_info(&plat_ipk->v4l2_dev, "Registered sensor subdevice: %s (%d)\n",
>> + subdev->name, plat_ipk->num_sensors);
>> +
>> + plat_ipk->num_sensors++;
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +subdev_notifier_complete(struct v4l2_async_notifier *notifier)
>> +{
>> + struct plat_ipk_dev *plat_ipk = notifier_to_plat_ipk(notifier);
>> + int ret;
>> +
>> + mutex_lock(&plat_ipk->media_dev.graph_mutex);
>> +
>> + ret = plat_ipk_create_links(plat_ipk);
>> + if (ret < 0)
>> + goto unlock;
>> +
>> + ret = v4l2_device_register_subdev_nodes(&plat_ipk->v4l2_dev);
>> +unlock:
>> + mutex_unlock(&plat_ipk->media_dev.graph_mutex);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return media_device_register(&plat_ipk->media_dev);
>> +}
>> +
>> +static const struct of_device_id plat_ipk_of_match[];
>
> This is redundant.
>
You're right.
>> +
>> +static int plat_ipk_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct v4l2_device *v4l2_dev;
>> + struct plat_ipk_dev *plat_ipk;
>> + int ret;
>> +
>> + dev_info(dev, "Installing DW MIPI CSI-2 IPK Platform module\n");
>> +
>> + plat_ipk = devm_kzalloc(dev, sizeof(*plat_ipk), GFP_KERNEL);
>> + if (!plat_ipk)
>> + return -ENOMEM;
>> +
>> + spin_lock_init(&plat_ipk->slock);
>> + INIT_LIST_HEAD(&plat_ipk->pipelines);
>> + plat_ipk->pdev = pdev;
>> +
>> + strlcpy(plat_ipk->media_dev.model, "SNPS IPK Platform",
>> + sizeof(plat_ipk->media_dev.model));
>> + plat_ipk->media_dev.ops = &plat_ipk_media_ops;
>> + plat_ipk->media_dev.dev = dev;
>> +
>> + v4l2_dev = &plat_ipk->v4l2_dev;
>> + v4l2_dev->mdev = &plat_ipk->media_dev;
>> + strlcpy(v4l2_dev->name, "plat-ipk", sizeof(v4l2_dev->name));
>> +
>> + media_device_init(&plat_ipk->media_dev);
>> +
>> + ret = v4l2_device_register(dev, &plat_ipk->v4l2_dev);
>> + if (ret < 0) {
>> + v4l2_err(v4l2_dev, "Failed to register v4l2_device: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + platform_set_drvdata(pdev, plat_ipk);
>> +
>> + ret = plat_ipk_register_platform_entities(plat_ipk, dev->of_node);
>> + if (ret)
>> + goto err_m_ent;
>> +
>> + ret = plat_ipk_register_sensor_entities(plat_ipk);
>> + if (ret)
>> + goto err_m_ent;
>> +
>> + if (plat_ipk->num_sensors > 0) {
>> + plat_ipk->subdev_notifier.subdevs = plat_ipk->async_subdevs;
>> + plat_ipk->subdev_notifier.num_subdevs = plat_ipk->num_sensors;
>> + plat_ipk->subdev_notifier.bound = subdev_notifier_bound;
>> + plat_ipk->subdev_notifier.complete = subdev_notifier_complete;
>> + plat_ipk->num_sensors = 0;
>> +
>> + ret = v4l2_async_notifier_register(&plat_ipk->v4l2_dev,
>> + &plat_ipk->subdev_notifier);
>> + if (ret)
>> + goto err_m_ent;
>> + }
>> +
>> + return 0;
>> +
>> +err_m_ent:
>> + plat_ipk_unregister_entities(plat_ipk);
>> + media_device_unregister(&plat_ipk->media_dev);
>> + media_device_cleanup(&plat_ipk->media_dev);
>> + v4l2_device_unregister(&plat_ipk->v4l2_dev);
>> + return ret;
>> +}
>> +
>> +static int plat_ipk_remove(struct platform_device *pdev)
>> +{
>> + struct plat_ipk_dev *dev = platform_get_drvdata(pdev);
>> +
>> + if (!dev)
>> + return 0;
>
> Wouldn't !dev here require a driver bug? If probe() succeeds, then dev is
> non-zero, right?
>
It was just a better safe than sorry situation, but if you think it's not
necessary I'll remove it.
>> +
>> + v4l2_async_notifier_unregister(&dev->subdev_notifier);
>> +
>> + v4l2_device_unregister(&dev->v4l2_dev);
>> + plat_ipk_unregister_entities(dev);
>> + plat_ipk_pipelines_free(dev);
>> + media_device_unregister(&dev->media_dev);
>> + media_device_cleanup(&dev->media_dev);
>> +
>> + dev_info(&pdev->dev, "Driver removed\n");
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short of_device_id structure
>> + */
>> +static const struct of_device_id plat_ipk_of_match[] = {
>> + {.compatible = "snps,plat-ipk"},
>> + {}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, plat_ipk_of_match);
>> +
>> +/**
>> + * @short Platform driver structure
>> + */
>> +static struct platform_driver plat_ipk_pdrv = {
>> + .remove = plat_ipk_remove,
>> + .probe = plat_ipk_probe,
>> + .driver = {
>> + .name = "snps,plat-ipk",
>> + .owner = THIS_MODULE,
>> + .of_match_table = plat_ipk_of_match,
>> + },
>> +};
>> +
>> +static int __init
>> +plat_ipk_init(void)
>> +{
>> + request_module("dw-mipi-csi");
>
> Why do you need this?
>
To make sure that module is loaded when I start loading this one.
Do you think it's not necessary to do this, or is there a better way to do this?
>> +
>> + return platform_driver_register(&plat_ipk_pdrv);
>> +}
>> +
>> +static void __exit
>> +plat_ipk_exit(void)
>> +{
>> + platform_driver_unregister(&plat_ipk_pdrv);
>> +}
>> +
>> +module_init(plat_ipk_init);
>> +module_exit(plat_ipk_exit);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Ramiro Oliveira <roliveir@synopsys.com>");
>> +MODULE_DESCRIPTION("Platform driver for MIPI CSI-2 Host IPK");
>> diff --git a/drivers/media/platform/dwc/plat_ipk.h b/drivers/media/platform/dwc/plat_ipk.h
>> new file mode 100644
>> index 0000000..ef569eb
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/plat_ipk.h
>> @@ -0,0 +1,97 @@
>> +/*
>> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef PLAT_IPK_H_
>> +#define PLAT_IPK_H_
>> +
>> +#include <linux/module.h>
>> +#include <linux/errno.h>
>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +#include <linux/slab.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/list.h>
>> +#include <linux/string.h>
>> +#include <media/v4l2-device.h>
>> +#include <linux/videodev2.h>
>> +#include <media/media-entity.h>
>> +
>> +#include "plat_ipk_video.h"
>> +
>> +#define VIDEODEV_OF_NODE_NAME "video-device"
>> +#define CSI_OF_NODE_NAME "csi2"
>> +
>> +enum plat_ipk_subdev_index {
>> + IDX_SENSOR,
>> + IDX_CSI,
>> + IDX_VDEV,
>> + IDX_MAX,
>> +};
>> +
>> +struct plat_ipk_sensor_info {
>> + struct plat_ipk_source_info pdata;
>> + struct v4l2_async_subdev asd;
>> + struct v4l2_subdev *subdev;
>> + struct mipi_csi_dev *host;
>> +};
>> +
>> +struct plat_ipk_pipeline {
>> + struct plat_ipk_media_pipeline ep;
>> + struct list_head list;
>> + struct media_entity *vdev_entity;
>> + struct v4l2_subdev *subdevs[IDX_MAX];
>> +};
>> +
>> +#define to_plat_ipk_pipeline(_ep)\
>> + container_of(_ep, struct plat_ipk_pipeline, ep)
>> +
>> +struct mipi_csi_info {
>> + struct v4l2_subdev *sd;
>> + int id;
>> +};
>> +
>> +/**
>> + * @short Structure to embed device driver information
>> + */
>> +struct plat_ipk_dev {
>> + struct mipi_csi_info mipi_csi[CSI_MAX_ENTITIES];
>> + struct video_device_dev *vid_dev;
>> + struct device *dev;
>> + struct media_device media_dev;
>> + struct v4l2_device v4l2_dev;
>> + struct platform_device *pdev;
>> + struct plat_ipk_sensor_info sensor[PLAT_MAX_SENSORS];
>> + struct v4l2_async_notifier subdev_notifier;
>> + struct v4l2_async_subdev *async_subdevs[PLAT_MAX_SENSORS];
>> + spinlock_t slock;
>> + struct list_head pipelines;
>> + int num_sensors;
>> + struct media_entity_graph link_setup_graph;
>> +};
>> +
>> +static inline struct plat_ipk_dev *
>> +entity_to_plat_ipk_mdev(struct media_entity *me)
>> +{
>> + return me->graph_obj.mdev == NULL ? NULL :
>> + container_of(me->graph_obj.mdev, struct plat_ipk_dev, media_dev);
>> +}
>> +
>> +static inline struct plat_ipk_dev *
>> +notifier_to_plat_ipk(struct v4l2_async_notifier *n)
>> +{
>> + return container_of(n, struct plat_ipk_dev, subdev_notifier);
>> +}
>> +
>> +static inline void
>> +plat_ipk_graph_unlock(struct plat_ipk_video_entity *ve)
>> +{
>> + mutex_unlock(&ve->vdev.entity.graph_obj.mdev->graph_mutex);
>> +}
>> +
>> +#endif /* PLAT_IPK_H_ */
>> diff --git a/drivers/media/platform/dwc/plat_ipk_video.h b/drivers/media/platform/dwc/plat_ipk_video.h
>> new file mode 100644
>> index 0000000..6bfc9f8
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/plat_ipk_video.h
>> @@ -0,0 +1,97 @@
>> +/*
>> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef PLAT_IPK_INCLUDES_H_
>> +#define PLAT_IPK_INCLUDES_H_
>> +
>> +#include <media/media-entity.h>
>> +#include <media/v4l2-dev.h>
>> +#include <media/v4l2-mediabus.h>
>> +#include <media/v4l2-subdev.h>
>> +
>> +/*
>> + * The subdevices' group IDs.
>> + */
>> +
>> +#define MAX_WIDTH 3280
>> +#define MAX_HEIGHT 1852
>> +
>> +#define MIN_WIDTH 640
>> +#define MIN_HEIGHT 480
>> +
>> +#define GRP_ID_SENSOR (10)
>> +#define GRP_ID_CSI (20)
>> +#define GRP_ID_VIDEODEV (30)
>> +
>> +#define CSI_MAX_ENTITIES 1
>> +#define PLAT_MAX_SENSORS 1
>> +
>> +enum video_dev_pads {
>> + VIDEO_DEV_SD_PAD_SINK_CSI = 0,
>> + VIDEO_DEV_SD_PAD_SOURCE_DMA = 1,
>> + VIDEO_DEV_SD_PADS_NUM = 2,
>> +};
>> +
>> +enum mipi_csi_pads {
>> + CSI_PAD_SINK = 0,
>> + CSI_PAD_SOURCE = 1,
>> + CSI_PADS_NUM = 2,
>> +};
>> +
>> +struct plat_ipk_source_info {
>> + u16 flags;
>> + u16 mux_id;
>> +};
>> +
>> +struct plat_ipk_fmt {
>> + u32 mbus_code;
>> + char *name;
>> + u32 fourcc;
>> + u8 depth;
>> +};
>> +
>> +struct plat_ipk_media_pipeline;
>> +
>> +/*
>> + * Media pipeline operations to be called from within a video node, i.e. the
>> + * last entity within the pipeline. Implemented by related media device driver.
>> + */
>> +struct plat_ipk_media_pipeline_ops {
>> + int (*prepare)(struct plat_ipk_media_pipeline *p,
>> + struct media_entity *me);
>> + int (*unprepare)(struct plat_ipk_media_pipeline *p);
>> + int (*open)(struct plat_ipk_media_pipeline *p,
>> + struct media_entity *me, bool resume);
>> + int (*close)(struct plat_ipk_media_pipeline *p);
>> + int (*set_stream)(struct plat_ipk_media_pipeline *p, bool state);
>> + int (*set_format)(struct plat_ipk_media_pipeline *p,
>> + struct v4l2_subdev_format *fmt);
>> +};
>> +
>> +struct plat_ipk_video_entity {
>> + struct video_device vdev;
>> + struct plat_ipk_media_pipeline *pipe;
>> +};
>> +
>> +struct plat_ipk_media_pipeline {
>> + struct media_pipeline mp;
>> + const struct plat_ipk_media_pipeline_ops *ops;
>> +};
>> +
>> +static inline struct plat_ipk_video_entity *
>> +vdev_to_plat_ipk_video_entity(struct video_device *vdev)
>> +{
>> + return container_of(vdev, struct plat_ipk_video_entity, vdev);
>> +}
>> +
>> +#define plat_ipk_pipeline_call(ent, op, args...)\
>> + (!(ent) ? -ENOENT : (((ent)->pipe->ops && (ent)->pipe->ops->op) ? \
>> + (ent)->pipe->ops->op(((ent)->pipe), ##args) : -ENOIOCTLCMD)) \
>> +
>> +
>> +#endif /* PLAT_IPK_INCLUDES_H_ */
>> diff --git a/drivers/media/platform/dwc/video_device.c b/drivers/media/platform/dwc/video_device.c
>> new file mode 100644
>> index 0000000..d827426
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/video_device.c
>> @@ -0,0 +1,741 @@
>> +/*
>> + * DWC MIPI CSI-2 Host IPK video device device driver
>> + *
>> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
>> + * Author: Ramiro Oliveira <ramiro.oliveira@synopsys.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published
>> + * by the Free Software Foundation, either version 2 of the License,
>> + * or (at your option) any later version.
>> + */
>> +
>> +#include "video_device.h"
>> +
>> +const struct plat_ipk_fmt vid_dev_formats[] = {
>> + {
>> + .name = "RGB888",
>> + .fourcc = V4L2_PIX_FMT_BGR24,
>> + .depth = 24,
>> + .mbus_code = MEDIA_BUS_FMT_RGB888_2X12_LE,
>> + }, {
>> + .name = "RGB565",
>> + .fourcc = V4L2_PIX_FMT_RGB565,
>> + .depth = 16,
>> + .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE,
>> + },
>> +};
>> +
>> +const struct plat_ipk_fmt *
>> +vid_dev_find_format(struct v4l2_format *f, int index)
>> +{
>> + const struct plat_ipk_fmt *fmt = NULL;
>> + unsigned int i;
>> +
>> + if (index >= (int) ARRAY_SIZE(vid_dev_formats))
>> + return NULL;
>> +
>> + for (i = 0; i < ARRAY_SIZE(vid_dev_formats); ++i) {
>> + fmt = &vid_dev_formats[i];
>> + if (fmt->fourcc == f->fmt.pix.pixelformat)
>> + return fmt;
>> + }
>> + return NULL;
>> +}
>> +
>> +/*
>> + * Video node ioctl operations
>> + */
>> +static int
>> +vidioc_querycap(struct file *file, void *priv, struct v4l2_capability *cap)
>> +{
>> + struct video_device_dev *vid_dev = video_drvdata(file);
>> +
>> + strlcpy(cap->driver, VIDEO_DEVICE_NAME, sizeof(cap->driver));
>> + strlcpy(cap->card, VIDEO_DEVICE_NAME, sizeof(cap->card));
>> + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
>> + dev_name(&vid_dev->pdev->dev));
>> +
>> + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
>> + cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
>> + return 0;
>> +}
>> +
>> +int
>> +vidioc_enum_fmt_vid_cap(struct file *file, void *priv, struct v4l2_fmtdesc *f)
>> +{
>> + const struct plat_ipk_fmt *p_fmt;
>> +
>> + if (f->index >= ARRAY_SIZE(vid_dev_formats))
>> + return -EINVAL;
>> +
>> + p_fmt = &vid_dev_formats[f->index];
>> +
>> + strlcpy(f->description, p_fmt->name, sizeof(f->description));
>> + f->pixelformat = p_fmt->fourcc;
>> +
>> + return 0;
>> +}
>> +
>> +int vidioc_g_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f)
>> +{
>> + struct video_device_dev *dev = video_drvdata(file);
>> +
>> + memcpy(&f->fmt.pix, &dev->format.fmt.pix,
>> + sizeof(struct v4l2_pix_format));
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +vidioc_try_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f)
>> +{
>> + const struct plat_ipk_fmt *fmt;
>> +
>> + fmt = vid_dev_find_format(f, -1);
>> + if (!fmt) {
>> + f->fmt.pix.pixelformat = V4L2_PIX_FMT_RGB565;
>> + fmt = vid_dev_find_format(f, -1);
>> + }
>> +
>> + f->fmt.pix.field = V4L2_FIELD_NONE;
>> + v4l_bound_align_image(&f->fmt.pix.width, 48, MAX_WIDTH, 2,
>> + &f->fmt.pix.height, 32, MAX_HEIGHT, 0, 0);
>> +
>> + f->fmt.pix.bytesperline = (f->fmt.pix.width * fmt->depth) >> 3;
>> + f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
>> + f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
>> + return 0;
>> +}
>> +
>> +int vidioc_s_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f)
>> +{
>> + struct video_device_dev *dev = video_drvdata(file);
>> + int ret;
>> + struct v4l2_subdev_format fmt;
>> +
>> + if (vb2_is_busy(&dev->vb_queue))
>> + return -EBUSY;
>> +
>> + ret = vidioc_try_fmt_vid_cap(file, dev, f);
>> + if (ret)
>> + return ret;
>> +
>> + dev->fmt = vid_dev_find_format(f, -1);
>> + pixel_format(dev) = f->fmt.pix.pixelformat;
>> + width(dev) = f->fmt.pix.width;
>> + height(dev) = f->fmt.pix.height;
>> + bytes_per_line(dev) = width(dev) * dev->fmt->depth / 8;
>> + size_image(dev) = height(dev) * bytes_per_line(dev);
>> +
>> + fmt.format.colorspace = V4L2_COLORSPACE_SRGB;
>> + fmt.format.code = dev->fmt->mbus_code;
>> +
>> + fmt.format.width = width(dev);
>> + fmt.format.height = height(dev);
>> +
>> + ret = plat_ipk_pipeline_call(&dev->ve, set_format, &fmt);
>> +
>> + return 0;
>> +}
>> +
>> +int vidioc_enum_framesizes(struct file *file, void *fh,
>> + struct v4l2_frmsizeenum *fsize)
>> +{
>> + static const struct v4l2_frmsize_stepwise sizes = {
>> + 48, MAX_WIDTH, 4,
>> + 32, MAX_HEIGHT, 1
>> + };
>> + int i;
>> +
>> + if (fsize->index)
>> + return -EINVAL;
>> + for (i = 0; i < ARRAY_SIZE(vid_dev_formats); i++)
>> + if (vid_dev_formats[i].fourcc == fsize->pixel_format)
>> + break;
>> + if (i == ARRAY_SIZE(vid_dev_formats))
>> + return -EINVAL;
>> + fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
>> + fsize->stepwise = sizes;
>> + return 0;
>> +}
>> +
>> +int vidioc_enum_input(struct file *file, void *priv, struct v4l2_input *input)
>> +{
>> + if (input->index != 0)
>> + return -EINVAL;
>> +
>> + input->type = V4L2_INPUT_TYPE_CAMERA;
>> + input->std = V4L2_STD_ALL; /* Not sure what should go here */
>> + strcpy(input->name, "Camera");
>> + return 0;
>> +}
>> +
>> +int vidioc_g_input(struct file *file, void *priv, unsigned int *i)
>> +{
>> + *i = 0;
>> + return 0;
>> +}
>> +
>> +int vidioc_s_input(struct file *file, void *priv, unsigned int i)
>> +{
>> + if (i != 0)
>> + return -EINVAL;
>> + return 0;
>> +}
>> +
>> +int vidioc_g_std(struct file *file, void *fh, v4l2_std_id *norm)
>> +{
>> + *norm = V4L2_STD_NTSC_M;
>> + return 0;
>> +}
>> +
>> +int vidioc_s_std(struct file *file, void *fh, v4l2_std_id a)
>> +{
>> + return 0;
>> +}
>> +
>> +static int
>> +vid_dev_pipeline_validate(struct video_device_dev *vid_dev)
>> +{
>> + return 0;
>> +}
>> +
>> +static int
>> +vid_dev_streamon(struct file *file, void *priv, enum v4l2_buf_type type)
>> +{
>> + struct video_device_dev *vid_dev = video_drvdata(file);
>> + struct media_entity *entity = &vid_dev->ve.vdev.entity;
>> + int ret;
>> +
>> + ret = media_entity_pipeline_start(entity, &vid_dev->ve.pipe->mp);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = vid_dev_pipeline_validate(vid_dev);
>> + if (ret < 0)
>> + goto err_p_stop;
>> +
>> + vb2_ioctl_streamon(file, priv, type);
>> + if (!ret)
>> + return ret;
>> +
>> +err_p_stop:
>> + media_entity_pipeline_stop(entity);
>> + return 0;
>> +}
>> +
>> +static int
>> +vid_dev_streamoff(struct file *file, void *priv, enum v4l2_buf_type type)
>> +{
>> + struct video_device_dev *vid_dev = video_drvdata(file);
>> + int ret;
>> +
>> + ret = vb2_ioctl_streamoff(file, priv, type);
>> + if (ret < 0)
>> + return ret;
>> +
>> + media_entity_pipeline_stop(&vid_dev->ve.vdev.entity);
>> + return 0;
>> +}
>> +
>> +static const struct v4l2_ioctl_ops vid_dev_ioctl_ops = {
>> + .vidioc_querycap = vidioc_querycap,
>> + .vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
>> + .vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap,
>> + .vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap,
>> + .vidioc_enum_framesizes = vidioc_enum_framesizes,
>> + .vidioc_enum_input = vidioc_enum_input,
>> + .vidioc_g_input = vidioc_g_input,
>> + .vidioc_s_input = vidioc_s_input,
>> +
>> + .vidioc_reqbufs = vb2_ioctl_reqbufs,
>> + .vidioc_create_bufs = vb2_ioctl_create_bufs,
>> + .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
>> + .vidioc_querybuf = vb2_ioctl_querybuf,
>> + .vidioc_qbuf = vb2_ioctl_qbuf,
>> + .vidioc_dqbuf = vb2_ioctl_dqbuf,
>> + .vidioc_streamon = vid_dev_streamon,
>> + .vidioc_streamoff = vid_dev_streamoff,
>> +};
>> +
>> +/* Capture subdev media entity operations */
>> +static int
>> +vid_dev_link_setup(struct media_entity *entity,
>> + const struct media_pad *local,
>> + const struct media_pad *remote, u32 flags)
>> +{
>> + return 0;
>> +}
>> +
>> +static const struct media_entity_operations vid_dev_subdev_media_ops = {
>> + .link_setup = vid_dev_link_setup,
>> +};
>> +
>> +static int
>> +vid_dev_open(struct file *file)
>> +{
>> + struct video_device_dev *vid_dev = video_drvdata(file);
>> + struct media_entity *me = &vid_dev->ve.vdev.entity;
>> + int ret;
>> +
>> + mutex_lock(&vid_dev->lock);
>> +
>> + ret = v4l2_fh_open(file);
>> + if (ret < 0)
>> + goto unlock;
>> +
>> + if (!v4l2_fh_is_singular_file(file))
>> + goto unlock;
>> +
>> + mutex_lock(&me->graph_obj.mdev->graph_mutex);
>> +
>> + ret = plat_ipk_pipeline_call(&vid_dev->ve, open, me, true);
>> + if (ret == 0)
>> + me->use_count++;
>> +
>> + mutex_unlock(&me->graph_obj.mdev->graph_mutex);
>> +
>> + if (!ret)
>> + goto unlock;
>> +
>> + v4l2_fh_release(file);
>> +unlock:
>> + mutex_unlock(&vid_dev->lock);
>> + return ret;
>> +}
>> +
>> +static int
>> +vid_dev_release(struct file *file)
>> +{
>> + struct video_device_dev *vid_dev = video_drvdata(file);
>> + struct media_entity *entity = &vid_dev->ve.vdev.entity;
>> +
>> + mutex_lock(&vid_dev->lock);
>> +
>> + if (v4l2_fh_is_singular_file(file)) {
>> + plat_ipk_pipeline_call(&vid_dev->ve, close);
>> + mutex_lock(&entity->graph_obj.mdev->graph_mutex);
>> + entity->use_count--;
>> + mutex_unlock(&entity->graph_obj.mdev->graph_mutex);
>> + }
>> +
>> + _vb2_fop_release(file, NULL);
>> +
>> + mutex_unlock(&vid_dev->lock);
>> + return 0;
>> +}
>> +
>> +static const struct v4l2_file_operations vid_dev_fops = {
>> + .owner = THIS_MODULE,
>> + .open = vid_dev_open,
>> + .release = vid_dev_release,
>> + .write = vb2_fop_write,
>> + .read = vb2_fop_read,
>> + .poll = vb2_fop_poll,
>> + .unlocked_ioctl = video_ioctl2,
>> + .mmap = vb2_fop_mmap,
>> +};
>> +
>> +/*
>> + * VideoBuffer2 operations
>> + */
>> +
>> +void fill_buffer(struct video_device_dev *dev, struct rx_buffer *buf,
>> + int buf_num, unsigned long flags)
>> +{
>> + int size = 0;
>> + void *vbuf = NULL;
>> +
>> + if (&buf->vb == NULL)
>> + return;
>> +
>> + size = vb2_plane_size(&buf->vb.vb2_buf, 0);
>> + vbuf = vb2_plane_vaddr(&buf->vb.vb2_buf, 0);
>> +
>> + if (vbuf) {
>> + spin_unlock_irqrestore(&dev->slock, flags);
>> +
>> + memcpy(vbuf, dev->dma_buf[buf_num].cpu_addr, size);
>> +
>> + spin_lock_irqsave(&dev->slock, flags);
>> +
>> + buf->vb.field = dev->format.fmt.pix.field;
>> + buf->vb.sequence++;
>> + buf->vb.vb2_buf.timestamp = ktime_get_ns();
>> + }
>> + vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
>> +}
>> +
>> +static void buffer_copy_process(void *param)
>> +{
>> + struct video_device_dev *dev = (struct video_device_dev *) param;
>> + unsigned long flags;
>> + struct dmaqueue *dma_q = &dev->vidq;
>> + struct rx_buffer *buf = NULL;
>> +
>> + spin_lock_irqsave(&dev->slock, flags);
>> +
>> + if (!list_empty(&dma_q->active)) {
>> + buf = list_entry(dma_q->active.next, struct rx_buffer, list);
>> + list_del(&buf->list);
>> + fill_buffer(dev, buf, dev->last_idx, flags);
>> + }
>> +
>> + spin_unlock_irqrestore(&dev->slock, flags);
>> +}
>> +
>> +static inline struct rx_buffer *to_rx_buffer(struct vb2_v4l2_buffer *vb2)
>> +{
>> + return container_of(vb2, struct rx_buffer, vb);
>> +}
>> +
>> +int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
>> + unsigned int *nplanes, unsigned int sizes[],
>> + struct device *alloc_devs[])
>> +{
>> + struct video_device_dev *dev = vb2_get_drv_priv(vq);
>> + unsigned long size = 0;
>> + int i;
>> +
>> + size = size_image(dev);
>> + if (size == 0)
>> + return -EINVAL;
>> +
>> + *nbuffers = N_BUFFERS;
>> +
>> + for (i = 0; i < N_BUFFERS; i++) {
>> + dev->dma_buf[i].cpu_addr = dma_alloc_coherent(&dev->pdev->dev,
>> + size_image(dev),
>> + &dev->
>> + dma_buf
>> + [i].dma_addr,
>> + GFP_KERNEL);
>> + }
>> +
>> + *nplanes = 1;
>> + sizes[0] = size;
>> +
>> + return 0;
>> +}
>> +
>> +int buffer_prepare(struct vb2_buffer *vb)
>> +{
>> + struct rx_buffer *buf = NULL;
>> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>> + int size = 0;
>> +
>> + if (vb == NULL) {
>> + pr_warn("%s:vb2_buffer is null\n", FUNC_NAME);
>> + return 0;
>> + }
>> +
>> + buf = to_rx_buffer(vbuf);
>> +
>> + size = vb2_plane_size(&buf->vb.vb2_buf, 0);
>> + vb2_set_plane_payload(&buf->vb.vb2_buf, 0, size);
>> +
>> + INIT_LIST_HEAD(&buf->list);
>> + return 0;
>> +}
>> +
>> +void buffer_queue(struct vb2_buffer *vb)
>> +{
>> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>> + struct video_device_dev *dev = NULL;
>> + struct rx_buffer *buf = NULL;
>> + struct dmaqueue *vidq = NULL;
>> + struct dma_async_tx_descriptor *desc;
>> + u32 flags;
>> +
>> + if (vb == NULL) {
>> + pr_warn("%s:vb2_buffer is null\n", FUNC_NAME);
>> + return;
>> + }
>> +
>> + dev = vb2_get_drv_priv(vb->vb2_queue);
>> + buf = to_rx_buffer(vbuf);
>> + vidq = &dev->vidq;
>> +
>> + flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
>> + dev->xt.dir = DMA_DEV_TO_MEM;
>> + dev->xt.src_sgl = false;
>> + dev->xt.dst_inc = false;
>> + dev->xt.dst_sgl = true;
>> + dev->xt.dst_start = dev->dma_buf[dev->idx].dma_addr;
>> +
>> + dev->last_idx = dev->idx;
>> + dev->idx++;
>> + if (dev->idx >= N_BUFFERS)
>> + dev->idx = 0;
>> +
>> + dev->xt.frame_size = 1;
>> + dev->sgl[0].size = bytes_per_line(dev);
>> + dev->sgl[0].icg = 0;
>> + dev->xt.numf = height(dev);
>> +
>> + desc = dmaengine_prep_interleaved_dma(dev->dma, &dev->xt, flags);
>> + if (!desc) {
>> + pr_err("Failed to prepare DMA transfer\n");
>> + vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
>> + return;
>> + }
>> +
>> + desc->callback = buffer_copy_process;
>> + desc->callback_param = dev;
>> +
>> + spin_lock(&dev->slock);
>> + list_add_tail(&buf->list, &vidq->active);
>> + spin_unlock(&dev->slock);
>> +
>> + dmaengine_submit(desc);
>> +
>> + if (vb2_is_streaming(&dev->vb_queue))
>> + dma_async_issue_pending(dev->dma);
>> +}
>> +
>> +int start_streaming(struct vb2_queue *vq, unsigned int count)
>> +{
>> + struct video_device_dev *dev = vb2_get_drv_priv(vq);
>> +
>> + dma_async_issue_pending(dev->dma);
>> +
>> + return 0;
>> +}
>> +
>> +void stop_streaming(struct vb2_queue *vq)
>> +{
>> + struct video_device_dev *dev = vb2_get_drv_priv(vq);
>> + struct dmaqueue *dma_q = &dev->vidq;
>> +
>> + /* Stop and reset the DMA engine. */
>> + dmaengine_terminate_all(dev->dma);
>> +
>> + while (!list_empty(&dma_q->active)) {
>> + struct rx_buffer *buf;
>> +
>> + buf = list_entry(dma_q->active.next, struct rx_buffer, list);
>> + if (buf) {
>> + list_del(&buf->list);
>> + vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
>> + }
>> + }
>> + list_del_init(&dev->vidq.active);
>> +}
>> +
>> +static const struct vb2_ops vb2_video_qops = {
>> + .queue_setup = queue_setup,
>> + .buf_prepare = buffer_prepare,
>> + .buf_queue = buffer_queue,
>> + .start_streaming = start_streaming,
>> + .stop_streaming = stop_streaming,
>> + .wait_prepare = vb2_ops_wait_prepare,
>> + .wait_finish = vb2_ops_wait_finish,
>> +};
>> +
>> +static int vid_dev_subdev_s_power(struct v4l2_subdev *sd, int on)
>> +{
>> + return 0;
>
> I think you could just omit defining the function if it does nothing. The
> same goes for core ops.
>
This is just here as a placeholder, since this is called in another driver. If
you really think I should remove it I'll have to adapt the other driver to be
prepared for non-existant functions.
>> +}
>> +
>> +static int vid_dev_subdev_registered(struct v4l2_subdev *sd)
>> +{
>> + struct video_device_dev *vid_dev = v4l2_get_subdevdata(sd);
>> + struct vb2_queue *q = &vid_dev->vb_queue;
>> + struct video_device *vfd = &vid_dev->ve.vdev;
>> + int ret;
>> +
>> + memset(vfd, 0, sizeof(*vfd));
>> +
>> + snprintf(vfd->name, sizeof(vfd->name), VIDEO_DEVICE_NAME);
>
> How about strlcpy()?
>
>> +
>> + vfd->fops = &vid_dev_fops;
>> + vfd->ioctl_ops = &vid_dev_ioctl_ops;
>> + vfd->v4l2_dev = sd->v4l2_dev;
>> + vfd->minor = -1;
>> + vfd->release = video_device_release_empty;
>> + vfd->queue = q;
>> +
>> + INIT_LIST_HEAD(&vid_dev->vidq.active);
>> + init_waitqueue_head(&vid_dev->vidq.wq);
>> + memset(q, 0, sizeof(*q));
>> + q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> + q->io_modes = VB2_MMAP | VB2_USERPTR;
>> + q->ops = &vb2_video_qops;
>> + q->mem_ops = &vb2_vmalloc_memops;
>> + q->buf_struct_size = sizeof(struct rx_buffer);
>> + q->drv_priv = vid_dev;
>> + q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>> + q->lock = &vid_dev->lock;
>> +
>> + ret = vb2_queue_init(q);
>> + if (ret < 0)
>> + return ret;
>> +
>> + vid_dev->vd_pad.flags = MEDIA_PAD_FL_SINK;
>> + ret = media_entity_pads_init(&vfd->entity, 1, &vid_dev->vd_pad);
>> + if (ret < 0)
>> + return ret;
>> +
>> + video_set_drvdata(vfd, vid_dev);
>> + vid_dev->ve.pipe = v4l2_get_subdev_hostdata(sd);
>> +
>> + ret = video_register_device(vfd, VFL_TYPE_GRABBER, -1);
>> + if (ret < 0) {
>> + media_entity_cleanup(&vfd->entity);
>> + vid_dev->ve.pipe = NULL;
>> + return ret;
>> + }
>> +
>> + v4l2_info(sd->v4l2_dev, "Registered %s as /dev/%s\n",
>> + vfd->name, video_device_node_name(vfd));
>> + return 0;
>> +}
>> +
>> +static void vid_dev_subdev_unregistered(struct v4l2_subdev *sd)
>> +{
>> + struct video_device_dev *vid_dev = v4l2_get_subdevdata(sd);
>> +
>> + if (vid_dev == NULL)
>> + return;
>
> Can this happen?
>
It's the same as before, a better safe than sorry situation. But if you think it
can't happen I'll remove it.
>> +
>> + mutex_lock(&vid_dev->lock);
>> +
>> + if (video_is_registered(&vid_dev->ve.vdev)) {
>> + video_unregister_device(&vid_dev->ve.vdev);
>> + media_entity_cleanup(&vid_dev->ve.vdev.entity);
>> + vid_dev->ve.pipe = NULL;
>> + }
>> +
>> + mutex_unlock(&vid_dev->lock);
>> +}
>> +
>> +static const struct v4l2_subdev_internal_ops vid_dev_subdev_internal_ops = {
>> + .registered = vid_dev_subdev_registered,
>> + .unregistered = vid_dev_subdev_unregistered,
>> +};
>> +
>> +static const struct v4l2_subdev_core_ops vid_dev_subdev_core_ops = {
>> + .s_power = vid_dev_subdev_s_power,
>> +};
>> +
>> +static struct v4l2_subdev_ops vid_dev_subdev_ops = {
>> + .core = &vid_dev_subdev_core_ops,
>> +};
>> +
>> +static int vid_dev_create_capture_subdev(struct video_device_dev *vid_dev)
>> +{
>> + struct v4l2_subdev *sd = &vid_dev->subdev;
>> + int ret;
>> +
>> + v4l2_subdev_init(sd, &vid_dev_subdev_ops);
>> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> + snprintf(sd->name, sizeof(sd->name), "Capture device");
>> +
>> + vid_dev->subdev_pads[VIDEO_DEV_SD_PAD_SINK_CSI].flags =
>> + MEDIA_PAD_FL_SOURCE;
>> + vid_dev->subdev_pads[VIDEO_DEV_SD_PAD_SOURCE_DMA].flags =
>> + MEDIA_PAD_FL_SINK;
>> + ret =
>
> No need for a line break.
>
Right.
>> + media_entity_pads_init(&sd->entity, VIDEO_DEV_SD_PADS_NUM,
>> + vid_dev->subdev_pads);
>> + if (ret)
>> + return ret;
>> +
>> + sd->internal_ops = &vid_dev_subdev_internal_ops;
>> + sd->entity.ops = &vid_dev_subdev_media_ops;
>> + sd->owner = THIS_MODULE;
>> + v4l2_set_subdevdata(sd, vid_dev);
>> +
>> + return 0;
>> +}
>> +
>> +static void vid_dev_unregister_subdev(struct video_device_dev *vid_dev)
>> +{
>> + struct v4l2_subdev *sd = &vid_dev->subdev;
>> +
>> + v4l2_device_unregister_subdev(sd);
>> + media_entity_cleanup(&sd->entity);
>> + v4l2_set_subdevdata(sd, NULL);
>> +}
>> +
>> +static const struct of_device_id vid_dev_of_match[];
>> +
>> +static int vid_dev_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + const struct of_device_id *of_id;
>> + int ret = 0;
>> + struct video_device_dev *vid_dev;
>> +
>> + dev_info(dev, "Installing IPK Video Device module\n");
>> +
>> + if (!dev->of_node)
>> + return -ENODEV;
>> +
>> + vid_dev = devm_kzalloc(dev, sizeof(*vid_dev), GFP_KERNEL);
>> + if (!vid_dev)
>> + return -ENOMEM;
>> +
>> + of_id = of_match_node(vid_dev_of_match, dev->of_node);
>> + if (WARN_ON(of_id == NULL))
>> + return -EINVAL;
>> +
>> + vid_dev->pdev = pdev;
>> +
>> + spin_lock_init(&vid_dev->slock);
>> + mutex_init(&vid_dev->lock);
>> +
>> + dev_info(&pdev->dev, "Requesting DMA\n");
>> + vid_dev->dma = dma_request_slave_channel(&pdev->dev, "vdma0");
>> + if (vid_dev->dma == NULL) {
>> + dev_err(&pdev->dev, "no VDMA channel found\n");
>> + ret = -ENODEV;
>> + goto end;
>> + }
>> +
>> + ret = vid_dev_create_capture_subdev(vid_dev);
>> + if (ret)
>> + goto end;
>> +
>> + platform_set_drvdata(pdev, vid_dev);
>> +
>> + dev_info(dev, "Video Device registered successfully\n");
>> + return 0;
>> +end:
>> + dev_err(dev, "Video Device not registered!!\n");
>> + return ret;
>> +}
>> +
>> +static int vid_dev_remove(struct platform_device *pdev)
>> +{
>> + struct video_device_dev *dev = platform_get_drvdata(pdev);
>> +
>> + vid_dev_unregister_subdev(dev);
>> + dev_info(&pdev->dev, "Driver removed\n");
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id vid_dev_of_match[] = {
>> + {.compatible = "snps,video-device"},
>> + {}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, vid_dev_of_match);
>> +
>> +static struct platform_driver __refdata vid_dev_pdrv = {
>> + .remove = vid_dev_remove,
>> + .probe = vid_dev_probe,
>> + .driver = {
>> + .name = VIDEO_DEVICE_NAME,
>> + .owner = THIS_MODULE,
>> + .of_match_table = vid_dev_of_match,
>> + },
>> +};
>> +
>> +module_platform_driver(vid_dev_pdrv);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Ramiro Oliveira <roliveir@synopsys.com>");
>> +MODULE_DESCRIPTION("Driver for configuring DMA and Video Device");
>> diff --git a/drivers/media/platform/dwc/video_device.h b/drivers/media/platform/dwc/video_device.h
>> new file mode 100644
>> index 0000000..e828d4b
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/video_device.h
>> @@ -0,0 +1,101 @@
>> +/*
>> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef VIDEO_DEVICE_H_
>> +#define VIDEO_DEVICE_H_
>> +
>> +#include <linux/module.h>
>> +#include <linux/errno.h>
>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/delay.h>
>> +#include <linux/list.h>
>> +#include <linux/wait.h>
>> +#include <linux/string.h>
>> +#include <linux/videodev2.h>
>> +#include <linux/dma/xilinx_dma.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-ioctl.h>
>> +#include <media/v4l2-fh.h>
>> +#include <media/v4l2-common.h>
>> +#include <media/videobuf2-vmalloc.h>
>> +#include <media/media-entity.h>
>> +#include <linux/io.h>
>> +
>> +#include "plat_ipk_video.h"
>> +
>> +#define N_BUFFERS 3
>> +
>> +#define VIDEO_DEVICE_NAME "video-device"
>> +
>> +#define FUNC_NAME __func__
>> +
>> +struct rx_buffer {
>> + /** @short Buffer for video frames */
>> + struct vb2_v4l2_buffer vb;
>> + struct list_head list;
>> +
>> + dma_addr_t dma_addr;
>> + void *cpu_addr;
>> +};
>> +
>> +struct dmaqueue {
>> + struct list_head active;
>> + wait_queue_head_t wq;
>> +};
>> +
>> +/**
>> + * @short Structure to embed device driver information
>> + */
>> +struct video_device_dev {
>> + struct platform_device *pdev;
>> + struct v4l2_device *v4l2_dev;
>> + struct v4l2_subdev subdev;
>> + struct media_pad vd_pad;
>> + struct media_pad subdev_pads[VIDEO_DEV_SD_PADS_NUM];
>> + struct mutex lock;
>> + spinlock_t slock;
>> + struct plat_ipk_video_entity ve;
>> + struct v4l2_format format;
>> + struct v4l2_pix_format pix_format;
>> + const struct plat_ipk_fmt *fmt;
>> + unsigned long *alloc_ctx;
>> +
>> + /* Buffer and DMA */
>> + struct vb2_queue vb_queue;
>> + int idx;
>> + int last_idx;
>> + struct dmaqueue vidq;
>> + struct rx_buffer dma_buf[N_BUFFERS];
>> + struct dma_chan *dma;
>> + struct dma_interleaved_template xt;
>> + struct data_chunk sgl[1];
>> +};
>> +
>> +/**
>> + * @short Defines to simplify the code reading
>> + */
>> +
>> +#define pixel_format(dev) \
>> + dev->format.fmt.pix.pixelformat
>> +#define bytes_per_line(dev) \
>> + dev->format.fmt.pix.bytesperline
>> +#define width(dev) \
>> + dev->format.fmt.pix.width
>> +#define height(dev) \
>> + dev->format.fmt.pix.height
>> +#define size_image(dev) \
>> + dev->format.fmt.pix.sizeimage
>
> How about referring to the field directly, or define a local variable for a
> pointer to dev->format.fmt.fix in functions it's needed.
>
Sure, I can do that.
Just to be clear what is the reason why these macros shouldn't be used?
>> +
>> +const struct plat_ipk_fmt *vid_dev_find_format(struct v4l2_format *f,
>> + int index);
>> +
>> +#endif /* VIDEO_DEVICE_H_ */
>
Thanks once again for you feedback.
BRs,
Ramiro Oliveira
^ permalink raw reply
* Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Sudeep Holla @ 2016-11-21 17:56 UTC (permalink / raw)
To: Robin Murphy, Bartosz Golaszewski, Sekhar Nori
Cc: Sudeep Holla, Mark Rutland, linux-devicetree, Tomi Valkeinen,
David Airlie, Kevin Hilman, Michael Turquette, Russell King,
linux-drm, LKML, Rob Herring, Jyri Sarha, Frank Rowand, arm-soc,
Laurent Pinchart
In-Reply-To: <d0f3bc66-6dfd-dcdc-a15d-a8f9fdda6048-5wv7dgnIgG8@public.gmane.org>
Hi Robin,
On 21/11/16 17:47, Robin Murphy wrote:
> Hi Bartosz, Sekhar,
>
> On 21/11/16 16:48, Bartosz Golaszewski wrote:
[...]
>> Hi Sekhar,
>>
>> thanks for spotting that.
>>
>> I think we should introduce this function right away, rather than
>> having two static functions doing the same thing. If you don't mind,
>> I'll try to find a good spot for it and send a follow-up series fixing
>> the issue.
>
> As it happens, that was already proposed last week, for much the same
> reason:
>
> http://www.mail-archive.com/linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org/msg111395.html
>
Thanks for pointing this out, yet another platform to move to the new
API after v4.10.
Hi Shekar, Bartosz,
For v4.10, please continue with the open coding as proposed in this
thread in order to avoid cross tree dependencies. I will rework on the
above patch once v4.10 merge window closes to include all these
occurrence and replace them.
--
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Robin Murphy @ 2016-11-21 17:47 UTC (permalink / raw)
To: Bartosz Golaszewski, Sekhar Nori
Cc: Mark Rutland, linux-devicetree, Tomi Valkeinen, David Airlie,
Kevin Hilman, Michael Turquette, Russell King, linux-drm, LKML,
Rob Herring, Jyri Sarha, Frank Rowand, arm-soc, Laurent Pinchart
In-Reply-To: <CAMpxmJUXJi6PDq0qc-0+r2mLPASLpJUt_njWtXr4Mx4k0Fa82g@mail.gmail.com>
Hi Bartosz, Sekhar,
On 21/11/16 16:48, Bartosz Golaszewski wrote:
> 2016-11-21 17:33 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
>> On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote:
>>> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>> +{
>>> + const struct da8xx_ddrctl_config_knob *knob;
>>> + const struct da8xx_ddrctl_setting *setting;
>>> + struct device_node *node;
>>> + struct resource *res;
>>> + void __iomem *ddrctl;
>>> + struct device *dev;
>>> + u32 reg;
>>> +
>>> + dev = &pdev->dev;
>>> + node = dev->of_node;
>>> +
>>> + setting = da8xx_ddrctl_get_board_settings();
>>> + if (!setting) {
>>> + dev_err(dev, "no settings for board '%s'\n",
>>> + of_flat_dt_get_machine_name());
>>> + return -EINVAL;
>>> + }
>>
>> This causes a section mismatch because of_flat_dt_get_machine_name()
>> has an __init annotation. I did not notice that before, sorry.
>>
>> It can be fixed with a patch like below:
>>
>> ---8<---
>> diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
>> index a20e7bbbcbe0..9ca5aab3ac54 100644
>> --- a/drivers/memory/da8xx-ddrctl.c
>> +++ b/drivers/memory/da8xx-ddrctl.c
>> @@ -102,6 +102,18 @@ static const struct da8xx_ddrctl_setting *da8xx_ddrctl_get_board_settings(void)
>> return NULL;
>> }
>>
>> +static const char* da8xx_ddrctl_get_machine_name(void)
>> +{
>> + const char *str;
>> + int ret;
>> +
>> + ret = of_property_read_string(of_root, "model", &str);
>> + if (ret)
>> + ret = of_property_read_string(of_root, "compatible", &str);
>> +
>> + return str;
>> +}
>> +
>> static int da8xx_ddrctl_probe(struct platform_device *pdev)
>> {
>> const struct da8xx_ddrctl_config_knob *knob;
>> @@ -118,7 +130,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev)
>> setting = da8xx_ddrctl_get_board_settings();
>> if (!setting) {
>> dev_err(dev, "no settings for board '%s'\n",
>> - of_flat_dt_get_machine_name());
>> + da8xx_ddrctl_get_machine_name());
>> return -EINVAL;
>> }
>> ---8<---
>>
>> A similar fix is required for the other driver in this series (patch
>> 2/5). I need some advise on whether I should introduce a common
>> function to get the machine name post kernel boot-up (I cannot see an
>> existing one). If yes, any advise on which file it should go into?
>>
>
> Hi Sekhar,
>
> thanks for spotting that.
>
> I think we should introduce this function right away, rather than
> having two static functions doing the same thing. If you don't mind,
> I'll try to find a good spot for it and send a follow-up series fixing
> the issue.
As it happens, that was already proposed last week, for much the same
reason:
http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg111395.html
Robin.
>
> Best regards,
> Bartosz Golaszewski
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: display: Add Sharp LQ150X1LG11 panel binding
From: Rob Herring @ 2016-11-21 17:38 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Thierry Reding, David Airlie,
Mark Rutland, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479740449-20201-2-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On Mon, Nov 21, 2016 at 04:00:48PM +0100, Peter Rosin wrote:
> The Sharp 15" LQ150X1LG11 panel is an XGA TFT LCD panel.
>
> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
> ---
> .../bindings/display/panel/sharp,lq150x1lg11.txt | 36 ++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] ARM: dts: AM571x-IDK Initial Support
From: Rob Herring @ 2016-11-21 17:36 UTC (permalink / raw)
To: Lokesh Vutla
Cc: Tony Lindgren, Linux OMAP Mailing List, Tero Kristo, Sekhar Nori,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux ARM Mailing List,
spatton-l0cyMroinI0, Dave Gerlach, Nishanth Menon
In-Reply-To: <20161121055801.573-1-lokeshvutla-l0cyMroinI0@public.gmane.org>
On Mon, Nov 21, 2016 at 11:28:01AM +0530, Lokesh Vutla wrote:
> From: Schuyler Patton <spatton-l0cyMroinI0@public.gmane.org>
>
> The AM571x-IDK board is a board based on TI's AM5718 SOC
> which has a single core 1.5GHz A15 processor. This board is a
> development platform for the Industrial market with:
> - 1GB of DDR3L
> - Dual 1Gbps Ethernet
> - HDMI,
> - PRU-ICSS
> - uSD
> - 16GB eMMC
> - CAN
> - RS-485
> - PCIe
> - USB3.0
> - Video Input Port
> - Industrial IO port and expansion connector
>
> The link to the data sheet and TRM can be found here:
>
> http://www.ti.com/product/AM5718
>
> Initial support is only for basic peripherals.
>
> Signed-off-by: Schuyler Patton <spatton-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
> ---
>
> Logs: http://pastebin.ubuntu.com/23510390/
>
> .../devicetree/bindings/arm/omap/omap.txt | 3 +
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/am571x-idk.dts | 82 ++++++++++++++++++++++
> 3 files changed, 86 insertions(+)
> create mode 100644 arch/arm/boot/dts/am571x-idk.dts
>
> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
> index f53e2ee..647ffd3 100644
> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
> @@ -175,6 +175,9 @@ Boards:
> - AM5728 IDK
> compatible = "ti,am5728-idk", "ti,am5728", "ti,dra742", "ti,dra74", "ti,dra7"
>
> +- AM5718 IDK
> + compatible = "ti,am5718-idk", "ti,am5728", "ti,dra722", "ti,dra72", "ti,dra7"
I've said this before I think, but 5 compat string is a bit much. Some
of these genericish ones should be dropped. Doesn't really hurt though.
A couple of nits below, otherwise:
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> +
> - DRA742 EVM: Software Development Board for DRA742
> compatible = "ti,dra7-evm", "ti,dra742", "ti,dra74", "ti,dra7"
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index befcd26..c298078 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -588,6 +588,7 @@ dtb-$(CONFIG_SOC_DRA7XX) += \
> am57xx-cl-som-am57x.dtb \
> am57xx-sbc-am57x.dtb \
> am572x-idk.dtb \
> + am571x-idk.dtb \
> dra7-evm.dtb \
> dra72-evm.dtb \
> dra72-evm-revc.dtb
> diff --git a/arch/arm/boot/dts/am571x-idk.dts b/arch/arm/boot/dts/am571x-idk.dts
> new file mode 100644
> index 0000000..a6a743e
> --- /dev/null
> +++ b/arch/arm/boot/dts/am571x-idk.dts
> @@ -0,0 +1,82 @@
> +/*
> + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +/dts-v1/;
> +
> +#include "dra72x.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include "am57xx-idk-common.dtsi"
> +
> +/ {
> + model = "TI AM5718 IDK";
> + compatible = "ti,am5718-idk", "ti,am5718", "ti,dra722",
> + "ti,dra72", "ti,dra7";
> +
> + memory@0 {
unit address is wrong.
> + device_type = "memory";
> + reg = <0x0 0x80000000 0x0 0x40000000>;
> + };
> +
> + status-leds {
Just "leds"
> + compatible = "gpio-leds";
> + cpu0-led {
> + label = "status0:red:cpu0";
> + gpios = <&gpio2 25 GPIO_ACTIVE_HIGH>;
> + default-state = "off";
> + linux,default-trigger = "cpu0";
> + };
> +
> + usr0-led {
> + label = "status0:green:usr";
> + gpios = <&gpio2 26 GPIO_ACTIVE_HIGH>;
> + default-state = "off";
> + };
> +
> + heartbeat-led {
> + label = "status0:blue:heartbeat";
> + gpios = <&gpio2 27 GPIO_ACTIVE_HIGH>;
> + default-state = "off";
> + linux,default-trigger = "heartbeat";
> + };
> +
> + usr1-led {
> + label = "status1:red:usr";
> + gpios = <&gpio2 28 GPIO_ACTIVE_HIGH>;
> + default-state = "off";
> + };
> +
> + usr2-led {
> + label = "status1:green:usr";
> + gpios = <&gpio2 21 GPIO_ACTIVE_HIGH>;
> + default-state = "off";
> + };
> +
> + mmc0-led {
> + label = "status1:blue:mmc0";
> + gpios = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> + default-state = "off";
> + linux,default-trigger = "mmc0";
> + };
> + };
> +
> + extcon_usb2: extcon_usb2 {
> + compatible = "linux,extcon-usb-gpio";
> + id-gpio = <&gpio5 7 GPIO_ACTIVE_HIGH>;
> + };
> +};
> +
> +&mmc1 {
> + status = "okay";
> + vmmc-supply = <&ldo1_reg>;
> + bus-width = <4>;
> + cd-gpios = <&gpio6 27 0>; /* gpio 219 */
> +};
> +
> +&omap_dwc3_2 {
> + extcon = <&extcon_usb2>;
> +};
> --
> 2.10.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] spi: sh-msiof: Add support for R-Car M3-W
From: Geert Uytterhoeven @ 2016-11-21 17:24 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Mark Rutland
Cc: linux-spi, devicetree, linux-renesas-soc, Geert Uytterhoeven
MSIOF in R-Car M3-W (r8a7796) is handled fine by the existing driver.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Tested with MSIOF2(B) on EXIO connector D of r8a7796/salvator-x, using
spidev, gpio-74x164, and a logic analyzer.
Documentation/devicetree/bindings/spi/sh-msiof.txt | 1 +
drivers/spi/spi-sh-msiof.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
index aa005c1d10d95756..da6614c6379604bb 100644
--- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
+++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
@@ -10,6 +10,7 @@ Required properties:
"renesas,msiof-r8a7792" (R-Car V2H)
"renesas,msiof-r8a7793" (R-Car M2-N)
"renesas,msiof-r8a7794" (R-Car E2)
+ "renesas,msiof-r8a7796" (R-Car M3-W)
"renesas,msiof-sh73a0" (SH-Mobile AG5)
- reg : A list of offsets and lengths of the register sets for
the device.
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 1de3a772eb7d23a8..0012ad02e5696d35 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -980,6 +980,7 @@ static int sh_msiof_transfer_one(struct spi_master *master,
{ .compatible = "renesas,msiof-r8a7792", .data = &r8a779x_data },
{ .compatible = "renesas,msiof-r8a7793", .data = &r8a779x_data },
{ .compatible = "renesas,msiof-r8a7794", .data = &r8a779x_data },
+ { .compatible = "renesas,msiof-r8a7796", .data = &r8a779x_data },
{},
};
MODULE_DEVICE_TABLE(of, sh_msiof_match);
--
1.9.1
^ permalink raw reply related
* Re: [PATCHv2] arm64: dts: exynos: add the mshc_2 node for supporting T-Flash
From: Krzysztof Kozlowski @ 2016-11-21 17:21 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, kgene-DgEjT+Ai2ygdnm+yROfE0A,
krzk-DgEjT+Ai2ygdnm+yROfE0A, cw00.choi-Sze3O3UU22JBDgjK7y7TUQ,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <20161121045839.1444-1-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Mon, Nov 21, 2016 at 01:58:39PM +0900, Jaehoon Chung wrote:
> Add the mshc_2 node for supporting T-flash.
>
> And it needs to add the "mshc*" aliases. Because dwmmc driver should be
> assigned to "ctrl_id" after parsing to "mshc".
> If there is no aliases for mshc, then it might be set to the wrong
> capabilities.
>
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Changelog on V2:
> - Changed from 0 to GPIO_ACTIVE_HIGH
Thanks, applied.
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 1/3] Documentation: DT: add dma compatible for sun50i A64 SOC
From: Rob Herring @ 2016-11-21 17:12 UTC (permalink / raw)
To: Hao Zhang
Cc: maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
vinod.koul-ral2JQCrhuEAvxtiuMwx3w, mark.rutland-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
dmaengine-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1479638740-20520-2-git-send-email-hao5781286-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Sun, Nov 20, 2016 at 06:45:38PM +0800, Hao Zhang wrote:
> This add the property of Allwinner sun50i A64 dma.
>
> Signed-off-by: Hao Zhang <hao5781286-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> Documentation/devicetree/bindings/dma/sun6i-dma.txt | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/2] ARM64: dts: Add support for Meson GXM
From: Kevin Hilman @ 2016-11-21 17:10 UTC (permalink / raw)
To: Neil Armstrong
Cc: carlo-KA+7E9HrN00dnm+yROfE0A,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161121162905.14285-1-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:
> The new Amlogic GXM SoC (S912) is part of the Meson GX family and is nearly
> identical to GXM but with a second Quad-A53 core cluster.
>
> The GXM dtsi includes the GXL dtsi and the p20x dtsi is refactored in a
> common p20x/q20x to support the GXM Q200 and Q201 board that uses the exact
> same board layout since the S905D and S912 are pinout compatible.
>
> The last patch adds support for the Nexbox A1 Set-Top-Box based on the S912.
>
> Changes since RFC :
> - Refactor the p20x/q20x dtsi into a single common file
> - Add support for Nexbox A1
Thanks for refactoring. Looks great.
Applied.
Kevin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 2/5] drm: sun8i: add HDMI video support to A83T and H3
From: Rob Herring @ 2016-11-21 17:09 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: Dave Airlie, Maxime Ripard, devicetree-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <cea6a5397664213c6a917028ec2e429d25972490.1479641523.git.moinejf-GANU6spQydw@public.gmane.org>
On Sun, Nov 20, 2016 at 10:56:23AM +0100, Jean-Francois Moine wrote:
> This patch adds a HDMI video driver to the Allwinner's SoCs A83T and H3.
>
> Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
> ---
> .../devicetree/bindings/display/sunxi/hdmi.txt | 53 ++
> drivers/gpu/drm/sun8i/Kconfig | 7 +
> drivers/gpu/drm/sun8i/Makefile | 2 +
> drivers/gpu/drm/sun8i/de2_hdmi.c | 394 ++++++++++
> drivers/gpu/drm/sun8i/de2_hdmi.h | 51 ++
> drivers/gpu/drm/sun8i/de2_hdmi_io.c | 839 +++++++++++++++++++++
> 6 files changed, 1346 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi.c
> create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi.h
> create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi_io.c
>
> diff --git a/Documentation/devicetree/bindings/display/sunxi/hdmi.txt b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> new file mode 100644
> index 0000000..85709ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> @@ -0,0 +1,53 @@
> +Allwinner HDMI Transmitter
> +==========================
> +
> +The Allwinner HDMI transmitters are included in the SoCs.
> +They support audio and video.
> +
> +Required properties:
> + - #address-cells : should be <1>
> + - #size-cells : should be <0>
> + - compatible : should be one of
> + "allwinner,sun8i-a83t-hdmi"
> + "allwinner,sun8i-h3-hdmi"
> + - clocks : phandles to the HDMI clocks as described in
> + Documentation/devicetree/bindings/clock/clock-bindings.txt
> + - clock-names : must be
> + "gate" : bus gate
> + "clock" : streaming clock
> + "ddc-clock" : DDC clock
> + - resets : One or two phandles to the HDMI resets
> + - reset-names : when 2 phandles, must be
> + "hdmi0" and "hdmi1"
> +
> +Required nodes:
> + - port: Audio and video input port nodes with endpoint definitions
> + as defined in Documentation/devicetree/bindings/graph.txt.
> + port@0 is video and port@1 is audio.
This should probably also have an output port to the hdmi-connector
binding. It is not needed so much if this block handles DDC and HPD
itself, but if those are a separate I2C controller and GPIO,
respectively, then you need it for sure. There's also power on the
connector or other connectors like muxed on Type-C.
> +
> +Example:
> +
> + hdmi: hdmi@01ee0000 {
> + compatible = "allwinner,sun8i-a83t-hdmi";
> + reg = <0x01ee0000 0x20000>;
> + clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>,
> + <&ccu CLK_HDMI_DDC>;
> + clock-names = "gate", "clock", "ddc-clock";
> + resets = <&ccu RST_HDMI0>, <&ccu RST_HDMI1>;
> + reset-names = "hdmi0", "hdmi1";
> + ...
Please show all properties in example.
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 { /* video */
> + reg = <0>;
> + hdmi_lcd1: endpoint {
> + remote-endpoint = <&lcd1_hdmi>;
> + };
> + };
> + port@1 { /* audio */
> + reg = <1>;
> + hdmi_i2s2: endpoint {
> + remote-endpoint = <&i2s2_hdmi>;
> + };
> + };
> + };
^ permalink raw reply
* Re: [PATCH 1/2] ARM64: dts: Add support for Meson GXM
From: Kevin Hilman @ 2016-11-21 17:06 UTC (permalink / raw)
To: Neil Armstrong
Cc: carlo-KA+7E9HrN00dnm+yROfE0A,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161121162905.14285-2-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:
> Following the Amlogic Linux kernel, it seem the only differences
> between the GXL and GXM SoCs are the CPU Clusters.
>
> This commit renames the gxl-s905d-p23x DTSI in a common file for
> S905D p20x and S912 q20x boards.
>
> Then adds a meson-gxm dtsi and reproduce the P23x to Q20x boards
> dts files since the S905D and S912 SoCs shares the same pinout
> and the P23x and Q20x boards are identical.
>
> Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
> Documentation/devicetree/bindings/arm/amlogic.txt | 6 +
> arch/arm64/boot/dts/amlogic/Makefile | 2 +
> .../arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi | 190 +++++++++++++++++++++
> arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts | 19 +++
> arch/arm64/boot/dts/amlogic/meson-gxbb-p201.dts | 7 +
> .../boot/dts/amlogic/meson-gxl-s905d-p230.dts | 3 +-
> .../boot/dts/amlogic/meson-gxl-s905d-p231.dts | 3 +-
> .../boot/dts/amlogic/meson-gxl-s905d-p23x.dtsi | 188 --------------------
> .../arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts | 77 +++++++++
> .../arm64/boot/dts/amlogic/meson-gxm-s912-q201.dts | 58 +++++++
> arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 114 +++++++++++++
> 11 files changed, 477 insertions(+), 190 deletions(-)
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
> delete mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p23x.dtsi
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm-s912-q201.dts
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
Applied.
Git tip: for future reference, set 'git config diff.renames true' so and
the diffstat shows a better summary when files are moved/renamed.
e.g, rather than the large ++ and -- above, you'd see:
...gxl-s905d-p23x.dtsi => meson-gx-p23x-q20x.dtsi} | 4 +-
Which is much more comforting to a maintainer than a bunch of
(potentially unrelated) adds and removes.
Thanks,
Kevin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] ARM: dts: exynos: remove the cd-gpios property for eMMC of odroid-xu3/4
From: Krzysztof Kozlowski @ 2016-11-21 17:06 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, kgene-DgEjT+Ai2ygdnm+yROfE0A,
krzk-DgEjT+Ai2ygdnm+yROfE0A, cw00.choi-Sze3O3UU22JBDgjK7y7TUQ,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
a.hajda-Sze3O3UU22JBDgjK7y7TUQ,
jy0922.shim-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <20161121071032.10183-1-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Mon, Nov 21, 2016 at 04:10:32PM +0900, Jaehoon Chung wrote:
> Odroid-xu3/4 didn't need to use the cd-gpios for detecting card.
> Because Host controller has the CDETECT register through SDx_CDN line.
> Host controller can know whether card is inserted or not with this
> register.
>
> When i have checked the Odroid-xu3/4, they are using CDETECT register.
> (Not using exteranl cd-gpio.)
Makes sense. Just one question: the sd0_cd pinctrl setting should stay,
right?
Best regards,
Krzysztof
> Fixes: fb1aeedb61ad ("ARM: dts: add mmc detect gpio for exynos5422-odroidxu3")
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> index 9e63328..05b9afdd 100644
> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> @@ -510,7 +510,6 @@
> &mmc_0 {
> status = "okay";
> mmc-pwrseq = <&emmc_pwrseq>;
> - cd-gpios = <&gpc0 2 GPIO_ACTIVE_LOW>;
> card-detect-delay = <200>;
> samsung,dw-mshc-ciu-div = <3>;
> samsung,dw-mshc-sdr-timing = <0 4>;
> --
> 2.10.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 1/5] drm: sun8i: Add a basic DRM driver for Allwinner DE2
From: Rob Herring @ 2016-11-21 16:59 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: devicetree, dri-devel, linux-sunxi, Maxime Ripard,
linux-arm-kernel
In-Reply-To: <ff2140c86ea3b06406c77d9e8746474b6f8400c4.1479641523.git.moinejf@free.fr>
On Sun, Nov 20, 2016 at 10:53:25AM +0100, Jean-Francois Moine wrote:
> Allwinner's recent SoCs, as A64, A83T and H3, contain a new display
> engine, DE2.
> This patch adds a DRM video driver for this device.
>
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
> .../bindings/display/sunxi/sun8i-de2.txt | 83 +++
It's preferred to split bindings to a separate patch.
> drivers/gpu/drm/Kconfig | 2 +
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/sun8i/Kconfig | 19 +
> drivers/gpu/drm/sun8i/Makefile | 7 +
> drivers/gpu/drm/sun8i/de2_crtc.c | 440 +++++++++++++
> drivers/gpu/drm/sun8i/de2_crtc.h | 50 ++
> drivers/gpu/drm/sun8i/de2_drm.h | 48 ++
> drivers/gpu/drm/sun8i/de2_drv.c | 379 +++++++++++
> drivers/gpu/drm/sun8i/de2_plane.c | 712 +++++++++++++++++++++
> 10 files changed, 1741 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> create mode 100644 drivers/gpu/drm/sun8i/Kconfig
> create mode 100644 drivers/gpu/drm/sun8i/Makefile
> create mode 100644 drivers/gpu/drm/sun8i/de2_crtc.c
> create mode 100644 drivers/gpu/drm/sun8i/de2_crtc.h
> create mode 100644 drivers/gpu/drm/sun8i/de2_drm.h
> create mode 100644 drivers/gpu/drm/sun8i/de2_drv.c
> create mode 100644 drivers/gpu/drm/sun8i/de2_plane.c
>
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> new file mode 100644
> index 0000000..b9edd4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> @@ -0,0 +1,83 @@
> +Allwinner sun8i Display Engine 2 subsystem
> +==========================================
> +
> +The Allwinner DE2 subsystem contains a display controller (DE2),
> +one or two LCD controllers (TCON) and their external interfaces.
> +
> +Display controller
> +==================
> +
> +Required properties:
> +
> +- compatible: value should be one of the following
> + "allwinner,sun8i-a83t-display-engine"
> + "allwinner,sun8i-h3-display-engine"
> +
> +- clocks: must include clock specifiers corresponding to entries in the
> + clock-names property.
> +
> +- clock-names: must contain
> + "gate": DE bus gate
> + "clock": DE clock
> +
> +- resets: phandle to the reset of the device
> +
> +- ports: phandle's to the LCD ports
This should use OF graph to describe the connection from the DE to the
LCD controllers like the sun4i binding does.
No registers for the DE?
> +
> +LCD controller
> +==============
> +
> +Required properties:
> +
> +- compatible: should be
> + "allwinner,sun8i-a83t-tcon"
> +
> +- clocks: must include clock specifiers corresponding to entries in the
> + clock-names property.
> +
> +- clock-names: must contain
> + "gate": TCON bus gate
> + "clock": TCON pixel clock
> +
> +- resets: phandle to the reset of the device
> +
> +- port: port node with endpoint definitions as defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt
Need to specify how many ports and endpoints.
> +
> +Example:
> +
> + de: de-controller@01000000 {
> + compatible = "allwinner,sun8i-h3-display-engine";
> + ...
What are you not showing?
> + clocks = <&&ccu CLK_BUS_DE>, <&ccu CLK_DE>;
> + clock-names = "gate", "clock";
> + resets = <&ccu RST_BUS_DE>;
> + ports = <&lcd0_p>;
> + };
> +
> + lcd0: lcd-controller@01c0c000 {
> + compatible = "allwinner,sun8i-a83t-tcon";
> + ...
ditto.
> + clocks = <&ccu CLK_BUS_TCON0>, <&ccu CLK_TCON0>;
> + clock-names = "gate", "clock";
> + resets = <&ccu RST_BUS_TCON0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + lcd0_p: port {
> + lcd0_ep: endpoint {
> + remote-endpoint = <&hdmi_ep>;
> + };
> + };
> + };
> +
> + hdmi: hdmi@01ee0000 {
> + ...
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port {
> + hdmi_ep: endpoint {
> + remote-endpoint = <&lcd0_ep>;
> + };
> + };
> + };
> +
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 03/13] devicetree/bindings: display: Add bindings for two Mitsubishi panels
From: Rob Herring @ 2016-11-21 16:49 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: dri-devel, linux-renesas-soc, Tomi Valkeinen, devicetree
In-Reply-To: <1479526093-7014-4-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
On Sat, Nov 19, 2016 at 05:28:03AM +0200, Laurent Pinchart wrote:
> The AA104XD12 and AA121TD01 are LVDS display panels. Their bindings are
> modelled on the the LVS panel bindings.
s/LVS/LVDS/
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> .../display/panel/mitsubishi,aa104xd12.txt | 47 ++++++++++++++++++++++
> .../display/panel/mitsubishi,aa121td01.txt | 47 ++++++++++++++++++++++
> 2 files changed, 94 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
> create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt
With that,
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH] ARM: dts: sunxi: Enable UEXT related nodes for Olimex A20 SOM EVB
From: Emmanuel Vadot @ 2016-11-21 16:49 UTC (permalink / raw)
To: robh+dt, mark.rutland, linux, maxime.ripard, wens
Cc: devicetree, Emmanuel Vadot, linux-kernel, linux-arm-kernel
UEXT are Universal EXTension connector from Olimex. They embed i2c, spi
and uart pins along power in one connector and are found on most,
if not all, Olimex boards.
The Olimex A20 SOM EVB have two UEXT connector so enable the nodes found on
those two connectors.
Signed-off-by: Emmanuel Vadot <manu@bidouilliste.com>
---
arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts | 36 ++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
index 23aacce..e879c119 100644
--- a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
@@ -116,6 +116,18 @@
};
};
+&i2c1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c1_pins_a>;
+ status = "okay";
+};
+
+&i2c2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2_pins_a>;
+ status = "okay";
+};
+
&lradc {
vref-supply = <®_vcc3v0>;
status = "okay";
@@ -284,12 +296,36 @@
status = "okay";
};
+&spi1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi1_pins_a>,
+ <&spi1_cs0_pins_a>;
+};
+
+&spi2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi2_pins_a>,
+ <&spi2_cs0_pins_a>;
+};
+
&uart0 {
pinctrl-names = "default";
pinctrl-0 = <&uart0_pins_a>;
status = "okay";
};
+&uart6 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart6_pins_a>;
+ status = "okay";
+};
+
+&uart7 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart7_pins_a>;
+ status = "okay";
+};
+
&usb_otg {
dr_mode = "otg";
status = "okay";
--
2.9.2
^ permalink raw reply related
* Re: [PATCH v2 02/13] devicetree/bindings: display: Add bindings for LVDS panels
From: Rob Herring @ 2016-11-21 16:48 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Tomi Valkeinen,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479526093-7014-3-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
On Sat, Nov 19, 2016 at 05:28:02AM +0200, Laurent Pinchart wrote:
> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> Multiple incompatible data link layers have been used over time to
> transmit image data to LVDS panels. This binding supports display panels
> compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> specifications.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> ---
> .../bindings/display/panel/panel-lvds.txt | 120 +++++++++++++++++++++
> 1 file changed, 120 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.txt
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Bartosz Golaszewski @ 2016-11-21 16:48 UTC (permalink / raw)
To: Sekhar Nori
Cc: Mark Rutland, linux-devicetree, Tomi Valkeinen, Kevin Hilman,
Michael Turquette, Russell King, linux-drm, LKML, Peter Ujfalusi,
Rob Herring, Jyri Sarha, Frank Rowand, arm-soc, Laurent Pinchart
In-Reply-To: <a309a738-7fa7-3aab-4457-f7d693e6b37f@ti.com>
2016-11-21 17:33 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
> On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote:
>> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
>> +{
>> + const struct da8xx_ddrctl_config_knob *knob;
>> + const struct da8xx_ddrctl_setting *setting;
>> + struct device_node *node;
>> + struct resource *res;
>> + void __iomem *ddrctl;
>> + struct device *dev;
>> + u32 reg;
>> +
>> + dev = &pdev->dev;
>> + node = dev->of_node;
>> +
>> + setting = da8xx_ddrctl_get_board_settings();
>> + if (!setting) {
>> + dev_err(dev, "no settings for board '%s'\n",
>> + of_flat_dt_get_machine_name());
>> + return -EINVAL;
>> + }
>
> This causes a section mismatch because of_flat_dt_get_machine_name()
> has an __init annotation. I did not notice that before, sorry.
>
> It can be fixed with a patch like below:
>
> ---8<---
> diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
> index a20e7bbbcbe0..9ca5aab3ac54 100644
> --- a/drivers/memory/da8xx-ddrctl.c
> +++ b/drivers/memory/da8xx-ddrctl.c
> @@ -102,6 +102,18 @@ static const struct da8xx_ddrctl_setting *da8xx_ddrctl_get_board_settings(void)
> return NULL;
> }
>
> +static const char* da8xx_ddrctl_get_machine_name(void)
> +{
> + const char *str;
> + int ret;
> +
> + ret = of_property_read_string(of_root, "model", &str);
> + if (ret)
> + ret = of_property_read_string(of_root, "compatible", &str);
> +
> + return str;
> +}
> +
> static int da8xx_ddrctl_probe(struct platform_device *pdev)
> {
> const struct da8xx_ddrctl_config_knob *knob;
> @@ -118,7 +130,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev)
> setting = da8xx_ddrctl_get_board_settings();
> if (!setting) {
> dev_err(dev, "no settings for board '%s'\n",
> - of_flat_dt_get_machine_name());
> + da8xx_ddrctl_get_machine_name());
> return -EINVAL;
> }
> ---8<---
>
> A similar fix is required for the other driver in this series (patch
> 2/5). I need some advise on whether I should introduce a common
> function to get the machine name post kernel boot-up (I cannot see an
> existing one). If yes, any advise on which file it should go into?
>
Hi Sekhar,
thanks for spotting that.
I think we should introduce this function right away, rather than
having two static functions doing the same thing. If you don't mind,
I'll try to find a good spot for it and send a follow-up series fixing
the issue.
Best regards,
Bartosz Golaszewski
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties
From: Rob Herring @ 2016-11-21 16:48 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: dri-devel, linux-renesas-soc, Tomi Valkeinen, devicetree
In-Reply-To: <1479526093-7014-2-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> Document properties common to several display panels in a central
> location that can be referenced by the panel device tree bindings.
>
Looks good. Just one comment...
[...]
> +Connectivity
> +------------
> +
> +- ports: Panels receive video data through one or multiple connections. While
> + the nature of those connections is specific to the panel type, the
> + connectivity is expressed in a standard fashion using ports as specified in
> + the device graph bindings defined in
> + Documentation/devicetree/bindings/graph.txt.
We allow panels to either use graph binding or be a child of the display
controller. Using the graph is preferred, but in the simple cases just a
child node is sufficient. This should be described here or somewhere in
this doc.
Rob
^ permalink raw reply
* Re: [RFC PATCH net v2 2/3] dt: bindings: add ethernet phy eee-disable-advert option documentation
From: Andrew Lunn @ 2016-11-21 16:47 UTC (permalink / raw)
To: Jerome Brunet
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
Florian Fainelli, Alexandre TORGUE, Neil Armstrong,
Martin Blumenstingl, Kevin Hilman,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andre Roth,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Carlo Caione,
Giuseppe Cavallaro,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1479744993.17538.85.camel-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> What I did not realize when doing this patch for the realtek driver is
> that there is already 6 valid modes defined in the kernel
>
> #define MDIO_EEE_100TX MDIO_AN_EEE_ADV_100TX /*
> 100TX EEE cap */
> #define MDIO_EEE_1000T MDIO_AN_EEE_ADV_1000T /*
> 1000T EEE cap */
> #define MDIO_EEE_10GT 0x0008 /* 10GT EEE cap */
> #define MDIO_EEE_1000KX 0x0010 /* 1000KX EEE cap
> */
> #define MDIO_EEE_10GKX4 0x0020 /* 10G KX4 EEE cap
> */
> #define MDIO_EEE_10GKR 0x0040 /* 10G KR EEE cap
> */
>
> I took care of only 2 in the case of realtek.c since it only support
> MDIO_EEE_100TX and MDIO_EEE_1000T.
>
> Defining a property for each is certainly doable but it does not look
> very nice either. If it extends in the future, it will get even more
> messier, especially if you want to disable everything.
Yes, agreed.
> What do you think about keeping a single mask value but use the define
> above in the DT ? It would be more readable than hex and easy to
> extend, don't you think ?
>
> These defines are already part of the uapi so I guess we can use those
> in the DT bindings ?
I don't think they are accessible from the dtc include path. You will
need to make a copy, in include/dt-bindings/net/phy.h
But yes, using these defines is a good idea.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] mfd: cpcap: Add minimal support
From: Rob Herring @ 2016-11-21 16:34 UTC (permalink / raw)
To: Lee Jones
Cc: Tony Lindgren, Samuel Ortiz, linux-kernel, linux-omap, devicetree,
Marcel Partap, Mark Rutland, Michael Scott
In-Reply-To: <20161121114558.GJ32509@dell>
On Mon, Nov 21, 2016 at 11:45:58AM +0000, Lee Jones wrote:
> On Fri, 18 Nov 2016, Tony Lindgren wrote:
>
> > Many Motorola phones like droid 4 are using a custom PMIC called CPCAP
> > or 6556002. We can support it's core features quite easily with regmap_spi
> > and regmap_irq.
> >
> > The children of cpcap, such as regulators, ADC and USB, can be just regular
> > device drivers and defined in the dts file. They get probed as we call
> > of_platform_populate() at the end of our probe, and then the children
> > can just call dev_get_regmap(dev.parent, NULL) to get the regmap.
> >
> > Cc: devicetree@vger.kernel.org
> > Cc: Marcel Partap <mpartap@gmx.net>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Michael Scott <michael.scott@linaro.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > ---
> > Documentation/devicetree/bindings/mfd/cpcap.txt | 36 ++++
> > drivers/mfd/Kconfig | 8 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/cpcap.c | 255 ++++++++++++++++++++++++
> > include/linux/mfd/cpcap.h | 238 ++++++++++++++++++++++
> > 5 files changed, 538 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mfd/cpcap.txt
> > create mode 100644 drivers/mfd/cpcap.c
> > create mode 100644 include/linux/mfd/cpcap.h
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/cpcap.txt b/Documentation/devicetree/bindings/mfd/cpcap.txt
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/cpcap.txt
> > @@ -0,0 +1,36 @@
> > +CPCAP PMIC device tree binding
> > +
> > +Required properties:
> > +- compatible : Motorola device "motorola,cpcap", others "st,6556002"
> > +- reg : Chip select and size
> > +- interrupt-parent : The parent interrupt controller
> > +- interrupts : The interrupt line the device is connected to
> > +- interrupt-controller : Marks the device node as an interrupt controller
> > +- #interrupt-cells : The number of cells to describe an IRQ, should be 2
> > +- #address-cells : Child device offset number of cells, typically 1
> > +- #size-cells : Child device size number of cells, typically 1
> > +- ranges : Child device register range
> > +- spi-max-frequency : Typically set to 3000000
> > +- spi-cs_high : SPI chip select direction
> > +
> > +Example:
> > +
> > +&mcspi1 {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges;
> > + cpcap: pmic@0 {
> > + compatible = "motorola,cpcap", "st,6556002";
> > + reg = <0 0>; /* cs0, size 0 */
>
> Is this really correct?
No, SPI devices are 1 cell and there shouldn't be a ranges prop.
>
> How can ranges have a size of 0x8000 and this 0?
reg here doesn't affect ranges and address translation.
Perhaps this is trying to make address translation work, but if that
does, it is by chance. Children of pmic addresses in the range of
0-0x8000 would get translated to "cpu address" 0-0x8000 as long as the
DT has empty ranges up to the root. If the parent bus (i.e. SoC bus) has
any base address, then that is going to get added which would not be
good.
>
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <7 IRQ_TYPE_EDGE_RISING>;
> > + interrupt-controller;
> > + #interrupt-cells = <2>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges = <0 0 0x8000>;
> > + spi-max-frequency = <3000000>;
> > + spi-cs-high;
> > + };
> > +};
> > +
^ permalink raw reply
* Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Sekhar Nori @ 2016-11-21 16:33 UTC (permalink / raw)
To: Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Rob Herring,
Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
Cc: linux-devicetree, David Airlie, LKML, linux-drm, Tomi Valkeinen,
Jyri Sarha, arm-soc, Laurent Pinchart
In-Reply-To: <1477925138-23457-2-git-send-email-bgolaszewski@baylibre.com>
On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote:
> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
> +{
> + const struct da8xx_ddrctl_config_knob *knob;
> + const struct da8xx_ddrctl_setting *setting;
> + struct device_node *node;
> + struct resource *res;
> + void __iomem *ddrctl;
> + struct device *dev;
> + u32 reg;
> +
> + dev = &pdev->dev;
> + node = dev->of_node;
> +
> + setting = da8xx_ddrctl_get_board_settings();
> + if (!setting) {
> + dev_err(dev, "no settings for board '%s'\n",
> + of_flat_dt_get_machine_name());
> + return -EINVAL;
> + }
This causes a section mismatch because of_flat_dt_get_machine_name()
has an __init annotation. I did not notice that before, sorry.
It can be fixed with a patch like below:
---8<---
diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
index a20e7bbbcbe0..9ca5aab3ac54 100644
--- a/drivers/memory/da8xx-ddrctl.c
+++ b/drivers/memory/da8xx-ddrctl.c
@@ -102,6 +102,18 @@ static const struct da8xx_ddrctl_setting *da8xx_ddrctl_get_board_settings(void)
return NULL;
}
+static const char* da8xx_ddrctl_get_machine_name(void)
+{
+ const char *str;
+ int ret;
+
+ ret = of_property_read_string(of_root, "model", &str);
+ if (ret)
+ ret = of_property_read_string(of_root, "compatible", &str);
+
+ return str;
+}
+
static int da8xx_ddrctl_probe(struct platform_device *pdev)
{
const struct da8xx_ddrctl_config_knob *knob;
@@ -118,7 +130,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev)
setting = da8xx_ddrctl_get_board_settings();
if (!setting) {
dev_err(dev, "no settings for board '%s'\n",
- of_flat_dt_get_machine_name());
+ da8xx_ddrctl_get_machine_name());
return -EINVAL;
}
---8<---
A similar fix is required for the other driver in this series (patch
2/5). I need some advise on whether I should introduce a common
function to get the machine name post kernel boot-up (I cannot see an
existing one). If yes, any advise on which file it should go into?
Thanks,
Sekhar
^ permalink raw reply related
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
From: Sudeep Holla @ 2016-11-21 16:30 UTC (permalink / raw)
To: Lina Iyer, ulf.hansson, khilman, rjw, linux-pm, linux-arm-kernel
Cc: devicetree, lorenzo.pieralisi, Juri.Lelli, linux-arm-msm, sboyd,
brendan.jackman, Sudeep Holla, andy.gross
In-Reply-To: <1478210075-92045-2-git-send-email-lina.iyer@linaro.org>
Hi Lina,
On 03/11/16 21:54, Lina Iyer wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
>
Thanks for doing this(reluctantly? :-)). Looks good to me.
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH v6 4/5] USB: ohci: da8xx: Add devicetree bindings
From: Axel Haslam @ 2016-11-21 16:30 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, nsekhar-l0cyMroinI0,
khilman-rdvid1DuHRBWk0Htik3J/w, david-nq/r/kbU++upp/zk7JDF2g
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Axel Haslam,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161121163014.28008-1-ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
This patch documents the device tree bindings required for
the ohci controller found in TI da8xx family of SoC's
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: mark.rutland-5wv7dgnIgG8@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Axel Haslam <ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
.../devicetree/bindings/usb/ohci-da8xx.txt | 23 ++++++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt
diff --git a/Documentation/devicetree/bindings/usb/ohci-da8xx.txt b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
new file mode 100644
index 0000000..2dc8f67
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
@@ -0,0 +1,23 @@
+DA8XX USB OHCI controller
+
+Required properties:
+
+ - compatible: Should be "ti,da830-ohci"
+ - reg: Should contain one register range i.e. start and length
+ - interrupts: Description of the interrupt line
+ - phys: Phandle for the PHY device
+ - phy-names: Should be "usb-phy"
+
+Optional properties:
+ - vbus-supply: phandle of regulator that controls vbus power / over-current
+
+Example:
+
+ohci: usb@0225000 {
+ compatible = "ti,da830-ohci";
+ reg = <0x225000 0x1000>;
+ interrupts = <59>;
+ phys = <&usb_phy 1>;
+ phy-names = "usb-phy";
+ vbus-supply = <®_usb_ohci>;
+};
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/2] Add support for the Nexbox A1 board based on the Amlogic S912 SoC.
From: Neil Armstrong @ 2016-11-21 16:29 UTC (permalink / raw)
To: khilman, carlo
Cc: linux-amlogic, devicetree, linux-kernel, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20161121162905.14285-1-narmstrong@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
Documentation/devicetree/bindings/arm/amlogic.txt | 1 +
arch/arm64/boot/dts/amlogic/Makefile | 1 +
.../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 169 +++++++++++++++++++++
3 files changed, 171 insertions(+)
create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
diff --git a/Documentation/devicetree/bindings/arm/amlogic.txt b/Documentation/devicetree/bindings/arm/amlogic.txt
index 1144214..6ef7c52 100644
--- a/Documentation/devicetree/bindings/arm/amlogic.txt
+++ b/Documentation/devicetree/bindings/arm/amlogic.txt
@@ -45,3 +45,4 @@ Board compatible values:
- "amlogic,p231" (Meson gxl s905d)
- "amlogic,q200" (Meson gxm s912)
- "amlogic,q201" (Meson gxm s912)
+ - "nexbox,a1" (Meson gxm s912)
diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
index 7752a16..2fbb8e3 100644
--- a/arch/arm64/boot/dts/amlogic/Makefile
+++ b/arch/arm64/boot/dts/amlogic/Makefile
@@ -10,6 +10,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905d-p230.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905d-p231.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-gxm-s912-q200.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-gxm-s912-q201.dtb
+dtb-$(CONFIG_ARCH_MESON) += meson-gxm-nexbox-a1.dtb
always := $(dtb-y)
subdir-y := $(dts-dirs)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
new file mode 100644
index 0000000..d320727
--- /dev/null
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
@@ -0,0 +1,169 @@
+/*
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * Copyright (c) 2016 Endless Computers, Inc.
+ * Author: Carlo Caione <carlo@endlessm.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+
+#include "meson-gxm.dtsi"
+
+/ {
+ compatible = "nexbox,a1", "amlogic,s912", "amlogic,meson-gxm";
+ model = "NEXBOX A1";
+
+ aliases {
+ serial0 = &uart_AO;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory@0 {
+ device_type = "memory";
+ reg = <0x0 0x0 0x0 0x80000000>;
+ };
+
+ vddio_boot: regulator-vddio_boot {
+ compatible = "regulator-fixed";
+ regulator-name = "VDDIO_BOOT";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ vddao_3v3: regulator-vddao_3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "VDDAO_3V3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ vcc_3v3: regulator-vcc_3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "VCC_3V3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ emmc_pwrseq: emmc-pwrseq {
+ compatible = "mmc-pwrseq-emmc";
+ reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
+ };
+};
+
+/* This UART is brought out to the DB9 connector */
+&uart_AO {
+ status = "okay";
+ pinctrl-0 = <&uart_ao_a_pins>;
+ pinctrl-names = "default";
+};
+
+&ir {
+ status = "okay";
+ pinctrl-0 = <&remote_input_ao_pins>;
+ pinctrl-names = "default";
+};
+
+/* SD card */
+&sd_emmc_b {
+ status = "okay";
+ pinctrl-0 = <&sdcard_pins>;
+ pinctrl-names = "default";
+
+ bus-width = <4>;
+ cap-sd-highspeed;
+ max-frequency = <100000000>;
+ disable-wp;
+
+ cd-gpios = <&gpio CARD_6 GPIO_ACTIVE_HIGH>;
+ cd-inverted;
+
+ vmmc-supply = <&vddao_3v3>;
+ vqmmc-supply = <&vddio_boot>;
+};
+
+/* eMMC */
+&sd_emmc_c {
+ status = "okay";
+ pinctrl-0 = <&emmc_pins>;
+ pinctrl-names = "default";
+
+ bus-width = <8>;
+ cap-sd-highspeed;
+ cap-mmc-highspeed;
+ max-frequency = <200000000>;
+ non-removable;
+ disable-wp;
+ mmc-ddr-1_8v;
+ mmc-hs200-1_8v;
+
+ mmc-pwrseq = <&emmc_pwrseq>;
+ vmmc-supply = <&vcc_3v3>;
+ vqmmc-supply = <&vddio_boot>;
+};
+
+ðmac {
+ status = "okay";
+
+ pinctrl-0 = <ð_pins>;
+ pinctrl-names = "default";
+
+ /* Select external PHY by default */
+ phy-handle = <&external_phy>;
+
+ snps,reset-gpio = <&gpio GPIOZ_14 0>;
+ snps,reset-delays-us = <0 10000 1000000>;
+ snps,reset-active-low;
+
+ /* External PHY is in RGMII */
+ phy-mode = "rgmii";
+};
+
+&external_mdio {
+ external_phy: ethernet-phy@0 {
+ compatible = "ethernet-phy-id001c.c916", "ethernet-phy-ieee802.3-c22";
+ reg = <0>;
+ max-speed = <1000>;
+ };
+};
--
2.7.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox