[PATCH] usb: usb-skeleton: make MODULE_LICENSE and SPDX tag match

[Marcus Folkesson]

GPL v2 is the original license according to the old license text.
See f64cdd0e94f1faf3b7f2b03af71f70dc6d8da0c2.

Signed-off-by: Marcus Folkesson 
---
 drivers/usb/usb-skeleton.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/usb-skeleton.c b/drivers/usb/usb-skeleton.c
index 26ca0ec01fd5..c3ddd0f1f449 100644
--- a/drivers/usb/usb-skeleton.c
+++ b/drivers/usb/usb-skeleton.c
@@ -640,4 +640,4 @@ static struct usb_driver skel_driver = {
 
 module_usb_driver(skel_driver);
 
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
-- 
2.16.2

[PATCH] usb: usb-skeleton: make MODULE_LICENSE and SPDX tag match

[Marcus Folkesson]

GPL v2 is the original license according to the old license text.
See f64cdd0e94f1faf3b7f2b03af71f70dc6d8da0c2.

Signed-off-by: Marcus Folkesson 
---
 drivers/usb/usb-skeleton.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/usb-skeleton.c b/drivers/usb/usb-skeleton.c
index 26ca0ec01fd5..c3ddd0f1f449 100644
--- a/drivers/usb/usb-skeleton.c
+++ b/drivers/usb/usb-skeleton.c
@@ -640,4 +640,4 @@ static struct usb_driver skel_driver = {
 
 module_usb_driver(skel_driver);
 
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
-- 
2.16.2

[GIT PULL] GPIO fixes for v4.16 - second try

[Linus Walleij]

Hi Linus,

here is this pull request again, fixing up the device tree property readouts
for the GPIOs. Last time the fix needed fixing, not I think the fix is fixed.

Please pull it in!

Yours,
Linus Walleij


The following changes since commit 7928b2cbe55b2a410a0f5c1f154610059c57b1b2:

  Linux 4.16-rc1 (2018-02-11 15:04:29 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
tags/gpio-v4.16-2

for you to fetch changes up to ce27fb2c56db6ccfe8099343bb4afdab15e77e7b:

  gpio: Handle deferred probing in of_find_gpio() properly (2018-02-27
09:48:07 +0100)


GPIO fixes for v4.16:

- Fix up device tree properties readout caused by my own
  refactorings.


Chen-Yu Tsai (1):
  gpio: Handle deferred probing in of_find_gpio() properly

Maxime Ripard (1):
  gpiolib: Keep returning EPROBE_DEFER when we should

 drivers/gpio/gpiolib-of.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

[GIT PULL] GPIO fixes for v4.16 - second try

[Linus Walleij]

Hi Linus,

here is this pull request again, fixing up the device tree property readouts
for the GPIOs. Last time the fix needed fixing, not I think the fix is fixed.

Please pull it in!

Yours,
Linus Walleij


The following changes since commit 7928b2cbe55b2a410a0f5c1f154610059c57b1b2:

  Linux 4.16-rc1 (2018-02-11 15:04:29 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
tags/gpio-v4.16-2

for you to fetch changes up to ce27fb2c56db6ccfe8099343bb4afdab15e77e7b:

  gpio: Handle deferred probing in of_find_gpio() properly (2018-02-27
09:48:07 +0100)


GPIO fixes for v4.16:

- Fix up device tree properties readout caused by my own
  refactorings.


Chen-Yu Tsai (1):
  gpio: Handle deferred probing in of_find_gpio() properly

Maxime Ripard (1):
  gpiolib: Keep returning EPROBE_DEFER when we should

 drivers/gpio/gpiolib-of.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Re[v2]: Use of GCC plugin instead ISO C

[Progyan Bhattacharya]

On Thu, 2018-02-22 at 20:07 +0530, Progyan Bhattacharya wrote:
> On Sun, 2018-02-18 at 08:47 -0800, Randy Dunlap wrote:
> > You could try (re)building with V=1 on the "make" command line and
> > capture
> > the output to see where the "pedantic" is coming from.
> 
> This is a late reply.
> I did what you told me and the build failed at following point:
> 
>   /bin/sh: /home/progyan/Programs/Linux/tools/objtool/.objtool-
> in.o.cmd: Permission denied
> 
> I am not sure what permission it wants or what to make of it.
> 
> Thanks in advance.

-- 
Regards,
Progyan Bhattacharya
(http://codeprogyan.me)

Re: [PATCH v4 1/4] HID: add driver for Valve Steam Controller

[Marcus Folkesson]

Rodrigo,

On Wed, Feb 28, 2018 at 11:49:26PM +0100, Rodrigo Rivas Costa wrote:
> On Wed, Feb 28, 2018 at 09:21:15PM +0200, Andy Shevchenko wrote:
> > On Wed, Feb 28, 2018 at 8:43 PM, Rodrigo Rivas Costa
> >  wrote:
> > > There are two ways to connect the Steam Controller: directly to the USB
> > > or with the USB wireless adapter.  Both methods are similar, but the
> > > wireless adapter can connect up to 4 devices at the same time.
> > >
> > > The wired device will appear as 3 interfaces: a virtual mouse, a virtual
> > > keyboard and a custom HID device.
> > >
> > > The wireless device will appear as 5 interfaces: a virtual keyboard and
> > > 4 custom HID devices, that will remain silent until a device is actually
> > > connected.
> > >
> > > The custom HID device has a report descriptor with all vendor specific
> > > usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
> > > Steam Client provices a software translation by using direct USB access
> > > and a creates a uinput virtual gamepad.
> > >
> > > This driver was reverse engineered to provide direct kernel support in
> > > case you cannot, or do not want to, use Valve Steam Client. It disables
> > > the virtual keyboard and mouse, as they are not so useful when you have
> > > a working gamepad.
> > 
> > 
> > > +// SPDX-License-Identifier: GPL-2.0
> > 
> > > +MODULE_LICENSE("GPL");
> > 
> > Not the same.
> 
> Hmmm... I copied from usb-skeleton.c, IIRC...
> I'll change to GPL-2.0+, that would be correct, I think.

Yep, the usb-skeleton.c is wrong.
I have prepared a patch, just not submitted it yet..

GPL-2.0+ is "GPLv2 or later" if that is what you want.

Best regards
Marcus Folkesson

Re[v2]: Use of GCC plugin instead ISO C

[Progyan Bhattacharya]

On Thu, 2018-02-22 at 20:07 +0530, Progyan Bhattacharya wrote:
> On Sun, 2018-02-18 at 08:47 -0800, Randy Dunlap wrote:
> > You could try (re)building with V=1 on the "make" command line and
> > capture
> > the output to see where the "pedantic" is coming from.
> 
> This is a late reply.
> I did what you told me and the build failed at following point:
> 
>   /bin/sh: /home/progyan/Programs/Linux/tools/objtool/.objtool-
> in.o.cmd: Permission denied
> 
> I am not sure what permission it wants or what to make of it.
> 
> Thanks in advance.

-- 
Regards,
Progyan Bhattacharya
(http://codeprogyan.me)

Re: [PATCH v4 1/4] HID: add driver for Valve Steam Controller

[Marcus Folkesson]

Rodrigo,

On Wed, Feb 28, 2018 at 11:49:26PM +0100, Rodrigo Rivas Costa wrote:
> On Wed, Feb 28, 2018 at 09:21:15PM +0200, Andy Shevchenko wrote:
> > On Wed, Feb 28, 2018 at 8:43 PM, Rodrigo Rivas Costa
> >  wrote:
> > > There are two ways to connect the Steam Controller: directly to the USB
> > > or with the USB wireless adapter.  Both methods are similar, but the
> > > wireless adapter can connect up to 4 devices at the same time.
> > >
> > > The wired device will appear as 3 interfaces: a virtual mouse, a virtual
> > > keyboard and a custom HID device.
> > >
> > > The wireless device will appear as 5 interfaces: a virtual keyboard and
> > > 4 custom HID devices, that will remain silent until a device is actually
> > > connected.
> > >
> > > The custom HID device has a report descriptor with all vendor specific
> > > usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
> > > Steam Client provices a software translation by using direct USB access
> > > and a creates a uinput virtual gamepad.
> > >
> > > This driver was reverse engineered to provide direct kernel support in
> > > case you cannot, or do not want to, use Valve Steam Client. It disables
> > > the virtual keyboard and mouse, as they are not so useful when you have
> > > a working gamepad.
> > 
> > 
> > > +// SPDX-License-Identifier: GPL-2.0
> > 
> > > +MODULE_LICENSE("GPL");
> > 
> > Not the same.
> 
> Hmmm... I copied from usb-skeleton.c, IIRC...
> I'll change to GPL-2.0+, that would be correct, I think.

Yep, the usb-skeleton.c is wrong.
I have prepared a patch, just not submitted it yet..

GPL-2.0+ is "GPLv2 or later" if that is what you want.

Best regards
Marcus Folkesson

Re: [PATCH v1 3/4] ASoC: codecs: pcm179x: Add reset gpio

[Mylène Josserand]

Hello,

On Tue, 27 Feb 2018 23:02:00 +0100
Thomas Petazzoni  wrote:

> Hello,
> 
> On Tue, 27 Feb 2018 22:24:32 +0100, Mylène Josserand wrote:
> > Add a reset gpio to be able to reset this line at startup.
> > 
> > Signed-off-by: Mylène Josserand   
> 
> This needs an updated of the DT binding, since it's adding support for
> a new reset-gpios property.

True, I will add it in my new series.

> 
> Thanks!
> 
> Thomas

Thanks,

Mylène

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

Re: [PATCH v1 3/4] ASoC: codecs: pcm179x: Add reset gpio

[Mylène Josserand]

Hello,

On Tue, 27 Feb 2018 23:02:00 +0100
Thomas Petazzoni  wrote:

> Hello,
> 
> On Tue, 27 Feb 2018 22:24:32 +0100, Mylène Josserand wrote:
> > Add a reset gpio to be able to reset this line at startup.
> > 
> > Signed-off-by: Mylène Josserand   
> 
> This needs an updated of the DT binding, since it's adding support for
> a new reset-gpios property.

True, I will add it in my new series.

> 
> Thanks!
> 
> Thomas

Thanks,

Mylène

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

Re: [PATCH v1 2/4] ASoC: codecs: pcm179x: Add support for PCM1789

[Mylène Josserand]

Hello,

On Tue, 27 Feb 2018 22:56:29 +0100
Thomas Petazzoni  wrote:

> Hello,
> 
> On Tue, 27 Feb 2018 22:24:31 +0100, Mylène Josserand wrote:
> 
> > diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c
> > index 795a0657c097..83a2e1508df8 100644
> > --- a/sound/soc/codecs/pcm179x-i2c.c
> > +++ b/sound/soc/codecs/pcm179x-i2c.c
> > @@ -26,10 +26,13 @@
> >  static int pcm179x_i2c_probe(struct i2c_client *client,
> >   const struct i2c_device_id *id)
> >  {
> > -   struct regmap *regmap;
> > +   struct regmap *regmap = NULL;  
> 
> I don't think this change is useful, since regmap is always initialized
> below anyway.

okay.

> 
> 
> > +   if (mute)
> > +   val = ~(PCM1789_MUTE_L_EN | PCM1789_MUTE_R_EN);  
> 
> That's not really useful with regmap_update_bits() which already does
> the masking, no?

Yep

> 
> > +   else
> > +   val = PCM1789_MUTE_L_EN | PCM1789_MUTE_R_EN;
> > +   ret = regmap_update_bits(priv->regmap, PCM1789_SOFT_MUTE,
> > +PCM1789_MUTE_MASK, val);  
> 
> Couldn't this be:
> 
>   if (mute)
>   val = 0;
>   else
>   val = PCM1789_MUTE_MASK;
> 
>   ret = regmap_update_bits(priv->regmap, PCM1789_SOFT_MUTE,
>PCM1789_MUTE_MASK, val);
> 

I will update my V2 with it.

> 
> > +static struct snd_soc_dai_driver pcm1789_dai = {
> > +   .name = "pcm1789-hifi",
> > +   .playback = {
> > +   .stream_name = "Playback",
> > +   .channels_min = 2,
> > +   .channels_max = 2,
> > +   .rates = SNDRV_PCM_RATE_CONTINUOUS,
> > +   .rate_min = 1,
> > +   .rate_max = 20,
> > +   .formats = PCM1792A_FORMATS, },  
> 
> Nit: the closing curly brace should be on a separate line.

Yep, thanks.

> 
> 
> > +   if (type == PCM1789)
> > +   return devm_snd_soc_register_component(dev,
> > +  
> > _component_dev_pcm1789,
> > +  _dai, 1);
> > +  
> 
> Perhaps a "else" here ?

Sure

> 
> > return devm_snd_soc_register_component(dev,
> > _component_dev_pcm179x, _dai, 1);  
> 
> Thanks!
> 
> Thomas

Thank you,

Mylène

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

Re: [PATCH v1 2/4] ASoC: codecs: pcm179x: Add support for PCM1789

[Mylène Josserand]

Hello,

On Tue, 27 Feb 2018 22:56:29 +0100
Thomas Petazzoni  wrote:

> Hello,
> 
> On Tue, 27 Feb 2018 22:24:31 +0100, Mylène Josserand wrote:
> 
> > diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c
> > index 795a0657c097..83a2e1508df8 100644
> > --- a/sound/soc/codecs/pcm179x-i2c.c
> > +++ b/sound/soc/codecs/pcm179x-i2c.c
> > @@ -26,10 +26,13 @@
> >  static int pcm179x_i2c_probe(struct i2c_client *client,
> >   const struct i2c_device_id *id)
> >  {
> > -   struct regmap *regmap;
> > +   struct regmap *regmap = NULL;  
> 
> I don't think this change is useful, since regmap is always initialized
> below anyway.

okay.

> 
> 
> > +   if (mute)
> > +   val = ~(PCM1789_MUTE_L_EN | PCM1789_MUTE_R_EN);  
> 
> That's not really useful with regmap_update_bits() which already does
> the masking, no?

Yep

> 
> > +   else
> > +   val = PCM1789_MUTE_L_EN | PCM1789_MUTE_R_EN;
> > +   ret = regmap_update_bits(priv->regmap, PCM1789_SOFT_MUTE,
> > +PCM1789_MUTE_MASK, val);  
> 
> Couldn't this be:
> 
>   if (mute)
>   val = 0;
>   else
>   val = PCM1789_MUTE_MASK;
> 
>   ret = regmap_update_bits(priv->regmap, PCM1789_SOFT_MUTE,
>PCM1789_MUTE_MASK, val);
> 

I will update my V2 with it.

> 
> > +static struct snd_soc_dai_driver pcm1789_dai = {
> > +   .name = "pcm1789-hifi",
> > +   .playback = {
> > +   .stream_name = "Playback",
> > +   .channels_min = 2,
> > +   .channels_max = 2,
> > +   .rates = SNDRV_PCM_RATE_CONTINUOUS,
> > +   .rate_min = 1,
> > +   .rate_max = 20,
> > +   .formats = PCM1792A_FORMATS, },  
> 
> Nit: the closing curly brace should be on a separate line.

Yep, thanks.

> 
> 
> > +   if (type == PCM1789)
> > +   return devm_snd_soc_register_component(dev,
> > +  
> > _component_dev_pcm1789,
> > +  _dai, 1);
> > +  
> 
> Perhaps a "else" here ?

Sure

> 
> > return devm_snd_soc_register_component(dev,
> > _component_dev_pcm179x, _dai, 1);  
> 
> Thanks!
> 
> Thomas

Thank you,

Mylène

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

Re: [PATCH v2 5/6] kconfig: invoke oldconfig instead of silentoldconfig from local*config

[Masahiro Yamada]

2018-02-28 14:15 GMT+09:00 Ulf Magnusson :
> On Wed, Feb 28, 2018 at 09:15:25AM +0900, Masahiro Yamada wrote:
>> The purpose of local{yes,mod}config is to arrange the .config file
>> based on actually loaded modules.  It is unnecessary to update
>> include/generated/autoconf.h and include/config/* stuff here.
>>
>> They will be automatically updated during the build.
>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>> Changes in v2:
>>   - newly added
>>
>>  scripts/kconfig/Makefile | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
>> index c5d1d1a..bf9289a 100644
>> --- a/scripts/kconfig/Makefile
>> +++ b/scripts/kconfig/Makefile
>> @@ -49,11 +49,11 @@ localyesconfig localmodconfig: 
>> $(obj)/streamline_config.pl $(obj)/conf
>
> (Unrelated: $(obj)/streamline_config.pl is a checked-in file, so I
> wonder if there's any point to having it as a prerequisite of the phony
> targets local{yes,mod}config.)
>

Indeed.

It it unrelated, but I think it is worth fixing.

Care to send a patch, please?




-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 5/6] kconfig: invoke oldconfig instead of silentoldconfig from local*config

[Masahiro Yamada]

2018-02-28 14:15 GMT+09:00 Ulf Magnusson :
> On Wed, Feb 28, 2018 at 09:15:25AM +0900, Masahiro Yamada wrote:
>> The purpose of local{yes,mod}config is to arrange the .config file
>> based on actually loaded modules.  It is unnecessary to update
>> include/generated/autoconf.h and include/config/* stuff here.
>>
>> They will be automatically updated during the build.
>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>> Changes in v2:
>>   - newly added
>>
>>  scripts/kconfig/Makefile | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
>> index c5d1d1a..bf9289a 100644
>> --- a/scripts/kconfig/Makefile
>> +++ b/scripts/kconfig/Makefile
>> @@ -49,11 +49,11 @@ localyesconfig localmodconfig: 
>> $(obj)/streamline_config.pl $(obj)/conf
>
> (Unrelated: $(obj)/streamline_config.pl is a checked-in file, so I
> wonder if there's any point to having it as a prerequisite of the phony
> targets local{yes,mod}config.)
>

Indeed.

It it unrelated, but I think it is worth fixing.

Care to send a patch, please?




-- 
Best Regards
Masahiro Yamada

Re: [PATCH] Bluetooth: btusb: Add Dell OptiPlex 3060 to btusb_needs_reset_resume_table

[Marcel Holtmann]

Hi Kai-Heng,

> The issue can be reproduced before commit fd865802c66b ("Bluetooth:
> btusb: fix QCA Rome suspend/resume") gets introduced, so the reset
> resume quirk is still needed for this system.
> 
> T:  Bus=01 Lev=01 Prnt=01 Port=13 Cnt=01 Dev#=  4 Spd=12  MxCh= 0
> D:  Ver= 2.01 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=0cf3 ProdID=e007 Rev=00.01
> C:  #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
> I:  If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> I:  If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> 
> Cc: sta...@vger.kernel.org
> Cc: Brian Norris 
> Cc: Hans de Goede 
> Signed-off-by: Kai-Heng Feng 
> ---
> drivers/bluetooth/btusb.c | 7 +++
> 1 file changed, 7 insertions(+)

patch has been applied to bluetooth-stable tree.

Regards

Marcel

Re: [PATCH] Bluetooth: btusb: Add Dell OptiPlex 3060 to btusb_needs_reset_resume_table

[Marcel Holtmann]

Hi Kai-Heng,

> The issue can be reproduced before commit fd865802c66b ("Bluetooth:
> btusb: fix QCA Rome suspend/resume") gets introduced, so the reset
> resume quirk is still needed for this system.
> 
> T:  Bus=01 Lev=01 Prnt=01 Port=13 Cnt=01 Dev#=  4 Spd=12  MxCh= 0
> D:  Ver= 2.01 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=0cf3 ProdID=e007 Rev=00.01
> C:  #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
> I:  If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> I:  If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> 
> Cc: sta...@vger.kernel.org
> Cc: Brian Norris 
> Cc: Hans de Goede 
> Signed-off-by: Kai-Heng Feng 
> ---
> drivers/bluetooth/btusb.c | 7 +++
> 1 file changed, 7 insertions(+)

patch has been applied to bluetooth-stable tree.

Regards

Marcel

Re: [PATCH v2 4/4] pintcrl: support bias-disable of generic and special pins simultaneously

[Sean Wang]

On Thu, 2018-03-01 at 11:47 +0800, Zhiyong Tao wrote:
> On Wed, 2018-02-28 at 15:49 +0800, Zhiyong Tao wrote:
> > On Wed, 2018-02-28 at 15:33 +0800, Sean Wang wrote:
> > > On Mon, 2018-02-26 at 16:34 +0800, Zhiyong Tao wrote:
> > > > For generic pins, parameter "arg" is 0 or 1.
> > > > For special pins, bias-disable is set by R0R1,
> > > > so we need transmited "00" to set bias-disable
> > > > When we set "bias-disable" as high-z property,
> > > > the parameter should be "MTK_PUPD_SET_R1R0_00".
> > > > 
> > > > Signed-off-by: Zhiyong Tao 
> > > > ---
> > > >  drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 13 +++--
> > > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c 
> > > > b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> > > > index 3cf384f..e88ba04 100644
> > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> > > > @@ -301,8 +301,17 @@ static int mtk_pconf_set_pull_select(struct 
> > > > mtk_pinctrl *pctl,
> > > >  * resistor bit, so we need this special handle.
> > > >  */
> > > > if (pctl->devdata->spec_pull_set) {
> > > > -   ret = pctl->devdata->spec_pull_set(mtk_get_regmap(pctl, 
> > > > pin),
> > > > -   pin, pctl->devdata->port_align, isup, arg);
> > > > +   if (enable) {
> > > > +   ret = pctl->devdata->spec_pull_set(
> > > > +   mtk_get_regmap(pctl, pin), pin,
> > > > +   pctl->devdata->port_align, isup,
> > > > +   arg);
> > > > +   } else {
> > > > +   ret = pctl->devdata->spec_pull_set(
> > > > +   mtk_get_regmap(pctl, pin), pin,
> > > > +   pctl->devdata->port_align, isup,
> > > > +   MTK_PUPD_SET_R1R0_00);
> > > > +   }
> > > 
> > > 
> > > it looks like you can use as following snippet with reusing original
> > > logic and less effort on maintaining these common code (?)
> > ==> Thanks for your suggestion very much. It seems better.
> > I will change it in v3.
> 
> ==>
>  Hi sean,
> 
> we try it in our site. we can't change like this.
> Because we will check "arg" after setting "arg = MTK_PUPD_SET_R1R0_00;".
> following snippet which check "arg" value is o or 1. If it not, which
> will print error. so we can't change like this.
> /* For generic pull config, default arg value should be 0 or 1. */
> if (arg != 0 && arg != 1) {
>   dev_err(pctl->dev, "invalid pull-up argument %d on pin %d .\n", arg,
> pin);
>   return -EINVAL;
> }
> 
> Thanks.

why not rewrite it with

@@ -301,8 +301,10 @@ static int mtk_pconf_set_pull_select(struct
mtk_pinctrl *pctl,
 * resistor bit, so we need this special handle.
 */
if (pctl->devdata->spec_pull_set) {
+   unsigned int r1r0 = enable ? arg : MTK_PUPD_SET_R1R0_00;
+
ret = pctl->devdata->spec_pull_set(mtk_get_regmap(pctl,
pin),
-   pin, pctl->devdata->port_align, isup, arg);
+   pin, pctl->devdata->port_align, isup, r1r0);
if (!ret)
return 0;
}

and also I appeared to find another bug in the original code when I had
a close look at the code path, but it's not related to the patch: 

a special pin would still go through the code path to handle
generic pin when the special pin uses an invalid argument.

It looks like err-code from pctl->devdata->spec_pull_set cannot be
used directly to judge its pin type. It's also possible to use it as an
indication of the real failure during special pin is being set up.

> > > 
> > > 
> > > if (pctl->devdata->spec_pull_set) {
> > >   + if (!enable)
> > >   +   arg = MTK_PUPD_SET_R1R0_00;
> > > 
> > >   ret = pctl->devdata->spec_pull_set(mtk_get_regmap(pctl, pin),
> > >   pin, pctl->devdata->port_align, isup, arg);
> > > 
> > > 
> > > Also, it's better to add more comments to such kind of special path for
> > > allowing future SoCs to follow and extend more easily.
> > 
> > ==> Thanks for your suggestion very much. I will add more comments here.
> > > 
> > > > if (!ret)
> > > > return 0;
> > > > }
> > > 
> > > 
> > 
> 
> 

Re: [PATCH v2 4/4] pintcrl: support bias-disable of generic and special pins simultaneously

[Sean Wang]

On Thu, 2018-03-01 at 11:47 +0800, Zhiyong Tao wrote:
> On Wed, 2018-02-28 at 15:49 +0800, Zhiyong Tao wrote:
> > On Wed, 2018-02-28 at 15:33 +0800, Sean Wang wrote:
> > > On Mon, 2018-02-26 at 16:34 +0800, Zhiyong Tao wrote:
> > > > For generic pins, parameter "arg" is 0 or 1.
> > > > For special pins, bias-disable is set by R0R1,
> > > > so we need transmited "00" to set bias-disable
> > > > When we set "bias-disable" as high-z property,
> > > > the parameter should be "MTK_PUPD_SET_R1R0_00".
> > > > 
> > > > Signed-off-by: Zhiyong Tao 
> > > > ---
> > > >  drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 13 +++--
> > > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c 
> > > > b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> > > > index 3cf384f..e88ba04 100644
> > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> > > > @@ -301,8 +301,17 @@ static int mtk_pconf_set_pull_select(struct 
> > > > mtk_pinctrl *pctl,
> > > >  * resistor bit, so we need this special handle.
> > > >  */
> > > > if (pctl->devdata->spec_pull_set) {
> > > > -   ret = pctl->devdata->spec_pull_set(mtk_get_regmap(pctl, 
> > > > pin),
> > > > -   pin, pctl->devdata->port_align, isup, arg);
> > > > +   if (enable) {
> > > > +   ret = pctl->devdata->spec_pull_set(
> > > > +   mtk_get_regmap(pctl, pin), pin,
> > > > +   pctl->devdata->port_align, isup,
> > > > +   arg);
> > > > +   } else {
> > > > +   ret = pctl->devdata->spec_pull_set(
> > > > +   mtk_get_regmap(pctl, pin), pin,
> > > > +   pctl->devdata->port_align, isup,
> > > > +   MTK_PUPD_SET_R1R0_00);
> > > > +   }
> > > 
> > > 
> > > it looks like you can use as following snippet with reusing original
> > > logic and less effort on maintaining these common code (?)
> > ==> Thanks for your suggestion very much. It seems better.
> > I will change it in v3.
> 
> ==>
>  Hi sean,
> 
> we try it in our site. we can't change like this.
> Because we will check "arg" after setting "arg = MTK_PUPD_SET_R1R0_00;".
> following snippet which check "arg" value is o or 1. If it not, which
> will print error. so we can't change like this.
> /* For generic pull config, default arg value should be 0 or 1. */
> if (arg != 0 && arg != 1) {
>   dev_err(pctl->dev, "invalid pull-up argument %d on pin %d .\n", arg,
> pin);
>   return -EINVAL;
> }
> 
> Thanks.

why not rewrite it with

@@ -301,8 +301,10 @@ static int mtk_pconf_set_pull_select(struct
mtk_pinctrl *pctl,
 * resistor bit, so we need this special handle.
 */
if (pctl->devdata->spec_pull_set) {
+   unsigned int r1r0 = enable ? arg : MTK_PUPD_SET_R1R0_00;
+
ret = pctl->devdata->spec_pull_set(mtk_get_regmap(pctl,
pin),
-   pin, pctl->devdata->port_align, isup, arg);
+   pin, pctl->devdata->port_align, isup, r1r0);
if (!ret)
return 0;
}

and also I appeared to find another bug in the original code when I had
a close look at the code path, but it's not related to the patch: 

a special pin would still go through the code path to handle
generic pin when the special pin uses an invalid argument.

It looks like err-code from pctl->devdata->spec_pull_set cannot be
used directly to judge its pin type. It's also possible to use it as an
indication of the real failure during special pin is being set up.

> > > 
> > > 
> > > if (pctl->devdata->spec_pull_set) {
> > >   + if (!enable)
> > >   +   arg = MTK_PUPD_SET_R1R0_00;
> > > 
> > >   ret = pctl->devdata->spec_pull_set(mtk_get_regmap(pctl, pin),
> > >   pin, pctl->devdata->port_align, isup, arg);
> > > 
> > > 
> > > Also, it's better to add more comments to such kind of special path for
> > > allowing future SoCs to follow and extend more easily.
> > 
> > ==> Thanks for your suggestion very much. I will add more comments here.
> > > 
> > > > if (!ret)
> > > > return 0;
> > > > }
> > > 
> > > 
> > 
> 
> 

Re: [PATCH v1 1/4] ASoC: codecs: pcm179x: Add PCM1789 id

[Mylène Josserand]

Hello,

Thank you for the review.

On Tue, 27 Feb 2018 22:51:40 +0100
Thomas Petazzoni  wrote:

> Hello,
> 
> On Tue, 27 Feb 2018 22:24:30 +0100, Mylène Josserand wrote:
> > To prepare the support for the PCM1789, add a new compatible
> > and use the i2c_id to handle, later, the differences between
> > these two DACs even if they are pretty similar.
> > 
> > Signed-off-by: Mylène Josserand 
> > ---
> >  Documentation/devicetree/bindings/sound/pcm179x.txt | 2 +-  
> 
> The DT binding change should be in a separate patch.
> 
> >  sound/soc/codecs/pcm179x-i2c.c  | 6 --
> >  sound/soc/codecs/pcm179x.c  | 3 ++-
> >  sound/soc/codecs/pcm179x.h  | 8 +++-  
> 
> And this should be together with the PCM1789 support patch. Otherwise
> your series is not bisectable: if we apply only PATCH 1/4, the driver
> supports the new compatible string, but it doesn't have the actual code
> to handle PCM1789. Am I missing something here ?

No, you are right, I will merge it with patch 02.

> 
> > -   return pcm179x_common_init(>dev, regmap);
> > +   return pcm179x_common_init(>dev, regmap, id->driver_data);  
> 
> This can be done in a preparation patch.
> 
> >  }
> >  
> >  static const struct of_device_id pcm179x_of_match[] = {
> > { .compatible = "ti,pcm1792a", },
> > +   { .compatible = "ti,pcm1789", },
> > { }
> >  };
> >  MODULE_DEVICE_TABLE(of, pcm179x_of_match);
> >  
> >  static const struct i2c_device_id pcm179x_i2c_ids[] = {
> > -   { "pcm179x", 0 },
> > +   { "pcm179x", PCM179X },  
> 
> And also this addition.
> 
> > +   { "pcm1789", PCM1789 },
> > { }
> >  };
> >  MODULE_DEVICE_TABLE(i2c, pcm179x_i2c_ids);
> > diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c
> > index 4b311c06f97d..81cbf09319f6 100644
> > --- a/sound/soc/codecs/pcm179x.c
> > +++ b/sound/soc/codecs/pcm179x.c
> > @@ -218,7 +218,8 @@ static const struct snd_soc_component_driver 
> > soc_component_dev_pcm179x = {
> > .non_legacy_dai_naming  = 1,
> >  };
> >  
> > -int pcm179x_common_init(struct device *dev, struct regmap *regmap)
> > +int pcm179x_common_init(struct device *dev, struct regmap *regmap,
> > +   enum pcm17xx_type type)  
> 
> And this done.
> 
> >  {
> > struct pcm179x_private *pcm179x;
> >  
> > diff --git a/sound/soc/codecs/pcm179x.h b/sound/soc/codecs/pcm179x.h
> > index cf8681c9a373..8c08689e3b8b 100644
> > --- a/sound/soc/codecs/pcm179x.h
> > +++ b/sound/soc/codecs/pcm179x.h
> > @@ -17,11 +17,17 @@
> >  #ifndef __PCM179X_H__
> >  #define __PCM179X_H__
> >  
> > +enum pcm17xx_type {
> > +   PCM179X,  
> 
> And this one.
> 
> > +   PCM1789,
> > +};
> > +
> >  #define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | 
> > SNDRV_PCM_FMTBIT_S24_LE | \
> >   SNDRV_PCM_FMTBIT_S16_LE)
> >  
> >  extern const struct regmap_config pcm179x_regmap_config;
> >  
> > -int pcm179x_common_init(struct device *dev, struct regmap *regmap);
> > +int pcm179x_common_init(struct device *dev, struct regmap *regmap,
> > +   enum pcm17xx_type type);  
> 
> And this one. Just as a "preparation patch" to support multiple codecs
> in the existing pcm179x.c driver.
> 
> Thanks!
> 
> Thomas

Thanks,

Mylène

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

Re: [PATCH v1 1/4] ASoC: codecs: pcm179x: Add PCM1789 id

[Mylène Josserand]

Hello,

Thank you for the review.

On Tue, 27 Feb 2018 22:51:40 +0100
Thomas Petazzoni  wrote:

> Hello,
> 
> On Tue, 27 Feb 2018 22:24:30 +0100, Mylène Josserand wrote:
> > To prepare the support for the PCM1789, add a new compatible
> > and use the i2c_id to handle, later, the differences between
> > these two DACs even if they are pretty similar.
> > 
> > Signed-off-by: Mylène Josserand 
> > ---
> >  Documentation/devicetree/bindings/sound/pcm179x.txt | 2 +-  
> 
> The DT binding change should be in a separate patch.
> 
> >  sound/soc/codecs/pcm179x-i2c.c  | 6 --
> >  sound/soc/codecs/pcm179x.c  | 3 ++-
> >  sound/soc/codecs/pcm179x.h  | 8 +++-  
> 
> And this should be together with the PCM1789 support patch. Otherwise
> your series is not bisectable: if we apply only PATCH 1/4, the driver
> supports the new compatible string, but it doesn't have the actual code
> to handle PCM1789. Am I missing something here ?

No, you are right, I will merge it with patch 02.

> 
> > -   return pcm179x_common_init(>dev, regmap);
> > +   return pcm179x_common_init(>dev, regmap, id->driver_data);  
> 
> This can be done in a preparation patch.
> 
> >  }
> >  
> >  static const struct of_device_id pcm179x_of_match[] = {
> > { .compatible = "ti,pcm1792a", },
> > +   { .compatible = "ti,pcm1789", },
> > { }
> >  };
> >  MODULE_DEVICE_TABLE(of, pcm179x_of_match);
> >  
> >  static const struct i2c_device_id pcm179x_i2c_ids[] = {
> > -   { "pcm179x", 0 },
> > +   { "pcm179x", PCM179X },  
> 
> And also this addition.
> 
> > +   { "pcm1789", PCM1789 },
> > { }
> >  };
> >  MODULE_DEVICE_TABLE(i2c, pcm179x_i2c_ids);
> > diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c
> > index 4b311c06f97d..81cbf09319f6 100644
> > --- a/sound/soc/codecs/pcm179x.c
> > +++ b/sound/soc/codecs/pcm179x.c
> > @@ -218,7 +218,8 @@ static const struct snd_soc_component_driver 
> > soc_component_dev_pcm179x = {
> > .non_legacy_dai_naming  = 1,
> >  };
> >  
> > -int pcm179x_common_init(struct device *dev, struct regmap *regmap)
> > +int pcm179x_common_init(struct device *dev, struct regmap *regmap,
> > +   enum pcm17xx_type type)  
> 
> And this done.
> 
> >  {
> > struct pcm179x_private *pcm179x;
> >  
> > diff --git a/sound/soc/codecs/pcm179x.h b/sound/soc/codecs/pcm179x.h
> > index cf8681c9a373..8c08689e3b8b 100644
> > --- a/sound/soc/codecs/pcm179x.h
> > +++ b/sound/soc/codecs/pcm179x.h
> > @@ -17,11 +17,17 @@
> >  #ifndef __PCM179X_H__
> >  #define __PCM179X_H__
> >  
> > +enum pcm17xx_type {
> > +   PCM179X,  
> 
> And this one.
> 
> > +   PCM1789,
> > +};
> > +
> >  #define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | 
> > SNDRV_PCM_FMTBIT_S24_LE | \
> >   SNDRV_PCM_FMTBIT_S16_LE)
> >  
> >  extern const struct regmap_config pcm179x_regmap_config;
> >  
> > -int pcm179x_common_init(struct device *dev, struct regmap *regmap);
> > +int pcm179x_common_init(struct device *dev, struct regmap *regmap,
> > +   enum pcm17xx_type type);  
> 
> And this one. Just as a "preparation patch" to support multiple codecs
> in the existing pcm179x.c driver.
> 
> Thanks!
> 
> Thomas

Thanks,

Mylène

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

Re: [RFC PATCH v4 6/7] xen/pvh: Add memory map pointer to hvm_start_info struct

[Jan Beulich]

 Juergen Gross  03/01/18 8:29 AM >>>
>On 28/02/18 19:28, Maran Wilson wrote:
>> The start info structure that is defined as part of the x86/HVM direct boot
>> ABI and used for starting Xen PVH guests would be more versatile if it also
>> included a way to pass information about the memory map to the guest. This
>> would allow KVM guests to share the same entry point.
>> 
>> Signed-off-by: Maran Wilson 
>
>I'm fine with this, but we need this change being accepted by the Xen
>community first. So an Ack from Jan or Andrew is required as the same
>change should be done on Xen side.

And for an ack to be given I continue to demand that a patch be sent against
the Xen tree. That said, the change looks fine to me now (as indicated before).

Jan

Re: [RFC PATCH v4 6/7] xen/pvh: Add memory map pointer to hvm_start_info struct

[Jan Beulich]

 Juergen Gross  03/01/18 8:29 AM >>>
>On 28/02/18 19:28, Maran Wilson wrote:
>> The start info structure that is defined as part of the x86/HVM direct boot
>> ABI and used for starting Xen PVH guests would be more versatile if it also
>> included a way to pass information about the memory map to the guest. This
>> would allow KVM guests to share the same entry point.
>> 
>> Signed-off-by: Maran Wilson 
>
>I'm fine with this, but we need this change being accepted by the Xen
>community first. So an Ack from Jan or Andrew is required as the same
>change should be done on Xen side.

And for an ack to be given I continue to demand that a patch be sent against
the Xen tree. That said, the change looks fine to me now (as indicated before).

Jan

Re: [PATCH v2] Input: gpio_keys: Add level trigger support for GPIO keys

[Baolin Wang]

On 28 February 2018 at 20:53, Linus Walleij  wrote:
> On Mon, Feb 26, 2018 at 7:24 AM, Baolin Wang  wrote:
>> On 21 February 2018 at 19:35, Baolin Wang  wrote:
>>> On 20 February 2018 at 02:11, Rob Herring  wrote:
>
> diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt 
> b/Documentation/devicetree/bindings/input/gpio-keys.txt
> index a949404..e3104bd 100644
> --- a/Documentation/devicetree/bindings/input/gpio-keys.txt
> +++ b/Documentation/devicetree/bindings/input/gpio-keys.txt
> @@ -29,6 +29,8 @@ Optional subnode-properties:
>   - linux,can-disable: Boolean, indicates that button is connected
> to dedicated (not shared) interrupt which can be disabled to
> suppress events from the button.
> + - gpio-key,level-trigger: Boolean, indicates that button's interrupt
> +   type is level trigger. Otherwise it is edge trigger as default.

 No. Just use 'interrupts' instead of 'gpios' and specify the trigger
 type. Or put both if you need to read the state.
>>>
>>> Okay, so something as below to get the level type from the
>>> 'interrupts' property.
>>> if (fwnode_property_read_u32(child, "interrupts", >level_type))
>>> button->level_type = IRQ_TYPE_NONE;
>>
>> After more thinking, if we use 'interrupts' to indicate the irq type
>> for this case, we cannot specify the irq number due to the irq number
>> should be get by gpiod_to_irq(). So the device nodes look weird, since
>> we should define the index of the interrupt controller instead of the
>> irq type if the #interrupt_cells is set to 1 according to the
>> interrupt controller documentation. What do you think about this?
>
> I think what you're ultimately seeing is a bad fit between this
> GPIO/irq-controller and the Linux gpio keys driver, it doesn't
> have very much to do with the device tree bindings.
>
> What I think is appropriate is to try to create a new input driver
> in Linux that just takes an interrupt, not a GPIO, and let the
> GPIO chip only act as an irqchip for this.
>
> This avoid the complicated step of converting a GPIO to an
> interrupt in order to use it, when all you really want to
> do is use an interrupt, you don't really care about the
> GPIO here, correct?

Sometimes we should set the GPIO debounce, and read GPIO value to
report the event, so I think we can not remove GPIO.

>
> So we would create
> drivers/input/keyboard/interrupt_keys.c
> however I suspect a bunch of code would need to be shared
> with gpio_keys.c so maybe it is necessary to break out the
> parts of gpio_keys.c into its own helper file.
>
> Or maybe even have the
> pure interrupt handling as part of gpio_keys.c, i.e. if the
> driver can't find any associated GPIO, it goes on to
> check if there is an interrupt assigned to the device node
> and use that directly instead.
>
> Either way, Dmitry must be involved.
>
> Yours,
> Linus Walleij



-- 
Baolin.wang
Best Regards

Re: [PATCH v2] Input: gpio_keys: Add level trigger support for GPIO keys

[Baolin Wang]

On 28 February 2018 at 20:53, Linus Walleij  wrote:
> On Mon, Feb 26, 2018 at 7:24 AM, Baolin Wang  wrote:
>> On 21 February 2018 at 19:35, Baolin Wang  wrote:
>>> On 20 February 2018 at 02:11, Rob Herring  wrote:
>
> diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt 
> b/Documentation/devicetree/bindings/input/gpio-keys.txt
> index a949404..e3104bd 100644
> --- a/Documentation/devicetree/bindings/input/gpio-keys.txt
> +++ b/Documentation/devicetree/bindings/input/gpio-keys.txt
> @@ -29,6 +29,8 @@ Optional subnode-properties:
>   - linux,can-disable: Boolean, indicates that button is connected
> to dedicated (not shared) interrupt which can be disabled to
> suppress events from the button.
> + - gpio-key,level-trigger: Boolean, indicates that button's interrupt
> +   type is level trigger. Otherwise it is edge trigger as default.

 No. Just use 'interrupts' instead of 'gpios' and specify the trigger
 type. Or put both if you need to read the state.
>>>
>>> Okay, so something as below to get the level type from the
>>> 'interrupts' property.
>>> if (fwnode_property_read_u32(child, "interrupts", >level_type))
>>> button->level_type = IRQ_TYPE_NONE;
>>
>> After more thinking, if we use 'interrupts' to indicate the irq type
>> for this case, we cannot specify the irq number due to the irq number
>> should be get by gpiod_to_irq(). So the device nodes look weird, since
>> we should define the index of the interrupt controller instead of the
>> irq type if the #interrupt_cells is set to 1 according to the
>> interrupt controller documentation. What do you think about this?
>
> I think what you're ultimately seeing is a bad fit between this
> GPIO/irq-controller and the Linux gpio keys driver, it doesn't
> have very much to do with the device tree bindings.
>
> What I think is appropriate is to try to create a new input driver
> in Linux that just takes an interrupt, not a GPIO, and let the
> GPIO chip only act as an irqchip for this.
>
> This avoid the complicated step of converting a GPIO to an
> interrupt in order to use it, when all you really want to
> do is use an interrupt, you don't really care about the
> GPIO here, correct?

Sometimes we should set the GPIO debounce, and read GPIO value to
report the event, so I think we can not remove GPIO.

>
> So we would create
> drivers/input/keyboard/interrupt_keys.c
> however I suspect a bunch of code would need to be shared
> with gpio_keys.c so maybe it is necessary to break out the
> parts of gpio_keys.c into its own helper file.
>
> Or maybe even have the
> pure interrupt handling as part of gpio_keys.c, i.e. if the
> driver can't find any associated GPIO, it goes on to
> check if there is an interrupt assigned to the device node
> and use that directly instead.
>
> Either way, Dmitry must be involved.
>
> Yours,
> Linus Walleij



-- 
Baolin.wang
Best Regards

Re: [PATCH] clk: tegra: fix pllu rate configuration

[Peter De Schrijver]

On Wed, Feb 28, 2018 at 08:20:47PM +0300, Dmitry Osipenko wrote:
> On 28.02.2018 17:14, Peter De Schrijver wrote:
> > On Wed, Feb 28, 2018 at 03:00:23PM +0300, Dmitry Osipenko wrote:
> >> On 28.02.2018 12:36, Peter De Schrijver wrote:
> >>> On Tue, Feb 27, 2018 at 02:59:11PM +0300, Dmitry Osipenko wrote:
>  On 27.02.2018 02:04, Marcel Ziswiler wrote:
> > On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
> >> On 23.02.2018 02:04, Marcel Ziswiler wrote:
> >>> Turns out latest upstream U-Boot does not configure/enable pllu
> >>> which
> >>> leaves it at some default rate of 500 kHz:
> >>>
> >>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
> >>> pll_u
> >>>pll_u  330  50  
> >>> 0
> >>>
> >>> Of course this won't quite work leading to the following messages:
> >>>
> >>> [6.559593] usb 2-1: new full-speed USB device number 2 using
> >>> tegra-
> >>> ehci
> >>> [   11.759173] usb 2-1: device descriptor read/64, error -110
> >>> [   27.119453] usb 2-1: device descriptor read/64, error -110
> >>> [   27.389217] usb 2-1: new full-speed USB device number 3 using
> >>> tegra-
> >>> ehci
> >>> [   32.559454] usb 2-1: device descriptor read/64, error -110
> >>> [   47.929777] usb 2-1: device descriptor read/64, error -110
> >>> [   48.049658] usb usb2-port1: attempt power cycle
> >>> [   48.759475] usb 2-1: new full-speed USB device number 4 using
> >>> tegra-
> >>> ehci
> >>> [   59.349457] usb 2-1: device not accepting address 4, error -110
> >>> [   59.509449] usb 2-1: new full-speed USB device number 5 using
> >>> tegra-
> >>> ehci
> >>> [   70.069457] usb 2-1: device not accepting address 5, error -110
> >>> [   70.079721] usb usb2-port1: unable to enumerate USB device
> >>>
> >>> Fix this by actually allowing the rate also being set from within
> >>> the Linux kernel.
> >>>
> >>> I think the best solution to this problem would be to make pll_u a fixed
> >>> clock and enable it and program the rate if it's not enabled at boot.
> >>
> >> Oh, right. PLL_U rate is actually configurable, somehow I missed it in TRM
> >> yesterday.. So set/round_rate() for PLL_U are actually needed and the 
> >> patch is
> >> correct. Seems only T20 misses PLL_U in the init table, probably worth to 
> >> add it
> >> there.
> >>
> > 
> > AFAIK we only use one rate ever?
> 
> IIUC, PLL_U has 3 outputs and output dividers are fixed in HW. So yes, we are
> setting PLL_U to one rate - 480MHz to get out1-480MHz, out2-60MHz and 
> out3-12MHz.
> 

Indeed. And given that it's hw controlled anyway, I don't see why we can't make
it a fixed clock and handle the init at kernel boot depending on what the
bootloader has done.

Peter.

Re: [PATCH] clk: tegra: fix pllu rate configuration

[Peter De Schrijver]

On Wed, Feb 28, 2018 at 08:20:47PM +0300, Dmitry Osipenko wrote:
> On 28.02.2018 17:14, Peter De Schrijver wrote:
> > On Wed, Feb 28, 2018 at 03:00:23PM +0300, Dmitry Osipenko wrote:
> >> On 28.02.2018 12:36, Peter De Schrijver wrote:
> >>> On Tue, Feb 27, 2018 at 02:59:11PM +0300, Dmitry Osipenko wrote:
>  On 27.02.2018 02:04, Marcel Ziswiler wrote:
> > On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
> >> On 23.02.2018 02:04, Marcel Ziswiler wrote:
> >>> Turns out latest upstream U-Boot does not configure/enable pllu
> >>> which
> >>> leaves it at some default rate of 500 kHz:
> >>>
> >>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
> >>> pll_u
> >>>pll_u  330  50  
> >>> 0
> >>>
> >>> Of course this won't quite work leading to the following messages:
> >>>
> >>> [6.559593] usb 2-1: new full-speed USB device number 2 using
> >>> tegra-
> >>> ehci
> >>> [   11.759173] usb 2-1: device descriptor read/64, error -110
> >>> [   27.119453] usb 2-1: device descriptor read/64, error -110
> >>> [   27.389217] usb 2-1: new full-speed USB device number 3 using
> >>> tegra-
> >>> ehci
> >>> [   32.559454] usb 2-1: device descriptor read/64, error -110
> >>> [   47.929777] usb 2-1: device descriptor read/64, error -110
> >>> [   48.049658] usb usb2-port1: attempt power cycle
> >>> [   48.759475] usb 2-1: new full-speed USB device number 4 using
> >>> tegra-
> >>> ehci
> >>> [   59.349457] usb 2-1: device not accepting address 4, error -110
> >>> [   59.509449] usb 2-1: new full-speed USB device number 5 using
> >>> tegra-
> >>> ehci
> >>> [   70.069457] usb 2-1: device not accepting address 5, error -110
> >>> [   70.079721] usb usb2-port1: unable to enumerate USB device
> >>>
> >>> Fix this by actually allowing the rate also being set from within
> >>> the Linux kernel.
> >>>
> >>> I think the best solution to this problem would be to make pll_u a fixed
> >>> clock and enable it and program the rate if it's not enabled at boot.
> >>
> >> Oh, right. PLL_U rate is actually configurable, somehow I missed it in TRM
> >> yesterday.. So set/round_rate() for PLL_U are actually needed and the 
> >> patch is
> >> correct. Seems only T20 misses PLL_U in the init table, probably worth to 
> >> add it
> >> there.
> >>
> > 
> > AFAIK we only use one rate ever?
> 
> IIUC, PLL_U has 3 outputs and output dividers are fixed in HW. So yes, we are
> setting PLL_U to one rate - 480MHz to get out1-480MHz, out2-60MHz and 
> out3-12MHz.
> 

Indeed. And given that it's hw controlled anyway, I don't see why we can't make
it a fixed clock and handle the init at kernel boot depending on what the
bootloader has done.

Peter.

[PATCH v4] Input: gpio_keys: Add level trigger support for GPIO keys

[Baolin Wang]

On some platforms (such as Spreadtrum platform), the GPIO keys can only
be triggered by level type. So this patch introduces one trigger_type to
indicate if the button's interrupt type is level trigger or edge trigger.

Signed-off-by: Baolin Wang 
---
Changes since v3:
 - Remove the reserse level logic into gpio irqchip.
 - Add one trigger_type to indicate teh button's trigger type.

Changes since v2:
 - Use 'interrupt' property to indicate the irq type.

Changes since v1:
 - Diable the GPIO irq until reversing the GPIO level type.
---
 drivers/input/keyboard/gpio_keys.c |   12 ++--
 include/linux/gpio_keys.h  |2 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c 
b/drivers/input/keyboard/gpio_keys.c
index 87e613d..614ee29 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -566,7 +566,10 @@ static int gpio_keys_setup_key(struct platform_device 
*pdev,
INIT_DELAYED_WORK(>work, gpio_keys_gpio_work_func);
 
isr = gpio_keys_gpio_isr;
-   irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
+   if (button->trigger_type)
+   irqflags = button->trigger_type;
+   else
+   irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
 
} else {
if (!button->irq) {
@@ -696,10 +699,15 @@ static void gpio_keys_close(struct input_dev *input)
device_property_read_string(dev, "label", >name);
 
device_for_each_child_node(dev, child) {
-   if (is_of_node(child))
+   if (is_of_node(child)) {
button->irq =
irq_of_parse_and_map(to_of_node(child), 0);
 
+   if (button->irq)
+   button->trigger_type =
+   irq_get_trigger_type(button->irq);
+   }
+
if (fwnode_property_read_u32(child, "linux,code",
 >code)) {
dev_err(dev, "Button without keycode\n");
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index d06bf77..eae87e2 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -18,6 +18,7 @@
  * disable button via sysfs
  * @value: axis value for %EV_ABS
  * @irq:   Irq number in case of interrupt keys
+ * @trigger_type:  indicate the button's interrupt type
  */
 struct gpio_keys_button {
unsigned int code;
@@ -30,6 +31,7 @@ struct gpio_keys_button {
bool can_disable;
int value;
unsigned int irq;
+   unsigned int trigger_type;
 };
 
 /**
-- 
1.7.9.5

[PATCH v4] Input: gpio_keys: Add level trigger support for GPIO keys

[Baolin Wang]

On some platforms (such as Spreadtrum platform), the GPIO keys can only
be triggered by level type. So this patch introduces one trigger_type to
indicate if the button's interrupt type is level trigger or edge trigger.

Signed-off-by: Baolin Wang 
---
Changes since v3:
 - Remove the reserse level logic into gpio irqchip.
 - Add one trigger_type to indicate teh button's trigger type.

Changes since v2:
 - Use 'interrupt' property to indicate the irq type.

Changes since v1:
 - Diable the GPIO irq until reversing the GPIO level type.
---
 drivers/input/keyboard/gpio_keys.c |   12 ++--
 include/linux/gpio_keys.h  |2 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c 
b/drivers/input/keyboard/gpio_keys.c
index 87e613d..614ee29 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -566,7 +566,10 @@ static int gpio_keys_setup_key(struct platform_device 
*pdev,
INIT_DELAYED_WORK(>work, gpio_keys_gpio_work_func);
 
isr = gpio_keys_gpio_isr;
-   irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
+   if (button->trigger_type)
+   irqflags = button->trigger_type;
+   else
+   irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
 
} else {
if (!button->irq) {
@@ -696,10 +699,15 @@ static void gpio_keys_close(struct input_dev *input)
device_property_read_string(dev, "label", >name);
 
device_for_each_child_node(dev, child) {
-   if (is_of_node(child))
+   if (is_of_node(child)) {
button->irq =
irq_of_parse_and_map(to_of_node(child), 0);
 
+   if (button->irq)
+   button->trigger_type =
+   irq_get_trigger_type(button->irq);
+   }
+
if (fwnode_property_read_u32(child, "linux,code",
 >code)) {
dev_err(dev, "Button without keycode\n");
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index d06bf77..eae87e2 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -18,6 +18,7 @@
  * disable button via sysfs
  * @value: axis value for %EV_ABS
  * @irq:   Irq number in case of interrupt keys
+ * @trigger_type:  indicate the button's interrupt type
  */
 struct gpio_keys_button {
unsigned int code;
@@ -30,6 +31,7 @@ struct gpio_keys_button {
bool can_disable;
int value;
unsigned int irq;
+   unsigned int trigger_type;
 };
 
 /**
-- 
1.7.9.5

Re: 4.16 regression: s2ram broken on non-PAE i686

[Thomas Gleixner]

On Wed, 28 Feb 2018, Woody Suwalski wrote:
> Thanks for the patch, good news, it did fix the problem.
> I did 2 builds and both worked OK over the s2ram cycle.

Good.

> It will be necessary to add the patch to 4.15-stable and 4.14-stable, I
> believe that both have now broken s2ram.

Right, it's already tagged for stable.

Thanks,

tglx

Re: [PATCH v3] Input: gpio_keys: Add level trigger support for GPIO keys

[Baolin Wang]

On 28 February 2018 at 22:44, Arnd Bergmann  wrote:
> On Wed, Feb 28, 2018 at 1:44 PM, Baolin Wang  wrote:
>> On some platforms (such as Spreadtrum platform), the GPIO keys can only
>> be triggered by level type. So this patch introduces one property to
>> indicate if the GPIO trigger type is level trigger or edge trigger.
>>
>> Signed-off-by: Baolin Wang 
>> ---
>> Changes since v2:
>>  - Use 'interrupt' property to indicate the irq type.
>>
>> Changes since v1:
>>  - Diable the GPIO irq until reversing the GPIO level type.
>
> I've looked at your patch in more detail now, and given it a bit more thought.
>
> I wonder if you could move that logic into your gpiochip/irqchip driver 
> instead.
> It seems that what you do in the gpio-keys driver is to emulate edge triggered
> behavior on a level triggered irqchip.
>
> If you put the same logic into the gpio driver, you could simply make it
> pretend to support an edge trigger on both edges and call into the interrupt
> handler whenever the state changes.
>

That is really a good suggestion, which can avoid duplicate level
reverse logic in different drivers. So this patch can be simplified
just adding one trigger_type to indicate the interrupt type (not
always edge trigger). Thanks for your suggestion.

-- 
Baolin.wang
Best Regards

Re: 4.16 regression: s2ram broken on non-PAE i686

[Thomas Gleixner]

On Wed, 28 Feb 2018, Woody Suwalski wrote:
> Thanks for the patch, good news, it did fix the problem.
> I did 2 builds and both worked OK over the s2ram cycle.

Good.

> It will be necessary to add the patch to 4.15-stable and 4.14-stable, I
> believe that both have now broken s2ram.

Right, it's already tagged for stable.

Thanks,

tglx

Re: [PATCH v3] Input: gpio_keys: Add level trigger support for GPIO keys

[Baolin Wang]

On 28 February 2018 at 22:44, Arnd Bergmann  wrote:
> On Wed, Feb 28, 2018 at 1:44 PM, Baolin Wang  wrote:
>> On some platforms (such as Spreadtrum platform), the GPIO keys can only
>> be triggered by level type. So this patch introduces one property to
>> indicate if the GPIO trigger type is level trigger or edge trigger.
>>
>> Signed-off-by: Baolin Wang 
>> ---
>> Changes since v2:
>>  - Use 'interrupt' property to indicate the irq type.
>>
>> Changes since v1:
>>  - Diable the GPIO irq until reversing the GPIO level type.
>
> I've looked at your patch in more detail now, and given it a bit more thought.
>
> I wonder if you could move that logic into your gpiochip/irqchip driver 
> instead.
> It seems that what you do in the gpio-keys driver is to emulate edge triggered
> behavior on a level triggered irqchip.
>
> If you put the same logic into the gpio driver, you could simply make it
> pretend to support an edge trigger on both edges and call into the interrupt
> handler whenever the state changes.
>

That is really a good suggestion, which can avoid duplicate level
reverse logic in different drivers. So this patch can be simplified
just adding one trigger_type to indicate the interrupt type (not
always edge trigger). Thanks for your suggestion.

-- 
Baolin.wang
Best Regards

Re: [RFC PATCH v4 7/7] KVM: x86: Allow Qemu/KVM to use PVH entry point

[Juergen Gross]

On 28/02/18 19:28, Maran Wilson wrote:
> For certain applications it is desirable to rapidly boot a KVM virtual
> machine. In cases where legacy hardware and software support within the
> guest is not needed, Qemu should be able to boot directly into the
> uncompressed Linux kernel binary without the need to run firmware.
> 
> There already exists an ABI to allow this for Xen PVH guests and the ABI
> is supported by Linux and FreeBSD:
> 
>https://xenbits.xen.org/docs/unstable/misc/pvh.html
> 
> This patch enables Qemu to use that same entry point for booting KVM
> guests.
> 
> Signed-off-by: Maran Wilson 

Reviewed-by: Juergen Gross 


Juergen

Re: [RFC PATCH v4 7/7] KVM: x86: Allow Qemu/KVM to use PVH entry point

[Juergen Gross]

On 28/02/18 19:28, Maran Wilson wrote:
> For certain applications it is desirable to rapidly boot a KVM virtual
> machine. In cases where legacy hardware and software support within the
> guest is not needed, Qemu should be able to boot directly into the
> uncompressed Linux kernel binary without the need to run firmware.
> 
> There already exists an ABI to allow this for Xen PVH guests and the ABI
> is supported by Linux and FreeBSD:
> 
>https://xenbits.xen.org/docs/unstable/misc/pvh.html
> 
> This patch enables Qemu to use that same entry point for booting KVM
> guests.
> 
> Signed-off-by: Maran Wilson 

Reviewed-by: Juergen Gross 


Juergen

Re: [RFC PATCH v4 6/7] xen/pvh: Add memory map pointer to hvm_start_info struct

[Juergen Gross]

On 28/02/18 19:28, Maran Wilson wrote:
> The start info structure that is defined as part of the x86/HVM direct boot
> ABI and used for starting Xen PVH guests would be more versatile if it also
> included a way to pass information about the memory map to the guest. This
> would allow KVM guests to share the same entry point.
> 
> Signed-off-by: Maran Wilson 

I'm fine with this, but we need this change being accepted by the Xen
community first. So an Ack from Jan or Andrew is required as the same
change should be done on Xen side.

Juergen

Re: [RFC PATCH v4 6/7] xen/pvh: Add memory map pointer to hvm_start_info struct

[Juergen Gross]

On 28/02/18 19:28, Maran Wilson wrote:
> The start info structure that is defined as part of the x86/HVM direct boot
> ABI and used for starting Xen PVH guests would be more versatile if it also
> included a way to pass information about the memory map to the guest. This
> would allow KVM guests to share the same entry point.
> 
> Signed-off-by: Maran Wilson 

I'm fine with this, but we need this change being accepted by the Xen
community first. So an Ack from Jan or Andrew is required as the same
change should be done on Xen side.

Juergen

Re: [PATCH v5 01/12] drivers: base: Unified device connection lookup

[Heikki Krogerus]

Hi,

On Thu, Mar 01, 2018 at 12:56:57AM +, Jun Li wrote:
> > +struct device *device_find_connection(struct device *dev, const char
> > +*con_id) {
> > +   return __device_find_connection(dev, con_id, generic_match, NULL); }
> 
> -   return __device_find_connection(dev, con_id, generic_match, NULL);
> +   return __device_find_connection(dev, con_id, NULL, generic_match);

Good catch!

Thanks,

-- 
heikki

Re: [PATCH v5 01/12] drivers: base: Unified device connection lookup

[Heikki Krogerus]

Hi,

On Thu, Mar 01, 2018 at 12:56:57AM +, Jun Li wrote:
> > +struct device *device_find_connection(struct device *dev, const char
> > +*con_id) {
> > +   return __device_find_connection(dev, con_id, generic_match, NULL); }
> 
> -   return __device_find_connection(dev, con_id, generic_match, NULL);
> +   return __device_find_connection(dev, con_id, NULL, generic_match);

Good catch!

Thanks,

-- 
heikki

Re: [PATCH v5 08/12] xhci: Add Intel extended cap / otg phy mux handling

[Heikki Krogerus]

On Wed, Feb 28, 2018 at 04:42:32PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 28-02-18 16:15, Heikki Krogerus wrote:
> > On Wed, Feb 28, 2018 at 04:07:45PM +0100, Hans de Goede wrote:
> > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> > > index 96099a245c69..5917e3095e2a 100644
> > > --- a/drivers/usb/host/xhci.h
> > > +++ b/drivers/usb/host/xhci.h
> > > @@ -1825,6 +1825,7 @@ struct xhci_hcd {
> > >   /* Reserved. It was XHCI_U2_DISABLE_WAKE */
> > >   #define XHCI_ASMEDIA_MODIFY_FLOWCONTROL (1 << 28)
> > >   #define XHCI_HW_LPM_DISABLE (1 << 29)
> > > +#define XHCI_INTEL_USB_ROLE_SW   (1 << 30)
> > 
> > Did you rebased these on tope of the latest usb-next?
> 
> No I did not expect that to be necessary, but I see now that it is.
> I've just done a rebase locally, any other remarks before I send
> out a v6?

Nothing from me.


Thanks,

-- 
heikki

Re: [PATCH v5 08/12] xhci: Add Intel extended cap / otg phy mux handling

[Heikki Krogerus]

On Wed, Feb 28, 2018 at 04:42:32PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 28-02-18 16:15, Heikki Krogerus wrote:
> > On Wed, Feb 28, 2018 at 04:07:45PM +0100, Hans de Goede wrote:
> > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> > > index 96099a245c69..5917e3095e2a 100644
> > > --- a/drivers/usb/host/xhci.h
> > > +++ b/drivers/usb/host/xhci.h
> > > @@ -1825,6 +1825,7 @@ struct xhci_hcd {
> > >   /* Reserved. It was XHCI_U2_DISABLE_WAKE */
> > >   #define XHCI_ASMEDIA_MODIFY_FLOWCONTROL (1 << 28)
> > >   #define XHCI_HW_LPM_DISABLE (1 << 29)
> > > +#define XHCI_INTEL_USB_ROLE_SW   (1 << 30)
> > 
> > Did you rebased these on tope of the latest usb-next?
> 
> No I did not expect that to be necessary, but I see now that it is.
> I've just done a rebase locally, any other remarks before I send
> out a v6?

Nothing from me.


Thanks,

-- 
heikki

[PATCH v3 2/2] phy: add inno-usb2-phy driver for hi3798cv200 SoC

[Shawn Guo]

From: Pengcheng Li 

It adds inno-usb2-phy driver for hi3798cv200 SoC USB 2.0 support.  One
inno-usb2-phy device can support up to two PHY ports.  While there is
device level reference clock and power reset to be controlled, each PHY
port has its own utmi reset that needs to assert/de-assert as needed.

Hi3798cv200 needs to access PHY port0 register via particular peripheral
syscon controller register to control PHY, like turning on PHY clock.

Signed-off-by: Pengcheng Li 
Signed-off-by: Jiancheng Xue 
Signed-off-by: Shawn Guo 
---
 drivers/phy/hisilicon/Kconfig  |  10 ++
 drivers/phy/hisilicon/Makefile |   1 +
 drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 218 +
 3 files changed, 229 insertions(+)
 create mode 100644 drivers/phy/hisilicon/phy-hisi-inno-usb2.c

diff --git a/drivers/phy/hisilicon/Kconfig b/drivers/phy/hisilicon/Kconfig
index 6164c4cd0f65..c21470eb7fba 100644
--- a/drivers/phy/hisilicon/Kconfig
+++ b/drivers/phy/hisilicon/Kconfig
@@ -11,6 +11,16 @@ config PHY_HI6220_USB
 
  To compile this driver as a module, choose M here.
 
+config PHY_HISI_INNO_USB2
+   tristate "HiSilicon INNO USB2 PHY support"
+   depends on (ARCH_HISI && ARM64) || COMPILE_TEST
+   select GENERIC_PHY
+   select MFD_SYSCON
+   help
+ Support for INNO USB2 PHY on HiSilicon SoCs. This Phy supports
+ USB 1.5Mb/s, USB 12Mb/s, USB 480Mb/s speeds. It supports one
+ USB host port to accept one USB device.
+
 config PHY_HIX5HD2_SATA
tristate "HIX5HD2 SATA PHY Driver"
depends on ARCH_HIX5HD2 && OF && HAS_IOMEM
diff --git a/drivers/phy/hisilicon/Makefile b/drivers/phy/hisilicon/Makefile
index 541b348187a8..e6c979458d3b 100644
--- a/drivers/phy/hisilicon/Makefile
+++ b/drivers/phy/hisilicon/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_PHY_HI6220_USB)   += phy-hi6220-usb.o
+obj-$(CONFIG_PHY_HISI_INNO_USB2)   += phy-hisi-inno-usb2.o
 obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o
diff --git a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c 
b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
new file mode 100644
index ..4b6b6fd43682
--- /dev/null
+++ b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
@@ -0,0 +1,218 @@
+/*
+ * HiSilicon INNO USB2 PHY Driver.
+ *
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define INNO_PHY_PORT_NUM  2
+#define REF_CLK_STABLE_TIME100 /* unit:us */
+#define UTMI_CLK_STABLE_TIME   200 /* unit:us */
+#define TEST_CLK_STABLE_TIME   2   /* unit:ms */
+#define PHY_CLK_STABLE_TIME2   /* unit:ms */
+#define UTMI_RST_COMPLETE_TIME 2   /* unit:ms */
+#define POR_RST_COMPLETE_TIME  300 /* unit:us */
+#define PHY_TEST_DATA  GENMASK(7, 0)
+#define PHY_TEST_ADDR  GENMASK(15, 8)
+#define PHY_TEST_PORT  GENMASK(18, 16)
+#define PHY_TEST_WREN  BIT(21)
+#define PHY_TEST_CLK   BIT(22) /* rising edge active */
+#define PHY_TEST_RST   BIT(23) /* low active */
+#define PHY_CLK_ENABLE BIT(2)
+
+struct hisi_inno_phy_port {
+   struct phy *phy;
+   struct device *dev;
+   struct reset_control *utmi_rst;
+};
+
+struct hisi_inno_phy_priv {
+   void __iomem *mmio;
+   struct clk *ref_clk;
+   struct reset_control *por_rst;
+   struct hisi_inno_phy_port ports[INNO_PHY_PORT_NUM];
+   u32 port_num;
+};
+
+static void hisi_inno_phy_write_reg(struct hisi_inno_phy_priv *priv,
+   u8 port, u32 addr, u32 data)
+{
+   void __iomem *reg = priv->mmio;
+   u32 val;
+
+   val = (data & PHY_TEST_DATA) |
+ ((addr << 8) & PHY_TEST_ADDR) |
+ ((port << 16) & PHY_TEST_PORT) |
+ PHY_TEST_WREN | PHY_TEST_RST;
+   writel(val, reg);
+
+   val |= PHY_TEST_CLK;
+   writel(val, reg);
+
+   val &= ~PHY_TEST_CLK;
+   writel(val, reg);
+}
+
+static void hisi_inno_phy_setup(struct hisi_inno_phy_priv *priv)
+{
+   /* The phy clk is controlled by the port0 register 0x06. */
+   hisi_inno_phy_write_reg(priv, 0, 0x06, PHY_CLK_ENABLE);
+   

[PATCH v3 1/2] dt-bindings: add bindings doc for HiSilicon INNO USB2 PHY

[Shawn Guo]

From: Pengcheng Li 

It adds device tree bindings document for HiSilicon INNO USB2 PHY.

Signed-off-by: Pengcheng Li 
Signed-off-by: Jiancheng Xue 
Signed-off-by: Shawn Guo 
---
 .../devicetree/bindings/phy/phy-hisi-inno-usb2.txt | 52 ++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt 
b/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt
new file mode 100644
index ..b563cf54ca7b
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt
@@ -0,0 +1,52 @@
+HiSilicon INNO USB2 PHY
+
+Required properties:
+- compatible: Should be one of the following strings:
+   "hisilicon,inno-usb2-phy",
+   "hisilicon,hi3798cv200-usb2-phy".
+- reg: Should be the address space for PHY configuration register in peripheral
+  controller, e.g. PERI_USB0 for USB 2.0 PHY01 on Hi3798cv200 SoC.
+- #phy-cells: Should be 1. The specifier is the index of the PHY port to
+  reference.
+- clocks: The phandle and clock specifier pair for reference clock utmi_refclk.
+- resets: The list of phandle and reset specifier pairs for each reset signal
+  in reset-names.
+- reset-names: Should contain "power_on", "utmi0" and "utmi1". The "utmi1"
+  should exist only if the device has two PHY port.
+
+Refer to phy/phy-bindings.txt for the generic PHY binding properties
+
+Note: the device node should be a child of peripheral controller that contains
+the PHY configuration register.
+
+Example:
+
+perictrl: peripheral-controller@8a2 {
+   compatible = "hisilicon,hi3798cv200-perictrl", "syscon",
+"simple-mfd";
+   reg = <0x8a2 0x1000>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges = <0x0 0x8a2 0x1000>;
+
+   usb2_phy1: phy@120 {
+   compatible = "hisilicon,hi3798cv200-usb2-phy";
+   reg = <0x120 0x4>;
+   #phy-cells = <1>;
+   clocks = < HISTB_USB2_PHY1_REF_CLK>;
+   resets = < 0xbc 4>,
+< 0xbc 8>,
+< 0xbc 9>;
+   reset-names = "power_on", "utmi0", "utmi1";
+   };
+
+   usb2_phy2: phy@124 {
+   compatible = "hisilicon,hi3798cv200-usb2-phy";
+   reg = <0x124 0x4>;
+   #phy-cells = <1>;
+   clocks = < HISTB_USB2_PHY2_REF_CLK>;
+   resets = < 0xbc 6>,
+< 0xbc 10>;
+   reset-names = "power_on", "utmi0";
+   };
+};
-- 
1.9.1

[PATCH v3 1/2] dt-bindings: add bindings doc for HiSilicon INNO USB2 PHY

[Shawn Guo]

From: Pengcheng Li 

It adds device tree bindings document for HiSilicon INNO USB2 PHY.

Signed-off-by: Pengcheng Li 
Signed-off-by: Jiancheng Xue 
Signed-off-by: Shawn Guo 
---
 .../devicetree/bindings/phy/phy-hisi-inno-usb2.txt | 52 ++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt 
b/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt
new file mode 100644
index ..b563cf54ca7b
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt
@@ -0,0 +1,52 @@
+HiSilicon INNO USB2 PHY
+
+Required properties:
+- compatible: Should be one of the following strings:
+   "hisilicon,inno-usb2-phy",
+   "hisilicon,hi3798cv200-usb2-phy".
+- reg: Should be the address space for PHY configuration register in peripheral
+  controller, e.g. PERI_USB0 for USB 2.0 PHY01 on Hi3798cv200 SoC.
+- #phy-cells: Should be 1. The specifier is the index of the PHY port to
+  reference.
+- clocks: The phandle and clock specifier pair for reference clock utmi_refclk.
+- resets: The list of phandle and reset specifier pairs for each reset signal
+  in reset-names.
+- reset-names: Should contain "power_on", "utmi0" and "utmi1". The "utmi1"
+  should exist only if the device has two PHY port.
+
+Refer to phy/phy-bindings.txt for the generic PHY binding properties
+
+Note: the device node should be a child of peripheral controller that contains
+the PHY configuration register.
+
+Example:
+
+perictrl: peripheral-controller@8a2 {
+   compatible = "hisilicon,hi3798cv200-perictrl", "syscon",
+"simple-mfd";
+   reg = <0x8a2 0x1000>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges = <0x0 0x8a2 0x1000>;
+
+   usb2_phy1: phy@120 {
+   compatible = "hisilicon,hi3798cv200-usb2-phy";
+   reg = <0x120 0x4>;
+   #phy-cells = <1>;
+   clocks = < HISTB_USB2_PHY1_REF_CLK>;
+   resets = < 0xbc 4>,
+< 0xbc 8>,
+< 0xbc 9>;
+   reset-names = "power_on", "utmi0", "utmi1";
+   };
+
+   usb2_phy2: phy@124 {
+   compatible = "hisilicon,hi3798cv200-usb2-phy";
+   reg = <0x124 0x4>;
+   #phy-cells = <1>;
+   clocks = < HISTB_USB2_PHY2_REF_CLK>;
+   resets = < 0xbc 6>,
+< 0xbc 10>;
+   reset-names = "power_on", "utmi0";
+   };
+};
-- 
1.9.1

[PATCH v3 2/2] phy: add inno-usb2-phy driver for hi3798cv200 SoC

[Shawn Guo]

From: Pengcheng Li 

It adds inno-usb2-phy driver for hi3798cv200 SoC USB 2.0 support.  One
inno-usb2-phy device can support up to two PHY ports.  While there is
device level reference clock and power reset to be controlled, each PHY
port has its own utmi reset that needs to assert/de-assert as needed.

Hi3798cv200 needs to access PHY port0 register via particular peripheral
syscon controller register to control PHY, like turning on PHY clock.

Signed-off-by: Pengcheng Li 
Signed-off-by: Jiancheng Xue 
Signed-off-by: Shawn Guo 
---
 drivers/phy/hisilicon/Kconfig  |  10 ++
 drivers/phy/hisilicon/Makefile |   1 +
 drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 218 +
 3 files changed, 229 insertions(+)
 create mode 100644 drivers/phy/hisilicon/phy-hisi-inno-usb2.c

diff --git a/drivers/phy/hisilicon/Kconfig b/drivers/phy/hisilicon/Kconfig
index 6164c4cd0f65..c21470eb7fba 100644
--- a/drivers/phy/hisilicon/Kconfig
+++ b/drivers/phy/hisilicon/Kconfig
@@ -11,6 +11,16 @@ config PHY_HI6220_USB
 
  To compile this driver as a module, choose M here.
 
+config PHY_HISI_INNO_USB2
+   tristate "HiSilicon INNO USB2 PHY support"
+   depends on (ARCH_HISI && ARM64) || COMPILE_TEST
+   select GENERIC_PHY
+   select MFD_SYSCON
+   help
+ Support for INNO USB2 PHY on HiSilicon SoCs. This Phy supports
+ USB 1.5Mb/s, USB 12Mb/s, USB 480Mb/s speeds. It supports one
+ USB host port to accept one USB device.
+
 config PHY_HIX5HD2_SATA
tristate "HIX5HD2 SATA PHY Driver"
depends on ARCH_HIX5HD2 && OF && HAS_IOMEM
diff --git a/drivers/phy/hisilicon/Makefile b/drivers/phy/hisilicon/Makefile
index 541b348187a8..e6c979458d3b 100644
--- a/drivers/phy/hisilicon/Makefile
+++ b/drivers/phy/hisilicon/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_PHY_HI6220_USB)   += phy-hi6220-usb.o
+obj-$(CONFIG_PHY_HISI_INNO_USB2)   += phy-hisi-inno-usb2.o
 obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o
diff --git a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c 
b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
new file mode 100644
index ..4b6b6fd43682
--- /dev/null
+++ b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
@@ -0,0 +1,218 @@
+/*
+ * HiSilicon INNO USB2 PHY Driver.
+ *
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define INNO_PHY_PORT_NUM  2
+#define REF_CLK_STABLE_TIME100 /* unit:us */
+#define UTMI_CLK_STABLE_TIME   200 /* unit:us */
+#define TEST_CLK_STABLE_TIME   2   /* unit:ms */
+#define PHY_CLK_STABLE_TIME2   /* unit:ms */
+#define UTMI_RST_COMPLETE_TIME 2   /* unit:ms */
+#define POR_RST_COMPLETE_TIME  300 /* unit:us */
+#define PHY_TEST_DATA  GENMASK(7, 0)
+#define PHY_TEST_ADDR  GENMASK(15, 8)
+#define PHY_TEST_PORT  GENMASK(18, 16)
+#define PHY_TEST_WREN  BIT(21)
+#define PHY_TEST_CLK   BIT(22) /* rising edge active */
+#define PHY_TEST_RST   BIT(23) /* low active */
+#define PHY_CLK_ENABLE BIT(2)
+
+struct hisi_inno_phy_port {
+   struct phy *phy;
+   struct device *dev;
+   struct reset_control *utmi_rst;
+};
+
+struct hisi_inno_phy_priv {
+   void __iomem *mmio;
+   struct clk *ref_clk;
+   struct reset_control *por_rst;
+   struct hisi_inno_phy_port ports[INNO_PHY_PORT_NUM];
+   u32 port_num;
+};
+
+static void hisi_inno_phy_write_reg(struct hisi_inno_phy_priv *priv,
+   u8 port, u32 addr, u32 data)
+{
+   void __iomem *reg = priv->mmio;
+   u32 val;
+
+   val = (data & PHY_TEST_DATA) |
+ ((addr << 8) & PHY_TEST_ADDR) |
+ ((port << 16) & PHY_TEST_PORT) |
+ PHY_TEST_WREN | PHY_TEST_RST;
+   writel(val, reg);
+
+   val |= PHY_TEST_CLK;
+   writel(val, reg);
+
+   val &= ~PHY_TEST_CLK;
+   writel(val, reg);
+}
+
+static void hisi_inno_phy_setup(struct hisi_inno_phy_priv *priv)
+{
+   /* The phy clk is controlled by the port0 register 0x06. */
+   hisi_inno_phy_write_reg(priv, 0, 0x06, PHY_CLK_ENABLE);
+   msleep(PHY_CLK_STABLE_TIME);
+}
+
+static int hisi_inno_phy_init(struct phy *phy)
+{
+   struct 

[PATCH v3 0/2] Add HiSilicon INNO USB2 PHY driver support

[Shawn Guo]

It adds device tree bindings and driver support for HiSilicon INNO USB2
PHY device, which can be found on HiSilicon STB SoC Hi3798cv200.

Changes for v3:
 - Make combphy device be child of peripheral controller and use 'reg'
   property for mapping combphy configuration register.

Changes for v2:
 - Move DT bindings into a separate patch.
 - Rename hisi_inno_phy_start() to hisi_inno_phy_init() for aligning
   name with .init hook.

Pengcheng Li (2):
  dt-bindings: add bindings doc for HiSilicon INNO USB2 PHY
  phy: add inno-usb2-phy driver for hi3798cv200 SoC

 .../devicetree/bindings/phy/phy-hisi-inno-usb2.txt |  52 +
 drivers/phy/hisilicon/Kconfig  |  10 +
 drivers/phy/hisilicon/Makefile |   1 +
 drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 218 +
 4 files changed, 281 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt
 create mode 100644 drivers/phy/hisilicon/phy-hisi-inno-usb2.c

-- 
1.9.1

[PATCH v3 0/2] Add HiSilicon INNO USB2 PHY driver support

[Shawn Guo]

It adds device tree bindings and driver support for HiSilicon INNO USB2
PHY device, which can be found on HiSilicon STB SoC Hi3798cv200.

Changes for v3:
 - Make combphy device be child of peripheral controller and use 'reg'
   property for mapping combphy configuration register.

Changes for v2:
 - Move DT bindings into a separate patch.
 - Rename hisi_inno_phy_start() to hisi_inno_phy_init() for aligning
   name with .init hook.

Pengcheng Li (2):
  dt-bindings: add bindings doc for HiSilicon INNO USB2 PHY
  phy: add inno-usb2-phy driver for hi3798cv200 SoC

 .../devicetree/bindings/phy/phy-hisi-inno-usb2.txt |  52 +
 drivers/phy/hisilicon/Kconfig  |  10 +
 drivers/phy/hisilicon/Makefile |   1 +
 drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 218 +
 4 files changed, 281 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt
 create mode 100644 drivers/phy/hisilicon/phy-hisi-inno-usb2.c

-- 
1.9.1

Re: [RFC PATCH v4 5/7] xen/pvh: Move Xen code for getting mem map via hcall out of common file

[Juergen Gross]

On 28/02/18 19:28, Maran Wilson wrote:
> We need to refactor PVH entry code so that support for other hypervisors
> like Qemu/KVM can be added more easily.
> 
> The original design for PVH entry in Xen guests relies on being able to
> obtain the memory map from the hypervisor using a hypercall. When we
> extend the PVH entry ABI to support other hypervisors like Qemu/KVM,
> a new mechanism will be added that allows the guest to get the memory
> map without needing to use hypercalls.
> 
> For Xen guests, the hypercall approach will still be supported. In
> preparation for adding support for other hypervisors, we can move the
> code that uses hypercalls into the Xen specific file. This will allow us
> to compile kernels in the future without CONFIG_XEN that are still capable
> of being booted as a Qemu/KVM guest via the PVH entry point.
> 
> Signed-off-by: Maran Wilson 

Reviewed-by: Juergen Gross 


Juergen

Re: [RFC PATCH v4 5/7] xen/pvh: Move Xen code for getting mem map via hcall out of common file

[Juergen Gross]

On 28/02/18 19:28, Maran Wilson wrote:
> We need to refactor PVH entry code so that support for other hypervisors
> like Qemu/KVM can be added more easily.
> 
> The original design for PVH entry in Xen guests relies on being able to
> obtain the memory map from the hypervisor using a hypercall. When we
> extend the PVH entry ABI to support other hypervisors like Qemu/KVM,
> a new mechanism will be added that allows the guest to get the memory
> map without needing to use hypercalls.
> 
> For Xen guests, the hypercall approach will still be supported. In
> preparation for adding support for other hypervisors, we can move the
> code that uses hypercalls into the Xen specific file. This will allow us
> to compile kernels in the future without CONFIG_XEN that are still capable
> of being booted as a Qemu/KVM guest via the PVH entry point.
> 
> Signed-off-by: Maran Wilson 

Reviewed-by: Juergen Gross 


Juergen

Re: [RFC PATCH v4 4/7] xen/pvh: Move Xen specific PVH VM initialization out of common code

[Juergen Gross]

On 28/02/18 19:28, Maran Wilson wrote:
> We need to refactor PVH entry code so that support for other hypervisors
> like Qemu/KVM can be added more easily.
> 
> This patch moves the small block of code used for initializing Xen PVH
> virtual machines into the Xen specific file. This initialization is not
> going to be needed for Qemu/KVM guests. Moving it out of the common file
> is going to allow us to compile kernels in the future without CONFIG_XEN
> that are still capable of being booted as a Qemu/KVM guest via the PVH
> entry point.
> 
> Signed-off-by: Maran Wilson 

Reviewed-by: Juergen Gross 


Juergen

Re: [RFC PATCH v4 4/7] xen/pvh: Move Xen specific PVH VM initialization out of common code

[Juergen Gross]

On 28/02/18 19:28, Maran Wilson wrote:
> We need to refactor PVH entry code so that support for other hypervisors
> like Qemu/KVM can be added more easily.
> 
> This patch moves the small block of code used for initializing Xen PVH
> virtual machines into the Xen specific file. This initialization is not
> going to be needed for Qemu/KVM guests. Moving it out of the common file
> is going to allow us to compile kernels in the future without CONFIG_XEN
> that are still capable of being booted as a Qemu/KVM guest via the PVH
> entry point.
> 
> Signed-off-by: Maran Wilson 

Reviewed-by: Juergen Gross 


Juergen

Re: [RFC PATCH v4 1/7] xen/pvh: Split CONFIG_XEN_PVH into CONFIG_PVH and CONFIG_XEN_PVH

[Juergen Gross]

On 28/02/18 19:27, Maran Wilson wrote:
> In order to pave the way for hypervisors other then Xen to use the PVH
> entry point for VMs, we need to factor the PVH entry code into Xen specific
> and hypervisor agnostic components. The first step in doing that, is to
> create a new config option for PVH entry that can be enabled
> independently from CONFIG_XEN.
> 
> Signed-off-by: Maran Wilson 

Reviewed-by: Juergen Gross 


Juergen

Re: [RFC PATCH v4 1/7] xen/pvh: Split CONFIG_XEN_PVH into CONFIG_PVH and CONFIG_XEN_PVH

[Juergen Gross]

On 28/02/18 19:27, Maran Wilson wrote:
> In order to pave the way for hypervisors other then Xen to use the PVH
> entry point for VMs, we need to factor the PVH entry code into Xen specific
> and hypervisor agnostic components. The first step in doing that, is to
> create a new config option for PVH entry that can be enabled
> independently from CONFIG_XEN.
> 
> Signed-off-by: Maran Wilson 

Reviewed-by: Juergen Gross 


Juergen

Re: [PATCH v3 0/4] Split page_type out from mapcount

[Martin Schwidefsky]

On Wed, 28 Feb 2018 14:31:53 -0800
Matthew Wilcox  wrote:

> From: Matthew Wilcox 
> 
> I want to use the _mapcount field to record what a page is in use as.
> This can help with debugging and we can also expose that information to
> userspace through /proc/kpageflags to help diagnose memory usage (not
> included as part of this patch set).
> 
> First, we need s390 to stop using _mapcount for its own purposes;
> Martin, I hope you have time to look at this patch.  I must confess I
> don't quite understand what the different bits are used for in the upper
> nybble of the _mapcount, but I tried to replicate what you were doing
> faithfully.

Yeah, that is a nasty bit of code. On s390 we have 2K page tables (pte)
but 4K pages. If we use full pages for the pte tables we waste 2K of
memory for each of the tables. So we allocate 4K and split it into two
2K pieces. Now we have to keep track of the pieces to be able to free
them again.

I try to give your patch a spin today. It should be stand-alone, no ?

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH v3 0/4] Split page_type out from mapcount

[Martin Schwidefsky]

On Wed, 28 Feb 2018 14:31:53 -0800
Matthew Wilcox  wrote:

> From: Matthew Wilcox 
> 
> I want to use the _mapcount field to record what a page is in use as.
> This can help with debugging and we can also expose that information to
> userspace through /proc/kpageflags to help diagnose memory usage (not
> included as part of this patch set).
> 
> First, we need s390 to stop using _mapcount for its own purposes;
> Martin, I hope you have time to look at this patch.  I must confess I
> don't quite understand what the different bits are used for in the upper
> nybble of the _mapcount, but I tried to replicate what you were doing
> faithfully.

Yeah, that is a nasty bit of code. On s390 we have 2K page tables (pte)
but 4K pages. If we use full pages for the pte tables we waste 2K of
memory for each of the tables. So we allocate 4K and split it into two
2K pieces. Now we have to keep track of the pieces to be able to free
them again.

I try to give your patch a spin today. It should be stand-alone, no ?

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [RFC PATCH v2 00/12] Rewrite asm-generic/bitops/{atomic,lock}.h and use on arm64

[Masahiro Yamada]

2018-02-27 0:04 GMT+09:00 Will Deacon :
> Hi everyone,
>
> This is version two of the RFC I previously posted here:
>
>   https://www.spinics.net/lists/arm-kernel/msg634719.html
>
> Changes since v1 include:
>
>   * Fixed __clear_bit_unlock to work on archs with lock-based atomics
>   * Moved lock ops into bitops/lock.h
>   * Fixed build breakage on lesser-spotted architectures
>
> Trying to fix the circular #includes introduced by pulling atomic.h
> into btops/lock.h has been driving me insane. I've ended up moving some
> basic BIT definitions into bits.h, but this might all be better in
> const.h which is being proposed by Masahiro. Feedback is especially
> welcome on this part.


Info for reviewers:

You can see my patches at the following:

1/5: https://patchwork.kernel.org/patch/10235457/
2/5: https://patchwork.kernel.org/patch/10235461/
3/5: https://patchwork.kernel.org/patch/10235463/
4/5: https://patchwork.kernel.org/patch/10235469/
5/5: https://patchwork.kernel.org/patch/10235471/


5/5 has conflict with Will's 2/12.

Fortunately, it is at the tail of the series.
It is easy to pick/drop/change
when we decide how to organize it.








> I've not bothered optimising for the case of a 64-bit, big-endian
> architecture that uses the generic implementation of atomic64_t because
> it's both messy and hypothetical. The code here should still work
> correctly for that case, it just sucks (as does the implementation
> currently in mainline).
>
> Cheers,
>
> Will
>
> --->8
>
> Will Deacon (12):
>   h8300: Don't include linux/kernel.h in asm/atomic.h
>   m68k: Don't use asm-generic/bitops/lock.h
>   asm-generic: Move some macros from linux/bitops.h to a new bits.h file
>   openrisc: Don't pull in all of linux/bitops.h in asm/cmpxchg.h
>   sh: Don't pull in all of linux/bitops.h in asm/cmpxchg-xchg.h
>   arm64: fpsimd: include  in fpsimd.h
>   arm64: lse: Include compiler_types.h and export.h for out-of-line
> LL/SC
>   arm64: cmpxchg: Include build_bug.h instead of bug.h for BUILD_BUG
>   asm-generic/bitops/atomic.h: Rewrite using atomic_fetch_*
>   asm-generic/bitops/lock.h: Rewrite using atomic_fetch_*
>   arm64: Replace our atomic/lock bitop implementations with asm-generic
>   arm64: bitops: Include 
>
>  arch/arm64/include/asm/bitops.h |  21 +---
>  arch/arm64/include/asm/cmpxchg.h|   2 +-
>  arch/arm64/include/asm/fpsimd.h |   1 +
>  arch/arm64/include/asm/lse.h|   2 +
>  arch/arm64/lib/Makefile |   2 +-
>  arch/arm64/lib/bitops.S |  76 ---
>  arch/h8300/include/asm/atomic.h |   4 +-
>  arch/m68k/include/asm/bitops.h  |   6 +-
>  arch/openrisc/include/asm/cmpxchg.h |   3 +-
>  arch/sh/include/asm/cmpxchg-xchg.h  |   3 +-
>  include/asm-generic/bitops/atomic.h | 188 
> +++-
>  include/asm-generic/bitops/lock.h   |  68 ++---
>  include/asm-generic/bits.h  |  26 +
>  include/linux/bitops.h  |  22 +
>  14 files changed, 135 insertions(+), 289 deletions(-)
>  delete mode 100644 arch/arm64/lib/bitops.S
>  create mode 100644 include/asm-generic/bits.h
>
> --
> 2.1.4
>



-- 
Best Regards
Masahiro Yamada

Re: [RFC PATCH v2 00/12] Rewrite asm-generic/bitops/{atomic,lock}.h and use on arm64

[Masahiro Yamada]

2018-02-27 0:04 GMT+09:00 Will Deacon :
> Hi everyone,
>
> This is version two of the RFC I previously posted here:
>
>   https://www.spinics.net/lists/arm-kernel/msg634719.html
>
> Changes since v1 include:
>
>   * Fixed __clear_bit_unlock to work on archs with lock-based atomics
>   * Moved lock ops into bitops/lock.h
>   * Fixed build breakage on lesser-spotted architectures
>
> Trying to fix the circular #includes introduced by pulling atomic.h
> into btops/lock.h has been driving me insane. I've ended up moving some
> basic BIT definitions into bits.h, but this might all be better in
> const.h which is being proposed by Masahiro. Feedback is especially
> welcome on this part.


Info for reviewers:

You can see my patches at the following:

1/5: https://patchwork.kernel.org/patch/10235457/
2/5: https://patchwork.kernel.org/patch/10235461/
3/5: https://patchwork.kernel.org/patch/10235463/
4/5: https://patchwork.kernel.org/patch/10235469/
5/5: https://patchwork.kernel.org/patch/10235471/


5/5 has conflict with Will's 2/12.

Fortunately, it is at the tail of the series.
It is easy to pick/drop/change
when we decide how to organize it.








> I've not bothered optimising for the case of a 64-bit, big-endian
> architecture that uses the generic implementation of atomic64_t because
> it's both messy and hypothetical. The code here should still work
> correctly for that case, it just sucks (as does the implementation
> currently in mainline).
>
> Cheers,
>
> Will
>
> --->8
>
> Will Deacon (12):
>   h8300: Don't include linux/kernel.h in asm/atomic.h
>   m68k: Don't use asm-generic/bitops/lock.h
>   asm-generic: Move some macros from linux/bitops.h to a new bits.h file
>   openrisc: Don't pull in all of linux/bitops.h in asm/cmpxchg.h
>   sh: Don't pull in all of linux/bitops.h in asm/cmpxchg-xchg.h
>   arm64: fpsimd: include  in fpsimd.h
>   arm64: lse: Include compiler_types.h and export.h for out-of-line
> LL/SC
>   arm64: cmpxchg: Include build_bug.h instead of bug.h for BUILD_BUG
>   asm-generic/bitops/atomic.h: Rewrite using atomic_fetch_*
>   asm-generic/bitops/lock.h: Rewrite using atomic_fetch_*
>   arm64: Replace our atomic/lock bitop implementations with asm-generic
>   arm64: bitops: Include 
>
>  arch/arm64/include/asm/bitops.h |  21 +---
>  arch/arm64/include/asm/cmpxchg.h|   2 +-
>  arch/arm64/include/asm/fpsimd.h |   1 +
>  arch/arm64/include/asm/lse.h|   2 +
>  arch/arm64/lib/Makefile |   2 +-
>  arch/arm64/lib/bitops.S |  76 ---
>  arch/h8300/include/asm/atomic.h |   4 +-
>  arch/m68k/include/asm/bitops.h  |   6 +-
>  arch/openrisc/include/asm/cmpxchg.h |   3 +-
>  arch/sh/include/asm/cmpxchg-xchg.h  |   3 +-
>  include/asm-generic/bitops/atomic.h | 188 
> +++-
>  include/asm-generic/bitops/lock.h   |  68 ++---
>  include/asm-generic/bits.h  |  26 +
>  include/linux/bitops.h  |  22 +
>  14 files changed, 135 insertions(+), 289 deletions(-)
>  delete mode 100644 arch/arm64/lib/bitops.S
>  create mode 100644 include/asm-generic/bits.h
>
> --
> 2.1.4
>



-- 
Best Regards
Masahiro Yamada

Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations

[Christophe LEROY]

Le 28/02/2018 à 07:53, Nicholas Piggin a écrit :

On Tue, 27 Feb 2018 18:11:07 +0530
"Aneesh Kumar K.V"  wrote:


Nicholas Piggin  writes:


On Tue, 27 Feb 2018 14:31:07 +0530
"Aneesh Kumar K.V"  wrote:
  

Christophe Leroy  writes:
   

The number of high slices a process might use now depends on its
address space size, and what allocation address it has requested.

This patch uses that limit throughout call chains where possible,
rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
This saves some cost for processes that don't use very large address
spaces.


I haven't really looked at the final code. One of the issue we had was
with the below scenario.

mmap(addr, len) where addr < 128TB and addr+len > 128TB  We want to make
sure we build the mask such that we don't find the addr available.


We should run it through the mmap regression tests. I *think* we moved
all of that logic from the slice code to get_ummapped_area before going
in to slices. I may have missed something though, it would be good to
have more eyes on it.
  


mmap(-1,...) failed with the test. Something like below fix it

@@ -756,7 +770,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned 
int psize)
 mm->context.low_slices_psize = lpsizes;
  
 hpsizes = mm->context.high_slices_psize;

-   high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+   high_slices = SLICE_NUM_HIGH;
 for (i = 0; i < high_slices; i++) {
 mask_index = i & 0x1;
 index = i >> 1;

I guess for everything in the mm_context_t, we should compute it till
SLICE_NUM_HIGH. The reason for failure was, even though we recompute the
slice mask cached in mm_context on slb_addr_limit, it was still derived
from the high_slices_psizes which was computed with lower value.


Okay thanks for catching that Aneesh. I guess that's a slow path so it
should be okay. Christophe if you're taking care of the series can you
fold it in? Otherwise I'll do that after yours gets merged.



I'm not really taking care of your serie, just took it once to see how 
it fits on the 8xx.
I prefer if you can handle them. If you need my help for any test on 
PPC32 don't hesitate to ask.


Christophe

Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations

[Christophe LEROY]

Le 28/02/2018 à 07:53, Nicholas Piggin a écrit :

On Tue, 27 Feb 2018 18:11:07 +0530
"Aneesh Kumar K.V"  wrote:


Nicholas Piggin  writes:


On Tue, 27 Feb 2018 14:31:07 +0530
"Aneesh Kumar K.V"  wrote:
  

Christophe Leroy  writes:
   

The number of high slices a process might use now depends on its
address space size, and what allocation address it has requested.

This patch uses that limit throughout call chains where possible,
rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
This saves some cost for processes that don't use very large address
spaces.


I haven't really looked at the final code. One of the issue we had was
with the below scenario.

mmap(addr, len) where addr < 128TB and addr+len > 128TB  We want to make
sure we build the mask such that we don't find the addr available.


We should run it through the mmap regression tests. I *think* we moved
all of that logic from the slice code to get_ummapped_area before going
in to slices. I may have missed something though, it would be good to
have more eyes on it.
  


mmap(-1,...) failed with the test. Something like below fix it

@@ -756,7 +770,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned 
int psize)
 mm->context.low_slices_psize = lpsizes;
  
 hpsizes = mm->context.high_slices_psize;

-   high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+   high_slices = SLICE_NUM_HIGH;
 for (i = 0; i < high_slices; i++) {
 mask_index = i & 0x1;
 index = i >> 1;

I guess for everything in the mm_context_t, we should compute it till
SLICE_NUM_HIGH. The reason for failure was, even though we recompute the
slice mask cached in mm_context on slb_addr_limit, it was still derived
from the high_slices_psizes which was computed with lower value.


Okay thanks for catching that Aneesh. I guess that's a slow path so it
should be okay. Christophe if you're taking care of the series can you
fold it in? Otherwise I'll do that after yours gets merged.



I'm not really taking care of your serie, just took it once to see how 
it fits on the 8xx.
I prefer if you can handle them. If you need my help for any test on 
PPC32 don't hesitate to ask.


Christophe

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Mathieu Malaterre]

On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman  wrote:
> Mathieu Malaterre  writes:
>
>> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
>> array feature_properties is defined as an empty array, which in turn
>> triggers the following warning (treated as error on W=1):
>>
>>   CC  arch/powerpc/kernel/prom.o
>> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
>> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression 
>> < 0 is always false [-Werror=type-limits]
>>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> ^
>> cc1: all warnings being treated as errors
>
> Ugh, that's annoying.
>
> This seems to work?
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 4dffef947b8a..5215119e249c 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long 
> node)
>
>  static void __init check_cpu_feature_properties(unsigned long node)
>  {
> -   unsigned long i;
> struct feature_property *fp = feature_properties;
> const __be32 *prop;
> +   int i;
>
> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> +   for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
> prop = of_get_flat_dt_prop(node, fp->name, NULL);
> if (prop && be32_to_cpup(prop) >= fp->min_value) {
> cur_cpu_spec->cpu_features |= fp->cpu_feature;
>

Indeed that looks like the less invasive solution, I'll re-submit.

Should I resubmit the entire patch series (21 indep patches) or
re-submit only the 3 patches that were discussed (as part of a
different series) ?

Thanks

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Mathieu Malaterre]

On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman  wrote:
> Mathieu Malaterre  writes:
>
>> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
>> array feature_properties is defined as an empty array, which in turn
>> triggers the following warning (treated as error on W=1):
>>
>>   CC  arch/powerpc/kernel/prom.o
>> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
>> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression 
>> < 0 is always false [-Werror=type-limits]
>>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> ^
>> cc1: all warnings being treated as errors
>
> Ugh, that's annoying.
>
> This seems to work?
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 4dffef947b8a..5215119e249c 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long 
> node)
>
>  static void __init check_cpu_feature_properties(unsigned long node)
>  {
> -   unsigned long i;
> struct feature_property *fp = feature_properties;
> const __be32 *prop;
> +   int i;
>
> -   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> +   for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
> prop = of_get_flat_dt_prop(node, fp->name, NULL);
> if (prop && be32_to_cpup(prop) >= fp->min_value) {
> cur_cpu_spec->cpu_features |= fp->cpu_feature;
>

Indeed that looks like the less invasive solution, I'll re-submit.

Should I resubmit the entire patch series (21 indep patches) or
re-submit only the 3 patches that were discussed (as part of a
different series) ?

Thanks

Re: [PATCH] sparc: Use generic pci_mmap_resource_range()

[David Woodhouse]

On Wed, 2018-02-28 at 17:08 -0600, Bjorn Helgaas wrote:
> 
> What's the bottom line?  Do we want this for sparc?  If so, do you
> want to take it, Dave M, or would you like me to?

I need to fix it first, and then the intention is for Dave to take it.

There'll be a final patch to remove ARCH_GENERIC_PCI_MMAP_RESOURCE once
it's actually ubiquitous, but I need to do all the "fun" architectures
through their own trees first.

I deliberately gave you only the trivial ones, in the first batch. For
precisely this reason — because I might have missed details.

smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH] sparc: Use generic pci_mmap_resource_range()

[David Woodhouse]

On Wed, 2018-02-28 at 17:08 -0600, Bjorn Helgaas wrote:
> 
> What's the bottom line?  Do we want this for sparc?  If so, do you
> want to take it, Dave M, or would you like me to?

I need to fix it first, and then the intention is for Dave to take it.

There'll be a final patch to remove ARCH_GENERIC_PCI_MMAP_RESOURCE once
it's actually ubiquitous, but I need to do all the "fun" architectures
through their own trees first.

I deliberately gave you only the trivial ones, in the first batch. For
precisely this reason — because I might have missed details.

smime.p7s
Description: S/MIME cryptographic signature

[PATCH v2] ASoC: topology: Fix bclk and fsync inversion in set_link_hw_format()

[Kirill Marinushkin]

The values of bclk and fsync are inverted WRT the codec. But the existing
solution already works for Broadwell, see the alsa-lib config:

`alsa-lib/src/conf/topology/broadwell/broadwell.conf`

This commit provides the backwards-compatible solution to fix this misuse.
This commit goes in pair with the corresponding patch for alsa-lib.

Signed-off-by: Kirill Marinushkin 
Cc: Mark Brown 
Cc: Takashi Iwai 
Cc: Jaroslav Kysela 
Cc: Pierre-Louis Bossart 
Cc: Pan Xiuli 
Cc: Liam Girdwood 
Cc: linux-kernel@vger.kernel.org
Cc: alsa-de...@alsa-project.org
---
 include/uapi/sound/asoc.h | 16 ++--
 sound/soc/soc-topology.c  | 12 +++-
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/include/uapi/sound/asoc.h b/include/uapi/sound/asoc.h
index 69c37ecbff7e..f0e5e21efa54 100644
--- a/include/uapi/sound/asoc.h
+++ b/include/uapi/sound/asoc.h
@@ -160,6 +160,18 @@
 #define SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_SAMPLEBITS(1 << 2)
 #define SND_SOC_TPLG_LNK_FLGBIT_VOICE_WAKEUP(1 << 3)
 
+/* DAI topology BCLK parameter
+ * For the backwards capability, by default codec is bclk master
+ */
+#define SND_SOC_TPLG_BCLK_CM 0 /* codec is bclk master */
+#define SND_SOC_TPLG_BCLK_CS 1 /* codec is bclk slave */
+
+/* DAI topology FSYNC parameter
+ * For the backwards capability, by default codec is fsync master
+ */
+#define SND_SOC_TPLG_FSYNC_CM 0 /* codec is fsync master */
+#define SND_SOC_TPLG_FSYNC_CS 1 /* codec is fsync slave */
+
 /*
  * Block Header.
  * This header precedes all object and object arrays below.
@@ -315,8 +327,8 @@ struct snd_soc_tplg_hw_config {
__u8 clock_gated;   /* 1 if clock can be gated to save power */
__u8 invert_bclk;   /* 1 for inverted BCLK, 0 for normal */
__u8 invert_fsync;  /* 1 for inverted frame clock, 0 for normal */
-   __u8 bclk_master;   /* 1 for master of BCLK, 0 for slave */
-   __u8 fsync_master;  /* 1 for master of FSYNC, 0 for slave */
+   __u8 bclk_master;   /* SND_SOC_TPLG_BCLK_ value */
+   __u8 fsync_master;  /* SND_SOC_TPLG_FSYNC_ value */
__u8 mclk_direction;/* 0 for input, 1 for output */
__le16 reserved;/* for 32bit alignment */
__le32 mclk_rate;   /* MCLK or SYSCLK freqency in Hz */
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 01a50413c66f..c5bdc673b195 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1994,13 +1994,15 @@ static void set_link_hw_format(struct snd_soc_dai_link 
*link,
link->dai_fmt |= SND_SOC_DAIFMT_IB_IF;
 
/* clock masters */
-   bclk_master = hw_config->bclk_master;
-   fsync_master = hw_config->fsync_master;
-   if (!bclk_master && !fsync_master)
+   bclk_master = (hw_config->bclk_master ==
+  SND_SOC_TPLG_BCLK_CM);
+   fsync_master = (hw_config->fsync_master ==
+   SND_SOC_TPLG_FSYNC_CM);
+   if (bclk_master && fsync_master)
link->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
-   else if (bclk_master && !fsync_master)
-   link->dai_fmt |= SND_SOC_DAIFMT_CBS_CFM;
else if (!bclk_master && fsync_master)
+   link->dai_fmt |= SND_SOC_DAIFMT_CBS_CFM;
+   else if (bclk_master && !fsync_master)
link->dai_fmt |= SND_SOC_DAIFMT_CBM_CFS;
else
link->dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
-- 
2.13.6

[PATCH v2] ASoC: topology: Fix bclk and fsync inversion in set_link_hw_format()

[Kirill Marinushkin]

The values of bclk and fsync are inverted WRT the codec. But the existing
solution already works for Broadwell, see the alsa-lib config:

`alsa-lib/src/conf/topology/broadwell/broadwell.conf`

This commit provides the backwards-compatible solution to fix this misuse.
This commit goes in pair with the corresponding patch for alsa-lib.

Signed-off-by: Kirill Marinushkin 
Cc: Mark Brown 
Cc: Takashi Iwai 
Cc: Jaroslav Kysela 
Cc: Pierre-Louis Bossart 
Cc: Pan Xiuli 
Cc: Liam Girdwood 
Cc: linux-kernel@vger.kernel.org
Cc: alsa-de...@alsa-project.org
---
 include/uapi/sound/asoc.h | 16 ++--
 sound/soc/soc-topology.c  | 12 +++-
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/include/uapi/sound/asoc.h b/include/uapi/sound/asoc.h
index 69c37ecbff7e..f0e5e21efa54 100644
--- a/include/uapi/sound/asoc.h
+++ b/include/uapi/sound/asoc.h
@@ -160,6 +160,18 @@
 #define SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_SAMPLEBITS(1 << 2)
 #define SND_SOC_TPLG_LNK_FLGBIT_VOICE_WAKEUP(1 << 3)
 
+/* DAI topology BCLK parameter
+ * For the backwards capability, by default codec is bclk master
+ */
+#define SND_SOC_TPLG_BCLK_CM 0 /* codec is bclk master */
+#define SND_SOC_TPLG_BCLK_CS 1 /* codec is bclk slave */
+
+/* DAI topology FSYNC parameter
+ * For the backwards capability, by default codec is fsync master
+ */
+#define SND_SOC_TPLG_FSYNC_CM 0 /* codec is fsync master */
+#define SND_SOC_TPLG_FSYNC_CS 1 /* codec is fsync slave */
+
 /*
  * Block Header.
  * This header precedes all object and object arrays below.
@@ -315,8 +327,8 @@ struct snd_soc_tplg_hw_config {
__u8 clock_gated;   /* 1 if clock can be gated to save power */
__u8 invert_bclk;   /* 1 for inverted BCLK, 0 for normal */
__u8 invert_fsync;  /* 1 for inverted frame clock, 0 for normal */
-   __u8 bclk_master;   /* 1 for master of BCLK, 0 for slave */
-   __u8 fsync_master;  /* 1 for master of FSYNC, 0 for slave */
+   __u8 bclk_master;   /* SND_SOC_TPLG_BCLK_ value */
+   __u8 fsync_master;  /* SND_SOC_TPLG_FSYNC_ value */
__u8 mclk_direction;/* 0 for input, 1 for output */
__le16 reserved;/* for 32bit alignment */
__le32 mclk_rate;   /* MCLK or SYSCLK freqency in Hz */
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 01a50413c66f..c5bdc673b195 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1994,13 +1994,15 @@ static void set_link_hw_format(struct snd_soc_dai_link 
*link,
link->dai_fmt |= SND_SOC_DAIFMT_IB_IF;
 
/* clock masters */
-   bclk_master = hw_config->bclk_master;
-   fsync_master = hw_config->fsync_master;
-   if (!bclk_master && !fsync_master)
+   bclk_master = (hw_config->bclk_master ==
+  SND_SOC_TPLG_BCLK_CM);
+   fsync_master = (hw_config->fsync_master ==
+   SND_SOC_TPLG_FSYNC_CM);
+   if (bclk_master && fsync_master)
link->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
-   else if (bclk_master && !fsync_master)
-   link->dai_fmt |= SND_SOC_DAIFMT_CBS_CFM;
else if (!bclk_master && fsync_master)
+   link->dai_fmt |= SND_SOC_DAIFMT_CBS_CFM;
+   else if (bclk_master && !fsync_master)
link->dai_fmt |= SND_SOC_DAIFMT_CBM_CFS;
else
link->dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
-- 
2.13.6

Re: [RFC PATCH v2] ptr_ring: linked list fallback

[Jason Wang]

On 2018年02月28日 23:43, Michael S. Tsirkin wrote:

On Wed, Feb 28, 2018 at 10:20:33PM +0800, Jason Wang wrote:


On 2018年02月28日 22:01, Michael S. Tsirkin wrote:

On Wed, Feb 28, 2018 at 02:28:21PM +0800, Jason Wang wrote:

On 2018年02月28日 12:09, Michael S. Tsirkin wrote:

Or we can add plist to a union:


struct sk_buff {
union {
struct {
/* These two members must be first. */
struct sk_buff  *next;
struct sk_buff  *prev;
union {
struct net_device   *dev;
/* Some protocols might use this space to 
store information,
 * while device pointer would be NULL.
 * UDP receive path is one user.
 */
unsigned long   dev_scratch;
};
};
struct rb_node  rbnode; /* used in netem & tcp stack */
+   struct plist plist; /* For use with ptr_ring */
};


This look ok.


For XDP, we need to embed plist in struct xdp_buff too,

Right - that's pretty straightforward, isn't it?

Yes, it's not clear to me this is really needed for XDP consider the lock
contention it brings.

Thanks

The contention is only when the ring overflows into the list though.


Right, but there's usually a mismatch of speed between producer and
consumer. In case of a fast producer, we may get this contention very
frequently.

Thanks

This is not true in my experiments.  In my experiments, ring size of 4k
bytes is enough to see packet drops in single %s of cases.

To you have workloads where rings are full most of the time?

E.g using xdp_redirect to redirect packets from ixgbe to tap. In my test,
ixgeb can produce ~8Mpps. But vhost can only consume ~3.5Mpps.

Then you are better off just using a small ring and dropping
packets early, right?


Yes, so I believe we won't use this for XDP.

Thanks


One other nice side effect of this patch is that instead of dropping
packets quickly it slows down producer to match consumer speeds.

In some case, producer may not want to be slowed down, e.g in devmap which
can redirect packets into several different interfaces.

IOW, it can go either way in theory, we will need to test and see the effect.


Yes.

Thanks

Re: [RFC PATCH v2] ptr_ring: linked list fallback

[Jason Wang]

On 2018年02月28日 23:43, Michael S. Tsirkin wrote:

On Wed, Feb 28, 2018 at 10:20:33PM +0800, Jason Wang wrote:


On 2018年02月28日 22:01, Michael S. Tsirkin wrote:

On Wed, Feb 28, 2018 at 02:28:21PM +0800, Jason Wang wrote:

On 2018年02月28日 12:09, Michael S. Tsirkin wrote:

Or we can add plist to a union:


struct sk_buff {
union {
struct {
/* These two members must be first. */
struct sk_buff  *next;
struct sk_buff  *prev;
union {
struct net_device   *dev;
/* Some protocols might use this space to 
store information,
 * while device pointer would be NULL.
 * UDP receive path is one user.
 */
unsigned long   dev_scratch;
};
};
struct rb_node  rbnode; /* used in netem & tcp stack */
+   struct plist plist; /* For use with ptr_ring */
};


This look ok.


For XDP, we need to embed plist in struct xdp_buff too,

Right - that's pretty straightforward, isn't it?

Yes, it's not clear to me this is really needed for XDP consider the lock
contention it brings.

Thanks

The contention is only when the ring overflows into the list though.


Right, but there's usually a mismatch of speed between producer and
consumer. In case of a fast producer, we may get this contention very
frequently.

Thanks

This is not true in my experiments.  In my experiments, ring size of 4k
bytes is enough to see packet drops in single %s of cases.

To you have workloads where rings are full most of the time?

E.g using xdp_redirect to redirect packets from ixgbe to tap. In my test,
ixgeb can produce ~8Mpps. But vhost can only consume ~3.5Mpps.

Then you are better off just using a small ring and dropping
packets early, right?


Yes, so I believe we won't use this for XDP.

Thanks


One other nice side effect of this patch is that instead of dropping
packets quickly it slows down producer to match consumer speeds.

In some case, producer may not want to be slowed down, e.g in devmap which
can redirect packets into several different interfaces.

IOW, it can go either way in theory, we will need to test and see the effect.


Yes.

Thanks

Re: [PATCH] e1000e: Fix link status in case of error.

[Benjamin Poirier]

On 2018/02/28 08:48, Alexander Duyck wrote:
> On Tue, Feb 27, 2018 at 9:20 PM, Benjamin Poirier  wrote:
> > Before commit 19110cfbb34d ("e1000e: Separate signaling for link check/link
> > up"), errors which happen after "get_link_status = false" in the copper
> > check_for_link callbacks would be ignored and the link considered up. After
> > that commit, any error implies that the link is down. Since all
> > combinations of link up/down and error/no error are possible, do the same
> > thing as e1000e_phy_has_link_generic() and return the link status in a
> > separate variable.
> >
> > Fixes: 19110cfbb34d ("e1000e: Separate signaling for link check/link up")
> > Signed-off-by: Benjamin Poirier 
> 
> This seems more like a refactor than a fix. There are valid cases
> where errors can be ignored after we set the link to up. For example
> if we cannot negotiate flow control we may not care as long as the
> link is established. In such a case we may see errors establishing
> flow control and they should be ignored.

Indeed, before commit 19110cfbb34d ("e1000e: Separate signaling for link
check/link up") a failure of e1000e_config_fc_after_link_up() in
e1000e_check_for_copper_link() would be ignored. After that commit,
there is an extra 2s delay before link up, like what was just fixed for
autoneg in commit d8ca384786c2 ("e1000e: Fix check_for_link return value
with autoneg off"). That was an unintended change in behavior. This is
what this patch purports to fix. The same is true for other failure
paths in e1000_check_for_copper_link_ich8lan() that happen after
get_link_status = false.

> 
> If there are cases where we are setting the link as up to early we
> should address that instead of changing all the functions to make them
> look like other ones.
> 
> > ---
[...]

Re: [PATCH] e1000e: Fix link status in case of error.

[Benjamin Poirier]

On 2018/02/28 08:48, Alexander Duyck wrote:
> On Tue, Feb 27, 2018 at 9:20 PM, Benjamin Poirier  wrote:
> > Before commit 19110cfbb34d ("e1000e: Separate signaling for link check/link
> > up"), errors which happen after "get_link_status = false" in the copper
> > check_for_link callbacks would be ignored and the link considered up. After
> > that commit, any error implies that the link is down. Since all
> > combinations of link up/down and error/no error are possible, do the same
> > thing as e1000e_phy_has_link_generic() and return the link status in a
> > separate variable.
> >
> > Fixes: 19110cfbb34d ("e1000e: Separate signaling for link check/link up")
> > Signed-off-by: Benjamin Poirier 
> 
> This seems more like a refactor than a fix. There are valid cases
> where errors can be ignored after we set the link to up. For example
> if we cannot negotiate flow control we may not care as long as the
> link is established. In such a case we may see errors establishing
> flow control and they should be ignored.

Indeed, before commit 19110cfbb34d ("e1000e: Separate signaling for link
check/link up") a failure of e1000e_config_fc_after_link_up() in
e1000e_check_for_copper_link() would be ignored. After that commit,
there is an extra 2s delay before link up, like what was just fixed for
autoneg in commit d8ca384786c2 ("e1000e: Fix check_for_link return value
with autoneg off"). That was an unintended change in behavior. This is
what this patch purports to fix. The same is true for other failure
paths in e1000_check_for_copper_link_ich8lan() that happen after
get_link_status = false.

> 
> If there are cases where we are setting the link as up to early we
> should address that instead of changing all the functions to make them
> look like other ones.
> 
> > ---
[...]

Re: [PATCH 3/5] staging: ks7010: Factor out repeated code into function 'ks_wlan_cap()'.

[Tobin C. Harding]

On Wed, Feb 28, 2018 at 09:19:09PM -0800, Quytelda Kahja wrote:
> The code that generates a WLAN capability mask is repeated in five
> functions.  This change refactors that code into a new function, which is
> called now in each of those functions.

Perhaps in future something like:

Code to generate the WLAN capability mask is duplicated five times

Add helper function to generate WLAN capability mask, refactor code to
use newly defined function.


See Documentation/process/submitting-patches.rst section 2 for more info.

2) Describe your changes

Hope this helps,
Tobin.

Re: [PATCH 3/5] staging: ks7010: Factor out repeated code into function 'ks_wlan_cap()'.

[Tobin C. Harding]

On Wed, Feb 28, 2018 at 09:19:09PM -0800, Quytelda Kahja wrote:
> The code that generates a WLAN capability mask is repeated in five
> functions.  This change refactors that code into a new function, which is
> called now in each of those functions.

Perhaps in future something like:

Code to generate the WLAN capability mask is duplicated five times

Add helper function to generate WLAN capability mask, refactor code to
use newly defined function.


See Documentation/process/submitting-patches.rst section 2 for more info.

2) Describe your changes

Hope this helps,
Tobin.

[PATCH v3 2/2] kconfig: rename silentoldconfig to syncconfig

[Masahiro Yamada]

As commit cedd55d49dee ("kconfig: Remove silentoldconfig from help
and docs; fix kconfig/conf's help") mentioned, 'silentoldconfig' is a
historical misnomer.  That commit removed it from help and docs since
it is an internal interface.  If so, it should be allowed to rename
it to something more intuitive.  'syncconfig' is the one I came up
with because it updates the .config if necessary, then synchronize
include/generated/autoconf.h and include/config/* with it.

You should not manually invoke 'silentoldcofig'.  Display warning if
used in case existing scripts are doing wrong.

Signed-off-by: Masahiro Yamada 
---

Changes in v3:
  - Fix Documentation/networking/i40e.txt
  - Display warning if silentoldconfig is used

Changes in v2:
  - newly added

 Documentation/kbuild/kconfig.txt  |  2 +-
 Documentation/networking/i40e.txt |  2 +-
 Makefile  |  2 +-
 scripts/kconfig/Makefile  | 13 ++---
 scripts/kconfig/conf.c| 20 ++--
 5 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/Documentation/kbuild/kconfig.txt b/Documentation/kbuild/kconfig.txt
index bbc99c0..7233118 100644
--- a/Documentation/kbuild/kconfig.txt
+++ b/Documentation/kbuild/kconfig.txt
@@ -119,7 +119,7 @@ Examples:
15% of tristates will be set to 'y', 15% to 'm', 70% to 'n'
 
 __
-Environment variables for 'silentoldconfig'
+Environment variables for 'syncconfig'
 
 KCONFIG_NOSILENTUPDATE
 --
diff --git a/Documentation/networking/i40e.txt 
b/Documentation/networking/i40e.txt
index 57e616e..c2d6e18 100644
--- a/Documentation/networking/i40e.txt
+++ b/Documentation/networking/i40e.txt
@@ -32,7 +32,7 @@ Enabling the driver
 The driver is enabled via the standard kernel configuration system,
 using the make command:
 
- Make oldconfig/silentoldconfig/menuconfig/etc.
+ make config/oldconfig/menuconfig/etc.
 
 The driver is located in the menu structure at:
 
diff --git a/Makefile b/Makefile
index 7f0aba6..4fb97e9 100644
--- a/Makefile
+++ b/Makefile
@@ -597,7 +597,7 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
 # include/generated/ and include/config/. Update them if .config is newer than
 # include/config/auto.conf (which mirrors .config).
 include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
-   $(Q)$(MAKE) -f $(srctree)/Makefile silentoldconfig
+   $(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
 else
 # external modules needs include/generated/autoconf.h and 
include/config/auto.conf
 # but do not care if they are up-to-date. Use auto.conf to trigger the test
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 41e2a9f..753a6de4 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -3,7 +3,7 @@
 # Kernel configuration targets
 # These targets are used from top-level makefile
 
-PHONY += xconfig gconfig menuconfig config silentoldconfig update-po-config \
+PHONY += xconfig gconfig menuconfig config syncconfig update-po-config \
localmodconfig localyesconfig
 
 ifdef KBUILD_KCONFIG
@@ -36,7 +36,7 @@ nconfig: $(obj)/nconf
 
 # This has become an internal implementation detail and is now deprecated
 # for external use.
-silentoldconfig: $(obj)/conf
+syncconfig: $(obj)/conf
$(Q)mkdir -p include/config include/generated
$(Q)test -e include/generated/autoksyms.h || \
touch   include/generated/autoksyms.h
@@ -88,7 +88,7 @@ PHONY += $(simple-targets)
 $(simple-targets): $(obj)/conf
$< $(silent) --$@ $(Kconfig)
 
-PHONY += oldnoconfig savedefconfig defconfig
+PHONY += oldnoconfig silentoldconfig savedefconfig defconfig
 
 # oldnoconfig is an alias of olddefconfig, because people already are dependent
 # on its behavior (sets new symbols to their default value but not 'n') with 
the
@@ -97,6 +97,13 @@ oldnoconfig: olddefconfig
@echo "  WARNING: \"oldnoconfig\" target will be removed after Linux 
4.19"
@echo "Please use \"olddefconfig\" instead, which is an 
alias."
 
+# We do not expect manual invokcation of "silentoldcofig" (or "syncconfig").
+silentoldconfig: syncconfig
+   @echo "  WARNING: \"silentoldconfig\" has been renamed to 
\"syncconfig\""
+   @echo "and is now an internal implementation detail."
+   @echo "What you want is probably \"oldconfig\"."
+   @echo "\"silentoldconfig\" will be removed after Linux 4.19"
+
 savedefconfig: $(obj)/conf
$< $(silent) --$@=defconfig $(Kconfig)
 
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 11a4e45..4e08121 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -23,7 +23,7 @@ static void check_conf(struct menu *menu);
 
 enum input_mode {
oldaskconfig,
-   silentoldconfig,
+   syncconfig,
oldconfig,

[PATCH v3 2/2] kconfig: rename silentoldconfig to syncconfig

[Masahiro Yamada]

As commit cedd55d49dee ("kconfig: Remove silentoldconfig from help
and docs; fix kconfig/conf's help") mentioned, 'silentoldconfig' is a
historical misnomer.  That commit removed it from help and docs since
it is an internal interface.  If so, it should be allowed to rename
it to something more intuitive.  'syncconfig' is the one I came up
with because it updates the .config if necessary, then synchronize
include/generated/autoconf.h and include/config/* with it.

You should not manually invoke 'silentoldcofig'.  Display warning if
used in case existing scripts are doing wrong.

Signed-off-by: Masahiro Yamada 
---

Changes in v3:
  - Fix Documentation/networking/i40e.txt
  - Display warning if silentoldconfig is used

Changes in v2:
  - newly added

 Documentation/kbuild/kconfig.txt  |  2 +-
 Documentation/networking/i40e.txt |  2 +-
 Makefile  |  2 +-
 scripts/kconfig/Makefile  | 13 ++---
 scripts/kconfig/conf.c| 20 ++--
 5 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/Documentation/kbuild/kconfig.txt b/Documentation/kbuild/kconfig.txt
index bbc99c0..7233118 100644
--- a/Documentation/kbuild/kconfig.txt
+++ b/Documentation/kbuild/kconfig.txt
@@ -119,7 +119,7 @@ Examples:
15% of tristates will be set to 'y', 15% to 'm', 70% to 'n'
 
 __
-Environment variables for 'silentoldconfig'
+Environment variables for 'syncconfig'
 
 KCONFIG_NOSILENTUPDATE
 --
diff --git a/Documentation/networking/i40e.txt 
b/Documentation/networking/i40e.txt
index 57e616e..c2d6e18 100644
--- a/Documentation/networking/i40e.txt
+++ b/Documentation/networking/i40e.txt
@@ -32,7 +32,7 @@ Enabling the driver
 The driver is enabled via the standard kernel configuration system,
 using the make command:
 
- Make oldconfig/silentoldconfig/menuconfig/etc.
+ make config/oldconfig/menuconfig/etc.
 
 The driver is located in the menu structure at:
 
diff --git a/Makefile b/Makefile
index 7f0aba6..4fb97e9 100644
--- a/Makefile
+++ b/Makefile
@@ -597,7 +597,7 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
 # include/generated/ and include/config/. Update them if .config is newer than
 # include/config/auto.conf (which mirrors .config).
 include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
-   $(Q)$(MAKE) -f $(srctree)/Makefile silentoldconfig
+   $(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
 else
 # external modules needs include/generated/autoconf.h and 
include/config/auto.conf
 # but do not care if they are up-to-date. Use auto.conf to trigger the test
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 41e2a9f..753a6de4 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -3,7 +3,7 @@
 # Kernel configuration targets
 # These targets are used from top-level makefile
 
-PHONY += xconfig gconfig menuconfig config silentoldconfig update-po-config \
+PHONY += xconfig gconfig menuconfig config syncconfig update-po-config \
localmodconfig localyesconfig
 
 ifdef KBUILD_KCONFIG
@@ -36,7 +36,7 @@ nconfig: $(obj)/nconf
 
 # This has become an internal implementation detail and is now deprecated
 # for external use.
-silentoldconfig: $(obj)/conf
+syncconfig: $(obj)/conf
$(Q)mkdir -p include/config include/generated
$(Q)test -e include/generated/autoksyms.h || \
touch   include/generated/autoksyms.h
@@ -88,7 +88,7 @@ PHONY += $(simple-targets)
 $(simple-targets): $(obj)/conf
$< $(silent) --$@ $(Kconfig)
 
-PHONY += oldnoconfig savedefconfig defconfig
+PHONY += oldnoconfig silentoldconfig savedefconfig defconfig
 
 # oldnoconfig is an alias of olddefconfig, because people already are dependent
 # on its behavior (sets new symbols to their default value but not 'n') with 
the
@@ -97,6 +97,13 @@ oldnoconfig: olddefconfig
@echo "  WARNING: \"oldnoconfig\" target will be removed after Linux 
4.19"
@echo "Please use \"olddefconfig\" instead, which is an 
alias."
 
+# We do not expect manual invokcation of "silentoldcofig" (or "syncconfig").
+silentoldconfig: syncconfig
+   @echo "  WARNING: \"silentoldconfig\" has been renamed to 
\"syncconfig\""
+   @echo "and is now an internal implementation detail."
+   @echo "What you want is probably \"oldconfig\"."
+   @echo "\"silentoldconfig\" will be removed after Linux 4.19"
+
 savedefconfig: $(obj)/conf
$< $(silent) --$@=defconfig $(Kconfig)
 
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 11a4e45..4e08121 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -23,7 +23,7 @@ static void check_conf(struct menu *menu);
 
 enum input_mode {
oldaskconfig,
-   silentoldconfig,
+   syncconfig,
oldconfig,
allnoconfig,
allyesconfig,

[PATCH v3 1/2] kconfig: invoke oldconfig instead of silentoldconfig from local*config

[Masahiro Yamada]

The purpose of local{yes,mod}config is to arrange the .config file
based on actually loaded modules.  It is unnecessary to update
include/generated/autoconf.h and include/config/* stuff here.
They will be updated as needed during the build.

Signed-off-by: Masahiro Yamada 
Reviewed-by: Ulf Magnusson 
---

Changes in v3:
  - Fix alignment of back-slashes

Changes in v2:
  - newly added

 scripts/kconfig/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index cb3ec53..41e2a9f 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -49,11 +49,11 @@ localyesconfig localmodconfig: $(obj)/streamline_config.pl 
$(obj)/conf
cmp -s .tmp.config .config ||   \
(mv -f .config .config.old.1;   \
 mv -f .tmp.config .config; \
-$(obj)/conf $(silent) --silentoldconfig $(Kconfig); \
+$(obj)/conf $(silent) --oldconfig $(Kconfig);  \
 mv -f .config.old.1 .config.old)   \
else\
mv -f .tmp.config .config;  \
-   $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \
+   $(obj)/conf $(silent) --oldconfig $(Kconfig);   \
fi
$(Q)rm -f .tmp.config
 
-- 
2.7.4

[PATCH v3 1/2] kconfig: invoke oldconfig instead of silentoldconfig from local*config

[Masahiro Yamada]

The purpose of local{yes,mod}config is to arrange the .config file
based on actually loaded modules.  It is unnecessary to update
include/generated/autoconf.h and include/config/* stuff here.
They will be updated as needed during the build.

Signed-off-by: Masahiro Yamada 
Reviewed-by: Ulf Magnusson 
---

Changes in v3:
  - Fix alignment of back-slashes

Changes in v2:
  - newly added

 scripts/kconfig/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index cb3ec53..41e2a9f 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -49,11 +49,11 @@ localyesconfig localmodconfig: $(obj)/streamline_config.pl 
$(obj)/conf
cmp -s .tmp.config .config ||   \
(mv -f .config .config.old.1;   \
 mv -f .tmp.config .config; \
-$(obj)/conf $(silent) --silentoldconfig $(Kconfig); \
+$(obj)/conf $(silent) --oldconfig $(Kconfig);  \
 mv -f .config.old.1 .config.old)   \
else\
mv -f .tmp.config .config;  \
-   $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \
+   $(obj)/conf $(silent) --oldconfig $(Kconfig);   \
fi
$(Q)rm -f .tmp.config
 
-- 
2.7.4

Re: [PATCH 1/5] staging: ks7010: Use constants from ieee80211_eid instead of literal ints.

[Tobin C. Harding]

On Wed, Feb 28, 2018 at 09:19:07PM -0800, Quytelda Kahja wrote:
...

I would normally respond to the cover letter but here goes.

Reviewed-by: Tobin C. Harding 

thanks,
Tobin.

[PATCH v3] hwmon: g762: handle cleanup with devm_add_action

[Peng Hao]

Simplify code and use devm_add_action() to handle cleanup.

Signed-off-by: Peng Hao 
---
 drivers/hwmon/g762.c | 58 +---
 1 file changed, 19 insertions(+), 39 deletions(-)

diff --git a/drivers/hwmon/g762.c b/drivers/hwmon/g762.c
index 6d1208b..0f2d468 100644
--- a/drivers/hwmon/g762.c
+++ b/drivers/hwmon/g762.c
@@ -128,7 +128,6 @@ enum g762_regs {
 G762_REG_FAN_CMD2_GEAR_MODE_1)) >> 2))
 
 struct g762_data {
-   struct device *hwmon_dev;
struct i2c_client *client;
struct clk *clk;
 
@@ -594,6 +593,17 @@ static int do_set_fan_startv(struct device *dev, unsigned 
long val)
  * call to g762_of_clock_disable(). Note that a reference to clock is kept
  * in our private data structure to be used in this function.
  */
+static void g762_of_clock_disable(void *data)
+{
+   struct g762_data *g762 = data;
+
+   if (!g762->clk)
+   return;
+
+   clk_disable_unprepare(g762->clk);
+   clk_put(g762->clk);
+}
+
 static int g762_of_clock_enable(struct i2c_client *client)
 {
struct g762_data *data;
@@ -625,7 +635,8 @@ static int g762_of_clock_enable(struct i2c_client *client)
 
data = i2c_get_clientdata(client);
data->clk = clk;
-
+
+   devm_add_action(>dev, g762_of_clock_disable, data);
return 0;
 
  clk_unprep:
@@ -637,17 +648,6 @@ static int g762_of_clock_enable(struct i2c_client *client)
return ret;
 }
 
-static void g762_of_clock_disable(struct i2c_client *client)
-{
-   struct g762_data *data = i2c_get_clientdata(client);
-
-   if (!data->clk)
-   return;
-
-   clk_disable_unprepare(data->clk);
-   clk_put(data->clk);
-}
-
 static int g762_of_prop_import_one(struct i2c_client *client,
   const char *pname,
   int (*psetter)(struct device *dev,
@@ -699,7 +699,7 @@ static int g762_of_clock_enable(struct i2c_client *client)
return 0;
 }
 
-static void g762_of_clock_disable(struct i2c_client *client) { }
+static void g762_of_clock_disable(void *data) { }
 #endif
 
 /*
@@ -1054,6 +1054,7 @@ static inline int g762_fan_init(struct device *dev)
 static int g762_probe(struct i2c_client *client, const struct i2c_device_id 
*id)
 {
struct device *dev = >dev;
+   struct device *hwmon_dev;
struct g762_data *data;
int ret;
 
@@ -1080,35 +1081,15 @@ static int g762_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
return ret;
ret = g762_of_prop_import(client);
if (ret)
-   goto clock_dis;
+   return ret;
/* ... or platform_data */
ret = g762_pdata_prop_import(client);
if (ret)
-   goto clock_dis;
+   return ret;
 
-   data->hwmon_dev = hwmon_device_register_with_groups(dev, client->name,
+   hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
data, g762_groups);
-   if (IS_ERR(data->hwmon_dev)) {
-   ret = PTR_ERR(data->hwmon_dev);
-   goto clock_dis;
-   }
-
-   return 0;
-
- clock_dis:
-   g762_of_clock_disable(client);
-
-   return ret;
-}
-
-static int g762_remove(struct i2c_client *client)
-{
-   struct g762_data *data = i2c_get_clientdata(client);
-
-   hwmon_device_unregister(data->hwmon_dev);
-   g762_of_clock_disable(client);
-
-   return 0;
+   return PTR_ERR_OR_ZERO(hwmon_dev);
 }
 
 static struct i2c_driver g762_driver = {
@@ -1117,7 +1098,6 @@ static int g762_remove(struct i2c_client *client)
.of_match_table = of_match_ptr(g762_dt_match),
},
.probe= g762_probe,
-   .remove   = g762_remove,
.id_table = g762_id,
 };
 
-- 
1.8.3.1

Re: [PATCH 1/5] staging: ks7010: Use constants from ieee80211_eid instead of literal ints.

[Tobin C. Harding]

On Wed, Feb 28, 2018 at 09:19:07PM -0800, Quytelda Kahja wrote:
...

I would normally respond to the cover letter but here goes.

Reviewed-by: Tobin C. Harding 

thanks,
Tobin.

[PATCH v3] hwmon: g762: handle cleanup with devm_add_action

[Peng Hao]

Simplify code and use devm_add_action() to handle cleanup.

Signed-off-by: Peng Hao 
---
 drivers/hwmon/g762.c | 58 +---
 1 file changed, 19 insertions(+), 39 deletions(-)

diff --git a/drivers/hwmon/g762.c b/drivers/hwmon/g762.c
index 6d1208b..0f2d468 100644
--- a/drivers/hwmon/g762.c
+++ b/drivers/hwmon/g762.c
@@ -128,7 +128,6 @@ enum g762_regs {
 G762_REG_FAN_CMD2_GEAR_MODE_1)) >> 2))
 
 struct g762_data {
-   struct device *hwmon_dev;
struct i2c_client *client;
struct clk *clk;
 
@@ -594,6 +593,17 @@ static int do_set_fan_startv(struct device *dev, unsigned 
long val)
  * call to g762_of_clock_disable(). Note that a reference to clock is kept
  * in our private data structure to be used in this function.
  */
+static void g762_of_clock_disable(void *data)
+{
+   struct g762_data *g762 = data;
+
+   if (!g762->clk)
+   return;
+
+   clk_disable_unprepare(g762->clk);
+   clk_put(g762->clk);
+}
+
 static int g762_of_clock_enable(struct i2c_client *client)
 {
struct g762_data *data;
@@ -625,7 +635,8 @@ static int g762_of_clock_enable(struct i2c_client *client)
 
data = i2c_get_clientdata(client);
data->clk = clk;
-
+
+   devm_add_action(>dev, g762_of_clock_disable, data);
return 0;
 
  clk_unprep:
@@ -637,17 +648,6 @@ static int g762_of_clock_enable(struct i2c_client *client)
return ret;
 }
 
-static void g762_of_clock_disable(struct i2c_client *client)
-{
-   struct g762_data *data = i2c_get_clientdata(client);
-
-   if (!data->clk)
-   return;
-
-   clk_disable_unprepare(data->clk);
-   clk_put(data->clk);
-}
-
 static int g762_of_prop_import_one(struct i2c_client *client,
   const char *pname,
   int (*psetter)(struct device *dev,
@@ -699,7 +699,7 @@ static int g762_of_clock_enable(struct i2c_client *client)
return 0;
 }
 
-static void g762_of_clock_disable(struct i2c_client *client) { }
+static void g762_of_clock_disable(void *data) { }
 #endif
 
 /*
@@ -1054,6 +1054,7 @@ static inline int g762_fan_init(struct device *dev)
 static int g762_probe(struct i2c_client *client, const struct i2c_device_id 
*id)
 {
struct device *dev = >dev;
+   struct device *hwmon_dev;
struct g762_data *data;
int ret;
 
@@ -1080,35 +1081,15 @@ static int g762_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
return ret;
ret = g762_of_prop_import(client);
if (ret)
-   goto clock_dis;
+   return ret;
/* ... or platform_data */
ret = g762_pdata_prop_import(client);
if (ret)
-   goto clock_dis;
+   return ret;
 
-   data->hwmon_dev = hwmon_device_register_with_groups(dev, client->name,
+   hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
data, g762_groups);
-   if (IS_ERR(data->hwmon_dev)) {
-   ret = PTR_ERR(data->hwmon_dev);
-   goto clock_dis;
-   }
-
-   return 0;
-
- clock_dis:
-   g762_of_clock_disable(client);
-
-   return ret;
-}
-
-static int g762_remove(struct i2c_client *client)
-{
-   struct g762_data *data = i2c_get_clientdata(client);
-
-   hwmon_device_unregister(data->hwmon_dev);
-   g762_of_clock_disable(client);
-
-   return 0;
+   return PTR_ERR_OR_ZERO(hwmon_dev);
 }
 
 static struct i2c_driver g762_driver = {
@@ -1117,7 +1098,6 @@ static int g762_remove(struct i2c_client *client)
.of_match_table = of_match_ptr(g762_dt_match),
},
.probe= g762_probe,
-   .remove   = g762_remove,
.id_table = g762_id,
 };
 
-- 
1.8.3.1

Re: [PATCH 1/5] staging: ks7010: Use constants from ieee80211_eid instead of literal ints.

[Tobin C. Harding]

On Wed, Feb 28, 2018 at 09:19:07PM -0800, Quytelda Kahja wrote:
> The case statement in get_ap_information() should not use literal integers
> to parse information element IDs when these values are provided by name
> in 'enum ieee80211_eid' in the header 'linux/ieee80211.h'.

Nice.  Magic number removal, I like it.

> Signed-off-by: Quytelda Kahja 
> ---
>  drivers/staging/ks7010/ks_hostif.c | 31 +++
>  drivers/staging/ks7010/ks_hostif.h |  1 +
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks_hostif.c 
> b/drivers/staging/ks7010/ks_hostif.c
> index 74a08417bd0b..67cf32433023 100644
> --- a/drivers/staging/ks7010/ks_hostif.c
> +++ b/drivers/staging/ks7010/ks_hostif.c
> @@ -251,9 +251,8 @@ int get_ap_information(struct ks_wlan_private *priv, 
> struct ap_info_t *ap_info,
>   offset = 0;
>  
>   while (bsize > offset) {
> - /* DPRINTK(4, "Element ID=%d\n",*bp); */

nit: Probably shouldn't remove debugging output in the same patch as
doing magic number removal.  (super nit-picky though.)

> - switch (*bp) {
> - case 0: /* ssid */
> + switch (*bp) { /* Information Element ID */
> + case WLAN_EID_SSID:
>   if (*(bp + 1) <= SSID_MAX_SIZE) {
>   ap->ssid.size = *(bp + 1);
>   } else {
> @@ -263,8 +262,8 @@ int get_ap_information(struct ks_wlan_private *priv, 
> struct ap_info_t *ap_info,
>   }
>   memcpy(ap->ssid.body, bp + 2, ap->ssid.size);
>   break;
> - case 1: /* rate */
> - case 50:/* ext rate */
> + case WLAN_EID_SUPP_RATES:
> + case WLAN_EID_EXT_SUPP_RATES:
>   if ((*(bp + 1) + ap->rate_set.size) <=
>   RATE_SET_MAX_SIZE) {
>   memcpy(>rate_set.body[ap->rate_set.size],
> @@ -280,9 +279,9 @@ int get_ap_information(struct ks_wlan_private *priv, 
> struct ap_info_t *ap_info,
>   (RATE_SET_MAX_SIZE - ap->rate_set.size);
>   }
>   break;
> - case 3: /* DS parameter */
> + case WLAN_EID_DS_PARAMS:
>   break;
> - case 48:/* RSN(WPA2) */
> + case WLAN_EID_RSN:
>   ap->rsn_ie.id = *bp;
>   if (*(bp + 1) <= RSN_IE_BODY_MAX) {
>   ap->rsn_ie.size = *(bp + 1);
> @@ -293,8 +292,8 @@ int get_ap_information(struct ks_wlan_private *priv, 
> struct ap_info_t *ap_info,
>   }
>   memcpy(ap->rsn_ie.body, bp + 2, ap->rsn_ie.size);
>   break;
> - case 221:   /* WPA */
> - if (memcmp(bp + 2, "\x00\x50\xf2\x01", 4) == 0) {   
> /* WPA OUI check */
> + case WLAN_EID_VENDOR_SPECIFIC: /* WPA */
> + if (memcmp(bp + 2, "\x00\x50\xf2\x01", 4) == 0) { /* 
> WPA OUI check */

Shouldn't have this line (unnecessary white space change)

>   ap->wpa_ie.id = *bp;
>   if (*(bp + 1) <= RSN_IE_BODY_MAX) {
>   ap->wpa_ie.size = *(bp + 1);
> @@ -309,18 +308,18 @@ int get_ap_information(struct ks_wlan_private *priv, 
> struct ap_info_t *ap_info,
>   }
>   break;
>  
> - case 2: /* FH parameter */
> - case 4: /* CF parameter */
> - case 5: /* TIM */
> - case 6: /* IBSS parameter */
> - case 7: /* Country */
> - case 42:/* ERP information */
> - case 47:/* Reserve ID 47 Broadcom AP */
> + case WLAN_EID_FH_PARAMS:
> + case WLAN_EID_CF_PARAMS:
> + case WLAN_EID_TIM:
> + case WLAN_EID_IBSS_PARAMS:
> + case WLAN_EID_COUNTRY:
> + case WLAN_EID_ERP_INFO:
>   break;
>   default:
>   DPRINTK(4, "unknown Element ID=%d\n", *bp);
>   break;
>   }
> +

FTR try not to include unrelated whitespace changes.  I see you changed
the case statement but the whitespace is after it and not really
related.  Again quite nit-picky but I'm guessing you are in staging to
learn so might as well learn it now than when you are patching the core
kernel :)


Good work,
Tobin.

Re: [PATCH 1/5] staging: ks7010: Use constants from ieee80211_eid instead of literal ints.

[Tobin C. Harding]

On Wed, Feb 28, 2018 at 09:19:07PM -0800, Quytelda Kahja wrote:
> The case statement in get_ap_information() should not use literal integers
> to parse information element IDs when these values are provided by name
> in 'enum ieee80211_eid' in the header 'linux/ieee80211.h'.

Nice.  Magic number removal, I like it.

> Signed-off-by: Quytelda Kahja 
> ---
>  drivers/staging/ks7010/ks_hostif.c | 31 +++
>  drivers/staging/ks7010/ks_hostif.h |  1 +
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks_hostif.c 
> b/drivers/staging/ks7010/ks_hostif.c
> index 74a08417bd0b..67cf32433023 100644
> --- a/drivers/staging/ks7010/ks_hostif.c
> +++ b/drivers/staging/ks7010/ks_hostif.c
> @@ -251,9 +251,8 @@ int get_ap_information(struct ks_wlan_private *priv, 
> struct ap_info_t *ap_info,
>   offset = 0;
>  
>   while (bsize > offset) {
> - /* DPRINTK(4, "Element ID=%d\n",*bp); */

nit: Probably shouldn't remove debugging output in the same patch as
doing magic number removal.  (super nit-picky though.)

> - switch (*bp) {
> - case 0: /* ssid */
> + switch (*bp) { /* Information Element ID */
> + case WLAN_EID_SSID:
>   if (*(bp + 1) <= SSID_MAX_SIZE) {
>   ap->ssid.size = *(bp + 1);
>   } else {
> @@ -263,8 +262,8 @@ int get_ap_information(struct ks_wlan_private *priv, 
> struct ap_info_t *ap_info,
>   }
>   memcpy(ap->ssid.body, bp + 2, ap->ssid.size);
>   break;
> - case 1: /* rate */
> - case 50:/* ext rate */
> + case WLAN_EID_SUPP_RATES:
> + case WLAN_EID_EXT_SUPP_RATES:
>   if ((*(bp + 1) + ap->rate_set.size) <=
>   RATE_SET_MAX_SIZE) {
>   memcpy(>rate_set.body[ap->rate_set.size],
> @@ -280,9 +279,9 @@ int get_ap_information(struct ks_wlan_private *priv, 
> struct ap_info_t *ap_info,
>   (RATE_SET_MAX_SIZE - ap->rate_set.size);
>   }
>   break;
> - case 3: /* DS parameter */
> + case WLAN_EID_DS_PARAMS:
>   break;
> - case 48:/* RSN(WPA2) */
> + case WLAN_EID_RSN:
>   ap->rsn_ie.id = *bp;
>   if (*(bp + 1) <= RSN_IE_BODY_MAX) {
>   ap->rsn_ie.size = *(bp + 1);
> @@ -293,8 +292,8 @@ int get_ap_information(struct ks_wlan_private *priv, 
> struct ap_info_t *ap_info,
>   }
>   memcpy(ap->rsn_ie.body, bp + 2, ap->rsn_ie.size);
>   break;
> - case 221:   /* WPA */
> - if (memcmp(bp + 2, "\x00\x50\xf2\x01", 4) == 0) {   
> /* WPA OUI check */
> + case WLAN_EID_VENDOR_SPECIFIC: /* WPA */
> + if (memcmp(bp + 2, "\x00\x50\xf2\x01", 4) == 0) { /* 
> WPA OUI check */

Shouldn't have this line (unnecessary white space change)

>   ap->wpa_ie.id = *bp;
>   if (*(bp + 1) <= RSN_IE_BODY_MAX) {
>   ap->wpa_ie.size = *(bp + 1);
> @@ -309,18 +308,18 @@ int get_ap_information(struct ks_wlan_private *priv, 
> struct ap_info_t *ap_info,
>   }
>   break;
>  
> - case 2: /* FH parameter */
> - case 4: /* CF parameter */
> - case 5: /* TIM */
> - case 6: /* IBSS parameter */
> - case 7: /* Country */
> - case 42:/* ERP information */
> - case 47:/* Reserve ID 47 Broadcom AP */
> + case WLAN_EID_FH_PARAMS:
> + case WLAN_EID_CF_PARAMS:
> + case WLAN_EID_TIM:
> + case WLAN_EID_IBSS_PARAMS:
> + case WLAN_EID_COUNTRY:
> + case WLAN_EID_ERP_INFO:
>   break;
>   default:
>   DPRINTK(4, "unknown Element ID=%d\n", *bp);
>   break;
>   }
> +

FTR try not to include unrelated whitespace changes.  I see you changed
the case statement but the whitespace is after it and not really
related.  Again quite nit-picky but I'm guessing you are in staging to
learn so might as well learn it now than when you are patching the core
kernel :)


Good work,
Tobin.

[PATCH v4 0/3] mm: improve zone->lock scalability

[Aaron Lu]

Patch 1/3 is a small cleanup suggested by Matthew Wilcox which doesn't
affect scalability or performance;
Patch 2/3 moves some code in free_pcppages_bulk() outside of zone->lock
and has Mel Gorman's ack;
Patch 3/3 uses prefetch in free_pcppages_bulk() outside of zone->lock to
speedup page merging under zone->lock but Mel Gorman has some concerns.

For details, please see their changelogs.

v4:
Address David Rientjes' comments to not update pcp->count in front of
free_pcppages_bulk() in patch 1/3.
Reword code comments in patch 2/3 as suggested by David Rientjes.

v3:
Added patch 1/3 to update pcp->count inside of free_pcppages_bulk();
Rebase to v4.16-rc2.

v2 & v1:
https://lkml.org/lkml/2018/1/23/879

Aaron Lu (3):
  mm/free_pcppages_bulk: update pcp->count inside
  mm/free_pcppages_bulk: do not hold lock when picking pages to free
  mm/free_pcppages_bulk: prefetch buddy while not holding lock

 mm/page_alloc.c | 62 +
 1 file changed, 40 insertions(+), 22 deletions(-)

-- 
2.14.3

[PATCH v4 0/3] mm: improve zone->lock scalability

[Aaron Lu]

Patch 1/3 is a small cleanup suggested by Matthew Wilcox which doesn't
affect scalability or performance;
Patch 2/3 moves some code in free_pcppages_bulk() outside of zone->lock
and has Mel Gorman's ack;
Patch 3/3 uses prefetch in free_pcppages_bulk() outside of zone->lock to
speedup page merging under zone->lock but Mel Gorman has some concerns.

For details, please see their changelogs.

v4:
Address David Rientjes' comments to not update pcp->count in front of
free_pcppages_bulk() in patch 1/3.
Reword code comments in patch 2/3 as suggested by David Rientjes.

v3:
Added patch 1/3 to update pcp->count inside of free_pcppages_bulk();
Rebase to v4.16-rc2.

v2 & v1:
https://lkml.org/lkml/2018/1/23/879

Aaron Lu (3):
  mm/free_pcppages_bulk: update pcp->count inside
  mm/free_pcppages_bulk: do not hold lock when picking pages to free
  mm/free_pcppages_bulk: prefetch buddy while not holding lock

 mm/page_alloc.c | 62 +
 1 file changed, 40 insertions(+), 22 deletions(-)

-- 
2.14.3

[PATCH v4 2/3] mm/free_pcppages_bulk: do not hold lock when picking pages to free

[Aaron Lu]

When freeing a batch of pages from Per-CPU-Pages(PCP) back to buddy,
the zone->lock is held and then pages are chosen from PCP's migratetype
list. While there is actually no need to do this 'choose part' under
lock since it's PCP pages, the only CPU that can touch them is us and
irq is also disabled.

Moving this part outside could reduce lock held time and improve
performance. Test with will-it-scale/page_fault1 full load:

kernel  Broadwell(2S)  Skylake(2S)   Broadwell(4S)  Skylake(4S)
v4.16-rc2+  90342157971818   13667135   15677465
this patch  9536374 +5.6%  8314710 +4.3% 14070408 +3.0% 16675866 +6.4%

What the test does is: starts $nr_cpu processes and each will repeatedly
do the following for 5 minutes:
1 mmap 128M anonymouse space;
2 write access to that space;
3 munmap.
The score is the aggregated iteration.

https://github.com/antonblanchard/will-it-scale/blob/master/tests/page_fault1.c

Acked-by: Mel Gorman 
Signed-off-by: Aaron Lu 
---
 mm/page_alloc.c | 39 +++
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index faa33eac1635..dafdcdec9c1f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1116,12 +1116,10 @@ static void free_pcppages_bulk(struct zone *zone, int 
count,
int migratetype = 0;
int batch_free = 0;
bool isolated_pageblocks;
-
-   spin_lock(>lock);
-   isolated_pageblocks = has_isolate_pageblock(zone);
+   struct page *page, *tmp;
+   LIST_HEAD(head);
 
while (count) {
-   struct page *page;
struct list_head *list;
 
/*
@@ -1143,27 +1141,36 @@ static void free_pcppages_bulk(struct zone *zone, int 
count,
batch_free = count;
 
do {
-   int mt; /* migratetype of the to-be-freed page */
-
page = list_last_entry(list, struct page, lru);
-   /* must delete as __free_one_page list manipulates */
+   /* must delete to avoid corrupting pcp list */
list_del(>lru);
pcp->count--;
 
-   mt = get_pcppage_migratetype(page);
-   /* MIGRATE_ISOLATE page should not go to pcplists */
-   VM_BUG_ON_PAGE(is_migrate_isolate(mt), page);
-   /* Pageblock could have been isolated meanwhile */
-   if (unlikely(isolated_pageblocks))
-   mt = get_pageblock_migratetype(page);
-
if (bulkfree_pcp_prepare(page))
continue;
 
-   __free_one_page(page, page_to_pfn(page), zone, 0, mt);
-   trace_mm_page_pcpu_drain(page, 0, mt);
+   list_add_tail(>lru, );
} while (--count && --batch_free && !list_empty(list));
}
+
+   spin_lock(>lock);
+   isolated_pageblocks = has_isolate_pageblock(zone);
+
+   /*
+* Use safe version since after __free_one_page(),
+* page->lru.next will not point to original list.
+*/
+   list_for_each_entry_safe(page, tmp, , lru) {
+   int mt = get_pcppage_migratetype(page);
+   /* MIGRATE_ISOLATE page should not go to pcplists */
+   VM_BUG_ON_PAGE(is_migrate_isolate(mt), page);
+   /* Pageblock could have been isolated meanwhile */
+   if (unlikely(isolated_pageblocks))
+   mt = get_pageblock_migratetype(page);
+
+   __free_one_page(page, page_to_pfn(page), zone, 0, mt);
+   trace_mm_page_pcpu_drain(page, 0, mt);
+   }
spin_unlock(>lock);
 }
 
-- 
2.14.3

[PATCH v4 2/3] mm/free_pcppages_bulk: do not hold lock when picking pages to free

[Aaron Lu]

When freeing a batch of pages from Per-CPU-Pages(PCP) back to buddy,
the zone->lock is held and then pages are chosen from PCP's migratetype
list. While there is actually no need to do this 'choose part' under
lock since it's PCP pages, the only CPU that can touch them is us and
irq is also disabled.

Moving this part outside could reduce lock held time and improve
performance. Test with will-it-scale/page_fault1 full load:

kernel  Broadwell(2S)  Skylake(2S)   Broadwell(4S)  Skylake(4S)
v4.16-rc2+  90342157971818   13667135   15677465
this patch  9536374 +5.6%  8314710 +4.3% 14070408 +3.0% 16675866 +6.4%

What the test does is: starts $nr_cpu processes and each will repeatedly
do the following for 5 minutes:
1 mmap 128M anonymouse space;
2 write access to that space;
3 munmap.
The score is the aggregated iteration.

https://github.com/antonblanchard/will-it-scale/blob/master/tests/page_fault1.c

Acked-by: Mel Gorman 
Signed-off-by: Aaron Lu 
---
 mm/page_alloc.c | 39 +++
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index faa33eac1635..dafdcdec9c1f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1116,12 +1116,10 @@ static void free_pcppages_bulk(struct zone *zone, int 
count,
int migratetype = 0;
int batch_free = 0;
bool isolated_pageblocks;
-
-   spin_lock(>lock);
-   isolated_pageblocks = has_isolate_pageblock(zone);
+   struct page *page, *tmp;
+   LIST_HEAD(head);
 
while (count) {
-   struct page *page;
struct list_head *list;
 
/*
@@ -1143,27 +1141,36 @@ static void free_pcppages_bulk(struct zone *zone, int 
count,
batch_free = count;
 
do {
-   int mt; /* migratetype of the to-be-freed page */
-
page = list_last_entry(list, struct page, lru);
-   /* must delete as __free_one_page list manipulates */
+   /* must delete to avoid corrupting pcp list */
list_del(>lru);
pcp->count--;
 
-   mt = get_pcppage_migratetype(page);
-   /* MIGRATE_ISOLATE page should not go to pcplists */
-   VM_BUG_ON_PAGE(is_migrate_isolate(mt), page);
-   /* Pageblock could have been isolated meanwhile */
-   if (unlikely(isolated_pageblocks))
-   mt = get_pageblock_migratetype(page);
-
if (bulkfree_pcp_prepare(page))
continue;
 
-   __free_one_page(page, page_to_pfn(page), zone, 0, mt);
-   trace_mm_page_pcpu_drain(page, 0, mt);
+   list_add_tail(>lru, );
} while (--count && --batch_free && !list_empty(list));
}
+
+   spin_lock(>lock);
+   isolated_pageblocks = has_isolate_pageblock(zone);
+
+   /*
+* Use safe version since after __free_one_page(),
+* page->lru.next will not point to original list.
+*/
+   list_for_each_entry_safe(page, tmp, , lru) {
+   int mt = get_pcppage_migratetype(page);
+   /* MIGRATE_ISOLATE page should not go to pcplists */
+   VM_BUG_ON_PAGE(is_migrate_isolate(mt), page);
+   /* Pageblock could have been isolated meanwhile */
+   if (unlikely(isolated_pageblocks))
+   mt = get_pageblock_migratetype(page);
+
+   __free_one_page(page, page_to_pfn(page), zone, 0, mt);
+   trace_mm_page_pcpu_drain(page, 0, mt);
+   }
spin_unlock(>lock);
 }
 
-- 
2.14.3

[PATCH v4 3/3] mm/free_pcppages_bulk: prefetch buddy while not holding lock

[Aaron Lu]

When a page is freed back to the global pool, its buddy will be checked
to see if it's possible to do a merge. This requires accessing buddy's
page structure and that access could take a long time if it's cache cold.

This patch adds a prefetch to the to-be-freed page's buddy outside of
zone->lock in hope of accessing buddy's page structure later under
zone->lock will be faster. Since we *always* do buddy merging and check
an order-0 page's buddy to try to merge it when it goes into the main
allocator, the cacheline will always come in, i.e. the prefetched data
will never be unused.

In the meantime, there are two concerns:
1 the prefetch could potentially evict existing cachelines, especially
  for L1D cache since it is not huge;
2 there is some additional instruction overhead, namely calculating
  buddy pfn twice.

For 1, it's hard to say, this microbenchmark though shows good result but
the actual benefit of this patch will be workload/CPU dependant;
For 2, since the calculation is a XOR on two local variables, it's expected
in many cases that cycles spent will be offset by reduced memory latency
later. This is especially true for NUMA machines where multiple CPUs are
contending on zone->lock and the most time consuming part under zone->lock
is the wait of 'struct page' cacheline of the to-be-freed pages and their
buddies.

Test with will-it-scale/page_fault1 full load:

kernel  Broadwell(2S)  Skylake(2S)   Broadwell(4S)  Skylake(4S)
v4.16-rc2+  90342157971818   13667135   15677465
patch2/39536374 +5.6%  8314710 +4.3% 14070408 +3.0% 16675866 +6.4%
this patch 10338868 +8.4%  8544477 +2.8% 14839808 +5.5% 17155464 +2.9%
Note: this patch's performance improvement percent is against patch2/3.

[changelog stole from Dave Hansen and Mel Gorman's comments]
https://lkml.org/lkml/2018/1/24/551
Suggested-by: Ying Huang 
Signed-off-by: Aaron Lu 
---
 mm/page_alloc.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index dafdcdec9c1f..1d838041931e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1141,6 +1141,9 @@ static void free_pcppages_bulk(struct zone *zone, int 
count,
batch_free = count;
 
do {
+   unsigned long pfn, buddy_pfn;
+   struct page *buddy;
+
page = list_last_entry(list, struct page, lru);
/* must delete to avoid corrupting pcp list */
list_del(>lru);
@@ -1150,6 +1153,18 @@ static void free_pcppages_bulk(struct zone *zone, int 
count,
continue;
 
list_add_tail(>lru, );
+
+   /*
+* We are going to put the page back to the global
+* pool, prefetch its buddy to speed up later access
+* under zone->lock. It is believed the overhead of
+* calculating buddy_pfn here can be offset by reduced
+* memory latency later.
+*/
+   pfn = page_to_pfn(page);
+   buddy_pfn = __find_buddy_pfn(pfn, 0);
+   buddy = page + (buddy_pfn - pfn);
+   prefetch(buddy);
} while (--count && --batch_free && !list_empty(list));
}
 
-- 
2.14.3

[PATCH v4 3/3] mm/free_pcppages_bulk: prefetch buddy while not holding lock

[Aaron Lu]

When a page is freed back to the global pool, its buddy will be checked
to see if it's possible to do a merge. This requires accessing buddy's
page structure and that access could take a long time if it's cache cold.

This patch adds a prefetch to the to-be-freed page's buddy outside of
zone->lock in hope of accessing buddy's page structure later under
zone->lock will be faster. Since we *always* do buddy merging and check
an order-0 page's buddy to try to merge it when it goes into the main
allocator, the cacheline will always come in, i.e. the prefetched data
will never be unused.

In the meantime, there are two concerns:
1 the prefetch could potentially evict existing cachelines, especially
  for L1D cache since it is not huge;
2 there is some additional instruction overhead, namely calculating
  buddy pfn twice.

For 1, it's hard to say, this microbenchmark though shows good result but
the actual benefit of this patch will be workload/CPU dependant;
For 2, since the calculation is a XOR on two local variables, it's expected
in many cases that cycles spent will be offset by reduced memory latency
later. This is especially true for NUMA machines where multiple CPUs are
contending on zone->lock and the most time consuming part under zone->lock
is the wait of 'struct page' cacheline of the to-be-freed pages and their
buddies.

Test with will-it-scale/page_fault1 full load:

kernel  Broadwell(2S)  Skylake(2S)   Broadwell(4S)  Skylake(4S)
v4.16-rc2+  90342157971818   13667135   15677465
patch2/39536374 +5.6%  8314710 +4.3% 14070408 +3.0% 16675866 +6.4%
this patch 10338868 +8.4%  8544477 +2.8% 14839808 +5.5% 17155464 +2.9%
Note: this patch's performance improvement percent is against patch2/3.

[changelog stole from Dave Hansen and Mel Gorman's comments]
https://lkml.org/lkml/2018/1/24/551
Suggested-by: Ying Huang 
Signed-off-by: Aaron Lu 
---
 mm/page_alloc.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index dafdcdec9c1f..1d838041931e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1141,6 +1141,9 @@ static void free_pcppages_bulk(struct zone *zone, int 
count,
batch_free = count;
 
do {
+   unsigned long pfn, buddy_pfn;
+   struct page *buddy;
+
page = list_last_entry(list, struct page, lru);
/* must delete to avoid corrupting pcp list */
list_del(>lru);
@@ -1150,6 +1153,18 @@ static void free_pcppages_bulk(struct zone *zone, int 
count,
continue;
 
list_add_tail(>lru, );
+
+   /*
+* We are going to put the page back to the global
+* pool, prefetch its buddy to speed up later access
+* under zone->lock. It is believed the overhead of
+* calculating buddy_pfn here can be offset by reduced
+* memory latency later.
+*/
+   pfn = page_to_pfn(page);
+   buddy_pfn = __find_buddy_pfn(pfn, 0);
+   buddy = page + (buddy_pfn - pfn);
+   prefetch(buddy);
} while (--count && --batch_free && !list_empty(list));
}
 
-- 
2.14.3

[PATCH v4 1/3] mm/free_pcppages_bulk: update pcp->count inside

[Aaron Lu]

Matthew Wilcox found that all callers of free_pcppages_bulk() currently
update pcp->count immediately after so it's natural to do it inside
free_pcppages_bulk().

No functionality or performance change is expected from this patch.

Suggested-by: Matthew Wilcox 
Signed-off-by: Aaron Lu 
---
 mm/page_alloc.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cb416723538f..faa33eac1635 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1148,6 +1148,7 @@ static void free_pcppages_bulk(struct zone *zone, int 
count,
page = list_last_entry(list, struct page, lru);
/* must delete as __free_one_page list manipulates */
list_del(>lru);
+   pcp->count--;
 
mt = get_pcppage_migratetype(page);
/* MIGRATE_ISOLATE page should not go to pcplists */
@@ -2416,10 +2417,8 @@ void drain_zone_pages(struct zone *zone, struct 
per_cpu_pages *pcp)
local_irq_save(flags);
batch = READ_ONCE(pcp->batch);
to_drain = min(pcp->count, batch);
-   if (to_drain > 0) {
+   if (to_drain > 0)
free_pcppages_bulk(zone, to_drain, pcp);
-   pcp->count -= to_drain;
-   }
local_irq_restore(flags);
 }
 #endif
@@ -2441,10 +2440,8 @@ static void drain_pages_zone(unsigned int cpu, struct 
zone *zone)
pset = per_cpu_ptr(zone->pageset, cpu);
 
pcp = >pcp;
-   if (pcp->count) {
+   if (pcp->count)
free_pcppages_bulk(zone, pcp->count, pcp);
-   pcp->count = 0;
-   }
local_irq_restore(flags);
 }
 
@@ -2668,7 +2665,6 @@ static void free_unref_page_commit(struct page *page, 
unsigned long pfn)
if (pcp->count >= pcp->high) {
unsigned long batch = READ_ONCE(pcp->batch);
free_pcppages_bulk(zone, batch, pcp);
-   pcp->count -= batch;
}
 }
 
-- 
2.14.3

[PATCH v4 1/3] mm/free_pcppages_bulk: update pcp->count inside

[Aaron Lu]

Matthew Wilcox found that all callers of free_pcppages_bulk() currently
update pcp->count immediately after so it's natural to do it inside
free_pcppages_bulk().

No functionality or performance change is expected from this patch.

Suggested-by: Matthew Wilcox 
Signed-off-by: Aaron Lu 
---
 mm/page_alloc.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cb416723538f..faa33eac1635 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1148,6 +1148,7 @@ static void free_pcppages_bulk(struct zone *zone, int 
count,
page = list_last_entry(list, struct page, lru);
/* must delete as __free_one_page list manipulates */
list_del(>lru);
+   pcp->count--;
 
mt = get_pcppage_migratetype(page);
/* MIGRATE_ISOLATE page should not go to pcplists */
@@ -2416,10 +2417,8 @@ void drain_zone_pages(struct zone *zone, struct 
per_cpu_pages *pcp)
local_irq_save(flags);
batch = READ_ONCE(pcp->batch);
to_drain = min(pcp->count, batch);
-   if (to_drain > 0) {
+   if (to_drain > 0)
free_pcppages_bulk(zone, to_drain, pcp);
-   pcp->count -= to_drain;
-   }
local_irq_restore(flags);
 }
 #endif
@@ -2441,10 +2440,8 @@ static void drain_pages_zone(unsigned int cpu, struct 
zone *zone)
pset = per_cpu_ptr(zone->pageset, cpu);
 
pcp = >pcp;
-   if (pcp->count) {
+   if (pcp->count)
free_pcppages_bulk(zone, pcp->count, pcp);
-   pcp->count = 0;
-   }
local_irq_restore(flags);
 }
 
@@ -2668,7 +2665,6 @@ static void free_unref_page_commit(struct page *page, 
unsigned long pfn)
if (pcp->count >= pcp->high) {
unsigned long batch = READ_ONCE(pcp->batch);
free_pcppages_bulk(zone, batch, pcp);
-   pcp->count -= batch;
}
 }
 
-- 
2.14.3

Re: [PATCH v4 13/24] fpga: region: add compat_id support

[Wu Hao]

On Wed, Feb 28, 2018 at 04:55:15PM -0600, Alan Tull wrote:
> On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao  wrote:
> 
> Hi Hao,

Hi Alan,

Thanks for the review.

> 
> > This patch introduces a compat_id member and sysfs interface for each
> > fpga-region, e.g userspace applications could read the compat_id
> > from the sysfs interface for compatibility checking before PR.
> >
> > Signed-off-by: Wu Hao 
> > ---
> >  Documentation/ABI/testing/sysfs-class-fpga-region |  5 +
> >  drivers/fpga/fpga-region.c| 19 +++
> >  include/linux/fpga/fpga-region.h  | 13 +
> >  3 files changed, 37 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-class-fpga-region
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-fpga-region 
> > b/Documentation/ABI/testing/sysfs-class-fpga-region
> > new file mode 100644
> > index 000..419d930
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-class-fpga-region
> > @@ -0,0 +1,5 @@
> > +What:  /sys/class/fpga_region//compat_id
> > +Date:  February 2018
> > +KernelVersion: 4.16
> > +Contact:   Wu Hao 
> > +Description:   FPGA region id for compatibility check.
> > diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> > index 660a91b..babec96 100644
> > --- a/drivers/fpga/fpga-region.c
> > +++ b/drivers/fpga/fpga-region.c
> > @@ -162,6 +162,24 @@ int fpga_region_program_fpga(struct fpga_region 
> > *region)
> >  }
> >  EXPORT_SYMBOL_GPL(fpga_region_program_fpga);
> >
> > +static ssize_t compat_id_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > +   struct fpga_region *region = to_fpga_region(dev);
> 
> This looks good, but not all users of FPGA are going to use compat_id.
> How would you feel about making it a pointer in struct fpga_region?
> With compat_id as a pointer, could check for non-null compat_id
> pointer and return an error if it wasn't initialized.

It sounds good to me.

if (!region->compat_id)
return -ENOENT;

> 
> > +
> > +   return sprintf(buf, "%016llx%016llx\n",
> > +  (unsigned long long)region->compat_id.id_h,
> > +  (unsigned long long)region->compat_id.id_l);
> > +}
> > +
> > +static DEVICE_ATTR_RO(compat_id);
> > +
> > +static struct attribute *fpga_region_attrs[] = {
> > +   _attr_compat_id.attr,
> > +   NULL,
> > +};
> > +ATTRIBUTE_GROUPS(fpga_region);
> > +
> >  int fpga_region_register(struct fpga_region *region)
> >  {
> > struct device *dev = region->parent;
> > @@ -226,6 +244,7 @@ static int __init fpga_region_init(void)
> > if (IS_ERR(fpga_region_class))
> > return PTR_ERR(fpga_region_class);
> >
> > +   fpga_region_class->dev_groups = fpga_region_groups;
> > fpga_region_class->dev_release = fpga_region_dev_release;
> >
> > return 0;
> > diff --git a/include/linux/fpga/fpga-region.h 
> > b/include/linux/fpga/fpga-region.h
> > index 423c87e..bf97dcc 100644
> > --- a/include/linux/fpga/fpga-region.h
> > +++ b/include/linux/fpga/fpga-region.h
> > @@ -6,6 +6,17 @@
> >  #include 
> >
> >  /**
> > + * struct fpga_region_compat_id - FPGA Region id for compatibility check
> > + *
> > + * @id_h: high 64bit of the compat_id
> > + * @id_l: low 64bit of the compat_id
> > + */
> > +struct fpga_region_compat_id {
> > +   u64 id_h;
> > +   u64 id_l;
> 
> I guess each user will choose how to define these bits.

Yes.

> 
> > +};
> > +
> > +/**
> >   * struct fpga_region - FPGA Region structure
> >   * @dev: FPGA Region device
> >   * @parent: parent device
> > @@ -13,6 +24,7 @@
> >   * @bridge_list: list of FPGA bridges specified in region
> >   * @mgr: FPGA manager
> >   * @info: FPGA image info
> > + * @compat_id: FPGA region id for compatibility check.
> >   * @priv: private data
> >   * @get_bridges: optional function to get bridges to a list
> >   * @groups: optional attribute groups.
> > @@ -24,6 +36,7 @@ struct fpga_region {
> > struct list_head bridge_list;
> > struct fpga_manager *mgr;
> > struct fpga_image_info *info;
> > +   struct fpga_region_compat_id compat_id;
> 
> Here it would be a pointer instead.

Yes. Will update this patch.

Thanks
Hao

> 
> Alan
> 
> > void *priv;
> > int (*get_bridges)(struct fpga_region *region);
> > const struct attribute_group **groups;
> > --
> > 2.7.4
> >

Re: [PATCH v4 13/24] fpga: region: add compat_id support

[Wu Hao]

On Wed, Feb 28, 2018 at 04:55:15PM -0600, Alan Tull wrote:
> On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao  wrote:
> 
> Hi Hao,

Hi Alan,

Thanks for the review.

> 
> > This patch introduces a compat_id member and sysfs interface for each
> > fpga-region, e.g userspace applications could read the compat_id
> > from the sysfs interface for compatibility checking before PR.
> >
> > Signed-off-by: Wu Hao 
> > ---
> >  Documentation/ABI/testing/sysfs-class-fpga-region |  5 +
> >  drivers/fpga/fpga-region.c| 19 +++
> >  include/linux/fpga/fpga-region.h  | 13 +
> >  3 files changed, 37 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-class-fpga-region
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-fpga-region 
> > b/Documentation/ABI/testing/sysfs-class-fpga-region
> > new file mode 100644
> > index 000..419d930
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-class-fpga-region
> > @@ -0,0 +1,5 @@
> > +What:  /sys/class/fpga_region//compat_id
> > +Date:  February 2018
> > +KernelVersion: 4.16
> > +Contact:   Wu Hao 
> > +Description:   FPGA region id for compatibility check.
> > diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> > index 660a91b..babec96 100644
> > --- a/drivers/fpga/fpga-region.c
> > +++ b/drivers/fpga/fpga-region.c
> > @@ -162,6 +162,24 @@ int fpga_region_program_fpga(struct fpga_region 
> > *region)
> >  }
> >  EXPORT_SYMBOL_GPL(fpga_region_program_fpga);
> >
> > +static ssize_t compat_id_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > +   struct fpga_region *region = to_fpga_region(dev);
> 
> This looks good, but not all users of FPGA are going to use compat_id.
> How would you feel about making it a pointer in struct fpga_region?
> With compat_id as a pointer, could check for non-null compat_id
> pointer and return an error if it wasn't initialized.

It sounds good to me.

if (!region->compat_id)
return -ENOENT;

> 
> > +
> > +   return sprintf(buf, "%016llx%016llx\n",
> > +  (unsigned long long)region->compat_id.id_h,
> > +  (unsigned long long)region->compat_id.id_l);
> > +}
> > +
> > +static DEVICE_ATTR_RO(compat_id);
> > +
> > +static struct attribute *fpga_region_attrs[] = {
> > +   _attr_compat_id.attr,
> > +   NULL,
> > +};
> > +ATTRIBUTE_GROUPS(fpga_region);
> > +
> >  int fpga_region_register(struct fpga_region *region)
> >  {
> > struct device *dev = region->parent;
> > @@ -226,6 +244,7 @@ static int __init fpga_region_init(void)
> > if (IS_ERR(fpga_region_class))
> > return PTR_ERR(fpga_region_class);
> >
> > +   fpga_region_class->dev_groups = fpga_region_groups;
> > fpga_region_class->dev_release = fpga_region_dev_release;
> >
> > return 0;
> > diff --git a/include/linux/fpga/fpga-region.h 
> > b/include/linux/fpga/fpga-region.h
> > index 423c87e..bf97dcc 100644
> > --- a/include/linux/fpga/fpga-region.h
> > +++ b/include/linux/fpga/fpga-region.h
> > @@ -6,6 +6,17 @@
> >  #include 
> >
> >  /**
> > + * struct fpga_region_compat_id - FPGA Region id for compatibility check
> > + *
> > + * @id_h: high 64bit of the compat_id
> > + * @id_l: low 64bit of the compat_id
> > + */
> > +struct fpga_region_compat_id {
> > +   u64 id_h;
> > +   u64 id_l;
> 
> I guess each user will choose how to define these bits.

Yes.

> 
> > +};
> > +
> > +/**
> >   * struct fpga_region - FPGA Region structure
> >   * @dev: FPGA Region device
> >   * @parent: parent device
> > @@ -13,6 +24,7 @@
> >   * @bridge_list: list of FPGA bridges specified in region
> >   * @mgr: FPGA manager
> >   * @info: FPGA image info
> > + * @compat_id: FPGA region id for compatibility check.
> >   * @priv: private data
> >   * @get_bridges: optional function to get bridges to a list
> >   * @groups: optional attribute groups.
> > @@ -24,6 +36,7 @@ struct fpga_region {
> > struct list_head bridge_list;
> > struct fpga_manager *mgr;
> > struct fpga_image_info *info;
> > +   struct fpga_region_compat_id compat_id;
> 
> Here it would be a pointer instead.

Yes. Will update this patch.

Thanks
Hao

> 
> Alan
> 
> > void *priv;
> > int (*get_bridges)(struct fpga_region *region);
> > const struct attribute_group **groups;
> > --
> > 2.7.4
> >

Re: [PATCH v2] pvcalls-front: 64-bit align flags

[Juergen Gross]

On 01/03/18 03:05, Stefano Stabellini wrote:
> We are using test_and_* operations on the status and flag fields of
> struct sock_mapping. However, these functions require the operand to be
> 64-bit aligned on arm64. Currently, only status is 64-bit aligned.
> 
> Make status and flags explicitly 64-bit aligned.
> 
> Signed-off-by: Stefano Stabellini 

Committed to xen/tip for-linus-4.16a


Juergen

Re: [PATCH v2] pvcalls-front: 64-bit align flags

[Juergen Gross]

On 01/03/18 03:05, Stefano Stabellini wrote:
> We are using test_and_* operations on the status and flag fields of
> struct sock_mapping. However, these functions require the operand to be
> 64-bit aligned on arm64. Currently, only status is 64-bit aligned.
> 
> Make status and flags explicitly 64-bit aligned.
> 
> Signed-off-by: Stefano Stabellini 

Committed to xen/tip for-linus-4.16a


Juergen

Re: [PATCH v3 1/6] jailhouse: Provide detection for non-x86 systems

[Juergen Gross]

On 01/03/18 06:40, Jan Kiszka wrote:
> From: Jan Kiszka 
> 
> Implement jailhouse_paravirt() via device tree probing on architectures
> != x86. Will be used by the PCI core.
> 
> CC: Rob Herring 
> CC: Mark Rutland 
> Signed-off-by: Jan Kiszka 
> ---
>  Documentation/devicetree/bindings/jailhouse.txt |  8 
>  arch/x86/include/asm/jailhouse_para.h   |  2 +-
>  include/linux/hypervisor.h  | 17 +++--

I'd appreciate if you'd Cc: the maintainers of the touched files...

Reviewed-by: Juergen Gross 


Juergen

Re: [PATCH v3 1/6] jailhouse: Provide detection for non-x86 systems

[Juergen Gross]

On 01/03/18 06:40, Jan Kiszka wrote:
> From: Jan Kiszka 
> 
> Implement jailhouse_paravirt() via device tree probing on architectures
> != x86. Will be used by the PCI core.
> 
> CC: Rob Herring 
> CC: Mark Rutland 
> Signed-off-by: Jan Kiszka 
> ---
>  Documentation/devicetree/bindings/jailhouse.txt |  8 
>  arch/x86/include/asm/jailhouse_para.h   |  2 +-
>  include/linux/hypervisor.h  | 17 +++--

I'd appreciate if you'd Cc: the maintainers of the touched files...

Reviewed-by: Juergen Gross 


Juergen

Re: [Xen-devel] [RFC PATCH v4 2/7] xen/pvh: Move PVH entry code out of Xen specific tree

[Juergen Gross]

On 28/02/18 22:35, Paolo Bonzini wrote:
> On 28/02/2018 22:08, Konrad Rzeszutek Wilk wrote:
>> +obj-$(CONFIG_XEN_PVH) += pvh.o
>> +obj-$(CONFIG_XEN_PVH) += pvh-head.o
>> +
> 
> Probably a better place for these would be
> arch/x86/platform/pvh/{enlighten.c,head.S}.  (Just because there are no
> .c or .S files in arch/x86).

Right.

> Maybe Xen ought to be moved under
> arch/x86/platform too.

And hyperv and kvm, too?

No, I think arch/x86/xen/ is fine.


Juergen

Re: [Xen-devel] [RFC PATCH v4 2/7] xen/pvh: Move PVH entry code out of Xen specific tree

[Juergen Gross]

On 28/02/18 22:35, Paolo Bonzini wrote:
> On 28/02/2018 22:08, Konrad Rzeszutek Wilk wrote:
>> +obj-$(CONFIG_XEN_PVH) += pvh.o
>> +obj-$(CONFIG_XEN_PVH) += pvh-head.o
>> +
> 
> Probably a better place for these would be
> arch/x86/platform/pvh/{enlighten.c,head.S}.  (Just because there are no
> .c or .S files in arch/x86).

Right.

> Maybe Xen ought to be moved under
> arch/x86/platform too.

And hyperv and kvm, too?

No, I think arch/x86/xen/ is fine.


Juergen

Re: [PATCH] arm: Initialize hrtimer-based broadcast clockevent

[Jan Kiszka]

On 2018-01-22 07:06, Jan Kiszka wrote:
> Analogously to 9358d755bd5c, this registers a broadcast clockevent in
> case no hardware broadcast timer is available and the per-CPU timers can
> be stopped in deep power states.
> 
> Partitions of the Jailhouse hypervisor fall in this category.
> Registering the workaround timer allows to enter high-resolution mode in
> that case.
> 
> Signed-off-by: Jan Kiszka 
> ---
>  arch/arm/kernel/time.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
> index 629f8e9981f1..0a45d861ef8e 100644
> --- a/arch/arm/kernel/time.c
> +++ b/arch/arm/kernel/time.c
> @@ -12,6 +12,7 @@
>   *  reading the RTC at bootup, etc...
>   */
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -121,5 +122,7 @@ void __init time_init(void)
>   of_clk_init(NULL);
>  #endif
>   timer_probe();
> +
> + tick_setup_hrtimer_broadcast();
>   }
>  }
> 

Gentle ping, just to avoid that this falls through the cracks because
it's so small.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] arm: Initialize hrtimer-based broadcast clockevent

[Jan Kiszka]

On 2018-01-22 07:06, Jan Kiszka wrote:
> Analogously to 9358d755bd5c, this registers a broadcast clockevent in
> case no hardware broadcast timer is available and the per-CPU timers can
> be stopped in deep power states.
> 
> Partitions of the Jailhouse hypervisor fall in this category.
> Registering the workaround timer allows to enter high-resolution mode in
> that case.
> 
> Signed-off-by: Jan Kiszka 
> ---
>  arch/arm/kernel/time.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
> index 629f8e9981f1..0a45d861ef8e 100644
> --- a/arch/arm/kernel/time.c
> +++ b/arch/arm/kernel/time.c
> @@ -12,6 +12,7 @@
>   *  reading the RTC at bootup, etc...
>   */
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -121,5 +122,7 @@ void __init time_init(void)
>   of_clk_init(NULL);
>  #endif
>   timer_probe();
> +
> + tick_setup_hrtimer_broadcast();
>   }
>  }
> 

Gentle ping, just to avoid that this falls through the cracks because
it's so small.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

[PATCH v5 1/3] x86/apic: Move pending intr check code into it's own function

[Dou Liyang]

the pending interrupt check code is mixed with the local APIC setup code,
that looks messy.

Extract the related code, move it into a new function named
apic_pending_intr_clear().

Signed-off-by: Dou Liyang 
Reviewed-by: Andy Shevchenko 
---
changelog:
 v4 --> v5:
  - Fix undeclared 'i' error reported by LKP

 arch/x86/kernel/apic/apic.c | 100 
 1 file changed, 55 insertions(+), 45 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 2ceac9f6c5d7..69d2936e3154 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1408,6 +1408,56 @@ static void lapic_setup_esr(void)
oldvalue, value);
 }
 
+static void apic_pending_intr_clear(void)
+{
+   long long max_loops = cpu_khz ? cpu_khz : 100;
+   unsigned long long tsc = 0, ntsc;
+   unsigned int value, queued;
+   int i, j, acked = 0;
+
+   if (boot_cpu_has(X86_FEATURE_TSC))
+   tsc = rdtsc();
+   /*
+* After a crash, we no longer service the interrupts and a pending
+* interrupt from previous kernel might still have ISR bit set.
+*
+* Most probably by now CPU has serviced that pending interrupt and
+* it might not have done the ack_APIC_irq() because it thought,
+* interrupt came from i8259 as ExtInt. LAPIC did not get EOI so it
+* does not clear the ISR bit and cpu thinks it has already serivced
+* the interrupt. Hence a vector might get locked. It was noticed
+* for timer irq (vector 0x31). Issue an extra EOI to clear ISR.
+*/
+   do {
+   queued = 0;
+   for (i = APIC_ISR_NR - 1; i >= 0; i--)
+   queued |= apic_read(APIC_IRR + i*0x10);
+
+   for (i = APIC_ISR_NR - 1; i >= 0; i--) {
+   value = apic_read(APIC_ISR + i*0x10);
+   for (j = 31; j >= 0; j--) {
+   if (value & (1< 256) {
+   printk(KERN_ERR "LAPIC pending interrupts after %d 
EOI\n",
+  acked);
+   break;
+   }
+   if (queued) {
+   if (boot_cpu_has(X86_FEATURE_TSC) && cpu_khz) {
+   ntsc = rdtsc();
+   max_loops = (cpu_khz << 10) - (ntsc - tsc);
+   } else
+   max_loops--;
+   }
+   } while (queued && max_loops > 0);
+   WARN_ON(max_loops <= 0);
+}
+
 /**
  * setup_local_APIC - setup the local APIC
  *
@@ -1417,13 +1467,11 @@ static void lapic_setup_esr(void)
 static void setup_local_APIC(void)
 {
int cpu = smp_processor_id();
-   unsigned int value, queued;
-   int i, j, acked = 0;
-   unsigned long long tsc = 0, ntsc;
-   long long max_loops = cpu_khz ? cpu_khz : 100;
+   unsigned int value;
+#ifdef CONFIG_X86_32
+   int i;
+#endif
 
-   if (boot_cpu_has(X86_FEATURE_TSC))
-   tsc = rdtsc();
 
if (disable_apic) {
disable_ioapic_support();
@@ -1475,45 +1523,7 @@ static void setup_local_APIC(void)
value &= ~APIC_TPRI_MASK;
apic_write(APIC_TASKPRI, value);
 
-   /*
-* After a crash, we no longer service the interrupts and a pending
-* interrupt from previous kernel might still have ISR bit set.
-*
-* Most probably by now CPU has serviced that pending interrupt and
-* it might not have done the ack_APIC_irq() because it thought,
-* interrupt came from i8259 as ExtInt. LAPIC did not get EOI so it
-* does not clear the ISR bit and cpu thinks it has already serivced
-* the interrupt. Hence a vector might get locked. It was noticed
-* for timer irq (vector 0x31). Issue an extra EOI to clear ISR.
-*/
-   do {
-   queued = 0;
-   for (i = APIC_ISR_NR - 1; i >= 0; i--)
-   queued |= apic_read(APIC_IRR + i*0x10);
-
-   for (i = APIC_ISR_NR - 1; i >= 0; i--) {
-   value = apic_read(APIC_ISR + i*0x10);
-   for (j = 31; j >= 0; j--) {
-   if (value & (1< 256) {
-   printk(KERN_ERR "LAPIC pending interrupts after %d 
EOI\n",
-  acked);
-   break;
-   }
-   

[PATCH v5 1/3] x86/apic: Move pending intr check code into it's own function

[Dou Liyang]

the pending interrupt check code is mixed with the local APIC setup code,
that looks messy.

Extract the related code, move it into a new function named
apic_pending_intr_clear().

Signed-off-by: Dou Liyang 
Reviewed-by: Andy Shevchenko 
---
changelog:
 v4 --> v5:
  - Fix undeclared 'i' error reported by LKP

 arch/x86/kernel/apic/apic.c | 100 
 1 file changed, 55 insertions(+), 45 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 2ceac9f6c5d7..69d2936e3154 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1408,6 +1408,56 @@ static void lapic_setup_esr(void)
oldvalue, value);
 }
 
+static void apic_pending_intr_clear(void)
+{
+   long long max_loops = cpu_khz ? cpu_khz : 100;
+   unsigned long long tsc = 0, ntsc;
+   unsigned int value, queued;
+   int i, j, acked = 0;
+
+   if (boot_cpu_has(X86_FEATURE_TSC))
+   tsc = rdtsc();
+   /*
+* After a crash, we no longer service the interrupts and a pending
+* interrupt from previous kernel might still have ISR bit set.
+*
+* Most probably by now CPU has serviced that pending interrupt and
+* it might not have done the ack_APIC_irq() because it thought,
+* interrupt came from i8259 as ExtInt. LAPIC did not get EOI so it
+* does not clear the ISR bit and cpu thinks it has already serivced
+* the interrupt. Hence a vector might get locked. It was noticed
+* for timer irq (vector 0x31). Issue an extra EOI to clear ISR.
+*/
+   do {
+   queued = 0;
+   for (i = APIC_ISR_NR - 1; i >= 0; i--)
+   queued |= apic_read(APIC_IRR + i*0x10);
+
+   for (i = APIC_ISR_NR - 1; i >= 0; i--) {
+   value = apic_read(APIC_ISR + i*0x10);
+   for (j = 31; j >= 0; j--) {
+   if (value & (1< 256) {
+   printk(KERN_ERR "LAPIC pending interrupts after %d 
EOI\n",
+  acked);
+   break;
+   }
+   if (queued) {
+   if (boot_cpu_has(X86_FEATURE_TSC) && cpu_khz) {
+   ntsc = rdtsc();
+   max_loops = (cpu_khz << 10) - (ntsc - tsc);
+   } else
+   max_loops--;
+   }
+   } while (queued && max_loops > 0);
+   WARN_ON(max_loops <= 0);
+}
+
 /**
  * setup_local_APIC - setup the local APIC
  *
@@ -1417,13 +1467,11 @@ static void lapic_setup_esr(void)
 static void setup_local_APIC(void)
 {
int cpu = smp_processor_id();
-   unsigned int value, queued;
-   int i, j, acked = 0;
-   unsigned long long tsc = 0, ntsc;
-   long long max_loops = cpu_khz ? cpu_khz : 100;
+   unsigned int value;
+#ifdef CONFIG_X86_32
+   int i;
+#endif
 
-   if (boot_cpu_has(X86_FEATURE_TSC))
-   tsc = rdtsc();
 
if (disable_apic) {
disable_ioapic_support();
@@ -1475,45 +1523,7 @@ static void setup_local_APIC(void)
value &= ~APIC_TPRI_MASK;
apic_write(APIC_TASKPRI, value);
 
-   /*
-* After a crash, we no longer service the interrupts and a pending
-* interrupt from previous kernel might still have ISR bit set.
-*
-* Most probably by now CPU has serviced that pending interrupt and
-* it might not have done the ack_APIC_irq() because it thought,
-* interrupt came from i8259 as ExtInt. LAPIC did not get EOI so it
-* does not clear the ISR bit and cpu thinks it has already serivced
-* the interrupt. Hence a vector might get locked. It was noticed
-* for timer irq (vector 0x31). Issue an extra EOI to clear ISR.
-*/
-   do {
-   queued = 0;
-   for (i = APIC_ISR_NR - 1; i >= 0; i--)
-   queued |= apic_read(APIC_IRR + i*0x10);
-
-   for (i = APIC_ISR_NR - 1; i >= 0; i--) {
-   value = apic_read(APIC_ISR + i*0x10);
-   for (j = 31; j >= 0; j--) {
-   if (value & (1< 256) {
-   printk(KERN_ERR "LAPIC pending interrupts after %d 
EOI\n",
-  acked);
-   break;
-   }
-   if (queued) {
-   if (boot_cpu_has(X86_FEATURE_TSC) && cpu_khz) {
-   ntsc = rdtsc();
-   max_loops = (cpu_khz << 10) - (ntsc - tsc);
-   } else
-   max_loops--;
-   }
-   } while (queued && max_loops > 0);
-   WARN_ON(max_loops <= 0);
+   apic_pending_intr_clear();
 
/*
 * Now that we are all set up, enable