* [PATCH 1/2] dma-mapping: let arch know origin of dma range passed to arch_setup_dma_ops()
From: Arnd Bergmann @ 2017-01-12 13:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170112121623.GH1771@arm.com>
On Thursday, January 12, 2017 12:16:24 PM CET Will Deacon wrote:
> On Thu, Jan 12, 2017 at 08:52:51AM +0300, Nikita Yushchenko wrote:
> > >> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> > >> index 5ac373c..480b644 100644
> > >> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> > >> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> > >> @@ -540,7 +540,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
> > >>
> > >> /* Objects are coherent, unless 'no shareability' flag set. */
> > >> if (!(obj_desc->flags & DPRC_OBJ_FLAG_NO_MEM_SHAREABILITY))
> > >> - arch_setup_dma_ops(&mc_dev->dev, 0, 0, NULL, true);
> > >> + arch_setup_dma_ops(&mc_dev->dev, 0, 0, false, NULL, true);
> > >>
> > >> /*
> > >> * The device-specific probe callback will get invoked by device_add()
> > >
> > > Why are these actually calling arch_setup_dma_ops() here in the first
> > > place? Are these all devices that are DMA masters without an OF node?
> >
> > I don't know, but that's a different topic. This patch just adds
> > argument and sets it to false everywhere but in the location when range
> > should be definitely enforced.
>
> I also wouldn't lose any sleep over a staging driver.
I think this is in the process of being moved out of staging, and
my question was about the other two as well:
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
drivers/iommu/rockchip-iommu.c
Arnd
^ permalink raw reply
* [PATCH 1/2] dma-mapping: let arch know origin of dma range passed to arch_setup_dma_ops()
From: Arnd Bergmann @ 2017-01-12 13:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7d7dc402-e527-c581-0b1c-37713759db31@cogentembedded.com>
On Thursday, January 12, 2017 9:33:32 AM CET Nikita Yushchenko wrote:
> >> Hmm, I think when the dma-ranges are missing, we should either enforce
> >> a 32-bit mask, or disallow DMA completely. It's probably too late for
> >> the latter, I wish we had done this earlier in order to force everyone
> >> on ARM64 to have a valid dma-ranges property for any DMA master.
> >
> > This can be done over time.
> >
> > However the very idea of this version of patch is - keep working pieces
> > as-is, thus for now setting enforce_range to false in case of no defined
> > dma-ranges is intentional.
>
> What we can do is - check bus width (as it is defined in DT) and set
> enforce_range to true if bus is 32-bit
>
> > What I should re-check is - does rcar dtsi set dma-ranges, and add it if
> > it does not.
>
> It does not, will have to add.
>
> In DT bus is defined as 64-bit. But looks like physically it is 32-bit.
> Maybe DT needs fixing.
I think we always assumed that the lack of a dma-ranges property
implied a 32-bit width, as that is the safe fallback as well as the
most common case.
AFAICT, this means you are actually fine on rcar, and all other
platforms will keep working as we enforce it, but might get slowed
down if they relied on the unintended behavior of allowing 64-bit
DMA.
Arnd
^ permalink raw reply
* [PATCH 1/2] dma-mapping: let arch know origin of dma range passed to arch_setup_dma_ops()
From: Nikita Yushchenko @ 2017-01-12 13:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3013606.q8hexDUEqF@wuerfel>
>>>> Hmm, I think when the dma-ranges are missing, we should either enforce
>>>> a 32-bit mask, or disallow DMA completely. It's probably too late for
>>>> the latter, I wish we had done this earlier in order to force everyone
>>>> on ARM64 to have a valid dma-ranges property for any DMA master.
>>>
>>> This can be done over time.
>>>
>>> However the very idea of this version of patch is - keep working pieces
>>> as-is, thus for now setting enforce_range to false in case of no defined
>>> dma-ranges is intentional.
>>
>> What we can do is - check bus width (as it is defined in DT) and set
>> enforce_range to true if bus is 32-bit
>>
>>> What I should re-check is - does rcar dtsi set dma-ranges, and add it if
>>> it does not.
>>
>> It does not, will have to add.
>>
>> In DT bus is defined as 64-bit. But looks like physically it is 32-bit.
>> Maybe DT needs fixing.
>
> I think we always assumed that the lack of a dma-ranges property
> implied a 32-bit width, as that is the safe fallback as well as the
> most common case.
Yes we assumed that, but that was combined with blindly accepting wider
dma_mask per driver's request. Later is being changed.
> AFAICT, this means you are actually fine on rcar, and all other
> platforms will keep working as we enforce it, but might get slowed
> down if they relied on the unintended behavior of allowing 64-bit
> DMA.
Yesterday Robin raised issue that a change starting to enforce default
dma_mask will break existing setups - i.e. those that depend in 64bit
DMA being implicitly supported without manually declaring such support.
In reply to that, I suggested this version of patchset that should keep
existing behavior by default.
I'm fine with both approaches regarding behavior on hw that I don't have
- but I'm not in position to make any decisions on that.
^ permalink raw reply
* [PATCH] PCI:MSI Return -ENOSPC when requested vectors is not enough
From: Christoph Hellwig @ 2017-01-12 13:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170111181853.GA14532@bhelgaas-glaptop.roam.corp.google.com>
On Wed, Jan 11, 2017 at 12:18:53PM -0600, Bjorn Helgaas wrote:
> Unless Christoph objects, I'll apply this, but I don't understand the
> situation with 17a51f12cfbd. That commit doesn't check for EINVAL or
> ENOSPC so I don't know what the connection with this patch is.
I don't think that commit is the culprit.
> I know Christoph said he changed something in ahci to treat all errors
> the same, but I don't know where that is, either.
"ahci: always fall back to single-MSI mode"
^ permalink raw reply
* [PATCH 1/2] dma-mapping: let arch know origin of dma range passed to arch_setup_dma_ops()
From: Robin Murphy @ 2017-01-12 13:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3273119.LjYLKcrNcz@wuerfel>
On 12/01/17 13:25, Arnd Bergmann wrote:
> On Thursday, January 12, 2017 12:16:24 PM CET Will Deacon wrote:
>> On Thu, Jan 12, 2017 at 08:52:51AM +0300, Nikita Yushchenko wrote:
>>>>> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>>>>> index 5ac373c..480b644 100644
>>>>> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>>>>> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>>>>> @@ -540,7 +540,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>>>>>
>>>>> /* Objects are coherent, unless 'no shareability' flag set. */
>>>>> if (!(obj_desc->flags & DPRC_OBJ_FLAG_NO_MEM_SHAREABILITY))
>>>>> - arch_setup_dma_ops(&mc_dev->dev, 0, 0, NULL, true);
>>>>> + arch_setup_dma_ops(&mc_dev->dev, 0, 0, false, NULL, true);
>>>>>
>>>>> /*
>>>>> * The device-specific probe callback will get invoked by device_add()
>>>>
>>>> Why are these actually calling arch_setup_dma_ops() here in the first
>>>> place? Are these all devices that are DMA masters without an OF node?
>>>
>>> I don't know, but that's a different topic. This patch just adds
>>> argument and sets it to false everywhere but in the location when range
>>> should be definitely enforced.
>>
>> I also wouldn't lose any sleep over a staging driver.
>
> I think this is in the process of being moved out of staging, and
> my question was about the other two as well:
The fsl-mc is actually a sort-of-bus-controller probing and configuring
its (directly DMA-capable) child devices, so is in fact legitimate here.
> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
That one is completely bogus, and should just go away.
> drivers/iommu/rockchip-iommu.c
That one is part of some ugly trickery involving creating a fake device
to represent multiple separate IOMMU devices. The driver could probably
be reworked to not need it (the Exynos IOMMU handles a similar situation
without such tricks), but it's non-trivial.
Robin.
>
> Arnd
>
^ permalink raw reply
* [PATCH v8 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2017-01-12 13:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170112120309.fmrt2lwz3vklqmti@pengutronix.de>
2017-01-12 13:03 GMT+01:00 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> Hello Cedric,
>
> On Thu, Jan 12, 2017 at 12:23:12PM +0100, M'boumba Cedric Madianga wrote:
>> 2017-01-11 16:39 GMT+01:00 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
>> > On Wed, Jan 11, 2017 at 02:58:44PM +0100, M'boumba Cedric Madianga wrote:
>> >> 2017-01-11 9:22 GMT+01:00 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
>> >> > This is surprising. I didn't recheck the manual, but that looks very
>> >> > uncomfortable.
>> >>
>> >> I agree but this exactly the hardware way of working described in the
>> >> reference manual.
>> >
>> > IMHO that's a hw bug. This makes it for example impossible to implement
>> > SMBus block transfers (I think).
>>
>> This is not correct.
>> Setting STOP/START bit does not mean the the pulse will be sent right now.
>> Here we have just to prepare the hardware for the 2 next pulse but the
>> STOP/START/ACK pulse will be generated at the right time as required
>> by I2C specification.
>> So SMBus block transfer will be possible.
>
> A block transfer consists of a byte that specifies the count of bytes
> yet to come. So the device sends for example:
>
> 0x01 0xab
>
> So when you read the 1 in the first byte it's already too late to set
> STOP to get it after the 2nd byte.
>
> Not sure I got all the required details right, though.
Ok I understand your use case but I always think that the harware manages it.
If I take the above example, the I2C SMBus block read transaction will
be as below:
S Addr Wr [A] Comm [A]
S Addr Rd [A] [Count] A [Data1] A [Data2] NA P
The first message is a single byte-transmission so there is no problem.
The second message is a N-byte reception with N = 3
When the I2C controller has finished to send the device address (S
Addr Rd), the ADDR flag is set and an interrupt is raised.
In the routine that handles ADDR event, we set ACK bit in order to
generate ACK pulse as soon as a data byte is received in the shift
register and then we clear the ADDR flag.
Please note that the SCL line is stretched low until ADDR flag is cleared.
So, as far I understand, the device could not sent any data as long as
the SCL line is stretched low. Right ?
Then, as soon as the SCL line is high, the device could send the first
data byte (Count).
When this byte is received in the shift register, an ACK is
automatically generated as defined during adress match phase and the
data byte is pushed in DR (data register).
Then, an interrupt is raised as RXNE (RX not empty) flag is set.
In the routine that handles RXNE event, as N=3, we just clear all
buffer interrupts in order to avoid another system preemption due to
RXNE event but we does not read the data in DR.
After receiving the ACK, the device could send the second data byte (Data1).
When this byte is received in the shift register, an ACK is
automatically generated.
As the first data byte has not been read yet in DR, the BTF (Byte
Transfer Finihed) flag is set and an interrupt is raised.
So, in that case, the SCL line is also streched low as long as the
data register has not been read.
In the routine that handle BTF event, we enable NACK in order to
generate this pulse as soon as the last data byte will be received and
then we read DR register ([Count])
At this moment, SCL line is released and the device could send the
last data byte.
After receiving the ACK, the device could send the third and last data
byte (Data2)
When this byte is received in the shift register, a NACK is
automatically generated as we enable it as explained above.
As the second data byte (Data1) has not been read yet in DR, the BTF
flag is set again and an interrupt is raised.
The SCL line is stretched low and in that way we could set the STOP
bit to generate this pulse.
Then we run 2 consecutives read of DR to retrieve [Data1] and [Data2]
and to set SCL high.
So, thanks to SCL stretching, it seems that NA and P will be generated
at the right time.
Please let me know if it is not correct.
Best regards,
Cedric
^ permalink raw reply
* [PATCH] usb: dwc3: exynos: Remove MODULE_ALIAS()
From: Javier Martinez Canillas @ 2017-01-12 13:59 UTC (permalink / raw)
To: linux-arm-kernel
Exynos is DT-only, so there's no need for a platform MODALIAS.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/usb/dwc3/dwc3-exynos.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index e27899bb5706..2e555a69c8ab 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -288,7 +288,6 @@ static struct platform_driver dwc3_exynos_driver = {
module_platform_driver(dwc3_exynos_driver);
-MODULE_ALIAS("platform:exynos-dwc3");
MODULE_AUTHOR("Anton Tikhomirov <av.tikhomirov@samsung.com>");
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("DesignWare USB3 EXYNOS Glue Layer");
--
2.7.4
^ permalink raw reply related
* [PATCH v5 3/3] dmaengine: xilinx_dma: Fix race condition in the driver for multiple descriptor scenario
From: Appana Durga Kedareswara Rao @ 2017-01-12 14:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110084917.GA3573@localhost>
Hi Vinod,
Thanks for the review...
> On Sat, Jan 07, 2017 at 12:15:30PM +0530, Kedareswara rao Appana wrote:
> > When driver is handling AXI DMA SoftIP When user submits multiple
> > descriptors back to back on the S2MM(recv) side with the current
> > driver flow the last buffer descriptor next bd points to a invalid
> > location resulting the invalid data or errors in the DMA engine.
>
> Can you rephrase this, it a bit hard to understand.
When DMA is receiving packets h/w expects the descriptors
Should be in the form of a ring (I mean h/w buffer descriptor
Next descriptor field should always point to valid address
So that when DMA engine go and fetch that next descriptor it always
Sees a valid address).
But with the current driver implementation when user queues
Multiple descriptors the last descriptor next descriptor field
Pointing to an invalid location causing data corruption or
Errors from the DMA h/w engine...
To avoid this issue creating a Buffer descriptor Chain during
Channel allocation and using those buffer descriptors for processing
User requested data.
Please let me know if the above explanation is not clear will explain in detail....
>
> >
> > This patch fixes this issue by creating a BD Chain during
>
> whats a BD?
Buffer descriptor.
>
> > channel allocation itself and use those BD's.
> >
> > Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> > ---
> >
> > drivers/dma/xilinx/xilinx_dma.c | 133
> > +++++++++++++++++++++++++---------------
> > 1 file changed, 83 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/dma/xilinx/xilinx_dma.c
> > b/drivers/dma/xilinx/xilinx_dma.c index 0e9c02e..af2159d 100644
> > --- a/drivers/dma/xilinx/xilinx_dma.c
> > +++ b/drivers/dma/xilinx/xilinx_dma.c
> > @@ -163,6 +163,7 @@
> > #define XILINX_DMA_BD_SOP BIT(27)
> > #define XILINX_DMA_BD_EOP BIT(26)
> > #define XILINX_DMA_COALESCE_MAX 255
> > +#define XILINX_DMA_NUM_DESCS 255
>
> why 255?
It is not an h/w limitation
Allocating 255 descriptors (Each descriptor is capable of sending 7MB data)
So roughly using allocated descriptors DMA engine can transfer 1GB data
And in the driver we are reusing the allocated descriptors when they are free.
Regards,
Kedar.
^ permalink raw reply
* [PATCH v5 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma
From: Appana Durga Kedareswara Rao @ 2017-01-12 14:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110062620.GZ3573@localhost>
Hi Vinod,
Thanks for the review...
>
> On Sat, Jan 07, 2017 at 12:15:29PM +0530, Kedareswara rao Appana wrote:
> > When VDMA is configured for more than one frame in the h/w for example
> > h/w is configured for n number of frames and user Submits n number of
> > frames and triggered the DMA using issue_pending API.
>
> title case in middle if sentence, no commas, can you make it easier to read
> please..
Ok sure will fix commit message in the next version.
>
> > In the current driver flow we are submitting one frame at a time but
> > we should submit all the n number of frames at one time as the h/w Is
> > configured for n number of frames.
>
> s/Is/is
Ok sure will fix in the next version....
Regards,
Kedar.
^ permalink raw reply
* [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
From: Jean-Jacques Hiblot @ 2017-01-12 14:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170112001957.GA5458@nuc>
2017-01-12 1:19 GMT+01:00 Abel Vesa <abelvesa@gmail.com>:
> On Tue, Jan 10, 2017 at 04:51:12PM +0100, Petr Mladek wrote:
>> On Thu 2016-12-08 22:46:55, Abel Vesa wrote:
>> > On Thu, Dec 08, 2016 at 09:46:35PM +0000, Abel Vesa wrote:
>> > > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>> > >
>> > > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>> >
>> > >From statement twice in the commit message. Will resend.
>> > >
>> > > The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
>> > > operation to specify if registers need to saved/restored by the ftrace handler.
>> > > This is needed by kgraft and possibly other ftrace-based tools, and the ARM
>> > > architecture is currently lacking this feature. It would also be the first step
>> > > to support the "Kprobes-on-ftrace" optimization on ARM.
>> > >
>> > > This patch introduces a new ftrace handler that stores the registers on the
>> > > stack before calling the next stage. The registers are restored from the stack
>> > > before going back to the instrumented function.
>> > >
>> > > A side-effect of this patch is to activate the support for ftrace_modify_call()
>> > > as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture
>> > >
>> > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>> > > Signed-off-by: Abel Vesa <abelvesa@linux.com>
>> > > ---
>> > > arch/arm/Kconfig | 2 ++
>> > > arch/arm/include/asm/ftrace.h | 4 +++
>> > > arch/arm/kernel/entry-ftrace.S | 78 ++++++++++++++++++++++++++++++++++++++++++
>> > > arch/arm/kernel/ftrace.c | 33 ++++++++++++++++++
>> > > 4 files changed, 117 insertions(+)
>> > >
>> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> > > index b5d529f..87f1a9f 100644
>> > > --- a/arch/arm/Kconfig
>> > > +++ b/arch/arm/Kconfig
>> > > @@ -50,6 +50,7 @@ config ARM
>> > > select HAVE_DMA_API_DEBUG
>> > > select HAVE_DMA_CONTIGUOUS if MMU
>> > > select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
>> > > + select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
>> > > select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
>> > > select HAVE_EXIT_THREAD
>> > > select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
>> > > @@ -90,6 +91,7 @@ config ARM
>> > > select PERF_USE_VMALLOC
>> > > select RTC_LIB
>> > > select SYS_SUPPORTS_APM_EMULATION
>> > > + select FRAME_POINTER if DYNAMIC_FTRACE_WITH_REGS && FUNCTION_GRAPH_TRACER
> Hi Petr,
>>
>> FRAME_POINTER is not for free. It takes space on the stack. Also there
>> is a performance penalty. Do we really need to depend on it? If so,
>> it might be worth a note in the commit message.
>
FRAME_POINTER is not needed. the dependency is wrong and should be removed.
The code must be modified to not use fp register:
--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -130,7 +130,8 @@ ftrace_graph_regs_call:
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
.macro __ftrace_graph_regs_caller
- sub r0, fp, #4 @ lr of instrumented
routine (parent)
+ add r0, sp, #64 @ r0 is now a pointer to lr of
+ @ instrumented routine
@ called from __ftrace_regs_caller
ldr r1, [sp, #56] @ instrumented routine (func)
@@ -139,8 +140,9 @@ ftrace_graph_regs_call:
mov r2, fp @ frame pointer
bl prepare_ftrace_return
- ldr lr, [fp, #-4] @ restore lr from the stack
- ldmia sp, {r0-r11, ip, sp} @ restore r0 through sp
+ ldr lr, [sp, #64] @ get the previous LR value from stack
+ ldmia sp, {r0-r11, ip, sp} @ pop the saved registers INCLUDING
+ @ the stack pointer
ret ip
.endm
#endif
Jean-Jacques
> I was trying to create my own patch when I found this work done by
> Jean-Jacques, so I haven't looked specifically for the FRAME_POINTER
> part. I looked now at it and you seem to be right, FRAME_POINTER is
> not needed.
>
> I will get rid of the FRAME_POINTER part, change the authorship and
> send it again in the following days.
>>
>> I made only a quick look at the patch. It looks reasonable. But I do
>> not have enough knowledge about the arm architecture, assembly, and
>> ftrace-specifics. Also I cannot test it easily. So issues might
>> be hidden to my eyes.
>>
>> Best Regards,
>> Petr
> Thanks,
> Abel
^ permalink raw reply
* [PATCH 4/4] ARM: dts: sun8i: add OTG function to Lichee Pi Zero
From: Bin Liu @ 2017-01-12 14:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170111210638.ppdnpzdd2l6x4lyo@lukather>
On Wed, Jan 11, 2017 at 10:06:38PM +0100, Maxime Ripard wrote:
> On Wed, Jan 11, 2017 at 02:08:11PM -0600, Bin Liu wrote:
> > On Thu, Jan 12, 2017 at 03:55:33AM +0800, Icenowy Zheng wrote:
> > >
> > >
> > > 11.01.2017, 04:24, "Bin Liu" <b-liu@ti.com>:
> > > > On Tue, Jan 03, 2017 at 11:25:34PM +0800, Icenowy Zheng wrote:
> > > >> ?Lichee Pi Zero features a USB OTG port.
> > > >>
> > > >> ?Add support for it.
> > > >>
> > > >> ?Note: in order to use the Host mode, the board must be powered via the
> > > >> ?+5V and GND pins.
> > > >>
> > > >> ?Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> > > >> ?---
> > > >> ??arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts | 10 ++++++++++
> > > >> ??1 file changed, 10 insertions(+)
> > > >>
> > > >> ?diff --git a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > >> ?index 0099affc6ce3..3d9168cbaeca 100644
> > > >> ?--- a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > >> ?+++ b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > >> ?@@ -71,3 +71,13 @@
> > > >> ??????????pinctrl-names = "default";
> > > >> ??????????status = "okay";
> > > >> ??};
> > > >> ?+
> > > >> ?+&usb_otg {
> > > >> ?+ dr_mode = "otg";
> > > >
> > > > Why not set this default mode in dtsi instead?
> > > >
> > > > Regards,
> > > > -Bin.
> > >
> > > There's possibly boards which do not have OTG functions.
> >
> > That is board specific.
>
> Exactly, and this is why it should be done in the board DT.
I am just suggesting based on the common practice. If a .dtsi exists for
a family, the .dtsi describes the device and common properties for all
possible boards, and each board .dts adds or overrides its specific
implementation. Kernel has many devices/boards done in this way - define
the default dr_mode in .dtsi.
In this case, I suggest to set the common dr_mode in .dtsi, then each
board .dts only overrides it if the implementation is different.
>
> The controller in the Allwinner SoCs do not handle directly the ID pin
> and VBUS, but rather rely on a GPIO to do so.
>
> So boards with OTG will need setup anyway, at least to tell which
> GPIOs are used. There's no point in enforcing a default if it doesn't
> work by default.
Then define a default which supposes to work for most boards.
Why I suggest this, is because defining a default dr_mode which works
for most cases in dtsi could prevent a little surprise in MUSB function.
If someone designs a new board but forgets to define dr_mode in the new
board DT, the MUSB driver will default to org mode, which might not be
intended.
Regards,
-Bin.
^ permalink raw reply
* [PATCH -next] soc: ti: wkup_m3_ipc: Fix error return code in wkup_m3_ipc_probe()
From: Wei Yongjun @ 2017-01-12 14:53 UTC (permalink / raw)
To: linux-arm-kernel
From: Wei Yongjun <weiyongjun1@huawei.com>
Fix to return a negative error code from the kthread_run() error
handling case instead of 0, as done elsewhere in this function.
Fixes: cdd5de500b2c ("soc: ti: Add wkup_m3_ipc driver")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/soc/ti/wkup_m3_ipc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
index 8bfa44b..369aef5 100644
--- a/drivers/soc/ti/wkup_m3_ipc.c
+++ b/drivers/soc/ti/wkup_m3_ipc.c
@@ -457,6 +457,7 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
if (IS_ERR(task)) {
dev_err(dev, "can't create rproc_boot thread\n");
+ ret = PTR_ERR(task);
goto err_put_rproc;
}
^ permalink raw reply related
* CONFIG_PCIEASPM breaks PCIe on Marvell Armada 385 machine
From: Bjorn Helgaas @ 2017-01-12 15:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9f48e2a8-e1e0-e3db-af48-12297f8521f2@kleine-koenig.org>
On Thu, Jan 12, 2017 at 02:18:46PM +0100, Uwe Kleine-K?nig wrote:
> On 01/11/2017 11:02 PM, Bjorn Helgaas wrote:
> > Hi Uwe,
> >
> > On Wed, Jan 11, 2017 at 08:49:46PM +0100, Uwe Kleine-K?nig wrote:
> >> Hello,
> >>
> >> on an Marvell Armada 385 based machine (Turris Omnia) with 4.9 the
> >> ath10k driver fails to bind to the matching hardware if CONFIG_PCIEASPM
> >> is enabled:
> >>
> >> [...]
> > We have several open issues related to ASPM:
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=102311 ASPM: ASMEDA asm1062 not working
> > https://bugzilla.kernel.org/show_bug.cgi?id=187731 Null pointer dereference in ASPM
> > https://bugzilla.kernel.org/show_bug.cgi?id=189951 Enabling ASPM causes NIC performance issue
> > https://bugzilla.kernel.org/show_bug.cgi?id=60111 NULL pointer deref in ASPM alloc_pcie_link_state()
> >
> > I don't recognize yours as being one of these. Can you open a new
> > issue and attach the complete dmesg log and "lspci -vv" output?
>
> Done: https://bugzilla.kernel.org/show_bug.cgi?id=192441
Thanks! Can you attach a dmesg with CONFIG_PCIEASPM turned off, too?
There are several interesting things going on with that ath10k device,
and not all of them seem ASPM-related:
pci_bus 0000:00: root bus resource [mem 0xe0000000-0xe7ffffff]
pci 0000:02:00.0: reg 0x10: [mem 0xe8000000-0xe81fffff 64bit]
pci 0000:02:00.0: reg 0x30: [mem 0xe8200000-0xe820ffff pref]
pci 0000:02:00.0: of_irq_parse_pci() failed with rc=134
pci 0000:02:00.0: BAR 0: assigned [mem 0xe0000000-0xe01fffff 64bit]
pci 0000:02:00.0: BAR 0: error updating (0xe0000004 != 0xffffffff)
pci 0000:02:00.0: BAR 0: error updating (high 0x000000 != 0xffffffff)
1) We found BAR 0 (reg 0x10) with 0xe8000000, so firmware probably
programmed it, and it probably works there.
2) The host bridge window doesn't include that BAR 0 space.
Unfortunately I don't think we print the initial 00:02.0 bridge
window leading to bus 02; we only print the new window we assign to
it.
3) No idea what the of_irq_parse_pci() issue is.
4) No idea why the BAR 0 update failed. Maybe a Marvell config
accessor problem?
I don't see any connection between these and ASPM, so I'm curious why
things work with CONFIG_PCIEASPM turned off.
^ permalink raw reply
* [PATCH -next] crypto: mediatek - make symbol of_crypto_id static
From: Wei Yongjun @ 2017-01-12 15:03 UTC (permalink / raw)
To: linux-arm-kernel
From: Wei Yongjun <weiyongjun1@huawei.com>
Fixes the following sparse warning:
drivers/crypto/mediatek/mtk-platform.c:585:27: warning:
symbol 'of_crypto_id' was not declared. Should it be static?
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/crypto/mediatek/mtk-platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/mediatek/mtk-platform.c b/drivers/crypto/mediatek/mtk-platform.c
index 286296f..a9c713d 100644
--- a/drivers/crypto/mediatek/mtk-platform.c
+++ b/drivers/crypto/mediatek/mtk-platform.c
@@ -582,7 +582,7 @@ static int mtk_crypto_remove(struct platform_device *pdev)
return 0;
}
-const struct of_device_id of_crypto_id[] = {
+static const struct of_device_id of_crypto_id[] = {
{ .compatible = "mediatek,eip97-crypto" },
{},
};
^ permalink raw reply related
* kvm: deadlock in kvm_vgic_map_resources
From: Christoffer Dall @ 2017-01-12 15:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a4acacfb-8178-b904-f27d-235fc4a58275@arm.com>
On Thu, Jan 12, 2017 at 10:50:04AM +0000, Marc Zyngier wrote:
> On 12/01/17 10:42, Christoffer Dall wrote:
> > On Thu, Jan 12, 2017 at 10:30:39AM +0000, Marc Zyngier wrote:
> >> On 12/01/17 09:55, Andre Przywara wrote:
> >>> Hi,
> >>>
> >>> On 12/01/17 09:32, Marc Zyngier wrote:
> >>>> Hi Dmitry,
> >>>>
> >>>> On 11/01/17 19:01, Dmitry Vyukov wrote:
> >>>>> Hello,
> >>>>>
> >>>>> While running syzkaller fuzzer I've got the following deadlock.
> >>>>> On commit 9c763584b7c8911106bb77af7e648bef09af9d80.
> >>>>>
> >>>>>
> >>>>> =============================================
> >>>>> [ INFO: possible recursive locking detected ]
> >>>>> 4.9.0-rc6-xc2-00056-g08372dd4b91d-dirty #50 Not tainted
> >>>>> ---------------------------------------------
> >>>>> syz-executor/20805 is trying to acquire lock:
> >>>>> (
> >>>>> &kvm->lock
> >>>>> ){+.+.+.}
> >>>>> , at:
> >>>>> [< inline >] kvm_vgic_dist_destroy
> >>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-init.c:271
> >>>>> [<ffff2000080ea4bc>] kvm_vgic_destroy+0x34/0x250
> >>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-init.c:294
> >>>>> but task is already holding lock:
> >>>>> (&kvm->lock){+.+.+.}, at:
> >>>>> [<ffff2000080ea7e4>] kvm_vgic_map_resources+0x2c/0x108
> >>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-init.c:343
> >>>>> other info that might help us debug this:
> >>>>> Possible unsafe locking scenario:
> >>>>> CPU0
> >>>>> ----
> >>>>> lock(&kvm->lock);
> >>>>> lock(&kvm->lock);
> >>>>> *** DEADLOCK ***
> >>>>> May be due to missing lock nesting notation
> >>>>> 2 locks held by syz-executor/20805:
> >>>>> #0:(&vcpu->mutex){+.+.+.}, at:
> >>>>> [<ffff2000080bcc30>] vcpu_load+0x28/0x1d0
> >>>>> arch/arm64/kvm/../../../virt/kvm/kvm_main.c:143
> >>>>> #1:(&kvm->lock){+.+.+.}, at:
> >>>>> [<ffff2000080ea7e4>] kvm_vgic_map_resources+0x2c/0x108
> >>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-init.c:343
> >>>>> stack backtrace:
> >>>>> CPU: 2 PID: 20805 Comm: syz-executor Not tainted
> >>>>> 4.9.0-rc6-xc2-00056-g08372dd4b91d-dirty #50
> >>>>> Hardware name: Hardkernel ODROID-C2 (DT)
> >>>>> Call trace:
> >>>>> [<ffff200008090560>] dump_backtrace+0x0/0x3c8 arch/arm64/kernel/traps.c:69
> >>>>> [<ffff200008090948>] show_stack+0x20/0x30 arch/arm64/kernel/traps.c:219
> >>>>> [< inline >] __dump_stack lib/dump_stack.c:15
> >>>>> [<ffff200008895840>] dump_stack+0x100/0x150 lib/dump_stack.c:51
> >>>>> [< inline >] print_deadlock_bug kernel/locking/lockdep.c:1728
> >>>>> [< inline >] check_deadlock kernel/locking/lockdep.c:1772
> >>>>> [< inline >] validate_chain kernel/locking/lockdep.c:2250
> >>>>> [<ffff2000081c8718>] __lock_acquire+0x1938/0x3440 kernel/locking/lockdep.c:3335
> >>>>> [<ffff2000081caa84>] lock_acquire+0xdc/0x1d8 kernel/locking/lockdep.c:3746
> >>>>> [< inline >] __mutex_lock_common kernel/locking/mutex.c:521
> >>>>> [<ffff200009700004>] mutex_lock_nested+0xdc/0x7b8 kernel/locking/mutex.c:621
> >>>>> [< inline >] kvm_vgic_dist_destroy
> >>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-init.c:271
> >>>>> [<ffff2000080ea4bc>] kvm_vgic_destroy+0x34/0x250
> >>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-init.c:294
> >>>>> [<ffff2000080ec290>] vgic_v2_map_resources+0x218/0x430
> >>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-v2.c:295
> >>>>> [<ffff2000080ea884>] kvm_vgic_map_resources+0xcc/0x108
> >>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-init.c:348
> >>>>> [< inline >] kvm_vcpu_first_run_init
> >>>>> arch/arm64/kvm/../../../arch/arm/kvm/arm.c:505
> >>>>> [<ffff2000080d2768>] kvm_arch_vcpu_ioctl_run+0xab8/0xce0
> >>>>> arch/arm64/kvm/../../../arch/arm/kvm/arm.c:591
> >>>>> [<ffff2000080c1fec>] kvm_vcpu_ioctl+0x434/0xc08
> >>>>> arch/arm64/kvm/../../../virt/kvm/kvm_main.c:2557
> >>>>> [< inline >] vfs_ioctl fs/ioctl.c:43
> >>>>> [<ffff200008450c38>] do_vfs_ioctl+0x128/0xfc0 fs/ioctl.c:679
> >>>>> [< inline >] SYSC_ioctl fs/ioctl.c:694
> >>>>> [<ffff200008451b78>] SyS_ioctl+0xa8/0xb8 fs/ioctl.c:685
> >>>>> [<ffff200008083ef0>] el0_svc_naked+0x24/0x28 arch/arm64/kernel/entry.S:755
> >>>>
> >>>> Nice catch, and many thanks for reporting this.
> >>>>
> >>>> The bug is fairly obvious. Christoffer, what do you think? I don't think
> >>>> we need to hold the kvm->lock all the way, but I'd like another pair of
> >>>> eyes (the coffee machine is out of order again, and tea doesn't cut it).
> >>>>
> >>>> Thanks,
> >>>>
> >>>> M.
> >>>>
> >>>> From 93f80b20fb9351a49ee8b74eed3fc59c84651371 Mon Sep 17 00:00:00 2001
> >>>> From: Marc Zyngier <marc.zyngier@arm.com>
> >>>> Date: Thu, 12 Jan 2017 09:21:56 +0000
> >>>> Subject: [PATCH] KVM: arm/arm64: vgic: Fix deadlock on error handling
> >>>>
> >>>> Dmitry Vyukov reported that the syzkaller fuzzer triggered a
> >>>> deadlock in the vgic setup code when an error was detected, as
> >>>> the cleanup code tries to take a lock that is already held by
> >>>> the setup code.
> >>>>
> >>>> The fix is pretty obvious: move the cleaup call after having
> >>>> dropped the lock, since not much can happen at that point.
> >>> ^^^^^^^^
> >>> Is that really true? If for instance the calls to
> >>> vgic_register_dist_iodev() or kvm_phys_addr_ioremap() in
> >>> vgic_v2_map_resources() fail, we leave the function with a half
> >>> initialized VGIC (because vgic_init() succeeded).
> >>
> >> But we only set dist->ready to true when everything went OK. How is
> >> that an issue?
> >>
> >>> Dropping the lock at
> >>> this point without having the GIC cleaned up before sounds a bit
> >>> suspicious (I may be wrong on this, though).
> >>
> >> Thinking of it, that may open a race with vgic init call, leading to
> >> leaking distributor memory.
> >>
> >>>
> >>> Can't we just document that kvm_vgic_destroy() needs to be called with
> >>> the kvm->lock held and take the lock around the only other caller
> >>> (kvm_arch_destroy_vm() in arch/arm/kvm/arm.c)?
> >>> We can then keep holding the lock in the map_resources calls.
> >>> Though we might still move the calls to kvm_vgic_destroy() into the
> >>> wrapper function as a cleanup (as shown below), just before dropping the
> >>> lock.
> >>
> >> I'd rather keep the changes limited to the vgic code, and save myself
> >> having to document more locking (we already have our fair share here).
> >> How about this (untested):
> >>
> >> From 24dc3f5750da20d89e0ce9b7855d125d0100bee8 Mon Sep 17 00:00:00 2001
> >> From: Marc Zyngier <marc.zyngier@arm.com>
> >> Date: Thu, 12 Jan 2017 09:21:56 +0000
> >> Subject: [PATCH] KVM: arm/arm64: vgic: Fix deadlock on error handling
> >>
> >> Dmitry Vyukov reported that the syzkaller fuzzer triggered a
> >> deadlock in the vgic setup code when an error was detected, as
> >> the cleanup code tries to take a lock that is already held by
> >> the setup code.
> >>
> >> The fix is to avoid retaking the lock when cleaning up, by
> >> telling the cleanup function that we already hold it.
> >>
> >> Cc: stable at vger.kernel.org
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >> ---
> >> virt/kvm/arm/vgic/vgic-init.c | 21 ++++++++++++++++-----
> >> virt/kvm/arm/vgic/vgic-v2.c | 2 --
> >> virt/kvm/arm/vgic/vgic-v3.c | 2 --
> >> 3 files changed, 16 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
> >> index 5114391..30d74e2 100644
> >> --- a/virt/kvm/arm/vgic/vgic-init.c
> >> +++ b/virt/kvm/arm/vgic/vgic-init.c
> >> @@ -264,11 +264,12 @@ int vgic_init(struct kvm *kvm)
> >> return ret;
> >> }
> >>
> >> -static void kvm_vgic_dist_destroy(struct kvm *kvm)
> >> +static void kvm_vgic_dist_destroy(struct kvm *kvm, bool locked)
> >> {
> >> struct vgic_dist *dist = &kvm->arch.vgic;
> >>
> >> - mutex_lock(&kvm->lock);
> >> + if (!locked)
> >> + mutex_lock(&kvm->lock);
> >
> > Hmm, not a fan of passing this variable around. How about this instead
> > then (untested):
> >
> > diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
> > index 5114391..a25806b 100644
> > --- a/virt/kvm/arm/vgic/vgic-init.c
> > +++ b/virt/kvm/arm/vgic/vgic-init.c
> > @@ -264,19 +264,16 @@ int vgic_init(struct kvm *kvm)
> > return ret;
> > }
> >
> > +/* Must be called with the kvm->lock held */
> > static void kvm_vgic_dist_destroy(struct kvm *kvm)
> > {
> > struct vgic_dist *dist = &kvm->arch.vgic;
> >
> > - mutex_lock(&kvm->lock);
> > -
> > dist->ready = false;
> > dist->initialized = false;
> >
> > kfree(dist->spis);
> > dist->nr_spis = 0;
> > -
> > - mutex_unlock(&kvm->lock);
> > }
> >
> > void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
> > @@ -286,7 +283,7 @@ void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
> > INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
> > }
> >
> > -void kvm_vgic_destroy(struct kvm *kvm)
> > +void __kvm_vgic_destroy(struct kvm *kvm)
> > {
> > struct kvm_vcpu *vcpu;
> > int i;
> > @@ -297,6 +294,13 @@ void kvm_vgic_destroy(struct kvm *kvm)
> > kvm_vgic_vcpu_destroy(vcpu);
> > }
> >
> > +void kvm_vgic_destroy(struct kvm *kvm)
> > +{
> > + mutex_lock(&kvm->lock);
> > + __kvm_vgic_destroy(kvm);
> > + mutex_unlock(&kvm->lock);
> > +}
> > +
>
> I initially wrote that exactly, but ended up deciding against as it
> changes the locking more than strictly necessary. On the other hand, I
> think this looks better, so if everyone agrees I'll take that.
>
> > /**
> > * vgic_lazy_init: Lazy init is only allowed if the GIC exposed to the guest
> > * is a GICv2. A GICv3 must be explicitly initialized by the guest using the
> > diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c
> > index 9bab867..c6f7ec7 100644
> > --- a/virt/kvm/arm/vgic/vgic-v2.c
> > +++ b/virt/kvm/arm/vgic/vgic-v2.c
> > @@ -294,7 +294,7 @@ int vgic_v2_map_resources(struct kvm *kvm)
> >
> > out:
> > if (ret)
> > - kvm_vgic_destroy(kvm);
> > + __kvm_vgic_destroy(kvm);
> > return ret;
> > }
> >
> > diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
> > index 5c9f974..f1c7819 100644
> > --- a/virt/kvm/arm/vgic/vgic-v3.c
> > +++ b/virt/kvm/arm/vgic/vgic-v3.c
> > @@ -303,7 +303,7 @@ int vgic_v3_map_resources(struct kvm *kvm)
> >
> > out:
> > if (ret)
> > - kvm_vgic_destroy(kvm);
> > + __kvm_vgic_destroy(kvm);
>
> I'm still keen on factoring the destroy calls in the calling function.
> Is there any reason why we wouldn't do it?
>
I was very slightly biased to not do it, because I feel like it was
clear that the scary function that does a lot of work cleans up nicely
after itself in case of failure with the current code, but I'm not
married to either approach, so whatever you prefer.
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH v29 3/9] arm64: kdump: reserve memory for crash dump kernel
From: Mark Rutland @ 2017-01-12 15:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161228043605.27470-2-takahiro.akashi@linaro.org>
Hi,
As a general note, I must apologise for my minimial review of the series
until this point. Judging by the way the DT parts are organised. I'm
very concerned with the way the DT parts are organised, and clearly I
did not communicate my concerns and suggestions effectively in prior
rounds of review.
On Wed, Dec 28, 2016 at 01:36:00PM +0900, AKASHI Takahiro wrote:
> "crashkernel=" kernel parameter specifies the size (and optionally
> the start address) of the system ram used by crash dump kernel.
> reserve_crashkernel() will allocate and reserve the memory at the startup
> of primary kernel.
>
> This memory range will be exported to userspace via:
> - an entry named "Crash kernel" in /proc/iomem, and
> - "linux,crashkernel-base" and "linux,crashkernel-size" under
> /sys/firmware/devicetree/base/chosen
> +#ifdef CONFIG_KEXEC_CORE
> +static unsigned long long crash_size, crash_base;
> +static struct property crash_base_prop = {
> + .name = "linux,crashkernel-base",
> + .length = sizeof(u64),
> + .value = &crash_base
> +};
> +static struct property crash_size_prop = {
> + .name = "linux,crashkernel-size",
> + .length = sizeof(u64),
> + .value = &crash_size,
> +};
> +
> +static int __init export_crashkernel(void)
> +{
> + struct device_node *node;
> + int ret;
> +
> + if (!crash_size)
> + return 0;
> +
> + /* Add /chosen/linux,crashkernel-* properties */
> + node = of_find_node_by_path("/chosen");
> + if (!node)
> + return -ENOENT;
> +
> + /*
> + * There might be existing crash kernel properties, but we can't
> + * be sure what's in them, so remove them.
> + */
> + of_remove_property(node, of_find_property(node,
> + "linux,crashkernel-base", NULL));
> + of_remove_property(node, of_find_property(node,
> + "linux,crashkernel-size", NULL));
> +
> + ret = of_add_property(node, &crash_base_prop);
> + if (ret)
> + goto ret_err;
> +
> + ret = of_add_property(node, &crash_size_prop);
> + if (ret)
> + goto ret_err;
> +
> + return 0;
> +
> +ret_err:
> + pr_warn("Exporting crashkernel region to device tree failed\n");
> + return ret;
> +}
> +late_initcall(export_crashkernel);
I very much do not like this.
I don't think we should be modifying the DT exposed to userspace in this
manner, in the usual boot path, especially given that the kernel itself
does not appear to be a consumer of this property. I do not think that
it is right to use the DT exposed to userspace as a communication
channel solely between the kernel and userspace.
So I think we should drop the above, and for arm64 have userspace
consistently use /proc/iomem (or perhaps a new kexec-specific file) to
determine the region reserved for the crash kernel, if it needs to know
this.
I'll have further comments on this front in the binding patch.
> +/*
> + * reserve_crashkernel() - reserves memory for crash kernel
> + *
> + * This function reserves memory area given in "crashkernel=" kernel command
> + * line parameter. The memory reserved is used by dump capture kernel when
> + * primary kernel is crashing.
> + */
> +static void __init reserve_crashkernel(void)
> +{
> + int ret;
> +
> + ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
> + &crash_size, &crash_base);
> + /* no crashkernel= or invalid value specified */
> + if (ret || !crash_size)
> + return;
> +
> + if (crash_base == 0) {
> + /* Current arm64 boot protocol requires 2MB alignment */
> + crash_base = memblock_find_in_range(0, ARCH_LOW_ADDRESS_LIMIT,
> + crash_size, SZ_2M);
> + if (crash_base == 0) {
> + pr_warn("Unable to allocate crashkernel (size:%llx)\n",
> + crash_size);
> + return;
> + }
> + } else {
> + /* User specifies base address explicitly. */
> + if (!memblock_is_region_memory(crash_base, crash_size) ||
> + memblock_is_region_reserved(crash_base, crash_size)) {
> + pr_warn("crashkernel has wrong address or size\n");
> + return;
> + }
> +
> + if (!IS_ALIGNED(crash_base, SZ_2M)) {
> + pr_warn("crashkernel base address is not 2MB aligned\n");
> + return;
> + }
> + }
> + memblock_reserve(crash_base, crash_size);
This will mean that the crash kernel will have a permanent alias in the linear
map which is vulnerable to being clobbered. There could also be issues
with mismatched attributes in future.
We're probably ok for now, but in future we'll likely want to fix this
up to remove the region (or mark it nomap), and only map it temporarily
when loading things into the region.
> +
> + pr_info("Reserving %lldMB of memory at %lldMB for crashkernel\n",
> + crash_size >> 20, crash_base >> 20);
> +
> + crashk_res.start = crash_base;
> + crashk_res.end = crash_base + crash_size - 1;
> +}
> +#else
> +static void __init reserve_crashkernel(void)
> +{
> + ;
Nit: the ';' line can go.
> +}
> +#endif /* CONFIG_KEXEC_CORE */
> +
> /*
> * Return the maximum physical address for ZONE_DMA (DMA_BIT_MASK(32)). It
> * currently assumes that for memory starting above 4G, 32-bit devices will
> @@ -331,6 +438,9 @@ void __init arm64_memblock_init(void)
> arm64_dma_phys_limit = max_zone_dma_phys();
> else
> arm64_dma_phys_limit = PHYS_MASK + 1;
> +
> + reserve_crashkernel();
> +
> dma_contiguous_reserve(arm64_dma_phys_limit);
>
> memblock_allow_resize();
> --
> 2.11.0
Other than my comments regarding the DT usage above, this looks fine to
me.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v3 0/3] arm64: dts: juno: CoreSight support updates
From: Sudeep Holla @ 2017-01-12 15:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mike,
I just did some refactoring to reduced thge churn and duplication.
Can you check if this work for you ? I could not give it much testing.
Regards,
Sudeep
--->8
Juno r1/r2 boards have different CoreSight infrastructure outside the
CPU clusters. This patchset adds the additional coreSight components to
separate .dtsi files to support these differences.
v1->v2:
- moved the addition of the STM component into a separate patch
v2->v3:
- moved the back the common coreSight components back into
juno-base.dtsi
Mike Leach (2):
arm64: dts: juno: fix CoreSight support for Juno r1/r2 variants
arm64: dts: juno: add missing CoreSight STM component
Sudeep Holla (1):
arm64: dts: juno: refactor CoreSight support on Juno r0
arch/arm64/boot/dts/arm/juno-base.dtsi | 31 ++++++---
arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi | 102 ++++++++++++++++++++++++++++++
arch/arm64/boot/dts/arm/juno-r1.dts | 13 ++++
arch/arm64/boot/dts/arm/juno-r2.dts | 13 ++++
arch/arm64/boot/dts/arm/juno.dts | 24 +++++++
5 files changed, 175 insertions(+), 8 deletions(-)
create mode 100644 arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
--
2.7.4
^ permalink raw reply
* [PATCH v3 1/3] arm64: dts: juno: refactor CoreSight support on Juno r0
From: Sudeep Holla @ 2017-01-12 15:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484234410-7670-1-git-send-email-sudeep.holla@arm.com>
Currently the Coresight components are supported only on Juno r0
variant. In preparation to add support to Juno r1/r2 variants, this
patch refactors the existing coresight device nodes so that r1/r2
support can be added easily.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/boot/dts/arm/juno-base.dtsi | 16 ++++++++--------
arch/arm64/boot/dts/arm/juno.dts | 8 ++++++++
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index 7d832247d0db..6d7f56573e89 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -83,7 +83,7 @@
* The actual size is just 4K though 64K is reserved. Access to the
* unmapped reserved region results in a DECERR response.
*/
- etf at 20010000 {
+ etf0 at 20010000 {
compatible = "arm,coresight-tmc", "arm,primecell";
reg = <0 0x20010000 0 0x1000>;
@@ -97,7 +97,7 @@
/* input port */
port at 0 {
reg = <0>;
- etf_in_port: endpoint {
+ etf0_in_port: endpoint {
slave-mode;
remote-endpoint = <&main_funnel_out_port>;
};
@@ -106,8 +106,7 @@
/* output port */
port at 1 {
reg = <0>;
- etf_out_port: endpoint {
- remote-endpoint = <&replicator_in_port0>;
+ etf0_out_port: endpoint {
};
};
};
@@ -128,7 +127,8 @@
};
};
- main-funnel at 20040000 {
+ /* This is labelled cssys0 funnel in the Juno r1/r2 TRM */
+ main_funnel: main-funnel at 20040000 {
compatible = "arm,coresight-funnel", "arm,primecell";
reg = <0 0x20040000 0 0x1000>;
@@ -139,13 +139,15 @@
#address-cells = <1>;
#size-cells = <0>;
+ /* output port */
port at 0 {
reg = <0>;
main_funnel_out_port: endpoint {
- remote-endpoint = <&etf_in_port>;
+ remote-endpoint = <&etf0_in_port>;
};
};
+ /* input ports */
port at 1 {
reg = <0>;
main_funnel_in_port0: endpoint {
@@ -161,7 +163,6 @@
remote-endpoint = <&cluster1_funnel_out_port>;
};
};
-
};
};
@@ -382,7 +383,6 @@
reg = <0>;
replicator_in_port0: endpoint {
slave-mode;
- remote-endpoint = <&etf_out_port>;
};
};
};
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index ac5ceb73f45f..ba10d688beaa 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -203,3 +203,11 @@
&etm5 {
cpu = <&A53_3>;
};
+
+&etf0_out_port {
+ remote-endpoint = <&replicator_in_port0>;
+};
+
+&replicator_in_port0 {
+ remote-endpoint = <&etf0_out_port>;
+};
--
2.7.4
^ permalink raw reply related
* [PATCH v3 2/3] arm64: dts: juno: fix CoreSight support for Juno r1/r2 variants
From: Sudeep Holla @ 2017-01-12 15:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484234410-7670-1-git-send-email-sudeep.holla@arm.com>
From: Mike Leach <mike.leach@linaro.org>
The CoreSight support added for Juno is valid for only Juno r0.
The Juno r1 and r2 variants have additional components and alternative
connection routes between trace source and sinks.
This patch builds on top of the existing r0 support and extends it to
Juno r1/r2 variants.
Signed-off-by: Mike Leach <mike.leach@linaro.org>
[sudeep.holla at arm.com: minor changelog update and reorganising the common
coresight components back into juno-base.dtsi to avoid duplication]
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi | 102 ++++++++++++++++++++++++++++++
arch/arm64/boot/dts/arm/juno-r1.dts | 9 +++
arch/arm64/boot/dts/arm/juno-r2.dts | 9 +++
3 files changed, 120 insertions(+)
create mode 100644 arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
diff --git a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
new file mode 100644
index 000000000000..89fcef366ff9
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
@@ -0,0 +1,102 @@
+ csys1_funnel at 20130000 {
+ compatible = "arm,coresight-funnel", "arm,primecell";
+ reg = <0 0x20130000 0 0x1000>;
+
+ clocks = <&soc_smc50mhz>;
+ clock-names = "apb_pclk";
+ power-domains = <&scpi_devpd 0>;
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* input port */
+ port at 0 {
+ reg = <0>;
+ csys1_funnel_out_port: endpoint {
+ remote-endpoint =
+ <&etf1_in_port>;
+ };
+ };
+
+ /* output port */
+ port at 1 {
+ reg = <0>;
+ csys1_funnel_in_port0: endpoint {
+ slave-mode;
+ };
+ };
+
+ };
+ };
+
+ etf1 at 20140000 {
+ compatible = "arm,coresight-tmc", "arm,primecell";
+ reg = <0 0x20140000 0 0x1000>;
+
+ clocks = <&soc_smc50mhz>;
+ clock-names = "apb_pclk";
+ power-domains = <&scpi_devpd 0>;
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* input port */
+ port at 0 {
+ reg = <0>;
+ etf1_in_port: endpoint {
+ slave-mode;
+ remote-endpoint =
+ <&csys1_funnel_out_port>;
+ };
+ };
+
+ /* output port */
+ port at 1 {
+ reg = <0>;
+ etf1_out_port: endpoint {
+ remote-endpoint =
+ <&csys2_funnel_in_port1>;
+ };
+ };
+ };
+ };
+
+ csys2_funnel at 20150000 {
+ compatible = "arm,coresight-funnel", "arm,primecell";
+ reg = <0 0x20150000 0 0x1000>;
+
+ clocks = <&soc_smc50mhz>;
+ clock-names = "apb_pclk";
+ power-domains = <&scpi_devpd 0>;
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* output port */
+ port at 0 {
+ reg = <0>;
+ csys2_funnel_out_port: endpoint {
+ remote-endpoint =
+ <&replicator_in_port0>;
+ };
+ };
+
+ /* input ports */
+ port at 1 {
+ reg = <0>;
+ csys2_funnel_in_port0: endpoint {
+ slave-mode;
+ remote-endpoint = <&etf0_out_port>;
+ };
+ };
+
+ port at 2 {
+ reg = <1>;
+ csys2_funnel_in_port1: endpoint {
+ slave-mode;
+ remote-endpoint = <&etf1_out_port>;
+ };
+ };
+
+ };
+ };
diff --git a/arch/arm64/boot/dts/arm/juno-r1.dts b/arch/arm64/boot/dts/arm/juno-r1.dts
index eec37feee8fc..d4b85d9d343e 100644
--- a/arch/arm64/boot/dts/arm/juno-r1.dts
+++ b/arch/arm64/boot/dts/arm/juno-r1.dts
@@ -178,6 +178,7 @@
};
#include "juno-base.dtsi"
+ #include "juno-cs-r1r2.dtsi"
};
&memtimer {
@@ -227,3 +228,11 @@
&gpu1_thermal_zone {
status = "okay";
};
+
+&etf0_out_port {
+ remote-endpoint = <&csys2_funnel_in_port0>;
+};
+
+&replicator_in_port0 {
+ remote-endpoint = <&csys2_funnel_out_port>;
+};
diff --git a/arch/arm64/boot/dts/arm/juno-r2.dts b/arch/arm64/boot/dts/arm/juno-r2.dts
index 28f40ec44090..14b29517065c 100644
--- a/arch/arm64/boot/dts/arm/juno-r2.dts
+++ b/arch/arm64/boot/dts/arm/juno-r2.dts
@@ -178,6 +178,7 @@
};
#include "juno-base.dtsi"
+ #include "juno-cs-r1r2.dtsi"
};
&memtimer {
@@ -227,3 +228,11 @@
&gpu1_thermal_zone {
status = "okay";
};
+
+&etf0_out_port {
+ remote-endpoint = <&csys2_funnel_in_port0>;
+};
+
+&replicator_in_port0 {
+ remote-endpoint = <&csys2_funnel_out_port>;
+};
--
2.7.4
^ permalink raw reply related
* [PATCH v3 3/3] arm64: dts: juno: add missing CoreSight STM component
From: Sudeep Holla @ 2017-01-12 15:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484234410-7670-1-git-send-email-sudeep.holla@arm.com>
From: Mike Leach <mike.leach@linaro.org>
This patch adds the missing CoreSight STM component definition to the
device tree of all the juno variants(r0,r1,r2)
STM component is connected to different funnels depending on Juno
platform variant.
Signed-off-by: Mike Leach <mike.leach@linaro.org>
[sudeep.holla at arm.com: minor changelog update and reorganising the STM
node back into juno-base.dtsi to avoid duplication]
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/boot/dts/arm/juno-base.dtsi | 15 +++++++++++++++
arch/arm64/boot/dts/arm/juno-r1.dts | 4 ++++
arch/arm64/boot/dts/arm/juno-r2.dts | 4 ++++
arch/arm64/boot/dts/arm/juno.dts | 16 ++++++++++++++++
4 files changed, 39 insertions(+)
diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index 6d7f56573e89..77fe75de5cae 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -182,6 +182,21 @@
};
};
+ stm at 20100000 {
+ compatible = "arm,coresight-stm", "arm,primecell";
+ reg = <0 0x20100000 0 0x1000>,
+ <0 0x28000000 0 0x180000>;
+ reg-names = "stm-base", "stm-stimulus-base";
+
+ clocks = <&soc_smc50mhz>;
+ clock-names = "apb_pclk";
+ power-domains = <&scpi_devpd 0>;
+ port {
+ stm_out_port: endpoint {
+ };
+ };
+ };
+
etm0: etm at 22040000 {
compatible = "arm,coresight-etm4x", "arm,primecell";
reg = <0 0x22040000 0 0x1000>;
diff --git a/arch/arm64/boot/dts/arm/juno-r1.dts b/arch/arm64/boot/dts/arm/juno-r1.dts
index d4b85d9d343e..6df25c7a6b90 100644
--- a/arch/arm64/boot/dts/arm/juno-r1.dts
+++ b/arch/arm64/boot/dts/arm/juno-r1.dts
@@ -236,3 +236,7 @@
&replicator_in_port0 {
remote-endpoint = <&csys2_funnel_out_port>;
};
+
+&stm_out_port {
+ remote-endpoint = <&csys1_funnel_in_port0>;
+};
diff --git a/arch/arm64/boot/dts/arm/juno-r2.dts b/arch/arm64/boot/dts/arm/juno-r2.dts
index 14b29517065c..dafe92c20f65 100644
--- a/arch/arm64/boot/dts/arm/juno-r2.dts
+++ b/arch/arm64/boot/dts/arm/juno-r2.dts
@@ -236,3 +236,7 @@
&replicator_in_port0 {
remote-endpoint = <&csys2_funnel_out_port>;
};
+
+&stm_out_port {
+ remote-endpoint = <&csys1_funnel_in_port0>;
+};
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index ba10d688beaa..dbec40a4c4a7 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -211,3 +211,19 @@
&replicator_in_port0 {
remote-endpoint = <&etf0_out_port>;
};
+
+&stm_out_port {
+ remote-endpoint = <&main_funnel_in_port2>;
+};
+
+&main_funnel {
+ ports {
+ port at 3 {
+ reg = <2>;
+ main_funnel_in_port2: endpoint {
+ slave-mode;
+ remote-endpoint = <&stm_out_port>;
+ };
+ };
+ };
+};
--
2.7.4
^ permalink raw reply related
* CONFIG_PCIEASPM breaks PCIe on Marvell Armada 385 machine
From: Andrew Lunn @ 2017-01-12 15:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170112150310.GA5746@bhelgaas-glaptop.roam.corp.google.com>
> pci_bus 0000:00: root bus resource [mem 0xe0000000-0xe7ffffff]
> pci 0000:02:00.0: reg 0x10: [mem 0xe8000000-0xe81fffff 64bit]
> pci 0000:02:00.0: reg 0x30: [mem 0xe8200000-0xe820ffff pref]
> pci 0000:02:00.0: of_irq_parse_pci() failed with rc=134
> pci 0000:02:00.0: BAR 0: assigned [mem 0xe0000000-0xe01fffff 64bit]
> pci 0000:02:00.0: BAR 0: error updating (0xe0000004 != 0xffffffff)
> pci 0000:02:00.0: BAR 0: error updating (high 0x000000 != 0xffffffff)
>
> 3) No idea what the of_irq_parse_pci() issue is.
134 is 0x86.
Could it be:
#define PCIBIOS_DEVICE_NOT_FOUND 0x86
pci-mvebu.c will return this in a few places, mvebu_pcie_wr_conf(),
mvebu_pcie_rd_conf().
Could this be
rc = pci_read_config_byte(pdev, PCI_INTERRUPT_PIN, &pin);
It looks like pci_read_config_byte() is expected to return a real
errno value, and maybe it is returning PCIBIOS_DEVICE_NOT_FOUND?
Andrew
^ permalink raw reply
* [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains
From: Dave Gerlach @ 2017-01-12 15:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_JsqKxU4c=KSn0=xX9kmbk498fhqpiXMeGjcoe8Ah48FAzUg@mail.gmail.com>
Rob,
On 01/11/2017 03:34 PM, Rob Herring wrote:
> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>> Rob,
>>
>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>
>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>
>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>> control device power states.
>>>>
>>>> Also, provide macros representing each device index as understood
>>>> by TI SCI to be used in the device node power-domain references.
>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>
>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>> ---
>>>> v2->v3:
>>>> Update k2g_pds node docs to show it should be a child of pmmc
>>>> node.
>>>> In early versions a phandle was used to point to pmmc and docs
>>>> still
>>>> incorrectly showed this.
>>>>
>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 ++++++++++++++
>>>> MAINTAINERS | 2 +
>>>> include/dt-bindings/genpd/k2g.h | 90
>>>> ++++++++++++++++++++++
>>>> 3 files changed, 151 insertions(+)
>>>> create mode 100644
>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>> create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>> new file mode 100644
>>>> index 000000000000..4c9064e512cb
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>> @@ -0,0 +1,59 @@
>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>> +---------------------------------------------
>>>> +
>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is
>>>> +responsible for controlling the state of the IPs that are present.
>>>> +Communication between the host processor running an OS and the system
>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>> domain
>>>> +implementation plugs into the generic pm domain framework and makes use
>>>> of
>>>> +the TI SCI protocol power on and off each device when needed.
>>>> +
>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>> +
>>>> +PM Domain Node
>>>> +==============
>>>> +The PM domain node represents the global PM domain managed by the PMMC,
>>>> +which in this case is the single implementation as documented by the
>>>> generic
>>>> +PM domain bindings in
>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>> +Because this relies on the TI SCI protocol to communicate with the PMMC
>>>> it
>>>> +must be a child of the pmmc node.
>>>> +
>>>> +Required Properties:
>>>> +--------------------
>>>> +- compatible: should be "ti,sci-pm-domain"
>>>> +- #power-domain-cells: Must be 0.
>>>> +
>>>> +Example (K2G):
>>>> +-------------
>>>> + pmmc: pmmc {
>>>> + compatible = "ti,k2g-sci";
>>>> + ...
>>>> +
>>>> + k2g_pds: k2g_pds {
>>>> + compatible = "ti,sci-pm-domain";
>>>> + #power-domain-cells = <0>;
>>>> + };
>>>> + };
>>>> +
>>>> +PM Domain Consumers
>>>> +===================
>>>> +Hardware blocks that require SCI control over their state must provide
>>>> +a reference to the sci-pm-domain they are part of and a unique device
>>>> +specific ID that identifies the device.
>>>> +
>>>> +Required Properties:
>>>> +--------------------
>>>> +- power-domains: phandle pointing to the corresponding PM domain node.
>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to
>>>> + be used for device control.
>>>
>>>
>>> As I've already stated before, this goes in power-domain cells. When you
>>> have a single thing (i.e. node) that controls multiple things, then you
>>> you need to specify the ID for each of them in phandle args. This is how
>>> irqs, gpio, clocks, *everything* in DT works.
>>
>>
>> You think the reasoning for doing it this way provided by both Ulf and
>> myself on v2 [1] is not valid then?
>>
>> From Ulf:
>>
>> To me, the TI SCI ID, is similar to a "conid" for any another "device
>> resource" (like clock, pinctrl, regulator etc) which we can describe
>> in DT and assign to a device node. The only difference here, is that
>> we don't have common API to fetch the resource (like clk_get(),
>> regulator_get()), but instead we fetches the device's resource from
>> SoC specific code, via genpd's device ->attach() callback.
>
> Sorry, but that sounds like a kernel problem to me and has nothing to
> do with DT bindings.
>
>> From me:
>>
>> Yes, you've pretty much hit it on the head. It is not an index into a list
>> of genpds but rather identifies the device *within* a single genpd. It is a
>> property specific to each device that resides in a ti-sci-genpd, not a
>> mapping describing which genpd the device belongs to. The generic power
>> domain binding is concerned with mapping the device to a specific genpd,
>> which is does fine for us, but we have a sub mapping for devices that exist
>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>
>>
>> So to summarize, the genpd framework does interpret the phandle arg as an
>> index into multiple genpds, just as you've said other frameworks do, but
>> this is not what I am trying to do, we have multiple devices within this
>> *single* genpd, hence the need for the ti,sci-id property.
>
> Fix the genpd framework rather than work around it in DT.
I still disagree that this has nothing to do with DT bindings, as the
current DT binding represents something different already. I am trying
to extend it to give me additional information needed for our platforms.
Are you saying that we should break what the current DT binding already
represents to mean something else?
Regards,
Dave
>
> Rob
>
^ permalink raw reply
* [PATCH] net: thunderx: acpi: fix LMAC initialization
From: Vadim Lomovtsev @ 2017-01-12 15:28 UTC (permalink / raw)
To: linux-arm-kernel
While probing BGX we requesting appropriate QLM for it's configuration
and get LMAC count by that request. Then, while reading configured
MAC values from SSDT table we need to save them in proper mapping:
BGX[i]->lmac[j].mac = <MAC value>
to later provide for initialization stuff. In order to fill
such mapping properly we need to add lmac index to be used while
acpi initialization since at this moment bgx->lmac_count already contains
actual value.
Signed-off-by: Vadim Lomovtsev <Vadim.Lomovtsev@caviumnetworks.com>
---
drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index be30ad0..a3f4f83 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -47,8 +47,9 @@ struct lmac {
struct bgx {
u8 bgx_id;
struct lmac lmac[MAX_LMAC_PER_BGX];
- int lmac_count;
+ u8 lmac_count;
u8 max_lmac;
+ u8 acpi_lmac_idx;
void __iomem *reg_base;
struct pci_dev *pdev;
bool is_dlm;
@@ -1073,13 +1074,13 @@ static acpi_status bgx_acpi_register_phy(acpi_handle handle,
if (acpi_bus_get_device(handle, &adev))
goto out;
- acpi_get_mac_address(dev, adev, bgx->lmac[bgx->lmac_count].mac);
+ acpi_get_mac_address(dev, adev, bgx->lmac[bgx->acpi_lmac_idx].mac);
- SET_NETDEV_DEV(&bgx->lmac[bgx->lmac_count].netdev, dev);
+ SET_NETDEV_DEV(&bgx->lmac[bgx->acpi_lmac_idx].netdev, dev);
- bgx->lmac[bgx->lmac_count].lmacid = bgx->lmac_count;
+ bgx->lmac[bgx->acpi_lmac_idx].lmacid = bgx->acpi_lmac_idx;
+ bgx->acpi_lmac_idx++; /* move to next LMAC */
out:
- bgx->lmac_count++;
return AE_OK;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/2][UPDATE] of: base: add support to get the number of cache levels
From: Sudeep Holla @ 2017-01-12 15:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_Jsq+wawaXeFPnmYw++YMiMzMm+9nSn498Dj7KsGzkfndfog@mail.gmail.com>
On 12/01/17 13:24, Rob Herring wrote:
> On Tue, Jan 10, 2017 at 6:00 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> It is useful to have helper function just to get the number of cache
>> levels for a given logical cpu. This patch adds the support for the
>> same.
>>
>> It will be used on ARM64 platform where the device tree provides the
>> information for the additional non-architected/transparent/external
>> last level caches that are not integrated with the processors.
>>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>> drivers/of/base.c | 23 +++++++++++++++++++++++
>> include/linux/of.h | 1 +
>> 2 files changed, 24 insertions(+)
>>
[...]
>> +int of_count_cache_levels(unsigned int cpu)
>> +{
>> + int level = 0;
>> + struct device_node *np = of_cpu_device_node_get(cpu);
>> +
>> + while (np) {
>> + level++;
>
> This will return 1 if you have a cpu node and no cache nodes. Are you
> assuming the cpu has a cache?
>
Ah right, that's completely wrong assumption.
> Perhaps you should just find the last level cache node and then just
> read "cache-level".
>
Yes, sounds better. I will update accordingly. Thanks for the suggestion.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH] arm64: errata: Provide macro for major and minor cpu revisions
From: Will Deacon @ 2017-01-12 15:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170111121143.9586-1-rrichter@cavium.com>
On Wed, Jan 11, 2017 at 01:11:42PM +0100, Robert Richter wrote:
> Definition of cpu ranges are hard to read if the cpu variant is not
> zero. Provide MIDR_CPU_FULL_REV() macro to describe the full hardware
> revision of a cpu including variant and (minor) revision.
>
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
> arch/arm64/include/asm/cputype.h | 3 +++
> arch/arm64/kernel/cpu_errata.c | 15 +++++++++------
> arch/arm64/kernel/cpufeature.c | 8 +++-----
> 3 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index 26a68ddb11c1..983e59cbdd54 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -56,6 +56,9 @@
> (0xf << MIDR_ARCHITECTURE_SHIFT) | \
> ((partnum) << MIDR_PARTNUM_SHIFT))
>
> +#define MIDR_CPU_FULL_REV(var, rev) \
> + (((var) << MIDR_VARIANT_SHIFT) | (rev))
Minor nit, but could you rename this to MIDR_CPU_VAR_REV instead please?
The revision field *is* the bottom 4 bits, so "full" rev doesn't really
make a lot of sense.
If you repost with that change, I'll pick it up for 4.11.
Thanks,
Will
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox