* Re: [PATCH RFC 06/11] drm/bridge: analogix-anx78xx: add support for avdd33 regulator
From: Brian Masney @ 2019-08-15 22:51 UTC (permalink / raw)
To: Linus Walleij
Cc: Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jernej Skrabec, Neil Armstrong, Dave Airlie,
linux-kernel@vger.kernel.org, Jonas Karlman, Andy Gross,
open list:DRM PANEL DRIVERS, Bjorn Andersson, Andrzej Hajda,
Rob Clark, Rob Herring, Laurent Pinchart, Daniel Vetter, MSM,
Enric Balletbo i Serra, freedreno, Sean Paul, Linux ARM
In-Reply-To: <CACRpkdYdQa+FVfpSjLi0SsBMDT4QC667z1P1dnapz7PXgRoB5Q@mail.gmail.com>
On Thu, Aug 15, 2019 at 10:22:45AM +0200, Linus Walleij wrote:
> On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:
>
> > Add support for the avdd33 regulator to the analogix-anx78xx driver.
> > Note that the regulator is currently enabled during driver probe and
> > disabled when the driver is removed. This is currently how the
> > downstream MSM kernel sources do this.
> >
> > Let's not merge this upstream for the mean time until I get the external
> > display fully working on the Nexus 5 and then I can submit proper
> > support then that powers down this regulator in the power off function.
> >
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
>
> > +static void anx78xx_disable_regulator_action(void *_data)
> > +{
> > + struct anx78xx_platform_data *pdata = _data;
> > +
> > + regulator_disable(pdata->avdd33);
> > +}
> (...)
> > + err = devm_add_action(dev, anx78xx_disable_regulator_action,
> > + pdata);
>
> Clever idea. Good for initial support, probably later on it would
> need to be reworked using runtime PM so it's not constantly
> powered up.
Yes, that's my plan. I suspect that I may have a regulator disabled
somewhere so I was planning to leave this on all the time for the time
being to match the downstream behavior until I get the hot plug detect
GPIO working.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Thanks,
Brian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL 1/3] bcm2835-dt-next-2019-08-15
From: Florian Fainelli @ 2019-08-15 22:56 UTC (permalink / raw)
To: Stefan Wahren, Florian Fainelli
Cc: Eric Anholt, bcm-kernel-feedback-list, linux-kernel,
linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <1565894043-5249-1-git-send-email-wahrenst@gmx.net>
On 8/15/19 11:34 AM, Stefan Wahren wrote:
> Hi Florian,
>
> The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
>
> Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
>
> are available in the git repository at:
>
> git://github.com/anholt/linux tags/bcm2835-dt-next-2019-08-15
>
> for you to fetch changes up to 60c833d5664e1b3f71c4471233469790adf505ca:
>
> ARM: dts: bcm283x: Enable HDMI at board level (2019-08-15 19:35:15 +0200)
>
> ----------------------------------------------------------------
> This pull request prepares the BCM2835 DTS files for the introduction
> of the new SoC BCM2711.
>
> ----------------------------------------------------------------
> Stefan Wahren (4):
> ARM: bcm283x: Reduce register ranges for UART, SPI and I2C
> ARM: dts: bcm283x: Define MMC interfaces at board level
> ARM: dts: bcm283x: Define memory at board level
> ARM: dts: bcm283x: Enable HDMI at board level
Merged into devicetree/next, thanks Stefan!
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL 2/3] bcm2835-defconfig-next-2019-08-15
From: Florian Fainelli @ 2019-08-15 22:56 UTC (permalink / raw)
To: Stefan Wahren, Florian Fainelli
Cc: Eric Anholt, bcm-kernel-feedback-list, linux-kernel,
linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <1565894043-5249-2-git-send-email-wahrenst@gmx.net>
On 8/15/19 11:34 AM, Stefan Wahren wrote:
> Hi Florian,
>
> The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
>
> Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
>
> are available in the git repository at:
>
> git://github.com/anholt/linux tags/bcm2835-defconfig-next-2019-08-15
>
> for you to fetch changes up to 4c6f5d4038af2c7332630bdd75cfdc0309e97242:
>
> ARM: defconfig: enable cpufreq driver for RPi (2019-07-23 22:53:35 +0200)
>
> ----------------------------------------------------------------
> This pull request enables the new RPi cpufreq driver in the 32-bit
> defconfigs.
>
> ----------------------------------------------------------------
Merged into defconfig/next, thanks Stefan!
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL 3/3] bcm2835-defconfig-64-next-2019-08-15
From: Florian Fainelli @ 2019-08-15 22:57 UTC (permalink / raw)
To: Stefan Wahren, Florian Fainelli
Cc: Eric Anholt, bcm-kernel-feedback-list, linux-kernel,
linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <1565894043-5249-3-git-send-email-wahrenst@gmx.net>
On 8/15/19 11:34 AM, Stefan Wahren wrote:
> Hi Florian,
>
> The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
>
> Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
>
> are available in the git repository at:
>
> git://github.com/anholt/linux tags/bcm2835-defconfig-64-next-2019-08-15
>
> for you to fetch changes up to e2dd73ac4440f7143e990e76bad9a46dc63a5951:
>
> arm64: defconfig: enable cpufreq support for RPi3 (2019-07-23 23:17:09 +0200)
>
> ----------------------------------------------------------------
> This pull request enables the new RPi cpufreq driver in the 64-bit
> defconfig.
>
> ----------------------------------------------------------------
Merged into defconfig-arm64/next, thanks Stefan!
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] arm64: defconfig: Enable the DesignWare watchdog
From: Dinh Nguyen @ 2019-08-15 23:12 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: olof, dinguyen, will, shawnguo, catalin.marinas
Enable the DesignWare watchdog driver that is present on the Intel
Stratix10 and Agilex platforms.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 0e58ef02880c..69c19d82d936 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -445,6 +445,7 @@ CONFIG_UNIPHIER_THERMAL=y
CONFIG_WATCHDOG=y
CONFIG_ARM_SP805_WATCHDOG=y
CONFIG_S3C2410_WATCHDOG=y
+CONFIG_DW_WATCHDOG=y
CONFIG_SUNXI_WATCHDOG=m
CONFIG_IMX2_WDT=y
CONFIG_IMX_SC_WDT=m
--
2.20.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* linux-next: build warning after merge of the arm-soc tree
From: Stephen Rothwell @ 2019-08-15 23:23 UTC (permalink / raw)
To: Olof Johansson, Arnd Bergmann, ARM
Cc: Linux Next Mailing List, Linux Kernel Mailing List
[-- Attachment #1.1: Type: text/plain, Size: 1686 bytes --]
Hi all,
After merging the arm-soc tree, today's linux-next build (x86_64
allmodconfig) produced this warning:
In file included from include/linux/kernel.h:15,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers/dma/iop-adma.c:13:
drivers/dma/iop-adma.c: In function '__iop_adma_slot_cleanup':
drivers/dma/iop-adma.c:118:12: warning: format '%x' expects argument of type 'unsigned int', but argument 6 has type 'dma_addr_t' {aka 'long long unsigned int'} [-Wformat=]
pr_debug("\tcookie: %d slot: %d busy: %d "
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:288:21: note: in definition of macro 'pr_fmt'
#define pr_fmt(fmt) fmt
^~~
include/linux/dynamic_debug.h:143:2: note: in expansion of macro '__dynamic_func_call'
__dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
^~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:153:2: note: in expansion of macro '_dynamic_func_call'
_dynamic_func_call(fmt, __dynamic_pr_debug, \
^~~~~~~~~~~~~~~~~~
include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
dynamic_pr_debug(fmt, ##__VA_ARGS__)
^~~~~~~~~~~~~~~~
drivers/dma/iop-adma.c:118:3: note: in expansion of macro 'pr_debug'
pr_debug("\tcookie: %d slot: %d busy: %d "
^~~~~~~~
drivers/dma/iop-adma.c:119:18: note: format string is defined here
"this_desc: %#x next_desc: %#llx ack: %d\n",
~~^
%#llx
Introduced (or exposed?) by commit
00c9755524fb ("dmaengine: iop-adma: use correct printk format strings")
--
Cheers,
Stephen Rothwell
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* linux-next: build warning after merge of the arm-soc tree
From: Stephen Rothwell @ 2019-08-15 23:25 UTC (permalink / raw)
To: Olof Johansson, Arnd Bergmann, ARM
Cc: Linux Next Mailing List, Linux Kernel Mailing List
[-- Attachment #1.1: Type: text/plain, Size: 1035 bytes --]
Hi all,
After merging the arm-soc tree, today's linux-next build (x86_64
allmodconfig) produced this warning:
drivers/usb/gadget/udc/lpc32xx_udc.c: In function 'udc_pop_fifo':
drivers/usb/gadget/udc/lpc32xx_udc.c:1156:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
switch (((u32) data) & 0x3) {
^
drivers/usb/gadget/udc/lpc32xx_udc.c: In function 'udc_stuff_fifo':
drivers/usb/gadget/udc/lpc32xx_udc.c:1257:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
switch (((u32) data) & 0x3) {
^
drivers/usb/gadget/udc/lpc32xx_udc.c: In function 'udc_handle_ep0_setup':
drivers/usb/gadget/udc/lpc32xx_udc.c:2230:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
switch (reqtype) {
^~~~~~
drivers/usb/gadget/udc/lpc32xx_udc.c:2269:2: note: here
case USB_REQ_SET_ADDRESS:
^~~~
Exposed by commit
50ad15282e7c ("usb: udc: lpc32xx: allow compile-testing")
--
Cheers,
Stephen Rothwell
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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 v3 0/7] soc/fsl/qbman: Enable Kexec for DPAA1 devices
From: Li Yang @ 2019-08-15 23:31 UTC (permalink / raw)
To: Roy Pledge
Cc: Madalin-cristian Bucur, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Laurentiu Tudor
In-Reply-To: <1564690599-29713-1-git-send-email-roy.pledge@nxp.com>
On Thu, Aug 1, 2019 at 3:20 PM Roy Pledge <roy.pledge@nxp.com> wrote:
>
> Most DPAA1 devices do not support a soft reset which is an issue if
> Kexec starts a new kernel. This patch series allows Kexec to function
> by detecting that the QBMan device was previously initialized.
>
> The patches fix some issues with device cleanup as well as ensuring
> that the location of the QBMan private memories has not changed
> after the execution of the Kexec.
>
> Changes since v1:
> - Removed a bug fix and sent it separately to ease backporting
> Changes since v2:
> - Expliciitly flush FQD memory from cache on PPC before unmapping
>
> Roy Pledge (7):
> soc/fsl/qbman: Rework QBMan private memory setup
> soc/fsl/qbman: Cleanup buffer pools if BMan was initialized prior to
> bootup
> soc/fsl/qbman: Cleanup QMan queues if device was already initialized
> soc/fsl/qbman: Fix drain_mr_fqni()
> soc/fsl/qbman: Disable interrupts during portal recovery
> soc/fsl/qbman: Fixup qman_shutdown_fq()
> soc/fsl/qbman: Update device tree with reserved memory
Series applied for next. Thanks!
>
> drivers/soc/fsl/qbman/bman.c | 17 ++++----
> drivers/soc/fsl/qbman/bman_ccsr.c | 36 +++++++++++++++-
> drivers/soc/fsl/qbman/bman_portal.c | 18 +++++++-
> drivers/soc/fsl/qbman/bman_priv.h | 5 +++
> drivers/soc/fsl/qbman/dpaa_sys.c | 63 ++++++++++++++++------------
> drivers/soc/fsl/qbman/qman.c | 83 +++++++++++++++++++++++++++++--------
> drivers/soc/fsl/qbman/qman_ccsr.c | 68 +++++++++++++++++++++++++++---
> drivers/soc/fsl/qbman/qman_portal.c | 18 +++++++-
> drivers/soc/fsl/qbman/qman_priv.h | 8 ++++
> 9 files changed, 255 insertions(+), 61 deletions(-)
>
> --
> 2.7.4
>
_______________________________________________
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 v8 05/14] media: rkisp1: add Rockchip ISP1 subdev driver
From: Laurent Pinchart @ 2019-08-16 0:13 UTC (permalink / raw)
To: Helen Koike
Cc: devicetree, eddie.cai.linux, kernel, heiko, jacob2.chen,
jeffy.chen, zyc, linux-kernel, tfiga, linux-rockchip, Allon Huang,
Jacob Chen, hans.verkuil, sakari.ailus, zhengsq, mchehab,
ezequiel, linux-arm-kernel, linux-media
In-Reply-To: <20190730184256.30338-6-helen.koike@collabora.com>
Hello Helen,
Thank you for the patch.
On Tue, Jul 30, 2019 at 03:42:47PM -0300, Helen Koike wrote:
> From: Jacob Chen <jacob2.chen@rock-chips.com>
>
> Add the subdev driver for rockchip isp1.
>
> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> Signed-off-by: Yichong Zhong <zyc@rock-chips.com>
> Signed-off-by: Jacob Chen <cc@rock-chips.com>
> Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> Signed-off-by: Allon Huang <allon.huang@rock-chips.com>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> [fixed unknown entity type / switched to PIXEL_RATE]
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> [update for upstream]
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>
> ---
>
> Changes in v8: None
> Changes in v7:
> - fixed warning because of unknown entity type
> - fixed v4l2-compliance errors regarding rkisp1 formats, try formats
> and default values
> - fix typo riksp1/rkisp1
> - redesign: remove mipi/csi subdevice, sensors connect directly to the
> isp subdevice in the media topology now. As a consequence, remove the
> hack in mipidphy_g_mbus_config() where information from the sensor was
> being propagated through the topology.
> - From the old dphy:
> * cache get_remote_sensor() in s_stream
> * use V4L2_CID_PIXEL_RATE instead of V4L2_CID_LINK_FREQ
> - Replace stream state with a boolean
> - code styling and checkpatch fixes
> - fix stop_stream (return after calling stop, do not reenable the stream)
> - fix rkisp1_isp_sd_get_selection when V4L2_SUBDEV_FORMAT_TRY is set
> - fix get format in output (isp_sd->out_fmt.mbus_code was being ignored)
> - s/intput/input
> - remove #define sd_to_isp_sd(_sd), add a static inline as it will be
> reused by the capture
>
> drivers/media/platform/rockchip/isp1/rkisp1.c | 1286 +++++++++++++++++
> drivers/media/platform/rockchip/isp1/rkisp1.h | 111 ++
> 2 files changed, 1397 insertions(+)
> create mode 100644 drivers/media/platform/rockchip/isp1/rkisp1.c
> create mode 100644 drivers/media/platform/rockchip/isp1/rkisp1.h
>
> diff --git a/drivers/media/platform/rockchip/isp1/rkisp1.c b/drivers/media/platform/rockchip/isp1/rkisp1.c
> new file mode 100644
> index 000000000000..6d0c0ffb5e03
> --- /dev/null
> +++ b/drivers/media/platform/rockchip/isp1/rkisp1.c
> @@ -0,0 +1,1286 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Rockchip isp1 driver
Shouldn't each file describe what it contains ? Maybe
* Rockchip ISP1 Driver - ISP Core
for this one ? Same for other .c or .h files.
> + *
> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> + */
> +
> +#include <linux/iopoll.h>
> +#include <linux/phy/phy.h>
> +#include <linux/phy/phy-mipi-dphy.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/videodev2.h>
> +#include <linux/vmalloc.h>
> +#include <media/v4l2-event.h>
> +
> +#include "common.h"
> +#include "regs.h"
common.h and regs.h aren't available yet. This won't break bisection as
this file isn't referenced from the makefile yet, but it makes it a bit
annoying when reviewing patches in order :-S
> +
> +#define CIF_ISP_INPUT_W_MAX 4032
> +#define CIF_ISP_INPUT_H_MAX 3024
> +#define CIF_ISP_INPUT_W_MIN 32
> +#define CIF_ISP_INPUT_H_MIN 32
> +#define CIF_ISP_OUTPUT_W_MAX CIF_ISP_INPUT_W_MAX
> +#define CIF_ISP_OUTPUT_H_MAX CIF_ISP_INPUT_H_MAX
> +#define CIF_ISP_OUTPUT_W_MIN CIF_ISP_INPUT_W_MIN
> +#define CIF_ISP_OUTPUT_H_MIN CIF_ISP_INPUT_H_MIN
> +
> +/*
> + * NOTE: MIPI controller and input MUX are also configured in this file,
> + * because ISP Subdev is not only describe ISP submodule(input size,format,
> + * output size, format), but also a virtual route device.
> + */
> +
> +/*
> + * There are many variables named with format/frame in below code,
> + * please see here for their meaning.
> + *
> + * Cropping regions of ISP
> + *
> + * +---------------------------------------------------------+
> + * | Sensor image |
> + * | +---------------------------------------------------+ |
> + * | | ISP_ACQ (for black level) | |
> + * | | in_frm | |
> + * | | +--------------------------------------------+ | |
> + * | | | ISP_OUT | | |
> + * | | | in_crop | | |
in_crop at the ISP output ? That seems a bit weird. I'm guessing that
this is really the ISP output, while ISP_IS is related to the resizer ?
> + * | | | +---------------------------------+ | | |
> + * | | | | ISP_IS | | | |
> + * | | | | rkisp1_isp_subdev: out_crop | | | |
> + * | | | +---------------------------------+ | | |
> + * | | +--------------------------------------------+ | |
> + * | +---------------------------------------------------+ |
> + * +---------------------------------------------------------+
> + */
> +
> +static inline struct rkisp1_device *sd_to_isp_dev(struct v4l2_subdev *sd)
> +{
> + return container_of(sd->v4l2_dev, struct rkisp1_device, v4l2_dev);
> +}
> +
> +/* Get sensor by enabled media link */
> +static struct v4l2_subdev *get_remote_sensor(struct v4l2_subdev *sd)
> +{
> + struct media_pad *local, *remote;
> + struct media_entity *sensor_me;
> +
> + local = &sd->entity.pads[RKISP1_ISP_PAD_SINK];
> + remote = media_entity_remote_pad(local);
> + if (!remote) {
> + v4l2_warn(sd, "No link between isp and sensor\n");
> + return NULL;
> + }
> +
> + sensor_me = media_entity_remote_pad(local)->entity;
> + return media_entity_to_v4l2_subdev(sensor_me);
> +}
> +
> +static struct rkisp1_sensor *sd_to_sensor(struct rkisp1_device *dev,
> + struct v4l2_subdev *sd)
> +{
> + struct rkisp1_sensor *sensor;
> +
> + list_for_each_entry(sensor, &dev->sensors, list)
> + if (sensor->sd == sd)
> + return sensor;
> +
> + return NULL;
> +}
> +
> +/**************** register operations ****************/
> +
> +/*
> + * Image Stabilization.
> + * This should only be called when configuring CIF
> + * or at the frame end interrupt
> + */
> +static void rkisp1_config_ism(struct rkisp1_device *dev)
> +{
> + void __iomem *base = dev->base_addr;
> + struct v4l2_rect *out_crop = &dev->isp_sdev.out_crop;
> + u32 val;
> +
> + writel(0, base + CIF_ISP_IS_RECENTER);
How about read/write wrappers that take a rkisp1_device pointer, a
register offset and a value (for the write wrapper) and compute
dev->base_addr + offset internally ? That would make the code easier to
read.
> + writel(0, base + CIF_ISP_IS_MAX_DX);
> + writel(0, base + CIF_ISP_IS_MAX_DY);
> + writel(0, base + CIF_ISP_IS_DISPLACE);
> + writel(out_crop->left, base + CIF_ISP_IS_H_OFFS);
> + writel(out_crop->top, base + CIF_ISP_IS_V_OFFS);
> + writel(out_crop->width, base + CIF_ISP_IS_H_SIZE);
> + writel(out_crop->height, base + CIF_ISP_IS_V_SIZE);
> +
> + /* IS(Image Stabilization) is always on, working as output crop */
> + writel(1, base + CIF_ISP_IS_CTRL);
> + val = readl(base + CIF_ISP_CTRL);
> + val |= CIF_ISP_CTRL_ISP_CFG_UPD;
> + writel(val, base + CIF_ISP_CTRL);
> +}
> +
> +/*
> + * configure isp blocks with input format, size......
> + */
> +static int rkisp1_config_isp(struct rkisp1_device *dev)
> +{
> + u32 isp_ctrl = 0, irq_mask = 0, acq_mult = 0, signal = 0;
> + struct v4l2_rect *out_crop, *in_crop;
> + void __iomem *base = dev->base_addr;
> + struct v4l2_mbus_framefmt *in_frm;
> + struct ispsd_out_fmt *out_fmt;
> + struct rkisp1_sensor *sensor;
> + struct ispsd_in_fmt *in_fmt;
> +
> + sensor = dev->active_sensor;
> + in_frm = &dev->isp_sdev.in_frm;
> + in_fmt = &dev->isp_sdev.in_fmt;
> + out_fmt = &dev->isp_sdev.out_fmt;
> + out_crop = &dev->isp_sdev.out_crop;
> + in_crop = &dev->isp_sdev.in_crop;
> +
> + if (in_fmt->fmt_type == FMT_BAYER) {
> + acq_mult = 1;
> + if (out_fmt->fmt_type == FMT_BAYER) {
> + if (sensor->mbus.type == V4L2_MBUS_BT656)
> + isp_ctrl =
> + CIF_ISP_CTRL_ISP_MODE_RAW_PICT_ITU656;
> + else
> + isp_ctrl =
> + CIF_ISP_CTRL_ISP_MODE_RAW_PICT;
> + } else {
> + writel(CIF_ISP_DEMOSAIC_TH(0xc),
> + base + CIF_ISP_DEMOSAIC);
> +
> + if (sensor->mbus.type == V4L2_MBUS_BT656)
> + isp_ctrl = CIF_ISP_CTRL_ISP_MODE_BAYER_ITU656;
> + else
> + isp_ctrl = CIF_ISP_CTRL_ISP_MODE_BAYER_ITU601;
> + }
> + } else if (in_fmt->fmt_type == FMT_YUV) {
> + acq_mult = 2;
> + if (sensor->mbus.type == V4L2_MBUS_CSI2_DPHY) {
> + isp_ctrl = CIF_ISP_CTRL_ISP_MODE_ITU601;
> + } else {
> + if (sensor->mbus.type == V4L2_MBUS_BT656)
> + isp_ctrl = CIF_ISP_CTRL_ISP_MODE_ITU656;
> + else
> + isp_ctrl = CIF_ISP_CTRL_ISP_MODE_ITU601;
> +
> + }
> +
> + irq_mask |= CIF_ISP_DATA_LOSS;
> + }
> +
> + /* Set up input acquisition properties */
> + if (sensor->mbus.type == V4L2_MBUS_BT656 ||
> + sensor->mbus.type == V4L2_MBUS_PARALLEL) {
> + if (sensor->mbus.flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
> + signal = CIF_ISP_ACQ_PROP_POS_EDGE;
> + }
> +
> + if (sensor->mbus.type == V4L2_MBUS_PARALLEL) {
> + if (sensor->mbus.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
> + signal |= CIF_ISP_ACQ_PROP_VSYNC_LOW;
> +
> + if (sensor->mbus.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)
> + signal |= CIF_ISP_ACQ_PROP_HSYNC_LOW;
> + }
> +
> + writel(isp_ctrl, base + CIF_ISP_CTRL);
> + writel(signal | in_fmt->yuv_seq |
> + CIF_ISP_ACQ_PROP_BAYER_PAT(in_fmt->bayer_pat) |
> + CIF_ISP_ACQ_PROP_FIELD_SEL_ALL, base + CIF_ISP_ACQ_PROP);
> + writel(0, base + CIF_ISP_ACQ_NR_FRAMES);
> +
> + /* Acquisition Size */
> + writel(0, base + CIF_ISP_ACQ_H_OFFS);
> + writel(0, base + CIF_ISP_ACQ_V_OFFS);
> + writel(acq_mult * in_frm->width, base + CIF_ISP_ACQ_H_SIZE);
> + writel(in_frm->height, base + CIF_ISP_ACQ_V_SIZE);
> +
> + /* ISP Out Area */
> + writel(in_crop->left, base + CIF_ISP_OUT_H_OFFS);
> + writel(in_crop->top, base + CIF_ISP_OUT_V_OFFS);
> + writel(in_crop->width, base + CIF_ISP_OUT_H_SIZE);
> + writel(in_crop->height, base + CIF_ISP_OUT_V_SIZE);
> +
> + /* interrupt mask */
> + irq_mask |= CIF_ISP_FRAME | CIF_ISP_V_START | CIF_ISP_PIC_SIZE_ERROR |
> + CIF_ISP_FRAME_IN;
> + writel(irq_mask, base + CIF_ISP_IMSC);
> +
> + if (out_fmt->fmt_type == FMT_BAYER)
> + rkisp1_params_disable_isp(&dev->params_vdev);
> + else
> + rkisp1_params_configure_isp(&dev->params_vdev, in_fmt,
> + dev->isp_sdev.quantization);
> +
> + return 0;
> +}
> +
> +static int rkisp1_config_dvp(struct rkisp1_device *dev)
> +{
> + struct ispsd_in_fmt *in_fmt = &dev->isp_sdev.in_fmt;
> + void __iomem *base = dev->base_addr;
> + u32 val, input_sel;
> +
> + switch (in_fmt->bus_width) {
> + case 8:
> + input_sel = CIF_ISP_ACQ_PROP_IN_SEL_8B_ZERO;
> + break;
> + case 10:
> + input_sel = CIF_ISP_ACQ_PROP_IN_SEL_10B_ZERO;
> + break;
> + case 12:
> + input_sel = CIF_ISP_ACQ_PROP_IN_SEL_12B;
> + break;
> + default:
> + v4l2_err(&dev->v4l2_dev, "Invalid bus width\n");
> + return -EINVAL;
> + }
> +
> + val = readl(base + CIF_ISP_ACQ_PROP);
> + writel(val | input_sel, base + CIF_ISP_ACQ_PROP);
> +
> + return 0;
> +}
> +
> +static int rkisp1_config_mipi(struct rkisp1_device *dev)
> +{
> + struct ispsd_in_fmt *in_fmt = &dev->isp_sdev.in_fmt;
> + struct rkisp1_sensor *sensor = dev->active_sensor;
> + void __iomem *base = dev->base_addr;
> + unsigned int lanes;
> + u32 mipi_ctrl;
> +
> + /*
> + * sensor->mbus is set in isp or d-phy notifier_bound function
> + */
> + switch (sensor->mbus.flags & V4L2_MBUS_CSI2_LANES) {
> + case V4L2_MBUS_CSI2_4_LANE:
> + lanes = 4;
> + break;
> + case V4L2_MBUS_CSI2_3_LANE:
> + lanes = 3;
> + break;
> + case V4L2_MBUS_CSI2_2_LANE:
> + lanes = 2;
> + break;
> + case V4L2_MBUS_CSI2_1_LANE:
> + lanes = 1;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + mipi_ctrl = CIF_MIPI_CTRL_NUM_LANES(lanes - 1) |
> + CIF_MIPI_CTRL_SHUTDOWNLANES(0xf) |
> + CIF_MIPI_CTRL_ERR_SOT_SYNC_HS_SKIP |
> + CIF_MIPI_CTRL_CLOCKLANE_ENA;
> +
> + writel(mipi_ctrl, base + CIF_MIPI_CTRL);
> +
> + /* Configure Data Type and Virtual Channel */
> + writel(CIF_MIPI_DATA_SEL_DT(in_fmt->mipi_dt) | CIF_MIPI_DATA_SEL_VC(0),
> + base + CIF_MIPI_IMG_DATA_SEL);
> +
> + /* Clear MIPI interrupts */
> + writel(~0, base + CIF_MIPI_ICR);
> + /*
> + * Disable CIF_MIPI_ERR_DPHY interrupt here temporary for
> + * isp bus may be dead when switch isp.
> + */
> + writel(CIF_MIPI_FRAME_END | CIF_MIPI_ERR_CSI | CIF_MIPI_ERR_DPHY |
> + CIF_MIPI_SYNC_FIFO_OVFLW(0x03) | CIF_MIPI_ADD_DATA_OVFLW,
> + base + CIF_MIPI_IMSC);
> +
> + v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev, "\n MIPI_CTRL 0x%08x\n"
> + " MIPI_IMG_DATA_SEL 0x%08x\n"
> + " MIPI_STATUS 0x%08x\n"
> + " MIPI_IMSC 0x%08x\n",
> + readl(base + CIF_MIPI_CTRL),
> + readl(base + CIF_MIPI_IMG_DATA_SEL),
> + readl(base + CIF_MIPI_STATUS),
> + readl(base + CIF_MIPI_IMSC));
> +
> + return 0;
> +}
> +
> +/* Configure MUX */
> +static int rkisp1_config_path(struct rkisp1_device *dev)
> +{
> + struct rkisp1_sensor *sensor = dev->active_sensor;
> + u32 dpcl = readl(dev->base_addr + CIF_VI_DPCL);
> + int ret = 0;
> +
> + if (sensor->mbus.type == V4L2_MBUS_BT656 ||
> + sensor->mbus.type == V4L2_MBUS_PARALLEL) {
> + ret = rkisp1_config_dvp(dev);
> + dpcl |= CIF_VI_DPCL_IF_SEL_PARALLEL;
> + } else if (sensor->mbus.type == V4L2_MBUS_CSI2_DPHY) {
> + ret = rkisp1_config_mipi(dev);
> + dpcl |= CIF_VI_DPCL_IF_SEL_MIPI;
> + }
> +
> + writel(dpcl, dev->base_addr + CIF_VI_DPCL);
> +
> + return ret;
> +}
> +
> +/* Hareware configure Entry */
s/Hareware/Hardware/
> +static int rkisp1_config_cif(struct rkisp1_device *dev)
> +{
> + u32 cif_id;
> + int ret;
> +
> + v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
> + "SP streaming = %d, MP streaming = %d\n",
> + dev->stream[RKISP1_STREAM_SP].streaming,
> + dev->stream[RKISP1_STREAM_MP].streaming);
> +
> + cif_id = readl(dev->base_addr + CIF_VI_ID);
> + v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev, "CIF_ID 0x%08x\n", cif_id);
> +
> + ret = rkisp1_config_isp(dev);
> + if (ret < 0)
> + return ret;
> + ret = rkisp1_config_path(dev);
> + if (ret < 0)
> + return ret;
> + rkisp1_config_ism(dev);
> +
> + return 0;
> +}
> +
> +/* Mess register operations to stop isp */
Is it such a mess ? :-)
I would capitalise ISP in all comments.
> +static int rkisp1_isp_stop(struct rkisp1_device *dev)
> +{
> + void __iomem *base = dev->base_addr;
> + u32 val;
> +
> + v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
> + "SP streaming = %d, MP streaming = %d\n",
> + dev->stream[RKISP1_STREAM_SP].streaming,
> + dev->stream[RKISP1_STREAM_MP].streaming);
> +
> + /*
> + * ISP(mi) stop in mi frame end -> Stop ISP(mipi) ->
> + * Stop ISP(isp) ->wait for ISP isp off
> + */
> + /* stop and clear MI, MIPI, and ISP interrupts */
> + writel(0, base + CIF_MIPI_IMSC);
> + writel(~0, base + CIF_MIPI_ICR);
> +
> + writel(0, base + CIF_ISP_IMSC);
> + writel(~0, base + CIF_ISP_ICR);
> +
> + writel(0, base + CIF_MI_IMSC);
> + writel(~0, base + CIF_MI_ICR);
> + val = readl(base + CIF_MIPI_CTRL);
> + writel(val & (~CIF_MIPI_CTRL_OUTPUT_ENA), base + CIF_MIPI_CTRL);
> + /* stop ISP */
> + val = readl(base + CIF_ISP_CTRL);
> + val &= ~(CIF_ISP_CTRL_ISP_INFORM_ENABLE | CIF_ISP_CTRL_ISP_ENABLE);
> + writel(val, base + CIF_ISP_CTRL);
> +
> + val = readl(base + CIF_ISP_CTRL);
> + writel(val | CIF_ISP_CTRL_ISP_CFG_UPD, base + CIF_ISP_CTRL);
> +
> + readx_poll_timeout(readl, base + CIF_ISP_RIS,
> + val, val & CIF_ISP_OFF, 20, 100);
> + v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
> + "streaming(MP:%d, SP:%d), MI_CTRL:%x, ISP_CTRL:%x, MIPI_CTRL:%x\n",
> + dev->stream[RKISP1_STREAM_SP].streaming,
> + dev->stream[RKISP1_STREAM_MP].streaming,
> + readl(base + CIF_MI_CTRL),
> + readl(base + CIF_ISP_CTRL),
> + readl(base + CIF_MIPI_CTRL));
> +
> + writel(CIF_IRCL_MIPI_SW_RST | CIF_IRCL_ISP_SW_RST, base + CIF_IRCL);
> + writel(0x0, base + CIF_IRCL);
> +
> + return 0;
> +}
> +
> +/* Mess register operations to start isp */
> +static int rkisp1_isp_start(struct rkisp1_device *dev)
> +{
> + struct rkisp1_sensor *sensor = dev->active_sensor;
> + void __iomem *base = dev->base_addr;
> + u32 val;
> +
> + v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
> + "SP streaming = %d, MP streaming = %d\n",
> + dev->stream[RKISP1_STREAM_SP].streaming,
> + dev->stream[RKISP1_STREAM_MP].streaming);
> +
> + /* Activate MIPI */
> + if (sensor->mbus.type == V4L2_MBUS_CSI2_DPHY) {
> + val = readl(base + CIF_MIPI_CTRL);
> + writel(val | CIF_MIPI_CTRL_OUTPUT_ENA, base + CIF_MIPI_CTRL);
> + }
> + /* Activate ISP */
> + val = readl(base + CIF_ISP_CTRL);
> + val |= CIF_ISP_CTRL_ISP_CFG_UPD | CIF_ISP_CTRL_ISP_ENABLE |
> + CIF_ISP_CTRL_ISP_INFORM_ENABLE;
> + writel(val, base + CIF_ISP_CTRL);
> +
> + /* XXX: Is the 1000us too long?
> + * CIF spec says to wait for sufficient time after enabling
> + * the MIPI interface and before starting the sensor output.
> + */
> + usleep_range(1000, 1200);
> +
> + v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
> + "SP streaming = %d, MP streaming = %d MI_CTRL 0x%08x\n"
> + " ISP_CTRL 0x%08x MIPI_CTRL 0x%08x\n",
> + dev->stream[RKISP1_STREAM_SP].streaming,
> + dev->stream[RKISP1_STREAM_MP].streaming,
> + readl(base + CIF_MI_CTRL),
> + readl(base + CIF_ISP_CTRL),
> + readl(base + CIF_MIPI_CTRL));
> +
> + return 0;
> +}
> +
> +static void rkisp1_config_clk(struct rkisp1_device *dev)
> +{
> + u32 val = CIF_ICCL_ISP_CLK | CIF_ICCL_CP_CLK | CIF_ICCL_MRSZ_CLK |
> + CIF_ICCL_SRSZ_CLK | CIF_ICCL_JPEG_CLK | CIF_ICCL_MI_CLK |
> + CIF_ICCL_IE_CLK | CIF_ICCL_MIPI_CLK | CIF_ICCL_DCROP_CLK;
> +
> + writel(val, dev->base_addr + CIF_ICCL);
> +}
> +
> +/***************************** isp sub-devs *******************************/
> +
> +static const struct ispsd_in_fmt rkisp1_isp_input_formats[] = {
> + {
> + .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10,
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW10,
> + .bayer_pat = RAW_BGGR,
> + .bus_width = 10,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10,
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW10,
> + .bayer_pat = RAW_RGGB,
> + .bus_width = 10,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10,
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW10,
> + .bayer_pat = RAW_GBRG,
> + .bus_width = 10,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10,
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW10,
> + .bayer_pat = RAW_GRBG,
> + .bus_width = 10,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12,
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW12,
> + .bayer_pat = RAW_RGGB,
> + .bus_width = 12,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12,
Is there a reason why the 10-bit Bayer format start with BGGR while the
12-bit formats start with RGGB ? Not a big deal, just OCD kicking in :-)
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW12,
> + .bayer_pat = RAW_BGGR,
> + .bus_width = 12,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12,
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW12,
> + .bayer_pat = RAW_GBRG,
> + .bus_width = 12,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12,
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW12,
> + .bayer_pat = RAW_GRBG,
> + .bus_width = 12,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8,
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW8,
> + .bayer_pat = RAW_RGGB,
> + .bus_width = 8,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8,
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW8,
> + .bayer_pat = RAW_BGGR,
> + .bus_width = 8,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8,
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW8,
> + .bayer_pat = RAW_GBRG,
> + .bus_width = 8,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8,
> + .fmt_type = FMT_BAYER,
> + .mipi_dt = CIF_CSI2_DT_RAW8,
> + .bayer_pat = RAW_GRBG,
> + .bus_width = 8,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_YUYV8_1X16,
> + .fmt_type = FMT_YUV,
> + .mipi_dt = CIF_CSI2_DT_YUV422_8b,
> + .yuv_seq = CIF_ISP_ACQ_PROP_YCBYCR,
> + .bus_width = 16,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_YVYU8_1X16,
> + .fmt_type = FMT_YUV,
> + .mipi_dt = CIF_CSI2_DT_YUV422_8b,
> + .yuv_seq = CIF_ISP_ACQ_PROP_YCRYCB,
> + .bus_width = 16,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_UYVY8_1X16,
> + .fmt_type = FMT_YUV,
> + .mipi_dt = CIF_CSI2_DT_YUV422_8b,
> + .yuv_seq = CIF_ISP_ACQ_PROP_CBYCRY,
> + .bus_width = 16,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_VYUY8_1X16,
> + .fmt_type = FMT_YUV,
> + .mipi_dt = CIF_CSI2_DT_YUV422_8b,
> + .yuv_seq = CIF_ISP_ACQ_PROP_CRYCBY,
> + .bus_width = 16,
> + },
> +};
> +
> +static const struct ispsd_out_fmt rkisp1_isp_output_formats[] = {
> + {
> + .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8,
> + .fmt_type = FMT_YUV,
> + }, {
This is the only entry not present in the previous table, so I'm
wondering if it would make sense to merge the two tables and rename
ispsd_in_fmt to rkisp1_format_info. You would need to add a field that
tells, for each format, if it's valid as an input format, and output
format, or both. Hmmmm and also make the enum logic a bit more complex.
Maybe it's not worth it after all, but it bothers me a bit to have two
tables :-) I'll let you decide what's best.
> + .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12,
> + .fmt_type = FMT_BAYER,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12,
> + .fmt_type = FMT_BAYER,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12,
> + .fmt_type = FMT_BAYER,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12,
> + .fmt_type = FMT_BAYER,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10,
> + .fmt_type = FMT_BAYER,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10,
> + .fmt_type = FMT_BAYER,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10,
> + .fmt_type = FMT_BAYER,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10,
> + .fmt_type = FMT_BAYER,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8,
> + .fmt_type = FMT_BAYER,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8,
> + .fmt_type = FMT_BAYER,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8,
> + .fmt_type = FMT_BAYER,
> + }, {
> + .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8,
> + .fmt_type = FMT_BAYER,
> + },
> +};
> +
> +static const struct ispsd_in_fmt *find_in_fmt(u32 mbus_code)
> +{
> + unsigned int i, array_size = ARRAY_SIZE(rkisp1_isp_input_formats);
> + const struct ispsd_in_fmt *fmt;
You can move this variable inside the loop.
> +
> + for (i = 0; i < array_size; i++) {
I would remove the array_size local variable, it doesn't improve
readability. Same for the next function.
> + fmt = &rkisp1_isp_input_formats[i];
> + if (fmt->mbus_code == mbus_code)
> + return fmt;
> + }
> +
> + return NULL;
> +}
> +
> +static const struct ispsd_out_fmt *find_out_fmt(u32 mbus_code)
> +{
> + unsigned int i, array_size = ARRAY_SIZE(rkisp1_isp_output_formats);
> + const struct ispsd_out_fmt *fmt;
> +
> + for (i = 0; i < array_size; i++) {
> + fmt = &rkisp1_isp_output_formats[i];
> + if (fmt->mbus_code == mbus_code)
> + return fmt;
> + }
> +
> + return NULL;
> +}
> +
> +static int rkisp1_isp_sd_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + unsigned int i = code->index;
> +
> + if ((code->pad != RKISP1_ISP_PAD_SINK) &&
> + (code->pad != RKISP1_ISP_PAD_SOURCE_PATH)) {
> + if (i > 0)
> + return -EINVAL;
> + code->code = MEDIA_BUS_FMT_FIXED;
> + return 0;
> + }
> +
> + if (code->pad == RKISP1_ISP_PAD_SINK) {
> + if (i >= ARRAY_SIZE(rkisp1_isp_input_formats))
> + return -EINVAL;
> + code->code = rkisp1_isp_input_formats[i].mbus_code;
> + } else {
> + if (i >= ARRAY_SIZE(rkisp1_isp_output_formats))
> + return -EINVAL;
> + code->code = rkisp1_isp_output_formats[i].mbus_code;
> + }
On the other hand, merging the two tables above into one, you could
merge the two branches here and only consider formats indicated as valid
for to the pad you want. Maybe the code would be cleaner in the end.
> +
> + return 0;
> +}
> +
> +static int rkisp1_isp_sd_init_config(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg)
> +{
> + struct v4l2_rect *mf_in_crop, *mf_out_crop;
> + struct v4l2_mbus_framefmt *mf_in, *mf_out;
> +
> + mf_in = v4l2_subdev_get_try_format(sd, cfg, RKISP1_ISP_PAD_SINK);
> + mf_in->width = RKISP1_DEFAULT_WIDTH;
> + mf_in->height = RKISP1_DEFAULT_HEIGHT;
> + mf_in->field = V4L2_FIELD_NONE;
> + mf_in->code = rkisp1_isp_input_formats[0].mbus_code;
> +
> + mf_in_crop = v4l2_subdev_get_try_crop(sd, cfg, RKISP1_ISP_PAD_SINK);
> + mf_in_crop->width = RKISP1_DEFAULT_WIDTH;
> + mf_in_crop->height = RKISP1_DEFAULT_HEIGHT;
> + mf_in_crop->left = 0;
> + mf_in_crop->top = 0;
> +
> + mf_out = v4l2_subdev_get_try_format(sd, cfg,
> + RKISP1_ISP_PAD_SOURCE_PATH);
> + *mf_out = *mf_in;
> + mf_out->code = rkisp1_isp_output_formats[0].mbus_code;
> + mf_out->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> +
> + mf_out_crop = v4l2_subdev_get_try_crop(sd, cfg,
> + RKISP1_ISP_PAD_SOURCE_PATH);
> + *mf_out_crop = *mf_in_crop;
> +
> + return 0;
> +}
> +
> +static int rkisp1_isp_sd_get_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct rkisp1_isp_subdev *isp_sd = sd_to_isp_sd(sd);
> + struct v4l2_mbus_framefmt *mf = &fmt->format;
> +
> + if ((fmt->pad != RKISP1_ISP_PAD_SINK) &&
> + (fmt->pad != RKISP1_ISP_PAD_SOURCE_PATH)) {
> + fmt->format.code = MEDIA_BUS_FMT_FIXED;
> + /*
> + * NOTE: setting a format here doesn't make much sense
> + * but v4l2-compliance complains
> + */
For the params pad I agreed it makes no sense, and I think
v4l2-compliance is at fault, so I'd set width and height to 0. For the
stats pad we *could* use the size of the image from which stats are
computed, but because v4l2_meta_format has no width/height, I think 0
would also be appropriate.
> + fmt->format.width = RKISP1_DEFAULT_WIDTH;
> + fmt->format.height = RKISP1_DEFAULT_HEIGHT;
> + fmt->format.field = V4L2_FIELD_NONE;
> + return 0;
> + }
> +
> + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> + mf = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
> + fmt->format = *mf;
> + return 0;
> + }
> +
> + if (fmt->pad == RKISP1_ISP_PAD_SINK) {
> + *mf = isp_sd->in_frm;
> + } else if (fmt->pad == RKISP1_ISP_PAD_SOURCE_PATH) {
> + /* format of source pad */
> + *mf = isp_sd->in_frm;
> + mf->code = isp_sd->out_fmt.mbus_code;
> + /* window size of source pad */
> + mf->width = isp_sd->out_crop.width;
> + mf->height = isp_sd->out_crop.height;
> + mf->quantization = isp_sd->quantization;
> + }
> + mf->field = V4L2_FIELD_NONE;
This can be simplified, please read through, or jump to the review of
rkisp1_isp_subdev at the end.
> +
> + return 0;
> +}
> +
> +static void rkisp1_isp_sd_try_fmt(struct v4l2_subdev *sd,
> + unsigned int pad,
> + struct v4l2_mbus_framefmt *fmt)
> +{
> + struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
> + struct rkisp1_isp_subdev *isp_sd = &isp_dev->isp_sdev;
> + const struct ispsd_out_fmt *out_fmt;
> + const struct ispsd_in_fmt *in_fmt;
> +
> + switch (pad) {
> + case RKISP1_ISP_PAD_SINK:
> + in_fmt = find_in_fmt(fmt->code);
> + if (in_fmt)
> + fmt->code = in_fmt->mbus_code;
> + else
> + fmt->code = MEDIA_BUS_FMT_SRGGB10_1X10;
You write MEDIA_BUS_FMT_SRGGB10_1X10 explicitly here, while you use
rkisp1_isp_output_formats[0].mbus_code below (and in other places). I
would standardise on one of the two (explicit formats or array[0]), with
a preference for the first as that would allow merging the input and
output arrays more easily. I would then create two #define,
RKISP1_DEF_INPUT_FORMAT and RKISP2_DEF_OUTPUT_FORMAT (or similar).
Similar macros for the default width and height could also be useful, to
make it easier to change them.
> + fmt->width = clamp_t(u32, fmt->width, CIF_ISP_INPUT_W_MIN,
> + CIF_ISP_INPUT_W_MAX);
> + fmt->height = clamp_t(u32, fmt->height, CIF_ISP_INPUT_H_MIN,
> + CIF_ISP_INPUT_H_MAX);
> + break;
> + case RKISP1_ISP_PAD_SOURCE_PATH:
> + out_fmt = find_out_fmt(fmt->code);
> + if (out_fmt)
> + fmt->code = out_fmt->mbus_code;
> + else
> + fmt->code = rkisp1_isp_output_formats[0].mbus_code;
> + /* window size is set in s_selection */
> + fmt->width = isp_sd->out_crop.width;
> + fmt->height = isp_sd->out_crop.height;
This function operates on the TRY configuration too, in which case you
should use the TRY crop rectangle here, not the ACTIVE one. If you've
already jumped to the review of rkisp1_isp_subdev you know my proposal
to simplify this. Otherwise now may be a good time to do so :-)
> + /* full range by default */
> + if (!fmt->quantization)
> + fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> + break;
> + }
> +
> + fmt->field = V4L2_FIELD_NONE;
> +}
> +
> +static int rkisp1_isp_sd_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
> + struct rkisp1_isp_subdev *isp_sd = &isp_dev->isp_sdev;
> + struct v4l2_mbus_framefmt *mf = &fmt->format;
> +
> + if ((fmt->pad != RKISP1_ISP_PAD_SINK) &&
> + (fmt->pad != RKISP1_ISP_PAD_SOURCE_PATH))
> + return rkisp1_isp_sd_get_fmt(sd, cfg, fmt);
> +
> + rkisp1_isp_sd_try_fmt(sd, fmt->pad, mf);
> +
> + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> + struct v4l2_mbus_framefmt *try_mf;
> +
> + try_mf = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
> + *try_mf = *mf;
> + return 0;
When setting the format on the sink pad the crop rectangles need to be
reset (here, and for the ACTIVE format below too).
> + }
> +
> + if (fmt->pad == RKISP1_ISP_PAD_SINK) {
> + const struct ispsd_in_fmt *in_fmt;
> +
> + in_fmt = find_in_fmt(mf->code);
> + isp_sd->in_fmt = *in_fmt;
> + isp_sd->in_frm = *mf;
> + } else if (fmt->pad == RKISP1_ISP_PAD_SOURCE_PATH) {
> + const struct ispsd_out_fmt *out_fmt;
> +
> + /* Ignore width/height */
> + out_fmt = find_out_fmt(mf->code);
> + isp_sd->out_fmt = *out_fmt;
I would return the in_fmt and out_fmt from rkisp1_isp_sd_try_fmt() as it
already looks them up. If you merge the input and output tables, you'll
have a single format info structure type, and rkisp1_isp_sd_try_fmt()
could return the entry for the pad it operates on.
> + /*
> + * It is quantization for output,
> + * isp use bt601 limit-range in internal
> + */
> + isp_sd->quantization = mf->quantization;
> + }
> +
> + return 0;
> +}
> +
> +static void rkisp1_isp_sd_try_crop(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_selection *sel)
> +{
> + struct rkisp1_isp_subdev *isp_sd = sd_to_isp_sd(sd);
> + struct v4l2_mbus_framefmt in_frm = isp_sd->in_frm;
> + struct v4l2_rect in_crop = isp_sd->in_crop;
> + struct v4l2_rect *input = &sel->r;
> +
> + if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
> + in_frm = *v4l2_subdev_get_try_format(sd, cfg,
> + RKISP1_ISP_PAD_SINK);
> + in_crop = *v4l2_subdev_get_try_crop(sd, cfg,
> + RKISP1_ISP_PAD_SINK);
> + }
> +
> + input->left = ALIGN(input->left, 2);
> + input->width = ALIGN(input->width, 2);
> +
> + if (sel->pad == RKISP1_ISP_PAD_SINK) {
> + input->left = clamp_t(u32, input->left, 0, in_frm.width);
> + input->top = clamp_t(u32, input->top, 0, in_frm.height);
> + input->width = clamp_t(u32, input->width, CIF_ISP_INPUT_W_MIN,
> + in_frm.width - input->left);
> + input->height = clamp_t(u32, input->height,
> + CIF_ISP_INPUT_H_MIN,
> + in_frm.height - input->top);
> + } else if (sel->pad == RKISP1_ISP_PAD_SOURCE_PATH) {
> + input->left = clamp_t(u32, input->left, 0, in_crop.width);
> + input->top = clamp_t(u32, input->top, 0, in_crop.height);
> + input->width = clamp_t(u32, input->width, CIF_ISP_OUTPUT_W_MIN,
> + in_crop.width - input->left);
> + input->height = clamp_t(u32, input->height,
> + CIF_ISP_OUTPUT_H_MIN,
> + in_crop.height - input->top);
> + }
> +}
> +
> +static int rkisp1_isp_sd_get_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_selection *sel)
> +{
> + struct rkisp1_isp_subdev *isp_sd = sd_to_isp_sd(sd);
> + struct v4l2_mbus_framefmt *frm;
> + struct v4l2_rect *rect;
> +
> + if (sel->pad != RKISP1_ISP_PAD_SOURCE_PATH &&
> + sel->pad != RKISP1_ISP_PAD_SINK)
> + return -EINVAL;
> +
> + switch (sel->target) {
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + if (sel->pad == RKISP1_ISP_PAD_SINK) {
> + if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
> + frm = v4l2_subdev_get_try_format(sd, cfg,
> + sel->pad);
> + else
> + frm = &isp_sd->in_frm;
> +
> + sel->r.height = frm->height;
> + sel->r.width = frm->width;
> + sel->r.left = 0;
> + sel->r.top = 0;
> + } else {
> + if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
> + rect = v4l2_subdev_get_try_crop(sd, cfg,
> + RKISP1_ISP_PAD_SINK);
> + else
> + rect = &isp_sd->in_crop;
> + sel->r = *rect;
> + }
> + break;
> + case V4L2_SEL_TGT_CROP:
> + if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
> + rect = v4l2_subdev_get_try_crop(sd, cfg, sel->pad);
> + else if (sel->pad == RKISP1_ISP_PAD_SINK)
> + rect = &isp_sd->in_crop;
> + else
> + rect = &isp_sd->out_crop;
> + sel->r = *rect;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int rkisp1_isp_sd_set_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_selection *sel)
> +{
> + struct rkisp1_isp_subdev *isp_sd = sd_to_isp_sd(sd);
> + struct rkisp1_device *dev = sd_to_isp_dev(sd);
> +
> + if (sel->pad != RKISP1_ISP_PAD_SOURCE_PATH &&
> + sel->pad != RKISP1_ISP_PAD_SINK)
> + return -EINVAL;
> + if (sel->target != V4L2_SEL_TGT_CROP)
> + return -EINVAL;
> +
> + v4l2_dbg(1, rkisp1_debug, &dev->v4l2_dev,
> + "%s: pad: %d sel(%d,%d)/%dx%d\n", __func__, sel->pad,
> + sel->r.left, sel->r.top, sel->r.width, sel->r.height);
> + rkisp1_isp_sd_try_crop(sd, cfg, sel);
> +
> + if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
> + struct v4l2_rect *try_sel =
> + v4l2_subdev_get_try_crop(sd, cfg, sel->pad);
> +
> + *try_sel = sel->r;
> + return 0;
When setting the crop rectangle on the sink pad the rectangle on the
source pad should be reset (and below for the ACTIVE format too).
The resizer logic, through selection rectangles, seems to be missing. I
assume it's configured through the video nodes, but that's not correct
I'm afraid, scaling should be configured on subdevs, see [1]. I'm afraid
this means that we'll need separate subdevs for the resizers :-S
[1] https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-subdev.html#order-of-configuration-and-format-propagation
> + }
> +
> + if (sel->pad == RKISP1_ISP_PAD_SINK)
> + isp_sd->in_crop = sel->r;
> + else
> + isp_sd->out_crop = sel->r;
> +
> + return 0;
> +}
> +
I'll skip the part related to power on/off below as Sakari requested it
to be handled through runtime PM.
> +static int mipi_csi2_s_stream_start(struct rkisp1_isp_subdev *isp_sd,
> + struct rkisp1_sensor *sensor)
> +{
> + union phy_configure_opts opts = { 0 };
> + struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy;
> + struct v4l2_ctrl *pixel_rate;
> + s64 pixel_clock;
> +
> + pixel_rate = v4l2_ctrl_find(sensor->sd->ctrl_handler,
> + V4L2_CID_PIXEL_RATE);
> + if (!pixel_rate) {
> + v4l2_warn(sensor->sd, "No pixel rate control in subdev\n");
> + return -EPIPE;
> + }
Would it make sense to retrieve (and cache) the control pointer (not its
value of course) at probe time already ?
> +
> + pixel_clock = v4l2_ctrl_g_ctrl_int64(pixel_rate);
> + if (!pixel_clock) {
> + v4l2_err(sensor->sd, "Invalid pixel rate value\n");
> + return -EINVAL;
> + }
> +
> + phy_mipi_dphy_get_default_config(pixel_clock, isp_sd->in_fmt.bus_width,
> + sensor->lanes, cfg);
> + phy_set_mode(sensor->dphy, PHY_MODE_MIPI_DPHY);
> + phy_configure(sensor->dphy, &opts);
> + phy_power_on(sensor->dphy);
> +
> + return 0;
> +}
> +
> +static void mipi_csi2_s_stream_stop(struct rkisp1_sensor *sensor)
> +{
> + phy_power_off(sensor->dphy);
> +}
> +
> +static int rkisp1_isp_sd_s_stream(struct v4l2_subdev *sd, int on)
> +{
> + struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
> + struct v4l2_subdev *sensor_sd;
> + int ret = 0;
> +
> + if (!on) {
> + ret = rkisp1_isp_stop(isp_dev);
> + if (ret < 0)
> + return ret;
> + mipi_csi2_s_stream_stop(isp_dev->active_sensor);
> + return 0;
> + }
> +
> + sensor_sd = get_remote_sensor(sd);
> + if (!sensor_sd)
> + return -ENODEV;
> +
> + isp_dev->active_sensor = sd_to_sensor(isp_dev, sensor_sd);
> + if (!isp_dev->active_sensor)
> + return -ENODEV;
> +
> + atomic_set(&isp_dev->isp_sdev.frm_sync_seq, 0);
> + ret = rkisp1_config_cif(isp_dev);
> + if (ret < 0)
> + return ret;
> +
> + /* TODO: support other interfaces */
> + if (isp_dev->active_sensor->mbus.type != V4L2_MBUS_CSI2_DPHY)
> + return -EINVAL;
> +
> + ret = mipi_csi2_s_stream_start(&isp_dev->isp_sdev,
> + isp_dev->active_sensor);
> + if (ret < 0)
> + return ret;
> +
> + ret = rkisp1_isp_start(isp_dev);
> + if (ret)
> + mipi_csi2_s_stream_stop(isp_dev->active_sensor);
> +
> + return ret;
> +}
> +
> +static int rkisp1_isp_sd_s_power(struct v4l2_subdev *sd, int on)
> +{
> + struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
> + int ret;
> +
> + v4l2_dbg(1, rkisp1_debug, &isp_dev->v4l2_dev, "s_power: %d\n", on);
> +
> + if (on) {
> + ret = pm_runtime_get_sync(isp_dev->dev);
> + if (ret < 0)
> + return ret;
> +
> + rkisp1_config_clk(isp_dev);
> + } else {
> + ret = pm_runtime_put(isp_dev->dev);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int rkisp1_subdev_link_validate(struct media_link *link)
> +{
> + if (link->source->index == RKISP1_ISP_PAD_SINK_PARAMS)
> + return 0;
> +
> + return v4l2_subdev_link_validate(link);
> +}
> +
> +static int rkisp1_subdev_fmt_link_validate(struct v4l2_subdev *sd,
> + struct media_link *link,
> + struct v4l2_subdev_format *source_fmt,
> + struct v4l2_subdev_format *sink_fmt)
> +{
> + if (source_fmt->format.code != sink_fmt->format.code)
> + return -EINVAL;
> +
> + /* Crop is available */
> + if (source_fmt->format.width < sink_fmt->format.width ||
> + source_fmt->format.height < sink_fmt->format.height)
> + return -EINVAL;
Crop should be handled on pads, not on links. The size at the ends of
the link should match. This will likely require a bit of a redesign of
the format and selection rectangles handling, but I think it's also an
opportunity to simplify the code.
> +
> + return 0;
> +}
> +
> +static void rkisp1_isp_queue_event_sof(struct rkisp1_isp_subdev *isp)
> +{
> + struct v4l2_event event = {
> + .type = V4L2_EVENT_FRAME_SYNC,
> + .u.frame_sync.frame_sequence =
> + atomic_inc_return(&isp->frm_sync_seq) - 1,
I would move the increment to the caller, hiding it in this function is
error-prone (and if you look at the caller I'm pointing out one possible
error :-)).
In general usage of frm_sync_seq through the driver seems to be very
race-prone. It's read in various IRQ handling functions, all coming from
the same IRQ, so that part is fine (and wouldn't require an atomic
variable), but when read from the buffer queue handlers I really get a
red light flashing in my head. I'll try to investigate more when
reviewing the next patches.
> + };
> + v4l2_event_queue(isp->sd.devnode, &event);
> +}
> +
> +static int rkisp1_isp_sd_subs_evt(struct v4l2_subdev *sd, struct v4l2_fh *fh,
> + struct v4l2_event_subscription *sub)
> +{
> + if (sub->type != V4L2_EVENT_FRAME_SYNC)
> + return -EINVAL;
> +
> + /* Line number. For now only zero accepted. */
Is the id a line number for V4L2_EVENT_FRAME_SYNC ? It's not mentioned
for V4L2_EVENT_FRAME_SYNC in the V4L2 spec, so I think this check is
correct, but the comment doesn't seem to be.
> + if (sub->id != 0)
> + return -EINVAL;
> +
> + return v4l2_event_subscribe(fh, sub, 0, NULL);
> +}
> +
> +static const struct v4l2_subdev_pad_ops rkisp1_isp_sd_pad_ops = {
> + .enum_mbus_code = rkisp1_isp_sd_enum_mbus_code,
> + .get_selection = rkisp1_isp_sd_get_selection,
> + .set_selection = rkisp1_isp_sd_set_selection,
> + .init_cfg = rkisp1_isp_sd_init_config,
> + .get_fmt = rkisp1_isp_sd_get_fmt,
> + .set_fmt = rkisp1_isp_sd_set_fmt,
> + .link_validate = rkisp1_subdev_fmt_link_validate,
> +};
> +
> +static const struct media_entity_operations rkisp1_isp_sd_media_ops = {
> + .link_validate = rkisp1_subdev_link_validate,
> +};
> +
> +static const struct v4l2_subdev_video_ops rkisp1_isp_sd_video_ops = {
> + .s_stream = rkisp1_isp_sd_s_stream,
> +};
> +
> +static const struct v4l2_subdev_core_ops rkisp1_isp_core_ops = {
> + .subscribe_event = rkisp1_isp_sd_subs_evt,
> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> + .s_power = rkisp1_isp_sd_s_power,
> +};
> +
> +static struct v4l2_subdev_ops rkisp1_isp_sd_ops = {
static const
> + .core = &rkisp1_isp_core_ops,
> + .video = &rkisp1_isp_sd_video_ops,
> + .pad = &rkisp1_isp_sd_pad_ops,
> +};
> +
> +static void rkisp1_isp_sd_init_default_fmt(struct rkisp1_isp_subdev *isp_sd)
> +{
> + struct v4l2_mbus_framefmt *in_frm = &isp_sd->in_frm;
> + struct v4l2_rect *in_crop = &isp_sd->in_crop;
> + struct v4l2_rect *out_crop = &isp_sd->out_crop;
> + struct ispsd_in_fmt *in_fmt = &isp_sd->in_fmt;
> + struct ispsd_out_fmt *out_fmt = &isp_sd->out_fmt;
> +
> + *in_fmt = rkisp1_isp_input_formats[0];
> + in_frm->width = RKISP1_DEFAULT_WIDTH;
> + in_frm->height = RKISP1_DEFAULT_HEIGHT;
> + in_frm->code = in_fmt->mbus_code;
> +
> + in_crop->width = in_frm->width;
> + in_crop->height = in_frm->height;
> + in_crop->left = 0;
> + in_crop->top = 0;
> +
> + /* propagate to source */
> + *out_crop = *in_crop;
> + *out_fmt = rkisp1_isp_output_formats[0];
> + isp_sd->quantization = V4L2_QUANTIZATION_FULL_RANGE;
I wonder, could this be implemented by sharing code with .init_cfg() if
you store the active configuration in v4l2_subdev_pad_config instances
(please see the comments about the rkisp1_isp_subdev structure below) ?
> +}
> +
> +int rkisp1_register_isp_subdev(struct rkisp1_device *isp_dev,
> + struct v4l2_device *v4l2_dev)
> +{
> + struct rkisp1_isp_subdev *isp_sdev = &isp_dev->isp_sdev;
> + struct v4l2_subdev *sd = &isp_sdev->sd;
> + int ret;
> +
> + v4l2_subdev_init(sd, &rkisp1_isp_sd_ops);
> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> + sd->entity.ops = &rkisp1_isp_sd_media_ops;
> + snprintf(sd->name, sizeof(sd->name), "rkisp1-isp-subdev");
> +
> + isp_sdev->pads[RKISP1_ISP_PAD_SINK].flags =
> + MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
> + isp_sdev->pads[RKISP1_ISP_PAD_SINK_PARAMS].flags = MEDIA_PAD_FL_SINK;
> + isp_sdev->pads[RKISP1_ISP_PAD_SOURCE_PATH].flags = MEDIA_PAD_FL_SOURCE;
> + isp_sdev->pads[RKISP1_ISP_PAD_SOURCE_STATS].flags = MEDIA_PAD_FL_SOURCE;
> + sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> + ret = media_entity_pads_init(&sd->entity, RKISP1_ISP_PAD_MAX,
> + isp_sdev->pads);
> + if (ret < 0)
> + return ret;
> +
> + sd->owner = THIS_MODULE;
> + v4l2_set_subdevdata(sd, isp_dev);
> +
> + sd->grp_id = GRP_ID_ISP;
I think you can drop this, as well as all the GRP_ID_* macros, they're
not used.
> + ret = v4l2_device_register_subdev(v4l2_dev, sd);
> + if (ret < 0) {
> + v4l2_err(sd, "Failed to register isp subdev\n");
> + goto err_cleanup_media_entity;
> + }
> +
> + rkisp1_isp_sd_init_default_fmt(isp_sdev);
> +
> + return 0;
> +err_cleanup_media_entity:
> + media_entity_cleanup(&sd->entity);
> + return ret;
> +}
> +
> +void rkisp1_unregister_isp_subdev(struct rkisp1_device *isp_dev)
> +{
> + struct v4l2_subdev *sd = &isp_dev->isp_sdev.sd;
> +
> + v4l2_device_unregister_subdev(sd);
> + media_entity_cleanup(&sd->entity);
> +}
> +
> +/**************** Interrupter Handler ****************/
s/Handler/Handlers/
> +
> +void rkisp1_mipi_isr(unsigned int mis, struct rkisp1_device *dev)
I would make the mis field an u32 as it contains register values. Should
it also be renamed to status ? Same for rkisp1_isp_isr() below.
> +{
> + struct v4l2_device *v4l2_dev = &dev->v4l2_dev;
> + void __iomem *base = dev->base_addr;
> + u32 val;
> +
How about moving the CIF_MIPI_MIS read here and removing the mis
argument to the function ? It would be more logical to group all
register access related to interrupts in a single place. Same for the
other interrupt handler functions.
> + writel(~0, base + CIF_MIPI_ICR);
> +
> + /*
> + * Disable DPHY errctrl interrupt, because this dphy
> + * erctrl signal is asserted until the next changes
> + * of line state. This time is may be too long and cpu
> + * is hold in this interrupt.
> + */
> + if (mis & CIF_MIPI_ERR_CTRL(0x0f)) {
> + val = readl(base + CIF_MIPI_IMSC);
> + writel(val & ~CIF_MIPI_ERR_CTRL(0x0f), base + CIF_MIPI_IMSC);
> + dev->isp_sdev.dphy_errctrl_disabled = true;
> + }
> +
> + /*
> + * Enable DPHY errctrl interrupt again, if mipi have receive
> + * the whole frame without any error.
> + */
> + if (mis == CIF_MIPI_FRAME_END) {
> + /*
> + * Enable DPHY errctrl interrupt again, if mipi have receive
> + * the whole frame without any error.
> + */
> + if (dev->isp_sdev.dphy_errctrl_disabled) {
> + val = readl(base + CIF_MIPI_IMSC);
> + val |= CIF_MIPI_ERR_CTRL(0x0f);
> + writel(val, base + CIF_MIPI_IMSC);
> + dev->isp_sdev.dphy_errctrl_disabled = false;
> + }
> + } else {
> + v4l2_warn(v4l2_dev, "MIPI mis error: 0x%08x\n", mis);
> + }
> +}
> +
> +void rkisp1_isp_isr(unsigned int isp_mis, struct rkisp1_device *dev)
> +{
> + void __iomem *base = dev->base_addr;
> + unsigned int isp_mis_tmp = 0;
_tmp are never good names :-S
> + unsigned int isp_err = 0;
Neither of these variable need to be initialised to 0.
> +
> + /* start edge of v_sync */
> + if (isp_mis & CIF_ISP_V_START) {
> + rkisp1_isp_queue_event_sof(&dev->isp_sdev);
This will increment the frame sequence number. What if the interrupt is
slightly delayed and the next frame starts before we get a change to
copy the sequence number to the buffers (before they will complete
below) ?
> +
> + writel(CIF_ISP_V_START, base + CIF_ISP_ICR);
Do you need to clear all interrupt bits individually, can't you write
isp_mis to CIF_ISP_ICR at the beginning of the function to clear them
all in one go ?
> + isp_mis_tmp = readl(base + CIF_ISP_MIS);
> + if (isp_mis_tmp & CIF_ISP_V_START)
> + v4l2_err(&dev->v4l2_dev, "isp icr v_statr err: 0x%x\n",
> + isp_mis_tmp);
This require some explanation. It looks like a naive way to protect
against something, but I think it could trigger under normal
circumstances if IRQ handling is delayed, and wouldn't do much anyway.
Same for the similar constructs below.
> + }
> +
> + if ((isp_mis & CIF_ISP_PIC_SIZE_ERROR)) {
> + /* Clear pic_size_error */
> + writel(CIF_ISP_PIC_SIZE_ERROR, base + CIF_ISP_ICR);
> + isp_err = readl(base + CIF_ISP_ERR);
> + v4l2_err(&dev->v4l2_dev,
> + "CIF_ISP_PIC_SIZE_ERROR (0x%08x)", isp_err);
What does this mean ?
> + writel(isp_err, base + CIF_ISP_ERR_CLR);
> + } else if ((isp_mis & CIF_ISP_DATA_LOSS)) {
Are CIF_ISP_PIC_SIZE_ERROR and CIF_ISP_DATA_LOSS mutually exclusive ?
> + /* Clear data_loss */
> + writel(CIF_ISP_DATA_LOSS, base + CIF_ISP_ICR);
> + v4l2_err(&dev->v4l2_dev, "CIF_ISP_DATA_LOSS\n");
> + writel(CIF_ISP_DATA_LOSS, base + CIF_ISP_ICR);
> + }
> +
> + /* sampled input frame is complete */
> + if (isp_mis & CIF_ISP_FRAME_IN) {
> + writel(CIF_ISP_FRAME_IN, base + CIF_ISP_ICR);
> + isp_mis_tmp = readl(base + CIF_ISP_MIS);
> + if (isp_mis_tmp & CIF_ISP_FRAME_IN)
> + v4l2_err(&dev->v4l2_dev, "isp icr frame_in err: 0x%x\n",
> + isp_mis_tmp);
> + }
> +
> + /* frame was completely put out */
"put out" ? :-) What's the difference between ISP_FRAME_IN and ISP_FRAME
? The two comments could do with a bit of brush up, and I think the
ISP_FRAME_IN interrupt could be disabled as it doesn't perform any
action.
> + if (isp_mis & CIF_ISP_FRAME) {
> + u32 isp_ris = 0;
No need to initialise this to 0.
> + /* Clear Frame In (ISP) */
> + writel(CIF_ISP_FRAME, base + CIF_ISP_ICR);
> + isp_mis_tmp = readl(base + CIF_ISP_MIS);
> + if (isp_mis_tmp & CIF_ISP_FRAME)
> + v4l2_err(&dev->v4l2_dev,
> + "isp icr frame end err: 0x%x\n", isp_mis_tmp);
> +
> + isp_ris = readl(base + CIF_ISP_RIS);
> + if (isp_ris & (CIF_ISP_AWB_DONE | CIF_ISP_AFM_FIN |
> + CIF_ISP_EXP_END | CIF_ISP_HIST_MEASURE_RDY))
> + rkisp1_stats_isr(&dev->stats_vdev, isp_ris);
Is there a guarantee that the statistics will be fully written out
before the video frame itself ? And doesn't this test if any of the
statistics is complete, not all of them ? I think the logic is wrong, it
seems it should be moved out of the CIF_ISP_FRAME test, to a test of its
own. It's hard to tell for sure without extra information though (for
instance why are the stats-related bits read from CIF_ISP_RIS, when
they seem to be documented as valid in CIF_ISP_ISR), but this should be
validated, and most probably fixed. Care should be taken to keep
synchronisation of sequence number between the different queues.
> + }
> +
> + /*
> + * Then update changed configs. Some of them involve
> + * lot of register writes. Do those only one per frame.
> + * Do the updates in the order of the processing flow.
> + */
> + rkisp1_params_isr(&dev->params_vdev, isp_mis);
You should pass dev to this function, and use it internally for the I/O
accessors instead of going through the vdev.
> +}
> diff --git a/drivers/media/platform/rockchip/isp1/rkisp1.h b/drivers/media/platform/rockchip/isp1/rkisp1.h
> new file mode 100644
> index 000000000000..b0366e354ec2
> --- /dev/null
> +++ b/drivers/media/platform/rockchip/isp1/rkisp1.h
> @@ -0,0 +1,111 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Rockchip isp1 driver
> + *
> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> + */
> +
> +#ifndef _RKISP1_H
> +#define _RKISP1_H
> +
> +#include <linux/platform_device.h>
> +#include <media/v4l2-fwnode.h>
I don't think you need these headers. You however need
#include <media/v4l2-ctrls.h>
#include <media/v4l2-subdev.h>
to make this header self-contained. Other headers could be added too
(videodev2.h, v4l2-mediabus.h and media-entity.h), they're included
through the above two headers at the moment.
> +
> +#include "common.h"
> +
> +struct rkisp1_stream;
> +
> +/*
> + * struct ispsd_in_fmt - ISP intput-pad format
> + *
> + * Translate mbus_code to hardware format values
> + *
> + * @bus_width: used for parallel
> + */
> +struct ispsd_in_fmt {
> + u32 mbus_code;
> + u8 fmt_type;
> + u32 mipi_dt;
> + u32 yuv_seq;
> + enum rkisp1_fmt_raw_pat_type bayer_pat;
> + u8 bus_width;
> +};
Some of the structures are prefixed with ispsd_, some with rkisp1_. I
think we should standardise on a single prefix.
> +
> +struct ispsd_out_fmt {
> + u32 mbus_code;
> + u8 fmt_type;
> +};
> +
> +struct rkisp1_ie_config {
> + unsigned int effect;
> +};
This structure isn't used.
> +
> +enum rkisp1_isp_pad {
> + RKISP1_ISP_PAD_SINK,
Should this be named SINK_VIDEO to match SINK_PARAMS ?
> + RKISP1_ISP_PAD_SINK_PARAMS,
> + RKISP1_ISP_PAD_SOURCE_PATH,
And SOURCE_VIDEO ?
> + RKISP1_ISP_PAD_SOURCE_STATS,
> + RKISP1_ISP_PAD_MAX
> +};
> +
> +/*
> + * struct rkisp1_isp_subdev - ISP sub-device
> + *
> + * See Cropping regions of ISP in rkisp1.c for details
Could you please document all fields from the structure ?
> + * @in_frm: input size, don't have to be equal to sensor size
> + * @in_fmt: input format
> + * @in_crop: crop for sink pad
> + * @out_fmt: output format
> + * @out_crop: output size
> + *
> + * @dphy_errctrl_disabled: if dphy errctrl is disabled(avoid endless interrupt)
s/disabled/disabled /
> + * @frm_sync_seq: frame sequence, to sync frame_id between video devices.
> + * @quantization: output quantization
> + */
> +struct rkisp1_isp_subdev {
> + struct v4l2_subdev sd;
> + struct media_pad pads[RKISP1_ISP_PAD_MAX];
> + struct v4l2_ctrl_handler ctrl_handler;
> + struct v4l2_mbus_framefmt in_frm;
> + struct ispsd_in_fmt in_fmt;
I think this (and out_fmt) could be stored as pointers.
> + struct v4l2_rect in_crop;
> + struct ispsd_out_fmt out_fmt;
> + struct v4l2_rect out_crop;
I think this could be greatly simplified if you stored all the data as
v4l2_subdev_pad_config instances for both the sink and source video pads
instead of using ad-hoc structures. In particular your get and set
format handlers would become much simpler. I recommend looking at the
mt9p031 sensor driver for an example. In a nutshell, the get handler
would retrieve the format from the v4l2_subdev_pad_config instance
either from rkisp1_isp_subdev (for V4L2_SUBDEV_FORMAT_ACTIVE) or from
the passed cfg argument (for V4L2_SUBDEV_FORMAT_TRY). The set handler
would operate similarly, with just a test one ACTIVE/TRY at the
beginning, followed by common code that only operates on the cfg, and
finally storing the format in rkisp1_isp_subdev for
V4L2_SUBDEV_FORMAT_ACTIVE.
> + bool dphy_errctrl_disabled;
> + atomic_t frm_sync_seq;
> + enum v4l2_quantization quantization;
The quantization would then be stored in the v4l2_subdev_pad_config too.
> +};
> +
> +int rkisp1_register_isp_subdev(struct rkisp1_device *isp_dev,
> + struct v4l2_device *v4l2_dev);
> +
> +void rkisp1_unregister_isp_subdev(struct rkisp1_device *isp_dev);
> +
> +void rkisp1_mipi_isr(unsigned int mipi_mis, struct rkisp1_device *dev);
> +
> +void rkisp1_isp_isr(unsigned int isp_mis, struct rkisp1_device *dev);
> +
> +static inline
> +struct ispsd_out_fmt *rkisp1_get_ispsd_out_fmt(struct rkisp1_isp_subdev *isp_sdev)
> +{
> + return &isp_sdev->out_fmt;
> +}
> +
> +static inline
> +struct ispsd_in_fmt *rkisp1_get_ispsd_in_fmt(struct rkisp1_isp_subdev *isp_sdev)
> +{
> + return &isp_sdev->in_fmt;
> +}
> +
> +static inline
> +struct v4l2_rect *rkisp1_get_isp_sd_win(struct rkisp1_isp_subdev *isp_sdev)
> +{
> + return &isp_sdev->out_crop;
> +}
I agree with Sakari that accessing fields directly would be more
readable.
> +
> +static inline struct rkisp1_isp_subdev *sd_to_isp_sd(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct rkisp1_isp_subdev, sd);
> +}
> +
> +#endif /* _RKISP1_H */
--
Regards,
Laurent Pinchart
_______________________________________________
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 v8 10/14] dt-bindings: Document the Rockchip ISP1 bindings
From: Laurent Pinchart @ 2019-08-16 0:21 UTC (permalink / raw)
To: Helen Koike
Cc: devicetree, eddie.cai.linux, kernel, heiko, Rob Herring,
jacob2.chen, jeffy.chen, zyc, linux-kernel, tfiga, linux-rockchip,
hans.verkuil, sakari.ailus, zhengsq, mchehab, ezequiel,
linux-arm-kernel, linux-media
In-Reply-To: <20190730184256.30338-11-helen.koike@collabora.com>
Hi Helen,
Thank you for the patch.
On Tue, Jul 30, 2019 at 03:42:52PM -0300, Helen Koike wrote:
> From: Jacob Chen <jacob2.chen@rock-chips.com>
>
> Add DT bindings documentation for Rockchip ISP1
>
> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> [update for upstream]
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>
> ---
>
> Changes in v8: None
> Changes in v7:
> - update document with new design and tested example
>
> .../bindings/media/rockchip-isp1.txt | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/rockchip-isp1.txt
>
> diff --git a/Documentation/devicetree/bindings/media/rockchip-isp1.txt b/Documentation/devicetree/bindings/media/rockchip-isp1.txt
> new file mode 100644
> index 000000000000..a97fef0f189f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rockchip-isp1.txt
If there wasn't enough work on your plate already I'd propose converting
this to yaml, but I'll refrain from doing so :-)
> @@ -0,0 +1,71 @@
> +Rockchip SoC Image Signal Processing unit v1
> +----------------------------------------------
> +
> +Rockchip ISP1 is the Camera interface for the Rockchip series of SoCs
> +which contains image processing, scaling, and compression funcitons.
s/funcitons/functions/
> +
> +Required properties:
> +- compatible: value should be one of the following
> + "rockchip,rk3288-cif-isp";
> + "rockchip,rk3399-cif-isp";
> +- reg : offset and length of the register set for the device.
> +- interrupts: should contain ISP interrupt.
> +- clocks: phandle to the required clocks.
> +- clock-names: required clock name.
> +- iommus: required a iommu node.
> +- phys: the phandle for the PHY port
> +- phy-names: must contain "dphy"
> +
> +port node
> +-------------------
> +
> +The device node should contain one 'ports' child node, with children 'port'
> +with child 'endpoint'.
Extra . and line break ?
> +nodes, according to the bindings defined in Documentation/devicetree/bindings/
> +media/video-interfaces.txt.
> +
> +- endpoint(mipi):
> + - remote-endpoint: Connecting to Rockchip MIPI-DPHY,
> + which is defined in rockchip-mipi-dphy.txt.
> +
> +The port node must contain at least one endpoint, either parallel or mipi.
If I understand things correctly, each ISP has a single CSI-2 receiver
and a single parallel output, and can select one of them at runtime.
This should be modelled as two separate ports. In addition to this,
multiple CSI-2 sensors can be connected to the same CSI-2 receiver as
long as all but one of them is held in reset (this is a poor man's CSI-2
switch, which exists in device out in the market, so we have to support
that). This should be modelled by multiple endpoints in the CSI-2 port.
> +It could have multiple endpoints, but please note the hardware don't support
> +two sensors work at a time, they are supposed to work asynchronously.
I assume you meant "are supposed to be mutually exclusive" or something
similar ?
> +
> +Device node example
> +-------------------
> +
> + isp0: isp0@ff910000 {
> + compatible = "rockchip,rk3399-cif-isp";
> + reg = <0x0 0xff910000 0x0 0x4000>;
> + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru SCLK_ISP0>,
> + <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
> + <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
> + clock-names = "clk_isp",
> + "aclk_isp", "aclk_isp_wrap",
> + "hclk_isp", "hclk_isp_wrap";
> + power-domains = <&power RK3399_PD_ISP0>;
> + iommus = <&isp0_mmu>;
> + phys = <&dphy>;
> + phy-names = "dphy";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + mipi_in_wcam: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&wcam_out>;
> + data-lanes = <1 2>;
> + };
> +
> + mipi_in_ucam: endpoint@1 {
> + reg = <1>;
> + remote-endpoint = <&ucam_out>;
> + data-lanes = <1>;
> + };
What are wcam and ucam ? It would help if you showed the sensor nodes in
the example.
> + };
> + };
> + };
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH V6 2/4] arm64: dts: imx8mm: Add system counter node
From: Anson Huang @ 2019-08-16 0:38 UTC (permalink / raw)
To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
daniel.lezcano, tglx, leonard.crestez, daniel.baluta, ping.bai,
jun.li, l.stach, abel.vesa, ccaione, andrew.smirnov, angus, agx,
devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
In-Reply-To: <1565915925-21009-1-git-send-email-Anson.Huang@nxp.com>
Add i.MX8MM system counter node to enable timer-imx-sysctr
broadcast timer driver.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No changes.
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index ef66bb5..94433c53 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -572,6 +572,14 @@
#pwm-cells = <2>;
status = "disabled";
};
+
+ system_counter: timer@306a0000 {
+ compatible = "nxp,sysctr-timer";
+ reg = <0x306a0000 0x20000>;
+ interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&osc_24m>;
+ clock-names = "per";
+ };
};
aips3: bus@30800000 {
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH V6 1/4] clocksource: imx-sysctr: Add internal clock divider handle
From: Anson Huang @ 2019-08-16 0:38 UTC (permalink / raw)
To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
daniel.lezcano, tglx, leonard.crestez, daniel.baluta, ping.bai,
jun.li, l.stach, abel.vesa, ccaione, andrew.smirnov, angus, agx,
devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
The system counter block guide states that the base clock is
internally divided by 3 before use, that means the clock input of
system counter defined in DT should be base clock which is normally
from OSC, and then internally divided by 3 before use.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No changes.
---
drivers/clocksource/timer-imx-sysctr.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/clocksource/timer-imx-sysctr.c b/drivers/clocksource/timer-imx-sysctr.c
index fd7d680..b7c80a3 100644
--- a/drivers/clocksource/timer-imx-sysctr.c
+++ b/drivers/clocksource/timer-imx-sysctr.c
@@ -20,6 +20,8 @@
#define SYS_CTR_EN 0x1
#define SYS_CTR_IRQ_MASK 0x2
+#define SYS_CTR_CLK_DIV 0x3
+
static void __iomem *sys_ctr_base;
static u32 cmpcr;
@@ -134,6 +136,9 @@ static int __init sysctr_timer_init(struct device_node *np)
if (ret)
return ret;
+ /* system counter clock is divided by 3 internally */
+ to_sysctr.of_clk.rate /= SYS_CTR_CLK_DIV;
+
sys_ctr_base = timer_of_base(&to_sysctr);
cmpcr = readl(sys_ctr_base + CMPCR);
cmpcr &= ~SYS_CTR_EN;
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH V6 3/4] arm64: dts: imx8mq: Add system counter node
From: Anson Huang @ 2019-08-16 0:38 UTC (permalink / raw)
To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
daniel.lezcano, tglx, leonard.crestez, daniel.baluta, ping.bai,
jun.li, l.stach, abel.vesa, ccaione, andrew.smirnov, angus, agx,
devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
In-Reply-To: <1565915925-21009-1-git-send-email-Anson.Huang@nxp.com>
Add i.MX8MQ system counter node to enable timer-imx-sysctr
broadcast timer driver.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No changes.
---
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 7c68df6..4fdd60f 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -644,6 +644,14 @@
#pwm-cells = <2>;
status = "disabled";
};
+
+ system_counter: timer@306a0000 {
+ compatible = "nxp,sysctr-timer";
+ reg = <0x306a0000 0x20000>;
+ interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&osc_25m>;
+ clock-names = "per";
+ };
};
bus@30800000 { /* AIPS3 */
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH V6 4/4] arm64: dts: imx8mm: Enable cpu-idle driver
From: Anson Huang @ 2019-08-16 0:38 UTC (permalink / raw)
To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
daniel.lezcano, tglx, leonard.crestez, daniel.baluta, ping.bai,
jun.li, l.stach, abel.vesa, ccaione, andrew.smirnov, angus, agx,
devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
In-Reply-To: <1565915925-21009-1-git-send-email-Anson.Huang@nxp.com>
Enable i.MX8MM cpu-idle using generic ARM cpu-idle driver, 2 states
are supported, details as below:
root@imx8mmevk:~# cat /sys/devices/system/cpu/cpu0/cpuidle/state0/name
WFI
root@imx8mmevk:~# cat /sys/devices/system/cpu/cpu0/cpuidle/state0/usage
3973
root@imx8mmevk:~# cat /sys/devices/system/cpu/cpu0/cpuidle/state1/name
cpu-pd-wait
root@imx8mmevk:~# cat /sys/devices/system/cpu/cpu0/cpuidle/state1/usage
6647
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V5:
- improve state1 idle name to better match PSCI doc;
- remove wakeup-latency-us property as it is NOT necessary when entry-latency-us/exit-latency-us
exist.
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 94433c53..9b2dc12 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -44,6 +44,19 @@
#address-cells = <1>;
#size-cells = <0>;
+ idle-states {
+ entry-method = "psci";
+
+ cpu_pd_wait: cpu-pd-wait {
+ compatible = "arm,idle-state";
+ arm,psci-suspend-param = <0x0010033>;
+ local-timer-stop;
+ entry-latency-us = <1000>;
+ exit-latency-us = <700>;
+ min-residency-us = <2700>;
+ };
+ };
+
A53_0: cpu@0 {
device_type = "cpu";
compatible = "arm,cortex-a53";
@@ -56,6 +69,7 @@
nvmem-cells = <&cpu_speed_grade>;
nvmem-cell-names = "speed_grade";
#cooling-cells = <2>;
+ cpu-idle-states = <&cpu_pd_wait>;
};
A53_1: cpu@1 {
@@ -68,6 +82,7 @@
next-level-cache = <&A53_L2>;
operating-points-v2 = <&a53_opp_table>;
#cooling-cells = <2>;
+ cpu-idle-states = <&cpu_pd_wait>;
};
A53_2: cpu@2 {
@@ -80,6 +95,7 @@
next-level-cache = <&A53_L2>;
operating-points-v2 = <&a53_opp_table>;
#cooling-cells = <2>;
+ cpu-idle-states = <&cpu_pd_wait>;
};
A53_3: cpu@3 {
@@ -92,6 +108,7 @@
next-level-cache = <&A53_L2>;
operating-points-v2 = <&a53_opp_table>;
#cooling-cells = <2>;
+ cpu-idle-states = <&cpu_pd_wait>;
};
A53_L2: l2-cache0 {
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* RE: [PATCH V5 5/5] arm64: dts: imx8mm: Enable cpu-idle driver
From: Anson Huang @ 2019-08-16 1:03 UTC (permalink / raw)
To: Daniel Lezcano, catalin.marinas@arm.com, will@kernel.org,
robh+dt@kernel.org, mark.rutland@arm.com, shawnguo@kernel.org,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
dl-linux-imx, tglx@linutronix.de, Leonard Crestez, Aisheng Dong,
Daniel Baluta, Jacky Bai, l.stach@pengutronix.de, Abel Vesa,
andrew.smirnov@gmail.com, ccaione@baylibre.com, angus@akkea.ca,
agx@sigxcpu.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <34c03d76-ae61-63b4-153f-3f9911cc962e@linaro.org>
Hi, Daniel
> Hi Anson,
>
> sorry for the late review, I've been pretty busy.
That is OK for sure.
>
> If Shawn is ok, I can pick the patches 1-4 in my tree and then this one after
> you fix the comments below.
Shawn should be OK for it, and he already took patch [PATCH V5 2/5] arm64: Enable TIMER_IMX_SYS_CTR for ARCH_MXC platforms
since I ever sent it before in other series when system counter driver is NOT landing on main line, now it landed, Shawn just apply
that old patch, so in V6 patch I just sent, I did NOT include this patch, you can just apply the 4 patches in V6.
Hi, Shawn
Daniel will pick this whole patch series, please raise if you have any concern, thanks.
>
> On 10/07/2019 08:30, Anson.Huang@nxp.com wrote:
>
> [ ... ]
>
> > + idle-states {
> > + entry-method = "psci";
> > +
> > + cpu_sleep_wait: cpu-sleep-wait {
>
> Is that a retention state or a powerdown? It is preferable to change the name
> to the idle state naming convention given in the PSCI documentation [1] page
> 16-17
Thanks for your detail reference, it is a power down state with SoC entering WAIT mode,
so in V6, I change the name to "cpu_pd_wait:cpu-pd-wait".
>
>
> > + compatible = "arm,idle-state";
> > + arm,psci-suspend-param = <0x0010033>;
> > + local-timer-stop;
> > + entry-latency-us = <1000>;
> > + exit-latency-us = <700>;
> > + min-residency-us = <2700>;
> > + wakeup-latency-us = <1500>;
>
> It is pointless to specify the entry + exit *and* the wakeup-latency [2].
Ah, yes, this is new to me, I will just remove the “wakeup-latency-us” property.
Thanks,
Anson.
_______________________________________________
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] perf machine: arm/arm64: Improve completeness for kernel address space
From: Leo Yan @ 2019-08-16 1:45 UTC (permalink / raw)
To: Adrian Hunter
Cc: Song Liu, Mathieu Poirier, Daniel Borkmann, Suzuki Poulouse,
Alexander Shishkin, netdev, coresight, Alexei Starovoitov,
Arnaldo Carvalho de Melo, linux-kernel, clang-built-linux,
Peter Zijlstra, Yonghong Song, Namhyung Kim, bpf, Jiri Olsa,
Martin KaFai Lau, linux-arm-kernel
In-Reply-To: <e0919e39-7607-815b-3a12-96f098e45a5f@intel.com>
Hi Adrian,
On Thu, Aug 15, 2019 at 02:45:57PM +0300, Adrian Hunter wrote:
[...]
> >> How come you cannot use kallsyms to get the information?
> >
> > Thanks for pointing out this. Sorry I skipped your comment "I don't
> > know how you intend to calculate ARM_PRE_START_SIZE" when you reviewed
> > the patch v3, I should use that chance to elaborate the detailed idea
> > and so can get more feedback/guidance before procceed.
> >
> > Actually, I have considered to use kallsyms when worked on the previous
> > patch set.
> >
> > As mentioned in patch set v4's cover letter, I tried to implement
> > machine__create_extra_kernel_maps() for arm/arm64, the purpose is to
> > parse kallsyms so can find more kernel maps and thus also can fixup
> > the kernel start address. But I found the 'perf script' tool directly
> > calls machine__get_kernel_start() instead of running into the flow for
> > machine__create_extra_kernel_maps();
>
> Doesn't it just need to loop through each kernel map to find the lowest
> start address?
Based on your suggestion, I worked out below change and verified it
can work well on arm64 for fixing up start address; please let me know
if the change works for you?
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index f6ee7fbad3e4..51d78313dca1 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2671,9 +2671,26 @@ int machine__nr_cpus_avail(struct machine *machine)
return machine ? perf_env__nr_cpus_avail(machine->env) : 0;
}
+static int machine__fixup_kernel_start(void *arg,
+ const char *name __maybe_unused,
+ char type,
+ u64 start)
+{
+ struct machine *machine = arg;
+
+ type = toupper(type);
+
+ /* Fixup for text, weak, data and bss sections. */
+ if (type == 'T' || type == 'W' || type == 'D' || type == 'B')
+ machine->kernel_start = min(machine->kernel_start, start);
+
+ return 0;
+}
+
int machine__get_kernel_start(struct machine *machine)
{
struct map *map = machine__kernel_map(machine);
+ char filename[PATH_MAX];
int err = 0;
/*
@@ -2687,6 +2704,7 @@ int machine__get_kernel_start(struct machine *machine)
machine->kernel_start = 1ULL << 63;
if (map) {
err = map__load(map);
/*
* On x86_64, PTI entry trampolines are less than the
* start of kernel text, but still above 2^63. So leave
@@ -2695,6 +2713,16 @@ int machine__get_kernel_start(struct machine *machine)
if (!err && !machine__is(machine, "x86_64"))
machine->kernel_start = map->start;
}
+
+ machine__get_kallsyms_filename(machine, filename, PATH_MAX);
+
+ if (symbol__restricted_filename(filename, "/proc/kallsyms"))
+ goto out;
+
+ if (kallsyms__parse(filename, machine, machine__fixup_kernel_start))
+ pr_warning("Fail to fixup kernel start address. skipping...\n");
+
+out:
return err;
}
Thanks,
Leo Yan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] arm64: defconfig: Enable CONFIG_ACPI_APEI_PCIEAER
From: Wei Xu @ 2019-08-16 2:30 UTC (permalink / raw)
To: Zhou Wang, xuwei5; +Cc: linuxarm, linux-arm-kernel
In-Reply-To: <1564363944-208181-1-git-send-email-wangzhou1@hisilicon.com>
Hi Zhou,
On 2019/7/29 9:32, Zhou Wang wrote:
> HiSilicon D06 board needs this config to support PCIe AER error report.
>
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
Thanks!
Patch applied and the commit message changed as follows:
Enable the ACPI_APEI_PCIEAER configuration to support PCIe AER
error report
for the Hisilicon D06 board and the dependencies PCIEAER and
ACPI_APEI have
been enabled in the default config.
Best Regards,
Wei
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 74f0a19..8167d6f 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -119,6 +119,7 @@ CONFIG_IMX_SCU_PD=y
> CONFIG_ACPI=y
> CONFIG_ACPI_APEI=y
> CONFIG_ACPI_APEI_GHES=y
> +CONFIG_ACPI_APEI_PCIEAER=y
> CONFIG_ACPI_APEI_MEMORY_FAILURE=y
> CONFIG_ACPI_APEI_EINJ=y
> CONFIG_VIRTUALIZATION=y
_______________________________________________
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] arm64: defconfig: Enable CONFIG_ACPI_APEI_PCIEAER
From: Zhou Wang @ 2019-08-16 2:53 UTC (permalink / raw)
To: Wei Xu, xuwei5; +Cc: linuxarm, linux-arm-kernel
In-Reply-To: <5D561543.3040505@hisilicon.com>
On 2019/8/16 10:30, Wei Xu wrote:
> Hi Zhou,
>
> On 2019/7/29 9:32, Zhou Wang wrote:
>> HiSilicon D06 board needs this config to support PCIe AER error report.
>>
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>
> Thanks!
> Patch applied and the commit message changed as follows:
>
> Enable the ACPI_APEI_PCIEAER configuration to support PCIe AER error report
> for the Hisilicon D06 board and the dependencies PCIEAER and ACPI_APEI have
> been enabled in the default config.
Thanks and Best,
Zhou
>
> Best Regards,
> Wei
>
>> ---
>> arch/arm64/configs/defconfig | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
>> index 74f0a19..8167d6f 100644
>> --- a/arch/arm64/configs/defconfig
>> +++ b/arch/arm64/configs/defconfig
>> @@ -119,6 +119,7 @@ CONFIG_IMX_SCU_PD=y
>> CONFIG_ACPI=y
>> CONFIG_ACPI_APEI=y
>> CONFIG_ACPI_APEI_GHES=y
>> +CONFIG_ACPI_APEI_PCIEAER=y
>> CONFIG_ACPI_APEI_MEMORY_FAILURE=y
>> CONFIG_ACPI_APEI_EINJ=y
>> CONFIG_VIRTUALIZATION=y
>
>
>
> .
>
_______________________________________________
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 01/10] PCI: designware-ep: Add multiple PFs support for DWC
From: Xiaowei Bao @ 2019-08-16 2:55 UTC (permalink / raw)
To: Andrew Murray
Cc: mark.rutland@arm.com, Roy Zang, lorenzo.pieralisi@arm.com,
arnd@arndb.de, gregkh@linuxfoundation.org, jingoohan1@gmail.com,
Z.q. Hou, linuxppc-dev@lists.ozlabs.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
kishon@ti.com, M.h. Lian, devicetree@vger.kernel.org,
gustavo.pimentel@synopsys.com, Leo Li, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190815113129.GF43882@e119886-lin.cambridge.arm.com>
> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月15日 19:32
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org;
> M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> Roy Zang <roy.zang@nxp.com>; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for
> DWC
>
> On Thu, Aug 15, 2019 at 04:37:07PM +0800, Xiaowei Bao wrote:
> > Add multiple PFs support for DWC, different PF have different config
> > space, we use pf-offset property which get from the DTS to access the
> > different pF config space.
>
> Thanks for the patch. I haven't seen a cover letter for this series, is there one
> missing?
Maybe I miss, I will add you to review next time, thanks a lot for your comments.
>
>
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > ---
> > drivers/pci/controller/dwc/pcie-designware-ep.c | 97
> +++++++++++++---------
> > drivers/pci/controller/dwc/pcie-designware.c | 105
> ++++++++++++++++++++++--
> > drivers/pci/controller/dwc/pcie-designware.h | 10 ++-
> > include/linux/pci-epc.h | 1 +
> > 4 files changed, 164 insertions(+), 49 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 2bf5a35..75e2955 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > pci_epc_linkup(epc);
> > }
> >
> > -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno
> bar,
> > - int flags)
> > +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> > + enum pci_barno bar, int flags)
> > {
> > u32 reg;
> > + struct pci_epc *epc = pci->ep.epc;
> > + u32 pf_base = func_no * epc->pf_offset;
> >
> > - reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > + reg = pf_base + PCI_BASE_ADDRESS_0 + (4 * bar);
>
> I think I'd rather see this arithmetic (and the one for determining pf_base)
> inside a macro or inline header function. This would make this code more
> readable and reduce the chances of an error by avoiding duplication of code.
>
> For example look at cdns_pcie_ep_fn_writeb and
> ROCKCHIP_PCIE_EP_FUNC_BASE for examples of other EP drivers that do
> this.
Agree, this looks fine, thanks a lot for your comments, I will use this way to access
the registers in next version patch.
>
>
> > dw_pcie_dbi_ro_wr_en(pci);
> > dw_pcie_writel_dbi2(pci, reg, 0x0);
> > dw_pcie_writel_dbi(pci, reg, 0x0);
> > @@ -37,7 +39,12 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie
> > *pci, enum pci_barno bar,
> >
> > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar) {
> > - __dw_pcie_ep_reset_bar(pci, bar, 0);
> > + u8 func_no, funcs;
> > +
> > + funcs = pci->ep.epc->max_functions;
> > +
> > + for (func_no = 0; func_no < funcs; func_no++)
> > + __dw_pcie_ep_reset_bar(pci, func_no, bar, 0);
> > }
> >
> > static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
> > @@ -78,28 +85,29 @@ static int dw_pcie_ep_write_header(struct pci_epc
> > *epc, u8 func_no, {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + u32 pf_base = func_no * epc->pf_offset;
> >
> > dw_pcie_dbi_ro_wr_en(pci);
> > - dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, hdr->vendorid);
> > - dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, hdr->deviceid);
> > - dw_pcie_writeb_dbi(pci, PCI_REVISION_ID, hdr->revid);
> > - dw_pcie_writeb_dbi(pci, PCI_CLASS_PROG, hdr->progif_code);
> > - dw_pcie_writew_dbi(pci, PCI_CLASS_DEVICE,
> > + dw_pcie_writew_dbi(pci, pf_base + PCI_VENDOR_ID, hdr->vendorid);
> > + dw_pcie_writew_dbi(pci, pf_base + PCI_DEVICE_ID, hdr->deviceid);
> > + dw_pcie_writeb_dbi(pci, pf_base + PCI_REVISION_ID, hdr->revid);
> > + dw_pcie_writeb_dbi(pci, pf_base + PCI_CLASS_PROG, hdr->progif_code);
> > + dw_pcie_writew_dbi(pci, pf_base + PCI_CLASS_DEVICE,
> > hdr->subclass_code | hdr->baseclass_code << 8);
> > - dw_pcie_writeb_dbi(pci, PCI_CACHE_LINE_SIZE,
> > + dw_pcie_writeb_dbi(pci, pf_base + PCI_CACHE_LINE_SIZE,
> > hdr->cache_line_size);
> > - dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_VENDOR_ID,
> > + dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_VENDOR_ID,
> > hdr->subsys_vendor_id);
> > - dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_ID, hdr->subsys_id);
> > - dw_pcie_writeb_dbi(pci, PCI_INTERRUPT_PIN,
> > + dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_ID,
> hdr->subsys_id);
> > + dw_pcie_writeb_dbi(pci, pf_base + PCI_INTERRUPT_PIN,
> > hdr->interrupt_pin);
> > dw_pcie_dbi_ro_wr_dis(pci);
> >
> > return 0;
> > }
> >
> > -static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum
> pci_barno bar,
> > - dma_addr_t cpu_addr,
> > +static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> > + enum pci_barno bar, dma_addr_t cpu_addr,
> > enum dw_pcie_as_type as_type)
> > {
> > int ret;
> > @@ -112,7 +120,7 @@ static int dw_pcie_ep_inbound_atu(struct
> dw_pcie_ep *ep, enum pci_barno bar,
> > return -EINVAL;
> > }
> >
> > - ret = dw_pcie_prog_inbound_atu(pci, free_win, bar, cpu_addr,
> > + ret = dw_pcie_prog_inbound_atu(pci, func_no, free_win, bar,
> > +cpu_addr,
> > as_type);
> > if (ret < 0) {
> > dev_err(pci->dev, "Failed to program IB window\n"); @@ -125,7
> > +133,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep,
> enum pci_barno bar,
> > return 0;
> > }
> >
> > -static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, phys_addr_t
> > phys_addr,
> > +static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> > + phys_addr_t phys_addr,
> > u64 pci_addr, size_t size)
> > {
> > u32 free_win;
> > @@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct
> dw_pcie_ep *ep, phys_addr_t phys_addr,
> > return -EINVAL;
> > }
> >
> > - dw_pcie_prog_outbound_atu(pci, free_win, PCIE_ATU_TYPE_MEM,
> > - phys_addr, pci_addr, size);
> > + dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win,
> PCIE_ATU_TYPE_MEM,
> > + phys_addr, pci_addr, size);
> >
> > set_bit(free_win, ep->ob_window_map);
> > ep->outbound_addr[free_win] = phys_addr; @@ -154,7 +163,7 @@
> static
> > void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no,
> > enum pci_barno bar = epf_bar->barno;
> > u32 atu_index = ep->bar_to_atu[bar];
> >
> > - __dw_pcie_ep_reset_bar(pci, bar, epf_bar->flags);
> > + __dw_pcie_ep_reset_bar(pci, func_no, bar, epf_bar->flags);
> >
> > dw_pcie_disable_atu(pci, atu_index, DW_PCIE_REGION_INBOUND);
> > clear_bit(atu_index, ep->ib_window_map); @@ -170,14 +179,16 @@
> > static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
> > size_t size = epf_bar->size;
> > int flags = epf_bar->flags;
> > enum dw_pcie_as_type as_type;
> > - u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > + u32 pf_base = func_no * epc->pf_offset;
> > + u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar) + pf_base;
> >
> > if (!(flags & PCI_BASE_ADDRESS_SPACE))
> > as_type = DW_PCIE_AS_MEM;
> > else
> > as_type = DW_PCIE_AS_IO;
> >
> > - ret = dw_pcie_ep_inbound_atu(ep, bar, epf_bar->phys_addr, as_type);
> > + ret = dw_pcie_ep_inbound_atu(ep, func_no, bar,
> > + epf_bar->phys_addr, as_type);
> > if (ret)
> > return ret;
> >
> > @@ -235,7 +246,7 @@ static int dw_pcie_ep_map_addr(struct pci_epc
> *epc, u8 func_no,
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >
> > - ret = dw_pcie_ep_outbound_atu(ep, addr, pci_addr, size);
> > + ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
> > if (ret) {
> > dev_err(pci->dev, "Failed to enable address\n");
> > return ret;
> > @@ -248,12 +259,13 @@ static int dw_pcie_ep_get_msi(struct pci_epc
> > *epc, u8 func_no) {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + u32 pf_base = func_no * epc->pf_offset;
> > u32 val, reg;
> >
> > if (!ep->msi_cap)
> > return -EINVAL;
> >
> > - reg = ep->msi_cap + PCI_MSI_FLAGS;
> > + reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> > val = dw_pcie_readw_dbi(pci, reg);
> > if (!(val & PCI_MSI_FLAGS_ENABLE))
> > return -EINVAL;
> > @@ -267,12 +279,13 @@ static int dw_pcie_ep_set_msi(struct pci_epc
> > *epc, u8 func_no, u8 interrupts) {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + u32 pf_base = func_no * epc->pf_offset;
> > u32 val, reg;
> >
> > if (!ep->msi_cap)
> > return -EINVAL;
> >
> > - reg = ep->msi_cap + PCI_MSI_FLAGS;
> > + reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> > val = dw_pcie_readw_dbi(pci, reg);
> > val &= ~PCI_MSI_FLAGS_QMASK;
> > val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK; @@ -287,12 +300,13
> > @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no) {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + u32 pf_base = func_no * epc->pf_offset;
> > u32 val, reg;
> >
> > if (!ep->msix_cap)
> > return -EINVAL;
> >
> > - reg = ep->msix_cap + PCI_MSIX_FLAGS;
> > + reg = ep->msix_cap + pf_base + PCI_MSIX_FLAGS;
> > val = dw_pcie_readw_dbi(pci, reg);
> > if (!(val & PCI_MSIX_FLAGS_ENABLE))
> > return -EINVAL;
> > @@ -306,12 +320,13 @@ static int dw_pcie_ep_set_msix(struct pci_epc
> > *epc, u8 func_no, u16 interrupts) {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + u32 pf_base = func_no * epc->pf_offset;
> > u32 val, reg;
> >
> > if (!ep->msix_cap)
> > return -EINVAL;
> >
> > - reg = ep->msix_cap + PCI_MSIX_FLAGS;
> > + reg = ep->msix_cap + pf_base + PCI_MSIX_FLAGS;
> > val = dw_pcie_readw_dbi(pci, reg);
> > val &= ~PCI_MSIX_FLAGS_QSIZE;
> > val |= interrupts;
> > @@ -400,6 +415,7 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep
> *ep, u8 func_no,
> > unsigned int aligned_offset;
> > u16 msg_ctrl, msg_data;
> > u32 msg_addr_lower, msg_addr_upper, reg;
> > + u32 pf_base = func_no * epc->pf_offset;
> > u64 msg_addr;
> > bool has_upper;
> > int ret;
> > @@ -408,19 +424,19 @@ int dw_pcie_ep_raise_msi_irq(struct
> dw_pcie_ep *ep, u8 func_no,
> > return -EINVAL;
> >
> > /* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
> > - reg = ep->msi_cap + PCI_MSI_FLAGS;
> > + reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> > msg_ctrl = dw_pcie_readw_dbi(pci, reg);
> > has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> > - reg = ep->msi_cap + PCI_MSI_ADDRESS_LO;
> > + reg = ep->msi_cap + pf_base + PCI_MSI_ADDRESS_LO;
> > msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
> > if (has_upper) {
> > - reg = ep->msi_cap + PCI_MSI_ADDRESS_HI;
> > + reg = ep->msi_cap + pf_base + PCI_MSI_ADDRESS_HI;
> > msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
> > - reg = ep->msi_cap + PCI_MSI_DATA_64;
> > + reg = ep->msi_cap + pf_base + PCI_MSI_DATA_64;
> > msg_data = dw_pcie_readw_dbi(pci, reg);
> > } else {
> > msg_addr_upper = 0;
> > - reg = ep->msi_cap + PCI_MSI_DATA_32;
> > + reg = ep->msi_cap + pf_base + PCI_MSI_DATA_32;
> > msg_data = dw_pcie_readw_dbi(pci, reg);
> > }
> > aligned_offset = msg_addr_lower & (epc->mem->page_size - 1); @@
> > -439,7 +455,7 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep,
> > u8 func_no, }
> >
> > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > - u16 interrupt_num)
> > + u16 interrupt_num)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > struct pci_epc *epc = ep->epc;
> > @@ -447,16 +463,17 @@ int dw_pcie_ep_raise_msix_irq(struct
> dw_pcie_ep *ep, u8 func_no,
> > u32 bar_addr_upper, bar_addr_lower;
> > u32 msg_addr_upper, msg_addr_lower;
> > u32 reg, msg_data, vec_ctrl;
> > + u32 pf_base = func_no * epc->pf_offset;
> > u64 tbl_addr, msg_addr, reg_u64;
> > void __iomem *msix_tbl;
> > int ret;
> >
> > - reg = ep->msix_cap + PCI_MSIX_TABLE;
> > + reg = ep->msix_cap + pf_base + PCI_MSIX_TABLE;
> > tbl_offset = dw_pcie_readl_dbi(pci, reg);
> > bir = (tbl_offset & PCI_MSIX_TABLE_BIR);
> > tbl_offset &= PCI_MSIX_TABLE_OFFSET;
> >
> > - reg = PCI_BASE_ADDRESS_0 + (4 * bir);
> > + reg = PCI_BASE_ADDRESS_0 + pf_base + (4 * bir);
> > bar_addr_upper = 0;
> > bar_addr_lower = dw_pcie_readl_dbi(pci, reg);
> > reg_u64 = (bar_addr_lower & PCI_BASE_ADDRESS_MEM_TYPE_MASK);
> @@
> > -592,13 +609,17 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > ep->epc = epc;
> > epc_set_drvdata(epc, ep);
> >
> > - if (ep->ops->ep_init)
> > - ep->ops->ep_init(ep);
> > -
> > ret = of_property_read_u8(np, "max-functions", &epc->max_functions);
> > if (ret < 0)
> > epc->max_functions = 1;
> >
> > + ret = of_property_read_u32(np, "pf-offset", &epc->pf_offset);
> > + if (ret < 0)
> > + epc->pf_offset = 0;
>
> Bad things will likely happen if max_functions > 1 and pf-offset isn't set.
> I think the driver should bail in this situation. It would be very easy for
> someone to misconfigure this.
Yes, you are right, but if the max-functions have defined in DTS, require the pf-offset
must define in DTS, I am not sure the correct value of pf-offsetfor other platforms,
so I think the max-functions and pf-offset should not have the dependence.
even though I didn't define pf-offset when I defined max-functions, the pf-offset is 0,
the DWC ep driver can continue run the progress of INIT but not return, of course,
thus the PF1 will not work, I don't know which way is better.
>
>
> > +
> > + if (ep->ops->ep_init)
> > + ep->ops->ep_init(ep);
> > +
> > ret = __pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
> > ep->page_size);
> > if (ret < 0) {
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > b/drivers/pci/controller/dwc/pcie-designware.c
> > index 7d25102..c99cee4 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -158,6 +158,43 @@ static void dw_pcie_writel_ob_unroll(struct
> dw_pcie *pci, u32 index, u32 reg,
> > dw_pcie_writel_atu(pci, offset + reg, val); }
> >
> > +static void dw_pcie_prog_ep_outbound_atu_unroll(struct dw_pcie *pci, u8
> func_no,
> > + int index, int type,
> > + u64 cpu_addr, u64 pci_addr,
> > + u32 size)
> > +{
> > + u32 retries, val;
> > +
> > + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_BASE,
> > + lower_32_bits(cpu_addr));
> > + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_BASE,
> > + upper_32_bits(cpu_addr));
> > + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LIMIT,
> > + lower_32_bits(cpu_addr + size - 1));
> > + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_TARGET,
> > + lower_32_bits(pci_addr));
> > + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
> > + upper_32_bits(pci_addr));
> > + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> > + type | PCIE_ATU_FUNC_NUM(func_no));
>
> With the exception of this line, the rest of this function is identical to
> dw_pcie_prog_outbound_atu_unroll.
Yes, I can integrate the same code, but I think we'd better use the different outbound
window set function between RC and EP, because the RC don't need the func_num parameter.
>
> > + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
> > + PCIE_ATU_ENABLE);
> > +
> > + /*
> > + * Make sure ATU enable takes effect before any subsequent config
> > + * and I/O accesses.
> > + */
> > + for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
> > + val = dw_pcie_readl_ob_unroll(pci, index,
> > + PCIE_ATU_UNR_REGION_CTRL2);
> > + if (val & PCIE_ATU_ENABLE)
> > + return;
> > +
> > + mdelay(LINK_WAIT_IATU);
> > + }
> > + dev_err(pci->dev, "Outbound iATU is not being enabled\n"); }
> > +
> > static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int
> index,
> > int type, u64 cpu_addr,
> > u64 pci_addr, u32 size)
> > @@ -194,6 +231,51 @@ static void
> dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
> > dev_err(pci->dev, "Outbound iATU is not being enabled\n"); }
> >
> > +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int
> index,
> > + int type, u64 cpu_addr, u64 pci_addr,
> > + u32 size)
> > +{
> > + u32 retries, val;
> > +
> > + if (pci->ops->cpu_addr_fixup)
> > + cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
> > +
> > + if (pci->iatu_unroll_enabled) {
> > + dw_pcie_prog_ep_outbound_atu_unroll(pci, func_no, index, type,
> > + cpu_addr, pci_addr, size);
> > + return;
> > + }
> > +
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT,
> > + PCIE_ATU_REGION_OUTBOUND | index);
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_BASE,
> > + lower_32_bits(cpu_addr));
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_BASE,
> > + upper_32_bits(cpu_addr));
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_LIMIT,
> > + lower_32_bits(cpu_addr + size - 1));
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_TARGET,
> > + lower_32_bits(pci_addr));
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_TARGET,
> > + upper_32_bits(pci_addr));
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> > + PCIE_ATU_FUNC_NUM(func_no));
>
> The same here, this is identical to dw_pcie_prog_outbound_atu with the
> exception of this line.
>
> Is there a way you can avoid all of this duplicated code?
As above, I can integrate the same code, but I keep to think the different outbound
Window set function should be used between RC and EP.
>
> Thanks,
>
> Andrew Murray
>
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE);
> > +
> > + /*
> > + * Make sure ATU enable takes effect before any subsequent config
> > + * and I/O accesses.
> > + */
> > + for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
> > + val = dw_pcie_readl_dbi(pci, PCIE_ATU_CR2);
> > + if (val & PCIE_ATU_ENABLE)
> > + return;
> > +
> > + mdelay(LINK_WAIT_IATU);
> > + }
> > + dev_err(pci->dev, "Outbound iATU is not being enabled\n"); }
> > +
> > void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> > u64 cpu_addr, u64 pci_addr, u32 size) { @@ -252,8
> +334,8
> > @@ static void dw_pcie_writel_ib_unroll(struct dw_pcie *pci, u32 index,
> u32 reg,
> > dw_pcie_writel_atu(pci, offset + reg, val); }
> >
> > -static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, int index,
> > - int bar, u64 cpu_addr,
> > +static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, u8
> func_no,
> > + int index, int bar, u64 cpu_addr,
> > enum dw_pcie_as_type as_type) {
> > int type;
> > @@ -275,8 +357,10 @@ static int dw_pcie_prog_inbound_atu_unroll(struct
> dw_pcie *pci, int index,
> > return -EINVAL;
> > }
> >
> > - dw_pcie_writel_ib_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> type);
> > + dw_pcie_writel_ib_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> type |
> > + PCIE_ATU_FUNC_NUM(func_no));
> > dw_pcie_writel_ib_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
> > + PCIE_ATU_FUNC_NUM_MATCH_EN |
> > PCIE_ATU_ENABLE |
> > PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> >
> > @@ -297,14 +381,15 @@ static int
> dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, int index,
> > return -EBUSY;
> > }
> >
> > -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int bar,
> > - u64 cpu_addr, enum dw_pcie_as_type as_type)
> > +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > + int bar, u64 cpu_addr,
> > + enum dw_pcie_as_type as_type)
> > {
> > int type;
> > u32 retries, val;
> >
> > if (pci->iatu_unroll_enabled)
> > - return dw_pcie_prog_inbound_atu_unroll(pci, index, bar,
> > + return dw_pcie_prog_inbound_atu_unroll(pci, func_no, index, bar,
> > cpu_addr, as_type);
> >
> > dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT,
> PCIE_ATU_REGION_INBOUND |
> > @@ -323,9 +408,11 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie
> *pci, int index, int bar,
> > return -EINVAL;
> > }
> >
> > - dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type);
> > - dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE
> > - | PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> > + PCIE_ATU_FUNC_NUM(func_no));
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE |
> > + PCIE_ATU_FUNC_NUM_MATCH_EN |
> > + PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> >
> > /*
> > * Make sure ATU enable takes effect before any subsequent config
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index ffed084..2b291e8 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -71,9 +71,11 @@
> > #define PCIE_ATU_TYPE_IO 0x2
> > #define PCIE_ATU_TYPE_CFG0 0x4
> > #define PCIE_ATU_TYPE_CFG1 0x5
> > +#define PCIE_ATU_FUNC_NUM(pf) (pf << 20)
> > #define PCIE_ATU_CR2 0x908
> > #define PCIE_ATU_ENABLE BIT(31)
> > #define PCIE_ATU_BAR_MODE_ENABLE BIT(30)
> > +#define PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19)
> > #define PCIE_ATU_LOWER_BASE 0x90C
> > #define PCIE_ATU_UPPER_BASE 0x910
> > #define PCIE_ATU_LIMIT 0x914
> > @@ -265,8 +267,12 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
> > void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> > int type, u64 cpu_addr, u64 pci_addr,
> > u32 size);
> > -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int bar,
> > - u64 cpu_addr, enum dw_pcie_as_type as_type);
> > +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int
> index,
> > + int type, u64 cpu_addr, u64 pci_addr,
> > + u32 size);
> > +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > + int bar, u64 cpu_addr,
> > + enum dw_pcie_as_type as_type);
> > void dw_pcie_disable_atu(struct dw_pcie *pci, int index,
> > enum dw_pcie_region_type type);
> > void dw_pcie_setup(struct dw_pcie *pci); diff --git
> > a/include/linux/pci-epc.h b/include/linux/pci-epc.h index
> > f641bad..fc2feee 100644
> > --- a/include/linux/pci-epc.h
> > +++ b/include/linux/pci-epc.h
> > @@ -96,6 +96,7 @@ struct pci_epc {
> > const struct pci_epc_ops *ops;
> > struct pci_epc_mem *mem;
> > u8 max_functions;
> > + u32 pf_offset;
> > struct config_group *group;
> > /* spinlock to protect against concurrent access of EP controller */
> > spinlock_t lock;
> > --
> > 2.9.5
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=02%
> 7C0
> >
> 1%7Cxiaowei.bao%40nxp.com%7C0e39168f6f144db6840308d721742040%7
> C686ea1d
> >
> 3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637014654998524452&sd
> ata=bP7eh
> > cjlGXCMVFE2b4f12Q6fGV7lQ%2F5i9qIi9FoPlbI%3D&reserved=0
_______________________________________________
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 02/10] PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
From: Xiaowei Bao @ 2019-08-16 2:58 UTC (permalink / raw)
To: Andrew Murray
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
lorenzo.pieralisi@arm.com, arnd@arndb.de,
gregkh@linuxfoundation.org, jingoohan1@gmail.com, Z.q. Hou,
linuxppc-dev@lists.ozlabs.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Leo Li, M.h. Lian,
robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org,
gustavo.pimentel@synopsys.com, bhelgaas@google.com, kishon@ti.com,
shawnguo@kernel.org, Mingkai Hu
In-Reply-To: <20190815115340.GG43882@e119886-lin.cambridge.arm.com>
> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月15日 19:54
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org;
> M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> Roy Zang <roy.zang@nxp.com>; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of
> MSI-X in EP mode
>
> On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote:
> > Add the doorbell mode of MSI-X in EP mode.
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > ---
> > drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++
> > drivers/pci/controller/dwc/pcie-designware.h | 14 ++++++++++++++
> > 2 files changed, 28 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 75e2955..e3a7cdf 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep
> *ep, u8 func_no,
> > return 0;
> > }
> >
> > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
> func_no,
> > + u16 interrupt_num)
> > +{
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + u32 msg_data;
> > +
> > + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) |
> > + (interrupt_num - 1);
> > +
> > + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data);
> > +
> > + return 0;
> > +}
> > +
> > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > u16 interrupt_num)
>
> Have I understood correctly that the hardware provides an alternative
> mechanism that allows for raising MSI-X interrupts without the bother of
> reading the capabilities registers?
Yes, the hardware provide two way to MSI-X, please check the page 492 of
DWC_pcie_dm_registers_4.30 Menu.
MSIX_DOORBELL_OFF on page 492 0x948 Description: MSI-X Doorbell Register....>
>
> If so is there any good reason to keep dw_pcie_ep_raise_msix_irq? (And thus
> use it in dw_plat_pcie_ep_raise_irq also)?
I am not sure, but I think the dw_pcie_ep_raise_msix_irq function is not correct,
because I think we can't get the MSIX table from the address ep->phys_base + tbl_addr,
but I also don't know where I can get the correct MSIX table.
>
>
> > {
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index 2b291e8..cd903e9 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -88,6 +88,11 @@
> > #define PCIE_MISC_CONTROL_1_OFF 0x8BC
> > #define PCIE_DBI_RO_WR_EN BIT(0)
> >
> > +#define PCIE_MSIX_DOORBELL 0x948
> > +#define PCIE_MSIX_DOORBELL_PF_SHIFT 24
> > +#define PCIE_MSIX_DOORBELL_VF_SHIFT 16
> > +#define PCIE_MSIX_DOORBELL_VF_ACTIVE BIT(15)
>
> The _VF defines are not used, I'd suggest removing them.
In fact, I will add the SRIOV support in this file, the SRIOV feature have verified
In my board, but I need wait the EP framework SRIOV patch merge,
so I defined these two macros.
>
> Thanks,
>
> Andrew Murray
>
> > +
> > /*
> > * iATU Unroll-specific register definitions
> > * From 4.80 core version the address translation will be made by
> > unroll @@ -399,6 +404,8 @@ int dw_pcie_ep_raise_msi_irq(struct
> dw_pcie_ep *ep, u8 func_no,
> > u8 interrupt_num);
> > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > u16 interrupt_num);
> > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
> func_no,
> > + u16 interrupt_num);
> > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
> > #else static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) @@
> > -431,6 +438,13 @@ static inline int dw_pcie_ep_raise_msix_irq(struct
> dw_pcie_ep *ep, u8 func_no,
> > return 0;
> > }
> >
> > +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep
> *ep,
> > + u8 func_no,
> > + u16 interrupt_num)
> > +{
> > + return 0;
> > +}
> > +
> > static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > pci_barno bar) { }
> > --
> > 2.9.5
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=02%
> 7C0
> >
> 1%7Cxiaowei.bao%40nxp.com%7C8489493003bb48a0139d08d721773972%
> 7C686ea1d
> >
> 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637014668369499298&sd
> ata=dyrXB
> >
> avljJBFUSNXW7K%2FRoXvwfWTE%2FoU2KMd1bZkJow%3D&reserved=0
_______________________________________________
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 05/10] PCI: layerscape: Modify the way of getting capability with different PEX
From: Xiaowei Bao @ 2019-08-16 3:00 UTC (permalink / raw)
To: Andrew Murray
Cc: mark.rutland@arm.com, Roy Zang, lorenzo.pieralisi@arm.com,
arnd@arndb.de, gregkh@linuxfoundation.org, jingoohan1@gmail.com,
Z.q. Hou, linuxppc-dev@lists.ozlabs.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Leo Li,
M.h. Lian, devicetree@vger.kernel.org, robh+dt@kernel.org,
linux-arm-kernel@lists.infradead.org,
gustavo.pimentel@synopsys.com, bhelgaas@google.com, kishon@ti.com,
shawnguo@kernel.org, Mingkai Hu
In-Reply-To: <20190815125103.GH43882@e119886-lin.cambridge.arm.com>
> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月15日 20:51
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org;
> M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> Roy Zang <roy.zang@nxp.com>; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 05/10] PCI: layerscape: Modify the way of getting
> capability with different PEX
>
> On Thu, Aug 15, 2019 at 04:37:11PM +0800, Xiaowei Bao wrote:
> > The different PCIe controller in one board may be have different
> > capability of MSI or MSIX, so change the way of getting the MSI
> > capability, make it more flexible.
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > ---
> > drivers/pci/controller/dwc/pci-layerscape-ep.c | 28
> > +++++++++++++++++++-------
> > 1 file changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > index be61d96..9404ca0 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -22,6 +22,7 @@
> >
> > struct ls_pcie_ep {
> > struct dw_pcie *pci;
> > + struct pci_epc_features *ls_epc;
> > };
> >
> > #define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > @@ -40,25 +41,26 @@ static const struct of_device_id
> ls_pcie_ep_of_match[] = {
> > { },
> > };
> >
> > -static const struct pci_epc_features ls_pcie_epc_features = {
> > - .linkup_notifier = false,
> > - .msi_capable = true,
> > - .msix_capable = false,
> > -};
> > -
> > static const struct pci_epc_features* ls_pcie_ep_get_features(struct
> > dw_pcie_ep *ep) {
> > - return &ls_pcie_epc_features;
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > +
> > + return pcie->ls_epc;
> > }
> >
> > static void ls_pcie_ep_init(struct dw_pcie_ep *ep) {
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > enum pci_barno bar;
> >
> > for (bar = BAR_0; bar <= BAR_5; bar++)
> > dw_pcie_ep_reset_bar(pci, bar);
> > +
> > + pcie->ls_epc->msi_capable = ep->msi_cap ? true : false;
> > + pcie->ls_epc->msix_capable = ep->msix_cap ? true : false;
> > }
> >
> > static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no, @@
> > -118,6 +120,7 @@ static int __init ls_pcie_ep_probe(struct platform_device
> *pdev)
> > struct device *dev = &pdev->dev;
> > struct dw_pcie *pci;
> > struct ls_pcie_ep *pcie;
> > + struct pci_epc_features *ls_epc;
> > struct resource *dbi_base;
> > int ret;
> >
> > @@ -129,6 +132,10 @@ static int __init ls_pcie_ep_probe(struct
> platform_device *pdev)
> > if (!pci)
> > return -ENOMEM;
> >
> > + ls_epc = devm_kzalloc(dev, sizeof(*ls_epc), GFP_KERNEL);
> > + if (!ls_epc)
> > + return -ENOMEM;
> > +
> > dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "regs");
> > pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> > if (IS_ERR(pci->dbi_base))
> > @@ -139,6 +146,13 @@ static int __init ls_pcie_ep_probe(struct
> platform_device *pdev)
> > pci->ops = &ls_pcie_ep_ops;
> > pcie->pci = pci;
> >
> > + ls_epc->linkup_notifier = false,
> > + ls_epc->msi_capable = true,
> > + ls_epc->msix_capable = true,
>
> As [msi,msix]_capable is shortly set from ls_pcie_ep_init - is there any reason
> to set them here (to potentially incorrect values)?
This is a INIT value, maybe false is better for msi_capable and msix_capable,
of course, we don't need to set it.
>
> Thanks,
>
> Andrew Murray
>
> > + ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> > +
> > + pcie->ls_epc = ls_epc;
> > +
> > platform_set_drvdata(pdev, pcie);
> >
> > ret = ls_add_pcie_ep(pcie, pdev);
> > --
> > 2.9.5
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [GIT PULL] Hisilicon fixes for v5.3
From: Wei Xu @ 2019-08-16 3:29 UTC (permalink / raw)
To: soc, arm@kernel.org, linux-arm-kernel@lists.infradead.org,
Olof Johansson, Arnd Bergmann
Cc: sashal, Salil Mehta, jinying, Tangkunshan, linux-pci, John Garry,
Rafael J. Wysocki, Linuxarm, Shameerali Kolothum Thodi,
Linux Kernel Mailing List, huangdaode, xuwei (O),
Jonathan Cameron, Bjorn Helgaas, stable, Liguozhu (Kenneth),
Zhangyi ac, Shiju Jose
Hi ARM-SoC team,
Please consider to pull the following fixes.
Thanks!
Best Regards,
Wei
---
The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
are available in the Git repository at:
git://github.com/hisilicon/linux-hisi.git tags/hisi-fixes-for-5.3
for you to fetch changes up to 10e62b47973b0b0ceda076255bcb147b83e20517:
bus: hisi_lpc: Add .remove method to avoid driver unbind crash
(2019-08-13 14:54:34 +0800)
----------------------------------------------------------------
Hisilicon fixes for v5.3-rc
- Fixed RCU usage in logical PIO
- Added a function to unregister a logical PIO range in logical PIO
to support the fixes in the hisi-lpc driver
- Fixed and optimized hisi-lpc driver to avoid potential use-after-free
and driver unbind crash
----------------------------------------------------------------
John Garry (5):
lib: logic_pio: Fix RCU usage
lib: logic_pio: Avoid possible overlap for unregistering regions
lib: logic_pio: Add logic_pio_unregister_range()
bus: hisi_lpc: Unregister logical PIO range to avoid potential
use-after-free
bus: hisi_lpc: Add .remove method to avoid driver unbind crash
drivers/bus/hisi_lpc.c | 47 ++++++++++++++++++++++++++----
include/linux/logic_pio.h | 1 +
lib/logic_pio.c | 73
+++++++++++++++++++++++++++++++++++------------
3 files changed, 96 insertions(+), 25 deletions(-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [GIT PULL] arm64: defconfig: hisilicon config updates for v5.4
From: Wei Xu @ 2019-08-16 3:39 UTC (permalink / raw)
To: soc, arm@kernel.org, linux-arm-kernel@lists.infradead.org,
Olof Johansson, Arnd Bergmann
Cc: Salil Mehta, jinying, Tangkunshan, John Garry, Linuxarm,
Shameerali Kolothum Thodi, Wangzhou (B), huangdaode, xuwei (O),
Jonathan Cameron, Liguozhu (Kenneth), Zhangyi ac, Shiju Jose
Hi ARM-SoC team,
Please consider to pull the following changes.
Thanks!
Best Regards,
Wei
---
The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
are available in the Git repository at:
git://github.com/hisilicon/linux-hisi.git
tags/hisi-arm64-defconfig-for-5.4
for you to fetch changes up to 0c26a3345b4f402b87d9be00df4d3054cd8ba46f:
arm64: defconfig: Enable CONFIG_ACPI_APEI_PCIEAER (2019-08-16
09:36:07 +0800)
----------------------------------------------------------------
ARM64: hisilicon: defconfig updates for v5.4
- Enable ACPI_APEI_PCIEAER for the hisilicon D06 board to
support PCIe AER error report
----------------------------------------------------------------
Zhou Wang (1):
arm64: defconfig: Enable CONFIG_ACPI_APEI_PCIEAER
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCHv3 2/3] arm64: dts: ls1028a: Add PCIe controller DT nodes
From: Z.q. Hou @ 2019-08-16 4:17 UTC (permalink / raw)
To: Xiaowei Bao, bhelgaas@google.com, robh+dt@kernel.org,
mark.rutland@arm.com, shawnguo@kernel.org, Leo Li, kishon@ti.com,
lorenzo.pieralisi@arm.com, arnd@arndb.de,
gregkh@linuxfoundation.org, M.h. Lian, Mingkai Hu, Roy Zang,
kstewart@linuxfoundation.org, pombredanne@nexb.com,
shawn.lin@rock-chips.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linuxppc-dev@lists.ozlabs.org
Cc: Xiaowei Bao
In-Reply-To: <20190806061553.19934-2-xiaowei.bao@nxp.com>
Hi Xiaowei,
> -----Original Message-----
> From: Xiaowei Bao <xiaowei.bao@nxp.com>
> Sent: 2019年8月6日 14:16
> To: bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org;
> M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> Z.q. Hou <zhiqiang.hou@nxp.com>; Roy Zang <roy.zang@nxp.com>;
> kstewart@linuxfoundation.org; pombredanne@nexb.com;
> shawn.lin@rock-chips.com; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Cc: Xiaowei Bao <xiaowei.bao@nxp.com>; Z.q. Hou
> <zhiqiang.hou@nxp.com>
> Subject: [PATCHv3 2/3] arm64: dts: ls1028a: Add PCIe controller DT nodes
>
> LS1028a implements 2 PCIe 3.0 controllers.
>
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
> v2:
> - Fix up the legacy INTx allocate failed issue.
> v3:
> - no change.
>
> arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 52
> ++++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> index aef5b06..0b542ed 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> @@ -503,6 +503,58 @@
> status = "disabled";
> };
>
> + pcie@3400000 {
> + compatible = "fsl,ls1028a-pcie";
> + reg = <0x00 0x03400000 0x0 0x00100000 /* controller
> registers */
> + 0x80 0x00000000 0x0 0x00002000>; /* configuration
> space */
> + reg-names = "regs", "config";
> + interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>, /* PME
> interrupt */
> + <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>; /* aer
> interrupt */
> + interrupt-names = "pme", "aer";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + dma-coherent;
> + num-lanes = <4>;
Remove the num-lanes, it is not needed by Layerscape PCIe controllers. see: http://patchwork.ozlabs.org/project/linux-pci/list/?series=124488
> + bus-range = <0x0 0xff>;
> + ranges = <0x81000000 0x0 0x00000000 0x80 0x00010000 0x0
> 0x00010000 /* downstream I/O */
> + 0x82000000 0x0 0x40000000 0x80 0x40000000 0x0
> 0x40000000>; /* non-prefetchable memory */
> + msi-parent = <&its>;
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0000 0 0 1 &gic 0 0 GIC_SPI 109
> IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 2 &gic 0 0 GIC_SPI 110
> IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 3 &gic 0 0 GIC_SPI 111
> IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 4 &gic 0 0 GIC_SPI 112
> IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + };
> +
> + pcie@3500000 {
> + compatible = "fsl,ls1028a-pcie";
> + reg = <0x00 0x03500000 0x0 0x00100000 /* controller
> registers */
> + 0x88 0x00000000 0x0 0x00002000>; /* configuration
> space */
> + reg-names = "regs", "config";
> + interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "pme", "aer";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + dma-coherent;
> + num-lanes = <4>;
Ditto
> + bus-range = <0x0 0xff>;
> + ranges = <0x81000000 0x0 0x00000000 0x88 0x00010000 0x0
> 0x00010000 /* downstream I/O */
> + 0x82000000 0x0 0x40000000 0x88 0x40000000 0x0
> 0x40000000>; /* non-prefetchable memory */
> + msi-parent = <&its>;
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0000 0 0 1 &gic 0 0 GIC_SPI 114
> IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 2 &gic 0 0 GIC_SPI 115
> IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 3 &gic 0 0 GIC_SPI 116
> IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 4 &gic 0 0 GIC_SPI 117
> IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + };
> +
> pcie@1f0000000 { /* Integrated Endpoint Root Complex */
> compatible = "pci-host-ecam-generic";
> reg = <0x01 0xf0000000 0x0 0x100000>;
> --
> 2.9.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ASoC: imx-audmux: Add driver suspend and resume to support MEGA Fast
From: Shengjiu Wang @ 2019-08-16 5:03 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, lgirdwood,
perex, tiwai, shawnguo, s.hauer, kernel, alsa-devel
Cc: linuxppc-dev, linux-imx, linux-arm-kernel, linux-kernel
For i.MX6 SoloX, there is a mode of the SoC to shutdown all power
source of modules during system suspend and resume procedure.
Thus, AUDMUX needs to save all the values of registers before the
system suspend and restore them after the system resume.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/imx-audmux.c | 54 +++++++++++++++++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/imx-audmux.c b/sound/soc/fsl/imx-audmux.c
index 7595f24a006e..3ce85a43e08f 100644
--- a/sound/soc/fsl/imx-audmux.c
+++ b/sound/soc/fsl/imx-audmux.c
@@ -23,6 +23,8 @@
static struct clk *audmux_clk;
static void __iomem *audmux_base;
+static u32 *regcache;
+static u32 reg_max;
#define IMX_AUDMUX_V2_PTCR(x) ((x) * 8)
#define IMX_AUDMUX_V2_PDCR(x) ((x) * 8 + 4)
@@ -315,8 +317,23 @@ static int imx_audmux_probe(struct platform_device *pdev)
if (of_id)
pdev->id_entry = of_id->data;
audmux_type = pdev->id_entry->driver_data;
- if (audmux_type == IMX31_AUDMUX)
+
+ switch (audmux_type) {
+ case IMX31_AUDMUX:
audmux_debugfs_init();
+ reg_max = 14;
+ break;
+ case IMX21_AUDMUX:
+ reg_max = 6;
+ break;
+ default:
+ dev_err(&pdev->dev, "unsupported version!\n");
+ return -EINVAL;
+ }
+
+ regcache = devm_kzalloc(&pdev->dev, sizeof(u32) * reg_max, GFP_KERNEL);
+ if (!regcache)
+ return -ENOMEM;
if (of_id)
imx_audmux_parse_dt_defaults(pdev, pdev->dev.of_node);
@@ -332,12 +349,47 @@ static int imx_audmux_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
+static int imx_audmux_suspend(struct device *dev)
+{
+ int i;
+
+ clk_prepare_enable(audmux_clk);
+
+ for (i = 0; i < reg_max; i++)
+ regcache[i] = readl(audmux_base + i * 4);
+
+ clk_disable_unprepare(audmux_clk);
+
+ return 0;
+}
+
+static int imx_audmux_resume(struct device *dev)
+{
+ int i;
+
+ clk_prepare_enable(audmux_clk);
+
+ for (i = 0; i < reg_max; i++)
+ writel(regcache[i], audmux_base + i * 4);
+
+ clk_disable_unprepare(audmux_clk);
+
+ return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops imx_audmux_pm = {
+ SET_SYSTEM_SLEEP_PM_OPS(imx_audmux_suspend, imx_audmux_resume)
+};
+
static struct platform_driver imx_audmux_driver = {
.probe = imx_audmux_probe,
.remove = imx_audmux_remove,
.id_table = imx_audmux_ids,
.driver = {
.name = DRIVER_NAME,
+ .pm = &imx_audmux_pm,
.of_match_table = imx_audmux_dt_ids,
}
};
--
2.21.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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