Re: [RFC][PATCH 1/2] memcg: Ensure every task that uses an mm is in the same memory cgroup

2018-06-06 Thread Michal Hocko
On Fri 01-06-18 09:53:09, Eric W. Biederman wrote:
[...]
> +static int mem_cgroup_mm_can_attach(struct cgroup_taskset *tset)
> +{
> + struct cgroup_subsys_state *css, *tcss;
> + struct task_struct *p, *t;
> + struct mm_struct *mm = NULL;
> + int ret = -EINVAL;
> +
> + /*
> +  * Ensure all references for every mm can be accounted for by
> +  * tasks that are being migrated.
> +  */
> + rcu_read_lock();
> + cgroup_taskset_for_each(p, css, tset) {
> + int mm_users, mm_count;
> +
> + /* Does this task have an mm that has not been visited? */
> + if (!p->mm || (p->flags & PF_KTHREAD) || (p->mm == mm))
> + continue;
> +
> + mm = p->mm;
> + mm_users = atomic_read(>mm_users);
> + if (mm_users == 1)
> + continue;
> +
> + mm_count = 0;
> + cgroup_taskset_for_each(t, tcss, tset) {
> + if (t->mm != mm)
> + continue;
> + mm_count++;
> + }
> + /*
> +  * If there are no non-task references mm_users will
> +  * be stable as holding cgroup_thread_rwsem for write
> +  * blocks fork and exit.
> +  */
> + if (mm_count != mm_users)
> + goto out;

Wouldn't it be much more simple to do something like this instead? Sure
it will break migration of non-thread tasks sharing mms but I would like
to see that this actually matters to anybody rather than make the code
more complicated and maintain it forever without a good reason. So yes
this is a bit harsh but considering that the migration suceeded silently
and that was simply broken as well, I am not sure this is any worse.

Btw. MMF_ALIEN_MM could be used in the OOM proper as well.

diff --git a/kernel/fork.c b/kernel/fork.c
index dfe8e14c0fba..285cd0397307 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1303,6 +1303,8 @@ static int copy_mm(unsigned long clone_flags, struct 
task_struct *tsk)
if (clone_flags & CLONE_VM) {
mmget(oldmm);
mm = oldmm;
+   if (unlikely(!(clone_flags & CLONE_THREAD)))
+   set_bit(MMF_ALIEN_MM, >flags);
goto good_mm;
}
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 2f5dac193431..fa0248fcdb36 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5103,6 +5103,18 @@ static int mem_cgroup_can_attach(struct cgroup_taskset 
*tset)
if (!mm)
return 0;
 
+   /*
+* Migrating a task which shares mm with other thread group
+* has never been really well defined. Shout to the log when
+* this happens and see whether anybody really complains.
+* If so make sure to support migration if all processes sharing
+* this mm are migrating together.
+*/
+   if (WARN_ON_ONCE(test_bit(MMF_ALIEN_MM, >flags))) {
+   mmput(mm);
+   return -EBUSY;
+   }
+
/* We move charges except for creative uses of CLONE_VM */
if (mm->memcg == from) {
VM_BUG_ON(mc.from);

-- 
Michal Hocko
SUSE Labs


Re: [RFC][PATCH 1/2] memcg: Ensure every task that uses an mm is in the same memory cgroup

2018-06-06 Thread Michal Hocko
On Fri 01-06-18 09:53:09, Eric W. Biederman wrote:
[...]
> +static int mem_cgroup_mm_can_attach(struct cgroup_taskset *tset)
> +{
> + struct cgroup_subsys_state *css, *tcss;
> + struct task_struct *p, *t;
> + struct mm_struct *mm = NULL;
> + int ret = -EINVAL;
> +
> + /*
> +  * Ensure all references for every mm can be accounted for by
> +  * tasks that are being migrated.
> +  */
> + rcu_read_lock();
> + cgroup_taskset_for_each(p, css, tset) {
> + int mm_users, mm_count;
> +
> + /* Does this task have an mm that has not been visited? */
> + if (!p->mm || (p->flags & PF_KTHREAD) || (p->mm == mm))
> + continue;
> +
> + mm = p->mm;
> + mm_users = atomic_read(>mm_users);
> + if (mm_users == 1)
> + continue;
> +
> + mm_count = 0;
> + cgroup_taskset_for_each(t, tcss, tset) {
> + if (t->mm != mm)
> + continue;
> + mm_count++;
> + }
> + /*
> +  * If there are no non-task references mm_users will
> +  * be stable as holding cgroup_thread_rwsem for write
> +  * blocks fork and exit.
> +  */
> + if (mm_count != mm_users)
> + goto out;

Wouldn't it be much more simple to do something like this instead? Sure
it will break migration of non-thread tasks sharing mms but I would like
to see that this actually matters to anybody rather than make the code
more complicated and maintain it forever without a good reason. So yes
this is a bit harsh but considering that the migration suceeded silently
and that was simply broken as well, I am not sure this is any worse.

Btw. MMF_ALIEN_MM could be used in the OOM proper as well.

diff --git a/kernel/fork.c b/kernel/fork.c
index dfe8e14c0fba..285cd0397307 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1303,6 +1303,8 @@ static int copy_mm(unsigned long clone_flags, struct 
task_struct *tsk)
if (clone_flags & CLONE_VM) {
mmget(oldmm);
mm = oldmm;
+   if (unlikely(!(clone_flags & CLONE_THREAD)))
+   set_bit(MMF_ALIEN_MM, >flags);
goto good_mm;
}
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 2f5dac193431..fa0248fcdb36 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5103,6 +5103,18 @@ static int mem_cgroup_can_attach(struct cgroup_taskset 
*tset)
if (!mm)
return 0;
 
+   /*
+* Migrating a task which shares mm with other thread group
+* has never been really well defined. Shout to the log when
+* this happens and see whether anybody really complains.
+* If so make sure to support migration if all processes sharing
+* this mm are migrating together.
+*/
+   if (WARN_ON_ONCE(test_bit(MMF_ALIEN_MM, >flags))) {
+   mmput(mm);
+   return -EBUSY;
+   }
+
/* We move charges except for creative uses of CLONE_VM */
if (mm->memcg == from) {
VM_BUG_ON(mc.from);

-- 
Michal Hocko
SUSE Labs


Re: [PATCH v3 3/6] mtd: rawnand: tegra: add devicetree binding

2018-06-06 Thread Thierry Reding
On Wed, Jun 06, 2018 at 12:45:40PM +0200, Boris Brezillon wrote:
> Hi Thierry,
> 
> On Wed, 6 Jun 2018 12:39:03 +0200
> Thierry Reding  wrote:
> 
> > On Tue, Jun 05, 2018 at 11:19:14PM +0300, Dmitry Osipenko wrote:
> > > On 01.06.2018 10:30, Boris Brezillon wrote:  
> > > > On Fri,  1 Jun 2018 00:16:34 +0200
> > > > Stefan Agner  wrote:
> > > >   
> > > >> This adds the devicetree binding for the Tegra 2 NAND flash
> > > >> controller.
> > > >>
> > > >> Signed-off-by: Lucas Stach 
> > > >> Signed-off-by: Stefan Agner 
> > > >> ---
> > > >>  .../bindings/mtd/nvidia-tegra20-nand.txt  | 64 +++
> > > >>  1 file changed, 64 insertions(+)
> > > >>  create mode 100644 
> > > >> Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > > >>
> > > >> diff --git 
> > > >> a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt 
> > > >> b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > > >> new file mode 100644
> > > >> index ..5cd984ef046b
> > > >> --- /dev/null
> > > >> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > > >> @@ -0,0 +1,64 @@
> > > >> +NVIDIA Tegra NAND Flash controller
> > > >> +
> > > >> +Required properties:
> > > >> +- compatible: Must be one of:
> > > >> +  - "nvidia,tegra20-nand"  
> > > > 
> > > > As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
> > > > "nvidia,tegra20-nfc".
> > > >   
> > > >> +- reg: MMIO address range
> > > >> +- interrupts: interrupt output of the NFC controller
> > > >> +- clocks: Must contain an entry for each entry in clock-names.
> > > >> +  See ../clocks/clock-bindings.txt for details.
> > > >> +- clock-names: Must include the following entries:
> > > >> +  - nand
> > > >> +- resets: Must contain an entry for each entry in reset-names.
> > > >> +  See ../reset/reset.txt for details.
> > > >> +- reset-names: Must include the following entries:
> > > >> +  - nand
> > > >> +
> > > >> +Optional children nodes:
> > > >> +Individual NAND chips are children of the NAND controller node. 
> > > >> Currently
> > > >> +only one NAND chip supported.
> > > >> +
> > > >> +Required children node properties:
> > > >> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> > > >> +
> > > >> +Optional children node properties:
> > > >> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. 
> > > >> Currently only
> > > >> +   "hw" is supported.
> > > >> +- nand-ecc-algo: string, algorithm of NAND ECC.
> > > >> +   Supported values with "hw" ECC mode are: "rs", "bch".
> > > >> +- nand-bus-width : See nand.txt
> > > >> +- nand-on-flash-bbt: See nand.txt
> > > >> +- nand-ecc-strength: integer representing the number of bits to 
> > > >> correct
> > > >> +   per ECC step (always 512). Supported strength 
> > > >> using HW ECC
> > > >> +   modes are:
> > > >> +   - RS: 4, 6, 8
> > > >> +   - BCH: 4, 8, 14, 16
> > > >> +- nand-ecc-maximize: See nand.txt
> > > >> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the 
> > > >> boot ROM
> > > >> + are choosen.
> > > >> +- wp-gpios: GPIO specifier for the write protect pin.
> > > >> +
> > > >> +Optional child node of NAND chip nodes:
> > > >> +Partitions: see partition.txt
> > > >> +
> > > >> +  Example:
> > > >> +  nand@70008000 {  
> > > > 
> > > > nand-controller@70008000 {
> > > >   
> > > >> +  compatible = "nvidia,tegra20-nand";  
> > > > 
> > > > compatible = "nvidia,tegra20-nand-controller";
> > > > 
> > > > or
> > > > 
> > > > compatible = "nvidia,tegra20-nfc";
> > > >   
> > > 
> > > Maybe it's just me, but when I'm reading "nfc", my first association is 
> > > the
> > > "Near Field Communication". Probably an explicit
> > > "nvidia,tegra20-nand-controller" variant is more preferable.  
> 
> I also prefer nvidia,tegra20-nand-controller.
> 
> > 
> > We don't really use a -controller suffix for any of the other
> > controllers because it is kind of implied. "nfc" is also not something
> > that is ever referred to in the technical documentation.
> > 
> > "nvidia,tegra20-nand" would be most consistent with all the rest of
> > Tegra (c.f. "nvidia,tegra*-ahci", "nvidia,tegra*-pci",
> > "nvidia,tegra*-hda", "nvidia,tegra*-gmi", ...).
> 
> People get confused about what this node represents when you just have
> "nvidia,tegra20-nand", and then you start seeing NAND related props or
> partition nodes being defined under the NAND controller node.
> I really prefer to have the "-controller" prefix here to avoid such
> confusions.

Hmm... odd. I mean, the node is already called nand-controller@...,
which makes it pretty obvious to me that this represents a controller
rather than a NAND chip. Also, the placement of this in the DT hierarchy
should make it pretty obvious that it is a controller since you can't
just put a NAND chip 

Re: [PATCH v3 3/6] mtd: rawnand: tegra: add devicetree binding

2018-06-06 Thread Thierry Reding
On Wed, Jun 06, 2018 at 12:45:40PM +0200, Boris Brezillon wrote:
> Hi Thierry,
> 
> On Wed, 6 Jun 2018 12:39:03 +0200
> Thierry Reding  wrote:
> 
> > On Tue, Jun 05, 2018 at 11:19:14PM +0300, Dmitry Osipenko wrote:
> > > On 01.06.2018 10:30, Boris Brezillon wrote:  
> > > > On Fri,  1 Jun 2018 00:16:34 +0200
> > > > Stefan Agner  wrote:
> > > >   
> > > >> This adds the devicetree binding for the Tegra 2 NAND flash
> > > >> controller.
> > > >>
> > > >> Signed-off-by: Lucas Stach 
> > > >> Signed-off-by: Stefan Agner 
> > > >> ---
> > > >>  .../bindings/mtd/nvidia-tegra20-nand.txt  | 64 +++
> > > >>  1 file changed, 64 insertions(+)
> > > >>  create mode 100644 
> > > >> Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > > >>
> > > >> diff --git 
> > > >> a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt 
> > > >> b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > > >> new file mode 100644
> > > >> index ..5cd984ef046b
> > > >> --- /dev/null
> > > >> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > > >> @@ -0,0 +1,64 @@
> > > >> +NVIDIA Tegra NAND Flash controller
> > > >> +
> > > >> +Required properties:
> > > >> +- compatible: Must be one of:
> > > >> +  - "nvidia,tegra20-nand"  
> > > > 
> > > > As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
> > > > "nvidia,tegra20-nfc".
> > > >   
> > > >> +- reg: MMIO address range
> > > >> +- interrupts: interrupt output of the NFC controller
> > > >> +- clocks: Must contain an entry for each entry in clock-names.
> > > >> +  See ../clocks/clock-bindings.txt for details.
> > > >> +- clock-names: Must include the following entries:
> > > >> +  - nand
> > > >> +- resets: Must contain an entry for each entry in reset-names.
> > > >> +  See ../reset/reset.txt for details.
> > > >> +- reset-names: Must include the following entries:
> > > >> +  - nand
> > > >> +
> > > >> +Optional children nodes:
> > > >> +Individual NAND chips are children of the NAND controller node. 
> > > >> Currently
> > > >> +only one NAND chip supported.
> > > >> +
> > > >> +Required children node properties:
> > > >> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> > > >> +
> > > >> +Optional children node properties:
> > > >> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. 
> > > >> Currently only
> > > >> +   "hw" is supported.
> > > >> +- nand-ecc-algo: string, algorithm of NAND ECC.
> > > >> +   Supported values with "hw" ECC mode are: "rs", "bch".
> > > >> +- nand-bus-width : See nand.txt
> > > >> +- nand-on-flash-bbt: See nand.txt
> > > >> +- nand-ecc-strength: integer representing the number of bits to 
> > > >> correct
> > > >> +   per ECC step (always 512). Supported strength 
> > > >> using HW ECC
> > > >> +   modes are:
> > > >> +   - RS: 4, 6, 8
> > > >> +   - BCH: 4, 8, 14, 16
> > > >> +- nand-ecc-maximize: See nand.txt
> > > >> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the 
> > > >> boot ROM
> > > >> + are choosen.
> > > >> +- wp-gpios: GPIO specifier for the write protect pin.
> > > >> +
> > > >> +Optional child node of NAND chip nodes:
> > > >> +Partitions: see partition.txt
> > > >> +
> > > >> +  Example:
> > > >> +  nand@70008000 {  
> > > > 
> > > > nand-controller@70008000 {
> > > >   
> > > >> +  compatible = "nvidia,tegra20-nand";  
> > > > 
> > > > compatible = "nvidia,tegra20-nand-controller";
> > > > 
> > > > or
> > > > 
> > > > compatible = "nvidia,tegra20-nfc";
> > > >   
> > > 
> > > Maybe it's just me, but when I'm reading "nfc", my first association is 
> > > the
> > > "Near Field Communication". Probably an explicit
> > > "nvidia,tegra20-nand-controller" variant is more preferable.  
> 
> I also prefer nvidia,tegra20-nand-controller.
> 
> > 
> > We don't really use a -controller suffix for any of the other
> > controllers because it is kind of implied. "nfc" is also not something
> > that is ever referred to in the technical documentation.
> > 
> > "nvidia,tegra20-nand" would be most consistent with all the rest of
> > Tegra (c.f. "nvidia,tegra*-ahci", "nvidia,tegra*-pci",
> > "nvidia,tegra*-hda", "nvidia,tegra*-gmi", ...).
> 
> People get confused about what this node represents when you just have
> "nvidia,tegra20-nand", and then you start seeing NAND related props or
> partition nodes being defined under the NAND controller node.
> I really prefer to have the "-controller" prefix here to avoid such
> confusions.

Hmm... odd. I mean, the node is already called nand-controller@...,
which makes it pretty obvious to me that this represents a controller
rather than a NAND chip. Also, the placement of this in the DT hierarchy
should make it pretty obvious that it is a controller since you can't
just put a NAND chip 

Re: [PATCH v5 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

2018-06-06 Thread Richard Genoud
Typo in the subject:
changed->change


On 04/06/2018 18:59, Radu Pirea wrote:
> This patch modifies the place where resources and device tree properties
> are searched.
> 
> Signed-off-by: Radu Pirea 
> ---
>  drivers/tty/serial/Kconfig|  1 +
>  drivers/tty/serial/atmel_serial.c | 41 ++-
>  2 files changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 3682fd3e960c..25e55332f8b1 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -119,6 +119,7 @@ config SERIAL_ATMEL
>   depends on ARCH_AT91 || COMPILE_TEST
>   select SERIAL_CORE
>   select SERIAL_MCTRL_GPIO if GPIOLIB
> + select MFD_AT91_USART
>   help
> This enables the driver for the on-chip UARTs of the Atmel
> AT91 processors.
> diff --git a/drivers/tty/serial/atmel_serial.c 
> b/drivers/tty/serial/atmel_serial.c
> index df46a9e88c34..5c74e03396ef 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -193,8 +193,8 @@ static struct console atmel_console;
>  
>  #if defined(CONFIG_OF)
>  static const struct of_device_id atmel_serial_dt_ids[] = {
> - { .compatible = "atmel,at91rm9200-usart" },
> - { .compatible = "atmel,at91sam9260-usart" },
> + { .compatible = "atmel,at91rm9200-usart-serial" },
> + { .compatible = "atmel,at91sam9260-usart-serial" },
Sorry, I didn't catch that before, but we can drop 
"atmel,at91sam9260-usart-serial" don't we ?
Only "atmel,at91rm9200-usart-serial" is used in the MFD driver.

>   { /* sentinel */ }
>  };
>  #endif
> @@ -915,6 +915,7 @@ static void atmel_tx_dma(struct uart_port *port)
>  static int atmel_prepare_tx_dma(struct uart_port *port)
>  {
>   struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + struct device *mfd_dev = port->dev->parent;
>   dma_cap_mask_t  mask;
>   struct dma_slave_config config;
>   int ret, nent;
> @@ -922,7 +923,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port)
>   dma_cap_zero(mask);
>   dma_cap_set(DMA_SLAVE, mask);
>  
> - atmel_port->chan_tx = dma_request_slave_channel(port->dev, "tx");
> + atmel_port->chan_tx = dma_request_slave_channel(mfd_dev, "tx");
>   if (atmel_port->chan_tx == NULL)
>   goto chan_err;
>   dev_info(port->dev, "using %s for tx DMA transfers\n",
> @@ -1093,6 +1094,7 @@ static void atmel_rx_from_dma(struct uart_port *port)
>  static int atmel_prepare_rx_dma(struct uart_port *port)
>  {
>   struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + struct device *mfd_dev = port->dev->parent;
>   struct dma_async_tx_descriptor *desc;
>   dma_cap_mask_t  mask;
>   struct dma_slave_config config;
> @@ -1104,7 +1106,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>   dma_cap_zero(mask);
>   dma_cap_set(DMA_CYCLIC, mask);
>  
> - atmel_port->chan_rx = dma_request_slave_channel(port->dev, "rx");
> + atmel_port->chan_rx = dma_request_slave_channel(mfd_dev, "rx");
>   if (atmel_port->chan_rx == NULL)
>   goto chan_err;
>   dev_info(port->dev, "using %s for rx DMA transfers\n",
> @@ -1631,7 +1633,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
>  static void atmel_init_property(struct atmel_uart_port *atmel_port,
>   struct platform_device *pdev)
>  {
> - struct device_node *np = pdev->dev.of_node;
> + struct device_node *np = pdev->dev.parent->of_node;
I think this is not needed anymore (cf atmel_probe())

>  
>   /* DMA/PDC usage specification */
>   if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> @@ -,8 +2224,8 @@ static const char *atmel_type(struct uart_port *port)
>   */
>  static void atmel_release_port(struct uart_port *port)
>  {
> - struct platform_device *pdev = to_platform_device(port->dev);
> - int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> + struct platform_device *mpdev = to_platform_device(port->dev->parent);
> + int size = resource_size(mpdev->resource);
>  
>   release_mem_region(port->mapbase, size);
>  
> @@ -2238,8 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
>   */
>  static int atmel_request_port(struct uart_port *port)
>  {
> - struct platform_device *pdev = to_platform_device(port->dev);
> - int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> + struct platform_device *mpdev = to_platform_device(port->dev->parent);
> + int size = resource_size(mpdev->resource);
>  
>   if (!request_mem_region(port->mapbase, size, "atmel_serial"))
>   return -EBUSY;
> @@ -2341,27 +2343,28 @@ static int atmel_init_port(struct atmel_uart_port 
> *atmel_port,
>  {
>   int ret;
>   struct uart_port *port = _port->uart;
> + struct platform_device *mpdev = to_platform_device(pdev->dev.parent);
>  

Re: [PATCH v2 5/5] memory: tegra: Introduce Tegra20 EMC driver

2018-06-06 Thread Thierry Reding
On Mon, Jun 04, 2018 at 01:36:54AM +0300, Dmitry Osipenko wrote:
> Introduce driver for the External Memory Controller (EMC) found on Tegra20
> chips, which controls the external DRAM on the board. The purpose of this
> driver is to program memory timing for external memory on the EMC clock
> rate change.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/memory/tegra/Kconfig   |  10 +
>  drivers/memory/tegra/Makefile  |   1 +
>  drivers/memory/tegra/tegra20-emc.c | 586 +
>  3 files changed, 597 insertions(+)
>  create mode 100644 drivers/memory/tegra/tegra20-emc.c
> 
> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
> index 6d74e499e18d..34e0b70f5c5f 100644
> --- a/drivers/memory/tegra/Kconfig
> +++ b/drivers/memory/tegra/Kconfig
> @@ -6,6 +6,16 @@ config TEGRA_MC
> This driver supports the Memory Controller (MC) hardware found on
> NVIDIA Tegra SoCs.
>  
> +config TEGRA20_EMC
> + bool "NVIDIA Tegra20 External Memory Controller driver"
> + default y
> + depends on ARCH_TEGRA_2x_SOC
> + help
> +   This driver is for the External Memory Controller (EMC) found on
> +   Tegra20 chips. The EMC controls the external DRAM on the board.
> +   This driver is required to change memory timings / clock rate for
> +   external memory.
> +
>  config TEGRA124_EMC
>   bool "NVIDIA Tegra124 External Memory Controller driver"
>   default y
> diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile
> index 94ab16ba075b..3971a6b7c487 100644
> --- a/drivers/memory/tegra/Makefile
> +++ b/drivers/memory/tegra/Makefile
> @@ -10,5 +10,6 @@ tegra-mc-$(CONFIG_ARCH_TEGRA_210_SOC) += tegra210.o
>  
>  obj-$(CONFIG_TEGRA_MC) += tegra-mc.o
>  
> +obj-$(CONFIG_TEGRA20_EMC)  += tegra20-emc.o
>  obj-$(CONFIG_TEGRA124_EMC) += tegra124-emc.o
>  obj-$(CONFIG_ARCH_TEGRA_186_SOC) += tegra186.o
> diff --git a/drivers/memory/tegra/tegra20-emc.c 
> b/drivers/memory/tegra/tegra20-emc.c
> new file mode 100644
> index ..26a18b5e7941
> --- /dev/null
> +++ b/drivers/memory/tegra/tegra20-emc.c
> @@ -0,0 +1,586 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Tegra20 External Memory Controller driver
> + *
> + * Author: Dmitry Osipenko 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#define EMC_INTSTATUS0x000
> +#define EMC_INTMASK  0x004
> +#define EMC_TIMING_CONTROL   0x028
> +#define EMC_RC   0x02c
> +#define EMC_RFC  0x030
> +#define EMC_RAS  0x034
> +#define EMC_RP   0x038
> +#define EMC_R2W  0x03c
> +#define EMC_W2R  0x040
> +#define EMC_R2P  0x044
> +#define EMC_W2P  0x048
> +#define EMC_RD_RCD   0x04c
> +#define EMC_WR_RCD   0x050
> +#define EMC_RRD  0x054
> +#define EMC_REXT 0x058
> +#define EMC_WDV  0x05c
> +#define EMC_QUSE 0x060
> +#define EMC_QRST 0x064
> +#define EMC_QSAFE0x068
> +#define EMC_RDV  0x06c
> +#define EMC_REFRESH  0x070
> +#define EMC_BURST_REFRESH_NUM0x074
> +#define EMC_PDEX2WR  0x078
> +#define EMC_PDEX2RD  0x07c
> +#define EMC_PCHG2PDEN0x080
> +#define EMC_ACT2PDEN 0x084
> +#define EMC_AR2PDEN  0x088
> +#define EMC_RW2PDEN  0x08c
> +#define EMC_TXSR 0x090
> +#define EMC_TCKE 0x094
> +#define EMC_TFAW 0x098
> +#define EMC_TRPAB0x09c
> +#define EMC_TCLKSTABLE   0x0a0
> +#define EMC_TCLKSTOP 0x0a4
> +#define EMC_TREFBW   0x0a8
> +#define EMC_QUSE_EXTRA   0x0ac
> +#define EMC_ODT_WRITE0x0b0
> +#define EMC_ODT_READ 0x0b4
> +#define EMC_FBIO_CFG50x104
> +#define EMC_FBIO_CFG60x114
> +#define EMC_AUTO_CAL_INTERVAL0x2a8
> +#define EMC_CFG_20x2b8
> +#define EMC_CFG_DIG_DLL  0x2bc
> +#define EMC_DLL_XFORM_DQS

Re: [PATCH v5 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

2018-06-06 Thread Richard Genoud
Typo in the subject:
changed->change


On 04/06/2018 18:59, Radu Pirea wrote:
> This patch modifies the place where resources and device tree properties
> are searched.
> 
> Signed-off-by: Radu Pirea 
> ---
>  drivers/tty/serial/Kconfig|  1 +
>  drivers/tty/serial/atmel_serial.c | 41 ++-
>  2 files changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 3682fd3e960c..25e55332f8b1 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -119,6 +119,7 @@ config SERIAL_ATMEL
>   depends on ARCH_AT91 || COMPILE_TEST
>   select SERIAL_CORE
>   select SERIAL_MCTRL_GPIO if GPIOLIB
> + select MFD_AT91_USART
>   help
> This enables the driver for the on-chip UARTs of the Atmel
> AT91 processors.
> diff --git a/drivers/tty/serial/atmel_serial.c 
> b/drivers/tty/serial/atmel_serial.c
> index df46a9e88c34..5c74e03396ef 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -193,8 +193,8 @@ static struct console atmel_console;
>  
>  #if defined(CONFIG_OF)
>  static const struct of_device_id atmel_serial_dt_ids[] = {
> - { .compatible = "atmel,at91rm9200-usart" },
> - { .compatible = "atmel,at91sam9260-usart" },
> + { .compatible = "atmel,at91rm9200-usart-serial" },
> + { .compatible = "atmel,at91sam9260-usart-serial" },
Sorry, I didn't catch that before, but we can drop 
"atmel,at91sam9260-usart-serial" don't we ?
Only "atmel,at91rm9200-usart-serial" is used in the MFD driver.

>   { /* sentinel */ }
>  };
>  #endif
> @@ -915,6 +915,7 @@ static void atmel_tx_dma(struct uart_port *port)
>  static int atmel_prepare_tx_dma(struct uart_port *port)
>  {
>   struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + struct device *mfd_dev = port->dev->parent;
>   dma_cap_mask_t  mask;
>   struct dma_slave_config config;
>   int ret, nent;
> @@ -922,7 +923,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port)
>   dma_cap_zero(mask);
>   dma_cap_set(DMA_SLAVE, mask);
>  
> - atmel_port->chan_tx = dma_request_slave_channel(port->dev, "tx");
> + atmel_port->chan_tx = dma_request_slave_channel(mfd_dev, "tx");
>   if (atmel_port->chan_tx == NULL)
>   goto chan_err;
>   dev_info(port->dev, "using %s for tx DMA transfers\n",
> @@ -1093,6 +1094,7 @@ static void atmel_rx_from_dma(struct uart_port *port)
>  static int atmel_prepare_rx_dma(struct uart_port *port)
>  {
>   struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + struct device *mfd_dev = port->dev->parent;
>   struct dma_async_tx_descriptor *desc;
>   dma_cap_mask_t  mask;
>   struct dma_slave_config config;
> @@ -1104,7 +1106,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>   dma_cap_zero(mask);
>   dma_cap_set(DMA_CYCLIC, mask);
>  
> - atmel_port->chan_rx = dma_request_slave_channel(port->dev, "rx");
> + atmel_port->chan_rx = dma_request_slave_channel(mfd_dev, "rx");
>   if (atmel_port->chan_rx == NULL)
>   goto chan_err;
>   dev_info(port->dev, "using %s for rx DMA transfers\n",
> @@ -1631,7 +1633,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
>  static void atmel_init_property(struct atmel_uart_port *atmel_port,
>   struct platform_device *pdev)
>  {
> - struct device_node *np = pdev->dev.of_node;
> + struct device_node *np = pdev->dev.parent->of_node;
I think this is not needed anymore (cf atmel_probe())

>  
>   /* DMA/PDC usage specification */
>   if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> @@ -,8 +2224,8 @@ static const char *atmel_type(struct uart_port *port)
>   */
>  static void atmel_release_port(struct uart_port *port)
>  {
> - struct platform_device *pdev = to_platform_device(port->dev);
> - int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> + struct platform_device *mpdev = to_platform_device(port->dev->parent);
> + int size = resource_size(mpdev->resource);
>  
>   release_mem_region(port->mapbase, size);
>  
> @@ -2238,8 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
>   */
>  static int atmel_request_port(struct uart_port *port)
>  {
> - struct platform_device *pdev = to_platform_device(port->dev);
> - int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> + struct platform_device *mpdev = to_platform_device(port->dev->parent);
> + int size = resource_size(mpdev->resource);
>  
>   if (!request_mem_region(port->mapbase, size, "atmel_serial"))
>   return -EBUSY;
> @@ -2341,27 +2343,28 @@ static int atmel_init_port(struct atmel_uart_port 
> *atmel_port,
>  {
>   int ret;
>   struct uart_port *port = _port->uart;
> + struct platform_device *mpdev = to_platform_device(pdev->dev.parent);
>  

Re: [PATCH v2 5/5] memory: tegra: Introduce Tegra20 EMC driver

2018-06-06 Thread Thierry Reding
On Mon, Jun 04, 2018 at 01:36:54AM +0300, Dmitry Osipenko wrote:
> Introduce driver for the External Memory Controller (EMC) found on Tegra20
> chips, which controls the external DRAM on the board. The purpose of this
> driver is to program memory timing for external memory on the EMC clock
> rate change.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/memory/tegra/Kconfig   |  10 +
>  drivers/memory/tegra/Makefile  |   1 +
>  drivers/memory/tegra/tegra20-emc.c | 586 +
>  3 files changed, 597 insertions(+)
>  create mode 100644 drivers/memory/tegra/tegra20-emc.c
> 
> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
> index 6d74e499e18d..34e0b70f5c5f 100644
> --- a/drivers/memory/tegra/Kconfig
> +++ b/drivers/memory/tegra/Kconfig
> @@ -6,6 +6,16 @@ config TEGRA_MC
> This driver supports the Memory Controller (MC) hardware found on
> NVIDIA Tegra SoCs.
>  
> +config TEGRA20_EMC
> + bool "NVIDIA Tegra20 External Memory Controller driver"
> + default y
> + depends on ARCH_TEGRA_2x_SOC
> + help
> +   This driver is for the External Memory Controller (EMC) found on
> +   Tegra20 chips. The EMC controls the external DRAM on the board.
> +   This driver is required to change memory timings / clock rate for
> +   external memory.
> +
>  config TEGRA124_EMC
>   bool "NVIDIA Tegra124 External Memory Controller driver"
>   default y
> diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile
> index 94ab16ba075b..3971a6b7c487 100644
> --- a/drivers/memory/tegra/Makefile
> +++ b/drivers/memory/tegra/Makefile
> @@ -10,5 +10,6 @@ tegra-mc-$(CONFIG_ARCH_TEGRA_210_SOC) += tegra210.o
>  
>  obj-$(CONFIG_TEGRA_MC) += tegra-mc.o
>  
> +obj-$(CONFIG_TEGRA20_EMC)  += tegra20-emc.o
>  obj-$(CONFIG_TEGRA124_EMC) += tegra124-emc.o
>  obj-$(CONFIG_ARCH_TEGRA_186_SOC) += tegra186.o
> diff --git a/drivers/memory/tegra/tegra20-emc.c 
> b/drivers/memory/tegra/tegra20-emc.c
> new file mode 100644
> index ..26a18b5e7941
> --- /dev/null
> +++ b/drivers/memory/tegra/tegra20-emc.c
> @@ -0,0 +1,586 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Tegra20 External Memory Controller driver
> + *
> + * Author: Dmitry Osipenko 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#define EMC_INTSTATUS0x000
> +#define EMC_INTMASK  0x004
> +#define EMC_TIMING_CONTROL   0x028
> +#define EMC_RC   0x02c
> +#define EMC_RFC  0x030
> +#define EMC_RAS  0x034
> +#define EMC_RP   0x038
> +#define EMC_R2W  0x03c
> +#define EMC_W2R  0x040
> +#define EMC_R2P  0x044
> +#define EMC_W2P  0x048
> +#define EMC_RD_RCD   0x04c
> +#define EMC_WR_RCD   0x050
> +#define EMC_RRD  0x054
> +#define EMC_REXT 0x058
> +#define EMC_WDV  0x05c
> +#define EMC_QUSE 0x060
> +#define EMC_QRST 0x064
> +#define EMC_QSAFE0x068
> +#define EMC_RDV  0x06c
> +#define EMC_REFRESH  0x070
> +#define EMC_BURST_REFRESH_NUM0x074
> +#define EMC_PDEX2WR  0x078
> +#define EMC_PDEX2RD  0x07c
> +#define EMC_PCHG2PDEN0x080
> +#define EMC_ACT2PDEN 0x084
> +#define EMC_AR2PDEN  0x088
> +#define EMC_RW2PDEN  0x08c
> +#define EMC_TXSR 0x090
> +#define EMC_TCKE 0x094
> +#define EMC_TFAW 0x098
> +#define EMC_TRPAB0x09c
> +#define EMC_TCLKSTABLE   0x0a0
> +#define EMC_TCLKSTOP 0x0a4
> +#define EMC_TREFBW   0x0a8
> +#define EMC_QUSE_EXTRA   0x0ac
> +#define EMC_ODT_WRITE0x0b0
> +#define EMC_ODT_READ 0x0b4
> +#define EMC_FBIO_CFG50x104
> +#define EMC_FBIO_CFG60x114
> +#define EMC_AUTO_CAL_INTERVAL0x2a8
> +#define EMC_CFG_20x2b8
> +#define EMC_CFG_DIG_DLL  0x2bc
> +#define EMC_DLL_XFORM_DQS

RE: [PATCH] tpm: separate cmd_ready/go_idle from runtime_pm

2018-06-06 Thread Winkler, Tomas
> 
> On Wed, May 30, 2018 at 10:52:28AM +, Winkler, Tomas wrote:
> > >
> > > On Wed, May 23, 2018 at 01:48:17PM +, Winkler, Tomas wrote:
> > > >
> > > > > On Tue, May 22, 2018 at 09:27:46AM +, Winkler, Tomas wrote:
> > > > > > >
> > > > > > > On Wed, May 16, 2018 at 10:46:00PM +0300, Tomas Winkler
> wrote:
> > > > > > > > New wrappers are added tpm_cmd_ready() and tpm_go_idle()
> > > > > wrappers
> > > > > > > > to streamline tpm_try_transmit code.
> TPM_TRANSMIT_UNLOCKED
> > > > > > > > flag
> > > > > is
> > > > > > > abused
> > > > > > > > to resolve tpm spaces recursive calls to tpm_transmit().
> > > > > > >
> > > > > > > This looks good and all but I don't think we want to abuse
> > > > > > > anything in the driver code, do we?
> > > > > >
> > > > > > It's not abuse just the flag UNLOCKED is not really named
> > > > > > correctly I think this has to be backported so wanted to do
> > > > > > less invasive
> > > change.
> > > > >
> > > > > It should be renamed anyway and possible merge conflicts are not
> > > > > hard to sort out in this change. Can you rename it as SPACE?
> > > >
> > > > Not sure, I believe UNLOCKED is still better name than SPACE, I'm
> > > >not  sure this is Do you also want to remove TPM_TRANSMIT_RAW?
> > > > clk_enable is handling its own anti recursion counter 'data-
> > > >clkrun_enabled'
> > > > but it should be all handled under one flag I guess.
> > > >
> > > > > Right, and even without rename this will probably cause merge
> > > > > conflicts at least in v4.4 an v4.9 since in-kernel RM landed in
> > > > > v4.12, so not much gain not do the rename :-)
> > > >
> > > > I belive we should do minimal change and the big cleanup after that.
> > > > Not sure, I believe UNLOCKED is still better name than SPACE even
> > > > it wasn't
> > > the original intention.
> > > > No the SPACE is the issue, but any recursion call into
> > > > tpm_transmit. A bigger change is needed and rename to SPACE would
> > > > be just another
> > > intermediat change.
> > > >
> > > > Please reconsider.
> > > >
> > > > Thanks
> > > > Tomas
> > >
> > > Reviewed-by: Jarkko Sakkinen 
> >
> >
> > Does it mean you're Okay with the patch now?
> > Thanks
> > Tomas
> 
> The change looks good but I'll have to test it.
Any updates?
Thanks



RE: [PATCH] tpm: separate cmd_ready/go_idle from runtime_pm

2018-06-06 Thread Winkler, Tomas
> 
> On Wed, May 30, 2018 at 10:52:28AM +, Winkler, Tomas wrote:
> > >
> > > On Wed, May 23, 2018 at 01:48:17PM +, Winkler, Tomas wrote:
> > > >
> > > > > On Tue, May 22, 2018 at 09:27:46AM +, Winkler, Tomas wrote:
> > > > > > >
> > > > > > > On Wed, May 16, 2018 at 10:46:00PM +0300, Tomas Winkler
> wrote:
> > > > > > > > New wrappers are added tpm_cmd_ready() and tpm_go_idle()
> > > > > wrappers
> > > > > > > > to streamline tpm_try_transmit code.
> TPM_TRANSMIT_UNLOCKED
> > > > > > > > flag
> > > > > is
> > > > > > > abused
> > > > > > > > to resolve tpm spaces recursive calls to tpm_transmit().
> > > > > > >
> > > > > > > This looks good and all but I don't think we want to abuse
> > > > > > > anything in the driver code, do we?
> > > > > >
> > > > > > It's not abuse just the flag UNLOCKED is not really named
> > > > > > correctly I think this has to be backported so wanted to do
> > > > > > less invasive
> > > change.
> > > > >
> > > > > It should be renamed anyway and possible merge conflicts are not
> > > > > hard to sort out in this change. Can you rename it as SPACE?
> > > >
> > > > Not sure, I believe UNLOCKED is still better name than SPACE, I'm
> > > >not  sure this is Do you also want to remove TPM_TRANSMIT_RAW?
> > > > clk_enable is handling its own anti recursion counter 'data-
> > > >clkrun_enabled'
> > > > but it should be all handled under one flag I guess.
> > > >
> > > > > Right, and even without rename this will probably cause merge
> > > > > conflicts at least in v4.4 an v4.9 since in-kernel RM landed in
> > > > > v4.12, so not much gain not do the rename :-)
> > > >
> > > > I belive we should do minimal change and the big cleanup after that.
> > > > Not sure, I believe UNLOCKED is still better name than SPACE even
> > > > it wasn't
> > > the original intention.
> > > > No the SPACE is the issue, but any recursion call into
> > > > tpm_transmit. A bigger change is needed and rename to SPACE would
> > > > be just another
> > > intermediat change.
> > > >
> > > > Please reconsider.
> > > >
> > > > Thanks
> > > > Tomas
> > >
> > > Reviewed-by: Jarkko Sakkinen 
> >
> >
> > Does it mean you're Okay with the patch now?
> > Thanks
> > Tomas
> 
> The change looks good but I'll have to test it.
Any updates?
Thanks



Re: [PATCH V2] xfs: fix string handling in get/set functions

2018-06-06 Thread Christoph Hellwig
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 82f7c83c1dad..596e176c19a6 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1828,13 +1828,13 @@ xfs_ioc_getlabel(
>   /* Paranoia */
>   BUILD_BUG_ON(sizeof(sbp->sb_fname) > FSLABEL_MAX);
>  
> + /* 1 larger than sb_fname, so this ensures a trailing NUL char */
> + memset(label, 0, sizeof(label));

I don't get the comment.  In fact I don't even get why we need any
comment here.  This is a structure that gets copied to userspace,
and we zero the whole structure, as we should do by default for
anything that goes to userspace.

Otherwise this looks fine to me.


Re: [PATCH V2] xfs: fix string handling in get/set functions

2018-06-06 Thread Christoph Hellwig
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 82f7c83c1dad..596e176c19a6 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1828,13 +1828,13 @@ xfs_ioc_getlabel(
>   /* Paranoia */
>   BUILD_BUG_ON(sizeof(sbp->sb_fname) > FSLABEL_MAX);
>  
> + /* 1 larger than sb_fname, so this ensures a trailing NUL char */
> + memset(label, 0, sizeof(label));

I don't get the comment.  In fact I don't even get why we need any
comment here.  This is a structure that gets copied to userspace,
and we zero the whole structure, as we should do by default for
anything that goes to userspace.

Otherwise this looks fine to me.


Re: [PATCH V3 02/17] kallsyms, x86: Export addresses of syscall trampolines

2018-06-06 Thread Peter Zijlstra
On Wed, Jun 06, 2018 at 11:02:08AM +0300, Adrian Hunter wrote:
> On 05/06/18 19:00, Andi Kleen wrote:
> >> +#ifdef CONFIG_X86_64
> >> +int arch_get_kallsym(unsigned int symnum, unsigned long *value, char 
> >> *type,
> >> +   char *name)
> >> +{
> >> +  unsigned int cpu, ncpu;
> >> +
> >> +  if (symnum >= num_possible_cpus())
> >> +  return -EINVAL;
> >> +
> >> +  for (cpu = cpumask_first(cpu_possible_mask), ncpu = 0;
> >> +   cpu < num_possible_cpus() && ncpu < symnum;
> >> +   cpu = cpumask_next(cpu, cpu_possible_mask), ncpu++)
> >> +  ;
> > 
> > That is max_t(unsigned, cpumask_last(cpu_possible_mask), symnum)
> 
> I think it should be:
> 
> -  cpu < num_possible_cpus() && ncpu < symnum;
> +  ncpu < symnum;

Since we're bike-shedding:

for_each_possible_cpu(cpu) {
if (ncpu++ >= symnum)
break;
}




Re: [PATCH V3 02/17] kallsyms, x86: Export addresses of syscall trampolines

2018-06-06 Thread Peter Zijlstra
On Wed, Jun 06, 2018 at 11:02:08AM +0300, Adrian Hunter wrote:
> On 05/06/18 19:00, Andi Kleen wrote:
> >> +#ifdef CONFIG_X86_64
> >> +int arch_get_kallsym(unsigned int symnum, unsigned long *value, char 
> >> *type,
> >> +   char *name)
> >> +{
> >> +  unsigned int cpu, ncpu;
> >> +
> >> +  if (symnum >= num_possible_cpus())
> >> +  return -EINVAL;
> >> +
> >> +  for (cpu = cpumask_first(cpu_possible_mask), ncpu = 0;
> >> +   cpu < num_possible_cpus() && ncpu < symnum;
> >> +   cpu = cpumask_next(cpu, cpu_possible_mask), ncpu++)
> >> +  ;
> > 
> > That is max_t(unsigned, cpumask_last(cpu_possible_mask), symnum)
> 
> I think it should be:
> 
> -  cpu < num_possible_cpus() && ncpu < symnum;
> +  ncpu < symnum;

Since we're bike-shedding:

for_each_possible_cpu(cpu) {
if (ncpu++ >= symnum)
break;
}




Re: [PATCH v5 8/8] staging: rtl8192e: remove unnecessary parentheses - Coding Style

2018-06-06 Thread Greg KH
On Wed, Jun 06, 2018 at 11:25:14AM +0100, John Whitmore wrote:
> On Sun, Jun 03, 2018 at 02:28:35PM +0200, Greg KH wrote:
> > On Sun, Jun 03, 2018 at 01:04:13PM +0100, John Whitmore wrote:
> > > Signed-off-by: John Whitmore 
> > > ---
> > >  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 12 ++--
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > I can not take patches without any changelog text at all :(
> 
> Sorry I created a one line commit message, which became the subject of the
> patch email. I couldn't think of anything more to add for a second line. My
> mistake. I did an ammend of that, as it was the last commit, to add a second
> line.
> 
> Now I'm probably going to get this next bit wrong. Can I simply bump the
> version of patch 8/8 to v6 and resend that single patch file, or bump all 8
> patches to v6 and resend all of them? I'll bump the last patch file and attach
> it to this email, hope for the best.

I can't take attachments, sorry.

Try resending the whole series, as a new version (v6), and all should be
fine.

Note that I can't do anything with patches until the merge window
closes, in 2 weeks, so don't expect a response until then.

thanks,

greg k-h


Re: [PATCH v5 8/8] staging: rtl8192e: remove unnecessary parentheses - Coding Style

2018-06-06 Thread Greg KH
On Wed, Jun 06, 2018 at 11:25:14AM +0100, John Whitmore wrote:
> On Sun, Jun 03, 2018 at 02:28:35PM +0200, Greg KH wrote:
> > On Sun, Jun 03, 2018 at 01:04:13PM +0100, John Whitmore wrote:
> > > Signed-off-by: John Whitmore 
> > > ---
> > >  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 12 ++--
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > I can not take patches without any changelog text at all :(
> 
> Sorry I created a one line commit message, which became the subject of the
> patch email. I couldn't think of anything more to add for a second line. My
> mistake. I did an ammend of that, as it was the last commit, to add a second
> line.
> 
> Now I'm probably going to get this next bit wrong. Can I simply bump the
> version of patch 8/8 to v6 and resend that single patch file, or bump all 8
> patches to v6 and resend all of them? I'll bump the last patch file and attach
> it to this email, hope for the best.

I can't take attachments, sorry.

Try resending the whole series, as a new version (v6), and all should be
fine.

Note that I can't do anything with patches until the merge window
closes, in 2 weeks, so don't expect a response until then.

thanks,

greg k-h


Re: [PATCH v3 3/6] mtd: rawnand: tegra: add devicetree binding

2018-06-06 Thread Boris Brezillon
Hi Thierry,

On Wed, 6 Jun 2018 12:39:03 +0200
Thierry Reding  wrote:

> On Tue, Jun 05, 2018 at 11:19:14PM +0300, Dmitry Osipenko wrote:
> > On 01.06.2018 10:30, Boris Brezillon wrote:  
> > > On Fri,  1 Jun 2018 00:16:34 +0200
> > > Stefan Agner  wrote:
> > >   
> > >> This adds the devicetree binding for the Tegra 2 NAND flash
> > >> controller.
> > >>
> > >> Signed-off-by: Lucas Stach 
> > >> Signed-off-by: Stefan Agner 
> > >> ---
> > >>  .../bindings/mtd/nvidia-tegra20-nand.txt  | 64 +++
> > >>  1 file changed, 64 insertions(+)
> > >>  create mode 100644 
> > >> Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > >>
> > >> diff --git 
> > >> a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt 
> > >> b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > >> new file mode 100644
> > >> index ..5cd984ef046b
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > >> @@ -0,0 +1,64 @@
> > >> +NVIDIA Tegra NAND Flash controller
> > >> +
> > >> +Required properties:
> > >> +- compatible: Must be one of:
> > >> +  - "nvidia,tegra20-nand"  
> > > 
> > > As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
> > > "nvidia,tegra20-nfc".
> > >   
> > >> +- reg: MMIO address range
> > >> +- interrupts: interrupt output of the NFC controller
> > >> +- clocks: Must contain an entry for each entry in clock-names.
> > >> +  See ../clocks/clock-bindings.txt for details.
> > >> +- clock-names: Must include the following entries:
> > >> +  - nand
> > >> +- resets: Must contain an entry for each entry in reset-names.
> > >> +  See ../reset/reset.txt for details.
> > >> +- reset-names: Must include the following entries:
> > >> +  - nand
> > >> +
> > >> +Optional children nodes:
> > >> +Individual NAND chips are children of the NAND controller node. 
> > >> Currently
> > >> +only one NAND chip supported.
> > >> +
> > >> +Required children node properties:
> > >> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> > >> +
> > >> +Optional children node properties:
> > >> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently 
> > >> only
> > >> + "hw" is supported.
> > >> +- nand-ecc-algo: string, algorithm of NAND ECC.
> > >> + Supported values with "hw" ECC mode are: "rs", "bch".
> > >> +- nand-bus-width : See nand.txt
> > >> +- nand-on-flash-bbt: See nand.txt
> > >> +- nand-ecc-strength: integer representing the number of bits to correct
> > >> + per ECC step (always 512). Supported strength 
> > >> using HW ECC
> > >> + modes are:
> > >> + - RS: 4, 6, 8
> > >> + - BCH: 4, 8, 14, 16
> > >> +- nand-ecc-maximize: See nand.txt
> > >> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the 
> > >> boot ROM
> > >> +   are choosen.
> > >> +- wp-gpios: GPIO specifier for the write protect pin.
> > >> +
> > >> +Optional child node of NAND chip nodes:
> > >> +Partitions: see partition.txt
> > >> +
> > >> +  Example:
> > >> +nand@70008000 {  
> > > 
> > >   nand-controller@70008000 {
> > >   
> > >> +compatible = "nvidia,tegra20-nand";  
> > > 
> > >   compatible = "nvidia,tegra20-nand-controller";
> > > 
> > > or
> > > 
> > >   compatible = "nvidia,tegra20-nfc";
> > >   
> > 
> > Maybe it's just me, but when I'm reading "nfc", my first association is the
> > "Near Field Communication". Probably an explicit
> > "nvidia,tegra20-nand-controller" variant is more preferable.  

I also prefer nvidia,tegra20-nand-controller.

> 
> We don't really use a -controller suffix for any of the other
> controllers because it is kind of implied. "nfc" is also not something
> that is ever referred to in the technical documentation.
> 
> "nvidia,tegra20-nand" would be most consistent with all the rest of
> Tegra (c.f. "nvidia,tegra*-ahci", "nvidia,tegra*-pci",
> "nvidia,tegra*-hda", "nvidia,tegra*-gmi", ...).

People get confused about what this node represents when you just have
"nvidia,tegra20-nand", and then you start seeing NAND related props or
partition nodes being defined under the NAND controller node.
I really prefer to have the "-controller" prefix here to avoid such
confusions.

Regards,

Boris


Re: [PATCH v3 3/6] mtd: rawnand: tegra: add devicetree binding

2018-06-06 Thread Boris Brezillon
Hi Thierry,

On Wed, 6 Jun 2018 12:39:03 +0200
Thierry Reding  wrote:

> On Tue, Jun 05, 2018 at 11:19:14PM +0300, Dmitry Osipenko wrote:
> > On 01.06.2018 10:30, Boris Brezillon wrote:  
> > > On Fri,  1 Jun 2018 00:16:34 +0200
> > > Stefan Agner  wrote:
> > >   
> > >> This adds the devicetree binding for the Tegra 2 NAND flash
> > >> controller.
> > >>
> > >> Signed-off-by: Lucas Stach 
> > >> Signed-off-by: Stefan Agner 
> > >> ---
> > >>  .../bindings/mtd/nvidia-tegra20-nand.txt  | 64 +++
> > >>  1 file changed, 64 insertions(+)
> > >>  create mode 100644 
> > >> Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > >>
> > >> diff --git 
> > >> a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt 
> > >> b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > >> new file mode 100644
> > >> index ..5cd984ef046b
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > >> @@ -0,0 +1,64 @@
> > >> +NVIDIA Tegra NAND Flash controller
> > >> +
> > >> +Required properties:
> > >> +- compatible: Must be one of:
> > >> +  - "nvidia,tegra20-nand"  
> > > 
> > > As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
> > > "nvidia,tegra20-nfc".
> > >   
> > >> +- reg: MMIO address range
> > >> +- interrupts: interrupt output of the NFC controller
> > >> +- clocks: Must contain an entry for each entry in clock-names.
> > >> +  See ../clocks/clock-bindings.txt for details.
> > >> +- clock-names: Must include the following entries:
> > >> +  - nand
> > >> +- resets: Must contain an entry for each entry in reset-names.
> > >> +  See ../reset/reset.txt for details.
> > >> +- reset-names: Must include the following entries:
> > >> +  - nand
> > >> +
> > >> +Optional children nodes:
> > >> +Individual NAND chips are children of the NAND controller node. 
> > >> Currently
> > >> +only one NAND chip supported.
> > >> +
> > >> +Required children node properties:
> > >> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> > >> +
> > >> +Optional children node properties:
> > >> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently 
> > >> only
> > >> + "hw" is supported.
> > >> +- nand-ecc-algo: string, algorithm of NAND ECC.
> > >> + Supported values with "hw" ECC mode are: "rs", "bch".
> > >> +- nand-bus-width : See nand.txt
> > >> +- nand-on-flash-bbt: See nand.txt
> > >> +- nand-ecc-strength: integer representing the number of bits to correct
> > >> + per ECC step (always 512). Supported strength 
> > >> using HW ECC
> > >> + modes are:
> > >> + - RS: 4, 6, 8
> > >> + - BCH: 4, 8, 14, 16
> > >> +- nand-ecc-maximize: See nand.txt
> > >> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the 
> > >> boot ROM
> > >> +   are choosen.
> > >> +- wp-gpios: GPIO specifier for the write protect pin.
> > >> +
> > >> +Optional child node of NAND chip nodes:
> > >> +Partitions: see partition.txt
> > >> +
> > >> +  Example:
> > >> +nand@70008000 {  
> > > 
> > >   nand-controller@70008000 {
> > >   
> > >> +compatible = "nvidia,tegra20-nand";  
> > > 
> > >   compatible = "nvidia,tegra20-nand-controller";
> > > 
> > > or
> > > 
> > >   compatible = "nvidia,tegra20-nfc";
> > >   
> > 
> > Maybe it's just me, but when I'm reading "nfc", my first association is the
> > "Near Field Communication". Probably an explicit
> > "nvidia,tegra20-nand-controller" variant is more preferable.  

I also prefer nvidia,tegra20-nand-controller.

> 
> We don't really use a -controller suffix for any of the other
> controllers because it is kind of implied. "nfc" is also not something
> that is ever referred to in the technical documentation.
> 
> "nvidia,tegra20-nand" would be most consistent with all the rest of
> Tegra (c.f. "nvidia,tegra*-ahci", "nvidia,tegra*-pci",
> "nvidia,tegra*-hda", "nvidia,tegra*-gmi", ...).

People get confused about what this node represents when you just have
"nvidia,tegra20-nand", and then you start seeing NAND related props or
partition nodes being defined under the NAND controller node.
I really prefer to have the "-controller" prefix here to avoid such
confusions.

Regards,

Boris


Re: [PATCH v2 0/5] Tegra20 External Memory Controller driver

2018-06-06 Thread Thierry Reding
On Mon, Jun 04, 2018 at 01:36:49AM +0300, Dmitry Osipenko wrote:
> Hello,
> 
> Couple years ago the Tegra20 EMC driver was removed from the kernel
> due to incompatible changes in the Tegra's clock driver. This patchset
> introduces a modernized EMC driver. Currently the sole purpose of the
> driver is to initialize DRAM frequency to maximum rate during of the
> kernels boot-up. Later we may consider implementing dynamic memory
> frequency scaling, utilizing functionality provided by this driver.
> 
> Changelog:
> 
> v2:
>   - Minor code cleanups like consistent use of writel_relaxed instead
> of non-relaxed version, reworded error messages, etc.
> 
>   - Factored out use_pllm_ud bit checking into a standalone patch for
> consistency.
> 
> Dmitry Osipenko (5):
>   dt: bindings: tegra20-emc: Document interrupt property
>   ARM: dts: tegra20: Add interrupt to External Memory Controller
>   clk: tegra20: Turn EMC clock gate into divider
>   clk: tegra20: Check whether direct PLLM sourcing is turned off for EMC
>   memory: tegra: Introduce Tegra20 EMC driver

I took a brief look and didn't spot any dependencies between the clk and
memory patches. Is it correct that these can be applied separately?

Thierry


signature.asc
Description: PGP signature


Re: [PATCH v2 0/5] Tegra20 External Memory Controller driver

2018-06-06 Thread Thierry Reding
On Mon, Jun 04, 2018 at 01:36:49AM +0300, Dmitry Osipenko wrote:
> Hello,
> 
> Couple years ago the Tegra20 EMC driver was removed from the kernel
> due to incompatible changes in the Tegra's clock driver. This patchset
> introduces a modernized EMC driver. Currently the sole purpose of the
> driver is to initialize DRAM frequency to maximum rate during of the
> kernels boot-up. Later we may consider implementing dynamic memory
> frequency scaling, utilizing functionality provided by this driver.
> 
> Changelog:
> 
> v2:
>   - Minor code cleanups like consistent use of writel_relaxed instead
> of non-relaxed version, reworded error messages, etc.
> 
>   - Factored out use_pllm_ud bit checking into a standalone patch for
> consistency.
> 
> Dmitry Osipenko (5):
>   dt: bindings: tegra20-emc: Document interrupt property
>   ARM: dts: tegra20: Add interrupt to External Memory Controller
>   clk: tegra20: Turn EMC clock gate into divider
>   clk: tegra20: Check whether direct PLLM sourcing is turned off for EMC
>   memory: tegra: Introduce Tegra20 EMC driver

I took a brief look and didn't spot any dependencies between the clk and
memory patches. Is it correct that these can be applied separately?

Thierry


signature.asc
Description: PGP signature


Re: [PATCH V5] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-06 Thread Viresh Kumar
On 06-06-18, 12:22, Daniel Lezcano wrote:
> (mb() are done in the atomic operations AFAICT).

AFAIU, it is required to make sure the operations are seen in a particular order
on another CPU and the compiler doesn't reorganize code to optimize it.

For example, in our case what if the compiler reorganizes the atomic-set
operation after wakeup-process ? But maybe that wouldn't happen across function
calls and we should be safe then.

> What about:
> 
>   get_online_cpus();
> 
>   nr_tasks = cpumask_weight(
>   cpumask_and(ii_dev->cpumask, cpu_online_mask);
>   
>   atomic_set(_dev->count, nr_tasks);
> 
>   for_each_cpu_and(cpu, ii_dev->cpumask, cpu_online_mask) {
>   iit = per_cpu_ptr(_injection_thread, cpu);
>   iit->should_run = 1;
>   wake_up_process(iit->tsk);
>   }
> 
>   put_online_cpus();
> ?

Looks good this time.

> I'm wondering if we can have a CPU hotplugged right after the
> 'put_online_cpus', resulting in the 'should park' flag set and then the
> thread goes in the kthread_parkme instead of jumping back the idle
> injection function and decrease the count, leading up to the timer not
> being set again.

True. That looks like a valid problem to me as well.

What about starting the hrtimer right from this routine itself, after taking
into account running-time, idle-time, delay, etc ? That would get rid of the
count stuff, this get_online_cpus(), etc.

Even if we restart the next play-idle cycle a bit early or with some delay, sky
wouldn't fall :)

-- 
viresh


Re: [PATCH V5] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-06 Thread Viresh Kumar
On 06-06-18, 12:22, Daniel Lezcano wrote:
> (mb() are done in the atomic operations AFAICT).

AFAIU, it is required to make sure the operations are seen in a particular order
on another CPU and the compiler doesn't reorganize code to optimize it.

For example, in our case what if the compiler reorganizes the atomic-set
operation after wakeup-process ? But maybe that wouldn't happen across function
calls and we should be safe then.

> What about:
> 
>   get_online_cpus();
> 
>   nr_tasks = cpumask_weight(
>   cpumask_and(ii_dev->cpumask, cpu_online_mask);
>   
>   atomic_set(_dev->count, nr_tasks);
> 
>   for_each_cpu_and(cpu, ii_dev->cpumask, cpu_online_mask) {
>   iit = per_cpu_ptr(_injection_thread, cpu);
>   iit->should_run = 1;
>   wake_up_process(iit->tsk);
>   }
> 
>   put_online_cpus();
> ?

Looks good this time.

> I'm wondering if we can have a CPU hotplugged right after the
> 'put_online_cpus', resulting in the 'should park' flag set and then the
> thread goes in the kthread_parkme instead of jumping back the idle
> injection function and decrease the count, leading up to the timer not
> being set again.

True. That looks like a valid problem to me as well.

What about starting the hrtimer right from this routine itself, after taking
into account running-time, idle-time, delay, etc ? That would get rid of the
count stuff, this get_online_cpus(), etc.

Even if we restart the next play-idle cycle a bit early or with some delay, sky
wouldn't fall :)

-- 
viresh


linux-next: build warning after merge of the y2038 tree

2018-06-06 Thread Stephen Rothwell
Hi all,

After merging the y2038 tree, today's linux-next build (x86_64
allmodconfig) produced this warning:

fs/pstore/ram.c: In function 'ramoops_read_kmsg_hdr':
fs/pstore/ram.c:39:29: warning: format '%ld' expects argument of type 'long int 
*', but argument 3 has type 'time64_t * {aka long long int *}' [-Wformat=]
 #define RAMOOPS_KERNMSG_HDR ""
 ^
fs/pstore/ram.c:167:21: note: in expansion of macro 'RAMOOPS_KERNMSG_HDR'
  if (sscanf(buffer, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n%n",
 ^~~
fs/pstore/ram.c:42:23: note: format string is defined here
 # define TVSEC_FMT "%ld"
 ~~^
 %lld
fs/pstore/ram.c:39:29: warning: format '%ld' expects argument of type 'long int 
*', but argument 3 has type 'time64_t * {aka long long int *}' [-Wformat=]
 #define RAMOOPS_KERNMSG_HDR ""
 ^
fs/pstore/ram.c:174:28: note: in expansion of macro 'RAMOOPS_KERNMSG_HDR'
  } else if (sscanf(buffer, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu\n%n",
^~~
fs/pstore/ram.c:42:23: note: format string is defined here
 # define TVSEC_FMT "%ld"
 ~~^
 %lld
fs/pstore/ram.c: In function 'ramoops_write_kmsg_hdr':
fs/pstore/ram.c:39:29: warning: format '%ld' expects argument of type 'long 
int', but argument 3 has type 'time64_t {aka long long int}' [-Wformat=]
 #define RAMOOPS_KERNMSG_HDR ""
 ^
fs/pstore/ram.c:370:30: note: in expansion of macro 'RAMOOPS_KERNMSG_HDR'
  hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n",
  ^~~
fs/pstore/ram.c:42:23: note: format string is defined here
 # define TVSEC_FMT "%ld"
 ~~^
 %lld

Introduced by commit

  0f0d83b99ef7 ("pstore: Convert internal records to timespec64")

-- 
Cheers,
Stephen Rothwell


pgpVkvLhYQEby.pgp
Description: OpenPGP digital signature


linux-next: build warning after merge of the y2038 tree

2018-06-06 Thread Stephen Rothwell
Hi all,

After merging the y2038 tree, today's linux-next build (x86_64
allmodconfig) produced this warning:

fs/pstore/ram.c: In function 'ramoops_read_kmsg_hdr':
fs/pstore/ram.c:39:29: warning: format '%ld' expects argument of type 'long int 
*', but argument 3 has type 'time64_t * {aka long long int *}' [-Wformat=]
 #define RAMOOPS_KERNMSG_HDR ""
 ^
fs/pstore/ram.c:167:21: note: in expansion of macro 'RAMOOPS_KERNMSG_HDR'
  if (sscanf(buffer, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n%n",
 ^~~
fs/pstore/ram.c:42:23: note: format string is defined here
 # define TVSEC_FMT "%ld"
 ~~^
 %lld
fs/pstore/ram.c:39:29: warning: format '%ld' expects argument of type 'long int 
*', but argument 3 has type 'time64_t * {aka long long int *}' [-Wformat=]
 #define RAMOOPS_KERNMSG_HDR ""
 ^
fs/pstore/ram.c:174:28: note: in expansion of macro 'RAMOOPS_KERNMSG_HDR'
  } else if (sscanf(buffer, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu\n%n",
^~~
fs/pstore/ram.c:42:23: note: format string is defined here
 # define TVSEC_FMT "%ld"
 ~~^
 %lld
fs/pstore/ram.c: In function 'ramoops_write_kmsg_hdr':
fs/pstore/ram.c:39:29: warning: format '%ld' expects argument of type 'long 
int', but argument 3 has type 'time64_t {aka long long int}' [-Wformat=]
 #define RAMOOPS_KERNMSG_HDR ""
 ^
fs/pstore/ram.c:370:30: note: in expansion of macro 'RAMOOPS_KERNMSG_HDR'
  hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR TVSEC_FMT ".%lu-%c\n",
  ^~~
fs/pstore/ram.c:42:23: note: format string is defined here
 # define TVSEC_FMT "%ld"
 ~~^
 %lld

Introduced by commit

  0f0d83b99ef7 ("pstore: Convert internal records to timespec64")

-- 
Cheers,
Stephen Rothwell


pgpVkvLhYQEby.pgp
Description: OpenPGP digital signature


Re: [PATCH v3 3/6] mtd: rawnand: tegra: add devicetree binding

2018-06-06 Thread Thierry Reding
On Tue, Jun 05, 2018 at 11:19:14PM +0300, Dmitry Osipenko wrote:
> On 01.06.2018 10:30, Boris Brezillon wrote:
> > On Fri,  1 Jun 2018 00:16:34 +0200
> > Stefan Agner  wrote:
> > 
> >> This adds the devicetree binding for the Tegra 2 NAND flash
> >> controller.
> >>
> >> Signed-off-by: Lucas Stach 
> >> Signed-off-by: Stefan Agner 
> >> ---
> >>  .../bindings/mtd/nvidia-tegra20-nand.txt  | 64 +++
> >>  1 file changed, 64 insertions(+)
> >>  create mode 100644 
> >> Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt 
> >> b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >> new file mode 100644
> >> index ..5cd984ef046b
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >> @@ -0,0 +1,64 @@
> >> +NVIDIA Tegra NAND Flash controller
> >> +
> >> +Required properties:
> >> +- compatible: Must be one of:
> >> +  - "nvidia,tegra20-nand"
> > 
> > As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
> > "nvidia,tegra20-nfc".
> > 
> >> +- reg: MMIO address range
> >> +- interrupts: interrupt output of the NFC controller
> >> +- clocks: Must contain an entry for each entry in clock-names.
> >> +  See ../clocks/clock-bindings.txt for details.
> >> +- clock-names: Must include the following entries:
> >> +  - nand
> >> +- resets: Must contain an entry for each entry in reset-names.
> >> +  See ../reset/reset.txt for details.
> >> +- reset-names: Must include the following entries:
> >> +  - nand
> >> +
> >> +Optional children nodes:
> >> +Individual NAND chips are children of the NAND controller node. Currently
> >> +only one NAND chip supported.
> >> +
> >> +Required children node properties:
> >> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> >> +
> >> +Optional children node properties:
> >> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently 
> >> only
> >> +   "hw" is supported.
> >> +- nand-ecc-algo: string, algorithm of NAND ECC.
> >> +   Supported values with "hw" ECC mode are: "rs", "bch".
> >> +- nand-bus-width : See nand.txt
> >> +- nand-on-flash-bbt: See nand.txt
> >> +- nand-ecc-strength: integer representing the number of bits to correct
> >> +   per ECC step (always 512). Supported strength using HW ECC
> >> +   modes are:
> >> +   - RS: 4, 6, 8
> >> +   - BCH: 4, 8, 14, 16
> >> +- nand-ecc-maximize: See nand.txt
> >> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the 
> >> boot ROM
> >> + are choosen.
> >> +- wp-gpios: GPIO specifier for the write protect pin.
> >> +
> >> +Optional child node of NAND chip nodes:
> >> +Partitions: see partition.txt
> >> +
> >> +  Example:
> >> +  nand@70008000 {
> > 
> > nand-controller@70008000 {
> > 
> >> +  compatible = "nvidia,tegra20-nand";
> > 
> > compatible = "nvidia,tegra20-nand-controller";
> > 
> > or
> > 
> > compatible = "nvidia,tegra20-nfc";
> > 
> 
> Maybe it's just me, but when I'm reading "nfc", my first association is the
> "Near Field Communication". Probably an explicit
> "nvidia,tegra20-nand-controller" variant is more preferable.

We don't really use a -controller suffix for any of the other
controllers because it is kind of implied. "nfc" is also not something
that is ever referred to in the technical documentation.

"nvidia,tegra20-nand" would be most consistent with all the rest of
Tegra (c.f. "nvidia,tegra*-ahci", "nvidia,tegra*-pci",
"nvidia,tegra*-hda", "nvidia,tegra*-gmi", ...).

Thierry


signature.asc
Description: PGP signature


Re: [PATCH v3 3/6] mtd: rawnand: tegra: add devicetree binding

2018-06-06 Thread Thierry Reding
On Tue, Jun 05, 2018 at 11:19:14PM +0300, Dmitry Osipenko wrote:
> On 01.06.2018 10:30, Boris Brezillon wrote:
> > On Fri,  1 Jun 2018 00:16:34 +0200
> > Stefan Agner  wrote:
> > 
> >> This adds the devicetree binding for the Tegra 2 NAND flash
> >> controller.
> >>
> >> Signed-off-by: Lucas Stach 
> >> Signed-off-by: Stefan Agner 
> >> ---
> >>  .../bindings/mtd/nvidia-tegra20-nand.txt  | 64 +++
> >>  1 file changed, 64 insertions(+)
> >>  create mode 100644 
> >> Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt 
> >> b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >> new file mode 100644
> >> index ..5cd984ef046b
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >> @@ -0,0 +1,64 @@
> >> +NVIDIA Tegra NAND Flash controller
> >> +
> >> +Required properties:
> >> +- compatible: Must be one of:
> >> +  - "nvidia,tegra20-nand"
> > 
> > As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
> > "nvidia,tegra20-nfc".
> > 
> >> +- reg: MMIO address range
> >> +- interrupts: interrupt output of the NFC controller
> >> +- clocks: Must contain an entry for each entry in clock-names.
> >> +  See ../clocks/clock-bindings.txt for details.
> >> +- clock-names: Must include the following entries:
> >> +  - nand
> >> +- resets: Must contain an entry for each entry in reset-names.
> >> +  See ../reset/reset.txt for details.
> >> +- reset-names: Must include the following entries:
> >> +  - nand
> >> +
> >> +Optional children nodes:
> >> +Individual NAND chips are children of the NAND controller node. Currently
> >> +only one NAND chip supported.
> >> +
> >> +Required children node properties:
> >> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> >> +
> >> +Optional children node properties:
> >> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently 
> >> only
> >> +   "hw" is supported.
> >> +- nand-ecc-algo: string, algorithm of NAND ECC.
> >> +   Supported values with "hw" ECC mode are: "rs", "bch".
> >> +- nand-bus-width : See nand.txt
> >> +- nand-on-flash-bbt: See nand.txt
> >> +- nand-ecc-strength: integer representing the number of bits to correct
> >> +   per ECC step (always 512). Supported strength using HW ECC
> >> +   modes are:
> >> +   - RS: 4, 6, 8
> >> +   - BCH: 4, 8, 14, 16
> >> +- nand-ecc-maximize: See nand.txt
> >> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the 
> >> boot ROM
> >> + are choosen.
> >> +- wp-gpios: GPIO specifier for the write protect pin.
> >> +
> >> +Optional child node of NAND chip nodes:
> >> +Partitions: see partition.txt
> >> +
> >> +  Example:
> >> +  nand@70008000 {
> > 
> > nand-controller@70008000 {
> > 
> >> +  compatible = "nvidia,tegra20-nand";
> > 
> > compatible = "nvidia,tegra20-nand-controller";
> > 
> > or
> > 
> > compatible = "nvidia,tegra20-nfc";
> > 
> 
> Maybe it's just me, but when I'm reading "nfc", my first association is the
> "Near Field Communication". Probably an explicit
> "nvidia,tegra20-nand-controller" variant is more preferable.

We don't really use a -controller suffix for any of the other
controllers because it is kind of implied. "nfc" is also not something
that is ever referred to in the technical documentation.

"nvidia,tegra20-nand" would be most consistent with all the rest of
Tegra (c.f. "nvidia,tegra*-ahci", "nvidia,tegra*-pci",
"nvidia,tegra*-hda", "nvidia,tegra*-gmi", ...).

Thierry


signature.asc
Description: PGP signature


Re: [PATCH 2/2] sched/fair: util_est: add running_sum tracking

2018-06-06 Thread Patrick Bellasi
Hi Vincent,

On 06-Jun 10:26, Vincent Guittot wrote:

[...]

> For the above 2 tasks of the example example we have the pattern
> 
> Task 1
> state   SSESSESS
> util_avgDD DD DD
> 
> Task 2
> state   SSESSESS
> util_avgDD DD DD
> running_avg DDCDDCDD
> 
> R : Running 1ms, S: Sleep 1ms , W: Wait 1ms, E: Enqueue event 
> A: Accumulate 1ms, D: Decay 1ms, C: Copy util_avg value
> 
> the util_avg of T1 and T2 have the same pattern which is:
>   DDDDDD
> and as a result, the same value which represents their utilization
> 
> For the running_avg of T2, there is only 2 decays aftert the last running
> phase and before the next enqueue
> so the pattern will be DD
> instead of the DD
> 
> the runninh_avg will report a higher value than reality

Right!... Your example above is really useful, thanks.

Reasoning on the same line, we can easily see that a 50% CFS task
co-scheduled with a 50% RT task, which delays the CFS one and has the
same period,  will make the CFS task appear as a 100% task.

Which is definitively not what we want to sample as estimated
utilization.

The good news, if we like, is instead that util_avg is already
correctly reporting 50% at the end of each activation and thus, when
we collect util_est samples we already have the best utilization value
we can collect for a task.

The only time we collect "wrong" estimation samples is when there
is not idle time, thus eventually util_est should be improved by
discarding samples in that cases... but I'm not entirely sure if and
how we can detect them.

Or just to ensure we have idle time... as you are proposing in
the other thread.

Thanks again for pointing out the issue above.

-- 
#include 

Patrick Bellasi


Re: [PATCH 2/2] powerpc: Add support for function error injection

2018-06-06 Thread Naveen N. Rao

Naveen N. Rao wrote:

Michael Ellerman wrote:


I guess if it doesn't already apply to tip you should rebase it. You've
probably missed 4.18 anyway.


Oh ok. I just tried and it seems to apply just fine. I'll post v2 after 
giving this a quick test.


I didn't post a v2 since I have decided against using this approach. The 
reason for that is Masami's series to remove jprobes. The discussion 
there reminded me that we can't easily override functions with kprobes 
on powerpc. Though it works for this particular scenario, we would just 
be setting a bad example.


As such, I won't be changing generic code, but will simply make the 
necessary changes in powerpc code.


Sorry for the noise.

- Naveen




Re: [PATCH 2/2] sched/fair: util_est: add running_sum tracking

2018-06-06 Thread Patrick Bellasi
Hi Vincent,

On 06-Jun 10:26, Vincent Guittot wrote:

[...]

> For the above 2 tasks of the example example we have the pattern
> 
> Task 1
> state   SSESSESS
> util_avgDD DD DD
> 
> Task 2
> state   SSESSESS
> util_avgDD DD DD
> running_avg DDCDDCDD
> 
> R : Running 1ms, S: Sleep 1ms , W: Wait 1ms, E: Enqueue event 
> A: Accumulate 1ms, D: Decay 1ms, C: Copy util_avg value
> 
> the util_avg of T1 and T2 have the same pattern which is:
>   DDDDDD
> and as a result, the same value which represents their utilization
> 
> For the running_avg of T2, there is only 2 decays aftert the last running
> phase and before the next enqueue
> so the pattern will be DD
> instead of the DD
> 
> the runninh_avg will report a higher value than reality

Right!... Your example above is really useful, thanks.

Reasoning on the same line, we can easily see that a 50% CFS task
co-scheduled with a 50% RT task, which delays the CFS one and has the
same period,  will make the CFS task appear as a 100% task.

Which is definitively not what we want to sample as estimated
utilization.

The good news, if we like, is instead that util_avg is already
correctly reporting 50% at the end of each activation and thus, when
we collect util_est samples we already have the best utilization value
we can collect for a task.

The only time we collect "wrong" estimation samples is when there
is not idle time, thus eventually util_est should be improved by
discarding samples in that cases... but I'm not entirely sure if and
how we can detect them.

Or just to ensure we have idle time... as you are proposing in
the other thread.

Thanks again for pointing out the issue above.

-- 
#include 

Patrick Bellasi


Re: [PATCH 2/2] powerpc: Add support for function error injection

2018-06-06 Thread Naveen N. Rao

Naveen N. Rao wrote:

Michael Ellerman wrote:


I guess if it doesn't already apply to tip you should rebase it. You've
probably missed 4.18 anyway.


Oh ok. I just tried and it seems to apply just fine. I'll post v2 after 
giving this a quick test.


I didn't post a v2 since I have decided against using this approach. The 
reason for that is Masami's series to remove jprobes. The discussion 
there reminded me that we can't easily override functions with kprobes 
on powerpc. Though it works for this particular scenario, we would just 
be setting a bad example.


As such, I won't be changing generic code, but will simply make the 
necessary changes in powerpc code.


Sorry for the noise.

- Naveen




Re: [PATCH] x86/iommu: Fix a typo in a macro parameter

2018-06-06 Thread Thomas Gleixner
On Wed, 6 Jun 2018, Masatake YAMATO wrote:

Can you please explain why 0 is the wrong value. That's not a typo, that's
a functional change and both the subject line and the changelog itself
should be explanatory.

Thanks,

tglx

> Signed-off-by: Masatake YAMATO 
> ---
>  arch/x86/include/asm/iommu_table.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/iommu_table.h 
> b/arch/x86/include/asm/iommu_table.h
> index 1fb3fd1a83c2..2a0d5f7d1ed1 100644
> --- a/arch/x86/include/asm/iommu_table.h
> +++ b/arch/x86/include/asm/iommu_table.h
> @@ -66,7 +66,7 @@ struct iommu_table_entry {
>  #define IOMMU_INIT_POST(_detect) \
>   __IOMMU_INIT(_detect, pci_swiotlb_detect_4gb,  NULL, NULL, 0)
>  
> -#define IOMMU_INIT_POST_FINISH(detect)   
> \
> +#define IOMMU_INIT_POST_FINISH(_detect)  
> \
>   __IOMMU_INIT(_detect, pci_swiotlb_detect_4gb,  NULL, NULL, 1)
>  
>  /*
> -- 
> 2.17.0
> 
> 


Re: [PATCH] x86/iommu: Fix a typo in a macro parameter

2018-06-06 Thread Thomas Gleixner
On Wed, 6 Jun 2018, Masatake YAMATO wrote:

Can you please explain why 0 is the wrong value. That's not a typo, that's
a functional change and both the subject line and the changelog itself
should be explanatory.

Thanks,

tglx

> Signed-off-by: Masatake YAMATO 
> ---
>  arch/x86/include/asm/iommu_table.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/iommu_table.h 
> b/arch/x86/include/asm/iommu_table.h
> index 1fb3fd1a83c2..2a0d5f7d1ed1 100644
> --- a/arch/x86/include/asm/iommu_table.h
> +++ b/arch/x86/include/asm/iommu_table.h
> @@ -66,7 +66,7 @@ struct iommu_table_entry {
>  #define IOMMU_INIT_POST(_detect) \
>   __IOMMU_INIT(_detect, pci_swiotlb_detect_4gb,  NULL, NULL, 0)
>  
> -#define IOMMU_INIT_POST_FINISH(detect)   
> \
> +#define IOMMU_INIT_POST_FINISH(_detect)  
> \
>   __IOMMU_INIT(_detect, pci_swiotlb_detect_4gb,  NULL, NULL, 1)
>  
>  /*
> -- 
> 2.17.0
> 
> 


Re: [PATCH] printk/nmi: Prevent deadlock when serializing NMI backtraces

2018-06-06 Thread Sergey Senozhatsky
On (06/06/18 14:10), Sergey Senozhatsky wrote:
> On (06/05/18 14:47), Petr Mladek wrote:
> [..]
> > Grr, the ABBA deadlock is still there. NMIs are not sent to the other
> > CPUs atomically. Even if we detect that logbuf_lock is available
> > in printk_nmi_enter() on some CPUs, it might still get locked on
> > another CPU before the other CPU gets NMI.
> 
> Can we do something about "B"? :) I mean - any chance we can rework
> locking in nmi_cpu_backtrace()?

Sorry, I don't have that much free time at the moment, so can't
fully concentrate on this issue.

Here is a quick-n-dirty thought.

The whole idea of printk_nmi() was to reduce the number of locks
we take performing printk() from NMI context - ideally down to 0.
We added logbuf spin_lock later on. One lock was fine. But then
we added another one - nmi_cpu_backtrace() lock. And two locks is
too many. So can we drop the nmi_cpu_backtrace() lock, and in
exchange extend printk-safe API with functions that will disable/enable
PRINTK_NMI_DEFERRED_CONTEXT_MASK on a particular CPU?

I refer to it as HARD and SOFT printk_nmi :) Just for fun. Hard one
has no right to use logbuf and will use only per-CPU buffer, while
soft one can use either per-CPU buffer or logbuf.

---

diff --git a/lib/nmi_backtrace.c b/lib/nmi_backtrace.c
index 0ace3c907290..b57d5daa90b5 100644
--- a/lib/nmi_backtrace.c
+++ b/lib/nmi_backtrace.c
@@ -90,11 +90,10 @@ void nmi_trigger_cpumask_backtrace(const cpumask_t *mask,
 
 bool nmi_cpu_backtrace(struct pt_regs *regs)
 {
-   static arch_spinlock_t lock = __ARCH_SPIN_LOCK_UNLOCKED;
int cpu = smp_processor_id();
 
if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
-   arch_spin_lock();
+   printk_nmi_hard();
if (regs && cpu_in_idle(instruction_pointer(regs))) {
pr_warn("NMI backtrace for cpu %d skipped: idling at 
%pS\n",
cpu, (void *)instruction_pointer(regs));
@@ -105,7 +104,7 @@ bool nmi_cpu_backtrace(struct pt_regs *regs)
else
dump_stack();
}
-   arch_spin_unlock();
+   printk_nmi_sort();
cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
return true;
}


Re: [PATCH] printk/nmi: Prevent deadlock when serializing NMI backtraces

2018-06-06 Thread Sergey Senozhatsky
On (06/06/18 14:10), Sergey Senozhatsky wrote:
> On (06/05/18 14:47), Petr Mladek wrote:
> [..]
> > Grr, the ABBA deadlock is still there. NMIs are not sent to the other
> > CPUs atomically. Even if we detect that logbuf_lock is available
> > in printk_nmi_enter() on some CPUs, it might still get locked on
> > another CPU before the other CPU gets NMI.
> 
> Can we do something about "B"? :) I mean - any chance we can rework
> locking in nmi_cpu_backtrace()?

Sorry, I don't have that much free time at the moment, so can't
fully concentrate on this issue.

Here is a quick-n-dirty thought.

The whole idea of printk_nmi() was to reduce the number of locks
we take performing printk() from NMI context - ideally down to 0.
We added logbuf spin_lock later on. One lock was fine. But then
we added another one - nmi_cpu_backtrace() lock. And two locks is
too many. So can we drop the nmi_cpu_backtrace() lock, and in
exchange extend printk-safe API with functions that will disable/enable
PRINTK_NMI_DEFERRED_CONTEXT_MASK on a particular CPU?

I refer to it as HARD and SOFT printk_nmi :) Just for fun. Hard one
has no right to use logbuf and will use only per-CPU buffer, while
soft one can use either per-CPU buffer or logbuf.

---

diff --git a/lib/nmi_backtrace.c b/lib/nmi_backtrace.c
index 0ace3c907290..b57d5daa90b5 100644
--- a/lib/nmi_backtrace.c
+++ b/lib/nmi_backtrace.c
@@ -90,11 +90,10 @@ void nmi_trigger_cpumask_backtrace(const cpumask_t *mask,
 
 bool nmi_cpu_backtrace(struct pt_regs *regs)
 {
-   static arch_spinlock_t lock = __ARCH_SPIN_LOCK_UNLOCKED;
int cpu = smp_processor_id();
 
if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
-   arch_spin_lock();
+   printk_nmi_hard();
if (regs && cpu_in_idle(instruction_pointer(regs))) {
pr_warn("NMI backtrace for cpu %d skipped: idling at 
%pS\n",
cpu, (void *)instruction_pointer(regs));
@@ -105,7 +104,7 @@ bool nmi_cpu_backtrace(struct pt_regs *regs)
else
dump_stack();
}
-   arch_spin_unlock();
+   printk_nmi_sort();
cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
return true;
}


[PATCH] ASoC: ak4458: make structure soc_codec_dev_ak4458 static const

2018-06-06 Thread Colin King
From: Colin Ian King 

The structure soc_codec_dev_ak4458 is local to the source and do not
need to be in global scope and can be const, make it static const.

Cleans up sparse warnings:
warning: symbol 'soc_codec_dev_ak4458' was not declared. Should it
be static?

Signed-off-by: Colin Ian King 
---
 sound/soc/codecs/ak4458.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/ak4458.c b/sound/soc/codecs/ak4458.c
index 31ec0ba2e639..299ada4dfaa0 100644
--- a/sound/soc/codecs/ak4458.c
+++ b/sound/soc/codecs/ak4458.c
@@ -558,7 +558,7 @@ static int __maybe_unused ak4458_runtime_resume(struct 
device *dev)
 }
 #endif /* CONFIG_PM */
 
-struct snd_soc_component_driver soc_codec_dev_ak4458 = {
+static const struct snd_soc_component_driver soc_codec_dev_ak4458 = {
.probe  = ak4458_probe,
.remove = ak4458_remove,
.controls   = ak4458_snd_controls,
-- 
2.17.0



[PATCH] ASoC: ak4458: make structure soc_codec_dev_ak4458 static const

2018-06-06 Thread Colin King
From: Colin Ian King 

The structure soc_codec_dev_ak4458 is local to the source and do not
need to be in global scope and can be const, make it static const.

Cleans up sparse warnings:
warning: symbol 'soc_codec_dev_ak4458' was not declared. Should it
be static?

Signed-off-by: Colin Ian King 
---
 sound/soc/codecs/ak4458.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/ak4458.c b/sound/soc/codecs/ak4458.c
index 31ec0ba2e639..299ada4dfaa0 100644
--- a/sound/soc/codecs/ak4458.c
+++ b/sound/soc/codecs/ak4458.c
@@ -558,7 +558,7 @@ static int __maybe_unused ak4458_runtime_resume(struct 
device *dev)
 }
 #endif /* CONFIG_PM */
 
-struct snd_soc_component_driver soc_codec_dev_ak4458 = {
+static const struct snd_soc_component_driver soc_codec_dev_ak4458 = {
.probe  = ak4458_probe,
.remove = ak4458_remove,
.controls   = ak4458_snd_controls,
-- 
2.17.0



Re: [PATCH] x86/spectre_v1: Disable compiler optimizations over array_index_mask_nospec()

2018-06-06 Thread Thomas Gleixner
Dan,

On Wed, 30 May 2018, Mark Rutland wrote:
> > diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
> > index 042b5e892ed1..41f7435c84a7 100644
> > --- a/arch/x86/include/asm/barrier.h
> > +++ b/arch/x86/include/asm/barrier.h
> > @@ -38,10 +38,11 @@ static inline unsigned long 
> > array_index_mask_nospec(unsigned long index,
> >  {
> > unsigned long mask;
> >  
> > -   asm ("cmp %1,%2; sbb %0,%0;"
> > +   asm volatile ("cmp %1,%2; sbb %0,%0;"
> > :"=r" (mask)
> > :"g"(size),"r" (index)
> > :"cc");
> > +   barrier();
> > return mask;
> >  }
> 
> What does the barrier() prevent?
> 
> I don't think that inhibits the insertion of branches, and AFAIK the volatile
> is sufficient to prevent elision of identical array_idx_nospec() calls.
> 
> I don't have an objection to it, regardless.
> 
> So long as the example is updated in the commit message, feel free to add:

Any update on this?

Thanks,

tglx


Re: [PATCH] x86/spectre_v1: Disable compiler optimizations over array_index_mask_nospec()

2018-06-06 Thread Thomas Gleixner
Dan,

On Wed, 30 May 2018, Mark Rutland wrote:
> > diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
> > index 042b5e892ed1..41f7435c84a7 100644
> > --- a/arch/x86/include/asm/barrier.h
> > +++ b/arch/x86/include/asm/barrier.h
> > @@ -38,10 +38,11 @@ static inline unsigned long 
> > array_index_mask_nospec(unsigned long index,
> >  {
> > unsigned long mask;
> >  
> > -   asm ("cmp %1,%2; sbb %0,%0;"
> > +   asm volatile ("cmp %1,%2; sbb %0,%0;"
> > :"=r" (mask)
> > :"g"(size),"r" (index)
> > :"cc");
> > +   barrier();
> > return mask;
> >  }
> 
> What does the barrier() prevent?
> 
> I don't think that inhibits the insertion of branches, and AFAIK the volatile
> is sufficient to prevent elision of identical array_idx_nospec() calls.
> 
> I don't have an objection to it, regardless.
> 
> So long as the example is updated in the commit message, feel free to add:

Any update on this?

Thanks,

tglx


Re: [PATCH v5 8/8] staging: rtl8192e: remove unnecessary parentheses - Coding Style

2018-06-06 Thread John Whitmore
On Sun, Jun 03, 2018 at 02:28:35PM +0200, Greg KH wrote:
> On Sun, Jun 03, 2018 at 01:04:13PM +0100, John Whitmore wrote:
> > Signed-off-by: John Whitmore 
> > ---
> >  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 12 ++--
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> I can not take patches without any changelog text at all :(

Sorry I created a one line commit message, which became the subject of the
patch email. I couldn't think of anything more to add for a second line. My
mistake. I did an ammend of that, as it was the last commit, to add a second
line.

Now I'm probably going to get this next bit wrong. Can I simply bump the
version of patch 8/8 to v6 and resend that single patch file, or bump all 8
patches to v6 and resend all of them? I'll bump the last patch file and attach
it to this email, hope for the best.
>From 4d01cfb7d18e507b163a25a69938f323cd61e9fe Mon Sep 17 00:00:00 2001
From: John Whitmore 
Date: Sun, 3 Jun 2018 12:54:22 +0100
Subject: [PATCH v6 8/8] staging: rtl8192e: remove unnecessary parentheses -
 Coding Style

Remove unneccessary parentheses - Coding Style change

Signed-off-by: John Whitmore 
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 9cca4a8f1cf5..fad740309558 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -529,7 +529,7 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
 		u8	EWC11NHTCap[] = {0x00, 0x90, 0x4c, 0x33};	// For 11n EWC definition, 2007.07.17, by Emily
 
 		memcpy(posHTCap, EWC11NHTCap, sizeof(EWC11NHTCap));
-		pCapELE = (PHT_CAPABILITY_ELE)&(posHTCap[4]);
+		pCapELE = (PHT_CAPABILITY_ELE)[4];
 	} else {
 		pCapELE = (PHT_CAPABILITY_ELE)posHTCap;
 	}
@@ -1072,10 +1072,10 @@ void HTInitializeHTInfo(struct ieee80211_device *ieee)
 	pHTInfo->CurrentAMPDUFactor = pHTInfo->AMPDU_Factor;
 
 	// Initialize all of the parameters related to 11n
-	memset((void *)(&(pHTInfo->SelfHTCap)), 0, sizeof(pHTInfo->SelfHTCap));
-	memset((void *)(&(pHTInfo->SelfHTInfo)), 0, sizeof(pHTInfo->SelfHTInfo));
-	memset((void *)(&(pHTInfo->PeerHTCapBuf)), 0, sizeof(pHTInfo->PeerHTCapBuf));
-	memset((void *)(&(pHTInfo->PeerHTInfoBuf)), 0, sizeof(pHTInfo->PeerHTInfoBuf));
+	memset((void *)(>SelfHTCap), 0, sizeof(pHTInfo->SelfHTCap));
+	memset((void *)(>SelfHTInfo), 0, sizeof(pHTInfo->SelfHTInfo));
+	memset((void *)(>PeerHTCapBuf), 0, sizeof(pHTInfo->PeerHTCapBuf));
+	memset((void *)(>PeerHTInfoBuf), 0, sizeof(pHTInfo->PeerHTInfoBuf));
 
 	pHTInfo->bSwBwInProgress = false;
 	pHTInfo->ChnlOp = CHNLOP_NONE;
@@ -1091,7 +1091,7 @@ void HTInitializeHTInfo(struct ieee80211_device *ieee)
 
 	//MCS rate initialized here
 	{
-		u8 *RegHTSuppRateSets = &(ieee->RegHTSuppRateSet[0]);
+		u8 *RegHTSuppRateSets = >RegHTSuppRateSet[0];
 
 		RegHTSuppRateSets[0] = 0xFF;	//support MCS 0~7
 		RegHTSuppRateSets[1] = 0xFF;	//support MCS 8~15
-- 
2.17.0



Re: [PATCH v5 8/8] staging: rtl8192e: remove unnecessary parentheses - Coding Style

2018-06-06 Thread John Whitmore
On Sun, Jun 03, 2018 at 02:28:35PM +0200, Greg KH wrote:
> On Sun, Jun 03, 2018 at 01:04:13PM +0100, John Whitmore wrote:
> > Signed-off-by: John Whitmore 
> > ---
> >  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 12 ++--
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> I can not take patches without any changelog text at all :(

Sorry I created a one line commit message, which became the subject of the
patch email. I couldn't think of anything more to add for a second line. My
mistake. I did an ammend of that, as it was the last commit, to add a second
line.

Now I'm probably going to get this next bit wrong. Can I simply bump the
version of patch 8/8 to v6 and resend that single patch file, or bump all 8
patches to v6 and resend all of them? I'll bump the last patch file and attach
it to this email, hope for the best.
>From 4d01cfb7d18e507b163a25a69938f323cd61e9fe Mon Sep 17 00:00:00 2001
From: John Whitmore 
Date: Sun, 3 Jun 2018 12:54:22 +0100
Subject: [PATCH v6 8/8] staging: rtl8192e: remove unnecessary parentheses -
 Coding Style

Remove unneccessary parentheses - Coding Style change

Signed-off-by: John Whitmore 
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 9cca4a8f1cf5..fad740309558 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -529,7 +529,7 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
 		u8	EWC11NHTCap[] = {0x00, 0x90, 0x4c, 0x33};	// For 11n EWC definition, 2007.07.17, by Emily
 
 		memcpy(posHTCap, EWC11NHTCap, sizeof(EWC11NHTCap));
-		pCapELE = (PHT_CAPABILITY_ELE)&(posHTCap[4]);
+		pCapELE = (PHT_CAPABILITY_ELE)[4];
 	} else {
 		pCapELE = (PHT_CAPABILITY_ELE)posHTCap;
 	}
@@ -1072,10 +1072,10 @@ void HTInitializeHTInfo(struct ieee80211_device *ieee)
 	pHTInfo->CurrentAMPDUFactor = pHTInfo->AMPDU_Factor;
 
 	// Initialize all of the parameters related to 11n
-	memset((void *)(&(pHTInfo->SelfHTCap)), 0, sizeof(pHTInfo->SelfHTCap));
-	memset((void *)(&(pHTInfo->SelfHTInfo)), 0, sizeof(pHTInfo->SelfHTInfo));
-	memset((void *)(&(pHTInfo->PeerHTCapBuf)), 0, sizeof(pHTInfo->PeerHTCapBuf));
-	memset((void *)(&(pHTInfo->PeerHTInfoBuf)), 0, sizeof(pHTInfo->PeerHTInfoBuf));
+	memset((void *)(>SelfHTCap), 0, sizeof(pHTInfo->SelfHTCap));
+	memset((void *)(>SelfHTInfo), 0, sizeof(pHTInfo->SelfHTInfo));
+	memset((void *)(>PeerHTCapBuf), 0, sizeof(pHTInfo->PeerHTCapBuf));
+	memset((void *)(>PeerHTInfoBuf), 0, sizeof(pHTInfo->PeerHTInfoBuf));
 
 	pHTInfo->bSwBwInProgress = false;
 	pHTInfo->ChnlOp = CHNLOP_NONE;
@@ -1091,7 +1091,7 @@ void HTInitializeHTInfo(struct ieee80211_device *ieee)
 
 	//MCS rate initialized here
 	{
-		u8 *RegHTSuppRateSets = &(ieee->RegHTSuppRateSet[0]);
+		u8 *RegHTSuppRateSets = >RegHTSuppRateSet[0];
 
 		RegHTSuppRateSets[0] = 0xFF;	//support MCS 0~7
 		RegHTSuppRateSets[1] = 0xFF;	//support MCS 8~15
-- 
2.17.0



Re: [PATCH] ASoC: ak5558: make two structures static

2018-06-06 Thread Daniel Baluta
On Mi, 2018-06-06 at 10:57 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> The structure ak5558_pm and soc_codec_dev_ak5558 are local to the
> source and do not need to be in global scope, so make them static.
> Also make soc_codec_dev_ak5558 static.
> 
> Cleans up sparse warnings:
> warning: symbol 'ak5558_pm' was not declared. Should it be static?
> warning: symbol 'soc_codec_dev_ak5558' was not declared. Should it be
> static?
> 
> Signed-off-by: Colin Ian King 

Reviewed-by: Daniel Baluta 


Re: [PATCH] ASoC: ak5558: make two structures static

2018-06-06 Thread Daniel Baluta
On Mi, 2018-06-06 at 10:57 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> The structure ak5558_pm and soc_codec_dev_ak5558 are local to the
> source and do not need to be in global scope, so make them static.
> Also make soc_codec_dev_ak5558 static.
> 
> Cleans up sparse warnings:
> warning: symbol 'ak5558_pm' was not declared. Should it be static?
> warning: symbol 'soc_codec_dev_ak5558' was not declared. Should it be
> static?
> 
> Signed-off-by: Colin Ian King 

Reviewed-by: Daniel Baluta 


RE: [upstream-release] [PATCH] irqchip/ls-scfg-msi: map MSIs in the iommu

2018-06-06 Thread Bharat Bhushan



> -Original Message-
> From: upstream-release-boun...@linux.freescale.net [mailto:upstream-
> release-boun...@linux.freescale.net] On Behalf Of Laurentiu Tudor
> Sent: Tuesday, June 5, 2018 5:57 PM
> To: t...@linutronix.de; ja...@lakedaemon.net; marc.zyng...@arm.com; linux-
> ker...@vger.kernel.org
> Cc: M.h. Lian ; Z.q. Hou ;
> Laurentiu Tudor 
> Subject: [upstream-release] [PATCH] irqchip/ls-scfg-msi: map MSIs in the iommu
> 
> Add the required iommu_dma_map_msi_msg() when composing the MSI
> message, otherwise the interrupts will not work.
> 
> Signed-off-by: Laurentiu Tudor 

Reviewed-by: Bharat Bhushan 

Thanks
-Bharat
> ---
>  drivers/irqchip/irq-ls-scfg-msi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-ls-scfg-msi.c 
> b/drivers/irqchip/irq-ls-scfg-msi.c
> index 57e3d900f19e..1ec3bfe56693 100644
> --- a/drivers/irqchip/irq-ls-scfg-msi.c
> +++ b/drivers/irqchip/irq-ls-scfg-msi.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #define MSI_IRQS_PER_MSIR32
>  #define MSI_MSIR_OFFSET  4
> @@ -94,6 +95,8 @@ static void ls_scfg_msi_compose_msg(struct irq_data
> *data, struct msi_msg *msg)
> 
>   if (msi_affinity_flag)
>   msg->data |= cpumask_first(data->common->affinity);
> +
> + iommu_dma_map_msi_msg(data->irq, msg);
>  }
> 
>  static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,
> --
> 2.17.0
> 
> ___
> upstream-release mailing list
> upstream-rele...@linux.freescale.net
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flinux.fr
> eescale.net%2Fmailman%2Flistinfo%2Fupstream-
> release=02%7C01%7Cbharat.bhushan%40nxp.com%7Cba9303002bf142d9
> b63308d5cadfc179%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> 6637984747938689=3Hg2OVpp%2Bhfdj0qbCFLWLwBn0RFoNhlPPdmxGf
> AspyA%3D=0


RE: [upstream-release] [PATCH] irqchip/ls-scfg-msi: map MSIs in the iommu

2018-06-06 Thread Bharat Bhushan



> -Original Message-
> From: upstream-release-boun...@linux.freescale.net [mailto:upstream-
> release-boun...@linux.freescale.net] On Behalf Of Laurentiu Tudor
> Sent: Tuesday, June 5, 2018 5:57 PM
> To: t...@linutronix.de; ja...@lakedaemon.net; marc.zyng...@arm.com; linux-
> ker...@vger.kernel.org
> Cc: M.h. Lian ; Z.q. Hou ;
> Laurentiu Tudor 
> Subject: [upstream-release] [PATCH] irqchip/ls-scfg-msi: map MSIs in the iommu
> 
> Add the required iommu_dma_map_msi_msg() when composing the MSI
> message, otherwise the interrupts will not work.
> 
> Signed-off-by: Laurentiu Tudor 

Reviewed-by: Bharat Bhushan 

Thanks
-Bharat
> ---
>  drivers/irqchip/irq-ls-scfg-msi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-ls-scfg-msi.c 
> b/drivers/irqchip/irq-ls-scfg-msi.c
> index 57e3d900f19e..1ec3bfe56693 100644
> --- a/drivers/irqchip/irq-ls-scfg-msi.c
> +++ b/drivers/irqchip/irq-ls-scfg-msi.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #define MSI_IRQS_PER_MSIR32
>  #define MSI_MSIR_OFFSET  4
> @@ -94,6 +95,8 @@ static void ls_scfg_msi_compose_msg(struct irq_data
> *data, struct msi_msg *msg)
> 
>   if (msi_affinity_flag)
>   msg->data |= cpumask_first(data->common->affinity);
> +
> + iommu_dma_map_msi_msg(data->irq, msg);
>  }
> 
>  static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,
> --
> 2.17.0
> 
> ___
> upstream-release mailing list
> upstream-rele...@linux.freescale.net
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flinux.fr
> eescale.net%2Fmailman%2Flistinfo%2Fupstream-
> release=02%7C01%7Cbharat.bhushan%40nxp.com%7Cba9303002bf142d9
> b63308d5cadfc179%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> 6637984747938689=3Hg2OVpp%2Bhfdj0qbCFLWLwBn0RFoNhlPPdmxGf
> AspyA%3D=0


Re: [PATCH V5] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-06 Thread Daniel Lezcano
On 06/06/2018 06:27, Viresh Kumar wrote:
> On 05-06-18, 16:54, Daniel Lezcano wrote:
>> On 05/06/2018 12:39, Viresh Kumar wrote:
>> I don't think you are doing a mistake. Even if this can happen
>> theoretically, I don't think practically that is the case.
>>
>> The play_idle() has 1ms minimum sleep time.
>>
>> The scenario you are describing means:
>>
>> 1. the loop in idle_injection_wakeup() takes more than 1ms to achieve
> 
> There are many ways in which idle_injection_wakeup() can get called.
> 
> - from hrtimer handler, this happens in softirq context, right? So interrupts
>   can still block the handler to run ?
> 
> - from idle_injection_start(), process context. RT or DL or IRQ activity can
>   block the CPU for long durations sometimes.
> 
>> 2. at the same time, the user of the idle injection unregisters while
>> the idle injection is acting precisely at CPU0 and exits before another
>> task was wakeup by the loop in 1. more than 1ms after.
>>
>> >From my POV, this scenario can't happen.
> 
> Maybe something else needs to be buggy as well to make this crap happen.
> 
>> Anyway, we must write rock solid code
> 
> That's my point.
> 
>> so may be we can use a refcount to
>> protect against that, so instead of freeing in unregister, we refput the
>> ii_dev pointer.
> 
> I think the solution can be a simple change in implementation of
> idle_injection_wakeup(), something like this..
> 
> +static void idle_injection_wakeup(struct idle_injection_device *ii_dev)
> +{
> + struct idle_injection_thread *iit;
> + int cpu;
> +
> + for_each_cpu_and(cpu, ii_dev->cpumask, cpu_online_mask)
> + atomic_inc(_dev->count);
>
> +
> +   mb(); //I am not sure but I think we need some kind of barrier here ?

(mb() are done in the atomic operations AFAICT).

What about:

get_online_cpus();

nr_tasks = cpumask_weight(
cpumask_and(ii_dev->cpumask, cpu_online_mask);

atomic_set(_dev->count, nr_tasks);

for_each_cpu_and(cpu, ii_dev->cpumask, cpu_online_mask) {
iit = per_cpu_ptr(_injection_thread, cpu);
iit->should_run = 1;
wake_up_process(iit->tsk);
}

put_online_cpus();
?

I'm wondering if we can have a CPU hotplugged right after the
'put_online_cpus', resulting in the 'should park' flag set and then the
thread goes in the kthread_parkme instead of jumping back the idle
injection function and decrease the count, leading up to the timer not
being set again.

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog



Re: [PATCH V5] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-06 Thread Daniel Lezcano
On 06/06/2018 06:27, Viresh Kumar wrote:
> On 05-06-18, 16:54, Daniel Lezcano wrote:
>> On 05/06/2018 12:39, Viresh Kumar wrote:
>> I don't think you are doing a mistake. Even if this can happen
>> theoretically, I don't think practically that is the case.
>>
>> The play_idle() has 1ms minimum sleep time.
>>
>> The scenario you are describing means:
>>
>> 1. the loop in idle_injection_wakeup() takes more than 1ms to achieve
> 
> There are many ways in which idle_injection_wakeup() can get called.
> 
> - from hrtimer handler, this happens in softirq context, right? So interrupts
>   can still block the handler to run ?
> 
> - from idle_injection_start(), process context. RT or DL or IRQ activity can
>   block the CPU for long durations sometimes.
> 
>> 2. at the same time, the user of the idle injection unregisters while
>> the idle injection is acting precisely at CPU0 and exits before another
>> task was wakeup by the loop in 1. more than 1ms after.
>>
>> >From my POV, this scenario can't happen.
> 
> Maybe something else needs to be buggy as well to make this crap happen.
> 
>> Anyway, we must write rock solid code
> 
> That's my point.
> 
>> so may be we can use a refcount to
>> protect against that, so instead of freeing in unregister, we refput the
>> ii_dev pointer.
> 
> I think the solution can be a simple change in implementation of
> idle_injection_wakeup(), something like this..
> 
> +static void idle_injection_wakeup(struct idle_injection_device *ii_dev)
> +{
> + struct idle_injection_thread *iit;
> + int cpu;
> +
> + for_each_cpu_and(cpu, ii_dev->cpumask, cpu_online_mask)
> + atomic_inc(_dev->count);
>
> +
> +   mb(); //I am not sure but I think we need some kind of barrier here ?

(mb() are done in the atomic operations AFAICT).

What about:

get_online_cpus();

nr_tasks = cpumask_weight(
cpumask_and(ii_dev->cpumask, cpu_online_mask);

atomic_set(_dev->count, nr_tasks);

for_each_cpu_and(cpu, ii_dev->cpumask, cpu_online_mask) {
iit = per_cpu_ptr(_injection_thread, cpu);
iit->should_run = 1;
wake_up_process(iit->tsk);
}

put_online_cpus();
?

I'm wondering if we can have a CPU hotplugged right after the
'put_online_cpus', resulting in the 'should park' flag set and then the
thread goes in the kthread_parkme instead of jumping back the idle
injection function and decrease the count, leading up to the timer not
being set again.

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog



linux-next: manual merge of the y2038 tree with the staging tree

2018-06-06 Thread Stephen Rothwell
Hi all,

Today's linux-next merge of the y2038 tree got conflicts in:

  drivers/staging/lustre/lustre/llite/llite_lib.c
  drivers/staging/lustre/lustre/llite/namei.c
  drivers/staging/lustre/lustre/lmv/lmv_obd.c
  drivers/staging/lustre/lustre/mdc/mdc_reint.c
  drivers/staging/lustre/lustre/obdclass/obdo.c

between commit:

  be65f9ed267f ("staging: lustre: delete the filesystem from the tree.")

from the staging tree and commit:

  18a592632e61 ("lustre: Use long long type to print inode time")

from the y2038 tree.

I fixed it up (I removed the files) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell


pgp3zD8Dsg55Y.pgp
Description: OpenPGP digital signature


linux-next: manual merge of the y2038 tree with the staging tree

2018-06-06 Thread Stephen Rothwell
Hi all,

Today's linux-next merge of the y2038 tree got conflicts in:

  drivers/staging/lustre/lustre/llite/llite_lib.c
  drivers/staging/lustre/lustre/llite/namei.c
  drivers/staging/lustre/lustre/lmv/lmv_obd.c
  drivers/staging/lustre/lustre/mdc/mdc_reint.c
  drivers/staging/lustre/lustre/obdclass/obdo.c

between commit:

  be65f9ed267f ("staging: lustre: delete the filesystem from the tree.")

from the staging tree and commit:

  18a592632e61 ("lustre: Use long long type to print inode time")

from the y2038 tree.

I fixed it up (I removed the files) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell


pgp3zD8Dsg55Y.pgp
Description: OpenPGP digital signature


Re: [PATCH] HV: Fix definition of struct hv_vp_assist_page.

2018-06-06 Thread Thomas Gleixner
On Mon, 21 May 2018, Tianyu Lan wrote:

KY 

> The struct hv_vp_assist_page was defined incorrectly.
> The "vtl_control" should be u64[3], "nested_enlightenments_control"
> should be a u64 and there is 7 reserved bytes following "enlighten_vmentry".
> This patch is to fix it.
> 
> Signed-off-by: Lan Tianyu 
> ---
>  arch/x86/include/asm/hyperv-tlfs.h | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h 
> b/arch/x86/include/asm/hyperv-tlfs.h
> index f7be6d03a310..fae0a5431cdd 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -496,10 +496,11 @@ struct hv_timer_message_payload {
>  /* Define virtual processor assist page structure. */
>  struct hv_vp_assist_page {
>   __u32 apic_assist;
> - __u32 reserved;
> - __u64 vtl_control[2];
> - __u64 nested_enlightenments_control[2];
> - __u32 enlighten_vmentry;
> + __u32 reserved1;
> + __u64 vtl_control[3];
> + __u64 nested_enlightenments_control;
> + __u8 enlighten_vmentry;
> + __u8 reserved2[7];
>   __u64 current_nested_vmcs;
>  };
>  
> -- 
> 2.14.3
> 


Re: [PATCH] HV: Fix definition of struct hv_vp_assist_page.

2018-06-06 Thread Thomas Gleixner
On Mon, 21 May 2018, Tianyu Lan wrote:

KY 

> The struct hv_vp_assist_page was defined incorrectly.
> The "vtl_control" should be u64[3], "nested_enlightenments_control"
> should be a u64 and there is 7 reserved bytes following "enlighten_vmentry".
> This patch is to fix it.
> 
> Signed-off-by: Lan Tianyu 
> ---
>  arch/x86/include/asm/hyperv-tlfs.h | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h 
> b/arch/x86/include/asm/hyperv-tlfs.h
> index f7be6d03a310..fae0a5431cdd 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -496,10 +496,11 @@ struct hv_timer_message_payload {
>  /* Define virtual processor assist page structure. */
>  struct hv_vp_assist_page {
>   __u32 apic_assist;
> - __u32 reserved;
> - __u64 vtl_control[2];
> - __u64 nested_enlightenments_control[2];
> - __u32 enlighten_vmentry;
> + __u32 reserved1;
> + __u64 vtl_control[3];
> + __u64 nested_enlightenments_control;
> + __u8 enlighten_vmentry;
> + __u8 reserved2[7];
>   __u64 current_nested_vmcs;
>  };
>  
> -- 
> 2.14.3
> 


Re: [PATCH 01/20] coresight: Fix memory leak in coresight_register

2018-06-06 Thread Suzuki K Poulose

On 06/06/2018 07:44 AM, Arvind Yadav wrote:

Hi Suzuki,


On Wednesday 06 June 2018 03:13 AM, Suzuki K Poulose wrote:

commit 6403587a930c ("coresight: use put_device() instead of kfree()")
introduced a memory leak where, if we fail to register the device
for coresight_device, we don't free the "coresight_device" object,
which was allocated via kzalloc(). Fix this by jumping to the
appropriate error path.

put_device() will decrement the last reference and then
free the memory by calling dev->release.  Internally
put_device() -> kobject_put() -> kobject_cleanup() which is
responsible to call 'dev -> release' and also free other kobject
resources. If you will see the coresight_device_release. There
we are releasing all allocated memory. Still if you call kfree() again
then it'll be redundancy.


You're right. I think it would be good to have a comment explaining this
to prevent this fix popping up in the future :-). I will add it

Thanks
Suzuki


Re: [PATCH 01/20] coresight: Fix memory leak in coresight_register

2018-06-06 Thread Suzuki K Poulose

On 06/06/2018 07:44 AM, Arvind Yadav wrote:

Hi Suzuki,


On Wednesday 06 June 2018 03:13 AM, Suzuki K Poulose wrote:

commit 6403587a930c ("coresight: use put_device() instead of kfree()")
introduced a memory leak where, if we fail to register the device
for coresight_device, we don't free the "coresight_device" object,
which was allocated via kzalloc(). Fix this by jumping to the
appropriate error path.

put_device() will decrement the last reference and then
free the memory by calling dev->release.  Internally
put_device() -> kobject_put() -> kobject_cleanup() which is
responsible to call 'dev -> release' and also free other kobject
resources. If you will see the coresight_device_release. There
we are releasing all allocated memory. Still if you call kfree() again
then it'll be redundancy.


You're right. I think it would be good to have a comment explaining this
to prevent this fix popping up in the future :-). I will add it

Thanks
Suzuki


linux-next: manual merge of the y2038 tree with the ceph tree

2018-06-06 Thread Stephen Rothwell
Hi all,

Today's linux-next merge of the y2038 tree got a conflict in:

  fs/ceph/addr.c

between commit:

  c843d13caefa ("libceph: make abort_on_full a per-osdc setting")

from the ceph tree and commit:

  213ae530f442 ("vfs: change inode times to use struct timespec64")

from the y2038 tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/ceph/addr.c
index ca0d5510ed50,0133ea2b784a..
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@@ -1934,7 -1936,8 +1936,7 @@@ static int __ceph_pool_perm_get(struct 
 0, false, true);
err = ceph_osdc_start_request(>client->osdc, rd_req, false);
  
-   wr_req->r_mtime = ci->vfs_inode.i_mtime;
+   wr_req->r_mtime = timespec64_to_timespec(ci->vfs_inode.i_mtime);
 -  wr_req->r_abort_on_full = true;
err2 = ceph_osdc_start_request(>client->osdc, wr_req, false);
  
if (!err)


pgpwC5Rr6RCNW.pgp
Description: OpenPGP digital signature


linux-next: manual merge of the y2038 tree with the ceph tree

2018-06-06 Thread Stephen Rothwell
Hi all,

Today's linux-next merge of the y2038 tree got a conflict in:

  fs/ceph/addr.c

between commit:

  c843d13caefa ("libceph: make abort_on_full a per-osdc setting")

from the ceph tree and commit:

  213ae530f442 ("vfs: change inode times to use struct timespec64")

from the y2038 tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/ceph/addr.c
index ca0d5510ed50,0133ea2b784a..
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@@ -1934,7 -1936,8 +1936,7 @@@ static int __ceph_pool_perm_get(struct 
 0, false, true);
err = ceph_osdc_start_request(>client->osdc, rd_req, false);
  
-   wr_req->r_mtime = ci->vfs_inode.i_mtime;
+   wr_req->r_mtime = timespec64_to_timespec(ci->vfs_inode.i_mtime);
 -  wr_req->r_abort_on_full = true;
err2 = ceph_osdc_start_request(>client->osdc, wr_req, false);
  
if (!err)


pgpwC5Rr6RCNW.pgp
Description: OpenPGP digital signature


[tip:x86/urgent] x86: Mark native_set_p4d() as __always_inline

2018-06-06 Thread tip-bot for Arnd Bergmann
Commit-ID:  046c0dbec0238c25b7526c26c9a9687664229ce2
Gitweb: https://git.kernel.org/tip/046c0dbec0238c25b7526c26c9a9687664229ce2
Author: Arnd Bergmann 
AuthorDate: Tue, 5 Jun 2018 13:35:15 +0200
Committer:  Thomas Gleixner 
CommitDate: Wed, 6 Jun 2018 12:09:45 +0200

x86: Mark native_set_p4d() as __always_inline

When CONFIG_OPTIMIZE_INLINING is enabled, the function native_set_p4d()
may not be fully inlined into the caller, resulting in a false-positive
warning about an access to the __pgtable_l5_enabled variable from a
non-__init function, despite the original caller being an __init function:

WARNING: vmlinux.o(.text.unlikely+0x1429): Section mismatch in reference from 
the function native_set_p4d() to the variable .init.data:__pgtable_l5_enabled
WARNING: vmlinux.o(.text.unlikely+0x1429): Section mismatch in reference from 
the function native_p4d_clear() to the variable .init.data:__pgtable_l5_enabled

The function native_set_p4d() references the variable __initdata
__pgtable_l5_enabled.  This is often because native_set_p4d lacks a
__initdata annotation or the annotation of __pgtable_l5_enabled is wrong.

Marking the native_set_p4d function and its caller native_p4d_clear()
avoids this problem.

I did not bisect the original cause, but I assume this is related to the
recent rework that turned pgtable_l5_enabled() into an inline function,
which in turn caused the compiler to make different inlining decisions.

Fixes: ad3fe525b950 ("x86/mm: Unify pgtable_l5_enabled usage in early boot 
code")
Signed-off-by: Arnd Bergmann 
Signed-off-by: Thomas Gleixner 
Acked-by: Kirill A. Shutemov 
Cc: Greg Kroah-Hartman 
Cc: Dave Hansen 
Cc: "H. Peter Anvin" 
Cc: Andrew Morton 
Cc: Zi Yan 
Cc: Naoya Horiguchi 
Link: https://lkml.kernel.org/r/20180605113715.1133726-1-a...@arndb.de

---
 arch/x86/include/asm/pgtable_64.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_64.h 
b/arch/x86/include/asm/pgtable_64.h
index 877bc27718ae..c750112cb416 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -216,7 +216,7 @@ static inline pgd_t pti_set_user_pgd(pgd_t *pgdp, pgd_t pgd)
 }
 #endif
 
-static inline void native_set_p4d(p4d_t *p4dp, p4d_t p4d)
+static __always_inline void native_set_p4d(p4d_t *p4dp, p4d_t p4d)
 {
pgd_t pgd;
 
@@ -230,7 +230,7 @@ static inline void native_set_p4d(p4d_t *p4dp, p4d_t p4d)
*p4dp = native_make_p4d(native_pgd_val(pgd));
 }
 
-static inline void native_p4d_clear(p4d_t *p4d)
+static __always_inline void native_p4d_clear(p4d_t *p4d)
 {
native_set_p4d(p4d, native_make_p4d(0));
 }


[tip:x86/urgent] x86: Mark native_set_p4d() as __always_inline

2018-06-06 Thread tip-bot for Arnd Bergmann
Commit-ID:  046c0dbec0238c25b7526c26c9a9687664229ce2
Gitweb: https://git.kernel.org/tip/046c0dbec0238c25b7526c26c9a9687664229ce2
Author: Arnd Bergmann 
AuthorDate: Tue, 5 Jun 2018 13:35:15 +0200
Committer:  Thomas Gleixner 
CommitDate: Wed, 6 Jun 2018 12:09:45 +0200

x86: Mark native_set_p4d() as __always_inline

When CONFIG_OPTIMIZE_INLINING is enabled, the function native_set_p4d()
may not be fully inlined into the caller, resulting in a false-positive
warning about an access to the __pgtable_l5_enabled variable from a
non-__init function, despite the original caller being an __init function:

WARNING: vmlinux.o(.text.unlikely+0x1429): Section mismatch in reference from 
the function native_set_p4d() to the variable .init.data:__pgtable_l5_enabled
WARNING: vmlinux.o(.text.unlikely+0x1429): Section mismatch in reference from 
the function native_p4d_clear() to the variable .init.data:__pgtable_l5_enabled

The function native_set_p4d() references the variable __initdata
__pgtable_l5_enabled.  This is often because native_set_p4d lacks a
__initdata annotation or the annotation of __pgtable_l5_enabled is wrong.

Marking the native_set_p4d function and its caller native_p4d_clear()
avoids this problem.

I did not bisect the original cause, but I assume this is related to the
recent rework that turned pgtable_l5_enabled() into an inline function,
which in turn caused the compiler to make different inlining decisions.

Fixes: ad3fe525b950 ("x86/mm: Unify pgtable_l5_enabled usage in early boot 
code")
Signed-off-by: Arnd Bergmann 
Signed-off-by: Thomas Gleixner 
Acked-by: Kirill A. Shutemov 
Cc: Greg Kroah-Hartman 
Cc: Dave Hansen 
Cc: "H. Peter Anvin" 
Cc: Andrew Morton 
Cc: Zi Yan 
Cc: Naoya Horiguchi 
Link: https://lkml.kernel.org/r/20180605113715.1133726-1-a...@arndb.de

---
 arch/x86/include/asm/pgtable_64.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_64.h 
b/arch/x86/include/asm/pgtable_64.h
index 877bc27718ae..c750112cb416 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -216,7 +216,7 @@ static inline pgd_t pti_set_user_pgd(pgd_t *pgdp, pgd_t pgd)
 }
 #endif
 
-static inline void native_set_p4d(p4d_t *p4dp, p4d_t p4d)
+static __always_inline void native_set_p4d(p4d_t *p4dp, p4d_t p4d)
 {
pgd_t pgd;
 
@@ -230,7 +230,7 @@ static inline void native_set_p4d(p4d_t *p4dp, p4d_t p4d)
*p4dp = native_make_p4d(native_pgd_val(pgd));
 }
 
-static inline void native_p4d_clear(p4d_t *p4d)
+static __always_inline void native_p4d_clear(p4d_t *p4d)
 {
native_set_p4d(p4d, native_make_p4d(0));
 }


Re: [PATCH v3 0/6] Add MCAN Support for dra76x

2018-06-06 Thread Faiz Abbas
Hi,

On Wednesday 06 June 2018 03:39 PM, Tony Lindgren wrote:
> * Faiz Abbas  [180606 06:09]:
>> The following patches add dts and sysconfig support
>> for MCAN on TI's dra76 SOCs
>>
>> The patches depend on the following series:
>> https://patchwork.kernel.org/patch/10221105/
>>
>> Changes in v3:
>>  1. Added reset functionality to the ti-sysc
>> driver. This enables me to drop the hwmod
>> data patch as everything is being done in dt.
>>
>>  2. Dropped ti,hwmods from the dts nodes
> 
> Cool good to hear that works :)
> 
>>  4. Removed the status="disabled" in dtsi
>> followed by status="okay" in dts.
> 
> Hmm okay is the default and is not needed. I did not notice
> okay in this set based on a quick glance so maybe you already
> dropped it?

I guess that wasn't clear. I meant to say I have dropped both
status=disabled and status=okay because okay is default.

Thanks,
Faiz


Re: [PATCH v3 0/6] Add MCAN Support for dra76x

2018-06-06 Thread Faiz Abbas
Hi,

On Wednesday 06 June 2018 03:39 PM, Tony Lindgren wrote:
> * Faiz Abbas  [180606 06:09]:
>> The following patches add dts and sysconfig support
>> for MCAN on TI's dra76 SOCs
>>
>> The patches depend on the following series:
>> https://patchwork.kernel.org/patch/10221105/
>>
>> Changes in v3:
>>  1. Added reset functionality to the ti-sysc
>> driver. This enables me to drop the hwmod
>> data patch as everything is being done in dt.
>>
>>  2. Dropped ti,hwmods from the dts nodes
> 
> Cool good to hear that works :)
> 
>>  4. Removed the status="disabled" in dtsi
>> followed by status="okay" in dts.
> 
> Hmm okay is the default and is not needed. I did not notice
> okay in this set based on a quick glance so maybe you already
> dropped it?

I guess that wasn't clear. I meant to say I have dropped both
status=disabled and status=okay because okay is default.

Thanks,
Faiz


Re: [PATCH v5 00/10] track CPU utilization

2018-06-06 Thread Quentin Perret
On Wednesday 06 Jun 2018 at 11:59:04 (+0200), Vincent Guittot wrote:
> On 6 June 2018 at 11:44, Quentin Perret  wrote:
> > On Tuesday 05 Jun 2018 at 16:18:09 (+0200), Peter Zijlstra wrote:
> >> On Mon, Jun 04, 2018 at 08:08:58PM +0200, Vincent Guittot wrote:
> >> > On 4 June 2018 at 18:50, Peter Zijlstra  wrote:
> >>
> >> > > So this patch-set tracks the !cfs occupation using the same function,
> >> > > which is all good. But what, if instead of using that to compensate the
> >> > > OPP selection, we employ that to renormalize the util signal?
> >> > >
> >> > > If we normalize util against the dynamic (rt_avg affected) 
> >> > > cpu_capacity,
> >> > > then I think your initial problem goes away. Because while the RT task
> >> > > will push the util to .5, it will at the same time push the CPU 
> >> > > capacity
> >> > > to .5, and renormalized that gives 1.
> >> > >
> >> > >   NOTE: the renorm would then become something like:
> >> > > scale_cpu = arch_scale_cpu_capacity() / rt_frac();
> >>
> >> Should probably be:
> >>
> >>   scale_cpu = atch_scale_cpu_capacity() / (1 - rt_frac())
> >>
> >> > >
> >> > >
> >> > > On IRC I mentioned stopping the CFS clock when preempted, and while 
> >> > > that
> >> > > would result in fixed numbers, Vincent was right in pointing out the
> >> > > numbers will be difficult to interpret, since the meaning will be 
> >> > > purely
> >> > > CPU local and I'm not sure you can actually fix it again with
> >> > > normalization.
> >> > >
> >> > > Imagine, running a .3 RT task, that would push the (always running) CFS
> >> > > down to .7, but because we discard all !cfs time, it actually has 1. If
> >> > > we try and normalize that we'll end up with ~1.43, which is of course
> >> > > completely broken.
> >> > >
> >> > >
> >> > > _However_, all that happens for util, also happens for load. So the 
> >> > > above
> >> > > scenario will also make the CPU appear less loaded than it actually is.
> >> >
> >> > The load will continue to increase because we track runnable state and
> >> > not running for the load
> >>
> >> Duh yes. So renormalizing it once, like proposed for util would actually
> >> do the right thing there too.  Would not that allow us to get rid of
> >> much of the capacity magic in the load balance code?
> >>
> >> /me thinks more..
> >>
> >> Bah, no.. because you don't want this dynamic renormalization part of
> >> the sums. So you want to keep it after the fact. :/
> >>
> >> > As you mentioned, scale_rt_capacity give the remaining capacity for
> >> > cfs and it will behave like cfs util_avg now that it uses PELT. So as
> >> > long as cfs util_avg <  scale_rt_capacity(we probably need a margin)
> >> > we keep using dl bandwidth + cfs util_avg + rt util_avg for selecting
> >> > OPP because we have remaining spare capacity but if  cfs util_avg ==
> >> > scale_rt_capacity, we make sure to use max OPP.
> >>
> >> Good point, when cfs-util < cfs-cap then there is idle time and the util
> >> number is 'right', when cfs-util == cfs-cap we're overcommitted and
> >> should go max.
> >>
> >> Since the util and cap values are aligned that should track nicely.
> >
> > So Vincent proposed to have a margin between cfs util and cfs cap to be
> > sure there is a little bit of idle time. This is _exactly_ what the
> > overutilized flag in EAS does. That would actually make a lot of sense
> > to use that flag in schedutil. The idea is basically to say, if there
> > isn't enough idle time on all CPUs, the util signal are kinda wrong, so
> > let's not make any decisions (task placement or OPP selection) based on
> > that. If overutilized, go to max freq. Does that make sense ?
> 
> Yes it's similar to the overutilized except that
> - this is done per cpu and whereas overutilization is for the whole system

Is this a good thing ? It has to be discussed. Anyways, the patch from
Morten which is part of the latest EAS posting (v3) introduces a
cpu_overutilized() function which does what you want I think.

> - the test is done at every freq update and not only during some cfs
> event and it uses the last up to date value and not a periodically
> updated snapshot of the value

Yeah good point. Now, the overutilized flag is attached to the root domain
so you should be able to set/clear it from RT/DL whenever that makes sense
I suppose. That's just a flag about the current state of the system so I
don't see why it should be touched only by CFS.

> - this is done also without EAS

The overutilized flag doesn't have to come with EAS if it is useful for
something else (OPP selection).

> 
> Then for the margin, it has to be discussed if it is really needed or not

+1

Thanks,
Quentin


Re: [PATCH v5 00/10] track CPU utilization

2018-06-06 Thread Quentin Perret
On Wednesday 06 Jun 2018 at 11:59:04 (+0200), Vincent Guittot wrote:
> On 6 June 2018 at 11:44, Quentin Perret  wrote:
> > On Tuesday 05 Jun 2018 at 16:18:09 (+0200), Peter Zijlstra wrote:
> >> On Mon, Jun 04, 2018 at 08:08:58PM +0200, Vincent Guittot wrote:
> >> > On 4 June 2018 at 18:50, Peter Zijlstra  wrote:
> >>
> >> > > So this patch-set tracks the !cfs occupation using the same function,
> >> > > which is all good. But what, if instead of using that to compensate the
> >> > > OPP selection, we employ that to renormalize the util signal?
> >> > >
> >> > > If we normalize util against the dynamic (rt_avg affected) 
> >> > > cpu_capacity,
> >> > > then I think your initial problem goes away. Because while the RT task
> >> > > will push the util to .5, it will at the same time push the CPU 
> >> > > capacity
> >> > > to .5, and renormalized that gives 1.
> >> > >
> >> > >   NOTE: the renorm would then become something like:
> >> > > scale_cpu = arch_scale_cpu_capacity() / rt_frac();
> >>
> >> Should probably be:
> >>
> >>   scale_cpu = atch_scale_cpu_capacity() / (1 - rt_frac())
> >>
> >> > >
> >> > >
> >> > > On IRC I mentioned stopping the CFS clock when preempted, and while 
> >> > > that
> >> > > would result in fixed numbers, Vincent was right in pointing out the
> >> > > numbers will be difficult to interpret, since the meaning will be 
> >> > > purely
> >> > > CPU local and I'm not sure you can actually fix it again with
> >> > > normalization.
> >> > >
> >> > > Imagine, running a .3 RT task, that would push the (always running) CFS
> >> > > down to .7, but because we discard all !cfs time, it actually has 1. If
> >> > > we try and normalize that we'll end up with ~1.43, which is of course
> >> > > completely broken.
> >> > >
> >> > >
> >> > > _However_, all that happens for util, also happens for load. So the 
> >> > > above
> >> > > scenario will also make the CPU appear less loaded than it actually is.
> >> >
> >> > The load will continue to increase because we track runnable state and
> >> > not running for the load
> >>
> >> Duh yes. So renormalizing it once, like proposed for util would actually
> >> do the right thing there too.  Would not that allow us to get rid of
> >> much of the capacity magic in the load balance code?
> >>
> >> /me thinks more..
> >>
> >> Bah, no.. because you don't want this dynamic renormalization part of
> >> the sums. So you want to keep it after the fact. :/
> >>
> >> > As you mentioned, scale_rt_capacity give the remaining capacity for
> >> > cfs and it will behave like cfs util_avg now that it uses PELT. So as
> >> > long as cfs util_avg <  scale_rt_capacity(we probably need a margin)
> >> > we keep using dl bandwidth + cfs util_avg + rt util_avg for selecting
> >> > OPP because we have remaining spare capacity but if  cfs util_avg ==
> >> > scale_rt_capacity, we make sure to use max OPP.
> >>
> >> Good point, when cfs-util < cfs-cap then there is idle time and the util
> >> number is 'right', when cfs-util == cfs-cap we're overcommitted and
> >> should go max.
> >>
> >> Since the util and cap values are aligned that should track nicely.
> >
> > So Vincent proposed to have a margin between cfs util and cfs cap to be
> > sure there is a little bit of idle time. This is _exactly_ what the
> > overutilized flag in EAS does. That would actually make a lot of sense
> > to use that flag in schedutil. The idea is basically to say, if there
> > isn't enough idle time on all CPUs, the util signal are kinda wrong, so
> > let's not make any decisions (task placement or OPP selection) based on
> > that. If overutilized, go to max freq. Does that make sense ?
> 
> Yes it's similar to the overutilized except that
> - this is done per cpu and whereas overutilization is for the whole system

Is this a good thing ? It has to be discussed. Anyways, the patch from
Morten which is part of the latest EAS posting (v3) introduces a
cpu_overutilized() function which does what you want I think.

> - the test is done at every freq update and not only during some cfs
> event and it uses the last up to date value and not a periodically
> updated snapshot of the value

Yeah good point. Now, the overutilized flag is attached to the root domain
so you should be able to set/clear it from RT/DL whenever that makes sense
I suppose. That's just a flag about the current state of the system so I
don't see why it should be touched only by CFS.

> - this is done also without EAS

The overutilized flag doesn't have to come with EAS if it is useful for
something else (OPP selection).

> 
> Then for the margin, it has to be discussed if it is really needed or not

+1

Thanks,
Quentin


[tip:irq/urgent] irqchip/ls-scfg-msi: Map MSIs in the iommu

2018-06-06 Thread tip-bot for Laurentiu Tudor
Commit-ID:  0cdd431c337e99177e68597f3de34bedd3a20a74
Gitweb: https://git.kernel.org/tip/0cdd431c337e99177e68597f3de34bedd3a20a74
Author: Laurentiu Tudor 
AuthorDate: Tue, 5 Jun 2018 15:27:27 +0300
Committer:  Thomas Gleixner 
CommitDate: Wed, 6 Jun 2018 12:05:19 +0200

irqchip/ls-scfg-msi: Map MSIs in the iommu

Add the required iommu_dma_map_msi_msg() when composing the MSI message,
otherwise the interrupts will not work.

Signed-off-by: Laurentiu Tudor 
Signed-off-by: Thomas Gleixner 
Cc: ja...@lakedaemon.net
Cc: marc.zyng...@arm.com
Cc: zhiqiang@nxp.com
Cc: minghuan.l...@nxp.com
Link: https://lkml.kernel.org/r/20180605122727.12831-1-laurentiu.tu...@nxp.com

---
 drivers/irqchip/irq-ls-scfg-msi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/irqchip/irq-ls-scfg-msi.c 
b/drivers/irqchip/irq-ls-scfg-msi.c
index 57e3d900f19e..1ec3bfe56693 100644
--- a/drivers/irqchip/irq-ls-scfg-msi.c
+++ b/drivers/irqchip/irq-ls-scfg-msi.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define MSI_IRQS_PER_MSIR  32
 #define MSI_MSIR_OFFSET4
@@ -94,6 +95,8 @@ static void ls_scfg_msi_compose_msg(struct irq_data *data, 
struct msi_msg *msg)
 
if (msi_affinity_flag)
msg->data |= cpumask_first(data->common->affinity);
+
+   iommu_dma_map_msi_msg(data->irq, msg);
 }
 
 static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,


[tip:irq/urgent] irqchip/ls-scfg-msi: Map MSIs in the iommu

2018-06-06 Thread tip-bot for Laurentiu Tudor
Commit-ID:  0cdd431c337e99177e68597f3de34bedd3a20a74
Gitweb: https://git.kernel.org/tip/0cdd431c337e99177e68597f3de34bedd3a20a74
Author: Laurentiu Tudor 
AuthorDate: Tue, 5 Jun 2018 15:27:27 +0300
Committer:  Thomas Gleixner 
CommitDate: Wed, 6 Jun 2018 12:05:19 +0200

irqchip/ls-scfg-msi: Map MSIs in the iommu

Add the required iommu_dma_map_msi_msg() when composing the MSI message,
otherwise the interrupts will not work.

Signed-off-by: Laurentiu Tudor 
Signed-off-by: Thomas Gleixner 
Cc: ja...@lakedaemon.net
Cc: marc.zyng...@arm.com
Cc: zhiqiang@nxp.com
Cc: minghuan.l...@nxp.com
Link: https://lkml.kernel.org/r/20180605122727.12831-1-laurentiu.tu...@nxp.com

---
 drivers/irqchip/irq-ls-scfg-msi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/irqchip/irq-ls-scfg-msi.c 
b/drivers/irqchip/irq-ls-scfg-msi.c
index 57e3d900f19e..1ec3bfe56693 100644
--- a/drivers/irqchip/irq-ls-scfg-msi.c
+++ b/drivers/irqchip/irq-ls-scfg-msi.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define MSI_IRQS_PER_MSIR  32
 #define MSI_MSIR_OFFSET4
@@ -94,6 +95,8 @@ static void ls_scfg_msi_compose_msg(struct irq_data *data, 
struct msi_msg *msg)
 
if (msi_affinity_flag)
msg->data |= cpumask_first(data->common->affinity);
+
+   iommu_dma_map_msi_msg(data->irq, msg);
 }
 
 static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,


[tip:irq/urgent] irqchip/stm32: Fix non-SMP build warning

2018-06-06 Thread tip-bot for Arnd Bergmann
Commit-ID:  a84277bf3efcc7bb5b5a5eec62de5c134cb47ee5
Gitweb: https://git.kernel.org/tip/a84277bf3efcc7bb5b5a5eec62de5c134cb47ee5
Author: Arnd Bergmann 
AuthorDate: Tue, 5 Jun 2018 13:43:34 +0200
Committer:  Thomas Gleixner 
CommitDate: Wed, 6 Jun 2018 12:05:19 +0200

irqchip/stm32: Fix non-SMP build warning

A CONFIG_SMP=n build emits a harmless compile-time warning:

drivers/irqchip/irq-stm32-exti.c:495:12: error: 'stm32_exti_h_set_affinity' 
defined but not used [-Werror=unused-function]

The #ifdef is inconsistent here, and it's better to use an IS_ENABLED() check
that lets the compiler silently drop that function.

Fixes: 927abfc4461e ("irqchip/stm32: Add stm32mp1 support with hierarchy 
domain")
Signed-off-by: Arnd Bergmann 
Signed-off-by: Thomas Gleixner 
Reviewed-by: Ludovic Barre 
Cc: Rob Herring 
Cc: Benjamin Gaignard 
Cc: Radoslaw Pietrzyk 
Cc: Jason Cooper 
Cc: Marc Zyngier 
Cc: linux-arm-ker...@lists.infradead.org
Cc: Maxime Coquelin 
Cc: Alexandre Torgue 
Link: https://lkml.kernel.org/r/20180605114347.1347128-1-a...@arndb.de

---
 drivers/irqchip/irq-stm32-exti.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
index 5089c1e2838d..3a7e8905a97e 100644
--- a/drivers/irqchip/irq-stm32-exti.c
+++ b/drivers/irqchip/irq-stm32-exti.c
@@ -552,9 +552,7 @@ static struct irq_chip stm32_exti_h_chip = {
.irq_set_type   = stm32_exti_h_set_type,
.irq_set_wake   = stm32_exti_h_set_wake,
.flags  = IRQCHIP_MASK_ON_SUSPEND,
-#ifdef CONFIG_SMP
-   .irq_set_affinity   = stm32_exti_h_set_affinity,
-#endif
+   .irq_set_affinity   = IS_ENABLED(CONFIG_SMP) ? 
stm32_exti_h_set_affinity : NULL,
 };
 
 static int stm32_exti_h_domain_alloc(struct irq_domain *dm,


Re: [PATCH v3 0/6] Add MCAN Support for dra76x

2018-06-06 Thread Tony Lindgren
* Faiz Abbas  [180606 06:09]:
> The following patches add dts and sysconfig support
> for MCAN on TI's dra76 SOCs
> 
> The patches depend on the following series:
> https://patchwork.kernel.org/patch/10221105/
> 
> Changes in v3:
>  1. Added reset functionality to the ti-sysc
> driver. This enables me to drop the hwmod
> data patch as everything is being done in dt.
>
>  2. Dropped ti,hwmods from the dts nodes

Cool good to hear that works :)

>  4. Removed the status="disabled" in dtsi
> followed by status="okay" in dts.

Hmm okay is the default and is not needed. I did not notice
okay in this set based on a quick glance so maybe you already
dropped it?

Regards,

Tony


[tip:irq/urgent] irqchip/stm32: Fix non-SMP build warning

2018-06-06 Thread tip-bot for Arnd Bergmann
Commit-ID:  a84277bf3efcc7bb5b5a5eec62de5c134cb47ee5
Gitweb: https://git.kernel.org/tip/a84277bf3efcc7bb5b5a5eec62de5c134cb47ee5
Author: Arnd Bergmann 
AuthorDate: Tue, 5 Jun 2018 13:43:34 +0200
Committer:  Thomas Gleixner 
CommitDate: Wed, 6 Jun 2018 12:05:19 +0200

irqchip/stm32: Fix non-SMP build warning

A CONFIG_SMP=n build emits a harmless compile-time warning:

drivers/irqchip/irq-stm32-exti.c:495:12: error: 'stm32_exti_h_set_affinity' 
defined but not used [-Werror=unused-function]

The #ifdef is inconsistent here, and it's better to use an IS_ENABLED() check
that lets the compiler silently drop that function.

Fixes: 927abfc4461e ("irqchip/stm32: Add stm32mp1 support with hierarchy 
domain")
Signed-off-by: Arnd Bergmann 
Signed-off-by: Thomas Gleixner 
Reviewed-by: Ludovic Barre 
Cc: Rob Herring 
Cc: Benjamin Gaignard 
Cc: Radoslaw Pietrzyk 
Cc: Jason Cooper 
Cc: Marc Zyngier 
Cc: linux-arm-ker...@lists.infradead.org
Cc: Maxime Coquelin 
Cc: Alexandre Torgue 
Link: https://lkml.kernel.org/r/20180605114347.1347128-1-a...@arndb.de

---
 drivers/irqchip/irq-stm32-exti.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
index 5089c1e2838d..3a7e8905a97e 100644
--- a/drivers/irqchip/irq-stm32-exti.c
+++ b/drivers/irqchip/irq-stm32-exti.c
@@ -552,9 +552,7 @@ static struct irq_chip stm32_exti_h_chip = {
.irq_set_type   = stm32_exti_h_set_type,
.irq_set_wake   = stm32_exti_h_set_wake,
.flags  = IRQCHIP_MASK_ON_SUSPEND,
-#ifdef CONFIG_SMP
-   .irq_set_affinity   = stm32_exti_h_set_affinity,
-#endif
+   .irq_set_affinity   = IS_ENABLED(CONFIG_SMP) ? 
stm32_exti_h_set_affinity : NULL,
 };
 
 static int stm32_exti_h_domain_alloc(struct irq_domain *dm,


Re: [PATCH v3 0/6] Add MCAN Support for dra76x

2018-06-06 Thread Tony Lindgren
* Faiz Abbas  [180606 06:09]:
> The following patches add dts and sysconfig support
> for MCAN on TI's dra76 SOCs
> 
> The patches depend on the following series:
> https://patchwork.kernel.org/patch/10221105/
> 
> Changes in v3:
>  1. Added reset functionality to the ti-sysc
> driver. This enables me to drop the hwmod
> data patch as everything is being done in dt.
>
>  2. Dropped ti,hwmods from the dts nodes

Cool good to hear that works :)

>  4. Removed the status="disabled" in dtsi
> followed by status="okay" in dts.

Hmm okay is the default and is not needed. I did not notice
okay in this set based on a quick glance so maybe you already
dropped it?

Regards,

Tony


Re: [PATCH] arm64: topology: Avoid checking numa mask for scheduler MC selection

2018-06-06 Thread Sudeep Holla



On 05/06/18 20:08, Jeremy Linton wrote:
> The numa mask subset check has problems if !CONFIG_NUMA, over hotplug
> operations or during early boot. Lets disable the NUMA siblings checks
> for the time being, as NUMA in socket machines have LLC's that will
> assure that the scheduler topology isn't "borken".
> 

^ broken ? (not sure if usage of borken is intentional :))

> Futher, as a defensive mechanism during hotplug, lets assure that the

^ Further

> LLC siblings are also masked.
>

Also add the symptoms of the issue we say as Geert suggested me.
Something like:
" This often leads to system hang or crash during CPU hotplug and system
suspend operation. This is mostly observed on HMP systems where the
CPU compute capacities are different and ends up in different scheduler
domains. Since cpumask_of_node is returned instead core_sibling, the
scheduler is confused with incorrect cpumasks(e.g. one CPU in two
different sched domains at the same time) on CPU hotplug."

You can add Reported-by: Geert... ?

> Signed-off-by: Jeremy Linton 
> ---
>  arch/arm64/kernel/topology.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 7415c166281f..f845a8617812 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -215,13 +215,8 @@ EXPORT_SYMBOL_GPL(cpu_topology);
>  
>  const struct cpumask *cpu_coregroup_mask(int cpu)
>  {
> - const cpumask_t *core_mask = cpumask_of_node(cpu_to_node(cpu));
> + const cpumask_t *core_mask = _topology[cpu].core_sibling;
>  
> - /* Find the smaller of NUMA, core or LLC siblings */
> - if (cpumask_subset(_topology[cpu].core_sibling, core_mask)) {
> - /* not numa in package, lets use the package siblings */
> - core_mask = _topology[cpu].core_sibling;
> - }
>   if (cpu_topology[cpu].llc_id != -1) {
>   if (cpumask_subset(_topology[cpu].llc_siblings, core_mask))
>   core_mask = _topology[cpu].llc_siblings;
> @@ -239,8 +234,10 @@ static void update_siblings_masks(unsigned int cpuid)
>   for_each_possible_cpu(cpu) {
>   cpu_topo = _topology[cpu];
>  
> - if (cpuid_topo->llc_id == cpu_topo->llc_id)
> + if (cpuid_topo->llc_id == cpu_topo->llc_id) {
>   cpumask_set_cpu(cpu, _topo->llc_siblings);
> + cpumask_set_cpu(cpuid, _topo->llc_siblings);
> + }
>  
>   if (cpuid_topo->package_id != cpu_topo->package_id)
>   continue;
> 

Looks good to me for now. I might need to tweek it a bit when I add the
support to update topology on hotplug. But that's for latter. For now,

Reviewed-by: Sudeep Holla 

-- 
Regards,
Sudeep


Re: [PATCH] arm64: topology: Avoid checking numa mask for scheduler MC selection

2018-06-06 Thread Sudeep Holla



On 05/06/18 20:08, Jeremy Linton wrote:
> The numa mask subset check has problems if !CONFIG_NUMA, over hotplug
> operations or during early boot. Lets disable the NUMA siblings checks
> for the time being, as NUMA in socket machines have LLC's that will
> assure that the scheduler topology isn't "borken".
> 

^ broken ? (not sure if usage of borken is intentional :))

> Futher, as a defensive mechanism during hotplug, lets assure that the

^ Further

> LLC siblings are also masked.
>

Also add the symptoms of the issue we say as Geert suggested me.
Something like:
" This often leads to system hang or crash during CPU hotplug and system
suspend operation. This is mostly observed on HMP systems where the
CPU compute capacities are different and ends up in different scheduler
domains. Since cpumask_of_node is returned instead core_sibling, the
scheduler is confused with incorrect cpumasks(e.g. one CPU in two
different sched domains at the same time) on CPU hotplug."

You can add Reported-by: Geert... ?

> Signed-off-by: Jeremy Linton 
> ---
>  arch/arm64/kernel/topology.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 7415c166281f..f845a8617812 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -215,13 +215,8 @@ EXPORT_SYMBOL_GPL(cpu_topology);
>  
>  const struct cpumask *cpu_coregroup_mask(int cpu)
>  {
> - const cpumask_t *core_mask = cpumask_of_node(cpu_to_node(cpu));
> + const cpumask_t *core_mask = _topology[cpu].core_sibling;
>  
> - /* Find the smaller of NUMA, core or LLC siblings */
> - if (cpumask_subset(_topology[cpu].core_sibling, core_mask)) {
> - /* not numa in package, lets use the package siblings */
> - core_mask = _topology[cpu].core_sibling;
> - }
>   if (cpu_topology[cpu].llc_id != -1) {
>   if (cpumask_subset(_topology[cpu].llc_siblings, core_mask))
>   core_mask = _topology[cpu].llc_siblings;
> @@ -239,8 +234,10 @@ static void update_siblings_masks(unsigned int cpuid)
>   for_each_possible_cpu(cpu) {
>   cpu_topo = _topology[cpu];
>  
> - if (cpuid_topo->llc_id == cpu_topo->llc_id)
> + if (cpuid_topo->llc_id == cpu_topo->llc_id) {
>   cpumask_set_cpu(cpu, _topo->llc_siblings);
> + cpumask_set_cpu(cpuid, _topo->llc_siblings);
> + }
>  
>   if (cpuid_topo->package_id != cpu_topo->package_id)
>   continue;
> 

Looks good to me for now. I might need to tweek it a bit when I add the
support to update topology on hotplug. But that's for latter. For now,

Reviewed-by: Sudeep Holla 

-- 
Regards,
Sudeep


Re: [PATCH v5 00/10] track CPU utilization

2018-06-06 Thread Vincent Guittot
On 6 June 2018 at 11:59, Vincent Guittot  wrote:
> On 6 June 2018 at 11:44, Quentin Perret  wrote:
>> On Tuesday 05 Jun 2018 at 16:18:09 (+0200), Peter Zijlstra wrote:

[snip]

>>>
>>> > As you mentioned, scale_rt_capacity give the remaining capacity for
>>> > cfs and it will behave like cfs util_avg now that it uses PELT. So as
>>> > long as cfs util_avg <  scale_rt_capacity(we probably need a margin)
>>> > we keep using dl bandwidth + cfs util_avg + rt util_avg for selecting
>>> > OPP because we have remaining spare capacity but if  cfs util_avg ==
>>> > scale_rt_capacity, we make sure to use max OPP.
>>>
>>> Good point, when cfs-util < cfs-cap then there is idle time and the util
>>> number is 'right', when cfs-util == cfs-cap we're overcommitted and
>>> should go max.
>>>
>>> Since the util and cap values are aligned that should track nicely.

I have re run my tests and and the results seem to be ok so far.

I'm going to clean up a bit the code used for the test and sent a new
version of the proposal

>>
>> So Vincent proposed to have a margin between cfs util and cfs cap to be
>> sure there is a little bit of idle time. This is _exactly_ what the
>> overutilized flag in EAS does. That would actually make a lot of sense
>> to use that flag in schedutil. The idea is basically to say, if there
>> isn't enough idle time on all CPUs, the util signal are kinda wrong, so
>> let's not make any decisions (task placement or OPP selection) based on
>> that. If overutilized, go to max freq. Does that make sense ?
>
> Yes it's similar to the overutilized except that
> - this is done per cpu and whereas overutilization is for the whole system
> - the test is done at every freq update and not only during some cfs
> event and it uses the last up to date value and not a periodically
> updated snapshot of the value
> - this is done also without EAS
>
> Then for the margin, it has to be discussed if it is really needed or not
>
>>
>> Thanks,
>> Quentin


Re: [PATCH v5 00/10] track CPU utilization

2018-06-06 Thread Vincent Guittot
On 6 June 2018 at 11:59, Vincent Guittot  wrote:
> On 6 June 2018 at 11:44, Quentin Perret  wrote:
>> On Tuesday 05 Jun 2018 at 16:18:09 (+0200), Peter Zijlstra wrote:

[snip]

>>>
>>> > As you mentioned, scale_rt_capacity give the remaining capacity for
>>> > cfs and it will behave like cfs util_avg now that it uses PELT. So as
>>> > long as cfs util_avg <  scale_rt_capacity(we probably need a margin)
>>> > we keep using dl bandwidth + cfs util_avg + rt util_avg for selecting
>>> > OPP because we have remaining spare capacity but if  cfs util_avg ==
>>> > scale_rt_capacity, we make sure to use max OPP.
>>>
>>> Good point, when cfs-util < cfs-cap then there is idle time and the util
>>> number is 'right', when cfs-util == cfs-cap we're overcommitted and
>>> should go max.
>>>
>>> Since the util and cap values are aligned that should track nicely.

I have re run my tests and and the results seem to be ok so far.

I'm going to clean up a bit the code used for the test and sent a new
version of the proposal

>>
>> So Vincent proposed to have a margin between cfs util and cfs cap to be
>> sure there is a little bit of idle time. This is _exactly_ what the
>> overutilized flag in EAS does. That would actually make a lot of sense
>> to use that flag in schedutil. The idea is basically to say, if there
>> isn't enough idle time on all CPUs, the util signal are kinda wrong, so
>> let's not make any decisions (task placement or OPP selection) based on
>> that. If overutilized, go to max freq. Does that make sense ?
>
> Yes it's similar to the overutilized except that
> - this is done per cpu and whereas overutilization is for the whole system
> - the test is done at every freq update and not only during some cfs
> event and it uses the last up to date value and not a periodically
> updated snapshot of the value
> - this is done also without EAS
>
> Then for the margin, it has to be discussed if it is really needed or not
>
>>
>> Thanks,
>> Quentin


Re: [PATCH v5 00/10] track CPU utilization

2018-06-06 Thread Vincent Guittot
On 6 June 2018 at 11:44, Quentin Perret  wrote:
> On Tuesday 05 Jun 2018 at 16:18:09 (+0200), Peter Zijlstra wrote:
>> On Mon, Jun 04, 2018 at 08:08:58PM +0200, Vincent Guittot wrote:
>> > On 4 June 2018 at 18:50, Peter Zijlstra  wrote:
>>
>> > > So this patch-set tracks the !cfs occupation using the same function,
>> > > which is all good. But what, if instead of using that to compensate the
>> > > OPP selection, we employ that to renormalize the util signal?
>> > >
>> > > If we normalize util against the dynamic (rt_avg affected) cpu_capacity,
>> > > then I think your initial problem goes away. Because while the RT task
>> > > will push the util to .5, it will at the same time push the CPU capacity
>> > > to .5, and renormalized that gives 1.
>> > >
>> > >   NOTE: the renorm would then become something like:
>> > > scale_cpu = arch_scale_cpu_capacity() / rt_frac();
>>
>> Should probably be:
>>
>>   scale_cpu = atch_scale_cpu_capacity() / (1 - rt_frac())
>>
>> > >
>> > >
>> > > On IRC I mentioned stopping the CFS clock when preempted, and while that
>> > > would result in fixed numbers, Vincent was right in pointing out the
>> > > numbers will be difficult to interpret, since the meaning will be purely
>> > > CPU local and I'm not sure you can actually fix it again with
>> > > normalization.
>> > >
>> > > Imagine, running a .3 RT task, that would push the (always running) CFS
>> > > down to .7, but because we discard all !cfs time, it actually has 1. If
>> > > we try and normalize that we'll end up with ~1.43, which is of course
>> > > completely broken.
>> > >
>> > >
>> > > _However_, all that happens for util, also happens for load. So the above
>> > > scenario will also make the CPU appear less loaded than it actually is.
>> >
>> > The load will continue to increase because we track runnable state and
>> > not running for the load
>>
>> Duh yes. So renormalizing it once, like proposed for util would actually
>> do the right thing there too.  Would not that allow us to get rid of
>> much of the capacity magic in the load balance code?
>>
>> /me thinks more..
>>
>> Bah, no.. because you don't want this dynamic renormalization part of
>> the sums. So you want to keep it after the fact. :/
>>
>> > As you mentioned, scale_rt_capacity give the remaining capacity for
>> > cfs and it will behave like cfs util_avg now that it uses PELT. So as
>> > long as cfs util_avg <  scale_rt_capacity(we probably need a margin)
>> > we keep using dl bandwidth + cfs util_avg + rt util_avg for selecting
>> > OPP because we have remaining spare capacity but if  cfs util_avg ==
>> > scale_rt_capacity, we make sure to use max OPP.
>>
>> Good point, when cfs-util < cfs-cap then there is idle time and the util
>> number is 'right', when cfs-util == cfs-cap we're overcommitted and
>> should go max.
>>
>> Since the util and cap values are aligned that should track nicely.
>
> So Vincent proposed to have a margin between cfs util and cfs cap to be
> sure there is a little bit of idle time. This is _exactly_ what the
> overutilized flag in EAS does. That would actually make a lot of sense
> to use that flag in schedutil. The idea is basically to say, if there
> isn't enough idle time on all CPUs, the util signal are kinda wrong, so
> let's not make any decisions (task placement or OPP selection) based on
> that. If overutilized, go to max freq. Does that make sense ?

Yes it's similar to the overutilized except that
- this is done per cpu and whereas overutilization is for the whole system
- the test is done at every freq update and not only during some cfs
event and it uses the last up to date value and not a periodically
updated snapshot of the value
- this is done also without EAS

Then for the margin, it has to be discussed if it is really needed or not

>
> Thanks,
> Quentin


Re: [PATCH v5 00/10] track CPU utilization

2018-06-06 Thread Vincent Guittot
On 6 June 2018 at 11:44, Quentin Perret  wrote:
> On Tuesday 05 Jun 2018 at 16:18:09 (+0200), Peter Zijlstra wrote:
>> On Mon, Jun 04, 2018 at 08:08:58PM +0200, Vincent Guittot wrote:
>> > On 4 June 2018 at 18:50, Peter Zijlstra  wrote:
>>
>> > > So this patch-set tracks the !cfs occupation using the same function,
>> > > which is all good. But what, if instead of using that to compensate the
>> > > OPP selection, we employ that to renormalize the util signal?
>> > >
>> > > If we normalize util against the dynamic (rt_avg affected) cpu_capacity,
>> > > then I think your initial problem goes away. Because while the RT task
>> > > will push the util to .5, it will at the same time push the CPU capacity
>> > > to .5, and renormalized that gives 1.
>> > >
>> > >   NOTE: the renorm would then become something like:
>> > > scale_cpu = arch_scale_cpu_capacity() / rt_frac();
>>
>> Should probably be:
>>
>>   scale_cpu = atch_scale_cpu_capacity() / (1 - rt_frac())
>>
>> > >
>> > >
>> > > On IRC I mentioned stopping the CFS clock when preempted, and while that
>> > > would result in fixed numbers, Vincent was right in pointing out the
>> > > numbers will be difficult to interpret, since the meaning will be purely
>> > > CPU local and I'm not sure you can actually fix it again with
>> > > normalization.
>> > >
>> > > Imagine, running a .3 RT task, that would push the (always running) CFS
>> > > down to .7, but because we discard all !cfs time, it actually has 1. If
>> > > we try and normalize that we'll end up with ~1.43, which is of course
>> > > completely broken.
>> > >
>> > >
>> > > _However_, all that happens for util, also happens for load. So the above
>> > > scenario will also make the CPU appear less loaded than it actually is.
>> >
>> > The load will continue to increase because we track runnable state and
>> > not running for the load
>>
>> Duh yes. So renormalizing it once, like proposed for util would actually
>> do the right thing there too.  Would not that allow us to get rid of
>> much of the capacity magic in the load balance code?
>>
>> /me thinks more..
>>
>> Bah, no.. because you don't want this dynamic renormalization part of
>> the sums. So you want to keep it after the fact. :/
>>
>> > As you mentioned, scale_rt_capacity give the remaining capacity for
>> > cfs and it will behave like cfs util_avg now that it uses PELT. So as
>> > long as cfs util_avg <  scale_rt_capacity(we probably need a margin)
>> > we keep using dl bandwidth + cfs util_avg + rt util_avg for selecting
>> > OPP because we have remaining spare capacity but if  cfs util_avg ==
>> > scale_rt_capacity, we make sure to use max OPP.
>>
>> Good point, when cfs-util < cfs-cap then there is idle time and the util
>> number is 'right', when cfs-util == cfs-cap we're overcommitted and
>> should go max.
>>
>> Since the util and cap values are aligned that should track nicely.
>
> So Vincent proposed to have a margin between cfs util and cfs cap to be
> sure there is a little bit of idle time. This is _exactly_ what the
> overutilized flag in EAS does. That would actually make a lot of sense
> to use that flag in schedutil. The idea is basically to say, if there
> isn't enough idle time on all CPUs, the util signal are kinda wrong, so
> let's not make any decisions (task placement or OPP selection) based on
> that. If overutilized, go to max freq. Does that make sense ?

Yes it's similar to the overutilized except that
- this is done per cpu and whereas overutilization is for the whole system
- the test is done at every freq update and not only during some cfs
event and it uses the last up to date value and not a periodically
updated snapshot of the value
- this is done also without EAS

Then for the margin, it has to be discussed if it is really needed or not

>
> Thanks,
> Quentin


Re: [PATCH 18/19] serdev: ttydev: Serdev driver that creates an standard TTY port

2018-06-06 Thread Ricardo Ribalda Delgado
On Wed, Jun 6, 2018 at 11:55 AM Andy Shevchenko
 wrote:
> >
> > https://github.com/ribalda/linux/tree/serdev2
>
> n _is_ default. So, just remove a line.
>

Done. Thanks!

> --
> With Best Regards,
> Andy Shevchenko



-- 
Ricardo Ribalda


Re: [PATCH 18/19] serdev: ttydev: Serdev driver that creates an standard TTY port

2018-06-06 Thread Ricardo Ribalda Delgado
On Wed, Jun 6, 2018 at 11:55 AM Andy Shevchenko
 wrote:
> >
> > https://github.com/ribalda/linux/tree/serdev2
>
> n _is_ default. So, just remove a line.
>

Done. Thanks!

> --
> With Best Regards,
> Andy Shevchenko



-- 
Ricardo Ribalda


[PATCH] ASoC: ak5558: make two structures static

2018-06-06 Thread Colin King
From: Colin Ian King 

The structure ak5558_pm and soc_codec_dev_ak5558 are local to the
source and do not need to be in global scope, so make them static.
Also make soc_codec_dev_ak5558 static.

Cleans up sparse warnings:
warning: symbol 'ak5558_pm' was not declared. Should it be static?
warning: symbol 'soc_codec_dev_ak5558' was not declared. Should it be
static?

Signed-off-by: Colin Ian King 
---
 sound/soc/codecs/ak5558.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/ak5558.c b/sound/soc/codecs/ak5558.c
index f4ed5cc40661..aebdd4f78f04 100644
--- a/sound/soc/codecs/ak5558.c
+++ b/sound/soc/codecs/ak5558.c
@@ -322,13 +322,13 @@ static int __maybe_unused ak5558_runtime_resume(struct 
device *dev)
return regcache_sync(ak5558->regmap);
 }
 
-const struct dev_pm_ops ak5558_pm = {
+static const struct dev_pm_ops ak5558_pm = {
SET_RUNTIME_PM_OPS(ak5558_runtime_suspend, ak5558_runtime_resume, NULL)
SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)
 };
 
-struct snd_soc_component_driver soc_codec_dev_ak5558 = {
+static const struct snd_soc_component_driver soc_codec_dev_ak5558 = {
.probe  = ak5558_probe,
.remove = ak5558_remove,
.controls   = ak5558_snd_controls,
-- 
2.17.0



Re: [PATCH v4 14/14] coresight: allow the coresight core driver to be built as a module

2018-06-06 Thread Suzuki K Poulose

On 06/05/2018 10:07 PM, Kim Phillips wrote:

Allow to build coresight as a module.  This enhances
coresight developer efficiency by allowing the development to
take place exclusively on the target, and without needing to
reboot in between changes.

- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to
   be called coresight by the Makefile
- modules can have only one init/exit, so we add the core bus
   register/unregister function calls to the etm_perf init/exit
   functions, since coresight.c does not have etm_pmu defined.


It feels a bit weird to move the coresight core specific steps to
etm_perf_init. Why not call etm_perf_init from coresight init routine
and keep everything core specific in the coresight-core.c ?


Suzuki


[PATCH] ASoC: ak5558: make two structures static

2018-06-06 Thread Colin King
From: Colin Ian King 

The structure ak5558_pm and soc_codec_dev_ak5558 are local to the
source and do not need to be in global scope, so make them static.
Also make soc_codec_dev_ak5558 static.

Cleans up sparse warnings:
warning: symbol 'ak5558_pm' was not declared. Should it be static?
warning: symbol 'soc_codec_dev_ak5558' was not declared. Should it be
static?

Signed-off-by: Colin Ian King 
---
 sound/soc/codecs/ak5558.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/ak5558.c b/sound/soc/codecs/ak5558.c
index f4ed5cc40661..aebdd4f78f04 100644
--- a/sound/soc/codecs/ak5558.c
+++ b/sound/soc/codecs/ak5558.c
@@ -322,13 +322,13 @@ static int __maybe_unused ak5558_runtime_resume(struct 
device *dev)
return regcache_sync(ak5558->regmap);
 }
 
-const struct dev_pm_ops ak5558_pm = {
+static const struct dev_pm_ops ak5558_pm = {
SET_RUNTIME_PM_OPS(ak5558_runtime_suspend, ak5558_runtime_resume, NULL)
SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)
 };
 
-struct snd_soc_component_driver soc_codec_dev_ak5558 = {
+static const struct snd_soc_component_driver soc_codec_dev_ak5558 = {
.probe  = ak5558_probe,
.remove = ak5558_remove,
.controls   = ak5558_snd_controls,
-- 
2.17.0



Re: [PATCH v4 14/14] coresight: allow the coresight core driver to be built as a module

2018-06-06 Thread Suzuki K Poulose

On 06/05/2018 10:07 PM, Kim Phillips wrote:

Allow to build coresight as a module.  This enhances
coresight developer efficiency by allowing the development to
take place exclusively on the target, and without needing to
reboot in between changes.

- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to
   be called coresight by the Makefile
- modules can have only one init/exit, so we add the core bus
   register/unregister function calls to the etm_perf init/exit
   functions, since coresight.c does not have etm_pmu defined.


It feels a bit weird to move the coresight core specific steps to
etm_perf_init. Why not call etm_perf_init from coresight init routine
and keep everything core specific in the coresight-core.c ?


Suzuki


Re: [PATCH 18/19] serdev: ttydev: Serdev driver that creates an standard TTY port

2018-06-06 Thread Andy Shevchenko
On Wed, Jun 6, 2018 at 10:47 AM, Ricardo Ribalda Delgado
 wrote:
> Hi Andy,
> On Wed, Jun 6, 2018 at 8:58 AM Ricardo Ribalda Delgado
>  wrote:

> I have defaulted the module to n as you suggested. You can take a look
> to the patches that I plan to submit tomorrow at:
>
> https://github.com/ribalda/linux/tree/serdev2

n _is_ default. So, just remove a line.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 18/19] serdev: ttydev: Serdev driver that creates an standard TTY port

2018-06-06 Thread Andy Shevchenko
On Wed, Jun 6, 2018 at 10:47 AM, Ricardo Ribalda Delgado
 wrote:
> Hi Andy,
> On Wed, Jun 6, 2018 at 8:58 AM Ricardo Ribalda Delgado
>  wrote:

> I have defaulted the module to n as you suggested. You can take a look
> to the patches that I plan to submit tomorrow at:
>
> https://github.com/ribalda/linux/tree/serdev2

n _is_ default. So, just remove a line.

-- 
With Best Regards,
Andy Shevchenko


[PATCH 2/4] serial: 8250_dw: allow enable rs485 at boot time

2018-06-06 Thread Giulio Benetti
If "linux,rs485-enabled-at-boot-time" is specified need to setup 485
in probe function.

Call uart_get_rs485_mode() to get rs485 configuration, then call
rs485_config() callback directly to setup port as rs485.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_dw.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c 
b/drivers/tty/serial/8250/8250_dw.c
index 45366e6e5411..0f8b4da03d4e 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -582,8 +582,11 @@ static int dw8250_probe(struct platform_device *pdev)
if (data->uart_16550_compatible)
p->handle_irq = NULL;
 
-   if (!data->skip_autocfg)
+   if (!data->skip_autocfg) {
dw8250_setup_port(p);
+   uart_get_rs485_mode(dev, >rs485);
+   dw8250_rs485_config(p, >rs485);
+   }
 
/* If we have a valid fifosize, try hooking up DMA */
if (p->fifosize) {
-- 
2.17.1



[PATCH 1/4] serial: 8250_dw: add em485 support

2018-06-06 Thread Giulio Benetti
Need to use rs485 transceiver so let's use existing em485 485 emulation
layer on top of 8250.

Add rs485_config callback to port.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_dw.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c 
b/drivers/tty/serial/8250/8250_dw.c
index 6fcdb90f616a..45366e6e5411 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -314,6 +314,36 @@ static void dw8250_set_ldisc(struct uart_port *p, struct 
ktermios *termios)
serial8250_do_set_ldisc(p, termios);
 }
 
+static int dw8250_rs485_config(struct uart_port *p,
+  struct serial_rs485 *rs485)
+{
+   struct uart_8250_port *up = up_to_u8250p(p);
+
+   /* Clamp the delays to [0, 100ms] */
+   rs485->delay_rts_before_send = min(rs485->delay_rts_before_send, 100U);
+   rs485->delay_rts_after_send  = min(rs485->delay_rts_after_send, 100U);
+
+   p->rs485 = *rs485;
+
+   /*
+* Both serial8250_em485_init and serial8250_em485_destroy
+* are idempotent
+*/
+   if (rs485->flags & SER_RS485_ENABLED) {
+   int ret = serial8250_em485_init(up);
+
+   if (ret) {
+   rs485->flags &= ~SER_RS485_ENABLED;
+   p->rs485.flags &= ~SER_RS485_ENABLED;
+   }
+   return ret;
+   }
+
+   serial8250_em485_destroy(up);
+
+   return 0;
+}
+
 /*
  * dw8250_fallback_dma_filter will prevent the UART from getting just any free
  * channel on platforms that have DMA engines, but don't have any channels
@@ -449,6 +479,7 @@ static int dw8250_probe(struct platform_device *pdev)
p->serial_out   = dw8250_serial_out;
p->set_ldisc= dw8250_set_ldisc;
p->set_termios  = dw8250_set_termios;
+   p->rs485_config = dw8250_rs485_config;
 
p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
if (!p->membase)
-- 
2.17.1



[PATCH 4/4] serial: 8250_dw: treat rpm suspend with -EBUSY if RS485 ON and RTS_AFTER_SEND

2018-06-06 Thread Giulio Benetti
If rs485 is on and RTS_AFTER_SEND it means that in idle state rts pin
must be asserted, othwerwise rs485 transceiver will enter tx state.
dw8250 when clocks are stopped keeps rts line de-asserted(high),
resulting in keeping rs485 transceiver in tx state when port is idle.

Check if rs485 is on with RTS_AFTER_SEND set, if so return -EBUSY in
rpm_suspend,

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_dw.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c 
b/drivers/tty/serial/8250/8250_dw.c
index 888280ff5451..6b0ee6dc8ad0 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -668,6 +668,11 @@ static int dw8250_resume(struct device *dev)
 static int dw8250_runtime_suspend(struct device *dev)
 {
struct dw8250_data *data = dev_get_drvdata(dev);
+   struct uart_8250_port *up = serial8250_get_port(data->line);
+   struct uart_port *p = >port;
+
+   if (p->rs485.flags & (SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND))
+   return -EBUSY;
 
if (!IS_ERR(data->clk))
clk_disable_unprepare(data->clk);
-- 
2.17.1



[PATCH 2/4] serial: 8250_dw: allow enable rs485 at boot time

2018-06-06 Thread Giulio Benetti
If "linux,rs485-enabled-at-boot-time" is specified need to setup 485
in probe function.

Call uart_get_rs485_mode() to get rs485 configuration, then call
rs485_config() callback directly to setup port as rs485.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_dw.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c 
b/drivers/tty/serial/8250/8250_dw.c
index 45366e6e5411..0f8b4da03d4e 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -582,8 +582,11 @@ static int dw8250_probe(struct platform_device *pdev)
if (data->uart_16550_compatible)
p->handle_irq = NULL;
 
-   if (!data->skip_autocfg)
+   if (!data->skip_autocfg) {
dw8250_setup_port(p);
+   uart_get_rs485_mode(dev, >rs485);
+   dw8250_rs485_config(p, >rs485);
+   }
 
/* If we have a valid fifosize, try hooking up DMA */
if (p->fifosize) {
-- 
2.17.1



[PATCH 1/4] serial: 8250_dw: add em485 support

2018-06-06 Thread Giulio Benetti
Need to use rs485 transceiver so let's use existing em485 485 emulation
layer on top of 8250.

Add rs485_config callback to port.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_dw.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c 
b/drivers/tty/serial/8250/8250_dw.c
index 6fcdb90f616a..45366e6e5411 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -314,6 +314,36 @@ static void dw8250_set_ldisc(struct uart_port *p, struct 
ktermios *termios)
serial8250_do_set_ldisc(p, termios);
 }
 
+static int dw8250_rs485_config(struct uart_port *p,
+  struct serial_rs485 *rs485)
+{
+   struct uart_8250_port *up = up_to_u8250p(p);
+
+   /* Clamp the delays to [0, 100ms] */
+   rs485->delay_rts_before_send = min(rs485->delay_rts_before_send, 100U);
+   rs485->delay_rts_after_send  = min(rs485->delay_rts_after_send, 100U);
+
+   p->rs485 = *rs485;
+
+   /*
+* Both serial8250_em485_init and serial8250_em485_destroy
+* are idempotent
+*/
+   if (rs485->flags & SER_RS485_ENABLED) {
+   int ret = serial8250_em485_init(up);
+
+   if (ret) {
+   rs485->flags &= ~SER_RS485_ENABLED;
+   p->rs485.flags &= ~SER_RS485_ENABLED;
+   }
+   return ret;
+   }
+
+   serial8250_em485_destroy(up);
+
+   return 0;
+}
+
 /*
  * dw8250_fallback_dma_filter will prevent the UART from getting just any free
  * channel on platforms that have DMA engines, but don't have any channels
@@ -449,6 +479,7 @@ static int dw8250_probe(struct platform_device *pdev)
p->serial_out   = dw8250_serial_out;
p->set_ldisc= dw8250_set_ldisc;
p->set_termios  = dw8250_set_termios;
+   p->rs485_config = dw8250_rs485_config;
 
p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
if (!p->membase)
-- 
2.17.1



[PATCH 4/4] serial: 8250_dw: treat rpm suspend with -EBUSY if RS485 ON and RTS_AFTER_SEND

2018-06-06 Thread Giulio Benetti
If rs485 is on and RTS_AFTER_SEND it means that in idle state rts pin
must be asserted, othwerwise rs485 transceiver will enter tx state.
dw8250 when clocks are stopped keeps rts line de-asserted(high),
resulting in keeping rs485 transceiver in tx state when port is idle.

Check if rs485 is on with RTS_AFTER_SEND set, if so return -EBUSY in
rpm_suspend,

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_dw.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c 
b/drivers/tty/serial/8250/8250_dw.c
index 888280ff5451..6b0ee6dc8ad0 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -668,6 +668,11 @@ static int dw8250_resume(struct device *dev)
 static int dw8250_runtime_suspend(struct device *dev)
 {
struct dw8250_data *data = dev_get_drvdata(dev);
+   struct uart_8250_port *up = serial8250_get_port(data->line);
+   struct uart_port *p = >port;
+
+   if (p->rs485.flags & (SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND))
+   return -EBUSY;
 
if (!IS_ERR(data->clk))
clk_disable_unprepare(data->clk);
-- 
2.17.1



[PATCH 3/4] serial: 8250: Handle case port doesn't have TEMT interrupt using em485.

2018-06-06 Thread Giulio Benetti
Some 8250 ports only have TEMT interrupt, so current implementation
can't work for ports without it. The only chance to make it work is to
loop-read on LSR register.

With NO TEMT interrupt check if both TEMT and THRE are set looping on
LSR register.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250.h  |  2 +-
 drivers/tty/serial/8250/8250_dw.c   |  2 +-
 drivers/tty/serial/8250/8250_omap.c |  2 +-
 drivers/tty/serial/8250/8250_port.c | 26 ++
 include/linux/serial_8250.h |  1 +
 5 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index ebfb0bd5bef5..5c6ae5f69432 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -136,7 +136,7 @@ void serial8250_rpm_put(struct uart_8250_port *p);
 void serial8250_rpm_get_tx(struct uart_8250_port *p);
 void serial8250_rpm_put_tx(struct uart_8250_port *p);
 
-int serial8250_em485_init(struct uart_8250_port *p);
+int serial8250_em485_init(struct uart_8250_port *p, bool has_temt_isr);
 void serial8250_em485_destroy(struct uart_8250_port *p);
 
 static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
diff --git a/drivers/tty/serial/8250/8250_dw.c 
b/drivers/tty/serial/8250/8250_dw.c
index 0f8b4da03d4e..888280ff5451 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -330,7 +330,7 @@ static int dw8250_rs485_config(struct uart_port *p,
 * are idempotent
 */
if (rs485->flags & SER_RS485_ENABLED) {
-   int ret = serial8250_em485_init(up);
+   int ret = serial8250_em485_init(up, false);
 
if (ret) {
rs485->flags &= ~SER_RS485_ENABLED;
diff --git a/drivers/tty/serial/8250/8250_omap.c 
b/drivers/tty/serial/8250/8250_omap.c
index 624b501fd253..ab483c8b30c8 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -725,7 +725,7 @@ static int omap_8250_rs485_config(struct uart_port *port,
 * are idempotent
 */
if (rs485->flags & SER_RS485_ENABLED) {
-   int ret = serial8250_em485_init(up);
+   int ret = serial8250_em485_init(up, true);
 
if (ret) {
rs485->flags &= ~SER_RS485_ENABLED;
diff --git a/drivers/tty/serial/8250/8250_port.c 
b/drivers/tty/serial/8250/8250_port.c
index c8c10b5ec6d6..8432445c80e5 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -602,15 +602,16 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put);
 /**
  * serial8250_em485_init() - put uart_8250_port into rs485 emulating
  * @p: uart_8250_port port instance
+ * @p: bool specify if 8250 port has TEMT interrupt or not
  *
  * The function is used to start rs485 software emulating on the
  *  uart_8250_port* @p. Namely, RTS is switched before/after
  * transmission. The function is idempotent, so it is safe to call it
  * multiple times.
  *
- * The caller MUST enable interrupt on empty shift register before
- * calling serial8250_em485_init(). This interrupt is not a part of
- * 8250 standard, but implementation defined.
+ * If has_temt_isr is passed as true, the caller MUST enable interrupt
+ * on empty shift register before calling serial8250_em485_init().
+ * This interrupt is not a part of 8250 standard, but implementation 
defined.
  *
  * The function is supposed to be called from .rs485_config callback
  * or from any other callback protected with p->port.lock spinlock.
@@ -619,7 +620,7 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put);
  *
  * Return 0 - success, -errno - otherwise
  */
-int serial8250_em485_init(struct uart_8250_port *p)
+int serial8250_em485_init(struct uart_8250_port *p, bool has_temt_isr)
 {
if (p->em485)
return 0;
@@ -636,6 +637,7 @@ int serial8250_em485_init(struct uart_8250_port *p)
p->em485->start_tx_timer.function = _em485_handle_start_tx;
p->em485->port = p;
p->em485->active_timer = NULL;
+   p->em485->has_temt_isr = has_temt_isr;
serial8250_em485_rts_after_send(p);
 
return 0;
@@ -1520,11 +1522,19 @@ static inline void __stop_tx(struct uart_8250_port *p)
/*
 * To provide required timeing and allow FIFO transfer,
 * __stop_tx_rs485() must be called only when both FIFO and
-* shift register are empty. It is for device driver to enable
-* interrupt on TEMT.
+* shift register are empty. If 8250 port supports it,
+* it is for device driver to enable interrupt on TEMT.
+* Otherwise must loop-read until TEMT and THRE flags are set.
 */
-   if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
-   return;
+   if (em485->has_temt_isr) {
+ 

[PATCH 3/4] serial: 8250: Handle case port doesn't have TEMT interrupt using em485.

2018-06-06 Thread Giulio Benetti
Some 8250 ports only have TEMT interrupt, so current implementation
can't work for ports without it. The only chance to make it work is to
loop-read on LSR register.

With NO TEMT interrupt check if both TEMT and THRE are set looping on
LSR register.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250.h  |  2 +-
 drivers/tty/serial/8250/8250_dw.c   |  2 +-
 drivers/tty/serial/8250/8250_omap.c |  2 +-
 drivers/tty/serial/8250/8250_port.c | 26 ++
 include/linux/serial_8250.h |  1 +
 5 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index ebfb0bd5bef5..5c6ae5f69432 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -136,7 +136,7 @@ void serial8250_rpm_put(struct uart_8250_port *p);
 void serial8250_rpm_get_tx(struct uart_8250_port *p);
 void serial8250_rpm_put_tx(struct uart_8250_port *p);
 
-int serial8250_em485_init(struct uart_8250_port *p);
+int serial8250_em485_init(struct uart_8250_port *p, bool has_temt_isr);
 void serial8250_em485_destroy(struct uart_8250_port *p);
 
 static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
diff --git a/drivers/tty/serial/8250/8250_dw.c 
b/drivers/tty/serial/8250/8250_dw.c
index 0f8b4da03d4e..888280ff5451 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -330,7 +330,7 @@ static int dw8250_rs485_config(struct uart_port *p,
 * are idempotent
 */
if (rs485->flags & SER_RS485_ENABLED) {
-   int ret = serial8250_em485_init(up);
+   int ret = serial8250_em485_init(up, false);
 
if (ret) {
rs485->flags &= ~SER_RS485_ENABLED;
diff --git a/drivers/tty/serial/8250/8250_omap.c 
b/drivers/tty/serial/8250/8250_omap.c
index 624b501fd253..ab483c8b30c8 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -725,7 +725,7 @@ static int omap_8250_rs485_config(struct uart_port *port,
 * are idempotent
 */
if (rs485->flags & SER_RS485_ENABLED) {
-   int ret = serial8250_em485_init(up);
+   int ret = serial8250_em485_init(up, true);
 
if (ret) {
rs485->flags &= ~SER_RS485_ENABLED;
diff --git a/drivers/tty/serial/8250/8250_port.c 
b/drivers/tty/serial/8250/8250_port.c
index c8c10b5ec6d6..8432445c80e5 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -602,15 +602,16 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put);
 /**
  * serial8250_em485_init() - put uart_8250_port into rs485 emulating
  * @p: uart_8250_port port instance
+ * @p: bool specify if 8250 port has TEMT interrupt or not
  *
  * The function is used to start rs485 software emulating on the
  *  uart_8250_port* @p. Namely, RTS is switched before/after
  * transmission. The function is idempotent, so it is safe to call it
  * multiple times.
  *
- * The caller MUST enable interrupt on empty shift register before
- * calling serial8250_em485_init(). This interrupt is not a part of
- * 8250 standard, but implementation defined.
+ * If has_temt_isr is passed as true, the caller MUST enable interrupt
+ * on empty shift register before calling serial8250_em485_init().
+ * This interrupt is not a part of 8250 standard, but implementation 
defined.
  *
  * The function is supposed to be called from .rs485_config callback
  * or from any other callback protected with p->port.lock spinlock.
@@ -619,7 +620,7 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put);
  *
  * Return 0 - success, -errno - otherwise
  */
-int serial8250_em485_init(struct uart_8250_port *p)
+int serial8250_em485_init(struct uart_8250_port *p, bool has_temt_isr)
 {
if (p->em485)
return 0;
@@ -636,6 +637,7 @@ int serial8250_em485_init(struct uart_8250_port *p)
p->em485->start_tx_timer.function = _em485_handle_start_tx;
p->em485->port = p;
p->em485->active_timer = NULL;
+   p->em485->has_temt_isr = has_temt_isr;
serial8250_em485_rts_after_send(p);
 
return 0;
@@ -1520,11 +1522,19 @@ static inline void __stop_tx(struct uart_8250_port *p)
/*
 * To provide required timeing and allow FIFO transfer,
 * __stop_tx_rs485() must be called only when both FIFO and
-* shift register are empty. It is for device driver to enable
-* interrupt on TEMT.
+* shift register are empty. If 8250 port supports it,
+* it is for device driver to enable interrupt on TEMT.
+* Otherwise must loop-read until TEMT and THRE flags are set.
 */
-   if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
-   return;
+   if (em485->has_temt_isr) {
+ 

Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver

2018-06-06 Thread Sricharan R
Hi Vinod,

On 6/6/2018 12:19 PM, Vinod wrote:
> Hi Sricharan,
> 
> On 06-06-18, 12:09, Sricharan R wrote:
> 
>> +config QCOM_Q6V5_WCSS
>> +tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader"
>> +depends on OF && ARCH_QCOM
>> +depends on QCOM_SMEM
>> +depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
>> +depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
>
> Is there a reason why it depends on RPMSG_QCOM_GLINK_SMEM=n? What would
> happen if distro wants both this and RPMSG_QCOM_GLINK_SMEM
>
   RPMSG_QCOM_GLINK_SMEM=n should be for the COMPILE_TEST. Probably that
>>>
>>> why would that be a limitation? I am more worried about
>>> RPMSG_QCOM_GLINK_SMEM=n being the condition here. In new drivers we
>>> should not typically have dependency on some symbol being not there
>>
>> Without that, if RPMSG_QCOM_GLINK_SMEM=m is compiled as a module, then
>> it would break the build.
> 
> Okay I do not know the details, but that doesn't sound correct to me.
> Breaking build sounds a bit extreme to me. Can you give details on this
> part..
> 

 Having, just, depends on RPMSG_QCOM_GLINK_SMEM || COMPILE_TEST,
 is going to break when RPMSG_QCOM_GLINK_SMEM=m and COMPILE_TEST=y.
 Hence the COMPILE_TEST && RPMSG_QCOM_GLINK_SMEM=n.

 Having said that, COMPILE_TEST is getting tested for RPMSG_QCOM_SMD=n in
 the previous line. So that's the reason for not having it in below line for
 RPMSG_QCOM_GLINK_SMEM.

   means that it should be corrected here and for ADSP, Q6V5_PIL as well.
   Bjorn, is that correct ?, should it be, below ?
  
   depends on (RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)) || 
 (RPMSG_QCOM_GLINK_SMEM || (COMPILE_TEST && RPMSG_QCOM_GLINK_SMEM=n))
>>>
>>> that doesnt really sound good :(
>>
>>  Hmm, but i was thinking it should functionally depend on either SMD or 
>> GLINK and not both.
> 
> If you are depedent upon a symbol provided by a module you should say
> depends on. If a machine is not supposed to have both SMD or GLINK then
> the driver will not get probed.
> 

This is where, i was thinking, it should be functional if either of SMD or GLINK
is there, but should not require both.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of 
Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus



Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver

2018-06-06 Thread Sricharan R
Hi Vinod,

On 6/6/2018 12:19 PM, Vinod wrote:
> Hi Sricharan,
> 
> On 06-06-18, 12:09, Sricharan R wrote:
> 
>> +config QCOM_Q6V5_WCSS
>> +tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader"
>> +depends on OF && ARCH_QCOM
>> +depends on QCOM_SMEM
>> +depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
>> +depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
>
> Is there a reason why it depends on RPMSG_QCOM_GLINK_SMEM=n? What would
> happen if distro wants both this and RPMSG_QCOM_GLINK_SMEM
>
   RPMSG_QCOM_GLINK_SMEM=n should be for the COMPILE_TEST. Probably that
>>>
>>> why would that be a limitation? I am more worried about
>>> RPMSG_QCOM_GLINK_SMEM=n being the condition here. In new drivers we
>>> should not typically have dependency on some symbol being not there
>>
>> Without that, if RPMSG_QCOM_GLINK_SMEM=m is compiled as a module, then
>> it would break the build.
> 
> Okay I do not know the details, but that doesn't sound correct to me.
> Breaking build sounds a bit extreme to me. Can you give details on this
> part..
> 

 Having, just, depends on RPMSG_QCOM_GLINK_SMEM || COMPILE_TEST,
 is going to break when RPMSG_QCOM_GLINK_SMEM=m and COMPILE_TEST=y.
 Hence the COMPILE_TEST && RPMSG_QCOM_GLINK_SMEM=n.

 Having said that, COMPILE_TEST is getting tested for RPMSG_QCOM_SMD=n in
 the previous line. So that's the reason for not having it in below line for
 RPMSG_QCOM_GLINK_SMEM.

   means that it should be corrected here and for ADSP, Q6V5_PIL as well.
   Bjorn, is that correct ?, should it be, below ?
  
   depends on (RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)) || 
 (RPMSG_QCOM_GLINK_SMEM || (COMPILE_TEST && RPMSG_QCOM_GLINK_SMEM=n))
>>>
>>> that doesnt really sound good :(
>>
>>  Hmm, but i was thinking it should functionally depend on either SMD or 
>> GLINK and not both.
> 
> If you are depedent upon a symbol provided by a module you should say
> depends on. If a machine is not supposed to have both SMD or GLINK then
> the driver will not get probed.
> 

This is where, i was thinking, it should be functional if either of SMD or GLINK
is there, but should not require both.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of 
Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus



[PATCH 2/4] serial: 8250: Copy mctrl when register port.

2018-06-06 Thread Giulio Benetti
RS485 can modify mctrl on startup, especially when RTS_AFTER_SEND is on
TIOCM_RTS is set, then need to keep it set when registering port.

Copy mctrl to new port too.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/8250/8250_core.c 
b/drivers/tty/serial/8250/8250_core.c
index c8c2b260c681..c8e62fbd6570 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -993,6 +993,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart->port.unthrottle   = up->port.unthrottle;
uart->port.rs485_config = up->port.rs485_config;
uart->port.rs485= up->port.rs485;
+   uart->port.mctrl= up->port.mctrl;
uart->dma   = up->dma;
uart->em485 = up->em485;
 
-- 
2.17.1



[PATCH 1/4] serial: 8250: Copy em485 from port to real port.

2018-06-06 Thread Giulio Benetti
em485 gets lost during serial8250_register_8250_port().

Copy em485 to final uart port.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/8250/8250_core.c 
b/drivers/tty/serial/8250/8250_core.c
index 9342fc2ee7df..c8c2b260c681 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -994,6 +994,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart->port.rs485_config = up->port.rs485_config;
uart->port.rs485= up->port.rs485;
uart->dma   = up->dma;
+   uart->em485 = up->em485;
 
/* Take tx_loadsz from fifosize if it wasn't set separately */
if (uart->port.fifosize && !uart->tx_loadsz)
-- 
2.17.1



[PATCH 2/4] serial: 8250: Copy mctrl when register port.

2018-06-06 Thread Giulio Benetti
RS485 can modify mctrl on startup, especially when RTS_AFTER_SEND is on
TIOCM_RTS is set, then need to keep it set when registering port.

Copy mctrl to new port too.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/8250/8250_core.c 
b/drivers/tty/serial/8250/8250_core.c
index c8c2b260c681..c8e62fbd6570 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -993,6 +993,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart->port.unthrottle   = up->port.unthrottle;
uart->port.rs485_config = up->port.rs485_config;
uart->port.rs485= up->port.rs485;
+   uart->port.mctrl= up->port.mctrl;
uart->dma   = up->dma;
uart->em485 = up->em485;
 
-- 
2.17.1



[PATCH 1/4] serial: 8250: Copy em485 from port to real port.

2018-06-06 Thread Giulio Benetti
em485 gets lost during serial8250_register_8250_port().

Copy em485 to final uart port.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/8250/8250_core.c 
b/drivers/tty/serial/8250/8250_core.c
index 9342fc2ee7df..c8c2b260c681 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -994,6 +994,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart->port.rs485_config = up->port.rs485_config;
uart->port.rs485= up->port.rs485;
uart->dma   = up->dma;
+   uart->em485 = up->em485;
 
/* Take tx_loadsz from fifosize if it wasn't set separately */
if (uart->port.fifosize && !uart->tx_loadsz)
-- 
2.17.1



[PATCH 4/4] serial: core: Mask mctrl with TIOCM_RTS too if rs485 on and RTS_AFTER_SEND set.

2018-06-06 Thread Giulio Benetti
If rs485 is enabled and RTS_AFTER_SEND is set on startup need to keep
TIOCM_RTS asserted to keep rs485 transceiver in RX when idle.

Check if rs485 is on and RTS_AFTER_SEND is set and mask port->mctrl with
TIOCM_RTS too and not only TIOCM_DTR.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/serial_core.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0466f9f08a91..06d9441f6d20 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2288,6 +2288,16 @@ uart_configure_port(struct uart_driver *drv, struct 
uart_state *state,
 
if (port->type != PORT_UNKNOWN) {
unsigned long flags;
+   int rs485_on = port->rs485_config &&
+   (port->rs485.flags & SER_RS485_ENABLED);
+   int RTS_after_send = !!(port->rs485.flags &
+   SER_RS485_RTS_AFTER_SEND);
+   int mctrl;
+
+   if (rs485_on && RTS_after_send)
+   mctrl = port->mctrl & (TIOCM_DTR | TIOCM_RTS);
+   else
+   mctrl = port->mctrl & TIOCM_DTR;
 
uart_report_port(drv, port);
 
@@ -2300,7 +2310,7 @@ uart_configure_port(struct uart_driver *drv, struct 
uart_state *state,
 * We probably don't need a spinlock around this, but
 */
spin_lock_irqsave(>lock, flags);
-   port->ops->set_mctrl(port, port->mctrl & TIOCM_DTR);
+   port->ops->set_mctrl(port, mctrl);
spin_unlock_irqrestore(>lock, flags);
 
/*
-- 
2.17.1



[PATCH 4/4] serial: core: Mask mctrl with TIOCM_RTS too if rs485 on and RTS_AFTER_SEND set.

2018-06-06 Thread Giulio Benetti
If rs485 is enabled and RTS_AFTER_SEND is set on startup need to keep
TIOCM_RTS asserted to keep rs485 transceiver in RX when idle.

Check if rs485 is on and RTS_AFTER_SEND is set and mask port->mctrl with
TIOCM_RTS too and not only TIOCM_DTR.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/serial_core.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0466f9f08a91..06d9441f6d20 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2288,6 +2288,16 @@ uart_configure_port(struct uart_driver *drv, struct 
uart_state *state,
 
if (port->type != PORT_UNKNOWN) {
unsigned long flags;
+   int rs485_on = port->rs485_config &&
+   (port->rs485.flags & SER_RS485_ENABLED);
+   int RTS_after_send = !!(port->rs485.flags &
+   SER_RS485_RTS_AFTER_SEND);
+   int mctrl;
+
+   if (rs485_on && RTS_after_send)
+   mctrl = port->mctrl & (TIOCM_DTR | TIOCM_RTS);
+   else
+   mctrl = port->mctrl & TIOCM_DTR;
 
uart_report_port(drv, port);
 
@@ -2300,7 +2310,7 @@ uart_configure_port(struct uart_driver *drv, struct 
uart_state *state,
 * We probably don't need a spinlock around this, but
 */
spin_lock_irqsave(>lock, flags);
-   port->ops->set_mctrl(port, port->mctrl & TIOCM_DTR);
+   port->ops->set_mctrl(port, mctrl);
spin_unlock_irqrestore(>lock, flags);
 
/*
-- 
2.17.1



[PATCH 3/4] serial: 8250: Make em485_rts_after_send() set mctrl according to rts state.

2018-06-06 Thread Giulio Benetti
When rs485 enabled and RTS_AFTER_SEND set on startup, need to preserve
mctrl status, because later functions will call set_mctrl passing
port->mctrl=0 overriding rts status, resulting in rts pin in
transmission when idle.

Make mctrl reflect rts pin state.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_port.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c 
b/drivers/tty/serial/8250/8250_port.c
index 95833cbc4338..c8c10b5ec6d6 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -562,10 +562,13 @@ static inline void serial8250_em485_rts_after_send(struct 
uart_8250_port *p)
 {
unsigned char mcr = serial8250_in_MCR(p);
 
-   if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND)
+   if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
mcr |= UART_MCR_RTS;
-   else
+   p->port.mctrl |= TIOCM_RTS;
+   } else {
mcr &= ~UART_MCR_RTS;
+   p->port.mctrl &= ~TIOCM_RTS;
+   }
serial8250_out_MCR(p, mcr);
 }
 
-- 
2.17.1



[PATCH 3/4] serial: 8250: Make em485_rts_after_send() set mctrl according to rts state.

2018-06-06 Thread Giulio Benetti
When rs485 enabled and RTS_AFTER_SEND set on startup, need to preserve
mctrl status, because later functions will call set_mctrl passing
port->mctrl=0 overriding rts status, resulting in rts pin in
transmission when idle.

Make mctrl reflect rts pin state.

Signed-off-by: Giulio Benetti 
---
 drivers/tty/serial/8250/8250_port.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c 
b/drivers/tty/serial/8250/8250_port.c
index 95833cbc4338..c8c10b5ec6d6 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -562,10 +562,13 @@ static inline void serial8250_em485_rts_after_send(struct 
uart_8250_port *p)
 {
unsigned char mcr = serial8250_in_MCR(p);
 
-   if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND)
+   if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
mcr |= UART_MCR_RTS;
-   else
+   p->port.mctrl |= TIOCM_RTS;
+   } else {
mcr &= ~UART_MCR_RTS;
+   p->port.mctrl &= ~TIOCM_RTS;
+   }
serial8250_out_MCR(p, mcr);
 }
 
-- 
2.17.1



Re: linux-next: manual merge of the staging tree with Linus' tree

2018-06-06 Thread Greg KH
On Wed, Jun 06, 2018 at 07:12:05PM +1000, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the staging tree got conflicts in:
> 
>   drivers/staging/ipx/af_ipx.c
>   drivers/staging/ipx/ipx_proc.c
> 
> between commits:
> 
>   fddda2b7b521 ("proc: introduce proc_create_seq{,_data}")
>   db5051ead64a ("net: convert datagram_poll users tp ->poll_mask")
> 
> from Linus' tree and commit:
> 
>   7a2e838d28cf ("staging: ipx: delete it from the tree")
> 
> from the staging tree.
> 
> I fixed it up (I just removed the files) and can carry the fix as
> necessary. This is now fixed as far as linux-next is concerned, but any
> non trivial conflicts should be mentioned to your upstream maintainer
> when your tree is submitted for merging.  You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.

Thanks, those files should just be removed.

greg k-h


Re: linux-next: manual merge of the staging tree with Linus' tree

2018-06-06 Thread Greg KH
On Wed, Jun 06, 2018 at 07:12:05PM +1000, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the staging tree got conflicts in:
> 
>   drivers/staging/ipx/af_ipx.c
>   drivers/staging/ipx/ipx_proc.c
> 
> between commits:
> 
>   fddda2b7b521 ("proc: introduce proc_create_seq{,_data}")
>   db5051ead64a ("net: convert datagram_poll users tp ->poll_mask")
> 
> from Linus' tree and commit:
> 
>   7a2e838d28cf ("staging: ipx: delete it from the tree")
> 
> from the staging tree.
> 
> I fixed it up (I just removed the files) and can carry the fix as
> necessary. This is now fixed as far as linux-next is concerned, but any
> non trivial conflicts should be mentioned to your upstream maintainer
> when your tree is submitted for merging.  You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.

Thanks, those files should just be removed.

greg k-h


<    4   5   6   7   8   9   10   11   12   >