* Re: [PATCH 0/6] drm+dma: cache support for arm, etc
From: Koenig, Christian @ 2019-08-15 18:21 UTC (permalink / raw)
To: Christoph Hellwig, Rob Clark
Cc: Kate Stewart, Masayoshi Mizuma, Maciej W. Rozycki, Eric Biggers,
Catalin Marinas, Imre Deak, dri-devel, Chris Wilson,
Masahiro Yamada, Benjamin Gaignard, Mauro Carvalho Chehab,
Will Deacon, Emil Velikov, Sharma, Deepak, Michael Ellerman,
Paul Burton, Mike Rapoport, Geert Uytterhoeven,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), Daniel Vetter,
open list:MIPS, Linus Walleij, Robin Murphy,
open list:DRM DRIVER FOR MSM ADRENO GPU, Joerg Roedel,
Arnd Bergmann, Anshuman Khandual, Hauke Mehrtens,
Jesper Dangaard Brouer, Wolfram Sang (Renesas),
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Alexios Zavras,
Russell King, Doug Anderson, Thomas Gleixner, Sean Paul,
Allison Randal, Christophe Leroy, Enrico Weigelt, Ard Biesheuvel,
Greg Kroah-Hartman, open list, Rob Clark, Souptick Joarder,
Andrew Morton, open list:DRM DRIVER FOR MSM ADRENO GPU
In-Reply-To: <20190815175346.GA19839@lst.de>
Am 15.08.19 um 19:53 schrieb Christoph Hellwig:
> On Thu, Aug 15, 2019 at 06:54:39AM -0700, Rob Clark wrote:
>> On Wed, Aug 14, 2019 at 11:51 PM Christoph Hellwig <hch@lst.de> wrote:
>>> As said before I don't think these low-level helpers are the
>>> right API to export, but even if they did you'd just cover a tiny
>>> subset of the architectures.
>> Are you thinking instead something like:
>>
>> void dma_sync_sg_for_{cpu,device}(struct device *dev, struct scatterlist *sgl,
>> int nents, enum dma_data_direction dir)
>> {
>> for_each_sg(sgl, sg, nents, i) {
>> arch_sync_dma_for_..(dev, sg_phys(sg), sg->length, dir);
>> }
>> }
>> EXPORT_SYMBOL_GPL(dma_sync_sg_for_..)
>>
>> or did you have something else in mind?
> No. We really need an interface thay says please give me uncached
> memory (for some definition of uncached that includes that grapics
> drivers call write combine), and then let the architecture do the right
> thing. Basically dma_alloc_coherent with DMA_ATTR_NO_KERNEL_MAPPING
> is superficially close to what you want, except that the way the drm
> drivers work you can't actually use it.
The terminology graphics driver use is USWC (Uncached Speculative Write
Combine).
Essentially it is a leftover from the AGP days where the graphics
devices couldn't snoop the CPU caches.
Nowadays they either don't want to snoop the CPU caches for performance
reasons.
Or because of special requirements that certain areas of system memory
are not cached for certain functionality.
For example the "VRAM" on AMD GPUs which are integrated into the CPU is
just stolen system memory. Then you can scanout your picture to the
display from this memory or "normal" system memory, but only if the
system memory is mapped as USWC.
> The reason for that is if we can we really need to not create another
> uncachable alias, but instead change the page attributes in place.
> On x86 we can and must do that for example, and based on the
> conversation with Will arm64 could do that fairly easily. arm32 can
> right now only do that for CMA, though.
>
> The big question is what API do we want. I had a pretty similar
> discussion with Christian on doing such an allocation for amdgpu,
> where the device normally is cache coherent, but they actually want
> to turn it into non-coherent by using PCIe unsnooped transactions.
>
> Here is my high level plan, which still has a few lose end:
>
> (1) provide a new API:
>
> struct page *dma_alloc_pages(struct device *dev, unsigned nr_pages,
> gfp_t gfp, unsigned long flags);
> void dma_free_pages(struct device *dev, unsigned nr_pages,
> unsigned long flags);
>
> These give you back page backed memory that is guaranteed to be
> addressable by the device (no swiotlb or similar). The memory can
> then be mapped using dma_map*, including unmap and dma_sync to
> bounce ownership around. This is the replacement for the current
> dma_alloc_attrs with DMA_ATTR_NON_CONSISTENT API, that is rather
> badly defined.
>
> (2) Add support for DMA_ATTR_NO_KERNEL_MAPPING to this new API instead
> of dma_alloc_attrs. The initial difference with that flag is just
> that we allow highmem, but in the future we could also unmap this
> memory from the kernel linear mapping entirely on architectures
> where we can easily do that.
Mhm, why would we want to do this?
>
> (3) Add a dma_pages_map/dma_pages_unmap or similar API that allows you
> to get a kernel mapping for parts or all of a
> DMA_ATTR_NO_KERNEL_MAPPING allocation. This is to replace things
> like your open-coded vmap in msm (or similarly elsewhere in dma-buf
> providers).
>
> (4) Add support for a DMA_ATTR_UNCACHABLE flags (or similar) to the new
> API, that maps the pages as uncachable iff they have a kernel
> mapping, including invalidating the caches at time of this page
> attribute change (or creation of a new mapping). This API will fail
> if the architecture does not allow in-place remapping. Note that for
> arm32 we could always dip into the CMA pool if one is present to not
> fail. We'll also need some helper to map from the DMA_ATTR_* flags
> to a pgprot for mapping the page to userspace. There is also a few
> other weird bits here, e.g. on architectures like mips that use an
> uncached segment we'll have to fail use with the plain
> DMA_ATTR_UNCACHABLE flag, but it could be supported with
> DMA_ATTR_UNCACHABLE | DMA_ATTR_NO_KERNEL_MAPPING.
>
> I was hoping to get most of this done for this merge window, but I'm
> probably lucky if I get at least parts done. Too much distraction.
Thanks again for taking care of this,
Christian.
>
>> Hmm, not entirely sure why.. you should be on the cc list for each
>> individual patch.
> They finally made it, although even with the delay they only ended up
> in the spam mailbox. I still can't see them on the various mailing
> lists.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 1/9] soc: samsung: Add exynos chipid driver support
From: Krzysztof Kozlowski @ 2019-08-15 18:18 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: devicetree, linux-samsung-soc, linux-pm, pankaj.dubey,
b.zolnierkie, linux-kernel, robh+dt, kgene, vireshk,
linux-arm-kernel, m.szyprowski
In-Reply-To: <20190813150827.31972-2-s.nawrocki@samsung.com>
On Tue, Aug 13, 2019 at 05:08:19PM +0200, Sylwester Nawrocki wrote:
> From: Pankaj Dubey <pankaj.dubey@samsung.com>
>
> Exynos SoCs have Chipid, for identification of product IDs and SoC
> revisions. This patch intends to provide initialization code for all
> these functionalities, at the same time it provides some sysfs entries
> for accessing these information to user-space.
>
Thanks, applied.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 11/14] dt-bindings: Document the Rockchip MIPI RX D-PHY bindings
From: Laurent Pinchart @ 2019-08-15 18:14 UTC (permalink / raw)
To: Helen Koike
Cc: devicetree, eddie.cai.linux, kernel, heiko, Rob Herring,
jacob2.chen, jeffy.chen, zyc, linux-kernel, tfiga, linux-rockchip,
hans.verkuil, sakari.ailus, zhengsq, mchehab, ezequiel,
linux-arm-kernel, linux-media
In-Reply-To: <20190730184256.30338-12-helen.koike@collabora.com>
Hi Helen,
Thank you for the patch.
On Tue, Jul 30, 2019 at 03:42:53PM -0300, Helen Koike wrote:
> From: Jacob Chen <jacob2.chen@rock-chips.com>
>
> Add DT bindings documentation for Rockchip MIPI D-PHY RX
>
> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> [update for upstream]
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>
> ---
>
> Changes in v8: None
> Changes in v7:
> - updated doc with new design and tested example
>
> .../bindings/media/rockchip-mipi-dphy.txt | 38 +++++++++++++++++++
Shouldn't this go to bindings/phy/ ?
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
>
> diff --git a/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt b/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
> new file mode 100644
> index 000000000000..2305d44d92db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
> @@ -0,0 +1,38 @@
> +Rockchip SoC MIPI RX D-PHY
> +-------------------------------------------------------------
How about already converting the bindings to yaml ? There's one example
in bindings/phy/ that looks quite similar to what you need here. Make
sure to have a look at Documentation/devicetree/writing-schema.md, and
in particular to run make dt_binding_check.
> +
> +Required properties:
> +- compatible: value should be one of the following
> + "rockchip,rk3288-mipi-dphy"
> + "rockchip,rk3399-mipi-dphy"
> +- clocks : list of clock specifiers, corresponding to entries in
> + clock-names property;
> +- clock-names: required clock name.
> +- #phy-cells: Number of cells in a PHY specifier; Should be 0.
> +
> +MIPI RX D-PHY use registers in "general register files", it
> +should be a child of the GRF.
> +
> +Optional properties:
> +- reg: offset and length of the register set for the device.
> +- rockchip,grf: MIPI TX1RX1 D-PHY not only has its own register but also
> + the GRF, so it is only necessary for MIPI TX1RX1 D-PHY.
> +
> +Device node example
> +-------------------
> +
> +grf: syscon@ff770000 {
> + compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
> +
> +...
> +
> + dphy: mipi-dphy {
> + compatible = "rockchip,rk3399-mipi-dphy";
> + clocks = <&cru SCLK_MIPIDPHY_REF>,
> + <&cru SCLK_DPHY_RX0_CFG>,
> + <&cru PCLK_VIO_GRF>;
> + clock-names = "dphy-ref", "dphy-cfg", "grf";
> + power-domains = <&power RK3399_PD_VIO>;
> + #phy-cells = <0>;
> + };
> +};
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 2/8] arm64, mm: transitional tables
From: James Morse @ 2019-08-15 18:11 UTC (permalink / raw)
To: Pavel Tatashin
Cc: sashal, vladimir.murzin, corbet, marc.zyngier, catalin.marinas,
bhsharma, kexec, linux-kernel, jmorris, linux-mm, ebiederm,
matthias.bgg, will, linux-arm-kernel
In-Reply-To: <20190801152439.11363-3-pasha.tatashin@soleen.com>
Hi Pavel,
On 01/08/2019 16:24, Pavel Tatashin wrote:
> There are cases where normal kernel pages tables, i.e. idmap_pg_dir
> and swapper_pg_dir are not sufficient because they may be overwritten.
>
> This happens when we transition from one world to another: for example
> during kexec kernel relocation transition, and also during hibernate
> kernel restore transition.
>
> In these cases, if MMU is needed, the page table memory must be allocated
> from a safe place. Transitional tables is intended to allow just that.
> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
> index db92950bb1a0..dcb4f13c7888 100644
> --- a/arch/arm64/include/asm/pgtable-hwdef.h
> +++ b/arch/arm64/include/asm/pgtable-hwdef.h
> @@ -110,6 +110,7 @@
> #define PUD_TABLE_BIT (_AT(pudval_t, 1) << 1)
> #define PUD_TYPE_MASK (_AT(pudval_t, 3) << 0)
> #define PUD_TYPE_SECT (_AT(pudval_t, 1) << 0)
> +#define PUD_SECT_RDONLY (_AT(pudval_t, 1) << 7) /* AP[2] */
This shouldn't be needed. As far as I'm aware, we only get read-only pages in the linear
map from debug-pagealloc, and the module aliases. Both of which require the linear map to
be made of page-size mappings.
Where are you seeing these?
> diff --git a/arch/arm64/include/asm/trans_table.h b/arch/arm64/include/asm/trans_table.h
> new file mode 100644
> index 000000000000..c7aef70587a1
> --- /dev/null
> +++ b/arch/arm64/include/asm/trans_table.h
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (c) 2019, Microsoft Corporation.
> + * Pavel Tatashin <patatash@linux.microsoft.com>
> + */
> +
> +#ifndef _ASM_TRANS_TABLE_H
> +#define _ASM_TRANS_TABLE_H
> +
> +#include <linux/bits.h>
> +#include <asm/pgtable-types.h>
> +
> +/*
> + * trans_alloc_page
> + * - Allocator that should return exactly one uninitilaized page, if this
> + * allocator fails, trans_table returns -ENOMEM error.
> + *
> + * trans_alloc_arg
> + * - Passed to trans_alloc_page as an argument
> + *
> + * trans_flags
> + * - bitmap with flags that control how page table is filled.
> + * TRANS_MKWRITE: during page table copy make PTE, PME, and PUD page
> + * writeable by removing RDONLY flag from PTE.
> + * TRANS_MKVALID: during page table copy, if PTE present, but not valid,
> + * make it valid.
> + * TRANS_CHECKPFN: During page table copy, for every PTE entry check that
> + * PFN that this PTE points to is valid. Otherwise return
> + * -ENXIO
Adding top-level global knobs to manipulate the copied linear map is going to lead to
bugs. The existing code will only change the PTE in specific circumstances, that it tests
for, that only happen at the PTE level.
> + * TRANS_FORCEMAP: During page map, if translation exists, force
> + * overwrite it. Otherwise -ENXIO may be returned by
> + * trans_table_map_* functions if conflict is detected.
This one, sounds like a very bad idea.
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 0/8] arm64: MMU enabled kexec relocation
From: James Morse @ 2019-08-15 18:11 UTC (permalink / raw)
To: Pavel Tatashin
Cc: Sasha Levin, Vladimir Murzin, Jonathan Corbet, Marc Zyngier,
Catalin Marinas, Bhupesh Sharma, kexec mailing list, LKML,
James Morris, linux-mm, Eric W. Biederman, Matthias Brugger, will,
Linux ARM
In-Reply-To: <CA+CK2bADiBMEx9cJuXT5fQkBYFZAtxUtc7ZzjrNfEjijPZkPtw@mail.gmail.com>
Hi Pavel,
On 08/08/2019 19:44, Pavel Tatashin wrote:
> Just a friendly reminder, please send your comments on this series.
(Please don't top-post)
> It's been a week since I sent out these patches, and no feedback yet.
A week is not a lot of time, people are busy, go to conferences, some even dare to take
holiday!
> Also, I'd appreciate if anyone could test this series on vhe hardware
> with vhe kernel, it does not look like QEMU can emulate it yet
This locks up during resume from hibernate on my AMD Seattle, a regular v8.0 machine.
Please try and build the series to reduce review time. What you have here is an all-new
page-table generation API, which you switch hibernate and kexec too. This is effectively a
new implementation of hibernate and kexec. There are three things here that need review.
You have a regression in your all-new implementation of hibernate. It took six months (and
lots of review) to get the existing code right, please don't rip it out if there is
nothing wrong with it.
Instead, please just move the hibernate copy_page_tables() code, and then wire kexec up.
You shouldn't need to change anything in the copy_page_tables() code as the linear map is
the same in both cases.
It looks like you are creating the page tables just after the kexec:segments have been
loaded. This will go horribly wrong if anything changes between then and kexec time. (e.g.
memory you've got mapped gets hot-removed).
This needs to be done as late as possible, so we don't waste memory, and the world can't
change around us. Reboot notifiers run before kexec, can't we do the memory-allocation there?
> On Thu, Aug 1, 2019 at 11:24 AM Pavel Tatashin
> <pasha.tatashin@soleen.com> wrote:
>>
>> Enable MMU during kexec relocation in order to improve reboot performance.
>>
>> If kexec functionality is used for a fast system update, with a minimal
>> downtime, the relocation of kernel + initramfs takes a significant portion
>> of reboot.
>>
>> The reason for slow relocation is because it is done without MMU, and thus
>> not benefiting from D-Cache.
>>
>> Performance data
>> ----------------
>> For this experiment, the size of kernel plus initramfs is small, only 25M.
>> If initramfs was larger, than the improvements would be greater, as time
>> spent in relocation is proportional to the size of relocation.
>>
>> Previously:
>> kernel shutdown 0.022131328s
>> relocation 0.440510736s
>> kernel startup 0.294706768s
>>
>> Relocation was taking: 58.2% of reboot time
>>
>> Now:
>> kernel shutdown 0.032066576s
>> relocation 0.022158152s
>> kernel startup 0.296055880s
>>
>> Now: Relocation takes 6.3% of reboot time
>>
>> Total reboot is x2.16 times faster.
When I first saw these numbers they were ~'0.29s', which I wrongly assumed was 29 seconds.
Savings in milliseconds, for _reboot_ is a hard sell. I'm hoping that on the machines that
take minutes to kexec we'll get numbers that make this change more convincing.
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 06/18] compat_ioctl: move WDIOC handling into wdt drivers
From: Guenter Roeck @ 2019-08-15 18:10 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-hwmon, linux-rtc, Alexandre Belloni, Jean Delvare,
Anatolij Gustschin, linux-um, linux-kernel, Ludovic Desroches,
Florian Fainelli, bcm-kernel-feedback-list, viro, linux-fsdevel,
openipmi-developer, Wim Van Sebroeck, linuxppc-dev,
linux-arm-kernel, linux-watchdog
In-Reply-To: <20190814205245.121691-1-arnd@arndb.de>
On Wed, Aug 14, 2019 at 10:49:18PM +0200, Arnd Bergmann wrote:
> All watchdog drivers implement the same set of ioctl commands, and
> fortunately all of them are compatible between 32-bit and 64-bit
> architectures.
>
> Modern drivers always go through drivers/watchdog/wdt.c as an abstraction
> layer, but older ones implement their own file_operations on a character
> device for this.
>
> Move the handling from fs/compat_ioctl.c into the individual drivers.
>
> Note that most of the legacy drivers will never be used on 64-bit
> hardware, because they are for an old 32-bit SoC implementation, but
> doing them all at once is safer than trying to guess which ones do
> or do not need the compat_ioctl handling.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
This patch doesn't seem to have a useful base (or at least git says so).
It does not apply to mainline nor to my own watchdog-next branch.
I assume you plan to apply the entire series together. Please not
that there will be conflicts against watchdog-next when you do so.
Guenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: Don't use KPTI where we have E0PD
From: Mark Brown @ 2019-08-15 18:00 UTC (permalink / raw)
To: Will Deacon; +Cc: Catalin Marinas, linux-arm-kernel, Suzuki K Poulose
In-Reply-To: <20190815163541.yngqvjmehpuf74ye@willie-the-truck>
[-- Attachment #1.1: Type: text/plain, Size: 1351 bytes --]
On Thu, Aug 15, 2019 at 05:35:42PM +0100, Will Deacon wrote:
> I'm still unsure as to how this works with the kaslr check in
> kpti_install_ng_mappings(). Imagine you have a big.LITTLE system using
> kaslr where the boot CPU has E0PD but the secondary CPU doesn't, and
> requires kpti.
Yes, in fact that is my default big.LITTLE test case.
> In this case, I think we'll:
> 1. Start off with global mappings installed by the boot CPU
> 2. Detect KPTI as being required on the secondary CPU
> 3. Avoid rewriting the page tables because kaslr_offset > 0
> At this point, we've got exposed global mappings on the secondary CPU.
Right, yes. It'd be enormously helpful if KASLR were a bit more visible
in the boot logs or something since I yet again managed to do that bit
of my testing without KASLR actually taking effect :/
> Thinking about this further, I think we can simply move all of the
> 'kaslr_offset() > 0' checks used by the kpti code (i.e. in
> arm64_kernel_unmapped_at_el0(), kpti_install_ng_mappings() and
> unmap_kernel_at_el0()) into a helper function which does the check for
> E0PD as well. Perhaps 'kaslr_requires_kpti()' ?
> I think that should simplify your patch as well. What do you think?
Dunno about simplifying the patch particularly, looks very similar but
in any case it does appear to solve the problem - thanks.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 04/14] media: rkisp1: add Rockchip MIPI Synopsys DPHY driver
From: Laurent Pinchart @ 2019-08-15 17:54 UTC (permalink / raw)
To: Helen Koike
Cc: devicetree, eddie.cai.linux, kernel, heiko, jacob2.chen,
jeffy.chen, zyc, linux-kernel, tfiga, linux-rockchip,
Sakari Ailus, sakari.ailus, zhengsq, hans.verkuil, mchehab,
ezequiel, linux-arm-kernel, linux-media
In-Reply-To: <c61498b0-dd4c-53af-db82-169f8dfdc6bd@collabora.com>
Hi Helen,
Thank you for the patch.
On Wed, Aug 07, 2019 at 10:37:55AM -0300, Helen Koike wrote:
> On 8/7/19 10:05 AM, Sakari Ailus wrote:
> > On Tue, Jul 30, 2019 at 03:42:46PM -0300, Helen Koike wrote:
> >> From: Jacob Chen <jacob2.chen@rock-chips.com>
> >>
> >> This commit adds a subdev driver for Rockchip MIPI Synopsys DPHY driver
> >>
> >> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
> >> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> >> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> >> [migrate to phy framework]
> >> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> >> [update for upstream]
> >> Signed-off-by: Helen Koike <helen.koike@collabora.com>
> >>
> >> ---
> >>
> >> Changes in v8:
> >> - Remove boiler plate license text
> >>
> >> Changes in v7:
> >> - Migrate dphy specific code from
> >> drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
> >> to drivers/phy/rockchip/phy-rockchip-dphy.c
> >> - Drop support for rk3288
> >> - Drop support for dphy txrx
> >> - code styling and checkpatch fixes
> >>
> >> drivers/phy/rockchip/Kconfig | 8 +
> >> drivers/phy/rockchip/Makefile | 1 +
> >> drivers/phy/rockchip/phy-rockchip-dphy.c | 408 +++++++++++++++++++++++
> >> 3 files changed, 417 insertions(+)
> >> create mode 100644 drivers/phy/rockchip/phy-rockchip-dphy.c
> >>
> >> diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
> >> index c454c90cd99e..afd072f135e6 100644
> >> --- a/drivers/phy/rockchip/Kconfig
> >> +++ b/drivers/phy/rockchip/Kconfig
> >> @@ -9,6 +9,14 @@ config PHY_ROCKCHIP_DP
> >> help
> >> Enable this to support the Rockchip Display Port PHY.
> >>
> >> +config PHY_ROCKCHIP_DPHY
> >> + tristate "Rockchip MIPI Synopsys DPHY driver"
How much of this PHY is Rockchip-specific ? Would it make sense to turn
it into a Synopsys DPHY driver, with some Rockchip glue ? I suppose this
could always be done later, if needed (and I also suppose there's no
existing driver in drivers/phy/ that support the same Synopsys IP).
> >> + depends on ARCH_ROCKCHIP && OF
> >
> > How about (...) || COMPILE_TEST ?
> >
> >> + select GENERIC_PHY_MIPI_DPHY
> >> + select GENERIC_PHY
> >> + help
> >> + Enable this to support the Rockchip MIPI Synopsys DPHY.
> >> +
> >> config PHY_ROCKCHIP_EMMC
> >> tristate "Rockchip EMMC PHY Driver"
> >> depends on ARCH_ROCKCHIP && OF
> >> diff --git a/drivers/phy/rockchip/Makefile b/drivers/phy/rockchip/Makefile
> >> index fd21cbaf40dd..f62e9010bcaf 100644
> >> --- a/drivers/phy/rockchip/Makefile
> >> +++ b/drivers/phy/rockchip/Makefile
> >> @@ -1,5 +1,6 @@
> >> # SPDX-License-Identifier: GPL-2.0
> >> obj-$(CONFIG_PHY_ROCKCHIP_DP) += phy-rockchip-dp.o
> >> +obj-$(CONFIG_PHY_ROCKCHIP_DPHY) += phy-rockchip-dphy.o
> >> obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o
> >> obj-$(CONFIG_PHY_ROCKCHIP_INNO_HDMI) += phy-rockchip-inno-hdmi.o
> >> obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2) += phy-rockchip-inno-usb2.o
> >> diff --git a/drivers/phy/rockchip/phy-rockchip-dphy.c b/drivers/phy/rockchip/phy-rockchip-dphy.c
> >> new file mode 100644
> >> index 000000000000..3a29976c2dff
> >> --- /dev/null
> >> +++ b/drivers/phy/rockchip/phy-rockchip-dphy.c
> >> @@ -0,0 +1,408 @@
> >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >> +/*
> >> + * Rockchip MIPI Synopsys DPHY driver
> >> + *
> >> + * Based on:
> >> + *
> >> + * Copyright (C) 2016 FuZhou Rockchip Co., Ltd.
> >> + * Author: Yakir Yang <ykk@@rock-chips.com>
> >> + */
> >> +
> >> +#include <linux/clk.h>
> >> +#include <linux/io.h>
> >> +#include <linux/mfd/syscon.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/phy/phy.h>
> >> +#include <linux/phy/phy-mipi-dphy.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/regmap.h>
> >> +
> >> +#define RK3399_GRF_SOC_CON9 0x6224
> >> +#define RK3399_GRF_SOC_CON21 0x6254
> >> +#define RK3399_GRF_SOC_CON22 0x6258
> >> +#define RK3399_GRF_SOC_CON23 0x625c
> >> +#define RK3399_GRF_SOC_CON24 0x6260
> >> +#define RK3399_GRF_SOC_CON25 0x6264
> >> +#define RK3399_GRF_SOC_STATUS1 0xe2a4
> >> +
> >> +#define CLOCK_LANE_HS_RX_CONTROL 0x34
> >> +#define LANE0_HS_RX_CONTROL 0x44
> >> +#define LANE1_HS_RX_CONTROL 0x54
> >> +#define LANE2_HS_RX_CONTROL 0x84
> >> +#define LANE3_HS_RX_CONTROL 0x94
> >> +#define HS_RX_DATA_LANES_THS_SETTLE_CONTROL 0x75
> >> +
> >> +#define MAX_DPHY_CLK 8
> >> +
> >> +#define PHY_TESTEN_ADDR (0x1 << 16)
> >> +#define PHY_TESTEN_DATA (0x0 << 16)
> >> +#define PHY_TESTCLK (0x1 << 1)
> >> +#define PHY_TESTCLR (0x1 << 0)
Maybe s/0x// for the previous four lines ?
> >> +#define THS_SETTLE_COUNTER_THRESHOLD 0x04
> >> +
> >> +#define HIWORD_UPDATE(val, mask, shift) \
> >> + ((val) << (shift) | (mask) << ((shift) + 16))
As you use this in a single place, I would inline it, possibly with a
small comment that explains what's happening.
> >> +
> >> +#define GRF_SOC_CON12 0x0274
> >> +
> >> +#define GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK BIT(20)
> >> +#define GRF_EDP_REF_CLK_SEL_INTER BIT(4)
> >> +
> >> +#define GRF_EDP_PHY_SIDDQ_HIWORD_MASK BIT(21)
> >> +#define GRF_EDP_PHY_SIDDQ_ON 0
> >> +#define GRF_EDP_PHY_SIDDQ_OFF BIT(5)
I would recommend aligning the value of of all macros in the same way.
> >> +
> >> +struct hsfreq_range {
> >> + u32 range_h;
The structure would be more compact if you turned this into a u16.
> >> + u8 cfg_bit;
> >> +};
> >> +
> >> +static const struct hsfreq_range rk3399_mipidphy_hsfreq_ranges[] = {
> >> + { 89, 0x00}, { 99, 0x10}, { 109, 0x20}, { 129, 0x01},
> >> + { 139, 0x11}, { 149, 0x21}, { 169, 0x02}, { 179, 0x12},
> >> + { 199, 0x22}, { 219, 0x03}, { 239, 0x13}, { 249, 0x23},
> >> + { 269, 0x04}, { 299, 0x14}, { 329, 0x05}, { 359, 0x15},
> >> + { 399, 0x25}, { 449, 0x06}, { 499, 0x16}, { 549, 0x07},
> >> + { 599, 0x17}, { 649, 0x08}, { 699, 0x18}, { 749, 0x09},
> >> + { 799, 0x19}, { 849, 0x29}, { 899, 0x39}, { 949, 0x0a},
> >> + { 999, 0x1a}, {1049, 0x2a}, {1099, 0x3a}, {1149, 0x0b},
> >> + {1199, 0x1b}, {1249, 0x2b}, {1299, 0x3b}, {1349, 0x0c},
> >> + {1399, 0x1c}, {1449, 0x2c}, {1500, 0x3c}
Maybe s/{/{ / and s/}/ }/ to give it a bit more air ? :-)
> >> +};
> >> +
> >> +static const char * const rk3399_mipidphy_clks[] = {
> >> + "dphy-ref",
> >> + "dphy-cfg",
> >> + "grf",
> >> +};
> >> +
> >> +enum dphy_reg_id {
> >> + GRF_DPHY_RX0_TURNDISABLE = 0,
> >> + GRF_DPHY_RX0_FORCERXMODE,
> >> + GRF_DPHY_RX0_FORCETXSTOPMODE,
> >> + GRF_DPHY_RX0_ENABLE,
> >> + GRF_DPHY_RX0_TESTCLR,
> >> + GRF_DPHY_RX0_TESTCLK,
> >> + GRF_DPHY_RX0_TESTEN,
> >> + GRF_DPHY_RX0_TESTDIN,
> >> + GRF_DPHY_RX0_TURNREQUEST,
> >> + GRF_DPHY_RX0_TESTDOUT,
> >> + GRF_DPHY_TX0_TURNDISABLE,
> >> + GRF_DPHY_TX0_FORCERXMODE,
> >> + GRF_DPHY_TX0_FORCETXSTOPMODE,
> >> + GRF_DPHY_TX0_TURNREQUEST,
> >> + GRF_DPHY_TX1RX1_TURNDISABLE,
> >> + GRF_DPHY_TX1RX1_FORCERXMODE,
> >> + GRF_DPHY_TX1RX1_FORCETXSTOPMODE,
> >> + GRF_DPHY_TX1RX1_ENABLE,
> >> + GRF_DPHY_TX1RX1_MASTERSLAVEZ,
> >> + GRF_DPHY_TX1RX1_BASEDIR,
> >> + GRF_DPHY_TX1RX1_ENABLECLK,
> >> + GRF_DPHY_TX1RX1_TURNREQUEST,
> >> + GRF_DPHY_RX1_SRC_SEL,
> >> + /* rk3288 only */
> >> + GRF_CON_DISABLE_ISP,
> >> + GRF_CON_ISP_DPHY_SEL,
> >> + GRF_DSI_CSI_TESTBUS_SEL,
> >> + GRF_DVP_V18SEL,
> >> + /* below is for rk3399 only */
> >> + GRF_DPHY_RX0_CLK_INV_SEL,
> >> + GRF_DPHY_RX1_CLK_INV_SEL,
> >> +};
> >> +
> >> +struct dphy_reg {
> >> + u32 offset;
> >> + u32 mask;
> >> + u32 shift;
The offset should hold in 16 bits and the mask and shift in 8 bits. That
would save space in the table below.
> >> +};
> >> +
> >> +#define PHY_REG(_offset, _width, _shift) \
> >> + { .offset = _offset, .mask = BIT(_width) - 1, .shift = _shift, }
> >> +
> >> +static const struct dphy_reg rk3399_grf_dphy_regs[] = {
> >> + [GRF_DPHY_RX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON9, 4, 0),
> >> + [GRF_DPHY_RX0_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 10),
> >> + [GRF_DPHY_RX1_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 11),
> >> + [GRF_DPHY_RX0_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 0),
> >> + [GRF_DPHY_RX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 4),
> >> + [GRF_DPHY_RX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 8),
> >> + [GRF_DPHY_RX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 12),
> >> + [GRF_DPHY_TX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 0),
> >> + [GRF_DPHY_TX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 4),
> >> + [GRF_DPHY_TX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 8),
> >> + [GRF_DPHY_TX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 12),
> >> + [GRF_DPHY_TX1RX1_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 0),
> >> + [GRF_DPHY_TX1RX1_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 4),
> >> + [GRF_DPHY_TX1RX1_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 8),
> >> + [GRF_DPHY_TX1RX1_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 12),
> >> + [GRF_DPHY_TX1RX1_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON24, 4, 0),
> >> + [GRF_DPHY_RX1_SRC_SEL] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 4),
> >> + [GRF_DPHY_TX1RX1_BASEDIR] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 5),
> >> + [GRF_DPHY_TX1RX1_ENABLECLK] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 6),
> >> + [GRF_DPHY_TX1RX1_MASTERSLAVEZ] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 7),
> >> + [GRF_DPHY_RX0_TESTDIN] = PHY_REG(RK3399_GRF_SOC_CON25, 8, 0),
> >> + [GRF_DPHY_RX0_TESTEN] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 8),
> >> + [GRF_DPHY_RX0_TESTCLK] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 9),
> >> + [GRF_DPHY_RX0_TESTCLR] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 10),
> >> + [GRF_DPHY_RX0_TESTDOUT] = PHY_REG(RK3399_GRF_SOC_STATUS1, 8, 0),
The annoying part with such an indirection is that you can't really
write multiple fields in a single register with a single operation. Is
the register mapping completely different between the rk3288 and the
rk3399, or are the fields grouped in registers in a similar way ? In the
latter case we could possibly optimise it.
> >> +};
> >> +
> >> +struct dphy_drv_data {
> >> + const char * const *clks;
> >> + int num_clks;
This is never negative, you can make it an unsigned int.
> >> + const struct hsfreq_range *hsfreq_ranges;
> >> + int num_hsfreq_ranges;
Same here.
> >> + const struct dphy_reg *regs;
> >> +};
> >> +
> >> +struct rockchip_dphy {
> >> + struct device *dev;
> >> + struct regmap *grf;
> >> + const struct dphy_reg *grf_regs;
> >> + struct clk_bulk_data clks[MAX_DPHY_CLK];
> >> +
> >> + const struct dphy_drv_data *drv_data;
> >> + struct phy_configure_opts_mipi_dphy config;
> >> +};
> >> +
> >> +static inline void write_grf_reg(struct rockchip_dphy *priv,
> >> + int index, u8 value)
Maybe unsigned int index ?
> >> +{
> >> + const struct dphy_reg *reg = &priv->grf_regs[index];
> >> + unsigned int val = HIWORD_UPDATE(value, reg->mask, reg->shift);
> >> +
> >> + WARN_ON(!reg->offset);
> >> + regmap_write(priv->grf, reg->offset, val);
> >> +}
> >> +
> >> +static void mipidphy0_wr_reg(struct rockchip_dphy *priv,
> >> + u8 test_code, u8 test_data)
Function (and structure) names have different prefixes, would it make
sense to standardise them ? Maybe rockchip_dphy_ ? Or rk_dphy_ for a
shorter version ? This could become rk_dphy_write_dphy(), and the
previous function rk_dphy_write_grf().
> >> +{
> >> + /*
> >> + * With the falling edge on TESTCLK, the TESTDIN[7:0] signal content
> >> + * is latched internally as the current test code. Test data is
> >> + * programmed internally by rising edge on TESTCLK.
> >> + */
I've never understood why PHYs tend to have a register named TEST that
contains way more than test data :-)
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTDIN, test_code);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTEN, 1);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 0);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTEN, 0);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTDIN, test_data);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
> >> +}
> >> +
> >> +/* should be move to power_on */
s/move/moved/
Do you mean merging the two functions together ? What prevents from
doing so ?
> >> +static int mipidphy_rx_stream_on(struct rockchip_dphy *priv)
> >> +{
> >> + const struct dphy_drv_data *drv_data = priv->drv_data;
> >> + const struct hsfreq_range *hsfreq_ranges = drv_data->hsfreq_ranges;
> >> + struct phy_configure_opts_mipi_dphy *config = &priv->config;
> >> + unsigned int i, hsfreq = 0, data_rate_mbps = config->hs_clk_rate;
> >> + int num_hsfreq_ranges = drv_data->num_hsfreq_ranges;
> >> +
> >> + do_div(data_rate_mbps, 1000 * 1000);
> >> +
> >> + dev_dbg(priv->dev, "%s: lanes %d - data_rate_mbps %u\n",
> >> + __func__, config->lanes, data_rate_mbps);
> >> +
> >> + for (i = 0; i < num_hsfreq_ranges; i++) {
> >> + if (hsfreq_ranges[i].range_h >= data_rate_mbps) {
> >> + hsfreq = hsfreq_ranges[i].cfg_bit;
> >> + break;
> >> + }
> >> + }
As num_hsfreq_ranges and hsfreq_ranges are only used in this loop, I
would remove the local variables.
> >> +
> >> + write_grf_reg(priv, GRF_DPHY_RX0_FORCERXMODE, 0);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_FORCETXSTOPMODE, 0);
> >> +
> >> + /* Disable lan turn around, which is ignored in receive mode */
Is it "lan turn around", or "lane turn around" ?
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TURNREQUEST, 0);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TURNDISABLE, 0xf);
> >> +
> >> + write_grf_reg(priv, GRF_DPHY_RX0_ENABLE, GENMASK(config->lanes - 1, 0));
> >> +
> >> + /* dphy start */
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLR, 1);
> >> + usleep_range(100, 150);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLR, 0);
> >> + usleep_range(100, 150);
> >> +
> >> + /* set clock lane */
> >> + /* HS hsfreq_range & lane 0 settle bypass */
> >> + mipidphy0_wr_reg(priv, CLOCK_LANE_HS_RX_CONTROL, 0);
> >> + /* HS RX Control of lane0 */
> >> + mipidphy0_wr_reg(priv, LANE0_HS_RX_CONTROL, hsfreq << 1);
> >> + /* HS RX Control of lane1 */
> >> + mipidphy0_wr_reg(priv, LANE1_HS_RX_CONTROL, 0);
> >> + /* HS RX Control of lane2 */
> >> + mipidphy0_wr_reg(priv, LANE2_HS_RX_CONTROL, 0);
> >> + /* HS RX Control of lane3 */
> >> + mipidphy0_wr_reg(priv, LANE3_HS_RX_CONTROL, 0);
Does this hardcode usage of a single lane ?
> >> + /* HS RX Data Lanes Settle State Time Control */
> >> + mipidphy0_wr_reg(priv, HS_RX_DATA_LANES_THS_SETTLE_CONTROL,
> >> + THS_SETTLE_COUNTER_THRESHOLD);
> >> +
> >> + /* Normal operation */
> >> + mipidphy0_wr_reg(priv, 0x0, 0);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int rockchip_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> >> +{
> >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> >> + int ret;
> >> +
> >> + /* pass with phy_mipi_dphy_get_default_config (with pixel rate?) */
I'm not sure to understand what this means.
> >> + ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + memcpy(&priv->config, opts, sizeof(priv->config));
> >
> > You could to:
> >
> > priv->config = *opts;
> >
> > Up to you. Some people like memcpy(). :-)
>
> your way is better thanks!
>
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int rockchip_dphy_power_on(struct phy *phy)
> >> +{
> >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> >> + int ret;
> >> +
> >> + ret = clk_bulk_enable(priv->drv_data->num_clks, priv->clks);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + return mipidphy_rx_stream_on(priv);
Should you call clk_bulk_disable() if mipidphy_rx_stream_on() fails ?
Actually that function never fails, so I'd make it a void function, and
return 0 here.
What happens if the clock rate is higher than the maximum supported by
the PHY ? Shouldn't rockchip_dphy_configure() fail in that case ?
> >> +}
> >> +
> >> +static int rockchip_dphy_power_off(struct phy *phy)
> >> +{
> >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> >> +
No need to write any register ? That's scary, what will happen on the
next power on, when the clocks gets enabled ?
> >> + clk_bulk_disable(priv->drv_data->num_clks, priv->clks);
> >> + return 0;
> >> +}
> >> +
> >> +static int rockchip_dphy_init(struct phy *phy)
> >> +{
> >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> >> + int ret;
> >> +
> >> + ret = clk_bulk_prepare(priv->drv_data->num_clks, priv->clks);
> >
> > return ...;
> >
> >> + if (ret)
> >> + return ret;
> >> + return 0;
> >> +}
> >> +
> >> +static int rockchip_dphy_exit(struct phy *phy)
> >> +{
> >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> >> +
> >> + clk_bulk_unprepare(priv->drv_data->num_clks, priv->clks);
> >> + return 0;
> >> +}
> >> +
> >> +static const struct phy_ops rockchip_dphy_ops = {
> >> + .power_on = rockchip_dphy_power_on,
> >> + .power_off = rockchip_dphy_power_off,
> >> + .init = rockchip_dphy_init,
> >> + .exit = rockchip_dphy_exit,
> >> + .configure = rockchip_dphy_configure,
> >> + .owner = THIS_MODULE,
> >> +};
> >> +
> >> +static const struct dphy_drv_data rk3399_mipidphy_drv_data = {
> >> + .clks = rk3399_mipidphy_clks,
> >> + .num_clks = ARRAY_SIZE(rk3399_mipidphy_clks),
> >> + .hsfreq_ranges = rk3399_mipidphy_hsfreq_ranges,
> >> + .num_hsfreq_ranges = ARRAY_SIZE(rk3399_mipidphy_hsfreq_ranges),
> >> + .regs = rk3399_grf_dphy_regs,
> >
> > Do you expect to support more of the similar PHY(s) --- are there such? If
> > not, you could put these in the code that uses them.
>
> Yes, for rk3288 in the future.
>
> >> +};
> >> +
> >> +static const struct of_device_id rockchip_dphy_dt_ids[] = {
> >> + {
> >> + .compatible = "rockchip,rk3399-mipi-dphy",
> >> + .data = &rk3399_mipidphy_drv_data,
> >> + },
> >> + {}
> >> +};
> >> +MODULE_DEVICE_TABLE(of, rockchip_dphy_dt_ids);
> >> +
> >> +static int rockchip_dphy_probe(struct platform_device *pdev)
> >> +{
> >> + struct device *dev = &pdev->dev;
> >> + struct device_node *np = dev->of_node;
> >> + const struct dphy_drv_data *drv_data;
> >> + struct phy_provider *phy_provider;
> >> + const struct of_device_id *of_id;
> >> + struct rockchip_dphy *priv;
> >> + struct regmap *grf;
> >> + struct phy *phy;
> >> + unsigned int i;
> >> + int ret;
> >> +
> >> + if (!dev->parent || !dev->parent->of_node)
> >> + return -ENODEV;
> >> +
> >> + if (platform_get_resource(pdev, IORESOURCE_MEM, 0)) {
> >> + dev_err(&pdev->dev, "Rockchip DPHY driver only suports rx\n");
You can replace pdev->dev with dev here and below.
s/rx/RX mode/ ?
> >> + return -EINVAL;
> >> + }
> >> +
> >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >> + if (!priv)
> >> + return -ENOMEM;
> >> + priv->dev = dev;
> >> +
> >> + grf = syscon_node_to_regmap(dev->parent->of_node);
> >> + if (IS_ERR(grf)) {
> >> + grf = syscon_regmap_lookup_by_phandle(dev->of_node,
> >> + "rockchip,grf");
> >> + if (IS_ERR(grf)) {
> >> + dev_err(dev, "Can't find GRF syscon\n");
> >> + return -ENODEV;
> >> + }
> >> + }
> >> + priv->grf = grf;
> >> +
> >> + of_id = of_match_device(rockchip_dphy_dt_ids, dev);
> >> + if (!of_id)
> >> + return -EINVAL;
> >> +
> >> + drv_data = of_id->data;
> >> + priv->grf_regs = drv_data->regs;
Do you have to store grf_regs in priv, or could it be accessed through
priv->drv_data->regs ?
> >> + priv->drv_data = drv_data;
> >> + for (i = 0; i < drv_data->num_clks; i++)
> >> + priv->clks[i].id = drv_data->clks[i];
> >> + ret = devm_clk_bulk_get(&pdev->dev, drv_data->num_clks, priv->clks);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + phy = devm_phy_create(dev, np, &rockchip_dphy_ops);
> >> + if (IS_ERR(phy)) {
> >> + dev_err(dev, "failed to create phy\n");
> >> + return PTR_ERR(phy);
> >> + }
> >> + phy_set_drvdata(phy, priv);
> >> +
> >> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> >> +
> >> + return PTR_ERR_OR_ZERO(phy_provider);
> >> +}
> >> +
> >> +static struct platform_driver rockchip_dphy_driver = {
> >> + .probe = rockchip_dphy_probe,
> >> + .driver = {
> >> + .name = "rockchip-mipi-dphy",
> >> + .of_match_table = rockchip_dphy_dt_ids,
> >> + },
> >> +};
> >> +module_platform_driver(rockchip_dphy_driver);
> >> +
> >> +MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>");
> >> +MODULE_DESCRIPTION("Rockchip MIPI Synopsys DPHY driver");
> >> +MODULE_LICENSE("Dual MIT/GPL");
Overall this is quite good, there are only small issues.
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/6] drm+dma: cache support for arm, etc
From: Christoph Hellwig @ 2019-08-15 17:53 UTC (permalink / raw)
To: Rob Clark
Cc: Kate Stewart, Masayoshi Mizuma, Maciej W. Rozycki, Eric Biggers,
Catalin Marinas, Imre Deak, dri-devel, Chris Wilson,
Masahiro Yamada, Benjamin Gaignard, Mauro Carvalho Chehab,
Will Deacon, Christoph Hellwig, Emil Velikov, Deepak Sharma,
Michael Ellerman, Paul Burton, Mike Rapoport, Geert Uytterhoeven,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), Daniel Vetter,
open list:MIPS, Linus Walleij, Robin Murphy,
open list:DRM DRIVER FOR MSM ADRENO GPU, Joerg Roedel,
Arnd Bergmann, Anshuman Khandual, Hauke Mehrtens,
Jesper Dangaard Brouer, Wolfram Sang (Renesas),
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Alexios Zavras,
Russell King, Doug Anderson, Thomas Gleixner, Sean Paul,
Allison Randal, Christophe Leroy, Enrico Weigelt, Ard Biesheuvel,
Greg Kroah-Hartman, open list, Rob Clark, Souptick Joarder,
Andrew Morton, open list:DRM DRIVER FOR MSM ADRENO GPU,
christian.koenig
In-Reply-To: <CAJs_Fx4bS64s7+xQqsead3N80ZQpofqegFQu+tT=b3wcGd_2pA@mail.gmail.com>
On Thu, Aug 15, 2019 at 06:54:39AM -0700, Rob Clark wrote:
> On Wed, Aug 14, 2019 at 11:51 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > As said before I don't think these low-level helpers are the
> > right API to export, but even if they did you'd just cover a tiny
> > subset of the architectures.
>
> Are you thinking instead something like:
>
> void dma_sync_sg_for_{cpu,device}(struct device *dev, struct scatterlist *sgl,
> int nents, enum dma_data_direction dir)
> {
> for_each_sg(sgl, sg, nents, i) {
> arch_sync_dma_for_..(dev, sg_phys(sg), sg->length, dir);
> }
> }
> EXPORT_SYMBOL_GPL(dma_sync_sg_for_..)
>
> or did you have something else in mind?
No. We really need an interface thay says please give me uncached
memory (for some definition of uncached that includes that grapics
drivers call write combine), and then let the architecture do the right
thing. Basically dma_alloc_coherent with DMA_ATTR_NO_KERNEL_MAPPING
is superficially close to what you want, except that the way the drm
drivers work you can't actually use it.
The reason for that is if we can we really need to not create another
uncachable alias, but instead change the page attributes in place.
On x86 we can and must do that for example, and based on the
conversation with Will arm64 could do that fairly easily. arm32 can
right now only do that for CMA, though.
The big question is what API do we want. I had a pretty similar
discussion with Christian on doing such an allocation for amdgpu,
where the device normally is cache coherent, but they actually want
to turn it into non-coherent by using PCIe unsnooped transactions.
Here is my high level plan, which still has a few lose end:
(1) provide a new API:
struct page *dma_alloc_pages(struct device *dev, unsigned nr_pages,
gfp_t gfp, unsigned long flags);
void dma_free_pages(struct device *dev, unsigned nr_pages,
unsigned long flags);
These give you back page backed memory that is guaranteed to be
addressable by the device (no swiotlb or similar). The memory can
then be mapped using dma_map*, including unmap and dma_sync to
bounce ownership around. This is the replacement for the current
dma_alloc_attrs with DMA_ATTR_NON_CONSISTENT API, that is rather
badly defined.
(2) Add support for DMA_ATTR_NO_KERNEL_MAPPING to this new API instead
of dma_alloc_attrs. The initial difference with that flag is just
that we allow highmem, but in the future we could also unmap this
memory from the kernel linear mapping entirely on architectures
where we can easily do that.
(3) Add a dma_pages_map/dma_pages_unmap or similar API that allows you
to get a kernel mapping for parts or all of a
DMA_ATTR_NO_KERNEL_MAPPING allocation. This is to replace things
like your open-coded vmap in msm (or similarly elsewhere in dma-buf
providers).
(4) Add support for a DMA_ATTR_UNCACHABLE flags (or similar) to the new
API, that maps the pages as uncachable iff they have a kernel
mapping, including invalidating the caches at time of this page
attribute change (or creation of a new mapping). This API will fail
if the architecture does not allow in-place remapping. Note that for
arm32 we could always dip into the CMA pool if one is present to not
fail. We'll also need some helper to map from the DMA_ATTR_* flags
to a pgprot for mapping the page to userspace. There is also a few
other weird bits here, e.g. on architectures like mips that use an
uncached segment we'll have to fail use with the plain
DMA_ATTR_UNCACHABLE flag, but it could be supported with
DMA_ATTR_UNCACHABLE | DMA_ATTR_NO_KERNEL_MAPPING.
I was hoping to get most of this done for this merge window, but I'm
probably lucky if I get at least parts done. Too much distraction.
> Hmm, not entirely sure why.. you should be on the cc list for each
> individual patch.
They finally made it, although even with the delay they only ended up
in the spam mailbox. I still can't see them on the various mailing
lists.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] coresight: tmc-etr: Fix perf_data check.
From: Mathieu Poirier @ 2019-08-15 17:37 UTC (permalink / raw)
To: Yabin Cui
Cc: Alexander Shishkin, Linux Kernel Mailing List, linux-arm-kernel,
Suzuki K Poulose
In-Reply-To: <20190812190320.209988-1-yabinc@google.com>
On Mon, 12 Aug 2019 at 13:03, Yabin Cui <yabinc@google.com> wrote:
>
> When tracing etm data of multiple threads on multiple cpus through
> perf interface, each cpu has a unique etr_perf_buffer while sharing
> the same etr device. There is no guarantee that the last cpu starts
> etm tracing also stops last. This makes perf_data check fail.
>
> Fix it by checking etr_buf instead of etr_perf_buffer.
>
> Fixes: 3147da92a8a8 ("coresight: tmc-etr: Allocate and free ETR memory buffers for CPU-wide scenarios")
> Signed-off-by: Yabin Cui <yabinc@google.com>
> ---
>
> v1 -> v2: rename perf_data to perf_buf. Add fixes tag.
>
> ---
> drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 +++---
> drivers/hwtracing/coresight/coresight-tmc.h | 6 +++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 17006705287a..90d1548ad268 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1484,7 +1484,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
> goto out;
> }
>
> - if (WARN_ON(drvdata->perf_data != etr_perf)) {
> + if (WARN_ON(drvdata->perf_buf != etr_buf)) {
> lost = true;
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> goto out;
> @@ -1497,7 +1497,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
>
> CS_LOCK(drvdata->base);
> /* Reset perf specific data */
> - drvdata->perf_data = NULL;
> + drvdata->perf_buf = NULL;
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> size = etr_buf->len;
> @@ -1556,7 +1556,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
> }
>
> etr_perf->head = PERF_IDX2OFF(handle->head, etr_perf);
> - drvdata->perf_data = etr_perf;
> + drvdata->perf_buf = etr_perf->etr_buf;
Ok for the fix. Looking a things again I don't see a need to do the
assignment for each event - this needs to be done only when the device
is assocated with a monitored process. Please move it here [1].
Thanks,
Mathieu
[1]. https://elixir.bootlin.com/linux/v5.3-rc4/source/drivers/hwtracing/coresight/coresight-tmc-etr.c#L1572
>
> /*
> * No HW configuration is needed if the sink is already in
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 1ed50411cc3c..f9a0c95e9ba2 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -178,8 +178,8 @@ struct etr_buf {
> * device configuration register (DEVID)
> * @idr: Holds etr_bufs allocated for this ETR.
> * @idr_mutex: Access serialisation for idr.
> - * @perf_data: PERF buffer for ETR.
> - * @sysfs_data: SYSFS buffer for ETR.
> + * @sysfs_buf: SYSFS buffer for ETR.
> + * @perf_buf: PERF buffer for ETR.
> */
> struct tmc_drvdata {
> void __iomem *base;
> @@ -202,7 +202,7 @@ struct tmc_drvdata {
> struct idr idr;
> struct mutex idr_mutex;
> struct etr_buf *sysfs_buf;
> - void *perf_data;
> + struct etr_buf *perf_buf;
> };
>
> struct etr_buf_operations {
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] coresight: tmc-etr: Fix perf_data check.
From: Mathieu Poirier @ 2019-08-15 17:22 UTC (permalink / raw)
To: Yabin Cui
Cc: Alexander Shishkin, Linux Kernel Mailing List, linux-arm-kernel,
Suzuki K Poulose
In-Reply-To: <20190814235058.184204-1-yabinc@google.com>
On Wed, 14 Aug 2019 at 17:51, Yabin Cui <yabinc@google.com> wrote:
>
> > Did you actually see the check fail or is this a theoretical thing?
> > I'm really perplex here has I have tested this scenario many times
> > without issues.
> >
> I have seen this warning in dmesg output, that's how I find the problem.
>
> > In CPU wide scenarios each perf event (one per CPU) is associated with
> > an event_data during the setup process. The event_data is the
> > etr_perf holding a reference to the perf ring buffer for that specific
> > event along with the etr_buf, regardless of who created the latter.
>
> Agree.
>
> > From there, when the event is installed on a CPU, the csdev for that
> > CPU is given a reference to the event_data of that event[1]. Before
> > going further notice how there is a per CPU csdev and event handle to
> > keep track of event specifics[2]. As such both (per CPU) csdev and
> > event handle carry the exact same reference to the etr_perf.
> >
> On my test device (Pixel 3), there is an ETM device on each cpu, but only
> one ETR device for the whole device. So there is only one instance of etr
> csdev in the kernel. If multiple cpus are scheduling on etm perf events at
> the same time, all of them are trying to set their event_data to the same
> etr csdev. And different perf events have different event_data. A warning
> situation is as below:
>
> cpu 0
> schedule on event A (set etr csdev->perf_data to event_a.etr_perf)
>
> cpu 1
> schedule on event B (set etr csdev->perf_data to event_b.etr_perf)
>
You are 100% right and looking at it again this morning it just jumped
at me. I simply can't understand how it did not manifest itself
during all the hammering I did on it.
Please see details in my other (and upcoming) email.
Thanks,
Mathieu
> cpu 1
> schedule off event B (update buffer, does nothing since csdev->refcnt != 1)
>
> cpu 0
> schedule off event A (update buffer, but etr csdev->perf_data check fail)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 0/8] arm64: MMU enabled kexec relocation
From: Pavel Tatashin @ 2019-08-15 17:16 UTC (permalink / raw)
To: Pavel Tatashin, James Morris, Sasha Levin, Eric W. Biederman,
kexec mailing list, LKML, Jonathan Corbet, Catalin Marinas, will,
Linux ARM, Marc Zyngier, James Morse, Vladimir Murzin,
Matthias Brugger, Bhupesh Sharma, linux-mm
In-Reply-To: <CA+CK2bADiBMEx9cJuXT5fQkBYFZAtxUtc7ZzjrNfEjijPZkPtw@mail.gmail.com>
Hi,
It is been two weeks, and no review activity yet. Please help with
reviewing this work.
Thank you,
Pasha
On Thu, Aug 8, 2019 at 2:44 PM Pavel Tatashin <pasha.tatashin@soleen.com> wrote:
>
> Just a friendly reminder, please send your comments on this series.
> It's been a week since I sent out these patches, and no feedback yet.
> Also, I'd appreciate if anyone could test this series on vhe hardware
> with vhe kernel, it does not look like QEMU can emulate it yet
>
> Thank you,
> Pasha
>
> On Thu, Aug 1, 2019 at 11:24 AM Pavel Tatashin
> <pasha.tatashin@soleen.com> wrote:
> >
> > Enable MMU during kexec relocation in order to improve reboot performance.
> >
> > If kexec functionality is used for a fast system update, with a minimal
> > downtime, the relocation of kernel + initramfs takes a significant portion
> > of reboot.
> >
> > The reason for slow relocation is because it is done without MMU, and thus
> > not benefiting from D-Cache.
> >
> > Performance data
> > ----------------
> > For this experiment, the size of kernel plus initramfs is small, only 25M.
> > If initramfs was larger, than the improvements would be greater, as time
> > spent in relocation is proportional to the size of relocation.
> >
> > Previously:
> > kernel shutdown 0.022131328s
> > relocation 0.440510736s
> > kernel startup 0.294706768s
> >
> > Relocation was taking: 58.2% of reboot time
> >
> > Now:
> > kernel shutdown 0.032066576s
> > relocation 0.022158152s
> > kernel startup 0.296055880s
> >
> > Now: Relocation takes 6.3% of reboot time
> >
> > Total reboot is x2.16 times faster.
> >
> > Previous approaches and discussions
> > -----------------------------------
> > https://lore.kernel.org/lkml/20190709182014.16052-1-pasha.tatashin@soleen.com
> > reserve space for kexec to avoid relocation, involves changes to generic code
> > to optimize a problem that exists on arm64 only:
> >
> > https://lore.kernel.org/lkml/20190716165641.6990-1-pasha.tatashin@soleen.com
> > The first attempt to enable MMU, some bugs that prevented performance
> > improvement. The page tables unnecessary configured idmap for the whole
> > physical space.
> >
> > https://lore.kernel.org/lkml/20190731153857.4045-1-pasha.tatashin@soleen.com
> > No linear copy, bug with EL2 reboots.
> >
> > Pavel Tatashin (8):
> > kexec: quiet down kexec reboot
> > arm64, mm: transitional tables
> > arm64: hibernate: switch to transtional page tables.
> > kexec: add machine_kexec_post_load()
> > arm64, kexec: move relocation function setup and clean up
> > arm64, kexec: add expandable argument to relocation function
> > arm64, kexec: configure transitional page table for kexec
> > arm64, kexec: enable MMU during kexec relocation
> >
> > arch/arm64/Kconfig | 4 +
> > arch/arm64/include/asm/kexec.h | 51 ++++-
> > arch/arm64/include/asm/pgtable-hwdef.h | 1 +
> > arch/arm64/include/asm/trans_table.h | 68 ++++++
> > arch/arm64/kernel/asm-offsets.c | 14 ++
> > arch/arm64/kernel/cpu-reset.S | 4 +-
> > arch/arm64/kernel/cpu-reset.h | 8 +-
> > arch/arm64/kernel/hibernate.c | 261 ++++++-----------------
> > arch/arm64/kernel/machine_kexec.c | 199 ++++++++++++++----
> > arch/arm64/kernel/relocate_kernel.S | 196 +++++++++---------
> > arch/arm64/mm/Makefile | 1 +
> > arch/arm64/mm/trans_table.c | 273 +++++++++++++++++++++++++
> > kernel/kexec.c | 4 +
> > kernel/kexec_core.c | 8 +-
> > kernel/kexec_file.c | 4 +
> > kernel/kexec_internal.h | 2 +
> > 16 files changed, 758 insertions(+), 340 deletions(-)
> > create mode 100644 arch/arm64/include/asm/trans_table.h
> > create mode 100644 arch/arm64/mm/trans_table.c
> >
> > --
> > 2.22.0
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Applied "ASoC: mt2701: remove unused variables" to the asoc tree
From: Mark Brown @ 2019-08-15 17:14 UTC (permalink / raw)
To: YueHaibing
Cc: alsa-devel, perex, tiwai, lgirdwood, linux-kernel, Hulk Robot,
Mark Brown, linux-mediatek, matthias.bgg, linux-arm-kernel
In-Reply-To: <20190813143811.31456-1-yuehaibing@huawei.com>
The patch
ASoC: mt2701: remove unused variables
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From a9e792d006edbd33724f2eb858887d3b591d82c5 Mon Sep 17 00:00:00 2001
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 13 Aug 2019 22:38:11 +0800
Subject: [PATCH] ASoC: mt2701: remove unused variables
sound/soc/mediatek/mt2701/mt2701-afe-pcm.c:799:38: warning:
mt2701_afe_o23_mix defined but not used [-Wunused-const-variable=]
sound/soc/mediatek/mt2701/mt2701-afe-pcm.c:803:38: warning:
mt2701_afe_o24_mix defined but not used [-Wunused-const-variable=]
sound/soc/mediatek/mt2701/mt2701-afe-pcm.c:835:38: warning:
mt2701_afe_multi_ch_out_i2s4 defined but not used [-Wunused-const-variable=]
They are never used, so can be removed.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Link: https://lore.kernel.org/r/20190813143811.31456-1-yuehaibing@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/mediatek/mt2701/mt2701-afe-pcm.c | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c b/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
index 9af76ae315a5..d7f5defa50c2 100644
--- a/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
+++ b/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
@@ -796,14 +796,6 @@ static const struct snd_kcontrol_new mt2701_afe_o22_mix[] = {
SOC_DAPM_SINGLE_AUTODISABLE("I19 Switch", AFE_CONN22, 19, 1, 0),
};
-static const struct snd_kcontrol_new mt2701_afe_o23_mix[] = {
- SOC_DAPM_SINGLE_AUTODISABLE("I20 Switch", AFE_CONN23, 20, 1, 0),
-};
-
-static const struct snd_kcontrol_new mt2701_afe_o24_mix[] = {
- SOC_DAPM_SINGLE_AUTODISABLE("I21 Switch", AFE_CONN24, 21, 1, 0),
-};
-
static const struct snd_kcontrol_new mt2701_afe_o31_mix[] = {
SOC_DAPM_SINGLE_AUTODISABLE("I35 Switch", AFE_CONN41, 9, 1, 0),
};
@@ -832,11 +824,6 @@ static const struct snd_kcontrol_new mt2701_afe_multi_ch_out_i2s3[] = {
PWR2_TOP_CON, 18, 1, 0),
};
-static const struct snd_kcontrol_new mt2701_afe_multi_ch_out_i2s4[] = {
- SOC_DAPM_SINGLE_AUTODISABLE("Multich I2S4 Out Switch",
- PWR2_TOP_CON, 19, 1, 0),
-};
-
static const struct snd_soc_dapm_widget mt2701_afe_pcm_widgets[] = {
/* inter-connections */
SND_SOC_DAPM_MIXER("I00", SND_SOC_NOPM, 0, 0, NULL, 0),
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Applied "ASoC: rockchip: rockchip_max98090: Set period size to 240" to the asoc tree
From: Mark Brown @ 2019-08-15 17:14 UTC (permalink / raw)
To: Cheng-Yi Chiang
Cc: alsa-devel, tzungbi, Heiko Stuebner, zhengxing, Liam Girdwood,
linux-kernel, jeffy.chen, cain.cai, dianders, linux-rockchip,
Mark Brown, eddie.cai, Takashi Iwai, enric.balletbo, dgreid,
Jaroslav Kysela, linux-arm-kernel
In-Reply-To: <20190813074430.191791-1-cychiang@chromium.org>
The patch
ASoC: rockchip: rockchip_max98090: Set period size to 240
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 7188f656cdf762d4ea8ce16b6aaf4c6b06e119ec Mon Sep 17 00:00:00 2001
From: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Tue, 13 Aug 2019 15:44:30 +0800
Subject: [PATCH] ASoC: rockchip: rockchip_max98090: Set period size to 240
From stress testing of arecord, we found that period size
greater than ~900 will bring pl330 to DYING state and
can not recover within 100 iterations.
The result is that arecord will stuck and get I/O error,
and issue can not be recovered until reboot.
This issue does not happen when period size is small.
Set constraint of period size to 240 to prevent such issue.
With the constraint, there will be no issue after 2000 iterations.
We can revert this patch once the root cause is found
in rockchip's pl330 implementation.
Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
Link: https://lore.kernel.org/r/20190813074430.191791-1-cychiang@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/rockchip/rockchip_max98090.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/sound/soc/rockchip/rockchip_max98090.c b/sound/soc/rockchip/rockchip_max98090.c
index 782e534d4c0d..d54f672d38d8 100644
--- a/sound/soc/rockchip/rockchip_max98090.c
+++ b/sound/soc/rockchip/rockchip_max98090.c
@@ -138,8 +138,19 @@ static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
return ret;
}
+static int rk_aif1_startup(struct snd_pcm_substream *substream)
+{
+ /*
+ * Set period size to 240 because pl330 has issue
+ * dealing with larger period in stress testing.
+ */
+ return snd_pcm_hw_constraint_minmax(substream->runtime,
+ SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 240, 240);
+}
+
static const struct snd_soc_ops rk_aif1_ops = {
.hw_params = rk_aif1_hw_params,
+ .startup = rk_aif1_startup,
};
SND_SOC_DAILINK_DEFS(hifi,
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Applied "ASoC: mediatek: mt8183-mt6358-ts3a227-max98357: remove unused variables" to the asoc tree
From: Mark Brown @ 2019-08-15 17:14 UTC (permalink / raw)
To: YueHaibing
Cc: alsa-devel, perex, tiwai, lgirdwood, linux-kernel, Hulk Robot,
Mark Brown, linux-mediatek, matthias.bgg, linux-arm-kernel
In-Reply-To: <20190813144122.67676-1-yuehaibing@huawei.com>
The patch
ASoC: mediatek: mt8183-mt6358-ts3a227-max98357: remove unused variables
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From d59170b42610c7cbc6e96431ca8357a8bdbf592b Mon Sep 17 00:00:00 2001
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 13 Aug 2019 22:41:22 +0800
Subject: [PATCH] ASoC: mediatek: mt8183-mt6358-ts3a227-max98357: remove unused
variables
sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c:50:1: warning:
mt8183_mt6358_ts3a227_max98357_dapm_widgets defined but not used [-Wunused-const-variable=]
sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c:55:1: warning:
mt8183_mt6358_ts3a227_max98357_dapm_routes defined but not used [-Wunused-const-variable=]
They are never used, so can be removed.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Link: https://lore.kernel.org/r/20190813144122.67676-1-yuehaibing@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
.../mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c b/sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c
index 53f54078f78c..272766c1b859 100644
--- a/sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c
+++ b/sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c
@@ -46,16 +46,6 @@ static int mt8183_i2s_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
return 0;
}
-static const struct snd_soc_dapm_widget
-mt8183_mt6358_ts3a227_max98357_dapm_widgets[] = {
- SND_SOC_DAPM_OUTPUT("IT6505_8CH"),
-};
-
-static const struct snd_soc_dapm_route
-mt8183_mt6358_ts3a227_max98357_dapm_routes[] = {
- {"IT6505_8CH", NULL, "TDM"},
-};
-
static int
mt8183_mt6358_ts3a227_max98357_bt_sco_startup(
struct snd_pcm_substream *substream)
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Applied "ASoC: mediatek: mt8183-da7219-max98357: remove unused variable" to the asoc tree
From: Mark Brown @ 2019-08-15 17:14 UTC (permalink / raw)
To: YueHaibing
Cc: alsa-devel, perex, tiwai, lgirdwood, linux-kernel, Hulk Robot,
Mark Brown, linux-mediatek, matthias.bgg, linux-arm-kernel
In-Reply-To: <20190813143952.29232-1-yuehaibing@huawei.com>
The patch
ASoC: mediatek: mt8183-da7219-max98357: remove unused variable
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 57c3ed42f52cdc51f416c93b19708ef6ceb4a00b Mon Sep 17 00:00:00 2001
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 13 Aug 2019 22:39:52 +0800
Subject: [PATCH] ASoC: mediatek: mt8183-da7219-max98357: remove unused
variable
sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c:120:1: warning:
mt8183_da7219_max98357_dapm_widgets defined but not used [-Wunused-const-variable=]
sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c:124:40: warning:
mt8183_da7219_max98357_dapm_routes defined but not used [-Wunused-const-variable=]
They are never used, so can be removed.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Link: https://lore.kernel.org/r/20190813143952.29232-1-yuehaibing@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c b/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
index 2a6097174614..43f99e59a078 100644
--- a/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
+++ b/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
@@ -116,15 +116,6 @@ static int mt8183_i2s_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
return 0;
}
-static const struct snd_soc_dapm_widget
-mt8183_da7219_max98357_dapm_widgets[] = {
- SND_SOC_DAPM_OUTPUT("IT6505_8CH"),
-};
-
-static const struct snd_soc_dapm_route mt8183_da7219_max98357_dapm_routes[] = {
- {"IT6505_8CH", NULL, "TDM"},
-};
-
/* FE */
SND_SOC_DAILINK_DEFS(playback1,
DAILINK_COMP_ARRAY(COMP_CPU("DL1")),
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v8 4/5] arm64: Define Documentation/arm64/tagged-address-abi.rst
From: Kevin Brodsky @ 2019-08-15 16:54 UTC (permalink / raw)
To: Catalin Marinas, linux-arm-kernel, linux-mm
Cc: linux-arch, linux-doc, Szabolcs Nagy, Andrey Konovalov,
Will Deacon, Dave Hansen, Andrew Morton, Vincenzo Frascino,
Dave P Martin
In-Reply-To: <20190815154403.16473-5-catalin.marinas@arm.com>
On 15/08/2019 16:44, Catalin Marinas wrote:
> From: Vincenzo Frascino <vincenzo.frascino@arm.com>
>
> On AArch64 the TCR_EL1.TBI0 bit is set by default, allowing userspace
> (EL0) to perform memory accesses through 64-bit pointers with a non-zero
> top byte. Introduce the document describing the relaxation of the
> syscall ABI that allows userspace to pass certain tagged pointers to
> kernel syscalls.
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Cc: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Cc: Kevin Brodsky <kevin.brodsky@arm.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Co-developed-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> Documentation/arm64/tagged-address-abi.rst | 155 +++++++++++++++++++++
> 1 file changed, 155 insertions(+)
> create mode 100644 Documentation/arm64/tagged-address-abi.rst
>
> diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
> new file mode 100644
> index 000000000000..8808337775d6
> --- /dev/null
> +++ b/Documentation/arm64/tagged-address-abi.rst
> @@ -0,0 +1,155 @@
> +==========================
> +AArch64 TAGGED ADDRESS ABI
> +==========================
> +
> +Authors: Vincenzo Frascino <vincenzo.frascino@arm.com>
> + Catalin Marinas <catalin.marinas@arm.com>
> +
> +Date: 15 August 2019
> +
> +This document describes the usage and semantics of the Tagged Address
> +ABI on AArch64 Linux.
> +
> +1. Introduction
> +---------------
> +
> +On AArch64 the TCR_EL1.TBI0 bit is set by default, allowing userspace
> +(EL0) to perform memory accesses through 64-bit pointers with a non-zero
> +top byte. This document describes the relaxation of the syscall ABI that
> +allows userspace to pass certain tagged pointers to kernel syscalls.
> +
> +2. AArch64 Tagged Address ABI
> +-----------------------------
> +
> +From the kernel syscall interface perspective and for the purposes of
> +this document, a "valid tagged pointer" is a pointer with a potentially
> +non-zero top-byte that references an address in the user process address
> +space obtained in one of the following ways:
> +
> +- mmap() done by the process itself (or its parent), where either:
The "parent" aspect is a useful addition, but technically, the mapping may have been
established by any process indirectly forked from the current process, not just its
immediate parent. I wonder if there is a better way to formulate this, to avoid this
complication. Maybe simply "mmap() syscall" (syscalls are always made from userspace,
and any mapping requested by userspace is eligible here)?
> +
> + - flags have the **MAP_ANONYMOUS** bit set
> + - the file descriptor refers to a regular file (including those
> + returned by memfd_create()) or **/dev/zero**
> +
> +- brk() system call done by the process itself (i.e. the heap area
Same idea.
> + between the initial location of the program break at process creation
> + and its current location).
> +
> +- any memory mapped by the kernel in the address space of the process
> + during creation and with the same restrictions as for mmap() above
> + (e.g. data, bss, stack).
> +
> +The AArch64 Tagged Address ABI has two stages of relaxation depending
> +how the user addresses are used by the kernel:
> +
> +1. User addresses not accessed by the kernel but used for address space
> + management (e.g. mmap(), mprotect(), madvise()). The use of valid
> + tagged pointers in this context is always allowed.
> +
> +2. User addresses accessed by the kernel (e.g. write()). This ABI
> + relaxation is disabled by default and the application thread needs to
> + explicitly enable it via **prctl()** as follows:
> +
> + - **PR_SET_TAGGED_ADDR_CTRL**: enable or disable the AArch64 Tagged
> + Address ABI for the calling thread.
> +
> + The (unsigned int) arg2 argument is a bit mask describing the
> + control mode used:
> +
> + - **PR_TAGGED_ADDR_ENABLE**: enable AArch64 Tagged Address ABI.
> + Default status is disabled.
> +
> + Arguments arg3, arg4, and arg5 must be 0.
> +
> + - **PR_GET_TAGGED_ADDR_CTRL**: get the status of the AArch64 Tagged
> + Address ABI for the calling thread.
> +
> + Arguments arg2, arg3, arg4, and arg5 must be 0.
> +
> + The ABI properties described above are thread-scoped, inherited on
> + clone() and fork() and cleared on exec().
> +
> + Calling prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0)
> + returns -EINVAL if the AArch64 Tagged Address ABI is globally disabled
> + by sysctl abi.tagged_addr_disabled=1. The default sysctl
> + abi.tagged_addr_disabled configuration is 0.
> +
> +When the AArch64 Tagged Address ABI is enabled for a thread, the
> +following behaviours are guaranteed:
> +
> +- All syscalls except the cases mentioned in section 3 can accept any
> + valid tagged pointer.
> +
> +- The syscall behaviour is undefined for invalid tagged pointers: it may
> + result in an error code being returned, a (fatal) signal being raised,
> + or other modes of failure.
> +
> +- A valid tagged pointer has the same semantics as the corresponding
> + untagged pointer.
> +
> +A definition of the meaning of tagged pointers on AArch64 can be found
> +in Documentation/arm64/tagged-pointers.rst.
> +
> +3. AArch64 Tagged Address ABI Exceptions
> +-----------------------------------------
> +
> +The following system call parameters must be untagged regardless of the
> +ABI relaxation:
> +
> +- prctl() other than arguments pointing to user structures to be
> + accessed by the kernel.
> +
> +- ioctl() other than arguments pointing to user structures to be
> + accessed by the kernel.
Isn't "user structures" too restrictive? For instance, PR_SET_NAME takes a char *,
and there's no reason not allow it to be tagged. Maybe a more generic "user data"?
There is the additional issue of user struct's containing pointers, I guess the
restriction should apply recursively...
Otherwise, the ABI looks pretty good to me, especially the new address space
management / user data distinction.
Kevin
> +
> +- shmat() and shmdt().
> +
> +Any attempt to use non-zero tagged pointers may result in an error code
> +being returned, a (fatal) signal being raised, or other modes of
> +failure.
> +
> +4. Example of correct usage
> +---------------------------
> +.. code-block:: c
> +
> + #include <stdlib.h>
> + #include <string.h>
> + #include <unistd.h>
> + #include <sys/mman.h>
> + #include <sys/prctl.h>
> +
> + #define PR_SET_TAGGED_ADDR_CTRL 55
> + #define PR_TAGGED_ADDR_ENABLE (1UL << 0)
> +
> + #define TAG_SHIFT 56
> +
> + int main(void)
> + {
> + int tbi_enabled = 0;
> + unsigned long tag = 0;
> + char *ptr;
> +
> + /* check/enable the tagged address ABI */
> + if (!prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0))
> + tbi_enabled = 1;
> +
> + /* memory allocation */
> + ptr = mmap(NULL, sysconf(_SC_PAGE_SIZE), PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + if (ptr == MAP_FAILED)
> + return 1;
> +
> + /* set a non-zero tag if the ABI is available */
> + if (tbi_enabled)
> + tag = rand() & 0xff;
> + ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
> +
> + /* memory access to a tagged address */
> + strcpy(ptr, "tagged pointer\n");
> +
> + /* syscall with a tagged pointer */
> + write(1, ptr, strlen(ptr));
> +
> + return 0;
> + }
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] arm64: do_csum: implement accelerated scalar version
From: Will Deacon @ 2019-08-15 16:46 UTC (permalink / raw)
To: Zhangshaokun
Cc: Ard Biesheuvel, netdev, ilias.apalodimas, huanglingyan (A),
Robin Murphy, linux-arm-kernel, steve.capper
In-Reply-To: <440eb674-0e59-a97e-4a90-0026e2327069@hisilicon.com>
On Thu, May 16, 2019 at 11:14:35AM +0800, Zhangshaokun wrote:
> On 2019/5/15 17:47, Will Deacon wrote:
> > On Mon, Apr 15, 2019 at 07:18:22PM +0100, Robin Murphy wrote:
> >> On 12/04/2019 10:52, Will Deacon wrote:
> >>> I'm waiting for Robin to come back with numbers for a C implementation.
> >>>
> >>> Robin -- did you get anywhere with that?
> >>
> >> Still not what I would call finished, but where I've got so far (besides an
> >> increasingly elaborate test rig) is as below - it still wants some unrolling
> >> in the middle to really fly (and actual testing on BE), but the worst-case
> >> performance already equals or just beats this asm version on Cortex-A53 with
> >> GCC 7 (by virtue of being alignment-insensitive and branchless except for
> >> the loop). Unfortunately, the advantage of C code being instrumentable does
> >> also come around to bite me...
> >
> > Is there any interest from anybody in spinning a proper patch out of this?
> > Shaokun?
>
> HiSilicon's Kunpeng920(Hi1620) benefits from do_csum optimization, if Ard and
> Robin are ok, Lingyan or I can try to do it.
> Of course, if any guy posts the patch, we are happy to test it.
> Any will be ok.
I don't mind who posts it, but Robin is super busy with SMMU stuff at the
moment so it probably makes more sense for you or Lingyan to do it.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] i2c: iproc: Make bcm_iproc_i2c_quirks constant
From: Ray Jui @ 2019-08-15 16:43 UTC (permalink / raw)
To: Nishka Dasgupta, rjui, sbranden, bcm-kernel-feedback-list,
linux-i2c, linux-arm-kernel
In-Reply-To: <20190815055550.1588-1-nishkadg.linux@gmail.com>
Hi Nishka,
On 8/14/19 10:55 PM, Nishka Dasgupta wrote:
> Static structure bcm_iproc_i2c_quirks, of type i2c_adapter_quirks, is
> only used when being assigned to constant field quirks of a variable
> having type i2c_adapter. Hence make bcm_iproc_i2c_quirks constant as
> well to prevent it from unintended modification.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
> drivers/i2c/busses/i2c-bcm-iproc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> index d7fd76baec92..e9f0e5b6eadc 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -805,7 +805,7 @@ static struct i2c_algorithm bcm_iproc_algo = {
> .unreg_slave = bcm_iproc_i2c_unreg_slave,
> };
>
> -static struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
> +static const struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
> .max_read_len = M_RX_MAX_READ_LEN,
> };
>
>
This looks good to me. Thanks!
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv3 6/6] smccc: make 1.1 macros value-returning
From: Will Deacon @ 2019-08-15 16:42 UTC (permalink / raw)
To: Mark Rutland
Cc: lorenzo.pieralisi, suzuki.poulose, marc.zyngier, catalin.marinas,
will.deacon, linux, james.morse, robin.murphy, linux-arm-kernel
In-Reply-To: <20190809132245.43505-7-mark.rutland@arm.com>
On Fri, Aug 09, 2019 at 02:22:45PM +0100, Mark Rutland wrote:
> The arm_smccc_1_1_{smc,hvc}() macros for inline invocation take a res
> pointer as their final argument, matching the out-of-line SMCCC
> invocation functions.
>
> However, the inline invocation macros are variadic, so it's easy to mess
> up passsing the correct parameters.
passing
> Instead, let's make them value-returning, which is less confusing.
I'm not completely sure I agree with you here because, as far as I can
tell, it means that we have a different calling convention for 1.0 (i.e.
arm_smccc_smc()) and 1.1 (i.e. arm_smccc_1_1_smc).
Can we do the same for 1.0 as well or am I missing something?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: Don't use KPTI where we have E0PD
From: Will Deacon @ 2019-08-15 16:35 UTC (permalink / raw)
To: Mark Brown; +Cc: Catalin Marinas, linux-arm-kernel, Suzuki K Poulose
In-Reply-To: <20190814183103.33707-3-broonie@kernel.org>
Hi Mark,
Thanks for respinning. Comments below...
On Wed, Aug 14, 2019 at 07:31:03PM +0100, Mark Brown wrote:
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index fd6161336653..85552f6fceda 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -38,6 +38,7 @@ static inline bool arm64_kernel_unmapped_at_el0(void)
> static inline bool arm64_kernel_use_ng_mappings(void)
> {
> bool tx1_bug;
> + u64 ftr;
>
> /* What's a kpti? Use global mappings if we don't know. */
> if (!IS_ENABLED(CONFIG_UNMAP_KERNEL_AT_EL0))
> @@ -59,7 +60,7 @@ static inline bool arm64_kernel_use_ng_mappings(void)
> * KASLR is enabled so we're going to be enabling kpti on non-broken
> * CPUs regardless of their susceptibility to Meltdown. Rather
> * than force everybody to go through the G -> nG dance later on,
> - * just put down non-global mappings from the beginning.
> + * just put down non-global mappings from the beginning...
> */
> if (!IS_ENABLED(CONFIG_CAVIUM_ERRATUM_27456)) {
> tx1_bug = false;
> @@ -74,6 +75,16 @@ static inline bool arm64_kernel_use_ng_mappings(void)
> tx1_bug = __cpus_have_const_cap(ARM64_WORKAROUND_CAVIUM_27456);
> }
>
> + /*
> + * ...unless we have E0PD in which case we may use that in
> + * preference to unmapping the kernel.
> + */
> + if (IS_ENABLED(CONFIG_ARM64_E0PD)) {
> + ftr = read_sysreg_s(SYS_ID_AA64MMFR2_EL1);
> + if ((ftr >> ID_AA64MMFR2_E0PD_SHIFT) & 0xf)
> + return false;
> + }
> +
> return !tx1_bug && kaslr_offset() > 0;
I'm still unsure as to how this works with the kaslr check in
kpti_install_ng_mappings(). Imagine you have a big.LITTLE system using
kaslr where the boot CPU has E0PD but the secondary CPU doesn't, and
requires kpti.
In this case, I think we'll:
1. Start off with global mappings installed by the boot CPU
2. Detect KPTI as being required on the secondary CPU
3. Avoid rewriting the page tables because kaslr_offset > 0
At this point, we've got exposed global mappings on the secondary CPU.
Thinking about this further, I think we can simply move all of the
'kaslr_offset() > 0' checks used by the kpti code (i.e. in
arm64_kernel_unmapped_at_el0(), kpti_install_ng_mappings() and
unmap_kernel_at_el0()) into a helper function which does the check for
E0PD as well. Perhaps 'kaslr_requires_kpti()' ?
I think that should simplify your patch as well. What do you think?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH V5 5/5] arm64: dts: imx8mm: Enable cpu-idle driver
From: Daniel Lezcano @ 2019-08-15 16:12 UTC (permalink / raw)
To: Anson.Huang, catalin.marinas, will, robh+dt, mark.rutland,
shawnguo, s.hauer, kernel, festevam, linux-imx, tglx,
leonard.crestez, aisheng.dong, daniel.baluta, ping.bai, l.stach,
abel.vesa, andrew.smirnov, ccaione, angus, agx, linux-arm-kernel,
linux-kernel, devicetree
In-Reply-To: <20190710063056.35689-5-Anson.Huang@nxp.com>
Hi Anson,
sorry for the late review, I've been pretty busy.
If Shawn is ok, I can pick the patches 1-4 in my tree and then this one
after you fix the comments below.
On 10/07/2019 08:30, Anson.Huang@nxp.com wrote:
[ ... ]
> + idle-states {
> + entry-method = "psci";
> +
> + cpu_sleep_wait: cpu-sleep-wait {
Is that a retention state or a powerdown? It is preferable to change the
name to the idle state naming convention given in the PSCI documentation
[1] page 16-17
> + compatible = "arm,idle-state";
> + arm,psci-suspend-param = <0x0010033>;
> + local-timer-stop;
> + entry-latency-us = <1000>;
> + exit-latency-us = <700>;
> + min-residency-us = <2700>;
> + wakeup-latency-us = <1500>;
It is pointless to specify the entry + exit *and* the wakeup-latency [2].
Thanks
-- Daniel
[1]
http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpuidle/dt_idle_states.c#n41
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v10 2/2] media: v4l: xilinx: Add Xilinx MIPI CSI-2 Rx Subsystem driver
From: Sakari Ailus @ 2019-08-15 16:09 UTC (permalink / raw)
To: Vishal Sagar
Cc: mark.rutland, devicetree, Jacopo Mondi, Dinesh Kumar, Hyun Kwon,
Hyun Kwon, Sandip Kothari, linux-kernel, robh+dt, Michal Simek,
laurent.pinchart, Luca Ceresoli, hans.verkuil, mchehab,
linux-arm-kernel, linux-media
In-Reply-To: <20190711091612.98175-3-vishal.sagar@xilinx.com>
Hi Vishal,
Thanks for the update.
On Thu, Jul 11, 2019 at 02:46:12PM +0530, Vishal Sagar wrote:
> The Xilinx MIPI CSI-2 Rx Subsystem soft IP is used to capture images
> from MIPI CSI-2 camera sensors and output AXI4-Stream video data ready
> for image processing. Please refer to PG232 for details.
>
> The CSI2 Rx controller filters out all packets except for the packets
> with data type fixed in hardware. RAW8 packets are always allowed to
> pass through.
>
> It is also used to setup and handle interrupts and enable the core. It
> logs all the events in respective counters between streaming on and off.
>
> The driver supports only the video format bridge enabled configuration.
> Some data types like YUV 422 10bpc, RAW16, RAW20 are supported when the
> CSI v2.0 feature is enabled in design. When the VCX feature is enabled,
> the maximum number of virtual channels becomes 16 from 4.
>
> Signed-off-by: Vishal Sagar <vishal.sagar@xilinx.com>
> Reviewed-by: Hyun Kwon <hyun.kwon@xilinx.com>
> ---
> v10
> - Removed all V4L2 controls and events based on Sakari's comments.
> - Now stop_stream() before toggling rst_gpio
> - Updated init_mbus() to throw error on array out of bound access
> - Make events and vcx_events as counters instead of structures
> - Minor fixes in set_format() enum_mbus_code() as suggested by Sakari
>
> v9
> - Moved all controls and events to xilinx-csi2rxss.h
> - Updated name and description of controls and events
> - Get control base address from v4l2-controls.h (0x10c0)
> - Fix KConfig for dependency on VIDEO_XILINX
> - Added enum_mbus_code() support
> - Added default format to be returned on open()
> - Mark variables are const
> - Remove references to short packet in comments
> - Add check for streaming before setting active lanes control
> - strlcpy -> strscpy
> - Fix xcsi2rxss_set_format()
>
> v8
> - Use clk_bulk* APIs
> - Add gpio reset for asserting video_aresetn when stream line buffer occurs
> - Removed short packet related events and irq handling
> - V4L2_EVENT_XLNXCSIRX_SPKT and V4L2_EVENT_XLNXCSIRX_SPKT_OVF removed
> - Removed frame counter control V4L2_CID_XILINX_MIPICSISS_FRAME_COUNTER
> and xcsi2rxss_g_volatile_ctrl()
> - Minor formatting fixes
>
> v7
> - No change
>
> v6
> - No change
>
> v5
> - Removed bayer and updated related parts like set default format based
> on Luca Cersoli's comments.
> - Added correct YUV422 10bpc media bus format
>
> v4
> - Removed irq member from core structure
> - Consolidated IP config prints in xcsi2rxss_log_ipconfig()
> - Return -EINVAL in case of invalid ioctl
> - Code formatting
> - Added reviewed by Hyun Kwon
>
> v3
> - Fixed comments given by Hyun.
> - Removed DPHY 200 MHz clock. This will be controlled by DPHY driver
> - Minor code formatting
> - en_csi_v20 and vfb members removed from struct and made local to dt parsing
> - lock description updated
> - changed to ratelimited type for all dev prints in irq handler
> - Removed YUV 422 10bpc media format
>
> v2
> - Fixed comments given by Hyun and Sakari.
> - Made all bitmask using BIT() and GENMASK()
> - Removed unused definitions
> - Removed DPHY access. This will be done by separate DPHY PHY driver.
> - Added support for CSI v2.0 for YUV 422 10bpc, RAW16, RAW20 and extra
> virtual channels
> - Fixed the ports as sink and source
> - Now use the v4l2fwnode API to get number of data-lanes
> - Added clock framework support
> - Removed the close() function
> - updated the set format function
> - support only VFB enabled configuration
> drivers/media/platform/xilinx/Kconfig | 11 +
> drivers/media/platform/xilinx/Makefile | 1 +
> .../media/platform/xilinx/xilinx-csi2rxss.c | 1230 +++++++++++++++++
> 3 files changed, 1242 insertions(+)
> create mode 100644 drivers/media/platform/xilinx/xilinx-csi2rxss.c
>
> diff --git a/drivers/media/platform/xilinx/Kconfig b/drivers/media/platform/xilinx/Kconfig
> index a2773ad7c185..349da877c846 100644
> --- a/drivers/media/platform/xilinx/Kconfig
> +++ b/drivers/media/platform/xilinx/Kconfig
> @@ -10,6 +10,17 @@ config VIDEO_XILINX
>
> if VIDEO_XILINX
>
> +config VIDEO_XILINX_CSI2RXSS
> + tristate "Xilinx CSI2 Rx Subsystem"
> + depends on VIDEO_XILINX
You don't need to depend on VIDEO_XILINX here due to the condition above.
> + help
> + Driver for Xilinx MIPI CSI2 Rx Subsystem. This is a V4L sub-device
> + based driver that takes input from CSI2 Tx source and converts
> + it into an AXI4-Stream. The subsystem comprises of a CSI2 Rx
> + controller, DPHY, an optional I2C controller and a Video Format
> + Bridge. The driver is used to set the number of active lanes and
> + get short packet data.
> +
> config VIDEO_XILINX_TPG
> tristate "Xilinx Video Test Pattern Generator"
> depends on VIDEO_XILINX
> diff --git a/drivers/media/platform/xilinx/Makefile b/drivers/media/platform/xilinx/Makefile
> index 4cdc0b1ec7a5..6119a34f3043 100644
> --- a/drivers/media/platform/xilinx/Makefile
> +++ b/drivers/media/platform/xilinx/Makefile
> @@ -3,5 +3,6 @@
> xilinx-video-objs += xilinx-dma.o xilinx-vip.o xilinx-vipp.o
>
> obj-$(CONFIG_VIDEO_XILINX) += xilinx-video.o
> +obj-$(CONFIG_VIDEO_XILINX_CSI2RXSS) += xilinx-csi2rxss.o
> obj-$(CONFIG_VIDEO_XILINX_TPG) += xilinx-tpg.o
> obj-$(CONFIG_VIDEO_XILINX_VTC) += xilinx-vtc.o
> diff --git a/drivers/media/platform/xilinx/xilinx-csi2rxss.c b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
> new file mode 100644
> index 000000000000..1a22ca80382e
> --- /dev/null
> +++ b/drivers/media/platform/xilinx/xilinx-csi2rxss.c
> @@ -0,0 +1,1230 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for Xilinx MIPI CSI2 Rx Subsystem
> + *
> + * Copyright (C) 2016 - 2019 Xilinx, Inc.
> + *
> + * Contacts: Vishal Sagar <vishal.sagar@xilinx.com>
> + *
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/v4l2-subdev.h>
> +#include <media/media-entity.h>
> +#include <media/v4l2-common.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-subdev.h>
> +#include "xilinx-vip.h"
> +
> +/* Register register map */
> +#define XCSI_CCR_OFFSET 0x00
> +#define XCSI_CCR_SOFTRESET BIT(1)
> +#define XCSI_CCR_ENABLE BIT(0)
> +
> +#define XCSI_PCR_OFFSET 0x04
> +#define XCSI_PCR_MAXLANES_MASK GENMASK(4, 3)
> +#define XCSI_PCR_ACTLANES_MASK GENMASK(1, 0)
> +
> +#define XCSI_CSR_OFFSET 0x10
> +#define XCSI_CSR_PKTCNT GENMASK(31, 16)
> +#define XCSI_CSR_SPFIFOFULL BIT(3)
> +#define XCSI_CSR_SPFIFONE BIT(2)
> +#define XCSI_CSR_SLBF BIT(1)
> +#define XCSI_CSR_RIPCD BIT(0)
> +
> +#define XCSI_GIER_OFFSET 0x20
> +#define XCSI_GIER_GIE BIT(0)
> +
> +#define XCSI_ISR_OFFSET 0x24
> +#define XCSI_IER_OFFSET 0x28
> +
> +#define XCSI_ISR_FR BIT(31)
> +#define XCSI_ISR_VCXFE BIT(30)
> +#define XCSI_ISR_WCC BIT(22)
> +#define XCSI_ISR_ILC BIT(21)
> +#define XCSI_ISR_SPFIFOF BIT(20)
> +#define XCSI_ISR_SPFIFONE BIT(19)
> +#define XCSI_ISR_SLBF BIT(18)
> +#define XCSI_ISR_STOP BIT(17)
> +#define XCSI_ISR_SOTERR BIT(13)
> +#define XCSI_ISR_SOTSYNCERR BIT(12)
> +#define XCSI_ISR_ECC2BERR BIT(11)
> +#define XCSI_ISR_ECC1BERR BIT(10)
> +#define XCSI_ISR_CRCERR BIT(9)
> +#define XCSI_ISR_DATAIDERR BIT(8)
> +#define XCSI_ISR_VC3FSYNCERR BIT(7)
> +#define XCSI_ISR_VC3FLVLERR BIT(6)
> +#define XCSI_ISR_VC2FSYNCERR BIT(5)
> +#define XCSI_ISR_VC2FLVLERR BIT(4)
> +#define XCSI_ISR_VC1FSYNCERR BIT(3)
> +#define XCSI_ISR_VC1FLVLERR BIT(2)
> +#define XCSI_ISR_VC0FSYNCERR BIT(1)
> +#define XCSI_ISR_VC0FLVLERR BIT(0)
> +
> +#define XCSI_INTR_PROT_MASK (XCSI_ISR_VC3FSYNCERR | XCSI_ISR_VC3FLVLERR |\
> + XCSI_ISR_VC2FSYNCERR | XCSI_ISR_VC2FLVLERR |\
> + XCSI_ISR_VC1FSYNCERR | XCSI_ISR_VC1FLVLERR |\
> + XCSI_ISR_VC0FSYNCERR | XCSI_ISR_VC0FLVLERR |\
> + XCSI_ISR_VCXFE)
> +
> +#define XCSI_INTR_PKTLVL_MASK (XCSI_ISR_ECC2BERR | XCSI_ISR_ECC1BERR |\
> + XCSI_ISR_CRCERR | XCSI_ISR_DATAIDERR)
> +
> +#define XCSI_INTR_DPHY_MASK (XCSI_ISR_SOTERR | XCSI_ISR_SOTSYNCERR)
> +
> +#define XCSI_INTR_SPKT_MASK (XCSI_ISR_SPFIFOF | XCSI_ISR_SPFIFONE)
> +
> +#define XCSI_INTR_ERR_MASK (XCSI_ISR_WCC | XCSI_ISR_ILC | XCSI_ISR_SLBF |\
> + XCSI_ISR_STOP)
> +
> +#define XCSI_INTR_FRAMERCVD_MASK (XCSI_ISR_FR)
> +
> +#define XCSI_ISR_ALLINTR_MASK (XCSI_INTR_PROT_MASK | XCSI_INTR_PKTLVL_MASK |\
> + XCSI_INTR_DPHY_MASK | XCSI_INTR_SPKT_MASK |\
> + XCSI_INTR_ERR_MASK | XCSI_INTR_FRAMERCVD_MASK)
> +
> +/*
> + * Removed VCXFE mask as it doesn't exist in IER
> + * Removed STOP state irq as this will keep driver in irq handler only
> + */
> +#define XCSI_IER_INTR_MASK (XCSI_ISR_ALLINTR_MASK &\
> + ~(XCSI_ISR_STOP | XCSI_ISR_VCXFE))
> +
> +#define XCSI_SPKTR_OFFSET 0x30
> +#define XCSI_SPKTR_DATA GENMASK(23, 8)
> +#define XCSI_SPKTR_VC GENMASK(7, 6)
> +#define XCSI_SPKTR_DT GENMASK(5, 0)
> +
> +#define XCSI_VCXR_OFFSET 0x34
> +#define XCSI_VCXR_VCERR GENMASK(23, 0)
> +#define XCSI_VCXR_VCSTART 4
> +#define XCSI_VCXR_VCEND 15
> +#define XCSI_VCXR_FSYNCERR BIT(1)
> +#define XCSI_VCXR_FLVLERR BIT(0)
> +
> +#define XCSI_CLKINFR_OFFSET 0x3C
> +#define XCSI_CLKINFR_STOP BIT(1)
> +
> +#define XCSI_DLXINFR_OFFSET 0x40
> +#define XCSI_DLXINFR_STOP BIT(5)
> +#define XCSI_DLXINFR_SOTERR BIT(1)
> +#define XCSI_DLXINFR_SOTSYNCERR BIT(0)
> +#define XCSI_MAXDL_COUNT 0x4
> +
> +#define XCSI_VCXINF1R_OFFSET 0x60
> +#define XCSI_VCXINF1R_LINECOUNT GENMASK(31, 16)
> +#define XCSI_VCXINF1R_LINECOUNT_SHIFT 16
> +#define XCSI_VCXINF1R_BYTECOUNT GENMASK(15, 0)
> +
> +#define XCSI_VCXINF2R_OFFSET 0x64
> +#define XCSI_VCXINF2R_DT GENMASK(5, 0)
> +#define XCSI_MAXVCX_COUNT 16
> +
> +/*
> + * The core takes less than 100 video clock cycles to reset.
> + * So choosing a timeout value larger than this.
> + */
> +#define XCSI_TIMEOUT_VAL 1000 /* us */
> +
> +/*
> + * Sink pad connected to sensor source pad.
> + * Source pad connected to next module like demosaic.
> + */
> +#define XCSI_MEDIA_PADS 2
> +#define XCSI_DEFAULT_WIDTH 1920
> +#define XCSI_DEFAULT_HEIGHT 1080
> +
> +/* Max media bus formats supported for enumeration */
> +#define XCSI_MAX_MBUS_FMTS 16
> +
> +/* Max string length for CSI Data type string */
> +#define XCSI_PXLFMT_STRLEN_MAX 16
> +
> +/* MIPI CSI2 Data Types from spec */
> +#define XCSI_DT_YUV4228B 0x1E
> +#define XCSI_DT_YUV42210B 0x1F
> +#define XCSI_DT_RGB444 0x20
> +#define XCSI_DT_RGB555 0x21
> +#define XCSI_DT_RGB565 0x22
> +#define XCSI_DT_RGB666 0x23
> +#define XCSI_DT_RGB888 0x24
> +#define XCSI_DT_RAW6 0x28
> +#define XCSI_DT_RAW7 0x29
> +#define XCSI_DT_RAW8 0x2A
> +#define XCSI_DT_RAW10 0x2B
> +#define XCSI_DT_RAW12 0x2C
> +#define XCSI_DT_RAW14 0x2D
> +#define XCSI_DT_RAW16 0x2E
> +#define XCSI_DT_RAW20 0x2F
> +
> +#define XCSI_VCX_START 4
> +#define XCSI_MAX_VC 4
> +#define XCSI_MAX_VCX 16
> +
> +#define XCSI_NEXTREG_OFFSET 4
> +
> +/* There are 2 events frame sync and frame level error per VC */
> +#define XCSI_VCX_NUM_EVENTS ((XCSI_MAX_VCX - XCSI_MAX_VC) * 2)
> +
> +/* Macro to return "true" or "false" string if bit is set */
> +#define XCSI_GET_BITSET_STR(val, mask) (val) & (mask) ? "true" : "false"
> +
> +#define XADD_MBUS(state, mbus_fmt) \
> + do { \
> + if ((state)->mbus_fmts_count < XCSI_MAX_MBUS_FMTS) { \
> + (state)->mbus_fmts[(state)->mbus_fmts_count++] =\
> + (mbus_fmt); \
> + } else { \
> + dev_err((state)->core.dev, \
> + "accessing array out of bounds!\n"); \
> + } \
> + } while (0)
> +
> +/**
> + * struct xcsi2rxss_event - Event log structure
> + * @mask: Event mask
> + * @name: Name of the event
> + */
> +struct xcsi2rxss_event {
> + u32 mask;
> + const char *name;
> +};
> +
> +static const struct xcsi2rxss_event xcsi2rxss_events[] = {
> + { XCSI_ISR_FR, "Frame Received" },
> + { XCSI_ISR_VCXFE, "VCX Frame Errors" },
> + { XCSI_ISR_WCC, "Word Count Errors" },
> + { XCSI_ISR_ILC, "Invalid Lane Count Error" },
> + { XCSI_ISR_SPFIFOF, "Short Packet FIFO OverFlow Error" },
> + { XCSI_ISR_SPFIFONE, "Short Packet FIFO Not Empty" },
> + { XCSI_ISR_SLBF, "Streamline Buffer Full Error" },
> + { XCSI_ISR_STOP, "Lane Stop State" },
> + { XCSI_ISR_SOTERR, "SOT Error" },
> + { XCSI_ISR_SOTSYNCERR, "SOT Sync Error" },
> + { XCSI_ISR_ECC2BERR, "2 Bit ECC Unrecoverable Error" },
> + { XCSI_ISR_ECC1BERR, "1 Bit ECC Recoverable Error" },
> + { XCSI_ISR_CRCERR, "CRC Error" },
> + { XCSI_ISR_DATAIDERR, "Data Id Error" },
> + { XCSI_ISR_VC3FSYNCERR, "Virtual Channel 3 Frame Sync Error" },
> + { XCSI_ISR_VC3FLVLERR, "Virtual Channel 3 Frame Level Error" },
> + { XCSI_ISR_VC2FSYNCERR, "Virtual Channel 2 Frame Sync Error" },
> + { XCSI_ISR_VC2FLVLERR, "Virtual Channel 2 Frame Level Error" },
> + { XCSI_ISR_VC1FSYNCERR, "Virtual Channel 1 Frame Sync Error" },
> + { XCSI_ISR_VC1FLVLERR, "Virtual Channel 1 Frame Level Error" },
> + { XCSI_ISR_VC0FSYNCERR, "Virtual Channel 0 Frame Sync Error" },
> + { XCSI_ISR_VC0FLVLERR, "Virtual Channel 0 Frame Level Error" }
> +};
> +
> +#define XCSI_NUM_EVENTS ARRAY_SIZE(xcsi2rxss_events)
> +
> +/*
> + * struct xcsi2rxss_core - Core configuration CSI2 Rx Subsystem device structure
> + * @dev: Platform structure
> + * @iomem: Base address of subsystem
> + * @enable_active_lanes: If number of active lanes can be modified
> + * @max_num_lanes: Maximum number of lanes present
> + * @datatype: Data type filter
> + * @events: counter for events
> + * @vcx_events: counter for vcx_events
> + * @en_vcx: If more than 4 VC are enabled
> + * @clks: array of clocks
> + * @num_clks: number of clocks
> + * @rst_gpio: reset to video_aresetn
> + */
> +struct xcsi2rxss_core {
> + struct device *dev;
> + void __iomem *iomem;
> + bool enable_active_lanes;
> + u32 max_num_lanes;
> + u32 datatype;
> + u32 events[XCSI_NUM_EVENTS];
> + u32 vcx_events[XCSI_VCX_NUM_EVENTS];
> + bool en_vcx;
> + struct clk_bulk_data *clks;
> + int num_clks;
> + struct gpio_desc *rst_gpio;
> +};
> +
> +/**
> + * struct xcsi2rxss_state - CSI2 Rx Subsystem device structure
> + * @core: Core structure for MIPI CSI2 Rx Subsystem
> + * @subdev: The v4l2 subdev structure
> + * @format: Active V4L2 formats on each pad
> + * @default_format: Default V4L2 format
> + * @lock: mutex for accessing this structure
> + * @pads: media pads
> + * @mbus_fmts: List of media bus formats for enum_mbus_code
> + * @mbus_fmts_count: Number of media bus formats
> + * @streaming: Flag for storing streaming state
> + *
> + * This structure contains the device driver related parameters
> + */
> +struct xcsi2rxss_state {
> + struct xcsi2rxss_core core;
> + struct v4l2_subdev subdev;
> + struct v4l2_mbus_framefmt format;
> + struct v4l2_mbus_framefmt default_format;
> + /* used to protect access to this struct */
> + struct mutex lock;
> + struct media_pad pads[XCSI_MEDIA_PADS];
> + u32 mbus_fmts[XCSI_MAX_MBUS_FMTS];
> + u32 mbus_fmts_count;
> + bool streaming;
> +};
> +
> +static const struct clk_bulk_data xcsi2rxss_clks[] = {
> + { .id = "lite_aclk" },
> + { .id = "video_aclk" },
> +};
> +
> +static inline struct xcsi2rxss_state *
> +to_xcsi2rxssstate(struct v4l2_subdev *subdev)
> +{
> + return container_of(subdev, struct xcsi2rxss_state, subdev);
> +}
> +
> +/*
> + * Register related operations
> + */
> +static inline u32 xcsi2rxss_read(struct xcsi2rxss_core *xcsi2rxss, u32 addr)
> +{
> + return ioread32(xcsi2rxss->iomem + addr);
> +}
> +
> +static inline void xcsi2rxss_write(struct xcsi2rxss_core *xcsi2rxss, u32 addr,
> + u32 value)
> +{
> + iowrite32(value, xcsi2rxss->iomem + addr);
> +}
> +
> +static inline void xcsi2rxss_clr(struct xcsi2rxss_core *xcsi2rxss, u32 addr,
> + u32 clr)
> +{
> + xcsi2rxss_write(xcsi2rxss, addr,
> + xcsi2rxss_read(xcsi2rxss, addr) & ~clr);
> +}
> +
> +static inline void xcsi2rxss_set(struct xcsi2rxss_core *xcsi2rxss, u32 addr,
> + u32 set)
> +{
> + xcsi2rxss_write(xcsi2rxss, addr,
> + xcsi2rxss_read(xcsi2rxss, addr) | set);
Fits on a single line.
> +}
> +
> +static void xcsi2rxss_enable(struct xcsi2rxss_core *core)
> +{
> + xcsi2rxss_set(core, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE);
> +}
> +
> +static void xcsi2rxss_disable(struct xcsi2rxss_core *core)
> +{
> + xcsi2rxss_clr(core, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE);
> +}
> +
> +static void xcsi2rxss_intr_enable(struct xcsi2rxss_core *core)
> +{
> + xcsi2rxss_clr(core, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> + xcsi2rxss_write(core, XCSI_IER_OFFSET, XCSI_IER_INTR_MASK);
> + xcsi2rxss_set(core, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> +}
> +
> +static void xcsi2rxss_intr_disable(struct xcsi2rxss_core *core)
> +{
> + xcsi2rxss_clr(core, XCSI_IER_OFFSET, XCSI_IER_INTR_MASK);
> + xcsi2rxss_clr(core, XCSI_GIER_OFFSET, XCSI_GIER_GIE);
> +}
> +
> +/**
> + * xcsi2rxss_reset - Does a soft reset of the MIPI CSI2 Rx Subsystem
> + * @core: Core Xilinx CSI2 Rx Subsystem structure pointer
> + *
> + * Core takes less than 100 video clock cycles to reset.
> + * So a larger timeout value is chosen for margin.
> + *
> + * Return: 0 - on success OR -ETIME if reset times out
> + */
> +static int xcsi2rxss_reset(struct xcsi2rxss_core *core)
> +{
> + u32 timeout = XCSI_TIMEOUT_VAL;
> +
> + xcsi2rxss_set(core, XCSI_CCR_OFFSET, XCSI_CCR_SOFTRESET);
> +
> + while (xcsi2rxss_read(core, XCSI_CSR_OFFSET) & XCSI_CSR_RIPCD) {
> + if (timeout == 0) {
> + dev_err(core->dev, "soft reset timed out!\n");
> + return -ETIME;
> + }
> +
> + timeout--;
> + udelay(1);
> + }
> +
> + xcsi2rxss_clr(core, XCSI_CCR_OFFSET, XCSI_CCR_SOFTRESET);
> + return 0;
> +}
> +
> +static void xcsi2rxss_reset_event_counters(struct xcsi2rxss_state *state)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < XCSI_NUM_EVENTS; i++)
> + state->core.events[i] = 0;
> +
> + for (i = 0; i < XCSI_VCX_NUM_EVENTS; i++)
> + state->core.vcx_events[i] = 0;
> +}
> +
> +/* Print event counters */
> +static void xcsi2rxss_log_counters(struct xcsi2rxss_state *state)
> +{
> + struct xcsi2rxss_core *core = &state->core;
> + unsigned int i;
> +
> + for (i = 0; i < XCSI_NUM_EVENTS; i++) {
> + if (core->events[i] > 0) {
> + dev_info(core->dev, "%s events: %d\n",
> + xcsi2rxss_events[i].name,
> + core->events[i]);
> + }
> + }
> +
> + if (core->en_vcx) {
> + for (i = 0; i < XCSI_VCX_NUM_EVENTS; i++) {
> + if (core->vcx_events[i] > 0) {
> + dev_info(core->dev,
> + "VC %d Frame %s err vcx events: %d\n",
> + (i / 2) + XCSI_VCX_START,
> + i & 1 ? "Sync" : "Level",
> + core->vcx_events[i]);
> + }
> + }
> + }
> +}
> +
> +static void xcsi2rxss_log_ipconfig(struct xcsi2rxss_state *state)
> +{
> + struct xcsi2rxss_core *core = &state->core;
> +
> + dev_dbg(core->dev, "****** Xilinx MIPI CSI2 Rx SS IP Config ******\n");
> + dev_dbg(core->dev, "vcx is %s", core->en_vcx ? "enabled" : "disabled");
> + dev_dbg(core->dev, "Enable active lanes property is %s\n",
> + core->enable_active_lanes ? "present" : "absent");
> + dev_dbg(core->dev, "Max lanes = %d", core->max_num_lanes);
> + dev_dbg(core->dev, "Pixel format set as 0x%x\n", core->datatype);
> + dev_dbg(core->dev, "**********************************************\n");
> +}
> +
> +/**
> + * xcsi2rxss_log_status - Logs the status of the CSI-2 Receiver
> + * @sd: Pointer to V4L2 subdevice structure
> + *
> + * This function prints the current status of Xilinx MIPI CSI-2
> + *
> + * Return: 0 on success
> + */
> +static int xcsi2rxss_log_status(struct v4l2_subdev *sd)
> +{
> + struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> + struct xcsi2rxss_core *core = &xcsi2rxss->core;
> + u32 reg, data;
> + unsigned int i, max_vc;
> +
> + mutex_lock(&xcsi2rxss->lock);
> +
> + xcsi2rxss_log_ipconfig(xcsi2rxss);
> +
> + xcsi2rxss_log_counters(xcsi2rxss);
> +
> + dev_info(core->dev, "***** Core Status *****\n");
> + data = xcsi2rxss_read(core, XCSI_CSR_OFFSET);
> + dev_info(core->dev, "Short Packet FIFO Full = %s\n",
> + XCSI_GET_BITSET_STR(data, XCSI_CSR_SPFIFOFULL));
> + dev_info(core->dev, "Short Packet FIFO Not Empty = %s\n",
> + XCSI_GET_BITSET_STR(data, XCSI_CSR_SPFIFONE));
> + dev_info(core->dev, "Stream line buffer full = %s\n",
> + XCSI_GET_BITSET_STR(data, XCSI_CSR_SLBF));
> + dev_info(core->dev, "Soft reset/Core disable in progress = %s\n",
> + XCSI_GET_BITSET_STR(data, XCSI_CSR_RIPCD));
> +
> + /* Clk & Lane Info */
> + dev_info(core->dev, "******** Clock Lane Info *********\n");
> + data = xcsi2rxss_read(core, XCSI_CLKINFR_OFFSET);
> + dev_info(core->dev, "Clock Lane in Stop State = %s\n",
> + XCSI_GET_BITSET_STR(data, XCSI_CLKINFR_STOP));
> +
> + dev_info(core->dev, "******** Data Lane Info *********\n");
> + dev_info(core->dev, "Lane\tSoT Error\tSoT Sync Error\tStop State\n");
> + reg = XCSI_DLXINFR_OFFSET;
> + for (i = 0; i < XCSI_MAXDL_COUNT; i++) {
> + data = xcsi2rxss_read(core, reg);
> +
> + dev_info(core->dev, "%d\t%s\t\t%s\t\t%s\n", i,
> + XCSI_GET_BITSET_STR(data, XCSI_DLXINFR_SOTERR),
> + XCSI_GET_BITSET_STR(data, XCSI_DLXINFR_SOTSYNCERR),
> + XCSI_GET_BITSET_STR(data, XCSI_DLXINFR_STOP));
> +
> + reg += XCSI_NEXTREG_OFFSET;
> + }
> +
> + /* Virtual Channel Image Information */
> + dev_info(core->dev, "********** Virtual Channel Info ************\n");
> + dev_info(core->dev, "VC\tLine Count\tByte Count\tData Type\n");
> + if (core->en_vcx)
> + max_vc = XCSI_MAX_VCX;
> + else
> + max_vc = XCSI_MAX_VC;
> +
> + reg = XCSI_VCXINF1R_OFFSET;
> + for (i = 0; i < max_vc; i++) {
> + u32 line_count, byte_count, data_type;
> +
> + /* Get line and byte count from VCXINFR1 Register */
> + data = xcsi2rxss_read(core, reg);
> + byte_count = data & XCSI_VCXINF1R_BYTECOUNT;
> + line_count = data & XCSI_VCXINF1R_LINECOUNT;
> + line_count >>= XCSI_VCXINF1R_LINECOUNT_SHIFT;
> +
> + /* Get data type from VCXINFR2 Register */
> + reg += XCSI_NEXTREG_OFFSET;
> + data = xcsi2rxss_read(core, reg);
> + data_type = data & XCSI_VCXINF2R_DT;
> +
> + dev_info(core->dev, "%d\t%d\t\t%d\t\t0x%x\n", i, line_count,
> + byte_count, data_type);
> +
> + /* Move to next pair of VC Info registers */
> + reg += XCSI_NEXTREG_OFFSET;
> + }
> +
> + mutex_unlock(&xcsi2rxss->lock);
> +
> + return 0;
> +}
> +
> +static int xcsi2rxss_start_stream(struct xcsi2rxss_state *state)
> +{
> + struct xcsi2rxss_core *core = &state->core;
> + int ret = 0;
> +
> + xcsi2rxss_enable(core);
> +
> + ret = xcsi2rxss_reset(core);
> + if (ret < 0) {
> + state->streaming = false;
> + return ret;
> + }
You'll need to start streaming on the upstream sub-device somewhere in this
function.
> +
> + xcsi2rxss_intr_enable(core);
> + state->streaming = true;
> +
> + return ret;
> +}
> +
> +static void xcsi2rxss_stop_stream(struct xcsi2rxss_state *state)
> +{
> + struct xcsi2rxss_core *core = &state->core;
> +
> + xcsi2rxss_intr_disable(core);
> + xcsi2rxss_disable(core);
> + state->streaming = false;
And stop it here.
> +}
> +
> +/**
> + * xcsi2rxss_irq_handler - Interrupt handler for CSI-2
> + * @irq: IRQ number
> + * @dev_id: Pointer to device state
> + *
> + * In the interrupt handler, a list of event counters are updated for
> + * corresponding interrupts. This is useful to get status / debug.
> + *
> + * Return: IRQ_HANDLED after handling interrupts
> + * IRQ_NONE is no interrupts
> + */
> +static irqreturn_t xcsi2rxss_irq_handler(int irq, void *dev_id)
> +{
> + struct xcsi2rxss_state *state = (struct xcsi2rxss_state *)dev_id;
> + struct xcsi2rxss_core *core = &state->core;
> + u32 status;
> +
> + status = xcsi2rxss_read(core, XCSI_ISR_OFFSET) & XCSI_ISR_ALLINTR_MASK;
> + dev_dbg_ratelimited(core->dev, "interrupt status = 0x%08x\n", status);
> +
> + if (!status)
> + return IRQ_NONE;
> +
> + /* Received a short packet */
> + if (status & XCSI_ISR_SPFIFONE) {
> + dev_dbg_ratelimited(core->dev, "Short packet = 0x%08x\n",
> + xcsi2rxss_read(core, XCSI_SPKTR_OFFSET));
> + }
> +
> + /* Short packet FIFO overflow */
> + if (status & XCSI_ISR_SPFIFOF)
> + dev_dbg_ratelimited(core->dev, "Short packet FIFO overflowed\n");
> +
> + /*
> + * Stream line buffer full
> + * This means there is a backpressure from downstream IP
> + */
> + if (status & XCSI_ISR_SLBF) {
> + dev_alert_ratelimited(core->dev, "Stream Line Buffer Full!\n");
> + xcsi2rxss_stop_stream(state);
> + if (core->rst_gpio) {
> + gpiod_set_value(core->rst_gpio, 1);
> + /* minimum 40 dphy_clk_200M cycles */
> + ndelay(250);
> + gpiod_set_value(core->rst_gpio, 0);
> + }
> + }
> +
> + /* Increment event counters */
> + if (status & XCSI_ISR_ALLINTR_MASK) {
> + unsigned int i;
> +
> + for (i = 0; i < XCSI_NUM_EVENTS; i++) {
> + if (!(status & xcsi2rxss_events[i].mask))
> + continue;
> + core->events[i]++;
> + dev_dbg_ratelimited(core->dev, "%s: %d\n",
> + xcsi2rxss_events[i].name,
> + core->events[i]);
> + }
> +
> + if (status & XCSI_ISR_VCXFE && core->en_vcx) {
> + u32 vcxstatus;
> +
> + vcxstatus = xcsi2rxss_read(core, XCSI_VCXR_OFFSET);
> + vcxstatus &= XCSI_VCXR_VCERR;
> + for (i = 0; i < XCSI_VCX_NUM_EVENTS; i++) {
> + if (!(vcxstatus & (1 << i)))
> + continue;
> + core->vcx_events[i]++;
> + }
> + xcsi2rxss_write(core, XCSI_VCXR_OFFSET, vcxstatus);
> + }
> + }
> +
> + xcsi2rxss_write(core, XCSI_ISR_OFFSET, status);
> + return IRQ_HANDLED;
> +}
> +
> +/**
> + * xcsi2rxss_s_stream - It is used to start/stop the streaming.
> + * @sd: V4L2 Sub device
> + * @enable: Flag (True / False)
> + *
> + * This function controls the start or stop of streaming for the
> + * Xilinx MIPI CSI-2 Rx Subsystem.
> + *
> + * Return: 0 on success, errors otherwise
> + */
> +static int xcsi2rxss_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> + int ret = 0;
> +
> + mutex_lock(&xcsi2rxss->lock);
> +
> + if (enable) {
> + if (!xcsi2rxss->streaming) {
> + /* reset the event counters */
> + xcsi2rxss_reset_event_counters(xcsi2rxss);
> + ret = xcsi2rxss_start_stream(xcsi2rxss);
> + }
> + } else {
> + if (xcsi2rxss->streaming) {
> + struct gpio_desc *rst = xcsi2rxss->core.rst_gpio;
> +
> + xcsi2rxss_stop_stream(xcsi2rxss);
> + if (rst) {
> + gpiod_set_value_cansleep(rst, 1);
> + usleep_range(1, 2);
> + gpiod_set_value_cansleep(rst, 0);
> + }
> + }
> + }
> +
> + mutex_unlock(&xcsi2rxss->lock);
> + return ret;
> +}
> +
> +static struct v4l2_mbus_framefmt *
> +__xcsi2rxss_get_pad_format(struct xcsi2rxss_state *xcsi2rxss,
> + struct v4l2_subdev_pad_config *cfg,
> + unsigned int pad, u32 which)
> +{
> + switch (which) {
> + case V4L2_SUBDEV_FORMAT_TRY:
> + return v4l2_subdev_get_try_format(&xcsi2rxss->subdev, cfg, pad);
> + case V4L2_SUBDEV_FORMAT_ACTIVE:
> + return &xcsi2rxss->format;
> + default:
> + return NULL;
> + }
> +}
> +
> +/**
> + * xcsi2rxss_get_format - Get the pad format
> + * @sd: Pointer to V4L2 Sub device structure
> + * @cfg: Pointer to sub device pad information structure
> + * @fmt: Pointer to pad level media bus format
> + *
> + * This function is used to get the pad format information.
> + *
> + * Return: 0 on success
> + */
> +static int xcsi2rxss_get_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> +
> + mutex_lock(&xcsi2rxss->lock);
> + fmt->format = *__xcsi2rxss_get_pad_format(xcsi2rxss, cfg, fmt->pad,
> + fmt->which);
> + mutex_unlock(&xcsi2rxss->lock);
> +
> + return 0;
> +}
> +
> +/**
> + * xcsi2rxss_set_format - This is used to set the pad format
> + * @sd: Pointer to V4L2 Sub device structure
> + * @cfg: Pointer to sub device pad information structure
> + * @fmt: Pointer to pad level media bus format
> + *
> + * This function is used to set the pad format. Since the pad format is fixed
> + * in hardware, it can't be modified on run time. So when a format set is
> + * requested by application, all parameters except the format type is saved
> + * for the pad and the original pad format is sent back to the application.
> + *
> + * Return: 0 on success
> + */
> +static int xcsi2rxss_set_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> + struct xcsi2rxss_core *core = &xcsi2rxss->core;
> + struct v4l2_mbus_framefmt *__format;
> +
> + /* only sink pad format can be updated */
> + mutex_lock(&xcsi2rxss->lock);
> +
> + /*
> + * Only the format->code parameter matters for CSI as the
> + * CSI format cannot be changed at runtime.
> + * Ensure that format to set is copied to over to CSI pad format
> + */
> + __format = __xcsi2rxss_get_pad_format(xcsi2rxss, cfg,
> + fmt->pad, fmt->which);
> +
> + if (fmt->pad == XVIP_PAD_SOURCE) {
> + fmt->format = *__format;
> + mutex_unlock(&xcsi2rxss->lock);
> + return 0;
> + }
> +
> + /*
> + * RAW8 is supported in all datatypes. So if requested media bus format
> + * is of RAW8 type, then allow to be set. In case core is configured to
> + * other RAW, YUV422 8/10 or RGB888, set appropriate media bus format.
> + */
> + if (!((fmt->format.code == MEDIA_BUS_FMT_SBGGR8_1X8 ||
> + fmt->format.code == MEDIA_BUS_FMT_SGBRG8_1X8 ||
> + fmt->format.code == MEDIA_BUS_FMT_SGRBG8_1X8 ||
> + fmt->format.code == MEDIA_BUS_FMT_SRGGB8_1X8) ||
> + (core->datatype == XCSI_DT_RAW10 &&
> + (fmt->format.code == MEDIA_BUS_FMT_SBGGR10_1X10 ||
> + fmt->format.code == MEDIA_BUS_FMT_SGBRG10_1X10 ||
> + fmt->format.code == MEDIA_BUS_FMT_SGRBG10_1X10 ||
> + fmt->format.code == MEDIA_BUS_FMT_SRGGB10_1X10)) ||
> + (core->datatype == XCSI_DT_RAW12 &&
> + (fmt->format.code == MEDIA_BUS_FMT_SBGGR12_1X12 ||
> + fmt->format.code == MEDIA_BUS_FMT_SGBRG12_1X12 ||
> + fmt->format.code == MEDIA_BUS_FMT_SGRBG12_1X12 ||
> + fmt->format.code == MEDIA_BUS_FMT_SRGGB12_1X12)) ||
> + (core->datatype == XCSI_DT_RAW14 &&
> + (fmt->format.code == MEDIA_BUS_FMT_SBGGR14_1X14 ||
> + fmt->format.code == MEDIA_BUS_FMT_SGBRG14_1X14 ||
> + fmt->format.code == MEDIA_BUS_FMT_SGRBG14_1X14 ||
> + fmt->format.code == MEDIA_BUS_FMT_SRGGB14_1X14)) ||
> + (core->datatype == XCSI_DT_RAW16 &&
> + (fmt->format.code == MEDIA_BUS_FMT_SBGGR16_1X16 ||
> + fmt->format.code == MEDIA_BUS_FMT_SGBRG16_1X16 ||
> + fmt->format.code == MEDIA_BUS_FMT_SGRBG16_1X16 ||
> + fmt->format.code == MEDIA_BUS_FMT_SRGGB16_1X16)) ||
> + (core->datatype == XCSI_DT_YUV4228B &&
> + fmt->format.code == MEDIA_BUS_FMT_UYVY8_1X16) ||
> + (core->datatype == XCSI_DT_YUV42210B &&
> + fmt->format.code == MEDIA_BUS_FMT_UYVY10_1X20) ||
> + (core->datatype == XCSI_DT_RGB888 &&
> + fmt->format.code == MEDIA_BUS_FMT_RBG888_1X24))) {
> + /* Restore the original pad format code */
> + dev_dbg(core->dev, "Unsupported media bus format");
> + fmt->format.code = __format->code;
> + }
> +
> + *__format = fmt->format;
Did I already ask whether there are any limits on width or height for this
device?
> + mutex_unlock(&xcsi2rxss->lock);
> +
> + return 0;
> +}
> +
> +/*
> + * xcsi2rxss_enum_mbus_code - Handle pixel format enumeration
> + * @sd : pointer to v4l2 subdev structure
> + * @cfg: V4L2 subdev pad configuration
> + * @code : pointer to v4l2_subdev_mbus_code_enum structure
> + *
> + * Return: -EINVAL or zero on success
> + */
> +int xcsi2rxss_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + struct xcsi2rxss_state *state = to_xcsi2rxssstate(sd);
> +
> + if (code->index >= state->mbus_fmts_count)
> + return -EINVAL;
> +
> + code->code = state->mbus_fmts[code->index];
> +
> + return 0;
> +}
> +
> +/**
> + * xcsi2rxss_open - Called on v4l2_open()
> + * @sd: Pointer to V4L2 sub device structure
> + * @fh: Pointer to V4L2 File handle
> + *
> + * This function is called on v4l2_open(). It sets the default format
> + * for both pads.
> + *
> + * Return: 0 on success
> + */
> +static int xcsi2rxss_open(struct v4l2_subdev *sd,
> + struct v4l2_subdev_fh *fh)
> +{
> + struct xcsi2rxss_state *xcsi2rxss = to_xcsi2rxssstate(sd);
> + struct v4l2_mbus_framefmt *format;
> + unsigned int i;
> +
> + for (i = 0; i < XCSI_MEDIA_PADS; i++) {
> + format = v4l2_subdev_get_try_format(sd, fh->pad, i);
> + *format = xcsi2rxss->default_format;
> + }
> +
> + return 0;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Media Operations
> + */
> +
> +static const struct media_entity_operations xcsi2rxss_media_ops = {
> + .link_validate = v4l2_subdev_link_validate
> +};
> +
> +static const struct v4l2_subdev_core_ops xcsi2rxss_core_ops = {
> + .log_status = xcsi2rxss_log_status,
> +};
> +
> +static const struct v4l2_subdev_video_ops xcsi2rxss_video_ops = {
> + .s_stream = xcsi2rxss_s_stream
> +};
> +
> +static const struct v4l2_subdev_pad_ops xcsi2rxss_pad_ops = {
> + .get_fmt = xcsi2rxss_get_format,
> + .set_fmt = xcsi2rxss_set_format,
> + .enum_mbus_code = xcsi2rxss_enum_mbus_code,
You'll need link_validate set to v4l2_subdev_link_validate_default here.
> +};
> +
> +static const struct v4l2_subdev_ops xcsi2rxss_ops = {
> + .core = &xcsi2rxss_core_ops,
> + .video = &xcsi2rxss_video_ops,
> + .pad = &xcsi2rxss_pad_ops
> +};
> +
> +static const struct v4l2_subdev_internal_ops xcsi2rxss_internal_ops = {
> + .open = xcsi2rxss_open,
> +};
> +
> +static void xcsi2rxss_set_default_format(struct xcsi2rxss_state *state)
> +{
> + struct xcsi2rxss_core *core = &state->core;
> +
> + memset(&state->default_format, 0, sizeof(state->default_format));
> +
> + switch (core->datatype) {
> + case XCSI_DT_YUV4228B:
> + state->default_format.code = MEDIA_BUS_FMT_UYVY8_1X16;
> + break;
> + case XCSI_DT_RGB888:
> + state->default_format.code = MEDIA_BUS_FMT_RBG888_1X24;
> + break;
> + case XCSI_DT_YUV42210B:
> + state->default_format.code = MEDIA_BUS_FMT_UYVY10_1X20;
> + break;
> + case XCSI_DT_RAW10:
> + state->default_format.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> + break;
> + case XCSI_DT_RAW12:
> + state->default_format.code = MEDIA_BUS_FMT_SRGGB12_1X12;
> + break;
> + case XCSI_DT_RAW14:
> + state->default_format.code = MEDIA_BUS_FMT_SRGGB14_1X14;
> + break;
> + case XCSI_DT_RAW16:
> + state->default_format.code = MEDIA_BUS_FMT_SRGGB16_1X16;
> + break;
> + case XCSI_DT_RAW8:
> + case XCSI_DT_RGB444:
> + case XCSI_DT_RGB555:
> + case XCSI_DT_RGB565:
> + case XCSI_DT_RGB666:
> + state->default_format.code = MEDIA_BUS_FMT_SRGGB8_1X8;
> + break;
> + }
> +
> + state->default_format.field = V4L2_FIELD_NONE;
> + state->default_format.colorspace = V4L2_COLORSPACE_SRGB;
> + state->default_format.width = XCSI_DEFAULT_WIDTH;
> + state->default_format.height = XCSI_DEFAULT_HEIGHT;
> +
> + dev_dbg(core->dev, "default mediabus format = 0x%x",
> + state->default_format.code);
> +}
> +
> +static void xcsi2rxss_init_mbus_fmts(struct xcsi2rxss_state *state)
> +{
> + struct xcsi2rxss_core *core = &state->core;
> +
> + XADD_MBUS(state, MEDIA_BUS_FMT_SRGGB8_1X8);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SBGGR8_1X8);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SGRBG8_1X8);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SGBRG8_1X8);
> +
> + switch (core->datatype) {
> + case XCSI_DT_RAW10:
> + XADD_MBUS(state, MEDIA_BUS_FMT_SRGGB10_1X10);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SBGGR10_1X10);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SGRBG10_1X10);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SGBRG10_1X10);
> + break;
> + case XCSI_DT_RAW12:
> + XADD_MBUS(state, MEDIA_BUS_FMT_SRGGB12_1X12);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SBGGR12_1X12);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SGRBG12_1X12);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SGBRG12_1X12);
> + break;
> + case XCSI_DT_RAW14:
> + XADD_MBUS(state, MEDIA_BUS_FMT_SRGGB14_1X14);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SBGGR14_1X14);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SGRBG14_1X14);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SGBRG14_1X14);
> + break;
> + case XCSI_DT_RAW16:
> + XADD_MBUS(state, MEDIA_BUS_FMT_SRGGB16_1X16);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SBGGR16_1X16);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SGRBG16_1X16);
> + XADD_MBUS(state, MEDIA_BUS_FMT_SGBRG16_1X16);
> + break;
> + case XCSI_DT_YUV4228B:
> + XADD_MBUS(state, MEDIA_BUS_FMT_UYVY8_1X16);
> + break;
> + case XCSI_DT_RGB888:
> + XADD_MBUS(state, MEDIA_BUS_FMT_RBG888_1X24);
> + break;
> + case XCSI_DT_YUV42210B:
> + XADD_MBUS(state, MEDIA_BUS_FMT_UYVY10_1X20);
> + break;
> + default:
> + dev_err(core->dev, "Invalid data type!\n");
> + }
> +}
> +
> +static int xcsi2rxss_parse_of(struct xcsi2rxss_state *xcsi2rxss)
> +{
> + struct xcsi2rxss_core *core = &xcsi2rxss->core;
> + struct device_node *node = xcsi2rxss->core.dev->of_node;
> + struct device_node *ports = NULL;
> + struct device_node *port = NULL;
> + unsigned int nports, irq;
> + bool en_csi_v20, vfb;
> + int ret;
> +
> + en_csi_v20 = of_property_read_bool(node, "xlnx,en-csi-v2-0");
> + if (en_csi_v20)
> + core->en_vcx = of_property_read_bool(node, "xlnx,en-vcx");
> +
> + core->enable_active_lanes =
> + of_property_read_bool(node, "xlnx,en-active-lanes");
> +
> + ret = of_property_read_u32(node, "xlnx,csi-pxl-format",
> + &core->datatype);
> + if (ret < 0) {
> + dev_err(core->dev, "missing xlnx,csi-pxl-format property\n");
> + return ret;
> + }
> +
> + switch (core->datatype) {
> + case XCSI_DT_YUV4228B:
> + case XCSI_DT_RGB444:
> + case XCSI_DT_RGB555:
> + case XCSI_DT_RGB565:
> + case XCSI_DT_RGB666:
> + case XCSI_DT_RGB888:
> + case XCSI_DT_RAW6:
> + case XCSI_DT_RAW7:
> + case XCSI_DT_RAW8:
> + case XCSI_DT_RAW10:
> + case XCSI_DT_RAW12:
> + case XCSI_DT_RAW14:
> + break;
> + case XCSI_DT_YUV42210B:
> + case XCSI_DT_RAW16:
> + case XCSI_DT_RAW20:
> + if (!en_csi_v20) {
> + ret = -EINVAL;
> + dev_dbg(core->dev, "enable csi v2 for this pixel format");
> + }
> + break;
> + default:
> + ret = -EINVAL;
> + }
> + if (ret < 0) {
> + dev_err(core->dev, "invalid csi-pxl-format property!\n");
> + return ret;
> + }
> +
> + vfb = of_property_read_bool(node, "xlnx,vfb");
> + if (!vfb) {
> + dev_err(core->dev, "failed as VFB is disabled!\n");
> + return -EINVAL;
> + }
> +
> + ports = of_get_child_by_name(node, "ports");
> + if (!ports)
> + ports = node;
> +
> + nports = 0;
> + for_each_child_of_node(ports, port) {
> + struct device_node *endpoint;
> + struct v4l2_fwnode_endpoint v4lendpoint;
> + int ret;
> +
> + if (!port->name || of_node_cmp(port->name, "port"))
> + continue;
> +
> + endpoint = of_get_next_child(port, NULL);
> + if (!endpoint) {
> + dev_err(core->dev, "No port at\n");
> + return -EINVAL;
> + }
You shouldn't parse the graph data structure yourself.
Could you use fwnode_graph_get_endpoint_by_id(), and work it from there
without using an enumeration? You have two endpoints in there after all in
two ports.
Please see drivers/media/pci/intel/ipu3/ipu3-cio2.c for an example.
> +
> + /*
> + * since first port is sink port and it contains
> + * all info about data-lanes and cfa-pattern,
> + * don't parse second port but only check if exists
> + */
> + if (nports == XVIP_PAD_SOURCE) {
> + dev_dbg(core->dev, "no need to parse source port");
> + nports++;
> + of_node_put(endpoint);
> + continue;
> + }
> +
> + ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint),
> + &v4lendpoint);
Please set the bus_type to what you need on that port, e.g.
V4L2_MBUS_CSI2_DPHY, and set the rest to zero.
> + of_node_put(endpoint);
> + if (ret)
> + return ret;
> +
> + dev_dbg(core->dev, "%s : port %d bus type = %d\n",
> + __func__, nports, v4lendpoint.bus_type);
> +
> + if (v4lendpoint.bus_type == V4L2_MBUS_CSI2_DPHY) {
> + dev_dbg(core->dev, "%s : base.port = %d base.id = %d\n",
> + __func__, v4lendpoint.base.port,
> + v4lendpoint.base.id);
> +
> + dev_dbg(core->dev, "%s : mipi number lanes = %d\n",
> + __func__,
> + v4lendpoint.bus.mipi_csi2.num_data_lanes);
> +
> + core->max_num_lanes =
> + v4lendpoint.bus.mipi_csi2.num_data_lanes;
> + }
> +
> + /* Count the number of ports. */
> + nports++;
> + }
> +
> + if (nports != XCSI_MEDIA_PADS) {
> + dev_err(core->dev, "invalid number of ports %u\n", nports);
> + return -EINVAL;
> + }
> +
> + /* Register interrupt handler */
> + irq = irq_of_parse_and_map(node, 0);
> + ret = devm_request_irq(core->dev, irq, xcsi2rxss_irq_handler,
> + IRQF_SHARED, "xilinx-csi2rxss", xcsi2rxss);
> + if (ret) {
> + dev_err(core->dev, "Err = %d Interrupt handler reg failed!\n",
> + ret);
> + return ret;
> + }
> +
> + xcsi2rxss_log_ipconfig(xcsi2rxss);
> +
> + return 0;
> +}
> +
> +static int xcsi2rxss_probe(struct platform_device *pdev)
> +{
> + struct v4l2_subdev *subdev;
> + struct xcsi2rxss_state *xcsi2rxss;
> + struct xcsi2rxss_core *core;
> + struct resource *res;
> + int ret;
> +
> + xcsi2rxss = devm_kzalloc(&pdev->dev, sizeof(*xcsi2rxss), GFP_KERNEL);
> + if (!xcsi2rxss)
> + return -ENOMEM;
> +
> + core = &xcsi2rxss->core;
> + core->dev = &pdev->dev;
> +
> + core->clks = devm_kmemdup(core->dev, xcsi2rxss_clks,
> + sizeof(xcsi2rxss_clks), GFP_KERNEL);
> + if (!core->clks)
> + return -ENOMEM;
> +
> + /* Reset GPIO */
> + core->rst_gpio = devm_gpiod_get_optional(core->dev, "reset",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(core->rst_gpio)) {
> + if (PTR_ERR(core->rst_gpio) != -EPROBE_DEFER)
> + dev_err(core->dev, "Video Reset GPIO not setup in DT");
> + return PTR_ERR(core->rst_gpio);
> + }
> +
> + mutex_init(&xcsi2rxss->lock);
> +
> + ret = xcsi2rxss_parse_of(xcsi2rxss);
> + if (ret < 0)
> + return ret;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + core->iomem = devm_ioremap_resource(core->dev, res);
> + if (IS_ERR(core->iomem))
> + return PTR_ERR(core->iomem);
> +
> + core->num_clks = ARRAY_SIZE(xcsi2rxss_clks);
num_clks never changes, please drop it and use ARRAY_SIZE(...) instead.
> +
> + ret = clk_bulk_get(core->dev, core->num_clks, core->clks);
> + if (ret)
> + return ret;
> +
> + ret = clk_bulk_prepare_enable(core->num_clks, core->clks);
> + if (ret)
> + goto err_clk_put;
> +
> + if (core->rst_gpio) {
> + gpiod_set_value_cansleep(core->rst_gpio, 1);
> + /* minimum of 40 dphy_clk_200M cycles */
> + usleep_range(1, 2);
> + gpiod_set_value_cansleep(core->rst_gpio, 0);
> + }
> +
> + xcsi2rxss_reset(core);
> +
> + /* Initialize V4L2 subdevice and media entity */
> + xcsi2rxss->pads[XVIP_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> + xcsi2rxss->pads[XVIP_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
> +
> + /* Initialize the default format */
> + xcsi2rxss_set_default_format(xcsi2rxss);
> +
> + /* Initialize the mbus formats supported */
> + xcsi2rxss_init_mbus_fmts(xcsi2rxss);
> +
> + /* Initialize V4L2 subdevice and media entity */
> + subdev = &xcsi2rxss->subdev;
> + v4l2_subdev_init(subdev, &xcsi2rxss_ops);
> + subdev->dev = &pdev->dev;
> + subdev->internal_ops = &xcsi2rxss_internal_ops;
> + strscpy(subdev->name, dev_name(&pdev->dev), sizeof(subdev->name));
> + subdev->flags |= V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_HAS_DEVNODE;
> + subdev->entity.ops = &xcsi2rxss_media_ops;
> + v4l2_set_subdevdata(subdev, xcsi2rxss);
> +
> + ret = media_entity_pads_init(&subdev->entity, XCSI_MEDIA_PADS,
> + xcsi2rxss->pads);
> + if (ret < 0)
> + goto error;
> +
> + platform_set_drvdata(pdev, xcsi2rxss);
> +
> + ret = v4l2_async_register_subdev(subdev);
> + if (ret < 0) {
> + dev_err(core->dev, "failed to register subdev\n");
> + goto error;
> + }
> +
> + dev_info(core->dev, "Xilinx CSI2 Rx Subsystem device found!\n");
> +
> + return 0;
> +error:
> + media_entity_cleanup(&subdev->entity);
> + mutex_destroy(&xcsi2rxss->lock);
> + clk_bulk_disable_unprepare(core->num_clks, core->clks);
> +err_clk_put:
> + clk_bulk_put(core->num_clks, core->clks);
> + return ret;
> +}
> +
> +static int xcsi2rxss_remove(struct platform_device *pdev)
> +{
> + struct xcsi2rxss_state *xcsi2rxss = platform_get_drvdata(pdev);
> + struct xcsi2rxss_core *core = &xcsi2rxss->core;
> + struct v4l2_subdev *subdev = &xcsi2rxss->subdev;
> +
> + v4l2_async_unregister_subdev(subdev);
> + media_entity_cleanup(&subdev->entity);
> + mutex_destroy(&xcsi2rxss->lock);
> + clk_bulk_disable_unprepare(core->num_clks, core->clks);
> + clk_bulk_put(core->num_clks, core->clks);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id xcsi2rxss_of_id_table[] = {
> + { .compatible = "xlnx,mipi-csi2-rx-subsystem-4.0", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, xcsi2rxss_of_id_table);
> +
> +static struct platform_driver xcsi2rxss_driver = {
> + .driver = {
> + .name = "xilinx-csi2rxss",
> + .of_match_table = xcsi2rxss_of_id_table,
> + },
> + .probe = xcsi2rxss_probe,
> + .remove = xcsi2rxss_remove,
> +};
> +
> +module_platform_driver(xcsi2rxss_driver);
> +
> +MODULE_AUTHOR("Vishal Sagar <vsagar@xilinx.com>");
> +MODULE_DESCRIPTION("Xilinx MIPI CSI2 Rx Subsystem Driver");
> +MODULE_LICENSE("GPL v2");
This does not align with the SPDX header. v2 or v2+?
--
Regards,
Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: aarch64 Kernel Panic Asynchronous SError Interrupt on large file IO
From: Philipp Richter @ 2019-08-15 16:00 UTC (permalink / raw)
To: Robin Murphy
Cc: heiko, catalin.marinas, vicencb, linux-rockchip, andre.przywara,
Will Deacon, linux-arm-kernel
In-Reply-To: <CA+Vb7hpi=pCC9viiof8y85Kw_vCawWQ0B6kGFALgxtZfCKoaTw@mail.gmail.com>
Reading from the raw eMMC block /dev/mmcblkp1 I can also produce a panic :
sudo dd if=/dev/mmcblk1 of=/dev/null bs=1M status=progress
2846883840 bytes (2.8 GB, 2.7 GiB) copied, 23 s, 124 MB/s
============
[ 428.794747] dwmmc_rockchip ff520000.dwmmc: Unexpected command
timeout, state 3
[ 428.984736] dwmmc_rockchip ff520000.dwmmc: Unexpected command
timeout, state 3
[ 429.174738] dwmmc_rockchip ff520000.dwmmc: Unexpected command
timeout, state 3
[ 429.179323] Internal error: synchronous external abort: 96000210
[#1] SMP
[ 429.179934] Modules linked in: wireguard(O) ip6_udp_tunnel
udp_tunnel lz4 lz4_compress iptable_filter iptable_raw xt_owner
iptable_nat xt_connmark iptable_mangle bpfilter rc_cec
snd_soc_hdmi_codec dw_hdmi_i2s_audio dw_hdmi_cec
snd_soc_audio_graph_cc
[ 429.186527] CPU: 0 PID: 1079 Comm: bash Tainted: G O
5.2.8-1-ARCH #1
[ 429.187193] Hardware name: Pine64 Rock64 (DT)
[ 429.187576] pstate: 20000005 (nzCv daif -PAN -UAO)
[ 429.188007] pc : copy_page_range+0x124/0x3d0
[ 429.188386] lr : dup_mm+0x3fc/0x478
[ 429.188692] sp : ffff00001277bb80
[ 429.188982] x29: ffff00001277bb80 x28: ffff8000dd17e450
[ 429.189446] x27: ffff8000dd17e470 x26: ffff8000dd17e460
[ 429.189912] x25: 0000aaaac4a01000 x24: ffff8000dca92a00
[ 429.190376] x23: ffff8000dd1fdf80 x22: ffff8000dd30c8a0
[ 429.190840] x21: ffff8000dca92a00 x20: ffff8000dd30c8a0
[ 429.191306] x19: ffff8000dd1fdf80 x18: 0000000000000000
[ 429.191771] x17: 0000000000000000 x16: 0000000000000000
[ 429.192236] x15: 0000000000000000 x14: ffff8000dd2b86d0
[ 429.192700] x13: 00000000000000f8 x12: 0000000000000000
[ 429.193165] x11: 0000000000000000 x10: ffff8000e44bde01
[ 429.193630] x9 : 0000000000100871 x8 : 0000000000000000
[ 429.194095] x7 : ffff8000e4481760 x6 : 0000000000000000
[ 429.194560] x5 : 0000aaaac49fc000 x4 : ffff0000102905c0
[ 429.195026] x3 : 0000000000000000 x2 : ffff800009c74aa8
[ 429.195491] x1 : 0000aaaac4a00fff x0 : ffff800009c74aa8
[ 429.195959] Call trace:
[ 429.196178] copy_page_range+0x124/0x3d0
[ 429.196521] dup_mm+0x3fc/0x478
[ 429.196801] copy_process.isra.4.part.5+0x143c/0x1450
[ 429.197244] _do_fork+0xec/0x410
[ 429.197529] __arm64_sys_clone+0x2c/0x38
[ 429.197877] el0_svc_handler+0xa4/0x180
[ 429.198215] el0_svc+0x8/0xc
[ 429.198474] Code: 360812e0 f9403fe0 b4000ac0 f9403fe0 (f9400000)
[ 429.199008] ---[ end trace 04beba7bac629e3f ]---
[ 429.200049] SError Interrupt on CPU1, code 0xbf000002 -- SError
[ 429.200052] CPU: 1 PID: 669 Comm: systemd-journal Tainted: G D
O 5.2.8-1-ARCH #1
[ 429.200054] Hardware name: Pine64 Rock64 (DT)
[ 429.200055] pstate: 20000005 (nzCv daif -PAN -UAO)
[ 429.200056] pc : allocate_slab+0x1d0/0x570
[ 429.200058] lr : allocate_slab+0x1e0/0x570
[ 429.200059] sp : ffff000011d8baa0
[ 429.200060] x29: ffff000011d8baa0 x28: 0000000000000003
[ 429.200063] x27: ffff7e0000276800 x26: ffff800009da6e00
[ 429.200068] x25: 0000000000000009 x24: 0000000000007bc0
[ 429.200071] x23: 0000000000000003 x22: 0000000000000003
[ 429.200075] x21: ffff800009da0000 x20: 0000000000005280
[ 429.200079] x19: ffff8000b3fa3980 x18: 0000000000000000
[ 429.200082] x17: 0000000000000000 x16: 0000000000000000
[ 429.200086] x15: 0000000000000000 x14: 0000000000000000
[ 429.200090] x13: 0000000000000000 x12: 0000000000000000
[ 429.200094] x11: 0000000000000000 x10: 0000000000000000
[ 429.200098] x9 : 0000000000000000 x8 : 0000000000000000
[ 429.200102] x7 : 00000000fee00000 x6 : 0000000000000018
[ 429.200106] x5 : 0000000000000040 x4 : 0000000000210d00
[ 429.200110] x3 : 0000000000000dc0 x2 : 0000000005a79795
[ 429.200112] x1 : 0000000000000000 x0 : ffff8000f2f35a80
[ 429.200117] Kernel panic - not syncing: Asynchronous SError
Interrupt
[ 429.200137] SMP: stopping secondary CPUs
[ 429.200139] Kernel Offset: disabled
[ 429.200140] CPU features: 0x0002,20002000
[ 429.200141] Memory Limit: none
============
Regards,
Philipp Richter
On Thu, 15 Aug 2019 at 17:35, Philipp Richter
<richterphilipp.pops@gmail.com> wrote:
>
> Yes, it's connected over the USB 3.0 port. I'll also try over USB 2.0
> as soon as possible.
>
> I first noticed the issue when my backup script froze the board, so
> this is while reading from the eMMC.
>
> My script that I invoke over ssh :
> ============
> #!/usr/bin/env bash
> IFS=$'\n\t'
> set -euo pipefail
>
> schedtool -B -n 8 "${BASHPID}"
> ionice -c 3 -p "${BASHPID}"
>
> EXCLUSION_FILE='/etc/tar-system-exclusion.txt'
> TOTAL_SIZE="$(sudo du --bytes --summarize
> --exclude-from="${EXCLUSION_FILE}" / | awk '{print $1}')"
> sudo tar --create --file - --numeric-owner --acls --xattrs
> --exclude-from="${EXCLUSION_FILE}" / | \
> pv --progress --timer --eta --fineta --rate --average-rate
> --bytes --force --size "${TOTAL_SIZE}" | \
> lz4 -z
> ============
>
> So it fails also around 2.8GB pushed and I get this panic on my serial
> console in "__memcpy" this time though :
>
> ============
> [12624.268933] SError Interrupt on CPU0, code 0xbf000002 -- SError
> [12624.268940] CPU: 0 PID: 14170 Comm: kworker/u8:4 Tainted: G
> O 5.2.8-1-ARCH #1
> [12624.268942] Hardware name: Pine64 Rock64 (DT)
> [12624.268944] Workqueue: btrfs-endio btrfs_endio_helper [btrfs]
> [12624.268946] pstate: 20000005 (nzCv daif -PAN -UAO)
> [12624.268948] pc : __memcpy+0x118/0x180
> [12624.268950] lr : btrfs_decompress_buf2page+0x124/0x228 [btrfs]
> [12624.268951] sp : ffff00001c28bb40
> [12624.268952] x29: ffff00001c28bb40 x28: ffff8000f2a2b870
> [12624.268955] x27: 0000000000001000 x26: ffff7e0000270200
> [12624.268958] x25: 0000000000001000 x24: 000000000001f000
> [12624.268961] x23: 0000000000000000 x22: 000000000001f000
> [12624.268964] x21: ffff8000fde46040 x20: 0000000000140000
> [12624.268967] x19: 0000000000001000 x18: ffff8000e830aef5
> [12624.268970] x17: 0000000000000ad3 x16: 0000000000000003
> [12624.268973] x15: 0000000000000002 x14: a8c37bfd9101e042
> [12624.268976] x13: a9425bf552800021 x12: a94153f3f0000b62
> [12624.268979] x11: f9400a80900011a4 x10: aa1603e3d63f0260
> [12624.268982] x9 : 9101c04252800021 x8 : 910003fda9b97bfd
> [12624.268985] x7 : d61f0080f9475c84 x6 : ffff800009c08390
> [12624.268988] x5 : ffff800065005050 x4 : 0000000000000000
> [12624.268990] x3 : 0000000000140000 x2 : 0000000000000c00
> [12624.268993] x1 : ffff8000dac023d0 x0 : ffff800009c08000
> [12624.268997] Kernel panic - not syncing: Asynchronous SError Interrupt
> [12624.269000] CPU: 0 PID: 14170 Comm: kworker/u8:4 Tainted: G
> O 5.2.8-1-ARCH #1
> [12624.269001] Hardware name: Pine64 Rock64 (DT)
> [12624.269003] Workqueue: btrfs-endio btrfs_endio_helper [btrfs]
> [12624.269004] Call trace:
> [12624.269006] dump_backtrace+0x0/0x168
> [12624.269007] show_stack+0x24/0x30
> [12624.269009] dump_stack+0xa8/0xcc
> [12624.269010] panic+0x150/0x320
> [12624.269011] __stack_chk_fail+0x0/0x28
> [12624.269013] arm64_serror_panic+0x80/0x8c
> [12624.269014] do_serror+0x11c/0x120
> [12624.269016] el1_error+0x84/0xf8
> [12624.269017] __memcpy+0x118/0x180
> [12624.269018] zstd_decompress_bio+0xf8/0x250 [btrfs]
> [12624.269020] end_compressed_bio_read+0x2ec/0x3f8 [btrfs]
> [12624.269021] bio_endio.part.12+0x10c/0x1a8
> [12624.269023] bio_endio+0x20/0x30
> [12624.269024] end_workqueue_fn+0x4c/0x58 [btrfs]
> [12624.269025] normal_work_helper+0x100/0x250 [btrfs]
> [12624.269027] btrfs_endio_helper+0x20/0x30 [btrfs]
> [12624.269029] process_one_work+0x1b4/0x408
> [12624.269030] worker_thread+0x54/0x4b8
> [12624.269031] kthread+0x12c/0x130
> [12624.269033] ret_from_fork+0x10/0x1c
> [12624.269068] SMP: stopping secondary CPUs
> [12624.269069] Kernel Offset: disabled
> [12624.269071] CPU features: 0x0002,20002000
> [12624.269072] Memory Limit: none
> ============
>
> Regards,
> Philipp Richter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v8 4/5] arm64: Define Documentation/arm64/tagged-address-abi.rst
From: Catalin Marinas @ 2019-08-15 15:44 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm
Cc: linux-arch, linux-doc, Szabolcs Nagy, Andrey Konovalov,
Kevin Brodsky, Will Deacon, Dave Hansen, Andrew Morton,
Vincenzo Frascino, Dave P Martin
In-Reply-To: <20190815154403.16473-1-catalin.marinas@arm.com>
From: Vincenzo Frascino <vincenzo.frascino@arm.com>
On AArch64 the TCR_EL1.TBI0 bit is set by default, allowing userspace
(EL0) to perform memory accesses through 64-bit pointers with a non-zero
top byte. Introduce the document describing the relaxation of the
syscall ABI that allows userspace to pass certain tagged pointers to
kernel syscalls.
Cc: Will Deacon <will.deacon@arm.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Szabolcs Nagy <szabolcs.nagy@arm.com>
Cc: Kevin Brodsky <kevin.brodsky@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Co-developed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
Documentation/arm64/tagged-address-abi.rst | 155 +++++++++++++++++++++
1 file changed, 155 insertions(+)
create mode 100644 Documentation/arm64/tagged-address-abi.rst
diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
new file mode 100644
index 000000000000..8808337775d6
--- /dev/null
+++ b/Documentation/arm64/tagged-address-abi.rst
@@ -0,0 +1,155 @@
+==========================
+AArch64 TAGGED ADDRESS ABI
+==========================
+
+Authors: Vincenzo Frascino <vincenzo.frascino@arm.com>
+ Catalin Marinas <catalin.marinas@arm.com>
+
+Date: 15 August 2019
+
+This document describes the usage and semantics of the Tagged Address
+ABI on AArch64 Linux.
+
+1. Introduction
+---------------
+
+On AArch64 the TCR_EL1.TBI0 bit is set by default, allowing userspace
+(EL0) to perform memory accesses through 64-bit pointers with a non-zero
+top byte. This document describes the relaxation of the syscall ABI that
+allows userspace to pass certain tagged pointers to kernel syscalls.
+
+2. AArch64 Tagged Address ABI
+-----------------------------
+
+From the kernel syscall interface perspective and for the purposes of
+this document, a "valid tagged pointer" is a pointer with a potentially
+non-zero top-byte that references an address in the user process address
+space obtained in one of the following ways:
+
+- mmap() done by the process itself (or its parent), where either:
+
+ - flags have the **MAP_ANONYMOUS** bit set
+ - the file descriptor refers to a regular file (including those
+ returned by memfd_create()) or **/dev/zero**
+
+- brk() system call done by the process itself (i.e. the heap area
+ between the initial location of the program break at process creation
+ and its current location).
+
+- any memory mapped by the kernel in the address space of the process
+ during creation and with the same restrictions as for mmap() above
+ (e.g. data, bss, stack).
+
+The AArch64 Tagged Address ABI has two stages of relaxation depending
+how the user addresses are used by the kernel:
+
+1. User addresses not accessed by the kernel but used for address space
+ management (e.g. mmap(), mprotect(), madvise()). The use of valid
+ tagged pointers in this context is always allowed.
+
+2. User addresses accessed by the kernel (e.g. write()). This ABI
+ relaxation is disabled by default and the application thread needs to
+ explicitly enable it via **prctl()** as follows:
+
+ - **PR_SET_TAGGED_ADDR_CTRL**: enable or disable the AArch64 Tagged
+ Address ABI for the calling thread.
+
+ The (unsigned int) arg2 argument is a bit mask describing the
+ control mode used:
+
+ - **PR_TAGGED_ADDR_ENABLE**: enable AArch64 Tagged Address ABI.
+ Default status is disabled.
+
+ Arguments arg3, arg4, and arg5 must be 0.
+
+ - **PR_GET_TAGGED_ADDR_CTRL**: get the status of the AArch64 Tagged
+ Address ABI for the calling thread.
+
+ Arguments arg2, arg3, arg4, and arg5 must be 0.
+
+ The ABI properties described above are thread-scoped, inherited on
+ clone() and fork() and cleared on exec().
+
+ Calling prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0)
+ returns -EINVAL if the AArch64 Tagged Address ABI is globally disabled
+ by sysctl abi.tagged_addr_disabled=1. The default sysctl
+ abi.tagged_addr_disabled configuration is 0.
+
+When the AArch64 Tagged Address ABI is enabled for a thread, the
+following behaviours are guaranteed:
+
+- All syscalls except the cases mentioned in section 3 can accept any
+ valid tagged pointer.
+
+- The syscall behaviour is undefined for invalid tagged pointers: it may
+ result in an error code being returned, a (fatal) signal being raised,
+ or other modes of failure.
+
+- A valid tagged pointer has the same semantics as the corresponding
+ untagged pointer.
+
+A definition of the meaning of tagged pointers on AArch64 can be found
+in Documentation/arm64/tagged-pointers.rst.
+
+3. AArch64 Tagged Address ABI Exceptions
+-----------------------------------------
+
+The following system call parameters must be untagged regardless of the
+ABI relaxation:
+
+- prctl() other than arguments pointing to user structures to be
+ accessed by the kernel.
+
+- ioctl() other than arguments pointing to user structures to be
+ accessed by the kernel.
+
+- shmat() and shmdt().
+
+Any attempt to use non-zero tagged pointers may result in an error code
+being returned, a (fatal) signal being raised, or other modes of
+failure.
+
+4. Example of correct usage
+---------------------------
+.. code-block:: c
+
+ #include <stdlib.h>
+ #include <string.h>
+ #include <unistd.h>
+ #include <sys/mman.h>
+ #include <sys/prctl.h>
+
+ #define PR_SET_TAGGED_ADDR_CTRL 55
+ #define PR_TAGGED_ADDR_ENABLE (1UL << 0)
+
+ #define TAG_SHIFT 56
+
+ int main(void)
+ {
+ int tbi_enabled = 0;
+ unsigned long tag = 0;
+ char *ptr;
+
+ /* check/enable the tagged address ABI */
+ if (!prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0))
+ tbi_enabled = 1;
+
+ /* memory allocation */
+ ptr = mmap(NULL, sysconf(_SC_PAGE_SIZE), PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (ptr == MAP_FAILED)
+ return 1;
+
+ /* set a non-zero tag if the ABI is available */
+ if (tbi_enabled)
+ tag = rand() & 0xff;
+ ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
+
+ /* memory access to a tagged address */
+ strcpy(ptr, "tagged pointer\n");
+
+ /* syscall with a tagged pointer */
+ write(1, ptr, strlen(ptr));
+
+ return 0;
+ }
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox