Re: [PATCH 1/1] lib: use sunday algorithm to do strstr() and strnstr()

[Zhaoxiu Zeng]

在 2018/7/28 22:38, Greg Kroah-Hartman 写道:
> On Sat, Jul 28, 2018 at 10:02:51PM +0800, Zhaoxiu Zeng wrote:
>> 在 2018/7/27 18:39, Andy Shevchenko 写道:
>>> On Fri, Jul 27, 2018 at 8:48 AM, Zhaoxiu Zeng  wrote:
 在 2018/7/27 1:17, Zhaoxiu Zeng 写道:
> 在 2018/7/23 2:37, Greg Kroah-Hartman 写道:
>> On Mon, Jul 23, 2018 at 01:37:15AM +0800, Zhaoxiu Zeng wrote:
>>>
>>> The Sunday algorithm is a variation of Boyer-Moore algorithm, it is 
>>> easy and fast.
>>> For the Sunday algorithm, to see
>>> http://www.inf.fh-flensburg.de/lang/algorithmen/pattern/sundayen.htm
>>
>> So you say, but what does this really buy us?  Why make this change?
>> How was it tested?  What is the downside of not taking this?
>>>
> I use the following program to test on fc28.
> Compile with O2, the new version is almost 2X faster than the original.
>>>
> The code size of the original is 0x80, the newer is 0xB0.
>>>
>>> So, output of bloat-o-meter would be good to have in commit message.
>>>
> The test result:
>>>
>>> Compact performance statistics as well.
>>>
> Thanks!
>>>
 The original strnstr might has a bug too!
 For example, assume s1 is "123\0abc" and s2 is "abc\0",
 call strnstr(s1, s2, 7) will return &s1[4], but the correct result is NULL.
>>>
>>> If there is a bug, send another patch to fix the bug first.
>>>
>>
>> The bug could be fixed by this patch.
> 
> Given that there doesn't seem to be a good reason to take your patch
> yet, that might be hard :)
> 
> You need to convince us that the patch is a valid thing to accept, by
> writing a correct changelog and showing proof of its correctness as this
> is modifying a core function in the kernel.
> 
> thanks,
> 
> greg k-h
> 

Thanks for responses!
I wrote a new changelog as described below. If it's ok, I will commit a new 
patch.
The details are as follow:


The Sunday algorithm is a variation of Boyer-Moore algorithm, it is easy and 
fast.
For the Sunday algorithm, to see
http://www.inf.fh-flensburg.de/lang/algorithmen/pattern/sundayen.htm

The new strstr implementation steps:
1) Gets the pattern length, and is denoted as l2.
2) Compares the first l2 bytes of the text with the pattern.
   If the comparison has matched, returns the pointer to starting of 
the text.
   Else, gets the index of the symbol which causes a mismatch, and is 
denoted as i.
3) Scans the text from index i to l2, to test whether the text is 
terminated.
   If the text is terminated early, returns NULL.
   Else, gets the symbol at index l2, and is denoted K.
4) Finds the index of K's last presence in the pattern, and is denoted 
as j.
   If there is no occurence of K in the pattern, j is -1.
5) Shifts the text window "l2 - j" bytes, and repeats step 2.

The original strstr scans the text two times. The 1st time gets the text length,
the 2nd time does the matches. The new strstr scans the text only one time.

The original strnstr has a bug. If the text length is smaller than the argument 
"len",
it maybe returns a wrong result. The bug could be fixed by this patch too.

I have tested using the following program on fc28.x86_64.
When compile with O2, the new version is almost 2X faster than the original,
the code size of the original strstr is 0x80 and the newer is 0xB0.


#include 
#include 
#include 
#include 
#include 
#include 

/**
 * strstr - Find the first substring in a %NUL terminated string
 * @s1: The string to be searched
 * @s2: The string to search for
 */
char *strstr1(const char *s1, const char *s2)
{
size_t l1, l2;

l2 = strlen(s2);
if (!l2)
return (char *)s1;
l1 = strlen(s1);
while (l1 >= l2) {
l1--;
if (!memcmp(s1, s2, l2))
return (char *)s1;
s1++;
}
return NULL;
}

char *strstr2(const char *s1, const char *s2)
{
size_t l2;
const char *pchk = s1;

#if 1//def __HAVE_ARCH_STRLEN
l2 = strlen(s2);
#else
for (l2 = 0; s2[l2] != 0; l2++)
/* nothing */;
#endif
if (!l2)
return (char *)s1;

/*
 * Sunday matching
 */
while (1) {
size_t i;
char k;

/* compare */
for (i = 0; i < l2; i++) {
if (s1[i] != s2[i])
break;
}
if (i >= l2)
return (char *)s1;

/* if s1 terminate early? */
if (pchk < s1 + i)
pchk = s1 + i;
do {
k = *pchk;
if (k == 0)
return NULL;
} while (pchk++ != s1 + l2);

/* find the k's last presence in s2 (k = s1[l2]) */
for

Re: [PATCH 01/10] staging:rtl8192u: Remove typedef of struct cmpk_txfb_t - Style

[Greg KH]

On Wed, Jul 25, 2018 at 11:16:21PM +0100, John Whitmore wrote:
> -typedef struct tag_cmd_pkt_tx_feedback {
> +struct cmpk_txfb_t {
>   /* DWORD 0 */
>   u8  element_id; /* Command packet type. */
>   u8  length; /* Command packet length. */
> @@ -53,7 +53,7 @@ typedef struct tag_cmd_pkt_tx_feedback {
>   /* DWORD 5 */
>   u16 reserve3;
>   u16 duration;
> -} cmpk_txfb_t;
> +};

People use the "_t" to try to denote a "typedef".  When converting to
just a structure, there is no need to keep the _t anymore at all, so
this should just be "struct cmpk_txfb".

Or just drop the typedef and use the name for the struct they had here
already, "struct tag_cmd_pkt_tx_feedback", that also would work.

Same for all of the other patches in this series.

thanks,

greg k-h

Re: [PATCH 02/10] staging:rtl8192u: Refactor use of enum dm_dig_sta_e - Style

[Greg KH]

On Sat, Jul 28, 2018 at 12:28:18AM +0100, John Whitmore wrote:
> Refactor the use of the enumerated type dm_dig_sta_e, which is not
> actually used for type checking by the compiler.
> 
> The enumerated type defines values for the enumeration, which are used
> by both dig_state and dig_highpwr_state, (members of the struct dig).
> Both of those variables were defined as being of type u8. This negates
> any usefulness of the use of the enumeration, (compiler type checking).
> 
> To make use of the compiler's type-checking the two member variables,
> dig_state and dig_highpwr_state have been changed to being of type
> enum dm_dig_sta_e. The enumerated type has been moved above the
> struct dig definition so that the enumeration is already defined when
> compiler reaches the two types using the enumerated type.
> 
> In addition the 'typedef' of the enumerated type has been removed to
> clear the checkpatch issue with declaring new types.
> 
> These changes, whilst convoluted, are purely coding style in nature and
> should not impact runtime code execution.
> 
> Signed-off-by: John Whitmore 
> ---
>  drivers/staging/rtl8192u/r8192U_dm.h | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/r8192U_dm.h 
> b/drivers/staging/rtl8192u/r8192U_dm.h
> index e86dda99c223..2444e1c1357b 100644
> --- a/drivers/staging/rtl8192u/r8192U_dm.h
> +++ b/drivers/staging/rtl8192u/r8192U_dm.h
> @@ -64,6 +64,13 @@
>  
>  
>  /*--Define 
> structure*/
> +
> +enum dm_dig_sta_e {

Don't end an enum with "_e", as that's not needed at all.

thanks,

greg k-h

Re: [PATCH 29/38] vfs: syscall: Add fsconfig() for configuring and managing a context [ver #10]

[David Howells]

Jann Horn  wrote:

> [...]
> > +   case fsconfig_set_binary:
> > +   param.type = fs_value_is_blob;
> > +   param.size = aux;
> > +   param.blob = memdup_user_nul(_value, aux);
> > +   if (IS_ERR(param.blob)) {
> > +   ret = PTR_ERR(param.blob);
> > +   goto out_key;
> > +   }
> > +   break;
> 
> This means that a namespace admin (iow, an unprivileged user) can
> allocate 1MB of unswappable kmalloc memory per userspace task, right?
> Using userfaultfd or FUSE, you can then stall the task as long as you
> want while it has that allocation. Is that problematic, or is that
> normal?

That's not exactly the case.  A userspace task can make a temporary
allocation, but unless the filesystem grabs it, it's released again on exit
from the system call.

Note that I should probably use vmalloc() rather than kmalloc(), but that
doesn't really affect your point.  I could also pass the user pointer through
to the filesystem instead - I wanted to avoid that for this interface, but it
make sense in this instance.

David

Re: [PATCH] m68k/mac: Rework patch "use time64_t in RTC handling"

[Geert Uytterhoeven]

Hi Finn, Arnd,

On Tue, Jul 24, 2018 at 9:07 AM Finn Thain  wrote:
> This addresses the issues arising from commit 324caa29cd04
> ("m68k: mac: use time64_t in RTC handling").
>
> Adopt __u32 for the union in via_read_time(), consistent with changes
> to via_write_time().
>
> Use low_32_bits() in via_write_time(), consistent with changes to

lower_32_bits()

> pmu_write_time() and cuda_write_time().
>
> Have via_read_time() return a time64_t, consistent with changes to
> pmu_read_time and cuda_read_time().

pmu_read_time()

> Drop the pointless wraparound conditional in via_read_time().
>
> Cc: Arnd Bergmann 
> Signed-off-by: Finn Thain 

Thanks!

Applied to master with the above fixed.
Folded into the original commit on for-v4.19, with:

[fthain: Adopt __u32 for the union in via_read_time(), consistent with
 changes to via_write_time()]
[fthain: Use lower_32_bits() in via_write_time(), consistent with changes
 to pmu_write_time() and cuda_write_time()]
[fthain: Have via_read_time() return a time64_t, consistent with changes
 to pmu_read_time() and cuda_read_time()]
[fthain: Drop the pointless wraparound conditional in via_read_time()]
Signed-off-by: Finn Thain 
Reviewed-by: Arnd Bergmann 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

How to secure erase PCI-E NVME SSD connected via USB3?

[Jeff Chua]

I'm testing the USB3-to-PCI-E NVME SSD. It's works using uas module,
recognized it as /dev/sda.

Since it's an USB device, the nvme-cli tools won't work, nor does
hdparm, as it's a NVME SSD.

So, how to secure-erase the NVME SSD connected via the JMS583 chip?

Thanks,
Jeff.

[PATCH v1 0/2] i2c: npcm7xx: new driver for I2C controller

[Tali Perry]

Nuvoton NPCM7XX I2C Controller
NPCM7xx includes 16 I2C contollers. THis driver operates the controller.
This module also includes a slave mode, which will be submitted later on.

Any feedback would be appreciated.  


Signed-off-by: Tali Perry 


---
Tali Perry (2):
  dt-binding: i2c: npcm7xx: add binding for i2c controller
  i2c: npcm7xx: add i2c controller master mode only

 .../devicetree/bindings/i2c/i2c-npcm7xx.txt|   27 +
 MAINTAINERS|9 +
 drivers/i2c/busses/Kconfig |   11 +
 drivers/i2c/busses/Makefile|1 +
 drivers/i2c/busses/i2c-npcm7xx.c   | 2574 
 5 files changed, 2622 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-npcm7xx.txt
 create mode 100644 drivers/i2c/busses/i2c-npcm7xx.c

-- 
2.14.1

[PATCH v1 2/2] i2c: npcm7xx: add i2c controller master mode only

[Tali Perry]

Nuvoton NPCM7XX I2C Controller
NPCM7xx includes 16 I2C contollers. THis driver operates the controller.
This module also includes a slave mode, which will be submitted later on.

Any feedback would be appreciated.  


Signed-off-by: Tali Perry 

---
 drivers/i2c/busses/Kconfig   |   11 +
 drivers/i2c/busses/Makefile  |1 +
 drivers/i2c/busses/i2c-npcm7xx.c | 2574 ++
 3 files changed, 2586 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-npcm7xx.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 4f8df2ec87b1..692ce1568fb0 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -742,6 +742,17 @@ config I2C_NOMADIK
  I2C interface from ST-Ericsson's Nomadik and Ux500 architectures,
  as well as the STA2X11 PCIe I/O HUB.
 
+config I2C_NPCM7XX
+   tristate "Nuvoton I2C Controller"
+   depends on ARCH_NPCM7XX
+   select I2C_SLAVE
+   help
+ If you say yes to this option, support will be included for the
+ Nuvoton I2C controller.
+
+ This driver can also be built as a module.  If so, the module
+ will be called i2c-npcm7xx.
+
 config I2C_OCORES
tristate "OpenCores I2C Controller"
help
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 5a869144a0c5..80d4ec8908e1 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_I2C_MT65XX)  += i2c-mt65xx.o
 obj-$(CONFIG_I2C_MV64XXX)  += i2c-mv64xxx.o
 obj-$(CONFIG_I2C_MXS)  += i2c-mxs.o
 obj-$(CONFIG_I2C_NOMADIK)  += i2c-nomadik.o
+obj-$(CONFIG_I2C_NPCM7XX)  += i2c-npcm7xx.o
 obj-$(CONFIG_I2C_OCORES)   += i2c-ocores.o
 obj-$(CONFIG_I2C_OMAP) += i2c-omap.o
 obj-$(CONFIG_I2C_PASEMI)   += i2c-pasemi.o
diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c
new file mode 100644
index ..442b23aacaaf
--- /dev/null
+++ b/drivers/i2c/busses/i2c-npcm7xx.c
@@ -0,0 +1,2574 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nuvoton NPCM7xx SMB Controller driver
+ *
+ * Copyright (C) 2018 Nuvoton Technologies tali.pe...@nuvoton.com
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+static struct regmap *gcr_regmap;
+static struct regmap *clk_regmap;
+
+#define I2CSEGCTL_OFFSET 0xE4
+
+#define NPCM7XX_SECCNT (0x68)
+#define NPCM7XX_CNTR25M(0x6C)
+
+#define I2CSEGCTL_VAL  0x0333F000
+
+#define ENABLE 1
+#define DISABLE0
+
+#define _1Hz_  (u32)1UL
+#define _1KHz_ (1000 * _1Hz_)
+#define _1MHz_ (1000 * _1KHz_)
+#define _1GHz_ (1000 * _1MHz_)
+
+#ifndef ASSERT
+#ifdef DEBUG
+#define ASSERT(cond)  {if (!(cond)) for (;;) ; }
+#else
+#define ASSERT(cond)
+#endif
+#endif
+
+#define ROUND_UP(val, n)   (((val)+(n)-1) & ~((n)-1))
+#define DIV_CEILING(a, b)   (((a) + ((b)-1)) / (b))
+
+#define I2C_VERSION "0.0.2"
+
+#define I2C_DEBUG2(f, x...)
+
+
+
+
+// HW module supports TO. in Linux, we choose to use SW TO to avoid conflicts
+// However this code is still available in this driver:
+// #define SMB_CAPABILITY_TIMEOUT_SUPPORT
+
+#define SMB_RECOVERY_SUPPORT
+
+// override HW SMBus may fail to supply stop condition in Master Write 
operation
+#define SMB_SW_BYPASS_HW_ISSUE_SMB_STOP
+
+// slave mode: if end device reads more data than available, ask issuer or
+// request for more data:
+#define SMB_WRAP_AROUND_BUFFER
+
+#define SMB_QUICK_PROT  0x
+#define SMB_BLOCK_PROT  0xFFFE
+#define SMB_EXCLUDE_BLOCK_SIZE_FROM_BUF0xFFFD
+
+
+enum smb_mode {
+   SMB_SLAVE = 1,
+   SMB_MASTER
+};
+
+/*
+ * External SMB Interface driver states values, which indicate to the
+ * upper-level layer the status of the
+ * operation it initiated or wake up events from one of the buses
+ */
+enum smb_state_ind {
+   SMB_NO_STATUS_IND = 0,
+   SMB_SLAVE_RCV_IND = 1,
+   SMB_SLAVE_XMIT_IND = 2,
+   SMB_SLAVE_XMIT_MISSING_DATA_IND = 3,
+   SMB_SLAVE_RESTART_IND = 4,
+   SMB_SLAVE_DONE_IND = 5,
+   SMB_MASTER_DONE_IND = 6,
+   SMB_NO_DATA_IND = 7,
+   SMB_NACK_IND = 8,
+   SMB_BUS_ERR_IND = 9,
+   SMB_WAKE_UP_IND = 10,
+   SMB_MASTER_PEC_ERR_IND = 11,
+   SMB_BLOCK_BYTES_ERR_IND = 12,
+   SMB_SLAVE_PEC_ERR_IND = 13,
+   SMB_SLAVE_RCV_MISSING_DATA_IND = 14,
+};
+
+// SMBus Operation type values
+enum smb_oper {
+   SMB_NO_OPER = 0,
+   SMB_WRITE_OPER = 1,
+   SMB_READ_OPER = 2
+};
+
+
+// SMBus Bank (FIFO mode)
+enum smb_bank {
+   SMB_BANK_0 = 0,
+   SMB_BANK_1 = 1
+};
+
+// Internal SMB states values, which reflect events which 

[PATCH v1 1/2] dt-binding: i2c: npcm7xx: add binding for i2c

[Tali Perry]

Nuvoton NPCM7XX I2C Controller
NPCM7xx includes 16 I2C contollers. This driver operates the controller.
This module also includes a slave mode, which will be submitted later on.

Any feedback would be appreciated.  


Signed-off-by: Tali Perry 

---
 .../devicetree/bindings/i2c/i2c-npcm7xx.txt| 27 ++
 MAINTAINERS|  9 
 2 files changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-npcm7xx.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-npcm7xx.txt 
b/Documentation/devicetree/bindings/i2c/i2c-npcm7xx.txt
new file mode 100644
index ..e2ee45d7a41e
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-npcm7xx.txt
@@ -0,0 +1,27 @@
+Nuvoton NPCM7XX I2C bus
+
+The NPCM750x includes sixteen I2C busses
+
+Required properties:
+- compatible  : must be "nuvoton,npcm750-i2c-bus"
+- reg : Offset and length of the register set for the device.
+- interrupts  : Contain the I2C interrupt with flags for falling edge.
+- clocks  : phandle of I2C reference clock.
+
+Optional:
+- bus-frequency   : Contain the I2C bus frequency,
+   the default I2C bus frequency is 10.
+- pinctrl-0   : must be <&smbX_pins>, X is module number
+   (on NPCM7XX it's 0 to 15)
+- pinctrl-names   : should be set to "default"
+Example:
+
+   i2c0: i2c-bus@8 {
+   compatible = "nuvoton,npcm750-i2c-bus";
+   reg = <0x8 0x1000>;
+   clocks = <&clk NPCM7XX_CLK_APB2>;
+   bus-frequency = <10>;
+   interrupts = ;
+   pinctrl-names = "default";
+   pinctrl-0 = <&smb0_pins>;
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index 192d7f73fd01..a28edc9daabb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1735,6 +1735,15 @@ F:   drivers/*/*npcm*
 F: Documentation/devicetree/bindings/*/*npcm*
 F: Documentation/devicetree/bindings/*/*/*npcm*
 
+ARM/NUVOTON I2C DRIVER
+M: Tali Perry 
+M: Avi Fishman 
+L: linux-...@vger.kernel.org
+L: open...@lists.ozlabs.org (moderated for non-subscribers)
+S: Supported
+F: drivers/i2c/busses/i2c-npcm7xx.c
+F: Documentation/devicetree/bindings/i2c/i2c-npcm7xx.txt
+
 ARM/NUVOTON W90X900 ARM ARCHITECTURE
 M: Wan ZongShun 
 L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers)
-- 
2.14.1

Re: [Linux-kernel] [PATCH 3/4] ASoC: tegra: Allow 32-bit and 24-bit samples

[Ben Dooks]

On 2018-07-28 23:28, kbuild test robot wrote:

Hi Edward,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tegra/for-next]
[also build test ERROR on v4.18-rc6 next-20180727]
[if your patch is applied to the wrong git tree, please drop us a note
to help improve the system]

url:
https://github.com/0day-ci/linux/commits/Jorge-Sanjuan/ASoC-Tegra30-TDM-support/20180728-163720
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git 
for-next

config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm

Note: the
linux-review/Jorge-Sanjuan/ASoC-Tegra30-TDM-support/20180728-163720
HEAD 14bbc96df0fa027f7bc057eb2da8181baff4e22c builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

   sound/soc/tegra/tegra30_i2s.c: In function 'tegra30_i2s_hw_params':
sound/soc/tegra/tegra30_i2s.c:155:3: error: 'audio_bits' undeclared 
(first use in this function); did you mean 'audit_names'?

  audio_bits = TEGRA30_AUDIOCIF_BITS_24;
  ^~
  audit_names
   sound/soc/tegra/tegra30_i2s.c:155:3: note: each undeclared
identifier is reported only once for each function it appears in

vim +155 sound/soc/tegra/tegra30_i2s.c

   133
   134	static int tegra30_i2s_hw_params(struct snd_pcm_substream 
*substream,

   135   struct snd_pcm_hw_params *params,
   136   struct snd_soc_dai *dai)
   137  {
   138  struct device *dev = dai->dev;
   139  struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
   140  unsigned int mask, val, reg;
   141  int ret, sample_size, srate, i2sclock, bitcnt;
   142  struct tegra30_ahub_cif_conf cif_conf;
   143
   144  if (params_channels(params) != 2)
   145  return -EINVAL;
   146
   147  mask = TEGRA30_I2S_CTRL_BIT_SIZE_MASK;
   148  switch (params_format(params)) {
   149  case SNDRV_PCM_FORMAT_S16_LE:
   150  val = TEGRA30_I2S_CTRL_BIT_SIZE_16;
   151  sample_size = 16;
   152  break;
   153  case SNDRV_PCM_FORMAT_S24_LE:
   154  val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
 > 155   audio_bits = TEGRA30_AUDIOCIF_BITS_24;
   156  sample_size = 24;
   157  break;
   158  case SNDRV_PCM_FORMAT_S32_LE:
   159  val = TEGRA30_I2S_CTRL_BIT_SIZE_32;
   160  sample_size = 32;
   161  break;
   162  default:
   163  return -EINVAL;
   164  }



looks like we failed to merge in a fix from later in the internal
series we have.

jorge: can we get the channel fix from here into this patch and 
resubmit?


commit dd439f5f0b748eba43da7f18cabec8850dcd18b1
Author: Edward Cragg 
Date:   Thu Sep 15 17:01:49 2016 +0100

ASoC: tegra: i2s: Add support for more than 2 channels

Re: [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support

[Hans de Goede]

Hi,

On 27-07-18 22:47, Srinivas Pandruvada wrote:

One of the requirement for modern x86 system to enter lowest power mode
(SLP_S0) is SATA IP block to be off. This is true even during when
platform is suspended to idle and not only in opportunistic (runtime)
suspend.
This series is to enable DEVSLP by default.

v2:
  As suggested by Hans, take care of the module param is same as default
  LPM policy


Thanks.

Series looks good to me:

Reviewed-by: Hans de Goede 

Note we need to keep an eye out for this causing regressions with
some disks / firmware-versions.

Regards,

Hans






-current (non-rfc) series
Implemented suggestions from Hans
- no override of policy set via module param
- Override lpm policy per host not global policy
- changed the new policy name to "min_power_with_partial"

rfc-v2
- As suggested by Hans, it is possible to have ASP with DEVSLP, so
add a new state.
- Removed usage of mem_sleep_current, instead just rely on low power
idle flag. Don't feel good to EXPORT from core suspend code.
- Depending host policy to decide if we enable DEVSLP by default.

Srinivas Pandruvada (2):
   ata: ahci: Support state with min power but Partial low power state
   ata: ahci: Enable DEVSLP by default on x86 with SLP_S0

  drivers/ata/ahci.c| 38 +-
  drivers/ata/libahci.c |  5 -
  drivers/ata/libata-core.c |  1 +
  drivers/ata/libata-scsi.c |  1 +
  include/linux/libata.h|  3 ++-
  5 files changed, 41 insertions(+), 7 deletions(-)

Re: [PATCH 01/10] staging:rtl8192u: Remove typedef of struct cmpk_txfb_t - Style

[John Whitmore]

On Sun, Jul 29, 2018 at 10:10:55AM +0200, Greg KH wrote:
> On Wed, Jul 25, 2018 at 11:16:21PM +0100, John Whitmore wrote:
> > -typedef struct tag_cmd_pkt_tx_feedback {
> > +struct cmpk_txfb_t {
> > /* DWORD 0 */
> > u8  element_id; /* Command packet type. */
> > u8  length; /* Command packet length. */
> > @@ -53,7 +53,7 @@ typedef struct tag_cmd_pkt_tx_feedback {
> > /* DWORD 5 */
> > u16 reserve3;
> > u16 duration;
> > -} cmpk_txfb_t;
> > +};
> 
> People use the "_t" to try to denote a "typedef".  When converting to
> just a structure, there is no need to keep the _t anymore at all, so
> this should just be "struct cmpk_txfb".
> 
> Or just drop the typedef and use the name for the struct they had here
> already, "struct tag_cmd_pkt_tx_feedback", that also would work.
> 
> Same for all of the other patches in this series.
> 
> thanks,
> 
> greg k-h

Oops again. Sorry, that should have occured to me. I'll fix up the affected
and resend.

thank you

jwhitmore

Re: [PATCH 02/10] staging:rtl8192u: Refactor use of enum dm_dig_sta_e - Style

[John Whitmore]

On Sun, Jul 29, 2018 at 10:17:26AM +0200, Greg KH wrote:
> On Sat, Jul 28, 2018 at 12:28:18AM +0100, John Whitmore wrote:
> > Refactor the use of the enumerated type dm_dig_sta_e, which is not
> > actually used for type checking by the compiler.
> > 
> > The enumerated type defines values for the enumeration, which are used
> > by both dig_state and dig_highpwr_state, (members of the struct dig).
> > Both of those variables were defined as being of type u8. This negates
> > any usefulness of the use of the enumeration, (compiler type checking).
> > 
> > To make use of the compiler's type-checking the two member variables,
> > dig_state and dig_highpwr_state have been changed to being of type
> > enum dm_dig_sta_e. The enumerated type has been moved above the
> > struct dig definition so that the enumeration is already defined when
> > compiler reaches the two types using the enumerated type.
> > 
> > In addition the 'typedef' of the enumerated type has been removed to
> > clear the checkpatch issue with declaring new types.
> > 
> > These changes, whilst convoluted, are purely coding style in nature and
> > should not impact runtime code execution.
> > 
> > Signed-off-by: John Whitmore 
> > ---
> >  drivers/staging/rtl8192u/r8192U_dm.h | 18 +-
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192u/r8192U_dm.h 
> > b/drivers/staging/rtl8192u/r8192U_dm.h
> > index e86dda99c223..2444e1c1357b 100644
> > --- a/drivers/staging/rtl8192u/r8192U_dm.h
> > +++ b/drivers/staging/rtl8192u/r8192U_dm.h
> > @@ -64,6 +64,13 @@
> >  
> >  
> >  /*--Define 
> > structure*/
> > +
> > +enum dm_dig_sta_e {
> 
> Don't end an enum with "_e", as that's not needed at all.
> 
> thanks,
> 
> greg k-h

Oh! Thanks for that, it never occured to me that the '_e' was notation.
Again I'll fix up all those affected and resend.

jwhitmore

[no subject]

[Sumitomo Rubber]

--
Did you receive our representative email ?

Re: [PATCH] ACPICA: AML Parser: ignore control method status in module-level code

[Rafael J. Wysocki]

On Sat, Jul 28, 2018 at 11:05 PM, Erik Schmauss  wrote:
> Previous change in the AML parser code blindly set all non-successful
> dispatcher statuses to AE_OK. This approach is incorrect because
> successful control method invocations from module-level return
> AE_CTRL_TRANSFER. Overwriting AE_OK to this status causes the AML
> parser to think that there was no return value from the control
> method invocation.
>
> fixes: 73c2a01c52b6 (ACPICA: AML Parser: ignore dispatcher error status 
> during table load)
>
> Reported-by: Linus Torvalds 
> Tested-by: Linus Torvalds 
> Tested-by: Oleksandr Natalenko 
> Signed-off-by: Erik Schmauss 
> ---
>  drivers/acpi/acpica/psloop.c | 19 ---
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/acpica/psloop.c b/drivers/acpi/acpica/psloop.c
> index ee840be150b5..44f35ab3347d 100644
> --- a/drivers/acpi/acpica/psloop.c
> +++ b/drivers/acpi/acpica/psloop.c
> @@ -709,15 +709,20 @@ acpi_status acpi_ps_parse_loop(struct acpi_walk_state 
> *walk_state)
> } else
> if ((walk_state->
>  parse_flags & ACPI_PARSE_MODULE_LEVEL)
> +   && status != AE_CTRL_TRANSFER
> && ACPI_FAILURE(status)) {
> /*
> -* ACPI_PARSE_MODULE_LEVEL means that we are 
> loading a table by
> -* executing it as a control method. However, 
> if we encounter
> -* an error while loading the table, we need 
> to keep trying to
> -* load the table rather than aborting the 
> table load. Set the
> -* status to AE_OK to proceed with the table 
> load. If we get a
> -* failure at this point, it means that the 
> dispatcher got an
> -* error while processing Op (most likely an 
> AML operand error.
> +* ACPI_PARSE_MODULE_LEVEL flag means that we 
> are currently
> +* loading a table by executing it as a 
> control method.
> +* However, if we encounter an error while 
> loading the table,
> +* we need to keep trying to load the table 
> rather than
> +* aborting the table load (setting the 
> status to AE_OK
> +* continues the table load). If we get a 
> failure at this
> +* point, it means that the dispatcher got an 
> error while
> +* processing Op (most likely an AML operand 
> error) or a
> +* control method was called from module 
> level and the
> +* dispatcher returned AE_CTRL_TRANSFER. In 
> the latter case,
> +* leave the status alone, there's nothing 
> wrong with it.
>  */
> status = AE_OK;
> }
> --

Applied, thanks!

Re: [PATCH v7 1/4] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller

[Wolfram Sang]

On Tue, Jun 12, 2018 at 11:09:05AM -0600, Karthikeyan Ramasubramanian wrote:
> This bus driver supports the GENI based i2c hardware controller in the
> Qualcomm SOCs. The Qualcomm Generic Interface (GENI) is a programmable
> module supporting a wide range of serial interfaces including I2C. The
> driver supports FIFO mode and DMA mode of transfer and switches modes
> dynamically depending on the size of the transfer.
> 
> Signed-off-by: Karthikeyan Ramasubramanian 
> Signed-off-by: Sagar Dharia 
> Signed-off-by: Girish Mahadevan 
> Reviewed-by: Douglas Anderson 

Looks good except one minor thing:

> + pm_runtime_set_suspended(gi2c->se.dev);
> + pm_runtime_set_autosuspend_delay(gi2c->se.dev, I2C_AUTO_SUSPEND_DELAY);
> + pm_runtime_use_autosuspend(gi2c->se.dev);
> + pm_runtime_enable(gi2c->se.dev);
> + i2c_add_adapter(&gi2c->adap);
> +
> + return 0;

i2c_add_adapter can fail. So, I'd guess you want to check the return
value and move it above the pm_runtime calls?



signature.asc
Description: PGP signature

Re: [PATCH] PM / devfreq: Generic cpufreq governor

[Rafael J. Wysocki]

On Sat, Jul 28, 2018 at 5:56 AM, Saravana Kannan  wrote:
> Many CPU architectures have caches that can scale independent of the CPUs.
> Frequency scaling of the caches is necessary to make sure the cache is not
> a performance bottleneck that leads to poor performance and power. The same
> idea applies for RAM/DDR.
>
> To achieve this, this patch adds a generic devfreq governor that can listen
> to the frequency transitions of each CPU frequency domain and then adjusts
> the frequency of the cache (or any devfreq device) based on the frequency
> of the CPUs.
>
> To decide the frequency of the device, the governor does one of the
> following:
>
> * Uses a CPU frequency to device frequency mapping table
>   - Either one mapping table used for all CPU freq policies (typically used
> for system with homogeneous cores/clusters that have the same OPPs.
>   - One mapping table per CPU freq policy (typically used for ASMP systems
> with heterogeneous CPUs with different OPPs)
>
> OR
>
> * Scales the device frequency in proportion to the CPU frequency. So, if
>   the CPUs are running at their max frequency, the device runs at its max
>   frequency.  If the CPUs are running at their min frequency, the device
>   runs at its min frequency. And interpolated for frequencies in between.

While not having looked at the details of the patch yet, I would
change the name of the feature to "Generic cpufreq transition
governor" to make it somewhat less ambiguous.

Re: Linux 4.17.11

[Greg K-H]

On July 29, 2018 12:47:16 PM GMT+02:00, "Jörg-Volker Peetz"  
wrote:
>Greg KH wrote on 07/28/18 08:27:
>> I'm announcing the release of the 4.17.11 kernel.
>> 
>> All users of the 4.17 kernel series must upgrade.
>> 
>> The updated 4.17.y git tree can be found at:
>>
>   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
>linux-4.17.y
>> and can be browsed at the normal kernel.org git web browser:
>>
>   
> http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary
>> 
>> thanks,
>> 
>> greg k-h
>> 
>
>
>Hi Greg,
>
>your signature files did not yet appear on
>https://www.kernel.org/pub/linux/kernel/v4.x/ .
>Any idea?
>
>Regards,
>Jörg.

https://www.kernel.org/minor-changes-to-tarball-release-format.html

Re: [PATCH 29/38] vfs: syscall: Add fsconfig() for configuring and managing a context [ver #10]

[Jann Horn]

On Sun, Jul 29, 2018 at 10:50 AM David Howells  wrote:
>
> Jann Horn  wrote:
>
> > [...]
> > > +   case fsconfig_set_binary:
> > > +   param.type = fs_value_is_blob;
> > > +   param.size = aux;
> > > +   param.blob = memdup_user_nul(_value, aux);
> > > +   if (IS_ERR(param.blob)) {
> > > +   ret = PTR_ERR(param.blob);
> > > +   goto out_key;
> > > +   }
> > > +   break;
> >
> > This means that a namespace admin (iow, an unprivileged user) can
> > allocate 1MB of unswappable kmalloc memory per userspace task, right?
> > Using userfaultfd or FUSE, you can then stall the task as long as you
> > want while it has that allocation. Is that problematic, or is that
> > normal?
>
> That's not exactly the case.  A userspace task can make a temporary
> allocation, but unless the filesystem grabs it, it's released again on exit
> from the system call.

That's what I said. Each userspace task can make a 1MB allocation by
calling this syscall, and this temporary allocation stays allocated
until the end of the syscall. But the runtime of the syscall is
unbounded - even just the memdup_user_nul() can stall forever if the
copy_from_user() call inside it faults on e.g. a userfault region or a
memory-mapped file from a FUSE filesystem.

> Note that I should probably use vmalloc() rather than kmalloc(), but that
> doesn't really affect your point.  I could also pass the user pointer through
> to the filesystem instead - I wanted to avoid that for this interface, but it
> make sense in this instance.

[no subject]

[Wang Shengkun]

Hi 

My name is Wang Shengkun, i am a software programmer and i work with a lottery 
company over here in the United States. I can give you the winning numbers in 
our admin to play and win the lottery and we will share the proceeds 50% each. 
If you are interested , kindly write me back at : 2148972...@qq.com 

Thanks 

Wang Shengkun. 

[PATCH] staging: fix platform_no_drv_owner.cocci warnings

[kbuild test robot]

From: kbuild test robot 

drivers/staging/axis-fifo/axis-fifo.c:1081:3-8: No need to set .owner here. The 
core will do it.

 Remove .owner field if calls are used which set it automatically

Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci

Fixes: 4a965c5f89de ("staging: add driver for Xilinx AXI-Stream FIFO v4.1 IP 
core")
CC: Jacob Feder 
Signed-off-by: kbuild test robot 
---

 axis-fifo.c |1 -
 1 file changed, 1 deletion(-)

--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -1078,7 +1078,6 @@ MODULE_DEVICE_TABLE(of, axis_fifo_of_mat
 static struct platform_driver axis_fifo_driver = {
.driver = {
.name = DRIVER_NAME,
-   .owner = THIS_MODULE,
.of_match_table = axis_fifo_of_match,
},
.probe  = axis_fifo_probe,

Re: [PATCH v1 2/2] i2c: npcm7xx: add i2c controller master mode only

[Andy Shevchenko]

On Sun, Jul 29, 2018 at 12:14 PM, Tali Perry  wrote:
> Nuvoton NPCM7XX I2C Controller
> NPCM7xx includes 16 I2C contollers. THis driver operates the controller.
> This module also includes a slave mode, which will be submitted later on.
>
> Any feedback would be appreciated.

Too much lines of code as for I2C driver.

Briefly looking it seems it doesn't utilize what kernel already has
(e.g. crc8 already is in kernel library) and doesn't follow modern
style of drivers there.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 

Just wow. Are you sure you need all of them? (Also, to see better what
it's used, keep them in order)

> +#define ENABLE 1
> +#define DISABLE0

So, what's wrong with 1, 0 or true, false?

> +#define _1Hz_  (u32)1UL
> +#define _1KHz_ (1000 * _1Hz_)
> +#define _1MHz_ (1000 * _1KHz_)
> +#define _1GHz_ (1000 * _1MHz_)

If you need such constants it would be something like for time periods we have

HZ_PER_KHZ 1000
HZ_PER_MHZ 100

etc.

I saw in a code some of those predefined. If they are not yet generic
(in scope of kernel) it might be worth to do in the future.

> +#ifndef ASSERT
> +#ifdef DEBUG
> +#define ASSERT(cond)  {if (!(cond)) for (;;) ; }
> +#else
> +#define ASSERT(cond)
> +#endif
> +#endif

Hmm... In production code?!

> +
> +#define ROUND_UP(val, n)   (((val)+(n)-1) & ~((n)-1))
> +#define DIV_CEILING(a, b)   (((a) + ((b)-1)) / (b))


You really need to check what kernel provides.

> +#define I2C_DEBUG2(f, x...)

???

> +
> +
> +
> +

Why to have so many blank lines?

> +
> +

Ditto.

> +// Common regs
> +#define NPCM7XX_SMBSDA(bus)(bus->base + 0x000)
> +#define NPCM7XX_SMBST(bus) (bus->base + 0x002)
> +#define NPCM7XX_SMBCST(bus)(bus->base + 0x004)
> +#define NPCM7XX_SMBCTL1(bus)   (bus->base + 0x006)
> +#define NPCM7XX_SMBADDR1(bus)  (bus->base + 0x008)
> +#define NPCM7XX_SMBCTL2(bus)   (bus->base + 0x00A)
> +#define NPCM7XX_SMBADDR2(bus)  (bus->base + 0x00C)
> +#define NPCM7XX_SMBCTL3(bus)   (bus->base + 0x00E)
> +#define NPCM7XX_SMBCST2(bus)   (bus->base + 0x018)  // Control 
> Status 2
> +#define NPCM7XX_SMBCST3(bus)   (bus->base + 0x019)  // Control 
> Status 3
> +#define SMB_VER(bus)   (bus->base + 0x01F)  // SMB Version 
> reg

Usually we put just numbers here and use custom I/O accessors which
take bus as a parameter.

> +
> +// BANK 0 regs
> +#define NPCM7XX_SMBADDR3(bus)  (bus->base + 0x010)
> +#define NPCM7XX_SMBADDR7(bus)  (bus->base + 0x011)
> +#define NPCM7XX_SMBADDR4(bus)  (bus->base + 0x012)
> +#define NPCM7XX_SMBADDR8(bus)  (bus->base + 0x013)
> +#define NPCM7XX_SMBADDR5(bus)  (bus->base + 0x014)
> +#define NPCM7XX_SMBADDR9(bus)  (bus->base + 0x015)
> +#define NPCM7XX_SMBADDR6(bus)  (bus->base + 0x016)
> +#define NPCM7XX_SMBADDR10(bus) (bus->base + 0x017)

> +#define NPCM7XX_SMBADDR(bus, i)(bus->base + 0x008 + \
> +   (u32)(((int)i * 4) + (((int)i < 2) ? 0 : \
> +   ((int)i - 2)*(-2)) + (((int)i < 6) ? 0 : (-7

It's rather complicated.

> +   // Current state of SMBus

I guess more compact is to put a kernel doc descriprion.

> +   enum smb_state  state;


> +static bool NPCM7XX_smb_init_module(struct NPCM7XX_i2c *bus,
> +   enum smb_mode mode, u16 bus_freq);

Do you need all forward declarations? Why?


So, I stopped here.

Try to make your code twice less in a lenght (looking at it I think
it's doable).


> +static const struct of_device_id NPCM7XX_i2c_bus_of_table[] = {
> +   { .compatible = "nuvoton,npcm750-i2c-bus", },

> +   {},

Slightly better without comma for terminator lines.

> +};
> +MODULE_DEVICE_TABLE(of, NPCM7XX_i2c_bus_of_table);

-- 
With Best Regards,
Andy Shevchenko

Contact Me

[marrthes]

RE: RECOVERED FUND.

In reference to the signal received from our Investigative Team concerning 
overdue payments. We write to inform you that some funds have been recovered 
from some Security Companies, and sub financial houses, meant to be paid out to 
owners but diverted.

However, it was found after due auditing and vetting of all payment files, that 
your name was tagged on unpaid payment list. Thus, we are informing you that 
all overdue payments have been approved as shown in our Gazette will be 
released to you after re-confirmation of your details.

1) Your full name and Address,
2) Phone, fax or mobile.

Contact this office directly via Email, Text or Call to speed up payment 
processing and release.

Do discard this information, if you are not the intended recipient.

All emails are read and attended.

Yours Sincerely,
Gabriel

Re: [PATCH 36/38] vfs: Add a sample program for the new mount API [ver #10]

[Pavel Machek]

Hi!

> Add a sample program to demonstrate fsopen/fsmount/move_mount to mount
> something.

> @@ -0,0 +1,118 @@
> +/* fd-based mount test.
> + *
> + * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowe...@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */

Can we do SPDX here?

> +static void check_messages(int fd)
> +{
> + char buf[4096];
> + int err, n;
> +
> + err = errno;
> +
> + for (;;) {
> + n = read(fd, buf, sizeof(buf));
> + if (n < 0)
> + break;
> + n -= 2;
> +
> + switch (buf[0]) {
> + case 'e':
> + fprintf(stderr, "Error: %*.*s\n", n, n, buf + 2);
> + break;
> + case 'w':
> + fprintf(stderr, "Warning: %*.*s\n", n, n, buf + 2);
> + break;
> + case 'i':
> + fprintf(stderr, "Info: %*.*s\n", n, n, buf + 2);
> + break;
> + }
> + }

Hmm, so kernel now returns messages in english? Not sure that is
reasonable, as that is going to cause problems with translations...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCH v3 0/2] Staging: octeon-usb: Changed CVMX_WAIT_FOR_FIELD32 macro

[Georgios Tsotsos]

Replying to previous message, i changed CVMX_WAIT_FOR_FIELD32 macro to function
and altered a multiple calling of CVMX_USBCX_GRSTCTL call to single call and
stored to variable.

Georgios Tsotsos (2):
  Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL
  Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function
call.

 drivers/staging/octeon-usb/octeon-hcd.c | 77 +++--
 1 file changed, 44 insertions(+), 33 deletions(-)

-- 
2.16.4

[PATCH v3 1/2] Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL

[Georgios Tsotsos]

Assign to variable the result of CVMX_USBCX_GRSTCTL instead of multiple
calling a macro.

Signed-off-by: Georgios Tsotsos 
---
 drivers/staging/octeon-usb/octeon-hcd.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index cff5e790b196..4615133292b5 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -598,6 +598,7 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb)
union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3;
union cvmx_usbcx_gnptxfsiz npsiz;
union cvmx_usbcx_hptxfsiz psiz;
+   u64 address;
 
usbcx_ghwcfg3.u32 = cvmx_usb_read_csr32(usb,
CVMX_USBCX_GHWCFG3(usb->index));
@@ -629,17 +630,16 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb)
psiz.s.ptxfstaddr = 3 * usbcx_ghwcfg3.s.dfifodepth / 4;
cvmx_usb_write_csr32(usb, CVMX_USBCX_HPTXFSIZ(usb->index), psiz.u32);
 
+   address = CVMX_USBCX_GRSTCTL(usb->index);
+
/* Flush all FIFOs */
-   USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
-   cvmx_usbcx_grstctl, txfnum, 0x10);
-   USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
-   cvmx_usbcx_grstctl, txfflsh, 1);
-   CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
- cvmx_usbcx_grstctl, c.s.txfflsh == 0, 100);
-   USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
-   cvmx_usbcx_grstctl, rxfflsh, 1);
-   CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
- cvmx_usbcx_grstctl, c.s.rxfflsh == 0, 100);
+   USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfnum, 0x10);
+   USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfflsh, 1);
+   CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl,
+ c.s.txfflsh == 0, 100);
+   USB_SET_FIELD32(address, cvmx_usbcx_grstctl, rxfflsh, 1);
+   CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl,
+ c.s.rxfflsh == 0, 100);
 }
 
 /**
-- 
2.16.4

[PATCH v3 2/2] Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function call.

[Georgios Tsotsos]

Replacing CVMX_WAIT_FOR_FIELD32 macro with equivalent function.

Signed-off-by: Georgios Tsotsos 
---
 drivers/staging/octeon-usb/octeon-hcd.c | 65 +++--
 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index 4615133292b5..8a7bdf1a9fe6 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -377,29 +377,6 @@ struct octeon_hcd {
struct cvmx_usb_tx_fifo nonperiodic;
 };
 
-/* This macro spins on a register waiting for it to reach a condition. */
-#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \
-   ({int result;   \
-   do {\
-   u64 done = cvmx_get_cycle() + (u64)timeout_usec *   \
-  octeon_get_clock_rate() / 100;   \
-   union _union c; \
-   \
-   while (1) { \
-   c.u32 = cvmx_usb_read_csr32(usb, address);  \
-   \
-   if (cond) { \
-   result = 0; \
-   break;  \
-   } else if (cvmx_get_cycle() > done) {   \
-   result = -1;\
-   break;  \
-   } else  \
-   __delay(100);   \
-   }   \
-   } while (0);\
-   result; })
-
 /*
  * This macro logically sets a single field in a CSR. It does the sequence
  * read, modify, and write
@@ -593,6 +570,42 @@ static inline int cvmx_usb_get_data_pid(struct 
cvmx_usb_pipe *pipe)
return 0; /* Data0 */
 }
 
+/**
+ * Loop through register until txfflsh or rxfflsh become zero.
+ *
+ * @usb:   USB block
+ * @address:   64bit address to read
+ * @timeout_usec:  Timeout
+ * @fflsh_type:Indicates fflsh type, 0 for txfflsh, 1 for rxfflsh
+ *
+ */
+static int cvmx_wait_for_field32(struct octeon_hcd *usb, u64 address,
+u64 timeout_usec, int fflsh_type)
+{
+   int result;
+   u64 done = cvmx_get_cycle() + timeout_usec *
+  (u64)octeon_get_clock_rate / 100;
+
+   union cvmx_usbcx_grstctl c;
+
+   while (1) {
+   c.u32 = cvmx_usb_read_csr32(usb, address);
+   if (fflsh_type == 0 && c.s.txfflsh == 0) {
+   result = 0;
+   break;
+   } else if (fflsh_type == 1 && c.s.rxfflsh == 0) {
+   result = 0;
+   break;
+   } else if (cvmx_get_cycle() > done) {
+   result = -1;
+   break;
+   }
+
+   __delay(100);
+   }
+   return result;
+}
+
 static void cvmx_fifo_setup(struct octeon_hcd *usb)
 {
union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3;
@@ -635,11 +648,9 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb)
/* Flush all FIFOs */
USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfnum, 0x10);
USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfflsh, 1);
-   CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl,
- c.s.txfflsh == 0, 100);
+   cvmx_wait_for_field32(usb, address, 0, 100);
USB_SET_FIELD32(address, cvmx_usbcx_grstctl, rxfflsh, 1);
-   CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl,
- c.s.rxfflsh == 0, 100);
+   cvmx_wait_for_field32(usb, address, 1, 100);
 }
 
 /**
-- 
2.16.4

[PATCH v3 0/1] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel()

[Georgios Tsotsos]

Changed the function in order to use only values instead passing unions
i think its better than passing pointers there.

Georgios Tsotsos (1):
  Staging: octeon-usb: Breaks down cvmx_usb_poll_channel().

 drivers/staging/octeon-usb/octeon-hcd.c | 81 +
 1 file changed, 53 insertions(+), 28 deletions(-)

-- 
2.16.4

[PATCH v3 1/1] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel().

[Georgios Tsotsos]

In order to make this function more clear a new function created that controls
channels halt on no DMA mode.

Signed-off-by: Georgios Tsotsos 
---
 drivers/staging/octeon-usb/octeon-hcd.c | 81 +
 1 file changed, 53 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index 8a7bdf1a9fe6..3f44ac260eff 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -2593,7 +2593,51 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd 
*usb,
cvmx_usb_complete(usb, pipe, transaction, CVMX_USB_STATUS_OK);
}
 }
+/**
+ * Handles channels halt in non DMA mode
+ * @hcchar_chena:
+ * @hcint_xfercompl:
+ * @usb: USB device
+ * @channel: Channel to poll
+ *
+ * In non DMA mode the channels don't halt themselves. We need
+ * to manually disable channels that are left running
+ *
+ * Returns: -1 on halt
+ */
+static int cvmx_usb_dma_halt(u32 hcchar_chena, u32 hcint_xfercompl,
+struct octeon_hcd *usb, int channel)
+{
+   struct usb_hcd *hcd = octeon_to_hcd(usb);
+   struct device *dev = hcd->self.controller;
 
+   if (hcchar_chena) {
+   union cvmx_usbcx_hcintmskx hcintmsk;
+   union cvmx_usbcx_hccharx usbc_hcchar;
+   /* Disable all interrupts except CHHLTD */
+   hcintmsk.u32 = 0;
+   hcintmsk.s.chhltdmsk = 1;
+   cvmx_usb_write_csr32(usb,
+CVMX_USBCX_HCINTMSKX(channel, usb->index),
+hcintmsk.u32);
+   usbc_hcchar.s.chdis = 1;
+   cvmx_usb_write_csr32(usb,
+CVMX_USBCX_HCCHARX(channel, usb->index),
+usbc_hcchar.u32);
+   return -1;
+   } else if (hcint_xfercompl) {
+   /*
+* Successful IN/OUT with transfer complete.
+* Channel halt isn't needed.
+*/
+   } else {
+   dev_err(dev, "USB%d: Channel %d interrupt without halt\n",
+   usb->index, channel);
+   return -1;
+   }
+
+   return 0;
+}
 /**
  * Poll a channel for status
  *
@@ -2604,8 +2648,6 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb,
  */
 static int cvmx_usb_poll_channel(struct octeon_hcd *usb, int channel)
 {
-   struct usb_hcd *hcd = octeon_to_hcd(usb);
-   struct device *dev = hcd->self.controller;
union cvmx_usbcx_hcintx usbc_hcint;
union cvmx_usbcx_hctsizx usbc_hctsiz;
union cvmx_usbcx_hccharx usbc_hcchar;
@@ -2637,34 +2679,17 @@ static int cvmx_usb_poll_channel(struct octeon_hcd 
*usb, int channel)
return 0;
}
 
-   /*
-* In non DMA mode the channels don't halt themselves. We need
-* to manually disable channels that are left running
-*/
+   /* In case of non DMA mode handle halt */
if (!usbc_hcint.s.chhltd) {
-   if (usbc_hcchar.s.chena) {
-   union cvmx_usbcx_hcintmskx hcintmsk;
-   /* Disable all interrupts except CHHLTD */
-   hcintmsk.u32 = 0;
-   hcintmsk.s.chhltdmsk = 1;
-   cvmx_usb_write_csr32(usb,
-
CVMX_USBCX_HCINTMSKX(channel, usb->index),
-hcintmsk.u32);
-   usbc_hcchar.s.chdis = 1;
-   cvmx_usb_write_csr32(usb,
-
CVMX_USBCX_HCCHARX(channel, usb->index),
-usbc_hcchar.u32);
-   return 0;
-   } else if (usbc_hcint.s.xfercompl) {
-   /*
-* Successful IN/OUT with transfer complete.
-* Channel halt isn't needed.
-*/
-   } else {
-   dev_err(dev, "USB%d: Channel %d interrupt 
without halt\n",
-   usb->index, channel);
+   int dma_halt_status = 0;
+   u32 xfercompl = usbc_hcint.s.xfercompl;
+
+   dma_halt_status = cvmx_usb_dma_halt(usbc_hcchar.s.chena,
+   xfercompl,
+   usb, channel);
+
+   if (dma_halt_status < 0)
return 0;
-   }
}
} else {
  

Re: [PATCH v2 1/3] iio: adc: add support for mcp3911

[Jonathan Cameron]

On Tue, 24 Jul 2018 20:30:02 +0200
Marcus Folkesson  wrote:

> MCP3911 is a dual channel Analog Front End (AFE) containing two
> synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
> 
> Signed-off-by: Marcus Folkesson 
> Signed-off-by: Kent Gustavsson 
Hi

I think you missed cleaning up the clock enable in the remove.
If it is safe to not do so it should also be safe in the error path
of probe.

A couple of trivial other things jumped out at me whilst reading.

Thanks,

Jonathan
> ---
> 
> Notes:
> v2:
>   - cleanups and bugfixes (thanks Peter Meerwald-Stadler)
>   - drop hardware gain
>   - use the presence or lack of regulator to indicate if we go for 
> internal or external voltage reference
>   - do not store device node in private struct
>   - drop support to set width in devicetree
>   - use the presence or lack of clock to indicate if we go for internal 
> or external clock
> 
>  drivers/iio/adc/Kconfig   |  10 ++
>  drivers/iio/adc/Makefile  |   1 +
>  drivers/iio/adc/mcp3911.c | 366 
> ++
>  3 files changed, 377 insertions(+)
>  create mode 100644 drivers/iio/adc/mcp3911.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 15606f237480..f9a41fa96fcc 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -501,6 +501,16 @@ config MCP3422
> This driver can also be built as a module. If so, the module will be
> called mcp3422.
>  
> +config MCP3911
> + tristate "Microchip Technology MCP3911 driver"
> + depends on SPI
> + help
> +   Say yes here to build support for Microchip Technology's MCP3911
> +   analog to digital converter.
> +
> +   This driver can also be built as a module. If so, the module will be
> +   called mcp3911.
> +
>  config MEDIATEK_MT6577_AUXADC
>  tristate "MediaTek AUXADC driver"
>  depends on ARCH_MEDIATEK || COMPILE_TEST
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 28a9423997f3..3cfebfff7d26 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
>  obj-$(CONFIG_MAX9611) += max9611.o
>  obj-$(CONFIG_MCP320X) += mcp320x.o
>  obj-$(CONFIG_MCP3422) += mcp3422.o
> +obj-$(CONFIG_MCP3911) += mcp3911.o
>  obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
>  obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
>  obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
> diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
> new file mode 100644
> index ..29aa39930ead
> --- /dev/null
> +++ b/drivers/iio/adc/mcp3911.c
> @@ -0,0 +1,366 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Microchip MCP3911, Two-channel Analog Front End
> + *
> + * Copyright (C) 2018 Marcus Folkesson 
> + * Copyright (C) 2018 Kent Gustavsson 
> + *
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define MCP3911_REG_CHANNEL0 0x00
> +#define MCP3911_REG_CHANNEL1 0x03
> +#define MCP3911_REG_MOD  0x06
> +#define MCP3911_REG_PHASE0x07
> +#define MCP3911_REG_GAIN 0x09
> +
> +#define MCP3911_REG_STATUSCOM0x0a
> +#define MCP3911_STATUSCOM_CH1_24WIDTHBIT(4)
> +#define MCP3911_STATUSCOM_CH0_24WIDTHBIT(3)
> +#define MCP3911_STATUSCOM_EN_OFFCAL  BIT(2)
> +#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
> +
> +#define MCP3911_REG_CONFIG   0x0c
> +#define MCP3911_CONFIG_CLKEXTBIT(1)
> +#define MCP3911_CONFIG_VREFEXT   BIT(2)
> +
> +#define MCP3911_REG_OFFCAL_CH0   0x0e
> +#define MCP3911_REG_GAINCAL_CH0  0x11
> +#define MCP3911_REG_OFFCAL_CH1   0x14
> +#define MCP3911_REG_GAINCAL_CH1  0x17
> +#define MCP3911_REG_VREFCAL  0x1a
> +
> +#define MCP3911_CHANNEL(x)   (MCP3911_REG_CHANNEL0 + x * 3)
> +#define MCP3911_OFFCAL(x)(MCP3911_REG_OFFCAL_CH0 + x * 6)
> +
> +/* Internal voltage reference in uV */
> +#define MCP3911_INT_VREF_UV  120
> +
> +#define MCP3911_REG_READ(reg, id)reg) << 1) | ((id) << 5) | (1 << 
> 0)) & 0xff)
> +#define MCP3911_REG_WRITE(reg, id)   reg) << 1) | ((id) << 5) | (0 << 
> 0)) & 0xff)
> +
> +#define MCP3911_NUM_CHANNELS 2
> +
> +struct mcp3911 {
> + struct spi_device *spi;
> + struct mutex lock;
> + struct regulator *vref;
> + struct clk *adc_clk;
> + u32 dev_addr;
> +};
> +
> +static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
> +{
> + int ret;
> +
> + reg = MCP3911_REG_READ(reg, adc->dev_addr);
> + ret = spi_write_then_read(adc->spi, ®, 1, val, len);
> + if (ret < 0)
> + return ret;
> +
> + be32_to_cpus(val);
> + *val >>= ((4 - len) * 8);
> + dev_dbg(&adc->spi->dev, "reading 0x%x from register 0x%x\n", *val,
> +  

Re: candidates for @devel-rt localversion-rt++

[Mike Galbraith]

FYI, per kvm unit tests, 4.16-rt definitely has more kvm issues.

huawei5:/abuild/mike/kvm-unit-tests # uname -r
4.16.18-rt11-rt
huawei5:/abuild/mike/kvm-unit-tests # ./run_tests.sh
PASS selftest-setup (2 tests)
FAIL selftest-vectors-kernel 
FAIL selftest-vectors-user 
PASS selftest-smp (65 tests)
PASS pci-test (1 tests)
PASS pmu (3 tests)
FAIL gicv2-ipi 
FAIL gicv3-ipi 
FAIL gicv2-active 
FAIL gicv3-active 
PASS psci (4 tests)
FAIL timer 
huawei5:/abuild/mike/kvm-unit-tests #

4.14-rt passes all tests.  The above is with the kvm raw_spinlock_t
conversion patch applied, but the 4.12 based SLERT tree I cloned to
explore arm-land in the first place shows only one timer failure, and
has/needs it applied as well, which would seem to vindicate it.

huawei5:/abuild/mike/kvm-unit-tests # uname -r
4.12.14-0.gec0b559-rt
huawei5:/abuild/mike/kvm-unit-tests # ./run_tests.sh
PASS selftest-setup (2 tests)
PASS selftest-vectors-kernel (2 tests)
PASS selftest-vectors-user (2 tests)
PASS selftest-smp (65 tests)
PASS pci-test (1 tests)
PASS pmu (3 tests)
PASS gicv2-ipi (3 tests)
PASS gicv3-ipi (3 tests)
PASS gicv2-active (1 tests)
PASS gicv3-active (1 tests)
PASS psci (4 tests)
FAIL timer (8 tests, 1 unexpected failures)
huawei5:/abuild/mike/kvm-unit-tests #

Re: [PATCH v2 1/4] iio: adc: xilinx: Check for return values in clk related functions

[Jonathan Cameron]

On Mon, 23 Jul 2018 20:32:00 +0530
Manish Narani  wrote:

> This patch adds check for return values from clock related functions.
> This was reported by static code analysis tool.
> 
> Signed-off-by: Manish Narani 
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/xilinx-xadc-core.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/xilinx-xadc-core.c 
> b/drivers/iio/adc/xilinx-xadc-core.c
> index 0127e85..23395fc 100644
> --- a/drivers/iio/adc/xilinx-xadc-core.c
> +++ b/drivers/iio/adc/xilinx-xadc-core.c
> @@ -340,6 +340,8 @@ static int xadc_zynq_setup(struct platform_device *pdev,
>   xadc->zynq_intmask = ~0;
>  
>   pcap_rate = clk_get_rate(xadc->clk);
> + if (!pcap_rate)
> + return -EINVAL;
>  
>   if (tck_rate > pcap_rate / 2) {
>   div = 2;
> @@ -887,6 +889,9 @@ static int xadc_write_raw(struct iio_dev *indio_dev,
>   unsigned long clk_rate = xadc_get_dclk_rate(xadc);
>   unsigned int div;
>  
> + if (!clk_rate)
> + return -EINVAL;
> +
>   if (info != IIO_CHAN_INFO_SAMP_FREQ)
>   return -EINVAL;
>  
> @@ -1237,8 +1242,10 @@ static int xadc_probe(struct platform_device *pdev)
>   goto err_free_irq;
>  
>   /* Disable all alarms */
> - xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_ALARM_MASK,
> - XADC_CONF1_ALARM_MASK);
> + ret = xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_ALARM_MASK,
> +   XADC_CONF1_ALARM_MASK);
> + if (ret)
> + goto err_free_irq;
>  
>   /* Set thresholds to min/max */
>   for (i = 0; i < 16; i++) {

Hello Beautiful

[Jack]

Cześć Drogi, nazywam się Jack i szukam związku, w którym będę czuć się kochany 
po serii nieudanych związków.

Mam nadzieję, że byłbyś zainteresowany i moglibyśmy się lepiej poznać, jeśli 
nie masz nic przeciwko. Jestem otwarty na udzielanie odpowiedzi na pytania od 
ciebie, ponieważ uważam, że moje podejście jest trochę niewłaściwe. Mam 
nadzieję, że odezwę się od ciebie.

Jacek.

Re: [PATCH v2 2/4] iio: adc: xilinx: limit pcap clock frequency value

[Jonathan Cameron]

On Mon, 23 Jul 2018 20:32:01 +0530
Manish Narani  wrote:

> This patch limits the xadc pcap clock frequency value to be less than
> 200MHz. This fixes the issue when zynq is booted at higher frequency
> values, pcap crosses the maximum limit of 200MHz(Fmax) as it is derived
> from IOPLL.
> If this limit is crossed it is required to alter the WEDGE and REDGE
> bits of XADC_CFG register to make timings better in the interface. So to
> avoid alteration of these bits every time, the pcap value should not
> cross the Fmax limit.
> 
> Signed-off-by: Manish Narani 

Applied, to the togreg branch of iio.git.  If you want this backported
to stable, then request it once this patch is upstream.  It may be
sometime given we've probably just missed the coming merge window.

If you do need it faster then let me know and I'll look at moving
it over to the branch of fixes during the RC phases.

Jonathan

> ---
>  drivers/iio/adc/xilinx-xadc-core.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/iio/adc/xilinx-xadc-core.c 
> b/drivers/iio/adc/xilinx-xadc-core.c
> index 23395fc..0dd306d 100644
> --- a/drivers/iio/adc/xilinx-xadc-core.c
> +++ b/drivers/iio/adc/xilinx-xadc-core.c
> @@ -322,6 +322,7 @@ static irqreturn_t xadc_zynq_interrupt_handler(int irq, 
> void *devid)
>  
>  #define XADC_ZYNQ_TCK_RATE_MAX 5000
>  #define XADC_ZYNQ_IGAP_DEFAULT 20
> +#define XADC_ZYNQ_PCAP_RATE_MAX 2
>  
>  static int xadc_zynq_setup(struct platform_device *pdev,
>   struct iio_dev *indio_dev, int irq)
> @@ -332,6 +333,7 @@ static int xadc_zynq_setup(struct platform_device *pdev,
>   unsigned int div;
>   unsigned int igap;
>   unsigned int tck_rate;
> + int ret;
>  
>   /* TODO: Figure out how to make igap and tck_rate configurable */
>   igap = XADC_ZYNQ_IGAP_DEFAULT;
> @@ -343,6 +345,13 @@ static int xadc_zynq_setup(struct platform_device *pdev,
>   if (!pcap_rate)
>   return -EINVAL;
>  
> + if (pcap_rate > XADC_ZYNQ_PCAP_RATE_MAX) {
> + ret = clk_set_rate(xadc->clk,
> +(unsigned long)XADC_ZYNQ_PCAP_RATE_MAX);
> + if (ret)
> + return ret;
> + }
> +
>   if (tck_rate > pcap_rate / 2) {
>   div = 2;
>   } else {
> @@ -368,6 +377,12 @@ static int xadc_zynq_setup(struct platform_device *pdev,
>   XADC_ZYNQ_CFG_REDGE | XADC_ZYNQ_CFG_WEDGE |
>   tck_div | XADC_ZYNQ_CFG_IGAP(igap));
>  
> + if (pcap_rate > XADC_ZYNQ_PCAP_RATE_MAX) {
> + ret = clk_set_rate(xadc->clk, pcap_rate);
> + if (ret)
> + return ret;
> + }
> +
>   return 0;
>  }
>  

Re: [PATCH 03/10] smp,cpumask: introduce on_each_cpu_cond_mask

[Rik van Riel]

On Sat, 2018-07-28 at 19:57 -0700, Andy Lutomirski wrote:
> On Sat, Jul 28, 2018 at 2:53 PM, Rik van Riel 
> wrote:
> > Introduce a variant of on_each_cpu_cond that iterates only over the
> > CPUs in a cpumask, in order to avoid making callbacks for every
> > single
> > CPU in the system when we only need to test a subset.
> 
> Nice.
> 
> Although, if you want to be really fancy, you could optimize this (or
> add a variant) that does the callback on the local CPU in parallel
> with the remote ones.  That would give a small boost to TLB flushes.

The test_func callbacks are not run remotely, but on
the local CPU, before deciding who to send callbacks
to.

The actual IPIs are sent in parallel, if the cpumask
allocation succeeds (it always should in many kernel
configurations, and almost always in the rest).

-- 
All Rights Reversed.

signature.asc
Description: This is a digitally signed message part

Re: [PATCH 04/10] x86,mm: use on_each_cpu_cond for TLB flushes

[Rik van Riel]

On Sat, 2018-07-28 at 19:58 -0700, Andy Lutomirski wrote:
> On Sat, Jul 28, 2018 at 2:53 PM, Rik van Riel 
> wrote:
> > Instead of open coding bitmap magic, use on_each_cpu_cond
> > to determine which CPUs to send TLB flush IPIs to.
> > 
> > This might be a little bit slower than examining the bitmaps,
> > but it should be a lot easier to maintain in the long run.
> 
> Looks good.
> 
> i assume it's not easy to get the remove-tables case to do a single
> on_each_cpu_cond() instead of two?  Currently it's doing the lazy
> ones and the non-lazy ones separately.

Indeed. The TLB gather batch size means we need to send
IPIs to the non-lazy CPUs whenever we have gathered so
many pages that our tlb_gather data structure is full.

This could result in many IPIs during a large munmap.

The lazy CPUs get one IPI before page table freeing.

-- 
All Rights Reversed.

signature.asc
Description: This is a digitally signed message part

Re: [PATCH 10/10] mm,sched: conditionally skip lazy TLB mm refcounting

[Rik van Riel]

On Sat, 2018-07-28 at 21:21 -0700, Andy Lutomirski wrote:
> On Sat, Jul 28, 2018 at 2:53 PM, Rik van Riel 
> wrote:
> > Conditionally skip lazy TLB mm refcounting. When an architecture
> > has
> > CONFIG_ARCH_NO_ACTIVE_MM_REFCOUNTING enabled, an mm that is used in
> > lazy TLB mode anywhere will get shot down from exit_mmap, and there
> > in no need to incur the cache line bouncing overhead of refcounting
> > a lazy TLB mm.
> 
> Unless I've misunderstood something, this patch results in idle tasks
> whose active_mm has been freed still having active_mm pointing at
> freed memory. 

Patch 9/10 is supposed to ensure that the lazy TLB CPUs get
switched to init_mm before an mm is freed. No CPU should ever
have its active_mm pointing at a freed mm.

Your message made me re-read the code, and now I realize that
leave_mm does not actually do that.

Looking at the other callers of leave_mm, I might not be the
only one surprised by that; xen_drop_mm_ref comes to mind.

I guess I should some code to leave_mm to have it actually
clear active_mm and call the conditional refcount drop helper
function.

Does that clear up the confusion?

-- 
All Rights Reversed.

signature.asc
Description: This is a digitally signed message part

جميع البرامج و الماجستير بشهادة جامعة ميزورى بخصم 50% فى حال التنفيذ بالقاهرة

[sahar saher]

*السلام عليكم ورحمة الله وبركاته*

*يهديكــم الاتحــاد العربــي لتنميــة المـــوارد البشريـــة*

*باعتمـــاد/ جامعـــة ميزوري – الولايــات المتحــدة الأمريكيــة*

*الماجستير المهني المصغر في ادارة الموارد البشرية*

*Mini MBA HRM*

*خلال شهرى  اغسطس - سبتمبر - اكتوبر 2018 م مكان الانعقاد (**القاهرة -
اسطنبول** )*

*بهدف **ترسيخ مفهوم أن الموارد البشرية اصل من أصول المنظمة، وكيفية اختيار
طرق التعامل مع الموارد البشرية بما يضمن فعاليتها وتزيد من قدرتها على تحقيق
الأهداف، والعرض للمفاهيم والمعارف الحديثة في مجال إدارة الموارد البشرية،
وتحديد القدرات اللازمة للتعامل مع بيئة المنظمة المرتبطة بسوق العمل، كان
انعقاد هذا البرنامج المهني الذي يتناول تخطيط الاحتياجات* *من الموارد
البشرية، الحصول على الموارد البشرية وتدريبها وتحفيزها وترقيتها، تخطيط
وتنميه المسار الوظيفي للموارد البشرية، إدارة تدريب وتنميه مهارات مديري
الموارد البشرية الأساليب الحديثة في تطوير وبناء القدرات البشرية، الادارة
الاستراتيجية للموارد البشرية في ضوء متغيرات العولمة، السياسات العامة لإدارة
الموارد البشرية، تكنولوجيا الموارد البشرية والأداء البشرى البعد الاقتصادي
والمالي والتسويقي لإدارة الموارد البشرية، اختيار الكفاءات مدخل لخفض تكلفه
التدريب، سمات وتحديات عصر العولمة على نظام العمل الجدي .*

*المشاركون المستهدفون:*

*القيادات الادارية والاشرافية فى القطاعات الحكومية والخاصة*

*موظفين ومشرفين ومدراء اقسام وإدارات الموارد البشرية*

*موظفين ومشرفين ومدراء اقسام وادارات التطوير الاداري*

*موظفين ومشرفين ومدراء اقسام وادارات الشؤون الادارية*

*موظفين ومشرفين ومدراء اقسام وادارات شؤون الموظفين*

*موظفين ومشرفين ومدراء اقسام وادارات تخطيط القوي العاملة*

*مستشاري التدريب والموارد البشرية والتطوير المؤسسي*

*وبهذه المناسبة / يسعدنا دعوتكم للمشاركة وتعميم خطابنا على المهتمين بموضوع
الماجستير وإفادتنا بمن تقترحون توجيه الدعوة لهم علما بان رسوم الاشتراك 2900
دولار امريكي .*

*و في حال تنفيذ البرنامج بالقاهرة 1500 دولار امريكي .*

*اولا: وحدة الماجستيرالمهنى المصغر المعتمد من جامعة ميزورى الامريكية.*

م

اسم البرنامج

الرسوم فى القاهرة

الرسوم فى اسطنبول

1

الماجستير المهنى المصغرفى ادارة الاعمال

1500دولار

2900 دولار

2

الماجستير المهنى المصغر فى ادارة المستشفيات

1500دولار

2900 دولار

3

الماجستير المهنى المصغر فى الادارة التعليمية

1500دولار

2900 دولار

4

الماجستير المهنى المصغر فى التخطيط الاستراتيجي

1500دولار

2900 دولار

5

الماجستير المهنى المصغر فى ادراة النفط والغاز

1500دولار

2900 دولار

6

الماجستير المهنى المصغر فى ادراة الرعاية الصحية

1500دولار

2900 دولار

8

الماجستير المهنى المصغر فى الادارة المدرسية

1500دولار

2900 دولار

ثانيا:وحدة الشهادات المتخصصة المعتمدة من جامعة ميزورى الامريكية.

م

اسم البرنامج

الرسوم فى القاهرة

الرسوم فى اسطنبول

1

شهادة المدير التنفيذى

1000 دولار

2000دولار

2

الخدمات المصرفية الاسلامية

1000 دولار

2000دولار

3

شهادة المدير التنفيذى المعتمد

1000 دولار

2000دولار

4

شهادة مراجع الحسابات الداخلى

1000 دولار

2000دولار

5

شهادة قانون الشركات

1000 دولار

2000دولار

*خطة الماجستير و البرامج المعتمدة من جامعة ميزورى الامريكية 2018م*


*لمزيد من المعلومات برجاء الاتصال*

*ادارة التدريب الاستاذة : رادا صبرى*

*جوال & واتس/ 00201027731551*

*هاتف/ 0020235866963 / 0020235860262 / 0020235860290*

*فاكس/ 0020235830285 / 0020235866953 / 0020235849316*

*r...@uhrda.net* 


*radasabr...@gmail.com* 

Re: [PATCH V5 3/3] PCI: Mask and unmask hotplug interrupts during reset

[Lukas Wunner]

On Tue, Jul 03, 2018 at 07:30:28AM -0400, ok...@codeaurora.org wrote:
> On 2018-07-03 04:34, Lukas Wunner wrote:
> > On Mon, Jul 02, 2018 at 06:52:47PM -0400, Sinan Kaya wrote:
> > > If a bridge supports hotplug and observes a PCIe fatal error, the
> > > following events happen:
> > >
> > > 1. AER driver removes the devices from PCI tree on fatal error
> > > 2. AER driver brings down the link by issuing a secondary bus reset
> > >waits for the link to come up.
> > > 3. Hotplug driver observes a link down interrupt
> > > 4. Hotplug driver tries to remove the devices waiting for the rescan
> > >lock but devices are already removed by the AER driver and AER
> > >driver is waiting for the link to come back up.
> > > 5. AER driver tries to re-enumerate devices after polling for the link
> > >state to go up.
> > > 6. Hotplug driver obtains the lock and tries to remove the devices
> > >again.
> > >
> > > If a bridge is a hotplug capable bridge, mask hotplug interrupts
> > > before the reset and unmask afterwards.
> >
> > Would it work for you if you just amended the AER driver to skip
> > removal and re-enumeration of devices if the port is a hotplug bridge?
> > Just check for is_hotplug_bridge in struct pci_dev.
> 
> The reason why we want to remove devices before secondary bus reset is to
> quiesce pcie bus traffic before issuing a reset.
> 
> Skipping this step might cause transactions to be lost in the middle of
> the reset as there will be active traffic flowing and drivers will
> suddenly start reading ffs.

That's par for the course for any hotplug bridge with support for
surprise removal (e.g. Thunderbolt) and drivers must be able to
cope with it.  Nothing to worry about IMHO.

Thanks,

Lukas

Re: [TRIVIAL RFC PATCH] Kconfigs - reduce use of "depends on EXPERT"

[Michael Büsch]

On Sat, 28 Jul 2018 15:13:00 -0700
Joe Perches  wrote:

>  config SSB_SILENT
> - bool "No SSB kernel messages"
> - depends on SSB && EXPERT
> + bool "No SSB kernel messages" if EXPERT
> + depends on SSB
>   help
> This option turns off all Sonics Silicon Backplane printks.
> Note that you won't be able to identify problems, once


What about removing this option entirely?
We would just have to remove it from Kconfig and from its only use in
drivers/ssb/ssb_private.h
I don't think anybody uses (or should use) this option anyway.
That would reduce the EXPERT dependencies by one, which is a good
thing. :)

-- 
Michael


pgpx9pHV1ovN2.pgp
Description: OpenPGP digital signature

Re: [PATCH v2 3/4] iio: adc: xilinx: Remove platform_get_irq from xadc_remove function

[Jonathan Cameron]

On Mon, 23 Jul 2018 20:32:02 +0530
Manish Narani  wrote:

> This patch avoids getting irq number in xadc_remove function. Instead
> store 'irq' in xadc struct and use xadc->irq wherever needed.
> This patch also resolves a warning reported by coverity where it asks to
> check return value of platform_get_irq() for any errors in xadc_remove.
> 
> Signed-off-by: Manish Narani 
Applied. 

Thanks,

Jonathan

> ---
>  drivers/iio/adc/xilinx-xadc-core.c | 10 +-
>  drivers/iio/adc/xilinx-xadc.h  |  1 +
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/xilinx-xadc-core.c 
> b/drivers/iio/adc/xilinx-xadc-core.c
> index 0dd306d..44a2519 100644
> --- a/drivers/iio/adc/xilinx-xadc-core.c
> +++ b/drivers/iio/adc/xilinx-xadc-core.c
> @@ -1175,6 +1175,7 @@ static int xadc_probe(struct platform_device *pdev)
>  
>   xadc = iio_priv(indio_dev);
>   xadc->ops = id->data;
> + xadc->irq = irq;
>   init_completion(&xadc->completion);
>   mutex_init(&xadc->mutex);
>   spin_lock_init(&xadc->lock);
> @@ -1225,11 +1226,11 @@ static int xadc_probe(struct platform_device *pdev)
>   if (ret)
>   goto err_free_samplerate_trigger;
>  
> - ret = xadc->ops->setup(pdev, indio_dev, irq);
> + ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
>   if (ret)
>   goto err_clk_disable_unprepare;
>  
> - ret = request_irq(irq, xadc->ops->interrupt_handler, 0,
> + ret = request_irq(xadc->irq, xadc->ops->interrupt_handler, 0,
>   dev_name(&pdev->dev), indio_dev);
>   if (ret)
>   goto err_clk_disable_unprepare;
> @@ -1288,7 +1289,7 @@ static int xadc_probe(struct platform_device *pdev)
>   return 0;
>  
>  err_free_irq:
> - free_irq(irq, indio_dev);
> + free_irq(xadc->irq, indio_dev);
>  err_clk_disable_unprepare:
>   clk_disable_unprepare(xadc->clk);
>  err_free_samplerate_trigger:
> @@ -1310,7 +1311,6 @@ static int xadc_remove(struct platform_device *pdev)
>  {
>   struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>   struct xadc *xadc = iio_priv(indio_dev);
> - int irq = platform_get_irq(pdev, 0);
>  
>   iio_device_unregister(indio_dev);
>   if (xadc->ops->flags & XADC_FLAGS_BUFFERED) {
> @@ -1318,7 +1318,7 @@ static int xadc_remove(struct platform_device *pdev)
>   iio_trigger_free(xadc->convst_trigger);
>   iio_triggered_buffer_cleanup(indio_dev);
>   }
> - free_irq(irq, indio_dev);
> + free_irq(xadc->irq, indio_dev);
>   clk_disable_unprepare(xadc->clk);
>   cancel_delayed_work(&xadc->zynq_unmask_work);
>   kfree(xadc->data);
> diff --git a/drivers/iio/adc/xilinx-xadc.h b/drivers/iio/adc/xilinx-xadc.h
> index 62edbda..8c00095 100644
> --- a/drivers/iio/adc/xilinx-xadc.h
> +++ b/drivers/iio/adc/xilinx-xadc.h
> @@ -68,6 +68,7 @@ struct xadc {
>   spinlock_t lock;
>  
>   struct completion completion;
> + int irq;
>  };
>  
>  struct xadc_ops {

Re: [PATCH v2 4/4] iio: adc: xilinx: Move request_irq before enabling interrupts

[Jonathan Cameron]

On Mon, 23 Jul 2018 20:32:03 +0530
Manish Narani  wrote:

> Enabling the Interrupts before registering the irq handler is a bad
> idea. This patch corrects the same for XADC driver.
> 
> Signed-off-by: Manish Narani 
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/xilinx-xadc-core.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/xilinx-xadc-core.c 
> b/drivers/iio/adc/xilinx-xadc-core.c
> index 44a2519..3f6be5a 100644
> --- a/drivers/iio/adc/xilinx-xadc-core.c
> +++ b/drivers/iio/adc/xilinx-xadc-core.c
> @@ -1226,15 +1226,15 @@ static int xadc_probe(struct platform_device *pdev)
>   if (ret)
>   goto err_free_samplerate_trigger;
>  
> - ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
> - if (ret)
> - goto err_clk_disable_unprepare;
> -
>   ret = request_irq(xadc->irq, xadc->ops->interrupt_handler, 0,
>   dev_name(&pdev->dev), indio_dev);
>   if (ret)
>   goto err_clk_disable_unprepare;
>  
> + ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
> + if (ret)
> + goto err_free_irq;
> +
>   for (i = 0; i < 16; i++)
>   xadc_read_adc_reg(xadc, XADC_REG_THRESHOLD(i),
>   &xadc->threshold[i]);

Re: [PATCH V5 3/3] PCI: Mask and unmask hotplug interrupts during reset

[Lukas Wunner]

On Tue, Jul 03, 2018 at 06:41:33PM +0530, p...@codeaurora.org wrote:
> pciehp_unconfigure_device doing little more than enumeration to quiescence
> the bus.
> 
>   /*
>* Ensure that no new Requests will be generated from
>* the device.
>*/
>   if (presence) {
>   pci_read_config_word(dev, PCI_COMMAND, &command);
>   command &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_SERR);
>   command |= PCI_COMMAND_INTX_DISABLE;
>   pci_write_config_word(dev, PCI_COMMAND, command);
>   }

That piece of code is supposed to be executed on safe removal via sysfs
or an Attention Button press:  The card remains in the slot, even though
the slot is brought down.  So the card is quiesced.

However IIUC, on fatal error the link goes down and for pciehp, that's
essentially a surprise removal.  In that case, the above code is not
intended to be executed, rather the devices below the hotplug bridge
are marked disconnected.  See this patch I posted yesterday:
https://www.spinics.net/lists/linux-pci/msg74763.html

Thanks,

Lukas

Re: [PATCH v3 1/1] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel().

[Greg Kroah-Hartman]

On Sun, Jul 29, 2018 at 02:41:53PM +0300, Georgios Tsotsos wrote:
> In order to make this function more clear a new function created that controls
> channels halt on no DMA mode.
> 
> Signed-off-by: Georgios Tsotsos 
> ---
>  drivers/staging/octeon-usb/octeon-hcd.c | 81 
> +
>  1 file changed, 53 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
> b/drivers/staging/octeon-usb/octeon-hcd.c
> index 8a7bdf1a9fe6..3f44ac260eff 100644
> --- a/drivers/staging/octeon-usb/octeon-hcd.c
> +++ b/drivers/staging/octeon-usb/octeon-hcd.c
> @@ -2593,7 +2593,51 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd 
> *usb,
>   cvmx_usb_complete(usb, pipe, transaction, CVMX_USB_STATUS_OK);
>   }
>  }
> +/**

Blank line between functions please.

Also, as this is not a global function, no need for kerneldoc
formatting, but you did it already, so no big deal.

> + * Handles channels halt in non DMA mode
> + * @hcchar_chena:
> + * @hcint_xfercompl:
> + * @usb: USB device
> + * @channel: Channel to poll
> + *
> + * In non DMA mode the channels don't halt themselves. We need
> + * to manually disable channels that are left running
> + *
> + * Returns: -1 on halt
> + */
> +static int cvmx_usb_dma_halt(u32 hcchar_chena, u32 hcint_xfercompl,
> +  struct octeon_hcd *usb, int channel)
> +{
> + struct usb_hcd *hcd = octeon_to_hcd(usb);
> + struct device *dev = hcd->self.controller;
>  
> + if (hcchar_chena) {
> + union cvmx_usbcx_hcintmskx hcintmsk;
> + union cvmx_usbcx_hccharx usbc_hcchar;
> + /* Disable all interrupts except CHHLTD */
> + hcintmsk.u32 = 0;
> + hcintmsk.s.chhltdmsk = 1;
> + cvmx_usb_write_csr32(usb,
> +  CVMX_USBCX_HCINTMSKX(channel, usb->index),
> +  hcintmsk.u32);
> + usbc_hcchar.s.chdis = 1;
> + cvmx_usb_write_csr32(usb,
> +  CVMX_USBCX_HCCHARX(channel, usb->index),
> +  usbc_hcchar.u32);
> + return -1;

Do not make up error values, return -EINVAL or something like that (what
ever the real error here is.)

> + } else if (hcint_xfercompl) {
> + /*
> +  * Successful IN/OUT with transfer complete.
> +  * Channel halt isn't needed.
> +  */
> + } else {
> + dev_err(dev, "USB%d: Channel %d interrupt without halt\n",
> + usb->index, channel);
> + return -1;

Same here.

thanks,

greg k-h

Re: [PATCH v3 1/2] Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL

[Greg Kroah-Hartman]

On Sun, Jul 29, 2018 at 02:40:34PM +0300, Georgios Tsotsos wrote:
> Assign to variable the result of CVMX_USBCX_GRSTCTL instead of multiple
> calling a macro.
> 
> Signed-off-by: Georgios Tsotsos 
> ---
>  drivers/staging/octeon-usb/octeon-hcd.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)

What changed from previous versions?  You should always list that below
the --- line so that we know what is going on...

v4?  :)

thanks,

greg k-h

RE: Privileged and Confidential:

[AHMED KARIM]

RE: Privileged and Confidential:
 
Here I brought a potential Business Proposal at your door step for 
consideration.
 
I have a client that is interested to Invest in your Country and would like to 
engage you and your company on this project. The Investment Amount is valued at 
US$500 million.
 
If you are interested, kindly include your direct telephone numbers for full 
discussion of this offer when responding to this email.
 
Respectfully,
 
AHMED KARIM
Email reply here mohamedabdul1...@gmail.com

partnership offer

[Rosarita Houmam]

I want us to join hands as partners because i have a deal for you

[PATCH v4 2/2] Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function call.

[Georgios Tsotsos]

Replacing CVMX_WAIT_FOR_FIELD32 macro with equivalent function.

Signed-off-by: Georgios Tsotsos 
---
v2: Changed CVMX_WAIT_FOR_FIELD32 syntax to avoid checkpatch check notice and
tried to make the macro more readable.
v3: Changed CVMX_WAIT_FOR_FIELD32 macro to function according as refereed in 
commit message and suggested by Greg Kroah-Hartman
v4: Added patch version text

 drivers/staging/octeon-usb/octeon-hcd.c | 65 +++--
 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index 4615133292b5..8a7bdf1a9fe6 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -377,29 +377,6 @@ struct octeon_hcd {
struct cvmx_usb_tx_fifo nonperiodic;
 };
 
-/* This macro spins on a register waiting for it to reach a condition. */
-#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \
-   ({int result;   \
-   do {\
-   u64 done = cvmx_get_cycle() + (u64)timeout_usec *   \
-  octeon_get_clock_rate() / 100;   \
-   union _union c; \
-   \
-   while (1) { \
-   c.u32 = cvmx_usb_read_csr32(usb, address);  \
-   \
-   if (cond) { \
-   result = 0; \
-   break;  \
-   } else if (cvmx_get_cycle() > done) {   \
-   result = -1;\
-   break;  \
-   } else  \
-   __delay(100);   \
-   }   \
-   } while (0);\
-   result; })
-
 /*
  * This macro logically sets a single field in a CSR. It does the sequence
  * read, modify, and write
@@ -593,6 +570,42 @@ static inline int cvmx_usb_get_data_pid(struct 
cvmx_usb_pipe *pipe)
return 0; /* Data0 */
 }
 
+/**
+ * Loop through register until txfflsh or rxfflsh become zero.
+ *
+ * @usb:   USB block
+ * @address:   64bit address to read
+ * @timeout_usec:  Timeout
+ * @fflsh_type:Indicates fflsh type, 0 for txfflsh, 1 for rxfflsh
+ *
+ */
+static int cvmx_wait_for_field32(struct octeon_hcd *usb, u64 address,
+u64 timeout_usec, int fflsh_type)
+{
+   int result;
+   u64 done = cvmx_get_cycle() + timeout_usec *
+  (u64)octeon_get_clock_rate / 100;
+
+   union cvmx_usbcx_grstctl c;
+
+   while (1) {
+   c.u32 = cvmx_usb_read_csr32(usb, address);
+   if (fflsh_type == 0 && c.s.txfflsh == 0) {
+   result = 0;
+   break;
+   } else if (fflsh_type == 1 && c.s.rxfflsh == 0) {
+   result = 0;
+   break;
+   } else if (cvmx_get_cycle() > done) {
+   result = -1;
+   break;
+   }
+
+   __delay(100);
+   }
+   return result;
+}
+
 static void cvmx_fifo_setup(struct octeon_hcd *usb)
 {
union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3;
@@ -635,11 +648,9 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb)
/* Flush all FIFOs */
USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfnum, 0x10);
USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfflsh, 1);
-   CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl,
- c.s.txfflsh == 0, 100);
+   cvmx_wait_for_field32(usb, address, 0, 100);
USB_SET_FIELD32(address, cvmx_usbcx_grstctl, rxfflsh, 1);
-   CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl,
- c.s.rxfflsh == 0, 100);
+   cvmx_wait_for_field32(usb, address, 1, 100);
 }
 
 /**
-- 
2.16.4

[PATCH v4 1/2] Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL

[Georgios Tsotsos]

Assign to variable the result of CVMX_USBCX_GRSTCTL instead of multiple
calling a macro.

Signed-off-by: Georgios Tsotsos 
---
v2: It wasn't exist in this or earlier versions of patch series
v3: It seems a logical to avoid multiple calls of CVMX_USBCX_GRSTCTL that  will
also help cleaning up calls of CVMX_WAIT_FOR_FIELD32
v4: Added patch version text

 drivers/staging/octeon-usb/octeon-hcd.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index cff5e790b196..4615133292b5 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -598,6 +598,7 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb)
union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3;
union cvmx_usbcx_gnptxfsiz npsiz;
union cvmx_usbcx_hptxfsiz psiz;
+   u64 address;
 
usbcx_ghwcfg3.u32 = cvmx_usb_read_csr32(usb,
CVMX_USBCX_GHWCFG3(usb->index));
@@ -629,17 +630,16 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb)
psiz.s.ptxfstaddr = 3 * usbcx_ghwcfg3.s.dfifodepth / 4;
cvmx_usb_write_csr32(usb, CVMX_USBCX_HPTXFSIZ(usb->index), psiz.u32);
 
+   address = CVMX_USBCX_GRSTCTL(usb->index);
+
/* Flush all FIFOs */
-   USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
-   cvmx_usbcx_grstctl, txfnum, 0x10);
-   USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
-   cvmx_usbcx_grstctl, txfflsh, 1);
-   CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
- cvmx_usbcx_grstctl, c.s.txfflsh == 0, 100);
-   USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
-   cvmx_usbcx_grstctl, rxfflsh, 1);
-   CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
- cvmx_usbcx_grstctl, c.s.rxfflsh == 0, 100);
+   USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfnum, 0x10);
+   USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfflsh, 1);
+   CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl,
+ c.s.txfflsh == 0, 100);
+   USB_SET_FIELD32(address, cvmx_usbcx_grstctl, rxfflsh, 1);
+   CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl,
+ c.s.rxfflsh == 0, 100);
 }
 
 /**
-- 
2.16.4

Re: [PATCH V2] riscv: Convert uses of REG_FMT to %p

[Andy Shevchenko]

On Sat, Jul 28, 2018 at 7:39 PM, Joe Perches  wrote:
> Use %p pointer output instead of REG_FMT and cast the unsigned longs to
> (void *) to avoid exposing kernel addresses.
>
> Miscellanea:
>
> o Convert pr_cont to printk(KERN_DEFAULT as these uses are
>   new logging lines and not previous line continuations
> o Remove the now unused REG_FMT defines

> +   pr_alert("Unable to handle kernel %s at virtual address %p\n",

> +(addr < PAGE_SIZE) ? "NULL pointer dereference" :
> +"paging request", (void *)addr);

Hmm... This is what printf() should take care of.

Petr, what is the status of your series to unify this kind of
addresses to be printed?

-- 
With Best Regards,
Andy Shevchenko

RE: Done

[Walker, Alan]

$ 3million donated to you, E-mail 
tsouam...@gmail.com for funds confirmation.

[PATCH v4 1/1] Staging: octeon-usb: Using defined error codes and applying coding style

[Georgios Tsotsos]

Replaced -1 with defined error code EINVAL

Signed-off-by: Georgios Tsotsos 
---
v2: Apply coding style to function cvmx_usb_poll_channel
v3: Break down function cvmx_usb_poll_channel
v4: Return defined error code and applying coding style for function calls
 drivers/staging/octeon-usb/octeon-hcd.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index 3f44ac260eff..edf87d1b3609 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -2590,6 +2590,7 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb,
}
} else {
pipe->next_tx_frame += pipe->interval;
+
cvmx_usb_complete(usb, pipe, transaction, CVMX_USB_STATUS_OK);
}
 }
@@ -2624,16 +2625,16 @@ static int cvmx_usb_dma_halt(u32 hcchar_chena, u32 
hcint_xfercompl,
cvmx_usb_write_csr32(usb,
 CVMX_USBCX_HCCHARX(channel, usb->index),
 usbc_hcchar.u32);
-   return -1;
+   return -EINVAL;
} else if (hcint_xfercompl) {
/*
 * Successful IN/OUT with transfer complete.
 * Channel halt isn't needed.
 */
} else {
-   dev_err(dev, "USB%d: Channel %d interrupt without halt\n",
+   dev_err(dev, "USB%d: Channel %d interrupt without halt\n",
usb->index, channel);
-   return -1;
+   return -EINVAL;
}
 
return 0;
-- 
2.16.4

[PATCH] Staging: octeon: Fixing coding style for minor notices.

[Georgios Tsotsos]

Fixing coding style for a few lines that were reported to check from
checkpatch.pl in minor cases for alignment and ending with parenthesis.

Signed-off-by: Georgios Tsotsos 
---
 drivers/staging/octeon/ethernet.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/octeon/ethernet.c 
b/drivers/staging/octeon/ethernet.c
index 9b15c9ed844b..1e258deecacc 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -141,8 +141,8 @@ static void cvm_oct_periodic_worker(struct work_struct 
*work)
if (priv->poll)
priv->poll(cvm_oct_device[priv->port]);
 
-   cvm_oct_device[priv->port]->netdev_ops->ndo_get_stats(
-   cvm_oct_device[priv->port]);
+   cvm_oct_device[priv->port]->netdev_ops
+   ->ndo_get_stats(cvm_oct_device[priv->port]);
 
if (!atomic_read(&cvm_oct_poll_queue_stopping))
schedule_delayed_work(&priv->port_periodic_work, HZ);
@@ -621,8 +621,8 @@ static const struct net_device_ops cvm_oct_pow_netdev_ops = 
{
 #endif
 };
 
-static struct device_node *cvm_oct_of_get_child(
-   const struct device_node *parent, int reg_val)
+static struct device_node *cvm_oct_of_get_child(const struct device_node 
*parent
+   , int reg_val)
 {
struct device_node *node = NULL;
int size;
@@ -818,7 +818,7 @@ static int cvm_oct_probe(struct platform_device *pdev)
priv = netdev_priv(dev);
priv->netdev = dev;
priv->of_node = cvm_oct_node_for_port(pip, interface,
-   port_index);
+ port_index);
 
INIT_DELAYED_WORK(&priv->port_periodic_work,
  cvm_oct_periodic_worker);
-- 
2.16.4

Re: [PATCH v3 1/1] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel().

[Georgios Tsotsos]

Hello,
Regarding your latest comment, i have notice many functions in this module
using kerneldoc and not been global, also there are various erroneous
situations that functions return not defined error codes.
I will try to fix them all and a new patch series for those two issues after
this one is ok. (I hope with less versions :))
On Sun, 29 Jul 2018 at 15:43, Greg Kroah-Hartman
 wrote:
>
> On Sun, Jul 29, 2018 at 02:41:53PM +0300, Georgios Tsotsos wrote:
> > In order to make this function more clear a new function created that 
> > controls
> > channels halt on no DMA mode.
> >
> > Signed-off-by: Georgios Tsotsos 
> > ---
> >  drivers/staging/octeon-usb/octeon-hcd.c | 81 
> > +
> >  1 file changed, 53 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
> > b/drivers/staging/octeon-usb/octeon-hcd.c
> > index 8a7bdf1a9fe6..3f44ac260eff 100644
> > --- a/drivers/staging/octeon-usb/octeon-hcd.c
> > +++ b/drivers/staging/octeon-usb/octeon-hcd.c
> > @@ -2593,7 +2593,51 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd 
> > *usb,
> >   cvmx_usb_complete(usb, pipe, transaction, CVMX_USB_STATUS_OK);
> >   }
> >  }
> > +/**
>
> Blank line between functions please.
>
> Also, as this is not a global function, no need for kerneldoc
> formatting, but you did it already, so no big deal.
>
> > + * Handles channels halt in non DMA mode
> > + * @hcchar_chena:
> > + * @hcint_xfercompl:
> > + * @usb: USB device
> > + * @channel: Channel to poll
> > + *
> > + * In non DMA mode the channels don't halt themselves. We need
> > + * to manually disable channels that are left running
> > + *
> > + * Returns: -1 on halt
> > + */
> > +static int cvmx_usb_dma_halt(u32 hcchar_chena, u32 hcint_xfercompl,
> > +  struct octeon_hcd *usb, int channel)
> > +{
> > + struct usb_hcd *hcd = octeon_to_hcd(usb);
> > + struct device *dev = hcd->self.controller;
> >
> > + if (hcchar_chena) {
> > + union cvmx_usbcx_hcintmskx hcintmsk;
> > + union cvmx_usbcx_hccharx usbc_hcchar;
> > + /* Disable all interrupts except CHHLTD */
> > + hcintmsk.u32 = 0;
> > + hcintmsk.s.chhltdmsk = 1;
> > + cvmx_usb_write_csr32(usb,
> > +  CVMX_USBCX_HCINTMSKX(channel, 
> > usb->index),
> > +  hcintmsk.u32);
> > + usbc_hcchar.s.chdis = 1;
> > + cvmx_usb_write_csr32(usb,
> > +  CVMX_USBCX_HCCHARX(channel, usb->index),
> > +  usbc_hcchar.u32);
> > + return -1;
>
> Do not make up error values, return -EINVAL or something like that (what
> ever the real error here is.)
>
> > + } else if (hcint_xfercompl) {
> > + /*
> > +  * Successful IN/OUT with transfer complete.
> > +  * Channel halt isn't needed.
> > +  */
> > + } else {
> > + dev_err(dev, "USB%d: Channel %d interrupt without halt\n",
> > + usb->index, channel);
> > + return -1;
>
> Same here.
>
> thanks,
>
> greg k-h



--
Best regards!
Georgios Tsotsos
Greece-Evia-Chalkida
tsot...@linux.com
skype: tsotsos

Georgios Tsotsos
*Greece - Evia - Chalkida*
tsotsos[at]linux.com
skype: tsotsos

Hello Beautiful

[Jack]

Hi Dear, my name is Jack and i am seeking for a relationship in which i will 
feel loved after a series of failed relationships. 

I am hoping that you would be interested and we could possibly get to know each 
other more if you do not mind. I am open to answering questions from you as i 
think my approach is a little inappropriate. Hope to hear back from you.

Jack.

RE: Done

[Walker, Alan]

$ 3million donated to you, E-mail 
tsouam...@gmail.com for funds confirmation.

Re: [PATCH 10/10] mm,sched: conditionally skip lazy TLB mm refcounting

[Andy Lutomirski]

> On Jul 29, 2018, at 5:11 AM, Rik van Riel  wrote:
> 
>> On Sat, 2018-07-28 at 21:21 -0700, Andy Lutomirski wrote:
>> On Sat, Jul 28, 2018 at 2:53 PM, Rik van Riel 
>> wrote:
>>> Conditionally skip lazy TLB mm refcounting. When an architecture
>>> has
>>> CONFIG_ARCH_NO_ACTIVE_MM_REFCOUNTING enabled, an mm that is used in
>>> lazy TLB mode anywhere will get shot down from exit_mmap, and there
>>> in no need to incur the cache line bouncing overhead of refcounting
>>> a lazy TLB mm.
>> 
>> Unless I've misunderstood something, this patch results in idle tasks
>> whose active_mm has been freed still having active_mm pointing at
>> freed memory. 
> 
> Patch 9/10 is supposed to ensure that the lazy TLB CPUs get
> switched to init_mm before an mm is freed. No CPU should ever
> have its active_mm pointing at a freed mm.
> 
> Your message made me re-read the code, and now I realize that
> leave_mm does not actually do that.
> 
> Looking at the other callers of leave_mm, I might not be the
> only one surprised by that; xen_drop_mm_ref comes to mind.
> 
> I guess I should some code to leave_mm to have it actually
> clear active_mm and call the conditional refcount drop helper
> function.
> 
> Does that clear up the confusion?
> 

Kind of. But what’s the point of keeping active_mm?  On architectures that opt 
in to the new mode, there won’t be any code that cares about it’s value.  
What’s the benefit of keeping it around?  If you ifdef it out, then it can’t 
possibly point to freed memory, and there’s nothing to worry about.

Cont: power_supply: Introduce power supply charging driver

[Andy Shevchenko]

Hi!

It has been a while. I didn't get from archives what happened to the
patch series [1]? Is it still actual to apply?

[1]: https://lwn.net/Articles/608527/

-- 
Andy Shevchenko 
Intel Finland Oy

Re: [PATCH 03/10] smp,cpumask: introduce on_each_cpu_cond_mask

[Andy Lutomirski]

>> On Jul 29, 2018, at 5:00 AM, Rik van Riel  wrote:
>> 
>> On Sat, 2018-07-28 at 19:57 -0700, Andy Lutomirski wrote:
>> On Sat, Jul 28, 2018 at 2:53 PM, Rik van Riel 
>> wrote:
>>> Introduce a variant of on_each_cpu_cond that iterates only over the
>>> CPUs in a cpumask, in order to avoid making callbacks for every
>>> single
>>> CPU in the system when we only need to test a subset.
>> 
>> Nice.
>> 
>> Although, if you want to be really fancy, you could optimize this (or
>> add a variant) that does the callback on the local CPU in parallel
>> with the remote ones.  That would give a small boost to TLB flushes.
> 
> The test_func callbacks are not run remotely, but on
> the local CPU, before deciding who to send callbacks
> to.
> 
> The actual IPIs are sent in parallel, if the cpumask
> allocation succeeds (it always should in many kernel
> configurations, and almost always in the rest).

What I meant is that on_each_cpu_mask does:

smp_call_function_many(mask, func, info, wait);
if (cpumask_test_cpu(cpu, mask)) {
   unsigned long flags;
   local_irq_save(flags); func(info);
   local_irq_restore(flags);
}

So it IPIs all the remote CPUs in parallel, then waits, then does the local 
work.  In principle, the local flush could be done after triggering the IPIs 
but before they all finish.


> -- 
> All Rights Reversed.

Re: [PATCH V2] riscv: Convert uses of REG_FMT to %p

[Joe Perches]

On Sun, 2018-07-29 at 17:21 +0300, Andy Shevchenko wrote:
> On Sat, Jul 28, 2018 at 7:39 PM, Joe Perches  wrote:
> > Use %p pointer output instead of REG_FMT and cast the unsigned longs to
> > (void *) to avoid exposing kernel addresses.
> > 
> > Miscellanea:
> > 
> > o Convert pr_cont to printk(KERN_DEFAULT as these uses are
> >   new logging lines and not previous line continuations
> > o Remove the now unused REG_FMT defines
> > +   pr_alert("Unable to handle kernel %s at virtual address %p\n",
> > +(addr < PAGE_SIZE) ? "NULL pointer dereference" :
> > +"paging request", (void *)addr);
> 
> Hmm... This is what printf() should take care of.

Why is that so here?

Here this is just a description of the address.
It's not actually dereferencing the address.

> Petr, what is the status of your series to unify this kind of
> addresses to be printed?

[no subject]

[Wang Shengkun]

Hi 

My name is Wang Shengkun, i am a software programmer and i work with a lottery 
company over here in the United States. I can give you the winning numbers in 
our admin to play and win the lottery and we will share the proceeds 50% each. 
If you are interested , kindly write me back at : 2148972...@qq.com 

Thanks 

Wang Shengkun. 

Re: [PATCH 10/10] mm,sched: conditionally skip lazy TLB mm refcounting

[Rik van Riel]

On Sun, 2018-07-29 at 08:29 -0700, Andy Lutomirski wrote:
> > On Jul 29, 2018, at 5:11 AM, Rik van Riel  wrote:
> > 
> > > On Sat, 2018-07-28 at 21:21 -0700, Andy Lutomirski wrote:
> > > On Sat, Jul 28, 2018 at 2:53 PM, Rik van Riel 
> > > wrote:
> > > > Conditionally skip lazy TLB mm refcounting. When an
> > > > architecture
> > > > has
> > > > CONFIG_ARCH_NO_ACTIVE_MM_REFCOUNTING enabled, an mm that is
> > > > used in
> > > > lazy TLB mode anywhere will get shot down from exit_mmap, and
> > > > there
> > > > in no need to incur the cache line bouncing overhead of
> > > > refcounting
> > > > a lazy TLB mm.
> > > 
> > > Unless I've misunderstood something, this patch results in idle
> > > tasks
> > > whose active_mm has been freed still having active_mm pointing at
> > > freed memory. 
> > 
> > Patch 9/10 is supposed to ensure that the lazy TLB CPUs get
> > switched to init_mm before an mm is freed. No CPU should ever
> > have its active_mm pointing at a freed mm.
> > 
> > Your message made me re-read the code, and now I realize that
> > leave_mm does not actually do that.
> > 
> > Looking at the other callers of leave_mm, I might not be the
> > only one surprised by that; xen_drop_mm_ref comes to mind.
> > 
> > I guess I should some code to leave_mm to have it actually
> > clear active_mm and call the conditional refcount drop helper
> > function.
> > 
> > Does that clear up the confusion?
> 
> Kind of. But what’s the point of keeping active_mm?  On architectures
> that opt in to the new mode, there won’t be any code that cares about
> it’s value.  What’s the benefit of keeping it around?  If you ifdef
> it out, then it can’t possibly point to freed memory, and there’s
> nothing to worry about.

I would like to get to that point, but in a way that does
not leave the code too difficult to follow.

Getting rid of ->active_mm in context_switch() is straightforward,
but I am not sure at all what to do about idle_task_exit() for
example.

All the subtleties I ran into just with this phase of the
code suggests (to me at least) that we should probably do
this one step at a time.

I agree on the same end goal, though :)

-- 
All Rights Reversed.

signature.asc
Description: This is a digitally signed message part

Re: [PATCH] Staging: octeon: Fixing coding style for minor notices.

[Georgios Tsotsos]

Please ignore this patch i will send series which will fix more coding
style and other issues

On Sun, 29 Jul 2018 at 17:44, Georgios Tsotsos  wrote:

> Fixing coding style for a few lines that were reported to check from
> checkpatch.pl in minor cases for alignment and ending with parenthesis.
>
> Signed-off-by: Georgios Tsotsos 
> ---
>  drivers/staging/octeon/ethernet.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet.c
> b/drivers/staging/octeon/ethernet.c
> index 9b15c9ed844b..1e258deecacc 100644
> --- a/drivers/staging/octeon/ethernet.c
> +++ b/drivers/staging/octeon/ethernet.c
> @@ -141,8 +141,8 @@ static void cvm_oct_periodic_worker(struct work_struct
> *work)
> if (priv->poll)
> priv->poll(cvm_oct_device[priv->port]);
>
> -   cvm_oct_device[priv->port]->netdev_ops->ndo_get_stats(
> -
>  cvm_oct_device[priv->port]);
> +   cvm_oct_device[priv->port]->netdev_ops
> +
>  ->ndo_get_stats(cvm_oct_device[priv->port]);
>
> if (!atomic_read(&cvm_oct_poll_queue_stopping))
> schedule_delayed_work(&priv->port_periodic_work, HZ);
> @@ -621,8 +621,8 @@ static const struct net_device_ops
> cvm_oct_pow_netdev_ops = {
>  #endif
>  };
>
> -static struct device_node *cvm_oct_of_get_child(
> -   const struct device_node *parent, int
> reg_val)
> +static struct device_node *cvm_oct_of_get_child(const struct device_node
> *parent
> +   , int reg_val)
>  {
> struct device_node *node = NULL;
> int size;
> @@ -818,7 +818,7 @@ static int cvm_oct_probe(struct platform_device *pdev)
> priv = netdev_priv(dev);
> priv->netdev = dev;
> priv->of_node = cvm_oct_node_for_port(pip,
> interface,
> -
>  port_index);
> + port_index);
>
> INIT_DELAYED_WORK(&priv->port_periodic_work,
>   cvm_oct_periodic_worker);
> --
> 2.16.4
>
>

-- 
Best regards!
*Georgios Tsotsos *
*Greece-Evia-Chalkida*
*tsot...@linux.com *
*skype:* *tsotsos*

Georgios Tsotsos
*Greece - Evia - Chalkida*
tsotsos[at]linux.com
skype: tsotsos

Re: [PATCH] perf c2c report: Fix crash for empty browser

[Jiri Olsa]

On Fri, Jul 27, 2018 at 11:13:40AM -0300, Arnaldo Carvalho de Melo wrote:
> Adding Joe to the CC list.
> 
> Em Fri, Jul 27, 2018 at 09:06:23AM +0200, Jiri Olsa escreveu:
> > On Thu, Jul 26, 2018 at 11:31:34PM +, ro...@autistici.org wrote:
> > > On 2018-07-26 19:30, Arnaldo Carvalho de Melo wrote:
> > > > Em Tue, Jul 24, 2018 at 08:20:08AM +0200, Jiri Olsa escreveu:
> > > > > Do not try to display entry details if there's
> > > > > not any. Currently this ends up in crash:
> > > > >   $ perf c2c report
> > > > >   perf: Segmentation fault
> 
> > > > How to replicate this?
> 
> > > > I tried:
> 
> > > > $ perf record sleep 1
> > > > $ perf c2c report
> 
> > > > But it didn't segfault
> 
> > > Similarly I have tried :
> > > $ perf record sleep 1
> > > $ perf c2c report
> > > Then Press `d` to show the cache-line contents.
> 
> > yep, sry I forgot to mention you need to press the 'd' to show details
>  
> > > This replies the segfault on my machine (4.17.8-1).
> > > The patch mentioned above should solve it, even tough I am not sure as I
> > > haven't been able to recompile the kernel.
>  
> > no need to recompile kernel
>  
> > > The segfault by itself seems to be due to the report logic, as it did not
> > > expect to report on an empty browser.
> > > What has stepped me back is that application which I have been testing 
> > > with
> > > rely on multiple threads instantiated through pthread, which should be
> > > counted in user-level threads right? But they still seem to return an 
> > > empty
> > > browser.
>  
> > right, c2c scans read/write accesses and tries to find false sharing
> > cases maybe there was nothing to be found
>  
> > > When instead c2c is runned system-wide, with an application running on
> > > multiple threads like firefox or julia, cache hits are measured and also
> > > they are traced back in the source code.
>  
> > I got a cache line (attached) for 'perf bench sched messaging'
> > NOT being traced system wide and just for user (you'll get plenty
> > of detected cachelines in kernel space):
> 
> With that info in mind, we get:
> 
> [root@seventh ~]# perf record sleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.001 MB perf.data (6 samples) ]
> [root@seventh ~]# 
> [root@seventh ~]# 
> [root@seventh ~]# perf c2c report # and press 'd'
> perf: Segmentation fault
>  backtrace 
> perf[0x5b1d2a]
> /lib64/libc.so.6(+0x346df)[0x7fcb566e36df]
> perf[0x46fcae]
> perf[0x4a9f1e]
> perf[0x4aa220]
> perf(main+0x301)[0x42c561]
> /lib64/libc.so.6(__libc_start_main+0xe9)[0x7fcb566cff29]
> perf(_start+0x29)[0x42c999]
> [root@seventh ~]#
> 
> With your patches the segfault is gone, but I'd do a follow up patch to
> show some message telling the user why 'd' showed nothing and
> instructing him about what is missing, i.e. is this done on a perf.data
> file that has no events of interest? Suggest using 'perf c2c record' or
> 'perf record -e events,of,interest,to,perf,c2c', was this done on some
> workload where no false sharing was detected? Say so, etc.

ok, will try to come up with something

> 
> I applied your patch with a more detailed commit log to state how this
> can reproduced, etc, as usual:
> 
> https://git.kernel.org/acme/c/983eb6aa7098

I checked, looks good, thanks

jirka

[PATCH 3/8] staging: rtl8188eu: remove unused rtw_handle_tkip_mic_err()

[Michael Straube]

The function rtw_handle_tkip_mic_err() is never used, so remove it.
Discovered by cppcheck.

Signed-off-by: Michael Straube 
---
 .../staging/rtl8188eu/include/recv_osdep.h|  2 --
 drivers/staging/rtl8188eu/os_dep/recv_linux.c | 36 ---
 2 files changed, 38 deletions(-)

diff --git a/drivers/staging/rtl8188eu/include/recv_osdep.h 
b/drivers/staging/rtl8188eu/include/recv_osdep.h
index d2341521cc8e..2c05964b757d 100644
--- a/drivers/staging/rtl8188eu/include/recv_osdep.h
+++ b/drivers/staging/rtl8188eu/include/recv_osdep.h
@@ -19,8 +19,6 @@ s32  rtw_recv_entry(struct recv_frame *precv_frame);
 int rtw_recv_indicatepkt(struct adapter *adapter,
 struct recv_frame *recv_frame);
 
-void rtw_handle_tkip_mic_err(struct adapter *padapter, u8 bgroup);
-
 int rtw_os_recvbuf_resource_alloc(struct adapter *adapt, struct recv_buf *buf);
 
 void rtw_init_recv_timer(struct recv_reorder_ctrl *preorder_ctrl);
diff --git a/drivers/staging/rtl8188eu/os_dep/recv_linux.c 
b/drivers/staging/rtl8188eu/os_dep/recv_linux.c
index deadf26ea2aa..148ee94bbf3d 100644
--- a/drivers/staging/rtl8188eu/os_dep/recv_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/recv_linux.c
@@ -25,42 +25,6 @@ int rtw_os_recvbuf_resource_alloc(struct adapter *padapter,
return _SUCCESS;
 }
 
-void rtw_handle_tkip_mic_err(struct adapter *padapter, u8 bgroup)
-{
-   union iwreq_data wrqu;
-   struct iw_michaelmicfailure ev;
-   struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
-   struct security_priv *psecuritypriv = &padapter->securitypriv;
-   u32 cur_time = 0;
-
-   if (psecuritypriv->last_mic_err_time == 0) {
-   psecuritypriv->last_mic_err_time = jiffies;
-   } else {
-   cur_time = jiffies;
-
-   if (cur_time - psecuritypriv->last_mic_err_time < 60*HZ) {
-   psecuritypriv->btkip_countermeasure = true;
-   psecuritypriv->last_mic_err_time = 0;
-   psecuritypriv->btkip_countermeasure_time = cur_time;
-   } else {
-   psecuritypriv->last_mic_err_time = jiffies;
-   }
-   }
-
-   memset(&ev, 0x00, sizeof(ev));
-   if (bgroup)
-   ev.flags |= IW_MICFAILURE_GROUP;
-   else
-   ev.flags |= IW_MICFAILURE_PAIRWISE;
-
-   ev.src_addr.sa_family = ARPHRD_ETHER;
-   memcpy(ev.src_addr.sa_data, &pmlmepriv->assoc_bssid[0], ETH_ALEN);
-   memset(&wrqu, 0x00, sizeof(wrqu));
-   wrqu.data.length = sizeof(ev);
-   wireless_send_event(padapter->pnetdev, IWEVMICHAELMICFAILURE,
-   &wrqu, (char *)&ev);
-}
-
 int rtw_recv_indicatepkt(struct adapter *padapter,
 struct recv_frame *precv_frame)
 {
-- 
2.18.0

[PATCH 5/8] staging: rtl8188eu: replace tabs with spaces

[Michael Straube]

Replace tabs with spaces in function definition and variable
declarations.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/hal_intf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/hal_intf.c 
b/drivers/staging/rtl8188eu/hal/hal_intf.c
index 0baf4e616b6a..c43e7b438058 100644
--- a/drivers/staging/rtl8188eu/hal/hal_intf.c
+++ b/drivers/staging/rtl8188eu/hal/hal_intf.c
@@ -8,9 +8,9 @@
 #define _HAL_INTF_C_
 #include 
 
-uintrtw_hal_init(struct adapter *adapt)
+uint rtw_hal_init(struct adapter *adapt)
 {
-   uintstatus = _SUCCESS;
+   uint status = _SUCCESS;
 
adapt->hw_init_completed = false;
 
@@ -34,7 +34,7 @@ uint   rtw_hal_init(struct adapter *adapt)
 
 uint rtw_hal_deinit(struct adapter *adapt)
 {
-   uintstatus = _SUCCESS;
+   uint status = _SUCCESS;
 
status = rtl8188eu_hal_deinit(adapt);
 
-- 
2.18.0

[PATCH 2/8] staging: rtl8188eu: remove unused should_forbid_n_rate()

[Michael Straube]

The function should_forbid_n_rate() is never used, so remove it.
Discovered by cppcheck.

Signed-off-by: Michael Straube 
---
 .../staging/rtl8188eu/core/rtw_wlan_util.c| 35 ---
 .../staging/rtl8188eu/include/rtw_mlme_ext.h  |  1 -
 2 files changed, 36 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c 
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 0fe35e141926..b9406583e501 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -1094,41 +1094,6 @@ unsigned int is_ap_in_tkip(struct adapter *padapter)
}
 }
 
-unsigned int should_forbid_n_rate(struct adapter *padapter)
-{
-   u32 i;
-   struct ndis_802_11_var_ie *pIE;
-   struct mlme_priv*pmlmepriv = &padapter->mlmepriv;
-   struct wlan_bssid_ex  *cur_network = &pmlmepriv->cur_network.network;
-
-   if (rtw_get_capability((struct wlan_bssid_ex *)cur_network) & 
WLAN_CAPABILITY_PRIVACY) {
-   for (i = sizeof(struct ndis_802_11_fixed_ie); i < 
cur_network->ie_length;) {
-   pIE = (struct ndis_802_11_var_ie *)(cur_network->ies + 
i);
-
-   switch (pIE->ElementID) {
-   case _VENDOR_SPECIFIC_IE_:
-   if (!memcmp(pIE->data, RTW_WPA_OUI, 4) &&
-   ((!memcmp((pIE->data + 12), 
WPA_CIPHER_SUITE_CCMP, 4)) ||
-   (!memcmp((pIE->data + 16), 
WPA_CIPHER_SUITE_CCMP, 4
-   return false;
-   break;
-   case _RSN_IE_2_:
-   if  ((!memcmp((pIE->data + 8), 
RSN_CIPHER_SUITE_CCMP, 4))  ||
-  (!memcmp((pIE->data + 12), 
RSN_CIPHER_SUITE_CCMP, 4)))
-   return false;
-   default:
-   break;
-   }
-
-   i += (pIE->Length + 2);
-   }
-
-   return true;
-   } else {
-   return false;
-   }
-}
-
 unsigned int is_ap_in_wep(struct adapter *padapter)
 {
u32 i;
diff --git a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h 
b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
index c072e1e25d61..ade68af15e04 100644
--- a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
@@ -534,7 +534,6 @@ unsigned char get_highest_rate_idx(u32 mask);
 int support_short_GI(struct adapter *padapter, struct ieee80211_ht_cap *caps);
 unsigned int is_ap_in_tkip(struct adapter *padapter);
 unsigned int is_ap_in_wep(struct adapter *padapter);
-unsigned int should_forbid_n_rate(struct adapter *padapter);
 
 void report_join_res(struct adapter *padapter, int res);
 void report_survey_event(struct adapter *padapter,
-- 
2.18.0

[PATCH 6/8] staging: rtl8188eu: fix comparsion to true

[Michael Straube]

Use if(x) instead of if(x == true).

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/hal_intf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/hal/hal_intf.c 
b/drivers/staging/rtl8188eu/hal/hal_intf.c
index c43e7b438058..b21ba01abc57 100644
--- a/drivers/staging/rtl8188eu/hal/hal_intf.c
+++ b/drivers/staging/rtl8188eu/hal/hal_intf.c
@@ -50,7 +50,7 @@ void rtw_hal_update_ra_mask(struct adapter *adapt, u32 
mac_id, u8 rssi_level)
 {
struct mlme_priv *pmlmepriv = &(adapt->mlmepriv);
 
-   if (check_fwstate(pmlmepriv, WIFI_AP_STATE) == true) {
+   if (check_fwstate(pmlmepriv, WIFI_AP_STATE)) {
 #ifdef CONFIG_88EU_AP_MODE
struct sta_info *psta = NULL;
struct sta_priv *pstapriv = &adapt->stapriv;
-- 
2.18.0

[PATCH 8/8] staging: rtl8188eu: use is_broadcast_ether_addr

[Michael Straube]

Use is_broadcast_ether_addr instead of checking each byte of the
address array for 0xff. Shortens the code and improves readability.

Signed-off-by: Michael Straube 
---
 .../staging/rtl8188eu/os_dep/ioctl_linux.c| 34 ++-
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c 
b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index 221fae22bab6..4a8124570e6e 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -361,9 +361,7 @@ static int wpa_set_encryption(struct net_device *dev, 
struct ieee_param *param,
goto exit;
}
 
-   if (param->sta_addr[0] == 0xff && param->sta_addr[1] == 0xff &&
-   param->sta_addr[2] == 0xff && param->sta_addr[3] == 0xff &&
-   param->sta_addr[4] == 0xff && param->sta_addr[5] == 0xff) {
+   if (is_broadcast_ether_addr(param->sta_addr)) {
if (param->u.crypt.idx >= WEP_KEYS) {
ret = -EINVAL;
goto exit;
@@ -2208,9 +2206,7 @@ static int rtw_set_encryption(struct net_device *dev, 
struct ieee_param *param,
ret =  -EINVAL;
goto exit;
}
-   if (param->sta_addr[0] == 0xff && param->sta_addr[1] == 0xff &&
-   param->sta_addr[2] == 0xff && param->sta_addr[3] == 0xff &&
-   param->sta_addr[4] == 0xff && param->sta_addr[5] == 0xff) {
+   if (is_broadcast_ether_addr(param->sta_addr)) {
if (param->u.crypt.idx >= WEP_KEYS) {
ret = -EINVAL;
goto exit;
@@ -2471,9 +2467,7 @@ static int rtw_add_sta(struct net_device *dev, struct 
ieee_param *param)
if (!check_fwstate(pmlmepriv, (_FW_LINKED|WIFI_AP_STATE)))
return -EINVAL;
 
-   if (param->sta_addr[0] == 0xff && param->sta_addr[1] == 0xff &&
-   param->sta_addr[2] == 0xff && param->sta_addr[3] == 0xff &&
-   param->sta_addr[4] == 0xff && param->sta_addr[5] == 0xff)
+   if (is_broadcast_ether_addr(param->sta_addr))
return -EINVAL;
 
psta = rtw_get_stainfo(pstapriv, param->sta_addr);
@@ -2528,9 +2522,7 @@ static int rtw_del_sta(struct net_device *dev, struct 
ieee_param *param)
if (check_fwstate(pmlmepriv, (_FW_LINKED|WIFI_AP_STATE)) != true)
return -EINVAL;
 
-   if (param->sta_addr[0] == 0xff && param->sta_addr[1] == 0xff &&
-   param->sta_addr[2] == 0xff && param->sta_addr[3] == 0xff &&
-   param->sta_addr[4] == 0xff && param->sta_addr[5] == 0xff)
+   if (is_broadcast_ether_addr(param->sta_addr))
return -EINVAL;
 
psta = rtw_get_stainfo(pstapriv, param->sta_addr);
@@ -2566,9 +2558,7 @@ static int rtw_ioctl_get_sta_data(struct net_device *dev, 
struct ieee_param *par
if (check_fwstate(pmlmepriv, (_FW_LINKED|WIFI_AP_STATE)) != true)
return -EINVAL;
 
-   if (param_ex->sta_addr[0] == 0xff && param_ex->sta_addr[1] == 0xff &&
-   param_ex->sta_addr[2] == 0xff && param_ex->sta_addr[3] == 0xff &&
-   param_ex->sta_addr[4] == 0xff && param_ex->sta_addr[5] == 0xff)
+   if (is_broadcast_ether_addr(param_ex->sta_addr))
return -EINVAL;
 
psta = rtw_get_stainfo(pstapriv, param_ex->sta_addr);
@@ -2622,9 +2612,7 @@ static int rtw_get_sta_wpaie(struct net_device *dev, 
struct ieee_param *param)
if (check_fwstate(pmlmepriv, (_FW_LINKED|WIFI_AP_STATE)) != true)
return -EINVAL;
 
-   if (param->sta_addr[0] == 0xff && param->sta_addr[1] == 0xff &&
-   param->sta_addr[2] == 0xff && param->sta_addr[3] == 0xff &&
-   param->sta_addr[4] == 0xff && param->sta_addr[5] == 0xff)
+   if (is_broadcast_ether_addr(param->sta_addr))
return -EINVAL;
 
psta = rtw_get_stainfo(pstapriv, param->sta_addr);
@@ -2779,10 +2767,9 @@ static int rtw_ioctl_acl_remove_sta(struct net_device 
*dev, struct ieee_param *p
if (check_fwstate(pmlmepriv, WIFI_AP_STATE) != true)
return -EINVAL;
 
-   if (param->sta_addr[0] == 0xff && param->sta_addr[1] == 0xff &&
-   param->sta_addr[2] == 0xff && param->sta_addr[3] == 0xff &&
-   param->sta_addr[4] == 0xff && param->sta_addr[5] == 0xff)
+   if (is_broadcast_ether_addr(param->sta_addr))
return -EINVAL;
+
return rtw_acl_remove_sta(padapter, param->sta_addr);
 }
 
@@ -2794,10 +2781,9 @@ static int rtw_ioctl_acl_add_sta(struct net_device *dev, 
struct ieee_param *para
if (check_fwstate(pmlmepriv, WIFI_AP_STATE) != true)
return -EINVAL;
 
-   if (param->sta_addr[0] == 0xff && param->sta_addr[1] == 0xff &&
-   param->sta_addr[2] == 0xff && param->sta_addr[3] == 0xff &&
-   param->sta_addr[4] == 0xff && param->sta_addr[5] == 0xff)
+   if (is_broadcast_ether_addr(param->sta_addr))
 

[PATCH 1/8] staging: rtl8188eu: remove unused dump_txrpt_ccx_88e()

[Michael Straube]

The function dump_txrpt_ccx_88e() is nerver used, so remove it.
Discovered by cppcheck.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/rtl8188e_xmit.c | 22 ---
 .../staging/rtl8188eu/include/rtl8188e_xmit.h |  1 -
 2 files changed, 23 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_xmit.c 
b/drivers/staging/rtl8188eu/hal/rtl8188e_xmit.c
index 6883746f3b8b..9b8a284544ac 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_xmit.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_xmit.c
@@ -10,28 +10,6 @@
 #include 
 #include 
 
-void dump_txrpt_ccx_88e(void *buf)
-{
-   struct txrpt_ccx_88e *txrpt_ccx = buf;
-
-   DBG_88E("%s:\n"
-   "tag1:%u, pkt_num:%u, txdma_underflow:%u, int_bt:%u, 
int_tri:%u, int_ccx:%u\n"
-   "mac_id:%u, pkt_ok:%u, bmc:%u\n"
-   "retry_cnt:%u, lifetime_over:%u, retry_over:%u\n"
-   "ccx_qtime:%u\n"
-   "final_data_rate:0x%02x\n"
-   "qsel:%u, sw:0x%03x\n",
-   __func__, txrpt_ccx->tag1, txrpt_ccx->pkt_num,
-   txrpt_ccx->txdma_underflow, txrpt_ccx->int_bt,
-   txrpt_ccx->int_tri, txrpt_ccx->int_ccx,
-   txrpt_ccx->mac_id, txrpt_ccx->pkt_ok, txrpt_ccx->bmc,
-   txrpt_ccx->retry_cnt, txrpt_ccx->lifetime_over,
-   txrpt_ccx->retry_over, txrpt_ccx_qtime_88e(txrpt_ccx),
-   txrpt_ccx->final_data_rate, txrpt_ccx->qsel,
-   txrpt_ccx_sw_88e(txrpt_ccx)
-   );
-}
-
 void handle_txrpt_ccx_88e(struct adapter *adapter, u8 *buf)
 {
struct txrpt_ccx_88e *txrpt_ccx = (struct txrpt_ccx_88e *)buf;
diff --git a/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h 
b/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
index 17e7b3016674..20d35480dab8 100644
--- a/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
+++ b/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
@@ -152,7 +152,6 @@ void rtl8188eu_xmit_tasklet(void *priv);
 s32 rtl8188eu_xmitframe_complete(struct adapter *padapter,
 struct xmit_priv *pxmitpriv);
 
-void dump_txrpt_ccx_88e(void *buf);
 void handle_txrpt_ccx_88e(struct adapter *adapter, u8 *buf);
 
 void _dbg_dump_tx_info(struct adapter *padapter, int frame_tag,
-- 
2.18.0

[PATCH 7/8] staging: rtl8188eu: remove unnecessary parentheses

[Michael Straube]

Remove unnecessary parentheses, also clears checkpatch issues about
missing spaces around '-'.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/hal_intf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/hal_intf.c 
b/drivers/staging/rtl8188eu/hal/hal_intf.c
index b21ba01abc57..b8fecc952cfc 100644
--- a/drivers/staging/rtl8188eu/hal/hal_intf.c
+++ b/drivers/staging/rtl8188eu/hal/hal_intf.c
@@ -48,15 +48,15 @@ uint rtw_hal_deinit(struct adapter *adapt)
 
 void rtw_hal_update_ra_mask(struct adapter *adapt, u32 mac_id, u8 rssi_level)
 {
-   struct mlme_priv *pmlmepriv = &(adapt->mlmepriv);
+   struct mlme_priv *pmlmepriv = &adapt->mlmepriv;
 
if (check_fwstate(pmlmepriv, WIFI_AP_STATE)) {
 #ifdef CONFIG_88EU_AP_MODE
struct sta_info *psta = NULL;
struct sta_priv *pstapriv = &adapt->stapriv;
 
-   if ((mac_id-1) > 0)
-   psta = pstapriv->sta_aid[(mac_id-1) - 1];
+   if (mac_id - 1 > 0)
+   psta = pstapriv->sta_aid[mac_id - 2];
if (psta)
add_RATid(adapt, psta, 0);/* todo: based on rssi_level*/
 #endif
-- 
2.18.0

[PATCH 4/8] staging: rtl8188eu: remove redundant includes

[Michael Straube]

Both osdep_service.h and drv_types.h are included from hal_intf.h,
so remove the redundant includes from hal_intf.c.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/hal_intf.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/hal_intf.c 
b/drivers/staging/rtl8188eu/hal/hal_intf.c
index aaa1718ca603..0baf4e616b6a 100644
--- a/drivers/staging/rtl8188eu/hal/hal_intf.c
+++ b/drivers/staging/rtl8188eu/hal/hal_intf.c
@@ -6,8 +6,6 @@
  
**/
 
 #define _HAL_INTF_C_
-#include 
-#include 
 #include 
 
 uintrtw_hal_init(struct adapter *adapt)
-- 
2.18.0

Re: [PATCH 8/8] staging: rtl8188eu: use is_broadcast_ether_addr

[Joe Perches]

On Sun, 2018-07-29 at 19:08 +0200, Michael Straube wrote:
> Use is_broadcast_ether_addr instead of checking each byte of the
> address array for 0xff. Shortens the code and improves readability.

You should show in the commit log that sta_addr is __aligned(2)
as required by is_broadcast_ether_addr, otherwise you could be
introducing runtime alignment defects.

> diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c 
> b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
[]
> @@ -361,9 +361,7 @@ static int wpa_set_encryption(struct net_device *dev, 
> struct ieee_param *param,
>   goto exit;
>   }
>  
> - if (param->sta_addr[0] == 0xff && param->sta_addr[1] == 0xff &&
> - param->sta_addr[2] == 0xff && param->sta_addr[3] == 0xff &&
> - param->sta_addr[4] == 0xff && param->sta_addr[5] == 0xff) {
> + if (is_broadcast_ether_addr(param->sta_addr)) {
>   if (param->u.crypt.idx >= WEP_KEYS) {
>   ret = -EINVAL;
>   goto exit;

etc...

Re: [PATCH] Staging: octeon: Fixing coding style for minor notices.

[Joe Perches]

On Sun, 2018-07-29 at 17:43 +0300, Georgios Tsotsos wrote:
> Fixing coding style for a few lines that were reported to check from
> checkpatch.pl in minor cases for alignment and ending with parenthesis.
> 
> Signed-off-by: Georgios Tsotsos 
> ---
>  drivers/staging/octeon/ethernet.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/octeon/ethernet.c 
> b/drivers/staging/octeon/ethernet.c
> index 9b15c9ed844b..1e258deecacc 100644
> --- a/drivers/staging/octeon/ethernet.c
> +++ b/drivers/staging/octeon/ethernet.c
> @@ -141,8 +141,8 @@ static void cvm_oct_periodic_worker(struct work_struct 
> *work)
>   if (priv->poll)
>   priv->poll(cvm_oct_device[priv->port]);
>  
> - cvm_oct_device[priv->port]->netdev_ops->ndo_get_stats(
> - cvm_oct_device[priv->port]);
> + cvm_oct_device[priv->port]->netdev_ops
> + ->ndo_get_stats(cvm_oct_device[priv->port]);
>  
>   if (!atomic_read(&cvm_oct_poll_queue_stopping))
>   schedule_delayed_work(&priv->port_periodic_work, HZ);

Probably more sensible to use a temporary.  Something like:
---
 drivers/staging/octeon/ethernet.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/octeon/ethernet.c 
b/drivers/staging/octeon/ethernet.c
index 9b15c9ed844b..2febf1229587 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -137,12 +137,12 @@ static void cvm_oct_periodic_worker(struct work_struct 
*work)
struct octeon_ethernet *priv = container_of(work,
struct octeon_ethernet,
port_periodic_work.work);
+   struct net_device *ndev = cvm_oct_device[priv->port];
 
if (priv->poll)
-   priv->poll(cvm_oct_device[priv->port]);
+   priv->poll(ndev);
 
-   cvm_oct_device[priv->port]->netdev_ops->ndo_get_stats(
-   cvm_oct_device[priv->port]);
+   ndev->netdev_ops->ndo_get_stats(ndev);
 
if (!atomic_read(&cvm_oct_poll_queue_stopping))
schedule_delayed_work(&priv->port_periodic_work, HZ);

Re: [PATCH 03/10] smp,cpumask: introduce on_each_cpu_cond_mask

[Rik van Riel]

On Sun, 2018-07-29 at 08:36 -0700, Andy Lutomirski wrote:
> On Jul 29, 2018, at 5:00 AM, Rik van Riel  wrote:
> 
> > On Sat, 2018-07-28 at 19:57 -0700, Andy Lutomirski wrote:
> > > On Sat, Jul 28, 2018 at 2:53 PM, Rik van Riel 
> > > wrote:
> > > > Introduce a variant of on_each_cpu_cond that iterates only over
> > > > the
> > > > CPUs in a cpumask, in order to avoid making callbacks for every
> > > > single
> > > > CPU in the system when we only need to test a subset.
> > > Nice.
> > > Although, if you want to be really fancy, you could optimize this
> > > (or
> > > add a variant) that does the callback on the local CPU in
> > > parallel
> > > with the remote ones.  That would give a small boost to TLB
> > > flushes.
> > 
> > The test_func callbacks are not run remotely, but on
> > the local CPU, before deciding who to send callbacks
> > to.
> > 
> > The actual IPIs are sent in parallel, if the cpumask
> > allocation succeeds (it always should in many kernel
> > configurations, and almost always in the rest).
> > 
> 
> What I meant is that on_each_cpu_mask does:
> 
> smp_call_function_many(mask, func, info, wait);
> if (cpumask_test_cpu(cpu, mask)) {
>unsigned long flags;
>local_irq_save(flags); func(info);
>local_irq_restore(flags);
> }
> 
> So it IPIs all the remote CPUs in parallel, then waits, then does the
> local work.  In principle, the local flush could be done after
> triggering the IPIs but before they all finish.

Sure, moving the function call for the local CPU
into smp_call_function_many might be a nice optimization.

A quick grep suggests it touch stuff all over the tree,
so it could be a nice Outreachy intern project :)

-- 
All Rights Reversed.

signature.asc
Description: This is a digitally signed message part

Re: [PATCH 8/8] staging: rtl8188eu: use is_broadcast_ether_addr

[Michael Straube]

On 07/29/18 19:21, Joe Perches wrote:

On Sun, 2018-07-29 at 19:08 +0200, Michael Straube wrote:

Use is_broadcast_ether_addr instead of checking each byte of the
address array for 0xff. Shortens the code and improves readability.


You should show in the commit log that sta_addr is __aligned(2)
as required by is_broadcast_ether_addr, otherwise you could be
introducing runtime alignment defects.



Ok, sta_addr is used from following structs.

struct ieee_param {
u32 cmd;
u8 sta_addr[ETH_ALEN];
union {
...
...
}; u
};

struct ieee_param_ex {
u32 cmd;
u8 sta_addr[ETH_ALEN];
u8 data[0];
};

Well, looking at it now, I'm not sure about the alignment anymore
in the struct that contains the union. Is sta_addr in the first
struct __aligned(2)?

Should I include the snippets in the commit message, or is just
writing that sta_addr is __aligned(2) enough? (if it is in the
first case...)

Regards,
Michael

Re: [PATCH 03/10] smp,cpumask: introduce on_each_cpu_cond_mask

[Rik van Riel]

On Sun, 2018-07-29 at 08:36 -0700, Andy Lutomirski wrote:
> On Jul 29, 2018, at 5:00 AM, Rik van Riel  wrote:
> 
> > On Sat, 2018-07-28 at 19:57 -0700, Andy Lutomirski wrote:
> > > On Sat, Jul 28, 2018 at 2:53 PM, Rik van Riel 
> > > wrote:
> > > > Introduce a variant of on_each_cpu_cond that iterates only over
> > > > the
> > > > CPUs in a cpumask, in order to avoid making callbacks for every
> > > > single
> > > > CPU in the system when we only need to test a subset.
> > > Nice.
> > > Although, if you want to be really fancy, you could optimize this
> > > (or
> > > add a variant) that does the callback on the local CPU in
> > > parallel
> > > with the remote ones.  That would give a small boost to TLB
> > > flushes.
> > 
> > The test_func callbacks are not run remotely, but on
> > the local CPU, before deciding who to send callbacks
> > to.
> > 
> > The actual IPIs are sent in parallel, if the cpumask
> > allocation succeeds (it always should in many kernel
> > configurations, and almost always in the rest).
> > 
> 
> What I meant is that on_each_cpu_mask does:
> 
> smp_call_function_many(mask, func, info, wait);
> if (cpumask_test_cpu(cpu, mask)) {
>unsigned long flags;
>local_irq_save(flags); func(info);
>local_irq_restore(flags);
> }
> 
> So it IPIs all the remote CPUs in parallel, then waits, then does the
> local work.  In principle, the local flush could be done after
> triggering the IPIs but before they all finish.

Grepping around the code, I found a few examples where the
calling code appears to expect that smp_call_function_many
also calls "func" on the local CPU.

For example, kvm_emulate_wbinvd_noskip has this:

if (kvm_x86_ops->has_wbinvd_exit()) {
int cpu = get_cpu();

cpumask_set_cpu(cpu, vcpu->arch.wbinvd_dirty_mask);
smp_call_function_many(vcpu->arch.wbinvd_dirty_mask,
wbinvd_ipi, NULL, 1);
put_cpu();
cpumask_clear(vcpu->arch.wbinvd_dirty_mask);
} else
wbinvd();

This seems to result in systems with ->has_wbinvd_exit
only calling wbinvd_ipi on OTHER CPUs, and not on the
CPU where the guest exited with wbinvd?

This seems unintended.

I guess looking into on_each_cpu_mask might be a little
higher priority than waiting until the next Outreachy
season :)

-- 
All Rights Reversed.

signature.asc
Description: This is a digitally signed message part

Re: [PATCH 8/8] staging: rtl8188eu: use is_broadcast_ether_addr

[Joe Perches]

On Sun, 2018-07-29 at 19:42 +0200, Michael Straube wrote:
> On 07/29/18 19:21, Joe Perches wrote:
> > On Sun, 2018-07-29 at 19:08 +0200, Michael Straube wrote:
> > > Use is_broadcast_ether_addr instead of checking each byte of the
> > > address array for 0xff. Shortens the code and improves readability.
> > 
> > You should show in the commit log that sta_addr is __aligned(2)
> > as required by is_broadcast_ether_addr, otherwise you could be
> > introducing runtime alignment defects.
> > 
> 
> Ok, sta_addr is used from following structs.
> 
> struct ieee_param {
>  u32 cmd;
>  u8 sta_addr[ETH_ALEN];
>  union {
>  ...
>  ...
>  }; u
> };
> 
> struct ieee_param_ex {
>   u32 cmd;
>   u8 sta_addr[ETH_ALEN];
>   u8 data[0];
> };
> 
> Well, looking at it now, I'm not sure about the alignment anymore
> in the struct that contains the union. Is sta_addr in the first
> struct __aligned(2)?
> 
> Should I include the snippets in the commit message, or is just
> writing that sta_addr is __aligned(2) enough? (if it is in the
> first case...)

It's enough to just state that the uses are properly aligned
as long as you looked and understand that it's required.

Re: [PATCH V5 3/3] PCI: Mask and unmask hotplug interrupts during reset

[Lukas Wunner]

On Fri, Jul 20, 2018 at 07:58:20PM -0700, Sinan Kaya wrote:
> My patch solves the problem if AER interrupt happens before the hotplug
> interrupt. We are masking the data link layer active interrupt. So,
> AER/DPC can perform their link operations without hotplug driver race.
> 
> We need to figure out how to gracefully return inside hotplug driver
> if link down happened and there is an error pending.
> 
> My first question is why hotplug driver is reacting to the link event
> if there was not an actual device insertion/removal.
> 
> Would it help to keep track of presence changed interrupts since last
> link event?
> 
> IF counter is 0 and device is present, hotplug driver bails out
> silently as an example.

Counting PDC events doesn't work reliably if multiple such events
occur in very short succession as the interrupt handler may not
run quickly enough.  See this commit message which shows unbalanced
Link Up / Link Down events:
https://patchwork.ozlabs.org/patch/867418/

And on Thunderbolt, interrupts can be signaled even though the port
and its parents are in D3hot (sic!).  A Thunderbolt daisy chain can
consist of up to 6 devices, each comprising a PCI switch, so there's
a cascade of over a dozen Upstream / Downstream ports between the
Root port and the hotplug port at the end of the daisy chain.
God knows how many events have occurred by the time all the parents
are resumed to D0 and the Slot Status register of the hotplug port
is read/written.  That was really the motivation for the event
handling rework.

Thanks,

Lukas

Re: [PATCH] pwm: pwm-omap-dmtimer: fix probing problems by returning EPROBE_DEFER

[Ladislav Michl]

On Sun, Jul 29, 2018 at 08:32:41AM +0200, H. Nikolaus Schaller wrote:
> Hi,
> 
> > Am 28.07.2018 um 22:35 schrieb Ladislav Michl :
> > 
> > Hi Andreas,
> > 
> > On Sat, Jul 28, 2018 at 06:59:14PM +0200, Andreas Kemnade wrote:
> >> I got this in the kernel log:
> >> [0.756042] omap-dmtimer-pwm dmtimer-pwm: dmtimer pdata structure NULL
> >> [0.756134] omap-dmtimer-pwm: probe of dmtimer-pwm failed with error -22
> >> 
> >> the probe function has to wait until omap_dm_timer_probe() in
> >> clocksource/timer-ti-dm.c has initialized pdata, so defer probing
> > 
> > There already is a patch by David Rivshin addressing the same issue...
> 
> Here it is:
> 
> https://patchwork.ozlabs.org/patch/943148/
> 
> but hasn't arrived in linux-next.

That's because there'll be v3.

> But it is questionable if a driver should dev_info() about doing deferred 
> probing.
> IMHO, it should just do it which is how Andreas' patch works.

See here: https://patchwork.ozlabs.org/patch/949659/

> >> Fixes: b7290cf6ff78 (pwm: pwm-omap-dmtimer: Adapt driver to utilize 
> >> dmtimer pdata ops)
> >> Signed-off-by: Andreas Kemnade 
> >> ---
> >> drivers/pwm/pwm-omap-dmtimer.c | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/pwm/pwm-omap-dmtimer.c 
> >> b/drivers/pwm/pwm-omap-dmtimer.c
> >> index 665da3c8fbce..808835179c2e 100644
> >> --- a/drivers/pwm/pwm-omap-dmtimer.c
> >> +++ b/drivers/pwm/pwm-omap-dmtimer.c
> >> @@ -264,8 +264,7 @@ static int pwm_omap_dmtimer_probe(struct 
> >> platform_device *pdev)
> >> 
> >>timer_pdata = dev_get_platdata(&timer_pdev->dev);
> >>if (!timer_pdata) {
> >> -  dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
> >> -  ret = -EINVAL;
> >> +  ret = -EPROBE_DEFER;
> >>goto put;
> >>}
> >> 
> >> -- 
> >> 2.11.0
> >> 
> 
> BR,
> Nikolaus
> 

Re: [TRIVIAL RFC PATCH] Kconfigs - reduce use of "depends on EXPERT"

[Joe Perches]

(removing a bunch of cc's)

On Sun, 2018-07-29 at 13:42 +0200, Michael Büsch wrote:
> On Sat, 28 Jul 2018 15:13:00 -0700
> Joe Perches  wrote:
> 
> >  config SSB_SILENT
> > -   bool "No SSB kernel messages"
> > -   depends on SSB && EXPERT
> > +   bool "No SSB kernel messages" if EXPERT
> > +   depends on SSB
> > help
> >   This option turns off all Sonics Silicon Backplane printks.
> >   Note that you won't be able to identify problems, once
> 
> 
> What about removing this option entirely?
> We would just have to remove it from Kconfig and from its only use in
> drivers/ssb/ssb_private.h
> I don't think anybody uses (or should use) this option anyway.
> That would reduce the EXPERT dependencies by one, which is a good
> thing. :)

I'm fine with that, but it was originally your code from
the first ssb commit in 2007:

This might only be desired for production kernels on
embedded devices to reduce the kernel size.

Presumably for ddwrt and such.

Removal could simplify the ssb_ printk logic a bit too.

Perhaps something like the below,
but your code, your decisions...:

---
 drivers/ssb/Kconfig | 14 +-
 drivers/ssb/bridge_pcmcia_80211.c   |  2 +-
 drivers/ssb/driver_chipcommon.c |  2 +-
 drivers/ssb/driver_chipcommon_pmu.c | 41 ++---
 drivers/ssb/driver_mipscore.c   | 26 +--
 drivers/ssb/driver_pcicore.c| 14 +-
 drivers/ssb/embedded.c  |  4 +--
 drivers/ssb/main.c  | 49 +-
 drivers/ssb/pci.c   | 52 ++---
 drivers/ssb/pcmcia.c| 44 ++-
 drivers/ssb/scan.c  | 30 ++---
 drivers/ssb/sprom.c |  4 +--
 drivers/ssb/ssb_private.h   | 25 --
 13 files changed, 130 insertions(+), 177 deletions(-)

diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index c574dd210500..6c438c819eb9 100644
--- a/drivers/ssb/Kconfig
+++ b/drivers/ssb/Kconfig
@@ -89,21 +89,9 @@ config SSB_HOST_SOC
 
  If unsure, say N
 
-config SSB_SILENT
-   bool "No SSB kernel messages"
-   depends on SSB && EXPERT
-   help
- This option turns off all Sonics Silicon Backplane printks.
- Note that you won't be able to identify problems, once
- messages are turned off.
- This might only be desired for production kernels on
- embedded devices to reduce the kernel size.
-
- Say N
-
 config SSB_DEBUG
bool "SSB debugging"
-   depends on SSB && !SSB_SILENT
+   depends on SSB
help
  This turns on additional runtime checks and debugging
  messages. Turn this on for SSB troubleshooting.
diff --git a/drivers/ssb/bridge_pcmcia_80211.c 
b/drivers/ssb/bridge_pcmcia_80211.c
index d70568ea02d5..fb623bb01d3b 100644
--- a/drivers/ssb/bridge_pcmcia_80211.c
+++ b/drivers/ssb/bridge_pcmcia_80211.c
@@ -70,7 +70,7 @@ static int ssb_host_pcmcia_probe(struct pcmcia_device *dev)
 err_kfree_ssb:
kfree(ssb);
 out_error:
-   ssb_err("Initialization failed (%d, %d)\n", res, err);
+   pr_err("Initialization failed (%d, %d)\n", res, err);
return err;
 }
 
diff --git a/drivers/ssb/driver_chipcommon.c b/drivers/ssb/driver_chipcommon.c
index 7cb7d2c8fd86..6df0929b26eb 100644
--- a/drivers/ssb/driver_chipcommon.c
+++ b/drivers/ssb/driver_chipcommon.c
@@ -354,7 +354,7 @@ void ssb_chipcommon_init(struct ssb_chipcommon *cc)
 
if (cc->dev->id.revision >= 11)
cc->status = chipco_read32(cc, SSB_CHIPCO_CHIPSTAT);
-   ssb_dbg("chipcommon status is 0x%x\n", cc->status);
+   pr_debug("chipcommon status is 0x%x\n", cc->status);
 
if (cc->dev->id.revision >= 20) {
chipco_write32(cc, SSB_CHIPCO_GPIOPULLUP, 0);
diff --git a/drivers/ssb/driver_chipcommon_pmu.c 
b/drivers/ssb/driver_chipcommon_pmu.c
index c5352ea4821e..25e3b15070eb 100644
--- a/drivers/ssb/driver_chipcommon_pmu.c
+++ b/drivers/ssb/driver_chipcommon_pmu.c
@@ -110,8 +110,8 @@ static void ssb_pmu0_pllinit_r0(struct ssb_chipcommon *cc,
return;
}
 
-   ssb_info("Programming PLL to %u.%03u MHz\n",
-crystalfreq / 1000, crystalfreq % 1000);
+   pr_info("Programming PLL to %u.%03u MHz\n",
+   crystalfreq / 1000, crystalfreq % 1000);
 
/* First turn the PLL off. */
switch (bus->chip_id) {
@@ -138,7 +138,7 @@ static void ssb_pmu0_pllinit_r0(struct ssb_chipcommon *cc,
}
tmp = chipco_read32(cc, SSB_CHIPCO_CLKCTLST);
if (tmp & SSB_CHIPCO_CLKCTLST_HAVEHT)
-   ssb_emerg("Failed to turn the PLL off!\n");
+   pr_emerg("Failed to turn the PLL off!\n");
 
/* Set PDIV in PLL control 0. */
pllctl = ssb_chipco_pll_read(cc, SSB_PMU0_PLLCTL0);
@@ -249,8 +249,8 @@ static void ssb_pmu1_pllinit_r0(struct ss

Re: [PATCH] pwm: pwm-omap-dmtimer: fix probing problems by returning EPROBE_DEFER

[H. Nikolaus Schaller]

Hi,

> Am 29.07.2018 um 20:08 schrieb Ladislav Michl :
> 
> On Sun, Jul 29, 2018 at 08:32:41AM +0200, H. Nikolaus Schaller wrote:
>> Hi,
>> 
>>> Am 28.07.2018 um 22:35 schrieb Ladislav Michl :
>>> 
>>> Hi Andreas,
>>> 
>>> On Sat, Jul 28, 2018 at 06:59:14PM +0200, Andreas Kemnade wrote:
 I got this in the kernel log:
 [0.756042] omap-dmtimer-pwm dmtimer-pwm: dmtimer pdata structure NULL
 [0.756134] omap-dmtimer-pwm: probe of dmtimer-pwm failed with error -22
 
 the probe function has to wait until omap_dm_timer_probe() in
 clocksource/timer-ti-dm.c has initialized pdata, so defer probing
>>> 
>>> There already is a patch by David Rivshin addressing the same issue...
>> 
>> Here it is:
>> 
>> https://patchwork.ozlabs.org/patch/943148/
>> 
>> but hasn't arrived in linux-next.
> 
> That's because there'll be v3.
> 
>> But it is questionable if a driver should dev_info() about doing deferred 
>> probing.
>> IMHO, it should just do it which is how Andreas' patch works.
> 
> See here: https://patchwork.ozlabs.org/patch/949659/

Ah, I see (neither Andreas nor me did follow the original discussions
and therefore came up independently with the same thoughts).

So we will wait for the v3.

BR and thanks,
Nikolaus

Re: [PATCH 8/8] staging: rtl8188eu: use is_broadcast_ether_addr

[Michael Straube]

On 07/29/18 19:59, Joe Perches wrote:

On Sun, 2018-07-29 at 19:42 +0200, Michael Straube wrote:

On 07/29/18 19:21, Joe Perches wrote:

On Sun, 2018-07-29 at 19:08 +0200, Michael Straube wrote:

Use is_broadcast_ether_addr instead of checking each byte of the
address array for 0xff. Shortens the code and improves readability.


You should show in the commit log that sta_addr is __aligned(2)
as required by is_broadcast_ether_addr, otherwise you could be
introducing runtime alignment defects.



Ok, sta_addr is used from following structs.

struct ieee_param {
  u32 cmd;
  u8 sta_addr[ETH_ALEN];
  union {
  ...
  ...
  }; u
};

struct ieee_param_ex {
u32 cmd;
u8 sta_addr[ETH_ALEN];
u8 data[0];
};

Well, looking at it now, I'm not sure about the alignment anymore
in the struct that contains the union. Is sta_addr in the first
struct __aligned(2)?

Should I include the snippets in the commit message, or is just
writing that sta_addr is __aligned(2) enough? (if it is in the
first case...)


It's enough to just state that the uses are properly aligned
as long as you looked and understand that it's required.



Ok, thank you.

I looked at it and understand that it's required.
But, as mentioned, at second look I'm not sure about the union.

I guess I need to read a little more about the alignment of unions.
Any hints are welcomed. :)

Michael

[PATCH v2 0/5] Add Actions Semi S700 pinctrl support

[Saravanan Sekar]

This patchset adds pinctrl support for Actions Semi S700 SoC.
Pinmux functions are only accessible for pin groups while pinconf
parameters are available for both pin groups and individual pins.

pinctrl driver is verified using the Cubieboard7.

common functionalities from s900 is moved for all Actions Semi
SoC's usage.
Common pad control configurations are moved to SoC specific

dts entry for pinctrl depends on clock driver which is still under review:
https://patchwork.kernel.org/patch/10533959/

Thanks,
Saravanan
Parthiban

Changelog in v2
- GPIO Interrupt support is added
- Device tree entry for GPIO and Interrupt configured
- GPIO and pinctrl define reorder, commit message findings fixed

depends on owl GPIO Interrupt support
https://patchwork.kernel.org/patch/10483477/

Saravanan Sekar (5):
  pinctrl: actions: define constructor generic to Actions Semi SoC's
  pinctrl: actions: define pad control configurtion to SoC specific
  dt-bindings: pinctrl: Add bindings for Actions Semi S700 SoC
  pinctrl: actions: Add Actions Semi S700 pinctrl driver
  arm64: dts: actions: Add pinctrl node for Actions Semi S700

 .../bindings/pinctrl/actions,s700-pinctrl.txt |  170 ++
 arch/arm64/boot/dts/actions/s700.dtsi |   16 +
 drivers/pinctrl/actions/Kconfig   |6 +
 drivers/pinctrl/actions/Makefile  |1 +
 drivers/pinctrl/actions/pinctrl-owl.c |   71 +-
 drivers/pinctrl/actions/pinctrl-owl.h |  142 +-
 drivers/pinctrl/actions/pinctrl-s700.c| 1925 +
 drivers/pinctrl/actions/pinctrl-s900.c|  205 +-
 8 files changed, 2337 insertions(+), 199 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/actions,s700-pinctrl.txt
 create mode 100644 drivers/pinctrl/actions/pinctrl-s700.c

-- 
2.18.0

[PATCH v2 3/5] dt-bindings: pinctrl: Add bindings for Actions Semi S700 SoC

[Saravanan Sekar]

Add pinctrl and pio bindings for Actions Semi S700 SoC.

Signed-off-by: Parthiban Nallathambi 
Signed-off-by: Saravanan Sekar 
---
 .../bindings/pinctrl/actions,s700-pinctrl.txt | 170 ++
 1 file changed, 170 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/actions,s700-pinctrl.txt

diff --git a/Documentation/devicetree/bindings/pinctrl/actions,s700-pinctrl.txt 
b/Documentation/devicetree/bindings/pinctrl/actions,s700-pinctrl.txt
new file mode 100644
index ..ceed32e836b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/actions,s700-pinctrl.txt
@@ -0,0 +1,170 @@
+Actions Semi S700 Pin Controller
+
+This binding describes the pin controller found in the S700 SoC.
+
+Required Properties:
+
+- compatible:   Should be "actions,s700-pinctrl"
+- reg:  Should contain the register base address and size of
+   the pin controller.
+- clocks:   phandle of the clock feeding the pin controller
+- gpio-controller: Marks the device node as a GPIO controller.
+- gpio-ranges: Specifies the mapping between gpio controller and
+   pin-controller pins.
+- #gpio-cells: Should be two. The first cell is the gpio pin number
+   and the second cell is used for optional parameters.
+- interrupt-controller: Marks the device node as an interrupt controller.
+- #interrupt-cells: Specifies the number of cells needed to encode an
+   interrupt.  Shall be set to 2.  The first cell
+   defines the interrupt number, the second encodes
+   the trigger flags described in
+   bindings/interrupt-controller/interrupts.txt
+- interrupts: The interrupt outputs from the controller. There is one GPIO
+  interrupt per GPIO bank. The number of interrupts listed depends
+  on the number of GPIO banks on the SoC. The interrupts must be
+  ordered by bank, starting with bank 0.
+
+Please refer to pinctrl-bindings.txt in this directory for details of the
+common pinctrl bindings used by client devices, including the meaning of the
+phrase "pin configuration node".
+
+The pin configuration nodes act as a container for an arbitrary number of
+subnodes. Each of these subnodes represents some desired configuration for a
+pin, a group, or a list of pins or groups. This configuration can include the
+mux function to select on those group(s), and various pin configuration
+parameters, such as pull-up, drive strength, etc.
+
+PIN CONFIGURATION NODES:
+
+The name of each subnode is not important; all subnodes should be enumerated
+and processed purely based on their content.
+
+Each subnode only affects those parameters that are explicitly listed. In
+other words, a subnode that lists a mux function but no pin configuration
+parameters implies no information about any pin configuration parameters.
+Similarly, a pin subnode that describes a pullup parameter implies no
+information about e.g. the mux function.
+
+Pinmux functions are available only for the pin groups while pinconf
+parameters are available for both pin groups and individual pins.
+
+The following generic properties as defined in pinctrl-bindings.txt are valid
+to specify in a pin configuration subnode:
+
+Required Properties:
+
+- pins:An array of strings, each string containing the name of 
a pin.
+   These pins are used for selecting the pull control and schmitt
+   trigger parameters. The following are the list of pins
+   available:
+
+   eth_txd0, eth_txd1, eth_txd2, eth_txd3, eth_txen, eth_rxer,
+   eth_crs_dv, eth_rxd1, eth_rxd0, eth_rxd2, eth_rxd3, eth_ref_clk,
+   eth_mdc, eth_mdio, sirq0, sirq1, sirq2, i2s_d0, i2s_bclk0,
+   i2s_lrclk0, i2s_mclk0, i2s_d1, i2s_bclk1, i2s_lrclk1, i2s_mclk1,
+   pcm1_in, pcm1_clk, pcm1_sync, pcm1_out, ks_in0, ks_in1, ks_in2,
+   ks_in3, ks_out0, ks_out1, ks_out2, lvds_oep, lvds_oen, lvds_odp,
+   lvds_odn, lvds_ocp, lvds_ocn, lvds_obp, lvds_obn, lvds_oap,
+   lvds_oan, lvds_eep, lvds_een, lvds_edp, lvds_edn, lvds_ecp,
+   lvds_ecn, lvds_ebp, lvds_ebn, lvds_eap, lvds_ean, lcd0_d18,
+   lcd0_d2, dsi_dp3, dsi_dn3, dsi_dp1, dsi_dn1, dsi_cp, dsi_cn,
+   dsi_dp0, dsi_dn0, dsi_dp2, dsi_dn2, sd0_d0, sd0_d1, sd0_d2,
+   sd0_d3, sd1_d0, sd1_d1, sd1_d2, sd1_d3, sd0_cmd, sd0_clk,
+   sd1_cmd, sd1_clk, spi0_ss, spi0_miso, uart0_rx, uart0_tx,
+   uart2_rx, uart2_tx, uart2_rtsb, uart2_ctsb, uart3_rx, uart3_tx,
+   uart3_rtsb, uart3_ctsb, i2c0_sclk, i2c0_sdata, i2c1_sclk,
+   i2c1_sdata, i2c2_sdata, csi_dn0, csi_dp0, csi_dn1, csi_dp1,
+   csi_cn, csi_cp, csi_dn2, csi_dp2, csi_dn3, csi_dp3,
+   sensor0_pclk, sensor0_ckout, dnand_d0, dnand_d1, dnand_d2,
+   dnand_d3, dnand_d4, dnand_d5, dnand_d6, dnand_d7,

[PATCH v2 5/5] arm64: dts: actions: Add pinctrl node for Actions Semi S700

[Saravanan Sekar]

Add pinctrl nodes for Actions Semi S700 SoC

Signed-off-by: Parthiban Nallathambi 
Signed-off-by: Saravanan Sekar 
---
 arch/arm64/boot/dts/actions/s700.dtsi | 16 
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/actions/s700.dtsi 
b/arch/arm64/boot/dts/actions/s700.dtsi
index 66dd5309f0a2..8fbd7b1570a7 100644
--- a/arch/arm64/boot/dts/actions/s700.dtsi
+++ b/arch/arm64/boot/dts/actions/s700.dtsi
@@ -165,5 +165,21 @@
interrupts = ;
interrupt-names = "timer1";
};
+
+   pinctrl: pinctrl@e01b {
+   compatible = "actions,s700-pinctrl";
+   reg = <0x0 0xe01b 0x0 0x1000>;
+   clocks = <&cmu CLK_GPIO>;
+   gpio-controller;
+   gpio-ranges = <&pinctrl 0 0 136>;
+   #gpio-cells = <2>;
+   interrupt-controller;
+   #interrupt-cells = <2>;
+   interrupts = ,
+,
+,
+,
+;
+   };
};
 };
-- 
2.18.0

[PATCH v2 4/5] pinctrl: actions: Add Actions Semi S700 pinctrl driver

[Saravanan Sekar]

Add pinctrl driver for Actions Semi S700 SoC. The driver supports pinctrl,
pinmux and pinconf functionalities through a range of registers common to
both gpio driver and pinctrl driver.

Signed-off-by: Parthiban Nallathambi 
Signed-off-by: Saravanan Sekar 
---
 drivers/pinctrl/actions/Kconfig|6 +
 drivers/pinctrl/actions/Makefile   |1 +
 drivers/pinctrl/actions/pinctrl-owl.c  |7 +-
 drivers/pinctrl/actions/pinctrl-s700.c | 1925 
 4 files changed, 1936 insertions(+), 3 deletions(-)
 create mode 100644 drivers/pinctrl/actions/pinctrl-s700.c

diff --git a/drivers/pinctrl/actions/Kconfig b/drivers/pinctrl/actions/Kconfig
index 2397cb0f6011..c7ed1d481802 100644
--- a/drivers/pinctrl/actions/Kconfig
+++ b/drivers/pinctrl/actions/Kconfig
@@ -9,6 +9,12 @@ config PINCTRL_OWL
help
  Say Y here to enable Actions Semi OWL pinctrl driver
 
+config PINCTRL_S700
+   bool "Actions Semi S700 pinctrl driver"
+   depends on PINCTRL_OWL
+   help
+ Say Y here to enable Actions Semi S700 pinctrl driver
+
 config PINCTRL_S900
bool "Actions Semi S900 pinctrl driver"
depends on PINCTRL_OWL
diff --git a/drivers/pinctrl/actions/Makefile b/drivers/pinctrl/actions/Makefile
index bd232d28400f..86521ed837dd 100644
--- a/drivers/pinctrl/actions/Makefile
+++ b/drivers/pinctrl/actions/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_PINCTRL_OWL)  += pinctrl-owl.o
+obj-$(CONFIG_PINCTRL_S700) += pinctrl-s700.o
 obj-$(CONFIG_PINCTRL_S900) += pinctrl-s900.o
diff --git a/drivers/pinctrl/actions/pinctrl-owl.c 
b/drivers/pinctrl/actions/pinctrl-owl.c
index 004bdc66e158..fa8bffa46011 100644
--- a/drivers/pinctrl/actions/pinctrl-owl.c
+++ b/drivers/pinctrl/actions/pinctrl-owl.c
@@ -739,7 +739,7 @@ static void owl_gpio_irq_mask(struct irq_data *data)
val = readl_relaxed(gpio_base + port->intc_msk);
if (val == 0)
owl_gpio_update_reg(gpio_base + port->intc_ctl,
-   OWL_GPIO_CTLR_ENABLE, false);
+   OWL_GPIO_CTLR_ENABLE + 
port->shared_ctl_offset * 5, false);
 
raw_spin_unlock_irqrestore(&pctrl->lock, flags);
 }
@@ -763,7 +763,8 @@ static void owl_gpio_irq_unmask(struct irq_data *data)
 
/* enable port interrupt */
value = readl_relaxed(gpio_base + port->intc_ctl);
-   value |= BIT(OWL_GPIO_CTLR_ENABLE) | BIT(OWL_GPIO_CTLR_SAMPLE_CLK_24M);
+   value |= ((BIT(OWL_GPIO_CTLR_ENABLE) | 
BIT(OWL_GPIO_CTLR_SAMPLE_CLK_24M))
+   << port->shared_ctl_offset * 5);
writel_relaxed(value, gpio_base + port->intc_ctl);
 
/* enable GPIO interrupt */
@@ -801,7 +802,7 @@ static void owl_gpio_irq_ack(struct irq_data *data)
raw_spin_lock_irqsave(&pctrl->lock, flags);
 
owl_gpio_update_reg(gpio_base + port->intc_ctl,
-   OWL_GPIO_CTLR_PENDING, true);
+   OWL_GPIO_CTLR_PENDING + port->shared_ctl_offset 
* 5, true);
 
raw_spin_unlock_irqrestore(&pctrl->lock, flags);
 }
diff --git a/drivers/pinctrl/actions/pinctrl-s700.c 
b/drivers/pinctrl/actions/pinctrl-s700.c
new file mode 100644
index ..7e9d23ad0e17
--- /dev/null
+++ b/drivers/pinctrl/actions/pinctrl-s700.c
@@ -0,0 +1,1925 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * OWL S700 Pinctrl driver
+ *
+ * Copyright (c) 2014 Actions Semi Inc.
+ * Author: David Liu 
+ *
+ * Author: Pathiban Nallathambi 
+ * Author: Saravanan Sekar 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "pinctrl-owl.h"
+
+/* Pinctrl registers offset */
+#define MFCTL0 (0x0040)
+#define MFCTL1 (0x0044)
+#define MFCTL2 (0x0048)
+#define MFCTL3 (0x004C)
+#define PAD_PULLCTL0   (0x0060)
+#define PAD_PULLCTL1   (0x0064)
+#define PAD_PULLCTL2   (0x0068)
+#define PAD_ST0(0x006C)
+#define PAD_ST1(0x0070)
+#define PAD_CTL(0x0074)
+#define PAD_DRV0   (0x0080)
+#define PAD_DRV1   (0x0084)
+#define PAD_DRV2   (0x0088)
+
+/*
+ * Most pins affected by the pinmux can also be GPIOs. Define these first.
+ * These must match how the GPIO driver names/numbers its pins.
+ */
+#define _GPIOA(offset) (offset)
+#define _GPIOB(offset) (32 + (offset))
+#define _GPIOC(offset) (64 + (offset))
+#define _GPIOD(offset) (96 + (offset))
+#define _GPIOE(offset) (128 + (offset))
+
+/* All non-GPIO pins follow */
+#define NUM_GPIOS  (_GPIOE(7) + 1)
+#define _PIN(offset)   (NUM_GPIOS + (offset))
+
+/* Ethernet MAC */
+#define ETH_TXD0   _GPIOA(14)
+#define ETH_TXD1   _GPIOA(15)
+#define ETH_TXD2   _GPIOE(4)
+#define ETH_TXD3   _GPIOE(5)
+#define ETH_TXEN   _GPIOA(16)
+#define ETH_RXER   

[PATCH v2 1/5] pinctrl: actions: define constructor generic to Actions Semi SoC's

[Saravanan Sekar]

Move generic defines common to the Owl family out of S900 driver.

Signed-off-by: Parthiban Nallathambi 
Signed-off-by: Saravanan Sekar 
---
 drivers/pinctrl/actions/pinctrl-owl.h  | 136 ++--
 drivers/pinctrl/actions/pinctrl-s900.c | 139 ++---
 2 files changed, 136 insertions(+), 139 deletions(-)

diff --git a/drivers/pinctrl/actions/pinctrl-owl.h 
b/drivers/pinctrl/actions/pinctrl-owl.h
index a724d1d406d4..a7bcacad11d5 100644
--- a/drivers/pinctrl/actions/pinctrl-owl.h
+++ b/drivers/pinctrl/actions/pinctrl-owl.h
@@ -15,12 +15,135 @@
 #define OWL_PINCONF_SLEW_SLOW 0
 #define OWL_PINCONF_SLEW_FAST 1
 
-enum owl_pinconf_pull {
-   OWL_PINCONF_PULL_HIZ,
-   OWL_PINCONF_PULL_DOWN,
-   OWL_PINCONF_PULL_UP,
-   OWL_PINCONF_PULL_HOLD,
-};
+#define MUX_PG(group_name, reg, shift, width)  \
+   {   \
+   .name = #group_name,\
+   .pads = group_name##_pads,  \
+   .npads = ARRAY_SIZE(group_name##_pads), \
+   .funcs = group_name##_funcs,\
+   .nfuncs = ARRAY_SIZE(group_name##_funcs),   \
+   .mfpctl_reg  = MFCTL##reg,  \
+   .mfpctl_shift = shift,  \
+   .mfpctl_width = width,  \
+   .drv_reg = -1,  \
+   .drv_shift = -1,\
+   .drv_width = -1,\
+   .sr_reg = -1,   \
+   .sr_shift = -1, \
+   .sr_width = -1, \
+   }
+
+#define DRV_PG(group_name, reg, shift, width)  \
+   {   \
+   .name = #group_name,\
+   .pads = group_name##_pads,  \
+   .npads = ARRAY_SIZE(group_name##_pads), \
+   .mfpctl_reg  = -1,  \
+   .mfpctl_shift = -1, \
+   .mfpctl_width = -1, \
+   .drv_reg = PAD_DRV##reg,\
+   .drv_shift = shift, \
+   .drv_width = width, \
+   .sr_reg = -1,   \
+   .sr_shift = -1, \
+   .sr_width = -1, \
+   }
+
+#define SR_PG(group_name, reg, shift, width)   \
+   {   \
+   .name = #group_name,\
+   .pads = group_name##_pads,  \
+   .npads = ARRAY_SIZE(group_name##_pads), \
+   .mfpctl_reg  = -1,  \
+   .mfpctl_shift = -1, \
+   .mfpctl_width = -1, \
+   .drv_reg = -1,  \
+   .drv_shift = -1,\
+   .drv_width = -1,\
+   .sr_reg = PAD_SR##reg,  \
+   .sr_shift = shift,  \
+   .sr_width = width,  \
+   }
+
+#define FUNCTION(fname)\
+   {   \
+   .name = #fname, \
+   .groups = fname##_groups,   \
+   .ngroups = ARRAY_SIZE(fname##_groups),  \
+   }
+
+/* PAD PULL UP/DOWN CONFIGURES */
+#define PULLCTL_CONF(pull_reg, pull_sft, pull_wdt) \
+   {   \
+   .reg = PAD_PULLCTL##pull_reg,   \
+   .shift = pull_sft,  \
+   .width = pull_wdt,  \
+   }
+
+#define PAD_PULLCTL_CONF(pad_name, pull_reg, pull_sft, pull_wdt)   \
+   struct owl_pullctl pad_name##_pullctl_conf  \
+   = PULLCTL_CONF(pull_reg, pull_sft, pull_wdt)
+
+#define ST_CONF(st_reg, st_sft, st_wdt)   

[PATCH v2 2/5] pinctrl: actions: define pad control configurtion to SoC specific

[Saravanan Sekar]

pad control for s900 and s700 are differs in number of
pull control configuraions
s900 has 4 pull controls - high impedence, pull up, pull down, repeater
s700, s500 has 2 pull controls - pull up and pull down

so pad control configuration has to SoC specific, moved out from pinctrl
common to s700, s900 specific.

Signed-off-by: Parthiban Nallathambi 
Signed-off-by: Saravanan Sekar 
---
 drivers/pinctrl/actions/pinctrl-owl.c  | 64 -
 drivers/pinctrl/actions/pinctrl-owl.h  |  6 +++
 drivers/pinctrl/actions/pinctrl-s900.c | 66 +-
 3 files changed, 79 insertions(+), 57 deletions(-)

diff --git a/drivers/pinctrl/actions/pinctrl-owl.c 
b/drivers/pinctrl/actions/pinctrl-owl.c
index 4fa9cc377b3b..004bdc66e158 100644
--- a/drivers/pinctrl/actions/pinctrl-owl.c
+++ b/drivers/pinctrl/actions/pinctrl-owl.c
@@ -246,60 +246,6 @@ static int owl_pad_pinconf_reg(const struct owl_padinfo 
*info,
return 0;
 }
 
-static int owl_pad_pinconf_arg2val(const struct owl_padinfo *info,
-   unsigned int param,
-   u32 *arg)
-{
-   switch (param) {
-   case PIN_CONFIG_BIAS_BUS_HOLD:
-   *arg = OWL_PINCONF_PULL_HOLD;
-   break;
-   case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
-   *arg = OWL_PINCONF_PULL_HIZ;
-   break;
-   case PIN_CONFIG_BIAS_PULL_DOWN:
-   *arg = OWL_PINCONF_PULL_DOWN;
-   break;
-   case PIN_CONFIG_BIAS_PULL_UP:
-   *arg = OWL_PINCONF_PULL_UP;
-   break;
-   case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
-   *arg = (*arg >= 1 ? 1 : 0);
-   break;
-   default:
-   return -ENOTSUPP;
-   }
-
-   return 0;
-}
-
-static int owl_pad_pinconf_val2arg(const struct owl_padinfo *padinfo,
-   unsigned int param,
-   u32 *arg)
-{
-   switch (param) {
-   case PIN_CONFIG_BIAS_BUS_HOLD:
-   *arg = *arg == OWL_PINCONF_PULL_HOLD;
-   break;
-   case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
-   *arg = *arg == OWL_PINCONF_PULL_HIZ;
-   break;
-   case PIN_CONFIG_BIAS_PULL_DOWN:
-   *arg = *arg == OWL_PINCONF_PULL_DOWN;
-   break;
-   case PIN_CONFIG_BIAS_PULL_UP:
-   *arg = *arg == OWL_PINCONF_PULL_UP;
-   break;
-   case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
-   *arg = *arg == 1;
-   break;
-   default:
-   return -ENOTSUPP;
-   }
-
-   return 0;
-}
-
 static int owl_pin_config_get(struct pinctrl_dev *pctrldev,
unsigned int pin,
unsigned long *config)
@@ -318,7 +264,10 @@ static int owl_pin_config_get(struct pinctrl_dev *pctrldev,
 
arg = owl_read_field(pctrl, reg, bit, width);
 
-   ret = owl_pad_pinconf_val2arg(info, param, &arg);
+   if (!pctrl->soc->padctl_val2arg)
+   return -ENOTSUPP;
+
+   ret = pctrl->soc->padctl_val2arg(info, param, &arg);
if (ret)
return ret;
 
@@ -349,7 +298,10 @@ static int owl_pin_config_set(struct pinctrl_dev *pctrldev,
if (ret)
return ret;
 
-   ret = owl_pad_pinconf_arg2val(info, param, &arg);
+   if (!pctrl->soc->padctl_arg2val)
+   return -ENOTSUPP;
+
+   ret = pctrl->soc->padctl_arg2val(info, param, &arg);
if (ret)
return ret;
 
diff --git a/drivers/pinctrl/actions/pinctrl-owl.h 
b/drivers/pinctrl/actions/pinctrl-owl.h
index a7bcacad11d5..dae2e8363fd5 100644
--- a/drivers/pinctrl/actions/pinctrl-owl.h
+++ b/drivers/pinctrl/actions/pinctrl-owl.h
@@ -298,6 +298,12 @@ struct owl_pinctrl_soc_data {
unsigned int ngpios;
const struct owl_gpio_port *ports;
unsigned int nports;
+   int (*padctl_val2arg)(const struct owl_padinfo *padinfo,
+   unsigned int param,
+   u32 *arg);
+   int (*padctl_arg2val)(const struct owl_padinfo *info,
+   unsigned int param,
+   u32 *arg);
 };
 
 int owl_pinctrl_probe(struct platform_device *pdev,
diff --git a/drivers/pinctrl/actions/pinctrl-s900.c 
b/drivers/pinctrl/actions/pinctrl-s900.c
index 0597009d8369..2e19fecbe5e9 100644
--- a/drivers/pinctrl/actions/pinctrl-s900.c
+++ b/drivers/pinctrl/actions/pinctrl-s900.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "pinctrl-owl.h"
 
 /* Pinctrl registers offset */
@@ -1717,6 +1718,67 @@ static const struct owl_gpio_port s900_gpio_ports[] = {
OWL_GPIO_PORT(F, 0x00F0, 8, 0x0, 0x4, 0x8, 0x460, 0x140, 0x144, 0x178, 
0)
 };
 
+enum owl_pinconf_pull {
+   OWL_PINCONF_PULL_HIZ,
+   OWL_PINCONF_PULL_DOWN,
+   OWL_PINCONF_PULL_UP,
+   

Re: [TRIVIAL RFC PATCH] Kconfigs - reduce use of "depends on EXPERT"

[Michael Büsch]

On Sun, 29 Jul 2018 11:16:37 -0700
Joe Perches  wrote:

> (removing a bunch of cc's)
> 
> On Sun, 2018-07-29 at 13:42 +0200, Michael Büsch wrote:
> > On Sat, 28 Jul 2018 15:13:00 -0700
> > Joe Perches  wrote:
> >   
> > >  config SSB_SILENT
> > > - bool "No SSB kernel messages"
> > > - depends on SSB && EXPERT
> > > + bool "No SSB kernel messages" if EXPERT
> > > + depends on SSB
> > >   help
> > > This option turns off all Sonics Silicon Backplane printks.
> > > Note that you won't be able to identify problems, once  
> > 
> > 
> > What about removing this option entirely?
> > We would just have to remove it from Kconfig and from its only use in
> > drivers/ssb/ssb_private.h
> > I don't think anybody uses (or should use) this option anyway.
> > That would reduce the EXPERT dependencies by one, which is a good
> > thing. :)  
> 
> I'm fine with that, but it was originally your code from
> the first ssb commit in 2007:
> 
>   This might only be desired for production kernels on
>   embedded devices to reduce the kernel size.
> 
> Presumably for ddwrt and such.

Yeah, but it doesn't make sense to do this in ssb only.
And it only saves a couple of k at best.

> Removal could simplify the ssb_ printk logic a bit too.
> 
> Perhaps something like the below,
> but your code, your decisions...:


This is great!
Thanks.

Reviewed-by: Michael Buesch 


-- 
Michael


pgpUyrQIFmckt.pgp
Description: OpenPGP digital signature

Re: [PATCH 5/9] arm64: dts: actions: Add Reset Controller support for S700 SoC

[Parthiban Nallathambi]

Hi Mani,

On 07/27/2018 08:45 PM, Manivannan Sadhasivam wrote:
> Add reset controller property and bindings header for the
> Actions Semi S700 SoC DTS.
> 
> Signed-off-by: Manivannan Sadhasivam 
> ---
>  arch/arm64/boot/dts/actions/s700.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/actions/s700.dtsi 
> b/arch/arm64/boot/dts/actions/s700.dtsi
> index 59d29e4ca404..db4c544d5311 100644
> --- a/arch/arm64/boot/dts/actions/s700.dtsi
> +++ b/arch/arm64/boot/dts/actions/s700.dtsi
> @@ -5,6 +5,7 @@
>  
>  #include 
>  #include 
> +#include 

Typo here, this should be s700-reset.h

>  
>  / {
>   compatible = "actions,s700";
> @@ -165,6 +166,7 @@
>   reg = <0x0 0xe0168000 0x0 0x1000>;
>   clocks = <&hosc>, <&losc>;
>   #clock-cells = <1>;
> + #reset-cells = <1>;
>   };
>  
>   sps: power-controller@e01b0100 {
> 

-- 
Thanks,
Parthiban N

Re: [PATCH 8/8] staging: rtl8188eu: use is_broadcast_ether_addr

[Michael Straube]

On 07/29/18 20:21, Michael Straube wrote:

On 07/29/18 19:59, Joe Perches wrote:

On Sun, 2018-07-29 at 19:42 +0200, Michael Straube wrote:

On 07/29/18 19:21, Joe Perches wrote:

On Sun, 2018-07-29 at 19:08 +0200, Michael Straube wrote:

Use is_broadcast_ether_addr instead of checking each byte of the
address array for 0xff. Shortens the code and improves readability.


You should show in the commit log that sta_addr is __aligned(2)
as required by is_broadcast_ether_addr, otherwise you could be
introducing runtime alignment defects.



Ok, sta_addr is used from following structs.

struct ieee_param {
  u32 cmd;
  u8 sta_addr[ETH_ALEN];
  union {
  ...
  ...
  }; u
};

struct ieee_param_ex {
u32 cmd;
u8 sta_addr[ETH_ALEN];
u8 data[0];
};

Well, looking at it now, I'm not sure about the alignment anymore
in the struct that contains the union. Is sta_addr in the first
struct __aligned(2)?

Should I include the snippets in the commit message, or is just
writing that sta_addr is __aligned(2) enough? (if it is in the
first case...)


It's enough to just state that the uses are properly aligned
as long as you looked and understand that it's required.



Ok, thank you.

I looked at it and understand that it's required.
But, as mentioned, at second look I'm not sure about the union.

I guess I need to read a little more about the alignment of unions.


For now I will resend the series without this patch.
I don't feel comfortable with sending something I don't fully understand, yet.

Michael

[PATCH v2 7/7] staging: rtl8188eu: remove unnecessary parentheses

[Michael Straube]

Remove unnecessary parentheses, also clears checkpatch issues about
missing spaces around '-'.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/hal_intf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/hal_intf.c 
b/drivers/staging/rtl8188eu/hal/hal_intf.c
index b21ba01abc57..b8fecc952cfc 100644
--- a/drivers/staging/rtl8188eu/hal/hal_intf.c
+++ b/drivers/staging/rtl8188eu/hal/hal_intf.c
@@ -48,15 +48,15 @@ uint rtw_hal_deinit(struct adapter *adapt)
 
 void rtw_hal_update_ra_mask(struct adapter *adapt, u32 mac_id, u8 rssi_level)
 {
-   struct mlme_priv *pmlmepriv = &(adapt->mlmepriv);
+   struct mlme_priv *pmlmepriv = &adapt->mlmepriv;
 
if (check_fwstate(pmlmepriv, WIFI_AP_STATE)) {
 #ifdef CONFIG_88EU_AP_MODE
struct sta_info *psta = NULL;
struct sta_priv *pstapriv = &adapt->stapriv;
 
-   if ((mac_id-1) > 0)
-   psta = pstapriv->sta_aid[(mac_id-1) - 1];
+   if (mac_id - 1 > 0)
+   psta = pstapriv->sta_aid[mac_id - 2];
if (psta)
add_RATid(adapt, psta, 0);/* todo: based on rssi_level*/
 #endif
-- 
2.18.0

[PATCH v2 6/7] staging: rtl8188eu: fix comparsion to true

[Michael Straube]

Use if(x) instead of if(x == true).

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/hal_intf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/hal/hal_intf.c 
b/drivers/staging/rtl8188eu/hal/hal_intf.c
index c43e7b438058..b21ba01abc57 100644
--- a/drivers/staging/rtl8188eu/hal/hal_intf.c
+++ b/drivers/staging/rtl8188eu/hal/hal_intf.c
@@ -50,7 +50,7 @@ void rtw_hal_update_ra_mask(struct adapter *adapt, u32 
mac_id, u8 rssi_level)
 {
struct mlme_priv *pmlmepriv = &(adapt->mlmepriv);
 
-   if (check_fwstate(pmlmepriv, WIFI_AP_STATE) == true) {
+   if (check_fwstate(pmlmepriv, WIFI_AP_STATE)) {
 #ifdef CONFIG_88EU_AP_MODE
struct sta_info *psta = NULL;
struct sta_priv *pstapriv = &adapt->stapriv;
-- 
2.18.0

[PATCH v2 4/7] staging: rtl8188eu: remove redundant includes

[Michael Straube]

Both osdep_service.h and drv_types.h are included from hal_intf.h,
so remove the redundant includes from hal_intf.c.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/hal_intf.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/hal_intf.c 
b/drivers/staging/rtl8188eu/hal/hal_intf.c
index aaa1718ca603..0baf4e616b6a 100644
--- a/drivers/staging/rtl8188eu/hal/hal_intf.c
+++ b/drivers/staging/rtl8188eu/hal/hal_intf.c
@@ -6,8 +6,6 @@
  
**/
 
 #define _HAL_INTF_C_
-#include 
-#include 
 #include 
 
 uintrtw_hal_init(struct adapter *adapt)
-- 
2.18.0

[PATCH v2 5/7] staging: rtl8188eu: replace tabs with spaces

[Michael Straube]

Replace tabs with spaces in function definition and variable
declarations.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/hal_intf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/hal_intf.c 
b/drivers/staging/rtl8188eu/hal/hal_intf.c
index 0baf4e616b6a..c43e7b438058 100644
--- a/drivers/staging/rtl8188eu/hal/hal_intf.c
+++ b/drivers/staging/rtl8188eu/hal/hal_intf.c
@@ -8,9 +8,9 @@
 #define _HAL_INTF_C_
 #include 
 
-uintrtw_hal_init(struct adapter *adapt)
+uint rtw_hal_init(struct adapter *adapt)
 {
-   uintstatus = _SUCCESS;
+   uint status = _SUCCESS;
 
adapt->hw_init_completed = false;
 
@@ -34,7 +34,7 @@ uint   rtw_hal_init(struct adapter *adapt)
 
 uint rtw_hal_deinit(struct adapter *adapt)
 {
-   uintstatus = _SUCCESS;
+   uint status = _SUCCESS;
 
status = rtl8188eu_hal_deinit(adapt);
 
-- 
2.18.0

[PATCH v2 3/7] staging: rtl8188eu: remove unused rtw_handle_tkip_mic_err()

[Michael Straube]

The function rtw_handle_tkip_mic_err() is never used, so remove it.
Discovered by cppcheck.

Signed-off-by: Michael Straube 
---
 .../staging/rtl8188eu/include/recv_osdep.h|  2 --
 drivers/staging/rtl8188eu/os_dep/recv_linux.c | 36 ---
 2 files changed, 38 deletions(-)

diff --git a/drivers/staging/rtl8188eu/include/recv_osdep.h 
b/drivers/staging/rtl8188eu/include/recv_osdep.h
index d2341521cc8e..2c05964b757d 100644
--- a/drivers/staging/rtl8188eu/include/recv_osdep.h
+++ b/drivers/staging/rtl8188eu/include/recv_osdep.h
@@ -19,8 +19,6 @@ s32  rtw_recv_entry(struct recv_frame *precv_frame);
 int rtw_recv_indicatepkt(struct adapter *adapter,
 struct recv_frame *recv_frame);
 
-void rtw_handle_tkip_mic_err(struct adapter *padapter, u8 bgroup);
-
 int rtw_os_recvbuf_resource_alloc(struct adapter *adapt, struct recv_buf *buf);
 
 void rtw_init_recv_timer(struct recv_reorder_ctrl *preorder_ctrl);
diff --git a/drivers/staging/rtl8188eu/os_dep/recv_linux.c 
b/drivers/staging/rtl8188eu/os_dep/recv_linux.c
index deadf26ea2aa..148ee94bbf3d 100644
--- a/drivers/staging/rtl8188eu/os_dep/recv_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/recv_linux.c
@@ -25,42 +25,6 @@ int rtw_os_recvbuf_resource_alloc(struct adapter *padapter,
return _SUCCESS;
 }
 
-void rtw_handle_tkip_mic_err(struct adapter *padapter, u8 bgroup)
-{
-   union iwreq_data wrqu;
-   struct iw_michaelmicfailure ev;
-   struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
-   struct security_priv *psecuritypriv = &padapter->securitypriv;
-   u32 cur_time = 0;
-
-   if (psecuritypriv->last_mic_err_time == 0) {
-   psecuritypriv->last_mic_err_time = jiffies;
-   } else {
-   cur_time = jiffies;
-
-   if (cur_time - psecuritypriv->last_mic_err_time < 60*HZ) {
-   psecuritypriv->btkip_countermeasure = true;
-   psecuritypriv->last_mic_err_time = 0;
-   psecuritypriv->btkip_countermeasure_time = cur_time;
-   } else {
-   psecuritypriv->last_mic_err_time = jiffies;
-   }
-   }
-
-   memset(&ev, 0x00, sizeof(ev));
-   if (bgroup)
-   ev.flags |= IW_MICFAILURE_GROUP;
-   else
-   ev.flags |= IW_MICFAILURE_PAIRWISE;
-
-   ev.src_addr.sa_family = ARPHRD_ETHER;
-   memcpy(ev.src_addr.sa_data, &pmlmepriv->assoc_bssid[0], ETH_ALEN);
-   memset(&wrqu, 0x00, sizeof(wrqu));
-   wrqu.data.length = sizeof(ev);
-   wireless_send_event(padapter->pnetdev, IWEVMICHAELMICFAILURE,
-   &wrqu, (char *)&ev);
-}
-
 int rtw_recv_indicatepkt(struct adapter *padapter,
 struct recv_frame *precv_frame)
 {
-- 
2.18.0

[PATCH v2 2/7] staging: rtl8188eu: remove unused should_forbid_n_rate()

[Michael Straube]

The function should_forbid_n_rate() is never used, so remove it.
Discovered by cppcheck.

Signed-off-by: Michael Straube 
---
 .../staging/rtl8188eu/core/rtw_wlan_util.c| 35 ---
 .../staging/rtl8188eu/include/rtw_mlme_ext.h  |  1 -
 2 files changed, 36 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c 
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 0fe35e141926..b9406583e501 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -1094,41 +1094,6 @@ unsigned int is_ap_in_tkip(struct adapter *padapter)
}
 }
 
-unsigned int should_forbid_n_rate(struct adapter *padapter)
-{
-   u32 i;
-   struct ndis_802_11_var_ie *pIE;
-   struct mlme_priv*pmlmepriv = &padapter->mlmepriv;
-   struct wlan_bssid_ex  *cur_network = &pmlmepriv->cur_network.network;
-
-   if (rtw_get_capability((struct wlan_bssid_ex *)cur_network) & 
WLAN_CAPABILITY_PRIVACY) {
-   for (i = sizeof(struct ndis_802_11_fixed_ie); i < 
cur_network->ie_length;) {
-   pIE = (struct ndis_802_11_var_ie *)(cur_network->ies + 
i);
-
-   switch (pIE->ElementID) {
-   case _VENDOR_SPECIFIC_IE_:
-   if (!memcmp(pIE->data, RTW_WPA_OUI, 4) &&
-   ((!memcmp((pIE->data + 12), 
WPA_CIPHER_SUITE_CCMP, 4)) ||
-   (!memcmp((pIE->data + 16), 
WPA_CIPHER_SUITE_CCMP, 4
-   return false;
-   break;
-   case _RSN_IE_2_:
-   if  ((!memcmp((pIE->data + 8), 
RSN_CIPHER_SUITE_CCMP, 4))  ||
-  (!memcmp((pIE->data + 12), 
RSN_CIPHER_SUITE_CCMP, 4)))
-   return false;
-   default:
-   break;
-   }
-
-   i += (pIE->Length + 2);
-   }
-
-   return true;
-   } else {
-   return false;
-   }
-}
-
 unsigned int is_ap_in_wep(struct adapter *padapter)
 {
u32 i;
diff --git a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h 
b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
index c072e1e25d61..ade68af15e04 100644
--- a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
@@ -534,7 +534,6 @@ unsigned char get_highest_rate_idx(u32 mask);
 int support_short_GI(struct adapter *padapter, struct ieee80211_ht_cap *caps);
 unsigned int is_ap_in_tkip(struct adapter *padapter);
 unsigned int is_ap_in_wep(struct adapter *padapter);
-unsigned int should_forbid_n_rate(struct adapter *padapter);
 
 void report_join_res(struct adapter *padapter, int res);
 void report_survey_event(struct adapter *padapter,
-- 
2.18.0

[PATCH v2 1/7] staging: rtl8188eu: remove unused dump_txrpt_ccx_88e()

[Michael Straube]

The function dump_txrpt_ccx_88e() is nerver used, so remove it.
Discovered by cppcheck.

Signed-off-by: Michael Straube 
---
v2: removed last patch (8/8) from the series

 drivers/staging/rtl8188eu/hal/rtl8188e_xmit.c | 22 ---
 .../staging/rtl8188eu/include/rtl8188e_xmit.h |  1 -
 2 files changed, 23 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_xmit.c 
b/drivers/staging/rtl8188eu/hal/rtl8188e_xmit.c
index 6883746f3b8b..9b8a284544ac 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_xmit.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_xmit.c
@@ -10,28 +10,6 @@
 #include 
 #include 
 
-void dump_txrpt_ccx_88e(void *buf)
-{
-   struct txrpt_ccx_88e *txrpt_ccx = buf;
-
-   DBG_88E("%s:\n"
-   "tag1:%u, pkt_num:%u, txdma_underflow:%u, int_bt:%u, 
int_tri:%u, int_ccx:%u\n"
-   "mac_id:%u, pkt_ok:%u, bmc:%u\n"
-   "retry_cnt:%u, lifetime_over:%u, retry_over:%u\n"
-   "ccx_qtime:%u\n"
-   "final_data_rate:0x%02x\n"
-   "qsel:%u, sw:0x%03x\n",
-   __func__, txrpt_ccx->tag1, txrpt_ccx->pkt_num,
-   txrpt_ccx->txdma_underflow, txrpt_ccx->int_bt,
-   txrpt_ccx->int_tri, txrpt_ccx->int_ccx,
-   txrpt_ccx->mac_id, txrpt_ccx->pkt_ok, txrpt_ccx->bmc,
-   txrpt_ccx->retry_cnt, txrpt_ccx->lifetime_over,
-   txrpt_ccx->retry_over, txrpt_ccx_qtime_88e(txrpt_ccx),
-   txrpt_ccx->final_data_rate, txrpt_ccx->qsel,
-   txrpt_ccx_sw_88e(txrpt_ccx)
-   );
-}
-
 void handle_txrpt_ccx_88e(struct adapter *adapter, u8 *buf)
 {
struct txrpt_ccx_88e *txrpt_ccx = (struct txrpt_ccx_88e *)buf;
diff --git a/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h 
b/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
index 17e7b3016674..20d35480dab8 100644
--- a/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
+++ b/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
@@ -152,7 +152,6 @@ void rtl8188eu_xmit_tasklet(void *priv);
 s32 rtl8188eu_xmitframe_complete(struct adapter *padapter,
 struct xmit_priv *pxmitpriv);
 
-void dump_txrpt_ccx_88e(void *buf);
 void handle_txrpt_ccx_88e(struct adapter *adapter, u8 *buf);
 
 void _dbg_dump_tx_info(struct adapter *padapter, int frame_tag,
-- 
2.18.0

Re: [PATCH 03/10] smp,cpumask: introduce on_each_cpu_cond_mask

[Andy Lutomirski]

> On Jul 29, 2018, at 10:51 AM, Rik van Riel  wrote:
> 
>> On Sun, 2018-07-29 at 08:36 -0700, Andy Lutomirski wrote:
>>> On Jul 29, 2018, at 5:00 AM, Rik van Riel  wrote:
>>> 
 On Sat, 2018-07-28 at 19:57 -0700, Andy Lutomirski wrote:
 On Sat, Jul 28, 2018 at 2:53 PM, Rik van Riel 
 wrote:
> Introduce a variant of on_each_cpu_cond that iterates only over
> the
> CPUs in a cpumask, in order to avoid making callbacks for every
> single
> CPU in the system when we only need to test a subset.
 Nice.
 Although, if you want to be really fancy, you could optimize this
 (or
 add a variant) that does the callback on the local CPU in
 parallel
 with the remote ones.  That would give a small boost to TLB
 flushes.
>>> 
>>> The test_func callbacks are not run remotely, but on
>>> the local CPU, before deciding who to send callbacks
>>> to.
>>> 
>>> The actual IPIs are sent in parallel, if the cpumask
>>> allocation succeeds (it always should in many kernel
>>> configurations, and almost always in the rest).
>>> 
>> 
>> What I meant is that on_each_cpu_mask does:
>> 
>> smp_call_function_many(mask, func, info, wait);
>> if (cpumask_test_cpu(cpu, mask)) {
>>   unsigned long flags;
>>   local_irq_save(flags); func(info);
>>   local_irq_restore(flags);
>> }
>> 
>> So it IPIs all the remote CPUs in parallel, then waits, then does the
>> local work.  In principle, the local flush could be done after
>> triggering the IPIs but before they all finish.
> 
> Grepping around the code, I found a few examples where the
> calling code appears to expect that smp_call_function_many
> also calls "func" on the local CPU.
> 
> For example, kvm_emulate_wbinvd_noskip has this:
> 
>if (kvm_x86_ops->has_wbinvd_exit()) {
>int cpu = get_cpu();
> 
>cpumask_set_cpu(cpu, vcpu->arch.wbinvd_dirty_mask);
>smp_call_function_many(vcpu->arch.wbinvd_dirty_mask,
>wbinvd_ipi, NULL, 1);
>put_cpu();
>cpumask_clear(vcpu->arch.wbinvd_dirty_mask);
>} else
>wbinvd();
> 
> This seems to result in systems with ->has_wbinvd_exit
> only calling wbinvd_ipi on OTHER CPUs, and not on the
> CPU where the guest exited with wbinvd?
> 
> This seems unintended.
> 
> I guess looking into on_each_cpu_mask might be a little
> higher priority than waiting until the next Outreachy
> season :)
> 

The right approach might be a tree wise rename from smp_call_... to 
on_other_cpus_mask() it similar. The current naming and semantics are extremely 
confusing.

Linus, this is the kind of thing you seem to like taking outside the merge 
window. What do you think about a straight-up search_and_replace to make rename 
the smp_call_... functions to exactly match the corresponding on_each_cpu 
functions except with “each” replaced with “other”?