from:"oliver"

Am Donnerstag, den 08.04.2021, 15:16 +0200 schrieb Johan Hovold:
> TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
> serial devices is only useful for setting the close_delay and
> closing_wait parameters.
> 
> The xmit_fifo_size parameter could be used to set the hardware transmit
> fifo size of a legacy UART when it could not be detected, but the
> interface is limited to eight bits and should be left unset when it is
> not used.
> 
> Similarly, baud_base could be used to set the UART base clock when it
> could not be detected, but might as well be left unset when it is not
> known (which is the case for CDC).
> 
> Fix the cdc-acm TIOCGSERIAL implementation by dropping its custom
> interpretation of the unused xmit_fifo_size and baud_base fields, which
> overflowed the former with the URB buffer size and set the latter to the
> current line speed. Also return the port line number, which is the only
> other value used besides the close parameters.
> 
> Note that the current line speed can still be retrieved through the
> standard termios interfaces.
> 
> Fixes: 18c75720e667 ("USB: allow users to run setserial with cdc-acm")
> Signed-off-by: Johan Hovold 
Acked-by: Oliver Neukum

Re: [PATCH v2 2/3] USB: cdc-acm: fix unprivileged TIOCCSERIAL

Am Donnerstag, den 08.04.2021, 15:16 +0200 schrieb Johan Hovold:
> TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
> serial devices is only useful for setting the close_delay and
> closing_wait parameters.
> 
> A non-privileged user has only ever been able to set the since long
> deprecated ASYNC_SPD flags and trying to change any other *supported*
> feature should result in -EPERM being returned. Setting the current
> values for any supported features should return success.
> 
> Fix the cdc-acm implementation which instead indicated that the
> TIOCSSERIAL ioctl was not even implemented when a non-privileged user
> set the current values.
> 
> Fixes: ba2d8ce9db0a ("cdc-acm: implement TIOCSSERIAL to avoid blocking 
> close(2)")
> Signed-off-by: Johan Hovold 
Acked-by: Oliver Neukum

Re: [PATCH v2 1/3] Revert "USB: cdc-acm: fix rounding error in TIOCSSERIAL"

Am Donnerstag, den 08.04.2021, 15:16 +0200 schrieb Johan Hovold:
> This reverts commit b401f8c4f492cbf74f3f59c9141e5be3071071bb.
> 
> The offending commit claimed that trying to set the values reported back
> by TIOCGSERIAL as a regular user could result in an -EPERM error when HZ
> is 250, but that was never the case.
> 
> With HZ=250, the default 0.5 second value of close_delay is converted to
> 125 jiffies when set and is converted back to 50 centiseconds by
> TIOCGSERIAL as expected (not 12 cs as was claimed, even if that was the
> case before an earlier fix).
> 
> Comparing the internal current and new jiffies values is just fine to
> determine if the value is about to change so drop the bogus workaround
> (which was also backported to stable).
> 
> For completeness: With different default values for these parameters or
> with a HZ value not divisible by two, the lack of rounding when setting
> the default values in tty_port_init() could result in an -EPERM being
> returned, but this is hardly something we need to worry about.
> 
> Cc: Anthony Mallet 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Johan Hovold 
Acked-by: Oliver Neukum

Re: [PATCH 3/3] USB: cdc-acm: fix TIOCGSERIAL implementation

Am Donnerstag, den 08.04.2021, 13:54 +0200 schrieb Johan Hovold:
> On Thu, Apr 08, 2021 at 01:34:12PM +0200, Oliver Neukum wrote:
> > Am Donnerstag, den 08.04.2021, 11:48 +0200 schrieb Johan Hovold:
> > > On Thu, Apr 08, 2021 at 10:36:46AM +0200, Oliver Neukum wrote:
> > > > Am Mittwoch, den 07.04.2021, 12:28 +0200 schrieb Johan Hovold:
> > > > Well, the devices report it. It is part of the standard.
> > > 
> > > No, the standard doesn't include anything about a baud-base clock
> > > AFAICT.
> > 
> > Unfortunately it does.
> > dwDTERate - chapter 6.3.11 - table 17
> 
> That's not the base clock rate, that's just the currently configured
> line speed which you can read from termios 
> > If we does this wrongly, we should certainly fix it, but just removing
> > the reporting doesn't look right to me.
> 
> The driver got its interpretation of baud_base wrong, and CDC doesn't
> even have a concept of base clock rate so removing it is the right thing
> to do.
> 
> Again, baud_base is really only relevant with legacy UARTs and when
> using the deprecated ASYNC_SPD_CUST.
> 
> And if the user wants to knows the current line speed we have a
> different interface for that.

Hi,

thank you, that clarifies things. I am happy with the patch itself,
but could I ask you to do two things:

1. Edit the commit description
making clear that the difference
between the base clock rate and the line speed.

2. Mark the patch specially to NOT be included in stable. We may
have
users misusing the current API.

Regards
Oliver

Re: [PATCH 2/3] USB: cdc-acm: fix unprivileged TIOCCSERIAL

Am Donnerstag, den 08.04.2021, 11:42 +0200 schrieb Johan Hovold:
> On Thu, Apr 08, 2021 at 09:48:38AM +0200, Oliver Neukum wrote:
> > Am Mittwoch, den 07.04.2021, 12:28 +0200 schrieb Johan Hovold:
> > > TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
> > > serial devices is only useful for setting the close_delay and
> > > closing_wait parameters.
> > > 
> > > A non-privileged user has only ever been able to set the since long
> > > deprecated ASYNC_SPD flags and trying to change any other *supported*
> > > feature should result in -EPERM being returned. Setting the current
> > > values for any supported features should return success.
> > > 
> > > Fix the cdc-acm implementation which instead indicated that the
> > > TIOCSSERIAL ioctl was not even implemented when a non-privileged user
> > > set the current values.
> > 
> > Hi,
> > 
> > the idea here was that you are setting something else, if you are
> > not changing a parameter that can be changed. That conclusion is
> > dubious, but at the same time, this implementation can change
> > only these two parameters. So can the test really be dropped
> > as opposed to be modified?
> 
> The de-facto standard for how to handle change requests for
> non-supported features (e.g. changing the I/O port or IRQ) is to simply
> ignore them and return 0.
> 
> For most (non-legacy) serial devices the only relevant parameters are
> close_delay and closing_wait. And as we need to return -EPERM when a
> non-privileged user tries to change these, we cannot drop the test.
> 
> (And returning -EOPNOTSUPP was never correct as the ioctl is indeed
> supported.)

OK, thanks for clarification. Yes the fix makes sense.

Regards
Oliver

Re: [PATCH 3/3] USB: cdc-acm: fix TIOCGSERIAL implementation

Am Donnerstag, den 08.04.2021, 11:48 +0200 schrieb Johan Hovold:
> On Thu, Apr 08, 2021 at 10:36:46AM +0200, Oliver Neukum wrote:
> > Am Mittwoch, den 07.04.2021, 12:28 +0200 schrieb Johan Hovold:

> > Well, the devices report it. It is part of the standard.
> 
> No, the standard doesn't include anything about a baud-base clock
> AFAICT.

Unfortunately it does.
dwDTERate - chapter 6.3.11 - table 17

If we does this wrongly, we should certainly fix it, but just removing
the reporting doesn't look right to me.

Regards
Oliver

Re: [PATCH 3/4] USB: serial: add support for multi-interface functions

Am Donnerstag, den 01.04.2021, 09:46 +0200 schrieb Johan Hovold:
> On Wed, Mar 31, 2021 at 01:21:15PM +0200, Oliver Neukum wrote:
> > Am Mittwoch, den 31.03.2021, 09:08 +0200 schrieb Oliver Neukum:

> > on the third hand, the more I look at this, would you mind putting
> > sibling_release() with a modified name into usbcore? This functionality
> > is not limited to serial drivers. btusb needs it; cdc-acm needs it;
> > usbaudio neds it. We have code duplication.
> 
> Tell me about it. ;) Unfortunately, drivers all tend to handle this
> slightly different, for example, using a disconnected flag, some claim
> more than one other interface, others look like they may be using their
> interface data as a flag for other purposes, etc.
> 
> At some point we could unify all this but until then I don't think
> putting only half of an interface into core makes much sense.

OK, very well, then let's look at this from a fundamental point
and design a bit. First, can we disregard the case of more than
two interfaces?

Regards
Oliver

Re: [PATCH 1/2] USB:ehci:Add a whitelist for EHCI controllers

Am Donnerstag, den 08.04.2021, 17:11 +0800 schrieb Longfang Liu:
> Some types of EHCI controllers do not have SBRN registers.
> By comparing the white list, the operation of reading the SBRN
> registers is skipped.
> 
> Subsequent EHCI controller types without SBRN registers can be
> directly added to the white list.

Hi,

shouldn't this set a flag for a missing functionality?

Regards
Oliver

Re: [PATCH 2/3] USB: cdc-acm: fix unprivileged TIOCCSERIAL

Am Mittwoch, den 07.04.2021, 12:28 +0200 schrieb Johan Hovold:
> TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
> serial devices is only useful for setting the close_delay and
> closing_wait parameters.
> 
> A non-privileged user has only ever been able to set the since long
> deprecated ASYNC_SPD flags and trying to change any other *supported*
> feature should result in -EPERM being returned. Setting the current
> values for any supported features should return success.
> 
> Fix the cdc-acm implementation which instead indicated that the
> TIOCSSERIAL ioctl was not even implemented when a non-privileged user
> set the current values.

Hi,

the idea here was that you are setting something else, if you are
not changing a parameter that can be changed. That conclusion is
dubious, but at the same time, this implementation can change
only these two parameters. So can the test really be dropped
as opposed to be modified?

    Regards
Oliver

Re: [PATCH 3/3] USB: cdc-acm: fix TIOCGSERIAL implementation

Am Mittwoch, den 07.04.2021, 12:28 +0200 schrieb Johan Hovold:
> TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
> serial devices is only useful for setting the close_delay and
> closing_wait parameters.
> 
> The xmit_fifo_size parameter could be used to set the hardware transmit
> fifo size of a legacy UART when it could not be detected, but the
> interface is limited to eight bits and should be left unset when it is
> not used.

OK.

> Similarly, baud_base could be used to set the uart base clock when it
> could not be detected, but might as well be left unset when it is not
> known.

Well, the devices report it. It is part of the standard.

> Fix the cdc-acm TIOCGSERIAL implementation by dropping its custom
> interpretation of the unused xmit_fifo_size and baud_base fields, which
> overflowed the former with the URB buffer size and set the latter to the
> current line speed. Also return the port line number, which is the only
> other value used besides the close parameters.
> 
> Fixes: 18c75720e667 ("USB: allow users to run setserial with cdc-acm")
> Signed-off-by: Johan Hovold 
> ---
>  drivers/usb/class/cdc-acm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 43e31dad4831..b74713518b3a 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -929,8 +929,7 @@ static int get_serial_info(struct tty_struct *tty, struct 
> serial_struct *ss)
>  {
>   struct acm *acm = tty->driver_data;
>  
> - ss->xmit_fifo_size = acm->writesize;
> - ss->baud_base = le32_to_cpu(acm->line.dwDTERate);

If we do this, we have a value that can be set, but is not reported.
That looks a bit strange to me.

Regards
Oliver

Re: [PATCH net-next v4 0/4] usbnet: speed reporting for devices without MDIO

2021-04-07 Thread Oliver Neukum

Am Dienstag, den 06.04.2021, 18:01 + schrieb Grant Grundler:

> > Ethernet does not support
> > different rates in each direction. So if RX and TX are different, i
> > would actually say something is broken.
> 
> Agreed. The question is: Is there data or some heuristics we can use
> to determine if the physical layer/link is ethernet?
> I'm pessimistic we will be able to since this is at odds with the
> intent of the CDC spec.

Yes, CDC intends to hide that. We can conclude that an asymmetric link
rules out ethernet, but anything else is difficult.

    Regards
Oliver

Re: [PATCH 3/4] USB: serial: add support for multi-interface functions

2021-03-31 Thread Oliver Neukum

Am Mittwoch, den 31.03.2021, 09:08 +0200 schrieb Oliver Neukum:
> Am Dienstag, den 30.03.2021, 17:22 +0200 schrieb Johan Hovold:
> > On Tue, Mar 30, 2021 at 04:44:32PM +0200, Oliver Neukum wrote:
> > > Am Dienstag, den 30.03.2021, 16:38 +0200 schrieb Johan Hovold:
> > > > @@ -1115,6 +1161,8 @@ static void usb_serial_disconnect(struct 
> > > > usb_interface *interface)
> > > > if (serial->type->disconnect)
> > > > serial->type->disconnect(serial);
> > > >  
> > > > +   release_sibling(serial, interface);
> > > > +
> > > > /* let the last holder of this object cause it to be cleaned up 
> > > > */
> > > > usb_serial_put(serial);
> > > > dev_info(dev, "device disconnected\n");
> > > 
> > > Hi,
> > > 
> > > does this assume you are called for the original interface first?
> > 
> > No, I handle either interface being unbound first (e.g. see
> > release_sibling()).
> > 
> > > I am afraid that is an assumption you cannot make. In fact, if somebody
> > > is doing odd things with sysfs you cannot even assume both will see a
> > > disconnect()
> > 
> > Right, but disconnect() will still be called also for the sibling
> > interface as part of release_sibling() above.
> 
> OK, sorry I overlooked that.

Hi,

on the third hand, the more I look at this, would you mind putting
sibling_release() with a modified name into usbcore? This functionality
is not limited to serial drivers. btusb needs it; cdc-acm needs it;
usbaudio neds it. We have code duplication.

Regards
Oliver

Re: [PATCH 3/4] USB: serial: add support for multi-interface functions

2021-03-31 Thread Oliver Neukum

Am Dienstag, den 30.03.2021, 17:22 +0200 schrieb Johan Hovold:
> On Tue, Mar 30, 2021 at 04:44:32PM +0200, Oliver Neukum wrote:
> > Am Dienstag, den 30.03.2021, 16:38 +0200 schrieb Johan Hovold:
> > > @@ -1115,6 +1161,8 @@ static void usb_serial_disconnect(struct 
> > > usb_interface *interface)
> > > if (serial->type->disconnect)
> > > serial->type->disconnect(serial);
> > >  
> > > +   release_sibling(serial, interface);
> > > +
> > > /* let the last holder of this object cause it to be cleaned up */
> > > usb_serial_put(serial);
> > > dev_info(dev, "device disconnected\n");
> > 
> > Hi,
> > 
> > does this assume you are called for the original interface first?
> 
> No, I handle either interface being unbound first (e.g. see
> release_sibling()).
> 
> > I am afraid that is an assumption you cannot make. In fact, if somebody
> > is doing odd things with sysfs you cannot even assume both will see a
> > disconnect()
> 
> Right, but disconnect() will still be called also for the sibling
> interface as part of release_sibling() above.

OK, sorry I overlooked that.

Regards
Oliver

Re: [PATCH 3/4] USB: serial: add support for multi-interface functions

2021-03-30 Thread Oliver Neukum

Am Dienstag, den 30.03.2021, 16:38 +0200 schrieb Johan Hovold:
> @@ -1115,6 +1161,8 @@ static void usb_serial_disconnect(struct usb_interface 
> *interface)
> if (serial->type->disconnect)
> serial->type->disconnect(serial);
>  
> +   release_sibling(serial, interface);
> +
> /* let the last holder of this object cause it to be cleaned up */
> usb_serial_put(serial);
> dev_info(dev, "device disconnected\n");

Hi,

does this assume you are called for the original interface first?
I am afraid that is an assumption you cannot make. In fact, if somebody
is doing odd things with sysfs you cannot even assume both will see a
disconnect()

Regards
Oliver

[PATCH v2] arm64: dts: imx8mm/q: Fix pad control of SD1_DATA0

2021-03-24 Thread Oliver Stäbler

Fix address of the pad control register
(IOMUXC_SW_PAD_CTL_PAD_SD1_DATA0) for SD1_DATA0_GPIO2_IO2.  This seems
to be a typo but it leads to an exception when pinctrl is applied due to
wrong memory address access.

Signed-off-by: Oliver Stäbler 
---
 arch/arm64/boot/dts/freescale/imx8mm-pinfunc.h | 2 +-
 arch/arm64/boot/dts/freescale/imx8mq-pinfunc.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-pinfunc.h 
b/arch/arm64/boot/dts/freescale/imx8mm-pinfunc.h
index 5ccc4cc91959d..a003e6af33533 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-pinfunc.h
+++ b/arch/arm64/boot/dts/freescale/imx8mm-pinfunc.h
@@ -124,7 +124,7 @@
 #define MX8MM_IOMUXC_SD1_CMD_USDHC1_CMD 
0x0A4 0x30C 0x000 0x0 0x0
 #define MX8MM_IOMUXC_SD1_CMD_GPIO2_IO1  
0x0A4 0x30C 0x000 0x5 0x0
 #define MX8MM_IOMUXC_SD1_DATA0_USDHC1_DATA0 
0x0A8 0x310 0x000 0x0 0x0
-#define MX8MM_IOMUXC_SD1_DATA0_GPIO2_IO2
0x0A8 0x31  0x000 0x5 0x0
+#define MX8MM_IOMUXC_SD1_DATA0_GPIO2_IO2
0x0A8 0x310 0x000 0x5 0x0
 #define MX8MM_IOMUXC_SD1_DATA1_USDHC1_DATA1 
0x0AC 0x314 0x000 0x0 0x0
 #define MX8MM_IOMUXC_SD1_DATA1_GPIO2_IO3
0x0AC 0x314 0x000 0x5 0x0
 #define MX8MM_IOMUXC_SD1_DATA2_USDHC1_DATA2 
0x0B0 0x318 0x000 0x0 0x0
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-pinfunc.h 
b/arch/arm64/boot/dts/freescale/imx8mq-pinfunc.h
index b94b02080a344..68e8fa1729741 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-pinfunc.h
+++ b/arch/arm64/boot/dts/freescale/imx8mq-pinfunc.h
@@ -130,7 +130,7 @@
 #define MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD 
0x0A4 0x30C 0x000 0x0 0x0
 #define MX8MQ_IOMUXC_SD1_CMD_GPIO2_IO1  
0x0A4 0x30C 0x000 0x5 0x0
 #define MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0 
0x0A8 0x310 0x000 0x0 0x0
-#define MX8MQ_IOMUXC_SD1_DATA0_GPIO2_IO2
0x0A8 0x31  0x000 0x5 0x0
+#define MX8MQ_IOMUXC_SD1_DATA0_GPIO2_IO2
0x0A8 0x310 0x000 0x5 0x0
 #define MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1 
0x0AC 0x314 0x000 0x0 0x0
 #define MX8MQ_IOMUXC_SD1_DATA1_GPIO2_IO3
0x0AC 0x314 0x000 0x5 0x0
 #define MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2 
0x0B0 0x318 0x000 0x0 0x0
-- 
2.26.2

Re: [kbuild-all] Re: include/linux/compiler_types.h:315:38: error: call to '__compiletime_assert_536' declared with attribute error: BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(st

2021-03-24 Thread Oliver Hartkopp





On 23.03.21 21:54, Rasmus Villemoes wrote:

On 23/03/2021 19.59, Oliver Hartkopp wrote:



On 23.03.21 15:00, Rasmus Villemoes wrote:



Now what CONFIG_* knobs are responsible for putting -mabi=apcs-gnu in
CFLAGS is left as an exercise for the reader. Regardless, it is not a
bug in the compiler. The error is the assumption that this language

"Aggregates and Unions

Structures and unions assume the alignment of their most strictly
aligned component.


(parse error in sentence)


It was a direct quote, but I can try to paraphrase with an example. If
you have a struct foo { T1 m1; T2 m2; T3 m3; }, then alignof(struct foo)
= max(alignof(T1), alignof(T2), alignof(T3)). Same for a "union foo".

But this is specifically for x86-64; for (some flavors of) ARM, other
rules apply - namely, alignof(T) is 4 unless T is char or short (or
(un)signed variants), ignoring bitfields which have their own rules.
Note that while

union u {char a; char b;}

has alignment 4 on ARM and 1 on x86-64, other types are less strictly
aligned on ARM; e.g. s64 aka long long is 8-byte aligned on x86-64 but
(still) just 4-byte aligned on ARM. And again, this is just for specific
-mabi= options.


Each member is assigned to the lowest available offset with the
appropriate
alignment. The size of any object is always a multiple of the object‘s
alignment."

from the x86-64 ABI applies on all other architectures/ABIs.


I'm not a compiler expert but this does not seem to be consistent.

Especially as we only have byte sizes (inside and outside of the union)
and "A field with a char type is aligned to the next available byte."


Yes, and that's exactly what you got before the anon union was
introduced.


Before(!) the union there is nothing to pad.


Just to be clear, my "before" was in the temporal sense, i.e. "prior to
commit ea7800565a128", all the u8s in struct can_frame were placed one
after the other. But after that commit, struct can_frame has a new
member replacing can_dlc which happens to occupy 4 bytes (for some
ABIs), pushing the subsequent members __pad, __res0 and len8_dlc
(formerly known as __res1) ahead.


The union is indeed aligned to the word boundary - but the following
byte is not aligned to the next available byte.


Yes it is, because the union occupies 4 bytes. The first byte is shared
by the two char members, the remaining three bytes are padding.


But why is the union 4 bytes long here and adds a padding of three bytes
at the end?


Essentially, because arrays. It's true for _any_ type T that sizeof(T)
must be a multiple of alignof(T). Take an array "T x[9]". If x[0] is
4-byte aligned, then in order for x[1] to be 4-byte aligned as well,
x[0] must occupy a multiple of 4 bytes.

It doesn't matter at all that this happens to be an anonymous union.
Layout-wise, you could as well have a definition

union uuu { __u8 len; __u8 can_dlc; }

and made struct can_frame

struct can_frame {
canid_t can_id;
union uuu u;
__u8 __pad;
...
};

(you lose the anonymity trick so you'd have to do frame->u.can_dlc
instead of just frame->can_dlc). You have a member with alignof()==4 and
  sizeof()==4; that sizeof() cannot magically become 1 just because that
particular instance of the type is not part of an array. Imagine what
would happen if the compiler pulled subsequent char members into
trailing padding of a previous compound member. E.g. consider

struct a { int x; char y; } // alignof==4, sizeof==8, offsetof(y)==4
struct b { struct a a; char z; }

If I have a "struct b *b", I'm allowed to do ">a" and get a "pointer
to struct a". Then I can do memset(>a, 0, sizeof(struct a)). Clearly,
z must not have been placed inside the trailing padding of struct a.

Rasmus



Thanks Rasmus!

@Marc: Looks like we can not get around the __packed() fix :-(

At least we now have some more documentation to be referenced and I 
would suggest to point out that some compilers handle the union 
alignment like this.


To make clear in the comments what we are suppressing here any why.

Many thanks,
Oliver

[PATCH] arm64: dts: imx8mm: Fix pad control of SD1_DATA0

2021-03-23 Thread Oliver Stäbler

Fix address of the pad control register
(IOMUXC_SW_PAD_CTL_PAD_SD1_DATA0) for SD1_DATA0_GPIO2_IO2.  This seems
to be a typo but it leads to an exception when pinctrl is applied due to
wrong memory address access.

Signed-off-by: Oliver Stäbler 
---
 arch/arm64/boot/dts/freescale/imx8mm-pinfunc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-pinfunc.h 
b/arch/arm64/boot/dts/freescale/imx8mm-pinfunc.h
index 5ccc4cc91959d..a003e6af33533 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-pinfunc.h
+++ b/arch/arm64/boot/dts/freescale/imx8mm-pinfunc.h
@@ -124,7 +124,7 @@
 #define MX8MM_IOMUXC_SD1_CMD_USDHC1_CMD 
0x0A4 0x30C 0x000 0x0 0x0
 #define MX8MM_IOMUXC_SD1_CMD_GPIO2_IO1  
0x0A4 0x30C 0x000 0x5 0x0
 #define MX8MM_IOMUXC_SD1_DATA0_USDHC1_DATA0 
0x0A8 0x310 0x000 0x0 0x0
-#define MX8MM_IOMUXC_SD1_DATA0_GPIO2_IO2
0x0A8 0x31  0x000 0x5 0x0
+#define MX8MM_IOMUXC_SD1_DATA0_GPIO2_IO2
0x0A8 0x310 0x000 0x5 0x0
 #define MX8MM_IOMUXC_SD1_DATA1_USDHC1_DATA1 
0x0AC 0x314 0x000 0x0 0x0
 #define MX8MM_IOMUXC_SD1_DATA1_GPIO2_IO3
0x0AC 0x314 0x000 0x5 0x0
 #define MX8MM_IOMUXC_SD1_DATA2_USDHC1_DATA2 
0x0B0 0x318 0x000 0x0 0x0
-- 
2.29.2

Re: [kbuild-all] Re: include/linux/compiler_types.h:315:38: error: call to '__compiletime_assert_536' declared with attribute error: BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(st





On 23.03.21 15:00, Rasmus Villemoes wrote:

On 23/03/2021 13.49, Oliver Hartkopp wrote:



On 23.03.21 12:36, Rasmus Villemoes wrote:


and more directly from the horse's mouth:

https://developer.arm.com/documentation/dui0067/d/arm-compiler-reference/c-and-c---implementation-details/structures--unions--enumerations--and-bitfields


Field alignment

  Structures are arranged with the first-named component at the lowest
address. Fields are aligned as follows:

  A field with a char type is aligned to the next available byte.

  A field with a short type is aligned to the next even-addressed
byte.

  Bitfield alignment depends on how the bitfield is declared. See
Bitfields in packed structures for more information.

  All other types are aligned on word boundaries.

That anonymous union falls into the "All other types" bullet.

__packed is the documented and standard way to overrule the
compiler's/ABI's layout decisions.


So why is there a difference between

gcc version 10.2.0

and

gcc version 10.2.1 20210110 (Debian 10.2.1-6)


I'm guessing there's no difference between those (in this respect), but
they are invoked differently.


Would this mean that either STRUCTURE_SIZE_BOUNDARY or the command line
option -mstructure_size_boundary=

are set differently?


Yes, though very likely -mstructure_size_boundary is not set explicitly
but via some other option.

gcc has a rather helpful but almost unknown feature that one can
actually query for lots of different parameters and their
default/current values. So on my Ubuntu system (20.04, gcc 9.3), for
example, if I do

$ arm-linux-gnueabihf-gcc -O2 -Q --help=target | grep struct
   -mstructure-size-boundary=8

So that would seem to say that the union should work as expected.
However, when I actually try to compile with the .config that kbuild
reports failing, I do see that BUILD_BUG_ON triggering.

So let us inspect the actual command line used to build some other
random .o file in net/can; look at net/can/.bcm.o.cmd

cmd_net/can/bcm.o := arm-linux-gnueabihf-gcc -Wp,-MMD,net/can/.bcm.o.d
-nostdinc -isystem /usr/lib/gcc-cross/arm-linux-gnueabihf/9/include
-I./arch/arm/include -I./arch/arm/include/generated  -I./include
-I./arch/arm/include/uapi -I./arch/arm/include/generated/uapi
-I./include/uapi -I./include/generated/uapi -include
./include/linux/compiler-version.h -include ./include/linux/kconfig.h
-include ./include/linux/compiler_types.h -D__KERNEL__ -mlittle-endian
-I./arch/arm/mach-footbridge/include -fmacro-prefix-map=./= -Wall
-Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing
-fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration
-Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu89
-fno-dwarf2-cfi-asm -mno-unaligned-access -fno-omit-frame-pointer -mapcs
-mno-sched-prolog -fno-ipa-sra -mabi=apcs-gnu -mno-thumb-interwork -marm
-Wa,-mno-warn-deprecated -D__LINUX_ARM_ARCH__=4 -march=armv4
-mtune=strongarm110 -msoft-float -Uarm -fno-delete-null-pointer-checks
-Wno-frame-address -Wno-format-truncation -Wno-format-overflow
-Wno-address-of-packed-member -O2 --param=allow-store-data-races=0
-Wframe-larger-than=1024 -fno-stack-protector
-Wno-unused-but-set-variable -Wimplicit-fallthrough
-Wno-unused-const-variable -fno-omit-frame-pointer
-fno-optimize-sibling-calls -fno-inline-functions-called-once
-Wdeclaration-after-statement -Wvla -Wno-pointer-sign
-Wno-stringop-truncation -Wno-array-bounds -Wno-stringop-overflow
-Wno-restrict -Wno-maybe-uninitialized -fno-strict-overflow
-fno-stack-check -fconserve-stack -Werror=date-time
-Werror=incompatible-pointer-types -Werror=designated-init
-Wno-packed-not-aligned-fsanitize-coverage=trace-pc
-DKBUILD_MODFILE='"net/can/can-bcm"' -DKBUILD_BASENAME='"bcm"'
-DKBUILD_MODNAME='"can_bcm"' -D__KBUILD_MODNAME=kmod_can_bcm -c -o
net/can/bcm.o net/can/bcm.c

Lots of gunk. But just to see if one of those options have affected the
-mstructure-size-boundary= value, just take that whole command line and
throw in -Q --help=target at the end, and we get

   -mstructure-size-boundary=32

So let us guess that it's the ABI choice -mabi=apcs-gnu

$ arm-linux-gnueabihf-gcc -O2 -msoft-float -mabi=apcs-gnu -Q
--help=target | grep struct
   -mstructure-size-boundary=32

Bingo. (-msoft-float is also included just as in the real command line
because gcc barfs otherwise).


Thanks for all the comprehensive explanations!


Now what CONFIG_* knobs are responsible for putting -mabi=apcs-gnu in
CFLAGS is left as an exercise for the reader. Regardless, it is not a
bug in the compiler. The error is the assumption that this language

"Aggregates and Unions

Structures and unions assume the alignment of their most strictly
aligned component.


(parse error in sentence)


Each member is assigned to the lowest available offset with the appropriate
alignment. The size of an

Re: [kbuild-all] Re: include/linux/compiler_types.h:315:38: error: call to '__compiletime_assert_536' declared with attribute error: BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(st

On 23.03.21 12:36, Rasmus Villemoes wrote:

On 23/03/2021 08.45, Oliver Hartkopp wrote:

IMO we facing a compiler problem here - and we should be very happy that
the BUILD_BUG_ON() triggered an issue after years of silence.

I do not have a good feeling about what kind of strange effects this
compiler issue might have in other code of other projects.

So I would explicitly suggest NOT to change the af_can.c code to work
around this compiler issue.

Let the gcc people fix their product and let them thank all of us for
detecting it.

I'm sure you'd be eligible for a full refund in case this was a bug in
gcc. It is not. It's a pretty clear ABI requirement for (at least some
flavors of) ARM:

https://stackoverflow.com/questions/43786747/struct-layout-in-apcs-gnu-abi

and more directly from the horse's mouth:

https://developer.arm.com/documentation/dui0067/d/arm-compiler-reference/c-and-c---implementation-details/structures--unions--enumerations--and-bitfields

Field alignment

Structures are arranged with the first-named component at the lowest
address. Fields are aligned as follows:

A field with a char type is aligned to the next available byte.

A field with a short type is aligned to the next even-addressed
byte.

Bitfield alignment depends on how the bitfield is declared. See
Bitfields in packed structures for more information.

All other types are aligned on word boundaries.

That anonymous union falls into the "All other types" bullet.

__packed is the documented and standard way to overrule the
compiler's/ABI's layout decisions.

So why is there a difference between

gcc version 10.2.0

and

gcc version 10.2.1 20210110 (Debian 10.2.1-6)

https://lore.kernel.org/linux-can/20210323073437.yo63wreqnubbe...@pengutronix.de/

Would this mean that either STRUCTURE_SIZE_BOUNDARY or the command line
option -mstructure_size_boundary=

are set differently?

https://stackoverflow.com/questions/43786747/struct-layout-in-apcs-gnu-abi/43829053#43829053

I'm not a compiler expert but this does not seem to be consistent.

Especially as we only have byte sizes (inside and outside of the union)
and "A field with a char type is aligned to the next available byte."

The union is indeed aligned to the word boundary - but the following
byte is not aligned to the next available byte.

Regards,
Oliver

Re: [kbuild-all] Re: include/linux/compiler_types.h:315:38: error: call to '__compiletime_assert_536' declared with attribute error: BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(st


Answering myself ...

On 23.03.21 08:45, Oliver Hartkopp wrote:

On 23.03.21 08:34, Marc Kleine-Budde wrote:

On 23.03.2021 10:54:40, Rong Chen wrote:

I tried arm-linux-gnueabi (gcc version 10.2.0) and the problem still
exists, btw we prefer to not use the latest gcc compiler to avoid
false positives.


FWIW:

I'm using latest debian arm compiler and the BUILD_BUG never triggered.
gcc version 10.2.1 20210110 (Debian 10.2.1-6)



@Rong / Marc:

I wonder if the compiler configurations (gcc -v) or the options used at 
kernel build time are identical.


Maybe there is a different optimization option selected which causes the 
compiler to extend the u8 union to a 32 bit space?!?


And maybe Debian is a bit more conservative in selecting their 
optimizations than the setup that Rong was using for the build ...


Best,
Oliver



Thanks Marc!

IMO we facing a compiler problem here - and we should be very happy that 
the BUILD_BUG_ON() triggered an issue after years of silence.


I do not have a good feeling about what kind of strange effects this 
compiler issue might have in other code of other projects.


So I would explicitly suggest NOT to change the af_can.c code to work 
around this compiler issue.


Let the gcc people fix their product and let them thank all of us for 
detecting it.


Regards,
Oliver

Re: [kbuild-all] Re: include/linux/compiler_types.h:315:38: error: call to '__compiletime_assert_536' declared with attribute error: BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(st


On 23.03.21 08:34, Marc Kleine-Budde wrote:

On 23.03.2021 10:54:40, Rong Chen wrote:

I tried arm-linux-gnueabi (gcc version 10.2.0) and the problem still
exists, btw we prefer to not use the latest gcc compiler to avoid
false positives.


FWIW:

I'm using latest debian arm compiler and the BUILD_BUG never triggered.
gcc version 10.2.1 20210110 (Debian 10.2.1-6)



Thanks Marc!

IMO we facing a compiler problem here - and we should be very happy that 
the BUILD_BUG_ON() triggered an issue after years of silence.


I do not have a good feeling about what kind of strange effects this 
compiler issue might have in other code of other projects.


So I would explicitly suggest NOT to change the af_can.c code to work 
around this compiler issue.


Let the gcc people fix their product and let them thank all of us for 
detecting it.


Regards,
Oliver

Re: [kbuild-all] Re: include/linux/compiler_types.h:315:38: error: call to '__compiletime_assert_536' declared with attribute error: BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(st

2021-03-22 Thread Oliver Hartkopp


Hi Rong,

On 22.03.21 09:52, Rong Chen wrote:


On 3/21/21 10:19 PM, Oliver Hartkopp wrote:

Two reminders in two days? ;-)

Did you check my answer here?
https://lore.kernel.org/lkml/afffeb73-ba4c-ca2c-75d0-9e7899e5c...@hartkopp.net/ 



And did you try the partly revert?


Hi Oliver,

Sorry for the delay, we tried the revert patch and the problem still 
exists,
we also found that commit c7b74967 changed the error message which 
triggered

the report.

The problem is that offsetof(struct can_frame, data) != offsetof(struct 
canfd_frame, data)
the following struct layout shows that the offset has been changed by 
union:


struct can_frame {
     canid_t    can_id;   /* 0 4 */
     union {
     __u8   len;  /* 4 1 */
     __u8   can_dlc;  /* 4 1 */
     };   /* 4 4 */


Ugh! Why did the compiler extend the space for the union to 4 bytes?!?


     __u8   __pad;    /* 8 1 */
     __u8   __res0;   /* 9 1 */
     __u8   len8_dlc; /* 10 1 */

     /* XXX 5 bytes hole, try to pack */

     __u8   data[8] 
__attribute__((__aligned__(8))); /*    16 8 */


     /* size: 24, cachelines: 1, members: 6 */
     /* sum members: 19, holes: 1, sum holes: 5 */
     /* forced alignments: 1, forced holes: 1, sum forced holes: 5 */
     /* last cacheline: 24 bytes */
} __attribute__((__aligned__(8)));

struct canfd_frame {
     canid_t    can_id;   /* 0 4 */
     __u8   len;  /* 4 1 */
     __u8   flags;    /* 5 1 */
     __u8   __res0;   /* 6 1 */
     __u8   __res1;   /* 7 1 */
     __u8   data[64] 
__attribute__((__aligned__(8))); /* 8    64 */


     /* size: 72, cachelines: 2, members: 6 */
     /* forced alignments: 1 */
     /* last cacheline: 8 bytes */
} __attribute__((__aligned__(8)))


and I tried to add "__attribute__((packed))" to the union, the issue is 
gone:


diff --git a/include/uapi/linux/can.h b/include/uapi/linux/can.h
index f75238ac6dce..9842bb55ffd9 100644
--- a/include/uapi/linux/can.h
+++ b/include/uapi/linux/can.h
@@ -113,7 +113,7 @@ struct can_frame {
  */
     __u8 len;
     __u8 can_dlc; /* deprecated */
-   };
+   } __attribute__((packed));
     __u8 __pad; /* padding */
     __u8 __res0; /* reserved / padding */
     __u8 len8_dlc; /* optional DLC for 8 byte payload length (9 .. 
15) */


This is pretty strange!

pahole on my x86_64 machine shows the correct data structure layout:

struct can_frame {
canid_tcan_id;   /* 0 4 */
union {
__u8   len;  /* 4 1 */
__u8   can_dlc;  /* 4 1 */
};   /* 4 1 */
__u8   __pad;/* 5 1 */
__u8   __res0;   /* 6 1 */
__u8   len8_dlc; /* 7 1 */
__u8   data[8] 
__attribute__((__aligned__(8))); /* 8 8 */


/* size: 16, cachelines: 1, members: 6 */
/* forced alignments: 1 */
/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));

Target: x86_64-linux-gnu
gcc version 10.2.1 20210110 (Debian 10.2.1-6)
Linux 5.12.0-rc3-00070-g8b12a62a4e3e x86_64 GNU/Linux

So it looks like your compiler does not behave correctly - and I wonder 
if it would be the correct approach to add the __packed() attribute or 
better fix/change the (ARM) compiler.


At least I'm very happy that the BUILD_BUG_ON() triggered correctly - so 
it was worth to have it ;-)


Best regards,
Oliver




Maybe there's a mismatch in include files - or BUILD_BUG_ON() 
generally does not work with unions on ARM as assumed here:


https://lore.kernel.org/lkml/6e57d5d2-9b88-aee6-fb7a-82e24144d...@hartkopp.net/ 



In both cases I can not really fix the issue.
When the partly revert (suggested above) works, this would be a hack too.

Best,
Oliver

On 20.03.21 21:43, kernel test robot wrote:

Hi Oliver,

FYI, the error/warning still remains.

tree: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master

head:   812da4d39463a060738008a46cfc9f775e4bfcf6
commit: c7b74967799b1af52b3045d69d4c26836b2d41de can: replace can_dlc 
as variable/element for payload length

date:   4 months ago
config: arm-randconfig-r016-20210321 (attached

Re: [PATCH 7/7] USB: cdc-acm: always claim data interface

Am Donnerstag, den 18.03.2021, 16:52 +0100 schrieb Johan Hovold:
> Make sure to always claim the data interface and bail out if it's
> already bound to another driver or isn't authorised.

Hi,

Thanks for the fixes. All previous ones are good work.
this one is problematic I am afraid.


acm_probe() has a test for the availability of the interface:

if (!combined_interfaces && usb_interface_claimed(data_interface)) {
/* valid in this context */
dev_dbg(>dev, "The data interface isn't available\n");
return -EBUSY;
}

That check is ancient. BKL still existed. If you want to remove it
and do proper error handling, that would be good. But if you do
error handling, the check has to go, too.

    Regards
Oliver

Re: [PATCH 6/7] USB: cdc-acm: use negation for NULL checks

Am Donnerstag, den 18.03.2021, 16:52 +0100 schrieb Johan Hovold:
> Use negation consistently throughout the driver for NULL checks.
> 
> Signed-off-by: Johan Hovold 
Acked-by: Oliver Neukum

Re: [PATCH 5/7] USB: cdc-acm: clean up probe error labels

Am Donnerstag, den 18.03.2021, 16:52 +0100 schrieb Johan Hovold:
> Name the probe error labels after what they do rather than using
> sequence numbers which is harder to review and maintain (e.g. may
> require renaming unrelated labels when a label is added or removed).
> 
> Signed-off-by: Johan Hovold 
Acked-by: Oliver Neukum

Re: [PATCH 4/7] USB: cdc-acm: drop redundant driver-data reset

Am Donnerstag, den 18.03.2021, 16:51 +0100 schrieb Johan Hovold:
> There's no need to clear the interface driver data on failed probe (and
> driver core will clear it anyway).
> 
> Signed-off-by: Johan Hovold 
Acked-by: Oliver Neukum

Re: [PATCH 3/7] USB: cdc-acm: drop redundant driver-data assignment

Am Donnerstag, den 18.03.2021, 16:51 +0100 schrieb Johan Hovold:
> The interface driver data has already been set by
> usb_driver_claim_interface() so drop the redundant subsequent
> assignment.
> 
> Signed-off-by: Johan Hovold 
Acked-by: Oliver Neukum

Re: [PATCH 2/7] USB: cdc-acm: fix use-after-free after probe failure

Am Donnerstag, den 18.03.2021, 16:51 +0100 schrieb Johan Hovold:
> If tty-device registration fails the driver would fail to release the
> data interface. When the device is later disconnected, the disconnect
> callback would still be called for the data interface and would go about
> releasing already freed resources.
> 
> Fixes: c93d81955005 ("usb: cdc-acm: fix error handling in acm_probe()")
> Cc: sta...@vger.kernel.org  # 3.9
> Cc: Alexey Khoroshilov 
> Signed-off-by: Johan Hovold ]
Acked-by: Oliver Neukum

Re: [PATCH 1/7] USB: cdc-acm: fix double free on probe failure

Am Donnerstag, den 18.03.2021, 16:51 +0100 schrieb Johan Hovold:
> If tty-device registration fails the driver copy of any Country
> Selection functional descriptor would end up being freed twice; first
> explicitly in the error path and then again in the tty-port destructor.
> 
> Drop the first erroneous free that was left when fixing a tty-port
> resource leak.
> 
> Fixes: cae2bc768d17 ("usb: cdc-acm: Decrement tty port's refcount if probe() 
> fail")
> Cc: sta...@vger.kernel.org  # 4.19
> Cc: Jaejoong Kim 
> Signed-off-by: Johan Hovold 
Acked-by: Oliver Neukum

Re: [selftests] e48d82b67a: BUG_TestSlub_RZ_alloc(Not_tainted):Redzone_overwritten

2021-03-21 Thread Oliver Sang

Hi Vlastimil,

On Wed, Mar 17, 2021 at 12:29:40PM +0100, Vlastimil Babka wrote:
> On 3/17/21 9:36 AM, kernel test robot wrote:
> > 
> > 
> > Greeting,
> > 
> > FYI, we noticed the following commit (built with gcc-9):
> > 
> > commit: e48d82b67a2b760eedf7b95ca15f41267496386c ("[PATCH 1/2] selftests: 
> > add a kselftest for SLUB debugging functionality")
> > url: 
> > https://github.com/0day-ci/linux/commits/glittao-gmail-com/selftests-add-a-kselftest-for-SLUB-debugging-functionality/20210316-204257
> > base: 
> > https://git.kernel.org/cgit/linux/kernel/git/shuah/linux-kselftest.git next
> > 
> > in testcase: trinity
> > version: trinity-static-i386-x86_64-f93256fb_2019-08-28
> > with following parameters:
> > 
> > group: group-04
> > 
> > test-description: Trinity is a linux system call fuzz tester.
> > test-url: http://codemonkey.org.uk/projects/trinity/
> > 
> > 
> > on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
> > 
> > caused below changes (please refer to attached dmesg/kmsg for entire 
> > log/backtrace):
> > 
> > 
> > +---+---++
> > |   
> > | v5.12-rc2 | e48d82b67a |
> > +---+---++
> > | BUG_TestSlub_RZ_alloc(Not_tainted):Redzone_overwritten
> > | 0 | 69 |
> > | INFO:0x(ptrval)-0x(ptrval)@offset=#.First_byte#instead_of 
> > | 0 | 69 |
> > | INFO:Allocated_in_resiliency_test_age=#cpu=#pid=  
> > | 0 | 69 |
> > | INFO:Slab0x(ptrval)objects=#used=#fp=0x(ptrval)flags= 
> > | 0 | 69 |
> > | INFO:Object0x(ptrval)@offset=#fp=0x(ptrval)   
> > | 0 | 69 |
> > | BUG_TestSlub_next_ptr_free(Tainted:G_B):Freechain_corrupt 
> > | 0 | 69 |
> > | INFO:Freed_in_resiliency_test_age=#cpu=#pid=  
> > | 0 | 69 |
> > | 
> > BUG_TestSlub_next_ptr_free(Tainted:G_B):Wrong_object_count.Counter_is#but_counted_were
> > | 0 | 69 |
> > | BUG_TestSlub_next_ptr_free(Tainted:G_B):Redzone_overwritten   
> > | 0 | 69 |
> > | 
> > BUG_TestSlub_next_ptr_free(Tainted:G_B):Objects_remaining_in_TestSlub_next_ptr_free_on__kmem_cache_shutdown()
> >  | 0 | 69 |
> > | INFO:Object0x(ptrval)@offset= 
> > | 0 | 69 |
> > | BUG_TestSlub_1th_word_free(Tainted:G_B):Poison_overwritten
> > | 0 | 69 |
> > | BUG_TestSlub_50th_word_free(Tainted:G_B):Poison_overwritten   
> > | 0 | 69 |
> > | BUG_TestSlub_RZ_free(Tainted:G_B):Redzone_overwritten 
> > | 0 | 69 |
> > +---+---++
> > 
> > 
> > If you fix the issue, kindly add following tag
> > Reported-by: kernel test robot 
> > 
> > 
> > 
> > [   22.154049] random: get_random_u32 called from 
> > __kmem_cache_create+0x23/0x3e0 with crng_init=0 
> > [   22.154070] random: get_random_u32 called from 
> > cache_random_seq_create+0x7c/0x140 with crng_init=0 
> > [   22.154167] random: get_random_u32 called from allocate_slab+0x155/0x5e0 
> > with crng_init=0 
> > [   22.154690] test_slub: 1. kmem_cache: Clobber Redzone 0x12->0x(ptrval)
> > [   22.164499] 
> > =
> > [   22.166629] BUG TestSlub_RZ_alloc (Not tainted): Redzone overwritten
> > [   22.168179] 
> > -
> > [   22.168179]
> > [   22.168372] Disabling lock debugging due to kernel taint
> > [   22.168372] INFO: 0x(ptrval)-0x(ptrval) @offset=1064. First byte 0x12 
> > instead of 0xcc
> > [   22.168372] INFO: Allocated in resiliency_test+0x47/0x1be age=3 cpu=0 
> > pid=1 
> > [   22.168372] __slab_alloc+0x57/0x80 
> > [   22.168372] kmem_cache_alloc (kbuild/src/consumer/mm/slub.c:2871 
> > kbuild/src/consumer/mm/slub.c:2915 kbuild/src/consumer/mm/slub.c:2920) 
> > [

Re: include/linux/compiler_types.h:315:38: error: call to '__compiletime_assert_536' declared with attribute error: BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(struct canfd_frame,

2021-03-21 Thread Oliver Hartkopp


Two reminders in two days? ;-)

Did you check my answer here?
https://lore.kernel.org/lkml/afffeb73-ba4c-ca2c-75d0-9e7899e5c...@hartkopp.net/

And did you try the partly revert?

Maybe there's a mismatch in include files - or BUILD_BUG_ON() generally 
does not work with unions on ARM as assumed here:


https://lore.kernel.org/lkml/6e57d5d2-9b88-aee6-fb7a-82e24144d...@hartkopp.net/

In both cases I can not really fix the issue.
When the partly revert (suggested above) works, this would be a hack too.

Best,
Oliver

On 20.03.21 21:43, kernel test robot wrote:

Hi Oliver,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   812da4d39463a060738008a46cfc9f775e4bfcf6
commit: c7b74967799b1af52b3045d69d4c26836b2d41de can: replace can_dlc as 
variable/element for payload length
date:   4 months ago
config: arm-randconfig-r016-20210321 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
 wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
 chmod +x ~/bin/make.cross
 # 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c7b74967799b1af52b3045d69d4c26836b2d41de
 git remote add linus 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
 git fetch --no-tags linus master
 git checkout c7b74967799b1af52b3045d69d4c26836b2d41de
 # save the attached .config to linux build tree
 COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

In file included from :
net/can/af_can.c: In function 'can_init':

include/linux/compiler_types.h:315:38: error: call to 
'__compiletime_assert_536' declared with attribute error: BUILD_BUG_ON failed: 
offsetof(struct can_frame, len) != offsetof(struct canfd_frame, len) || 
offsetof(struct can_frame, data) != offsetof(struct canfd_frame, data)

  315 |  _compiletime_assert(condition, msg, __compiletime_assert_, 
__COUNTER__)
  |  ^
include/linux/compiler_types.h:296:4: note: in definition of macro 
'__compiletime_assert'
  296 |prefix ## suffix();\
  |^~
include/linux/compiler_types.h:315:2: note: in expansion of macro 
'_compiletime_assert'
  315 |  _compiletime_assert(condition, msg, __compiletime_assert_, 
__COUNTER__)
  |  ^~~
include/linux/build_bug.h:39:37: note: in expansion of macro 
'compiletime_assert'
   39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
  | ^~
include/linux/build_bug.h:50:2: note: in expansion of macro 
'BUILD_BUG_ON_MSG'
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
  |  ^~~~
net/can/af_can.c:891:2: note: in expansion of macro 'BUILD_BUG_ON'
  891 |  BUILD_BUG_ON(offsetof(struct can_frame, len) !=
  |  ^~~~


vim +/__compiletime_assert_536 +315 include/linux/compiler_types.h

eb5c2d4b45e3d2 Will Deacon 2020-07-21  301
eb5c2d4b45e3d2 Will Deacon 2020-07-21  302  #define 
_compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21  303  __compiletime_assert(condition, 
msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  304
eb5c2d4b45e3d2 Will Deacon 2020-07-21  305  /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21  306   * compiletime_assert - break build 
and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  307   * @condition: a compile-time 
constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21  308   * @msg:   a message to emit if 
condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  309   *
eb5c2d4b45e3d2 Will Deacon 2020-07-21  310   * In tradition of POSIX assert, 
this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  311   * supplied condition is *false*, 
emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  312   * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21  313   */
eb5c2d4b45e3d2 Will Deacon 2020-07-21  314  #define 
compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @315  _compiletime_assert(condition, 
msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  316

:: The code at line 315 was first introduced by commit
:: eb5c2d4b45e3d2d5d052ea6b8f1463976b1020d5 compiler.h: Move 
compiletime_assert() macros into compiler_types.h

:: TO: Will Deacon 
:: CC: Will Deacon 

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org

Re: include/linux/compiler_types.h:315:38: error: call to '__compiletime_assert_511' declared with attribute error: BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(struct canfd_frame,

2021-03-19 Thread Oliver Hartkopp





On 19.03.21 09:06, kernel test robot wrote:

Hi Oliver,

FYI, the error/warning still remains.



Hm - I have no clue either.


tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   8b12a62a4e3ed4ae99c715034f557eb391d6b196
commit: c7b74967799b1af52b3045d69d4c26836b2d41de can: replace can_dlc as 
variable/element for payload length


The patch which introduced the union I suspected to be the problem is 
some commits earlier ...



net/can/af_can.c:891:2: note: in expansion of macro 'BUILD_BUG_ON'
  891 |  BUILD_BUG_ON(offsetof(struct can_frame, len) !=
  |  ^~~~


The only idea which does not change the functionality but may help the 
macro expansion is to revert the change from "can_dlc" -> "len" in 
af_can.c :


https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/diff/net/can/af_can.c?id=c7b74967799b1af52b3045d69d4c26836b2d41de

Is it possible for you to revert that single line for a test?

diff --git a/net/can/af_can.c b/net/can/af_can.c
index cce2af10eb3e..1c95ede2c9a6 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -865,11 +865,11 @@ static struct pernet_operations can_pernet_ops 
__read_mostly = {

 static __init int can_init(void)
 {
int err;

/* check for correct padding to be able to use the structs 
similarly */

-   BUILD_BUG_ON(offsetof(struct can_frame, len) !=
+   BUILD_BUG_ON(offsetof(struct can_frame, can_dlc) !=
 offsetof(struct canfd_frame, len) ||
 offsetof(struct can_frame, data) !=
 offsetof(struct canfd_frame, data));

pr_info("can: controller area network core\n");

Unfortunately I was not able to reproduce the issue here.

Best regards,
Oliver






vim +/__compiletime_assert_511 +315 include/linux/compiler_types.h

eb5c2d4b45e3d2 Will Deacon 2020-07-21  301
eb5c2d4b45e3d2 Will Deacon 2020-07-21  302  #define 
_compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21  303  __compiletime_assert(condition, 
msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  304
eb5c2d4b45e3d2 Will Deacon 2020-07-21  305  /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21  306   * compiletime_assert - break build 
and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  307   * @condition: a compile-time 
constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21  308   * @msg:   a message to emit if 
condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  309   *
eb5c2d4b45e3d2 Will Deacon 2020-07-21  310   * In tradition of POSIX assert, 
this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  311   * supplied condition is *false*, 
emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  312   * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21  313   */
eb5c2d4b45e3d2 Will Deacon 2020-07-21  314  #define 
compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @315  _compiletime_assert(condition, 
msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  316

:: The code at line 315 was first introduced by commit
:: eb5c2d4b45e3d2d5d052ea6b8f1463976b1020d5 compiler.h: Move 
compiletime_assert() macros into compiler_types.h

:: TO: Will Deacon 
:: CC: Will Deacon 

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org

Re: [mm] 8fd8d23ab1: WARNING:at_fs/buffer.c:#__brelse

2021-03-19 Thread Oliver Sang

Hi Minchan,

On Wed, Mar 17, 2021 at 09:29:38AM -0700, Minchan Kim wrote:
> On Wed, Mar 17, 2021 at 10:37:57AM +0800, kernel test robot wrote:
> > 
> > 
> > Greeting,
> > 
> > FYI, we noticed the following commit (built with gcc-9):
> > 
> > commit: 8fd8d23ab10cc2fceeac25ea7b0e2eaf98e78d64 ("[PATCH v3 3/3] mm: fs: 
> > Invalidate BH LRU during page migration")
> > url: 
> > https://github.com/0day-ci/linux/commits/Minchan-Kim/mm-replace-migrate_prep-with-lru_add_drain_all/20210311-001714
> > base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 
> > 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32
> > 
> > in testcase: blktests
> > version: blktests-x86_64-a210761-1_20210124
> > with following parameters:
> > 
> > test: nbd-group-01
> > ucode: 0xe2
> > 
> > 
> > 
> > on test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz with 32G 
> > memory
> > 
> > caused below changes (please refer to attached dmesg/kmsg for entire 
> > log/backtrace):
> > 
> > 
> > 
> > If you fix the issue, kindly add following tag
> > Reported-by: kernel test robot 
> > 
> > 
> > [   40.465061] WARNING: CPU: 2 PID: 5207 at fs/buffer.c:1177 __brelse 
> > (kbuild/src/consumer/fs/buffer.c:1177 kbuild/src/consumer/fs/buffer.c:1171) 
> > [   40.465066] Modules linked in: nbd loop xfs libcrc32c dm_multipath 
> > dm_mod ipmi_devintf ipmi_msghandler sd_mod t10_pi sg intel_rapl_msr 
> > intel_rapl_common x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel 
> > kvm irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel 
> > ghash_clmulni_intel rapl i915 mei_wdt intel_cstate wmi_bmof intel_gtt 
> > drm_kms_helper syscopyarea ahci intel_uncore sysfillrect sysimgblt libahci 
> > fb_sys_fops drm libata mei_me mei intel_pch_thermal wmi video 
> > intel_pmc_core acpi_pad ip_tables
> > [   40.465086] CPU: 2 PID: 5207 Comm: mount_clear_soc Tainted: G  I 
> >   5.12.0-rc2-00062-g8fd8d23ab10c #1
> > [   40.465088] Hardware name: Dell Inc. OptiPlex 7040/0Y7WYT, BIOS 1.1.1 
> > 10/07/2015
> > [   40.465089] RIP: 0010:__brelse (kbuild/src/consumer/fs/buffer.c:1177 
> > kbuild/src/consumer/fs/buffer.c:1171) 
> > [ 40.465091] Code: 00 00 00 00 00 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 8b 
> > 47 60 85 c0 74 05 f0 ff 4f 60 c3 48 c7 c7 d8 99 57 82 e8 02 5d 80 00 <0f> 
> > 0b c3 0f 1f 44 00 00 55 65 ff 05 13 79 c8 7e 53 48 c7 c3 c0 89
> 
> Hi,
> 
> Unfortunately, I couldn't set the lkp test in my local mahcine
> since installation failed(I guess my linux distribution is
> very minor)
> 
> Do you mind testing this patch? (Please replace the original
> patch with this one)

by replacing the original patch with below one, we confirmed the issue fixed. 
Thanks

> 
> From 23cfe5a8e939e2c077223e009887af8a0b5d6381 Mon Sep 17 00:00:00 2001
> From: Minchan Kim 
> Date: Tue, 2 Mar 2021 12:05:08 -0800
> Subject: [PATCH] mm: fs: Invalidate BH LRU during page migration
> 
> Pages containing buffer_heads that are in one of the per-CPU
> buffer_head LRU caches will be pinned and thus cannot be migrated.
> This can prevent CMA allocations from succeeding, which are often used
> on platforms with co-processors (such as a DSP) that can only use
> physically contiguous memory. It can also prevent memory
> hot-unplugging from succeeding, which involves migrating at least
> MIN_MEMORY_BLOCK_SIZE bytes of memory, which ranges from 8 MiB to 1
> GiB based on the architecture in use.
> 
> Correspondingly, invalidate the BH LRU caches before a migration
> starts and stop any buffer_head from being cached in the LRU caches,
> until migration has finished.
> 
> Signed-off-by: Chris Goldsworthy 
> Signed-off-by: Minchan Kim 
> ---
>  fs/buffer.c | 36 ++--
>  include/linux/buffer_head.h |  4 
>  mm/swap.c   |  5 -
>  3 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 0cb7ffd4977c..e9872d0dcbf1 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -1264,6 +1264,15 @@ static void bh_lru_install(struct buffer_head *bh)
>   int i;
>  
>   check_irqs_on();
> + /*
> +  * the refcount of buffer_head in bh_lru prevents dropping the
> +  * attached page(i.e., try_to_free_buffers) so it could cause
> +  * failing page migration.
> +  * Skip putting upcoming bh into bh_lru until migration is done.
> +  */
> + if (lru_cache_disabled())
> + return;
> +
>   bh_lru_lock();
>  
>   b = this_cpu_ptr(_lrus);
> @@ -1404,6 +1413,15 @@ __bread_gfp(struct block_device *bdev, sector_t block,
>  }
>  EXPORT_SYMBOL(__bread_gfp);
>  
> +static void __invalidate_bh_lrus(struct bh_lru *b)
> +{
> + int i;
> +
> + for (i = 0; i < BH_LRU_SIZE; i++) {
> + brelse(b->bhs[i]);
> + b->bhs[i] = NULL;
> + }
> +}
>  /*
>   * invalidate_bh_lrus() is called rarely - but not only at unmount.
>   * This doesn't race because it runs in each cpu either in irq
> @@ -1412,16 +1430,12

Re: [vdpa_sim_net] 79991caf52: net/ipv4/ipmr.c:#RCU-list_traversed_in_non-reader_section

2021-03-18 Thread Oliver Sang

; | 0  | 1  |
> >> | Kernel_panic-not_syncing:Fatal_exception
> >> | 0  | 1  |
> >> | net/ipv4/ipmr.c:#RCU-list_traversed_in_non-reader_section   
> >> | 0  | 8  |
> >> | RIP:arch_local_irq_restore  
> >> | 0  | 1  |
> >> | RIP:idr_get_free
> >> | 0  | 1  |
> >> | net/ipv6/ip6mr.c:#RCU-list_traversed_in_non-reader_section  
> >> | 0  | 2  |
> >> +-+++
> >>
> >>
> >> If you fix the issue, kindly add following tag
> >> Reported-by: kernel test robot 
> >>
> >>
> >> [  890.196279] =
> >> [  890.212608] WARNING: suspicious RCU usage
> >> [  890.228281] 5.11.0-rc4-8-g79991caf5202 #1 Tainted: GW
> >> [  890.244087] -
> >> [  890.259417] net/ipv4/ipmr.c:138 RCU-list traversed in non-reader 
> >> section!!
> >> [  890.275043]
> >> [  890.275043] other info that might help us debug this:
> >> [  890.275043]
> >> [  890.318497]
> >> [  890.318497] rcu_scheduler_active = 2, debug_locks = 1
> >> [  890.346089] 2 locks held by trinity-c1/2476:
> >> [  890.360897]  #0: 888149d6f400 (>f_pos_lock){+.+.}-{3:3}, at: 
> >> __fdget_pos+0xc0/0xe0
> >> [  890.375165]  #1: 8881cabfd5c8 (>lock){+.+.}-{3:3}, at: 
> >> seq_read_iter+0xa0/0x9c0
> >> [  890.389706]
> >> [  890.389706] stack backtrace:
> >> [  890.416375] CPU: 1 PID: 2476 Comm: trinity-c1 Tainted: GW   
> >>   5.11.0-rc4-8-g79991caf5202 #1
> >> [  890.430706] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> >> 1.12.0-1 04/01/2014
> >> [  890.444971] Call Trace:
> >> [  890.458554]  dump_stack+0x15f/0x1bf
> >> [  890.471996]  ipmr_get_table+0x140/0x160
> >> [  890.485328]  ipmr_vif_seq_start+0x4d/0xe0
> >> [  890.498620]  seq_read_iter+0x1b2/0x9c0
> >> [  890.511469]  ? kvm_sched_clock_read+0x14/0x40
> >> [  890.524008]  ? sched_clock+0x1b/0x40
> >> [  890.536095]  ? iov_iter_init+0x7c/0xa0
> >> [  890.548028]  seq_read+0x2fd/0x3e0
> >> [  890.559948]  ? seq_hlist_next_percpu+0x140/0x140
> >> [  890.572204]  ? should_fail+0x78/0x2a0
> >> [  890.584189]  ? write_comp_data+0x2a/0xa0
> >> [  890.596235]  ? __sanitizer_cov_trace_pc+0x1d/0x60
> >> [  890.608134]  ? seq_hlist_next_percpu+0x140/0x140
> >> [  890.620042]  proc_reg_read+0x14e/0x180
> >> [  890.631585]  do_iter_read+0x397/0x420
> >> [  890.642843]  vfs_readv+0xf5/0x160
> >> [  890.653833]  ? vfs_iter_read+0x80/0x80
> >> [  890.664229]  ? __fdget_pos+0xc0/0xe0
> >> [  890.674236]  ? pvclock_clocksource_read+0xd9/0x1a0
> >> [  890.684259]  ? kvm_sched_clock_read+0x14/0x40
> >> [  890.693852]  ? sched_clock+0x1b/0x40
> >> [  890.702898]  ? sched_clock_cpu+0x18/0x120
> >> [  890.711648]  ? write_comp_data+0x2a/0xa0
> >> [  890.720243]  ? __sanitizer_cov_trace_pc+0x1d/0x60
> >> [  890.729290]  do_readv+0x111/0x260
> >> [  890.738205]  ? vfs_readv+0x160/0x160
> >> [  890.747154]  ? lockdep_hardirqs_on+0x77/0x100
> >> [  890.756100]  ? syscall_enter_from_user_mode+0x8a/0x100
> >> [  890.765126]  do_syscall_64+0x34/0x80
> >> [  890.773795]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >> [  890.782630] RIP: 0033:0x453b29
> >> [  890.791189] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 
> >> 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 
> >> <48> 3d 01 f0 ff ff 0f 83 3b 84 00 00 c3 66 2e 0f 1f 84 00 00 00 00
> >> [  890.810866] RSP: 002b:7ffcda44fb18 EFLAGS: 0246 ORIG_RAX: 
> >> 0013
> >> [  890.820764] RAX: ffda RBX: 0013 RCX: 
> >> 00453b29
> >> [  890.830792] RDX: 009a RSI: 01de1c00 RDI: 
> >> 00b9
> >> [  890.840626] RBP: 7ffcda44fbc0 R08: 722c279d69ffc468 R09: 
> >> 0400
> >> [  890.850366] R10: 0098d82a42c63c22 R11: 0246 R12: 
> >> 0002
> >> [  890.860001] R13: 7f042ae6f058 R14: 010a2830 R15: 
> >> 7f042ae6f000
> >>
> >>
> >>
> >> To reproduce:
> >>
> >> # build kernel
> >>cd linux
> >>cp config-5.11.0-rc4-8-g79991caf5202 .config
> >>make HOSTCC=gcc-9 CC=gcc-9 ARCH=x86_64 olddefconfig prepare 
> >> modules_prepare bzImage
> >>
> >> git clone 
> >> https://urldefense.com/v3/__https://github.com/intel/lkp-tests.git__;!!GqivPVa7Brio!LfgrgVVtPAjwjqTZX8yANgsix4f3cJmAA_CcMeCVymh5XYcamWdR9dnbIQA-Qkr9TyI$
> >>  
> >> cd lkp-tests
> >> bin/lkp qemu -k  job-script # job-script is attached in 
> >> this email
> >>
> >>
> >>
> >> Thanks,
> >> Oliver Sang
> >>
>

Re: [btrfs] 5297199a8b: xfstests.btrfs.220.fail

2021-03-17 Thread Oliver Sang

Hi Nikolay,

On Tue, Mar 09, 2021 at 10:36:52AM +0200, Nikolay Borisov wrote:
> 
> 
> On 9.03.21 г. 10:49 ч., kernel test robot wrote:
> > 
> > 
> > Greeting,
> > 
> > FYI, we noticed the following commit (built with gcc-9):
> > 
> > commit: 5297199a8bca12b8b96afcbf2341605efb6005de ("btrfs: remove inode 
> > number cache feature")
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
> > 
> > 
> > in testcase: xfstests
> > version: xfstests-x86_64-d41dcbd-1_20201218
> > with following parameters:
> > 
> > disk: 6HDD
> > fs: btrfs
> > test: btrfs-group-22
> > ucode: 0x28
> > 
> > test-description: xfstests is a regression test suite for xfs and other 
> > files ystems.
> > test-url: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
> > 
> > 
> > on test machine: 8 threads Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz with 8G 
> > memory
> > 
> > caused below changes (please refer to attached dmesg/kmsg for entire 
> > log/backtrace):
> > 
> > 
> > 
> > 
> > If you fix the issue, kindly add following tag
> > Reported-by: kernel test robot 
> > 
> > 2021-03-09 04:13:26 export TEST_DIR=/fs/sdb1
> > 2021-03-09 04:13:26 export TEST_DEV=/dev/sdb1
> > 2021-03-09 04:13:26 export FSTYP=btrfs
> > 2021-03-09 04:13:26 export SCRATCH_MNT=/fs/scratch
> > 2021-03-09 04:13:26 mkdir /fs/scratch -p
> > 2021-03-09 04:13:26 export SCRATCH_DEV_POOL="/dev/sdb2 /dev/sdb3 /dev/sdb4 
> > /dev/sdb5 /dev/sdb6"
> > 2021-03-09 04:13:26 sed "s:^:btrfs/:" 
> > //lkp/benchmarks/xfstests/tests/btrfs-group-22
> > 2021-03-09 04:13:26 ./check btrfs/220 btrfs/221 btrfs/222 btrfs/223 
> > btrfs/224 btrfs/225 btrfs/226 btrfs/227
> > FSTYP -- btrfs
> > PLATFORM  -- Linux/x86_64 lkp-hsw-d01 5.10.0-rc7-00162-g5297199a8bca #1 
> > SMP Sat Feb 27 21:06:26 CST 2021
> > MKFS_OPTIONS  -- /dev/sdb2
> > MOUNT_OPTIONS -- /dev/sdb2 /fs/scratch
> > 
> > btrfs/220   - output mismatch (see 
> > /lkp/benchmarks/xfstests/results//btrfs/220.out.bad)
> > --- tests/btrfs/220.out 2021-01-14 07:40:58.0 +
> > +++ /lkp/benchmarks/xfstests/results//btrfs/220.out.bad 2021-03-09 
> > 04:13:32.880794446 +
> > @@ -1,2 +1,3 @@
> >  QA output created by 220
> > +Unexepcted mount options, checking for 
> > 'inode_cache,relatime,space_cache,subvol=/,subvolid=5' in 
> > 'rw,relatime,space_cache,subvolid=5,subvol=/' using 'inode_cache'
> 
> 
> Given that the commit removes the inode_cache feature that's expected, I
> assume you need to adjust your fstests configuration to not use
> inode_cache.

Thanks for information, we will change test options accordingly.

Re: [mm/highmem] 61b205f579: WARNING:at_mm/highmem.c:#__kmap_local_sched_out

2021-03-11 Thread Oliver Sang

Hi Ira,

On Thu, Mar 11, 2021 at 08:02:20AM -0800, Ira Weiny wrote:
> On Tue, Mar 09, 2021 at 08:53:04PM +, Chaitanya Kulkarni wrote:
> > Ira,
> > 
> > On 3/4/21 00:23, kernel test robot wrote:
> > > Greeting,
> > >
> > > FYI, we noticed the following commit (built with gcc-9):
> > >
> > > commit: 61b205f579911a11f0b576f73275eca2aed0d108 ("mm/highmem: Convert 
> > > memcpy_[to|from]_page() to kmap_local_page()")
> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
> > >
> > >
> > > in testcase: trinity
> > > version: trinity-static-i386-x86_64-f93256fb_2019-08-28
> > > with following parameters:
> > >
> > >   runtime: 300s
> > >
> > > test-description: Trinity is a linux system call fuzz tester.
> > > test-url: http://codemonkey.org.uk/projects/trinity/
> > >
> > >
> > > on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 
> > > 8G
> > >
> > > caused below changes (please refer to attached dmesg/kmsg for entire 
> > > log/backtrace):
> > 
> > Is the fix for this been posted yet ?
> 
> No.  I've been unable to reproduce it yet.

just FYI
the issue does not always happen but the rate on 61b205f579 is not low,
while we didn't observe it happen on parent commit.

bb90d4bc7b6a536b 61b205f579911a11f0b576f7327
 ---
   fail:runs  %reproductionfail:runs
   | | |
   :38  16%   6:38dmesg.EIP:__kmap_local_sched_in
   :38  16%   6:38dmesg.EIP:__kmap_local_sched_out
   :38  16%   6:38
dmesg.WARNING:at_mm/highmem.c:#__kmap_local_sched_in
   :38  16%   6:38
dmesg.WARNING:at_mm/highmem.c:#__kmap_local_sched_out

also please permit me to quote our internal analysis by Zhengjun (cced)
(Thanks a lot, Zhengjun)

"the commit has the potential to cause the issue.
It replaces " kmap_atomic" to " kmap_local_page".

Most of the two API is the same, except for " kmap_atomic" disable preemption 
and cannot sleep.
I check the issue happened when there is a preemption,  in FBC " 
kmap_local_page",
the  preemption is enabled,  the issue may happen."
"

> 
> Ira
> 
> > 
> > (asking since I didn't see the fix and my mailer is dropping emails from
> >  lkml).

Re: usb: cdc-acm: BUG kmalloc-128 Poison overwritten

2021-03-08 Thread Oliver Neukum

Am Montag, den 08.03.2021, 09:50 +0100 schrieb Bruno Thomsen:
> 
> Tested-by: Bruno Thomsen 
> 
> I have not observed any oops with patches applied. Patches have seen
> more than 10 weeks of runtime testing across multiple devices.

Hi,

that is good news. I shall send them upstream.

Regards
    Oliver

Re: [PATCH net v4 1/1] can: can_skb_set_owner(): fix ref counting if socket was closed before setting skb ownership

2021-02-26 Thread Oliver Hartkopp





On 26.02.21 10:24, Oleksij Rempel wrote:

There are two ref count variables controlling the free()ing of a socket:
- struct sock::sk_refcnt - which is changed by sock_hold()/sock_put()
- struct sock::sk_wmem_alloc - which accounts the memory allocated by
   the skbs in the send path.

In case there are still TX skbs on the fly and the socket() is closed,
the struct sock::sk_refcnt reaches 0. In the TX-path the CAN stack
clones an "echo" skb, calls sock_hold() on the original socket and
references it. This produces the following back trace:

| WARNING: CPU: 0 PID: 280 at lib/refcount.c:25 
refcount_warn_saturate+0x114/0x134
| refcount_t: addition on 0; use-after-free.
| Modules linked in: coda_vpu(E) v4l2_jpeg(E) videobuf2_vmalloc(E) imx_vdoa(E)
| CPU: 0 PID: 280 Comm: test_can.sh Tainted: GE 
5.11.0-04577-gf8ff6603c617 #203
| Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
| Backtrace:
| [<80bafea4>] (dump_backtrace) from [<80bb0280>] (show_stack+0x20/0x24) 
r7: r6:600f0113 r5: r4:81441220
| [<80bb0260>] (show_stack) from [<80bb593c>] (dump_stack+0xa0/0xc8)
| [<80bb589c>] (dump_stack) from [<8012b268>] (__warn+0xd4/0x114) r9:0019 
r8:80f4a8c2 r7:83e4150c r6: r5:0009 r4:80528f90
| [<8012b194>] (__warn) from [<80bb09c4>] (warn_slowpath_fmt+0x88/0xc8) 
r9:83f26400 r8:80f4a8d1 r7:0009 r6:80528f90 r5:0019 r4:80f4a8c2
| [<80bb0940>] (warn_slowpath_fmt) from [<80528f90>] 
(refcount_warn_saturate+0x114/0x134) r8: r7: r6:82b44000 r5:834e5600 
r4:83f4d540
| [<80528e7c>] (refcount_warn_saturate) from [<8079a4c8>] 
(__refcount_add.constprop.0+0x4c/0x50)
| [<8079a47c>] (__refcount_add.constprop.0) from [<8079a57c>] 
(can_put_echo_skb+0xb0/0x13c)
| [<8079a4cc>] (can_put_echo_skb) from [<8079ba98>] 
(flexcan_start_xmit+0x1c4/0x230) r9:0010 r8:83f48610 r7:0fdc r6:0c08 
r5:82b44000 r4:834e5600
| [<8079b8d4>] (flexcan_start_xmit) from [<80969078>] 
(netdev_start_xmit+0x44/0x70) r9:814c0ba0 r8:80c8790c r7: r6:834e5600 r5:82b44000 
r4:82ab1f00
| [<80969034>] (netdev_start_xmit) from [<809725a4>] 
(dev_hard_start_xmit+0x19c/0x318) r9:814c0ba0 r8: r7:82ab1f00 r6:82b44000 
r5: r4:834e5600
| [<80972408>] (dev_hard_start_xmit) from [<809c6584>] 
(sch_direct_xmit+0xcc/0x264) r10:834e5600 r9: r8: r7:82b44000 r6:82ab1f00 
r5:834e5600 r4:83f27400
| [<809c64b8>] (sch_direct_xmit) from [<809c6c0c>] (__qdisc_run+0x4f0/0x534)

To fix this problem, only set skb ownership to sockets which have still
a ref count > 0.

Cc: Oliver Hartkopp 
Cc: Andre Naujoks 
Suggested-by: Eric Dumazet 
Fixes: 0ae89beb283a ("can: add destructor for self generated skbs")
Signed-off-by: Oleksij Rempel 
---
  include/linux/can/skb.h | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index 685f34cfba20..d82018cc0d0b 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -65,8 +65,12 @@ static inline void can_skb_reserve(struct sk_buff *skb)
  
  static inline void can_skb_set_owner(struct sk_buff *skb, struct sock *sk)

  {
-   if (sk) {
-   sock_hold(sk);
+   /*
+* If the socket has already been closed by user space, the refcount may
+* already be 0 (and the socket will be freed after the last TX skb has
+* been freed). So only increase socket refcount if the refcount is > 0.
+*/
+   if (sk && refcount_inc_not_zero(>sk_refcnt)) {
skb->destructor = sock_efree;
    skb->sk = sk;
    }


Ah, now I got the problem. You are replacing sock_hold(). Maybe I was a 
bit slow-witted here - but with the comment it is ok.


Reviewed-by: Oliver Hartkopp 

Many thanks!

Oliver

Re: usb: cdc-acm: BUG kmalloc-128 Poison overwritten

2021-02-25 Thread Oliver Neukum

Am Mittwoch, den 24.02.2021, 16:21 +0100 schrieb Bruno Thomsen:

Hi,

> No, this is not a regression from 5.10. It seems that many attempts to
> fix cdc-acm in the 5.x kernel series have failed to fix the root cause of
> these oops. I have not seen this on 4.14 and 4.19, but I have observed
> it on at least 5.3 and newer kernels in slight variations.
> I guess this is because cdc-acm is very common in the embedded
> ARM world and rarely used on servers or laptops. Combined with
> ARM devices still commonly use 4.x LTS kernels. Not sure if
> hardening options on the kernel has increased change of reproducing
> oops.

OK, so this is not an additional problem.
According to your logs, an URB that should have been killed wasn't.

> I am ready to test new patches and will continue to report oops

Could you test the attached patches?

    Regards
Oliver

From 307097e80657ca44ac99da8efc8397070b1aff3f Mon Sep 17 00:00:00 2001
From: Oliver Neukum 
Date: Thu, 18 Feb 2021 13:42:40 +0100
Subject: [PATCH 1/2] cdc-wdm: untangle a circular dependency between callback
 and softint

We have a cycle of callbacks scheduling works which submit
URBs with thos callbacks. This needs to be blocked, stopped
and unblocked to untangle the circle..

Signed-off-by: Oliver Neukum 
---
 drivers/usb/class/cdc-wdm.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 508b1c3f8b73..d1e4a7379beb 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -321,12 +321,23 @@ static void wdm_int_callback(struct urb *urb)
 
 }
 
-static void kill_urbs(struct wdm_device *desc)
+static void poison_urbs(struct wdm_device *desc)
 {
 	/* the order here is essential */
-	usb_kill_urb(desc->command);
-	usb_kill_urb(desc->validity);
-	usb_kill_urb(desc->response);
+	usb_poison_urb(desc->command);
+	usb_poison_urb(desc->validity);
+	usb_poison_urb(desc->response);
+}
+
+static void unpoison_urbs(struct wdm_device *desc)
+{
+	/*
+	 *  the order here is not essential
+	 *  it is symmetrical just to be nice
+	 */
+	usb_unpoison_urb(desc->response);
+	usb_unpoison_urb(desc->validity);
+	usb_unpoison_urb(desc->command);
 }
 
 static void free_urbs(struct wdm_device *desc)
@@ -741,11 +752,12 @@ static int wdm_release(struct inode *inode, struct file *file)
 	if (!desc->count) {
 		if (!test_bit(WDM_DISCONNECTING, >flags)) {
 			dev_dbg(>intf->dev, "wdm_release: cleanup\n");
-			kill_urbs(desc);
+			poison_urbs(desc);
 			spin_lock_irq(>iuspin);
 			desc->resp_count = 0;
 			spin_unlock_irq(>iuspin);
 			desc->manage_power(desc->intf, 0);
+			unpoison_urbs(desc);
 		} else {
 			/* must avoid dev_printk here as desc->intf is invalid */
 			pr_debug(KBUILD_MODNAME " %s: device gone - cleaning up\n", __func__);
@@ -1037,9 +1049,9 @@ static void wdm_disconnect(struct usb_interface *intf)
 	wake_up_all(>wait);
 	mutex_lock(>rlock);
 	mutex_lock(>wlock);
+	poison_urbs(desc);
 	cancel_work_sync(>rxwork);
 	cancel_work_sync(>service_outs_intr);
-	kill_urbs(desc);
 	mutex_unlock(>wlock);
 	mutex_unlock(>rlock);
 
@@ -1080,9 +1092,10 @@ static int wdm_suspend(struct usb_interface *intf, pm_message_t message)
 		set_bit(WDM_SUSPENDING, >flags);
 		spin_unlock_irq(>iuspin);
 		/* callback submits work - order is essential */
-		kill_urbs(desc);
+		poison_urbs(desc);
 		cancel_work_sync(>rxwork);
 		cancel_work_sync(>service_outs_intr);
+		unpoison_urbs(desc);
 	}
 	if (!PMSG_IS_AUTO(message)) {
 		mutex_unlock(>wlock);
@@ -1140,7 +1153,7 @@ static int wdm_pre_reset(struct usb_interface *intf)
 	wake_up_all(>wait);
 	mutex_lock(>rlock);
 	mutex_lock(>wlock);
-	kill_urbs(desc);
+	poison_urbs(desc);
 	cancel_work_sync(>rxwork);
 	cancel_work_sync(>service_outs_intr);
 	return 0;
@@ -1151,6 +1164,7 @@ static int wdm_post_reset(struct usb_interface *intf)
 	struct wdm_device *desc = wdm_find_device(intf);
 	int rv;
 
+	unpoison_urbs(desc);
 	clear_bit(WDM_OVERFLOW, >flags);
 	clear_bit(WDM_RESETTING, >flags);
 	rv = recover_from_urb_loss(desc);
-- 
2.26.2

From 3eeb644af140174ebad6ddce5526bcaf42ccd9c9 Mon Sep 17 00:00:00 2001
From: Oliver Neukum 
Date: Thu, 18 Feb 2021 13:52:28 +0100
Subject: [PATCH 2/2] cdc-acm: untangle a circular dependency between callback
 and softint

We have a cycle of callbacks scheduling works which submit
URBs with thos callbacks. This needs to be blocked, stopped
and unblocked to untangle the circle.

Signed-off-by: Oliver Neukum 
---
 drivers/usb/class/cdc-acm.c | 41 +
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 781905745812..235fd1f654a4 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@

Re: [tcp] 9d9b1ee0b2: packetdrill.packetdrill/gtests/net/tcp/user_timeout/user-timeout-probe_ipv4-mapped-v6.fail

2021-02-25 Thread Oliver Sang

Hi, Neal,

On Wed, Feb 24, 2021 at 10:13:02PM +0800, Oliver Sang wrote:
> Hi, Neal,
> 
> On Fri, Feb 19, 2021 at 09:52:04AM -0500, Neal Cardwell wrote:
> > On Thu, Feb 18, 2021 at 8:33 PM kernel test robot  
> > wrote:
> > >
> > >
> > > Greeting,
> > >
> > > FYI, we noticed the following commit (built with gcc-9):
> > >
> > > commit: 9d9b1ee0b2d1c9e02b2338c4a4b0a062d2d3edac ("tcp: fix 
> > > TCP_USER_TIMEOUT with zero window")
> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
> > 
> > I have pushed to the packetdrill repo a commit that should fix this test:
> > 
> > 094da3bc77e5 (HEAD, packetdrill/master) net-test: update TCP tests for
> > USER_TIMEOUT ZWP fix
> > https://github.com/google/packetdrill/commit/094da3bc77e518d820ebc0ef8b94a5b4cf707a39
> > 
> > Can someone please pull that commit into the repo used by the test
> > bot, if needed? (Or does it automatically use the latest packetdrill
> > master branch?)
> 
> We updated our tool to use this latest packetdrill. seems improved, but not 
> totally fix.
> 
> before upgrading, we have:
> b889c7c8c02ebb0b 9d9b1ee0b2d1c9e02b2338c4a4b
>  ---
>fail:runs  %reproductionfail:runs
>| | |
>:6  100%   6:6 
> packetdrill.packetdrill/gtests/net/tcp/user_timeout/user-timeout-probe_ipv4-mapped-v6.fail
>:6  100%   6:6 
> packetdrill.packetdrill/gtests/net/tcp/user_timeout/user-timeout-probe_ipv4.fail
> 
> after upgrading, we have:
> b889c7c8c02ebb0b 9d9b1ee0b2d1c9e02b2338c4a4b
>  ---
>fail:runs  %reproductionfail:runs
>| | |
>:6  100%   5:6 
> packetdrill.packetdrill/gtests/net/tcp/user_timeout/user-timeout-probe_ipv4-mapped-v6.fail
>:6  100%   3:6 
> packetdrill.packetdrill/gtests/net/tcp/user_timeout/user-timeout-probe_ipv4.fail
> 
> 
> attached kmsg.xz and packetdrill from one run where both tests failed.

here is an update. we did't re-test parent with latest packetdrill yesterday,
so above results about b889c7c8c02ebb0b are still from old version packetdrill.

today, we did further tests based on latest packetdrill, and found the tests
always failed upon b889c7c8c02ebb0b. not sure if a kernel before your commit
(9d9b1ee0b2d1c9e02b2338c4a4b) is still valid to run latest packetdrill?

attached kmsg and test log from latest packetdrill upon parent commit FYI.


> 
> 
> > 
> > thanks,
> > neal


> Running packetdrill/tests/bsd/fast_retransmit/fr-4pkt-sack-bsd.pkt ...
> 2021-02-24 08:46:09 packetdrill/packetdrill --tolerance_usecs=4 
> packetdrill/tests/bsd/fast_retransmit/fr-4pkt-sack-bsd.pkt
> packetdrill/tests/bsd/fast_retransmit/fr-4pkt-sack-bsd.pkt:25: error handling 
> packet: live packet payload: expected 1000 bytes vs actual 2000 bytes
> packetdrill/tests/bsd/fast_retransmit/fr-4pkt-sack-bsd.pkt failed
> Running packetdrill/tests/linux/fast_retransmit/fr-4pkt-sack-linux.pkt ...
> 2021-02-24 08:46:10 packetdrill/packetdrill --tolerance_usecs=4 
> packetdrill/tests/linux/fast_retransmit/fr-4pkt-sack-linux.pkt
> packetdrill/tests/linux/fast_retransmit/fr-4pkt-sack-linux.pkt pass
> Running packetdrill/tests/linux/packetdrill/socket_err.pkt ...
> 2021-02-24 08:46:10 packetdrill/packetdrill --tolerance_usecs=4 
> packetdrill/tests/linux/packetdrill/socket_err.pkt
> packetdrill/tests/linux/packetdrill/socket_err.pkt:6: runtime error in socket 
> call: Expected non-negative result but got -1 with errno 93 (Protocol not 
> supported)
> packetdrill/tests/linux/packetdrill/socket_err.pkt failed
> Running packetdrill/tests/linux/packetdrill/socket_wrong_err.pkt ...
> 2021-02-24 08:46:10 packetdrill/packetdrill --tolerance_usecs=4 
> packetdrill/tests/linux/packetdrill/socket_wrong_err.pkt
> packetdrill/tests/linux/packetdrill/socket_wrong_err.pkt:4: runtime error in 
> socket call: Expected result -99 but got -1 with errno 93 (Protocol not 
> supported)
> packetdrill/tests/linux/packetdrill/socket_wrong_err.pkt failed
> OK   [/lkp/benchmarks/packetdrill/gtests/net/tcp/blocking/blocking-accept.pkt 
> (ipv4)]
> stdout: 
> stderr: 
> OK   [/lkp/benchmarks/packetdrill/gtests/net/tcp/blocking/blocking-accept.pkt 
> (ipv6)]
> stdout: 
> stderr: 
> OK   
> [/lkp/benchmarks/packetdrill/gtests/net/tcp/blocking/blocking-connect.pkt 
> (ipv4-mapped-v6)]
> stdout: 
> stderr: 
> OK   [/lkp/benchmarks/packetdrill/gtests/net/tcp/blocking/blocking-read.pkt 
>

Re: [PATCH net v3 1/1] can: can_skb_set_owner(): fix ref counting if socket was closed before setting skb ownership

2021-02-24 Thread Oliver Hartkopp





On 24.02.21 09:53, Eric Dumazet wrote:

On Wed, Feb 24, 2021 at 8:59 AM Oleksij Rempel  wrote:


There are two ref count variables controlling the free()ing of a socket:
- struct sock::sk_refcnt - which is changed by sock_hold()/sock_put()
- struct sock::sk_wmem_alloc - which accounts the memory allocated by
   the skbs in the send path.

In case there are still TX skbs on the fly and the socket() is closed,
the struct sock::sk_refcnt reaches 0. In the TX-path the CAN stack
clones an "echo" skb, calls sock_hold() on the original socket and
references it. This produces the following back trace:

| WARNING: CPU: 0 PID: 280 at lib/refcount.c:25 
refcount_warn_saturate+0x114/0x134
| refcount_t: addition on 0; use-after-free.
| Modules linked in: coda_vpu(E) v4l2_jpeg(E) videobuf2_vmalloc(E) imx_vdoa(E)
| CPU: 0 PID: 280 Comm: test_can.sh Tainted: GE 
5.11.0-04577-gf8ff6603c617 #203
| Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
| Backtrace:
| [<80bafea4>] (dump_backtrace) from [<80bb0280>] (show_stack+0x20/0x24) 
r7: r6:600f0113 r5: r4:81441220
| [<80bb0260>] (show_stack) from [<80bb593c>] (dump_stack+0xa0/0xc8)
| [<80bb589c>] (dump_stack) from [<8012b268>] (__warn+0xd4/0x114) r9:0019 
r8:80f4a8c2 r7:83e4150c r6: r5:0009 r4:80528f90
| [<8012b194>] (__warn) from [<80bb09c4>] (warn_slowpath_fmt+0x88/0xc8) 
r9:83f26400 r8:80f4a8d1 r7:0009 r6:80528f90 r5:0019 r4:80f4a8c2
| [<80bb0940>] (warn_slowpath_fmt) from [<80528f90>] 
(refcount_warn_saturate+0x114/0x134) r8: r7: r6:82b44000 r5:834e5600 
r4:83f4d540
| [<80528e7c>] (refcount_warn_saturate) from [<8079a4c8>] 
(__refcount_add.constprop.0+0x4c/0x50)
| [<8079a47c>] (__refcount_add.constprop.0) from [<8079a57c>] 
(can_put_echo_skb+0xb0/0x13c)
| [<8079a4cc>] (can_put_echo_skb) from [<8079ba98>] 
(flexcan_start_xmit+0x1c4/0x230) r9:0010 r8:83f48610 r7:0fdc r6:0c08 
r5:82b44000 r4:834e5600
| [<8079b8d4>] (flexcan_start_xmit) from [<80969078>] 
(netdev_start_xmit+0x44/0x70) r9:814c0ba0 r8:80c8790c r7: r6:834e5600 r5:82b44000 
r4:82ab1f00
| [<80969034>] (netdev_start_xmit) from [<809725a4>] 
(dev_hard_start_xmit+0x19c/0x318) r9:814c0ba0 r8: r7:82ab1f00 r6:82b44000 
r5: r4:834e5600
| [<80972408>] (dev_hard_start_xmit) from [<809c6584>] 
(sch_direct_xmit+0xcc/0x264) r10:834e5600 r9: r8: r7:82b44000 r6:82ab1f00 
r5:834e5600 r4:83f27400
| [<809c64b8>] (sch_direct_xmit) from [<809c6c0c>] (__qdisc_run+0x4f0/0x534)

To fix this problem, only set skb ownership to sockets which have still
a ref count > 0.

Cc: Oliver Hartkopp 
Cc: Andre Naujoks 
Suggested-by: Eric Dumazet 
Fixes: 0ae89beb283a ("can: add destructor for self generated skbs")
Signed-off-by: Oleksij Rempel 


SGTM

Reviewed-by: Eric Dumazet 


---
  include/linux/can/skb.h | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index 685f34cfba20..655f33aa99e3 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -65,8 +65,7 @@ static inline void can_skb_reserve(struct sk_buff *skb)

  static inline void can_skb_set_owner(struct sk_buff *skb, struct sock *sk)
  {
-   if (sk) {
-   sock_hold(sk);


Although the commit message gives a comprehensive reason for this patch: 
Can you please add some comment here as I do not think the use of 
refcount_inc_not_zero() makes clear what is checked here.


Many thanks,
Oliver



+   if (sk && refcount_inc_not_zero(>sk_refcnt)) {
 skb->destructor = sock_efree;
 skb->sk = sk;
 }
--
2.29.2

Re: [tcp] 9d9b1ee0b2: packetdrill.packetdrill/gtests/net/tcp/user_timeout/user-timeout-probe_ipv4-mapped-v6.fail

2021-02-24 Thread Oliver Sang

Hi, Neal,

On Fri, Feb 19, 2021 at 09:52:04AM -0500, Neal Cardwell wrote:
> On Thu, Feb 18, 2021 at 8:33 PM kernel test robot  
> wrote:
> >
> >
> > Greeting,
> >
> > FYI, we noticed the following commit (built with gcc-9):
> >
> > commit: 9d9b1ee0b2d1c9e02b2338c4a4b0a062d2d3edac ("tcp: fix 
> > TCP_USER_TIMEOUT with zero window")
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
> 
> I have pushed to the packetdrill repo a commit that should fix this test:
> 
> 094da3bc77e5 (HEAD, packetdrill/master) net-test: update TCP tests for
> USER_TIMEOUT ZWP fix
> https://github.com/google/packetdrill/commit/094da3bc77e518d820ebc0ef8b94a5b4cf707a39
> 
> Can someone please pull that commit into the repo used by the test
> bot, if needed? (Or does it automatically use the latest packetdrill
> master branch?)

We updated our tool to use this latest packetdrill. seems improved, but not 
totally fix.

before upgrading, we have:
b889c7c8c02ebb0b 9d9b1ee0b2d1c9e02b2338c4a4b
 ---
   fail:runs  %reproductionfail:runs
   | | |
   :6  100%   6:6 
packetdrill.packetdrill/gtests/net/tcp/user_timeout/user-timeout-probe_ipv4-mapped-v6.fail
   :6  100%   6:6 
packetdrill.packetdrill/gtests/net/tcp/user_timeout/user-timeout-probe_ipv4.fail

after upgrading, we have:
b889c7c8c02ebb0b 9d9b1ee0b2d1c9e02b2338c4a4b
 ---
   fail:runs  %reproductionfail:runs
   | | |
   :6  100%   5:6 
packetdrill.packetdrill/gtests/net/tcp/user_timeout/user-timeout-probe_ipv4-mapped-v6.fail
   :6  100%   3:6 
packetdrill.packetdrill/gtests/net/tcp/user_timeout/user-timeout-probe_ipv4.fail

attached kmsg.xz and packetdrill from one run where both tests failed.

> 
> thanks,
> neal

kmsg.xz
Description: application/xz
Running packetdrill/tests/bsd/fast_retransmit/fr-4pkt-sack-bsd.pkt ...
2021-02-24 08:46:09 packetdrill/packetdrill --tolerance_usecs=4 
packetdrill/tests/bsd/fast_retransmit/fr-4pkt-sack-bsd.pkt
packetdrill/tests/bsd/fast_retransmit/fr-4pkt-sack-bsd.pkt:25: error handling 
packet: live packet payload: expected 1000 bytes vs actual 2000 bytes
packetdrill/tests/bsd/fast_retransmit/fr-4pkt-sack-bsd.pkt failed
Running packetdrill/tests/linux/fast_retransmit/fr-4pkt-sack-linux.pkt ...
2021-02-24 08:46:10 packetdrill/packetdrill --tolerance_usecs=4 
packetdrill/tests/linux/fast_retransmit/fr-4pkt-sack-linux.pkt
packetdrill/tests/linux/fast_retransmit/fr-4pkt-sack-linux.pkt pass
Running packetdrill/tests/linux/packetdrill/socket_err.pkt ...
2021-02-24 08:46:10 packetdrill/packetdrill --tolerance_usecs=4 
packetdrill/tests/linux/packetdrill/socket_err.pkt
packetdrill/tests/linux/packetdrill/socket_err.pkt:6: runtime error in socket 
call: Expected non-negative result but got -1 with errno 93 (Protocol not 
supported)
packetdrill/tests/linux/packetdrill/socket_err.pkt failed
Running packetdrill/tests/linux/packetdrill/socket_wrong_err.pkt ...
2021-02-24 08:46:10 packetdrill/packetdrill --tolerance_usecs=4 
packetdrill/tests/linux/packetdrill/socket_wrong_err.pkt
packetdrill/tests/linux/packetdrill/socket_wrong_err.pkt:4: runtime error in 
socket call: Expected result -99 but got -1 with errno 93 (Protocol not 
supported)
packetdrill/tests/linux/packetdrill/socket_wrong_err.pkt failed
OK   [/lkp/benchmarks/packetdrill/gtests/net/tcp/blocking/blocking-accept.pkt 
(ipv4)]
stdout: 
stderr: 
OK   [/lkp/benchmarks/packetdrill/gtests/net/tcp/blocking/blocking-accept.pkt 
(ipv6)]
stdout: 
stderr: 
OK   [/lkp/benchmarks/packetdrill/gtests/net/tcp/blocking/blocking-connect.pkt 
(ipv4-mapped-v6)]
stdout: 
stderr: 
OK   [/lkp/benchmarks/packetdrill/gtests/net/tcp/blocking/blocking-read.pkt 
(ipv4)]
stdout: 
stderr: 
OK   [/lkp/benchmarks/packetdrill/gtests/net/tcp/blocking/blocking-read.pkt 
(ipv6)]
stdout: 
stderr: 
OK   [/lkp/benchmarks/packetdrill/gtests/net/tcp/blocking/blocking-write.pkt 
(ipv4-mapped-v6)]
stdout: 
stderr: 
OK   
[/lkp/benchmarks/packetdrill/gtests/net/tcp/close/close-local-close-then-remote-fin.pkt
 (ipv4)]
stdout: 
stderr: 
OK   
[/lkp/benchmarks/packetdrill/gtests/net/tcp/close/close-local-close-then-remote-fin.pkt
 (ipv6)]
stdout: 
stderr: 
OK   [/lkp/benchmarks/packetdrill/gtests/net/tcp/close/close-on-syn-sent.pkt 
(ipv4-mapped-v6)]
stdout: 
stderr: 
OK   
[/lkp/benchmarks/packetdrill/gtests/net/tcp/close/close-remote-fin-then-close.pkt
 (ipv4)]
stdout: 
stderr: 
OK   
[/lkp/benchmarks/packetdrill/gtests/net/tcp/close/close-remote-fin-then-close.pkt
 (ipv6)]
stdout: 
stderr: 
OK   
[/lkp/benchmarks/packetdrill/gtests/net/tcp/cwnd_moderation/cwnd-moderation-disorder-no-moderation.pkt
 (ipv4-mapped-v6)]
stdout: 
stderr: 
FAIL

Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.12-1 tag

2021-02-22 Thread Oliver O'Halloran

On Tue, Feb 23, 2021 at 9:44 AM Linus Torvalds
 wrote:
>
> On Mon, Feb 22, 2021 at 4:06 AM Michael Ellerman  wrote:
> >
> > Please pull powerpc updates for 5.12.
>
> Pulled. However:
>
> >  mode change 100755 => 100644 
> > tools/testing/selftests/powerpc/eeh/eeh-functions.sh
> >  create mode 100755 tools/testing/selftests/powerpc/eeh/eeh-vf-aware.sh
> >  create mode 100755 tools/testing/selftests/powerpc/eeh/eeh-vf-unaware.sh
>
> Somebody is being confused.
>
> Why create two new shell scripts with the proper executable bit, and
> then remove the executable bit from an existing one?
>
> That just seems very inconsistent.

eeh-function.sh just provides some helper functions for the other
scripts and doesn't do anything when executed directly. I thought
making it non-executable made sense.

>
>  Linus

Re: [PATCH] usbnet: ipheth: fix connectivity with iOS 14

2021-02-22 Thread Oliver Neukum

Am Sonntag, den 21.02.2021, 10:42 + schrieb Sam Bingner:
> There seems to be a problem with this patch:
> 
> Whenever the iPhone sends a packet to the tethered device that is 1500 bytes 
> long, it gets the error "ipheth 1-1:4.2: ipheth_rcvbulk_callback: urb status: 
> -79" on the connected device and stops passing traffic.  I am able to bring 
> it back up by shutting and unshutting the interface, but the same thing 
> happens very quickly.   I noticed that this patch dropped the max USB packet 
> size from 1516 to 1514 bytes, so I decided to try lowering the MTU to 1498; 
> this made the connection reliable and no more errors occurred.
> 
> It appears to me that the iPhone is still sending out 1516 bytes over USB for 
> a 1500 byte packet and this patch makes USB abort when that happens?  I could 
> duplicate reliably by sending a ping from the iphone (ping -s 1472) to the 
> connected device, or vice versa as the reply would then break it.
> 
> I apologize if this reply doesn't end up where it should - I tried to reply 
> to the last message in this thread but I wasn't actually *on* the thread so I 
> had to just build it as much as possible myself.

Is this a regression? Does it work after reverting the patch? Which
version of iOS?

Regards
Oliver

Re: usb: cdc-acm: BUG kmalloc-128 Poison overwritten

2021-02-22 Thread Oliver Neukum

Am Donnerstag, den 18.02.2021, 16:52 +0100 schrieb Bruno Thomsen:
> Den fre. 12. feb. 2021 kl. 16.33 skrev Bruno Thomsen 
> :
> > Hi,
> > 
> > I have been experience random kernel oops in the cdc-acm driver on
> > imx7 (arm arch). Normally it happens during the first 1-3min runtime
> > after power-on. Below oops is from 5.8.17 mainline kernel with an
> > extra patch back-ported in an attempt to fix it:
> > 38203b8385 ("usb: cdc-acm: fix cooldown mechanism")
> 
> I can now boot board with 5.11 kernel without any extra patches and
> it produce similar issue. Hopefully that make the oops more useful.
> Issue has been seen on multiple devices, so I don't think it's a bad
> hardware issue.

Hi,

is this a regression from 5.10?

Regards
Oliver

[PATCH v4 3/3] dt-bindings: arm: fsl: Add Variscite i.MX6UL compatibles

Add the compatibles for Variscite i.MX6UL compatibles

Signed-off-by: Oliver Graute 
---

Changelog:

v4:
 - added missing 6 in i.MX6

 v3:
 - rebased

 v2:
 - renamed binding
 - removed superflous "

 Documentation/devicetree/bindings/arm/fsl.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml 
b/Documentation/devicetree/bindings/arm/fsl.yaml
index 297c87f..e67b622 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -499,6 +499,7 @@ properties:
   - technexion,imx6ul-pico-dwarf   # TechNexion i.MX6UL Pico-Dwarf
   - technexion,imx6ul-pico-hobbit  # TechNexion i.MX6UL Pico-Hobbit
   - technexion,imx6ul-pico-pi  # TechNexion i.MX6UL Pico-Pi
+  - variscite,imx6ul-var-6ulcustomboard # i.MX6 UltraLite 
Carrier-board
   - const: fsl,imx6ul
 
   - description: i.MX6UL Armadeus Systems OPOS6UL SoM Board
-- 
2.7.4

[PATCH v9 2/3] ARM: dts: Add support for i.MX6 UltraLite DART Variscite Customboard

This patch adds DeviceTree Source for the i.MX6 UltraLite DART NAND/WIFI

Signed-off-by: Oliver Graute 
Cc: Shawn Guo 
Cc: Neil Armstrong 
Cc: Marco Felsch 
Cc: Parthiban Nallathambi 
---
Changelog:

v9:
 - removed display-timing node
 - added 5V power supply for display
 - added assigned clocks for display

v8:
 - backlight droped the status line
 - port the display panel
 - added pinctrl for touch

v7:
 - fixed wakeup-source

v6:
 - added some muxing
 - added codec in sound node
 - added adc1 node

 arch/arm/boot/dts/Makefile  |   1 +
 arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts | 255 
 2 files changed, 256 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index ce66ffd..9f72446 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -626,6 +626,7 @@ dtb-$(CONFIG_SOC_IMX6UL) += \
imx6ul-tx6ul-0010.dtb \
imx6ul-tx6ul-0011.dtb \
imx6ul-tx6ul-mainboard.dtb \
+   imx6ul-var-6ulcustomboard.dtb \
imx6ull-14x14-evk.dtb \
imx6ull-colibri-eval-v3.dtb \
imx6ull-colibri-wifi-eval-v3.dtb \
diff --git a/arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts 
b/arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts
new file mode 100644
index ..bf1
--- /dev/null
+++ b/arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts
@@ -0,0 +1,255 @@
+// SPDX-License-Identifier: (GPL-2.0)
+/*
+ * Support for Variscite DART-6UL Module
+ *
+ * Copyright (C) 2015 Freescale Semiconductor, Inc.
+ * Copyright (C) 2015-2016 Variscite Ltd. - http://www.variscite.com
+ * Copyright (C) 2018-2021 Oliver Graute 
+ */
+
+/dts-v1/;
+
+#include 
+#include "imx6ul-imx6ull-var-dart-common.dtsi"
+
+/ {
+   model = "Variscite i.MX6 UltraLite Carrier-board";
+   compatible = "variscite,6ulcustomboard", "fsl,imx6ul";
+
+   backlight_lcd: backlight {
+   compatible = "pwm-backlight";
+   pwms = < 0 2>;
+   brightness-levels = <0 4 8 16 32 64 128 255>;
+   default-brightness-level = <6>;
+   status = "okay";
+   };
+
+   gpio-keys {
+   compatible = "gpio-keys";
+   pinctrl-names = "default";
+   pinctrl-0 = <_gpio_keys>;
+
+   user {
+   gpios = < 0 GPIO_ACTIVE_LOW>;
+   linux,code = ;
+   wakeup-source;
+   };
+   };
+
+   gpio-leds {
+   compatible = "gpio-leds";
+   pinctrl-names = "default";
+   pinctrl-0 = <_gpio_leds>;
+
+   d16-led {
+   gpios = < 20 GPIO_ACTIVE_HIGH>;
+   linux,default-trigger = "heartbeat";
+   };
+   };
+
+   panel1: panel-lcd {
+   compatible = "sgd,gktw70sdad1sd";
+
+   backlight = <_lcd>;
+   label = "gktw70sdad1sd";
+
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
+
+   sound {
+   compatible = "simple-audio-card";
+   simple-audio-card,name = "wm8731audio";
+   simple-audio-card,widgets =
+   "Headphone", "Headphone Jack",
+   "Line", "Line Jack",
+   "Microphone", "Mic Jack";
+   simple-audio-card,routing =
+   "Headphone Jack", "RHPOUT",
+   "Headphone Jack", "LHPOUT",
+   "LLINEIN", "Line Jack",
+   "RLINEIN", "Line Jack",
+   "MICIN", "Mic Bias",
+   "Mic Bias", "Mic Jack";
+   simple-audio-card,format = "i2s";
+   simple-audio-card,bitclock-master = <_dai>;
+   simple-audio-card,frame-master = <_dai>;
+
+   codec_dai: simple-audio-card,codec {
+   sound-dai = <>;
+   system-clock-frequency = <12288000>;
+   };
+   };
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   clock_frequency = <10>;
+   pinctrl-names = "

[PATCH v9 1/3] ARM: dts: imx6ul: Add Variscite DART-6UL SoM support

This patch adds support for the i.MX6UL variant of the Variscite DART-6UL
SoM Carrier-Board

Signed-off-by: Oliver Graute 
Cc: Shawn Guo 
Cc: Neil Armstrong 
Cc: Marco Felsch 
Cc: Parthiban Nallathambi 
---
 .../boot/dts/imx6ul-imx6ull-var-dart-common.dtsi   | 314 +
 1 file changed, 314 insertions(+)

Changelog:

v9:
 - added 3V and 5V regulator
 - move phy reset to subnode
 - added pwm-cells
 - fixed pad pin conflict

v8:
 - remove can node
 - remove flexscan pinctrl
 - moved lcd and i2c pinctrl
 - sorted regulators
 - add dedicated pinctrl for dvfs regulator

v7:
 - removed cpu0 node
 - fixed phy problem

v6:
 - renamed touch regulator
 - renamed rmii clock
 - moved some muxing to baseboard
 - added pinctrl for gpio key
 - added bus-width to usdhc1
 - fixed missing subnode on partitions

 create mode 100644 arch/arm/boot/dts/imx6ul-imx6ull-var-dart-common.dtsi

diff --git a/arch/arm/boot/dts/imx6ul-imx6ull-var-dart-common.dtsi 
b/arch/arm/boot/dts/imx6ul-imx6ull-var-dart-common.dtsi
new file mode 100644
index ..b95fdc5
--- /dev/null
+++ b/arch/arm/boot/dts/imx6ul-imx6ull-var-dart-common.dtsi
@@ -0,0 +1,314 @@
+// SPDX-License-Identifier: (GPL-2.0)
+/dts-v1/;
+
+#include "imx6ul.dtsi"
+/ {
+   chosen {
+   stdout-path = 
+   };
+
+   memory@8000 {
+   device_type = "memory";
+   reg = <0x8000 0x2000>;
+   };
+
+   clk_rmii_ref: clock-rmii-ref {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <2500>;
+   clock-output-names = "rmii-ref";
+   };
+
+   reg_3v3: regulator-3v3 {
+   compatible = "regulator-fixed";
+   regulator-name = "3.3V";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   };
+
+   reg_5v0: regulator-5v0 {
+   compatible = "regulator-fixed";
+   regulator-name = "5V";
+   regulator-min-microvolt = <500>;
+   regulator-max-microvolt = <500>;
+   };
+
+   reg_gpio_dvfs: regulator-gpio {
+   compatible = "regulator-gpio";
+   gpios = < 13 GPIO_ACTIVE_HIGH>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_dvfs_reg>;
+   regulator-min-microvolt = <130>;
+   regulator-max-microvolt = <140>;
+   regulator-name = "gpio_dvfs";
+   regulator-type = "voltage";
+   enable-active-high;
+   states = <130 0x1 140 0x0>;
+   };
+
+   reg_sd1_vmmc: regulator-sd1-vmmc {
+   compatible = "regulator-fixed";
+   regulator-name = "VSD_3V3";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   };
+
+   reg_touch_3v3: regulator-touch-3v3 {
+   compatible = "regulator-fixed";
+   regulator-name = "touch_3v3_supply";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   };
+
+};
+
+ {
+   vref-supply = <_touch_3v3>;
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_enet1>;
+   phy-mode = "rmii";
+   phy-handle = <>;
+
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ethphy0: ethernet-phy@1 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   micrel,rmii-reference-clock-select-25-mhz;
+   clocks = <_rmii_ref>;
+   clock-names = "rmii-ref";
+   reset-names = "phy";
+   reset-gpios=< 10 1>;
+   reset-assert-us = <100>;
+   reg = <1>;
+   };
+
+   ethphy1: ethernet-phy@3 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   micrel,rmii-reference-clock-select-25-mhz;
+   clocks = <_rmii_ref>;
+   clock-names = "rmii-ref";
+   reg = <3>;
+   };
+   };
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_gpmi_nand>;
+   status = "okay";
+};
+
+ {
+   clock-frequency = <40>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_i2c1>;
+};
+
+ {
+   #pwm-cells = <2>;
+   pinctrl-names = "default";
+

[PATCH v9 0/3] Variscite DART-6UL SoM support

This patch series adds support for the Variscite DART-6UL SoM

Product Page: https://www.variscite.com/product/evaluation-kits/dart-6ul-kits

Oliver Graute (3):
  ARM: dts: imx6ul: Add Variscite DART-6UL SoM support
  ARM: dts: Add support for i.MX6 UltraLite DART Variscite Customboard
  dt-bindings: arm: fsl: Add Variscite i.MX6UL compatibles

 Documentation/devicetree/bindings/arm/fsl.yaml |   1 +
 arch/arm/boot/dts/Makefile |   1 +
 .../boot/dts/imx6ul-imx6ull-var-dart-common.dtsi   | 314 +
 arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts| 255 +
 4 files changed, 604 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6ul-imx6ull-var-dart-common.dtsi
 create mode 100644 arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts

-- 
2.7.4

Re: include/linux/compiler_types.h:315:38: error: call to '__compiletime_assert_515' declared with attribute error: BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(struct canfd_frame,

2021-02-13 Thread Oliver Hartkopp





On 13.02.21 20:57, kernel test robot wrote:

Hi Oliver,

FYI, the error/warning still remains.


Yes, because of the this union, see:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/can.h?id=c7b74967799b1af52b3045d69d4c26836b2d41de#n109

Maybe I was not clear on this.

On ARM the BUILD_BUG_ON() seems to stumble about this new union{} 
introduced here:


https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/can.h?id=ea7800565a128c1adafa1791ce80afd6016fe21c

So every commit after ea7800565a128c1 ("can: add optional DLC element to 
Classical CAN frame structure") should show this issue UNTIL 
BUILD_BUG_ON() is fixed on ARM (my assuption) so that the union{} is 
supported like on the x86.


Best,
Oliver



tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   dcc0b49040c70ad827a7f3d58a21b01fdb14e749
commit: c7b74967799b1af52b3045d69d4c26836b2d41de can: replace can_dlc as 
variable/element for payload length
date:   3 months ago
config: arm-randconfig-r013-20210214 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
 wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
 chmod +x ~/bin/make.cross
 # 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c7b74967799b1af52b3045d69d4c26836b2d41de
 git remote add linus 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
 git fetch --no-tags linus master
 git checkout c7b74967799b1af52b3045d69d4c26836b2d41de
 # save the attached .config to linux build tree
 COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

In file included from :
net/can/af_can.c: In function 'can_init':

include/linux/compiler_types.h:315:38: error: call to 
'__compiletime_assert_515' declared with attribute error: BUILD_BUG_ON failed: 
offsetof(struct can_frame, len) != offsetof(struct canfd_frame, len) || 
offsetof(struct can_frame, data) != offsetof(struct canfd_frame, data)

  315 |  _compiletime_assert(condition, msg, __compiletime_assert_, 
__COUNTER__)
  |  ^
include/linux/compiler_types.h:296:4: note: in definition of macro 
'__compiletime_assert'
  296 |prefix ## suffix();\
  |^~
include/linux/compiler_types.h:315:2: note: in expansion of macro 
'_compiletime_assert'
  315 |  _compiletime_assert(condition, msg, __compiletime_assert_, 
__COUNTER__)
  |  ^~~
include/linux/build_bug.h:39:37: note: in expansion of macro 
'compiletime_assert'
   39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
  | ^~
include/linux/build_bug.h:50:2: note: in expansion of macro 
'BUILD_BUG_ON_MSG'
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
  |  ^~~~
net/can/af_can.c:891:2: note: in expansion of macro 'BUILD_BUG_ON'
  891 |  BUILD_BUG_ON(offsetof(struct can_frame, len) !=
  |  ^~~~


vim +/__compiletime_assert_515 +315 include/linux/compiler_types.h

eb5c2d4b45e3d2 Will Deacon 2020-07-21  301
eb5c2d4b45e3d2 Will Deacon 2020-07-21  302  #define 
_compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21  303  __compiletime_assert(condition, 
msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  304
eb5c2d4b45e3d2 Will Deacon 2020-07-21  305  /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21  306   * compiletime_assert - break build 
and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  307   * @condition: a compile-time 
constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21  308   * @msg:   a message to emit if 
condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  309   *
eb5c2d4b45e3d2 Will Deacon 2020-07-21  310   * In tradition of POSIX assert, 
this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  311   * supplied condition is *false*, 
emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  312   * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21  313   */
eb5c2d4b45e3d2 Will Deacon 2020-07-21  314  #define 
compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @315  _compiletime_assert(condition, 
msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  316

:: The code at line 315 was first introduced by commit
:: eb5c2d4b45e3d2d5d052ea6b8f1463976b1020d5 compiler.h: Move 
compiletime_assert() macros into compiler_types

Re: [RFC PATCH net v2] net: introduce CAN specific pointer in the struct net_device

2021-02-12 Thread Oliver Hartkopp


Hello Oleksij,

nice cleanup - and I like the removal of the notifier in af_can.c :-)

Two questions/hints from my side:

On 12.02.21 13:52, Oleksij Rempel wrote:


diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index d9281ae853f8..912401788d93 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -239,6 +239,7 @@ void can_setup(struct net_device *dev)
  struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int 
echo_skb_max,
unsigned int txqs, unsigned int rxqs)
  {
+   struct can_ml_priv *can;


This should not be named 'can' but e.g. 'can_ml'.

'can' is already used for the struct netns_can:

$ git grep netns_can
include/net/net_namespace.h:struct netns_cancan;
include/net/netns/can.h:struct netns_can {

which is also used in af_can.c and will create some naming confusion.

Maybe the latter could be renamed to can_ns too (later).

But 'can' alone does not tell what the variable is about IMO.


diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index bfadf3b82f9c..9a4c6d14098c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1584,6 +1584,16 @@ enum netdev_priv_flags {
  #define IFF_L3MDEV_RX_HANDLER IFF_L3MDEV_RX_HANDLER
  #define IFF_LIVE_RENAME_OKIFF_LIVE_RENAME_OK
  
+/**

+ * enum netdev_ml_priv_type -  net_device ml_priv_type
+ *
+ * This enum specifies the type of the struct net_device::ml_priv pointer.
+ */
+enum netdev_ml_priv_type {
+   ML_PRIV_NONE,
+   ML_PRIV_CAN,
+};
+
  /**
   *struct net_device - The DEVICE structure.
   *
@@ -1779,6 +1789,7 @@ enum netdev_priv_flags {
   *@nd_net:Network namespace this network device is inside
   *
   *@ml_priv:   Mid-layer private
+   @ml_priv_type:  Mid-layer private type
   *@lstats:Loopback statistics
   *@tstats:Tunnel statistics
   *@dstats:Dummy statistics
@@ -2100,6 +2111,7 @@ struct net_device {
struct pcpu_sw_netstats __percpu*tstats;
struct pcpu_dstats __percpu *dstats;
};
+   enum netdev_ml_priv_typeml_priv_type;


I wonder if it makes more sense to *remove* ml_priv from this union in 
include/linux/netdevice.h and just put it behind the union:


/* mid-layer private */
union {
void *ml_priv;
struct pcpu_lstats __percpu *lstats;
struct pcpu_sw_netstats __percpu *tstats;
struct pcpu_dstats __percpu *dstats;
};

When doing git grep for ml_priv a bunch of users shows up - which all 
have nothing to do with statistics.


I just looks fishy to combine things into a union that have a completely 
different purpose - and we might finally run into similar problems like 
today.


Best regards,
Oliver

Re: include/linux/compiler_types.h:315:38: error: call to '__compiletime_assert_498' declared with attribute error: BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(struct canfd_frame,

2021-02-11 Thread Oliver Hartkopp


Hello,

On 11.02.21 17:51, kernel test robot wrote:

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   291009f656e8eaebbdfd3a8d99f6b190a9ce9deb
commit: c7b74967799b1af52b3045d69d4c26836b2d41de can: replace can_dlc as 
variable/element for payload length
date:   3 months ago
config: arm-randconfig-r015-20210210 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
 wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
 chmod +x ~/bin/make.cross
 # 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c7b74967799b1af52b3045d69d4c26836b2d41de


This hash references to the commit ("can: replace can_dlc as 
variable/element for payload length") which makes a change to the 
BUILD_BUG_ON() content but this does not cause the root problem.


Going back some commits shows the commit ("can: add optional DLC element 
to Classical CAN frame structure") with hash


https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ea7800565a128c1adafa1791ce80afd6016fe21c

is introducing the reported problem.

I assume BUILD_BUG_ON() does not work with the newly introduced union{} 
in struct can_frame in this specific (arm) architecture.


There are no compile issues like this on x86.

When removing the union{} from the struct can_frame and replacing it 
just with 'len' in the reported c7b74967799b1 commit, it compiles 
without problems.


Regards,
Oliver


 git remote add linus 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
 git fetch --no-tags linus master
 git checkout c7b74967799b1af52b3045d69d4c26836b2d41de
 # save the attached .config to linux build tree
 COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

In file included from :
net/can/af_can.c: In function 'can_init':

include/linux/compiler_types.h:315:38: error: call to 
'__compiletime_assert_498' declared with attribute error: BUILD_BUG_ON failed: 
offsetof(struct can_frame, len) != offsetof(struct canfd_frame, len) || 
offsetof(struct can_frame, data) != offsetof(struct canfd_frame, data)

  315 |  _compiletime_assert(condition, msg, __compiletime_assert_, 
__COUNTER__)
  |  ^
include/linux/compiler_types.h:296:4: note: in definition of macro 
'__compiletime_assert'
  296 |prefix ## suffix();\
  |^~
include/linux/compiler_types.h:315:2: note: in expansion of macro 
'_compiletime_assert'
  315 |  _compiletime_assert(condition, msg, __compiletime_assert_, 
__COUNTER__)
  |  ^~~
include/linux/build_bug.h:39:37: note: in expansion of macro 
'compiletime_assert'
   39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
  | ^~
include/linux/build_bug.h:50:2: note: in expansion of macro 
'BUILD_BUG_ON_MSG'
   50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
  |  ^~~~
net/can/af_can.c:891:2: note: in expansion of macro 'BUILD_BUG_ON'
  891 |  BUILD_BUG_ON(offsetof(struct can_frame, len) !=
  |  ^~~~


vim +/__compiletime_assert_498 +315 include/linux/compiler_types.h

eb5c2d4b45e3d2 Will Deacon 2020-07-21  301
eb5c2d4b45e3d2 Will Deacon 2020-07-21  302  #define 
_compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21  303  __compiletime_assert(condition, 
msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  304
eb5c2d4b45e3d2 Will Deacon 2020-07-21  305  /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21  306   * compiletime_assert - break build 
and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  307   * @condition: a compile-time 
constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21  308   * @msg:   a message to emit if 
condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  309   *
eb5c2d4b45e3d2 Will Deacon 2020-07-21  310   * In tradition of POSIX assert, 
this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  311   * supplied condition is *false*, 
emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  312   * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21  313   */
eb5c2d4b45e3d2 Will Deacon 2020-07-21  314  #define 
compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @315  _compiletime_assert(condition, 
msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  316

:: The code at line 315 was first introduced by commit
:: eb5c2d4b45e3d2d

[PATCH v4] drm/panel: simple: add SGD GKTW70SDAD1SD

2021-02-08 Thread Oliver Graute

Add support for the Solomon Goldentek Display Model: GKTW70SDAD1SD
to panel-simple.

The panel spec from Variscite can be found at:
https://www.variscite.com/wp-content/uploads/2017/12/VLCD-CAP-GLD-RGB.pdf

Signed-off-by: Oliver Graute 
Reviewed-by: Marco Felsch 
Reviewed-by: Fabio Estevam 
---

v4:

- added the datasheet labels
- added Reviewed-by

v3:

- added flags
- added delay

v2:

- changed bpc to 6
- set max value of pixelclock
- increased hfront_porch and hback_porch
- dropped connector-type

adding of bus_format = MEDIA_BUS_FMT_RGB666_1X18 results in wrong colors.
omitting bus_format and using some default is better (Tux Pinguin is colored
fine)

 drivers/gpu/drm/panel/panel-simple.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 2be358f..c63f6a8 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -3336,6 +3336,36 @@ static const struct panel_desc satoz_sat050at40h12r2 = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
+static const struct display_timing sgd_gktw70sdad1sd_timing = {
+   .pixelclock = {3000, 3000, 4000},
+   .hactive = { 800, 800, 800},
+   .hfront_porch = {40, 40, 40},
+   .hback_porch = {40, 40, 40},
+   .hsync_len = {48, 48, 48},
+   .vactive = {480, 480, 480},
+   .vfront_porch = {13, 13, 13},
+   .vback_porch = {29, 29, 29},
+   .vsync_len = {3, 3, 3},
+   .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
+   DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_NEGEDGE,
+};
+
+static const struct panel_desc sgd_gktw70sdad1sd = {
+   .timings = _gktw70sdad1sd_timing,
+   .num_timings = 1,
+   .bpc = 6,
+   .size = {
+   .width = 153,
+   .height = 86,
+   },
+   .delay = {
+   .prepare = 20 + 20 + 10 + 10, /* T0 + T2 + T3 + T4 */
+   .enable = 50, /* T5 */
+   .disable = 50, /* T5 */
+   .unprepare =  10 + 10 + 20 + 20, /* T4 + T3 + T2 + T0 */
+   },
+};
+
 static const struct drm_display_mode sharp_ld_d5116z01b_mode = {
.clock = 168480,
.hdisplay = 1920,
@@ -4222,6 +4252,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "satoz,sat050at40h12r2",
.data = _sat050at40h12r2,
}, {
+   .compatible = "sgd,gktw70sdad1sd",
+   .data = _gktw70sdad1sd,
+   }, {
.compatible = "sharp,ld-d5116z01b",
.data = _ld_d5116z01b,
}, {
-- 
2.7.4

Re: [PATCH] arch:powerpc simple_write_to_buffer return check

2021-02-04 Thread Oliver O'Halloran

On Fri, Feb 5, 2021 at 5:17 AM Mayank Suman  wrote:
>
> Signed-off-by: Mayank Suman 

commit messages aren't optional

> ---
>  arch/powerpc/kernel/eeh.c| 8 
>  arch/powerpc/platforms/powernv/eeh-powernv.c | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 813713c9120c..2dbe1558a71f 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1628,8 +1628,8 @@ static ssize_t eeh_force_recover_write(struct file 
> *filp,
> char buf[20];
> int ret;
>
> -   ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
> -   if (!ret)
> +   ret = simple_write_to_buffer(buf, sizeof(buf)-1, ppos, user_buf, 
> count);

We should probably be zeroing the buffer. Reading to sizeof(buf) - 1
is done in a few places to guarantee that the string is nul
terminated, but without the preceeding memset() that isn't actually
guaranteed.

> +   if (ret <= 0)
> return -EFAULT;

EFAULT is supposed to be returned when the user supplies a buffer to
write(2) which is outside their address space. I figured letting the
sscanf() in the next step fail if the user passes writes a zero-length
buffer and returning EINVAL made more sense. That said, the exact
semantics around zero length writes are pretty handwavy so I guess
this isn't wrong, but I don't think it's better either.

> /*
> @@ -1696,7 +1696,7 @@ static ssize_t eeh_dev_check_write(struct file *filp,
>
> memset(buf, 0, sizeof(buf));
> ret = simple_write_to_buffer(buf, sizeof(buf)-1, ppos, user_buf, 
> count);
> -   if (!ret)
> +   if (ret <= 0)
> return -EFAULT;
>
> ret = sscanf(buf, "%x:%x:%x.%x", , , , );
> @@ -1836,7 +1836,7 @@ static ssize_t eeh_dev_break_write(struct file *filp,
>
> memset(buf, 0, sizeof(buf));
> ret = simple_write_to_buffer(buf, sizeof(buf)-1, ppos, user_buf, 
> count);
> -   if (!ret)
> +   if (ret <= 0)
> return -EFAULT;
>
> ret = sscanf(buf, "%x:%x:%x.%x", , , , );
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c 
> b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 89e22c460ebf..36ed2b8f7375 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -76,8 +76,8 @@ static ssize_t pnv_eeh_ei_write(struct file *filp,
> return -ENXIO;
>
> /* Copy over argument buffer */
> -   ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
> -   if (!ret)
> +   ret = simple_write_to_buffer(buf, sizeof(buf)-1, ppos, user_buf, 
> count);
> +   if (ret <= 0)
> return -EFAULT;
>
> /* Retrieve parameters */
> --
> 2.30.0
>

Re: [PATCH v3] drm/panel: simple: add SGD GKTW70SDAD1SD

2021-02-04 Thread Oliver Graute

On 02/02/21, Marco Felsch wrote:
> Hi Oliver,
> 
> On 21-02-02 18:35, Oliver Graute wrote:
> > Add support for the Solomon Goldentek Display Model: GKTW70SDAD1SD
> > to panel-simple.
> > 
> > The panel spec from Variscite can be found at:
> > https://www.variscite.com/wp-content/uploads/2017/12/VLCD-CAP-GLD-RGB.pdf
> > 
> > Signed-off-by: Oliver Graute 
> > Cc: Marco Felsch 
> > Cc: Fabio Estevam 
> > ---
> > 
> > v3:
> > 
> > - added flags
> > - added delay
> 
> Thanks, did you test the changes?
> I just picked it from the datasheet.

yes, it didn't break anything. 

Best regards,

Oliver

[PATCH v3] drm/panel: simple: add SGD GKTW70SDAD1SD

2021-02-02 Thread Oliver Graute

Add support for the Solomon Goldentek Display Model: GKTW70SDAD1SD
to panel-simple.

The panel spec from Variscite can be found at:
https://www.variscite.com/wp-content/uploads/2017/12/VLCD-CAP-GLD-RGB.pdf

Signed-off-by: Oliver Graute 
Cc: Marco Felsch 
Cc: Fabio Estevam 
---

v3:

- added flags
- added delay

v2:

- changed bpc to 6
- set max value of pixelclock
- increased hfront_porch and hback_porch
- dropped connector-type

adding of bus_format = MEDIA_BUS_FMT_RGB666_1X18 results in wrong colors.
omitting bus_format and using some default is better (Tux Pinguin is colored
fine)

 drivers/gpu/drm/panel/panel-simple.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 2be358f..c63f6a8 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -3336,6 +3336,36 @@ static const struct panel_desc satoz_sat050at40h12r2 = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
+static const struct display_timing sgd_gktw70sdad1sd_timing = {
+   .pixelclock = {3000, 3000, 4000},
+   .hactive = { 800, 800, 800},
+   .hfront_porch = {40, 40, 40},
+   .hback_porch = {40, 40, 40},
+   .hsync_len = {48, 48, 48},
+   .vactive = {480, 480, 480},
+   .vfront_porch = {13, 13, 13},
+   .vback_porch = {29, 29, 29},
+   .vsync_len = {3, 3, 3},
+   .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
+   DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_NEGEDGE,
+};
+
+static const struct panel_desc sgd_gktw70sdad1sd = {
+   .timings = _gktw70sdad1sd_timing,
+   .num_timings = 1,
+   .bpc = 6,
+   .size = {
+   .width = 153,
+   .height = 86,
+   },
+   .delay = {
+   .prepare = 20 + 20 + 10 + 10,
+   .enable = 50,
+   .disable = 50,
+   .unprepare =  10 + 10 + 20 + 20,
+   },
+};
+
 static const struct drm_display_mode sharp_ld_d5116z01b_mode = {
.clock = 168480,
.hdisplay = 1920,
@@ -4222,6 +4252,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "satoz,sat050at40h12r2",
.data = _sat050at40h12r2,
}, {
+   .compatible = "sgd,gktw70sdad1sd",
+   .data = _gktw70sdad1sd,
+   }, {
.compatible = "sharp,ld-d5116z01b",
.data = _ld_d5116z01b,
}, {
-- 
2.7.4

Re: [PATCH] powerpc/eeh: remove unneeded semicolon

2021-02-01 Thread Oliver O'Halloran

On Tue, Feb 2, 2021 at 2:21 PM Yang Li  wrote:
>
> Eliminate the following coccicheck warning:
> ./arch/powerpc/kernel/eeh.c:782:2-3: Unneeded semicolon

Doesn't appear to be a load-bearing semicolon. It's hard to tell with EEH.

Reviewed-by: Oliver O'Halloran 

>
> Reported-by: Abaci Robot 
> Signed-off-by: Yang Li 
> ---
>  arch/powerpc/kernel/eeh.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 813713c..02c058d 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -779,7 +779,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, 
> enum pcie_reset_state stat
> default:
> eeh_pe_state_clear(pe, EEH_PE_ISOLATED | EEH_PE_CFG_BLOCKED, 
> true);
> return -EINVAL;
> -   };
> +   }
>
> return 0;
>  }
> --
> 1.8.3.1
>

Re: [PATCH v2] drm/panel: simple: add SGD GKTW70SDAD1SD

2021-02-01 Thread Oliver Graute

On 01/02/21, Marco Felsch wrote:
> Hi Oliver,
> 
> thanks for the patch :)
> 
> On 21-01-29 20:09, Oliver Graute wrote:
> > Add support for the Solomon Goldentek Display Model: GKTW70SDAD1SD
> > to panel-simple.
> > 
> > The panel spec from Variscite can be found at:
> > https://www.variscite.com/wp-content/uploads/2017/12/VLCD-CAP-GLD-RGB.pdf
> > 
> > Signed-off-by: Oliver Graute 
> > Cc: Marco Felsch 
> > Cc: Fabio Estevam 
> > ---
> > 
> > v2:
> > 
> > - changed bpc to 6
> > - set max value of pixelclock
> > - increased hfront_porch and hback_porch
> > - dropped connector-type
> > 
> > adding of bus_format = MEDIA_BUS_FMT_RGB666_1X18 results in wrong colors.
> > omitting bus_format and using some default is good (Tux Pinguin is colored
> > fine)
> > 
> >  drivers/gpu/drm/panel/panel-simple.c | 26 ++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> > b/drivers/gpu/drm/panel/panel-simple.c
> > index 2be358f..c129a8c 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -3336,6 +3336,28 @@ static const struct panel_desc satoz_sat050at40h12r2 
> > = {
> > .connector_type = DRM_MODE_CONNECTOR_LVDS,
> >  };
> >  
> > +static const struct display_timing sgd_gktw70sdad1sd_timing = {
> > +   .pixelclock = {3000, 3000, 4000},
> > +   .hactive = { 800, 800, 800},
> > +   .hfront_porch = {40, 40, 40},
> > +   .hback_porch = {40, 40, 40},
> > +   .hsync_len = {48, 48, 48},
> > +   .vactive = {480, 480, 480},
> > +   .vfront_porch = {13, 13, 13},
> > +   .vback_porch = {29, 29, 29},
> > +   .vsync_len = {3, 3, 3},
> 
> Please add also:
> 
>   .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
>DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_NEGEDGE,
ok will do

> 
> > +};
> > +
> > +static const struct panel_desc sgd_gktw70sdad1sd = {
> > +   .timings = _gktw70sdad1sd_timing,
> > +   .num_timings = 1,
> > +   .bpc = 6,
> > +   .size = {
> > +   .width = 153,
> > +   .height = 86,
> > +   },
> 
> and:
> 
>   .delay = {
>   .prepare = 20 + 20 + 10 + 10, /* T0 + T2 + T3 + T4 */
>   .enable = 50, /* T5 */
>   .disable = 50, /* T5 */
>   .unprepare =  10 + 10 + 20 + 20, /* T4 + T3 + T2 + T0 */
>   };

ok will do

thx for your review.

Best regards,

Oliver

Re: [PATCH 2/2] net: usbnet: use new tasklet API

2021-02-01 Thread Oliver Neukum

Am Samstag, den 23.01.2021, 18:32 +0100 schrieb Emil Renner Berthing:
> From: Emil Renner Berthing 
> 
> This converts the driver to use the new tasklet API introduced in
> commit 12cc923f1ccc ("tasklet: Introduce new initialization API")
> 
> Signed-off-by: Emil Renner Berthing 
Acked-by: Oliver Neukum

Re: [PATCH 1/2] net: usbnet: initialize tasklet using tasklet_init

2021-02-01 Thread Oliver Neukum

Am Samstag, den 23.01.2021, 18:32 +0100 schrieb Emil Renner Berthing:
> From: Emil Renner Berthing 
> 
> Initialize tasklet using tasklet_init() rather than open-coding it.
> 
> Signed-off-by: Emil Renner Berthing 
Acked-by: Oliver Neukum

Re: [PATCH] usbnet: fix the indentation of one code snippet

2021-02-01 Thread Oliver Neukum

Am Samstag, den 23.01.2021, 13:11 +0800 schrieb Dongliang Mu:
> Every line of code should start with tab (8 characters)
> 
> Signed-off-by: Dongliang Mu 
Acked-by: Oliver Neukum

[PATCH v2] drm/panel: simple: add SGD GKTW70SDAD1SD

2021-01-29 Thread Oliver Graute

Add support for the Solomon Goldentek Display Model: GKTW70SDAD1SD
to panel-simple.

The panel spec from Variscite can be found at:
https://www.variscite.com/wp-content/uploads/2017/12/VLCD-CAP-GLD-RGB.pdf

Signed-off-by: Oliver Graute 
Cc: Marco Felsch 
Cc: Fabio Estevam 
---

v2:

- changed bpc to 6
- set max value of pixelclock
- increased hfront_porch and hback_porch
- dropped connector-type

adding of bus_format = MEDIA_BUS_FMT_RGB666_1X18 results in wrong colors.
omitting bus_format and using some default is good (Tux Pinguin is colored
fine)

 drivers/gpu/drm/panel/panel-simple.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 2be358f..c129a8c 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -3336,6 +3336,28 @@ static const struct panel_desc satoz_sat050at40h12r2 = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
+static const struct display_timing sgd_gktw70sdad1sd_timing = {
+   .pixelclock = {3000, 3000, 4000},
+   .hactive = { 800, 800, 800},
+   .hfront_porch = {40, 40, 40},
+   .hback_porch = {40, 40, 40},
+   .hsync_len = {48, 48, 48},
+   .vactive = {480, 480, 480},
+   .vfront_porch = {13, 13, 13},
+   .vback_porch = {29, 29, 29},
+   .vsync_len = {3, 3, 3},
+};
+
+static const struct panel_desc sgd_gktw70sdad1sd = {
+   .timings = _gktw70sdad1sd_timing,
+   .num_timings = 1,
+   .bpc = 6,
+   .size = {
+   .width = 153,
+   .height = 86,
+   },
+};
+
 static const struct drm_display_mode sharp_ld_d5116z01b_mode = {
.clock = 168480,
.hdisplay = 1920,
@@ -4222,6 +4244,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "satoz,sat050at40h12r2",
.data = _sat050at40h12r2,
}, {
+   .compatible = "sgd,gktw70sdad1sd",
+   .data = _gktw70sdad1sd,
+   }, {
.compatible = "sharp,ld-d5116z01b",
.data = _ld_d5116z01b,
}, {
-- 
2.7.4

Re: [binfmt_elf] d97e11e25d: ltp.DS000.fail

2021-01-28 Thread Oliver Sang

On Tue, Jan 26, 2021 at 09:03:26AM +0100, Geert Uytterhoeven wrote:
> Hi Oliver,
> 
> On Tue, Jan 26, 2021 at 6:35 AM kernel test robot  
> wrote:
> > FYI, we noticed the following commit (built with gcc-9):
> >
> > commit: d97e11e25dd226c44257284f95494bb06d1ebf5a ("[PATCH v2] binfmt_elf: 
> > Fix fill_prstatus() call in fill_note_info()")
> > url: 
> > https://github.com/0day-ci/linux/commits/Geert-Uytterhoeven/binfmt_elf-Fix-fill_prstatus-call-in-fill_note_info/20210106-155236
> > base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 
> > e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62
> 
> My patch (which you applied on top of v5.11-rc2) is a build fix for
> a commit that is not part of v5.11-rc2.  Hence the test run is invalid.

sorry for false report. we've fixed the problem. Thanks

> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds

Re: [PATCH v1] drm/panel: simple: add SGD GKTW70SDAD1SD

2021-01-26 Thread Oliver Graute

On 26/01/21, Fabio Estevam wrote:
> Hi Oliver,
> 
> On Mon, Jan 25, 2021 at 7:17 PM Oliver Graute  wrote:
> 
> > I would prefer mine, because I got a wrong colored penguin on bootup
> > with yours :-)

The wrong colored Tux is caused by the bus_format:

.bus_format = MEDIA_BUS_FMT_RGB888_1X24,

So I assume I need another bus_format here.  

> 
> I have originally passed .bpc = 8, but looking at the panel datasheet,
> this should be:
> .bpc = 6 instead.

 yes this is right. I found it too in the datasheet. I'll fix it in next
 version of the patch.
> 
> In your patch, you pass the timing parameters three times, but they
> are all the same.
> 
> Usually, it is meant to be: minimal, typical, maximum values.

yes because on a lot of entries there is just the typical value and no min
and max. But not on all of them.

Best regards,

Oliver

Re: [BUG]: KVM: arm64: Fix symbol dependency in __hyp_call_panic_nvhe

2021-01-26 Thread Oliver Upton

On Mon, Jan 25, 2021 at 12:56 PM Marc Zyngier  wrote:
> - Cherry-pick 9fd339a45be5 ("arm64: Work around broken GCC 4.9
>   handling of "S" constraint"), which works around this particular GCC
>   bug
>
> - Cherry-pick dca5244d2f5b ("compiler.h: Raise minimum version of GCC
>   to 5.1 for arm64"), which forbids GCC 4.9 as it has been
>   demonstrated to mis-compile the kernel (and this patch is targeting
>   stable anyway)

If the latter is hitting stable then it sounds like high time to throw
out my broken GCC. Thanks Marc!

--
Oliver

Re: [PATCH v1] drm/panel: simple: add SGD GKTW70SDAD1SD

2021-01-25 Thread Oliver Graute

On 16/01/21, Fabio Estevam wrote:
> On Sat, Jan 16, 2021 at 9:49 AM Oliver Graute  wrote:
> 
> > > power-supply = <_touch_3v3> is not correct, as the reg_touch_3v3
> > > does not power the LCD.
> >
> > yes, but how is the LCD correctly powered then?
> 
> J4 is powered by VCC_5V and VCC_3V#.
> 
> > [7.795980] pwm-backlight backlight: supply power not found, using dummy 
> > regulator
> > [7.804436] OF: /backlight: #pwm-cells = 3 found -1
> > [7.806563] of_pwm_get(): can't parse "pwms" property
> > [7.812026] pwm-backlight backlight: unable to request PWM
> > [7.822929] pwm-backlight: probe of backlight failed with error -22
> 
> You need to fix this.
> 
> > [7.876831] imx-sdma 20ec000.sdma: Direct firmware load for 
> > imx/sdma/sdma-imx6q.bin failed with error -2
> > [7.884231] imx-sdma 20ec000.sdma: Falling back to sysfs fallback for: 
> > imx/sdma/sdma-imx6q.bin
> > [7.916013] printk: console [ttymxc0] enabled
> > [7.916013] printk: console [ttymxc0] enabled
> > [7.922351] printk: bootconsole [ec_imx6q0] disabled
> > [7.922351] printk: bootconsole [ec_imx6q0] disabled
> > [7.952397] 21e8000.serial: ttymxc1 at MMIO 0x21e8000 (irq = 68, 
> > base_baud = 500) is a IMX
> > [7.970794] 21ec000.serial: ttymxc2 at MMIO 0x21ec000 (irq = 69, 
> > base_baud = 500) is a IMX
> > [8.033015] [ cut here ]
> > [8.037826] WARNING: CPU: 0 PID: 1 at 
> > ../drivers/gpu/drm/panel/panel-simple.c:577 panel_simple_probe+0x5dc/0x6b8
> 
> And this too
> 
> > [8.846104] imx6ul-pinctrl 20e.pinctrl: pin MX6UL_PAD_NAND_CE0_B 
> > already requested by regulator-gpio; cannot claim for 
> > 1806000.nand-controller
> > [8.859641] imx6ul-pinctrl 20e.pinctrl: pin-107 
> > (1806000.nand-controller) status -22
> > [8.867851] imx6ul-pinctrl 20e.pinctrl: could not request pin 107 
> > (MX6UL_PAD_NAND_CE0_B) from group gpminandgrp  on device 20e.pinctrl
> > [8.880930] gpmi-nand 1806000.nand-controller: Error applying setting, 
> > reverse things back
> > [8.889726] gpmi-nand: probe of 1806000.nand-controller failed with 
> > error -22
> 
> And this pin conflict too.

Ok I fixed the pin conflict with regulator-gpio and added a 5V
regulator node in my dts file. Now the display is working fine!

I'll post the dts files soon and check if there is something to
improve for this patch.

Many thanks for your help,

Oliver

Re: [PATCH v1] drm/panel: simple: add SGD GKTW70SDAD1SD

2021-01-25 Thread Oliver Graute

On 25/01/21, Fabio Estevam wrote:
> Hi Oliver,
> 
> On Mon, Jan 25, 2021 at 6:29 PM Oliver Graute  wrote:
> 
> > Ok I fixed the pin conflict with regulator-gpio and added a 5V
> > regulator node in my dts file. Now the display is working fine!
> 
> That's good news :-)
> 
> > I'll post the dts files soon and check if there is something to
> > improve for this patch.
> 
> Did the panel patch I sent earlier work?
> https://pastebin.com/raw/diTMVsh8
> 
> If it does, I can send it formally if you want.

I would prefer mine, because I got a wrong colored penguin on bootup
with yours :-)

Best regards,

Oliver

Re: [BUG]: KVM: arm64: Fix symbol dependency in __hyp_call_panic_nvhe

2021-01-25 Thread Oliver Upton

> That means we have two options:
> (a) define __hyp_panic_string in a different .c file in all pre-5.9 branches, 
> or
> (b) revert the backported patch.
> 
> The patch was needed in 5.9 and should stay there. It wasn't needed in earlier
> versions because the symbol was being kept alive by another user. It did "fix"
> the inline asm semantics, but the problem was never triggered in pre-5.9.
> 
> Sasha, with this and the GCC bug in mind, would you agree that (b) is the
> better course of action?

Sasha,

Any chance we can get this patch reverted as David has suggested? It was
backported to 5.4 LTS in commit 653ae33b030b ("KVM: arm64: Fix symbol 
dependency in __hyp_call_panic_nvhe")
and is causing build issues with a 4.9.4 vintage of GCC.

Thanks!

--
Oliver

Re: [PATCH] sh: fix sparse annotation in SH's __get_user_check()

2021-01-25 Thread Oliver Hartkopp


On 23.01.21 18:32, Luc Van Oostenryck wrote:

The pointer in get_user() and friends is supposed to be a __user pointer.
But in SH's implementation of __get_user_check(), the pointer is
assigned to a local variable __gu_addr which is lacking the __user
annotation. As consequence, a warning is issued for every of its uses.

So, add the missing __user annotation (this remove ~700 warnings when
using defconfig).


Many thanks that you were able to spot this issue, Luc!

Best regards,
Oliver



Link: https://lore.kernel.org/r/202101141753.ropiz9nh-...@intel.com
Reported-by: kernel test robot 
Reported-by: Oliver Hartkopp 
Signed-off-by: Luc Van Oostenryck 
---
  arch/sh/include/asm/uaccess.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sh/include/asm/uaccess.h b/arch/sh/include/asm/uaccess.h
index 73f3b48d4a34..e2623fe2ac09 100644
--- a/arch/sh/include/asm/uaccess.h
+++ b/arch/sh/include/asm/uaccess.h
@@ -68,7 +68,7 @@ struct __large_struct { unsigned long buf[100]; };
  ({\
long __gu_err = -EFAULT;\
unsigned long __gu_val = 0; \
-   const __typeof__(*(ptr)) *__gu_addr = (ptr);\
+   const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
if (likely(access_ok(__gu_addr, (size   \
__get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
(x) = (__force __typeof__(*(ptr)))__gu_val; \

base-commit: 5c8fe583cce542aa0b84adc939ce85293de36e5e

Re: [workqueue] d5bff968ea: WARNING:at_kernel/workqueue.c:#process_one_work

2021-01-20 Thread Oliver Sang

On Fri, Jan 15, 2021 at 03:24:32PM +0800, Hillf Danton wrote:
> Thu, 14 Jan 2021 15:45:11 +0800
> > 
> > FYI, we noticed the following commit (built with gcc-9):
> > 
> > commit: d5bff968ea9cc005e632d9369c26cbd8148c93d5 ("workqueue: break 
> > affinity initiatively")
> > https://git.kernel.org/cgit/linux/kernel/git/paulmck/linux-rcu.git 
> > dev.2021.01.11b
> > 
> [...]
> > 
> > [   73.794288] WARNING: CPU: 0 PID: 22 at kernel/workqueue.c:2192 
> > process_one_work
> 
> Thanks for your report.
> 
> We can also break CPU affinity by checking POOL_DISASSOCIATED at attach 
> time without extra cost paid; that way we have the same behavior as at
> the unbind time.
> 
> What is more the change that makes kworker pcpu is cut because they are
> going to not help either hotplug or the mechanism of stop machine.

hi, by applying below patch, the issue still happened.

[ 4.574467] pci :00:00.0: Limiting direct PCI/PCI transfers
[ 4.575651] pci :00:01.0: Activating ISA DMA hang workarounds
[ 4.576900] pci :00:02.0: Video device with shadowed ROM at [mem 
0x000c-0x000d]
[ 4.578648] PCI: CLS 0 bytes, default 64
[ 4.579685] Unpacking initramfs...
[ 8.878031] ---[ cut here ]---
[ 8.879083] WARNING: CPU: 0 PID: 22 at kernel/workqueue.c:2187 
process_one_work+0x92/0x9e0
[ 8.880688] Modules linked in:
[ 8.881274] CPU: 0 PID: 22 Comm: kworker/1:0 Not tainted 
5.11.0-rc3-gc213503139bb #2
[ 8.882518] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.12.0-1 04/01/2014
[ 8.887539] Workqueue: 0x0 (events)
[ 8.887838] EIP: process_one_work+0x92/0x9e0
[ 8.887838] Code: 37 64 a1 58 54 4c 43 39 45 24 74 2c 31 c9 ba 01 00 00 00 c7 
04 24 01 00 00 00 b8 08 1d f5 42 e8 74 85 13 00 ff 05 b8 30 04 43 <0f> 0b ba 01 
00 00 00 eb 22 8d 74 26 00 90 c7 04 24 01 00 00 00 31
[ 8.887838] EAX: 42f51d08 EBX:  ECX:  EDX: 0001
[ 8.887838] ESI: 43c04720 EDI: 42e45620 EBP: de7f23c0 ESP: 43d7bf08
[ 8.887838] DS: 007b ES: 007b FS: 00d8 GS:  SS: 0068 EFLAGS: 00010002
[ 8.887838] CR0: 80050033 CR2:  CR3: 034e3000 CR4: 000406d0
[ 8.887838] Call Trace:
[ 8.887838] ? worker_thread+0x98/0x6a0
[ 8.887838] ? worker_thread+0x2dd/0x6a0
[ 8.887838] ? kthread+0x1ba/0x1e0
[ 8.887838] ? create_worker+0x1e0/0x1e0
[ 8.887838] ? kzalloc+0x20/0x20
[ 8.887838] ? ret_from_fork+0x1c/0x28
[ 8.887838] _warn_unseeded_randomness: 63 callbacks suppressed
[ 8.887838] random: get_random_bytes called from init_oops_id+0x2b/0x60 with 
crng_init=0
[ 8.887838] --[ end trace ac461b4d54c37cfa ]--
[ 11.287055] Freeing initrd memory: 174228K
[ 11.289225] RAPL PMU: API unit is 2^-32 Joules, 0 fixed counters, 10737418240 
ms ovfl timer
[ 11.290889] clocksource: tsc: mask: 0x max_cycles: 
0x26d34b60feb, max_idle_ns: 440795225049 ns
[ 11.292884] mce: Machine check injector initialized
[ 11.313019] The force parameter has not been set to 1. The Iris poweroff 
handler will not be installed.

> 
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1847,22 +1847,17 @@ static void worker_attach_to_pool(struct
>  struct worker_pool *pool)
>  {
>   mutex_lock(_pool_attach_mutex);
> -
> - /*
> -  * set_cpus_allowed_ptr() will fail if the cpumask doesn't have any
> -  * online CPUs.  It'll be re-applied when any of the CPUs come up.
> -  */
> - set_cpus_allowed_ptr(worker->task, pool->attrs->cpumask);
> -
>   /*
>* The wq_pool_attach_mutex ensures %POOL_DISASSOCIATED remains
>* stable across this function.  See the comments above the flag
>* definition for details.
>*/
> - if (pool->flags & POOL_DISASSOCIATED)
> + if (pool->flags & POOL_DISASSOCIATED) {
>   worker->flags |= WORKER_UNBOUND;
> - else
> - kthread_set_per_cpu(worker->task, true);
> + set_cpus_allowed_ptr(worker->task, cpu_possible_mask);
> + } else {
> + set_cpus_allowed_ptr(worker->task, pool->attrs->cpumask);
> + }
>  
>   list_add_tail(>node, >workers);
>   worker->pool = pool;
> @@ -4922,7 +4917,6 @@ static void unbind_workers(int cpu)
>   raw_spin_unlock_irq(>lock);
>  
>   for_each_pool_worker(worker, pool) {
> - kthread_set_per_cpu(worker->task, false);
>   WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task, 
> cpu_possible_mask) < 0);
>   }
>  
> @@ -4979,7 +4973,6 @@ static void rebind_workers(struct worker
>   for_each_pool_worker(worker, pool) {
>   WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task,
> pool->attrs->cpumask) < 0);
> - kthread_set_per_cpu(worker->task, true);
>   }
>  
>   raw_spin_lock_irq(>lock);
> --


dmesg-2.xz
Description: application/xz

Re: [workqueue] d5bff968ea: WARNING:at_kernel/workqueue.c:#process_one_work

2021-01-20 Thread Oliver Sang

On Thu, Jan 14, 2021 at 04:42:48PM +0800, Hillf Danton wrote:
> Thu, 14 Jan 2021 15:45:11 +0800
> > 
> > FYI, we noticed the following commit (built with gcc-9):
> > 
> > commit: d5bff968ea9cc005e632d9369c26cbd8148c93d5 ("workqueue: break 
> > affinity initiatively")
> > https://git.kernel.org/cgit/linux/kernel/git/paulmck/linux-rcu.git 
> > dev.2021.01.11b
> > 
> > 
> > in testcase: rcutorture
> > version: 
> > with following parameters:
> > 
> > runtime: 300s
> > test: cpuhotplug
> > torture_type: srcud
> > 
> > test-description: rcutorture is rcutorture kernel module load/unload test.
> > test-url: https://www.kernel.org/doc/Documentation/RCU/torture.txt
> > 
> > 
> > on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
> > 
> > caused below changes (please refer to attached dmesg/kmsg for entire 
> > log/backtrace):
> > 
> > 
> > +--+++
> > |  | 6211b34f6e | 
> > d5bff968ea |
> > +--+++
> > | boot_successes   | 4  | 0 
> >  |
> > | boot_failures| 0  | 12
> >  |
> > | WARNING:at_kernel/workqueue.c:#process_one_work  | 0  | 12
> >  |
> > | EIP:process_one_work | 0  | 12
> >  |
> > | WARNING:at_kernel/kthread.c:#kthread_set_per_cpu | 0  | 4 
> >  |
> > | EIP:kthread_set_per_cpu  | 0  | 4 
> >  |
> > +--+++
> > 
> > 
> > If you fix the issue, kindly add following tag
> > Reported-by: kernel test robot 
> > 
> > 
> > [   73.794288] WARNING: CPU: 0 PID: 22 at kernel/workqueue.c:2192 
> > process_one_work (kbuild/src/consumer/kernel/workqueue.c:2192) 
> > [   73.795012] Modules linked in: rcutorture torture mousedev evbug 
> > input_leds led_class psmouse pcspkr tiny_power_button button
> > [   73.795949] CPU: 0 PID: 22 Comm: kworker/1:0 Not tainted 
> > 5.11.0-rc3-gd5bff968ea9c #2
> > [   73.796592] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> > 1.12.0-1 04/01/2014
> > [   73.797280] Workqueue:  0x0 (rcu_gp)
> > [   73.797592] EIP: process_one_work 
> > (kbuild/src/consumer/kernel/workqueue.c:2192) 
> 
> 
> Can you run the reproducer with the changes to WQ cut?

hi, by applying below patch, the issue still happened. detail dmesg is attached.

[ 2.505530] TCP: Hash tables configured (established 32768 bind 32768)
[ 2.506668] ---[ cut here ]---
[ 2.508080] WARNING: CPU: 0 PID: 23 at kernel/workqueue.c:2190 
process_one_work+0x92/0x9e0
[ 2.509963] Modules linked in:
[ 2.510692] CPU: 0 PID: 23 Comm: kworker/1:0H Not tainted 
5.11.0-rc3-00186-ge7792535d216 #2
[ 2.512608] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.12.0-1 04/01/2014
[ 2.514499] EIP: process_one_work+0x92/0x9e0
[ 2.515468] Code: 37 64 a1 58 54 4c 43 39 45 24 74 2c 31 c9 ba 01 00 00 00 c7 
04 24 01 00 00 00 b8 08 1d f5 42 e8 74 85 13 00 ff 05 b8 30 04 43 <0f> 0b ba 01 
00 00 00 eb 22 8d 74 26 00 90 c7 04 24 01 00 00 00 31
[ 2.516539] EAX: 42f51d08 EBX:  ECX:  EDX: 0001
[ 2.516539] ESI: 43c04780 EDI: de7eb3ec EBP: de7f25e0 ESP: 43d83f08
[ 2.516539] DS: 007b ES: 007b FS: 00d8 GS:  SS: 0068 EFLAGS: 00010002
[ 2.516539] CR0: 80050033 CR2:  CR3: 034e3000 CR4: 000406d0
[ 2.516539] Call Trace:
[ 2.516539] ? rcuwait_wake_up+0x53/0x80
[ 2.516539] ? rcuwait_wake_up+0x5/0x80
[ 2.516539] ? worker_thread+0x2dd/0x6a0
[ 2.516539] ? kthread+0x1ba/0x1e0
[ 2.516539] ? create_worker+0x1e0/0x1e0
[ 2.516539] ? kzalloc+0x20/0x20
[ 2.516539] ? ret_from_fork+0x1c/0x28
[ 2.516539] --[ end trace 71c162214dd31179 ]--
[ 2.534063] UDP hash table entries: 2048 (order: 5, 196608 bytes, linear)
[ 2.535774] UDP-Lite hash table entries: 2048 (order: 5, 196608 bytes, linear)
[ 2.537661] NET: Registered protocol family 1

> 
> It seems special to make kworker pcpu because they are going not to
> help either hotplug or stop. If it quiesces the warning then we have
> a fresh start for breaking CPU affinity.
> 
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1861,8 +1861,6 @@ static void worker_attach_to_pool(struct
>*/
>   if (pool->flags & POOL_DISASSOCIATED)
>   worker->flags |= WORKER_UNBOUND;
> - else
> - kthread_set_per_cpu(worker->task, true);
>  
>   list_add_tail(>node, >workers);
>   worker->pool = pool;
> @@ -4922,7 +4920,6 @@ static void unbind_workers(int cpu)
>   raw_spin_unlock_irq(>lock);
>  
>   for_each_pool_worker(worker, pool) {
> - kthread_set_per_cpu(worker->task, false);
>   WARN_ON_ONCE(set_cpus_allowed_ptr(worker->task, 
> cpu_possible_mask) < 0);
>

Re: Splicing to/from a tty

2021-01-20 Thread Oliver Giles

On Wed Jan 20, 2021 at 5:44 PM NZDT, Linus Torvalds wrote:
>
> But in the meantime, here's two more patches to try on top of the
> previous four. They shouldn't matter, other than making the non-icanon
> throughput a lot better. But the more coverage they get, the happier
> I'll be.
>

I tried these out too, my use case is still working well.

Re: Splicing to/from a tty

2021-01-19 Thread Oliver Giles

On Wed Jan 20, 2021 at 9:24 AM NZDT, Linus Torvalds wrote:
> Anyway, anybody willing to test these tty/pipe patches on the loads
> that failed? Oliver?

Writing this from a kernel with those patches in; happily splice()ing
to and from a pty.

Re: Splicing to/from a tty

2021-01-19 Thread Oliver Giles

On Wed Jan 20, 2021 at 5:56 AM NZDT, Robert Karszniewicz wrote:
>
> I have bisected this issue down to this commit:
> 4d03e3cc5982 ("fs: don't allow kernel reads and writes without iter
> ops")
>
> Another case I've also noticed is writing to a serial connection:
> kernel write not supported for file /ttymxc0 (pid: 252 comm: cat)
>

Tangentially, I hit the same thing when hacking on this. Here's a diff
making the implementation match the comment:

--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -541,7 +541,7 @@ ssize_t __kernel_write(struct file *file, const void *buf, 
size_t count, loff_t
 * Also fail if ->write_iter and ->write are both wired up as that
 * implies very convoluted semantics.
 */
-   if (unlikely(!file->f_op->write_iter || file->f_op->write))
+   if (unlikely(file->f_op->write_iter && file->f_op->write))
return warn_unsupported(file, "write");
 
init_sync_kiocb(, file);

Re: Splicing to/from a tty

2021-01-16 Thread Oliver Giles

On Sun Jan 17, 2021 at 5:46 AM NZDT, Johannes Berg wrote:
> On Sat, 2021-01-16 at 20:35 +1300, Oliver Giles wrote:
> > Commit 36e2c7421f02 (fs: don't allow splice read/write without
> > explicit ops) broke my userspace application which talks to an SSL VPN
> > by splice()ing between "openssl s_client" and "pppd". The latter
> > operates over a pty, and since that commit there is no fallback for
> > splice()ing between a pipe and a pty, or any tty for that matter.
> > 
> > The above commit mentions switching them to the iter ops and using
> > generic_file_splice_read. IIUC, this would require implementing iter
> > ops also on the line disciplines, which sounds pretty disruptive.
> > 
> > For my case, I attempted to instead implement splice_write and
> > splice_read in tty_fops; I managed to get splice_write working calling
> > ld->ops->write, but splice_read is not so simple because the
> > tty_ldisc_ops read method expects a userspace buffer. So I cannot see
> > how to implement this without either (a) using set_fs, or (b)
> > implementing iter ops on all line disciplines.
> > 
> > Is splice()ing between a tty and a pipe worth supporting at all? Not a
> > big deal for my use case at least, but it used to work.
>
> Is it even strictly related to the tty?
>
> I was just now looking into why my cgit/fcgi/nginx setup no longer
> works, and the reason is getting -EINVAL from sendfile() when the input
> is a file and the output is a pipe().
>
> So I wrote a simple test program (below) and that errors out on kernel
> 5.10.4, while it works fine on the 5.9.16 I currently have. Haven't
> tried reverting anything yet, but now that I haev a test program it
> should be simple to even bisect.
>
> johannes
>
>
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
>
> int main(int argc, char **argv)
> {
> int in = open(argv[0], O_RDONLY);
> int p[2], out;
> off_t off = 0;
> int err;
>
> assert(in >= 0);
> assert(pipe(p) >= 0);
> out = p[1];
> err = sendfile(out, in, , 1024);
> if (err < 0)
> perror("sendfile");
> assert(err == 1024);
>
> return 0;
> }

I can confirm the behaviour you see, and that it starts occurring from the same
commit 36e2c7421f02a22 (fs: don't allow splice read/write without explicit ops).

In my tty case, it is clear that removing the default fallback would cause this
to fail, but assuming the sendfile() source is on a regular filesystem, I am
unsure why splice cannot find the appropriate splice_write method. Could be
connected to the fact that the message from warn_unsupported in fs/splice.c
outputs "splice write not supported for file  (pid: 983819 comm: 
test-sendfile)",
i.e. the file path is blank. In my case of directly calling splice on a pty, I
do see a path such as /ptyp0 in that error before implementing 
splice_(read/write)
callbacks. Maybe splice is getting a bogus file pointer from sendfile?

Re: [PATCH v1] drm/panel: simple: add SGD GKTW70SDAD1SD

2021-01-16 Thread Oliver Graute

On 10/01/21, Fabio Estevam wrote:
> On Sun, Jan 10, 2021 at 5:09 PM Oliver Graute  wrote:
> 
> > here the schematics and my dts. The board is using a LVDS connector for
> > the display.
> 
> The schematics shows the GKTW70SDAD1SD panel in the J4 connector, not
> the LVDS J7 connector.

yes you are right. But how to I map this fact correctly in my dts?

> > https://www.variscite.de/wp-content/uploads/2017/12/VAR-6ULCustomboard-Schematics.pdf
> > https://lore.kernel.org/linux-arm-kernel/1610144511-19018-3-git-send-email-oliver.gra...@gmail.com/
> 
> As I mentioned earlier you should remove the display timings from the
> dts when using the compatible string for the panel.

I got this and removed the display timings.

> 
> power-supply = <_touch_3v3> is not correct, as the reg_touch_3v3
> does not power the LCD.

yes, but how is the LCD correctly powered then? 

> 
> Another hint is to use the PLL5_VIDEO as the clock source for the
> lcdif controller as done in the imx6ul evk dtsi.

I already figured it out and tried it. But because of the faults above
it didn't make any difference.  

> 
> It would also help if you could share the complete boot log.

here is the boot log


Starting kernel ...

[0.00] Booting Linux on physical CPU 0x0
[0.00] Linux version 5.10.0-test-6-gc24ef7716d4b (oliver@ripley) 
(arm-linux-gnueabihf-gcc (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609, 
GNU ld (GNU Binutils for Ubuntu) 2.26.1) #1 SMP Sat Jan 16 13:28:37 CET 2021
[0.00] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), cr=10c5387d
[0.00] CPU: div instructions available: patching division code
[0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing 
instruction cache
[0.00] OF: fdt: Machine model: Variscite i.MX6 UltraLite Carrier-board
[0.00] earlycon: ec_imx6q0 at MMIO 0x0202 (options '')
[0.00] printk: bootconsole [ec_imx6q0] enabled
[0.00] Memory policy: Data cache writealloc
[0.00] cma: Reserved 64 MiB at 0x9c00
[0.00] Zone ranges:
[0.00]   Normal   [mem 0x8000-0x9fff]
[0.00]   HighMem  empty
[0.00] Movable zone start for each node
[0.00] Early memory node ranges
[0.00]   node   0: [mem 0x8000-0x9fff]
[0.00] Initmem setup node 0 [mem 0x8000-0x9fff]
[0.00] percpu: Embedded 18 pages/cpu s51692 r0 d22036 u73728
[0.00] Built 1 zonelists, mobility grouping on.  Total pages: 130048
[0.00] Kernel command line: console=ttymxc0,115200,115200 root=/dev/nfs 
ip=dhcp nfsroot=192.168.3.13:/volume1/nfs/rootfs/,v3,tcp rw earlycon
[0.00] Dentry cache hash table entries: 65536 (order: 6, 262144 bytes, 
linear)
[0.00] Inode-cache hash table entries: 32768 (order: 5, 131072 bytes, 
linear)
[0.00] mem auto-init: stack:off, heap alloc:off, heap free:off
[0.00] Memory: 422620K/524288K available (16384K kernel code, 2090K 
rwdata, 4636K rodata, 1024K init, 6680K bss, 36132K reserved, 65536K 
cma-reserved, 0K highmem)
[0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[0.00] Running RCU self tests
[0.00] rcu: Hierarchical RCU implementation.
[0.00] rcu: RCU event tracing is enabled.
[0.00] rcu: RCU lockdep checking is enabled.
[0.00] rcu: RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=1.
[0.00] rcu: RCU calculated value of scheduler-enlistment delay is 10 
jiffies.
[0.00] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=1
[0.00] NR_IRQS: 16, nr_irqs: 16, preallocated irqs: 16
[0.00] random: get_random_bytes called from start_kernel+0x350/0x570 
with crng_init=0
[0.00] Switching to timer-based delay loop, resolution 41ns
[0.23] sched_clock: 32 bits at 24MHz, resolution 41ns, wraps every 
89478484971ns
[0.007847] clocksource: mxc_timer1: mask: 0x max_cycles: 
0x, max_idle_ns: 79635851949 ns
[0.021360] Console: colour dummy device 80x30
[0.023088] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., 
Ingo Molnar
[0.031763] ... MAX_LOCKDEP_SUBCLASSES:  8
[0.034908] ... MAX_LOCK_DEPTH:  48
[0.039082] ... MAX_LOCKDEP_KEYS:8192
[0.043532] ... CLASSHASH_SIZE:  4096
[0.047784] ... MAX_LOCKDEP_ENTRIES: 32768
[0.052216] ... MAX_LOCKDEP_CHAINS:  65536
[0.056739] ... CHAINHASH_SIZE:  32768
[0.061095]  memory used by lock dependency info: 4061 kB
[0.066484]  memory used for stack traces: 2112 kB
[0.071350]  per task-struct memory footprint: 1536 bytes
[0.076832] Calibrating delay loop (skipped), value calculated using timer 
frequency.. 48.00 BogoMIPS (lpj=24)
[0.087133] pid_max: default: 32768 minimum: 301
[0.092643] Moun

Splicing to/from a tty

2021-01-16 Thread Oliver Giles

Commit 36e2c7421f02 (fs: don't allow splice read/write without explicit ops) 
broke my userspace application which talks to an SSL VPN by splice()ing between 
"openssl s_client" and "pppd". The latter operates over a pty, and since that 
commit there is no fallback for splice()ing between a pipe and a pty, or any 
tty for that matter.

The above commit mentions switching them to the iter ops and using 
generic_file_splice_read. IIUC, this would require implementing iter ops also 
on the line disciplines, which sounds pretty disruptive.

For my case, I attempted to instead implement splice_write and splice_read in 
tty_fops; I managed to get splice_write working calling ld->ops->write, but 
splice_read is not so simple because the tty_ldisc_ops read method expects a 
userspace buffer. So I cannot see how to implement this without either (a) 
using set_fs, or (b) implementing iter ops on all line disciplines.

Is splice()ing between a tty and a pipe worth supporting at all? Not a big deal 
for my use case at least, but it used to work.

Re: net/can/isotp.c:1240:13: sparse: sparse: incorrect type in initializer (different address spaces)

2021-01-14 Thread Oliver Hartkopp





On 14.01.21 10:47, kernel test robot wrote:

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   65f0d2414b7079556fbbcc070b3d1c9f9587606d
commit: e057dd3fc20ffb3d7f150af46542a51b59b90127 can: add ISO 15765-2:2016 
transport protocol
date:   3 months ago
config: sh-randconfig-s032-20210114 (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0
reproduce:
 wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
 chmod +x ~/bin/make.cross
 # apt-get install sparse
 # sparse version: v0.6.3-208-g46a52ca4-dirty
 # 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e057dd3fc20ffb3d7f150af46542a51b59b90127
 git remote add linus 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
 git fetch --no-tags linus master
 git checkout e057dd3fc20ffb3d7f150af46542a51b59b90127
 # save the attached .config to linux build tree
 COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=sh

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 


"sparse warnings: (new ones prefixed by >>)"

net/can/isotp.c:1240:13: sparse: sparse: incorrect type in initializer 
(different address spaces) @@ expected int const *__gu_addr @@ got int 
[noderef] __user *optlen @@

net/can/isotp.c:1240:13: sparse: expected int const *__gu_addr
net/can/isotp.c:1240:13: sparse: got int [noderef] __user *optlen

net/can/isotp.c:1240:13: sparse: sparse: incorrect type in argument 1 
(different address spaces) @@ expected void const volatile [noderef] __user 
*ptr @@ got int const *__gu_addr @@

net/can/isotp.c:1240:13: sparse: expected void const volatile [noderef] 
__user *ptr
net/can/isotp.c:1240:13: sparse: got int const *__gu_addr



This seems to be a problem with the sh4 arch and/or its cross 
compilation tools.


There are tons of similar code snippets in the kernel, e.g. in 
net/can/raw.c or net/bluetooth/hci_sock.c ...


And these code snippets do not trigger such sparse warnings?!?

Any idea?

Regards,
Oliver


vim +1240 net/can/isotp.c

   1229 
   1230 static int isotp_getsockopt(struct socket *sock, int level, int optname,
   1231 char __user *optval, int __user *optlen)
   1232 {
   1233 struct sock *sk = sock->sk;
   1234 struct isotp_sock *so = isotp_sk(sk);
   1235 int len;
   1236 void *val;
   1237 
   1238 if (level != SOL_CAN_ISOTP)
   1239 return -EINVAL;

1240if (get_user(len, optlen))

   1241 return -EFAULT;
   1242 if (len < 0)
   1243 return -EINVAL;
   1244 
   1245 switch (optname) {
   1246 case CAN_ISOTP_OPTS:
   1247 len = min_t(int, len, sizeof(struct can_isotp_options));
   1248 val = >opt;
   1249 break;
   1250 
   1251 case CAN_ISOTP_RECV_FC:
   1252 len = min_t(int, len, sizeof(struct 
can_isotp_fc_options));
   1253 val = >rxfc;
   1254 break;
   1255 
   1256 case CAN_ISOTP_TX_STMIN:
   1257 len = min_t(int, len, sizeof(u32));
   1258 val = >force_tx_stmin;
   1259 break;
   1260 
   1261 case CAN_ISOTP_RX_STMIN:
   1262 len = min_t(int, len, sizeof(u32));
   1263 val = >force_rx_stmin;
   1264 break;
   1265 
   1266 case CAN_ISOTP_LL_OPTS:
   1267 len = min_t(int, len, sizeof(struct 
can_isotp_ll_options));
   1268 val = >ll;
   1269 break;
   1270 
   1271 default:
   1272 return -ENOPROTOOPT;
   1273 }
   1274 
   1275 if (put_user(len, optlen))
   1276 return -EFAULT;
   1277 if (copy_to_user(optval, val, len))
   1278 return -EFAULT;
   1279 return 0;
   1280 }
   1281 

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org

Re: KMSAN: kernel-infoleak in move_addr_to_user (4)

2021-01-12 Thread Oliver Hartkopp





On 12.01.21 01:17, Cong Wang wrote:

On Mon, Jan 11, 2021 at 11:33 AM Jakub Kicinski  wrote:


Looks like a AF_CAN socket:

r0 = socket(0x1d, 0x2, 0x6)
getsockname$packet(r0, &(0x7f000100)={0x11, 0x0, 0x0, 0x1, 0x0, 0x6, 
@broadcast}, &(0x7f00)=0x14)



Right, it seems we need a memset(0) in isotp_getname().


Yes m(

Sent a patch to fix it:

https://lore.kernel.org/linux-can/20210112090457.11262-1-socket...@hartkopp.net/T/#u

Many thanks!

Oliver

Re: [PATCH v2] can: mcba_usb: Fix memory leak when cancelling urb

2021-01-11 Thread Oliver Neukum

Am Montag, den 11.01.2021, 10:49 + schrieb Bui Quang Minh:
> In mcba_usb_read_bulk_callback(), when we don't resubmit or fails to
> resubmit the urb, we need to deallocate the transfer buffer that is
> allocated in mcba_usb_start().
> 
> Reported-by: syzbot+57281c762a3922e14...@syzkaller.appspotmail.com
> Signed-off-by: Bui Quang Minh 
> ---
> v1: add memory leak fix when not resubmitting urb
> v2: add memory leak fix when failing to resubmit urb
> 
>  drivers/net/can/usb/mcba_usb.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
> index df54eb7d4b36..30236e640116 100644
> --- a/drivers/net/can/usb/mcba_usb.c
> +++ b/drivers/net/can/usb/mcba_usb.c
> @@ -584,6 +584,8 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
>   case -EPIPE:
>   case -EPROTO:
>   case -ESHUTDOWN:
> + usb_free_coherent(urb->dev, urb->transfer_buffer_length,
> +   urb->transfer_buffer, urb->transfer_dma);
>   return;
>  

Can you call usb_free_coherent() in what can be hard IRQ context?

Regards
Oliver

Re: [PATCH v1] drm/panel: simple: add SGD GKTW70SDAD1SD

On 10/01/21, Fabio Estevam wrote:
> Hi Oliver,
> 
> On Sun, Jan 10, 2021 at 12:35 PM Oliver Graute  
> wrote:
> 
> > the first two errors are gone. But I still get this:
> >
> > [   42.387107] mxsfb 21c8000.lcdif: Cannot connect bridge: -517
> >
> > The panel is still off perhaps I miss something else.
> 
> Some suggestions:
> 
> - Take a look at arch/arm/boot/dts/imx6ul-14x14-evk.dtsi as a
> reference as it has display functional
> - Use imx_v6_v7_defconfig to make sure all the required drivers are selected

ok I checked imx6ul-14x14-evk.dtsi and use imx_v6_v7_defconfig

> - If it still does not work, share the dts and schematics

here the schematics and my dts. The board is using a LVDS connector for
the display.

https://www.variscite.de/wp-content/uploads/2017/12/VAR-6ULCustomboard-Schematics.pdf
https://lore.kernel.org/linux-arm-kernel/1610144511-19018-3-git-send-email-oliver.gra...@gmail.com/

Thx for your help,

Best Regards,

Oliver

Re: [PATCH v8 2/3] ARM: dts: Add support for i.MX6 UltraLite DART Variscite Customboard

On 09/01/21, Fabio Estevam wrote:
> On Fri, Jan 8, 2021 at 7:23 PM Oliver Graute  wrote:
> 
> > +   panel1: panel-lcd {
> > +   compatible = "sgd,gktw70sdad1sd";
> > +
> > +   backlight = <_lcd>;
> > +   power-supply = <_touch_3v3>;
> > +   label = "gktw70sdad1sd";
> > +
> > +   display-timing {
> 
> If you pass the compatible, then you don't need to add the
> display-timing in the device tree.

thx I`ll drop it

Best Regards,

Oliver

Re: [PATCH v3 3/3] dt-bindings: arm: fsl: Add Variscite i.MX6UL compatibles

On 09/01/21, Fabio Estevam wrote:
> On Fri, Jan 8, 2021 at 7:23 PM Oliver Graute  wrote:
> 
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml 
> > b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index 05906e2..5f74d78 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -240,6 +240,7 @@ properties:
> >- technexion,imx6ul-pico-dwarf   # TechNexion i.MX6UL 
> > Pico-Dwarf
> >- technexion,imx6ul-pico-hobbit  # TechNexion i.MX6UL 
> > Pico-Hobbit
> >- technexion,imx6ul-pico-pi  # TechNexion i.MX6UL Pico-Pi
> > +  - variscite,imx6ul-var-6ulcustomboard # i.MX UltraLite 
> > Carrier-board
> 
> You missed to add a "6" in the description: i.MX6 UltraLite Carrier-board

I will add it.

thx

Best regards,

Oliver

Re: [PATCH v1] drm/panel: simple: add SGD GKTW70SDAD1SD

On 09/01/21, Fabio Estevam wrote:
> Hi Oliver,
> 
> On Fri, Jan 8, 2021 at 7:24 PM Oliver Graute  wrote:
> >
> > On 19/12/20, Oliver Graute wrote:
> > > Add support for the Solomon Goldentek Display Model: GKTW70SDAD1SD
> > > to panel-simple.
> > >
> > > The panel spec from Variscite can be found at:
> > > https://www.variscite.com/wp-content/uploads/2017/12/VLCD-CAP-GLD-RGB.pdf
> >
> > some clue what bus_format and bus_flags I have to use?
> >
> > [   42.505156] panel-simple panel-lcd: Specify missing bus_flags
> > [   42.511090] panel-simple panel-lcd: Specify missing bus_format
> > [   42.615131] mxsfb 21c8000.lcdif: Cannot connect bridge: -517
> 
> Does this patch work?
> https://pastebin.com/raw/diTMVsh8

the first two errors are gone. But I still get this:

[   42.387107] mxsfb 21c8000.lcdif: Cannot connect bridge: -517

The panel is still off perhaps I miss something else. 

Best Regards,

Oliver

Re: [PATCH v1] drm/panel: simple: add SGD GKTW70SDAD1SD

On 19/12/20, Oliver Graute wrote:
> Add support for the Solomon Goldentek Display Model: GKTW70SDAD1SD
> to panel-simple.
> 
> The panel spec from Variscite can be found at:
> https://www.variscite.com/wp-content/uploads/2017/12/VLCD-CAP-GLD-RGB.pdf

some clue what bus_format and bus_flags I have to use?

[   42.505156] panel-simple panel-lcd: Specify missing bus_flags
[   42.511090] panel-simple panel-lcd: Specify missing bus_format
[   42.615131] mxsfb 21c8000.lcdif: Cannot connect bridge: -517

Best Regards,

Oliver

[PATCH v3 3/3] dt-bindings: arm: fsl: Add Variscite i.MX6UL compatibles

Add the compatibles for Variscite i.MX6UL compatibles

Signed-off-by: Oliver Graute 
---

Changelog:

 v3:
 - rebased

 v2:
 - renamed binding
 - removed superflous "

 Documentation/devicetree/bindings/arm/fsl.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml 
b/Documentation/devicetree/bindings/arm/fsl.yaml
index 05906e2..5f74d78 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -240,6 +240,7 @@ properties:
   - technexion,imx6ul-pico-dwarf   # TechNexion i.MX6UL Pico-Dwarf
   - technexion,imx6ul-pico-hobbit  # TechNexion i.MX6UL Pico-Hobbit
   - technexion,imx6ul-pico-pi  # TechNexion i.MX6UL Pico-Pi
+  - variscite,imx6ul-var-6ulcustomboard # i.MX UltraLite 
Carrier-board
   - const: fsl,imx6ul
 
   - description: i.MX6UL PHYTEC phyBOARD-Segin
-- 
2.7.4

[PATCH v8 1/3] ARM: dts: imx6ul: Add Variscite DART-6UL SoM support

This patch adds support for the i.MX6UL variant of the Variscite DART-6UL
SoM Carrier-Board

Signed-off-by: Oliver Graute 
Cc: Shawn Guo 
Cc: Neil Armstrong 
Cc: Marco Felsch 
Cc: Parthiban Nallathambi 
---
 .../boot/dts/imx6ul-imx6ull-var-dart-common.dtsi   | 300 +
 1 file changed, 300 insertions(+)

Changelog:

v8:
 - remove can node
 - remove flexscan pinctrl
 - moved lcd and i2c pinctrl
 - sorted regulators
 - add dedicated pinctrl for dvfs regulator

v7:
 - removed cpu0 node
 - fixed phy problem

v6:
 - renamed touch regulator
 - renamed rmii clock
 - moved some muxing to baseboard
 - added pinctrl for gpio key
 - added bus-width to usdhc1
 - fixed missing subnode on partitions

 create mode 100644 arch/arm/boot/dts/imx6ul-imx6ull-var-dart-common.dtsi

diff --git a/arch/arm/boot/dts/imx6ul-imx6ull-var-dart-common.dtsi 
b/arch/arm/boot/dts/imx6ul-imx6ull-var-dart-common.dtsi
new file mode 100644
index ..913a66f
--- /dev/null
+++ b/arch/arm/boot/dts/imx6ul-imx6ull-var-dart-common.dtsi
@@ -0,0 +1,300 @@
+// SPDX-License-Identifier: (GPL-2.0)
+/dts-v1/;
+
+#include "imx6ul.dtsi"
+/ {
+   chosen {
+   stdout-path = 
+   };
+
+   memory@8000 {
+   device_type = "memory";
+   reg = <0x8000 0x2000>;
+   };
+
+   clk_rmii_ref: clock-rmii-ref {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <2500>;
+   clock-output-names = "rmii-ref";
+   };
+
+   reg_gpio_dvfs: regulator-gpio {
+   compatible = "regulator-gpio";
+   gpios = < 13 GPIO_ACTIVE_HIGH>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_dvfs_reg>;
+   regulator-min-microvolt = <130>;
+   regulator-max-microvolt = <140>;
+   regulator-name = "gpio_dvfs";
+   regulator-type = "voltage";
+   enable-active-high;
+   states = <130 0x1 140 0x0>;
+   };
+
+   reg_sd1_vmmc: regulator-sd1-vmmc {
+   compatible = "regulator-fixed";
+   regulator-name = "VSD_3V3";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   };
+
+   reg_touch_3v3: regulator-touch-3v3 {
+   compatible = "regulator-fixed";
+   regulator-name = "touch_3v3_supply";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   };
+
+};
+
+ {
+   vref-supply = <_touch_3v3>;
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_enet1>;
+   phy-mode = "rmii";
+   phy-handle = <>;
+   phy-reset-gpios=< 10 1>;
+   phy-reset-duration=<100>;
+   phy-reset-on-resume;
+
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ethphy0: ethernet-phy@1 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   micrel,rmii-reference-clock-select-25-mhz;
+   clocks = <_rmii_ref>;
+   clock-names = "rmii-ref";
+   reg = <1>;
+   };
+
+   ethphy1: ethernet-phy@3 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   micrel,rmii-reference-clock-select-25-mhz;
+   clocks = <_rmii_ref>;
+   clock-names = "rmii-ref";
+   reg = <3>;
+   };
+   };
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_gpmi_nand>;
+   status = "okay";
+};
+
+ {
+   clock-frequency = <40>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_i2c1>;
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pwm1>;
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_sai2>;
+   assigned-clocks = < IMX6UL_CLK_SAI2_SEL>,
+ < IMX6UL_CLK_SAI2>;
+   assigned-clock-parents = < IMX6UL_CLK_PLL4_AUDIO_DIV>;
+   assigned-clock-rates = <0>, <12288000>;
+   fsl,sai-mclk-direction-output;
+   status = "okay";
+};
+
+_poweroff {
+   status = "okay";
+};
+
+_rtc {
+   status = "disabled";
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_uart2>;
+   uart-has-rtscts;
+};
+
+ {
+   pinctrl-names = "default", "state_100mhz", "state_200mhz

[PATCH v8 2/3] ARM: dts: Add support for i.MX6 UltraLite DART Variscite Customboard

This patch adds DeviceTree Source for the i.MX6 UltraLite DART NAND/WIFI

Signed-off-by: Oliver Graute 
Cc: Shawn Guo 
Cc: Neil Armstrong 
Cc: Marco Felsch 
Cc: Parthiban Nallathambi 
---
Changelog:

v8:
 - backlight droped the status line
 - port the display panel
 - added pinctrl for touch

v7:
 - fixed wakeup-source

v6:
 - added some muxing
 - added codec in sound node
 - added adc1 node

 arch/arm/boot/dts/Makefile  |   1 +
 arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts | 270 
 2 files changed, 271 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 3d1ea0b..7a73b72 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -634,6 +634,7 @@ dtb-$(CONFIG_SOC_IMX6UL) += \
imx6ul-tx6ul-0010.dtb \
imx6ul-tx6ul-0011.dtb \
imx6ul-tx6ul-mainboard.dtb \
+   imx6ul-var-6ulcustomboard.dtb \
imx6ull-14x14-evk.dtb \
imx6ull-colibri-eval-v3.dtb \
imx6ull-colibri-wifi-eval-v3.dtb \
diff --git a/arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts 
b/arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts
new file mode 100644
index ..e647d22
--- /dev/null
+++ b/arch/arm/boot/dts/imx6ul-var-6ulcustomboard.dts
@@ -0,0 +1,270 @@
+// SPDX-License-Identifier: (GPL-2.0)
+/*
+ * Support for Variscite DART-6UL Module
+ *
+ * Copyright (C) 2015 Freescale Semiconductor, Inc.
+ * Copyright (C) 2015-2016 Variscite Ltd. - http://www.variscite.com
+ * Copyright (C) 2018-2021 Oliver Graute 
+ */
+
+/dts-v1/;
+
+#include 
+#include "imx6ul-imx6ull-var-dart-common.dtsi"
+
+/ {
+   model = "Variscite i.MX6 UltraLite Carrier-board";
+   compatible = "variscite,6ulcustomboard", "fsl,imx6ul";
+
+   backlight_lcd: backlight {
+   compatible = "pwm-backlight";
+   pwms = < 0 2>;
+   brightness-levels = <0 4 8 16 32 64 128 255>;
+   default-brightness-level = <6>;
+   status = "okay";
+   };
+
+   gpio-keys {
+   compatible = "gpio-keys";
+   pinctrl-names = "default";
+   pinctrl-0 = <_gpio_keys>;
+
+   user {
+   gpios = < 0 GPIO_ACTIVE_LOW>;
+   linux,code = ;
+   wakeup-source;
+   };
+   };
+
+   gpio-leds {
+   compatible = "gpio-leds";
+   pinctrl-names = "default";
+   pinctrl-0 = <_gpio_leds>;
+
+   d16-led {
+   gpios = < 20 GPIO_ACTIVE_HIGH>;
+   linux,default-trigger = "heartbeat";
+   };
+   };
+
+   panel1: panel-lcd {
+   compatible = "sgd,gktw70sdad1sd";
+
+   backlight = <_lcd>;
+   power-supply = <_touch_3v3>;
+   label = "gktw70sdad1sd";
+
+   display-timing {
+   /* clock-frequency = <29232000>; */
+   clock-frequency = <3500>;
+   hactive = <800>;
+   vactive = <480>;
+   hfront-porch = <40>;
+   hback-porch = <40>;
+   vsync-len = <48>;
+   vback-porch = <29>;
+   vfront-porch = <13>;
+   hsync-len = <3>;
+   hsync-active = <0>;
+   vsync-active = <0>;
+   de-active = <1>;
+   pixelclk-active = <0>;
+   };
+
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
+
+   sound {
+   compatible = "simple-audio-card";
+   simple-audio-card,name = "wm8731audio";
+   simple-audio-card,widgets =
+   "Headphone", "Headphone Jack",
+   "Line", "Line Jack",
+   "Microphone", "Mic Jack";
+   simple-audio-card,routing =
+   "Headphone Jack", "RHPOUT",
+   "Headphone Jack", "LHPOUT",
+   "LLINEIN", "Line Jack",
+   "RLINEIN", "Line Jack",
+   "MICIN", "Mic Bias",
+   "Mic Bias", "Mic Jack";
+   simple-audio-card,format =

[PATCH v8 0/3] Variscite DART-6UL SoM support