Re: [PATCH v8 00/12] media: staging/imx7: add i.MX7 media driver
Hi Hans, On Wed 21 Nov 2018 at 11:53, Hans Verkuil wrote: On 11/21/2018 12:15 PM, Rui Miguel Silva wrote: Hi, This series introduces the Media driver to work with the i.MX7 SoC. it uses the already existing imx media core drivers but since the i.MX7, contrary to i.MX5/6, do not have an IPU and because of that some changes in the imx media core are made along this series to make it support that case. Can you run scripts/checkpatch.pl --strict over these patches? I get too many messages from it. Most should be easy to fix. Yeah, I will fix as much as I think is sane, some I will leave it to make code more readable. --- Cheers, Rui ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v8 05/12] media: dt-bindings: add bindings for i.MX7 media driver
Hi Sakari,
On Wed 21 Nov 2018 at 22:16, Sakari Ailus wrote:
Hi Rui,
On Wed, Nov 21, 2018 at 11:15:51AM +, Rui Miguel Silva
wrote:
Add bindings documentation for i.MX7 media drivers.
The imx7 MIPI CSI2 and imx7 CMOS Sensor Interface.
Signed-off-by: Rui Miguel Silva
Reviewed-by: Rob Herring
Acked-by: Sakari Ailus
---
.../devicetree/bindings/media/imx7-csi.txt| 45 ++
.../bindings/media/imx7-mipi-csi2.txt | 90
+++
2 files changed, 135 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/imx7-csi.txt
create mode 100644
Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
diff --git
a/Documentation/devicetree/bindings/media/imx7-csi.txt
b/Documentation/devicetree/bindings/media/imx7-csi.txt
new file mode 100644
index ..171b089ee91f
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx7-csi.txt
@@ -0,0 +1,45 @@
+Freescale i.MX7 CMOS Sensor Interface
+=
+
+csi node
+
+
+This is device node for the CMOS Sensor Interface (CSI) which
enables the chip
+to connect directly to external CMOS image sensors.
+
+Required properties:
+
+- compatible: "fsl,imx7-csi";
+- reg : base address and length of the register set
for the device;
+- interrupts: should contain CSI interrupt;
+- clocks: list of clock specifiers, see
+
Documentation/devicetree/bindings/clock/clock-bindings.txt for
details;
+- clock-names : must contain "axi", "mclk" and "dcic"
entries, matching
+ entries in the clock property;
+
+The device node shall contain one 'port' child node with one
child 'endpoint'
+node, according to the bindings defined in:
+ Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+In the following example a remote endpoint is a video
multiplexer.
+
+example:
+
+csi: csi@3071 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+compatible = "fsl,imx7-csi";
+reg = <0x3071 0x1>;
+interrupts = IRQ_TYPE_LEVEL_HIGH>;
+clocks = <&clks IMX7D_CLK_DUMMY>,
+<&clks
IMX7D_CSI_MCLK_ROOT_CLK>,
+<&clks
IMX7D_CLK_DUMMY>;
+clock-names = "axi", "mclk", "dcic";
+
+port {
+csi_from_csi_mux: endpoint {
+remote-endpoint =
<&csi_mux_to_csi>;
+};
+};
+};
diff --git
a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
b/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
new file mode 100644
index ..71fd74ed3ec8
--- /dev/null
+++
b/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
@@ -0,0 +1,90 @@
+Freescale i.MX7 Mipi CSI2
+=
+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 receiver core in
i.MX7 SoC. It is
+compatible with previous version of Samsung D-phy.
+
+Required properties:
+
+- compatible: "fsl,imx7-mipi-csi2";
+- reg : base address and length of the register set
for the device;
+- interrupts: should contain MIPI CSIS interrupt;
+- clocks: list of clock specifiers, see
+
Documentation/devicetree/bindings/clock/clock-bindings.txt for
details;
+- clock-names : must contain "pclk", "wrap" and "phy"
entries, matching
+ entries in the clock property;
+- power-domains : a phandle to the power domain, see
+
Documentation/devicetree/bindings/power/power_domain.txt for
details.
+- reset-names : should include following entry "mrst";
+- resets: a list of phandle, should contain reset
entry of
+ reset-names;
+- phy-supply: from the generic phy bindings, a phandle to
a regulator that
+ provides power to MIPI CSIS core;
+
+Optional properties:
+
+- clock-frequency : The IP's main (system bus) clock frequency
in Hz, default
+ value when this property is not specified is
166 MHz;
+- fsl,csis-hs-settle : differential receiver (HS-RX) settle
time;
+
+The device node should contain two 'port' child nodes with one
child 'endpoint'
+node, according to the bindings defined in:
+ Documentation/devicetree/bindings/
media/video-interfaces.txt.
Extra space. The two lines are also prefixed by a space; is that
intended?
Yes, that will be fix by going over the checkpatch strict.
+ The following are properties specific to those nodes.
+
+port node
+-
+
+- reg : (required) can take the values 0 or 1,
where 0 shall be
+ related to the sink port and port 1 shall
be the source
+ one;
+
+endpoint node
+-
+
+- data-lanes: (required) an
Re: [PATCH v8 03/12] media: staging/imx7: add imx7 CSI subdev driver
Hi Sakari, thanks for all the reviews. On Wed 21 Nov 2018 at 22:29, Sakari Ailus wrote: Hi Rui, On Wed, Nov 21, 2018 at 11:15:49AM +, Rui Miguel Silva wrote: This add the media entity subdevice and control driver for the i.MX7 CMOS Sensor Interface. Signed-off-by: Rui Miguel Silva --- drivers/staging/media/imx/Kconfig |9 +- drivers/staging/media/imx/Makefile |2 + drivers/staging/media/imx/imx7-media-csi.c | 1352 3 files changed, 1362 insertions(+), 1 deletion(-) create mode 100644 drivers/staging/media/imx/imx7-media-csi.c diff --git a/drivers/staging/media/imx/Kconfig b/drivers/staging/media/imx/Kconfig index bfc17de56b17..40a11f988fc6 100644 --- a/drivers/staging/media/imx/Kconfig +++ b/drivers/staging/media/imx/Kconfig @@ -11,7 +11,7 @@ config VIDEO_IMX_MEDIA driver for the i.MX5/6 SOC. if VIDEO_IMX_MEDIA -menu "i.MX5/6 Media Sub devices" +menu "i.MX5/6/7 Media Sub devices" config VIDEO_IMX_CSI tristate "i.MX5/6 Camera Sensor Interface driver" @@ -20,5 +20,12 @@ config VIDEO_IMX_CSI ---help--- A video4linux camera sensor interface driver for i.MX5/6. +config VIDEO_IMX7_CSI + tristate "i.MX7 Camera Sensor Interface driver" + depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C If you need V4L2 fwnode, you should select V4L2_FWNODE. (I ignore if it's already selected by VIDEO_IMX_MEDIA.) Yes it is selected by VIDEO_IMX_MEDIA. + default y + ---help--- + A video4linux camera sensor interface driver for i.MX7. + endmenu endif diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile index a30b3033f9a3..074f016d3519 100644 --- a/drivers/staging/media/imx/Makefile +++ b/drivers/staging/media/imx/Makefile @@ -12,3 +12,5 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-ic.o obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o + +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c new file mode 100644 index ..ec5a20880bb6 --- /dev/null +++ b/drivers/staging/media/imx/imx7-media-csi.c @@ -0,0 +1,1352 @@ +// SPDX-License-Identifier: GPL +/* + * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC + * + * Copyright (c) 2018 Linaro Ltd + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include +#include "imx-media.h" + +#define IMX7_CSI_PAD_SINK 0 +#define IMX7_CSI_PAD_SRC 1 +#define IMX7_CSI_PADS_NUM 2 + +/* reset values */ +#define CSICR1_RESET_VAL 0x4800 +#define CSICR2_RESET_VAL 0x0 +#define CSICR3_RESET_VAL 0x0 + +/* csi control reg 1 */ +#define BIT_SWAP16_EN BIT(31) +#define BIT_EXT_VSYNC BIT(30) +#define BIT_EOF_INT_EN BIT(29) +#define BIT_PRP_IF_EN BIT(28) +#define BIT_CCIR_MODE BIT(27) +#define BIT_COF_INT_EN BIT(26) +#define BIT_SF_OR_INTENBIT(25) +#define BIT_RF_OR_INTENBIT(24) +#define BIT_SFF_DMA_DONE_INTEN BIT(22) +#define BIT_STATFF_INTEN BIT(21) +#define BIT_FB2_DMA_DONE_INTEN BIT(20) +#define BIT_FB1_DMA_DONE_INTEN BIT(19) +#define BIT_RXFF_INTEN BIT(18) +#define BIT_SOF_POLBIT(17) +#define BIT_SOF_INTEN BIT(16) +#define BIT_MCLKDIV(0xF << 12) +#define BIT_HSYNC_POL BIT(11) +#define BIT_CCIR_ENBIT(10) +#define BIT_MCLKEN BIT(9) +#define BIT_FCCBIT(8) +#define BIT_PACK_DIR BIT(7) +#define BIT_CLR_STATFIFO BIT(6) +#define BIT_CLR_RXFIFO BIT(5) +#define BIT_GCLK_MODE BIT(4) +#define BIT_INV_DATA BIT(3) +#define BIT_INV_PCLK BIT(2) +#define BIT_REDGE BIT(1) +#define BIT_PIXEL_BIT BIT(0) + +#define SHIFT_MCLKDIV 12 + +/* control reg 3 */ +#define BIT_FRMCNT (0x << 16) +#define BIT_FRMCNT_RST BIT(15) +#define BIT_DMA_REFLASH_RFFBIT(14) +#define BIT_DMA_REFLASH_SFFBIT(13) +#define BIT_DMA_REQ_EN_RFF BIT(12) +#define BIT_DMA_REQ_EN_SFF BIT(11) +#define BIT_STATFF_LEVEL (0x7 << 8) +#define BIT_HRESP_ERR_EN BIT(7) +#define BIT_RXFF_LEVEL (0x7 << 4) +#define BIT_TWO_8BIT_SENSORBIT(3) +#define BIT_ZERO_PACK_EN BIT(2) +#define BIT_ECC_INT_EN BIT(1) +#define BIT_ECC_AUTO_ENBIT(0) + +#define SHIFT_FRMCNT 16 +#define SHIFT_RXFIFO_LEVEL 4 + +/* csi status reg */ +#define BIT_ADDR_CH_ERR_INTBIT(28) +#define BIT_FIELD0_INT BIT(27) +#define BIT_FIELD1_INT BIT(26) +#define BIT_SFF_OR_INT BIT(25) +#define BIT_RFF_OR_INT BIT(24) +#define BIT_DMA_TSF_DONE_SFF BIT(22) +#define BIT_STATF
[PATCH v2 2/8] mm: convert PG_balloon to PG_offline
PG_balloon was introduced to implement page migration/compaction for pages
inflated in virtio-balloon. Nowadays, it is only a marker that a page is
part of virtio-balloon and therefore logically offline.
We also want to make use of this flag in other balloon drivers - for
inflated pages or when onlining a section but keeping some pages offline
(e.g. used right now by XEN and Hyper-V via set_online_page_callback()).
We are going to expose this flag to dump tools like makedumpfile. But
instead of exposing PG_balloon, let's generalize the concept of marking
pages as logically offline, so it can be reused for other purposes
later on.
Rename PG_balloon to PG_offline. This is an indicator that the page is
logically offline, the content stale and that it should not be touched
(e.g. a hypervisor would have to allocate backing storage in order for the
guest to dump an unused page). We can then e.g. exclude such pages from
dumps.
We replace and reuse KPF_BALLOON (23), as this shouldn't really harm
(and for now the semantics stay the same). In following patches, we will
make use of this bit also in other balloon drivers. While at it, document
PGTABLE.
Cc: Jonathan Corbet
Cc: Alexey Dobriyan
Cc: Mike Rapoport
Cc: Andrew Morton
Cc: Christian Hansen
Cc: Vlastimil Babka
Cc: "Kirill A. Shutemov"
Cc: Stephen Rothwell
Cc: Matthew Wilcox
Cc: "Michael S. Tsirkin"
Cc: Michal Hocko
Cc: Pavel Tatashin
Cc: Alexander Duyck
Cc: Naoya Horiguchi
Cc: Miles Chen
Cc: David Rientjes
Cc: Konstantin Khlebnikov
Cc: Kazuhito Hagio
Acked-by: Konstantin Khlebnikov
Acked-by: Michael S. Tsirkin
Acked-by: Pankaj gupta
Signed-off-by: David Hildenbrand
---
Documentation/admin-guide/mm/pagemap.rst | 9 ++---
fs/proc/page.c | 4 ++--
include/linux/balloon_compaction.h | 8
include/linux/page-flags.h | 11 +++
include/uapi/linux/kernel-page-flags.h | 2 +-
tools/vm/page-types.c| 2 +-
6 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/Documentation/admin-guide/mm/pagemap.rst
b/Documentation/admin-guide/mm/pagemap.rst
index 3f7bade2c231..340a5aee9b80 100644
--- a/Documentation/admin-guide/mm/pagemap.rst
+++ b/Documentation/admin-guide/mm/pagemap.rst
@@ -75,9 +75,10 @@ number of times a page is mapped.
20. NOPAGE
21. KSM
22. THP
-23. BALLOON
+23. OFFLINE
24. ZERO_PAGE
25. IDLE
+26. PGTABLE
* ``/proc/kpagecgroup``. This file contains a 64-bit inode number of the
memory cgroup each page is charged to, indexed by PFN. Only available when
@@ -118,8 +119,8 @@ Short descriptions to the page flags
identical memory pages dynamically shared between one or more processes
22 - THP
contiguous pages which construct transparent hugepages
-23 - BALLOON
-balloon compaction page
+23 - OFFLINE
+page is logically offline
24 - ZERO_PAGE
zero page for pfn_zero or huge_zero page
25 - IDLE
@@ -128,6 +129,8 @@ Short descriptions to the page flags
Note that this flag may be stale in case the page was accessed via
a PTE. To make sure the flag is up-to-date one has to read
``/sys/kernel/mm/page_idle/bitmap`` first.
+26 - PGTABLE
+page is in use as a page table
IO related page flags
-
diff --git a/fs/proc/page.c b/fs/proc/page.c
index 6c517b11acf8..378401af4d9d 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -152,8 +152,8 @@ u64 stable_page_flags(struct page *page)
else if (page_count(page) == 0 && is_free_buddy_page(page))
u |= 1 << KPF_BUDDY;
- if (PageBalloon(page))
- u |= 1 << KPF_BALLOON;
+ if (PageOffline(page))
+ u |= 1 << KPF_OFFLINE;
if (PageTable(page))
u |= 1 << KPF_PGTABLE;
diff --git a/include/linux/balloon_compaction.h
b/include/linux/balloon_compaction.h
index cbe50da5a59d..f111c780ef1d 100644
--- a/include/linux/balloon_compaction.h
+++ b/include/linux/balloon_compaction.h
@@ -95,7 +95,7 @@ extern int balloon_page_migrate(struct address_space *mapping,
static inline void balloon_page_insert(struct balloon_dev_info *balloon,
struct page *page)
{
- __SetPageBalloon(page);
+ __SetPageOffline(page);
__SetPageMovable(page, balloon->inode->i_mapping);
set_page_private(page, (unsigned long)balloon);
list_add(&page->lru, &balloon->pages);
@@ -111,7 +111,7 @@ static inline void balloon_page_insert(struct
balloon_dev_info *balloon,
*/
static inline void balloon_page_delete(struct page *page)
{
- __ClearPageBalloon(page);
+ __ClearPageOffline(page);
__ClearPageMovable(page);
set_page_private(page, 0);
/*
@@ -141,13 +141,13 @@ static inline gfp_t balloon_mapping_gfp_mask(void)
static inline void balloon_page_insert(struct balloon_dev_info *balloon,
struct page *page)
{
-
[PATCH v2 0/8] mm/kdump: allow to exclude pages that are logically offline
Right now, pages inflated as part of a balloon driver will be dumped by dump tools like makedumpfile. While XEN is able to check in the crash kernel whether a certain pfn is actually backed by memory in the hypervisor (see xen_oldmem_pfn_is_ram) and optimize this case, dumps of virtio-balloon, hv-balloon and VMWare balloon inflated memory will essentially result in zero pages getting allocated by the hypervisor and the dump getting filled with this data. The allocation and reading of zero pages can directly be avoided if a dumping tool could know which pages only contain stale information not to be dumped. Also for XEN, calling into the kernel and asking the hypervisor if a pfn is backed can be avoided if the duming tool would skip such pages right from the beginning. Dumping tools have no idea whether a given page is part of a balloon driver and shall not be dumped. Esp. PG_reserved cannot be used for that purpose as all memory allocated during early boot is also PG_reserved, see discussion at [1]. So some other way of indication is required and a new page flag is frowned upon. We have PG_balloon (MAPCOUNT value), which is essentially unused now. I suggest renaming it to something more generic (PG_offline) to mark pages as logically offline. This flag can than e.g. also be used by virtio-mem in the future to mark subsections as offline. Or by other code that wants to put pages logically offline (e.g. later maybe poisoned pages that shall no longer be used). This series converts PG_balloon to PG_offline, allows dumping tools to query the value to detect such pages and marks pages in the hv-balloon and XEN balloon properly as PG_offline. Note that virtio-balloon already set pages to PG_balloon (and now PG_offline). Please note that this is also helpful for a problem we were seeing under Hyper-V: Dumping logically offline memory (pages kept fake offline while onlining a section via online_page_callback) would under some condicions result in a kernel panic when dumping them. As I don't have access to neither XEN nor Hyper-V nor VMWare installations, this was only tested with the virtio-balloon and pages were properly skipped when dumping. I'll also attach the makedumpfile patch to this series. [1] https://lkml.org/lkml/2018/7/20/566 v1 -> v2: - "kexec: export PG_offline to VMCOREINFO" -- Add description why it is exported as a macro - "vmw_balloon: mark inflated pages PG_offline" -- Use helper function + adapt comments - "PM / Hibernate: exclude all PageOffline() pages" -- Perform the check separate from swsusp checks. - Added RBs/ACKs David Hildenbrand (8): mm: balloon: update comment about isolation/migration/compaction mm: convert PG_balloon to PG_offline kexec: export PG_offline to VMCOREINFO xen/balloon: mark inflated pages PG_offline hv_balloon: mark inflated pages PG_offline vmw_balloon: mark inflated pages PG_offline PM / Hibernate: use pfn_to_online_page() PM / Hibernate: exclude all PageOffline() pages Documentation/admin-guide/mm/pagemap.rst | 9 --- drivers/hv/hv_balloon.c | 14 -- drivers/misc/vmw_balloon.c | 32 ++ drivers/xen/balloon.c| 3 +++ fs/proc/page.c | 4 +-- include/linux/balloon_compaction.h | 34 +--- include/linux/page-flags.h | 11 +--- include/uapi/linux/kernel-page-flags.h | 2 +- kernel/crash_core.c | 2 ++ kernel/power/snapshot.c | 17 +++- tools/vm/page-types.c| 2 +- 11 files changed, 90 insertions(+), 40 deletions(-) -- 2.17.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/8] mm: balloon: update comment about isolation/migration/compaction
Commit b1123ea6d3b3 ("mm: balloon: use general non-lru movable page
feature") reworked balloon handling to make use of the general
non-lru movable page feature. The big comment block in
balloon_compaction.h contains quite some outdated information. Let's fix
this.
Cc: Andrew Morton
Cc: Matthew Wilcox
Cc: Michal Hocko
Cc: "Michael S. Tsirkin"
Acked-by: Michael S. Tsirkin
Signed-off-by: David Hildenbrand
---
include/linux/balloon_compaction.h | 26 +-
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/include/linux/balloon_compaction.h
b/include/linux/balloon_compaction.h
index 53051f3d8f25..cbe50da5a59d 100644
--- a/include/linux/balloon_compaction.h
+++ b/include/linux/balloon_compaction.h
@@ -4,15 +4,18 @@
*
* Common interface definitions for making balloon pages movable by compaction.
*
- * Despite being perfectly possible to perform ballooned pages migration, they
- * make a special corner case to compaction scans because balloon pages are not
- * enlisted at any LRU list like the other pages we do compact / migrate.
+ * Balloon page migration makes use of the general non-lru movable page
+ * feature.
+ *
+ * page->private is used to reference the responsible balloon device.
+ * page->mapping is used in context of non-lru page migration to reference
+ * the address space operations for page isolation/migration/compaction.
*
* As the page isolation scanning step a compaction thread does is a lockless
* procedure (from a page standpoint), it might bring some racy situations
while
* performing balloon page compaction. In order to sort out these racy
scenarios
* and safely perform balloon's page compaction and migration we must, always,
- * ensure following these three simple rules:
+ * ensure following these simple rules:
*
* i. when updating a balloon's page ->mapping element, strictly do it under
* the following lock order, independently of the far superior
@@ -21,19 +24,8 @@
* +--spin_lock_irq(&b_dev_info->pages_lock);
* ... page->mapping updates here ...
*
- * ii. before isolating or dequeueing a balloon page from the balloon device
- * pages list, the page reference counter must be raised by one and the
- * extra refcount must be dropped when the page is enqueued back into
- * the balloon device page list, thus a balloon page keeps its reference
- * counter raised only while it is under our special handling;
- *
- * iii. after the lockless scan step have selected a potential balloon page for
- * isolation, re-test the PageBalloon mark and the PagePrivate flag
- * under the proper page lock, to ensure isolating a valid balloon page
- * (not yet isolated, nor under release procedure)
- *
- * iv. isolation or dequeueing procedure must clear PagePrivate flag under
- * page lock together with removing page from balloon device page list.
+ * ii. isolation or dequeueing procedure must remove the page from balloon
+ * device page list under b_dev_info->pages_lock.
*
* The functions provided by this interface are placed to help on coping with
* the aforementioned balloon page corner case, as well as to ensure the simple
--
2.17.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/8] kexec: export PG_offline to VMCOREINFO
Right now, pages inflated as part of a balloon driver will be dumped by dump tools like makedumpfile. While XEN is able to check in the crash kernel whether a certain pfn is actuall backed by memory in the hypervisor (see xen_oldmem_pfn_is_ram) and optimize this case, dumps of other balloon inflated memory will essentially result in zero pages getting allocated by the hypervisor and the dump getting filled with this data. The allocation and reading of zero pages can directly be avoided if a dumping tool could know which pages only contain stale information not to be dumped. We now have PG_offline which can be (and already is by virtio-balloon) used for marking pages as logically offline. Follow up patches will make use of this flag also in other balloon implementations. Let's export PG_offline via PAGE_OFFLINE_MAPCOUNT_VALUE, so makedumpfile can directly skip pages that are logically offline and the content therefore stale. (we export is as a macro to match how it is done for PG_buddy. This way it is clearer that this is not actually a flag but only a very specific mapcount value to represent page types). Please note that this is also helpful for a problem we were seeing under Hyper-V: Dumping logically offline memory (pages kept fake offline while onlining a section via online_page_callback) would under some condicions result in a kernel panic when dumping them. Cc: Andrew Morton Cc: Dave Young Cc: "Kirill A. Shutemov" Cc: Baoquan He Cc: Omar Sandoval Cc: Arnd Bergmann Cc: Matthew Wilcox Cc: Michal Hocko Cc: "Michael S. Tsirkin" Cc: Lianbo Jiang Cc: Borislav Petkov Cc: Kazuhito Hagio Acked-by: Michael S. Tsirkin Acked-by: Dave Young Signed-off-by: David Hildenbrand --- kernel/crash_core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/crash_core.c b/kernel/crash_core.c index 933cb3e45b98..093c9f917ed0 100644 --- a/kernel/crash_core.c +++ b/kernel/crash_core.c @@ -464,6 +464,8 @@ static int __init crash_save_vmcoreinfo_init(void) VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE); #ifdef CONFIG_HUGETLB_PAGE VMCOREINFO_NUMBER(HUGETLB_PAGE_DTOR); +#define PAGE_OFFLINE_MAPCOUNT_VALUE(~PG_offline) + VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE); #endif arch_crash_save_vmcoreinfo(); -- 2.17.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 8/8] PM / Hibernate: exclude all PageOffline() pages
The content of pages that are marked PG_offline is not of interest (e.g. inflated by a balloon driver), let's skip these pages. In saveable_highmem_page(), move the PageReserved() check to a new check along with the PageOffline() check to separate it from the swsusp checks. Cc: "Rafael J. Wysocki" Cc: Pavel Machek Cc: Len Brown Cc: Andrew Morton Cc: Matthew Wilcox Cc: Michal Hocko Cc: "Michael S. Tsirkin" Acked-by: Pavel Machek Acked-by: Rafael J. Wysocki Signed-off-by: David Hildenbrand --- kernel/power/snapshot.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index 87e6dd57819f..4802b039b89f 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -1221,8 +1221,10 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn) BUG_ON(!PageHighMem(page)); - if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page) || - PageReserved(page)) + if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page)) + return NULL; + + if (PageReserved(page) || PageOffline(page)) return NULL; if (page_is_guard(page)) @@ -1286,6 +1288,9 @@ static struct page *saveable_page(struct zone *zone, unsigned long pfn) if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page)) return NULL; + if (PageOffline(page)) + return NULL; + if (PageReserved(page) && (!kernel_page_present(page) || pfn_is_nosave(pfn))) return NULL; -- 2.17.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/8] hv_balloon: mark inflated pages PG_offline
Mark inflated and never onlined pages PG_offline, to tell the world that
the content is stale and should not be dumped.
Cc: "K. Y. Srinivasan"
Cc: Haiyang Zhang
Cc: Stephen Hemminger
Cc: Kairui Song
Cc: Vitaly Kuznetsov
Cc: Andrew Morton
Cc: Matthew Wilcox
Cc: Michal Hocko
Cc: "Michael S. Tsirkin"
Acked-by: Pankaj gupta
Signed-off-by: David Hildenbrand
---
drivers/hv/hv_balloon.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 211f3fe3a038..47719862e57f 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -681,8 +681,13 @@ static struct notifier_block hv_memory_nb = {
/* Check if the particular page is backed and can be onlined and online it. */
static void hv_page_online_one(struct hv_hotadd_state *has, struct page *pg)
{
- if (!has_pfn_is_backed(has, page_to_pfn(pg)))
+ if (!has_pfn_is_backed(has, page_to_pfn(pg))) {
+ if (!PageOffline(pg))
+ __SetPageOffline(pg);
return;
+ }
+ if (PageOffline(pg))
+ __ClearPageOffline(pg);
/* This frame is currently backed; online the page. */
__online_page_set_limits(pg);
@@ -1201,6 +1206,7 @@ static void free_balloon_pages(struct hv_dynmem_device
*dm,
for (i = 0; i < num_pages; i++) {
pg = pfn_to_page(i + start_frame);
+ __ClearPageOffline(pg);
__free_page(pg);
dm->num_pages_ballooned--;
}
@@ -1213,7 +1219,7 @@ static unsigned int alloc_balloon_pages(struct
hv_dynmem_device *dm,
struct dm_balloon_response *bl_resp,
int alloc_unit)
{
- unsigned int i = 0;
+ unsigned int i, j;
struct page *pg;
if (num_pages < alloc_unit)
@@ -1245,6 +1251,10 @@ static unsigned int alloc_balloon_pages(struct
hv_dynmem_device *dm,
if (alloc_unit != 1)
split_page(pg, get_order(alloc_unit << PAGE_SHIFT));
+ /* mark all pages offline */
+ for (j = 0; j < (1 << get_order(alloc_unit << PAGE_SHIFT)); j++)
+ __SetPageOffline(pg + j);
+
bl_resp->range_count++;
bl_resp->range_array[i].finfo.start_page =
page_to_pfn(pg);
--
2.17.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/8] vmw_balloon: mark inflated pages PG_offline
Mark inflated and never onlined pages PG_offline, to tell the world that
the content is stale and should not be dumped.
Cc: Xavier Deguillard
Cc: Nadav Amit
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Julien Freche
Cc: Andrew Morton
Cc: Matthew Wilcox
Cc: Michal Hocko
Cc: "Michael S. Tsirkin"
Acked-by: Nadav Amit
Signed-off-by: David Hildenbrand
---
drivers/misc/vmw_balloon.c | 32
1 file changed, 32 insertions(+)
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index e6126a4b95d3..877611b5659b 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -556,6 +556,36 @@ vmballoon_page_in_frames(enum vmballoon_page_size_type
page_size)
return 1 << vmballoon_page_order(page_size);
}
+/**
+ * vmballoon_mark_page_offline() - mark a page as offline
+ * @page: pointer for the page.
+ * @page_size: the size of the page.
+ */
+static void
+vmballoon_mark_page_offline(struct page *page,
+ enum vmballoon_page_size_type page_size)
+{
+ int i;
+
+ for (i = 0; i < vmballoon_page_in_frames(page_size); i++)
+ __SetPageOffline(page + i);
+}
+
+/**
+ * vmballoon_mark_page_online() - mark a page as online
+ * @page: pointer for the page.
+ * @page_size: the size of the page.
+ */
+static void
+vmballoon_mark_page_online(struct page *page,
+ enum vmballoon_page_size_type page_size)
+{
+ int i;
+
+ for (i = 0; i < vmballoon_page_in_frames(page_size); i++)
+ __ClearPageOffline(page + i);
+}
+
/**
* vmballoon_send_get_target() - Retrieve desired balloon size from the host.
*
@@ -612,6 +642,7 @@ static int vmballoon_alloc_page_list(struct vmballoon *b,
ctl->page_size);
if (page) {
+ vmballoon_mark_page_offline(page, ctl->page_size);
/* Success. Add the page to the list and continue. */
list_add(&page->lru, &ctl->pages);
continue;
@@ -850,6 +881,7 @@ static void vmballoon_release_page_list(struct list_head
*page_list,
list_for_each_entry_safe(page, tmp, page_list, lru) {
list_del(&page->lru);
+ vmballoon_mark_page_online(page, page_size);
__free_pages(page, vmballoon_page_order(page_size));
}
--
2.17.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 7/8] PM / Hibernate: use pfn_to_online_page()
Let's use pfn_to_online_page() instead of pfn_to_page() when checking for saveable pages to not save/restore offline memory sections. Cc: "Rafael J. Wysocki" Cc: Pavel Machek Cc: Len Brown Cc: Andrew Morton Cc: Matthew Wilcox Cc: Michal Hocko Cc: "Michael S. Tsirkin" Suggested-by: Michal Hocko Acked-by: Michal Hocko Acked-by: Pavel Machek Acked-by: Rafael J. Wysocki Signed-off-by: David Hildenbrand --- kernel/power/snapshot.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index 640b2034edd6..87e6dd57819f 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -1215,8 +1215,8 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn) if (!pfn_valid(pfn)) return NULL; - page = pfn_to_page(pfn); - if (page_zone(page) != zone) + page = pfn_to_online_page(pfn); + if (!page || page_zone(page) != zone) return NULL; BUG_ON(!PageHighMem(page)); @@ -1277,8 +1277,8 @@ static struct page *saveable_page(struct zone *zone, unsigned long pfn) if (!pfn_valid(pfn)) return NULL; - page = pfn_to_page(pfn); - if (page_zone(page) != zone) + page = pfn_to_online_page(pfn); + if (!page || page_zone(page) != zone) return NULL; BUG_ON(PageHighMem(page)); -- 2.17.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/8] xen/balloon: mark inflated pages PG_offline
Mark inflated and never onlined pages PG_offline, to tell the world that
the content is stale and should not be dumped.
Cc: Boris Ostrovsky
Cc: Juergen Gross
Cc: Stefano Stabellini
Cc: Andrew Morton
Cc: Matthew Wilcox
Cc: Michal Hocko
Cc: "Michael S. Tsirkin"
Signed-off-by: David Hildenbrand
---
drivers/xen/balloon.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 12148289debd..14dd6b814db3 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -425,6 +425,7 @@ static int xen_bring_pgs_online(struct page *pg, unsigned
int order)
for (i = 0; i < size; i++) {
p = pfn_to_page(start_pfn + i);
__online_page_set_limits(p);
+ __SetPageOffline(p);
__balloon_append(p);
}
mutex_unlock(&balloon_mutex);
@@ -493,6 +494,7 @@ static enum bp_state increase_reservation(unsigned long
nr_pages)
xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);
/* Relinquish the page back to the allocator. */
+ __ClearPageOffline(page);
free_reserved_page(page);
}
@@ -519,6 +521,7 @@ static enum bp_state decrease_reservation(unsigned long
nr_pages, gfp_t gfp)
state = BP_EAGAIN;
break;
}
+ __SetPageOffline(page);
adjust_managed_page_count(page, -1);
xenmem_reservation_scrub_page(page);
list_add(&page->lru, &pages);
--
2.17.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] makedumpfile: exclude pages that are logically offline
Linux marks pages that are logically offline via a page flag (map count).
Such pages e.g. include pages infated as part of a balloon driver or
pages that were not actually onlined when onlining the whole section.
While the hypervisor usually allows to read such inflated memory, we
basically read and dump data that is completely irrelevant. Also, this
might result in quite some overhead in the hypervisor. In addition,
we saw some problems under Hyper-V, whereby we can crash the kernel by
dumping, when reading memory of a partially onlined memory segment
(for memory added by the Hyper-V balloon driver).
Therefore, don't read and dump pages that are marked as being logically
offline.
Signed-off-by: David Hildenbrand
---
v1 -> v2:
- Fix PAGE_BUDDY_MAPCOUNT_VALUE vs. PAGE_OFFLINE_MAPCOUNT_VALUE
makedumpfile.c | 34 ++
makedumpfile.h | 1 +
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index 8923538..a5f2ea9 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -88,6 +88,7 @@ mdf_pfn_t pfn_cache_private;
mdf_pfn_t pfn_user;
mdf_pfn_t pfn_free;
mdf_pfn_t pfn_hwpoison;
+mdf_pfn_t pfn_offline;
mdf_pfn_t num_dumped;
@@ -249,6 +250,21 @@ isHugetlb(unsigned long dtor)
&& (SYMBOL(free_huge_page) == dtor));
}
+static int
+isOffline(unsigned long flags, unsigned int _mapcount)
+{
+ if (NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE) == NOT_FOUND_NUMBER)
+ return FALSE;
+
+ if (flags & (1UL << NUMBER(PG_slab)))
+ return FALSE;
+
+ if (_mapcount == (int)NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE))
+ return TRUE;
+
+ return FALSE;
+}
+
static int
is_cache_page(unsigned long flags)
{
@@ -2287,6 +2303,8 @@ write_vmcoreinfo_data(void)
WRITE_NUMBER("PG_hwpoison", PG_hwpoison);
WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
+ WRITE_NUMBER("PAGE_OFFLINE_MAPCOUNT_VALUE",
+PAGE_OFFLINE_MAPCOUNT_VALUE);
WRITE_NUMBER("phys_base", phys_base);
WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
@@ -2687,6 +2705,7 @@ read_vmcoreinfo(void)
READ_SRCFILE("pud_t", pud_t);
READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
+ READ_NUMBER("PAGE_OFFLINE_MAPCOUNT_VALUE", PAGE_OFFLINE_MAPCOUNT_VALUE);
READ_NUMBER("phys_base", phys_base);
#ifdef __aarch64__
READ_NUMBER("VA_BITS", VA_BITS);
@@ -6041,6 +6060,12 @@ __exclude_unnecessary_pages(unsigned long mem_map,
else if (isHWPOISON(flags)) {
pfn_counter = &pfn_hwpoison;
}
+ /*
+* Exclude pages that are logically offline.
+*/
+ else if (isOffline(flags, _mapcount)) {
+ pfn_counter = &pfn_offline;
+ }
/*
* Unexcludable page
*/
@@ -7522,7 +7547,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header,
struct cache_data *cd_page)
*/
if (info->flag_cyclic) {
pfn_zero = pfn_cache = pfn_cache_private = 0;
- pfn_user = pfn_free = pfn_hwpoison = 0;
+ pfn_user = pfn_free = pfn_hwpoison = pfn_offline = 0;
pfn_memhole = info->max_mapnr;
}
@@ -8804,7 +8829,7 @@ write_kdump_pages_and_bitmap_cyclic(struct cache_data
*cd_header, struct cache_d
* Reset counter for debug message.
*/
pfn_zero = pfn_cache = pfn_cache_private = 0;
- pfn_user = pfn_free = pfn_hwpoison = 0;
+ pfn_user = pfn_free = pfn_hwpoison = pfn_offline = 0;
pfn_memhole = info->max_mapnr;
/*
@@ -9749,7 +9774,7 @@ print_report(void)
pfn_original = info->max_mapnr - pfn_memhole;
pfn_excluded = pfn_zero + pfn_cache + pfn_cache_private
- + pfn_user + pfn_free + pfn_hwpoison;
+ + pfn_user + pfn_free + pfn_hwpoison + pfn_offline;
shrinking = (pfn_original - pfn_excluded) * 100;
shrinking = shrinking / pfn_original;
@@ -9763,6 +9788,7 @@ print_report(void)
REPORT_MSG("User process data pages : 0x%016llx\n", pfn_user);
REPORT_MSG("Free pages : 0x%016llx\n", pfn_free);
REPORT_MSG("Hwpoison pages : 0x%016llx\n", pfn_hwpoison);
+ REPORT_MSG("Offline pages : 0x%016llx\n", pfn_offline);
REPORT_MSG(" Remaining pages : 0x%016llx\n",
pfn_original - pfn_excluded);
REPORT_MSG(" (The number of pages is reduced to %lld%%.)\n",
@@ -9790,7 +9816,7 @@ print_mem_usage(void)
pfn_original = info->max_mapnr - pfn_memhole;
pfn_excluded = pfn_zero + pfn_cache + pfn_cache_private
- + pfn_user + pfn_free + pfn_hwpoison;
+ + pfn_user + pfn_free + pfn_hwpoison + pfn_offline;
Re: [PATCH 02/10] staging: erofs: fix race when the managed cache is enabled
On Tue, Nov 20, 2018 at 10:34:17PM +0800, Gao Xiang wrote: > When the managed cache is enabled, the last reference count > of a workgroup must be used for its workstation. > > Otherwise, it could lead to incorrect (un)freezes in > the reclaim path, and it would be harmful. > > A typical race as follows: > > Thread 1 (In the reclaim path) Thread 2 > workgroup_freeze(grp, 1)refcnt = 1 > ... > workgroup_unfreeze(grp, 1) refcnt = 1 > workgroup_get(grp) refcnt = 2 (x) > workgroup_put(grp) refcnt = 1 (x) > ...unexpected behaviors > > * grp is detached but still used, which violates cache-managed > freeze constraint. > > Reviewed-by: Chao Yu > Signed-off-by: Gao Xiang > --- > drivers/staging/erofs/internal.h | 1 + > drivers/staging/erofs/utils.c| 131 > +++ > 2 files changed, 93 insertions(+), 39 deletions(-) > > diff --git a/drivers/staging/erofs/internal.h > b/drivers/staging/erofs/internal.h > index 57575c7f5635..89dbd0888e53 100644 > --- a/drivers/staging/erofs/internal.h > +++ b/drivers/staging/erofs/internal.h > @@ -250,6 +250,7 @@ static inline bool erofs_workgroup_get(struct > erofs_workgroup *grp, int *ocnt) > } > > #define __erofs_workgroup_get(grp) atomic_inc(&(grp)->refcount) > +#define __erofs_workgroup_put(grp) atomic_dec(&(grp)->refcount) Any specific reason why you are not using the refcount.h api instead of "doing it yourself" with atomic_inc/dec()? I'm not rejecting this, just curious. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/10] staging: erofs: fix race when the managed cache is enabled
On Tue, Nov 20, 2018 at 10:34:17PM +0800, Gao Xiang wrote:
> When the managed cache is enabled, the last reference count
> of a workgroup must be used for its workstation.
>
> Otherwise, it could lead to incorrect (un)freezes in
> the reclaim path, and it would be harmful.
>
> A typical race as follows:
>
> Thread 1 (In the reclaim path) Thread 2
> workgroup_freeze(grp, 1)refcnt = 1
> ...
> workgroup_unfreeze(grp, 1) refcnt = 1
> workgroup_get(grp) refcnt = 2 (x)
> workgroup_put(grp) refcnt = 1 (x)
> ...unexpected behaviors
>
> * grp is detached but still used, which violates cache-managed
> freeze constraint.
>
> Reviewed-by: Chao Yu
> Signed-off-by: Gao Xiang
> ---
> drivers/staging/erofs/internal.h | 1 +
> drivers/staging/erofs/utils.c| 131
> +++
> 2 files changed, 93 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/staging/erofs/internal.h
> b/drivers/staging/erofs/internal.h
> index 57575c7f5635..89dbd0888e53 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -250,6 +250,7 @@ static inline bool erofs_workgroup_get(struct
> erofs_workgroup *grp, int *ocnt)
> }
>
> #define __erofs_workgroup_get(grp) atomic_inc(&(grp)->refcount)
> +#define __erofs_workgroup_put(grp) atomic_dec(&(grp)->refcount)
>
> extern int erofs_workgroup_put(struct erofs_workgroup *grp);
>
> diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
> index ea8a962e5c95..90de8d9195b7 100644
> --- a/drivers/staging/erofs/utils.c
> +++ b/drivers/staging/erofs/utils.c
> @@ -83,12 +83,21 @@ int erofs_register_workgroup(struct super_block *sb,
>
> grp = xa_tag_pointer(grp, tag);
>
> - err = radix_tree_insert(&sbi->workstn_tree,
> - grp->index, grp);
> + /*
> + * Bump up reference count before making this workgroup
> + * visible to other users in order to avoid potential UAF
> + * without serialized by erofs_workstn_lock.
> + */
> + __erofs_workgroup_get(grp);
>
> - if (!err) {
> - __erofs_workgroup_get(grp);
> - }
> + err = radix_tree_insert(&sbi->workstn_tree,
> + grp->index, grp);
> + if (unlikely(err))
> + /*
> + * it's safe to decrease since the workgroup isn't visible
> + * and refcount >= 2 (cannot be freezed).
> + */
> + __erofs_workgroup_put(grp);
>
> erofs_workstn_unlock(sbi);
> radix_tree_preload_end();
> @@ -97,19 +106,91 @@ int erofs_register_workgroup(struct super_block *sb,
>
> extern void erofs_workgroup_free_rcu(struct erofs_workgroup *grp);
>
> +static void __erofs_workgroup_free(struct erofs_workgroup *grp)
> +{
> + atomic_long_dec(&erofs_global_shrink_cnt);
> + erofs_workgroup_free_rcu(grp);
> +}
> +
> int erofs_workgroup_put(struct erofs_workgroup *grp)
> {
> int count = atomic_dec_return(&grp->refcount);
>
> if (count == 1)
> atomic_long_inc(&erofs_global_shrink_cnt);
> - else if (!count) {
> - atomic_long_dec(&erofs_global_shrink_cnt);
> - erofs_workgroup_free_rcu(grp);
> - }
> + else if (!count)
> + __erofs_workgroup_free(grp);
> return count;
> }
>
> +#ifdef EROFS_FS_HAS_MANAGED_CACHE
> +/* for cache-managed case, customized reclaim paths exist */
> +static void erofs_workgroup_unfreeze_final(struct erofs_workgroup *grp)
> +{
> + erofs_workgroup_unfreeze(grp, 0);
> + __erofs_workgroup_free(grp);
> +}
> +
> +bool erofs_try_to_release_workgroup(struct erofs_sb_info *sbi,
> + struct erofs_workgroup *grp,
> + bool cleanup)
> +{
> + /*
> + * for managed cache enabled, the refcount of workgroups
> + * themselves could be < 0 (freezed). So there is no guarantee
> + * that all refcount > 0 if managed cache is enabled.
> + */
> + if (!erofs_workgroup_try_to_freeze(grp, 1))
> + return false;
> +
> + /*
> + * note that all cached pages should be unlinked
> + * before delete it from the radix tree.
> + * Otherwise some cached pages of an orphan old workgroup
> + * could be still linked after the new one is available.
> + */
> + if (erofs_try_to_free_all_cached_pages(sbi, grp)) {
> + erofs_workgroup_unfreeze(grp, 1);
> + return false;
> + }
> +
> + /* it is impossible to fail after we freeze the workgroup */
> + BUG_ON(xa_untag_pointer(radix_tree_delete(&sbi->workstn_tree,
> + grp->index)) != grp);
> +
It is not a good idea to crash the system. Please try to recover from
this, a BUG_ON() implies that the developer has n
Re: [PATCH 01/10] staging: erofs: fix `trace_erofs_readpage' position
On Tue, Nov 20, 2018 at 10:34:16PM +0800, Gao Xiang wrote:
> `trace_erofs_readpage' should be placed in .readpage()
> rather than in the internal `z_erofs_do_read_page'.
Why? What happens with the code today?
>
> Fixes: 284db12cfda3 ("staging: erofs: add trace points for reading zipped
> data")
Should this get into 4.20-final?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/10] staging: erofs: atomic_cond_read_relaxed on ref-locked workgroup
On Tue, Nov 20, 2018 at 10:34:18PM +0800, Gao Xiang wrote:
> It's better to use atomic_cond_read_relaxed, which is implemented
> in hardware instructions to monitor a variable changes currently
> for ARM64, instead of open-coded busy waiting.
>
> Reviewed-by: Chao Yu
> Signed-off-by: Gao Xiang
> ---
> drivers/staging/erofs/internal.h | 30 ++
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/erofs/internal.h
> b/drivers/staging/erofs/internal.h
> index 89dbd0888e53..eb80ba44d072 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -221,23 +221,29 @@ static inline void erofs_workgroup_unfreeze(
> preempt_enable();
> }
>
> +#if defined(CONFIG_SMP)
> +static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup
> *grp)
> +{
> + return atomic_cond_read_relaxed(&grp->refcount,
> + VAL != EROFS_LOCKED_MAGIC);
> +}
> +#else
> +static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup
> *grp)
> +{
> + int v = atomic_read(&grp->refcount);
Again, why not use the refcount api instead of doing this yourself?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/10] staging: erofs: fix `erofs_workgroup_{try_to_freeze, unfreeze}'
On Tue, Nov 20, 2018 at 10:34:19PM +0800, Gao Xiang wrote: > There are two minor issues in the current freeze interface: > >1) Freeze interfaces have not related with CONFIG_DEBUG_SPINLOCK, > therefore fix the incorrect conditions; > >2) For SMP platforms, it should also disable preemption before > doing atomic_cmpxchg in case that some high priority tasks > preempt between atomic_cmpxchg and disable_preempt, then spin > on the locked refcount later. spinning on a refcount implies that you are trying to do your own type of locking. Why not use the in-kernel locking api instead? It will always do better than trying to do your own logic as the developers there know locking across all types of cpus better than filesystem developers :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/10] staging: erofs: add a full barrier in erofs_workgroup_unfreeze
On Tue, Nov 20, 2018 at 10:34:20PM +0800, Gao Xiang wrote:
> Just like other generic locks, insert a full barrier
> in case of memory reorder.
>
> Reviewed-by: Chao Yu
> Signed-off-by: Gao Xiang
> ---
> drivers/staging/erofs/internal.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/erofs/internal.h
> b/drivers/staging/erofs/internal.h
> index 2e0ef92c138b..f77653d33633 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -209,6 +209,7 @@ static inline bool erofs_workgroup_try_to_freeze(struct
> erofs_workgroup *grp,
> static inline void erofs_workgroup_unfreeze(struct erofs_workgroup *grp,
> int orig_val)
> {
> + smp_mb();
Please document this memory barrier. It does not make much sense to
me...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/10] staging: erofs: separate into init_once / always
On Tue, Nov 20, 2018 at 10:34:22PM +0800, Gao Xiang wrote:
> `z_erofs_vle_workgroup' is heavily generated in the decompression,
> for example, it resets 32 bytes redundantly for 64-bit platforms
> even through Z_EROFS_VLE_INLINE_PAGEVECS + Z_EROFS_CLUSTER_MAX_PAGES,
> default 4, pages are stored in `z_erofs_vle_workgroup'.
>
> As an another example, `struct mutex' takes 72 bytes for our kirin
> 64-bit platforms, it's unnecessary to be reseted at first and
> be initialized each time.
>
> Let's avoid filling all `z_erofs_vle_workgroup' with 0 at first
> since most fields are reinitialized to meaningful values later,
> and pagevec is no need to initialized at all.
>
> Reviewed-by: Chao Yu
> Signed-off-by: Gao Xiang
> ---
> drivers/staging/erofs/unzip_vle.c | 34 +-
> 1 file changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/erofs/unzip_vle.c
> b/drivers/staging/erofs/unzip_vle.c
> index ede3383ac601..4e5843e8ee35 100644
> --- a/drivers/staging/erofs/unzip_vle.c
> +++ b/drivers/staging/erofs/unzip_vle.c
> @@ -43,12 +43,38 @@ static inline int init_unzip_workqueue(void)
> return z_erofs_workqueue ? 0 : -ENOMEM;
> }
>
> +static void init_once(void *ptr)
> +{
> + struct z_erofs_vle_workgroup *grp = ptr;
> + struct z_erofs_vle_work *const work =
> + z_erofs_vle_grab_primary_work(grp);
> + unsigned int i;
> +
> + mutex_init(&work->lock);
> + work->nr_pages = 0;
> + work->vcnt = 0;
> + for (i = 0; i < Z_EROFS_CLUSTER_MAX_PAGES; ++i)
> + grp->compressed_pages[i] = NULL;
> +}
> +
> +static void init_always(struct z_erofs_vle_workgroup *grp)
> +{
> + struct z_erofs_vle_work *const work =
> + z_erofs_vle_grab_primary_work(grp);
> +
> + atomic_set(&grp->obj.refcount, 1);
> + grp->flags = 0;
> +
> + DBG_BUGON(work->nr_pages);
> + DBG_BUGON(work->vcnt);
How can these ever be triggered? I understand the need for debugging
code when you are writing code, but at this point it shouldn't be needed
anymore, right?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 08/10] staging: erofs: locked before registering for all new workgroups
On Tue, Nov 20, 2018 at 10:34:23PM +0800, Gao Xiang wrote: > Let's make sure that the one registering a workgroup will also > take the primary work lock at first for two reasons: > 1) There's no need to introduce such a race window (and consequently > overhead) between registering and locking, other tasks could break > in by chance, and the race seems unnecessary (no benefit at all); > > 2) It's better to take the primary work when a workgroup > is registered to apply the cache managed policy, for example, > if some other tasks break in, it could turn into the in-place > decompression rather than use as the cached decompression. > > Reviewed-by: Chao Yu > Signed-off-by: Gao Xiang > --- > drivers/staging/erofs/unzip_vle.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/erofs/unzip_vle.c > b/drivers/staging/erofs/unzip_vle.c > index 4e5843e8ee35..a1376f3c6065 100644 > --- a/drivers/staging/erofs/unzip_vle.c > +++ b/drivers/staging/erofs/unzip_vle.c > @@ -420,18 +420,22 @@ z_erofs_vle_work_register(const struct > z_erofs_vle_work_finder *f, > work = z_erofs_vle_grab_primary_work(grp); > work->pageofs = f->pageofs; > > + /* lock all primary followed works before visible to others */ > + if (unlikely(!mutex_trylock(&work->lock))) > + /* for a new workgroup, try_lock *never* fails */ > + DBG_BUGON(1); Again, drop this, if it never fails, then there's no need for this. If it can fail, then properly handle it. And trylock can fail, so this needs to be fixed. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/10] staging: erofs: fix `erofs_workgroup_{try_to_freeze, unfreeze}'
Hi Greg, On 2018/11/22 18:21, Greg Kroah-Hartman wrote: > On Tue, Nov 20, 2018 at 10:34:19PM +0800, Gao Xiang wrote: >> There are two minor issues in the current freeze interface: >> >>1) Freeze interfaces have not related with CONFIG_DEBUG_SPINLOCK, >> therefore fix the incorrect conditions; >> >>2) For SMP platforms, it should also disable preemption before >> doing atomic_cmpxchg in case that some high priority tasks >> preempt between atomic_cmpxchg and disable_preempt, then spin >> on the locked refcount later. > > spinning on a refcount implies that you are trying to do your own type > of locking. Why not use the in-kernel locking api instead? It will > always do better than trying to do your own logic as the developers > there know locking across all types of cpus better than filesystem > developers :) It is because refcount also plays a role as a spinlock on a specific value (== EROFS_LOCKED_MAGIC), no need to introduce such a value since the spin window is small. Thanks, Gao Xiang > > thanks, > > greg k-h > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/10] staging: erofs: separate into init_once / always
Hi Greg, On 2018/11/22 18:23, Greg Kroah-Hartman wrote: >> + >> +DBG_BUGON(work->nr_pages); >> +DBG_BUGON(work->vcnt); > How can these ever be triggered? I understand the need for debugging > code when you are writing code, but at this point it shouldn't be needed > anymore, right? I need to avoid some fields is not 0 when the new workgroup is created (because work->nr_pages and work->vcnt == 0 usually after the previous workgroup is freed). But that is not obvious, it is promised by the current logic. In order to not introduce such a issue in the future, or there are some potential race (work->nr_pages and work->vcnt != 0 when the previous workgroup == 0), it need to be noticed to developpers as early as possible. Thanks, Gao Xiang > > thanks, > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 08/10] staging: erofs: locked before registering for all new workgroups
Hi Greg, On 2018/11/22 18:24, Greg Kroah-Hartman wrote: >> +/* lock all primary followed works before visible to others */ >> +if (unlikely(!mutex_trylock(&work->lock))) >> +/* for a new workgroup, try_lock *never* fails */ >> +DBG_BUGON(1); > Again, drop this, if it never fails, then there's no need for this. If > it can fail, then properly handle it. > > And trylock can fail, so this needs to be fixed. OK, I will drop this. Thanks, Gao Xiang > > thanks, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/10] staging: erofs: fix race when the managed cache is enabled
Hi Greg,
On 2018/11/22 18:17, Greg Kroah-Hartman wrote:
> Any specific reason why you are not using the refcount.h api instead of
> "doing it yourself" with atomic_inc/dec()?
>
> I'm not rejecting this, just curious.
As I explained in the previous email,
Re: [PATCH 04/10] staging: erofs: fix `erofs_workgroup_{try_to_freeze,
unfreeze}'
we need such a function when the value is >= 0, it plays as a refcount,
but when the refcount == EROFS_LOCKED_MAGIC (<0, but not 0 as refcount.h),
and actually there is no need to introduce a seperate spinlock_t because
we don't actually care about its performance (rarely locked). and
the corresponding struct is too large for now, we need to decrease its size.
Thanks,
Gao Xiang
>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vboxvideo: Rename uint32_t type to u32
Issue found by checkpatch.
CHECK: Prefer kernel type 'u32' over 'uint32_t'
+static const uint32_t vbox_cursor_plane_formats[] = {
CHECK: Prefer kernel type 'u32' over 'uint32_t'
+static const uint32_t vbox_primary_plane_formats[] = {
CHECK: Prefer kernel type 'u32' over 'uint32_t'
+ const uint32_t *formats;
Signed-off-by: Ricardo Reis Marques Silva
---
drivers/staging/vboxvideo/vbox_mode.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_mode.c
b/drivers/staging/vboxvideo/vbox_mode.c
index 06e921844b1e..c43bec4628ae 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -479,7 +479,7 @@ static void vbox_cursor_cleanup_fb(struct drm_plane *plane,
vbox_bo_unpin(bo);
}
-static const uint32_t vbox_cursor_plane_formats[] = {
+static const u32 vbox_cursor_plane_formats[] = {
DRM_FORMAT_ARGB,
};
@@ -500,7 +500,7 @@ static const struct drm_plane_funcs vbox_cursor_plane_funcs
= {
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
};
-static const uint32_t vbox_primary_plane_formats[] = {
+static const u32 vbox_primary_plane_formats[] = {
DRM_FORMAT_XRGB,
DRM_FORMAT_ARGB,
};
@@ -529,7 +529,7 @@ static struct drm_plane *vbox_create_plane(struct
vbox_private *vbox,
const struct drm_plane_helper_funcs *helper_funcs = NULL;
const struct drm_plane_funcs *funcs;
struct drm_plane *plane;
- const uint32_t *formats;
+ const u32 *formats;
int num_formats;
int err;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/10] staging: erofs: fix `trace_erofs_readpage' position
Hi Greg,
On 2018/11/22 18:19, Greg Kroah-Hartman wrote:
> On Tue, Nov 20, 2018 at 10:34:16PM +0800, Gao Xiang wrote:
>> `trace_erofs_readpage' should be placed in .readpage()
>> rather than in the internal `z_erofs_do_read_page'.
> Why? What happens with the code today?
trace_erofs_readpage is used to trace .readpage() interface (it represents sync
read)
hook rather than its internal implementation z_erofs_do_read_page (which both
.readpage()
and .readpages() uses it). Chen Gong places the tracepoint to a wrong place by
mistake.
And we found it by our internal test using this tracepoint.
>
>> Fixes: 284db12cfda3 ("staging: erofs: add trace points for reading zipped
>> data")
> Should this get into 4.20-final?
I think so, which is not very important but I think it should be fixed...
Thanks,
Gao Xiang
>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/10] staging: erofs: add a full barrier in erofs_workgroup_unfreeze
Hi Greg, On 2018/11/22 18:22, Greg Kroah-Hartman wrote: > Please document this memory barrier. It does not make much sense to > me... Because we need to make the other observers noticing the latest values modified in this locking period before unfreezing the whole workgroup, one way is to use a memory barrier and the other way is to use ACQUIRE and RELEASE. we selected the first one. Hmmm...ok, I will add a simple message to explain this, but I think that is plain enough for a lock... Thanks, Gao Xiang > > thanks, > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/10] staging: erofs: fix `trace_erofs_readpage' position
On Thu, Nov 22, 2018 at 06:49:53PM +0800, Gao Xiang wrote: > Hi Greg, > > On 2018/11/22 18:19, Greg Kroah-Hartman wrote: > > On Tue, Nov 20, 2018 at 10:34:16PM +0800, Gao Xiang wrote: > >> `trace_erofs_readpage' should be placed in .readpage() > >> rather than in the internal `z_erofs_do_read_page'. > > Why? What happens with the code today? > trace_erofs_readpage is used to trace .readpage() interface (it represents > sync read) > hook rather than its internal implementation z_erofs_do_read_page (which both > .readpage() > and .readpages() uses it). Chen Gong places the tracepoint to a wrong place > by mistake. > And we found it by our internal test using this tracepoint. Ok, you should put this in the changelog text when you redo this series. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/10] staging: erofs: fix `erofs_workgroup_{try_to_freeze, unfreeze}'
On Thu, Nov 22, 2018 at 06:29:34PM +0800, Gao Xiang wrote: > Hi Greg, > > On 2018/11/22 18:21, Greg Kroah-Hartman wrote: > > On Tue, Nov 20, 2018 at 10:34:19PM +0800, Gao Xiang wrote: > >> There are two minor issues in the current freeze interface: > >> > >>1) Freeze interfaces have not related with CONFIG_DEBUG_SPINLOCK, > >> therefore fix the incorrect conditions; > >> > >>2) For SMP platforms, it should also disable preemption before > >> doing atomic_cmpxchg in case that some high priority tasks > >> preempt between atomic_cmpxchg and disable_preempt, then spin > >> on the locked refcount later. > > > > spinning on a refcount implies that you are trying to do your own type > > of locking. Why not use the in-kernel locking api instead? It will > > always do better than trying to do your own logic as the developers > > there know locking across all types of cpus better than filesystem > > developers :) > > It is because refcount also plays a role as a spinlock on a specific value > (== EROFS_LOCKED_MAGIC), no need to introduce such a value since the spin > window is small. Do not try to overload a reference count as a spinlock, you will run into problems and in the end have to implement everything that the core locking code has done. Your use of this is highly suspect, what happens when you use a "real" spinlock and a "real" reference count instead? Those are two different things from a logical point of view and you are mixing them together which is odd. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/10] staging: erofs: fix `erofs_workgroup_{try_to_freeze, unfreeze}'
On Thu, Nov 22, 2018 at 06:29:34PM +0800, Gao Xiang wrote: > Hi Greg, > > On 2018/11/22 18:21, Greg Kroah-Hartman wrote: > > On Tue, Nov 20, 2018 at 10:34:19PM +0800, Gao Xiang wrote: > >> There are two minor issues in the current freeze interface: > >> > >>1) Freeze interfaces have not related with CONFIG_DEBUG_SPINLOCK, > >> therefore fix the incorrect conditions; > >> > >>2) For SMP platforms, it should also disable preemption before > >> doing atomic_cmpxchg in case that some high priority tasks > >> preempt between atomic_cmpxchg and disable_preempt, then spin > >> on the locked refcount later. > > > > spinning on a refcount implies that you are trying to do your own type > > of locking. Why not use the in-kernel locking api instead? It will > > always do better than trying to do your own logic as the developers > > there know locking across all types of cpus better than filesystem > > developers :) > > It is because refcount also plays a role as a spinlock on a specific value > (== EROFS_LOCKED_MAGIC), no need to introduce such a value since the spin > window is small. As I said in another email, doing two things with one variable is odd as those are two different types of functions. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/10] staging: erofs: separate into init_once / always
On Thu, Nov 22, 2018 at 06:34:10PM +0800, Gao Xiang wrote: > Hi Greg, > > On 2018/11/22 18:23, Greg Kroah-Hartman wrote: > >> + > >> + DBG_BUGON(work->nr_pages); > >> + DBG_BUGON(work->vcnt); > > How can these ever be triggered? I understand the need for debugging > > code when you are writing code, but at this point it shouldn't be needed > > anymore, right? > > I need to avoid some fields is not 0 when the new workgroup is created > (because > work->nr_pages and work->vcnt == 0 usually after the previous workgroup is > freed). > But that is not obvious, it is promised by the current logic. Then delete these lines if they can never happen :) > In order to not introduce such a issue in the future, or there are some > potential > race (work->nr_pages and work->vcnt != 0 when the previous workgroup == 0), > it need > to be noticed to developpers as early as possible. Then make it a real call, do not wrap it in odd macros that do not really explain why it is "debugging only". Your code is "real" now, make the logic real for all developers and users. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/10] staging: erofs: fix race when the managed cache is enabled
On Thu, Nov 22, 2018 at 06:42:52PM +0800, Gao Xiang wrote:
> Hi Greg,
>
> On 2018/11/22 18:17, Greg Kroah-Hartman wrote:
> > Any specific reason why you are not using the refcount.h api instead of
> > "doing it yourself" with atomic_inc/dec()?
> >
> > I'm not rejecting this, just curious.
>
> As I explained in the previous email,
> Re: [PATCH 04/10] staging: erofs: fix `erofs_workgroup_{try_to_freeze,
> unfreeze}'
>
> we need such a function when the value is >= 0, it plays as a refcount,
> but when the refcount == EROFS_LOCKED_MAGIC (<0, but not 0 as refcount.h),
> and actually there is no need to introduce a seperate spinlock_t because
> we don't actually care about its performance (rarely locked). and
> the corresponding struct is too large for now, we need to decrease its size.
Why do you need to decrease the size? How many of these structures are
created?
And you will care about the performance when a lock is being held, as is
evident by your logic to try to fix those issues in this patch series.
Using a "real" lock will solve all of that and keep you from having to
implement it all "by hand".
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/10] staging: erofs: separate into init_once / always
Hi Greg, On 2018/11/22 19:05, Greg Kroah-Hartman wrote: > On Thu, Nov 22, 2018 at 06:34:10PM +0800, Gao Xiang wrote: >> Hi Greg, >> >> On 2018/11/22 18:23, Greg Kroah-Hartman wrote: + + DBG_BUGON(work->nr_pages); + DBG_BUGON(work->vcnt); >>> How can these ever be triggered? I understand the need for debugging >>> code when you are writing code, but at this point it shouldn't be needed >>> anymore, right? >> >> I need to avoid some fields is not 0 when the new workgroup is created >> (because >> work->nr_pages and work->vcnt == 0 usually after the previous workgroup is >> freed). >> But that is not obvious, it is promised by the current logic. > > Then delete these lines if they can never happen :) I don't know how to observe such a race in our beta test and community users. Because if the kernel is crashed, we could collect the whole kernel dump to observe the memory and all registers, if we only have some warning, it will be not easy to get the state as early as possible. Thank, Gao Xiang > >> In order to not introduce such a issue in the future, or there are some >> potential >> race (work->nr_pages and work->vcnt != 0 when the previous workgroup == 0), >> it need >> to be noticed to developpers as early as possible. > > Then make it a real call, do not wrap it in odd macros that do not > really explain why it is "debugging only". Your code is "real" now, > make the logic real for all developers and users. > > thanks, > > greg k-h > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad7780: Add gain & filter gpio support
On Mi, 2018-11-21 at 16:04 -0200, Giuliano Belinassi wrote:
> Previously, the AD7780 driver only supported gpio for the 'powerdown'
> pin. This commit adds suppport for the 'gain' and 'filter' pin.
Hey,
Comments inline.
>
> Signed-off-by: Giuliano Belinassi
> ---
> drivers/staging/iio/adc/ad7780.c | 61 --
> include/linux/iio/adc/ad_sigma_delta.h | 5 +++
> 2 files changed, 62 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index c4a85789c2db..69794f06dbcd 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -39,6 +39,9 @@
> #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2)
> #define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 |
> AD7170_PAT2)
>
> +#define AD7780_GAIN_GPIO 0
> +#define AD7780_FILTER_GPIO 1
> +
> struct ad7780_chip_info {
> struct iio_chan_specchannel;
> unsigned intpattern_mask;
> @@ -50,6 +53,8 @@ struct ad7780_state {
> const struct ad7780_chip_info *chip_info;
> struct regulator*reg;
> struct gpio_desc*powerdown_gpio;
> + struct gpio_desc*gain_gpio;
> + struct gpio_desc*filter_gpio;
> unsigned intgain;
>
> struct ad_sigma_delta sd;
> @@ -115,18 +120,51 @@ static int ad7780_read_raw(struct iio_dev
> *indio_dev,
> return -EINVAL;
> }
>
> +static int ad7780_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
> + long m)
> +{
> + struct ad7780_state *st = iio_priv(indio_dev);
> +
> + if (m != IIO_CHAN_INFO_RAW)
> + return -EINVAL;
> +
> + if (st->chip_info->is_ad778x) {
> + switch(val) {
> + case AD7780_GAIN_GPIO:
I think that instead of setting the gain directly, we should use
the IIO_CHAN_INFO_SCALE attribute. At page 12 of the ad7780 datasheet there
is a formula from which the output code can be calculated:
Code = 2^(N − 1)
× [(AIN × Gain /VREF) + 1]. So, by setting the scale from user space, the
driver can calculate the correct gain by using the formula above. Also, it
would be useful to introduce scale available.
Furthermore, there is a new
ad7124 adc driver which does this exact thing. Take a look here: https://gi
thub.com/analogdevicesinc/linux/blob/master/drivers/iio/adc/ad7124.c#L337.
> + gpiod_set_value(st->gain_gpio, val2);
> + break;
> + case AD7780_FILTER_GPIO:
The attribute that should be used to configure the filter gpio is
IIO_CHAN_INFO_SAMP_FREQ. So, we should have 10 Hz and 16.7 Hz available
sampling frequencies. If from user space the 10 Hz sampling freq is
requested, then we set the FILTER pin high, while for 16.7 Hz the FILTER
pin will be low.
> + gpiod_set_value(st->filter_gpio, val2);
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta,
> unsigned int raw_sample)
> {
> struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta);
> const struct ad7780_chip_info *chip_info = st->chip_info;
> + int val;
>
> if ((raw_sample & AD7780_ERR) ||
> ((raw_sample & chip_info->pattern_mask) != chip_info-
> >pattern))
> return -EIO;
>
> if (chip_info->is_ad778x) {
> - if (raw_sample & AD7780_GAIN)
> + val = raw_sample & AD7780_GAIN;
> +
> + if (val != gpiod_get_value(st->gain_gpio))
> + return -EIO;
> +
> + if (val)
> st->gain = 1;
> else
> st->gain = 128;
> @@ -141,18 +179,20 @@ static const struct ad_sigma_delta_info
> ad7780_sigma_delta_info = {
> .has_registers = false,
> };
>
> -#define AD7780_CHANNEL(bits, wordsize) \
> +#define AD7170_CHANNEL(bits, wordsize) \
> AD_SD_CHANNEL_NO_SAMP_FREQ(1, 0, 0, bits, 32, wordsize - bits)
> +#define AD7780_CHANNEL(bits, wordsize) \
> + AD_SD_CHANNEL_GAIN_FILTER(1, 0, 0, bits, 32, wordsize - bits)
>
> static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
> [ID_AD7170] = {
> - .channel = AD7780_CHANNEL(12, 24),
> + .channel = AD7170_CHANNEL(12, 24),
> .pattern = AD7170_PATTERN,
> .pattern_mask = AD7170_PATTERN_MASK,
> .is_ad778x = false,
> },
> [ID_AD7171] = {
> - .channel = AD7780_CHANNEL(16, 24),
> + .channel = AD7170_CHANNEL(16, 24),
> .pattern = AD7170_PATTERN,
> .pattern_mask = AD7170_PATTERN_MASK,
>
Re: [PATCH 04/10] staging: erofs: fix `erofs_workgroup_{try_to_freeze, unfreeze}'
Hi Greg, On 2018/11/22 19:05, Greg Kroah-Hartman wrote: > As I said in another email, doing two things with one variable is odd as > those are two different types of functions. I think lockref_put_or_lock do the similar thing, it is heavily used in dcache.c, but 1) 0 is special for us, we need lock it on a < 0 value rather than 0. 2) spinlock_t is too large for us because every compressed page will have the structure, but the locking rarely happens. Thanks, Gao Xiang > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/10] staging: erofs: separate into init_once / always
On Thu, Nov 22, 2018 at 07:11:08PM +0800, Gao Xiang wrote:
> Hi Greg,
>
> On 2018/11/22 19:05, Greg Kroah-Hartman wrote:
> > On Thu, Nov 22, 2018 at 06:34:10PM +0800, Gao Xiang wrote:
> >> Hi Greg,
> >>
> >> On 2018/11/22 18:23, Greg Kroah-Hartman wrote:
> +
> +DBG_BUGON(work->nr_pages);
> +DBG_BUGON(work->vcnt);
> >>> How can these ever be triggered? I understand the need for debugging
> >>> code when you are writing code, but at this point it shouldn't be needed
> >>> anymore, right?
> >>
> >> I need to avoid some fields is not 0 when the new workgroup is created
> >> (because
> >> work->nr_pages and work->vcnt == 0 usually after the previous workgroup is
> >> freed).
> >> But that is not obvious, it is promised by the current logic.
> >
> > Then delete these lines if they can never happen :)
>
> I don't know how to observe such a race in our beta test and community users.
/*
* Let developers know something went really wrong with their
* initialization code
*/
if (!work->nr_pages) {
pr_err("nr_pages == NULL!");
WARN_ON(1);
}
if (!work->vcnt) {
pr_err("vcnt == NULL!");
WARN_ON(1);
}
or something like that.
Don't make people rebuild your code with different options for
debugging. That will never work in the 'real world' when people start
using the code. You need to have things enabled for people all the
time, which is why we have dynamic debugging in the kernel now, and not
a zillion different "DRIVER_DEBUG" build options anymore.
> Because if the kernel is crashed, we could collect the whole kernel dump to
> observe the memory
> and all registers, if we only have some warning, it will be not easy to get
> the state as early as possible.
When the kernel crashes, geting a dump is hard on almost all hardware.
It is only rare systems that you can get a kernel dump.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/10] staging: erofs: separate into init_once / always
Hi Greg,
On 2018/11/22 19:26, Greg Kroah-Hartman wrote:
> On Thu, Nov 22, 2018 at 07:11:08PM +0800, Gao Xiang wrote:
>> Hi Greg,
>>
>> On 2018/11/22 19:05, Greg Kroah-Hartman wrote:
>>> On Thu, Nov 22, 2018 at 06:34:10PM +0800, Gao Xiang wrote:
Hi Greg,
On 2018/11/22 18:23, Greg Kroah-Hartman wrote:
>> +
>> +DBG_BUGON(work->nr_pages);
>> +DBG_BUGON(work->vcnt);
> How can these ever be triggered? I understand the need for debugging
> code when you are writing code, but at this point it shouldn't be needed
> anymore, right?
I need to avoid some fields is not 0 when the new workgroup is created
(because
work->nr_pages and work->vcnt == 0 usually after the previous workgroup is
freed).
But that is not obvious, it is promised by the current logic.
>>>
>>> Then delete these lines if they can never happen :)
>>
>> I don't know how to observe such a race in our beta test and community users.
>
> /*
>* Let developers know something went really wrong with their
>* initialization code
>*/
> if (!work->nr_pages) {
> pr_err("nr_pages == NULL!");
> WARN_ON(1);
> }
> if (!work->vcnt) {
> pr_err("vcnt == NULL!");
> WARN_ON(1);
> }
>
> or something like that.
>
> Don't make people rebuild your code with different options for
> debugging. That will never work in the 'real world' when people start
> using the code. You need to have things enabled for people all the
> time, which is why we have dynamic debugging in the kernel now, and not
> a zillion different "DRIVER_DEBUG" build options anymore.
>
>> Because if the kernel is crashed, we could collect the whole kernel dump to
>> observe the memory
>> and all registers, if we only have some warning, it will be not easy to get
>> the state as early as possible.
>
> When the kernel crashes, geting a dump is hard on almost all hardware.
> It is only rare systems that you can get a kernel dump.
This piece of code is already used in our hisilicon beta test, all memorydump,
registers, stack can be collected.
Apart from that, I observed many f2fs bugs are observed in this way and
many erofs bugs are collected in that way...sigh...
Thanks,
Gao Xiang
>
> thanks,
>
> greg k-h
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/10] staging: erofs: fix race when the managed cache is enabled
Hi Greg,
On 2018/11/22 19:06, Greg Kroah-Hartman wrote:
> On Thu, Nov 22, 2018 at 06:42:52PM +0800, Gao Xiang wrote:
>> Hi Greg,
>>
>> On 2018/11/22 18:17, Greg Kroah-Hartman wrote:
>>> Any specific reason why you are not using the refcount.h api instead of
>>> "doing it yourself" with atomic_inc/dec()?
>>>
>>> I'm not rejecting this, just curious.
>> As I explained in the previous email,
>> Re: [PATCH 04/10] staging: erofs: fix `erofs_workgroup_{try_to_freeze,
>> unfreeze}'
>>
>> we need such a function when the value is >= 0, it plays as a refcount,
>> but when the refcount == EROFS_LOCKED_MAGIC (<0, but not 0 as refcount.h),
>> and actually there is no need to introduce a seperate spinlock_t because
>> we don't actually care about its performance (rarely locked). and
>> the corresponding struct is too large for now, we need to decrease its size.
> Why do you need to decrease the size? How many of these structures are
> created?
As I said in the previous email, every compressed page will have a managed
structure
called erofs_workgroup, and it is heavily allocated like page/inode/dentry in
the erofs.
>
> And you will care about the performance when a lock is being held, as is
> evident by your logic to try to fix those issues in this patch series.
> Using a "real" lock will solve all of that and keep you from having to
> implement it all "by hand".
The function is much like lockref (aligned_u64 lock_count;) with the exception
as my previous email
explained.
Thanks,
Gao Xiang
>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/10] staging: erofs: separate into init_once / always
Hi Greg, On 2018/11/22 19:26, Greg Kroah-Hartman wrote: > Don't make people rebuild your code with different options for > debugging. That will never work in the 'real world' when people start > using the code. You need to have things enabled for people all the > time, which is why we have dynamic debugging in the kernel now, and not > a zillion different "DRIVER_DEBUG" build options anymore. Actually, current erofs handle differently for beta users (in eng mode) and commercial users. CONFIG_EROFS_FS_DEBUG is enable for all beta users, after an observed expression is false, we could get the whole memorydump as early as possible. But for commercial users, there are no such observing points to promise the kernel stability and performance. It has helped us to find several bug, and I cannot find some alternative way to get the the first scene of the accident... Thanks, Gao Xiang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/10] staging: erofs: fix race when the managed cache is enabled
On Thu, Nov 22, 2018 at 07:43:50PM +0800, Gao Xiang wrote:
> Hi Greg,
>
> On 2018/11/22 19:06, Greg Kroah-Hartman wrote:
> > On Thu, Nov 22, 2018 at 06:42:52PM +0800, Gao Xiang wrote:
> >> Hi Greg,
> >>
> >> On 2018/11/22 18:17, Greg Kroah-Hartman wrote:
> >>> Any specific reason why you are not using the refcount.h api instead of
> >>> "doing it yourself" with atomic_inc/dec()?
> >>>
> >>> I'm not rejecting this, just curious.
> >> As I explained in the previous email,
> >> Re: [PATCH 04/10] staging: erofs: fix `erofs_workgroup_{try_to_freeze,
> >> unfreeze}'
> >>
> >> we need such a function when the value is >= 0, it plays as a refcount,
> >> but when the refcount == EROFS_LOCKED_MAGIC (<0, but not 0 as refcount.h),
> >> and actually there is no need to introduce a seperate spinlock_t because
> >> we don't actually care about its performance (rarely locked). and
> >> the corresponding struct is too large for now, we need to decrease its
> >> size.
> > Why do you need to decrease the size? How many of these structures are
> > created?
>
> As I said in the previous email, every compressed page will have a managed
> structure
> called erofs_workgroup, and it is heavily allocated like page/inode/dentry in
> the erofs.
Ugh, every page? Ok, nevermind, I take back my objections. You all are
crazy and need to do crazy things like this :)
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/10] staging: erofs: fix race when the managed cache is enabled
Hi Greg, On 2018/11/22 20:26, Greg Kroah-Hartman wrote: > Ugh, every page? Ok, nevermind, I take back my objections. You all are > crazy and need to do crazy things like this :) ...Do you have some idea about this? ... I think it is fairly normal... :( We have a large number of managed workgroups, the current use case is the 4k compressed size, each compressed page has a workgroup, but erofs also has reclaim paths to free them for low memory cases. But since the structure (z_erofs_vle_workgroup, erofs_workgroup) is critical, I need to make it as small as possible. Thanks, Gao Xiang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: ad5933: replaced kfifo by triggered_buffer
Previously, there was an implicit creation of a kfifo which was replaced
by a call to triggered_buffer_setup, which is already implemented in iio
infrastructure.
Signed-off-by: Marcelo Schmitt
---
.../staging/iio/impedance-analyzer/Kconfig| 2 +-
.../staging/iio/impedance-analyzer/ad5933.c | 25 ---
2 files changed, 6 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/Kconfig
b/drivers/staging/iio/impedance-analyzer/Kconfig
index dd97b6bb3fd0..d0af5aa55dc0 100644
--- a/drivers/staging/iio/impedance-analyzer/Kconfig
+++ b/drivers/staging/iio/impedance-analyzer/Kconfig
@@ -7,7 +7,7 @@ config AD5933
tristate "Analog Devices AD5933, AD5934 driver"
depends on I2C
select IIO_BUFFER
- select IIO_KFIFO_BUF
+ select IIO_TRIGGERED_BUFFER
help
Say yes here to build support for Analog Devices Impedance Converter,
Network Analyzer, AD5933/4, provides direct access via sysfs.
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index f9bcb8310e21..edb8b540bbf1 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -20,7 +20,7 @@
#include
#include
#include
-#include
+#include
/* AD5933/AD5934 Registers */
#define AD5933_REG_CONTROL_HB 0x80/* R/W, 1 byte */
@@ -615,22 +615,6 @@ static const struct iio_buffer_setup_ops
ad5933_ring_setup_ops = {
.postdisable = ad5933_ring_postdisable,
};
-static int ad5933_register_ring_funcs_and_init(struct iio_dev *indio_dev)
-{
- struct iio_buffer *buffer;
-
- buffer = iio_kfifo_allocate();
- if (!buffer)
- return -ENOMEM;
-
- iio_device_attach_buffer(indio_dev, buffer);
-
- /* Ring buffer functions - here trigger setup related */
- indio_dev->setup_ops = &ad5933_ring_setup_ops;
-
- return 0;
-}
-
static void ad5933_work(struct work_struct *work)
{
struct ad5933_state *st = container_of(work,
@@ -744,7 +728,8 @@ static int ad5933_probe(struct i2c_client *client,
indio_dev->channels = ad5933_channels;
indio_dev->num_channels = ARRAY_SIZE(ad5933_channels);
- ret = ad5933_register_ring_funcs_and_init(indio_dev);
+ ret = iio_triggered_buffer_setup(indio_dev, NULL, NULL,
+&ad5933_ring_setup_ops);
if (ret)
goto error_disable_reg;
@@ -759,7 +744,7 @@ static int ad5933_probe(struct i2c_client *client,
return 0;
error_unreg_ring:
- iio_kfifo_free(indio_dev->buffer);
+ iio_triggered_buffer_cleanup(indio_dev);
error_disable_reg:
regulator_disable(st->reg);
@@ -772,7 +757,7 @@ static int ad5933_remove(struct i2c_client *client)
struct ad5933_state *st = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
- iio_kfifo_free(indio_dev->buffer);
+ iio_triggered_buffer_cleanup(indio_dev);
regulator_disable(st->reg);
return 0;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/10] staging: erofs: separate into init_once / always
Hi Greg, On 2018/11/22 20:00, Gao Xiang wrote: > Hi Greg, > > On 2018/11/22 19:26, Greg Kroah-Hartman wrote: >> Don't make people rebuild your code with different options for >> debugging. That will never work in the 'real world' when people start >> using the code. You need to have things enabled for people all the >> time, which is why we have dynamic debugging in the kernel now, and not >> a zillion different "DRIVER_DEBUG" build options anymore. > Actually, current erofs handle differently for beta users (in eng mode) > and commercial users. > > CONFIG_EROFS_FS_DEBUG is enable for all beta users, after an observed > expression is false, we could get the whole memorydump as early as possible. > But for commercial users, there are no such observing points to promise > the kernel stability and performance. > > It has helped us to find several bug, and I cannot find some alternative way > to get the the first scene of the accident... I'm about to send v2 of this patchset... I need to get your opinion... I think for the current erofs development state, I tend to leave such a developping switch because the code could be modified frequently, many potential bugs could be avoid when this debugging mode is on and it will be dropped after erofs becomes more stable... Thanks, Gao Xiang > > Thanks, > Gao Xiang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/10] staging: erofs: fix race when the managed cache is enabled
On Thu, Nov 22, 2018 at 08:41:15PM +0800, Gao Xiang wrote: > Hi Greg, > > On 2018/11/22 20:26, Greg Kroah-Hartman wrote: > > Ugh, every page? Ok, nevermind, I take back my objections. You all are > > crazy and need to do crazy things like this :) > > ...Do you have some idea about this? ... I think it is fairly normal... :( > > We have a large number of managed workgroups, the current use case is the 4k > compressed size, > each compressed page has a workgroup, but erofs also has reclaim paths to > free them for low memory cases. > > But since the structure (z_erofs_vle_workgroup, erofs_workgroup) is critical, > I need to make it as small as possible. I do not know "real" filesystems (i.e. ones that put stuff on disks), only virtual ones (like sysfs/kernfs), so I do not know what to suggest here, sorry. I thought this was a much higner-level structure, if this is per-compressed-page, then you are more than welcome to optimize for this. Good luck with your "fake" spinlocks though :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/10] staging: erofs: separate into init_once / always
On Thu, Nov 22, 2018 at 09:01:31PM +0800, Gao Xiang wrote: > Hi Greg, > > On 2018/11/22 20:00, Gao Xiang wrote: > > Hi Greg, > > > > On 2018/11/22 19:26, Greg Kroah-Hartman wrote: > >> Don't make people rebuild your code with different options for > >> debugging. That will never work in the 'real world' when people start > >> using the code. You need to have things enabled for people all the > >> time, which is why we have dynamic debugging in the kernel now, and not > >> a zillion different "DRIVER_DEBUG" build options anymore. > > Actually, current erofs handle differently for beta users (in eng mode) > > and commercial users. > > > > CONFIG_EROFS_FS_DEBUG is enable for all beta users, after an observed > > expression is false, we could get the whole memorydump as early as possible. > > But for commercial users, there are no such observing points to promise > > the kernel stability and performance. > > > > It has helped us to find several bug, and I cannot find some alternative way > > to get the the first scene of the accident... > > I'm about to send v2 of this patchset... I need to get your opinion... > > I think for the current erofs development state, I tend to leave such a > developping switch because the code could be modified frequently, > many potential bugs could be avoid when this debugging mode is on and > it will be dropped after erofs becomes more stable... You have two competing issues here. You need to have some sort of debug build that allows you to get feedback from users, and you need to provide a solid, reliable system for those not wanting to debug anything. If you use a kernel build option, then you can do what you are doing now, just that do not expect for any user that has problems with the filesystem, they will rebuild the code just to try to help debug it. That is going to require run-time debugging to be enabled as they will not usually have built their own kernel/module at all. For now, keep doing what you are doing, I just started to worry when I saw the initial BUG_ON() as we had just talked about these at the Maintainers summit a few weeks ago, and how we need to get rid of them from the kernel as they are a crutch that we need to solve. thanks, and sorry for the noise. Please resend this series again, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/8] staging: rtl8188eu: cleanup line ending with a '('
Cleanup line ending with a '(' to follow kernel coding style.
Reported by checkpatch.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
index f8d2b141a6c6..d792301a5ac2 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
@@ -155,13 +155,11 @@ static int recvbuf2recvframe(struct adapter *adapt,
struct sk_buff *pskb)
handle_txrpt_ccx_88e(adapt, precvframe->pkt->data);
rtw_free_recvframe(precvframe, pfree_recv_queue);
} else if (pattrib->pkt_rpt_type == TX_REPORT2) {
- ODM_RA_TxRPT2Handle_8188E(
- &haldata->odmpriv,
- precvframe->pkt->data,
- pattrib->pkt_len,
- pattrib->MacIDValidEntry[0],
- pattrib->MacIDValidEntry[1]
- );
+ ODM_RA_TxRPT2Handle_8188E(&haldata->odmpriv,
+ precvframe->pkt->data,
+ pattrib->pkt_len,
+ pattrib->MacIDValidEntry[0],
+ pattrib->MacIDValidEntry[1]);
rtw_free_recvframe(precvframe, pfree_recv_queue);
} else if (pattrib->pkt_rpt_type == HIS_REPORT) {
interrupt_handler_8188eu(adapt, pattrib->pkt_len,
precvframe->pkt->data);
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/8] staging: rtl8188eu: use __func__ in usb_ops_linux.c
Use __func__ instead of hardcoded function names.
Reported by checkpatch.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/os_dep/usb_ops_linux.c | 84 ---
1 file changed, 53 insertions(+), 31 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
index d6a499692e96..f8d2b141a6c6 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
@@ -52,14 +52,16 @@ static int recvbuf2recvframe(struct adapter *adapt, struct
sk_buff *pskb)
do {
RT_TRACE(_module_rtl871x_recv_c_, _drv_info_,
-("recvbuf2recvframe: rxdesc=offsset 0:0x%08x,
4:0x%08x, 8:0x%08x, C:0x%08x\n",
- prxstat->rxdw0, prxstat->rxdw1, prxstat->rxdw2,
prxstat->rxdw4));
+("%s: rxdesc=offsset 0:0x%08x, 4:0x%08x, 8:0x%08x,
C:0x%08x\n",
+ __func__, prxstat->rxdw0, prxstat->rxdw1,
+ prxstat->rxdw2, prxstat->rxdw4));
prxstat = (struct recv_stat *)pbuf;
precvframe = rtw_alloc_recvframe(pfree_recv_queue);
if (!precvframe) {
- RT_TRACE(_module_rtl871x_recv_c_, _drv_err_,
("recvbuf2recvframe: precvframe==NULL\n"));
+ RT_TRACE(_module_rtl871x_recv_c_, _drv_err_,
+("%s: precvframe==NULL\n", __func__));
DBG_88E("%s()-%d: rtw_alloc_recvframe() failed! RX
Drop!\n", __func__, __LINE__);
goto _exit_recvbuf2recvframe;
}
@@ -83,7 +85,8 @@ static int recvbuf2recvframe(struct adapter *adapt, struct
sk_buff *pskb)
pkt_offset = RXDESC_SIZE + pattrib->drvinfo_sz +
pattrib->shift_sz + pattrib->pkt_len;
if ((pattrib->pkt_len <= 0) || (pkt_offset > transfer_len)) {
- RT_TRACE(_module_rtl871x_recv_c_, _drv_info_,
("recvbuf2recvframe: pkt_len<=0\n"));
+ RT_TRACE(_module_rtl871x_recv_c_, _drv_info_,
+("%s: pkt_len<=0\n", __func__));
DBG_88E("%s()-%d: RX Warning!,pkt_len<=0 or pkt_offset>
transfer_len\n", __func__, __LINE__);
rtw_free_recvframe(precvframe, pfree_recv_queue);
goto _exit_recvbuf2recvframe;
@@ -121,7 +124,8 @@ static int recvbuf2recvframe(struct adapter *adapt, struct
sk_buff *pskb)
memcpy(pkt_copy->data, (pbuf + pattrib->drvinfo_sz +
RXDESC_SIZE), skb_len);
skb_put(precvframe->pkt, skb_len);
} else {
- DBG_88E("recvbuf2recvframe: alloc_skb fail , drop frag
frame\n");
+ DBG_88E("%s: alloc_skb fail , drop frag frame\n",
+ __func__);
rtw_free_recvframe(precvframe, pfree_recv_queue);
goto _exit_recvbuf2recvframe;
}
@@ -143,7 +147,8 @@ static int recvbuf2recvframe(struct adapter *adapt, struct
sk_buff *pskb)
update_recvframe_phyinfo_88e(precvframe,
pphy_status);
if (rtw_recv_entry(precvframe) != _SUCCESS) {
RT_TRACE(_module_rtl871x_recv_c_, _drv_err_,
- ("recvbuf2recvframe:
rtw_recv_entry(precvframe) != _SUCCESS\n"));
+("%s: rtw_recv_entry(precvframe) !=
_SUCCESS\n",
+__func__));
}
} else if (pattrib->pkt_rpt_type == TX_REPORT1) {
/* CCX-TXRPT ack for xmit mgmt frames. */
@@ -206,7 +211,9 @@ static int usbctrl_vendorreq(struct adapter *adapt, u8
request, u16 value, u16 i
int vendorreq_times = 0;
if ((adapt->bSurpriseRemoved) || (adapt->pwrctrlpriv.pnp_bstop_trx)) {
- RT_TRACE(_module_hci_ops_os_c_, _drv_err_,
("usbctrl_vendorreq:(adapt->bSurpriseRemoved
||adapter->pwrctrlpriv.pnp_bstop_trx)!!!\n"));
+ RT_TRACE(_module_hci_ops_os_c_, _drv_err_,
+("%s:(adapt->bSurpriseRemoved
||adapter->pwrctrlpriv.pnp_bstop_trx)!!!\n",
+ __func__));
status = -EPERM;
goto exit;
}
@@ -348,12 +355,13 @@ static void usb_read_port_complete(struct urb *purb,
struct pt_regs *regs)
struct adapter *adapt = (struct adapter *)precvbuf->adapter;
struct recv_priv *precvpriv = &adapt->recvpriv;
- RT_TRACE(_module_hci_ops_os_c_, _drv_err_,
("usb_read_port_complete!!!\n"));
+ RT_TRACE(_module_hci_ops_os_c_, _drv_err_, ("%s!!!\n", __func__));
if (adapt->bSurpriseRemoved || adapt->bDriverStopped ||
adapt->bReadPortCancel) {
RT_TRACE(_module_hci_ops_os_c_, _drv_err_,
[PATCH 4/8] staging: rtl8188eu: correct spelling mistake in a comment
Correct spelling mistake in a comment reported by checkpatch. checksumed -> checksummed Signed-off-by: Michael Straube --- drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c index 5a41766bba68..75b7d8c2cc50 100644 --- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c @@ -273,7 +273,7 @@ static int usbctrl_vendorreq(struct adapter *adapt, u8 request, u16 value, u16 i } } - /* firmware download is checksumed, don't retry */ + /* firmware download is checksummed, don't retry */ if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || status == len) break; } -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/8] staging: rtl8188eu: cleanup declarations in usb_ops_linux.c
Replace tabs with spaces and/or remove spaces in declarations
to cleanup whitespace.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/os_dep/usb_ops_linux.c | 40 +--
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
index 4821ce2fa762..9a732adbf1b1 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
@@ -31,18 +31,18 @@ static void interrupt_handler_8188eu(struct adapter *adapt,
u16 pkt_len, u8 *pbu
static int recvbuf2recvframe(struct adapter *adapt, struct sk_buff *pskb)
{
- u8 *pbuf;
- u8 shift_sz = 0;
- u16 pkt_cnt;
- u32 pkt_offset, skb_len, alloc_sz;
- s32 transfer_len;
- struct recv_stat*prxstat;
- struct phy_stat *pphy_status = NULL;
+ u8 *pbuf;
+ u8 shift_sz = 0;
+ u16 pkt_cnt;
+ u32 pkt_offset, skb_len, alloc_sz;
+ s32 transfer_len;
+ struct recv_stat *prxstat;
+ struct phy_stat *pphy_status = NULL;
struct sk_buff *pkt_copy = NULL;
- struct recv_frame *precvframe = NULL;
- struct rx_pkt_attrib*pattrib = NULL;
+ struct recv_frame *precvframe = NULL;
+ struct rx_pkt_attrib *pattrib = NULL;
struct hal_data_8188e *haldata = adapt->HalData;
- struct recv_priv*precvpriv = &adapt->recvpriv;
+ struct recv_priv *precvpriv = &adapt->recvpriv;
struct __queue *pfree_recv_queue = &precvpriv->free_recv_queue;
transfer_len = (s32)pskb->len;
@@ -201,7 +201,7 @@ unsigned int ffaddr2pipehdl(struct dvobj_priv *pdvobj, u32
addr)
static int usbctrl_vendorreq(struct adapter *adapt, u8 request, u16 value, u16
index, void *pdata, u16 len, u8 requesttype)
{
- struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
+ struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
struct usb_device *udev = dvobjpriv->pusbdev;
unsigned int pipe;
int status = 0;
@@ -349,8 +349,8 @@ u32 usb_read32(struct adapter *adapter, u32 addr)
static void usb_read_port_complete(struct urb *purb, struct pt_regs *regs)
{
- struct recv_buf *precvbuf = (struct recv_buf *)purb->context;
- struct adapter *adapt = (struct adapter *)precvbuf->adapter;
+ struct recv_buf *precvbuf = (struct recv_buf *)purb->context;
+ struct adapter *adapt = (struct adapter *)precvbuf->adapter;
struct recv_priv *precvpriv = &adapt->recvpriv;
RT_TRACE(_module_hci_ops_os_c_, _drv_err_, ("%s!!!\n", __func__));
@@ -426,9 +426,9 @@ static void usb_read_port_complete(struct urb *purb, struct
pt_regs *regs)
u32 usb_read_port(struct adapter *adapter, u32 addr, struct recv_buf *precvbuf)
{
struct urb *purb = NULL;
- struct dvobj_priv *pdvobj = adapter_to_dvobj(adapter);
- struct recv_priv*precvpriv = &adapter->recvpriv;
- struct usb_device *pusbd = pdvobj->pusbdev;
+ struct dvobj_priv *pdvobj = adapter_to_dvobj(adapter);
+ struct recv_priv *precvpriv = &adapter->recvpriv;
+ struct usb_device *pusbd = pdvobj->pusbdev;
int err;
unsigned int pipe;
u32 ret = _SUCCESS;
@@ -573,8 +573,8 @@ int usb_write32(struct adapter *adapter, u32 addr, u32 val)
static void usb_write_port_complete(struct urb *purb, struct pt_regs *regs)
{
struct xmit_buf *pxmitbuf = (struct xmit_buf *)purb->context;
- struct adapter *padapter = pxmitbuf->padapter;
- struct xmit_priv*pxmitpriv = &padapter->xmitpriv;
+ struct adapter *padapter = pxmitbuf->padapter;
+ struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
switch (pxmitbuf->flags) {
case VO_QUEUE_INX:
@@ -662,8 +662,8 @@ u32 usb_write_port(struct adapter *padapter, u32 addr, u32
cnt, struct xmit_buf
int status;
u32 ret = _FAIL;
struct urb *purb = NULL;
- struct dvobj_priv *pdvobj = adapter_to_dvobj(padapter);
- struct xmit_priv*pxmitpriv = &padapter->xmitpriv;
+ struct dvobj_priv *pdvobj = adapter_to_dvobj(padapter);
+ struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
struct xmit_frame *pxmitframe = (struct xmit_frame *)xmitbuf->priv_data;
struct usb_device *pusbd = pdvobj->pusbdev;
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/8] staging: rtl8188eu: remove braces from single if else statement
Remove braces from single line if else statement.
Reported by checkpatch.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
index d792301a5ac2..5a41766bba68 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
@@ -259,11 +259,10 @@ static int usbctrl_vendorreq(struct adapter *adapt, u8
request, u16 value, u16 i
len, status, *(u32 *)pdata, vendorreq_times);
if (status < 0) {
- if (status == (-ESHUTDOWN) || status ==
-ENODEV) {
+ if (status == (-ESHUTDOWN) || status == -ENODEV)
adapt->bSurpriseRemoved = true;
- } else {
+ else
adapt->HalData->srestpriv.Wifi_Error_Status = USB_VEN_REQ_CMD_FAIL;
- }
} else { /* status != len && status >= 0 */
if (status > 0) {
if (requesttype == 0x01) {
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
driverdev-devel@linuxdriverproject.org
Add spaces around '>>' and '&' to follow kernel coding style. Reported by checkpatch. Signed-off-by: Michael Straube --- drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c index 73d2d312e8ab..4821ce2fa762 100644 --- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c @@ -173,7 +173,7 @@ static int recvbuf2recvframe(struct adapter *adapt, struct sk_buff *pskb) pkt_copy = NULL; if (transfer_len > 0 && pkt_cnt == 0) - pkt_cnt = (le32_to_cpu(prxstat->rxdw2)>>16) & 0xff; + pkt_cnt = (le32_to_cpu(prxstat->rxdw2) >> 16) & 0xff; } while ((transfer_len > 0) && (pkt_cnt > 0)); @@ -299,7 +299,7 @@ u8 usb_read8(struct adapter *adapter, u32 addr) requesttype = 0x01;/* read_in */ index = 0;/* n/a */ - wvalue = (u16)(addr&0x); + wvalue = (u16)(addr & 0x); len = 1; usbctrl_vendorreq(adapter, request, wvalue, index, &data, len, requesttype); @@ -319,11 +319,11 @@ u16 usb_read16(struct adapter *adapter, u32 addr) request = 0x05; requesttype = 0x01;/* read_in */ index = 0;/* n/a */ - wvalue = (u16)(addr&0x); + wvalue = (u16)(addr & 0x); len = 2; usbctrl_vendorreq(adapter, request, wvalue, index, &data, len, requesttype); - return (u16)(le32_to_cpu(data)&0x); + return (u16)(le32_to_cpu(data) & 0x); } u32 usb_read32(struct adapter *adapter, u32 addr) @@ -339,7 +339,7 @@ u32 usb_read32(struct adapter *adapter, u32 addr) requesttype = 0x01;/* read_in */ index = 0;/* n/a */ - wvalue = (u16)(addr&0x); + wvalue = (u16)(addr & 0x); len = 4; usbctrl_vendorreq(adapter, request, wvalue, index, &data, len, requesttype); @@ -520,7 +520,7 @@ int usb_write8(struct adapter *adapter, u32 addr, u8 val) request = 0x05; requesttype = 0x00;/* write_out */ index = 0;/* n/a */ - wvalue = (u16)(addr&0x); + wvalue = (u16)(addr & 0x); len = 1; data = val; return usbctrl_vendorreq(adapter, request, wvalue, @@ -540,7 +540,7 @@ int usb_write16(struct adapter *adapter, u32 addr, u16 val) requesttype = 0x00;/* write_out */ index = 0;/* n/a */ - wvalue = (u16)(addr&0x); + wvalue = (u16)(addr & 0x); len = 2; data = cpu_to_le32(val & 0x); @@ -562,7 +562,7 @@ int usb_write32(struct adapter *adapter, u32 addr, u32 val) requesttype = 0x00;/* write_out */ index = 0;/* n/a */ - wvalue = (u16)(addr&0x); + wvalue = (u16)(addr & 0x); len = 4; data = cpu_to_le32(val); -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 8/8] staging: rtl8188eu: remove variable from rtl8188eu_xmit_tasklet()
The local variable ret is only used to test the return value of the
call to rtl8188eu_xmitframe_complete(). Use the function directly in
the if test and remove the variable ret.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
index 9a732adbf1b1..7dc7028c1cd5 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
@@ -796,7 +796,6 @@ void rtl8188eu_recv_tasklet(void *priv)
void rtl8188eu_xmit_tasklet(void *priv)
{
- int ret = false;
struct adapter *adapt = priv;
struct xmit_priv *pxmitpriv = &adapt->xmitpriv;
@@ -811,9 +810,7 @@ void rtl8188eu_xmit_tasklet(void *priv)
break;
}
- ret = rtl8188eu_xmitframe_complete(adapt, pxmitpriv);
-
- if (!ret)
+ if (!rtl8188eu_xmitframe_complete(adapt, pxmitpriv))
break;
}
}
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/8] staging: rtl8188eu: remove unnecessary parentheses
Remove unnecessary parentheses in usb_ops_linux.c. Reported by checkpatch. Signed-off-by: Michael Straube --- drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c index 75b7d8c2cc50..73d2d312e8ab 100644 --- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c @@ -25,7 +25,8 @@ static void interrupt_handler_8188eu(struct adapter *adapt, u16 pkt_len, u8 *pbu /* C2H Event */ if (pbuf[0] != 0) - memcpy(&(haldata->C2hArray[0]), &(pbuf[USB_INTR_CONTENT_C2H_OFFSET]), 16); + memcpy(&haldata->C2hArray[0], + &pbuf[USB_INTR_CONTENT_C2H_OFFSET], 16); } static int recvbuf2recvframe(struct adapter *adapt, struct sk_buff *pskb) -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/10] staging: erofs: separate into init_once / always
Hi Greg, On 2018/11/22 21:23, Greg Kroah-Hartman wrote: > On Thu, Nov 22, 2018 at 09:01:31PM +0800, Gao Xiang wrote: >> Hi Greg, >> >> On 2018/11/22 20:00, Gao Xiang wrote: >>> Hi Greg, >>> >>> On 2018/11/22 19:26, Greg Kroah-Hartman wrote: Don't make people rebuild your code with different options for debugging. That will never work in the 'real world' when people start using the code. You need to have things enabled for people all the time, which is why we have dynamic debugging in the kernel now, and not a zillion different "DRIVER_DEBUG" build options anymore. >>> Actually, current erofs handle differently for beta users (in eng mode) >>> and commercial users. >>> >>> CONFIG_EROFS_FS_DEBUG is enable for all beta users, after an observed >>> expression is false, we could get the whole memorydump as early as possible. >>> But for commercial users, there are no such observing points to promise >>> the kernel stability and performance. >>> >>> It has helped us to find several bug, and I cannot find some alternative way >>> to get the the first scene of the accident... >> >> I'm about to send v2 of this patchset... I need to get your opinion... >> >> I think for the current erofs development state, I tend to leave such a >> developping switch because the code could be modified frequently, >> many potential bugs could be avoid when this debugging mode is on and >> it will be dropped after erofs becomes more stable... > > You have two competing issues here. You need to have some sort of debug > build that allows you to get feedback from users, and you need to > provide a solid, reliable system for those not wanting to debug > anything. Yes, you are right :) > > If you use a kernel build option, then you can do what you are doing > now, just that do not expect for any user that has problems with the > filesystem, they will rebuild the code just to try to help debug it. > That is going to require run-time debugging to be enabled as they will > not usually have built their own kernel/module at all. Yes, actually the current Android system have 2 modes -- eng and commerical build for all versions. A large number of beta users will receive eng build, and many buges were observed from these users. That is the debugging benefit what Android system does now. I do not expect real users to rebuild kernels for debugging, I think after the Android use case, erofs has become solid enough. For commerical builds, it will have enough error handing case found by Android users and developers. > > For now, keep doing what you are doing, I just started to worry when I > saw the initial BUG_ON() as we had just talked about these at the > Maintainers summit a few weeks ago, and how we need to get rid of them > from the kernel as they are a crutch that we need to solve. Thanks for understanding... Thanks, Gao Xiang > > thanks, and sorry for the noise. Please resend this series again, > > greg k-h > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v9 01/13] media: staging/imx: refactor imx media device probe
Refactor and move media device initialization code to a new common
module, so it can be used by other devices, this will allow for example
a near to introduce imx7 CSI driver, to use this media device.
Signed-off-by: Rui Miguel Silva
---
drivers/staging/media/imx/Makefile| 1 +
.../staging/media/imx/imx-media-dev-common.c | 102 ++
drivers/staging/media/imx/imx-media-dev.c | 88 ---
drivers/staging/media/imx/imx-media-of.c | 6 +-
drivers/staging/media/imx/imx-media.h | 15 +++
5 files changed, 141 insertions(+), 71 deletions(-)
create mode 100644 drivers/staging/media/imx/imx-media-dev-common.c
diff --git a/drivers/staging/media/imx/Makefile
b/drivers/staging/media/imx/Makefile
index 698a4210316e..a30b3033f9a3 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
imx-media-objs := imx-media-dev.o imx-media-internal-sd.o imx-media-of.o
+imx-media-objs += imx-media-dev-common.o
imx-media-common-objs := imx-media-utils.o imx-media-fim.o
imx-media-ic-objs := imx-ic-common.o imx-ic-prp.o imx-ic-prpencvf.o
diff --git a/drivers/staging/media/imx/imx-media-dev-common.c
b/drivers/staging/media/imx/imx-media-dev-common.c
new file mode 100644
index ..55fe94fb72f2
--- /dev/null
+++ b/drivers/staging/media/imx/imx-media-dev-common.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL
+/*
+ * V4L2 Media Controller Driver for Freescale common i.MX5/6/7 SOC
+ *
+ * Copyright (c) 2018 Linaro Ltd
+ * Copyright (c) 2016 Mentor Graphics Inc.
+ */
+
+#include
+#include
+#include "imx-media.h"
+
+static const struct v4l2_async_notifier_operations imx_media_subdev_ops = {
+ .bound = imx_media_subdev_bound,
+ .complete = imx_media_probe_complete,
+};
+
+static const struct media_device_ops imx_media_md_ops = {
+ .link_notify = imx_media_link_notify,
+};
+
+struct imx_media_dev *imx_media_dev_init(struct device *dev)
+{
+ struct imx_media_dev *imxmd;
+ int ret;
+
+ imxmd = devm_kzalloc(dev, sizeof(*imxmd), GFP_KERNEL);
+ if (!imxmd)
+ return ERR_PTR(-ENOMEM);
+
+ dev_set_drvdata(dev, imxmd);
+
+ strlcpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model));
+ imxmd->md.ops = &imx_media_md_ops;
+ imxmd->md.dev = dev;
+
+ mutex_init(&imxmd->mutex);
+
+ imxmd->v4l2_dev.mdev = &imxmd->md;
+ strlcpy(imxmd->v4l2_dev.name, "imx-media",
+ sizeof(imxmd->v4l2_dev.name));
+
+ media_device_init(&imxmd->md);
+
+ ret = v4l2_device_register(dev, &imxmd->v4l2_dev);
+ if (ret < 0) {
+ v4l2_err(&imxmd->v4l2_dev,
+"Failed to register v4l2_device: %d\n", ret);
+ goto cleanup;
+ }
+
+ dev_set_drvdata(imxmd->v4l2_dev.dev, imxmd);
+
+ INIT_LIST_HEAD(&imxmd->vdev_list);
+
+ v4l2_async_notifier_init(&imxmd->notifier);
+
+ return imxmd;
+
+cleanup:
+ media_device_cleanup(&imxmd->md);
+
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(imx_media_dev_init);
+
+int imx_media_dev_notifier_register(struct imx_media_dev *imxmd)
+{
+ int ret;
+
+ /* no subdevs? just bail */
+ if (list_empty(&imxmd->notifier.asd_list)) {
+ v4l2_err(&imxmd->v4l2_dev, "no subdevs\n");
+ return -ENODEV;
+ }
+
+ /* prepare the async subdev notifier and register it */
+ imxmd->notifier.ops = &imx_media_subdev_ops;
+ ret = v4l2_async_notifier_register(&imxmd->v4l2_dev,
+ &imxmd->notifier);
+ if (ret) {
+ v4l2_err(&imxmd->v4l2_dev,
+"v4l2_async_notifier_register failed with %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(imx_media_dev_notifier_register);
+
+void imx_media_dev_cleanup(struct imx_media_dev *imxmd)
+{
+ v4l2_device_unregister(&imxmd->v4l2_dev);
+ media_device_cleanup(&imxmd->md);
+}
+EXPORT_SYMBOL_GPL(imx_media_dev_cleanup);
+
+void imx_media_dev_notifier_unregister(struct imx_media_dev *imxmd)
+{
+ v4l2_async_notifier_cleanup(&imxmd->notifier);
+}
+EXPORT_SYMBOL_GPL(imx_media_dev_notifier_unregister);
diff --git a/drivers/staging/media/imx/imx-media-dev.c
b/drivers/staging/media/imx/imx-media-dev.c
index 4b344a4a3706..21f65af5c738 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -116,9 +116,9 @@ static int imx_media_get_ipu(struct imx_media_dev *imxmd,
}
/* async subdev bound notifier */
-static int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
- struct v4l2_subdev *sd,
- struct v4l2_async_subdev *asd)
+int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *sd,
+
[PATCH v9 05/13] media: dt-bindings: add bindings for i.MX7 media driver
Add bindings documentation for i.MX7 media drivers.
The imx7 MIPI CSI2 and imx7 CMOS Sensor Interface.
Signed-off-by: Rui Miguel Silva
Reviewed-by: Rob Herring
Acked-by: Sakari Ailus
---
.../devicetree/bindings/media/imx7-csi.txt| 45 ++
.../bindings/media/imx7-mipi-csi2.txt | 90 +++
2 files changed, 135 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/imx7-csi.txt
create mode 100644 Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
diff --git a/Documentation/devicetree/bindings/media/imx7-csi.txt
b/Documentation/devicetree/bindings/media/imx7-csi.txt
new file mode 100644
index ..3c07bc676bc3
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx7-csi.txt
@@ -0,0 +1,45 @@
+Freescale i.MX7 CMOS Sensor Interface
+=
+
+csi node
+
+
+This is device node for the CMOS Sensor Interface (CSI) which enables the chip
+to connect directly to external CMOS image sensors.
+
+Required properties:
+
+- compatible: "fsl,imx7-csi";
+- reg : base address and length of the register set for the device;
+- interrupts: should contain CSI interrupt;
+- clocks: list of clock specifiers, see
+Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
+- clock-names : must contain "axi", "mclk" and "dcic" entries, matching
+ entries in the clock property;
+
+The device node shall contain one 'port' child node with one child 'endpoint'
+node, according to the bindings defined in:
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+In the following example a remote endpoint is a video multiplexer.
+
+example:
+
+csi: csi@3071 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+compatible = "fsl,imx7-csi";
+reg = <0x3071 0x1>;
+interrupts = ;
+clocks = <&clks IMX7D_CLK_DUMMY>,
+<&clks IMX7D_CSI_MCLK_ROOT_CLK>,
+<&clks IMX7D_CLK_DUMMY>;
+clock-names = "axi", "mclk", "dcic";
+
+port {
+csi_from_csi_mux: endpoint {
+remote-endpoint = <&csi_mux_to_csi>;
+};
+};
+};
diff --git a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
b/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
new file mode 100644
index ..71fd74ed3ec8
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
@@ -0,0 +1,90 @@
+Freescale i.MX7 Mipi CSI2
+=
+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
+compatible with previous version of Samsung D-phy.
+
+Required properties:
+
+- compatible: "fsl,imx7-mipi-csi2";
+- reg : base address and length of the register set for the device;
+- interrupts: should contain MIPI CSIS interrupt;
+- clocks: list of clock specifiers, see
+Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
+- clock-names : must contain "pclk", "wrap" and "phy" entries, matching
+ entries in the clock property;
+- power-domains : a phandle to the power domain, see
+ Documentation/devicetree/bindings/power/power_domain.txt for details.
+- reset-names : should include following entry "mrst";
+- resets: a list of phandle, should contain reset entry of
+ reset-names;
+- phy-supply: from the generic phy bindings, a phandle to a regulator that
+ provides power to MIPI CSIS core;
+
+Optional properties:
+
+- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
+ value when this property is not specified is 166 MHz;
+- fsl,csis-hs-settle : differential receiver (HS-RX) settle time;
+
+The device node should contain two 'port' child nodes with one child 'endpoint'
+node, according to the bindings defined in:
+ Documentation/devicetree/bindings/ media/video-interfaces.txt.
+ The following are properties specific to those nodes.
+
+port node
+-
+
+- reg: (required) can take the values 0 or 1, where 0 shall be
+ related to the sink port and port 1 shall be the source
+ one;
+
+endpoint node
+-
+
+- data-lanes: (required) an array specifying active physical MIPI-CSI2
+ data input lanes and their mapping to logical lanes; this
+shall only be applied to port 0 (sink port), the array's
+content is unused only its length is meaningful,
+in this case the maximum le
[PATCH v9 04/13] media: staging/imx7: add MIPI CSI-2 receiver subdev for i.MX7
Adds MIPI CSI-2 subdev for i.MX7 to connect with sensors with a MIPI CSI-2 interface. Signed-off-by: Rui Miguel Silva --- drivers/staging/media/imx/Makefile |1 + drivers/staging/media/imx/imx7-mipi-csis.c | 1135 2 files changed, 1136 insertions(+) create mode 100644 drivers/staging/media/imx/imx7-mipi-csis.c diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile index 074f016d3519..d2d909a36239 100644 --- a/drivers/staging/media/imx/Makefile +++ b/drivers/staging/media/imx/Makefile @@ -14,3 +14,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c new file mode 100644 index ..56963d0c2043 --- /dev/null +++ b/drivers/staging/media/imx/imx7-mipi-csis.c @@ -0,0 +1,1135 @@ +// SPDX-License-Identifier: GPL +/* + * Freescale i.MX7 SoC series MIPI-CSI V3.3 receiver driver + * + * Copyright (C) 2018 Linaro Ltd + * Copyright (C) 2015-2016 Freescale Semiconductor, Inc. All Rights Reserved. + * Copyright (C) 2011 - 2013 Samsung Electronics Co., Ltd. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#include "imx-media.h" + +static int debug; +module_param(debug, int, 0644); +MODULE_PARM_DESC(debug, "Debug level (0-2)"); + +#define CSIS_DRIVER_NAME "imx7-mipi-csis" +#define CSIS_SUBDEV_NAME CSIS_DRIVER_NAME + +#define CSIS_PAD_SINK 0 +#define CSIS_PAD_SOURCE1 +#define CSIS_PADS_NUM 2 + +#define MIPI_CSIS_DEF_PIX_WIDTH640 +#define MIPI_CSIS_DEF_PIX_HEIGHT 480 + +/* Register map definition */ + +/* CSIS common control */ +#define MIPI_CSIS_CMN_CTRL 0x04 +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW BIT(16) +#define MIPI_CSIS_CMN_CTRL_INTER_MODE BIT(10) +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL BIT(2) +#define MIPI_CSIS_CMN_CTRL_RESET BIT(1) +#define MIPI_CSIS_CMN_CTRL_ENABLE BIT(0) + +#define MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET 8 +#define MIPI_CSIS_CMN_CTRL_LANE_NR_MASK(3 << 8) + +/* CSIS clock control */ +#define MIPI_CSIS_CLK_CTRL 0x08 +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH3(x)((x) << 28) +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH2(x)((x) << 24) +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH1(x)((x) << 20) +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(x)((x) << 16) +#define MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MSK (0xf << 4) +#define MIPI_CSIS_CLK_CTRL_WCLK_SRCBIT(0) + +/* CSIS Interrupt mask */ +#define MIPI_CSIS_INTMSK 0x10 +#define MIPI_CSIS_INTMSK_EVEN_BEFORE BIT(31) +#define MIPI_CSIS_INTMSK_EVEN_AFTERBIT(30) +#define MIPI_CSIS_INTMSK_ODD_BEFOREBIT(29) +#define MIPI_CSIS_INTMSK_ODD_AFTER BIT(28) +#define MIPI_CSIS_INTMSK_FRAME_START BIT(24) +#define MIPI_CSIS_INTMSK_FRAME_END BIT(20) +#define MIPI_CSIS_INTMSK_ERR_SOT_HSBIT(16) +#define MIPI_CSIS_INTMSK_ERR_LOST_FS BIT(12) +#define MIPI_CSIS_INTMSK_ERR_LOST_FE BIT(8) +#define MIPI_CSIS_INTMSK_ERR_OVER BIT(4) +#define MIPI_CSIS_INTMSK_ERR_WRONG_CFG BIT(3) +#define MIPI_CSIS_INTMSK_ERR_ECC BIT(2) +#define MIPI_CSIS_INTMSK_ERR_CRC BIT(1) +#define MIPI_CSIS_INTMSK_ERR_UNKNOWN BIT(0) + +/* CSIS Interrupt source */ +#define MIPI_CSIS_INTSRC 0x14 +#define MIPI_CSIS_INTSRC_EVEN_BEFORE BIT(31) +#define MIPI_CSIS_INTSRC_EVEN_AFTERBIT(30) +#define MIPI_CSIS_INTSRC_EVEN BIT(30) +#define MIPI_CSIS_INTSRC_ODD_BEFOREBIT(29) +#define MIPI_CSIS_INTSRC_ODD_AFTER BIT(28) +#define MIPI_CSIS_INTSRC_ODD (0x3 << 28) +#define MIPI_CSIS_INTSRC_NON_IMAGE_DATA(0xf << 28) +#define MIPI_CSIS_INTSRC_FRAME_START BIT(24) +#define MIPI_CSIS_INTSRC_FRAME_END BIT(20) +#define MIPI_CSIS_INTSRC_ERR_SOT_HSBIT(16) +#define MIPI_CSIS_INTSRC_ERR_LOST_FS BIT(12) +#define MIPI_CSIS_INTSRC_ERR_LOST_FE BIT(8) +#define MIPI_CSIS_INTSRC_ERR_OVER BIT(4) +#define MIPI_CSIS_INTSRC_ERR_WRONG_CFG BIT(3) +#define MIPI_CSIS_INTSRC_ERR_ECC BIT(2) +#define MIPI_CSIS_INTSRC_ERR_CRC BIT(1) +#define MIPI_CSIS_INTSRC_ERR_UNKNOWN BIT(0) +#define MIPI_CSIS_INTSRC_ERRORS0xf + +/* D-PHY status control */ +#define MIPI_CSIS_DPHYSTATUS 0x20 +#define MIPI_CSIS_DPHYSTATUS_ULPS_DAT BIT(8) +#define MIPI_CSIS_DPHYSTATUS_STOPSTATE_DAT BIT(4) +#define MIPI_CSIS_DPHYSTATUS_ULPS_CLK BIT(1) +#define MIPI_CSIS_DPHYSTATUS_STOPSTATE_CLK BIT(0) + +/* D-PHY common control */ +#define MIPI_CSIS_DPHYCTRL 0x24 +#de
[PATCH v9 09/13] ARM: dts: imx7s-warp: add ov2680 sensor node
Warp7 comes with a Omnivision OV2680 sensor, add the node here to make
complete the camera data path for this system. Add the needed regulator
to the analog voltage supply, the port and endpoints in mipi_csi node
and the pinctrl for the reset gpio.
Signed-off-by: Rui Miguel Silva
---
arch/arm/boot/dts/imx7s-warp.dts | 44
1 file changed, 44 insertions(+)
diff --git a/arch/arm/boot/dts/imx7s-warp.dts b/arch/arm/boot/dts/imx7s-warp.dts
index 757856a3964b..4ada85850411 100644
--- a/arch/arm/boot/dts/imx7s-warp.dts
+++ b/arch/arm/boot/dts/imx7s-warp.dts
@@ -54,6 +54,14 @@
regulator-always-on;
};
+ reg_peri_3p15v: regulator-peri-3p15v {
+ compatible = "regulator-fixed";
+ regulator-name = "peri_3p15v_reg";
+ regulator-min-microvolt = <315>;
+ regulator-max-microvolt = <315>;
+ regulator-always-on;
+ };
+
sound {
compatible = "simple-audio-card";
simple-audio-card,name = "imx7-sgtl5000";
@@ -177,6 +185,27 @@
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_i2c2>;
status = "okay";
+
+ ov2680: camera@36 {
+ compatible = "ovti,ov2680";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ov2680>;
+ reg = <0x36>;
+ clocks = <&osc>;
+ clock-names = "xvclk";
+ reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
+ DOVDD-supply = <&sw2_reg>;
+ DVDD-supply = <&sw2_reg>;
+ AVDD-supply = <®_peri_3p15v>;
+
+ port {
+ ov2680_to_mipi: endpoint {
+ remote-endpoint = <&mipi_from_sensor>;
+ clock-lanes = <0>;
+ data-lanes = <1>;
+ };
+ };
+ };
};
&i2c3 {
@@ -318,6 +347,15 @@
#size-cells = <0>;
fsl,csis-hs-settle = <3>;
+ port@0 {
+ reg = <0>;
+
+ mipi_from_sensor: endpoint {
+ remote-endpoint = <&ov2680_to_mipi>;
+ data-lanes = <1>;
+ };
+ };
+
port@1 {
reg = <1>;
@@ -381,6 +419,12 @@
>;
};
+ pinctrl_ov2680: ov2660grp {
+ fsl,pins = <
+ MX7D_PAD_LPSR_GPIO1_IO03__GPIO1_IO3 0x14
+ >;
+ };
+
pinctrl_sai1: sai1grp {
fsl,pins = <
MX7D_PAD_SAI1_RX_DATA__SAI1_RX_DATA00x1f
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v9 00/13] media: staging/imx7: add i.MX7 media driver
Hi, This series introduces the Media driver to work with the i.MX7 SoC. it uses the already existing imx media core drivers but since the i.MX7, contrary to i.MX5/6, do not have an IPU and because of that some changes in the imx media core are made along this series to make it support that case. This patches adds CSI and MIPI-CSI2 drivers for i.MX7, along with several configurations changes for this to work as a capture subsystem. Some bugs are also fixed along the line. And necessary documentation. For a more detailed view of the capture paths, pads links in the i.MX7 please take a look at the documentation in PATCH 10. The system used to test and develop this was the Warp7 board with an OV2680 sensor, which output format is 10-bit bayer. So, only MIPI interface was tested, a scenario with an parallel input would nice to have. Bellow goes an example of the output of the pads and links and the output of v4l2-compliance testing. The v4l-utils version used is: v4l2-compliance SHA : 044d5ab7b0d02683070d01a369c73d462d7a0cee from Nov 19th The Media Driver fail some tests but this failures are coming from code out of scope of this series (imx-capture), and some from the sensor OV2680 but that I think not related with the sensor driver but with the testing and core. The csi and mipi-csi entities pass all compliance tests. Cheers, Rui v8->v9: Hans Verkuil: - Fix issues detected by checkpatch strict, still some left: - bigger kconfig option description - some alignement parenthesis that were left as they are, to be more readable - added new patch (PATCH13) for Maintainers update - SPDX in documentation rst file Sakari Ailus: - remove pad check in csi, this is done by core already - destroy mutex in probe error path (add label) - swap order in driver release - initialize endpoint in stack - use clk_bulk kbuild test robot: - add the missing imx-media-dev-common.c in patch 1/13 - remove OWNER of module csis Myself: - add MAINTAINERS entries - new patch v7->v8: Myself: - rebase to latest linux-next (s/V4L2_MBUS_CSI2/V4L2_MBUS_CSI2_DPHY/) - Rebuild and test with latest v4l2-compliance - add Sakari reviewed-by tag to dt-bindings v6->v7: Myself: - Clock patches removed from this version since they were already merged - Rebuild and test with the latest v4l2-compliance - Add patch to video-mux regarding bayer formats - remove reference to dependent patch serie (was already merged) Sakari Ailus: - add port and endpoint explanantions - fix some wording should -> shall v5->v6: Rob Herring: - rename power-domain node name from: pgc-power-domain to power-domain - change mux-control-cells to 0 - remove bus-width from mipi bindings and dts - remove err... regarding clock names line - remove clk-settle from example - split mipi-csi2 and csi bindings per file - add OF graph description to CSI Philipp Zabel: - rework group IDs and rename them with an _IPU_ prefix, this allowed to remove the ipu_present flag need. v4->v5: Sakari Ailus: - fix remove of the capture entries in dts bindings in the right patch Stephen Boyd: - Send all series to clk list v3->v4: Philipp Zabel: - refactor initialization code from media device probe to be possible to used from other modules - Remove index of csi from all accurrencs (dts, code, documentation) - Remove need for capture node for imx7 - fix pinctrl for ov2680 - add reviewed tag to add multiplexer controls patch Fabio Estevam: - remove always on from new regulator Randy Dunlap: - several text editing fixes in documentation Myself: - rebase on top of v4 of Steve series - change CSI probe to initialize imx media device - remove csi mux parallel endpoint from mux to avoid warning message v2->v3: Philipp Zabel: - use of_match_device in imx-media-dev instead of of_device_match - fix number of data lanes from 4 to 2 - change the clock definitions and use of mipi - move hs-settle from endpoint Rob Herring: - fix phy-supply description - add vendor properties - fix examples indentations Stephen Boyd: patch 3/14 - fix double sign-off - add fixes tag Dong Aisheng: patch 3/14 - fix double sign-off - add Acked-by tag Shawn Guo: patch 4/14 - remove line breakage in parent redifiniton - added Acked-by tag - dropped CMA area increase and add more verbose information in case of dma allocation failure patch 9/14 - remove extra line between cells and reg masks Myself: - rework on frame end in csi - add rxcount in csi driver - add power supplies to ov2680 node and fix gpio polarity v1->v2: Dan Carpenter: - fix return paths and codes; - fix clk_frequency validation and return code; - handle the csi remove (release resources that was missing) - revert the logic arround the ipu_present flag Philipp Zabel: - drop patch that changed the rgb formats and address the pixel/bus format in mipi_csis code. MySelf: - add patch that add ov2680 node to the warp7 dts, so the all data path is co
[PATCH v9 10/13] media: imx7.rst: add documentation for i.MX7 media driver
Add rst document to describe the i.MX7 media driver and also a working example from the Warp7 board usage with a OV2680 sensor. Signed-off-by: Rui Miguel Silva --- Documentation/media/v4l-drivers/imx7.rst | 157 ++ Documentation/media/v4l-drivers/index.rst | 1 + 2 files changed, 158 insertions(+) create mode 100644 Documentation/media/v4l-drivers/imx7.rst diff --git a/Documentation/media/v4l-drivers/imx7.rst b/Documentation/media/v4l-drivers/imx7.rst new file mode 100644 index ..cd1195d391c5 --- /dev/null +++ b/Documentation/media/v4l-drivers/imx7.rst @@ -0,0 +1,157 @@ +i.MX7 Video Capture Driver +== + +Introduction + + +The i.MX7 contrary to the i.MX5/6 family does not contain an Image Processing +Unit (IPU); because of that the capabilities to perform operations or +manipulation of the capture frames are less feature rich. + +For image capture the i.MX7 has three units: +- CMOS Sensor Interface (CSI) +- Video Multiplexer +- MIPI CSI-2 Receiver + +:: + |\ + MIPI Camera Input ---> MIPI CSI-2 --- > | \ + | \ + | M | + | U | --> CSI ---> Capture + | X | + | / + Parallel Camera Input > | / + |/ + +For additional information, please refer to the latest versions of the i.MX7 +reference manual [#f1]_. + +Entities + + +imx7-mipi-csi2 +-- + +This is the MIPI CSI-2 receiver entity. It has one sink pad to receive the pixel +data from MIPI CSI-2 camera sensor. It has one source pad, corresponding to the +virtual channel 0. This module is compliant to previous version of Samsung +D-phy, and supports two D-PHY Rx Data lanes. + +csi_mux +--- + +This is the video multiplexer. It has two sink pads to select from either camera +sensor with a parallel interface or from MIPI CSI-2 virtual channel 0. It has +a single source pad that routes to the CSI. + +csi +--- + +The CSI enables the chip to connect directly to external CMOS image sensor. CSI +can interface directly with Parallel and MIPI CSI-2 buses. It has 256 x 64 FIFO +to store received image pixel data and embedded DMA controllers to transfer data +from the FIFO through AHB bus. + +This entity has one sink pad that receives from the csi_mux entity and a single +source pad that routes video frames directly to memory buffers. This pad is +routed to a capture device node. + +Usage Notes +--- + +To aid in configuration and for backward compatibility with V4L2 applications +that access controls only from video device nodes, the capture device interfaces +inherit controls from the active entities in the current pipeline, so controls +can be accessed either directly from the subdev or from the active capture +device interface. For example, the sensor controls are available either from the +sensor subdevs or from the active capture device. + +Warp7 with OV2680 +- + +On this platform an OV2680 MIPI CSI-2 module is connected to the internal MIPI +CSI-2 receiver. The following example configures a video capture pipeline with +an output of 800x600, and BGGR 10 bit bayer format: + +.. code-block:: none + # Setup links + media-ctl -l "'ov2680 1-0036':0 -> 'imx7-mipi-csis.0':0[1]" + media-ctl -l "'imx7-mipi-csis.0':1 -> 'csi_mux':1[1]" + media-ctl -l "'csi_mux':2 -> 'csi':0[1]" + media-ctl -l "'csi':1 -> 'csi capture':0[1]" + + # Configure pads for pipeline + media-ctl -V "'ov2680 1-0036':0 [fmt:SBGGR10_1X10/800x600 field:none]" + media-ctl -V "'csi_mux':1 [fmt:SBGGR10_1X10/800x600 field:none]" + media-ctl -V "'csi_mux':2 [fmt:SBGGR10_1X10/800x600 field:none]" + media-ctl -V "'imx7-mipi-csis.0':0 [fmt:SBGGR10_1X10/800x600 field:none]" + media-ctl -V "'csi':0 [fmt:SBGGR10_1X10/800x600 field:none]" + +After this streaming can start. The v4l2-ctl tool can be used to select any of +the resolutions supported by the sensor. + +.. code-block:: none +root@imx7s-warp:~# media-ctl -p +Media controller API version 4.17.0 + +Media device information + +driver imx-media +model imx-media +serial +bus info +hw revision 0x0 +driver version 4.17.0 + +Device topology +- entity 1: csi (2 pads, 2 links) + type V4L2 subdev subtype Unknown flags 0 + device node name /dev/v4l-subdev0 + pad0: Sink + [fmt:SBGGR10_1X10/800x600 field:none] + <- "csi_mux":2 [ENABLED] + pad1: Source + [fmt:SBGGR10_1X10/800x600 field:none] + -> "csi capture":0 [ENABLED] + +- entity 4: csi capture (1 pad, 1 link) + type Node subtype V4
[PATCH v9 07/13] ARM: dts: imx7s: add multiplexer controls
The IOMUXC General Purpose Register has bitfield to control video bus
multiplexer to control the CSI input between the MIPI-CSI2 and parallel
interface. Add that register and mask.
Signed-off-by: Rui Miguel Silva
Reviewed-by: Philipp Zabel
---
arch/arm/boot/dts/imx7s.dtsi | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 6e2e4f99cdb0..174635a73fb6 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -499,8 +499,15 @@
gpr: iomuxc-gpr@3034 {
compatible = "fsl,imx7d-iomuxc-gpr",
- "fsl,imx6q-iomuxc-gpr", "syscon";
+ "fsl,imx6q-iomuxc-gpr", "syscon",
+ "simple-mfd";
reg = <0x3034 0x1>;
+
+ mux: mux-controller {
+ compatible = "mmio-mux";
+ #mux-control-cells = <0>;
+ mux-reg-masks = <0x14 0x0010>;
+ };
};
ocotp: ocotp-ctrl@3035 {
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v9 13/13] media: MAINTAINERS: add entry for Freescale i.MX7 media driver
Add maintainer entry for the imx7 media csi, mipi csis driver, dt-bindings and documentation. Signed-off-by: Rui Miguel Silva --- MAINTAINERS | 11 +++ 1 file changed, 11 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 0abecc528dac..afa2ad3c5600 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9134,6 +9134,17 @@ T: git git://linuxtv.org/media_tree.git S: Maintained F: drivers/media/platform/imx-pxp.[ch] +MEDIA DRIVERS FOR FREESCALE IMX7 +M: Rui Miguel Silva +L: linux-me...@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +S: Maintained +F: Documentation/devicetree/bindings/media/imx7-csi.txt +F: Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt +F: Documentation/media/v4l-drivers/imx7.rst +F: drivers/staging/media/imx/imx7-media-csi.c +F: drivers/staging/media/imx/imx7-mipi-csis.c + MEDIA DRIVERS FOR HELENE M: Abylay Ospan L: linux-me...@vger.kernel.org -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v9 08/13] ARM: dts: imx7: Add video mux, csi and mipi_csi and connections
This patch adds the device tree nodes for csi, video multiplexer and
mipi-csi besides the graph connecting the necessary endpoints to make
the media capture entities to work in imx7 Warp board.
Signed-off-by: Rui Miguel Silva
---
arch/arm/boot/dts/imx7s-warp.dts | 51
arch/arm/boot/dts/imx7s.dtsi | 27 +
2 files changed, 78 insertions(+)
diff --git a/arch/arm/boot/dts/imx7s-warp.dts b/arch/arm/boot/dts/imx7s-warp.dts
index f7ba2c0a24ad..757856a3964b 100644
--- a/arch/arm/boot/dts/imx7s-warp.dts
+++ b/arch/arm/boot/dts/imx7s-warp.dts
@@ -276,6 +276,57 @@
status = "okay";
};
+&gpr {
+ csi_mux {
+ compatible = "video-mux";
+ mux-controls = <&mux 0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <1>;
+
+ csi_mux_from_mipi_vc0: endpoint {
+ remote-endpoint = <&mipi_vc0_to_csi_mux>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+
+ csi_mux_to_csi: endpoint {
+ remote-endpoint = <&csi_from_csi_mux>;
+ };
+ };
+ };
+};
+
+&csi {
+ status = "okay";
+
+ port {
+ csi_from_csi_mux: endpoint {
+ remote-endpoint = <&csi_mux_to_csi>;
+ };
+ };
+};
+
+&mipi_csi {
+ clock-frequency = <16600>;
+ status = "okay";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ fsl,csis-hs-settle = <3>;
+
+ port@1 {
+ reg = <1>;
+
+ mipi_vc0_to_csi_mux: endpoint {
+ remote-endpoint = <&csi_mux_from_mipi_vc0>;
+ };
+ };
+};
+
&wdog1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_wdog>;
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 174635a73fb6..0c495082e2bf 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include "imx7d-pinfunc.h"
/ {
@@ -711,6 +712,17 @@
status = "disabled";
};
+ csi: csi@3071 {
+ compatible = "fsl,imx7-csi";
+ reg = <0x3071 0x1>;
+ interrupts = ;
+ clocks = <&clks IMX7D_CLK_DUMMY>,
+ <&clks IMX7D_CSI_MCLK_ROOT_CLK>,
+ <&clks IMX7D_CLK_DUMMY>;
+ clock-names = "axi", "mclk", "dcic";
+ status = "disabled";
+ };
+
lcdif: lcdif@3073 {
compatible = "fsl,imx7d-lcdif",
"fsl,imx28-lcdif";
reg = <0x3073 0x1>;
@@ -720,6 +732,21 @@
clock-names = "pix", "axi";
status = "disabled";
};
+
+ mipi_csi: mipi-csi@3075 {
+ compatible = "fsl,imx7-mipi-csi2";
+ reg = <0x3075 0x1>;
+ interrupts = ;
+ clocks = <&clks IMX7D_IPG_ROOT_CLK>,
+ <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
+ <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
+ clock-names = "pclk", "wrap", "phy";
+ power-domains = <&pgc_mipi_phy>;
+ phy-supply = <®_1p0d>;
+ resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
+ reset-names = "mrst";
+ status = "disabled";
+ };
};
aips3: aips-bus@3080 {
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v9 06/13] ARM: dts: imx7s: add mipi phy power domain
Add power domain index 0 related with mipi-phy to imx7s.
While at it rename pcie power-domain node to remove pgc prefix.
Signed-off-by: Rui Miguel Silva
---
arch/arm/boot/dts/imx7s.dtsi | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index aa8df7d93b2e..6e2e4f99cdb0 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -608,7 +608,13 @@
#address-cells = <1>;
#size-cells = <0>;
- pgc_pcie_phy: pgc-power-domain@1 {
+ pgc_mipi_phy: power-domain@0 {
+ #power-domain-cells = <0>;
+ reg = <0>;
+ power-supply = <®_1p0d>;
+ };
+
+ pgc_pcie_phy: power-domain@1 {
#power-domain-cells = <0>;
reg = <1>;
power-supply = <®_1p0d>;
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v9 03/13] media: staging/imx7: add imx7 CSI subdev driver
This add the media entity subdevice and control driver for the i.MX7 CMOS Sensor Interface. Signed-off-by: Rui Miguel Silva --- drivers/staging/media/imx/Kconfig |9 +- drivers/staging/media/imx/Makefile |2 + drivers/staging/media/imx/imx7-media-csi.c | 1354 3 files changed, 1364 insertions(+), 1 deletion(-) create mode 100644 drivers/staging/media/imx/imx7-media-csi.c diff --git a/drivers/staging/media/imx/Kconfig b/drivers/staging/media/imx/Kconfig index bfc17de56b17..36b276ea2ecc 100644 --- a/drivers/staging/media/imx/Kconfig +++ b/drivers/staging/media/imx/Kconfig @@ -11,7 +11,7 @@ config VIDEO_IMX_MEDIA driver for the i.MX5/6 SOC. if VIDEO_IMX_MEDIA -menu "i.MX5/6 Media Sub devices" +menu "i.MX5/6/7 Media Sub devices" config VIDEO_IMX_CSI tristate "i.MX5/6 Camera Sensor Interface driver" @@ -20,5 +20,12 @@ config VIDEO_IMX_CSI ---help--- A video4linux camera sensor interface driver for i.MX5/6. +config VIDEO_IMX7_CSI + tristate "i.MX7 Camera Sensor Interface driver" + depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C + default y + help + Enable support for video4linux camera sensor interface driver for + i.MX7. endmenu endif diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile index a30b3033f9a3..074f016d3519 100644 --- a/drivers/staging/media/imx/Makefile +++ b/drivers/staging/media/imx/Makefile @@ -12,3 +12,5 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-ic.o obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o + +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c new file mode 100644 index ..9f0a37766cbb --- /dev/null +++ b/drivers/staging/media/imx/imx7-media-csi.c @@ -0,0 +1,1354 @@ +// SPDX-License-Identifier: GPL +/* + * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC + * + * Copyright (c) 2018 Linaro Ltd + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include +#include "imx-media.h" + +#define IMX7_CSI_PAD_SINK 0 +#define IMX7_CSI_PAD_SRC 1 +#define IMX7_CSI_PADS_NUM 2 + +/* reset values */ +#define CSICR1_RESET_VAL 0x4800 +#define CSICR2_RESET_VAL 0x0 +#define CSICR3_RESET_VAL 0x0 + +/* csi control reg 1 */ +#define BIT_SWAP16_EN BIT(31) +#define BIT_EXT_VSYNC BIT(30) +#define BIT_EOF_INT_EN BIT(29) +#define BIT_PRP_IF_EN BIT(28) +#define BIT_CCIR_MODE BIT(27) +#define BIT_COF_INT_EN BIT(26) +#define BIT_SF_OR_INTENBIT(25) +#define BIT_RF_OR_INTENBIT(24) +#define BIT_SFF_DMA_DONE_INTEN BIT(22) +#define BIT_STATFF_INTEN BIT(21) +#define BIT_FB2_DMA_DONE_INTEN BIT(20) +#define BIT_FB1_DMA_DONE_INTEN BIT(19) +#define BIT_RXFF_INTEN BIT(18) +#define BIT_SOF_POLBIT(17) +#define BIT_SOF_INTEN BIT(16) +#define BIT_MCLKDIV(0xF << 12) +#define BIT_HSYNC_POL BIT(11) +#define BIT_CCIR_ENBIT(10) +#define BIT_MCLKEN BIT(9) +#define BIT_FCCBIT(8) +#define BIT_PACK_DIR BIT(7) +#define BIT_CLR_STATFIFO BIT(6) +#define BIT_CLR_RXFIFO BIT(5) +#define BIT_GCLK_MODE BIT(4) +#define BIT_INV_DATA BIT(3) +#define BIT_INV_PCLK BIT(2) +#define BIT_REDGE BIT(1) +#define BIT_PIXEL_BIT BIT(0) + +#define SHIFT_MCLKDIV 12 + +/* control reg 3 */ +#define BIT_FRMCNT (0x << 16) +#define BIT_FRMCNT_RST BIT(15) +#define BIT_DMA_REFLASH_RFFBIT(14) +#define BIT_DMA_REFLASH_SFFBIT(13) +#define BIT_DMA_REQ_EN_RFF BIT(12) +#define BIT_DMA_REQ_EN_SFF BIT(11) +#define BIT_STATFF_LEVEL (0x7 << 8) +#define BIT_HRESP_ERR_EN BIT(7) +#define BIT_RXFF_LEVEL (0x7 << 4) +#define BIT_TWO_8BIT_SENSORBIT(3) +#define BIT_ZERO_PACK_EN BIT(2) +#define BIT_ECC_INT_EN BIT(1) +#define BIT_ECC_AUTO_ENBIT(0) + +#define SHIFT_FRMCNT 16 +#define SHIFT_RXFIFO_LEVEL 4 + +/* csi status reg */ +#define BIT_ADDR_CH_ERR_INTBIT(28) +#define BIT_FIELD0_INT BIT(27) +#define BIT_FIELD1_INT BIT(26) +#define BIT_SFF_OR_INT BIT(25) +#define BIT_RFF_OR_INT BIT(24) +#define BIT_DMA_TSF_DONE_SFF BIT(22) +#define BIT_STATFF_INT BIT(21) +#define BIT_DMA_TSF_DONE_FB2 BIT(20) +#define BIT_DMA_TSF_DONE_FB1 BIT(19) +#define BIT_RXFF_INT BIT(18) +#define BIT_EOF_INTBIT(17) +#define BIT_SOF_INTBIT(16) +#define BIT_F2_INT BIT(15) +#define BIT_F1_INT BIT(14) +#define BIT_COF
[PATCH v9 02/13] media: staging/imx: rearrange group id to take in account IPU
Some imx system do not have IPU, so prepare the imx media drivers to
support this kind of devices. Rename the group ids to include an _IPU_
prefix, add a new group id to support systems with only a CSI without
IPU, and also rename the create internal links to make it clear that
only systems with IPU have internal subdevices.
Signed-off-by: Rui Miguel Silva
---
drivers/staging/media/imx/imx-ic-common.c | 6 ++---
drivers/staging/media/imx/imx-ic-prp.c| 16 ++---
drivers/staging/media/imx/imx-media-csi.c | 6 ++---
drivers/staging/media/imx/imx-media-dev.c | 22 ++
.../staging/media/imx/imx-media-internal-sd.c | 20
drivers/staging/media/imx/imx-media-utils.c | 12 +-
drivers/staging/media/imx/imx-media.h | 23 ++-
7 files changed, 55 insertions(+), 50 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-common.c
b/drivers/staging/media/imx/imx-ic-common.c
index cfdd4900a3be..765919487a73 100644
--- a/drivers/staging/media/imx/imx-ic-common.c
+++ b/drivers/staging/media/imx/imx-ic-common.c
@@ -41,13 +41,13 @@ static int imx_ic_probe(struct platform_device *pdev)
pdata = priv->dev->platform_data;
priv->ipu_id = pdata->ipu_id;
switch (pdata->grp_id) {
- case IMX_MEDIA_GRP_ID_IC_PRP:
+ case IMX_MEDIA_GRP_ID_IPU_IC_PRP:
priv->task_id = IC_TASK_PRP;
break;
- case IMX_MEDIA_GRP_ID_IC_PRPENC:
+ case IMX_MEDIA_GRP_ID_IPU_IC_PRPENC:
priv->task_id = IC_TASK_ENCODER;
break;
- case IMX_MEDIA_GRP_ID_IC_PRPVF:
+ case IMX_MEDIA_GRP_ID_IPU_IC_PRPVF:
priv->task_id = IC_TASK_VIEWFINDER;
break;
default:
diff --git a/drivers/staging/media/imx/imx-ic-prp.c
b/drivers/staging/media/imx/imx-ic-prp.c
index 98923fc844ce..2702548f83cf 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -77,7 +77,7 @@ static int prp_start(struct prp_priv *priv)
priv->ipu = priv->md->ipu[ic_priv->ipu_id];
/* set IC to receive from CSI or VDI depending on source */
- src_is_vdic = !!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC);
+ src_is_vdic = !!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_IPU_VDIC);
ipu_set_ic_src_mux(priv->ipu, priv->csi_id, src_is_vdic);
@@ -237,8 +237,8 @@ static int prp_link_setup(struct media_entity *entity,
ret = -EBUSY;
goto out;
}
- if (priv->sink_sd_prpenc && (remote_sd->grp_id &
-IMX_MEDIA_GRP_ID_VDIC)) {
+ if (priv->sink_sd_prpenc &&
+ (remote_sd->grp_id & IMX_MEDIA_GRP_ID_IPU_VDIC)) {
ret = -EINVAL;
goto out;
}
@@ -259,7 +259,7 @@ static int prp_link_setup(struct media_entity *entity,
goto out;
}
if (priv->src_sd && (priv->src_sd->grp_id &
-IMX_MEDIA_GRP_ID_VDIC)) {
+IMX_MEDIA_GRP_ID_IPU_VDIC)) {
ret = -EINVAL;
goto out;
}
@@ -309,13 +309,13 @@ static int prp_link_validate(struct v4l2_subdev *sd,
return ret;
csi = imx_media_find_upstream_subdev(priv->md, &ic_priv->sd.entity,
-IMX_MEDIA_GRP_ID_CSI);
+IMX_MEDIA_GRP_ID_IPU_CSI);
if (IS_ERR(csi))
csi = NULL;
mutex_lock(&priv->lock);
- if (priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC) {
+ if (priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_IPU_VDIC) {
/*
* the ->PRPENC link cannot be enabled if the source
* is the VDIC
@@ -334,10 +334,10 @@ static int prp_link_validate(struct v4l2_subdev *sd,
if (csi) {
switch (csi->grp_id) {
- case IMX_MEDIA_GRP_ID_CSI0:
+ case IMX_MEDIA_GRP_ID_IPU_CSI0:
priv->csi_id = 0;
break;
- case IMX_MEDIA_GRP_ID_CSI1:
+ case IMX_MEDIA_GRP_ID_IPU_CSI1:
priv->csi_id = 1;
break;
default:
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 4223f8d418ae..a12fa1dd989e 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1029,10 +1029,10 @@ static int csi_link_setup(struct media_entity *entity,
remote_sd = media_entity_to_v4l2_subdev(remote->entity);
[PATCH v9 12/13] media: video-mux: add bayer formats
Add non vendor bayer formats to the allowed format array. Signed-off-by: Rui Miguel Silva --- drivers/media/platform/video-mux.c | 20 1 file changed, 20 insertions(+) diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c index c33900e3c23e..0ba30756e1e4 100644 --- a/drivers/media/platform/video-mux.c +++ b/drivers/media/platform/video-mux.c @@ -263,6 +263,26 @@ static int video_mux_set_format(struct v4l2_subdev *sd, case MEDIA_BUS_FMT_UYYVYY16_0_5X48: case MEDIA_BUS_FMT_JPEG_1X8: case MEDIA_BUS_FMT_AHSV_1X32: + case MEDIA_BUS_FMT_SBGGR8_1X8: + case MEDIA_BUS_FMT_SGBRG8_1X8: + case MEDIA_BUS_FMT_SGRBG8_1X8: + case MEDIA_BUS_FMT_SRGGB8_1X8: + case MEDIA_BUS_FMT_SBGGR10_1X10: + case MEDIA_BUS_FMT_SGBRG10_1X10: + case MEDIA_BUS_FMT_SGRBG10_1X10: + case MEDIA_BUS_FMT_SRGGB10_1X10: + case MEDIA_BUS_FMT_SBGGR12_1X12: + case MEDIA_BUS_FMT_SGBRG12_1X12: + case MEDIA_BUS_FMT_SGRBG12_1X12: + case MEDIA_BUS_FMT_SRGGB12_1X12: + case MEDIA_BUS_FMT_SBGGR14_1X14: + case MEDIA_BUS_FMT_SGBRG14_1X14: + case MEDIA_BUS_FMT_SGRBG14_1X14: + case MEDIA_BUS_FMT_SRGGB14_1X14: + case MEDIA_BUS_FMT_SBGGR16_1X16: + case MEDIA_BUS_FMT_SGBRG16_1X16: + case MEDIA_BUS_FMT_SGRBG16_1X16: + case MEDIA_BUS_FMT_SRGGB16_1X16: break; default: sdformat->format.code = MEDIA_BUS_FMT_Y8_1X8; -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v9 11/13] media: staging/imx: add i.MX7 entries to TODO file
Add some i.MX7 related entries to TODO file. Signed-off-by: Rui Miguel Silva --- drivers/staging/media/imx/TODO | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/staging/media/imx/TODO b/drivers/staging/media/imx/TODO index aeeb15494a49..6f29b5ca5324 100644 --- a/drivers/staging/media/imx/TODO +++ b/drivers/staging/media/imx/TODO @@ -45,3 +45,12 @@ Which means a port must not contain mixed-use endpoints, they must all refer to media links between V4L2 subdevices. + +- i.MX7: all of the above, since it uses the imx media core + +- i.MX7: use Frame Interval Monitor + +- i.MX7: runtime testing with parallel sensor, links setup and streaming + +- i.MX7: runtime testing with different formats, for the time only 10-bit bayer + is tested -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: arche-platform: Switch to the gpio descriptor interface
On Tue, Nov 20, 2018 at 10:49:54AM +0100, Greg Kroah-Hartman wrote: > On Sat, Nov 17, 2018 at 04:40:27PM +0100, Johan Hovold wrote: > > On Sat, Nov 17, 2018 at 12:41:11PM +0530, Nishad Kamdar wrote: > > > On Fri, Nov 16, 2018 at 05:06:22PM +0100, Johan Hovold wrote: > > > > On Fri, Nov 16, 2018 at 08:47:44PM +0530, Nishad Kamdar wrote: > > > > > Use the gpiod interface instead of the deprecated > > > > > old non-descriptor interface. > > > > > > > > > > Signed-off-by: Nishad Kamdar > > > > > --- > > > > > > > > Always include a change log here after the cut-off line so we know what > > > > changed since previous versions. > > > > > > > > Also include the patch revision in the Subject (e.g. "[PATCH v3] > > > > staging: greybus: ..."). > > > > > > > > > > Ok, but this is the first patch version that I submitted > > > for greybus: arche-platform. > > > > Ah, sorry. I thought this was an update of the arche-apb-ctrl patch. > > Me too. > > I am totally confused as to what is, and is not, the latest versions of > all of these patches :( > > Nishad, can you resend all of your pending greybus patches, as a patch > series, as "v3" so that I know which to properly review? I've dropped > all of your others from my review queue now. > > thanks, > > greg k-h Ok, I'll do that. Thanks for the review. Regards, Nishad ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/3] greybus: gpio: Switch to the gpio descriptor interface
This patch series converts uses of the old GPIO API to the GPIO descriptor API. It also converts the GPIO driver to use the GPIO irqchip library GPIOLIB_IRQCHIP instead of repimplementing the same. Changes in v3: - Combines the latest versions of the three greybus patches together in a patch series. Nishad Kamdar (3): staging: greybus: gpio: switch GPIO portions to use GPIOLIB_IRQCHIP staging: greybus: arche-apb-ctrl.c: Switch to the gpio descriptor interface staging: greybus: arche-platform: Switch to the gpio descriptor interface drivers/staging/greybus/Kconfig | 1 + drivers/staging/greybus/arche-apb-ctrl.c | 159 drivers/staging/greybus/arche-platform.c | 119 +-- drivers/staging/greybus/gpio.c | 184 +++ 4 files changed, 130 insertions(+), 333 deletions(-) -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/3] staging: greybus: gpio: switch GPIO portions to use GPIOLIB_IRQCHIP
Convert the GPIO driver to use the GPIO irqchip library
GPIOLIB_IRQCHIP instead of reimplementing the same.
Signed-off-by: Nishad Kamdar
---
Changes in v2:
- Retained irq.h and irqdomain.h headers.
- Dropped function gb_gpio_irqchip_add() and
called gpiochip_irqchip_add() from probe().
- Referred
https://lkml.kernel.org/r/1476054589-28422-1-git-send-email-linus.wall...@linaro.org.
---
drivers/staging/greybus/Kconfig | 1 +
drivers/staging/greybus/gpio.c | 184
2 files changed, 24 insertions(+), 161 deletions(-)
diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig
index ab096bcef98c..b571e4e8060b 100644
--- a/drivers/staging/greybus/Kconfig
+++ b/drivers/staging/greybus/Kconfig
@@ -148,6 +148,7 @@ if GREYBUS_BRIDGED_PHY
config GREYBUS_GPIO
tristate "Greybus GPIO Bridged PHY driver"
depends on GPIOLIB
+ select GPIOLIB_IRQCHIP
---help---
Select this option if you have a device that follows the
Greybus GPIO Bridged PHY Class specification.
diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c
index b1d4698019a1..2ec54744171d 100644
--- a/drivers/staging/greybus/gpio.c
+++ b/drivers/staging/greybus/gpio.c
@@ -9,9 +9,9 @@
#include
#include
#include
-#include
#include
#include
+#include
#include
#include "greybus.h"
@@ -39,15 +39,8 @@ struct gb_gpio_controller {
struct gpio_chipchip;
struct irq_chip irqc;
- struct irq_chip *irqchip;
- struct irq_domain *irqdomain;
- unsigned intirq_base;
- irq_flow_handler_t irq_handler;
- unsigned intirq_default_type;
struct mutexirq_lock;
};
-#define gpio_chip_to_gb_gpio_controller(chip) \
- container_of(chip, struct gb_gpio_controller, chip)
#define irq_data_to_gpio_chip(d) (d->domain->host_data)
static int gb_gpio_line_count_operation(struct gb_gpio_controller *ggc)
@@ -276,7 +269,7 @@ static void _gb_gpio_irq_set_type(struct gb_gpio_controller
*ggc,
static void gb_gpio_irq_mask(struct irq_data *d)
{
struct gpio_chip *chip = irq_data_to_gpio_chip(d);
- struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+ struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
struct gb_gpio_line *line = &ggc->lines[d->hwirq];
line->masked = true;
@@ -286,7 +279,7 @@ static void gb_gpio_irq_mask(struct irq_data *d)
static void gb_gpio_irq_unmask(struct irq_data *d)
{
struct gpio_chip *chip = irq_data_to_gpio_chip(d);
- struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+ struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
struct gb_gpio_line *line = &ggc->lines[d->hwirq];
line->masked = false;
@@ -296,7 +289,7 @@ static void gb_gpio_irq_unmask(struct irq_data *d)
static int gb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
{
struct gpio_chip *chip = irq_data_to_gpio_chip(d);
- struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+ struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
struct gb_gpio_line *line = &ggc->lines[d->hwirq];
struct device *dev = &ggc->gbphy_dev->dev;
u8 irq_type;
@@ -334,7 +327,7 @@ static int gb_gpio_irq_set_type(struct irq_data *d,
unsigned int type)
static void gb_gpio_irq_bus_lock(struct irq_data *d)
{
struct gpio_chip *chip = irq_data_to_gpio_chip(d);
- struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+ struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
mutex_lock(&ggc->irq_lock);
}
@@ -342,7 +335,7 @@ static void gb_gpio_irq_bus_lock(struct irq_data *d)
static void gb_gpio_irq_bus_sync_unlock(struct irq_data *d)
{
struct gpio_chip *chip = irq_data_to_gpio_chip(d);
- struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+ struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
struct gb_gpio_line *line = &ggc->lines[d->hwirq];
if (line->irq_type_pending) {
@@ -391,7 +384,7 @@ static int gb_gpio_request_handler(struct gb_operation *op)
return -EINVAL;
}
- irq = irq_find_mapping(ggc->irqdomain, event->which);
+ irq = irq_find_mapping(ggc->chip.irq.domain, event->which);
if (!irq) {
dev_err(dev, "failed to find IRQ\n");
return -EINVAL;
@@ -411,21 +404,21 @@ static int gb_gpio_request_handler(struct gb_operation
*op)
static int gb_gpio_request(struct gpio_chip *chip, unsigned int offset)
{
- struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+ struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
return gb_gpio_activate_operation(ggc, (u8)offset);
}
static void gb_gpio_free(struct gpio_chip *chip, unsigned int o
[PATCH v3 2/3] staging: greybus: arche-apb-ctrl.c: Switch to the gpio descriptor interface
Use the gpiod interface instead of the deprecated old non-descriptor
interface.
Signed-off-by: Nishad Kamdar
---
Changes in v2:
- Resolved compilation errors.
---
drivers/staging/greybus/arche-apb-ctrl.c | 159 +--
1 file changed, 65 insertions(+), 94 deletions(-)
diff --git a/drivers/staging/greybus/arche-apb-ctrl.c
b/drivers/staging/greybus/arche-apb-ctrl.c
index be5ffed90bcf..e887f6aee20b 100644
--- a/drivers/staging/greybus/arche-apb-ctrl.c
+++ b/drivers/staging/greybus/arche-apb-ctrl.c
@@ -8,9 +8,8 @@
#include
#include
-#include
+#include
#include
-#include
#include
#include
#include
@@ -24,12 +23,12 @@ static void apb_bootret_deassert(struct device *dev);
struct arche_apb_ctrl_drvdata {
/* Control GPIO signals to and from AP <=> AP Bridges */
- int resetn_gpio;
- int boot_ret_gpio;
- int pwroff_gpio;
- int wake_in_gpio;
- int wake_out_gpio;
- int pwrdn_gpio;
+ struct gpio_desc *resetn;
+ struct gpio_desc *boot_ret;
+ struct gpio_desc *pwroff;
+ struct gpio_desc *wake_in;
+ struct gpio_desc *wake_out;
+ struct gpio_desc *pwrdn;
enum arche_platform_state state;
bool init_disabled;
@@ -37,28 +36,28 @@ struct arche_apb_ctrl_drvdata {
struct regulator *vcore;
struct regulator *vio;
- int clk_en_gpio;
+ struct gpio_desc *clk_en;
struct clk *clk;
struct pinctrl *pinctrl;
struct pinctrl_state *pin_default;
/* V2: SPI Bus control */
- int spi_en_gpio;
+ struct gpio_desc *spi_en;
bool spi_en_polarity_high;
};
/*
* Note that these low level api's are active high
*/
-static inline void deassert_reset(unsigned int gpio)
+static inline void deassert_reset(struct gpio_desc *gpio)
{
- gpio_set_value(gpio, 1);
+ gpiod_set_value(gpio, 1);
}
-static inline void assert_reset(unsigned int gpio)
+static inline void assert_reset(struct gpio_desc *gpio)
{
- gpio_set_value(gpio, 0);
+ gpiod_set_value(gpio, 0);
}
/*
@@ -75,11 +74,11 @@ static int coldboot_seq(struct platform_device *pdev)
return 0;
/* Hold APB in reset state */
- assert_reset(apb->resetn_gpio);
+ assert_reset(apb->resetn);
if (apb->state == ARCHE_PLATFORM_STATE_FW_FLASHING &&
- gpio_is_valid(apb->spi_en_gpio))
- devm_gpio_free(dev, apb->spi_en_gpio);
+ apb->spi_en)
+ devm_gpiod_put(dev, apb->spi_en);
/* Enable power to APB */
if (!IS_ERR(apb->vcore)) {
@@ -101,13 +100,13 @@ static int coldboot_seq(struct platform_device *pdev)
apb_bootret_deassert(dev);
/* On DB3 clock was not mandatory */
- if (gpio_is_valid(apb->clk_en_gpio))
- gpio_set_value(apb->clk_en_gpio, 1);
+ if (apb->clk_en)
+ gpiod_set_value(apb->clk_en, 1);
usleep_range(100, 200);
/* deassert reset to APB : Active-low signal */
- deassert_reset(apb->resetn_gpio);
+ deassert_reset(apb->resetn);
apb->state = ARCHE_PLATFORM_STATE_ACTIVE;
@@ -119,6 +118,7 @@ static int fw_flashing_seq(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct arche_apb_ctrl_drvdata *apb = platform_get_drvdata(pdev);
int ret;
+ unsigned long flags;
if (apb->init_disabled ||
apb->state == ARCHE_PLATFORM_STATE_FW_FLASHING)
@@ -136,25 +136,20 @@ static int fw_flashing_seq(struct platform_device *pdev)
return ret;
}
- if (gpio_is_valid(apb->spi_en_gpio)) {
- unsigned long flags;
+ if (apb->spi_en_polarity_high)
+ flags = GPIOD_OUT_HIGH;
+ else
+ flags = GPIOD_OUT_LOW;
- if (apb->spi_en_polarity_high)
- flags = GPIOF_OUT_INIT_HIGH;
- else
- flags = GPIOF_OUT_INIT_LOW;
-
- ret = devm_gpio_request_one(dev, apb->spi_en_gpio,
- flags, "apb_spi_en");
- if (ret) {
- dev_err(dev, "Failed requesting SPI bus en gpio %d\n",
- apb->spi_en_gpio);
- return ret;
- }
+ apb->spi_en = devm_gpiod_get(dev, "gb,spi-en-gpio", flags);
+ if (IS_ERR(apb->spi_en)) {
+ ret = PTR_ERR(apb->spi_en);
+ dev_err(dev, "Failed requesting SPI bus en GPIO: %d\n", ret);
+ return ret;
}
/* for flashing device should be in reset state */
- assert_reset(apb->resetn_gpio);
+ assert_reset(apb->resetn);
apb->state = ARCHE_PLATFORM_STATE_FW_FLASHING;
return 0;
@@ -177,8 +172,8 @@ static int standby_boot_seq(struct platform_device *pdev)
return 0;
if (apb->state == ARCHE_PLATFORM_STATE_FW_F
[PATCH v3 3/3] staging: greybus: arche-platform: Switch to the gpio descriptor interface
Use the gpiod interface instead of the deprecated
old non-descriptor interface.
Signed-off-by: Nishad Kamdar
---
Changes in v2:
- Move comment to the same line as to what it applies to.
---
drivers/staging/greybus/arche-platform.c | 119 ---
1 file changed, 41 insertions(+), 78 deletions(-)
diff --git a/drivers/staging/greybus/arche-platform.c
b/drivers/staging/greybus/arche-platform.c
index 4c36e88766e7..a5cea79d8e32 100644
--- a/drivers/staging/greybus/arche-platform.c
+++ b/drivers/staging/greybus/arche-platform.c
@@ -8,10 +8,9 @@
#include
#include
-#include
+#include
#include
#include
-#include
#include
#include
#include
@@ -45,14 +44,14 @@ enum svc_wakedetect_state {
struct arche_platform_drvdata {
/* Control GPIO signals to and from AP <=> SVC */
- int svc_reset_gpio;
+ struct gpio_desc *svc_reset;
+ struct gpio_desc *svc_sysboot;
bool is_reset_act_hi;
- int svc_sysboot_gpio;
- int wake_detect_gpio; /* bi-dir,maps to WAKE_MOD & WAKE_FRAME signals */
+ struct gpio_desc *wake_detect; /* bi-dir,maps to WAKE_MOD & WAKE_FRAME
signals */
enum arche_platform_state state;
- int svc_refclk_req;
+ struct gpio_desc *svc_refclk_req;
struct clk *svc_ref_clk;
struct pinctrl *pinctrl;
@@ -85,9 +84,9 @@ static void arche_platform_set_wake_detect_state(
arche_pdata->wake_detect_state = state;
}
-static inline void svc_reset_onoff(unsigned int gpio, bool onoff)
+static inline void svc_reset_onoff(struct gpio_desc *gpio, bool onoff)
{
- gpio_set_value(gpio, onoff);
+ gpiod_set_value(gpio, onoff);
}
static int apb_cold_boot(struct device *dev, void *data)
@@ -116,7 +115,6 @@ static int apb_poweroff(struct device *dev, void *data)
static void arche_platform_wd_irq_en(struct arche_platform_drvdata
*arche_pdata)
{
/* Enable interrupt here, to read event back from SVC */
- gpio_direction_input(arche_pdata->wake_detect_gpio);
enable_irq(arche_pdata->wake_detect_irq);
}
@@ -160,7 +158,7 @@ static irqreturn_t arche_platform_wd_irq(int irq, void
*devid)
spin_lock_irqsave(&arche_pdata->wake_lock, flags);
- if (gpio_get_value(arche_pdata->wake_detect_gpio)) {
+ if (gpiod_get_value(arche_pdata->wake_detect)) {
/* wake/detect rising */
/*
@@ -224,10 +222,10 @@ arche_platform_coldboot_seq(struct arche_platform_drvdata
*arche_pdata)
dev_info(arche_pdata->dev, "Booting from cold boot state\n");
- svc_reset_onoff(arche_pdata->svc_reset_gpio,
+ svc_reset_onoff(arche_pdata->svc_reset,
arche_pdata->is_reset_act_hi);
- gpio_set_value(arche_pdata->svc_sysboot_gpio, 0);
+ gpiod_set_value(arche_pdata->svc_sysboot, 0);
usleep_range(100, 200);
ret = clk_prepare_enable(arche_pdata->svc_ref_clk);
@@ -238,7 +236,7 @@ arche_platform_coldboot_seq(struct arche_platform_drvdata
*arche_pdata)
}
/* bring SVC out of reset */
- svc_reset_onoff(arche_pdata->svc_reset_gpio,
+ svc_reset_onoff(arche_pdata->svc_reset,
!arche_pdata->is_reset_act_hi);
arche_platform_set_state(arche_pdata, ARCHE_PLATFORM_STATE_ACTIVE);
@@ -259,10 +257,10 @@ arche_platform_fw_flashing_seq(struct
arche_platform_drvdata *arche_pdata)
dev_info(arche_pdata->dev, "Switching to FW flashing state\n");
- svc_reset_onoff(arche_pdata->svc_reset_gpio,
+ svc_reset_onoff(arche_pdata->svc_reset,
arche_pdata->is_reset_act_hi);
- gpio_set_value(arche_pdata->svc_sysboot_gpio, 1);
+ gpiod_set_value(arche_pdata->svc_sysboot, 1);
usleep_range(100, 200);
@@ -273,7 +271,7 @@ arche_platform_fw_flashing_seq(struct
arche_platform_drvdata *arche_pdata)
return ret;
}
- svc_reset_onoff(arche_pdata->svc_reset_gpio,
+ svc_reset_onoff(arche_pdata->svc_reset,
!arche_pdata->is_reset_act_hi);
arche_platform_set_state(arche_pdata, ARCHE_PLATFORM_STATE_FW_FLASHING);
@@ -305,7 +303,7 @@ arche_platform_poweroff_seq(struct arche_platform_drvdata
*arche_pdata)
clk_disable_unprepare(arche_pdata->svc_ref_clk);
/* As part of exit, put APB back in reset state */
- svc_reset_onoff(arche_pdata->svc_reset_gpio,
+ svc_reset_onoff(arche_pdata->svc_reset,
arche_pdata->is_reset_act_hi);
arche_platform_set_state(arche_pdata, ARCHE_PLATFORM_STATE_OFF);
@@ -435,6 +433,7 @@ static int arche_platform_probe(struct platform_device
*pdev)
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
int ret;
+ unsigned int flags;
arche_pdata = devm_kzalloc(&pdev->dev, sizeof(*arche_pdata),
GFP_KERNEL);
@@ -444,61 +443,33 @@ static int arche_platform
[PATCH v2 00/10] staging: erofs: decompression stability enhancement
Hi,
This patchset mainly focuses on erofs decompression stability, most of
them are found in the internal beta test. Some patches which are still
in testing or reviewing aren't included in this patchset and will be
sent after carefully testing.
Thanks,
Gao Xiang
Changelog from v1
===
o fix a redundant BUG_ON suggested by Greg since it never happens at all;
o add more description suggested by Greg in commit message of
[01/10] staging: erofs: fix `trace_erofs_readpage' position
o add some description suggested by Greg of a memory barrier occurred in
[06/10] staging: erofs: add a full barrier in erofs_workgroup_unfreeze
Shortlog
===
Gao Xiang (10):
staging: erofs: fix `trace_erofs_readpage' position
staging: erofs: fix the definition of DBG_BUGON
staging: erofs: fix race when the managed cache is enabled
staging: erofs: atomic_cond_read_relaxed on ref-locked workgroup
staging: erofs: fix `erofs_workgroup_{try_to_freeze, unfreeze}'
staging: erofs: add a full barrier in erofs_workgroup_unfreeze
staging: erofs: separate into init_once / always
staging: erofs: locked before registering for all new workgroups
staging: erofs: decompress asynchronously if PG_readahead page at
first
staging: erofs: rename strange variable names in z_erofs_vle_frontend
drivers/staging/erofs/internal.h | 73 +
drivers/staging/erofs/unzip_vle.c | 79 +++---
drivers/staging/erofs/utils.c | 135 +++---
3 files changed, 199 insertions(+), 88 deletions(-)
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 02/10] staging: erofs: fix the definition of DBG_BUGON
From: Gao Xiang
It's better not to positively BUG_ON the kernel, however developers
need a way to locate issues as soon as possible.
DBG_BUGON is introduced and it could only crash when EROFS_FS_DEBUG
(EROFS developping feature) is on. It is helpful for developers
to find and solve bugs quickly by eng builds.
Previously, DBG_BUGON is defined as ((void)0) if EROFS_FS_DEBUG is off,
but some unused variable warnings as follows could occur:
drivers/staging/erofs/unzip_vle.c: In function `init_alway:':
drivers/staging/erofs/unzip_vle.c:61:33: warning: unused variable `work'
[-Wunused-variable]
struct z_erofs_vle_work *const work =
^~~~
Fix it to #define DBG_BUGON(x) ((void)(x)).
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 57575c7f5635..048fb034b5aa 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -39,7 +39,7 @@
#define debugln(x, ...) ((void)0)
#define dbg_might_sleep() ((void)0)
-#define DBG_BUGON(...) ((void)0)
+#define DBG_BUGON(x)((void)(x))
#endif
enum {
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 01/10] staging: erofs: fix `trace_erofs_readpage' position
From: Gao Xiang
`trace_erofs_readpage' was designed for .readpage() interface hook
in order to trace the detailed synchronized read rather than
the internal implementation `z_erofs_do_read_page' which both
.readpage() and .readpages() use.
It seems the tracepoint was placed to a wrong place by mistake and
it should be moved to the right place.
Fixes: 284db12cfda3 ("staging: erofs: add trace points for reading zipped data")
Reviewed-by: Chen Gong
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index 6a283f618f46..ede3383ac601 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -598,8 +598,6 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend
*fe,
unsigned int cur, end, spiltted, index;
int err = 0;
- trace_erofs_readpage(page, false);
-
/* register locked file pages as online pages in pack */
z_erofs_onlinepage_init(page);
@@ -1288,6 +1286,8 @@ static int z_erofs_vle_normalaccess_readpage(struct file
*file,
int err;
LIST_HEAD(pagepool);
+ trace_erofs_readpage(page, false);
+
#if (EROFS_FS_ZIP_CACHE_LVL >= 2)
f.cachedzone_la = (erofs_off_t)page->index << PAGE_SHIFT;
#endif
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 03/10] staging: erofs: fix race when the managed cache is enabled
From: Gao Xiang
When the managed cache is enabled, the last reference count
of a workgroup must be used for its workstation.
Otherwise, it could lead to incorrect (un)freezes in
the reclaim path, and it would be harmful.
A typical race as follows:
Thread 1 (In the reclaim path) Thread 2
workgroup_freeze(grp, 1)refcnt = 1
...
workgroup_unfreeze(grp, 1) refcnt = 1
workgroup_get(grp) refcnt = 2 (x)
workgroup_put(grp) refcnt = 1 (x)
...unexpected behaviors
* grp is detached but still used, which violates cache-managed
freeze constraint.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/internal.h | 1 +
drivers/staging/erofs/utils.c| 134 +++
2 files changed, 96 insertions(+), 39 deletions(-)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 048fb034b5aa..3ac4599bbe01 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -250,6 +250,7 @@ static inline bool erofs_workgroup_get(struct
erofs_workgroup *grp, int *ocnt)
}
#define __erofs_workgroup_get(grp) atomic_inc(&(grp)->refcount)
+#define __erofs_workgroup_put(grp) atomic_dec(&(grp)->refcount)
extern int erofs_workgroup_put(struct erofs_workgroup *grp);
diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
index ea8a962e5c95..d2e3ace91046 100644
--- a/drivers/staging/erofs/utils.c
+++ b/drivers/staging/erofs/utils.c
@@ -83,12 +83,21 @@ int erofs_register_workgroup(struct super_block *sb,
grp = xa_tag_pointer(grp, tag);
- err = radix_tree_insert(&sbi->workstn_tree,
- grp->index, grp);
+ /*
+* Bump up reference count before making this workgroup
+* visible to other users in order to avoid potential UAF
+* without serialized by erofs_workstn_lock.
+*/
+ __erofs_workgroup_get(grp);
- if (!err) {
- __erofs_workgroup_get(grp);
- }
+ err = radix_tree_insert(&sbi->workstn_tree,
+ grp->index, grp);
+ if (unlikely(err))
+ /*
+* it's safe to decrease since the workgroup isn't visible
+* and refcount >= 2 (cannot be freezed).
+*/
+ __erofs_workgroup_put(grp);
erofs_workstn_unlock(sbi);
radix_tree_preload_end();
@@ -97,19 +106,94 @@ int erofs_register_workgroup(struct super_block *sb,
extern void erofs_workgroup_free_rcu(struct erofs_workgroup *grp);
+static void __erofs_workgroup_free(struct erofs_workgroup *grp)
+{
+ atomic_long_dec(&erofs_global_shrink_cnt);
+ erofs_workgroup_free_rcu(grp);
+}
+
int erofs_workgroup_put(struct erofs_workgroup *grp)
{
int count = atomic_dec_return(&grp->refcount);
if (count == 1)
atomic_long_inc(&erofs_global_shrink_cnt);
- else if (!count) {
- atomic_long_dec(&erofs_global_shrink_cnt);
- erofs_workgroup_free_rcu(grp);
- }
+ else if (!count)
+ __erofs_workgroup_free(grp);
return count;
}
+#ifdef EROFS_FS_HAS_MANAGED_CACHE
+/* for cache-managed case, customized reclaim paths exist */
+static void erofs_workgroup_unfreeze_final(struct erofs_workgroup *grp)
+{
+ erofs_workgroup_unfreeze(grp, 0);
+ __erofs_workgroup_free(grp);
+}
+
+bool erofs_try_to_release_workgroup(struct erofs_sb_info *sbi,
+ struct erofs_workgroup *grp,
+ bool cleanup)
+{
+ /*
+* for managed cache enabled, the refcount of workgroups
+* themselves could be < 0 (freezed). So there is no guarantee
+* that all refcount > 0 if managed cache is enabled.
+*/
+ if (!erofs_workgroup_try_to_freeze(grp, 1))
+ return false;
+
+ /*
+* note that all cached pages should be unlinked
+* before delete it from the radix tree.
+* Otherwise some cached pages of an orphan old workgroup
+* could be still linked after the new one is available.
+*/
+ if (erofs_try_to_free_all_cached_pages(sbi, grp)) {
+ erofs_workgroup_unfreeze(grp, 1);
+ return false;
+ }
+
+ /*
+* it is impossible to fail after the workgroup is freezed,
+* however in order to avoid some race conditions, add a
+* DBG_BUGON to observe this in advance.
+*/
+ DBG_BUGON(xa_untag_pointer(radix_tree_delete(&sbi->workstn_tree,
+grp->index)) != grp);
+
+ /*
+* if managed cache is enable, the last refcount
+* should indicate the related workstation.
+*/
+ erofs_workgroup_unfreeze_f
[PATCH v2 04/10] staging: erofs: atomic_cond_read_relaxed on ref-locked workgroup
From: Gao Xiang
It's better to use atomic_cond_read_relaxed, which is implemented
in hardware instructions to monitor a variable changes currently
for ARM64, instead of open-coded busy waiting.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/internal.h | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 3ac4599bbe01..f933ab602c37 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -221,23 +221,29 @@ static inline void erofs_workgroup_unfreeze(
preempt_enable();
}
+#if defined(CONFIG_SMP)
+static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
+{
+ return atomic_cond_read_relaxed(&grp->refcount,
+ VAL != EROFS_LOCKED_MAGIC);
+}
+#else
+static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
+{
+ int v = atomic_read(&grp->refcount);
+
+ /* workgroup is never freezed on uniprocessor systems */
+ DBG_BUGON(v == EROFS_LOCKED_MAGIC);
+ return v;
+}
+#endif
+
static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
{
- const int locked = (int)EROFS_LOCKED_MAGIC;
int o;
repeat:
- o = atomic_read(&grp->refcount);
-
- /* spin if it is temporarily locked at the reclaim path */
- if (unlikely(o == locked)) {
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
- do
- cpu_relax();
- while (atomic_read(&grp->refcount) == locked);
-#endif
- goto repeat;
- }
+ o = erofs_wait_on_workgroup_freezed(grp);
if (unlikely(o <= 0))
return -1;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 05/10] staging: erofs: fix `erofs_workgroup_{try_to_freeze, unfreeze}'
From: Gao Xiang
There are two minor issues in the current freeze interface:
1) Freeze interfaces have not related with CONFIG_DEBUG_SPINLOCK,
therefore fix the incorrect conditions;
2) For SMP platforms, it should also disable preemption before
doing atomic_cmpxchg in case that some high priority tasks
preempt between atomic_cmpxchg and disable_preempt, then spin
on the locked refcount later.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/internal.h | 41
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index f933ab602c37..399a7003e783 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -194,40 +194,49 @@ struct erofs_workgroup {
#define EROFS_LOCKED_MAGIC (INT_MIN | 0xE0F510CCL)
-static inline bool erofs_workgroup_try_to_freeze(
- struct erofs_workgroup *grp, int v)
+#if defined(CONFIG_SMP)
+static inline bool erofs_workgroup_try_to_freeze(struct erofs_workgroup *grp,
+int val)
{
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
- if (v != atomic_cmpxchg(&grp->refcount,
- v, EROFS_LOCKED_MAGIC))
- return false;
preempt_disable();
-#else
- preempt_disable();
- if (atomic_read(&grp->refcount) != v) {
+ if (val != atomic_cmpxchg(&grp->refcount, val, EROFS_LOCKED_MAGIC)) {
preempt_enable();
return false;
}
-#endif
return true;
}
-static inline void erofs_workgroup_unfreeze(
- struct erofs_workgroup *grp, int v)
+static inline void erofs_workgroup_unfreeze(struct erofs_workgroup *grp,
+ int orig_val)
{
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
- atomic_set(&grp->refcount, v);
-#endif
+ atomic_set(&grp->refcount, orig_val);
preempt_enable();
}
-#if defined(CONFIG_SMP)
static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
{
return atomic_cond_read_relaxed(&grp->refcount,
VAL != EROFS_LOCKED_MAGIC);
}
#else
+static inline bool erofs_workgroup_try_to_freeze(struct erofs_workgroup *grp,
+int val)
+{
+ preempt_disable();
+ /* no need to spin on UP platforms, let's just disable preemption. */
+ if (val != atomic_read(&grp->refcount)) {
+ preempt_enable();
+ return false;
+ }
+ return true;
+}
+
+static inline void erofs_workgroup_unfreeze(struct erofs_workgroup *grp,
+ int orig_val)
+{
+ preempt_enable();
+}
+
static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
{
int v = atomic_read(&grp->refcount);
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 06/10] staging: erofs: add a full barrier in erofs_workgroup_unfreeze
From: Gao Xiang
Just like other generic locks, insert a full barrier
in case of memory reorder.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/internal.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 399a7003e783..892944355867 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -209,6 +209,11 @@ static inline bool erofs_workgroup_try_to_freeze(struct
erofs_workgroup *grp,
static inline void erofs_workgroup_unfreeze(struct erofs_workgroup *grp,
int orig_val)
{
+ /*
+* other observers should notice all modifications
+* in the freezing period.
+*/
+ smp_mb();
atomic_set(&grp->refcount, orig_val);
preempt_enable();
}
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 08/10] staging: erofs: locked before registering for all new workgroups
From: Gao Xiang
Let's make sure that the one registering a workgroup will also
take the primary work lock at first for two reasons:
1) There's no need to introduce such a race window (and consequently
overhead) between registering and locking, other tasks could break
in by chance, and the race seems unnecessary (no benefit at all);
2) It's better to take the primary work when a workgroup
is registered to apply the cache managed policy, for example,
if some other tasks break in, it could turn into the in-place
decompression rather than use as the cached decompression.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index 4e5843e8ee35..6aa3c989dd4e 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -420,18 +420,23 @@ z_erofs_vle_work_register(const struct
z_erofs_vle_work_finder *f,
work = z_erofs_vle_grab_primary_work(grp);
work->pageofs = f->pageofs;
+ /*
+* lock all primary followed works before visible to others
+* and mutex_trylock *never* fails for a new workgroup.
+*/
+ mutex_trylock(&work->lock);
+
if (gnew) {
int err = erofs_register_workgroup(f->sb, &grp->obj, 0);
if (err) {
+ mutex_unlock(&work->lock);
kmem_cache_free(z_erofs_workgroup_cachep, grp);
return ERR_PTR(-EAGAIN);
}
}
*f->owned_head = *f->grp_ret = grp;
-
- mutex_lock(&work->lock);
return work;
}
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 07/10] staging: erofs: separate into init_once / always
From: Gao Xiang
`z_erofs_vle_workgroup' is heavily generated in the decompression,
for example, it resets 32 bytes redundantly for 64-bit platforms
even through Z_EROFS_VLE_INLINE_PAGEVECS + Z_EROFS_CLUSTER_MAX_PAGES,
default 4, pages are stored in `z_erofs_vle_workgroup'.
As an another example, `struct mutex' takes 72 bytes for our kirin
64-bit platforms, it's unnecessary to be reseted at first and
be initialized each time.
Let's avoid filling all `z_erofs_vle_workgroup' with 0 at first
since most fields are reinitialized to meaningful values later,
and pagevec is no need to initialized at all.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 34 +-
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index ede3383ac601..4e5843e8ee35 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -43,12 +43,38 @@ static inline int init_unzip_workqueue(void)
return z_erofs_workqueue ? 0 : -ENOMEM;
}
+static void init_once(void *ptr)
+{
+ struct z_erofs_vle_workgroup *grp = ptr;
+ struct z_erofs_vle_work *const work =
+ z_erofs_vle_grab_primary_work(grp);
+ unsigned int i;
+
+ mutex_init(&work->lock);
+ work->nr_pages = 0;
+ work->vcnt = 0;
+ for (i = 0; i < Z_EROFS_CLUSTER_MAX_PAGES; ++i)
+ grp->compressed_pages[i] = NULL;
+}
+
+static void init_always(struct z_erofs_vle_workgroup *grp)
+{
+ struct z_erofs_vle_work *const work =
+ z_erofs_vle_grab_primary_work(grp);
+
+ atomic_set(&grp->obj.refcount, 1);
+ grp->flags = 0;
+
+ DBG_BUGON(work->nr_pages);
+ DBG_BUGON(work->vcnt);
+}
+
int __init z_erofs_init_zip_subsystem(void)
{
z_erofs_workgroup_cachep =
kmem_cache_create("erofs_compress",
Z_EROFS_WORKGROUP_SIZE, 0,
- SLAB_RECLAIM_ACCOUNT, NULL);
+ SLAB_RECLAIM_ACCOUNT, init_once);
if (z_erofs_workgroup_cachep) {
if (!init_unzip_workqueue())
@@ -370,10 +396,11 @@ z_erofs_vle_work_register(const struct
z_erofs_vle_work_finder *f,
BUG_ON(grp);
/* no available workgroup, let's allocate one */
- grp = kmem_cache_zalloc(z_erofs_workgroup_cachep, GFP_NOFS);
+ grp = kmem_cache_alloc(z_erofs_workgroup_cachep, GFP_NOFS);
if (unlikely(!grp))
return ERR_PTR(-ENOMEM);
+ init_always(grp);
grp->obj.index = f->idx;
grp->llen = map->m_llen;
@@ -381,7 +408,6 @@ z_erofs_vle_work_register(const struct
z_erofs_vle_work_finder *f,
(map->m_flags & EROFS_MAP_ZIPPED) ?
Z_EROFS_VLE_WORKGRP_FMT_LZ4 :
Z_EROFS_VLE_WORKGRP_FMT_PLAIN);
- atomic_set(&grp->obj.refcount, 1);
/* new workgrps have been claimed as type 1 */
WRITE_ONCE(grp->next, *f->owned_head);
@@ -394,8 +420,6 @@ z_erofs_vle_work_register(const struct
z_erofs_vle_work_finder *f,
work = z_erofs_vle_grab_primary_work(grp);
work->pageofs = f->pageofs;
- mutex_init(&work->lock);
-
if (gnew) {
int err = erofs_register_workgroup(f->sb, &grp->obj, 0);
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 09/10] staging: erofs: decompress asynchronously if PG_readahead page at first
From: Gao Xiang
For the case of nr_to_read == lookahead_size, it is better to
decompress asynchronously as well since no page will be needed immediately.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index 6aa3c989dd4e..fab907e0fe06 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1345,8 +1345,8 @@ static int z_erofs_vle_normalaccess_readpages(struct file
*filp,
{
struct inode *const inode = mapping->host;
struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
- const bool sync = __should_decompress_synchronously(sbi, nr_pages);
+ bool sync = __should_decompress_synchronously(sbi, nr_pages);
struct z_erofs_vle_frontend f = VLE_FRONTEND_INIT(inode);
gfp_t gfp = mapping_gfp_constraint(mapping, GFP_KERNEL);
struct page *head = NULL;
@@ -1364,6 +1364,13 @@ static int z_erofs_vle_normalaccess_readpages(struct
file *filp,
prefetchw(&page->flags);
list_del(&page->lru);
+ /*
+* A pure asynchronous readahead is indicated if
+* a PG_readahead marked page is hitted at first.
+* Let's also do asynchronous decompression for this case.
+*/
+ sync &= !(PageReadahead(page) && !head);
+
if (add_to_page_cache_lru(page, mapping, page->index, gfp)) {
list_add(&page->lru, &pagepool);
continue;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 10/10] staging: erofs: rename strange variable names in z_erofs_vle_frontend
From: Gao Xiang
Previously, 2 members called `initial' and `cachedzone_la' are used
for applying caching policy (whether the workgroup is at either end),
which are hard to understand, rename them to `backmost' and `headoffset'.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index fab907e0fe06..f5d088fdf7f2 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -587,10 +587,9 @@ struct z_erofs_vle_frontend {
z_erofs_vle_owned_workgrp_t owned_head;
- bool initial;
-#if (EROFS_FS_ZIP_CACHE_LVL >= 2)
- erofs_off_t cachedzone_la;
-#endif
+ /* used for applying cache strategy on the fly */
+ bool backmost;
+ erofs_off_t headoffset;
};
#define VLE_FRONTEND_INIT(__i) { \
@@ -601,7 +600,7 @@ struct z_erofs_vle_frontend {
}, \
.builder = VLE_WORK_BUILDER_INIT(), \
.owned_head = Z_EROFS_VLE_WORKGRP_TAIL, \
- .initial = true, }
+ .backmost = true, }
static int z_erofs_do_read_page(struct z_erofs_vle_frontend *fe,
struct page *page,
@@ -644,7 +643,7 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend
*fe,
debugln("%s: [out-of-range] pos %llu", __func__, offset + cur);
if (z_erofs_vle_work_iter_end(builder))
- fe->initial = false;
+ fe->backmost = false;
map->m_la = offset + cur;
map->m_llen = 0;
@@ -670,8 +669,8 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend
*fe,
erofs_blknr(map->m_pa),
grp->compressed_pages, erofs_blknr(map->m_plen),
/* compressed page caching selection strategy */
- fe->initial | (EROFS_FS_ZIP_CACHE_LVL >= 2 ?
- map->m_la < fe->cachedzone_la : 0));
+ fe->backmost | (EROFS_FS_ZIP_CACHE_LVL >= 2 ?
+ map->m_la < fe->headoffset : 0));
if (noio_outoforder && builder_is_followed(builder))
builder->role = Z_EROFS_VLE_WORK_PRIMARY;
@@ -1317,9 +1316,8 @@ static int z_erofs_vle_normalaccess_readpage(struct file
*file,
trace_erofs_readpage(page, false);
-#if (EROFS_FS_ZIP_CACHE_LVL >= 2)
- f.cachedzone_la = (erofs_off_t)page->index << PAGE_SHIFT;
-#endif
+ f.headoffset = (erofs_off_t)page->index << PAGE_SHIFT;
+
err = z_erofs_do_read_page(&f, page, &pagepool);
(void)z_erofs_vle_work_iter_end(&f.builder);
@@ -1355,9 +1353,8 @@ static int z_erofs_vle_normalaccess_readpages(struct file
*filp,
trace_erofs_readpages(mapping->host, lru_to_page(pages),
nr_pages, false);
-#if (EROFS_FS_ZIP_CACHE_LVL >= 2)
- f.cachedzone_la = (erofs_off_t)lru_to_page(pages)->index << PAGE_SHIFT;
-#endif
+ f.headoffset = (erofs_off_t)lru_to_page(pages)->index << PAGE_SHIFT;
+
for (; nr_pages; --nr_pages) {
struct page *page = lru_to_page(pages);
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/10] staging: erofs: add a full barrier in erofs_workgroup_unfreeze
On Thu, Nov 22, 2018 at 06:56:32PM +0800, Gao Xiang wrote: > Hi Greg, > > On 2018/11/22 18:22, Greg Kroah-Hartman wrote: > > Please document this memory barrier. It does not make much sense to > > me... > > Because we need to make the other observers noticing the latest values > modified > in this locking period before unfreezing the whole workgroup, one way is to > use > a memory barrier and the other way is to use ACQUIRE and RELEASE. we selected > the first one. > > Hmmm...ok, I will add a simple message to explain this, but I think that is > plain enough for a lock... Sympathizing with Greg's request, let me add some specific suggestions: 1. It wouldn't hurt to indicate a pair of memory accesses which are intended to be "ordered" by the memory barrier in question (yes, this pair might not be unique, but you should be able to provide an example). 2. Memory barriers always come matched by other memory barriers, or dependencies (it really does not make sense to talk about a full barrier "in isolation"): please also indicate (an instance of) a matching barrier or the matching barriers. 3. How do the hardware threads communicate? In the acquire/release pattern you mentioned above, the load-acquire *reads from* a/the previous store-release, a memory access that follows the acquire somehow communicate with a memory access preceding the release... 4. It is a good practice to include the above information within an (inline) comment accompanying the added memory barrier (in fact, IIRC, checkpatch.pl gives you a "memory barrier without comment" warning when you omit to do so); not just in the commit message. Hope this helps. Please let me know if something I wrote is unclear, Andrea > > Thanks, > Gao Xiang > > > > > thanks, > > > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next 01/12] switchdev: SWITCHDEV_OBJ_PORT_{VLAN, MDB}(): Sanitize
The two macros SWITCHDEV_OBJ_PORT_VLAN() and SWITCHDEV_OBJ_PORT_MDB()
expand to a container_of() call, yielding an appropriate container of
their sole argument. However, due to a name collision, the first
argument, i.e. the contained object pointer, is not the only one to get
expanded. The third argument, which is a structure member name, and
should be kept literal, gets expanded as well. The only safe way to use
these two macros is therefore to name the local variable passed to them
"obj".
To fix this, rename the sole argument of the two macros from
"obj" (which collides with the member name) to "OBJ". Additionally,
instead of passing "OBJ" to container_of() verbatim, parenthesize it, so
that a comma in the passed-in expression doesn't pollute the
container_of() invocation.
Signed-off-by: Petr Machata
Acked-by: Jiri Pirko
Reviewed-by: Ido Schimmel
---
include/net/switchdev.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 7b371e7c4bc6..dd969224a9b9 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -95,8 +95,8 @@ struct switchdev_obj_port_vlan {
u16 vid_end;
};
-#define SWITCHDEV_OBJ_PORT_VLAN(obj) \
- container_of(obj, struct switchdev_obj_port_vlan, obj)
+#define SWITCHDEV_OBJ_PORT_VLAN(OBJ) \
+ container_of((OBJ), struct switchdev_obj_port_vlan, obj)
/* SWITCHDEV_OBJ_ID_PORT_MDB */
struct switchdev_obj_port_mdb {
@@ -105,8 +105,8 @@ struct switchdev_obj_port_mdb {
u16 vid;
};
-#define SWITCHDEV_OBJ_PORT_MDB(obj) \
- container_of(obj, struct switchdev_obj_port_mdb, obj)
+#define SWITCHDEV_OBJ_PORT_MDB(OBJ) \
+ container_of((OBJ), struct switchdev_obj_port_mdb, obj)
void switchdev_trans_item_enqueue(struct switchdev_trans *trans,
void *data, void (*destructor)(void const *),
--
2.4.11
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next 00/12] switchdev: Convert switchdev_port_obj_{add,del}() to notifiers
An offloading driver may need to have access to switchdev events on
ports that aren't directly under its control. An example is a VXLAN port
attached to a bridge offloaded by a driver. The driver needs to know
about VLANs configured on the VXLAN device. However the VXLAN device
isn't stashed between the bridge and a front-panel-port device (such as
is the case e.g. for LAG devices), so the usual switchdev ops don't
reach the driver.
VXLAN is likely not the only device type like this: in theory any L2
tunnel device that needs offloading will prompt requirement of this
sort.
A way to fix this is to give up the notion of port object addition /
deletion as a switchdev operation, which assumes somewhat tight coupling
between the message producer and consumer. And instead send the message
over a notifier chain.
The series starts with a clean-up patch #1, where
SWITCHDEV_OBJ_PORT_{VLAN, MDB}() are fixed up to lift the constraint
that the passed-in argument be a simple variable named "obj".
switchdev_port_obj_add and _del are invoked in a context that permits
blocking. Not only that, at least for the VLAN notification, being able
to signal failure is actually important. Therefore introduce a new
blocking notifier chain that the new events will be sent on. That's done
in patch #2. Retain the current (atomic) notifier chain for the
preexisting notifications.
In patch #3, introduce two new switchdev notifier types,
SWITCHDEV_PORT_OBJ_ADD and SWITCHDEV_PORT_OBJ_DEL. These notifier types
communicate the same event as the corresponding switchdev op, except in
a form of a notification. struct switchdev_notifier_port_obj_info was
added to carry the fields that correspond to the switchdev op arguments.
An additional field, handled, will be used to communicate back to
switchdev that the event has reached an interested party, which will be
important for the two-phase commit.
In patches #4, #5, and #7, rocker, DSA resp. ethsw are updated to
subscribe to the switchdev blocking notifier chain, and handle the new
notifier types. #6 introduces a helper to determine whether a
netdevice corresponds to a front panel port.
What these three drivers have in common is that their ports don't
support any uppers besides bridge. That makes it possible to ignore any
notifiers that don't reference a front-panel port device, because they
are certainly out of scope.
Unlike the previous three, mlxsw and ocelot drivers admit stacked
devices as uppers. While the current switchdev code recursively descends
through layers of lower devices, eventually calling the op on a
front-panel port device, the notifier would reference a stacking device
that's one of front-panel ports uppers. The filtering is thus more
complex.
For ocelot, such iteration is currently pretty much required, because
there's no bookkeeping of LAG devices. mlxsw does keep the list of LAGs,
however it iterates the lower devices anyway when deciding whether an
event on a tunnel device pertains to the driver or not.
Therefore this patch set instead introduces, in patch #8, a helper to
iterate through lowers, much like the current switchdev code does,
looking for devices that match a given predicate.
Then in patches #9 and #10, first mlxsw and then ocelot are updated to
dispatch the newly-added notifier types to the preexisting
port_obj_add/_del handlers. The dispatch is done via the new helper, to
recursively descend through lower devices.
Finally in patch #11, the actual switch is made, retiring the current
SDO-based code in favor of a notifier.
Now that the event is distributed through a notifier, the explicit
netdevice check in rocker, DSA and ethsw doesn't let through any events
except those done on a front-panel port itself. It is therefore
unnecessary to check in VLAN-handling code whether a VLAN was added to
the bridge itself: such events will simply be ignored much sooner.
Therefore remove it in patch #12.
Petr Machata (12):
switchdev: SWITCHDEV_OBJ_PORT_{VLAN, MDB}(): Sanitize
switchdev: Add a blocking notifier chain
switchdev: Add SWITCHDEV_PORT_OBJ_ADD, SWITCHDEV_PORT_OBJ_DEL
rocker: Handle SWITCHDEV_PORT_OBJ_ADD/_DEL
net: dsa: slave: Handle SWITCHDEV_PORT_OBJ_ADD/_DEL
staging: fsl-dpaa2: ethsw: Introduce ethsw_port_dev_check()
staging: fsl-dpaa2: ethsw: Handle SWITCHDEV_PORT_OBJ_ADD/_DEL
switchdev: Add helpers to aid traversal through lower devices
mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_OBJ_ADD/_DEL
ocelot: Handle SWITCHDEV_PORT_OBJ_ADD/_DEL
switchdev: Replace port obj add/del SDO with a notification
rocker, dsa, ethsw: Don't filter VLAN events on bridge itself
.../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 47 -
drivers/net/ethernet/mscc/ocelot.c | 30 +++-
drivers/net/ethernet/mscc/ocelot.h | 1 +
drivers/net/ethernet/mscc/ocelot_board.c | 3 +
drivers/net/ethernet/rocker/rocker_main.c | 60 ++-
drivers/staging/fsl-dpaa2/ethsw/ethsw.c| 68 +
[PATCH net-next 04/12] rocker: Handle SWITCHDEV_PORT_OBJ_ADD/_DEL
Following patches will change the way of distributing port object
changes from a switchdev operation to a switchdev notifier. The
switchdev code currently recursively descends through layers of lower
devices, eventually calling the op on a front-panel port device. The
notifier will instead be sent referencing the bridge port device, which
may be a stacking device that's one of front-panel ports uppers, or a
completely unrelated device.
rocker currently doesn't support any uppers other than bridge. Thus the
only case that a stacked device could be validly referenced by port
object notifications are bridge notifications for VLAN objects added to
the bridge itself. But the driver explicitly rejects such notifications
in rocker_world_port_obj_vlan_add(). It is therefore safe to assume that
the only interesting case is that the notification is on a front-panel
port netdevice.
Subscribe to the blocking notifier chain. In the handler, filter out
notifications on any foreign netdevices. Dispatch the new notifiers to
rocker_port_obj_add() resp. _del() to maintain the behavior that the
switchdev operation based code currently has.
Signed-off-by: Petr Machata
Acked-by: Jiri Pirko
---
drivers/net/ethernet/rocker/rocker_main.c | 55 +++
1 file changed, 55 insertions(+)
diff --git a/drivers/net/ethernet/rocker/rocker_main.c
b/drivers/net/ethernet/rocker/rocker_main.c
index beb06628f22d..806ffe1d906e 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2812,12 +2812,54 @@ static int rocker_switchdev_event(struct notifier_block
*unused,
return NOTIFY_DONE;
}
+static int
+rocker_switchdev_port_obj_event(unsigned long event, struct net_device *netdev,
+ struct switchdev_notifier_port_obj_info *port_obj_info)
+{
+ int err = -EOPNOTSUPP;
+
+ switch (event) {
+ case SWITCHDEV_PORT_OBJ_ADD:
+ err = rocker_port_obj_add(netdev, port_obj_info->obj,
+ port_obj_info->trans);
+ break;
+ case SWITCHDEV_PORT_OBJ_DEL:
+ err = rocker_port_obj_del(netdev, port_obj_info->obj);
+ break;
+ }
+
+ port_obj_info->handled = true;
+ return notifier_from_errno(err);
+}
+
+static int rocker_switchdev_blocking_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+
+ if (!rocker_port_dev_check(dev))
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case SWITCHDEV_PORT_OBJ_ADD:
+ case SWITCHDEV_PORT_OBJ_DEL:
+ return rocker_switchdev_port_obj_event(event, dev, ptr);
+ }
+
+ return NOTIFY_DONE;
+}
+
static struct notifier_block rocker_switchdev_notifier = {
.notifier_call = rocker_switchdev_event,
};
+static struct notifier_block rocker_switchdev_blocking_notifier = {
+ .notifier_call = rocker_switchdev_blocking_event,
+};
+
static int rocker_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
+ struct notifier_block *nb;
struct rocker *rocker;
int err;
@@ -2933,6 +2975,13 @@ static int rocker_probe(struct pci_dev *pdev, const
struct pci_device_id *id)
goto err_register_switchdev_notifier;
}
+ nb = &rocker_switchdev_blocking_notifier;
+ err = register_switchdev_blocking_notifier(nb);
+ if (err) {
+ dev_err(&pdev->dev, "Failed to register switchdev blocking
notifier\n");
+ goto err_register_switchdev_blocking_notifier;
+ }
+
rocker->hw.id = rocker_read64(rocker, SWITCH_ID);
dev_info(&pdev->dev, "Rocker switch with id %*phN\n",
@@ -2940,6 +2989,8 @@ static int rocker_probe(struct pci_dev *pdev, const
struct pci_device_id *id)
return 0;
+err_register_switchdev_blocking_notifier:
+ unregister_switchdev_notifier(&rocker_switchdev_notifier);
err_register_switchdev_notifier:
unregister_fib_notifier(&rocker->fib_nb);
err_register_fib_notifier:
@@ -2971,6 +3022,10 @@ static int rocker_probe(struct pci_dev *pdev, const
struct pci_device_id *id)
static void rocker_remove(struct pci_dev *pdev)
{
struct rocker *rocker = pci_get_drvdata(pdev);
+ struct notifier_block *nb;
+
+ nb = &rocker_switchdev_blocking_notifier;
+ unregister_switchdev_blocking_notifier(nb);
unregister_switchdev_notifier(&rocker_switchdev_notifier);
unregister_fib_notifier(&rocker->fib_nb);
--
2.4.11
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next 02/12] switchdev: Add a blocking notifier chain
In general one can't assume that a switchdev notifier is called in a
non-atomic context, and correspondingly, the switchdev notifier chain is
an atomic one.
However, port object addition and deletion messages are delivered from a
process context. Even the MDB addition messages, whose delivery is
scheduled from atomic context, are queued and the delivery itself takes
place in blocking context. For VLAN messages in particular, keeping the
blocking nature is important for error reporting.
Therefore introduce a blocking notifier chain and related service
functions to distribute the notifications for which a blocking context
can be assumed.
Signed-off-by: Petr Machata
Reviewed-by: Jiri Pirko
Reviewed-by: Ido Schimmel
---
include/net/switchdev.h | 27 +++
net/switchdev/switchdev.c | 26 ++
2 files changed, 53 insertions(+)
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index dd969224a9b9..e021b67b9b32 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -182,10 +182,17 @@ int switchdev_port_obj_add(struct net_device *dev,
const struct switchdev_obj *obj);
int switchdev_port_obj_del(struct net_device *dev,
const struct switchdev_obj *obj);
+
int register_switchdev_notifier(struct notifier_block *nb);
int unregister_switchdev_notifier(struct notifier_block *nb);
int call_switchdev_notifiers(unsigned long val, struct net_device *dev,
struct switchdev_notifier_info *info);
+
+int register_switchdev_blocking_notifier(struct notifier_block *nb);
+int unregister_switchdev_blocking_notifier(struct notifier_block *nb);
+int call_switchdev_blocking_notifiers(unsigned long val, struct net_device
*dev,
+ struct switchdev_notifier_info *info);
+
void switchdev_port_fwd_mark_set(struct net_device *dev,
struct net_device *group_dev,
bool joining);
@@ -241,6 +248,26 @@ static inline int call_switchdev_notifiers(unsigned long
val,
return NOTIFY_DONE;
}
+static inline int
+register_switchdev_blocking_notifier(struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int
+unregister_switchdev_blocking_notifier(struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int
+call_switchdev_blocking_notifiers(unsigned long val,
+ struct net_device *dev,
+ struct switchdev_notifier_info *info)
+{
+ return NOTIFY_DONE;
+}
+
static inline bool switchdev_port_same_parent_id(struct net_device *a,
struct net_device *b)
{
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 74b9d916a58b..e109bb97ce3f 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -535,6 +535,7 @@ int switchdev_port_obj_del(struct net_device *dev,
EXPORT_SYMBOL_GPL(switchdev_port_obj_del);
static ATOMIC_NOTIFIER_HEAD(switchdev_notif_chain);
+static BLOCKING_NOTIFIER_HEAD(switchdev_blocking_notif_chain);
/**
* register_switchdev_notifier - Register notifier
@@ -576,6 +577,31 @@ int call_switchdev_notifiers(unsigned long val, struct
net_device *dev,
}
EXPORT_SYMBOL_GPL(call_switchdev_notifiers);
+int register_switchdev_blocking_notifier(struct notifier_block *nb)
+{
+ struct blocking_notifier_head *chain = &switchdev_blocking_notif_chain;
+
+ return blocking_notifier_chain_register(chain, nb);
+}
+EXPORT_SYMBOL_GPL(register_switchdev_blocking_notifier);
+
+int unregister_switchdev_blocking_notifier(struct notifier_block *nb)
+{
+ struct blocking_notifier_head *chain = &switchdev_blocking_notif_chain;
+
+ return blocking_notifier_chain_unregister(chain, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_switchdev_blocking_notifier);
+
+int call_switchdev_blocking_notifiers(unsigned long val, struct net_device
*dev,
+ struct switchdev_notifier_info *info)
+{
+ info->dev = dev;
+ return blocking_notifier_call_chain(&switchdev_blocking_notif_chain,
+ val, info);
+}
+EXPORT_SYMBOL_GPL(call_switchdev_blocking_notifiers);
+
bool switchdev_port_same_parent_id(struct net_device *a,
struct net_device *b)
{
--
2.4.11
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next 03/12] switchdev: Add SWITCHDEV_PORT_OBJ_ADD, SWITCHDEV_PORT_OBJ_DEL
An offloading driver may need to have access to switchdev events on
ports that aren't directly under its control. An example is a VXLAN port
attached to a bridge offloaded by a driver. The driver needs to know
about VLANs configured on the VXLAN device. However the VXLAN device
isn't stashed between the bridge and a front-panel-port device (such as
is the case e.g. for LAG devices), so the usual switchdev ops don't
reach the driver.
VXLAN is likely not the only device type like this: in theory any L2
tunnel device that needs offloading will prompt requirement of this
sort. This falsifies the assumption that only the lower devices of a
front panel port need to be notified to achieve flawless offloading.
A way to fix this is to give up the notion of port object addition /
deletion as a switchdev operation, which assumes somewhat tight coupling
between the message producer and consumer. And instead send the message
over a notifier chain.
To that end, introduce two new switchdev notifier types,
SWITCHDEV_PORT_OBJ_ADD and SWITCHDEV_PORT_OBJ_DEL. These notifier types
communicate the same event as the corresponding switchdev op, except in
a form of a notification. struct switchdev_notifier_port_obj_info was
added to carry the fields that the switchdev op carries. An additional
field, handled, will be used to communicate back to switchdev that the
event has reached an interested party, which will be important for the
two-phase commit.
The two switchdev operations themselves are kept in place. Following
patches first convert individual clients to the notifier protocol, and
only then are the operations removed.
Signed-off-by: Petr Machata
Acked-by: Jiri Pirko
Reviewed-by: Ido Schimmel
---
include/net/switchdev.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index e021b67b9b32..a2f3ebf39301 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -146,6 +146,9 @@ enum switchdev_notifier_type {
SWITCHDEV_FDB_DEL_TO_DEVICE,
SWITCHDEV_FDB_OFFLOADED,
+ SWITCHDEV_PORT_OBJ_ADD, /* Blocking. */
+ SWITCHDEV_PORT_OBJ_DEL, /* Blocking. */
+
SWITCHDEV_VXLAN_FDB_ADD_TO_BRIDGE,
SWITCHDEV_VXLAN_FDB_DEL_TO_BRIDGE,
SWITCHDEV_VXLAN_FDB_ADD_TO_DEVICE,
@@ -165,6 +168,13 @@ struct switchdev_notifier_fdb_info {
offloaded:1;
};
+struct switchdev_notifier_port_obj_info {
+ struct switchdev_notifier_info info; /* must be first */
+ const struct switchdev_obj *obj;
+ struct switchdev_trans *trans;
+ bool handled;
+};
+
static inline struct net_device *
switchdev_notifier_info_to_dev(const struct switchdev_notifier_info *info)
{
--
2.4.11
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next 05/12] net: dsa: slave: Handle SWITCHDEV_PORT_OBJ_ADD/_DEL
Following patches will change the way of distributing port object
changes from a switchdev operation to a switchdev notifier. The
switchdev code currently recursively descends through layers of lower
devices, eventually calling the op on a front-panel port device. The
notifier will instead be sent referencing the bridge port device, which
may be a stacking device that's one of front-panel ports uppers, or a
completely unrelated device.
DSA currently doesn't support any other uppers than bridge.
SWITCHDEV_OBJ_ID_HOST_MDB and _PORT_MDB objects are always notified on
the bridge port device. Thus the only case that a stacked device could
be validly referenced by port object notifications are bridge
notifications for VLAN objects added to the bridge itself. But the
driver explicitly rejects such notifications in dsa_port_vlan_add(). It
is therefore safe to assume that the only interesting case is that the
notification is on a front-panel port netdevice. Therefore keep the
filtering by dsa_slave_dev_check() in place.
To handle SWITCHDEV_PORT_OBJ_ADD and _DEL, subscribe to the blocking
notifier chain. Dispatch to rocker_port_obj_add() resp. _del() to
maintain the behavior that the switchdev operation based code currently
has.
Signed-off-by: Petr Machata
Acked-by: Jiri Pirko
---
net/dsa/slave.c | 56
1 file changed, 56 insertions(+)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 7d0c19e7edcf..d00a0b6d4ce0 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1557,6 +1557,44 @@ static int dsa_slave_switchdev_event(struct
notifier_block *unused,
return NOTIFY_BAD;
}
+static int
+dsa_slave_switchdev_port_obj_event(unsigned long event,
+ struct net_device *netdev,
+ struct switchdev_notifier_port_obj_info *port_obj_info)
+{
+ int err = -EOPNOTSUPP;
+
+ switch (event) {
+ case SWITCHDEV_PORT_OBJ_ADD:
+ err = dsa_slave_port_obj_add(netdev, port_obj_info->obj,
+port_obj_info->trans);
+ break;
+ case SWITCHDEV_PORT_OBJ_DEL:
+ err = dsa_slave_port_obj_del(netdev, port_obj_info->obj);
+ break;
+ }
+
+ port_obj_info->handled = true;
+ return notifier_from_errno(err);
+}
+
+static int dsa_slave_switchdev_blocking_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+
+ if (!dsa_slave_dev_check(dev))
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case SWITCHDEV_PORT_OBJ_ADD: /* fall through */
+ case SWITCHDEV_PORT_OBJ_DEL:
+ return dsa_slave_switchdev_port_obj_event(event, dev, ptr);
+ }
+
+ return NOTIFY_DONE;
+}
+
static struct notifier_block dsa_slave_nb __read_mostly = {
.notifier_call = dsa_slave_netdevice_event,
};
@@ -1565,8 +1603,13 @@ static struct notifier_block
dsa_slave_switchdev_notifier = {
.notifier_call = dsa_slave_switchdev_event,
};
+static struct notifier_block dsa_slave_switchdev_blocking_notifier = {
+ .notifier_call = dsa_slave_switchdev_blocking_event,
+};
+
int dsa_slave_register_notifier(void)
{
+ struct notifier_block *nb;
int err;
err = register_netdevice_notifier(&dsa_slave_nb);
@@ -1577,8 +1620,15 @@ int dsa_slave_register_notifier(void)
if (err)
goto err_switchdev_nb;
+ nb = &dsa_slave_switchdev_blocking_notifier;
+ err = register_switchdev_blocking_notifier(nb);
+ if (err)
+ goto err_switchdev_blocking_nb;
+
return 0;
+err_switchdev_blocking_nb:
+ unregister_switchdev_notifier(&dsa_slave_switchdev_notifier);
err_switchdev_nb:
unregister_netdevice_notifier(&dsa_slave_nb);
return err;
@@ -1586,8 +1636,14 @@ int dsa_slave_register_notifier(void)
void dsa_slave_unregister_notifier(void)
{
+ struct notifier_block *nb;
int err;
+ nb = &dsa_slave_switchdev_blocking_notifier;
+ err = unregister_switchdev_blocking_notifier(nb);
+ if (err)
+ pr_err("DSA: failed to unregister switchdev blocking notifier
(%d)\n", err);
+
err = unregister_switchdev_notifier(&dsa_slave_switchdev_notifier);
if (err)
pr_err("DSA: failed to unregister switchdev notifier (%d)\n",
err);
--
2.4.11
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next 08/12] switchdev: Add helpers to aid traversal through lower devices
After the transition from switchdev operations to notifier chain (which
will take place in following patches), the onus is on the driver to find
its own devices below possible layer of LAG or other uppers.
The logic to do so is fairly repetitive: each driver is looking for its
own devices among the lowers of the notified device. For those that it
finds, it calls a handler. To indicate that the event was handled,
struct switchdev_notifier_port_obj_info.handled is set. The differences
lie only in what constitutes an "own" device and what handler to call.
Therefore abstract this logic into two helpers,
switchdev_handle_port_obj_add() and switchdev_handle_port_obj_del(). If
a driver only supports physical ports under a bridge device, it will
simply avoid this layer of indirection.
One area where this helper diverges from the current switchdev behavior
is the case of mixed lowers, some of which are switchdev ports and some
of which are not. Previously, such scenario would fail with -EOPNOTSUPP.
The helper could do that for lowers for which the passed-in predicate
doesn't hold. That would however break the case that switchdev ports
from several different drivers are stashed under one master, a scenario
that switchdev currently happily supports. Therefore tolerate any and
all unknown netdevices, whether they are backed by a switchdev driver
or not.
Signed-off-by: Petr Machata
Reviewed-by: Ido Schimmel
---
include/net/switchdev.h | 33 +++
net/switchdev/switchdev.c | 100 ++
2 files changed, 133 insertions(+)
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index a2f3ebf39301..6dc7de576167 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -210,6 +210,18 @@ void switchdev_port_fwd_mark_set(struct net_device *dev,
bool switchdev_port_same_parent_id(struct net_device *a,
struct net_device *b);
+int switchdev_handle_port_obj_add(struct net_device *dev,
+ struct switchdev_notifier_port_obj_info *port_obj_info,
+ bool (*check_cb)(const struct net_device *dev),
+ int (*add_cb)(struct net_device *dev,
+ const struct switchdev_obj *obj,
+ struct switchdev_trans *trans));
+int switchdev_handle_port_obj_del(struct net_device *dev,
+ struct switchdev_notifier_port_obj_info *port_obj_info,
+ bool (*check_cb)(const struct net_device *dev),
+ int (*del_cb)(struct net_device *dev,
+ const struct switchdev_obj *obj));
+
#define SWITCHDEV_SET_OPS(netdev, ops) ((netdev)->switchdev_ops = (ops))
#else
@@ -284,6 +296,27 @@ static inline bool switchdev_port_same_parent_id(struct
net_device *a,
return false;
}
+static inline int
+switchdev_handle_port_obj_add(struct net_device *dev,
+ struct switchdev_notifier_port_obj_info *port_obj_info,
+ bool (*check_cb)(const struct net_device *dev),
+ int (*add_cb)(struct net_device *dev,
+ const struct switchdev_obj *obj,
+ struct switchdev_trans *trans))
+{
+ return 0;
+}
+
+static inline int
+switchdev_handle_port_obj_del(struct net_device *dev,
+ struct switchdev_notifier_port_obj_info *port_obj_info,
+ bool (*check_cb)(const struct net_device *dev),
+ int (*del_cb)(struct net_device *dev,
+ const struct switchdev_obj *obj))
+{
+ return 0;
+}
+
#define SWITCHDEV_SET_OPS(netdev, ops) do {} while (0)
#endif
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index e109bb97ce3f..099434ec7996 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -621,3 +621,103 @@ bool switchdev_port_same_parent_id(struct net_device *a,
return netdev_phys_item_id_same(&a_attr.u.ppid, &b_attr.u.ppid);
}
EXPORT_SYMBOL_GPL(switchdev_port_same_parent_id);
+
+static int __switchdev_handle_port_obj_add(struct net_device *dev,
+ struct switchdev_notifier_port_obj_info *port_obj_info,
+ bool (*check_cb)(const struct net_device *dev),
+ int (*add_cb)(struct net_device *dev,
+ const struct switchdev_obj *obj,
+ struct switchdev_trans *trans))
+{
+ struct net_device *lower_dev;
+ struct list_head *iter;
+ int err = -EOPNOTSUPP;
+
+ if (check_cb(dev)) {
+ /* This flag is only checked if the return value is success. */
+ port_obj_info->handled = true;
+ return add_cb(dev, port_obj_info->obj, port_obj_info->trans);
+ }
+
+ /* Switch p
[PATCH net-next 06/12] staging: fsl-dpaa2: ethsw: Introduce ethsw_port_dev_check()
ethsw currently uses an open-coded comparison of netdev_ops to determine
whether whether a device represents a front panel port. Wrap this into a
named function to simplify reuse.
Signed-off-by: Petr Machata
Acked-by: Jiri Pirko
---
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index 7a7ca67822c5..e379b0fa936f 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -972,6 +972,11 @@ static int port_bridge_leave(struct net_device *netdev)
return err;
}
+static bool ethsw_port_dev_check(const struct net_device *netdev)
+{
+ return netdev->netdev_ops == ðsw_port_ops;
+}
+
static int port_netdevice_event(struct notifier_block *unused,
unsigned long event, void *ptr)
{
@@ -980,7 +985,7 @@ static int port_netdevice_event(struct notifier_block
*unused,
struct net_device *upper_dev;
int err = 0;
- if (netdev->netdev_ops != ðsw_port_ops)
+ if (!ethsw_port_dev_check(netdev))
return NOTIFY_DONE;
/* Handle just upper dev link/unlink for the moment */
--
2.4.11
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next 10/12] ocelot: Handle SWITCHDEV_PORT_OBJ_ADD/_DEL
Following patches will change the way of distributing port object
changes from a switchdev operation to a switchdev notifier. The
switchdev code currently recursively descends through layers of lower
devices, eventually calling the op on a front-panel port device. The
notifier will instead be sent referencing the bridge port device, which
may be a stacking device that's one of front-panel ports uppers, or a
completely unrelated device.
Dispatch the new events to ocelot_port_obj_add() resp. _del() to
maintain the same behavior that the switchdev operation based code
currently has. Pass through switchdev_handle_port_obj_add() / _del() to
handle the recursive descend, because Ocelot supports LAG uppers.
Register to the new switchdev blocking notifier chain to get the new
events when they start getting distributed.
Signed-off-by: Petr Machata
Acked-by: Jiri Pirko
---
drivers/net/ethernet/mscc/ocelot.c | 28
drivers/net/ethernet/mscc/ocelot.h | 1 +
drivers/net/ethernet/mscc/ocelot_board.c | 3 +++
3 files changed, 32 insertions(+)
diff --git a/drivers/net/ethernet/mscc/ocelot.c
b/drivers/net/ethernet/mscc/ocelot.c
index 3238b9ee42f3..01403b530522 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1595,6 +1595,34 @@ struct notifier_block ocelot_netdevice_nb __read_mostly
= {
};
EXPORT_SYMBOL(ocelot_netdevice_nb);
+static int ocelot_switchdev_blocking_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+ int err;
+
+ switch (event) {
+ /* Blocking events. */
+ case SWITCHDEV_PORT_OBJ_ADD:
+ err = switchdev_handle_port_obj_add(dev, ptr,
+ ocelot_netdevice_dev_check,
+ ocelot_port_obj_add);
+ return notifier_from_errno(err);
+ case SWITCHDEV_PORT_OBJ_DEL:
+ err = switchdev_handle_port_obj_del(dev, ptr,
+ ocelot_netdevice_dev_check,
+ ocelot_port_obj_del);
+ return notifier_from_errno(err);
+ }
+
+ return NOTIFY_DONE;
+}
+
+struct notifier_block ocelot_switchdev_blocking_nb __read_mostly = {
+ .notifier_call = ocelot_switchdev_blocking_event,
+};
+EXPORT_SYMBOL(ocelot_switchdev_blocking_nb);
+
int ocelot_probe_port(struct ocelot *ocelot, u8 port,
void __iomem *regs,
struct phy_device *phy)
diff --git a/drivers/net/ethernet/mscc/ocelot.h
b/drivers/net/ethernet/mscc/ocelot.h
index 62c7c8eb00d9..086775f7b52f 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -499,5 +499,6 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port,
struct phy_device *phy);
extern struct notifier_block ocelot_netdevice_nb;
+extern struct notifier_block ocelot_switchdev_blocking_nb;
#endif
diff --git a/drivers/net/ethernet/mscc/ocelot_board.c
b/drivers/net/ethernet/mscc/ocelot_board.c
index 4c23d18bbf44..ca3ea2fbfcd0 100644
--- a/drivers/net/ethernet/mscc/ocelot_board.c
+++ b/drivers/net/ethernet/mscc/ocelot_board.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include "ocelot.h"
@@ -328,6 +329,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
}
register_netdevice_notifier(&ocelot_netdevice_nb);
+ register_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
dev_info(&pdev->dev, "Ocelot switch probed\n");
@@ -342,6 +344,7 @@ static int mscc_ocelot_remove(struct platform_device *pdev)
struct ocelot *ocelot = platform_get_drvdata(pdev);
ocelot_deinit(ocelot);
+ unregister_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
unregister_netdevice_notifier(&ocelot_netdevice_nb);
return 0;
--
2.4.11
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next 09/12] mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_OBJ_ADD/_DEL
Following patches will change the way of distributing port object
changes from a switchdev operation to a switchdev notifier. The
switchdev code currently recursively descends through layers of lower
devices, eventually calling the op on a front-panel port device. The
notifier will instead be sent referencing the bridge port device, which
may be a stacking device that's one of front-panel ports uppers, or a
completely unrelated device.
To handle SWITCHDEV_PORT_OBJ_ADD and _DEL, subscribe to the blocking
notifier chain. Dispatch to mlxsw_sp_port_obj_add() resp. _del() to
maintain the behavior that the switchdev operation based code currently
has. Defer to switchdev_handle_port_obj_add() / _del() to handle the
recursive descend, because mlxsw supports a number of upper types.
Signed-off-by: Petr Machata
Reviewed-by: Ido Schimmel
---
.../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 45 +-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index b32a5ee57fb9..3756aaecd39c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -3118,6 +3118,32 @@ static struct notifier_block mlxsw_sp_switchdev_notifier
= {
.notifier_call = mlxsw_sp_switchdev_event,
};
+static int mlxsw_sp_switchdev_blocking_event(struct notifier_block *unused,
+unsigned long event, void *ptr)
+{
+ struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+ int err;
+
+ switch (event) {
+ case SWITCHDEV_PORT_OBJ_ADD:
+ err = switchdev_handle_port_obj_add(dev, ptr,
+ mlxsw_sp_port_dev_check,
+ mlxsw_sp_port_obj_add);
+ return notifier_from_errno(err);
+ case SWITCHDEV_PORT_OBJ_DEL:
+ err = switchdev_handle_port_obj_del(dev, ptr,
+ mlxsw_sp_port_dev_check,
+ mlxsw_sp_port_obj_del);
+ return notifier_from_errno(err);
+ }
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block mlxsw_sp_switchdev_blocking_notifier = {
+ .notifier_call = mlxsw_sp_switchdev_blocking_event,
+};
+
u8
mlxsw_sp_bridge_port_stp_state(struct mlxsw_sp_bridge_port *bridge_port)
{
@@ -3127,6 +3153,7 @@ mlxsw_sp_bridge_port_stp_state(struct
mlxsw_sp_bridge_port *bridge_port)
static int mlxsw_sp_fdb_init(struct mlxsw_sp *mlxsw_sp)
{
struct mlxsw_sp_bridge *bridge = mlxsw_sp->bridge;
+ struct notifier_block *nb;
int err;
err = mlxsw_sp_ageing_set(mlxsw_sp, MLXSW_SP_DEFAULT_AGEING_TIME);
@@ -3141,17 +3168,33 @@ static int mlxsw_sp_fdb_init(struct mlxsw_sp *mlxsw_sp)
return err;
}
+ nb = &mlxsw_sp_switchdev_blocking_notifier;
+ err = register_switchdev_blocking_notifier(nb);
+ if (err) {
+ dev_err(mlxsw_sp->bus_info->dev, "Failed to register switchdev
blocking notifier\n");
+ goto err_register_switchdev_blocking_notifier;
+ }
+
INIT_DELAYED_WORK(&bridge->fdb_notify.dw, mlxsw_sp_fdb_notify_work);
bridge->fdb_notify.interval = MLXSW_SP_DEFAULT_LEARNING_INTERVAL;
mlxsw_sp_fdb_notify_work_schedule(mlxsw_sp);
return 0;
+
+err_register_switchdev_blocking_notifier:
+ unregister_switchdev_notifier(&mlxsw_sp_switchdev_notifier);
+ return err;
}
static void mlxsw_sp_fdb_fini(struct mlxsw_sp *mlxsw_sp)
{
+ struct notifier_block *nb;
+
cancel_delayed_work_sync(&mlxsw_sp->bridge->fdb_notify.dw);
- unregister_switchdev_notifier(&mlxsw_sp_switchdev_notifier);
+ nb = &mlxsw_sp_switchdev_blocking_notifier;
+ unregister_switchdev_blocking_notifier(nb);
+
+ unregister_switchdev_notifier(&mlxsw_sp_switchdev_notifier);
}
int mlxsw_sp_switchdev_init(struct mlxsw_sp *mlxsw_sp)
--
2.4.11
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next 07/12] staging: fsl-dpaa2: ethsw: Handle SWITCHDEV_PORT_OBJ_ADD/_DEL
Following patches will change the way of distributing port object
changes from a switchdev operation to a switchdev notifier. The
switchdev code currently recursively descends through layers of lower
devices, eventually calling the op on a front-panel port device. The
notifier will instead be sent referencing the bridge port device, which
may be a stacking device that's one of front-panel ports uppers, or a
completely unrelated device.
ethsw currently doesn't support any uppers other than bridge.
SWITCHDEV_OBJ_ID_HOST_MDB and _PORT_MDB objects are always notified on
the bridge port device. Thus the only case that a stacked device could
be validly referenced by port object notifications are bridge
notifications for VLAN objects added to the bridge itself. But the
driver explicitly rejects such notifications in port_vlans_add(). It is
therefore safe to assume that the only interesting case is that the
notification is on a front-panel port netdevice.
To handle SWITCHDEV_PORT_OBJ_ADD and _DEL, subscribe to the blocking
notifier chain. Dispatch to swdev_port_obj_add() resp. _del() to
maintain the behavior that the switchdev operation based code currently
has.
Signed-off-by: Petr Machata
Acked-by: Jiri Pirko
---
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 56 +
1 file changed, 56 insertions(+)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index e379b0fa936f..83e1d92dc7f3 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -1088,10 +1088,51 @@ static int port_switchdev_event(struct notifier_block
*unused,
return NOTIFY_BAD;
}
+static int
+ethsw_switchdev_port_obj_event(unsigned long event, struct net_device *netdev,
+ struct switchdev_notifier_port_obj_info *port_obj_info)
+{
+ int err = -EOPNOTSUPP;
+
+ switch (event) {
+ case SWITCHDEV_PORT_OBJ_ADD:
+ err = swdev_port_obj_add(netdev, port_obj_info->obj,
+port_obj_info->trans);
+ break;
+ case SWITCHDEV_PORT_OBJ_DEL:
+ err = swdev_port_obj_del(netdev, port_obj_info->obj);
+ break;
+ }
+
+ port_obj_info->handled = true;
+ return notifier_from_errno(err);
+}
+
+static int port_switchdev_blocking_event(struct notifier_block *unused,
+unsigned long event, void *ptr)
+{
+ struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+
+ if (!ethsw_port_dev_check(dev))
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case SWITCHDEV_PORT_OBJ_ADD: /* fall through */
+ case SWITCHDEV_PORT_OBJ_DEL:
+ return ethsw_switchdev_port_obj_event(event, dev, ptr);
+ }
+
+ return NOTIFY_DONE;
+}
+
static struct notifier_block port_switchdev_nb = {
.notifier_call = port_switchdev_event,
};
+static struct notifier_block port_switchdev_blocking_nb = {
+ .notifier_call = port_switchdev_blocking_event,
+};
+
static int ethsw_register_notifier(struct device *dev)
{
int err;
@@ -1108,8 +1149,16 @@ static int ethsw_register_notifier(struct device *dev)
goto err_switchdev_nb;
}
+ err = register_switchdev_blocking_notifier(&port_switchdev_blocking_nb);
+ if (err) {
+ dev_err(dev, "Failed to register switchdev blocking
notifier\n");
+ goto err_switchdev_blocking_nb;
+ }
+
return 0;
+err_switchdev_blocking_nb:
+ unregister_switchdev_notifier(&port_switchdev_nb);
err_switchdev_nb:
unregister_netdevice_notifier(&port_nb);
return err;
@@ -1296,8 +1345,15 @@ static int ethsw_port_init(struct ethsw_port_priv
*port_priv, u16 port)
static void ethsw_unregister_notifier(struct device *dev)
{
+ struct notifier_block *nb;
int err;
+ nb = &port_switchdev_blocking_nb;
+ err = unregister_switchdev_blocking_notifier(nb);
+ if (err)
+ dev_err(dev,
+ "Failed to unregister switchdev blocking notifier
(%d)\n", err);
+
err = unregister_switchdev_notifier(&port_switchdev_nb);
if (err)
dev_err(dev,
--
2.4.11
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next 11/12] switchdev: Replace port obj add/del SDO with a notification
Drop switchdev_ops.switchdev_port_obj_add and _del. Drop the uses of
this field from all clients, which were migrated to use switchdev
notification in the previous patches.
Add a new function switchdev_port_obj_notify() that sends the switchdev
notifications SWITCHDEV_PORT_OBJ_ADD and _DEL.
Update switchdev_port_obj_del_now() to dispatch to this new function.
Drop __switchdev_port_obj_add() and update switchdev_port_obj_add()
likewise.
Signed-off-by: Petr Machata
Reviewed-by: Ido Schimmel
---
.../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 2 -
drivers/net/ethernet/mscc/ocelot.c | 2 -
drivers/net/ethernet/rocker/rocker_main.c | 2 -
drivers/staging/fsl-dpaa2/ethsw/ethsw.c| 2 -
include/net/switchdev.h| 9 ---
net/dsa/slave.c| 2 -
net/switchdev/switchdev.c | 67 --
7 files changed, 25 insertions(+), 61 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 3756aaecd39c..73e5db176d7e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1968,8 +1968,6 @@ static struct mlxsw_sp_port *mlxsw_sp_lag_rep_port(struct
mlxsw_sp *mlxsw_sp,
static const struct switchdev_ops mlxsw_sp_port_switchdev_ops = {
.switchdev_port_attr_get= mlxsw_sp_port_attr_get,
.switchdev_port_attr_set= mlxsw_sp_port_attr_set,
- .switchdev_port_obj_add = mlxsw_sp_port_obj_add,
- .switchdev_port_obj_del = mlxsw_sp_port_obj_del,
};
static int
diff --git a/drivers/net/ethernet/mscc/ocelot.c
b/drivers/net/ethernet/mscc/ocelot.c
index 01403b530522..7f8da8873a96 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1337,8 +1337,6 @@ static int ocelot_port_obj_del(struct net_device *dev,
static const struct switchdev_ops ocelot_port_switchdev_ops = {
.switchdev_port_attr_get= ocelot_port_attr_get,
.switchdev_port_attr_set= ocelot_port_attr_set,
- .switchdev_port_obj_add = ocelot_port_obj_add,
- .switchdev_port_obj_del = ocelot_port_obj_del,
};
static int ocelot_port_bridge_join(struct ocelot_port *ocelot_port,
diff --git a/drivers/net/ethernet/rocker/rocker_main.c
b/drivers/net/ethernet/rocker/rocker_main.c
index 806ffe1d906e..f05d5c1341b6 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2145,8 +2145,6 @@ static int rocker_port_obj_del(struct net_device *dev,
static const struct switchdev_ops rocker_port_switchdev_ops = {
.switchdev_port_attr_get= rocker_port_attr_get,
.switchdev_port_attr_set= rocker_port_attr_set,
- .switchdev_port_obj_add = rocker_port_obj_add,
- .switchdev_port_obj_del = rocker_port_obj_del,
};
struct rocker_fib_event_work {
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index 83e1d92dc7f3..06a233c7cdd3 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -930,8 +930,6 @@ static int swdev_port_obj_del(struct net_device *netdev,
static const struct switchdev_ops ethsw_port_switchdev_ops = {
.switchdev_port_attr_get= swdev_port_attr_get,
.switchdev_port_attr_set= swdev_port_attr_set,
- .switchdev_port_obj_add = swdev_port_obj_add,
- .switchdev_port_obj_del = swdev_port_obj_del,
};
/* For the moment, only flood setting needs to be updated */
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 6dc7de576167..866b6d148b77 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -121,10 +121,6 @@ typedef int switchdev_obj_dump_cb_t(struct switchdev_obj
*obj);
* @switchdev_port_attr_get: Get a port attribute (see switchdev_attr).
*
* @switchdev_port_attr_set: Set a port attribute (see switchdev_attr).
- *
- * @switchdev_port_obj_add: Add an object to port (see switchdev_obj_*).
- *
- * @switchdev_port_obj_del: Delete an object from port (see switchdev_obj_*).
*/
struct switchdev_ops {
int (*switchdev_port_attr_get)(struct net_device *dev,
@@ -132,11 +128,6 @@ struct switchdev_ops {
int (*switchdev_port_attr_set)(struct net_device *dev,
const struct switchdev_attr *attr,
struct switchdev_trans *trans);
- int (*switchdev_port_obj_add)(struct net_device *dev,
- const struct switchdev_obj *obj,
- struct switchdev_trans *trans);
- int (*switchdev_port_obj_del)(struct net_device *dev,
-
Re: [PATCH net-next 00/12] switchdev: Convert switchdev_port_obj_{add,del}() to notifiers
Petr Machata writes: > An offloading driver may need to have access to switchdev events on > ports that aren't directly under its control. An example is a VXLAN port > attached to a bridge offloaded by a driver. The driver needs to know > about VLANs configured on the VXLAN device. However the VXLAN device > isn't stashed between the bridge and a front-panel-port device (such as > is the case e.g. for LAG devices), so the usual switchdev ops don't > reach the driver. mlxsw will use these notifications to offload VXLAN devices attached to a VLAN-aware bridge. The patches are available here should anyone wish to take a look: https://github.com/idosch/linux/commits/vxlan Thanks, Petr ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/10] staging: erofs: add a full barrier in erofs_workgroup_unfreeze
Hi Andrea,
On 2018/11/23 2:50, Andrea Parri wrote:
> On Thu, Nov 22, 2018 at 06:56:32PM +0800, Gao Xiang wrote:
>> Hi Greg,
>>
>> On 2018/11/22 18:22, Greg Kroah-Hartman wrote:
>>> Please document this memory barrier. It does not make much sense to
>>> me...
>>
>> Because we need to make the other observers noticing the latest values
>> modified
>> in this locking period before unfreezing the whole workgroup, one way is to
>> use
>> a memory barrier and the other way is to use ACQUIRE and RELEASE. we selected
>> the first one.
>>
>> Hmmm...ok, I will add a simple message to explain this, but I think that is
>> plain enough for a lock...
>
> Sympathizing with Greg's request, let me add some specific suggestions:
>
> 1. It wouldn't hurt to indicate a pair of memory accesses which are
> intended to be "ordered" by the memory barrier in question (yes,
> this pair might not be unique, but you should be able to provide
> an example).
>
> 2. Memory barriers always come matched by other memory barriers, or
> dependencies (it really does not make sense to talk about a full
> barrier "in isolation"): please also indicate (an instance of) a
> matching barrier or the matching barriers.
>
> 3. How do the hardware threads communicate? In the acquire/release
> pattern you mentioned above, the load-acquire *reads from* a/the
> previous store-release, a memory access that follows the acquire
> somehow communicate with a memory access preceding the release...
>
> 4. It is a good practice to include the above information within an
> (inline) comment accompanying the added memory barrier (in fact,
> IIRC, checkpatch.pl gives you a "memory barrier without comment"
> warning when you omit to do so); not just in the commit message.
>
> Hope this helps. Please let me know if something I wrote is unclear,
Thanks for taking time on the detailed explanation. I think it is helpful for
me. :)
And you are right, barriers should be in pairs, and I think I need to explain
more:
255 static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int
*ocnt)
256 {
257 int o;
258
259 repeat:
260 o = erofs_wait_on_workgroup_freezed(grp);
261
262 if (unlikely(o <= 0))
263 return -1;
264
265 if (unlikely(atomic_cmpxchg(&grp->refcount, o, o + 1) != o)) <- *
266 goto repeat;
imply a memory barrier here
267
268 *ocnt = o;
269 return 0;
270 }
I think atomic_cmpxchg implies a memory barrier semantics when the value
comparison (*) succeeds...
I don't know whether my understanding is correct, If I am wrong..please correct
me, or
I need to add more detailed code comments to explain in the code?
Thanks,
Gao Xiang
>
> Andrea
>
>
>>
>> Thanks,
>> Gao Xiang
>>
>>>
>>> thanks,
>>>
>>> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
