* Re: [PATCH AUTOSEL 5.5 222/542] Revert "tty/serial: atmel: fix out of range clock divider handling"
From: Greg Kroah-Hartman @ 2020-02-14 21:51 UTC (permalink / raw)
To: Sasha Levin
Cc: Stephen Rothwell, linux-kernel, stable, linux-serial,
David Engraf, linux-arm-kernel
In-Reply-To: <20200214154854.6746-222-sashal@kernel.org>
On Fri, Feb 14, 2020 at 10:43:34AM -0500, Sasha Levin wrote:
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> [ Upstream commit 6dbd54e4154dfe386b3333687de15be239576617 ]
>
> This reverts commit 751d0017334db9c4d68a8909c59f662a6ecbcec6.
>
> The wrong commit got added to the tty-next tree, the correct one is in
> the tty-linus branch.
>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: David Engraf <david.engraf@sysgo.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> drivers/tty/serial/atmel_serial.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 1ba9bc667e136..ab4d4a0b36497 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2270,6 +2270,9 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> mode |= ATMEL_US_USMODE_NORMAL;
> }
>
> + /* set the mode, clock divisor, parity, stop bits and data size */
> + atmel_uart_writel(port, ATMEL_US_MR, mode);
> +
> /*
> * Set the baud rate:
> * Fractional baudrate allows to setup output frequency more
> --
> 2.20.1
>
Are you sure this is correct to be added? This was the result of some
fun merge problems, I don't think it's needed anywhere else...
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH AUTOSEL 5.5 222/542] Revert "tty/serial: atmel: fix out of range clock divider handling"
From: Greg Kroah-Hartman @ 2020-02-14 21:51 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, stable, Stephen Rothwell, David Engraf,
linux-serial, linux-arm-kernel
In-Reply-To: <20200214154854.6746-222-sashal@kernel.org>
On Fri, Feb 14, 2020 at 10:43:34AM -0500, Sasha Levin wrote:
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> [ Upstream commit 6dbd54e4154dfe386b3333687de15be239576617 ]
>
> This reverts commit 751d0017334db9c4d68a8909c59f662a6ecbcec6.
>
> The wrong commit got added to the tty-next tree, the correct one is in
> the tty-linus branch.
>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: David Engraf <david.engraf@sysgo.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> drivers/tty/serial/atmel_serial.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 1ba9bc667e136..ab4d4a0b36497 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2270,6 +2270,9 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> mode |= ATMEL_US_USMODE_NORMAL;
> }
>
> + /* set the mode, clock divisor, parity, stop bits and data size */
> + atmel_uart_writel(port, ATMEL_US_MR, mode);
> +
> /*
> * Set the baud rate:
> * Fractional baudrate allows to setup output frequency more
> --
> 2.20.1
>
Are you sure this is correct to be added? This was the result of some
fun merge problems, I don't think it's needed anywhere else...
greg k-h
^ permalink raw reply
* Re: [PATCH AUTOSEL 5.5 346/542] driver core: platform: Prevent resouce overflow from causing infinite loops
From: Greg Kroah-Hartman @ 2020-02-14 21:49 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, stable, Simon Schwartz
In-Reply-To: <20200214154854.6746-346-sashal@kernel.org>
On Fri, Feb 14, 2020 at 10:45:38AM -0500, Sasha Levin wrote:
> From: Simon Schwartz <kern.simon@theschwartz.xyz>
>
> [ Upstream commit 39cc539f90d035a293240c9443af50be55ee81b8 ]
>
> num_resources in the platform_device struct is declared as a u32. The
> for loops that iterate over num_resources use an int as the counter,
> which can cause infinite loops on architectures with smaller ints.
> Change the loop counters to u32.
>
> Signed-off-by: Simon Schwartz <kern.simon@theschwartz.xyz>
> Link: https://lore.kernel.org/r/2201ce63a2a171ffd2ed14e867875316efcf71db.camel@theschwartz.xyz
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> drivers/base/platform.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index cf6b6b722e5c9..864b53b3d5980 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -27,6 +27,7 @@
> #include <linux/limits.h>
> #include <linux/property.h>
> #include <linux/kmemleak.h>
> +#include <linux/types.h>
>
> #include "base.h"
> #include "power/power.h"
> @@ -48,7 +49,7 @@ EXPORT_SYMBOL_GPL(platform_bus);
> struct resource *platform_get_resource(struct platform_device *dev,
> unsigned int type, unsigned int num)
> {
> - int i;
> + u32 i;
>
> for (i = 0; i < dev->num_resources; i++) {
> struct resource *r = &dev->resource[i];
> @@ -255,7 +256,7 @@ struct resource *platform_get_resource_byname(struct platform_device *dev,
> unsigned int type,
> const char *name)
> {
> - int i;
> + u32 i;
>
> for (i = 0; i < dev->num_resources; i++) {
> struct resource *r = &dev->resource[i];
> @@ -501,7 +502,8 @@ EXPORT_SYMBOL_GPL(platform_device_add_properties);
> */
> int platform_device_add(struct platform_device *pdev)
> {
> - int i, ret;
> + u32 i;
> + int ret;
>
> if (!pdev)
> return -EINVAL;
> @@ -590,7 +592,7 @@ EXPORT_SYMBOL_GPL(platform_device_add);
> */
> void platform_device_del(struct platform_device *pdev)
> {
> - int i;
> + u32 i;
>
> if (!IS_ERR_OR_NULL(pdev)) {
> device_del(&pdev->dev);
> --
> 2.20.1
>
This doesn't solve a real issue, so please drop from everywhere.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH AUTOSEL 5.5 343/542] misc: genwqe: fix compile warnings
From: Greg Kroah-Hartman @ 2020-02-14 21:49 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, stable, Hongbo Yao, Hulk Robot
In-Reply-To: <20200214154854.6746-343-sashal@kernel.org>
On Fri, Feb 14, 2020 at 10:45:35AM -0500, Sasha Levin wrote:
> From: Hongbo Yao <yaohongbo@huawei.com>
>
> [ Upstream commit 8edf4cd193067ac5e03fd9580f1affbb6a3f729b ]
>
> Using the following command will get compile warnings:
> make W=1 drivers/misc/genwqe/card_ddcb.o ARCH=x86_64
>
> drivers/misc/genwqe/card_ddcb.c: In function setup_ddcb_queue:
> drivers/misc/genwqe/card_ddcb.c:1024:6: warning: variable rc set but not
> used [-Wunused-but-set-variable]
> drivers/misc/genwqe/card_ddcb.c: In function genwqe_card_thread:
> drivers/misc/genwqe/card_ddcb.c:1190:23: warning: variable rc set but
> not used [-Wunused-but-set-variable]
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Hongbo Yao <yaohongbo@huawei.com>
> Link: https://lore.kernel.org/r/20191205111655.170382-1-yaohongbo@huawei.com
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> drivers/misc/genwqe/card_ddcb.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/misc/genwqe/card_ddcb.c b/drivers/misc/genwqe/card_ddcb.c
> index 026c6ca245408..905106579935a 100644
> --- a/drivers/misc/genwqe/card_ddcb.c
> +++ b/drivers/misc/genwqe/card_ddcb.c
> @@ -1084,7 +1084,7 @@ static int setup_ddcb_queue(struct genwqe_dev *cd, struct ddcb_queue *queue)
> queue->ddcb_daddr);
> queue->ddcb_vaddr = NULL;
> queue->ddcb_daddr = 0ull;
> - return -ENODEV;
> + return rc;
>
> }
>
> @@ -1179,7 +1179,7 @@ static irqreturn_t genwqe_vf_isr(int irq, void *dev_id)
> */
> static int genwqe_card_thread(void *data)
> {
> - int should_stop = 0, rc = 0;
> + int should_stop = 0;
> struct genwqe_dev *cd = (struct genwqe_dev *)data;
>
> while (!kthread_should_stop()) {
> @@ -1187,12 +1187,12 @@ static int genwqe_card_thread(void *data)
> genwqe_check_ddcb_queue(cd, &cd->queue);
>
> if (GENWQE_POLLING_ENABLED) {
> - rc = wait_event_interruptible_timeout(
> + wait_event_interruptible_timeout(
> cd->queue_waitq,
> genwqe_ddcbs_in_flight(cd) ||
> (should_stop = kthread_should_stop()), 1);
> } else {
> - rc = wait_event_interruptible_timeout(
> + wait_event_interruptible_timeout(
> cd->queue_waitq,
> genwqe_next_ddcb_ready(cd) ||
> (should_stop = kthread_should_stop()), HZ);
> --
> 2.20.1
>
Again, please drop.
^ permalink raw reply
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
From: Greg KH @ 2020-02-14 21:40 UTC (permalink / raw)
To: Linus Walleij
Cc: Stephen Rothwell, stefano.stabellini@xilinx.com,
Benjamin GAIGNARD, arnd@arndb.de, robh@kernel.org,
shawnguo@kernel.org, s.hauer@pengutronix.de,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Grant Likely, lkml@metux.net, linux-imx@nxp.com,
kernel@pengutronix.de, sudeep.holla@arm.com,
fabio.estevam@nxp.com, Loic PALLARDY,
system-dt@lists.openampproject.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CACRpkdajhivkOkZ63v-hr7+6ObhTffYOx5uZP0P-MYvuVnyweA@mail.gmail.com>
On Fri, Feb 14, 2020 at 05:05:07PM +0100, Linus Walleij wrote:
> On Fri, Jan 31, 2020 at 10:06 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > Why do people want to abuse the platform bus so much? If a device is on
> > a bus that can have such a controller, then it is on a real bus, use it!
>
> I'm not saying it is a good thing, but the reason why it is (ab)used so
> much can be found in:
> drivers/of/platform.c
>
> TL;DR: struct platform_device is the Device McDeviceFace and
> platform bus the Bus McBusFace used by the device tree parser since
> it is slightly to completely unaware of what devices it is actually
> spawning.
<snip>
Yeah, great explaination, and I understand. DT stuff really is ok to be
on a platform bus, as that's what almost all of them are.
But, when you try to start messing around with things like this
"firewall" says it is doing, it's then obvious that this really isn't a
DT like thing, but rather you do have a bus involved with a controller
so that should be used instead.
Or just filter away the DT stuff so that the kernel never even sees
those devices, which might just be simplest :)
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH AUTOSEL 5.5 451/542] tty: n_hdlc: Use flexible-array member and struct_size() helper
From: Greg Kroah-Hartman @ 2020-02-14 21:49 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, stable, Gustavo A. R. Silva, Jiri Slaby
In-Reply-To: <20200214154854.6746-451-sashal@kernel.org>
On Fri, Feb 14, 2020 at 10:47:23AM -0500, Sasha Levin wrote:
> From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
>
> [ Upstream commit 85f4c95172d606dd66f7ee1fa50c45a245535ffd ]
>
> Old code in the kernel uses 1-byte and 0-byte arrays to indicate the
> presence of a "variable length array":
>
> struct something {
> int length;
> u8 data[1];
> };
>
> struct something *instance;
>
> instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
> instance->length = size;
> memcpy(instance->data, source, size);
>
> There is also 0-byte arrays. Both cases pose confusion for things like
> sizeof(), CONFIG_FORTIFY_SOURCE, etc.[1] Instead, the preferred mechanism
> to declare variable-length types such as the one above is a flexible array
> member[2] which need to be the last member of a structure and empty-sized:
>
> struct something {
> int stuff;
> u8 data[];
> };
>
> Also, by making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertenly introduced[3] to the codebase from now on.
>
> Lastly, make use of the struct_size() helper to safely calculate the
> allocation size for instances of struct n_hdlc_buf and avoid any potential
> type mistakes[4][5].
>
> [1] https://github.com/KSPP/linux/issues/21
> [2] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
> [4] https://lore.kernel.org/lkml/60e14fb7-8596-e21c-f4be-546ce39e7bdb@embeddedor.com/
> [5] commit 553d66cb1e86 ("iommu/vt-d: Use struct_size() helper")
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Reviewed-by: Jiri Slaby <jslaby@suse.cz>
> Link: https://lore.kernel.org/r/20200121172138.GA3162@embeddedor
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> drivers/tty/n_hdlc.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
> index 98361acd3053f..27b506bf03ced 100644
> --- a/drivers/tty/n_hdlc.c
> +++ b/drivers/tty/n_hdlc.c
> @@ -115,11 +115,9 @@
> struct n_hdlc_buf {
> struct list_head list_item;
> int count;
> - char buf[1];
> + char buf[];
> };
>
> -#define N_HDLC_BUF_SIZE (sizeof(struct n_hdlc_buf) + maxframe)
> -
> struct n_hdlc_buf_list {
> struct list_head list;
> int count;
> @@ -524,7 +522,8 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
> /* no buffers in free list, attempt to allocate another rx buffer */
> /* unless the maximum count has been reached */
> if (n_hdlc->rx_buf_list.count < MAX_RX_BUF_COUNT)
> - buf = kmalloc(N_HDLC_BUF_SIZE, GFP_ATOMIC);
> + buf = kmalloc(struct_size(buf, buf, maxframe),
> + GFP_ATOMIC);
> }
>
> if (!buf) {
> @@ -853,7 +852,7 @@ static struct n_hdlc *n_hdlc_alloc(void)
>
> /* allocate free rx buffer list */
> for(i=0;i<DEFAULT_RX_BUF_COUNT;i++) {
> - buf = kmalloc(N_HDLC_BUF_SIZE, GFP_KERNEL);
> + buf = kmalloc(struct_size(buf, buf, maxframe), GFP_KERNEL);
> if (buf)
> n_hdlc_buf_put(&n_hdlc->rx_free_buf_list,buf);
> else if (debuglevel >= DEBUG_LEVEL_INFO)
> @@ -862,7 +861,7 @@ static struct n_hdlc *n_hdlc_alloc(void)
>
> /* allocate free tx buffer list */
> for(i=0;i<DEFAULT_TX_BUF_COUNT;i++) {
> - buf = kmalloc(N_HDLC_BUF_SIZE, GFP_KERNEL);
> + buf = kmalloc(struct_size(buf, buf, maxframe), GFP_KERNEL);
> if (buf)
> n_hdlc_buf_put(&n_hdlc->tx_free_buf_list,buf);
> else if (debuglevel >= DEBUG_LEVEL_INFO)
> --
> 2.20.1
>
There's no need for any of these variable length array patches to be
backported anywhere.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH AUTOSEL 5.5 155/542] drm/amdkfd: remove set but not used variable 'top_dev'
From: Greg KH @ 2020-02-14 21:44 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, stable, zhengbin, Hulk Robot, Felix Kuehling,
Alex Deucher, amd-gfx, dri-devel
In-Reply-To: <20200214154854.6746-155-sashal@kernel.org>
On Fri, Feb 14, 2020 at 10:42:27AM -0500, Sasha Levin wrote:
> From: zhengbin <zhengbin13@huawei.com>
>
> [ Upstream commit d191bd678153307573d615bb42da4fcca19fe477 ]
>
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/gpu/drm/amd/amdkfd/kfd_iommu.c: In function kfd_iommu_device_init:
> drivers/gpu/drm/amd/amdkfd/kfd_iommu.c:65:30: warning: variable top_dev set but not used [-Wunused-but-set-variable]
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Fixes: 1ae99eab34f9 ("drm/amdkfd: Initialize HSA_CAP_ATS_PRESENT capability in topology codes")
> Signed-off-by: zhengbin <zhengbin13@huawei.com>
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_iommu.c | 3 ---
> 1 file changed, 3 deletions(-)
Unless all of these "unused bt set variable" patches are needed for
"real" fixes, there's no need to add them here as we are NOT building
the kernel with that option enabled any time soon from what I can tell.
So you can drop a ton of these patches from all of these AUTOSEL
branches please.
thanks,
greg kh
^ permalink raw reply
* Re: [PATCH AUTOSEL 5.5 155/542] drm/amdkfd: remove set but not used variable 'top_dev'
From: Greg KH @ 2020-02-14 21:44 UTC (permalink / raw)
To: Sasha Levin
Cc: dri-devel, Felix Kuehling, linux-kernel, stable, zhengbin,
Hulk Robot, amd-gfx, Alex Deucher
In-Reply-To: <20200214154854.6746-155-sashal@kernel.org>
On Fri, Feb 14, 2020 at 10:42:27AM -0500, Sasha Levin wrote:
> From: zhengbin <zhengbin13@huawei.com>
>
> [ Upstream commit d191bd678153307573d615bb42da4fcca19fe477 ]
>
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/gpu/drm/amd/amdkfd/kfd_iommu.c: In function kfd_iommu_device_init:
> drivers/gpu/drm/amd/amdkfd/kfd_iommu.c:65:30: warning: variable top_dev set but not used [-Wunused-but-set-variable]
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Fixes: 1ae99eab34f9 ("drm/amdkfd: Initialize HSA_CAP_ATS_PRESENT capability in topology codes")
> Signed-off-by: zhengbin <zhengbin13@huawei.com>
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_iommu.c | 3 ---
> 1 file changed, 3 deletions(-)
Unless all of these "unused bt set variable" patches are needed for
"real" fixes, there's no need to add them here as we are NOT building
the kernel with that option enabled any time soon from what I can tell.
So you can drop a ton of these patches from all of these AUTOSEL
branches please.
thanks,
greg kh
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH AUTOSEL 5.5 418/542] pwm: Remove set but not set variable 'pwm'
From: Greg KH @ 2020-02-14 21:46 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Sasha Levin, linux-kernel, stable, yu kuai, Thierry Reding,
linux-pwm
In-Reply-To: <20200214174014.lfnhsl6d7nyfkfbm@pengutronix.de>
On Fri, Feb 14, 2020 at 06:40:14PM +0100, Uwe Kleine-König wrote:
> Hello Sasha,
>
> On Fri, Feb 14, 2020 at 10:46:50AM -0500, Sasha Levin wrote:
> > From: yu kuai <yukuai3@huawei.com>
> >
> > [ Upstream commit 9871abffc81048e20f02e15d6aa4558a44ad53ea ]
> >
> > Fixes gcc '-Wunused-but-set-variable' warning:
> >
> > drivers/pwm/pwm-pca9685.c: In function ‘pca9685_pwm_gpio_free’:
> > drivers/pwm/pwm-pca9685.c:162:21: warning: variable ‘pwm’ set but not used [-Wunused-but-set-variable]
> >
> > It is never used, and so can be removed. In that case, hold and release
> > the lock 'pca->lock' can be removed since nothing will be done between
> > them.
> >
> > Fixes: e926b12c611c ("pwm: Clear chip_data in pwm_put()")
> > Signed-off-by: yu kuai <yukuai3@huawei.com>
> > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > ---
> > drivers/pwm/pwm-pca9685.c | 4 ----
> > 1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c
> > index 168684b02ebce..b07bdca3d510d 100644
> > --- a/drivers/pwm/pwm-pca9685.c
> > +++ b/drivers/pwm/pwm-pca9685.c
> > @@ -159,13 +159,9 @@ static void pca9685_pwm_gpio_set(struct gpio_chip *gpio, unsigned int offset,
> > static void pca9685_pwm_gpio_free(struct gpio_chip *gpio, unsigned int offset)
> > {
> > struct pca9685 *pca = gpiochip_get_data(gpio);
> > - struct pwm_device *pwm;
> >
> > pca9685_pwm_gpio_set(gpio, offset, 0);
> > pm_runtime_put(pca->chip.dev);
> > - mutex_lock(&pca->lock);
> > - pwm = &pca->chip.pwms[offset];
> > - mutex_unlock(&pca->lock);
>
> Even though I bet this change won't introduce a regression, it only
> fixes a harmless warning. So I wonder if it objectively qualifies to be
> applied for stable.
See my response to another one of these types of patches. In order
words, I agree, these aren't needed unless they are prereqs for other
real fixes.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
From: Greg KH @ 2020-02-14 21:40 UTC (permalink / raw)
To: Linus Walleij
Cc: Stephen Rothwell, Grant Likely, Benjamin GAIGNARD,
robh@kernel.org, Loic PALLARDY, arnd@arndb.de,
devicetree@vger.kernel.org, system-dt@lists.openampproject.org,
s.hauer@pengutronix.de, linux-kernel@vger.kernel.org,
lkml@metux.net, linux-imx@nxp.com, kernel@pengutronix.de,
sudeep.holla@arm.com, fabio.estevam@nxp.com,
stefano.stabellini@xilinx.com, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CACRpkdajhivkOkZ63v-hr7+6ObhTffYOx5uZP0P-MYvuVnyweA@mail.gmail.com>
On Fri, Feb 14, 2020 at 05:05:07PM +0100, Linus Walleij wrote:
> On Fri, Jan 31, 2020 at 10:06 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > Why do people want to abuse the platform bus so much? If a device is on
> > a bus that can have such a controller, then it is on a real bus, use it!
>
> I'm not saying it is a good thing, but the reason why it is (ab)used so
> much can be found in:
> drivers/of/platform.c
>
> TL;DR: struct platform_device is the Device McDeviceFace and
> platform bus the Bus McBusFace used by the device tree parser since
> it is slightly to completely unaware of what devices it is actually
> spawning.
<snip>
Yeah, great explaination, and I understand. DT stuff really is ok to be
on a platform bus, as that's what almost all of them are.
But, when you try to start messing around with things like this
"firewall" says it is doing, it's then obvious that this really isn't a
DT like thing, but rather you do have a bus involved with a controller
so that should be used instead.
Or just filter away the DT stuff so that the kernel never even sees
those devices, which might just be simplest :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 2/2] drm/bridge: anx7688: Add anx7688 bridge driver support
From: Enric Balletbo Serra @ 2020-02-14 21:53 UTC (permalink / raw)
To: Vasily Khoruzhick
Cc: Enric Balletbo i Serra, Jernej Skrabec, Nicolas Boichat,
Laurent Pinchart, Neil Armstrong, David Airlie, Torsten Duwe,
Jonas Karlman, linux-kernel, dri-devel, Andrzej Hajda,
Maxime Ripard, Hsin-Yi Wang, Matthias Brugger, Thomas Gleixner,
Collabora Kernel ML, Icenowy Zheng
In-Reply-To: <CA+E=qVffVzZwRTk9K7=xhWn-AOKExkew0aPcyL_W1nokx-mDdg@mail.gmail.com>
Hi Vasily,
Missatge de Vasily Khoruzhick <anarsoul@gmail.com> del dia dv., 14 de
febr. 2020 a les 22:36:
>
> On Thu, Feb 13, 2020 at 6:54 AM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
> >
> > From: Nicolas Boichat <drinkcat@chromium.org>
> >
> > ANX7688 is a HDMI to DP converter (as well as USB-C port controller),
> > that has an internal microcontroller.
> >
> > The only reason a Linux kernel driver is necessary is to reject
> > resolutions that require more bandwidth than what is available on
> > the DP side. DP bandwidth and lane count are reported by the bridge
> > via 2 registers on I2C.
>
> It is true only for your particular platform where usb-c part is
> managed by firmware. Pinephone has the same anx7688 but linux will
> need a driver that manages usb-c in addition to DP.
>
> I'd suggest making it MFD driver from the beginning, or at least make
> proper bindings so we don't have to rework it and introduce binding
> incompatibilities in future.
>
Do you have example code on how the ANX7866 is used in pinephone?
There is a repo somewhere?
Thanks,
Enric
> > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > ---
> >
> > Changes in v2:
> > - Move driver to drivers/gpu/drm/bridge/analogix.
> > - Make the driver OF only so we can reduce the ifdefs.
> > - Update the Copyright to 2020.
> > - Use probe_new so we can get rid of the i2c_device_id table.
> >
> > drivers/gpu/drm/bridge/analogix/Kconfig | 12 ++
> > drivers/gpu/drm/bridge/analogix/Makefile | 1 +
> > .../drm/bridge/analogix/analogix-anx7688.c | 188 ++++++++++++++++++
> > 3 files changed, 201 insertions(+)
> > create mode 100644 drivers/gpu/drm/bridge/analogix/analogix-anx7688.c
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
> > index e1fa7d820373..af7c2939403c 100644
> > --- a/drivers/gpu/drm/bridge/analogix/Kconfig
> > +++ b/drivers/gpu/drm/bridge/analogix/Kconfig
> > @@ -11,6 +11,18 @@ config DRM_ANALOGIX_ANX6345
> > ANX6345 transforms the LVTTL RGB output of an
> > application processor to eDP or DisplayPort.
> >
> > +config DRM_ANALOGIX_ANX7688
> > + tristate "Analogix ANX7688 bridge"
> > + depends on OF
> > + select DRM_KMS_HELPER
> > + select REGMAP_I2C
> > + help
> > + ANX7688 is an ultra-low power 4k Ultra-HD (4096x2160p60)
> > + mobile HD transmitter designed for portable devices. The
> > + ANX7688 converts HDMI 2.0 to DisplayPort 1.3 Ultra-HD
> > + including an intelligent crosspoint switch to support
> > + USB Type-C.
> > +
> > config DRM_ANALOGIX_ANX78XX
> > tristate "Analogix ANX78XX bridge"
> > select DRM_ANALOGIX_DP
> > diff --git a/drivers/gpu/drm/bridge/analogix/Makefile b/drivers/gpu/drm/bridge/analogix/Makefile
> > index 97669b374098..27cd73635c8c 100644
> > --- a/drivers/gpu/drm/bridge/analogix/Makefile
> > +++ b/drivers/gpu/drm/bridge/analogix/Makefile
> > @@ -1,5 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o analogix-i2c-dptx.o
> > obj-$(CONFIG_DRM_ANALOGIX_ANX6345) += analogix-anx6345.o
> > +obj-$(CONFIG_DRM_ANALOGIX_ANX7688) += analogix-anx7688.o
> > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> > obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c b/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c
> > new file mode 100644
> > index 000000000000..10a7cd0f9126
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c
> > @@ -0,0 +1,188 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * ANX7688 HDMI->DP bridge driver
> > + *
> > + * Copyright 2020 Google LLC
> > + */
> > +
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <drm/drm_bridge.h>
> > +
> > +/* Register addresses */
> > +#define VENDOR_ID_REG 0x00
> > +#define DEVICE_ID_REG 0x02
> > +
> > +#define FW_VERSION_REG 0x80
> > +
> > +#define DP_BANDWIDTH_REG 0x85
> > +#define DP_LANE_COUNT_REG 0x86
> > +
> > +#define VENDOR_ID 0x1f29
> > +#define DEVICE_ID 0x7688
> > +
> > +/* First supported firmware version (0.85) */
> > +#define MINIMUM_FW_VERSION 0x0085
> > +
> > +struct anx7688 {
> > + struct drm_bridge bridge;
> > + struct i2c_client *client;
> > + struct regmap *regmap;
> > +
> > + bool filter;
> > +};
> > +
> > +static inline struct anx7688 *bridge_to_anx7688(struct drm_bridge *bridge)
> > +{
> > + return container_of(bridge, struct anx7688, bridge);
> > +}
> > +
> > +static bool anx7688_bridge_mode_fixup(struct drm_bridge *bridge,
> > + const struct drm_display_mode *mode,
> > + struct drm_display_mode *adjusted_mode)
> > +{
> > + struct anx7688 *anx7688 = bridge_to_anx7688(bridge);
> > + int totalbw, requiredbw;
> > + u8 dpbw, lanecount;
> > + u8 regs[2];
> > + int ret;
> > +
> > + if (!anx7688->filter)
> > + return true;
> > +
> > + /* Read both regs 0x85 (bandwidth) and 0x86 (lane count). */
> > + ret = regmap_bulk_read(anx7688->regmap, DP_BANDWIDTH_REG, regs, 2);
> > + if (ret < 0) {
> > + dev_err(&anx7688->client->dev,
> > + "Failed to read bandwidth/lane count\n");
> > + return false;
> > + }
> > + dpbw = regs[0];
> > + lanecount = regs[1];
> > +
> > + /* Maximum 0x19 bandwidth (6.75 Gbps Turbo mode), 2 lanes */
> > + if (dpbw > 0x19 || lanecount > 2) {
> > + dev_err(&anx7688->client->dev,
> > + "Invalid bandwidth/lane count (%02x/%d)\n",
> > + dpbw, lanecount);
> > + return false;
> > + }
> > +
> > + /* Compute available bandwidth (kHz) */
> > + totalbw = dpbw * lanecount * 270000 * 8 / 10;
> > +
> > + /* Required bandwidth (8 bpc, kHz) */
> > + requiredbw = mode->clock * 8 * 3;
> > +
> > + dev_dbg(&anx7688->client->dev,
> > + "DP bandwidth: %d kHz (%02x/%d); mode requires %d Khz\n",
> > + totalbw, dpbw, lanecount, requiredbw);
> > +
> > + if (totalbw == 0) {
> > + dev_warn(&anx7688->client->dev,
> > + "Bandwidth/lane count are 0, not rejecting modes\n");
> > + return true;
> > + }
> > +
> > + return totalbw >= requiredbw;
> > +}
> > +
> > +static const struct drm_bridge_funcs anx7688_bridge_funcs = {
> > + .mode_fixup = anx7688_bridge_mode_fixup,
> > +};
> > +
> > +static const struct regmap_config anx7688_regmap_config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > +};
> > +
> > +static int anx7688_i2c_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct anx7688 *anx7688;
> > + u16 vendor, device;
> > + u16 fwversion;
> > + u8 buffer[4];
> > + int ret;
> > +
> > + anx7688 = devm_kzalloc(dev, sizeof(*anx7688), GFP_KERNEL);
> > + if (!anx7688)
> > + return -ENOMEM;
> > +
> > + anx7688->bridge.of_node = dev->of_node;
> > + anx7688->client = client;
> > + i2c_set_clientdata(client, anx7688);
> > +
> > + anx7688->regmap = devm_regmap_init_i2c(client, &anx7688_regmap_config);
> > +
> > + /* Read both vendor and device id (4 bytes). */
> > + ret = regmap_bulk_read(anx7688->regmap, VENDOR_ID_REG, buffer, 4);
> > + if (ret) {
> > + dev_err(dev, "Failed to read chip vendor/device id\n");
> > + return ret;
> > + }
> > +
> > + vendor = (u16)buffer[1] << 8 | buffer[0];
> > + device = (u16)buffer[3] << 8 | buffer[2];
> > + if (vendor != VENDOR_ID || device != DEVICE_ID) {
> > + dev_err(dev, "Invalid vendor/device id %04x/%04x\n",
> > + vendor, device);
> > + return -ENODEV;
> > + }
> > +
> > + ret = regmap_bulk_read(anx7688->regmap, FW_VERSION_REG, buffer, 2);
> > + if (ret) {
> > + dev_err(&client->dev, "Failed to read firmware version\n");
> > + return ret;
> > + }
> > +
> > + fwversion = (u16)buffer[0] << 8 | buffer[1];
> > + dev_info(dev, "ANX7688 firwmare version %02x.%02x\n",
> > + buffer[0], buffer[1]);
> > +
> > + /* FW version >= 0.85 supports bandwidth/lane count registers */
> > + if (fwversion >= MINIMUM_FW_VERSION) {
> > + anx7688->filter = true;
> > + } else {
> > + /* Warn, but not fail, for backwards compatibility. */
> > + dev_warn(dev,
> > + "Old ANX7688 FW version (%02x.%02x), not filtering\n",
> > + buffer[0], buffer[1]);
> > + }
> > +
> > + anx7688->bridge.funcs = &anx7688_bridge_funcs;
> > + drm_bridge_add(&anx7688->bridge);
> > +
> > + return 0;
> > +}
> > +
> > +static int anx7688_i2c_remove(struct i2c_client *client)
> > +{
> > + struct anx7688 *anx7688 = i2c_get_clientdata(client);
> > +
> > + drm_bridge_remove(&anx7688->bridge);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id anx7688_match_table[] = {
> > + { .compatible = "analogix,anx7688", },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, anx7688_match_table);
> > +
> > +static struct i2c_driver anx7688_driver = {
> > + .probe_new = anx7688_i2c_probe,
> > + .remove = anx7688_i2c_remove,
> > + .driver = {
> > + .name = "anx7688",
> > + .of_match_table = anx7688_match_table,
> > + },
> > +};
> > +
> > +module_i2c_driver(anx7688_driver);
> > +
> > +MODULE_DESCRIPTION("ANX7688 HDMI->DP bridge driver");
> > +MODULE_AUTHOR("Nicolas Boichat <drinkcat@chromium.org>");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.0
> >
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 2/2] drm/bridge: anx7688: Add anx7688 bridge driver support
From: Enric Balletbo Serra @ 2020-02-14 21:53 UTC (permalink / raw)
To: Vasily Khoruzhick
Cc: Jernej Skrabec, Nicolas Boichat, Neil Armstrong, David Airlie,
Torsten Duwe, Jonas Karlman, Icenowy Zheng, linux-kernel,
dri-devel, Andrzej Hajda, Matthias Brugger, Laurent Pinchart,
Hsin-Yi Wang, Enric Balletbo i Serra, Thomas Gleixner,
Collabora Kernel ML, Maxime Ripard
In-Reply-To: <CA+E=qVffVzZwRTk9K7=xhWn-AOKExkew0aPcyL_W1nokx-mDdg@mail.gmail.com>
Hi Vasily,
Missatge de Vasily Khoruzhick <anarsoul@gmail.com> del dia dv., 14 de
febr. 2020 a les 22:36:
>
> On Thu, Feb 13, 2020 at 6:54 AM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
> >
> > From: Nicolas Boichat <drinkcat@chromium.org>
> >
> > ANX7688 is a HDMI to DP converter (as well as USB-C port controller),
> > that has an internal microcontroller.
> >
> > The only reason a Linux kernel driver is necessary is to reject
> > resolutions that require more bandwidth than what is available on
> > the DP side. DP bandwidth and lane count are reported by the bridge
> > via 2 registers on I2C.
>
> It is true only for your particular platform where usb-c part is
> managed by firmware. Pinephone has the same anx7688 but linux will
> need a driver that manages usb-c in addition to DP.
>
> I'd suggest making it MFD driver from the beginning, or at least make
> proper bindings so we don't have to rework it and introduce binding
> incompatibilities in future.
>
Do you have example code on how the ANX7866 is used in pinephone?
There is a repo somewhere?
Thanks,
Enric
> > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > ---
> >
> > Changes in v2:
> > - Move driver to drivers/gpu/drm/bridge/analogix.
> > - Make the driver OF only so we can reduce the ifdefs.
> > - Update the Copyright to 2020.
> > - Use probe_new so we can get rid of the i2c_device_id table.
> >
> > drivers/gpu/drm/bridge/analogix/Kconfig | 12 ++
> > drivers/gpu/drm/bridge/analogix/Makefile | 1 +
> > .../drm/bridge/analogix/analogix-anx7688.c | 188 ++++++++++++++++++
> > 3 files changed, 201 insertions(+)
> > create mode 100644 drivers/gpu/drm/bridge/analogix/analogix-anx7688.c
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
> > index e1fa7d820373..af7c2939403c 100644
> > --- a/drivers/gpu/drm/bridge/analogix/Kconfig
> > +++ b/drivers/gpu/drm/bridge/analogix/Kconfig
> > @@ -11,6 +11,18 @@ config DRM_ANALOGIX_ANX6345
> > ANX6345 transforms the LVTTL RGB output of an
> > application processor to eDP or DisplayPort.
> >
> > +config DRM_ANALOGIX_ANX7688
> > + tristate "Analogix ANX7688 bridge"
> > + depends on OF
> > + select DRM_KMS_HELPER
> > + select REGMAP_I2C
> > + help
> > + ANX7688 is an ultra-low power 4k Ultra-HD (4096x2160p60)
> > + mobile HD transmitter designed for portable devices. The
> > + ANX7688 converts HDMI 2.0 to DisplayPort 1.3 Ultra-HD
> > + including an intelligent crosspoint switch to support
> > + USB Type-C.
> > +
> > config DRM_ANALOGIX_ANX78XX
> > tristate "Analogix ANX78XX bridge"
> > select DRM_ANALOGIX_DP
> > diff --git a/drivers/gpu/drm/bridge/analogix/Makefile b/drivers/gpu/drm/bridge/analogix/Makefile
> > index 97669b374098..27cd73635c8c 100644
> > --- a/drivers/gpu/drm/bridge/analogix/Makefile
> > +++ b/drivers/gpu/drm/bridge/analogix/Makefile
> > @@ -1,5 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o analogix-i2c-dptx.o
> > obj-$(CONFIG_DRM_ANALOGIX_ANX6345) += analogix-anx6345.o
> > +obj-$(CONFIG_DRM_ANALOGIX_ANX7688) += analogix-anx7688.o
> > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> > obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c b/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c
> > new file mode 100644
> > index 000000000000..10a7cd0f9126
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx7688.c
> > @@ -0,0 +1,188 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * ANX7688 HDMI->DP bridge driver
> > + *
> > + * Copyright 2020 Google LLC
> > + */
> > +
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <drm/drm_bridge.h>
> > +
> > +/* Register addresses */
> > +#define VENDOR_ID_REG 0x00
> > +#define DEVICE_ID_REG 0x02
> > +
> > +#define FW_VERSION_REG 0x80
> > +
> > +#define DP_BANDWIDTH_REG 0x85
> > +#define DP_LANE_COUNT_REG 0x86
> > +
> > +#define VENDOR_ID 0x1f29
> > +#define DEVICE_ID 0x7688
> > +
> > +/* First supported firmware version (0.85) */
> > +#define MINIMUM_FW_VERSION 0x0085
> > +
> > +struct anx7688 {
> > + struct drm_bridge bridge;
> > + struct i2c_client *client;
> > + struct regmap *regmap;
> > +
> > + bool filter;
> > +};
> > +
> > +static inline struct anx7688 *bridge_to_anx7688(struct drm_bridge *bridge)
> > +{
> > + return container_of(bridge, struct anx7688, bridge);
> > +}
> > +
> > +static bool anx7688_bridge_mode_fixup(struct drm_bridge *bridge,
> > + const struct drm_display_mode *mode,
> > + struct drm_display_mode *adjusted_mode)
> > +{
> > + struct anx7688 *anx7688 = bridge_to_anx7688(bridge);
> > + int totalbw, requiredbw;
> > + u8 dpbw, lanecount;
> > + u8 regs[2];
> > + int ret;
> > +
> > + if (!anx7688->filter)
> > + return true;
> > +
> > + /* Read both regs 0x85 (bandwidth) and 0x86 (lane count). */
> > + ret = regmap_bulk_read(anx7688->regmap, DP_BANDWIDTH_REG, regs, 2);
> > + if (ret < 0) {
> > + dev_err(&anx7688->client->dev,
> > + "Failed to read bandwidth/lane count\n");
> > + return false;
> > + }
> > + dpbw = regs[0];
> > + lanecount = regs[1];
> > +
> > + /* Maximum 0x19 bandwidth (6.75 Gbps Turbo mode), 2 lanes */
> > + if (dpbw > 0x19 || lanecount > 2) {
> > + dev_err(&anx7688->client->dev,
> > + "Invalid bandwidth/lane count (%02x/%d)\n",
> > + dpbw, lanecount);
> > + return false;
> > + }
> > +
> > + /* Compute available bandwidth (kHz) */
> > + totalbw = dpbw * lanecount * 270000 * 8 / 10;
> > +
> > + /* Required bandwidth (8 bpc, kHz) */
> > + requiredbw = mode->clock * 8 * 3;
> > +
> > + dev_dbg(&anx7688->client->dev,
> > + "DP bandwidth: %d kHz (%02x/%d); mode requires %d Khz\n",
> > + totalbw, dpbw, lanecount, requiredbw);
> > +
> > + if (totalbw == 0) {
> > + dev_warn(&anx7688->client->dev,
> > + "Bandwidth/lane count are 0, not rejecting modes\n");
> > + return true;
> > + }
> > +
> > + return totalbw >= requiredbw;
> > +}
> > +
> > +static const struct drm_bridge_funcs anx7688_bridge_funcs = {
> > + .mode_fixup = anx7688_bridge_mode_fixup,
> > +};
> > +
> > +static const struct regmap_config anx7688_regmap_config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > +};
> > +
> > +static int anx7688_i2c_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct anx7688 *anx7688;
> > + u16 vendor, device;
> > + u16 fwversion;
> > + u8 buffer[4];
> > + int ret;
> > +
> > + anx7688 = devm_kzalloc(dev, sizeof(*anx7688), GFP_KERNEL);
> > + if (!anx7688)
> > + return -ENOMEM;
> > +
> > + anx7688->bridge.of_node = dev->of_node;
> > + anx7688->client = client;
> > + i2c_set_clientdata(client, anx7688);
> > +
> > + anx7688->regmap = devm_regmap_init_i2c(client, &anx7688_regmap_config);
> > +
> > + /* Read both vendor and device id (4 bytes). */
> > + ret = regmap_bulk_read(anx7688->regmap, VENDOR_ID_REG, buffer, 4);
> > + if (ret) {
> > + dev_err(dev, "Failed to read chip vendor/device id\n");
> > + return ret;
> > + }
> > +
> > + vendor = (u16)buffer[1] << 8 | buffer[0];
> > + device = (u16)buffer[3] << 8 | buffer[2];
> > + if (vendor != VENDOR_ID || device != DEVICE_ID) {
> > + dev_err(dev, "Invalid vendor/device id %04x/%04x\n",
> > + vendor, device);
> > + return -ENODEV;
> > + }
> > +
> > + ret = regmap_bulk_read(anx7688->regmap, FW_VERSION_REG, buffer, 2);
> > + if (ret) {
> > + dev_err(&client->dev, "Failed to read firmware version\n");
> > + return ret;
> > + }
> > +
> > + fwversion = (u16)buffer[0] << 8 | buffer[1];
> > + dev_info(dev, "ANX7688 firwmare version %02x.%02x\n",
> > + buffer[0], buffer[1]);
> > +
> > + /* FW version >= 0.85 supports bandwidth/lane count registers */
> > + if (fwversion >= MINIMUM_FW_VERSION) {
> > + anx7688->filter = true;
> > + } else {
> > + /* Warn, but not fail, for backwards compatibility. */
> > + dev_warn(dev,
> > + "Old ANX7688 FW version (%02x.%02x), not filtering\n",
> > + buffer[0], buffer[1]);
> > + }
> > +
> > + anx7688->bridge.funcs = &anx7688_bridge_funcs;
> > + drm_bridge_add(&anx7688->bridge);
> > +
> > + return 0;
> > +}
> > +
> > +static int anx7688_i2c_remove(struct i2c_client *client)
> > +{
> > + struct anx7688 *anx7688 = i2c_get_clientdata(client);
> > +
> > + drm_bridge_remove(&anx7688->bridge);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id anx7688_match_table[] = {
> > + { .compatible = "analogix,anx7688", },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, anx7688_match_table);
> > +
> > +static struct i2c_driver anx7688_driver = {
> > + .probe_new = anx7688_i2c_probe,
> > + .remove = anx7688_i2c_remove,
> > + .driver = {
> > + .name = "anx7688",
> > + .of_match_table = anx7688_match_table,
> > + },
> > +};
> > +
> > +module_i2c_driver(anx7688_driver);
> > +
> > +MODULE_DESCRIPTION("ANX7688 HDMI->DP bridge driver");
> > +MODULE_AUTHOR("Nicolas Boichat <drinkcat@chromium.org>");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.0
> >
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [dpdk-dev] [RFC] meter: fix ABI break due to experimental tag removal
From: Bruce Richardson @ 2020-02-14 21:52 UTC (permalink / raw)
To: Neil Horman
Cc: Ray Kinsella, Luca Boccassi, Ferruh Yigit, Cristian Dumitrescu,
Eelco Chaudron, dev, Thomas Monjalon, David Marchand, Ian Stokes
In-Reply-To: <20200214204848.GA27176@hmswarspite.think-freely.org>
On Fri, Feb 14, 2020 at 03:48:48PM -0500, Neil Horman wrote:
> On Fri, Feb 14, 2020 at 11:36:34AM +0000, Bruce Richardson wrote:
> > On Thu, Feb 13, 2020 at 09:40:40PM -0500, Neil Horman wrote:
> > > On Thu, Feb 13, 2020 at 05:40:43PM +0000, Ray Kinsella wrote:
> > > >
> > > >
> > > > On 05/02/2020 11:32, Neil Horman wrote:
> > > > > On Wed, Feb 05, 2020 at 11:04:29AM +0100, Luca Boccassi wrote:
> > > > >> On Tue, 2020-02-04 at 07:02 -0500, Neil Horman wrote:
> > > > >>>> But if we can do the versioning in the master, LTS can backport it
> > > > >>>> and can have mature version of that API in LTS without breaking
> > > > >>>> the existing users.
> > > > >>>>
> > > > >>>
> > > > >>> But why bother? The only thing you've changed is the version
> > > > >>> tagging. Its ok to leave that alone in LTS, you just cant change
> > > > >>> it.
> > > > >>>
> > > > >>> Thats part of the burden of an LTS release, it will have some drift
> > > > >>> from the upstream head, because you have to keep things stable. So
> > > > >>> you stabilize the upstream ABI version for this API and just never
> > > > >>> backport it to the current LTS release.
> > > > >>
> > > > >> Hi,
> > > > >>
> > > > >> A customer (OVS) explicitly and specifically requested backporting
> > > > >> the symbol change to 19.11, as they don't want to enable
> > > > >> experimental APIs in their releases. I replied that it would only be
> > > > >> acceptable with aliasing to keep compatibility, and Ferruh very
> > > > >> kindly did the work to implement that.
> > > > >>
> > > > > but, thats part of the agreement, no? You can't always have new
> > > > > features and stability at the same time.
> > > > >
> > > > > I get that this is an odd corner case, because strictly speaking you
> > > > > could waive the ABI change that libabigail is reporting, and most
> > > > > application users (like OVS) could get away with it, because their
> > > > > application does all the right things to make it ok, but I don't
> > > > > think you can make a decsion like this for all users based on the
> > > > > request of a single user.
> > > > >
> > > > > It seems like the right thing is for OVS to augment their build time
> > > > > configuration to allow developers to select the ability to use
> > > > > experimental apis at compile time, and then the decision is placed in
> > > > > the hands of end users.
> > > >
> > > > So this is not isolated case right ... it is common from API's to
> > > > graduate from experimental to mature, we _want_ that to happen, we
> > > > _want_ APIs to mature.
> > > >
> > > Sure, I can absolutely agree with that, though I would suggest that the
> > > maturity of the ABI is orthogonal to its labeling as such (more on that
> > > below)
> > >
> > > > I am worried what you are proposing will encourage a behavior whereby
> > > > maintainers to wait until the declaration of the next major ABI version
> > > > to make these symbol changes, causing these kinds of changes to queue
> > > > up for that release.
> > > >
> > > I think you're probably right about that, and would make the agrument
> > > that thats perfectly fine (again I'll clarify below)
> > >
> > > > And you would have to ask why? In the case of a reasonably mature API,
> > > > there will be no functional or signature change - its mature! So what
> > > > is the harm of providing an alias back to Experimental until the next
> > > > ABI version is declared?
> > > >
> > > From a philosophical standpoint, there is absoluely no harm, and I don't
> > > think anyone would disagree, the harm comes from the details of the
> > > implementation, as you've noted.
> > >
> > > > So while yes, you are 100% right - experimental mean no guarantees.
> > > > But if the API is baked, it is not going to change, so I don't see the
> > > > harm.
> > > >
> > > > We don't want the unnecessary triumph of policy over pragmatism.
> > > >
> > > I would make the converse argument here. While I agree that when an API
> > > is mature, theres no point in calling it experimental anymore, I would
> > > also suggest that, if an API is mature, and not expected to change,
> > > theres no harm in leaving its version tag as experimental for an LTS
> > > release. This is what I was referring to above. Once an application
> > > developer has done the work to integrate an API from DPDK into its
> > > application, that work is done. If the API remains stable, then I
> > > honestly don't know that they care that the version label is EXPERIMENTAL
> > > versus 20.11 or 20.05 or whatever. They care about the API being stable
> > > more so than its version label. As such, it seems....reasonable to me to
> > > have developers queue their migration of experimental APIs to official
> > > versioned APIs at the next major release deliniation point.
> > >
> > > I'd welcome counter arguments, but it seems pretty natural to me to make
> > > these sorts of changes at the major release mark. People using
> > > experimantal apis have already implicity agreed to manage changes to
> > > them, so if we just hold it stable in an LTS release (and don't update
> > > the version label), its just gravy for them.
> > >
> > The counter argument that I see is that while the experimental tag remains
> > in place the user has no way to know that an API is stable or not, and so
> > in many projects cannot use the API. If for example an API that is
> > experimental in 19.11 is unchanged through 20.05 at which point we decide
> > to promote it to stable. Once the change to the exp tag it is made, any
> > users of 19.11 can then see that it is an unchanged and now-stable ABI and
> > can start using it in their software, if they wish, without having to wait
> > for the 20.11 release. Changing the tag early allows ABIs to be potentially
> > used immediately.
> >
>
> I would agree with this, however, when using an LTS release, in my mind at
> least, part of the agreement there is you get stability in the fuctions that
> were declared stable at the time of release. I'm not sure there should be an
> expectation of additional stabilization within the lifetime of the release
> (thats really what the 12 month LTS release cycle is for, no)? You never have
> to wait more than a year for a new set of stable features. If you want faster
> feature integration than that, you can choose to enable the experimental API's
> and accept the benefits and drawbacks thereof.
>
> That said, if (as I understand it) the goal is to provide a mechanism to allow
> experimental features to be promoted to stable status, perhaps we can find a
> happy middle ground. What if we were to create a construct such as this:
>
> #pragma push_macro("ALLOW_EXPERIMENTAL_APIS")
> #undef ALLOW_EXPERIMENTAL_APIS
> void __rte_experimental func(...);
> #pragma pop_macro("ALLOW_EXPERIMENTAL_APIS")
>
> Such a consruct would allow the maintainer of an API to pseudo-promote an API
> from a experimental to stable status, at least so far as compilation would be
> concerned. Its messy and clunky, and it wouldn't change the function version at
> all, but the end result would be that:
> a) such a wraped experimental function would no longer issue a warning when used
> during compilation/linking
> and
> b) provide a semi-easy grepable pattern for application writers to look for when
> considering the use of an API that was previously experimental
>
> such a construct would have to be used very judiciously, in that once its
> implemented, the API has to be treated as stable, even though its 'excused' from
> the normal checking, but it could provide something of the more rapid promotion
> path being sought.
>
Sure it could work and does meet the end goal. I'm not really sure it's a
better solution than just promoting the function and putting in an alias
for it back to experimental, though.
/Bruce
^ permalink raw reply
* Re: [PATCH] memcg: net: do not associate sock with unrelated memcg
From: Shakeel Butt @ 2020-02-14 21:52 UTC (permalink / raw)
To: Roman Gushchin
Cc: Johannes Weiner, Eric Dumazet, Greg Thelen, Michal Hocko,
Vladimir Davydov, Andrew Morton, Cgroups, Linux MM, LKML
In-Reply-To: <20200214214730.GA99109@carbon.DHCP.thefacebook.com>
On Fri, Feb 14, 2020 at 1:47 PM Roman Gushchin <guro@fb.com> wrote:
>
> Hello, Shakeel!
>
> On Thu, Feb 13, 2020 at 11:12:33PM -0800, Shakeel Butt wrote:
> > We are testing network memory accounting in our setup and noticed
> > inconsistent network memory usage and often unrelated memcgs network
> > usage correlates with testing workload. On further inspection, it seems
> > like mem_cgroup_sk_alloc() is broken in irq context specially for
> > cgroup v1.
>
> A great catch!
>
> >
> > mem_cgroup_sk_alloc() can be called in irq context and kind
> > of assumes that it can only happen from sk_clone_lock() and the source
> > sock object has already associated memcg. However in cgroup v1, where
> > network memory accounting is opt-in, the source sock can be not
> > associated with any memcg and the new cloned sock can get associated
> > with unrelated interrupted memcg.
> >
> > Cgroup v2 can also suffer if the source sock object was created by
> > process in the root memcg or if sk_alloc() is called in irq context.
>
> Do you mind sharing a call trace?
>
Sure, see below. I added a dump_stack() in mem_cgroup_sk_alloc().
[ 647.255327] CPU: 68 PID: 15859 Comm: ssh Tainted: G O
5.6.0-smp-DEV #1
[ 647.255328] Hardware name: ...
[ 647.255328] Call Trace:
[ 647.255329] <IRQ>
[ 647.255333] dump_stack+0x57/0x75
[ 647.255336] mem_cgroup_sk_alloc+0xe9/0xf0
[ 647.255337] sk_clone_lock+0x2a7/0x420
[ 647.255339] inet_csk_clone_lock+0x1b/0x110
[ 647.255340] tcp_create_openreq_child+0x23/0x3b0
[ 647.255342] tcp_v6_syn_recv_sock+0x88/0x730
[ 647.255343] tcp_check_req+0x429/0x560
[ 647.255345] tcp_v6_rcv+0x72d/0xa40
[ 647.255347] ip6_protocol_deliver_rcu+0xc9/0x400
[ 647.255348] ip6_input+0x44/0xd0
[ 647.255349] ? ip6_protocol_deliver_rcu+0x400/0x400
[ 647.255350] ip6_rcv_finish+0x71/0x80
[ 647.255351] ipv6_rcv+0x5b/0xe0
[ 647.255352] ? ip6_sublist_rcv+0x2e0/0x2e0
[ 647.255354] process_backlog+0x108/0x1e0
[ 647.255355] net_rx_action+0x26b/0x460
[ 647.255357] __do_softirq+0x104/0x2a6
[ 647.255358] do_softirq_own_stack+0x2a/0x40
[ 647.255359] </IRQ>
[ 647.255361] do_softirq.part.19+0x40/0x50
[ 647.255362] __local_bh_enable_ip+0x51/0x60
[ 647.255363] ip6_finish_output2+0x23d/0x520
[ 647.255365] ? ip6table_mangle_hook+0x55/0x160
[ 647.255366] __ip6_finish_output+0xa1/0x100
[ 647.255367] ip6_finish_output+0x30/0xd0
[ 647.255368] ip6_output+0x73/0x120
[ 647.255369] ? __ip6_finish_output+0x100/0x100
[ 647.255370] ip6_xmit+0x2e3/0x600
[ 647.255372] ? ipv6_anycast_cleanup+0x50/0x50
[ 647.255373] ? inet6_csk_route_socket+0x136/0x1e0
[ 647.255374] ? skb_free_head+0x1e/0x30
[ 647.255375] inet6_csk_xmit+0x95/0xf0
[ 647.255377] __tcp_transmit_skb+0x5b4/0xb20
[ 647.255378] __tcp_send_ack.part.60+0xa3/0x110
[ 647.255379] tcp_send_ack+0x1d/0x20
[ 647.255380] tcp_rcv_state_process+0xe64/0xe80
[ 647.255381] ? tcp_v6_connect+0x5d1/0x5f0
[ 647.255383] tcp_v6_do_rcv+0x1b1/0x3f0
[ 647.255384] ? tcp_v6_do_rcv+0x1b1/0x3f0
[ 647.255385] __release_sock+0x7f/0xd0
[ 647.255386] release_sock+0x30/0xa0
[ 647.255388] __inet_stream_connect+0x1c3/0x3b0
[ 647.255390] ? prepare_to_wait+0xb0/0xb0
[ 647.255391] inet_stream_connect+0x3b/0x60
[ 647.255394] __sys_connect+0x101/0x120
[ 647.255395] ? __sys_getsockopt+0x11b/0x140
[ 647.255397] __x64_sys_connect+0x1a/0x20
[ 647.255398] do_syscall_64+0x51/0x200
[ 647.255399] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 647.255401] RIP: 0033:0x7f45464fcd50
> Also, shouldn't cgroup_sk_alloc() be changed in a similar way?
>
I will check cgroup_sk_alloc() too.
Thanks,
Shakeel
^ permalink raw reply
* Re: [PATCH] memcg: net: do not associate sock with unrelated memcg
From: Shakeel Butt @ 2020-02-14 21:52 UTC (permalink / raw)
To: Roman Gushchin
Cc: Johannes Weiner, Eric Dumazet, Greg Thelen, Michal Hocko,
Vladimir Davydov, Andrew Morton, Cgroups, Linux MM, LKML
In-Reply-To: <20200214214730.GA99109-lLJQVQxiE4uLfgCeKHXN1g2O0Ztt9esIQQ4Iyu8u01E@public.gmane.org>
On Fri, Feb 14, 2020 at 1:47 PM Roman Gushchin <guro-b10kYP2dOMg@public.gmane.org> wrote:
>
> Hello, Shakeel!
>
> On Thu, Feb 13, 2020 at 11:12:33PM -0800, Shakeel Butt wrote:
> > We are testing network memory accounting in our setup and noticed
> > inconsistent network memory usage and often unrelated memcgs network
> > usage correlates with testing workload. On further inspection, it seems
> > like mem_cgroup_sk_alloc() is broken in irq context specially for
> > cgroup v1.
>
> A great catch!
>
> >
> > mem_cgroup_sk_alloc() can be called in irq context and kind
> > of assumes that it can only happen from sk_clone_lock() and the source
> > sock object has already associated memcg. However in cgroup v1, where
> > network memory accounting is opt-in, the source sock can be not
> > associated with any memcg and the new cloned sock can get associated
> > with unrelated interrupted memcg.
> >
> > Cgroup v2 can also suffer if the source sock object was created by
> > process in the root memcg or if sk_alloc() is called in irq context.
>
> Do you mind sharing a call trace?
>
Sure, see below. I added a dump_stack() in mem_cgroup_sk_alloc().
[ 647.255327] CPU: 68 PID: 15859 Comm: ssh Tainted: G O
5.6.0-smp-DEV #1
[ 647.255328] Hardware name: ...
[ 647.255328] Call Trace:
[ 647.255329] <IRQ>
[ 647.255333] dump_stack+0x57/0x75
[ 647.255336] mem_cgroup_sk_alloc+0xe9/0xf0
[ 647.255337] sk_clone_lock+0x2a7/0x420
[ 647.255339] inet_csk_clone_lock+0x1b/0x110
[ 647.255340] tcp_create_openreq_child+0x23/0x3b0
[ 647.255342] tcp_v6_syn_recv_sock+0x88/0x730
[ 647.255343] tcp_check_req+0x429/0x560
[ 647.255345] tcp_v6_rcv+0x72d/0xa40
[ 647.255347] ip6_protocol_deliver_rcu+0xc9/0x400
[ 647.255348] ip6_input+0x44/0xd0
[ 647.255349] ? ip6_protocol_deliver_rcu+0x400/0x400
[ 647.255350] ip6_rcv_finish+0x71/0x80
[ 647.255351] ipv6_rcv+0x5b/0xe0
[ 647.255352] ? ip6_sublist_rcv+0x2e0/0x2e0
[ 647.255354] process_backlog+0x108/0x1e0
[ 647.255355] net_rx_action+0x26b/0x460
[ 647.255357] __do_softirq+0x104/0x2a6
[ 647.255358] do_softirq_own_stack+0x2a/0x40
[ 647.255359] </IRQ>
[ 647.255361] do_softirq.part.19+0x40/0x50
[ 647.255362] __local_bh_enable_ip+0x51/0x60
[ 647.255363] ip6_finish_output2+0x23d/0x520
[ 647.255365] ? ip6table_mangle_hook+0x55/0x160
[ 647.255366] __ip6_finish_output+0xa1/0x100
[ 647.255367] ip6_finish_output+0x30/0xd0
[ 647.255368] ip6_output+0x73/0x120
[ 647.255369] ? __ip6_finish_output+0x100/0x100
[ 647.255370] ip6_xmit+0x2e3/0x600
[ 647.255372] ? ipv6_anycast_cleanup+0x50/0x50
[ 647.255373] ? inet6_csk_route_socket+0x136/0x1e0
[ 647.255374] ? skb_free_head+0x1e/0x30
[ 647.255375] inet6_csk_xmit+0x95/0xf0
[ 647.255377] __tcp_transmit_skb+0x5b4/0xb20
[ 647.255378] __tcp_send_ack.part.60+0xa3/0x110
[ 647.255379] tcp_send_ack+0x1d/0x20
[ 647.255380] tcp_rcv_state_process+0xe64/0xe80
[ 647.255381] ? tcp_v6_connect+0x5d1/0x5f0
[ 647.255383] tcp_v6_do_rcv+0x1b1/0x3f0
[ 647.255384] ? tcp_v6_do_rcv+0x1b1/0x3f0
[ 647.255385] __release_sock+0x7f/0xd0
[ 647.255386] release_sock+0x30/0xa0
[ 647.255388] __inet_stream_connect+0x1c3/0x3b0
[ 647.255390] ? prepare_to_wait+0xb0/0xb0
[ 647.255391] inet_stream_connect+0x3b/0x60
[ 647.255394] __sys_connect+0x101/0x120
[ 647.255395] ? __sys_getsockopt+0x11b/0x140
[ 647.255397] __x64_sys_connect+0x1a/0x20
[ 647.255398] do_syscall_64+0x51/0x200
[ 647.255399] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 647.255401] RIP: 0033:0x7f45464fcd50
> Also, shouldn't cgroup_sk_alloc() be changed in a similar way?
>
I will check cgroup_sk_alloc() too.
Thanks,
Shakeel
^ permalink raw reply
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/selftests: Fix selftest_mocs for DGFX (rev2)
From: Patchwork @ 2020-02-14 21:52 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
In-Reply-To: <20200213001418.5899-1-brian.welty@intel.com>
== Series Details ==
Series: drm/i915/selftests: Fix selftest_mocs for DGFX (rev2)
URL : https://patchwork.freedesktop.org/series/73387/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_7942 -> Patchwork_16577
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16577/index.html
Known issues
------------
Here are the changes found in Patchwork_16577 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_close_race@basic-threads:
- fi-hsw-peppy: [PASS][1] -> [TIMEOUT][2] ([fdo#112271] / [i915#1084])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7942/fi-hsw-peppy/igt@gem_close_race@basic-threads.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16577/fi-hsw-peppy/igt@gem_close_race@basic-threads.html
- fi-byt-n2820: [PASS][3] -> [INCOMPLETE][4] ([i915#45])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7942/fi-byt-n2820/igt@gem_close_race@basic-threads.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16577/fi-byt-n2820/igt@gem_close_race@basic-threads.html
#### Possible fixes ####
* igt@i915_selftest@live_gem_contexts:
- fi-cfl-8700k: [INCOMPLETE][5] ([i915#424]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7942/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16577/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
[i915#1084]: https://gitlab.freedesktop.org/drm/intel/issues/1084
[i915#424]: https://gitlab.freedesktop.org/drm/intel/issues/424
[i915#45]: https://gitlab.freedesktop.org/drm/intel/issues/45
[i915#937]: https://gitlab.freedesktop.org/drm/intel/issues/937
Participating hosts (47 -> 45)
------------------------------
Additional (4): fi-bsw-kefka fi-blb-e6850 fi-cfl-8109u fi-ilk-650
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7942 -> Patchwork_16577
CI-20190529: 20190529
CI_DRM_7942: f4805f5a516d0a107438ff0f236c9f4187434819 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5442: 3f6080996885b997685f08ecb8b416b2dc485290 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_16577: 9f9e214b4a7dc0d5e5e49a9f0b1224a21120960d @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
9f9e214b4a7d drm/i915/selftests: Fix selftest_mocs for DGFX
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16577/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH] pinctrl: ingenic: Make unreachable path more robust
From: Randy Dunlap @ 2020-02-14 21:52 UTC (permalink / raw)
To: Josh Poimboeuf, Paul Cercueil
Cc: Linus Walleij, linux-gpio, linux-kernel, Peter Zijlstra
In-Reply-To: <73f0c9915473d9e4b3681fb5cc55144291a43192.1581698101.git.jpoimboe@redhat.com>
On 2/14/20 8:37 AM, Josh Poimboeuf wrote:
> In the second loop of ingenic_pinconf_set(), it annotates the switch
> default case as unreachable(). The annotation is technically correct,
> because that same case would have resulted in an early return in the
> previous loop.
>
> However, if a bug were to get introduced later, for example if an
> additional case were added to the first loop without adjusting the
> second loop, it would result in nasty undefined behavior: most likely
> the function's generated code would fall through to the next function.
>
> Another issue is that, while objtool normally understands unreachable()
> annotations, there's one special case where it doesn't: when the
> annotation occurs immediately after a 'ret' instruction. That happens
> to be the case here because unreachable() is immediately before the
> return.
>
> So change the unreachable() to BUG() so that the unreachable code, if
> ever executed, would panic instead of introducing undefined behavior.
> This also makes objtool happy.
>
> This fixes the following objtool warning:
>
> drivers/pinctrl/pinctrl-ingenic.o: warning: objtool: ingenic_pinconf_set() falls through to next function ingenic_pinconf_group_set()
>
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
Thanks.
> ---
> drivers/pinctrl/pinctrl-ingenic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c
> index 96f04d121ebd..6b61ac6cd4d2 100644
> --- a/drivers/pinctrl/pinctrl-ingenic.c
> +++ b/drivers/pinctrl/pinctrl-ingenic.c
> @@ -2158,7 +2158,7 @@ static int ingenic_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> break;
>
> default:
> - unreachable();
> + BUG();
> }
> }
>
>
--
~Randy
^ permalink raw reply
* Re: [PATCH v2] net: phy: restore mdio regs in the iproc mdio driver
From: Andrew Lunn @ 2020-02-14 21:50 UTC (permalink / raw)
To: Scott Branden
Cc: Florian Fainelli, Ray Jui, Arun Parameswaran, Russell King,
linux-kernel, bcm-kernel-feedback-list, netdev, David S . Miller,
linux-arm-kernel, Heiner Kallweit
In-Reply-To: <20200214214746.10153-1-scott.branden@broadcom.com>
On Fri, Feb 14, 2020 at 01:47:46PM -0800, Scott Branden wrote:
> From: Arun Parameswaran <arun.parameswaran@broadcom.com>
>
> The mii management register in iproc mdio block
> does not have a retention register so it is lost on suspend.
> Save and restore value of register while resuming from suspend.
You actually just reconfigure, you don't save it. But that is minor.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] net: phy: restore mdio regs in the iproc mdio driver
From: Andrew Lunn @ 2020-02-14 21:50 UTC (permalink / raw)
To: Scott Branden
Cc: Florian Fainelli, Heiner Kallweit, Russell King, David S . Miller,
Ray Jui, bcm-kernel-feedback-list, netdev, linux-arm-kernel,
linux-kernel, Arun Parameswaran
In-Reply-To: <20200214214746.10153-1-scott.branden@broadcom.com>
On Fri, Feb 14, 2020 at 01:47:46PM -0800, Scott Branden wrote:
> From: Arun Parameswaran <arun.parameswaran@broadcom.com>
>
> The mii management register in iproc mdio block
> does not have a retention register so it is lost on suspend.
> Save and restore value of register while resuming from suspend.
You actually just reconfigure, you don't save it. But that is minor.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Linux kernel 4.19.104 released
From: Linux Kernel Distribution System @ 2020-02-14 21:50 UTC (permalink / raw)
To: linux-kernel-announce
Linux kernel version 4.19.104 is now available:
Full source: https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.19.104.tar.xz
Patch: https://cdn.kernel.org/pub/linux/kernel/v4.x/patch-4.19.104.xz
PGP Signature: https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.19.104.tar.sign
You can view the summary of the changes at the following URL:
https://git.kernel.org/stable/ds/v4.19.104/v4.19.103
^ permalink raw reply
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/4] i915/gem_ctx_engine: Exercise for_each_context_engine() with custom engine[]
From: Antonio Argenziano @ 2020-02-14 21:49 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
In-Reply-To: <20200214194016.4054376-2-chris@chris-wilson.co.uk>
On 14/02/20 11:40, Chris Wilson wrote:
> Set up a custom engine map with no engines, and check that the
> for_each_context_engine() correctly iterates over nothing.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> tests/i915/gem_ctx_engines.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/tests/i915/gem_ctx_engines.c b/tests/i915/gem_ctx_engines.c
> index 063140e0f..6a2edd1e0 100644
> --- a/tests/i915/gem_ctx_engines.c
> +++ b/tests/i915/gem_ctx_engines.c
> @@ -549,6 +549,31 @@ static void independent(int i915)
> gem_context_destroy(i915, param.ctx_id);
> }
>
> +static void libapi(int i915)
> +{
> + struct i915_context_param_engines engines = {};
Is there a case for invalid engines as well?
Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>
> + struct drm_i915_gem_context_param p = {
> + .ctx_id = gem_context_create(i915),
> + .param = I915_CONTEXT_PARAM_ENGINES,
> + .value = to_user_pointer(&engines),
> + .size = sizeof(engines),
> + };
> + const struct intel_execution_engine2 *e;
> + unsigned int count = 0;
> +
> + gem_context_set_param(i915, &p);
> +
> + for_each_context_engine(i915, p.ctx_id, e)
> + count++;
> + igt_assert_eq(count, 0);
> +
> + ____for_each_physical_engine(i915, p.ctx_id, e)
> + count++;
> + igt_assert_eq(count, 0);
> +
> + gem_context_destroy(i915, p.ctx_id);
> +}
> +
> igt_main
> {
> int i915 = -1;
> @@ -584,6 +609,9 @@ igt_main
> igt_subtest("independent")
> independent(i915);
>
> + igt_subtest("libapi")
> + libapi(i915);
> +
> igt_fixture
> igt_stop_hang_detector();
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [igt-dev] [PATCH i-g-t 2/4] i915/gem_ctx_engine: Exercise for_each_context_engine() with custom engine[]
From: Antonio Argenziano @ 2020-02-14 21:49 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
In-Reply-To: <20200214194016.4054376-2-chris@chris-wilson.co.uk>
On 14/02/20 11:40, Chris Wilson wrote:
> Set up a custom engine map with no engines, and check that the
> for_each_context_engine() correctly iterates over nothing.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> tests/i915/gem_ctx_engines.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/tests/i915/gem_ctx_engines.c b/tests/i915/gem_ctx_engines.c
> index 063140e0f..6a2edd1e0 100644
> --- a/tests/i915/gem_ctx_engines.c
> +++ b/tests/i915/gem_ctx_engines.c
> @@ -549,6 +549,31 @@ static void independent(int i915)
> gem_context_destroy(i915, param.ctx_id);
> }
>
> +static void libapi(int i915)
> +{
> + struct i915_context_param_engines engines = {};
Is there a case for invalid engines as well?
Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>
> + struct drm_i915_gem_context_param p = {
> + .ctx_id = gem_context_create(i915),
> + .param = I915_CONTEXT_PARAM_ENGINES,
> + .value = to_user_pointer(&engines),
> + .size = sizeof(engines),
> + };
> + const struct intel_execution_engine2 *e;
> + unsigned int count = 0;
> +
> + gem_context_set_param(i915, &p);
> +
> + for_each_context_engine(i915, p.ctx_id, e)
> + count++;
> + igt_assert_eq(count, 0);
> +
> + ____for_each_physical_engine(i915, p.ctx_id, e)
> + count++;
> + igt_assert_eq(count, 0);
> +
> + gem_context_destroy(i915, p.ctx_id);
> +}
> +
> igt_main
> {
> int i915 = -1;
> @@ -584,6 +609,9 @@ igt_main
> igt_subtest("independent")
> independent(i915);
>
> + igt_subtest("libapi")
> + libapi(i915);
> +
> igt_fixture
> igt_stop_hang_detector();
> }
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply
* [PATCH v2 4.9] padata: Remove broken queue flushing
From: Daniel Jordan @ 2020-02-14 21:48 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin
Cc: Steffen Klassert, linux-kernel, Herbert Xu, stable, Daniel Jordan
In-Reply-To: <20200213151856.812892483@linuxfoundation.org>
From: Herbert Xu <herbert@gondor.apana.org.au>
[ Upstream commit 07928d9bfc81640bab36f5190e8725894d93b659 ]
The function padata_flush_queues is fundamentally broken because
it cannot force padata users to complete the request that is
underway. IOW padata has to passively wait for the completion
of any outstanding work.
As it stands flushing is used in two places. Its use in padata_stop
is simply unnecessary because nothing depends on the queues to
be flushed afterwards.
The other use in padata_replace is more substantial as we depend
on it to free the old pd structure. This patch instead uses the
pd->refcnt to dynamically free the pd structure once all requests
are complete.
Fixes: 2b73b07ab8a4 ("padata: Flush the padata queues actively")
Cc: <stable@vger.kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Reviewed-by: Daniel Jordan <daniel.m.jordan@oracle.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
[dj: leave "pd->pinst = pinst" assignment in padata_alloc_pd()]
Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
---
kernel/padata.c | 45 ++++++++++++---------------------------------
1 file changed, 12 insertions(+), 33 deletions(-)
diff --git a/kernel/padata.c b/kernel/padata.c
index 63449fc584da..286c5142a0f7 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -34,6 +34,8 @@
#define MAX_OBJ_NUM 1000
+static void padata_free_pd(struct parallel_data *pd);
+
static int padata_index_to_cpu(struct parallel_data *pd, int cpu_index)
{
int cpu, target_cpu;
@@ -301,6 +303,7 @@ static void padata_serial_worker(struct work_struct *serial_work)
struct padata_serial_queue *squeue;
struct parallel_data *pd;
LIST_HEAD(local_list);
+ int cnt;
local_bh_disable();
squeue = container_of(serial_work, struct padata_serial_queue, work);
@@ -310,6 +313,8 @@ static void padata_serial_worker(struct work_struct *serial_work)
list_replace_init(&squeue->serial.list, &local_list);
spin_unlock(&squeue->serial.lock);
+ cnt = 0;
+
while (!list_empty(&local_list)) {
struct padata_priv *padata;
@@ -319,9 +324,12 @@ static void padata_serial_worker(struct work_struct *serial_work)
list_del_init(&padata->list);
padata->serial(padata);
- atomic_dec(&pd->refcnt);
+ cnt++;
}
local_bh_enable();
+
+ if (atomic_sub_and_test(cnt, &pd->refcnt))
+ padata_free_pd(pd);
}
/**
@@ -444,7 +452,7 @@ static struct parallel_data *padata_alloc_pd(struct padata_instance *pinst,
setup_timer(&pd->timer, padata_reorder_timer, (unsigned long)pd);
atomic_set(&pd->seq_nr, -1);
atomic_set(&pd->reorder_objects, 0);
- atomic_set(&pd->refcnt, 0);
+ atomic_set(&pd->refcnt, 1);
pd->pinst = pinst;
spin_lock_init(&pd->lock);
@@ -469,31 +477,6 @@ static void padata_free_pd(struct parallel_data *pd)
kfree(pd);
}
-/* Flush all objects out of the padata queues. */
-static void padata_flush_queues(struct parallel_data *pd)
-{
- int cpu;
- struct padata_parallel_queue *pqueue;
- struct padata_serial_queue *squeue;
-
- for_each_cpu(cpu, pd->cpumask.pcpu) {
- pqueue = per_cpu_ptr(pd->pqueue, cpu);
- flush_work(&pqueue->work);
- }
-
- del_timer_sync(&pd->timer);
-
- if (atomic_read(&pd->reorder_objects))
- padata_reorder(pd);
-
- for_each_cpu(cpu, pd->cpumask.cbcpu) {
- squeue = per_cpu_ptr(pd->squeue, cpu);
- flush_work(&squeue->work);
- }
-
- BUG_ON(atomic_read(&pd->refcnt) != 0);
-}
-
static void __padata_start(struct padata_instance *pinst)
{
pinst->flags |= PADATA_INIT;
@@ -507,10 +490,6 @@ static void __padata_stop(struct padata_instance *pinst)
pinst->flags &= ~PADATA_INIT;
synchronize_rcu();
-
- get_online_cpus();
- padata_flush_queues(pinst->pd);
- put_online_cpus();
}
/* Replace the internal control structure with a new one. */
@@ -531,8 +510,8 @@ static void padata_replace(struct padata_instance *pinst,
if (!cpumask_equal(pd_old->cpumask.cbcpu, pd_new->cpumask.cbcpu))
notification_mask |= PADATA_CPU_SERIAL;
- padata_flush_queues(pd_old);
- padata_free_pd(pd_old);
+ if (atomic_dec_and_test(&pd_old->refcnt))
+ padata_free_pd(pd_old);
if (notification_mask)
blocking_notifier_call_chain(&pinst->cpumask_change_notifier,
--
2.25.0
^ permalink raw reply related
* [Intel-gfx] [v3] drm/i915/tgl: Add Wa_1606054188:tgl
From: Matt Atwood @ 2020-02-15 6:49 UTC (permalink / raw)
To: intel-gfx; +Cc: Ville Syrjälä
On Tiger Lake we do not support source keying in the pixel formats P010,
P012, P016.
v2: Move WA to end of function. Create helper function for format
check. Less verbose debugging messaging.
v3: whitespace
Bspec: 52890
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
CC: Ville Syrjälä <ville.syrjala@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
drivers/gpu/drm/i915/display/intel_sprite.c | 22 +++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 2f277d1fc6f1..71ff9849b5c0 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -2070,6 +2070,18 @@ vlv_sprite_check(struct intel_crtc_state *crtc_state,
return 0;
}
+static bool intel_format_is_p01x(int format)
+{
+ switch (format) {
+ case DRM_FORMAT_P010:
+ case DRM_FORMAT_P012:
+ case DRM_FORMAT_P016:
+ return true;
+ default:
+ return false;
+ }
+}
+
static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
@@ -2143,6 +2155,16 @@ static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
return -EINVAL;
}
+ /* Wa_1606054188:tgl
+ *
+ * TODO: Add format RGB64i when implemented.
+ *
+ */
+ if (IS_GEN(dev_priv, 12) &&
+ plane_state->ckey.flags & I915_SET_COLORKEY_SOURCE &&
+ intel_format_is_p01x(fb->format->format))
+ DRM_DEBUG_KMS("Source color keying not supported with P01x formats\n");
+
return 0;
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* [igt-dev] [PATCH i-g-t] lib: Update selftests to use dynamic subtests
From: Chris Wilson @ 2020-02-14 21:48 UTC (permalink / raw)
To: igt-dev; +Cc: Tomi Sarvela, Petri Latvala
Use the new igt_subtest_with_dynamic to nicely group the dynamic
subtests together.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
---
lib/igt_kmod.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 05019c24c..e701545d3 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -21,6 +21,7 @@
* IN THE SOFTWARE.
*/
+#include <ctype.h>
#include <signal.h>
#include <errno.h>
@@ -602,6 +603,18 @@ void igt_kselftest_fini(struct igt_kselftest *tst)
kmod_module_unref(tst->kmod);
}
+static const char *unfilter(const char *filter, const char *name)
+{
+ if (!filter)
+ return name;
+
+ name += strlen(filter);
+ if (!isalpha(*name))
+ name++;
+
+ return name;
+}
+
void igt_kselftests(const char *module_name,
const char *options,
const char *result,
@@ -618,10 +631,12 @@ void igt_kselftests(const char *module_name,
igt_require(igt_kselftest_begin(&tst) == 0);
igt_kselftest_get_tests(tst.kmod, filter, &tests);
- igt_list_for_each_entry_safe(tl, tn, &tests, link) {
- igt_subtest_f("%s", tl->name)
- igt_kselftest_execute(&tst, tl, options, result);
- free(tl);
+ igt_subtest_with_dynamic(filter ?: "all") {
+ igt_list_for_each_entry_safe(tl, tn, &tests, link) {
+ igt_dynamic_f("%s", unfilter(filter, tl->name))
+ igt_kselftest_execute(&tst, tl, options, result);
+ free(tl);
+ }
}
igt_fixture {
--
2.25.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.