* [PATCH] ARM: dts: imx6q: Add Engicam i.CoreM6 Quad/Dual OpenFrame Cap 7 initial support
From: Fabio Estevam @ 2018-01-03 17:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1514966447-30068-1-git-send-email-jagan@amarulasolutions.com>
Hi Jagan,
On Wed, Jan 3, 2018 at 6:00 AM, Jagan Teki <jagan@amarulasolutions.com> wrote:
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6q-icore-ofcap7.dts
> @@ -0,0 +1,100 @@
> +/*
> + * Copyright (C) 2016 Amarula Solutions B.V.
> + * Copyright (C) 2016 Engicam S.r.l.
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + * a) This file is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This file is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + * b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
Please consider using SPDX tag instead of this big license text:
// SPDX-License-Identifier: (GPL-2.0 OR MIT)
(Put it in the very first line of the dts file).
> + panel {
> + compatible = "ampire,am-800480aytzqw-00h";
I checked against linux-next and could not see this compatible string
documented.
^ permalink raw reply
* [PATCH 0/3] irqchip: irq-bcm2836: add support for DT interrupt polarity
From: Marc Zyngier @ 2018-01-03 17:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <60587e4d-d285-34e7-3d27-ea1daa1f97d1@i2se.com>
On 03/01/18 17:10, Stefan Wahren wrote:
> Hi Marc,
>
>
> Am 19.12.2017 um 17:36 schrieb Marc Zyngier:
>> On 19/12/17 07:02, Stefan Wahren wrote:
>>> Hi Marc,
>>>
>>> Am 11.12.2017 um 21:39 schrieb Stefan Wahren:
>>>> This patch series implements DT polarity support for the 1st level interrupt
>>>> controller.
>>>>
>>>> Stefan Wahren (3):
>>>> dt-bindings: bcm2836-l1-intc: add interrupt polarity support
>>>> irqchip: irq-bcm2836: add support for DT interrupt polarity
>>>> ARM: dts: bcm283x: Define polarity of per-cpu interrupts
>>>>
>>>> .../interrupt-controller/brcm,bcm2836-l1-intc.txt | 4 +-
>>>> arch/arm/boot/dts/bcm2836.dtsi | 14 +++----
>>>> arch/arm/boot/dts/bcm2837.dtsi | 12 +++---
>>>> arch/arm/boot/dts/bcm283x.dtsi | 1 +
>>>> drivers/irqchip/irq-bcm2836.c | 46 +++++++++++++---------
>>>> 5 files changed, 44 insertions(+), 33 deletions(-)
>>>>
>>> is this series okay?
>> Yes, it does look good. I'll queue that for 4.16.
>>
>> Thanks,
>>
>> M.
>
> since i didn't found this in linux-next, please take this as a gentle
> reminder.
It will take some time before I push this to tglx, as I'm otherwise
engaged. Patches are not lost.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH V3 3/3] arm64: Extend early page table code to allow for larger kernels
From: Steve Capper @ 2018-01-03 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu_uQ-5TNmWRcmADR6Rnm0qDL7FHZ7FKGR7Kd-toXTxqww@mail.gmail.com>
On Wed, Jan 03, 2018 at 04:38:47PM +0000, Ard Biesheuvel wrote:
> On 3 January 2018 at 16:20, Steve Capper <steve.capper@arm.com> wrote:
> > On Tue, Jan 02, 2018 at 10:01:29PM +0000, Ard Biesheuvel wrote:
> >> Hi Steve,
> >
> > Hi Ard,
> >
> >>
> >> On 2 January 2018 at 15:12, Steve Capper <steve.capper@arm.com> wrote:
> >> > Currently the early assembler page table code assumes that precisely
> >> > 1xpgd, 1xpud, 1xpmd are sufficient to represent the early kernel text
> >> > mappings.
> >> >
> >> > Unfortunately this is rarely the case when running with a 16KB granule,
> >> > and we also run into limits with 4KB granule when building much larger
> >> > kernels.
> >> >
> >> > This patch re-writes the early page table logic to compute indices of
> >> > mappings for each level of page table, and if multiple indices are
> >> > required, the next-level page table is scaled up accordingly.
> >> >
> >> > Also the required size of the swapper_pg_dir is computed at link time
> >> > to cover the mapping [KIMAGE_ADDR + VOFFSET, _end]. When KASLR is
> >> > enabled, an extra page is set aside for each level that may require extra
> >> > entries at runtime.
> >> >
> >> > Signed-off-by: Steve Capper <steve.capper@arm.com>
> >> >
> >> > ---
> >> > Changed in V3:
> >> > Corrected KASLR computation
> >> > Rebased against arm64/for-next/core, particularly Kristina's 52-bit
> >> > PA series.
> >> > ---
> >> > arch/arm64/include/asm/kernel-pgtable.h | 47 ++++++++++-
> >> > arch/arm64/include/asm/pgtable.h | 1 +
> >> > arch/arm64/kernel/head.S | 145 +++++++++++++++++++++++---------
> >> > arch/arm64/kernel/vmlinux.lds.S | 1 +
> >> > arch/arm64/mm/mmu.c | 3 +-
> >> > 5 files changed, 157 insertions(+), 40 deletions(-)
> >> >
> >> > diff --git a/arch/arm64/include/asm/kernel-pgtable.h b/arch/arm64/include/asm/kernel-pgtable.h
> >> > index 77a27af01371..82386e860dd2 100644
> >> > --- a/arch/arm64/include/asm/kernel-pgtable.h
> >> > +++ b/arch/arm64/include/asm/kernel-pgtable.h
[...]
> >> > - * Preserves: tbl, flags
> >> > - * Corrupts: phys, start, end, tmp, pstate
> >> > + * Preserves: vstart, vend, shift, ptrs
> >> > + * Returns: istart, iend, count
> >> > */
> >> > - .macro create_block_map, tbl, flags, phys, start, end, tmp
> >> > - lsr \start, \start, #SWAPPER_BLOCK_SHIFT
> >> > - and \start, \start, #PTRS_PER_PTE - 1 // table index
> >> > - bic \phys, \phys, #SWAPPER_BLOCK_SIZE - 1
> >> > - lsr \end, \end, #SWAPPER_BLOCK_SHIFT
> >> > - and \end, \end, #PTRS_PER_PTE - 1 // table end index
> >> > -9999: phys_to_pte \phys, \tmp
> >> > - orr \tmp, \tmp, \flags // table entry
> >> > - str \tmp, [\tbl, \start, lsl #3] // store the entry
> >> > - add \start, \start, #1 // next entry
> >> > - add \phys, \phys, #SWAPPER_BLOCK_SIZE // next block
> >> > - cmp \start, \end
> >> > - b.ls 9999b
> >> > + .macro compute_indices, vstart, vend, shift, ptrs, istart, iend, count
> >> > + lsr \iend, \vend, \shift
> >> > + mov \istart, \ptrs
> >> > + sub \istart, \istart, #1
> >> > + and \iend, \iend, \istart // iend = (vend >> shift) & (ptrs - 1)
> >> > + mov \istart, \ptrs
> >> > + sub \count, \count, #1
> >> > + mul \istart, \istart, \count
> >> > + add \iend, \iend, \istart // iend += (count - 1) * ptrs
> >> > + // our entries span multiple tables
> >> > +
> >> > + lsr \istart, \vstart, \shift
> >> > + mov \count, \ptrs
> >> > + sub \count, \count, #1
> >> > + and \istart, \istart, \count
> >> > +
> >> > + sub \count, \iend, \istart
> >> > + add \count, \count, #1
> >> > + .endm
> >> > +
> >>
> >> You can simplify this macro by using an immediate for \ptrs. Please
> >> see the diff below [whitespace mangling courtesy of Gmail]
> >
> > Thanks, I like the simplification a lot. For 52-bit kernel VAs though, I
> > will need a variable PTRS_PER_PGD at compile time.
> >
> > For 52-bit kernel PAs one can just use the maximum number of ptrs available.
> > For a 48-bit PA the leading address bits will always be zero, thus we
> > will derive the same PGD indices from both 48 and 52 bit PTRS_PER_PGD.
> >
> > For kernel VAs, because the leading address bits are one, we need to use a
> > PTRS_PER_PGD corresponding to the VA size.
> >
>
> OK, so you are saying you shouldn't mask off too many bits, right? In
> any case, I suppose you can just use the same trick as I used for
> .Lidmap_ptrs_per_pgd, i.e., use .set to assign the correct value and
> pass that into the macro.
>
Yeah, that's right, one needs to mask off the correct number of bits.
If I've understood correctly, we choose which .set to use at compile
time rather than runtime though?
The problem I have is the number of PGDs is only known precisely at boot
time when one has a kernel that switches between 48/52 bit VAs. That's
why I had the number of PGDs in a register.
Cheers,
--
Steve
^ permalink raw reply
* [PATCH 0/3] irqchip: irq-bcm2836: add support for DT interrupt polarity
From: Stefan Wahren @ 2018-01-03 17:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a7939fae-31d1-c743-5957-d88be98a0c63@arm.com>
Hi Marc,
Am 19.12.2017 um 17:36 schrieb Marc Zyngier:
> On 19/12/17 07:02, Stefan Wahren wrote:
>> Hi Marc,
>>
>> Am 11.12.2017 um 21:39 schrieb Stefan Wahren:
>>> This patch series implements DT polarity support for the 1st level interrupt
>>> controller.
>>>
>>> Stefan Wahren (3):
>>> dt-bindings: bcm2836-l1-intc: add interrupt polarity support
>>> irqchip: irq-bcm2836: add support for DT interrupt polarity
>>> ARM: dts: bcm283x: Define polarity of per-cpu interrupts
>>>
>>> .../interrupt-controller/brcm,bcm2836-l1-intc.txt | 4 +-
>>> arch/arm/boot/dts/bcm2836.dtsi | 14 +++----
>>> arch/arm/boot/dts/bcm2837.dtsi | 12 +++---
>>> arch/arm/boot/dts/bcm283x.dtsi | 1 +
>>> drivers/irqchip/irq-bcm2836.c | 46 +++++++++++++---------
>>> 5 files changed, 44 insertions(+), 33 deletions(-)
>>>
>> is this series okay?
> Yes, it does look good. I'll queue that for 4.16.
>
> Thanks,
>
> M.
since i didn't found this in linux-next, please take this as a gentle
reminder.
Thanks
Stefan
^ permalink raw reply
* [PATCH] clk: imx: imx7d: correct video pll clock tree
From: Anson Huang @ 2018-01-03 17:09 UTC (permalink / raw)
To: linux-arm-kernel
There is a test divider and post divider in video PLL,
test divider is placed before post divider, all clocks
that can select parent from video PLL should be from
post divider, NOT from pll_video_main, below are
clock tree dump before and after this patch:
Before:
pll_video_main
pll_video_main_bypass
pll_video_main_clk
lcdif_pixel_src
lcdif_pixel_cg
lcdif_pixel_pre_div
lcdif_pixel_post_div
lcdif_pixel_root_clk
After:
pll_video_main
pll_video_main_bypass
pll_video_main_clk
pll_video_test_div
pll_video_post_div
lcdif_pixel_src
lcdif_pixel_cg
lcdif_pixel_pre_div
lcdif_pixel_post_div
lcdif_pixel_root_clk
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/clk/imx/clk-imx7d.c | 84 ++++++++++++++++++++++++---------------------
1 file changed, 44 insertions(+), 40 deletions(-)
diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index 80dc211..992938b 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -51,20 +51,20 @@
static const char *arm_m4_sel[] = { "osc", "pll_sys_main_240m_clk",
"pll_enet_250m_clk", "pll_sys_pfd2_270m_clk",
- "pll_dram_533m_clk", "pll_audio_post_div", "pll_video_main_clk",
+ "pll_dram_533m_clk", "pll_audio_post_div", "pll_video_post_div",
"pll_usb_main_clk", };
static const char *axi_sel[] = { "osc", "pll_sys_pfd1_332m_clk",
"pll_dram_533m_clk", "pll_enet_250m_clk", "pll_sys_pfd5_clk",
- "pll_audio_post_div", "pll_video_main_clk", "pll_sys_pfd7_clk", };
+ "pll_audio_post_div", "pll_video_post_div", "pll_sys_pfd7_clk", };
static const char *disp_axi_sel[] = { "osc", "pll_sys_pfd1_332m_clk",
"pll_dram_533m_clk", "pll_enet_250m_clk", "pll_sys_pfd6_clk",
- "pll_sys_pfd7_clk", "pll_audio_post_div", "pll_video_main_clk", };
+ "pll_sys_pfd7_clk", "pll_audio_post_div", "pll_video_post_div", };
static const char *enet_axi_sel[] = { "osc", "pll_sys_pfd2_270m_clk",
"pll_dram_533m_clk", "pll_enet_250m_clk",
- "pll_sys_main_240m_clk", "pll_audio_post_div", "pll_video_main_clk",
+ "pll_sys_main_240m_clk", "pll_audio_post_div", "pll_video_post_div",
"pll_sys_pfd4_clk", };
static const char *nand_usdhc_bus_sel[] = { "osc", "pll_sys_pfd2_270m_clk",
@@ -75,7 +75,7 @@
static const char *ahb_channel_sel[] = { "osc", "pll_sys_pfd2_270m_clk",
"pll_dram_533m_clk", "pll_sys_pfd0_392m_clk",
"pll_enet_125m_clk", "pll_usb_main_clk", "pll_audio_post_div",
- "pll_video_main_clk", };
+ "pll_video_post_div", };
static const char *dram_phym_sel[] = { "pll_dram_main_clk",
"dram_phym_alt_clk", };
@@ -86,7 +86,7 @@
static const char *dram_phym_alt_sel[] = { "osc", "pll_dram_533m_clk",
"pll_sys_main_clk", "pll_enet_500m_clk",
"pll_usb_main_clk", "pll_sys_pfd7_clk", "pll_audio_post_div",
- "pll_video_main_clk", };
+ "pll_video_post_div", };
static const char *dram_alt_sel[] = { "osc", "pll_dram_533m_clk",
"pll_sys_main_clk", "pll_enet_500m_clk",
@@ -108,62 +108,62 @@
static const char *epdc_pixel_sel[] = { "osc", "pll_sys_pfd1_332m_clk",
"pll_dram_533m_clk", "pll_sys_main_clk", "pll_sys_pfd5_clk",
- "pll_sys_pfd6_clk", "pll_sys_pfd7_clk", "pll_video_main_clk", };
+ "pll_sys_pfd6_clk", "pll_sys_pfd7_clk", "pll_video_post_div", };
static const char *lcdif_pixel_sel[] = { "osc", "pll_sys_pfd5_clk",
"pll_dram_533m_clk", "ext_clk_3", "pll_sys_pfd4_clk",
- "pll_sys_pfd2_270m_clk", "pll_video_main_clk",
+ "pll_sys_pfd2_270m_clk", "pll_video_post_div",
"pll_usb_main_clk", };
static const char *mipi_dsi_sel[] = { "osc", "pll_sys_pfd5_clk",
"pll_sys_pfd3_clk", "pll_sys_main_clk", "pll_sys_pfd0_196m_clk",
- "pll_dram_533m_clk", "pll_video_main_clk", "pll_audio_post_div", };
+ "pll_dram_533m_clk", "pll_video_post_div", "pll_audio_post_div", };
static const char *mipi_csi_sel[] = { "osc", "pll_sys_pfd4_clk",
"pll_sys_pfd3_clk", "pll_sys_main_clk", "pll_sys_pfd0_196m_clk",
- "pll_dram_533m_clk", "pll_video_main_clk", "pll_audio_post_div", };
+ "pll_dram_533m_clk", "pll_video_post_div", "pll_audio_post_div", };
static const char *mipi_dphy_sel[] = { "osc", "pll_sys_main_120m_clk",
"pll_dram_533m_clk", "pll_sys_pfd5_clk", "ref_1m_clk", "ext_clk_2",
- "pll_video_main_clk", "ext_clk_3", };
+ "pll_video_post_div", "ext_clk_3", };
static const char *sai1_sel[] = { "osc", "pll_sys_pfd2_135m_clk",
- "pll_audio_post_div", "pll_dram_533m_clk", "pll_video_main_clk",
+ "pll_audio_post_div", "pll_dram_533m_clk", "pll_video_post_div",
"pll_sys_pfd4_clk", "pll_enet_125m_clk", "ext_clk_2", };
static const char *sai2_sel[] = { "osc", "pll_sys_pfd2_135m_clk",
- "pll_audio_post_div", "pll_dram_533m_clk", "pll_video_main_clk",
+ "pll_audio_post_div", "pll_dram_533m_clk", "pll_video_post_div",
"pll_sys_pfd4_clk", "pll_enet_125m_clk", "ext_clk_2", };
static const char *sai3_sel[] = { "osc", "pll_sys_pfd2_135m_clk",
- "pll_audio_post_div", "pll_dram_533m_clk", "pll_video_main_clk",
+ "pll_audio_post_div", "pll_dram_533m_clk", "pll_video_post_div",
"pll_sys_pfd4_clk", "pll_enet_125m_clk", "ext_clk_3", };
static const char *spdif_sel[] = { "osc", "pll_sys_pfd2_135m_clk",
- "pll_audio_post_div", "pll_dram_533m_clk", "pll_video_main_clk",
+ "pll_audio_post_div", "pll_dram_533m_clk", "pll_video_post_div",
"pll_sys_pfd4_clk", "pll_enet_125m_clk", "ext_3_clk", };
static const char *enet1_ref_sel[] = { "osc", "pll_enet_125m_clk",
"pll_enet_50m_clk", "pll_enet_25m_clk",
- "pll_sys_main_120m_clk", "pll_audio_post_div", "pll_video_main_clk",
+ "pll_sys_main_120m_clk", "pll_audio_post_div", "pll_video_post_div",
"ext_clk_4", };
static const char *enet1_time_sel[] = { "osc", "pll_enet_100m_clk",
"pll_audio_post_div", "ext_clk_1", "ext_clk_2", "ext_clk_3",
- "ext_clk_4", "pll_video_main_clk", };
+ "ext_clk_4", "pll_video_post_div", };
static const char *enet2_ref_sel[] = { "osc", "pll_enet_125m_clk",
"pll_enet_50m_clk", "pll_enet_25m_clk",
- "pll_sys_main_120m_clk", "pll_audio_post_div", "pll_video_main_clk",
+ "pll_sys_main_120m_clk", "pll_audio_post_div", "pll_video_post_div",
"ext_clk_4", };
static const char *enet2_time_sel[] = { "osc", "pll_enet_100m_clk",
"pll_audio_post_div", "ext_clk_1", "ext_clk_2", "ext_clk_3",
- "ext_clk_4", "pll_video_main_clk", };
+ "ext_clk_4", "pll_video_post_div", };
static const char *enet_phy_ref_sel[] = { "osc", "pll_enet_25m_clk",
"pll_enet_50m_clk", "pll_enet_125m_clk",
- "pll_dram_533m_clk", "pll_audio_post_div", "pll_video_main_clk",
+ "pll_dram_533m_clk", "pll_audio_post_div", "pll_video_post_div",
"pll_sys_pfd3_clk", };
static const char *eim_sel[] = { "osc", "pll_sys_pfd2_135m_clk",
@@ -174,7 +174,7 @@
static const char *nand_sel[] = { "osc", "pll_sys_main_clk",
"pll_dram_533m_clk", "pll_sys_pfd0_392m_clk", "pll_sys_pfd3_clk",
"pll_enet_500m_clk", "pll_enet_250m_clk",
- "pll_video_main_clk", };
+ "pll_video_post_div", };
static const char *qspi_sel[] = { "osc", "pll_sys_pfd4_clk",
"pll_dram_533m_clk", "pll_enet_500m_clk", "pll_sys_pfd3_clk",
@@ -204,22 +204,22 @@
static const char *i2c1_sel[] = { "osc", "pll_sys_main_120m_clk",
"pll_enet_50m_clk", "pll_dram_533m_clk",
- "pll_audio_post_div", "pll_video_main_clk", "pll_usb_main_clk",
+ "pll_audio_post_div", "pll_video_post_div", "pll_usb_main_clk",
"pll_sys_pfd2_135m_clk", };
static const char *i2c2_sel[] = { "osc", "pll_sys_main_120m_clk",
"pll_enet_50m_clk", "pll_dram_533m_clk",
- "pll_audio_post_div", "pll_video_main_clk", "pll_usb_main_clk",
+ "pll_audio_post_div", "pll_video_post_div", "pll_usb_main_clk",
"pll_sys_pfd2_135m_clk", };
static const char *i2c3_sel[] = { "osc", "pll_sys_main_120m_clk",
"pll_enet_50m_clk", "pll_dram_533m_clk",
- "pll_audio_post_div", "pll_video_main_clk", "pll_usb_main_clk",
+ "pll_audio_post_div", "pll_video_post_div", "pll_usb_main_clk",
"pll_sys_pfd2_135m_clk", };
static const char *i2c4_sel[] = { "osc", "pll_sys_main_120m_clk",
"pll_enet_50m_clk", "pll_dram_533m_clk",
- "pll_audio_post_div", "pll_video_main_clk", "pll_usb_main_clk",
+ "pll_audio_post_div", "pll_video_post_div", "pll_usb_main_clk",
"pll_sys_pfd2_135m_clk", };
static const char *uart1_sel[] = { "osc", "pll_sys_main_240m_clk",
@@ -279,27 +279,27 @@
static const char *pwm1_sel[] = { "osc", "pll_enet_100m_clk",
"pll_sys_main_120m_clk", "pll_enet_40m_clk", "pll_audio_post_div",
- "ext_clk_1", "ref_1m_clk", "pll_video_main_clk", };
+ "ext_clk_1", "ref_1m_clk", "pll_video_post_div", };
static const char *pwm2_sel[] = { "osc", "pll_enet_100m_clk",
"pll_sys_main_120m_clk", "pll_enet_40m_clk", "pll_audio_post_div",
- "ext_clk_1", "ref_1m_clk", "pll_video_main_clk", };
+ "ext_clk_1", "ref_1m_clk", "pll_video_post_div", };
static const char *pwm3_sel[] = { "osc", "pll_enet_100m_clk",
"pll_sys_main_120m_clk", "pll_enet_40m_clk", "pll_audio_post_div",
- "ext_clk_2", "ref_1m_clk", "pll_video_main_clk", };
+ "ext_clk_2", "ref_1m_clk", "pll_video_post_div", };
static const char *pwm4_sel[] = { "osc", "pll_enet_100m_clk",
"pll_sys_main_120m_clk", "pll_enet_40m_clk", "pll_audio_post_div",
- "ext_clk_2", "ref_1m_clk", "pll_video_main_clk", };
+ "ext_clk_2", "ref_1m_clk", "pll_video_post_div", };
static const char *flextimer1_sel[] = { "osc", "pll_enet_100m_clk",
"pll_sys_main_120m_clk", "pll_enet_40m_clk", "pll_audio_post_div",
- "ext_clk_3", "ref_1m_clk", "pll_video_main_clk", };
+ "ext_clk_3", "ref_1m_clk", "pll_video_post_div", };
static const char *flextimer2_sel[] = { "osc", "pll_enet_100m_clk",
"pll_sys_main_120m_clk", "pll_enet_40m_clk", "pll_audio_post_div",
- "ext_clk_3", "ref_1m_clk", "pll_video_main_clk", };
+ "ext_clk_3", "ref_1m_clk", "pll_video_post_div", };
static const char *sim1_sel[] = { "osc", "pll_sys_pfd2_135m_clk",
"pll_sys_main_120m_clk", "pll_dram_533m_clk",
@@ -308,23 +308,23 @@
static const char *sim2_sel[] = { "osc", "pll_sys_pfd2_135m_clk",
"pll_sys_main_120m_clk", "pll_dram_533m_clk",
- "pll_usb_main_clk", "pll_video_main_clk", "pll_enet_125m_clk",
+ "pll_usb_main_clk", "pll_video_post_div", "pll_enet_125m_clk",
"pll_sys_pfd7_clk", };
static const char *gpt1_sel[] = { "osc", "pll_enet_100m_clk",
- "pll_sys_pfd0_392m_clk", "pll_enet_40m_clk", "pll_video_main_clk",
+ "pll_sys_pfd0_392m_clk", "pll_enet_40m_clk", "pll_video_post_div",
"ref_1m_clk", "pll_audio_post_div", "ext_clk_1", };
static const char *gpt2_sel[] = { "osc", "pll_enet_100m_clk",
- "pll_sys_pfd0_392m_clk", "pll_enet_40m_clk", "pll_video_main_clk",
+ "pll_sys_pfd0_392m_clk", "pll_enet_40m_clk", "pll_video_post_div",
"ref_1m_clk", "pll_audio_post_div", "ext_clk_2", };
static const char *gpt3_sel[] = { "osc", "pll_enet_100m_clk",
- "pll_sys_pfd0_392m_clk", "pll_enet_40m_clk", "pll_video_main_clk",
+ "pll_sys_pfd0_392m_clk", "pll_enet_40m_clk", "pll_video_post_div",
"ref_1m_clk", "pll_audio_post_div", "ext_clk_3", };
static const char *gpt4_sel[] = { "osc", "pll_enet_100m_clk",
- "pll_sys_pfd0_392m_clk", "pll_enet_40m_clk", "pll_video_main_clk",
+ "pll_sys_pfd0_392m_clk", "pll_enet_40m_clk", "pll_video_post_div",
"ref_1m_clk", "pll_audio_post_div", "ext_clk_4", };
static const char *trace_sel[] = { "osc", "pll_sys_pfd2_135m_clk",
@@ -339,12 +339,12 @@
static const char *csi_mclk_sel[] = { "osc", "pll_sys_pfd2_135m_clk",
"pll_sys_main_120m_clk", "pll_dram_533m_clk",
- "pll_enet_125m_clk", "pll_audio_post_div", "pll_video_main_clk",
+ "pll_enet_125m_clk", "pll_audio_post_div", "pll_video_post_div",
"pll_usb_main_clk", };
static const char *audio_mclk_sel[] = { "osc", "pll_sys_pfd2_135m_clk",
"pll_sys_main_120m_clk", "pll_dram_533m_clk",
- "pll_enet_125m_clk", "pll_audio_post_div", "pll_video_main_clk",
+ "pll_enet_125m_clk", "pll_audio_post_div", "pll_video_post_div",
"pll_usb_main_clk", };
static const char *wrclk_sel[] = { "osc", "pll_enet_40m_clk",
@@ -358,13 +358,13 @@
static const char *clko2_sel[] = { "osc", "pll_sys_main_240m_clk",
"pll_sys_pfd0_392m_clk", "pll_sys_pfd1_166m_clk", "pll_sys_pfd4_clk",
- "pll_audio_post_div", "pll_video_main_clk", "ckil", };
+ "pll_audio_post_div", "pll_video_post_div", "ckil", };
static const char *lvds1_sel[] = { "pll_arm_main_clk",
"pll_sys_main_clk", "pll_sys_pfd0_392m_clk", "pll_sys_pfd1_332m_clk",
"pll_sys_pfd2_270m_clk", "pll_sys_pfd3_clk", "pll_sys_pfd4_clk",
"pll_sys_pfd5_clk", "pll_sys_pfd6_clk", "pll_sys_pfd7_clk",
- "pll_audio_post_div", "pll_video_main_clk", "pll_enet_500m_clk",
+ "pll_audio_post_div", "pll_video_post_div", "pll_enet_500m_clk",
"pll_enet_250m_clk", "pll_enet_125m_clk", "pll_enet_100m_clk",
"pll_enet_50m_clk", "pll_enet_40m_clk", "pll_enet_25m_clk",
"pll_dram_main_clk", };
@@ -450,6 +450,10 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE, base + 0xf0, 19, 2, 0, test_div_table, &imx_ccm_lock);
clks[IMX7D_PLL_AUDIO_POST_DIV] = clk_register_divider_table(NULL, "pll_audio_post_div", "pll_audio_test_div",
CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE, base + 0xf0, 22, 2, 0, post_div_table, &imx_ccm_lock);
+ clks[IMX7D_PLL_VIDEO_TEST_DIV] = clk_register_divider_table(NULL, "pll_video_test_div", "pll_video_main_clk",
+ CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE, base + 0x130, 19, 2, 0, test_div_table, &imx_ccm_lock);
+ clks[IMX7D_PLL_VIDEO_POST_DIV] = clk_register_divider_table(NULL, "pll_video_post_div", "pll_video_test_div",
+ CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE, base + 0x130, 22, 2, 0, post_div_table, &imx_ccm_lock);
clks[IMX7D_PLL_SYS_PFD0_392M_CLK] = imx_clk_pfd("pll_sys_pfd0_392m_clk", "pll_sys_main_clk", base + 0xc0, 0);
clks[IMX7D_PLL_SYS_PFD1_332M_CLK] = imx_clk_pfd("pll_sys_pfd1_332m_clk", "pll_sys_main_clk", base + 0xc0, 1);
--
1.9.1
^ permalink raw reply related
* [PATCH v2] ARM: dts: imx6: RDU2: disable internal watchdog
From: Fabio Estevam @ 2018-01-03 17:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHQ1cqGE8GzHiHq0NRPoAKmrqv6ZDcFO+0=hnAEtog8jcnapyQ@mail.gmail.com>
On Wed, Jan 3, 2018 at 3:04 PM, Andrey Smirnov <andrew.smirnov@gmail.com> wrote:
> It seems that with exception of "audmux" and "iomuxc" the nodes are in
> alphabetical order (and I suspect "audmux" falls under the same
> exception "iomuxc" does?). What did you have in mind for that node?
> Placing it after "audmux"?
Yes, putting 'wdog' after audmux would be better.
Thanks
^ permalink raw reply
* [PATCH v2] ARM: dts: imx6: RDU2: disable internal watchdog
From: Andrey Smirnov @ 2018-01-03 17:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5AZU0LOcz6s2w-mThJfJeO0_mXga_1Cc2-aBkMOXk_-dA@mail.gmail.com>
On Mon, Jan 1, 2018 at 1:50 PM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Andrey,
>
> On Mon, Jan 1, 2018 at 7:24 PM, Andrey Smirnov <andrew.smirnov@gmail.com> wrote:
>
>> diff --git a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
>> index 6bef9a98678e..818bfc8692a5 100644
>> --- a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
>> +++ b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
>> @@ -838,6 +838,10 @@
>> status = "okay";
>> };
>>
>> +&wdog1 {
>> + status = "disabled";
>> +};
>> +
>> &audmux {
>
> We should keep the nodes in alphabetical order. Other than that:
>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
It seems that with exception of "audmux" and "iomuxc" the nodes are in
alphabetical order (and I suspect "audmux" falls under the same
exception "iomuxc" does?). What did you have in mind for that node?
Placing it after "audmux"?
Thanks,
Andrey Smirnov
^ permalink raw reply
* [GIT PULL 1/3] ARM: dts: exynos: DTS for v4.16
From: Krzysztof Kozlowski @ 2018-01-03 17:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAK8P3a2wSt5WjT26MT1QhYVxCcYJ8YV0cdsx19zvYZPqUgF+7Q@mail.gmail.com>
On Thu, Dec 21, 2017 at 10:29 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wed, Dec 20, 2017 at 6:36 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
>> Krzysztof Kozlowski (2):
>> ARM: dts: exynos: Add missing interrupt-controller properties to Exynos5410 PMU
>
> I just looked through the remaining warnings in 4.15 and noticed that
> you had sent this
> only for 4.16, not as a bug fix.
>
> Looking closer, it seems incorrect:
>
> According to the comment added to exynos_dt_pmu_match[] in commit
> 8b283c025443 ("ARM: exynos4/5: convert pmu wakeup to stacked domains"),
> the RTC is not able to wake up the system through the PMU on Exynos5410,
> unlike Exynos5420. However, when the RTC DT node got added in commit
> e1e146b1b062 ("ARM: dts: exynos: Add RTC and I2C to Exynos5410"), it was
> a straight copy of the Exynos5420 node, which now causes the warning from dtc,
> and now you add the "interrupt-controller" property to the device node, but
> the code still doesn't handle it at all.
I received a Odroid XU board, based on Exynos5410, from Markus Reichl
(big thanks! it is second board from fivetechno.de) but I did not
setup testing infrastructure on it yet.
I see you pulled my entire DT branch, including the change around
Exynos5410 PMU's interrupts. I think your analysis and your follow-up
patch are correct which means that RTC on Exynso5410 might be broken
now. It is getting late in RC-cycle so I could send a follow-up pull
with fixed DT including your patch [1].
The other way is to drop my commit entirely. I could prepare a rebased
DT branch for this as well.
Let me know if you have any preferences.
Best regards,
Krzysztof
[1] https://patchwork.kernel.org/patch/10128431/
^ permalink raw reply
* [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface
From: Stefan Chulski @ 2018-01-03 17:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180101132520.GB28752@n2100.armlinux.org.uk>
> > > -----Original Message-----
> > > Hi Russell,
> > >
> > > Indeed. RGMII MAC behaves same way, although it shouldn't be named
> > > as 'in- band' to be on par with the specifications. Anyway - this
> > > one is rather a stub for being able to work with ACPI, so once the
> > > MDIO bus works there, this will be out of any concerns.
> >
> > Hi Marcin,
> >
> > This is correct.
> > "in-band" supported only for SGMII mode.
> > IRQ link interrupt depend on "in-band"' auto negation only if "in-band"'
> enabled.
> > But IRQ link interrupt could be triggered with "in-band", "out-band" or with
> specific fixed speed/duplex/flow_contol.
>
> Hi Stefan,
>
> How does this work in RGMII mode - is this handled by the PP2 polling the PHY
> to get the speed, duplex and flow control settings?
>
IRQ interrupt doesn't handled speed, duplex and flow control settings.
It's just raised if number of criterions met:
1) Physical signal detected by MAC
2) MAC auto negotiation succeeded(valid only auto negotiation enabled in MAC and "in-band" bypass disabled)
So if auto negotiation mechanism disabled in MAC or bypassed, link status would changes to up and IRQ interrupt be triggered.
In case of RGMII mode obviously we don't have "in-band" auto negotiation and "out-band" cannot be used in Kernel(due to missed locks).
So auto negotiation should be disabled on MAC level and speed/duplex/flow_contol would be negotiate by PHY.
phylink/phylib infrastructure should provide speed/duplex/flow_contol(that were agreed between PHY's) to ppv2 driver.
Stefan.
^ permalink raw reply
* Re: [PATCH v4 0/7] ARM: davinci: convert to common clock framework
From: Sekhar Nori @ 2018-01-03 16:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHCN7x+50anim4-F++Fd78tuPo9+OdKgvOZ-+jWc=0QFcng5CA@mail.gmail.com>
On Tuesday 02 January 2018 08:40 PM, Adam Ford wrote:
> On Sun, Dec 31, 2017 at 5:39 PM, David Lechner <david@lechnology.com> wrote:
>> This series converts macreqh-davinci to use the common clock framework.
>>
>> Basically, this series does some cleanup and rearranging to get things
>> ready for the conversion. Then there is a patch to add new driver in
>> drivers/clk and finally a patch to make the conversion from the mach
>> clock drivers to the new drivers.
>>
>> I have tested this on LEGO MINDSTORMS EV3 (TI AM1808), so I am confident
>> that I haven't broken anything (too badly) in da850. But, I don't have
>> other hardware to test.
>
> I tested this on a DA850-EVM, but I was not able to successfully get it to boot.
>
> It hangs during boot with no errors, oops's, or panics, but the
> standard 4.15.0-rc5 from linux-stable booted just fine.
>
> Maybe one of the TI guys will have some suggestions, but as-is, it
> appears to break at least the DA850-evm.
I haven't gotten to looking into this series yet. Should happen by
Friday this week.
>> The one thing that I know I have broken is CPU frequency scaling on da850.
>> I don't think it was working with device tree anyway, so I can't really test
>> it with the hardware I have. I'm hoping that it will be OK to defer fixing
>> it and add device tree support at the same time.
>>
>
> I agree with you that it's broken in the device tree, but I don't
> think it ever worked.
It was working with legacy board file method. Its not supported with DT,
yes. Most probably, the generic cpufreq-dt support needs to be used for
the DT case.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH v5 6/9] ACPI/PPTT: Add topology parsing code
From: Jeremy Linton @ 2018-01-03 16:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <019601d3846f$c8708550$59518ff0$@codeaurora.org>
Hi,
On 01/03/2018 02:49 AM, vkilari at codeaurora.org wrote:
> Hi Jeremy,
>
> Sorry, I don't have your previous patch emails to reply on right patch
> context.
> So commenting on top of this patch.
>
> AFAIU, the PPTT v5 patches still rely on CLIDR_EL1 register to know the type
> of
> Caches enabled/available on the platform. With PPTT, it should not rely on
> architecture
> registers. There can be platforms which can report cache availability in
> PPTT but not in
> architecture registers.
>
> The following code snippet shows usage of CLIDR_EL1
>
> In arch/arm64/kernel/cacheinfo.c
>
> static inline enum cache_type get_cache_type(int level)
> {
> u64 clidr;
>
> if (level > MAX_CACHE_LEVEL)
> return CACHE_TYPE_NOCACHE;
> clidr = read_sysreg(clidr_el1);
> return CLIDR_CTYPE(clidr, level);
> }
>
> static int __populate_cache_leaves(unsigned int cpu)
> {
> unsigned int level, idx;
> enum cache_type type;
> struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
> struct cacheinfo *this_leaf = this_cpu_ci->info_list;
>
> for (idx = 0, level = 1; level <= this_cpu_ci->num_levels &&
> idx < this_cpu_ci->num_leaves; idx++, level++) {
> type = get_cache_type(level);
> if (type == CACHE_TYPE_SEPARATE) {
> ci_leaf_init(this_leaf++, CACHE_TYPE_DATA, level);
> ci_leaf_init(this_leaf++, CACHE_TYPE_INST, level);
> } else {
> ci_leaf_init(this_leaf++, type, level);
> }
> }
> return 0;
> }
>
> In populate_cache_leaves() the cache type is read from CLIDR_EL1 register.
> If CLIDR_EL1 reports CACHE_TYPE_NOCACHE for a particular level then sysfs
> entry
> /sys/devices/system/cpu/cpu0/index<n>/type is not created and hence
> userspace tools
> like lstopo will not report this cache level.
This sounds suspiciously like one of things tweaked between v4->v5. If
you look at update_cache_properties() in patch 2/9, you will see that we
only update/find NOCACHE nodes and convert them to UNIFIED when all the
attributes in the node are supplied.
This means that if the node has an incomplete set of attributes we won't
update it. Can you verify that you have all those attributes set for
nodes which aren't being described by the hardware?
Thanks,
>
> Regards
> Vijay
>
>> -----Original Message-----
>> From: linux-arm-kernel
> [mailto:linux-arm-kernel-bounces at lists.infradead.org]
>> On Behalf Of Rafael J. Wysocki
>> Sent: Thursday, December 14, 2017 4:40 AM
>> To: Jeremy Linton <jeremy.linton@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>; Jonathan.Zhang at cavium.com;
>> Jayachandran.Nair at cavium.com; Lorenzo Pieralisi
>> <lorenzo.pieralisi@arm.com>; Catalin Marinas <catalin.marinas@arm.com>;
>> Rafael J. Wysocki <rafael@kernel.org>; jhugo at codeaurora.org; Will Deacon
>> <will.deacon@arm.com>; Linux PM <linux-pm@vger.kernel.org>; Rafael J.
>> Wysocki <rjw@rjwysocki.net>; Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org>; Linux Kernel Mailing List <linux-
>> kernel at vger.kernel.org>; ACPI Devel Maling List
> <linux-acpi@vger.kernel.org>;
>> Viresh Kumar <viresh.kumar@linaro.org>; Hanjun Guo
>> <hanjun.guo@linaro.org>; Al Stone <ahs3@redhat.com>; Sudeep Holla
>> <sudeep.holla@arm.com>; austinwc at codeaurora.org;
>> wangxiongfeng2 at huawei.com; linux-arm-kernel at lists.infradead.org; Len
>> Brown <lenb@kernel.org>
>> Subject: Re: [PATCH v5 6/9] ACPI/PPTT: Add topology parsing code
>>
>> On Thu, Dec 14, 2017 at 12:06 AM, Jeremy Linton <jeremy.linton@arm.com>
>> wrote:
>>> Hi,
>>>
>>>
>>> On 12/13/2017 04:28 PM, Rafael J. Wysocki wrote:
>>>>
>>>> On Wed, Dec 13, 2017 at 6:38 PM, Lorenzo Pieralisi
>>>> <lorenzo.pieralisi@arm.com> wrote:
>>>>>
>>>>> On Tue, Dec 12, 2017 at 10:13:08AM -0600, Jeremy Linton wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> First, thanks for taking a look at this.
>>>>>>
>>>>>> On 12/11/2017 07:12 PM, Rafael J. Wysocki wrote:
>>>>>>>
>>>>>>> On Friday, December 1, 2017 11:23:27 PM CET Jeremy Linton wrote:
>>>>>>>>
>>>>>>>> The PPTT can be used to determine the groupings of CPU's at given
>>>>>>>> levels in the system. Lets add a few routines to the PPTT parsing
>>>>>>>> code to return a unique id for each unique level in the processor
>>>>>>>> hierarchy. This can then be matched to build
>>>>>>>> thread/core/cluster/die/package/etc mappings for each processing
>>>>>>>> element in the system.
>>>>>>>>
>>>>>>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>>>>>
>>>>>>>
>>>>>>> Why can't this be folded into patch [2/9]?
>>>>>>
>>>>>>
>>>>>> It can, and I will be happy squash it.
>>>>>>
>>>>>> It was requested that the topology portion of the parser be split
>>>>>> out back in v3.
>>>>>>
>>>>>> https://www.spinics.net/lists/linux-acpi/msg78487.html
>>>>>
>>>>>
>>>>> I asked to split cache/topology since I am not familiar with cache
>>>>> code and Sudeep - who looks after the cache code - won't be able to
>>>>> review this series in time for v4.16.
>>>>
>>>>
>>>> OK, so why do we need it in 4.16?
>>>
>>>
>>> I think its more case of as soon as possible. That is because there
>>> are machines where the topology is completely incorrect due to
>>> assumptions the kernel makes based on registers that aren't defined
>>> for that purpose (say describing which cores are in a physical socket,
>>> or LLC's attached to interconnects or memory controllers).
>>>
>>> This incorrect topology information is reported to things like the
>>> kernel scheduler, which then makes poor scheduling decisions resulting
>>> in sub-optimal system performance.
>>>
>>> This patchset (and ACPI 6.2) clears up a lot of those problems.
>>
>> As long as the ACPI tables are as expected that is, I suppose?
>>
>> Anyway, fair enough, but I don't want to rush it in.
>>
>> Thanks,
>> Rafael
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH] arch: drop duplicate exports of abort()
From: Vineet Gupta @ 2018-01-03 16:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102103311.706364-1-arnd@arndb.de>
On 01/02/2018 02:33 AM, Arnd Bergmann wrote:
> We now have exports in both architecture code in in common code,
> which causes a link failure when symbol versioning is eanbled, on
> four architectures:
>
> kernel/exit.o: In function `__crc_abort':
> exit.c:(*ABS*+0xc0e2ec8b): multiple definition of `__crc_abort'
>
> This removes the four architecture specific exports and only
> leaves the export next to the __weak symbol.
>
> Fixes: mmotm ("kernel/exit.c: export abort() to modules")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Andrew, can you apply this to -mm on top of the other patch?
> ---
> arch/arc/kernel/traps.c | 1 -
> arch/arm/kernel/traps.c | 1 -
> arch/m32r/kernel/traps.c | 1 -
> arch/unicore32/kernel/traps.c | 1 -
> 4 files changed, 4 deletions(-)
>
> diff --git a/arch/arc/kernel/traps.c b/arch/arc/kernel/traps.c
> index 51a55b06cb2a..133a4dae41fe 100644
> --- a/arch/arc/kernel/traps.c
> +++ b/arch/arc/kernel/traps.c
> @@ -169,4 +169,3 @@ void abort(void)
> {
> __asm__ __volatile__("trap_s 5\n");
> }
> -EXPORT_SYMBOL(abort);
FWIW, this hunk did not yet hit mainline. I've removed it from my patch in
for-curr and re-pushed !
Thx,
-Vineet
^ permalink raw reply
* [PATCH] s3mci: mark debug_regs[] as static
From: Ulf Hansson @ 2018-01-03 16:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180103092705.1949763-1-arnd@arndb.de>
On 3 January 2018 at 10:26, Arnd Bergmann <arnd@arndb.de> wrote:
> The global array clashes with a newly added symbol of the same name:
>
> drivers/staging/ccree/cc_debugfs.o:(.data+0x0): multiple definition of `debug_regs'
> drivers/mmc/host/s3cmci.o:(.data+0x70): first defined here
>
> We should fix both, this one addresses the s3cmci driver by removing
> the symbol from the global namespace.
>
> Fixes: 9bdd203b4dc8 ("s3cmci: add debugfs support for examining driver and hardware state")
Seems like we need a stable tag as well, would you mind adding it in
the next re-spin?
> Fixes: b3ec9a6736f2 ("staging: ccree: staging: ccree: replace sysfs by debugfs interface")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/mmc/host/s3cmci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
> index 36daee1e6588..24b27e0957e7 100644
> --- a/drivers/mmc/host/s3cmci.c
> +++ b/drivers/mmc/host/s3cmci.c
> @@ -1421,7 +1421,7 @@ static const struct file_operations s3cmci_fops_state = {
>
> #define DBG_REG(_r) { .addr = S3C2410_SDI##_r, .name = #_r }
>
> -struct s3cmci_reg {
> +static struct s3cmci_reg {
> unsigned short addr;
> unsigned char *name;
> } debug_regs[] = {
I am not very fond of these kind of declarations/definitions. How
about if we instead move the declaration of debug_regs[] to a separate
line? Moreover, should it be const?
static struct s3cmci_reg debug_regs[] = {
Kind regards
Uffe
^ permalink raw reply
* [PATCH V3 3/3] arm64: Extend early page table code to allow for larger kernels
From: Ard Biesheuvel @ 2018-01-03 16:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180103162011.xswbj25mszudlmyt@capper-debian.cambridge.arm.com>
On 3 January 2018 at 16:20, Steve Capper <steve.capper@arm.com> wrote:
> On Tue, Jan 02, 2018 at 10:01:29PM +0000, Ard Biesheuvel wrote:
>> Hi Steve,
>
> Hi Ard,
>
>>
>> On 2 January 2018 at 15:12, Steve Capper <steve.capper@arm.com> wrote:
>> > Currently the early assembler page table code assumes that precisely
>> > 1xpgd, 1xpud, 1xpmd are sufficient to represent the early kernel text
>> > mappings.
>> >
>> > Unfortunately this is rarely the case when running with a 16KB granule,
>> > and we also run into limits with 4KB granule when building much larger
>> > kernels.
>> >
>> > This patch re-writes the early page table logic to compute indices of
>> > mappings for each level of page table, and if multiple indices are
>> > required, the next-level page table is scaled up accordingly.
>> >
>> > Also the required size of the swapper_pg_dir is computed at link time
>> > to cover the mapping [KIMAGE_ADDR + VOFFSET, _end]. When KASLR is
>> > enabled, an extra page is set aside for each level that may require extra
>> > entries at runtime.
>> >
>> > Signed-off-by: Steve Capper <steve.capper@arm.com>
>> >
>> > ---
>> > Changed in V3:
>> > Corrected KASLR computation
>> > Rebased against arm64/for-next/core, particularly Kristina's 52-bit
>> > PA series.
>> > ---
>> > arch/arm64/include/asm/kernel-pgtable.h | 47 ++++++++++-
>> > arch/arm64/include/asm/pgtable.h | 1 +
>> > arch/arm64/kernel/head.S | 145 +++++++++++++++++++++++---------
>> > arch/arm64/kernel/vmlinux.lds.S | 1 +
>> > arch/arm64/mm/mmu.c | 3 +-
>> > 5 files changed, 157 insertions(+), 40 deletions(-)
>> >
>> > diff --git a/arch/arm64/include/asm/kernel-pgtable.h b/arch/arm64/include/asm/kernel-pgtable.h
>> > index 77a27af01371..82386e860dd2 100644
>> > --- a/arch/arm64/include/asm/kernel-pgtable.h
>> > +++ b/arch/arm64/include/asm/kernel-pgtable.h
>> > @@ -52,7 +52,52 @@
>> > #define IDMAP_PGTABLE_LEVELS (ARM64_HW_PGTABLE_LEVELS(PHYS_MASK_SHIFT))
>> > #endif
>> >
>> > -#define SWAPPER_DIR_SIZE (SWAPPER_PGTABLE_LEVELS * PAGE_SIZE)
>> > +
>> > +/*
>> > + * If KASLR is enabled, then an offset K is added to the kernel address
>> > + * space. The bottom 21 bits of this offset are zero to guarantee 2MB
>> > + * alignment for PA and VA.
>> > + *
>> > + * For each pagetable level of the swapper, we know that the shift will
>> > + * be larger than 21 (for the 4KB granule case we use section maps thus
>> > + * the smallest shift is actually 30) thus there is the possibility that
>> > + * KASLR can increase the number of pagetable entries by 1, so we make
>> > + * room for this extra entry.
>> > + *
>> > + * Note KASLR cannot increase the number of required entries for a level
>> > + * by more than one because it increments both the virtual start and end
>> > + * addresses equally (the extra entry comes from the case where the end
>> > + * address is just pushed over a boundary and the start address isn't).
>> > + */
>> > +
>> > +#ifdef CONFIG_RANDOMIZE_BASE
>> > +#define EARLY_KASLR (1)
>> > +#else
>> > +#define EARLY_KASLR (0)
>> > +#endif
>> > +
>> > +#define EARLY_ENTRIES(vstart, vend, shift) (((vend) >> (shift)) \
>> > + - ((vstart) >> (shift)) + 1 + EARLY_KASLR)
>> > +
>> > +#define EARLY_PGDS(vstart, vend) (EARLY_ENTRIES(vstart, vend, PGDIR_SHIFT))
>> > +
>> > +#if SWAPPER_PGTABLE_LEVELS > 3
>> > +#define EARLY_PUDS(vstart, vend) (EARLY_ENTRIES(vstart, vend, PUD_SHIFT))
>> > +#else
>> > +#define EARLY_PUDS(vstart, vend) (0)
>> > +#endif
>> > +
>> > +#if SWAPPER_PGTABLE_LEVELS > 2
>> > +#define EARLY_PMDS(vstart, vend) (EARLY_ENTRIES(vstart, vend, SWAPPER_TABLE_SHIFT))
>> > +#else
>> > +#define EARLY_PMDS(vstart, vend) (0)
>> > +#endif
>> > +
>> > +#define EARLY_PAGES(vstart, vend) ( 1 /* PGDIR page */ \
>> > + + EARLY_PGDS((vstart), (vend)) /* each PGDIR needs a next level page table */ \
>> > + + EARLY_PUDS((vstart), (vend)) /* each PUD needs a next level page table */ \
>> > + + EARLY_PMDS((vstart), (vend))) /* each PMD needs a next level page table */
>> > +#define SWAPPER_DIR_SIZE (PAGE_SIZE * EARLY_PAGES(KIMAGE_VADDR + TEXT_OFFSET, _end))
>> > #define IDMAP_DIR_SIZE (IDMAP_PGTABLE_LEVELS * PAGE_SIZE)
>> >
>> > #ifdef CONFIG_ARM64_SW_TTBR0_PAN
>> > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> > index bfa237e892f1..54b0a8398055 100644
>> > --- a/arch/arm64/include/asm/pgtable.h
>> > +++ b/arch/arm64/include/asm/pgtable.h
>> > @@ -706,6 +706,7 @@ static inline void pmdp_set_wrprotect(struct mm_struct *mm,
>> > #endif
>> >
>> > extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
>> > +extern pgd_t swapper_pg_end[];
>> > extern pgd_t idmap_pg_dir[PTRS_PER_PGD];
>> > extern pgd_t tramp_pg_dir[PTRS_PER_PGD];
>> >
>> > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> > index 66f01869e97c..539e2642ed41 100644
>> > --- a/arch/arm64/kernel/head.S
>> > +++ b/arch/arm64/kernel/head.S
>> > @@ -191,44 +191,110 @@ ENDPROC(preserve_boot_args)
>> > .endm
>> >
>> > /*
>> > - * Macro to populate the PGD (and possibily PUD) for the corresponding
>> > - * block entry in the next level (tbl) for the given virtual address.
>> > + * Macro to populate page table entries, these entries can be pointers to the next level
>> > + * or last level entries pointing to physical memory.
>> > *
>> > - * Preserves: tbl, next, virt
>> > - * Corrupts: ptrs_per_pgd, tmp1, tmp2
>> > + * tbl: page table address
>> > + * rtbl: pointer to page table or physical memory
>> > + * index: start index to write
>> > + * eindex: end index to write - [index, eindex] written to
>> > + * flags: flags for pagetable entry to or in
>> > + * inc: increment to rtbl between each entry
>> > + * tmp1: temporary variable
>> > + *
>> > + * Preserves: tbl, eindex, flags, inc
>> > + * Corrupts: index, tmp1
>> > + * Returns: rtbl
>> > */
>> > - .macro create_pgd_entry, tbl, virt, ptrs_per_pgd, tmp1, tmp2
>> > - create_table_entry \tbl, \virt, PGDIR_SHIFT, \ptrs_per_pgd, \tmp1, \tmp2
>> > -#if SWAPPER_PGTABLE_LEVELS > 3
>> > - mov \ptrs_per_pgd, PTRS_PER_PUD
>> > - create_table_entry \tbl, \virt, PUD_SHIFT, \ptrs_per_pgd, \tmp1, \tmp2
>> > -#endif
>> > -#if SWAPPER_PGTABLE_LEVELS > 2
>> > - mov \ptrs_per_pgd, PTRS_PER_PTE
>> > - create_table_entry \tbl, \virt, SWAPPER_TABLE_SHIFT, \ptrs_per_pgd, \tmp1, \tmp2
>> > -#endif
>> > + .macro populate_entries, tbl, rtbl, index, eindex, flags, inc, tmp1
>> > +9999: phys_to_pte \rtbl, \tmp1
>>
>> I know this is existing code, but you could take the opportunity to
>> replace this label with
>>
>> .L\@ :
>>
>> (and update the branch instruction accordingly) so that we don't have
>> to rely on the uniqueness of '9999'
>
> Thanks, I must confess I was unaware of that technique and had to look
> it up; I will incorporate this into the patch.
>
I only discovered it recently myself, but it is much better than
relying on pseudo-random numeric labels.
>>
>> > + orr \tmp1, \tmp1, \flags // tmp1 = table entry
>> > + str \tmp1, [\tbl, \index, lsl #3]
>> > + add \rtbl, \rtbl, \inc // rtbl = pa next level
>> > + add \index, \index, #1
>> > + cmp \index, \eindex
>> > + b.ls 9999b
>> > .endm
>> >
>> > /*
>> > - * Macro to populate block entries in the page table for the start..end
>> > - * virtual range (inclusive).
>> > + * Compute indices of table entries from virtual address range. If multiple entries
>> > + * were needed in the previous page table level then the next page table level is assumed
>> > + * to be composed of multiple pages. (This effectively scales the end index).
>> > + *
>> > + * vstart: virtual address of start of range
>> > + * vend: virtual address of end of range
>> > + * shift: shift used to transform virtual address into index
>> > + * ptrs: number of entries in page table
>> > + * istart: index in table corresponding to vstart
>> > + * iend: index in table corresponding to vend
>> > + * count: On entry: how many entries required in previous level, scales our end index
>> > + * On exit: returns how many entries required for next page table level
>> > *
>>
>> If you make 'count' the number of /additional/ entries, you no longer
>> have to add/sub #1 each time.
>
> Agreed, I'll simplify this.
>
>>
>> > - * Preserves: tbl, flags
>> > - * Corrupts: phys, start, end, tmp, pstate
>> > + * Preserves: vstart, vend, shift, ptrs
>> > + * Returns: istart, iend, count
>> > */
>> > - .macro create_block_map, tbl, flags, phys, start, end, tmp
>> > - lsr \start, \start, #SWAPPER_BLOCK_SHIFT
>> > - and \start, \start, #PTRS_PER_PTE - 1 // table index
>> > - bic \phys, \phys, #SWAPPER_BLOCK_SIZE - 1
>> > - lsr \end, \end, #SWAPPER_BLOCK_SHIFT
>> > - and \end, \end, #PTRS_PER_PTE - 1 // table end index
>> > -9999: phys_to_pte \phys, \tmp
>> > - orr \tmp, \tmp, \flags // table entry
>> > - str \tmp, [\tbl, \start, lsl #3] // store the entry
>> > - add \start, \start, #1 // next entry
>> > - add \phys, \phys, #SWAPPER_BLOCK_SIZE // next block
>> > - cmp \start, \end
>> > - b.ls 9999b
>> > + .macro compute_indices, vstart, vend, shift, ptrs, istart, iend, count
>> > + lsr \iend, \vend, \shift
>> > + mov \istart, \ptrs
>> > + sub \istart, \istart, #1
>> > + and \iend, \iend, \istart // iend = (vend >> shift) & (ptrs - 1)
>> > + mov \istart, \ptrs
>> > + sub \count, \count, #1
>> > + mul \istart, \istart, \count
>> > + add \iend, \iend, \istart // iend += (count - 1) * ptrs
>> > + // our entries span multiple tables
>> > +
>> > + lsr \istart, \vstart, \shift
>> > + mov \count, \ptrs
>> > + sub \count, \count, #1
>> > + and \istart, \istart, \count
>> > +
>> > + sub \count, \iend, \istart
>> > + add \count, \count, #1
>> > + .endm
>> > +
>>
>> You can simplify this macro by using an immediate for \ptrs. Please
>> see the diff below [whitespace mangling courtesy of Gmail]
>
> Thanks, I like the simplification a lot. For 52-bit kernel VAs though, I
> will need a variable PTRS_PER_PGD at compile time.
>
> For 52-bit kernel PAs one can just use the maximum number of ptrs available.
> For a 48-bit PA the leading address bits will always be zero, thus we
> will derive the same PGD indices from both 48 and 52 bit PTRS_PER_PGD.
>
> For kernel VAs, because the leading address bits are one, we need to use a
> PTRS_PER_PGD corresponding to the VA size.
>
OK, so you are saying you shouldn't mask off too many bits, right? In
any case, I suppose you can just use the same trick as I used for
.Lidmap_ptrs_per_pgd, i.e., use .set to assign the correct value and
pass that into the macro.
>> > +/*
>> > + * Map memory for specified virtual address range. Each level of page table needed supports
>> > + * multiple entries. If a level requires n entries the next page table level is assumed to be
>> > + * formed from n pages.
>> > + *
>> > + * tbl: location of page table
>> > + * rtbl: address to be used for first level page table entry (typically tbl + PAGE_SIZE)
>> > + * vstart: start address to map
>> > + * vend: end address to map - we map [vstart, vend]
>> > + * flags: flags to use to map last level entries
>> > + * phys: physical address corresponding to vstart - physical memory is contiguous
>> > + * pgds: the number of pgd entries
>> > + *
>> > + * Temporaries: istart, iend, tmp, count, sv - these need to be different registers
>> > + * Preserves: vstart, vend, flags
>> > + * Corrupts: tbl, rtbl, istart, iend, tmp, count, sv
>> > + */
>> > + .macro map_memory, tbl, rtbl, vstart, vend, flags, phys, pgds, istart, iend, tmp, count, sv
>> > + add \rtbl, \tbl, #PAGE_SIZE
>> > + mov \sv, \rtbl
>> > + mov \count, #1
>>
>> #0 if you make \count the number of additional entries.
>>
>> In any case, for the series:
>>
>> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Thanks!
>
>>
>> > + compute_indices \vstart, \vend, #PGDIR_SHIFT, \pgds, \istart, \iend, \count
>> > + populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
>> > + mov \tbl, \sv
>> > + mov \sv, \rtbl
>> > +
>> > +#if SWAPPER_PGTABLE_LEVELS > 3
>> > + compute_indices \vstart, \vend, #PUD_SHIFT, #PTRS_PER_PUD, \istart, \iend, \count
>> > + populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
>> > + mov \tbl, \sv
>> > + mov \sv, \rtbl
>> > +#endif
>> > +
>> > +#if SWAPPER_PGTABLE_LEVELS > 2
>> > + compute_indices \vstart, \vend, #SWAPPER_TABLE_SHIFT, #PTRS_PER_PMD, \istart, \iend, \count
>> > + populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
>> > + mov \tbl, \sv
>> > +#endif
>> > +
>> > + compute_indices \vstart, \vend, #SWAPPER_BLOCK_SHIFT, #PTRS_PER_PTE, \istart, \iend, \count
>> > + bic \count, \phys, #SWAPPER_BLOCK_SIZE - 1
>> > + populate_entries \tbl, \count, \istart, \iend, \flags, #SWAPPER_BLOCK_SIZE, \tmp
>> > .endm
>> >
>> > /*
>> > @@ -246,14 +312,16 @@ __create_page_tables:
>> > * dirty cache lines being evicted.
>> > */
>> > adrp x0, idmap_pg_dir
>> > - ldr x1, =(IDMAP_DIR_SIZE + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE)
>> > + adrp x1, swapper_pg_end
>> > + sub x1, x1, x0
>> > bl __inval_dcache_area
>> >
>> > /*
>> > * Clear the idmap and swapper page tables.
>> > */
>> > adrp x0, idmap_pg_dir
>> > - ldr x1, =(IDMAP_DIR_SIZE + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE)
>> > + adrp x1, swapper_pg_end
>> > + sub x1, x1, x0
>> > 1: stp xzr, xzr, [x0], #16
>> > stp xzr, xzr, [x0], #16
>> > stp xzr, xzr, [x0], #16
>> > @@ -318,10 +386,10 @@ __create_page_tables:
>> > #endif
>> > 1:
>> > ldr_l x4, idmap_ptrs_per_pgd
>> > - create_pgd_entry x0, x3, x4, x5, x6
>> > mov x5, x3 // __pa(__idmap_text_start)
>> > adr_l x6, __idmap_text_end // __pa(__idmap_text_end)
>> > - create_block_map x0, x7, x3, x5, x6, x4
>> > +
>> > + map_memory x0, x1, x3, x6, x7, x3, x4, x10, x11, x12, x13, x14
>> >
>> > /*
>> > * Map the kernel image (starting with PHYS_OFFSET).
>> > @@ -330,12 +398,12 @@ __create_page_tables:
>> > mov_q x5, KIMAGE_VADDR + TEXT_OFFSET // compile time __va(_text)
>> > add x5, x5, x23 // add KASLR displacement
>> > mov x4, PTRS_PER_PGD
>> > - create_pgd_entry x0, x5, x4, x3, x6
>> > adrp x6, _end // runtime __pa(_end)
>> > adrp x3, _text // runtime __pa(_text)
>> > sub x6, x6, x3 // _end - _text
>> > add x6, x6, x5 // runtime __va(_end)
>> > - create_block_map x0, x7, x3, x5, x6, x4
>> > +
>> > + map_memory x0, x1, x5, x6, x7, x3, x4, x10, x11, x12, x13, x14
>> >
>> > /*
>> > * Since the page tables have been populated with non-cacheable
>> > @@ -343,7 +411,8 @@ __create_page_tables:
>> > * tables again to remove any speculatively loaded cache lines.
>> > */
>> > adrp x0, idmap_pg_dir
>> > - ldr x1, =(IDMAP_DIR_SIZE + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE)
>> > + adrp x1, swapper_pg_end
>> > + sub x1, x1, x0
>> > dmb sy
>> > bl __inval_dcache_area
>> >
>> > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
>> > index 4c7112a47469..0221aca6493d 100644
>> > --- a/arch/arm64/kernel/vmlinux.lds.S
>> > +++ b/arch/arm64/kernel/vmlinux.lds.S
>> > @@ -230,6 +230,7 @@ SECTIONS
>> > #endif
>> > swapper_pg_dir = .;
>> > . += SWAPPER_DIR_SIZE;
>> > + swapper_pg_end = .;
>> >
>> > __pecoff_data_size = ABSOLUTE(. - __initdata_begin);
>> > _end = .;
>> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> > index 4071602031ed..fdac11979bae 100644
>> > --- a/arch/arm64/mm/mmu.c
>> > +++ b/arch/arm64/mm/mmu.c
>> > @@ -644,7 +644,8 @@ void __init paging_init(void)
>> > * allocated with it.
>> > */
>> > memblock_free(__pa_symbol(swapper_pg_dir) + PAGE_SIZE,
>> > - SWAPPER_DIR_SIZE - PAGE_SIZE);
>> > + __pa_symbol(swapper_pg_end) - __pa_symbol(swapper_pg_dir)
>> > + - PAGE_SIZE);
>> > }
>> >
>> > /*
>> > --
>> > 2.11.0
>> >
>>
>>
>> ------8<---------
>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 539e2642ed41..0432dd8d083c 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -235,9 +235,7 @@ ENDPROC(preserve_boot_args)
>> */
>> .macro compute_indices, vstart, vend, shift, ptrs, istart, iend, count
>> lsr \iend, \vend, \shift
>> - mov \istart, \ptrs
>> - sub \istart, \istart, #1
>> - and \iend, \iend, \istart // iend = (vend >> shift) & (ptrs - 1)
>> + and \iend, \iend, \ptrs - 1 // iend = (vend >> shift) & (ptrs - 1)
>> mov \istart, \ptrs
>> sub \count, \count, #1
>> mul \istart, \istart, \count
>> @@ -245,9 +243,7 @@ ENDPROC(preserve_boot_args)
>> // our entries span multiple tables
>>
>> lsr \istart, \vstart, \shift
>> - mov \count, \ptrs
>> - sub \count, \count, #1
>> - and \istart, \istart, \count
>> + and \istart, \istart, \ptrs - 1
>>
>> sub \count, \iend, \istart
>> add \count, \count, #1
>> @@ -376,6 +372,7 @@ __create_page_tables:
>>
>> mov x4, EXTRA_PTRS
>> create_table_entry x0, x3, EXTRA_SHIFT, x4, x5, x6
>> + .set .Lidmap_ptrs_per_pgd, PTRS_PER_PGD
>> #else
>> /*
>> * If VA_BITS == 48, we don't have to configure an additional
>> @@ -383,13 +380,13 @@ __create_page_tables:
>> */
>> mov x4, #1 << (PHYS_MASK_SHIFT - PGDIR_SHIFT)
>> str_l x4, idmap_ptrs_per_pgd, x5
>> + .set .Lidmap_ptrs_per_pgd, 1 << (PHYS_MASK_SHIFT - PGDIR_SHIFT)
>> #endif
>> 1:
>> - ldr_l x4, idmap_ptrs_per_pgd
>> mov x5, x3 // __pa(__idmap_text_start)
>> adr_l x6, __idmap_text_end // __pa(__idmap_text_end)
>>
>> - map_memory x0, x1, x3, x6, x7, x3, x4, x10, x11, x12, x13, x14
>> + map_memory x0, x1, x3, x6, x7, x3, .Lidmap_ptrs_per_pgd, x10,
>> x11, x12, x13, x14
>>
>> /*
>> * Map the kernel image (starting with PHYS_OFFSET).
>> @@ -397,13 +394,12 @@ __create_page_tables:
>> adrp x0, swapper_pg_dir
>> mov_q x5, KIMAGE_VADDR + TEXT_OFFSET // compile time __va(_text)
>> add x5, x5, x23 // add KASLR displacement
>> - mov x4, PTRS_PER_PGD
>> adrp x6, _end // runtime __pa(_end)
>> adrp x3, _text // runtime __pa(_text)
>> sub x6, x6, x3 // _end - _text
>> add x6, x6, x5 // runtime __va(_end)
>>
>> - map_memory x0, x1, x5, x6, x7, x3, x4, x10, x11, x12, x13, x14
>> + map_memory x0, x1, x5, x6, x7, x3, PTRS_PER_PGD, x10, x11, x12, x13, x14
>>
>> /*
>> * Since the page tables have been populated with non-cacheable
>
> Cheers,
> --
> Steve
^ permalink raw reply
* [PATCH] ARM: decompressor: Make RAM sections Outer non-cacheable
From: Silvano di Ninno @ 2018-01-03 16:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1514997170-31906-1-git-send-email-silvano.dininno@nxp.com>
The kernel decompressor is not aware of external caches.
If those caches are enabled (It can be so if Linux runs in
Non-secure world and the secure world OS has enabled them),
it leads to stale data in external caches and coherency
problem when Linux enables them.
This patch makes the RAM sections Outer non-cacheable
Signed-off-by: Silvano di Ninno <silvano.dininno@nxp.com>
---
arch/arm/boot/compressed/head.S | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 45c8823..1cbba2a 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -775,6 +775,7 @@ __armv7_mmu_cache_on:
mrc p15, 0, r11, c0, c1, 4 @ read ID_MMFR0
tst r11, #0xf @ VMSA
movne r6, #CB_BITS | 0x02 @ !XN
+ orrne r6, r6, #4 << 12 @ TEX[2:0] = 100
blne __setup_mmu
mov r0, #0
mcr p15, 0, r0, c7, c10, 4 @ drain write buffer
--
2.7.4
^ permalink raw reply related
* [RFC] ARM: decompressor: Make RAM sections Outer non-cacheable
From: Silvano di Ninno @ 2018-01-03 16:32 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
There has been few attempts [1] [2] to make i.MX6 SoC run linux in Non-secure world,
with OP-TEE running in Secure world.
As you may know, the OP-TEE firmware (on Cortex A9) enables PL310 cache
controller at boot time and before the switch to Non-secure. The Way to allow
Linux to boot normally is currently to lock the ways so that L2 cache lines
are not filled.
It looks like Linux Kernel itself does not suffer from this (as it
does not have to perform cache management at boot time prior to l2 controller
initialization).
However, the decompressor code does and (at least for i.MX6 SoC families) this
is why Linux cannot boot correctly.
The patch [3] makes the RAM section outer non-cacheable so that the Linux code gets
correctly flush to DDR.
Is that solution acceptable?
Note on write_sec:
I am not a fan of the write_sec solution.
The idea behing Trustzone and Secure World is that the code being run in Non-secure
world (in this case linux) should not be (or is less) trusted.
In this context, it makes sense to rely as less as possible on Linux and so if
a solution can be found that does not require Linux intervention, I would favor that one.
Thanks,
Silvano
[1] http://archive.armlinux.org.uk/lurker/message/20171230.123403.ea9ef177.en.html
[2] http://archive.armlinux.org.uk/lurker/message/20171126.122530.ea0a4791.en.html
[3] [PATCH] ARM: decompressor: Make RAM sections Outer non-cacheable
Silvano di Ninno (1):
ARM: decompressor: Make RAM sections Outer non-cacheable
arch/arm/boot/compressed/head.S | 1 +
1 file changed, 1 insertion(+)
--
2.7.4
^ permalink raw reply
* [PATCH] ARM: dts: sun[47]i: Fix display backend 1 output to TCON0 remote endpoint
From: Chen-Yu Tsai @ 2018-01-03 16:31 UTC (permalink / raw)
To: linux-arm-kernel
There is a copy-paste error in the display pipeline device tree graph.
The remote endpoint of the display backend 1's output to TCON0 points
to the wrong endpoint. This will result in the driver incorrectly
parsing the relationship of the components.
Reported-by: Andrea Venturi <ennesimamail.av@gmail.com>
Fixes: 0df4cf33a594 ("ARM: dts: sun4i: Add device nodes for display
pipelines")
Fixes: 5b92b29bed45 ("ARM: dts: sun7i: Add device nodes for display
pipelines")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
This is a late fix for 4.15 that corrects an error in the newly
added display pipeline graph for the A10 and A20.
---
arch/arm/boot/dts/sun4i-a10.dtsi | 2 +-
arch/arm/boot/dts/sun7i-a20.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 5840f5c75c3b..4f2f2eea0755 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -1104,7 +1104,7 @@
be1_out_tcon0: endpoint at 0 {
reg = <0>;
- remote-endpoint = <&tcon1_in_be0>;
+ remote-endpoint = <&tcon0_in_be1>;
};
be1_out_tcon1: endpoint at 1 {
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 59655e42e4b0..bd0cd3204273 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -1354,7 +1354,7 @@
be1_out_tcon0: endpoint at 0 {
reg = <0>;
- remote-endpoint = <&tcon1_in_be0>;
+ remote-endpoint = <&tcon0_in_be1>;
};
be1_out_tcon1: endpoint at 1 {
--
2.15.1
^ permalink raw reply related
* Applied "ASoC: mediatek: update clock related properties of MT2701 AFE" to the asoc tree
From: Mark Brown @ 2018-01-03 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8693f44aac4304ce99836fc5e715a796347541d3.1514881870.git.ryder.lee@mediatek.com>
The patch
ASoC: mediatek: update clock related properties of MT2701 AFE
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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 0739fdfc0617a86781799d033e8fe758e8e48554 Mon Sep 17 00:00:00 2001
From: Ryder Lee <ryder.lee@mediatek.com>
Date: Tue, 2 Jan 2018 19:47:21 +0800
Subject: [PATCH] ASoC: mediatek: update clock related properties of MT2701 AFE
Add 'assigned-clocks*' properties which are used to initialize default
domain sources of audio system. we could configure different sets of
input clocks through DTS now. Hence driver no longer cares about that.
Also we change some 'clock-names' to make them more generic so that
other chips can reuse gracefully.
Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
.../devicetree/bindings/sound/mt2701-afe-pcm.txt | 207 +++++++++------------
1 file changed, 91 insertions(+), 116 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/mt2701-afe-pcm.txt b/Documentation/devicetree/bindings/sound/mt2701-afe-pcm.txt
index 77a57f84bed4..0450baad2813 100644
--- a/Documentation/devicetree/bindings/sound/mt2701-afe-pcm.txt
+++ b/Documentation/devicetree/bindings/sound/mt2701-afe-pcm.txt
@@ -6,51 +6,44 @@ Required properties:
- interrupts: should contain AFE and ASYS interrupts
- interrupt-names: should be "afe" and "asys"
- power-domains: should define the power domain
+- clocks: Must contain an entry for each entry in clock-names
+ See ../clocks/clock-bindings.txt for details
- clock-names: should have these clock names:
- "infra_sys_audio_clk",
"top_audio_mux1_sel",
"top_audio_mux2_sel",
- "top_audio_mux1_div",
- "top_audio_mux2_div",
- "top_audio_48k_timing",
- "top_audio_44k_timing",
- "top_audpll_mux_sel",
- "top_apll_sel",
- "top_aud1_pll_98M",
- "top_aud2_pll_90M",
- "top_hadds2_pll_98M",
- "top_hadds2_pll_294M",
- "top_audpll",
- "top_audpll_d4",
- "top_audpll_d8",
- "top_audpll_d16",
- "top_audpll_d24",
- "top_audintbus_sel",
- "clk_26m",
- "top_syspll1_d4",
- "top_aud_k1_src_sel",
- "top_aud_k2_src_sel",
- "top_aud_k3_src_sel",
- "top_aud_k4_src_sel",
- "top_aud_k5_src_sel",
- "top_aud_k6_src_sel",
- "top_aud_k1_src_div",
- "top_aud_k2_src_div",
- "top_aud_k3_src_div",
- "top_aud_k4_src_div",
- "top_aud_k5_src_div",
- "top_aud_k6_src_div",
- "top_aud_i2s1_mclk",
- "top_aud_i2s2_mclk",
- "top_aud_i2s3_mclk",
- "top_aud_i2s4_mclk",
- "top_aud_i2s5_mclk",
- "top_aud_i2s6_mclk",
- "top_asm_m_sel",
- "top_asm_h_sel",
- "top_univpll2_d4",
- "top_univpll2_d2",
- "top_syspll_d5";
+ "i2s0_src_sel",
+ "i2s1_src_sel",
+ "i2s2_src_sel",
+ "i2s3_src_sel",
+ "i2s0_src_div",
+ "i2s1_src_div",
+ "i2s2_src_div",
+ "i2s3_src_div",
+ "i2s0_mclk_en",
+ "i2s1_mclk_en",
+ "i2s2_mclk_en",
+ "i2s3_mclk_en",
+ "i2so0_hop_ck",
+ "i2so1_hop_ck",
+ "i2so2_hop_ck",
+ "i2so3_hop_ck",
+ "i2si0_hop_ck",
+ "i2si1_hop_ck",
+ "i2si2_hop_ck",
+ "i2si3_hop_ck",
+ "asrc0_out_ck",
+ "asrc1_out_ck",
+ "asrc2_out_ck",
+ "asrc3_out_ck",
+ "audio_afe_pd",
+ "audio_afe_conn_pd",
+ "audio_a1sys_pd",
+ "audio_a2sys_pd",
+ "audio_mrgif_pd";
+- assigned-clocks: list of input clocks and dividers for the audio system.
+ See ../clocks/clock-bindings.txt for details.
+- assigned-clocks-parents: parent of input clocks of assigned clocks.
+- assigned-clock-rates: list of clock frequencies of assigned clocks.
Example:
@@ -62,93 +55,75 @@ Example:
<GIC_SPI 132 IRQ_TYPE_LEVEL_LOW>;
interrupt-names = "afe", "asys";
power-domains = <&scpsys MT2701_POWER_DOMAIN_IFR_MSC>;
- clocks = <&infracfg CLK_INFRA_AUDIO>,
- <&topckgen CLK_TOP_AUD_MUX1_SEL>,
+ clocks = <&topckgen CLK_TOP_AUD_MUX1_SEL>,
<&topckgen CLK_TOP_AUD_MUX2_SEL>,
- <&topckgen CLK_TOP_AUD_MUX1_DIV>,
- <&topckgen CLK_TOP_AUD_MUX2_DIV>,
- <&topckgen CLK_TOP_AUD_48K_TIMING>,
- <&topckgen CLK_TOP_AUD_44K_TIMING>,
- <&topckgen CLK_TOP_AUDPLL_MUX_SEL>,
- <&topckgen CLK_TOP_APLL_SEL>,
- <&topckgen CLK_TOP_AUD1PLL_98M>,
- <&topckgen CLK_TOP_AUD2PLL_90M>,
- <&topckgen CLK_TOP_HADDS2PLL_98M>,
- <&topckgen CLK_TOP_HADDS2PLL_294M>,
- <&topckgen CLK_TOP_AUDPLL>,
- <&topckgen CLK_TOP_AUDPLL_D4>,
- <&topckgen CLK_TOP_AUDPLL_D8>,
- <&topckgen CLK_TOP_AUDPLL_D16>,
- <&topckgen CLK_TOP_AUDPLL_D24>,
- <&topckgen CLK_TOP_AUDINTBUS_SEL>,
- <&clk26m>,
- <&topckgen CLK_TOP_SYSPLL1_D4>,
<&topckgen CLK_TOP_AUD_K1_SRC_SEL>,
<&topckgen CLK_TOP_AUD_K2_SRC_SEL>,
<&topckgen CLK_TOP_AUD_K3_SRC_SEL>,
<&topckgen CLK_TOP_AUD_K4_SRC_SEL>,
- <&topckgen CLK_TOP_AUD_K5_SRC_SEL>,
- <&topckgen CLK_TOP_AUD_K6_SRC_SEL>,
<&topckgen CLK_TOP_AUD_K1_SRC_DIV>,
<&topckgen CLK_TOP_AUD_K2_SRC_DIV>,
<&topckgen CLK_TOP_AUD_K3_SRC_DIV>,
<&topckgen CLK_TOP_AUD_K4_SRC_DIV>,
- <&topckgen CLK_TOP_AUD_K5_SRC_DIV>,
- <&topckgen CLK_TOP_AUD_K6_SRC_DIV>,
<&topckgen CLK_TOP_AUD_I2S1_MCLK>,
<&topckgen CLK_TOP_AUD_I2S2_MCLK>,
<&topckgen CLK_TOP_AUD_I2S3_MCLK>,
<&topckgen CLK_TOP_AUD_I2S4_MCLK>,
- <&topckgen CLK_TOP_AUD_I2S5_MCLK>,
- <&topckgen CLK_TOP_AUD_I2S6_MCLK>,
- <&topckgen CLK_TOP_ASM_M_SEL>,
- <&topckgen CLK_TOP_ASM_H_SEL>,
- <&topckgen CLK_TOP_UNIVPLL2_D4>,
- <&topckgen CLK_TOP_UNIVPLL2_D2>,
- <&topckgen CLK_TOP_SYSPLL_D5>;
+ <&audiosys CLK_AUD_I2SO1>,
+ <&audiosys CLK_AUD_I2SO2>,
+ <&audiosys CLK_AUD_I2SO3>,
+ <&audiosys CLK_AUD_I2SO4>,
+ <&audiosys CLK_AUD_I2SIN1>,
+ <&audiosys CLK_AUD_I2SIN2>,
+ <&audiosys CLK_AUD_I2SIN3>,
+ <&audiosys CLK_AUD_I2SIN4>,
+ <&audiosys CLK_AUD_ASRCO1>,
+ <&audiosys CLK_AUD_ASRCO2>,
+ <&audiosys CLK_AUD_ASRCO3>,
+ <&audiosys CLK_AUD_ASRCO4>,
+ <&audiosys CLK_AUD_AFE>,
+ <&audiosys CLK_AUD_AFE_CONN>,
+ <&audiosys CLK_AUD_A1SYS>,
+ <&audiosys CLK_AUD_A2SYS>,
+ <&audiosys CLK_AUD_AFE_MRGIF>;
- clock-names = "infra_sys_audio_clk",
- "top_audio_mux1_sel",
+ clock-names = "top_audio_mux1_sel",
"top_audio_mux2_sel",
- "top_audio_mux1_div",
- "top_audio_mux2_div",
- "top_audio_48k_timing",
- "top_audio_44k_timing",
- "top_audpll_mux_sel",
- "top_apll_sel",
- "top_aud1_pll_98M",
- "top_aud2_pll_90M",
- "top_hadds2_pll_98M",
- "top_hadds2_pll_294M",
- "top_audpll",
- "top_audpll_d4",
- "top_audpll_d8",
- "top_audpll_d16",
- "top_audpll_d24",
- "top_audintbus_sel",
- "clk_26m",
- "top_syspll1_d4",
- "top_aud_k1_src_sel",
- "top_aud_k2_src_sel",
- "top_aud_k3_src_sel",
- "top_aud_k4_src_sel",
- "top_aud_k5_src_sel",
- "top_aud_k6_src_sel",
- "top_aud_k1_src_div",
- "top_aud_k2_src_div",
- "top_aud_k3_src_div",
- "top_aud_k4_src_div",
- "top_aud_k5_src_div",
- "top_aud_k6_src_div",
- "top_aud_i2s1_mclk",
- "top_aud_i2s2_mclk",
- "top_aud_i2s3_mclk",
- "top_aud_i2s4_mclk",
- "top_aud_i2s5_mclk",
- "top_aud_i2s6_mclk",
- "top_asm_m_sel",
- "top_asm_h_sel",
- "top_univpll2_d4",
- "top_univpll2_d2",
- "top_syspll_d5";
+ "i2s0_src_sel",
+ "i2s1_src_sel",
+ "i2s2_src_sel",
+ "i2s3_src_sel",
+ "i2s0_src_div",
+ "i2s1_src_div",
+ "i2s2_src_div",
+ "i2s3_src_div",
+ "i2s0_mclk_en",
+ "i2s1_mclk_en",
+ "i2s2_mclk_en",
+ "i2s3_mclk_en",
+ "i2so0_hop_ck",
+ "i2so1_hop_ck",
+ "i2so2_hop_ck",
+ "i2so3_hop_ck",
+ "i2si0_hop_ck",
+ "i2si1_hop_ck",
+ "i2si2_hop_ck",
+ "i2si3_hop_ck",
+ "asrc0_out_ck",
+ "asrc1_out_ck",
+ "asrc2_out_ck",
+ "asrc3_out_ck",
+ "audio_afe_pd",
+ "audio_afe_conn_pd",
+ "audio_a1sys_pd",
+ "audio_a2sys_pd",
+ "audio_mrgif_pd";
+
+ assigned-clocks = <&topckgen CLK_TOP_AUD_MUX1_SEL>,
+ <&topckgen CLK_TOP_AUD_MUX2_SEL>,
+ <&topckgen CLK_TOP_AUD_MUX1_DIV>,
+ <&topckgen CLK_TOP_AUD_MUX2_DIV>;
+ assigned-clock-parents = <&topckgen CLK_TOP_AUD1PLL_98M>,
+ <&topckgen CLK_TOP_AUD2PLL_90M>;
+ assigned-clock-rates = <0>, <0>, <49152000>, <45158400>;
};
--
2.15.1
^ permalink raw reply related
* [RESEND PATCH v2 15/15] arm64: dts: msm8996: db820c: Add sound card support
From: Srinivas Kandagatla @ 2018-01-03 16:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180103002229.GV478@tuxbook>
Thanks for the comments,
On 03/01/18 00:22, Bjorn Andersson wrote:
> On Thu 14 Dec 09:34 PST 2017, srinivas.kandagatla at linaro.org wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch adds hdmi sound card support to db820c via qdsp.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>> arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 5 +++++
>> arch/arm64/boot/dts/qcom/msm8996.dtsi | 33 ++++++++++++++++++++++++++++
>> 2 files changed, 38 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
>> index 9769053957af..b955769b100d 100644
>> --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
>> @@ -190,6 +190,11 @@
>> };
>> };
>>
>> + snd {
>> + compatible = "qcom,apq8096-sndcard";
>> + qcom,model = "DB820c";
>> + iommus = <&lpass_q6_smmu 1>;
>> + };
>>
>> gpio_keys {
>> compatible = "gpio-keys";
>> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> index a144cec7bb71..25c43fb8ab49 100644
>> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> @@ -1262,6 +1262,7 @@
>>
>> phys = <&hdmi_phy>;
>> phy-names = "hdmi_phy";
>> + #sound-dai-cells = <0>;
>>
>> ports {
>> #address-cells = <1>;
>> @@ -1297,6 +1298,33 @@
>> "ref_clk";
>> };
>> };
>> +
>> + lpass_q6_smmu: arm,smmu-lpass_q6 at 1600000 {
>
> name this node "iommu"
will rename it to arm,smmu at 1600000
>
>> + compatible = "qcom,msm8996-smmu-v2";
>> + reg = <0x1600000 0x20000>;
>> + #iommu-cells = <1>;
>> + power-domains = <&gcc HLOS1_VOTE_LPASS_CORE_GDSC>;
>
> Indentation
sure.
>
>> +
>> + #global-interrupts = <1>;
>> + interrupts = <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 393 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 394 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 395 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 396 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 397 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 398 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 399 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 401 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 402 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 403 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> + clocks = <&gcc GCC_HLOS1_VOTE_LPASS_CORE_SMMU_CLK>,
>> + <&gcc GCC_HLOS1_VOTE_LPASS_ADSP_SMMU_CLK>;
>> + clock-names = "iface", "bus";
>> + status = "okay";
>> + };
>> };
>>
>> adsp-pil {
>> @@ -1325,6 +1353,11 @@
>> qcom,ipc = <&apcs 16 8>;
>> qcom,smd-edge = <1>;
>> qcom,remote-pid = <2>;
>> +
>> + apr {
>
> "apr-audio-svc", as this is not the only apr channel on this edge.
yep.
>
>> + compatible = "qcom,apr-msm8996";
>> + qcom,smd-channels = "apr_audio_svc";
>> + };
>> };
>
> Regards,
> Bjorn
>
^ permalink raw reply
* [RESEND PATCH v2 14/15] ASoC: qcom: apq8096: Add db820c machine driver
From: Srinivas Kandagatla @ 2018-01-03 16:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180103001654.GU478@tuxbook>
Thanks for the review comments.
On 03/01/18 00:16, Bjorn Andersson wrote:
> On Thu 14 Dec 09:34 PST 2017, srinivas.kandagatla at linaro.org wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> uThis patch adds support to DB820c machine driver.
>
> Drop 'u' and expand the message to claim that this is the machine driver
> for 8996, used by the db820c.
>
> [..]
>> +static struct snd_soc_dai_link msm8996_dai_links[] = {
>
> Are there any differences between the DAI links of apq8096 and msm8996?
>
This driver is more of board specific,
Am not 100% sure about msm8996, on apq8096 in particularly on db820c we
got hdmi and analog audio connected via slimbus and also I2S on lowspeed
connector.
>> + /* FrontEnd DAI Links */
>> + {
>> + .name = "MultiMedia1 Playback",
>> + .stream_name = "MultiMedia1",
>> + .cpu_dai_name = "MM_DL1",
>> + .platform_name = "q6asm_dai",
>> + .dynamic = 1,
>> + .dpcm_playback = 1,
>> +
>> + .codec_dai_name = "snd-soc-dummy-dai",
>> + .codec_name = "snd-soc-dummy",
>> + },
>> + /* Backend DAI Links */
>> + {
>> + .name = "HDMI Playback",
>> + .stream_name = "q6afe_dai",
>> + .cpu_dai_name = "HDMI",
>> + .platform_name = "q6routing",
>> + .no_pcm = 1,
>> + .dpcm_playback = 1,
>> + .be_hw_params_fixup = msm8996_be_hw_params_fixup,
>> + .codec_dai_name = "i2s-hifi",
>> + .codec_name = "hdmi-audio-codec.0.auto",
>> + },
>> +};
>> +
>> +static int apq8096_sbc_parse_of(struct snd_soc_card *card)
>
> msm8996_parse_of()
sure if it helps.
>
>> +{
>> + struct device *dev = card->dev;
>> + int ret;
>> +
>> + ret = snd_soc_of_parse_card_name(card, "qcom,model");
>> + if (ret)
>> + dev_err(dev, "Error parsing card name: %d\n", ret);
>> +
>> + return ret;
>> +}
>> +
>> +static int msm_snd_apq8096_probe(struct platform_device *pdev)
>
> msm_snd_msm8996_probe()?
sure
>
>> +{
>> + int ret;
>> + struct snd_soc_card *card;
>> +
>> + card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL);
>> + if (!card)
>> + return -ENOMEM;
>> +
>> + card->dev = &pdev->dev;
>> +
>> + ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(32));
>> + if (ret)
>> + return ret;
>> +
>> + card->dai_link = msm8996_dai_links;
>> + card->num_links = ARRAY_SIZE(msm8996_dai_links);
>> +
>> + ret = apq8096_sbc_parse_of(card);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Error parsing OF data\n");
>
> No need to print in both parse_of() and here.
>
yep.
>> + return ret;
>> + }
>> +
>> + ret = devm_snd_soc_register_card(&pdev->dev, card);
>> + if (ret)
>> + dev_err(&pdev->dev, "sound card register failed (%d)!\n", ret);
>> + else
>> + dev_err(&pdev->dev, "sound card register Sucessfull\n");
>
> This isn't an error, skip the print here.
>
yep.
>> +
>> + return ret;
>> +}
>> +
>> +static const struct of_device_id msm_snd_apq8096_dt_match[] = {
>> + {.compatible = "qcom,apq8096-sndcard"},
>> + {}
>> +};
>> +
>> +static struct platform_driver msm_snd_apq8096_driver = {
>> + .probe = msm_snd_apq8096_probe,
>> + .driver = {
>> + .name = "msm-snd-apq8096",
>> + .owner = THIS_MODULE,
>
> Drop the .owner
>
yep
> Regards,
> Bjorn
>
^ permalink raw reply
* [RESEND PATCH v2 13/15] dt-bindings: sound: qcom: Add devicetree bindings for apq8096
From: Srinivas Kandagatla @ 2018-01-03 16:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180103002835.GW478@tuxbook>
On 03/01/18 00:28, Bjorn Andersson wrote:
> On Thu 14 Dec 09:34 PST 2017, srinivas.kandagatla at linaro.org wrote:
>
>> +++ b/Documentation/devicetree/bindings/sound/qcom,apq8096.txt
>
> Wouldn't it be possible to describe all(?) qdsp based machines in this
> one document? I.e. should we name it a little bit more generic?
You mean like downstream ?
I see no harm in trying it out and see how it looks like.
>
>> @@ -0,0 +1,22 @@
>> +* Qualcomm Technologies APQ8096 ASoC sound card driver
>> +
>> +This binding describes the APQ8096 sound card, which uses qdsp for audio.
>> +
>> +- compatible:
>> + Usage: required
>> + Value type: <stringlist>
>> + Definition: must be "qcom,apq8096-sndcard"
>> +
>> +- qcom,audio-routing:
>> + Usage: Optional
>> + Value type: <stringlist>
>> + Definition: A list of the connections between audio components.
>
> Double space before A
yep.
>
>> + Each entry is a pair of strings, the first being the
>> + connection's sink, the second being the connection's
>> + source. Valid names could be power supplies, MicBias
>> + of codec and the jacks on the board:
>> +Example:
>> + sound {
>> + compatible = "qcom,snd-apq8096";
>
> Indentation
yep.
>
>> + qcom,model = "DB820c";
>> + };
>
> Regards,
> Bjorn
>
^ permalink raw reply
* [RESEND PATCH v2 12/15] ASoC: qcom: qdsp6: Add support to q6asm dai driver
From: Srinivas Kandagatla @ 2018-01-03 16:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180103000306.GT478@tuxbook>
Thanks for the comments.
On 03/01/18 00:03, Bjorn Andersson wrote:
> On Thu 14 Dec 09:33 PST 2017, srinivas.kandagatla at linaro.org wrote:
>
> [..]
>> +
>> +enum stream_state {
>> + IDLE = 0,
>> + STOPPED,
>> + RUNNING,
>
> These are too generic.
>
Yep, I will prefix them with Q6ASM.
>> +};
>> +
>> +struct q6asm_dai_rtd {
>> + struct snd_pcm_substream *substream;
>> + dma_addr_t phys;
>> + unsigned int pcm_size;
>> + unsigned int pcm_count;
>> + unsigned int pcm_irq_pos; /* IRQ position */
>> + unsigned int periods;
>> + uint16_t bits_per_sample;
>> + uint16_t source; /* Encoding source bit mask */
>> +
>> + struct audio_client *audio_client;
>> + uint16_t session_id;
>> +
>> + enum stream_state state;
>> + bool set_channel_map;
>> + char channel_map[8];
>
> There's a define for this 8
Yes, this is max channels.
>
>> +};
>> +
>> +struct q6asm_dai_data {
>> + u64 sid;
>> +};
>> +
>> +static struct snd_pcm_hardware q6asm_dai_hardware_playback = {
>> + .info = (SNDRV_PCM_INFO_MMAP |
>> + SNDRV_PCM_INFO_BLOCK_TRANSFER |
>> + SNDRV_PCM_INFO_MMAP_VALID |
>> + SNDRV_PCM_INFO_INTERLEAVED |
>> + SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME),
>> + .formats = (SNDRV_PCM_FMTBIT_S16_LE |
>> + SNDRV_PCM_FMTBIT_S24_LE),
>> + .rates = SNDRV_PCM_RATE_8000_192000,
>> + .rate_min = 8000,
>> + .rate_max = 192000,
>> + .channels_min = 1,
>> + .channels_max = 8,
>> + .buffer_bytes_max = (PLAYBACK_MAX_NUM_PERIODS *
>> + PLAYBACK_MAX_PERIOD_SIZE),
>> + .period_bytes_min = PLAYBACK_MIN_PERIOD_SIZE,
>> + .period_bytes_max = PLAYBACK_MAX_PERIOD_SIZE,
>> + .periods_min = PLAYBACK_MIN_NUM_PERIODS,
>> + .periods_max = PLAYBACK_MAX_NUM_PERIODS,
>
> If you just put the numbers here, instead of using the PLAYBACK_
> defines, it's possible to grok the values of this struct without having
> to jump to the defines for each one.
This is usually done this way in may other drivers!,
>
>> + .fifo_size = 0,
>> +};
>> +
>> +/* Conventional and unconventional sample rate supported */
>> +static unsigned int supported_sample_rates[] = {
>> + 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000,
>> + 88200, 96000, 176400, 192000
>> +};
>> +
>> +static struct snd_pcm_hw_constraint_list constraints_sample_rates = {
>
It is used in q6asm_dai_open().
>
>> + .count = ARRAY_SIZE(supported_sample_rates),
>> + .list = supported_sample_rates,
>> + .mask = 0,
>> +};
>> +
>
>> +
>> +static int q6asm_dai_prepare(struct snd_pcm_substream *substream)
>> +{
>> + struct snd_pcm_runtime *runtime = substream->runtime;
>> + struct snd_soc_pcm_runtime *soc_prtd = substream->private_data;
>> + struct q6asm_dai_rtd *prtd = runtime->private_data;
>> + struct q6asm_dai_data *pdata;
>> + int ret;
>> +
>> + pdata = dev_get_drvdata(soc_prtd->platform->dev);
>> + if (!pdata)
>> + return -EINVAL;
>> +
>> + if (!prtd || !prtd->audio_client) {
>> + pr_err("%s: private data null or audio client freed\n",
>> + __func__);
>> + return -EINVAL;
>> + }
>> +
>> + prtd->pcm_count = snd_pcm_lib_period_bytes(substream);
>> + prtd->pcm_irq_pos = 0;
>> + /* rate and channels are sent to audio driver */
>> + if (prtd->state) {
>> + /* clear the previous setup if any */
>> + q6asm_cmd(prtd->audio_client, CMD_CLOSE);
>> + q6asm_unmap_memory_regions(substream->stream,
>> + prtd->audio_client);
>> + q6routing_dereg_phy_stream(soc_prtd->dai_link->id,
>> + SNDRV_PCM_STREAM_PLAYBACK);
>> + }
>> +
>> + ret = q6asm_map_memory_regions(substream->stream, prtd->audio_client,
>> + prtd->phys,
>> + (prtd->pcm_size / prtd->periods),
>> + prtd->periods);
>> +
>> + if (ret < 0) {
>> + pr_err("Audio Start: Buffer Allocation failed rc = %d\n",
>> + ret);
>> + return -ENOMEM;
>> + }
>> +
>> + ret = q6asm_open_write(prtd->audio_client, FORMAT_LINEAR_PCM,
>> + prtd->bits_per_sample);
>> + if (ret < 0) {
>> + pr_err("%s: q6asm_open_write failed\n", __func__);
>> + q6asm_audio_client_free(prtd->audio_client);
>> + prtd->audio_client = NULL;
>
> Do you need to roll back the q6asm_map_memory_regions?
>
yes you are correct, we should roll back the map.
>> + return -ENOMEM;
>> + }
>> +
>> + prtd->session_id = q6asm_get_session_id(prtd->audio_client);
>> + ret = q6routing_reg_phy_stream(soc_prtd->dai_link->id, LEGACY_PCM_MODE,
>> + prtd->session_id, substream->stream);
>> + if (ret) {
>> + pr_err("%s: stream reg failed ret:%d\n", __func__, ret);
>> + return ret;
>> + }
>> +
>> + ret = q6asm_media_format_block_multi_ch_pcm(
>> + prtd->audio_client, runtime->rate,
>> + runtime->channels, !prtd->set_channel_map,
>> + prtd->channel_map, prtd->bits_per_sample);
>
> set_channel_map and channel_map aren't referenced elsewhere. If this
> isn't used consider removing it for now.
>
Will take a closer look before sending next version.
>> + if (ret < 0)
>> + pr_info("%s: CMD Format block failed\n", __func__);
>> +
>> + prtd->state = RUNNING;
>> +
>> + return 0;
>> +}
>> +
> [..]
>> +static int q6asm_dai_pcm_new(struct snd_soc_pcm_runtime *rtd)
>> +{
>> + struct snd_pcm *pcm = rtd->pcm;
>> + struct snd_pcm_substream *substream;
>> + struct snd_card *card = rtd->card->snd_card;
>> + struct device *dev = card->dev;
>> + struct device_node *node = dev->of_node;
>> + struct q6asm_dai_data *pdata = dev_get_drvdata(rtd->platform->dev);
>> + struct of_phandle_args args;
>> +
>> + int size, ret = 0;
>> +
>> + ret = of_parse_phandle_with_fixed_args(node, "iommus", 1, 0, &args);
>> + if (ret < 0)
>> + pdata->sid = -1;
>> + else
>> + pdata->sid = args.args[0];
>> +
>
> Is this really how you're supposed to deal with the iommu?
>
Any suggestions are welcome, I did not find a better way to append sid
to iova address from iommu.
Currently downstream abstracts this in ion apis.
>> +
>> +
>> + substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
>> + size = q6asm_dai_hardware_playback.buffer_bytes_max;
>> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, dev, size,
>> + &substream->dma_buffer);
>> + if (ret) {
>> + dev_err(dev, "Cannot allocate buffer(s)\n");
>> + return ret;
>
> Just fall through.
>
yep
>> + }
>> +
>> + return ret;
>> +}
>> +
> [..]
>> +static struct snd_soc_dai_driver q6asm_fe_dais[] = {
>> + {
>> + .playback = {
>> + .stream_name = "MultiMedia1 Playback",
>> + .rates = (SNDRV_PCM_RATE_8000_192000|
>> + SNDRV_PCM_RATE_KNOT),
>> + .formats = (SNDRV_PCM_FMTBIT_S16_LE |
>> + SNDRV_PCM_FMTBIT_S24_LE),
>> + .channels_min = 1,
>> + .channels_max = 8,
>> + .rate_min = 8000,
>> + .rate_max = 192000,
>> + },
>> + .name = "MM_DL1",
>> + .probe = fe_dai_probe,
>> + .id = MSM_FRONTEND_DAI_MULTIMEDIA1,
>> + },
>> + {
>> + .playback = {
>> + .stream_name = "MultiMedia2 Playback",
>> + .rates = (SNDRV_PCM_RATE_8000_192000|
>> + SNDRV_PCM_RATE_KNOT),
>> + .formats = (SNDRV_PCM_FMTBIT_S16_LE |
>> + SNDRV_PCM_FMTBIT_S24_LE),
>> + .channels_min = 1,
>> + .channels_max = 8,
>> + .rate_min = 8000,
>> + .rate_max = 192000,
>
> I presume the listed frontend DAIs needs to match the firmware of the
> DSP (and features of hardware)? Can we get away with a single list for
> all versions of the adsp?
>
Yes, DSP supports 8 concurrent streams both playback and record streams.
For now I have only added two entires to keep the patch simple but this
should be ideally 8 entries.
> In msm-4.4 the max rate for these where changed to 384000, see:
>
> 9c46f74b2724 ("ASoC: msm: add 384KHz playback support")
sure i will include that in next version.
>
>> + },
>> + .name = "MM_DL2",
>> + .probe = fe_dai_probe,
>> + .id = MSM_FRONTEND_DAI_MULTIMEDIA2,
>> + },
>> +};
>> +
>
> Regards,
> Bjorn
>
^ permalink raw reply
* [RESEND PATCH v2 11/15] ASoC: qcom: qdsp6: Add support to q6afe dai driver
From: Srinivas Kandagatla @ 2018-01-03 16:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102232811.GS478@tuxbook>
On 02/01/18 23:28, Bjorn Andersson wrote:
> On Thu 14 Dec 09:33 PST 2017, srinivas.kandagatla at linaro.org wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch adds support to q6afe backend dais driver.
>>
>
> Isn't the list of backend DAIs platform-dependent?
dai links and connections between backend and front ends are platform
dependent.
>
> [..]
>> +static const struct snd_soc_dapm_widget hdmi_dapm_widgets[] = {
>> + SND_SOC_DAPM_AIF_OUT("HDMI", "HDMI Playback", 0, 0, 0, 0),
>> + SND_SOC_DAPM_OUTPUT("HDMI-RX"),
>> +};
>> +
>> +static const struct snd_soc_component_driver msm_dai_hdmi_q6_component = {
>
> How will this look beyond HDMI? I'm having issues mapping this to
> downstream.
ex:
For slimbus dais, we would have more entries of "struct
snd_soc_dai_driver" in this file.
Basically these are the dais that are exposed by the dsp firmware.
Depending on the actually platform some of these dais would be setup
accordingly.
>
>> + .name = "msm-dai-q6-hdmi",
>> + .dapm_widgets = hdmi_dapm_widgets,
>> + .num_dapm_widgets = ARRAY_SIZE(hdmi_dapm_widgets),
>> + .controls = hdmi_config_controls,
>> + .num_controls = ARRAY_SIZE(hdmi_config_controls),
>> + .dapm_routes = hdmi_dapm_routes,
>> + .num_dapm_routes = ARRAY_SIZE(hdmi_dapm_routes),
>> +};
>> +
>
> Regards,
> Bjorn
>
^ permalink raw reply
* [RESEND PATCH v2 10/15] ASoC: qcom: qdsp6: Add support to q6routing driver
From: Srinivas Kandagatla @ 2018-01-03 16:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102230007.GR478@tuxbook>
On 02/01/18 23:00, Bjorn Andersson wrote:
> On Thu 14 Dec 09:33 PST 2017, srinivas.kandagatla at linaro.org wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch adds support to q6 routing driver which configures route
>> between ASM and AFE module using ADM apis.
>>
>> This driver uses dapm widgets to setup the matrix between AFE ports and
>> ASM streams.
>>
>
> Why is this a separate driver from the q6adm?
This is actually exposing the mixer controls for routing streams on to
different audio sink/sources using a matrix.
>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>> sound/soc/qcom/Kconfig | 5 +
>> sound/soc/qcom/qdsp6/Makefile | 1 +
>> sound/soc/qcom/qdsp6/q6routing.c | 386 +++++++++++++++++++++++++++++++++++++++
>> sound/soc/qcom/qdsp6/q6routing.h | 9 +
>> 4 files changed, 401 insertions(+)
>> create mode 100644 sound/soc/qcom/qdsp6/q6routing.c
>> create mode 100644 sound/soc/qcom/qdsp6/q6routing.h
>> diff --git a/sound/soc/qcom/qdsp6/q6routing.c b/sound/soc/qcom/qdsp6/q6routing.c
>> +/**
>> + * q6routing_reg_phy_stream() - Register a new stream for route setup
>> + *
>> + * @fedai_id: Frontend dai id.
>> + * @perf_mode: Performace mode.
>
> "Performance"
yep.
>
>> + * @stream_id: ASM stream id to map.
>> + * @stream_type: Direction of stream
>> + *
>> + * Return: Will be an negative on error or a zero on success.
>> + */
>> +int q6routing_reg_phy_stream(int fedai_id, int perf_mode,
>
> q6routing_stream_open() ?
sure if it helps reading.
>
>> + int stream_id, int stream_type)
>> +{
>> + int j, topology, num_copps = 0;
>> + struct route_payload payload;
>> + int copp_idx;
>> + struct session_data *session;
>> +
>> + if (!routing_data) {
>> + pr_err("Routing driver not yet ready\n");
>> + return -EINVAL;
>> + }
>> +
>> + session = &routing_data->sessions[stream_id - 1];
>> + mutex_lock(&routing_data->lock);
>> + session->fedai_id = fedai_id;
>> + payload.num_copps = 0; /* only RX needs to use payload */
>> + topology = NULL_COPP_TOPOLOGY;
>> + copp_idx = q6adm_open(routing_data->dev, session->port_id,
>> + session->path_type, session->sample_rate,
>> + session->channels, topology, perf_mode,
>> + session->bits_per_sample, 0, 0);
>> + if ((copp_idx < 0) || (copp_idx >= MAX_COPPS_PER_PORT)) {
>
> Make q6adm_open() not return >= MAX_COPPS_PER_PORT.
>
> And drop the extra parenthesis.
>
>> + mutex_unlock(&routing_data->lock);
>> + return -EINVAL;
>> + }
>> +
>> + set_bit(copp_idx, &session->copp_map);
>> + for (j = 0; j < MAX_COPPS_PER_PORT; j++) {
>
> Use for_each_set_bit()
>
>> + unsigned long copp = session->copp_map;
>> +
>> + if (test_bit(j, &copp)) {
>> + payload.port_id[num_copps] = session->port_id;
>> + payload.copp_idx[num_copps] = j;
>> + num_copps++;
>> + }
>> + }
>> +
>> + if (num_copps) {
>> + payload.num_copps = num_copps;
>> + payload.session_id = stream_id;
>> + q6adm_matrix_map(routing_data->dev, session->path_type,
>> + payload, perf_mode);
>> + }
>> + mutex_unlock(&routing_data->lock);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(q6routing_reg_phy_stream);
>> +
>> +static struct session_data *routing_get_session(struct msm_routing_data *data,
>> + int port_id, int port_type)
>
> port_type is ignored
It will be used once we add capture support.
>
>> +{
>> + int i;
>> +
>> + for (i = 0; i < MAX_SESSIONS; i++)
>> + if (port_id == data->sessions[i].port_id)
>> + return &data->sessions[i];
>> +
>> + return NULL;
>> +}
>> +
>> +/**
>> + * q6routing_dereg_phy_stream() - Deregister a stream
>> + *
>> + * @fedai_id: Frontend dai id.
>> + * @stream_type: Direction of stream
>> + *
>> + * Return: Will be an negative on error or a zero on success.
>> + */
>> +void q6routing_dereg_phy_stream(int fedai_id, int stream_type)
>
> q6routing_stream_close()?
>
will rename it.
>> +{
>> + struct session_data *session;
>> + int idx;
>> +
>> + session = get_session_from_id(routing_data, fedai_id);
>> + if (!session)
>> + return;
>> +
>> + for_each_set_bit(idx, &session->copp_map, MAX_COPPS_PER_PORT)
>> + q6adm_close(routing_data->dev, session->port_id,
>> + session->perf_mode, idx);
>> +
>> + session->fedai_id = -1;
>> + session->copp_map = 0;
>> +}
>> +EXPORT_SYMBOL_GPL(q6routing_dereg_phy_stream);
>> +
>> +
>> +
>> +static const struct snd_soc_dapm_widget msm_qdsp6_widgets[] = {
>> + /* Frontend AIF */
>> + /* Widget name equals to Front-End DAI name<Need confirmation>,
>
> Perhaps this should be confirmed and the comment updated?
>
Some leftover from old code...
>> + * Stream name must contains substring of front-end dai name
>> + */
>> + SND_SOC_DAPM_AIF_IN("MM_DL1", "MultiMedia1 Playback", 0, 0, 0, 0),
>> + SND_SOC_DAPM_AIF_IN("MM_DL2", "MultiMedia2 Playback", 0, 0, 0, 0),
>> + SND_SOC_DAPM_AIF_IN("MM_DL3", "MultiMedia3 Playback", 0, 0, 0, 0),
>> + SND_SOC_DAPM_AIF_IN("MM_DL4", "MultiMedia4 Playback", 0, 0, 0, 0),
>> + SND_SOC_DAPM_AIF_IN("MM_DL5", "MultiMedia5 Playback", 0, 0, 0, 0),
>> + SND_SOC_DAPM_AIF_IN("MM_DL6", "MultiMedia6 Playback", 0, 0, 0, 0),
>> + SND_SOC_DAPM_AIF_IN("MM_DL7", "MultiMedia7 Playback", 0, 0, 0, 0),
>> + SND_SOC_DAPM_AIF_IN("MM_DL8", "MultiMedia8 Playback", 0, 0, 0, 0),
>> +
>> + /* Mixer definitions */
>> + SND_SOC_DAPM_MIXER("HDMI Mixer", SND_SOC_NOPM, 0, 0,
>> + hdmi_mixer_controls,
>> + ARRAY_SIZE(hdmi_mixer_controls)),
>> +};
>
>> +static int routing_hw_params(struct snd_pcm_substream *substream,
>> + struct snd_pcm_hw_params *params)
>> +{
>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> + unsigned int be_id = rtd->cpu_dai->id;
>> + struct snd_soc_platform *platform = rtd->platform;
>> + struct msm_routing_data *data = snd_soc_platform_get_drvdata(platform);
>> + struct session_data *session;
>> + int port_id, port_type, path_type, bits_per_sample;
>
> bits_per_sample is likely unused.
>
yep.
>> +
>> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> + path_type = ADM_PATH_PLAYBACK;
>> + port_type = MSM_AFE_PORT_TYPE_RX;
>> + }
>> +
>> + port_id = be_id;
>
> Why alias this variable?
will fix this in next version.
>
>> +
>> + session = routing_get_session(data, port_id, port_type);
>> +
>> + if (!session) {
>> + pr_err("No session matrix setup yet..\n");
>> + return -EINVAL;
>> + }
>> +
>> + mutex_lock(&data->lock);
>> +
>> + session->path_type = path_type;
>> + session->sample_rate = params_rate(params);
>> + session->channels = params_channels(params);
>> + session->format = params_format(params);
>> +
>> + if (session->format == SNDRV_PCM_FORMAT_S16_LE)
>> + session->bits_per_sample = 16;
>> + else if (session->format == SNDRV_PCM_FORMAT_S24_LE)
>> + bits_per_sample = 24;
>
> session-> ?
I agree, will fix it.
>
> Perhaps switch on params_format(params) instead? And fail in the default
> case.
will give that a go.
>
>> +
>> + mutex_unlock(&data->lock);
>> + return 0;
>> +}
>> +
>> +static struct snd_pcm_ops q6pcm_routing_ops = {
>> + .hw_params = routing_hw_params,
>> + .close = routing_close,
>> + .prepare = routing_prepare,
>> +};
>> +
>> +/* Not used but frame seems to require it */
>
> Remove comment?
>
looks like leftover.. will remove it.
[...]
>> +
>> +static int q6pcm_routing_remove(struct platform_device *pdev)
>> +{
>
> As you return here routing_data will be freed. The early check in
> q6routing_reg_phy_stream() seems to indicate that this driver can be
> called even though the routing device isn't available.
>
> So you probably want to clear that variable, at least.
sure, will fix this in next version.
>
>> + return 0;
>> +}
>> +
>> +static struct platform_driver q6pcm_routing_driver = {
>> + .driver = {
>> + .name = "q6routing",
>> + .owner = THIS_MODULE,
>
> Drop .owner
>
yep.
>> + },
>> + .probe = q6pcm_routing_probe,
>> + .remove = q6pcm_routing_remove,
>> +};
>>
^ permalink raw reply
* [RESEND PATCH v2 09/15] ASoC: qcom: qdsp6: Add support to Q6CORE
From: Srinivas Kandagatla @ 2018-01-03 16:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102221520.GQ478@tuxbook>
Thanks for the review.
On 02/01/18 22:15, Bjorn Andersson wrote:
> On Thu 14 Dec 09:33 PST 2017, srinivas.kandagatla at linaro.org wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch adds support to core apr service, which is used to query
>> status of other static and dynamic services on the dsp.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>> sound/soc/qcom/Kconfig | 5 +
>> sound/soc/qcom/qdsp6/Makefile | 1 +
>> sound/soc/qcom/qdsp6/q6core.c | 227 ++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 233 insertions(+)
>> create mode 100644 sound/soc/qcom/qdsp6/q6core.c
>>
>> +obj-$(CONFIG_SND_SOC_QDSP6_CORE) += q6core.o
>> diff --git a/sound/soc/qcom/qdsp6/q6core.c b/sound/soc/qcom/qdsp6/q6core.c
>> new file mode 100644
>> index 000000000000..d4e2dbc62489
>> +struct q6core {
>> + struct apr_device *adev;
>> + wait_queue_head_t wait;
>> + uint32_t avcs_state;
>> + int resp_received;
>
> bool
yep.
>
>> + uint32_t num_services;
>> + struct avcs_svc_info *svcs_info;
>> +};
>> +static int core_callback(struct apr_device *adev, struct apr_client_data *data)
>> +{
>> + struct q6core *core = dev_get_drvdata(&adev->dev);
>> + uint32_t *payload;
>> +
>> + switch (data->opcode) {
>> + case AVCS_GET_VERSIONS_RSP:
>> + payload = data->payload;
>> + core->num_services = payload[1];
>
> Describe the payload in a struct (with flexible array member for the
> svcs_info list).
sure!
>
>> +
>> + if (!core->svcs_info)
>> + core->svcs_info = kcalloc(core->num_services,
>> + sizeof(*core->svcs_info),
>> + GFP_ATOMIC);
>> + if (!core->svcs_info)
>> + return -ENOMEM;
>> +
>
> If we receive this twice with different num_services for some reason the
> memcpy might trash the heap.
>
> But as this is the get_version response and we're only going to issue
> that once you should remove the check for !core->svcs_info above.
>
yes, I agree
> And don't forget to free svcs_info once you have added your services.
yep.
>
>> + /* svc info is after 8 bytes */
>> + memcpy(core->svcs_info, payload + 2,
>> + core->num_services * sizeof(*core->svcs_info));
>> +
>> + core->resp_received = 1;
>> + wake_up(&core->wait);
>> +
>> + break;
>> + case AVCS_CMDRSP_ADSP_EVENT_GET_STATE:
>> + payload = data->payload;
>> + core->avcs_state = payload[0];
>> +
>> + core->resp_received = 1;
>> + wake_up(&core->wait);
>> + break;
>> + default:
>> + dev_err(&adev->dev, "Message id from adsp core svc: 0x%x\n",
>> + data->opcode);
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +void q6core_add_service(struct device *dev, uint32_t svc_id, uint32_t version)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(static_services); i++) {
>> + if (static_services[i].svc_id == svc_id) {
>> + static_services[i].svc_version = version;
>> + apr_add_device(dev->parent, &static_services[i]);
>> + dev_info(dev,
>
> Please don't spam the logs, dev_dbg() should be enough. And as
> apr_add_device() returns you can find the devices registered in /sys
sure!
>
>> + "Adding SVC: %s: id 0x%x API Ver 0x%x:0x%x\n",
>> + static_services[i].name, svc_id,
>> + APR_SVC_MAJOR_VERSION(version),
>> + APR_SVC_MINOR_VERSION(version));
>> + }
>> + }
>> +}
>> +
>> +static void q6core_add_static_services(struct q6core *core)
>
> The name of this function is deceiving, it doesn't really add the static
> services. It adds devices for the services that we've been informed
> exists, by the other side - using the static list of services.
>
> Per the comment on a previous patch I don't think the "name" in
> apr_device_id provides any real value and in this case if forces you to
> perform a lookup using this table.
>
> If you drop the name, you can loop over the list of service ids returned
> from the remote and just register them with a hard coded domain id
> (based on apr instance?) and client_id. You don't need the lookup table.
>
yes, correct.
>> +{
>> + int i;
>> + struct apr_device *adev = core->adev;
>> + struct avcs_svc_info *svcs_info = core->svcs_info;
>> +
>> + for (i = 0; i < core->num_services; i++)
>> + q6core_add_service(&adev->dev, svcs_info[i].service_id,
>> + svcs_info[i].version);
>> +}
>> +
>> +static int q6core_get_svc_versions(struct q6core *core)
>> +{
>> + struct apr_device *adev = core->adev;
>> + struct apr_hdr hdr = {0};
>> + int rc;
>> +
>> + hdr.hdr_field = APR_HDR_FIELD(APR_MSG_TYPE_SEQ_CMD,
>> + APR_HDR_LEN(APR_HDR_SIZE), APR_PKT_VER);
>> + hdr.pkt_size = APR_PKT_SIZE(APR_HDR_SIZE, 0);
>> + hdr.opcode = AVCS_GET_VERSIONS;
>> +
>> + rc = apr_send_pkt(adev, (uint32_t *)&hdr);
>> + if (rc < 0)
>> + return rc;
>> +
>> + rc = wait_event_timeout(core->wait, (core->resp_received == 1),
>> + msecs_to_jiffies(Q6_READY_TIMEOUT_MS));
>
> The wait and resp_received could favourably be expressed as a completion
> instead, as all we care about is that this happened once.
will give that a try.
>
>> + if (rc > 0 && core->resp_received) {
>> + core->resp_received = 0;
>> + return 0;
>> + }
>
> It wasn't obvious at first sight that this is the success case and the
> return rc below was the error case...
>
okay, I can add some comments here to help.
>> +
>> + return rc;
>
> And this will actually be 0 if core->resp_received has not become 1 at
> the timeout.
>
yep, will return an error value here.
>> +}
>> +
>> +static bool q6core_is_adsp_ready(struct q6core *core)
>> +{
>> + struct apr_device *adev = core->adev;
>> + struct apr_hdr hdr = {0};
>> + int rc;
>> +
>> + hdr.hdr_field = APR_HDR_FIELD(APR_MSG_TYPE_SEQ_CMD,
>> + APR_HDR_LEN(APR_HDR_SIZE), APR_PKT_VER);
>> + hdr.pkt_size = APR_PKT_SIZE(APR_HDR_SIZE, 0);
>> + hdr.opcode = AVCS_CMD_ADSP_EVENT_GET_STATE;
>> +
>> + rc = apr_send_pkt(adev, (uint32_t *)&hdr);
>> + if (rc < 0)
>> + return false;
>> +
>> + rc = wait_event_timeout(core->wait, (core->resp_received == 1),
>> + msecs_to_jiffies(Q6_READY_TIMEOUT_MS));
>
> I think this too would be nicer to describe with a completion.
>
> Currently it's possible to ask is the dsp is ready, time out and ask
> again, just to receive the first ack and continue. The service request
> sleep might then wake up on this previous ack.
>
> If you describe this by two different completions for the two waits you
> avoid any such race conditions occurring.
>
sure i will make a note of it when I try completions.
>> + if (rc > 0 && core->resp_received) {
>> + core->resp_received = 0;
>> + if (core->avcs_state == 0x1)
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static int q6core_probe(struct apr_device *adev)
>> +{
>> + struct q6core *core;
>> + unsigned long timeout = jiffies +
>> + msecs_to_jiffies(ADSP_STATE_READY_TIMEOUT_MS);
>> + int ret = 0;
>> +
>> + core = devm_kzalloc(&adev->dev, sizeof(*core), GFP_KERNEL);
>> + if (!core)
>> + return -ENOMEM;
>> +
>> + dev_set_drvdata(&adev->dev, core);
>> +
>> + core->adev = adev;
>> + init_waitqueue_head(&core->wait);
>> +
>> + do {
>> + if (!q6core_is_adsp_ready(core)) {
>> + dev_info(&adev->dev, "ADSP Audio isn't ready\n");
>> + } else {
>> + dev_info(&adev->dev, "ADSP Audio is ready\n");
>> +
>> + ret = q6core_get_svc_versions(core);
>> + if (!ret)
>> + q6core_add_static_services(core);
>> +
>> + break;
>> + }
>> + } while (time_after(timeout, jiffies));
>
> This would be much better rewritten as:
>
> for (;;) {
> if (q6core_is_adsp_ready(core))
> break;
>
> if (time_after(timeout, jiffies))
> return -ETIMEDOUT;
> }
>
> ret = q6core_get_svc_versions(core);
> if (ret)
> return ret;
>
> q6core_add_static_services(core);
>
Sure.
>> +
>> + return ret;
>> +}
>> +
>> +
>> +static struct apr_driver qcom_q6core_driver = {
>> + .probe = q6core_probe,
>> + .remove = q6core_exit,
>> + .callback = core_callback,
>> + .id_table = core_id,
>> + .driver = {
>> + .name = "qcom-q6core",
>> + },
>
> Indentation.
Sure.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox