Re: [net-next 1/3] net: dsa: optimize tx timestamp request handling
On Fri Apr 16 2021, Yangbo Lu wrote: > Optimization could be done on dsa_skb_tx_timestamp(), and dsa device > drivers should adapt to it. > > - Check SKBTX_HW_TSTAMP request flag at the very beginning, instead of in > port_txtstamp, so that most skbs not requiring tx timestamp just return. > > - No longer to identify PTP packets, and limit tx timestamping only for PTP > packets. If device driver likes, let device driver do. > > - It is a waste to clone skb directly in dsa_skb_tx_timestamp(). > For one-step timestamping, a clone is not needed. For any failure of > port_txtstamp (this may usually happen), the skb clone has to be freed. > So put skb cloning into port_txtstamp where it really needs. > > Signed-off-by: Yangbo Lu PTP still works. Tested-by: Kurt Kanzenbach # hellcreek signature.asc Description: PGP signature
Re: [RFC v4 net-next 3/4] dt-bindings: net: dsa: add MT7530 interrupt controller binding
On Mon Apr 12 2021, DENG Qingfang wrote: > Add device tree binding to support MT7530 interrupt controller. > > Signed-off-by: DENG Qingfang > Reviewed-by: Andrew Lunn > --- > RFC v3 -> RFC v4: > - Add #interrupt-cells property. > > Documentation/devicetree/bindings/net/dsa/mt7530.txt | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/dsa/mt7530.txt > b/Documentation/devicetree/bindings/net/dsa/mt7530.txt > index de04626a8e9d..892b1570c496 100644 > --- a/Documentation/devicetree/bindings/net/dsa/mt7530.txt > +++ b/Documentation/devicetree/bindings/net/dsa/mt7530.txt > @@ -81,6 +81,12 @@ Optional properties: > - gpio-controller: Boolean; if defined, MT7530's LED controller will run on > GPIO mode. > - #gpio-cells: Must be 2 if gpio-controller is defined. > +- interrupt-controller: Boolean; Enables the internal interrupt controller. > + > +If interrupt-controller is defined, the following property is required. > + > +- #interrupt-cells: Must be 1. > +- interrupts: Parent interrupt for the interrupt controller. > > See Documentation/devicetree/bindings/net/dsa/dsa.txt for a list of > additional That dsa.txt still exists, but it refers to the dsa.yaml binding. Any chance to convert this mt7530.txt to yaml as well? Then, device trees can be automatically validated for correct entries. There are a few examples of dsa yaml bindings already: ksz, b53, sf2, hellcreek, ... Thanks, Kurt signature.asc Description: PGP signature
Re: [ANNOUNCE] v5.12-rc3-rt3
On Sat Mar 20 2021, Mike Galbraith wrote: > On Fri, 2021-03-19 at 23:33 +0100, Sebastian Andrzej Siewior wrote: >> Dear RT folks! >> >> I'm pleased to announce the v5.12-rc3-rt3 patch set. > > My little rpi4b is fairly unhappy with 5.12-rt, whereas 5.11-rt works > fine on it. The below spew is endless, making boot endless. I turned > it into a WARN_ON_ONCE to see if the thing would finish boot, and > surprisingly, it seems perfectly fine with that bad idea. Having not > the foggiest clue what I'm doing down in arm arch-land, bug is in no > immediate danger :) Yeah, I've had the same issue with my Marvell ESPRESSObin. WARN_ON_ONCE() made it boot and it runs without any problems since then. Thanks! Thanks, Kurt signature.asc Description: PGP signature
Re: linux-next: Tree for Jan 28 [drivers/net/dsa/hirschmann/hellcreek_sw]
On Thu Jan 28 2021, Randy Dunlap wrote: > On 1/28/21 1:11 AM, Stephen Rothwell wrote: >> Hi all, >> >> Changes since 20210127: >> > > > on i386: > > ERROR: modpost: "taprio_offload_get" > [drivers/net/dsa/hirschmann/hellcreek_sw.ko] undefined! > ERROR: modpost: "taprio_offload_free" > [drivers/net/dsa/hirschmann/hellcreek_sw.ko] undefined! > > Full randconfig file is attached. Thanks for the config file. I've submitted a patch to fix it. Thanks, Kurt signature.asc Description: PGP signature
Re: [PATCH] net: dsa: fix led_classdev build errors
On Tue Jan 05 2021, Randy Dunlap wrote:
> Fix build errors when LEDS_CLASS=m and NET_DSA_HIRSCHMANN_HELLCREEK=y.
> This limits the latter to =m when LEDS_CLASS=m.
>
> microblaze-linux-ld: drivers/net/dsa/hirschmann/hellcreek_ptp.o: in function
> `hellcreek_ptp_setup':
> (.text+0xf80): undefined reference to `led_classdev_register_ext'
> microblaze-linux-ld: (.text+0xf94): undefined reference to
> `led_classdev_register_ext'
> microblaze-linux-ld: drivers/net/dsa/hirschmann/hellcreek_ptp.o: in function
> `hellcreek_ptp_free':
> (.text+0x1018): undefined reference to `led_classdev_unregister'
> microblaze-linux-ld: (.text+0x1024): undefined reference to
> `led_classdev_unregister'
>
> Signed-off-by: Randy Dunlap
> Reported-by: kernel test robot
> Link: lore.kernel.org/r/202101060655.iuvmjqs2-...@intel.com
> Cc: Kurt Kanzenbach
> Cc: net...@vger.kernel.org
> Cc: "David S. Miller"
> Cc: Jakub Kicinski
Fixes: 7d9ee2e8ff15 ("net: dsa: hellcreek: Add PTP status LEDs")
Reviewed-by: Kurt Kanzenbach
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: [PATCH net-next v2 1/3] net: phy: dp83640: use new PTP_MSGTYPE_SYNC define
On Tue Nov 24 2020, Christian Eggers wrote: > Replace use of magic number with recently introduced define. > > Signed-off-by: Christian Eggers > Cc: Kurt Kanzenbach Reviewed-by: Kurt Kanzenbach signature.asc Description: PGP signature
Re: [PATCH] net: dsa: sja1105: Fix return value check in sja1105_ptp_clock_register()
On Thu Nov 12 2020, YueHaibing wrote:
> drivers/net/dsa/sja1105/sja1105_ptp.c:869 sja1105_ptp_clock_register() warn:
> passing zero to 'PTR_ERR'
>
> ptp_clock_register() returns ERR_PTR() and never returns
> NULL. The NULL test should be removed.
Which is not true. From the documentation:
* Returns a valid pointer on success or PTR_ERR on failure. If PHC
* support is missing at the configuration level, this function
* returns NULL, and drivers are expected to gracefully handle that
* case separately.
Please, always Cc Richard for PTP patches.
Actually you can have a look at this discussion here:
https://lkml.kernel.org/netdev/1605086686-5140-1-git-send-email-wangq...@vivo.com/
>
> Fixes: bb77f36ac21d ("net: dsa: sja1105: Add support for the PTP clock")
> Signed-off-by: YueHaibing
> ---
> drivers/net/dsa/sja1105/sja1105_ptp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c
> b/drivers/net/dsa/sja1105/sja1105_ptp.c
> index 1b90570b257b..1e41d491c854 100644
> --- a/drivers/net/dsa/sja1105/sja1105_ptp.c
> +++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
> @@ -865,7 +865,7 @@ int sja1105_ptp_clock_register(struct dsa_switch *ds)
> spin_lock_init(_data->meta_lock);
>
> ptp_data->clock = ptp_clock_register(_data->caps, ds->dev);
> - if (IS_ERR_OR_NULL(ptp_data->clock))
> + if (IS_ERR(ptp_data->clock))
When you do this, you'll have to make sure that the driver handles the
NULL case "gracefully".
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: [PATCH net-next] MAINTAINERS: Add entry for Hirschmann Hellcreek Switch Driver
On Tue Nov 10 2020, Vladimir Oltean wrote: > On Tue, Nov 10, 2020 at 08:18:29AM +0100, Kurt Kanzenbach wrote: >> Add myself to cover the Hirschmann Hellcreek TSN Ethernet Switch Driver. >> >> Suggested-by: Andrew Lunn >> Signed-off-by: Kurt Kanzenbach >> --- >> MAINTAINERS | 9 + >> 1 file changed, 9 insertions(+) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 2a0fde12b650..7fe936fc7e76 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -7897,6 +7897,15 @@ F:include/linux/hippidevice.h >> F: include/uapi/linux/if_hippi.h >> F: net/802/hippi.c >> >> +HIRSCHMANN HELLCREEK ETHERNET SWITCH DRIVER >> +M: Kurt Kanzenbach >> +L: net...@vger.kernel.org >> +S: Maintained > > Just want to make sure you're aware of the difference: > > Supported: Someone is actually paid to look after this. > Maintained: Someone actually looks after it. Yea, that's fine. Thanks, Kurt signature.asc Description: PGP signature
Re: [PATCH 10/10] dt-bindings: net: dsa: b53: Add YAML bindings
On Mon Nov 09 2020, Florian Fainelli wrote: > From: Kurt Kanzenbach > > Convert the b53 DSA device tree bindings to YAML in order to allow > for automatic checking and such. > > Suggested-by: Florian Fainelli > Signed-off-by: Kurt Kanzenbach > --- > .../devicetree/bindings/net/dsa/b53.txt | 149 --- > .../devicetree/bindings/net/dsa/b53.yaml | 249 ++ Maybe it should be renamed to brcm,b53.yaml to be consistent with the ksz and hellcreek bindings. Thanks, Kurt signature.asc Description: PGP signature
[PATCH net-next] MAINTAINERS: Add entry for Hirschmann Hellcreek Switch Driver
Add myself to cover the Hirschmann Hellcreek TSN Ethernet Switch Driver. Suggested-by: Andrew Lunn Signed-off-by: Kurt Kanzenbach --- MAINTAINERS | 9 + 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 2a0fde12b650..7fe936fc7e76 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7897,6 +7897,15 @@ F: include/linux/hippidevice.h F: include/uapi/linux/if_hippi.h F: net/802/hippi.c +HIRSCHMANN HELLCREEK ETHERNET SWITCH DRIVER +M: Kurt Kanzenbach +L: net...@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml +F: drivers/net/dsa/hirschmann/* +F: include/linux/platform_data/hirschmann-hellcreek.h +F: net/dsa/tag_hellcreek.c + HISILICON DMA DRIVER M: Zhou Wang L: dmaeng...@vger.kernel.org -- 2.20.1
re: net: dsa: hellcreek: Add support for hardware timestamping
Hi Colin, On Mon Nov 09 2020, Colin Ian King wrote: > Hi > > Static analysis on linux-next with Coverity has detected a potential > null pointer dereference issue on the following commit: > > commit f0d4ba9eff75a79fccb7793f4d9f12303d458603 > Author: Kamil Alkhouri > Date: Tue Nov 3 08:10:58 2020 +0100 > > net: dsa: hellcreek: Add support for hardware timestamping > > The analysis is as follows: > > 323/* Get nanoseconds from ptp packet */ > 324type = SKB_PTP_TYPE(skb); > >4. returned_null: ptp_parse_header returns NULL (checked 10 out of 12 > times). >5. var_assigned: Assigning: hdr = NULL return value from > ptp_parse_header. > > 325hdr = ptp_parse_header(skb, type); > >Dereference null return value (NULL_RETURNS) >6. dereference: Dereferencing a pointer that might be NULL hdr when > calling hellcreek_get_reserved_field. > > 326ns = hellcreek_get_reserved_field(hdr); > 327hellcreek_clear_reserved_field(hdr); > > This issue can only occur if the type & PTP_CLASS_PMASK is not one of > PTP_CLASS_IPV4, PTP_CLASS_IPV6 or PTP_CLASS_L2. I'm not sure if this is > a possibility or not, but I'm assuming that it would be useful to > perform the null check just in case, but I'm not sure how this affects > the hw timestamping code in this function. I don't see how the null pointer dereference could happen. That's the Rx path you showed above. The counter part code is: hellcreek_port_rxtstamp: /* Make sure the message is a PTP message that needs to be timestamped * and the interaction with the HW timestamping is enabled. If not, stop * here */ hdr = hellcreek_should_tstamp(hellcreek, port, skb, type); if (!hdr) return false; SKB_PTP_TYPE(skb) = type; Here the type is stored and hellcreek_should_tstamp() also calls ptp_parse_header() internally. Only when ptp_parse_header() didn't return NULL the first time the timestamping continues. It should be safe. Also the error handling would be interesting at that point. What should happen if the header is null then? Returning an invalid timestamp? Ignore it? Hm. I think we have to make sure that it is a valid ptp packet before reaching this code and that's what we've implemented. So, I guess it's OK. Thanks, Kurt signature.asc Description: PGP signature
Re: [PATCH][next] net: dsa: fix unintended sign extension on a u16 left shift
On Mon Nov 09 2020, Colin King wrote:
> From: Colin Ian King
>
> The left shift of u16 variable high is promoted to the type int and
> then sign extended to a 64 bit u64 value. If the top bit of high is
> set then the upper 32 bits of the result end up being set by the
> sign extension. Fix this by explicitly casting the value in high to
> a u64 before left shifting by 16 places.
>
> Also, remove the initialisation of variable value to 0 at the start
> of each loop iteration as the value is never read and hence the
> assignment it is redundant.
>
> Addresses-Coverity: ("Unintended sign extension")
> Fixes: e4b27ebc780f ("net: dsa: Add DSA driver for Hirschmann Hellcreek
> switches")
> Signed-off-by: Colin Ian King
Reviewed-by: Kurt Kanzenbach
signature.asc
Description: PGP signature
Re: [PATCH][next] net: dsa: fix unintended sign extension on a u16 left shift
On Mon Nov 09 2020, Colin King wrote:
> From: Colin Ian King
>
> The left shift of u16 variable high is promoted to the type int and
> then sign extended to a 64 bit u64 value. If the top bit of high is
> set then the upper 32 bits of the result end up being set by the
> sign extension. Fix this by explicitly casting the value in high to
> a u64 before left shifting by 16 places.
>
> Also, remove the initialisation of variable value to 0 at the start
> of each loop iteration as the value is never read and hence the
> assignment it is redundant.
>
> Addresses-Coverity: ("Unintended sign extension")
> Fixes: e4b27ebc780f ("net: dsa: Add DSA driver for Hirschmann Hellcreek
> switches")
> Signed-off-by: Colin Ian King
> ---
> drivers/net/dsa/hirschmann/hellcreek.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c
> b/drivers/net/dsa/hirschmann/hellcreek.c
> index dfa66f7260d6..d42f40c76ba5 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -308,7 +308,7 @@ static void hellcreek_get_ethtool_stats(struct dsa_switch
> *ds, int port,
> const struct hellcreek_counter *counter = _counter[i];
> u8 offset = counter->offset + port * 64;
> u16 high, low;
> - u64 value = 0;
> + u64 value;
>
> mutex_lock(>reg_lock);
>
> @@ -320,7 +320,7 @@ static void hellcreek_get_ethtool_stats(struct dsa_switch
> *ds, int port,
>*/
> high = hellcreek_read(hellcreek, HR_CRDH);
> low = hellcreek_read(hellcreek, HR_CRDL);
> - value = (high << 16) | low;
> + value = ((u64)high << 16) | low;
Looks good to me. Thank you.
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: [PATCH] net/ethernet: update ret when ptp_clock is ERROR
On Fri Nov 06 2020, Arnd Bergmann wrote:
> On Fri, Nov 6, 2020 at 12:35 PM Grygorii Strashko
> wrote:
>> On 06/11/2020 09:56, Wang Qing wrote:
>
>> > +++ b/drivers/net/ethernet/ti/am65-cpts.c
>> > @@ -1001,8 +1001,7 @@ struct am65_cpts *am65_cpts_create(struct device
>> > *dev, void __iomem *regs,
>>
>> there is
>> cpts->ptp_clock = ptp_clock_register(>ptp_info, cpts->dev);
>>
>>
>> > if (IS_ERR_OR_NULL(cpts->ptp_clock)) {
>>
>> And ptp_clock_register() can return NULL only if PTP support is disabled.
>> In which case, we should not even get here.
>>
>> So, I'd propose to s/IS_ERR_OR_NULL/IS_ERR above,
>> and just assign ret = PTR_ERR(cpts->ptp_clock) here.
>
> Right, using IS_ERR_OR_NULL() is almost ever a mistake, either
> from misunderstanding the interface, or from a badly designed
> interface that needs to be changed.
The NULL case should be handled differently and it is documented:
/**
* ptp_clock_register() - register a PTP hardware clock driver
[...]
* Returns a valid pointer on success or PTR_ERR on failure. If PHC
* support is missing at the configuration level, this function
* returns NULL, and drivers are expected to gracefully handle that
* case separately.
*/
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: [RFC PATCH net-next 1/9] dt-bindings: net: dsa: convert ksz bindings document to yaml
On Wed Oct 21 2020, Florian Fainelli wrote:
> On 10/21/2020 5:16 PM, Vladimir Oltean wrote:
>> On Wed, Oct 21, 2020 at 08:52:01AM +0200, Kurt Kanzenbach wrote:
>>> On Mon Oct 19 2020, Christian Eggers wrote:
>>> The node names should be switch. See dsa.yaml.
>>>
>>>> +compatible = "microchip,ksz9477";
>>>> +reg = <0>;
>>>> +reset-gpios = < 0 GPIO_ACTIVE_LOW>;
>>>> +
>>>> +spi-max-frequency = <4400>;
>>>> +spi-cpha;
>>>> +spi-cpol;
>>>> +
>>>> +ports {
>>>
>>> ethernet-ports are preferred.
>>
>> This is backwards to me, instead of an 'ethernet-switch' with 'ports',
>> we have a 'switch' with 'ethernet-ports'. Whatever.
>
> The rationale AFAIR was that dual Ethernet port controllers like TI's
> CPSW needed to describe each port as a pseudo Ethernet MAC and using
> 'ethernet-ports' as a contained allowed to disambiguate with the 'ports'
> container used in display subsystem descriptions.
Yes, that was the outcome of previous discussions.
> We should probably enforce or recommend 'ethernet-switch' to be used
> as the node name for Ethernet switch devices though.
After using grep, it seems like 'ethernet-switch' as well as simply
'switch' are being used today. Yes, maybe both should be allowed.
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: [RFC PATCH net-next 1/9] dt-bindings: net: dsa: convert ksz bindings document to yaml
On Mon Oct 19 2020, Christian Eggers wrote:
> Convert the bindings document for Microchip KSZ Series Ethernet switches
> from txt to yaml.
A few comments/questions below.
> diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> new file mode 100644
> index ..f93c3bdd0b83
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
Currently the bindings don't have the company names in front of it.
> @@ -0,0 +1,147 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/microchip,ksz.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip KSZ Series Ethernet switches
> +
> +maintainers:
> + - Marek Vasut
> + - Woojung Huh
> +
> +properties:
> + # See Documentation/devicetree/bindings/net/dsa/dsa.yaml for a list of
> additional
> + # required and optional properties.
Don't you need to reference the dsa.yaml binding somehow?
> + compatible:
> +enum:
> + - "microchip,ksz8765"
> + - "microchip,ksz8794"
> + - "microchip,ksz8795"
> + - "microchip,ksz9477"
> + - "microchip,ksz9897"
> + - "microchip,ksz9896"
> + - "microchip,ksz9567"
> + - "microchip,ksz8565"
> + - "microchip,ksz9893"
> + - "microchip,ksz9563"
> + - "microchip,ksz8563"
> +
> + reset-gpios:
> +description:
> + Should be a gpio specifier for a reset line.
> +maxItems: 1
> +
> + microchip,synclko-125:
> +$ref: /schemas/types.yaml#/definitions/flag
> +description:
> + Set if the output SYNCLKO frequency should be set to 125MHz instead of
> 25MHz.
> +
> +required:
> + - compatible
> + - reg
> +
> +examples:
> + - |
> +#include
> +
> +// Ethernet switch connected via SPI to the host, CPU port wired to eth0:
> +eth0 {
> +fixed-link {
> +speed = <1000>;
> +full-duplex;
> +};
> +};
> +
> +spi0 {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +pinctrl-0 = <_spi_ksz>;
> +cs-gpios = < 25 0>;
> +id = <1>;
> +
> +ksz9477: ksz9477@0 {
The node names should be switch. See dsa.yaml.
> +compatible = "microchip,ksz9477";
> +reg = <0>;
> +reset-gpios = < 0 GPIO_ACTIVE_LOW>;
> +
> +spi-max-frequency = <4400>;
> +spi-cpha;
> +spi-cpol;
> +
> +ports {
ethernet-ports are preferred.
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: [PATCH net] net: dsa: ksz: fix padding size of skb
On Thu Oct 15 2020, Christian Eggers wrote: > On Wednesday, 14 October 2020, 19:31:03 CEST, Vladimir Oltean wrote: >> What problem are you actually trying to solve? > After (hopefully) understanding the important bits, I would like to solve the > problem that after calling __skb_put_padto() there may be no tailroom for the > tail tag. > > The conditions where this can happen are quite special. You need a skb->len < > ETH_ZLEN and the skb must be marked as cloned. One condition where this > happens in practice is when the skb has been selected for TX time stamping in > dsa_skb_tx_timestamp() [cloned] and L2 is used as transport for PTP [size < > ETH_ZLEN]. But maybe cloned sk_buffs can also happen for other reasons. Hmm. I've never observed any problems using DSA with L2 PTP time stamping with this tail tag code. What's the impact exactly? Memory corruption? Thanks, Kurt signature.asc Description: PGP signature
Re: [PATCH] ptp: add stub function for ptp_get_msgtype()
On Sun Sep 27 2020, Yangbo Lu wrote:
> Added the missing stub function for ptp_get_msgtype().
>
> Reported-by: Randy Dunlap
> Fixes: 036c508ba95e ("ptp: Add generic ptp message type function")
> Signed-off-by: Yangbo Lu
Oh, my bad. Thanks for fixing it.
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: [RFC PATCH] arm64: defconfig: Disable fine-grained task level IRQ time accounting
On Thu Jul 30 2020, Vladimir Oltean wrote: > On Thu, Jul 30, 2020 at 09:23:44AM +0200, Kurt Kanzenbach wrote: >> On Wed Jul 29 2020, Vladimir Oltean wrote: >> > For more context, here is my original report of the issue: >> > https://lkml.org/lkml/2020/6/4/1062 >> > >> > Just like you, I could not reproduce the RCU stalls and system hang on a >> > 5.6-rt kernel, just on mainline and derivatives, using the plain >> > defconfig. >> > >> > The issue is not specific to Layerscape or i.MX8, but rather I was able >> > to see the same behavior on Marvell Armada 37xx as well as Qualcomm >> > MSM8976. >> > >> > So, while of course I agree that disabling IRQ time accounting for arm64 >> > isn't a real solution, it isn't by far an exaggerated proposal either. >> > Nonetheless, the patch is just a RFC and should be treated as such. We >> > are at a loss when it comes to debugging this any further and we would >> > appreciate some pointers. >> >> Yeah, sure. I'll try to reproduce this issue first. So it triggers with: >> >> * arm64 >> * mainline, not -rt kernel >> * opened serial console >> * irq accounting enabled >> >> Anything else? >> >> Thanks, >> Kurt > > Thanks for giving a helping hand, Kurt. The defconfig should be enough. > In the interest of full disclosure, the only arm64 device on which we > didn't reproduce this was the 16-core LX2160A. But we did reproduce on > that with maxcpus=1 though. And also on msm8976 with all 8 cores booted. > Just mentioning this in case you're testing on a 16-core system, you > might want to reduce the number a bit. OK. I've reproduced it on a Marvell Armada SoC with v5.6 mainline. See splats below. Running with irq time accounting enabled, kills the machine immediately. However, I'm not getting the possible deadlock warnings in 8250 as you did. So that might be unrelated. Unfortunately I have no idea what to debug here. Thanks, Kurt Splats: |root@marvell ~ # [ 150.996487] rcu: INFO: rcu_sched detected stalls on CPUs/tasks: |[ 150.999764] (detected by 0, t=2602 jiffies, g=9697, q=24) |[ 151.005403] rcu: All QSes seen, last rcu_sched kthread activity 2602 (4294952389-4294949787), jiffies_till_next_fqs=1, root ->qsmask 0x0 |[ 151.018034] stress-ng-hrtim R running task0 2251 2248 0x0001 |[ 151.025290] Call trace: |[ 151.027808] dump_backtrace+0x0/0x198 |[ 151.031563] show_stack+0x1c/0x28 |[ 151.034971] sched_show_task+0x224/0x248 |[ 151.039002] rcu_sched_clock_irq+0x7c4/0x840 |[ 151.043392] update_process_times+0x34/0x70 |[ 151.047694] tick_sched_handle.isra.0+0x34/0x70 |[ 151.052350] tick_sched_timer+0x50/0xa0 |[ 151.056295] __hrtimer_run_queues+0x18c/0x5b0 |[ 151.060773] hrtimer_interrupt+0xec/0x248 |[ 151.064896] arch_timer_handler_phys+0x38/0x48 |[ 151.069465] handle_percpu_devid_irq+0xd0/0x3c8 |[ 151.074124] generic_handle_irq+0x2c/0x40 |[ 151.078246] __handle_domain_irq+0x68/0xc0 |[ 151.082456] gic_handle_irq+0x64/0x150 |[ 151.086308] el1_irq+0xbc/0x140 |[ 151.089536] lock_acquire+0xdc/0x250 |[ 151.093210] __might_fault+0x68/0x88 |[ 151.096880] setup_rt_frame+0x2a4/0xee8 |[ 151.100823] do_notify_resume+0x3b0/0x488 |[ 151.104945] work_pending+0x8/0x14 |[ 151.108443] rcu: rcu_sched kthread starved for 2613 jiffies! g9697 f0x2 RCU_GP_WAIT_FQS(5) ->state=0x0 ->cpu=1 |[ 151.118741] rcu: RCU grace-period kthread stack dump: |[ 151.123939] rcu_sched R running task010 2 0x0028 |[ 151.131196] Call trace: |[ 151.133707] __switch_to+0x104/0x170 |[ 151.137384] __schedule+0x36c/0x860 |[ 151.140966] schedule+0x7c/0x108 |[ 151.144280] schedule_timeout+0x204/0x4c0 |[ 151.148403] rcu_gp_kthread+0x5b0/0x1798 |[ 151.152435] kthread+0x110/0x140 |[ 151.155749] ret_from_fork+0x10/0x18 |[ 181.665482] BUG: workqueue lockup - pool cpus=0 node=0 flags=0x0 nice=0 stuck for 55s! |[ 181.671022] BUG: workqueue lockup - pool cpus=1 node=0 flags=0x0 nice=0 stuck for 56s! |[ 181.679198] BUG: workqueue lockup - pool cpus=0-1 flags=0x4 nice=0 stuck for 56s! |[ 181.686902] Showing busy workqueues and worker pools: |[ 181.692059] workqueue events: flags=0x0 |[ 181.696226] pwq 2: cpus=1 node=0 flags=0x0 nice=0 active=2/256 refcnt=3 |[ 181.703039] pending: dbs_work_handler, mv88e6xxx_ptp_overflow_check |[ 181.709826] pwq 0: cpus=0 node=0 flags=0x0 nice=0 active=2/256 refcnt=3 |[ 181.716783] pending: vmstat_shepherd, switchdev_deferred_process_work |[ 181.723804] workqueue events_unbound: flags=0x2 |[ 181.728526] pwq 4: cpus=0-1 flags=0x4 nice=0 active=1/512 refcnt=3 |[ 181.735029] pending: flush_to_ldisc |[ 181.738947] workqueue events_freezable: flags=0x4 |[ 181.743755] pwq 2: cpus=1 nod
Re: [RFC PATCH] arm64: defconfig: Disable fine-grained task level IRQ time accounting
Hi Vladimir, On Wed Jul 29 2020, Vladimir Oltean wrote: > For more context, here is my original report of the issue: > https://lkml.org/lkml/2020/6/4/1062 > > Just like you, I could not reproduce the RCU stalls and system hang on a > 5.6-rt kernel, just on mainline and derivatives, using the plain > defconfig. > > The issue is not specific to Layerscape or i.MX8, but rather I was able > to see the same behavior on Marvell Armada 37xx as well as Qualcomm > MSM8976. > > So, while of course I agree that disabling IRQ time accounting for arm64 > isn't a real solution, it isn't by far an exaggerated proposal either. > Nonetheless, the patch is just a RFC and should be treated as such. We > are at a loss when it comes to debugging this any further and we would > appreciate some pointers. Yeah, sure. I'll try to reproduce this issue first. So it triggers with: * arm64 * mainline, not -rt kernel * opened serial console * irq accounting enabled Anything else? Thanks, Kurt signature.asc Description: PGP signature
Re: [RFC PATCH] arm64: defconfig: Disable fine-grained task level IRQ time accounting
Hi Alison, On Wed Jul 29 2020, Alison Wang wrote: > In the current arm64 defconfig, CONFIG_IRQ_TIME_ACCOUNTING is enabled as > default. According to my tests on NXP's LayerScape and i.MX platforms, > the system hangs when running the command "stress-ng --hrtimers 1" with > CONFIG_IRQ_TIME_ACCOUNTING enabled. Disabling this option, the issue > disappears. CONFIG_IRQ_TIME_ACCOUNTING causes serious performance impact > when running hrtimer stress test at the same time. I think instead of disabling the option for all arm64 devices, it might be better to analyze the root-cause why the hrtimer test hangs when this option is enabled. +Cc hrtimer maintainers: Thomas and Anna-Maria Thanks, Kurt signature.asc Description: PGP signature
Re: UART/TTY console deadlock
On Mon Jul 06 2020, Sergey Senozhatsky wrote:
> On (20/07/06 13:31), Kurt Kanzenbach wrote:
>> >> @@ -2275,6 +2275,7 @@ int serial8250_do_startup(struct uart_port *port)
>> >>
>> >> if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) {
>> >> unsigned char iir1;
>> >> +
>> >> /*
>> >> * Test for UARTs that do not reassert THRE when the
>> >> * transmitter is idle and the interrupt has already
>> >> @@ -2284,8 +2285,6 @@ int serial8250_do_startup(struct uart_port *port)
>> >> * allow register changes to become visible.
>> >> */
>> >> spin_lock_irqsave(>lock, flags);
>> >> - if (up->port.irqflags & IRQF_SHARED)
>> >> - disable_irq_nosync(port->irq);
>> >>
>> >> wait_for_xmitr(up, UART_LSR_THRE);
>> >> serial_port_out_sync(port, UART_IER, UART_IER_THRI);
>> >> @@ -2297,8 +2296,6 @@ int serial8250_do_startup(struct uart_port *port)
>> >> iir = serial_port_in(port, UART_IIR);
>> >> serial_port_out(port, UART_IER, 0);
>> >>
>> >> - if (port->irqflags & IRQF_SHARED)
>> >> - enable_irq(port->irq);
>> >> spin_unlock_irqrestore(>lock, flags);
>> >>
>> >> /*
>> >
>> > ...which effectively is a revert of
>> >
>> > 768aec0b5bcc ("serial: 8250: fix shared interrupts issues with SMP and
>> > RT kernels")
>>
>> Please, don't revert that commit. I've faced the same issue as described
>> in the commit log. There is hardware available with shared UART
>> interrupt lines.
>
> Will this patch break that hardware?
> https://lore.kernel.org/lkml/20200702051213.GB3450@jagdpanzerIV.localdomain/
I'm not sure how this patch will help with the situation. Because at the
point of that THRE test the irq handler isn't registered. It's
registered a few lines below (up->ops->setup_irq()) meaning the irq line
has to be disabled if shared. Otherwise the kernel might detect a
spurious irq and disables it. That's at least my understanding of the
problem (see commit message from 54e53b2e8081 ("tty: serial: 8250: pass
IRQ shared flag to UART ports")).
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: UART/TTY console deadlock
Hi,
On Sat Jul 04 2020, Andy Shevchenko wrote:
> On Fri, Jul 3, 2020 at 1:32 PM Sergey Senozhatsky
> wrote:
>>
>> On (20/07/02 09:05), Tony Lindgren wrote:
>> > * Sergey Senozhatsky [200702 05:13]:
>> > > On (20/06/30 11:02), Tony Lindgren wrote:
>> > > > This conditional disable for irq_shared does not look nice to me
>> > > > from the other device point of view :)
>> > > >
>> > > > Would it be possible to just set up te dummy interrupt handler
>> > > > for the startup, then change it back afterwards? See for example
>> > > > omap8250_no_handle_irq().
>> > >
>> > > I think we can do it. serial8250_do_startup() and irq handler take
>> > > port->lock, so they should be synchronized.
>> > >
>> > > Something like this then?
>> >
>> > Yeah thanks this should work better for shared irq.
>>
>> This is, basically, an equivalent of
>>
>> ---
>> drivers/tty/serial/8250/8250_port.c | 5 +
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_port.c
>> b/drivers/tty/serial/8250/8250_port.c
>> index d64ca77d9cfa..dba7747d2ddd 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -2275,6 +2275,7 @@ int serial8250_do_startup(struct uart_port *port)
>>
>> if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) {
>> unsigned char iir1;
>> +
>> /*
>> * Test for UARTs that do not reassert THRE when the
>> * transmitter is idle and the interrupt has already
>> @@ -2284,8 +2285,6 @@ int serial8250_do_startup(struct uart_port *port)
>> * allow register changes to become visible.
>> */
>> spin_lock_irqsave(>lock, flags);
>> - if (up->port.irqflags & IRQF_SHARED)
>> - disable_irq_nosync(port->irq);
>>
>> wait_for_xmitr(up, UART_LSR_THRE);
>> serial_port_out_sync(port, UART_IER, UART_IER_THRI);
>> @@ -2297,8 +2296,6 @@ int serial8250_do_startup(struct uart_port *port)
>> iir = serial_port_in(port, UART_IIR);
>> serial_port_out(port, UART_IER, 0);
>>
>> - if (port->irqflags & IRQF_SHARED)
>> - enable_irq(port->irq);
>> spin_unlock_irqrestore(>lock, flags);
>>
>> /*
>
> ...which effectively is a revert of
>
> 768aec0b5bcc ("serial: 8250: fix shared interrupts issues with SMP and
> RT kernels")
Please, don't revert that commit. I've faced the same issue as described
in the commit log. There is hardware available with shared UART
interrupt lines.
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: [EXT] Re: stress-ng --hrtimers hangs system
Hi Jiafei, On Friday, June 12, 2020 2:49:17 AM CEST Jiafei Pan wrote: > Hi, Kurt, > > May I know whether you used "root" user to run stress-ng? using "root" > user will change the scheduler to be "SCHED_RR", so would you please > share test result with root and non-root users? Thanks. Performed the test now as root and non-root user. Same result: No problem. Thanks, Kurt
[tip: timers/urgent] timekeeping: Fix kerneldoc system_device_crosststamp & al
The following commit has been merged into the timers/urgent branch of tip:
Commit-ID: f097eb38f71391ff2cf078788bad5a00eb3bd96a
Gitweb:
https://git.kernel.org/tip/f097eb38f71391ff2cf078788bad5a00eb3bd96a
Author:Kurt Kanzenbach
AuthorDate:Tue, 09 Jun 2020 10:17:26 +02:00
Committer: Thomas Gleixner
CommitterDate: Thu, 18 Jun 2020 11:37:03 +02:00
timekeeping: Fix kerneldoc system_device_crosststamp & al
Make kernel doc comments actually work and fix the syncronized typo.
[ tglx: Added the missing /** and fixed up formatting ]
Signed-off-by: Kurt Kanzenbach
Signed-off-by: Thomas Gleixner
Link: https://lkml.kernel.org/r/20200609081726.5657-1-k...@linutronix.de
---
include/linux/timekeeping.h | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index b27e2ff..d5471d6 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -222,9 +222,9 @@ extern bool timekeeping_rtc_skipresume(void);
extern void timekeeping_inject_sleeptime64(const struct timespec64 *delta);
-/*
+/**
* struct system_time_snapshot - simultaneous raw/real time capture with
- * counter value
+ * counter value
* @cycles:Clocksource counter value to produce the system times
* @real: Realtime system time
* @raw: Monotonic raw system time
@@ -239,9 +239,9 @@ struct system_time_snapshot {
u8 cs_was_changed_seq;
};
-/*
+/**
* struct system_device_crosststamp - system/device cross-timestamp
- * (syncronized capture)
+ * (synchronized capture)
* @device:Device time
* @sys_realtime: Realtime simultaneous with device time
* @sys_monoraw: Monotonic raw simultaneous with device time
@@ -252,12 +252,12 @@ struct system_device_crosststamp {
ktime_t sys_monoraw;
};
-/*
+/**
* struct system_counterval_t - system counter value with the pointer to the
- * corresponding clocksource
+ * corresponding clocksource
* @cycles:System counter value
* @cs:Clocksource corresponding to system counter value. Used
by
- * timekeeping code to verify comparibility of two cycle values
+ * timekeeping code to verify comparibility of two cycle values
*/
struct system_counterval_t {
u64 cycles;
Re: stress-ng --hrtimers hangs system
Hi Vladimir, On Tue Jun 09 2020, Vladimir Oltean wrote: > Just out of curiosity, what and how many CPU cores does your ARM64 box > have, and what frequency are you running them at? > Mine is a dual-core A72 machine running at 1500 MHz. That particular machine has a dual core Cortex A53 running at 1GHz. Thanks, Kurt signature.asc Description: PGP signature
[PATCH] timekeeping: Fix typo
Fix typo in comment for cross timestamping structure:
syncronized -> synchronized.
Signed-off-by: Kurt Kanzenbach
---
include/linux/timekeeping.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index b27e2ffa96c1..17a1a2b6be40 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -241,7 +241,7 @@ struct system_time_snapshot {
/*
* struct system_device_crosststamp - system/device cross-timestamp
- * (syncronized capture)
+ * (synchronized capture)
* @device:Device time
* @sys_realtime: Realtime simultaneous with device time
* @sys_monoraw: Monotonic raw simultaneous with device time
--
2.20.1
Re: stress-ng --hrtimers hangs system
Hi Vladimir, On Fri Jun 05 2020, Vladimir Oltean wrote: > Hi, > > I was testing stress-ng on an ARM64 box and I found that it can be killed > instantaneously with a --hrtimers 1 test: > https://github.com/ColinIanKing/stress-ng/blob/master/stress-hrtimers.c > The console shell locks up immediately after starting the process, and I get > this rcu_preempt splat after 21 seconds, > letting me know that the grace-periods kernel thread could not run: interesting. Just tested this on an ARM64 box with v5.6-rt and the stress-ng hrtimer test works fine. No lockups, cyclictest results are looking good. So maybe this is v5.7 related. Thanks, Kurt signature.asc Description: PGP signature
Re: [PATCH v6 2/2] dt/bindings: Add bindings for Layerscape external irqs
On Fri, Sep 27, 2019 at 09:16:50PM +, Rasmus Villemoes wrote:
> On 27/09/2019 18.11, Rob Herring wrote:
> > On Mon, Sep 23, 2019 at 12:15:13PM +0200, Kurt Kanzenbach wrote:
> >> +Required properties:
> >> +- compatible: should be "fsl,-extirq", e.g.
> >> "fsl,ls1021a-extirq".
> >> +- interrupt-controller: Identifies the node as an interrupt controller
> >> +- #interrupt-cells: Must be 2. The first element is the index of the
> >> + external interrupt line. The second element is the trigger type.
> >> +- interrupt-parent: phandle of GIC.
> >> +- reg: Specifies the Interrupt Polarity Control Register (INTPCR) in the
> >> SCFG.
> >> +- fsl,extirq-map: Specifies the mapping to interrupt numbers in the parent
> >> + interrupt controller. Interrupts are mapped one-to-one to parent
> >> + interrupts.
> >
> > This should be an 'interrupt-map' instead.
>
> Rob, thanks for your review comments. I know you said the same thing at
> v5, and it might seem like they are ignored.
Well, I didn't ignore them. It just wasn't clear to me from the previous
discussions which way you want to go.
> However, I asked a couple of followup questions
> (https://lore.kernel.org/lkml/0bb4533d-c749-d8ff-e1f2-4b08eb724...@prevas.dk/).
> I'd really appreciate it if you could (a) point to some documentation
> on how to write that interrupt-map and (b) explain how this is
> different from the Qualcomm PDC case I tried to copy and which had
> your Reviewed-By.
I guess, we can have a look at other interrupt controllers and how they
handle the interrupt-map property. For example:
https://www.kernel.org/doc/Documentation/devicetree/bindings/interrupt-controller/renesas,rza1-irqc.txt
I need to send a v7 anyway, because I forgot to include the SOC_LS1021A
in the build process.
>
> >> +
> >> +Optional properties:
> >> +- fsl,bit-reverse: This boolean property should be set on the LS1021A
> >> + if the SCFGREVCR register has been set to all-ones (which is usually
> >> + the case), meaning that all reads and writes of SCFG registers are
> >> + implicitly bit-reversed. Other compatible platforms do not have such
> >> + a register.
> >
> > Couldn't you just read that register and tell?
>
> In theory, yes, but as far as I understand (and as I wrote) it's
> specific to the ls1021a. Of course one can decide whether it's
> necessary/possible to read it based on the compatible string, but one
> would also need an extra reg property to have its address - but that
> register is not really part of the extirq "device" we're trying to
> describe. So would it need to be represented as its own subnode of scfg?
Keep in mind, that not all Layerscapes have that feature and the
corresponding register. As you said that may be handled via compatible
string. However, the bit-reverse property looks like the simplest
solution to me.
>
> If it is set at all, it's done within the first few instructions after
> reset (before control is even handed to the bootloader), so I see it as
> a kind of quirk of the hardware. The data sheet says "SCFG bit reverse
> (SCFG_SCFGREVCR) must be written 0x_ as a part of initialization
> sequence before writing to any other SCFG register." which, taken
> literally, means we don't need the property at all and can just assume
> it for the ls1021a (i.e., do it based on compatible string alone) - but
> I think it should be read as "if you're going to write this register, it
> must be done first thing".
>
> > Does this apply to only the extirq register or all of scfg?
>
> All of scfg. It really seems like some accident/bad joke coming out of a
> discussion between a hardware and software engineer on the enumeration
> of bits, with the hardware guy ending up saying "alright, have it
> whichever way you want it", causing even more pain :(
>
> >> +
> >> +Example:
> >> + scfg: scfg@157 {
> >> + compatible = "fsl,ls1021a-scfg", "syscon";
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >
> > As the child node(s) are memory mapped, this should not be 0. And you
> > need 'ranges'.
>
> Indeed - I think I understand this a little better now than I did back then.
>
Okay.
> Thanks,
> Rasmus
Thanks,
Kurt
signature.asc
Description: PGP signature
[PATCH v6 0/2] Add support for Layerscape external interrupt lines
Hi, this is a respin of getting the support for the Layerscape's external interrupt lines. The last version from Rasmus Villemoes can be found here: https://lkml.kernel.org/r/20180223210901.23480-1-rasmus.villem...@prevas.dk/ Rasmus Villemoes ran out of time, so I prepared v6. Changes since v5: - Rebase to v5.3.0 - Integrated Thomas Gleixner comments: - Adjust order of local variables - Use irq_chip_set_type_parent() - Cleanup of irq headers - Integrated Rob Herring comments: - Add #address-cells and #size-cells to parent - Use ARCH_LAYERSCAPE, as this feature is not LS1021A specific It is tested on a Layerscape LS2088A. Thanks, Kurt Rasmus Villemoes (2): irqchip: Add support for Layerscape external interrupt lines dt/bindings: Add bindings for Layerscape external irqs .../interrupt-controller/fsl,ls-extirq.txt| 47 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-ls-extirq.c | 174 ++ 3 files changed, 222 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt create mode 100644 drivers/irqchip/irq-ls-extirq.c -- 2.20.1
[PATCH v6 2/2] dt/bindings: Add bindings for Layerscape external irqs
From: Rasmus Villemoes
This adds Device Tree binding documentation for the external interrupt
lines with configurable polarity present on some Layerscape SOCs.
Signed-off-by: Rasmus Villemoes
Signed-off-by: Kurt Kanzenbach
---
Changes since v5:
- Add #address-cells and #size-cells to parent
- Mention LS2088A and the ISC unit
.../interrupt-controller/fsl,ls-extirq.txt| 47 +++
1 file changed, 47 insertions(+)
create mode 100644
Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt
b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt
new file mode 100644
index ..7b53f9cc8019
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt
@@ -0,0 +1,47 @@
+* Freescale Layerscape external IRQs
+
+Some Layerscape SOCs (LS1021A, LS1043A, LS1046A, LS2088A) support
+inverting the polarity of certain external interrupt lines.
+
+The device node must be a child of the node representing the
+Supplemental Configuration Unit (SCFG) or the Interrupt Sampling
+Control (ISC) Unit.
+
+Required properties:
+- compatible: should be "fsl,-extirq", e.g. "fsl,ls1021a-extirq".
+- interrupt-controller: Identifies the node as an interrupt controller
+- #interrupt-cells: Must be 2. The first element is the index of the
+ external interrupt line. The second element is the trigger type.
+- interrupt-parent: phandle of GIC.
+- reg: Specifies the Interrupt Polarity Control Register (INTPCR) in the SCFG.
+- fsl,extirq-map: Specifies the mapping to interrupt numbers in the parent
+ interrupt controller. Interrupts are mapped one-to-one to parent
+ interrupts.
+
+Optional properties:
+- fsl,bit-reverse: This boolean property should be set on the LS1021A
+ if the SCFGREVCR register has been set to all-ones (which is usually
+ the case), meaning that all reads and writes of SCFG registers are
+ implicitly bit-reversed. Other compatible platforms do not have such
+ a register.
+
+Example:
+ scfg: scfg@157 {
+ compatible = "fsl,ls1021a-scfg", "syscon";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ...
+ extirq: interrupt-controller {
+ compatible = "fsl,ls1021a-extirq";
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ interrupt-parent = <>;
+ reg = <0x1ac>;
+ fsl,extirq-map = <163 164 165 167 168 169>;
+ fsl,bit-reverse;
+ };
+ };
+
+
+ interrupts-extended = < GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>,
+ < 1 IRQ_TYPE_LEVEL_LOW>;
--
2.20.1
[PATCH v6 1/2] irqchip: Add support for Layerscape external interrupt lines
From: Rasmus Villemoes
The LS1021A allows inverting the polarity of six interrupt lines
IRQ[0:5] via the scfg_intpcr register, effectively allowing
IRQ_TYPE_LEVEL_LOW and IRQ_TYPE_EDGE_FALLING for those. We just need to
check the type, set the relevant bit in INTPCR accordingly, and fixup
the type argument before calling the GIC's irq_set_type.
In fact, the power-on-reset value of the INTPCR register on the LS1021A
is so that all six lines have their polarity inverted. Hence any
hardware connected to those lines is unusable without this: If the line
is indeed active low, the generic GIC code will reject an irq spec with
IRQ_TYPE_LEVEL_LOW, while if the line is active high, we must obviously
disable the polarity inversion (writing 0 to the relevant bit) before
unmasking the interrupt.
Some other Layerscape SOCs (LS1043A, LS1046A, LS2088A) reportedly have a
similar feature, just with a different number of external interrupt lines
(and a different POR value for the INTPCR register). This driver should be
prepared for supporting those by properly filling out the device tree
node, but I don't have the full reference manual, nor the hardware to be
able to test it. I do know, from a tiny clipout from one of the other
reference manuals I was shown, that 1U<
Signed-off-by: Kurt Kanzenbach
---
Changes since v5:
- Adjust order of local variables
- Cleanup of irq headers
- Use irq_chip_set_type_parent()
- Compile with ARCH_LAYERSCAPE
- Removed last paragraph of changelog, as ARCH_LAYERSCAPE should be used
- Also mention the LS2088A SoC in changelog
drivers/irqchip/Makefile| 1 +
drivers/irqchip/irq-ls-extirq.c | 174
2 files changed, 175 insertions(+)
create mode 100644 drivers/irqchip/irq-ls-extirq.c
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 8d0fcec6ab23..ce6db511124a 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_MVEBU_ODMI) += irq-mvebu-odmi.o
obj-$(CONFIG_MVEBU_PIC)+= irq-mvebu-pic.o
obj-$(CONFIG_MVEBU_SEI)+= irq-mvebu-sei.o
obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o
+obj-$(CONFIG_ARCH_LAYERSCAPE) += irq-ls-extirq.o
obj-$(CONFIG_EZNPS_GIC)+= irq-eznps.o
obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o
obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o
diff --git a/drivers/irqchip/irq-ls-extirq.c b/drivers/irqchip/irq-ls-extirq.c
new file mode 100644
index ..c1d1bfee585c
--- /dev/null
+++ b/drivers/irqchip/irq-ls-extirq.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define pr_fmt(fmt) "irq-ls-extirq: " fmt
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAXIRQ 12
+
+struct extirq_chip_data {
+ struct regmap *syscon;
+ u32 intpcr;
+ bool bit_reverse;
+ u32 nirq;
+ u32 parent_irq[MAXIRQ];
+};
+
+static int
+ls_extirq_set_type(struct irq_data *data, unsigned int type)
+{
+ struct extirq_chip_data *chip_data = data->chip_data;
+ irq_hw_number_t hwirq = data->hwirq;
+ u32 value, mask;
+
+ if (chip_data->bit_reverse)
+ mask = 1U << (31 - hwirq);
+ else
+ mask = 1U << hwirq;
+
+ switch (type) {
+ case IRQ_TYPE_LEVEL_LOW:
+ type = IRQ_TYPE_LEVEL_HIGH;
+ value = mask;
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ type = IRQ_TYPE_EDGE_RISING;
+ value = mask;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ case IRQ_TYPE_EDGE_RISING:
+ value = 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(chip_data->syscon, chip_data->intpcr, mask, value);
+
+ return irq_chip_set_type_parent(data, type);
+}
+
+static struct irq_chip extirq_chip = {
+ .name = "extirq",
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_eoi= irq_chip_eoi_parent,
+ .irq_set_type = ls_extirq_set_type,
+ .irq_retrigger = irq_chip_retrigger_hierarchy,
+ .irq_set_affinity = irq_chip_set_affinity_parent,
+ .flags = IRQCHIP_SET_TYPE_MASKED,
+};
+
+static int
+ls_extirq_domain_translate(struct irq_domain *d, struct irq_fwspec *fwspec,
+ unsigned long *hwirq, unsigned int *type)
+{
+ if (!is_of_node(fwspec->fwnode))
+ return -EINVAL;
+
+ if (fwspec->param_count != 2)
+ return -EINVAL;
+
+ *hwirq = fwspec->param[0];
+ *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
Re: [PATCH] rt-tests: backfire: Don't include asm/uaccess.h directly
Hi, On Tue, Sep 17, 2019 at 09:15:46AM +0200, Sultan Alsawaf wrote: > On Mon, Sep 16, 2019 at 11:57:32PM +0200, John Kacur wrote: > > Signed-off-by: John Kacur > > But please in the future > > 1. Don't cc lkml on this > > 2. Include the maintainers in your patch > > Hi, > > Thanks for the sign-off. I was following the instructions listed here: > https://wiki.linuxfoundation.org/realtime/communication/send_rt_patches I guess, that's for rt kernel patches. > > I couldn't find any documentation of how to send patches for > rt-tests. Is there a different set of patch submission instructions on > a wiki somewhere I missed? For rt-tests see the top level MAINTAINERS file on how to send patches. > > Thanks, > Sultan Thanks, Kurt signature.asc Description: PGP signature
Re: [PATCH v5 1/2] irqchip: add support for Layerscape external interrupt lines
Hi,
On Fri, May 04, 2018 at 09:44:25AM +0200, Rasmus Villemoes wrote:
> >> +static int
> >> +ls_extirq_set_type(struct irq_data *data, unsigned int type)
> >> +{
> >> + irq_hw_number_t hwirq = data->hwirq;
> >> + struct extirq_chip_data *chip_data = data->chip_data;
> >> + u32 value, mask;
> >
> > Please order local variables in reverse fir tree fashion whenever
> > possible. That's way simpler to read:
> >
> > struct extirq_chip_data *chip_data = data->chip_data;
> > irq_hw_number_t hwirq = data->hwirq;
> > u32 value, mask;
>
> Fixed, thanks.
Did you send a sixth version of this patch set? It seems like the code
hasn't been merged, yet. I also need support for the external interrupt
lines on a different Layerscape.
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: [PATCH RT v2] Fix a lockup in wait_for_completion() and friends
Hi,
On Mon, Jul 01, 2019 at 05:28:25PM -0400, Steven Rostedt wrote:
> On Mon, 1 Jul 2019 17:13:33 -0400
> Steven Rostedt wrote:
>
> > On Mon, 1 Jul 2019 17:06:02 -0400
> > Steven Rostedt wrote:
> >
> > > On Mon, 1 Jul 2019 15:43:25 -0500
> > > Corey Minyard wrote:
> > >
> > >
> > > > I show that patch is already applied at
> > > >
> > > > 1921ea799b7dc561c97185538100271d88ee47db
> > > > sched/completion: Fix a lockup in wait_for_completion()
> > > >
> > > > git describe --contains 1921ea799b7dc561c97185538100271d88ee47db
> > > > v4.19.37-rt20~1
> > > >
> > > > So I'm not sure what is going on.
> > >
> > > Bah, I'm replying to the wrong commit that I'm having issues with.
> > >
> > > I searched your name to find the patch that is of trouble, and picked
> > > this one.
> > >
> > > I'll go find the problem patch, sorry for the noise on this one.
> > >
> >
> > No, I did reply to the right email, but it wasn't the top patch I was
> > having issues with. It was the patch I replied to:
> >
> > This change below that Sebastian marked as stable-rt is what is causing
> > me an issue. Not the patch that started the thread.
> >
>
> In fact, my system doesn't boot with this commit in 5.0-rt.
>
> If I revert 90e1b18eba2ae4a729 ("swait: Delete the task from after a
> wakeup occured") the machine boots again.
>
> Sebastian, I think that's a bad commit, please revert it.
I'm having the same problem on a Cyclone V based ARM board. Reverting
this commit solves the boot issue for me as well.
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: [PATCH 1/2] net: axienet: recheck condition after timeout in mdio_wait()
On Tue, Oct 30, 2018 at 11:25:11AM -0700, David Miller wrote:
> From: Kurt Kanzenbach
> Date: Tue, 30 Oct 2018 10:31:38 +0100
>
> > The function could report a false positive if it gets preempted between
> > reading
> > the XAE_MDIO_MCR_OFFSET register and checking for the timeout. In such a
> > case,
> > the condition has to be rechecked to avoid false positives.
> >
> > Therefore, check for expected condition even after the timeout occurred.
> >
> > Signed-off-by: Kurt Kanzenbach
> ...
> > if (time_before_eq(end, jiffies)) {
> > - WARN_ON(1);
> > - return -ETIMEDOUT;
> > + val = axienet_ior(lp, XAE_MDIO_MCR_OFFSET);
> > + break;
> > }
> > +
> > udelay(1);
> > }
> > - return 0;
> > + if (val & XAE_MDIO_MCR_READY_MASK)
> > + return 0;
> > +
> > + WARN_ON(1);
> > + return -ETIMEDOUT;
>
> You are not fundamentally changing the situation at all.
>
> The condtion could change right after your last read of
> XAR_MDIO_MCR_OFFSET, which is the same thing that happens before your
> modifications to this code.
That's true. The problem is different: If the current task gets
preempted by a higher priority task between checking the condition and
the timeout code, then a timeout might be falsely detected. Consider the
following events:
loop:
check mdio condition
task with real time priority may run for a long time
check for timeout
wait
That's why I've added the recheck of the condition in the timeout case.
>
> It sounds more like the timeout is slightly too short, and that's the
> real problem that causes whatever behavior you think you are fixing
> here.
The timeout value is not the problem here.
Thanks,
Kurt
Re: [PATCH 1/2] net: axienet: recheck condition after timeout in mdio_wait()
On Tue, Oct 30, 2018 at 11:25:11AM -0700, David Miller wrote:
> From: Kurt Kanzenbach
> Date: Tue, 30 Oct 2018 10:31:38 +0100
>
> > The function could report a false positive if it gets preempted between
> > reading
> > the XAE_MDIO_MCR_OFFSET register and checking for the timeout. In such a
> > case,
> > the condition has to be rechecked to avoid false positives.
> >
> > Therefore, check for expected condition even after the timeout occurred.
> >
> > Signed-off-by: Kurt Kanzenbach
> ...
> > if (time_before_eq(end, jiffies)) {
> > - WARN_ON(1);
> > - return -ETIMEDOUT;
> > + val = axienet_ior(lp, XAE_MDIO_MCR_OFFSET);
> > + break;
> > }
> > +
> > udelay(1);
> > }
> > - return 0;
> > + if (val & XAE_MDIO_MCR_READY_MASK)
> > + return 0;
> > +
> > + WARN_ON(1);
> > + return -ETIMEDOUT;
>
> You are not fundamentally changing the situation at all.
>
> The condtion could change right after your last read of
> XAR_MDIO_MCR_OFFSET, which is the same thing that happens before your
> modifications to this code.
That's true. The problem is different: If the current task gets
preempted by a higher priority task between checking the condition and
the timeout code, then a timeout might be falsely detected. Consider the
following events:
loop:
check mdio condition
task with real time priority may run for a long time
check for timeout
wait
That's why I've added the recheck of the condition in the timeout case.
>
> It sounds more like the timeout is slightly too short, and that's the
> real problem that causes whatever behavior you think you are fixing
> here.
The timeout value is not the problem here.
Thanks,
Kurt
[PATCH 2/2] net: xilinx_emaclite: recheck condition after timeout in mdio_wait()
The function could report a false positive if it gets preempted between reading
the XEL_MDIOCTRL_OFFSET register and checking for the timeout. In such a case,
the condition has to be rechecked to avoid false positives.
Therefore, check for expected condition even after the timeout occurred.
Signed-off-by: Kurt Kanzenbach
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 639e3e99af46..957d03085bd0 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -714,19 +714,29 @@ static irqreturn_t xemaclite_interrupt(int irq, void
*dev_id)
static int xemaclite_mdio_wait(struct net_local *lp)
{
unsigned long end = jiffies + 2;
+ u32 val;
/* wait for the MDIO interface to not be busy or timeout
* after some time.
*/
- while (xemaclite_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET) &
- XEL_MDIOCTRL_MDIOSTS_MASK) {
+ while (1) {
+ val = xemaclite_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET);
+
+ if (!(val & XEL_MDIOCTRL_MDIOSTS_MASK))
+ return 0;
+
if (time_before_eq(end, jiffies)) {
- WARN_ON(1);
- return -ETIMEDOUT;
+ val = xemaclite_readl(lp->base_addr +
XEL_MDIOCTRL_OFFSET);
+ break;
}
+
msleep(1);
}
- return 0;
+ if (!(val & XEL_MDIOCTRL_MDIOSTS_MASK))
+ return 0;
+
+ WARN_ON(1);
+ return -ETIMEDOUT;
}
/**
--
2.11.0
[PATCH 1/2] net: axienet: recheck condition after timeout in mdio_wait()
The function could report a false positive if it gets preempted between reading
the XAE_MDIO_MCR_OFFSET register and checking for the timeout. In such a case,
the condition has to be rechecked to avoid false positives.
Therefore, check for expected condition even after the timeout occurred.
Signed-off-by: Kurt Kanzenbach
---
drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
index 757a3b37ae8a..4f13125e4942 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
@@ -21,15 +21,26 @@
int axienet_mdio_wait_until_ready(struct axienet_local *lp)
{
unsigned long end = jiffies + 2;
- while (!(axienet_ior(lp, XAE_MDIO_MCR_OFFSET) &
-XAE_MDIO_MCR_READY_MASK)) {
+ u32 val;
+
+ while (1) {
+ val = axienet_ior(lp, XAE_MDIO_MCR_OFFSET);
+
+ if (val & XAE_MDIO_MCR_READY_MASK)
+ return 0;
+
if (time_before_eq(end, jiffies)) {
- WARN_ON(1);
- return -ETIMEDOUT;
+ val = axienet_ior(lp, XAE_MDIO_MCR_OFFSET);
+ break;
}
+
udelay(1);
}
- return 0;
+ if (val & XAE_MDIO_MCR_READY_MASK)
+ return 0;
+
+ WARN_ON(1);
+ return -ETIMEDOUT;
}
/**
--
2.11.0
[PATCH 2/2] net: xilinx_emaclite: recheck condition after timeout in mdio_wait()
The function could report a false positive if it gets preempted between reading
the XEL_MDIOCTRL_OFFSET register and checking for the timeout. In such a case,
the condition has to be rechecked to avoid false positives.
Therefore, check for expected condition even after the timeout occurred.
Signed-off-by: Kurt Kanzenbach
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 639e3e99af46..957d03085bd0 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -714,19 +714,29 @@ static irqreturn_t xemaclite_interrupt(int irq, void
*dev_id)
static int xemaclite_mdio_wait(struct net_local *lp)
{
unsigned long end = jiffies + 2;
+ u32 val;
/* wait for the MDIO interface to not be busy or timeout
* after some time.
*/
- while (xemaclite_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET) &
- XEL_MDIOCTRL_MDIOSTS_MASK) {
+ while (1) {
+ val = xemaclite_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET);
+
+ if (!(val & XEL_MDIOCTRL_MDIOSTS_MASK))
+ return 0;
+
if (time_before_eq(end, jiffies)) {
- WARN_ON(1);
- return -ETIMEDOUT;
+ val = xemaclite_readl(lp->base_addr +
XEL_MDIOCTRL_OFFSET);
+ break;
}
+
msleep(1);
}
- return 0;
+ if (!(val & XEL_MDIOCTRL_MDIOSTS_MASK))
+ return 0;
+
+ WARN_ON(1);
+ return -ETIMEDOUT;
}
/**
--
2.11.0
[PATCH 1/2] net: axienet: recheck condition after timeout in mdio_wait()
The function could report a false positive if it gets preempted between reading
the XAE_MDIO_MCR_OFFSET register and checking for the timeout. In such a case,
the condition has to be rechecked to avoid false positives.
Therefore, check for expected condition even after the timeout occurred.
Signed-off-by: Kurt Kanzenbach
---
drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
index 757a3b37ae8a..4f13125e4942 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
@@ -21,15 +21,26 @@
int axienet_mdio_wait_until_ready(struct axienet_local *lp)
{
unsigned long end = jiffies + 2;
- while (!(axienet_ior(lp, XAE_MDIO_MCR_OFFSET) &
-XAE_MDIO_MCR_READY_MASK)) {
+ u32 val;
+
+ while (1) {
+ val = axienet_ior(lp, XAE_MDIO_MCR_OFFSET);
+
+ if (val & XAE_MDIO_MCR_READY_MASK)
+ return 0;
+
if (time_before_eq(end, jiffies)) {
- WARN_ON(1);
- return -ETIMEDOUT;
+ val = axienet_ior(lp, XAE_MDIO_MCR_OFFSET);
+ break;
}
+
udelay(1);
}
- return 0;
+ if (val & XAE_MDIO_MCR_READY_MASK)
+ return 0;
+
+ WARN_ON(1);
+ return -ETIMEDOUT;
}
/**
--
2.11.0
Re: [Problem] Cache line starvation
On Fri, Sep 28, 2018 at 11:05:21AM +0200, Kurt Kanzenbach wrote:
> Hi Thomas,
>
> On Thu, Sep 27, 2018 at 04:47:47PM +0200, Thomas Gleixner wrote:
> > On Thu, 27 Sep 2018, Kurt Kanzenbach wrote:
> > > On Thu, Sep 27, 2018 at 04:25:47PM +0200, Kurt Kanzenbach wrote:
> > > > However, the issue still triggers fine. With stress-ng we're able to
> > > > generate latency in millisecond range. The only workaround we've found
> > > > so far is to add a "delay" in cpu_relax().
> > >
> > > It might interesting for you, how we added the delay. We've used:
> > >
> > > static inline void cpu_relax(void)
> > > {
> > > volatile int i = 0;
> > >
> > > asm volatile("yield" ::: "memory");
> > > while (i++ <= 1000);
> > > }
> > >
> > > Of course it's not efficient, but it works.
> >
> > I wonder if it's just the store on the stack which makes it work. I've seen
> > that when instrumenting x86. When the careful instrumentation just stayed
> > in registers it failed. Once it was too much and stack got involved it
> > vanished away.
>
> I've performed more tests: Adding a store to a global variable just
> before calling cpu_relax() doesn't help. Furthermore, adding up to 20
> yield instructions (just like you did on x86) didn't work either.
In addition, the stress-ng test triggers on v4.14-rt and v4.18-rt as
well.
As v4.18-rt still uses the old spin lock implementation, I've backported
the qspinlock implementation to v4.18-rt. The commits I've identified
are:
- 598865c5f32d ("arm64: barrier: Implement smp_cond_load_relaxed")
- c11090474d70 ("arm64: locking: Replace ticket lock implementation with
qspinlock")
Using these commits it's still possible to trigger the issue. But it
takes longer.
Did I miss anything?
Thanks,
Kurt
Re: [Problem] Cache line starvation
On Fri, Sep 28, 2018 at 11:05:21AM +0200, Kurt Kanzenbach wrote:
> Hi Thomas,
>
> On Thu, Sep 27, 2018 at 04:47:47PM +0200, Thomas Gleixner wrote:
> > On Thu, 27 Sep 2018, Kurt Kanzenbach wrote:
> > > On Thu, Sep 27, 2018 at 04:25:47PM +0200, Kurt Kanzenbach wrote:
> > > > However, the issue still triggers fine. With stress-ng we're able to
> > > > generate latency in millisecond range. The only workaround we've found
> > > > so far is to add a "delay" in cpu_relax().
> > >
> > > It might interesting for you, how we added the delay. We've used:
> > >
> > > static inline void cpu_relax(void)
> > > {
> > > volatile int i = 0;
> > >
> > > asm volatile("yield" ::: "memory");
> > > while (i++ <= 1000);
> > > }
> > >
> > > Of course it's not efficient, but it works.
> >
> > I wonder if it's just the store on the stack which makes it work. I've seen
> > that when instrumenting x86. When the careful instrumentation just stayed
> > in registers it failed. Once it was too much and stack got involved it
> > vanished away.
>
> I've performed more tests: Adding a store to a global variable just
> before calling cpu_relax() doesn't help. Furthermore, adding up to 20
> yield instructions (just like you did on x86) didn't work either.
In addition, the stress-ng test triggers on v4.14-rt and v4.18-rt as
well.
As v4.18-rt still uses the old spin lock implementation, I've backported
the qspinlock implementation to v4.18-rt. The commits I've identified
are:
- 598865c5f32d ("arm64: barrier: Implement smp_cond_load_relaxed")
- c11090474d70 ("arm64: locking: Replace ticket lock implementation with
qspinlock")
Using these commits it's still possible to trigger the issue. But it
takes longer.
Did I miss anything?
Thanks,
Kurt
Re: [Problem] Cache line starvation
Hi Thomas,
On Thu, Sep 27, 2018 at 04:47:47PM +0200, Thomas Gleixner wrote:
> On Thu, 27 Sep 2018, Kurt Kanzenbach wrote:
> > On Thu, Sep 27, 2018 at 04:25:47PM +0200, Kurt Kanzenbach wrote:
> > > However, the issue still triggers fine. With stress-ng we're able to
> > > generate latency in millisecond range. The only workaround we've found
> > > so far is to add a "delay" in cpu_relax().
> >
> > It might interesting for you, how we added the delay. We've used:
> >
> > static inline void cpu_relax(void)
> > {
> > volatile int i = 0;
> >
> > asm volatile("yield" ::: "memory");
> > while (i++ <= 1000);
> > }
> >
> > Of course it's not efficient, but it works.
>
> I wonder if it's just the store on the stack which makes it work. I've seen
> that when instrumenting x86. When the careful instrumentation just stayed
> in registers it failed. Once it was too much and stack got involved it
> vanished away.
I've performed more tests: Adding a store to a global variable just
before calling cpu_relax() doesn't help. Furthermore, adding up to 20
yield instructions (just like you did on x86) didn't work either.
Thanks,
Kurt
Re: [Problem] Cache line starvation
Hi Thomas,
On Thu, Sep 27, 2018 at 04:47:47PM +0200, Thomas Gleixner wrote:
> On Thu, 27 Sep 2018, Kurt Kanzenbach wrote:
> > On Thu, Sep 27, 2018 at 04:25:47PM +0200, Kurt Kanzenbach wrote:
> > > However, the issue still triggers fine. With stress-ng we're able to
> > > generate latency in millisecond range. The only workaround we've found
> > > so far is to add a "delay" in cpu_relax().
> >
> > It might interesting for you, how we added the delay. We've used:
> >
> > static inline void cpu_relax(void)
> > {
> > volatile int i = 0;
> >
> > asm volatile("yield" ::: "memory");
> > while (i++ <= 1000);
> > }
> >
> > Of course it's not efficient, but it works.
>
> I wonder if it's just the store on the stack which makes it work. I've seen
> that when instrumenting x86. When the careful instrumentation just stayed
> in registers it failed. Once it was too much and stack got involved it
> vanished away.
I've performed more tests: Adding a store to a global variable just
before calling cpu_relax() doesn't help. Furthermore, adding up to 20
yield instructions (just like you did on x86) didn't work either.
Thanks,
Kurt
Re: [Problem] Cache line starvation
Hi Will,
On Thu, Sep 27, 2018 at 04:25:47PM +0200, Kurt Kanzenbach wrote:
> Hi Will,
>
> On Wed, Sep 26, 2018 at 01:53:02PM +0100, Will Deacon wrote:
> > Hi all,
> >
> > On Fri, Sep 21, 2018 at 02:02:26PM +0200, Sebastian Andrzej Siewior wrote:
> > > We reproducibly observe cache line starvation on a Core2Duo E6850 (2
> > > cores), a i5-6400 SKL (4 cores) and on a NXP LS2044A ARM Cortex-A72 (4
> > > cores).
> > >
> > > Instrumentation show always the picture:
> > >
> > > CPU0 CPU1
> > > => do_syscall_64 => do_syscall_64
> > > => SyS_ptrace => syscall_slow_exit_work
> > > => ptrace_check_attach => ptrace_do_notify /
> > > rt_read_unlock
> > > => wait_task_inactive
> > > rt_spin_lock_slowunlock()
> > >-> while task_running()
> > > __rt_mutex_unlock_common()
> > > / check_task_state()
> > > mark_wakeup_next_waiter()
> > > | raw_spin_lock_irq(>pi_lock);
> > > raw_spin_lock(>pi_lock);
> > > | . .
> > > | raw_spin_unlock_irq(>pi_lock); .
> > > \ cpu_relax() .
> > >- .
> > > *IRQ*
> > >
> > > In the error case we observe that the while() loop is repeated more than
> > > 5000 times which indicates that the pi_lock can be acquired. CPU1 on the
> > > other side does not make progress waiting for the same lock with
> > > interrupts
> > > disabled.
> > >
> > > This continues until an IRQ hits CPU0. Once CPU0 starts processing the IRQ
> > > the other CPU is able to acquire pi_lock and the situation relaxes.
> > >
> > > Peter suggested to do a clwb(>pi_lock); before the cpu_relax() in
> > > wait_task_inactive() which on both the Core2Duo and the SKL gets runtime
> > > patched to clflush(). That hides it as well.
> >
> > Given the broadcast nature of cache-flushing, I'd be pretty nervous about
> > adding it on anything other than a case-by-case basis. That doesn't sound
> > like something we'd want to maintain... It would also be interesting to know
> > whether the problem is actually before the cache (i.e. if the lock actually
> > sits in the store buffer on CPU0). Does MFENCE/DSB after the unlock() help
> > at
> > all?
> >
> > We've previously seen something similar to this on arm64 in big/little
> > systems where the big cores can loop around and re-take a spinlock before
> > the little guys can get in the queue or take a ticket. I bodged that in
> > cpu_relax(), but there's a magic heuristic which I couldn't figure out how
> > to specify:
> >
> > https://lkml.org/lkml/2017/7/28/172
> >
> > For A72 (which is the core I think you're using) it would be interesting to
> > try both:
> >
> > (1) Removing the prfm instruction from spin_lock(), and
> > (2) Setting bit 42 of CPUACTLR_EL1 on each CPU (probably needs a
> > firmware change)
>
> correct, we use the Cortex A72.
>
> I followed your suggestions. I've removed the prefetch instructions from
> the spin lock implementation in the v4.9 kernel. In addition I've
> modified armv8/start.S in U-Boot to setup bit 42 in CPUACTLR_EL1
> (S3_1_c15_c2_0). We've also made sure, that this bit is actually written
> for each CPU by reading their register value in the kernel.
>
> However, the issue still triggers fine. With stress-ng we're able to
> generate latency in millisecond range. The only workaround we've found
> so far is to add a "delay" in cpu_relax().
It might interesting for you, how we added the delay. We've used:
static inline void cpu_relax(void)
{
volatile int i = 0;
asm volatile("yield" ::: "memory");
while (i++ <= 1000);
}
Of course it's not efficient, but it works.
Thanks,
Kurt
>
> Any ideas, what we can test further?
>
> Thanks,
> Kurt
>
> >
> > That should prevent the lock() operation from speculatively pulling in the
> > cacheline in a unique state.
> >
> > More recent Arm CPUs have atomic instructions which, apart from CAS,
> > *should* avoid this starvation issue entirely.
> >
> > Will
> >
Re: [Problem] Cache line starvation
Hi Will,
On Thu, Sep 27, 2018 at 04:25:47PM +0200, Kurt Kanzenbach wrote:
> Hi Will,
>
> On Wed, Sep 26, 2018 at 01:53:02PM +0100, Will Deacon wrote:
> > Hi all,
> >
> > On Fri, Sep 21, 2018 at 02:02:26PM +0200, Sebastian Andrzej Siewior wrote:
> > > We reproducibly observe cache line starvation on a Core2Duo E6850 (2
> > > cores), a i5-6400 SKL (4 cores) and on a NXP LS2044A ARM Cortex-A72 (4
> > > cores).
> > >
> > > Instrumentation show always the picture:
> > >
> > > CPU0 CPU1
> > > => do_syscall_64 => do_syscall_64
> > > => SyS_ptrace => syscall_slow_exit_work
> > > => ptrace_check_attach => ptrace_do_notify /
> > > rt_read_unlock
> > > => wait_task_inactive
> > > rt_spin_lock_slowunlock()
> > >-> while task_running()
> > > __rt_mutex_unlock_common()
> > > / check_task_state()
> > > mark_wakeup_next_waiter()
> > > | raw_spin_lock_irq(>pi_lock);
> > > raw_spin_lock(>pi_lock);
> > > | . .
> > > | raw_spin_unlock_irq(>pi_lock); .
> > > \ cpu_relax() .
> > >- .
> > > *IRQ*
> > >
> > > In the error case we observe that the while() loop is repeated more than
> > > 5000 times which indicates that the pi_lock can be acquired. CPU1 on the
> > > other side does not make progress waiting for the same lock with
> > > interrupts
> > > disabled.
> > >
> > > This continues until an IRQ hits CPU0. Once CPU0 starts processing the IRQ
> > > the other CPU is able to acquire pi_lock and the situation relaxes.
> > >
> > > Peter suggested to do a clwb(>pi_lock); before the cpu_relax() in
> > > wait_task_inactive() which on both the Core2Duo and the SKL gets runtime
> > > patched to clflush(). That hides it as well.
> >
> > Given the broadcast nature of cache-flushing, I'd be pretty nervous about
> > adding it on anything other than a case-by-case basis. That doesn't sound
> > like something we'd want to maintain... It would also be interesting to know
> > whether the problem is actually before the cache (i.e. if the lock actually
> > sits in the store buffer on CPU0). Does MFENCE/DSB after the unlock() help
> > at
> > all?
> >
> > We've previously seen something similar to this on arm64 in big/little
> > systems where the big cores can loop around and re-take a spinlock before
> > the little guys can get in the queue or take a ticket. I bodged that in
> > cpu_relax(), but there's a magic heuristic which I couldn't figure out how
> > to specify:
> >
> > https://lkml.org/lkml/2017/7/28/172
> >
> > For A72 (which is the core I think you're using) it would be interesting to
> > try both:
> >
> > (1) Removing the prfm instruction from spin_lock(), and
> > (2) Setting bit 42 of CPUACTLR_EL1 on each CPU (probably needs a
> > firmware change)
>
> correct, we use the Cortex A72.
>
> I followed your suggestions. I've removed the prefetch instructions from
> the spin lock implementation in the v4.9 kernel. In addition I've
> modified armv8/start.S in U-Boot to setup bit 42 in CPUACTLR_EL1
> (S3_1_c15_c2_0). We've also made sure, that this bit is actually written
> for each CPU by reading their register value in the kernel.
>
> However, the issue still triggers fine. With stress-ng we're able to
> generate latency in millisecond range. The only workaround we've found
> so far is to add a "delay" in cpu_relax().
It might interesting for you, how we added the delay. We've used:
static inline void cpu_relax(void)
{
volatile int i = 0;
asm volatile("yield" ::: "memory");
while (i++ <= 1000);
}
Of course it's not efficient, but it works.
Thanks,
Kurt
>
> Any ideas, what we can test further?
>
> Thanks,
> Kurt
>
> >
> > That should prevent the lock() operation from speculatively pulling in the
> > cacheline in a unique state.
> >
> > More recent Arm CPUs have atomic instructions which, apart from CAS,
> > *should* avoid this starvation issue entirely.
> >
> > Will
> >
Re: [Problem] Cache line starvation
Hi Will, On Wed, Sep 26, 2018 at 01:53:02PM +0100, Will Deacon wrote: > Hi all, > > On Fri, Sep 21, 2018 at 02:02:26PM +0200, Sebastian Andrzej Siewior wrote: > > We reproducibly observe cache line starvation on a Core2Duo E6850 (2 > > cores), a i5-6400 SKL (4 cores) and on a NXP LS2044A ARM Cortex-A72 (4 > > cores). > > > > Instrumentation show always the picture: > > > > CPU0 CPU1 > > => do_syscall_64 => do_syscall_64 > > => SyS_ptrace => syscall_slow_exit_work > > => ptrace_check_attach => ptrace_do_notify / > > rt_read_unlock > > => wait_task_inactive rt_spin_lock_slowunlock() > >-> while task_running() > > __rt_mutex_unlock_common() > > / check_task_state() mark_wakeup_next_waiter() > > | raw_spin_lock_irq(>pi_lock); > > raw_spin_lock(>pi_lock); > > | . . > > | raw_spin_unlock_irq(>pi_lock); . > > \ cpu_relax() . > >- . > > *IRQ* > > > > In the error case we observe that the while() loop is repeated more than > > 5000 times which indicates that the pi_lock can be acquired. CPU1 on the > > other side does not make progress waiting for the same lock with interrupts > > disabled. > > > > This continues until an IRQ hits CPU0. Once CPU0 starts processing the IRQ > > the other CPU is able to acquire pi_lock and the situation relaxes. > > > > Peter suggested to do a clwb(>pi_lock); before the cpu_relax() in > > wait_task_inactive() which on both the Core2Duo and the SKL gets runtime > > patched to clflush(). That hides it as well. > > Given the broadcast nature of cache-flushing, I'd be pretty nervous about > adding it on anything other than a case-by-case basis. That doesn't sound > like something we'd want to maintain... It would also be interesting to know > whether the problem is actually before the cache (i.e. if the lock actually > sits in the store buffer on CPU0). Does MFENCE/DSB after the unlock() help at > all? > > We've previously seen something similar to this on arm64 in big/little > systems where the big cores can loop around and re-take a spinlock before > the little guys can get in the queue or take a ticket. I bodged that in > cpu_relax(), but there's a magic heuristic which I couldn't figure out how > to specify: > > https://lkml.org/lkml/2017/7/28/172 > > For A72 (which is the core I think you're using) it would be interesting to > try both: > > (1) Removing the prfm instruction from spin_lock(), and > (2) Setting bit 42 of CPUACTLR_EL1 on each CPU (probably needs a > firmware change) correct, we use the Cortex A72. I followed your suggestions. I've removed the prefetch instructions from the spin lock implementation in the v4.9 kernel. In addition I've modified armv8/start.S in U-Boot to setup bit 42 in CPUACTLR_EL1 (S3_1_c15_c2_0). We've also made sure, that this bit is actually written for each CPU by reading their register value in the kernel. However, the issue still triggers fine. With stress-ng we're able to generate latency in millisecond range. The only workaround we've found so far is to add a "delay" in cpu_relax(). Any ideas, what we can test further? Thanks, Kurt > > That should prevent the lock() operation from speculatively pulling in the > cacheline in a unique state. > > More recent Arm CPUs have atomic instructions which, apart from CAS, > *should* avoid this starvation issue entirely. > > Will >
Re: [Problem] Cache line starvation
Hi Will, On Wed, Sep 26, 2018 at 01:53:02PM +0100, Will Deacon wrote: > Hi all, > > On Fri, Sep 21, 2018 at 02:02:26PM +0200, Sebastian Andrzej Siewior wrote: > > We reproducibly observe cache line starvation on a Core2Duo E6850 (2 > > cores), a i5-6400 SKL (4 cores) and on a NXP LS2044A ARM Cortex-A72 (4 > > cores). > > > > Instrumentation show always the picture: > > > > CPU0 CPU1 > > => do_syscall_64 => do_syscall_64 > > => SyS_ptrace => syscall_slow_exit_work > > => ptrace_check_attach => ptrace_do_notify / > > rt_read_unlock > > => wait_task_inactive rt_spin_lock_slowunlock() > >-> while task_running() > > __rt_mutex_unlock_common() > > / check_task_state() mark_wakeup_next_waiter() > > | raw_spin_lock_irq(>pi_lock); > > raw_spin_lock(>pi_lock); > > | . . > > | raw_spin_unlock_irq(>pi_lock); . > > \ cpu_relax() . > >- . > > *IRQ* > > > > In the error case we observe that the while() loop is repeated more than > > 5000 times which indicates that the pi_lock can be acquired. CPU1 on the > > other side does not make progress waiting for the same lock with interrupts > > disabled. > > > > This continues until an IRQ hits CPU0. Once CPU0 starts processing the IRQ > > the other CPU is able to acquire pi_lock and the situation relaxes. > > > > Peter suggested to do a clwb(>pi_lock); before the cpu_relax() in > > wait_task_inactive() which on both the Core2Duo and the SKL gets runtime > > patched to clflush(). That hides it as well. > > Given the broadcast nature of cache-flushing, I'd be pretty nervous about > adding it on anything other than a case-by-case basis. That doesn't sound > like something we'd want to maintain... It would also be interesting to know > whether the problem is actually before the cache (i.e. if the lock actually > sits in the store buffer on CPU0). Does MFENCE/DSB after the unlock() help at > all? > > We've previously seen something similar to this on arm64 in big/little > systems where the big cores can loop around and re-take a spinlock before > the little guys can get in the queue or take a ticket. I bodged that in > cpu_relax(), but there's a magic heuristic which I couldn't figure out how > to specify: > > https://lkml.org/lkml/2017/7/28/172 > > For A72 (which is the core I think you're using) it would be interesting to > try both: > > (1) Removing the prfm instruction from spin_lock(), and > (2) Setting bit 42 of CPUACTLR_EL1 on each CPU (probably needs a > firmware change) correct, we use the Cortex A72. I followed your suggestions. I've removed the prefetch instructions from the spin lock implementation in the v4.9 kernel. In addition I've modified armv8/start.S in U-Boot to setup bit 42 in CPUACTLR_EL1 (S3_1_c15_c2_0). We've also made sure, that this bit is actually written for each CPU by reading their register value in the kernel. However, the issue still triggers fine. With stress-ng we're able to generate latency in millisecond range. The only workaround we've found so far is to add a "delay" in cpu_relax(). Any ideas, what we can test further? Thanks, Kurt > > That should prevent the lock() operation from speculatively pulling in the > cacheline in a unique state. > > More recent Arm CPUs have atomic instructions which, apart from CAS, > *should* avoid this starvation issue entirely. > > Will >
[PATCH RT] tty: serial: pl011: fix warning about uninitialized variable
Silence the following gcc warning:
drivers/tty/serial/amba-pl011.c: In function ‘pl011_console_write’:
./include/linux/spinlock.h:260:3: warning: ‘flags’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
_raw_spin_unlock_irqrestore(lock, flags); \
^~~
drivers/tty/serial/amba-pl011.c:2214:16: note: ‘flags’ was declared here
unsigned long flags;
^
The code is correct. Thus, initializing flags to zero doesn't change the
behavior and resolves the warning.
Signed-off-by: Kurt Kanzenbach
---
drivers/tty/serial/amba-pl011.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 484861278e9c..a658214486e7 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2211,7 +2211,7 @@ pl011_console_write(struct console *co, const char *s,
unsigned int count)
{
struct uart_amba_port *uap = amba_ports[co->index];
unsigned int old_cr = 0, new_cr;
- unsigned long flags;
+ unsigned long flags = 0;
int locked = 1;
clk_enable(uap->clk);
--
2.11.0
[PATCH RT] tty: serial: pl011: fix warning about uninitialized variable
Silence the following gcc warning:
drivers/tty/serial/amba-pl011.c: In function ‘pl011_console_write’:
./include/linux/spinlock.h:260:3: warning: ‘flags’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
_raw_spin_unlock_irqrestore(lock, flags); \
^~~
drivers/tty/serial/amba-pl011.c:2214:16: note: ‘flags’ was declared here
unsigned long flags;
^
The code is correct. Thus, initializing flags to zero doesn't change the
behavior and resolves the warning.
Signed-off-by: Kurt Kanzenbach
---
drivers/tty/serial/amba-pl011.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 484861278e9c..a658214486e7 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2211,7 +2211,7 @@ pl011_console_write(struct console *co, const char *s,
unsigned int count)
{
struct uart_amba_port *uap = amba_ports[co->index];
unsigned int old_cr = 0, new_cr;
- unsigned long flags;
+ unsigned long flags = 0;
int locked = 1;
clk_enable(uap->clk);
--
2.11.0
[PATCH] arm64: dts: ls208xa: add second duart
The NXP LS208xA SoCs have two dual uarts. Thus, add the second one.
Signed-off-by: Kurt Kanzenbach
---
arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 16
1 file changed, 16 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index 8cb78dd99672..547a86ec7cd2 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -22,6 +22,8 @@
crypto =
serial0 =
serial1 =
+ serial2 =
+ serial3 =
};
cpu: cpus {
@@ -221,6 +223,20 @@
interrupts = <0 32 0x4>; /* Level high type */
};
+ serial2: serial@21d0500 {
+ compatible = "fsl,ns16550", "ns16550a";
+ reg = <0x0 0x21d0500 0x0 0x100>;
+ clocks = < 4 3>;
+ interrupts = <0 33 0x4>; /* Level high type */
+ };
+
+ serial3: serial@21d0600 {
+ compatible = "fsl,ns16550", "ns16550a";
+ reg = <0x0 0x21d0600 0x0 0x100>;
+ clocks = < 4 3>;
+ interrupts = <0 33 0x4>; /* Level high type */
+ };
+
cluster1_core0_watchdog: wdt@c00 {
compatible = "arm,sp805-wdt", "arm,primecell";
reg = <0x0 0xc00 0x0 0x1000>;
--
2.11.0
[PATCH] arm64: dts: ls208xa: add second duart
The NXP LS208xA SoCs have two dual uarts. Thus, add the second one.
Signed-off-by: Kurt Kanzenbach
---
arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 16
1 file changed, 16 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index 8cb78dd99672..547a86ec7cd2 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -22,6 +22,8 @@
crypto =
serial0 =
serial1 =
+ serial2 =
+ serial3 =
};
cpu: cpus {
@@ -221,6 +223,20 @@
interrupts = <0 32 0x4>; /* Level high type */
};
+ serial2: serial@21d0500 {
+ compatible = "fsl,ns16550", "ns16550a";
+ reg = <0x0 0x21d0500 0x0 0x100>;
+ clocks = < 4 3>;
+ interrupts = <0 33 0x4>; /* Level high type */
+ };
+
+ serial3: serial@21d0600 {
+ compatible = "fsl,ns16550", "ns16550a";
+ reg = <0x0 0x21d0600 0x0 0x100>;
+ clocks = < 4 3>;
+ interrupts = <0 33 0x4>; /* Level high type */
+ };
+
cluster1_core0_watchdog: wdt@c00 {
compatible = "arm,sp805-wdt", "arm,primecell";
reg = <0x0 0xc00 0x0 0x1000>;
--
2.11.0
[PATCH v2 1/2] mtd: rawnand: fsl_ifc: check result of SRAM initialization
The SRAM initialization might fail. If that happens further NAND operations
won't be successful. Therefore, the chip init routine should fail if the SRAM
initialization didn't work.
Signed-off-by: Kurt Kanzenbach
---
Changes since v1:
- Subject
drivers/mtd/nand/raw/fsl_ifc_nand.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c
b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index 24f59d0066af..e4f5792dc589 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -761,7 +761,7 @@ static const struct nand_controller_ops
fsl_ifc_controller_ops = {
.attach_chip = fsl_ifc_attach_chip,
};
-static void fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
+static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
{
struct fsl_ifc_ctrl *ctrl = priv->ctrl;
struct fsl_ifc_runtime __iomem *ifc_runtime = ctrl->rregs;
@@ -805,12 +805,16 @@ static void fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
wait_event_timeout(ctrl->nand_wait, ctrl->nand_stat,
msecs_to_jiffies(IFC_TIMEOUT_MSECS));
- if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC)
+ if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC) {
pr_err("fsl-ifc: Failed to Initialise SRAM\n");
+ return -ETIMEDOUT;
+ }
/* Restore CSOR and CSOR_ext */
ifc_out32(csor, _global->csor_cs[cs].csor);
ifc_out32(csor_ext, _global->csor_cs[cs].csor_ext);
+
+ return 0;
}
static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
@@ -914,8 +918,13 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
chip->ecc.algo = NAND_ECC_HAMMING;
}
- if (ctrl->version >= FSL_IFC_VERSION_1_1_0)
- fsl_ifc_sram_init(priv);
+ if (ctrl->version >= FSL_IFC_VERSION_1_1_0) {
+ int ret;
+
+ ret = fsl_ifc_sram_init(priv);
+ if (ret)
+ return ret;
+ }
/*
* As IFC version 2.0.0 has 16KB of internal SRAM as compared to older
--
2.11.0
[PATCH v2 2/2] mtd: rawnand: fsl_ifc: fixup SRAM init for newer ctrl versions
Newer versions of the IFC controller use a different method of initializing the
internal SRAM: Instead of reading from flash, a bit in the NAND configuration
register has to be set in order to trigger the self-initializing process.
Signed-off-by: Kurt Kanzenbach
---
Changes since v1:
- Subject
- Coding style
- Using timeout constant
- Move version check completely into fsl_ifc_sram_init()
drivers/mtd/nand/raw/fsl_ifc_nand.c | 33 ++---
include/linux/fsl_ifc.h | 2 ++
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c
b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index e4f5792dc589..7e7729df7827 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#define ERR_BYTE 0xFF /* Value returned for read
bytes when read failed */
@@ -769,6 +770,27 @@ static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
uint32_t csor = 0, csor_8k = 0, csor_ext = 0;
uint32_t cs = priv->bank;
+ if (ctrl->version < FSL_IFC_VERSION_1_1_0)
+ return 0;
+
+ if (ctrl->version > FSL_IFC_VERSION_1_1_0) {
+ u32 ncfgr, status;
+ int ret;
+
+ /* Trigger auto initialization */
+ ncfgr = ifc_in32(_runtime->ifc_nand.ncfgr);
+ ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN,
_runtime->ifc_nand.ncfgr);
+
+ /* Wait until done */
+ ret = readx_poll_timeout(ifc_in32, _runtime->ifc_nand.ncfgr,
+status, !(status &
IFC_NAND_NCFGR_SRAM_INIT_EN),
+10, IFC_TIMEOUT_MSECS * 1000);
+ if (ret)
+ dev_err(priv->dev, "Failed to initialize SRAM!\n");
+
+ return ret;
+ }
+
/* Save CSOR and CSOR_ext */
csor = ifc_in32(_global->csor_cs[cs].csor);
csor_ext = ifc_in32(_global->csor_cs[cs].csor_ext);
@@ -825,6 +847,7 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
struct nand_chip *chip = >chip;
struct mtd_info *mtd = nand_to_mtd(>chip);
u32 csor;
+ int ret;
/* Fill in fsl_ifc_mtd structure */
mtd->dev.parent = priv->dev;
@@ -918,13 +941,9 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
chip->ecc.algo = NAND_ECC_HAMMING;
}
- if (ctrl->version >= FSL_IFC_VERSION_1_1_0) {
- int ret;
-
- ret = fsl_ifc_sram_init(priv);
- if (ret)
- return ret;
- }
+ ret = fsl_ifc_sram_init(priv);
+ if (ret)
+ return ret;
/*
* As IFC version 2.0.0 has 16KB of internal SRAM as compared to older
diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
index 3fdfede2f0f3..5f343b796ad9 100644
--- a/include/linux/fsl_ifc.h
+++ b/include/linux/fsl_ifc.h
@@ -274,6 +274,8 @@
*/
/* Auto Boot Mode */
#define IFC_NAND_NCFGR_BOOT0x8000
+/* SRAM Initialization */
+#define IFC_NAND_NCFGR_SRAM_INIT_EN0x2000
/* Addressing Mode-ROW0+n/COL0 */
#define IFC_NAND_NCFGR_ADDR_MODE_RC0 0x
/* Addressing Mode-ROW0+n/COL0+n */
--
2.11.0
[PATCH v2 0/2] mtd: rawnand: fsl_ifc: fix SRAM initialization for newer controller
Hi, this is version two of lkml.kernel.org/r/20180806092137.9287-1-k...@linutronix.de Changes since v1: - Addressed comments from Miquel Raynal - Changed subject - Coding style - Using timeout constants - Moving version check completely into fsl_ifc_sram_init() Thanks, Kurt Kurt Kanzenbach (2): mtd: rawnand: fsl_ifc: check result of SRAM initialization mtd: rawnand: fsl_ifc: fixup SRAM init for newer ctrl versions drivers/mtd/nand/raw/fsl_ifc_nand.c | 36 include/linux/fsl_ifc.h | 2 ++ 2 files changed, 34 insertions(+), 4 deletions(-) -- 2.11.0
[PATCH v2 2/2] mtd: rawnand: fsl_ifc: fixup SRAM init for newer ctrl versions
Newer versions of the IFC controller use a different method of initializing the
internal SRAM: Instead of reading from flash, a bit in the NAND configuration
register has to be set in order to trigger the self-initializing process.
Signed-off-by: Kurt Kanzenbach
---
Changes since v1:
- Subject
- Coding style
- Using timeout constant
- Move version check completely into fsl_ifc_sram_init()
drivers/mtd/nand/raw/fsl_ifc_nand.c | 33 ++---
include/linux/fsl_ifc.h | 2 ++
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c
b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index e4f5792dc589..7e7729df7827 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#define ERR_BYTE 0xFF /* Value returned for read
bytes when read failed */
@@ -769,6 +770,27 @@ static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
uint32_t csor = 0, csor_8k = 0, csor_ext = 0;
uint32_t cs = priv->bank;
+ if (ctrl->version < FSL_IFC_VERSION_1_1_0)
+ return 0;
+
+ if (ctrl->version > FSL_IFC_VERSION_1_1_0) {
+ u32 ncfgr, status;
+ int ret;
+
+ /* Trigger auto initialization */
+ ncfgr = ifc_in32(_runtime->ifc_nand.ncfgr);
+ ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN,
_runtime->ifc_nand.ncfgr);
+
+ /* Wait until done */
+ ret = readx_poll_timeout(ifc_in32, _runtime->ifc_nand.ncfgr,
+status, !(status &
IFC_NAND_NCFGR_SRAM_INIT_EN),
+10, IFC_TIMEOUT_MSECS * 1000);
+ if (ret)
+ dev_err(priv->dev, "Failed to initialize SRAM!\n");
+
+ return ret;
+ }
+
/* Save CSOR and CSOR_ext */
csor = ifc_in32(_global->csor_cs[cs].csor);
csor_ext = ifc_in32(_global->csor_cs[cs].csor_ext);
@@ -825,6 +847,7 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
struct nand_chip *chip = >chip;
struct mtd_info *mtd = nand_to_mtd(>chip);
u32 csor;
+ int ret;
/* Fill in fsl_ifc_mtd structure */
mtd->dev.parent = priv->dev;
@@ -918,13 +941,9 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
chip->ecc.algo = NAND_ECC_HAMMING;
}
- if (ctrl->version >= FSL_IFC_VERSION_1_1_0) {
- int ret;
-
- ret = fsl_ifc_sram_init(priv);
- if (ret)
- return ret;
- }
+ ret = fsl_ifc_sram_init(priv);
+ if (ret)
+ return ret;
/*
* As IFC version 2.0.0 has 16KB of internal SRAM as compared to older
diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
index 3fdfede2f0f3..5f343b796ad9 100644
--- a/include/linux/fsl_ifc.h
+++ b/include/linux/fsl_ifc.h
@@ -274,6 +274,8 @@
*/
/* Auto Boot Mode */
#define IFC_NAND_NCFGR_BOOT0x8000
+/* SRAM Initialization */
+#define IFC_NAND_NCFGR_SRAM_INIT_EN0x2000
/* Addressing Mode-ROW0+n/COL0 */
#define IFC_NAND_NCFGR_ADDR_MODE_RC0 0x
/* Addressing Mode-ROW0+n/COL0+n */
--
2.11.0
[PATCH v2 0/2] mtd: rawnand: fsl_ifc: fix SRAM initialization for newer controller
Hi, this is version two of lkml.kernel.org/r/20180806092137.9287-1-k...@linutronix.de Changes since v1: - Addressed comments from Miquel Raynal - Changed subject - Coding style - Using timeout constants - Moving version check completely into fsl_ifc_sram_init() Thanks, Kurt Kurt Kanzenbach (2): mtd: rawnand: fsl_ifc: check result of SRAM initialization mtd: rawnand: fsl_ifc: fixup SRAM init for newer ctrl versions drivers/mtd/nand/raw/fsl_ifc_nand.c | 36 include/linux/fsl_ifc.h | 2 ++ 2 files changed, 34 insertions(+), 4 deletions(-) -- 2.11.0
[PATCH v2 1/2] mtd: rawnand: fsl_ifc: check result of SRAM initialization
The SRAM initialization might fail. If that happens further NAND operations
won't be successful. Therefore, the chip init routine should fail if the SRAM
initialization didn't work.
Signed-off-by: Kurt Kanzenbach
---
Changes since v1:
- Subject
drivers/mtd/nand/raw/fsl_ifc_nand.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c
b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index 24f59d0066af..e4f5792dc589 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -761,7 +761,7 @@ static const struct nand_controller_ops
fsl_ifc_controller_ops = {
.attach_chip = fsl_ifc_attach_chip,
};
-static void fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
+static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
{
struct fsl_ifc_ctrl *ctrl = priv->ctrl;
struct fsl_ifc_runtime __iomem *ifc_runtime = ctrl->rregs;
@@ -805,12 +805,16 @@ static void fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
wait_event_timeout(ctrl->nand_wait, ctrl->nand_stat,
msecs_to_jiffies(IFC_TIMEOUT_MSECS));
- if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC)
+ if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC) {
pr_err("fsl-ifc: Failed to Initialise SRAM\n");
+ return -ETIMEDOUT;
+ }
/* Restore CSOR and CSOR_ext */
ifc_out32(csor, _global->csor_cs[cs].csor);
ifc_out32(csor_ext, _global->csor_cs[cs].csor_ext);
+
+ return 0;
}
static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
@@ -914,8 +918,13 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
chip->ecc.algo = NAND_ECC_HAMMING;
}
- if (ctrl->version >= FSL_IFC_VERSION_1_1_0)
- fsl_ifc_sram_init(priv);
+ if (ctrl->version >= FSL_IFC_VERSION_1_1_0) {
+ int ret;
+
+ ret = fsl_ifc_sram_init(priv);
+ if (ret)
+ return ret;
+ }
/*
* As IFC version 2.0.0 has 16KB of internal SRAM as compared to older
--
2.11.0
Re: [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions
Hi Miquel, On Wed, Aug 08, 2018 at 02:33:52PM +0200, Miquel Raynal wrote: > Hi Kurt, > > > > > > + u32 ncfgr, status; > > > > + int ret; > > > > + > > > > + /* Trigger auto initialization */ > > > > + ncfgr = ifc_in32(_runtime->ifc_nand.ncfgr); > > > > + ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN, > > > > _runtime->ifc_nand.ncfgr); > > > > + > > > > + /* Wait until done */ > > > > + ret = readx_poll_timeout(ifc_in32, > > > > _runtime->ifc_nand.ncfgr, > > > > +status, !(status & > > > > IFC_NAND_NCFGR_SRAM_INIT_EN), > > > > +10, 1000); > > > > > > Nit: I always prefer when delays/timeouts are defined (and may be > > > reused). > > > > Me too. I've missed that there is already a timeout constant > > IFC_TIMEOUT_MSECS (500). As it's huge, I'll add a second one. > > Well, I'm not bothered with huge timeouts, it's in the error path so we > don't really care. okay. I'll send a v2 next week addressing your comments. Thanks, Kurt
Re: [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions
Hi Miquel, On Wed, Aug 08, 2018 at 02:33:52PM +0200, Miquel Raynal wrote: > Hi Kurt, > > > > > > + u32 ncfgr, status; > > > > + int ret; > > > > + > > > > + /* Trigger auto initialization */ > > > > + ncfgr = ifc_in32(_runtime->ifc_nand.ncfgr); > > > > + ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN, > > > > _runtime->ifc_nand.ncfgr); > > > > + > > > > + /* Wait until done */ > > > > + ret = readx_poll_timeout(ifc_in32, > > > > _runtime->ifc_nand.ncfgr, > > > > +status, !(status & > > > > IFC_NAND_NCFGR_SRAM_INIT_EN), > > > > +10, 1000); > > > > > > Nit: I always prefer when delays/timeouts are defined (and may be > > > reused). > > > > Me too. I've missed that there is already a timeout constant > > IFC_TIMEOUT_MSECS (500). As it's huge, I'll add a second one. > > Well, I'm not bothered with huge timeouts, it's in the error path so we > don't really care. okay. I'll send a v2 next week addressing your comments. Thanks, Kurt
Re: [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions
Hi Miquel,
On Wed, Aug 08, 2018 at 11:48:32AM +0200, Miquel Raynal wrote:
> Hi Kurt,
>
> Subject prefix should be "mtd: rawnand: fsl_ifc:".
okay, noted.
>
> Kurt Kanzenbach wrote on Mon, 6 Aug 2018 11:21:37
> +0200:
>
> > Newer versions of the IFC controller use a different method of initializing
> > the
> > internal SRAM: Instead of reading from flash, a bit in the NAND
> > configuration
> > register has to be set in order to trigger the self-initializing process.
> >
> > Signed-off-by: Kurt Kanzenbach
> > ---
> > drivers/mtd/nand/raw/fsl_ifc_nand.c | 18 ++
> > include/linux/fsl_ifc.h | 2 ++
> > 2 files changed, 20 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c
> > b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> > index e4f5792dc589..384d5e12b05c 100644
> > --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
> > +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> > @@ -30,6 +30,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #define ERR_BYTE 0xFF /* Value returned for read
> > bytes when read failed */
> > @@ -769,6 +770,23 @@ static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
> > uint32_t csor = 0, csor_8k = 0, csor_ext = 0;
> > uint32_t cs = priv->bank;
> >
> > + if (ctrl->version > FSL_IFC_VERSION_1_1_0) {
>
> This is redundant and fsl_ifc_sram_init() is called only if
> "ctrl->version > FSL_FC_VERSION_1_1_0".
No, it's not. It's called when ctrl->version >=
FSL_IFC_VERSION_1_1_0. Therefore, this check is needed.
>
> So this means this function has never worked?
It did work for e.g. IFC controller in version 1.1.0.
However, it worked for the newer versions by accident, because U-Boot
already initialized the SRAM correctly. If you boot without NAND
initialization in U-Boot, then you'll hit the issue.
>
> If this is the case, there should be at least a Fixes: tag.
>
> Maybe it would be cleaner to always call fsl_ifc_sram_init() from the
> probe(), and just exit with a "return 0" here if the version is old?
> (I'll let you choose the way you prefer).
Sounds like a good idea. Otherwise we have to check the version twice.
>
> > + u32 ncfgr, status;
> > + int ret;
> > +
> > + /* Trigger auto initialization */
> > + ncfgr = ifc_in32(_runtime->ifc_nand.ncfgr);
> > + ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN,
> > _runtime->ifc_nand.ncfgr);
> > +
> > + /* Wait until done */
> > + ret = readx_poll_timeout(ifc_in32, _runtime->ifc_nand.ncfgr,
> > +status, !(status &
> > IFC_NAND_NCFGR_SRAM_INIT_EN),
> > +10, 1000);
>
> Nit: I always prefer when delays/timeouts are defined (and may be
> reused).
Me too. I've missed that there is already a timeout constant
IFC_TIMEOUT_MSECS (500). As it's huge, I'll add a second one.
>
> > + if (ret)
> > + dev_err(priv->dev, "Failed to initialize SRAM!\n");
>
> Space
okay.
Thanks,
Kurt
>
> > + return ret;
> > + }
> > +
> > /* Save CSOR and CSOR_ext */
> > csor = ifc_in32(_global->csor_cs[cs].csor);
> > csor_ext = ifc_in32(_global->csor_cs[cs].csor_ext);
> > diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
> > index 3fdfede2f0f3..5f343b796ad9 100644
> > --- a/include/linux/fsl_ifc.h
> > +++ b/include/linux/fsl_ifc.h
> > @@ -274,6 +274,8 @@
> > */
> > /* Auto Boot Mode */
> > #define IFC_NAND_NCFGR_BOOT0x8000
> > +/* SRAM Initialization */
> > +#define IFC_NAND_NCFGR_SRAM_INIT_EN0x2000
> > /* Addressing Mode-ROW0+n/COL0 */
> > #define IFC_NAND_NCFGR_ADDR_MODE_RC0 0x
> > /* Addressing Mode-ROW0+n/COL0+n */
>
>
> Thanks,
> Miquèl
Re: [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions
Hi Miquel,
On Wed, Aug 08, 2018 at 11:48:32AM +0200, Miquel Raynal wrote:
> Hi Kurt,
>
> Subject prefix should be "mtd: rawnand: fsl_ifc:".
okay, noted.
>
> Kurt Kanzenbach wrote on Mon, 6 Aug 2018 11:21:37
> +0200:
>
> > Newer versions of the IFC controller use a different method of initializing
> > the
> > internal SRAM: Instead of reading from flash, a bit in the NAND
> > configuration
> > register has to be set in order to trigger the self-initializing process.
> >
> > Signed-off-by: Kurt Kanzenbach
> > ---
> > drivers/mtd/nand/raw/fsl_ifc_nand.c | 18 ++
> > include/linux/fsl_ifc.h | 2 ++
> > 2 files changed, 20 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c
> > b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> > index e4f5792dc589..384d5e12b05c 100644
> > --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
> > +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> > @@ -30,6 +30,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #define ERR_BYTE 0xFF /* Value returned for read
> > bytes when read failed */
> > @@ -769,6 +770,23 @@ static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
> > uint32_t csor = 0, csor_8k = 0, csor_ext = 0;
> > uint32_t cs = priv->bank;
> >
> > + if (ctrl->version > FSL_IFC_VERSION_1_1_0) {
>
> This is redundant and fsl_ifc_sram_init() is called only if
> "ctrl->version > FSL_FC_VERSION_1_1_0".
No, it's not. It's called when ctrl->version >=
FSL_IFC_VERSION_1_1_0. Therefore, this check is needed.
>
> So this means this function has never worked?
It did work for e.g. IFC controller in version 1.1.0.
However, it worked for the newer versions by accident, because U-Boot
already initialized the SRAM correctly. If you boot without NAND
initialization in U-Boot, then you'll hit the issue.
>
> If this is the case, there should be at least a Fixes: tag.
>
> Maybe it would be cleaner to always call fsl_ifc_sram_init() from the
> probe(), and just exit with a "return 0" here if the version is old?
> (I'll let you choose the way you prefer).
Sounds like a good idea. Otherwise we have to check the version twice.
>
> > + u32 ncfgr, status;
> > + int ret;
> > +
> > + /* Trigger auto initialization */
> > + ncfgr = ifc_in32(_runtime->ifc_nand.ncfgr);
> > + ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN,
> > _runtime->ifc_nand.ncfgr);
> > +
> > + /* Wait until done */
> > + ret = readx_poll_timeout(ifc_in32, _runtime->ifc_nand.ncfgr,
> > +status, !(status &
> > IFC_NAND_NCFGR_SRAM_INIT_EN),
> > +10, 1000);
>
> Nit: I always prefer when delays/timeouts are defined (and may be
> reused).
Me too. I've missed that there is already a timeout constant
IFC_TIMEOUT_MSECS (500). As it's huge, I'll add a second one.
>
> > + if (ret)
> > + dev_err(priv->dev, "Failed to initialize SRAM!\n");
>
> Space
okay.
Thanks,
Kurt
>
> > + return ret;
> > + }
> > +
> > /* Save CSOR and CSOR_ext */
> > csor = ifc_in32(_global->csor_cs[cs].csor);
> > csor_ext = ifc_in32(_global->csor_cs[cs].csor_ext);
> > diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
> > index 3fdfede2f0f3..5f343b796ad9 100644
> > --- a/include/linux/fsl_ifc.h
> > +++ b/include/linux/fsl_ifc.h
> > @@ -274,6 +274,8 @@
> > */
> > /* Auto Boot Mode */
> > #define IFC_NAND_NCFGR_BOOT0x8000
> > +/* SRAM Initialization */
> > +#define IFC_NAND_NCFGR_SRAM_INIT_EN0x2000
> > /* Addressing Mode-ROW0+n/COL0 */
> > #define IFC_NAND_NCFGR_ADDR_MODE_RC0 0x
> > /* Addressing Mode-ROW0+n/COL0+n */
>
>
> Thanks,
> Miquèl
[PATCH 1/2] mtd: nand: fsl-ifc: check result of SRAM initialization
The SRAM initialization might fail. If that happens further NAND operations
won't be successful. Therefore, the chip init routine should fail if the SRAM
initialization didn't work.
Signed-off-by: Kurt Kanzenbach
---
drivers/mtd/nand/raw/fsl_ifc_nand.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c
b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index 24f59d0066af..e4f5792dc589 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -761,7 +761,7 @@ static const struct nand_controller_ops
fsl_ifc_controller_ops = {
.attach_chip = fsl_ifc_attach_chip,
};
-static void fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
+static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
{
struct fsl_ifc_ctrl *ctrl = priv->ctrl;
struct fsl_ifc_runtime __iomem *ifc_runtime = ctrl->rregs;
@@ -805,12 +805,16 @@ static void fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
wait_event_timeout(ctrl->nand_wait, ctrl->nand_stat,
msecs_to_jiffies(IFC_TIMEOUT_MSECS));
- if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC)
+ if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC) {
pr_err("fsl-ifc: Failed to Initialise SRAM\n");
+ return -ETIMEDOUT;
+ }
/* Restore CSOR and CSOR_ext */
ifc_out32(csor, _global->csor_cs[cs].csor);
ifc_out32(csor_ext, _global->csor_cs[cs].csor_ext);
+
+ return 0;
}
static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
@@ -914,8 +918,13 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
chip->ecc.algo = NAND_ECC_HAMMING;
}
- if (ctrl->version >= FSL_IFC_VERSION_1_1_0)
- fsl_ifc_sram_init(priv);
+ if (ctrl->version >= FSL_IFC_VERSION_1_1_0) {
+ int ret;
+
+ ret = fsl_ifc_sram_init(priv);
+ if (ret)
+ return ret;
+ }
/*
* As IFC version 2.0.0 has 16KB of internal SRAM as compared to older
--
2.11.0
[PATCH 0/2] mtd: nand: fsl-ifc: fix SRAM initialization for newer controller
Hi, the current way of initializing the internal SRAM of the IFC controller only works for older controller versions. Newer versions require a different method. So, adding support for it. Thereby, the result of the SRAM initialization should be checked. If it's not successful, further commands such as read won't work. Tested on hardware. Thanks, Kurt Kurt Kanzenbach (2): mtd: nand: fsl-ifc: check result of SRAM initialization mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions drivers/mtd/nand/raw/fsl_ifc_nand.c | 35 +++ include/linux/fsl_ifc.h | 2 ++ 2 files changed, 33 insertions(+), 4 deletions(-) -- 2.11.0
[PATCH 1/2] mtd: nand: fsl-ifc: check result of SRAM initialization
The SRAM initialization might fail. If that happens further NAND operations
won't be successful. Therefore, the chip init routine should fail if the SRAM
initialization didn't work.
Signed-off-by: Kurt Kanzenbach
---
drivers/mtd/nand/raw/fsl_ifc_nand.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c
b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index 24f59d0066af..e4f5792dc589 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -761,7 +761,7 @@ static const struct nand_controller_ops
fsl_ifc_controller_ops = {
.attach_chip = fsl_ifc_attach_chip,
};
-static void fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
+static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
{
struct fsl_ifc_ctrl *ctrl = priv->ctrl;
struct fsl_ifc_runtime __iomem *ifc_runtime = ctrl->rregs;
@@ -805,12 +805,16 @@ static void fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
wait_event_timeout(ctrl->nand_wait, ctrl->nand_stat,
msecs_to_jiffies(IFC_TIMEOUT_MSECS));
- if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC)
+ if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC) {
pr_err("fsl-ifc: Failed to Initialise SRAM\n");
+ return -ETIMEDOUT;
+ }
/* Restore CSOR and CSOR_ext */
ifc_out32(csor, _global->csor_cs[cs].csor);
ifc_out32(csor_ext, _global->csor_cs[cs].csor_ext);
+
+ return 0;
}
static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
@@ -914,8 +918,13 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
chip->ecc.algo = NAND_ECC_HAMMING;
}
- if (ctrl->version >= FSL_IFC_VERSION_1_1_0)
- fsl_ifc_sram_init(priv);
+ if (ctrl->version >= FSL_IFC_VERSION_1_1_0) {
+ int ret;
+
+ ret = fsl_ifc_sram_init(priv);
+ if (ret)
+ return ret;
+ }
/*
* As IFC version 2.0.0 has 16KB of internal SRAM as compared to older
--
2.11.0
[PATCH 0/2] mtd: nand: fsl-ifc: fix SRAM initialization for newer controller
Hi, the current way of initializing the internal SRAM of the IFC controller only works for older controller versions. Newer versions require a different method. So, adding support for it. Thereby, the result of the SRAM initialization should be checked. If it's not successful, further commands such as read won't work. Tested on hardware. Thanks, Kurt Kurt Kanzenbach (2): mtd: nand: fsl-ifc: check result of SRAM initialization mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions drivers/mtd/nand/raw/fsl_ifc_nand.c | 35 +++ include/linux/fsl_ifc.h | 2 ++ 2 files changed, 33 insertions(+), 4 deletions(-) -- 2.11.0
[PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions
Newer versions of the IFC controller use a different method of initializing the
internal SRAM: Instead of reading from flash, a bit in the NAND configuration
register has to be set in order to trigger the self-initializing process.
Signed-off-by: Kurt Kanzenbach
---
drivers/mtd/nand/raw/fsl_ifc_nand.c | 18 ++
include/linux/fsl_ifc.h | 2 ++
2 files changed, 20 insertions(+)
diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c
b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index e4f5792dc589..384d5e12b05c 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#define ERR_BYTE 0xFF /* Value returned for read
bytes when read failed */
@@ -769,6 +770,23 @@ static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
uint32_t csor = 0, csor_8k = 0, csor_ext = 0;
uint32_t cs = priv->bank;
+ if (ctrl->version > FSL_IFC_VERSION_1_1_0) {
+ u32 ncfgr, status;
+ int ret;
+
+ /* Trigger auto initialization */
+ ncfgr = ifc_in32(_runtime->ifc_nand.ncfgr);
+ ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN,
_runtime->ifc_nand.ncfgr);
+
+ /* Wait until done */
+ ret = readx_poll_timeout(ifc_in32, _runtime->ifc_nand.ncfgr,
+status, !(status &
IFC_NAND_NCFGR_SRAM_INIT_EN),
+10, 1000);
+ if (ret)
+ dev_err(priv->dev, "Failed to initialize SRAM!\n");
+ return ret;
+ }
+
/* Save CSOR and CSOR_ext */
csor = ifc_in32(_global->csor_cs[cs].csor);
csor_ext = ifc_in32(_global->csor_cs[cs].csor_ext);
diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
index 3fdfede2f0f3..5f343b796ad9 100644
--- a/include/linux/fsl_ifc.h
+++ b/include/linux/fsl_ifc.h
@@ -274,6 +274,8 @@
*/
/* Auto Boot Mode */
#define IFC_NAND_NCFGR_BOOT0x8000
+/* SRAM Initialization */
+#define IFC_NAND_NCFGR_SRAM_INIT_EN0x2000
/* Addressing Mode-ROW0+n/COL0 */
#define IFC_NAND_NCFGR_ADDR_MODE_RC0 0x
/* Addressing Mode-ROW0+n/COL0+n */
--
2.11.0
[PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions
Newer versions of the IFC controller use a different method of initializing the
internal SRAM: Instead of reading from flash, a bit in the NAND configuration
register has to be set in order to trigger the self-initializing process.
Signed-off-by: Kurt Kanzenbach
---
drivers/mtd/nand/raw/fsl_ifc_nand.c | 18 ++
include/linux/fsl_ifc.h | 2 ++
2 files changed, 20 insertions(+)
diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c
b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index e4f5792dc589..384d5e12b05c 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#define ERR_BYTE 0xFF /* Value returned for read
bytes when read failed */
@@ -769,6 +770,23 @@ static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
uint32_t csor = 0, csor_8k = 0, csor_ext = 0;
uint32_t cs = priv->bank;
+ if (ctrl->version > FSL_IFC_VERSION_1_1_0) {
+ u32 ncfgr, status;
+ int ret;
+
+ /* Trigger auto initialization */
+ ncfgr = ifc_in32(_runtime->ifc_nand.ncfgr);
+ ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN,
_runtime->ifc_nand.ncfgr);
+
+ /* Wait until done */
+ ret = readx_poll_timeout(ifc_in32, _runtime->ifc_nand.ncfgr,
+status, !(status &
IFC_NAND_NCFGR_SRAM_INIT_EN),
+10, 1000);
+ if (ret)
+ dev_err(priv->dev, "Failed to initialize SRAM!\n");
+ return ret;
+ }
+
/* Save CSOR and CSOR_ext */
csor = ifc_in32(_global->csor_cs[cs].csor);
csor_ext = ifc_in32(_global->csor_cs[cs].csor_ext);
diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
index 3fdfede2f0f3..5f343b796ad9 100644
--- a/include/linux/fsl_ifc.h
+++ b/include/linux/fsl_ifc.h
@@ -274,6 +274,8 @@
*/
/* Auto Boot Mode */
#define IFC_NAND_NCFGR_BOOT0x8000
+/* SRAM Initialization */
+#define IFC_NAND_NCFGR_SRAM_INIT_EN0x2000
/* Addressing Mode-ROW0+n/COL0 */
#define IFC_NAND_NCFGR_ADDR_MODE_RC0 0x
/* Addressing Mode-ROW0+n/COL0+n */
--
2.11.0
Re: [PATCH 1/1] mmc: sdhci-pci: fix eMMC controller issue on Intel Baytrail SoCs
> On 06/20/2018 04:15 PM, Kurt Kanzenbach wrote: > > Hi, > > > > thanks for your response. > > > > On Tue, Jun 19, 2018 at 10:03:01AM +0300, Adrian Hunter wrote: > >> On 19/06/18 09:31, Kurt Kanzenbach wrote: > >>> Sometimes the eMMC controller doesn't respond anymore on Intel Baytrail > >>> SoCs. The resulting error looks like: > >>> > >>> |mmc1: Reset 0x1 never completed. > >>> |sdhci: === REGISTER DUMP (mmc1)=== > >>> |sdhci: Sys addr: 0x | Version: 0x > >>> |sdhci: Blk size: 0x | Blk cnt: 0x > >>> |sdhci: Argument: 0x | Trn mode: 0x > >>> |sdhci: Present: 0x | Host ctl: 0x00ff > >>> |sdhci: Power:0x00ff | Blk gap: 0x00ff > >>> |sdhci: Wake-up: 0x00ff | Clock:0x > >>> |sdhci: Timeout: 0x00ff | Int stat: 0x > >>> |sdhci: Int enab: 0x | Sig enab: 0x > >>> |sdhci: AC12 err: 0x | Slot int: 0x > >>> |sdhci: Caps: 0x | Caps_1: 0x > >>> |sdhci: Cmd: 0x | Max curr: 0x > >>> |sdhci: Host ctl2: 0x > >>> |sdhci: ADMA Err: 0x | ADMA Ptr: 0x > >>> > >>> The behavior was observed on an Intel Atom E3825 performing lots of > >>> reboots. The > >> > >> So you are saying this only happens at boot time? And only when > >> re-booting? > > > > well, exactly. This issue was only observed when rebooting, not on cold > > boots. > > > >> Can you send all the kernel messages? Can you send an acpidump? > > > > The kernel log is straightforward. The system is booting and starting a > > few applications. Afterwards the issue happens. The rootfilesystem is > > located on the eMMC. > > The full messages can be more revealing such as showing what else was > happening and the order of events, so I would still like to see them. > > > > > The error message above is from the Linux v4.9 boot log. > > > > On v4.17 the same issue happens, but the error messages are different: > > > > |mmc1: Timeout waiting for hardware interrupt. > > |mmc1: sdhci: SDHCI REGISTER DUMP === > > |mmc1: sdhci: Sys addr: 0x0002 | Version: 0x1002 > > |mmc1: sdhci: Blk size: 0x7200 | Blk cnt: 0x > > |mmc1: sdhci: Argument: 0x00040fd4 | Trn mode: 0x003b > > |mmc1: sdhci: Present: 0x1fff | Host ctl: 0x0035 > > |mmc1: sdhci: Power: 0x000b | Blk gap: 0x0080 > > |mmc1: sdhci: Wake-up: 0x | Clock:0x0207 > > |mmc1: sdhci: Timeout: 0x | Int stat: 0x0003 > > |mmc1: sdhci: Int enab: 0x02ff000b | Sig enab: 0x02ff000b > > |mmc1: sdhci: AC12 err: 0x | Slot int: 0x0001 > > |mmc1: sdhci: Caps: 0x446cc801 | Caps_1: 0x0005 > > |mmc1: sdhci: Cmd: 0x123a | Max curr: 0x > > |mmc1: sdhci: Resp[0]: 0x0900 | Resp[1]: 0x > > |mmc1: sdhci: Resp[2]: 0x320f5913 | Resp[3]: 0x0900 > > |mmc1: sdhci: Host ctl2: 0x000c > > |mmc1: sdhci: ADMA Err: 0x | ADMA Ptr: 0x34ee5208 > > |mmc1: sdhci: > > |[...] > > Those messages show that the interrupt did happen but the driver did not see > it. Are you doing anything unusual like using threadirqs? No, I'm not doing anything unusual. The mmc core uses threaded irqs by default. But, most of the work is performed in the primary handler. So, that shouldn't be a problem. But in the v4.9 case, we use preempt rt. I took a few scheduler traces in order to see if there might be any task blocking or preempting the mmc irqs. However, that's not the case. The common pattern is: mmc1 is suspended, afterwards some applications use mmc0 and finally a different application accesses mmc1. The suspend function is called and during initialization the reset doesn't work anymore. Anyway, I'll perform more tests. Thanks, Kurt > > > > > Both issues disappear when disabling runtime pm. > > > > Anyway I'll prepare an acpidump for you. > > > >> > >>> issue seems to occur if runtime power management is used. Found by > >>> utilizing > >>> ftrace. > >>> > >>> The erratum VLI10 for the Intel E3825 states, that the eMMC controller > >>> incorrectly announces that it supports suspend/resume. However, that > >>> shouldn't > >>> be used, as the controller may incorr
Re: [PATCH 1/1] mmc: sdhci-pci: fix eMMC controller issue on Intel Baytrail SoCs
> On 06/20/2018 04:15 PM, Kurt Kanzenbach wrote: > > Hi, > > > > thanks for your response. > > > > On Tue, Jun 19, 2018 at 10:03:01AM +0300, Adrian Hunter wrote: > >> On 19/06/18 09:31, Kurt Kanzenbach wrote: > >>> Sometimes the eMMC controller doesn't respond anymore on Intel Baytrail > >>> SoCs. The resulting error looks like: > >>> > >>> |mmc1: Reset 0x1 never completed. > >>> |sdhci: === REGISTER DUMP (mmc1)=== > >>> |sdhci: Sys addr: 0x | Version: 0x > >>> |sdhci: Blk size: 0x | Blk cnt: 0x > >>> |sdhci: Argument: 0x | Trn mode: 0x > >>> |sdhci: Present: 0x | Host ctl: 0x00ff > >>> |sdhci: Power:0x00ff | Blk gap: 0x00ff > >>> |sdhci: Wake-up: 0x00ff | Clock:0x > >>> |sdhci: Timeout: 0x00ff | Int stat: 0x > >>> |sdhci: Int enab: 0x | Sig enab: 0x > >>> |sdhci: AC12 err: 0x | Slot int: 0x > >>> |sdhci: Caps: 0x | Caps_1: 0x > >>> |sdhci: Cmd: 0x | Max curr: 0x > >>> |sdhci: Host ctl2: 0x > >>> |sdhci: ADMA Err: 0x | ADMA Ptr: 0x > >>> > >>> The behavior was observed on an Intel Atom E3825 performing lots of > >>> reboots. The > >> > >> So you are saying this only happens at boot time? And only when > >> re-booting? > > > > well, exactly. This issue was only observed when rebooting, not on cold > > boots. > > > >> Can you send all the kernel messages? Can you send an acpidump? > > > > The kernel log is straightforward. The system is booting and starting a > > few applications. Afterwards the issue happens. The rootfilesystem is > > located on the eMMC. > > The full messages can be more revealing such as showing what else was > happening and the order of events, so I would still like to see them. > > > > > The error message above is from the Linux v4.9 boot log. > > > > On v4.17 the same issue happens, but the error messages are different: > > > > |mmc1: Timeout waiting for hardware interrupt. > > |mmc1: sdhci: SDHCI REGISTER DUMP === > > |mmc1: sdhci: Sys addr: 0x0002 | Version: 0x1002 > > |mmc1: sdhci: Blk size: 0x7200 | Blk cnt: 0x > > |mmc1: sdhci: Argument: 0x00040fd4 | Trn mode: 0x003b > > |mmc1: sdhci: Present: 0x1fff | Host ctl: 0x0035 > > |mmc1: sdhci: Power: 0x000b | Blk gap: 0x0080 > > |mmc1: sdhci: Wake-up: 0x | Clock:0x0207 > > |mmc1: sdhci: Timeout: 0x | Int stat: 0x0003 > > |mmc1: sdhci: Int enab: 0x02ff000b | Sig enab: 0x02ff000b > > |mmc1: sdhci: AC12 err: 0x | Slot int: 0x0001 > > |mmc1: sdhci: Caps: 0x446cc801 | Caps_1: 0x0005 > > |mmc1: sdhci: Cmd: 0x123a | Max curr: 0x > > |mmc1: sdhci: Resp[0]: 0x0900 | Resp[1]: 0x > > |mmc1: sdhci: Resp[2]: 0x320f5913 | Resp[3]: 0x0900 > > |mmc1: sdhci: Host ctl2: 0x000c > > |mmc1: sdhci: ADMA Err: 0x | ADMA Ptr: 0x34ee5208 > > |mmc1: sdhci: > > |[...] > > Those messages show that the interrupt did happen but the driver did not see > it. Are you doing anything unusual like using threadirqs? No, I'm not doing anything unusual. The mmc core uses threaded irqs by default. But, most of the work is performed in the primary handler. So, that shouldn't be a problem. But in the v4.9 case, we use preempt rt. I took a few scheduler traces in order to see if there might be any task blocking or preempting the mmc irqs. However, that's not the case. The common pattern is: mmc1 is suspended, afterwards some applications use mmc0 and finally a different application accesses mmc1. The suspend function is called and during initialization the reset doesn't work anymore. Anyway, I'll perform more tests. Thanks, Kurt > > > > > Both issues disappear when disabling runtime pm. > > > > Anyway I'll prepare an acpidump for you. > > > >> > >>> issue seems to occur if runtime power management is used. Found by > >>> utilizing > >>> ftrace. > >>> > >>> The erratum VLI10 for the Intel E3825 states, that the eMMC controller > >>> incorrectly announces that it supports suspend/resume. However, that > >>> shouldn't > >>> be used, as the controller may incorr
Re: [PATCH 1/1] mmc: sdhci-pci: fix eMMC controller issue on Intel Baytrail SoCs
Hi,
thanks for your response.
On Tue, Jun 19, 2018 at 10:03:01AM +0300, Adrian Hunter wrote:
> On 19/06/18 09:31, Kurt Kanzenbach wrote:
> > Sometimes the eMMC controller doesn't respond anymore on Intel Baytrail
> > SoCs. The resulting error looks like:
> >
> > |mmc1: Reset 0x1 never completed.
> > |sdhci: === REGISTER DUMP (mmc1)===
> > |sdhci: Sys addr: 0x | Version: 0x
> > |sdhci: Blk size: 0x | Blk cnt: 0x
> > |sdhci: Argument: 0x | Trn mode: 0x
> > |sdhci: Present: 0x | Host ctl: 0x00ff
> > |sdhci: Power:0x00ff | Blk gap: 0x00ff
> > |sdhci: Wake-up: 0x00ff | Clock:0x
> > |sdhci: Timeout: 0x00ff | Int stat: 0x
> > |sdhci: Int enab: 0x | Sig enab: 0x
> > |sdhci: AC12 err: 0x | Slot int: 0x
> > |sdhci: Caps: 0x | Caps_1: 0x
> > |sdhci: Cmd: 0x | Max curr: 0x
> > |sdhci: Host ctl2: 0x
> > |sdhci: ADMA Err: 0x | ADMA Ptr: 0x
> >
> > The behavior was observed on an Intel Atom E3825 performing lots of
> > reboots. The
>
> So you are saying this only happens at boot time? And only when
> re-booting?
well, exactly. This issue was only observed when rebooting, not on cold
boots.
> Can you send all the kernel messages? Can you send an acpidump?
The kernel log is straightforward. The system is booting and starting a
few applications. Afterwards the issue happens. The rootfilesystem is
located on the eMMC.
The error message above is from the Linux v4.9 boot log.
On v4.17 the same issue happens, but the error messages are different:
|mmc1: Timeout waiting for hardware interrupt.
|mmc1: sdhci: SDHCI REGISTER DUMP ===
|mmc1: sdhci: Sys addr: 0x0002 | Version: 0x1002
|mmc1: sdhci: Blk size: 0x7200 | Blk cnt: 0x
|mmc1: sdhci: Argument: 0x00040fd4 | Trn mode: 0x003b
|mmc1: sdhci: Present: 0x1fff | Host ctl: 0x0035
|mmc1: sdhci: Power: 0x000b | Blk gap: 0x0080
|mmc1: sdhci: Wake-up: 0x | Clock:0x0207
|mmc1: sdhci: Timeout: 0x | Int stat: 0x0003
|mmc1: sdhci: Int enab: 0x02ff000b | Sig enab: 0x02ff000b
|mmc1: sdhci: AC12 err: 0x | Slot int: 0x0001
|mmc1: sdhci: Caps: 0x446cc801 | Caps_1: 0x0005
|mmc1: sdhci: Cmd: 0x123a | Max curr: 0x
|mmc1: sdhci: Resp[0]: 0x0900 | Resp[1]: 0x
|mmc1: sdhci: Resp[2]: 0x320f5913 | Resp[3]: 0x0900
|mmc1: sdhci: Host ctl2: 0x000c
|mmc1: sdhci: ADMA Err: 0x | ADMA Ptr: 0x34ee5208
|mmc1: sdhci:
|[...]
Both issues disappear when disabling runtime pm.
Anyway I'll prepare an acpidump for you.
>
> > issue seems to occur if runtime power management is used. Found by utilizing
> > ftrace.
> >
> > The erratum VLI10 for the Intel E3825 states, that the eMMC controller
> > incorrectly announces that it supports suspend/resume. However, that
> > shouldn't
> > be used, as the controller may incorrectly transfer data between memory and
> > the
> > SD device.
>
> That erratum is not related to this problem. The suspend/resume that is
> documented is an internal SDHCI feature, not the kernel's suspend/resume.
> The SDHCI Suspend/Resume Mechanism is not supported in the driver, so it is
> not being used anyway.
Thanks for the clarification.
Do you have any idea why this issue might happen?
Thanks, Kurt
>
> >
> > Therefore, disallowing runtime pm resolves the issue. Tested on the E3825.
> >
> > Signed-off-by: Kurt Kanzenbach
> > ---
> > drivers/mmc/host/sdhci-pci-core.c | 17 -
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-pci-core.c
> > b/drivers/mmc/host/sdhci-pci-core.c
> > index 77dd3521daae..df89381944cd 100644
> > --- a/drivers/mmc/host/sdhci-pci-core.c
> > +++ b/drivers/mmc/host/sdhci-pci-core.c
> > @@ -870,6 +870,21 @@ static const struct sdhci_pci_fixes
> > sdhci_intel_byt_emmc = {
> > .priv_size = sizeof(struct intel_host),
> > };
> >
> > +/*
> > + * See Erratum VLI10 from Errata List for Intel Atom E3825, Link:
> > + *
> > https://www.intel.ca/content/dam/www/public/us/en/documents/specification-updates/atom-e3800-family-spec-update.pdf
> > + */
> > +static const struct sdhci_pci_fixes sdhci_intel_byt_emmc_no_runtime_pm = {
> > + .allow_runtime_pm = false,
> > + .probe_slot = byt_emmc_probe_slot,
> > + .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
> > +
Re: [PATCH 1/1] mmc: sdhci-pci: fix eMMC controller issue on Intel Baytrail SoCs
Hi,
thanks for your response.
On Tue, Jun 19, 2018 at 10:03:01AM +0300, Adrian Hunter wrote:
> On 19/06/18 09:31, Kurt Kanzenbach wrote:
> > Sometimes the eMMC controller doesn't respond anymore on Intel Baytrail
> > SoCs. The resulting error looks like:
> >
> > |mmc1: Reset 0x1 never completed.
> > |sdhci: === REGISTER DUMP (mmc1)===
> > |sdhci: Sys addr: 0x | Version: 0x
> > |sdhci: Blk size: 0x | Blk cnt: 0x
> > |sdhci: Argument: 0x | Trn mode: 0x
> > |sdhci: Present: 0x | Host ctl: 0x00ff
> > |sdhci: Power:0x00ff | Blk gap: 0x00ff
> > |sdhci: Wake-up: 0x00ff | Clock:0x
> > |sdhci: Timeout: 0x00ff | Int stat: 0x
> > |sdhci: Int enab: 0x | Sig enab: 0x
> > |sdhci: AC12 err: 0x | Slot int: 0x
> > |sdhci: Caps: 0x | Caps_1: 0x
> > |sdhci: Cmd: 0x | Max curr: 0x
> > |sdhci: Host ctl2: 0x
> > |sdhci: ADMA Err: 0x | ADMA Ptr: 0x
> >
> > The behavior was observed on an Intel Atom E3825 performing lots of
> > reboots. The
>
> So you are saying this only happens at boot time? And only when
> re-booting?
well, exactly. This issue was only observed when rebooting, not on cold
boots.
> Can you send all the kernel messages? Can you send an acpidump?
The kernel log is straightforward. The system is booting and starting a
few applications. Afterwards the issue happens. The rootfilesystem is
located on the eMMC.
The error message above is from the Linux v4.9 boot log.
On v4.17 the same issue happens, but the error messages are different:
|mmc1: Timeout waiting for hardware interrupt.
|mmc1: sdhci: SDHCI REGISTER DUMP ===
|mmc1: sdhci: Sys addr: 0x0002 | Version: 0x1002
|mmc1: sdhci: Blk size: 0x7200 | Blk cnt: 0x
|mmc1: sdhci: Argument: 0x00040fd4 | Trn mode: 0x003b
|mmc1: sdhci: Present: 0x1fff | Host ctl: 0x0035
|mmc1: sdhci: Power: 0x000b | Blk gap: 0x0080
|mmc1: sdhci: Wake-up: 0x | Clock:0x0207
|mmc1: sdhci: Timeout: 0x | Int stat: 0x0003
|mmc1: sdhci: Int enab: 0x02ff000b | Sig enab: 0x02ff000b
|mmc1: sdhci: AC12 err: 0x | Slot int: 0x0001
|mmc1: sdhci: Caps: 0x446cc801 | Caps_1: 0x0005
|mmc1: sdhci: Cmd: 0x123a | Max curr: 0x
|mmc1: sdhci: Resp[0]: 0x0900 | Resp[1]: 0x
|mmc1: sdhci: Resp[2]: 0x320f5913 | Resp[3]: 0x0900
|mmc1: sdhci: Host ctl2: 0x000c
|mmc1: sdhci: ADMA Err: 0x | ADMA Ptr: 0x34ee5208
|mmc1: sdhci:
|[...]
Both issues disappear when disabling runtime pm.
Anyway I'll prepare an acpidump for you.
>
> > issue seems to occur if runtime power management is used. Found by utilizing
> > ftrace.
> >
> > The erratum VLI10 for the Intel E3825 states, that the eMMC controller
> > incorrectly announces that it supports suspend/resume. However, that
> > shouldn't
> > be used, as the controller may incorrectly transfer data between memory and
> > the
> > SD device.
>
> That erratum is not related to this problem. The suspend/resume that is
> documented is an internal SDHCI feature, not the kernel's suspend/resume.
> The SDHCI Suspend/Resume Mechanism is not supported in the driver, so it is
> not being used anyway.
Thanks for the clarification.
Do you have any idea why this issue might happen?
Thanks, Kurt
>
> >
> > Therefore, disallowing runtime pm resolves the issue. Tested on the E3825.
> >
> > Signed-off-by: Kurt Kanzenbach
> > ---
> > drivers/mmc/host/sdhci-pci-core.c | 17 -
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-pci-core.c
> > b/drivers/mmc/host/sdhci-pci-core.c
> > index 77dd3521daae..df89381944cd 100644
> > --- a/drivers/mmc/host/sdhci-pci-core.c
> > +++ b/drivers/mmc/host/sdhci-pci-core.c
> > @@ -870,6 +870,21 @@ static const struct sdhci_pci_fixes
> > sdhci_intel_byt_emmc = {
> > .priv_size = sizeof(struct intel_host),
> > };
> >
> > +/*
> > + * See Erratum VLI10 from Errata List for Intel Atom E3825, Link:
> > + *
> > https://www.intel.ca/content/dam/www/public/us/en/documents/specification-updates/atom-e3800-family-spec-update.pdf
> > + */
> > +static const struct sdhci_pci_fixes sdhci_intel_byt_emmc_no_runtime_pm = {
> > + .allow_runtime_pm = false,
> > + .probe_slot = byt_emmc_probe_slot,
> > + .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
> > +
[PATCH 0/1] eMMC controller issue on Intel Baytrail SoC
Hi, I've encountered a problem on an Intel Atom E3825. When performing lots of reboots (10, 50, 100, ...) the eMMC controller stops working. The reset commands won't work anymore and you get error messages such as: |mmc1: Reset 0x1 never completed. |sdhci: === REGISTER DUMP (mmc1)=== |sdhci: Sys addr: 0x | Version: 0x |sdhci: Blk size: 0x | Blk cnt: 0x |sdhci: Argument: 0x | Trn mode: 0x |sdhci: Present: 0x | Host ctl: 0x00ff |sdhci: Power:0x00ff | Blk gap: 0x00ff |sdhci: Wake-up: 0x00ff | Clock:0x |sdhci: Timeout: 0x00ff | Int stat: 0x |sdhci: Int enab: 0x | Sig enab: 0x |sdhci: AC12 err: 0x | Slot int: 0x |sdhci: Caps: 0x | Caps_1: 0x |sdhci: Cmd: 0x | Max curr: 0x |sdhci: Host ctl2: 0x |sdhci: ADMA Err: 0x | ADMA Ptr: 0x After using ftrace, I've discovered that this issue happens when runtime power management is utilized. So after searching a bit, I've found the errata list for the E3825: https://www.intel.ca/content/dam/www/public/us/en/documents/specification-updates/atom-e3800-family-spec-update.pdf Erratum VLI10 basically states, that suspend/resume shouldn't be used. Otherwise wrong data between memory the device may be transferred. Therefore, I've disabled runtime power management and the issue disappeared. That's what the following patch does. This patch is tested against v4.17 and v4.9. Any suggestions? Kurt Kanzenbach (1): mmc: sdhci-pci: fix eMMC controller issue on Intel Baytrail SoCs drivers/mmc/host/sdhci-pci-core.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) -- 2.11.0
[PATCH 1/1] mmc: sdhci-pci: fix eMMC controller issue on Intel Baytrail SoCs
Sometimes the eMMC controller doesn't respond anymore on Intel Baytrail
SoCs. The resulting error looks like:
|mmc1: Reset 0x1 never completed.
|sdhci: === REGISTER DUMP (mmc1)===
|sdhci: Sys addr: 0x | Version: 0x
|sdhci: Blk size: 0x | Blk cnt: 0x
|sdhci: Argument: 0x | Trn mode: 0x
|sdhci: Present: 0x | Host ctl: 0x00ff
|sdhci: Power:0x00ff | Blk gap: 0x00ff
|sdhci: Wake-up: 0x00ff | Clock:0x
|sdhci: Timeout: 0x00ff | Int stat: 0x
|sdhci: Int enab: 0x | Sig enab: 0x
|sdhci: AC12 err: 0x | Slot int: 0x
|sdhci: Caps: 0x | Caps_1: 0x
|sdhci: Cmd: 0x | Max curr: 0x
|sdhci: Host ctl2: 0x
|sdhci: ADMA Err: 0x | ADMA Ptr: 0x
The behavior was observed on an Intel Atom E3825 performing lots of reboots. The
issue seems to occur if runtime power management is used. Found by utilizing
ftrace.
The erratum VLI10 for the Intel E3825 states, that the eMMC controller
incorrectly announces that it supports suspend/resume. However, that shouldn't
be used, as the controller may incorrectly transfer data between memory and the
SD device.
Therefore, disallowing runtime pm resolves the issue. Tested on the E3825.
Signed-off-by: Kurt Kanzenbach
---
drivers/mmc/host/sdhci-pci-core.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-pci-core.c
b/drivers/mmc/host/sdhci-pci-core.c
index 77dd3521daae..df89381944cd 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -870,6 +870,21 @@ static const struct sdhci_pci_fixes sdhci_intel_byt_emmc =
{
.priv_size = sizeof(struct intel_host),
};
+/*
+ * See Erratum VLI10 from Errata List for Intel Atom E3825, Link:
+ *
https://www.intel.ca/content/dam/www/public/us/en/documents/specification-updates/atom-e3800-family-spec-update.pdf
+ */
+static const struct sdhci_pci_fixes sdhci_intel_byt_emmc_no_runtime_pm = {
+ .allow_runtime_pm = false,
+ .probe_slot = byt_emmc_probe_slot,
+ .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
+ .quirks2= SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+ SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400 |
+ SDHCI_QUIRK2_STOP_WITH_TC,
+ .ops= _intel_byt_ops,
+ .priv_size = sizeof(struct intel_host),
+};
+
static const struct sdhci_pci_fixes sdhci_intel_glk_emmc = {
.allow_runtime_pm = true,
.probe_slot = glk_emmc_probe_slot,
@@ -1470,7 +1485,7 @@ static const struct pci_device_id pci_ids[] = {
SDHCI_PCI_SUBDEVICE(INTEL, BYT_SDIO, NI, 7884, ni_byt_sdio),
SDHCI_PCI_DEVICE(INTEL, BYT_SDIO, intel_byt_sdio),
SDHCI_PCI_DEVICE(INTEL, BYT_SD,intel_byt_sd),
- SDHCI_PCI_DEVICE(INTEL, BYT_EMMC2, intel_byt_emmc),
+ SDHCI_PCI_DEVICE(INTEL, BYT_EMMC2, intel_byt_emmc_no_runtime_pm),
SDHCI_PCI_DEVICE(INTEL, BSW_EMMC, intel_byt_emmc),
SDHCI_PCI_DEVICE(INTEL, BSW_SDIO, intel_byt_sdio),
SDHCI_PCI_DEVICE(INTEL, BSW_SD,intel_byt_sd),
--
2.11.0
[PATCH 0/1] eMMC controller issue on Intel Baytrail SoC
Hi, I've encountered a problem on an Intel Atom E3825. When performing lots of reboots (10, 50, 100, ...) the eMMC controller stops working. The reset commands won't work anymore and you get error messages such as: |mmc1: Reset 0x1 never completed. |sdhci: === REGISTER DUMP (mmc1)=== |sdhci: Sys addr: 0x | Version: 0x |sdhci: Blk size: 0x | Blk cnt: 0x |sdhci: Argument: 0x | Trn mode: 0x |sdhci: Present: 0x | Host ctl: 0x00ff |sdhci: Power:0x00ff | Blk gap: 0x00ff |sdhci: Wake-up: 0x00ff | Clock:0x |sdhci: Timeout: 0x00ff | Int stat: 0x |sdhci: Int enab: 0x | Sig enab: 0x |sdhci: AC12 err: 0x | Slot int: 0x |sdhci: Caps: 0x | Caps_1: 0x |sdhci: Cmd: 0x | Max curr: 0x |sdhci: Host ctl2: 0x |sdhci: ADMA Err: 0x | ADMA Ptr: 0x After using ftrace, I've discovered that this issue happens when runtime power management is utilized. So after searching a bit, I've found the errata list for the E3825: https://www.intel.ca/content/dam/www/public/us/en/documents/specification-updates/atom-e3800-family-spec-update.pdf Erratum VLI10 basically states, that suspend/resume shouldn't be used. Otherwise wrong data between memory the device may be transferred. Therefore, I've disabled runtime power management and the issue disappeared. That's what the following patch does. This patch is tested against v4.17 and v4.9. Any suggestions? Kurt Kanzenbach (1): mmc: sdhci-pci: fix eMMC controller issue on Intel Baytrail SoCs drivers/mmc/host/sdhci-pci-core.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) -- 2.11.0
[PATCH 1/1] mmc: sdhci-pci: fix eMMC controller issue on Intel Baytrail SoCs
Sometimes the eMMC controller doesn't respond anymore on Intel Baytrail
SoCs. The resulting error looks like:
|mmc1: Reset 0x1 never completed.
|sdhci: === REGISTER DUMP (mmc1)===
|sdhci: Sys addr: 0x | Version: 0x
|sdhci: Blk size: 0x | Blk cnt: 0x
|sdhci: Argument: 0x | Trn mode: 0x
|sdhci: Present: 0x | Host ctl: 0x00ff
|sdhci: Power:0x00ff | Blk gap: 0x00ff
|sdhci: Wake-up: 0x00ff | Clock:0x
|sdhci: Timeout: 0x00ff | Int stat: 0x
|sdhci: Int enab: 0x | Sig enab: 0x
|sdhci: AC12 err: 0x | Slot int: 0x
|sdhci: Caps: 0x | Caps_1: 0x
|sdhci: Cmd: 0x | Max curr: 0x
|sdhci: Host ctl2: 0x
|sdhci: ADMA Err: 0x | ADMA Ptr: 0x
The behavior was observed on an Intel Atom E3825 performing lots of reboots. The
issue seems to occur if runtime power management is used. Found by utilizing
ftrace.
The erratum VLI10 for the Intel E3825 states, that the eMMC controller
incorrectly announces that it supports suspend/resume. However, that shouldn't
be used, as the controller may incorrectly transfer data between memory and the
SD device.
Therefore, disallowing runtime pm resolves the issue. Tested on the E3825.
Signed-off-by: Kurt Kanzenbach
---
drivers/mmc/host/sdhci-pci-core.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-pci-core.c
b/drivers/mmc/host/sdhci-pci-core.c
index 77dd3521daae..df89381944cd 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -870,6 +870,21 @@ static const struct sdhci_pci_fixes sdhci_intel_byt_emmc =
{
.priv_size = sizeof(struct intel_host),
};
+/*
+ * See Erratum VLI10 from Errata List for Intel Atom E3825, Link:
+ *
https://www.intel.ca/content/dam/www/public/us/en/documents/specification-updates/atom-e3800-family-spec-update.pdf
+ */
+static const struct sdhci_pci_fixes sdhci_intel_byt_emmc_no_runtime_pm = {
+ .allow_runtime_pm = false,
+ .probe_slot = byt_emmc_probe_slot,
+ .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
+ .quirks2= SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+ SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400 |
+ SDHCI_QUIRK2_STOP_WITH_TC,
+ .ops= _intel_byt_ops,
+ .priv_size = sizeof(struct intel_host),
+};
+
static const struct sdhci_pci_fixes sdhci_intel_glk_emmc = {
.allow_runtime_pm = true,
.probe_slot = glk_emmc_probe_slot,
@@ -1470,7 +1485,7 @@ static const struct pci_device_id pci_ids[] = {
SDHCI_PCI_SUBDEVICE(INTEL, BYT_SDIO, NI, 7884, ni_byt_sdio),
SDHCI_PCI_DEVICE(INTEL, BYT_SDIO, intel_byt_sdio),
SDHCI_PCI_DEVICE(INTEL, BYT_SD,intel_byt_sd),
- SDHCI_PCI_DEVICE(INTEL, BYT_EMMC2, intel_byt_emmc),
+ SDHCI_PCI_DEVICE(INTEL, BYT_EMMC2, intel_byt_emmc_no_runtime_pm),
SDHCI_PCI_DEVICE(INTEL, BSW_EMMC, intel_byt_emmc),
SDHCI_PCI_DEVICE(INTEL, BSW_SDIO, intel_byt_sdio),
SDHCI_PCI_DEVICE(INTEL, BSW_SD,intel_byt_sd),
--
2.11.0
[PATCH] tty: serial: 8250: pass IRQ shared flag to UART ports
On some systems IRQ lines between multiple UARTs might be shared. If so, the irqflags have to be configured accordingly. The reason is: The 8250 port startup code performs IRQ tests *before* the IRQ handler for that particular port is registered. This is performed in serial8250_do_startup(). This function checks whether IRQF_SHARED is configured and only then disables the IRQ line while testing. This test is performed upon each open() of the UART device. Imagine two UARTs share the same IRQ line: On is already opened and the IRQ is active. When the second UART is opened, the IRQ line has to be disabled while performing IRQ tests. Otherwise an IRQ might handler might be invoked, but the the IRQ itself cannot be handled, because the corresponding handler isn't registered, yet. That's because the 8250 code uses a chain-handler and invokes the corresponding port's IRQ handling rountines himself. Unfortunately this IRQF_SHARED flag isn't configured for UARTs probed via device tree even if the IRQs are shared. This way, the actual and shared IRQ line isn't disabled while performing tests and the kernel correctly detects a spurious IRQ. So, adding this flag to the DT probe solves the issue. Note: The UPF_SHARE_IRQ flag is configured unconditionally. Therefore, the IRQF_SHARED flag can be set unconditionally as well. Example stacktrace by performing echo 1 > /dev/ttyS2 on a non-patched system: |irq 85: nobody cared (try booting with the "irqpoll" option) | [...] |handlers: |[] irq_default_primary_handler threaded [] serial8250_interrupt |Disabling IRQ #85 Signed-off-by: Kurt Kanzenbach <k...@linutronix.de> --- drivers/tty/serial/8250/8250_of.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c index 160b8906d9b9..575f91e04770 100644 --- a/drivers/tty/serial/8250/8250_of.c +++ b/drivers/tty/serial/8250/8250_of.c @@ -149,6 +149,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev, port->uartclk = clk; port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP | UPF_FIXED_PORT | UPF_FIXED_TYPE; + port->irqflags |= IRQF_SHARED; if (of_property_read_bool(np, "no-loopback-test")) port->flags |= UPF_SKIP_TEST; -- 2.11.0
[PATCH] tty: serial: 8250: pass IRQ shared flag to UART ports
On some systems IRQ lines between multiple UARTs might be shared. If so, the irqflags have to be configured accordingly. The reason is: The 8250 port startup code performs IRQ tests *before* the IRQ handler for that particular port is registered. This is performed in serial8250_do_startup(). This function checks whether IRQF_SHARED is configured and only then disables the IRQ line while testing. This test is performed upon each open() of the UART device. Imagine two UARTs share the same IRQ line: On is already opened and the IRQ is active. When the second UART is opened, the IRQ line has to be disabled while performing IRQ tests. Otherwise an IRQ might handler might be invoked, but the the IRQ itself cannot be handled, because the corresponding handler isn't registered, yet. That's because the 8250 code uses a chain-handler and invokes the corresponding port's IRQ handling rountines himself. Unfortunately this IRQF_SHARED flag isn't configured for UARTs probed via device tree even if the IRQs are shared. This way, the actual and shared IRQ line isn't disabled while performing tests and the kernel correctly detects a spurious IRQ. So, adding this flag to the DT probe solves the issue. Note: The UPF_SHARE_IRQ flag is configured unconditionally. Therefore, the IRQF_SHARED flag can be set unconditionally as well. Example stacktrace by performing echo 1 > /dev/ttyS2 on a non-patched system: |irq 85: nobody cared (try booting with the "irqpoll" option) | [...] |handlers: |[] irq_default_primary_handler threaded [] serial8250_interrupt |Disabling IRQ #85 Signed-off-by: Kurt Kanzenbach --- drivers/tty/serial/8250/8250_of.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c index 160b8906d9b9..575f91e04770 100644 --- a/drivers/tty/serial/8250/8250_of.c +++ b/drivers/tty/serial/8250/8250_of.c @@ -149,6 +149,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev, port->uartclk = clk; port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP | UPF_FIXED_PORT | UPF_FIXED_TYPE; + port->irqflags |= IRQF_SHARED; if (of_property_read_bool(np, "no-loopback-test")) port->flags |= UPF_SKIP_TEST; -- 2.11.0
[PATCH] spi: spi-fsl-dspi: add SPI_LSB_FIRST to driver capabilities
The driver as well as the controller support the SPI lsb first mode. However, it's not possible to configure it e.g. when using spidev. Adding this flag to mode_bits resolves the issue and lsb first mode can be used. Signed-off-by: Kurt Kanzenbach <k...@linutronix.de> --- drivers/spi/spi-fsl-dspi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c index f652f70cb8db..02d3ed7f2558 100644 --- a/drivers/spi/spi-fsl-dspi.c +++ b/drivers/spi/spi-fsl-dspi.c @@ -980,7 +980,7 @@ static int dspi_probe(struct platform_device *pdev) master->dev.of_node = pdev->dev.of_node; master->cleanup = dspi_cleanup; - master->mode_bits = SPI_CPOL | SPI_CPHA; + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST; master->bits_per_word_mask = SPI_BPW_MASK(4) | SPI_BPW_MASK(8) | SPI_BPW_MASK(16); -- 2.11.0
[PATCH] spi: spi-fsl-dspi: add SPI_LSB_FIRST to driver capabilities
The driver as well as the controller support the SPI lsb first mode. However, it's not possible to configure it e.g. when using spidev. Adding this flag to mode_bits resolves the issue and lsb first mode can be used. Signed-off-by: Kurt Kanzenbach --- drivers/spi/spi-fsl-dspi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c index f652f70cb8db..02d3ed7f2558 100644 --- a/drivers/spi/spi-fsl-dspi.c +++ b/drivers/spi/spi-fsl-dspi.c @@ -980,7 +980,7 @@ static int dspi_probe(struct platform_device *pdev) master->dev.of_node = pdev->dev.of_node; master->cleanup = dspi_cleanup; - master->mode_bits = SPI_CPOL | SPI_CPHA; + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST; master->bits_per_word_mask = SPI_BPW_MASK(4) | SPI_BPW_MASK(8) | SPI_BPW_MASK(16); -- 2.11.0
[PATCH 8/8] Staging: rtl8192u: ieee80211: added missing blank lines
This patch resolves the following checkpatch warnings:
- WARNING: Missing a blank line after declarations
Signed-off-by: Kurt Kanzenbach
---
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 157af31..908bc2e 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -297,6 +297,7 @@ static void tkip_mixing_phase2(u8 *WEPSeed, const u8 *TK,
const u16 *TTAK,
#ifdef __BIG_ENDIAN
{
int i;
+
for (i = 0; i < 6; i++)
PPK[i] = (PPK[i] << 8) | (PPK[i] >> 8);
}
@@ -395,6 +396,7 @@ static int ieee80211_tkip_decrypt(struct sk_buff *skb, int
hdr_len, void *priv)
u32 crc;
struct scatterlist sg;
int plen;
+
if (skb->len < hdr_len + 8 + 4)
return -1;
@@ -630,6 +632,7 @@ static int ieee80211_michael_mic_verify(struct sk_buff
*skb, int keyidx,
if (memcmp(mic, skb->data + skb->len - 8, 8) != 0) {
struct rtl_80211_hdr_4addr *hdr;
hdr = (struct rtl_80211_hdr_4addr *) skb->data;
+
printk(KERN_DEBUG "%s: Michael MIC verification failed for "
"MSDU from %pM keyidx=%d\n",
skb->dev ? skb->dev->name : "N/A", hdr->addr2,
@@ -703,6 +706,7 @@ static int ieee80211_tkip_get_key(void *key, int len, u8
*seq, void *priv)
/* Return the sequence number of the last transmitted frame. */
u16 iv16 = tkey->tx_iv16;
u32 iv32 = tkey->tx_iv32;
+
if (iv16 == 0)
iv32--;
iv16--;
@@ -721,6 +725,7 @@ static int ieee80211_tkip_get_key(void *key, int len, u8
*seq, void *priv)
static char *ieee80211_tkip_print_stats(char *p, void *priv)
{
struct ieee80211_tkip_data *tkip = priv;
+
p += sprintf(p, "key[%d] alg=TKIP key_set=%d "
"tx_pn=%02x%02x%02x%02x%02x%02x "
"rx_pn=%02x%02x%02x%02x%02x%02x "
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/8] Staging: rtl8192u: ieee80211: added missing spaces after if
This patch fixes the following checkpatch error:
- ERROR: space required before the open parenthesis '('
Signed-off-by: Kurt Kanzenbach
---
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 6c48df1..06def48 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -563,7 +563,7 @@ static int ieee80211_michael_mic_add(struct sk_buff *skb,
int hdr_len, void *pri
// { david, 2006.9.1
// fix the wpa process with wmm enabled.
- if(IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl))) {
+ if (IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl))) {
tkey->tx_hdr[12] = *(skb->data + hdr_len - 2) & 0x07;
}
// }
@@ -612,7 +612,7 @@ static int ieee80211_michael_mic_verify(struct sk_buff
*skb, int keyidx,
michael_mic_hdr(skb, tkey->rx_hdr);
// { david, 2006.9.1
// fix the wpa process with wmm enabled.
- if(IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl))) {
+ if (IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl))) {
tkey->rx_hdr[12] = *(skb->data + hdr_len - 2) & 0x07;
}
// }
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/8] Staging: rtl8192u: ieee80211: corrected block comments
This patch reformats some block comments in order to
match the Linux kernel coding style.
Signed-off-by: Kurt Kanzenbach
---
.../rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 31 +++---
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index bf71501..7c166cf 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -256,8 +256,10 @@ static void tkip_mixing_phase1(u16 *TTAK, const u8 *TK,
const u8 *TA, u32 IV32)
static void tkip_mixing_phase2(u8 *WEPSeed, const u8 *TK, const u16 *TTAK,
u16 IV16)
{
- /* Make temporary area overlap WEP seed so that the final copy can be
-* avoided on little endian hosts. */
+ /*
+* Make temporary area overlap WEP seed so that the final copy can be
+* avoided on little endian hosts.
+*/
u16 *PPK = (u16 *) [4];
/* Step 1 - make copy of TTAK and bring in TSC */
@@ -283,8 +285,10 @@ static void tkip_mixing_phase2(u8 *WEPSeed, const u8 *TK,
const u16 *TTAK,
PPK[4] += RotR1(PPK[3]);
PPK[5] += RotR1(PPK[4]);
- /* Step 3 - bring in last of TK bits, assign 24-bit WEP IV value
-* WEPSeed[0..2] is transmitted as WEP IV */
+ /*
+* Step 3 - bring in last of TK bits, assign 24-bit WEP IV value
+* WEPSeed[0..2] is transmitted as WEP IV
+*/
WEPSeed[0] = Hi8(IV16);
WEPSeed[1] = (Hi8(IV16) | 0x20) & 0x7F;
WEPSeed[2] = Lo8(IV16);
@@ -463,8 +467,11 @@ static int ieee80211_tkip_decrypt(struct sk_buff *skb, int
hdr_len, void *priv)
if (memcmp(icv, pos + plen, 4) != 0) {
if (iv32 != tkey->rx_iv32) {
- /* Previously cached Phase1 result was already
lost, so
- * it needs to be recalculated for the next
packet. */
+ /*
+* Previously cached Phase1 result was already
+* lost, so it needs to be recalculated for the
+* next packet.
+*/
tkey->rx_phase1_done = 0;
}
if (net_ratelimit()) {
@@ -477,8 +484,10 @@ static int ieee80211_tkip_decrypt(struct sk_buff *skb, int
hdr_len, void *priv)
}
- /* Update real counters only after Michael MIC verification has
-* completed */
+ /*
+* Update real counters only after Michael MIC verification has
+* completed.
+*/
tkey->rx_iv32_new = iv32;
tkey->rx_iv16_new = iv16;
@@ -633,8 +642,10 @@ static int ieee80211_michael_mic_verify(struct sk_buff
*skb, int keyidx,
return -1;
}
- /* Update TSC counters for RX now that the packet verification has
-* completed. */
+ /*
+* Update TSC counters for RX now that the packet verification has
+* completed.
+*/
tkey->rx_iv32 = tkey->rx_iv32_new;
tkey->rx_iv16 = tkey->rx_iv16_new;
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/8] Staging: rtl8192u: ieee80211: fixed open brace positions
This patch fixes the following checkpatch error:
- ERROR: that open brace { should be on the previous line
Signed-off-by: Kurt Kanzenbach
---
.../staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 1f80c52..dd058b6 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -185,8 +185,7 @@ static inline u16 Mk16_le(u16 *v)
}
-static const u16 Sbox[256] =
-{
+static const u16 Sbox[256] = {
0xC6A5, 0xF884, 0xEE99, 0xF68D, 0xFF0D, 0xD6BD, 0xDEB1, 0x9154,
0x6050, 0x0203, 0xCEA9, 0x567D, 0xE719, 0xB562, 0x4DE6, 0xEC9A,
0x8F45, 0x1F9D, 0x8940, 0xFA87, 0xEF15, 0xB2EB, 0x8EC9, 0xFB0B,
@@ -320,8 +319,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int
hdr_len, void *priv)
hdr = (struct rtl_80211_hdr_4addr *) skb->data;
- if (!tcb_desc->bHwSec)
- {
+ if (!tcb_desc->bHwSec) {
if (!tkey->tx_phase1_done) {
tkip_mixing_phase1(tkey->tx_ttak, tkey->key, hdr->addr2,
tkey->tx_iv32);
@@ -338,14 +336,12 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb,
int hdr_len, void *priv)
memmove(pos, pos + 8, hdr_len);
pos += hdr_len;
- if (tcb_desc->bHwSec)
- {
+ if (tcb_desc->bHwSec) {
*pos++ = Hi8(tkey->tx_iv16);
*pos++ = (Hi8(tkey->tx_iv16) | 0x20) & 0x7F;
*pos++ = Lo8(tkey->tx_iv16);
}
- else
- {
+ else {
*pos++ = rc4key[0];
*pos++ = rc4key[1];
*pos++ = rc4key[2];
@@ -357,8 +353,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int
hdr_len, void *priv)
*pos++ = (tkey->tx_iv32 >> 16) & 0xff;
*pos++ = (tkey->tx_iv32 >> 24) & 0xff;
- if (!tcb_desc->bHwSec)
- {
+ if (!tcb_desc->bHwSec) {
icv = skb_put(skb, 4);
crc = ~crc32_le(~0, pos, len);
icv[0] = crc;
@@ -429,8 +424,7 @@ static int ieee80211_tkip_decrypt(struct sk_buff *skb, int
hdr_len, void *priv)
iv32 = pos[4] | (pos[5] << 8) | (pos[6] << 16) | (pos[7] << 24);
pos += 8;
- if (!tcb_desc->bHwSec)
- {
+ if (!tcb_desc->bHwSec) {
if (iv32 < tkey->rx_iv32 ||
(iv32 == tkey->rx_iv32 && iv16 <= tkey->rx_iv16)) {
if (net_ratelimit()) {
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/8] Staging: rtl8192u: ieee80211: checkpatch cleanups
This patch series corrects most checkpatch.pl errors and some warnings in the ieee80211_crypt_tkip.c file. Kurt Kanzenbach (8): Staging: rtl8192u: ieee80211: fixed open brace positions Staging: rtl8192u: ieee80211: fixed position of else statements Staging: rtl8192u: ieee80211: added missing space around '=' Staging: rtl8192u: ieee80211: added missing spaces after if Staging: rtl8192u: ieee80211: corrected indent Staging: rtl8192u: ieee80211: corrected block comments Staging: rtl8192u: ieee80211: removed unnecessary braces Staging: rtl8192u: ieee80211: added missing blank lines .../rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 76 -- 1 file changed, 41 insertions(+), 35 deletions(-) -- 2.4.10 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/8] Staging: rtl8192u: ieee80211: removed unnecessary braces
This patch fixes the following checkpatch warning:
- WARNING: braces {} are not necessary for single statement blocks
Signed-off-by: Kurt Kanzenbach
---
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 7c166cf..157af31 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -572,9 +572,8 @@ static int ieee80211_michael_mic_add(struct sk_buff *skb,
int hdr_len, void *pri
// { david, 2006.9.1
// fix the wpa process with wmm enabled.
- if (IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl))) {
+ if (IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl)))
tkey->tx_hdr[12] = *(skb->data + hdr_len - 2) & 0x07;
- }
// }
pos = skb_put(skb, 8);
@@ -621,9 +620,8 @@ static int ieee80211_michael_mic_verify(struct sk_buff
*skb, int keyidx,
michael_mic_hdr(skb, tkey->rx_hdr);
// { david, 2006.9.1
// fix the wpa process with wmm enabled.
- if (IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl))) {
+ if (IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl)))
tkey->rx_hdr[12] = *(skb->data + hdr_len - 2) & 0x07;
- }
// }
if (michael_mic(tkey->rx_tfm_michael, >key[24], tkey->rx_hdr,
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/8] Staging: rtl8192u: ieee80211: fixed open brace positions
This patch fixes the following checkpatch error:
- ERROR: that open brace { should be on the previous line
Signed-off-by: Kurt Kanzenbach
---
.../staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 1f80c52..dd058b6 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -185,8 +185,7 @@ static inline u16 Mk16_le(u16 *v)
}
-static const u16 Sbox[256] =
-{
+static const u16 Sbox[256] = {
0xC6A5, 0xF884, 0xEE99, 0xF68D, 0xFF0D, 0xD6BD, 0xDEB1, 0x9154,
0x6050, 0x0203, 0xCEA9, 0x567D, 0xE719, 0xB562, 0x4DE6, 0xEC9A,
0x8F45, 0x1F9D, 0x8940, 0xFA87, 0xEF15, 0xB2EB, 0x8EC9, 0xFB0B,
@@ -320,8 +319,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int
hdr_len, void *priv)
hdr = (struct rtl_80211_hdr_4addr *) skb->data;
- if (!tcb_desc->bHwSec)
- {
+ if (!tcb_desc->bHwSec) {
if (!tkey->tx_phase1_done) {
tkip_mixing_phase1(tkey->tx_ttak, tkey->key, hdr->addr2,
tkey->tx_iv32);
@@ -338,14 +336,12 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb,
int hdr_len, void *priv)
memmove(pos, pos + 8, hdr_len);
pos += hdr_len;
- if (tcb_desc->bHwSec)
- {
+ if (tcb_desc->bHwSec) {
*pos++ = Hi8(tkey->tx_iv16);
*pos++ = (Hi8(tkey->tx_iv16) | 0x20) & 0x7F;
*pos++ = Lo8(tkey->tx_iv16);
}
- else
- {
+ else {
*pos++ = rc4key[0];
*pos++ = rc4key[1];
*pos++ = rc4key[2];
@@ -357,8 +353,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int
hdr_len, void *priv)
*pos++ = (tkey->tx_iv32 >> 16) & 0xff;
*pos++ = (tkey->tx_iv32 >> 24) & 0xff;
- if (!tcb_desc->bHwSec)
- {
+ if (!tcb_desc->bHwSec) {
icv = skb_put(skb, 4);
crc = ~crc32_le(~0, pos, len);
icv[0] = crc;
@@ -429,8 +424,7 @@ static int ieee80211_tkip_decrypt(struct sk_buff *skb, int
hdr_len, void *priv)
iv32 = pos[4] | (pos[5] << 8) | (pos[6] << 16) | (pos[7] << 24);
pos += 8;
- if (!tcb_desc->bHwSec)
- {
+ if (!tcb_desc->bHwSec) {
if (iv32 < tkey->rx_iv32 ||
(iv32 == tkey->rx_iv32 && iv16 <= tkey->rx_iv16)) {
if (net_ratelimit()) {
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/8] Staging: rtl8192u: ieee80211: fixed position of else statements
This patch fixes the following checkpatch error:
- ERROR: else should follow close brace '}'
Signed-off-by: Kurt Kanzenbach
---
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index dd058b6..8091f49 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -326,8 +326,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int
hdr_len, void *priv)
tkey->tx_phase1_done = 1;
}
tkip_mixing_phase2(rc4key, tkey->key, tkey->tx_ttak,
tkey->tx_iv16);
- }
- else
+ } else
tkey->tx_phase1_done = 1;
@@ -340,8 +339,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int
hdr_len, void *priv)
*pos++ = Hi8(tkey->tx_iv16);
*pos++ = (Hi8(tkey->tx_iv16) | 0x20) & 0x7F;
*pos++ = Lo8(tkey->tx_iv16);
- }
- else {
+ } else {
*pos++ = rc4key[0];
*pos++ = rc4key[1];
*pos++ = rc4key[2];
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/8] Staging: rtl8192u: ieee80211: checkpatch cleanups
This patch series corrects most checkpatch.pl errors and some warnings in the ieee80211_crypt_tkip.c file. Kurt Kanzenbach (8): Staging: rtl8192u: ieee80211: fixed open brace positions Staging: rtl8192u: ieee80211: fixed position of else statements Staging: rtl8192u: ieee80211: added missing space around '=' Staging: rtl8192u: ieee80211: added missing spaces after if Staging: rtl8192u: ieee80211: corrected indent Staging: rtl8192u: ieee80211: corrected block comments Staging: rtl8192u: ieee80211: removed unnecessary braces Staging: rtl8192u: ieee80211: added missing blank lines .../rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 76 -- 1 file changed, 41 insertions(+), 35 deletions(-) -- 2.4.10 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/8] Staging: rtl8192u: ieee80211: corrected indent
This patch corrects the indentation in five instances in the
ieee80211_crypt_tkip.c file.
Signed-off-by: Kurt Kanzenbach
---
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 06def48..bf71501 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -303,7 +303,7 @@ static void tkip_mixing_phase2(u8 *WEPSeed, const u8 *TK,
const u16 *TTAK,
static int ieee80211_tkip_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
{
struct ieee80211_tkip_data *tkey = priv;
- int len;
+ int len;
u8 *pos;
struct rtl_80211_hdr_4addr *hdr;
cb_desc *tcb_desc = (cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
@@ -322,12 +322,12 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb,
int hdr_len, void *priv)
if (!tcb_desc->bHwSec) {
if (!tkey->tx_phase1_done) {
tkip_mixing_phase1(tkey->tx_ttak, tkey->key, hdr->addr2,
- tkey->tx_iv32);
+ tkey->tx_iv32);
tkey->tx_phase1_done = 1;
}
tkip_mixing_phase2(rc4key, tkey->key, tkey->tx_ttak,
tkey->tx_iv16);
} else
- tkey->tx_phase1_done = 1;
+ tkey->tx_phase1_done = 1;
len = skb->len - hdr_len;
@@ -598,7 +598,7 @@ static void ieee80211_michael_mic_failure(struct net_device
*dev,
}
static int ieee80211_michael_mic_verify(struct sk_buff *skb, int keyidx,
-int hdr_len, void *priv)
+ int hdr_len, void *priv)
{
struct ieee80211_tkip_data *tkey = priv;
u8 mic[8];
@@ -618,7 +618,7 @@ static int ieee80211_michael_mic_verify(struct sk_buff
*skb, int keyidx,
// }
if (michael_mic(tkey->rx_tfm_michael, >key[24], tkey->rx_hdr,
- skb->data + hdr_len, skb->len - 8 - hdr_len,
mic))
+ skb->data + hdr_len, skb->len - 8 - hdr_len, mic))
return -1;
if (memcmp(mic, skb->data + skb->len - 8, 8) != 0) {
struct rtl_80211_hdr_4addr *hdr;
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/8] Staging: rtl8192u: ieee80211: added missing space around '='
This patch fixes the following checkpatch error: - ERROR: spaces required around that '=' Signed-off-by: Kurt Kanzenbach --- drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c index 8091f49..6c48df1 100644 --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c @@ -360,7 +360,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int hdr_len, void *priv) icv[3] = crc >> 24; crypto_blkcipher_setkey(tkey->tx_tfm_arc4, rc4key, 16); sg_init_one(, pos, len+4); - ret= crypto_blkcipher_encrypt(, , , len + 4); + ret = crypto_blkcipher_encrypt(, , , len + 4); } tkey->tx_iv16++; -- 2.4.10 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/8] Staging: rtl8192u: ieee80211: removed unnecessary braces
This patch fixes the following checkpatch warning:
- WARNING: braces {} are not necessary for single statement blocks
Signed-off-by: Kurt Kanzenbach <k...@kmk-computers.de>
---
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 7c166cf..157af31 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -572,9 +572,8 @@ static int ieee80211_michael_mic_add(struct sk_buff *skb,
int hdr_len, void *pri
// { david, 2006.9.1
// fix the wpa process with wmm enabled.
- if (IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl))) {
+ if (IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl)))
tkey->tx_hdr[12] = *(skb->data + hdr_len - 2) & 0x07;
- }
// }
pos = skb_put(skb, 8);
@@ -621,9 +620,8 @@ static int ieee80211_michael_mic_verify(struct sk_buff
*skb, int keyidx,
michael_mic_hdr(skb, tkey->rx_hdr);
// { david, 2006.9.1
// fix the wpa process with wmm enabled.
- if (IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl))) {
+ if (IEEE80211_QOS_HAS_SEQ(le16_to_cpu(hdr->frame_ctl)))
tkey->rx_hdr[12] = *(skb->data + hdr_len - 2) & 0x07;
- }
// }
if (michael_mic(tkey->rx_tfm_michael, >key[24], tkey->rx_hdr,
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/8] Staging: rtl8192u: ieee80211: fixed open brace positions
This patch fixes the following checkpatch error:
- ERROR: that open brace { should be on the previous line
Signed-off-by: Kurt Kanzenbach <k...@kmk-computers.de>
---
.../staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 1f80c52..dd058b6 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -185,8 +185,7 @@ static inline u16 Mk16_le(u16 *v)
}
-static const u16 Sbox[256] =
-{
+static const u16 Sbox[256] = {
0xC6A5, 0xF884, 0xEE99, 0xF68D, 0xFF0D, 0xD6BD, 0xDEB1, 0x9154,
0x6050, 0x0203, 0xCEA9, 0x567D, 0xE719, 0xB562, 0x4DE6, 0xEC9A,
0x8F45, 0x1F9D, 0x8940, 0xFA87, 0xEF15, 0xB2EB, 0x8EC9, 0xFB0B,
@@ -320,8 +319,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int
hdr_len, void *priv)
hdr = (struct rtl_80211_hdr_4addr *) skb->data;
- if (!tcb_desc->bHwSec)
- {
+ if (!tcb_desc->bHwSec) {
if (!tkey->tx_phase1_done) {
tkip_mixing_phase1(tkey->tx_ttak, tkey->key, hdr->addr2,
tkey->tx_iv32);
@@ -338,14 +336,12 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb,
int hdr_len, void *priv)
memmove(pos, pos + 8, hdr_len);
pos += hdr_len;
- if (tcb_desc->bHwSec)
- {
+ if (tcb_desc->bHwSec) {
*pos++ = Hi8(tkey->tx_iv16);
*pos++ = (Hi8(tkey->tx_iv16) | 0x20) & 0x7F;
*pos++ = Lo8(tkey->tx_iv16);
}
- else
- {
+ else {
*pos++ = rc4key[0];
*pos++ = rc4key[1];
*pos++ = rc4key[2];
@@ -357,8 +353,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int
hdr_len, void *priv)
*pos++ = (tkey->tx_iv32 >> 16) & 0xff;
*pos++ = (tkey->tx_iv32 >> 24) & 0xff;
- if (!tcb_desc->bHwSec)
- {
+ if (!tcb_desc->bHwSec) {
icv = skb_put(skb, 4);
crc = ~crc32_le(~0, pos, len);
icv[0] = crc;
@@ -429,8 +424,7 @@ static int ieee80211_tkip_decrypt(struct sk_buff *skb, int
hdr_len, void *priv)
iv32 = pos[4] | (pos[5] << 8) | (pos[6] << 16) | (pos[7] << 24);
pos += 8;
- if (!tcb_desc->bHwSec)
- {
+ if (!tcb_desc->bHwSec) {
if (iv32 < tkey->rx_iv32 ||
(iv32 == tkey->rx_iv32 && iv16 <= tkey->rx_iv16)) {
if (net_ratelimit()) {
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/8] Staging: rtl8192u: ieee80211: added missing space around '='
This patch fixes the following checkpatch error: - ERROR: spaces required around that '=' Signed-off-by: Kurt Kanzenbach <k...@kmk-computers.de> --- drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c index 8091f49..6c48df1 100644 --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c @@ -360,7 +360,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int hdr_len, void *priv) icv[3] = crc >> 24; crypto_blkcipher_setkey(tkey->tx_tfm_arc4, rc4key, 16); sg_init_one(, pos, len+4); - ret= crypto_blkcipher_encrypt(, , , len + 4); + ret = crypto_blkcipher_encrypt(, , , len + 4); } tkey->tx_iv16++; -- 2.4.10 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/8] Staging: rtl8192u: ieee80211: corrected indent
This patch corrects the indentation in five instances in the
ieee80211_crypt_tkip.c file.
Signed-off-by: Kurt Kanzenbach <k...@kmk-computers.de>
---
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 06def48..bf71501 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -303,7 +303,7 @@ static void tkip_mixing_phase2(u8 *WEPSeed, const u8 *TK,
const u16 *TTAK,
static int ieee80211_tkip_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
{
struct ieee80211_tkip_data *tkey = priv;
- int len;
+ int len;
u8 *pos;
struct rtl_80211_hdr_4addr *hdr;
cb_desc *tcb_desc = (cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
@@ -322,12 +322,12 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb,
int hdr_len, void *priv)
if (!tcb_desc->bHwSec) {
if (!tkey->tx_phase1_done) {
tkip_mixing_phase1(tkey->tx_ttak, tkey->key, hdr->addr2,
- tkey->tx_iv32);
+ tkey->tx_iv32);
tkey->tx_phase1_done = 1;
}
tkip_mixing_phase2(rc4key, tkey->key, tkey->tx_ttak,
tkey->tx_iv16);
} else
- tkey->tx_phase1_done = 1;
+ tkey->tx_phase1_done = 1;
len = skb->len - hdr_len;
@@ -598,7 +598,7 @@ static void ieee80211_michael_mic_failure(struct net_device
*dev,
}
static int ieee80211_michael_mic_verify(struct sk_buff *skb, int keyidx,
-int hdr_len, void *priv)
+ int hdr_len, void *priv)
{
struct ieee80211_tkip_data *tkey = priv;
u8 mic[8];
@@ -618,7 +618,7 @@ static int ieee80211_michael_mic_verify(struct sk_buff
*skb, int keyidx,
// }
if (michael_mic(tkey->rx_tfm_michael, >key[24], tkey->rx_hdr,
- skb->data + hdr_len, skb->len - 8 - hdr_len,
mic))
+ skb->data + hdr_len, skb->len - 8 - hdr_len, mic))
return -1;
if (memcmp(mic, skb->data + skb->len - 8, 8) != 0) {
struct rtl_80211_hdr_4addr *hdr;
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/8] Staging: rtl8192u: ieee80211: fixed position of else statements
This patch fixes the following checkpatch error:
- ERROR: else should follow close brace '}'
Signed-off-by: Kurt Kanzenbach <k...@kmk-computers.de>
---
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index dd058b6..8091f49 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -326,8 +326,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int
hdr_len, void *priv)
tkey->tx_phase1_done = 1;
}
tkip_mixing_phase2(rc4key, tkey->key, tkey->tx_ttak,
tkey->tx_iv16);
- }
- else
+ } else
tkey->tx_phase1_done = 1;
@@ -340,8 +339,7 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int
hdr_len, void *priv)
*pos++ = Hi8(tkey->tx_iv16);
*pos++ = (Hi8(tkey->tx_iv16) | 0x20) & 0x7F;
*pos++ = Lo8(tkey->tx_iv16);
- }
- else {
+ } else {
*pos++ = rc4key[0];
*pos++ = rc4key[1];
*pos++ = rc4key[2];
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/8] Staging: rtl8192u: ieee80211: checkpatch cleanups
This patch series corrects most checkpatch.pl errors and some warnings in the ieee80211_crypt_tkip.c file. Kurt Kanzenbach (8): Staging: rtl8192u: ieee80211: fixed open brace positions Staging: rtl8192u: ieee80211: fixed position of else statements Staging: rtl8192u: ieee80211: added missing space around '=' Staging: rtl8192u: ieee80211: added missing spaces after if Staging: rtl8192u: ieee80211: corrected indent Staging: rtl8192u: ieee80211: corrected block comments Staging: rtl8192u: ieee80211: removed unnecessary braces Staging: rtl8192u: ieee80211: added missing blank lines .../rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 76 -- 1 file changed, 41 insertions(+), 35 deletions(-) -- 2.4.10 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 8/8] Staging: rtl8192u: ieee80211: added missing blank lines
This patch resolves the following checkpatch warnings:
- WARNING: Missing a blank line after declarations
Signed-off-by: Kurt Kanzenbach <k...@kmk-computers.de>
---
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 157af31..908bc2e 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -297,6 +297,7 @@ static void tkip_mixing_phase2(u8 *WEPSeed, const u8 *TK,
const u16 *TTAK,
#ifdef __BIG_ENDIAN
{
int i;
+
for (i = 0; i < 6; i++)
PPK[i] = (PPK[i] << 8) | (PPK[i] >> 8);
}
@@ -395,6 +396,7 @@ static int ieee80211_tkip_decrypt(struct sk_buff *skb, int
hdr_len, void *priv)
u32 crc;
struct scatterlist sg;
int plen;
+
if (skb->len < hdr_len + 8 + 4)
return -1;
@@ -630,6 +632,7 @@ static int ieee80211_michael_mic_verify(struct sk_buff
*skb, int keyidx,
if (memcmp(mic, skb->data + skb->len - 8, 8) != 0) {
struct rtl_80211_hdr_4addr *hdr;
hdr = (struct rtl_80211_hdr_4addr *) skb->data;
+
printk(KERN_DEBUG "%s: Michael MIC verification failed for "
"MSDU from %pM keyidx=%d\n",
skb->dev ? skb->dev->name : "N/A", hdr->addr2,
@@ -703,6 +706,7 @@ static int ieee80211_tkip_get_key(void *key, int len, u8
*seq, void *priv)
/* Return the sequence number of the last transmitted frame. */
u16 iv16 = tkey->tx_iv16;
u32 iv32 = tkey->tx_iv32;
+
if (iv16 == 0)
iv32--;
iv16--;
@@ -721,6 +725,7 @@ static int ieee80211_tkip_get_key(void *key, int len, u8
*seq, void *priv)
static char *ieee80211_tkip_print_stats(char *p, void *priv)
{
struct ieee80211_tkip_data *tkip = priv;
+
p += sprintf(p, "key[%d] alg=TKIP key_set=%d "
"tx_pn=%02x%02x%02x%02x%02x%02x "
"rx_pn=%02x%02x%02x%02x%02x%02x "
--
2.4.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/8] Staging: rtl8192u: ieee80211: checkpatch cleanups
This patch series corrects most checkpatch.pl errors and some warnings in the ieee80211_crypt_tkip.c file. Kurt Kanzenbach (8): Staging: rtl8192u: ieee80211: fixed open brace positions Staging: rtl8192u: ieee80211: fixed position of else statements Staging: rtl8192u: ieee80211: added missing space around '=' Staging: rtl8192u: ieee80211: added missing spaces after if Staging: rtl8192u: ieee80211: corrected indent Staging: rtl8192u: ieee80211: corrected block comments Staging: rtl8192u: ieee80211: removed unnecessary braces Staging: rtl8192u: ieee80211: added missing blank lines .../rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 76 -- 1 file changed, 41 insertions(+), 35 deletions(-) -- 2.4.10 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
