* [RFC 50/50] staging: vchiq: Move vchiq.h into include directory
From: Nicolas Saenz Julienne @ 2020-05-27 11:53 UTC (permalink / raw)
To: bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
Nicolas Saenz Julienne, Florian Fainelli, Ray Jui, Scott Branden
Cc: devel, kernel-list, laurent.pinchart, linux-kernel, gregkh
In-Reply-To: <20200527115400.31391-1-nsaenzjulienne@suse.de>
To make the separation clear between vchiq's header files and vchiq.h,
which is to be used by services and is the 'public' API, move it into a
dedicated includes directory.
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
drivers/staging/vc04_services/Makefile | 2 +-
drivers/staging/vc04_services/bcm2835-audio/Makefile | 2 +-
drivers/staging/vc04_services/bcm2835-audio/bcm2835.h | 2 +-
.../{interface/vchiq_arm => include/linux/raspberrypi}/vchiq.h | 0
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h | 2 +-
.../staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h | 2 +-
drivers/staging/vc04_services/vc-sm-cma/Makefile | 1 -
drivers/staging/vc04_services/vc-sm-cma/vc_sm.c | 2 +-
drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.c | 2 +-
drivers/staging/vc04_services/vchiq-mmal/Makefile | 1 +
drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 3 +--
11 files changed, 9 insertions(+), 10 deletions(-)
rename drivers/staging/vc04_services/{interface/vchiq_arm => include/linux/raspberrypi}/vchiq.h (100%)
diff --git a/drivers/staging/vc04_services/Makefile b/drivers/staging/vc04_services/Makefile
index e32c0744e7fc..e1de39303ffe 100644
--- a/drivers/staging/vc04_services/Makefile
+++ b/drivers/staging/vc04_services/Makefile
@@ -14,5 +14,5 @@ obj-$(CONFIG_VIDEO_ISP_BCM2835) += bcm2835-isp/
obj-$(CONFIG_BCM_VC_SM_CMA) += vc-sm-cma/
obj-$(CONFIG_BCM2835_VCHIQ_MMAL) += vchiq-mmal/
-ccflags-y += -D__VCCOREVER__=0x04000000
+ccflags-y += -I $(srctree)/$(src)/include -D__VCCOREVER__=0x04000000
diff --git a/drivers/staging/vc04_services/bcm2835-audio/Makefile b/drivers/staging/vc04_services/bcm2835-audio/Makefile
index 13fa6d7d9745..d59fe4dde615 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/Makefile
+++ b/drivers/staging/vc04_services/bcm2835-audio/Makefile
@@ -2,4 +2,4 @@
obj-$(CONFIG_SND_BCM2835) += snd-bcm2835.o
snd-bcm2835-objs := bcm2835.o bcm2835-ctl.o bcm2835-pcm.o bcm2835-vchiq.o
-ccflags-y += -I $(srctree)/$(src)/.. -D__VCCOREVER__=0x04000000
+ccflags-y += -I $(srctree)/$(src)/../include -D__VCCOREVER__=0x04000000
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
index ca220f5230ec..1b36475872d6 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
@@ -6,10 +6,10 @@
#include <linux/device.h>
#include <linux/wait.h>
+#include <linux/raspberrypi/vchiq.h>
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm-indirect.h>
-#include "interface/vchiq_arm/vchiq.h"
#define MAX_SUBSTREAMS (8)
#define AVAIL_SUBSTREAMS_MASK (0xff)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq.h b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
similarity index 100%
rename from drivers/staging/vc04_services/interface/vchiq_arm/vchiq.h
rename to drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index 8a27f3d7217e..e67692879249 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -10,10 +10,10 @@
#include <linux/kref.h>
#include <linux/rcupdate.h>
#include <linux/wait.h>
+#include <linux/raspberrypi/vchiq.h>
#include "vchiq_cfg.h"
-#include "vchiq.h"
/* Do this so that we can test-build the code on non-rpi systems */
#if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h
index f285d754ad28..3653fd99d8a1 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h
@@ -5,7 +5,7 @@
#define VCHIQ_IOCTLS_H
#include <linux/ioctl.h>
-#include "vchiq.h"
+#include <linux/raspberrypi/vchiq.h>
#define VCHIQ_IOC_MAGIC 0xc4
#define VCHIQ_INVALID_HANDLE (~0)
diff --git a/drivers/staging/vc04_services/vc-sm-cma/Makefile b/drivers/staging/vc04_services/vc-sm-cma/Makefile
index 77d173694fbf..c92a5775c62e 100644
--- a/drivers/staging/vc04_services/vc-sm-cma/Makefile
+++ b/drivers/staging/vc04_services/vc-sm-cma/Makefile
@@ -1,6 +1,5 @@
ccflags-y += \
-I$(srctree)/$(src)/../ \
- -I$(srctree)/$(src)/../interface/vchi \
-I$(srctree)/$(src)/../interface/vchiq_arm\
-I$(srctree)/$(src)/../include
diff --git a/drivers/staging/vc04_services/vc-sm-cma/vc_sm.c b/drivers/staging/vc04_services/vc-sm-cma/vc_sm.c
index e4f7bdeef66d..cc69ce932317 100644
--- a/drivers/staging/vc04_services/vc-sm-cma/vc_sm.c
+++ b/drivers/staging/vc04_services/vc-sm-cma/vc_sm.c
@@ -46,9 +46,9 @@
#include <linux/seq_file.h>
#include <linux/syscalls.h>
#include <linux/types.h>
+#include <linux/raspberrypi/vchiq.h>
#include <asm/cacheflush.h>
-#include "vchiq.h"
#include "vchiq_connected.h"
#include "vc_sm_cma_vchi.h"
diff --git a/drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.c b/drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.c
index 2c65416cd331..8d8eda1a8142 100644
--- a/drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.c
+++ b/drivers/staging/vc04_services/vc-sm-cma/vc_sm_cma_vchi.c
@@ -18,8 +18,8 @@
#include <linux/semaphore.h>
#include <linux/slab.h>
#include <linux/types.h>
+#include <linux/raspberrypi/vchiq.h>
-#include "vchiq.h"
#include "vc_sm_cma_vchi.h"
#define VC_SM_VER 1
diff --git a/drivers/staging/vc04_services/vchiq-mmal/Makefile b/drivers/staging/vc04_services/vchiq-mmal/Makefile
index f8164c33aec3..b2a830f48acc 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/Makefile
+++ b/drivers/staging/vc04_services/vchiq-mmal/Makefile
@@ -5,4 +5,5 @@ obj-$(CONFIG_BCM2835_VCHIQ_MMAL) += bcm2835-mmal-vchiq.o
ccflags-y += \
-I$(srctree)/$(src)/.. \
+ -I$(srctree)/$(src)/../include \
-D__VCCOREVER__=0x04000000
diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
index 2101b79780eb..e057e21961d5 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
@@ -23,6 +23,7 @@
#include <linux/slab.h>
#include <linux/completion.h>
#include <linux/vmalloc.h>
+#include <linux/raspberrypi/vchiq.h>
#include <media/videobuf2-vmalloc.h>
#include "mmal-common.h"
@@ -32,8 +33,6 @@
#include "vc-sm-cma/vc_sm_knl.h"
-#include "interface/vchiq_arm/vchiq.h"
-
/*
* maximum number of components supported.
* This matches the maximum permitted by default on the VPU
--
2.26.2
_______________________________________________
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 v2] ARM: mm: Simplify act_mm macro
From: Linus Walleij @ 2020-05-27 11:56 UTC (permalink / raw)
To: Russell King; +Cc: Linus Walleij, linux-arm-kernel
The act_mm assembly macro is actually partly reimplementing
get_thread_info so let's just use that.
Suggested-by: Russell King <linux@armlinux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Accidentally removed the index to the active task which
is what we want here.
---
arch/arm/mm/proc-macros.S | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
index 60ac7c5999a9..e2c743aa2eb2 100644
--- a/arch/arm/mm/proc-macros.S
+++ b/arch/arm/mm/proc-macros.S
@@ -5,7 +5,6 @@
* VMA_VM_FLAGS
* VM_EXEC
*/
-#include <linux/const.h>
#include <asm/asm-offsets.h>
#include <asm/thread_info.h>
@@ -31,8 +30,7 @@
* act_mm - get current->active_mm
*/
.macro act_mm, rd
- bic \rd, sp, #(THREAD_SIZE - 1) & ~63
- bic \rd, \rd, #63
+ get_thread_info \rd
ldr \rd, [\rd, #TI_TASK]
.if (TSK_ACTIVE_MM > IMM12_MASK)
add \rd, \rd, #TSK_ACTIVE_MM & ~IMM12_MASK
--
2.25.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 v13 2/2] media: v4l: xilinx: Add Xilinx MIPI CSI-2 Rx Subsystem driver
From: Vishal Sagar @ 2020-05-27 12:03 UTC (permalink / raw)
To: Laurent Pinchart
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, Jacopo Mondi,
Dinesh Kumar, Hyun Kwon, Sandip Kothari,
linux-kernel@vger.kernel.org, robh+dt@kernel.org, Michal Simek,
Luca Ceresoli, hans.verkuil@cisco.com, mchehab@kernel.org,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <20200524022722.GC6026@pendragon.ideasonboard.com>
Hi Laurent,
Thanks for reviewing this patch.
> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Sunday, May 24, 2020 7:57 AM
> To: Vishal Sagar <vsagar@xilinx.com>
> Cc: Hyun Kwon <hyunk@xilinx.com>; mchehab@kernel.org;
> robh+dt@kernel.org; mark.rutland@arm.com; Michal Simek
> <michals@xilinx.com>; linux-media@vger.kernel.org;
> devicetree@vger.kernel.org; hans.verkuil@cisco.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Dinesh Kumar
> <dineshk@xilinx.com>; Sandip Kothari <sandipk@xilinx.com>; Luca Ceresoli
> <luca@lucaceresoli.net>; Jacopo Mondi <jacopo@jmondi.org>; Hyun Kwon
> <hyunk@xilinx.com>
> Subject: Re: [PATCH v13 2/2] media: v4l: xilinx: Add Xilinx MIPI CSI-2 Rx
> Subsystem driver
>
> Hi Vishal,
>
> Thank you for the patch.
>
> On Tue, May 12, 2020 at 08:49:47PM +0530, Vishal Sagar wrote:
> > The Xilinx MIPI CSI-2 Rx Subsystem soft IP is used to capture images
> > from MIPI CSI-2 camera sensors and output AXI4-Stream video data ready
> > for image processing. Please refer to PG232 for details.
> >
> > The CSI2 Rx controller filters out all packets except for the packets
> > with data type fixed in hardware. RAW8 packets are always allowed to
> > pass through.
> >
> > It is also used to setup and handle interrupts and enable the core. It
> > logs all the events in respective counters between streaming on and off.
> >
> > The driver supports only the video format bridge enabled configuration.
> > Some data types like YUV 422 10bpc, RAW16, RAW20 are supported when
> > the CSI v2.0 feature is enabled in design. When the VCX feature is
> > enabled, the maximum number of virtual channels becomes 16 from 4.
> >
> > Signed-off-by: Vishal Sagar <vishal.sagar@xilinx.com>
> > Reviewed-by: Hyun Kwon <hyun.kwon@xilinx.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > v13
> > - Based on Laurent's suggestions
> > - Removed unnecessary debug statement for vep
> > - Added TODO for clock to enable disable at streamon/off
> > - Fix for index to start from 0 for get_nth_mbus_format
> > - Removed macro XCSI_TIMEOUT_VAL
> > - Remove ndelay from hard reset
> > - Remove hard reset from irq handler
> > - Fix short packet fifo clear
> > - Add TODO for v4l2_subdev_notify for SLBF error
> > - Fix the enable condition in s_stream
> > - Fix condition in xcsi2rxss_set_format
> > - Fix enum_mbus_code for double enumeration of RAW8 Data type
> > - Removed core struct
> > - Added reviewed by Laurent
> >
> > v12
> > - Changes done as suggested by Laurent Pinchart and Luca Ceresoli
> > - Removed unused macros
> > - No local storage of supported formats
> > - Dropped init mbus fmts and removed xcsi2rxss_init_mbus_fmts()
> > - XCSI_GET_BITSET_STR removed
> > - Add data type and mbus LUT
> > - Added xcsi2rxss_get_nth_mbus() and xcsi2rxss_get_dt()
> > - Replaced all core->dev with dev in dev_dbg() and related debug
> > prints
> > - Replaced xcsi2rxss_log_ipconfig() with single line
> > - Removed small functions to enable/disable interrupts and core
> > - Now save remote subdev in state struct before streaming on
> > - Made xcsi2rxss_reset as soft_reset()
> > - Added hard reset using video-reset gpio
> > - 2 modes one with delay and another sleep
> > - Instead of reset-gpios it is not video-reset-gpios
> > - In irq handler
> > - Moved clearing of ISR up
> > - Dump / empty short packet fifo
> > - Irq handler is now threaded
> > - Added init_cfg pad ops and removed open()
> > - Updated xcsi2rxss_set_format(), xcsi2rxss_enum_mbus_code() to use
> > the dt mbus lut
> > - xcsi2rxss_set_default_format() updated
> > - Moved mutex_init()
> > - Updated graph handling
> > - Removed unnecessary prints
> > - Use devm_platform_ioremap_resource() and platform_get_irq()
> > - Update KConfig description
> >
> > v11
> > - Fixed changes as suggested by Sakari Ailus
> > - Removed VIDEO_XILINX from KConfig
> > - Minor formatting
> > - Start / Stop upstream sub-device in xcsi2rxss_start_stream()
> > and xcsi2rxss_stop_stream()
> > - Added v4l2_subdev_link_validate_default() in v4l2_subdev_pad_ops()
> > - Use fwnode_graph_get_endpoint_by_id() instead of parsing by self
> > - Set bus type as V4L2_MBUS_CSI2_DPHY in struct v4l2_fwnode_endpoint
> > - Remove num_clks from core. Instead use ARRAY_SIZE()
> > - Fixed SPDX header to GPL-2.0
> > - Update copyright year to 2020
> >
> > v10
> > - Removed all V4L2 controls and events based on Sakari's comments.
> > - Now stop_stream() before toggling rst_gpio
> > - Updated init_mbus() to throw error on array out of bound access
> > - Make events and vcx_events as counters instead of structures
> > - Minor fixes in set_format() enum_mbus_code() as suggested by Sakari
> >
> > v9
> > - Moved all controls and events to xilinx-csi2rxss.h
> > - Updated name and description of controls and events
> > - Get control base address from v4l2-controls.h (0x10c0)
> > - Fix KConfig for dependency on VIDEO_XILINX
> > - Added enum_mbus_code() support
> > - Added default format to be returned on open()
> > - Mark variables are const
> > - Remove references to short packet in comments
> > - Add check for streaming before setting active lanes control
> > - strlcpy -> strscpy
> > - Fix xcsi2rxss_set_format()
> >
> > v8
> > - Use clk_bulk* APIs
> > - Add gpio reset for asserting video_aresetn when stream line buffer
> > occurs
> > - Removed short packet related events and irq handling
> > - V4L2_EVENT_XLNXCSIRX_SPKT and V4L2_EVENT_XLNXCSIRX_SPKT_OVF
> > removed
> > - Removed frame counter control
> V4L2_CID_XILINX_MIPICSISS_FRAME_COUNTER
> > and xcsi2rxss_g_volatile_ctrl()
> > - Minor formatting fixes
> >
> > v7
> > - No change
> >
> > v6
> > - No change
> >
> > v5
> > - Removed bayer and updated related parts like set default format based
> > on Luca Cersoli's comments.
> > - Added correct YUV422 10bpc media bus format
> >
> > v4
> > - Removed irq member from core structure
> > - Consolidated IP config prints in xcsi2rxss_log_ipconfig()
> > - Return -EINVAL in case of invalid ioctl
> > - Code formatting
> > - Added reviewed by Hyun Kwon
> >
> > v3
> > - Fixed comments given by Hyun.
> > - Removed DPHY 200 MHz clock. This will be controlled by DPHY driver
> > - Minor code formatting
> > - en_csi_v20 and vfb members removed from struct and made local to dt
> > parsing
> > - lock description updated
> > - changed to ratelimited type for all dev prints in irq handler
> > - Removed YUV 422 10bpc media format
> >
> > v2
> > - Fixed comments given by Hyun and Sakari.
> > - Made all bitmask using BIT() and GENMASK()
> > - Removed unused definitions
> > - Removed DPHY access. This will be done by separate DPHY PHY driver.
> > - Added support for CSI v2.0 for YUV 422 10bpc, RAW16, RAW20 and extra
> > virtual channels
> > - Fixed the ports as sink and source
> > - Now use the v4l2fwnode API to get number of data-lanes
> > - Added clock framework support
> > - Removed the close() function
> > - updated the set format function
> > - support only VFB enabled configuration
> >
> > drivers/media/platform/xilinx/Kconfig | 7 +
> > drivers/media/platform/xilinx/Makefile | 1 +
> > .../media/platform/xilinx/xilinx-csi2rxss.c | 1114 +++++++++++++++++
> > 3 files changed, 1122 insertions(+)
> > create mode 100644 drivers/media/platform/xilinx/xilinx-csi2rxss.c
> >
> > diff --git a/drivers/media/platform/xilinx/Kconfig
> > b/drivers/media/platform/xilinx/Kconfig
> > index 01c96fb66414..44587dccacf1 100644
> > --- a/drivers/media/platform/xilinx/Kconfig
> > +++ b/drivers/media/platform/xilinx/Kconfig
> > @@ -12,6 +12,13 @@ config VIDEO_XILINX
> >
> > if VIDEO_XILINX
> >
> > +config VIDEO_XILINX_CSI2RXSS
> > + tristate "Xilinx CSI-2 Rx Subsystem"
> > + help
> > + Driver for Xilinx MIPI CSI-2 Rx Subsystem. This is a V4L sub-device
> > + based driver that takes input from CSI-2 Tx source and converts
> > + it into an AXI4-Stream.
> > +
> > config VIDEO_XILINX_TPG
> > tristate "Xilinx Video Test Pattern Generator"
> > depends on VIDEO_XILINX
> > diff --git a/drivers/media/platform/xilinx/Makefile
> > b/drivers/media/platform/xilinx/Makefile
> > index 4cdc0b1ec7a5..6119a34f3043 100644
> > --- a/drivers/media/platform/xilinx/Makefile
> > +++ b/drivers/media/platform/xilinx/Makefile
> > @@ -3,5 +3,6 @@
> > xilinx-video-objs += xilinx-dma.o xilinx-vip.o xilinx-vipp.o
> >
> > obj-$(CONFIG_VIDEO_XILINX) += xilinx-video.o
> > +obj-$(CONFIG_VIDEO_XILINX_CSI2RXSS) += xilinx-csi2rxss.o
> > obj-$(CONFIG_VIDEO_XILINX_TPG) += xilinx-tpg.o
> > obj-$(CONFIG_VIDEO_XILINX_VTC) += xilinx-vtc.o diff --git
> > a/drivers/media/platform/xilinx/xilinx-csi2rxss.c
> > b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
> > new file mode 100644
> > index 000000000000..cac08149ae7a
> > --- /dev/null
> > +++ b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
> > @@ -0,0 +1,1114 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Xilinx MIPI CSI-2 Rx Subsystem
> > + *
> > + * Copyright (C) 2016 - 2020 Xilinx, Inc.
> > + *
> > + * Contacts: Vishal Sagar <vishal.sagar@xilinx.com>
> > + *
> > + */
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/of.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/v4l2-subdev.h>
> > +#include <media/media-entity.h>
> > +#include <media/v4l2-common.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-subdev.h>
> > +#include "xilinx-vip.h"
> > +
> > +/* Register register map */
> > +#define XCSI_CCR_OFFSET 0x00
> > +#define XCSI_CCR_SOFTRESET BIT(1)
> > +#define XCSI_CCR_ENABLE BIT(0)
> > +
> > +#define XCSI_PCR_OFFSET 0x04
> > +#define XCSI_PCR_MAXLANES_MASK GENMASK(4, 3)
> > +#define XCSI_PCR_ACTLANES_MASK GENMASK(1, 0)
> > +
> > +#define XCSI_CSR_OFFSET 0x10
> > +#define XCSI_CSR_PKTCNT GENMASK(31, 16)
> > +#define XCSI_CSR_SPFIFOFULL BIT(3)
> > +#define XCSI_CSR_SPFIFONE BIT(2)
> > +#define XCSI_CSR_SLBF BIT(1)
> > +#define XCSI_CSR_RIPCD BIT(0)
> > +
> > +#define XCSI_GIER_OFFSET 0x20
> > +#define XCSI_GIER_GIE BIT(0)
> > +
> > +#define XCSI_ISR_OFFSET 0x24
> > +#define XCSI_IER_OFFSET 0x28
> > +
> > +#define XCSI_ISR_FR BIT(31)
> > +#define XCSI_ISR_VCXFE BIT(30)
> > +#define XCSI_ISR_WCC BIT(22)
> > +#define XCSI_ISR_ILC BIT(21)
> > +#define XCSI_ISR_SPFIFOF BIT(20)
> > +#define XCSI_ISR_SPFIFONE BIT(19)
> > +#define XCSI_ISR_SLBF BIT(18)
> > +#define XCSI_ISR_STOP BIT(17)
> > +#define XCSI_ISR_SOTERR BIT(13)
> > +#define XCSI_ISR_SOTSYNCERR BIT(12)
> > +#define XCSI_ISR_ECC2BERR BIT(11)
> > +#define XCSI_ISR_ECC1BERR BIT(10)
> > +#define XCSI_ISR_CRCERR BIT(9)
> > +#define XCSI_ISR_DATAIDERR BIT(8)
> > +#define XCSI_ISR_VC3FSYNCERR BIT(7)
> > +#define XCSI_ISR_VC3FLVLERR BIT(6)
> > +#define XCSI_ISR_VC2FSYNCERR BIT(5)
> > +#define XCSI_ISR_VC2FLVLERR BIT(4)
> > +#define XCSI_ISR_VC1FSYNCERR BIT(3)
> > +#define XCSI_ISR_VC1FLVLERR BIT(2)
> > +#define XCSI_ISR_VC0FSYNCERR BIT(1)
> > +#define XCSI_ISR_VC0FLVLERR BIT(0)
> > +
> > +#define XCSI_ISR_ALLINTR_MASK (0xc07e3fff)
> > +
> > +/*
> > + * Removed VCXFE mask as it doesn't exist in IER
> > + * Removed STOP state irq as this will keep driver in irq handler
> > +only */
> > +#define XCSI_IER_INTR_MASK (XCSI_ISR_ALLINTR_MASK &\
> > + ~(XCSI_ISR_STOP | XCSI_ISR_VCXFE))
> > +
> > +#define XCSI_SPKTR_OFFSET 0x30
> > +#define XCSI_SPKTR_DATA GENMASK(23, 8)
> > +#define XCSI_SPKTR_VC GENMASK(7, 6)
> > +#define XCSI_SPKTR_DT GENMASK(5, 0)
> > +#define XCSI_SPKT_FIFO_DEPTH 31
> > +
> > +#define XCSI_VCXR_OFFSET 0x34
> > +#define XCSI_VCXR_VCERR GENMASK(23, 0)
> > +#define XCSI_VCXR_FSYNCERR BIT(1)
> > +#define XCSI_VCXR_FLVLERR BIT(0)
> > +
> > +#define XCSI_CLKINFR_OFFSET 0x3C
> > +#define XCSI_CLKINFR_STOP BIT(1)
> > +
> > +#define XCSI_DLXINFR_OFFSET 0x40
> > +#define XCSI_DLXINFR_STOP BIT(5)
> > +#define XCSI_DLXINFR_SOTERR BIT(1)
> > +#define XCSI_DLXINFR_SOTSYNCERR BIT(0)
> > +#define XCSI_MAXDL_COUNT 0x4
> > +
> > +#define XCSI_VCXINF1R_OFFSET 0x60
> > +#define XCSI_VCXINF1R_LINECOUNT GENMASK(31, 16)
> > +#define XCSI_VCXINF1R_LINECOUNT_SHIFT 16
> > +#define XCSI_VCXINF1R_BYTECOUNT GENMASK(15, 0)
> > +
> > +#define XCSI_VCXINF2R_OFFSET 0x64
> > +#define XCSI_VCXINF2R_DT GENMASK(5, 0)
> > +#define XCSI_MAXVCX_COUNT 16
> > +
> > +/*
> > + * Sink pad connected to sensor source pad.
> > + * Source pad connected to next module like demosaic.
> > + */
> > +#define XCSI_MEDIA_PADS 2
> > +#define XCSI_DEFAULT_WIDTH 1920
> > +#define XCSI_DEFAULT_HEIGHT 1080
> > +
> > +/* MIPI CSI-2 Data Types from spec */
> > +#define XCSI_DT_YUV4228B 0x1e
> > +#define XCSI_DT_YUV42210B 0x1f
> > +#define XCSI_DT_RGB444 0x20
> > +#define XCSI_DT_RGB555 0x21
> > +#define XCSI_DT_RGB565 0x22
> > +#define XCSI_DT_RGB666 0x23
> > +#define XCSI_DT_RGB888 0x24
> > +#define XCSI_DT_RAW6 0x28
> > +#define XCSI_DT_RAW7 0x29
> > +#define XCSI_DT_RAW8 0x2a
> > +#define XCSI_DT_RAW10 0x2b
> > +#define XCSI_DT_RAW12 0x2c
> > +#define XCSI_DT_RAW14 0x2d
> > +#define XCSI_DT_RAW16 0x2e
> > +#define XCSI_DT_RAW20 0x2f
> > +
> > +#define XCSI_VCX_START 4
> > +#define XCSI_MAX_VC 4
> > +#define XCSI_MAX_VCX 16
> > +
> > +#define XCSI_NEXTREG_OFFSET 4
> > +
> > +/* There are 2 events frame sync and frame level error per VC */
> > +#define XCSI_VCX_NUM_EVENTS ((XCSI_MAX_VCX - XCSI_MAX_VC) * 2)
> > +
> > +/**
> > + * struct xcsi2rxss_event - Event log structure
> > + * @mask: Event mask
> > + * @name: Name of the event
> > + */
> > +struct xcsi2rxss_event {
> > + u32 mask;
> > + const char *name;
> > +};
> > +
> > +static const struct xcsi2rxss_event xcsi2rxss_events[] = {
> > + { XCSI_ISR_FR, "Frame Received" },
> > + { XCSI_ISR_VCXFE, "VCX Frame Errors" },
> > + { XCSI_ISR_WCC, "Word Count Errors" },
> > + { XCSI_ISR_ILC, "Invalid Lane Count Error" },
> > + { XCSI_ISR_SPFIFOF, "Short Packet FIFO OverFlow Error" },
> > + { XCSI_ISR_SPFIFONE, "Short Packet FIFO Not Empty" },
> > + { XCSI_ISR_SLBF, "Streamline Buffer Full Error" },
> > + { XCSI_ISR_STOP, "Lane Stop State" },
> > + { XCSI_ISR_SOTERR, "SOT Error" },
> > + { XCSI_ISR_SOTSYNCERR, "SOT Sync Error" },
> > + { XCSI_ISR_ECC2BERR, "2 Bit ECC Unrecoverable Error" },
> > + { XCSI_ISR_ECC1BERR, "1 Bit ECC Recoverable Error" },
> > + { XCSI_ISR_CRCERR, "CRC Error" },
> > + { XCSI_ISR_DATAIDERR, "Data Id Error" },
> > + { XCSI_ISR_VC3FSYNCERR, "Virtual Channel 3 Frame Sync Error" },
> > + { XCSI_ISR_VC3FLVLERR, "Virtual Channel 3 Frame Level Error" },
> > + { XCSI_ISR_VC2FSYNCERR, "Virtual Channel 2 Frame Sync Error" },
> > + { XCSI_ISR_VC2FLVLERR, "Virtual Channel 2 Frame Level Error" },
> > + { XCSI_ISR_VC1FSYNCERR, "Virtual Channel 1 Frame Sync Error" },
> > + { XCSI_ISR_VC1FLVLERR, "Virtual Channel 1 Frame Level Error" },
> > + { XCSI_ISR_VC0FSYNCERR, "Virtual Channel 0 Frame Sync Error" },
> > + { XCSI_ISR_VC0FLVLERR, "Virtual Channel 0 Frame Level Error" } };
> > +
> > +#define XCSI_NUM_EVENTS ARRAY_SIZE(xcsi2rxss_events)
> > +
> > +/*
> > + * This table provides a mapping between CSI-2 Data type
> > + * and media bus formats
> > + */
> > +static const u32 xcsi2dt_mbus_lut[][2] = {
> > + { XCSI_DT_YUV4228B, MEDIA_BUS_FMT_UYVY8_1X16 },
> > + { XCSI_DT_YUV42210B, MEDIA_BUS_FMT_UYVY10_1X20 },
> > + { XCSI_DT_RGB444, 0 },
> > + { XCSI_DT_RGB555, 0 },
> > + { XCSI_DT_RGB565, 0 },
> > + { XCSI_DT_RGB666, 0 },
> > + { XCSI_DT_RGB888, MEDIA_BUS_FMT_RBG888_1X24 },
> > + { XCSI_DT_RAW6, 0 },
> > + { XCSI_DT_RAW7, 0 },
> > + { XCSI_DT_RAW8, MEDIA_BUS_FMT_SRGGB8_1X8 },
> > + { XCSI_DT_RAW8, MEDIA_BUS_FMT_SBGGR8_1X8 },
> > + { XCSI_DT_RAW8, MEDIA_BUS_FMT_SGBRG8_1X8 },
> > + { XCSI_DT_RAW8, MEDIA_BUS_FMT_SGRBG8_1X8 },
> > + { XCSI_DT_RAW10, MEDIA_BUS_FMT_SRGGB10_1X10 },
> > + { XCSI_DT_RAW10, MEDIA_BUS_FMT_SBGGR10_1X10 },
> > + { XCSI_DT_RAW10, MEDIA_BUS_FMT_SGBRG10_1X10 },
> > + { XCSI_DT_RAW10, MEDIA_BUS_FMT_SGRBG10_1X10 },
> > + { XCSI_DT_RAW12, MEDIA_BUS_FMT_SRGGB12_1X12 },
> > + { XCSI_DT_RAW12, MEDIA_BUS_FMT_SBGGR12_1X12 },
> > + { XCSI_DT_RAW12, MEDIA_BUS_FMT_SGBRG12_1X12 },
> > + { XCSI_DT_RAW12, MEDIA_BUS_FMT_SGRBG12_1X12 },
> > + { XCSI_DT_RAW16, MEDIA_BUS_FMT_SRGGB16_1X16 },
> > + { XCSI_DT_RAW16, MEDIA_BUS_FMT_SBGGR16_1X16 },
> > + { XCSI_DT_RAW16, MEDIA_BUS_FMT_SGBRG16_1X16 },
> > + { XCSI_DT_RAW16, MEDIA_BUS_FMT_SGRBG16_1X16 },
> > + { XCSI_DT_RAW20, 0 },
> > +};
> > +
> > +/**
> > + * struct xcsi2rxss_state - CSI-2 Rx Subsystem device structure
> > + * @subdev: The v4l2 subdev structure
> > + * @format: Active V4L2 formats on each pad
> > + * @default_format: Default V4L2 format
> > + * @events: counter for events
> > + * @vcx_events: counter for vcx_events
> > + * @dev: Platform structure
> > + * @rsubdev: Remote subdev connected to sink pad
> > + * @rst_gpio: reset to video_aresetn
> > + * @clks: array of clocks
> > + * @iomem: Base address of subsystem
> > + * @max_num_lanes: Maximum number of lanes present
> > + * @datatype: Data type filter
> > + * @lock: mutex for accessing this structure
> > + * @pads: media pads
> > + * @streaming: Flag for storing streaming state
> > + * @enable_active_lanes: If number of active lanes can be modified
> > + * @en_vcx: If more than 4 VC are enabled
> > + *
> > + * This structure contains the device driver related parameters */
> > +struct xcsi2rxss_state {
> > + struct v4l2_subdev subdev;
> > + struct v4l2_mbus_framefmt format;
> > + struct v4l2_mbus_framefmt default_format;
> > + u32 events[XCSI_NUM_EVENTS];
> > + u32 vcx_events[XCSI_VCX_NUM_EVENTS];
> > + struct device *dev;
> > + struct v4l2_subdev *rsubdev;
> > + struct gpio_desc *rst_gpio;
> > + struct clk_bulk_data *clks;
> > + void __iomem *iomem;
> > + u32 max_num_lanes;
> > + u32 datatype;
> > + /* used to protect access to this struct */
> > + struct mutex lock;
> > + struct media_pad pads[XCSI_MEDIA_PADS];
> > + bool streaming;
> > + bool enable_active_lanes;
> > + bool en_vcx;
> > +};
> > +
> > +static const struct clk_bulk_data xcsi2rxss_clks[] = {
> > + { .id = "lite_aclk" },
> > + { .id = "video_aclk" },
> > +};
> > +
> > +static inline struct xcsi2rxss_state * to_xcsi2rxssstate(struct
> > +v4l2_subdev *subdev) {
> > + return container_of(subdev, struct xcsi2rxss_state, subdev); }
> > +
> > +/*
> > + * Register related operations
> > + */
> > +static inline u32 xcsi2rxss_read(struct xcsi2rxss_state *xcsi2rxss,
> > +u32 addr) {
> > + return ioread32(xcsi2rxss->iomem + addr); }
> > +
> > +static inline void xcsi2rxss_write(struct xcsi2rxss_state *xcsi2rxss, u32 addr,
> > + u32 value)
> > +{
> > + iowrite32(value, xcsi2rxss->iomem + addr); }
> > +
> > +static inline void xcsi2rxss_clr(struct xcsi2rxss_state *xcsi2rxss, u32 addr,
> > + u32 clr)
> > +{
> > + xcsi2rxss_write(xcsi2rxss, addr,
> > + xcsi2rxss_read(xcsi2rxss, addr) & ~clr); }
> > +
> > +static inline void xcsi2rxss_set(struct xcsi2rxss_state *xcsi2rxss, u32 addr,
> > + u32 set)
> > +{
> > + xcsi2rxss_write(xcsi2rxss, addr, xcsi2rxss_read(xcsi2rxss, addr) |
> > +set); }
> > +
> > +/*
> > + * This function returns the nth mbus for a data type.
> > + * In case of error, mbus code returned is 0.
> > + */
> > +static u32 xcsi2rxss_get_nth_mbus(u32 dt, u32 n) {
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(xcsi2dt_mbus_lut); i++) {
> > + if (xcsi2dt_mbus_lut[i][0] == dt) {
> > + if (n-- == 0)
> > + return xcsi2dt_mbus_lut[i][1];
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* This returns the data type for a media bus format else 0 */ static
> > +u32 xcsi2rxss_get_dt(u32 mbus) {
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(xcsi2dt_mbus_lut); i++) {
> > + if (xcsi2dt_mbus_lut[i][1] == mbus)
> > + return xcsi2dt_mbus_lut[i][0];
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcsi2rxss_soft_reset - Does a soft reset of the MIPI CSI-2 Rx
> > +Subsystem
> > + * @state: Xilinx CSI-2 Rx Subsystem structure pointer
> > + *
> > + * Core takes less than 100 video clock cycles to reset.
> > + * So a larger timeout value is chosen for margin.
> > + *
> > + * Return: 0 - on success OR -ETIME if reset times out */ static int
> > +xcsi2rxss_soft_reset(struct xcsi2rxss_state *state) {
> > + u32 timeout = 1000; /* us */
> > +
> > + xcsi2rxss_set(state, XCSI_CCR_OFFSET, XCSI_CCR_SOFTRESET);
> > +
> > + while (xcsi2rxss_read(state, XCSI_CSR_OFFSET) & XCSI_CSR_RIPCD) {
> > + if (timeout == 0) {
> > + dev_err(state->dev, "soft reset timed out!\n");
> > + return -ETIME;
> > + }
> > +
> > + timeout--;
> > + udelay(1);
> > + }
> > +
> > + xcsi2rxss_clr(state, XCSI_CCR_OFFSET, XCSI_CCR_SOFTRESET);
> > + return 0;
> > +}
> > +
> > +static void xcsi2rxss_hard_reset(struct xcsi2rxss_state *state) {
> > + if (!state->rst_gpio)
> > + return;
> > +
> > + /* minimum of 40 dphy_clk_200M cycles */
> > + gpiod_set_value_cansleep(state->rst_gpio, 1);
> > + usleep_range(1, 2);
> > + gpiod_set_value_cansleep(state->rst_gpio, 0); }
> > +
> > +static void xcsi2rxss_reset_event_counters(struct xcsi2rxss_state
> > +*state) {
> > + unsigned int i;
> > +
> > + for (i = 0; i < XCSI_NUM_EVENTS; i++)
> > + state->events[i] = 0;
> > +
> > + for (i = 0; i < XCSI_VCX_NUM_EVENTS; i++)
> > + state->vcx_events[i] = 0;
> > +}
> > +
> > +/* Print event counters */
> > +static void xcsi2rxss_log_counters(struct xcsi2rxss_state *state) {
> > + struct device *dev = state->dev;
> > + unsigned int i;
> > +
> > + for (i = 0; i < XCSI_NUM_EVENTS; i++) {
> > + if (state->events[i] > 0) {
> > + dev_info(dev, "%s events: %d\n",
> > + xcsi2rxss_events[i].name,
> > + state->events[i]);
> > + }
> > + }
> > +
> > + if (state->en_vcx) {
> > + for (i = 0; i < XCSI_VCX_NUM_EVENTS; i++) {
> > + if (state->vcx_events[i] > 0) {
> > + dev_info(dev,
> > + "VC %d Frame %s err vcx events:
> %d\n",
> > + (i / 2) + XCSI_VCX_START,
> > + i & 1 ? "Sync" : "Level",
> > + state->vcx_events[i]);
> > + }
> > + }
> > + }
> > +}
> > +
> > +/**
> > + * xcsi2rxss_log_status - Logs the status of the CSI-2 Receiver
> > + * @sd: Pointer to V4L2 subdevice structure
> > + *
> > + * This function prints the current status of Xilinx MIPI CSI-2
> > + *
> > + * Return: 0 on success
> > + */
> > +static int xcsi2rxss_log_status(struct v4l2_subdev *sd) {
> > + struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> > + struct device *dev = xcsi2rxss->dev;
> > + const char *tr = "true";
> > + const char *fa = "false";
>
> I didn't notice this in the previous version, but you could just write "true" and
> "false" below. The compiler should perform string de-duplication automatically.
>
I had this as a macro XCSI_GET_BITSET_STR till v11 and removed in v12
/* Macro to return "true" or "false" string if bit is set */
#define XCSI_GET_BITSET_STR(val, mask) (val) & (mask) ? "true" : "false"
I will update this to "true" and "false".
> > + u32 reg, data;
> > + unsigned int i, max_vc;
> > +
> > + mutex_lock(&xcsi2rxss->lock);
> > +
> > + xcsi2rxss_log_counters(xcsi2rxss);
> > +
> > + dev_info(dev, "***** Core Status *****\n");
> > + data = xcsi2rxss_read(xcsi2rxss, XCSI_CSR_OFFSET);
> > + dev_info(dev, "Short Packet FIFO Full = %s\n",
> > + data & XCSI_CSR_SPFIFOFULL ? tr : fa);
> > + dev_info(dev, "Short Packet FIFO Not Empty = %s\n",
> > + data & XCSI_CSR_SPFIFONE ? tr : fa);
> > + dev_info(dev, "Stream line buffer full = %s\n",
> > + data & XCSI_CSR_SLBF ? tr : fa);
> > + dev_info(dev, "Soft reset/Core disable in progress = %s\n",
> > + data & XCSI_CSR_RIPCD ? tr : fa);
> > +
> > + /* Clk & Lane Info */
> > + dev_info(dev, "******** Clock Lane Info *********\n");
> > + data = xcsi2rxss_read(xcsi2rxss, XCSI_CLKINFR_OFFSET);
> > + dev_info(dev, "Clock Lane in Stop State = %s\n",
> > + data & XCSI_CLKINFR_STOP ? tr : fa);
> > +
> > + dev_info(dev, "******** Data Lane Info *********\n");
> > + dev_info(dev, "Lane\tSoT Error\tSoT Sync Error\tStop State\n");
> > + reg = XCSI_DLXINFR_OFFSET;
> > + for (i = 0; i < XCSI_MAXDL_COUNT; i++) {
> > + data = xcsi2rxss_read(xcsi2rxss, reg);
> > +
> > + dev_info(dev, "%d\t%s\t\t%s\t\t%s\n", i,
> > + data & XCSI_DLXINFR_SOTERR ? tr : fa,
> > + data & XCSI_DLXINFR_SOTSYNCERR ? tr : fa,
> > + data & XCSI_DLXINFR_STOP ? tr : fa);
> > +
> > + reg += XCSI_NEXTREG_OFFSET;
> > + }
> > +
> > + /* Virtual Channel Image Information */
> > + dev_info(dev, "********** Virtual Channel Info ************\n");
> > + dev_info(dev, "VC\tLine Count\tByte Count\tData Type\n");
> > + if (xcsi2rxss->en_vcx)
> > + max_vc = XCSI_MAX_VCX;
> > + else
> > + max_vc = XCSI_MAX_VC;
> > +
> > + reg = XCSI_VCXINF1R_OFFSET;
> > + for (i = 0; i < max_vc; i++) {
> > + u32 line_count, byte_count, data_type;
> > +
> > + /* Get line and byte count from VCXINFR1 Register */
> > + data = xcsi2rxss_read(xcsi2rxss, reg);
> > + byte_count = data & XCSI_VCXINF1R_BYTECOUNT;
> > + line_count = data & XCSI_VCXINF1R_LINECOUNT;
> > + line_count >>= XCSI_VCXINF1R_LINECOUNT_SHIFT;
> > +
> > + /* Get data type from VCXINFR2 Register */
> > + reg += XCSI_NEXTREG_OFFSET;
> > + data = xcsi2rxss_read(xcsi2rxss, reg);
> > + data_type = data & XCSI_VCXINF2R_DT;
> > +
> > + dev_info(dev, "%d\t%d\t\t%d\t\t0x%x\n", i, line_count,
> > + byte_count, data_type);
> > +
> > + /* Move to next pair of VC Info registers */
> > + reg += XCSI_NEXTREG_OFFSET;
> > + }
> > +
> > + mutex_unlock(&xcsi2rxss->lock);
> > +
> > + return 0;
> > +}
> > +
> > +static struct v4l2_subdev *xcsi2rxss_get_remote_subdev(struct
> > +media_pad *local) {
> > + struct media_pad *remote;
> > +
> > + remote = media_entity_remote_pad(local);
> > + if (!remote || !is_media_entity_v4l2_subdev(remote->entity))
> > + return NULL;
> > +
> > + return media_entity_to_v4l2_subdev(remote->entity);
> > +}
> > +
> > +static int xcsi2rxss_start_stream(struct xcsi2rxss_state *state) {
> > + int ret = 0;
> > +
> > + /* enable core */
> > + xcsi2rxss_set(state, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE);
> > +
> > + ret = xcsi2rxss_soft_reset(state);
> > + if (ret < 0) {
> > + state->streaming = false;
> > + return ret;
> > + }
> > +
> > + /* enable interrupts */
> > + xcsi2rxss_clr(state, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> > + xcsi2rxss_write(state, XCSI_IER_OFFSET, XCSI_IER_INTR_MASK);
> > + xcsi2rxss_set(state, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> > +
> > + state->streaming = true;
> > +
> > + state->rsubdev =
> > + xcsi2rxss_get_remote_subdev(&state->pads[XVIP_PAD_SINK]);
> > +
> > + ret = v4l2_subdev_call(state->rsubdev, video, s_stream, 1);
> > + if (ret) {
> > + /* disable interrupts */
> > + xcsi2rxss_clr(state, XCSI_IER_OFFSET, XCSI_IER_INTR_MASK);
> > + xcsi2rxss_clr(state, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> > +
> > + /* disable core */
> > + xcsi2rxss_clr(state, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE);
> > + state->streaming = false;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static void xcsi2rxss_stop_stream(struct xcsi2rxss_state *state) {
> > + v4l2_subdev_call(state->rsubdev, video, s_stream, 0);
> > +
> > + /* disable interrupts */
> > + xcsi2rxss_clr(state, XCSI_IER_OFFSET, XCSI_IER_INTR_MASK);
> > + xcsi2rxss_clr(state, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> > +
> > + /* disable core */
> > + xcsi2rxss_clr(state, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE);
> > + state->streaming = false;
> > +}
> > +
> > +/**
> > + * xcsi2rxss_irq_handler - Interrupt handler for CSI-2
> > + * @irq: IRQ number
> > + * @data: Pointer to device state
> > + *
> > + * In the interrupt handler, a list of event counters are updated for
> > + * corresponding interrupts. This is useful to get status / debug.
> > + *
> > + * Return: IRQ_HANDLED after handling interrupts */ static
> > +irqreturn_t xcsi2rxss_irq_handler(int irq, void *data) {
> > + struct xcsi2rxss_state *state = (struct xcsi2rxss_state *)data;
> > + struct device *dev = state->dev;
> > + u32 status;
> > +
> > + status = xcsi2rxss_read(state, XCSI_ISR_OFFSET) &
> XCSI_ISR_ALLINTR_MASK;
> > + xcsi2rxss_write(state, XCSI_ISR_OFFSET, status);
> > +
> > + /* Received a short packet */
> > + if (status & XCSI_ISR_SPFIFONE) {
> > + u32 count = 0;
> > +
> > + /*
> > + * Drain generic short packet FIFO by reading max 31
> > + * (fifo depth) short packets from fifo or till fifo is empty.
> > + */
> > + for (count = 0; count < XCSI_SPKT_FIFO_DEPTH; ++count) {
> > + u32 spfifostat, spkt;
> > +
> > + spkt = xcsi2rxss_read(state, XCSI_SPKTR_OFFSET);
> > + dev_dbg(dev, "Short packet = 0x%08x\n", spkt);
> > + spfifostat = xcsi2rxss_read(state, XCSI_ISR_OFFSET);
> > + spfifostat &= XCSI_ISR_SPFIFONE;
> > + if (!spfifostat)
> > + break;
> > + xcsi2rxss_write(state, XCSI_ISR_OFFSET, spfifostat);
> > + }
> > + }
> > +
> > + /* Short packet FIFO overflow */
> > + if (status & XCSI_ISR_SPFIFOF)
> > + dev_dbg_ratelimited(dev, "Short packet FIFO overflowed\n");
> > +
> > + /*
> > + * Stream line buffer full
> > + * This means there is a backpressure from downstream IP
> > + */
> > + if (status & XCSI_ISR_SLBF) {
> > + dev_alert_ratelimited(dev, "Stream Line Buffer Full!\n");
> > +
> > + /* disable interrupts */
> > + xcsi2rxss_clr(state, XCSI_IER_OFFSET, XCSI_IER_INTR_MASK);
> > + xcsi2rxss_clr(state, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> > +
> > + /* disable core */
> > + xcsi2rxss_clr(state, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE);
> > + state->streaming = false;
>
> If you set streaming to false, a later call to xcsi2rxss_s_stream(0) will consider
> that the stream is already stopped, and skip calling
> xcsi2rxss_stop_stream() and xcsi2rxss_hard_reset(). Now that you have
> dropped the hard reset from the interrupt handler, we need to rely on it being
> called at stream stop time. Should you just drop the line here ?
>
Yes I can drop the line "state->streaming = false" here in next version.
The comment below should guide the user to stop streaming in stream line buffer full case.
> > +
> > + /*
> > + * The IP needs to be hard reset before it can be used now.
> > + * This will be done in streamoff.
> > + */
> > +
> > + /*
> > + * TODO: Notify the whole pipeline with v4l2_subdev_notify()
> to
> > + * inform userspace.
> > + */
> > + }
> > +
> > + /* Increment event counters */
> > + if (status & XCSI_ISR_ALLINTR_MASK) {
> > + unsigned int i;
> > +
> > + for (i = 0; i < XCSI_NUM_EVENTS; i++) {
> > + if (!(status & xcsi2rxss_events[i].mask))
> > + continue;
> > + state->events[i]++;
> > + dev_dbg_ratelimited(dev, "%s: %u\n",
> > + xcsi2rxss_events[i].name,
> > + state->events[i]);
> > + }
> > +
> > + if (status & XCSI_ISR_VCXFE && state->en_vcx) {
> > + u32 vcxstatus;
> > +
> > + vcxstatus = xcsi2rxss_read(state, XCSI_VCXR_OFFSET);
> > + vcxstatus &= XCSI_VCXR_VCERR;
> > + for (i = 0; i < XCSI_VCX_NUM_EVENTS; i++) {
> > + if (!(vcxstatus & (1 << i)))
> > + continue;
> > + state->vcx_events[i]++;
> > + }
> > + xcsi2rxss_write(state, XCSI_VCXR_OFFSET, vcxstatus);
> > + }
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +/**
> > + * xcsi2rxss_s_stream - It is used to start/stop the streaming.
> > + * @sd: V4L2 Sub device
> > + * @enable: Flag (True / False)
> > + *
> > + * This function controls the start or stop of streaming for the
> > + * Xilinx MIPI CSI-2 Rx Subsystem.
> > + *
> > + * Return: 0 on success, errors otherwise */ static int
> > +xcsi2rxss_s_stream(struct v4l2_subdev *sd, int enable) {
> > + struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> > + int ret = 0;
> > +
> > + mutex_lock(&xcsi2rxss->lock);
> > +
> > + if (enable == xcsi2rxss->streaming)
> > + goto stream_done;
> > +
> > + if (enable) {
> > + xcsi2rxss_reset_event_counters(xcsi2rxss);
> > + ret = xcsi2rxss_start_stream(xcsi2rxss);
> > + } else {
> > + xcsi2rxss_stop_stream(xcsi2rxss);
> > + xcsi2rxss_hard_reset(xcsi2rxss);
> > + }
> > +
> > +stream_done:
> > + mutex_unlock(&xcsi2rxss->lock);
> > + return ret;
> > +}
> > +
> > +static struct v4l2_mbus_framefmt *
> > +__xcsi2rxss_get_pad_format(struct xcsi2rxss_state *xcsi2rxss,
> > + struct v4l2_subdev_pad_config *cfg,
> > + unsigned int pad, u32 which)
> > +{
> > + switch (which) {
> > + case V4L2_SUBDEV_FORMAT_TRY:
> > + return v4l2_subdev_get_try_format(&xcsi2rxss->subdev, cfg,
> pad);
> > + case V4L2_SUBDEV_FORMAT_ACTIVE:
> > + return &xcsi2rxss->format;
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > +/**
> > + * xcsi2rxss_init_cfg - Initialise the pad format config to default
> > + * @sd: Pointer to V4L2 Sub device structure
> > + * @cfg: Pointer to sub device pad information structure
> > + *
> > + * This function is used to initialize the pad format with the
> > +default
> > + * values.
> > + *
> > + * Return: 0 on success
> > + */
> > +static int xcsi2rxss_init_cfg(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg) {
> > + struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> > + struct v4l2_mbus_framefmt *format;
> > + unsigned int i;
> > +
> > + mutex_lock(&xcsi2rxss->lock);
> > + for (i = 0; i < XCSI_MEDIA_PADS; i++) {
> > + format = v4l2_subdev_get_try_format(sd, cfg, i);
> > + *format = xcsi2rxss->default_format;
> > + }
> > + mutex_unlock(&xcsi2rxss->lock);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcsi2rxss_get_format - Get the pad format
> > + * @sd: Pointer to V4L2 Sub device structure
> > + * @cfg: Pointer to sub device pad information structure
> > + * @fmt: Pointer to pad level media bus format
> > + *
> > + * This function is used to get the pad format information.
> > + *
> > + * Return: 0 on success
> > + */
> > +static int xcsi2rxss_get_format(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *fmt)
> > +{
> > + struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> > +
> > + mutex_lock(&xcsi2rxss->lock);
> > + fmt->format = *__xcsi2rxss_get_pad_format(xcsi2rxss, cfg, fmt->pad,
> > + fmt->which);
> > + mutex_unlock(&xcsi2rxss->lock);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcsi2rxss_set_format - This is used to set the pad format
> > + * @sd: Pointer to V4L2 Sub device structure
> > + * @cfg: Pointer to sub device pad information structure
> > + * @fmt: Pointer to pad level media bus format
> > + *
> > + * This function is used to set the pad format. Since the pad format
> > +is fixed
> > + * in hardware, it can't be modified on run time. So when a format
> > +set is
> > + * requested by application, all parameters except the format type is
> > +saved
> > + * for the pad and the original pad format is sent back to the application.
> > + *
> > + * Return: 0 on success
> > + */
> > +static int xcsi2rxss_set_format(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *fmt)
> > +{
> > + struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> > + struct v4l2_mbus_framefmt *__format;
> > + u32 dt;
> > +
> > + /* only sink pad format can be updated */
> > + mutex_lock(&xcsi2rxss->lock);
> > +
> > + /*
> > + * Only the format->code parameter matters for CSI as the
> > + * CSI format cannot be changed at runtime.
> > + * Ensure that format to set is copied to over to CSI pad format
> > + */
> > + __format = __xcsi2rxss_get_pad_format(xcsi2rxss, cfg,
> > + fmt->pad, fmt->which);
> > +
> > + if (fmt->pad == XVIP_PAD_SOURCE) {
> > + fmt->format = *__format;
> > + mutex_unlock(&xcsi2rxss->lock);
> > + return 0;
> > + }
> > +
> > + /*
> > + * RAW8 is supported in all datatypes. So if requested media bus
> format
> > + * is of RAW8 type, then allow to be set. In case core is configured to
> > + * other RAW, YUV422 8/10 or RGB888, set appropriate media bus
> format.
> > + */
> > + dt = xcsi2rxss_get_dt(fmt->format.code);
> > + if (dt != xcsi2rxss->datatype && dt != XCSI_DT_RAW8) {
> > + dev_dbg(xcsi2rxss->dev, "Unsupported media bus format");
> > + /* set the default format for the data type */
> > + fmt->format.code = xcsi2rxss_get_nth_mbus(xcsi2rxss-
> >datatype,
> > + 0);
> > + }
> > +
> > + *__format = fmt->format;
> > + mutex_unlock(&xcsi2rxss->lock);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * xcsi2rxss_enum_mbus_code - Handle pixel format enumeration
> > + * @sd : pointer to v4l2 subdev structure
> > + * @cfg: V4L2 subdev pad configuration
> > + * @code : pointer to v4l2_subdev_mbus_code_enum structure
> > + *
> > + * Return: -EINVAL or zero on success */ int
> > +xcsi2rxss_enum_mbus_code(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_mbus_code_enum *code)
>
> As commented by the kbuild bot, this function should be static.
Yes I will update this in next version.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Good to go :-)
>
Great! 😊
> > +{
> > + struct xcsi2rxss_state *state = to_xcsi2rxssstate(sd);
> > + u32 dt, n;
> > + int ret = 0;
> > +
> > + /* RAW8 dt packets are available in all DT configurations */
> > + if (code->index < 4) {
> > + n = code->index;
> > + dt = XCSI_DT_RAW8;
> > + } else if (state->datatype != XCSI_DT_RAW8) {
> > + n = code->index - 4;
> > + dt = state->datatype;
> > + } else {
> > + return -EINVAL;
> > + }
> > +
> > + code->code = xcsi2rxss_get_nth_mbus(dt, n);
> > + if (!code->code)
> > + ret = -EINVAL;
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > +---------------------------------------------------------------------
> > +--------
> > + * Media Operations
> > + */
> > +
> > +static const struct media_entity_operations xcsi2rxss_media_ops = {
> > + .link_validate = v4l2_subdev_link_validate };
> > +
> > +static const struct v4l2_subdev_core_ops xcsi2rxss_core_ops = {
> > + .log_status = xcsi2rxss_log_status,
> > +};
> > +
> > +static const struct v4l2_subdev_video_ops xcsi2rxss_video_ops = {
> > + .s_stream = xcsi2rxss_s_stream
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops xcsi2rxss_pad_ops = {
> > + .init_cfg = xcsi2rxss_init_cfg,
> > + .get_fmt = xcsi2rxss_get_format,
> > + .set_fmt = xcsi2rxss_set_format,
> > + .enum_mbus_code = xcsi2rxss_enum_mbus_code,
> > + .link_validate = v4l2_subdev_link_validate_default,
> > +};
> > +
> > +static const struct v4l2_subdev_ops xcsi2rxss_ops = {
> > + .core = &xcsi2rxss_core_ops,
> > + .video = &xcsi2rxss_video_ops,
> > + .pad = &xcsi2rxss_pad_ops
> > +};
> > +
> > +static int xcsi2rxss_parse_of(struct xcsi2rxss_state *xcsi2rxss) {
> > + struct device *dev = xcsi2rxss->dev;
> > + struct device_node *node = dev->of_node;
> > +
> > + struct fwnode_handle *ep;
> > + struct v4l2_fwnode_endpoint vep = {
> > + .bus_type = V4L2_MBUS_CSI2_DPHY
> > + };
> > + bool en_csi_v20, vfb;
> > + int ret;
> > +
> > + en_csi_v20 = of_property_read_bool(node, "xlnx,en-csi-v2-0");
> > + if (en_csi_v20)
> > + xcsi2rxss->en_vcx = of_property_read_bool(node, "xlnx,en-
> vcx");
> > +
> > + xcsi2rxss->enable_active_lanes =
> > + of_property_read_bool(node, "xlnx,en-active-lanes");
> > +
> > + ret = of_property_read_u32(node, "xlnx,csi-pxl-format",
> > + &xcsi2rxss->datatype);
> > + if (ret < 0) {
> > + dev_err(dev, "missing xlnx,csi-pxl-format property\n");
> > + return ret;
> > + }
> > +
> > + switch (xcsi2rxss->datatype) {
> > + case XCSI_DT_YUV4228B:
> > + case XCSI_DT_RGB444:
> > + case XCSI_DT_RGB555:
> > + case XCSI_DT_RGB565:
> > + case XCSI_DT_RGB666:
> > + case XCSI_DT_RGB888:
> > + case XCSI_DT_RAW6:
> > + case XCSI_DT_RAW7:
> > + case XCSI_DT_RAW8:
> > + case XCSI_DT_RAW10:
> > + case XCSI_DT_RAW12:
> > + case XCSI_DT_RAW14:
> > + break;
> > + case XCSI_DT_YUV42210B:
> > + case XCSI_DT_RAW16:
> > + case XCSI_DT_RAW20:
> > + if (!en_csi_v20) {
> > + ret = -EINVAL;
> > + dev_dbg(dev, "enable csi v2 for this pixel format");
> > + }
> > + break;
> > + default:
> > + ret = -EINVAL;
> > + }
> > + if (ret < 0) {
> > + dev_err(dev, "invalid csi-pxl-format property!\n");
> > + return ret;
> > + }
> > +
> > + vfb = of_property_read_bool(node, "xlnx,vfb");
> > + if (!vfb) {
> > + dev_err(dev, "operation without VFB is not supported\n");
> > + return -EINVAL;
> > + }
> > +
> > + ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
> > + XVIP_PAD_SINK, 0,
> > +
> FWNODE_GRAPH_ENDPOINT_NEXT);
> > + if (!ep) {
> > + dev_err(dev, "no sink port found");
> > + return -EINVAL;
> > + }
> > +
> > + ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> > + fwnode_handle_put(ep);
> > + if (ret) {
> > + dev_err(dev, "error parsing sink port");
> > + return ret;
> > + }
> > +
> > + dev_dbg(dev, "mipi number lanes = %d\n",
> > + vep.bus.mipi_csi2.num_data_lanes);
> > +
> > + xcsi2rxss->max_num_lanes = vep.bus.mipi_csi2.num_data_lanes;
> > +
> > + ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
> > + XVIP_PAD_SOURCE, 0,
> > +
> FWNODE_GRAPH_ENDPOINT_NEXT);
> > + if (!ep) {
> > + dev_err(dev, "no source port found");
> > + return -EINVAL;
> > + }
> > +
> > + fwnode_handle_put(ep);
> > +
> > + dev_dbg(dev, "vcx %s, %u data lanes (%s), data type 0x%02x\n",
> > + xcsi2rxss->en_vcx ? "enabled" : "disabled",
> > + xcsi2rxss->max_num_lanes,
> > + xcsi2rxss->enable_active_lanes ? "dynamic" : "static",
> > + xcsi2rxss->datatype);
> > +
> > + return 0;
> > +}
> > +
> > +static int xcsi2rxss_probe(struct platform_device *pdev) {
> > + struct v4l2_subdev *subdev;
> > + struct xcsi2rxss_state *xcsi2rxss;
> > + int num_clks = ARRAY_SIZE(xcsi2rxss_clks);
> > + struct device *dev = &pdev->dev;
> > + int irq, ret;
> > +
> > + xcsi2rxss = devm_kzalloc(dev, sizeof(*xcsi2rxss), GFP_KERNEL);
> > + if (!xcsi2rxss)
> > + return -ENOMEM;
> > +
> > + xcsi2rxss->dev = dev;
> > +
> > + xcsi2rxss->clks = devm_kmemdup(dev, xcsi2rxss_clks,
> > + sizeof(xcsi2rxss_clks), GFP_KERNEL);
> > + if (!xcsi2rxss->clks)
> > + return -ENOMEM;
> > +
> > + /* Reset GPIO */
> > + xcsi2rxss->rst_gpio = devm_gpiod_get_optional(dev, "video-reset",
> > + GPIOD_OUT_HIGH);
> > + if (IS_ERR(xcsi2rxss->rst_gpio)) {
> > + if (PTR_ERR(xcsi2rxss->rst_gpio) != -EPROBE_DEFER)
> > + dev_err(dev, "Video Reset GPIO not setup in DT");
> > + return PTR_ERR(xcsi2rxss->rst_gpio);
> > + }
> > +
> > + ret = xcsi2rxss_parse_of(xcsi2rxss);
> > + if (ret < 0)
> > + return ret;
> > +
> > + xcsi2rxss->iomem = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(xcsi2rxss->iomem))
> > + return PTR_ERR(xcsi2rxss->iomem);
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0)
> > + return irq;
> > +
> > + ret = devm_request_threaded_irq(dev, irq, NULL,
> > + xcsi2rxss_irq_handler,
> IRQF_ONESHOT,
> > + dev_name(dev), xcsi2rxss);
> > + if (ret) {
> > + dev_err(dev, "Err = %d Interrupt handler reg failed!\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = clk_bulk_get(dev, num_clks, xcsi2rxss->clks);
> > + if (ret)
> > + return ret;
> > +
> > + /* TODO: Enable/disable clocks at stream on/off time. */
> > + ret = clk_bulk_prepare_enable(num_clks, xcsi2rxss->clks);
> > + if (ret)
> > + goto err_clk_put;
> > +
> > + mutex_init(&xcsi2rxss->lock);
> > +
> > + xcsi2rxss_hard_reset(xcsi2rxss);
> > + xcsi2rxss_soft_reset(xcsi2rxss);
> > +
> > + /* Initialize V4L2 subdevice and media entity */
> > + xcsi2rxss->pads[XVIP_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> > + xcsi2rxss->pads[XVIP_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
> > +
> > + /* Initialize the default format */
> > + xcsi2rxss->default_format.code =
> > + xcsi2rxss_get_nth_mbus(xcsi2rxss->datatype, 0);
> > + xcsi2rxss->default_format.field = V4L2_FIELD_NONE;
> > + xcsi2rxss->default_format.colorspace = V4L2_COLORSPACE_SRGB;
> > + xcsi2rxss->default_format.width = XCSI_DEFAULT_WIDTH;
> > + xcsi2rxss->default_format.height = XCSI_DEFAULT_HEIGHT;
> > + xcsi2rxss->format = xcsi2rxss->default_format;
> > +
> > + /* Initialize V4L2 subdevice and media entity */
> > + subdev = &xcsi2rxss->subdev;
> > + v4l2_subdev_init(subdev, &xcsi2rxss_ops);
> > + subdev->dev = dev;
> > + strscpy(subdev->name, dev_name(dev), sizeof(subdev->name));
> > + subdev->flags |= V4L2_SUBDEV_FL_HAS_EVENTS |
> V4L2_SUBDEV_FL_HAS_DEVNODE;
> > + subdev->entity.ops = &xcsi2rxss_media_ops;
> > + v4l2_set_subdevdata(subdev, xcsi2rxss);
> > +
> > + ret = media_entity_pads_init(&subdev->entity, XCSI_MEDIA_PADS,
> > + xcsi2rxss->pads);
> > + if (ret < 0)
> > + goto error;
> > +
> > + platform_set_drvdata(pdev, xcsi2rxss);
> > +
> > + ret = v4l2_async_register_subdev(subdev);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to register subdev\n");
> > + goto error;
> > + }
> > +
> > + return 0;
> > +error:
> > + media_entity_cleanup(&subdev->entity);
> > + mutex_destroy(&xcsi2rxss->lock);
> > + clk_bulk_disable_unprepare(num_clks, xcsi2rxss->clks);
> > +err_clk_put:
> > + clk_bulk_put(num_clks, xcsi2rxss->clks);
> > + return ret;
> > +}
> > +
> > +static int xcsi2rxss_remove(struct platform_device *pdev) {
> > + struct xcsi2rxss_state *xcsi2rxss = platform_get_drvdata(pdev);
> > + struct v4l2_subdev *subdev = &xcsi2rxss->subdev;
> > + int num_clks = ARRAY_SIZE(xcsi2rxss_clks);
> > +
> > + v4l2_async_unregister_subdev(subdev);
> > + media_entity_cleanup(&subdev->entity);
> > + mutex_destroy(&xcsi2rxss->lock);
> > + clk_bulk_disable_unprepare(num_clks, xcsi2rxss->clks);
> > + clk_bulk_put(num_clks, xcsi2rxss->clks);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id xcsi2rxss_of_id_table[] = {
> > + { .compatible = "xlnx,mipi-csi2-rx-subsystem-5.0", },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, xcsi2rxss_of_id_table);
> > +
> > +static struct platform_driver xcsi2rxss_driver = {
> > + .driver = {
> > + .name = "xilinx-csi2rxss",
> > + .of_match_table = xcsi2rxss_of_id_table,
> > + },
> > + .probe = xcsi2rxss_probe,
> > + .remove = xcsi2rxss_remove,
> > +};
> > +
> > +module_platform_driver(xcsi2rxss_driver);
> > +
> > +MODULE_AUTHOR("Vishal Sagar <vsagar@xilinx.com>");
> > +MODULE_DESCRIPTION("Xilinx MIPI CSI-2 Rx Subsystem Driver");
> > +MODULE_LICENSE("GPL v2");
>
> --
> Regards,
>
> Laurent Pinchart
Regards
Vishal Sagar
_______________________________________________
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: ptrace: Fix PTRACE_SINGLESTEP into signal handler
From: Will Deacon @ 2020-05-27 12:06 UTC (permalink / raw)
To: Keno Fischer; +Cc: catalin.marinas, oleg, linux-arm-kernel, linux-kernel
In-Reply-To: <20200524043827.GA33001@juliacomputing.com>
On Sun, May 24, 2020 at 12:38:27AM -0400, Keno Fischer wrote:
> Executing PTRACE_SINGLESTEP at a signal stop is special. It
> is supposed to step merely the signal setup work that the
> kernel does, but not any instructions in user space. Since
> not all architectures have the ability to generate a
> single-step exception directly upon return from user-space,
> there is a generic pseudo-single-step-stop that may be used
> for this purpose (tracehook_signal_handler). Now, arm64 does
> have the ability to generate single-step exceptions directly
> upon return to userspace and was using this capability (rather
> than the generic pseudo-trap) to obtain a similar effect. However,
> there is actually a subtle difference that becomes noticeable
> when the signal handler in question attempts to block SIGTRAP
> (either because it is set in sa_mask, or because it is a handler
> for SIGTRAP itself and SA_NODEFER is not set). In such a
> situation, a real single step exception will cause the SIGTRAP
> signal to be forcibly unblocked and the signal disposition
> to be reset to SIG_DFL. The generic pseudo-single-step does
> not suffer from this problem, because the SIGTRAP it delivers
> is not real. The arm64 behavior is problematic, because a forced
> reset of the signal disposition can be quite disruptive to the
> userspace program. This patch brings the arm64 behavior in line
> with the other major architectures by using the generic
> pseudo-single-step-stop, avoiding the problematic interaction
> with SIGTRAP masks.
>
> Fixes: 2c020ed8 ("arm64: Signal handling support")
nit: please use a 12-character ID here.
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---
> arch/arm64/kernel/signal.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> index 339882db5a91..cf237ee9443b 100644
> --- a/arch/arm64/kernel/signal.c
> +++ b/arch/arm64/kernel/signal.c
> @@ -808,14 +808,7 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs)
> */
> ret |= !valid_user_regs(®s->user_regs, current);
>
> - /*
> - * Fast forward the stepping logic so we step into the signal
> - * handler.
> - */
> - if (!ret)
> - user_fastforward_single_step(tsk);
> -
> - signal_setup_done(ret, ksig, 0);
> + signal_setup_done(ret, ksig, test_thread_flag(TIF_SINGLESTEP));
another nit: tsk is now unused, so this generates a compiler warning:
arch/arm64/kernel/signal.c:787:22: warning: unused variable 'tsk' [-Wunused-variable]
struct task_struct *tsk = current;
^
1 warning generated.
Also, the si_code used by signal_setup_done seems to be SIGTRAP, whereas
we usually set TRAP_TRACE. What's the correct behaviour here? Looks like x86
uses TRAP_BRKPT... :/
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC 00/50] staging: vchiq: Getting rid of the vchi/vchiq split
From: Greg KH @ 2020-05-27 12:08 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: devel, kernel-list, linux-kernel, bcm-kernel-feedback-list,
linux-rpi-kernel, linux-arm-kernel, laurent.pinchart
In-Reply-To: <20200527115400.31391-1-nsaenzjulienne@suse.de>
On Wed, May 27, 2020 at 01:53:05PM +0200, Nicolas Saenz Julienne wrote:
> vchi acts as a mid layer between vchiq and its kernel services, while
> arguably providing little to no benefit: half of the functions exposed
> are a 1:1 copy of vchiq's, and the rest provide some functionality which
> can be easly integrated into vchiq without all the churn. Moreover it
> has been found in the past as a blockage to further fixes in vchiq as
> every change needed its vchi counterpart, if even possible.
>
> Hence this series, which merges all vchi functionality into vchiq and
> provies a simpler and more concise API to services.
>
> I'm aware that kernel's vchi API tries to mimic its userspace
> counterpart (or vice versa). Obviously this breaks the parity, but I
> don't think it's a sane goal to have. There is little sense or gain from
> it, and adds impossible constraints to upstreaming the driver.
>
> Overall the series falls short of removing 1500 lines of code, which is
> pretty neat on itself.
>
> So far it has been tested trough bcm2835-camera, audio and vchiq-test. I
> can't do much about vc-sm-cma for now, but the changes are done in a way
> that shouldn't affect its behaviour.
>
> Note that the series builds up on RPi/Laurent's camera support series[1]
> and can't yet be merged. We'd have to coordinate here. We could either
> wait for the vc_sm_cma rework (if it's not going to take months and
> months to finish), or factor out all the vc-sm-cma stuff, merge that into
> the downstream kernel and take the rest of the series on top of
> Laurent's mmal-vchiq changes.
These look great, but due to the dependancy, I can't take them all.
But I can take the first 10, so I did that, they are now queued up,
thanks!
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
From: Valentin Schneider @ 2020-05-27 12:14 UTC (permalink / raw)
To: Benjamin GAIGNARD
Cc: len.brown@intel.com, Alexandre TORGUE, linux-pm@vger.kernel.org,
viresh.kumar@linaro.org, pavel@ucw.cz, rjw@rjwysocki.net,
linux-kernel@vger.kernel.org, mcoquelin.stm32@gmail.com,
Hugues FRUCHET, mchehab@kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <ab4340c0-bda3-e752-9073-e162e6325bb1@st.com>
On 27/05/20 12:17, Benjamin GAIGNARD wrote:
> On 5/27/20 12:09 PM, Valentin Schneider wrote:
>> Hi Benjamin,
>>
>> On 26/05/20 16:16, Benjamin Gaignard wrote:
>>> A first round [1] of discussions and suggestions have already be done on
>>> this series but without found a solution to the problem. I resend it to
>>> progress on this topic.
>>>
>> Apologies for sleeping on that previous thread.
>>
>> So what had been suggested over there was to use uclamp to boost the
>> frequency of the handling thread; however if you use threaded IRQs you
>> get RT threads, which already get the max frequency by default (at least
>> with schedutil).
>>
>> Does that not work for you, and if so, why?
>
> That doesn't work because almost everything is done by the hardware blocks
> without charge the CPU so the thread isn't running.
I'm not sure I follow; the frequency of the CPU doesn't matter while
your hardware blocks are spinning, right? AIUI what matters is running
your interrupt handler / action at max freq, which you get if you use
threaded IRQs and schedutil.
I think it would help if you could clarify which tasks / parts of your
pipeline you need running at high frequencies. The point is that setting
a QoS request affects all tasks, whereas we could be smarter and only
boost the required tasks.
> I have done the
> tests with schedutil
> and ondemand scheduler (which is the one I'm targeting). I have no
> issues when using
> performance scheduler because it always keep the highest frequencies.
>
>
>>
>>> When start streaming from the sensor the CPU load could remain very low
>>> because almost all the capture pipeline is done in hardware (i.e. without
>>> using the CPU) and let believe to cpufreq governor that it could use lower
>>> frequencies. If the governor decides to use a too low frequency that
>>> becomes a problem when we need to acknowledge the interrupt during the
>>> blanking time.
>>> The delay to ack the interrupt and perform all the other actions before
>>> the next frame is very short and doesn't allow to the cpufreq governor to
>>> provide the required burst of power. That led to drop the half of the frames.
>>>
>>> To avoid this problem, DCMI driver informs the cpufreq governors by adding
>>> a cpufreq minimum load QoS resquest.
>>>
>>> Benjamin
>>>
>>> [1] https://lkml.org/lkml/2020/4/24/360
>>>
>>> Benjamin Gaignard (3):
>>> PM: QoS: Introduce cpufreq minimum load QoS
>>> cpufreq: governor: Use minimum load QoS
>>> media: stm32-dcmi: Inform cpufreq governors about cpu load needs
>>>
>>> drivers/cpufreq/cpufreq_governor.c | 5 +
>>> drivers/media/platform/stm32/stm32-dcmi.c | 8 ++
>>> include/linux/pm_qos.h | 12 ++
>>> kernel/power/qos.c | 213 ++++++++++++++++++++++++++++++
>>> 4 files changed, 238 insertions(+)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
From: Vincent Guittot @ 2020-05-27 12:22 UTC (permalink / raw)
To: Benjamin GAIGNARD
Cc: len.brown@intel.com, Alexandre TORGUE, linux-pm@vger.kernel.org,
viresh.kumar@linaro.org, pavel@ucw.cz, rjw@rjwysocki.net,
linux-kernel@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
mcoquelin.stm32@gmail.com, Hugues FRUCHET, mchehab@kernel.org,
Valentin Schneider, linux-arm-kernel@lists.infradead.org,
linux-media@vger.kernel.org
In-Reply-To: <ab4340c0-bda3-e752-9073-e162e6325bb1@st.com>
On Wed, 27 May 2020 at 13:17, Benjamin GAIGNARD
<benjamin.gaignard@st.com> wrote:
>
>
>
> On 5/27/20 12:09 PM, Valentin Schneider wrote:
> > Hi Benjamin,
> >
> > On 26/05/20 16:16, Benjamin Gaignard wrote:
> >> A first round [1] of discussions and suggestions have already be done on
> >> this series but without found a solution to the problem. I resend it to
> >> progress on this topic.
> >>
> > Apologies for sleeping on that previous thread.
> >
> > So what had been suggested over there was to use uclamp to boost the
> > frequency of the handling thread; however if you use threaded IRQs you
> > get RT threads, which already get the max frequency by default (at least
> > with schedutil).
> >
> > Does that not work for you, and if so, why?
> That doesn't work because almost everything is done by the hardware blocks
> without charge the CPU so the thread isn't running. I have done the
> tests with schedutil
> and ondemand scheduler (which is the one I'm targeting). I have no
> issues when using
> performance scheduler because it always keep the highest frequencies.
IMHO, the only way to ensure a min frequency for anything else than a
thread is to use freq_qos_add_request() just like cpufreq cooling
device but for the opposite QoS. This can be applied only on the
frequency domain of the CPU which handles the interrupt.
Have you also checked the wakeup latency of your idle state ?
>
>
> >
> >> When start streaming from the sensor the CPU load could remain very low
> >> because almost all the capture pipeline is done in hardware (i.e. without
> >> using the CPU) and let believe to cpufreq governor that it could use lower
> >> frequencies. If the governor decides to use a too low frequency that
> >> becomes a problem when we need to acknowledge the interrupt during the
> >> blanking time.
> >> The delay to ack the interrupt and perform all the other actions before
> >> the next frame is very short and doesn't allow to the cpufreq governor to
> >> provide the required burst of power. That led to drop the half of the frames.
> >>
> >> To avoid this problem, DCMI driver informs the cpufreq governors by adding
> >> a cpufreq minimum load QoS resquest.
> >>
> >> Benjamin
> >>
> >> [1] https://lkml.org/lkml/2020/4/24/360
> >>
> >> Benjamin Gaignard (3):
> >> PM: QoS: Introduce cpufreq minimum load QoS
> >> cpufreq: governor: Use minimum load QoS
> >> media: stm32-dcmi: Inform cpufreq governors about cpu load needs
> >>
> >> drivers/cpufreq/cpufreq_governor.c | 5 +
> >> drivers/media/platform/stm32/stm32-dcmi.c | 8 ++
> >> include/linux/pm_qos.h | 12 ++
> >> kernel/power/qos.c | 213 ++++++++++++++++++++++++++++++
> >> 4 files changed, 238 insertions(+)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ARM: asm: Rewrite get_thread_info using BIC
From: Linus Walleij @ 2020-05-27 12:22 UTC (permalink / raw)
To: Russell King; +Cc: Linus Walleij, Ard Biesheuvel, linux-arm-kernel
By using two BIC instructions we can replace the ARM/thumb
split instructions with something that works on either
and also save one instruction.
Based on code from proc-macros.S and an idea from Ard
Biesheuvel.
We need to include <linux/const.h> to expand the
THREAD_SIZE definition properly in the preprocessor.
Suggested-by: Russell King <linux@armlinux.org.uk>
Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
arch/arm/include/asm/assembler.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 99929122dad7..f218e8cf7f88 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -17,6 +17,7 @@
#error "Only include this from assembly code"
#endif
+#include <linux/const.h>
#include <asm/ptrace.h>
#include <asm/domain.h>
#include <asm/opcodes-virt.h>
@@ -203,10 +204,8 @@
* Get current thread_info.
*/
.macro get_thread_info, rd
- ARM( mov \rd, sp, lsr #THREAD_SIZE_ORDER + PAGE_SHIFT )
- THUMB( mov \rd, sp )
- THUMB( lsr \rd, \rd, #THREAD_SIZE_ORDER + PAGE_SHIFT )
- mov \rd, \rd, lsl #THREAD_SIZE_ORDER + PAGE_SHIFT
+ bic \rd, sp, #(THREAD_SIZE - 1) & ~63
+ bic \rd, \rd, #63
.endm
/*
--
2.25.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 V3] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn()
From: Will Deacon @ 2020-05-27 12:25 UTC (permalink / raw)
To: linux-arm-kernel, Anshuman Khandual
Cc: mark.rutland, suzuki.poulose, catalin.marinas, linux-kernel,
Mark Brown, Will Deacon
In-Reply-To: <1590573876-19120-1-git-send-email-anshuman.khandual@arm.com>
On Wed, 27 May 2020 15:34:36 +0530, Anshuman Khandual wrote:
> There is no way to proceed when requested register could not be searched in
> arm64_ftr_reg[]. Requesting for a non present register would be an error as
> well. Hence lets just WARN_ON() when search fails in get_arm64_ftr_reg()
> rather than checking for return value and doing a BUG_ON() instead in some
> individual callers. But there are also caller instances that dont error out
> when register search fails. Add a new helper get_arm64_ftr_reg_nowarn() for
> such cases.
Applied to arm64 (for-next/cpufeature), thanks!
[1/1] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn()
https://git.kernel.org/arm64/c/3577dd37c703
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
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] thermal: imx8mm: Add get_trend ops
From: Anson Huang @ 2020-05-27 12:26 UTC (permalink / raw)
To: Daniel Lezcano, rui.zhang@intel.com, amit.kucheria@verdurent.com,
shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <DB3PR0402MB391695A412B26134060D1D1BF5B30@DB3PR0402MB3916.eurprd04.prod.outlook.com>
Hi, Daniel
> Subject: RE: [PATCH] thermal: imx8mm: Add get_trend ops
>
> Hi, Daniel
>
> > Subject: Re: [PATCH] thermal: imx8mm: Add get_trend ops
> >
> > On 25/05/2020 04:46, Anson Huang wrote:
> > > Hi, Daniel
> >
> > [ ... ]
> >
> > > I tried modifying the min/max to '2' in cooling map, it works that
> > > whenever cooling action is needed, the max cooling action will be
> > > applied. But I also noticed some behaviors which NOT as expected:
> > >
After looking further into the min/max setting in cooling map, it looks like NOT
suitable for our i.MX platforms, although OPP table is defined in DT, but the OPP
table is a full list of all available set points, and chips with different fuse settings
will ONLY enable some of set points in the OPP table, that introduces the trouble
of calculating the max state of cpufreq cooling, for example, on i.MX8MM, there are
3 set points defined in OPP table, but if the chip is with speed_grading fuse set to
1.6GHz, then ONLY 1.2GHz/1.6GHz are available for cpufreq, so the real max state
for cpufreq cooling is '1' actually, so how do I handle such scenario?
If thermal_zone_bind_cooling_device() can support parsing other macro new definition
like 'THERMAL_MAX_STATE' in DT, then in thermal_core.c, it can get real max state via
cdev->ops->get_max_state(cdev, &max_state) and set to lower/upper state, that will help
a lot for the case of our i.MX platforms. Do you have any suggestion?
122 opp-1200000000 {
123 opp-hz = /bits/ 64 <1200000000>;
124 opp-microvolt = <850000>;
125 opp-supported-hw = <0xe>, <0x7>;
126 clock-latency-ns = <150000>;
127 opp-suspend;
128 };
129
130 opp-1600000000 {
131 opp-hz = /bits/ 64 <1600000000>;
132 opp-microvolt = <900000>;
133 opp-supported-hw = <0xc>, <0x7>;
134 clock-latency-ns = <150000>;
135 opp-suspend;
136 };
137
138 opp-1800000000 {
139 opp-hz = /bits/ 64 <1800000000>;
140 opp-microvolt = <1000000>;
141 opp-supported-hw = <0x8>, <0x3>;
142 clock-latency-ns = <150000>;
143 opp-suspend;
144 };
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 06/11] net: ethernet: mtk-star-emac: new driver
From: Arnd Bergmann @ 2020-05-27 12:28 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Edwin Peer, devicetree, Stephane Le Provost, Bartosz Golaszewski,
netdev, Sean Wang, Linux Kernel Mailing List, Pedro Tsai,
Mark Lee, Fabien Parent, Rob Herring,
moderated list:ARM/Mediatek SoC..., clang-built-linux,
Andrew Perepech, John Crispin, Matthias Brugger, Jakub Kicinski,
Nathan Chancellor, David S . Miller, Linux ARM, Heiner Kallweit
In-Reply-To: <CAMRc=Md1w_6+dU9gCwiiB5R+dMcYMPFLPrA++RBkKp5zaY6Riw@mail.gmail.com>
On Wed, May 27, 2020 at 1:49 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> śr., 27 maj 2020 o 13:33 Arnd Bergmann <arnd@arndb.de> napisał(a):
> >
> > On Wed, May 27, 2020 at 10:46 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > Thanks for reporting this! I have a fix ready and will send it shortly.
> >
> > I already have a workaround for this bug as well as another one
> > in my tree that I'll send later today after some more testing.
> >
> > Feel free to wait for that, or just ignore mine if you already have a fix.
> >
>
> I already posted a fix[1]. Sorry for omitting you, but somehow your
> name didn't pop up in get_maintainers.pl.
I'm not a maintainer for this, I just do a lot of build fixes on the side,
as I verify the stuff that I merge myself ;-)
> [1] https://lkml.org/lkml/2020/5/27/378
Ok, perfect, that is indeed the correct fix and mine was wrong. I'll
just send a fix for the other bug (unused-function warning) then.
Arnd
_______________________________________________
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 v2 1/3] arm64: perf: Add support caps in sysfs
From: Shaokun Zhang @ 2020-05-27 12:31 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Catalin Marinas, Will Deacon
In-Reply-To: <1588652200-12341-1-git-send-email-zhangshaokun@hisilicon.com>
Hi Will,
A gentle ping, or shall I send another version to fix [PATCH 3/3] mistake?
Thanks,
Shaokun
On 2020/5/5 12:16, Shaokun Zhang wrote:
> ARMv8.4-PMU introduces the PMMIR_EL1 registers and some new PMU events,
> like STALL_SLOT etc, are related to it. Let's add a caps directory to
> /sys/bus/event_source/devices/armv8_pmuv3_0/ and support slots from
> PMMIR_EL1 registers in this entry. The user programs can get the slots
> from sysfs directly.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
> Hi Will,
>
> I send another patch[3/3] arm64: perf: Correct the event index in sysfs
> meanwhile because it is dependent on patch2.
>
> ChangeLog in v2:
> * Add caps entry in sysfs
> * Fix the PMU events typos
> * Add one new patch to correct event ID in sysfs
>
> arch/arm64/include/asm/sysreg.h | 2 +
> arch/arm64/kernel/perf_event.c | 87 +++++++++++++++++++++++++++++++----------
> include/linux/perf/arm_pmu.h | 1 +
> 3 files changed, 69 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index c4ac0ac25a00..aa14083de466 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -314,6 +314,8 @@
> #define SYS_PMINTENSET_EL1 sys_reg(3, 0, 9, 14, 1)
> #define SYS_PMINTENCLR_EL1 sys_reg(3, 0, 9, 14, 2)
>
> +#define SYS_PMMIR_EL1 sys_reg(3, 0, 9, 14, 6)
> +
> #define SYS_MAIR_EL1 sys_reg(3, 0, 10, 2, 0)
> #define SYS_AMAIR_EL1 sys_reg(3, 0, 10, 3, 0)
>
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 4d7879484cec..5f2ac87e4b91 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -277,6 +277,51 @@ static struct attribute_group armv8_pmuv3_format_attr_group = {
> .attrs = armv8_pmuv3_format_attrs,
> };
>
> +static inline int armv8pmu_get_pmu_version(void)
> +{
> + int pmuver;
> + u64 dfr0;
> +
> + dfr0 = read_sysreg(id_aa64dfr0_el1);
> + pmuver = cpuid_feature_extract_unsigned_field(dfr0,
> + ID_AA64DFR0_PMUVER_SHIFT);
> +
> + return pmuver;
> +}
> +
> +static umode_t
> +armv8pmu_caps_attr_is_visible(struct kobject *kobj, struct attribute *attr,
> + int unused)
> +{
> + int pmuver = armv8pmu_get_pmu_version();
> +
> + if (pmuver >= ID_AA64DFR0_PMUVER_8_4)
> + return attr->mode;
> +
> + return 0;
> +}
> +
> +static ssize_t slots_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + int slots = read_sysreg_s(SYS_PMMIR_EL1) & 0xFF;
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", slots);
> +}
> +
> +static DEVICE_ATTR_RO(slots);
> +
> +static struct attribute *armv8_pmuv3_caps_attrs[] = {
> + &dev_attr_slots.attr,
> + NULL,
> +};
> +
> +static struct attribute_group armv8_pmuv3_caps_attr_group = {
> + .name = "caps",
> + .attrs = armv8_pmuv3_caps_attrs,
> + .is_visible = armv8pmu_caps_attr_is_visible,
> +};
> +
> /*
> * Perf Events' indices
> */
> @@ -940,14 +985,11 @@ static void __armv8pmu_probe_pmu(void *info)
> {
> struct armv8pmu_probe_info *probe = info;
> struct arm_pmu *cpu_pmu = probe->pmu;
> - u64 dfr0;
> u64 pmceid_raw[2];
> u32 pmceid[2];
> int pmuver;
>
> - dfr0 = read_sysreg(id_aa64dfr0_el1);
> - pmuver = cpuid_feature_extract_unsigned_field(dfr0,
> - ID_AA64DFR0_PMUVER_SHIFT);
> + pmuver = armv8pmu_get_pmu_version();
> if (pmuver == 0xf || pmuver == 0)
> return;
>
> @@ -994,7 +1036,8 @@ static int armv8pmu_probe_pmu(struct arm_pmu *cpu_pmu)
> static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
> int (*map_event)(struct perf_event *event),
> const struct attribute_group *events,
> - const struct attribute_group *format)
> + const struct attribute_group *format,
> + const struct attribute_group *caps)
> {
> int ret = armv8pmu_probe_pmu(cpu_pmu);
> if (ret)
> @@ -1019,6 +1062,8 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
> events : &armv8_pmuv3_events_attr_group;
> cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] = format ?
> format : &armv8_pmuv3_format_attr_group;
> + cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_CAPS] = caps ?
> + caps : &armv8_pmuv3_caps_attr_group;
>
> return 0;
> }
> @@ -1026,97 +1071,97 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
> static int armv8_pmuv3_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_pmuv3",
> - armv8_pmuv3_map_event, NULL, NULL);
> + armv8_pmuv3_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_a34_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cortex_a34",
> - armv8_pmuv3_map_event, NULL, NULL);
> + armv8_pmuv3_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_a35_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cortex_a35",
> - armv8_a53_map_event, NULL, NULL);
> + armv8_a53_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_a53_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cortex_a53",
> - armv8_a53_map_event, NULL, NULL);
> + armv8_a53_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_a55_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cortex_a55",
> - armv8_pmuv3_map_event, NULL, NULL);
> + armv8_pmuv3_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_a57_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cortex_a57",
> - armv8_a57_map_event, NULL, NULL);
> + armv8_a57_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_a65_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cortex_a65",
> - armv8_pmuv3_map_event, NULL, NULL);
> + armv8_pmuv3_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_a72_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cortex_a72",
> - armv8_a57_map_event, NULL, NULL);
> + armv8_a57_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_a73_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cortex_a73",
> - armv8_a73_map_event, NULL, NULL);
> + armv8_a73_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_a75_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cortex_a75",
> - armv8_pmuv3_map_event, NULL, NULL);
> + armv8_pmuv3_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_a76_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cortex_a76",
> - armv8_pmuv3_map_event, NULL, NULL);
> + armv8_pmuv3_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_a77_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cortex_a77",
> - armv8_pmuv3_map_event, NULL, NULL);
> + armv8_pmuv3_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_e1_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_neoverse_e1",
> - armv8_pmuv3_map_event, NULL, NULL);
> + armv8_pmuv3_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_n1_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_neoverse_n1",
> - armv8_pmuv3_map_event, NULL, NULL);
> + armv8_pmuv3_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_thunder_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_cavium_thunder",
> - armv8_thunder_map_event, NULL, NULL);
> + armv8_thunder_map_event, NULL, NULL, NULL);
> }
>
> static int armv8_vulcan_pmu_init(struct arm_pmu *cpu_pmu)
> {
> return armv8_pmu_init(cpu_pmu, "armv8_brcm_vulcan",
> - armv8_vulcan_map_event, NULL, NULL);
> + armv8_vulcan_map_event, NULL, NULL, NULL);
> }
>
> static const struct of_device_id armv8_pmu_of_device_ids[] = {
> diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
> index 5b616dde9a4c..1e129b57d51a 100644
> --- a/include/linux/perf/arm_pmu.h
> +++ b/include/linux/perf/arm_pmu.h
> @@ -73,6 +73,7 @@ enum armpmu_attr_groups {
> ARMPMU_ATTR_GROUP_COMMON,
> ARMPMU_ATTR_GROUP_EVENTS,
> ARMPMU_ATTR_GROUP_FORMATS,
> + ARMPMU_ATTR_GROUP_CAPS,
> ARMPMU_NR_ATTR_GROUPS
> };
>
>
_______________________________________________
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 v2 0/4] PCI, iommu: Factor 'untrusted' check for ATS enablement
From: Joerg Roedel @ 2020-05-27 12:36 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: alex.williamson, ashok.raj, linux-pci, robin.murphy, hch, iommu,
bhelgaas, will, dwmw2, linux-arm-kernel, baolu.lu
In-Reply-To: <20200520152201.3309416-1-jean-philippe@linaro.org>
On Wed, May 20, 2020 at 05:21:59PM +0200, Jean-Philippe Brucker wrote:
> IOMMU drivers currently check themselves if a device is untrusted
> (plugged into an external-facing port) before enabling ATS. Move the
> check to drivers/pci. The only functional change should be to the AMD
> IOMMU driver. With this change all IOMMU drivers block 'Translated' PCIe
> transactions and Translation Requests from untrusted devices.
>
> Since v1 [1] I added tags, addressed comments on patches 1 and 3, and
> fixed a regression in patch 3.
>
> [1] https://lore.kernel.org/linux-iommu/20200515104359.1178606-1-jean-philippe@linaro.org/
>
> Jean-Philippe Brucker (4):
> PCI/ATS: Only enable ATS for trusted devices
> iommu/amd: Use pci_ats_supported()
> iommu/arm-smmu-v3: Use pci_ats_supported()
> iommu/vt-d: Use pci_ats_supported()
>
> include/linux/pci-ats.h | 3 +++
> drivers/iommu/amd_iommu.c | 12 ++++--------
> drivers/iommu/arm-smmu-v3.c | 20 +++++++-------------
> drivers/iommu/intel-iommu.c | 9 +++------
> drivers/pci/ats.c | 18 +++++++++++++++++-
> 5 files changed, 34 insertions(+), 28 deletions(-)
Applied, thanks.
_______________________________________________
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 0/2] drivers/iommu: Constify structs
From: Joerg Roedel @ 2020-05-27 12:39 UTC (permalink / raw)
To: Rikard Falkeborn
Cc: Wei Liu, Stephen Hemminger, linux-hyperv, Haiyang Zhang,
linux-kernel, Maxime Ripard, Chen-Yu Tsai, iommu,
K. Y. Srinivasan, linux-arm-kernel
In-Reply-To: <20200525214958.30015-1-rikard.falkeborn@gmail.com>
On Mon, May 25, 2020 at 11:49:56PM +0200, Rikard Falkeborn wrote:
> Constify some structs with function pointers to allow the compiler to
> put them in read-only memory. There is not dependency between the
> patches.
>
> Rikard Falkeborn (2):
> iommu/hyper-v: Constify hyperv_ir_domain_ops
> iommu/sun50i: Constify sun50i_iommu_ops
>
> drivers/iommu/hyperv-iommu.c | 2 +-
> drivers/iommu/sun50i-iommu.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Applied both, thanks.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
From: Benjamin GAIGNARD @ 2020-05-27 12:48 UTC (permalink / raw)
To: Vincent Guittot
Cc: len.brown@intel.com, Alexandre TORGUE, linux-pm@vger.kernel.org,
viresh.kumar@linaro.org, pavel@ucw.cz, rjw@rjwysocki.net,
linux-kernel@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
mcoquelin.stm32@gmail.com, Hugues FRUCHET, mchehab@kernel.org,
Valentin Schneider, linux-arm-kernel@lists.infradead.org,
linux-media@vger.kernel.org
In-Reply-To: <CAKfTPtBt6Ju-CnETnn6_FkgR0CAJ+jYnySz9OHP9X2hmxWHM7w@mail.gmail.com>
On 5/27/20 2:22 PM, Vincent Guittot wrote:
> On Wed, 27 May 2020 at 13:17, Benjamin GAIGNARD
> <benjamin.gaignard@st.com> wrote:
>>
>>
>> On 5/27/20 12:09 PM, Valentin Schneider wrote:
>>> Hi Benjamin,
>>>
>>> On 26/05/20 16:16, Benjamin Gaignard wrote:
>>>> A first round [1] of discussions and suggestions have already be done on
>>>> this series but without found a solution to the problem. I resend it to
>>>> progress on this topic.
>>>>
>>> Apologies for sleeping on that previous thread.
>>>
>>> So what had been suggested over there was to use uclamp to boost the
>>> frequency of the handling thread; however if you use threaded IRQs you
>>> get RT threads, which already get the max frequency by default (at least
>>> with schedutil).
>>>
>>> Does that not work for you, and if so, why?
>> That doesn't work because almost everything is done by the hardware blocks
>> without charge the CPU so the thread isn't running. I have done the
>> tests with schedutil
>> and ondemand scheduler (which is the one I'm targeting). I have no
>> issues when using
>> performance scheduler because it always keep the highest frequencies.
> IMHO, the only way to ensure a min frequency for anything else than a
> thread is to use freq_qos_add_request() just like cpufreq cooling
> device but for the opposite QoS. This can be applied only on the
> frequency domain of the CPU which handles the interrupt.
I will give a try with this idea.
Thanks.
> Have you also checked the wakeup latency of your idle state ?
It just could go in WFI so latency should be minimal.
>
>>
>>>> When start streaming from the sensor the CPU load could remain very low
>>>> because almost all the capture pipeline is done in hardware (i.e. without
>>>> using the CPU) and let believe to cpufreq governor that it could use lower
>>>> frequencies. If the governor decides to use a too low frequency that
>>>> becomes a problem when we need to acknowledge the interrupt during the
>>>> blanking time.
>>>> The delay to ack the interrupt and perform all the other actions before
>>>> the next frame is very short and doesn't allow to the cpufreq governor to
>>>> provide the required burst of power. That led to drop the half of the frames.
>>>>
>>>> To avoid this problem, DCMI driver informs the cpufreq governors by adding
>>>> a cpufreq minimum load QoS resquest.
>>>>
>>>> Benjamin
>>>>
>>>> [1] https://lkml.org/lkml/2020/4/24/360
>>>>
>>>> Benjamin Gaignard (3):
>>>> PM: QoS: Introduce cpufreq minimum load QoS
>>>> cpufreq: governor: Use minimum load QoS
>>>> media: stm32-dcmi: Inform cpufreq governors about cpu load needs
>>>>
>>>> drivers/cpufreq/cpufreq_governor.c | 5 +
>>>> drivers/media/platform/stm32/stm32-dcmi.c | 8 ++
>>>> include/linux/pm_qos.h | 12 ++
>>>> kernel/power/qos.c | 213 ++++++++++++++++++++++++++++++
>>>> 4 files changed, 238 insertions(+)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: High interrupt latency with low power idle mode on i.MX6
From: Schrempf Frieder @ 2020-05-27 12:50 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Kate Stewart, linux-pm@vger.kernel.org, Enrico Weigelt,
Sascha Hauer, Daniel Lezcano, Rafael J. Wysocki,
linux-kernel@vger.kernel.org, Thomas Gleixner, Shawn Guo,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20200527115347.GL1551@shell.armlinux.org.uk>
On 27.05.20 13:53, Russell King - ARM Linux admin wrote:
> On Wed, May 27, 2020 at 10:39:12AM +0000, Schrempf Frieder wrote:
>> Hi,
>>
>> on our i.MX6UL/ULL boards running mainline kernels, we see an issue with
>> RS485 collisions on the bus. These are caused by the resetting of the
>> RTS signal being delayed after each transmission. The TXDC interrupt
>> takes several milliseconds to trigger and the slave on the bus already
>> starts to send a reply in the meantime.
>>
>> We found out that these delays only happen when the CPU is in "low power
>> idle" mode (ARM power off). When we disable cpuidle state 2 or put some
>> background load on the CPU everything works fine and the delays are gone.
>>
>> echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/state2/disable
>>
>> It seems like also other interfaces (I2C, etc.) might be affected by
>> these increased latencies, we haven't investigated this more closely,
>> though.
>>
>> We currently apply a patch to our kernel, that disables low power idle
>> mode by default, but I'm wondering if there's a way to fix this
>> properly? Any ideas?
>
> Let's examine a basic fact about power management:
>
> The deeper PM modes that the system enters, the higher the latency to
> resume operation.
>
> So, I'm not surprised that you have higher latency when you allow the
> system to enter lower power modes. Does that mean that the kernel
> should not permit entering lower power modes - no, it's policy and
> application dependent.
>
> If the hardware is designed to use software to manage the RTS signal
> to control the RS485 receiver, then I'm afraid that your report really
> does not surprise me - throwing that at software to manage is a really
> stupid idea, but it seems lots of people do this. I've held this view
> since I worked on a safety critical system that used RS485 back in the
> 1990s (London Underground Jubilee Line Extension public address system.)
>
> So, what we have here is several things that come together to create a
> problem:
>
> 1) higher power savings produce higher latency to resume from
> 2) lack of hardware support for RS485 half duplex communication needing
> software support
> 3) an application that makes use of RS485 half duplex communication
> without disabling the higher latency power saving modes
>
> The question is, who should disable those higher latency power saving
> modes - the kernel, or userspace?
>
> The kernel knows whether it needs to provide software control of the
> RTS signal or not, but the kernel does not know the maximum permissible
> latency (which is application specific.) So, the kernel doesn't have
> all the information it needs. However, there is a QoS subsystem which
> may help you.
>
> There's also tweaks available via
> /sys/devices/system/cpu/cpu*/power/pm_qos_resume_latency_us
>
> which can be poked to configure the latency that is required, and will
> prevent the deeper PM states being entered.
Thanks for the detailed explanation. This all makes perfect sense to me.
I will keep in mind that we need to consider this aspect of power saving
vs. latency when designing systems and also that we need to provide the
information for the kernel to decide which of the two is more important.
Also thanks for pointing out the QoS subsystem. I'm not quite sure if it
would work for us to use pm_qos_resume_latency_us in our specific case.
The actual latency we observe is something like 2 to 3 milliseconds
longer with low power idle than without, but the exit_latency for low
power idle specified in the cpuidle driver is only 300 us.
So as far as I can see with this difference even if we would set
pm_qos_resume_latency_us to 1000 us (which should be fast enough for the
RS485 to work properly), the low power idle wouldn't be disabled.
It's rather this discrepancy between the latency set in the driver and
what we see in reality which makes me wonder if there's something I'm
missing.
_______________________________________________
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 for-5.8 2/2] usb: dwc3: meson-g12a: fix USB2 PHY initialization on G12A and A1 SoCs
From: Felipe Balbi @ 2020-05-27 12:56 UTC (permalink / raw)
To: Greg KH, Neil Armstrong
Cc: hanjie.lin, kernelci.org bot, Martin Blumenstingl, linux-usb,
linux-kernel, yue.wang, linux-amlogic, linux-arm-kernel
In-Reply-To: <20200527085315.GA168054@kroah.com>
[-- Attachment #1.1: Type: text/plain, Size: 2166 bytes --]
Greg KH <gregkh@linuxfoundation.org> writes:
> On Wed, May 27, 2020 at 10:17:31AM +0200, Neil Armstrong wrote:
>> Hi Martin,
>>
>> On 26/05/2020 22:29, Martin Blumenstingl wrote:
>> > dwc3_meson_g12a_usb2_init_phy() crashes with NULL pointer on an SM1
>> > board (which uses the same USB setup as G12A) dereference as reported
>> > by the Kernel CI bot. This is because of the following call flow:
>> > dwc3_meson_g12a_probe
>> > priv->drvdata->setup_regmaps
>> > dwc3_meson_g12a_setup_regmaps
>> > priv->usb2_ports is still 0 so priv->u2p_regmap[i] will be NULL
>> > dwc3_meson_g12a_get_phys
>> > initializes priv->usb2_ports
>> > priv->drvdata->usb_init
>> > dwc3_meson_g12a_usb_init
>> > dwc3_meson_g12a_usb_init_glue
>> > dwc3_meson_g12a_usb2_init
>> > priv->drvdata->usb2_init_phy
>> > dwc3_meson_g12a_usb2_init_phy
>> > dereferences priv->u2p_regmap[i]
>> >
>> > Call priv->drvdata->setup_regmaps only after dwc3_meson_g12a_get_phys so
>> > priv->usb2_ports is initialized and the regmaps will be set up
>> > correctly. This fixes the NULL dereference later on.
>> >
>> > Fixes: 013af227f58a97 ("usb: dwc3: meson-g12a: handle the phy and glue registers separately")
>> > Reported-by: "kernelci.org bot" <bot@kernelci.org>
>> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> > ---
>> > drivers/usb/dwc3/dwc3-meson-g12a.c | 8 ++++----
>> > 1 file changed, 4 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
>> > index ce5388338389..1f7f4d88ed9d 100644
>> > --- a/drivers/usb/dwc3/dwc3-meson-g12a.c
>> > +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
>>
>> [...]
>>
>> Fixes regression reported at [1] on SEI510 board based on Amlogic G12A.
>>
>> Felipe, Greg, can this be queued on uxb-next for 5.8 ?
>>
>> Acked-by: Neil Armstrong <narmstron@baylibre.com>
>
> I can take this and patch 1/2 here if Felipe acks them.
Sure thing, Greg. Thanks.
Acked-by: Felipe Balbi <balbi@kernel.org>
--
balbi
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 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: [RFC RESEND 0/3] Introduce cpufreq minimum load QoS
From: Benjamin GAIGNARD @ 2020-05-27 13:11 UTC (permalink / raw)
To: Valentin Schneider
Cc: len.brown@intel.com, Alexandre TORGUE, linux-pm@vger.kernel.org,
viresh.kumar@linaro.org, pavel@ucw.cz, rjw@rjwysocki.net,
linux-kernel@vger.kernel.org, mcoquelin.stm32@gmail.com,
Hugues FRUCHET, mchehab@kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <jhjwo4xinhb.mognet@arm.com>
On 5/27/20 2:14 PM, Valentin Schneider wrote:
> On 27/05/20 12:17, Benjamin GAIGNARD wrote:
>> On 5/27/20 12:09 PM, Valentin Schneider wrote:
>>> Hi Benjamin,
>>>
>>> On 26/05/20 16:16, Benjamin Gaignard wrote:
>>>> A first round [1] of discussions and suggestions have already be done on
>>>> this series but without found a solution to the problem. I resend it to
>>>> progress on this topic.
>>>>
>>> Apologies for sleeping on that previous thread.
>>>
>>> So what had been suggested over there was to use uclamp to boost the
>>> frequency of the handling thread; however if you use threaded IRQs you
>>> get RT threads, which already get the max frequency by default (at least
>>> with schedutil).
>>>
>>> Does that not work for you, and if so, why?
>> That doesn't work because almost everything is done by the hardware blocks
>> without charge the CPU so the thread isn't running.
> I'm not sure I follow; the frequency of the CPU doesn't matter while
> your hardware blocks are spinning, right? AIUI what matters is running
> your interrupt handler / action at max freq, which you get if you use
> threaded IRQs and schedutil.
Yes but not limited to schedutil.
Given the latency needed to change of frequencies I think it could
already too late
to change the CPU frequency when handling the threaded interrupt.
>
> I think it would help if you could clarify which tasks / parts of your
> pipeline you need running at high frequencies. The point is that setting
> a QoS request affects all tasks, whereas we could be smarter and only
> boost the required tasks.
What make us drop frames is that the threaded IRQ is scheduled too late.
The not thread part of the interrupt handler where we clear the
interrupt flags
is going fine but the thread part not.
>
>> I have done the
>> tests with schedutil
>> and ondemand scheduler (which is the one I'm targeting). I have no
>> issues when using
>> performance scheduler because it always keep the highest frequencies.
>>
>>
>>>> When start streaming from the sensor the CPU load could remain very low
>>>> because almost all the capture pipeline is done in hardware (i.e. without
>>>> using the CPU) and let believe to cpufreq governor that it could use lower
>>>> frequencies. If the governor decides to use a too low frequency that
>>>> becomes a problem when we need to acknowledge the interrupt during the
>>>> blanking time.
>>>> The delay to ack the interrupt and perform all the other actions before
>>>> the next frame is very short and doesn't allow to the cpufreq governor to
>>>> provide the required burst of power. That led to drop the half of the frames.
>>>>
>>>> To avoid this problem, DCMI driver informs the cpufreq governors by adding
>>>> a cpufreq minimum load QoS resquest.
>>>>
>>>> Benjamin
>>>>
>>>> [1] https://lkml.org/lkml/2020/4/24/360
>>>>
>>>> Benjamin Gaignard (3):
>>>> PM: QoS: Introduce cpufreq minimum load QoS
>>>> cpufreq: governor: Use minimum load QoS
>>>> media: stm32-dcmi: Inform cpufreq governors about cpu load needs
>>>>
>>>> drivers/cpufreq/cpufreq_governor.c | 5 +
>>>> drivers/media/platform/stm32/stm32-dcmi.c | 8 ++
>>>> include/linux/pm_qos.h | 12 ++
>>>> kernel/power/qos.c | 213 ++++++++++++++++++++++++++++++
>>>> 4 files changed, 238 insertions(+)
_______________________________________________
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 v2] ARM: mm: Simplify act_mm macro
From: Russell King - ARM Linux admin @ 2020-05-27 13:13 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-arm-kernel
In-Reply-To: <20200527115618.92792-1-linus.walleij@linaro.org>
On Wed, May 27, 2020 at 01:56:18PM +0200, Linus Walleij wrote:
> The act_mm assembly macro is actually partly reimplementing
> get_thread_info so let's just use that.
>
> Suggested-by: Russell King <linux@armlinux.org.uk>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Accidentally removed the index to the active task which
> is what we want here.
Yep, this version is purrfec. Thanks.
> ---
> arch/arm/mm/proc-macros.S | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index 60ac7c5999a9..e2c743aa2eb2 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -5,7 +5,6 @@
> * VMA_VM_FLAGS
> * VM_EXEC
> */
> -#include <linux/const.h>
> #include <asm/asm-offsets.h>
> #include <asm/thread_info.h>
>
> @@ -31,8 +30,7 @@
> * act_mm - get current->active_mm
> */
> .macro act_mm, rd
> - bic \rd, sp, #(THREAD_SIZE - 1) & ~63
> - bic \rd, \rd, #63
> + get_thread_info \rd
> ldr \rd, [\rd, #TI_TASK]
> .if (TSK_ACTIVE_MM > IMM12_MASK)
> add \rd, \rd, #TSK_ACTIVE_MM & ~IMM12_MASK
> --
> 2.25.4
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
_______________________________________________
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] ARM: asm: Rewrite get_thread_info using BIC
From: Russell King - ARM Linux admin @ 2020-05-27 13:13 UTC (permalink / raw)
To: Linus Walleij; +Cc: Ard Biesheuvel, linux-arm-kernel
In-Reply-To: <20200527122201.124090-1-linus.walleij@linaro.org>
On Wed, May 27, 2020 at 02:22:01PM +0200, Linus Walleij wrote:
> By using two BIC instructions we can replace the ARM/thumb
> split instructions with something that works on either
> and also save one instruction.
>
> Based on code from proc-macros.S and an idea from Ard
> Biesheuvel.
>
> We need to include <linux/const.h> to expand the
> THREAD_SIZE definition properly in the preprocessor.
>
> Suggested-by: Russell King <linux@armlinux.org.uk>
> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Looks good to me, thanks.
> ---
> arch/arm/include/asm/assembler.h | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index 99929122dad7..f218e8cf7f88 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -17,6 +17,7 @@
> #error "Only include this from assembly code"
> #endif
>
> +#include <linux/const.h>
> #include <asm/ptrace.h>
> #include <asm/domain.h>
> #include <asm/opcodes-virt.h>
> @@ -203,10 +204,8 @@
> * Get current thread_info.
> */
> .macro get_thread_info, rd
> - ARM( mov \rd, sp, lsr #THREAD_SIZE_ORDER + PAGE_SHIFT )
> - THUMB( mov \rd, sp )
> - THUMB( lsr \rd, \rd, #THREAD_SIZE_ORDER + PAGE_SHIFT )
> - mov \rd, \rd, lsl #THREAD_SIZE_ORDER + PAGE_SHIFT
> + bic \rd, sp, #(THREAD_SIZE - 1) & ~63
> + bic \rd, \rd, #63
> .endm
>
> /*
> --
> 2.25.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
_______________________________________________
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] Statsfs: a new ram-based file system for Linux kernel statistics
From: Emanuele Giuseppe Esposito @ 2020-05-27 13:14 UTC (permalink / raw)
To: Jakub Kicinski
Cc: linux-s390, kvm, linux-doc, netdev, Emanuele Giuseppe Esposito,
linux-kernel, kvm-ppc, Jonathan Adams, Christian Borntraeger,
Andrew Lunn, Alexander Viro, David Rientjes, linux-fsdevel,
Paolo Bonzini, linux-mips, linuxppc-dev, linux-arm-kernel,
Jim Mattson
In-Reply-To: <20200526153128.448bfb43@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
>>
>> The file system is mounted on /sys/kernel/stats and would be already used
>> by kvm. Statsfs was initially introduced by Paolo Bonzini [1].
>
> What's the direct motivation for this work? Moving KVM stats out of
> debugfs?
There's many reasons: one of these is not using debugfs for statistics,
but also (and mainly) to try and have a single tool that automatically
takes care and displays them, instead of leaving each subsystem "on its
own".
Sure, everyone gathers and processes stats in different ways, and the
aim of this tool is to hopefully be extensible enough to cover all needs.
> In my experience stats belong in the API used for creating/enumerating
> objects, statsfs sounds like going in the exact opposite direction -
> creating a parallel structure / hierarchy for exposing stats.
I know
> nothing about KVM but are you sure all the info that has to be exposed
> will be stats?I don't understand, what do you mean here?
>
> In case of networking we have the basic stats in sysfs, under the
> netdevice's kobject. But since we're not using sysfs much any more
> for config, new stats are added in netlink APIs. Again - same APIs
> used for enumeration and config.
I don't really know a lot about the networking subsystem, and as it was
pointed out in another email on patch 7 by Andrew, networking needs to
atomically gather and display statistics in order to make them
consistent, and currently this is not supported by stats_fs but could be
added in future.
In addition, right now it won't work properly if the networking
namespaces are enabled. That is another issue to take into
consideration. That's also why I marked patch 7 as "not for merge"
Regarding the config, as I said the idea is to gather multiple
subsystems' statistics, therefore there wouldn't be a single
configuration method like in netlink.
For example in kvm there are file descriptors for configuration, and
creating them requires no privilege, contrary to the network interfaces.
Thank you,
Emanuele
_______________________________________________
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 v13 1/2] media: dt-bindings: media: xilinx: Add Xilinx MIPI CSI-2 Rx Subsystem
From: Laurent Pinchart @ 2020-05-27 13:23 UTC (permalink / raw)
To: Vishal Sagar
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, Jacopo Mondi,
Dinesh Kumar, Hyun Kwon, Sandip Kothari,
linux-kernel@vger.kernel.org, robh+dt@kernel.org, Michal Simek,
Luca Ceresoli, hans.verkuil@cisco.com, mchehab@kernel.org,
Rob Herring, linux-arm-kernel@lists.infradead.org,
linux-media@vger.kernel.org
In-Reply-To: <DM6PR02MB68762DFB8F24E485B9B302B6A7B10@DM6PR02MB6876.namprd02.prod.outlook.com>
Hi Vishal,
On Wed, May 27, 2020 at 11:53:01AM +0000, Vishal Sagar wrote:
> On Sunday, May 24, 2020 7:32 AM, Laurent Pinchart wrote:
> > On Tue, May 12, 2020 at 08:49:46PM +0530, Vishal Sagar wrote:
> > > Add bindings documentation for Xilinx MIPI CSI-2 Rx Subsystem.
> > >
> > > The Xilinx MIPI CSI-2 Rx Subsystem consists of a CSI-2 Rx controller,
> > > a D-PHY in Rx mode and a Video Format Bridge.
> > >
> > > Signed-off-by: Vishal Sagar <vishal.sagar@xilinx.com>
> > > Reviewed-by: Hyun Kwon <hyun.kwon@xilinx.com>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > v13
> > > - Based on Laurent's suggestions
> > > - Fixed the datatypes values as minimum and maximum
> > > - condition added for en-vcx property
> > >
> > > v12
> > > - Moved to yaml format
> > > - Update CSI-2 and D-PHY
> > > - Mention that bindings for D-PHY not here
> > > - reset -> video-reset
> > >
> > > v11
> > > - Modify compatible string from 4.0 to 5.0
> > >
> > > v10
> > > - No changes
> > >
> > > v9
> > > - Fix xlnx,vfb description.
> > > - s/Optional/Required endpoint property.
> > > - Move data-lanes description from Ports to endpoint property section.
> > >
> > > v8
> > > - Added reset-gpios optional property to assert video_aresetn
> > >
> > > v7
> > > - Removed the control name from dt bindings
> > > - Updated the example dt node name to csi2rx
> > >
> > > v6
> > > - Added "control" after V4L2_CID_XILINX_MIPICSISS_ACT_LANES as
> > > suggested by Luca
> > > - Added reviewed by Rob Herring
> > >
> > > v5
> > > - Incorporated comments by Luca Cersoli
> > > - Removed DPHY clock from description and example
> > > - Removed bayer pattern from device tree MIPI CSI IP
> > > doesn't deal with bayer pattern.
> > >
> > > v4
> > > - Added reviewed by Hyun Kwon
> > >
> > > v3
> > > - removed interrupt parent as suggested by Rob
> > > - removed dphy clock
> > > - moved vfb to optional properties
> > > - Added required and optional port properties section
> > > - Added endpoint property section
> > >
> > > v2
> > > - updated the compatible string to latest version supported
> > > - removed DPHY related parameters
> > > - added CSI v2.0 related property (including VCX for supporting upto 16
> > > virtual channels).
> > > - modified csi-pxl-format from string to unsigned int type where the value
> > > is as per the CSI specification
> > > - Defined port 0 and port 1 as sink and source ports.
> > > - Removed max-lanes property as suggested by Rob and Sakari
> > > .../bindings/media/xilinx/xlnx,csi2rxss.yaml | 226 ++++++++++++++++++
> > > 1 file changed, 226 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> > > b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> > > new file mode 100644
> > > index 000000000000..b0885f461785
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yam
> > > +++ l
> > > @@ -0,0 +1,226 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/xilinx/xlnx,csi2rxss.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Xilinx MIPI CSI-2 Receiver Subsystem
> > > +
> > > +maintainers:
> > > + - Vishal Sagar <vishal.sagar@xilinx.com>
> > > +
> > > +description: |
> > > + The Xilinx MIPI CSI-2 Receiver Subsystem is used to capture MIPI
> > > +CSI-2
> > > + traffic from compliant camera sensors and send the output as AXI4
> > > +Stream
> > > + video data for image processing.
> > > + The subsystem consists of a MIPI D-PHY in slave mode which captures
> > > +the
> > > + data packets. This is passed along the MIPI CSI-2 Rx IP which
> > > +extracts the
> > > + packet data. The optional Video Format Bridge (VFB) converts this
> > > +data to
> > > + AXI4 Stream video data.
> > > + For more details, please refer to PG232 Xilinx MIPI CSI-2 Receiver
> > Subsystem.
> > > + Please note that this bindings includes only the MIPI CSI-2 Rx
> > > +controller
> > > + and Video Format Bridge and not D-PHY.
> > > +
> > > +properties:
> > > + compatible:
> > > + items:
> > > + - enum:
> > > + - xlnx,mipi-csi2-rx-subsystem-5.0
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + interrupts:
> > > + maxItems: 1
> > > +
> > > + clocks:
> > > + description: List of clock specifiers
> > > + items:
> > > + - description: AXI Lite clock
> > > + - description: Video clock
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: lite_aclk
> > > + - const: video_aclk
> > > +
> > > + xlnx,csi-pxl-format:
> > > + description: |
> > > + This denotes the CSI Data type selected in hw design.
> > > + Packets other than this data type (except for RAW8 and
> > > + User defined data types) will be filtered out.
> > > + Possible values are as below -
> > > + 0x1e - YUV4228B
> > > + 0x1f - YUV42210B
> > > + 0x20 - RGB444
> > > + 0x21 - RGB555
> > > + 0x22 - RGB565
> > > + 0x23 - RGB666
> > > + 0x24 - RGB888
> > > + 0x28 - RAW6
> > > + 0x29 - RAW7
> > > + 0x2a - RAW8
> > > + 0x2b - RAW10
> > > + 0x2c - RAW12
> > > + 0x2d - RAW14
> > > + 0x2e - RAW16
> > > + 0x2f - RAW20
> > > + allOf:
> > > + - $ref: /schemas/types.yaml#/definitions/uint32
> > > + - anyOf:
> > > + - minimum: 0x1e
> > > + - maximum: 0x24
> > > + - minimum: 0x28
> > > + - maximum: 0x2f
> > > +
> > > + xlnx,vfb:
> > > + type: boolean
> > > + description: Present when Video Format Bridge is enabled in IP
> > > + configuration
> > > +
> > > + xlnx,en-csi-v2-0:
> > > + type: boolean
> > > + description: Present if CSI v2 is enabled in IP configuration.
> > > +
> > > + xlnx,en-vcx:
> > > + type: boolean
> > > + description: |
> > > + When present, there are maximum 16 virtual channels, else only 4.
> > > +
> > > + xlnx,en-active-lanes:
> > > + type: boolean
> > > + description: |
> > > + Present if the number of active lanes can be re-configured at
> > > + runtime in the Protocol Configuration Register. Otherwise all lanes,
> > > + as set in IP configuration, are always active.
> > > +
> > > + video-reset-gpios:
> > > + description: Optional specifier for a GPIO that asserts video_aresetn.
> > > + maxItems: 1
> > > +
> > > + ports:
> > > + type: object
> > > +
> > > + properties:
> > > + port@0:
> > > + type: object
> > > + description: |
> > > + Input / sink port node, single endpoint describing the
> > > + CSI-2 transmitter.
> > > +
> > > + properties:
> > > + reg:
> > > + const: 0
> > > +
> > > + endpoint:
> > > + type: object
> > > +
> > > + properties:
> > > +
> > > + data-lanes:
> > > + description: |
> > > + This is required only in the sink port 0 endpoint which
> > > + connects to MIPI CSI-2 source like sensor.
> > > + The possible values are -
> > > + 1 - For 1 lane enabled in IP.
> > > + 1 2 - For 2 lanes enabled in IP.
> > > + 1 2 3 - For 3 lanes enabled in IP.
> > > + 1 2 3 4 - For 4 lanes enabled in IP.
> > > + items:
> > > + - const: 1
> > > + - const: 2
> > > + - const: 3
> > > + - const: 4
> > > +
> > > + remote-endpoint: true
> > > +
> > > + required:
> > > + - data-lanes
> > > + - remote-endpoint
> > > +
> > > + additionalProperties: false
> > > +
> > > + additionalProperties: false
> > > +
> > > + port@1:
> > > + type: object
> > > + description: |
> > > + Output / source port node, endpoint describing modules
> > > + connected the CSI-2 receiver.
> > > +
> > > + properties:
> > > +
> > > + reg:
> > > + const: 1
> > > +
> > > + endpoint:
> > > + type: object
> > > +
> > > + properties:
> > > +
> > > + remote-endpoint: true
> > > +
> > > + required:
> > > + - remote-endpoint
> > > +
> > > + additionalProperties: false
> > > +
> > > + additionalProperties: false
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - interrupts
> > > + - clocks
> > > + - clock-names
> > > + - xlnx,csi-pxl-format
> > > + - ports
> > > +
> > > +if:
> > > + not:
> > > + required:
> > > + - xlnx,en-csi-v2-0
> > > +then:
> > > + properties:
> > > + xlnx,en-vcx: false
> >
> > As I've just commented on v12, I think we should condition the xlnx,csi-pxl-
> > format property to xlnx,vfb being set. xlnx,csi-pxl-format should be removed
> > from the required properties above, and the following conditions added:
> >
> > allOf:
> > - if:
> > required:
> > - xlnx,vfb
> > then:
> > required:
> > - xlnx,csi-pxl-format
> > else:
> > properties:
> > xlnx,csi-pxl-format: false
> >
> > - if:
> > not:
> > required:
> > - xlnx,en-csi-v2-0
> > then:
> > properties:
> > xlnx,en-vcx: false
> >
> > The 'allOf' is needed as you can't have two 'if' constructs at the top level.
> >
> Thanks for sharing the explanation for this.
> Can you please share where I can get this info?
The json-schema specification is available at
https://json-schema.org/specification.html. allOf is defined in
https://json-schema.org/draft/2019-09/json-schema-core.html#allOf.
JSON schemas are expressed in JSON format, and YAML is a (more readable)
superset syntax of JSON. A YAML document contains lists and objects:
- this
- is
- a
- list
object:
can: have
properties:
that: can
be: other
objects
An object is similar to a Python dictionary, it can't have multiple
entries with the same key. So having
if:
required:
- xlnx,vfb
then:
required:
- xlnx,csi-pxl-format
else:
properties:
xlnx,csi-pxl-format: false
if:
not:
required:
- xlnx,en-csi-v2-0
then:
properties:
xlnx,en-vcx: false
at the top level is not valid, the same way that
properties:
reg:
maxItems: 1
reg:
maxItems: 1
wouldn't be valid. The allOf object has a value that is a list of
schemas:
allOf:
- schema1
- schema2
- schema3
and in this case, we use it with a if...then...else for each of the
schemas. As documented in the spec, "An instance validates successfully
against [allOf] if it validates successfully against all schemas defined
by [allOf]'s value".
allOf is also used to include sub-schemas, as explained in
Documentation/devicetree/bindings/example-schema.yaml.
vendor,int-property:
description: Vendor specific properties must have a description
# 'allOf' is the json-schema way of subclassing a schema. Here the base
# type schema is referenced and then additional constraints on the values
# are added.
allOf:
- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [2, 4, 6, 8, 10]
If this was written
vendor,int-property:
$ref: /schemas/types.yaml#/definitions/uint32
enum: [2, 4, 6, 8, 10]
we would have an issue (among other problems) if
/schemas/types.yaml#/definitions/uint32 contained an enum, as there
would be two enum properties for vendor,int-property.
> > Please however let me know if my understanding is wrong and xlnx,csi-pxl-
> > format is needed even when xlnx,vfb is not set. In that case please ignore this
> > change (but please add the ... below).
>
> Ok. I will add ... in the end.
>
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + #include <dt-bindings/gpio/gpio.h>
> > > + xcsi2rxss_1: csi2rx@a0020000 {
> > > + compatible = "xlnx,mipi-csi2-rx-subsystem-5.0";
> > > + reg = <0x0 0xa0020000 0x0 0x10000>;
> > > + interrupt-parent = <&gic>;
> > > + interrupts = <0 95 4>;
> > > + xlnx,csi-pxl-format = <0x2a>;
> > > + xlnx,vfb;
> > > + xlnx,en-active-lanes;
> > > + xlnx,en-csi-v2-0;
> > > + xlnx,en-vcx;
> > > + clock-names = "lite_aclk", "video_aclk";
> > > + clocks = <&misc_clk_0>, <&misc_clk_1>;
> > > + video-reset-gpios = <&gpio 86 GPIO_ACTIVE_LOW>;
> > > +
> > > + ports {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + port@0 {
> > > + /* Sink port */
> > > + reg = <0>;
> > > + csiss_in: endpoint {
> > > + data-lanes = <1 2 3 4>;
> > > + /* MIPI CSI-2 Camera handle */
> > > + remote-endpoint = <&camera_out>;
> > > + };
> > > + };
> > > + port@1 {
> > > + /* Source port */
> > > + reg = <1>;
> > > + csiss_out: endpoint {
> > > + remote-endpoint = <&vproc_in>;
> > > + };
> > > + };
> > > + };
> > > + };
> >
> > YAML files usually end with
> >
> > ...
> >
> > on the last line to mark the end of file.
> >
>
> Ok I will add this to the end of the file.
>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
--
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: High interrupt latency with low power idle mode on i.MX6
From: Russell King - ARM Linux admin @ 2020-05-27 13:23 UTC (permalink / raw)
To: Schrempf Frieder
Cc: Kate Stewart, linux-pm@vger.kernel.org, Enrico Weigelt,
Sascha Hauer, Daniel Lezcano, Rafael J. Wysocki,
linux-kernel@vger.kernel.org, Thomas Gleixner, Shawn Guo,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <a6bb55aa-5c47-ba7a-2f74-56da4aef4a42@kontron.de>
On Wed, May 27, 2020 at 12:50:01PM +0000, Schrempf Frieder wrote:
> On 27.05.20 13:53, Russell King - ARM Linux admin wrote:
> > On Wed, May 27, 2020 at 10:39:12AM +0000, Schrempf Frieder wrote:
> >> Hi,
> >>
> >> on our i.MX6UL/ULL boards running mainline kernels, we see an issue with
> >> RS485 collisions on the bus. These are caused by the resetting of the
> >> RTS signal being delayed after each transmission. The TXDC interrupt
> >> takes several milliseconds to trigger and the slave on the bus already
> >> starts to send a reply in the meantime.
> >>
> >> We found out that these delays only happen when the CPU is in "low power
> >> idle" mode (ARM power off). When we disable cpuidle state 2 or put some
> >> background load on the CPU everything works fine and the delays are gone.
> >>
> >> echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/state2/disable
> >>
> >> It seems like also other interfaces (I2C, etc.) might be affected by
> >> these increased latencies, we haven't investigated this more closely,
> >> though.
> >>
> >> We currently apply a patch to our kernel, that disables low power idle
> >> mode by default, but I'm wondering if there's a way to fix this
> >> properly? Any ideas?
> >
> > Let's examine a basic fact about power management:
> >
> > The deeper PM modes that the system enters, the higher the latency to
> > resume operation.
> >
> > So, I'm not surprised that you have higher latency when you allow the
> > system to enter lower power modes. Does that mean that the kernel
> > should not permit entering lower power modes - no, it's policy and
> > application dependent.
> >
> > If the hardware is designed to use software to manage the RTS signal
> > to control the RS485 receiver, then I'm afraid that your report really
> > does not surprise me - throwing that at software to manage is a really
> > stupid idea, but it seems lots of people do this. I've held this view
> > since I worked on a safety critical system that used RS485 back in the
> > 1990s (London Underground Jubilee Line Extension public address system.)
> >
> > So, what we have here is several things that come together to create a
> > problem:
> >
> > 1) higher power savings produce higher latency to resume from
> > 2) lack of hardware support for RS485 half duplex communication needing
> > software support
> > 3) an application that makes use of RS485 half duplex communication
> > without disabling the higher latency power saving modes
> >
> > The question is, who should disable those higher latency power saving
> > modes - the kernel, or userspace?
> >
> > The kernel knows whether it needs to provide software control of the
> > RTS signal or not, but the kernel does not know the maximum permissible
> > latency (which is application specific.) So, the kernel doesn't have
> > all the information it needs. However, there is a QoS subsystem which
> > may help you.
> >
> > There's also tweaks available via
> > /sys/devices/system/cpu/cpu*/power/pm_qos_resume_latency_us
> >
> > which can be poked to configure the latency that is required, and will
> > prevent the deeper PM states being entered.
>
> Thanks for the detailed explanation. This all makes perfect sense to me.
> I will keep in mind that we need to consider this aspect of power saving
> vs. latency when designing systems and also that we need to provide the
> information for the kernel to decide which of the two is more important.
>
> Also thanks for pointing out the QoS subsystem. I'm not quite sure if it
> would work for us to use pm_qos_resume_latency_us in our specific case.
> The actual latency we observe is something like 2 to 3 milliseconds
> longer with low power idle than without, but the exit_latency for low
> power idle specified in the cpuidle driver is only 300 us.
I wonder whether the exit latencies are correct in that case.
From the comments, it seems 80us is allowed for the software overhead
of entering/leaving the idle state vs 220us for the hardware.
It may be a good idea for someone to add some tracing points in there
to try and measure the minimum software latencies.
> So as far as I can see with this difference even if we would set
> pm_qos_resume_latency_us to 1000 us (which should be fast enough for the
> RS485 to work properly), the low power idle wouldn't be disabled.
>
> It's rather this discrepancy between the latency set in the driver and
> what we see in reality which makes me wonder if there's something I'm
> missing.
It's possible that there's something missing from the kernel's
estimation of the latency required for entering / exiting those
states.
There is an amount of cache flushing that is required when entering
those lower states, and I wonder if that has been accounted for.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
_______________________________________________
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 3/7] kunit: tests for stats_fs API
From: Emanuele Giuseppe Esposito @ 2020-05-27 13:26 UTC (permalink / raw)
To: Alan Maguire
Cc: linux-s390, kvm, linux-doc, netdev, kunit-dev,
Emanuele Giuseppe Esposito, linux-kernel, kvm-ppc, Jonathan Adams,
brendanhiggins, Christian Borntraeger, Alexander Viro,
linux-kselftest, David Rientjes, linux-fsdevel, Paolo Bonzini,
linux-mips, linuxppc-dev, linux-arm-kernel, Jim Mattson
In-Reply-To: <alpine.LRH.2.21.2005271054360.24819@localhost>
>> In order to run them, the kernel .config must set CONFIG_KUNIT=y
>> and a new .kunitconfig file must be created with CONFIG_STATS_FS=y
>> and CONFIG_STATS_FS_TEST=y
>>
>
> It looks like CONFIG_STATS_FS is built-in, but it exports
> much of the functionality you are testing. However could the
> tests also be built as a module (i.e. make CONFIG_STATS_FS_TEST
> a tristate variable)? To test this you'd need to specify
> CONFIG_KUNIT=m and CONFIG_STATS_FS_TEST=m, and testing would
> simply be a case of "modprobe"ing the stats fs module and collecting
> results in /sys/kernel/debug/kunit/<module_name> (rather
> than running kunit.py). Are you relying on unexported internals in
> the the tests that would prevent building them as a module?
>
I haven't checked it yet, but tests should work as separate module.
I will look into it, thanks.
Emanuele
_______________________________________________
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 1/5] power: vexpress: add suppress_bind_attrs to true
From: Rob Herring @ 2020-05-27 13:26 UTC (permalink / raw)
To: Anders Roxell
Cc: SoC Team, linux-kernel@vger.kernel.org,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Linus Walleij
In-Reply-To: <20200527112608.3886105-2-anders.roxell@linaro.org>
On Wed, May 27, 2020 at 5:26 AM Anders Roxell <anders.roxell@linaro.org> wrote:
>
> Make sure that the POWER_RESET_VEXPRESS driver won't have bind/unbind
> attributes available via the sysfs, so lets be explicit here and use
> ".suppress_bind_attrs = true" to prevent userspace from doing something
> silly.
This doesn't really make sense if we're going to make this a module.
Module unloading and unbind introduce the same requirements of
cleaning up (undoing whatever probe did).
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
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