* [PATCH] ARM: dts: Add missing CPU frequencies for Exynos5422/5800
From: Markus Reichl @ 2016-12-19 9:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f4f0dd96-abf1-d1ba-86a6-99f216b5175b@osg.samsung.com>
Hi Javier,
Am 16.12.2016 um 17:22 schrieb Javier Martinez Canillas:
> Hello Markus,
>
> On 12/16/2016 06:08 AM, Markus Reichl wrote:
>> Am 16.12.2016 um 08:37 schrieb Krzysztof Kozlowski:
>>> On Thu, Dec 15, 2016 at 04:52:58PM -0800, Doug Anderson wrote:
>>>>> [ I added Arjun to Cc:, maybe he can help in explaining this issue
>>>>> (unfortunately Inderpal's email is no longer working). ]
>>>>>
>>>>> Please also note that on Exynos5422/5800 SoCs the same ARM rail
>>>>> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
>>>>> IOW if the problem exists it is already present in the mainline
>>>>> kernel.
>>>>
>>>> Interesting. In the ChromeOS tree I see significantly higher voltages
>>>> needed... Note that one might naively look at
>>>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#178>.
>>>>
>>>> 1362500, /* L0 2100 */
>>>> 1312500, /* L1 2000 */
>>>>
>>>> ..but, amazingly enough those voltages aren't used at all. Surprise!
>>>>
>>>> I believe that the above numbers are actually not used and the ASV
>>>> numbers are used instead. See
>>>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/mach-exynos/include/mach/asv-exynos542x.h#452>
>>>>
>>>> { 2100000,
>>>> 1350000, 1350000, 1350000, 1350000, 1350000,
>>>> 1337500, 1325000, 1312500, 1300000, 1287500,
>>>> 1275000, 1262500, 1250000, 1237500 },
>>>>
>>>> I believe that interpretation there is: some bins of the CPU can run
>>>> at 2.1 GHz just fine at 1.25 V but others need up to 1.35V.
>>>
>>> That is definitely the case. One could just look at vendors ASV table
>>> (for 1.9 GHz):
>>> { 1900000, 1300000, 1287500, 1262500, 1237500, 1225000, 1212500,
>>> 1200000, 1187500, 1175000, 1162500, 1150000,
>>> 1137500, 1125000, 1112500, 1112500},
>>>
>>> The theoretical difference is up to 1.875V! From my experiments I saw
>>> BIN1 chips which should be the same... but some working on 1.2V, some on
>>> 1.225V (@1.9 GHz). I didn't see any requiring higher voltages but that
>>> does not mean that there aren't such...
>>>
>>>> ...so if you're running at 2.1 GHz at 1.25V then perhaps you're just
>>>> running on a CPU from a nice bin?
>>
>> I've been running the proposed frequency/voltage combinations without any
>> stability problems on my XU4, XU3 and even XU3-lite ( I did not delete the
>> nodes on XU3-lite dts) with make -j8 kernel and ssvb-cpuburn.
>> The chips are poorly cooled, especially the XU4 and quickly step down.
>>
>>>
>>> Would be nice to see a dump of PKG_ID and AUX_INFO chipid registers
>>> along with name of tested board. Because the "Tested on XU3" is not
>>> sufficient.
>>
>> If you point me to how to read these values out, I will publish them.
>>
>
> You can use the exynos-chipid driver posted by Pankaj. Apply patches 1 and
> 2 from this series (http://www.spinics.net/lists/arm-kernel/msg548384.html)
> and then this diff to get the values of the registers that Krzysztof asked:
>
Thanks for the code.
XU4: [ 0.080039] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x1c04832a] AUX_INFO[0x43]
XU3: [ 0.080034] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x1604832a] AUX_INFO[0x43]
XU3-lite:[ 0.080033] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x5a12832a] AUX_INFO[0x13000054]
Servus,
--
Markus Reichl
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> index cf0128b18ee2..49fa76ec6d49 100644
> --- a/drivers/soc/samsung/exynos-chipid.c
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -22,6 +22,9 @@
> #define EXYNOS_MAINREV_MASK (0xF << 0)
> #define EXYNOS_REV_MASK (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
>
> +#define EXYNOS_PKG_ID 0x04
> +#define EXYNOS_AUX_INFO 0x1C
> +
> static const struct exynos_soc_id {
> const char *name;
> unsigned int id;
> @@ -71,6 +74,8 @@ int __init exynos_chipid_early_init(void)
> const struct of_device_id *match;
> u32 product_id;
> u32 revision;
> + u32 pkg_id;
> + u32 aux_info;
>
> np = of_find_matching_node_and_match(NULL,
> of_exynos_chipid_ids, &match);
> @@ -84,6 +89,8 @@ int __init exynos_chipid_early_init(void)
>
> product_id = readl_relaxed(exynos_chipid_base);
> revision = product_id & EXYNOS_REV_MASK;
> + pkg_id = readl_relaxed(exynos_chipid_base + EXYNOS_PKG_ID);
> + aux_info = readl_relaxed(exynos_chipid_base + EXYNOS_AUX_INFO);
> iounmap(exynos_chipid_base);
>
> soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> @@ -100,8 +107,8 @@ int __init exynos_chipid_early_init(void)
> soc_dev_attr->soc_id = product_id_to_soc_id(product_id);
>
>
> - pr_info("Exynos: CPU[%s] CPU_REV[0x%x] Detected\n",
> - product_id_to_soc_id(product_id), revision);
> + pr_info("Exynos: CPU[%s] CPU_REV[0x%x] PKG_ID[0x%x] AUX_INFO[0x%x] \n",
> + product_id_to_soc_id(product_id), revision, pkg_id, aux_info);
>
> soc_dev = soc_device_register(soc_dev_attr);
> if (IS_ERR(soc_dev)) {
>
> Best regards,
>
^ permalink raw reply
* [PATCH v3 1/3] USB3/DWC3: Add definition for global soc bus configuration register
From: Changming Huang @ 2016-12-19 9:25 UTC (permalink / raw)
To: linux-arm-kernel
Add the macro definition for global soc bus configuration register 0/1
Signed-off-by: Changming Huang <jerry.huang@nxp.com>
---
Changes in v3:
- no change
Changes in v2:
- split the patch
- add more macro definition for soc bus configuration register
drivers/usb/dwc3/core.h | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index de5a857..065aa6f 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -161,6 +161,32 @@
/* Bit fields */
+/* Global SoC Bus Configuration Register 0 */
+#define AXI3_CACHE_TYPE_AW 0x8 /* write allocate */
+#define AXI3_CACHE_TYPE_AR 0x4 /* read allocate */
+#define AXI3_CACHE_TYPE_SNP 0x2 /* cacheable */
+#define AXI3_CACHE_TYPE_BUF 0x1 /* bufferable */
+#define DWC3_GSBUSCFG0_DATARD_SHIFT 28
+#define DWC3_GSBUSCFG0_DESCRD_SHIFT 24
+#define DWC3_GSBUSCFG0_DATAWR_SHIFT 20
+#define DWC3_GSBUSCFG0_DESCWR_SHIFT 16
+#define DWC3_GSBUSCFG0_SNP_MASK 0xffff0000
+#define DWC3_GSBUSCFG0_DATABIGEND (1 << 11)
+#define DWC3_GSBUSCFG0_DESCBIGEND (1 << 10)
+#define DWC3_GSBUSCFG0_INCR256BRSTENA (1 << 7) /* INCR256 burst */
+#define DWC3_GSBUSCFG0_INCR128BRSTENA (1 << 6) /* INCR128 burst */
+#define DWC3_GSBUSCFG0_INCR64BRSTENA (1 << 5) /* INCR64 burst */
+#define DWC3_GSBUSCFG0_INCR32BRSTENA (1 << 4) /* INCR32 burst */
+#define DWC3_GSBUSCFG0_INCR16BRSTENA (1 << 3) /* INCR16 burst */
+#define DWC3_GSBUSCFG0_INCR8BRSTENA (1 << 2) /* INCR8 burst */
+#define DWC3_GSBUSCFG0_INCR4BRSTENA (1 << 1) /* INCR4 burst */
+#define DWC3_GSBUSCFG0_INCRBRSTENA (1 << 0) /* undefined length enable */
+#define DWC3_GSBUSCFG0_INCRBRST_MASK 0xff
+
+/* Global SoC Bus Configuration Register 1 */
+#define DWC3_GSBUSCFG1_1KPAGEENA (1 << 12) /* 1K page boundary enable */
+#define DWC3_GSBUSCFG1_PTRANSLIMIT_MASK 0xf00
+
/* Global Debug Queue/FIFO Space Available Register */
#define DWC3_GDBGFIFOSPACE_NUM(n) ((n) & 0x1f)
#define DWC3_GDBGFIFOSPACE_TYPE(n) (((n) << 5) & 0x1e0)
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 2/3] USB3/DWC3: Add property "snps, incr-burst-type-adjustment" for INCR burst type
From: Changming Huang @ 2016-12-19 9:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1482139554-13618-1-git-send-email-jerry.huang@nxp.com>
New property "snps,incr-burst-type-adjustment = <x>, <y>" for USB3.0 DWC3.
Field "x": 1/0 - undefined length INCR burst type enable or not;
Field "y": INCR4/INCR8/INCR16/INCR32/INCR64/INCR128/INCR256 burst type.
While enabling undefined length INCR burst type and INCR16 burst type,
get better write performance on NXP Layerscape platform:
around 3% improvement (from 364MB/s to 375MB/s).
Signed-off-by: Changming Huang <jerry.huang@nxp.com>
---
Changes in v3:
- add new property for INCR burst in usb node.
Documentation/devicetree/bindings/usb/dwc3.txt | 5 +++++
arch/arm/boot/dts/ls1021a.dtsi | 1 +
arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 3 +++
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 2 ++
4 files changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
index e3e6983..8c405a3 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -55,6 +55,10 @@ Optional properties:
fladj_30mhz_sdbnd signal is invalid or incorrect.
- <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated.
+ - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0
+ register, undefined length INCR burst type enable and INCRx type.
+ First field is for undefined length INCR burst type enable or not.
+ Second field is for largest INCRx type enabled.
This is usually a subnode to DWC3 glue to which it is connected.
@@ -63,4 +67,5 @@ dwc3 at 4a030000 {
reg = <0x4a030000 0xcfff>;
interrupts = <0 92 4>
usb-phy = <&usb2_phy>, <&usb3,phy>;
+ snps,incr-burst-type-adjustment = <0x1>, <16>;
};
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 368e219..2999edb 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -627,6 +627,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <0x1>, <16>;
};
pcie at 3400000 {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 97d331e..64828ce 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -482,6 +482,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <0x1>, <16>;
};
usb1: usb3 at 3000000 {
@@ -491,6 +492,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <0x1>, <16>;
};
usb2: usb3 at 3100000 {
@@ -500,6 +502,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <0x1>, <16>;
};
sata: sata at 3200000 {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
index d058e56..414af27 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
@@ -710,6 +710,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <0x1>, <16>;
};
usb1: usb3 at 3110000 {
@@ -720,6 +721,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <0x1>, <16>;
};
ccn at 4000000 {
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 3/3] USB3/DWC3: Enable undefined length INCR burst type
From: Changming Huang @ 2016-12-19 9:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1482139554-13618-1-git-send-email-jerry.huang@nxp.com>
Enable the undefined length INCR burst type and set INCRx.
Different platform may has the different burst size type.
In order to get best performance, we need to tune the burst size to
one special value, instead of the default value.
Signed-off-by: Changming Huang <jerry.huang@nxp.com>
Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
---
Changes in v3:
- add new property for INCR burst in usb node to reset GSBUSCFG0.
Changes in v2:
- split patch
- create one new function to handle soc bus configuration register.
drivers/usb/dwc3/core.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/usb/dwc3/core.h | 7 +++++++
2 files changed, 58 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 369bab1..404d7e9 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -650,6 +650,55 @@ static void dwc3_core_setup_global_control(struct dwc3 *dwc)
dwc3_writel(dwc->regs, DWC3_GCTL, reg);
}
+/* set global soc bus configuration registers */
+static void dwc3_set_soc_bus_cfg(struct dwc3 *dwc)
+{
+ struct device *dev = dwc->dev;
+ u32 cfg;
+ int ret;
+
+ cfg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);
+
+ /* Get INCR burst type, if return !NULL, not to change this type */
+ ret = device_property_read_u32_array(dev,
+ "snps,incr-burst-type-adjustment",
+ dwc->incr_burst_type, 2);
+ if (!ret) {
+ /* Enable Undefined Length INCR Burst and Enable INCRx Burst */
+ cfg &= ~DWC3_GSBUSCFG0_INCRBRST_MASK;
+ if (*dwc->incr_burst_type)
+ cfg |= DWC3_GSBUSCFG0_INCRBRSTENA;
+ switch (*(dwc->incr_burst_type + 1)) {
+ case 256:
+ cfg |= DWC3_GSBUSCFG0_INCR256BRSTENA;
+ break;
+ case 128:
+ cfg |= DWC3_GSBUSCFG0_INCR128BRSTENA;
+ break;
+ case 64:
+ cfg |= DWC3_GSBUSCFG0_INCR64BRSTENA;
+ break;
+ case 32:
+ cfg |= DWC3_GSBUSCFG0_INCR32BRSTENA;
+ break;
+ case 16:
+ cfg |= DWC3_GSBUSCFG0_INCR16BRSTENA;
+ break;
+ case 8:
+ cfg |= DWC3_GSBUSCFG0_INCR8BRSTENA;
+ break;
+ case 4:
+ cfg |= DWC3_GSBUSCFG0_INCR4BRSTENA;
+ break;
+ default:
+ dev_err(dev, "Invalid property\n");
+ break;
+ }
+ }
+
+ dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, cfg);
+}
+
/**
* dwc3_core_init - Low-level initialization of DWC3 Core
* @dwc: Pointer to our controller context structure
@@ -698,6 +747,8 @@ static int dwc3_core_init(struct dwc3 *dwc)
/* Adjust Frame Length */
dwc3_frame_length_adjustment(dwc);
+ dwc3_set_soc_bus_cfg(dwc);
+
usb_phy_set_suspend(dwc->usb2_phy, 0);
usb_phy_set_suspend(dwc->usb3_phy, 0);
ret = phy_power_on(dwc->usb2_generic_phy);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 065aa6f..cfe389b 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -805,6 +805,7 @@ struct dwc3_scratchpad_array {
* @regs: base address for our registers
* @regs_size: address space size
* @fladj: frame length adjustment
+ * @incr_burst_type: INCR burst type adjustment
* @irq_gadget: peripheral controller's IRQ number
* @nr_scratch: number of scratch buffers
* @u1u2: only used on revisions <1.83a for workaround
@@ -928,6 +929,12 @@ struct dwc3 {
enum usb_phy_interface hsphy_mode;
u32 fladj;
+ /*
+ * For INCR burst type.
+ * First field: for undefined length INCR burst type enable.
+ * Second field: for INCRx burst type enable
+ */
+ u32 incr_burst_type[2];
u32 irq_gadget;
u32 nr_scratch;
u32 u1u2;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v10 6/8] arm/arm64: vgic: Implement VGICv3 CPU interface access
From: Vijay Kilari @ 2016-12-19 9:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <af6f17a5-f20a-91f7-c280-1669e9b805a4@redhat.com>
On Fri, Dec 16, 2016 at 5:55 PM, Auger Eric <eric.auger@redhat.com> wrote:
> Hi Vijaya,
>
> On 01/12/2016 08:09, vijay.kilari at gmail.com wrote:
>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> VGICv3 CPU interface registers are accessed using
>> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
>> as 64-bit. The cpu MPIDR value is passed along with register id.
>> is used to identify the cpu for registers access.
> s/is/It is
>>
>> The VM that supports SEIs expect it on destination machine to handle
>> guest aborts and hence checked for ICC_CTLR_EL1.SEIS compatibility.
>> Similarly, VM that supports Affinity Level 3 that is required for AArch64
>> mode, is required to be supported on destination machine. Hence checked
>> for ICC_CTLR_EL1.A3V compatibility.
> We make sure ICC_CTLR_EL1 SEIS and A3V are compatible on source and target?
>>
>> The arch/arm64/kvm/vgic-sys-reg-v3.c handles read and write of VGIC
>> CPU registers for AArch64.
>>
>> For AArch32 mode, arch/arm/kvm/vgic-v3-coproc.c file is created but
>> APIs are not implemented.
>>
>> Updated arch/arm/include/uapi/asm/kvm.h with new definitions
>> required to compile for AArch32.
>>
>> The version of VGIC v3 specification is define here
> s/define/defined
>> Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>>
>> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> ---
>> --- /dev/null
>> +++ b/arch/arm64/kvm/vgic-sys-reg-v3.c
>> @@ -0,0 +1,338 @@
>> +/*
>> + * VGIC system registers handling functions for AArch64 mode
>> + *
>> + * This program 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 program 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.
>> + */
>> +
>> +#include <linux/irqchip/arm-gic-v3.h>
>> +#include <linux/kvm.h>
>> +#include <linux/kvm_host.h>
>> +#include <asm/kvm_emulate.h>
>> +#include "vgic.h"
>> +#include "sys_regs.h"
>> +
>> +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + u32 host_pri_bits, host_id_bits, host_seis, host_a3v, seis, a3v;
>> + struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
>> + struct vgic_vmcr vmcr;
>> + u64 val;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (p->is_write) {
>> + val = p->regval;
>> +
>> + /*
>> + * Disallow restoring VM state if not supported by this
>> + * hardware.
>> + */
>> + host_pri_bits = ((val & ICC_CTLR_EL1_PRI_BITS_MASK) >>
>> + ICC_CTLR_EL1_PRI_BITS_SHIFT) + 1;
> I am confused by the "host" terminology. Those are the "source" values
> we want to restore and we compare with the destination current value, right?
yes
>> + if (host_pri_bits > vgic_v3_cpu->num_pri_bits)
>> + return false;
> I am lost. Who did set num_pri_bits and num_id_bits we compare with?
In vgic_v3_enable() these values are computed
> Seis and a3v I get this is computed on probe
>> +
>> + vgic_v3_cpu->num_pri_bits = host_pri_bits;
>> +
>> + host_id_bits = (val & ICC_CTLR_EL1_ID_BITS_MASK) >>
>> + ICC_CTLR_EL1_ID_BITS_SHIFT;
>> + if (host_id_bits > vgic_v3_cpu->num_id_bits)
>> + return false;
>> +
>> + vgic_v3_cpu->num_id_bits = host_id_bits;
>> +
>> + host_seis = ((kvm_vgic_global_state.ich_vtr_el2 &
>> + ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT);
>> + seis = (val & ICC_CTLR_EL1_SEIS_MASK) >>
>> + ICC_CTLR_EL1_SEIS_SHIFT;
>> + if (host_seis != seis)
>> + return false;
>> +
>> + host_a3v = ((kvm_vgic_global_state.ich_vtr_el2 &
>> + ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT);
>> + a3v = (val & ICC_CTLR_EL1_A3V_MASK) >> ICC_CTLR_EL1_A3V_SHIFT;
>> + if (host_a3v != a3v)
>> + return false;
>> +
>> + vmcr.ctlr = val & ICC_CTLR_EL1_CBPR_MASK;
>> + vmcr.ctlr |= val & ICC_CTLR_EL1_EOImode_MASK;
> nit: I still don't get why the vmcr has CPBR and EOImode set with the
> ICC_CTLR_EL1 layout and then this gets transformed in the proper vmcr
> format by vgic_set_vmcr. This is confusing to me and would at least
> deserve a comment attached to struct vgic_vmcr definition.
I will add a comment
>
> Why don't we set the vmcr.ctlr directly in its ICH_VMCR format? In
> set/get_vmcr all the other struct vgic_vmcr fields are handled in
> ICH_VMCR native layout except the ctrl field.
None of the fields of struct vgic_vmcr are in ICH_VMCR native layout.
Except ctlr all the other fields are registers having single field value.
Ex: pmr, bpr0 etc.,
>
>> + vgic_set_vmcr(vcpu, &vmcr);
>> + } else {
>> + val = 0;
>> + val |= (vgic_v3_cpu->num_pri_bits - 1) <<
>> + ICC_CTLR_EL1_PRI_BITS_SHIFT;
>> + val |= vgic_v3_cpu->num_id_bits << ICC_CTLR_EL1_ID_BITS_SHIFT;
>> + val |= ((kvm_vgic_global_state.ich_vtr_el2 &
>> + ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT) <<
>> + ICC_CTLR_EL1_SEIS_SHIFT;
>> + val |= ((kvm_vgic_global_state.ich_vtr_el2 &
>> + ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT) <<
>> + ICC_CTLR_EL1_A3V_SHIFT;
>> + val |= vmcr.ctlr & ICC_CTLR_EL1_CBPR_MASK;
>> + val |= vmcr.ctlr & ICC_CTLR_EL1_EOImode_MASK;
>> +
>> + p->regval = val;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_vmcr vmcr;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (p->is_write) {
>> + vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> ICC_PMR_EL1_SHIFT;
>> + vgic_set_vmcr(vcpu, &vmcr);
>> + } else {
>> + p->regval = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & ICC_PMR_EL1_MASK;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool access_gic_bpr0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_vmcr vmcr;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (p->is_write) {
>> + vmcr.bpr = (p->regval & ICC_BPR0_EL1_MASK) >>
>> + ICC_BPR0_EL1_SHIFT;
>> + vgic_set_vmcr(vcpu, &vmcr);
>> + } else {
>> + p->regval = (vmcr.bpr << ICC_BPR0_EL1_SHIFT) &
>> + ICC_BPR0_EL1_MASK;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool access_gic_bpr1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_vmcr vmcr;
>> +
>> + if (!p->is_write)
>> + p->regval = 0;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (!((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >> ICH_VMCR_CBPR_SHIFT)) {
>> + if (p->is_write) {
>> + vmcr.abpr = (p->regval & ICC_BPR1_EL1_MASK) >>
>> + ICC_BPR1_EL1_SHIFT;
>> + vgic_set_vmcr(vcpu, &vmcr);
>> + } else {
>> + p->regval = (vmcr.abpr << ICC_BPR1_EL1_SHIFT) &
>> + ICC_BPR1_EL1_MASK;
>> + }
>> + } else {
>> + if (!p->is_write)
>> + p->regval = min((vmcr.bpr + 1), 7U);
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool access_gic_grpen0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_vmcr vmcr;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (p->is_write) {
>> + vmcr.grpen0 = (p->regval & ICC_IGRPEN0_EL1_MASK) >>
>> + ICC_IGRPEN0_EL1_SHIFT;
>> + vgic_set_vmcr(vcpu, &vmcr);
>> + } else {
>> + p->regval = (vmcr.grpen0 << ICC_IGRPEN0_EL1_SHIFT) &
>> + ICC_IGRPEN0_EL1_MASK;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool access_gic_grpen1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_vmcr vmcr;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (p->is_write) {
>> + vmcr.grpen1 = (p->regval & ICC_IGRPEN1_EL1_MASK) >>
>> + ICC_IGRPEN1_EL1_SHIFT;
>> + vgic_set_vmcr(vcpu, &vmcr);
>> + } else {
>> + p->regval = (vmcr.grpen1 << ICC_IGRPEN1_EL1_SHIFT) &
>> + ICC_IGRPEN1_EL1_MASK;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static void vgic_v3_access_apr_reg(struct kvm_vcpu *vcpu,
>> + struct sys_reg_params *p, u8 apr, u8 idx)
>> +{
>> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
>> + uint32_t *ap_reg;
>> +
>> + if (apr)
>> + ap_reg = &vgicv3->vgic_ap1r[idx];
>> + else
>> + ap_reg = &vgicv3->vgic_ap0r[idx];
>> +
>> + if (p->is_write)
>> + *ap_reg = p->regval;
>> + else
>> + p->regval = *ap_reg;
>> +}
>> +
>> +static bool access_gic_aprn(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r, u8 apr)
>> +{
>> + struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
>> + u8 idx = r->Op2 & 3;
>> +
>> + /*
>> + * num_pri_bits are initialized with HW supported values.
>> + * We can rely safely on num_pri_bits even if VM has not
>> + * restored ICC_CTLR_EL1 before restoring APnR registers.
>> + */
>> + switch (vgic_v3_cpu->num_pri_bits) {
>> + case 7:
>> + vgic_v3_access_apr_reg(vcpu, p, apr, idx);
>> + break;
>> + case 6:
>> + if (idx > 1)
>> + goto err;
>> + vgic_v3_access_apr_reg(vcpu, p, apr, idx);
>> + break;
>> + default:
>> + if (idx > 0)
>> + goto err;
>> + vgic_v3_access_apr_reg(vcpu, p, apr, idx);
>> + }
>> +
>> + return true;
>> +err:
>> + if (!p->is_write)
>> + p->regval = 0;
>> +
>> + return false;
>> +}
>> +
>> +static bool access_gic_ap0r(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +
>> +{
>> + return access_gic_aprn(vcpu, p, r, 0);
>> +}
>> +
>> +static bool access_gic_ap1r(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + return access_gic_aprn(vcpu, p, r, 1);
>> +}
>> +
>> +static bool access_gic_sre(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
>> +
>> + /* Validate SRE bit */
>> + if (p->is_write) {
>> + if (!(p->regval & ICC_SRE_EL1_SRE))
>> + return false;
>> + } else {
>> + p->regval = vgicv3->vgic_sre;
>> + }
>> +
>> + return true;
>> +}
>> +static const struct sys_reg_desc gic_v3_icc_reg_descs[] = {
>> + /* ICC_PMR_EL1 */
>> + { Op0(3), Op1(0), CRn(4), CRm(6), Op2(0), access_gic_pmr },
>> + /* ICC_BPR0_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(3), access_gic_bpr0 },
>> + /* ICC_AP0R0_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(4), access_gic_ap0r },
>> + /* ICC_AP0R1_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(5), access_gic_ap0r },
>> + /* ICC_AP0R2_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(6), access_gic_ap0r },
>> + /* ICC_AP0R3_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(7), access_gic_ap0r },
>> + /* ICC_AP1R0_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(0), access_gic_ap1r },
>> + /* ICC_AP1R1_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(1), access_gic_ap1r },
>> + /* ICC_AP1R2_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(2), access_gic_ap1r },
>> + /* ICC_AP1R3_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(3), access_gic_ap1r },
>> + /* ICC_BPR1_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(3), access_gic_bpr1 },
>> + /* ICC_CTLR_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(4), access_gic_ctlr },
>> + /* ICC_SRE_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(5), access_gic_sre },
>> + /* ICC_IGRPEN0_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(6), access_gic_grpen0 },
>> + /* ICC_GRPEN1_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(7), access_gic_grpen1 },
>> +};
>> +
>> +int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
>> + u64 *reg)
>> +{
>> + struct sys_reg_params params;
>> + u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
> At the beginning I asked myself why we did not OR with KVM_REG_ARM64 as
> stated in include/uapi/linux/kvm.h but then looking at
> index_to_params() implementation it looks it is not used.
>> +
>> + params.regval = *reg;
>> + params.is_write = is_write;
>> + params.is_aarch32 = false;
>> + params.is_32bit = false;
>> +
>> + if (find_reg_by_id(sysreg, ¶ms, gic_v3_icc_reg_descs,
>> + ARRAY_SIZE(gic_v3_icc_reg_descs)))
>> + return 0;
>> +
>> + return -ENXIO;
>> +}
>> +
>> +int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write, u64 id,
>> + u64 *reg)
>> +{
>> + struct sys_reg_params params;
>> + const struct sys_reg_desc *r;
>> + u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
>> +
>> + if (is_write)
>> + params.regval = *reg;
>> + params.is_write = is_write;
>> + params.is_aarch32 = false;
>> + params.is_32bit = false;
>> +
>> + r = find_reg_by_id(sysreg, ¶ms, gic_v3_icc_reg_descs,
>> + ARRAY_SIZE(gic_v3_icc_reg_descs));
>> + if (!r)
>> + return -ENXIO;
>> +
>> + if (!r->access(vcpu, ¶ms, r))
>> + return -EINVAL;
>> +
>> + if (!is_write)
>> + *reg = params.regval;
>> +
>> + return 0;
>> +}
>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>> index 002f092..730a18a 100644
>> --- a/include/kvm/arm_vgic.h
>> +++ b/include/kvm/arm_vgic.h
>> @@ -71,6 +71,9 @@ struct vgic_global {
>>
>> /* GIC system register CPU interface */
>> struct static_key_false gicv3_cpuif;
>> +
>> + /* Cache ICH_VTR_EL2 reg value */
> nit: comment does not bring much value I think
>> + u32 ich_vtr_el2;
>> };
>>
>> extern struct vgic_global kvm_vgic_global_state;
>> @@ -269,6 +272,12 @@ struct vgic_cpu {
>> u64 pendbaser;
>>
>> bool lpis_enabled;
>> +
>> + /* Cache guest priority bits */
>> + u32 num_pri_bits;
>> +
>> + /* Cache guest interrupt ID bits */
>> + u32 num_id_bits;
>> };
>>
>> extern struct static_key_false vgic_v2_cpuif_trap;
>> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
>> index bc7de95..b6266fe 100644
>> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
>> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
>> @@ -16,6 +16,7 @@
>> #include <linux/kvm_host.h>
>> #include <kvm/arm_vgic.h>
>> #include <linux/uaccess.h>
>> +#include <asm/kvm_emulate.h>
> not needed I think
OK.
^ permalink raw reply
* [PATCH] ARM: dts: da850-lcdk: add gpio-keys
From: Bartosz Golaszewski @ 2016-12-19 9:53 UTC (permalink / raw)
To: linux-arm-kernel
Add a gpio-keys node for two user buttons present on the board.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/boot/dts/da850-lcdk.dts | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 397c77a..628f4de 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -6,6 +6,7 @@
/dts-v1/;
#include "da850.dtsi"
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
/ {
model = "DA850/AM1808/OMAP-L138 LCDK";
@@ -90,6 +91,23 @@
};
};
};
+
+ gpio-keys {
+ compatible = "gpio-keys";
+ autorepeat;
+
+ user1 {
+ label = "GPIO Key USER1";
+ linux,code = <BTN_0>;
+ gpios = <&gpio 36 GPIO_ACTIVE_LOW>;
+ };
+
+ user2 {
+ label = "GPIO Key USER2";
+ linux,code = <BTN_1>;
+ gpios = <&gpio 37 GPIO_ACTIVE_LOW>;
+ };
+ };
};
&pmx_core {
--
2.9.3
^ permalink raw reply related
* [PATCH] bus: da850-mstpri: fix my e-mail address
From: Bartosz Golaszewski @ 2016-12-19 9:58 UTC (permalink / raw)
To: linux-arm-kernel
I noticed my e-mail address is wrong in this one. This patch fixes it.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/bus/da8xx-mstpri.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bus/da8xx-mstpri.c b/drivers/bus/da8xx-mstpri.c
index 063397f..9af9bcc 100644
--- a/drivers/bus/da8xx-mstpri.c
+++ b/drivers/bus/da8xx-mstpri.c
@@ -4,7 +4,7 @@
* Copyright (C) 2016 BayLibre SAS
*
* Author:
- * Bartosz Golaszewski <bgolaszewski@baylibre.com.com>
+ * Bartosz Golaszewski <bgolaszewski@baylibre.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
--
2.9.3
^ permalink raw reply related
* [bug report v4.8] fs/locks.c: kernel oops during posix lock stress test
From: Ming Lei @ 2016-12-19 9:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACVXFVMA4mKCUt9gnwbdYVMqSTk-H+LX7dQiAxz_FqwKPQ1X9Q@mail.gmail.com>
Hi,
On Thu, Dec 8, 2016 at 11:57 PM, Ming Lei <ming.lei@canonical.com> wrote:
> Hi,
>
> On Mon, Nov 28, 2016 at 9:40 PM, Jeff Layton <jlayton@poochiereds.net> wrote:
>> On Mon, 2016-11-28 at 11:10 +0800, Ming Lei wrote:
>>> Hi Guys,
>>>
>>> When I run stress-ng via the following steps on one ARM64 dual
>>> socket system(Cavium Thunder), the kernel oops[1] can often be
>>> triggered after running the stress test for several hours(sometimes
>>> it may take longer):
>>>
>>> - git clone git://kernel.ubuntu.com/cking/stress-ng.git
>>> - apply the attachment patch which just makes the posix file
>>> lock stress test more aggressive
>>> - run the test via '~/git/stress-ng$./stress-ng --lockf 128 --aggressive'
>>>
>>>
>>> From the oops log, looks one garbage file_lock node is got
>>> from the linked list of 'ctx->flc_posix' when the issue happens.
>>>
>>> BTW, the issue isn't observed on single socket Cavium Thunder yet,
>>> and the same issue can be seen on Ubuntu Xenial(v4.4 based kernel)
>>> too.
>>>
>>> Thanks,
>>> Ming
>>>
>>
>> Some questions just for clarification:
>>
>> - I assume this is being run on a local fs of some sort? ext4 or xfs or
>> something?
>>
>> - have you seen this on any other arch, besides ARM?
>>
>> The file locking code does do some lockless checking to see whether the
>> i_flctx is even present and whether the list is empty in
>> locks_remove_posix. It's possible we have some barrier problems there,
>
> I have used ebpf trace to see what is going on when 'stress-ng --lockf'
> is running, and almost all exported symbols in fs/locks.c are covered.
>
> Except for locks_alloc/locks_free/locks_copy/locks_init, the only observable
> symbols are fcntl_setlk, vfs_lock_file and locks_remove_posix, but
> locks_remove_posix() is just run at the begining and ending of the
> test.
>
> So seems not related with locks_remove_posix().
>
> Then looks only fcntl_setlk() is running from different contexts
> during the test,
> but in this path, the 'ctx->flc_lock' is always held when operating the list.
> That said it is very strange to see the list corrupted even though it is
> protected by the lock.
After some analysis on traces collected recently, there are a few discoveries:
1) the spinlock scenario(ctx->flc_lock) is correct
2) the kernel oops(file lock corruption) always happens in the
task of stress-ng-lockf's child, which isn't affected by
sched_setaffinity(), and the process of stress-ng-lockf is schedued
from one CPU to another one from another socket at random according to
sched_setaffinity() called
from stress-ng main task.
Thanks,
Ming
^ permalink raw reply
* [PATCH v6 2/5] lib: implement __arch_bitrev8x4()
From: Will Deacon @ 2016-12-19 10:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6c1c052d3c1d0c02a791aaaf8e114360ab1cb4e7.1481918884.git.stillcompiling@gmail.com>
On Fri, Dec 16, 2016 at 03:17:51PM -0800, Joshua Clayton wrote:
> Implement faster bitrev8x4() for arm, arm64 and mips, all the platforms
> with CONFIG_HAVE_ARCH_BITREVERSE.
> ARM platforms just need a byteswap added to the existing __arch_bitrev32()
> Amusingly, the mips implementation is exactly the opposite, requiring
> removal of the byteswap from its __arch_bitrev32()
>
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
> ---
> arch/arm/include/asm/bitrev.h | 6 ++++++
> arch/arm64/include/asm/bitrev.h | 6 ++++++
> arch/mips/include/asm/bitrev.h | 6 ++++++
> include/linux/bitrev.h | 1 +
> 4 files changed, 19 insertions(+)
>
> diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
> index ec291c3..9482f78 100644
> --- a/arch/arm/include/asm/bitrev.h
> +++ b/arch/arm/include/asm/bitrev.h
> @@ -17,4 +17,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
> return __arch_bitrev32((u32)x) >> 24;
> }
>
> +static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
> +{
> + __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
> + return x;
> +}
> +
> #endif
> diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
> index a5a0c36..1801078 100644
> --- a/arch/arm64/include/asm/bitrev.h
> +++ b/arch/arm64/include/asm/bitrev.h
> @@ -16,4 +16,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
> return __arch_bitrev32((u32)x) >> 24;
> }
>
> +static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
> +{
> + __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
This is broken -- you're operating on 64-bit registers. I only noticed
because if you write:
swab32(bitrev32(x))
then GCC generates:
rbit w0, w0
rev w0, w0
so perhaps we should just implement the asm-generic version like that
and not bother with the arch-specific stuff?
Will
^ permalink raw reply
* [PATCH] ARM: dts: sun8i-q8-common: enable bluetooth on SDIO Wi-Fi
From: Maxime Ripard @ 2016-12-19 10:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4720181481899200@web7g.yandex.ru>
On Fri, Dec 16, 2016 at 10:40:00PM +0800, Icenowy Zheng wrote:
> >> ?> >? &r_pio {
> >> ?> >? wifi_pwrseq_pin_q8: wifi_pwrseq_pin at 0 {
> >> ?> > - pins = "PL6", "PL7", "PL11";
> >> ?> > + pins = "PL6", "PL7", "PL8", "PL11";
> >> ?> >? function = "gpio_in";
> >> ?> >? bias-pull-up;
> >> ?> >? };
> >> ?>
> >> ?> There's several things wrong here. The first one is that you rely
> >> ?> solely on the pinctrl state to maintain a reset line. This is very
> >> ?> fragile (especially since the GPIO pinctrl state are likely to go away
> >> ?> at some point), but it also means that if your driver wants to recover
> >> ?> from that situation at some point, it won't work.
> >> ?>
> >> ?> The other one is that the bluetooth and wifi chips are two devices in
> >> ?> linux, and you assign that pin to the wrong device (wifi).
> >> ?>
> >> ?> rfkill-gpio is made just for that, so please use it.
> >>
> >> ?The GPIO is not for the radio, but for the full Bluetooth part.
> >
> > I know.
> >
> >> ?If it's set to 0, then the bluetooth part will reset, and the
> >> ?hciattach will fail.
> >
> > Both rfkill-gpio and rfkill-regulator will shutdown when called
> > (either by poking the reset pin or shutting down the regulator), so
> > that definitely seems like an expected behavior to put the device in
> > reset.
> >
> >> ?The BSP uses this as a rfkill, and the result is that the bluetooth
> >> ?on/off switch do not work properly.
> >
> > Then rfkill needs fixing, but working around it by hoping that the
> > core will probe an entirely different device, and enforcing a default
> > that the rest of the kernel might or might not change is both fragile
> > and wrong.
>
> I think a rfkill-gpio here works just like the BSP rfkill...
>
> The real problem is that the Realtek UART bluetooth driver is a userspace
> program (a modified hciattach), which is not capable of the GPIO reset...
Can't you run rfkill before attaching? What is the problem exactly?
It's not in reset for long enough?
This seems more and more like an issue in the BT stack you're
using. We might consider workarounds in the kernel, but they have to
be correct.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161219/7f84717e/attachment.sig>
^ permalink raw reply
* [PATCH v10 6/8] arm/arm64: vgic: Implement VGICv3 CPU interface access
From: Auger Eric @ 2016-12-19 10:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CALicx6spkmp4ipvCA_TUfzaJ0K38tpWHsV9CGU4MEQzTwa00sw@mail.gmail.com>
Hi Vijaya,
On 19/12/2016 10:47, Vijay Kilari wrote:
> On Fri, Dec 16, 2016 at 5:55 PM, Auger Eric <eric.auger@redhat.com> wrote:
>> Hi Vijaya,
>>
>> On 01/12/2016 08:09, vijay.kilari at gmail.com wrote:
>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>
>>> VGICv3 CPU interface registers are accessed using
>>> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
>>> as 64-bit. The cpu MPIDR value is passed along with register id.
>>> is used to identify the cpu for registers access.
>> s/is/It is
>>>
>>> The VM that supports SEIs expect it on destination machine to handle
>>> guest aborts and hence checked for ICC_CTLR_EL1.SEIS compatibility.
>>> Similarly, VM that supports Affinity Level 3 that is required for AArch64
>>> mode, is required to be supported on destination machine. Hence checked
>>> for ICC_CTLR_EL1.A3V compatibility.
>> We make sure ICC_CTLR_EL1 SEIS and A3V are compatible on source and target?
>>>
>>> The arch/arm64/kvm/vgic-sys-reg-v3.c handles read and write of VGIC
>>> CPU registers for AArch64.
>>>
>>> For AArch32 mode, arch/arm/kvm/vgic-v3-coproc.c file is created but
>>> APIs are not implemented.
>>>
>>> Updated arch/arm/include/uapi/asm/kvm.h with new definitions
>>> required to compile for AArch32.
>>>
>>> The version of VGIC v3 specification is define here
>> s/define/defined
>>> Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>>>
>>> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>> ---
>>> --- /dev/null
>>> +++ b/arch/arm64/kvm/vgic-sys-reg-v3.c
>>> @@ -0,0 +1,338 @@
>>> +/*
>>> + * VGIC system registers handling functions for AArch64 mode
>>> + *
>>> + * This program 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 program 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.
>>> + */
>>> +
>>> +#include <linux/irqchip/arm-gic-v3.h>
>>> +#include <linux/kvm.h>
>>> +#include <linux/kvm_host.h>
>>> +#include <asm/kvm_emulate.h>
>>> +#include "vgic.h"
>>> +#include "sys_regs.h"
>>> +
>>> +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + u32 host_pri_bits, host_id_bits, host_seis, host_a3v, seis, a3v;
>>> + struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
>>> + struct vgic_vmcr vmcr;
>>> + u64 val;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + val = p->regval;
>>> +
>>> + /*
>>> + * Disallow restoring VM state if not supported by this
>>> + * hardware.
>>> + */
>>> + host_pri_bits = ((val & ICC_CTLR_EL1_PRI_BITS_MASK) >>
>>> + ICC_CTLR_EL1_PRI_BITS_SHIFT) + 1;
>> I am confused by the "host" terminology. Those are the "source" values
>> we want to restore and we compare with the destination current value, right?
>
> yes
>
>>> + if (host_pri_bits > vgic_v3_cpu->num_pri_bits)
>>> + return false;
>> I am lost. Who did set num_pri_bits and num_id_bits we compare with?
>
> In vgic_v3_enable() these values are computed
OK I missed that.
>
>> Seis and a3v I get this is computed on probe
>>> +
>>> + vgic_v3_cpu->num_pri_bits = host_pri_bits;
>>> +
>>> + host_id_bits = (val & ICC_CTLR_EL1_ID_BITS_MASK) >>
>>> + ICC_CTLR_EL1_ID_BITS_SHIFT;
>>> + if (host_id_bits > vgic_v3_cpu->num_id_bits)
>>> + return false;
>>> +
>>> + vgic_v3_cpu->num_id_bits = host_id_bits;
>>> +
>>> + host_seis = ((kvm_vgic_global_state.ich_vtr_el2 &
>>> + ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT);
>>> + seis = (val & ICC_CTLR_EL1_SEIS_MASK) >>
>>> + ICC_CTLR_EL1_SEIS_SHIFT;
>>> + if (host_seis != seis)
>>> + return false;
>>> +
>>> + host_a3v = ((kvm_vgic_global_state.ich_vtr_el2 &
>>> + ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT);
>>> + a3v = (val & ICC_CTLR_EL1_A3V_MASK) >> ICC_CTLR_EL1_A3V_SHIFT;
>>> + if (host_a3v != a3v)
>>> + return false;
>>> +
>>> + vmcr.ctlr = val & ICC_CTLR_EL1_CBPR_MASK;
>>> + vmcr.ctlr |= val & ICC_CTLR_EL1_EOImode_MASK;
>> nit: I still don't get why the vmcr has CPBR and EOImode set with the
>> ICC_CTLR_EL1 layout and then this gets transformed in the proper vmcr
>> format by vgic_set_vmcr. This is confusing to me and would at least
>> deserve a comment attached to struct vgic_vmcr definition.
>
> I will add a comment
>>
>> Why don't we set the vmcr.ctlr directly in its ICH_VMCR format? In
>> set/get_vmcr all the other struct vgic_vmcr fields are handled in
>> ICH_VMCR native layout except the ctrl field.
>
> None of the fields of struct vgic_vmcr are in ICH_VMCR native layout.
> Except ctlr all the other fields are registers having single field value.
> Ex: pmr, bpr0 etc.,
OK but to me would be more natural to keep vmcr.ctlr in original format
but well that's just my pov. If you add a comment that helps already.
Thanks
Eric
>
>>
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + val = 0;
>>> + val |= (vgic_v3_cpu->num_pri_bits - 1) <<
>>> + ICC_CTLR_EL1_PRI_BITS_SHIFT;
>>> + val |= vgic_v3_cpu->num_id_bits << ICC_CTLR_EL1_ID_BITS_SHIFT;
>>> + val |= ((kvm_vgic_global_state.ich_vtr_el2 &
>>> + ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT) <<
>>> + ICC_CTLR_EL1_SEIS_SHIFT;
>>> + val |= ((kvm_vgic_global_state.ich_vtr_el2 &
>>> + ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT) <<
>>> + ICC_CTLR_EL1_A3V_SHIFT;
>>> + val |= vmcr.ctlr & ICC_CTLR_EL1_CBPR_MASK;
>>> + val |= vmcr.ctlr & ICC_CTLR_EL1_EOImode_MASK;
>>> +
>>> + p->regval = val;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> ICC_PMR_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->regval = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & ICC_PMR_EL1_MASK;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_bpr0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + vmcr.bpr = (p->regval & ICC_BPR0_EL1_MASK) >>
>>> + ICC_BPR0_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->regval = (vmcr.bpr << ICC_BPR0_EL1_SHIFT) &
>>> + ICC_BPR0_EL1_MASK;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_bpr1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + if (!p->is_write)
>>> + p->regval = 0;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (!((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >> ICH_VMCR_CBPR_SHIFT)) {
>>> + if (p->is_write) {
>>> + vmcr.abpr = (p->regval & ICC_BPR1_EL1_MASK) >>
>>> + ICC_BPR1_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->regval = (vmcr.abpr << ICC_BPR1_EL1_SHIFT) &
>>> + ICC_BPR1_EL1_MASK;
>>> + }
>>> + } else {
>>> + if (!p->is_write)
>>> + p->regval = min((vmcr.bpr + 1), 7U);
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_grpen0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + vmcr.grpen0 = (p->regval & ICC_IGRPEN0_EL1_MASK) >>
>>> + ICC_IGRPEN0_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->regval = (vmcr.grpen0 << ICC_IGRPEN0_EL1_SHIFT) &
>>> + ICC_IGRPEN0_EL1_MASK;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_grpen1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + vmcr.grpen1 = (p->regval & ICC_IGRPEN1_EL1_MASK) >>
>>> + ICC_IGRPEN1_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->regval = (vmcr.grpen1 << ICC_IGRPEN1_EL1_SHIFT) &
>>> + ICC_IGRPEN1_EL1_MASK;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static void vgic_v3_access_apr_reg(struct kvm_vcpu *vcpu,
>>> + struct sys_reg_params *p, u8 apr, u8 idx)
>>> +{
>>> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
>>> + uint32_t *ap_reg;
>>> +
>>> + if (apr)
>>> + ap_reg = &vgicv3->vgic_ap1r[idx];
>>> + else
>>> + ap_reg = &vgicv3->vgic_ap0r[idx];
>>> +
>>> + if (p->is_write)
>>> + *ap_reg = p->regval;
>>> + else
>>> + p->regval = *ap_reg;
>>> +}
>>> +
>>> +static bool access_gic_aprn(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r, u8 apr)
>>> +{
>>> + struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
>>> + u8 idx = r->Op2 & 3;
>>> +
>>> + /*
>>> + * num_pri_bits are initialized with HW supported values.
>>> + * We can rely safely on num_pri_bits even if VM has not
>>> + * restored ICC_CTLR_EL1 before restoring APnR registers.
>>> + */
>>> + switch (vgic_v3_cpu->num_pri_bits) {
>>> + case 7:
>>> + vgic_v3_access_apr_reg(vcpu, p, apr, idx);
>>> + break;
>>> + case 6:
>>> + if (idx > 1)
>>> + goto err;
>>> + vgic_v3_access_apr_reg(vcpu, p, apr, idx);
>>> + break;
>>> + default:
>>> + if (idx > 0)
>>> + goto err;
>>> + vgic_v3_access_apr_reg(vcpu, p, apr, idx);
>>> + }
>>> +
>>> + return true;
>>> +err:
>>> + if (!p->is_write)
>>> + p->regval = 0;
>>> +
>>> + return false;
>>> +}
>>> +
>>> +static bool access_gic_ap0r(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +
>>> +{
>>> + return access_gic_aprn(vcpu, p, r, 0);
>>> +}
>>> +
>>> +static bool access_gic_ap1r(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + return access_gic_aprn(vcpu, p, r, 1);
>>> +}
>>> +
>>> +static bool access_gic_sre(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
>>> +
>>> + /* Validate SRE bit */
>>> + if (p->is_write) {
>>> + if (!(p->regval & ICC_SRE_EL1_SRE))
>>> + return false;
>>> + } else {
>>> + p->regval = vgicv3->vgic_sre;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +static const struct sys_reg_desc gic_v3_icc_reg_descs[] = {
>>> + /* ICC_PMR_EL1 */
>>> + { Op0(3), Op1(0), CRn(4), CRm(6), Op2(0), access_gic_pmr },
>>> + /* ICC_BPR0_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(3), access_gic_bpr0 },
>>> + /* ICC_AP0R0_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(4), access_gic_ap0r },
>>> + /* ICC_AP0R1_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(5), access_gic_ap0r },
>>> + /* ICC_AP0R2_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(6), access_gic_ap0r },
>>> + /* ICC_AP0R3_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(7), access_gic_ap0r },
>>> + /* ICC_AP1R0_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(0), access_gic_ap1r },
>>> + /* ICC_AP1R1_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(1), access_gic_ap1r },
>>> + /* ICC_AP1R2_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(2), access_gic_ap1r },
>>> + /* ICC_AP1R3_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(3), access_gic_ap1r },
>>> + /* ICC_BPR1_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(3), access_gic_bpr1 },
>>> + /* ICC_CTLR_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(4), access_gic_ctlr },
>>> + /* ICC_SRE_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(5), access_gic_sre },
>>> + /* ICC_IGRPEN0_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(6), access_gic_grpen0 },
>>> + /* ICC_GRPEN1_EL1 */
>>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(7), access_gic_grpen1 },
>>> +};
>>> +
>>> +int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
>>> + u64 *reg)
>>> +{
>>> + struct sys_reg_params params;
>>> + u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
>> At the beginning I asked myself why we did not OR with KVM_REG_ARM64 as
>> stated in include/uapi/linux/kvm.h but then looking at
>> index_to_params() implementation it looks it is not used.
>>> +
>>> + params.regval = *reg;
>>> + params.is_write = is_write;
>>> + params.is_aarch32 = false;
>>> + params.is_32bit = false;
>>> +
>>> + if (find_reg_by_id(sysreg, ¶ms, gic_v3_icc_reg_descs,
>>> + ARRAY_SIZE(gic_v3_icc_reg_descs)))
>>> + return 0;
>>> +
>>> + return -ENXIO;
>>> +}
>>> +
>>> +int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write, u64 id,
>>> + u64 *reg)
>>> +{
>>> + struct sys_reg_params params;
>>> + const struct sys_reg_desc *r;
>>> + u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
>>> +
>>> + if (is_write)
>>> + params.regval = *reg;
>>> + params.is_write = is_write;
>>> + params.is_aarch32 = false;
>>> + params.is_32bit = false;
>>> +
>>> + r = find_reg_by_id(sysreg, ¶ms, gic_v3_icc_reg_descs,
>>> + ARRAY_SIZE(gic_v3_icc_reg_descs));
>>> + if (!r)
>>> + return -ENXIO;
>>> +
>>> + if (!r->access(vcpu, ¶ms, r))
>>> + return -EINVAL;
>>> +
>>> + if (!is_write)
>>> + *reg = params.regval;
>>> +
>>> + return 0;
>>> +}
>>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>>> index 002f092..730a18a 100644
>>> --- a/include/kvm/arm_vgic.h
>>> +++ b/include/kvm/arm_vgic.h
>>> @@ -71,6 +71,9 @@ struct vgic_global {
>>>
>>> /* GIC system register CPU interface */
>>> struct static_key_false gicv3_cpuif;
>>> +
>>> + /* Cache ICH_VTR_EL2 reg value */
>> nit: comment does not bring much value I think
>>> + u32 ich_vtr_el2;
>>> };
>>>
>>> extern struct vgic_global kvm_vgic_global_state;
>>> @@ -269,6 +272,12 @@ struct vgic_cpu {
>>> u64 pendbaser;
>>>
>>> bool lpis_enabled;
>>> +
>>> + /* Cache guest priority bits */
>>> + u32 num_pri_bits;
>>> +
>>> + /* Cache guest interrupt ID bits */
>>> + u32 num_id_bits;
>>> };
>>>
>>> extern struct static_key_false vgic_v2_cpuif_trap;
>>> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
>>> index bc7de95..b6266fe 100644
>>> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
>>> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
>>> @@ -16,6 +16,7 @@
>>> #include <linux/kvm_host.h>
>>> #include <kvm/arm_vgic.h>
>>> #include <linux/uaccess.h>
>>> +#include <asm/kvm_emulate.h>
>> not needed I think
> OK.
>
> _______________________________________________
> 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 1/1] mm: call force_sig_info before prints
From: Maninder Singh @ 2016-12-19 10:37 UTC (permalink / raw)
To: linux-arm-kernel
prints can delay queuing of signal, so better to print
after force_sig_info.
Let's say process generated SIGSEGV , and some other thread sends
SIGKILL to crashing process and it gets queued before SIGSEGV becuase
of little delay due to prints so in this case coredump might not generate.
Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
Signed-off-by: Amit Nagal <amit.nagal@samsung.com>
Reviewed-by: Ajeet Yadav <ajeet.y@samsung.com>
---
arch/arm/mm/fault.c | 18 +++++++++---------
arch/arm64/mm/fault.c | 16 ++++++++--------
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 3a2e678..f92f90b 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -163,6 +163,15 @@ void show_pte(struct mm_struct *mm, unsigned long addr)
{
struct siginfo si;
+ tsk->thread.address = addr;
+ tsk->thread.error_code = fsr;
+ tsk->thread.trap_no = 14;
+ si.si_signo = sig;
+ si.si_errno = 0;
+ si.si_code = code;
+ si.si_addr = (void __user *)addr;
+ force_sig_info(sig, &si, tsk);
+
#ifdef CONFIG_DEBUG_USER
if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
((user_debug & UDBG_BUS) && (sig == SIGBUS))) {
@@ -172,15 +181,6 @@ void show_pte(struct mm_struct *mm, unsigned long addr)
show_regs(regs);
}
#endif
-
- tsk->thread.address = addr;
- tsk->thread.error_code = fsr;
- tsk->thread.trap_no = 14;
- si.si_signo = sig;
- si.si_errno = 0;
- si.si_code = code;
- si.si_addr = (void __user *)addr;
- force_sig_info(sig, &si, tsk);
}
void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index a78a5c4..eb5d0e3 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -197,14 +197,6 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
{
struct siginfo si;
- if (unhandled_signal(tsk, sig) && show_unhandled_signals_ratelimited()) {
- pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
- tsk->comm, task_pid_nr(tsk), fault_name(esr), sig,
- addr, esr);
- show_pte(tsk->mm, addr);
- show_regs(regs);
- }
-
tsk->thread.fault_address = addr;
tsk->thread.fault_code = esr;
si.si_signo = sig;
@@ -212,6 +204,14 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
si.si_code = code;
si.si_addr = (void __user *)addr;
force_sig_info(sig, &si, tsk);
+
+ if (unhandled_signal(tsk, sig) && show_unhandled_signals_ratelimited()) {
+ pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
+ tsk->comm, task_pid_nr(tsk), fault_name(esr), sig,
+ addr, esr);
+ show_pte(tsk->mm, addr);
+ show_regs(regs);
+ }
}
static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs)
--
1.9.1
^ permalink raw reply related
* [PATCH 1/1] mm: call force_sig_info before prints
From: Russell King - ARM Linux @ 2016-12-19 11:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1482143832-11158-1-git-send-email-maninder1.s@samsung.com>
On Mon, Dec 19, 2016 at 04:07:12PM +0530, Maninder Singh wrote:
> prints can delay queuing of signal, so better to print
> after force_sig_info.
>
> Let's say process generated SIGSEGV , and some other thread sends
> SIGKILL to crashing process and it gets queued before SIGSEGV becuase
> of little delay due to prints so in this case coredump might not generate.
In any case, that's going to be a race - you can't predict exactly when
the "other thread" will send the SIGKILL in relation to the SIGSEGV.
So, I don't see the point of this change.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
From: Markus Reichl @ 2016-12-19 11:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481375323-29724-2-git-send-email-pankaj.dubey@samsung.com>
Hi Pankaj,
tested your patches 1/8 and 2/8 + Javiers diff for verbose output:
https://www.spinics.net/lists/linux-samsung-soc/msg56576.html
on Odroid U3:
[ 0.080178] Exynos: CPU[UNKNOWN] CPU_REV[0x20] PKG_ID[0x602d058] AUX_INFO[0x0]
on Odroid X:
[ 0.080169] Exynos: CPU[UNKNOWN] CPU_REV[0x11] PKG_ID[0x1b0f6008] AUX_INFO[0x0]
XU4: [ 0.080039] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x1c04832a] AUX_INFO[0x43]
XU3: [ 0.080034] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x1604832a] AUX_INFO[0x43]
XU3-lite:[ 0.080033] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x5a12832a] AUX_INFO[0x13000054]
Regards,
--
Markus Reichl
Am 10.12.2016 um 14:08 schrieb Pankaj Dubey:
> Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
> This patch intends to provide initialization code for all these functionalities,
> at the same time it provides some sysfs entries for accessing these information
> to user-space.
>
> This driver uses existing binding for exynos-chipid.
>
> CC: Grant Likely <grant.likely@linaro.org>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> [m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/soc/samsung/Kconfig | 5 ++
> drivers/soc/samsung/Makefile | 1 +
> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 122 insertions(+)
> create mode 100644 drivers/soc/samsung/exynos-chipid.c
>
> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> index 2455339..f9ab858 100644
> --- a/drivers/soc/samsung/Kconfig
> +++ b/drivers/soc/samsung/Kconfig
> @@ -14,4 +14,9 @@ config EXYNOS_PM_DOMAINS
> bool "Exynos PM domains" if COMPILE_TEST
> depends on PM_GENERIC_DOMAINS || COMPILE_TEST
>
> +config EXYNOS_CHIPID
> + bool "Exynos Chipid controller driver" if COMPILE_TEST
> + depends on (ARM && ARCH_EXYNOS) || ((ARM || ARM64) && COMPILE_TEST)
> + select SOC_BUS
> +
> endif
> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
> index 3619f2e..2a8a85e 100644
> --- a/drivers/soc/samsung/Makefile
> +++ b/drivers/soc/samsung/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos3250-pmu.o exynos4-pmu.o \
> exynos5250-pmu.o exynos5420-pmu.o
> obj-$(CONFIG_EXYNOS_PM_DOMAINS) += pm_domains.o
> +obj-$(CONFIG_EXYNOS_CHIPID) += exynos-chipid.o
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> new file mode 100644
> index 0000000..cf0128b
> --- /dev/null
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -0,0 +1,116 @@
> +/*
> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com/
> + *
> + * EXYNOS - CHIP ID support
> + * Author: Pankaj Dubey <pankaj.dubey@samsung.com>
> + *
> + * This program 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.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/sys_soc.h>
> +
> +#define EXYNOS_SUBREV_MASK (0xF << 4)
> +#define EXYNOS_MAINREV_MASK (0xF << 0)
> +#define EXYNOS_REV_MASK (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
> +
> +static const struct exynos_soc_id {
> + const char *name;
> + unsigned int id;
> + unsigned int mask;
> +} soc_ids[] = {
> + { "EXYNOS3250", 0xE3472000, 0xFFFFF000 },
> + { "EXYNOS4210", 0x43200000, 0xFFFE0000 },
> + { "EXYNOS4212", 0x43220000, 0xFFFE0000 },
> + { "EXYNOS4412", 0xE4412000, 0xFFFE0000 },
> + { "EXYNOS5250", 0x43520000, 0xFFFFF000 },
> + { "EXYNOS5260", 0xE5260000, 0xFFFFF000 },
> + { "EXYNOS5410", 0xE5410000, 0xFFFFF000 },
> + { "EXYNOS5420", 0xE5420000, 0xFFFFF000 },
> + { "EXYNOS5440", 0xE5440000, 0xFFFFF000 },
> + { "EXYNOS5800", 0xE5422000, 0xFFFFF000 },
> + { "EXYNOS7420", 0xE7420000, 0xFFFFF000 },
> + { "EXYNOS5433", 0xE5433000, 0xFFFFF000 },
> +};
> +
> +static const char * __init product_id_to_soc_id(unsigned int product_id)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(soc_ids); i++)
> + if ((product_id & soc_ids[i].mask) == soc_ids[i].id)
> + return soc_ids[i].name;
> + return "UNKNOWN";
> +}
> +
> +static const struct of_device_id of_exynos_chipid_ids[] = {
> + {
> + .compatible = "samsung,exynos4210-chipid",
> + },
> + {},
> +};
> +
> +/**
> + * exynos_chipid_early_init: Early chipid initialization
> + */
> +int __init exynos_chipid_early_init(void)
> +{
> + struct soc_device_attribute *soc_dev_attr;
> + struct soc_device *soc_dev;
> + struct device_node *root;
> + struct device_node *np;
> + void __iomem *exynos_chipid_base;
> + const struct of_device_id *match;
> + u32 product_id;
> + u32 revision;
> +
> + np = of_find_matching_node_and_match(NULL,
> + of_exynos_chipid_ids, &match);
> + if (!np)
> + return -ENODEV;
> +
> + exynos_chipid_base = of_iomap(np, 0);
> +
> + if (!exynos_chipid_base)
> + return PTR_ERR(exynos_chipid_base);
> +
> + product_id = readl_relaxed(exynos_chipid_base);
> + revision = product_id & EXYNOS_REV_MASK;
> + iounmap(exynos_chipid_base);
> +
> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> + if (!soc_dev_attr)
> + return -ENODEV;
> +
> + soc_dev_attr->family = "Samsung Exynos";
> +
> + root = of_find_node_by_path("/");
> + of_property_read_string(root, "model", &soc_dev_attr->machine);
> + of_node_put(root);
> +
> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x", revision);
> + soc_dev_attr->soc_id = product_id_to_soc_id(product_id);
> +
> +
> + pr_info("Exynos: CPU[%s] CPU_REV[0x%x] Detected\n",
> + product_id_to_soc_id(product_id), revision);
> +
> + soc_dev = soc_device_register(soc_dev_attr);
> + if (IS_ERR(soc_dev)) {
> + kfree(soc_dev_attr->revision);
> + kfree_const(soc_dev_attr->soc_id);
> + kfree(soc_dev_attr);
> + return PTR_ERR(soc_dev);
> + }
> +
> + return 0;
> +}
> +early_initcall(exynos_chipid_early_init);
>
^ permalink raw reply
* [PATCH v2 02/11] mfd: axp20x: add volatile and writeable reg ranges for VBUS power supply driver
From: Quentin Schulz @ 2016-12-19 12:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v66jDsU2rt4d-0ydWR926y-hHTfOBJQmyYVdKd7rcm1WWQ@mail.gmail.com>
Hi Chen-Yu,
On 14/12/2016 16:43, Chen-Yu Tsai wrote:
> On Fri, Dec 9, 2016 at 7:04 PM, Quentin Schulz
> <quentin.schulz@free-electrons.com> wrote:
>> The X-Powers AXP20X and AXP22X PMICs allow to choose the maximum voltage
>> and minimum current delivered by the VBUS power supply.
>>
>> This adds the register used by the VBUS power supply driver to the range
>> of volatile and writeable regs ranges.
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>> ---
>>
>> added in v2
>>
>> drivers/mfd/axp20x.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index ba130be..6ee2cc6 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -65,12 +65,14 @@ static const struct regmap_access_table axp152_volatile_table = {
>>
>> static const struct regmap_range axp20x_writeable_ranges[] = {
>> regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
>> + regmap_reg_range(AXP20X_VBUS_IPSOUT_MGMT, AXP20X_VBUS_IPSOUT_MGMT),
>
> This is already covered by the previous entry.
>
>> regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES),
>> regmap_reg_range(AXP20X_RDC_H, AXP20X_OCV(AXP20X_OCV_MAX)),
>> };
>>
>> static const struct regmap_range axp20x_volatile_ranges[] = {
>> regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP20X_USB_OTG_STATUS),
>> + regmap_reg_range(AXP20X_VBUS_IPSOUT_MGMT, AXP20X_VBUS_IPSOUT_MGMT),
>
> And I'm not sure why you specify it as volatile? The PMIC doesn't change any
> of the bits in this register on its own.
>
I got things mixed up between the work on the battery driver I'll soon
send (which actually needs updated reg ranges) and this VBUS driver.
Sorry for that.
> Same for the AXP22x bits. So basically I think you don't need this patch.
>
Indeed. Should I send a v3 to remove this patch or is it fine for you to
ignore this one?
Thanks!
Quentin
--
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v6 3/8] PWM: add pwm-stm32 DT bindings
From: Lee Jones @ 2016-12-19 12:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_JsqL8pJSFb8LbABAYJOQ0URaMpyupbFryk_mS2ToN1kStdA@mail.gmail.com>
On Tue, 13 Dec 2016, Rob Herring wrote:
> On Tue, Dec 13, 2016 at 5:11 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 12 Dec 2016, Rob Herring wrote:
> >
> >> On Fri, Dec 09, 2016 at 03:15:14PM +0100, Benjamin Gaignard wrote:
> >> > Define bindings for pwm-stm32
> >> >
> >> > version 6:
> >> > - change st,breakinput parameter format to make it usuable on stm32f7 too.
> >> >
> >> > version 2:
> >> > - use parameters instead of compatible of handle the hardware configuration
> >> >
> >> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >> > ---
> >> > .../devicetree/bindings/pwm/pwm-stm32.txt | 33 ++++++++++++++++++++++
> >> > 1 file changed, 33 insertions(+)
> >> > create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> > new file mode 100644
> >> > index 0000000..866f222
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> > @@ -0,0 +1,33 @@
> >> > +STMicroelectronics STM32 Timers PWM bindings
> >> > +
> >> > +Must be a sub-node of an STM32 Timers device tree node.
> >> > +See ../mfd/stm32-timers.txt for details about the parent node.
> >> > +
> >> > +Required parameters:
> >> > +- compatible: Must be "st,stm32-pwm".
> >> > +- pinctrl-names: Set to "default".
> >> > +- pinctrl-0: List of phandles pointing to pin configuration nodes for PWM module.
> >> > + For Pinctrl properties see ../pinctrl/pinctrl-bindings.txt
> >> > +
> >> > +Optional parameters:
> >> > +- st,breakinput: Arrays of three u32 <index level filter> to describe break input configurations.
> >> > + "index" indicates on which break input the configuration should be applied.
> >> > + "level" gives the active level (0=low or 1=high) for this configuration.
> >> > + "filter" gives the filtering value to be applied.
> >> > +
> >> > +Example:
> >> > + timers at 40010000 {
> >>
> >> timer at ...
> >
> > No, it should be timers.
>
> Read the spec. "timer" is a generic node name. "timers" is not. How
> many is not relevant.
It's not the amount of timers that there are, it's the different types
of timers which this one IP contains.
In MFD we usually list the part number or IP name, however in this
case its difficult because the same IP doesn't have a specific name
listed, and provides; advanced, general purpose and basic timers, as
well as PWM functionality.
This IP is not a timer (although one of its children is one). It's
the parent device of many different types of timer.
timer {
timer {
};
pwm {
};
};
... looks weird.
"timer" is not right for the parent IP. Happy for you to provide an
alternative to "timers" though?
> > The 's' is intentional, since this parent (MFD) device houses 3
> > different types of timers. The "timer" node is a child of this one.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH] drm: tilcdc: simplify the recovery from sync lost error on rev1
From: Jyri Sarha @ 2016-12-19 13:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481641656-25227-1-git-send-email-bgolaszewski@baylibre.com>
One comment bellow.
On 12/13/16 17:07, Bartosz Golaszewski wrote:
> Revision 2 of LCDC suffers from an issue where a SYNC_LOST error
> caused by limited memory bandwidth may leave the picture shifted a
> couple pixels to the right.
>
> This issue has not been observed on revision 1, while the recovery
> mechanism introduces a different issue, where the END_OF_FRAME
> interrupt doesn't fire while drm is waiting for vblanks.
>
> On rev1: recover from sync lost errors by simply clearing the
> RASTER_ENABLE bit in the RASTER_CTRL register and re-enabling it
> again as is suggested by the datasheet.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 24 +++++++++++++++---------
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 9942b05..70e57a7 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -921,17 +921,23 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
> dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost",
> __func__, stat);
> tilcdc_crtc->frame_intact = false;
> - if (tilcdc_crtc->sync_lost_count++ >
> - SYNC_LOST_COUNT_LIMIT) {
> - dev_err(dev->dev, "%s(0x%08x): Sync lost flood detected, recovering", __func__, stat);
> - queue_work(system_wq, &tilcdc_crtc->recover_work);
> - if (priv->rev == 1)
> - tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
> - LCDC_V1_SYNC_LOST_INT_ENA);
> - else
> + if (priv->rev == 1) {
I would add here:
+ if ((tilcdc_read(dev, LCDC_RASTER_CTRL_REG) &
+ LCDC_RASTER_ENABLE)) {
> + tilcdc_clear(dev,
> + LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
> + tilcdc_set(dev,
> + LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+ }
Just in case the interrupt is for some reason handled right after the
crtc is disabled.
With this addition I could send a pull request for this fix still today,
if you agree with the change.
> + } else {
> + if (tilcdc_crtc->sync_lost_count++ >
> + SYNC_LOST_COUNT_LIMIT) {
> + dev_err(dev->dev,
> + "%s(0x%08x): Sync lost flood detected, recovering",
> + __func__, stat);
> + queue_work(system_wq,
> + &tilcdc_crtc->recover_work);
> tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
> LCDC_SYNC_LOST);
> - tilcdc_crtc->sync_lost_count = 0;
> + tilcdc_crtc->sync_lost_count = 0;
> + }
> }
> }
>
>
^ permalink raw reply
* [PATCH v8 0/8] Introducing Exynos ChipId driver
From: pankaj.dubey @ 2016-12-19 13:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7504d7e6-4f1e-52c9-a526-33af47ab5d99@samsung.com>
Hi Marek,
On Friday 16 December 2016 06:41 PM, Marek Szyprowski wrote:
> Hi Pankaj
>
>
> On 2016-12-10 14:08, Pankaj Dubey wrote:
>> Each Exynos SoC has ChipID block which can give information about SoC's
>> product Id and revision number.
>>
>> This patch series introduces Exynos Chipid SoC driver. At the same time
>> it reduces dependency of mach-exynos files from plat-samsung, by removing
>> soc_is_exynosMMMM and samsung_rev API. Instead of it now we can use
>> soc_device_match API proposed by Arnd and getting discussed in thread
>> [1].
>>
>> Until now we are using static mapping of Exynos Chipid and using this
>> static
>> mapping to know about SoC name and revision via soc_is_exynosMMMM
>> macro. This
>> is quite cumbersome and every time new ARMv7 based Exynos SoC supports
>> lands in
>> mainline a bunch of such new macros needs to be added. Quite long back
>> during
>> support of Exynos5260 it has been discussed to solve this problem.
>>
>> To solve this issue this patchset replaces use of soc_is_exynosMMMM by
>> either
>> of_machine_is_compatible or soc_device_match depending upon usecase.
>>
>> I have tested this patch series on Exynos4210 based Origen board for
>> normal SMP
>> boot.
>>
>>
>> Although I submitted this series as a whole of 8 patchsets, following
>> are dependency
>> details among the patches.
>>
>> Patch 1/8 can be taken without any dependency on other patches.
>> Patch 2/8 and 3/8 has dependency on 1/8 and can be taken along with 1/8.
>> Patch 4/8 has no dependency and can be taken without chipid driver
>> patch 1/8.
>> Patch 5/8 has depency on 1/8
>> Patch 6/8 has no dependency and can be taken without any other patches.
>> Patch 7/8 has dependency on 6/8 and 1/8
>> Patch 8/8 has dependency on rest of patches
>
> Which kernel should I use as a base for applying those patches? I wanted
> to test them,
> but I got conflicts both for v4.9 and current linux-next (next-20161216).
>
Sorry for late reply.
Actually these patches were created on top of Krzysztof's for-next
dated: 05/11/2016, I can think for because of following patches [1] and
[2] you might be getting conflict in latest tree. So if possible please
apply following patches on top of Krzysztof's current for-next and then try
[1]: https://patchwork.kernel.org/patch/9421017/
[2]: https://patchwork.kernel.org/patch/9419099/
Hopefully all chipid patches will get applied on top of it. Otherwise I
will be posting v9 very soon after addressing all review comments of v8.
So please test on v9.
Thanks,
Pankaj Dubey
^ permalink raw reply
* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
From: pankaj.dubey @ 2016-12-19 13:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ceba534d-108d-9ac4-d29c-c2a55af49849@fivetechno.de>
Hi Markus,
On Monday 19 December 2016 05:29 PM, Markus Reichl wrote:
> Hi Pankaj,
>
> tested your patches 1/8 and 2/8 + Javiers diff for verbose output:
> https://www.spinics.net/lists/linux-samsung-soc/msg56576.html
>
> on Odroid U3:
> [ 0.080178] Exynos: CPU[UNKNOWN] CPU_REV[0x20] PKG_ID[0x602d058] AUX_INFO[0x0]
> on Odroid X:
> [ 0.080169] Exynos: CPU[UNKNOWN] CPU_REV[0x11] PKG_ID[0x1b0f6008] AUX_INFO[0x0]
>
> XU4: [ 0.080039] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x1c04832a] AUX_INFO[0x43]
> XU3: [ 0.080034] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x1604832a] AUX_INFO[0x43]
> XU3-lite:[ 0.080033] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x5a12832a] AUX_INFO[0x13000054]
>
> Regards,
> --
> Markus Reichl
Thanks for testing and letting us know.
Hopefully this driver will be of more use in near future, as Javiers
pointed out it can help us to select better ASV tables based on PKG_ID
and AUX_INFO. Even though these fields are not part of the driver in my
patch, it can be extended as per need.
Thanks,
Pankaj Dubey
^ permalink raw reply
* [PATCH] ARM: dts: Add missing CPU frequencies for Exynos5422/5800
From: Alim Akhtar @ 2016-12-19 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216073720.GA3489@kozik-lap>
Hi,
On 12/16/2016 01:07 PM, Krzysztof Kozlowski wrote:
> On Thu, Dec 15, 2016 at 04:52:58PM -0800, Doug Anderson wrote:
>>> [ I added Arjun to Cc:, maybe he can help in explaining this issue
>>> (unfortunately Inderpal's email is no longer working). ]
>>>
>>> Please also note that on Exynos5422/5800 SoCs the same ARM rail
>>> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
>>> IOW if the problem exists it is already present in the mainline
>>> kernel.
>>
>> Interesting. In the ChromeOS tree I see significantly higher voltages
>> needed... Note that one might naively look at
>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#178>.
>>
>> 1362500, /* L0 2100 */
>> 1312500, /* L1 2000 */
>>
>> ..but, amazingly enough those voltages aren't used at all. Surprise!
>>
>> I believe that the above numbers are actually not used and the ASV
>> numbers are used instead. See
>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/mach-exynos/include/mach/asv-exynos542x.h#452>
>>
>> { 2100000,
>> 1350000, 1350000, 1350000, 1350000, 1350000,
>> 1337500, 1325000, 1312500, 1300000, 1287500,
>> 1275000, 1262500, 1250000, 1237500 },
>>
>> I believe that interpretation there is: some bins of the CPU can run
>> at 2.1 GHz just fine at 1.25 V but others need up to 1.35V.
>
> That is definitely the case. One could just look at vendors ASV table
> (for 1.9 GHz):
> { 1900000, 1300000, 1287500, 1262500, 1237500, 1225000, 1212500,
> 1200000, 1187500, 1175000, 1162500, 1150000,
> 1137500, 1125000, 1112500, 1112500},
>
> The theoretical difference is up to 1.875V! From my experiments I saw
> BIN1 chips which should be the same... but some working on 1.2V, some on
> 1.225V (@1.9 GHz). I didn't see any requiring higher voltages but that
> does not mean that there aren't such...
>
>> ...so if you're running at 2.1 GHz at 1.25V then perhaps you're just
>> running on a CPU from a nice bin?
>
> Would be nice to see a dump of PKG_ID and AUX_INFO chipid registers
> along with name of tested board. Because the "Tested on XU3" is not
> sufficient.
>
I agree, we should be dumping PKG_ID and other chip info to know on
which BIN sample this patch is tested on...
As far as Peach-{pit/pi} boards are concerns, this is what I can remember:
1> 5420 (PIT) -> max recommended target frequency is 1800 MHz for A15
2> 5800 (PI)-> max recommended target frequency can go upto 2000 MHz,
with INT rail locking.
INT rail locking schemes never made to mainline, so to be safer side
instead of bumping the clock and voltages better to keep it at safer
range for pit and pi, probably thats why it was kept at 1800MHz.
I am not sure if the same limitation applies to Odroid-XU3 samples.
> Best regards,
> Krzysztof
>
>
>
^ permalink raw reply
* [RFC] Question about freeing of resource in 'atlas7_pinmux_probe()', in file 'drivers/pinctrl/sirf/pinctrl-atlas7.c'
From: Vladimir Zapolskiy @ 2016-12-19 13:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <612eedc5-0f28-7a38-abad-4b9573208875@wanadoo.fr>
Hi,
On 12/19/2016 08:19 AM, Marion & Christophe JAILLET wrote:
> Hi,
>
> while playing with coccinelle, a missing 'of_node_put()' triggered in
> 'atlas7_pinmux_probe()', in file 'drivers/pinctrl/sirf/pinctrl-atlas7.c'.
>
> /* The sd3 and sd9 shared all pins, and the function select by
> * SYS2PCI_SDIO9SEL register
> */
> sys2pci_np = of_find_node_by_name(NULL, "sys2pci");
> if (!sys2pci_np)
> return -EINVAL;
> ret = of_address_to_resource(sys2pci_np, 0, &res);
> if (ret) <------------- missing of_node_put(sys2pci_np);
> return ret;
> pmx->sys2pci_base = devm_ioremap_resource(&pdev->dev, &res);
> if (IS_ERR(pmx->sys2pci_base)) {
> of_node_put(sys2pci_np); <------------- added by commit
> 151b8c5ba1eb
> return -ENOMEM;
> }
>
> Looking at the history of this file, I found a recent commit that added
> another missing of_node_put (see above).
>
>
> Adding missing 'of_node_put()' in error handling paths is fine, but in
> this particular case, I was wondering if one was not also missing in the
> normal path?
Right, in this particular case 'of_node_put()' should be both on error
and normal paths.
>
> In such a case, I would revert 151b8c5ba1eb and propose something like:
> /* The sd3 and sd9 shared all pins, and the function select by
> * SYS2PCI_SDIO9SEL register
> */
> sys2pci_np = of_find_node_by_name(NULL, "sys2pci");
> if (!sys2pci_np)
> return -EINVAL;
> ret = of_address_to_resource(sys2pci_np, 0, &res);
> if (ret) {
> of_node_put(sys2pci_np);
> return ret;
> }
> of_node_put(sys2pci_np);
Functionally it looks good, I have two comments though.
1) you don't need to revert 151b8c5ba1eb, the commit is a proper fix
per se but incomplete, please add your change on top of it,
2) minimizing the lines of code by removing duplicates is always good,
so here a better and complete fix will be like the following one:
sys2pci_np = of_find_node_by_name(NULL, "sys2pci");
if (!sys2pci_np)
return -EINVAL;
ret = of_address_to_resource(sys2pci_np, 0, &res);
of_node_put(sys2pci_np);
if (ret)
return ret;
pmx->sys2pci_base = devm_ioremap_resource(&pdev->dev, &res);
if (IS_ERR(pmx->sys2pci_base))
return -ENOMEM;
>
> pmx->sys2pci_base = devm_ioremap_resource(&pdev->dev, &res);
> if (IS_ERR(pmx->sys2pci_base))
> return -ENOMEM;
--
With best wishes,
Vladimir
^ permalink raw reply
* [PATCH v2] ARM: dts: sun8i: add opp-v2 table for A33
From: Icenowy Zheng @ 2016-12-19 14:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v65YWntCrFHtSbxhtTWkt+UwkW3xoY-ae26MpH+-ULLXSQ@mail.gmail.com>
19.12.2016, 16:54, "Chen-Yu Tsai" <wens@csie.org>:
> On Mon, Dec 19, 2016 at 4:46 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> ?On Fri, Dec 16, 2016 at 02:27:54AM +0800, Icenowy Zheng wrote:
>>> ?An operating point table is needed for the cpu frequency adjusting to
>>> ?work.
>>>
>>> ?The operating point table is converted from the common value in
>>> ?extracted script.fex from many A33 board/tablets.
>>>
>>> ?Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>>> ?---
>>> ?Changes since v1:
>>> ?- Fix format problem (blank lines).
>>> ?- Removed the 1.344GHz operating point, as it's overvoltage and overclocked.
>>>
>>> ?This patch depends on the following patchset:
>>>
>>> ?http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/473962.html
>>>
>>> ?It's the v2 of the [PATCH 4/6] in this patchset.
>>>
>>> ?I think this operating point table may also apply to A23, as there's no
>>> ?difference except the points over 1.2GHz between A23 and A33's stock dvfs table.
>>>
>>> ?But as A23 CCU may not have the necessary fixes, I won't add the table to A23
>>> ?now.
>>>
>>> ?Chen-Yu, could you test the CCU fixes I described in the patchset above on A23,
>>> ?then test this operating points table?
>>>
>>> ?If it's necessary, you can send out the CCU fixes and add one more patch that
>>> ?moves this opp-v2 table to sun8i-a23-a33.dtsi .
>>>
>>> ??arch/arm/boot/dts/sun8i-a33.dtsi | 35 +++++++++++++++++++++++++++++++++++
>>> ??1 file changed, 35 insertions(+)
>>>
>>> ?diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
>>> ?index 504996cbee29..0f5b2af72981 100644
>>> ?--- a/arch/arm/boot/dts/sun8i-a33.dtsi
>>> ?+++ b/arch/arm/boot/dts/sun8i-a33.dtsi
>>> ?@@ -46,7 +46,42 @@
>>> ??#include <dt-bindings/dma/sun4i-a10.h>
>>>
>>> ??/ {
>>> ?+ cpu0_opp_table: opp_table0 {
>>> ?+ compatible = "operating-points-v2";
>>> ?+ opp-shared;
>>> ?+
>>> ?+ opp at 648000000 {
>>> ?+ opp-hz = /bits/ 64 <648000000>;
>>> ?+ opp-microvolt = <1040000>;
>>> ?+ clock-latency-ns = <244144>; /* 8 32k periods */
>>> ?+ };
>>> ?+
>>> ?+ opp at 816000000 {
>>> ?+ opp-hz = /bits/ 64 <816000000>;
>>> ?+ opp-microvolt = <1100000>;
>>> ?+ clock-latency-ns = <244144>; /* 8 32k periods */
>>> ?+ };
>>> ?+
>>> ?+ opp at 1008000000 {
>>> ?+ opp-hz = /bits/ 64 <1008000000>;
>>> ?+ opp-microvolt = <1200000>;
>>> ?+ clock-latency-ns = <244144>; /* 8 32k periods */
>>> ?+ };
>>> ?+
>>> ?+ opp at 1200000000 {
>>> ?+ opp-hz = /bits/ 64 <1200000000>;
>>> ?+ opp-microvolt = <1320000>;
>>> ?+ clock-latency-ns = <244144>; /* 8 32k periods */
>>> ?+ };
>>> ?+ };
>>> ?+
>>> ???????cpus {
>>> ?+ cpu0: cpu at 0 {
>>
>> ?There's no need to duplicate the label here. I removed it and applied.
>
> I think using the label to directly reference cpu0 would be better,
> instead of duplicating the cpu at 0 block.
After proper testing of A23 ccu, the operating points can also apply to A23.
(According to A23 devices' fex)
>
> ChenYu
^ permalink raw reply
* [PATCH] ARM: dts: sun8i-q8-common: enable bluetooth on SDIO Wi-Fi
From: Icenowy Zheng @ 2016-12-19 14:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161219100952.bspym36nsehda3za@lukather>
19.12.2016, 18:09, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> On Fri, Dec 16, 2016 at 10:40:00PM +0800, Icenowy Zheng wrote:
>> ?>> ?> >? &r_pio {
>> ?>> ?> >? wifi_pwrseq_pin_q8: wifi_pwrseq_pin at 0 {
>> ?>> ?> > - pins = "PL6", "PL7", "PL11";
>> ?>> ?> > + pins = "PL6", "PL7", "PL8", "PL11";
>> ?>> ?> >? function = "gpio_in";
>> ?>> ?> >? bias-pull-up;
>> ?>> ?> >? };
>> ?>> ?>
>> ?>> ?> There's several things wrong here. The first one is that you rely
>> ?>> ?> solely on the pinctrl state to maintain a reset line. This is very
>> ?>> ?> fragile (especially since the GPIO pinctrl state are likely to go away
>> ?>> ?> at some point), but it also means that if your driver wants to recover
>> ?>> ?> from that situation at some point, it won't work.
>> ?>> ?>
>> ?>> ?> The other one is that the bluetooth and wifi chips are two devices in
>> ?>> ?> linux, and you assign that pin to the wrong device (wifi).
>> ?>> ?>
>> ?>> ?> rfkill-gpio is made just for that, so please use it.
>> ?>>
>> ?>> ?The GPIO is not for the radio, but for the full Bluetooth part.
>> ?>
>> ?> I know.
>> ?>
>> ?>> ?If it's set to 0, then the bluetooth part will reset, and the
>> ?>> ?hciattach will fail.
>> ?>
>> ?> Both rfkill-gpio and rfkill-regulator will shutdown when called
>> ?> (either by poking the reset pin or shutting down the regulator), so
>> ?> that definitely seems like an expected behavior to put the device in
>> ?> reset.
>> ?>
>> ?>> ?The BSP uses this as a rfkill, and the result is that the bluetooth
>> ?>> ?on/off switch do not work properly.
>> ?>
>> ?> Then rfkill needs fixing, but working around it by hoping that the
>> ?> core will probe an entirely different device, and enforcing a default
>> ?> that the rest of the kernel might or might not change is both fragile
>> ?> and wrong.
>>
>> ?I think a rfkill-gpio here works just like the BSP rfkill...
>>
>> ?The real problem is that the Realtek UART bluetooth driver is a userspace
>> ?program (a modified hciattach), which is not capable of the GPIO reset...
>
> Can't you run rfkill before attaching? What is the problem exactly?
> It's not in reset for long enough?
>
> This seems more and more like an issue in the BT stack you're
> using. We might consider workarounds in the kernel, but they have to
> be correct.
One more rfkill interface will be generated for hci0 after hciattach, which can
be safely toggled block and unblock.
However, if the GPIO is toggled down, the hciattach program will die.
The bluetooth stack I used is fd.o's BlueZ.
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
^ permalink raw reply
* [PATCH 0/4] Add support for the ethernet switch on the EXPRESSObin
From: Romain Perier @ 2016-12-19 14:16 UTC (permalink / raw)
To: linux-arm-kernel
This set of patches adds support for the Marvell ethernet switch 88E6341.
It also add the devicetree definition of thid switch to the DT board.
Romain Perier (4):
net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address
0x1
net: dsa: mv88e6xxx: Don't forbid MDIO I/Os for PHY addr >=
num_of_ports
net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341/88E6141
arm64: dts: marvell: Add ethernet switch definition for the
EXPRESSObin
.../boot/dts/marvell/armada-3720-espressobin.dts | 67 ++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/chip.c | 24 ++++----
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 4 +-
3 files changed, 84 insertions(+), 11 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH 1/4] net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address 0x1
From: Romain Perier @ 2016-12-19 14:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161219141610.30934-1-romain.perier@free-electrons.com>
Currently, the function mv88e6xxx_smi_init() returns -EINVAL if the bit
zero of sw_addr is 0x1. However, on some platforms, ethernet switches
are configured in Multi chip addressing mode and available at MDIO
address 0x1.
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index f7222dc..b5f0e1e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4240,10 +4240,6 @@ static void mv88e6xxx_phy_destroy(struct mv88e6xxx_chip *chip)
static int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
struct mii_bus *bus, int sw_addr)
{
- /* ADDR[0] pin is unavailable externally and considered zero */
- if (sw_addr & 0x1)
- return -EINVAL;
-
if (sw_addr == 0)
chip->smi_ops = &mv88e6xxx_smi_single_chip_ops;
else if (mv88e6xxx_has(chip, MV88E6XXX_FLAGS_MULTI_CHIP))
--
2.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox