Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V7 4/4] dmaengine: qcom_hidma: add MSI support for interrupts
From: okaya at codeaurora.org @ 2016-10-24 11:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAHp75VettN=-S+iqKQbqixRe2BM=AiS26VGM+8yLophbkc+Z8g@mail.gmail.com>

On 2016-10-24 03:30, Andy Shevchenko wrote:
> On Mon, Oct 24, 2016 at 5:55 AM, Sinan Kaya <okaya@codeaurora.org> 
> wrote:
>> On 10/21/2016 12:11 PM, Andy Shevchenko wrote:
>>>> +static void hidma_free_msis(struct hidma_dev *dmadev)
>>>> > +{
>>>> > +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
>>> Perhaps one #ifdef and two definitions of functions?
>> 
>> I don't think it will make a difference. I'll have to move
>> #ifdef around the caller of hidma_free_msis instead which
>> I think is uglier.
>> 
>> The hidma_write_msi_msg function gets called only when
>> CONFIG_GENERIC_MSI_IRQ_DOMAIN is defined. If I don't put
>> this around the function definition, I get unused function
>> warning from the compiler. This is the reason why preprocessor
>> definition is outside of the function definition.
> 
> I am talking about something like below:
> 
> #ifdef UGLY_DEFINE
> myfunc_a()
> {
> }
> 
> myfunc_b()
> {
> }
> #else
> static inline myfunc_a() {}
> static inline myfunc_b() {}
> #endif
> 
> 
> There is another way as well, namely use of IS_ENABLED(), IS_BUILTIN()
> macros (I don't remember how exactly second one is spelt).


This was my initial approach. I was asked to remove the stub functions. 
So, I did it.

^ permalink raw reply

* [PATCH] iommu: arm-smmu-v2: Enable 16 bit ASID
From: Robin Murphy @ 2016-10-24 11:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477112524-13023-1-git-send-email-gakula@caviumnetworks.com>

Hi Geetha,

On 22/10/16 06:02, Geetha sowjanya wrote:
>  Support up to 16 bit ASID. This patch enables
>  16 bit ASID when supported.

Why? "Because we can" is not a compelling rationale. I assume this is
actually related to the globally-unique ASID workaround for the TLB
sharing erratum, but if so that should be called out explicitly -
describe the problem being fixed.

> Signed-off-by: Geetha sowjanya <gakula@cavium.com>
> Acked-by: Tirumalesh Chalamarla <Tirumalesh.Chalamarla@cavium.com>
> ---
>  drivers/iommu/arm-smmu.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index c841eb7..9740846 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -258,6 +258,9 @@ enum arm_smmu_s2cr_privcfg {
>  #define TTBCR2_SEP_SHIFT		15
>  #define TTBCR2_SEP_UPSTREAM		(0x7 << TTBCR2_SEP_SHIFT)
>  
> +#define TTBCR2_AS_SHIFT			4
> +#define TTBCR2_AS_ENABLE		(1 << TTBCR2_AS_SHIFT)
> +

The style here for single-bit fields is simply:

#define TTBCR2_AS			(1 << 4)

>  #define TTBRn_ASID_SHIFT		48
>  
>  #define FSR_MULTI			(1 << 31)
> @@ -773,6 +776,9 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
>  			reg = pgtbl_cfg->arm_lpae_s1_cfg.tcr;
>  			reg2 = pgtbl_cfg->arm_lpae_s1_cfg.tcr >> 32;
>  			reg2 |= TTBCR2_SEP_UPSTREAM;
> +		/* Enable 16 bit ASID if 16 bit VMID is supported */
> +			if (smmu->features & ARM_SMMU_FEAT_VMID16)

This should be a check for AArch64 context format - VMID16 has nothing
to do with it. 16-bit ASID support for stage 1 AArch64 contexts is
already implied by SMMUv2 (TCR2.AS is not optional).

Robin.

> +				reg |= TTBCR2_AS_ENABLE;
>  		}
>  		if (smmu->version > ARM_SMMU_V1)
>  			writel_relaxed(reg2, cb_base + ARM_SMMU_CB_TTBCR2);
> 

^ permalink raw reply

* [PATCH] ARM: OMAP2+: PRM: initialize en_uart4_mask and grpsel_uart4_mask
From: Colin King @ 2016-10-24 11:00 UTC (permalink / raw)
  To: linux-arm-kernel

From: Colin Ian King <colin.king@canonical.com>

In the case where has_uart4 is false, en_uart4_mask and grpsel_uart4_mask
are not initialized and so any garbage value is being logically or'd into
the write of PM_WKEN and OMAP3430_PM_MPUGRPSEL.  Fix this by initializing
these masks to zero.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 arch/arm/mach-omap2/prm3xxx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-omap2/prm3xxx.c b/arch/arm/mach-omap2/prm3xxx.c
index 62680aa..718981b 100644
--- a/arch/arm/mach-omap2/prm3xxx.c
+++ b/arch/arm/mach-omap2/prm3xxx.c
@@ -319,6 +319,9 @@ void __init omap3_prm_init_pm(bool has_uart4, bool has_iva)
 	if (has_uart4) {
 		en_uart4_mask = OMAP3630_EN_UART4_MASK;
 		grpsel_uart4_mask = OMAP3630_GRPSEL_UART4_MASK;
+	} else {
+		en_uart4_mask = 0;
+		grpsel_uart4_mask = 0;
 	}
 
 	/* Enable wakeups in PER */
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 3/9] vcodec: mediatek: Add Mediatek V4L2 Video Decoder Driver
From: Tiffany Lin @ 2016-10-24 10:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161024070542.65e11e37@vento.lan>

Hi Mauro,

On Mon, 2016-10-24 at 07:05 -0200, Mauro Carvalho Chehab wrote:
> Em Mon, 24 Oct 2016 11:22:08 +0800
> Tiffany Lin <tiffany.lin@mediatek.com> escreveu:
> 
> > Hi Mauro,
> > 
> > On Fri, 2016-10-21 at 11:01 -0200, Mauro Carvalho Chehab wrote:
> > > Em Fri, 2 Sep 2016 20:19:54 +0800
> > > Tiffany Lin <tiffany.lin@mediatek.com> escreveu:
> > >   
> > > > Add v4l2 layer decoder driver for MT8173
> > > > 
> > > > Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>  
> > >   
> > > > +int vdec_if_init(struct mtk_vcodec_ctx *ctx, unsigned int fourcc)
> > > > +{
> > > > +	int ret = 0;
> > > > +
> > > > +	switch (fourcc) {
> > > > +	case V4L2_PIX_FMT_H264:
> > > > +	case V4L2_PIX_FMT_VP8:
> > > > +	default:
> > > > +		return -EINVAL;
> > > > +	}  
> > > 
> > > Did you ever test this driver? The above code will *always* return
> > > -EINVAL, with will cause vidioc_vdec_s_fmt() to always fail!
> > > 
> > > I suspect that what you wanted to do, instead, is:
> > > 
> > > 	switch (fourcc) {
> > > 	case V4L2_PIX_FMT_H264:
> > > 	case V4L2_PIX_FMT_VP8:
> > > 		break;
> > > 	default:
> > > 		return -EINVAL;
> > >   
> > 
> > The original idea here is that vp8 and h264 are added in later patches.
> > If get this patch without later patches, it should return -EINVAL.
> 
> I noticed your idea, but next time, don't add dead code like that.
> Reviewers check patch by patch at the order they're present at the
> patch series.
> 
> So, don't add something broken by purpose, assuming that it would
> be fixed later.
> 
Got it.
> > 
> > 
> > > Btw, this patch series has also several issues that were pointed by
> > > checkpatch. Please *always* run checkpatch when submitting your work.
> > > 
> > > You should take a look at the Kernel documentation about how to
> > > submit patches, at:
> > > 	https://mchehab.fedorapeople.org/kernel_docs/process/index.html
> > > 
> > > PS.: this time, I fixed the checkpatch issues for you. So, let me know
> > > if the patch below is OK, and I'll merge it at media upstream,
> > > assuming that the other patches in this series are ok.
> > >   
> > 
> > I did run checkpatch, but I don't know why these issues missed.
> > probably I run checkpatch for all files not for patches.
> > I will take a look at the documentation and keep this in mind for future
> > upstream.
> > Appreciated for your help.
> 
> Checkpatch should be run patch by patch, as we expect that all patches
> will follow the coding style and will compile fine, without introducing
> warnings.
> 
> I do compile the Kernel for every single patch I merge.
> 
Got it. I will follow this.

best regards,
Tiffany

> Regards,
> Mauro

^ permalink raw reply

* [PATCH] mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading
From: Wolfram Sang @ 2016-10-24 10:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161024103428.GA8574@dell>


> I was under the impression it was all but ready.

Then, I would have applied it.

> What are you waiting on?

Lee, I don't want to explain it *again*. Please re-read Kieran's last
attempt.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161024/241ed28c/attachment.sig>

^ permalink raw reply

* [PATCH] mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading
From: Lee Jones @ 2016-10-24 10:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161024101659.GA1440@katana>

On Mon, 24 Oct 2016, Wolfram Sang wrote:

> 
> > > > Even though axp20x-i2c seems to be the only "DT only" i2c client,
> > > > would it make sense to add DT-based module autoloading to the i2c
> > > > core?
> > > 
> > > If it is not too invasive, then yes that would be a sensible addition IMHO.
> > 
> > If I understand you correctly, I already have a patch-set on the ML
> > that does this.
> 
> Incomplete, though. But we will see if Kieran finds time somewhen...

I was under the impression it was all but ready.

What are you waiting on?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH] kernel: irq: fix build failure
From: Lee Jones @ 2016-10-24 10:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.20.1610211006370.4797@nanos>

On Fri, 21 Oct 2016, Thomas Gleixner wrote:

> On Fri, 21 Oct 2016, Stephen Rothwell wrote:
> > On Thu, 20 Oct 2016 14:55:45 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote:
> > > I know. This is under discussion with the driver folks as we are not going
> > > to blindly export stuff just because someone slapped a irq_set_parent()
> > > into the code w/o knowing why.
> > 
> > Do we have any idea if a resolution is close.  This was first reported
> > in linux-next in September 14/15.  :-(
> 
> Grr. Yes. As much as I hate it, I'll go and export it for now. Should be
> able to get it into rc2.

Did this get in?  I still have people complaining about it.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Question about arm64 earlycon
From: Marc Zyngier @ 2016-10-24 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4966376.bRx4fgSjbT@wuerfel>

On 24/10/16 11:06, Arnd Bergmann wrote:
> On Sunday, October 23, 2016 12:26:59 AM CEST Duc Dang wrote:
>> Hi Catalin, Marc, Mark, Arnd,
>>
>> I am testing with 3.12 kernel with earlyprintk enabled and I see some
>> garbage characters in the console log right before the message
>> indicating that the real console device is initialized:
>>
>> <some garbage characters here>01c020000.serial: ttyS0 at MMIO
>> 0x1c020000 (irq = 108, base_baud = 3125000) is a U6_16550A
>> console [ttyS0] enabled, bootconsole disabled

Well, you get two independent, unsynchronized bits of code writing to
the same peripheral. Things are bound to go badly. Unless you're trying
to debug things, do not use earlyprintk in production.

>>
>> I looked through early_prink.c file and printk.c file and it looks
>> like there is case that some early boot code can touch the UART
>> hardware via ealy console driver while the 'real' console driver is
>> setting up the same UART port? Please let me know if I missed some
>> important piece of code that can prevent this.
> 
> I don't think we every supported earlyprintk on arm64, and
> earlycon support may have been added later.

We did support some form of earlyprintk for a while (though not in the
same way as 32bit ARM does), until Rob introduced earlycon.

> If you can't use a modern kernel, try backporting all the
> relevant earlycon changes.

Agreed. That's the most sensible course of action.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH] mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading
From: Wolfram Sang @ 2016-10-24 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161024100938.GI14477@dell>


> > > Even though axp20x-i2c seems to be the only "DT only" i2c client,
> > > would it make sense to add DT-based module autoloading to the i2c
> > > core?
> > 
> > If it is not too invasive, then yes that would be a sensible addition IMHO.
> 
> If I understand you correctly, I already have a patch-set on the ML
> that does this.

Incomplete, though. But we will see if Kieran finds time somewhen...

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161024/f6b74cf4/attachment.sig>

^ permalink raw reply

* [PATCH 3/3] arm64: arch timer: Add timer erratum property for Hip05-d02 and Hip06-d03
From: Marc Zyngier @ 2016-10-24 10:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <bf391642-82f7-2a3e-3118-1a8384a133e5@huawei.com>

Please write a proper commit message, no matter how trivial the patch is.

Thanks,

	M.

On 23/10/16 04:21, Ding Tianhong wrote:
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
>  arch/arm64/boot/dts/hisilicon/hip05.dtsi | 1 +
>  arch/arm64/boot/dts/hisilicon/hip06.dtsi | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/hisilicon/hip05.dtsi b/arch/arm64/boot/dts/hisilicon/hip05.dtsi
> index bf322ed..9d18875 100644
> --- a/arch/arm64/boot/dts/hisilicon/hip05.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hip05.dtsi
> @@ -281,6 +281,7 @@
>  			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
>  			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
>  			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> +		hisilicon,erratum-161x01;
>  	};
> 
>  	pmu {
> diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi b/arch/arm64/boot/dts/hisilicon/hip06.dtsi
> index 5927bc4..c38c658 100644
> --- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi
> @@ -260,6 +260,7 @@
>  			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
>  			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
>  			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> +		hisilicon,erratum-161x01;
>  	};
> 
>  	pmu {
> 


-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH 3/5] ARM: multi_v7_defconfig: Remove CONFIG_ST_THERMAL_MEMMAP Kconfig symbol
From: Peter Griffin @ 2016-10-24 10:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477065443-10668-4-git-send-email-patrice.chotard@st.com>

Hi Patrice,

On Fri, 21 Oct 2016, patrice.chotard at st.com wrote:

> From: Patrice Chotard <patrice.chotard@st.com>
> 
> Driver code has been already removed, see
> http://www.spinics.net/lists/devicetree/msg143322.html
> Remove the multi_v7_defconfig part
> 
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> Cc: <rui.zhang@intel.com>
> Cc: <edubezval@gmail.com>
> ---
>  arch/arm/configs/multi_v7_defconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index 15b2f99..45e252b 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -450,7 +450,6 @@ CONFIG_RCAR_THERMAL=y
>  CONFIG_ARMADA_THERMAL=y
>  CONFIG_DAVINCI_WATCHDOG=m
>  CONFIG_EXYNOS_THERMAL=m
> -CONFIG_ST_THERMAL_MEMMAP=y

Are you sure you want to remove this?

This Kconfig symbol is still used for the stih407 family
thermal parts in drivers/thermal/st/st_thermal_memmap.c?

My patch only removed the stih415/6 parts from the driver.

regards,

Peter.

^ permalink raw reply

* [PATCH 2/3] arm64: arch_timer: Work around QorIQ Erratum Hisilicon-161x01
From: Marc Zyngier @ 2016-10-24 10:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <07862fac-b32a-9246-c01f-fc3e55635f54@huawei.com>

On 23/10/16 04:21, Ding Tianhong wrote:
> Erratum Hisilicon-161x01 says that the ARM generic timer counter "has the
> potential to contain an erroneous value when the timer value changes".
> Accesses to TVAL (both read and write) are also affected due to the implicit counter
> read.  Accesses to CVAL are not affected.
> 
> The workaround is to reread TVAL and count registers until successive
> reads return the limited range value (32) by back-to-back reads.  Writes to TVAL are
> replaced with an equivalent write to CVAL.
> 
> The workaround is enabled if the hisilicon,erratum-161x01 property is found in
> the timer node in the device tree.  This can be overridden with the
> clocksource.arm_arch_timer.hisilicon-161x01 boot parameter, which allows KVM
> users to enable the workaround until a mechanism is implemented to
> automatically communicate this information.
> 
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
>  Documentation/arm64/silicon-errata.txt |   1 +
>  Documentation/kernel-parameters.txt    |   9 ++
>  arch/arm64/include/asm/arch_timer.h    |  41 +++++++--
>  drivers/clocksource/Kconfig            |  14 ++-
>  drivers/clocksource/arm_arch_timer.c   | 153 +++++++++++++++++++++++++++------
>  5 files changed, 182 insertions(+), 36 deletions(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 405da11..3a79803 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -63,3 +63,4 @@ stable kernels.
>  | Cavium         | ThunderX SMMUv2 | #27704          | N/A		       |
>  |                |                 |                 |                         |
>  | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
> +| Hisilicon	 | Hip05/Hip06/Hip07 | #161x01	     | HISILICON_ERRATUM_161x01|

Please keep the columns aligned. If the affected component doesn't fit
in the allocated space, use multiple lines, or write it in a condensed
way (Hip0{5,6,7} for example). Also, please use spaces instead of tabs
to match the rest of the file.

> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 6fa1d8a..175f349 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -707,6 +707,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  			erratum.  If unspecified, the workaround is
>  			enabled based on the device tree.
> 
> +	clocksource.arm_arch_timer.hisilicon-161x01=
> +			[ARM64]
> +			Format: <bool>
> +			Enable/disable the workaround of Hisilicon
> +			erratum 161x01.  This can be useful for KVM
> +			guests, if the guest device tree doesn't show the
> +			erratum.  If unspecified, the workaround is
> +			enabled based on the device tree.
> +
>  	clearcpuid=BITNUM [X86]
>  			Disable CPUID feature X for the kernel. See
>  			arch/x86/include/asm/cpufeatures.h for the valid bit
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index eaa5bbe..6b510db 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -29,17 +29,24 @@
> 
>  #include <clocksource/arm_arch_timer.h>
> 
> -#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585)
> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161X01)
>  extern struct static_key_false arch_timer_read_ool_enabled;
> -#define needs_fsl_a008585_workaround() \
> +extern struct arch_timer_erratum_workaround *erratum_workaround;
> +#define needs_timer_erratum_workaround() \
>  	static_branch_unlikely(&arch_timer_read_ool_enabled)
>  #else
> -#define needs_fsl_a008585_workaround()  false
> +#define needs_timer_erratum_workaround()  false
>  #endif
> 
> -u32 __fsl_a008585_read_cntp_tval_el0(void);
> -u32 __fsl_a008585_read_cntv_tval_el0(void);
> -u64 __fsl_a008585_read_cntvct_el0(void);
> +struct clock_event_device;
> +
> +struct arch_timer_erratum_workaround {
> +	int erratum;
> +	u32 (*read_cntp_tval_el0)(void);
> +	u32 (*read_cntv_tval_el0)(void);
> +	u64 (*read_cntvct_el0)(void);
> +};
> +extern struct arch_timer_erratum_workaround *erratum_workaround;

You seem to be doing two things in this patch:
- Introducing a more generic erratum handling mechanism
- Adding a workaround for your particular erratum

Please make this two patches.

> 
>  /*
>   * The number of retries is an arbitrary value well beyond the highest number
> @@ -59,16 +66,34 @@ u64 __fsl_a008585_read_cntvct_el0(void);
>  	_new;						\
>  })
> 
> +#define __hisi_161x01_read_reg(reg) ({			\
> +	u64 _old, _new;					\
> +	int _retries = 200;				\

How has this number of retries been determined?

> +							\
> +	do {						\
> +		_old = read_sysreg(reg);		\
> +		_new = read_sysreg(reg);		\
> +		_retries--;				\
> +	} while (unlikely((_new - _old) >> 5) && _retries);	\

Please document why ignoring the bottom 5 bits is a reasonable thing to do.

> +							\
> +	WARN_ON_ONCE(!_retries);			\
> +	_new;						\
> +})
> +
> +
> +
>  #define arch_timer_reg_read_stable(reg) 		\
>  ({							\
>  	u64 _val;					\
> -	if (needs_fsl_a008585_workaround())		\
> -		_val = __fsl_a008585_read_##reg();	\
> +	if (needs_timer_erratum_workaround())		\
> +		_val = erratum_workaround->read_##reg();	\
>  	else						\
>  		_val = read_sysreg(reg);		\
>  	_val;						\
>  })
> 
> +
> +
>  /*
>   * These register accessors are marked inline so the compiler can
>   * nicely work out which register we want, and chuck away the rest of
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 8a753fd..fcfcdc7 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -312,8 +312,20 @@ config FSL_ERRATUM_A008585
>  	help
>  	  This option enables a workaround for Freescale/NXP Erratum
>  	  A-008585 ("ARM generic timer may contain an erroneous
> -	  value").  The workaround will only be active if the
> +	  value").  The workaround will be active if the
>  	  fsl,erratum-a008585 property is found in the timer node.
> +	  This can be overridden with the clocksource.arm_arch_timer.fsl-a008585
> +	  boot parameter.
> +
> +config HISILICON_ERRATUM_161X01
> +	bool "Workaround for Hisilicon Erratum 161201"
> +	default y
> +	depends on ARM_ARCH_TIMER && ARM64
> +	help
> +	  This option enables a workaround for Hisilicon Erratum
> +	  161201. The workaround will be active if the hisi,erratum-161201
> +	  property is found in the timer node. This can be overridden with
> +	  the clocksource.arm_arch_timer.hisi-161201 boot parameter.

Please pick a side. This is either called 161X01 or 161201.

> 
>  config ARM_GLOBAL_TIMER
>  	bool "Support for the ARM global timer" if COMPILE_TEST
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 73c487d..e1cf0ad 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -90,16 +90,23 @@ static int __init early_evtstrm_cfg(char *buf)
>  }
>  early_param("clocksource.arm_arch_timer.evtstrm", early_evtstrm_cfg);
> 
> -/*
> - * Architected system timer support.
> - */
> +#define 	FSL_A008585		1
> +#define 	HISILICON_161X01	2
> +
> +struct arch_timer_erratum_workaround *erratum_workaround;
> +
> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161X01)
> +static int arch_timer_uses_erratum = 0;
> 
> -#ifdef CONFIG_FSL_ERRATUM_A008585
>  DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);
>  EXPORT_SYMBOL_GPL(arch_timer_read_ool_enabled);
> +#endif
> 
> -static int fsl_a008585_enable = -1;
> +/*
> + * Architected system timer support.
> + */
> 
> +#ifdef CONFIG_FSL_ERRATUM_A008585
>  static int __init early_fsl_a008585_cfg(char *buf)
>  {
>  	int ret;
> @@ -109,28 +116,96 @@ static int __init early_fsl_a008585_cfg(char *buf)
>  	if (ret)
>  		return ret;
> 
> -	fsl_a008585_enable = val;
> +	if (val)
> +		arch_timer_uses_erratum = FSL_A008585;
> +

I don't think you need this indirection. You should be able to set the
erratum_workaround pointer, and test that only to enable the OOL access.

>  	return 0;
>  }
>  early_param("clocksource.arm_arch_timer.fsl-a008585", early_fsl_a008585_cfg);
> 
> -u32 __fsl_a008585_read_cntp_tval_el0(void)
> +u32 fsl_a008585_read_cntp_tval_el0(void)
>  {
>  	return __fsl_a008585_read_reg(cntp_tval_el0);
>  }
> 
> -u32 __fsl_a008585_read_cntv_tval_el0(void)
> +u32 fsl_a008585_read_cntv_tval_el0(void)
>  {
>  	return __fsl_a008585_read_reg(cntv_tval_el0);
>  }
> 
> -u64 __fsl_a008585_read_cntvct_el0(void)
> +u64 fsl_a008585_read_cntvct_el0(void)
>  {
>  	return __fsl_a008585_read_reg(cntvct_el0);
>  }
> -EXPORT_SYMBOL(__fsl_a008585_read_cntvct_el0);
> +EXPORT_SYMBOL(fsl_a008585_read_cntvct_el0);

Since you're now going to through a pointer indirection
(erratum_workaround), why do you need this export? Why aren't all these
function static? How does it work with modules that need to access
cntvct_el0 (hint: it probably doesn't...)?

> +#else
> +u32 fsl_a008585_read_cntp_tval_el0(void)
> +{
> +	return 0;
> +}
> +
> +u32 fsl_a008585_read_cntv_tval_el0(void)
> +{
> +	return 0;
> +}
> +
> +u64 fsl_a008585_read_cntvct_el0(void)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL(fsl_a008585_read_cntvct_el0);

I don't think we need any of this.

>  #endif /* CONFIG_FSL_ERRATUM_A008585 */
> 
> +#ifdef CONFIG_HISILICON_ERRATUM_161X01
> +static int __init early_hisi_161x01_cfg(char *buf)
> +{
> +	int ret;
> +	bool val;
> +
> +	ret = strtobool(buf, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val)
> +		arch_timer_uses_erratum = HISILICON_161X01;
> +
> +	return 0;
> +}
> +early_param("clocksource.arm_arch_timer.hisilicon-161x01", early_hisi_161x01_cfg);
> +
> +u32 hisi_161x01_read_cntp_tval_el0(void)
> +{
> +	return __hisi_161x01_read_reg(cntp_tval_el0);
> +}
> +
> +u32 hisi_161x01_read_cntv_tval_el0(void)
> +{
> +	return __hisi_161x01_read_reg(cntv_tval_el0);
> +}
> +
> +u64 hisi_161x01_read_cntvct_el0(void)
> +{
> +	return __hisi_161x01_read_reg(cntvct_el0);
> +}
> +EXPORT_SYMBOL(hisi_161x01_read_cntvct_el0);

Same issue.

> +#else
> +u32 hisi_161x01_read_cntp_tval_el0(void)
> +{
> +	return 0;
> +}
> +
> +u32 hisi_161x01_read_cntv_tval_el0(void)
> +{
> +	return 0;
> +}
> +
> +u64 hisi_161x01_read_cntvct_el0(void)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL(hisi_161x01_read_cntvct_el0);
> +#endif
> +
>  static __always_inline
>  void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val,
>  			  struct clock_event_device *clk)
> @@ -280,8 +355,8 @@ static __always_inline void set_next_event(const int access, unsigned long evt,
>  	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
>  }
> 
> -#ifdef CONFIG_FSL_ERRATUM_A008585
> -static __always_inline void fsl_a008585_set_next_event(const int access,
> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161X01)
> +static __always_inline void erratum_set_next_event(const int access,
>  		unsigned long evt, struct clock_event_device *clk)
>  {
>  	unsigned long ctrl;
> @@ -299,20 +374,35 @@ static __always_inline void fsl_a008585_set_next_event(const int access,
>  	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
>  }
> 
> -static int fsl_a008585_set_next_event_virt(unsigned long evt,
> +static int erratum_set_next_event_virt(unsigned long evt,
>  					   struct clock_event_device *clk)
>  {
> -	fsl_a008585_set_next_event(ARCH_TIMER_VIRT_ACCESS, evt, clk);
> +	erratum_set_next_event(ARCH_TIMER_VIRT_ACCESS, evt, clk);
>  	return 0;
>  }
> 
> -static int fsl_a008585_set_next_event_phys(unsigned long evt,
> +static int erratum_set_next_event_phys(unsigned long evt,
>  					   struct clock_event_device *clk)
>  {
> -	fsl_a008585_set_next_event(ARCH_TIMER_PHYS_ACCESS, evt, clk);
> +	erratum_set_next_event(ARCH_TIMER_PHYS_ACCESS, evt, clk);
>  	return 0;
>  }
> -#endif /* CONFIG_FSL_ERRATUM_A008585 */
> +#endif /* CONFIG_FSL_ERRATUM_A008585 || CONFIG_HISILICON_ERRATUM_161X01 */
> +
> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161X01)
> +static struct arch_timer_erratum_workaround arch_timer_erratum[] = {
> +{
> +	.erratum = FSL_A008585,
> +	.read_cntp_tval_el0 = fsl_a008585_read_cntp_tval_el0,
> +	.read_cntv_tval_el0 = fsl_a008585_read_cntv_tval_el0,
> +	.read_cntvct_el0 = fsl_a008585_read_cntvct_el0,
> +},{
> +	.erratum = HISILICON_161X01,
> +	.read_cntp_tval_el0 = hisi_161x01_read_cntp_tval_el0,
> +	.read_cntv_tval_el0 = hisi_161x01_read_cntv_tval_el0,
> +	.read_cntvct_el0 = hisi_161x01_read_cntvct_el0,
> +} };

Since the two erratum are allowed to be selected independently, you
shouldn't lump them together here.

> +#endif
> 
>  static int arch_timer_set_next_event_virt(unsigned long evt,
>  					  struct clock_event_device *clk)
> @@ -342,16 +432,16 @@ static int arch_timer_set_next_event_phys_mem(unsigned long evt,
>  	return 0;
>  }
> 
> -static void fsl_a008585_set_sne(struct clock_event_device *clk)
> +static void erratum_set_sne(struct clock_event_device *clk)
>  {
> -#ifdef CONFIG_FSL_ERRATUM_A008585
> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161X01)
>  	if (!static_branch_unlikely(&arch_timer_read_ool_enabled))
>  		return;
> 
>  	if (arch_timer_uses_ppi == VIRT_PPI)
> -		clk->set_next_event = fsl_a008585_set_next_event_virt;
> +		clk->set_next_event = erratum_set_next_event_virt;
>  	else
> -		clk->set_next_event = fsl_a008585_set_next_event_phys;
> +		clk->set_next_event = erratum_set_next_event_phys;
>  #endif
>  }
> 
> @@ -384,7 +474,7 @@ static void __arch_timer_setup(unsigned type,
>  			BUG();
>  		}
> 
> -		fsl_a008585_set_sne(clk);
> +		erratum_set_sne(clk);
>  	} else {
>  		clk->features |= CLOCK_EVT_FEAT_DYNIRQ;
>  		clk->name = "arch_mem_timer";
> @@ -890,12 +980,21 @@ static int __init arch_timer_of_init(struct device_node *np)
> 
>  	arch_timer_c3stop = !of_property_read_bool(np, "always-on");
> 
> -#ifdef CONFIG_FSL_ERRATUM_A008585
> -	if (fsl_a008585_enable < 0)
> -		fsl_a008585_enable = of_property_read_bool(np, "fsl,erratum-a008585");
> -	if (fsl_a008585_enable) {
> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161X01)
> +	if (!arch_timer_uses_erratum) {
> +		if (IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) &&
> +		    of_property_read_bool(np, "fsl,erratum-a008585"))
> +			arch_timer_uses_erratum = FSL_A008585;
> +		else if (IS_ENABLED(CONFIG_HISI_ERRATUM_161X01) &&
> +			 of_property_read_bool(np, "hisilicon,erratum-161x01"))
> +			arch_timer_uses_erratum = HISILICON_161X01;
> +	}
> +
> +	if (arch_timer_uses_erratum) {
> +		erratum_workaround = &arch_timer_erratum[arch_timer_uses_erratum - 1];
> +		pr_info("Enabling workaround for %s\n", arch_timer_uses_erratum == FSL_A008585 ?
> +			"FSL erratum A-008585" : "HISILICON ERRATUM 161x01");
>  		static_branch_enable(&arch_timer_read_ool_enabled);
> -		pr_info("Enabling workaround for FSL erratum A-008585\n");

Get rid of arch_timer_uses_erratum, of the erratum identifier in the
structure, and put a static string in there. That should do everything
you need.

>  	}
>  #endif
> 

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH] mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading
From: Lee Jones @ 2016-10-24 10:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <bbd16837-a260-b574-1b1a-1bede038e3de@redhat.com>

On Tue, 18 Oct 2016, Hans de Goede wrote:
> On 18-10-16 07:25, Chen-Yu Tsai wrote:
> > On Wed, Oct 5, 2016 at 11:51 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> > > The i2c subsys does not load modules by compatible, only by
> > > i2c-id, with e.g. a modalias of: "i2c:axp209".
> > > 
> > > Populate the axp20x_i2c_id[] table with supported ids, so that
> > > module auto-loading will work.
> > > 
> > > Reported-by: Dennis Gilmore <dennis@ausil.us>
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > 
> > Acked-by: Chen-Yu Tsai <wens@csie.org>
> > 
> > Even though axp20x-i2c seems to be the only "DT only" i2c client,
> > would it make sense to add DT-based module autoloading to the i2c
> > core?
> 
> If it is not too invasive, then yes that would be a sensible addition IMHO.

If I understand you correctly, I already have a patch-set on the ML
that does this.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Question about arm64 earlycon
From: Arnd Bergmann @ 2016-10-24 10:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CADaLNDnB3QBjODYNTndBsexGQfnL1rj6DAUPcf1nT9+aJaqCtw@mail.gmail.com>

On Sunday, October 23, 2016 12:26:59 AM CEST Duc Dang wrote:
> Hi Catalin, Marc, Mark, Arnd,
> 
> I am testing with 3.12 kernel with earlyprintk enabled and I see some
> garbage characters in the console log right before the message
> indicating that the real console device is initialized:
> 
> <some garbage characters here>01c020000.serial: ttyS0 at MMIO
> 0x1c020000 (irq = 108, base_baud = 3125000) is a U6_16550A
> console [ttyS0] enabled, bootconsole disabled
> 
> I looked through early_prink.c file and printk.c file and it looks
> like there is case that some early boot code can touch the UART
> hardware via ealy console driver while the 'real' console driver is
> setting up the same UART port? Please let me know if I missed some
> important piece of code that can prevent this.

I don't think we every supported earlyprintk on arm64, and
earlycon support may have been added later.

If you can't use a modern kernel, try backporting all the
relevant earlycon changes.

	Arnd

^ permalink raw reply

* [PATCH V4 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
From: Suzuki K Poulose @ 2016-10-24  9:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477071013-29563-3-git-send-email-tbaicar@codeaurora.org>

On 21/10/16 18:30, Tyler Baicar wrote:
> Currently when a RAS error is reported it is not timestamped.
> The ACPI 6.1 spec adds the timestamp field to the generic error
> data entry v3 structure. The timestamp of when the firmware
> generated the error is now being reported.
>
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Signed-off-by: Richard Ruigrok <rruigrok@codeaurora.org>
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
> ---
>  drivers/acpi/apei/ghes.c    | 14 +++++++---
>  drivers/firmware/efi/cper.c | 67 +++++++++++++++++++++++++++++++++++++--------
>  include/acpi/ghes.h         | 10 +++++++
>  include/linux/cper.h        | 12 ++++++++
>  4 files changed, 88 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 7d020b0..7610f08 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -419,7 +419,8 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int
>  	int flags = -1;
>  	int sec_sev = ghes_severity(gdata->error_severity);
>  	struct cper_sec_mem_err *mem_err;
> -	mem_err = (struct cper_sec_mem_err *)(gdata + 1);
> +
> +	mem_err = acpi_hest_generic_data_payload(gdata);
>
>  	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
>  		return;
> @@ -449,14 +450,18 @@ static void ghes_do_proc(struct ghes *ghes,
>  {
>  	int sev, sec_sev;
>  	struct acpi_hest_generic_data *gdata;
> +	uuid_le sec_type;
>
>  	sev = ghes_severity(estatus->error_severity);
>  	apei_estatus_for_each_section(estatus, gdata) {
>  		sec_sev = ghes_severity(gdata->error_severity);
> -		if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
> +		sec_type = *(uuid_le *)gdata->section_type;
> +
> +		if (!uuid_le_cmp(sec_type,
>  				 CPER_SEC_PLATFORM_MEM)) {
>  			struct cper_sec_mem_err *mem_err;
> -			mem_err = (struct cper_sec_mem_err *)(gdata+1);
> +
> +			mem_err = acpi_hest_generic_data_payload(gdata);
>  			ghes_edac_report_mem_error(ghes, sev, mem_err);
>
>  			arch_apei_report_mem_error(sev, mem_err);
> @@ -466,7 +471,8 @@ static void ghes_do_proc(struct ghes *ghes,
>  		else if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
>  				      CPER_SEC_PCIE)) {
>  			struct cper_sec_pcie *pcie_err;
> -			pcie_err = (struct cper_sec_pcie *)(gdata+1);
> +
> +			pcie_err = acpi_hest_generic_data_payload(gdata);
>  			if (sev == GHES_SEV_RECOVERABLE &&
>  			    sec_sev == GHES_SEV_RECOVERABLE &&
>  			    pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index d425374..af7e1e9 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -32,6 +32,9 @@
>  #include <linux/acpi.h>
>  #include <linux/pci.h>
>  #include <linux/aer.h>
> +#include <linux/printk.h>
> +#include <linux/bcd.h>
> +#include <acpi/ghes.h>
>
>  #define INDENT_SP	" "
>
> @@ -386,13 +389,37 @@ static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
>  	pfx, pcie->bridge.secondary_status, pcie->bridge.control);
>  }
>
> +static void cper_estatus_print_section_v300(const char *pfx,
> +	const struct acpi_hest_generic_data_v300 *gdata)
> +{
> +	__u8 hour, min, sec, day, mon, year, century, *timestamp;
> +
> +	if (gdata->validation_bits & ACPI_HEST_GEN_VALID_TIMESTAMP) {
> +		timestamp = (__u8 *)&(gdata->time_stamp);
> +		sec = bcd2bin(timestamp[0]);
> +		min = bcd2bin(timestamp[1]);
> +		hour = bcd2bin(timestamp[2]);
> +		day = bcd2bin(timestamp[4]);
> +		mon = bcd2bin(timestamp[5]);
> +		year = bcd2bin(timestamp[6]);
> +		century = bcd2bin(timestamp[7]);
> +		printk("%stime: %7s %02d%02d-%02d-%02d %02d:%02d:%02d\n", pfx,
> +			0x01 & *(timestamp + 3) ? "precise" : "", century,
> +			year, mon, day, hour, min, sec);
> +	}
> +}
> +
>  static void cper_estatus_print_section(
> -	const char *pfx, const struct acpi_hest_generic_data *gdata, int sec_no)
> +	const char *pfx, struct acpi_hest_generic_data *gdata, int sec_no)
>  {
>  	uuid_le *sec_type = (uuid_le *)gdata->section_type;
>  	__u16 severity;
>  	char newpfx[64];
>
> +	if (acpi_hest_generic_data_version(gdata))
> +		cper_estatus_print_section_v300(pfx,
> +			(const struct acpi_hest_generic_data_v300 *)gdata);
> +
>  	severity = gdata->error_severity;
>  	printk("%s""Error %d, type: %s\n", pfx, sec_no,
>  	       cper_severity_str(severity));
> @@ -403,14 +430,18 @@ static void cper_estatus_print_section(
>
>  	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
>  	if (!uuid_le_cmp(*sec_type, CPER_SEC_PROC_GENERIC)) {
> -		struct cper_sec_proc_generic *proc_err = (void *)(gdata + 1);
> +		struct cper_sec_proc_generic *proc_err;
> +
> +		proc_err = acpi_hest_generic_data_payload(gdata);
>  		printk("%s""section_type: general processor error\n", newpfx);
>  		if (gdata->error_data_length >= sizeof(*proc_err))
>  			cper_print_proc_generic(newpfx, proc_err);
>  		else
>  			goto err_section_too_small;
>  	} else if (!uuid_le_cmp(*sec_type, CPER_SEC_PLATFORM_MEM)) {
> -		struct cper_sec_mem_err *mem_err = (void *)(gdata + 1);
> +		struct cper_sec_mem_err *mem_err;
> +
> +		mem_err = acpi_hest_generic_data_payload(gdata);
>  		printk("%s""section_type: memory error\n", newpfx);
>  		if (gdata->error_data_length >=
>  		    sizeof(struct cper_sec_mem_err_old))
> @@ -419,7 +450,9 @@ static void cper_estatus_print_section(
>  		else
>  			goto err_section_too_small;
>  	} else if (!uuid_le_cmp(*sec_type, CPER_SEC_PCIE)) {
> -		struct cper_sec_pcie *pcie = (void *)(gdata + 1);
> +		struct cper_sec_pcie *pcie;
> +
> +		pcie = acpi_hest_generic_data_payload(gdata);
>  		printk("%s""section_type: PCIe error\n", newpfx);
>  		if (gdata->error_data_length >= sizeof(*pcie))
>  			cper_print_pcie(newpfx, pcie, gdata);
> @@ -438,6 +471,7 @@ void cper_estatus_print(const char *pfx,
>  			const struct acpi_hest_generic_status *estatus)
>  {
>  	struct acpi_hest_generic_data *gdata;
> +	struct acpi_hest_generic_data_v300 *gdata_v3 = NULL;
>  	unsigned int data_len, gedata_len;
>  	int sec_no = 0;
>  	char newpfx[64];
> @@ -451,12 +485,22 @@ void cper_estatus_print(const char *pfx,
>  	printk("%s""event severity: %s\n", pfx, cper_severity_str(severity));
>  	data_len = estatus->data_length;
>  	gdata = (struct acpi_hest_generic_data *)(estatus + 1);
> +	if (acpi_hest_generic_data_version(gdata))
> +		gdata_v3 = (struct acpi_hest_generic_data_v300 *)gdata;

I think the acpi_hest_generic_data_version() doesn't check if the version is
V3 or higher ?

> +
>  	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
> +
>  	while (data_len >= sizeof(*gdata)) {
>  		gedata_len = gdata->error_data_length;
>  		cper_estatus_print_section(newpfx, gdata, sec_no);
> -		data_len -= gedata_len + sizeof(*gdata);
> -		gdata = (void *)(gdata + 1) + gedata_len;
> +		if(gdata_v3) {
> +			data_len -= gedata_len + sizeof(*gdata_v3);
> +			gdata_v3 = (void *)(gdata_v3 + 1) + gedata_len;
> +			gdata = (struct acpi_hest_generic_data *)gdata_v3;
> +		} else {
> +			data_len -= gedata_len + sizeof(*gdata);
> +			gdata = (void *)(gdata + 1) + gedata_len;
> +		}

Could we not use the helpers we define below to unify the code here and avoid the
switch ?



>  		sec_no++;
>  	}
>  }
> @@ -486,12 +530,13 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
>  		return rc;
>  	data_len = estatus->data_length;
>  	gdata = (struct acpi_hest_generic_data *)(estatus + 1);
> -	while (data_len >= sizeof(*gdata)) {
> -		gedata_len = gdata->error_data_length;
> -		if (gedata_len > data_len - sizeof(*gdata))
> +
> +	while (data_len >= acpi_hest_generic_data_size(gdata)) {
> +		gedata_len = acpi_hest_generic_data_error_length(gdata);
> +		if (gedata_len > data_len - acpi_hest_generic_data_size(gdata))
>  			return -EINVAL;
> -		data_len -= gedata_len + sizeof(*gdata);
> -		gdata = (void *)(gdata + 1) + gedata_len;
> +		data_len -= gedata_len + acpi_hest_generic_data_size(gdata);
> +		gdata = acpi_hest_generic_data_next(gdata);
>  	}
>  	if (data_len)
>  		return -EINVAL;
> diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
> index 68f088a..56b9679 100644
> --- a/include/acpi/ghes.h
> +++ b/include/acpi/ghes.h
> @@ -73,3 +73,13 @@ static inline void ghes_edac_unregister(struct ghes *ghes)
>  {
>  }
>  #endif
> +
> +#define acpi_hest_generic_data_version(gdata)			\
> +	(gdata->revision >> 8)
> +


>
> +#define acpi_hest_generic_data_error_length(gdata)	\
> +	(((struct acpi_hest_generic_data *)(gdata))->error_data_length)
> +#define acpi_hest_generic_data_size(gdata)		\
> +	((acpi_hest_generic_data_version(gdata) >= 3) ?	\
> +	sizeof(struct acpi_hest_generic_data_v300) :	\
> +	sizeof(struct acpi_hest_generic_data))
> +#define acpi_hest_generic_data_record_size(gdata)	\
> +	(acpi_hest_generic_data_size(gdata) +		\
> +	acpi_hest_generic_data_error_length(gdata))
> +#define acpi_hest_generic_data_next(gdata)		\
> +	((void *)(gdata) + acpi_hest_generic_data_record_size(gdata))
> +

Rest looks good

Cheers
Suzuki

^ permalink raw reply

* [PATCH 2/3] ARM: convert to generated system call tables
From: Geert Uytterhoeven @ 2016-10-24  9:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3851270.xZRcP9hae0@wuerfel>

Hi Arnd,

On Fri, Oct 21, 2016 at 3:06 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> - a lot of the less common architectures just don't get updated
>   in time, out of 22 architectures that don't use asm-generic/unistd.h,
>   only 12 have pwritev2 in linux-next, and only three have pkey_mprotect

Should everybody implement pkey_mprotect?

"x86, pkeys: remove cruft from never-merged syscalls" and

arch/powerpc/include/asm/unistd.h:#define __IGNORE_pkey_mprotect

aren't that reassuring...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 2/3] ARM: convert to generated system call tables
From: Geert Uytterhoeven @ 2016-10-24  9:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <87eg382kv9.fsf@belgarion.home>

On Sat, Oct 22, 2016 at 10:23 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
>
>> On Friday, October 21, 2016 4:48:56 PM CEST Russell King - ARM Linux wrote:
>>> What's the point of the x32 mode?
>>
>> On x86, the motivation is faster code for most use cases that
>> don't need a lot of memory, as the 64-bit opcodes have 16 registers
>> rather than 8 in 32-bit mode but 32-bit pointers have lower
>> cache footprint than 64-bit pointers.
>
> For completness, the second point of x32 AFAIU is the IP-relative addressing
> which is not available in standard 32 bit mode, which improves PIC code. For
> simple not algorithmic code (think Android HAL for example) with many shared
> libraries, it's better in the Hardware Abstraction Layer Libraries, instead of
> the push-to-stack and pop register.

But that's not an advantage compared to full am64 mode, right?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] asm-generic: Drop getrlimit and setrlimit syscalls from default list
From: James Hogan @ 2016-10-24  9:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477138444-14993-1-git-send-email-ynorov@caviumnetworks.com>

On Sat, Oct 22, 2016 at 03:14:04PM +0300, Yury Norov wrote:
> The newer prlimit64 syscall provides all the functionality provided by
> the getrlimit and setrlimit syscalls and adds the pid of target process,
> so future architectures won't need to include getrlimit and setrlimit.
> 
> Therefore drop getrlimit and setrlimit syscalls from the generic syscall
> list unless __ARCH_WANT_SET_GET_RLIMIT is defined by the architecture's
> unistd.h prior to including asm-generic/unistd.h, and adjust all
> architectures using the generic syscall list to define it so that no
> in-tree architectures are affected.
> 
> Cc: Vineet Gupta <vgupta@synopsys.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Aurelien Jacquiot <a-jacquiot@ti.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Richard Kuo <rkuo@codeaurora.org>
> Cc: James Hogan <james.hogan@imgtec.com>
> Cc: Ley Foon Tan <lftan@altera.com>
> Cc: Jonas Bonn <jonas@southpole.se>
> Cc: Chen Liqin <liqin.linux@gmail.com>
> Cc: Lennox Wu <lennox.wu@gmail.com>
> Cc: Chris Metcalf <cmetcalf@mellanox.com>
> Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Andrew Pinski <Andrew.Pinski@cavium.com>
> Cc: linux-snps-arc at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-c6x-dev at linux-c6x.org
> Cc: uclinux-h8-devel at lists.sourceforge.jp
> Cc: linux-hexagon at vger.kernel.org
> Cc: linux-metag at vger.kernel.org
> Cc: nios2-dev at lists.rocketboards.org
> Cc: linux-arch at vger.kernel.or
> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> 
> ---
>  arch/arc/include/uapi/asm/unistd.h       | 1 +
>  arch/arm64/include/uapi/asm/unistd.h     | 1 +
>  arch/c6x/include/uapi/asm/unistd.h       | 1 +
>  arch/h8300/include/uapi/asm/unistd.h     | 1 +
>  arch/hexagon/include/uapi/asm/unistd.h   | 1 +
>  arch/metag/include/uapi/asm/unistd.h     | 1 +

Acked-by: James Hogan <james.hogan@imgtec.com> [metag]

Cheers
James

>  arch/nios2/include/uapi/asm/unistd.h     | 1 +
>  arch/openrisc/include/uapi/asm/unistd.h  | 1 +
>  arch/score/include/uapi/asm/unistd.h     | 1 +
>  arch/tile/include/uapi/asm/unistd.h      | 1 +
>  arch/unicore32/include/uapi/asm/unistd.h | 1 +
>  include/uapi/asm-generic/unistd.h        | 5 +++++
>  12 files changed, 16 insertions(+)
> 
> diff --git a/arch/arc/include/uapi/asm/unistd.h b/arch/arc/include/uapi/asm/unistd.h
> index 41fa2ec..928546d 100644
> --- a/arch/arc/include/uapi/asm/unistd.h
> +++ b/arch/arc/include/uapi/asm/unistd.h
> @@ -16,6 +16,7 @@
>  #define _UAPI_ASM_ARC_UNISTD_H
>  
>  #define __ARCH_WANT_RENAMEAT
> +#define __ARCH_WANT_SET_GET_RLIMIT
>  #define __ARCH_WANT_SYS_EXECVE
>  #define __ARCH_WANT_SYS_CLONE
>  #define __ARCH_WANT_SYS_VFORK
> diff --git a/arch/arm64/include/uapi/asm/unistd.h b/arch/arm64/include/uapi/asm/unistd.h
> index 043d17a..48355a6 100644
> --- a/arch/arm64/include/uapi/asm/unistd.h
> +++ b/arch/arm64/include/uapi/asm/unistd.h
> @@ -15,5 +15,6 @@
>   */
>  
>  #define __ARCH_WANT_RENAMEAT
> +#define __ARCH_WANT_SET_GET_RLIMIT
>  
>  #include <asm-generic/unistd.h>
> diff --git a/arch/c6x/include/uapi/asm/unistd.h b/arch/c6x/include/uapi/asm/unistd.h
> index 12d73d9..f676231 100644
> --- a/arch/c6x/include/uapi/asm/unistd.h
> +++ b/arch/c6x/include/uapi/asm/unistd.h
> @@ -15,6 +15,7 @@
>   */
>  
>  #define __ARCH_WANT_RENAMEAT
> +#define __ARCH_WANT_SET_GET_RLIMIT
>  #define __ARCH_WANT_SYS_CLONE
>  
>  /* Use the standard ABI for syscalls. */
> diff --git a/arch/h8300/include/uapi/asm/unistd.h b/arch/h8300/include/uapi/asm/unistd.h
> index 7dd20ef..2f98394 100644
> --- a/arch/h8300/include/uapi/asm/unistd.h
> +++ b/arch/h8300/include/uapi/asm/unistd.h
> @@ -1,5 +1,6 @@
>  #define __ARCH_NOMMU
>  
>  #define __ARCH_WANT_RENAMEAT
> +#define __ARCH_WANT_SET_GET_RLIMIT
>  
>  #include <asm-generic/unistd.h>
> diff --git a/arch/hexagon/include/uapi/asm/unistd.h b/arch/hexagon/include/uapi/asm/unistd.h
> index 2151760..52d585c 100644
> --- a/arch/hexagon/include/uapi/asm/unistd.h
> +++ b/arch/hexagon/include/uapi/asm/unistd.h
> @@ -28,6 +28,7 @@
>  
>  #define sys_mmap2 sys_mmap_pgoff
>  #define __ARCH_WANT_RENAMEAT
> +#define __ARCH_WANT_SET_GET_RLIMIT
>  #define __ARCH_WANT_SYS_EXECVE
>  #define __ARCH_WANT_SYS_CLONE
>  #define __ARCH_WANT_SYS_VFORK
> diff --git a/arch/metag/include/uapi/asm/unistd.h b/arch/metag/include/uapi/asm/unistd.h
> index 459b6ec..16b5cb3 100644
> --- a/arch/metag/include/uapi/asm/unistd.h
> +++ b/arch/metag/include/uapi/asm/unistd.h
> @@ -8,6 +8,7 @@
>   */
>  
>  #define __ARCH_WANT_RENAMEAT
> +#define __ARCH_WANT_SET_GET_RLIMIT
>  
>  /* Use the standard ABI for syscalls. */
>  #include <asm-generic/unistd.h>
> diff --git a/arch/nios2/include/uapi/asm/unistd.h b/arch/nios2/include/uapi/asm/unistd.h
> index 51a32c7..b0dda4d 100644
> --- a/arch/nios2/include/uapi/asm/unistd.h
> +++ b/arch/nios2/include/uapi/asm/unistd.h
> @@ -18,6 +18,7 @@
>   #define sys_mmap2 sys_mmap_pgoff
>  
>  #define __ARCH_WANT_RENAMEAT
> +#define __ARCH_WANT_SET_GET_RLIMIT
>  
>  /* Use the standard ABI for syscalls */
>  #include <asm-generic/unistd.h>
> diff --git a/arch/openrisc/include/uapi/asm/unistd.h b/arch/openrisc/include/uapi/asm/unistd.h
> index 471905b..6812d81 100644
> --- a/arch/openrisc/include/uapi/asm/unistd.h
> +++ b/arch/openrisc/include/uapi/asm/unistd.h
> @@ -21,6 +21,7 @@
>  #define sys_mmap2 sys_mmap_pgoff
>  
>  #define __ARCH_WANT_RENAMEAT
> +#define __ARCH_WANT_SET_GET_RLIMIT
>  #define __ARCH_WANT_SYS_FORK
>  #define __ARCH_WANT_SYS_CLONE
>  
> diff --git a/arch/score/include/uapi/asm/unistd.h b/arch/score/include/uapi/asm/unistd.h
> index d4008c3..7ad1bdc 100644
> --- a/arch/score/include/uapi/asm/unistd.h
> +++ b/arch/score/include/uapi/asm/unistd.h
> @@ -1,6 +1,7 @@
>  #define __ARCH_HAVE_MMU
>  
>  #define __ARCH_WANT_RENAMEAT
> +#define __ARCH_WANT_SET_GET_RLIMIT
>  #define __ARCH_WANT_SYSCALL_NO_AT
>  #define __ARCH_WANT_SYSCALL_NO_FLAGS
>  #define __ARCH_WANT_SYSCALL_OFF_T
> diff --git a/arch/tile/include/uapi/asm/unistd.h b/arch/tile/include/uapi/asm/unistd.h
> index 24e9187..cf0505f 100644
> --- a/arch/tile/include/uapi/asm/unistd.h
> +++ b/arch/tile/include/uapi/asm/unistd.h
> @@ -13,6 +13,7 @@
>   */
>  
>  #define __ARCH_WANT_RENAMEAT
> +#define __ARCH_WANT_SET_GET_RLIMIT
>  #if !defined(__LP64__) || defined(__SYSCALL_COMPAT)
>  /* Use the flavor of this syscall that matches the 32-bit API better. */
>  #define __ARCH_WANT_SYNC_FILE_RANGE2
> diff --git a/arch/unicore32/include/uapi/asm/unistd.h b/arch/unicore32/include/uapi/asm/unistd.h
> index 1f63c47..ef25aec 100644
> --- a/arch/unicore32/include/uapi/asm/unistd.h
> +++ b/arch/unicore32/include/uapi/asm/unistd.h
> @@ -11,6 +11,7 @@
>   */
>  
>  #define __ARCH_WANT_RENAMEAT
> +#define __ARCH_WANT_SET_GET_RLIMIT
>  
>  /* Use the standard ABI for syscalls. */
>  #include <asm-generic/unistd.h>
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 9b1462e..bbaeac0 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -465,10 +465,15 @@ __SYSCALL(__NR_uname, sys_newuname)
>  __SYSCALL(__NR_sethostname, sys_sethostname)
>  #define __NR_setdomainname 162
>  __SYSCALL(__NR_setdomainname, sys_setdomainname)
> +
> +#ifdef __ARCH_WANT_SET_GET_RLIMIT
> +/* getrlimit and setrlimit are superseded with prlimit64 */
>  #define __NR_getrlimit 163
>  __SC_COMP(__NR_getrlimit, sys_getrlimit, compat_sys_getrlimit)
>  #define __NR_setrlimit 164
>  __SC_COMP(__NR_setrlimit, sys_setrlimit, compat_sys_setrlimit)
> +#endif
> +
>  #define __NR_getrusage 165
>  __SC_COMP(__NR_getrusage, sys_getrusage, compat_sys_getrusage)
>  #define __NR_umask 166
> -- 
> 2.7.4
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161024/43e985fd/attachment-0001.sig>

^ permalink raw reply

* [PATCH v5 3/9] vcodec: mediatek: Add Mediatek V4L2 Video Decoder Driver
From: Mauro Carvalho Chehab @ 2016-10-24  9:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477279328.10501.10.camel@mtksdaap41>

Em Mon, 24 Oct 2016 11:22:08 +0800
Tiffany Lin <tiffany.lin@mediatek.com> escreveu:

> Hi Mauro,
> 
> On Fri, 2016-10-21 at 11:01 -0200, Mauro Carvalho Chehab wrote:
> > Em Fri, 2 Sep 2016 20:19:54 +0800
> > Tiffany Lin <tiffany.lin@mediatek.com> escreveu:
> >   
> > > Add v4l2 layer decoder driver for MT8173
> > > 
> > > Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>  
> >   
> > > +int vdec_if_init(struct mtk_vcodec_ctx *ctx, unsigned int fourcc)
> > > +{
> > > +	int ret = 0;
> > > +
> > > +	switch (fourcc) {
> > > +	case V4L2_PIX_FMT_H264:
> > > +	case V4L2_PIX_FMT_VP8:
> > > +	default:
> > > +		return -EINVAL;
> > > +	}  
> > 
> > Did you ever test this driver? The above code will *always* return
> > -EINVAL, with will cause vidioc_vdec_s_fmt() to always fail!
> > 
> > I suspect that what you wanted to do, instead, is:
> > 
> > 	switch (fourcc) {
> > 	case V4L2_PIX_FMT_H264:
> > 	case V4L2_PIX_FMT_VP8:
> > 		break;
> > 	default:
> > 		return -EINVAL;
> >   
> 
> The original idea here is that vp8 and h264 are added in later patches.
> If get this patch without later patches, it should return -EINVAL.

I noticed your idea, but next time, don't add dead code like that.
Reviewers check patch by patch at the order they're present at the
patch series.

So, don't add something broken by purpose, assuming that it would
be fixed later.

> 
> 
> > Btw, this patch series has also several issues that were pointed by
> > checkpatch. Please *always* run checkpatch when submitting your work.
> > 
> > You should take a look at the Kernel documentation about how to
> > submit patches, at:
> > 	https://mchehab.fedorapeople.org/kernel_docs/process/index.html
> > 
> > PS.: this time, I fixed the checkpatch issues for you. So, let me know
> > if the patch below is OK, and I'll merge it at media upstream,
> > assuming that the other patches in this series are ok.
> >   
> 
> I did run checkpatch, but I don't know why these issues missed.
> probably I run checkpatch for all files not for patches.
> I will take a look at the documentation and keep this in mind for future
> upstream.
> Appreciated for your help.

Checkpatch should be run patch by patch, as we expect that all patches
will follow the coding style and will compile fine, without introducing
warnings.

I do compile the Kernel for every single patch I merge.

Regards,
Mauro

^ permalink raw reply

* [PATCH v2] drivers: psci: PSCI checker module
From: Geert Uytterhoeven @ 2016-10-24  9:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <80506636-d4a1-9a1b-4169-d153326b54aa@arm.com>

Hi Sudeep,

On Mon, Oct 24, 2016 at 10:57 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> On 24/10/16 09:09, Geert Uytterhoeven wrote:
>> On Thu, Oct 20, 2016 at 4:21 PM, Sudeep Holla <sudeep.holla@arm.com>
>> wrote:
>>> On 20/10/16 14:38, Kevin Brodsky wrote:
>>>
>>> [...]
>>>
>>>>
>>>> Thanks for the heads-up! I'll rebase on 4.9-rc1 and see what needs to be
>>>> done.
>>>>
>>>
>>> Just be aware that v4.9-rc1 doesn't have commit 9cfb38a7ba5a
>>> ("sched/fair: Fix sched domains NULL dereference in
>>> select_idle_sibling()") which fixes the cpuhotplug issue you would
>>> observe.
>>
>>
>> Good to know. I saw that issue during resume from s2ram on
>> r8a7795/Salvator-X
>> once, but couldn't reproduce it.
>>
>
> Did you try v4.9-rc1 itself ? The above commit is present post -rc1 and
> must be in -rc2. I can consistently see the crash without the commit.

I used a tree based on v4.9-rc1, not including commit 9cfb38a7ba5a.
Will try v4.9-rc2 shortly...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH v2] drivers: psci: PSCI checker module
From: Sudeep Holla @ 2016-10-24  8:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdVgccJ0A2d_-uU+8r0h-NBeet3eGkKq14eaFo34ydqtzQ@mail.gmail.com>



On 24/10/16 09:09, Geert Uytterhoeven wrote:
> On Thu, Oct 20, 2016 at 4:21 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> On 20/10/16 14:38, Kevin Brodsky wrote:
>>
>> [...]
>>
>>>
>>> Thanks for the heads-up! I'll rebase on 4.9-rc1 and see what needs to be
>>> done.
>>>
>>
>> Just be aware that v4.9-rc1 doesn't have commit 9cfb38a7ba5a
>> ("sched/fair: Fix sched domains NULL dereference in
>> select_idle_sibling()") which fixes the cpuhotplug issue you would
>> observe.
>
> Good to know. I saw that issue during resume from s2ram on r8a7795/Salvator-X
> once, but couldn't reproduce it.
>

Did you try v4.9-rc1 itself ? The above commit is present post -rc1 and
must be in -rc2. I can consistently see the crash without the commit.

-- 
Regards,
Sudeep

^ permalink raw reply

* [PATCH v2 2/2] arm64: dts: zx: Add clock controller nodes
From: Shawn Guo @ 2016-10-24  8:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161017204841.GD3264@localhost>

On Mon, Oct 17, 2016 at 01:48:41PM -0700, Olof Johansson wrote:
> On Thu, Oct 13, 2016 at 08:31:21PM +0800, Jun Nie wrote:
> > Add clock controller nodes, including one top controller
> > two low speed controllers and one audio controller.
> > 
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > ---
> >  arch/arm64/boot/dts/zte/zx296718.dtsi | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> 
> This isn't a fix for a bug, so please send this with your other updates for
> v4.10.

I queued it up for 4.10.

Shawn

^ permalink raw reply

* Build regressions/improvements in v4.9-rc2
From: Geert Uytterhoeven @ 2016-10-24  8:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477298556-1522-1-git-send-email-geert@linux-m68k.org>

On Mon, Oct 24, 2016 at 10:42 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> JFYI, when comparing v4.9-rc2[1] to v4.9-rc1[3], the summaries are:
>   - build errors: +10/-21

  + /home/kisskb/slave/src/arch/arm/include/asm/dma-mapping.h: error:
invalid suffix "xUL" on integer constant:  => 84:1
  + /home/kisskb/slave/src/include/linux/dma-mapping.h: error: invalid
suffix "xUL" on integer constant:  => 190:1, 193:1
  + /home/kisskb/slave/src/include/linux/mm.h: error: invalid suffix
"xUL" on integer constant:  => 555:1
  + /home/kisskb/slave/src/include/linux/scatterlist.h: error: invalid
suffix "xUL" on integer constant:  => 142:1

arm-randconfig

Not a new problem. Real issue is "CONFIG_PAGE_OFFSET=" in .config,
leading to:

    ./include/generated/autoconf.h:1113:28: error: invalid suffix
"xUL" on integer constant
     #define CONFIG_PAGE_OFFSET 0x

Why is CONFIG_PAGE_OFFSET empty?

    scripts/kconfig/conf  --oldconfig Kconfig
    .config:316:warning: symbol value '' invalid for PAGE_OFFSET

config PAGE_OFFSET
        hex
        default PHYS_OFFSET if !MMU

# CONFIG_MMU is not set
but CONFIG_PHYS_OFFSET does not exist in .config??

config PHYS_OFFSET
        hex "Physical address of main memory" if MMU
        depends on !ARM_PATCH_PHYS_VIRT
        default DRAM_BASE if !MMU
        default 0x00000000 if ARCH_EBSA110 || \
                        ARCH_FOOTBRIDGE || \
                        ARCH_INTEGRATOR || \
                        ARCH_IOP13XX || \
                        ARCH_KS8695 || \
                        ARCH_REALVIEW
        default 0x10000000 if ARCH_OMAP1 || ARCH_RPC
        default 0x20000000 if ARCH_S5PV210
        default 0xc0000000 if ARCH_SA1100
        help
          Please provide the physical address corresponding to the
          location of main memory in your system.

CONFIG_DRAM_BASE=0x00800000

Aha, CONFIG_ARM_PATCH_PHYS_VIRT=y

> [1] http://kisskb.ellerman.id.au/kisskb/head/11104/ (all 262 configs)
> [3] http://kisskb.ellerman.id.au/kisskb/head/11053/ (all 262 configs)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] ARM: sti: stih410-clocks: Add PROC_STFE as a critical clock
From: Lee Jones @ 2016-10-24  8:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476804957-24000-1-git-send-email-peter.griffin@linaro.org>

On Tue, 18 Oct 2016, Peter Griffin wrote:

> Once the ST frontend demux HW IP has been enabled, the clock can't
> be disabled otherwise the system will hang and the board will
> be unserviceable.
> 
> To allow balanced clock enable/disable calls in the driver we use
> the critical clock infrastructure to take an extra reference on the
> clock so the clock will never actually be disabled.

This is an abuse of the critical-clocks framework, and is exactly the
type of hack I promised the clk guys I'd try to prevent.  If this, or
any other IP has some quirks (i.e. once enabled, if this clock is
subsequently disabled it will have a catastrophic effect on the
platform), then they should be worked around in the driver.

The correct thing to do here is craft a clk-keep-on flag and ensure it
is set to true for the effected platform(s)' platform data.

> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  arch/arm/boot/dts/stih410-clock.dtsi | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/stih410-clock.dtsi b/arch/arm/boot/dts/stih410-clock.dtsi
> index 8598eff..07c8ef9 100644
> --- a/arch/arm/boot/dts/stih410-clock.dtsi
> +++ b/arch/arm/boot/dts/stih410-clock.dtsi
> @@ -208,7 +208,8 @@
>  						     "clk-clust-hades",
>  						     "clk-hwpe-hades",
>  						     "clk-fc-hades";
> -				clock-critical = <CLK_ICN_CPU>,
> +				clock-critical = <CLK_PROC_STFE>,
> +						 <CLK_ICN_CPU>,
>  						 <CLK_TX_ICN_DMU>,
>  						 <CLK_EXT2F_A9>,
>  						 <CLK_ICN_LMI>,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH V4 01/10] acpi: apei: read ack upon ghes record consumption
From: Suzuki K Poulose @ 2016-10-24  8:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477071013-29563-2-git-send-email-tbaicar@codeaurora.org>

On 21/10/16 18:30, Tyler Baicar wrote:
> A RAS (Reliability, Availability, Serviceability) controller
> may be a separate processor running in parallel with OS
> execution, and may generate error records for consumption by
> the OS. If the RAS controller produces multiple error records,
> then they may be overwritten before the OS has consumed them.
>
> The Generic Hardware Error Source (GHES) v2 structure
> introduces the capability for the OS to acknowledge the
> consumption of the error record generated by the RAS
> controller. A RAS controller supporting GHESv2 shall wait for
> the acknowledgment before writing a new error record, thus
> eliminating the race condition.
>
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Signed-off-by: Richard Ruigrok <rruigrok@codeaurora.org>
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
> ---
>  drivers/acpi/apei/ghes.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/apei/hest.c |  7 +++++--
>  include/acpi/ghes.h      |  5 ++++-
>  3 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 60746ef..7d020b0 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -45,6 +45,7 @@
>  #include <linux/aer.h>
>  #include <linux/nmi.h>
>
> +#include <acpi/actbl1.h>
>  #include <acpi/ghes.h>
>  #include <acpi/apei.h>
>  #include <asm/tlbflush.h>
> @@ -79,6 +80,10 @@
>  	((struct acpi_hest_generic_status *)				\
>  	 ((struct ghes_estatus_node *)(estatus_node) + 1))
>
> +#define HEST_TYPE_GENERIC_V2(ghes)				\
> +	((struct acpi_hest_header *)ghes->generic)->type ==	\
> +	 ACPI_HEST_TYPE_GENERIC_ERROR_V2
> +
>  /*
>   * This driver isn't really modular, however for the time being,
>   * continuing to use module_param is the easiest way to remain
> @@ -248,7 +253,15 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
>  	ghes = kzalloc(sizeof(*ghes), GFP_KERNEL);
>  	if (!ghes)
>  		return ERR_PTR(-ENOMEM);
> +
>  	ghes->generic = generic;
> +	if (HEST_TYPE_GENERIC_V2(ghes)) {
> +		rc = apei_map_generic_address(
> +			&ghes->generic_v2->read_ack_register);
> +		if (rc)
> +			goto err_unmap;

I think should be goto err_free, see more below.

> +	}
> +
>  	rc = apei_map_generic_address(&generic->error_status_address);
>  	if (rc)
>  		goto err_free;
> @@ -270,6 +283,9 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
>
>  err_unmap:
>  	apei_unmap_generic_address(&generic->error_status_address);
> +	if (HEST_TYPE_GENERIC_V2(ghes))
> +		apei_unmap_generic_address(
> +			&ghes->generic_v2->read_ack_register);

We might end up trying to unmap (error_status_address) which is not mapped
if we hit the error in mapping read_ack_register. The read_ack_register unmap
hunk should be moved below to err_free.


>  err_free:
>  	kfree(ghes);
>  	return ERR_PTR(rc);
> @@ -279,6 +295,9 @@ static void ghes_fini(struct ghes *ghes)
>  {
>  	kfree(ghes->estatus);
>  	apei_unmap_generic_address(&ghes->generic->error_status_address);
> +	if (HEST_TYPE_GENERIC_V2(ghes))
> +		apei_unmap_generic_address(
> +			&ghes->generic_v2->read_ack_register);
>  }
>
>  static inline int ghes_severity(int severity)
> @@ -648,6 +667,23 @@ static void ghes_estatus_cache_add(
>  	rcu_read_unlock();
>  }
>

> +static int ghes_do_read_ack(struct acpi_hest_generic_v2 *generic_v2)

nit: We are actually writing something to the read_ack_register. The names
read_ack_register (which may be as per standard) and more importantly the
function name (ghes_do_read_ack) sounds a bit misleading.

Rest looks fine to me.

Suzuki

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox