* [RFC PATCH 0/5] Add an overlay manager to handle board capes
From: Antoine Tenart @ 2016-10-26 14:57 UTC (permalink / raw)
To: linux-arm-kernel
Hi all,
Many boards now come with dips and compatible capes; among others the
C.H.I.P, or Beaglebones. All these boards have a kernel implementing an
out-of-tree "cape manager" which is used to detected capes, retrieve
their description and apply a corresponding overlay. This series is an
attempt to start a discussion, with an implementation of such a manager
which is somehow generic (i.e. formats or cape detectors can be added).
Other use cases could make use of this manager to dynamically load dt
overlays based on some input / hw presence.
The proposed design is a library which can be used by detector drivers
to parse headers and load the corresponding overlay. Helpers are
provided for this purpose. The whole thing is divided into 3 entities:
- The parser which is project-specific (to allow supporting headers
already into the wild). It registers a function parsing an header's
data and filling one or more strings which will be used to find
matching dtbo on the fs.
- The overlay manager helpers allowing to parse a header to retrieve
the previously mentioned strings and to load a compatible overlay.
- The detectors which are used to detect capes and get their description
(to be parsed).
An example of parser and detector is given, compatible with what's done
for the C.H.I.P. As the w1 framework is really bad (and we should
probably do something about that) the detector code is far from being
perfect; but that's not related to what we try to achieve here.
The actual implementation has a limitation: the detectors cannot be
built-in the kernel image as they would likely detect capes at boot time
but will fail to get their corresponding dt overlays as the fs isn't
mounted yet. The only case this can work is when dt overlays are
built-in firmwares. This isn't an issue for the C.H.I.P. use case right
now. There was a discussion about making an helper to wait for the
rootfs to be mount but the answer was "this is the driver's problem".
I'd like to get comments, specifically from people using custom cape
managers, to see if this could fill their needs (with I guess some
modifications).
Thanks!
Antoine
Antoine Tenart (5):
of: introduce the overlay manager
of: overlay-mgr: add the CHIP format
w1: report errors returned by w1_family_notify
w1: add a callback to call slave when a new device is connected
of: overlay-mgr: add a detector for headers stored on a ds2431 eeprom
over w1
drivers/of/Kconfig | 2 +
drivers/of/Makefile | 1 +
drivers/of/overlay-manager/Kconfig | 29 ++++
drivers/of/overlay-manager/Makefile | 2 +
drivers/of/overlay-manager/format-chip.c | 72 ++++++++++
drivers/of/overlay-manager/overlay-manager.c | 199 +++++++++++++++++++++++++++
drivers/w1/slaves/w1_ds2431.c | 39 ++++++
drivers/w1/w1.c | 14 +-
drivers/w1/w1_family.h | 2 +
include/linux/overlay-manager.h | 51 +++++++
10 files changed, 410 insertions(+), 1 deletion(-)
create mode 100644 drivers/of/overlay-manager/Kconfig
create mode 100644 drivers/of/overlay-manager/Makefile
create mode 100644 drivers/of/overlay-manager/format-chip.c
create mode 100644 drivers/of/overlay-manager/overlay-manager.c
create mode 100644 include/linux/overlay-manager.h
--
2.10.1
^ permalink raw reply
* [PATCH] tty/serial: at91: fix hardware handshake on Atmel platforms
From: Richard Genoud @ 2016-10-26 14:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025171754.a4lult2ld3gv5a2q@pengutronix.de>
On 25/10/2016 19:17, Uwe Kleine-K?nig wrote:
> Hello,
>
> On Tue, Oct 25, 2016 at 06:11:35PM +0200, Richard Genoud wrote:
>> commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
>> hardware handshake is enabled"), despite its title, broke hardware
>> handshake on *every* Atmel platforms.
>
> s/platforms/platform/
fixed.
>> The only one partially working is the SAMA5D2.
>>
>> To understand why, one has to understand the flag ATMEL_US_USMODE_HWHS
>> first:
>> Before commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management
>> when hardware handshake is enabled"), this flag was never set.
>> Thus, the CTS/RTS where only handled by serial_core (and everything
>> worked just fine).
>>
>> This commit introduced the use of the ATMEL_US_USMODE_HWHS flag,
>> enabling it for all boards when the user space enables flow control.
>>
>> When the ATMEL_US_USMODE_HWHS is set, the Atmel USART controller
>> handles a part of the flow control job:
>> - disable the transmitter when the CTS pin gets high.
>> - drive the RTS pin high when the DMA buffer transfer is completed or
>> PDC RX buffer full or RX FIFO is beyond threshold. (depending on the
>> controller version).
>
> I don't understand the DMA buffer part.
see below.
>
>> NB: This feature is *not* mandatory for the flow control to work.
>>
>> Now, the specifics of the ATMEL_US_USMODE_HWHS flag:
>>
>> - For platforms with DMAC and no FIFOs (sam9x25, sam9x35, sama5D3,
>> sama5D4, sam9g15, sam9g25, sam9g35)* this feature simply doesn't work.
>> ( source: https://lkml.org/lkml/2016/9/7/598 )
>
> What does "doesn't work" mean? Is ATMEL_US_USMODE_HWHS a noop, or does
> it break something?
Here is what the datasheet says:
"The RTS pin is driven high if the receiver is disabled or if the DMA
status flag indicates that the buffer transfer is completed.[...]
As soon as the receiver is enabled, the RTS falls, indicating to the
remote device that it can start transmitting. Defining a new transfer
descriptor in the DMA clears the status flag and, as a result, asserts
the pin RTS low."
And what's really happening:
RTS pin is stuck at high level.
Even if the receiver is enabled, or if there's a new descriptor is
the DMA, RTS is always high.
And if RTS is high, nothing can be received.
(RTS==1 means NOT ready to receive).
So it's definitely not a noop, since RX is blocked.
>
>> Tested it on sam9g35, the RTS pins always stays up, even when RXEN=1
>> or a new DMA transfer descriptor is set.
>> => ATMEL_US_USMODE_HWHS should not be used for those platforms
>
> Depending on the answer to the above question it might not matter if it
> is set or not.
>
>> - For platforms with a PDC (sam926{0,1,3}, sam9g10, sam9g20, sam9g45,
>> sam9g46)*, there's another kind of problem. Once the flag
>> ATMEL_US_USMODE_HWHS is set, the RTS pin can't be driven anymore via
>> RTSEN/RTSDIS in USART Control Register. The RTS pin can only be driven
>> by enabling/disabling the receiver or setting RCR=RNCR=0 in the PDC
>> (Receive (Next) Counter Register).
>> => Doing this is beyond the scope of this patch and could add other
>> bugs, so the original (and working) behaviour should be set for those
>> platforms (meaning ATMEL_US_USMODE_HWHS flag should be unset).
>
> Then maybe just revert the faulty patch for now and do it better later
> on top of this?
Well, we can't just revert 1cf6e8fc8341 without breaking sama5d2 platform.
And at the end, reverting 1cf6e8fc8341 without breaking sama5d2 will result
in a patch exactly like (or at least *very* similar to) this one.
>> - For platforms with a FIFO (sama5d2)*, the RTS pin is driven according
>> to the RX FIFO thresholds, and can be also driven by RTSEN/RTSDIS in
>> USART Control Register. No problem here.
>> (This was the use case of commit 1cf6e8fc8341 ("tty/serial: at91: fix
>> RTS line management when hardware handshake is enabled"))
>> NB: If the CTS pin declared as a GPIO in the DTS, (for instance
>> cts-gpios = <&pioA PIN_PB31 GPIO_ACTIVE_LOW>), the transmitter will be
>> disabled.
>> => ATMEL_US_USMODE_HWHS flag can be set for this platform ONLY IF the
>> CTS pin is not a GPIO.
>
> How did you test this? What I consider interesting here is if the
> hardware CTS function was muxed on a pin and in which state this pin (if
> any) is. If it is not muxed anywhere and disables the transmitter
> because of an internal pull up that is IMHO a hw bug and should be
> mentioned more explicitly in the comment.
I see your point, and I did some more tests.
NB: The CTS pin is PD29 (on flx2 on sama5D2).
- If I use PD24 as CTS-gpio for flx2 and PD29 is not muxed (so, as an input):
The transmitter is disabled no matter what value PD24 or PD29 are.
In the same use case, if PD29 is exported as GPIO (output), the transmitter stay disabled for every value.
- If I use PD24 as CTS-gpio for flx2 and PD39 is muxed as another function (I tried TWD0), the transmitter is also disabled.
- If I use PD24 as CTS-gpio for flx2 and PD29 is muxed as CTS function, I can activate the transmitter by setting PD29 to 0V.
- If I use PD29 as CTS-gpio, the transmitter is still disabled.
In all this tests, if I do:
devmem2 0xFC010204 w 0xC00008C0
i.e. remove the ATMEL_US_USMODE_HWHS flag in flx2 usart, the transmitter starts transmitting.
So, I guess, like you said, that there's an internal pull-up on the CTS input.
I can add some comment about that.
>> So, the only case when ATMEL_US_USMODE_HWHS can be enabled is when
>> (atmel_use_fifo(port) &&
>> !mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS))
>>
>> Tested on all Atmel USART controller flavours:
>> AT91SAM9G35-CM, AT91SAM9G20-EK and SAMA5D2xplained
>> ^^^^ ^^^^ ^^^^
>> (DMAC flavour), (PDC flavour) and (FIFO flavour)
>
> I'd write that as: Tested on all Atmel USART controller flavours:
> AT91SAM9G35-CM (DMAC flavour), AT91SAM9G20-EK (PDC flavour),
> SAMA5D2xplained (FIFO flavour).
alright.
>> Changes since v4:
>> - the mctrl_gpio_use_rtscts() is gone since it was atmel_serial
>> specific. (so patch 1 is gone)
>> - patches 2 and 3 have been merged together since it didn't make
>> a lot of sense to correct the GPIO case in one separate patch.
>> - ATMEL_US_USMODE_HWHS is now unset for platform with PDC
>>
>> Changes since v3:
>> - remove superfluous #include <linux/err.h> (thanks to Uwe)
>> - rebase on next-20160930
>>
>> Changes since v2:
>> - remove IS_ERR_OR_NULL() test in patch 1/3 as Uwe suggested.
>> - fix typos in patch 2/3
>> - rebase on next-20160927
>> - simplify the logic in patch 3/3.
>>
>> Changes since v1:
>> - Correct patch 1 with the error found by kbuild.
>> - Add Alexandre's Acked-by on patch 2
>> - Rewrite patch 3 logic in the light of the on-going discussion
>> with Cyrille and Alexandre.
>>
>> * the list may not be exhaustive
>
> Add a Fixes: line please.
ok.
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> ---
>> drivers/tty/serial/atmel_serial.c | 25 +++++++++++++++++++++----
>> 1 file changed, 21 insertions(+), 4 deletions(-)
>>
>> I think this should go in the stable tree since it fixes the flow
>> control broken since v4.0.
>> But It won't compile on versions before 4.9rc1 because:
>> function atmel_use_fifo was introduced in 4.4.12 / 4.7
>> variable atmel_port was introduced in 4.9rc1
>>
>> That's why I didn't add the Cc stable in the email body.
>>
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index fd8aa1f4ba78..2c7c45904ba7 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -2132,11 +2132,28 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>> mode |= ATMEL_US_USMODE_RS485;
>> } else if (termios->c_cflag & CRTSCTS) {
>> /* RS232 with hardware handshake (RTS/CTS) */
>> - if (atmel_use_dma_rx(port) && !atmel_use_fifo(port)) {
>> - dev_info(port->dev, "not enabling hardware flow control because DMA is used");
>> - termios->c_cflag &= ~CRTSCTS;
>
> This if was not introduced in commit 1cf6e8fc8341. Is it still right to
> remove this here?
This was introduced by commit 5be605ac9af9 that tried to fix
commit 1cf6e8fc8341 but based on a false assumption.
Quote from the commit message:
" Commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
hardware handshake is enabled") actually allowed to enable hardware
handshaking.
Before, the CRTSCTS flags was silently ignored.
"
This wasn't true.
This was a misunderstanding of the ATMEL_US_USMODE_HWHS flag:
Commit 1cf6e8fc8341 didn't allowed to enable hardware handshaking, but
introduced the ATMEL_US_USMODE_HWHS flag.
And before 1cf6e8fc8341, the CRTSCTS flags wasn't silently ignored, it
was perfectly respected.
So, yes, I'm quite comfortable removing those lines here.
(maybe I should add a Fixes: 5be605ac9af9 ?)
Otherwise, we can do it the hard way, and:
- revert 5be605ac9af9 ("tty/serial: atmel: fix hardware handshake selection)"
- then apply something like:
mode |= ATMEL_US_USMODE_RS485;
} else if (termios->c_cflag & CRTSCTS) {
/* RS232 with hardware handshake (RTS/CTS) */
- mode |= ATMEL_US_USMODE_HWHS;
+ if (atmel_use_fifo(port) &&
+ !mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS)) {
+ mode |= ATMEL_US_USMODE_HWHS;
+ } else {
+ mode |= ATMEL_US_USMODE_NORMAL;
+ }
} else {
/* RS232 without hadware handshake */
mode |= ATMEL_US_USMODE_NORMAL;
Or even harder:
- revert 5be605ac9af9 ("tty/serial: atmel: fix hardware handshake selection)"
- then apply something like:
if (port->rs485.flags & SER_RS485_ENABLED) {
atmel_uart_writel(port, ATMEL_US_TTGR,
port->rs485.delay_rts_after_send);
mode |= ATMEL_US_USMODE_RS485;
- } else if (termios->c_cflag & CRTSCTS) {
- /* RS232 with hardware handshake (RTS/CTS) */
- mode |= ATMEL_US_USMODE_HWHS;
- } else {
- /* RS232 without hadware handshake */
+ } else {
mode |= ATMEL_US_USMODE_NORMAL;
}
(at this point, every platform will work with flow control without problem)
- and finally introduce the ATMEL_US_USMODE_HWHS flag for platforms with FIFO:
if (port->rs485.flags & SER_RS485_ENABLED) {
atmel_uart_writel(port, ATMEL_US_TTGR,
port->rs485.delay_rts_after_send);
mode |= ATMEL_US_USMODE_RS485;
} else {
- mode |= ATMEL_US_USMODE_NORMAL;
+ if (termios->c_cflag & CRTSCTS) &&
+ atmel_use_fifo(port) &&
+ !mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS)) {
+ mode |= ATMEL_US_USMODE_HWHS;
+ } else {
+ mode |= ATMEL_US_USMODE_NORMAL;
+ }
}
>
>> - } else {
>> + if (atmel_use_fifo(port) &&
>> + !mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS)) {
>> + /*
>> + * with ATMEL_US_USMODE_HWHS set, the controller will
>> + * be able to drive the RTS pin high/low when the RX
>> + * FIFO is above RXFTHRES/below RXFTHRES2.
>> + * It will also disable the transmitter when the CTS
>> + * pin is high.
>> + * This mode is not activated if CTS pin is a GPIO
>> + * because in this case, the transmitter is always
>> + * disabled.
>> + * If the RTS pin is a GPIO, the controller won't be
>> + * able to drive it according to the FIFO thresholds,
>> + * but it will be handled by the driver.
>> + */
>> mode |= ATMEL_US_USMODE_HWHS;
>> + } else {
>> + /*
>> + * For platforms without FIFO, the flow control is
>> + * handled by the driver.
>> + */
>> + mode |= ATMEL_US_USMODE_NORMAL;
>> }
>> } else {
>> /* RS232 without hadware handshake */
>
> (unrelated to this patch) s/hadware/hardware/
>
I plan to to some janitor work on this driver, I'll add it then.
Thanks.
Richard.
^ permalink raw reply
* [PATCH v7 2/4] soc: mediatek: Init MT8173 scpsys driver earlier
From: Matthias Brugger @ 2016-10-26 14:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1468314071.24705.4.camel@mhfsdcap03>
Hi Yong,
On 07/12/2016 11:01 AM, Yong Wu wrote:
> Hi Matthias,
>
> On Fri, 2016-07-08 at 14:47 +0200, Matthias Brugger wrote:
>>
>> On 06/07/16 07:22, James Liao wrote:
>>> On Sat, 2016-07-02 at 18:35 +0200, Matthias Brugger wrote:
>>>>
>>>> On 05/16/2016 11:28 AM, James Liao wrote:
>>>>> Some power domain comsumers may init before module_init.
>>>>> So the power domain provider (scpsys) need to be initialized
>>>>> earlier too.
>>>>>
>>>>> Take an example for our IOMMU (M4U) and SMI. SMI is a bridge
>>>>> between IOMMU and multimedia HW. SMI is responsible to
>>>>> enable/disable iommu and help transfer data for each multimedia
>>>>> HW. Both of them have to wait until the power and clocks are
>>>>> enabled.
>>>>>
>>>>> So scpsys driver should be initialized before SMI, and SMI should
>>>>> be initialized before IOMMU, and then init IOMMU consumers
>>>>> (display/vdec/venc/camera etc.).
>>>>>
>>>>> IOMMU is subsys_init by default. So we need to init scpsys driver
>>>>> before subsys_init.
>>>>>
>>>>> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
>>>>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>>>>> ---
>>>>> drivers/soc/mediatek/mtk-scpsys.c | 19 ++++++++++++++++++-
>>>>> 1 file changed, 18 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
>>>>> index 5870a24..00c0adb 100644
>>>>> --- a/drivers/soc/mediatek/mtk-scpsys.c
>>>>> +++ b/drivers/soc/mediatek/mtk-scpsys.c
>>>>> @@ -617,4 +617,21 @@ static struct platform_driver scpsys_drv = {
>>>>> .of_match_table = of_match_ptr(of_scpsys_match_tbl),
>>>>> },
>>>>> };
>>>>> -builtin_platform_driver(scpsys_drv);
>>>>> +
>>>>> +static int __init scpsys_drv_init(void)
>>>>> +{
>>>>> + return platform_driver_register(&scpsys_drv);
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * There are some Mediatek drivers which depend on the power domain driver need
>>>>> + * to probe in earlier initcall levels. So scpsys driver also need to probe
>>>>> + * earlier.
>>>>> + *
>>>>> + * IOMMU(M4U) and SMI drivers for example. SMI is a bridge between IOMMU and
>>>>> + * multimedia HW. IOMMU depends on SMI, and SMI is a power domain consumer,
>>>>> + * so the proper probe sequence should be scpsys -> SMI -> IOMMU driver.
>>>>> + * IOMMU drivers are initialized during subsys_init by default, so we need to
>>>>> + * move SMI and scpsys drivers to subsys_init or earlier init levels.
>>>>> + */
>>>>> +subsys_initcall(scpsys_drv_init);
>>>>>
>>>>
>>>> Can't we achieve this with probe deferring? I'm not really keen on
>>>> coding the order of the different drivers like this.
>>>
>>> Hi Matthias,
>>>
>>> Some drivers such as IOMMU don't support probe deferring. So scpsys need
>>> to init before them by changing init level.
>>>
>>
>> SMI larbs return EPROBE_DEFER if it can't find the PM provider for it's
>> domain, so this part should not be the problem. The larbs get added as
>> components in the probe, when the PM provider is present.
>>
>> The iommu driver uses the larbs as components. As long as not all
>> components are present the iommu does not bind them. So from what I see
>> this should work without any problem.
>>
>> Can you please specify where the iommu dirver has a problem. Maybe we
>> need to fix the driver rather then scpsys.
>
> We got a problem while bootup, the hang backtrace is like below(Sorry, I
> can't find the full backtrace now):
>
> [ 7.832359] Call trace:
> [ 7.834778] [<ffffffc000764424>] mtk_smi_larb_get+0x24/0xa8
> [ 7.840300] [<ffffffc0005a1390>] mtk_drm_crtc_enable+0x6c/0x450
>
> The reason is that "larb->mmu" is NULL while DRM call mtk_smi_larb_get.
>
> DRM call iommu_present to wait for whether IOMMU is ready[1].
> But in the MTK-IOMMU, It will call
> bus_set_iommu->mtk_iommu_add_device->mtk_iommu_attach_device, then it's
> able to transfer "struct mtk_smi_iommu" to SMI whose probe maybe delayed
> by power-domain, then SMI could finish its probe.
>
> So If DRM probe is called after the time of calling bus_set_iommu and
> before the time of SMI probe finish, this problem can be reproduced.
>
> The root cause is that iommu_present cann't indicate that MTK-IOMMU and
> SMI is ready actually. in other words, the DRM cann't detect
> whether the SMI has probed done.
>
> If we move scpsys_init from module_initcall to subsys_initcall, the
> IOMMU and SMI could be probe done during the subsys_initcall period.
> this issue can be avoid.
> And as we grep before, there are also some examples whose power-domain
> is also not module_init[2].
> core_initcall(exynos4_pm_init_power_domain);
> subsys_initcall(imx_pgc_init);
>
> If you think that this patch will hide the problem above, then I have to
> add a new function, like below:
> //==================
> bool mtk_smi_larb_ready(struct device *larbdev)
> {
> struct mtk_smi_larb *larb = dev_get_drvdata(larbdev);
>
> return larb && !!larb->mmu;
> }
> //=================
>
> And in the probe of all the MTK-IOMMU consumer, we have to add this:
> //====================
> if (!mtk_smi_larb_ready(&larb_pdev->dev))
> return -EPROBE_DEFER;
> //====================
>
> The sequence is a little complex, If I don't explain clearly, please
> tell me.
> Any other suggestion about this? Thanks.
I'm still trying to find a different way to solve this. Unfortunately
I'm not able to reproduce this on my mt8173-evb...
...but: iommu_present checkes if the bus->iommu_ops is set. If we set
these option in the iommu after all components got initialized, we
should be fine. Or did I miss something?
Would you mind trying the patch below (against v4.9-rc1)?
If you know of any way to reproduce this issue, I would be happy to know.
I'm adding Joerg, the iommu maintainer, maybe he has an idea.
Thanks a lot,
Matthias
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index b12c12d..9249011 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -602,10 +602,12 @@ static int mtk_iommu_probe(struct platform_device
*pdev)
if (ret)
return ret;
- if (!iommu_present(&platform_bus_type))
+ ret = component_master_add_with_match(dev, &mtk_iommu_com_ops,
match);
+
+ if (ret && !iommu_present(&platform_bus_type))
bus_set_iommu(&platform_bus_type, &mtk_iommu_ops);
- return component_master_add_with_match(dev, &mtk_iommu_com_ops,
match);
+ return ret;
}
static int mtk_iommu_remove(struct platform_device *pdev)
^ permalink raw reply related
* [PATCH V3 5/8] iommu: of: Handle IOMMU lookup failure with deferred probing or error
From: Robin Murphy @ 2016-10-26 14:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475600632-21289-6-git-send-email-sricharan@codeaurora.org>
On 04/10/16 18:03, Sricharan R wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> Failures to look up an IOMMU when parsing the DT iommus property need to
> be handled separately from the .of_xlate() failures to support deferred
> probing.
>
> The lack of a registered IOMMU can be caused by the lack of a driver for
> the IOMMU, the IOMMU device probe not having been performed yet, having
> been deferred, or having failed.
>
> The first case occurs when the device tree describes the bus master and
> IOMMU topology correctly but no device driver exists for the IOMMU yet
> or the device driver has not been compiled in. Return NULL, the caller
> will configure the device without an IOMMU.
>
> The second and third cases are handled by deferring the probe of the bus
> master device which will eventually get reprobed after the IOMMU.
>
> The last case is currently handled by deferring the probe of the bus
> master device as well. A mechanism to either configure the bus master
> device without an IOMMU or to fail the bus master device probe depending
> on whether the IOMMU is optional or mandatory would be a good
> enhancement.
>
> The current iommu framework handles pci and non-pci devices separately,
> so taken care of both the paths in this patch. The iommu's add_device
> callback is invoked after the master's configuration data is added in
> xlate. This is needed because the iommu core calls add_device callback
> during the BUS_ADD_DEVICE notifier, which is of no use now. Eventually
> that call has to be removed.
Laurent's signoff seems to have gone missing here.
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
> drivers/iommu/of_iommu.c | 47 +++++++++++++++++++++++++++++++++++++++++++----
> drivers/of/device.c | 7 ++++++-
> include/linux/of_device.h | 6 ++++--
> 3 files changed, 53 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 5b82862..1a5e28b 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -23,6 +23,7 @@
> #include <linux/of.h>
> #include <linux/of_iommu.h>
> #include <linux/of_pci.h>
> +#include <linux/pci.h>
> #include <linux/slab.h>
>
> static const struct of_device_id __iommu_of_table_sentinel
> @@ -167,12 +168,29 @@ static const struct iommu_ops
> return NULL;
>
> ops = of_iommu_get_ops(iommu_spec.np);
> +
> + if (!ops) {
> + const struct of_device_id *oid;
> +
> + oid = of_match_node(&__iommu_of_table, iommu_spec.np);
> + ops = oid ? ERR_PTR(-EPROBE_DEFER) : NULL;
It would seem even simpler and more convenient to roll this into the end
of of_iommu_get_ops():
if (!ops && of_match_node(&__iommu_of_table, iommu_spec.np))
ops = ERR_PTR(-EPROBE_DEFER);
then just fix up the existing !ops checks to !IS_ERR(ops) appropriately.
> + return ops;
> + }
> +
> if (!ops || !ops->of_xlate ||
> iommu_fwspec_init(&pdev->dev, &iommu_spec.np->fwnode, ops) ||
> ops->of_xlate(&pdev->dev, &iommu_spec))
> ops = NULL;
>
> + if (ops && ops->add_device) {
> + ops = (ops->add_device(&pdev->dev) == 0) ? ops : NULL;
This is a really obtuse way of writing
if (ops->add_device(...))
ops = NULL;
However, given that we're now returning an ERR_PTR, it would be worth
capturing the return value of add_device and propagating the error back
up - if the IOMMU driver has refused one of its masters for some reason,
it probably isn't safe to allow that device to do DMA either way, so we
ought to prevent it probing at all.
> +
> + if (!ops)
> + dev_err(&pdev->dev, "Failed to setup iommu ops\n");
Given the above, I think this should be more along the lines of "Device
rejected by IOMMU: %d" with the actual error code as well. It's one of
those "if you ever see it, you're going to have to debug it" cases, so
the clearer the better.
> + }
> +
> of_node_put(iommu_spec.np);
> +
> return ops;
> }
>
> @@ -183,9 +201,15 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> struct device_node *np;
> const struct iommu_ops *ops = NULL;
> int idx = 0;
> + struct device *bridge;
> +
> + if (dev_is_pci(dev)) {
> + bridge = pci_get_host_bridge_device(to_pci_dev(dev));
>
> - if (dev_is_pci(dev))
> - return of_pci_iommu_configure(to_pci_dev(dev), master_np);
> + if (bridge && bridge->parent && bridge->parent->of_node)
> + return of_pci_iommu_configure(to_pci_dev(dev),
> + bridge->parent->of_node);
else fall through to treating it as a platform device?
...that's not right. Anyway, this is PCI-specific stuff so should be in
the PCI-specific helper function.
> + }
>
> /*
> * We don't currently walk up the tree looking for a parent IOMMU.
> @@ -198,6 +222,14 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> np = iommu_spec.np;
> ops = of_iommu_get_ops(np);
>
> + if (!ops) {
> + const struct of_device_id *oid;
> +
> + oid = of_match_node(&__iommu_of_table, np);
> + ops = oid ? ERR_PTR(-EPROBE_DEFER) : NULL;
> + goto err_put_node;
> + }
Same comment as above. Especially since moving it to of_iommu_get_ops()
would obviate the duplication.
> +
> if (!ops || !ops->of_xlate ||
> iommu_fwspec_init(dev, &np->fwnode, ops) ||
> ops->of_xlate(dev, &iommu_spec))
> @@ -207,11 +239,18 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> idx++;
> }
>
> + if (ops && ops->add_device) {
> + ops = (ops->add_device(dev) == 0) ? ops : NULL;
> +
> + if (!ops)
> + dev_err(dev, "Failed to setup iommu_ops\n");
> + }
> +
It would be nice to avoid duplicating this as well.
Robin.
> return ops;
>
> err_put_node:
> of_node_put(np);
> - return NULL;
> + return ops;
> }
>
> static int __init of_iommu_init(void)
> @@ -222,7 +261,7 @@ static int __init of_iommu_init(void)
> for_each_matching_node_and_match(np, matches, &match) {
> const of_iommu_init_fn init_fn = match->data;
>
> - if (init_fn(np))
> + if (init_fn && init_fn(np))
> pr_err("Failed to initialise IOMMU %s\n",
> of_node_full_name(np));
> }
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 1c843e2..c8e74d7 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -82,7 +82,7 @@ int of_device_add(struct platform_device *ofdev)
> * can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
> * to fix up DMA configuration.
> */
> -void of_dma_configure(struct device *dev, struct device_node *np)
> +int of_dma_configure(struct device *dev, struct device_node *np)
> {
> u64 dma_addr, paddr, size;
> int ret;
> @@ -129,10 +129,15 @@ void of_dma_configure(struct device *dev, struct device_node *np)
> coherent ? " " : " not ");
>
> iommu = of_iommu_configure(dev, np);
> + if (IS_ERR(iommu))
> + return PTR_ERR(iommu);
> +
> dev_dbg(dev, "device is%sbehind an iommu\n",
> iommu ? " " : " not ");
>
> arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
> +
> + return 0;
> }
> EXPORT_SYMBOL_GPL(of_dma_configure);
>
> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
> index d20a31a..2bdb872 100644
> --- a/include/linux/of_device.h
> +++ b/include/linux/of_device.h
> @@ -55,7 +55,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
> return of_node_get(cpu_dev->of_node);
> }
>
> -void of_dma_configure(struct device *dev, struct device_node *np);
> +int of_dma_configure(struct device *dev, struct device_node *np);
> void of_dma_deconfigure(struct device *dev);
> #else /* CONFIG_OF */
>
> @@ -100,7 +100,9 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
> return NULL;
> }
> static inline void of_dma_configure(struct device *dev, struct device_node *np)
> -{}
> +{
> + return 0;
> +}
> static inline void of_dma_deconfigure(struct device *dev)
> {}
> #endif /* CONFIG_OF */
>
^ permalink raw reply
* [PATCH] mfd: qcom-pm8xxx: Clean up PM8XXX namespace
From: Jacek Anaszewski @ 2016-10-26 14:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477487453-15801-1-git-send-email-linus.walleij@linaro.org>
Hi Linus,
On 10/26/2016 03:10 PM, Linus Walleij wrote:
> The Kconfig and file naming for the PM8xxx driver is totally
> confusing:
>
> - Kconfig options MFD_PM8XXX and MFD_PM8921_CORE, some in-kernel
> users depending on or selecting either at random.
> - A driver file named pm8921-core.c even if it is indeed
> used by the whole PM8xxx family of chips.
> - An irqchip named pm8xxx since it was (I guess) realized that
> the driver was generic for all pm8xxx PMICs.
>
> As I may want to add support for PM8901 this is starting to get
> really messy. Fix this situation by:
>
> - Remove the MFD_PM8921_CORE symbol and rely solely on MFD_PM8XXX
> and convert all users, including LEDs Kconfig and ARM defconfigs
> for qcom and multi_v7 to use that single symbol.
> - Renaming the driver to qcom-pm8xxx.c to fit along the two
> other qcom* prefixed drivers.
> - Rename functions withing the driver from 8921 to 8xxx to
> indicate it is generic.
> - Just drop the =m config from the pxa_defconfig, I have no clue
> why it is even there, it is not a Qualcomm platform. (Possibly
> older Kconfig noise from saveconfig.)
>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> I do NOT think it is a good idea to try to split this commit up,
> I rather prefer that Lee simply merge it into MFD.
>
> The reason is that files like qcom_defconfig already contain both
> the right symbols, but the MFD_PM8921_CORE symbol cannot be removed
> until this rename has happened, whereas multi_v7_defconfig needs
> it added etc, and this is just a clean nice cut.
>
> Jacek, ARM SoC person: please ACK this patch to get merged into
> MFD.
> ---
[...]
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 7a628c6516f6..86bb1515a00e 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -645,7 +645,7 @@ config LEDS_VERSATILE
>
> config LEDS_PM8058
> tristate "LED Support for the Qualcomm PM8058 PMIC"
> - depends on MFD_PM8921_CORE
> + depends on MFD_PM8XXX
> depends on LEDS_CLASS
> help
> Choose this option if you want to use the LED drivers in
Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* [PATCH V3 0/8] IOMMU probe deferral support
From: Sricharan @ 2016-10-26 14:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <60ee8066-f167-e9df-ae3e-4138f1133bad@arm.com>
Hi Robin,
>On 04/10/16 18:03, Sricharan R wrote:
>> Initial post from Laurent Pinchart[1]. This is
>> series calls the dma ops configuration for the devices
>> at a generic place so that it works for all busses.
>> The dma_configure_ops for a device is now called during
>> the device_attach callback just before the probe of the
>> bus/driver is called. Similarly dma_deconfigure is called during
>> device/driver_detach path.
>>
>>
>> pci_bus_add_devices (platform/amba)(_device_create/driver_register)
>> | |
>> pci_bus_add_device (device_add/driver_register)
>> | |
>> device_attach device_initial_probe
>> | |
>> __device_attach_driver __device_attach_driver
>> |
>> driver_probe_device
>> |
>> really_probe
>> |
>> dma_configure
>>
>> Similarly on the device/driver_unregister path __device_release_driver is
>> called which inturn calls dma_deconfigure.
>>
>> If the ACPI bus code follows the same, we can add acpi_dma_configure
>> at the same place as of_dma_configure.
>>
>> This series is based on the recently merged Generic DT bindings for
>> PCI IOMMUs and ARM SMMU from Robin Murphy robin.murphy at arm.com [2]
>>
>> This time tested this with platform and pci device for probe deferral
>> and reprobe on arm64 based platform. There is an issue on the cleanup
>> path for arm64 though, where there is WARN_ON if the dma_ops is reset while
>> device is attached to an domain in arch_teardown_dma_ops.
>> But with iommu_groups created from the iommu driver, the device is always
>> attached to a domain/default_domain. So so the WARN has to be removed/handled
>> probably.
>
>I've finally got the chance to take a proper look at this series (sorry
>for the delay). First up, with these patches on top of 4.9-rc2, my
>little script for unbinding some PCI devices and rebinding them to VFIO
>now goes horribly, horribly wrong.
>
>Robin.
>
Thanks for looking in to this.
I was trying to reproduce the below with a command like this in my setup.
echo 0002\:00\:00.0 > /sys/bus/pci/devices/0002\:00\:000.0/driver/unbind0.0/driver/unbind
echo 0x17cb 0x0104 > /sys/bus/pci/drivers/vfio-pci/new_id
But for me the unbind and reconfiguring/adding the iommus_ops to vfio-pci did
succeed, although the WARN_ON in arch_teardown_dma_ops was there, that
could be suppresed. The vfio_pci_probe was not going through because
the pci hdr_type was not PCI_HEADER_TYPE_NORMAL.
But anyways iommu unbind/rebind seemed to be going through.
If i can get what your script is doing, i can try that and see what happens.
Regards,
Sricharan
>[ 39.901592] iommu: Removing device 0000:08:00.0 from group 0
>[ 39.907383] ------------[ cut here ]------------
>[ 39.911969] WARNING: CPU: 0 PID: 174 at
>arch/arm64/mm/dma-mapping.c:856 arch_teardown_dma_ops+0x48/0x68
>[ 39.921266] Modules linked in:
>[ 39.924290]
>[ 39.925766] CPU: 0 PID: 174 Comm: vfio Not tainted 4.9.0-rc2+ #1249
>[ 39.931967] Hardware name: ARM Juno development board (r1) (DT)
>[ 39.937826] task: ffffffc975ee9900 task.stack: ffffffc974d60000
>[ 39.943687] PC is at arch_teardown_dma_ops+0x48/0x68
>[ 39.948603] LR is at arch_teardown_dma_ops+0x34/0x68
>[ 39.953516] pc : [<ffffff80080948f8>] lr : [<ffffff80080948e4>]
>pstate: 60000145
>[ 39.960834] sp : ffffffc974d63ca0
>[ 39.964112] x29: ffffffc974d63ca0 x28: ffffffc974d60000
>[ 39.969377] x27: ffffff80088a2000 x26: 0000000000000040
>[ 39.974642] x25: 0000000000000123 x24: ffffffc976a48918
>[ 39.979907] x23: ffffffc974d63eb8 x22: ffffff8008db7550
>[ 39.985171] x21: 000000000000000d x20: ffffffc9763e9b50
>[ 39.990435] x19: ffffffc9763f20a0 x18: 0000000000000010
>[ 39.995699] x17: 0000007f99c18018 x16: ffffff80080c0580
>[ 40.000964] x15: ffffff8008bb7000 x14: 000137c100013798
>[ 40.006228] x13: ffffffffff000000 x12: ffffffffffffffff
>[ 40.011492] x11: 0000000000000018 x10: 000000000000000d
>[ 40.016757] x9 : 0000000040000000 x8 : 0000000000210d00
>[ 40.022021] x7 : 00000049771bc000 x6 : 00000000001f17ed
>[ 40.027286] x5 : ffffff80084c4208 x4 : 0000000000000080
>[ 40.032551] x3 : ffffffc975ea9800 x2 : ffffffbf25d7aa50
>[ 40.037815] x1 : 0000000000000000 x0 : 0000000000000080
>[ 40.043078]
>[ 40.044549] ---[ end trace 35c1e743d6e6c035 ]---
>[ 40.049117] Call trace:
>[ 40.051537] Exception stack(0xffffffc974d63ad0 to 0xffffffc974d63c00)
>[ 40.057914] 3ac0: ffffffc9763f20a0
>0000008000000000
>[ 40.065668] 3ae0: ffffffc974d63ca0 ffffff80080948f8 ffffffbf25d7aa40
>ffffffc975ea9800
>[ 40.073421] 3b00: ffffffc974d60000 000000000002fc80 ffffffc976801e00
>ffffffc974d60000
>[ 40.081175] 3b20: ffffff80084c4208 0000000000000040 ffffff80088a2000
>ffffffc974d60000
>[ 40.088928] 3b40: ffffff80084caf78 ffffffc974d60000 ffffffc974d60000
>ffffffc974d60000
>[ 40.096682] 3b60: ffffffc975ea9800 ffffffc974d60000 0000000000000080
>0000000000000000
>[ 40.104435] 3b80: ffffffbf25d7aa50 ffffffc975ea9800 0000000000000080
>ffffff80084c4208
>[ 40.112188] 3ba0: 00000000001f17ed 00000049771bc000 0000000000210d00
>0000000040000000
>[ 40.119941] 3bc0: 000000000000000d 0000000000000018 ffffffffffffffff
>ffffffffff000000
>[ 40.127695] 3be0: 000137c100013798 ffffff8008bb7000 ffffff80080c0580
>0000007f99c18018
>[ 40.135450] [<ffffff80080948f8>] arch_teardown_dma_ops+0x48/0x68
>[ 40.141400] [<ffffff8008764a14>] of_dma_deconfigure+0xc/0x18
>[ 40.147005] [<ffffff8008552804>] dma_deconfigure+0xc/0x18
>[ 40.152353] [<ffffff800853ba10>] __device_release_driver+0x88/0x120
>[ 40.158560] [<ffffff800853bacc>] device_release_driver+0x24/0x38
>[ 40.164507] [<ffffff800853a868>] unbind_store+0xe8/0x110
>[ 40.169767] [<ffffff8008539c70>] drv_attr_store+0x20/0x30
>[ 40.175113] [<ffffff800823ab18>] sysfs_kf_write+0x48/0x58
>[ 40.180458] [<ffffff8008239ea8>] kernfs_fop_write+0xb0/0x1d8
>[ 40.186063] [<ffffff80081c507c>] __vfs_write+0x1c/0x100
>[ 40.191237] [<ffffff80081c5e80>] vfs_write+0xa0/0x1b8
>[ 40.196239] [<ffffff80081c7274>] SyS_write+0x44/0xa0
>[ 40.201155] [<ffffff8008082ef0>] el0_svc_naked+0x24/0x28
>[ 40.206703] vfio-pci 0000:08:00.0: Failed to setup iommu ops
>[ 40.212382] vfio-pci: probe of 0000:08:00.0 failed with error -22
>[ 40.228075] ------------[ cut here ]------------
>[ 40.235263] WARNING: CPU: 1 PID: 174 at ./include/linux/kref.h:46
>kobject_get+0x64/0x88
>[ 40.243181] Modules linked in:
>[ 40.246201]
>[ 40.247673] CPU: 1 PID: 174 Comm: vfio Tainted: G W
>4.9.0-rc2+ #1249
>[ 40.255076] Hardware name: ARM Juno development board (r1) (DT)
>[ 40.260932] task: ffffffc975ee9900 task.stack: ffffffc974d60000
>[ 40.266787] PC is at kobject_get+0x64/0x88
>[ 40.270840] LR is at iommu_bus_notifier+0x40/0x110
>[ 40.275577] pc : [<ffffff800834d20c>] lr : [<ffffff80084c3fd0>]
>pstate: 80000145
>[ 40.282894] sp : ffffffc974d63c00
>[ 40.286169] x29: ffffffc974d63c00 x28: ffffffc974d60000
>[ 40.291431] x27: ffffff80088a2000 x26: 0000000000000040
>[ 40.296692] x25: 0000000000000123 x24: ffffffc974c8f418
>[ 40.301953] x23: 0000000000000006 x22: ffffffc9763f10a0
>[ 40.307214] x21: ffffffc9763e9a00 x20: ffffffc9763f10a0
>[ 40.312474] x19: ffffffc9763ebc80 x18: 0000007fd65069e0
>[ 40.317734] x17: 0000007f8d0ae3c0 x16: ffffff80081c7230
>[ 40.322995] x15: 0000007f8d136588 x14: ffffffffffffffff
>[ 40.328255] x13: 0000000000000004 x12: 0000000000000030
>[ 40.333515] x11: 0000000000000030 x10: 0101010101010101
>[ 40.338775] x9 : feff716475687163 x8 : 7f7f7f7f7f7f7f7f
>[ 40.344035] x7 : feff716475687163 x6 : ffffffc976abf400
>[ 40.349295] x5 : ffffffc976abf400 x4 : 0000000000000000
>[ 40.354555] x3 : ffffff80084c3f90 x2 : ffffffc9763ebcb8
>[ 40.359814] x1 : 0000000000000001 x0 : ffffff8008d4f000
>[ 40.365074]
>[ 40.366542] ---[ end trace 35c1e743d6e6c036 ]---
>[ 40.371107] Call trace:
>[ 40.373523] Exception stack(0xffffffc974d63a30 to 0xffffffc974d63b60)
>[ 40.379895] 3a20: ffffffc9763ebc80
>0000008000000000
>[ 40.387643] 3a40: ffffffc974d63c00 ffffff800834d20c ffffffc976812400
>ffffff8008237d94
>[ 40.395391] 3a60: ffffffbf25d78940 ffffffc974d60000 ffffffc975e259d8
>0000000000005b81
>[ 40.403139] 3a80: ffffffc974d60000 ffffff8008d4b31f ffffff8008b0f000
>ffffffc976811c80
>[ 40.410887] 3aa0: ffffffc974d60000 ffffffc974d60000 ffffffc974d60000
>ffffff8008237000
>[ 40.418634] 3ac0: ffffffc975e259d8 ffffff8008b1b9a8 ffffff8008d4f000
>0000000000000001
>[ 40.426382] 3ae0: ffffffc9763ebcb8 ffffff80084c3f90 0000000000000000
>ffffffc976abf400
>[ 40.434130] 3b00: ffffffc976abf400 feff716475687163 7f7f7f7f7f7f7f7f
>feff716475687163
>[ 40.441877] 3b20: 0101010101010101 0000000000000030 0000000000000030
>0000000000000004
>[ 40.449625] 3b40: ffffffffffffffff 0000007f8d136588 ffffff80081c7230
>0000007f8d0ae3c0
>[ 40.457372] [<ffffff800834d20c>] kobject_get+0x64/0x88
>[ 40.462455] [<ffffff80084c3fd0>] iommu_bus_notifier+0x40/0x110
>[ 40.468227] [<ffffff80080da288>] notifier_call_chain+0x50/0x90
>[ 40.473997] [<ffffff80080da694>] __blocking_notifier_call_chain+0x4c/0x90
>[ 40.480713] [<ffffff80080da6ec>] blocking_notifier_call_chain+0x14/0x20
>[ 40.487259] [<ffffff800853b9e4>] __device_release_driver+0x5c/0x120
>[ 40.493460] [<ffffff800853bacc>] device_release_driver+0x24/0x38
>[ 40.499402] [<ffffff800853a868>] unbind_store+0xe8/0x110
>[ 40.504656] [<ffffff8008539c70>] drv_attr_store+0x20/0x30
>[ 40.509997] [<ffffff800823ab18>] sysfs_kf_write+0x48/0x58
>[ 40.515337] [<ffffff8008239ea8>] kernfs_fop_write+0xb0/0x1d8
>[ 40.520936] [<ffffff80081c507c>] __vfs_write+0x1c/0x100
>[ 40.526104] [<ffffff80081c5e80>] vfs_write+0xa0/0x1b8
>[ 40.531100] [<ffffff80081c7274>] SyS_write+0x44/0xa0
>[ 40.536011] [<ffffff8008082ef0>] el0_svc_naked+0x24/0x28
>[ 40.541324] ata1.00: disabled
>[ 40.544878] sd 0:0:0:0: [sda] Synchronizing SCSI cache
>[ 40.550062] sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result:
>hostbyte=0x04 driverbyte=0x00
>[ 40.558871] sd 0:0:0:0: [sda] Stopping disk
>[ 40.563037] sd 0:0:0:0: [sda] Start/Stop Unit failed: Result:
>hostbyte=0x04 driverbyte=0x00
>[ 40.586990] Unable to handle kernel paging request at virtual address
>0002003e
>[ 40.594702] pgd = ffffffc974c80000
>[ 40.598165] [0002003e] *pgd=00000009f5102003[ 40.602241] ,
>*pud=00000009f5102003
>, *pmd=0000000000000000[ 40.607694]
>[ 40.609171] Internal error: Oops: 96000006 [#1] PREEMPT SMP
>[ 40.614684] Modules linked in:
>[ 40.617712] CPU: 3 PID: 174 Comm: vfio Tainted: G W
>4.9.0-rc2+ #1249
>[ 40.625118] Hardware name: ARM Juno development board (r1) (DT)
>[ 40.630977] task: ffffffc975ee9900 task.stack: ffffffc974d60000
>[ 40.636841] PC is at kobject_get+0x14/0x88
>[ 40.640897] LR is at iommu_get_domain_for_dev+0x1c/0x48
>[ 40.646068] pc : [<ffffff800834d1bc>] lr : [<ffffff80084c3dec>]
>pstate: 60000145
>[ 40.653387] sp : ffffffc974d63c60
>[ 40.656664] x29: ffffffc974d63c60 x28: ffffffc974d60000
>[ 40.661928] x27: ffffff80088a2000 x26: 0000000000000040
>[ 40.667193] x25: 0000000000000123 x24: ffffffc974c8f418
>[ 40.672457] x23: ffffffc974d63eb8 x22: ffffff8008dab568
>[ 40.677720] x21: 000000000000000d x20: ffffff8008dab568
>[ 40.682984] x19: 0000000000020002 x18: 0000000000000000
>[ 40.688246] x17: 0000000000000007 x16: 0000000000000001
>[ 40.693509] x15: ffffffc974cd091c x14: ffffffffffffffff
>[ 40.698773] x13: ffffffc974cd01cd x12: 0000000000000030
>[ 40.704036] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
>[ 40.709300] x9 : 0000000040000000 x8 : 0000000000210d00
>[ 40.714563] x7 : ffffffc975f95018 x6 : 0000000000000000
>[ 40.719826] x5 : 0000000000000000 x4 : ffffffc9763f1210
>[ 40.725088] x3 : 0000000000000000 x2 : ffffffc9763f10e8
>[ 40.730351] x1 : 0000000000000000 x0 : 0000000000020002
>[ 40.735613]
>[ 40.737085] Process vfio (pid: 174, stack limit = 0xffffffc974d60020)
>[ 40.743460] Stack: (0xffffffc974d63c60 to 0xffffffc974d64000)
>[ 40.749150] 3c60: ffffffc974d63c80 ffffff80084c3dec ffffffc9763e9a00
>ffffff80085377a4
>[ 40.756904] 3c80: ffffffc974d63ca0 ffffff80080948c4 ffffffc9763f10a0
>ffffff8008dab568
>[ 40.764658] 3ca0: ffffffc974d63cc0 ffffff8008764a14 ffffffc9763f10a0
>ffffff8008dab568
>[ 40.772411] 3cc0: ffffffc974d63cd0 ffffff8008552804 ffffffc974d63ce0
>ffffff800853ba10
>[ 40.780165] 3ce0: ffffffc974d63d00 ffffff800853bacc ffffffc9763f1100
>ffffffc9763f10a0
>[ 40.787918] 3d00: ffffffc974d63d20 ffffff800853a868 ffffff8008d68f18
>ffffffc9763f10a0
>[ 40.795672] 3d20: ffffffc974d63d50 ffffff8008539c70 000000000000000d
>ffffffc974c8f400
>[ 40.803425] 3d40: ffffffc9757d5880 0000000000000000 ffffffc974d63d60
>ffffff800823ab18
>[ 40.811178] 3d60: ffffffc974d63d70 ffffff8008239ea8 ffffffc974d63dc0
>ffffff80081c507c
>[ 40.818931] 3d80: 000000000000000d 0000000000000000 ffffffc974c8f100
>ffffffc974d63eb8
>[ 40.826684] 3da0: 000000001285f6a0 0000000000000015 0000000000000123
>ffffff80080bf6ac
>[ 40.834437] 3dc0: ffffffc974d63e40 ffffff80081c5e80 000000000000000d
>0000000000000000
>[ 40.842190] 3de0: ffffffc974d63e30 ffffff80080c087c ffffffc974d63e20
>ffffff80081c5c0c
>[ 40.849943] 3e00: ffffffc974c8f100 0000000000000001 ffffffc974c8f100
>ffffffc974d63eb8
>[ 40.857696] 3e20: ffffffc974d63e40 ffffff80081c5f48 000000000000000d
>ffffffc974c8f100
>[ 40.865450] 3e40: ffffffc974d63e80 ffffff80081c7274 ffffffc974c8f100
>ffffffc974c8f100
>[ 40.873203] 3e60: 000000001285f6a0 000000000000000d 0000000060000000
>0000000000000000
>[ 40.880956] 3e80: 0000000000000000 ffffff8008082ef0 0000000000000000
>0000000000000001
>[ 40.888709] 3ea0: ffffffffffffffff 0000007f8d0ae3dc 0000000000000000
>0000000000000000
>[ 40.896461] 3ec0: 0000000000000001 000000001285f6a0 000000000000000d
>0000000000000000
>[ 40.904215] 3ee0: ae2e2e2e3f464b49 0000000000000000 000000001285f6b0
>39322f392f2f2f2f
>[ 40.911968] 3f00: 0000000000000040 fefefeff2f2d2f2f 7f7f7f7f7f7f7f7f
>0101010101010101
>[ 40.919721] 3f20: 0000000000000002 0000000000000004 ffffffffffffffff
>0000007f8d136588
>[ 40.927474] 3f40: 0000000000000000 0000007f8d0ae3c0 0000007fd65069e0
>00000000004ee000
>[ 40.935226] 3f60: 0000000000000001 000000001285f6a0 000000000000000d
>0000000000000001
>[ 40.942980] 3f80: 0000000000000020 000000001285eed8 00000000004ba158
>0000000000000000
>[ 40.950732] 3fa0: 0000000000000000 0000007fd6507f30 000000000040e74c
>0000007fd6507130
>[ 40.958485] 3fc0: 0000007f8d0ae3dc 0000000060000000 0000000000000001
>0000000000000040
>[ 40.966238] 3fe0: 0000000000000000 0000000000000000 0000002000103a00
>4000000010000000
>[ 40.973986] Call trace:
>[ 40.976405] Exception stack(0xffffffc974d63a90 to 0xffffffc974d63bc0)
>[ 40.982780] 3a80: 0000000000020002
>0000008000000000
>[ 40.990533] 3aa0: ffffffc974d63c60 ffffff800834d1bc ffffffc974d63ae0
>ffffff80085377a4
>[ 40.998287] 3ac0: ffffffc974d63b10 ffffff8008537424 ffffffc975e3ac28
>ffffffc975e3ac38
>[ 41.006041] 3ae0: ffffffc974d63b30 ffffff80081737cc ffffffc975e3ac38
>ffffff8008da62c0
>[ 41.013794] 3b00: ffffffc975e98100 ffffff80085401b0 0000000000000001
>ffffff8008540a08
>[ 41.021547] 3b20: 00000000000036b8 0000000000000040 0000000000020002
>0000000000000000
>[ 41.029300] 3b40: ffffffc9763f10e8 0000000000000000 ffffffc9763f1210
>0000000000000000
>[ 41.037053] 3b60: 0000000000000000 ffffffc975f95018 0000000000210d00
>0000000040000000
>[ 41.044805] 3b80: 7f7f7f7f7f7f7f7f 0101010101010101 0000000000000030
>ffffffc974cd01cd
>[ 41.052558] 3ba0: ffffffffffffffff ffffffc974cd091c 0000000000000001
>0000000000000007
>[ 41.060311] [<ffffff800834d1bc>] kobject_get+0x14/0x88
>[ 41.065398] [<ffffff80084c3dec>] iommu_get_domain_for_dev+0x1c/0x48
>[ 41.071607] [<ffffff80080948c4>] arch_teardown_dma_ops+0x14/0x68
>[ 41.077556] [<ffffff8008764a14>] of_dma_deconfigure+0xc/0x18
>[ 41.083161] [<ffffff8008552804>] dma_deconfigure+0xc/0x18
>[ 41.088509] [<ffffff800853ba10>] __device_release_driver+0x88/0x120
>[ 41.094715] [<ffffff800853bacc>] device_release_driver+0x24/0x38
>[ 41.100663] [<ffffff800853a868>] unbind_store+0xe8/0x110
>[ 41.105922] [<ffffff8008539c70>] drv_attr_store+0x20/0x30
>[ 41.111268] [<ffffff800823ab18>] sysfs_kf_write+0x48/0x58
>[ 41.116612] [<ffffff8008239ea8>] kernfs_fop_write+0xb0/0x1d8
>[ 41.122216] [<ffffff80081c507c>] __vfs_write+0x1c/0x100
>[ 41.127390] [<ffffff80081c5e80>] vfs_write+0xa0/0x1b8
>[ 41.132391] [<ffffff80081c7274>] SyS_write+0x44/0xa0
>[ 41.137307] [<ffffff8008082ef0>] el0_svc_naked+0x24/0x28
>[ 41.142567] Code: 910003fd f9000bf3 aa0003f3 b4000180 (3940f000)
>[ 41.148667] ---[ end trace 35c1e743d6e6c037 ]---
>Segmentation fault
>/ #
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel at lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Linus Walleij @ 2016-10-26 14:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477491816.2482.160.camel@baylibre.com>
On Wed, Oct 26, 2016 at 4:23 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> [Me]
>> We usually refer to the local numberspace on the GPIO controller
>> as "offsets", so line offsets 0...31 on a gpiochip with 31 lines.
>>
>> The ngpio in struct gpio_chip is the number of lines on that
>> controller,
>> and should nominally map 1:1 to hwirq sources.
>
> Indeed it should be the the case, and for meson, it is pretty close.
> The irqchip controller provide a number of hwirq. Each hwirq maps to
> one, and only one, pin. But since not every pins are connected to the
> irqchip controller, the opposite is not true.
>
> Taking an example with 16 gpios, here is what it could look like with
> the exception we have on meson :
>
> gpio offset [ 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 ]
> hwirq num [ 0 1 2 3] NC NC[4 5 6 7 8 9 10]NC NC NC
>
> Like gpio offset are used (internally) in the driver to find
> appropriate gpio registers and bit, the hwirq has a meaning too.
> It is the setting you put in the channel multiplexer of the controller
> to select the proper pin to spy on.
>
> In the end, these gpio offset and hwirq number are different. I would
> prefer to have hwirq == gpio and go your way, it would make my life
> easier, but I don't see how it would work.
>
> The irqchip controller cares only about the hwirq number. You can
> actually request an interrupt directly to the controller by asking the
> proper hwirq number (in DT for example), without involving the gpio
> driver (tested).
>
> The relation between the pins and the interrupt number is provided by
> the manufacturer in the Datasheet [1], in the section GPIO Interrupt.
I think I kind of get it.
This reminds me of recent patches to the generic GPIOLIB_IRQCHIP
where we made it possible to "mask" set of IRQ lines, saying
"these are in the irqdomain, but you cannot map them".
See
commit 79b804cb6af4f128b2c53f0887c02537a7eb5824
"gpiolib: Make it possible to exclude GPIOs from IRQ domain"
commit 96b2cca64fa3e1a31b573bb308b2944c802aacf3
"gpio: stmpe: forbid unused lines to be mapped as IRQs"
So what we do in the generic case is put a linear map over all
the lines/offsets, then punch holes in that map and say
"this and this and this can not be mapped as IRQ".
As you can see in _gpiochip_irqchip_add() we pre-map all
except those with irq_create_mapping().
Does this scheme fit with your usecase? I would guess so,
just that your domain is hierarchic, not simple/linear.
Maybe the takeaway is that your map does not need to
be "dense", i.e. every hwirq is in use. It can be sparse.
It is stored in a radix tree anyways.
> Looking at other gpio drivers, it is not uncommon to have some simple
> calculation to get from gpio offset to the hwirq number. I don't get
> what is the specific problem here ?
It's OK to use the offset vs hwirq.
I just misunderstand it as the global GPIO number, that is
not OK.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Linus Walleij @ 2016-10-26 14:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477491766.2482.159.camel@baylibre.com>
On Wed, Oct 26, 2016 at 4:22 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Tue, 2016-10-25 at 20:20 +0200, Linus Walleij wrote:
>> However the semantic is such, that it is not necessary to call
>> to_irq()
>> before using an IRQ: the irqchip and gpiochip abstractions should be
>> orthogonal.
>
> Linus,
>
> They are orthogonal. You can request an irq from the irqchip controller
> without the gpiochip, like any other irq controller.
OK good, sorry if I'm stating the obvious.
> irq_create_mapping (and irq_create_fwspec_mapping) internally calls
> irq_find_mapping. So if the mapping already exist (the irq is already
> used before calling to_irq), the existing mapping will be returned. The
> mapping will be actually created only if needed. It seems to be in line
> with your explanation, no ?
Yes, but you want to call irq_create_mapping() in slowpath (irq setup)
and irq_find_mapping() in fastpath (irq handler). Else the first IRQ
may result in unwelcomed surprises.
> There is really a *lot* of gpio drivers which use irq_create_mapping in
> the to_irq callback, are these all wrong ?
Yes they are all wrong. They should all be using irq_find_mapping().
> If this should not be used, what should we all do instead ?
Call irq_create_mapping() in some other place.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Jerome Brunet @ 2016-10-26 14:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdba_iaOK61KiQGvJZYuz7-PiBgtN4vxv__tJLsZ8ZVL=w@mail.gmail.com>
On Tue, 2016-10-25 at 20:10 +0200, Linus Walleij wrote:
> On Tue, Oct 25, 2016 at 4:47 PM, Marc Zyngier <marc.zyngier@arm.com>
> wrote:
> >
> > On 25/10/16 15:22, Jerome Brunet wrote:
>
> >
> > >
> > > There is a few problems to guarantee that gpio == hwirq.
> > > 1. We have 2 instances of pinctrl, to guarantee that the linux
> > > gpio
> > > number == hwirq, we would have to guarantee the order in which
> > > they are
> > > probed. At least this my understanding
> >
> > Maybe I wasn't clear enough, and my use of gpio is probably wrong.
> > So
> > Linux has a gpio number, which is obviously an abstract number
> > (just
> > like the Linux irq number). But the pad number, in the context of
> > given
> > SoC, is constant. So we have:
> >
> > ????????pad->gpio
> > ????????hwirq->irq
> >
> > Why can't you have pad == hwirq, always? This is already what you
> > have
> > in the irqchip driver. This would simplify a lot of things.
>
> My thought as well.
>
> We usually refer to the local numberspace on the GPIO controller
> as "offsets", so line offsets 0...31 on a gpiochip with 31 lines.
>
> The ngpio in struct gpio_chip is the number of lines on that
> controller,
> and should nominally map 1:1 to hwirq sources.
Indeed it should be the the case, and for meson, it is pretty close.
The irqchip controller provide a number of hwirq. Each hwirq maps to
one, and only one, pin. But since not every pins are connected to the
irqchip controller, the opposite is not true.
Taking an example with 16 gpios, here is what it could look like with
the exception we have on meson :
gpio offset [ 0 ?1 ?2 ?3 ?4 ?5 ?6 ?7 ?8 ?9 ?10 11 12 13 14 15 ]
hwirq num ? [ 0 ?1 ?2 ?3] NC NC[4 ?5 ?6 ?7 ?8 ?9 ?10]NC NC NC
Like gpio offset are used (internally) in the driver to find
appropriate gpio registers and bit, the hwirq has a meaning too.
It is the setting you put in the channel multiplexer of the controller
to select the proper pin to spy on.
In the end, these gpio offset and hwirq number are different. I would
prefer to have hwirq == gpio and go your way, it would make my life
easier, but I don't see how it would work.
The irqchip controller cares only about the hwirq number. You can
actually request an interrupt directly to the controller by asking the
proper hwirq number (in DT for example), without involving the gpio
driver (tested).
The relation between the pins and the interrupt number is provided by
the manufacturer in the Datasheet [1], in the section GPIO Interrupt.
Looking at other gpio drivers, it is not uncommon to have some simple
calculation to get from gpio offset to the hwirq number. I don't get
what is the specific problem here ?
If I missed something, feel free to point it out.
[1]?http://dn.odroid.com/S805/Datasheet/S805_Datasheet%20V0.8%202015012
6.pdf
>
> Yours,
> Linus Walleij
^ permalink raw reply
* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Jerome Brunet @ 2016-10-26 14:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdbGo4BJOdzkgBrE9jT-rKodd4zssCnOtOuGS+OqV-Uc6Q@mail.gmail.com>
On Tue, 2016-10-25 at 20:20 +0200, Linus Walleij wrote:
> On Tue, Oct 25, 2016 at 5:31 PM, Jerome Brunet <jbrunet@baylibre.com>
> wrote:
> >
> > On Tue, 2016-10-25 at 15:47 +0100, Marc Zyngier wrote:
>
> >
> > >
> > > Is gpio_to_irq() supposed to allocate an interrupt? Or merely to
> > > report the existence of a mapping?
>
> It should provide an IRQ corresponding to the gpio line, if possible.
>
> However the semantic is such, that it is not necessary to call
> to_irq()
> before using an IRQ: the irqchip and gpiochip abstractions should be
> orthogonal.
Linus,
They are?orthogonal. You can request an irq from the irqchip controller
without the gpiochip, like any other irq controller.
>
> This goes especially when using device tree or ACPI, where you
> may reference an IRQ from something modeled as irqchip, which
> is simultaneously a gpiochip.
>
> >
> > Linus, please correct me if I'm wrong,
> > .to_irq gets the linux gpio number and returns the linux virtual
> > irq
> > numbers, 0 if there is no interrupt.
>
> Yes. But it may *or may not* be called before using the IRQ.
>
> So it should look up or try to create a mapping on request, but not
> assume to have been called before using some line as IRQ.
>
> The only thing you should assume to be called before an interrupt
> is put to use is the stuff in irqchip. So you have to do your
> dynamic irqdomain mapping elsewhere than .to_irq().
irq_create_mapping (and irq_create_fwspec_mapping) internally calls
irq_find_mapping. So if the mapping already exist (the irq is already
used before calling to_irq), the existing mapping will be returned. The
mapping will be actually created only if needed. It seems to be in line
with your explanation, no ?
There is really a *lot* of gpio drivers which use irq_create_mapping in
the to_irq callback, are these all wrong ?
If this should not be used, what should we all do instead ??
>
> Yours,
> Linus Walleij
^ permalink raw reply
* [PATCH V3 4/8] drivers: platform: Configure dma operations at probe time
From: Robin Murphy @ 2016-10-26 14:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475600632-21289-5-git-send-email-sricharan@codeaurora.org>
+Lorenzo
On 04/10/16 18:03, Sricharan R wrote:
> Configuring DMA ops at probe time will allow deferring device probe when
> the IOMMU isn't available yet. The dma_configure for the device is now called
> from the generic device_attach callback just before the bus/driver probe
> is called. This way, configuring the dma ops for the device would be called
> at the same place for all bus_types, hence the deferred probing mechanism
> should work for all buses as well.
>
> pci_bus_add_devices (platform/amba)(_device_create/driver_register)
> | |
> pci_bus_add_device (device_add/driver_register)
> | |
> device_attach device_initial_probe
> | |
> __device_attach_driver __device_attach_driver
> |
> driver_probe_device
> |
> really_probe
> |
> dma_configure
>
> Similarly on the device/driver_unregister path __device_release_driver is
> called which inturn calls dma_deconfigure.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
> drivers/base/dd.c | 10 ++++++++++
> drivers/base/dma-mapping.c | 11 +++++++++++
> drivers/of/platform.c | 4 ----
> drivers/pci/probe.c | 5 +----
> include/linux/dma-mapping.h | 3 +++
> 5 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 16688f5..cfebd48 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -19,6 +19,7 @@
>
> #include <linux/device.h>
> #include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> #include <linux/module.h>
> #include <linux/kthread.h>
> #include <linux/wait.h>
> @@ -353,6 +354,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
> if (ret)
> goto pinctrl_bind_failed;
>
> + ret = dma_configure(dev);
> + if (ret)
> + goto dma_failed;
> +
> if (driver_sysfs_add(dev)) {
> printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n",
> __func__, dev_name(dev));
> @@ -395,6 +400,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
> goto done;
>
> probe_failed:
> + dma_deconfigure(dev);
> +dma_failed:
> if (dev->bus)
> blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
> BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
> @@ -780,6 +787,9 @@ static void __device_release_driver(struct device *dev)
> dev->bus->remove(dev);
> else if (drv->remove)
> drv->remove(dev);
> +
> + dma_deconfigure(dev);
> +
> devres_release_all(dev);
> dev->driver = NULL;
> dev_set_drvdata(dev, NULL);
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index d799662..54e87f5 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -10,6 +10,7 @@
> #include <linux/dma-mapping.h>
> #include <linux/export.h>
> #include <linux/gfp.h>
> +#include <linux/of_device.h>
> #include <linux/slab.h>
> #include <linux/vmalloc.h>
>
> @@ -166,6 +167,16 @@ void dmam_free_noncoherent(struct device *dev, size_t size, void *vaddr,
> }
> EXPORT_SYMBOL(dmam_free_noncoherent);
>
> +int dma_configure(struct device *dev)
> +{
> + return of_dma_configure(dev, dev->of_node);
> +}
> +
> +void dma_deconfigure(struct device *dev)
> +{
> + of_dma_deconfigure(dev);
> +}
> +
> #ifdef CONFIG_HAVE_GENERIC_DMA_COHERENT
>
> static void dmam_coherent_decl_release(struct device *dev, void *res)
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 9cb7090..adbd77c 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -181,11 +181,9 @@ static struct platform_device *of_platform_device_create_pdata(
>
> dev->dev.bus = &platform_bus_type;
> dev->dev.platform_data = platform_data;
> - of_dma_configure(&dev->dev, dev->dev.of_node);
> of_msi_configure(&dev->dev, dev->dev.of_node);
>
> if (of_device_add(dev) != 0) {
> - of_dma_deconfigure(&dev->dev);
> platform_device_put(dev);
> goto err_clear_flag;
> }
> @@ -242,7 +240,6 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
> dev_set_name(&dev->dev, "%s", bus_id);
> else
> of_device_make_bus_id(&dev->dev);
> - of_dma_configure(&dev->dev, dev->dev.of_node);
>
> /* Allow the HW Peripheral ID to be overridden */
> prop = of_get_property(node, "arm,primecell-periphid", NULL);
> @@ -536,7 +533,6 @@ static int of_platform_device_destroy(struct device *dev, void *data)
> amba_device_unregister(to_amba_device(dev));
> #endif
>
> - of_dma_deconfigure(dev);
> of_node_clear_flag(dev->of_node, OF_POPULATED);
> of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
> return 0;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 93f280d..85c9553 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1724,10 +1724,7 @@ static void pci_dma_configure(struct pci_dev *dev)
> {
> struct device *bridge = pci_get_host_bridge_device(dev);
>
> - if (IS_ENABLED(CONFIG_OF) &&
> - bridge->parent && bridge->parent->of_node) {
> - of_dma_configure(&dev->dev, bridge->parent->of_node);
> - } else if (has_acpi_companion(bridge)) {
> + if (has_acpi_companion(bridge)) {
> struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
> enum dev_dma_attr attr = acpi_get_dma_attr(adev);
It seems a bit awkward leaving pci_dma_configure here, doing DMA
configuration at device add, after we've allegedly moved DMA
configuration to driver probe. Lorenzo, do you foresee any issues if
this probe-time of_dma_configure() path were to also multiplex
acpi_dma_configure() in future, such that everything would be back in
the same place eventually?
Conversely, is there actually any issue with leaving pci_dma_configure()
unchanged, and simply moving the call from pci_device_add() into
dma_configure()?
Robin.
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 66533e1..2766dbe 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -656,6 +656,9 @@ dma_mark_declared_memory_occupied(struct device *dev,
> }
> #endif /* CONFIG_HAVE_GENERIC_DMA_COHERENT */
>
> +int dma_configure(struct device *dev);
> +void dma_deconfigure(struct device *dev);
> +
> /*
> * Managed DMA API
> */
>
^ permalink raw reply
* [PATCH 05/14] mfd: sun6i-prcm: Add sun8i analog codec as subnode
From: Lee Jones @ 2016-10-26 14:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c74aea452bd1831f439dd67884cb2879e0849f03.1475571575.git.mylene.josserand@free-electrons.com>
On Tue, 04 Oct 2016, Myl?ne Josserand wrote:
> The sun8i audio codec is using PRCM registers to configure all the
> analog part of the audio codec. It is added as a subnode of the PRCM
> with his resource (offset of 0x1c0).
>
> Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com>
> ---
> drivers/mfd/sun6i-prcm.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
> index 011fcc5..e0c6bf5 100644
> --- a/drivers/mfd/sun6i-prcm.c
> +++ b/drivers/mfd/sun6i-prcm.c
> @@ -12,6 +12,8 @@
> #include <linux/init.h>
> #include <linux/of.h>
>
> +#define SUN6I_AUDIO_CODEC_ANALOG 0x1c0
> +
> struct prcm_data {
> int nsubdevs;
> const struct mfd_cell *subdevs;
> @@ -57,6 +59,14 @@ static const struct resource sun6i_a31_apb0_rstc_res[] = {
> },
> };
>
> +static const struct resource sun8i_adda_res[] = {
> + {
> + .start = SUN6I_AUDIO_CODEC_ANALOG,
> + .end = 0x1c3,
This also needs defining. No magic numbers please.
> + .flags = IORESOURCE_MEM,
> + },
> +};
> +
> static const struct mfd_cell sun6i_a31_prcm_subdevs[] = {
> {
> .name = "sun6i-a31-ar100-clk",
> @@ -109,6 +119,12 @@ static const struct mfd_cell sun8i_a23_prcm_subdevs[] = {
> .num_resources = ARRAY_SIZE(sun6i_a31_apb0_rstc_res),
> .resources = sun6i_a31_apb0_rstc_res,
> },
> + {
> + .name = "sun8i-codec-analog",
> + .of_compatible = "allwinner,sun8i-codec-analog",
> + .num_resources = ARRAY_SIZE(sun8i_adda_res),
> + .resources = sun8i_adda_res,
> + },
> };
>
> static const struct prcm_data sun6i_a31_prcm_data = {
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH] mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading
From: Lee Jones @ 2016-10-26 14:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161005155112.13774-2-hdegoede@redhat.com>
On Wed, 05 Oct 2016, Hans de Goede 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>
> ---
> drivers/mfd/axp20x-i2c.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
Applied (for now), thanks.
> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
> index b1b8658..d35a5fe 100644
> --- a/drivers/mfd/axp20x-i2c.c
> +++ b/drivers/mfd/axp20x-i2c.c
> @@ -69,10 +69,11 @@ static const struct of_device_id axp20x_i2c_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
>
> -/*
> - * This is useless for OF-enabled devices, but it is needed by I2C subsystem
> - */
> static const struct i2c_device_id axp20x_i2c_id[] = {
> + { "axp152", 0 },
> + { "axp202", 0 },
> + { "axp209", 0 },
> + { "axp221", 0 },
> { },
> };
> MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH] mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading
From: Lee Jones @ 2016-10-26 14:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024104747.GC1440@katana>
On Mon, 24 Oct 2016, Wolfram Sang wrote:
>
> > 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.
I met with Kieran. He's submitted a new version.
Here's hoping! ;)
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH 7/7] mfd: tps65217: Fix mismatched interrupt number
From: Lee Jones @ 2016-10-26 13:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161021140106.21531-8-woogyom.kim@gmail.com>
On Fri, 21 Oct 2016, Milo Kim wrote:
> Enum value of 'tps65217_irq_type' is not matched with DT parsed hwirq
> number[*].
>
> The MFD driver gets the IRQ data by referencing hwirq, but the value is
> different. So, irq_to_tps65217_irq() returns mismatched IRQ data.
> Eventually, the power button driver enables not PB but USB interrupt
> when it is probed.
>
> According to the TPS65217 register map[**], USB interrupt is the LSB.
> This patch synchronizes TPS65217 IRQ index.
>
> [*] include/dt-bindings/mfd/tps65217.h
> [**] http://www.ti.com/lit/ds/symlink/tps65217.pdf
>
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
> include/linux/mfd/tps65217.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/mfd/tps65217.h b/include/linux/mfd/tps65217.h
> index 4ccda89..75a3a5f 100644
> --- a/include/linux/mfd/tps65217.h
> +++ b/include/linux/mfd/tps65217.h
> @@ -235,9 +235,9 @@ struct tps65217_bl_pdata {
> };
>
> enum tps65217_irq_type {
> - TPS65217_IRQ_PB,
> - TPS65217_IRQ_AC,
> TPS65217_IRQ_USB,
> + TPS65217_IRQ_AC,
> + TPS65217_IRQ_PB,
> TPS65217_NUM_IRQ
> };
This is why using enum for these types of assignments is sometimes
dangerous. It's probably best to be explicit.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH 0/7] ARM: dts: support I2SE Duckbill device
From: Shawn Guo @ 2016-10-26 13:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2694949.M3JJFNG6Xz@kerker>
On Mon, Oct 24, 2016 at 09:45:56PM +0200, Michael Heimpold wrote:
> Actually it would even be 11 files for 8 boards, thats right.
> There is no mechanism (no EEPROM, GPIO...) to automatically probe for the
> device variants, and the "variant-forming daugther boards" are
> not switchable like the expansion boards e.g. for Raspberry Pi.
> Another point is, that this would really require to use U-Boot to apply
> the overlay (or another DT capable bootloader with this functionality).
> For booting very fast, the good old Freescale bootlets are still
> an option, which could not handle the overlays - at least at the moment...
> However, it still an option to have a second look...
>
> To reduce the number of files, I see the following other approaches:
> - don't use the three "common" files which are included: -3 files (8 total)
> - don't mainline Duckbill EnOcean, Duckbill 485 and Duckbill SPI:
> they are EOL, only Duckbill 2 variants are still sold;
> however, since Duckbill (without 2) is/was already mainlined, we
> should keep this, leaving 4 new variants + 1 old variant: -3 files (5 total)
> - only mainline Duckbill 2 (generic board) and maintain all other variants
> in a private Github repo (this is what we are doing now): -3 files (2 total)
>
> So, I'll discuss this again internally. Do you have any preference?
I'm fine with the second one.
Shawn
^ permalink raw reply
* [PATCH 4/7] ARM: dts: add I2SE Duckbill common definitions
From: Shawn Guo @ 2016-10-26 13:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477164150-13553-5-git-send-email-mhei@heimpold.de>
On Sat, Oct 22, 2016 at 09:22:27PM +0200, Michael Heimpold wrote:
> + pinctrl at 80018000 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hog_pins_a>;
> +
> + mac0_phy_reset_pin: mac0_phy_reset_pin at 0 {
Please use hyphen instead of underscore in node name.
Shawn
> + reg = <0>;
> + fsl,pinmux-ids = <
> + MX28_PAD_GPMI_ALE__GPIO_0_26 /* PHY Reset */
> + >;
> + fsl,drive-strength = <MXS_DRIVE_4mA>;
> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
> + fsl,pull-up = <MXS_PULL_DISABLE>;
> + };
^ permalink raw reply
* [PATCH v2] clocksource/arm_arch_timer: Map frame with of_io_request_and_map()
From: Marc Zyngier @ 2016-10-26 13:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161026073550.22918-1-sboyd@codeaurora.org>
On 26/10/16 08:35, Stephen Boyd wrote:
> Let's use the of_io_request_and_map() API so that the frame
> region is protected and shows up in /proc/iomem.
>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>
> Changes from v1:
> * Check for IS_ERR instead
>
> drivers/clocksource/arm_arch_timer.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 73c487da6d2a..723cc924d8d1 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -964,8 +964,9 @@ static int __init arch_timer_mem_init(struct device_node *np)
> }
>
> ret= -ENXIO;
> - base = arch_counter_base = of_iomap(best_frame, 0);
> - if (!base) {
> + base = arch_counter_base = of_io_request_and_map(best_frame, 0,
> + "arch_mem_timer");
> + if (IS_ERR(base)) {
> pr_err("arch_timer: Can't map frame's registers\n");
> goto out;
> }
>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH 1/2] of, numa: Add function to disable of_node_to_nid().
From: Robert Richter @ 2016-10-26 13:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477431061-7258-2-git-send-email-ddaney.cavm@gmail.com>
On 25.10.16 14:31:00, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
>
> On arm64 NUMA kernels we can pass "numa=off" on the command line to
> disable NUMA. A side effect of this is that kmalloc_node() calls to
> non-zero nodes will crash the system with an OOPS:
>
> [ 0.000000] [<fffffc00081bba84>] __alloc_pages_nodemask+0xa4/0xe68
> [ 0.000000] [<fffffc00082163a8>] new_slab+0xd0/0x57c
> [ 0.000000] [<fffffc000821879c>] ___slab_alloc+0x2e4/0x514
> [ 0.000000] [<fffffc000823882c>] __slab_alloc+0x48/0x58
> [ 0.000000] [<fffffc00082195a0>] __kmalloc_node+0xd0/0x2e0
> [ 0.000000] [<fffffc00081119b8>] __irq_domain_add+0x7c/0x164
> [ 0.000000] [<fffffc0008b75d30>] its_probe+0x784/0x81c
> [ 0.000000] [<fffffc0008b75e10>] its_init+0x48/0x1b0
> .
> .
> .
>
> This is caused by code like this in kernel/irq/irqdomain.c
>
> domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
> GFP_KERNEL, of_node_to_nid(of_node));
>
> When NUMA is disabled, the concept of a node is really undefined, so
> of_node_to_nid() should unconditionally return NUMA_NO_NODE.
>
> Add __of_force_no_numa() to allow of_node_to_nid() to be forced to
> return NUMA_NO_NODE.
>
> The follow on patch will call this new function from the arm64 numa
> code.
Didn't that work before? numa=off just maps all mem to node 0. If mem
allocation is requested for another node it should just fall back to a
node with mem (node 0 then). I suspect there is something wrong with
the page initialization, see:
http://www.spinics.net/lists/arm-kernel/msg535191.html
https://bugzilla.redhat.com/show_bug.cgi?id=1387793
What is the complete oops?
So I think k*alloc_node() must be able to handle requests to
non-existing nodes. Otherwise your fix is incomplete, assume a failed
of_numa_init() causing a dummy init but still some devices reporting a
node.
-Robert
^ permalink raw reply
* [PATCH v14 4/9] acpi/arm64: Add GTDT table parse driver
From: Fu Wei @ 2016-10-26 13:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57010864-b2f0-d230-3138-8ace484acb04@arm.com>
Hi Marc,
On 26 October 2016 at 20:11, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 26/10/16 12:10, Fu Wei wrote:
>> Hi Mark,
>>
>> On 21 October 2016 at 00:37, Mark Rutland <mark.rutland@arm.com> wrote:
>>> Hi,
>>>
>>> As a heads-up, on v4.9-rc1 I see conflicts at least against
>>> arch/arm64/Kconfig. Luckily git am -3 seems to be able to fix that up
>>> automatically, but this will need to be rebased before the next posting
>>> and/or merging.
>>>
>>> On Thu, Sep 29, 2016 at 02:17:12AM +0800, fu.wei at linaro.org wrote:
>>>> +static int __init map_gt_gsi(u32 interrupt, u32 flags)
>>>> +{
>>>> + int trigger, polarity;
>>>> +
>>>> + if (!interrupt)
>>>> + return 0;
>>>
>>> Urgh.
>>>
>>> Only the secure interrupt (which we do not need) is optional in this
>>> manner, and (hilariously), zero appears to also be a valid GSIV, per
>>> figure 5-24 in the ACPI 6.1 spec.
>>>
>>> So, I think that:
>>>
>>> (a) we should not bother parsing the secure interrupt
>>
>> If I understand correctly, from this point of view, kernel don't
>> handle the secure interrupt.
>> But the current arm_arch_timer driver still enable/disable/request
>> PHYS_SECURE_PPI
>> with PHYS_NONSECURE_PPI.
>> That means we still need to parse the secure interrupt.
>> Please correct me, if I misunderstand something? :-)
>
> That's because we can use the per-cpu timer when 32bit Linux is running
> on the secure side (and we cannot distinguish between secure and
> non-secure at runtime). ACPI is 64bit only, and Linux on 64bit isn't
> supported on the secure side, so only registering the non-secure timer
> is perfectly acceptable.
Great thanks for your explanation :-)
So we just don't need to fill arch_timer_ppi[PHYS_SECURE_PPI] , just skip it.
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
--
Best regards,
Fu Wei
Software Engineer
Red Hat
^ permalink raw reply
* [PATCH v4 0/7] Add R8A7743/SK-RZG1M board support
From: Geert Uytterhoeven @ 2016-10-26 13:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1a85f851-29ec-1c14-8905-8b0645f5bb1c@cogentembedded.com>
On Wed, Oct 26, 2016 at 3:23 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 10/26/2016 03:08 PM, Geert Uytterhoeven wrote:
>>> Here's the set of 8 patches against Simon Horman's 'renesas.git'
>>> repo's
>>> 'renesas-devel-20161021-v4.9-rc1' tag. I'm adding the device tree support
>>> for
>>> the R8A7743-based SK-RZG1M board. The SoC is close to R8A7791 and the
>>> board
>>> seems identical to the R8A7791/Porter board. The device tree patches
>>> depend on
>>> the R8A7743 CPG/MSSR driver series just posted in order to compile and
>>> work.
>>
>>
>> They depend only on "[PATCH v3 1/2] ARM: shmobile: r8a7743: add CPG clock
>> index macros" of that series, right?
>>
>> "[PATCH v3 2/2] clk: renesas: cpg-mssr: add R8A7743 support" is not
>> needed,
>
> How would "clocks" props _work_ without this patch?
Sorry, I was focusing too much on "compile"...
Got my coke, switching brain to overdrive mode...
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 v4 0/7] Add R8A7743/SK-RZG1M board support
From: Sergei Shtylyov @ 2016-10-26 13:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdUNgnYc1O7DATNH1r9SQ72eqtxwFvqcen08Tq+Er3LuvQ@mail.gmail.com>
On 10/26/2016 03:08 PM, Geert Uytterhoeven wrote:
>> Here's the set of 8 patches against Simon Horman's 'renesas.git' repo's
>> 'renesas-devel-20161021-v4.9-rc1' tag. I'm adding the device tree support for
>> the R8A7743-based SK-RZG1M board. The SoC is close to R8A7791 and the board
>> seems identical to the R8A7791/Porter board. The device tree patches depend on
>> the R8A7743 CPG/MSSR driver series just posted in order to compile and work.
>
> They depend only on "[PATCH v3 1/2] ARM: shmobile: r8a7743: add CPG clock
> index macros" of that series, right?
>
> "[PATCH v3 2/2] clk: renesas: cpg-mssr: add R8A7743 support" is not needed,
How would "clocks" props _work_ without this patch?
> and introduces an additional dependency on "[PATCH 1/3] clk: renesas:
> cpg-mssr: add common R-Car Gen2 support".
Yes, of course...
> Gr{oetje,eeting}s,
>
> Geert
MBR, Sergei
^ permalink raw reply
* [PATCH v3] drivers: psci: PSCI checker module
From: Lorenzo Pieralisi @ 2016-10-26 13:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025183436.GF3716@linux.vnet.ibm.com>
On Tue, Oct 25, 2016 at 11:34:36AM -0700, Paul E. McKenney wrote:
[...]
> > > +static int __init psci_checker(void)
> > > +{
> > > + int ret;
> > > +
> > > + /*
> > > + * Since we're in an initcall, we assume that all the CPUs that all
> > > + * CPUs that can be onlined have been onlined.
> > > + *
> > > + * The tests assume that hotplug is enabled but nobody else is using it,
> > > + * otherwise the results will be unpredictable. However, since there
> > > + * is no userspace yet in initcalls, that should be fine.
> >
> > I do not think it is. If you run a kernel with, say,
> > CONFIG_LOCK_TORTURE_TEST, cpus may disappear from the radar while
> > running the PSCI checker test itself; that at least would confuse the
> > checker, and that's just an example.
> >
> > I added Paul to check what are the assumptions behind the torture test
> > hotplug tests, in particular if there are any implicit assumptions for
> > it to work (ie it is the only kernel test hotplugging cpus in and out
> > (?)), what I know is that the PSCI checker assumptions are not correct.
>
> Both CONFIG_LOCK_TORTURE_TEST and CONFIG_RCU_TORTURE_TEST can and will
> hotplug CPUs. The locktorture.onoff_holdoff and rcutorture.onoff_holdoff
> kernel parameters can delay the start of CPU-hotplug testing, and in
> my testing I set this delay to 30 seconds after boot.
>
> One approach would be to make your test refuse to run if either of
> the lock/RCU torture tests was running. Or do what Lorenzo suggests
> below. The torture tests aren't crazy enough to offline the last CPU.
> Though they do try, just for effect, in cases where the last CPU is
> marked cpu_is_hotpluggable(). ;-)
Thank you Paul. I have an additional question though. Is there any
implicit assumption in LOCK/RCU torture tests whereby nothing else
in the kernel is hotplugging cpus in/out (through cpu_down()/up())
while they are running ?
I am asking because that's the main reason behind my query. Those tests
hotplug cpus in and out through cpu_down/up() but AFAICS nothing
prevents another piece of code in the kernel to call those functions and
the tests may just fail in that case (ie trying to cpu_down() a cpu
that is not online).
Are false negatives contemplated (or I am missing something) ?
I just would like to understand if what this patch currently does
is safe and sound. I think that calling cpu_down() and cpu_up()
is always safe, but the test can result in false negatives if
other kernel subsystems (eg LOCK torture test) are calling those
APIs in parallel (because cpu_down()/cpu_up() calls can fail - eg
trying to cpu_down() a cpu that is not online any longer, since it
was taken down by another kernel control path), that's the question
I have.
Thanks !
Lorenzo
>
> Thanx, Paul
>
> > There is something simple you can do to get this "fixed".
> >
> > You can use the new API James implemented for hibernate,
> > that allows you to disable (ie PSCI CPU OFF) all "secondary" cpus
> > other than the primary one passed in as parameter:
> >
> > freeze_secondary_cpus(int primary);
> >
> > that function will _cpu_down() all online cpus other than "primary"
> > in one go, without any interference allowed from other bits of the
> > kernel. It requires an enable_nonboot_cpus() counterpart, and you
> > can do that for every online cpus you detect (actually you can even
> > avoid using the online cpu mask and use the present mask to carry
> > out the test). If there is a resident trusted OS you can just
> > trigger the test with primary == tos_resident_cpu, since all
> > others are bound to fail (well, you can run them and check they
> > do fail, it is a checker after all).
> >
> > You would lose the capability of hotplugging a "cluster" at a time, but
> > I do not think it is a big problem, the test above would cover it
> > and more importantly, it is safe to execute.
> >
> > Or we can augment the torture test API to restrict the cpumask
> > it actually uses to offline/online cpus (I am referring to
> > torture_onoff(), kernel/torture.c).
> >
> > Further comments appreciated since I am not sure I grokked the
> > assumption the torture tests make about hotplugging cpus in and out,
> > I will go through the commits logs again to find more info.
> >
> > Thanks !
> > Lorenzo
> >
> > > + */
> > > + nb_available_cpus = num_online_cpus();
> > > +
> > > + /* Check PSCI operations are set up and working. */
> > > + ret = psci_ops_check();
> > > + if (ret)
> > > + return ret;
> > > +
> > > + pr_info("PSCI checker started using %u CPUs\n", nb_available_cpus);
> > > +
> > > + pr_info("Starting hotplug tests\n");
> > > + ret = hotplug_tests();
> > > + if (ret == 0)
> > > + pr_info("Hotplug tests passed OK\n");
> > > + else if (ret > 0)
> > > + pr_err("%d error(s) encountered in hotplug tests\n", ret);
> > > + else {
> > > + pr_err("Out of memory\n");
> > > + return ret;
> > > + }
> > > +
> > > + pr_info("Starting suspend tests (%d cycles per state)\n",
> > > + NUM_SUSPEND_CYCLE);
> > > + ret = suspend_tests();
> > > + if (ret == 0)
> > > + pr_info("Suspend tests passed OK\n");
> > > + else if (ret > 0)
> > > + pr_err("%d error(s) encountered in suspend tests\n", ret);
> > > + else {
> > > + switch (ret) {
> > > + case -ENOMEM:
> > > + pr_err("Out of memory\n");
> > > + break;
> > > + case -ENODEV:
> > > + pr_warn("Could not start suspend tests on any CPU\n");
> > > + break;
> > > + }
> > > + }
> > > +
> > > + pr_info("PSCI checker completed\n");
> > > + return ret < 0 ? ret : 0;
> > > +}
> > > +late_initcall(psci_checker);
> > > --
> > > 2.10.0
> > >
> >
>
^ permalink raw reply
* [PATCH] mfd: qcom-pm8xxx: Clean up PM8XXX namespace
From: Linus Walleij @ 2016-10-26 13:10 UTC (permalink / raw)
To: linux-arm-kernel
The Kconfig and file naming for the PM8xxx driver is totally
confusing:
- Kconfig options MFD_PM8XXX and MFD_PM8921_CORE, some in-kernel
users depending on or selecting either at random.
- A driver file named pm8921-core.c even if it is indeed
used by the whole PM8xxx family of chips.
- An irqchip named pm8xxx since it was (I guess) realized that
the driver was generic for all pm8xxx PMICs.
As I may want to add support for PM8901 this is starting to get
really messy. Fix this situation by:
- Remove the MFD_PM8921_CORE symbol and rely solely on MFD_PM8XXX
and convert all users, including LEDs Kconfig and ARM defconfigs
for qcom and multi_v7 to use that single symbol.
- Renaming the driver to qcom-pm8xxx.c to fit along the two
other qcom* prefixed drivers.
- Rename functions withing the driver from 8921 to 8xxx to
indicate it is generic.
- Just drop the =m config from the pxa_defconfig, I have no clue
why it is even there, it is not a Qualcomm platform. (Possibly
older Kconfig noise from saveconfig.)
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I do NOT think it is a good idea to try to split this commit up,
I rather prefer that Lee simply merge it into MFD.
The reason is that files like qcom_defconfig already contain both
the right symbols, but the MFD_PM8921_CORE symbol cannot be removed
until this rename has happened, whereas multi_v7_defconfig needs
it added etc, and this is just a clean nice cut.
Jacek, ARM SoC person: please ACK this patch to get merged into
MFD.
---
arch/arm/configs/multi_v7_defconfig | 2 +-
arch/arm/configs/pxa_defconfig | 1 -
arch/arm/configs/qcom_defconfig | 1 -
drivers/leds/Kconfig | 2 +-
drivers/mfd/Kconfig | 14 ++++------
drivers/mfd/Makefile | 2 +-
drivers/mfd/{pm8921-core.c => qcom-pm8xxx.c} | 42 ++++++++++++++--------------
7 files changed, 29 insertions(+), 35 deletions(-)
rename drivers/mfd/{pm8921-core.c => qcom-pm8xxx.c} (92%)
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 437d0740dec6..4280de7b9b4e 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -489,7 +489,7 @@ CONFIG_MFD_MAX8907=y
CONFIG_MFD_MAX8997=y
CONFIG_MFD_MAX8998=y
CONFIG_MFD_RK808=y
-CONFIG_MFD_PM8921_CORE=y
+CONFIG_MFD_PM8XXX=y
CONFIG_MFD_QCOM_RPM=y
CONFIG_MFD_SPMI_PMIC=y
CONFIG_MFD_SEC_CORE=y
diff --git a/arch/arm/configs/pxa_defconfig b/arch/arm/configs/pxa_defconfig
index a016ecc0084b..e4314b1227a3 100644
--- a/arch/arm/configs/pxa_defconfig
+++ b/arch/arm/configs/pxa_defconfig
@@ -411,7 +411,6 @@ CONFIG_MFD_MAX77693=y
CONFIG_MFD_MAX8907=m
CONFIG_EZX_PCAP=y
CONFIG_UCB1400_CORE=m
-CONFIG_MFD_PM8921_CORE=m
CONFIG_MFD_SEC_CORE=y
CONFIG_MFD_PALMAS=y
CONFIG_MFD_TPS65090=y
diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig
index c2dff4fd5fc4..74e9cd759b99 100644
--- a/arch/arm/configs/qcom_defconfig
+++ b/arch/arm/configs/qcom_defconfig
@@ -119,7 +119,6 @@ CONFIG_POWER_RESET=y
CONFIG_POWER_RESET_MSM=y
CONFIG_THERMAL=y
CONFIG_MFD_PM8XXX=y
-CONFIG_MFD_PM8921_CORE=y
CONFIG_MFD_QCOM_RPM=y
CONFIG_MFD_SPMI_PMIC=y
CONFIG_REGULATOR=y
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 7a628c6516f6..86bb1515a00e 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -645,7 +645,7 @@ config LEDS_VERSATILE
config LEDS_PM8058
tristate "LED Support for the Qualcomm PM8058 PMIC"
- depends on MFD_PM8921_CORE
+ depends on MFD_PM8XXX
depends on LEDS_CLASS
help
Choose this option if you want to use the LED drivers in
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c6df6442ba2b..1ed0584f494e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -756,24 +756,20 @@ config UCB1400_CORE
module will be called ucb1400_core.
config MFD_PM8XXX
- tristate
-
-config MFD_PM8921_CORE
- tristate "Qualcomm PM8921 PMIC chip"
+ tristate "Qualcomm PM8xxx PMIC chips driver"
depends on (ARM || HEXAGON)
select IRQ_DOMAIN
select MFD_CORE
- select MFD_PM8XXX
select REGMAP
help
If you say yes to this option, support will be included for the
- built-in PM8921 PMIC chip.
+ built-in PM8xxx PMIC chips.
- This is required if your board has a PM8921 and uses its features,
+ This is required if your board has a PM8xxx and uses its features,
such as: MPPs, GPIOs, regulators, interrupts, and PWM.
- Say M here if you want to include support for PM8921 chip as a module.
- This will build a module called "pm8921-core".
+ Say M here if you want to include support for PM8xxx chips as a
+ module. This will build a module called "pm8xxx-core".
config MFD_QCOM_RPM
tristate "Qualcomm Resource Power Manager (RPM)"
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 9834e669d985..7bb5a50127cb 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -172,7 +172,7 @@ obj-$(CONFIG_MFD_SI476X_CORE) += si476x-core.o
obj-$(CONFIG_MFD_CS5535) += cs5535-mfd.o
obj-$(CONFIG_MFD_OMAP_USB_HOST) += omap-usb-host.o omap-usb-tll.o
-obj-$(CONFIG_MFD_PM8921_CORE) += pm8921-core.o ssbi.o
+obj-$(CONFIG_MFD_PM8XXX) += qcom-pm8xxx.o ssbi.o
obj-$(CONFIG_MFD_QCOM_RPM) += qcom_rpm.o
obj-$(CONFIG_MFD_SPMI_PMIC) += qcom-spmi-pmic.o
obj-$(CONFIG_TPS65911_COMPARATOR) += tps65911-comparator.o
diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/qcom-pm8xxx.c
similarity index 92%
rename from drivers/mfd/pm8921-core.c
rename to drivers/mfd/qcom-pm8xxx.c
index 0e3a2ea25942..7f9620ec61e8 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/qcom-pm8xxx.c
@@ -53,7 +53,7 @@
#define REG_HWREV 0x002 /* PMIC4 revision */
#define REG_HWREV_2 0x0E8 /* PMIC4 revision 2 */
-#define PM8921_NR_IRQS 256
+#define PM8XXX_NR_IRQS 256
struct pm_irq_chip {
struct regmap *regmap;
@@ -308,22 +308,22 @@ static const struct regmap_config ssbi_regmap_config = {
.reg_write = ssbi_reg_write
};
-static const struct of_device_id pm8921_id_table[] = {
+static const struct of_device_id pm8xxx_id_table[] = {
{ .compatible = "qcom,pm8018", },
{ .compatible = "qcom,pm8058", },
{ .compatible = "qcom,pm8921", },
{ }
};
-MODULE_DEVICE_TABLE(of, pm8921_id_table);
+MODULE_DEVICE_TABLE(of, pm8xxx_id_table);
-static int pm8921_probe(struct platform_device *pdev)
+static int pm8xxx_probe(struct platform_device *pdev)
{
struct regmap *regmap;
int irq, rc;
unsigned int val;
u32 rev;
struct pm_irq_chip *chip;
- unsigned int nirqs = PM8921_NR_IRQS;
+ unsigned int nirqs = PM8XXX_NR_IRQS;
irq = platform_get_irq(pdev, 0);
if (irq < 0)
@@ -384,46 +384,46 @@ static int pm8921_probe(struct platform_device *pdev)
return rc;
}
-static int pm8921_remove_child(struct device *dev, void *unused)
+static int pm8xxx_remove_child(struct device *dev, void *unused)
{
platform_device_unregister(to_platform_device(dev));
return 0;
}
-static int pm8921_remove(struct platform_device *pdev)
+static int pm8xxx_remove(struct platform_device *pdev)
{
int irq = platform_get_irq(pdev, 0);
struct pm_irq_chip *chip = platform_get_drvdata(pdev);
- device_for_each_child(&pdev->dev, NULL, pm8921_remove_child);
+ device_for_each_child(&pdev->dev, NULL, pm8xxx_remove_child);
irq_set_chained_handler_and_data(irq, NULL, NULL);
irq_domain_remove(chip->irqdomain);
return 0;
}
-static struct platform_driver pm8921_driver = {
- .probe = pm8921_probe,
- .remove = pm8921_remove,
+static struct platform_driver pm8xxx_driver = {
+ .probe = pm8xxx_probe,
+ .remove = pm8xxx_remove,
.driver = {
- .name = "pm8921-core",
- .of_match_table = pm8921_id_table,
+ .name = "pm8xxx-core",
+ .of_match_table = pm8xxx_id_table,
},
};
-static int __init pm8921_init(void)
+static int __init pm8xxx_init(void)
{
- return platform_driver_register(&pm8921_driver);
+ return platform_driver_register(&pm8xxx_driver);
}
-subsys_initcall(pm8921_init);
+subsys_initcall(pm8xxx_init);
-static void __exit pm8921_exit(void)
+static void __exit pm8xxx_exit(void)
{
- platform_driver_unregister(&pm8921_driver);
+ platform_driver_unregister(&pm8xxx_driver);
}
-module_exit(pm8921_exit);
+module_exit(pm8xxx_exit);
MODULE_LICENSE("GPL v2");
-MODULE_DESCRIPTION("PMIC 8921 core driver");
+MODULE_DESCRIPTION("PMIC 8xxx core driver");
MODULE_VERSION("1.0");
-MODULE_ALIAS("platform:pm8921-core");
+MODULE_ALIAS("platform:pm8xxx-core");
--
2.7.4
^ permalink raw reply related
* [PATCH v2] modversions: treat symbol CRCs as 32 bit quantities on 64 bit archs
From: Ard Biesheuvel @ 2016-10-26 13:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <871sz3xw1s.fsf@concordia.ellerman.id.au>
On 26 October 2016 at 11:07, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Hi Ard,
>
> I like the concept, but ...
>
> Ard Biesheuvel <ard.biesheuvel@linaro.org> writes:
>> The symbol CRCs are emitted as ELF symbols, which allows us to easily
>> populate the kcrctab sections by relying on the linker to associate
>> each kcrctab slot with the correct value.
>>
>> This has two downsides:
>> - given that the CRCs are treated as pointers, we waste 4 bytes for
>> each CRC on 64 bit architectures,
>> - on architectures that support runtime relocation, a relocation entry is
>> emitted for each CRC value, which may take up 24 bytes of __init space
>> (on ELF64 systems)
>>
>> This comes down to a x8 overhead in [uncompressed] kernel size. In addition,
>> each relocation has to be reverted before the CRC value can be used.
>>
>> Switching to explicit 32 bit values on 64 bit architectures fixes both
>> issues, since 32 bit values are not treated as relocatable quantities on
>> ELF64 systems, even if the value ultimately resolves to a linker supplied
>> value.
>
> Are we sure that part is true? ("not treated as relocatable")
>
Thanks for testing this.
> A quick test build on powerpc gives me:
>
> WARNING: 6829 bad relocations
> c000000000ca3748 R_PPC64_ADDR16 *ABS*+0x0000000013f53da6
> c000000000ca374a R_PPC64_ADDR16 *ABS*+0x00000000f7272059
> c000000000ca374c R_PPC64_ADDR16 *ABS*+0x0000000002013d36
> c000000000ca374e R_PPC64_ADDR16 *ABS*+0x00000000a59dffc8
> ...
>
> Which is coming from our relocs_check.sh script, which checks that the
> generated relocations are ones we know how to handle.
>
OK, first of all, it appears the ppc64 assembler interprets .word
differently than the arm64 one, so I will need to fold this in
"""
diff --git a/include/linux/export.h b/include/linux/export.h
index fa51ab2ad190..a000d421526d 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -54,7 +54,7 @@ extern struct module __this_module;
#define __CRC_SYMBOL(sym, sec) \
asm(" .section \"___kcrctab" sec "+" #sym "\", \"a\" \n" \
" .weak " VMLINUX_SYMBOL_STR(__crc_##sym) " \n" \
- " .word " VMLINUX_SYMBOL_STR(__crc_##sym) " \n" \
+ " .long " VMLINUX_SYMBOL_STR(__crc_##sym) " \n" \
" .previous \n");
#endif
#else
"""
With that change, the CRCs are actually emitted as
WARNING: 7525 bad relocations
c000000000ce7f50 R_PPC64_ADDR32 *ABS*+0x0000000013f53da6
c000000000ce7f54 R_PPC64_ADDR32 *ABS*+0x0000000004f7c64e
c000000000ce7f58 R_PPC64_ADDR32 *ABS*+0x0000000092be8a3e
which is still a bit disappointing, given that we still have 7525 RELA
entries to process.
I tried several thing, i.e., adding -Bsymbolic to the linker command
line, emitting the reference above as .hidden or emit the definition
from the linker script as HIDDEN(), but nothing seems to make any
difference. (On arm64, -Bsymbolic eliminates *all* runtime relocations
except R_<arch>_RELATIVE ones)
> And when I try to boot it I get:
>
> virtio: disagrees about version of symbol module_layout
> virtio: disagrees about version of symbol module_layout
> scsi_mod: disagrees about version of symbol module_layout
>
> And it can't find my root file system (unsurprisingly as it's on scsi).
>
Something like the below should fix it, I hope.
"""
diff --git a/arch/powerpc/kernel/reloc_64.S b/arch/powerpc/kernel/reloc_64.S
index d88736fbece6..99cdf2311ab5 100644
--- a/arch/powerpc/kernel/reloc_64.S
+++ b/arch/powerpc/kernel/reloc_64.S
@@ -14,6 +14,7 @@
RELA = 7
RELACOUNT = 0x6ffffff9
R_PPC64_RELATIVE = 22
+R_PPC64_ADDR32 = 1
/*
* r3 = desired final address of kernel
@@ -77,9 +78,22 @@ _GLOBAL(relocate)
add r0,r0,r3
stdx r0,r7,r6
addi r9,r9,24
- bdnz 5b
+ b 7f
+
+ /*
+ * CRCs of exported symbols are emitted as 32-bit relocations against
+ * the *ABS* section with the CRC value recorded in the addend.
+ */
+6: cmpdi r0,R_PPC64_ADDR32
+ bne 7f
+ ld r6,0(r9) /* reloc->r_offset */
+ ld r0,16(r9) /* reloc->r_addend */
+ stwx r0,r7,r6
+ addi r9,r9,24
+
+7: bdnz 5b
+ blr
-6: blr
.balign 8
p_dyn: .llong __dynamic_start - 0b
"""
Note that the loop no longer terminates at the first
non-R_PPC64_RELATIVE relocation, but that seems safer to me in any
case. It simply stores the value of r_addend at r_offset, which is the
correct thing to do for R_PPC64_ADDR32 relocations against the *ABS*
section, regardless of whether we are dealing with CRCs or something
else. Note that the comparison above will fail for R_PPC64_ADDR32
relocations against named symbols, since we compare the entire r_info
field and not just the type (as the comment a few lines higher up
suggests)
Also a fix for relocs_check.sh:
"""
diff --git a/arch/powerpc/relocs_check.sh b/arch/powerpc/relocs_check.sh
index ec2d5c835170..2f510fbc87da 100755
--- a/arch/powerpc/relocs_check.sh
+++ b/arch/powerpc/relocs_check.sh
@@ -43,7 +43,8 @@ R_PPC_ADDR16_HA
R_PPC_RELATIVE
R_PPC_NONE' |
grep -E -v '\<R_PPC64_ADDR64[[:space:]]+mach_' |
- grep -E -v '\<R_PPC64_ADDR64[[:space:]]+__crc_'
+ grep -E -v '\<R_PPC64_ADDR64[[:space:]]+__crc_' |
+ grep -E -v '\<R_PPC64_ADDR32[[:space:]]+\*ABS\*'
)
if [ -z "$bad_relocs" ]; then
"""
If these changes work for PPC, I think we should fold them in.
Hopefully, GNU ld for PPC will gain that ability to resolve absolute
relocations@build time (like other architectures), and then the
R_PPC64_ADDR32 handling will simply become dead code. In any case, it
is an improvement over the mangling of CRC values to undo the runtime
relocation, imo.
Regards,
Ard.
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox