* Re: [Qemu-devel] [PATCH 0/6] accel/tcg: Support execution from MMIO and small MMU regions
From: Cédric Le Goater @ 2018-07-23 15:11 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel
Cc: patches, Richard Henderson, Emilio G . Cota, Paolo Bonzini,
Edgar E. Iglesias, KONRAD Frederic
In-Reply-To: <20180710160013.26559-1-peter.maydell@linaro.org>
On 07/10/2018 06:00 PM, Peter Maydell wrote:
> This series adds support to TCG for executing from MMIO regions
> and small MMU regions. The basic principle is that if get_page_addr_code()
> finds that the region is not backed by a full page of RAM then it
> returns -1, and tb_gen_code() then generates a non-cached TB
> containing a single instruction. Execution from these regions
> thus performs the instruction fetch every time, ensuring that we
> get the read-from-MMIO and check-small-MMU-region permissions
> checks right.
>
> This means that the code path for "generate bus fault for failing
> to load an instruction" no longer goes through get_page_addr_code(),
> but instead via each target's translate code and its calls to
> the cpu_ld*_code() or similar functions. Patch 1 makes sure we
> can distinguish insn fetches from data loads when generating the
> bus fault exceptions. (Aside: I have assumed that all cpu_ld*_code()
> loads should trigger iside faults rather than dside. Hopefully this
> is true...)
>
> Patches 2 and 3 make trivial fixes to various callers of
> get_page_addr_code(); patch 4 does the work of generating our
> single-insn TBs. Patch 5 can then remove all the code that
> (mis)handles MMIO regions from get_page_addr_code(). Finally
> patch 6 drops the target/arm workarounds for not having support
> for executing from small MPU regions.
>
> Note for the Xilinx folks: this patchset makes the mmio-exec
> testcase for running from the SPI flash pass. Cedric: you might
> like to test the aspeed image you had that relies on execution
> from an MMIO region too.
For the series,
Tested-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
>
> The diffstat is pretty satisfying for a patchset that adds
> a feature, but it actually undersells it: this code renders the
> hw/misc/mmio_interface.c and the mmio_ptr related code in memory.c
> and the xilinx-spips device all obsolete, so there are another
> couple of hundred lines of code to be deleted there. I opted not
> to include that in this patchset, for ease of review.
>
> NB: I tested this with icount, but there are potentially
> some weird things that could happen with interactions between
> icount's io-recompile and execution from an MMIO device
> that returns different instructions each time it's read.
>
> thanks
> -- PMM
>
>
> Peter Maydell (6):
> accel/tcg: Pass read access type through to io_readx()
> accel/tcg: Handle get_page_addr_code() returning -1 in hashtable
> lookups
> accel/tcg: Handle get_page_addr_code() returning -1 in
> tb_check_watchpoint()
> accel/tcg: tb_gen_code(): Create single-insn TB for execution from
> non-RAM
> accel/tcg: Return -1 for execution from MMIO regions in
> get_page_addr_code()
> target/arm: Allow execution from small regions
>
> accel/tcg/softmmu_template.h | 11 ++--
> include/qom/cpu.h | 6 +++
> accel/tcg/cpu-exec.c | 3 ++
> accel/tcg/cputlb.c | 100 +++++------------------------------
> accel/tcg/translate-all.c | 23 +++++++-
> memory.c | 3 +-
> target/arm/helper.c | 23 --------
> 7 files changed, 52 insertions(+), 117 deletions(-)
>
^ permalink raw reply
* Re: [RFC PATCH ghak90 (was ghak32) V3 02/10] audit: log container info of syscalls
From: Richard Guy Briggs @ 2018-07-23 15:11 UTC (permalink / raw)
To: Steve Grubb
Cc: Paul Moore, cgroups, containers, linux-api, linux-audit,
linux-fsdevel, linux-kernel, netdev, ebiederm, luto, carlos,
dhowells, viro, simo, Eric Paris, serge
In-Reply-To: <3318670.y8SpQ3RC2W@x2>
On 2018-07-23 09:19, Steve Grubb wrote:
> On Sunday, July 22, 2018 4:55:10 PM EDT Richard Guy Briggs wrote:
> > On 2018-07-22 09:32, Steve Grubb wrote:
> > > On Saturday, July 21, 2018 4:29:30 PM EDT Richard Guy Briggs wrote:
> > > > > > + * audit_log_contid - report container info
> > > > > > + * @tsk: task to be recorded
> > > > > > + * @context: task or local context for record
> > > > > > + * @op: contid string description
> > > > > > + */
> > > > > > +int audit_log_contid(struct task_struct *tsk,
> > > > > > + struct audit_context *context, char
> > > > > > *op)
> > > > > > +{
> > > > > > + struct audit_buffer *ab;
> > > > > > +
> > > > > > + if (!audit_contid_set(tsk))
> > > > > > + return 0;
> > > > > > + /* Generate AUDIT_CONTAINER record with container ID */
> > > > > > + ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER);
> > > > > > + if (!ab)
> > > > > > + return -ENOMEM;
> > > > > > + audit_log_format(ab, "op=%s contid=%llu",
> > > > > > + op, audit_get_contid(tsk));
> > > > >
> > > > > Can you explain your reason for including an "op" field in this
> > > > > record
> > > > > type? I've been looking at the rest of the patches in this patchset
> > > > > and it seems to be used more as an indicator of the record's
> > > > > generating context rather than any sort of audit container ID
> > > > > operation.
> > > >
> > > > "action" might work, but that's netfilter and numeric... "kind"?
> > > > Nothing else really seems to fit from a field name, type or lack of
> > > > searchability perspective.
> > > >
> > > > Steve, do you have an opinion?
> > >
> > > We only have 1 sample event where we have op=task. What are the other
> > > possible values?
> >
> > For the AUDIT_CONTAINER record we have op= "task", "target" (from the
> > ptrace and signals patch), "tty".
> >
> > For the AUDIT_CONTAINER_ID record we have "op=set".
>
> Since the purpose of this record is to log the container id, I think that is
> all that is needed. We can get the context from the other records in the
> event. I'd suggest dropping the "op" field.
Ok, the information above it for two different audit container
identifier records. Which one should drop the "op=" field? Both? Or
just the AUDIT_CONTAINER record? The AUDIT_CONTAINER_ID record (which
might be renamed) could use it to distinguish a "set" record from a
dropped audit container identifier that is no longer registered by any
task or namespace.
> -Steve
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH 4.4 000/107] 4.4.144-stable review
From: Nathan Chancellor @ 2018-07-23 15:11 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, torvalds, akpm, linux, shuah, patches,
ben.hutchings, lkft-triage, stable
In-Reply-To: <20180723122413.003644357@linuxfoundation.org>
On Mon, Jul 23, 2018 at 02:40:54PM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 4.4.144 release.
> There are 107 patches in this series, all will be posted as a response
> to this one. If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Wed Jul 25 12:23:53 UTC 2018.
> Anything received after that time might be too late.
>
> The whole patch series can be found in one patch at:
> https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.4.144-rc1.gz
> or in the git tree and branch at:
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-4.4.y
> and the diffstat can be found below.
>
> thanks,
>
> greg k-h
>
Merged, compiled with -Werror, and installed onto my Pixel 2 XL.
No initial issues noticed in dmesg or general usage.
Thanks!
Nathan
^ permalink raw reply
* Re: powerpc/ps3: Set driver coherent_dma_mask
From: Michael Ellerman @ 2018-07-23 15:11 UTC (permalink / raw)
To: Geoff Levand, Alan Stern, Takashi Iwai, Jaroslav Kysela
Cc: linux-usb, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <068ebcfa-7cd0-bd06-42e7-577a4624f0b0@infradead.org>
On Wed, 2018-07-18 at 22:08:33 UTC, Geoff Levand wrote:
> Set the coherent_dma_mask for the PS3 ehci, ohci, and snd devices.
>
> Silences WARN_ON_ONCE messages emitted by the dma_alloc_attrs() routine.
>
> Reported-by: Fredrik Noring <noring@nocrew.org>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/48e91846865cd61bfdb701eb737de2
cheers
^ permalink raw reply
* powerpc/ps3: Set driver coherent_dma_mask
From: Michael Ellerman @ 2018-07-23 15:11 UTC (permalink / raw)
To: Geoff Levand, Alan Stern, Takashi Iwai, Jaroslav Kysela
Cc: linux-usb, linuxppc-dev@lists.ozlabs.org
On Wed, 2018-07-18 at 22:08:33 UTC, Geoff Levand wrote:
> Set the coherent_dma_mask for the PS3 ehci, ohci, and snd devices.
>
> Silences WARN_ON_ONCE messages emitted by the dma_alloc_attrs() routine.
>
> Reported-by: Fredrik Noring <noring@nocrew.org>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/48e91846865cd61bfdb701eb737de2
cheers
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [v2] powerpc/prom_init: remove linux,stdout-package property
From: Michael Ellerman @ 2018-07-23 15:11 UTC (permalink / raw)
To: Murilo Opsfelder Araujo, linux-kernel
Cc: Kees Cook, Mathieu Malaterre, Aneesh Kumar K . V,
Murilo Opsfelder Araujo, Nicholas Piggin, Alexey Kardashevskiy,
Michael Bringmann, Paul Mackerras, Cédric Le Goater,
Bharata B Rao, Nathan Fontenot, linuxppc-dev
In-Reply-To: <20180718161544.12134-1-muriloo@linux.ibm.com>
On Wed, 2018-07-18 at 16:15:44 UTC, Murilo Opsfelder Araujo wrote:
> This property was added in 2004 and the only use of it, which was already inside
> `#if 0`, was removed a month later.
>
> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/ec9336396a98f61290f45e8bb94245
cheers
^ permalink raw reply
* Re: powerpc/hugetlbpage: Rmove unhelpful HUGEPD_*_SHIFT macros
From: Michael Ellerman @ 2018-07-23 15:11 UTC (permalink / raw)
To: David Gibson, aneesh.kumar, paulus
Cc: linuxppc-dev, linux-kernel, David Gibson
In-Reply-To: <20180717042430.15395-1-david@gibson.dropbear.id.au>
On Tue, 2018-07-17 at 04:24:30 UTC, David Gibson wrote:
> The HUGEPD_*_SHIFT macros are always defined to be PGDIR_SHIFT and
> PUD_SHIFT, and have to have those values to work properly. They once used
> to have different values, but that was really only because they were used
> to mean different things in different contexts.
>
> 6fa50483 "powerpc/mm/hugetlb: initialize the pagetable cache correctly for
> hugetlb" removed that double meaning, but left the now useless constants.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/fdf743c5c51d5b45db4dada3910954
cheers
^ permalink raw reply
* Re: chrp/nvram.c: add MODULE_LICENSE()
From: Michael Ellerman @ 2018-07-23 15:11 UTC (permalink / raw)
To: Randy Dunlap, linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <22eb2f31-3b79-3702-071c-d0fe06a50150@infradead.org>
On Sat, 2018-07-14 at 04:27:48 UTC, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Add MODULE_LICENSE() to the chrp nvram.c driver to fix the build
> warning message:
>
> WARNING: modpost: missing MODULE_LICENSE() in arch/powerpc/platforms/chrp/nvram.o
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/a8bf9e504a6a2bb058b4f020d4ccc5
cheers
^ permalink raw reply
* Re: [1/3] selftests/powerpc: Remove Power9 copy_unaligned test
From: Michael Ellerman @ 2018-07-23 15:11 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: mikey, chris
In-Reply-To: <20180711071017.5128-1-mpe@ellerman.id.au>
On Wed, 2018-07-11 at 07:10:15 UTC, Michael Ellerman wrote:
> This is a test of the ISA 3.0 "copy" instruction. That instruction has
> an L field, which if set to 1 specifies that "the instruction
> identifies the beginning of a move group" (pp 858). That's also
> referred to as "copy first" vs "copy".
>
> In ISA 3.0B the copy instruction does not have an L field, and the
> corresponding bit in the instruction must be set to 1.
>
> This test is generating a "copy" instruction, not a "copy first", and
> so on Power9 (which implements 3.0B), this results in an illegal
> instruction.
>
> So just drop the test entirely. We still have copy_first_unaligned to
> test the "copy first" behaviour.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Acked-by: Michael Neuling <mikey@neuling.org>
Series applied to powerpc next.
https://git.kernel.org/powerpc/c/83039f22ba2f6aff935a2acbb6bf67
cheers
^ permalink raw reply
* Re: powerpc/8xx: fix handling of early NULL pointer dereference
From: Michael Ellerman @ 2018-07-23 15:11 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4302f36bd615c179c92f2007a0f3a1215ba96c55.1531439662.git.christophe.leroy@c-s.fr>
On Fri, 2018-07-13 at 13:10:47 UTC, Christophe Leroy wrote:
> NULL pointers are pointers to user memory space. So user pagetable
> has to be set in order to avoid random behaviour in case of NULL
> pointer dereference, otherwise we may encounter random memory
> access hence Machine Check Exception from TLB Miss handlers.
>
> Set user pagetable as early as possible in order to properly
> catch early kernel NULL pointer dereference.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/8c8c10b90d88bfaad41ea34df17bf6
cheers
^ permalink raw reply
* Re: cxl: Fix wrong comparison in cxl_adapter_context_get()
From: Michael Ellerman @ 2018-07-23 15:11 UTC (permalink / raw)
To: Vaibhav Jain, linuxppc-dev, Frederic Barrat, Andrew Donnellan,
Dan Carpenter
Cc: Philippe Bergheaud, Vaibhav Jain, Alastair D'Silva,
Christophe Lombard, stable
In-Reply-To: <20180704152833.2288-1-vaibhav@linux.ibm.com>
On Wed, 2018-07-04 at 15:28:33 UTC, Vaibhav Jain wrote:
> Function atomic_inc_unless_negative() returns a bool to indicate
> success/failure. However cxl_adapter_context_get() wrongly compares
> the return value against '>=0' which will always be true. The patch
> fixes this comparison to '==0' there by also fixing this compile time
> warning:
>
> drivers/misc/cxl/main.c:290 cxl_adapter_context_get()
> warn: 'atomic_inc_unless_negative(&adapter->contexts_num)' is unsigned
>
> Cc: stable@vger.kernel.org
> Fixes: 70b565bbdb91 ("cxl: Prevent adapter reset if an active context exists")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/ef6cb5f1a048fdf91ccee6d63d2bfa
cheers
^ permalink raw reply
* Re: powerpc/mpic: Cleanup irq vector accounting
From: Michael Ellerman @ 2018-07-23 15:11 UTC (permalink / raw)
To: Bharat Bhushan, benh, paulus, robh, geoff, tyreld, linuxppc-dev,
linux-kernel
Cc: Bharat Bhushan
In-Reply-To: <1530267872-31244-1-git-send-email-Bharat.Bhushan@nxp.com>
On Fri, 2018-06-29 at 10:24:32 UTC, Bharat Bhushan wrote:
> Available vector space accounts ipis and timer interrupts
> while spurious vector was not accounted. Also later
> mpic_setup_error_int() escape one more vector, seemingly it
> assumes one spurious vector.
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/fca7bf946e31688edce446fdaa571c
cheers
^ permalink raw reply
* Re: [2/2] powernv/npu: Add a debugfs setting to change ATSD threshold
From: Michael Ellerman @ 2018-07-23 15:11 UTC (permalink / raw)
To: Alistair Popple, linuxppc-dev; +Cc: Alistair Popple, mhairgrove, arbab
In-Reply-To: <20180417091129.23069-2-alistair@popple.id.au>
On Tue, 2018-04-17 at 09:11:29 UTC, Alistair Popple wrote:
> The threshold at which it becomes more efficient to coalesce a range of
> ATSDs into a single per-PID ATSD is currently not well understood due to a
> lack of real-world work loads. This patch adds a debugfs parameter allowing
> the threshold to be altered at runtime in order to aid future development
> and refinement of the value.
>
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/99c3ce33a00bc40cb218af770ef00c
cheers
^ permalink raw reply
* Re: Mark ams driver as orphaned in MAINTAINERS
From: Michael Ellerman @ 2018-07-23 15:11 UTC (permalink / raw)
To: Michael Hanselmann, linuxppc-dev, linux-kernel; +Cc: Michael Hanselmann
In-Reply-To: <8fe08e4d44c56ee0b70517fc8609ab5a116bc407.1517265227.git.linux-kernel@hansmi.ch>
On Mon, 2018-01-29 at 22:40:09 UTC, Michael Hanselmann wrote:
> I no longer have any hardware with the Apple motion sensor and thus
> relinquish maintainership of the driver.
>
> Signed-off-by: Michael Hanselmann <linux-kernel@hansmi.ch>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/d69ccc00c497a4d81fca2dca9bda9f
cheers
^ permalink raw reply
* [Buildroot] [PATCH 2/2] package/ti-utils: update website infos
From: julien.boibessot at free.fr @ 2018-07-23 15:10 UTC (permalink / raw)
To: buildroot
In-Reply-To: <1532358612-4150-1-git-send-email-julien.boibessot@free.fr>
From: Julien BOIBESSOT <julien.boibessot@armadeus.com>
As kernel wireless wiki doesn't seem to have the doc about wl12xx calibration,
points to a backup of the old site.
Signed-off-by: Julien BOIBESSOT <julien.boibessot@armadeus.com>
---
package/ti-utils/Config.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/package/ti-utils/Config.in b/package/ti-utils/Config.in
index 0b75c4d..9923234 100644
--- a/package/ti-utils/Config.in
+++ b/package/ti-utils/Config.in
@@ -6,7 +6,7 @@ config BR2_PACKAGE_TI_UTILS
The calibrator and other useful utilities for TI wireless
solution, based on wl12xx driver.
- http://linuxwireless.org/en/users/Drivers/wl12xx/calibrator
+ http://linuxwireless.sipsolutions.net/en/users/Drivers/wl12xx/calibrator/
comment "ti-utils needs a toolchain w/ threads"
depends on !BR2_TOOLCHAIN_HAS_THREADS
--
2.1.4
^ permalink raw reply related
* [Buildroot] [PATCH 1/2] package/crda: update website infos
From: julien.boibessot at free.fr @ 2018-07-23 15:10 UTC (permalink / raw)
To: buildroot
From: Julien BOIBESSOT <julien.boibessot@armadeus.com>
Now Linux wireless infos are on corresponding wiki.
Signed-off-by: Julien BOIBESSOT <julien.boibessot@armadeus.com>
---
package/crda/Config.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/package/crda/Config.in b/package/crda/Config.in
index b9a2613..469fa77 100644
--- a/package/crda/Config.in
+++ b/package/crda/Config.in
@@ -15,7 +15,7 @@ config BR2_PACKAGE_CRDA
query and apply the regulatory domain settings wireless
devices may operate within for a given location.
- http://linuxwireless.org/en/developers/Regulatory/CRDA
+ https://wireless.wiki.kernel.org/en/developers/regulatory/crda
comment "crda needs a toolchain w/ threads, dynamic library"
depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v2 00/19] Fixes for sched/numa_balancing
From: Srikar Dronamraju @ 2018-07-23 15:09 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, LKML, Mel Gorman, Rik van Riel, Thomas Gleixner
In-Reply-To: <20180723135700.GX2458@hirez.programming.kicks-ass.net>
* Peter Zijlstra <peterz@infradead.org> [2018-07-23 15:57:00]:
> On Wed, Jun 20, 2018 at 10:32:41PM +0530, Srikar Dronamraju wrote:
> > Srikar Dronamraju (19):
> > sched/numa: Remove redundant field.
> > sched/numa: Evaluate move once per node
> > sched/numa: Simplify load_too_imbalanced
> > sched/numa: Set preferred_node based on best_cpu
> > sched/numa: Use task faults only if numa_group is not yet setup
> > sched/debug: Reverse the order of printing faults
> > sched/numa: Skip nodes that are at hoplimit
> > sched/numa: Remove unused task_capacity from numa_stats
> > sched/numa: Modify migrate_swap to accept additional params
> > sched/numa: Restrict migrating in parallel to the same node.
> > sched/numa: Remove numa_has_capacity
> > sched/numa: Use group_weights to identify if migration degrades locality
> > sched/numa: Move task_placement closer to numa_migrate_preferred
>
> I took the above, but left the below for next time.
>
> > sched/numa: Stop multiple tasks from moving to the cpu at the same time
This patch has go-ahead from Mel and Rik and no outstanding comments.
In my analysis, I did find a lot of cases where the same cpu ended up
being the target. + I am not sure you can apply "sched/numa: Restrict
migrating in parallel to the same node" cleanly without this patch.
So I am a bit confused. If possible, please clarify.
> > mm/migrate: Use xchg instead of spinlock
Will try with spin_trylock and get back.
> > sched/numa: Updation of scan period need not be in lock
I didnt see any comments for this apart from an ack from Rik.
+ It thought it was trivial and shouldnt have any side-effect.
> > sched/numa: Detect if node actively handling migration
> > sched/numa: Pass destination cpu as a parameter to migrate_task_rq
> > sched/numa: Reset scan rate whenever task moves across nodes
>
^ permalink raw reply
* Re: [PATCH 2/2] media: i2c: Add driver for Aptina MT9V111
From: jacopo mondi @ 2018-07-23 14:08 UTC (permalink / raw)
To: Kieran Bingham
Cc: Jacopo Mondi, mchehab, linux-media, linux-kernel,
linux-renesas-soc
In-Reply-To: <533bd1c1-47b0-6e9c-b78b-70db26e791dd@ideasonboard.com>
[-- Attachment #1: Type: text/plain, Size: 45880 bytes --]
Hi Kieran,
thanks for comments
On Tue, Jul 17, 2018 at 10:48:33AM +0100, Kieran Bingham wrote:
> Hi Jacopo,
>
> Thank you for the patch,
>
> Some comments inline.
>
> Feel free to add my
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
> when relevant comments are addressed.
>
> Regards
>
> Kieran
>
>
> On 11/06/18 16:17, Jacopo Mondi wrote:
> > Add V4L2 sensor driver for Aptina MT9V111 CMOS image sensor.
> >
> > The MT9V111 is a 1/4-Inch CMOS image sensor based on MT9V011 with an
> > integrated Image Flow Processor.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > MAINTAINERS | 8 +
> > drivers/media/i2c/Kconfig | 12 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/mt9v111.c | 1297 +++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 1318 insertions(+)
> > create mode 100644 drivers/media/i2c/mt9v111.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index a38e24a..2c2fe60 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9523,6 +9523,14 @@ F: Documentation/devicetree/bindings/media/i2c/mt9v032.txt
> > F: drivers/media/i2c/mt9v032.c
> > F: include/media/i2c/mt9v032.h
> >
> > +MT9V111 APTINA CAMERA SENSOR
> > +M: Jacopo Mondi <jacopo@jmondi.org>
> > +L: linux-media@vger.kernel.org
> > +T: git git://linuxtv.org/media_tree.git
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/media/i2c/aptina,mt9v111.txt
> > +F: drivers/media/i2c/mt9v111.c
> > +
> > MULTIFUNCTION DEVICES (MFD)
> > M: Lee Jones <lee.jones@linaro.org>
> > T: git git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index 341452f..0bd867d 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -841,6 +841,18 @@ config VIDEO_MT9V032
> > This is a Video4Linux2 sensor-level driver for the Micron
> > MT9V032 752x480 CMOS sensor.
> >
> > +config VIDEO_MT9V111
> > + tristate "Aptina MT9V111 sensor support"
> > + depends on I2C && VIDEO_V4L2
> > + depends on MEDIA_CAMERA_SUPPORT
> > + depends on OF
> > + help
> > + This is a Video4Linux2 sensor-level driver for the Aptina/Micron
> > + MT9V111 sensor.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called mt9v111.
> > +
> > config VIDEO_SR030PC30
> > tristate "Siliconfile SR030PC30 sensor support"
> > depends on I2C && VIDEO_V4L2
> > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> > index d679d57..f0e8618 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -84,6 +84,7 @@ obj-$(CONFIG_VIDEO_MT9T001) += mt9t001.o
> > obj-$(CONFIG_VIDEO_MT9T112) += mt9t112.o
> > obj-$(CONFIG_VIDEO_MT9V011) += mt9v011.o
> > obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o
> > +obj-$(CONFIG_VIDEO_MT9V111) += mt9v111.o
> > obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
> > obj-$(CONFIG_VIDEO_NOON010PC30) += noon010pc30.o
> > obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
> > diff --git a/drivers/media/i2c/mt9v111.c b/drivers/media/i2c/mt9v111.c
> > new file mode 100644
> > index 0000000..36e7424
> > --- /dev/null
> > +++ b/drivers/media/i2c/mt9v111.c
> > @@ -0,0 +1,1297 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * V4L2 sensor driver for Aptina MT9V111 image sensor
> > + * Copyright (C) 2018 Jacopo Mondi <jacopo@jmondi.org>
> > + *
> > + * Based on mt9v032 driver
> > + * Copyright (C) 2010, Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > + * Copyright (C) 2008, Guennadi Liakhovetski <kernel@pengutronix.de>
> > + *
> > + * Base on mt9v011 driver
>
> I realise it's probably a copy-paste from another driver and not your
> text, but s/Base/Based/
>
> > + * Copyright (c) 2009 Mauro Carvalho Chehab <mchehab@kernel.org>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/i2c.h>
> > +#include <linux/of.h>
> > +#include <linux/slab.h>
> > +#include <linux/videodev2.h>
> > +#include <linux/v4l2-mediabus.h>
> > +#include <linux/module.h>
> > +
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-image-sizes.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +/*
> > + * MT9V111 is a 1/4-Inch CMOS digital image sensor with an integrated
> > + * Image Flow Processing (IFP) engine and a sensor core loosely based on
> > + * MT9V011.
> > + *
> > + * The IFP can produce several output image formats from the sensor core
> > + * output, this driver currently supports only YUYV format permutations.
>
> s/output, this/output. This/
>
> > + *
> > + * As the auto exposure algorithms controls exposure time and modifies the
> > + * frame rate, this driver disables auto exposure control, auto white balancing
> > + * and auto flickering avoidance to allow manual frame rate control through
> > + * s_frame_interval subdev operation or V4L2_CID_V/HBLANK controls.
>
> Should/Could this be an option perhaps (in the future) to allow someone
> to accept that the framerate will be a bit 'variable' but allow
> algorithms to run?
>
> > + *
> > + * While it seems possible to instruct the auto-exposure control algorithm to
> > + * respect a programmed frame rate when adjusting the pixel integration time,
> > + * registers controlling this feature are not documented in the public
> > + * available sensor manual used to develop this driver (09005aef80e90084,
> > + * MT9V111_1.fm - Rev. G 1/05 EN).
>
> Aha - Ok - so the 'right' way is to fix the rate, but we don't have the
> documentation to be able to do so. :(
>
That's my understanding, yes.
I went with auto-exposure disabled by default, but it actually might
be disabled if and only if frame rate is explicitely modified. It is
not worth doing that in my opinion, but I should add a control to
handle auto-exposure enabling/disabling probably.
>
> > + */
> > +
> > +#define MT9V111_CHIP_ID_HIGH 0x82
> > +#define MT9V111_CHIP_ID_LOW 0x3a
> > +
> > +#define MT9V111_R01_ADDR_SPACE 0x01
> > +#define MT9V111_R01_IFP 0x01
>
> Two identical values (0x01)... there's not a copy-paste error there is
> there ? (I don't yet have the doc to check)
>
> Edit: I assume it's correct actually - as the first is the register
> space, and then IFP is probably the first register in that space...
It is correct, but the first is the register address
(MT9V111_R01_ADDR_SPACE) the second one (and the third one here blow)
are the register values to select the address space where to write to.
>
>
> > +#define MT9V111_R01_CORE 0x04
> > +
> > +#define MT9V111_IFP_R06_OPMODE_CTRL 0x06
> > +#define MT9V111_IFP_R06_OPMODE_CTRL_AWB_EN BIT(1)
> > +#define MT9V111_IFP_R06_OPMODE_CTRL_AE_EN BIT(14)
> > +#define MT9V111_IFP_R07_IFP_RESET 0x07
> > +#define MT9V111_IFP_R07_IFP_RESET_MASK BIT(0)
> > +#define MT9V111_IFP_R08_OUTFMT_CTRL 0x08
> > +#define MT9V111_IFP_R08_OUTFMT_CTRL_FLICKER BIT(11)
> > +#define MT9V111_IFP_R08_OUTFMT_CTRL_PCLK BIT(5)
> > +#define MT9V111_IFP_R3A_OUTFMT_CTRL2 0x3a
> > +#define MT9V111_IFP_R3A_OUTFMT_CTRL2_SWAP_CBCR BIT(0)
> > +#define MT9V111_IFP_R3A_OUTFMT_CTRL2_SWAP_YC BIT(1)
> > +#define MT9V111_IFP_R3A_OUTFMT_CTRL2_SWAP_MASK GENMASK(2, 0)
> > +#define MT9V111_IFP_RA5_HPAN 0xa5
> > +#define MT9V111_IFP_RA6_HZOOM 0xa6
> > +#define MT9V111_IFP_RA7_HOUT 0xa7
> > +#define MT9V111_IFP_RA8_VPAN 0xa8
> > +#define MT9V111_IFP_RA9_VZOOM 0xa9
> > +#define MT9V111_IFP_RAA_VOUT 0xaa
> > +#define MT9V111_IFP_DECIMATION_MASK GENMASK(9, 0)
> > +#define MT9V111_IFP_DECIMATION_FREEZE BIT(15)
> > +
> > +#define MT9V111_CORE_R03_WIN_HEIGHT 0x03
> > +#define MT9V111_CORE_R03_WIN_V_OFFS 2
> > +#define MT9V111_CORE_R04_WIN_WIDTH 0x04
> > +#define MT9V111_CORE_R04_WIN_H_OFFS 114
> > +#define MT9V111_CORE_R05_HBLANK 0x05
> > +#define MT9V111_CORE_R05_MIN_HBLANK 0x09
> > +#define MT9V111_CORE_R05_MAX_HBLANK GENMASK(9, 0)
> > +#define MT9V111_CORE_R05_DEF_HBLANK 0x26
> > +#define MT9V111_CORE_R06_VBLANK 0x06
> > +#define MT9V111_CORE_R06_MIN_VBLANK 0x03
> > +#define MT9V111_CORE_R06_MAX_VBLANK GENMASK(11, 0)
> > +#define MT9V111_CORE_R06_DEF_VBLANK 0x04
> > +#define MT9V111_CORE_R07_OUT_CTRL 0x07
> > +#define MT9V111_CORE_R07_OUT_CTRL_SAMPLE BIT(4)
> > +#define MT9V111_CORE_R09_PIXEL_INT 0x09
> > +#define MT9V111_CORE_R09_PIXEL_INT_MASK GENMASK(11, 0)
> > +#define MT9V111_CORE_R0D_CORE_RESET 0x0d
> > +#define MT9V111_CORE_R0D_CORE_RESET_MASK BIT(0)
> > +#define MT9V111_CORE_RFF_CHIP_VER 0xff
> > +
> > +#define MT9V111_PIXEL_ARRAY_WIDTH 640
> > +#define MT9V111_PIXEL_ARRAY_HEIGHT 480
> > +
> > +#define MT9V111_MAX_CLKIN 27000000
> > +
> > +struct mt9v111_dev {
> > + struct device *dev;
> > + struct i2c_client *client;
> > +
> > + /* Protects address space change operations. */
> > + spinlock_t addr_lock;
> > + u8 addr_space;
> > +
> > + struct v4l2_subdev sd;
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
>
> I wonder if one day we'll have better integration so that we don't have
> to be conditional on CONFIG_MEDIA_CONTROLLER.
>
> That said, would it be better to use?:
> #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)
My understanding is that they work the same, but yes, IS_ENABLED is
nicer.
>
>
> > + struct media_pad pad;
> > +#endif
> > +
> > + struct v4l2_ctrl *hblank;
> > + struct v4l2_ctrl *vblank;
> > + struct v4l2_ctrl_handler ctrls;
> > +
> > + /* Output image format and sizes. */
> > + struct v4l2_mbus_framefmt fmt;
> > + unsigned int fps;
> > +
> > + /* Protects power up/down sequences. */
> > + struct mutex pwr_mutex;
> > + int pwr_count;
> > +
> > + /* Protects stream on/off sequences. */
> > + struct mutex stream_mutex;
> > + bool streaming;
> > +
> > + /* Flags to mark HW settings as not yet applied. */
> > + bool pending;
> > +
> > + /* Clock provider and system clock frequency. */
> > + struct clk *clk;
> > + u32 sysclk;
> > +
> > + struct gpio_desc *oe;
> > + struct gpio_desc *standby;
> > + struct gpio_desc *reset;
> > +};
> > +
> > +#define sd_to_mt9v111(__sd) container_of(__sd, struct mt9v111_dev, sd)
>
> I believe it's recommended to bracket macro parameters:
> #define sd_to_mt9v111(__sd) container_of((__sd), struct mt9v111_dev, sd)
>
Correct, I'll fix it.
> > +
> > +/*
> > + * mt9v111_mbus_fmt - List all media bus formats supported by the driver.
> > + *
> > + * Only list the media bus code here. The image sizes are freely configurable
> > + * in the pixel array sizes range.
> > + *
> > + * The frame desired frame interval, in the supported frame interval range, is
>
> "The frame desired frame interval,"?
>
> Did you mean "The desired frame interval,"?
Yup, thanks for spotting it.
>
>
> > + * obtained by configuring blanking as the sensor does not have a PLL but
> > + * only a fixed clock divider that generates the output pixel clock.
> > + */
> > +static struct mt9v111_mbus_fmt {
> > + u32 code;
> > +} mt9v111_formats[] = {
> > + {
> > + .code = MEDIA_BUS_FMT_UYVY8_2X8,
> > + },
> > + {
> > + .code = MEDIA_BUS_FMT_YUYV8_2X8,
> > + },
> > + {
> > + .code = MEDIA_BUS_FMT_VYUY8_2X8,
> > + },
> > + {
> > + .code = MEDIA_BUS_FMT_YVYU8_2X8,
> > + },
> > +};
> > +
> > +static u32 mt9v111_frame_intervals[] = {5, 10, 15, 20, 30};
> > +
> > +/*
> > + * mt9v111_frame_sizes - List sensor's supported resolutions.
> > + *
> > + * Resolution generated through decimation in the IFP block from the
> > + * full VGA pixel array.
> > + */
> > +static struct v4l2_rect mt9v111_frame_sizes[] = {
> > + {
> > + .width = 640,
> > + .height = 480,
> > + },
> > + {
> > + .width = 352,
> > + .height = 288
> > + },
> > + {
> > + .width = 320,
> > + .height = 240,
> > + },
> > + {
> > + .width = 176,
> > + .height = 144,
> > + },
> > + {
> > + .width = 160,
> > + .height = 120,
> > + },
> > +};
> > +
> > +/* Initial register configuration. */
> > +static struct mt9v111_reg_list {
> > + u8 space;
> > + u8 reg;
> > + u16 mask;
> > + u16 val;
> > +} mt9v111_init_config[] = {
> > + {
> > + /* Disable AE and AWB to enable manual frame rate control. */
> > + .space = MT9V111_R01_IFP,
> > + .reg = MT9V111_IFP_R06_OPMODE_CTRL,
> > + .mask = MT9V111_IFP_R06_OPMODE_CTRL_AWB_EN |
> > + MT9V111_IFP_R06_OPMODE_CTRL_AE_EN,
> > + .val = 0,
> > + },
> > + {
> > + /* Disable anti-flicker to enable manual frame rate control. */
> > + .space = MT9V111_R01_IFP,
> > + .reg = MT9V111_IFP_R08_OUTFMT_CTRL,
> > + .mask = MT9V111_IFP_R08_OUTFMT_CTRL_FLICKER,
> > + .val = 0,
> > + },
> > +
> > + /* TODO: AE and AWB manual configuration. */
> > +};
> > +
> > +/* --- Device I/O access --- */
> > +
> > +static int __mt9v111_read(struct i2c_client *c, u8 reg, u16 *val)
> > +{
> > + struct i2c_msg msg[2];
> > + __be16 buf;
> > + int ret;
> > +
> > + msg[0].addr = c->addr;
> > + msg[0].flags = 0;
> > + msg[0].len = 1;
> > + msg[0].buf = ®
> > +
> > + msg[1].addr = c->addr;
> > + msg[1].flags = I2C_M_RD;
> > + msg[1].len = 2;
> > + msg[1].buf = (char *)&buf;
> > +
> > + ret = i2c_transfer(c->adapter, msg, 2);
> > + if (ret < 0) {
> > + dev_err(&c->dev, "i2c read transfer error: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + *val = be16_to_cpu(buf);
> > +
> > + dev_info(&c->dev, "%s: %x=%x\n", __func__, reg, *val);
>
> dev_info seems a bit verbose to be dumping all register writes ...
> shouldn't this be dev_dbg?
Oh look:
#define KERN_INFO KERN_SOH "6" /* informational */
#define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
#define KERN_DEFAULT KERN_SOH "d" /* the default kernel loglevel */
I was sure it was the other way around (informational sound less
urgent than debug to me :/ )
>
> > +
> > + return 0;
> > +}
> > +
> > +static int __mt9v111_write(struct i2c_client *c, u8 reg, u16 val)
> > +{
> > + struct i2c_msg msg;
> > + u8 buf[3] = { 0 };
> > + int ret;
> > +
> > + buf[0] = reg;
> > + buf[1] = val >> 8;
> > + buf[2] = val & 0xff;
> > +
> > + msg.addr = c->addr;
> > + msg.flags = 0;
> > + msg.len = 3;
> > + msg.buf = (char *)buf;
> > +
> > + dev_info(&c->dev, "%s: %x = %x%x\n", __func__, reg, buf[1], buf[2]);
>
> same here ... should this be dev_dbg?
>
>
> > +
> > + ret = i2c_transfer(c->adapter, &msg, 1);
> > + if (ret < 0) {
> > + dev_err(&c->dev, "i2c write transfer error: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int __mt9v111_addr_space_select(struct i2c_client *c, u16 addr_space)
> > +{
> > + struct v4l2_subdev *sd = i2c_get_clientdata(c);
> > + struct mt9v111_dev *mt9v111 = sd_to_mt9v111(sd);
> > + u16 val;
> > + int ret;
> > +
> > + spin_lock(&mt9v111->addr_lock);
> > + if (mt9v111->addr_space == addr_space) {
> > + spin_unlock(&mt9v111->addr_lock);
> > + return 0;
> > + }
> > + spin_unlock(&mt9v111->addr_lock);
> > +
> > + ret = __mt9v111_write(c, MT9V111_R01_ADDR_SPACE, addr_space);
> > + if (ret)
> > + return ret;
> > +
> > + /* Verify address space has been updated */
> > + ret = __mt9v111_read(c, MT9V111_R01_ADDR_SPACE, &val);
> > + if (ret)
> > + return ret;
> > +
> > + if (val != addr_space)
> > + return -EINVAL;
> > +
> > + mt9v111->addr_space = addr_space;
>
> Should this write to the mt9v111->addr_space be locked, considering the
> read at the beginning of the function is locked?
>
It should.
Or either or the whole function should be mutex protected, or neither
the single per-driver variable should be. Probably I should just drop
the spinlock and that's it.. Concurrent read/writes on a driver with a
single devnode shouldn't be an issue actually.
> > +
> > + return 0;
> > +}
> > +
> > +static int mt9v111_read(struct i2c_client *c, u8 addr_space, u8 reg, u16 *val)
> > +{
> > + int ret;
> > +
> > + /* Select register address space first. */
> > + ret = __mt9v111_addr_space_select(c, addr_space);
> > + if (ret)
> > + return ret;
> > +
> > + ret = __mt9v111_read(c, reg, val);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int mt9v111_write(struct i2c_client *c, u8 addr_space, u8 reg, u16 val)
> > +{
> > + int ret;
> > +
> > + /* Select register address space first. */
> > + ret = __mt9v111_addr_space_select(c, addr_space);
> > + if (ret)
> > + return ret;
> > +
> > + ret = __mt9v111_write(c, reg, val);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int mt9v111_update(struct i2c_client *c, u8 addr_space, u8 reg,
> > + u16 mask, u16 val)
> > +{
> > + u16 current_val;
> > + int ret;
> > +
> > + /* Select register address space first. */
> > + ret = __mt9v111_addr_space_select(c, addr_space);
> > + if (ret)
> > + return ret;
> > +
> > + /* Read the current register value, then update it. */
> > + ret = __mt9v111_read(c, reg, ¤t_val);
> > + if (ret)
> > + return ret;
> > +
> > + current_val &= ~mask;
> > + current_val |= (val & mask);
> > + ret = __mt9v111_write(c, reg, current_val);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int mt9v111_update_reglist(struct i2c_client *c,
> > + struct mt9v111_reg_list *reg_list,
> > + unsigned int n_regs)
> > +{
> > + unsigned int i;
> > + int ret;
> > +
> > + for (i = 0; i < n_regs; i++) {
> > + struct mt9v111_reg_list *reg = ®_list[i];
> > +
> > + ret = mt9v111_update(c, reg->space, reg->reg, reg->mask,
> > + reg->val);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* --- Sensor HW operations --- */
> > +
> > +static int __mt9v111_power_on(struct v4l2_subdev *sd)
> > +{
> > + struct mt9v111_dev *mt9v111 = sd_to_mt9v111(sd);
> > + int ret;
> > +
> > + ret = clk_prepare_enable(mt9v111->clk);
> > + if (ret)
> > + return ret;
> > +
> > + clk_set_rate(mt9v111->clk, mt9v111->sysclk);
> > +
> > + gpiod_set_value(mt9v111->standby, 0);
> > + usleep_range(500, 1000);
> > +
> > + gpiod_set_value(mt9v111->oe, 1);
> > + usleep_range(500, 1000);
> > +
> > + return 0;
> > +}
> > +
> > +static int __mt9v111_power_off(struct v4l2_subdev *sd)
> > +{
> > + struct mt9v111_dev *mt9v111 = sd_to_mt9v111(sd);
> > +
> > + gpiod_set_value(mt9v111->oe, 0);
> > + usleep_range(500, 1000);
> > +
> > + gpiod_set_value(mt9v111->standby, 1);
> > + usleep_range(500, 1000);
> > +
> > + clk_disable_unprepare(mt9v111->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static int __mt9v111_hw_reset(struct mt9v111_dev *mt9v111)
> > +{
> > + if (!mt9v111->reset)
> > + return -EINVAL;
> > +
> > + gpiod_set_value(mt9v111->reset, 1);
> > + usleep_range(500, 1000);
> > +
> > + gpiod_set_value(mt9v111->reset, 0);
> > + usleep_range(500, 1000);
> > +
> > + return 0;
> > +}
> > +
> > +static int __mt9v111_sw_reset(struct mt9v111_dev *mt9v111)
> > +{
> > + struct i2c_client *c = mt9v111->client;
> > + unsigned int ret;
>
> But the return type of the function is not unsigned?
More than that, the return value from mt9v111_update() is case of
errors is one coming from i2c_transfer(), so it must be signed. Sorry,
this was a trivial mistake.
>
> > +
> > + /* Software reset core and IFP blocks. */
> > +
> > + ret = mt9v111_update(c, MT9V111_R01_CORE,
> > + MT9V111_CORE_R0D_CORE_RESET,
> > + MT9V111_CORE_R0D_CORE_RESET_MASK, 1);
> > + if (ret)
> > + return ret;
> > + usleep_range(500, 1000);
> > +
> > + ret = mt9v111_update(c, MT9V111_R01_CORE,
> > + MT9V111_CORE_R0D_CORE_RESET,
> > + MT9V111_CORE_R0D_CORE_RESET_MASK, 0);
> > + if (ret)
> > + return ret;
> > + usleep_range(500, 1000);
> > +
> > + ret = mt9v111_update(c, MT9V111_R01_IFP,
> > + MT9V111_IFP_R07_IFP_RESET,
> > + MT9V111_IFP_R07_IFP_RESET_MASK, 1);
> > + if (ret)
> > + return ret;
> > + usleep_range(500, 1000);
> > +
> > + ret = mt9v111_update(c, MT9V111_R01_IFP,
> > + MT9V111_IFP_R07_IFP_RESET,
> > + MT9V111_IFP_R07_IFP_RESET_MASK, 0);
> > + if (ret)
> > + return ret;
> > + usleep_range(500, 1000);
> > +
> > + return 0;
> > +}
> > +
> > +static int mt9v111_calc_frame_rate(struct mt9v111_dev *mt9v111,
> > + struct v4l2_fract *tpf)
> > +{
> > + unsigned int fps = tpf->numerator ?
> > + tpf->denominator / tpf->numerator :
> > + tpf->denominator;
> > + unsigned int best_diff;
> > + unsigned int frm_cols;
> > + unsigned int row_pclk;
> > + unsigned int best_fps;
> > + unsigned int pclk;
> > + unsigned int diff;
> > + unsigned int idx;
> > + unsigned int hb;
> > + unsigned int vb;
> > + unsigned int i;
> > + int ret;
> > +
> > + /* Approximate to the closest supported frame interval. */
> > + best_diff = ~0L;> + for (i = 0, idx = 0; i < ARRAY_SIZE(mt9v111_frame_intervals); i++) {
> > + diff = abs(fps - mt9v111_frame_intervals[i]);
> > + if (diff < best_diff) {> + idx = i;
> > + best_diff = diff;
> > + }
> > + }
> > + fps = mt9v111_frame_intervals[idx];
> > +
> > + /*
> > + * The sensor does not provide a PLL circuitry and pixel clock is
> > + * generated dividing by two the master clock source.
>
> "The sensor does not provide a PLL circuitry and the pixel clock is
> generated by dividing the master clock source by two." ?
>
One day I'll get english language syntax properly...
> > + *
> > + * Trow = (W + Hblank + 114) * 2 * (1 / SYSCLK)
> > + * TFrame = Trow * (H + Vblank + 2)
> > + *
> > + * FPS = (SYSCLK / 2) / (Trow * (H + Vblank + 2))
> > + *
> > + * This boils down to tune H and V blanks to best approximate the
> > + * above equation.
> > + *
> > + * Test all available H/V blank values, until we reach the
> > + * desired frame rate.
> > + */
> > + best_fps = vb = hb = 0;
> > + pclk = DIV_ROUND_CLOSEST(mt9v111->sysclk, 2);
> > + row_pclk = MT9V111_PIXEL_ARRAY_WIDTH + 7 + MT9V111_CORE_R04_WIN_H_OFFS;
> > + frm_cols = MT9V111_PIXEL_ARRAY_HEIGHT + 7 + MT9V111_CORE_R03_WIN_V_OFFS;
> > +
> > + best_diff = ~0L;
> > + for (vb = MT9V111_CORE_R06_MIN_VBLANK;
> > + vb < MT9V111_CORE_R06_MAX_VBLANK; vb++) {
> > + for (hb = MT9V111_CORE_R05_MIN_HBLANK;
> > + hb < MT9V111_CORE_R05_MAX_HBLANK; hb += 10) {
> > + unsigned int t_frame = (row_pclk + hb) *
> > + (frm_cols + vb);
> > + unsigned int t_fps = DIV_ROUND_CLOSEST(pclk, t_frame);
> > +
> > + diff = abs(fps - t_fps);
> > + if (diff < best_diff) {
> > + best_diff = diff;
> > + best_fps = t_fps;
> > +
> > + if (diff == 0)
> > + break;
> > + }
> > + }
> > +
> > + if (diff == 0)
> > + break;
> > + }
> > +
> > + dev_info(mt9v111->dev, "FPS: %u, hb = %u, vb = %u\n", fps, hb, vb);
> > +
> > + ret = v4l2_ctrl_s_ctrl_int64(mt9v111->hblank, hb);
> > + if (ret)
> > + return ret;
> > +
> > + ret = v4l2_ctrl_s_ctrl_int64(mt9v111->vblank, vb);
> > + if (ret)
> > + return ret;
> > +
> > + tpf->numerator = 1;
> > + tpf->denominator = best_fps;
> > +
> > + return 0;
> > +}
> > +
> > +static int mt9v111_hw_config(struct mt9v111_dev *mt9v111)
> > +{
> > + struct i2c_client *c = mt9v111->client;
> > + unsigned int ret;
> > + u16 outfmtctrl2;
> > +
> > + /* Force device reset. */
> > + ret = __mt9v111_hw_reset(mt9v111);
> > + if (ret == -EINVAL)
> > + ret = __mt9v111_sw_reset(mt9v111);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * Disable AE, AWB and anti-flickering to allow manual frame rate
> > + * control.
> > + */
> > + ret = mt9v111_update_reglist(c, mt9v111_init_config,
> > + ARRAY_SIZE(mt9v111_init_config));
> > + if (ret)
> > + return ret;
> > +
> > + /* Configure internal clock sample rate. */
> > + ret = mt9v111->sysclk < DIV_ROUND_CLOSEST(MT9V111_MAX_CLKIN, 2) ?
> > + mt9v111_update(c, MT9V111_R01_CORE,
> > + MT9V111_CORE_R07_OUT_CTRL,
> > + MT9V111_CORE_R07_OUT_CTRL_SAMPLE, 1) :
> > + mt9v111_update(c, MT9V111_R01_CORE,
> > + MT9V111_CORE_R07_OUT_CTRL,
> > + MT9V111_CORE_R07_OUT_CTRL_SAMPLE, 0);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * Configure output image format components ordering.
> > + *
> > + * TODO: IFP block can also output several RGB permutations, we only
> > + * support YUYV permutations at the moment.
> > + */
> > + switch (mt9v111->fmt.code) {
> > + case MEDIA_BUS_FMT_YUYV8_2X8:
> > + outfmtctrl2 = MT9V111_IFP_R3A_OUTFMT_CTRL2_SWAP_YC;
> > + break;
> > + case MEDIA_BUS_FMT_VYUY8_2X8:
> > + outfmtctrl2 = MT9V111_IFP_R3A_OUTFMT_CTRL2_SWAP_CBCR;
> > + break;
> > + case MEDIA_BUS_FMT_YVYU8_2X8:
> > + outfmtctrl2 = MT9V111_IFP_R3A_OUTFMT_CTRL2_SWAP_YC |
> > + MT9V111_IFP_R3A_OUTFMT_CTRL2_SWAP_CBCR;
> > + break;
> > + case MEDIA_BUS_FMT_UYVY8_2X8:
> > + default:
> > + outfmtctrl2 = 0;
> > + break;
> > + }
> > +
> > + ret = mt9v111_update(c, MT9V111_R01_IFP, MT9V111_IFP_R3A_OUTFMT_CTRL2,
> > + MT9V111_IFP_R3A_OUTFMT_CTRL2_SWAP_MASK,
> > + outfmtctrl2);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * Do not change default sensor's core configuration:
> > + * output the whole 640x480 pixel array, skip 18 columns and 6 rows.
>
> I'm not sure I understand why we skip 18 columns and 6 rows?
> (or does it not matter, and it's just necessary)
Sensor usually a variable number of columns and rows which are blank
or non active and are used for eg. black level calibration.
>
> > + *
> > + * Instead, control the output image size through IFP block.
> > + *
> > + * TODO: No zoom&pan support. Currently we control the output image
> > + * size only through decimation, with no zoom support.
> > + */
> > + ret = mt9v111_write(c, MT9V111_R01_IFP, MT9V111_IFP_RA5_HPAN,
> > + MT9V111_IFP_DECIMATION_FREEZE);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mt9v111_write(c, MT9V111_R01_IFP, MT9V111_IFP_RA8_VPAN,
> > + MT9V111_IFP_DECIMATION_FREEZE);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mt9v111_write(c, MT9V111_R01_IFP, MT9V111_IFP_RA6_HZOOM,
> > + MT9V111_IFP_DECIMATION_FREEZE |
> > + MT9V111_PIXEL_ARRAY_WIDTH);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mt9v111_write(c, MT9V111_R01_IFP, MT9V111_IFP_RA9_VZOOM,
> > + MT9V111_IFP_DECIMATION_FREEZE |
> > + MT9V111_PIXEL_ARRAY_HEIGHT);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mt9v111_write(c, MT9V111_R01_IFP, MT9V111_IFP_RA7_HOUT,
> > + MT9V111_IFP_DECIMATION_FREEZE |
> > + mt9v111->fmt.width);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mt9v111_write(c, MT9V111_R01_IFP, MT9V111_IFP_RAA_VOUT,
> > + mt9v111->fmt.height);
> > + if (ret)
> > + return ret;
> > +
> > + /* Update blankings to match the programmed frame rate. */
> > + ret = mt9v111_update(c, MT9V111_R01_CORE, MT9V111_CORE_R05_HBLANK,
> > + MT9V111_CORE_R05_MAX_HBLANK,
> > + mt9v111->hblank->val);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mt9v111_update(c, MT9V111_R01_CORE, MT9V111_CORE_R06_VBLANK,
> > + MT9V111_CORE_R06_MAX_VBLANK,
> > + mt9v111->vblank->val);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * Set pixel integration time to the whole frame time.
> > + * This value controls the the shutter delay when running with AE
> > + * disabled. If longer than frame time, it affects the output
> > + * frame rate.
> > + */
> > + ret = mt9v111_write(c, MT9V111_R01_CORE, MT9V111_CORE_R09_PIXEL_INT,
> > + MT9V111_PIXEL_ARRAY_HEIGHT);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +/* --- V4L2 subdev operations --- */
> > +
> > +static int mt9v111_s_power(struct v4l2_subdev *sd, int on)
> > +{
> > + struct mt9v111_dev *mt9v111 = sd_to_mt9v111(sd);
> > + int pwr_count;
> > + int ret = 0;
> > +
> > + mutex_lock(&mt9v111->pwr_mutex);
> > +
> > + /*
> > + * Make sure we're transitioning from 0 to 1, or viceversa, only
> > + * before actually changing the power state.
> > + */
> > + pwr_count = mt9v111->pwr_count;
> > + pwr_count += on ? 1 : -1;
> > + if (pwr_count == !!on) {
> > + ret = on ? __mt9v111_power_on(sd) :
> > + __mt9v111_power_off(sd);
> > + if (!ret)
> > + /* All went well, updated power counter. */
> > + mt9v111->pwr_count = pwr_count;
> > +
> > + mutex_unlock(&mt9v111->pwr_mutex);
> > +
> > + return ret;
> > + }
> > +
> > + /*
> > + * Update power counter to keep track of how many nested calls we
> > + * received.
> > + */
> > + WARN_ON(pwr_count < 0 || pwr_count > 1);
> > + mt9v111->pwr_count = pwr_count;
> > +
> > + mutex_unlock(&mt9v111->pwr_mutex);
> > +
> > + return ret;
> > +}
> > +
> > +static int mt9v111_s_stream(struct v4l2_subdev *subdev, int enable)
> > +{
> > + struct mt9v111_dev *mt9v111 = sd_to_mt9v111(subdev);
> > + int ret;
> > +
> > + mutex_lock(&mt9v111->stream_mutex);
> > +
> > + if (mt9v111->streaming == enable) {
> > + mutex_unlock(&mt9v111->stream_mutex);
> > + return 0;
> > + }
> > +
> > + ret = mt9v111_s_power(subdev, enable);
> > + if (ret)
> > + goto error_unlock;
> > +
> > + if (enable && mt9v111->pending) {
> > + ret = mt9v111_hw_config(mt9v111);
> > + if (ret)
> > + goto error_unlock;
> > +
> > + /*
> > + * No need to update control here as far as only H/VBLANK are
> > + * supported and immediately programmed to registers in .s_ctrl
> > + */
> > +
> > + mt9v111->pending = false;
> > + }
> > +
> > + mt9v111->streaming = enable ? true : false;
> > + mutex_unlock(&mt9v111->stream_mutex);
> > +
> > + return 0;
> > +
> > +error_unlock:
> > + mutex_unlock(&mt9v111->stream_mutex);
> > +
> > + return ret;
> > +}
> > +
> > +static int mt9v111_s_frame_interval(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_frame_interval *ival)
> > +{
> > + struct mt9v111_dev *mt9v111 = sd_to_mt9v111(sd);
> > + struct v4l2_fract *tpf = &ival->interval;
> > + unsigned int fps = tpf->numerator ?
> > + tpf->denominator / tpf->numerator :
> > + tpf->denominator;
> > + unsigned int max_fps;
> > +
> > + mutex_lock(&mt9v111->stream_mutex);
> > + if (mt9v111->streaming) {
> > + mutex_unlock(&mt9v111->stream_mutex);
> > + return -EBUSY;
> > + }
> > + mutex_unlock(&mt9v111->stream_mutex);
> > +
> > + if (mt9v111->fps == fps)
> > + return 0;
> > +
> > + /* Make sure frame rate/image sizes constraints are respected. */
> > + if (mt9v111->fmt.width < QVGA_WIDTH &&
> > + mt9v111->fmt.height < QVGA_HEIGHT)
> > + max_fps = 90;
> > + else if (mt9v111->fmt.width < CIF_WIDTH &&
> > + mt9v111->fmt.height < CIF_HEIGHT)
> > + max_fps = 60;
> > + else
> > + max_fps = mt9v111->sysclk <
> > + DIV_ROUND_CLOSEST(MT9V111_MAX_CLKIN, 2) ? 15 :
> > + 30;
> > +
> > + if (fps > max_fps)
> > + return -EINVAL;
> > +
> > + mt9v111_calc_frame_rate(mt9v111, tpf);
> > +
> > + mt9v111->fps = fps;
> > + mt9v111->pending = true;
> > +
> > + return 0;
> > +}
> > +
> > +static int mt9v111_g_frame_interval(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_frame_interval *ival)
> > +{
> > + struct mt9v111_dev *mt9v111 = sd_to_mt9v111(sd);
> > + struct v4l2_fract *tpf = &ival->interval;
> > +
> > + tpf->numerator = 1;
> > + tpf->denominator = mt9v111->fps;
> > +
> > + return 0;
> > +}
> > +
> > +static struct v4l2_mbus_framefmt *__mt9v111_get_pad_format(
> > + struct mt9v111_dev *mt9v111,
> > + struct v4l2_subdev_pad_config *cfg,
> > + unsigned int pad,
> > + enum v4l2_subdev_format_whence which)
> > +{
> > + switch (which) {
> > + case V4L2_SUBDEV_FORMAT_TRY:
> > +#ifdef CONFIG_MEDIA_CONTROLLER
>
> #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)?
>
> > + return v4l2_subdev_get_try_format(&mt9v111->sd, cfg, pad);
> > +#else
> > + return &cfg->try_fmt;
> > +#endif
> > + case V4L2_SUBDEV_FORMAT_ACTIVE:
> > + return &mt9v111->fmt;
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > +static int mt9v111_enum_mbus_code(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_mbus_code_enum *code)
> > +{
> > + if (code->pad || code->index > ARRAY_SIZE(mt9v111_formats) - 1)
> > + return -EINVAL;
> > +
> > + code->code = mt9v111_formats[code->index].code;
> > +
> > + return 0;
> > +}
> > +
> > +static int mt9v111_enum_frame_interval(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_frame_interval_enum *fie)
> > +{
> > + unsigned int i;
> > +
> > + if (fie->pad || fie->index >= ARRAY_SIZE(mt9v111_frame_intervals))
> > + return -EINVAL;
> > +
> > + for (i = 0; i < ARRAY_SIZE(mt9v111_frame_sizes); i++)
> > + if (fie->width == mt9v111_frame_sizes[i].width &&
> > + fie->height == mt9v111_frame_sizes[i].height)
> > + break;
> > +
> > + if (i == ARRAY_SIZE(mt9v111_frame_sizes))
> > + return -EINVAL;
> > +
> > + fie->interval.numerator = 1;
> > + fie->interval.denominator = mt9v111_frame_intervals[fie->index];
> > +
> > + return 0;
> > +}
> > +
> > +static int mt9v111_enum_frame_size(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_frame_size_enum *fse)
> > +{
> > + if (fse->pad || fse->index > ARRAY_SIZE(mt9v111_frame_sizes))
> > + return -EINVAL;
> > +
> > + fse->min_width = mt9v111_frame_sizes[fse->index].width;
> > + fse->max_width = mt9v111_frame_sizes[fse->index].width;
> > + fse->min_height = mt9v111_frame_sizes[fse->index].height;
> > + fse->max_height = mt9v111_frame_sizes[fse->index].height;
> > +
> > + return 0;
> > +}
> > +
> > +static int mt9v111_get_format(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *format)
> > +{
> > + struct mt9v111_dev *mt9v111 = sd_to_mt9v111(subdev);
> > +
> > + if (format->pad)
> > + return -EINVAL;
> > +
> > + format->format = *__mt9v111_get_pad_format(mt9v111, cfg, format->pad,
> > + format->which);
> > + return 0;
> > +}
> > +
> > +static int mt9v111_set_format(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *format)
> > +{
> > + struct mt9v111_dev *mt9v111 = sd_to_mt9v111(subdev);
> > + struct v4l2_mbus_framefmt new_fmt;
> > + struct v4l2_mbus_framefmt *__fmt;
> > + unsigned int best_fit = ~0L;
> > + unsigned int idx;
> > + unsigned int i;
> > +
> > + mutex_lock(&mt9v111->stream_mutex);
> > + if (mt9v111->streaming) {
> > + mutex_unlock(&mt9v111->stream_mutex);
> > + return -EBUSY;
> > + }
> > + mutex_unlock(&mt9v111->stream_mutex);
> > +
> > + if (format->pad)
> > + return -EINVAL;
> > +
> > + /* Update mbus format code and sizes. */
> > + for (i = 0; i < ARRAY_SIZE(mt9v111_formats); i++) {
> > + if (format->format.code == mt9v111_formats[i].code) {
> > + new_fmt.code = mt9v111_formats[i].code;
> > + break;
> > + }
> > + }
> > + if (i == ARRAY_SIZE(mt9v111_formats))
> > + new_fmt.code = mt9v111_formats[0].code;
> > +
> > + for (i = 0; i < ARRAY_SIZE(mt9v111_frame_sizes); i++) {
> > + unsigned int fit = abs(mt9v111_frame_sizes[i].width -
> > + format->format.width) +
> > + abs(mt9v111_frame_sizes[i].height -
> > + format->format.height);
> > + if (fit < best_fit) {
> > + best_fit = fit;
> > + idx = i;
> > +
> > + if (fit == 0)
> > + break;
> > + }
> > + }
> > + new_fmt.width = mt9v111_frame_sizes[idx].width;
> > + new_fmt.height = mt9v111_frame_sizes[idx].height;
> > +
> > + /* Update the device (or pad) format if it has changed. */
> > + __fmt = __mt9v111_get_pad_format(mt9v111, cfg, format->pad,
> > + format->which);
> > +
> > + /* Format hasn't changed, stop here. */
> > + if (__fmt->code == new_fmt.code &&
> > + __fmt->width == new_fmt.width &&
> > + __fmt->height == new_fmt.height)
> > + goto done;
> > +
> > + /* Update the format and sizes, then mark changes as pending. */
> > + __fmt->code = new_fmt.code;
> > + __fmt->width = new_fmt.width;
> > + __fmt->height = new_fmt.height;
> > +
> > + if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> > + mt9v111->pending = true;
> > +
> > + dev_info(mt9v111->dev, "%s: mbus_code: %x - (%ux%u)\n",
> > + __func__, __fmt->code, __fmt->width, __fmt->height);
> > +
> > +done:
> > + format->format = *__fmt;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_subdev_core_ops mt9v111_core_ops = {
> > + .s_power = mt9v111_s_power,
> > +};
> > +
> > +static const struct v4l2_subdev_video_ops mt9v111_video_ops = {
> > + .s_stream = mt9v111_s_stream,
> > + .s_frame_interval = mt9v111_s_frame_interval,
> > + .g_frame_interval = mt9v111_g_frame_interval,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops mt9v111_pad_ops = {
> > + .enum_mbus_code = mt9v111_enum_mbus_code,
> > + .enum_frame_size = mt9v111_enum_frame_size,
> > + .enum_frame_interval = mt9v111_enum_frame_interval,
> > + .get_fmt = mt9v111_get_format,
> > + .set_fmt = mt9v111_set_format,
> > +};
> > +
> > +static const struct v4l2_subdev_ops mt9v111_ops = {
> > + .core = &mt9v111_core_ops,
> > + .video = &mt9v111_video_ops,
> > + .pad = &mt9v111_pad_ops,
> > +};
> > +
> > +#ifdef CONFIG_MEDIA_CONTROLLER
>
> #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)?
>
> > +static const struct media_entity_operations mt9v111_subdev_entity_ops = {
> > + .link_validate = v4l2_subdev_link_validate,
> > +};
> > +
> > +static int mt9v111_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > +{
> > + struct mt9v111_dev *mt9v111 = sd_to_mt9v111(sd);
> > + struct v4l2_mbus_framefmt *__fmt =
> > + v4l2_subdev_get_try_format(sd, fh->pad, 0);
> > +
> > + /* Initialize try format: copy the device format. */
> > + *__fmt = mt9v111->fmt;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_subdev_internal_ops mt9v111_internal_ops = {
> > + .open = mt9v111_open,
> > +};
> > +#endif
> > +
> > +/* --- V4L2 ctrl --- */
> > +static int mt9v111_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct mt9v111_dev *mt9v111 = container_of(ctrl->handler,
> > + struct mt9v111_dev,
> > + ctrls);
> > + int ret = 0;
> > +
> > + mutex_lock(&mt9v111->pwr_mutex);
> > + /*
> > + * If sensor is powered down, just cache new control values,
> > + * no actual register access.
> > + */
> > + if (!mt9v111->pwr_count) {
> > + mutex_unlock(&mt9v111->pwr_mutex);
> > + return 0;
> > + }
> > + mutex_unlock(&mt9v111->pwr_mutex);
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_HBLANK:
> > + ret = mt9v111_update(mt9v111->client, MT9V111_R01_CORE,
> > + MT9V111_CORE_R05_HBLANK,
> > + MT9V111_CORE_R05_MAX_HBLANK,
> > + mt9v111->hblank->val);
> > + break;
> > + case V4L2_CID_VBLANK:
> > + ret = mt9v111_update(mt9v111->client, MT9V111_R01_CORE,
> > + MT9V111_CORE_R06_VBLANK,
> > + MT9V111_CORE_R06_MAX_VBLANK,
> > + mt9v111->vblank->val);
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops mt9v111_ctrl_ops = {
> > + .s_ctrl = mt9v111_s_ctrl,
> > +};
> > +
> > +static int mt9v111_chip_probe(struct mt9v111_dev *mt9v111)
> > +{
> > + int ret;
> > + u16 val;
> > +
> > + ret = __mt9v111_power_on(&mt9v111->sd);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mt9v111_read(mt9v111->client, MT9V111_R01_CORE,
> > + MT9V111_CORE_RFF_CHIP_VER, &val);
> > + if (ret)
> > + return ret;
> > +
> > + if ((val >> 8) != MT9V111_CHIP_ID_HIGH &&
> > + (val & 0xff) != MT9V111_CHIP_ID_LOW) {
> > + dev_err(mt9v111->dev,
> > + "Unable to identify MT9V111 chip: 0x%2x%2x\n",
> > + val >> 8, val & 0xff);
> > + return ret;
> > + }
> > +
> > + ret = __mt9v111_power_off(&mt9v111->sd);
> > + if (ret)
> > + return ret;
> > +
> > + dev_info(mt9v111->dev, "Chip identified: 0x%2x%2x\n",
> > + val >> 8, val & 0xff);
> > +
> > + return 0;
> > +}
> > +
> > +static int mt9v111_probe(struct i2c_client *client)
> > +{
> > + struct mt9v111_dev *mt9v111;
> > + struct v4l2_fract tpf;
> > + int ret;
> > +
> > + mt9v111 = devm_kzalloc(&client->dev, sizeof(*mt9v111), GFP_KERNEL);
> > + if (!mt9v111)
> > + return -ENOMEM;
> > +
> > + mt9v111->dev = &client->dev;
> > + mt9v111->client = client;
> > +
> > + mt9v111->clk = devm_clk_get(&client->dev, NULL);
> > + if (IS_ERR(mt9v111->clk))
> > + return PTR_ERR(mt9v111->clk);
> > +
> > + mt9v111->sysclk = clk_get_rate(mt9v111->clk);
> > + if (mt9v111->sysclk > MT9V111_MAX_CLKIN)
> > + return -EINVAL;
> > +
> > + mt9v111->oe = devm_gpiod_get_optional(&client->dev, "enable",
> > + GPIOD_OUT_LOW);
> > + if (IS_ERR(mt9v111->oe)) {
> > + dev_err(&client->dev, "Unable to get GPIO \"enable\": %ld\n",
> > + PTR_ERR(mt9v111->oe));
> > + return PTR_ERR(mt9v111->oe);
> > + }
> > +
> > + mt9v111->standby = devm_gpiod_get_optional(&client->dev, "standby",
> > + GPIOD_OUT_HIGH);
> > + if (IS_ERR(mt9v111->standby)) {
> > + dev_err(&client->dev, "Unable to get GPIO \"standby\": %ld\n",
> > + PTR_ERR(mt9v111->standby));
> > + return PTR_ERR(mt9v111->standby);
> > + }
> > +
> > + mt9v111->reset = devm_gpiod_get_optional(&client->dev, "reset",
> > + GPIOD_OUT_LOW);
> > + if (IS_ERR(mt9v111->reset)) {
> > + dev_err(&client->dev, "Unable to get GPIO \"reset\": %ld\n",
> > + PTR_ERR(mt9v111->reset));
> > + return PTR_ERR(mt9v111->reset);
> > + }
> > +
> > + mutex_init(&mt9v111->pwr_mutex);
> > + mutex_init(&mt9v111->stream_mutex);
> > + spin_lock_init(&mt9v111->addr_lock);
> > +
> > + v4l2_ctrl_handler_init(&mt9v111->ctrls, 3);
> > +
> > + mt9v111->hblank = v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops,
> > + V4L2_CID_HBLANK,
> > + MT9V111_CORE_R05_MIN_HBLANK,
> > + MT9V111_CORE_R05_MAX_HBLANK, 1,
> > + MT9V111_CORE_R05_DEF_HBLANK);
> > + if (IS_ERR_OR_NULL(mt9v111->hblank)) {
> > + ret = PTR_ERR(mt9v111->hblank);
> > + goto error_free_ctrls;
> > + }
> > +
> > + mt9v111->vblank = v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops,
> > + V4L2_CID_VBLANK,
> > + MT9V111_CORE_R06_MIN_VBLANK,
> > + MT9V111_CORE_R06_MAX_VBLANK, 1,
> > + MT9V111_CORE_R06_DEF_VBLANK);
> > + if (IS_ERR_OR_NULL(mt9v111->vblank)) {
> > + ret = PTR_ERR(mt9v111->vblank);
> > + goto error_free_ctrls;
> > + }
> > +
> > + /* PIXEL_RATE is fixed: just expose it to user space. */
> > + v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops,
> > + V4L2_CID_PIXEL_RATE, 0,
> > + DIV_ROUND_CLOSEST(mt9v111->sysclk, 2), 1,
> > + DIV_ROUND_CLOSEST(mt9v111->sysclk, 2));
> > +
> > + mt9v111->sd.ctrl_handler = &mt9v111->ctrls;
> > +
> > + /* Start with default configuration: 640x480 UYVY. */
> > + mt9v111->fmt.width = 640;
> > + mt9v111->fmt.height = 480;
> > + mt9v111->fmt.code = MEDIA_BUS_FMT_UYVY8_2X8;
> > +
> > + /* These are fixed for all supported formats. */
> > + mt9v111->fmt.field = V4L2_FIELD_NONE;
> > + mt9v111->fmt.colorspace = V4L2_COLORSPACE_SRGB;
> > + mt9v111->fmt.ycbcr_enc = V4L2_YCBCR_ENC_601;
> > + mt9v111->fmt.quantization = V4L2_QUANTIZATION_LIM_RANGE;
> > + mt9v111->fmt.xfer_func = V4L2_XFER_FUNC_SRGB;
> > +
> > + /* Re-calculate blankings for 640x480@15fps. */
> > + mt9v111->fps = 15;
> > + tpf.numerator = 1;
> > + tpf.denominator = mt9v111->fps;
> > + ret = mt9v111_calc_frame_rate(mt9v111, &tpf);
> > + if (ret)
> > + goto error_free_ctrls;
> > +
> > + mt9v111->pwr_count = 0;
> > + mt9v111->addr_space = MT9V111_R01_IFP;
> > + mt9v111->pending = true;
> > +
> > + v4l2_i2c_subdev_init(&mt9v111->sd, client, &mt9v111_ops);
> > +
> > +#ifdef CONFIG_MEDIA_CONTROLLER
>
> #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)?
>
> > + mt9v111->sd.internal_ops = &mt9v111_internal_ops;
> > + mt9v111->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > + mt9v111->sd.entity.ops = &mt9v111_subdev_entity_ops;
> > + mt9v111->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > + mt9v111->pad.flags = MEDIA_PAD_FL_SOURCE;
> > + ret = media_entity_pads_init(&mt9v111->sd.entity, 1, &mt9v111->pad);
> > + if (ret)
> > + goto error_free_ctrls;
> > +#endif
> > +
> > + ret = mt9v111_chip_probe(mt9v111);
> > + if (ret)
> > + goto error_free_ctrls;
> > +
> > + ret = v4l2_async_register_subdev(&mt9v111->sd);
> > + if (ret)
> > + goto error_free_ctrls;
> > +
> > + return 0;
> > +
> > +error_free_ctrls:
> > + v4l2_ctrl_handler_free(&mt9v111->ctrls);
> > +
> > +#ifdef CONFIG_MEDIA_CONTROLLER
> > + media_entity_cleanup(&mt9v111->sd.entity);
> > +#endif
>
>
> media_entity_cleanup() has #if IS_ENABLED(MC) barriers around it with a
> null implementation so we could drop the #ifdef CONFIG_ here ... but
> perhaps while we have to have a non-MC supported implementation it keeps
> the distinction clear...
>
> Otherwise,
> #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)?
Ah, I see. Actually I would prefer to keep it explicit since it makes
the code clearer.
Thanks for your comments
j
>
>
> > +
> > + mutex_destroy(&mt9v111->pwr_mutex);
> > + mutex_destroy(&mt9v111->stream_mutex);
> > +
> > + return ret;
> > +}
> > +
> > +static int mt9v111_remove(struct i2c_client *client)
> > +{
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct mt9v111_dev *mt9v111 = sd_to_mt9v111(sd);
> > +
> > + v4l2_async_unregister_subdev(sd);
> > +
> > + v4l2_ctrl_handler_free(&mt9v111->ctrls);
> > +
> > +#ifdef CONFIG_MEDIA_CONTROLLER
> > + media_entity_cleanup(&sd->entity);
> > +#endif
>
> and here...
>
> > +
> > + mutex_destroy(&mt9v111->pwr_mutex);
> > + mutex_destroy(&mt9v111->stream_mutex);
> > +
> > + devm_gpiod_put(mt9v111->dev, mt9v111->oe);
> > + devm_gpiod_put(mt9v111->dev, mt9v111->standby);
> > + devm_gpiod_put(mt9v111->dev, mt9v111->reset);
> > +
> > + devm_clk_put(mt9v111->dev, mt9v111->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id mt9v111_of_match[] = {
> > + { .compatible = "aptina,mt9v111", },
> > + { /* sentinel */ },
> > +};
> > +
> > +static struct i2c_driver mt9v111_driver = {
> > + .driver = {
> > + .name = "mt9v111",
> > + .of_match_table = of_match_ptr(mt9v111_of_match),
> > + },
> > + .probe_new = mt9v111_probe,
> > + .remove = mt9v111_remove,
> > +};
> > +
> > +module_i2c_driver(mt9v111_driver);
> > +
> > +MODULE_DESCRIPTION("V4L2 sensor driver for Aptina MT9V111");
> > +MODULE_AUTHOR("Jacopo Mondi <jacopo@jmondi.org>");
> > +MODULE_LICENSE("GPL v2");
>
>
> --
> Regards
> --
> Kieran
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v3 for 3.0 09/18] docker: disable debian-powerpc-user-cross
From: Alex Bennée @ 2018-07-23 15:09 UTC (permalink / raw)
To: cota, famz, berrange, f4bug, richard.henderson, balrogg, aurelien,
agraf
Cc: qemu-devel, David Gibson, qemu-ppc
In-Reply-To: <20180717195553.9111-10-alex.bennee@linaro.org>
Alex Bennée <alex.bennee@linaro.org> writes:
> While we can build we can't run the unit tests due to a missing system
> call (249 - swapcontext). This could be a code generation issue as
> other architectures don't seem to have this system call.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: qemu-ppc@nongnu.org
Dropping this patch now rth's setcontext fix is in.
> ---
> tests/docker/Makefile.include | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 09fb7db7fa..fe63aacf69 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -140,6 +140,9 @@ docker-binfmt-image-debian-powerpc-user: QEMU_TARGET = ppc-linux-user
> docker-binfmt-image-debian-powerpc-user: EXECUTABLE = ${BUILD_DIR}/ppc-linux-user/qemu-ppc
> docker-image-debian-powerpc-user-cross: docker-binfmt-image-debian-powerpc-user
> DOCKER_USER_IMAGES += debian-powerpc-user
> +# Unfortunalty while we can build with this image we can't run unit
> +# tests due to missing system call: 249 which causes an abort
> +DOCKER_PARTIAL_IMAGES += debian-powerpc-user-cross
>
> # Expand all the pre-requistes for each docker image and test combination
> $(foreach i,$(filter-out $(DOCKER_PARTIAL_IMAGES),$(DOCKER_IMAGES) $(DOCKER_DEPRECATED_IMAGES)), \
--
Alex Bennée
^ permalink raw reply
* [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support
From: Jassi Brar @ 2018-07-23 15:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180712070129.tyetqm2yesouiatf@pengutronix.de>
On Thu, Jul 12, 2018 at 12:31 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Thu, Jul 12, 2018 at 12:32:39AM +0800, Dong Aisheng wrote:
>> Add SCU protocol support in the generic mailbox driver.
>>
>> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
>> ---
>> drivers/mailbox/Kconfig | 6 +-
>> drivers/mailbox/imx-mailbox.c | 252 ++++++++++++++++++++++++++++++++++++------
>> 2 files changed, 223 insertions(+), 35 deletions(-)
>>
>> +static bool imx_mu_scu_peek_data(struct mbox_chan *chan)
>> +{
>> + struct imx_mu_priv *priv = chan->con_priv;
>> + u8 *raw_data;
>> + int i, size;
>> + int ret;
>> +
>> + ret = imx_mu_scu_receive_msg(chan, 0, priv->msg);
>> + if (ret)
>> + return false;
>> +
>> + raw_data = (u8 *)priv->msg;
>> + size = raw_data[1];
>> +
>> + dev_dbg(chan->mbox->dev, "receive data: hdr 0x%x size %d\n",
>> + *priv->msg, size);
>> +
>> + for (i = 1; i < size; i++) {
>> + ret = imx_mu_scu_receive_msg(chan, i % 4, priv->msg + i);
>> + if (ret)
>> + return false;
>> + }
>> +
>> + mbox_chan_received_data(chan, (void *)priv->msg);
>> +
>> + return true;
>> +}
>
> Receiving data now works by calling mbox_client_peek_data(). I think
> your plan is not to use rx_callback as triggered by mbox_chan_received_data,
> but instead exploiting the fact that you have put the receive data into
> the same buffer that you previously used to send a message. That way the
> client knows where to find the answer.
>
> While that should work I don't think this is very clean. That is the
> point where I mentioned I think that the mailbox framework would need a
> way to receive messages synchronously.
>
> Jassi, this is for you probably. Do you have an idea how synchronous
> receive could be implemented? We'll need some mbox_client_receive_data
> function.
>
Most platforms call mbox_chan_received_data() from RX interrupt
handler. Which is actually calling Client's rx_callback()
Or I miss your point?
^ permalink raw reply
* Re: cgroup-aware OOM killer, how to move forward
From: Tejun Heo @ 2018-07-23 15:09 UTC (permalink / raw)
To: Michal Hocko
Cc: Roman Gushchin, hannes, David Rientjes, linux-mm, akpm, gthelen
In-Reply-To: <20180723141748.GH31229@dhcp22.suse.cz>
Hello,
On Mon, Jul 23, 2018 at 04:17:48PM +0200, Michal Hocko wrote:
> I am not sure. If you are going to delegate then you are basically
> losing control of the group_oom at A-level. Is this good? What if I
> _want_ to tear down the whole thing if it starts misbehaving because I
> do not trust it?
>
> The more I think about it the more I am concluding that we should start
> with a more contrained model and require that once parent is
> group_oom == 1 then children have to as well. If we ever find a usecase
> to require a different scheme we can weaker it later. We cannot do that
> other way around.
>
> Tejun, Johannes what do you think about that?
I'd find the cgroup closest to the root which has the oom group set
and kill the entire subtree. There's no reason to put any
restrictions on what each cgroup can configure. The only thing which
matters is is that the effective behavior is what the highest in the
ancestry configures, and, at the system level, it'd conceptually map
to panic_on_oom.
Thanks.
--
tejun
^ permalink raw reply
* [PATCH v5 net-next] net/sched: add skbprio scheduler
From: Nishanth Devarajan @ 2018-07-23 14:07 UTC (permalink / raw)
To: xiyou.wangcong, jhs, jiri, davem; +Cc: netdev, doucette, michel
net/sched: add skbprio scheduler
Skbprio (SKB Priority Queue) is a queueing discipline that prioritizes packets
according to their skb->priority field. Under congestion, already-enqueued lower
priority packets will be dropped to make space available for higher priority
packets. Skbprio was conceived as a solution for denial-of-service defenses that
need to route packets with different priorities as a means to overcome DoS
attacks.
v5
*Do not reference qdisc_dev(sch)->tx_queue_len for setting limit. Instead set
default sch->limit to 64.
v4
*Drop Documentation/networking/sch_skbprio.txt doc file to move it to tc man
page for Skbprio, in iproute2.
v3
*Drop max_limit parameter in struct skbprio_sched_data and instead use
sch->limit.
*Reference qdisc_dev(sch)->tx_queue_len only once, during initialisation for
qdisc (previously being referenced every time qdisc changes).
*Move qdisc's detailed description from in-code to Documentation/networking.
*When qdisc is saturated, enqueue incoming packet first before dequeueing
lowest priority packet in queue - improves usage of call stack registers.
*Introduce and use overlimit stat to keep track of number of dropped packets.
v2
*Use skb->priority field rather than DS field. Rename queueing discipline as
SKB Priority Queue (previously Gatekeeper Priority Queue).
*Queueing discipline is made classful to expose Skbprio's internal priority
queues.
Signed-off-by: Nishanth Devarajan <ndev2021@gmail.com>
Reviewed-by: Sachin Paryani <sachin.paryani@gmail.com>
Reviewed-by: Cody Doucette <doucette@bu.edu>
Reviewed-by: Michel Machado <michel@digirati.com.br>
---
include/uapi/linux/pkt_sched.h | 15 ++
net/sched/Kconfig | 13 ++
net/sched/Makefile | 1 +
net/sched/sch_skbprio.c | 320 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 349 insertions(+)
create mode 100644 net/sched/sch_skbprio.c
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index d9cc9dc..8975fd1 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -124,6 +124,21 @@ struct tc_fifo_qopt {
__u32 limit; /* Queue length: bytes for bfifo, packets for pfifo */
};
+/* SKBPRIO section */
+
+/*
+ * Priorities go from zero to (SKBPRIO_MAX_PRIORITY - 1).
+ * SKBPRIO_MAX_PRIORITY should be at least 64 in order for skbprio to be able
+ * to map one to one the DS field of IPV4 and IPV6 headers.
+ * Memory allocation grows linearly with SKBPRIO_MAX_PRIORITY.
+ */
+
+#define SKBPRIO_MAX_PRIORITY 64
+
+struct tc_skbprio_qopt {
+ __u32 limit; /* Queue length in packets. */
+};
+
/* PRIO section */
#define TCQ_PRIO_BANDS 16
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 7af2467..7699344 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -251,6 +251,19 @@ config NET_SCH_MQPRIO
If unsure, say N.
+config NET_SCH_SKBPRIO
+ tristate "SKB priority queue scheduler (SKBPRIO)"
+ help
+ Say Y here if you want to use the SKB priority queue
+ scheduler. This schedules packets according to skb->priority,
+ which is useful for request packets in DoS mitigation systems such
+ as Gatekeeper.
+
+ To compile this driver as a module, choose M here: the module will
+ be called sch_skbprio.
+
+ If unsure, say N.
+
config NET_SCH_CHOKE
tristate "CHOose and Keep responsive flow scheduler (CHOKE)"
help
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 673ee7d..112ef70 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_NET_SCH_NETEM) += sch_netem.o
obj-$(CONFIG_NET_SCH_DRR) += sch_drr.o
obj-$(CONFIG_NET_SCH_PLUG) += sch_plug.o
obj-$(CONFIG_NET_SCH_MQPRIO) += sch_mqprio.o
+obj-$(CONFIG_NET_SCH_SKBPRIO) += sch_skbprio.o
obj-$(CONFIG_NET_SCH_CHOKE) += sch_choke.o
obj-$(CONFIG_NET_SCH_QFQ) += sch_qfq.o
obj-$(CONFIG_NET_SCH_CODEL) += sch_codel.o
diff --git a/net/sched/sch_skbprio.c b/net/sched/sch_skbprio.c
new file mode 100644
index 0000000..52c0b6d
--- /dev/null
+++ b/net/sched/sch_skbprio.c
@@ -0,0 +1,320 @@
+/*
+ * net/sched/sch_skbprio.c SKB Priority Queue.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Nishanth Devarajan, <ndev2021@gmail.com>
+ * Cody Doucette, <doucette@bu.edu>
+ * original idea by Michel Machado, Cody Doucette, and Qiaobin Fu
+ */
+
+#include <linux/string.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <net/pkt_sched.h>
+#include <net/sch_generic.h>
+#include <net/inet_ecn.h>
+
+/* SKB Priority Queue
+ * =================================
+ *
+ * Skbprio (SKB Priority Queue) is a queueing discipline that prioritizes
+ * packets according to their skb->priority field. Under congestion,
+ * Skbprio drops already-enqueued lower priority packets to make space
+ * available for higher priority packets; it was conceived as a solution
+ * for denial-of-service defenses that need to route packets with different
+ * priorities as a mean to overcome DoS attacks.
+ */
+
+struct skbprio_sched_data {
+ /* Queue state. */
+ struct sk_buff_head qdiscs[SKBPRIO_MAX_PRIORITY];
+ struct gnet_stats_queue qstats[SKBPRIO_MAX_PRIORITY];
+ u16 highest_prio;
+ u16 lowest_prio;
+};
+
+static u16 calc_new_high_prio(const struct skbprio_sched_data *q)
+{
+ int prio;
+
+ for (prio = q->highest_prio - 1; prio >= q->lowest_prio; prio--) {
+ if (!skb_queue_empty(&q->qdiscs[prio]))
+ return prio;
+ }
+
+ /* SKB queue is empty, return 0 (default highest priority setting). */
+ return 0;
+}
+
+static u16 calc_new_low_prio(const struct skbprio_sched_data *q)
+{
+ int prio;
+
+ for (prio = q->lowest_prio + 1; prio <= q->highest_prio; prio++) {
+ if (!skb_queue_empty(&q->qdiscs[prio]))
+ return prio;
+ }
+
+ /* SKB queue is empty, return SKBPRIO_MAX_PRIORITY - 1
+ * (default lowest priority setting).
+ */
+ return SKBPRIO_MAX_PRIORITY - 1;
+}
+
+static int skbprio_enqueue(struct sk_buff *skb, struct Qdisc *sch,
+ struct sk_buff **to_free)
+{
+ const unsigned int max_priority = SKBPRIO_MAX_PRIORITY - 1;
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ struct sk_buff_head *qdisc;
+ struct sk_buff_head *lp_qdisc;
+ struct sk_buff *to_drop;
+ u16 prio, lp;
+
+ /* Obtain the priority of @skb. */
+ prio = min(skb->priority, max_priority);
+
+ qdisc = &q->qdiscs[prio];
+ if (sch->q.qlen < sch->limit) {
+ __skb_queue_tail(qdisc, skb);
+ qdisc_qstats_backlog_inc(sch, skb);
+ q->qstats[prio].backlog += qdisc_pkt_len(skb);
+
+ /* Check to update highest and lowest priorities. */
+ if (prio > q->highest_prio)
+ q->highest_prio = prio;
+
+ if (prio < q->lowest_prio)
+ q->lowest_prio = prio;
+
+ sch->q.qlen++;
+ return NET_XMIT_SUCCESS;
+ }
+
+ /* If this packet has the lowest priority, drop it. */
+ lp = q->lowest_prio;
+ if (prio <= lp) {
+ q->qstats[prio].drops++;
+ q->qstats[prio].overlimits++;
+ return qdisc_drop(skb, sch, to_free);
+ }
+
+ __skb_queue_tail(qdisc, skb);
+ qdisc_qstats_backlog_inc(sch, skb);
+ q->qstats[prio].backlog += qdisc_pkt_len(skb);
+
+ /* Drop the packet at the tail of the lowest priority qdisc. */
+ lp_qdisc = &q->qdiscs[lp];
+ to_drop = __skb_dequeue_tail(lp_qdisc);
+ BUG_ON(!to_drop);
+ qdisc_qstats_backlog_dec(sch, to_drop);
+ qdisc_drop(to_drop, sch, to_free);
+
+ q->qstats[lp].backlog -= qdisc_pkt_len(to_drop);
+ q->qstats[lp].drops++;
+ q->qstats[lp].overlimits++;
+
+ /* Check to update highest and lowest priorities. */
+ if (skb_queue_empty(lp_qdisc)) {
+ if (q->lowest_prio == q->highest_prio) {
+ /* The incoming packet is the only packet in queue. */
+ BUG_ON(sch->q.qlen != 1);
+ q->lowest_prio = prio;
+ q->highest_prio = prio;
+ } else {
+ q->lowest_prio = calc_new_low_prio(q);
+ }
+ }
+
+ if (prio > q->highest_prio)
+ q->highest_prio = prio;
+
+ return NET_XMIT_CN;
+}
+
+static struct sk_buff *skbprio_dequeue(struct Qdisc *sch)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ struct sk_buff_head *hpq = &q->qdiscs[q->highest_prio];
+ struct sk_buff *skb = __skb_dequeue(hpq);
+
+ if (unlikely(!skb))
+ return NULL;
+
+ sch->q.qlen--;
+ qdisc_qstats_backlog_dec(sch, skb);
+ qdisc_bstats_update(sch, skb);
+
+ q->qstats[q->highest_prio].backlog -= qdisc_pkt_len(skb);
+
+ /* Update highest priority field. */
+ if (skb_queue_empty(hpq)) {
+ if (q->lowest_prio == q->highest_prio) {
+ BUG_ON(sch->q.qlen);
+ q->highest_prio = 0;
+ q->lowest_prio = SKBPRIO_MAX_PRIORITY - 1;
+ } else {
+ q->highest_prio = calc_new_high_prio(q);
+ }
+ }
+ return skb;
+}
+
+static int skbprio_change(struct Qdisc *sch, struct nlattr *opt,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_skbprio_qopt *ctl = nla_data(opt);
+
+ sch->limit = ctl->limit;
+ return 0;
+}
+
+static int skbprio_init(struct Qdisc *sch, struct nlattr *opt,
+ struct netlink_ext_ack *extack)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ int prio;
+
+ /* Initialise all queues, one for each possible priority. */
+ for (prio = 0; prio < SKBPRIO_MAX_PRIORITY; prio++)
+ __skb_queue_head_init(&q->qdiscs[prio]);
+
+ memset(&q->qstats, 0, sizeof(q->qstats));
+ q->highest_prio = 0;
+ q->lowest_prio = SKBPRIO_MAX_PRIORITY - 1;
+ sch->limit = 64;
+ if (!opt)
+ return 0;
+
+ return skbprio_change(sch, opt, extack);
+}
+
+static int skbprio_dump(struct Qdisc *sch, struct sk_buff *skb)
+{
+ struct tc_skbprio_qopt opt;
+
+ opt.limit = sch->limit;
+
+ if (nla_put(skb, TCA_OPTIONS, sizeof(opt), &opt))
+ return -1;
+
+ return skb->len;
+}
+
+static void skbprio_reset(struct Qdisc *sch)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ int prio;
+
+ sch->qstats.backlog = 0;
+ sch->q.qlen = 0;
+
+ for (prio = 0; prio < SKBPRIO_MAX_PRIORITY; prio++)
+ __skb_queue_purge(&q->qdiscs[prio]);
+
+ memset(&q->qstats, 0, sizeof(q->qstats));
+ q->highest_prio = 0;
+ q->lowest_prio = SKBPRIO_MAX_PRIORITY - 1;
+}
+
+static void skbprio_destroy(struct Qdisc *sch)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ int prio;
+
+ for (prio = 0; prio < SKBPRIO_MAX_PRIORITY; prio++)
+ __skb_queue_purge(&q->qdiscs[prio]);
+}
+
+static struct Qdisc *skbprio_leaf(struct Qdisc *sch, unsigned long arg)
+{
+ return NULL;
+}
+
+static unsigned long skbprio_find(struct Qdisc *sch, u32 classid)
+{
+ return 0;
+}
+
+static int skbprio_dump_class(struct Qdisc *sch, unsigned long cl,
+ struct sk_buff *skb, struct tcmsg *tcm)
+{
+ tcm->tcm_handle |= TC_H_MIN(cl);
+ return 0;
+}
+
+static int skbprio_dump_class_stats(struct Qdisc *sch, unsigned long cl,
+ struct gnet_dump *d)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ if (gnet_stats_copy_queue(d, NULL, &q->qstats[cl - 1],
+ q->qstats[cl - 1].qlen) < 0)
+ return -1;
+ return 0;
+}
+
+static void skbprio_walk(struct Qdisc *sch, struct qdisc_walker *arg)
+{
+ unsigned int i;
+
+ if (arg->stop)
+ return;
+
+ for (i = 0; i < SKBPRIO_MAX_PRIORITY; i++) {
+ if (arg->count < arg->skip) {
+ arg->count++;
+ continue;
+ }
+ if (arg->fn(sch, i + 1, arg) < 0) {
+ arg->stop = 1;
+ break;
+ }
+ arg->count++;
+ }
+}
+
+static const struct Qdisc_class_ops skbprio_class_ops = {
+ .leaf = skbprio_leaf,
+ .find = skbprio_find,
+ .dump = skbprio_dump_class,
+ .dump_stats = skbprio_dump_class_stats,
+ .walk = skbprio_walk,
+};
+
+static struct Qdisc_ops skbprio_qdisc_ops __read_mostly = {
+ .cl_ops = &skbprio_class_ops,
+ .id = "skbprio",
+ .priv_size = sizeof(struct skbprio_sched_data),
+ .enqueue = skbprio_enqueue,
+ .dequeue = skbprio_dequeue,
+ .peek = qdisc_peek_dequeued,
+ .init = skbprio_init,
+ .reset = skbprio_reset,
+ .change = skbprio_change,
+ .dump = skbprio_dump,
+ .destroy = skbprio_destroy,
+ .owner = THIS_MODULE,
+};
+
+static int __init skbprio_module_init(void)
+{
+ return register_qdisc(&skbprio_qdisc_ops);
+}
+
+static void __exit skbprio_module_exit(void)
+{
+ unregister_qdisc(&skbprio_qdisc_ops);
+}
+
+module_init(skbprio_module_init)
+module_exit(skbprio_module_exit)
+
+MODULE_LICENSE("GPL");
--
1.9.1
^ permalink raw reply related
* [PATCH 5/5] powerpc/powernv: Query firmware for count cache flush settings
From: Michael Ellerman @ 2018-07-23 15:07 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20180723150756.11108-1-mpe@ellerman.id.au>
Look for fw-features properties to determine the appropriate settings
for the count cache flush, and then call the generic powerpc code to
set it up based on the security feature flags.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/platforms/powernv/setup.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index f96df0a25d05..0988d050becd 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -78,6 +78,12 @@ static void init_fw_feat_flags(struct device_node *np)
if (fw_feature_is("enabled", "fw-count-cache-disabled", np))
security_ftr_set(SEC_FTR_COUNT_CACHE_DISABLED);
+ if (fw_feature_is("enabled", "fw-count-cache-flush-bcctr2,0,0", np))
+ security_ftr_set(SEC_FTR_BCCTR_FLUSH_ASSIST);
+
+ if (fw_feature_is("enabled", "needs-count-cache-flush-on-context-switch", np))
+ security_ftr_set(SEC_FTR_FLUSH_COUNT_CACHE);
+
/*
* The features below are enabled by default, so we instead look to see
* if firmware has *disabled* them, and clear them if so.
@@ -125,6 +131,7 @@ static void pnv_setup_rfi_flush(void)
setup_rfi_flush(type, enable);
setup_barrier_nospec();
+ setup_count_cache_flush();
}
static void __init pnv_setup_arch(void)
--
2.14.1
^ permalink raw reply related
* [PATCH 4/5] powerpc/pseries: Query hypervisor for count cache flush settings
From: Michael Ellerman @ 2018-07-23 15:07 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20180723150756.11108-1-mpe@ellerman.id.au>
Use the existing hypercall to determine the appropriate settings for
the count cache flush, and then call the generic powerpc code to set
it up based on the security feature flags.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/hvcall.h | 2 ++
arch/powerpc/platforms/pseries/setup.c | 7 +++++++
2 files changed, 9 insertions(+)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 662c8347d699..a0b17f9f1ea4 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -342,10 +342,12 @@
#define H_CPU_CHAR_BRANCH_HINTS_HONORED (1ull << 58) // IBM bit 5
#define H_CPU_CHAR_THREAD_RECONFIG_CTRL (1ull << 57) // IBM bit 6
#define H_CPU_CHAR_COUNT_CACHE_DISABLED (1ull << 56) // IBM bit 7
+#define H_CPU_CHAR_BCCTR_FLUSH_ASSIST (1ull << 54) // IBM bit 9
#define H_CPU_BEHAV_FAVOUR_SECURITY (1ull << 63) // IBM bit 0
#define H_CPU_BEHAV_L1D_FLUSH_PR (1ull << 62) // IBM bit 1
#define H_CPU_BEHAV_BNDS_CHK_SPEC_BAR (1ull << 61) // IBM bit 2
+#define H_CPU_BEHAV_FLUSH_COUNT_CACHE (1ull << 58) // IBM bit 5
/* Flag values used in H_REGISTER_PROC_TBL hcall */
#define PROC_TABLE_OP_MASK 0x18
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 139f0af6c3d9..04805a79cbda 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -484,6 +484,12 @@ static void init_cpu_char_feature_flags(struct h_cpu_char_result *result)
if (result->character & H_CPU_CHAR_COUNT_CACHE_DISABLED)
security_ftr_set(SEC_FTR_COUNT_CACHE_DISABLED);
+ if (result->character & H_CPU_CHAR_BCCTR_FLUSH_ASSIST)
+ security_ftr_set(SEC_FTR_BCCTR_FLUSH_ASSIST);
+
+ if (result->behaviour & H_CPU_BEHAV_FLUSH_COUNT_CACHE)
+ security_ftr_set(SEC_FTR_FLUSH_COUNT_CACHE);
+
/*
* The features below are enabled by default, so we instead look to see
* if firmware has *disabled* them, and clear them if so.
@@ -535,6 +541,7 @@ void pseries_setup_rfi_flush(void)
setup_rfi_flush(types, enable);
setup_barrier_nospec();
+ setup_count_cache_flush();
}
#ifdef CONFIG_PCI_IOV
--
2.14.1
^ permalink raw reply related
* [PATCH 3/5] powerpc/64s: Add support for software count cache flush
From: Michael Ellerman @ 2018-07-23 15:07 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20180723150756.11108-1-mpe@ellerman.id.au>
Some CPU revisions support a mode where the count cache needs to be
flushed by software on context switch. Additionally some revisions may
have a hardware accelerated flush, in which case the software flush
sequence can be shortened.
If we detect the appropriate flag from firmware we patch a branch
into _switch() which takes us to a count cache flush sequence.
That sequence in turn may be patched to return early if we detect that
the CPU supports accelerating the flush sequence in hardware.
Add debugfs support for reporting the state of the flush, as well as
runtime disabling it.
And modify the spectre_v2 sysfs file to report the state of the
software flush.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/asm-prototypes.h | 6 ++
arch/powerpc/include/asm/security_features.h | 1 +
arch/powerpc/kernel/entry_64.S | 54 ++++++++++++++++
arch/powerpc/kernel/security.c | 96 ++++++++++++++++++++++++++--
4 files changed, 152 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 769567b66c0c..70fdc5b9b9fb 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -143,4 +143,10 @@ struct kvm_vcpu;
void _kvmppc_restore_tm_pr(struct kvm_vcpu *vcpu, u64 guest_msr);
void _kvmppc_save_tm_pr(struct kvm_vcpu *vcpu, u64 guest_msr);
+/* Patch sites */
+extern s32 patch__call_flush_count_cache;
+extern s32 patch__flush_count_cache_return;
+
+extern long flush_count_cache;
+
#endif /* _ASM_POWERPC_ASM_PROTOTYPES_H */
diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
index a0d47bc18a5c..759597bf0fd8 100644
--- a/arch/powerpc/include/asm/security_features.h
+++ b/arch/powerpc/include/asm/security_features.h
@@ -22,6 +22,7 @@ enum stf_barrier_type {
void setup_stf_barrier(void);
void do_stf_barrier_fixups(enum stf_barrier_type types);
+void setup_count_cache_flush(void);
static inline void security_ftr_set(unsigned long feature)
{
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 0357f87a013c..017cf70f01d7 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -25,6 +25,7 @@
#include <asm/page.h>
#include <asm/mmu.h>
#include <asm/thread_info.h>
+#include <asm/code-patching-asm.h>
#include <asm/ppc_asm.h>
#include <asm/asm-offsets.h>
#include <asm/cputable.h>
@@ -504,6 +505,57 @@ _GLOBAL(ret_from_kernel_thread)
li r3,0
b .Lsyscall_exit
+#ifdef CONFIG_PPC_BOOK3S_64
+
+#define FLUSH_COUNT_CACHE \
+1: nop; \
+ patch_site 1b, patch__call_flush_count_cache
+
+
+#define BCCTR_FLUSH .long 0x4c400420
+
+.macro nops number
+ .rept \number
+ nop
+ .endr
+.endm
+
+.balign 32
+.global flush_count_cache
+flush_count_cache:
+ /* Save LR into r9 */
+ mflr r9
+
+ .rept 64
+ bl .+4
+ .endr
+ b 1f
+ nops 6
+
+ .balign 32
+ /* Restore LR */
+1: mtlr r9
+ li r9,0x7fff
+ mtctr r9
+
+ BCCTR_FLUSH
+
+2: nop
+ patch_site 2b patch__flush_count_cache_return
+
+ nops 3
+
+ .rept 278
+ .balign 32
+ BCCTR_FLUSH
+ nops 7
+ .endr
+
+ blr
+#else
+#define FLUSH_COUNT_CACHE
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
/*
* This routine switches between two different tasks. The process
* state of one is saved on its kernel stack. Then the state
@@ -535,6 +587,8 @@ _GLOBAL(_switch)
std r23,_CCR(r1)
std r1,KSP(r3) /* Set old stack pointer */
+ FLUSH_COUNT_CACHE
+
/*
* On SMP kernels, care must be taken because a task may be
* scheduled off CPUx and on to CPUy. Memory ordering must be
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 4cb8f1f7b593..fa9366b53eb7 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -8,6 +8,8 @@
#include <linux/device.h>
#include <linux/seq_buf.h>
+#include <asm/asm-prototypes.h>
+#include <asm/code-patching.h>
#include <asm/debugfs.h>
#include <asm/security_features.h>
#include <asm/setup.h>
@@ -15,6 +17,13 @@
unsigned long powerpc_security_features __read_mostly = SEC_FTR_DEFAULT;
+enum count_cache_flush_type {
+ COUNT_CACHE_FLUSH_NONE = 0x1,
+ COUNT_CACHE_FLUSH_SW = 0x2,
+ COUNT_CACHE_FLUSH_HW = 0x4,
+};
+static enum count_cache_flush_type count_cache_flush_type;
+
bool barrier_nospec_enabled;
static void enable_barrier_nospec(bool enable)
@@ -147,17 +156,29 @@ ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, c
bcs = security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED);
ccd = security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED);
- if (bcs || ccd) {
+ if (bcs || ccd || count_cache_flush_type != COUNT_CACHE_FLUSH_NONE) {
+ bool comma = false;
seq_buf_printf(&s, "Mitigation: ");
- if (bcs)
+ if (bcs) {
seq_buf_printf(&s, "Indirect branch serialisation (kernel only)");
+ comma = true;
+ }
+
+ if (ccd) {
+ if (comma)
+ seq_buf_printf(&s, ", ");
+ seq_buf_printf(&s, "Indirect branch cache disabled");
+ comma = true;
+ }
- if (bcs && ccd)
+ if (comma)
seq_buf_printf(&s, ", ");
- if (ccd)
- seq_buf_printf(&s, "Indirect branch cache disabled");
+ seq_buf_printf(&s, "Software count cache flush");
+
+ if (count_cache_flush_type == COUNT_CACHE_FLUSH_HW)
+ seq_buf_printf(&s, "(hardware accelerated)");
} else
seq_buf_printf(&s, "Vulnerable");
@@ -313,3 +334,68 @@ static __init int stf_barrier_debugfs_init(void)
}
device_initcall(stf_barrier_debugfs_init);
#endif /* CONFIG_DEBUG_FS */
+
+static void toggle_count_cache_flush(bool enable)
+{
+ if (!enable || !security_ftr_enabled(SEC_FTR_FLUSH_COUNT_CACHE)) {
+ patch_instruction_site(&patch__call_flush_count_cache, PPC_INST_NOP);
+ count_cache_flush_type = COUNT_CACHE_FLUSH_NONE;
+ pr_info("count-cache-flush: software flush disabled.\n");
+ return;
+ }
+
+ patch_branch_site(&patch__call_flush_count_cache,
+ (u64)&flush_count_cache, BRANCH_SET_LINK);
+
+ if (!security_ftr_enabled(SEC_FTR_BCCTR_FLUSH_ASSIST)) {
+ count_cache_flush_type = COUNT_CACHE_FLUSH_SW;
+ pr_info("count-cache-flush: full software flush sequence enabled.\n");
+ return;
+ }
+
+ patch_instruction_site(&patch__flush_count_cache_return, PPC_INST_BLR);
+ count_cache_flush_type = COUNT_CACHE_FLUSH_HW;
+ pr_info("count-cache-flush: hardware assisted flush sequence enabled\n");
+}
+
+void setup_count_cache_flush(void)
+{
+ toggle_count_cache_flush(true);
+}
+
+#ifdef CONFIG_DEBUG_FS
+static int count_cache_flush_set(void *data, u64 val)
+{
+ bool enable;
+
+ if (val == 1)
+ enable = true;
+ else if (val == 0)
+ enable = false;
+ else
+ return -EINVAL;
+
+ toggle_count_cache_flush(enable);
+
+ return 0;
+}
+
+static int count_cache_flush_get(void *data, u64 *val)
+{
+ if (count_cache_flush_type == COUNT_CACHE_FLUSH_NONE)
+ *val = 0;
+ else
+ *val = 1;
+
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(fops_count_cache_flush, count_cache_flush_get, count_cache_flush_set, "%llu\n");
+
+static __init int count_cache_flush_debugfs_init(void)
+{
+ debugfs_create_file("count_cache_flush", 0600, powerpc_debugfs_root, NULL, &fops_count_cache_flush);
+ return 0;
+}
+device_initcall(count_cache_flush_debugfs_init);
+#endif /* CONFIG_DEBUG_FS */
--
2.14.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.